Repository navigation
清单确认与审定：员工确认 → 主管审定/打回，未审定的职责不派发 #107
Description
售前演示优先级 p0。 方案 v7 p6「现有清单批量导入，员工确认、主管审定」和 p25「漏报两道机制」的第一道。目前 duly_duty 没有任何确认/审定状态，导入或自建的清单立刻生效、立刻派发。
要做成什么样（界面上看得见）
duly_duty 增加一个 review_status 单选，trackHistory: true（这样记录页会免费渲染平台自带的状态流水线——已实测：我们自己声明的 action 一个都渲染不出来（objectui#7234），但 trackHistory select 的流水线能渲染并能点）：
| 值 | 含义 | 谁推进 |
|---|---|---|
to_confirm |
待员工确认（导入/目录应用生成的清单初始态） | 员工 → to_review |
to_review |
待主管审定（员工确认后；员工自建的初始态） | 主管 → approved / returned |
approved |
已审定，参与派发 | — |
returned |
打回，附 review_note |
员工修改后 → to_review |
source = 'catalog'/'assigned'生成的职责初始to_confirm；source = 'self'初始to_review。- 用
state_machine校验规则钉住合法转移（例如不能从to_confirm直接到approved）。谁能推进哪一步：先量平台字段级/记录级权限能不能按 position 限制某个值的写入；能就用，不能就在卡片上如实记录为平台缺口并上报，不要手写 hook 做权限。 review_note（打回原因）文本字段，returned时必填（validation rule）。
派发必须尊重它
dispatch.plan.ts 的职责选择加上 review_status = 'approved'。打回的职责当天就不再生成任务——这是演示时最有说服力的一幕（改状态 → 明天的待办变了）。test/dispatch.test.ts 加断言：未审定的职责一个任务都不生成。种子数据里让绝大多数 catalog 职责为 approved，留 2–3 条 to_confirm、1 条 returned，演示才有东西看。
视图
- 员工：「待我确认」（
review_status in (to_confirm, returned)，owner = 当前用户）。 - 主管：「待我审定」（
to_review）。 - 两者都进导航；主管的放 Team 组。
不做
- 不加自定义 action（渲染不出来）；全部走状态流水线 + 表单。
- 仪表盘「清单完备度」磁贴不在本卡——由仪表盘卡片（同批派发）做，口径是按
review_status计数、不按条目数（AGENTS.md「条目数不进排名」的不变量不动）。本卡只要保证字段和数据集可用；两张卡都会碰src/datasets/，后落地的合 main 重跑门。
验收
- 记录页能看到并点状态流水线走完
to_confirm → to_review → approved和→ returned → to_review；浏览器截图 - 打回一条职责后运行派发，它不产生任务；审定后产生
- 四道门绿；中文 bundle 补齐新字段/选项（i18n — en source bundle, zh-CN translation, CI coverage gate #18 的门会红，把它变绿）
Activity
os-warren commentedon Sep 2, 2026
Claiming this issue.
01SqkTcrxUFci7nqXdbBSe2p(https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p)claude/issue-107-duty-review-workflow../duly-issue-107Starting now:
review_status(trackHistory select) + state_machine rule +review_note, dispatch gating onapproved, the two views, seed data, zh-CN entries. Will measure field-level/record-level write permission by position before deciding on the "who may advance which step" half — no hand-written authorization hook either way.Generated by Claude Code
Confirm and approve a duty before it dispatches (#107)
Point the record-page stepper at the review pipeline (#107)
Keep the import sample gate about columns whose absence refuses a row (…
Widen the helper's return type to match its cast (#107)
os-warren commentedon Sep 2, 2026
{ "issue": 107, "status": "done", "branch": "claude/issue-107-duty-review-workflow", "pr": "https://github.com/objectstack-ai/duly/pull/114", "premise_still_valid": true, "summary": "review_status (to_confirm/to_review/approved/returned) on duly_duty with a CEL default keyed on source, a state_machine rule pinning the transitions AND initialStates (so a duty cannot be born approved), review_note required on a return, dispatch gated on approved in both DISPATCH_DUTY_FIELDS and the sweep's query, the two views in the navigation, a seed mix of 25/2/2/1, and zh-CN for every new label. TWO OF THE CARD'S MECHANISM CLAIMS DID NOT SURVIVE MEASUREMENT, neither fatal: (1) trackHistory does not render the pipeline — stageField does; absent it the renderer heuristically picked duly_duty.status (Active/Paused/Retired), so the card's own acceptance screen was showing the wrong field. Fixed by declaring stageField: 'review_status' (ADR-0085). (2) The pipeline is NOT clickable — the stages are div role=listitem with no handler, on duly_task.status too. That is an upstream DECISION, not a defect: objectui#5768 asked for exactly this and was closed 2026-08-24 by objectui#5917, 'render record:path as a readout, not a fake segmented control'. Nothing filed; advancing happens in the form, which is the other half of the card's own plan and is what the screenshots show. On 'who may advance which step': the platform CAN restrict a value by caller — per-option visibleWhen is the one predicate scope that binds current_user and the rule validator enforces it on the write path (measured through REST: 400 invalid_option). It is used, but keyed on record.owner != current_user.id rather than on a position, because current_user.positions comes from sys_user_position rows a package may not declare, so a position predicate fails CLOSED in every deployment that has not done that manual step, pnpm demo included. Landed second: merged main (#110, #112), kept both nav entries, and narrowed #110's import-sample gate to exempt record-derived defaults only.", "tests": "All four gates green on ddcd075 (run after the final commit, exits captured before any pipe): `pnpm validate` → '✓ Validation passed (567ms)' + the one expected @objectstack/security-enterprise warning; `pnpm typecheck` → exit 0; `pnpm test` → 'Test Files 32 passed (32) / Tests 808 passed (808)'; `pnpm build` → 'Artifact: dist/objectstack.json (196.3 KB)'. RED FIRST on the dispatch gate: adding the review check to planForDuty took test/dispatch.test.ts to 'Tests 32 failed | 25 passed (57)' before any fixture was touched. New test/duty-review.test.ts (23 assertions) drives the real engine through ordinary writes — no isSystem, no skipStateMachine — covering the source-keyed defaults, initialStates refusing a born-approved insert, every legal and illegal transition, the note rule (including blanking the note while still returned), and the owner gate driven with three callers (owner refused, other caller allowed, no-user write allowed and pinned as the seed path). REVERSE VERIFICATION on the narrowed import gate: deleted the `owner` column from samples/duties.csv, confirmed the mutation on disk ('owner' occurrences 3 → 2, header printed), gate went red naming ['owner']; a trap EXIT restored the file and the restore was confirmed (git diff on samples/ empty). BROWSER, against pnpm demo on port 4107: walked to_confirm → to_review on the demo account's own duty, to_review → approved and → returned on other people's, returned → to_review, with the refusal for a return with no reason rendering in product voice; the picker offers ['To confirm','To review'] on the caller's own duty and all four on somebody else's. Both server refusals reproduced over REST as the signed-in admin (400 VALIDATION_FAILED / invalid_option and invalid_transition). DISPATCH, real dispatcher against the live demo database: to_confirm → sweep read 22 duties, created 0, that duty has 0 tasks; approved → 23 duties, created 1, one task for 2026-09.", "open_questions": [ { "question": "The state machine admits `approved → returned`, which the card's table does not draw (its 谁推进 column for approved is '—'). Keep it?", "options": [ "A — keep `approved: ['returned']`. A manager may pull an approved duty back, with a mandatory reason; tasks already dispatched are untouched.", "B — revert to `approved: []`, exactly as the card's table reads." ], "recommendation": "A. Measured: with approved terminal a live duty has no correction path at all — not for its owner, not for a manager, not for an administrator (state_machine has no role exemption; the only remaining doors are a treatAsHistorical 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 then dispatches work every night with no way back into the pipeline that governs it. Returning is the auditable way out and is not a loophole — approval is still not self-issuable. One line to revert if you meant the table literally." }, { "question": "No Duly permission set can write `review_status` in a real deployment. Which half do you want opened?", "options": [ "A — grant `duly_member.duly_duty.allowEdit` (writeScope stays `own`) so an owner can confirm their own list. The bit is object-level, not per-field, so it also lets a member edit their own cadence — which src/security/permission-sets.ts refuses on purpose ('editing cadence after the fact is a correction, which belongs to duly_admin and to duly_catalog_sync').", "B — widen `duly_manager.duly_duty.writeScope` to `unit_and_below` so a manager can approve a report's duty. Contradicts an explicit ⛔ in that file and is pinned by test/security.test.ts; also brushes AGENTS.md's 'Managers do not enter status. Assigning is their only write.'", "C — both, which is what makes the workflow usable end to end by real users.", "D — neither for now: the demo runs as a platform admin and works, and the deployment question waits for a permission-model pass." ], "recommendation": "D for this card and C as the next one, filed separately. Real business need: the feature is p0 for a DEMO, and the demo account is a platform admin, so nothing is blocked this month; a deployment needs C or the pipeline is decoration. Long-term soundness: A and B each overturn a decision that is written down with its reasoning, and B's cost is not 'a manager writes duties' but that the object-level bit cannot say WHICH field — approving is indistinguishable from rewriting cadence on somebody else's duty. That is a permission-model question (field-level write permission, or an approval that is not an ordinary field write), not a line to slip into this PR. Hard-to-get-wrong: opening object edit to get one field is exactly the over-grant that never gets narrowed later. Startup scope: shipping the visible, demoable workflow now and deciding the permission model deliberately is the cheaper order." } ], "out_of_scope_findings": [ "filed as objectstack-ai/objectstack#14416: a gated select option logs one WARN per seeded row ('option visibleWhen … failed to evaluate — allowed through') because a system write can never bind current_user — 27 lines per demo boot, and the identical line is what a genuine unevaluable predicate produces for an authenticated caller", "not filed, already decided upstream: the record-page stage pipeline is a readout, not a control (objectui#5768 → objectui#5917, 2026-08-24). Recorded here because this card was written on the opposite assumption", "no change made, reported above as an open question: no Duly permission set can write review_status — duly_member has no allowEdit on duly_duty and every set's writeScope there is `own`" ] }Generated by Claude Code
os-warren commentedon Sep 2, 2026
os-dev-report
(Reposting the report above with a plain-text marker: GitHub's body sanitizer ate the HTML-comment
os-dev-reportmarker on the previous comment, so a scan for it would not have found this. Same JSON, unchanged.){ "issue": 107, "status": "done", "branch": "claude/issue-107-duty-review-workflow", "pr": "https://github.com/objectstack-ai/duly/pull/114", "premise_still_valid": true, "summary": "review_status (to_confirm/to_review/approved/returned) on duly_duty with a CEL default keyed on source, a state_machine rule pinning the transitions AND initialStates (so a duty cannot be born approved), review_note required on a return, dispatch gated on approved in both DISPATCH_DUTY_FIELDS and the sweep's query, the two views in the navigation, a seed mix of 25/2/2/1, and zh-CN for every new label. TWO OF THE CARD'S MECHANISM CLAIMS DID NOT SURVIVE MEASUREMENT, neither fatal: (1) trackHistory does not render the pipeline — stageField does; absent it the renderer heuristically picked duly_duty.status (Active/Paused/Retired), so the card's own acceptance screen was showing the wrong field. Fixed by declaring stageField: 'review_status' (ADR-0085). (2) The pipeline is NOT clickable — the stages are div role=listitem with no handler, on duly_task.status too. That is an upstream DECISION, not a defect: objectui#5768 asked for exactly this and was closed 2026-08-24 by objectui#5917, 'render record:path as a readout, not a fake segmented control'. Nothing filed; advancing happens in the form, which is the other half of the card's own plan and is what the screenshots show. On 'who may advance which step': the platform CAN restrict a value by caller — per-option visibleWhen is the one predicate scope that binds current_user and the rule validator enforces it on the write path (measured through REST: 400 invalid_option). It is used, but keyed on record.owner != current_user.id rather than on a position, because current_user.positions comes from sys_user_position rows a package may not declare, so a position predicate fails CLOSED in every deployment that has not done that manual step, pnpm demo included. Landed second: merged main (#110, #112), kept both nav entries, and narrowed #110's import-sample gate to exempt record-derived defaults only.", "tests": "All four gates green on ddcd075 (run after the final commit, exits captured before any pipe): `pnpm validate` → '✓ Validation passed (567ms)' + the one expected @objectstack/security-enterprise warning; `pnpm typecheck` → exit 0; `pnpm test` → 'Test Files 32 passed (32) / Tests 808 passed (808)'; `pnpm build` → 'Artifact: dist/objectstack.json (196.3 KB)'. RED FIRST on the dispatch gate: adding the review check to planForDuty took test/dispatch.test.ts to 'Tests 32 failed | 25 passed (57)' before any fixture was touched. New test/duty-review.test.ts (23 assertions) drives the real engine through ordinary writes — no isSystem, no skipStateMachine — covering the source-keyed defaults, initialStates refusing a born-approved insert, every legal and illegal transition, the note rule (including blanking the note while still returned), and the owner gate driven with three callers (owner refused, other caller allowed, no-user write allowed and pinned as the seed path). REVERSE VERIFICATION on the narrowed import gate: deleted the `owner` column from samples/duties.csv, confirmed the mutation on disk ('owner' occurrences 3 → 2, header printed), gate went red naming ['owner']; a trap EXIT restored the file and the restore was confirmed (git diff on samples/ empty). BROWSER, against pnpm demo on port 4107: walked to_confirm → to_review on the demo account's own duty, to_review → approved and → returned on other people's, returned → to_review, with the refusal for a return with no reason rendering in product voice; the picker offers ['To confirm','To review'] on the caller's own duty and all four on somebody else's. Both server refusals reproduced over REST as the signed-in admin (400 VALIDATION_FAILED / invalid_option and invalid_transition). DISPATCH, real dispatcher against the live demo database: to_confirm → sweep read 22 duties, created 0, that duty has 0 tasks; approved → 23 duties, created 1, one task for 2026-09.", "open_questions": [ { "question": "The state machine admits `approved → returned`, which the card's table does not draw (its 谁推进 column for approved is '—'). Keep it?", "options": [ "A — keep `approved: ['returned']`. A manager may pull an approved duty back, with a mandatory reason; tasks already dispatched are untouched.", "B — revert to `approved: []`, exactly as the card's table reads." ], "recommendation": "A. Measured: with approved terminal a live duty has no correction path at all — not for its owner, not for a manager, not for an administrator (state_machine has no role exemption; the only remaining doors are a treatAsHistorical 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 then dispatches work every night with no way back into the pipeline that governs it. Returning is the auditable way out and is not a loophole — approval is still not self-issuable. One line to revert if you meant the table literally." }, { "question": "No Duly permission set can write `review_status` in a real deployment. Which half do you want opened?", "options": [ "A — grant `duly_member.duly_duty.allowEdit` (writeScope stays `own`) so an owner can confirm their own list. The bit is object-level, not per-field, so it also lets a member edit their own cadence — which src/security/permission-sets.ts refuses on purpose ('editing cadence after the fact is a correction, which belongs to duly_admin and to duly_catalog_sync').", "B — widen `duly_manager.duly_duty.writeScope` to `unit_and_below` so a manager can approve a report's duty. Contradicts an explicit ⛔ in that file and is pinned by test/security.test.ts; also brushes AGENTS.md's 'Managers do not enter status. Assigning is their only write.'", "C — both, which is what makes the workflow usable end to end by real users.", "D — neither for now: the demo runs as a platform admin and works, and the deployment question waits for a permission-model pass." ], "recommendation": "D for this card and C as the next one, filed separately. Real business need: the feature is p0 for a DEMO, and the demo account is a platform admin, so nothing is blocked this month; a deployment needs C or the pipeline is decoration. Long-term soundness: A and B each overturn a decision that is written down with its reasoning, and B's cost is not 'a manager writes duties' but that the object-level bit cannot say WHICH field — approving is indistinguishable from rewriting cadence on somebody else's duty. That is a permission-model question (field-level write permission, or an approval that is not an ordinary field write), not a line to slip into this PR. Hard-to-get-wrong: opening object edit to get one field is exactly the over-grant that never gets narrowed later. Startup scope: shipping the visible, demoable workflow now and deciding the permission model deliberately is the cheaper order." } ], "out_of_scope_findings": [ "filed as objectstack-ai/objectstack#14416: a gated select option logs one WARN per seeded row ('option visibleWhen … failed to evaluate — allowed through') because a system write can never bind current_user — 27 lines per demo boot, and the identical line is what a genuine unevaluable predicate produces for an authenticated caller", "not filed, already decided upstream: the record-page stage pipeline is a readout, not a control (objectui#5768 → objectui#5917, 2026-08-24). Recorded here because this card was written on the opposite assumption", "no change made, reported above as an open question: no Duly permission set can write review_status — duly_member has no allowEdit on duly_duty and every set's writeScope there is `own`" ] }Generated by Claude Code