Workers fork their channel; reflection reads worker transcripts; sandbox traversal fixes - #633
Merged
Merged
Conversation
- allow file-read-metadata on the top-level /etc, /tmp, /var symlinks so resolver config and the CA bundle resolve through their usual paths - allow metadata on ancestors of allowed paths; git clone stats and mkdirs each leading component and a denied stat surfaces as EPERM where the caller expects EEXIST, killing checkout after a full object transfer - add /private/var/select to the read-only set so every sh invocation stops printing 'Operation not permitted' on stderr
…eflection Workers now fork the channel's conversation history under their own system prompt, the way branches do — the difference between a worker and a branch is the tools it gets, not the context it has. - WorkerHistoryMode collapses to fork (default) | clean. summary was a stub that logged a warning and handed the worker nothing; recent(n) paid for history without any guarantee the load-bearing part was in the window. clean stays for fan-out and mechanical tasks; detached workers are clean by definition. - fork pre-compacts an oversized history before the worker's first LLM call, via a helper shared with the branch pre-flight. - spawn_worker's description now reflects the conversation's live setting, so the model stops writing lossy context summaries out of habit. - the reflection signal carries completed worker ids instead of a bare bool; the persistence prompt lists them and instructs worker_inspect on each, plus failed predecessors — the lesson lives in what the worker tried, not the summary it returned. per-result transcript cap raised to 2000 bytes for reflection passes. - the 'worker process chatter' exclusion is scoped to memory extraction so it no longer argues against reading transcripts during reflection.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR replaces legacy worker history modes with ChangesWorker history and reflection
macOS sandbox profile updates
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
jamiepine
marked this pull request as ready for review
August 10, 2026 02:50
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/agent/channel.rs (1)
3424-3434: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude failed worker IDs in reflection input.
Line 3426 records a worker ID only when
successis true. A failed worker cannot trigger reflection and is absent when another signal triggers it. Reflection therefore cannot inspect failed predecessor transcripts.Collect every completed worker ID. If failed completion must not independently trigger reflection, keep that scheduling policy separate from worker-ID collection.
🤖 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 `@src/agent/channel.rs` around lines 3424 - 3434, Update the worker-completion handling around mark_reflection_worker to record every completed worker ID, regardless of success, so failed predecessor transcripts are available to reflection. Keep the existing success-only condition for triggering reflection scheduling, separating ID collection from the policy that calls check_memory_persistence.
🤖 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 `@src/agent/compactor.rs`:
- Around line 311-320: Update the pre-compaction logic surrounding history.drain
and the compaction marker to repeatedly estimate token usage, removing older
messages until history plus reserved prompt and response capacity fits the
target budget. Recalculate after each removal rather than relying only on the
fraction, and explicitly handle the case where preserved messages alone exceed
the budget without allowing the function to return over budget.
---
Outside diff comments:
In `@src/agent/channel.rs`:
- Around line 3424-3434: Update the worker-completion handling around
mark_reflection_worker to record every completed worker ID, regardless of
success, so failed predecessor transcripts are available to reflection. Keep the
existing success-only condition for triggering reflection scheduling, separating
ID collection from the policy that calls check_memory_persistence.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 04f4b9bf-9019-48a0-bf8e-aa19f8eb0be9
📒 Files selected for processing (16)
interface/src/api/schema.d.tsinterface/src/api/types.tsinterface/src/components/ConversationSettingsPanel.tsxprompts/en/memory_persistence.md.j2prompts/en/tools/spawn_worker_description.md.j2src/agent/branch.rssrc/agent/channel.rssrc/agent/channel_dispatch.rssrc/agent/compactor.rssrc/api/portal.rssrc/conversation/settings.rssrc/prompts/engine.rssrc/sandbox.rssrc/tools.rssrc/tools/spawn_worker.rssrc/tools/worker_inspect.rs
- collect failed worker completions in the reflection signal too — their transcripts carry the trials — while keeping success-only as the trigger policy. Entries are handed to the pass annotated succeeded/failed. - precompact_forked_history re-estimates after each drain instead of trusting one fractional cut; a few large messages could previously leave the fork over budget. Stops at a 4-message floor and leaves the rest to overflow recovery. Covered by tests.
Conflict in src/agent/channel.rs (WorkerComplete handling): main added wake-event emission on completion, this branch moved reflection recording so every completion is tracked. Kept both — wake emission runs first, then reflection records the worker and only success fires the persistence check.
Pre-compaction stopped at a raw 70% usage check against the whole context window, so a fork that fit still had no room for its own system prompt and response. Budget history against the window minus a 30% reserve, re-estimate after every cut, and count the compaction marker itself against the budget. The retention floor is now a named constant and applies to the drain clamp too (it previously allowed cutting to 2 while the loop guarded at 4). A fork whose retained tail alone busts the budget — one oversized tool result can — keeps that tail and logs a warning rather than silently returning over budget. Reflection worker recording moves onto ReflectionSignal so the lifecycle is testable: failed workers are collected for their transcripts but don't fire the signal alone, and repeat completions for a worker are ignored.
Contributor
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 `@src/agent/compactor.rs`:
- Around line 333-369: Update precompact_forked_history and every first-call
path that invokes it to account for the tokens required by the subsequently
attached system prompt, task/user prompt, dynamic skills, and ambient memory.
Pass an actual reserved-token value into the precompaction budget calculation,
or enforce equivalent input-size limits that guarantee the existing reserve;
ensure the resulting history plus reserved prompt input stays within the context
window before prompt_once.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f75a9932-cade-47fa-9f6a-a6d4c624d2cd
📒 Files selected for processing (8)
interface/src/api/schema.d.tssrc/agent/branch.rssrc/agent/channel.rssrc/agent/channel_dispatch.rssrc/agent/compactor.rssrc/conversation/settings.rssrc/prompts/engine.rssrc/tools.rs
🚧 Files skipped from review as they are similar to previous changes (7)
- interface/src/api/schema.d.ts
- src/tools.rs
- src/conversation/settings.rs
- src/agent/branch.rs
- src/agent/channel_dispatch.rs
- src/agent/channel.rs
- src/prompts/engine.rs
The 30% reserve was a guess made before the caller attached anything. precompact_forked_history now takes the prompt size the caller is about to send — worker forks measure the fully rendered preamble (skills and ambient memory included) plus the task, branches measure their system prompt plus the user prompt — and reserves 15% of the window for the response on top. A preamble that outgrows the window collapses the budget to zero; the fork keeps its retention floor and warns rather than being emptied.
Contributor
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 `@src/agent/compactor.rs`:
- Around line 300-304: Update forked_history_budget so response_reserve uses
integer ceiling arithmetic for the 15% FORK_RESPONSE_RESERVE instead of
truncating fractional tokens, ensuring small windows still reserve response
capacity. Add a boundary test using a context window not divisible by 20, such
as 10, and verify the resulting budget reserves the rounded-up token count.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cd1eae27-6d66-4a1b-9189-7448f0d0a609
📒 Files selected for processing (3)
src/agent/branch.rssrc/agent/channel_dispatch.rssrc/agent/compactor.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/agent/branch.rs
- src/agent/channel_dispatch.rs
Comment on lines
+300
to
+304
| /// Tokens available to a forked history, once the prompt the caller is about | ||
| /// to attach and room for a response are both reserved. | ||
| fn forked_history_budget(context_window: usize, prompt_tokens: usize) -> usize { | ||
| let response_reserve = (context_window as f32 * FORK_RESPONSE_RESERVE) as usize; | ||
| context_window.saturating_sub(response_reserve.saturating_add(prompt_tokens)) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Round the response reserve up to a whole token.
Line 303 truncates the 15% reserve. For a context_window of 10, it reserves 1 token instead of 2. For a context window of 1, it reserves no response capacity. This permits input that consumes capacity intended for the worker response.
Use integer ceiling arithmetic for the reserve. Add a boundary test for a context window that is not divisible by 20.
🤖 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 `@src/agent/compactor.rs` around lines 300 - 304, Update forked_history_budget
so response_reserve uses integer ceiling arithmetic for the 15%
FORK_RESPONSE_RESERVE instead of truncating fractional tokens, ensuring small
windows still reserve response capacity. Add a boundary test using a context
window not divisible by 20, such as 10, and verify the resulting budget reserves
the rounded-up token count.
This was referenced Aug 10, 2026
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.
Phases 7 and 8 of docs/design-docs/worker-reliability.md, plus the sandbox profile fixes that came out of the same investigation.
Workers are forks
Delegation was lossy by construction: workers defaulted to receiving no conversation history, so the channel agent compressed the session into a task paragraph at exactly the moment the details mattered most.
WorkerHistoryModecollapses fromnone | summary | recent(n) | fullto:fork(default) — full clone of the channel's history under the worker's own system prompt, the branch semantic. Pre-compacts before the first LLM call via a helper now shared with the branch pre-flight.clean— task only. For fan-out and mechanical tasks; detached workers are clean by definition.summarywas a stub that logged a warning and handed the worker nothing.recent(n)paid for history with no guarantee the load-bearing part was in the window. Both are gone, not deprecated. Stored settings with dead variants fall back to the default via the existing fail-soft parse.spawn_worker's tool description now reads the conversation's live setting:so the model stops writing lossy context summaries out of habit when the worker will see everything anyway.
Reflection reads worker transcripts
Skill reflection triggered on worker success but could only see the channel's summary view — a worker could spend eleven minutes discovering a working procedure, report a two-line success, and reflection saw nothing to teach. Now:
AtomicBoolworker_inspecton each, plus failed predecessors from the list viewworker_inspect's per-result display cap rises to 2000 bytes for reflection passes so error text survivesSandbox traversal fixes
Three profile gaps that turned a plain
git cloneinto a 30-call ordeal:/etc,/tmp,/varsymlinks getfile-read-metadata, so resolver config and the CA bundle resolve through their usual paths instead of pushing workers towardGIT_SSL_NO_VERIFY/private/var/selectjoins the read-only set, ending the 'Operation not permitted' stderr spam on every shell callFork budget
precompact_forked_historyre-estimates after every cut instead of trusting one fractional pass, and budgets against the prompt the fork is about to carry rather than a fixed share of the window. Worker forks measure the fully rendered preamble — skills and ambient memory included — plus the task; branches measure their system prompt plus the user prompt. 15% of the window is reserved for the response on top, and the compaction marker counts against the budget so the notice can't push a fork back over the line it was just trimmed to.The most recent four messages are never dropped. A single oversized tool result can outweigh the whole window, and dropping it would strip the fork of the context it was forked for, so the fork keeps its tail and logs a warning instead of silently returning over budget.
Terminal states
WorkerCompleteis the terminal event for both outcomes and fires once per worker — the handler returns early when the id is already gone fromworker_handles, so a duplicate event is a no-op. Every completion is now recorded against the reflection signal, which is what makes a failed predecessor's transcript available to the pass a later success triggers. Only success fires the signal (ReflectionSignal::is_setrequiresturn_workor at least one successful worker), so an unresolved failure can't schedule a reflection pass on its own, andrecord_workerignores repeat completions for an id already held.Collection and scheduling stay separate:
mark_reflection_workeralways records, and only the success branch callscheck_memory_persistence. The signal itself is cleared when the persistence branch actually spawns, so a failed spawn retries on the next check rather than losing the reflection for a cooldown.Merge with main
Merged current
main(autonomy + wakes). One conflict, inWorkerCompletehandling: main added wake-event emission per completion, this branch had moved reflection recording out from under the success check. Both kept — wake emission spawns off the event loop first, then reflection records the worker and only success callscheck_memory_persistence.Tests
test_sbpl_profile_traversal_rulesasserts the symlink rules, per-ancestor metadata rules, and that the data-dir deny still wins orderingworker_inspectinstructions when ids are present, stays clean when notNote
This PR improves worker reliability through three major changes. Workers now fork full channel history by default (instead of receiving only a lossy summary), allowing them to access complete context while remaining clean for fan-out tasks. The reflection system now reads worker transcripts to extract and learn from their actual problem-solving process, not just final results. Sandbox profile fixes eliminate 30+ permission-related syscall failures that plagued git operations by correctly handling symlinks, ancestor path metadata, and common /var directories.
Written by Tembo for commit d859edf. This will update automatically on new commits.