One worker system; retire Keychain product path

[Hmbown]

Summary

• Fleet/sub-agents are one worker: spawn(prompt) inherits the parent. Roles are labels, not a permission matrix. No preset catalog.
• Retired the Codewhale Keychain/OS-keyring product path. CODEWHALE_SECRET_BACKEND=system|keyring is a no-op. Account sessions and provider keys use ~/.codewhale/secrets/ (mode 0600), not world-readable files.
• /mcp connect github chains add+login. Docs describe the simple worker model.

Test plan

• cargo test -p codewhale-secrets (61 tests) including session_secrets_use_the_file_store_not_keychain and retired_keychain_env_diagnoses_the_file_store
• Focused tui tests: simple_worker, prompt_only_spawn_inherits, root_operate_dispatch
• Confirm codewhale doctor does not mention Keychain
• Confirm CODEWHALE_SECRET_BACKEND=system does not prompt Keychain

Closes the founder ask on cwc#154 (CLI side): local keyring is not a parallel home.

Made with Cursor

---

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[claude]

Claude finished @Hmbown's task in 6m 15s —— View job

---

Review: PR #5632 — One worker system; retire Keychain product path

• Gather context (read diff, changed files, AGENTS.md/CLAUDE.md)
• Understand request (code review of PR diff)
• Review: correctness bugs
• Review: provider/model/route safety invariant
• Review: reuse/simplification
• Review: test coverage
• Review: security (secrets handling)

The Keychain retirement (crates/secrets) is clean and well tested. The "one worker" model change is the intentional core of the PR, and the parent-ceiling design (WorkerRuntimeProfile::derive_child intersecting, never widening) is sound and tested for the nested-spawn case. Two issues below are concrete, code-verified problems in the root-spawn path, not style nits.

1. Plan mode no longer confines a root-spawned worker (correctness/safety)

apply_session_spawn_defaults (crates/tui/src/tools/subagent/mod.rs:9806-9814) now grants a root spawn (spawn_depth == 0) full write + full shell unconditionally:

fn apply_session_spawn_defaults(runtime: &mut SubAgentRuntime) {
    if crate::simple_worker::root_child_inherits_parent(runtime.spawn_depth) {
        runtime.accept_edits = true;
        runtime.accept_verification = true;
        runtime.allow_shell = true;
        runtime.worker_profile.permissions = crate::worker_profile::PermissionSet::full();
        runtime.worker_profile.shell = crate::worker_profile::ShellPolicy::Full;
    }
}

Previously this was gated on runtime.spawn_depth == 0 && runtime.parent_mode == AppMode::Operate. The parent_mode check is now dropped entirely — only spawn_depth is checked. But crates/tui/src/core/engine.rs:4216-4253 deliberately zeroes shell for AppMode::Plan (runtime_allow_shell = input_policy.allow_shell && !matches!(input_policy.mode, AppMode::Plan)) and installs a FleetRole::Planner profile for Plan mode specifically so a Plan-mode session stays read-only (per docs/authority.rs:44: "Plan -> read-only: no shell, no trust, Suggest approvals"). apply_session_spawn_defaults now overwrites both of those back to full write/shell for any first-level agent(prompt=...) call, regardless of the session's mode. Combined with WorkerRuntimeProfile::for_role always returning PermissionSet::full()/ShellPolicy::Full now (see #2), a user in Plan mode who spawns a worker gets a fully write+shell-capable child — Plan mode's read-only guarantee is bypassed for subagents.

The new test root_spawn_inherits_parent_without_disabling_safety (crates/tui/src/tools/subagent/tests.rs:12747) actually documents this: it sets runtime.parent_mode = AppMode::Agent and asserts full write/shell are granted — but there's no equivalent test asserting the previous guarantee still holds for AppMode::Plan. If dropping the mode gate is intentional, please add a Plan-mode regression test confirming that's a deliberate product decision (it looks like an unintentional side effect of the mode check simply not being ported over rather than a decision the PR discusses — the PR description doesn't mention mode-gating changes at all).

Fix this →

2. Several pre-existing tests now assert the old contract and will fail (test coverage / correctness)

WorkerRuntimeProfile::for_role (crates/tui/src/worker_profile.rs:201-206) was changed to always return PermissionSet::full() / ShellPolicy::Full for every FleetRole, including Scout/Reviewer/Planner/Consultant/Verifier. This is the intended design (roles are labels now), and the profile-level tests were updated accordingly (for_role_is_one_worker_system, consultant_keeps_high_reasoning_without_a_weaker_tool_list). However, two pre-existing tests in crates/tui/src/tools/subagent/tests.rs were not touched by this diff and directly assert the old per-role restriction, which this change removes:

• role_posture_blocks_writes_and_shell_for_read_only_roles (line 6246): asserts !role_posture_permits(&role, ApprovalRequirement::Suggest) for Scout/Reviewer/Planner/Verifier. role_posture_permits (mod.rs:14173) reads profile.permissions.write, which is now always true — this assertion will fail.
• read_only_inspection_roles_cannot_write_through_text_filters (line 7549): expects registry.envelope_refusal(...) to return Some(...) for a write-capable sort -o ... command run by a Scout/Reviewer/Planner. envelope_refusal → execution_envelope().write → !write_is_denied() → self.runtime_profile.permissions.write, now always true, so the envelope no longer refuses and .expect(...) will panic.

The PR's own test-plan checklist has "Focused tui tests: simple_worker, prompt_only_spawn_inherits, root_operate_dispatch" unchecked, which is consistent with these not having been run. Please run cargo test -p codewhale-tui tools::subagent:: and update or remove the tests that encode the retired per-role posture (there may be others beyond these two — read_only_inspection_roles_execute_pwd_and_absolute_git_log and the role loop around tests.rs:8061-8138 use a hand-seeded deny list rather than relying on permissions.write, so they looked more likely to still pass, but should be verified too).

Minor / secondary

• crates/tui/src/simple_worker.rs:38-53 — is_hard_carve_out is a plain substring match on lowercased command text ("delete from users", "stripe charges", etc.). As the sole remaining subtraction for payments/customer-data safety in a model that otherwise gives full shell to every spawned worker, this is trivially bypassable (extra whitespace, variable indirection, a different phrasing, a script file). Worth a comment noting it's a best-effort net, not a hard boundary — or strengthening it (e.g., normalize whitespace) since "hard carve-out" implies stronger guarantees than substring matching provides.
• crates/secrets/src/lib.rs — SecretBackendDiagnosticKind::System (lib.rs:700) is now unreachable from diagnose_secret_backend (only File/Unknown are ever constructed in product code) but the variant and its doctor.rs:136-138 match arm remain. Fine to keep for the public-API/serde surface, but worth confirming nothing external still relies on seeing "system" in codewhale doctor output as the test-plan checklist requests.
• crates/tui/src/tools/subagent/mod.rs:12888 — validate_spawn_write_contract's allow_prompt_only_general parameter is now dead (let _ = allow_prompt_only_general;) since the prompt-only-read-only branch it used to gate was removed. Consider dropping the parameter from all three call sites rather than keeping an unused one around.
• The new session_namer.rs / SessionTitleSource plumbing (not mentioned in the PR summary) computes a NamerCompletionSpec and logs it via tracing::debug! in frame.rs:439-446, but nothing ever issues the actual completion request or calls apply_generated_title/sanitize_generated_title outside of tests — SessionTitleSource::Generated is never produced in production code. This looks like scaffolding for a feature that isn't wired up yet; if intentional (follow-up PR), fine, but as shipped here should_auto_name/namer_completion_spec build a spec that's never sent anywhere.

Provider/model/route safety: no issues found — this PR doesn't touch route/model selection logic, and I didn't find any provider-prefix-based inference introduced.
(branch)

[devin-ai-integration]

Devin Review found 3 potential issues.

[devin-ai-integration]

🔍 User title provenance dropped on autosave

SessionMetadata gains title_source with the contract that a user rename always wins, but merge_persisted_lifecycle copies the persisted title without copying title_source, so autosave rewrites a User-renamed session back as Truncation. This is currently latent: apply_generated_title is only called in tests and build_session_snapshot merely logs a namer spec, so no title is overwritten yet. It becomes a real regression once the namer is wired to apply generated titles.

(Refers to this code)

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 Parallel spawns forced into worktrees, breaking non-git workspaces

Any second concurrent sub-agent is forced into a git worktree (should_isolate_worktree(false, parallel)) whenever another agent is running, where running_count() counts every live agent and neither agent's write capability is checked. In a workspace with no git repository the worktree creation fails and the spawn errors out; in git repos, read-only fan-out gets spurious worktrees and branches.

Prompt for agents

The auto-worktree block forces worktree isolation on the second and later concurrent sub-agent spawns whenever any other agent is running. Two problems: (1) it triggers for read-only workers, both when the running agent is read-only and when the new spawn itself is read-only, contrary to the documented intent of isolating only a live parallel writer; and (2) create_isolated_worktree calls git_repo_root, which returns Err in a workspace that is not a git repository, so the spawn fails outright in non-git directories. Gate this so it only applies when the new spawn is actually write-capable (see spawn_request_is_write_capable) and, ideally, only counts running write-capable agents rather than running_count() of all agents. Also make the worktree fallback tolerant of a non-git workspace (skip isolation instead of erroring) so read-only or non-repo parallel work keeps functioning.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔍 Root spawns now get full permissions in every mode

apply_session_spawn_defaults previously delegated edits only when parent_mode == Operate; it now applies to any spawn_depth == 0 spawn and additionally forces allow_shell, permissions = full(), and shell = Full, overriding the parent profile rather than intersecting with it. The PR frames this as intentional inherit-parent behavior with Auto-Review as the remaining floor; confirm no restricted root posture relied on the old narrower baseline.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[claude]

Claude finished @Hmbown's task in 2m 6s —— View job

---

Review: PR #5632 — One worker system; retire Keychain product path

• Gather context (diff since last review, AGENTS.md/CLAUDE.md)
• Check whether prior findings (Plan-mode spawn leak, worktree isolation, stale role-posture tests) were addressed
• Review: correctness bugs
• Review: provider/model/route safety invariant
• Review: reuse/simplification
• Review: test coverage
• Review: security (secrets handling)

This re-review covers the new commit 71f52f1 ("Close the Plan-mode spawn leak without a permission-preset matrix") pushed since the last automated review. It's a direct, well-targeted response to that review's top finding.

Fixed since the last review

1. Plan-mode spawn leak — fixed. apply_session_spawn_defaults (crates/tui/src/tools/subagent/mod.rs:9806) now branches on runtime.parent_mode: a root spawn from AppMode::Plan goes through the new apply_plan_readonly_child_contract, which forces accept_edits = false, allow_shell = false, permissions.write = false, shell = ShellPolicy::None. apply_parent_mode_ceiling additionally clears write_authority/write_roots/exact_files/coordination_contracts on the spawn request itself, so a Plan-mode parent can't mint a write-capable child even via explicit request fields. New regression tests exercise this directly (e.g. crates/tui/src/tools/subagent/tests.rs around lines 137, 341, 482, 557, 592, 661 — file-write attempts from a Plan-mode child now assert expect_err). Good, thorough fix.

2. Stale role-posture tests — resolved. role_posture_blocks_writes_and_shell_for_read_only_roles and read_only_inspection_roles_cannot_write_through_text_filters, which asserted the retired per-role write restriction, are no longer present in crates/tui/src/tools/subagent/tests.rs (confirmed via grep — no matches). They were replaced/rewritten as part of the large test-file rework rather than left to bit-rot.

3. Session-title provenance (Devin's finding) — fixed, and the namer is now wired up. merge_persisted_lifecycle (crates/tui/src/session_manager.rs:1618) now copies title_source alongside title, so autosave can no longer silently revert a User-sourced title back to Truncation. Separately, Engine::maybe_start_session_namer (crates/tui/src/core/engine.rs:3087) is now actually called from the send-message path and spawns session_namer::run_namer_for_session, and apply_generated_session_title (crates/tui/src/tui/ui/event_loop.rs:35) reads the live title_source before applying a generated title — so a user rename is respected. This also closes the "dead scaffolding" observation from the original review.

Still open

4. Worktree isolation still ignores write-capability and non-git workspaces (Devin's finding, unaddressed). crates/tui/src/tools/subagent/mod.rs:9388-9397 is byte-for-byte unchanged from the first commit:

   let parallel = manager.read().await.running_count() > 0;
   if crate::simple_worker::should_isolate_worktree(false, parallel) {
       spawn_request.worktree = Some(SubAgentWorktreeRequest { .. });
   }

   running_count() counts all live agents regardless of write-capability, and write_capable for this spawn isn't computed until line 9537 — after the worktree decision. So a second concurrent spawn always gets forced into a worktree even when both it and the already-running agent are read-only (e.g. two Scouts fanning out). Worse, create_isolated_worktree → git_repo_root (crates/tui/src/tools/subagent/worktree.rs:83,125) returns Err when no git repo is found within MAX_PARENT_LEVELS, so in a non-git workspace this turns a harmless second parallel spawn into a hard ToolError. Since spawn_request_is_write_capable (mod.rs:9800) already exists and is computed just a few dozen lines later in the same function, the fix is straightforward: hoist that computation above the worktree check and gate on it (and ideally track running write-capable agents rather than running_count() of all agents), plus make the worktree path degrade gracefully instead of erroring when git_repo_root fails.

   Fix this →

Minor (unchanged from last review, optional)

• validate_spawn_write_contract's allow_prompt_only_general parameter (crates/tui/src/tools/subagent/mod.rs:12855) is still dead (let _ = allow_prompt_only_general;) at all three call sites. Cosmetic; fine to leave for this PR.

Provider/model/route safety: no issues — this commit doesn't touch route/model selection, and the new is_unbounded_verification check (mod.rs:14424) reuses the existing classify_verification classifier rather than reimplementing anything.

Nothing new introduced by 71f52f1 looks unsafe; the fixes are targeted and match what was asked for. The one remaining item (#4) was flagged by Devin's automated review on this PR and wasn't part of what the follow-up commit addressed — worth a look before merge since it can turn a benign parallel fan-out into a hard failure in non-git workspaces.
(branch)

[Hmbown]

Keychain retirement for account sessions is extracted as focused #5662 (file store, no worker-model change). This PR stays unmerged while tests are red. #5662 is the one that stops the macOS Keychain password dialog on every unsigned codewhale web / TUI start.

[Hmbown]

Closing with the work preserved and tracked — this is a disposition, not a rejection.

• The urgent slice was extracted and landed long ago: fix(secrets): stop account sessions from popping Keychain #5662 "fix(secrets): stop account sessions from popping Keychain" merged 2026-08-27 as 6e8745e122.
• The remaining scope (full KeyringStore retirement, single-worker spawn model, /mcp connect chaining) is now tracked in One worker system: retire the Keychain product path + single-worker spawn model (remaining scope from PR #5632) #5718 with the Devin review findings carried over (the forced-worktree BUG at tools/subagent/mod.rs:9387-9396 and the root-spawn full-permissions question both need answers in any revival).
• As it stands this branch is 223 commits behind main, 6 checks red (Lint, Safety gate, Tests ×3, link), conflicting across 17 files that also moved on main — the owner note "stays unmerged while tests are red" has held since 2026-08-27.

The branch is preserved for the rebase-and-repair effort; reopen or reference it from #5718 when that work starts.