Skip to content

fix(plugin-approvals): the approval request Timeline shows each step by its label, not the node id - #25526

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-25501-approval-timeline-step-label
Oct 23, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/issue-25501-approval-timeline-step-label

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Sales · project thread

Fixes #25501

Clause-②: no
Metadata-protocol: none

What was wrong

The sys_approval_request record page (packages/plugins/plugin-approvals/src/sys-approval-request.page.ts) has a Timeline tab. That tab is a record:related_list over sys_approval_action, and its columns asked for step_name. Every action row stores two step fields, both declared on sys-approval-action.object.ts:

  • step_name: "Machine name of the step at the time of the action". Every writer in approval-service.ts sets it to the Approval node id (step_name: nodeId, or raw.flow_node_id ?? raw.current_step).
  • step_label: "Label of the step at the time of the action". ApprovalService.actionRow writes it on every row from the request's current_step_label, and display-label-backfill.ts fills it on older rows.

So every Timeline row showed the node id (dual_signoff), while the highlights strip right above it showed the label (current_step_label). The object's own list views (recent, by_actor, all_actions) already use step_label.

The card's premise holds with one correction: step_name is not missing. It is a declared, stored field that holds the machine id, so the column named a real field. It was the wrong one of the two.

The change

  • The Timeline column step_name is now step_label. Nothing else on the page changes, and the page's doc comment says why the step column is step_label.
  • Landing: the file the claim names. packages/spec is not touched.
  • I checked every other field the page names against its object. The Timeline columns created_at, action, actor_id, comment and attachments and its sort field created_at are all sys_approval_action fields. The highlights strip's process_label, status, current_step_label, submitter_id and updated_at are all sys_approval_request fields, and none of them is a machine id where a label exists.

Tests

src/sys-approval-request-page.test.ts gains names each Timeline step by its label, step_label, never by the node's machine id, step_name. It reads the page's Timeline columns and asserts two things: step_label is listed and step_name is not, and step_label is the declared Approval Step text field. A control asserts that step_name stays a declared field.

The file's existing test every field the page names exists on its object already checks that each Timeline column names a field of sys_approval_action. It stayed green on the defect, because step_name is a declared field. That is why this test pins the choice, the way the strip's current_step_label is already pinned.

Ablation, at head 4e80cfeeaf, under the verify lock. node scripts/ablation-replace.mjs --file packages/plugins/plugin-approvals/src/sys-approval-request.page.ts --anchor "'actor_id', 'step_label', 'comment'" --replacement "'actor_id', 'step_name', 'comment'" -- pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2 src/sys-approval-request-page.test.ts:

  • mutation landed: anchor 1 -> 0, blob 1b31bf215cb3 -> 48f41780fd75
  • × names each Timeline step by its label, step_label, never by the node's machine id, step_name and Tests 1 failed | 19 passed (20). Only the new test went red, and the existence test stayed green, as described above.
  • restored: blob == HEAD (1b31bf215cb3) and git diff HEAD is empty

The test imports the page from src through a relative import, so the ablation needs no dist/ rebuild.

Local verification (head 4e80cfeeaf)

  • pnpm --filter '@objectstack/plugin-approvals^...' build (dependency closure): VERDICT command-exit 0.
  • pnpm --filter @objectstack/plugin-approvals test: Test Files 95 passed (95), Tests 1234 passed (1234).
  • pnpm --filter @objectstack/plugin-approvals typecheck: exit 0, and check:test-typecheck: OK.
  • node scripts/pm/dispatch-gates.mjs --commands derived 69 commands. 68 exit 0. pnpm check:i18n exited 3 at first because its prerequisite build was missing; after its named build closure it gave check-i18n-bundles: OK.
  • pnpm check:dual-build-cjs-loads exited 3: PREREQUISITE NOT MET, because it needs a whole-workspace pnpm build. NOT MEASURED locally; CI runs it.
  • --ran: 69 derived famil(ies) accounted for — 68 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3).

Acceptance notes

  • The sys_approval_action object itself still names the step by step_name in three places that this card does not cover:
    • its titleFormat ('{action} · {step_name}');
    • its display_title formula, the record title (nameField);
    • its highlightFields.
      So an action record's own title and strip read for example approve · dual_signoff. This is not filed: it is a separate surface (the action record's own page and title, not the request page's Timeline), and changing a formula's record title has a wider reach than one column. Carrier: none.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_016g7nYP9wdNx1GDa5XDr7Uy

…ode id

The sys_approval_request page's Timeline related list asked for
`step_name` (the Approval node's machine id, such as `dual_signoff`)
where every sys_approval_action row also stores `step_label`, the step's
label. Swap the column, and pin the choice: the existing field-existence
check cannot tell the two apart, since both are declared fields.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 3 documentable anchor(s).

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

  • content/docs/automation/approvals.mdx (via actor_id (literal, a string literal in slots))

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

  • content/docs/releases/v17/17-7.mdx (via actor_id (literal, a string literal in slots))

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 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 38 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 82f5a111b464cd9bfe5999d8bc0befbfdf0708b7 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 82f5a111b464cd9bfe5999d8bc0befbfdf0708b7

⚠️ 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 82f5a111b464cd9bfe5999d8bc0befbfdf0708b7 → 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

席位核对 · ACCEPT · 2026-10-23T19:46Z

domain:services seat 1 (seat post #6021) · os-sales · session_016g7nYP9wdNx1GDa5XDr7Uy · card #25501 (S, in-seat subagent; claim on the card; os-dev-report 6373339616) · reviewed head 4e80cfeeaf

  • Diff: 3 files, +32 / −2.
    • The Timeline record:related_list on the sys_approval_request page (sys-approval-request.page.ts:126) now asks for step_label where it asked for step_name.
    • The page docblock says why: step_name is the Approval node's machine id (for example dual_signoff), and the strip's step chip is already current_step_label.
  • Seat check:
    • Premise. It holds, with one correction the report states: step_name is not absent, it is the machine id.
    • Data. step_label is declared on sys_approval_action. ApprovalService.actionRow writes it on every row, and display-label-backfill.ts backfills it.
    • Scope. The one-word column swap is the whole fix, and packages/spec is untouched.
  • Pins (report):
    • A new test asserts that the Timeline names step_label, never step_name, and that step_label is the declared "Approval Step" text field. It keeps step_name declared as a control.
    • The existing "every field the page names exists" test could not pin this, because step_name is declared. The report says so.
    • Ablation (step_label → step_name) turns only the new test red, and the restore is clean.
    • plugin-approvals: 1234 / 1234, and typecheck exits 0.
    • dispatch-gates: 68 of 69. dual-build-cjs-loads was not measured locally (prerequisite not met) and is covered by CI.
  • CI on this head: 34 success and 7 expected skips, none failed or pending (41 runs).
  • Mergeable: git merge-tree onto today's main (f38832be9c) is clean.
  • Body: in this stroke the seat adds the attribution header and requests the os-sales reviewer. The footer is present.
  • Declarations: fix(plugin-approvals), patch changeset, Clause-②: no, Metadata-protocol: none. No contract-review record and no dogfood battery are owed.
  • Out-of-scope note (in the PR, not filed): sys_approval_action's own titleFormat, display_title and highlightFields still name step_name. That is a separate surface, and its reach was not measured through a public door. The seat carries it to the lane rescan.
  • Landing: not governed (check-governed-merges --pr 25526: 34 lines). Ready + auto-merge through the relay. Fixes #25501.

Generated by Claude Code

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/s tests tooling

Projects

None yet

2 participants