refactor!: ADR-0112 batch 2 — sweep the lowercase error-code emitters (#4003) - #4021
Merged
Merged
Conversation
…-0112 D9a) packages/spec exported two mutually incompatible ErrorCategory types and two RetryStrategy types — api/errors.zod.ts (HTTP error responses) and integration/connector.zod.ts (connector normalisation). Separate subpath exports kept TypeScript quiet, but the doc/schema generators key on the stripped name, so a cross-reference to 'ErrorCategory' resolved to whichever was scanned last. Rename the connector-side pair to ConnectorErrorCategory / ConnectorRetryStrategy and say in each docblock why the name is prefixed. Both are spec-internal (only connector.zod.ts and its test referenced them), so no downstream break. The json-schema ratchet caught the rename as an unpublish; the two manifest keys are renamed alongside, per its instructions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
…owercase code clusters (ADR-0112 batch 2) Converts 105 lowercase snake_case error codes to the declared vocabulary and registers the service-specific ones in ERROR_CODE_LEDGER. Generic conditions collapse onto the standard catalog rather than keeping a synonym: 'unauthorized'/'unauthenticated' -> UNAUTHENTICATED, 'forbidden' -> PERMISSION_DENIED, 'not_found' -> RESOURCE_NOT_FOUND, 'internal' -> INTERNAL_ERROR, 'unavailable' -> SERVICE_UNAVAILABLE, 'not_supported' -> NOT_IMPLEMENTED, and 'bad_request' -> INVALID_REQUEST (a status name is not a semantic code). Domain conditions get registered codes, prefixed where the bare name wouldn't carry meaning (MARKETPLACE_STORAGE_FAILED, PLUGIN_MANIFEST_INVALID, ENVIRONMENT_BIND_FAILED, RESEED_SKIPPED, ...). New ledger entries: @objectstack/cloud-connection and @objectstack/hono. Also closes an unbounded code source found while sweeping: the OAuth client registration route put better-auth's arbitrary `body.error` string straight into `error.code`, so any upstream string became a code and the closed set could not hold. The code is now ours (OAUTH_REGISTER_FAILED) and the upstream discriminator moves to `details.upstreamError`. Per-package consumer check (ADR-0112 batch-2 precondition): no reader outside either package branches on the swept spellings; the in-package test assertions move with the emitters. Refs #4003, #3841. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
…n-error codes; keep the audit column out (ADR-0112 batch 2)
Three clusters, all of which reach `error.code` on the wire — a thrown error's
`.code` is copied straight into the response body (rest-server.ts:268), so these
were never merely internal.
- metadata-protocol: 24 thrown codes (ITEM_LOCKED, NOT_OVERRIDABLE,
METADATA_CONFLICT, DESTRUCTIVE_CHANGE, ...) registered under the package, plus
the two `evaluateLockFor*` helpers in spec that feed them.
- service-automation resume refusals: 'forbidden' -> PERMISSION_DENIED,
'invalid_signal' -> INVALID_SIGNAL, together with the runtime branch that
reads them and the contract docstring that documents them.
Two decisions worth keeping:
`sys_metadata_audit.code` is deliberately NOT swept. It spells its denial
reasons identically to the codes the protocol throws, so a naive sweep renames
it too — but the column is persisted audit history (rows predating ADR-0112 keep
their spelling forever) and it also holds non-error outcomes ('ok',
'lock_override'). Following the catalog would buy a data migration or one column
with two vocabularies mixed by write date. It stays lowercase and self-contained;
the divergence is recorded at both sites and as ADR-0112 D6b, so it reads as a
decision rather than a missed rename.
The ADR's "no consumer branches on the lowercase dialect" claim was wrong: the
per-service re-verification it mandated found three, each renamed atomically
here. The ADR now records the correction plus a sweep checklist of the six
literal forms a `code: '...'` grep misses (comparisons, property assignment,
computed ternaries, regex assertions, toBe assertions, docstrings) — every one
of them cost a red suite in this batch.
Refs #4003, #3841.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
…-level codes and un-nest three flat envelopes (ADR-0112 batch 2)
- rest: OBJECT_NOT_FOUND, UNAUTHENTICATED (it sat in the same if/else as an
already-SCREAMING PERMISSION_DENIED), INVALID_REQUEST.
- service-messaging redeliver: RESOURCE_NOT_FOUND / DELIVERY_NOT_ELIGIBLE, with
the plugin-webhooks route that branches on them.
Three routes also had a bare code string in the `error:` slot with a real
`message:` beside it, so `error` was a code and the declared object envelope was
nowhere. They now emit `error: { code, message }` — a rename alone would have
left them unvalidated, which is how the lowercase dialect survived the first
place.
`mapDataError`'s downcasing is inverted, not just renamed. It deliberately
translated the internal SCREAMING OBJECT_NOT_FOUND down to `object_not_found` on
the way out, and a test pinned exactly that ("does not let the generic 4xx
passthrough ship the internal SCREAMING_CASE code") — correct when the data
routes were believed to speak their own dialect, backwards once there is one
vocabulary. The translation is gone and the test is rewritten to pin what still
matters: no per-route dialect translation, what the gate threw is what ships.
Two comments that still taught the lowercase wire code are updated with it.
Field-level codes in `import-coerce.ts` are deliberately untouched (ADR-0112 D6,
#3977).
Refs #4003, #3841.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 14 package(s): 118 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…velope Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
…lope; draw the catalog's boundary (ADR-0112 batch 2)
The API-trigger webhook endpoint returned `{ error: '<code>' }` — a bare code in
the message slot, six times, three of them with no message at all. It now answers
`{ success: false, error: { code, message } }` with catalog codes
(RESOURCE_NOT_FOUND, INVALID_REQUEST, SERVICE_UNAVAILABLE) plus two registered
ones (INVALID_SIGNATURE, ENQUEUE_FAILED). Every branch gains the message it was
missing.
Also records what batch 2 decided NOT to sweep, as ADR-0112 D6c, because three
lowercase clusters look like stragglers and are not:
- Diagnostics records (`build-probes.ts` RuntimeBuildIssue,
`metadata-diagnostics.ts`) ship as the payload of a 200, describe an artifact
rather than the request, and carry a severity that can be 'warning'.
- Param/field-addressed issues (`ActionParamIssue`, plugin-sharing's `fields[]`)
are D6 territory; plugin-sharing's own top-level code is already
VALIDATION_FAILED.
- CLI `emitJson({ error: '...' })` output is a terminal contract read by shell
scripts, not a request response — renaming it would break users' pipelines
with neither a schema nor this ADR asking for it.
The line, now written down once: the catalog governs the code a failing REQUEST
answers with. Field-addressed (D6), persisted (D6b), or shipped inside a
successful response (D6c) means a different vocabulary with different consumers.
Refs #4003, #3841.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
…name The resume result's `code` is a literal-union TYPE in the service contract, not just a value, and batch 2 renamed only the values plus the docstring beside it. Nothing in spec failed: `tsc --noEmit` on the declaring package is happy with a narrower union than its callers emit. The break surfaced two packages away — the dts build of every consumer of @objectstack/spec/integration, which is what took Build Core, Test Core and the dogfood shards red at once. Added to the ADR's sweep checklist as its own row, since it is the one form whose error appears in a package the sweep never touched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
…s of the automation and publish codes Four more consumers of codes batch 2 renamed, found by running the full suite rather than grepping harder: - plugin-approvals asserts the resume refusal code (its approval node IS the service-gated case), a package the sweep never touched. - http-dispatcher.test mocks the automation service's result, so its two literals are the emitter's contract restated in a mock. - batch_aborted -> BATCH_ABORTED: it rides in the same failed[] array as the causal item's catalog code, so leaving it lowercase made one array carry two vocabularies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
…name Sixth consumer found, and the one my local verification could not have caught: the full-suite run I used as the safety net excludes @objectstack/dogfood by design, so the dogfood shards were the one gate I had not actually run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
13 tasks
The ledger's admission test enforces casing on every code someone REGISTERS.
Nothing enforced the ones nobody registers — and an unregistered lowercase
literal in a code position is invisible twice: to the ledger, which has nothing
to check, and to the schema, on any route that never parses its own response.
That is how 208 lowercase literals accumulated across 10 packages. Without a
guard, number 209 lands the same way.
`scripts/check-error-code-casing.mjs` scans packages/ for four code positions —
emission, property assignment, comparison, and literal-union type — which are
the forms batch 2 actually got bitten by, comparison and union-type being the two
that fail silently or two packages away.
The design problem is that `code` is also an ordinary domain field name: a
license plan code, a seed industry code, a locale. A guard that flags those gets
switched off within a week, so a hit only counts when an error-shaped neighbour
(error/message/throw/status/issues/...) appears within a few lines, and the
D6 families are recognised structurally — a `field`/`param`/`path`/`target` key
in the same record, checked over a window because `{ code, field, message }` is
routinely spread across three lines.
Three escape hatches, in descending preference:
- structural: the D6 shapes above, no annotation needed;
- `// adr0112-ok: <reason>` at the site, for a file that legitimately holds
both vocabularies (protocol.ts throws a catalog code and writes an audit row
beside it) — the reason is required, a bare marker does not count;
- EXEMPT_FILES, for a file that OWNS a non-catalog vocabulary (a diagnostics
producer, OData's foreign error vocabulary), each line carrying its reason.
`--self-test` runs 17 cases (each violating form, each exemption route, each
known false-positive shape) before the scan, so a green run means the matcher
was checked, not just quiet. End-to-end verified by reintroducing
`code: 'object_not_found'` in rest-server.ts: the guard fails, and passes again
when reverted.
Two real fixes fell out of turning it on: the `'locked'` fixtures in two
publish tests now spell what the protocol actually throws (ITEM_LOCKED), and the
audit-row sites carry their D6b marker so the divergence reads as a decision.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
github-actions
Bot
added
ci/cd
dependencies
…the new guard Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK
os-zhuang
marked this pull request as ready for review
July 30, 2026 04:29
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
2 participants
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.
Batch 2 of ADR-0112's rollout, tracked in #4003. Batch 1 was #3988. Console side: objectui#2977 (merged).
What
Converts the lowercase
snake_caseemitters to the declared vocabulary — the standard catalog where the condition is generic, a registered ledger code where it is domain-specific — and adds four packages toERROR_CODE_LEDGER(cloud-connection,hono,service-messaging,trigger-api) plus entries underrest,plugin-auth,metadata-protocol,service-automation.cloud-connectionplugin-auth(+hono)metadata-protocolthrown codes (+ thespeclock evaluators feeding them)resttop-level +error:-slot codestrigger-api(converted to the declared envelope)service-messagingredeliver (+ theplugin-webhooksroute)service-automationresume refusals (+runtimebranch,specdocstring and contract type)Generic conditions collapse onto the standard catalog rather than keeping a synonym:
unauthorized/unauthenticated→UNAUTHENTICATED,forbidden→PERMISSION_DENIED,not_found→RESOURCE_NOT_FOUND,internal→INTERNAL_ERROR,unavailable→SERVICE_UNAVAILABLE,not_supported→NOT_IMPLEMENTED,bad_request→INVALID_REQUEST(a status name is not a semantic code).Four routes also had a bare code string in the
error:message slot, soerrorwas a code and the declared object envelope was nowhere. They now emiterror: { code, message }, and threetrigger-apibranches gained the message they never had. Renaming alone would have left them unvalidated — which is how the lowercase dialect survived in the first place.Also included: D9a from the ADR's ride-along list —
specexported two mutually incompatibleErrorCategorytypes and twoRetryStrategytypes; the connector-side pair becomesConnectorErrorCategory/ConnectorRetryStrategy. Side effect worth noting: the api-sideErrorCategoryandRetryStrategynow appear in the generated API reference at all — the name collision had been silently dropping them.New: a guard so this cannot regress
The ledger's admission test enforces casing on every code someone registers. Nothing enforced the ones nobody registers, and an unregistered lowercase literal is invisible twice over — to the ledger, which has nothing to check, and to the schema, on any route that never parses its own response. That is how 208 accumulated across 10 packages.
scripts/check-error-code-casing.mjs(wired into lint.yml) scans for the four positions batch 2 actually got bitten by: emission, property assignment, comparison, and literal-union type. The design constraint is thatcodeis also an ordinary domain field name — a license plan code, a seed industry code, a locale — and a guard that flags those gets switched off within a week. So a hit counts only with an error-shaped neighbour nearby, and the D6 families are recognised structurally (afield/param/path/targetkey in the same record, over a window, because{ code, field, message }is routinely spread over three lines). Escape hatches, in descending preference: structural, then// adr0112-ok: <reason>at the site (a bare marker without a reason does not count), thenEXEMPT_FILESfor a file that owns a non-catalog vocabulary.--self-testruns 17 cases — every violating form, every exemption route, every known false-positive shape — before the scan, so a green run means the matcher was checked rather than merely quiet. Verified end-to-end by reintroducingcode: 'object_not_found'intorest-server.ts: the guard fails, and passes again on revert.Five things worth a reviewer's attention
1. The ADR's "no consumer branches on the lowercase dialect" was wrong — six times. Each was renamed atomically with its emitter, and each was found by a different method, which is the actual lesson:
runtime/domains/automation.ts(forbidden,invalid_signal),metadata-protocol/protocol.ts:4353(destructive_change), thespeccontract docstringcode === '…'spec/contracts/automation-service.tsliteral-union typedtsbuild two packages away —tsc --noEmiton the declaring package passedplugin-approvalstest (its approval node is the service-gated case)qa/dogfoodpackage-first authoringThe ADR now carries the correction plus a sweep checklist of the seven forms a
code: '…'grep misses. Two deserve repeating: a missed comparison fails silently rather than loudly, and a missed literal-union type surfaces in a package the sweep never touched — that one took Build Core, Test Core and both dogfood shards red simultaneously, reading like four unrelated failures.2. Three vocabularies are deliberately NOT swept, now recorded as decisions.
sys_metadata_audit.codespells its denials exactly like the thrown codes, so a naive sweep renames it too. But it is audit history — rows written before this ADR keep their spelling forever — and the same column holds outcomes that are not errors (ok,lock_override). Following the catalog buys a data migration or one column with two vocabularies mixed by write date. Recorded at both sites, so it reads as a decision rather than a missed rename.build-probes.ts/metadata-diagnostics.tsrecords ship as payload of a 200, describe an artifact rather than the request, and carry a severity that can bewarning.emitJson({ error: '…' })is a terminal output contract read by shell scripts, not a request response; renaming it would break users' pipelines with neither a schema nor this ADR asking for it.The line, written down once: the catalog governs the code a failing request answers with. Field-addressed (D6), persisted (D6b), or inside a successful response (D6c) is a different vocabulary with different consumers.
3. An unbounded code source is closed.
plugin-auth's OAuth client registration put better-auth's arbitrarybody.errorstring straight intoerror.code. A closed set cannot hold with an unbounded source feeding it; the code is ours now (OAUTH_REGISTER_FAILED) and the upstream discriminator moves todetails.upstreamError.4.
mapDataError's translation is inverted, not renamed. It deliberately downcased the internalOBJECT_NOT_FOUNDtoobject_not_foundon the way out, and a test pinned exactly that ("does not let the generic 4xx passthrough ship the internal SCREAMING_CASE code") — correct when the data routes were believed to speak their own dialect, backwards once there is one vocabulary. The translation is gone; the test is rewritten to pin what still matters (no per-route dialect translation) rather than deleted.5. objectui was actively broken, not merely dead-branched (fixed in objectui#2977, merged first on purpose). Eleven
===comparisons on the old spelling: the destructive-change confirm dialog, the writable-package hint, field-scoped validation issues, the all-or-nothing publish summary, unknown-object tolerance in the header and record search, three marketplace install messages, andisNotFoundErrorin the data layer.RECORD_NOT_FOUNDhad been renamed in batch 1, so that branch was already dead a release before anyone noticed. Fixed with a case-insensitive read, since the console ships separately from its server and both spellings are live peers until old servers are unsupported.Out of scope (deliberately)
Field-level codes (
{ field, code }inimport-coerce.ts,record-validator.ts,rule-validator.ts,action-params.zod.ts) stay as they are — ADR-0112 D6 puts them under #3977.Test evidence
pnpm turbo run test(the same command CI runs): 129/129 package tasks green.turbo run buildacross all 57 packages: green.check:error-code-casing(2398 files, 0 violations).service-automationcarries 6 failures innotify-node/screen-nodes/crud-config-aliases/enginethat reproduce with these changes stashed — pre-existing, not from this PR.Refs #3841, #3988, objectui#2977. #4003 can close with this.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MaSQn77TT5fUgHK9CesaDK