Skip to content

The /data door ships the ADR-0111 CODE: prefix inside the user-facing error string, so a localized refusal renders as FORBIDDEN: … in a toast #12975

Closed

Description

@os-litant

Measured on origin/main@f4e741bd1 (re-confirmed on ae4cfcce7). Filed unassigned from the #12260 dev seat; not a regression from #12260 — see the last section.

The measurement

classifyDataError's declared-status 4xx arm ships the producer's message verbatim:

packages/rest/src/error-response.ts:1019-1033

const msg = typeof error?.message === 'string' && error.message.length > 0
    ? truncateClientMessage(error.message)
    : 'Request failed';
return { status: declaredStatus, body: { error: msg, ...thrownCodeFields(error, declaredStatus), ... } };

The sharing middleware's by-id write refusal declares { code: 'FORBIDDEN', status: 403 }, so it takes exactly this arm. Its thrown message is the ADR-0111 CODE: message idiom, and the whole thing — machine prefix included — becomes the flat body's human-readable error, which Console and the reporting deployment's own client render verbatim.

Pinned byte-for-byte by tests that assert the prefix survives:

  • packages/rest/src/rest-4xx-message-truncation.test.ts:123 — expect(r.body.error).toBe(msg) where msg starts FORBIDDEN:
  • packages/rest/src/rest-5xx-status-passthrough.test.ts:301 — expect(r.body).toEqual({ error: msg, code: 'FORBIDDEN', object: 'showcase_inquiry' })

The two doors disagree, and one of them says so in prose

The share-route family and handleApprovalError both strip it:

  • packages/rest/src/rest-server.ts:10867 — res.status(status).json({ code, error: msg.replace(/^[A-Z_]+:\s*/, '') })
  • respondSharingError's ADR-0111 prefix arm, same file

And rest-server.ts's own #8111 comment above that arm states:

The prefix is a SERVER-INTERNAL service→REST derivation: it is stripped below and never reaches the wire, so no consumer can read it

That is true for that door and not true for /data. The same refusal therefore reaches a client with the prefix on one route and without it on another — the door-disagreement shape #7525 / #8016 / #11588 keep producing.

Why it is worth a card now rather than before

#12260 converted the sharing write denial's sentence onto the Operation Message Catalog, so what a zh-CN user now reads on /data is:

FORBIDDEN: 您无权修改或删除这条记录,如需修改请联系该记录的负责人或管理员。

The actionable half is localized human language; the machine token glued in front of it is now the only non-human fragment left in the user's face. Before that conversion the whole sentence was English, so the prefix was not the salient part.

Not a regression, and deliberately out of #12260's scope

The prefix rode in front of the old English sentence identically — nothing about its handling moved. It is left alone there on purpose: the fence on that card was that the prefix is wire contract and must not be shadowed, and changing which bytes /data ships is a REST envelope decision, not a plugin one. Any fix moves the two pinned assertions above, which are legitimate wire-behaviour tests today.

