Skip to content

fix(rest): the package door carries the producer-marked userMessage (#12502) - #12541

Merged
os-litant merged 2 commits into
mainfrom
claude/issue-12502-package-routes-usermessage
Aug 26, 2026
Merged

fix(rest): the package door carries the producer-marked userMessage (#12502)#12541
os-litant merged 2 commits into
mainfrom
claude/issue-12502-package-routes-usermessage

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes#12502

sendThrownError (packages/rest/src/package-routes.ts) resolved every throw through the shared ADR-0112 rule and then forwarded { details, declaredCode } and nothing else — so the producer-marked userMessage it was already holding in the same local was dropped one line later. It now spreads thrown.userMessage into the envelope writer's extra, the third independent spread into the object PR #12508 introduced.

Nothing invalid shipped, which is what made the loss silent: code, status and message were all correct and every body parsed, while an author's deliberate, end-user-addressed sentence vanished and a consumer told by ADR-0112 to render userMessage verbatim found nothing there and fell back to its generic #3821 substitution — the behaviour this channel exists to override.

The idiom is the INVERSE of the declaredCode half, deliberately

constdeclaredCode=demotedDeclaredCode(thrown);// helper: presence MEANS demotionconstextra={
...(thrown.details ? {details: thrown.details} : {}),
...(declaredCode!==undefined ? { declaredCode } : {}),
...(thrown.userMessage!==undefined ? {userMessage: thrown.userMessage} : {}),// RAW};

declaredCode must be read through demotedDeclaredCode because its raw field carries a second meaning — it is also set when the producer's spelling IS the registered member, so forwarding it raw would put two spellings of one fact on every registered refusal. userMessage has no second meaning: declaredUserMessage already decided what counts as marked (a non-empty string, or nothing), so presence means only "the producer opted in" and the caller passes the field straight through. That is byte for byte what the dispatcher twin serving this same path does (errorFromThrown, packages/runtime/src/http-dispatcher.ts). Inventing a caller obligation here to match the sibling would have been the mistake, not the safe choice, and section 3 of the new suite is what holds the two apart mechanically.

The reachability standard this door is now judged on

Stated here rather than left implicit, because it is the honest cost and the next channel added to this writer should not have to rediscover which bar applies.

Measured on origin/main33c5fd3d2: the IN-TREE producer set at this door is EMPTY — for this channel and for declaredCode alike. The only in-tree writer of userMessage onto a throwable is the QuickJS sandbox relay (packages/runtime/src/sandbox/quickjs-runner.ts:1404), and it cannot reach these seams: packageService.publish/get/list/delete reach the store through raw objectql.execute, which dispatches no hook and no sandbox, while protocol.getMetaItems and protocol.deletePackage re-wrap every engine.find failure into a fresh metadataStoreUnavailableError (only code/status/cause survive; declaredUserMessage never reads cause) and absorb per-item and cleanup throws into failed[]/cleanups[], which carry no such channel. The same holds for declaredCode: every in-tree throw escaping these seams spells a registered code (TENANT_SCOPE_REQUIRED, SERVICE_UNAVAILABLE) or none.

That is not a reason to withhold either channel, and the ruling is on the card:

  1. This door is composed, not closed.resolvePackageService() and the protocol slice are open composition points, and all four handlers forward their throws to sendThrownError verbatim with nothing in between — demonstrated rather than argued, see below.
  2. A third-party implementation of either seam is a real deployment shape.
  3. ADR-0112's federation amendment exists precisely because the producer set is not enumerable in-tree. An in-tree-only bar declares the federated limb dead by construction, and a liveness test that can only ever answer "dead" for the federated case is not measuring liveness.

So the live population here is the injected/federated limb, and that is exactly the population the new pins drive.

⚠️ One seam in the table is drivable in tests but not production-reachable, and it is named in the suite so nobody reads it as evidence of the opposite: the capability-gate resolver. In production it is RestServer.resolvePackageRouteExecutionContext, which returns this.resolveExecCtx(...).catch(() => undefined) over a private async method, so it can never throw synchronously and its rejections are swallowed. It is kept because it covers the fourth catch-reaching seam under a composed host; its value is coverage of the catch site, never a claim about producers.

Not in scope, and it does not go away under this change

A metadata app's sandboxed hook that marks a refusal has that mark destroyed at the producer (metadataStoreUnavailableError keeps only code/status/cause), so widening this writer does not make that path live. Filed separately by the PM seat; the contract-first fix is at metadata-protocol, and it trades against the #8136 disclosure fix, so it needs a ruling rather than a patch. ⛔ Not touched here.

packages/rest/src/rest-server.ts is untouched (hard serial behind PR #12421). packages/spec/** is the contract being honoured, not changed. packages/types/src/response-envelope.ts is read-only here — its extra has admitted this field since #12404.

Verification

Ran on the final commit 36f9ba12e:

  • pnpm --filter '@objectstack/rest^...' build — EXIT 0 (so no verdict is read off a stale dist).
  • pnpm --filter @objectstack/rest test149 files, 2441 tests passed, EXIT 0.
  • Door suites together (new + package-door-declared-code + package-routes-coded-error-mapping + package-door-5xx-message-sanitization + package-envelope.conformance) — 5 files, 238 tests passed, EXIT 0.
  • pnpm --filter @objectstack/rest typecheck — EXIT 0. ⚠️That command does not cover the new test file: packages/rest/tsconfig.json excludes **/*.test.ts and this package has no tsconfig.test.json and no test-typecheck-debt.json, so no tsc program compiles its tests (pre-existing, reported to the PM, not repaired here). The new file was therefore type-checked explicitly through a throwaway config that does not exclude tests — tsc --noEmit --listFiles EXIT 0 with the file confirmed present in that program, so this is a measurement rather than a green over source nothing read.
  • Gate families derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no hand-written path list) and run: check:route-envelope, check:test-source-alias, check:cross-package-test-inputs, check:dispatcher-error-vocabulary, check:type-source-resolution, check:type-check-coverage, check:engine-double-contract, check:where-matcher, check:objectql-double-limit, check:query-options-erasure, check:page-declaration-shape, check:slot-lookup, check:published-files, check:objectui-changeset, check:changeset-gate-self-tests, plus check-empty-changeset, check-changeset-no-major, check-adr-0087-registration, check-comment-mask-adoption, check-plugin-teardown-shape, check-ci-filter-parity, check-cross-package-test-inputs — all EXIT 0. The ratchet subset was re-run on the final commit after the last edit, head unchanged before and after.
  • ⛔ Declared narrowing: check:type-check-debt --re-measure was not run locally — it requires the full workspace closure built and refuses on an unbuilt one, so a local run here would have been NOT MEASURED rather than green. CI runs it.

What the tests pin

38 new assertions in packages/rest/src/package-door-user-message.test.ts, over all four seams:

  1. The mark reaches the wire at every seam, on a 403, a 409 (statusCode spelling), a 503 and a throw that declared no status — status-agnostic by the ruling that created the channel — with codeandstatus asserted on every row and a witness that the throw really travelled the seam.
  2. Absence — no userMessage is invented: a throw that declared none, plus the three shapes declaredUserMessage rejects (empty string, whitespace-only, non-string), each asserted at the door and at the resolver.
  3. Independence — one throw declaring BOTH channels carries both, each under its own rule; a registered code plus a mark carries userMessage and withholds declaredCode; an unregistered code with no mark does the reverse; details rides untouched alongside. A later author who "harmonises" the two by wrapping userMessage in a demote-style helper goes red here.
  4. The 5xx prose withhold does not reach the marked channel — the leaked SQLITE_ERROR: no such table: sys_packages message is replaced by INTERNAL_ERROR_MESSAGE while the mark rides through, and the body contains neither the physical table name nor the phrase. Pinned rather than argued because it is a premise that can rot.

Generated by Claude Code

…#12502)
`sendThrownError` in `packages/rest/src/package-routes.ts` resolved every throw
through the shared ADR-0112 rule and then forwarded `{ details, declaredCode }`
and nothing else, so the marked text it was already holding in the same local
was dropped one line later. It now spreads `thrown.userMessage` into the
envelope writer's `extra` — the third independent spread into that object, and
the idiom the dispatcher twin serving this same path (`errorFromThrown`, #9934)
already applies.
The idiom is the INVERSE of the `declaredCode` half directly above it, and that
is the whole reason this is a separate change. `declaredCode` must be read
through `demotedDeclaredCode` because its raw field carries a second meaning; it
is also set when the producer's spelling IS the registered member. `userMessage`
has no second meaning — `declaredUserMessage` already decided what counts as
marked — so the caller passes the raw field straight through and invents no
obligation to match the sibling.
Additive: an unmarked refusal still carries none, and the three shapes
`declaredUserMessage` rejects (empty, whitespace-only, non-string) still carry
none, so nothing invents a mark for a producer that never wrote one.
`details`, `declaredCode`, `code`, `status` and `message` are unchanged on every
existing path, and the 5xx prose withhold never sees the marked channel.
The reachability standard this door is now judged on is stated in the changeset,
in the new suite's docblock and in the PR body rather than left implicit: the
in-tree producer set here is empty for BOTH channels, and the live population is
the injected/federated limb this door composes rather than closes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
@github-actions

github-actionsBot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 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 a8d6b1d16a5edf2cbba87291f8ecfc6032b3c2d3packageMentionDocs.

Which tree this was computed on

This run read content/docs from 08e0f8fbf92d72d2d2bd62c25e30558d2a1230e9 — the merge of head 1c6250c68e05890a553ac1dfd7753f3d8e0ff834 into base a8d6b1d16a5edf2cbba87291f8ecfc6032b3c2d3, 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 08e0f8fbf92d72d2d2bd62c25e30558d2a1230e9 && git checkout 08e0f8fbf92d72d2d2bd62c25e30558d2a1230e9
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a8d6b1d16a5edf2cbba87291f8ecfc6032b3c2d3 1c6250c68e05890a553ac1dfd7753f3d8e0ff834 && git checkout -B drift-repro a8d6b1d16a5edf2cbba87291f8ecfc6032b3c2d3 && git merge --no-ff 1c6250c68e05890a553ac1dfd7753f3d8e0ff834
node scripts/docs-audit/affected-docs.mjs --json a8d6b1d16a5edf2cbba87291f8ecfc6032b3c2d3

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

…eset (#12502)
`.changeset/package-door-declared-code.md` closed with "the shared `sendError`'s
`extra` does not admit it yet". That clause was already false when it merged:
PR #12477 (#12404, `0e5fe7fc0`) widened the `extra` Pick to admit `userMessage`
and landed BEFORE `14b114553`. Both changesets are unreleased and compile into
the same CHANGELOG, so this fixes a document before it is published rather than
rewriting history — and it is fixed in place, because a correction sitting
beside the false sentence would ship both to the reader.
One clause. The rest of that changeset's prose is untouched, and no other
changeset is touched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
@os-litant
os-litant marked this pull request as ready for review August 26, 2026 09:50
@os-litant
os-litant added this pull request to the merge queueAug 26, 2026
Merged via the queue into main with commit 092b9daAug 26, 2026
34 checks passed
@os-litant
os-litant deleted the claude/issue-12502-package-routes-usermessage branch August 26, 2026 10:05
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rest: package-routes' sendThrownError drops thrown.userMessage too — the second declared channel, now that sendError's extra admits it

2 participants

@os-litant@claude