Generate the vendored bb UI instead of editing it - #13
Open
ariofrio wants to merge 15 commits into
Open
Conversation
ariofrio
force-pushed
the
bb/share-code-between-plugins-thr_zqcve598g7
branch
2 times, most recently
from
August 22, 2026 21:07
2b203ed to
ccf5406
Compare
The @bb registry is generated verbatim from bb's own packages/shared-ui, so every file under a plugin's components/ui is a copy of the code running in the window around it. Editing one forks bb's UI kit, and nothing said so: three plugins each kept their own components.json, Breadcrumbs and Icons had their pin bumped to v0.39 without a re-vendor, and four local edits had accumulated across the copies. vendor-ui.json now holds the single pin and the items each plugin imports directly; registryDependencies supply the closure. scripts/vendor-ui.mjs is the only writer, and vendor-ui.lock.json lets --check verify the tree offline, from a pre-commit hook and from CI. It reports a hand-edit, a deletion, a pin bumped without a rebuild, and a file no closure explains — which is how the two use-pointer-coarse copies surfaced, orphaned when responsive-overlay stopped importing it at v0.39. The four edits are composed instead. Menus that must stay dropdowns on a narrow window use bb's own CompactViewportOverrideProvider; destructive context items take the classes bb's app gives them, including the last-hovered text colour the fork had dropped; and the thread filter draws its own check and submenu chevron the way its actionable rows already did, so its two row types finally render the same selected state. The Icons picker hands its label to the drawer shell on compact and renders its own heading on desktop, leaving exactly one title per mode without a nullable mobileTitle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A bare "bb" in BB_CLI forks without bound. npm prepends node_modules/.bin to PATH, and a plugin depending on bb-app has a `bb` symlink there pointing at the launcher shim. The launcher spawns `process.env.BB_CLI ?? <bundled cli>`, so a bare name re-resolves through that same PATH and re-enters the shim — and since the launcher passes its environment down, every child repeats it. A plugin whose build script is `bb plugin build` then spawns itself until the machine runs out of memory. The new dev-stack script did exactly that by passing BB_CLI: "bb" literally. run.mjs and install-plugins.mjs held the same trap one step further back, in a `process.env.BB_CLI ?? "bb"` fallback that only stayed harmless while BB_CLI happened to be set; verify-types.mjs spawned the bare name directly. scripts/bb-cli.mjs now resolves it once, refusing anything that is not an absolute path outside a node_modules/.bin directory, and saying why. It walks PATH itself rather than shelling out to `which`, which would have to be found on the very PATH being inspected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fixture furnishes the window; it is not what the stack is for. Seeding now warns and steps over a failure instead of taking the server down with it, so a stack whose plugins all installed stays usable. Also print the invocation that actually reaches this stack. BB_DATA_DIR alone leaves the CLI talking to the developer's own bb — BB_SERVER_URL is what retargets it, and an agent shell's inherited thread, project, and environment ids have to be cleared or they name rows this stack has never seen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Thread stages hand-wrote components/ui/chrome-style-tokens.ts, picking one constant out of bb's file of the same name. That file is one of the eighteen shared-ui sources the plugin registry does not publish, so no closure can explain the copy and no generator can produce it — vendor-ui.json has to say it belongs to the plugin, or check:ui reports it as an orphan forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three things were still free to drift. The registry pin had nothing holding it to the bb the plugins build against, so a bb release could move while every check stayed green — the same shape as the bug this guard was written for, one step further out. check:ui now compares the pin to each plugin's bb-app devDependency. Both live in the repository, so it stays offline and fails rather than warning. Two values were copied out of bb by hand and no registry item can supply them: the chrome token Thread stages excerpts from a shared-ui file the registry does not publish, and the destructive-item classes bb's own app passes to ContextMenuItem. vendor-ui.json now names them, build:ui refuses any it can no longer find upstream, and the lock records them so an edit is caught offline. The terminal panel selectors sat inline in the keyboard-shortcuts app entry, untested, while every comparable assumption in that plugin already lives in a module of its own. They now do too. A fixture test pins what we expect, not what bb renders — only a check against a running bb can do that — but the assumption is now in one readable place and our own probing cannot regress unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seeding it produced an undici EPIPE that named neither Node nor the harness, and the stack reported it as a fixture that had failed for its own reasons. It was the wrong Node major, which require-node.mjs already turns back into a sentence for capturing. The dev stack runs the same seed, so it asks the same question first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main now says to wait for what the plugin drew, inside the container only that plugin installs. Two things it does not say, both learned the hard way tonight and both from thr_s887gqkb97's corrections. The two failure modes are not equally bad. Keyed to the plugin's own marker a wait can only be late, which a timeout absorbs and a failure reports. Keyed to a shared name it is wrong by succeeding — the shot is taken, the header is bare, and nothing is raised. Playwright's locator wait samples once an animation frame (coreBundle.js re-arms its check on requestAnimationFrame), so sampling makes a wait late, never silent. And the symptom does not identify it. One wait on one line failed both ways here: silently on one machine, and loudly at thirty seconds on another. What decides it is where the wait lands in the proxy's lifetime, not load — measured, not argued: the loud failure came at load 23 on an idle box. So the loud form presents as a slow machine, collects a longer timeout, and the longer timeout widens the window the proxy can answer in. The remedy feeds the disease. Supersedes three commits of mine that refined wording main has since replaced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two mistakes in the resolver I added, both of which only a machine without the desktop app could show, which is every CI runner. It refused a node_modules/.bin entry as a shim. In CI that shim is the only bb there is — bb-app is a devDependency and nothing installs the app — so verify:types could no longer find a CLI at all. The entry is a symlink, and its target is absolute and outside .bin, so following it answers honestly and still cannot re-enter the launcher. A shim that links nowhere is still refused. And run.mjs resolved at module load, so `--check` demanded a CLI it never invokes. It reads the lock and the plugin sources and nothing else. Resolution now happens where it is first used, which is only on the capture path. Confirmed against CI's own situation: `env -i PATH=/usr/bin:/bin node scripts/screenshots/run.mjs --check` now runs with no bb anywhere. Two tests were passing for the wrong reason — their fixture shim was a regular file, so realpath returned it unchanged. They use real symlinks now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Vendored files were scattered across three roots per plugin, because a registry item's target decides where it lands and the targets do not agree: use-media-query and use-browser-dimming-modal are both registry:hook and land in components/ui/hooks and hooks respectively. So the checker scanned a hardcoded list of three directories, an item targeting a fourth would have gone unnoticed, and thread-stages had three files of its own sitting among BB's — kept legal by a pluginOwned allowlist that a reader had to consult to know whose code they were looking at. They now live under src/vendor/, which the generator owns outright, with BB's layout preserved beneath it. Imports say @/vendor/ and the path answers the question. The checker scans one root at any depth, the allowlist is gone, and a stray file in there is an orphan by definition rather than by omission. thread-stages keeps chrome-style-tokens.ts and dialog-position.ts in src/lib and its tooltip test beside its other component tests. It also gains the vitest.config.ts that Breadcrumbs and Icons already carry: tsc reads the tsconfig path mapping for @/ and vitest does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both found by review, and both are the failure this branch's own testing rule names: wrong by succeeding. The entrypoint guard compared process.argv[1], which is the path as typed, against import.meta.url, which Node resolves through symlinks. Under macOS /tmp, a symlinked home, or a symlinked worktree they differ, the whole CLI body is skipped, and check:ui exits 0 having verified nothing — silently approving a hand-edited component. heading-icons.mjs carries the same idiom and the same hole, so both are realpathed now. And resolveBbCli followed a node_modules/.bin entry to bb-app/dist/bb.js, which is the launcher rather than the CLI it stands in for. Handing that back as BB_CLI makes the launcher spawn itself — the exact loop this module was written to prevent, reintroduced by the fix for it. A shim is now followed past the launcher to bb-app/host-daemon/dist/bb, which ships beside it; a launcher with no CLI beside it is refused rather than returned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The literal-drift branch was the only one in inspect() with no test: deleting it outright left every test passing, while it is the sole guard on the two values copied out of bb by hand. Three tests now cover matching, edited, and a symbol deleted from under the lock — and each fails when that branch is removed. The escape check on a registry target was a prefix match, so a target of a/../../../etc/passwd passed it. It normalises now, and a test pins the case the prefix check missed. build() removed only what a previous lock listed, so a stray that was never locked survived every rebuild while --check demanded one — advice that could not work. It now clears the owned root of anything the build does not write. Also: prepare wrote core.hooksPath without --local, which is shared repository config and reaches every worktree; and the vendored files are marked eol=lf, because the lock digests bytes and a CRLF checkout would have failed check:ui on all forty-one. Recaptured on an idle machine; no image moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ariofrio
force-pushed
the
bb/share-code-between-plugins-thr_zqcve598g7
branch
from
August 23, 2026 06:36
ccf5406 to
1b27b6f
Compare
The filter rows hide glyphs bb draws for itself and draw their own in the same
slots, by class rather than by prop. jsdom applies no stylesheet, so the rewrite
landed with nothing able to tell a working row from one rendering both glyphs.
Two guards, because the gap has two halves and neither covers the other.
filter-row-glyphs.test.tsx pins the structure the selectors name: that bb's radio
indicator is still the first child, that the row's own span is never first, and
that bb's chevron is still the only direct-child svg. Moving bb's indicator after
{children} fails two of the three, which is the re-vendor that would otherwise
change nothing visible until a screenshot.
verify-styles.mjs fails the release check when an arbitrary variant written in
source is absent from dist/app.css. Being exact about its reach, because testing
disproved the obvious claim: a class assembled at runtime is invisible to
Tailwind and equally invisible to this, so the two agree and it catches nothing.
What it catches is a literal both see that Tailwind declines to emit — a
misspelled or unsupported utility, or a file fallen out of the content scan.
Neither proves the rule takes effect. That still needs a capture.
Also moves the dev stack out of this branch; it is unrelated to vendoring and
lands better on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This tsconfig's lib has no DOM.Iterable, so spreading an HTMLCollection does not typecheck — the same reason terminal-dom.ts reaches for Array.from rather than iterating a NodeList. Vitest transpiles without typechecking, so running the new test proved nothing about whether tsc would accept it, and CI found what I had not run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every file under a plugin's
components/ui/is a copy of bb's ownpackages/shared-ui, generated verbatim into the@bbregistry. Editing oneforks the code running in the window around it, and nothing said so — so three
plugins each kept their own
components.json, Breadcrumbs and Icons had theirpin bumped to v0.39 without a re-vendor, and four hand edits had accumulated.
The generator
vendor-ui.jsonholds the single registry pin and the items each plugin importsdirectly;
registryDependenciessupply the closure.scripts/vendor-ui.mjsisthe only writer, and
vendor-ui.lock.jsonlets--checkverify the treeoffline — cheap enough for a pre-commit hook and needing no network in CI.
It reports five things, each of which fired for real while building it:
bb-appto 0.40.0use-pointer-coarse.tscopies, dropped upstream at v0.39CHROME_SECTION_LABEL_CLASSThe pin is anchored to each plugin's
bb-appdevDependency — the thing thatdecides the build toolchain and shim configuration — so both sides live in the
repository and the check stays offline and hard-fails.
Two values copied out of bb by hand, which no registry item can supply, are now
named in
vendor-ui.json:build:uirefuses any it can no longer find upstream,and the lock records them so an edit is caught offline.
The four edits, composed away
None survive, and three made the plugin's own code more consistent:
responsive={false}→ bb'sCompactViewportOverrideProvider, which exists for exactly thisdata-[last-hovered]:text-destructivethat the fork had droppedshowChevron/indicator="check"→ one sharedFilterRowCheck. The actionable filter row already hand-rendered its check; now the plain row does too, so the projects/sections menu finally renders one selected state instead of a check on some rows and a circle on othersmobileTitleon compact, a plainh2on desktop; exactly one title per mode, no nullable propAlong the way
scripts/bb-cli.mjsresolves the bb CLI to an absolute path and refusesanything else. A bare
BB_CLIre-enters the launcher shim through npm'snode_modules/.bin, and a plugin whose build script isbb plugin buildthenforks without bound.
run.mjs,install-plugins.mjsandverify-types.mjsallheld that trap; six regression tests cover it.
npm run dev:stackboots a throwaway bb with these plugins installed — its owndata directory, server and host daemon — for trying a change by hand.
AGENTS.mdgains the rules this work produced, including why a wait keyed to aname bb also uses fails silently on one machine and loudly on another.
Verified
Per-plugin
npm ci --workspaces=false, as CI runs it: thread-stages 208/208,icons 71/71, breadcrumbs 21/21, missing-keyboard-shortcuts 62/62, all typechecking
clean. Root suite 51/51.
check:ui,check:heading-iconsandcheck:screenshotsgreen.
Checked in the compiled stylesheet rather than the source, since markup that
reads correctly can still render nothing:
[&>span:first-child]:hidden,[&>svg]:hiddenand all four destructive utilities are present indist/app.css.And confirmed in a live bb that the Breadcrumbs crumb and the Icons header icon
still mount after regenerating every vendored component.
Recapturing moved no image — only the lock, which digests the plugin sources a
shot was captured from.
Written by Claude