Skip to content

test(rest): state IHttpRequest once, in a typed request test builder - #13466

Merged
os-trump merged 2 commits into
mainfrom
claude/issue-13377-typed-ihttprequest-builder
Aug 30, 2026
Merged

test(rest): state IHttpRequest once, in a typed request test builder#13466
os-trump merged 2 commits into
mainfrom
claude/issue-13377-typed-ihttprequest-builder

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes#13377

IHttpRequest declares 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 in test-typecheck-debt.json as ledgered TS2345. 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.ts states the five once. Test layer only — nothing in src/index.ts reaches it, so tsup never emits it. Same placement, and for the same reason, as src/xlsx-test-loader.ts from #13425.

The design decision: what is defaulted, and what is derived

The card's own warning is that "a default path that 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:

  • method and path are 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.path is live — RestServer.enforceAuth feeds it to isAuthGateAllowlisted (src/rest-server.ts:1254), where a wrong value decides whether the ADR-0069 gate fires at all.
  • path is materialized from params, not copied from the pattern. A transport hands the handler a concrete path; /api/v1/data/:object is a pattern no request ever carries. Substituting the request's own params also means path and params cannot drift apart. A :segment with 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 yield undefined against {}. It is also strictly better-formed than what the literals produced — they left req.headersundefined, so the unguarded req.headers['if-none-match'] (src/rest-server.ts:5232) would have thrown had those tests reached it.
  • remoteAddress is 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 (Parameters of RouteHandler at index 0) rather than spelled by hand, so the builder cannot drift from the contract — the same discipline xlsx-test-loader.ts applies to its dependency.

Measurement

Authoritative command, in packages/rest, workspace closure built first:

npx tsc --noEmit --pretty false -p tsconfig.test.json

Before (4 errors, all TS2345, matching the ledger exactly):

src/meta-public-book-grant.test.ts(138,25) src/rest-batch-size-cap.test.ts(152,26)
src/rest.test.ts(2063,7) src/rest.test.ts(2088,7)

⚠️ One correction to the dispatch brief, which said the red site in rest-batch-size-cap.test.ts was :64: it is :152. :64 sits inside a post(rest: any, ...) helper, so its route is any and it was never red. The same explains why only 1 of 3 grep hits in meta-public-book-grant.test.ts is red: :73 and :147 reach their handler through any-typed helpers.

All four migrated. pnpm --filter @objectstack/rest ... vitest run over 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:

src/rest.test.ts(2065,7): error TS2345: Argument of type '{ json: Mock<Procedure>; status: Mock<Procedure>; }' is not assignable to parameter of type 'IHttpResponse'.
src/rest.test.ts(2090,7): error TS2345: ... same ...

