diff --git a/cmd/claw-api/client_timeout_integration_test.go b/cmd/claw-api/client_timeout_integration_test.go new file mode 100644 index 0000000..8fe9d5b --- /dev/null +++ b/cmd/claw-api/client_timeout_integration_test.go @@ -0,0 +1,42 @@ +//go:build integration + +package main + +import ( + "bytes" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + schedulepkg "github.com/mostlydev/clawdapus/internal/schedule" +) + +func TestManualFireRequestOutlivesOldFifteenSecondDeadline(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(16 * time.Second) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer srv.Close() + + principalsPath := writePrincipalsFixture(t, `{"principals":[{"name":"claw-scheduler","token":"capi_sched","verbs":["schedule.control"],"pods":["ops"]}]}`) + cfg := config{ + Addr: strings.TrimPrefix(srv.URL, "http://"), + PrincipalsPath: principalsPath, + } + + started := time.Now() + var stdout bytes.Buffer + err := runLocalRequest(cfg, &stdout, http.MethodPost, "/schedule/test/fire", "", "claw-scheduler", schedulepkg.ManualFireRequestTimeout) + if err != nil { + t.Fatalf("manual fire request failed after the old deadline: %v", err) + } + if elapsed := time.Since(started); elapsed <= 15*time.Second { + t.Fatalf("delayed response returned in %v; test must cross the old 15s deadline", elapsed) + } + if !strings.Contains(stdout.String(), `"ok": true`) { + t.Fatalf("unexpected delayed response: %q", stdout.String()) + } +} diff --git a/cmd/claw-api/scheduler.go b/cmd/claw-api/scheduler.go index fd9f883..553ed12 100644 --- a/cmd/claw-api/scheduler.go +++ b/cmd/claw-api/scheduler.go @@ -71,7 +71,7 @@ type dispatchOptions struct { } const defaultWakeExecTimeout = 30 * time.Second -const openclawWakeExecTimeout = 2 * time.Minute +const openclawWakeExecTimeout = schedulepkg.MaxWakeExecTimeout var errScheduleInvocationInFlight = errors.New("schedule invocation already in flight") diff --git a/cmd/claw-api/wake_budget_test.go b/cmd/claw-api/wake_budget_test.go new file mode 100644 index 0000000..c380cf8 --- /dev/null +++ b/cmd/claw-api/wake_budget_test.go @@ -0,0 +1,58 @@ +package main + +import ( + "testing" + "time" + + schedulepkg "github.com/mostlydev/clawdapus/internal/schedule" +) + +// knownWakeAdapters mirrors every adapter resolveWakeAdapter can emit in +// cmd/claw/schedule_manifest.go. Adding an adapter there without adding it here +// leaves its budget outside this invariant. +var knownWakeAdapters = []string{ + "openclaw-exec", + "hermes-exec", + "nanobot-exec", + "picoclaw-exec", + "nullclaw-exec", +} + +// schedulepkg.MaxWakeExecTimeout is what the synchronous operator client sizes +// its request and transport budgets against. If any adapter is allowed to run +// longer than it, `claw api schedule fire` goes back to being cut short by its +// own client while the server keeps waiting -- the exact failure #348 fixed. +// Nothing but this test holds the constant to its name. +func TestEveryWakeAdapterFitsMaxWakeExecTimeout(t *testing.T) { + for _, adapter := range knownWakeAdapters { + if got := wakeExecTimeout(adapter); got > schedulepkg.MaxWakeExecTimeout { + t.Errorf("adapter %s wake budget %v exceeds MaxWakeExecTimeout %v; the manual fire client cannot cover it", + adapter, got, schedulepkg.MaxWakeExecTimeout) + } + } +} + +// The margins exist so the outer transport outlives the inner request, which in +// turn outlives the longest wake. Assert the ordering rather than the literals, +// so tuning a margin cannot silently invert it. +func TestManualFireBudgetsAreStrictlyNested(t *testing.T) { + if schedulepkg.ManualFireRequestTimeout <= schedulepkg.MaxWakeExecTimeout { + t.Fatalf("manual fire request budget %v must exceed the longest wake %v", + schedulepkg.ManualFireRequestTimeout, schedulepkg.MaxWakeExecTimeout) + } + if schedulepkg.ManualFireTransportTimeout <= schedulepkg.ManualFireRequestTimeout { + t.Fatalf("manual fire transport budget %v must exceed the request budget %v", + schedulepkg.ManualFireTransportTimeout, schedulepkg.ManualFireRequestTimeout) + } +} + +// Adapters with no evidence that they need more keep the generic budget. +func TestUnknownAdapterKeepsDefaultWakeBudget(t *testing.T) { + if got := wakeExecTimeout("some-future-exec"); got != defaultWakeExecTimeout { + t.Fatalf("expected default wake timeout %v, got %v", defaultWakeExecTimeout, got) + } + if defaultWakeExecTimeout > schedulepkg.MaxWakeExecTimeout { + t.Fatalf("default wake budget %v exceeds max %v", defaultWakeExecTimeout, schedulepkg.MaxWakeExecTimeout) + } + var _ time.Duration = defaultWakeExecTimeout +} diff --git a/cmd/claw/api.go b/cmd/claw/api.go index 8d283be..877e07a 100644 --- a/cmd/claw/api.go +++ b/cmd/claw/api.go @@ -10,6 +10,7 @@ import ( "strings" "time" + schedulepkg "github.com/mostlydev/clawdapus/internal/schedule" "github.com/spf13/cobra" "gopkg.in/yaml.v3" ) @@ -31,11 +32,17 @@ type composeServiceIndex struct { } const ( - httpMethodGet = "GET" - httpMethodPost = "POST" - clawAPIServiceName = "claw-api" + httpMethodGet = "GET" + httpMethodPost = "POST" + clawAPIServiceName = "claw-api" + defaultAPIExecTimeout = 15 * time.Second ) +type clawAPIRequestBudget struct { + requestTimeout time.Duration + execTimeout time.Duration +} + var apiCmd = &cobra.Command{ Use: "api", Short: "Call the in-pod governance API through docker compose exec", @@ -153,6 +160,10 @@ func callClawAPICompose(composePath, principalName, method, requestPath string, "-request-path", strings.TrimSpace(requestPath), "-request-principal", defaultAPIPrincipal(principalName), } + budget := clawAPIRequestBudgetFor(method, requestPath) + if budget.requestTimeout > 0 { + args = append(args, "-request-timeout", budget.requestTimeout.String()) + } if body != nil { raw, err := json.Marshal(body) if err != nil { @@ -162,13 +173,31 @@ func callClawAPICompose(composePath, principalName, method, requestPath string, args = append(args, "-request-body", string(raw)) } } - out, err := runClawAPIComposeCommand(args...) + execTimeout := budget.execTimeout + if apiExecTimeout > 0 { + execTimeout = apiExecTimeout + } + out, err := runClawAPIComposeCommand(execTimeout, args...) if err != nil { return nil, formatComposeOutputError("docker compose exec "+clawAPIServiceName, err, out) } return out, nil } +func clawAPIRequestBudgetFor(method, requestPath string) clawAPIRequestBudget { + budget := clawAPIRequestBudget{execTimeout: defaultAPIExecTimeout} + if !strings.EqualFold(strings.TrimSpace(method), httpMethodPost) { + return budget + } + path := strings.SplitN(strings.TrimSpace(requestPath), "?", 2)[0] + parts := strings.Split(strings.Trim(path, "/"), "/") + if len(parts) == 3 && parts[0] == "schedule" && parts[1] != "" && parts[2] == "fire" { + budget.requestTimeout = schedulepkg.ManualFireRequestTimeout + budget.execTimeout = schedulepkg.ManualFireTransportTimeout + } + return budget +} + func defaultAPIPrincipal(name string) string { name = strings.TrimSpace(name) if name == "" { @@ -195,10 +224,9 @@ func ensureComposeService(composePath, service string) error { return fmt.Errorf("service %q not found in compose.generated.yml", service) } -func runClawAPIComposeCommandDefault(args ...string) ([]byte, error) { - timeout := apiExecTimeout +func runClawAPIComposeCommandDefault(timeout time.Duration, args ...string) ([]byte, error) { if timeout <= 0 { - timeout = 15 * time.Second + timeout = defaultAPIExecTimeout } ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -212,7 +240,7 @@ func runClawAPIComposeCommandDefault(args ...string) ([]byte, error) { func init() { apiCmd.PersistentFlags().StringVar(&apiPrincipalName, "principal", "claw-scheduler", "Principal name inside claw-api principals.json to use for the request; not a host-side access boundary") - apiCmd.PersistentFlags().DurationVar(&apiExecTimeout, "exec-timeout", 15*time.Second, "Maximum time to wait for the docker compose exec transport") + apiCmd.PersistentFlags().DurationVar(&apiExecTimeout, "exec-timeout", 0, "Maximum time to wait for the docker compose exec transport (0 selects an operation-specific default)") apiSchedulePauseCmd.Flags().StringVar(&schedulePauseUntil, "until", "", "Pause until this RFC3339 timestamp instead of indefinitely") apiSchedulePauseCmd.Flags().StringVar(&schedulePauseReason, "reason", "", "Optional operator reason recorded with the pause") diff --git a/cmd/claw/api_test.go b/cmd/claw/api_test.go index ed1b2b4..c8df343 100644 --- a/cmd/claw/api_test.go +++ b/cmd/claw/api_test.go @@ -6,6 +6,7 @@ import ( "reflect" "strings" "testing" + "time" ) func TestCallClawAPIComposeBuildsExecCommand(t *testing.T) { @@ -17,7 +18,9 @@ services: prev := runClawAPIComposeCommand var gotArgs []string - runClawAPIComposeCommand = func(args ...string) ([]byte, error) { + var gotTimeout time.Duration + runClawAPIComposeCommand = func(timeout time.Duration, args ...string) ([]byte, error) { + gotTimeout = timeout gotArgs = append([]string(nil), args...) return []byte("{\"ok\":true}\n"), nil } @@ -41,6 +44,9 @@ services: if !reflect.DeepEqual(gotArgs, want) { t.Fatalf("unexpected args:\n got: %#v\nwant: %#v", gotArgs, want) } + if gotTimeout != defaultAPIExecTimeout { + t.Fatalf("expected default exec timeout %v, got %v", defaultAPIExecTimeout, gotTimeout) + } } func TestCallClawAPIComposeMarshalsBody(t *testing.T) { @@ -52,7 +58,7 @@ services: prev := runClawAPIComposeCommand var gotArgs []string - runClawAPIComposeCommand = func(args ...string) ([]byte, error) { + runClawAPIComposeCommand = func(_ time.Duration, args ...string) ([]byte, error) { gotArgs = append([]string(nil), args...) return []byte("{}\n"), nil } @@ -71,6 +77,61 @@ services: } } +func TestCallClawAPIComposeGivesScheduleFireAFullWakeRequestBudget(t *testing.T) { + composePath := writeComposeFixture(t, ` +services: + claw-api: + image: ghcr.io/mostlydev/claw-api:latest +`) + + prev := runClawAPIComposeCommand + var gotArgs []string + var gotTimeout time.Duration + runClawAPIComposeCommand = func(timeout time.Duration, args ...string) ([]byte, error) { + gotTimeout = timeout + gotArgs = append([]string(nil), args...) + return []byte("{}\n"), nil + } + defer func() { runClawAPIComposeCommand = prev }() + + if _, err := callClawAPICompose(composePath, "ops-admin", "POST", "/schedule/westin/fire", nil); err != nil { + t.Fatalf("callClawAPICompose: %v", err) + } + joined := strings.Join(gotArgs, " ") + if !strings.Contains(joined, "-request-timeout 2m5s") { + t.Fatalf("expected schedule fire to cover the two-minute runner wake budget, got %#v", gotArgs) + } + if gotTimeout != 2*time.Minute+10*time.Second { + t.Fatalf("expected outer transport to outlive the request budget, got %v", gotTimeout) + } +} + +func TestCallClawAPIComposeHonorsExplicitExecTimeoutForScheduleFire(t *testing.T) { + composePath := writeComposeFixture(t, ` +services: + claw-api: + image: ghcr.io/mostlydev/claw-api:latest +`) + + previousTimeout := apiExecTimeout + apiExecTimeout = 45 * time.Second + defer func() { apiExecTimeout = previousTimeout }() + prev := runClawAPIComposeCommand + var gotTimeout time.Duration + runClawAPIComposeCommand = func(timeout time.Duration, _ ...string) ([]byte, error) { + gotTimeout = timeout + return []byte("{}\n"), nil + } + defer func() { runClawAPIComposeCommand = prev }() + + if _, err := callClawAPICompose(composePath, "ops-admin", "POST", "/schedule/westin/fire", nil); err != nil { + t.Fatalf("callClawAPICompose: %v", err) + } + if gotTimeout != 45*time.Second { + t.Fatalf("expected explicit exec timeout to win, got %v", gotTimeout) + } +} + func TestCallClawAPIComposeRejectsMissingClawAPIService(t *testing.T) { composePath := writeComposeFixture(t, ` services: diff --git a/cmd/claw/api_timeout_integration_test.go b/cmd/claw/api_timeout_integration_test.go new file mode 100644 index 0000000..7d623b9 --- /dev/null +++ b/cmd/claw/api_timeout_integration_test.go @@ -0,0 +1,40 @@ +//go:build integration + +package main + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + schedulepkg "github.com/mostlydev/clawdapus/internal/schedule" +) + +func TestScheduleFireTransportOutlivesOldFifteenSecondDefault(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake docker transport uses a POSIX shell") + } + + binDir := t.TempDir() + dockerPath := filepath.Join(binDir, "docker") + shim := "#!/bin/sh\nsleep 16\nprintf '{\"ok\":true}\\n'\n" + if err := os.WriteFile(dockerPath, []byte(shim), 0o755); err != nil { + t.Fatalf("write fake docker transport: %v", err) + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + started := time.Now() + out, err := runClawAPIComposeCommandDefault(schedulepkg.ManualFireTransportTimeout, "compose", "exec", "claw-api") + if err != nil { + t.Fatalf("manual fire transport failed after the old default: %v (%s)", err, strings.TrimSpace(string(out))) + } + if elapsed := time.Since(started); elapsed <= defaultAPIExecTimeout { + t.Fatalf("fake response returned in %v; test must cross the old %v deadline", elapsed, defaultAPIExecTimeout) + } + if strings.TrimSpace(string(out)) != `{"ok":true}` { + t.Fatalf("unexpected delayed response: %q", string(out)) + } +} diff --git a/cmd/claw/skill_data/SKILL.md b/cmd/claw/skill_data/SKILL.md index 1123e14..4ededea 100644 --- a/cmd/claw/skill_data/SKILL.md +++ b/cmd/claw/skill_data/SKILL.md @@ -69,6 +69,9 @@ Lifecycle commands block if `claw-pod.yml` is newer than `compose.generated.yml` `claw api schedule ...` does not require a host-published claw-api port. It tunnels through `docker compose exec -T claw-api /claw-api -request-*`, so the pod must already be up and include an injected `claw-api` service. +`claw api schedule fire` waits synchronously for the final runner outcome and +therefore uses a longer operation-specific timeout. `--exec-timeout` overrides +the outer compose transport when an operator needs a different bound. Trust boundary: if you can run `docker compose exec` against the pod, you can select any principal present in claw-api's `principals.json`. The `--principal` diff --git a/internal/schedule/timeouts.go b/internal/schedule/timeouts.go new file mode 100644 index 0000000..fcedd66 --- /dev/null +++ b/internal/schedule/timeouts.go @@ -0,0 +1,11 @@ +package schedule + +import "time" + +// MaxWakeExecTimeout is the longest bounded runner wake that claw-api supports. +// Operator transports that synchronously wait for a final fire result must +// allow this budget plus their own request and process margins. +const MaxWakeExecTimeout = 2 * time.Minute + +const ManualFireRequestTimeout = MaxWakeExecTimeout + 5*time.Second +const ManualFireTransportTimeout = ManualFireRequestTimeout + 5*time.Second diff --git a/site/changelog.md b/site/changelog.md index d410d1a..5df2d78 100644 --- a/site/changelog.md +++ b/site/changelog.md @@ -30,6 +30,7 @@ outline: deep ## Unreleased - **Slow scheduled wakes no longer block unrelated targets** -- claw-api dispatches due targets concurrently while serializing wakes per runner, coalesces overlapping slots without regressing next-fire state, rejects duplicate manual fires with a conflict, and drains active scheduler dispatches cleanly on shutdown. Coalesced slots are now recorded in schedule state (`suppressed_slots`, `last_suppressed_at`) and surfaced on the clawdash schedule card, so a schedule whose wake outruns its own cadence no longer reads as perfectly healthy. Closes [#347](https://github.com/mostlydev/clawdapus/issues/347). +- **Manual schedule fires honor runner wake budgets** -- `claw api schedule fire` now gives the in-container request 2 minutes 5 seconds and its outer compose transport 2 minutes 10 seconds, enough to return the final result of the longest supported runner wake. Other schedule operations retain their short defaults, and an explicit `--exec-timeout` still overrides the outer transport. Closes [#348](https://github.com/mostlydev/clawdapus/issues/348). ## v0.27.0 {#v0-27-0} diff --git a/skills/clawdapus/SKILL.md b/skills/clawdapus/SKILL.md index 1123e14..4ededea 100644 --- a/skills/clawdapus/SKILL.md +++ b/skills/clawdapus/SKILL.md @@ -69,6 +69,9 @@ Lifecycle commands block if `claw-pod.yml` is newer than `compose.generated.yml` `claw api schedule ...` does not require a host-published claw-api port. It tunnels through `docker compose exec -T claw-api /claw-api -request-*`, so the pod must already be up and include an injected `claw-api` service. +`claw api schedule fire` waits synchronously for the final runner outcome and +therefore uses a longer operation-specific timeout. `--exec-timeout` overrides +the outer compose transport when an operator needs a different bound. Trust boundary: if you can run `docker compose exec` against the pod, you can select any principal present in claw-api's `principals.json`. The `--principal`