Skip to content

fix(daytona): allow long-running SDK commands - #1457

Open
arisylafeta wants to merge 3 commits into
openclaw:mainfrom
arisylafeta:rebattery/daytona-exec-timeout-simple
Open

fix(daytona): allow long-running SDK commands#1457
arisylafeta wants to merge 3 commits into
openclaw:mainfrom
arisylafeta:rebattery/daytona-exec-timeout-simple

Conversation

@arisylafeta

@arisylafeta arisylafeta commented Aug 21, 2026

Copy link
Copy Markdown

What Problem This Solves

Fixes an issue where direct Daytona SDK setup, sync extraction, and user commands could be cut short by the SDK's default 60-second HTTP timeout. The original proposed 60-minute process timeout and 61-minute HTTP timeout would also have imposed a new fixed ceiling on otherwise valid longer workloads.

Why This Change Was Made

Daytona process requests now receive the remaining caller-context budget, rounded upward to whole seconds and capped at the SDK's signed 32-bit maximum. A caller without a deadline receives that same maximum. The run-owned toolbox HTTP client no longer imposes an independent wall-clock timeout; its request still stops when the caller context is canceled. Existing provider-owned sandbox cleanup remains separately bounded.

User Impact

Direct Daytona commands follow the execution budget already selected by the caller instead of being stopped by an unrelated SDK HTTP deadline or a new one-hour cap. No public CLI flag or configuration option changes. A caller without a deadline relies on explicit context cancellation, the provider's maximum execution timeout, and ordinary sandbox lifecycle cleanup.

Evidence

Validated exact head: e8d9e57b73fb83b337df2db73018466d0df5b46f.

Hosted Daytona lifecycle

The exact-head CLI created one real Daytona sandbox, executed a bounded command successfully, and released it:

$ /private/tmp/crabbox-pr1457 run --provider daytona --no-sync --idle-timeout 5m --ttl 15m --timing-json -- sh -c 'printf "daytona-live-start\n"; sleep 2; printf "daytona-live-ok\n"'
daytona-live-start
daytona-live-ok
{"provider":"daytona","runStatus":"succeeded","exitCode":0,"commandMs":31581,"totalMs":71052}

The coordinator initially reported asynchronous release, so cleanup was explicitly reconciled and independently verified afterward:

$ /private/tmp/crabbox-pr1457 stop --provider daytona --id <task-created-lease>
released lease=<redacted> server=<redacted>

$ /private/tmp/crabbox-pr1457 doctor --provider daytona --json
{"ok":true,"daytona":{"inventory":"ready","leases":"0","mutation":"false"}}

$ /private/tmp/crabbox-pr1457 claims list --json
{"remainingClaims":0}

These last two outputs are sanitized, narrowed excerpts from the inspected JSON responses. The available hosted account is coordinator-managed and therefore runs commands over SSH, not through the direct Daytona SDK command runner changed here; the hosted smoke proves real Daytona provisioning, execution, release, and absence of leftover claims, not the direct SDK transport.

Direct SDK request and cancellation boundary

The exact-head regression exercises the pinned Daytona Go SDK against an instrumented local HTTP server and inspects the actual serialized /process/execute request:

$ go test ./internal/providers/daytona -run '^TestDaytonaCommandRunner' -count=1 -timeout=30s -v
=== RUN   TestDaytonaCommandRunnerPreservesCallerExecutionBudget
    caller deadline=none remote timeout=2147483647s HTTP client timeout=0s
    caller deadline=90m remote timeout=5400s HTTP client timeout=0s
    caller deadline=1.5s remote timeout=2s
    caller deadline=900ms remote timeout=1s
    caller deadline exceeds int32 remote timeout=2147483647s
    expired caller deadline rejected before a remote request was sent
--- PASS: TestDaytonaCommandRunnerPreservesCallerExecutionBudget
=== RUN   TestDaytonaCommandRunnerCancelsHTTPWithCallerContext
    caller cancellation stopped in-flight Daytona HTTP request; HTTP client timeout=0s
--- PASS: TestDaytonaCommandRunnerCancelsHTTPWithCallerContext
PASS

