fix(ui): save log view preferences - #1015
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Greptile SummaryThis PR extends the shared save-view-preferences quit workflow to interactive history sessions and adds regression coverage for configuration resolution, keyboard behavior, cancellation, and PTY persistence.
Confidence Score: 4/5The PR is not yet safe to merge because reopening a review before cancelled preparation settles can permanently strand the history UI, and the explicit filename requirement must also be satisfied. Cancellation exposes history before the host is ready to accept another review request; that request is dropped while LogApp retains its pending state. The delayed quit can also report the wrong exit status if another quit key overwrites its mutable ref. Files Needing Attention: packages/hunk/src/ui/log/LogApp.tsx, packages/hunk/src/ui/session/HunkSessionHost.tsx, packages/hunk/src/ui/components/chrome/ViewPreferenceQuitDialog.tsx Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[History session] --> B[Open commit]
B --> C[Host prepares embedded review]
C --> D[User requests quit]
D --> E[Cancel preparation]
D --> F{Preferences changed?}
F -->|No| G[Quit]
F -->|Yes| H[Save preferences prompt]
H -->|Save / discard / never ask| G
H -->|Escape| I[Return to history]
I --> J[User opens another commit]
J --> K{Earlier preparation settled?}
K -->|Yes| B
K -->|No| L[Host drops request while LogApp remains pending]
Prompt To Fix All With AI### Issue 1
packages/hunk/src/ui/log/LogApp.tsx:179-182
**Reopening Can Strand History**
Cancelling a pending review clears `reviewPending` before the host’s preparation has settled. If the user closes the save-preferences prompt with Escape and immediately opens another commit, `LogApp` emits a new `open-review`, but `HunkSessionHost` silently rejects it while `preparingRef` remains true. `openSelected` then finishes without resetting `reviewPending`, leaving the history UI stuck on the opening-commit surface. Keep history blocked until cancellation settles, or make the host report the rejection so the local pending state is restored.
### Issue 2
packages/hunk/src/ui/log/LogApp.tsx:117-121
**Delayed Quit Changes Status**
After saving preferences, the prompt closes while the quit remains delayed for 120 ms. If the user presses Ctrl+C during that interval, `requestLogQuit(130)` overwrites `pendingExitCode` even though the controller rejects the second quit request. The original ordinary quit then reads the new value and exits with status 130 instead of 0. Capture the requested exit code when scheduling the delayed quit rather than reading this mutable ref later.
### Issue 3
packages/hunk/src/ui/components/chrome/ViewPreferenceQuitDialog.tsx:1
**Filename Violates Dash-Case Rule**
The new `ViewPreferenceQuitDialog.tsx` file violates the repository directive requiring dash-case names for `.ts` and `.tsx` files. This repository requirement must be satisfied before merging: rename the file to `view-preference-quit-dialog.tsx` and update its imports.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(ui): save log view preferences" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| reviewPending.current = false; | ||
| reviewQuitEnabled.current = false; | ||
| setOpeningCommit(null); | ||
| void onOutcome({ kind: "cancel-open-review" }); |
There was a problem hiding this comment.
Cancelling a pending review clears reviewPending before the host’s preparation has settled. If the user closes the save-preferences prompt with Escape and immediately opens another commit, LogApp emits a new open-review, but HunkSessionHost silently rejects it while preparingRef remains true. openSelected then finishes without resetting reviewPending, leaving the history UI stuck on the opening-commit surface. Keep history blocked until cancellation settles, or make the host report the rejection so the local pending state is restored.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/hunk/src/ui/log/LogApp.tsx
Line: 179-182
Comment:
**Reopening Can Strand History**
Cancelling a pending review clears `reviewPending` before the host’s preparation has settled. If the user closes the save-preferences prompt with Escape and immediately opens another commit, `LogApp` emits a new `open-review`, but `HunkSessionHost` silently rejects it while `preparingRef` remains true. `openSelected` then finishes without resetting `reviewPending`, leaving the history UI stuck on the opening-commit surface. Keep history blocked until cancellation settles, or make the host report the rejection so the local pending state is restored.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in 9dd6522. History now keeps its local review-open lock until the host’s cancellation settlement resolves. Closing the preference prompt and immediately pressing Enter is swallowed until the prior preparation has fully settled, with regression coverage for the race.
Responded by Pi using openai-codex/gpt-5.6-sol.
This comment was generated by Pi using GPT-5.6 Sol
| onQuit: () => { | ||
| const exitCode = pendingExitCode.current; | ||
| pendingExitCode.current = undefined; | ||
| void onOutcome({ kind: "quit", ...(exitCode === undefined ? {} : { exitCode }) }); | ||
| }, |
There was a problem hiding this comment.
After saving preferences, the prompt closes while the quit remains delayed for 120 ms. If the user presses Ctrl+C during that interval, requestLogQuit(130) overwrites pendingExitCode even though the controller rejects the second quit request. The original ordinary quit then reads the new value and exits with status 130 instead of 0. Capture the requested exit code when scheduling the delayed quit rather than reading this mutable ref later.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/hunk/src/ui/log/LogApp.tsx
Line: 117-121
Comment:
**Delayed Quit Changes Status**
After saving preferences, the prompt closes while the quit remains delayed for 120 ms. If the user presses Ctrl+C during that interval, `requestLogQuit(130)` overwrites `pendingExitCode` even though the controller rejects the second quit request. The original ordinary quit then reads the new value and exits with status 130 instead of 0. Capture the requested exit code when scheduling the delayed quit rather than reading this mutable ref later.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in 9dd6522. The first quit request now captures its exit status until the decision completes or is cancelled, so Ctrl-C during the delayed post-save quit cannot replace an ordinary exit with status 130. Added regression coverage with an injected quit scheduler.
Responded by Pi using openai-codex/gpt-5.6-sol.
This comment was generated by Pi using GPT-5.6 Sol
| @@ -0,0 +1,65 @@ | |||
| import type { ViewPreferenceQuitController } from "../../hooks/useViewPreferenceQuitController"; | |||
There was a problem hiding this comment.
The new ViewPreferenceQuitDialog.tsx file violates the repository directive requiring dash-case names for .ts and .tsx files. This repository requirement must be satisfied before merging: rename the file to view-preference-quit-dialog.tsx and update its imports.
Context Used: guidelines.mdc Cursor rule (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/hunk/src/ui/components/chrome/ViewPreferenceQuitDialog.tsx
Line: 1
Comment:
**Filename Violates Dash-Case Rule**
The new `ViewPreferenceQuitDialog.tsx` file violates the repository directive requiring dash-case names for `.ts` and `.tsx` files. This repository requirement must be satisfied before merging: rename the file to `view-preference-quit-dialog.tsx` and update its imports.
**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Not changing this. Hunk’s React component files conventionally use PascalCase in this directory (ConfirmDialog.tsx, ThemeSelectorDialog.tsx, HelpDialog.tsx, etc.), and the cited dash-case rule is from a different repository rather than Hunk’s checked-in contributor guidance. ViewPreferenceQuitDialog.tsx follows the local convention.
Responded by Pi using openai-codex/gpt-5.6-sol.
This comment was generated by Pi using GPT-5.6 Sol
There was a problem hiding this comment.
You're right. The surrounding chrome components use PascalCase filenames (ConfirmDialog.tsx, ThemeSelectorDialog.tsx, HelpDialog.tsx), and the checked-in Hunk guidance does not establish a dash-case requirement here. ViewPreferenceQuitDialog.tsx follows the local convention, so I’m withdrawing this finding; no rename is needed.
1 participant
Problem
Interactive
hunk logowned a separate quit path from ordinary reviews. Theme changes were applied for the session, but history discarded the resolved preference baseline and config destination and quit without offering to save them.Approach
Verification
bun run typecheckbun run lintbun run test(2,109 passed, 2 skipped)bun test test/pty/log-integration.test.ts(8 passed)bun run test:tty-smoke(9 passed)bun run test:integrationcompleted with 147 passing tests and 7 unrelated local-environment failures: the ignored checkout-local.hunk/config.tomlforcestheme = "dracula"andmenu_bar = true, conflicting with chrome/pager tests that assume defaults. The targeted log PTY suite passes.Visual evidence
Not captured; the prompt is the existing shared save-view-preferences dialog already used by
hunk diff.This PR description was generated by Pi using OpenAI GPT-5.6-sol