Skip to content

fix(plugin-approvals): a revise-window recall on a batch item finds its batch run past the first 1000 parked runs - #28268

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-k86l1s
Oct 30, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-k86l1s

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Elon · project thread

domain:server#4 · os-elon-musk · session_016WV4d1FQJt7HxgWZZ5cYz9

Fixes #28230

Clause-②: no (a recall on a store with more than 1000 parked runs now wakes its batch, as it already does below that; no schema, accept set, error code or result field changes)
Metadata-protocol: none

维护者速读

  • 改了什么:批量签批里,修改窗口内撤回某一项时,撤回改为在 cancelRun 之前按该项 run 自身的 $parentRunId / $parentMapNode 键读出它上面的批次 run(新增引擎成员 AutomationEngine.suspendedMapParentOf),不再扫 listSuspendedRunsDurable() 的 1000 行首页。
  • 为什么改:实测 main(f40b777d):两个副本共用一个 ObjectStoreSuspendedRunStore,批次之前已挂起 1000 条别的 run,在另一副本上撤回:该项被取消,批次 run 一直停在这一项上,第 2 项不开,resumeError 为空,什么都不说;998 条时一切正常。
  • 风险与回滚:只换了“找批次”这一步;找到之后的恢复、只认内置 map 节点的检查、失败的告知方式都没变。新成员只读,两次按 id 读,存储读不出时抛错并照旧写进 resumeError。回滚即 revert 本 PR。
  • 先看:packages/services/service-automation/src/engine.ts 的 suspendedMapParentOf,与 approval-service.ts 里 recall 修改窗口分支和新私有方法 batchAboveItem。
  • 两分钟验证:pnpm --filter @objectstack/plugin-approvals exec vitest run src/batch-item-recall-resumes-batch.test.ts(预期 9 passed,需先 build @objectstack/service-automation)与 pnpm --filter @objectstack/service-automation exec vitest run src/suspended-map-parent-of.test.ts(预期 5 passed)。
  • 需要你做的:无,席位在 CI 绿后按落地授权落地。

What changes

  • packages/services/service-automation/src/engine.ts: one new public read, suspendedMapParentOf(itemRunId). It loads the item run's suspension by id (the strict, store-authoritative loader hasSuspendedRun uses), reads the $parentRunId / $parentMapNode the map node launched it with, loads the parent's suspension by that id, and answers { runId, flowName, nodeId, correlation } only while the parent is parked on map:<itemRunId> at the node the item names. Otherwise undefined. Two keyed reads, never a listing. It rejects when the store cannot be read, as hasSuspendedRun does. No other engine hunk; run-resume.ts and the store are untouched.
  • packages/plugins/plugin-approvals/src/approval-service.ts. ApprovalResumeSurface declares the member (optional). In recall's revise-window branch the new private batchAboveItem(runId) reads it BEFORE cancelRun, because the cancel removes the item's suspension and the link with it. It never throws; a failed read is carried to resumeBatchOfRecalledItem, which now takes the parent it read instead of scanning listSuspendedRunsDurable(), and tells a failed lookup in resumeError after the cancel, as before. The built-in map check through getFlow and everything after it are unchanged.
  • Tests. suspended-map-parent-of.test.ts (new, service-automation) pins the engine member; batch-item-recall-resumes-batch.test.ts (plugin-approvals) gains a #28230 block over two replicas and one ObjectStoreSuspendedRunStore. Its existing wiring moved into a wireReplica helper; fix(plugin-approvals): a revise-window recall on a batch item resumes the batch #28228's five cells are unchanged and green. No new engine double: the block reuses the file's already-pinned one, so scripts/engine-double-contract.pinned.json is untouched.
  • .changeset/28230-batch-recall-past-first-page.md. @objectstack/plugin-approvals and @objectstack/service-automation, patch.

Why

Before (measured at origin/main f40b777d)

Replicas a and b share one ObjectStoreSuspendedRunStore and one table set. N paused rows of another flow are stored first. The batch (map over two task_signoff items) is launched on a, item 1 is sent back on a, and the submitter recalls item 1 on b.

N = 1000, before N = 998, before N = 1000, after
batch on b.listSuspendedRunsDurable() no yes no (no longer read)
item 1's run cancelled cancelled cancelled
item 2's request none: the batch stays parked on item 1 pending pending
recall answer resumed: false, no resumeError resumed: false, no resumeError resumed: false, no resumeError
after item 2 is approved (never reached) marker in slot 1, count 1 marker in slot 1, count 1, batch completed

