fix: resume built-in Pi sessions#1197
Open
vilmos-brainco wants to merge 1 commit into
Open
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a buildPiCommand helper that creates a per-instance ${HOME}/.pi/agent-deck/ session directory, exports AGENTDECK_INSTANCE_ID, and launches pi with --continue --session-dir. Start, StartWithMessage, and Restart fallback route pi tool invocations through the builder; CanRestart now returns true for waiting pi instances. Tests assert command construction, quoting, passthrough for non-pi tools, and restart eligibility. ChangesPi Tool Session Lifecycle Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Owner
|
Thanks @vilmos-brainco — clean addition. Instance-scoped Pi session dirs ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/session/command_override_test.go (1)
194-196: ⚡ Quick winTighten quote assertion to validate the exact quoted path.
This assertion can pass even if the command quotes the wrong directory. Assert the full expected
--session-dir '<full-path>'(and ideallymkdir -p '<full-path>') to make the regression test strict.Proposed test hardening
- if !strings.Contains(got, "--session-dir '") { - t.Errorf("buildPiCommand() should shell-quote session dir with spaces, got %q", got) - } + wantSessionDir := tmpDir + "/.pi/agent-deck/test-instance-id" + if !strings.Contains(got, "--session-dir '"+wantSessionDir+"'") { + t.Errorf("buildPiCommand() should include quoted session dir %q, got %q", wantSessionDir, got) + } + if !strings.Contains(got, "mkdir -p '"+wantSessionDir+"'") { + t.Errorf("buildPiCommand() should include quoted mkdir path %q, got %q", wantSessionDir, got) + }🤖 Prompt for 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. In `@internal/session/command_override_test.go` around lines 194 - 196, Tighten the test by asserting the exact quoted path instead of a substring: build the expected strings using the test's sessionDir variable and require that got contains "--session-dir '<sessionDir>'" (including single quotes) and also contains "mkdir -p '<sessionDir>'" to ensure the command quotes the correct directory and creates it; update the assertions in the buildPiCommand() test to check these full expected substrings rather than just "--session-dir '".
🤖 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/session/instance.go`:
- Around line 1340-1354: The code currently resolves the host's user home with
os.UserHomeDir and bakes an absolute path into the command; change
buildPiCommand (the code that constructs sessionDir/quotedSessionDir and returns
the mkdir+command string) to use the target-side $HOME literal instead of
resolving the host home: build sessionDir from the literal "$HOME" (e.g.
filepath.Join("$HOME", ".pi", "agent-deck", i.ID)) and then shell-quote that
string and i.ID as before so the generated command uses $HOME on the
remote/target runtime and preserves Local/Remote parity.
---
Nitpick comments:
In `@internal/session/command_override_test.go`:
- Around line 194-196: Tighten the test by asserting the exact quoted path
instead of a substring: build the expected strings using the test's sessionDir
variable and require that got contains "--session-dir '<sessionDir>'" (including
single quotes) and also contains "mkdir -p '<sessionDir>'" to ensure the command
quotes the correct directory and creates it; update the assertions in the
buildPiCommand() test to check these full expected substrings rather than just
"--session-dir '".
🪄 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: 15b181f5-b29c-4adf-9b59-57d955ce358d
📒 Files selected for processing (2)
internal/session/command_override_test.gointernal/session/instance.go
vilmos-brainco
force-pushed
the
pi-builtin-resume
branch
from
6e720ed to
9fa34a9
Compare
vilmos-brainco
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--continue --session-dir ~/.pi/agent-deck/<instance-id>so Pi restarts resume the same Agent Deck session without cwd-level collisionsVerification
go test ./internal/session -run 'TestBuildPiCommand|TestCanRestartPi' -count=1make build VERSION=1.9.36-pi-resume-prpibinary through a real tmux session, verifyingAGENTDECK_INSTANCE_IDand--continue --session-dir ~/.pi/agent-deck/<instance-id>Summary by CodeRabbit
New Features
Tests