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

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 realHttpDispatcher, 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 buildApiErrororigin/main harnessthis harness
httpStatus dropped from the error member5 passed (5) — sees nothing4 failed, 5 passed
details.code left unpromoted (no splitSemanticCode)5 passed (5) — sees nothing1 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 httpStatusoptional 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 caseVerdict
a body object cannot move the read to another objectProduction. Asserts handled + findData arguments through the real callData.
still forwards the rest of the body as the queryProduction.findData arguments.
still honours an explicit query envelopeProduction.findData arguments.
threads the caller execution context through unchangedProduction. Identity of the threaded context.
the exposure gate and the read agree on the path objectMostly 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-helpos-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). ✅

@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 queueAug 10, 2026
Merged via the queue into main with commit 4d94308Aug 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 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.

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

2 participants

@os-help@claude