F5 approval ladder · F7 signature record and execution formalities (card 06, M2) #16

Activity

1. hotlong

   added

   pm:queue

   pm:dispatched

   and removed

   pm:queue

   on Sep 7, 2026
2. hotlong

   self-assigned this

   on Sep 7, 2026
3. hotlong commented on Sep 7, 2026

   

   hotlong

   on Sep 7, 2026 – with Claude

   ContributorAuthor

   More actions

   Claim: PM loop round 5
   Session: session_01KcrVDXSptwDukFsHPHPR1V
   Branch: claude/issue-16-approval-ladder
   Worktree: hotclm-issue-16
   Domain: repo:hotclm (single-lane repo)
   File surface: src/flows/, plus objectstack.config.ts (the approvals + messaging tokens — declared in advance)
   Container & model: L, mode:subagent, model: opus (a five-rung conditional ladder with group sign-off; a wrong edge silently mis-routes an approval)
   Clause-②: no
   Serial constraints cleared: #13 merged as c2c895a, base is main @ c2c895a (11 objects, 1 flow, 1 action, 7 positions, 5 permission sets). No other in-flight claim. This card adds new files under src/flows/ and does not touch contract.hook.ts, which cards 02/03/05 all edited — no serial conflict. Decision card #11 question 2 (which sets hold approve_contract) is consumed by this card but does not block it: card 04's placement is implemented and a change is a one-line edit in src/profiles/.

   ---

   Generated by Claude Code
4. hotlong commented on Sep 7, 2026

   

   hotlong

   on Sep 7, 2026 – with Claude

   ContributorAuthor

   More actions

   os-dev-report

   {
     "issue": 16,
     "status": "done",
     "branch": "claude/issue-16-approval-ladder",
     "pr": "https://github.com/objectstack-ai/hotclm/pull/17",
     "premise_still_valid": true,
     "summary": "Continuation run: verified the inherited F5/F7 work in a real browser and fixed forward in one new commit (14329d6); the two inherited commits are untouched. Found and fixed a real defect in F7: the covered branch was gated on executed_at being empty, so a round whose formalities were ticked BEFORE the executed copy was uploaded — the exact wet-ink order DESIGN.md §06 F7 prescribes — stamped executed_at on the tick and short-circuited on 'already executed' forever after, so no final_signed version was ever filed and §03's signing→active guard could never be met, with executed_at readonly and no way back. The stamp, the version and the missing-copy notice now each carry their own idempotence key (a new get_final_version node reads the version live). Also corrected the F5 header: { type: 'manager' } resolves the CONTRACT OWNER's manager, not the submitter's — measured with a contract owned by a user with a manager and submitted by one without. Four of the four 'as measured' claims in 1ed2b79 re-measured and held (route flag storage/read, approvals ablation, messaging always-on, the rejected-contract 403/200 window); one claim in a098c68's header did not.",
     "tests": "Gates on the final commit, all through scripts/pm/os-verify-lock.sh: pnpm validate ✓ 'Validation passed (372ms)' Data 11 Objects 169 Fields · Logic 4 Flows · Security 7 Positions 5 Permissions; pnpm lint '5 suggestion(s)' exit 0, ALL approval-approvers-may-resolve-empty and NONE approval-approver-not-membership-tier (the card's acceptance); pnpm typecheck exit 0. VERDICT command-exit 0. BROWSER (pnpm dev --seed-admin, OS_PORT=3116, Chromium at /opt/pw-browsers/chromium-1194; boot clean — 36 plugins incl. ApprovalsServicePlugin/MessagingServicePlugin/RecordChangeTriggerPlugin, 'Flows: 4 flow(s) 3 bound to triggers', exactly ONE boot warning, the known ADR-0087 artifact conversion (#8)). Six users seeded, each approval taken by signing that person into the real approvals inbox at /_console/apps/setup/system/approvals and clicking Approve/Reject. FULL LADDER (1.2M purchase + accepted deviation, all four flags true): Mike POST .../areq_97d5.../approve 200 → legal_finance_joint opens pending on BOTH Lena and Frank; Lena approves ALONE → 200 and the page says 'Approved — waiting on the remaining approvers', request stays pending with pending_approvers=[Frank], NO new request (会签 confirmed); Frank approves → executive_signoff opens on Eva; Eva approves → gm_signoff opens on Grace; Grace approves → contract approved, approval_status approved, approved_at stamped, 4 requests. LOCKRECORD: PATCH /api/v1/data/clm_contract/ilqlq... during a pending rung → 409 RECORD_LOCKED for the platform admin AND for a pending approver; 200 after the ladder finished. REJECTION (own contract): Frank rejected at the 会签 rung while Lena had not acted → POST .../reject 200, request rejected, NO further rung opened, contract rejected/approval_status rejected/approved_at null, record unlocked (409→200), Lena's inbox cleared, owner notified. NO-FLAGS (NDA at 0): exactly ONE request, manager_review; approving it took the contract straight to approved — positive control is the full-ladder run above, plus a second control (1.2M NDA stamping route_gm only) that produced exactly manager_review + gm_signoff, rungs 2 and 3 skipped and rung 4 climbed in the same run. NOTIFY: sys_notification clm_contract_approved, both delivery rows (inbox+email) status success, owner's sys_inbox_message carries the interpolated title and body. F7 in the prescribed order: short → one notice naming 'Company seal'; formalities completed with no copy → executed_at stamped + one 'Executed copy missing'; unrelated edit → NO repeat notice; copy uploaded LAST → v2:final_signed filed (impossible before this commit); edit again → still exactly one. Copy-first path unchanged (v2:final_signed, is_current false, v1:clean still current); a contract stranded by the old shape recovers on its next touch; zero flow-run failures since the fix. Ladder re-driven in the browser on the rebuilt artifact (manager + GM, both 200, approved). ABLATIONS, each mutated in one shell with an EXIT trap, proven on disk by an occurrence count before/after, and restored with `git checkout HEAD -- objectstack.config.ts` verified by an empty `git diff HEAD`: (a) triggers removed → occurrences 2→1, pnpm validate EXIT=1 with three issues, one per record_change flow, verbatim as the comment quotes; (b) approvals removed → occurrences 1→0, boot 35 plugins with ApprovalsServicePlugin ABSENT, submit PATCH still 200, contract in_approval with approval_status not_required, NO sys_approval_request row, run error 'No executor registered for node type approval' (no NO_EXECUTOR: prefix — comment corrected); (c) messaging removed → occurrences 1→0, boot 36 plugins with MessagingServicePlugin still loaded and the notify still delivered to the owner's inbox (always-on confirmed). GM STRANDING (the lint decision): unstaffed clm_general_manager, drove a routed contract into gm_signoff → request OPENS with pending_approvers=['position:clm_general_manager'], both warnings in the log, ex-GM sees 'You're all caught up', PATCH 409 RECORD_LOCKED for the platform admin, and the recovery is real but API-only — platform admin POST .../approve 200 resumed the run to approved, non-approver non-admin 403 FORBIDDEN, while the Console renders NO override control on either the inbox All tab or the sys_approval_request record page.",
     "mcp_calls": "5 — issue_read(get), issue_read(get_comments), create_pull_request, pull_request_read(get), add_issue_comment. Channel switch declared: the repo-scoped REST probe in this container answered 403 (`GET /repos/objectstack-ai/hotclm`), so every GitHub write went through MCP instead of REST.",
     "open_questions": [],
     "out_of_scope_findings": [
       "noted, not filed: the #3424 admin override has no Console control — the approvals service admits a platform/tenant admin on a pending request and the REST call works, but neither the inbox All tab nor the sys_approval_request record page renders approve/reject/reassign for the override actor, so on an app shipping lockRecord:true a tenant without API access cannot release a stranded record. AGENTS.md routes platform gaps to objectstack-ai/objectstack; not filed because the required duplicate search is in a repo outside this card's lane — recommend the PM seat file it there.",
       "noted, not filed: sys_user.manager_id has no product write surface. ADR-0092 makes the data API refuse it ('Editable fields: name, image, locale') and the auth admin endpoints do not accept it either, so it is system-write-only (SCIM/import). F5's first rung is that column for EVERY contract, so an install with no directory sync has a first rung that resolves to nobody on every card. Set directly in the datastore for this verification, which is what a sync would do.",
       "noted, not filed: a pending approver may not be able to open the record they are approving — Frank (clm_finance_controller) can see and decide the 会签 request, but the record link in the same row is a 403 for him because §04 grants finance edit only from approved onward. The decision works (the service reads under system identity) but he decides on the row summary alone. Whether §04 should grant a routed approver a read is a maintainer product question, not a flow bug.",
       "noted, not filed: sys_file rows cannot be referenced by two records (ERR_FILE_REFERENCE_COPY). Hit while seeding; recorded only so the next person seeding contract versions mints one file per record."
     ]
   }

   ---

   Generated by Claude Code