The listing reads one 1000-row page of paused rows in driver order, and a process adds only what it parked itself. So the batch is missed exactly when another process (a replica, or this one before a restart) parked it behind more than 1000 runs.

Why this path

Triage 6531219559 / 6532374421 ruled 「a correlation-keyed lookup instead of a full listing」. The item run already carries its batch: map-node.ts launches every item with $parentRunId and $parentMapNode. Reading that before the cancel needs no store change, no schema change and no new code. The parked-on-this-item check guards one real shape: a subflow inside a map item inherits the item's $parentMapNode through the spread context, so its $parentRunId names the item run, which is parked at a subflow node, not the map (pinned).

Tests

suspended-map-parent-of.test.ts (5 cells): a map item answers its batch, also from a second engine over the same store with nothing in memory; CONTROL a subflow child, a subflow nested in a map item, an item whose batch has moved on, a top-level run and an unknown id answer undefined.

batch-item-recall-resumes-batch.test.ts, #28230 block (4 cells):

  1. the batch past the first page resumes, item 1's slot holds fix(service-automation)!: a map whose item run was cancelled records the item as cancelled on a parent-id resume #28176's cancelled marker, count 1, the batch completes (red on main: no request opens for item 2);
  2. CONTROL: a batch within the first page behaves as before;
  3. CONTROL: a recall with no batch above it, past the first page, behaves as before, and the batch beside it stays parked on its item;
  4. CONTROL: a parent the engine answers (a home-grown fan-out that writes the same linkage) but parked at a node other than the built-in map is left alone.

Ablations through scripts/ablation-replace.mjs, each restored to the HEAD blob with git diff HEAD empty:

Ablation Red
A1: batchAboveItem answers no parent #28183 cell 1, #28230 cells 1 and 2
A2: the batch is read after cancelRun #28183 cell 1, #28230 cells 1 and 2
A3: drop the built-in map node check #28230 cell 4
A4: engine member drops the parked-on-this-item check engine cells 2, 3, 4

(A5, dropping the typeof parentRunId check, stayed green: a missing id loads no suspension, so it answers undefined either way. It is the type narrowing the next read needs, not a guard.)

Verification

Head 776907f9, after a whole-tree turbo run build, pnpm install --frozen-lockfile and fetching the teardown self-test's pinned commit.

  • pnpm --filter @objectstack/service-automation run test: 234 files, 2952 passed. pnpm --filter @objectstack/plugin-approvals run test: 106 files, 1361 passed. Both packages' typecheck: exit 0.
  • dispatch-gates --commands: 64 families, all exit 0 (check:dual-build-cjs-loads and check:i18n exited 3 before the whole-tree build and 0 after it). --ran: 64 derived, 64 run, 0 NOT-MEASURED, 0 UNRUN. check:engine-double-contract and check:objectql-double-limit are green with no ledger edit.
  • pnpm lint: exit 0. check-commit-card-trailers --range origin/main..HEAD: exit 0.

Out of scope

  • None filed. Probed while pinning: a subflow inside a map item inherits the item's $parentMapNode, but on main the nested chain still completes into the item run and the batch moves on, so it is not a defect. The engine member's parked-on-this-item check keeps it from answering that shape.
  • listSuspendedRunsDurable still reads one 1000-row page for its other caller, the restored-continuation proof in approval-service.ts, which refuses on an absent entry (a retry, not a silent strand). Not this card's.

🤖 Generated with Claude Code

https://claude.ai/code/session_016WV4d1FQJt7HxgWZZ5cYz9

…ts batch run past the first 1000 parked runs

The recall looked for the batch above the item it cancels in
listSuspendedRunsDurable(), which reads one 1000-row page of a
database-backed store plus what this process parked itself. A batch
another process parked behind more than 1000 runs was missed and stayed
parked on the cancelled item.

AutomationEngine.suspendedMapParentOf(itemRunId) now answers the batch
from the item's own $parentRunId / $parentMapNode with two keyed reads,
and only while the parent is parked on map:<itemRunId> at that node. The
recall reads it before cancelRun, which removes the item's suspension.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/service-automation, touching 9 documentable anchor(s).

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

  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class))

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

  • content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class), AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v18/18-1.mdx (via ApprovalService (symbol, a top-level class))

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

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 59 of 211 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 11 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 744ad0ad3afbc29fd0bfee9e8a8e3b7ac3958aec → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 087d6ed9ac43be29c3109d38424027b1e315234b — the merge of head 776907f9fc296bcd492f03c87531fc544fedc671 into base 744ad0ad3afbc29fd0bfee9e8a8e3b7ac3958aec, 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 087d6ed9ac43be29c3109d38424027b1e315234b && git checkout 087d6ed9ac43be29c3109d38424027b1e315234b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 744ad0ad3afbc29fd0bfee9e8a8e3b7ac3958aec 776907f9fc296bcd492f03c87531fc544fedc671 && git checkout -B drift-repro 744ad0ad3afbc29fd0bfee9e8a8e3b7ac3958aec && git merge --no-ff 776907f9fc296bcd492f03c87531fc544fedc671

