Skip to content

[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

@os-warren

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/spec 17.1.0 (dist/data/index.d.ts):

The three supported routes for row-specific work are: throw (which is what the guard case wants), write through ctx.api per row, or have the CALLER paginate the batch into by-id updates.

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:

input, previous, user, session, event, object, result, api, log, crypto

No dispatch. And input is unwrapProxyToPlain(engineCtx.input), i.e. Object.fromEntries(Object.entries(v)) — enumerable own keys only. On the per-row path dispatchPerRowBeforeHooks builds input: { id: rowId, data: batchCtx.input.data, options }, and installFlatInput wraps it in a proxy whose ownKeys returns only the payload's data keys and whose getOwnPropertyDescriptor marks id / options / data non-enumerable. So the unwrap copies the payload and drops id and options with it.

Probe body run through the real QuickJSScriptRunner + hookBodyRunnerFactory, against an engine context shaped exactly as dispatchPerRowBeforeHooks builds one:

hasDispatch  : "undefined"
inputKeys    : ["title"]        <- payload only
inputId      : undefined
inputOptions : undefined
previousKeys : ["id","status","published_at"]
ctxKeys      : ["input","previous","user","session","event","object","api","log","crypto"]

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 the before* phase ("input.options is the CALLER's bag (where and multi visible)"). That is true of the engine context and false of the body context;
  • ctx.input.id — absent, so route 2 (ctx.api per row) has no row id from input and, 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:

if (ctx.dispatch?.mode === 'per-row') { /* refuse, or skip the row-conditioned write */ }

lowers cleanly through extractHookBody, passes in-process handler tests (which run the closure with a full engine context), and evaluates to false on every dispatch in production — so the widening continues while the code reads as though it were prevented. This is the hint / ctx.log.debug / crypto.hash family (#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 beforeUpdate handlers against the mechanical test — does a payload write depend on the row? — 9 of the 17 previous-reading hook files carry at least one row-conditioned payload write, and none of them can be fixed app-side for the reason above:

hook shape
knowledge_article_publish_timestamps published_at existence criterion
campaign_member_lifecycle response_date existence criterion + wasResponded from previous
forecast_derive_period snapshot_date: !input.x && !previous?.x
case_sla_defaults sla_due_date gated on !previous?.sla_due_date, value from the row's account tier
task_completion completed_date / progress_percent gated on previous?.status
opportunity_lifecycle close_date / stage_entry_date gated on previous.stage
event_schedule_derive duration_minutes / end_datetime from previous fallbacks
lead_duplicate_check duplicate_of_type gated on previous?.duplicate_status
account_protection last_activity_date gated on input.owner_id !== previous.owner_id

The clean ones are instructive too: product_catalog and contract_validation read previous only 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:

  1. Marshal a per-row signal into the sandbox context. Smallest change: add dispatch (or a boolean isPerRow) to buildSandboxContext. Makes route 1 expressible immediately.
  2. Also surface input.options (multi / where), which D2 already declares visible to the before* phase and which the body context contradicts today. This is arguably a spec-vs-engine drift independent of D3.
  3. Advisory lint at the seam the ADR itself names (packages/lint's validate-hook-body-writes) — flag a payload write whose guard or value reads previous / 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-payload carries 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.id binding semantics), #3700 (hook body writes not statically checkable), #7661 / #4391 (same declared-but-absent-in-sandbox family).

Activity

  1. claude commented on Aug 25, 2026

    @claude
    Contributor

    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

  2. added theissue type on Aug 25, 2026
  3. os-support-ai commented on Aug 25, 2026

    @os-support-ai
    Collaborator

    Maintainer ruling — options 1 + 2 together: the sandbox context gains the per-row signal AND the declared input.options visibility

    Source: 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 — dispatch or equivalent), so ADR-0058 Addendum II D3's routes 1 (scoped throw) and 2 (ctx.api per 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 the before* 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:queue in the same write; domain:spec stands.


    Generated by Claude Code

  4. self-assigned this
    on Aug 25, 2026
  5. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator

    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.options visibility) + 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.ts EXCLUDED (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

  6. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator
    {
      "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

  7. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator

    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

  8. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator

    Contract review: PASS — PR #12217, head 9cf3615 (round R6, session session_01NDGG54XF5gbTLdQzCtnaVV).

    Tier fuse read this sub-round: get_session.external_metadata.last_served_model = claude-fable-5 — meets CONTRACT_REVIEW_TIER. Review performed against the PR's full 7-file diff, not the report alone.

    Contract surface findings:

    1. Spec accept/reject set: unchanged. The only packages/spec/src/** path is data/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).
    2. The widening is where the card ruled it: the sandbox body face gains ctx.dispatch (frozen { mode, index }) and ctx.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.
    3. Security posture holds the allowlist line: projection not whole bag (raw bag can carry execution context / driver-facing state); scope withheld 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 via setGlobalJson).
    4. 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 body the documented input.data spelling 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-run JSON.stringify dump; the integration test additionally pins the caller's live bag unclobbered).
    5. 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.
    6. 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.ts untouched.
    7. Changeset runtime: minor / spec: patch is 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-registration and check-changeset-no-major both green in the dev's gate run.

    Verdict: PASS. Dual-carrier needs:contract-review clears with this comment; the PR proceeds to ready + auto-merge once every CI check on head 9cf3615 is green (queue eligibility = all checks, not just required).


    Generated by Claude Code

  9. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator

    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

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

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions