Repository navigation
[finding] registerSecurityEndpoints answers THREE different error-envelope shapes on the same three routes (ADR-0112) #7981
Description
Activity
Triage:
pm:queue, routeddomain:cli(packages/rest/src/rest-server.ts), type Bug.- Grade rationale (ruling inheritance): the destination is already ruled — ADR-0112 D5 fixes
error.codeas the semantic-code carrier, and finding:rest-server.ts里三个相邻/metahandler 的错误信封是三种不同形状,其中两种不符合 ADR-0112 #7035 (PR fix(rest): put the/meta501 refusals inside the ADR-0112 error envelope (#7035) #7293) retired the bare-stringerrordialect in this very file. Converging the three arms ofregisterSecurityEndpointsonto the nested{ error: { code, message } }shape the routes' own validation refusals use is implementation of an existing ruling, not a new contract question. - Dispatch preconditions: run the finding:
rest-server.ts里三个相邻/metahandler 的错误信封是三种不同形状,其中两种不符合 ADR-0112 #7035 consumer-sweep methodology first (grepcode === '…'as well ascode: '…') before moving any shape a consumer might branch on — the card names this itself. ThehandleErrorarm carries the typed 403/404/409 codes, i.e. the arm most likely to be branched on. - Explicitly out of scope: the flat-REST vs
{ success: false, … }dispatcher-wrapper question (Envelope drift is not just service-storage: four more route modules emit bare bodies, two of them the pre-#3675{ error: '<string>' }#3843 family) is not re-opened; converge within the REST flat family per D5. The behaviour-vs-mount-presence argument for the parity gate stays on Three ledgered /meta routes are never mounted and die in the/meta/:typecatch-all — the route audit can't see this class because it treats the ledger as ground truth for what's mounted #7526/suggested-binding-loop (b): unknown ?status on /security/suggested-bindings returns 200 empty instead of 400 (live REST route skips validation) #7678 — this card is the three-arm convergence only.
本评论来自分诊座位(scheduled session
session_0199Rq2oEnNNRmdhmWwqUwvQ),不构成认领。
Generated by Claude Code
- Grade rationale (ruling inheritance): the destination is already ruled — ADR-0112 D5 fixes
Claimed by the
domain:cliPM seat (#6024, sessionsession_01B3Kurx8qufrDzNjk4rag7V).
Branch:claude/issue-7981-security-endpoints-envelope-convergence· dispatch:mode:subagent,model: claude-opus-5.Anchor re-verified on
origin/main:registerSecurityEndpointscarriesrespond501doingres.status(501).json({…})andrespondSharingErrordoingres.status(status).json({ …, error: msg.replace(…) })— a bare-stringerrorfield, the dialect #7035 retired.Ruling: converge on ADR-0112, and do not invent a fourth
One region, three envelope shapes, on three routes that a single client calls in sequence. Land all three on the standard envelope the rest of the route family already answers in. ⛔ Do not author a new shape "to unify them" — the target already exists.
Take the shape from a sibling in the same file rather than from the ADR text. This lane merged #7979 into
registerSecurityEndpointstoday; match what it answers so the region converges rather than acquiring a fourth dialect from a different reading of the same document.The one thing to decide carefully
The bare-string
errorfield is a wire-visible change for any caller currently readingbody.erroras a string. Determine whether anything reads it — client SDK, console, tests — with a control grep (a query you know returns hits) so a zero-hit result proves absence rather than a broken search. If a reader exists, say so and stop: that turns this from a convergence into a breaking change needing a ruling.respond501's status is not in scope — this is about the body shape, not about whether 501 is the right code.Pins
- Each of the three arms asserts the ADR-0112 pair —
codeand HTTP status. Asserting one is not acceptable. - A pin that the three arms now answer the same shape as each other, derived rather than three hand-written literals.
- Reverse-verify: revert one arm, confirm its test fails on the shape, restore.
- ⛔
check:type-check-debtmust not rise —@objectstack/restis at 155 and that entry has no margin.
Generated by Claude Code
- Each of the three arms asserts the ADR-0112 pair —
{ "issue": 7981, "branch": "claude/issue-7981-security-endpoints-envelope-convergence", "pr_url": "https://github.com/objectstack-ai/objectstack/pull/8071", "status": "needs_decision", "premise_still_valid": true, "summary": "The three arms of registerSecurityEndpoints now emit the ADR-0112 body { error: { code, message } } through ONE local respondError helper: respond501 (was flat { code, message }), handleError (was { code, error: 'bare string' }) and the ?status validation guard (already conformant, re-routed through the same emitter so it is the reference point by construction). No code value and no HTTP status moves; nothing in packages/spec changes. Implementation, tests, changeset and a DRAFT PR are complete and green. Status is needs_decision because the dispatch reserved the wire-visibility call for the PM: readers of the bare-string `error` DO exist (two), so per the card I report and stop short of the ruling rather than treating the change as landable.", "target_shape_source": "packages/rest/src/rest-server.ts — the sibling arm in the SAME file, not the ADR prose: the #7678 ?status guard PR #7979 landed in registerSecurityEndpoints today writing { error: { code, message } }, and refuseRepeatedQueryParams (packages/rest/src/query-multiplicity.ts:139-141) writes the identical body for the whole file. No fourth dialect authored.", "arms_converged": [ "respond501 — 501 NOT_IMPLEMENTED: was { code, message } (no error wrapper)", "handleError typed arm — 403 PERMISSION_DENIED / 404 SUGGESTION_NOT_FOUND / 409 SUGGESTION_STATE: was { code, error: 'bare string' }", "handleError fault arm — 500 SUGGESTION_{LIST,CONFIRM,DISMISS}_FAILED: was { code, error: 'bare string' }, 500-char cap preserved", "?status validation guard — 400 VALIDATION_ERROR: already ADR-0112, re-routed through the shared emitter", "refuseRepeatedQueryParams — 400 VALIDATION_ERROR: untouched, shared with the whole file, pinned as the reference point" ], "bare_string_readers": { "found": true, "count": 2, "verdict": "Both readers are deliberately MULTI-ENVELOPE and read the ADR-0112 position too, so the migrated values arrive identical or better. No reader breaks. The go/no-go is still the PM's — reporting, not deciding.", "readers": [ "packages/client/src/index.ts — ObjectStackClient.fetch, the if (!res.ok) branch (NOT unwrapResponse, which never sees a failure because fetch throws first). Chain: errorMessage = errorBody?.message ?? errorBody?.error?.message ?? (typeof errorBody?.error === 'string' ? errorBody.error : undefined) ?? res.statusText; errorCode = asSemanticCode(errorBody?.code) ?? asSemanticCode(errorBody?.error?.code). Its own comment calls the two reads 'the two LIVE envelopes declared spots, not a fallback chain'. After the change every exposed field is value-identical: 501 message/code move top-level -> nested with the same strings; the typed arm moves off the plain-string limb onto error.message; err.httpStatus comes from the transport; err.details already fell through to the whole body on both flat shapes and still does.", "objectui packages/app-shell/src/services/suggestedBindingsApi.ts:41-42 — payload?.error?.code ?? 'HTTP_'+status, and payload?.error?.message ?? payload?.error ?? res.statusText. Canonical-FIRST, so the change strictly improves it: a 403 confirm denial reports HTTP_403 today and PERMISSION_DENIED afterwards, with the same message text. Cannot render [object Object] — the chain ends in a typeof message === 'string' guard. Nothing branches on the code (errorCodeIs is used on the marketplace API, never on this one); the panel toasts err.message, unchanged." ], "control_grep": [ "objectui branching: grep -rn 'code ===' in the 3 suggested-bindings files -> 0 hits; CONTROL same query over packages/app-shell/src -> 15 hits (RecordApprovalsPanel THROTTLED, DatasetPreview ANALYTICS_NOT_INSTALLED, ...). Zero is absence, not a broken search.", "cloud: grep -rn 'suggested-bindings|suggestedBindings' -> 0 hits; CONTROL 'api/v1/security' -> 2 hits (service-cloud/src/cloud-stack.ts:307, apps/cloud/test/control-plane-default-profile.test.ts:505).", "objectstack tests: grep -rn 'SUGGESTION_LIST_FAILED|SUGGESTION_CONFIRM_FAILED|SUGGESTION_DISMISS_FAILED' -> 6 hits, all emitter (rest-server.ts) or vocabulary registry (spec/src/api/error-code-ledger.zod.ts); no assertion anywhere. CONTROL 'NOT_IMPLEMENTED' in packages/rest/src/*.test.ts -> 8+ files.", "docs/QA: docs/qa/platform-checklist/areas/access-security.json names statuses and codes ('409 SUGGESTION_STATE', '404 SUGGESTION_NOT_FOUND') but never a body POSITION; CONTROL grep 'body.code|body.error|error.code' over that file -> 3 hits, all on OTHER items, proving the query would have found a position clause." ] }, "tests_added": "packages/rest/src/security-suggested-bindings-envelope.test.ts (23 cases). Every refusal case asserts the ADR-0112 PAIR — HTTP status AND nested body.error.code — plus both retired dialects absent (no top-level `code` sibling, `error` an object not a string). Arms: 501 on all three routes + the duck-typed 'service predates this surface' variant; typed 403/404/409 on all three routes (table-driven); untyped 500 with each route's own default code; the 500-char cap; the 400 validation and 400 multiplicity refusals. CROSS-ARM PIN IS DERIVED: shapeOf() reduces a body to its sorted key-path/value-type skeleton and the family case asserts all 9 refusals collapse to ONE skeleton without naming it, so a fourth dialect fails even if a matching literal case is added alongside. Preservation: list/confirm/dismiss still 200 with { data } and no `error` key.", "tests": "pnpm --filter @objectstack/rest test --maxWorkers=2 -> 'Test Files 98 passed (98) / Tests 1589 passed (1589)'. pnpm --filter @objectstack/rest typecheck (tsc --noEmit) -> clean, no output. pnpm check:type-check-debt (after pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*', which the gate demands) -> 'check-type-check-coverage --re-measure: OK — 35 ledger entr(ies) re-measured, 1975 raw tsc error(s) total, none above its recorded number'; @objectstack/rest is absent from the 9-entry 'can be lowered' list, i.e. it re-measured at exactly its recorded 155 — the ledger does not move. pnpm check:error-code-casing -> 'no lowercase error codes in 3740 scanned file(s)'. node scripts/check-nul-bytes.mjs -> OK, 7333 files, no raw control bytes.", "reverse_verification": "Fix committed FIRST (083848eeb) so the restore point was a real commit. Reverted the handleError typed arm alone back to res.status(status).json({ code: err?.code ?? defaultCode, error: String(...) }) and re-ran the new suite: 12 failed / 11 passed — all SHAPE failures, no compile error, and the file still type-checked. Representative reds: (1) 'typed 403 list has no nested code: expected undefined to be string'; (2) 'typed 403 list still has a top-level code sibling: expected { code: PERMISSION_DENIED } to not have property code'; (3) the derived cross-arm pin 'expected 1, received 2' distinct skeletons — i.e. it detected the split without being told what the shape is. Direction was as predicted (before-green/after-red), no inversion. Restored with git checkout HEAD -- packages/rest/src/rest-server.ts; git status clean, line re-verified present. git stash never used (worktree does not isolate refs/stash).", "open_questions": [ { "question": "Two readers of the bare-string `error` exist, so the card's stop rule fires — but its stated consequence ('this becomes a breaking change') is measurably FALSE for both: each reads the ADR-0112 position FIRST or alongside, and both come out value-identical (SDK) or strictly better (console: HTTP_403 -> PERMISSION_DENIED). Land the convergence, or hold it as a breaking change?", "options": [ "A — flip PR #8071 ready and land it. Both readers keep working with no change on their side; the console gains the real semantic code it currently cannot see; the ADR-0112 D5 destination was already ruled and #7035 retired this dialect in this same file.", "B — treat any bare-string reader as breaking and hold for a maintainer ruling / a coordinated objectui change. Costs the convergence for now and leaves body.error.code reading undefined on the arm carrying the typed 403/404/409 codes." ], "recommendation": "A. The rule's trigger is met but its rationale is not: neither reader can break, because neither is single-dialect — the SDK's own comment declares it reads both live envelopes on purpose, and the console's string limb is the SECOND limb of a canonical-first chain that this change stops reaching. Under Prime Directive #12 those limbs are consumer-side tolerance to be paid down at the producer, which is exactly what this PR does. I have NOT taken that decision: the PR is parked in draft and no ready-flip was made." } ], "out_of_scope_findings": [ "filed as #8073: the two registrars adjacent to this one in the same file still answer the retired dialects — registerSecurityExplainEndpoints (/security/explain, /security/my-delegable-scope: 401/501/400/403 flat { code, message }, two 500s bare-string) and registerSharingEndpoints' respondSharingError ({ code, error: msg.replace(...) } feeding 400/403/404/409/422). Searched first (keyword + file path) — no existing issue; #7295 and #7035 are the closed relatives. Filed unassigned and unlabeled for PM triage, standalone (not a sub-issue: neither fix falls inside this card's completion scope), and it does not block on this PR." ], "risks": [ "Wire shape change on three live routes. Mitigated by the two-reader measurement above and stated plainly in .changeset/security-endpoints-envelope-convergence.md, which names the bare-string field as the wire-visible half.", "The claim comment's anchor named `respondSharingError` as living in registerSecurityEndpoints; measured, it is at rest-server.ts:9218 inside registerSharingEndpoints, one registrar earlier — the security family's bare-string emitter is `handleError` (the issue BODY named it correctly). The premise holds; only the paraphrase pointed at the neighbour, which is now filed as #8073. Nothing was fixed in the sharing family.", "objectui carries no test that stubs these wire bodies (its two touching tests mock the panel component wholesale), so nothing on that side goes red either way — the console's improvement is measured by reading the chain, not by a failing test. If the PM wants that pinned, it is an objectui-side card.", "Local verification was scoped per the dev contract (rest package + the gate families this card touches: type-check-debt, error-code-casing, nul-bytes) rather than the full local farm; CI convergence is the PM's read." ] }
Generated by Claude Code
- added a commit that references this issue
on Aug 12, 2026 os-dev-report
{ "issue": 7981, "status": "done", "branch": "claude/issue-7981-security-endpoints-envelope-convergence", "pr": "https://github.com/objectstack-ai/objectstack/pull/8071", "premise_still_valid": true, "summary": "PR #8071's red 'ESLint' job was actually pnpm check:route-envelope refusing to let banked progress go unrecorded: the PR's conversion of registerSecurityEndpoints' two handleError arms onto the shared respondError helper moved 2 sites off the siblingCode dialect (found 75, declared 77). Confirmed the delta is this PR's own (not a stale-main artifact) by measuring siblingCode at the merge-base with origin/main (77) vs branch head before this commit (75) -- exact delta of 2 -- then diffing site line numbers under the PR's own uniform line-shift: every other siblingCode site persists unmoved, and the only two that vanished are merge-base lines 9529/9532, both inside registerSecurityEndpoints and both converted by this PR's diff. Lowered the declared siblingCode for packages/rest/src/rest-server.ts from 77 to 75 in scripts/check-route-envelope.mjs MODULES (one-line change plus a note), committed, and pushed to the existing branch. No numbers raised, no envelope-divergent sites reintroduced, releases/ untouched, PR left in draft, no new changeset (this PR already carries one).", "tests": "pnpm check:route-envelope (self-test + real audit): green -- '9 route module(s) audited: 7 conformant, 1 ratcheted, 1 exempt', ratchet line now reads 'siblingCode 75'. pnpm --filter @objectstack/rest test: 'Test Files 98 passed (98) / Tests 1589 passed (1589)'. pnpm --filter @objectstack/rest typecheck (tsc --noEmit): clean, no output. pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*': 70 successful/70 total (closure needed before type-check-debt). pnpm check:type-check-debt: 'OK -- 64/77 workspace packages type-checked ... none above its recorded number'; @objectstack/rest's TEST_DEBT ceiling (155, zero margin) is absent from the 'can be lowered' informational list, i.e. unchanged and not raised. node scripts/check-nul-bytes.mjs: OK. Merge-base vs head siblingCode measurement (via scanSource copied out of the gate script, run against git show <rev>:packages/rest/src/rest-server.ts): merge-base e3c8ed0f8 -> 77, branch head 5eb732c84 (pre-fix) -> 75; delta 2, matching the gate's own report; the two vanished sites (merge-base lines 9529, 9532) fall inside registerSecurityEndpoints, the exact function PR #8071's diff touches.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
Generated by Claude Code
observation-class finding, recorded while fixing #7678 (PR #7979). Not fixed there — that card was scoped to the
?statusvalidation, and this is a separate, wider shape. Filed unassigned.Fact
The three
/api/v1/security/suggested-bindingsroutes inpackages/rest/src/rest-server.ts→registerSecurityEndpointsemit three mutually incompatible error envelopes, depending only on which arm refuses:?status{ error: { code, message } }refuseRepeatedQueryParams/ the new status guard{ code, message }— noerrorwrapper at allrespond501{ code, error: "<string>" }—erroris a bare stringhandleErrorSo a client cannot read one field to learn why its call failed on these routes:
erroris an object in one arm, absent in another, and a human-readable string in the third — and the semantic code sits aterror.codein the first and at top-levelcodein the other two.The third shape is specifically the dialect #7035 retired (PR #7293 converged this file's
/meta501 refusals off it).handleErroris the arm that carries the typed service errors the route's own docblock advertises — permission 403, not-found 404, state-conflict 409 — i.e. the arm consumers are most likely to branch on.Why it is worth recording separately
Under ADR-0112 D5 the destination is fixed:
error.codecarries the semantic code, the HTTP status lives on the transport. All three arms here predate that convergence, and none of them is wrong individually — they are wrong as a set, which is exactly the class no per-arm review catches.It is also a behaviour divergence against the runtime dispatcher twin (
packages/runtime/src/domains/security.ts), which routes every one of these outcomes throughdeps.error/deps.errorFromThrown→apiErrorResponse, i.e. one shape:{ success: false, error: { code, message, httpStatus } }. That makes it a concrete input to the route-ledger↔live-mount parity gate #7526, and specifically to the argument #7678 makes — that the gate should cover behaviour divergence, not just mount presence. A gate comparing only mount presence is green on all three of these.Not claimed here
{ success: false, … }dispatcher wrapper or the REST flat shape is the target: that is the envelope-convergence line (Envelope drift is not just service-storage: four more route modules emit bare bodies, two of them the pre-#3675{ error: '<string>' }#3843 family), not this issue.rest-server.ts里三个相邻/metahandler 的错误信封是三种不同形状,其中两种不符合 ADR-0112 #7035's sweep methodology (grep forcode === '…', not onlycode: '…') applies before any rename.Suggested handling
Triage-level, not decided here: converge the three arms in
registerSecurityEndpointsonto the nested{ error: { code, message } }shape the same route already uses for its two validation refusals — or fold it into whatever batch is carrying the #7035 convergence throughrest-server.ts, since this is the same edit repeated per route family.Generated by Claude Code