node scripts/docs-audit/affected-docs.mjs --json 744ad0ad3afbc29fd0bfee9e8a8e3b7ac3958aec

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#4 · os-elon-musk · session_016WV4d1Np3hU7RQnA13YZcL

维护者速读

  • 改了什么:批量签批里,修改窗口内撤回某一项时,撤回改为在 cancelRun 之前按该项 run 自身的 $parentRunId / $parentMapNode 键读出批次 run(新增只读引擎成员 AutomationEngine.suspendedMapParentOf,两次按 id 读),不再扫 listSuspendedRunsDurable() 的 1000 行首页。
  • 为什么改:实测 main(f40b777d):两个副本共用一个持久化挂起存储、批次之前已挂起 1000 条别的 run,在另一副本撤回后批次一直停住且不告知;998 条时正常。这是 plugin-approvals: a revise-window recall on a batch sign-off cancels the map item's run and never resumes the parent, so the batch stays parked #28183 修过的同一种“批次卡死”,只是在繁忙部署上。
  • 风险与回滚:Clause-②: no;只换“找批次”这一步,找到后的恢复、只认内置 map 节点的检查、失败告知方式不变;新成员不在 api-surface,不动 spec。回滚即 revert 本 PR。
  • 先看:packages/services/service-automation/src/engine.ts 的 suspendedMapParentOf,approval-service.ts 的 batchAboveItem。
  • 两分钟验证:pnpm --filter @objectstack/service-automation exec vitest run src/suspended-map-parent-of.test.ts(5 passed)与 plugin-approvals 的 src/batch-item-recall-resumes-batch.test.ts(9 passed,需先 build service-automation)。
  • 需要你做的:无(非受管;不欠契约复审;席位按本任落地授权落地)。

席位意见:ACCEPT。

席位核对

Head-sha: 776907f9fc296bcd492f03c87531fc544fedc671

  • Shape: 5 files, +452 / −31.

    check-governed-merges --pr 28268: NOT governed (0 of 5 paths). The member is not on the api-surface, and packages/spec is untouched.

  • Premise measured: on main (f40b777d), a store with 1000 parked rows ahead of the batch, with the recall on a second replica, leaves the batch parked silently. With 998 rows ahead it resumes. After the change the batch beyond the first page resumes, with fix(service-automation)!: a map whose item run was cancelled records the item as cancelled on a parent-id resume #28176's cancelled marker in the slot. The pins cover: a batch within the page unchanged, a non-batch recall unchanged, and a parent not parked on this item's map left alone.

    • Ablations each turned their pins red and were restored.
    • service-automation: 2952 passed. plugin-approvals: 1361 passed. Both typechecks exit 0.
    • dispatch-gates --ran: 64 derived, 64 run, 0 NOT-MEASURED. pnpm lint: 0.
  • Gate check-runs on the head: 48 runs, 35 success, 13 skipped, 0 failure/pending; mergeable_state clean.

  • Contract review: not owed. The paths decide no face, the changeset's Clause-②: no puts it on the declaration face at TIER_DEFAULT, and no public contract member is added.

  • Docs drift advisory: it names flows.mdx. The dev re-read its batch-recall sentence (from fix(plugin-approvals): a revise-window recall on a batch item resumes the batch #28228), and it stays accurate.

  • Noted, not filed by the dev: listSuspendedRunsDurable still reads one 1000-row page for its other caller, the restored-continuation proof in approval-service.ts, which refuses on an absent entry. The dev reads that as a retryable refusal rather than a silent strand. The seat notes that past the first page a retry meets the same page, so it can keep refusing. The seat will file it as its own finding after landing.

  • Landing: under the maintainer's landing grant for this seat term (seat 4 thread, 2026-10-29 22:04Z): pr_ready + automerge_enable through the relay.

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-approvals: past the 1000-row suspended-run page, a revise-window recall on a batch item cannot find its batch run and leaves it parked

1 participant