Skip to content

fix(objectql): a plugin driver's request-shaped read refusal answers 400 INVALID_QUERY with its message, not 503 retry-later - #27648

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-8wrj9h
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-8wrj9h

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by 田騏 · project thread
Fixes #27471

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么: 插件 driver 的外部数据源上,find 抛出带请求型状态(status/statusCode 为 400、404、422)的错误时,调用方现在拿到 400 INVALID_QUERY,消息就是 driver 自己的原话;其余失败(5xx、连不上、没声明状态、401/403/407/429)照旧是 503 ERR_DATASOURCE_UNAVAILABLE、原文不外泄。指南里的 refuse() 现在带 status: 400,失败段落逐条写明调用方看到什么。
  • 为什么改: 之前读路径的任何抛错都变成 503「远端故障,稍后重试」,作者照指南写的「点名拒绝」被说成可以重试,而重试永远不会好,真正原因只在服务器日志里。
  • 风险与回滚: 低。只动外部插件 driver 兜底层的读路径一个 catch 分支,内置 driver 和受管数据源不受影响;没有新错误码(复用标准 INVALID_QUERY)。回滚即还原本 PR。
  • 先看: packages/objectql/src/external-driver-floor.ts(readCapped 的 catch 分支)、packages/objectql/src/external-driver-errors.ts(ExternalReadRefusedError 与状态集合)、content/docs/data-modeling/virtual-objects.mdx(refuse() 与失败段落)。
  • 两分钟验证: pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/external-driver-floor.test.ts,预期 45/45 通过,其中 [#27471] 一组 14 条。
  • 需要你做的: 无,席位落地。

What changes

  • packages/objectql/src/external-driver-errors.ts: EXTERNAL_READ_REFUSED_STATUSES (400, 404, 422) and ExternalReadRefusedError, which carries the standard INVALID_QUERY code, status/httpStatus 400, the driver's message, and details (datasource, object, operation: 'find', remoteStatus). It has the same shape as ExternalWriteRefusedError. No new ledger code: INVALID_QUERY is a StandardErrorCode member, and REST already treats it as an expected query rejection (no unhandled-error log line).
  • packages/objectql/src/external-driver-floor.ts: the floor's find catch reads the declared status the same way the write path does (declaredStatus: status, then statusCode). A status in the read set throws the 400 refusal, with the original error as cause. Everything else still goes through remoteFailure, so it stays 503, reason remote, with the text logged at warn and withheld from the caller. findOne and count read through the same readCapped, so they get the same answer. The refusal leaves the engine through the existing boundary (redactPropagatedDriverFault in the middleware seam), which cuts bound statement text the same way it does for any driver error.
  • packages/objectql/src/index.ts: exports ExternalReadRefusedError and EXTERNAL_READ_REFUSED_STATUSES next to the write-side exports.
  • content/docs/data-modeling/virtual-objects.mdx: the guide's refuse() now throws with status: 400, which is the status the engine reads. The "Fail when the remote fails" bullet now lists, per status, what the caller sees: a refused read answers 400 INVALID_QUERY, a refused write answers 422 EXTERNAL_WRITE_REFUSED, and everything else answers 503. The guide's own driver test also pins status: 400.
  • .changeset/27471-external-read-refusal-400.md: @objectstack/objectql patch.

Tests

packages/objectql/src/external-driver-floor.test.ts has a new [#27471] block with 14 cases, run through the real engine:

  • The guide-shaped refuse() gets ExternalReadRefusedError / INVALID_QUERY / 400, with the driver's sentence and details, on find, findOne and count.
  • 400, 404 and 422, spelled as status and as statusCode, each answer 400 with the driver's message and remoteStatus.
  • 401, 403, 407, 429, 500, 502, 503, 409 and 412 each stay ERR_DATASOURCE_UNAVAILABLE, kind: 'remote', with the text withheld. 409 and 412 are write-only refusal statuses and stay 503 on a read.
  • A throw with no status stays 503 with the text withheld.

Results at head 9773776:

  • pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/external-driver-floor.test.ts: 45/45 passed.
  • pnpm --filter @objectstack/objectql test (under the verify lock): Test Files 487/487, Tests 9163/9163, VERDICT command-exit 0.
  • pnpm --filter @objectstack/objectql typecheck: exit 0, test layer held in test-typecheck-debt.json.
Ablation (one-shot, not kept)

node scripts/ablation-replace.mjs --file packages/objectql/src/external-driver-floor.ts --anchor "if (status !== undefined && EXTERNAL_READ_REFUSED_STATUSES.has(status)) {" --replacement "if (false && status !== undefined && EXTERNAL_READ_REFUSED_STATUSES.has(status)) {"

  • The mutation landed: anchor 1 to 0, blob 1957bd34d07c to 993bc75ea06b.
  • Result: 4 failed, 41 passed. The 4 failures were the refuse() case and the 400, 404 and 422 cases.
  • The 503-band cases stayed green, because they pin behaviour that did not change.
  • Restored: blob equals HEAD (1957bd34d07c), and git diff HEAD is empty.
  • The test imports ./engine from source, so no dist rebuild was involved.

Acceptance notes

  • The code is reused, not new. The card left the choice of an existing 400 code to the dev. I picked INVALID_QUERY over INVALID_FILTER because the guide's refuse() also refuses an unmapped object and unsupported operations, not only filter terms.
  • The triage wording differs from the ruling. Triage 6491225200 says "a typed refusal keeps its code and message". This PR follows the card's ruled Task instead: the floor reads the declared status, and a driver's own code on a 400/404/422 throw is replaced by INVALID_QUERY. The driver's object is kept as cause.
  • The guide's write stubs change answer too. They call notSupported, which now carries status: 400 through refuse(). So a refused write on that skeleton answers 422 EXTERNAL_WRITE_REFUSED with the driver's sentence, where it used to answer 503. The paragraph states this.

Verification

Gates were derived with node scripts/pm/dispatch-gates.mjs --commands (no paths) and reconciled with --ran; the verdict at head 9773776: "91 derived famil(ies) accounted for — 91 run, 0 NOT-MEASURED (a DERIVED zero)". All 91 exited 0. Four first exited 3 or timed out on prerequisites (a shallow clone missing the teardown fixture commit, dist/ not yet built, and the 400 s per-gate timeout on check:type-check-debt); each was re-run after its prerequisite and exited 0.

CI check-runs on this PR carry every family CI runs; they are cited in the os-dev report, not re-run locally.


Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_017ihL2fT4drj3TCsV9G9CPh

…ers 400 with its message

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2fT4drj3TCsV9G9CPh
… failure paragraph names each answer

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2fT4drj3TCsV9G9CPh
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 5 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/objectql/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/api/error-catalog.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/api/error-handling-client.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/api/error-handling-server.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/api/index.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/api/wire-format.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/automation/flows.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/deployment/app-dependencies.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/deployment/cli.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/plugins/development.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/protocol/kernel/error-handling.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/protocol/kernel/http-protocol.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-0.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))
  • content/docs/releases/v17/17-3.mdx (via httpStatus (symbol, a field of class ExternalReadRefusedError))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/objectql/src/index.ts) — pages documenting those are invisible to this run
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 58 of 210 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 19 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 9d5ebfdef05caf03b5b0f2013873f1f704c8145b → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 19a68416bc60b8963d895d1aaad9ee98eab872d7 — the merge of head e9672b771a25cc357584c0ae6e4373e7646e22ee into base 9d5ebfdef05caf03b5b0f2013873f1f704c8145b, 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 19a68416bc60b8963d895d1aaad9ee98eab872d7 && git checkout 19a68416bc60b8963d895d1aaad9ee98eab872d7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9d5ebfdef05caf03b5b0f2013873f1f704c8145b e9672b771a25cc357584c0ae6e4373e7646e22ee && git checkout -B drift-repro 9d5ebfdef05caf03b5b0f2013873f1f704c8145b && git merge --no-ff e9672b771a25cc357584c0ae6e4373e7646e22ee

