add session abort command and doctor recovery options#23
Merged
rsnodgrass merged 3 commits intomainfrom Feb 20, 2026
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduces a new session abort command to destructively terminate active recording sessions without uploading to the ledger, including command routing, confirmation enforcement, and integration with health checks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI Handler
participant State as Recording State
participant FS as File System
participant Output as JSON Output
User->>CLI: ox agent <id> session abort [--force]
CLI->>State: Load recording state
alt Recording is active
alt Non-interactive mode
alt --force flag present
CLI->>State: Clear recording state
CLI->>FS: Remove session directory
FS-->>CLI: Directory removed
CLI->>Output: Generate success result
Output-->>User: JSON output + session aborted
else --force flag missing
CLI-->>User: Error: destructive action requires --force
end
else Interactive mode
CLI->>User: Prompt for confirmation
User-->>CLI: Confirm/Deny
alt User confirms
CLI->>State: Clear recording state
CLI->>FS: Remove session directory
FS-->>CLI: Directory removed
CLI->>Output: Generate success result
Output-->>User: JSON output + session aborted
else User denies
CLI-->>User: Abort cancelled
end
end
else Recording is inactive
CLI-->>User: Error: no active session
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/ox/agent_doctor.go (1)
97-117:⚠️ Potential issue | 🔴 Critical
output.NextStepsfor the orphaned session is silently discarded.The orphaned-session step appended at lines 103–105 is immediately overwritten at line 117 because
buildNextStepscreates a freshvar steps []stringand never readsoutput.NextSteps. The new doctor guidance will never appear in JSON output.🐛 Proposed fix
- // build next steps based on state - output.NextSteps = buildNextSteps(output) + // build next steps based on state, preserving any steps already appended above + output.NextSteps = append(output.NextSteps, buildNextSteps(output)...)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/ox/agent_doctor.go` around lines 97 - 117, The orphaned-session message appended to output.NextSteps is being overwritten because buildNextSteps(output) builds a fresh steps slice and ignores output.NextSteps; update the logic so existing steps are preserved: either modify buildNextSteps (function buildNextSteps) to accept and merge any pre-existing output.NextSteps (or read output.NextSteps inside it) or, after calling buildNextSteps, append its returned steps to the already-populated output.NextSteps instead of replacing them; make the change where output.NextSteps is set/returned so the orphaned session message from session.IsRecording/session.LoadRecordingState is retained in the final output.
🧹 Nitpick comments (3)
cmd/ox/agent_session_abort.go (2)
13-20:sessionAbortOutputis missing aguidancefield.Per coding guidelines, agent behavioral guidance belongs in CLI JSON output under a
guidancefield, not in agent-specific skill files.♻️ Suggested addition
type sessionAbortOutput struct { Success bool `json:"success"` Type string `json:"type"` AgentID string `json:"agent_id"` SessionName string `json:"session_name,omitempty"` Message string `json:"message"` + Guidance string `json:"guidance,omitempty"` }Then set it when constructing the output:
output := sessionAbortOutput{ Success: true, Type: "session_abort", AgentID: inst.AgentID, SessionName: sessionName, Message: "session aborted and discarded", + Guidance: "Session data has been permanently deleted and was not uploaded to the ledger. Run 'ox agent prime' to start a new session.", }As per coding guidelines: "Place agent behavioral guidance in CLI JSON output (e.g.,
guidancefield), not in agent-specific skill files—skills should be thin wrappers relaying CLI output."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/ox/agent_session_abort.go` around lines 13 - 20, The sessionAbortOutput struct is missing the guidance field required by CLI JSON output; add a Guidance string `guidance` field to the sessionAbortOutput type and ensure any code that constructs or returns sessionAbortOutput (e.g., where sessionAbortOutput instances are created for abort responses) sets the Guidance value with the agent behavioral guidance text so the CLI emits guidance in JSON rather than embedding it in agent skill files.
99-106:hasFlagwon't match--force=truesyntax.
hasFlagdoes exact string comparison, so--force=true(a valid boolean flag form in some shells/tools) won't be recognized as--force. Since the only documented form is--forceand this is an agent-facing command, this is low-risk, but worth noting if cobra's pflag-style parsing is used elsewhere.♻️ More robust alternative using `strings.HasPrefix`
func hasFlag(args []string, flag string) bool { for _, arg := range args { - if arg == flag { + if arg == flag || strings.HasPrefix(arg, flag+"=") { return true } } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/ox/agent_session_abort.go` around lines 99 - 106, The hasFlag function currently does exact string equality and will miss forms like "--force=true"; update hasFlag (function name: hasFlag) to treat an argument as a match if it equals the flag OR if it has the flag followed by '=' (e.g., use strings.HasPrefix to check arg == flag || strings.HasPrefix(arg, flag+"=")) so both "--force" and "--force=true" are recognized; ensure you import "strings" if not already present.cmd/ox/agent_session_abort_test.go (1)
43-115: Tests should be table-driven per coding guidelines.The five tests cover distinct sub-scenarios (no recording, state cleared, folder removed, corrupt path, force required) that are well-suited to a table-driven structure. As per coding guidelines, test files should use table-driven tests.
As per coding guidelines, "
**/*_test.go: Use table-driven tests and test error paths in Go test files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/ox/agent_session_abort_test.go` around lines 43 - 115, The five separate tests (TestAbortNotRecording, TestAbortClearsRecordingState, TestAbortRemovesSessionFolder, TestAbortEmptySessionPathDoesNotDeleteCwd, TestAbortRequiresForce) should be consolidated into a single table-driven test named e.g. TestAbort with a slice of cases describing the scenario name, setup function or flags, expected error/side-effects, and any pre/post checks; inside the loop call t.Run(case.name, func(t *testing.T){ ... }) and invoke existing helpers (setupSessionTestProject, setupAbortTest, runAgentSessionAbort, session.IsRecording) per-case, performing the same assertions (error contains/NoError, IsRecording false, session folder removed via os.Stat, projectRoot exists for corrupt SessionPath, and cli.SetNoInteractive toggling for the "requires force" case) so behavior is preserved while following table-driven test guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/ox-session-abort.md:
- Line 4: The example command currently shows "$ox agent session abort --force"
which omits the required <agent_id> and will fail unless SAGEOX_AGENT_ID is set;
update the example to the canonical form "ox agent <agent_id> session abort
--force" (or show both forms and add a short note about SAGEOX_AGENT_ID) so
users can copy-paste a working command; modify the example string in the
ox-session-abort command doc and include the <agent_id> placeholder (and
optionally a one-line note about the env var) to prevent the "no agent ID"
error.
In `@cmd/ox/agent_session_abort_test.go`:
- Around line 85-102: The tests in this file (including
TestAbortEmptySessionPathDoesNotDeleteCwd and TestAbortNotRecording) should be
converted to a table-driven style: create a slice of test cases with names and
input setup, loop over them with t.Run and reuse the shared setup/teardown logic
(setupAbortTest, corruptState steps, and runAgentSessionAbort) to avoid
duplicated code. Replace manual defer os.Chdir(origDir) and any other
defer-based teardowns with t.Cleanup(...) calls so cleanup is consistent across
all cases, and ensure you capture and handle the error returned by os.Getwd()
(or require.NoError/ t.Fatalf when obtaining origDir) instead of ignoring it.
Keep assertions (e.g., os.Stat(projectRoot) check) per-case inside the table
loop so each subtest validates its own state.
---
Outside diff comments:
In `@cmd/ox/agent_doctor.go`:
- Around line 97-117: The orphaned-session message appended to output.NextSteps
is being overwritten because buildNextSteps(output) builds a fresh steps slice
and ignores output.NextSteps; update the logic so existing steps are preserved:
either modify buildNextSteps (function buildNextSteps) to accept and merge any
pre-existing output.NextSteps (or read output.NextSteps inside it) or, after
calling buildNextSteps, append its returned steps to the already-populated
output.NextSteps instead of replacing them; make the change where
output.NextSteps is set/returned so the orphaned session message from
session.IsRecording/session.LoadRecordingState is retained in the final output.
---
Duplicate comments:
In `@extensions/claude/commands/ox-session-abort.md`:
- Line 4: The documentation example for the CLI command is missing the required
agent identifier; update the usage and examples for the "ox agent session abort"
entry (ox-session-abort.md) to include the positional <agent_id> argument (e.g.
use "$ox agent session abort <agent_id> --force"), and ensure any surrounding
help text or examples mention and describe the <agent_id> parameter so callers
know it is required.
---
Nitpick comments:
In `@cmd/ox/agent_session_abort_test.go`:
- Around line 43-115: The five separate tests (TestAbortNotRecording,
TestAbortClearsRecordingState, TestAbortRemovesSessionFolder,
TestAbortEmptySessionPathDoesNotDeleteCwd, TestAbortRequiresForce) should be
consolidated into a single table-driven test named e.g. TestAbort with a slice
of cases describing the scenario name, setup function or flags, expected
error/side-effects, and any pre/post checks; inside the loop call
t.Run(case.name, func(t *testing.T){ ... }) and invoke existing helpers
(setupSessionTestProject, setupAbortTest, runAgentSessionAbort,
session.IsRecording) per-case, performing the same assertions (error
contains/NoError, IsRecording false, session folder removed via os.Stat,
projectRoot exists for corrupt SessionPath, and cli.SetNoInteractive toggling
for the "requires force" case) so behavior is preserved while following
table-driven test guidelines.
In `@cmd/ox/agent_session_abort.go`:
- Around line 13-20: The sessionAbortOutput struct is missing the guidance field
required by CLI JSON output; add a Guidance string `guidance` field to the
sessionAbortOutput type and ensure any code that constructs or returns
sessionAbortOutput (e.g., where sessionAbortOutput instances are created for
abort responses) sets the Guidance value with the agent behavioral guidance text
so the CLI emits guidance in JSON rather than embedding it in agent skill files.
- Around line 99-106: The hasFlag function currently does exact string equality
and will miss forms like "--force=true"; update hasFlag (function name: hasFlag)
to treat an argument as a match if it equals the flag OR if it has the flag
followed by '=' (e.g., use strings.HasPrefix to check arg == flag ||
strings.HasPrefix(arg, flag+"=")) so both "--force" and "--force=true" are
recognized; ensure you import "strings" if not already present.
This was referenced Feb 23, 2026
This was referenced Mar 3, 2026
3 tasks
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
1 participant
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
Add session abort capability for users to discard active sessions without uploading to the ledger, plus improved recovery confirmation prompts. Addresses the need for users to control when sessions are published.
New feature:
ox agent <id> session abort --forcediscards the active session, clearing all local session data. Interactive terminals get a y/N confirmation prompt; non-interactive callers (agents/pipes) require--force.Improved doctor: When orphaned/stale sessions are detected (>24h old), doctor now suggests both recovery and abort options, giving users explicit control.
Testing: 5 focused tests cover the abort lifecycle, non-interactive forced flow, empty-path safety guard, and confirmation behavior.
Test Plan
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests