Skip to content

fix(metadata): state the raw-driver remedy once in migrateProjectIdToEnvironmentId - #13243

Open
os-zhuang wants to merge 1 commit into
mainfrom
claude/issue-13219-dedupe-raw-driver-guard-message
Open

fix(metadata): state the raw-driver remedy once in migrateProjectIdToEnvironmentId#13243
os-zhuang wants to merge 1 commit into
mainfrom
claude/issue-13219-dedupe-raw-driver-guard-message

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #13219

The raw-driver guard in packages/metadata/src/migrations/migrate-project-id-to-environment-id.ts concatenated its instruction sentence twice, so an operator calling the migration with a driver that has no raw() read the same remedy twice in one message. A copy-paste artifact rather than intent: the sibling migrateEnvIdToProjectId carries the correct single-sentence form of the identical guard. Cosmetic and operator-facing only — the guard fires on exactly the same condition, the remedy it names is unchanged, and nothing parses the message.

Branch cut from main e452ad542, at-or-after the landing of PR #13220 that the triage sequencing constraint was waiting on. That PR moved the guard from :58 to :121; the line numbers here were re-derived at the branch head, not taken from the card.

The change

One deleted line in the source. The full source diff:

     if (typeof driverAny.raw !== 'function') {
         throw new Error(
-            'migrateProjectIdToEnvironmentId: driver must expose a .raw(sql, bindings?) method. ' +
             'migrateProjectIdToEnvironmentId: driver must expose a .raw(sql, bindings?) method. ' +
             'SqlDriver (better-sqlite3/knex) supports this; cloud-side TursoDriver also conforms.'
         );

The assembled message, before and after

Captured at runtime by invoking the guard, printed through JSON.stringify so the trailing space is visible rather than inferred:

Before — 250 characters, instruction sentence twice:

"migrateProjectIdToEnvironmentId: driver must expose a .raw(sql, bindings?) method. migrateProjectIdToEnvironmentId: driver must expose a .raw(sql, bindings?) method. SqlDriver (better-sqlite3/knex) supports this; cloud-side TursoDriver also conforms."

After — 167 characters, instruction sentence once:

"migrateProjectIdToEnvironmentId: driver must expose a .raw(sql, bindings?) method. SqlDriver (better-sqlite3/knex) supports this; cloud-side TursoDriver also conforms."

250 minus 167 is 83, exactly the deleted sentence plus its trailing space. The space that separates the two surviving sentences lives inside the string literal (method. ' +), so deleting the whole duplicated line preserves it — the assembled text still reads method. SqlDriver, not method.SqlDriver. That run-together form is this card's own defect inverted, and it is pinned below.

The test

The package already had a refusal case, still refuses a driver without .raw(), added by #13220. Its assertion was rejects.toThrow(/must expose a \.raw\(sql, bindings\?\) method/) — a substring match, which cannot see a second copy and so passed both before and after the fix. That case is extended in place (no second test file) to pin the properties of the assembled message rather than a full-string copy of today's wording:

  1. the instruction sentence appears exactly once — counted with a global regex, so a later rewording still leaves this asserting;
  2. no sentence runs into the next — asserted as the absence of a lowercase-period-uppercase boundary, which catches method.SqlDriver while correctly not matching the .raw( in the text itself;
  3. non-vacuity — the supporting sentence naming the conforming drivers is still present, since deleting it instead would satisfy (1) and (2).

Reverse verification, run test-first before the source fix, with the dependency closure built:

 FAIL  src/migrations/migrate-project-id-to-environment-id.test.ts > still refuses a driver without .raw(), stating the remedy exactly once
AssertionError: expected [ ...(2) ] to have a length of 1 but got 2
 Test Files  1 failed (1)
      Tests  1 failed | 8 passed (9)

The observed direction is a plain red on the new assertion only; the eight pre-existing cases in the file stayed green, confirming the new case is what moved.

Verification

All of the following re-run after the final commit, at f2ff3b77d.

pnpm --filter @objectstack/metadata test
   Test Files  39 passed (39)
        Tests  687 passed (687)

pnpm lint                                (eslint . --no-inline-config, whole repo)   EXIT=0
pnpm check:nul-bytes                                                                 EXIT=0
pnpm check:engine-double-contract                                                    EXIT=0
pnpm check:where-matcher                                                             EXIT=0
pnpm check:cross-package-test-inputs                                                 EXIT=0
pnpm check:query-options-erasure                                                     EXIT=0
pnpm check:type-check-coverage                                                       EXIT=0
pnpm check:test-source-alias                                                         EXIT=0

check:query-options-erasure reports baseline key set verified against e452ad5: no files added — this extends an existing test file rather than adding one, so its test-surface ceiling does not move.

Typecheck, and why it is a measurement and not a silence. @objectstack/metadata has no typecheck script, so tsc was run directly over the package program: tsc -p packages/metadata/tsconfig.json --noEmit. That tsconfig includes src/**/* and excludes only node_modules/dist, so test files are inside the program — confirmed with --listFiles, which matches both edited files (count 2). The run reports 89 errors, zero of them in either edited file; all 89 are pre-existing debt in other files of this package (metadata.test.ts, register-notifies-watchers.test.ts and others). 89 is exactly the frozen count recorded for @objectstack/metadata in the TEST_DEBT ledger in scripts/check-type-check-coverage.mjs, so the ratchet quantity is unmoved.

The gate family was derived mechanically from the real change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, not from a recalled list. Two members of that family returned exit 3, PREREQUISITE NOT MET, which is NOT MEASURED and not a red: scripts/check-test-completeness.mjs grades a saved turbo run test log that only CI produces, and scripts/pm/check-half-states.mjs needs a real GitHub credential this container does not carry. check:type-check-debt --re-measure and check:dual-build-cjs-loads need the whole workspace built and are left to CI; the quantity the first of them ratchets is the 89 measured directly above.

Scope

Card scope only, one deleted line plus the test that pins it. The sibling migrate-env-id-to-project-id.ts is untouched — it is already correct, and #13220 recorded a deliberate reason it is not gated the same way. Changeset added, graded patch: an operator-facing message fix, nothing structural. No release-notes file touched. No out-of-scope findings.

Generated by Claude Code


Generated by Claude Code

…EnvironmentId (#13219)

The guard concatenated its instruction sentence twice, so an operator calling
the migration with a driver that has no `raw()` read the same remedy twice in
one message. A copy-paste artifact: the sibling `migrateEnvIdToProjectId`
carries the correct single-sentence form of the identical guard.

Cosmetic and operator-facing only — the guard fires on the same condition and
names the same remedy. The surviving line keeps the trailing space inside its
literal, which is what separates it from the sentence naming the conforming
drivers; trimming it would run the two sentences together, this defect
inverted.

The package's refusal case now pins the properties of the assembled message
(the instruction appears exactly once, no sentence runs into the next, and the
supporting sentence is still present) instead of substring-matching it, which
could not see a second copy and so passed either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9e0ba21a1bf863c44e341fdad1bbecb755d450c2packageMentionDocs.

Which tree this was computed on

This run read content/docs from c57cb74096ad3f96496615cedfc6e96719b56108 — the merge of head f2ff3b77d9081461bb7a042644433ea8e7a3fae5 into base 9e0ba21a1bf863c44e341fdad1bbecb755d450c2, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c57cb74096ad3f96496615cedfc6e96719b56108 && git checkout c57cb74096ad3f96496615cedfc6e96719b56108
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e0ba21a1bf863c44e341fdad1bbecb755d450c2 f2ff3b77d9081461bb7a042644433ea8e7a3fae5 && git checkout -B drift-repro 9e0ba21a1bf863c44e341fdad1bbecb755d450c2 && git merge --no-ff f2ff3b77d9081461bb7a042644433ea8e7a3fae5

node scripts/docs-audit/affected-docs.mjs --json 9e0ba21a1bf863c44e341fdad1bbecb755d450c2

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

PM review — seat session_01LZbWd2jNV1FErXTPSS4Dry (PM seat #6367, domain:engine lane). Read at head f2ff3b77d, three-dot against merge base e452ad542.

Verified

claim reading
3 files, source −1 line exact — one deleted line, nothing else in the source
trailing space preserved structurally, not by luck: the space lives inside the literal ('…method. ' +), so deleting a whole line cannot strip it
sibling untouched migrate-env-id-to-project-id.ts not in the diff
protected paths content/docs/releases, docs/adr, .claude, skills, AGENTS.md, CLAUDE.md, packages/spec/src, sql-driver.tsempty
Clause-② does not fire

Independently checked the docs surface the drift bot flagged as uncovered: no page in content/docs quotes this guard sentence (zero hits for must expose a .raw). Nothing owed there.

The test is the part worth reading

It pins three properties and none of them is a copy of today's wording:

  1. Counted, not compared. message.match(/…/g) must have length 1. A later rewording of the sentence leaves this still asserting something.
  2. ⚠️ The inverted defect is pinned in the same case. expect(message).not.toMatch(/[a-z]\.[A-Z]/) catches the other way of "fixing" this — trimming the surviving line's trailing space, which satisfies (1) while gluing method.SqlDriver. The comment also disposes of the obvious false positive: .raw( is lowercase-after-period and correctly not a sentence boundary.
  3. Non-vacuity. Deleting the supporting sentence instead would satisfy (1) and (2) equally, so SqlDriver is asserted present — that is the half an operator actually acts on.

It also records why the defect survived: the previous assertion was rejects.toThrow(/must expose a \.raw…/), a substring match that "cannot see a second copy". That sentence is worth more than the fix; it explains how a guard could ship doubled with a test standing over it.

The reverse verification was test-first rather than an ablation — assertions written and run before the source change, observed red on the new case only with the eight pre-existing cases staying green. That is the stronger form here: no implementation was ever mutated, so there is no restore leg to go wrong.

One process note, and the dev was right

The report flags that my dispatch said the issue was already claimed and told it not to touch the assignee, while the issue was in fact unassigned. ⚠️ The dev is correct and I was wrong — I wrote that boilerplate from the pattern of earlier dispatches without checking this card's state. It assigned the issue and posted a claim comment, which is what CLAUDE.md's first-action rule requires and what the card's own FIRST ACTION section said. Raising the conflict instead of silently picking one was the right call.

Also noted: the guard had moved to :121-123 from the :58-60 I quoted, because #13220 landed in between — the dispatch warned line numbers might shift and they did.

Not landing yet

CI just started. Holding in draft until every check is completed and green — total_count grows as aggregate rows appear, so a partial read is not a pass.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 29, 2026 14:50
@os-zhuang
os-zhuang enabled auto-merge August 29, 2026 14:50
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

migrateProjectIdToEnvironmentId's raw-driver guard prints its instruction sentence twice

2 participants