Fix plan-approval mode not sticking across app restarts#3737
Conversation
Settings persist asynchronously (an IPC round trip on desktop), so
PlanApprovalSelector can mount before lastPlanApprovalMode has loaded
from disk -- e.g. resuming a task with an already-pending plan
approval. Since useState's initializer only runs once, the selector
locked onto the pre-hydration fallback ("auto") and never picked up
the real remembered mode once hydration completed.
Re-sync the selected mode once the settings store finishes
hydrating, unless the user has already picked a mode themselves.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
haacked
added
the
Stamphog
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/ui/src/features/permissions/PlanApprovalSelector.tsx:145-147
**Interaction Guard Stays Set Across Requests**
If this component remains mounted while its `options` change for a later approval request, a mode selected during the first request leaves `userChangedModeRef` permanently true. The effect then ignores the new `initialMode`, so the later request can retain a stale mode or show the fallback selector when that mode is no longer available.
Reviews (1): Last reviewed commit: "Fix plan-approval mode not sticking acro..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (hasHydrated && !userChangedModeRef.current) { | ||
| setSelectedMode(initialMode); | ||
| } |
There was a problem hiding this comment.
If this component remains mounted while its options change for a later approval request, a mode selected during the first request leaves userChangedModeRef permanently true. The effect then ignores the new initialMode, so the later request can retain a stale mode or show the fallback selector when that mode is no longer available.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/permissions/PlanApprovalSelector.tsx
Line: 145-147
Comment:
**Interaction Guard Stays Set Across Requests**
If this component remains mounted while its `options` change for a later approval request, a mode selected during the first request leaves `userChangedModeRef` permanently true. The effect then ignores the new `initialMode`, so the later request can retain a stale mode or show the fallback selector when that mode is no longer available.
How can I resolve this? If you propose a fix, please make it concise.![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contained UI fix for a settings-hydration race in the plan-approval mode selector, with unit tests covering both the hydration re-sync and the don't-clobber-user-choice case; not in risky territory (no auth/billing/API/CI/dependency/migration surface touched).
- greptile-apps[bot] reviewed the current head.
- Greptile flagged (P2, non-blocking) that if this component stays mounted across multiple sequential approval requests, the userChangedModeRef guard could keep blocking re-sync for a later request — a pre-existing limitation this PR doesn't worsen for the common case, but worth a follow-up if the component is ever reused without remounting between requests.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 19L, 1F substantive, 73L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (73L, 2F, single-area, unknown) |
| stamphog 2.0.0b3 | .stamphog/policy.yml @ 744d66a · reviewed head 505456a |
1 participant
Problem
Settings persist asynchronously (an IPC round trip on desktop), so PlanApprovalSelector can mount before lastPlanApprovalMode has loaded from disk, such as when resuming a task with an already-pending plan approval. Since useState's initializer only runs once, the selector locked onto the pre-hydration fallback of "auto" and never picked up the real remembered mode once hydration finished, so the remembered plan-approval mode didn't stick across app restarts.
Changes
Re-sync the selected mode once the settings store finishes hydrating, unless the user has already picked a mode themselves. A
userChangedModeReftracks explicit user selection so the re-sync doesn't clobber a choice made before hydration lands.How did you test this?
Added two tests covering the hydration race: one confirms the selector re-syncs to the remembered mode once hydration lands after mount, and one confirms an explicit user selection made before hydration lands isn't overwritten by it.
Automatic notifications