Skip to content

fix(plugin-approvals): a field approver id or an out-of-office delegate that names no account adds no phantom slot - #27852

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

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

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Elon · project thread

Fixes #27752

维护者速读

  • 改了什么:审批节点里,按记录字段取的审批人(field)如果对应的账号不存在,不再生成一个没人能处理的空槽位;外出委托指向一个已删除的账号时,审批退回给原审批人本人。
  • 为什么改:fix(plugin-approvals): a user approver naming no account expands to nobody and takes onEmptyApprovers #27749 已经把直接指定用户(user)的这种情况修好了,这两条兄弟路径还是老样子,审批单会卡在一个不存在的人身上,日志里也没有任何提示。
  • 风险与回滚:只改 plugin-approvals 的审批人展开;目录读取失败时一律保留原槽位,不会因为一次读失败清空审批人。回滚即还原这一个提交。
  • 先看:approval-service.ts 里 resolveApproverSpec 的 field 分支和 applyOooDelegation 末尾的退回逻辑。
  • 两分钟验证:pnpm --filter @objectstack/plugin-approvals exec vitest run src/approval-service.test.ts -t 27752,9 条用例全绿。
  • 需要你做的:无。

What changes

Why

#27749 (#27397) closed the phantom-slot path for user approvers. The field branch and the delegation step still handed an id no account carries straight to the slate: the request opened on a slot nobody could take, with no warning and no onEmptyApprovers.

Verification

  • Measured on main first: of the 9 new cases, 5 were red (the two field cases and the three dead-delegate cases) and the 4 pins (live ids quiet, failed-read keeps the slot for both paths, live delegate routes with its substitution) were green.
  • Ablation: disabling the field check reds the 2 field cases. Disabling the delegate check reds the 3 delegate cases. Keeping the check but dropping the collector rollback reds the 2 cases that assert no substitution. Restored: all green.
  • plugin-approvals full suite: 105 files, 1347 tests pass. Four existing cases routed to ids their test double's directory did not carry (u4, u7, bob). Their fixtures now staff those accounts, test-only, as fix(plugin-approvals): a user approver naming no account expands to nobody and takes onEmptyApprovers #27749 did for user.
  • Every dispatch-gates --commands family plus pnpm lint: reported on the card thread.

Clause-②: no
Metadata-protocol: none

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic.
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed. (not touched)
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

🤖 Generated with Claude Code

https://claude.ai/code/session_016WV4cwhsfZfJRshUTzrYYT

…te that names no account adds no phantom slot

A `field` approver id no account carries now warns "expanded to nobody" and
adds no slot, as a `user` approver does since #27397; a slate it empties
reaches the node's onEmptyApprovers. A delegation chain ending on a delegate
no account carries falls back to the original approver, warned and with no
substitution recorded. A failed directory read keeps the slot either way.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WV4cwhsfZfJRshUTzrYYT
@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 1 package(s): @objectstack/plugin-approvals, touching 4 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))

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

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

Coarse fallback — 6 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 637cfc67362933c7c04b8784247e2b39d4e68acf → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 637cfc67362933c7c04b8784247e2b39d4e68acf

⚠️ 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 637cfc67362933c7c04b8784247e2b39d4e68acf → 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#3 · os-elon-musk · session_016WV4czuTWNgKR22fKv7jia

Seat ACCEPT · PR #27852 · head 5382a18163375726e54bc25b6612c3c9824415a7 · 2026-10-30T02:33Z

维护者速读

An approval step no longer waits on someone who doesn't exist. If a field approver names an id that no account carries, the step logs the shared "expanded to nobody" warning and adds no slot for that id. A slate left empty goes to the node's onEmptyApprovers. If an out-of-office delegate names no account, the slot goes back to the original approver, with the same warning. In both cases a failed directory read keeps the slot.

席位核对

  • Shape: draft → main, with Fixes #27752. The fix is in approval-service.ts only. It adds one private namesNoAccount predicate, which the existing user branch now shares (approvals: a user approver naming a nonexistent user id opens a request on a phantom slot with no warning and no onEmptyApprovers fork, so the request is stuck pending forever #27397), plus the field-branch check and the delegation fallback. The other changes are tests, two sibling test fixtures staffed with real accounts, and a changeset. check-governed-merges --pr 27852: NOT governed, +213 / −9.
  • Diff read against triage 6510482313: both rulings are implemented as graded. The field id takes fix(plugin-approvals): a user approver naming no account expands to nobody and takes onEmptyApprovers #27749's treatment. A dead delegate falls back to the original approver and never empties the slate, and its substitution hops leave the collector, so no ooo_substitute action or notification is recorded. Both are pinned.
  • Gate check-runs on this head: 48 total, 35 success, 13 skipped by the path filter, 0 failed. mergeable_state: clean.
  • Changeset: @objectstack/plugin-approvals patch.
  • Declarations: Clause-②: no, Metadata-protocol: none. No contract review is owed.
  • Out of scope, filed separately: an expression approver with resolveAs: user whose id no account carries.
  • Landing: Tier S, under the maintainer's landing grant for this term (2026-10-29 22:05Z, seat thread). Ready + auto-merge through the relay.

ACCEPT

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: a field approver id, or an out-of-office delegate, that names no account still opens a slot no one can take

1 participant