Skip to content

fix(spec): ActionEngineFacade.delete declares the id array the runtime already accepts, and says which convention is the contract - #17608

Merged
os-bill merged 6 commits into
mainfrom
claude/issue-15117-action-engine-delete-id-array
Sep 11, 2026
Merged

fix(spec): ActionEngineFacade.delete declares the id array the runtime already accepts, and says which convention is the contract#17608
os-bill merged 6 commits into
mainfrom
claude/issue-15117-action-engine-delete-id-array

Conversation

@os-bill

@os-bill os-bill commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15117

  • Clause-②: yes — widening a published declared parameter widens the accept set of a published contract. needs:contract-review hangs on both carriers from the moment this PR exists, and nothing lands until an at-tier review returns.

What the two ends said

end said
spec packages/spec/src/ui/action-params.zod.ts:242 delete(object: string, id: string) — one id
runtime packages/runtime/src/action-execution.ts:1471 async delete(object: string, idOrIds: string | string[]), iterating ql.delete once per id
runtime :1469 "Tolerant of both the single-id and array conventions handler suites use"
a shipped example examples/app-todo/src/actions/task.handlers.ts:18 a local ActionContext copy of the facade, reaching the array form at :87

Measured on this branch's merge base 86c50528, not taken from the card — the card cites the runtime arm at :1174-1181, which had drifted by about three hundred lines. Lit control: ActionEngineFacade occurs 5 times in action-params.zod.ts, so those readings are readings.

What this PR does

  1. The widening plus the contract sentence. The slot is delete(object: string, idOrIds: string | string[]), and the member's doc comment now states the contract rather than leaving a reader to infer it from a runtime comment two packages away: both spellings are contract; the single id is one row, the array is a set; the array form is a convenience over the same per-row path, never a bulk or atomic delete (no transaction around the set, a part-way failure leaves the earlier ids deleted); an empty array deletes nothing and resolves. The interface-level docblock, which said find was the one member whose argument shape the signature never settled, now names both.
  2. The runtime's "accident" comment is retired. It recorded the tolerance as something two handler suites caused; after this card it is a declared contract, and the comment says so. Comment only — the arm's behaviour, its normalisation and its per-id loop are byte-identical, and the replacement is deliberately the same two lines so the arm stays on line 1471.
  3. The example's hand-rolled ActionContext is gone, replaced by ActionHandlerContext from @objectstack/spec/ui on all 8 handlers. That also removes the already-drifted find copy at :27 (the pre-ActionEngineFacade.find(object, query) takes a bare filter while insert/update/delete take explicit shapes — the type says neither, and reading it wrong returns empty with no error #14175 query: Record... spelling). ActionHandlerContext is used rather than ActionHandler for the annotation because ActionHandler returns unknown, which would erase the precise return types of cloneTask and exportTasksToCSV; the two are named together in the file's own comment.
  4. Pins. action-params.test.ts gains a #15117 block modelled on its #14175 neighbour: a type-level Assert that the slot is exactly string | string[] (read off the interface, never retyped), positive controls for both conventions and the empty set, and four @ts-expect-error refusals.

Proven by behaviour, both directions

Every reading below is a real run in this worktree; exit codes were captured before any pipe.

The defect, before the widening — the example annotated with the published type, against the unwidened dist:

src/actions/task.handlers.ts(88,38): error TS2345: Argument of type 'string[]' is not assignable to parameter of type 'string'.

Afterpnpm --filter @objectstack/example-todo typecheck exits 0 with zero error TS lines. The single-id convention still type-checks (pinned as a positive control in the spec suite).

Lit control — the same call site with an obviously-wrong argument, on the widened type, is still refused:

src/actions/task.handlers.ts(88,38): error TS2345: Argument of type 'number' is not assignable to parameter of type 'string | string[]'.

That control matters twice: it proves the green above is not a program that stopped checking the file. Restore was proven by state — git hash-object equal to the HEAD blob, whole-tree git status --porcelain empty.

Ablation — the widening reverted in src, packages/spec rebuilt, and ablation-dist-preflight used to prove the mutation reached the artifact the consumers resolve through (marker absent from all 216 built files) before any colour was believed:

examples/app-todo  src/actions/task.handlers.ts(88,38): error TS2345: Argument of type 'string[]' is not assignable to parameter of type 'string'.
packages/spec      src/ui/action-params.test.ts(485,49): error TS2344: Type 'false' does not satisfy the constraint 'true'.
                   src/ui/action-params.test.ts(493,11) / (504,11) / (506,11): error TS2322 on the positive controls

Line 485 is the Assert pin, so the pin is capable of failing. The restore leg rebuilt and re-proved the marker present in 2 built files; the tree was verified clean by whole-tree git status --porcelain and by blob hash, and both channels went back to green.

