test(runtime): drive /data's error exits through the REAL ADR-0112 envelope (#6719) - #7364
Merged
Merged
Conversation
Contributor
📓 Docs Drift CheckNo hand-written docs reference the 0 changed package(s). ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6719
The
/datadomain'sDomainHandlerDepsstand-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/datacase 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 exactDomainHandlerDepsobject every/datarequest runs against in production — borrowed off a dispatcher built over a kernel stub, exactly aserror-envelope.conformance.test.ts'smakeDispatcher()does. This goes one step further than #6649's/share-linksharness, which re-expresseserrorasapiErrorResponse({ … }): that restatement drops the #3867 5xx message-leak guard thatHttpDispatcher.error()carries. Borrowing the deps object cannot drift, because it is not a copy.routeNotFoundanderrorFromThrownare supplied real too, and the code comment is honest about why:handleDataRequesthas no route-resolution exit of its own, and its throws are mapped one layer up by the dispatcher rather than throughdeps. They are real so a future/databranch 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 atorigin/main, once with this change. Both breaks were reverted;git statusis clean and the diff is the test file alone.buildApiErrororigin/mainharnesshttpStatusdropped from the error memberdetails.codeleft unpromoted (nosplitSemanticCode)Real output, break 1 against this harness:
Break 2 against this harness:
Each break leaves today's file fully green. That gap is the card.
Worth recording, because it decides one assertion:
ApiErrorSchemadeclareshttpStatusoptional andenvelopeViolationsdoes not look at it, so the schemas alone cannot produce break 1's red. The explicitexpect(body.error.httpStatus).toBe(response.status)line is what does — the same lineexpectConformantErrorcarries, 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:
objectcannot move the read to another objecthandled+findDataarguments through the realcallData.findDataarguments.queryenvelopefindDataarguments.That last case's
rejects.toMatchObject({ statusCode: 404 })asserts the raw throw shape produced byaction-execution.ts— production, and it survives. Itsexpect(res.response.status).toBe(200)is the one weak line: the200is a constant thesuccessstand-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
successis still a stand-in. This card and its ruling scope the fix to the three error exits, sosuccess: (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/dataharness: thesuccessexit 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.routeNotFoundcannot be made to fail from this domain.handleDataRequestnever calls it. It is real now, but unexercised — a reachability fact about/data, not a gap I can close with an assertion.errorFromThrownis exercised at the boundary, not through the domain.handleDataRequestthrows; 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 test— 119 files / 1874 tests passed (1870 before; +4).pnpm --filter @objectstack/runtime typecheck— clean (tsc --noEmit, no output).pnpm check:type-check-debtafter the full closure build — OK;@objectstack/runtimeTEST_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.tsand a shared validation module. None of those were touched — this PR changes exactly one file.Generated by Claude Code