Confirm and approve a duty before it dispatches

[os-warren]

Fixes #107

duly_duty gains a review_status pipeline — to_confirm → to_review → approved | returned — and the dispatcher refuses to produce tasks for anything that is not approved. A returned duty stops generating work the same night; the work it already generated stays exactly where it was.

---

The premise this card was written on, re-measured

The card said an app-declared action renders nowhere in the Console (true — objectui#7234) but that a trackHistory: true select "renders the platform's pipeline and it can be clicked". Half of that holds, and the half that does not is settled upstream rather than broken:

Claim	Measured on @objectstack/console 17.2.0
the pipeline renders for free	not for free — with stageField absent the renderer takes the first field named status / stage / state / phase, so this object's pipeline was Active · Paused · Retired
the pipeline is clickable	no — the stages are div role="listitem", no handler, no tabindex; a full pointer sequence changes nothing. Same on duly_task.status, so it is the generic chrome, not this object

The second one is a decision, not a defect: objectui#5768 asked for exactly this and was closed on 2026-08-24 by objectui#5917, "render record:path as a readout, not a fake segmented control". So nothing is filed about it here. It costs this card nothing either — the card's own plan is "pipeline + form", and the form is where advancing happens.

The first one is fixed by naming the field, which is what [ADR-0085] stageField is for. status (active/paused/retired) is an unordered state set — ADR-0085's own words — and review_status is the linear one, so the honest declaration puts the stepper where the progression actually is:

---

Three enforcements, all platform surfaces, no authorization hook

1. review_status_transitions — a state_machine validation rule. ADR-0020 retired the standalone workflow type and made a record's legal transitions a validation rule with a flat { from: [to] } table; the rule validator enforces it on the update path with the prior row in hand. Two details are load-bearing, both measured on @objectstack/objectql 17.2.0:

• A state with no row is ungoverned, not frozen. const allowed = transitions[from]; if (!Array.isArray(allowed)) return null — a missing key reads like "locked" and behaves like "wide open". Every state is written down.
• initialStates closes the insert door, which transitions does not cover at all: a select accepts any declared option on create, so without it a duty could be born approved and dispatch immediately — the defect this card exists to close, arriving through the other door.

2. returned_needs_note makes the reason mandatory (and the reason survives the trip back up — the owner is reading it while correcting the duty).

3. Who may issue a verdict. The card asked me to measure whether the platform can restrict writing a value by position before writing any hook. It can, and the surface is per-option visibleWhen — the one authoring predicate whose scope binds the caller, enforced server-side by the rule validator on every insert and update, not merely hidden in the picker. SelectOptionSchema says so in as many words: "Client-side hiding is UX, not authorization … the server MUST also reject writes of its value."

The predicate names the record relationship rather than a position, and that deserves its own line because the card asked for a position:

• 'duly_manager' in current_user.positions evaluates and enforces correctly (measured both ways). But current_user.positions comes from sys_user_position rows, which a package may not declare (src/security/positions.ts) — they are a manual rollout step. Until somebody does it, the list is empty for every caller, pnpm demo included, and a position predicate fails closed: nobody can approve anything. A gate that is inert-or-fatal depending on a manual step is not a gate.
• record.owner != current_user.id needs nothing installed and says what the product means: a review you issue on your own list is not a review. Who else may write the record at all is already decided one layer up by the permission set's write scope — the platform's own axis for that question.

Both refusals, through the REST boundary, signed in as the demo admin (ids elided):

PATCH /api/v1/data/duly_duty/{a duty I own}   {"review_status":"approved"}
→ 400  {"code":"VALIDATION_FAILED","fields":[{"field":"review_status","code":"invalid_option",
         "message":"Review status: option 'approved' is not available"}]}

PATCH /api/v1/data/duly_duty/{one still to_confirm}   {"review_status":"approved"}
→ 400  {"code":"VALIDATION_FAILED","fields":[{"field":"review_status","code":"invalid_transition",
         "message":"That is not a step this review can take. …"}]}

The picker agrees with the server rather than being the only thing that knows: on the caller's own duty it offers ["To confirm","To review"]; on somebody else's it offers all four.

Fail-open case, stated because it is real: a write with no acting user (the seed, an in-process job) cannot bind current_user, so the predicate fails to evaluate and the platform admits the write with a warning. That is what lets the demo seed carry approved rows, and it means this predicate is a rule about people, not a containment boundary for server code. The log noise it produces (one WARN per seeded row) is filed upstream as objectstack-ai/objectstack#14416; the behaviour itself should stay.

---

One edge the card's table does not draw

approved → returned is admitted. With approved terminal (approved: []) a live duty has no correction path at all — not for its owner, not for a manager, not for an administrator; the only remaining doors are a historical import and a re-seed. A cadence that turns out wrong, a duty approved on the wrong person, a catalog sync nobody re-read: each dispatches work every night with no way back into the pipeline that governs it.

Returning is the auditable way out — returned_needs_note makes the reason mandatory, and the tasks already dispatched are untouched. Approval is still not self-issuable, so this is not a loophole around review. One line to revert (approved: ['returned'] → approved: []) if the card meant the table literally.

---

Dispatch

review_status = 'approved' joins status and form in the sweep's query and in planForDuty, because the planner has two callers: the nightly job passes through that query, a backfill and every planner test do not. The gate is === 'approved', deliberately not "not returned": a row carrying no review state at all is not dispatched. Between "an obligation nobody approved goes out" and "an obligation stops until somebody looks at it", the second is the recoverable failure, and it shows up in the run's skipped list rather than in silence.

Run against the live demo database, same duty, one field changed between the two runs:

review_status: to_confirm  →  sweep read 22 duties, created 0,  this duty has 0 tasks
review_status: approved    →  sweep read 23 duties, created 1,  this duty has 1 task (2026-09)

---

Seed

25 approved, 2 waiting on their owner, 2 waiting on a reviewer, 1 returned with a reason. Two of the five are the demo account's own, so the pipeline is walkable on a record page without a second login.

History is planned as approved throughout (demo-history.ts): those tasks were dispatched, which is precisely why returning a duty stops the next run rather than retracting the last six months. A duty sitting in to_confirm with tasks behind it is the import case this card is about — the work was always being done; the list is only now being formalised.

---

Browser walk

pnpm demo, seeded demo organisation, signed in as the dev admin.

Both views in the navigation
「待我确认」 — the owner's queue, with the return reason as a column
「待我审定」 — the reviewer's queue (other people's duties)
The owner confirms: to_confirm → to_review
A reviewer approves somebody else's: → approved
Returning with no reason is refused, in product voice
returned, reason on the record, → to_review available to the owner

