Skip to content

feat(spec,lint): flows translation face gains approvals.NODE.label for an approval node's step label - #24943

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-24915-approval-node-translation-key
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-24915-approval-node-translation-key

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Closes #24915

Clause-②: yes (widening)

What changes

TranslationDataSchema.flows.FLOW carried label, successMessage, errorMessage, screens.NODE (screen nodes) and refusals.NODE (refusing end nodes), and nothing for an approval node. A language pack had nowhere to put an approval step's name. HotCRM measured this: flows.opportunity_approval.screens.manager_review.title is refused translation-target-unknown, because manager_review is an approval node and not a screen.

  • Spec (packages/spec/src/system/translation.zod.ts): flows.FLOW.approvals.NODE.label is a declared key. It is addressed by the approval node's FlowNode.id and overlays that node's own label. The entry is closed (strictObject). title, name, heading and stepLabel are renames onto label, and approval on the flow level is a rename onto approvals.
  • objectstack validate (packages/lint/src/validate-translation-references.ts): the translation-reference check collects the flow's approval node ids at any depth, using the same walkFlowNodes universe, so regions are included. An approvals.NODE key over an approval node resolves. Over anything else it is refused translation-target-unknown, with the existing diagnosis style:
    • a node of another type: "...declares as a decision node, not an approval, so nothing resolves this key."
    • a screen node: "...declares as a screen node; its heading is keyed screens.NODE.title."
    • an unknown id: "...declares no node for, so the step keeps its source-locale label.", followed by "Did you mean" over the approval nodes.
  • Wrong-group guidance: a screens.NODE or refusals.NODE key written over an approval node now says "...declares as an approval node; its step label is keyed approvals.NODE.label." The old message only said the node is not a screen. It also read "as a approval node".

Why this spelling

  • Group approvals, keyed by node id: this follows the grammar the face already has. screens addresses screen nodes and refusals addresses refusing end nodes, each by FlowNode.id, the identifier the approvals service already holds (sys_approval_request stores it as flow_node_id).
  • Leaf label, nothing else. I read ApprovalNodeConfigSchema (packages/spec/src/automation/approval.zod.ts). Its keys are approvers, behavior, minApprovals, lockRecord, approvalStatusField, onEmptyApprovers, fallbackApprovers, decisionOutputs, escalation and maxRevisions. None of them is a heading, instructions text or a comment prompt. The one person-facing string an approval step carries is FlowNode.label. plugin-approvals copies that label onto current_step_label when a request opens (approval-service.ts, labelOrMachineName(input.nodeLabel, ...)). A screen spells its leaf title because it overlays ScreenConfigSchema.title. This leaf overlays the node label, so it is spelled label, and title is an alias onto it.
  • Plain string, not a {{ }} text slot: the node label is plain text that is copied, not rendered through renderTextSlot. This is the same as the flow's own label.
  • Out of scope, on purpose: a typed decisionOutputs[] entry's label (a field label in the decision dialog). It stays an owed row in the family pin, with its reason updated. It is a separate surface that needs its own reader.

Tests

Measured at head 606faa06ee (merged with origin/main).

  • Repro first: commit ebb460a714 added the tests before the fix.
  • Reverse verification: after the fix was committed, I restored translation.zod.ts and validate-translation-references.ts to the base 81e6edd463 (tree only, git restore --source). The mutation was confirmed on disk (approvals: z.record count 0, APPROVAL_NODE_TYPE count 0). I rebuilt spec and ran the suites:
    • spec: 6 failed | 158 passed. The four new approvals cases failed, and so did the two face-pin cases ("every keyed row ... exists in the face", "every face key is filled").
    • lint: 6 failed | 180 passed. The five new refusal/redirect cases failed, and so did the one-line-verdict arm coverage.
    • Restored with git checkout HEAD -- PATHS. git diff HEAD was empty and git status was clean. I rebuilt spec, and ablation-dist-preflight found the marker present in 20 built files.
  • Controls:
    • screen and refusal keys of the same flow still resolve beside an approval key;
    • a screens key over a decision node keeps its own diagnosis;
    • an approval key on a non-approval node is refused by name;
    • a key the approval entry does not declare (instructions) is refused at parse.
  • Family pin (flows-translation-face.test.ts): node.label is now keyed to both screens.NODE.title and approvals.NODE.label (new also field), so the face-key-filled check accounts for the new key.
  • Package runs:
    • pnpm --filter @objectstack/spec test: 703 files, 21365 passed.
    • pnpm --filter @objectstack/spec typecheck: exit 0.
    • pnpm --filter @objectstack/lint test: 151 files, 7002 passed (after the merge).
    • pnpm --filter @objectstack/lint typecheck: OK.
  • Generated artifacts and data checks:
    • pnpm --filter @objectstack/spec check:generated: all 14 artifacts are up to date. These were regenerated with --fix: content/docs/references/system/translation.mdx, the strictness-ledger counts and the liveness state counts.
    • pnpm check:i18n: OK, and check:nul-bytes: OK.
  • Derived gates (dispatch-gates --commands, reconciled with --ran): 113 derived, 113 run, all exit 0, 0 NOT-MEASURED (a derived zero: every family recorded an exit code), re-run on the merged head 606faa06ee.
  • eslint, narrowed to the 5 touched source and test files (--no-inline-config --format json): 5 files, 0 errors, 0 warnings. All 5 are inside the config (--print-config resolves each). The config declares no type-aware linting (no parserOptions.project), so this diff cannot change the verdict on any untouched file.

