Skip to content

refactor(rest)!: ImportProtocolLike declares the request each of its three required members receives - #17420

Merged
os-justin merged 10 commits into
mainfrom
claude/issue-16952-import-protocol-typed-args
Sep 10, 2026
Merged

refactor(rest)!: ImportProtocolLike declares the request each of its three required members receives#17420
os-justin merged 10 commits into
mainfrom
claude/issue-16952-import-protocol-typed-args

Conversation

@os-justin

Copy link
Copy Markdown
Collaborator

Fixes #16952

Clause-②: yes

ImportProtocolLike is an exported extension point, and all three of its required members declared args: any. runImport accepts an injected protocol through it, so an implementor had no contract to compile against and could only freeze on the spelling it happened to observe. This gives the declaration a real type.

Option 1 as ruled. All three methods, not only findData#16950 deliberately left all three alone because narrowing a published extension point is a contract decision rather than a spelling fix.

What changed

The envelope the runner adds is declared once and exported, and each required member names the spec request it receives:

export type ImportProtocolRequest<R> = R & { context?: any; environmentId?: string };

export interface ImportProtocolLike {
  findData(args: ImportProtocolRequest<FindDataRequest>): Promise<any>;
  createData(args: ImportProtocolRequest<CreateDataRequest>): Promise<any>;
  updateData(args: ImportProtocolRequest<UpdateDataRequest>): Promise<any>;
  // …and the three already-typed optional members now use the same envelope
  // instead of re-spelling `context?: any; environmentId?: string` inline.
}

Nothing about the values the runner sends moves — the three literals are byte-for-byte the canonical ones #16950 landed. findArgsBase is untouched, deliberately: the census pin reads its exact signature, and with the interface typed the return annotation would be redundant.

Converging the three non-authoritative sites, in the same stroke

Triage's binding instruction:

三个站点观察到同一个方言(:352 写成注释、:354 写成运行时读取、测试 :52 写成类型),没有一个是权威。 这比「无人声明」更能说明问题:大家都知道,只是没写在能被编译器执行的那一处。⛔ 认领方修 option 1 时,这三处应当同笔收敛到导出的那一处,否则会留下第四份真理。

Two of the three moved when #16950 landed, before this branch existed: admin-import-users.ts:352 and :354 now read the canonical keys and say why. What was left was the third — a local parameter annotation in a test double, plus its siblings. Every double in this package now derives its annotation from the exported declaration itself rather than restating a shape:

type FindArgs = Parameters<ImportProtocolLike['findData']>[0];

A derived alias cannot become a fourth truth: revert the declaration and the alias reverts with it, which is what makes the ablation below possible. Ten inline restatements and five args: any annotations are gone; the file-local FindProbe type in the idempotency suite is now that alias.

The pins, and the ablation that shows they can fail

