v0.9.14 slice run 2: lazy MCP, session recovery + picker UX, launch remedy row - #6175
Conversation
…6099) Headless runs fired no hooks at all; --hooks arms the same HookExecutor the TUI builds (global config, reviewed plugin snapshots, trusted project hooks.toml) and threads it through the engine config, the SendMessage op, and the tool runtime services. tool_call_before can still deny and shell_env still applies; hook `ask` resolves fail-closed headlessly. Fleet worker subprocesses never opt in. permissions.toml typed rules already ran in this turn loop and are unchanged. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
A role member pinned to the fleet's own operator route resolved to that route either way — the pin only stopped it following when the operator moved. FleetFile::parse now reads the redundant pin as inheritance, and add_fleet_model writes inherit for role members on the operator route. models_of attributes inherited roles to the operator route they resolve to, and the picker toggle still reports that coverage as present. Different-route pins and shortlist rows keep their pins — the pin is the deliberate opt-out. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The engine drains rx_op only between turns, so an awaited send into a saturated 32-slot mailbox froze all input for the rest of the turn. The SendMessage dispatch already reserved capacity off the render thread; this finishes the audit for the remaining input-path sites: - try_send for droppable ops whose rejection is reported and retryable: sidebar/apply CancelSubAgent, PreviewOutboundRequest, bang shell input, PurgeContext, and the single-op settings updates. - try_reserve before committing state, then send_reserved_op, where the op must land if the UI changed: BacktrackConfirm's SyncSession and the search-provider update. - sync_mode_update uses try_send because ChangeMode's live authority is published even on a full channel and applied at the next drain. Left awaited, now documented on EngineHandle::send and at the site: the ordered must-deliver ops of committed transitions (session/provider reload, Shutdown+SyncSession+SetCompaction) where a drop would desync engine and UI. Removing those wedges needs an engine-side mid-turn op drain or deferred application — a separate slice. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Audit of `thread::sleep` and `std::fs` against the runtime: the named sites were already safe (`cloud_dispatch::wait_ready` runs on the dedicated dispatch thread; `lane::runtime`'s 10ms sleep is test-only), but the sweep found real violations — sync contention retries and filesystem calls reachable from async tool/UI/engine paths. Fixes: - `work_graph::retry_lock`, `App::retry_lock`, and the `/clear`-style lock retries in `commands::user_registry` parked a worker up to 100ms on `try_lock` contention; they now `yield_now` between attempts and still degrade to the existing "state is busy" error. - `TerminalInputPump::pause_for_child_terminal` (event-loop task, up to 500ms) is now async on `tokio::time::sleep`. - `transcribe_local_whisper` ran `Command::output()` plus temp-file I/O inline; the whole body now runs on the blocking pool. - `std::fs` inside `async fn`s converted to `tokio::fs` (session load in apply.rs, notes tool, resource admission, daemon socket lifecycle, doctor config read, speech output) or moved into `spawn_blocking` (MCP registry cache write, Fleet manager lock setup) where the sync helper's retry semantics should be preserved. Convention recorded in AGENTS.md: async code uses tokio::fs/tokio::time or isolates sync work in `spawn_blocking`; `thread::sleep` is for dedicated threads and bounded sync-API retries only. New ratchet `scripts/check-blocking-calls-budget.py` (+ JSON budget, CI step) counts unprotected blocking-call sites — anything outside spawn_blocking/dedicated-thread/test scopes — so the 616-site ceiling can only shrink. Hermetic self-tests in `scripts/test_check_blocking_calls_budget.py`. Verified: cargo check -p codewhale-tui -p codewhale-app-server --locked; cargo nextest run -p codewhale-tui (750 passed across work_graph, mcp_registry, resource_admission, fleet, notes, voice, registry); cargo test -p codewhale-app-server (4 passed); gate self-tests 9/9; budget gate fails on an injected async-scope sleep (exit 1).
`tui/src/execpolicy/` was the last private copy of the policy surface — ~110 call sites already use `codewhale_execpolicy`, and shell.rs was the sole remaining consumer. Migrate the last consumer or do not start: this finishes it. - `matcher.rs` and `rules.rs` move to `crates/execpolicy/src/` as `matcher` and `toml_rules`. The TOML verdict enum is renamed `ExecPolicyDecision` -> `RuleDecision` so it stops colliding with the crate's engine output type of the same name. - The `~/.deepseek/execpolicy.toml` path resolution (a config-home concern) stays in shell.rs as `load_default_policy`; the crate only knows how to parse and evaluate. - The load is now wrapped in `spawn_blocking` — `from_path` does a synchronous `read_to_string` and `execute` runs on the Tokio runtime (blocking-call convention, #6149). - The stale doc reference in `command_safety.rs` pointing at the old `crate::execpolicy::matcher` path is corrected. Verified: cargo test -p codewhale-execpolicy --offline (matcher 7 + toml_rules 4 incl. the shell-spelling deny-pattern suite, all pass); cargo nextest -p codewhale-tui -E 'execpolicy or shell' (424 passed); cargo check -p codewhale-tui --offline clean; dead-code budget 413/413; blocking-calls budget within limits.
…#5529) A wall-time or token death used to report only what the model managed to say in its hand-back — the uncommitted files it actually left behind were invisible, so salvage meant guessing. (Observed: two workers died at the 1800s cap with a complete fix uncommitted and no record of where it lived.) Both terminal paths now inventory the worker's workspace changes against its spawn-time delivery baseline (`DeliveryEvidence::changed_paths`) and append the receipt to the result text: the changed paths (bounded to 12 + a count) and the workspace they survive under. A read-only worker has no baseline and gets no invented inventory; a write-scoped worker that changed nothing gets an explicit "no workspace changes" line. The git/fs reads run under `spawn_blocking` per the #6149 convention, and in the task-error path before the manager write lock is taken. Deliberately not done: `checkpoint.continuable` stays false and `BudgetExhausted` stays non-resumable. Continuation narrows inherited allowances rather than resetting them ("continuation cannot reset its deadline"), so resuming a budget-dead child always fails honestly — flagging the checkpoint continuable would promise a path that cannot exist. Preservation here means naming the surviving work, not laundering the dead allowance. Verified: cargo test -p codewhale-tui --lib --offline subagent (783 passed, 0 failed) incl. new budget_death_preservation_note_names_surviving_workspace_changes; cargo check clean; fmt clean; blocking-calls budget within limits.
A member or profile pin is only drift when the provider's *own* roster no longer offers the id — the same id may still be served by other hosts, so a global retire-the-id rule would be wrong. The operate_fleet report now collects every (provider, model) pin across fleet files (operator + members) and the effective roster, and flags a pin only when a FRESH cached live roster for that exact route exists and does not list it. Stale, failed, or absent rosters prove nothing and are counted under `unverifiable` instead of producing false warnings. Each drifted row names the route and every owner of the pin so the operator can see the full blast radius; the message states the id may still answer and that the pin is left unchanged. No writes, no rewrites — surfacing only, per the no-silent-rewrite contract. Verified: new doctor_fleet_report_flags_pins_absent_from_fresh_live_roster (member pin flagged, operator pin on the listed id not flagged); all 123 doctor tests pass; cargo check clean; fmt clean.
Convert 40+ #[allow(dead_code)] markers to #[cfg(test)] on items whose only callers are unit tests, so they stop compiling into production builds instead of being lint-suppressed there: - runtime_threads: AgentRebindHint/AgentRebindStatus/ collect_agent_rebind_hints and the pending_*_count probes - app.rs: cycle_mode_reverse, accrue_*_cost, displayed_session_cost, push_pending_steer - tools: spec.rs with_trust_mode/with_lsp_manager/is_sandboxable, shell.rs remember_stale_job, subagent queued_mail_depth/ child_was_woken/pending_child_approvals - the whole tideline translation-scaffolding archipelago (#5698 landing slice): work_surface::tideline and work_surface::panels modules, views::tideline_preview module, the settings rail/stage items in views, the inbox cluster in notifications, the theme-list cluster in theme_picker, composer shell/render fns, and the history.rs tideline re-export - palette::ui_theme_from_settings Items with zero callers in any config (_assert_var_handle_shape, _diagnostic_level_label/_diagnostic_path) keep their allow markers — cfg(test) would re-trigger dead_code in test builds. Dead-code budget ratcheted 413 -> 370. Verified: cargo check (lib + --tests) clean; 135 focused tests pass including the tideline golden suites, collect_agent_rebind_hints_*, steer/cost, and palette theme tests. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The method was introduced by the #6141 TOML-rules port; clippy's should_implement_trait flags a from_str that is not FromStr. parse is the same code with a name that does not shadow the trait convention.
Configured MCP servers used to spawn all at session start: connect_all drove every enabled server, and "connecting" was inferred as enabled-minus-connected, so a server nobody asked for still paid a handshake and read as mid-flight. The pool now owns the truth with a `connecting` set marked at spawn and cleared on resolution or abort. The boot pass scopes to the eager set — `required` servers plus ones covered by `tools.always_load`/`allowed_tools` selections; `collect_pending_connects` takes a scope and skips names already in flight. A turn whose selection names an unstarted server spawns its connects beside the boot pass and waits on both under the existing five-second deadline, with the same config-invalidation abort discipline. `mcp_tools` no longer sweeps every turn; required-server gaps still get an honest diagnosis. Lazy dispatch itself was already there through `resolve_advertised_tool`/`get_or_connect` — this wires boot and the tool catalog to match it. Surfaces stop inferring: session-boot rows, the Extensions tab, the launch card, and the pre-event prediction all read the real in-flight set, so a configured-but-unstarted server shows as configured, never connecting. docs/MCP.md documents the lifecycle. Verified: 444 focused nextest tests pass including lazy_boot_leaves_unselected_servers_unspawned and the invalidate-config abort path; cargo fmt clean; dead-code budget 369 at budget; blocking-call budget within.
…ls (#6140) Two cleanups in the stdio MCP server behind `codewhale serve --mcp`: - The loop ran on its own `Runtime::new()` under `block_in_place` and `block_on`ed `registry.execute_full` per request. A stdio JSON-RPC server is serialized by definition, so it now runs `async` on the caller's runtime — `tokio::io::stdin` lines in, `execute_full().await` per call — and the private runtime, `block_in_place`, and every `block_on` are gone. Synchronous config and session-listing reads move to the blocking pool per the workspace convention. - The `deepseek`/`deepseek-reply` tools called `DeepSeekClient:: create_message` directly with their own thread map — a second model authority beside the engine, and provider-exclusive naming to boot. Per the issue ("delete it or generalize through the engine; never a special-cased client call") they are deleted: there is no engine in this surface to route through, and the resources it exposed already exist elsewhere. Configs still naming them degrade to a tool error instead of silently succeeding. Session resource URIs are now `codewhale://session/`. Verified: 16 mcp_server-focused nextest tests pass including a new retirement test; fmt clean; dead-code and blocking-call budgets hold.
…5715) After a force-quit the previous session's work sat on disk but the model had no way to know it existed. Two bounded additions on the existing session store: - `SessionManager::interrupted_workspace_session` finds the newest workspace-scoped session that still holds a crash-recovery checkpoint — the durable sign a session ended mid-turn. It reads metadata only, excludes the live session's own id and sessions this process instance created (boot-owner stamp), and skips malformed records rather than letting one poison the scan. - `session_recovery_hint` renders that as a one-line notice in a new `## Prior Session` block of the session-pinned prompt prefix, computed at engine construction and refresh. Clean sessions get no block, so their prefix bytes are unchanged. - `session_search` / `session_get` give the model bounded, read-only, workspace-scoped recall over the same store: one-line session summaries and an 8-message text tail, labeled untrusted user data. Registered on the standard agent runtime surface; both run their filesystem reads on the blocking pool. Resuming remains the user's decision — the hint tells the model to offer a summary or continuation (e.g. /resume), not to silently continue. Tests: newest-first pick, current-session and current-instance exclusion, other-workspace isolation, settled-session silence, hint render/absence byte stability, tool scoping, bounds, and trust labeling. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
- Hide empty auto-created sessions in the browse view through the shared projection (`SessionQuery::without_empty_auto_created`) — the same definition the launch list and `--continue` apply. `new_selecting` lifts the filter as a last resort so an explicit handoff still lands. - Mark the live session's row with a localized `current` label plus WHALE_ACTION ink — text, not colour alone. - List the full session id in the history preview; the row only has room for the truncated form. - PgUp/PgDn page the session list by one viewport (clamped); the history preview keeps paging on Shift+PgUp/PgDn. Pane titles updated in every complete locale pack; wide layout gives the list 44% instead of 36%. Evidence: 29 session_picker tests, 42 session_control acceptance tests, 51 localization parity tests, 766 session-related lib tests — all green. FMT-OK, dead-code budget PASS (369, exactly at budget). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
) The row already printed `/mcp login <name>` or `/mcp` but the user had to retype it by hand. It now joins `LaunchRowId` — the shared paint/click/keyboard ordering — so Up/Down lands on it and Enter/click type the printed remedy into the composer. Typing beats copying: no clipboard dependency over SSH, and the user sees the command before a second Enter sends it. One `mcp_remedy_command` helper serves the row's tail and its action, so what is painted is what runs. Evidence: 16 launch_card tests green including three new ones covering row-ordering parity, composer fill, and the no-problem noop. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_74341abb-60f8-410c-bc25-7306ad9d2a85) |
|
Claude finished @Hmbown's task in 1m 51s —— View job Review in progress
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if !explicit.connects.is_empty() { | ||
| explicit.connects.abort_all(); | ||
| if let Some(pool) = self.mcp_pool.as_ref() { | ||
| pool.lock().await.cancel_connecting(&explicit.names); | ||
| } |
There was a problem hiding this comment.
🟡 Slow selected MCP servers never connect
wait_for_explicit_mcp_boot aborts a selected server when its handshake exceeds five seconds. Slow cold-start servers restart every turn and never expose their tools.
Learn more
Explicit lazy connects use the five-second UI deadline, while each MCP server can have a longer connection timeout. When the deadline expires, abort_all cancels those handshakes and clears their in-flight markers. The comment above the loop says connects continue for later turns, but only the separate boot pass continues. The next turn starts the selected server from scratch, so a consistently slow cold start can never complete.
Example: An npx MCP server needs eight seconds on a cold cache and has a 30-second connect timeout. Every turn aborts it at five seconds. The expected tool schema never appears, although the configured timeout allows the handshake.
Recommended fix: Detach unfinished explicit connects into an engine-owned background task that stores authority-checked results after the turn proceeds. Keep the five-second bound only on waiting for schemas, not on the connection lifecycle. Ensure config reload and shutdown still abort and clear the exact pending names.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if let Ok(guard) = mutex.try_lock() { | ||
| return Some(guard); | ||
| } | ||
| std::thread::sleep(std::time::Duration::from_millis(1)); | ||
| std::thread::yield_now(); |
There was a problem hiding this comment.
🟡 Lock retries collapse into immediate failures
retry_lock performs all retries without yielding the Tokio task that may hold the mutex. Brief contention now becomes save, restore, or clear failures.
(Refers to this code)
Learn more
std::thread::yield_now yields the operating-system thread, not the current async task. On a current-thread runtime, or when the holder is queued on the same worker, the mutex holder cannot run between attempts. Even on a multithreaded runtime, 100 immediate attempts provide almost no contention window compared with the previous 100 milliseconds. The same mechanism also affects command resets in try_dispatch and work-graph operations in retry_lock.
Example: A task holds plan_state briefly and is ready to release it after its next poll. A session restore calls retry_lock; all 100 attempts finish before the holder is polled, so restore reports that plan state is busy.
Recommended fix: Make these call paths async and retry with tokio::task::yield_now or a bounded tokio::time wait. Where a synchronous API is mandatory, redesign ownership so it does not poll an async mutex from the runtime task.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Recomputed on each refresh (#5715): the prior session's checkpoint | ||
| // may have settled or been resumed since construction. | ||
| let recovery_hint = crate::session_manager::session_recovery_hint( | ||
| &self.config.workspace, | ||
| Some(self.session.id.as_str()), | ||
| ); |
There was a problem hiding this comment.
🟡 Recovery refresh rewrites pinned prompt prefix
A prompt refresh recomputes recovery_hint, so checkpoint changes rewrite the session-pinned prefix. Later turns lose prefix-cache reuse and receive different prior-session context.
Learn more
The repository contract treats the system prompt and tool catalog as a session-pinned KV-cache prefix. Engine construction computes a recovery hint, but every mode, model, goal, or system refresh scans mutable checkpoint state again. Removing a checkpoint or adding a newer interrupted session therefore changes prefix bytes after the session starts. This also contradicts the field's frozen-prefix contract in PromptSessionContext.
Example: A session starts with a hint for interrupted session A. Session A is resumed elsewhere and its checkpoint disappears. Changing mode rebuilds this session's prompt without the hint, invalidating the provider prefix cache and changing prior-session context mid-session.
Recommended fix: Store the construction-time recovery hint on Engine and reuse it for every prompt refresh. If live checkpoint changes must reach the model, append them as user-role history instead of changing the pinned prefix.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let mut sessions = manager.list_sessions()?; | ||
| sessions.retain(|session| { | ||
| workspace_scope_matches(&session.workspace, &workspace) | ||
| && query.as_deref().is_none_or(|query| { | ||
| let query = query.to_lowercase(); | ||
| session.title.to_lowercase().contains(&query) | ||
| || session.id.starts_with(query.as_str()) | ||
| }) | ||
| }); | ||
| sessions.truncate(limit); |
There was a problem hiding this comment.
🟡 Current session pollutes prior search
session_search includes the active session among prior sessions and truncates afterward. The newest active session can displace interrupted work from bounded results.
Learn more
The tool is registered as prior-session recall, but ToolContext supplies only the workspace here. Once the live session has been persisted, list_sessions normally returns it first because it is newest. The workspace filter retains it and the limit is applied before any current-session exclusion. The output then labels that row as a prior session.
Example: The current session and eight older sessions exist in one workspace. Calling session_search with the default limit of eight returns the current session plus seven older sessions. The eighth older session is omitted even though the tool claims to list prior work.
Recommended fix: Thread the active session ID into ToolContext or the runtime session services. Exclude that exact ID before truncating in SessionSearchTool, and reject it consistently in SessionGetTool when these tools are used for prior-session recall.
Was this helpful? React with 👍 or 👎 to provide feedback.
| pub(crate) fn tool_selection_covers_server(requested: &[String], server: &str) -> bool { | ||
| let prefix = format!("mcp_{}_", server.to_ascii_lowercase()); | ||
| requested.iter().any(|name| { | ||
| name.starts_with(&prefix) | ||
| || name | ||
| .strip_suffix('*') | ||
| .is_some_and(|rule| prefix.starts_with(rule)) | ||
| }) |
There was a problem hiding this comment.
🟡 Overlapping MCP names start extra servers
Selecting mcp_a_b_tool also covers server a, because matching accepts every shorter server prefix. Lazy boot starts unrelated servers and waits for them.
Learn more
MCP tool names concatenate the server and tool with underscores, so server names can prefix one another. Existing live routing handles this ambiguity by choosing the longest configured server name in needs_auth_server_for_tool_name. This new coverage predicate independently matches every prefix. Lazy eager-set construction and explicit per-turn connection both use it, so both start the extra server.
Example: Configure servers git and git_enterprise. Selecting mcp_git_enterprise_search is routed to git_enterprise, but this predicate also covers git. Both servers start and the turn can wait five seconds for an unrelated git handshake.
Recommended fix: Resolve each exact requested tool against the configured and dynamic server-name set with the same longest-name rule as runtime routing. Apply wildcard coverage separately, preserving its intentional multi-server semantics.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if "spawn_blocking" in tok or tok in ("thread::spawn", "thread::Builder::new"): | ||
| pending_blocking = True | ||
| continue |
There was a problem hiding this comment.
🟡 Blocking-call gate misses later calls
A call-form spawn_blocking(work) leaves pending_blocking set until the next brace. Blocking calls inside a later unrelated block escape the budget.
Learn more
The scanner assumes every blocking spawn is followed by a closure brace. Rust also accepts a function item or closure variable directly, such as spawn_blocking(read_config). In that form no brace belongs to the spawn, but pending_blocking remains true across lines. The next unrelated { opens a synthetic blocking scope, and matching std::fs or thread::sleep calls inside it are not counted.
Example: spawn_blocking(load).await; if retry { std::fs::read(path); } records zero filesystem sites. The if block is not executed by spawn_blocking, so the read remains inline on the async task.
Recommended fix: Parse the call expression sufficiently to associate protection only with an inline closure body, or conservatively exempt only recognized spawn_blocking(... || { ... }) forms. Add a regression test for a function-argument call followed by an unrelated braced block.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - Blocking-call convention (#6149): code on the Tokio runtime — tool | ||
| handlers, engine tasks, the UI event loop, anything reached through an | ||
| `async` call chain — must not run blocking operations inline. | ||
| `std::fs`/`std::process` calls inside `async` code use `tokio::fs`/ | ||
| `tokio::process`, or move the synchronous work into | ||
| `tokio::task::spawn_blocking` (`utils::spawn_blocking_supervised` for | ||
| fire-and-forget). `thread::sleep` is for dedicated `std::thread`s and | ||
| bounded contention retries in synchronous APIs that are only reachable | ||
| from blocking scopes — an async-path wait uses `tokio::time`. A sync | ||
| helper containing blocking calls must only be called under | ||
| `spawn_blocking` or from a dedicated thread; `scripts/ | ||
| check-blocking-calls-budget.py` ratchets the unprotected-site count. |
There was a problem hiding this comment.
🔍 PR spans many behavior boundaries
This PR combines MCP, sessions, hooks, fleet, UI, runtime, and CI changes. Repository guidance asks layered PRs to keep one behavior boundary per review.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // #6150: the input path never awaits a full op channel. | ||
| if engine_handle | ||
| .try_send(Op::SetStreamChunkTimeout { timeout_secs }) | ||
| .is_err() | ||
| { | ||
| app.status_message = | ||
| Some("Engine busy — setting not applied; try again".to_string()); |
There was a problem hiding this comment.
🔍 Busy notices use legacy status sink
New mailbox-rejection messages write status_message. TUI guidance requires new notices to use typed toasts with explicit levels and lifetimes.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "search".to_string(), | ||
| "apply_patch".to_string(), | ||
| "shell".to_string(), | ||
| "deepseek".to_string(), | ||
| "deepseek-reply".to_string(), | ||
| ] |
There was a problem hiding this comment.
🔍 Retired MCP tools lack migration notice
Configurations naming deepseek or deepseek-reply now expose neither tool. Existing users receive no targeted migration diagnostic for the removed surface.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| - name: Check blocking-calls budget | ||
| if: needs.changes.outputs.heavy == 'true' | ||
| continue-on-error: ${{ github.event_name == 'pull_request' }} | ||
| run: python3 scripts/check-blocking-calls-budget.py |
There was a problem hiding this comment.
🔍 Blocking-call ratchet remains advisory
Pull requests continue after this check fails. New blocking calls can merge despite the newly documented mandatory runtime convention.
Was this helpful? React with 👍 or 👎 to provide feedback.
The 23-field struct variant was the god-payload every ACP/app-server fix had to widen. `Op::SendMessage(TurnSpec)` on both the engine op and the serializable protocol op gives per-turn authority one home; the protocol newtype keeps the internally-tagged wire shape byte-identical, verified by the existing round-trip tests. `handle_send_message` now takes the spec and drops its 23-parameter signature and too_many_arguments allow; protocol parity projects TurnSpec -> wire TurnSpec in one helper. Also converts the audit's one missed input-path site: the agent-focus FollowUpSubAgent send in dispatch_composer_message now uses try_send and reports a full mailbox as "engine busy" instead of awaiting it mid-turn. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Audit of `cargo tree --duplicates`: the only splits the workspace itself caused were base64 (config on 0.22 vs tui on 0.23) and shlex (our 1.3 pins vs cc's 2.0 build-dep). Both now resolve to one version. Every remaining duplicate pair is pinned by an upstream crate at its latest release: oauth2 5 alone holds reqwest 0.12, thiserror 1, sha2 0.10, and tower-http 0.6; portable-pty 0.9 holds bitflags 1 and filedescriptor; rust-i18n-support 4.2.2 still requires globwalk 0.8 and toml 0.8; bindgen 0.72 holds shlex 1 for the rquickjs-sys build; hyper-util holds base64 0.22; shellexpand 3.1.2 caps dirs at <7. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Strip 112 uncommented `#[allow(dead_code)]` across crates/tui/src and let the compiler re-adjudicate each under `-D warnings` in every target: - ~50 items are prod-dead but test-reached: `#[cfg_attr(not(test), expect(dead_code))]` asserts that and self-verifies against future callers. - ~33 items are dead in every build (documented wire captures, reserved enum variants, pre-wired seams): `#[expect(dead_code)]` keeps them honestly. - `install`/`update` (skills) and `log_exec`/`append_log_static` (shell_dispatcher) are used only from the `#[path]`-included integration crate: `#[cfg_attr(test, allow(dead_code))]` covers exactly those compiles. - `utils::ensure_dir` had zero callers and no stated intent: deleted. - `subagent_routing` one cfg_attr was fully stale (used in both builds): gone. Budget ratchets 369 -> 254. Verified: cargo check --all-targets --locked clean, fmt clean, integration 177 passed / 0 failed.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_9e216b10-4c3b-447b-bdf8-b3d7811c8a9e) |
|
Claude finished @Hmbown's task in 1m 38s —— View job Review in progress
|
The always-on session_get/session_search registration (#5715) widened every role surface and the parent prompt+catalog envelope without updating the drift fixtures: re-pin the role constants and the read-only inline list, and raise the measured parent ceiling (85,913B actual at commit time). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Claude finished @Hmbown's task in 1m 44s —— View job Review completeTodo list
This push added three commits since the last review round (
New in this push (
|
#6105 bumped rustls 0.23.43 -> 0.23.44 and landed on main, but the advisory still fires: cargo-deny's own output says "Solution: Upgrade to >=0.23.45". 0.23.44 is not the patched release, so every open PR kept a red cargo-deny (advisories) and cargo-audit for a reason nobody's diff caused. RUSTSEC-2026-0285 is "TLS 1.3 handshake messages incorrectly accepted across encryption level boundaries" — it reaches us through every HTTP path we have (reqwest 0.12 and 0.13, hyper-rustls, tokio-rustls, rustls-platform-verifier), so this is the whole provider wire, not a dev-only dependency. `cargo update -p rustls` only: one package moved, 102 unchanged dependencies left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch added two tools. `web/lib/facts.generated.ts` is derived and was regenerated in aeabd5a, but `docs/public-surface-facts.json` holds the same fact by hand and still said 76, so `public-surface-contract.test.ts:142` — which exists precisely to keep the two in step — failed with "expected 76 to be 78" and took `Lint & Type Check` down with it. Only `sourceCandidate.toolCount` moved; version 0.9.13, providerCount 47 and the sandbox backends already agreed with the derived facts. The duplication is the real defect: one fact, two files, one of them manual. Worth folding into the derive step, but not in a CI-unblocking commit. Checks: `cd web && npm test` — "Test Files 51 passed (51) / Tests 470 passed (470)", up from 469 passed / 1 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @Hmbown's task in 57s —— View job Review completeTodo list
This push added exactly two new commits since the last review round (
New in this push
No correctness bugs, provider/model/route safety issues, reuse/dead-code, test-coverage gaps, or security issues in this increment — both commits are mechanical and outside application logic. |
`session/new` returned `codewhale-<uuid>`; `session/list` advertised bare uuids; `session/load` accepted only the bare form and answered `-32602` for anything else. A client has no id for a session it just created other than the one `session/new` handed it, so persisting and replaying that id — the normal thing to do — could never resume the session. The prefix was the visible seam, not the defect. `session/new` inserts only into the process-local `self.sessions` map (a cap-64 LRU); nothing on the ACP path writes the durable store, and `session/list`/`session/load` read that store. So the two methods addressed disjoint namespaces, and merely stripping the prefix would have left the bug intact with a different error message: a bare uuid from `session/new` still has no durable record to find. That also explains the reporter's last observation — naming a session file after the prefixed id changed nothing, because `resolve_session_id_prefix` filters on ids from `list_sessions()`, not on filenames. Two changes: - `new_session` mints a bare uuid, the shape `create_saved_session` produces and `session/list` advertises, so there is one id namespace instead of two. - `load_session` resolves an id this connection already holds before consulting the store. That is what makes a `session/new` id loadable at all, and it also makes reloading an already-loaded durable session free of store side effects. Deliberately NOT done: persisting ACP sessions on turn commit. That would make a `session/new` id survive a restart, but it also deepens `acp_server.rs` as a second conversation store — exactly what #6088 exists to retire, and what #5835 would have to undo when ACP moves onto the real thread/turn runtime. The in-memory session remains honestly ephemeral: an id evicted from the cap-64 LRU still fails, and it should. No code or test depended on the old prefix (checked repo-wide; the other `codewhale-` matches are remote-bridge unit names and update asset stems). Checks: `./scripts/dev-test.sh tui acp_server` — 57 tests run: 57 passed, 12669 skipped. Includes the two new regressions and the pre-existing `session_list_and_load_reach_the_durable_codewhale_sessions`, which still passes, so the durable path is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| warning: Some(WHALE_IGNORED_WARNING.to_string()), | ||
| }); | ||
| } | ||
| let mut scope_loaded = false; |
| }); | ||
| } | ||
| } | ||
| let mut scope_loaded = false; |
| // mark the files the bounded loader's own selection walk picks. | ||
| let fragment_candidates = fragment_candidates_for(imports); | ||
| let selected: std::collections::BTreeSet<PathBuf> = | ||
| codewhale_core::fragments::selected_project_instruction_candidate_files( |
| }, | ||
| bytes: file_len(&path), | ||
| warning: if selected_file && !loaded { | ||
| Some("selected but empty or unreadable".to_string()) |
| path: file.clone(), | ||
| exists: true, | ||
| status: if loaded { | ||
| InstructionSourceStatus::Loaded |
|
|
||
| // `.codewhale/constitution.json`, workspace → repository root. The loader | ||
| // stops at the first existing candidate whether it parses or not, so | ||
| // later candidates — even ones that exist — are never evaluated. |
`Lint` has been red on every push to this branch. CI runs the release gate
shape — `cargo clippy --workspace --all-targets --all-features` — so it lints
test targets too, which is where five of the seven were hiding.
- `runtime_api.rs`: `user_commands.get(**alias)` — explicit deref the compiler
does anyway (`explicit_auto_deref`).
- `session_picker.rs`: `Vec::new()` then an immediate `push`
(`vec_init_then_push`); now one `vec![]`.
- `session_manager.rs` and `tools/session.rs` (×3): `&home.join("codewhale")`
borrowed a temporary that `EnvVarGuard::set` takes by value anyway
(`needless_borrows_for_generic_args`).
- `markdown_render.rs`: `probe_incremental_stream_cost` prints its per-append
timings, which `#![deny(clippy::print_stdout)]` forbids module-wide. The
printing is the point of a probe, so the ban is lifted locally with a reason,
exactly as `core/engine/tests.rs` already does for its four probes. Left
running rather than silenced: it is the measurement #6196 is built on.
No behavior change; the probe keeps printing and every other edit is a
compile-time simplification.
Checks: the exact CI invocation —
`cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
-A clippy::uninlined_format_args -A clippy::too_many_arguments
-A clippy::unnecessary_map_or` — exits 0, from 7 errors.
`./scripts/dev-test.sh tui` on the same tree: 12707 tests run, 12707 passed,
19 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @Hmbown's task in 1m 27s —— View job Review completeTodo list
This push added exactly two new commits since the last review round (
New in this push
No provider/model/route safety, reuse/dead-code, test-coverage, or security issues in this increment — neither commit touches provider/model selection, exec policy, or secrets. |
`Test (windows-latest)` has not compiled on this branch: five errors, all in shell_dispatcher.rs, and all the same mistake seen from two sides. Which `ShellKind` variants are live is exactly a platform split. `detect` builds `Pwsh`, `WindowsPowerShell`, `Cmd` and `Bash` inside `cfg(windows)` (:446-468) and `Sh` and `Custom` inside `cfg(not(windows))` (:471-504). The annotations from the #5587 sweep were written from the unix side only: the four Windows variants carried an unconditional `cfg_attr(not(test), expect(dead_code))`, and `Sh`/`Custom` carried none. On unix that is right and CI stayed green. On Windows both halves invert: the four expectations go unfulfilled because those variants *are* constructed, and `Sh`/`Custom` become genuinely unconstructed. `expect` is an error when unfulfilled, so the lib failed to compile before a single test ran. Each annotation now names the platform it is dead on. No variant is silenced on a platform where it is live, so the ratchet #5587 installed still bites on both. Verification, stated honestly: `cargo check -p codewhale-tui --lib --locked` is clean on unix, and the full CI clippy gate (`--workspace --all-targets --all-features -D warnings`) exits 0. The Windows half could NOT be checked locally — `cargo check --target x86_64-pc-windows-msvc` cannot build `ring`, whose build script needs an MSVC-targeting C compiler this host does not have. The Windows claim rests on the cfg structure above, and CI is its first real test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @Hmbown's task in 1m 24s —— View job Review completeTodo list
This push added exactly one new commit since the last review round (
New in this push (
|
Four PTY suites fail on any developer machine that happens to be running
Ollama, and pass everywhere else. The frame from a failing run says why:
Note Provider switched: deepseek -> ollama
Model: deepseek-flash -> nomic-embed-text:latest
Endpoint: localhost:11434
The startup adoption probe finds a live :11434 catalog and rewrites the launch
screen — so `wait_for_text("Choose your model provider")` never matches and the
test times out. Nothing is wrong with the product here; the harness simply is
not as sealed as it claims.
`seal_home` exists so "the spawned binary cannot read or mutate the developer's
real user config", but it only seals the filesystem. This leak arrives over
loopback, so no amount of HOME redirection closes it.
`spawn_local_ollama_adoption_probe` is already inert under `cfg(test)`, and that
guard looks sufficient until you notice the PTY suites spawn the real binary,
where `cfg(test)` is not set.
So the probe gains an explicit env opt-out and `seal_home` sets it, next to the
config-path seals and for the same reason.
Scope, stated honestly: this fixes the developer-machine failure, which is the
one reproduced here. CI's failures on this branch ran 60-66s against 18s
locally, and a GitHub runner is unlikely to have Ollama on :11434 — so this may
not be the same defect CI is hitting, and CI is the test of that.
Checks: `cargo nextest run -p codewhale-tui --test cucumber --features
long-running-tests -E 'test(/_pty::/)'` — 9 tests run: 9 passed (1 slow), 28
skipped, from 5 passed / 4 failed on the same machine before this change.
CI clippy gate exits 0; `cargo fmt --check` clean on both files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @Hmbown's task in 46s —— View job Review completeTodo list
This push added exactly one new commit since the last review round (
New in this push (
|
|
Overnight CI triage on this branch. Five of the six red checks are fixed; one is not mine and is described below so it does not get lost. Fixed on this branch
Also on the branch: Not mine:
|
Opening a saved session could fail with "This session belongs to another Runtime host" while the store it names was sitting on disk, validating, and held by nobody: `validate_existing_store` accepted it, the process-owner lock was free, and no other Codewhale process was running. The predicate was not wrong, it was incomplete. The switch path can rebind a conversation but cannot carry a store's durable work into the running host — queued tasks, pending approvals, agent mail, scope-pinned automations — so recovery was scoped to "there is nothing on disk to abandon", i.e. a *missing* store. That policy has no case for the state a force-quit actually leaves: a store that exists, is ownerless, and is empty. Refusing it protected nothing and made the session unopenable. `RuntimeStoreBinding::has_no_durable_work` adds that case. A store qualifies only when it is confined to `<state>/sessions/<id>/runtime[-recovered-*]` with no symlink on the path, every work directory is empty, and `state.json`'s sequence is still at its initial value — because events can be appended and later pruned, and the sequence remembers. Anything unreadable, unconfined or non-empty fails closed, so the guarantee the guard exists to protect is unchanged; the seven pre-existing tests that pin it still pass untouched. Deliberately NOT used as the predicate: lock-freeness. A crashed host leaves a store unlocked whether or not it holds work, so adopting on "the owner lock is free" would be a policy reversal that silently drops queued work, not a bug fix. Emptiness is the discriminator that fixes the reported case without weakening anything. The refusal message was also describing the wrong thing — it claimed another host owned the session, when what it means is that the saved store holds work this process cannot carry across. It now says that. Confinement checks are now shared by both predicates via `is_confined_session_store` rather than duplicated. Possibly also #6185(a): an empty transcript on resume with an intact journal is what every early return in `apply_loaded_session_with_goal` produces, and this guard is one of them. Asked the reporter there to confirm the status-line text before claiming it. Checks: `./scripts/dev-test.sh tui` — 12708 tests run, 12708 passed, 19 skipped. `./scripts/dev-test.sh tui runtime_store_binding` — 8 passed, including the new `empty_existing_runtime_store_reports_no_durable_work`, which asserts each work directory is load-bearing on its own so a directory added to `open` without being added to the list is caught. CI clippy gate exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @Hmbown's task in 1m 33s —— View job Review completeTodo list
This push added exactly one new commit since the last review round (
New in this push (
|
…6207)" This reverts commit 68431ba. CI caught what my local runs did not: `runtime_store_binding_survives_launch_snapshot_and_resume` fails at its line 284 assertion on macOS and ubuntu, while passing in isolation here even under `--all-features`. Passing alone and failing under the full suite is the signature of a race, and the race is in the predicate I added. `has_no_durable_work` reads emptiness at one instant. It has no liveness check, so a store that is empty *right now* because the host holding it has not flushed yet is indistinguishable from a store that is empty because it was abandoned. That test has a live foreign TaskManager holding the store; under load the adopt path wins the race, the switch succeeds where it must refuse, and the assertion sees a different error. Two further gaps, both mine: - The predicate only walks the runtime store's own directories. Scope-pinned automations are recorded outside them, so an automation whose execution scope matches the binding is durable work my check cannot see. The review that scoped this said the predicate needed "no automation whose execution_scope matches the binding"; I did not implement that. - I reported that the seven pre-existing tests "pass untouched". That was true of the run I did and false in general, and the distinction matters precisely here. This sits on a data-loss boundary — queued tasks, approvals, agent mail, automations — so it comes out rather than being patched forward at speed. #6207 stays open with the corrected design: adopt only when the store is both provably empty *and* provably unheld, with automation scope included in "empty", and the shape of the liveness check settled before any of it is written. The behaviour returns to refusing, which is the pre-existing bug the issue reports — a worse user experience than the fix, and a better one than silently dropping someone's scheduled work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NWzjx9Q7Mw2G7K8rpiJy9p
|
Claude finished @Hmbown's task in 59s —— View job Review completeTodo list
This push added exactly one new commit since the last review round ( Why it was reverted (author's own diagnosis, and it holds up)The commit message is unusually precise about its own failure, and it's worth taking at face value rather than re-deriving independently — it names a real race the prior review round's static analysis couldn't have caught:
This is a good outcome for the branch: a CI-only race (empty-by-timing vs. empty-by-abandonment) surfaced after merge, and rather than patching around it or weakening the test, the author reverted cleanly and wrote down precisely what was wrong and what a correct fix would still need (a liveness check, plus the missing automation-scope check). That's exactly the "misconfiguration fails loud" / write-down-what-a-design-does-not-do discipline this same branch's own No new correctness bugs, provider/model/route safety issues, reuse/dead-code, or security issues in this increment — it's a pure revert. Standing findings (unchanged, still present in
|
|
Update, including a retraction. Reverted: I also need to correct something I wrote earlier in this thread: I said the seven pre-existing binding tests "pass untouched", offering that as evidence the policy widened by exactly the empty case. A green targeted run is not evidence about a test whose failure mode is ordering-dependent, and I presented it as though it were.
Raising it needs a human to approve provider spend, so I have not. It is also a size signal — this branch is large enough to need five review passes. Still standing from the earlier triage: the rustls advisory ( Still not mine: |
| // uuid shape `session/list` advertises for durable sessions. | ||
| assert!( | ||
| !session_id.starts_with("codewhale-"), | ||
| "session/new must not mint a prefixed id, got {session_id}" |
| "session/new must not mint a prefixed id, got {session_id}" | ||
| ); | ||
| uuid::Uuid::parse_str(&session_id) | ||
| .unwrap_or_else(|e| panic!("session/new must mint a bare uuid, got {session_id}: {e}")); |
`Test (windows-latest)` compiles again after aca2174 and now runs, which surfaced the next thing: 15271 tests, 1 failed — `get_v1_workspace_instructions_lists_effective_sources` panicking on "shadowed workspace instructions row". Two separator bugs, one in the contract and one in the test. The contract: `relative_path` was built with `relative.display().to_string()`, which emits the native separator. A wire field describing a repo-relative path would then read `.codewhale/rules/style.md` from a Unix host and `.codewhale\rules\style.md` from a Windows one, forcing every client to branch on the server's OS. It is now joined with forward slashes on all platforms. The absolute `path` alongside it stays native, because that one is only meaningful on the machine that produced it. No consumer reads this field yet — the route landed hours ago in d5291df and GPUI has not wired it — so the contract is free to fix now and would not have been later. The test: the shadowed-row finder matched the *native* `path` against the literal `".codewhale/instructions.md"`, so on Windows it never matched and the `.expect` fired. It now compares separator-agnostically. Note this does not weaken the assertion — if that row is absent for any other reason the test still fails, which matters because the Windows half of this cannot be reproduced here. The neighbouring `AGENTS.md` assertion passed only because that literal happens to contain no separator. Checks: `./scripts/dev-test.sh tui workspace_instructions` — 1 passed; `./scripts/dev-test.sh tui runtime_api` — 243 tests run, 243 passed, 12483 skipped. `cargo fmt --check` clean on both files. The Windows half rests on the separator reasoning above and on CI, for the same reason as aca2174: this host cannot build for `x86_64-pc-windows-msvc`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NWzjx9Q7Mw2G7K8rpiJy9p
|
Claude finished @Hmbown's task in 45s —— View job Review completeTodo list
This push added exactly one new commit since the last review round (
New in this push (
|
…indow (#6224) `Test (windows-latest)` is a required check and this was the one test failing it: 15271 run, 1 failed, green on ubuntu and macOS. assertion failed: sink.failed() The cause is a wall clock in the code under test, not flakiness. `Target::send` drops stale audio on purpose: fn current(&self) -> bool { self.requested.elapsed() <= Duration::from_millis(500) } ... if !self.current() { return Ok(()); } // never reaches try_send and `failed` is only set when `try_send` observes `Disconnected`. `sink.target()` stamps `requested` at call time, so the test was implicitly requiring the worker to consume that target within 500 ms. On a slower Windows runner it does not, `send` returns early, no failure is ever recorded, and the precondition assert fires. The staleness guard is correct product behaviour — audio requested half a second ago should not play — so the test is what needed fixing, not `send`. The real assertion in this test is at the end (`after.time_ms > before.time_ms`, `after.hollow`): output failures must not stop the world. `sink.failed()` is only the precondition that a failure happened at all. That precondition now runs through the same path with the wall clock removed — a fresh target, a live `active()`, and a `try_send` that observes `Disconnected` because `packets` was dropped. It is not weakened: a receiver that failed to disconnect still fails the test, and everything downstream is untouched. Checks: `./scripts/dev-test.sh tui pet_watch` — 16 tests run, 16 passed; `./scripts/dev-test.sh tui output_failures_do_not_stop_the_world` — 1 passed. CI clippy gate (`--workspace --all-targets --all-features -D warnings`) exits 0; `cargo fmt --check` clean. The Windows half rests on the mechanism above and on CI, since this host cannot build for `x86_64-pc-windows-msvc`. Touches another agent's subsystem, so it is one assertion and a comment, with no change to `audio.rs` or to the worker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NWzjx9Q7Mw2G7K8rpiJy9p
|
Claude finished @Hmbown's task in 1m 15s —— View job Review completeTodo list
This push added exactly one new commit since the last review round (
New in this push (
|
`portable` (Pet conformance) has never passed. It did not regress at
`a06d2296`; that is only when the workflow started running. The failure goes
back to `334c720057`, whose message claims "380 matching checkpoints across
v1/v2, with pinned v1 digests unchanged" — a gate that was not true of the
content it committed. `pet_sim.rs` is byte-identical at that commit and at
HEAD, the v1 fixtures were created there and never changed, and Rust has never
matched them.
The cause is not the simulation. `field_target` is line-for-line identical
between `pet_sim.rs` and `pet/src/core/pet-sim.ts` — every channel, every
constant — and both mulberry32 implementations agree (Rust's `u32`
`wrapping_*` is exactly `Math.imul`/`>>>`). The tape's label sequence matched
byte-for-byte all along: the two cores agreed on what the pet was doing and
disagreed on where its particles were, from frame 0.
The authored body is checked in **five** times:
pet/whale-points.tsv f54c19a76021
pet/dist/whale-points.tsv f54c19a76021
pet/rs/whale-points.tsv f54c19a76021
pet/public/whale-points.tsv f54c19a76021
crates/tui/src/tui/ambient_life/whale-points.tsv 9e56d5280d9b
Four agree; the Rust product core — the one the TUI actually renders — was the
lone dissenter, with a different derivation of the same 980-point shape. No
sync step produces these files, so the existing `git diff --exit-code` guard on
embedded sources could never see the drift.
Rust is canonical (founder's call), so the four pet copies are synced to it and
the v1 fixtures are regenerated from the Rust binary. That makes verify.sh's
fixture check an assertion that TypeScript matches the Rust-derived contract,
rather than TypeScript agreeing with itself.
A CI step now compares the copies directly, because nothing else can: this is
five hand-maintained copies of one fact, and the drift was invisible for days.
Folding them into one generated artifact is the real fix and is left as
follow-up.
Checks: `./pet/verify.sh --no-swift` exits 0 — 380 checkpoints across 2
expression versions and 4 tapes/modes, all eight combinations PASS, up from a
hard failure on the first v1 diff. `npm --prefix pet run check` — 68 pass, 0
fail. `npm --prefix pet run sync` leaves `git diff --exit-code` on the embedded
sources clean.
Refs #6227
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NWzjx9Q7Mw2G7K8rpiJy9p
|
Claude finished @Hmbown's task in 2m 9s —— View job Review completeTodo list
This push added exactly one new commit since the last review round (
New in this push (
|
What / why
Second stacked slice run on the v0.9.14 milestone, stacked on
origin/main(433685b2, post-#6161 merge). Nine issue slices plus one lint fix, one commit per issue, each verified before commit.Closes
codewhale execopt into hooks and typed permission rules #6099 —codewhale exec --hooksopts into configured hooks and typed permission rules.spawn_blocking/block_in_placeat the async boundary), the citedthread::sleep/std::fssites audited (thecloud_dispatchpoll loop already runs on a dedicatedcw-dispatch-*OS thread, not a runtime worker), andscripts/check-blocking-calls-budget.pyratchets the remaining inventory.tui/src/execpolicy/deleted; matcher + TOML rules live incodewhale-execpolicy;tools/shell.rs(the last consumer) uses the crate.required/always_load/tool-selected servers, the per-turn backstop is removed,/mcpand the launch card distinguish idle from in-flight connects.serve --mcpstdio loop is async-native (noblock_in_place); the DeepSeek-specificdeepseek_chat/deepseek_reasonertool calls are retired.session_search/session_gettools (workspace-scoped, bounded, untrusted-data-marked) plus a one-line recovery hint injected only when a durable checkpoint marks an interrupted prior session.SessionQuery::without_empty_auto_created), explicit selection still lands via a filter-widening fallback, the live session's row gets a localizedcurrentlabel +WHALE_ACTIONink, the preview lists the full session id, PgUp/PgDn page the list (clamped) with Shift+PgUp/PgDn keeping history scroll, wide layout gives the list 44%.LaunchRowId(the shared paint/click/keyboard ordering): Up/Down lands on it and Enter/click types the printed remedy (/mcp login <name>or/mcp) into the composer — no clipboard dependency, and the command is visible before a second Enter sends it.Refs (partial slices — status comments on the tracker)
Op::SendMessage→TurnSpecextraction remains.doctorflags fleet/agent model pins missing from the live roster; propagation/migration is not implemented.#[allow(dead_code)]markers converted to#[cfg(test)]and the budget ratcheted to 369; later phases remain.Commits
6e524d07feat(exec): opt in to configured hooks withcodewhale exec --hooks(feat(exec): letcodewhale execopt into hooks and typed permission rules #6099)ac6a881dfix(fleet): inherit the operator route instead of pinning it (Fleet members over-pin their model, so nothing follows the provider when a vendor retires an id #6037)4d70e050fix(tui): stop awaiting the op channel on the input path (Op::SendMessage god-payload → TurnSpec, and prove the UI never awaits send() on the input path #6150)47834cc6fix(runtime): keep blocking calls off Tokio workers (Blocking calls in async paths: audit thread::sleep and std::fs in tool paths, pick one convention #6149)290938ccrefactor(execpolicy): port TOML rules into codewhale-execpolicy (Finish the execpolicy migration: one consumer of tui/src/execpolicy is left #6141)a4c4aa29fix(subagent): name surviving workspace work in budget-death receipts (Sub-agents cannot reliably execute: wall-time deaths lose uncommitted work, provider-route failures block dispatch, shell tooling needs workarounds #5529)47666265feat(doctor): flag fleet model pins missing from the live roster (Model pins don't propagate: fleet members and agent profiles keep a retired id after the provider moves on #6035)890b7d03refactor(tests): gate audited test-only helpers behind cfg(test) (Dead-code sweep phases 2-4: 75 test-only markers, ~242 stale allows, blanket-allow conversions #5587)711c538erefactor(execpolicy): rename from_str to parse for clippybe214c86feat(mcp): connect servers lazily instead of all at boot (MCP: connect servers lazily at point of use instead of eagerly at boot #6033)75e3a819refactor(mcp): make serve --mcp async-native and retire the model tools (MCP server: retire the DeepSeek-specific tool calls and make the stdio loop async-native #6140)eefdcb42feat(session): make prior interrupted sessions visible to the model (Session recovery is invisible to the model: after a force-quit, the agent cannot find what it was doing #5715)7192b50afix(tui): make the session picker browsable and current-aware (feat(tui): Session Picker UX — hide empty auto-sessions, highlight current session, page-scroll the list, widen the list pane #6014)c5ed2e6cfeat(tui): make the launch card's MCP problems row run its remedy (feat(tui): launch-screen MCP block — decide copy/paste (and click) affordances; it is read-only by design today #6085)Gate output (what actually ran on this branch)
cargo fmt --all -- --check→ cleancargo check -p codewhale-tui --all-targets --locked→ cleanscripts/check-dead-code-budget.py→ PASS (369, exactly at budget) ·check-blocking-calls-budget→ PASS (616 sites)compaction-count flake); it fails identically onorigin/mainwithout these changes.npm test,npm run check:web— hosted CI on this PR is the gate.Generated with Devin
Note
Medium Risk
Changes MCP connection timing, engine turn admission, and daemon async I/O on core runtime paths; wire
send_messageJSON stays compatible but behavior shifts when servers connect.Overview
This PR lands a v0.9.14-style slice across runtime conventions, MCP boot, sessions UI, and execpolicy structure.
Blocking calls (#6149): Documents the Tokio rule in
AGENTS.md, adds a CI blocking-calls budget ratchet, moves daemon socket setup totokio::fs, runs local Whisper transcription onspawn_blocking, and replacesthread::sleepwithyield_nowon async user-command paths.MCP (#6033): Session boot connects only required / always-load / tool-selected servers;
mcp_toolsno longer sweeps every server each turn. Explicit tool selections start and await their own connects (with deadline/cancel), and “connecting” is tracked from the pool instead of inferred as enabled−connected.Sessions (#5715, #6014): Injects a recovery hint into the frozen system prompt when a prior interrupted checkpoint exists; the picker gets current-session labeling, full ID in preview, updated PgUp/PgDn hints, and passes
with_current_sessionwhen opened.Protocol / engine: Extracts per-turn fields into
TurnSpec(Op::SendMessage(TurnSpec)and protocol twin) without changing the tagged JSON shape; refactorshandle_send_messageto take one spec struct.Execpolicy (#6141): Consolidates TOML rules in
codewhale-execpolicy(matcher,toml_rules,RuleDecision,parse); drops default-path loaders from the crate.Also documents
codewhale exec --hooks, bumps base64/shlex, and updates session-related locale strings.Reviewed by Cursor Bugbot for commit 137fb70. Bugbot is set up for automated code reviews on this repo. Configure here.