Skip to content

fix(frontend): let approve-any holders approve scheduler-created rows under four-eyes - #1122

Merged
cristim merged 1 commit into
mainfrom
fix/233-frontend-system-row-approval
Oct 19, 2026
Merged

cristim merged 1 commit into
mainfrom
fix/233-frontend-system-row-approval

Conversation

@cristim

@cristim cristim commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Summary

Closes #233: the History UI's four-eyes gate now lets an approve-any holder approve rows the scheduler created.

#1071 shipped the backend half of the owner's Option B (2026-10-17). Rows created by the notification sweep carry created_by_system, and requireDifferentApprover lets an identified approver approve them. The frontend still treated every row without a creator as legacy. canApproveUnder4Eyes returned false, so approve-any holders saw "No recorded creator - an admin must disable 4-eyes mode" on rows the backend would accept.

Change

  • frontend/src/types.ts: optional created_by_system on HistoryPurchase. This mirrors the created_by_system,omitempty tag in internal/config/types.go that internal/api/handler_history.go projects.
  • frontend/src/history.ts: with four-eyes on, a row without a creator passes canApproveUnder4Eyes only when created_by_system === true. Rows with a creator still require a different approver. Four-eyes off is unchanged. The approve-own exclusion for system rows stays in rbacAllowsApprove, which already denies approve-own on rows without a creator, matching authorizeSessionApprove.
  • Tests: 4 new cases in four-eyes-approval.test.ts:
    • a system row plus approve-any can approve;
    • a system row plus approve-own only cannot;
    • a row without a creator that is not a system row stays denied, with the existing message;
    • four-eyes off is unchanged.

Verification (local, macOS)

  • RED before the fix: npx jest src/__tests__/four-eyes-approval.test.ts, 1 failed / 12 passed. GREEN after: 13/13.
  • Deletion probe: reverting the history.ts change makes the new approve-any test fail again.
  • npm test: 108 suites, 3400 passed, 1 skipped. npm run lint: 0 errors. npm run build: OK. Pre-commit hooks passed.

The backend still enforces approve-any constraints. This only stops the UI from hiding a valid action.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Users with approval-any permission can now approve scheduler-created purchases when four-eyes approval is enabled; these purchases no longer show a no-creator badge.
    • Users with only approval-own permission cannot approve scheduler-created purchases.
    • Purchases without a recorded creator continue to show the no-creator badge.

… under four-eyes

Refs #233

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added severity/medium Moderate harm urgency/this-sprint Within the current sprint triaged Item has been triaged priority/p1 Next up; this sprint impact/many Affects most users effort/s Hours type/bug Defect labels Oct 19, 2026
@cristim

cristim commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 970a9bc7-7874-4dce-85cc-cee00fb05527
📥 Commits

Reviewing files that changed from the base of the PR and between 519d8ee and 4375a74.

📒 Files selected for processing: 3
  • frontend/src/__tests__/four-eyes-approval.test.ts
  • frontend/src/history.ts
  • frontend/src/types.ts

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

The frontend now distinguishes scheduler-created rows from rows without a recorded creator. With four-eyes mode enabled, users with approval rights can approve scheduler-created rows. Tests cover approval buttons and badges in enabled and disabled modes.

Changes: System-created row approval

Layer / File(s) Summary
System-created row approval gate and rendering
frontend/src/types.ts, frontend/src/history.ts, frontend/src/__tests__/four-eyes-approval.test.ts
HistoryPurchase gains the optional created_by_system field. The four-eyes check permits system-created rows without a user creator. Tests cover approval buttons and badges for system-created rows and the no-creator badge for non-system rows.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 4375a

The History UI can expose approval for eligible scheduler-created rows. No issue identified in this review needs resolution before merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: allowing approve-any holders to approve scheduler-created rows when four-eyes approval is enabled.
Linked Issues check Passed Issue #233 requires the History UI to allow dual-control approval for scheduler-created rows and to keep legacy rows without a creator denied. frontend/src/types.ts adds created_by_system. `canApp…
Out of Scope Changes check Passed The diff is limited to the #233 behavior. It adds the API field type, updates the four-eyes approval gate, and adds focused regression tests. These changes directly support the linked issue and introd…
Full details: Linked Issues check

Explanation

Issue #233 requires the History UI to allow dual-control approval for scheduler-created rows and to keep legacy rows without a creator denied. frontend/src/types.ts adds created_by_system. canApproveUnder4Eyes permits a system row for an approve-any holder, still requires a different human creator for normal rows, and preserves four-eyes-off behavior. Existing rendering keeps the required no-creator message. The added tests cover approve-any, approve-own denial, legacy denial and message, and four-eyes-off behavior. The linked context identifies the required backend support as already shipped in #1071.

Full details: Out of Scope Changes check

Explanation

The diff is limited to the #233 behavior. It adds the API field type, updates the four-eyes approval gate, and adds focused regression tests. These changes directly support the linked issue and introduce no unrelated product behavior.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

Independent review A (Claude Sonnet, high, non-author): APPROVE 4375a74

  • Truth table: canApproveUnder4Eyes (history.ts:597-601) matches requireDifferentApprover (handler_purchases.go:1368-1411) in every case: mode off; no creator + system; no creator + not system; same creator; different creator.
  • approve-own on a system row: hidden by rbacAllowsApprove (history.ts:607-621). This matches the 403 from authorizeSessionApprove (handler_purchases.go:1076-1108).
  • "No recorded creator" badge: now shown only for rows without a creator that are not system rows.
  • Cancel path and other gates: unchanged and unaffected.
  • Mutation probes on four-eyes-approval.test.ts (13/13 baseline): six mutations of the change, each caught by 1-4 failures.
  • Non-blocking: no test covers created_by_system together with a set creator. The code handles that case like the backend, and the DB CHECK constraint prevents it from occurring.

@cristim

cristim commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

Independent review B (Claude Opus, high, non-author): APPROVE 4375a74

  • Field delivery: created_by_system reaches the UI through /api/history. Its only projection is executionToHistoryRow (handler_history.go:209, field set at :390). The store selects the column (store_postgres.go:1543) and scans it (:2102). The approval-queue card and the table both use renderPendingActionButtons (history.ts:932, :1837).
  • approve-own on system rows: denied on both sides (handler_purchases.go:1103-1105; history.ts:619-620).
  • Token and session paths: the email-token path calls requireDifferentApprover (:861), so anonymous holders stay denied. The manager also checks checkSystemCreatedApprover (approvals.go:263, :276).
  • Other frontend gates: dashboard, plans, RI exchange and deep-link views are unaffected. RI exchange passes false for system rows (handler_ri_exchange.go:2585).
  • Security: the History buttons only hint at what is allowed. The backend still enforces scope, approve-any limits and four-eyes.
  • Nit (non-blocking): only the approve-any case is RED without the fix. The other three new tests document unchanged behavior.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
cristim merged commit a2c2aac into main Oct 19, 2026
27 checks passed
@cristim
cristim deleted the fix/233-frontend-system-row-approval branch October 19, 2026 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours impact/many Affects most users priority/p1 Next up; this sprint severity/medium Moderate harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(purchase): scheduler-created executions have no creator and cannot pass four-eyes

1 participant