Verification

  • pnpm --filter @objectstack/spec typecheck — exit 0 (src, scripts and the test layer; check:test-typecheck OK).
  • pnpm --filter @objectstack/spec exec vitest run --project local src/ui/action-params.test.ts — 32 passed.
  • pnpm --filter @objectstack/runtime typecheck — exit 0; the four action-execution* suites — 119 passed.
  • pnpm --filter @objectstack/example-todo typecheck — exit 0.
  • pnpm --filter @objectstack/spec check:generated — all 15 generated artifacts up to date, after a real build (nothing to regenerate).
  • All gate families scripts/pm/dispatch-gates.mjs derives for this diff were run, all green — 84 before the review, and 106 after it re-derived with content/docs/** in the change set; --ran reconciles 106 derived / 106 run / 0 unrun on cc5b7925cb. Across both rounds nine arrived first as prerequisite refusals — an unbuilt dist, or check:type-check-debt OOM-ing under a tighter NODE_OPTIONS than its own CI-shaped 6144 MB ceiling — each of which says in its own words that nothing was measured. Every one was satisfied and re-run; ⛔ not one was reported as a pass, and ⛔ not one as a finding. The single genuine local finding was mine: check-spec-docblock-symbol-anchors refused the :1471 line anchor I had written, repaired to a symbol anchor.
  • pnpm lint — the repo-wide eslint . --no-inline-config pass, not a narrowed subset — exit 0, re-run at each head; the current head is 8c90c0b0f3.
  • origin/main was merged in with scripts/pm/os-regen-merge.sh; the spec generated-artifact gates were re-run green on the merged tree.

Changeset

minor on @objectstack/spec. Corrected under contract review — this PR first shipped patch, and that was wrong. The written rule (.github/workflows/pr-automation.yml, maintainer ruling 2026-09-04 batch #35) is that a purely additive widening of a published package's public surface takes at least minor, and a commit type may raise a bump but never lower it below what the act requires. This PR's own Clause-②: yes line says the change widens the accept set, in the same words. The find precedent the first version leaned on does not reach: #14175 was a narrowing, it landed the day the rule was ruled, and the rule explicitly disclaims pre-rule patch precedents. "Nothing is removed and nothing narrows" remains true in the changeset body — it argues the change is not breaking, which nobody disputed.

How the wrong level survived this PR's own verification, recorded because the shape matters more than the fix. The first report cited check-changeset-no-major --base origin/main :: exit 0 as evidence the level was fine. That gate reads the clause-② declaration from the event payload and nothing else, so a local run with no --event cannot exercise the LEVEL axis at all: the exit 0 said the script ran, not that the level was graded. CI, which had the payload, had already refused 86 seconds earlier. A gate that exits 0 is not a reading unless the instrument could have come back the other way.

And the first correction of that paragraph was itself wrong, so it is corrected here rather than quietly dropped. It said "the level question here has exactly one instrument — CI with the payload". That is false by the gate's own header: scripts/check-changeset-no-major.mjs documents --base SHA --event event.json as the level axis driven offline — labels and body only, no token, no network — and I authored this body and knew the labels, so writing that payload was always within reach. Declining to re-run the instrument that could not answer was right; concluding that no instrument could was not. The response to "your probe could not have come back the other way" is to pick up one that can, not to stop measuring.

So it is picked up. One detached tree, one payload, one base, the committed level the only variable — the gate reads the changeset out of git, not the working tree, which is why a working-tree flip measures nothing (my first two attempts at this pair were exactly that mistake, and both came back green):

committed '@objectstack/spec': patch   ->  exit 1   declares clause-② YES, and it grades NO package whose
                                                    packages/**/src/** it moves at `minor` or above
                                                      - @objectstack/spec: patch
committed '@objectstack/spec': minor   ->  exit 0   LEVEL AXIS: declares clause-② `yes`, and no package whose
                                                    packages/**/src/** it moves is graded `patch`

The red leg reproduces CI's refusal on the head CI refused. The shipped head reads exit 0 on the same axis. That is a reading: the instrument came back the other way when the level was wrong.

examples/app-todo is private and packages/runtime's change is a comment, so neither is in the changeset.

Docs — the flagged page, disposed of

The repo's Docs Drift Check named content/docs/ui/actions.mdx on this PR (via ActionEngineFacade). It is owed, and it is fixed here rather than dismissed:

  • Its handler snippet, titled src/actions/task.handlers.ts, still annotated ctx: ActionContext — a type that file no longer declares after this PR. It now imports and annotates the published ActionHandlerContext, with a sentence on why a copy of the context drifts silently, and why a file of function declarations annotates the context type rather than ActionHandler.
  • The same snippet wrote completed_date. Removing it is right; the reason this PR first gave for removing it was false, and is corrected. The first version said the write is stripped and made the action refuse itself against completed_date_required. It is not, and it does not. A handler's ctx.engine runs elevatedbuildActionExecutionContext returns the caller's envelope plus isSystem: true — and the read-only strip is gated on !opCtx.context?.isSystem (packages/objectql/src/engine.ts), so nothing is stripped on that path at all. The refusal does not exist for an ordinary caller either: the hook's stamp is unconditional on the transition precisely so a caller-supplied value is overwritten and survives the strip, and a live test asserts it — "a caller that still sends completed_date is not punished for it — the hook value wins". The REJECTED row whose mechanism I described is labelled "Measured before the fix". The true reason is close to the inverse, and is what the page now says: the write is not stripped, it lands, and on a write that is not a completion transition the hook does not stamp, so the handler's "now" silently replaces the real completion timestamp. I had cited three sites that exist; the conclusion depended on four others I had not opened. A citation that exists is not a citation that entails.
  • The page states find's convention in a Callout at the old :149-162; the delete convention now sits directly beside it — both spellings contract, one delete per id in order, no transaction, an empty array resolving.

content/blog/protocol-first-development.mdx also shows the old single-id signature. It is a dated post, so it is not owed and is deliberately untouched.

验收备注 (acceptance notes)

Observed, deliberately not filed and not fixed here:

  • check:spec-docblock-symbol-anchors carries 7 day-one residual line-number anchors in spec docblocks, one of them the find twin of the citation this PR adds (:1183). Each already has a dated residual row and a prescribed repair in the gate's own output, so the gate is the carrier — no card is owed. This PR's own citation is a symbol anchor, not a line anchor.
  • examples/app-todo/src/actions/register-handlers.ts:78 hand-rolls the ObjectQL registration seam (registerAction, whose handler slot is (ctx: any) returning unknown) — a different surface from ActionEngineFacade, in a file this card does not name, and the (ctx: any) is exactly the untyped-bag shape ActionHandler exists to replace. Not a defect, not a contract violation, not an authoring trap, so not filed; the natural home for it is a card on typing the registration seam, which does not exist today.
  • examples/app-todo's test/ files cannot resolve their workspace deps until those packages are built. That is a prerequisite, not a defect, and it is why the first typecheck reading in this worktree carried a pile of TS2307 alongside the real TS2345.

⛔ Draft on purpose: do not flip ready, enqueue, or arm auto-merge — the contract review returns first.


Generated by Claude Code

…h convention is the contract

`ActionEngineFacade.delete` declared `id: string` while the runtime facade has
always accepted `string | string[]`, iterating one `ql.delete` per id. The
declaration is now `string | string[]`, and the member's doc comment states
what the runtime comment used to record as an accident of two handler suites:
both spellings are contract, the array form is a convenience over the same
per-row path, and it is neither bulk nor atomic.

Two consumers follow from the declaration:

- `packages/runtime`'s "tolerant of both conventions" comment is retired (a
  comment correction only; the arm's behaviour is untouched).
- `examples/app-todo/src/actions/task.handlers.ts` drops the hand-rolled
  `ActionContext` copy of the facade — which existed because the published
  type could not express its array call, and which had already drifted on
  `find` — for the published `ActionHandlerContext`.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
…#15117)

The level is measured, not assumed: nothing is removed and nothing narrows,
no runtime behaviour changes, and the sibling re-declaration of the neighbouring
member (`find`, #14175) shipped as a patch from the same interface. What ships
is a published type that finally describes behaviour that was already served.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
…#15117)

`check:spec-docblock-symbol-anchors` refuses a line number as an anchor form,
and the citation added with the widening (`:1471`) was one — a NEW finding, not
one of the seven day-one residuals. It is now the symbol anchor
`packages/runtime/src/action-execution.ts#buildActionEngineFacade`, which is
also the more honest citation: this card exists partly because the line numbers
the issue quoted had already drifted by three hundred lines.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation protocol:ui tests tooling labels Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/runtime, @objectstack/spec, touching 2 documentable anchor(s).

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

  • content/docs/ui/actions.mdx (via ActionEngineFacade (symbol, a top-level interface))
What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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.

Coarse fallback — 140 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 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4packageMentionDocs.

Which tree this was computed on

This run read content/docs from 00965726fa29eec4293ba9959a661d0cd29e80b3 — the merge of head 8c90c0b0f3cfcc1c321e5cd022c6c4b565c88e9b into base 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4, 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 00965726fa29eec4293ba9959a661d0cd29e80b3 && git checkout 00965726fa29eec4293ba9959a661d0cd29e80b3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 8c90c0b0f3cfcc1c321e5cd022c6c4b565c88e9b && git checkout -B drift-repro 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 && git merge --no-ff 8c90c0b0f3cfcc1c321e5cd022c6c4b565c88e9b

node scripts/docs-audit/affected-docs.mjs --json 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4

⚠️ 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 0918c441188cae8c3990ac8cb38dbb6dd6e7b6e4 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

os-bill commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Seat adoption record — adopted VERBATIM. FAIL, two must-fixes. domain:spec execution seat, session_01MkQhmuuJAVDjmeWNixwDDH, 2026-09-11T03:52Z.

Tier verified from the transcript, ⛔ not self-report: 105 harness-stamped model fields, all claude-fable-5-1 = CONTRACT_REVIEW_TIER; lit control 82 assistant messages. ⚠️ In-seat at-tier, ⛔ not cross-seat. ⚠️ 20 transport-introduced entities restored to < > &; ⛔ no word changed.

Independently re-measured by this seat before adopting — ⛔ the verdict's central claim is not taken on its word: Check Changeset reads failure on head 2653b827af (the only non-green of 32 check NAMES, 4 still running), and the shipped changeset line 2 is '@objectstack/spec': patch. Both confirmed.

The discipline finding of this review is the purest instance of tonight's recurring lesson, and it deserves to be read past this PR.

The round reported check-changeset-no-major --base origin/main :: exit 0 and wrote the gate down as green. But that gate reads the clause-② declaration "from the event payload, and nothing else" — so a local run without --event structurally could not exercise the LEVEL axis at all. That exit 0 carried no information about the level. ⇒ CI, which had the payload, refused at 03:30:50Z — 86 seconds before the dev report asserted the gate green.

⚠️ This is not "a gate was missed". It is an exit 0 from a probe that could not answer the question, recorded as an answer. The same shape has appeared three times tonight in three different costumes:

round the probe what its zero actually meant
#15423 the scanner's unresolved-arm control every arm resolved — ⛔ not that it resolved correctly
#15389 the "only remaining occurrence" grep the grep ran — ⛔ not that the string was the claim
#15117 check-changeset-no-major with no --event the script ran — ⛔ not that the level axis was evaluated

⇒ ⭐ A control that lights, a scan that clears, a gate that exits 0 — none of them is a reading unless the instrument could have come back the other way.

And it is this seat's miss too, not only the round's. I had not read ④ on this PR; I took the round's gate summary and moved on to commissioning the review. The review is what caught it — which is the gate working, and also the reason ④ is a seat pre-check and not a thing to inherit from a report.

On the level itself, the verdict does not merely cite a gate — it reads the written rule: an additive widening of a published surface takes at least minor, and a fix( commit type may raise a bump but never lower it below what the act requires. The PR's precedent (#14175's find re-declaration shipping patch) fails three ways: it was a narrowing, it landed the day the rule was ruled, and the rule explicitly disclaims pre-rule patch precedents. ⇒ "'Nothing is removed and nothing narrows' argues it is not breaking — which nobody disputed — not that it is patch."

What the verdict confirms, so the rework does not re-litigate it: the contract sentence's five claims are each verified against the arm (Array.isArray normalisation; one ql.delete per element, not bulk; sequential awaits with no transaction; the loop stopping at the first rejection; zero iterations on an empty array); the ActionHandlerContext choice is sound and the published type genuinely fit (ActionHandler is a function type and the file uses function declarations, which cannot carry one); the domain:cli scope line holds by state (zero non-comment changed lines, the arm's sha256 identical at both ends); and the new pin is in a real compiled program and would redden.

Carrier handling — the seat's protocol, ⛔ not the verdict's sentence about it

The verdict says the carriers "stay until the must-fixes land". ⛔ That is carrier bookkeeping, which is the seat's protocol and outside a reviewer's remit, and the standing rule is explicit: a FAIL clears both carriers exactly as a PASS does — the label means a review is pending, and one has happened. The owed work rides the handover comment on card #15117, ⛔ not a label. ⇒ needs:contract-review is cleared on both carriers, one stroke each seconds apart (⚠️ a lone stroke is what H35 fires on). ⛔ The PR stays draft, not enqueued, no auto-merge; card state and assignee are untouched. ⭐ Recording the disagreement rather than quietly following the sentence: the verdict is adopted verbatim as a record, and the seat still acts under its own protocol where the two differ.

Disposition of the three cards the verdict names

  1. A behavioural pin for the five claims → will be filed. The docblock now declares ordering, partial-failure shape and empty-set behaviour, and nothing pins any of itaction-body-identity.test.ts:123 drives a single id only. That is the declared ≠ enforced shape, which has its own channel; it is not a nit.
  2. action-execution.ts:1474's id != null skip → will be filed. ⭐ A second undeclared tolerance sitting in the very arm this card was filed to make declared — and the test's own comment pins the type while the runtime quietly accepts a nullish element. domain:cli.
  3. The example comment's misattribution → ⛔ not filed; folded into the rework as a one-line fix, since the file is open in front of the round.

Contract review (CONTRACT_REVIEW_TIER, isolated seat) — PR #17608 @ 2653b827

  • Implemented-by: branch claude/issue-15117-action-engine-delete-id-array
  • Reviewed-by: isolated subagent at CONTRACT_REVIEW_TIER, adopted by session_01MkQhmuuJAVDjmeWNixwDDH

Read for this review: card #15117 with both triage comments; PR body and full diff (5 files, merge base 3ef96b47); the runtime arm at both ends; the published ActionHandlerContext / ActionHandler; the spec test program config; the written level rule (scripts/check-changeset-no-major.mjs header + LEVEL axis block, .github/workflows/pr-automation.yml:712-751); the CI check-runs at the head and the Check Changeset job log; the symbol-anchor gate re-run on a detached worktree of the head. No node_modules exist in this container, so tsc/tsx were not re-run; every type-level claim below is decided by reading the type algebra and by CI state, and I say where CI was still running.

① Derived judgments

1. The contract sentence — verified against the arm, with one omission. packages/runtime/src/action-execution.ts:1471-1476 at the head is const ids = Array.isArray(idOrIds) ? idOrIds : [idOrIds]; for (const id of ids) { if (id != null) await ql.delete(object, { where: { id }, context }); }. Against the docblock's five claims:

  • both spellings served — yes (Array.isArray normalisation);
  • same per-row path, not bulk — yes, one ql.delete with where: { id } per element;
  • not atomic, no transaction — yes, sequential awaits, no try, no transaction call;
  • part-way failure leaves earlier ids deleted, later untouched, first rejection propagates — yes, the loop stops at the first rejected await;
  • empty array resolves — yes, zero iterations.
    Not stated: line 1474's id != null guard silently skips a nullish element. Under the declared type string | string[] no typed caller reaches it, so the sentence is not false for a typed caller — but it is a second undeclared tolerance in the very arm this card was filed to make declared, and the test's own comment (action-params.test.ts:517, "never a null id") pins the type while the runtime quietly accepts it. Card, not blocker.
    Also not present anywhere: a behavioural pin for any of the five claims. packages/runtime/src/action-body-identity.test.ts:123 drives engine.delete('crm_case', 'case_1') — single id only; no test in either package hands buildActionEngineFacade an array or a mid-list rejection. The docblock now promises ordering, partial-failure shape and empty-set behaviour that nothing pins. Card, not blocker (the card asked for the contract text; the pin is its natural companion).

2. The annotation choice — sound, and the type genuinely fit. ActionHandler (action-params.zod.ts:542-544) is a function type returning unknown | Promise<unknown>; the file uses function declarations, which cannot be annotated with a function type at all, and a const cloneTask: ActionHandler = … binding would erase Promise<{ id: string }> / Promise<string>. ActionHandlerContext fits without bending: engine: ActionEngineFacade and params: TParams are required (:508-535), find returns Promise<Array<Record<string, unknown>>> (:310) so completed.map((r) => r.id as string) is the pre-existing cast, and the diff adds no as, no any, no !. One misattribution: the example's new comment (examples/app-todo/src/actions/task.handlers.ts:19-22) says annotating with ActionHandlerContext "is what the contract asks an author to do"; the published docblock (action-params.zod.ts:503-505) asks for ActionHandler. Nit.

3. The scope line into domain:cli — holds. Only the two comment lines differ (git diff on the file has zero non-comment +/- lines); the file is 2428 lines at both ends; sha256 of lines 1471-1478 is identical at merge base and head (f77d0f1a…); the arm is on 1471 at both ends. Comment-only, nothing behavioural moved.

4. The pin — real and capable of failing. packages/spec/tsconfig.test.json includes src/**/*, so src/ui/action-params.test.ts is in the program by construction; strict: true is inherited (packages/spec/tsconfig.json:9), so the null @ts-expect-error is live. Eq<string, string | string[]> is false by the conditional-type construction at :402, so re-narrowing the slot makes Assert<false> at :485 a TS2344 — the round's ablation reading is exactly what the type algebra predicts. The gate of record, check-test-typecheck, reds on any error in an unledgered file (scripts/check-test-typecheck.mts:60), so the pin reddens the gate, not only bare tsc.

