feat(providers): catalog-driven account keys, first-class codewhale route, sandbox/ShannonNet batch (rebase) - #5966
Conversation
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_92993abb-dfcf-4967-8706-4852b7e49b8f) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @Hmbown's task in 1m 16s —— View job Claude is reviewing this PR
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Live catalog never drives routing
A successful refresh stores rows only in the provider lake. RouteResolver::new still uses bundled rows, ignoring live protocols and defaults. Requests can use the wrong endpoint, while auto can select an unavailable bootstrap model.
(Refers to this code)
Prompt for agents
The Codewhale /models response is parsed with authoritative per-model endpoint keys and default flags, but spawn_active_provider_catalog_refresh only publishes those rows to provider_lake. Runtime route resolution in crates/tui/src/route_runtime.rs and the model rebinding paths in crates/tui/src/client.rs continue constructing RouteResolver::new(), which contains bundled offerings only. Route resolution must consume a coherent live-over-bundled authority snapshot for Codewhale. Preserve fallback behavior only when no usable live catalog exists, and ensure both explicit models and auto/default selection use the live protocol metadata.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🟡 Removed account models stay cached
Each refresh calls merge_live_offerings, which preserves rows absent from the new account listing. Disconnected models remain selectable until the process-wide snapshot resets.
(Refers to this code)
Prompt for agents
The Codewhale account catalog is an authoritative full listing, not an incremental delta. Publishing it through merge_live_offerings preserves models omitted by later successful responses. Replace the Codewhale provider partition atomically on successful refresh while preserving other providers and Models.dev data. Failed refreshes must continue retaining the previous successful partition.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🔍 Feature batch exceeds one review boundary
Repository policy asks PRs to cover one behavior boundary. This branch combines provider credentials, routing, sandboxing, delegation, UI, and test-lock fixes.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if let Some(child) = child_backend { | ||
| let (summary, succeeded) = match &result { | ||
| Ok(outcome) => ( | ||
| outcome.result.as_deref(), | ||
| matches!(outcome.status, SubAgentStatus::Completed), | ||
| ), | ||
| Err(_) => (None, false), | ||
| }; | ||
| // Token accounting stays in Codewhale's own usage record; the join | ||
| // carries the child's conclusion and outcome. | ||
| if let Err(err) = child.child_joined(summary, 0, succeeded).await { | ||
| tracing::warn!(target: "subagent", ?err, agent_id, "sub-agent join was not recorded"); | ||
| } |
There was a problem hiding this comment.
🔴 Cancelled children leak delegated worlds
A timeout or Stop aborts run_subagent before child_joined runs. ShannonBackend::drop skips closing child worlds, so cancellation leaves their resources alive.
Prompt for agents
Delegated Shannon children are joined only in the normal epilogue of run_subagent. The enclosing timeout_at and finish_terminal_result's task abort can cancel this future before that epilogue, while ShannonBackend::drop intentionally closes only session worlds. Introduce cancellation-safe ownership for a spawned child so every terminal path attempts join or explicit child-world retirement. Cover normal completion, timeout, Stop, coordination interruption, and failures during context compilation or model execution.
Was this helpful? React with 👍 or 👎 to provide feedback.
| async fn run_json(binary: &Path, home: &Path, args: &[&str]) -> Result<Value> { | ||
| let output = tokio::process::Command::new(binary) | ||
| .args(cli_args(home, args)) | ||
| .stdin(Stdio::null()) | ||
| .output() | ||
| .await | ||
| .with_context(|| format!("failed to run {}", binary.display()))?; | ||
| parse_cli_output(output.status, &output.stdout, &output.stderr) |
There was a problem hiding this comment.
🔴 Sandbox calls can hang indefinitely
A stalled shannon process leaves run_json awaiting output forever. The 30-second value reaches only the worker payload, so transport hangs never expire.
Prompt for agents
All asynchronous Shannon CLI calls await tokio::process::Command::output without a host-side deadline. The timeout_secs field only becomes timeout_ms inside the remote invocation payload and cannot bound hangs before or after worker execution. Apply a real timeout to CLI processes, terminate and reap timed-out children, and use suitable bounded deadlines for invoke, sync, join, context, and inspection operations. Ensure cancellation does not leave local shannon subprocesses running.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// ShannonNet: a signed capability invocation on a worker that may live | ||
| /// on another tailnet node (see `shannon.rs`). | ||
| Shannon, |
There was a problem hiding this comment.
🟡 Shannon commands report wrong sandbox
Adding SandboxKind::Shannon leaves ShellResult hardcoded to opensandbox. Structured results therefore misreport every Shannon execution.
Prompt for agents
The external shell path now has backend.kind(), but only the metadata field uses it. Update the ShellResult sandbox_type assignment in crates/tui/src/tools/shell.rs to derive from the backend kind as well, then verify structured tool consumers receive shannon for Shannon executions and opensandbox for OpenSandbox.
Was this helpful? React with 👍 or 👎 to provide feedback.
| - `codewhale` provider: account-backed model access over the provider keys a | ||
| customer connected to their Codewhale account. One base URL | ||
| (`https://api.codewhale.net/v1`, overridable with `CODEWHALE_API_BASE`; | ||
| HTTPS required except on loopback), one `cwc_key_…` account API key with the | ||
| `models:infer` scope (`CODEWHALE_API_KEY`), and a per-model wire chosen from | ||
| the account's authenticated `GET /v1/models`: ids are `provider/model` and | ||
| each row states `chat-completions` (`/v1/chat/completions`) or | ||
| `anthropic-messages` (`/v1/messages`). Both protocols authenticate with | ||
| `Authorization: Bearer`, never `x-api-key`. The key is read from | ||
| `CODEWHALE_API_KEY`, the `codewhale` secret-store slot, or | ||
| `[providers.codewhale] api_key` / `api_key_env`, and a missing key fails | ||
| before any request instead of dispatching unauthenticated. If the catalog | ||
| cannot be fetched the route falls back to three bootstrap ids and says so. | ||
| - `codewhale account api-keys create --scope` now accepts `models:infer` | ||
| alongside `account:read` and `agent:run`, and an omitted `--scope` sends all | ||
| three explicitly. `--use` saves the new secret as this machine's local | ||
| `codewhale` provider credential in the same secret store `codewhale auth` | ||
| uses; nothing is uploaded. | ||
| - `sandbox_backend = "shannon"`: shell commands run as signed ShannonNet | ||
| capability invocations (`cap://sandbox/exec`) on a worker that may live on | ||
| another tailnet node. Codewhale opens a Task World per session for its | ||
| durable `codewhale` Agent and every command leaves a receipt in | ||
| `shannon trace`. New keys `sandbox_shannon_home` and | ||
| `sandbox_shannon_capability`; tool metadata now reports the actual | ||
| external backend kind instead of always `opensandbox`. | ||
| - `/shannon [world|trace|children]` inspects the session's ShannonNet | ||
| World: agent, projected capabilities, children, and receipts. | ||
| - ShannonNet sub-agents get compiled context: the session's native-memory | ||
| hits are imported with provenance and the child's projected World decides | ||
| what it sees (confidential notes never cross); the session World is | ||
| checkpointed and closed when the backend drops. | ||
| - Sub-agents under delegated authority: with the ShannonNet backend the | ||
| `agent` tool spawns a child identity with a World projected from the | ||
| session World, the child's shell commands are signed as that child, and a | ||
| join receipt is recorded when it finishes. `SandboxBackend::for_child` / | ||
| `child_joined` default to sharing the parent backend for other backends. | ||
| - Workspace sync for the ShannonNet backend (`sandbox_shannon_sync`, default | ||
| on): the session's non-ignored files are shipped into the worker's | ||
| per-World session container before each command — full tree first, then | ||
| only changes and deletions — so remote builds and tests run on the files | ||
| just edited locally and their outputs persist across commands. |
There was a problem hiding this comment.
🔍 Remove the branch changelog entry
Repository policy reserves changelog edits for merge-time commits on main. This branch adds an Unreleased entry and creates avoidable conflicts.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "ModelsSourceFallback": "Models inclosos o configurats; disponibilitat no verificada.", | ||
| "CmdModelDbDescription": "Explora la base de dades de models inclosa", | ||
| "CmdNetworkDescription": "Gestiona les regles de xarxa de permís i denegació", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
There was a problem hiding this comment.
🔍 Translate the Shannon command description
Locale policy forbids copying English into complete packs. Every non-English CmdShannonDescription currently contains the English reference text.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let sandbox_backend = | ||
| crate::sandbox::backend::create_backend(api_config, &config.workspace) | ||
| .unwrap_or_else(|e| { | ||
| tracing::warn!("Failed to create sandbox backend: {e}"); | ||
| None | ||
| }) |
There was a problem hiding this comment.
🟥 Sandbox failure enables local execution
When ShannonNet setup fails, the engine silently removes the external boundary. Later shell commands run on the host with local access.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let walker = ignore::WalkBuilder::new(root) | ||
| .hidden(false) | ||
| .git_ignore(true) | ||
| .git_global(true) | ||
| .git_exclude(true) | ||
| .require_git(false) | ||
| .filter_entry(|entry| entry.file_name() != ".git") | ||
| .build(); |
There was a problem hiding this comment.
🟨 Repository rules control remote uploads
Workspace sync lets checkout-controlled ignore files decide what leaves the machine. A crafted repository can include nearby credentials in remote archives.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🟡 Changes recommended
A confirmed cross-platform locking bug exists in RuntimeProcessOwnerLock::is_contention (Windows error codes treated as Unix contention), and several non-English locales ship an untranslated English command description.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR rebases and lands a feature batch that (1) introduces a first-class codewhale provider route (account-backed, model-aware protocol selection, always Bearer auth) and corresponding docs/web “facts” updates, (2) adds a new ShannonNet external sandbox backend (sandbox_backend = "shannon") with delegated sub-agent authority and a new /shannon inspection command, and (3) includes targeted tui changes intended to reduce load-dependent test flakes (env/live-snapshot lock ordering + same-process lock reopen handling).
Changes:
- Add
codewhaleas a first-class provider across config/runtime/CLI/docs and surface it in generated web facts. - Add ShannonNet sandbox backend implementation +
/shannonsession inspection command; make tool metadata report the actual backend kind. - Address flake sources in
codewhale-tuiby tightening test/global snapshot serialization and improving file-lock reacquisition behavior.
File summaries
| File | Description |
|---|---|
| web/scripts/facts-lib.mjs | Add codewhale to the provider label/env map used by the web facts generator. |
| web/lib/facts.generated.ts | Regenerate facts output to include codewhale provider and updated generation timestamp. |
| web/lib/facts-drift.ts | Keep drift-derived provider listing in sync by adding codewhale. |
| docs/SANDBOX.md | Document sandbox_backend = "shannon" and related env/config knobs. |
| docs/PROVIDERS.md | Add codewhale to canonical provider IDs and provider registry documentation. |
| docs/CONFIGURATION.md | Document codewhale provider selection and its base/key semantics. |
| docs/ARCHITECTURE.md | Add the ShannonNet sandbox backend module to architecture docs. |
| crates/tui/src/tui/ui/session_state.rs | Mirror saved API key support extended to ApiProvider::Codewhale. |
| crates/tui/src/tools/subagent/tests.rs | Serialize live-snapshot mutations in subagent catalog tests. |
| crates/tui/src/tools/subagent/mod.rs | Run sub-agents under delegated sandbox authority when supported; add bounded memory notes path. |
| crates/tui/src/tools/shell/tests.rs | Update sandbox backend test doubles to implement SandboxBackend::kind(). |
| crates/tui/src/tools/shell.rs | Report real sandbox backend kind in tool metadata instead of hardcoding opensandbox. |
| crates/tui/src/sandbox/shannon.rs | New ShannonNet sandbox backend: signed capability invocations, workspace sync, delegated children, and context compilation support. |
| crates/tui/src/sandbox/opensandbox.rs | Implement SandboxBackend::kind() for OpenSandbox backend. |
| crates/tui/src/sandbox/mod.rs | Export the new shannon sandbox module. |
| crates/tui/src/sandbox/backend.rs | Extend sandbox abstraction with kind() + child delegation hooks; add shannon backend construction (workspace-aware). |
| crates/tui/src/runtime_threads.rs | Reduce same-process lock drop/reopen flakes by retrying on contention and unlocking explicitly on drop. |
| crates/tui/src/provider_lake.rs | Fix test deadlock risk by enforcing env-barrier-before-live-snapshot lock ordering. |
| crates/tui/src/models_dev_live.rs | Ensure tests that mutate live snapshot take the snapshot lock. |
| crates/tui/src/localization.rs | Register the new CmdShannonDescription message id. |
| crates/tui/src/lib.rs | Pass workspace into external sandbox backend construction. |
| crates/tui/src/core/engine.rs | Pass workspace into external sandbox backend construction during engine init. |
| crates/tui/src/config/tests.rs | Add regression tests for Codewhale route key resolution/boundary behavior. |
| crates/tui/src/config/models.rs | Add default Codewhale model/base URL constants for tui config layer. |
| crates/tui/src/config.rs | Add ApiProvider::Codewhale; add ShannonNet backend config keys; enforce Codewhale key behavior and env-base validation. |
| crates/tui/src/config_persistence.rs | Allow persistence of Codewhale provider base URL table key. |
| crates/tui/src/commands/mod.rs | Export native memory store helper and register /shannon command group entry. |
| crates/tui/src/commands/groups/utility/shannon.rs | New /shannon command to inspect ShannonNet session world/children/trace via pointer file. |
| crates/tui/src/commands/groups/utility/mod.rs | Wire /shannon into the utility commands group. |
| crates/tui/src/commands/contract.rs | Add cmd_shannon_description mapping; expose native_store_from_memory_path for reuse. |
| crates/tui/src/client.rs | Ensure Codewhale provider uses Bearer auth on Messages too; parse Codewhale /models protocol metadata and add tests. |
| crates/tui/src/acp_server.rs | Pass workspace into external sandbox backend construction for ACP registry. |
| crates/tui/locales/en.json | Add CmdShannonDescription string for the new command. |
| crates/tui/locales/ca.json | Add CmdShannonDescription entry. |
| crates/tui/locales/de.json | Add CmdShannonDescription entry. |
| crates/tui/locales/es-419.json | Add CmdShannonDescription entry. |
| crates/tui/locales/fr.json | Add CmdShannonDescription entry. |
| crates/tui/locales/hi.json | Add CmdShannonDescription entry. |
| crates/tui/locales/id.json | Add CmdShannonDescription entry. |
| crates/tui/locales/ja.json | Add CmdShannonDescription entry. |
| crates/tui/locales/ko.json | Add CmdShannonDescription entry. |
| crates/tui/locales/pt-BR.json | Add CmdShannonDescription entry. |
| crates/tui/locales/ru.json | Add CmdShannonDescription entry. |
| crates/tui/locales/uk.json | Add CmdShannonDescription entry. |
| crates/tui/locales/vi.json | Add CmdShannonDescription entry. |
| crates/tui/locales/zh-Hans.json | Add CmdShannonDescription entry. |
| crates/tui/locales/zh-Hant.json | Add CmdShannonDescription entry. |
| crates/secrets/src/lib.rs | Teach secrets env resolution about codewhale → CODEWHALE_API_KEY. |
| crates/secrets/src/account.rs | Extend account key state model with non-secret state and label metadata. |
| crates/config/src/tests.rs | Update provider registry cardinality and wire expectations for new provider kind. |
| crates/config/src/route/tests.rs | Treat Codewhale route as model-aware and validate Codewhale API base rules. |
| crates/config/src/route/resolver.rs | Allow Codewhale route to fail-open on unknown model ids (account catalog authority) while still picking protocol by namespace. |
| crates/config/src/route/providers-export.golden.json | Add Codewhale provider to the exported provider catalog golden file. |
| crates/config/src/route/offering.rs | Add Codewhale fallback model set and endpoint-key inference helper; include in bundled offerings. |
| crates/config/src/route/mod.rs | Re-export Codewhale route fallback models and endpoint-key helper. |
| crates/config/src/route/golden_route_ids.txt | Add codewhale to the golden route id list. |
| crates/config/src/provider.rs | Add Codewhale provider implementation + CODEWHALE_API_BASE validation helpers and credential help. |
| crates/config/src/provider_kind.rs | Add ProviderKind::Codewhale and include in ProviderKind::ALL. |
| crates/config/src/provider_defaults.rs | Add default Codewhale base URL + model constants. |
| crates/config/src/lib.rs | Plumb Codewhale provider into TOML provider set, defaults, official-base checks, and env overrides. |
| crates/config/src/device_code.rs | Expose minimal URL parsing helpers for reuse in Codewhale base validation. |
| crates/cli/src/lib.rs | Update tests for expanded provider registry and ProviderKind::ALL size. |
| crates/cli/src/cloud/tests.rs | Add cloud keys tests for catalog-driven provider ids and Codewhale --use API key behavior. |
| crates/cli/src/cloud/machine/tests.rs | Update scope normalization behavior: omitted scopes now send all explicitly (incl. models:infer). |
| crates/cli/src/cloud/machine.rs | Add models:infer scope, explicit-scope behavior, and --use to save new key locally as Codewhale provider credential. |
| crates/cli/src/cloud.rs | Replace hardcoded cloud provider enum with catalog-driven provider ids + local validation/mapping. |
| config.example.toml | Document ShannonNet sandbox backend config keys and env overrides. |
| CHANGELOG.md | Document new codewhale provider route, ShannonNet backend, /shannon, and cloud keys catalog changes. |
Review details
- Files reviewed: 68/68 changed files
- Comments generated: 15
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| fn is_contention(error: &std::io::Error) -> bool { | ||
| error.kind() == std::io::ErrorKind::WouldBlock | ||
| || matches!(error.raw_os_error(), Some(32 | 33)) | ||
| } |
| "ModelsSourceFallback": "Models inclosos o configurats; disponibilitat no verificada.", | ||
| "CmdModelDbDescription": "Explora la base de dades de models inclosa", | ||
| "CmdNetworkDescription": "Gestiona les regles de xarxa de permís i denegació", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
| "ModelsSourceFallback": "Mitgelieferte oder konfigurierte Modelle; Verfügbarkeit nicht geprüft.", | ||
| "CmdModelDbDescription": "Mitgelieferte Modelldatenbank durchsuchen", | ||
| "CmdNetworkDescription": "Netzwerk-Allow- und Deny-Regeln verwalten", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
| "ModelsSourceFallback": "Modelos incluidos o configurados; disponibilidad sin verificar.", | ||
| "CmdModelDbDescription": "Explorar la base de datos de modelos integrada", | ||
| "CmdNetworkDescription": "Gestionar reglas de red permitidas y bloqueadas", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
| "ModelsSourceFallback": "Modèles fournis ou configurés ; disponibilité non vérifiée.", | ||
| "CmdModelDbDescription": "Parcourir la base de modèles intégrée", | ||
| "CmdNetworkDescription": "Gérer les règles réseau d'autorisation et de refus", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
| "ModelsSourceFallback": "Встроенные или настроенные модели; доступность не проверена.", | ||
| "CmdModelDbDescription": "Просмотр встроенной базы моделей", | ||
| "CmdNetworkDescription": "Управление правилами разрешения и запрета сети", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
| "ModelsSourceFallback": "Вбудовані або налаштовані моделі; доступність не перевірено.", | ||
| "CmdModelDbDescription": "Переглянути вбудовану базу моделей", | ||
| "CmdNetworkDescription": "Керувати мережевими правилами дозволу та заборони", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
| "ModelsSourceFallback": "Mô hình đi kèm hoặc đã cấu hình; chưa xác minh tính khả dụng.", | ||
| "CmdModelDbDescription": "Duyệt cơ sở dữ liệu mô hình tích hợp", | ||
| "CmdNetworkDescription": "Quản lý các quy tắc cho phép và từ chối mạng", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
| "ModelsSourceFallback": "内置或已配置的模型；尚未验证可用性。", | ||
| "CmdModelDbDescription": "参考：浏览内置的模型数据库", | ||
| "CmdNetworkDescription": "管理网络允许和拒绝规则", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
| "ModelsCodexHint": "ChatGPT 模型透過 Codex 載入。查詢時間並不代表供應商已重新整理模型目錄。", | ||
| "ModelsSourceFallback": "內建或已設定的模型；尚未驗證可用性。", | ||
| "CmdNetworkDescription": "管理網路允許和拒絕規則", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
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_fd36bbf1-6aad-4733-a579-566586663664) |
|
Claude finished @Hmbown's task in 1m 53s —— View job Claude is reviewing this PR
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84bff00f26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "ModelsSourceFallback": "Models inclosos o configurats; disponibilitat no verificada.", | ||
| "CmdModelDbDescription": "Explora la base de dades de models inclosa", | ||
| "CmdNetworkDescription": "Gestiona les regles de xarxa de permís i denegació", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
There was a problem hiding this comment.
Every non-English locale copies the English CmdShannonDescription verbatim, so users of Catalan and the other complete packs see untranslated command-palette prose. Provide an actual translation for each complete locale rather than satisfying key parity with the English text.
AGENTS.md reference: crates/tui/locales/AGENTS.md:L7-L11
Useful? React with 👍 / 👎.
| if builder.is_some() && raw + len > chunk_bytes { | ||
| archives.push(finish_archive(builder.take().unwrap())?); | ||
| raw = 0; | ||
| } |
There was a problem hiding this comment.
When a changed file itself exceeds chunk_bytes, builder is initially None, so this condition does not split or reject it and the whole file is placed in one archive. For an incompressible file of roughly 13–16 MiB, base64 expansion alone pushes the JSON request over the documented 16 MiB worker limit; because sync is mandatory before execution, every Shannon shell command then fails even though files up to 16 MiB are explicitly admitted by WorkspaceSync::list.
Useful? React with 👍 / 👎.
| let catalog = client.provider_catalog()?; | ||
| let _ = catalog_row(&catalog, &provider)?; | ||
| client.remove_key(&provider)?; |
There was a problem hiding this comment.
If a provider is retired, temporarily omitted, or filtered as malformed from /api/model-providers, this catalog check rejects account keys remove <id> before issuing the DELETE, even when /api/me still reports a stored credential for that id. That leaves users unable to delete an orphaned provider secret; removal should require only the local id validation and let the authenticated deletion endpoint determine whether the key exists.
Useful? React with 👍 / 👎.
| // Token accounting stays in Codewhale's own usage record; the join | ||
| // carries the child's conclusion and outcome. | ||
| if let Err(err) = child.child_joined(summary, 0, succeeded).await { |
There was a problem hiding this comment.
Every delegated child is joined with tokens = 0, even though run_subagent_in records each response's token count in the manager's AgentRunUsage. Consequently all ShannonNet join receipts claim zero usage for nonempty sub-agent runs, making the signed audit record inaccurate; read the completed worker's recorded total and pass that value to child_joined.
Useful? React with 👍 / 👎.
| } else { | ||
| "invoke" | ||
| }; | ||
| run(&[ |
There was a problem hiding this comment.
If world attach fails after world create succeeded—for example because the capability is unavailable—new returns immediately without closing the newly created World because no ShannonBackend exists for Drop to clean up. Engine startup then falls back to local execution while leaving an orphan World behind, and repeated failed starts accumulate remote state; explicitly close the created World on this error path.
Useful? React with 👍 / 👎.
| if create.use_locally { | ||
| save_key_as_local_codewhale_credential(provider_secrets, &created.secret, out)?; | ||
| } |
There was a problem hiding this comment.
--use
When the user combines --use with an explicit scope set that omits models:infer (for example --scope account:read --use), this saves the newly created key over the local codewhale credential even though that key cannot call any model endpoint. The option promises immediate model use and can replace a working saved key with an unusable one, so reject this combination or ensure the created key includes models:infer before persisting it.
Useful? React with 👍 / 👎.
| Some("anthropic-messages") => "messages", | ||
| Some("chat-completions") => "chat", | ||
| _ => codewhale_config::route::codewhale_endpoint_key_for_model(&id), |
There was a problem hiding this comment.
If the live catalog returns a nonempty protocol this client does not understand, such as a future responses protocol, the wildcard arm publishes the model as Chat Completions unless its id starts with anthropic/. Selecting that advertised row then calls the wrong endpoint rather than reporting that the CLI needs an upgrade; distinguish a missing legacy protocol from an explicitly unknown value and drop or reject the latter.
Useful? React with 👍 / 👎.
| #[cfg(not(unix))] | ||
| { | ||
| // Without a shell to sequence them, run only the first step; the | ||
| // worker reaper and a later `shannon world close` cover the rest. | ||
| if let Some(args) = steps.first() { | ||
| let _ = std::process::Command::new(binary) | ||
| .args(args) |
There was a problem hiding this comment.
With the default sandbox_shannon_sync = true, teardown queues destroy followed by world close, but this non-Unix branch executes only the first step. Every successful Windows Shannon session therefore tears down its worker container while leaving its Task World open, contrary to the documented session lifecycle; execute both commands sequentially without relying on a Unix shell.
Useful? React with 👍 / 👎.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ); | ||
| result.unwrap(); | ||
| // The secret is still printed exactly once, and the local save is stated. | ||
| assert_eq!(output.matches(MACHINE_TOKEN).count(), 1, "{output}"); |
| assert_eq!(output.matches(MACHINE_TOKEN).count(), 1, "{output}"); | ||
| assert!( | ||
| output.contains("local `codewhale` provider credential"), | ||
| "{output}" |
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
PR #5966 adds a catalog-driven account-key CLI, a first-class Codewhale model provider with account-catalog model routing, and a ShannonNet sandbox backend. The branch is broad but generally well-tested. Review found a few lifecycle, validation, and localization gaps around the sandbox env overrides, Codewhale base-URL trust, Shannon session cleanup, and locale coverage.
Findings
- [WARNING] Empty CODEWHALE_SANDBOX_SHANNON_HOME is not ignored (
crates/tui/src/config.rs:9589)
The env override for CODEWHALE_SANDBOX_SHANNON_HOME is applied unconditionally. An empty value sets config.sandbox_shannon_home = Some(""), which wins over the SHANNON_HOME env fallback and the default ~/.shannon. The Shannon backend then runs the CLI with --home "" and likely fails to create or inspect the agent. The capability and sync env overrides are handled more defensively; HOME should ignore blank values too. - [WARNING] Provider config base_url can bypass Codewhale HTTPS/loopback validation (
crates/tui/src/config.rs:8332)
provider_env_base_url_override validates only CODEWHALE_API_BASE, but [providers.codewhale] base_url is applied directly by the config table. A non-loopback http:// base_url combined with a stored or configured Codewhale API key would send Authorization: Bearer cwc_key_... over cleartext, contrary to the stated route contract. The direct provider base_url path should receive the same codewhale_api_base validation, or reject insecure URLs when a key is present. - [WARNING] ShannonBackend::new can leak a Task World on partial initialization failure (
crates/tui/src/sandbox/shannon.rs:128)
Afterworld createsucceeds, the new backend needs to extract the world id and attach the sandbox capability. If either step fails, the function returns Err before a ShannonBackend value exists, so Drop never runs and the freshly created Task World is never closed. The caller logs a warning and falls back to local execution, leaving a stale World/session in ShannonNet. - [INFO] /shannon command description is untranslated in non-English locales (
crates/tui/locales/de.json:495)
All non-English locale files add the English string for CmdShannonDescription. This makes /shannon help appear in English for every localized build. Existing command descriptions are translated, so this should either be translated or omitted from non-English locale files until translations are ready.
Suggestions
-
crates/tui/src/config.rs:9589— Ignore an empty CODEWHALE_SANDBOX_SHANNON_HOME value so it does not override $SHANNON_HOME or the default ~/.shannon.if let Ok(value) = std::env::var("CODEWHALE_SANDBOX_SHANNON_HOME") { if !value.trim().is_empty() { config.sandbox_shannon_home = Some(value); } }
Assessment
This is a large feature batch with substantial test coverage. The main risks are edge-case validation around the new sandbox configuration and a lifecycle leak in the Shannon backend construction path. The provider routing work is well covered by tests, but the direct config base_url trust boundary should be tightened before merge.
Advisory review by Codewhale (codewhale review --pr 5966 --post, head 240b14b46e4fed920a2130cf05fd3478668b0e54). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| { | ||
| config.sandbox_api_key = Some(value); | ||
| } | ||
| if let Ok(value) = std::env::var("CODEWHALE_SANDBOX_SHANNON_HOME") { |
There was a problem hiding this comment.
[WARNING] Empty CODEWHALE_SANDBOX_SHANNON_HOME is not ignored
The env override for CODEWHALE_SANDBOX_SHANNON_HOME is applied unconditionally. An empty value sets config.sandbox_shannon_home = Some(""), which wins over the SHANNON_HOME env fallback and the default ~/.shannon. The Shannon backend then runs the CLI with --home "" and likely fails to create or inspect the agent. The capability and sync env overrides are handled more defensively; HOME should ignore blank values too.
| @@ -8261,6 +8332,16 @@ fn provider_env_base_url_override(provider: ApiProvider) -> Option<String> { | |||
| | ApiProvider::LongCat | |||
There was a problem hiding this comment.
[WARNING] Provider config base_url can bypass Codewhale HTTPS/loopback validation
provider_env_base_url_override validates only CODEWHALE_API_BASE, but [providers.codewhale] base_url is applied directly by the config table. A non-loopback http:// base_url combined with a stored or configured Codewhale API key would send Authorization: Bearer cwc_key_... over cleartext, contrary to the stated route contract. The direct provider base_url path should receive the same codewhale_api_base validation, or reject insecure URLs when a key is present.
| AGENT_NAME, | ||
| "--name", | ||
| &name, | ||
| "--objective", |
There was a problem hiding this comment.
[WARNING] ShannonBackend::new can leak a Task World on partial initialization failure
After world create succeeds, the new backend needs to extract the world id and attach the sandbox capability. If either step fails, the function returns Err before a ShannonBackend value exists, so Drop never runs and the freshly created Task World is never closed. The caller logs a warning and falls back to local execution, leaving a stale World/session in ShannonNet.
| "ModelsSourceFallback": "Mitgelieferte oder konfigurierte Modelle; Verfügbarkeit nicht geprüft.", | ||
| "CmdModelDbDescription": "Mitgelieferte Modelldatenbank durchsuchen", | ||
| "CmdNetworkDescription": "Netzwerk-Allow- und Deny-Regeln verwalten", | ||
| "CmdShannonDescription": "Inspect this session's ShannonNet World, capabilities, children, and receipts", |
There was a problem hiding this comment.
[INFO] /shannon command description is untranslated in non-English locales
All non-English locale files add the English string for CmdShannonDescription. This makes /shannon help appear in English for every localized build. Existing command descriptions are translated, so this should either be translated or omitted from non-English locale files until translations are ready.
| if let Ok(value) = std::env::var("CODEWHALE_SANDBOX_SHANNON_HOME") { | ||
| config.sandbox_shannon_home = Some(value); | ||
| } |
There was a problem hiding this comment.
Ignore an empty CODEWHALE_SANDBOX_SHANNON_HOME value so it does not override $SHANNON_HOME or the default ~/.shannon.
| if let Ok(value) = std::env::var("CODEWHALE_SANDBOX_SHANNON_HOME") { | |
| config.sandbox_shannon_home = Some(value); | |
| } | |
| if let Ok(value) = std::env::var("CODEWHALE_SANDBOX_SHANNON_HOME") { | |
| if !value.trim().is_empty() { | |
| config.sandbox_shannon_home = Some(value); | |
| } | |
| } |
…ocations `sandbox_backend = "shannon"` routes every exec_shell command through ShannonNet: at session start the backend resolves this installation's durable `codewhale` Agent (created once; `agent inspect` first, because `agent create` on a taken name mints a sibling), creates a Task World named after the workspace, and attaches `cap://sandbox/exec`. Each command is one `shannon cap invoke` whose signed receipt carries stdout, stderr, and the exit code; the worker that runs it is whichever admitted provider the ShannonNet router selects, possibly on another tailnet node, and Codewhale never learns its address. `shannon trace` lists every command with its provider. The protocol is spoken through the `shannon` CLI (`--json`), not reimplemented, so there is one signer and one verifier. No second turn loop: the Engine stays the only cognition loop and this is one exec at a time behind the existing SandboxBackend trait (the OpenSandbox path is untouched). - crates/tui/src/sandbox/shannon.rs: backend + tests with a stand-in `shannon` script that records argv (session-start sequence, exec mapping, existing agent not recreated, provider error surfaced, missing binary is a construction error so the engine falls back to local execution). - SandboxKind::Shannon; `SandboxBackend::kind()` so shell tool metadata reports the actual backend instead of a hardcoded "opensandbox". - create_backend takes the workspace (all three call sites had one). - Config: sandbox_shannon_home, sandbox_shannon_capability, with CODEWHALE_SANDBOX_SHANNON_HOME / _CAPABILITY overrides; binary from $SHANNON or PATH. - Docs: config.example.toml, docs/SANDBOX.md, docs/ARCHITECTURE.md, CHANGELOG. Counterpart in ShannonNet 82ea83c (`agent inspect`, docker kind exit-status semantics, `make demo-codewhale` contract receipt). Gates: cargo fmt --check ok; cargo clippy -p codewhale-tui --lib --tests -D warnings ok; cargo test -p codewhale-tui --lib (RUST_MIN_STACK=8388608 as CI): test result: ok. 11685 passed; 0 failed; 12 ignored. Targeted: sandbox::shannon 6 passed; 0 failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jw7EuFvtYMHHK5Cu6JLEjc (cherry picked from commit a609fb438ff17d8057085d5937691cbee363fa3a)
…ommand
`sandbox_shannon_sync` (default on): the Shannon backend walks the workspace
with git's ignore rules (.gitignore, local/global excludes, .git itself;
symlinks and files over 16 MiB skipped), stamps files by length and mtime,
and before each command ships only what the worker's per-World session has
not seen — the whole tree once, then edits and deletions — as gzip tar
chunks of at most 6 MiB raw through `shannon cap invoke --action sync`.
Commands then run in that writable /work with the files the Engine just
edited, and their outputs persist across commands, so remote builds and test
suites work. A failed sync fails the command rather than running it on a
stale tree; the backend sends `destroy` (detached) when it drops. The World
attachment grants invoke,sync,destroy only when sync is on.
Tests (stand-in `shannon` CLI recording argv): first command ships exactly
the non-ignored tree, an unchanged tree ships nothing, an edit plus a
deletion ship exactly those, every sync precedes its command; chunking at
the raw-byte limit. Docs: config.example.toml, docs/SANDBOX.md, CHANGELOG.
Counterpart in ShannonNet abf72c1 (ADR-0012: per-World session containers,
path-safe extraction, `make demo-codewhale` runs that repository's Go tests
inside a synced session).
Gates: cargo fmt --check ok; cargo clippy -p codewhale-tui --lib --tests
-D warnings ok; sandbox::shannon 8 passed / 0 failed. Full suite
(RUST_MIN_STACK=8388608 as CI): 11686 passed; 1 failed —
remote_control::tests::separate_predispatch_crashes_on_one_run_get_distinct_recovery_turn_ids
("saved session still owns an unfinished account turn"), untouched by this
diff, passing in isolation and with its module (remote_control::tests: 77
passed / 0 failed); the run overlapped a Docker Go build on this host. An
earlier full run of the same tree shape was 11685 passed / 0 failed.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jw7EuFvtYMHHK5Cu6JLEjc
(cherry picked from commit 31830b635d944c493aac6159d10b89949014e999)
…join) The `agent` tool now maps to Shannon spawn/join when the session's sandbox backend can delegate. `SandboxBackend::for_child(role, objective)` returns a child backend (default `None`: OpenSandbox and friends keep sharing the parent's backend), and `child_joined(summary, tokens, succeeded)` records the join (default no-op). ShannonBackend::for_child runs `shannon agent spawn --parent <agent> --world <session world> --role <role> --objective <prompt> --world-project cap://sandbox/exec`: a child identity certified by the session's codewhale Agent with a World projected from the session World (only the sandbox capability, delegation depth attenuated). The child backend signs every command as that child in that World and ships the tree into the child's own session container. child_joined runs `shannon agent join --child <id> --tokens N --confidence 1|0 --conclusion <summary>`, which records a JoinReceipt on the task and destroys the child's World. run_subagent is now a thin wrapper around run_subagent_in: it derives the child backend, clones the runtime with that backend in its ToolContext, runs the sub-agent, and joins on completion. A delegation that cannot be established fails the spawn rather than silently running the child with the parent's authority. Token usage stays in Codewhale's own usage record; the join carries the conclusion and outcome. Tests (stand-in `shannon` CLI): the child's exec signs as `--agent child-1 --world world-child` and never as codewhale; the spawn names the session World and projection; the join carries tokens, confidence, and conclusion; role names are sanitized. Counterpart in ShannonNet 82a7c39 (`agent spawn --world`, `agent join`, demo step 6b: child sees only the projected capability, is denied the unprojected one, join retires its World). Gates: cargo fmt --check ok; cargo clippy -p codewhale-tui --lib --tests -D warnings ok; cargo test -p codewhale-tui --lib (RUST_MIN_STACK=8388608 as CI): test result: ok. 11688 passed; 0 failed; 12 ignored. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jw7EuFvtYMHHK5Cu6JLEjc (cherry picked from commit 43683c2901f70f1b380668812b29196139d5421b)
… inspection - SandboxBackend::child_context(task, notes) (default None): the Shannon backend imports the session's native-memory hits for the task into the codewhale Agent's memory graph (`shannon memory add`, idempotent, with codewhale-memory:<source>:<lines> provenance) and appends `shannon context compile` output for the child in its projected World to the child's prompt — the items its World may see, with provenance and the compiler's information-flow notes (confidential notes never cross). The spawn projection is now `cap://sandbox/exec,memory`. run_subagent gathers the notes from the native store when `[memory]` is enabled. - Session end: dropping the session backend runs the worker session `destroy` and then `shannon world close` (checkpoint, then destroy) in one detached shell; a child backend never closes a World (its join did). - /shannon [world|trace|children]: read-only inspection of the session's World (agent, projected capabilities with grant depth, agent tree, last receipts) through a per-workspace pointer file the backend writes under the Codewhale home and removes on close. Registered in the utility group with a portable handler; CmdShannonDescription added to every locale. Counterpart in ShannonNet 77baab1 (`memory add/list`, `context compile`, `world close`). Gates: cargo fmt --check ok; cargo clippy -p codewhale-tui --lib --tests -D warnings ok; targeted: sandbox::shannon 9 passed, commands::groups::utility::shannon 2 passed, commands::tests green after listing /shannon among migrated groups. Full suite (RUST_MIN_STACK=8388608 as CI): 11689 passed; 2 failed — model_inventory::tests::ollama_default_prefers_live_local_tags_over_the_unresolved_marker (third time across this session's full runs) and runtime_chat_relay::tests::failed_state_writes_never_become_in_memory_authority_and_exact_retry_reopens; both untouched by this diff and both pass in isolation with their modules (runtime_chat_relay::tests: 17 passed / 0 failed). Per .config/nextest.toml a flaky test is a bug report: filed here as one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jw7EuFvtYMHHK5Cu6JLEjc (cherry picked from commit a528a11fec586bda66ba69cec6e0ed8209f3d835)
Two tests failed only under full-suite load: 1. model_inventory::tests::ollama_default_prefers_live_local_tags_ over_the_unresolved_marker — five tests mutated the process-wide provider-lake live snapshot without the serialization lock, so a concurrent clear_live_snapshot() could wipe merged rows between the test's merge and its assertion. models_dev_live's four tests and the subagent catalog-facade test now hold lock_live_snapshot(). 2. runtime_chat_relay::tests::failed_state_writes_never_become_in_ memory_authority_and_exact_retry_reopens — RuntimeProcessOwnerLock still had the same-process drop-then-reopen WouldBlock exposure that #5735 fixed for RelayScopeLock in this same test (unlock on Drop + bounded WouldBlock retry); the reopen reacquires it unmitigated. It now mirrors the RelayScopeLock fix: flock LOCK_UN before close and a 25 ms WouldBlock-only retry, so a genuinely held lock is still reported as ownership. Serializing the live snapshot exposed a latent ABBA deadlock: under #[cfg(test)] every codewhale_env_var read blocks on the test env barrier, so a thread holding the live-snapshot mutex while constructing a client (deep env read) deadlocks against an env-barrier holder waiting for the live-snapshot mutex; libtest has no per-test timeout, so one inverted pair hangs the whole binary. lock_live_snapshot() now takes the env barrier first itself (skipped when the caller already sealed the environment), which makes the inversion impossible for every current and future caller. Gates (RUST_MIN_STACK=8388608, --locked env): - cargo test -p codewhale-tui --lib: 11691 passed; 0 failed (twice, 117s each; both formerly flaky tests ok in both runs) - targeted: provider_lake+pricing+models_dev_live+model_inventory+ runtime_chat_relay+subagent catalog test: 144 passed; 0 failed - cargo fmt -p codewhale-tui -- --check: clean - cargo clippy -p codewhale-tui -- -D warnings: clean (cherry picked from commit eec9b3fb63c3fd9bea3423dc74281bd961a653e5)
…hale` route
Two changes that both delete a compiled list the control plane already owns.
**`codewhale account keys` reads the account catalog.** The hardcoded
eight-provider `CloudProvider` enum in `crates/cli/src/cloud.rs` is gone.
`set`, `remove`, and `list` accept any id the control plane's public catalog
(`GET /api/model-providers`) lists, fetched through the same transport as
every other account call (no redirects replaying bodies, `MAX_RESPONSE_BYTES`,
30s timeout). The catalog is data, not code: every id — from the response or
from the command line — is re-validated against `^[a-z0-9][a-z0-9-]{0,63}$`
before it can reach a URL path, unknown ids fail locally naming what the
account does offer, and `list` shows every catalog provider with its label and
stored-key state. `--from-local` maps a catalog row onto the local runtime
provider through the catalog's own `runtimeProvider` field (`xiaomi` →
`xiaomi-mimo`) rather than a compiled slug table, so a newly supported provider
needs no CLI release. The existing safety rules are untouched: no `--api-key`
on the command line, stdin or hidden prompt only, length bounds, no redirects.
`codewhale account api-keys` keeps its shape and gains `models:infer` in the
closed scope set; an omitted `--scope` now sends all three explicitly instead
of relying on a server default. `--use` saves the freshly minted secret as this
machine's local `codewhale` provider credential in the same secret store
`codewhale auth` writes — a local write, never an upload.
**`codewhale` is a runtime provider.** Modeled on OpenCode Zen
(`TransportKind::ModelAware`): one base URL `https://api.codewhale.net/v1`
(overridable by `CODEWHALE_API_BASE`, HTTPS required except loopback — the
value is validated in both the config and TUI env paths, and the workspace-wide
insecure-HTTP escape hatch deliberately does not reopen it), one
`CODEWHALE_API_KEY` (`cwc_key_…` with `models:infer`), and a per-model wire.
Discovery runs on the existing per-provider live-catalog seam: the
authenticated `GET {base}/models` is the authority, ids are `provider/model`
verbatim, and `codewhale.protocol` picks chat completions or the Anthropic
Messages passthrough. Both protocols authenticate with `Authorization: Bearer`
— the Messages route's usual `x-api-key` default is explicitly excluded. When
the catalog is unreachable the route falls back to three bootstrap ids with the
protocol inferred from the namespace, and every surface that shows them says
they are a bootstrap, not the account's catalog.
The route flows into `codewhale providers export --json`; golden route ids,
`providers-export.golden.json`, `docs/PROVIDERS.md`, `docs/CONFIGURATION.md`,
and the web facts are regenerated. OAuth/subscription routes, local runtimes,
and every other provider are untouched, and no credential is copied anywhere
automatically.
Verified locally on this host:
- `cargo fmt --all -- --check`: clean
- `cargo clippy -p codewhale-config -p codewhale-cli -p codewhale-secrets
--all-targets --all-features -- -D warnings`: clean
- `cargo clippy -p codewhale-tui --all-targets --all-features -- -D warnings`: clean
- `cargo test -p codewhale-config`: 638 passed, 0 failed, 1 ignored (+1 doc test)
- `cargo test -p codewhale-cli`: 382 passed, 0 failed (355 lib + 27 integration)
- `cargo test -p codewhale-secrets`: 62 passed, 0 failed, 1 ignored
- `cargo test -p codewhale-tui --lib`: 11693 passed, 0 failed, 12 ignored
- `scripts/check-provider-registry.py`: PASS
- `scripts/check-dead-code-budget.py`: PASS (420, at budget)
- `web/scripts/check-facts.mjs`: OK
- `codewhale providers export --json | jq '.routes[]|select(.id=="codewhale")'`
shows the new route
Not done: no live call against api.codewhale.net (needs an account key and
founder-gated spend); the account control plane lives in a separate repo, so
the catalog and `/v1/models` contracts are exercised against local fakes only.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LnHs3mJt7WKheqqqX9btLq
(cherry picked from commit 3d2e07fdd86c8cfc959dfc4ac122c1aa82935523)
The `codewhale` route dispatched with no `Authorization` header at all.
Reproduced from this worktree's build against a loopback echo server: `POST
/v1/chat/completions` carried only content-type, accept, user-agent, host, and
content-length, and the real control plane answered 401 `api_key_required`.
Two compounding causes, both in credential scoping rather than in the header
builder:
1. `CODEWHALE_API_BASE` is the route's *own* documented override, but
`provider_base_url_is_official` knew only the default base, so any declared
origin — a loopback stub, or the same host spelled without `/v1` — made the
endpoint look "custom". A custom endpoint must never inherit an official
provider's ambient credential, so the saved and exported key were both
withheld. `provider_base_url_is_official` now accepts the default base plus
the origin the operator declared in `CODEWHALE_API_BASE`, which is already
validated as HTTPS-or-loopback. Anything else stays custom and keyless: an
undeclared host still gets nothing, and there is a test for that.
2. With no key resolved, both `should_skip_secret_store_for_provider` and the
resolver's keyless tail treated a loopback origin as a self-hosted runtime
and returned an empty key, so the request went out unauthenticated instead
of failing. The Codewhale API is authenticated on every origin it may reach;
it is now excluded from both keyless arms and fails closed.
A missing key now fails before any request with a message naming
`CODEWHALE_API_KEY`, `codewhale account api-keys create --name <name> --scope
models:infer --use`, and the key console.
Verified from this worktree's `target/debug/codewhale` against a loopback echo
server, one scenario per credential source. The exact header the request now
carries is:
authorization: Bearer cwc_key_3f2a9c1e4b7d8a0f5c6e2b91_AAAA…
- `CODEWHALE_API_KEY` export → `POST /v1/chat/completions` with that header
- `codewhale` secret-store slot (what `--use` writes) → same header
- `[providers.codewhale] api_key` → same header
- `[providers.codewhale] api_key_env` → same header
- `--model anthropic/claude-sonnet-5` → `POST /v1/messages` with
`authorization: Bearer …` plus `anthropic-version: 2023-06-01` and no
`x-api-key`
- no key configured → the CLI errors and the server receives 0 requests
Tests added (5): `codewhale_chat_request_carries_the_account_bearer` and
`codewhale_messages_request_carries_the_account_bearer_not_x_api_key` drive the
real client against a wiremock and assert the recorded request's bearer;
`codewhale_route_resolves_its_key_from_every_documented_source`,
`codewhale_ambient_key_never_follows_an_undeclared_host`, and
`codewhale_route_without_a_key_fails_before_any_request` cover resolution,
the scoping guard, and the fail-closed message.
Verified locally on this host:
- `cargo fmt --all -- --check`: clean
- `cargo clippy -p codewhale-config -p codewhale-cli -p codewhale-secrets
--all-targets --all-features -- -D warnings`: clean
- `cargo clippy -p codewhale-tui --all-targets --all-features -- -D warnings`: clean
- `cargo test -p codewhale-config`: 638 passed, 0 failed, 1 ignored
- `cargo test -p codewhale-cli`: 382 passed, 0 failed
- `cargo test -p codewhale-secrets`: 62 passed, 0 failed, 1 ignored
- `cargo test -p codewhale-tui --lib`: 11698 passed, 0 failed, 12 ignored
Not done: still no live call against api.codewhale.net (needs an account key
and founder-gated spend). `GET /v1/models` discovery remains an
interactive-startup background refresh and is not called on the `exec` path —
pre-existing behavior shared with TelecomJS, Eden AI, Concentrate, and Ollama.
The route works without it because the resolver infers the wire from the model
namespace; wiring discovery into `exec` would add a network round-trip to every
headless run for five providers and is a separate decision.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LnHs3mJt7WKheqqqX9btLq
(cherry picked from commit 131488ed44252ccba26df99b58338c4c15eddfaa)
…unt-key provider) The account-key feature branch adds a first-class `codewhale` provider route (46 -> 47). facts.generated.ts already carries 47; only the committed docs/public-surface-facts.json matrix was stale, so public-surface-contract.test.ts asserted 46 against FACTS.providers.length. Verified: node scripts/derive-facts.mjs -> providers=47; check-facts.mjs OK; facts.generated.ts regenerates identically (no other drift).
…ox batch The rebased branch adds CHANGELOG entries for ShannonNet backend, sub-agent delegated authority, sandbox-compiled context, and the codewhale account-key provider. changelog.generated.ts was stale, failing changelog.test.ts. Verified: derive-changelog.mjs is idempotent (regenerates identical); npx vitest run -> 407 passed / 0 failed.
…erate derived changelogs Rebased the batch onto main after #5957/#5958/#5960/#5962/#5967 landed. The merged [Unreleased] section had grown a second '### Changed' header from the two-sided conflict resolution; folded into one. Regenerated crates/tui/CHANGELOG.md (sync-changelog.sh) and web/lib/changelog.generated.ts (derive-changelog.mjs) so Version drift and the web changelog test pass. cargo check -p codewhale-tui --lib --tests: clean. check-feature-release-notes.sh v0.9.12 HEAD: OK. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SJrzNAmppg4vt3LNJbaeri Signed-off-by: CodeWhale Bot <bot@codewhale.net>
240b14b to
faa860c
Compare
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_26bc29ee-9db4-4f91-aa16-75e7867b33e7) |
|
Claude finished @Hmbown's task in 2m 6s —— View job Claude's review of PR #5966
This PR already has extensive bot review coverage (Devin, Copilot, Codex, CodeQL, codewhale-agent). Rather than repeat all ~30 findings, I read the actual code paths to confirm which ones are real and add anything new. Provider/model route-safety invariant (EPIC #2608) — OK
Confirmed: live catalog does not actually drive Codewhale routing (agrees with Devin's #1 finding)I traced this end-to-end and it's real, not just a bot guess:
This undercuts the PR's headline claim ("picks chat-completions vs anthropic-messages per model from the live account catalog"). Worth fixing before merge or in fast-follow, since it's a functional/correctness gap in the new provider's core selling point, not just a nice-to-have. Confirmed: Shannon executions still report
|
| fn world_name(workspace: &Path) -> String { | ||
| let slug: String = workspace | ||
| .to_string_lossy() | ||
| .chars() | ||
| .map(|c| if c.is_ascii_alphanumeric() { c } else { '-' }) | ||
| .collect(); | ||
| format!("codewhale:{}", slug.trim_matches('-')) |
There was a problem hiding this comment.
🟡 Distinct workspaces share session pointers
world_name maps different paths such as /a-b and /a/b to one slug. Their pointers overwrite each other, so inspection targets the wrong session.
Prompt for agents
world_name is also used as the session pointer filename key, but its lossy punctuation replacement is not injective. Distinct workspace paths can overwrite and remove each other's pointer files. Keep a readable prefix if useful, but include a stable hash of the canonical workspace path in the pointer key. Consider multiple concurrent sessions for the same workspace when defining ownership and cleanup.
Was this helpful? React with 👍 or 👎 to provide feedback.
| binary: PathBuf::from(field("shannon_binary")?), | ||
| home: PathBuf::from(field("shannon_home")?), |
There was a problem hiding this comment.
🟥 Session pointer permits arbitrary execution
/shannon executes shannon_binary from an unauthenticated pointer file. Replacing that file runs an attacker-chosen program with the user's privileges.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let capability = config | ||
| .sandbox_shannon_capability | ||
| .clone() | ||
| .filter(|cap| !cap.trim().is_empty()) | ||
| .unwrap_or_else(|| super::shannon::DEFAULT_CAPABILITY.to_string()); |
There was a problem hiding this comment.
🟨 Custom capabilities bypass sandbox intent
Any non-empty sandbox_shannon_capability is accepted as the execution authority. A broader capability can run while Codewhale still reports sandboxed execution.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Codewhale review
PR #5966 adds catalog-driven account keys, a first-class Codewhale provider with bearer auth, ShannonNet sandbox execution, and two flake fixes. Provider/catalog and auth changes are well-tested and largely sound, but the ShannonNet sync path has a critical argv-size bug and can exfiltrate hidden local data, and the /shannon inspection command trusts an unvalidated binary path.
Findings
- [ERROR] ShannonNet workspace sync passes large base64 archives via command-line arguments (
crates/tui/src/sandbox/shannon.rs)
sync_workspace builds JSON containing base64-encoded gzip archives and passes each as the--inputargument toshannon cap invoke. SYNC_CHUNK_BYTES is 6 MiB raw; base64 inflates that to about 8 MiB, far above typical OS ARG_MAX (~2 MiB on Linux). A full initial sync of a normal repository will fail with E2BIG, making the ShannonNet backend unable to sync any non-trivial workspace. The archive must be streamed via stdin or a temp file, not argv. - [WARNING] Workspace sync can exfiltrate hidden local directories such as .codewhale (
crates/tui/src/sandbox/shannon.rs)
WorkspaceSync::list uses ignore::WalkBuilder::hidden(false), so hidden files and directories are included. The only explicit exclusion is.git. A workspace-local.codewhaledirectory (notes, session state, possibly local config/secrets) is therefore considered for sync and shipped to the remote ShannonNet worker. This leaks local-only Codewhale data to a remote capability provider. Exclude.codewhale(and consider other local-only directories) from sync. - [WARNING] /shannon executes an unvalidated binary path from a session pointer file (
crates/tui/src/commands/groups/utility/shannon.rs)
read_pointer deserializesshannon_binaryandshannon_homefrom ~/.codewhale/shannon-sessions/*.json and then executes that binary with--home. A tampered pointer file (or one written by a compromised process) can cause arbitrary local code execution when the/shannoncommand runs. Validate that the binary is the expectedshannonCLI (e.g., absolute path owned by user, or resolve from PATH without trusting pointer), or at least avoid accepting arbitrary paths from this file. - [INFO] Missing localized strings for /shannon command (
crates/tui/locales/en.json)
All locale JSON files addCmdShannonDescriptionwith the English sentence only; non-English locales now show English for this command. This is not a functional bug but degrades localized UX.
Suggestions
crates/tui/src/sandbox/shannon.rs— Changerun_json/run_json_blockingor the sync path so the--inputpayload is delivered to theshannonprocess via stdin (e.g.Command::stdin(Stdio::piped())and writing the JSON) or a temporary file, instead of as a command-line argument. This avoids ARG_MAX/E2BIG failures for workspace archives and keeps arbitrary command strings/environment values out of process arguments.crates/tui/src/sandbox/shannon.rs— InWorkspaceSync::list, extendfilter_entryto also exclude the.codewhaledirectory (and optionally other local-only state directories) so workspace-local Codewhale data is never shipped to the remote worker.
Assessment
The provider catalog and Codewhale route changes are solid and well-tested, as are the flake fixes. The ShannonNet backend, however, has a critical sync design issue (argv size) and a hidden-file exfiltration risk that should be addressed before shipping.
Advisory review by Codewhale (codewhale review --pr 5966 --post, head faa860cce824cc1cd339a6d314b739ea3ddc178e). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
What
Rebase of the account-key provider registry + sandbox/ShannonNet batch onto current
origin/main, with the #5929 shell-source conflicts resolved.feat(providers): catalog-driven account keys and a first-class
codewhalerouteaccount keysproviders;codewhale account api-keys create --scope/--use.codewhaleprovider route (CODEWHALE_API_KEY,CODEWHALE_API_BASE).x-api-key.sandbox: ShannonNet backend
sandbox_backend = "shannon": shell commands run as signed ShannonNet capability invocations./shannoninspection.fix(tui): root-cause both load-dependent test flakes (part of #5929)
model_inventory::…ollama_default…(live snapshot serialization) +runtime_chat_relay::…failed_state_writes…(same-process drop-then-reopen).lock_live_snapshot()takes the env barrier first, eliminating the ABBA inversion.Verification (rebased onto current origin/main)
cargo test -p codewhale-config --lib: 640 passed; 0 failed.cargo test -p codewhale-tui --lib: 11862 passed; 0 failed; 13 ignored (~157s).cargo fmt -p codewhale-tui -- --check: clean.cargo clippy -p codewhale-tui -- -D warnings: clean.Closes nothing on its own; the flake half of #5929 is reduced by the
model_inventory+runtime_chat_relayfixes.No-Issue: this branch lands a feature batch — the two flake fixes it contains are tracked under #5929 (the tracker stays open for the remaining listed tests); the account-key providers are their own standalone feature.
Note
High Risk
Touches authentication, remote catalog trust boundaries, and a new remote execution sandbox (ShannonNet) where misconfiguration could run commands off-host or send bearer tokens to untrusted origins.
Overview
Account provider keys no longer depend on a compiled eight-provider list.
codewhale account keys list|set|removeloads ids from the control plane (GET /api/model-providers), validates them locally before they hit URL paths, lists every catalog row with label and key status, and maps--from-localthrough each row’sruntimeProvider.First-class
codewhaleprovider adds account-backed inference:cwc_key_…withmodels:infer, optionalCODEWHALE_API_BASE(HTTPS except loopback), model-aware routing from liveGET /v1/models(codewhale.protocol) with offline bootstrap rows, and Bearer auth on both Chat Completions and Anthropic Messages (neverx-api-key). Config, route resolver, secrets, and HTTP client/catalog refresh are wired end-to-end.Account API keys gain
models:infer; omitting--scopenow sends all three scopes explicitly.--usestores the new secret only in the localcodewhaleprovider slot after create.ShannonNet sandbox (
sandbox_backend = "shannon"): signed capability exec, per-session Task World, optional workspace sync, delegated sub-agent worlds, and read-only/shannoninspection via a session pointer file. Sandbox creation now takes the workspace path (e.g. ACP).Changelog and locale strings document parked-agent UI and fleet tweaks where included in this batch.
Reviewed by Cursor Bugbot for commit faa860c. Bugbot is set up for automated code reviews on this repo. Configure here.