Two layers, because a type-level narrowing is invisible to a runtime test and a source-level revert is invisible to the type checker.

  • §1b source census (rest-server-canonical-query-ast.test.ts) — the existing erasure detector matched (query: any and (request: any, which is why it swept the whole file and reported nothing while the exported extension point declared no dialect at all. The name an implementor actually writes is args; it is now in the alternation, with a control asserting the detector fires on the exact spelling this PR removed and stays quiet on the declared form.
  • §2 type-level — five live @ts-expect-error directives over the wire dialect and the required members, all written against aliases derived from the declaration, plus a positive case showing an implementor that leaves its parameter unannotated gets typed by the contract.

Ablation — the declaration reverted to its pre-card spelling, both legs proven on disk and in dist/:

on-disk proof: typed members before=3 after=0 | `(args: any)` before=0 after=3
ablation-dist-preflight: ✓ dist/: marker present in 2 built files
MUTATED tsc(test layer) exit=1  total errors=5
  src/rest-server-canonical-query-ast.test.ts(430,9): error TS2578: Unused '@ts-expect-error' directive.
  …(432,9) (434,9) (436,9) (438,9)
MUTATED vitest exit=1
  FAIL §1 the three sites import-runner.ts names are canonical … not `any`
  FAIL §1b the exported `ImportProtocolLike` declares what it is handed
  Tests  2 failed | 30 passed | 1 skipped (33)

restore: git diff HEAD empty · ✓ dist/: marker absent from all 6 built files · tree clean

Every directive is live rather than decorative: tsc --listFiles -p tsconfig.test.json lists this file, so an unused directive is TS2578 here, and the ablation is exactly the run that turns them unused.

⭐ The card asked for pins that would go red on the degraded behaviour rather than pass vacuously. The vacuity being closed is measurable: reverting the declaration does not merely leave the pins passing, it makes five of them structurally incapable of failing — and that is what reddens.

The cross-lane consequence: measured, and FALSIFIED

Triage expected plugin-auth's hand-written implementor to stop compiling, and flagged it unmeasured. It does not break.

pnpm --filter @objectstack/plugin-auth exec tsc --noEmit   →  exit 0, 0 errors

async findData(args: any) stays assignable to the narrower signature — an any parameter is bivariant with everything. packages/plugins/plugin-auth/** is untouched by this diff.

That green is reverse-verified rather than assumed, because a clean run against a stale .d.ts would look identical. A probe carrying a key the new type refuses was appended to that file, and it reddens:

src/admin-import-users.ts(620,32): error TS2353: Object literal may only specify
  known properties, and '$filter' does not exist in type 'QueryInput'.

restored by blob hash against HEAD (a557f3ca… both sides) with git diff HEAD empty for that path. The probe was a measurement, never a delivered edit.

Acceptance notes

  • rest-server.ts was not needed and is not touched. It declares a structurally identical local envelope alias for its own dispatch sites. Converging the two would edit a file a sibling PR holds, so this PR declares its own and the divergence (context?: any here, matching what this interface's members already spelled, versus context?: unknown there) is stated in the type's docblock. noted, not filed — the successor is whoever next unifies the two envelopes, which is one edit once rest-server.ts is free.
  • An implementor that annotates its own parameter any opts back out — the annotation wins over the contextual type, so the contract reaches nothing. That is exactly the state plugin-auth's implementor is in today: it compiles, it reads the right keys, and it is not held to them. Stated in the interface docblock and in the changeset's migration line; not fixed here, because that file is another lane's. noted, not filed.
  • check-adr-0087-registration's type-surface-only category cannot express a PARAMETER narrowing. Its predicate 4 reads a return annotation or an exported type declaration, and an interface whose members take any reads as already CONCRETE, so the category refuses. A truthful disposition was still available (no-migration-prescription, verified green) so nothing was blocked and nothing is filed — but the next author narrowing a published parameter will reach for type-surface-only first and be refused by a predicate that is measuring the wrong position. noted, not filed.
  • $top ?? 2, the sibling default triage named, is already gonefix(rest): import-runner builds the canonical QueryAST through a typed findData envelope #16950 removed it along with ?? {}. No ?? fallback was added anywhere in this diff; the doubles read the canonical key straight and throw on absence, which is the loudness the card asked for.
  • The changeset is minor, not majorcheck-changeset-no-major is green, and this repo records breaking-ish narrowings as minor. The breaking nature, FROM/TO and the implementor migration line are spelled in prose there.

Verification

what result
pnpm --filter @objectstack/rest test 187 files, 3136 passed, 1 skipped, 0 failed
pnpm --filter @objectstack/rest typecheck exit 0 (tsc --noEmit + check:test-typecheck: 0 files / 0 errors)
pnpm --filter '@objectstack/rest^...' build exit 0 (dependency closure, built before any verdict was read)
pnpm --filter @objectstack/plugin-auth exec tsc --noEmit exit 0 — read-only measurement, reverse-verified
pnpm lint exit 0, whole repo, no narrowing claimed
derived gate families 60 derived · 58 run green · 2 NOT MEASURED
dispatch-gates --ran exit 0 — 60 accounted for, 0 unrun

NOT MEASURED: check:dual-build-cjs-loads and check:type-check-debt, both PREREQUISITE NOT MET (exit 3) — they read built output for the whole workspace and ask for a full pnpm build first, which is CI's farm-wide run, not this container's. ⛔ Neither is a finding and neither is a pass.

Gate verdicts were captured to disk before any of them was read, and reconciled with node scripts/pm/dispatch-gates.mjs --ran; the sweep and the table above were run at 3ce4fc73, the final commit, after origin/main was merged in.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 23 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/rest/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json cefe06870260ba7a5a85251189d6e663e991d9b9.

8 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/rest/src/index.ts) — pages documenting those are invisible to this run
  • 1 cross-cutting symbol(s) contributed no route anchor: updateData (4 routes)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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 — 14 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 cefe06870260ba7a5a85251189d6e663e991d9b9packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6cbf6588aa3d37fdf02f62ddf7d066c6d78d5675 — the merge of head 3ce4fc73220d8d12d00a2cfd0b30ea3f20b6161c into base cefe06870260ba7a5a85251189d6e663e991d9b9, 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 6cbf6588aa3d37fdf02f62ddf7d066c6d78d5675 && git checkout 6cbf6588aa3d37fdf02f62ddf7d066c6d78d5675
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cefe06870260ba7a5a85251189d6e663e991d9b9 3ce4fc73220d8d12d00a2cfd0b30ea3f20b6161c && git checkout -B drift-repro cefe06870260ba7a5a85251189d6e663e991d9b9 && git merge --no-ff 3ce4fc73220d8d12d00a2cfd0b30ea3f20b6161c

node scripts/docs-audit/affected-docs.mjs --json cefe06870260ba7a5a85251189d6e663e991d9b9

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs cefe06870260ba7a5a85251189d6e663e991d9b9 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 10, 2026
@os-justin
os-justin marked this pull request as ready for review September 10, 2026 14:24
@os-justin
os-justin enabled auto-merge September 10, 2026 14:24
@os-justin
os-justin added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit ab56ea3 Sep 10, 2026
39 checks passed
@os-justin
os-justin deleted the claude/issue-16952-import-protocol-typed-args branch September 10, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants