feat: persist TUI sessions#69
Conversation
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR adds session state tracking and event persistence to the TUI, enabling transcript reconstruction across multiple agent runs via a ChangesSession state tracking and resume flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/tui/session_test.go`:
- Line 164: The test contains an ineffectual assignment to updated from
next.Update(cmd()) that is never used; replace the assignment with the blank
identifier so the return values are ignored (e.g. use "_, _ =
next.Update(cmd())" or otherwise handle the error) for the call in
session_test.go (reference: updated, next.Update, cmd()); apply the same change
to the other occurrence noted around line 354.
In `@internal/tui/session.go`:
- Line 133: Change the capitalized, punctuated error strings to idiomatic Go by
lowercasing the first letter and removing the trailing period in the errors.New
calls; specifically replace errors.New("No Zero sessions available to resume.")
(and the similar errors.New(...) at the other occurrence around line 143) with
errors.New("no zero sessions available to resume") in internal/tui/session.go so
both returned errors follow ST1005 style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 22be84f2-a792-4e26-bd19-707d41f7df12
📒 Files selected for processing (5)
internal/tui/model.gointernal/tui/model_test.gointernal/tui/session.gointernal/tui/session_controls_test.gointernal/tui/session_test.go
gnanam1990
left a comment
There was a problem hiding this comment.
Thanks, this slice is structurally solid and the local verification passed for me:
go test -count=1 ./internal/tuigo test -count=1 ./internal/sessions ./internal/tuigo test -count=1 -p 1 ./...
Requesting changes only for the two current CodeRabbit findings, which I verified are still valid on 78e8c50:
internal/tui/session_test.go: replace the two unusedupdated, _ = next.Update(cmd())assignments with ignored return values or consume the returned model.internal/tui/session.go: normalize the returned error strings to idiomatic Go style by lowercasing and removing trailing punctuation.
No additional blockers found from my local review.
gnanam1990
left a comment
There was a problem hiding this comment.
Re-reviewed the updated head 32e66f9.
The previously requested fixes are in place:
- the two unused
next.Update(cmd())assignments now ignore returned values - resume errors now use idiomatic lowercase/no-period Go strings
Local validation passed:
go test -count=1 ./internal/tuigo test -count=1 -p 1 ./...
No further blockers from me.
Summary
zero exec/resume <id>and/resume latestinto the visible TUI transcript/resumeduring active runs and record cancellation markers to avoid corrupt or orphaned session historyTests
go test ./...bun run typecheckbun test ./tests --timeout 15000bun run buildNotes
Summary by CodeRabbit
New Features
/resumecommand to restore and continue previous sessions (session ID orlatest), rebuilding transcripts with past eventsBug Fixes / Behavior
/resumeis blocked while a run is active; cancelling a run records a session errorTests