stack R3: runtime P0s (transcript escapes, #5099 route inheritance, roster shadowing + trust gate) - #5148
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Pull request overview
This PR (stack R3) hardens runtime safety and correctness across the TUI/CLI by (1) sanitizing transcript/pager output to prevent terminal escape injection, (2) fixing sub-agent spawn route resolution to correctly inherit the session route for provider-less defaults (#5099) and re-reading disk-backed roster sources at spawn time, and (3) making fleet roster layering/trust and coordination-lock loss visible and actionable in the UI.
Changes:
- Strip ANSI/OSC/CSI/control bytes at pager and
fleet logspreview chokepoints, with regression tests for hostile transcript-shaped inputs. - Adjust sub-agent spawn model routing: provider-less foreign defaults downgrade to
ModelRoute::Inherit, and spawn refreshes roster/role-model sources from current disk (#5099). - Add roster shadowing receipts + UI surfacing and trust-gate workspace agent profiles behind
--no-project-config(#5098), plus coordination lock ownership projection and UI warnings.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/tui/src/tui/work_surface/mod.rs | Updates work-surface tests to include new coordination lock fields in projections. |
| crates/tui/src/tui/views/fleet_roster.rs | Displays roster shadowing in list + detail pane and threads shadow receipts through view rendering/tests. |
| crates/tui/src/tui/ui/tests.rs | Extends UI tests to include coordination lock ownership fields in projection state. |
| crates/tui/src/tui/ui.rs | Surfaces coordination lock loss as a sticky warning to avoid misleading “running” rows. |
| crates/tui/src/tui/pager.rs | Sanitizes pager text before rendering; adds regression tests for CSI/mouse/OSC-bearing transcripts. |
| crates/tui/src/tui/coordination_detail.rs | Treats lock loss as “needs attention” and prints lock-unavailable status in formatted detail. |
| crates/tui/src/tools/subagent/tests.rs | Adds tests for lock ownership projection and orphan terminalization behavior; adds #5099 regression tests. |
| crates/tui/src/tools/subagent/mod.rs | Implements lock ownership fields, spawn-time roster refresh, provider-less foreign-default downgrade to inherit, and orphan cleanup changes. |
| crates/tui/src/tools/subagent/coord.rs | Extends coordination projection schema with process_lock_held + process_lock_note (serde defaults). |
| crates/tui/src/tools/skill.rs | Fails loudly when a native SKILL.md disappears, naming the exact missing path (no stale cached body). |
| crates/tui/src/snapshot/mod.rs | Documents loud opt-in/disable behavior for snapshot size gating. |
| crates/tui/src/skills/tests.rs | Pins that global skill roots resolve only under OS home (or explicit $CODEWHALE_HOME). |
| crates/tui/src/prompts.rs | Updates tool-description contract test to assert the live Bash tool name (not exec_shell). |
| crates/tui/src/main.rs | Sets project-agent-profile trust gate from --no-project-config; sanitizes fleet logs preview output. |
| crates/tui/src/fleet/roster.rs | Records shadowed profile receipts; adds trust gate for workspace profiles; adds shadowing/trust tests. |
| crates/tui/src/core/turn.rs | Emits a once-per-workspace stderr warning when snapshots/undo are disabled due to size gate. |
| crates/cli/src/cloud/tests.rs | Adds regression test ensuring device-login timeout returns non-zero exit status. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| process_lock_held: { | ||
| // Retry acquire on each projection so a session that lost the | ||
| // flock at construction recovers once the previous owner exits. | ||
| let _ = self.coordination_process_lock_status(); | ||
| self.holds_coordination_process_lock() |
| // agent without a live task handle cannot make durable progress from | ||
| // here. Terminalize immediately so the UI never shows a ticking | ||
| // counter on a dead job while we wait for heartbeat timeout (#2.6). | ||
| if !self.holds_coordination_process_lock() { |
| let shadow_badge = if self | ||
| .shadowed | ||
| .iter() | ||
| .any(|shadow| shadow.id.trim().eq_ignore_ascii_case(member.id.trim())) | ||
| { | ||
| " ⚠shadows" | ||
| } else { | ||
| "" | ||
| }; |
| for shadow in shadowed | ||
| .iter() | ||
| .filter(|shadow| shadow.id.trim().eq_ignore_ascii_case(member.id.trim())) | ||
| { |
Viewing a sub-agent transcript (agent chat pager) or a fleet worker log emitted raw captured terminal bytes to the parent terminal: a child TUI's mouse-tracking handshake (`ESC[?1003h`, `ESC[?1006h`) and mouse event reports (`ESC[<65;72;17M`) re-armed mouse reporting in the user's shell, so after exit zsh received click bytes as input and tried to execute fragments (operator checklist §2.1). - PagerView::from_text now sanitizes through osc8::strip_ansi_into at the single chokepoint every pager surface (agent transcript, activity detail, MCP, approval, constitution) builds from; CSI/OSC/DCS sequences and lone control bytes are stripped, visible text and \n/\t formatting kept. - `codewhale fleet logs` (main.rs print_logs) sanitizes the log preview before printing for the same reason. Tests: tui::pager::tests::from_text_strips_csi_mouse_and_osc_sequences and from_text_sanitizes_jsonl_transcript_shaped_content feed transcripts containing CSI/mouse/OSC sequences and assert the rendered body is inert. Receipt: cargo test -p codewhale-tui --bin codewhale-tui -- tui::pager:: tui::osc8:: → 55 passed, 0 failed.
… spawn #5099 / operator checklist §2.2 — two defects, one spawn path: 1. A child spawned with no explicit model on a moonshot/xai session was handed a provider-less deepseek default (role default or unpinned fleet profile model) and the known-foreign guard hard-failed the spawn: 'Model deepseek-v4-flash was supplied without an explicit provider pin, but the resolved route is moonshot...'. The guard is correct for an explicit caller pin, but a DEFAULT the session never chose must not fail closed. resolve_fixed_spawn_model_route now distinguishes the source: task.model keeps the pin-vs-inherit error; agent_profile.model and role.default downgrade to ModelRoute::Inherit (session route) with a tracing::warn naming the skipped model, and the spawn receipt records run.model provenance. 2. The runtime's fleet roster and role_models were launch-time snapshots (built once in main.rs), so after both builder.toml files were edited pro->flash a spawn was still rejected supplying deepseek-v4-pro — a value that existed nowhere on disk. spawn_subagent_from_input now calls refresh_spawn_route_sources, which re-loads FleetRoster from current disk (personal + project profile files) and rebuilds role_models (explicit [subagents] config overrides still win) before profile resolution. Without the session Config the launch snapshot is kept. Tests (cargo test -p codewhale-tui --bin codewhale-tui): - providerless_foreign_spawn_default_inherits_session_route: moonshot parent + provider-less deepseek default -> Inherit/run.model; explicit task.model keeps the error naming the fix; same-provider default still resolves fixed. - spawn_route_sources_refresh_reads_current_disk: stale launch snapshot vs fresh .codewhale/agents/builder.toml -> roster and role_models read current disk. Receipts: tools::subagent:: 402 passed; fleet::/route/spawn filters 293 passed; providerless/refresh filters 5 passed.
Operator checklist §2.3 (#5033 class): the owner saw 'error: Codewhale account login timed out' with exit code 0 on 0.9.3. At the 0.9.4 release train the propagation chain is already correct — poll_device bails, run_with/run propagate, run_cli maps Err to ExitCode::FAILURE — verified live against a stub device-flow server (pending 202s, --timeout-seconds 2): the CLI printed the timeout and exited 1. What was missing at this base is a regression pin at the run_with seam (the exact spot that decides the process exit code). The new test drives CloudCommand::Login against a transport whose token polls stay pending forever and asserts the command returns Err containing 'login timed out', so a future refactor cannot silently report success after a failed login. Receipts: cargo test -p codewhale-cli --lib account_login_timeout → 1 passed (1.01s, real client timeout); cargo test -p codewhale-cli --lib → 205 passed.
Operator checklist §2.5: a session logged 'Failed to read /Users/hmbown/.codewhale/skills/delegate/SKILL.md' — hmbown is the GitHub handle, the OS user is hunterbown, and the delegate skill silently never loaded. Investigation at this base: every home-resolution path in the workspace funnels through codewhale_paths::user_home() (HOME -> USERPROFILE -> dirs::home_dir, i.e. the OS user) or an explicit $CODEWHALE_HOME. No code constructs a home directory from an account/GitHub handle — searched /Users/ literals (test fixtures only), whoami/USER/LOGNAME consumers, path joins on login/handle, and both shellexpand call sites. The observed path shape matches a read_file argument constructed from the account handle visible in account/git surfaces, i.e. a model-side path, not a runtime construction. What was genuinely broken and is fixed here: the silent half. A native registry entry whose SKILL.md vanished from disk after discovery was served from the stale cached body, so a skill could silently never load from the path the user expected. load_skill now fails loudly via ensure_native_skill_file_present, naming the exact path it checked and the knobs that control it (skills_dir, $CODEWHALE_HOME, OS home); reviewed plugin snapshots keep skipping the disk check. Tests: - tools::skill::tests::native_skill_with_vanished_file_fails_loudly_with_the_path - skills::tests::global_skill_roots_come_from_the_os_home_only (pins that global roots come from the OS home / $CODEWHALE_HOME only) Receipt: cargo test -p codewhale-tui --bin codewhale-tui -- tools::skill:: skills:: → 217 passed.
When this process does not hold the workspace coordination flock, durable fleet writes are skipped. Surface that on the coordination projection and sticky status, and mark Running agents with no live task handle Interrupted locally so Work never ticks a counter on a settled/dead job (#2.6 / #5036). Does not tighten read-only agent shell or tool gates. Tests: tools::subagent::tests::coordination_detail_projection_reports_process_lock_ownership tools::subagent::tests::cleanup_terminalizes_running_orphans_without_task_handle_when_lock_missing tools::subagent::tests::isolated_worktree_workers_skip_the_coordination_process_lock
Large workspaces no longer disable undo silently. Print one prominent warning with the max_workspace_gb opt-in, and document the failure model.
…gate #5098 (partial — the two non-owner-decision halves): (a) Shadowing is visible. FleetRoster now records every layer displacement as a ShadowedProfile receipt (id, shadowed origin+source, winner origin+source) instead of silently dropping the losing file from the merged roster. File-on-file shadows (personal/config losing to another file layer) log a warning at load; built-in overrides stay debug-quiet since they are the intended customization path. The roster view badges shadowing rows ('⚠shadows') and the detail pane lists each ignored layer ('Shadows: personal copy at <path> (ignored)'), so editing the wrong builder.toml no longer changes nothing with no signal why. (b) Project-scope trust gate. load_workspace_agent_profiles_tolerant applied no workspace-trust check — a cloned repo's .codewhale/agents/*.toml silently joined the dispatch roster. Project profiles now join only when project-level config is trusted for the launch: --no-project-config opts the whole layer out (same gate as .codewhale/config.toml, #485). The launch decision is recorded once in main (set_project_agent_profiles_enabled) so every roster re-read — spawn refresh, dispatch, views — honors it consistently; the private loader takes the flag as a parameter so tests stay hermetic. Not done here (owner decision, flagged): the three-layer collapse (Built-in -> Personal -> Project, deprecating [fleet.profiles]) and the edit-what-you-see editor change. Tests: - fleet::roster::shadow_and_trust_tests::workspace_shadow_of_personal_file_is_recorded_and_reported - fleet::roster::shadow_and_trust_tests::project_scope_profiles_are_skipped_when_the_layer_is_not_trusted - tui::views::fleet_roster::tests::detail_pane_reports_shadowed_lower_layers Receipt: cargo test -p codewhale-tui --bin codewhale-tui -- fleet:: views::fleet -> 324 passed.
The §2.6/§2.7 cherry-picks from agent/runtime-bugs-20260802 tripped two lints that the base commit had just cleared: - print_stderr (deny): the snapshots-disabled once-notice in core/turn.rs uses eprintln deliberately (headless/CLI stderr is the user surface); the allow now sits on the function, matching the runtime_log.rs precedent (macro-level allows are ignored). - manual_ok_err (warn): the process_lock_note match in tools/subagent/mod.rs is now a plain .err() call. Receipts: cargo clippy -p codewhale-tui --bin codewhale-tui -> clean; cargo clippy --workspace -> clean; cargo test core::turn:: + coordination/cleanup filters -> 12 passed.
R1's exec_shell rename fix (9c1614d88 lineage) removed the retired name from write_file's description; this prompts.rs pin still asserted the old misdiagnosis. The pin now asserts the new contract: guidance names the live Bash tool and never references exec_shell. Coverage kept, not deleted. Verified: cargo test -p codewhale-tui --bin codewhale-tui prompts:: — 111 passed, 0 failed (exit=0).
a0c6130 to
7a7e6b4
Compare
Stack R3 of the v0.9.4 program — chains on R1 (#5147), which chains on the train (#5135). 9 commits, one concern each.
What's in it
7b087a272lineage):PagerView::from_textsanitizes throughosc8::strip_ansi_intoat the single chokepoint — child-TUI bytes (mouse tracking, CSI) can never reach the parent terminal raw again;codewhale fleet logspreview sanitized too. Tests feed CSI/mouse/OSC-laden transcripts and assert inert output.380f342de): provider-less defaults that fail the known-foreign guard now downgrade toModelRoute::Inherit(session route) with a warn — explicit pins still error correctly. The stale second resolution path is gone: spawn re-reads the roster from current disk (refresh_spawn_route_sources). Moonshot-parent/default-child pinned by test. Closes Sub-agent / Fleet spawn rejects unpinned model when session route provider differs (deepseek-v4-flash × xai/moonshot) #5099.account_login_timeout_fails_the_command, live-verified exit=1 against a stub device-flow server)./Users/hmbownwas a model-constructed argument. The real silent half fixed: a vanished native `SKILL.md` now fails loudly with the exact path instead of serving a stale cached body.05b374dc6cherry-pick): coordination projection carriesprocess_lock_held; the Work surface shows the sticky "Delegated coordination unavailable" status; Running agents with no live task handle are terminalized locally.0304b412ccherry-pick): once-per-workspace prominent notice naming[snapshots] max_workspace_gbas the documented opt-in.190e4138d): (a) every layer displacement recorded asShadowedProfile, badged⚠shadowsin the roster with the ignored path named; (b) project-scope agent profiles join the roster only when project config is trusted (--no-project-configopts out — same gate as.codewhale/config.toml). The layer-collapse proposal stays owner-decision-pending, so this does NOT close Fleet config has one layer too many — and silent shadowing between the rest #5098.Bashname, never retiredexec_shell.Gates
cargo fmt --all -- --checkexit=0;cargo check --workspace --all-targets --lockedexit=0; full tui bin suite 9600 pass / 2 pre-existing + 1 stale pin (fixed ina0c613094;tui_and_api_listings_agreeis the known parallel-load flake — passes solo).No-Issue: v0.9.4 program stack PR. Closes #5099. Refs #5098, #5033.