This proves that a 90-minute caller budget transmits 5400 seconds, beyond the superseded 3600-second cap, while request cancellation still interrupts an in-flight HTTP exchange. It does not claim a hosted direct-SDK workload actually ran for more than one hour. No direct Daytona API credential, authenticated Daytona CLI profile, or direct-mode snapshot was available; hosted direct-SDK execution would require that separate access.

Additional exact-head validation:

$ go test -race ./internal/providers/daytona -count=1
ok github.com/openclaw/crabbox/internal/providers/daytona

$ go vet ./...
success

$ go build -trimpath -o /private/tmp/crabbox-pr1457 ./cmd/crabbox
success

$ scripts/check-docs.sh
checked 59 command docs; checked 80 providers; checked 242 Markdown files; 14 docs-site tests passed

$ .agents/skills/autoreview/scripts/autoreview --mode local
autoreview clean: no accepted/actionable findings reported

The original contributor commit retains Arianit Sylafeta's authorship, the follow-up carries a matching Co-authored-by trailer, and the current Unreleased changelog thanks @arisylafeta.

@clawsweeper

clawsweeper Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 21, 2026
@clawsweeper

clawsweeper Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed August 24, 2026, 2:56 AM ET / 06:56 UTC.

ClawSweeper review

What this changes

The PR makes direct Daytona process commands and toolbox requests honor the caller’s context budget instead of fixed SDK HTTP or one-hour limits.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open: the Daytona timeout fix remains necessary, has sufficient exact-head behavior proof, and is functionally correct aside from one non-blocking release-note ownership cleanup.

Priority: P2
Reviewed head: e8d9e57b73fb83b337df2db73018466d0df5b46f

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused implementation and exact-head behavior evidence are strong; removing the release-owned changelog line would complete the remaining review cleanup.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): Exact-head terminal evidence shows a real Daytona lifecycle plus the real pinned SDK’s serialized timeout and context-cancellation behavior; redact any private identifiers if the evidence is reposted.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): Exact-head terminal evidence shows a real Daytona lifecycle plus the real pinned SDK’s serialized timeout and context-cancellation behavior; redact any private identifiers if the evidence is reposted.
Evidence reviewed 7 items Current-main gap: Current main still calls the Daytona process service directly for no-sync setup and the user command, without a caller-budget wrapper.
Proposed timeout boundary: The PR creates a runner that removes the client-wide timeout, derives the remote timeout from the context deadline, caps it to the SDK limit, and routes all three direct process calls through it.
SDK contract exercised: The focused regression constructs the pinned Daytona SDK process service, observes its serialized HTTP requests, covers no deadline, a 90-minute deadline, fractional rounding, the signed-32-bit cap, expiry, and cancellation.
Findings 1 actionable finding [P3] Remove the release-owned changelog entry
Security None None.

Live Verification

Command: go test ./internal/providers/daytona -run '^TestDaytonaCommandRunner' -count=1 -timeout=30s -v

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.23.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.23.0.tgz

Assertions:

  • FAIL expect_output: TestDaytonaCommandRunnerPreservesCallerExecutionBudget
  • FAIL expect_output: caller deadline=90m remote timeout=5400s
  • FAIL expect_output: PASS

How this fits together

Crabbox’s direct Daytona provider creates or resolves a sandbox, synchronizes workspace content through the toolbox API, and executes a remote process. The caller context flows into these operations and determines when the request and remote command should stop.

flowchart LR
  A[CLI run request] --> B[Caller execution context]
  B --> C[Daytona provider]
  C --> D[Workspace toolbox sync]
  C --> E[Remote process request]
  D --> F[Daytona sandbox]
  E --> F
  F --> G[Command result and cleanup]
Loading

Before merge

  • Remove the release-owned changelog entry (P3) - CHANGELOG.md is release-owned, so this provider fix should leave release-note selection and attribution to the release process. This is the still-unfixed P3 finding from the prior review cycle.
  • Resolve merge risk (P2) - Changing timeout ownership affects direct Daytona command availability; preserve caller-context cancellation while removing the independent client-wide wall-clock limit.
  • Complete next step (P2) - The sole remaining item is release-note ownership cleanup, which is not appropriate for an automated repair lane.

Findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus regression coverage production +39, tests +151, docs +5, release notes +1 The timeout behavior is concentrated in one provider wrapper and has substantial focused coverage of the SDK request boundary.

Merge-risk options

Maintainer options:

  1. Accept caller-owned timeout control (recommended)
    Merge the focused timeout change after the exact-head checks complete, retaining the context-cancellation regression coverage as the availability guardrail.

Technical review

Best possible solution:

Land the caller-owned timeout behavior with its focused SDK serialization and cancellation coverage, while leaving release-note ownership to the release process.

Do we have a high-confidence way to reproduce the issue?

Yes—source reproducibly identifies direct process calls on current main without caller-derived execution timeouts, and the exact-head SDK trace demonstrates the corrected request values and cancellation path.

Is this the best way to solve the issue?

Yes—the wrapper keeps the caller context as the authority, covers setup, extraction, and user commands, and avoids adding public configuration; remove only the release-owned changelog entry.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7
    CHANGELOG.md is release-owned, so this provider fix should leave release-note selection and attribution to the release process. This is the still-unfixed P3 finding from the prior review cycle.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against dcbea5cf10a8.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. Exact-head terminal evidence shows a real Daytona lifecycle plus the real pinned SDK’s serialized timeout and context-cancellation behavior; redact any private identifiers if the evidence is reposted.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Exact-head terminal evidence shows a real Daytona lifecycle plus the real pinned SDK’s serialized timeout and context-cancellation behavior; redact any private identifiers if the evidence is reposted.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This fixes direct Daytona command timing with a bounded provider-specific blast radius.
  • merge-risk: 🚨 availability: An error in timeout or cancellation handling could prematurely stop or indefinitely retain direct Daytona commands.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Exact-head terminal evidence shows a real Daytona lifecycle plus the real pinned SDK’s serialized timeout and context-cancellation behavior; redact any private identifiers if the evidence is reposted.
  • proof: sufficient: Contributor real behavior proof is sufficient. Exact-head terminal evidence shows a real Daytona lifecycle plus the real pinned SDK’s serialized timeout and context-cancellation behavior; redact any private identifiers if the evidence is reposted.

Evidence

What I checked:

  • Current-main gap: Current main still calls the Daytona process service directly for no-sync setup and the user command, without a caller-budget wrapper. (internal/providers/daytona/backend_run.go:133, dcbea5cf10a8)
  • Proposed timeout boundary: The PR creates a runner that removes the client-wide timeout, derives the remote timeout from the context deadline, caps it to the SDK limit, and routes all three direct process calls through it. (internal/providers/daytona/backend_run.go:26, e8d9e57b73fb)
  • SDK contract exercised: The focused regression constructs the pinned Daytona SDK process service, observes its serialized HTTP requests, covers no deadline, a 90-minute deadline, fractional rounding, the signed-32-bit cap, expiry, and cancellation. (internal/providers/daytona/backend_run_test.go:27, e8d9e57b73fb)
  • Dependency signal: The changed provider imports and tests the existing Daytona SDK and toolbox API clients; the repository pins SDK v0.190.0, so its request behavior is directly relevant to this patch. (go.mod:25, e8d9e57b73fb)
  • Real behavior proof: The PR body supplies exact-head terminal output for a real Daytona sandbox lifecycle and for the actual SDK request serialization/cancellation boundary; it explicitly distinguishes the hosted SSH smoke from the local direct-SDK trace. (e8d9e57b73fb)
  • History and release status: The relevant Daytona path has recent work from Peter Steinberger and Vincent Koc; the proposed head is not contained by a tag or branch, while v0.46.0/current main still lack this wrapper. (internal/providers/daytona/backend_run.go:133, dcbea5cf10a8)

