Port six sandcastle features to Eden#2
Conversation
- Adds ``eden/orchestrator/_recovery.format_agent_error_recovery`` and emits its output as a stream event before the loop raises ``AgentError``. Users now see a copy-pastable recovery hint (worktree, log, branch, next-step commands) in the run log even when the calling code swallows the exception. - Fixes a latent bug where the ``AgentError`` raise path read ``runner.exit_code()`` AFTER the ``with`` block had already invoked ``terminate()`` and nulled the process handle. Captures the exit code and stderr inside the ``with`` block instead. - Adds e2e coverage in ``tests/e2e/test_agent_error.py`` driving the full loop with a Codex-shape stdout error event, an empty-stdout agent that writes only stderr, and clean-exit baselines. - New ``CHANGELOG.md`` documenting all changes since the last release, including the inline-prompt-literal and prompt-arg shell-injection behavior shifts.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements a new error recovery messaging system. A new public function, ChangesAgent Error Recovery Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@eden/orchestrator/_recovery.py`:
- Around line 48-53: The shell commands appended to lines use unquoted variables
(worktree_path, log_path, branch) which will break on spaces/special chars; wrap
these values with a proper shell-quoting function (e.g., import shlex and use
shlex.quote(value)) when building the strings passed to lines.append for the cd,
less and git diff commands (and any other places that interpolate those
variables), leaving the if log_path is not None guard intact so you only quote
when present.
🪄 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
Run ID: ac58e088-a31d-460a-9593-6e1741a607b1
📒 Files selected for processing (5)
CHANGELOG.mdeden/orchestrator/_loop.pyeden/orchestrator/_recovery.pytests/e2e/test_agent_error.pytests/unit/test_recovery_message.py
CodeRabbit caught that the ``cd`` / ``less`` / ``git diff`` lines in the
recovery message interpolate ``worktree_path`` / ``log_path`` / ``branch``
without quoting — a worktree under ``/Users/Foo Bar/...`` (a real macOS
case) would break the copy-paste. Wrap each interpolated value with
``shlex.quote``.
Also fixes Windows CI failures: the unit tests asserted ``"/tmp/eden/wt"``
appeared in the formatter output, but on Windows ``Path("/tmp/eden/wt")``
stringifies as ``\\tmp\\eden\\wt``. Switched the tests to ``tmp_path``
fixtures and ``str(path)`` so the assertion matches the OS-native
representation. Added two new tests pinning the shell-quoting behavior
for paths with spaces and branches with special characters.
The two tests added in 14d53b6 (``test_quotes_path_with_spaces`` and ``test_quotes_branch_with_special_chars``) constructed ``Path("/Users/Foo Bar/...")`` and asserted against POSIX-style literals. On Windows that ``Path`` stringifies to ``\\Users\\Foo Bar\\...``, so ``shlex.quote`` and the assertion didn't match. Switch to ``tmp_path / "Foo Bar" / ...`` and compare against ``shlex.quote(str(wt))`` so both halves use the same OS-native repr.
1 participant
Audit of sandcastle v0.5.0–v0.5.10 against Eden surfaced five gaps; this PR closes them and adds a recovery-message helper as a sixth port. Two of the changes are user-visible behavior shifts — see CHANGELOG.md.
Changes
1. Shell-injection guard in
prompt_argsrender_promptnow substitutes built-ins, expands!`cmd`shell blocks against the raw template only, and substitutes user args last. An arg value containing!`...`text is treated as inert data instead of triggering subprocess execution. Mirrors sandcastle's6bc4d74.2. Inline prompts pass through verbatim
prompt="..."strings now skip{{KEY}}substitution,!`cmd`shell expansion, and{{SOURCE_BRANCH}}/{{TARGET_BRANCH}}injection. Onlyprompt_file=...goes through the render pipeline. Mirrors sandcastle's4032e64.3.
AgentErrorwith parsed-stdout detailsWhen an agent EOFs with a non-zero exit and no completion signal, the loop raises a typed
AgentErrorimmediately instead of waiting foridle_timeout.parse_stdout_errorextracts text from Codex / Pi{"type":"error",...}events and OpenCode'sresulttext — these emit on stdout, not stderr.4.
CopyToWorktreeError+Timeouts.copy_to_worktreeIsolated provider's worktree clone now bounds at 60s by default and raises a typed error on timeout or copy failure. New
isolated.provider(copy_timeout=...)knob (passNoneto disable).5. Commit-aware finalize log
Replaces
[eden] finalized: applied=True files=N bytes=Bwith[eden] no changes to sync/[eden] syncing N file(s) to host (B bytes)/[eden] sync incomplete: ....6. Recovery hint stream event (port of sandcastle's
RecoveryMessage)format_agent_error_recoveryemits a copy-pastable hint via the run sink before raisingAgentError: agent name, exit code, parsed error, branch, worktree, log path, andcd/git status/git diff/eden cleannext-step commands.Behavior changes (worth flagging in release notes)
prompt="..."no longer expands{{SOURCE_BRANCH}}. Move templated prompts to a file or build the string in Python before passing it inline.AgentErrorinstead of waiting foridle_timeoutto fire.Test plan
mypy --strict eden testsclean (191 source files)tests/unit/test_agent_errors.py,tests/unit/test_recovery_message.pytests/e2e/test_agent_error.pycovering parsed-stdout, stderr-fallback, clean-exit-with-completion, and zero-exit-without-completion paths[eden] syncing ...wording🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
New Features
Changed
Documentation