Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): create the SDUI manifest output directory, and stop blaming Playwright for every dump failure - #10162
Merged
Conversation
…laywright for every dump failure `scripts/gen-sdui-manifest.sh` assigned TARGET=packages/console/dist and nothing created it. That directory is gitignored and exists only after a successful scripts/build-console.sh, so on a tree whose console build is broken the run died `ENOENT ... sdui.manifest.json` -- after it had already paid for a vite dev server and a chromium launch. The ADR-0082 D4 ratchet does not even need the built dist (this script drives a vite DEV server over .cache/objectui-<sha>; dist/ is only where the manifest lands), so the ratchet was unrunnable precisely on the trees that most want an independent read on the registry. Two changes: 1. `mkdir -p "$TARGET"` runs with the other preconditions, before the dev server and the browser, so a write-side problem fails cheaply instead of at the end of a long run. 2. The dump's failure advice is now chosen from what failed. Every failure branch used to print `playwright install chromium-headless-shell`; in the measured failure the browser had launched and the dump had succeeded, and only the write failed, so that remedy sent the reader to the wrong layer -- and inside an agent dispatch container it cannot be followed at all (PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1, docs/releases-maintenance.md). The Playwright remedy now needs Playwright evidence; the fallback is "unclassified", never "Playwright". Pinned by packages/spec/scripts/gen-sdui-manifest-write-target.test.ts, which measures the ORDER (the failing precondition spawns zero dev servers) rather than grepping for `mkdir`, and feeds real failure text to the real classifier. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. |
This was referenced Aug 20, 2026
os-zhuang
marked this pull request as ready for review
August 20, 2026 12:49
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#10138
scripts/gen-sdui-manifest.shassignedTARGET="${FRAMEWORK_ROOT}/packages/console/dist"and nothing created it, while objectui's dumper callswriteFileSyncstraight out. That directory is gitignored and exists only after a successfulscripts/build-console.sh, so on a tree whose console build is broken the run died — after it had already paid for a vite dev server and a chromium launch — and then printed the Playwright remedy for a failure in which the browser had already done its job.Both halves of the card ship here. The second one is the expensive half: shipping only the
mkdircloses this ENOENT and leaves the misdiagnosis in place for the next failure that is not Playwright's.1. The output directory is a precondition, not a surprise at write time
mkdir -p "$TARGET"runs with the other preconditions — before the dev server and the browser — so a write-side problem fails cheaply instead of at the end of a long run. That ordering is the fix: amkdirplaced beside the write would close the ENOENT while leaving the wasted prelude exactly where it was.It also removes a false coupling. The ADR-0082 D4 ratchet never needed the built dist: this script drives a vite dev server over
.cache/objectui-*, anddist/is only where the manifest lands. Verified onorigin/main—sdui_dev_server_cmdbuildspnpm --filter @object-ui/console exec vite dev --port PORT --strictPort, and the only other read ofTARGETis the manifest path itself. So a broken console build made the ratchet unrunnable for want of one directory, precisely on the trees that most want an independent read on the registry.2. The remedy is chosen from what failed
Every failure branch of the dump used to print
pnpm exec playwright install chromium-headless-shell. For a write failure that advice is wrong, and in an agent dispatch container it cannot even be followed —PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1is set there (docs/releases-maintenance.md).The dump's combined output is now captured (
tee, so it still streams live) and classified bysdui_dump_failure_advice:browserType.launch: Executable doesn't exist at ...is verbatim fromdocs/releases-maintenance.md's revision-mismatch section.OUTpath in the text) → names the output file and its directory, and says the browser side is not implicated.The status capture is
${PIPESTATUS[0]}, never$?: after a pipeline$?istee's status, andteedoes not fail, so a plain$?would read every failure of the dump as a success.Reproduction — the harm, then each half separately
A literal end-to-end run is not possible in this container and this is stated rather than papered over: there is no vendored
.cache/objectui-*tree (that needspnpm objectui:build) and no Playwright browser (installing one is forbidden here). So the real script runs against an isolated fake framework root — the script is copied, which relocatesFRAMEWORK_ROOTand thereforeTARGET, so nothing writes into the realpackages/console/dist/. vite stands in as a one-line http server behind a fakepnpmonPATH; objectui's chromium dumper stands in as a node stub that writesOUTwithwriteFileSyncexactly as the real one does. Everything between them — port picking, the detached spawn, the liveness probe, the failure branch, the cleanup — is the real script.Before (
origin/main),packages/console/dist/absent:Both defects in one run: the dump had succeeded (it enumerated the registry over HTTP 200), the dev server had already been paid for, and the advice points at Playwright anyway.
After, same conditions:
A green run does not demonstrate half 2, so the diagnosis path is shown on its own. A non-Playwright dump failure (
TypeError, nothing to do with a browser):A write failure that survives the new precondition (the directory removed mid-run) now names the right layer:
And the vacuity guard in the other direction — the remedy is still printed when Playwright really is what failed, fed the verbatim container signature:
Finally, half 1's ordering claim, measured rather than asserted — with
mkdir -punable to succeed, the run stops before anything expensive starts:The pin
packages/spec/scripts/gen-sdui-manifest-write-target.test.ts, beside the existing cleanup and collision pins for this script and following their shape — executed assertions, not greps:mkdir -pwould pass against one written after the dump, which fixes nothing about the cost, so the test measures the order: the failing-precondition case asserts the dev server was spawned zero times, against the successful run's one.Gates
Re-derived from the actual diff with
node scripts/pm/dispatch-gates.mjs(no paths — it takes its own change set from the merge base), which named more than the dispatch list. Run at1d72a9a:pnpm check:cross-package-test-inputsOK: 12 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.(scripts/**already covers the new test)pnpm check:nul-bytesOK (scanned 6067 text file(s) ... no raw ASCII control bytes)pnpm check:merge-driver✓ check-regen-pending self-test passed.pnpm check:slot-lookup✓ slot-lookup ratchet holds ... none newpnpm check:type-source-resolutioncheck-type-source-resolution OK — 76 packages ... scannedpnpm check:engine-double-contractOK — 329 pinned, 133 in the DEBT ledger, 2 exemptpnpm check:where-matcher✓ where-matcher conformance holds ... none newpnpm check:query-options-erasure✓ query-options-erasure ratchet holds ... none newnode scripts/docs-audit/check-affected-docs.mjspnpm --filter @objectstack/spec check:scripts-typechecktsconfig.scripts.json, includescripts/**/*, plaintscwith zero ledger entries)pnpm --filter @objectstack/spec testTest Files 415 passed (415) · Tests 11045 passed (11045)That typecheck green was reverse-verified rather than trusted: a deliberate type error in the new test file turned it red naming
scripts/gen-sdui-manifest-write-target.test.ts(240,7): error TS2322, and restoring the file (byte-identical to the commit) returned it to green — so the program genuinely reads the added file.Two declared narrowings, neither hiding a red:
node scripts/check-dev-prereqs.mjsexits 1 on this worktree withThe workspace is not built — 67 of 67 workspace packages declare an entry point under dist/ that is not on disk. That is a standing property of any unbuilt worktree, independent of this two-file diff.check:type-check-debt --re-measureneeds the full workspace closure built and is left to CI. Its per-file question for the added test is answered directly bycheck:scripts-typecheckabove, which is the program that includes it — and being ledger-free, it permits no growth at all.No changeset: the diff is a root
scripts/file plus a test file, neither of which is published.skip-changesetapplied.Out of scope, deliberately
#10142 is not addressed here and remains open — ADR-0082's addendum naming three carriers of the
pnpm sdui:manifestreminder is a separate card.Generated by Claude Code