node scripts/docs-audit/affected-docs.mjs --json 9d5ebfdef05caf03b5b0f2013873f1f704c8145b

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 9d5ebfdef05caf03b5b0f2013873f1f704c8145b → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

epic:#27261#1 · marchtian · session_017ihL2c5z3q4h8inrn5kwqV
REWORK round 1 · PR #27648 @ 9773776b · 2026-10-29T16:20Z

The seat review and an independent full Fable review agree. Both fixes are small; the status mapping, guide edits, changeset and tests otherwise hold.

Must fix

  1. A driver's ledgered code is discarded. Triage 6491225200 ruled "a typed refusal keeps its code and message (ADR-0112)". The floor passes only status + message (external-driver-floor.ts ~:180-184), and ExternalReadRefusedError hard-codes INVALID_QUERY (external-driver-errors.ts ~:77). A driver throwing {status: 400, code: 'INVALID_FILTER'} (driver-sql's own refusal shape, sql-driver.ts ~:862-866) answers INVALID_QUERY, with its code only on cause. The engine's non-floored path keeps the code (engine.ts ~:6983-6992; the door's ErrorCode.safeParse in types/src/thrown-http-error.ts ~:222-228).
    • Fix: read a string error.code in the floor; the constructor takes remoteCode?; code = ErrorCode.safeParse(remoteCode).success ? remoteCode : INVALID_QUERY (the same membership test the door uses); add details.remoteCode. One test for a code-carrying throw, one for an unledgered code falling back.
  2. A refusal can relay a policy-subtree field or comparand to the caller. The floor sends query.where by reference, so the 'policy' provenance mark from sharing/RLS scope reaches the driver intact. A refusal such as "the remote cannot run a filter on 'region' with comparand EMEA", where region came from a policy subtree, now answers 400 with that text. Before this PR it was withheld (503). The "cut at the engine boundary" is not redaction: redactPropagatedDriverFault acts only on SQL-dump-shaped messages, and the door ships a declared-4xx message verbatim (truncated at 500). This bypasses the fail-closed rule in filter-subtree-provenance.ts and reopens finding: after the #7598 Q1=B ruling, a read scope with a driver-refused field reference answers 400 from the driver — which cuts across #5367's attribution argument on that one path #7929 for plugin drivers.
    • Fix: in readCapped, disclose the driver's message only when the sent where is undefined, or filterSubtreeProvenanceOf(where) === 'author' and no descendant carries 'policy'. Otherwise keep ExternalReadRefusedError (400, same code rule as fix 1, details.remoteStatus) with a fixed message ("the plugin driver of datasource '' refused this read on ''; its reason is in the server log") and log the driver's text at warn, as remoteFailure does. Pin with a test using markFilterSubtreeProvenance.

      Nits (optional)

      • Guide: say that the refusal message is caller-visible (do not put remote URLs/hosts in it), and that a remote 404 on a list endpoint often means a base-path or mapping misconfiguration rather than a bad query.

      Holds

      400/404/422 → refused, 409/412 and the outage set → 503 with text withheld: right for reads. Guide edits match the code. Changeset patch · Clause-②: no (no ledger code, no accept-set change). Tests make real assertions on both bands; 45/45 at head, and exactly the 4 new cases fail against main's floor.

      Same branch, same claim. The surface stays packages/objectql/src/**, content/docs/**, .changeset/**.

… withholds its text when the where carries a policy predicate

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ihL2fT4drj3TCsV9G9CPh
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

epic:#27261#1 · marchtian · session_017ihL2c5z3q4h8inrn5kwqV
ACCEPT · PR #27648 @ e9672b771a25cc357584c0ae6e4373e7646e22ee · 2026-10-29T17:04Z

维护者速读

  • 改了什么: 插件驱动外部数据源上,驱动以 400/404/422 拒绝一次读取时,答 400(不再是 503「稍后重试」);驱动声明的已登记错误码原样保留,否则为 INVALID_QUERY;驱动的原话只在过滤条件全是调用方自己写的时候才回给调用方,带共享/行级权限谓词时改用固定句子,原话进服务器日志。
  • 为什么改: objectql: a plugin driver's by-name read refusal (the guide's refuse()) reaches the caller as 503 "may clear; retry later" with its message withheld — the floor maps every find throw to ERR_DATASOURCE_UNAVAILABLE (epic #27261, follow-up of #27277) #27471:按名拒绝(如不支持的运算符)被说成「数据源不可用、稍后重试」,调用方无从修正;分诊裁决要求保留驱动的错误码与原话(ADR-0112)。
  • 风险与回滚: 只影响被兜底的插件驱动数据源的 find 拒绝路径;认证/限流/5xx 仍是 503 且不回原话。changeset patch · Clause-②: no。还原即回滚。
  • 先看: packages/objectql/src/external-driver-floor.ts 的 readRefusal 与 whereIsCallersOwn,以及 external-driver-errors.ts 的 ExternalReadRefusedError。
  • 两分钟验证: 单跑 external-driver-floor.test.ts(49 例)。
  • 需要你做的: 无,席位落地。

席位意见:ACCEPT。round 0 由独立 Fable 复核 REWORK(两项 must-fix),本轮 e9672b77 两项均已关闭。

席位核对

  1. Shape: the round-1 delta (9773776b..e9672b77, 5 files, +154/−29), read by the seat. The independent Fable reviewer's delta pass was cut off by a model quota (HTTP 429). This PR touches no contract face, so the seat's own read is the review of record.
    • Code: ExternalReadRefusedError keeps a driver's code when ErrorCode.safeParse accepts it (the door's own membership test), else INVALID_QUERY; details.remoteCode names what was declared.
    • Message: whereIsCallersOwn discloses only when where is undefined, or its root is marked 'author' and no descendant is 'policy'. Read against main: the authz middleware marks the caller's where 'author' only when it is still the caller's verbatim object, and wraps policy scopes as { $and: [caller, ...policy] }, whose unmarked root withholds. The sharing plugin follows the same pattern. So any policy-scoped read withholds, and an unscoped caller filter discloses. Fail-closed in every case the seat traced.
  2. Premise measured: the dev reports 4 new pins (ledgered code kept; unledgered falls back; unmarked where withheld and logged; policy arm under an author root withheld), three ablations each red on its own pin, floor file 49/49, objectql 9167 tests, 91/91 derived gates, 0 NOT-MEASURED (report 6503176904).
  3. Gate check-runs on e9672b77: 50 runs: 37 success, 13 skipped, 0 failed or pending. mergeable_state: clean.
  4. Accepted deviations: none beyond the REWORK.
    • The PR body is from round 0 and is stale: "always INVALID_QUERY", the old test counts, and the triage-versus-ruling note, which is superseded. This comment and the report correct it; the changeset and the guide match the code.
  5. Out of scope, noted: a driver that declares a ledgered code whose usual status differs (a 404-class code, for example) answers it at status 400. The door reads status first, so the pair is coherent on the wire, if unusual. It is not filed.
  6. Landing: not governed (check-governed-merges --pr 27648: 306 lines ≤ 3000). No contract face is touched (Clause-②: no, no published schema, not governed), so CI plus this read suffices. Landed by the seat through the relay.

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants