From d0159c7e42e8e6b4ed3f7b06d092d3c1fa7d1caf Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Sun, 4 Oct 2026 02:35:58 -0700 Subject: [PATCH 1/2] 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) Session-Id: 27242e9e-7fb5-448f-a698-8fa8e9644610 --- web/lib/flow-workflows.ts | 97 ++++++++++++++---- web/lib/test/flow-budget.test.ts | 148 ++++++++++++++++++++++++--- web/lib/test/flow-onboarding.test.ts | 11 +- web/lib/test/flow-workflows.test.ts | 129 +++++++++++++++++++++++ 4 files changed, 346 insertions(+), 39 deletions(-) diff --git a/web/lib/flow-workflows.ts b/web/lib/flow-workflows.ts index 4c028072..5f2c5ce5 100644 --- a/web/lib/flow-workflows.ts +++ b/web/lib/flow-workflows.ts @@ -277,12 +277,18 @@ export const FLOW_CHECK_BLOCKED_COMMAND = [ /** * The generated flow's time plan (AgentWorkforce/cloud#4108). The header's * wallclock is charged per step and checked before each one, so once it is - * spent no step can run, not even the push that keeps the work. Agent steps - * take no time limit of their own, so the flow reads the clock (a journaled - * `date +%s`) before each optional, expensive step and starts it only when - * that step's allowance and publishing still fit. The allowances cover the - * longest steps measured on 2026-10-01: a 14m check run (its lease is 15m) - * and repair agents of 37m and 44m. + * spent no step can run, not even the push that keeps the work. So the flow + * reads the clock (a journaled `date +%s`) before each optional, expensive + * step and starts it only when that step's allowance and publishing still + * fit. The allowances cover the longest steps measured on 2026-10-01: a 14m + * check run (its lease is 15m) and repair agents of 37m and 44m. + * + * Starting a step only when its allowance fits did not stop it there: a + * check-repair agent ran 38m / 592 turns / $54 (agentrelay.com#138). Since + * relayflows 2.0.40 (AgentWorkforce/flows#606) an agent step takes a hard + * `timeout`, at most agentLimitMaxMinutes, and the cloud flow states each + * long agent's allowance as its limit; see workflowCode for which agents + * have one, and for the local target. */ export const FLOW_TIME = (() => { const headerMinutes = 180; // Cloud's maximum run budget (RELAYFLOW_V2_RUN_BUDGET_MAX_MINUTES). @@ -291,6 +297,12 @@ export const FLOW_TIME = (() => { const repairMinutes = 45; const reviewMinutes = 20; const fixerMinutes = 45; + // Check discovery reads CI configuration and writes one script; it took + // 6m and 9m24s in bda21b91. A discovery stopped at its limit falls back to + // the ecosystem default, so a hard stop costs little. + const discoveryMinutes = 15; + // relayflows refuses an agent timeout above 60m (AgentWorkforce/flows#606). + const agentLimitMaxMinutes = 60; // A step with no timeout of its own gets the kernel's 30s default, and a // timed-out step throws: run ccbc27c8 lost 1h46m of finished work when its // push to a large repository took longer (agentrelay.com#135). So each step @@ -315,7 +327,7 @@ export const FLOW_TIME = (() => { // fix round's re-publish (5 + 12 + 0.5 + 2 = 19.5m) fits. const publishMinutes = Math.ceil(forgeMinutes + pushMinutes + forgeMinutes + 4 * defaultStepMinutes + closeMinutes); return Object.freeze({ - headerMinutes, setupMinutes, checkMinutes, repairMinutes, reviewMinutes, fixerMinutes, + headerMinutes, setupMinutes, checkMinutes, repairMinutes, reviewMinutes, fixerMinutes, discoveryMinutes, agentLimitMaxMinutes, forgeMinutes, pushMinutes, followUpMinutes, defaultStepMinutes, closeMinutes, publishMinutes, bodyMinutes: headerMinutes - setupMinutes, // A repair, its re-check, the base-commit check a still-failing check @@ -599,6 +611,7 @@ export const FLOW_PUBLISH_CHECK_COMMAND = [ ].join('; '); const REVIEW_BLOCKED_HEADING ='**Relayflow: the adversarial review did not pass.** This branch is not approved: the flow stopped here and did not mark it ready to merge.'; +const REVIEW_TIMEOUT_NOTE = `The last review was stopped at its ${FLOW_TIME.reviewMinutes}-minute limit before it finished, so it is not a clean review. What it wrote before then is below.`; /** * Puts the failed review where an operator acts on it: on the pull request. @@ -622,10 +635,14 @@ const REVIEW_BLOCKED_HEADING ='**Relayflow: the adversarial review did not pass. * review.md, an older `gh` without `pr ready --undo`, or a repository that * refuses drafts must not cost the run the report it is trying to leave behind. * Each branch prints which way it went, so the journal records the outcome. + * + * The caller may prefix `review_timeout=yes` when the last reviewer was + * stopped at its time limit (agentrelay.com#138); the report then says so, + * because whatever review.md holds is unfinished. */ export const FLOW_REVIEW_BLOCKED_COMMAND = [ 'set -e', - `{ printf '%s\\n\\n' "${REVIEW_BLOCKED_HEADING}"; if [ -s review.md ]; then cat review.md; else printf '%s\\n' "_The reviewer left no review.md; see the review step in the run journal._"; fi; } > review-blocked.md || true`, + `{ printf '%s\\n\\n' "${REVIEW_BLOCKED_HEADING}"; if [ "\${review_timeout:-}" = yes ]; then printf '%s\\n\\n' "${REVIEW_TIMEOUT_NOTE}"; fi; if [ -s review.md ]; then cat review.md; else printf '%s\\n' "_The reviewer left no review.md; see the review step in the run journal._"; fi; } > review-blocked.md || true`, 'echo "relayflow: the adversarial review did not pass; wrote review-blocked.md."', `if ${FLOW_DRAFT_CHANGE_COMMAND} >/dev/null 2>&1; then echo "relayflow: converted the pull request to a draft."; else echo "relayflow: could not convert the pull request to a draft; review-blocked.md still holds the findings." >&2; fi`, `if ${flowCommentChangeCommand('review-blocked.md')} >/dev/null 2>&1; then echo "relayflow: posted the unresolved review to the pull request."; else echo "relayflow: could not comment on the pull request; review-blocked.md still holds the findings." >&2; fi`, @@ -673,12 +690,25 @@ export function workflowAgents(selected: readonly string[]) { return { builder, reviewer, prototypes: [builder, reviewer, builder] }; } -export function workflowCode(workflow: WorkflowId, agents: ReturnType, instructions: string, _target: 'cloud' | 'local' = 'cloud', settings: FlowAgentSettings = {}, selected: readonly string[] = [agents.builder, agents.reviewer]) { +export function workflowCode(workflow: WorkflowId, agents: ReturnType, instructions: string, target: 'cloud' | 'local' = 'cloud', settings: FlowAgentSettings = {}, selected: readonly string[] = [agents.builder, agents.reviewer]) { const config = (role: AgentRole) => resolveGeneratedAgentSettings(workflow, role, selected, settings); - const options = (role: AgentRole, fallback: string, context = '') => { + // Agent time limits (agentrelay.com#138). The optional agents the time plan + // guards are stopped at their FLOW_TIME allowance: check-repair, the + // adversary reviews and the fixer, plus check-discovery, whose fallback is + // cheap. The planner, plan reviewer, implementer, prototypes and comparator + // stay unbounded: they are the mandatory path that produces the change, a + // hard stop there leaves nothing worth publishing, and no measured run + // overran on them. The header's wallclock still bounds them. + // + // Only the cloud target states limits. Cloud runs relayflows 2.0.40 or + // later; the local kit pins RELAYFLOWS_VERSION (2.0.26), whose runtime and + // types refuse an agent `timeout`. A local flow keeps the same branches, + // which never fire there. Emit limits locally once that pin reaches 2.0.40. + const limit = (minutes: number) => target === 'cloud' ? `\n timeout: "${minutes}m",` : ''; + const options = (role: AgentRole, fallback: string, context = '', minutes?: number) => { const value = config(role); const cli = value.agent === agents.builder && fallback === 'builder' ? 'builder' : JSON.stringify(value.agent); - return `cli: ${cli},${value.model ? `\n model: ${JSON.stringify(value.model)},` : ''}\n task: task + "\\n" + ${JSON.stringify(value.prompt)}${context},`; + return `cli: ${cli},${value.model ? `\n model: ${JSON.stringify(value.model)},` : ''}\n task: task + "\\n" + ${JSON.stringify(value.prompt)}${context},${minutes === undefined ? '' : limit(minutes)}`; }; const prototypeConfigs = (['prototype-1', 'prototype-2', 'prototype-3'] as const).map(config); const sections = [{ id: 'task', code: ` const normalizedTitle = issue.title.trim().replace(/\\s+/g, " "); @@ -724,7 +754,11 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType ${FLOW_TIME.bodyMinutes} - (await clock() - startedAt) / 60 - parallelMinutes;` }]; + const minutesLeft = async () => ${FLOW_TIME.bodyMinutes} - (await clock() - startedAt) / 60 - parallelMinutes; + // A long agent step may be stopped at its time limit. Its step then resolves + // with completionReason "timeout" instead of throwing, and what the agent + // committed stays. That is never success: each caller says what it means. + const timedOut = (result: unknown) => (result as { completionReason?: string } | undefined)?.completionReason === "timeout";` }]; if (workflow === 'traditional') sections.push({ id: 'plan', code: ` // Read the ticket and agree on a plan before changing code. await f.agent("planner", { ${options('planner', 'builder')} @@ -772,9 +806,15 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType { + const step = (name: string, ms: number, parallel = false, command?: string) => { // Strictly greater, as the kernel compares (flows authored-budget.ts, // machine/budget.rs): a step is refused once the charged time exceeds the limit. if (charged > budgetMs) { refused = name; throw new Error(`Flow budget exceeded before step "${name}"`); } - calls.push({ name, atMinute: charged / MINUTE }); + calls.push({ name, atMinute: charged / MINUTE, minutes: ms / MINUTE, ...(command === undefined ? {} : { command }) }); charged += ms; if (parallel) { groupStart ??= wallMs; @@ -95,10 +106,15 @@ async function runTimed(timing: Timing, workflow: FactoryDraft['workflow'] = 'tr console.error = (message: string) => { errors.push(String(message)); }; try { await exports.default!({ - agent: async (name: string) => { + agent: async (name: string, agentOptions?: { timeout?: string }) => { // Agents started together (prototypes) run at once: each is charged // in full, but the wall clock moves once for the whole group. - step(name, agentMinutes(name) * MINUTE, options.parallel?.some(prefix => name.startsWith(prefix)) ?? false); + const limit = durationMs(agentOptions?.timeout); + const wanted = agentMinutes(name) * MINUTE; + const timedOut = limit !== undefined && wanted > limit; + step(name, timedOut ? limit : wanted, options.parallel?.some(prefix => name.startsWith(prefix)) ?? false); + if (timedOut) calls[calls.length - 1]!.timedOut = true; + return { completionReason: timedOut ? 'timeout' : 'success', summary: '', artifacts: [] }; }, run: async (command: string, runOptions?: { timeout?: string }) => { if (command === FLOW_CHECK_RUN_COMMAND) { @@ -111,15 +127,14 @@ async function runTimed(timing: Timing, workflow: FactoryDraft['workflow'] = 'tr step('base-check', base.minutes * MINUTE); return base.verdict; } - const limit = /^(\d+)(ms|s|m|h)$/.exec(runOptions?.timeout ?? ''); - const ms = !timing.fullTimeouts ? 5000 : limit ? Number(limit[1]) * UNITS[limit[2]!]! : FLOW_TIME.defaultStepMinutes * MINUTE; - step(command.startsWith(FLOW_OPEN_CHANGE_COMMAND) ? 'open-change' : command.includes(FLOW_PUSH_COMMAND) ? 'push' : command === FLOW_TIME_STOP_COMMAND ? 'time-stop' : command === FLOW_REVIEW_BLOCKED_COMMAND ? 'review-blocked' : 'run', ms); + const ms = !timing.fullTimeouts ? 5000 : durationMs(runOptions?.timeout) ?? FLOW_TIME.defaultStepMinutes * MINUTE; + step(command.startsWith(FLOW_OPEN_CHANGE_COMMAND) ? 'open-change' : command.includes(FLOW_PUSH_COMMAND) ? 'push' : command === FLOW_TIME_STOP_COMMAND ? 'time-stop' : command.endsWith(FLOW_REVIEW_BLOCKED_COMMAND) ? 'review-blocked' : 'run', ms, false, command); // The flow reads the clock through a journaled step. if (command === 'date +%s') return String(epoch + Math.floor(wallMs / 1000)); if (command.endsWith(FLOW_PUBLISH_CHECK_COMMAND)) return 'publish'; if (command.endsWith(FLOW_VALIDATE_CHANGE_METADATA_COMMAND)) return 'valid'; if (command === 'git rev-parse HEAD') return 'abc123'; - if (command.startsWith('test -f review.clean')) return 'no'; + if (command.startsWith('test -f review.clean')) return timing.reviewClean ? 'yes' : 'no'; // No committed check script, so check-discovery runs (and is charged). if (command.startsWith('test -s .relayflow/check.sh')) return 'no'; if (command.startsWith('test -s')) return 'yes'; @@ -209,10 +224,13 @@ describe('Garden flow time budget (cloud#4108)', () => { const fixRounds: Record = { traditional: 0, prototype: 0, simple: 0 }; const guarded: Record = { traditional: 0, prototype: 0, simple: 0 }; let checked = 0; + let timedOut = 0; for (const workflow of ['traditional', 'prototype', 'simple'] as const) for (const [verdict, early] of [['timeout', 10], ['pass', 1]] as const) { for (let implementer = 0; implementer <= 150; implementer += 0.5) { const run = await runTimed({ - agents: { 'planner': early, 'plan-reviewer': early, 'check-discovery': early, 'prototype': 2 * early, 'comparator': early, 'implementer': implementer, 'check-repair': FLOW_TIME.repairMinutes, 'adversary': FLOW_TIME.reviewMinutes, 'fixer': FLOW_TIME.fixerMinutes }, + // The long agents would run on well past their limits (agentrelay.com#138), + // so each is stopped at, and charged exactly, its FLOW_TIME allowance. + agents: { 'planner': early, 'plan-reviewer': early, 'check-discovery': early, 'prototype': 2 * early, 'comparator': early, 'implementer': implementer, 'check-repair': 10 * FLOW_TIME.repairMinutes, 'adversary': 10 * FLOW_TIME.reviewMinutes, 'fixer': 10 * FLOW_TIME.fixerMinutes }, checks: [{ minutes: FLOW_TIME.checkMinutes, verdict }], baseline: { minutes: FLOW_TIME.checkMinutes, verdict: 'fail' }, fullTimeouts: true, @@ -231,6 +249,13 @@ describe('Garden flow time budget (cloud#4108)', () => { // exclude and that read), charged here at 30s each. expect(run.chargedMinutes).toBeLessThanOrEqual(FLOW_TIME.bodyMinutes + 2 * FLOW_TIME.defaultStepMinutes); const names = run.calls.map(call => call.name); + for (const call of run.calls) { + const limit = call.name.startsWith('check-repair') ? FLOW_TIME.repairMinutes : call.name.startsWith('adversary') ? FLOW_TIME.reviewMinutes : call.name === 'fixer' ? FLOW_TIME.fixerMinutes : undefined; + if (limit === undefined) continue; + expect(call.timedOut, call.name).toBe(true); + expect(call.minutes, call.name).toBe(limit); + timedOut++; + } // The run reached a guard that chose to run its optional step. if (names.some(name => name.startsWith('check-repair') || name === 'base-check' || name.startsWith('adversary'))) guarded[workflow]!++; if (names.includes('fixer')) { @@ -245,6 +270,7 @@ describe('Garden flow time budget (cloud#4108)', () => { // in parallel, as the budget charges them), and traditional, the only one // with a fixer, reaches a fix round. expect(checked).toBeGreaterThan(100); + expect(timedOut).toBeGreaterThan(100); for (const workflow of ['traditional', 'prototype', 'simple']) expect(guarded[workflow], workflow).toBeGreaterThan(0); expect(fixRounds.traditional).toBeGreaterThan(0); }, 30_000); // About a thousand simulated runs. @@ -309,3 +335,99 @@ describe('Garden flow time budget (cloud#4108)', () => { expect(run.errors.join('\n')).not.toMatch(/as far as the base commit shows/); }); }); + +describe('timed-out agent steps (agentrelay.com#138)', () => { + const errorsOf = (run: Awaited>) => run.errors.join('\n'); + const after = (run: Awaited>, name: string) => run.calls.slice(run.calls.findIndex(call => call.name === name) + 1); + + it('counts a timed-out repair as a failed attempt: re-check, then the draft and its report', async () => { + const run = await runTimed({ + agents: { 'check-repair': 200 }, + checks: [{ minutes: 5, verdict: 'fail' }], + baseline: { minutes: 5, verdict: 'fail' }, + }, 'simple'); + const repairs = run.calls.filter(call => call.name.startsWith('check-repair')); + // Stopped at its allowance and charged exactly that; not tried again. + expect(repairs).toHaveLength(1); + expect(repairs[0]).toMatchObject({ minutes: FLOW_TIME.repairMinutes, timedOut: true }); + expect(after(run, 'check-repair-1')[0]!.name).toBe('check'); + expect(errorsOf(run)).toContain(`check-repair-1 was stopped at its ${FLOW_TIME.repairMinutes}m limit`); + // The existing path for checks that still fail: the base commit is + // compared and the pull request opens as a draft. + expect(run.calls.map(call => call.name)).toContain('base-check'); + expect(run.opened?.command).toContain(' --draft'); + expect(run.refused).toBeNull(); + }); + + it('keeps a repair that passes the re-check despite its time-out', async () => { + const run = await runTimed({ + agents: { 'check-repair': 200 }, + checks: [{ minutes: 5, verdict: 'fail' }, { minutes: 5, verdict: 'pass' }], + }, 'simple'); + expect(run.calls.filter(call => call.name.startsWith('check-repair'))).toHaveLength(1); + expect(run.calls.map(call => call.name)).not.toContain('base-check'); + expect(run.opened?.command).not.toContain(' --draft'); + }); + + it('never reads a timed-out review as clean: the pull request goes to draft with the report', async () => { + // Even a review.clean the stopped reviewer left behind does not count. + const run = await runTimed({ + agents: { 'adversary': 200 }, + checks: [{ minutes: 5, verdict: 'pass' }], + reviewClean: true, + }, 'prototype'); + const review = run.calls.find(call => call.name === 'adversary-1')!; + expect(review).toMatchObject({ minutes: FLOW_TIME.reviewMinutes, timedOut: true }); + expect(after(run, 'adversary-1').some(call => call.command?.startsWith('test -f review.clean'))).toBe(false); + const blocked = run.calls.find(call => call.name === 'review-blocked'); + expect(blocked?.command).toMatch(/^review_timeout=yes; /); + expect(errorsOf(run)).toContain(`adversary-1 was stopped at its ${FLOW_TIME.reviewMinutes}m limit`); + expect(run.finish).toBe('step_failed'); + }); + + it('treats a timed-out first review like an unresolved one: a fix round, then the second review decides', async () => { + const run = await runTimed({ + agents: { 'adversary-1': 200, 'adversary-2': 5 }, + checks: [{ minutes: 5, verdict: 'pass' }], + reviewClean: true, + }); + expect(run.calls.filter(call => call.name.startsWith('adversary'))).toHaveLength(2); + expect(run.calls.find(call => call.name === 'adversary-1')?.timedOut).toBe(true); + expect(run.calls.map(call => call.name)).toContain('fixer'); + // The second review finished and left review.clean: the run parks for a person. + expect(run.calls.map(call => call.name)).not.toContain('review-blocked'); + expect(run.finish).toBe('needs_human'); + }); + + it('keeps a timed-out fixer\'s work and checks it as usual', async () => { + const run = await runTimed({ + agents: { 'fixer': 200 }, + checks: [{ minutes: 5, verdict: 'pass' }], + }); + expect(run.calls.find(call => call.name === 'fixer')).toMatchObject({ minutes: FLOW_TIME.fixerMinutes, timedOut: true }); + expect(errorsOf(run)).toContain(`fixer was stopped at its ${FLOW_TIME.fixerMinutes}m limit`); + const next = after(run, 'fixer').map(call => call.name); + expect(next[0]).toBe('check'); + expect(next).toContain('push'); + expect(next).toContain('adversary-2'); + expect(run.refused).toBeNull(); + }); + + it('falls back to the ecosystem default when check discovery times out', async () => { + const run = await runTimed({ agents: { 'check-discovery': 200 }, checks: [{ minutes: 5, verdict: 'pass' }] }, 'simple'); + expect(run.calls.find(call => call.name === 'check-discovery')).toMatchObject({ minutes: FLOW_TIME.discoveryMinutes, timedOut: true }); + // What it left half-written is not a recipe; the resolver writes the default. + expect(after(run, 'check-discovery')[0]!.command).toBe('rm -f .relayflow/check.sh'); + expect(errorsOf(run)).toContain(`check-discovery was stopped at its ${FLOW_TIME.discoveryMinutes}m limit`); + }); + + it('states no agent limits for a local run, so an older runtime still runs it', async () => { + const run = await runTimed({ + agents: { 'check-repair': 50, 'adversary': 25 }, + checks: [{ minutes: 5, verdict: 'fail' }, { minutes: 5, verdict: 'pass' }], + }, 'traditional', { target: 'local' }); + expect(run.calls.some(call => call.timedOut)).toBe(false); + expect(run.calls.find(call => call.name === 'check-repair-1')?.minutes).toBe(50); + expect(errorsOf(run)).not.toMatch(/was stopped at its/); + }); +}); diff --git a/web/lib/test/flow-onboarding.test.ts b/web/lib/test/flow-onboarding.test.ts index 60c3e61f..5ad23d91 100644 --- a/web/lib/test/flow-onboarding.test.ts +++ b/web/lib/test/flow-onboarding.test.ts @@ -384,13 +384,16 @@ describe('software factory onboarding', () => { // outcome that tells an operator nothing. The reason is back, and the pull // request still carries the findings, which no exit code can. expect(finish).toBe('step_failed'); - expect(calls).toContain(FLOW_REVIEW_BLOCKED_COMMAND); - expect(calls.indexOf(FLOW_REVIEW_BLOCKED_COMMAND)).toBeGreaterThan(calls.lastIndexOf('adversary-2:codex')); + // Both reviewers finished, so the report does not say one was stopped + // at its time limit (agentrelay.com#138). + const blocked = 'review_timeout=no; ' + FLOW_REVIEW_BLOCKED_COMMAND; + expect(calls).toContain(blocked); + expect(calls.indexOf(blocked)).toBeGreaterThan(calls.lastIndexOf('adversary-2:codex')); // The run outcome says only "its own checks did not pass", so the findings // are printed by a step of their own just before done("step_failed"), and // repeated in the stop message (AgentWorkforce/flows#542 would carry them // on done() itself). - expect(calls.indexOf(FLOW_REPORT_REVIEW_FINDINGS_COMMAND)).toBeGreaterThan(calls.indexOf(FLOW_REVIEW_BLOCKED_COMMAND)); + expect(calls.indexOf(FLOW_REPORT_REVIEW_FINDINGS_COMMAND)).toBeGreaterThan(calls.indexOf(blocked)); expect(calls.at(-1)).toBe(FLOW_REPORT_REVIEW_FINDINGS_COMMAND); expect(errors.join('\n')).toContain('One P2 remains.'); expect(factorySource(completed)).toContain('AgentWorkforce/flows#542'); @@ -407,7 +410,7 @@ describe('software factory onboarding', () => { // The paired negative for the failed-review case above: a clean run parks // with the same reason, so the difference has to be visible somewhere. It // is — a clean run never marks the pull request as unapproved. - expect(calls).not.toContain(FLOW_REVIEW_BLOCKED_COMMAND); + expect(calls.some(call => call.endsWith(FLOW_REVIEW_BLOCKED_COMMAND))).toBe(false); expect(calls).not.toContain(FLOW_REPORT_REVIEW_FINDINGS_COMMAND); expect(calls.some(call => call.includes('pr merge'))).toBe(false); expect(factorySource({ ...completed, workflow })).not.toContain('f.human('); diff --git a/web/lib/test/flow-workflows.test.ts b/web/lib/test/flow-workflows.test.ts index 9ad66c7c..9cff997f 100644 --- a/web/lib/test/flow-workflows.test.ts +++ b/web/lib/test/flow-workflows.test.ts @@ -12,6 +12,7 @@ import { WORKFLOWS, FLOW_TIME, } from '../flow-workflows'; import { factorySource, type FactoryDraft } from '../flow-onboarding'; +import { RELAYFLOWS_VERSION } from '../flow-local'; /** * Every generated check step runs under `sh`, and its exit code is the only @@ -870,3 +871,131 @@ describe('publish-path step timeouts (agentrelay.com#135)', () => { }); } }); + +/** + * FLOW_TIME starts a long agent step only when its allowance still fits, but + * until relayflows 2.0.40 nothing stopped an agent at that allowance: a + * check-repair agent ran 38m / 592 turns / $54, another 44m / $26.57 + * (agentrelay.com#138, cloud#4108). Each long agent step now states its + * allowance as a hard `timeout`, which resolves the step with + * `completionReason: "timeout"` instead of throwing (AgentWorkforce/flows#606). + */ +describe('agent step time limits (agentrelay.com#138)', () => { + const draft = (workflow: FactoryDraft['workflow']): FactoryDraft => ({ version: 4, sources: ['github'], sourceSettings: { github: { repository: 'acme/app', labels: 'ready' } }, agents: ['claude', 'codex'], otherAgent: '', task: 'Add a test', workflow, step: 3 }); + + /** Every `f.agent` call in the generated source: its name expression and its `timeout`, if any. */ + function agentSteps(source: string) { + const file = ts.createSourceFile('flow.ts', source, ts.ScriptTarget.ES2022, true); + const steps: { name: string; timeout?: string; source: string }[] = []; + const visit = (node: ts.Node) => { + if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression) + && node.expression.name.text === 'agent' && ts.isIdentifier(node.expression.expression) && node.expression.expression.text === 'f') { + const [name, options] = node.arguments; + const timeout = options && ts.isObjectLiteralExpression(options) + ? options.properties.find(property => ts.isPropertyAssignment(property) && property.name.getText(file) === 'timeout') + : undefined; + // "check-repair-" + (++repairs) and "adversary-" + (round + 1) name a family by their prefix. + const prefix = name && ts.isBinaryExpression(name) ? name.left : name; + steps.push({ + name: prefix && ts.isStringLiteralLike(prefix) ? prefix.text.replace(/-$/, '') : prefix?.getText(file) ?? '', + timeout: timeout && ts.isPropertyAssignment(timeout) && ts.isStringLiteralLike(timeout.initializer) ? timeout.initializer.text : undefined, + source: node.getText(file), + }); + } + ts.forEachChild(node, visit); + }; + visit(file); + return steps; + } + + // The allowance each agent step is stopped at; undefined stays unbounded. + const LIMITS: Record = { + 'check-repair': FLOW_TIME.repairMinutes, + 'adversary': FLOW_TIME.reviewMinutes, + 'fixer': FLOW_TIME.fixerMinutes, + 'check-discovery': FLOW_TIME.discoveryMinutes, + 'planner': undefined, + 'plan-reviewer': undefined, + 'implementer': undefined, + 'prototype': undefined, + 'comparator': undefined, + }; + + it('keeps every allowance within the runtime\'s 60m ceiling', () => { + for (const minutes of Object.values(LIMITS)) if (minutes !== undefined) { + expect(minutes).toBeGreaterThan(0); + expect(minutes).toBeLessThanOrEqual(FLOW_TIME.agentLimitMaxMinutes); + } + expect(FLOW_TIME.agentLimitMaxMinutes).toBe(60); + }); + + for (const { id } of WORKFLOWS) { + it(`stops each long agent step in the ${id} cloud flow at its FLOW_TIME allowance`, () => { + const steps = agentSteps(factorySource(draft(id))); + expect(steps.length).toBeGreaterThan(0); + for (const step of steps) { + expect(Object.keys(LIMITS), step.source).toContain(step.name); + const minutes = LIMITS[step.name]; + expect(step.timeout, step.source).toBe(minutes === undefined ? undefined : `${minutes}m`); + } + // Never vacuous: the repair agent is in every workflow, the reviewer and + // fixer where the workflow has them. + const names = steps.map(step => step.name); + expect(names).toContain('check-repair'); + expect(names).toContain('check-discovery'); + if (id !== 'simple') expect(names).toContain('adversary'); + if (id === 'traditional') expect(names).toContain('fixer'); + }); + + it(`handles every timed-out agent in the ${id} flow, and never as success`, () => { + const source = factorySource(draft(id)); + // The results are read, not dropped, and each branch says what happened. + expect(source).toMatch(/const repair = await f\.agent\("check-repair-"/); + expect(source).toMatch(/if \(timedOut\(repair\)\)/); + expect(source).toMatch(/const discovery = await f\.agent\("check-discovery"/); + expect(source).toMatch(/if \(timedOut\(discovery\)\)/); + if (id !== 'simple') { + expect(source).toMatch(/const review = await f\.agent\("adversary-"/); + expect(source).toMatch(/reviewTimedOut = timedOut\(review\)/); + expect(source).toContain('clean = !reviewTimedOut && '); + } + if (id === 'traditional') { + expect(source).toMatch(/const fix = await f\.agent\("fixer"/); + expect(source).toMatch(/if \(timedOut\(fix\)\)/); + } + }); + + it(`states no agent time limit in the ${id} local flow, whose pinned runtime predates them`, () => { + // The local kit installs RELAYFLOWS_VERSION, older than 2.0.40, and an + // older runtime refuses an agent `timeout`. Once the pin reaches 2.0.40, + // emit the limits for the local target too and change this test. + const [major, minor, patch] = RELAYFLOWS_VERSION.split('.').map(Number); + expect(major! * 1e6 + minor! * 1e3 + patch!).toBeLessThan(2_000_040); + const source = factorySource(draft(id), 'local'); + expect(agentSteps(source).filter(step => step.timeout !== undefined).map(step => step.source)).toEqual([]); + // The branches are still there, and still typecheck against 2.0.26's + // AgentResult (which has no completionReason): they never fire. + expect(source).toContain('const timedOut = (result: unknown) =>'); + }); + } +}); + +describe('FLOW_REVIEW_BLOCKED_COMMAND after a timed-out review (agentrelay.com#138)', () => { + it('says the review was stopped at its limit, and still posts what it wrote', () => { + const root = fixture(review); + const bin = path.join(root, 'bin'); + mkdirSync(bin, { recursive: true }); + writeFileSync(path.join(bin, 'gh'), '#!/bin/sh\necho "$@" >> gh-calls.txt\nexit 0\n', { mode: 0o755 }); + const run = (timedOut: string) => { + const result = spawnSync('/bin/sh', ['-c', `review_timeout=${timedOut}; ${FLOW_REVIEW_BLOCKED_COMMAND}`], { cwd: root, encoding: 'utf8', env: { ...process.env, PATH: `${bin}:${process.env.PATH}` } }); + return { code: result.status, blocked: read(root, 'review-blocked.md') }; + }; + const stopped = run('yes'); + expect(stopped.code).toBe(0); + expect(stopped.blocked).toContain(`stopped at its ${FLOW_TIME.reviewMinutes}-minute limit`); + expect(stopped.blocked).toContain('The retry loop still drops the last error.'); + const finished = run('no'); + expect(finished.blocked).not.toContain('stopped at its'); + expect(finished.blocked).toContain('The retry loop still drops the last error.'); + }); +}); From 2d639df5a868035cc93c938b1cd31c4e385c61b3 Mon Sep 17 00:00:00 2001 From: agentrelaybot Date: Sun, 4 Oct 2026 02:42:01 -0700 Subject: [PATCH 2/2] fix(flows): clear review.md before each review; catch non-literal agent timeouts in tests Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: 27242e9e-7fb5-448f-a698-8fa8e9644610 --- web/lib/flow-workflows.ts | 4 +++- web/lib/test/flow-budget.test.ts | 3 +++ web/lib/test/flow-workflows.test.ts | 6 +++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/web/lib/flow-workflows.ts b/web/lib/flow-workflows.ts index 5f2c5ce5..6c84142a 100644 --- a/web/lib/flow-workflows.ts +++ b/web/lib/flow-workflows.ts @@ -954,7 +954,9 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType { const review = run.calls.find(call => call.name === 'adversary-1')!; expect(review).toMatchObject({ minutes: FLOW_TIME.reviewMinutes, timedOut: true }); expect(after(run, 'adversary-1').some(call => call.command?.startsWith('test -f review.clean'))).toBe(false); + // The reviewer starts without a previous round's review.md, so a stopped + // one can never pass off older findings as its own. + expect(run.calls[run.calls.indexOf(review) - 1]?.command).toBe('rm -f review.clean review.md'); const blocked = run.calls.find(call => call.name === 'review-blocked'); expect(blocked?.command).toMatch(/^review_timeout=yes; /); expect(errorsOf(run)).toContain(`adversary-1 was stopped at its ${FLOW_TIME.reviewMinutes}m limit`); diff --git a/web/lib/test/flow-workflows.test.ts b/web/lib/test/flow-workflows.test.ts index 9cff997f..276d547d 100644 --- a/web/lib/test/flow-workflows.test.ts +++ b/web/lib/test/flow-workflows.test.ts @@ -898,7 +898,11 @@ describe('agent step time limits (agentrelay.com#138)', () => { const prefix = name && ts.isBinaryExpression(name) ? name.left : name; steps.push({ name: prefix && ts.isStringLiteralLike(prefix) ? prefix.text.replace(/-$/, '') : prefix?.getText(file) ?? '', - timeout: timeout && ts.isPropertyAssignment(timeout) && ts.isStringLiteralLike(timeout.initializer) ? timeout.initializer.text : undefined, + // A timeout that is not a string literal is still a timeout: record it + // so the no-limit checks cannot pass over it. + timeout: timeout === undefined ? undefined + : ts.isPropertyAssignment(timeout) && ts.isStringLiteralLike(timeout.initializer) ? timeout.initializer.text + : ``, source: node.getText(file), }); }