fix(rest): converge the record-sharing family onto the ADR-0112 D5 envelope (#8111) - #8212
Conversation
…velope (#8111) `registerSharingEndpoints` answered two retired dialects across its nine refusal arms: `respond501` was flat `{ code, message }`, and the five mapped verdicts (400/403/404/409/422) plus the three verb-specific 500s were `{ code, error: '<bare string>' }`. `body.error.code` — the one position ADR-0112 D5 declares — read `undefined` on all nine. Every arm now emits through the shared `sendError` from `@objectstack/types` (imported here as `sendEnvelopeError`; this module has a local `sendError` of its own for thrown errors), so the family agrees by construction rather than by nine literals that happen to match. No status code moves and no code VALUE changes. The 409 arm's `CONFLICT` was registered in neither `StandardErrorCode` nor `ERROR_CODE_LEDGER` — so `ApiErrorSchema`, whose `code` is a closed enum, would have rejected that body — and is now registered under `@objectstack/rest`, keeping the emitted value byte-identical. Renaming it onto `RESOURCE_CONFLICT` would change what clients read and is filed separately. The `CODE:` message prefix stays: censused as a server-internal service→REST derivation, stripped before the response is written, never on the wire. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 2 package(s): 108 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
PM review —
|
…8111) Registering `CONFLICT` in `ERROR_CODE_LEDGER` grows `ErrorCode` — the union `ApiErrorSchema.code` parses against — by exactly one member, and every generated reference page that renders an `ApiError`-shaped field prints that enum as a truncated summary with a "+N more" count. So one ledger row moves 134 counter lines across 11 pages, plus the two pages that list the vocabulary in full gain a `CONFLICT` bullet. Generated output only (`gen:schema` + `gen:docs`), never hand-edited. Measured as a genuine cascade, not absorbed drift: a pristine `origin/main` tree regenerates completely clean under the identical commands, and every one of the 136 changed lines here is either the `+264 more` → `+265 more` counter (134) or a `CONFLICT` bullet (2) — nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
|
CI fix pushed ( The six extra files are (a) — a genuine cascade of this PR, not pre-existing drift. Measured rather than assumed, and the measurement also corrected two details of the brief:
The causal chain is mechanical: registering Verified exhaustively: of 136 changed lines, 134 are that No hand-edits: every file is generator output. Nothing filed as a finding here — the finding class the brief anticipated (a generator drifting from committed artefacts until an unrelated PR notices) is exactly what the pristine- Generated by Claude Code |
PM review of the follow-up (
|
Ejected from the merge queue (CI_FAILURE) — re-queued, with a hypothesis rather than a blind retryState when ejected: this PR's own head ( Re-queued (auto-merge re-enabled; the ejection clears it). But recording the most likely cause first, so that if it ejects again nobody re-tries a third time on hope: The likely cause is order-sensitivity that this PR introduced by constructionThe regenerated pages under That makes the committed artefacts a function of the whole
The queue was processing #8217, #8219 and #8199 in the same window, so a batch-mate touching the same enum is a live possibility, as is a plain flake or an unrelated batch-mate failure. If it ejects again⛔ Do not retry a third time. The fix is not a code change — it is to rebase onto current ⛔ And do not "fix" it by hand-editing the count in the generated pages — This is worth a finding in its own right regardless of how the retry goes: a generated artefact that encodes a repo-global count is inherently merge-queue-hostile, because every concurrent PR touching that global invalidates every other PR's committed copy. The count is a rendering convenience; the invariant it breaks is "my artefacts are a function of my diff." I will file that separately rather than expand this card. Generated by Claude Code |
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31641134661 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Correction — my hypothesis above was wrong, and I have dequeued this PRThe triage bot extracted the real failure from queue build 31641134661:
Nothing to do with the generated enum counter. My comment above reasoned confidently from a plausible mechanism I had constructed rather than from the failure, and posted it as the likely cause — on a durable record, where the next reader would have started from it. That is the same mistake I flagged two comments earlier about reading a truncated log as an inventory: I again theorised where I should have fetched. Ironically I had also just written "the PR being green tells us nothing about why the queue leg failed" — and then explained the queue leg anyway, without looking at it. Also corrected: I framed the queue's difference as testing a different commit. The bot names the mechanism that actually matters here — the queue runs the FULL suite; PR-side CI runs only the affected subset. So this test never ran on the PR head at all. "Our CI is 26/26 green" was never evidence about it. Dequeued, not re-queuedI had re-enabled auto-merge before the triage comment arrived. That is exactly the blind re-queue the bot warns against — every one burns a rebuild for every PR behind it — so auto-merge is now disabled and this PR is out of the queue until the failure is understood. Being triaged now, and one thing that matters more than this testSent back to the implementer to determine whether this is (1) a real regression from the envelope change, (2) pre-existing on The larger question: the census reported C7 — dogfood suites, no No further re-queue until that comes back. Generated by Claude Code |
…ring-envelope-convergence
⛔ merge queue 构建失败 — 先分诊，再决定要不要重排队列构建 31642003913 红了。队列跑的是全量套件（PR 侧 CI 只跑 affected 子集）， 失败的 job（日志抽取，best effort）：
历史信号：
分诊清单：
Generated by Claude Code · merge-queue-triage workflow (#4859) |
…position (#8111) `federated-phantom-share-grant.dogfood.test.ts` (added by #8209 / PR #8209 for #8119) asserts the `/data/:object/:id/shares` refusal envelope. It was written against the FLAT dialect — `body.code` / `body.error` as a bare string — because that is what the route emitted when it landed. This PR converges that family onto `{ error: { code, message } }`, so the pin reads `undefined` and fails. Migrated, not loosened: the assertions move to `body.error.code` and `body.error.message` and assert the D5 position ALONE. Accepting either shape would re-admit the dialect this card retires. The CONTROL case's negative assertion moves too. It did not fail — a negative assertion on the vacated flat position passes for free (`undefined !== 'SHARING_NOT_ENABLED'`) — which is exactly why it had to move: left alone it would have gone on "passing" while reading a key no response carries any more. Why the queue caught this and PR CI did not: the merge queue runs the FULL suite, PR-side CI only the affected subset, and neither PR alone is red — #8209 was green before this convergence existed and this branch was green before #8209's pin existed. The interaction is only visible composed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
|
Queue failure diagnosed and fixed ( Mechanism
const body = await res.json() as { code?: string; error?: string };
expect(body.code).toBe('SHARING_NOT_ENABLED');Post-convergence Neither PR is wrong alone: #8209 was green because the flat shape was live when it landed; this branch was green because PR CI runs the affected subset and never executes that dogfood file, while the queue runs the full suite. Timing — #8209 landed Fixed by migrating the pin to D5, not by loosening it — On the C7 census claim — the generous reading is only half right, and I want the record accurateThe suggested framing was that the census was fine and merely went stale. Stale is true, but it was also wrong when I ran it, and that part is mine.
Worth noting as a general hazard rather than a one-off: a census is a measurement with a timestamp, and on a repo landing ~18 merges a day, a claim about "what reads this" can be falsified by a merge between the census and the queue — with no signal on the PR, because PR CI structurally cannot see it. Verification on the merged branch
Integrated by merge, not rebase, deliberately: the branch is already pushed, so rebasing would need a force-push. Not re-queued, still draft, auto-merge untouched — yours. Generated by Claude Code |
PM review of the fix (
|
|
跨席位通报 + 一条可选加固。⛔ 不是要求,这是你的 PR。 来自 通报:本 PR 与 #8209 的 pin 在合并队列树里曾语义冲突,而你已经修好了约一小时前,队列构建 31641477795 红在
实测确认:只应用本 PR 的 你已经吸收了那两处迁移( 可选加固(可以直接忽略)那个测试文件里有一条本地对照断言的是 若顺手,可以给它加一个显式的 一条我这边的教训,顺带记下我最初把 也顺带记一件本仓层面的事实,与谁对谁错无关:这类冲突 PR 侧 CI 结构上看不见 —— 两个 PR 各自全绿,冲突只在队列把它们堆叠起来时出现。这次队列是按设计在工作,不是不稳定。 Generated by Claude Code |
3 participants
Fixes #8111
Follows PR #8174 (#8073) exactly: one family-local emitter delegating to the shared
sendErrorfrom@objectstack/types, flat-shape pins migrated rather than deleted, abody.error.messageassertion on the bare-string arms, a derived cross-arm skeleton pin, and the SDK measurement re-run against these consumers.STEP 1 — the census decided the card
The dispatch made this conditional: the
msg.startsWith(CODE)protocol survives only if nothing reads it. Nothing reads it, so this proceeded as a plain convergence card. What was searched, not just concluded:startsWith('VALIDATION_FAILED/PERMISSION_DENIED/NOT_FOUND/CONFLICT/SHARING_NOT_ENABLED/startsWith(code)acrosspackages apps examples scriptsrest-server.ts(this file's own route mappings), 1 inplugin-approvals.ts,.tsx,.mjs,.md,.mdx) incl.content/plugin-sharing/sharing-service.ts; docs describe status+code pairs only/sharesroutesclient.shares.*; route ledger rowsdisposition: sdkObjectStackClient.fetch)errorBody?.code ?? errorBody?.error?.code; never parses message textapproval-service.ts:1126re-checks a prefix on an error it threw itself three lines above, in-process — not a wire consumer, not this familycontent/docs/**for prefix-parsing guidancecode+ statussharesreferences at allSHARES_LIST_FAILED/SHARE_GRANT_FAILED/SHARE_REVOKE_FAILEDThe structural reason behind the count: the emitter strips the prefix (
msg.replace(new RegExp('^CODE:\\s*'), '')) before writing the body, so it has never been on the wire and no consumer could read it. It is a server-internal service to REST derivation and is left exactly as it was —stripsThePrefixnow pins that, so a later reader neither restores a prefix the wire never carried nor assumes the message is the raw throw.STEP 2 — the conversion
All nine refusal arms of
registerSharingEndpointswent through one family-localrespondErrordelegating to the shared writer (imported assendEnvelopeError, since this module has a localsendErrorfor thrown errors):NOT_IMPLEMENTED(respond501){ code, message }— flat{ success: false, error: { code, message } }respondSharingError){ code, error: 'bare string' }{ code, error: 'bare string' }No status code moves. No
codeVALUE changes. The 500 arms keep their 500-char sanitization cap.The one finding:
CONFLICTwas registered nowhereThe 409 arm emits
code: 'CONFLICT', which was in neitherStandardErrorCodenorERROR_CODE_LEDGER— andErrorCode(whatApiErrorSchema.codeparses against) is exactly their union. So that body could never have parsed as the declared envelope, and routing it through the shared writer surfaced it as a compile error, which is the closed vocabulary working as designed. The arm is live:plugin-sharing/sharing-service.ts:1042throws it forrevokeon a rule-materialised share.Per the dispatch (report, do not invent or rename), the existing value is now registered under
@objectstack/rest— keeping the wire byte-identical — and the consolidation question (CONFLICTvs the catalog'sRESOURCE_CONFLICT, plus three sibling synonyms already in that block) is filed unassigned as #8211 for a maintainer ruling.STEP 4 — SDK re-measurement, run against these consumers
packages/client/src/shares-envelope-compat.test.tsdrives the realObjectStackClientagainst a stubbed transport answering the old and new body for each of 7 arms, throughshares.list/.grant/.revoke.code,message,httpStatus,category,retryable,fields— identical on every arm.detailschanges on every arm, the same surprise #8174 found: its last fallback is?? errorBody, the whole response body, and the body is what this card reshapes. Discharged by census — no in-repo consumer readserr.detailsoff ashares.*call (C11) — and pinned rather than left implicit.STEP 5 — reverse verification
Conversion reverted with tests at HEAD; predictions stated before running, then confirmed:
The derived skeleton pin is the one that fails structurally — it names the two dialects diverging rather than restating a literal.
Green under revert, and why (naming these is part of the check): the healthy-path case (no envelope involved),
rest.test.ts's 501 case (asserts status only), and the entire 9-case client SDK suite — it stubs the transport with literal bodies and never executes the server, so it is a compatibility measurement, not a server pin.check-route-envelopewent red as predicted:siblingCode: found 73, declared 69 — a NEW non-conforming body.Restored;git diff --stat HEADempty.Gates
pnpm --filter @objectstack/rest test— 105 files / 1756 tests passed;@objectstack/client— 23 files / 296 tests passed;@objectstack/spec— 385 files / 10195 tests passed (ledger admission suite green with the new entry).typecheckclean on both packages. ThreeTS2307reds in untouched client test files were the stale-artefact trap —@objectstack/runtimewas simply unbuilt; building the closure cleared them.check:type-check-debtOK, none above its recorded number.@objectstack/restunchanged at 155 and the ledger file untouched; measured after the full closure build the gate requires.check:route-envelopegreen incl.--self-test.siblingCodelowered 73 → 69, banking the progress. Verified mine: merge-base 73, head 69; the four vanished sites are merge-base lines 9480, 9507, 9541, 9570, all insideregisterSharingEndpoints, with zero head sites left in that range.stringErrorunmoved at 44 by construction — all four messages are computed (msg.replace(...),String(...).slice(0, 500)), which that counter cannot see, andrespond501was never counted by either dialect, having noerrorkey.node scripts/check-nul-bytes.mjsOK;check-error-code-casingOK.Changeset included (user-visible response bodies).
Generated by Claude Code