Liveness

packages/spec/liveness/translation.json gains a flows.approvals row (drilled to label) with status planned, not live. The reader that renders the step label in the reader's locale is the plugin-approvals half of #24863, which this PR does not touch. It is a contract-first split, the same way screens and the toasts landed. #24863 remains open.

Review tier

packages/spec is a published schema, so this PR owes a CONTRACT_REVIEW_TIER review of record before it enters the queue (pm-dispatch references/contract-review.md). It touches no governed surface (.claude/**, docs/adr/**, AGENTS.md, workflows, skills/**), so no Tier S or Tier H landing rule applies.

Acceptance notes


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

5 anchor(s) derived from 2 changed package(s); no hand-written page names any of them. ⚠️ 2 changed file(s) yielded no anchor (packages/spec/liveness/state-counts/translation.md, packages/spec/liveness/translation.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 2 changed file(s) yielded no anchor (packages/spec/liveness/state-counts/translation.md, packages/spec/liveness/translation.json) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 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 — 143 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 71696121a64d8bd057fac969a72a84295890ed37 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 71696121a64d8bd057fac969a72a84295890ed37

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 606faa06ee40f4f065ade6632f939476269f20bd
Local-runs: none
Scope: packages/spec/src/system/translation.zod.ts + packages/lint/src/validate-translation-references.ts, packages/spec/liveness/translation.json, packages/spec/src/system/flows-translation-face.test.ts, packages/spec/src/automation/approval.zod.ts (read for the leaf), packages/lint/src/flow-walk.ts (read for the node universe), packages/plugins/plugin-approvals/src/approval-service.ts (read for the seam), packages/cli/src/utils/i18n-extract.ts (read for the extractor's demand)

Read at 2026-10-20T17:44Z: card #24915 (body, the claim 6294228866, the os-dev-report 6296626936), parent #24863, the standing ruling #6015 6292005854, the PR body and file list, and git diff origin/main...FETCH_HEAD at this head. Verdicts of the derived gate families are the head's check-runs, read, not re-run.

① Derived judgments

Every accept-set and public-surface change the diff implies, each named:

  1. flows.FLOW.approvals.NODE.label enters the accept set at both doors (TranslationDataSchema and TranslationItemSchema share appTranslationDataShape). RIGHT. It is what the card's "Done when" asks for, spelled in the grammar screens.NODE / refusals.NODE already set: a group named by node type, keyed by FlowNode.id. The leaf label is the source's own spelling: I read ApprovalNodeConfigSchema (approval.zod.ts, keys approvers, behavior, minApprovals, lockRecord, approvalStatusField, onEmptyApprovers, fallbackApprovers, decisionOutputs, escalation, maxRevisions), and none is a heading or instruction text, so the step's one person-facing string is FlowNode.label, the string approval-service.ts snapshots onto current_step_label via labelOrMachineName(input.nodeLabel, input.nodeId). A screen spells title because it overlays ScreenConfigSchema.title; this leaf overlays the node label. Consistent, not merely plausible.
  2. The entry is closed (strictObject): an undeclared leaf (instructions) is refused at parse. RIGHT: the same shape as the screens and refusals entries, and a planned key with an open entry would let a pack carry text no reader will ever read.
  3. Entry aliases title, name, heading, stepLabel -> label. RIGHT. title is the measured HotCRM spelling (screens.manager_review.title), so the rename message is the one an author actually arrives holding; name/heading mirror the screens entry's own alias table; stepLabel is the column name's camelCase. None collides with a declared leaf.
  4. Flow-level alias approval -> approvals. RIGHT, the singular/plural pair the group already carries for refusal -> refusals and screen -> screens. The pre-existing steps -> screens alias is left alone (see ③).
  5. objectstack validate acceptance: an approvals.NODE key resolves over an approval node at any depth. RIGHT. buildUniverse collects the ids from the same walkFlowNodes universe the screen and refusal arms use (regions included), gated on APPROVAL_NODE_TYPE imported from @objectstack/spec/automation (a real export, approval.zod.ts:388), not a string literal.
  6. objectstack validate refusal: translation-target-unknown over a node of another type, a screen node (pointed at screens.NODE.title), and an unknown id (with Did-you-mean over the approval ids). RIGHT, and it meets the card's "refuses it on other node types" clause. Approval nodes already land in otherNodes (every non-screen node does), so the redirects below read the real node type.
  7. Redirects: a screens.NODE or refusals.NODE key over an approval node now says the step label is keyed approvals.NODE.label. RIGHT; a diagnosis change only, the verdict (error, same rule) is unchanged, and the pre-existing "a approval node" article slip goes with it. The [#22161] one-line-verdict arm list gains the four new message shapes, so the arm-coverage pin stays whole.
  8. Public type surface: TranslationData.flows[].approvals?: Record<string, { label?: string }> is new and optional; nothing existing is removed, renamed or narrowed. The docs mdx tables (three doors) and the strictness-ledger count (375 -> 376) are regenerated by repo tooling, and check:generated is a derived family the head's check-runs answer.
  9. Liveness: flows.approvals and its child label enter the ledger as planned, without authorWarn. RIGHT. A planned row warns only with authorWarn (liveness README), and setting it here would switch the CLI i18n coverage gate's whole-group flows.* demand off (the authorWarnedTranslationGroups coupling the screens row records), which would silence the live screens/label demand to add a warning on a key nobody authors yet. The flip condition (the plugin-approvals reader, Approvals show their flow and step names in the authoring language only: the request snapshots Flow.label / node.label at open, and the translation contract has no key for an approval step #24863) is written into the row.
  10. The family pin (flows-translation-face.test.ts): node.label is keyed to screens.NODE.title and also approvals.NODE.label. RIGHT. One slot is read by two node types under two keys; the new also field keeps the "every face key is filled" check exact instead of loosening it. decisionOutputs[].label stays owed with its reason rewritten to name this group; it is a decision-dialog field label, a surface of its own.

Reverse verification was reported by the dev (restoring the two source files to base failed exactly the 6 + 6 new/pin cases); I did not re-run it, and it is not a gate.

② Semver level

.changeset/24915-approval-step-translation-key.md declares @objectstack/spec: minor and @objectstack/lint: minor. That matches what the diff publishes: a new optional key on a published schema (an additive widening of the accept set) and a new acceptance in objectstack validate with no existing verdict flipped from accept to refuse. No key is removed, no spelling is narrowed, no existing pack is refused that parsed before, so minor is right and major is not owed. skip-changeset does not apply.

Clause-②: yes (widening) is on the PR body, the changeset and the claim, and the three agree with the diff. Under the standing ruling (#6015 6292005854, 「甲」) that rule governs breaking narrowings; this PR carries none, so a widening ships in any 18.x minor with nothing to convert.

③ Boundary flags

Dev open_questions: none declared. Dev flags and acceptance notes, each answered:

Nothing escalated.

Head check-runs at the time of this record: every check-run on 606faa06ee is success or skipped (skipped: the opt-in and matrix-placeholder families), none failing, none in progress.

Implemented-by: claude/issue-24915-approval-node-translation-key
Reviewed-by: session_01D6F9axfoPzR7t8G1J7Ytzz

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 17:46
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit b9a79ef Oct 20, 2026
43 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24915-approval-node-translation-key branch October 20, 2026 18:20
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:system size/m tests tooling

Projects

None yet

2 participants