Skip to content

cloud agent: product-owned Daytona snapshot + two fixes found by running agents inside a Computer - #5745

Open
Hmbown wants to merge 4 commits into
mainfrom
fix/cloud-agent-e2e-20260830
Open

cloud agent: product-owned Daytona snapshot + two fixes found by running agents inside a Computer#5745
Hmbown wants to merge 4 commits into
mainfrom
fix/cloud-agent-e2e-20260830

Conversation

@Hmbown

@Hmbown Hmbown commented Aug 30, 2026

Copy link
Copy Markdown
Owner

What

Receipts and fixes from the first end-to-end run of the Codewhale Engine inside a Daytona Computer (PRD §4.5), with real turns on Alibaba Model Studio (DeepSeek V4 Pro, Qwen 3.8 Flash).

  • computer/snapshots/cloud-agent/ — product-owned definition of the codewhale-cloud-agent snapshot feat(cli): cloud-dispatch remote runner — sandbox to forge PR #5712 expects (v0.9.11 static musl x64 release asset, sha256-pinned, non-root agent user, no secrets baked in) + README incl. a credential-exposure note (Daytona persists create-time env server-side → keys must be injected post-create from stdin into a 0600 file).
  • exec: honour dispatcher-forwarded --provider/--model on --resume/--continuecodewhale --provider X --model Y exec --continue … silently ran on the config default provider.
  • subagent: scope session-name reservation to the live session — a second codewhale exec in the same workspace could not spawn worker-a/worker-b because completed agents from earlier boots (hydrated from .codewhale/state/subagents.v1.json) still held the names.

Proven inside the Computer (level 6/7, sandboxes deleted afterwards)