5. hotlong commented on Sep 7, 2026

   

   hotlong

   on Sep 7, 2026 – with Claude

   ContributorAuthor

   More actions

   ACCEPT — PM review, round 5 (continuation run)

   Reviewed against GitHub. PR #17, head 14329d6.

   Item	Verdict
   PR shape	draft, base main, Fixes #16 ✓
   CI	Validate green on 14329d6 ✓
   File surface	4 files — src/flows/ plus objectstack.config.ts, the surface the claim declared ✓
   Gates	validate ✓ (4 flows) · lint ✓ (5 suggestions, no approval-approver-not-membership-tier — the card's acceptance) · typecheck ✓
   History	the two inherited commits untouched, fixed forward in 14329d6 ✓

   The headline is the defect it found in F7, and it is the kind that ships silently.

   The covered branch was keyed on executed_at being empty. So a round whose formalities were ticked before the executed copy was uploaded — which is the exact wet-ink order §06 F7 prescribes ("法务或档案岗在签署记录上传执行副本并勾选形式") — stamped executed_at on the tick and then short-circuited on "already executed" on every later run. No final_signed version was ever filed, §03's signing → active guard could therefore never be met, and executed_at is readonly, so the contract was permanently stuck with no way back. On the prescribed path.

   The fix gives each of the three outcomes its own idempotence key, with a new node reading the version live rather than inferring it from the stamp. The reasoning is committed in the code, not just the PR body, including which key it is and why that choice is what broke it. A contract already stranded by the old shape recovers on its next touch.

   One inherited claim corrected. { type: 'manager' } resolves the contract owner's manager, not the submitter's — measured with a contract owned by a user with a manager and submitted by one without. That is a routing difference, not a wording nit. Of the four "as measured" claims in the docs commit, all four held; one in the feature commit's header did not. Checking rather than inheriting is the whole reason continuation runs review first.

   Verification is the deepest this repo has seen. Six users seeded, and every approval taken by signing that person into the real approvals inbox and clicking the button — not by poking the service.

   • Full ladder: manager → 会签 opening pending on both legal and finance → Lena approves alone → "Approved — waiting on the remaining approvers", request still pending with pending_approvers=[Frank], no new request → Frank → executive → GM → approved. That middle reading is 会签 proven, not assumed.
   • Rejection: taken at the 会签 rung while the other approver had not acted. No further rung opened, contract rejected, record unlocked (409 → 200), the other inbox cleared, owner notified.
   • lockRecord: 409 RECORD_LOCKED during a pending rung for the platform admin and for a pending approver; 200 after.
   • Skipped rungs, with the discriminating control: an NDA at 0 produced exactly one request; and a 1.2M NDA stamping only route_gm produced exactly manager_review + gm_signoff — rungs 2 and 3 skipped and rung 4 climbed in the same run. A skipped rung and a dead flow look identical from outside; that single run separates them.

   Three ablations, each done properly: mutated in one shell under an EXIT trap, proven on disk by an occurrence count before and after, restored and the restoration verified by an empty git diff HEAD.

   • triggers removed → validate exit 1, three issues, one per record-change flow.
   • approvals removed → 35 plugins, service absent, submit still 200 and the contract sitting in in_approval with approval_status: not_required and no approval request row at all. The run error also corrected a comment that had claimed a NO_EXECUTOR: prefix.
   • messaging removed → still loaded, notify still delivered. The token is not load-bearing and it says so: kept because the app must state what it means, with the explicit note that no gate catches its absence. Declaring intent while recording that the gate has no teeth is the honest shape; cargo-culting it as "required" would not be.

   The gm_signoff decision was driven out rather than argued. Unstaffed the position, drove a routed contract into the rung: the request opens with pending_approvers=['position:clm_general_manager'], the record is 409-locked, and recovery is real but API-only — a platform admin's approve resumes the run, a non-approver non-admin gets 403, and the Console renders no override control on either the inbox or the request record. So the hazard is bounded for anyone with API access and unbounded for a tenant without it. That is the right answer to a lint suggestion: measure the failure mode, then decide.

   Two operational findings worth more than the code.

   sys_user.manager_id has no product write surface — the data API refuses it under ADR-0092 and the auth admin endpoints do not accept it either. F5's first rung is that column for every contract, so an install without directory sync has a first rung resolving to nobody, every time. This one is going upstream.

   And a pending approver may not be able to open the record they are deciding: finance can see and decide the 会签 request but gets 403 on the record link, because §04 grants finance read only from approved onward. The decision works — the service reads under system identity — but they decide on the row summary alone. Whether a routed approver should get a read is a product question for the maintainer, not a flow bug.

   Merging: CI green, ACCEPT recorded, no governed surface touched.

   ---

   Generated by Claude Code
6. hotlong

   closed this as completedin #17on Sep 7, 2026