Skip to content

fix(scripts): export sync-template-versions' declaration surface and guard its entry point - #9648

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-9554-sync-template-versions-exports
Aug 18, 2026
Merged

fix(scripts): export sync-template-versions' declaration surface and guard its entry point#9648
os-steve merged 1 commit into
mainfrom
claude/issue-9554-sync-template-versions-exports

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9554

scripts/sync-template-versions.mjs already modelled its outputs well — a TEXT_STAMPS
table whose header says adding a fourth version surface should be "a row", and a
findTemplateDirs() walk that is "deliberately not a curated list". None of it was
reachable: nothing was exported, and the sync ran at module scope. A consumer that
imported the file to ask which paths does the version pass write? rewrote every
bundled template instead of getting an answer.

Measured on origin/main before the change:

$ node --input-type=module -e 'const m = await import("./scripts/sync-template-versions.mjs"); console.log("exported keys:", Object.keys(m))'
✓ blank/package.json already pins ^17.0.0 across 7 @objectstack/* dep(s) — ...
✓ blank/objectstack.config.ts already stamps engines.protocol '^17'
✓ blank/objectstack.manifest.json already stamps specVersion '^17.0.0'
✓ sync-template-versions: 1 template(s) in lockstep with create-objectstack@17.0.0 ...
exported keys: []

The whole sync ran, and the module exported nothing.

What changed

Why stampedPaths() reports three files per template, not two

It names package.json alongside the two text-stamp files. A consumer asking which files
the version pass writes is asking about package.json too — it carries the
@objectstack/* ranges this script rewrites. An export that answered the narrower
question while being named for the wider one would seed the next restatement.

This is a superset of the two literals in the release allowlist by exactly one path per
template, and that path is already allowlisted there by the '*package.json' pathspec
and the (^|/)package\.json$ re-check alternation. So consuming the full list permits
nothing new; it only moves the answer from a literal to a declaration.

⛔ The fenced half is NOT in this PR

.github/workflows/cut-rc.yml is claimed by epic #9465 and is not touched here, nor
is any adjacent file routed around it. This branch changes exactly two files:

scripts/sync-template-versions.mjs
packages/create-objectstack/src/template-version-stamps.test.ts

That workflow's own comment already asks for this change, which is why the split is clean:

sync-template-versions.mjs does declare its targets (a TEXT_STAMPS table plus a walk
of src/templates/), but exports none of them and has no entry-point guard, so importing
it would RUN the sync instead of reading its list. Give it those two things and this
pathspec should read from it too.

It now has those two things. The hand-off below is posted on #9465 for that lane to apply;
following the precedent of PR #9620, which hit the same fence and handed its piece over
rather than reaching across it.

Verification

H2 — every invocation site, before and after. The only executable invocation is the
root version script (changeset version && ... && node scripts/sync-template-versions.mjs && ...), reached from cut-rc.yml's pnpm run version step and from changesets/action
on the release PR. The other two references are not invocations: turbo.json names the
script as a cache input to create-objectstack#test, and
check-cross-package-test-inputs.mjs asserts that declaration exists. Both still hold.

CLI output is byte-identical to origin/main, proven by running the pre-change file at
the same directory depth:

$ git show origin/main:scripts/sync-template-versions.mjs > scripts/.baseline-9554.mjs
$ node scripts/.baseline-9554.mjs > before.txt ; node scripts/sync-template-versions.mjs > after.txt
$ diff before.txt after.txt && echo IDENTICAL
IDENTICAL # both exit 0

After the change, importing is silent and writes nothing, and the declarations are
readable:

exported keys: ["TEMPLATE_DIR","TEMPLATE_PKG_FILE","TEMPLATE_ROOT","TEXT_STAMPS",
"VERSION_SOURCE","findTemplateDirs","loadScaffolderVersion","stampedPaths"]
$ node --input-type=module -e 'import { stampedPaths } from "./scripts/sync-template-versions.mjs"; for (const p of stampedPaths()) console.log(p)'
packages/create-objectstack/src/templates/blank/objectstack.config.ts
packages/create-objectstack/src/templates/blank/objectstack.manifest.json
packages/create-objectstack/src/templates/blank/package.json

H3 — the test that keeps this fixed rather than re-found. The repo ships exactly one
template today, so on the live tree the walk and a literal blank pair agree and nothing
is red — which is why this could only be found by reading. The load-bearing cases therefore
run against a temp checkout carrying two templates.

That fixture is deliberately built stale (pinning ^17 while its scaffolder reads
42.0.0), which is what makes the import-safety assertion non-vacuous: an unguarded module
imported against a stale tree rewrites it, so "byte-identical after import" is evidence
only when there was something for a rewrite to do.

Both limbs were reverse-verified from the committed state. Predicted direction for each was
turns red, and both did:

AblationResult
remove the entry-point guardimporting ... rewrites nothing fails — the import rewrote all six stale fixture surfaces
replace the derived walk with a restated literal blank triplecovers every discovered template fails

The second ablation is the more informative one: the restated triple passed 7 of the 8
assertions
, including names only paths that exist and agrees with the live walk,
because on a one-template tree a restatement is indistinguishable from a derivation. Only
the two-template fixture caught it. That is the finding, reproduced as a test.

Commands run, all at 6372dc9a0:

pnpm --filter create-objectstack test → 6 files, 73 tests passed
(new file alone: 8 passed)
pnpm --filter create-objectstack typecheck → tsc --noEmit, exit 0
pnpm check:cross-package-test-inputs → OK, 12 packages read outside themselves, all declared
pnpm check:query-options-erasure → ratchet holds, none new
pnpm check:type-check-coverage → OK, 64/77 packages type-checked
pnpm check:engine-double-contract → OK, 319 pinned
pnpm check:where-matcher → 255 matchers, 0 silently-wrong, none new
node scripts/docs-audit/check-affected-docs.mjs → self-test 242 cases pass
node scripts/check-nul-bytes.mjs → OK, 6174 files, no raw control bytes

The upstream build closure is genuinely empty — create-objectstack depends only on
chalk and commander, no workspace packages — confirmed rather than assumed, since a
zero-match --filter exits 0 having run nothing.

No changeset: skip-changeset

This PR publishes nothing. scripts/ is not a published package, and the new .test.ts is
not shipped (create-objectstack's files is dist/README.md/CHANGELOG.md). The
script's user-visible behaviour is byte-identical, as measured above. An empty-frontmatter
changeset is not a route — #5471 shut it for new files — so a real changeset here would
declare a release of create-objectstack containing no user-visible change. The
skip-changeset label is the correct mechanism, and it also keeps this PR out of the
fenced .changeset/ tree entirely.

Scope

#9348 (same file: no --self-test, runs nowhere in CI) is not implemented here and
remains open; it is a separate change to the same file. The import-safety added here is its
prerequisite, since a self-test needs to call the module without running the sync.

Generated by Claude Code


Generated by Claude Code

…guard its entry point
The script models its targets well — a TEXT_STAMPS table and a findTemplateDirs()
walk that is deliberately not a curated list — but exported none of it and ran the
sync at module scope, so a consumer that imported it to ask which paths the version
pass writes rewrote every template instead of getting an answer.
Export stampedPaths() plus the raw declarations, and add the entry-point guard
#9064 added to check-docs-image-tag.mjs. The version read moved out of module
scope into loadScaffolderVersion(), which throws instead of exiting: an import
that can kill its host process is a worse hazard than the sync, not a smaller one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing 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.

@claude

claudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

✅ PM ACCEPT — #9554 / PR #9648 · both open questions ruled

Verified independently: 2 files +559/-119, zero governed-surface hits, no non-green gates (four still running). File surface is exactly scripts/sync-template-versions.mjs + the new test — no #9465-fenced path touched, confirmed against the diff rather than the report.


⭐ Ruling on Q1: A — keep skip-changeset. My ruling 3 was boilerplate and you were right to refuse it.

I attach "ship a .changeset/*.md" to every dispatch as the tail of the content/docs/releases/** clause. It is a statement about where release-notes input belongs, not a finding that your PR publishes something. This PR publishes nothing: scripts/ is not a published package and the new .test.ts is outside create-objectstack's files list.

But the deciding argument is the one I should have seen when I wrote the dispatch: .changeset/** is inside the #9465 fence I drew in ruling 1 of the same message. My two rulings contradicted each other, and you resolved the conflict in favour of the fence — which is the correct precedence, because a fence protects another lane's work while a changeset convention protects a document. Option B would have had you writing into the fenced tree to satisfy a clause that did not apply.

This is the second dev today to correctly identify that clause as inapplicable (#9595's did the same, and also found that check-empty-changeset.mjs shuts route C for new files). Twice is a pattern in my dispatch template, not in your judgement. Noted and mine to fix.

⭐ Ruling on Q2: Re-scope #9348 before dispatch. I verified your premise myself and it holds.

$ git show origin/main:turbo.json | ... create-objectstack#test
["$TURBO_DEFAULT$", "!dist/**", "!coverage/**", "!.turbo/**",
"$TURBO_ROOT$/content/**", "$TURBO_ROOT$/scripts/sync-template-versions.mjs"]

The input declaration was already on main. What your PR adds is the half that was missing: a test that actually executes the script.

And that goes to #9348's central argument, not a side claim. Its case is:

the ratchets catch a stale committed value; they cannot catch a rewriter that has stopped rewriting, because on a green corpus a working rewriter and a broken one produce identical (empty) results.

Your test runs the real CLI via execFileSyncagainst a two-template fixture — a deliberately non-green corpus — and asserts the actual rewrites. That is precisely the observation #9348 says CI can never make. Combined with the pre-existing turbo input, a PR that edits the stamper now schedules a test that watches it work.

So #9348's "runs nowhere in CI" premise is substantially false as of this PR, and I am re-scoping it rather than dispatching it as written. What survives is the narrow question you named: is a --self-test flag still wanted now that equivalent coverage lives in a vitest file CI already schedules? The #9064 precedent put the self-test in the script; yours puts it beside the script. I will dispatch that question with your evidence attached, and I am telling the next dev to consider "confirm and close" a legitimate outcome.

I note you deliberately implemented none of #9348's scope. Correct — the sequencing held.


⭐ The reverse verification's second limb is the best thing in this report

Limb 2 — replacing the derived walk with a restated literal blank triple — passed 7 of 8 assertions, including "names only paths that exist" and "agrees with the live walk". Only the two-template fixture caught it.

That is #9554's entire thesis, demonstrated rather than asserted. The card's argument was: today the walk and the literals agree, so nothing is red; the day a second template ships, the literals do not cover it and the cut refuses to push. You built the tree where that day has arrived and showed that every other assertion is blind to the difference. A test suite that cannot distinguish a derivation from a restatement on today's tree is exactly how this defect got here, and you added the one fixture that can.

The two restructures the card did not name

Both were forced by the guard, and both are improvements you had to discover:

  1. TEXT_STAMPS's value/replacement became functions of the version. They were strings baked in at module load, so the raw table could not be read without the module first reading a package.json — i.e. exporting the table without this change would have exported a table that could not be imported cleanly. The card's "just export TEXT_STAMPS" was not actually available.
  2. loadScaffolderVersion() throws instead of process.exit(1). Your reasoning is the right one and worth quoting: an import that can kill its host process is a worse import hazard than the sync, not a smaller one. Making a module import-safe while leaving it able to process.exit the importer would have moved the hazard, not removed it.

H2 — the guard's risk was measured, not argued

Before, on unmodified origin/main: importing the module printed all four sync log lines and yielded exported keys: [] — the import ran the whole sync and exported nothing. After: the same import is silent, writes nothing (git status clean), and yields 8 exports.

And CLI byte-identity was proven, not asserted: origin/main's copy extracted to the same directory depth so it resolves the same repo root, both run, diff → identical, both exit 0, baseline removed. That is the check that matters, because the one thing this change must not do is stop the release path from stamping.

Invocation inventory, and the distinction that makes it trustworthy: the only executable invocation is root package.json's version script (reached from cut-rc.yml's pnpm run version and from changesets/action). The other two references are not invocationsturbo.json names it as a cache input, and check-cross-package-test-inputs.mjs asserts that declaration exists. Separating "mentions the path" from "runs the script" is the difference between an inventory and a grep.

On the lock

One typecheck attempt returned 99 after 540s. You named the holder (live bash/node pair, PIDs 26452/26453, three queued flocks — busy, not stalled), spent the interval on lock-free gate runs, and re-acquired on retry. Exactly right: 99 is a queue timeout, not a failure, and distinguishing "contended" from "wedged" before reporting is what keeps a lock timeout from being read as a red gate.

Also correctly verified rather than assumed: the upstream build closure is genuinely empty (create-objectstack depends only on chalk and commander) — and you checked that separately, because a zero-match --filter exits 0 having run nothing. A command that succeeds by doing nothing is the same trap as a green check that never ran.

The hand-off

cut-rc.yml's rewiring went to #9465 as a ready-to-apply comment rather than as a PR against a fenced file. Third time today a dev has hit that fence and handed the piece over instead of routing around it. That is the fence working.

#9651

check-cross-package-test-inputs.mjs's rationale still says the stamper is "named in a comment rather than read" — which your test made untrue. Low severity and self-correcting (the gate reddens if anyone acts on the stale prose). Correctly filed as an observation without proposing a change.

Verdict: ACCEPT. Arming once the four running gates converge.


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 18, 2026 14:45
@os-steve
os-steve added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit 4b2de3cAug 18, 2026
31 checks passed
@os-steve
os-steve deleted the claude/issue-9554-sync-template-versions-exports branch August 18, 2026 15:03
os-steve pushed a commit that referenced this pull request Aug 18, 2026
… read it now is
`CROSS_PACKAGE_TEST_INPUTS`' `create-objectstack` entry justified its
`scripts/sync-template-versions.mjs` glob by the file being "named in a comment
rather than read". PR #9648 landed `template-version-stamps.test.ts`, which
imports the script's declaration surface and executes it, so that stopped being
true. The glob is unchanged and still correct -- only its stated reason moved.
The rewrite states three things instead of one, so a later edit cannot make it
silently wrong again: the read that exists today, the ratchets in
`template-consistency.test.ts` that would still justify the glob if that test
were deleted, and the measured fact that what actually FORCES the glob is
neither read -- both tests spell the path split-segment, which the literal
collector cannot see, so it picks up the quoted mention in each test header.
Also drops the same stale claim from the `@objectstack/dogfood` entry, which
cross-referenced `sync-template-versions.mjs` as a fellow mention-only
declaration. The `check-nul-bytes.mjs` comparison on the `@objectstack/cli`
entry is untouched -- measured, it still holds.
The collector blind spot the rewrite documents is recorded separately as #9763,
a sub-issue of #9747.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

1 participant

@os-steve