Skip to content

test(runtime): drive /data's error exits through the REAL ADR-0112 envelope (#6719) - #7364

Merged
os-help merged 1 commit into
mainfrom
claude/issue-6719-data-domain-envelope-double
Aug 10, 2026
Merged

test(runtime): drive /data's error exits through the REAL ADR-0112 envelope (#6719)#7364
os-help merged 1 commit into
mainfrom
claude/issue-6719-data-domain-envelope-double

Conversation

@os-help

@os-help os-help commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes #6719

The /data domain's DomainHandlerDeps stand-in hand-wrote its three error exits, and what they answered was not the ADR-0112 envelope — a bare string where production answers { success: false, error: { code, message, httpStatus, details? } }. Every /data case driven through this harness was therefore structurally incapable of going red on an envelope regression. The exits are now taken off a real HttpDispatcher, and four new cases drive the domain's error branches and assert the ADR-0112 triple.

What changed

One file: packages/runtime/src/domains/data-path-object.test.ts. No production code, no shared test-util extraction, no new dependency.

The three hand-written exits are replaced by HttpDispatcher.domainDeps — the exact DomainHandlerDeps object every /data request runs against in production — borrowed off a dispatcher built over a kernel stub, exactly as error-envelope.conformance.test.ts's makeDispatcher() does. This goes one step further than #6649's /share-links harness, which re-expresses error as apiErrorResponse({ … }): that restatement drops the #3867 5xx message-leak guard that HttpDispatcher.error() carries. Borrowing the deps object cannot drift, because it is not a copy.

routeNotFound and errorFromThrown are supplied real too, and the code comment is honest about why: handleDataRequest has no route-resolution exit of its own, and its throws are mapped one layer up by the dispatcher rather than through deps. They are real so a future /data branch is born conformant instead of inheriting a stand-in.

The proof (the deliverable, per the seat grade)

Convergence alone proves nothing, so each envelope rule was deliberately broken in production (packages/runtime/src/error-envelope.ts) and the same file run twice — once at origin/main, once with this change. Both breaks were reverted; git status is clean and the diff is the test file alone.

Break in buildApiError origin/main harness this harness
httpStatus dropped from the error member 5 passed (5) — sees nothing 4 failed, 5 passed
details.code left unpromoted (no splitSemanticCode) 5 passed (5) — sees nothing 1 failed, 8 passed

Real output, break 1 against this harness:

 × a missing object name is refused in the declared envelope (400)
 × an unresolved environment on a multi-tenant host is refused ... (428)
 × the exposure gate's throw reaches the wire as a conformant 404
 × a record miss keeps its OWN code ...
AssertionError: expected undefined to be 400 // Object.is equality
 Test Files  1 failed (1)
      Tests  4 failed | 5 passed (9)

Break 2 against this harness:

 × a record miss keeps its OWN code — promoted into `error.code`, not derived from the status
AssertionError: expected 'RECORD_NOT_FOUND' to be undefined
      Tests  1 failed | 8 passed (9)

Each break leaves today's file fully green. That gap is the card.

Worth recording, because it decides one assertion: ApiErrorSchema declares httpStatus optional and envelopeViolations does not look at it, so the schemas alone cannot produce break 1's red. The explicit expect(body.error.httpStatus).toBe(response.status) line is what does — the same line expectConformantError carries, and load-bearing for the same reason.

Phantom-check audit (#4984 family)

The direct answer: no existing assertion needed deleting, because not one of them ever touched an error exit. The audit's real finding is stronger than a list of phantom assertions — the three hand-written error exits were entirely unexercised. Nothing in the file read an error body, so the stand-in was not producing wrong answers that tests believed; it was dead scaffolding that made the wrong answers unreachable.

Case by case, all five survive the swap unchanged and were re-run green:

Existing case Verdict
a body object cannot move the read to another object Production. Asserts handled + findData arguments through the real callData.
still forwards the rest of the body as the query Production. findData arguments.
still honours an explicit query envelope Production. findData arguments.
threads the caller execution context through unchanged Production. Identity of the threaded context.
the exposure gate and the read agree on the path object Mostly production, one weak line — see below.

That last case's rejects.toMatchObject({ statusCode: 404 }) asserts the raw throw shape produced by action-execution.ts — production, and it survives. Its expect(res.response.status).toBe(200) is the one weak line: the 200 is a constant the success stand-in writes, so it discriminates which branch was taken (not refused, not 428) and nothing about the success envelope. Not empty, so not deleted under the "delete, don't port" rule — but not envelope coverage either, which is the bounded blind spot below.

Bounded blind spots, stated rather than claimed away

  1. success is still a stand-in. This card and its ruling scope the fix to the three error exits, so success: (data) => ({ status: 200, body: data }) is untouched — production answers { success: true, data, meta }. A regression in the success envelope still cannot go red here. Filed as /data harness: the success exit is still a stand-in, so a success-envelope regression cannot go red there either #7362 (observation-class, unassigned) rather than fixed as a rider.
  2. routeNotFound cannot be made to fail from this domain. handleDataRequest never calls it. It is real now, but unexercised — a reachability fact about /data, not a gap I can close with an assertion.
  3. errorFromThrown is exercised at the boundary, not through the domain. handleDataRequest throws; the dispatcher maps. The two throw cases apply the real method where production applies it rather than pretending the domain returns an envelope there.

Verification

  • pnpm --filter @objectstack/runtime test119 files / 1874 tests passed (1870 before; +4).
  • pnpm --filter @objectstack/runtime typecheck — clean (tsc --noEmit, no output).
  • pnpm check:type-check-debt after the full closure build — OK; @objectstack/runtime TEST_DEBT records 227, tsc now reports 225. Not raised.
  • node scripts/check-nul-bytes.mjs — OK, 6730 files, no raw control bytes; plus a targeted control-byte self-scan of the changed file.
  • pnpm exec eslint packages/runtime/src/domains/data-path-object.test.ts — clean.

skip-changeset: tests only, nothing user-visible ships, so there is nothing to release-note.

Scope: #7300 is in flight in the same directory on automation.ts / notifications.ts and a shared validation module. None of those were touched — this PR changes exactly one file.


Generated by Claude Code

@os-help os-help added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 10, 2026 — with Claude
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

No hand-written docs reference the 0 changed package(s). ✅

@github-actions github-actions Bot added the tests label Aug 10, 2026
@os-help
os-help marked this pull request as ready for review August 10, 2026 08:15
@os-help
os-help added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 4d94308 Aug 10, 2026
29 checks passed
@os-help
os-help deleted the claude/issue-6719-data-domain-envelope-double branch August 10, 2026 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/data domain 的 DomainHandlerDeps 替身答的不是 ADR-0112 信封,该架子测不出任何 error.code / httpStatus 缺陷

2 participants