Screenshots live on the throwaway branch claude/issue-107-screenshots; delete it with this PR.

---

Landing second: what merging main cost

main moved twice while this was in flight (#110, #112). Merged, both conflicts resolved by keeping both sides — nav_people and nav_to_review are two entries in the Team group, and both zh-CN labels stay.

One of #110's gates then went red on this branch, and it is worth a paragraph because the fix is in their test: test/import-samples.test.ts requires samples/duties.csv to carry every field that is required and writable, and review_status is now both — plus defaulted. An imported catalog row lands to_confirm with no column at all, which is exactly what the product wants an imported list to be, and docs/import/walkthrough.md already documents blank-cell-defers-to-default as the mechanism. So the gate now excludes a field the ROW supplies for itself.

The exclusion is deliberately narrow: only a record-derived default (a CEL expression over other columns of the same row) drops out. A caller-derived default stays in the required set — duly_duty.owner defaults to current_user, so an omitted column would silently give every imported duty to whoever ran the import, which is the worse half of the failure that gate exists for. Reverse-verified by deleting the owner column from the sample: the gate still fails, naming owner.

---

Two permission-set decisions this PR deliberately does NOT take

Both are real, both block the workflow for non-platform-admin callers in a deployment, and both contradict something recorded on purpose — so they are the maintainer's call, not a rider on this card:

1. No Duly set grants allowEdit on duly_duty. src/security/permission-sets.ts says why ("editing cadence after the fact is a correction, which belongs to duly_admin and to duly_catalog_sync"). An object-level edit bit is not per-field, so granting it to let members confirm their own list also lets them rewrite their own cadence.
2. Every set's writeScope on duly_duty is own, with an explicit ⛔ against widening it, pinned by test/security.test.ts — so a manager cannot write a report's duty, and cannot approve one. AGENTS.md's product invariant ("Managers do not enter status. Assigning is their only write.") is about task status, and this card introduces a manager write of a different kind; that tension is worth deciding rather than assuming.

Neither is a blocker for the demo, which runs as a platform admin.

Gates

pnpm validate && pnpm typecheck && pnpm test && pnpm build — all four green on ddcd075, run after the last commit (808 tests, 32 files; Validation passed). validate prints the one expected warning naming @objectstack/security-enterprise, which is this repo's documented steady state.

New coverage: test/duty-review.test.ts (23 assertions — defaults, initialStates, every legal and illegal transition, the note rule, and the owner gate driven with three different callers), plus the dispatch gate in test/dispatch.test.ts (red first: 32 tests failed the moment the planner started refusing unapproved duties) and the seed mix in test/seed.test.ts. No changeset — this repo has none.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p

[os-warren]

Reviewed — merging. Q1: A. Q2: D now, the deployment half is filed as #116.

Gates, re-run by me on the head merged with current main (which now also carries #115): clean merge, validate 0, typecheck 0, test 0 (Test Files 32 passed, Tests 822 passed), build 0. Verified in the source: stageField: 'review_status', initialStates: ['to_confirm', 'to_review'], approved: ['returned'], the dispatch gate at dispatch.plan.ts:380 with review_status in DISPATCH_DUTY_FIELDS, the owner gate on visibleWhen binding current_user, both nav entries.

Two claims in my card did not survive measurement, and both corrections are the valuable part of this PR:

1. trackHistory does not render the pipeline — stageField does. Worse than "no pipeline": without stageField the renderer heuristically picked duly_duty.status (Active / Paused / Retired), so the acceptance screen my card described would have shown a pipeline for the wrong field and I would have read it as done. Declaring stageField per ADR-0085 is the fix and the lesson.
2. The pipeline is a readout, not a control. The stages are div role=listitem with no handler — an upstream decision (objectui#5768 → closed by objectui#5917, "render record:path as a readout, not a fake segmented control"), not a defect, so correctly nothing was filed. Advancing happens in the form. My card was written on the opposite assumption; the browser overruled it.

Q1 — keep approved → returned. With approved terminal a live duty has no correction path at all: not its owner, not a manager, not an administrator (state_machine has no role exemption; the only doors left are a historical import or a re-seed). A wrong cadence, a duty approved on the wrong person, a catalog sync nobody re-read — each would dispatch work every night with no way back into the pipeline that governs it. A return with a mandatory reason is the auditable way out, and approval is still not self-issuable. The table's — was mine and was wrong.

Q2 — D, and the reasoning is exactly right. The platform's edit bit is object-level; opening it to let a member confirm a list also lets them rewrite their own cadence, and widening a manager's write scope makes approving indistinguishable from editing someone else's duty. Both overturn decisions written down with their reasons. This is p0 for a demo that runs as a platform admin, and it works there. The deployment question — whether approval is an ordinary field write at all — is a security-model decision for the maintainer, filed as #116 with the three routes and a recommendation to measure field-level write permission first.

The owner gate is a good piece of work: visibleWhen is the one authoring surface whose predicate scope binds the caller, and it is enforced on the write path (400 invalid_option, measured over REST). Keying it on record.owner != current_user.id rather than on a position is correct for the stated reason — a position predicate fails closed in every deployment that has not populated sys_user_position, pnpm demo included.

Dispatch red-first (32 of 57 failures before any fixture moved) and the live dispatcher runs (to_confirm → 0 tasks; approved → 1) are the evidence the gate is real. objectstack#14416 (27 WARN lines per demo boot from a system write that can never bind current_user) is a fair platform report.

---

Generated by Claude Code