Options, not a recommendation

  1. Strip the CODE: prefix in the declared-4xx arm, matching the share door. Changes the flat 4xx body for every producer using the idiom, and moves both pinned tests.
  2. Leave error alone and have the producers mark userMessage (the rest: respondSharingError's nested re-dress also drops userMessage and the producer's structured context that /data carries for the same throw #12669 channel withDeclaredUserMessage already puts on the flat body top level at error-response.ts:569-571). Strictly additive on the wire; needs each emitter to opt in.
  3. Rule that the prefix in error is intended on /data and correct the #8111 comment, which currently asserts the opposite.

Option 2 looks cheapest and least contract-moving, but which door is right is an envelope decision for the maintainer rather than something a dev seat should pick.

Activity

  1. added theissue type on Aug 28, 2026
  2. self-assigned this
    on Aug 29, 2026
  3. os-litant commented on Aug 29, 2026

    @os-litant
    ContributorAuthor

    🔒 Claimed by os-dev session session_01UjujZN219uFzBhSYfMykCd, branch claude/issue-12975-data-door-strips-code-prefix (dispatched from the domain:cli seat). Executing the 2026-08-29 maintainer ruling (option 1) — no re-litigation.

    Generated by Claude Code


    Generated by Claude Code

  4. os-litant commented on Aug 29, 2026

    @os-litant
    ContributorAuthor

    os-dev-report

    {
      "issue": 12975,
      "status": "done",
      "branch": "claude/issue-12975-data-door-strips-code-prefix",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/13124",
      "premise_still_valid": true,
      "summary": "Executed the 2026-08-29 ruling (option 1) exactly: classifyDataError's declared-4xx arm now strips the ADR-0111 CODE: prefix from `error`, via a new `withoutDeclaredCodePrefix` helper anchored to the PRODUCER's own declared `code` rather than to a SCREAMING_SNAKE pattern — so the strip can only remove a duplicate of a token already on the wire (a 4xx with no `code` keeps its prefix; a prefix naming some other word is untouched; a message that is only the prefix degrades to 'Request failed'). Re-located coordinates: the arm moved from the issue's error-response.ts:1019-1033 to 1064-1096 on db39dfc1c; the share-door strip the issue cited as rest-server.ts:10867 is now 10840 (handleApprovalError), and respondSharingError's prefix arm is at 9916-9928. ⚠️ ONE MEASURED FACT THE ISSUE AND THE RULING DID NOT COVER, reported for PM/maintainer rather than fixed: error-response.ts has TWO declared-4xx arms, and the ruling named one. resolveErrorResponse's own passthrough — reached by /data's bulk exits and, through classifiedRefusalAnswer, by the record-share family's CLASSIFIED arm — still ships the prefix after this change. Converging it would move pins beyond the two the ruling authorised, so it was not taken; filed unassigned as #13095 with measurements and three options, and pinned in-tree by a case titled 'MEASURED, NOT REPAIRED HERE' so the follow-up moves it deliberately.",
      "consumer_census": {
        "verdict": "nothing parses meaning out of the prefix",
        "method": "grep -r (never git grep) over two corpora: objectstack @ db39dfc1c and the objectui checkout in this container @ 190fbd01d",
        "wire_readers_found": [
          "objectui packages/react/src/utils/error-message.ts:74 — display-side strip, becomes a no-op",
          "objectui packages/plugin-detail/src/InlineEditSaveBar.tsx:72 — display-side strip ('Strip noisy backend prefixes so the inline error reads cleanly')",
          "objectui packages/plugin-detail/src/DetailView.tsx:663 — display-side strip"
        ],
        "in_process_readers_untouched": [
          "packages/rest/src/rest-server.ts route mappings (8348, 10089-10129, 10519-10526, 10840, 9916-9928) — read the thrown message upstream of the wire",
          "packages/plugins/plugin-email/src/email-service.ts:1221 — checks an error it threw itself in-process",
          "plugin-sharing / plugin-approvals tests that MODEL the REST strip; they operate on the thrown message, not the wire body"
        ],
        "positive_controls_for_every_zero_reading": [
          "objectstack, split-on-colon-over-an-error: 0 relevant (1 hit is a CLI test parsing its own exit-code shim) — CONTROL 'mapDataError' = 574 hits, same globs and filters",
          "objectstack, indexOf-colon-over-an-error: 0 — CONTROL 'thrownCodeFields' = 30 hits, same corpus and filters",
          "objectui, split-on-colon-over-an-error: 0 — CONTROL 'cleanError' = 4 hits, same globs and filters",
          "control terms were chosen so none is a substring of the term under test"
        ],
        "documentation_checked": "skills/objectstack-data/references/data-hooks.md:498 shows the prefixed sentence, but it documents what a HOOK BODY sees in-process, not the wire body — it stays accurate, so no skills/** file is touched and no skills line/token budget applies",
        "boundary": "This scan sees only the two repositories checked out in this container. It CANNOT see published consumers — third-party apps, the reporting deployment's own client, or the `cloud` repo, which is not present here. The ruling assessed that risk as low because the sibling `code` field has long been on the wire; this change does not move `code`."
      },
      "pin_moves": [
        {
          "file": "packages/rest/src/rest-4xx-message-truncation.test.ts",
          "case": "a normal-length message passes through with no ellipsis and no slicing",
          "from": "expect(r.body.error).toBe(msg) — msg = 'FORBIDDEN: insufficient privileges to update showcase_inquiry rec1'",
          "to": "expect(r.body.error).toBe('insufficient privileges to update showcase_inquiry rec1') PLUS expect(r.body.code).toBe('FORBIDDEN') PLUS an explicit no-ellipsis assertion",
          "why": "the case's subject is the #5423 BOUND (no ellipsis, no slicing) and that is unchanged; its fixture merely happens to use the ADR-0111 idiom. The added `code` assertion exists because asserting `error` alone would also pass for the other way of getting this wrong — dropping the token together with the prefix."
        },
        {
          "file": "packages/rest/src/rest-5xx-status-passthrough.test.ts",
          "case": "§4, retitled 'a short 4xx is still byte-for-byte verbatim, with its object' -> 'a short 4xx keeps its authored sentence — nothing withheld — with its object'",
          "from": "expect(r.body).toEqual({ error: msg, code: 'FORBIDDEN', object: 'showcase_inquiry' }) with the prefixed msg",
          "to": "the same whole-body equality with error: 'insufficient privileges to update showcase_inquiry rec1', plus expect(r.body.error).not.toBe(INTERNAL_ERROR_MESSAGE)",
          "why": "§4's subject is that the 5xx WITHHOLD does not reach the 4xx half, unchanged. The title dropped 'byte-for-byte verbatim' because the AUTHORED half is verbatim while the restatement of `code` in front of it was never authored for the caller. Whole-body equality kept so losing the token would red."
        }
      ],
      "measured_bodies_before_after": {
        "method": "real RestServer booted, real routes registered, registered handler called — no mock envelope assertions",
        "CHANGED": [
          "PATCH /data/:object/:id  BEFORE {\"error\":\"FORBIDDEN: 您无权修改或删除这条记录,如需修改请联系该记录的负责人或管理员。\",\"code\":\"FORBIDDEN\",\"object\":\"showcase_inquiry\"}  AFTER {\"error\":\"您无权修改或删除这条记录,如需修改请联系该记录的负责人或管理员。\",\"code\":\"FORBIDDEN\",\"object\":\"showcase_inquiry\"}",
          "DELETE /data/:object/:id — same before/after as PATCH",
          "unregistered code  BEFORE {\"error\":\"RECORD_LOCKED_BY_APP: the row is checked out\",\"code\":\"VALIDATION_ERROR\",\"declaredCode\":\"RECORD_LOCKED_BY_APP\",...}  AFTER {\"error\":\"the row is checked out\",...same code/declaredCode}",
          "message is exactly 'FORBIDDEN:'  BEFORE {\"error\":\"FORBIDDEN:\",\"code\":\"FORBIDDEN\",...}  AFTER {\"error\":\"Request failed\",\"code\":\"FORBIDDEN\",...}",
          "declared 4xx + userMessage mark — prefix gone, mark unchanged"
        ],
        "BYTE_IDENTICAL": [
          "declared 4xx, no prefix in message: {\"error\":\"insufficient privileges\",\"code\":\"FORBIDDEN\",\"object\":\"showcase_inquiry\"}",
          "declared 4xx with NO code + prefixed message: {\"error\":\"FORBIDDEN: 您无权…\",\"object\":\"showcase_inquiry\"} — token preserved, this is the control against 'dropping the token with the prefix'",
          "declared 4xx whose prefix does not name the declared code: {\"error\":\"SQLITE_ERROR: no such table: x\",\"code\":\"FORBIDDEN\",...}",
          "declared 5xx: {\"error\":\"Internal server error\",\"code\":\"SERVICE_UNAVAILABLE\"}",
          "undeclared: {\"error\":\"Internal server error\",\"code\":\"INTERNAL_ERROR\"}",
          "sandbox unwrap: {\"error\":\"Opportunity is closed.\",\"object\":\"showcase_inquiry\"}",
          "DELETE_RESTRICTED: {\"error\":\"DELETE_RESTRICTED: dependents exist\",\"code\":\"DELETE_RESTRICTED\",\"dependentCount\":3,...}",
          "OBJECT_NOT_FOUND: {\"error\":\"Object 'zz' is not registered\",\"code\":\"OBJECT_NOT_FOUND\",\"object\":\"zz\"}",
          "POST /data/:object/batch (the OTHER door): {\"error\":\"FORBIDDEN: 您无权…\",\"code\":\"FORBIDDEN\"} — STILL PREFIXED",
          "record-share GET and DELETE shares, CLASSIFIED arm: {\"success\":false,\"error\":{\"code\":\"FORBIDDEN\",\"message\":\"FORBIDDEN: 您无权…\"}} — STILL PREFIXED",
          "record-share GET shares, ADR-0111 prefix-idiom arm: {\"success\":false,\"error\":{\"code\":\"NOT_FOUND\",\"message\":\"record showcase_inquiry/rec1 does not exist\"}} — already stripped, unchanged"
        ]
      },
      "convergence_and_8111": "The `/data` by-id door now agrees with the share family's ADR-0111 prefix-idiom arm and with handleApprovalError: `error` is human language, `code` carries the token. The #8111 'never reaches the wire' claim was VERIFIED by driving the doors, not assumed: TRUE for the producers it was written about (sharing-service.ts's bare-Error prefix throws, which take the prefix-idiom arm and are stripped) and TRUE now for the card's producer on /data — but NOT true for the record-share family's newer CLASSIFIED limb nor for /data's bulk exits. The comment is left byte-identical as the ruling required.",
      "tests": "Union re-run AFTER the final commit, at `git rev-parse --short HEAD` = 714d2e7 (merge of origin/main@431d2fb42), working tree clean. (1) `pnpm --filter @objectstack/rest test` -> 'Test Files 155 passed (155) / Tests 2560 passed (2560)', exit captured as REST_TEST_EXIT=0 before any pipe. (2) `pnpm --filter @objectstack/rest typecheck` -> TYPECHECK_EXIT=0, and its own verdict line reads 'check:test-typecheck: OK — @objectstack/rest's test layer compiles under packages/rest/tsconfig.test.json', so the new and edited TEST files are genuinely covered (not the excluded-tests NOT-MEASURED shape). (3) Dependency closure built first: `pnpm --workspace-concurrency=2 --filter '@objectstack/rest^...' build` -> BUILD_DEPS_EXIT=0. (4) ABLATION, fix committed first: `error-response.ts` ALONE reverted with `git checkout db39dfc1c -- \"$REPO_ROOT/packages/rest/src/error-response.ts\"` under `trap ... EXIT INT TERM` with an ABSOLUTE path. Mutation proven ON DISK before reading any verdict: `git hash-object` returned bb8bde347f83f7522a88ef72d30c087ece43164a, equal to the branch point's blob, and `grep -c withoutDeclaredCodePrefix` on the mutated file returned 0. NO REBUILD between legs, deliberately and stated: every symbol under test is reached by a RELATIVE import inside this package, which vitest transforms from source, so no dist/ sits between mutation and assertion; the exports-resolved workspace deps are untouched by the mutation. Result on the reverted tree: 9 red / 48 green — §1 4 red (predicted 4), §3 1 red (predicted 1), §4 1 red (predicted 1), §2 all 8 GREEN (predicted green; these are the real controls — a pattern-based strip would red on the no-code and non-matching-prefix cases), and the two moved pins reddened ONE case each while EVERY OTHER case in both files stayed green (the check that only the two authorised pins moved and nothing was loosened to make room). TWO PREDICTIONS OF MINE WERE WRONG and are recorded rather than re-fitted: (a) I predicted §5 all-green but its CONVERGENCE case is RED under the revert — it asserts BOTH doors, so half of it reads the /data side the revert removes, and red is the correct answer for it; (b) I predicted the pre-fix arm text would grep once and it greps TWICE, because the same expression also lives in resolveErrorResponse's 4xx arm — which is exactly the finding filed as #13095. RESTORE PROVEN, not assumed: `git checkout HEAD -- <abs path>` (never a bare checkout), blob hash back to 25ed097978f89e76dac64c1564e2aa2bd565f111 = HEAD's, `git diff HEAD` empty for that path, and the restored tree re-ran GREEN. (5) GATES: derived at the merged head with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (no paths passed — the script read its own change set; no STALE TREE warning after merging origin/main; change set = exactly the 5 files). All 28 derived families run plus check:nul-bytes. 26 GREEN, including the three named by the dispatch: check:route-envelope, check:dispatcher-error-vocabulary, check:nul-bytes. Every exit code captured with a redirect-then-capture pattern, never after a pipe.",
      "not_measured": [
        "check:dual-build-cjs-loads — exit 3, its own line: 'PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/'. Its self-test passed (53 cases). Needs a whole-repo `pnpm build`, which is CI's run; this diff changes no package's build shape. NOT a red gate.",
        "scripts/pm/check-half-states.mjs — exit 3, its own line: 'PREREQUISITE NOT MET — the token in the environment is not a valid GitHub credential ... Nothing was swept ... it is no reading at all'. Independently corroborated: repo-scoped REST from this seat answers 403 'GitHub access is not enabled for this session' while GET /rate_limit reports a live 15000/hour core quota — the documented os-dev session-gate shape, not a repo problem. NOT a red gate.",
        "scripts/check-test-completeness.mjs — exit 1 on a USAGE error (it requires a turbo-test-log argument). A CI-shaped invocation, not a measurement in either direction.",
        "CI convergence itself — per the standing contract the report is delivered at draft-PR time; the CI job conclusions are PM's read, not mine. No CI polling was done."
      ],
      "seat_channel_note": "REST is closed to this os-dev seat (repo-scoped paths 403 'GitHub access is not enabled for this session'; `gh` absent; /rate_limit shows core 15000/hour, so the credential is alive and repo-scoping is what refuses). Per the rest-channel reference's documented fallback for such a seat, the out-of-scope dedup search used exactly ONE targeted MCP search_issues call plus local grep — no wide table scan. The issue and PR writes went through MCP for the same reason.",
      "open_questions": [
        {
          "question": "The ruling's stated goal is convergence on ONE envelope semantics, but error-response.ts has TWO declared-4xx arms and the ruling named only classifyDataError's. After this change, `resolveErrorResponse`'s passthrough still ships the prefix — measured on /data's bulk exits and on the record-share family's CLASSIFIED arm. Should that second arm converge too?",
          "options": [
            "A — apply the same declared-code-anchored strip in resolveErrorResponse's 4xx arm: converges every /data exit and, because the share family re-dresses that same classification, the record-share classified arm with it. Costs pin moves beyond the two the ruling authorised.",
            "B — leave it: rule that the by-id door is the only one converging, and correct the #8111 comment to name the limb it does not cover.",
            "C — converge the record-share family only, leaving /data's bulk exits as they are."
          ],
          "recommendation": "A on the merits — it is the only option that delivers the ruling's own words ('one envelope semantics') and it closes the door-disagreement class rather than relocating it. But I deliberately did NOT take it in this PR: the ruling authorised exactly two pin moves, and A moves more, which is the line between pin maintenance and silent alteration. Filed as #13095 for the maintainer to rule; the change itself is a few lines and one shared helper call once ruled."
        }
      ],
      "out_of_scope_findings": [
        "filed as #13095: `resolveErrorResponse`'s declared-4xx passthrough still ships the ADR-0111 CODE: prefix — measured on POST /data/:object/batch and on the record-share family's classified arm (GET and DELETE shares), unassigned, no labels, with the before/after bodies and three options"
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions