Agent steps need a time limit: one f.agent can spend a flow's whole budget - #606
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
Relayflow: the adversarial review did not pass. This branch is not approved: the flow stopped here and did not mark it ready to merge. Review of PR #606Reviewed head: Changes requested: one P2 finding remains. P2 — Preserve reported usage when a wrapper reaches its deadlineLocation: The newly recoverable timeout goes through The reproduction emits the same complete result envelope (100 input tokens, 20 output tokens), then delays exit. With no timeout its charge is Preserve validated usage independently of the output returned on timeout; the author-facing timeout may still have no output/artifacts. Where usage was never reported, do not describe missing measurement as a measured zero. Add a regression covering an emitted usage envelope followed by a hung process and assert the timeout completion's journaled charge, including on resume. Reproduction source: agent-timeout-usage.mjs. node review-evidence/agent-timeout-usage.mjsCaptured output: Coverage and commentsInspected validation/lowering, SDK/kernel spec parity, process termination, authored continuation, named/predicate gates, index-write failure behavior, workspace preservation, and timeout replay. The documented resolve-and-branch interface is an explicit choice allowed by the request; it does not throw an exception for the author to catch. The live tests exercise that chosen interface and kill/resume behavior. They preserve a workspace file, rather than making an actual Git commit. Read the PR conversation and reviews with: gh pr view --json number,url,baseRefName,headRefOid,comments,reviews
gh api repos/AgentWorkforce/flows/pulls/606/comments --paginateThe first response identified the head above, one CodeRabbit conversation comment, and The PR's existing evidence includes a failing real-Claude analyzer run and a reported baseline reproduction. Those are author-supplied evidence, not a provider acceptance test independently re-executed by this review. No mutation-verification claim is made. Commands and captured outputAll commands ran from the repository root unless a working directory is specified. Complete build and initial-failure outputs are retained alongside this report:
The first SDK test invocation below failed three live tests because the daemon executable did not exist. Its complete output is tests.txt. After building the daemon, the same command produced the following output. This is a focused test run, not the entire repository suite. Working directory: npx vitest run tests/agent-timeout.test.ts tests/agent-timeout-outcome.test.ts tests/agent-timeout-worker.test.ts tests/agent-timeout-live.test.ts tests/authored-node-result.test.ts tests/spec-parity.test.tsnpm run typecheck --prefix packages/sdknpm run typecheck:tests --prefix packages/sdknpm run typecheck:regressions --prefix packages/surfacesh ops/cargo.sh test --manifest-path kernel/Cargo.toml -p relayflowd-core --test spec_parity |
|
The review's P2 (usage lost on timeout) is being addressed by fleet agent |
A wrapper that emitted a valid result envelope and then hung past its
execution timeout lost its token usage: the timeout result dropped captured
stdout, so the step journaled a priced $0. The timeout now keeps what the
wrapper wrote for usage decoding while the step output stays empty, and
usage that was never reported is journaled as dollar-unmetered instead of
a measured $0.
Regression: an envelope (100/20 tokens) followed by a hung process now
journals budget {100, 20, $0.001000} on the live run and after kill+resume.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Addressed the review's P2 (preserve reported usage when a wrapper reaches its deadline) in e248802. Fix
Regression (red on 249aa6a, green on e248802)
Other
|
There was a problem hiding this comment.
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 e248802. Configure here.
…e armed Every agent `timeout` now arms an execution deadline, and startDrainGrace returned early whenever one was set. A wrapper that exited 0 while a descendant held its stdio therefore waited out the full deadline and was journaled completionReason 'timeout'. The post-exit drain now arms whenever the wrapper has exited and acknowledged, regardless of the deadline, and the deadline callback hands an already-exited wrapper to the drain instead of reporting a timeout. The deadline still bounds a wrapper that is running. Regression: wrapper exits 0, a descendant keeps the pipe open, an 8s timeout is declared; it settles from the drain in well under the deadline with exit 0 and no timeout cause. The two worker-cli leak tests that pinned the old timeout classification now pin the drain settle. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…#139) * fix(flows): stop long agent steps at their FLOW_TIME allowance (#138) Pass the FLOW_TIME allowances as hard f.agent timeouts (relayflows 2.0.40, AgentWorkforce/flows#606) on check-repair, the adversary reviews, the fixer and check-discovery, and handle completionReason "timeout" explicitly on each: a repair is a failed attempt (re-check, no further repair), a review is unresolved (never clean), a fixer's work is kept and checked, and a discovery falls back to the ecosystem default. Only the cloud target states limits; the local kit's pinned 2.0.26 refuses the option. The budget sweep now runs the long agents past their limits and charges each exactly its limit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Session-Id: 27242e9e-7fb5-448f-a698-8fa8e9644610 * fix(flows): clear review.md before each review; catch non-literal agent timeouts in tests Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Session-Id: 27242e9e-7fb5-448f-a698-8fa8e9644610 --------- Co-authored-by: agentrelaybot <agentrelaybot@agentrelay.dev>

Add recoverable agent execution timeouts
An optional repair agent could consume a flow's remaining wallclock budget and
prevent publishing.
f.agent(..., { timeout: '45m' })now stops its CLI processgroup at the declared deadline and journals
completionReason: 'timeout'.Following the reviewed plan, this resolves with an
AgentResultcontainingcompletionReason: 'timeout'; authors branch on that result to continue. Itdoes not throw a catchable exception, because caught step failures otherwise
poison the authored operation and budget. Omission retains unlimited execution.
35% headroom. YAML uses integer
timeoutMs; the journal usestimeout_ms.The kernel checks positive/i64 bounds; it does not enforce the SDK ceiling.
Only a wrapper execution deadline receives timeout evidence; protocol errors
remain failures. Workspace edits and commits are not reset. Resume replays
the timed-out child without dispatching it again.
index writes still fail closed. IPC acceptance requires an explicit timeout
declaration, settled timeout evidence, and the matching failed child state.
because their producer timed out still fail the operation. The failed child
remains visible as failed while the authored flow can finish successfully;
SURFACE.mdexplicitly amends its caught-failure rule for this exception.Reviewed-plan choices: refuse
maxIterations > 1with a timeout (B2 option b)and refuse relay transport, rather than declare a bound those paths cannot
honor. Transport recovery before timeout can still start a fresh CLI timer.
Keep
artifacts: []on timeout (C2 option i), documenting the native journal'sbounded artifact paths and the wrapper transcript gap. No kernel failure-output
change. The trailing
runAgentCliargument preserves existing positionalcallers. Authored validation reuses
agent_cli_unresolved.This deliberately extends the former deterministic-only
timeoutMsrule toagents while keeping timeout fields per verb; LLM declarations remain unchanged.
Agent validation includes a maximum, unlike deterministic
timeoutMs. Olderdaemons reject the new field and older workers do not enforce it, so deploy
matching versions. No dollar-budget enforcement, Garden generator changes,
wrapper default cap, or workflow-file changes are included.
Verification commands and full captured output, including the initial failures
and their environment diagnosis, are in the evidence index.
Final SDK regression output (command and full output):
This includes real CLI termination, retained workspace content, following
f.rununder a budget header, predicate-gate handling, root/daemon kill and resume,
undeclared timeout refusal, failed journal indexing, IPC forgery rejection,
option validation/lowering, and existing lease/process-group/wrapper regressions.
Kernel command:
sh ops/cargo.sh test --manifest-path kernel/Cargo.toml -p relayflowd-core(full captured output):
Schema command:
npm test --prefix packages/schema(full captured output):
SDK source and test typechecks have no diagnostics; their literal commands and
captured output are linked in the evidence index. This work is not described as
mutation-verified.
Broader live-kernel command (changed code, isolated checkout to avoid the parent
CommonJS package scope):
Captured output:
The remaining real Claude analyzer test reports
execution/failinstead ofjson_schema/pass. It also fails against the originalc88c3d0surface/SDKusing the same daemon:
Captured original-head output:
That existing live-provider failure remains unresolved; no gate was weakened or
changed to make it pass.
Checks
The checks fail on the base commit too, so these failures were not introduced by this change: they come from the repository itself or from the environment the checks ran in. This pull request is a draft until someone looks.
What ran (.relayflow/check.sh)
Output on this branch (last 80 lines)
Output on the base commit (last 80 lines)
What the repair agent found
Check repair notes
.relayflow/check.shon this branch died before running a single test, and twofurther failure clusters appeared once it got past that. Three of the four causes
were setup differences between this machine and CI and are fixed in
.relayflow/check.sh(uncommitted, as instructed). One is outside my control andis documented here with what I tried.
Nothing on the branch was changed to make a check pass: no test was skipped,
deleted or weakened, and the branch's own commit (
243c37a, agent steptimeouts) is untouched.
State after the fixes
sh .relayflow/check.shnow runs the whole gate. Before: it exited after thebwrappreflight, with no test run at all. After the preflight fix but beforethe other three, the SDK suite reported:
With all four fixes:
Green, in order:
scripts/surface-package-gate.sh(surface build, suite,regression typecheck, pack, packed-consumer runtime + typecheck, and
tests/authored-flow.test.ts),node --test scripts/cloud-artifact.test.mjs,node --test scripts/publish.test.mjs,cargo build --locked --release -p relayflowd,cargo test --workspace(33 result groups, 0 failures), the SDKtypecheck,buildandtypecheck:tests, and the SDK suite apart from thebwrapcluster below.set -estops the script at the SDK suite, so the three stages after it neverran in either attempt. I ran them by hand, with the same commands and
environment the script uses:
The committed
packages/schema/flows.schema.jsonis current for this branch'stimeout_msaddition: regenerating it leaves no diff.The remaining 24 failures are all the same cause, below. The gate's exit status is
still 1 because of them.
Fixed in
.relayflow/check.sh1.
bwrappreflight aborted the whole gate (set -e)The original script ended its dependency section with
which fails here (see "Not fixable" below). Under
set -ethat killed the runbefore the surface gate, the kernel or any test — the state the task found. The
preflight now reports the result and continues, so the rest of the gate produces
signal instead of one line of output.
2.
bunwas 1.3.6; CI pins 1.4.0packages/sdk/tests/authored-node-runtime.test.ts:18asserts the exact pinnedversion in
beforeAll, so the whole file errored out as a failed suite withits 16 tests skipped:
CI gets 1.4.0 from
oven-sh/setup-bun@v2(bun-version: "1.4.0"incloud-runtime-artifact.yml,surface-package.yml,schema-publish.ymlandpublish.yml). This machine'sbuncomes from nvm at 1.3.6. The script nowinstalls the pin to
~/.bun/binand puts it first onPATH.3. An ancestor
"type": "commonjs"silently broke every extensionless ESM fixtureSeven
tests/live-kernel.test.tscases failed with an agent step that producednothing at all:
Cause: the fixtures in
testdata/preflight/are extensionless filescontaining ESM (
analyze-story-stub-cliline 3 isimport { receiveWrapperRequest } from './wrapper-session.mjs';). Node decidessuch a file's module kind from the nearest ancestor
package.json. This repo hasno
package.jsonat its root, so the lookup walks out to/home/daytona/package.json, which declares"type": "commonjs"— and anexplicit
commonjsdisables Node's module-syntax detection. The fixture thenproduces no output and exits 0, with nothing on stderr. CI checks out with no
package.jsonabove the repo at all, so detection applies there.Measured, with one
import { platform } from "node:os"; console.log(...)probein an extensionless file and only the ancestor
package.jsonvarying:The script now writes an empty, type-less
package.jsonin the directoryabove the checkout, which restores CI's resolution, and then proves it by
executing the fixture and failing if its stdout is empty. It is written outside
the repository, so
git statusstays clean.4.
ENOSPCon a 10GB filesystem (a disk adaptation, not a CI step)With bun pinned,
authored-node-runtime.test.tsactually ran — and 12 of its 16tests then died on disk:
The file copies the 145MB surface tree once per fixture (16 fixtures, ~2.3GB).
The filesystem is 10GB and was at 93% (730MB free).
cargo test --workspaceleaves 3.6GB of debug test binaries in
kernel/target/debugthat nothing belowreads — the SDK suite gets the release binary through
RELAYFLOWD_BIN— so thescript now removes that directory after the kernel step (
cargo testrebuildsit on the next run) and prints
df. That took free space from 730MB to 6.0GB.I also deleted, outside the repo and outside
check.sh, regenerable scratchleft behind by earlier runs:
/home/daytona/.relayflows-toolchain/target(1.5GBof
ops/cargo.shbuild output, which that script's own header documents asdeliberately disposable and outside the tree) and the
/tmp/agent-timeout-*worktrees from the previous agent's evidence gathering. The captured evidence
those runs produced is committed under
evidence/agent-timeout/and isunaffected. Nothing committed was touched.
Not fixable here:
bwrapcannot create a sandbox on this machine24 tests across four files fail, all of them requiring a working bubblewrap
sandbox:
tests/hosted-extension-isolation.test.tstests/hosted-extension-protocol.test.tstests/software-garden-babysitter-composition.test.tstests/babysitter-native-extension.test.tsEach one names the cause in its own failure message — the sandbox child never
starts, and the reason it gives is the namespace refusal, not anything the flow
did:
That string appears 19 times in the SDK run's output. Reproduced directly:
Unprivileged user namespaces are restricted by AppArmor and the knob cannot be
relaxed, because
/proc/sysis read-only here even for root:CI relaxes exactly this sysctl on its ephemeral runner, which is why the tests
pass there. The usual fallback for a host without unprivileged user namespaces —
installing
bwrapsetuid root — is not available either, because Debian buildsit without setuid support:
(I restored the mode bit afterwards:
/usr/bin/bwrapis-rwxr-xr-x root root.)bwrapdoes work undersudo, but these testsexec/usr/bin/bwrapdirectlyfrom the product code path, so there is nothing to point at a wrapper without
changing the tests — which I did not do.
Three of the four files guard on
existsSync('/usr/bin/bwrap')and would haveskipped had
check.shnot installed bubblewrap. I left the install in place:uninstalling a dependency to turn 24 red tests into silent skips would hide a
real gap in this machine's coverage rather than report it. The two
hosted-extension-*files have no such guard and would fail either way.These 24 failures are not reachable by the branch's change: the sandbox child
exits before any flow step runs, and the error it reports is
bwrap's ownrefusal to create the namespace (quoted above). The agent-timeout commit adds no
hosted-extension or sandbox code — its diff is
packages/surface/src/context.ts,packages/sdk/src/{compile,validate,spec,step-fields,worker,worker-cli, wrapper-session,authored-worker-step,authored-node-runner}.tsandkernel/relayflowd-core/src/spec.rs. I did not re-run these four files againstthe base commit, so I am not claiming a measured before/after comparison — only
that the failure each one reports is the unavailable namespace.
Fixes #604
Summary by cubic
Adds an optional per-step time limit for agent steps so a single agent can't consume a flow's whole wallclock budget.
f.agent(..., { timeout: '45m' })stops the agent's CLI process group at the deadline and resolves withcompletionReason: 'timeout'rather than throwing, letting authors branch on that result; without a declared timeout, execution remains unlimited.maxIterations > 1with a timeout; resume replays the timed-out child without re-dispatching it.timeout_msand older workers don't enforce it.The GitHub checks fail on the base commit too, so those failures are pre-existing and not introduced by this change; the remaining live-provider analyzer failure also reproduces on the original head.
Written for commit 065029c. Summary will update on new commits.
Note
Medium Risk
Changes agent execution, journaling, budget accounting, and authored failure semantics (timeout resolves while the child run fails); version skew on daemon/worker can leave timeouts unenforced or refused.
Overview
Adds an optional per-agent execution deadline so a long-running
f.agentcannot consume an entire flow wallclock budget. Authors passtimeoutonf.agent(samems/s/msyntax asf.run, 60m SDK ceiling); YAML/JSON uses integertimeoutMs, lowered to journaltimeout_mswith kernel positive/i64 validation.Runtime behavior: the worker arms the deadline on native Claude/Codex and wrapper paths (shared process-group stop). On deadline the child run journals
completionReason: 'timeout'and stays failed in the kernel, but the authored call resolves withAgentResult.completionReason: 'timeout'(not a catchable step failure). Predicate gates still run; relay transport andmaxIterations > 1with a timeout are refused at compile/admission.Related adjustments:
AgentResultalways carriescompletionReason; wrapper sessions distinguish “still running” timeouts from post-exit drain when a deadline is set; usage reported before a wrapper hang is still priced, otherwise spend is unmetered; IPC verification accepts durably settled agent timeouts only whentimeout_mswas declared.SURFACE.md, schema, fixtures, and regression tests document and pin the contract.Reviewed by Cursor Bugbot for commit 065029c. Bugbot is set up for automated code reviews on this repo. Configure here.