Repository navigation
[hooks] ADR-0058 Addendum II D3 names three routes out of the batch-scoped payload, and a body-only hook can reach none of them — the sandbox context carries no per-row signal (declared ≠ observable) #11552
Description
Activity
Concentrated triage round: →
needs-user-decision+domain:spec, Feature — ADR-0058 Addendum II D3 names three supported routes for row-specific work, and a body-only hook cannot reach two of them because the batch-path signal exists on the engine context but is dropped at the sandbox boundary (declared ≠ observable). Exposing a context key is a contract-surface change: manual floor.① 项目长远合理性:ADR 名言的三条路线必须对 body-only hook 可达,否则 ADR 在声明一个作者够不着的契约;补一个只读信号键是缩小 declared≠observable 缝,不是新能力面。
② 实际业务拉动:真实应用排查撞上(hotcrm 侧调查记录)—— 作者想按 ADR 路线 1/2 做按行守卫,却无法从 hook 内部判断自己在批处理路径上,只能猜或放弃。
③ 防 AI 犯错:AI 写 hook 会照 ADR 引文选路线;信号不可达时它写出「看起来符合 ADR」的 hook 静默做错;有显式只读信号则 hook 能在批路径上响亮拒绝(throw),错误当场可见。
④ 创业阶段不扩散:A 案只加一个只读键到既有 context,是最小面;若不做,诚实的替代是把 ADR 文本收窄成「仅 caller 分页可用」(docs-only,零运行时改动)。
推荐:A = 在 sandbox context 暴露既有 engine 侧批路径信号(只读一键 + conformance);B = 收窄 ADR 文本(docs-only);C = defer。回一个字母即可。
置信缺口:未核对 sandbox context 的键裁剪是否是有意的安全边界(若裁剪本身是裁过的,A 需先翻那份出处)。
Generated by Claude Code
os-support-ai commented
on Aug 25, 2026 CollaboratorMore actionsMaintainer ruling — options 1 + 2 together: the sandbox context gains the per-row signal AND the declared
input.optionsvisibilitySource: maintainer, 2026-08-25, live PM chat (decision-inbox batch 5 review, session
session_01KWRU3s15AJz7PGW7a7wdCh), verbatim: 「全部同意」 — accepting the batch's presented recommendation for this card: the card's options 1 and 2, in one motion.- Option 1: marshal the per-row dispatch signal into
buildSandboxContext(a read-only key —dispatchor equivalent), so ADR-0058 Addendum II D3's routes 1 (scoped throw) and 2 (ctx.apiper row) become expressible from a shipped body. D3's contract statement itself is untouched — this closes declared≠observable, it does not re-litigate the unenforcement reasoning. - Option 2: surface
input.options(multi/where) to the body context — D2 already declares them visible to thebefore*phase; today that is true of the engine context and false of the shipped body. This is a spec-vs-engine drift fix riding the same PR shape. - Option 3 (advisory lint in
validate-hook-body-writes) is not part of this ruling — its value is real only once the signal is reachable; it may return later as a graded finding. - Pre-step from the stated confidence gap: verify the sandbox key-pruning was not itself a ruled security boundary — check the provenance of
buildSandboxContext's key list first; if the pruning was deliberate, stop and fork back rather than widening past a ruling. - The tripwire test on
hotcrm@claude/issue-1265-...is written to go red when this lands — use it as the acceptance signal; hotcrm#1265 follows mechanically.
Discipline: Clause-② yes (the hook body contract surface widens);
CONTRACT_REVIEW_TIER; spec seat coordinates the runtime half per the seam.State:
needs-user-decision→pm:queuein the same write;domain:specstands.
Generated by Claude Code
- Option 1: marshal the per-row dispatch signal into
Claim: PM loop round R6 (wave 3)
Session:session_01NDGG54XF5gbTLdQzCtnaVV
Branch:claude/issue-11552-sandbox-perrow-signal
Worktree:objectstack-issue-11552
Domain:domain:spec
File surface:packages/runtime/src/sandbox/body-runner.ts(buildSandboxContext — the ruled read-only per-row signal +input.optionsvisibility) + its tests + the spec-side hook-body context declaration (packages/spec/src/data/hook-body*/ the contract face that declares the sandbox context) + conformance + changeset;packages/runtime/src/http-dispatcher.tsEXCLUDED (H17 trigger file of holds #7898/#8347 — stop and report if the seam genuinely requires it) (stop on breach; explain in the report)
Container & model: M,mode:subagent,model: claude-fable-5— dispatch-gates 2026-08-25 @8619f95: "no path-derived mandate … a card changing contract accept/reject behaviour or widening the public surface is claude-fable-5"; clause-② content leg (the hook-body contract surface widens, per the recorded ruling) ⇒ fable mandatory
Clause-②: yes
Serial constraints cleared: sole writer of body-runner.ts and the hook-body contract face this batch; disjoint from all in-flight surfaces. Ruled pre-step carried: verify the sandbox key-pruning was not itself a ruled security boundary (provenance of buildSandboxContext's key list) BEFORE widening — deliberate pruning ⇒ stop and fork back. Acceptance signal: the hotcrm tripwire test (hotcrm@claude/issue-1265-...) is written to flip red when this lands. No pin assertions on sibling behavior.
Generated by Claude Code
{ "issue": 11552, "status": "done", "branch": "claude/issue-11552-sandbox-perrow-signal", "pr": "https://github.com/objectstack-ai/objectstack/pull/12217", "premise_still_valid": true, "summary": "Ruled pre-step first: provenance of buildSandboxContext's key pruning checked — installFlatInput's ownKeys hiding is documented as ergonomic flat-record enumeration, the sandbox key list evolved only by deleting phantom keys (#5906/#6316/#7448, PD #12), and hook.zod.ts (#7254) records the body-face split with convergence 'deliberately not decided' — an open question, not a ruled security boundary, so no fork-back. Implemented ruling options 1+2 in one motion: buildSandboxContext marshals the #6966 dispatch marker as read-only { mode, index } (scope deliberately not marshalled — a JSON copy cannot keep its shared-identity contract and would be a silently-inert write channel), and input.options crosses as the D2 projection { multi, where }, grafted frozen + NON-ENUMERABLE onto the VM input so enumeration stays payload-only (the #7254 witness holds) and the mutatedInput write-back can never clobber the engine's live options bag. input.id stays absent (not ruled in; per-row row id channel is ctx.previous.id). Spec side: hook.zod.ts input/dispatch TSDoc updated so declared = observable; no generated artifact staled. http-dispatcher.ts untouched; no option-3 lint. Changeset: runtime minor + spec patch.", "tests": "All at HEAD 9cf3615, clean tree, via os-verify-lock (VERDICT lines read, not piped $?). Premise re-measured on BASE 22c42c9 with a real ObjectQL+SqlDriver+QuickJS probe: all four issue facts reproduced verbatim (hasDispatch 'undefined', inputKeys ['title'], inputId undefined, inputOptions undefined, previousKeys bound). Post-fix: new tripwire-mirror packages/runtime/src/sandbox/perrow-dispatch-signal.integration.test.ts (hotcrm tripwire replicated in-repo — no hotcrm access this session) + 4 unit cases in body-runner.test.ts; runtime src/sandbox/ full directory 14 files / 163 passed; runtime tsc --noEmit green (script echo confirmed); spec hook.test.ts+hook-body.test.ts+bulk-write-hook-conformance.test.ts 107 passed; spec typecheck green; spec build + check:generated 'All 14 generated artifacts are up to date'; showcase hook-body-persisted-writes 6/6 incl. THE #7254 WITNESS, run against rebuilt runtime dist with the graft anchor grep-confirmed in dist/index.js (4 hits) before the run — no unbuilt-dist leg. Reverse verification from the committed state: git restore --source=BASE on the 3 runtime sources, mutation proven on disk (change anchor greps 0 in all three), re-run went red in 3 assertions (integration lost the signal: expected null to be 'record'; unit lost ctx.dispatch: expected 'undefined' to be 'object'; bare-object edge showed the whole bag where the projection hides context: expected 'object' to be 'undefined' — direction differs from template on that one leg and is reported as observed); restore proven by clean git status + anchors back (4/1/2). No build in the reverse legs — runtime vitest reads in-package src directly. dispatch-gates recomputed from the real change set (7 paths, merge base 22c42c9): all matched script gates green (cross-package-test-inputs, test-source-alias, type-source-resolution, published-files, slot-lookup, spec-parsed-alias, merge-driver, objectui-changeset, changeset-gate-self-tests, adr-0087-registration, changeset-no-major, ci-filter-parity, doc-formula-expressions, empty-state, liveness, strictness-ledger, variant-docs) + check:nul-bytes OK. Declared narrowing: runtime full 189-file suite and repo-wide lint left to CI — diff's runtime surface is src/sandbox/** (run in full), package-wide tsc covers in-package consumers, new keys additive+optional. One verify-lock queue-timeout (exit 99) waited out with lock-free gate work; holder was a sibling spec vitest run, later cleared.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
os-dev-report
(Supersedes the previous comment, whose HTML-comment marker the platform sanitizer removed — the standing failure mode; content identical.)
{ "issue": 11552, "status": "done", "branch": "claude/issue-11552-sandbox-perrow-signal", "pr": "https://github.com/objectstack-ai/objectstack/pull/12217", "premise_still_valid": true, "summary": "Ruled pre-step first: provenance of buildSandboxContext's key pruning checked — installFlatInput's ownKeys hiding is documented as ergonomic flat-record enumeration, the sandbox key list evolved only by deleting phantom keys (#5906/#6316/#7448, PD #12), and hook.zod.ts (#7254) records the body-face split with convergence 'deliberately not decided' — an open question, not a ruled security boundary, so no fork-back. Implemented ruling options 1+2 in one motion: buildSandboxContext marshals the #6966 dispatch marker as read-only { mode, index } (scope deliberately not marshalled — a JSON copy cannot keep its shared-identity contract and would be a silently-inert write channel), and input.options crosses as the D2 projection { multi, where }, grafted frozen + NON-ENUMERABLE onto the VM input so enumeration stays payload-only (the #7254 witness holds) and the mutatedInput write-back can never clobber the engine's live options bag. input.id stays absent (not ruled in; per-row row id channel is ctx.previous.id). Spec side: hook.zod.ts input/dispatch TSDoc updated so declared = observable; no generated artifact staled. http-dispatcher.ts untouched; no option-3 lint. Changeset: runtime minor + spec patch.", "tests": "All at HEAD 9cf3615, clean tree, via os-verify-lock (VERDICT lines read, not piped $?). Premise re-measured on BASE 22c42c9 with a real ObjectQL+SqlDriver+QuickJS probe: all four issue facts reproduced verbatim (hasDispatch 'undefined', inputKeys ['title'], inputId undefined, inputOptions undefined, previousKeys bound). Post-fix: new tripwire-mirror packages/runtime/src/sandbox/perrow-dispatch-signal.integration.test.ts (hotcrm tripwire replicated in-repo — no hotcrm access this session) + 4 unit cases in body-runner.test.ts; runtime src/sandbox/ full directory 14 files / 163 passed; runtime tsc --noEmit green (script echo confirmed); spec hook.test.ts+hook-body.test.ts+bulk-write-hook-conformance.test.ts 107 passed; spec typecheck green; spec build + check:generated 'All 14 generated artifacts are up to date'; showcase hook-body-persisted-writes 6/6 incl. THE #7254 WITNESS, run against rebuilt runtime dist with the graft anchor grep-confirmed in dist/index.js (4 hits) before the run — no unbuilt-dist leg. Reverse verification from the committed state: git restore --source=BASE on the 3 runtime sources, mutation proven on disk (change anchor greps 0 in all three), re-run went red in 3 assertions (integration lost the signal: expected null to be 'record'; unit lost ctx.dispatch: expected 'undefined' to be 'object'; bare-object edge showed the whole bag where the projection hides context: expected 'object' to be 'undefined' — direction differs from template on that one leg and is reported as observed); restore proven by clean git status + anchors back (4/1/2). No build in the reverse legs — runtime vitest reads in-package src directly. dispatch-gates recomputed from the real change set (7 paths, merge base 22c42c9): all matched script gates green (cross-package-test-inputs, test-source-alias, type-source-resolution, published-files, slot-lookup, spec-parsed-alias, merge-driver, objectui-changeset, changeset-gate-self-tests, adr-0087-registration, changeset-no-major, ci-filter-parity, doc-formula-expressions, empty-state, liveness, strictness-ledger, variant-docs) + check:nul-bytes OK. Declared narrowing: runtime full 189-file suite and repo-wide lint left to CI — diff's runtime surface is src/sandbox/** (run in full), package-wide tsc covers in-package consumers, new keys additive+optional. One verify-lock queue-timeout (exit 99) waited out with lock-free gate work; holder was a sibling spec vitest run, later cleared.", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
Contract review: PASS — PR #12217, head
9cf3615(round R6, sessionsession_01NDGG54XF5gbTLdQzCtnaVV).Tier fuse read this sub-round:
get_session.external_metadata.last_served_model=claude-fable-5— meetsCONTRACT_REVIEW_TIER. Review performed against the PR's full 7-file diff, not the report alone.Contract surface findings:
- Spec accept/reject set: unchanged. The only
packages/spec/src/**path isdata/hook.zod.ts, and its diff is TSDoc-only — no schema shape, no.describe()strings, no parse behaviour moves.check:generated's "all 14 artifacts up to date" corroborates (no artifact staled). - The widening is where the card ruled it: the sandbox body face gains
ctx.dispatch(frozen{ mode, index }) andctx.input.options(frozen, non-enumerable{ multi, where }projection). Both additive and optional; maintainer ruling (comment 5405062624, options 1+2) implemented to the letter, option 3 correctly absent, D3 untouched. - Security posture holds the allowlist line: projection not whole bag (raw bag can carry execution context / driver-facing state);
scopewithheld on shared-identity grounds — marshalling it would ship a silently-inert write channel, the same defect family the card closes; unrecognised marker shape → absent (the back-compatible read direction); the graft snippet interpolates no caller data (values cross viasetGlobalJson). - The two boundary invariants are proven, not asserted: enumeration stays payload-only (the HookContextSchema.input's contract table documents only the raw-engine envelope — every declarative hook sees a flat record view, and in a sandboxed
bodythe documentedinput.dataspelling is a TypeError #7254 witness re-run green against rebuilt dist), and the write-back cannot carry the graft (non-enumerable ⇒ excluded from the post-runJSON.stringifydump; the integration test additionally pins the caller's live bag unclobbered). - Reverse verification is sound: base-revert proven on disk (anchor greps 0/0/0), three assertions red in the expected direction, restore proven (4/1/2), no build step in the src-read legs.
- Ruled pre-step (provenance of the sandbox key pruning) executed and reported: pruning is ergonomic/PD-#12 history, not a ruled security boundary — no fork-back owed. H17 exclusion held:
http-dispatcher.tsuntouched. - Changeset
runtime: minor / spec: patchis right for an additive sandbox-face widening with a docs-only spec diff; no ADR-0087 entry owed (no accept/reject change) —check-adr-0087-registrationandcheck-changeset-no-majorboth green in the dev's gate run.
Verdict: PASS. Dual-carrier
needs:contract-reviewclears with this comment; the PR proceeds to ready + auto-merge once every CI check on head9cf3615is green (queue eligibility = all checks, not just required).
Generated by Claude Code
- Spec accept/reject set: unchanged. The only
- added a commit that references this issue
on Aug 25, 2026 os-dev-report
(Follow-up delta over comment 5411417406 — CI red fixed and pushed; that comment's content otherwise stands.)
{ "issue": 11552, "status": "done", "branch": "claude/issue-11552-sandbox-perrow-signal", "pr": "https://github.com/objectstack-ai/objectstack/pull/12217", "premise_still_valid": true, "summary": "Follow-up on the coordinator-relayed CI red (Type Check · debt ledger, run 32855958824, head 9cf3615): the TEST_DEBT re-measure lifts runtime's tsconfig test exclusion, and this PR's conformance harness added exactly the +1 — verbatim: packages/runtime/src/sandbox/perrow-dispatch-signal.integration.test.ts(129,21): error TS2554: Expected 2-5 arguments, but got 1. Cause: registry.registerObject(schema, packageId, ...) requires packageId; the 1-arg spelling was copied from a sibling harness whose identical error is frozen debt. Fix at the call site (registerObject(ARTICLE as any, 'probe')) — no ledger raise, no ts-ignore/exclusion. Pushed as bf9ac67 (head 9cf3615 -> bf9ac67), same branch, ordinary commit. Everything else from the prior report stands: options 1+2 implemented, pre-step provenance verdict no-fork, PR stays draft for the contract-review chain.", "tests": "Reproduced with the gate's own method: lint.yml parity closure build (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70 tasks OK) then the TEST_DEBT-equivalent generated project for runtime — 228 raw errors pre-fix with the +1 in my file, 227 with 0 in my files post-fix. Authoritative gate re-run at the fixed tree, verdict verbatim: 'check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 397.1s, 1843 raw tsc error(s) total, none above its recorded number.' (os-verify-lock VERDICT command-exit 0). Full sandbox suite re-run at the fixed tree: 14 files / 163 passed (VERDICT command-exit 0). check:nul-bytes OK. Clean git status before push, so both verdicts describe exactly the tree committed as bf9ac67. Two verify-lock queue-timeouts (exit 99) during this leg were waited out per discipline (interval work, then re-acquire; second acquisition waited 8m34s).", "open_questions": [], "out_of_scope_findings": [] }
Generated by Claude Code
- added a commit that references this issue
on Sep 1, 2026
Filed unassigned — a record from an app-side investigation (hotcrm#1265), not a claim.
The gap
ADR-0058 Addendum II D3 makes the predicate-update payload BATCH-scoped and says so plainly, including that a rewrite conditioned on the row is outside the contract. It then names the three supported routes for row-specific work, quoted from the installed
@objectstack/spec17.1.0 (dist/data/index.d.ts):The ADR is explicit that D3 is a contract statement and not an enforcement, and gives the reason (no static rule can decide whether a rewrite is row-invariant). That part is a deliberate, reasoned ruling and this card does not argue with it.
The gap is narrower and, I think, unintended: routes 1 and 2 require the handler to know it is on the per-row predicate path, and a hook shipped body-only cannot know that. The signal exists on the engine context and is dropped at the sandbox boundary.
Measured on 17.1.0
buildSandboxContext(@objectstack/runtime) builds the hook body's context with exactly:No
dispatch. AndinputisunwrapProxyToPlain(engineCtx.input), i.e.Object.fromEntries(Object.entries(v))— enumerable own keys only. On the per-row pathdispatchPerRowBeforeHooksbuildsinput: { id: rowId, data: batchCtx.input.data, options }, andinstallFlatInputwraps it in a proxy whoseownKeysreturns only the payload's data keys and whosegetOwnPropertyDescriptormarksid/options/datanon-enumerable. So the unwrap copies the payload and dropsidandoptionswith it.Probe body run through the real
QuickJSScriptRunner+hookBodyRunnerFactory, against an engine context shaped exactly asdispatchPerRowBeforeHooksbuilds one:So inside a shipped body:
ctx.dispatch?.mode— absent, so route 1 (throw) cannot be scoped to the batch path;ctx.input.options.multi/.where— absent, though D2 states these are visible to thebefore*phase ("input.optionsis the CALLER's bag (whereandmultivisible)"). That is true of the engine context and false of the body context;ctx.input.id— absent, so route 2 (ctx.apiper row) has no row id frominputand, more fundamentally, cannot tell whether it should fire at all (on the single-record path it would double-write);ctx.previous— present. The one row-conditioned input that does cross is exactly the one D3 says must not aim a rewrite.Route 3 (caller paginates) is not available to a handler at all.
Net: the surface hands the author the ingredient that makes the mistake expressible and withholds every ingredient that would let them comply.
Why this bites harder than "unenforced"
The ADR's own reasoning for leaving D3 unenforced is that no static rule can decide row-invariance, and that the hazard therefore belongs in authoring docs plus a possible advisory lint. That reasoning assumes the author who reads the docs can then act on them. Today they cannot, if the hook ships body-only.
Worse, the natural fix is silently inert. A guard written as:
lowers cleanly through
extractHookBody, passes in-process handler tests (which run the closure with a full engine context), and evaluates tofalseon every dispatch in production — so the widening continues while the code reads as though it were prevented. This is thehint/ctx.log.debug/crypto.hashfamily (#7661, #4391): declared on one face, absent on the face that ships.Blast radius, measured in the exemplar app
hotcrm#1265 was filed for one hook. Triaging its
beforeUpdatehandlers against the mechanical test — does a payload write depend on the row? — 9 of the 17previous-reading hook files carry at least one row-conditioned payload write, and none of them can be fixed app-side for the reason above:knowledge_article_publish_timestampspublished_atexistence criterioncampaign_member_lifecycleresponse_dateexistence criterion +wasRespondedfrompreviousforecast_derive_periodsnapshot_date:!input.x && !previous?.xcase_sla_defaultssla_due_dategated on!previous?.sla_due_date, value from the row's account tiertask_completioncompleted_date/progress_percentgated onprevious?.statusopportunity_lifecycleclose_date/stage_entry_dategated onprevious.stageevent_schedule_deriveduration_minutes/end_datetimefrompreviousfallbackslead_duplicate_checkduplicate_of_typegated onprevious?.duplicate_statusaccount_protectionlast_activity_dategated oninput.owner_id !== previous.owner_idThe clean ones are instructive too:
product_catalogandcontract_validationreadpreviousonly to throw, which is precisely D3's blessed pattern — and they get away with it only because they refuse unconditionally, never "refuse on the batch path".What would close it
Options, roughly in increasing cost — the choice is a maintainer's, and I have deliberately not assumed one:
dispatch(or a booleanisPerRow) tobuildSandboxContext. Makes route 1 expressible immediately.input.options(multi/where), which D2 already declares visible to thebefore*phase and which the body context contradicts today. This is arguably a spec-vs-engine drift independent of D3.packages/lint'svalidate-hook-body-writes) — flag a payload write whose guard or value readsprevious/input.id. Complements 1 rather than replacing it: a lint tells the author they have a problem, but only 1 gives them a way to fix it.If the intended answer is instead "a row-conditioned stamp does not belong in a hook at all, use an action/flow", that is a legitimate ruling too — but it should be written down, because D3 currently reads as though routes 1 and 2 are available.
Reproduction
hotcrm@claude/issue-1265-batch-scoped-payloadcarries a passing tripwire test asserting the four facts above (test/hooks-runtime-service.test.ts,#1265 — the shipped hook body cannot tell it is on a per-row predicate dispatch). It is written to go red when this card is fixed.Related: hotcrm#1265 (the app-side card, blocked on this), #5574 (per-row
before*dispatch), #6966 (input.idbinding semantics), #3700 (hook body writes not statically checkable), #7661 / #4391 (same declared-but-absent-in-sandbox family).