That res expression is byte-identical to main — 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-built IHttpResponse literals: send and header absent, and status not typed as returning the interface). Out of scope for this card, which names IHttpRequest; 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 with pnpm --filter @objectstack/rest gen:test-typecheck-debt; _note is the authored half and is preserved verbatim, _comment and entries are generated): src/rest.test.ts was 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-changeset as both a closed path list and a publication test, and a src/*.ts file that never reaches dist satisfies one while violating the other. Resolved the way #13425 resolved it in this same package and landed — by measuring the built artifact after pnpm --filter @objectstack/rest build:

symboldist/index.jsdist/index.cjsdist/index.d.tsdist/index.d.cts
httpRequestForRoute0000
RouteUnderTest0000
materializeRoutePath0000
controlRestServer36371818
control loadXlsxWorkbook (#13425's helper)0000

The RestServer control proves the instrument reads these files; the loadXlsxWorkbook row 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 reaches dist. 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_TIER read by symbol at scripts/pm/dispatch-gates.mjs:5704.

All runs below at a631764c7b, the commit this PR pushes. Exit codes captured before any pipe.

gateits own verdict line
check:test-typecheck (rest)OK — @objectstack/rest's test layer compiles under packages/rest/tsconfig.test.json; 1 file(s) / 2 error(s) held
check: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 number
check:type-check-coverageOK — 66/78 workspace packages type-checked (plus the root), 12 in the DEBT ledger, 1 exempt
check:cross-package-test-inputsOK: 24 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob
check:engine-double-contractOK — 709 pinned, 134 in the DEBT ledger, 3 exempt
check:where-matcherwhere-matcher conformance holds: 317 matcher(s) discovered, 317 answer the combinator battery correctly or refuse it loudly
check:query-options-erasurequery-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new
check:objectql-double-limitOK ObjectQL double limit conformance holds: 293 double(s) graded
check:published-files69 publishable package(s) of 78 workspace member(s) declare a files whitelist ... and admits no test, test-harness config or build script
check:dual-build-cjs-loads102 published require entry point(s) across 66 package(s) load; 610 emitted CommonJS file(s) parse
check: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 scanned
check:doc-authoring48 published skill files clean / 14064 customer-facing string(s) across 691 spec sources clean
check:dispatcher-error-vocabularyOK — 22 unregistered code-stamping site(s), all classified
check: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 declaration
check:slot-lookupslot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none new
check: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 covered
check-comment-mask-adoption.mjsOK check:comment-mask-adoption — 14 private comment-stripper(s) ... all 14 recorded
check-keyed-text-bounds.mjs112 *.object.ts files ... 148 keyed text-family columns judged, 148 bounded
check-plugin-teardown-shape.mjs64 Plugin implementation(s) across 5044 source(s) ... baseline fully burned down
check-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 intact
check-affected-docs.mjsaffected-docs self-test: 487 cases pass
check-drift-comment.mjscheck-drift-comment: 56 cases pass across 5 fixture diff(s)

Refused — listed separately, NOT folded into the green list:

gateverdict
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 TERM restore on absolute paths, finishing on an empty git diff HEAD and an empty git status --porcelain. Never judged by exit code. No rebuild needed between legs, and that is measured rather than assumed: --listFiles puts the builder and all three test files in the tsc program (1 hit each of 503 files) while packages/rest/dist/index.d.ts scores 0, so the subject resolves from source, not from dist.

A1 — revert the migrated meta-public-book-grant site. Predicted: TS2345 returns, ratchet RED (GREW). Observed exactly that.

  • blob 722c9793d095b0664c7c160075b5ba0b26bf0403 (HEAD) → 7c241c977bfd102a73c9a9aa2e13144007ee4a2e (mutated) → 722c9793d095b0664c7c160075b5ba0b26bf0403 (restored)
  • removed-text count 1 → 0, injected-text count 1
  • src/meta-public-book-grant.test.ts(139,25): error TS2345: ... not assignable to parameter of type 'IHttpRequest' (plus a TS6133 for the then-unused import)
  • verdict: src/meta-public-book-grant.test.ts: 2 type error(s) in a file the ledger does not cover.

A2 — revert ONE rest.test.ts site. 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 _note above.

  • blob 5d5571a97515230b3da5584cbb9823f4666286722603085cce8377b69a74a00c67743ed45b5994bf5d5571a97515230b3da5584cbb9823f466628672
  • tsc: 2 errors — (2064,7) ... 'IHttpRequest' and (2090,7) ... 'IHttpResponse'
  • verdict: 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 Partial of the handler's parameter. Predicted: the TS2345 class returns at all four migrated sites. Observed exactly that.

  • blob 223f10eaa59c67165397d6f14fe47584783e5408ddf093643cfc460176f50dddedcc4b90333177c4223f10eaa59c67165397d6f14fe47584783e5408
  • four TS2345, 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)
  • verdict: check:test-typecheck: 2 problem(s) on the two now-unledgered files

Serial 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}.tszero file overlap with this diff, and it does not touch packages/rest/test-typecheck-debt.json. A scan of the in-flight claude/* branches found no other branch touching that ledger either.

Scope

Exactly the four ledgered sites. The other 26 inline res literals and the 151 as any call sites the card counts are untouched: they are not errors, and typing the any helpers that hide them is a different change.

Generated by Claude Code


Generated by Claude Code

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
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx(via data.create (sdk), data.find (sdk))
  • content/docs/api/data-flow.mdx(via data.create (sdk))
  • content/docs/api/environment-routing.mdx(via data.find (sdk))
  • content/docs/api/error-catalog.mdx(via data.create (sdk))
  • content/docs/deployment/troubleshooting.mdx(via data.find (sdk))
  • content/docs/kernel/runtime-services/data-service.mdx(via data.create (sdk), data.find (sdk))
  • content/docs/permissions/authentication.mdx(via data.find (sdk))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx(via data.create (sdk))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/rest/test-typecheck-debt.json) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: /api/v1/data/:object (route, 29 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 13 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 c38b7eff7816751f5b7c45cd5de5cb094a5262cbpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 002cac301de98a074d3dd8f6a6b6bb17e3df124c — the merge of head a631764c7bfada4e1831dd1bd0aa0353acff531f into base c38b7eff7816751f5b7c45cd5de5cb094a5262cb, 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 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

⚠️ 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 c38b7eff7816751f5b7c45cd5de5cb094a5262cb → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-trumpos-trump added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 30, 2026 — with Claude
@os-trump
os-trump marked this pull request as ready for review August 30, 2026 14:05
@os-trump
os-trump enabled auto-merge August 30, 2026 14:05
@os-trump
os-trump added this pull request to the merge queueAug 30, 2026
Merged via the queue into main with commit e10cf34Aug 30, 2026
37 checks passed
@os-trump
os-trump deleted the claude/issue-13377-typed-ihttprequest-builder branch August 30, 2026 14:23
os-project-manager pushed a commit that referenced this pull request Aug 30, 2026
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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A typed IHttpRequest test builder — 4 ledgered TS2345 in packages/rest are hand-built request literals, and the only in-repo alternative is as any

2 participants

@os-trump@claude