Skip to content

fix(app-showcase): write the expense approvals' decision back onto the report - #25525

Merged
objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-25469-showcase-signoff-mirror
Oct 23, 2026
Merged

objectstack-fleet[bot] merged 2 commits into
mainfrom
claude/issue-25469-showcase-signoff-mirror

Conversation

@objectstack-fleet

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

Copy link
Copy Markdown
Contributor

Requested by Sales · project thread

Fixes #25469

Clause-②: no
Metadata-protocol: none

What changed

showcase_expense_signoff (per-group 会签, node dual_signoff) and showcase_committee_quorum (2-of-3, node committee) ran start → approval → end, with bare end nodes on both outcome arms. The approval request finalized, but nothing wrote the decision onto the expense report, so EXP-2001 kept reading status: submitted after both groups approved.

Each flow now runs an update_record on each outcome arm and has runAs: 'system':

  • approve → mark_approved → status: 'approved'
  • reject → return_to_draft → status: 'draft' (the report goes back to the submitter, who can rework and resubmit; this is the same revert-on-reject showcase_invoice_signoff's flag_held does)

runAs: 'system' follows the invoice flow's precedent. The write-back is an outcome of the approval, not an act of the submitter, so it has to land whatever the submitter's edit rights are.

Landing: examples/app-showcase/src/automation/flows/index.ts (the file the claim names) plus the new pin examples/app-showcase/test/expense-approval-write-back.test.ts. No package source changed.

Why update_record per branch and not the approvalStatusField mirror

I checked how the mirror behaves in packages/plugins/plugin-approvals/src/approval-service.ts (mirrorStatusField / mirroredStatusValue):

  • It writes the request's status words exactly as they are: pending when the request opens, approved / rejected on a decision, recalled on a recall or dead-run sweep, returned on a send-back, and rejected on the revision-limit auto-reject.
  • It has no value mapping. The spec key is a bare approvalStatusField: z.string(), and adding a mapping would mean editing packages/spec, which is out of bounds for this card.
  • It writes only values the target field lists among its options. Any other value is skipped and the service logs a warning (approval status 'X' was not written to OBJECT.FIELD). The only exception is a transition that abandons the request, where it restores the value from before submission.

showcase_expense_report.status declares draft / submitted / approved / reimbursed. So a mirror on status would:

  • skip pending and log a warning on every submit
  • write approved correctly
  • skip rejected, so a rejected report would keep reading "Submitted", which is the same defect on the other arm

The spec also says the mirror target "should be readonly on the object", and status is the field the submitter edits. When the decision drives a business-lifecycle field, the right tool is an explicit write per branch. Both flows' docblocks now explain this.

Committee quorum flow

showcase_committee_quorum had the same gap, with bare end nodes on both arms. It gets the same fix.

Pin

test/expense-approval-write-back.test.ts boots the production stack: a real ObjectKernel with the ObjectQL, automation and approvals plugins over real sqlite, and the app's real ExpenseReport / ExpenseLine objects and real flows. It opens each request the way the seed's launchSignoff does, then decides it through ApprovalService.decide. describe.each runs it over both flows:

  • approve: after the first of two approvals the report still reads submitted and the request is not finalized. After the finalizing approval the run is completed, it stepped mark_approved and not return_to_draft, and the report reads approved.
  • reject: the run steps return_to_draft and the report reads draft, a value the status field declares.
  • CONTROL: the flow is rebuilt in its pre-fix shape (bare end arms, no update_record nodes) on the same harness. The same approvals finalize and resume the run, and the report stays at submitted, which is the defect as reported.

The committee case adds an expense line of 8,900 so the total_amount >= 5000 start gate opens (the seeded EXP-DEMO is 8,900). The per-group case uses 1,500, under the threshold like EXP-2001.

Ablation (scripts/ablation-replace.mjs, on committed HEAD 3e661a73f6)

The mutation pointed the signoff approve edge back at the bare end node: anchor { id: 'e2', source: 'dual_signoff', target: 'mark_approved', label: 'approve' }, was replaced with target: 'approved'.

ablation-replace: anchor   x1 -> x0
ablation-replace: blob     a47c39e3548331eb29d0879152dd745527556d8c -> 78466ece32b2df854bf4e326c1c6ab5f8618a75f
     × approve: the report reads `approved` once the approval finalizes, not before
AssertionError: expected [ 'start', 'dual_signoff' ] to include 'mark_approved'
      Tests  1 failed | 5 passed (6)
ablation-replace: ok restored: blob == HEAD (a47c39e35483) and `git diff HEAD` is empty

The test went red under the mutation and green again once the file was restored. The example is source-imported by the test (relative ../src import), so there is no dist/ leg.

Verification (head 3e661a73f6)

  • pnpm --filter @objectstack/example-showcase exec vitest run test/expense-approval-write-back.test.ts: 6/6 passed
  • pnpm --filter @objectstack/example-showcase typecheck && pnpm --filter @objectstack/example-showcase test: lock VERDICT command-exit 0; Test Files 38/38, Tests 461/461. The new test file is in the tsc program (--listFiles: 1 hit).
  • node scripts/pm/dispatch-gates.mjs --commands: 45 commands, each run with its exit code recorded. 44 exited 0. pnpm check:dual-build-cjs-loads exited 3 (PREREQUISITE NOT MET: unbuilt packages outside this diff's closure), so it is NOT MEASURED locally and left to CI.
  • --ran reconciliation: 45 derived famil(ies) accounted for — 44 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3).

No changeset: @objectstack/example-showcase is private: true, so nothing publishes (skip-changeset).

Acceptance notes

  • docs/qa/platform-checklist/areas/approvals.json → approvals.status-mirror-field still records under knownGaps that no showcase flow declares approvalStatusField. That stays true after this PR, deliberately (see above). The checklist is not edited here.
  • showcase_expense_signoff's start gate has no amount ceiling, so a report of $5,000 or more matches both flows' start conditions on submit. The seed routes the two demos to different records on purpose. I did not measure what a live submit of such a report does, so this is noted here and not filed.
  • On the issue's lockRecord question: the write-back runs after the request has left pending, and the lock is keyed on pending. It also runs as the system principal, which the lock hook exempts. The pin shows the write lands under lockRecord: true.

🤖 Generated with Claude Code

https://claude.ai/code/session_016g7nYP9wdNx1GDa5XDr7Uy

…e report

showcase_expense_signoff and showcase_committee_quorum ended both outcome
arms on bare end nodes, so an approved expense report kept reading
Submitted. Each arm now runs an update_record (approve -> approved,
reject -> draft) under runAs: system, and a showcase test drives a real
decision through the approvals service on both flows with a pre-fix
control.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016g7nYP9wdNx1GDa5XDr7Uy
@objectstack-fleet objectstack-fleet Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Oct 23, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

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

domain:services seat 1 (seat post #6021) · os-sales · session_016g7nYP9wdNx1GDa5XDr7Uy · card #25469 (S, in-seat subagent; claim on the card; os-dev-report 6373022320) · reviewed head 3e661a73f6

  • Diff: 2 files, +307 / −4.
    • showcase_expense_signoff (dual_signoff, per_group) and showcase_committee_quorum (committee, quorum) each gain one update_record per outcome arm: approve sets status: 'approved', and reject sets status: 'draft'.
    • Both flows gain runAs: 'system', and the edges are re-pointed through the new nodes.
    • Before this change both arms ended on bare end nodes, so a decided report kept reading "Submitted".
  • Seat check:
    • Precedent. The shape and the elevation match InvoiceDualSignoffFlow (runAs: 'system' at flows/index.ts:1307), which also reverts to draft on reject.
    • Why no mirror. approvalStatusField was measured and rejected for a reason the docblock states. The mirror writes the request vocabulary verbatim (pending, rejected, recalled, returned). The report's status options declare none of those, so each would be skipped with a warning, and the report would still read wrong. The seat agrees with this route.
  • Pins (report):
    • The new test/expense-approval-write-back.test.ts runs the real kernel, ObjectQL, automation and approvals over sqlite. It decides through ApprovalService.decide, for both flows across approve, reject and a CONTROL with the pre-fix bare-end shape that leaves the report submitted. 6/6 pass.
    • Ablation of e2 → mark_approved turns the approve case red. The restore is clean.
    • The showcase suite gives 38/38 files and 461/461 tests, and typecheck exits 0.
    • dispatch-gates: 44 of 45. dual-build-cjs-loads was not measured locally (prerequisite not met) and is covered by CI.
  • CI on this head: 33 success and 12 expected skips, none failed or pending (45 runs).
  • Mergeable: git merge-tree onto today's main (b2411c001d) is clean. It is also clean pairwise with chore: version packages #24876, the only other open PR touching examples/app-showcase.
  • Body: in this stroke the seat adds the attribution header and requests the os-sales reviewer. The footer is present.
  • Declarations: fix(app-showcase), Clause-②: no, Metadata-protocol: none, skip-changeset. No contract-review record and no dogfood battery are owed.
  • Out-of-scope notes (in the PR, not filed):
    • The approvals.status-mirror-field known gap stays true by design.
    • A report of 5000 or more matches both expense flows' start conditions. The seed routes the two demos to different records on purpose. This is a read-only inference and was not measured.
  • Landing: not governed (check-governed-merges --pr 25525: 311 lines). Ready + auto-merge through the relay. Fixes #25469.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot requested a review from os-sales October 23, 2026 19:30
@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 23, 2026 19:30
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 23, 2026 19:30
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit 1ff43b6 Oct 23, 2026
53 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-25469-showcase-signoff-mirror branch October 23, 2026 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

2 participants