test(spec): discover the migrate-sentence markdown corpus instead of naming one file - #14557
Merged
Merged
Conversation
…naming one file The retired-key migrate-sentence pin scanned exactly one markdown file (`.claude/skills/spec-property-retirement/SKILL.md`), so the withdrawn "rewrite it automatically" claim could reach the published skill catalog — the artifacts a customer agent actually loads — with nothing scanning it. The markdown corpus is now DISCOVERED: the internal retirement playbook plus every `.md` file under `skills/`. Discovery is sorted (stable failure messages), skips symlinks rather than following one out of the repo, and is itself asserted non-vacuous — it must contain the playbook and at least one published skill, so a walk that silently reached zero published files cannot read green. The two-shape judge keeps its leading-`Run` requirement (naming the command mid-prose is not a prescription) and gains the distinction the wider population needs: a taught TEMPLATE is a double-backtick code span and must close it, while a sentence quoted in prose or a rendered transcript ends at its own period. Under a span-only anchor every quoted occurrence is an unconditional RED — the published upgrade skill quotes the house sentence verbatim inside an example parse error — which is a trap, not a property. Burying stays RED inside a template and in every `.ts` prescription. The BOTH-shapes anti-vacuity floor stays pinned to the playbook, the one file that owns both templates; a published skill that names the command once is not a regression. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GDA48PuRFrHyRfdkBz8m21
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. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
This was referenced Sep 2, 2026
os-sam
marked this pull request as ready for review
September 2, 2026 14:37
os-sam
enabled auto-merge
September 2, 2026 14:38
baozhoutao
pushed a commit
that referenced
this pull request
Sep 3, 2026
`scripts/cross-package-test-inputs.mjs`'s `@objectstack/spec` declaration block carried two comments PR #14557 made stale: - the `.claude/skills/spec-property-retirement/SKILL.md` entry said the retirement pin (`retired-key-migrate-sentence.test.ts`) judges "the ONE governed markdown file" — after #14557 the pin judges that file PLUS every `skills/**/*.md`. "One file, not `.claude/**`" was still literally true about the `.claude` entry while reading as an exhaustive scope claim about the test's population, so the sentence needed re-aiming, not a bumped count: it now names the two-corpus structure and points at the `skills/**` entry below for the second corpus. - the `skills/**` entry named `scripts/export-list.test.ts` as its only holder. `retired-key-migrate-sentence.test.ts` reads the same root (`PUBLISHED_SKILLS_ROOT`, `:102`) as its second corpus — "plus every `.md` file under `skills/`" (`:92`), emitted as `skills:`-prefixed entries (`:484-485`). The comment now names both holders. Comments only: every glob is unchanged, `check:cross-package-test-inputs` was already green on #14557's head, and it stays green here. Fixes #14560 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV
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
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 #13859
What was wrong
packages/spec/src/shared/retired-key-migrate-sentence.test.tspinned the house tombstone sentence over two source corpora and a markdown corpus of exactly one hard-coded file — the internal retirement playbook (RETIREMENT_SKILL_MD).The regex was never the defect; the population was.
WITHDRAWN_CLAIMmatches the withdrawn "rewrite it automatically" claim verbatim, and that claim nevertheless reachedskills/objectstack-upgrade/SKILL.md— published, customer-facing — with nothing scanning it. It was corrected by hand in #13861 (commit 88c30af, one line:rewrite it automatically.becomeslist the mechanical edits for existing sources; apply them by hand.). Nothing mechanical found it, and until this PR nothing stopped it coming back.What this changes
One test file. The markdown corpus is discovered, not named: the internal retirement playbook plus every
.mdfile underskills/. Discovery isreaddirSync(...).sort()at every level, so the corpus order and every failure message read the same on every machine and every run;lstatSyncskips a symlink instead of following one out of the repo (or back in, judging a file twice);The three existing markdown assertions keep their intent:
The BOTH-shapes floor stays pinned to the one file that owns both templates, deliberately: a published skill that names the command once is not a regression, so widening that floor would fail on the catalog's editorial choices rather than on drift.
SKILL_MARKERkeeps its leading-Runrequirement — naming the command mid-prose is not prescribing a sentence, and the published upgrade skill names it a dozen times without prescribing anything.WITHDRAWN_CLAIMstays English-only by design, so Chinese prose anywhere in the wider corpus cannot fabricate a match.The one thing the widening needed that the card could not predict
A naive widening does not land green, and the file that reds it is the very file this card is about.
The old anchors required a judged sentence to end at a double-backtick code-span close. That is how the playbook writes its two taught templates, so on a one-file corpus it read as "the sentence must be last in its container". On a wider corpus it means something else: any occurrence that is not inside a code span is an unconditional RED, because nothing but a code span can supply the terminator. That is not a property — it is a trap that fires the moment the corpus stops being one file.
Measured on
origin/mainat 4a37870, with the pin's own regexes over the widened corpus: 3 markers, 1 violation —skills/objectstack-upgrade/SKILL.md, quoting the corrected house sentence verbatim inside a rendered parse-error transcript whose next line isexpected: never. The span-only anchor would have red-flagged the text #13861 had just fixed.So "container" is now two things, and the judge asks which one it is looking at:
What that gives up, deliberately and only in prose: burying (
... apply them by hand. Also do X.) passes there. It stays RED inside a template and RED in every.tsprescription — the two places from which a reader actually copies text.skills/**is read by the widened corpus and never written by this PR: the diff is one test file, and the corpus found no other violation to report.Verification
Union re-run after the final commit, at
db374e225.The changed pin,
pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/shared/retired-key-migrate-sentence.test.ts—Test Files 1 passed (1) / Tests 14 passed (14).Whole
@objectstack/specsuite —Test Files 451 passed (451) / Tests 12190 passed (12190). Not sharded.pnpm --filter @objectstack/spec build && ... typecheck—check:test-typecheck: OK ... 54 file(s) / 262 error(s) / 146 pinned signature(s), the ledgered counts unchanged. The edited file is genuinely in that program, not merely adjacent to it:tsc -p tsconfig.test.json --listFileslists it (1 hit), and none of the 262 ledgered errors names it.Reverse verification (the widening is real, not declarative). The exact withdrawn sentence was appended to
skills/objectstack-upgrade/SKILL.mdin the worktree, after the implementation was committed. Mutation proven on disk before the run — occurrence count 0 then 1, blob16b7a39ethen87b24b72. The pin went RED, 2 failed / 12 passed, both failures naming the file:every prescription sentence in the markdown corpus is house-form or MIXED two-clause—AssertionError: skills:objectstack-upgrade/SKILL.md [prose] — "Run ... to rewrite it automatically."[#9529] the withdrawn automatic-rewrite claim is absent from the corpus, in every spelling— same file.Restored with
git checkout HEAD -- (absolute path)under anEXIT INT TERMtrap on absolute paths, and proven restored by bytes rather than by an exit code: blob back to16b7a39e(equal to the HEAD blob, non-empty), occurrence count back to 0,git diff HEADempty,git status --porcelainempty. The mutation was never committed or pushed. Onorigin/mainneither assertion could have seen that file at all.Gate family, derived mechanically with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackatdb374e225(1 path vs merge base 4a37870): 47 commands. 44 measured green — each exit code captured to a file before any pipe. Among themcheck:cross-package-test-inputs(the widened read ofskills/is already inside this package's declared globs, andturbo.jsonalready hashes them),check:test-source-alias,check:type-check-coverage,check:published-files, and every@objectstack/spec check:*in the family. Pluspnpm check:nul-bytes—OK (scanned 7938 text file(s) ... no raw ASCII control bytes).3 gates NOT MEASURED, all prerequisite-not-met on an unbuilt workspace, none a finding:
check-dev-prereqs(The workspace is not built — 1 unmet precondition),check:doc-formula-expressions(exit 3, needs@objectstack/formuladist),check:dual-build-cjs-loads(exit 3,PREREQUISITE NOT MET — this gate reads built output),check:type-check-debt(exit 3,PREREQUISITE NOT MET), andcheck-test-completeness(exit 3, whose own text says to record it as NOT MEASURED when run without aturbo run testlog). CI builds the workspace and measures all of them.Declared narrowing: the repo-wide
pnpm lintsweep was not run locally. It is CI-owned and CI runs it on this PR regardless.Changeset
None. This is a test-only change — one
*.test.tsfile, publishing nothing from any package — so it carries the repo's documented test-only handling, theskip-changesetlabel, rather than a fabricated release note.Draft on purpose: the PM seat reviews and lands. Not marked ready, no auto-merge.
🤖 Generated with Claude Code
Generated by Claude Code
Generated by Claude Code