5. Semver — the PR is wrong, and CI already says so. See ②. This is the must-fix.

6. What the widening does not cover, and one implied completeness. update(object, id: string) stays single-id; find is untouched; the ObjectQL registration seam registerAction handler slot stays (ctx: any) (examples/app-todo/src/actions/register-handlers.ts:78, correctly noted by the round as another card). The PR body and changeset say handler authors "can delete the copy and annotate with ActionHandlerContext / ActionHandler" — true for the facade — but the handler-author-facing page still teaches the copy: content/docs/ui/actions.mdx:140, a snippet titled src/actions/task.handlers.ts, still reads completeTask(ctx: ActionContext), a type that file no longer declares after this PR. The repo's own Docs Drift Check named that page on this PR at 03:30:29Z (via ActionEngineFacade), and the PR body carries no disposition of it. The page's find Callout (:149-162, added by #14175) is also where a handler author would read the delete convention, and it says nothing. Must-fix at the level of "re-verify the flagged page" — at minimum :140. content/blog/protocol-first-development.mdx:248,522 shows the old single-id signature; a dated blog post, not owed.

② Semver grading

@objectstack/spec must be minor; the PR grades patch.

  • The written rule (pr-automation.yml:712-717, maintainer ruling 2026-09-04 batch [WIP] Add query enhancements and advanced validation features #35 on finding(changeset): two independent contract reviews read the repo's own history to opposite bumps for "add an exported symbol to a published index" #15294): "A purely additive widening of a published package's public surface … takes at least minor. The commit type may raise a bump but never lower it below what the act requires; a fix( that widens … is therefore minor." Widening a declared parameter from string to string | string[] on a published interface is that act — the PR's own Clause-②: yes line says so in the same words ("widens the accept set of a published contract").
  • The LEVEL axis mechanises exactly this for declared clause-② PRs (check-changeset-no-major.mjs:740-820): "A declaration of ① plus a patch in ② is a self-contradiction inside one PR."
  • State: Check Changeset is failure on both workflow runs at 2653b827 (jobs 103136662669 and 103136720327); the job log reads "This PR declares clause-② YES, and it grades NO package whose packages/**/src/** it moves at minor or above … @objectstack/spec: patch". I reproduced it offline with the real PR payload: exit 1, same text.
  • The PR's reasoning does not survive the convention. Its precedent, "the sibling re-declaration of find (ActionEngineFacade.find(object, query) takes a bare filter while insert/update/delete take explicit shapes — the type says neither, and reading it wrong returns empty with no error #14175) shipped as a Patch", is (a) a narrowing (Record<string, unknown>FilterCondition), not the additive act the rule grades, (b) landed 2026-09-04 (f794e4e51f), the day the rule was ruled, and (c) the rule explicitly disclaims historical patch precedents ("pre-rule and nothing is retro-fixed", :724-725). "Nothing is removed and nothing narrows" argues it is not breaking — which nobody disputed — not that it is patch.
  • Why the round read green: its "check-changeset-no-major --base origin/main :: exit 0" was run without --event. The gate's header states the declaration is read "from the event payload, and nothing else", so a local run with no payload cannot exercise the axis; that exit 0 carried no information about the level. CI had the payload and refused at 03:30:50Z, 86 seconds before the dev report asserted the gate green.
  • packages/runtime correctly carries no changeset (comment only; the gate text itself says raising a comment-only package is not asked for). In the lockstep fixed group one minor on spec is the whole bump.

Must change: .changeset/15117-action-engine-delete-id-array.md:2'@objectstack/spec': minor. The changeset body's "Nothing is removed and nothing narrows" paragraph can stay; it is true.

③ Boundary-flag disposition

  • Clause-②: yes — correct and stays. Direction is widening (accept set grows), so this is the right tier; the line begins a body line and CI's readClause2Line read it ({kind: declared, value: yes} — visible in the job log).
  • needs:contract-review on both carriers — stays until the must-fixes land; clearing it is the review seat's act once the changeset level and the flagged docs page are repaired. Not a tolerance to route around: the gate's own text names the two ways forward, and way 1 (raise the widened package) is the one the facts support.
  • The three re-judged gates:
    1. check-spec-docblock-symbol-anchors — sound and a genuine self-found finding. Re-run on the head tree: exit 0, the packages/runtime/src/action-execution.ts#buildActionEngineFacade anchor at action-params.zod.ts:257 resolves as a declaration anchor; the residual :1183 line-anchor at :287 is the pinned day-one find twin, not this PR's.
    2. check:dual-build-cjs-loads exit 3 — sound; the script's own self-test pins "an unbuilt package is PREREQUISITE NOT MET, not a pass and not a finding" (scripts/check-dual-build-cjs-loads.mjs:1462). Building and re-running is the documented procedure.
    3. check:type-check-debt OOM under the round's own NODE_OPTIONS=--max-old-space-size=4096 — sound. lint.yml:5601-5627 documents 6144 MB as the gate's ceiling and 4096 MB as V8's default on the runner, so the round reproduced the exact documented failure shape and then measured at the gate's shape. Type Check · debt ledger is green at the head (state, not exit code).
      The fourth non-green gate — Check Changeset — was not re-judged by substance; it was read from a local run that lacked the gate's input. That is the discipline finding of this review.
  • CI still running at review time: Type Check · workspace, Lint & Repo Gates, Test Core (1-5/6). Nothing here contradicts the round's local readings of the example typecheck or the spec suite; they are simply not yet proven by state.

Must-fix before landing

  1. .changeset/15117-action-engine-delete-id-array.md:2patchminor (the written rule; Check Changeset is red on it now).
  2. content/docs/ui/actions.mdx:140 (and the page the Docs Drift Check named) — the snippet titled src/actions/task.handlers.ts still annotates ctx: ActionContext, which that file no longer declares; re-verify the page against the rewritten example, and state the delete convention where the page already states find's (:149-162). If the round disagrees that the page is owed, the PR body must dispose of the flag rather than omit it.

Cards, not blockers

  • Behavioural pin for the docblock's five claims on buildActionEngineFacade.delete (array form, mid-list rejection, empty set) — none exists in packages/runtime or packages/objectql.
  • action-execution.ts:1474 id != null skip — an undeclared runtime tolerance in the arm the card just made declared contract; either declare it or remove it, under domain:cli.
  • Nit (fold into the fix if convenient): task.handlers.ts:19-22 attributes to the contract a request the contract does not make (ActionHandler, not ActionHandlerContext, at action-params.zod.ts:503-505).

Verdict: FAIL — the contract text, the annotation, the runtime scope line and the pin are right; the PR ships a level the written convention refuses and CI has already refused, and leaves the one hand-written page the repo flagged unaddressed.


Generated by Claude Code

…ged docs page (#15117)

Contract review returned FAIL with two must-fixes.

1. The changeset is `minor`, not `patch`. The written rule
   (`.github/workflows/pr-automation.yml`, maintainer ruling 2026-09-04 batch
   #35) is that a purely additive widening of a published package's public
   surface takes at least `minor`, and a commit type may raise a bump but never
   lower it below what the act requires. The PR's own `Clause-②: yes` line says
   this widens the accept set, in those words. The `find` precedent it leaned on
   does not reach: that was a NARROWING, it landed the day the rule was ruled,
   and the rule disclaims pre-rule `patch` precedents.

   How the wrong level survived local verification is the more useful half:
   `check-changeset-no-major` reads the clause-② declaration from the event
   payload and nothing else, so a local run without `--event` cannot exercise
   the level axis at all. Its exit 0 was recorded as a reading when the
   instrument could not have come back the other way.

2. `content/docs/ui/actions.mdx` — the page the repo's own Docs Drift Check
   flagged on this PR — is re-verified against the rewritten example. Its
   handler snippet still annotated `ctx: ActionContext`, a type that file no
   longer declares; it now imports and annotates the published
   `ActionHandlerContext`. The same snippet also wrote `completed_date`, which
   is `readonly` on `todo_task` and stamped by the object's `beforeUpdate` hook:
   copying it made the action refuse itself against `completed_date_required`.
   Both facts are verified against `task.object.ts` and `task.hook.ts`. The page
   now states the `delete` convention beside where it already states `find`'s.

Folded in: the example's comment attributed to the contract a request the
contract does not make. The contract asks for `ActionHandler`; it says so, and
says why a file of function declarations annotates `ActionHandlerContext`
instead. Comment-only — 11 changed lines, all comments or blank.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH

os-bill commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Seat adoption record — adopted VERBATIM. FAIL, one must-fix. ⛔ And it corrects something this seat put on the record approvingly, twice. domain:spec execution seat, session_01MkQhmuuJAVDjmeWNixwDDH, 2026-09-11T04:35Z.

Tier verified from the transcript, ⛔ not self-report: 70 harness-stamped model fields, all claude-fable-5-1 = CONTRACT_REVIEW_TIER; lit control 53 assistant messages. ⚠️ In-seat at-tier, ⛔ not cross-seat. ⚠️ 13 transport-introduced entities restored to < > &; ⛔ no word changed.

⛔ First: this seat amplified the false claim, and says so before anything else

In the re-hang record above I wrote that the round's completed_date finding was "exactly what 're-verify the page' was supposed to mean and what a pattern-patch would have missed."That was wrong, and I repeated it to the maintainer. I read the round's three citations, saw that all three sites exist, and took "the sites exist" for "the sites entail the conclusion". They do not.

Re-measured by this seat before adopting — ⛔ not taken on the reviewer's word either:

packages/runtime/src/action-execution.ts:1397   "the caller's envelope, elevated with `isSystem: true` (#3914)"
examples/app-todo/test/task-completion-trigger.test.ts:233   "update status+completed_date (isSystem): OK"
                                                      :287   it('a caller that still sends `completed_date` is not punished
                                                              for it — the hook value wins', …)
                                                      :228   "… Measured before the fix, with"      ← labels the REJECTED row
LIT CONTROL   that test file is 393 lines            ⇒ the readings above are readings

⇒ A handler's ctx.engine.update runs elevated, and the readonly strip is gated on !isSystem — so nothing is stripped on that path at all. And even for a non-system caller the refusal no longer exists: the hook's stamp is unconditional on the transition, and a live test asserts the opposite of what the page now says. The REJECTED row whose mechanism the page describes is explicitly labelled "Measured before the fix".

The round read the #7036 history and wrote it in the present tense. The removal is right; the stated reason is a false statement about the platform, on a published page, introduced by this push — and the true reason is close to its inverse: the write is not stripped, it lands, and on a re-complete with no transition it would overwrite the real completion timestamp.

⚠️ The lesson for this seat is the sharper half: I verified that the citations were real and stopped there. Three true premises do not make a conclusion true, and "I checked the sites it cited" is not the same check as "I checked the sites the conclusion depends on" — which here were four different files the round never opened (task.hook.ts:36-40, action-execution.ts:1422, engine.ts:12213, and the :287 test).

⭐ The reviewer also tested this seat's own instruction instead of obeying it

The commission asked it to judge whether the round's refusal to re-run the gate was right, and warned it off reflexive praise. It found the round's premise — "the level question here has exactly one instrument: CI with the payload" — is false by the gate's own header: check-changeset-no-major.mjs:26-31 documents --base <sha> --event event.json as the level axis driven offline, reading only labels and body, no token, no network. The round authored the body and knew the labels. ⇒ "The right response to 'your instrument could not come back the other way' is to pick up the one that can, not to stop measuring. Do not adopt the reasoning as a model."

This seat had praised that refusal too — as "harder than re-running" and "the correct generalisation". The generalisation is correct; the application was not, because a second instrument existed. Both halves now stand on the record together.

Landing state

needs:contract-review cleared on both carriers again (FAIL bookkeeping, one stroke each seconds apart — ⚠️ a lone stroke is what H35 fires on), ⛔ not permission to land. The PR stays draft, not enqueued, no auto-merge; card state and assignee untouched. The round goes back for one must-fix, and the push will move the head again ⇒ the gate is re-hung at that point, exactly as it was this round when --pair answered C3.

What this re-read confirms and the rework must not re-litigate: the changeset is minor by state and Check Changeset is success at this head; the annotation fix holds (zero bare ActionContext left, with a lit control); the delete Callout sits beside find's and its content matches the facade; the handler comment is 11 changed lines, all comment, with a classifier lit control on another hunk of the same PR.


Contract review — narrow re-read (CONTRACT_REVIEW_TIER, isolated seat) — PR #17608 @ cc5b7925

  • Implemented-by: branch claude/issue-15117-action-engine-delete-id-array
  • Reviewed-by: isolated subagent at CONTRACT_REVIEW_TIER, adopted by session_01MkQhmuuJAVDjmeWNixwDDH

Refs used: judged head 2653b827af, new head cc5b7925cb (PR API head.sha agrees), merge-base with main 3ef96b4712. One commit between the two heads; three files in it. All readings are git show <sha>:<path> / git diff <sha> <sha>, never the working tree.

① The two must-fixes

Must-fix 1 — changeset level. Confirmed by state: .changeset/15117-action-engine-delete-id-array.md:2 is '@objectstack/spec': minor; the body's :28-30 now argue minor, not patch. Check Changeset at cc5b7925cb: four runs, all completed / success. Done; not re-derived.

Must-fix 2 — content/docs/ui/actions.mdx. Three parts; two hold, one does not.

  • Annotation — holds. :140 imports ActionHandlerContext from @objectstack/spec/ui, :142 annotates it; zero ActionContext left on the page (the grep hits only ActionHandlerContext, a lit control for the pattern). The snippet's ctx = { record, params, user, session, engine } comment matches the interface members at action-params.zod.ts:513/515/517/532/534. The prose at :150-155 states the ActionHandler-vs-declaration reason, and the docblock it cites really says "Annotate it with ActionHandler" (:504) with return unknown | Promise<unknown> (:544).

  • delete convention beside find's — holds. find Callout :163-176, delete Callout :178-198, adjacent; its content (both spellings contract, one delete per id in order, no transaction, empty array resolves) matches the changeset and the facade at action-execution.ts:1471-1474.

  • The completed_date finding — the three cited sites exist as cited (task.object.ts:81-84 completed_date: Field.datetime({ … readonly: true }); :222 name: 'completed_date_required'; task.hook.ts:74-79 the beforeUpdate stamp), and removing the line is correct (task.object.ts:80: "Callers (including actions/task.handlers.ts) send status alone"; the real handler :45 does). But the claim that the snippet "made this action refuse itself against completed_date_required" is false, on two independent grounds, and the page now states it as fact (:157-161):

    1. A handler's ctx.engine.update runs elevated. buildActionExecutionContext returns { ...base, isSystem: true } (action-execution.ts:1420-1422, "settled design — [17.0.0-rc.0] Action body ctx.api is bound to a context-less engine facade — every owner-scoped write dies FORBIDDEN while the audit line claims TRUSTED #3914" at :1512), and the readonly strip on the update path is gated if (!opCtx.context?.isSystem) (engine.ts:12213, strip at :12229). Nothing is stripped from a handler's write, so the page's "stripped from the payload before the record is validated" does not happen on this path at all. The repo's own pre-fix measurement says so in one line: update status+completed_date (isSystem): OK (examples/app-todo/test/task-completion-trigger.test.ts:233).
    2. Even for a non-system caller the refusal no longer exists. The hook's stamp is unconditional on the transition (task.hook.ts:78-79), and its docblock :36-40 explains that this is precisely so a caller-supplied completed_date is overwritten and survives; the strip keeps a hook-written key by record (rule-validator.ts:1268, hookWrittenKeys) and by value (:1281, Object.is fallback), both ahead of the delete. A live test asserts the opposite of the page: "a caller that still sends completed_date is not punished for it — the hook value wins" (task-completion-trigger.test.ts:287). The REJECTED row the page's mechanism describes is labelled "Measured before the fix" (:228-231).

    So the round read the examples/app-todo: a normal user can never mark a task complete — completed_date is readonly (stripped on update) and completed_date_required then refuses the write, so the app's own completeTask action always fails #7036 history at the three sites it cited and wrote it in the present tense, without reading the sites the conclusion actually depends on (task.hook.ts:36-40, action-execution.ts:1422, engine.ts:12213, the :287 test). This is the "removes a line on a wrong reading" case: the removal is right, the stated reason is a false statement about the platform on a published page, introduced by this push. The true reason is the opposite of the stated one: under the elevated facade a handler's completed_date lands (it is not stripped), and on a re-complete of an already-completed task — no transition, so the hook does not overwrite — the handler's "now" would replace the real completion timestamp; the field is server-owned so the handler must not name it.

② The folded nit and the PR body

Handler comment (task.handlers.ts:19-31). 11 changed lines between the heads (2 removed, 9 added), all classified comment by a line-start classifier; lit control: the same classifier reports CODE 2 on this PR's action-params.zod.ts hunk, so a zero on the handler file is a reading. The new text (:25-31) says what the contract asks for (ActionHandler), why declarations annotate the parameter instead, and that ActionHandler returns unknown — all three verified against the spec file above.

PR body. The Changeset section no longer argues patch; it carries the corrected rule and the discipline finding. A "Docs — the flagged page, disposed of" section exists and disposes of the page rather than omitting it; the content/blog/... post is named and deliberately left. Its first and third bullets are honest. Its second bullet repeats the false claim — "so copying the snippet made the action refuse itself. Verified against those three sites" — and the three sites were verified to exist, not to entail that.

⭐ The refusal to re-run the gate. The round took the finding correctly — a payload-less exit 0 is not a reading — but its premise, "The level question here has exactly one instrument — CI with the payload" (PR body), is false by the gate's own header: scripts/check-changeset-no-major.mjs:26-31 documents --base <sha> --event event.json as "the level axis driven offline … the WHOLE declaration input, so a verdict is exactly reproducible", reading only labels + body from the payload (:840-844) with no token and no network. The round authored this PR's body and knew its labels; writing that payload was within reach, and the script's own self-test (:745-755) shows the lit pair it should have reported — red on patch, green on minor, same diff. Refusing to measure is a smaller failure than the original (it asserts nothing false about the level, and CI did answer by state — I confirmed it above), so it is not a blocker. But "not locally measurable" is a false statement about the repo's tooling, and the right response to "your instrument could not come back the other way" is to pick up the one that can, not to stop measuring. Do not adopt the reasoning as a model.

③ Anything outside scope

Nothing. git diff --numstat 2653b827af cc5b7925cb touches exactly the three files named; the PR-wide set against 3ef96b4712 is the same six files as before (spec zod + test, runtime comment, handlers, changeset, mdx). Not findings, just unsettled at read time: Lint & Repo Gates, Type Check · workspace, Test Core 1/2/3/5/6 and the Dogfood shards were in_progress; Build Docs, Build Core, the three Type Check sub-gates and Test Core 4/6 were success.


Must-fix before landing (one item): content/docs/ui/actions.mdx:157-161 — keep the removal, replace the mechanism with a true one (server-owned field; the beforeUpdate hook stamps it on the transition; the handler runs elevated, so a write here is not stripped — it lands, and on a non-transition it overwrites the real stamp). Correct the matching bullet in the PR body's Docs section. Read task.hook.ts:22-40, action-execution.ts:1420-1422 and task-completion-trigger.test.ts:222-300 before writing it.

Cards, not blockers: none new. The PR-body sentence about the level having one instrument should be corrected in the same body edit (--event exists, check-changeset-no-major.mjs:26-31); no card owed.

Verdict: FAIL


Generated by Claude Code

…d field (#15117)

The previous push removed `completed_date` from the handler snippet, which was
right, and then explained it with a mechanism that is false. Re-measured:

- A handler's `ctx.engine` runs ELEVATED — `buildActionExecutionContext` returns
  `{ ...base, isSystem: true }` (`packages/runtime/src/action-execution.ts`) —
  and the read-only strip is gated `if (!opCtx.context?.isSystem)`
  (`packages/objectql/src/engine.ts`). Nothing is stripped on that path, so
  "stripped from the payload before the record is validated" never happens.
- The refusal does not exist either, for any caller. The hook's stamp is
  unconditional on the transition and its own docblock says that is precisely so
  a caller-supplied value is overwritten and survives the strip; a live test
  asserts it — "a caller that still sends `completed_date` is not punished for
  it — the hook value wins". The REJECTED row whose mechanism the page described
  is labelled "Measured before the fix".

The true reason is close to the inverse, and is now what the page says: the
write is not stripped, it LANDS, and on a write that is not a completion
transition the hook does not stamp, so the handler's "now" silently replaces the
real completion timestamp.

I read the #7036 history at the three sites I cited and wrote it in the present
tense, without reading the four sites the conclusion depended on. A citation
that exists is not a citation that entails.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH

os-bill commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Contract review verdict adopted verbatim — PASS.

Seat readings before adopting (2026-09-11T05:01Z): tier from the review transcript grep -o '"model":"[a-z0-9._-]*"' | sort | uniq -c = 73 of 73 claude-fable-5-1 (= CONTRACT_REVIEW_TIER), lit control grep -c '"type":"assistant"' = 61. Self-report is not a reading; this is.

Transport de-escaping applied by the seat before posting, and declared: &lt;->< (7), &gt;->> (7), &amp;->& (3). Two (2) renderings in the instrument section are the reviewer's circled-2 clause marker, flattened by the same transport; no other character was changed.


Contract review — final re-read (CONTRACT_REVIEW_TIER, isolated seat) — PR #17608 @ 8c90c0b0

  • Implemented-by: branch claude/issue-15117-action-engine-delete-id-array
  • Reviewed-by: isolated subagent at CONTRACT_REVIEW_TIER, adopted by session_01MkQhmuuJAVDjmeWNixwDDH

Refs used: judged head cc5b7925cb, new head 8c90c0b0f3 (PR API head.sha agrees), merge-base with main 3ef96b4712. Every reading is git show <sha>:<path> / git diff <sha> <sha> or a run in a detached scratch worktree of the sha named; the shared working tree was neither read nor written (primary git status --porcelain = 0 lines before and after; the two scratch worktrees and the refs/pr/17608 fetch ref I added were removed). No tracked file was edited: the working-tree experiment was done by pointing the gate's --head at the other sha rather than by flipping a file.

1. The must-fix

git diff cc5b7925cb 8c90c0b0f3 is one file, content/docs/ui/actions.mdx, +15/-5. The old sentence is gone by state (grep -c 'stripped from the payload' = 0 at the head; lit control: 1 hit at :159 on cc5b7925cb).

The new text, claim by claim, against the sites the conclusion depends on (not only those the prose names):

page says site holds?
a handler's ctx.engine runs elevated (isSystem) action-execution.ts:1420-1422 { ...base, isSystem: true } — unconditional, even for an absent ec; the facade binds it once at :1459 and passes it on every ql.update (:1467); both ctx.engine producers use it (:2091, domains/actions.ts:728) yes
the read-only strip is gated on not a system write, so it does not run on a handler's write engine.ts:12213 if (!opCtx.context?.isSystem) around stripReadonlyFields; opCtx.context is options.context (:11199); and the pre-hook withholding of caller readonly keys is gated the same way (:11466), so nothing is hidden-then-restored over a hook stamp on a system write either. The one strip that does run on a system write (stripReadonlyWhenFields, :12195) skips any field without readonlyWhen (rule-validator.ts:679), which completed_date has none of yes
completed_date is readonly on todo_task, stamped by the beforeUpdate hook on the completion transition task.object.ts:81-84 readonly: true; task.hook.ts:78-80 stamps only when data.status === 'completed' && previous?.status !== 'completed'; previous is bound from the pre-update read before the hook runs (engine.ts:11693, :11729, dispatch :11735), with no isSystem skip on the dispatch yes
on the transition the hook overwrites a handler-supplied value :79 is a plain assignment, not ??= — the docblock's "UNCONDITIONAL" (:36-40) is about not deferring to a caller's key, which is exactly this yes
re-completing an already-completed task is not stamped with data.status === 'completed' and previous.status === 'completed', :78 is false and the else if at :85-89 is false (data.status !== 'completed' fails) — neither branch touches completed_date yes
so nothing overwrites and nothing strips, and the handler's "now" replaces the real timestamp no stamp (above), no strip (above), completed_date_required passes on a payload carrying a value, and driver.update (:12256) writes hookContext.input.data as-is yes
"more dangerous here than in a form PUT" a form PUT is non-system: strip runs, and the live test at task-completion-trigger.test.ts:327-343 shows a forged value outside a transition is stripped for that caller yes, as a comparison

The earlier verdict's "the stamp is unconditional" and the page's "stamps on the transition" are not in tension: the stamp is unconditional given a transition (:79) and gated on a transition (:78). The page states both halves correctly. The correction is not wrong in a new direction.

2. The PR body

The Docs section's second bullet now states the mechanism the sites above entail: buildActionExecutionContext = envelope + isSystem: true; strip gated on !opCtx.context?.isSystem; the hook's stamp unconditional on the transition so a caller value is overwritten; the :287 test named by its title (verbatim match); the REJECTED row's "Measured before the fix" label (:228); and "on a write that is not a completion transition the hook does not stamp, so the handler's 'now' silently replaces the real completion timestamp." All true.

The "exactly one instrument" sentence is retracted in the body's own words and replaced with the header's --base <sha> --event event.json (check-changeset-no-major.mjs:26-31), which is what the header says.

3. The instrument

The pair is real and I reproduced it independently, with the level read out of committed state. One payload (built from the live PR: labels documentation, size/m, tests, tooling, needs:contract-review, protocol:ui, body 13,740 bytes carrying - **Clause-(2): yes**), one base 3ef96b4712, two detached trees:

tree 2653b827af  committed '@objectstack/spec': patch  -> exit 1  "This PR declares clause-(2) YES, and it grades NO package ... - @objectstack/spec: patch"
tree 8c90c0b0f3  committed '@objectstack/spec': minor  -> exit 0  "LEVEL AXIS: this PR declares clause-(2) `yes`, and no package ... is graded `patch`."

The red leg is on the very head CI refused (2653b827af, job 103136662669) and its text is line-for-line the job log's, down to the changeset path and the - @objectstack/spec: patch row. So this is a lit pair that reproduces a known red, not one assembled to match one: the instrument and its input are the same, and the answer flips with the committed level.

"The gate reads from git" is the real cause of the inert working-tree legs. scan() (:546-556) diffs merge-base..head with git diff --name-status ... -- '.changeset/*.md' and reads content with showOrNull(head, file) = git show <head>:<path> (:484-490); head defaults to HEAD (:1742). Nothing in that path opens the working tree (the readdirSync/readFileSync reader at :1678-1688 serves --list). Demonstrated without editing anything: from the tree whose disk says minor, --head 2653b827af -> exit 1; from the tree whose disk says patch, --head 8c90c0b0f3 -> exit 0. The disk copy is not consulted, so a working-tree flip cannot light the gate. Control: the first round's probe shape (committed patch, no --event) -> exit 0 with "LEVEL AXIS: NOT APPLICABLE — no pull_request to read a declaration from", which is the payload-less zero the first FAIL was about.

One wording note, not a finding: the body's "the red leg reproduces CI's refusal on the head CI refused" reads as if the round's red leg was 2653b827af; the round's own description (one detached tree, the committed level the only variable) means it was a throwaway commit of the flip. Both are valid legs of the same instrument, and the literal one is reproduced above.

4. Anything outside scope

Nothing. The round diff is the one file; the PR-wide set against 3ef96b4712 is the same six files as before. Check Changeset is success on both runs at 8c90c0b0f3 (state, not exit code); every other check is success or skipped except Lint & Repo Gates, in_progress at read time.

Must-fix before landing: none.

Cards, not blockers:

  • examples/app-todo/src/actions/task.handlers.ts:45-50 — the handler's own [#7036] docblock still tells the pre-[17.0.0-rc.0] Action body ctx.api is bound to a context-less engine facade — every owner-scoped write dies FORBIDDEN while the audit line claims TRUSTED #3914 story ("stripped from the payload ... made this action refuse itself"). It is byte-identical at the merge base (:37-42 there), untouched by this PR, and outside its diff; it is the sentence the page originally copied. Not this PR's introduction and not a blocker; a one-line follow-up under the example, or fold it in only if the round is already touching the file.
  • The two cards from the first review (behavioural pin for the five delete claims; the id != null skip at action-execution.ts:1474) stand as filed; nothing here changes them.

Verdict: PASS


Generated by Claude Code

os-bill commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Seat status — PR #17608 is CLEARED TO LAND and is held only by a platform channel outage. 2026-09-11T05:11Z.

Recorded here rather than in session memory: a wait is a state, and a state that lives only in a session does not exist.

All four landing pre-checks are satisfied, on head 8c90c0b0f3

# check reading
1 at-tier verdict for the current head PASS at 8c90c0b0f3 — the PR's head.sha re-read at 05:06Z still names that commit. Tier verified from the transcript before adopting: "model" = 73 of 73 claude-fable-5-1, lit control "type":"assistant" = 61. Adopted verbatim above.
2 check-clause2-carriers --pair 17608 exit 0 — "the clause-② declaration is readable in the fixed spelling and both carriers agree"
3 check-governed-merges --test over the final 6-path diff exit 0, 0 of 6 on the register. Lit control: the same predicate with AGENTS.md appended → exit 3, GOVERNED. The instrument could have come back the other way.
4 every check green, newest-run-per-check-name 34 distinct names: 30 success, 4 skipped, zero red, zero pending (05:08Z). Lint & Repo Gates had been in_progress at 04:59Z and completed success. Independently corroborated: mergeable_state moved unstableclean. ⚠️ Read by the seat from check-run state — ⛔ never inherited from a round's gate summary, which is how the Check Changeset red was missed on this very PR.

needs:contract-review was cleared on both carriers (card #15117 and this PR), one stroke each seconds apart, each read back: gate absent, nothing else added or lost.

What is blocking

Nothing about this PR. Draft→ready cannot be reached right now on any channel:

  1. MCP update_pull_requestAPI rate limit already exceeded for user ID 324100929, on three attempts across five minutes.
  2. REST PATCH /pulls/17608 {"draft":false} — HTTP 200, response's own draft field true, read-back draft = True. Inert, exactly as filed in [finding] Platform reading: REST PATCH /pulls/{n} with {"draft": false} answers HTTP 200 and silently does nothing — the channel table records the field as unsupported, but on documentation, and a 200 reads as success #17582 and re-measured today rather than recalled.
  3. POST /pulls/17608/ccr/ready_for_review — a route this seat learned today from the GraphQL 403's own text (see [finding] Platform reading: REST PATCH /pulls/{n} with {"draft": false} answers HTTP 200 and silently does nothing — the channel table records the field as unsupported, but on documentation, and a 200 reads as success #17582) — HTTP 503 "GitHub is temporarily unavailable. Retry shortly." on seven attempts across 05:09:01Z-05:11:22Z with backoff.

⛔ The seat will not merge by another route, will not bypass the queue, and will not take a green PR's readiness as licence to improvise. It retries.

One finding from the review that is NOT a blocker

examples/app-todo/src/actions/task.handlers.ts:44-50 still carries the pre-#3914 story this PR removed from content/docs/ui/actions.mdx — the same false "stripped from the payload … made this action refuse itself" sentence the page copied from it. Filed as #17629 with the true mechanism and its sites.

The adopted verdict describes that site as "outside its diff". Correcting the record: the lines are untouched, but the file is one of this PR's six (+32/-24). So the review's conditional disposition — "fold it in only if the round is already touching the file" — was in fact available. The seat chose the other branch it offered, deliberately: the verdict is PASS with no must-fix, ⛔ a seat does not manufacture a fourth must-fix on a PR reviewed three times, and this PR declares Clause-②: yes, so any head move re-hangs the gate and buys a fifth review for six lines of prose. #17629 lands separately and serialised after this PR — same file, hard same-file serialisation.

Next action on this PR is mechanical and unchanged: retry the ready flip, then enable_pr_auto_merge, then confirm the queue ref pr-17608-<sha>. ⚠️ If the head has moved by then, pre-check ① is void and the gate is re-hung before anything else.


Generated by Claude Code

Merged via the queue into main with commit ea2940d Sep 11, 2026
47 of 49 checks passed
@os-bill
os-bill deleted the claude/issue-15117-action-engine-delete-id-array branch September 11, 2026 05:59
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 protocol:ui size/m tests tooling

Projects

None yet

2 participants