Skip to content

schedule: honor runner budgets for manual fires - #354

Merged
mostlydev merged 4 commits into
masterfrom
issue-348-manual-fire-timeouts
Aug 3, 2026
Merged

schedule: honor runner budgets for manual fires#354
mostlydev merged 4 commits into
masterfrom
issue-348-manual-fire-timeouts

Conversation

@mostlydev

Copy link
Copy Markdown
Owner

Summary

  • make claw api schedule fire pass a two-minute-five-second request budget to claw-api local client mode
  • give the outer compose-exec transport two minutes ten seconds while preserving the explicit --exec-timeout override
  • keep all non-fire governance requests on their existing short defaults
  • centralize the longest supported runner wake budget so the server and synchronous operator client cannot drift independently
  • document the synchronous behavior under Unreleased and in the embedded operator skill

TDD

The first focused test failed because the generated docker compose exec command had no -request-timeout; after implementation it proves both the inner request and outer transport budgets. Separate coverage proves list/get-style calls retain the 15-second outer default and an explicit --exec-timeout still wins.

Verification

  • go test -count=1 ./...
  • go vet ./...
  • go build -o bin/claw ./cmd/claw
  • go test -tags integration -count=1 ./...
  • go generate ./cmd/claw/...
  • git diff --check

Closes #348

Centralizing the longest supported wake budget only helps if something
enforces that it is actually the longest. Nothing did: wakeExecTimeout()
could hand any adapter a budget above schedulepkg.MaxWakeExecTimeout and
the manual fire client, which sizes its request and transport budgets
from that constant, would silently go back to cutting a healthy wake
short -- the exact failure #348 exists to fix. The risk is immediate,
not hypothetical: #346 adds hermes-exec as its own literal.

Adds an invariant test over every adapter resolveWakeAdapter can emit,
plus a nesting assertion so tuning the 5s margins cannot invert the
transport > request > wake ordering. Verified the guard actually bites
by temporarily giving hermes-exec a 3-minute budget, which fails it with
the drift named explicitly.

go test ./... , go vet ./... , git diff --check, and the integration-tag
suite all pass.
@mostlydev
mostlydev force-pushed the issue-348-manual-fire-timeouts branch from af9bbed to 9ca73ea Compare August 3, 2026 00:58
@mostlydev

Copy link
Copy Markdown
Owner Author

Independent adversarial review + rebase onto current master (which now contains #349). Pushed as 9ca73ea. Verified rather than trusted: go test -count=1 ./..., go vet ./..., git diff --check, and the integration-tag suite all pass; go generate ./cmd/claw/... produced no diff, so the embedded skill mirror is in sync.

Rebase. One conflict, in site/changelog.md — both #349 and this PR add an ## Unreleased bullet. Resolved by keeping both, #347 first then #348, since they describe different fixes. No pins, badges, or nav touched.

Taking your four challenges in order.

Shared constant placement — correct, but it was unenforced, and I fixed that. internal/schedule is the right home: both cmd/claw and cmd/claw-api already import it, so neither owns it. The problem was that MaxWakeExecTimeout is a promise nothing kept. wakeExecTimeout() could return a budget above it and the manual fire client, which sizes both its budgets from that constant, would silently go back to cutting a healthy wake short — the exact failure this PR exists to fix. That is not hypothetical: #346 adds hermesWakeExecTimeout as its own literal 2 * time.Minute, so the moment both land, the adapter that motivated all of this is outside the centralization. Added an invariant test over every adapter resolveWakeAdapter can emit (openclaw / hermes / nanobot / picoclaw / nullclaw). I did not trust it to work — I temporarily gave hermes-exec a 3-minute budget and confirmed it fails with the drift named, then restored. Recommend #346 change its constant to schedulepkg.MaxWakeExecTimeout rather than a literal when it rebases.

The 5s margins — sound, and I made the ordering structural. 2m0s wake / 2m5s request / 2m10s transport is correctly nested, and the outer margin has to cover docker compose exec startup before the inner request even begins, which on a loaded host is the thin part. I did not change the values — 5s is defensible and you have the operational context — but I replaced trust in the literals with an assertion that transport > request > max wake, so tuning one margin later cannot silently invert the nesting.

Fire-path detection — acceptable, with one caveat I would not block on. Re-parsing a path that runScheduleRequest constructed 40 lines earlier is indirection, and threading an explicit budget from the caller would be cleaner. But the surface is small, the guard is strict (exactly three segments, non-empty id, POST only), and both directions are tested, so I would not churn it. The caveat: cmd/claw/api.go:124 builds the path with strings.TrimSpace(args[0]) and no url.PathEscape, while cmd/clawdash/schedule_page.go does escape. An id containing a slash would produce four segments, fall back to the 15s budget, and reintroduce this bug — though it would also 404 server-side, so the request is already broken and this is pre-existing rather than something you introduced. Worth a follow-up, not a blocker here.

--exec-timeout precedence — correct, and the default change is the right call. Moving the flag default from 15s to 0 is what makes operation-specific budgets possible at all, an explicit value still wins for both fire and non-fire calls, and the help text explains the sentinel. One asymmetry to be aware of: an explicit --exec-timeout shorter than the fire request budget will cut the outer transport while the inner -request-timeout stays at 2m5s. That is the operator overriding deliberately, so I would leave it, but it is worth a sentence in the flag help if you feel strongly.

No correctness bug found. Marking ready for review.

@mostlydev
mostlydev marked this pull request as ready for review August 3, 2026 00:59
@mostlydev

Copy link
Copy Markdown
Owner Author

Added the literal delayed-response gate requested by #348 in 81472f6. An integration-tagged fake Docker transport sleeps 16 seconds—past the former 15-second outer deadline—and runClawAPIComposeCommandDefault still returns its final JSON under the fire-specific budget. Verified with go test -tags integration -run '^TestScheduleFireTransportOutlivesOldFifteenSecondDefault$' -count=1 ./cmd/claw (16.587s).

@mostlydev
mostlydev merged commit 9a39ba6 into master Aug 3, 2026
2 checks passed
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.

Let manual schedule fire honor runner wake budgets

1 participant