R1 --version + live Model Studio model list ✅ · R2 one agent turn, DeepSeek V4 Pro: 4 turns read→edit→bash, docstring diff landed, 16 s ✅ (api.deepseek.com fallback 7 s ✅) · R3 two subagents spawned inside (spawn_depth 1, both settled) ✅ · R3b Qwen 3.8 Flash workers ✅ after the name-reservation fix · R4 kill -9 mid-turn → partial recovery (headless exec has no mid-turn checkpoint; #5715) · R5 #5712 launch compatibility: paths/labels OK, credentials fail as shipped (only CODEWHALE_API_KEY is injected and nothing reads it as a provider credential — high, not fixed here) · R6 Computer uptime 873 s; parent usage 159,542 input tokens; child usage not propagated.

Open defects logged for follow-up: #5712 credential plane; collect_patch base branch resolves to HEAD on Daytona clones (origin/HEAD is a plain ref); .codewhale/state written inside the clone unignored; wrapper lacks --max-subagents; non-enum providers rejected by models; child usage/cost unknown for Model Studio routes.

Evidence

Fixes verified by cargo check --tests + 5 focused unit tests on the host and by the R3b/resume re-runs inside the Computer; dogfood re-run reviewer approved; secrets reviewer's major (README credential path) addressed in the last commit. No key material in any artifact; keys were read only from the local Codewhale store into process env.

No-Issue: PRD §4.5 flagship proof (agents inside Daytona Computers); refs #5715, #5712

🤖 Generated with Claude Code

https://claude.ai/code/session_014aDEyM2a4pPZ9qqMDrP5YX

CodeWhale Bot and others added 4 commits August 30, 2026 11:13
…finition

Pins the released v0.9.11 Linux x64 engine (commit 96d13a0, sha256
c02969556e51e138afa3fe9c97a1359878cd3d1986b1ce1f5fa96c93c6909416) into a
debian:bookworm-slim Computer image with the agent toolchain (git, node 22,
python3, ripgrep, build-essential), non-root agent user, and /workspace
prepared for the cloud dispatcher (PRD 4.5). Built and probed on Daytona
as snapshot codewhale-cloud-agent (4 CPU / 8 GB / 10 GB).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014aDEyM2a4pPZ9qqMDrP5YX
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
…ntinue

The `codewhale` dispatcher rejects `--provider`/`--model` after `exec`
and forwards the top-level flags as CODEWHALE_PROVIDER / CODEWHALE_MODEL.
`resolve_exec_resume_route` only looked at the exec-level flags, so
`codewhale --provider modelstudio-token-plan --model qwen3.8-flash exec
--continue ...` silently restored the saved route (observed in the Daytona
cloud-agent e2e on 2026-08-30: the resumed turn ran on deepseek-v4-pro).

Fold the launch overrides into the explicit route signals for resume; a
fresh session already treated them as explicit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014aDEyM2a4pPZ9qqMDrP5YX
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
A completed worker hydrated from a previous session's workspace ledger is
invisible to status/peek/followup (`Agent not found in the active
session`), yet spawn still refused its name with "already in use by
agent_id ... (status: completed)". A second `codewhale exec` in the same
workspace could not spawn worker-a/worker-b again and burned six turns
recovering (Daytona cloud-agent e2e, 2026-08-30).

Skip prior-session agents in the collision check, matching the other
model-facing actions. The existing duplicate-name test now stamps the
manager's own boot id so it keeps covering the live-session case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014aDEyM2a4pPZ9qqMDrP5YX
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
…create key injection

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-30T18:53:24.318952Z c6051a3 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

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


Claude finished the review

Todo list


Summary

Two focused runtime fixes plus a product-owned Daytona snapshot definition. The two code fixes are correct, minimal, and reuse existing helpers rather than adding new ones. One real issue found in the new README.

Findings

1. computer/snapshots/cloud-agent/README.md:44-58 — contradictory credential guidance (should fix)

The "Credential exposure note" (lines 44-54) is explicit: "Do NOT put provider keys in create-time env. Inject them after the Computer starts" — because Daytona persists -e KEY=VALUE server-side and returns it via GET /sandbox/{id}.

The very next paragraph (lines 56-58) says the opposite: "an end-to-end run must also inject provider env at create time, never on a command line."

As written, a reader following the second instruction would recreate the exact leak the first paragraph just warned about. This should say "inject post-create" (matching the note above and the 0600 file / stdin approach it describes), not "at create time." Given this file's whole purpose is to document the safe credential path, this contradiction is worth tightening before it's followed literally.
Fix this →

Correctness — crates/tui/src/lib.rs

exec_resume_route_overrides (lib.rs:977-1000) and its wiring into the exec --resume/--continue path (lib.rs:2309-2321) look correct:

  • It folds CODEWHALE_PROVIDER/CODEWHALE_MODEL (dispatcher-forwarded launch overrides) together with the exec-level --provider/--model flags, with exec flags winning — matches the stated precedence and the doc comment.
  • Trimming/blank handling (non_empty helper) correctly treats " "/"" as absent, matching existing conventions elsewhere in the file (e.g. explicit_model parsing at line 2286-2290).
  • The non-resume path (resolve_exec_model, line 848-855) already falls back to exec_model_env_override internally, so it didn't need the same fix — only resolve_exec_resume_route was missing that fallback. Scoping the fix to the resume path only is correct, not an inconsistency.
  • Verified the custom-a/custom-b test cases (lib.rs:15008-15032) against custom_exec_config (lib.rs:14143-14172): the expected "model-b" default and provider-retention assertions are correct given resolve_exec_resume_route's existing explicit_provider short-circuit (skips scope_to_provider_identity, line 946).

Correctness — crates/tui/src/tools/subagent/mod.rs

The name-reservation fix (line 6479-6497) scopes the collision check to !self.is_from_prior_session(existing), reusing the existing is_from_prior_session helper (mod.rs:4192-4198) already used for listing/status filtering elsewhere (lines 4929, 4943, 7034, 7115, 7120) — good reuse, no new parallel classification logic introduced. Since the check is keyed on session_boot_id rather than agent status, it also correctly excludes a stale-but-still-Running prior-boot agent, not just Completed ones, which is more robust than the PR description's "completed agents" framing suggests.

Provider/model/route safety (EPIC #2608)

No violations. Both fixes route strictly through explicit signals — CLI flags, CODEWHALE_PROVIDER/CODEWHALE_MODEL env, and session-boot identity — never inferring provider/model from a model-id prefix or freeform text. The README's provider table (lines 60-64) is documentation only, not inference logic.

Reuse/simplification

Good marks here: both fixes reuse pre-existing helpers (explicit_launch_provider_override/explicit_launch_model_override in config.rs, and is_from_prior_session in subagent/mod.rs) instead of adding parallel logic.

Tests

Both fixes ship targeted unit tests exercising the exact regression:

  • exec_resume_honours_dispatcher_forwarded_launch_overrides (lib.rs) covers no-override, launch-provider-only, launch-model-only, exec-flags-win-over-launch, and blank-value cases, plus two full resolve_exec_resume_route integration cases.
  • spawn_session_name_held_by_prior_session_agent_does_not_collide (subagent/tests.rs) covers the stale-prior-session-name case and asserts both the happy path (fresh agent gets the name) and that no "already in use" error path degrades to a false collision.

I wasn't able to run cargo check/cargo test in this sandbox (Bash commands required interactive approval that wasn't available), so this review is based on manual reading of the diff and surrounding code, not a compiled/executed check. The logic traced cleanly through the existing helper signatures and call sites, but a cargo check -p codewhale-tui --tests before merge is worth doing if it hasn't run in CI yet.

Security

No secrets in the diff. The Dockerfile pins the release binary by exact URL + sha256 (verified both hashes are well-formed 64-hex-char sha256 digests) and bakes in no keys, consistent with the PR's stated secrets-reviewer fix — aside from the README contradiction above.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c6051a38c5

ℹ️ 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".

Comment on lines +56 to +58
The dispatcher injects only `CODEWHALE_API_KEY` (account machine token). The
engine resolves model-provider keys from ambient env, so an end-to-end run
must also inject provider env at create time, never on a command line:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove the create-time provider-key instruction

When an operator follows this end-to-end setup, this line tells them to inject MODELSTUDIO_API_KEY or DEEPSEEK_API_KEY through Daytona's create-time environment, even though the immediately preceding warning establishes that these values persist server-side and are retrievable by anyone with Daytona API access. Following the documented procedure therefore exposes provider credentials; it should consistently describe a usable post-create secret-injection path instead.

Useful? React with 👍 / 👎.

Comment on lines +6490 to +6491
if let Some(existing) = self.agents.values().find(|existing| {
existing.session_name == name && !self.is_from_prior_session(existing)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Scope duplicate names by conversation rather than process boot

When Codewhale restarts and resumes the same saved conversation, load_state retains each agent's persisted owner_session_id while the manager gets a new boot ID. This predicate consequently ignores an archived agent belonging to the resumed conversation and admits another child with the same name; resolve_agent_ref_for_session then finds both rows by that shared owner and makes name-based status/peek/followup ambiguous. Conversely, switching conversations within one process still lets a same-boot foreign row reserve the name. The collision check should use the active runtime.context.state_namespace against owner_session_id, which is the identity used by those user-facing lookups.

Useful? React with 👍 / 👎.

Comment on lines +35 to +39
- Snapshot name: `codewhale-cloud-agent` (`DEFAULT_CLOUD_AGENT_SNAPSHOT`);
override with `CODEWHALE_DISPATCH_SNAPSHOT`.
- The runner clones the repository into `/workspace` (`SANDBOX_WORKSPACE`)
and runs `codewhale exec --auto "<prompt>"` there through the Daytona
toolbox `process/execute` endpoint.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wire the new snapshot into the production dispatcher

In the reviewed tree this advertised contract has no implementation: a repository-wide symbol search finds none of DEFAULT_CLOUD_AGENT_SNAPSHOT, CODEWHALE_DISPATCH_SNAPSHOT, or SANDBOX_WORKSPACE outside this new documentation, and LiveDaytonaLauncher::create_sandbox in crates/tui/src/cloud_dispatch.rs:663-688 posts only a generated name and labels. It neither selects this snapshot nor clones the remote or invokes process/execute, so /dispatch continues creating an unspecified idle sandbox and the newly added image is never used by the product flow.

Useful? React with 👍 / 👎.

Comment on lines +27 to +31
# ---- pins (release v0.9.11, tag commit 96d13a0bc3f40280ea3865280ad5ccf0e2845e6f)
ENV CODEWHALE_VERSION=0.9.11 \
CODEWHALE_COMMIT=96d13a0bc3f40280ea3865280ad5ccf0e2845e6f \
CODEWHALE_ASSET_URL=https://github.com/Hmbown/CodeWhale/releases/download/v0.9.11/codewhale-linux-x64 \
CODEWHALE_ASSET_SHA256=c02969556e51e138afa3fe9c97a1359878cd3d1986b1ce1f5fa96c93c6909416 \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Ship a binary containing the accompanying cloud fixes

When this snapshot is built and used, it always installs commit 96d13a0…, which is an ancestor of the reviewed commit and does not contain either exec_resume_route_overrides or the changed subagent name-reservation logic added here. Consequently the advertised cloud image still exhibits the wrong-provider resume and stale-name failures that this commit claims to fix, even after rebuilding from this Dockerfile. Pin a release containing these changes or build and checksum the reviewed source revision.

Useful? React with 👍 / 👎.

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codewhale review

Adds a product-owned Daytona cloud-agent snapshot and fixes two issues found by running agents inside a Computer: dispatcher-forwarded --provider/--model are now honored on exec --resume/--continue, and subagent session-name reservation is scoped to the live session. The Rust changes include focused unit tests, and the Dockerfile/README are generally well structured. Main concerns are documentation contradictions around credential injection and a couple of test-correctness/integration gaps.

Findings

  • [WARNING] Dockerfile comment still says provider keys arrive via Daytona create-time env injection (computer/snapshots/cloud-agent/Dockerfile:8)
    The header comment claims provider keys and the account machine token arrive only through sandbox create-time env injection, but the README's credential-exposure note states create-time env is persisted server-side and readable via GET /sandbox/{id}; provider keys must be injected post-create from stdin into a 0600 file. Update this comment to avoid leading users to inject provider keys at create time.
  • [WARNING] README instructs provider env injection at create time, contradicting its own security guidance (computer/snapshots/cloud-agent/README.md:58)
    The paragraph after the credential-exposure note says an end-to-end run 'must also inject provider env at create time', while the preceding note says 'Do NOT put provider keys in create-time env' and requires post-create injection. This ambiguity can cause a Daytona API user to expose provider credentials. Clarify that 'create time' refers to the toolbox process/execute environment, not daytona create envVars, or change the wording.
  • [WARNING] Launch provider override is reduced to a bool and lacks an integration test for the resumed dispatch path (crates/tui/src/lib.rs:997)
    exec_resume_route_overrides returns only (explicit_provider: bool, explicit_model: Option), so the actual provider value is not passed to resolve_exec_resume_route. The caller depends on config.provider already being mutated from the launch env before this code. The new unit tests exercise only the helper, not the real path where CODEWHALE_PROVIDER/MODEL are set and a saved session is resumed, so a regression in that side effect would keep silently restoring the saved route. Add an integration test or pass the resolved provider value through.
  • [INFO] Subagent regression test passes on any non-collision failure (crates/tui/src/tools/subagent/tests.rs:8842)
    In spawn_session_name_held_by_prior_session_agent_does_not_collide, the Err arm asserts only that the error message does not contain 'already in use'. If spawn_background_with_assignment_options fails for an unrelated reason such as setup or tool context, the test still passes and does not actually verify the session-name scoping fix. Prefer panicking on Err or asserting the Ok path with an explicit expect.

Assessment

The core code changes are sound and well-targeted, but the documentation has contradictory guidance on provider key injection that should be reconciled before merge. The subagent test should be tightened so unexpected errors do not cause a false pass, and the resumed dispatch path would benefit from an integration-level test proving the forwarded launch overrides are actually applied.


Advisory review by Codewhale (codewhale review --pr 5745 --post, head c6051a38c5c286d7a51474570f895edf9a082306). 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.

# binary (static musl, no glibc floor) from the GitHub release by exact URL,
# verifies its sha256 against codewhale-artifacts-sha256.txt, and records the
# commit + digest as OCI labels (PRD 4.5: commit- and digest-pinned Linux
# binary inside the Computer). No secrets are baked in: provider keys and the

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[WARNING] Dockerfile comment still says provider keys arrive via Daytona create-time env injection

The header comment claims provider keys and the account machine token arrive only through sandbox create-time env injection, but the README's credential-exposure note states create-time env is persisted server-side and readable via GET /sandbox/{id}; provider keys must be injected post-create from stdin into a 0600 file. Update this comment to avoid leading users to inject provider keys at create time.


The dispatcher injects only `CODEWHALE_API_KEY` (account machine token). The
engine resolves model-provider keys from ambient env, so an end-to-end run
must also inject provider env at create time, never on a command line:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[WARNING] README instructs provider env injection at create time, contradicting its own security guidance

The paragraph after the credential-exposure note says an end-to-end run 'must also inject provider env at create time', while the preceding note says 'Do NOT put provider keys in create-time env' and requires post-create injection. This ambiguity can cause a Daytona API user to expose provider credentials. Clarify that 'create time' refers to the toolbox process/execute environment, not daytona create envVars, or change the wording.

Comment thread crates/tui/src/lib.rs
.map(str::to_string)
};
let explicit_provider =
non_empty(exec_provider).is_some() || non_empty(launch_provider).is_some();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[WARNING] Launch provider override is reduced to a bool and lacks an integration test for the resumed dispatch path

exec_resume_route_overrides returns only (explicit_provider: bool, explicit_model: Option), so the actual provider value is not passed to resolve_exec_resume_route. The caller depends on config.provider already being mutated from the launch env before this code. The new unit tests exercise only the helper, not the real path where CODEWHALE_PROVIDER/MODEL are set and a saved session is resumed, so a regression in that side effect would keep silently restoring the saved route. Add an integration test or pass the resolved provider value through.

.agents
.get(&result.agent_id)
.expect("fresh agent registered");
assert_eq!(fresh.session_name, "researcher");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[INFO] Subagent regression test passes on any non-collision failure

In spawn_session_name_held_by_prior_session_agent_does_not_collide, the Err arm asserts only that the error message does not contain 'already in use'. If spawn_background_with_assignment_options fails for an unrelated reason such as setup or tool context, the test still passes and does not actually verify the session-name scoping fix. Prefer panicking on Err or asserting the Ok path with an explicit expect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant