Skip to content

feat: distinct "Approved" state for review-finish modal#381

Merged
tomasz-tomczyk merged 2 commits intomainfrom
approved-modal-redesign
Apr 29, 2026
Merged

feat: distinct "Approved" state for review-finish modal#381
tomasz-tomczyk merged 2 commits intomainfrom
approved-modal-redesign

Conversation

@tomasz-tomczyk
Copy link
Copy Markdown
Owner

@tomasz-tomczyk tomasz-tomczyk commented Apr 29, 2026

Summary

The post-finish modal showed a dot spinner + "Your agent has been notified. Waiting for updates…" in every case, including pure approvals where no further work is expected. The server already returns approved: bool from /api/finish, but the frontend was ignoring it and inferring "is the agent expected to do work?" from prompt presence — so the resolve-all-then-finish path was misclassified and showed the misleading waiting copy.

Now the modal has two distinct states driven by data.approved:

  • Approved — animated SVG checkmark drawn into a green ring (one-shot, no looping), heading "Approved", conclusive copy. Prompt block + Copy button hidden; the prompt is still auto-copied to the clipboard in doFinishReview, so the manual-paste fallback is preserved silently.
  • Waiting — unchanged: dot spinner, "Review Complete", prompt + Copy button visible.

Review

  • Code review: passed (frontend-expert, no blockers)
  • gofmt / golangci-lint / go test -race: clean
  • All 35 round-complete E2E tests pass

Test plan

  • Light + dark theme rendering verified via Playwright screenshots
  • e2e/tests/round-complete.spec.ts and round-complete.filemode.spec.ts updated to assert the new .approved class + heading
  • Existing waiting-state assertions still pass (test for "finish review with comments shows prompt" unchanged behavior)

🤖 Generated with Claude Code

The post-finish modal showed a dot spinner and "Your agent has been
notified. Waiting for updates..." in every case — including approvals
where no further work is expected. Drives confusion when a plan is
approved with no changes needed.

Server already returns data.approved from /api/finish; switch the
frontend to read it instead of inferring from prompt presence. When
approved, the modal now shows an animated SVG checkmark drawn into a
green ring (one-shot, no looping), heading "Approved", and conclusive
copy. Prompt block + Copy button are hidden — the prompt is auto-copied
to clipboard in doFinishReview, so the manual-paste fallback is
preserved silently.

Waiting state (data.approved=false) is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.59%. Comparing base (f79d9f9) to head (6eb7364).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #381      +/-   ##
==========================================
+ Coverage   66.58%   66.59%   +0.01%     
==========================================
  Files          19       19              
  Lines        8176     8176              
==========================================
+ Hits         5444     5445       +1     
+ Misses       2310     2309       -1     
  Partials      422      422              
Flag Coverage Δ
e2e 33.94% <ø> (ø)
unit 62.61% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tomasz-tomczyk tomasz-tomczyk merged commit 649c55b into main Apr 29, 2026
5 of 6 checks passed
@tomasz-tomczyk tomasz-tomczyk deleted the approved-modal-redesign branch April 29, 2026 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant