feat(surface,sdk,kernel): deterministic-step timeout option f.run with timeout (#343)

[kjgbot]

Closes #343. Delivered via implement-slice.flow.ts (flows-driven dogfood, mixed codex/claude).

---

Note

Medium Risk
Changes deterministic execution timing, spec validation, and lease journaling across SDK and kernel; misaligned builds could refuse specs or behave differently on long-running commands.

Overview
Adds per-command timeouts for authored f.run: optional { timeout: string | number } (default 30s, hard cap 15 minutes), documented in SURFACE.md.

The SDK parses durations (ms / s / m or raw ms) via parseStepTimeout, refuses bad values with timeout_invalid and over-cap with lease_exceeded before journal I/O, and lowers each call to a deterministic step carrying optional lease_ms. Journal timeout / lease_expired completions surface to authors as lease_exceeded.

The kernel accepts lease_ms only on deterministic steps (validated, fail-closed on llm/agent), journals the matching lease_deadline_ms, and the deterministic executor waits min(lease_ms, timeout_ms) when both are set—then kills the process group and records timeout on expiry.

^{Reviewed by Cursor Bugbot for commit eea6901. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 32f3c67e-98a1-4b75-99a5-93e292da4773

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b9627be. Configure here.}

[kjgbot]

maintainability lens — FAIL

Maintainability review — PR #350 (per-invocation deterministic lease)

Blockers

1. lease_ms breaks authoring-surface naming convention.
packages/sdk/src/spec.ts:174 adds lease_ms?: number to DeterministicStepSpec — but every sibling authoring field is camelCase (timeoutMs, maxIterations, dependsOn, recoveryMode; see spec.ts:153/155/173/219). The value even flows in as snake_case at authored-worker-step.ts:151 ({ …, lease_ms: leaseMs }) and out through camel-to-snake in compile.ts:574. A stranger reading DeterministicStepSpec six months from now will read lease_ms as an oversight and either "fix" it or paper over it with a rename. If snake_case is intentional (kernel-parity), say so where the field is declared. Otherwise rename to leaseMs and translate at toKernelStep.

Concerns

2. Three names for one concept, no doc pointer. Ctx.run (context.ts:35) takes { timeout }; the executor renames it to leaseMs (authored-flow-executor.ts:240); the spec calls it lease_ms. Nothing in SURFACE.md's new section (:429–443) tells a maintainer these are the same knob. Add a one-line pointer in Ctx.run's JSDoc or in authoredDeterministicRunner.

3. min(lease_ms, timeout_ms) is an implicit contract. exec_det.rs:97–101 silently takes the smaller of the two. SURFACE.md promises "the kernel kills the process group and journals completionReason: timeout" at the lease, so a maintainer will not know that setting timeout_ms below lease_ms shortens the effective deadline. Worse, the journaled lease_deadline_ms (machine.rs:283–289) reflects lease_ms alone — so debugging a "why did this die early?" involves reading two files. Either document the interaction, refuse the pair, or align the two.

4. Ceiling enforced in SDK only. SURFACE.md declares 15 min a "hard ceiling," but spec.rs:149–158 only checks >0 && ≤ i64::MAX. A YAML author bypassing the TS SDK — a documented capability — gets a 1 h lease with no complaint. Either enforce 900 000 in RunSpec::validate, or soften the doc.

5. Two error surfaces for one code. parseStepTimeout throws CompileError with kind: 'lease_exceeded' (compile.ts:64); readSuccessfulOutput throws AuthoredFlowExecutionError with code: 'lease_exceeded' (authored-step-output.ts:53–59). Same string, different classes, different property names — the tests at step-lease.test.ts:23 vs :80 have to key off different fields. A future reader chasing this code word will keep finding half of it.

6. Redundant re-parsing. parseStepTimeout runs at f.run (executor), then again in compileStep (compile.ts:174), then again in toKernelStep (compile.ts:574). Three enforcement points can drift; consolidate.

Notes

• every_reason_label_matches_its_serialized_form was not extended — good, no new journal reason was invented for lease_exceeded.
• verb-field-lint.test.ts:194–219 closes lease_ms on llm/agent; solid.
• The test authoredDeterministicRunner mutates options.timeout after f.run returns (step-lease.test.ts:60) — nice guard against snapshot-vs-live confusion.

REVIEW_FAILED

[kjgbot]

history lens — PASS

Blockers: none. The supplied diff passes the HISTORY lens.

Notes

• The lease change does not repeat the worker-expiry mistake recorded in ops/DRIVE-LOG.md:6383–6465 and resolved through withWorkerLease at 7118–7144. In kernel/relayflowd-core/src/machine.rs:283–290, only deterministic steps receive the optional override; other step types retain LEASE_DURATION_MS. Worker heartbeat renewal is untouched.
• kernel/relayflowd/src/exec_det.rs:97–104 preserves the existing command timeout when no lease override is supplied and selects the shorter limit when both exist. It retains the process-group termination path. This does not restore the previously removed bounded journal-request timeout documented in the drive log.
• No new contradiction with settled RFC decisions is apparent. The override crosses the spec boundary in packages/sdk/src/authored-worker-step.ts:147–159, and the kernel calculates the journaled deadline. This remains consistent with decisions regressions: red/green flows for the 2026-08-27 platform bugs (dormant until gates 2+6) #5 and drive: WP-13: Fix SDK test failures from sandbox environment gaps #13: spec/protocol composition and a closed kernel vocabulary.
• The matching commit, 484690d2, describes a deterministic-step timeout option across surface, SDK, and kernel. Its message makes no test-pass, mutation-verification, or evidence claims that contradict the supplied changes.

Concerns

• The documented 15-minute ceiling (docs/SURFACE.md:437–442) is enforced by SDK compilation (packages/sdk/src/compile.ts:57–71), while kernel validation accepts positive values through i64::MAX (kernel/relayflowd-core/src/spec.rs:149–161). Clarifying that enforcement boundary would help direct protocol consumers. This is not an identified HISTORY blocker.

This verdict covers the supplied diff, matching 484690d2. PR #350 now also contains parser follow-up eea6901b, which is outside this review. Tests were not executed; no runtime verification is claimed.

REVIEW_PASSED

[kjgbot]

structure lens — MISSING

[kjgbot]

🎯 review-swarm: FAILED (M:fail H:pass S:missing)

Lens transcripts posted as sibling comments above.