test(rest): state IHttpRequest once, in a typed request test builder - #13466
Conversation
The four ledgered TS2345 in this package's test layer were hand-built request literals: IHttpRequest declares five required members and each literal supplied two or four of them. No annotation repairs them - adding the missing members changes what the handler RECEIVES, so inventing values silently changes what each test measures. src/http-request-test-builder.ts states the five once. It defaults only what has a neutral default (headers/params/query empty) and DERIVES the rest: method and path are read off the route under test, with path materialized from the request's own params, so the request and the route it is sent to cannot disagree. remoteAddress is deliberately left absent - its #4910 contract note makes it the unforgeable half of caller identification, and a plausible default would forge exactly the member whose worth is that it cannot be forged. Repairing all four did not empty the ledger: two IHttpResponse errors appeared at the same two call sites in rest.test.ts, on argument 2. That res expression is byte-identical to main - tsc reports at most one argument error per call, so the request literals were masking them. Filed separately; the ledger's authored _note now records the mechanism, since an EXACT per-file count measures a quantity and not an identity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k
…ped-ihttprequest-builder
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 002cac301de98a074d3dd8f6a6b6bb17e3df124c && git checkout 002cac301de98a074d3dd8f6a6b6bb17e3df124c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c38b7eff7816751f5b7c45cd5de5cb094a5262cb a631764c7bfada4e1831dd1bd0aa0353acff531f && git checkout -B drift-repro c38b7eff7816751f5b7c45cd5de5cb094a5262cb && git merge --no-ff a631764c7bfada4e1831dd1bd0aa0353acff531f
node scripts/docs-audit/affected-docs.mjs --json c38b7eff7816751f5b7c45cd5de5cb094a5262cb
|
The EXACT ratchet in scripts/check-test-typecheck.mts recorded ONE INTEGER per file. An integer measures a quantity, never an identity, so a file's entire error population could rotate underneath a constant number and the gate printed OK. Measured by ablation in packages/rest, not argued (#13470): src/rest.test.ts was recorded at 2, PR #13466 replaced both hand-built IHttpRequest literals with a typed builder, and it measured 2 again -- while NEITHER error was the same error. tsc reports at most ONE argument-assignability error per call, so the request literals had been MASKING response-literal errors at the very same two call sites; repairing the request unmasked the response one line down. A ledger entry is now a map of normalized error SIGNATURE to count. A signature is the TS code plus the diagnostic message with churn-prone spans collapsed, and it carries NO line or column -- so unrelated edits above an error do not touch the ledger, and only a change to WHICH error is present does. Both halves of the set difference are named: which signature ARRIVED and which VANISHED. Counts are provably unmoved by the migration: rest 1 file / 2 errors, spec 54 files / 262 errors, identical before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
Fixes #13377
IHttpRequestdeclares five required members —params,query,headers,method,path— and a route handler's first parameter is that interface. Four sites in this package's test layer handed a handler an object literal supplying two or four of them, and sat intest-typecheck-debt.jsonas ledgeredTS2345. No annotation repairs them: adding the missing members changes what the handler receives, so inventing values silently changes what each test measures.packages/rest/src/http-request-test-builder.tsstates the five once. Test layer only — nothing insrc/index.tsreaches it, so tsup never emits it. Same placement, and for the same reason, assrc/xlsx-test-loader.tsfrom #13425.The design decision: what is defaulted, and what is derived
The card's own warning is that "a default
paththat does not match the route under test would make a passing test measure something other than what it names". Rather than pick a careful default, the builder removes the question:methodandpathare derived from the route under test, not defaulted. Every call site already locates its route (getRoutes().find(...)); it now hands that same route object to the builder. The request and the route it is sent to cannot disagree, because one is computed from the other. This is not cosmetic:req.pathis live —RestServer.enforceAuthfeeds it toisAuthGateAllowlisted(src/rest-server.ts:1254), where a wrong value decides whether the ADR-0069 gate fires at all.pathis materialized fromparams, not copied from the pattern. A transport hands the handler a concrete path;/api/v1/data/:objectis a pattern no request ever carries. Substituting the request's ownparamsalso meanspathandparamscannot drift apart. A:segmentwith no matching param is refused loudly rather than emitted — a path holding a literal:is one no transport produces.headers: {}is the one required member with a genuinely neutral default: every header read in this package is optional-chained or an index access, both of which yieldundefinedagainst{}. It is also strictly better-formed than what the literals produced — they leftreq.headersundefined, so the unguardedreq.headers['if-none-match'](src/rest-server.ts:5232) would have thrown had those tests reached it.remoteAddressis deliberately absent, with no default at all. Its 「v17」入站 rateLimit 接执行:ApiEndpoint / HttpServer 的 RateLimitConfig 推导为 runtime token bucket 配置,dispatcher 生效(#4686 拆向之一) #4910 contract note makes it "the TRANSPORT's own peer address ... the unforgeable half of caller identification". A plausible'127.0.0.1'would forge exactly the member whose entire worth is that it cannot be forged. Optional in the interface because "not every runtime exposes it", so absent is legal and true.The return type is read off the handler signature (
ParametersofRouteHandlerat index 0) rather than spelled by hand, so the builder cannot drift from the contract — the same disciplinexlsx-test-loader.tsapplies to its dependency.Measurement
Authoritative command, in
packages/rest, workspace closure built first:Before (4 errors, all
TS2345, matching the ledger exactly):rest-batch-size-cap.test.tswas:64: it is:152.:64sits inside apost(rest: any, ...)helper, so its route isanyand it was never red. The same explains why only 1 of 3 grep hits inmeta-public-book-grant.test.tsis red::73and:147reach their handler throughany-typed helpers.All four migrated.
pnpm --filter @objectstack/rest ... vitest runover the three touched files: 3 passed (3) / 244 passed (244).Repairing all four did NOT empty the ledger — and that is a finding
Two errors appeared in
src/rest.test.ts, at the same two call sites, on argument 2:That
resexpression is byte-identical tomain— this diff edits only argument 1. tsc reports at most one argument-assignability error per call, so the request literals were masking response literals. Not created here; uncovered here. Filed separately as #13454 (hand-builtIHttpResponseliterals:sendandheaderabsent, andstatusnot typed as returning the interface). Out of scope for this card, which namesIHttpRequest; a mock response builder has to decide what it records and what a test may assert on it — a second test-semantics design question, not an annotation.Consequence recorded in the ledger's authored
_note(regenerated withpnpm --filter @objectstack/rest gen:test-typecheck-debt;_noteis the authored half and is preserved verbatim,_commentandentriesare generated):src/rest.test.tswas recorded at 2 before and measures 2 after, and neither of the two is the same error. An EXACT per-file count measures a quantity, never an identity.Changeset:
skip-changeset, measured rather than argued#13432 notes the os-dev contract states
skip-changesetas both a closed path list and a publication test, and asrc/*.tsfile that never reachesdistsatisfies one while violating the other. Resolved the way #13425 resolved it in this same package and landed — by measuring the built artifact afterpnpm --filter @objectstack/rest build:dist/index.jsdist/index.cjsdist/index.d.tsdist/index.d.ctshttpRequestForRouteRouteUnderTestmaterializeRoutePathRestServerloadXlsxWorkbook(#13425's helper)The
RestServercontrol proves the instrument reads these files; theloadXlsxWorkbookrow shows the landed precedent is unpublished for the same reason. Nothing published moves. Cross-check: #13426 is in flight in this same package and does carry a changeset — because it edits production source that reachesdist. The distinction is publication, and it holds in both directions.Gates
Union derived on the merged tree,
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack. stderr read: no STALE TREE banner —gate list derived from the tree ... at commit a631764c7b,change set derived from git — 5 path(s) vs merge base 71627f7b4. Tier line:no path-derived mandate; clause ② not engaged —packages/spec/src/**untouched, no contract accept/reject behaviour changed, no public surface widened (the table above is that measurement).CONTRACT_REVIEW_TIERread by symbol atscripts/pm/dispatch-gates.mjs:5704.All runs below at
a631764c7b, the commit this PR pushes. Exit codes captured before any pipe.check:test-typecheck(rest)OK — @objectstack/rest's test layer compiles under packages/rest/tsconfig.test.json; 1 file(s) / 2 error(s) heldcheck:type-check-debt--re-measure: OK — 29 ledger entr(ies) re-measured in 301.5s, 1547 raw tsc error(s) total, none above its recorded numbercheck:type-check-coverageOK — 66/78 workspace packages type-checked (plus the root), 12 in the DEBT ledger, 1 exemptcheck:cross-package-test-inputsOK: 24 package(s) read outside themselves, all declared, and turbo.json hashes every declared globcheck:engine-double-contractOK — 709 pinned, 134 in the DEBT ledger, 3 exemptcheck:where-matcherwhere-matcher conformance holds: 317 matcher(s) discovered, 317 answer the combinator battery correctly or refuse it loudlycheck:query-options-erasurequery-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none newcheck:objectql-double-limitOK ObjectQL double limit conformance holds: 293 double(s) gradedcheck:published-files69 publishable package(s) of 78 workspace member(s) declare a files whitelist ... and admits no test, test-harness config or build scriptcheck:dual-build-cjs-loads102 published require entry point(s) across 66 package(s) load; 610 emitted CommonJS file(s) parsecheck:test-source-aliasOK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through dist/check:type-source-resolutionOK — 95 tsc program(s) across 77 packages scannedcheck:doc-authoring48 published skill files clean/14064 customer-facing string(s) across 691 spec sources cleancheck:dispatcher-error-vocabularyOK — 22 unregistered code-stamping site(s), all classifiedcheck:logger-receiver-detachOK every log channel keeps its receiver: 2309 non-test TS file(s) walked, 0 detach(es)check:page-declaration-shapeOK — 34 page entries across 2320 sources ... all reach the kernel through a discoverable declarationcheck:slot-lookupslot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none newcheck:nul-bytesOK (scanned 7448 text file(s) ... no raw ASCII control bytes)check-ci-filter-parity.mjsOK: all 123 declared cross-package glob(s) (88 unique) are coveredcheck-comment-mask-adoption.mjsOK check:comment-mask-adoption — 14 private comment-stripper(s) ... all 14 recordedcheck-keyed-text-bounds.mjs112 *.object.ts files ... 148 keyed text-family columns judged, 148 boundedcheck-plugin-teardown-shape.mjs64 Plugin implementation(s) across 5044 source(s) ... baseline fully burned downcheck-shard-attestation.mjs2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s)check-undeclared-dep-imports.mjs78 workspace packages ... 2 ledger row(s), all evidence intactcheck-affected-docs.mjsaffected-docs self-test: 487 cases passcheck-drift-comment.mjscheck-drift-comment: 56 cases pass across 5 fixture diff(s)Refused — listed separately, NOT folded into the green list:
check-test-completeness.mjsPREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named.Exit 3, distinct from a finding's 1. Its own text: "the local reading for this gate is NOT MEASURED. ⛔ It is not a red". CI tees the log and passes the path, so this branch is unreachable there.Reverse verification
Direction predicted before each run. Mutation and restore each proven on disk by blob hash, with a
trap ... EXIT INT TERMrestore on absolute paths, finishing on an emptygit diff HEADand an emptygit status --porcelain. Never judged by exit code. No rebuild needed between legs, and that is measured rather than assumed:--listFilesputs the builder and all three test files in the tsc program (1 hit each of 503 files) whilepackages/rest/dist/index.d.tsscores 0, so the subject resolves from source, not fromdist.A1 — revert the migrated
meta-public-book-grantsite. Predicted:TS2345returns, ratchet RED (GREW). Observed exactly that.722c9793d095b0664c7c160075b5ba0b26bf0403(HEAD) →7c241c977bfd102a73c9a9aa2e13144007ee4a2e(mutated) →722c9793d095b0664c7c160075b5ba0b26bf0403(restored)src/meta-public-book-grant.test.ts(139,25): error TS2345: ... not assignable to parameter of type 'IHttpRequest'(plus aTS6133for the then-unused import)src/meta-public-book-grant.test.ts: 2 type error(s) in a file the ledger does not cover.A2 — revert ONE
rest.test.tssite. Predicted: the reverted site re-masks its own response error, count stays 2, and the EXACT ratchet stays GREEN despite both errors having changed identity. Observed exactly that — this is the empirical basis for the_noteabove.5d5571a97515230b3da5584cbb9823f466628672→2603085cce8377b69a74a00c67743ed45b5994bf→5d5571a97515230b3da5584cbb9823f466628672(2064,7) ... 'IHttpRequest'and(2090,7) ... 'IHttpResponse'check:test-typecheck: OK — ... 1 file(s) / 2 error(s) held. A green that is a non-detection, reported as such and not as a pass.B — relax the builder's type derivation to a
Partialof the handler's parameter. Predicted: theTS2345class returns at all four migrated sites. Observed exactly that.223f10eaa59c67165397d6f14fe47584783e5408→ddf093643cfc460176f50dddedcc4b90333177c4→223f10eaa59c67165397d6f14fe47584783e5408TS2345, one per migrated site:meta-public-book-grant.test.ts(139,25),rest-batch-size-cap.test.ts(153,26),rest.test.ts(2064,7),rest.test.ts(2089,7)check:test-typecheck: 2 problem(s)on the two now-unledgered filesSerial check
#13426 is in flight in this same package. Verified rather than assumed: its branch changes
.changeset/rest-production-exceljs-typing.md,content/docs/permissions/system-context.mdx,packages/rest/src/{import-prepare,rest-server,xlsx-module,xlsx-module.test,package-door-declared-code.test}.ts— zero file overlap with this diff, and it does not touchpackages/rest/test-typecheck-debt.json. A scan of the in-flightclaude/*branches found no other branch touching that ledger either.Scope
Exactly the four ledgered sites. The other 26 inline
resliterals and the 151as anycall sites the card counts are untouched: they are not errors, and typing theanyhelpers that hide them is a different change.Generated by Claude Code
Generated by Claude Code