refactor: use filesystem scope architecture - #397
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
skill-check — worker0 verified, 31 skipped (no docs/).
Four for four. Nicely done. |
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (122)
✨ 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 |
ytallo
changed the title
This was referenced Jul 3, 2026
ytallo
marked this pull request as ready for review
July 3, 2026 16:20
When the agent hits a folder outside the session's allowed roots, the approval-gate now parks the call as a folder_access pending record. The chat renders a dedicated prompt card for it — allow once / allow this session / always allow (confirmed, writes shell fs.host_roots) / deny — in place of the standard approval row. - FolderAccessPrompt: inline prompt with scope buttons and destructive confirm for permanent grants; remounts per grant request so a re-parked call never inherits stale submit state - FolderAccessDialog: per-session management surface (workspace, session grants with revoke, permanent roots read-only with deep-link to the shell configuration editor) - use-folder-grants: harness::workspace::grants/revoke adapter with optimistic updates and response-ordering guards; hidden when the harness predates workspace grants - translate/entry-mapper/ChatView carry the folderAccess payload through dedupe and snapshot re-derivation; approval::resolve gains grant_scope only for folder_access resolutions (old gates unaffected) - discoverability copy in DirectoryPicker and console settings
…ution A new approval::grant-watch hook on harness::hook::post-trigger (shell::*/coder::*, fail_open) watches dispatch failures for the shell's jail-scope grant_hint tail (S215/S220/C215/C218) and converts the raw rejection into a held folder_access pending approval, so the console can ask "allow access to <dir>?" instead of surfacing a jail error to the model. The ladder fails toward delivery at every rung: workspace dirs never prompt (stamp holes log loudly), already-granted and user-denied dirs are suppressed, and a per-call re-ask cap (grant_reask_limit, default 3) prevents ping-pong. approval::resolve gains grant_scope for folder_access allows: - once: releases the held call with one-shot extra_roots - session: harness::workspace::grant first (falls back to extra_roots if the RPC fails), then release - always: session grant plus best-effort persistence of the dir into the shell configuration's fs.host_roots - deny: records the dir in per-session denied memory before delivering New modules: grant.rs (pure hint parsing + workspace guard), grant_state.rs (denied memory, attempt caps), shell_config.rs (fs.host_roots append), functions/grant_watch.rs (orchestration). Session/turn purge handlers clean up the new state; record/request fields are wire-compatible with older consoles and stored records.
ytallo
force-pushed
the
feat/filesystem-scope-architecture
branch
from
July 3, 2026 16:45
a7656bd to
4355574
Compare
andersonleal
added a commit
that referenced
this pull request
Jul 3, 2026
Main's fs-scope refactor (#397) introduced the bare DispatchError struct; the react-bridge reconcile pass logs it with %e, which needs Display — an error type should carry one anyway.
andersonleal
added a commit
that referenced
this pull request
Jul 3, 2026
…-in, wire hardening, and spawn console view (#401) * feat(harness): reactive trigger bridge (harness::react) with join fan-in and lifecycle hardening Ports the engine's trigger/notify primitives into a harness-native reactive sub-agent bridge and hardens the full registration/fire/teardown lifecycle against gaps found in live testing. - harness::react: sub-agent spec fires on engine triggers (turn events, state, cron, stream); join fan-in with an expect array, fire-once accumulator, and rearm for standing watchers. - Interceptor pass-through (subscribe.rs): agent-issued engine::register_trigger calls get owner + subscription id stamped server-side into the react metadata, closing several trust gaps in the raw registration path. - Idempotent registration (dedup by canonical request key) and a durable owner sweep on session::deleted, replacing two pipelines that could double-register the same reaction. - Startup reconcile: GC react bindings whose owner session is gone and notify bindings unknown to the local registry; never GC on doubt. - Loop breakers: self-edge drop, reactive-depth cap, per-subscription fire-rate limit. - Join results deliver into the registering (owner) session by default instead of a detached, unread child session; parent nesting falls back from the event's session through the owner stamp to resolve_root. - Registration advisories for turn-event filters naming a nonexistent session, and for a join key wired to the same event source as a sibling key. - Policy aid: narrowed sub-agents are told their allowed/denied function surface directly in the system prompt instead of discovering it via a denied functions::list call. - Heal dangling function_calls left by interrupted/compacted turns before the next generate step. - Docs: tech spec, skill, and all prompt variants updated for the react/join doctrine. * feat(console): dedicated chat view for harness::spawn Replace the raw-JSON fallback card with an instrument-panel view: policy chips (model/mode/turns/thinking/output/allow/deny), the task rendered as markdown, and the child's result as markdown, highlighted JSON, or the direct-call child ids. Guard errors and failed children route through the existing SandboxErrorView; the approval gate gets a policy-first preview. Session ids link to the child conversation via the sidebar's select when the console knows the session. Includes Zod parsers for the spawn wire schema (excerpt-tolerant), fixtures for all six card states, a gated-spawn playground scenario, and parser tests locking envelope unwrapping and error-before-success dispatch. * fix(harness): seed react-bridge TurnRecord fields in subagent test Upstream #388 added a test TurnRecord literal that predates this branch's display_parent_session_id / spawned_by_subscription_id / reactive_depth fields; the rebase merged clean but test compilation broke. * feat(harness): prompt doctrine — name every spawned child session Every harness::spawn must pass session_id: a short readable job slug plus a few random characters (fetch-headlines-b4k9), replacing the opaque engine-minted UUIDs in the console tree. Never the parent session id as a prefix; the random suffix carries the run-uniqueness guarantee instead (a reused id silently resumes the old session). Scoped to direct spawn calls only — in a react trigger's metadata a fixed session_id funnels every firing into one session and re-aims join delivery. Fan-in doctrine updated to the same naming across all five prompt variants. * fix(providers): keep displaced tool results adjacent to their call A notification or steering user entry injected while a call window is open (a parked harness::spawn holds one open for minutes) lands between function_call and function_result in the durable transcript. Every wire mapper only repaired MISSING results (orphan placeholder) — a DISPLACED result survived to the wire as assistant(tool_use) / user(text) / user(tool_result), which Anthropic 400s ('tool_use ids were found without tool_result blocks immediately after') and OpenAI/xAI/Responses reject as a user row between tool_calls and its tool rows. The durable transcript replays the shape on every retry, permanently wedging the turn. Fix: shared llm_router::types::messages::reorder_displaced_results runs first in all four providers' to_wire_messages — each FunctionResult moves directly after the assistant that emitted its call, order preserved, orphan results untouched. Wedged sessions self-heal: the transcript itself was never illegal, only the wire projection. Repro test written first and failed with the exact live shape; regression tests in all four providers plus unit tests on the shared helper. * fix(harness): rotate mid-generation user arrivals past the interrupted reply A user entry appended while a step is generating (or assembling — the compaction/hook window) lands before that step's assistant entry in the durable log. The steering check then re-generates, but the assembled context ENDS with a call-less assistant message — a prefill request newer Anthropic models reject ('This model does not support assistant message prefill. The conversation must end with a user message.'), wedging the turn on every retry. Older models silently accepted prefill, hiding this path. Fix: rotate_mid_generation_users presents arrivals after the previous step's watermark AFTER the reply they interrupted — semantically exact, the model answered without seeing them. Two invariants hardened by adversarial review: - the new watermark is assigned only after router.chat returns; the pre-generate put_turn persists the OLD one, so a redelivered step keeps its rotation window instead of re-issuing the rejected shape forever - rotation runs on the FINAL assembled values, never on the candidate: compaction persists tail_start_entry_id as a log-order cursor indexed from the candidate, and rotating first would silently drop the rotated message from every future window Also: has_user_after_watermark now loads include_custom=true, matching the list the watermark comes from (a watermark landing on a custom entry silently disabled the steering check). * style: cargo fmt (harness, provider-openai, provider-xai) * fix(harness): Display for DispatchError + fmt (rebase fallout) Main's fs-scope refactor (#397) introduced the bare DispatchError struct; the react-bridge reconcile pass logs it with %e, which needs Display — an error type should carry one anyway. * docs(harness): revert harness.md spec changes Restore tech-specs/2026-06-agentic/harness.md to main's version — the react-bridge spec additions come out of this PR. * fix(harness): address CodeRabbit review on #401 - react: include the join (id, key) in the fallback fire-gate hash — state-based join predecessors share the whole downstream spec except their key, so a wide join shared one 10-fires/min budget and tripped the breaker spuriously - react: retry the join accumulator delete (3 attempts) — a failed delete left fire=1 behind, permanently wedging a rearmed join's fire-once guard; persistent failure on a rearmed join now logs at error level with the recovery path - spawn: strip spawned_by_subscription_id / reactive_depth on the model-reachable dispatch path — react-internal bookkeeping a model could spoof to defeat the self-edge breaker and depth cap - skills: align SKILL.md fan-in naming with the prompt doctrine (slug + random suffix, never the originating session id as a prefix) - tests: multi-owner displaced-result reorder case; round-trip the react-bridge TurnRecord fields with real values
This branch was successfully deployed
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
This is the review target for the filesystem access architecture.
fs_scope { root, grants }and exposeharness::filesystem::*control-plane functions.access_durationandfilesystem_access_requestrecords.shell/fs/host_rootsin the configuration UI.PR Organization
feat/filesystem-scope-architecture->main).Validation
Local verification:
cargo test --manifest-path shell/Cargo.tomlcargo test --manifest-path harness/Cargo.tomlcargo test --manifest-path approval-gate/Cargo.tomlpnpm --dir console/web typecheckpnpm --dir console/web testcargo clippy --manifest-path shell/Cargo.toml --all-targets --all-features -- -D warningscargo test --manifest-path shell/Cargo.toml --test code_golden_errorscargo test --manifest-path shell/Cargo.toml --lib from_jsoncargo test --manifest-path shell/Cargo.toml --lib removed_host_rootcargo test --manifest-path shell/Cargo.toml --lib seed_default_matches_shipped_config_yamlnpm run --silent buildfromshell/tests/e2e/workers/harnessgit diff --checkLatest follow-ups pushed:
4e7866ecfixes shell clippy on config helpers.94c7ffcfnormalizes filesystem access error goldens across platforms.7b61f764updates the shell E2E configs/scripts to usefs.host_roots, quotes thefalseexecutable in YAML, isolates generated configuration state, and removes stale publichost_rootterminology.a7656bdcre-owns E2Esandbox::fs::*mocks per mock case so the wire-shape tests stay isolated while realiii-sandboxremains available for exec sandbox coverage.