Likely related people:

  • steipete: Feature history shows the most contributions in the Daytona provider area, including recent current-main lifecycle and ownership work and the PR’s timeout follow-up. (role: major area contributor; confidence: high; commits: 8ba71f913bbe, 8106f4a21547, e8d9e57b73fb; files: internal/providers/daytona/backend_run.go, internal/providers/daytona/client.go)
  • Vincent Koc: Introduced adjacent Daytona cleanup and timing behavior that shares the command lifecycle boundary. (role: adjacent provider contributor; confidence: medium; commits: f389d0ee3b97, eb46726bf8fc; files: internal/providers/daytona/backend_run.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Remove the release-owned CHANGELOG.md entry.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)
  • reviewed 2026-08-21T15:15:16.749Z sha 834d085 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-23T15:54:44.480Z sha 65d521a :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-24T06:46:30.091Z sha e8d9e57 :: needs real behavior proof before merge. :: [P3] Remove the release-owned changelog entry

@steipete

Copy link
Copy Markdown
Contributor

Maintainer follow-up pushed at 65d521ac.

The original fixed 60-minute process timeout and 61-minute HTTP timeout still truncated valid longer workloads. The provider now derives Daytona's remote timeout from the caller's actual context deadline, rounds fractional seconds upward, and uses the maximum supported 32-bit duration when the caller has no deadline. Toolbox HTTP requests remain governed by their request context instead of a second hard wall-clock limit.

Verification:

  • go test ./internal/providers/daytona -run '^TestDaytonaCommandRunnerPreservesCallerExecutionBudget$' -count=1
  • go test -race ./internal/providers/daytona
  • go vet ./internal/providers/daytona
  • go build -trimpath -o /private/tmp/crabbox-t30-daytona ./cmd/crabbox
  • Structured project autoreview: clean, with no accepted/actionable findings.

The regression covers an unbounded caller, a 90-minute deadline, and upward rounding of a 1.5-second deadline. Existing live Daytona create/use/destroy proof from this PR still applies to the transport path; exact-head CI should complete before landing.

@steipete
steipete force-pushed the rebattery/daytona-exec-timeout-simple branch from 65d521a to e8d9e57 Compare August 24, 2026 06:42
@steipete

Copy link
Copy Markdown
Contributor

Maintainer verification for exact head e8d9e57b73fb83b337df2db73018466d0df5b46f is now in the updated PR description.

One real Daytona sandbox was provisioned, executed successfully, and released:

daytona-live-start
daytona-live-ok
{"provider":"daytona","runStatus":"succeeded","exitCode":0,"commandMs":31581,"totalMs":71052}
post-cleanup provider inventory: 0 leases
post-cleanup task-created local claims: 0

The pinned Daytona SDK's actual serialized process request and cancellation behavior were independently observed on the same head:

caller deadline=90m remote timeout=5400s HTTP client timeout=0s
caller deadline=1.5s remote timeout=2s
caller deadline=900ms remote timeout=1s
caller deadline exceeds int32 remote timeout=2147483647s
expired caller deadline rejected before a remote request was sent
caller cancellation stopped in-flight Daytona HTTP request; HTTP client timeout=0s

The hosted smoke used the available coordinator-managed SSH path; the SDK request trace used a local instrumented HTTP server. A hosted direct-SDK run longer than one hour is not claimed because no direct Daytona credential, authenticated CLI profile, or direct-mode snapshot is available. Provider race tests, repository-wide vet, CLI build, the complete docs gate, and project autoreview all passed. Contributor authorship and co-author credit are preserved.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 24, 2026

@steipete steipete left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved after verifying exact head e8d9e57b73fb83b337df2db73018466d0df5b46f against the complete green CI matrix, the cleanup-verified hosted Daytona lifecycle, and the pinned SDK's actual serialized 5400-second process request plus caller-context cancellation.

The Unreleased changelog entry thanking @arisylafeta is intentional and must remain: this is a user-visible contributor fix, and repository guidance assigns changelog selection and contributor attribution to the maintainer. The automated suggestion to remove that maintainer-added entry conflicts with that requirement and is rejected.

The hosted smoke exercised coordinator-managed SSH, while the direct SDK timeout and cancellation were proven against an instrumented local HTTP endpoint. No hosted direct-SDK execution longer than one hour is claimed.

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

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants