Repository navigation
fix: restore local session result exports - #534
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 38 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe HTTP server adds runtime-event-based result reconstruction for missing tape and prefers local Git computation for workspace diff/patch operations when the session targets a local path, with corresponding contract tests validating both code paths and fallback to remote export. ChangesSession Result Recovery and Local Workspace Operations
Sequence Diagram(s)sequenceDiagram
participant Client
participant GetSessionResult
participant LatestTurnCheck
participant RuntimeEventReplay
participant TurnResultBuilder
Client->>GetSessionResult: GET /sessions/{id}/result
GetSessionResult->>LatestTurnCheck: Check if latest_turn exists
alt latest_turn is None
LatestTurnCheck->>RuntimeEventReplay: Replay persisted runtime events
RuntimeEventReplay->>RuntimeEventReplay: Parse StreamDelta, ToolCallDelta
RuntimeEventReplay->>TurnResultBuilder: Build content_parts, ToolCallRecords
TurnResultBuilder->>GetSessionResult: Return reconstructed TurnResult
else latest_turn exists
LatestTurnCheck->>GetSessionResult: Return existing turn_result
end
GetSessionResult->>Client: Return result with final output & verification summary
sequenceDiagram
participant Client
participant WorkspaceDiffEndpoint
participant LocalGitCheck
participant LocalGitUtil
participant CloudExportFallback
Client->>WorkspaceDiffEndpoint: GET /sessions/{id}/workspace/diff
WorkspaceDiffEndpoint->>LocalGitCheck: Check session.default_run_target
alt LocalPathWorkspaceRef available
LocalGitCheck->>LocalGitUtil: _local_workspace_diff (via asyncio.to_thread)
LocalGitUtil->>LocalGitUtil: Validate Git repo, run git diff --cached
LocalGitUtil->>WorkspaceDiffEndpoint: Return WorkspaceDiff
else No LocalPathWorkspaceRef
LocalGitCheck->>CloudExportFallback: export_workspace_archive fallback
CloudExportFallback->>WorkspaceDiffEndpoint: Return remote diff
end
WorkspaceDiffEndpoint->>Client: Return WorkspaceDiff or patch JSON
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
delete-cloud
force-pushed
the
fix/o6n-product-verification
branch
from
June 4, 2026 06:22
9a8eab1 to
816c209
Compare
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
coding-agent/src/coding_agent/server/http_server.py (1)
6016-6037:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPopulate
turn_idfrom the reconstructed runtime run.When this endpoint rebuilds
turn_resultfrom persisted runtime events after a restart, Line 6031 still returnssession.current_turn_id. That leaves callers with a recoveredfinal_answerbut a nullturn_id, so they cannot correlate the result back to/runs/{id}.🤖 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 `@coding-agent/src/coding_agent/server/http_server.py` around lines 6016 - 6037, The response uses session.current_turn_id even when turn_result was reconstructed from runtime events, leaving turn_id null for recovered runs; update the SessionResultResponse construction so turn_id is taken from the reconstructed result when available (e.g., use turn_id = session.current_turn_id if turn_result is None else turn_result.run_id or turn_result.turn_id depending on the attribute on the TurnResult object). Modify the block that builds SessionResultResponse (where turn_result, verification_summary are computed) to prefer the turn identifier from turn_result (e.g., turn_result.run_id / turn_result.turn_id) and fall back to session.current_turn_id.
🤖 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 `@coding-agent/src/coding_agent/server/http_server.py`:
- Around line 5739-5747: The current _session_result_from_runtime_events
function swallows KeyError from
session_manager.load_runtime_run/replay_runtime_events and returns None, which
hides storage failures; change this to not collapse missing persisted runtime
data into a null result by removing the KeyError except-return-None behavior (or
re-raising the caught KeyError) so that failures from
session_manager.load_runtime_run and session_manager.replay_runtime_events
propagate (causing a 500) rather than returning None; locate symbols
_session_result_from_runtime_events, _session_result_runtime_run_id,
session_manager.load_runtime_run, and session_manager.replay_runtime_events to
implement this change.
- Around line 5941-5977: The numstat invocation is not rename-aware or
null-separated, causing mismatches with the rename-aware name-status output;
update _local_workspace_diff to call _run_local_workspace_git for numstat with
the same rename and NUL flags as name-status (include "--find-renames" and "-z"
in the args and adjust the description string), and update
_parse_local_git_numstat to handle NUL-separated output (split on "\0" when NUL
mode is used, then parse each record by tab as before) so additions/deletions
are keyed to the same new path names as _parse_local_git_name_status.
- Around line 5807-5857: The current flow runs git with ambient config and
executes "git add -A" which can invoke repo-defined clean filters and external
diff/textconv drivers; change _run_local_workspace_git to always set
GIT_CONFIG_GLOBAL=/dev/null and GIT_CONFIG_SYSTEM=/dev/null (in addition to
GIT_TERMINAL_PROMPT) to reduce global/system influence, and ensure any git diff
invocations (in _local_workspace_diff / _local_workspace_patch) include
--no-ext-diff and --no-textconv when calling git diff --cached; remove/replace
the unsafe git add -A in _prepare_local_git_result_index (do not call
_run_local_workspace_git with ["add","-A"]) and instead populate the temporary
index using a non-filter-invoking plumbing command such as git update-index
--refresh or writing the index via git read-tree/read-cache or git update-index
--index-info so that clean filters are not executed (modify
_prepare_local_git_result_index to use one of these alternatives and pass the
same temporary GIT_INDEX_FILE env).
In `@coding-agent/tests/ui/test_http_server.py`:
- Around line 6279-6306: The subprocess.run calls in
test_workspace_diff_returns_local_path_git_changes (and the sibling test around
lines 6343-6370) call "git" as a bare command, triggering Ruff S607/S603;
resolve by locating the absolute git executable once (e.g., git_path =
shutil.which("git") and assert it's not None) and replace subprocess.run(["git",
...]) invocations with subprocess.run([git_path, ...]) throughout those tests
(also update any other helper calls in the same tests to use git_path) so PATH
resolution is explicit and Ruff warnings are avoided.
---
Outside diff comments:
In `@coding-agent/src/coding_agent/server/http_server.py`:
- Around line 6016-6037: The response uses session.current_turn_id even when
turn_result was reconstructed from runtime events, leaving turn_id null for
recovered runs; update the SessionResultResponse construction so turn_id is
taken from the reconstructed result when available (e.g., use turn_id =
session.current_turn_id if turn_result is None else turn_result.run_id or
turn_result.turn_id depending on the attribute on the TurnResult object). Modify
the block that builds SessionResultResponse (where turn_result,
verification_summary are computed) to prefer the turn identifier from
turn_result (e.g., turn_result.run_id / turn_result.turn_id) and fall back to
session.current_turn_id.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 924a2c49-03db-4edf-98f0-a59cc6cd7cea
📒 Files selected for processing (2)
coding-agent/src/coding_agent/server/http_server.pycoding-agent/tests/ui/test_http_server.py
Add local-path workspace diff/patch support through a temporary Git index, preserving the existing cloud workspace provider path. Restore session results from persisted runtime events when a restarted process has no live turn id or tape turn available. Verification: - uv run pytest tests/ui/test_http_server.py -k "persisted_runtime_events or workspace_diff or workspace_patch or session_result" -v - uv run pytest tests/ui/test_http_server_workspace_transfer.py -v - uv run ruff check src/coding_agent/server/http_server.py tests/ui/test_http_server.py
delete-cloud
force-pushed
the
fix/o6n-product-verification
branch
from
June 4, 2026 06:36
816c209 to
8e15e3b
Compare
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
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
/sessions/{id}/resultfrom persisted runtime events when a restarted process has no live tape turn/current turn idContext
Live o6n verification exposed two product gaps:
/sessions/{id}/workspace/diffreturned400 Workspace export requires cloud session./runs/{run_id}/display-eventspersisted, but/sessions/{id}/resultreturnedfinal_answer=null.Verification
uv run pytest tests/ui/test_http_server.py -k "persisted_runtime_events or workspace_diff or workspace_patch or session_result" -vuv run pytest tests/ui/test_http_server_workspace_transfer.py -vuv run ruff check src/coding_agent/server/http_server.py tests/ui/test_http_server.pyuv run ruff format src/coding_agent/server/http_server.py tests/ui/test_http_server.py --checkNotes
GIT_INDEX_FILEwith a temporary index and removes it after export, so the real workspace index is not modified.Summary by CodeRabbit
New Features
Tests