From cab6295ccd4668f2e310fd631f13838fa0e94326 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 18 Sep 2026 08:50:26 -0700 Subject: [PATCH 1/8] fix(flows): check the repository the way its CI does, and never throw the work away The generated flow ran one hard-coded recipe (npm ci && npm test behind a lockfile detector) and treated its exit code as the whole verdict, so a failing test step killed the run with the agents' finished work unpushed. Two production runs died that way: - AgentWorkforce/cloud acbe30c1: cloud's suite needs @cloud/core built first, which its CI does and the recipe did not. - AgentWorkforce/relay 139d1a46: the branch passed all 3,227 tests in a clean shell of the same sandbox, but 18 failed inside the flow, among them `transport 'file' not allowed` from Cloud's own git configuration. The check step is now language-agnostic and resilient: - The check script (.relayflow/check.sh) comes from the author's checkCommand, a committed .relayflow/check.sh, a discovery agent that reads CI config, Makefile/justfile and README, or an ecosystem default (make/just test, Node, Cargo, Go, Python, Ruby, Maven, Gradle, .NET, Mix). - Each check step prints one token and exits 0; output goes to .relayflow/*.log and its tail to stderr for the journal. The check runs without Cloud's GIT_CONFIG_* hardening and stops itself before the 15-minute f.run lease. - A repair agent (two attempts) fixes missing setup in check.sh or bugs in the change, never by weakening tests. - What still fails is compared with the base commit in a throwaway worktree. The branch is always pushed and the pull request opens as a draft with the verdict, the script and both outputs in its body. A change that breaks checks the base passes ends step_failed; a failure the base shares ends needs_human after the reviews. - Working files (summary.md, plans, reviews, .relayflow/, kit files) are excluded through .git/info/exclude and, if an agent committed them anyway, removed from the branch before every push. Both production runs had Codex commit summary.md. Co-Authored-By: Claude Opus 5 (1M context) --- web/app/flows/onboarding/AgentStepEditor.tsx | 2 +- web/app/flows/onboarding/RunOptions.tsx | 2 +- web/lib/flow-agent-settings.ts | 8 +- web/lib/flow-local.ts | 5 +- web/lib/flow-workflows.ts | 356 ++++++++++++++++-- web/lib/test/flow-local.test.ts | 70 ++-- web/lib/test/flow-onboarding.test.ts | 128 ++++++- web/lib/test/flow-workflows.test.ts | 374 ++++++++++++++++--- 8 files changed, 816 insertions(+), 129 deletions(-) diff --git a/web/app/flows/onboarding/AgentStepEditor.tsx b/web/app/flows/onboarding/AgentStepEditor.tsx index 82220d7..ef213ef 100644 --- a/web/app/flows/onboarding/AgentStepEditor.tsx +++ b/web/app/flows/onboarding/AgentStepEditor.tsx @@ -5,7 +5,7 @@ import { defaultAgentPrompt, resolveAgentSettings, type AgentRole, type FlowAgen import type { FlowTrack } from '../../../lib/flow-analytics'; import s from './onboarding.module.css'; -const roleLabels: Record = { planner: 'Plan', 'plan-reviewer': 'Review plan', 'prototype-1': 'Implementation 1', 'prototype-2': 'Implementation 2', 'prototype-3': 'Implementation 3', comparator: 'Compare', implementer: 'Implement', adversary: 'Review', fixer: 'Fix review findings' }; +const roleLabels: Record = { planner: 'Plan', 'plan-reviewer': 'Review plan', 'prototype-1': 'Implementation 1', 'prototype-2': 'Implementation 2', 'prototype-3': 'Implementation 3', comparator: 'Compare', implementer: 'Implement', adversary: 'Review', fixer: 'Fix review findings', 'check-discovery': 'Find how to run checks', 'check-repair': 'Repair failing checks' }; export function AgentStepEditor({ draft, roles, onChange, onClose, onTrack }: { draft: FactoryDraft; roles: AgentRole[]; onChange: (draft: FactoryDraft) => void; onClose: () => void; onTrack: FlowTrack }) { const dialog = useRef(null); diff --git a/web/app/flows/onboarding/RunOptions.tsx b/web/app/flows/onboarding/RunOptions.tsx index fbc3058..626ba1a 100644 --- a/web/app/flows/onboarding/RunOptions.tsx +++ b/web/app/flows/onboarding/RunOptions.tsx @@ -78,7 +78,7 @@ export function RunOptions({ draft, chosen, setDestination, onTrack, getJourneyI
  • Check and run on a new branch{command(LOCAL_RUN, 'run commands')}
    • onTrack('help_toggled', { section: 'local_requirements', open: event.currentTarget.open })}>Requirements and local behavior -

    Node.js 22.18+, macOS (Apple silicon) or Linux (x64), your selected coding agents installed and signed in, and GitHub CLI authenticated. Start with a clean Git repository and a working npm test command.

    +

    Node.js 22.18+, macOS (Apple silicon) or Linux (x64), your selected coding agents installed and signed in, and GitHub CLI authenticated. Start with a clean Git repository whose tests run the way its CI runs them; the flow reads your CI configuration to find the command, or you can set checkCommand in the flow.

    The local version uses a one-hour runtime limit, not a dollar cap. Your coding agent’s usage charges still apply. Every preset stops at “needs_human” for you to review and merge the PR in GitHub.

    In GitHub, require approving reviews and passing CI checks in your target branch’s rules. A repository administrator needs to configure these protections.

    This runs one ticket. Automatic triggers from your issue tracker require a separate connection.

    diff --git a/web/lib/flow-agent-settings.ts b/web/lib/flow-agent-settings.ts index 9d56c5c..1a038b1 100644 --- a/web/lib/flow-agent-settings.ts +++ b/web/lib/flow-agent-settings.ts @@ -1,7 +1,7 @@ import { isCodingAgent, type CodingAgent } from './flow-agents'; import type { WorkflowId, WorkflowStep } from './flow-workflows'; -export const AGENT_ROLES = ['planner', 'plan-reviewer', 'prototype-1', 'prototype-2', 'prototype-3', 'comparator', 'implementer', 'adversary', 'fixer'] as const; +export const AGENT_ROLES = ['planner', 'plan-reviewer', 'prototype-1', 'prototype-2', 'prototype-3', 'comparator', 'implementer', 'adversary', 'fixer', 'check-discovery', 'check-repair'] as const; export type AgentRole = typeof AGENT_ROLES[number]; export type AgentSettings = { agent?: CodingAgent; model?: string; prompt?: string }; export type FlowAgentSettings = Partial>; @@ -20,7 +20,11 @@ export function defaultAgentPrompt(workflow: WorkflowId, role: AgentRole): strin case 'comparator': return 'Compare the implementations and test results in the provided worktrees. Read their code and prototype-notes.md. Write comparison.md with each prototype path, strengths, weaknesses, and which ideas to combine. Do not modify the prototypes or implement yet.'; case 'implementer': return (workflow === 'traditional' ? 'Follow reviewed-plan.md. ' : workflow === 'prototype' ? 'Read comparison.md and inspect the prototype implementations it references. Combine the strongest ideas into the final implementation on the current branch, not in the prototype worktrees. ' : '') + 'Implement on the current branch. Add regression tests. Commit changes. Write a PR summary to summary.md.'; case 'adversary': return 'Review the PR diff, tests, and all PR comments. ' + (workflow === 'prototype' ? 'Read comparison.md to check that the final implementation combines the strongest ideas. ' : '') + 'Find bugs and edge cases. Write review.md. Create review.clean only if no issues remain.'; - case 'fixer': return 'Read review.md and gh pr view --comments. Address every issue. Commit fixes without pushing. The workflow runs tests and pushes only after they pass.'; + case 'fixer': return 'Read review.md and gh pr view --comments. Address every issue. Commit fixes without pushing. The workflow runs the checks and pushes the revision.'; + // Setup, not the ticket: the ticket text arrives with every task, so the + // prompt says plainly not to start on it. + case 'check-discovery': return 'This is a setup step: do not start on the ticket. Work out how this repository checks itself on a fresh machine, the way its CI does. Read the CI configuration (.github/workflows, .gitlab-ci.yml, .circleci and similar), any Makefile, justfile or Taskfile, and AGENTS.md, CLAUDE.md, CONTRIBUTING and README. Write .relayflow/check.sh: a POSIX sh script starting with set -e that installs dependencies, runs whatever CI runs before its tests (builds, code generation), then runs the tests. Leave out steps that need secrets, deployments or services this machine does not have, with a comment saying why. Do not run the full test suite, do not change any other file, and do not commit. If the repository has no tests, do not create the file.'; + case 'check-repair': return 'The repository\'s checks failed on this branch. The command that ran is .relayflow/check.sh and its full output is in .relayflow/check.log. For each failure, work out whether it comes from missing setup (a build, code generation or install step the tests expect, often named in the error) or from a bug in the change on this branch. Fix missing setup by adding the step to .relayflow/check.sh the way the repository\'s CI does it; do not commit that file. Fix bugs in the change and commit the fix. Never skip, delete or weaken a test, and never change a test only to make it pass. If a failure is outside your control, such as a tool that is not installed, no network, or missing credentials, leave it and write what you found to .relayflow/repair-notes.md.'; default: return ''; } } diff --git a/web/lib/flow-local.ts b/web/lib/flow-local.ts index 0d1a742..3527f21 100644 --- a/web/lib/flow-local.ts +++ b/web/lib/flow-local.ts @@ -114,7 +114,7 @@ const RUN = ${JSON.stringify(LOCAL_RUN)}; const KIT_FILES = new Set([ "START-HERE.txt", "flow-input.json", ${JSON.stringify(LOCAL_PREFLIGHT)}, "software-factory.flow.mts", "package.json", "package-lock.json", - "node_modules/", ".relayflowd/", "summary.md", + "node_modules/", ".relayflowd/", ".relayflow/", "summary.md", ]); // KIT_FILES exempts names from the dirty-tree check; it is not a description of @@ -427,7 +427,7 @@ Requirements - macOS on Apple silicon or Linux x64 (bundled runtime platforms). - ${names}, installed and signed in. - GitHub CLI (gh), signed in, and a repository with push access to origin. -- The flow installs your repository's dependencies with its package manager (pnpm, Yarn, Bun, or npm, chosen by lockfile) and runs its test script. Without a package.json or a test script it reports that it skipped the tests and carries on. If your project uses another test command, change testCommand in software-factory.flow.mts before running. +- The flow runs your repository's own checks. Before changing any code, an agent reads your CI configuration, Makefile and README and writes .relayflow/check.sh; failing that it uses your ecosystem's default (a make or just test target, npm/pnpm/Yarn/Bun, cargo, go, pytest, bundle, Maven, Gradle, dotnet or mix). To use your own command instead, set checkCommand in software-factory.flow.mts, or commit a .relayflow/check.sh. The tools your checks need must be installed. 1. Extract this kit into your repository root. Keep any existing files before replacing them. Open a terminal in that directory. @@ -443,6 +443,7 @@ ${LOCAL_PREFLIGHT} runs first and stops before any model usage if this is not a The flows command then starts the local runtime and attaches the local worker. Coding agents use their existing local sign-in; no Agent Relay Cloud account is needed. This flow edits code, runs tests, pushes the branch, and opens a pull request. If the agents commit nothing — a ticket with nothing to do in this repository — the run stops before pushing: no branch, no pull request, and a line saying why. +If the checks fail, a repair agent reads the output and fixes missing setup or its own bugs, never by weakening tests. Whatever still fails is compared with the commit the branch started from, and the pull request opens as a draft with both outputs in its body: the work is never thrown away. A change that breaks checks which pass on the starting commit ends as step_failed (exit code 1). Working files (summary.md, plans, reviews, .relayflow/) are kept out of the commits through .git/info/exclude, and removed from the branch before pushing if an agent committed them anyway. Local runtime behavior This local version uses a one-hour wall-clock budget. Model usage is billed by your coding-agent provider; this is not a dollar cap. diff --git a/web/lib/flow-workflows.ts b/web/lib/flow-workflows.ts index 7b85b8f..c68dc2f 100644 --- a/web/lib/flow-workflows.ts +++ b/web/lib/flow-workflows.ts @@ -15,7 +15,7 @@ export const WORKFLOW_STEP_DETAILS: Record = { '3 implementations': 'Build three different solutions in parallel. Each gets its own isolated workspace.', 'Compare': 'Compare the code and test results, then identify the strongest ideas from each solution.', 'Build': 'Use the comparison to combine the strongest ideas into one final implementation.', - 'Run checks': 'Run the repository’s tests with a script. Stop if any check fails.', + 'Run checks': 'Run the repository’s own checks, the way its CI does. Repair missing setup, and compare any failure with the starting commit.', 'Open PR': 'Push the branch and open a pull request with a summary of the changes.', '2× adversarial review': 'Challenge the code in two fresh reviews, fixing and retesting issues between rounds.', 'Review': 'Check the final code against the ticket and comparison. Stop if issues remain.', @@ -24,6 +24,17 @@ export const WORKFLOW_STEP_DETAILS: Record = { export type WorkflowId = (typeof WORKFLOWS)[number]['id']; +/** Single-quotes a value for `sh`, so a generated command can carry any text. */ +const shq = (value: string) => `'${value.replace(/'/g, `'\\''`)}'`; + +/** + * Where the check step keeps everything it writes. None of it belongs in the + * change: FLOW_EXCLUDE_WORKING_FILES_COMMAND hides it from `git add -A`, and + * FLOW_DROP_WORKING_FILES_COMMAND removes it from the branch if an agent + * committed it anyway. + */ +export const FLOW_CHECK_SCRIPT = '.relayflow/check.sh'; + /** * Reads `package.json` with Node, which every sandbox has because it has npm. * A missing file, unparseable JSON, or an empty/absent `test` script all exit @@ -32,33 +43,240 @@ export type WorkflowId = (typeof WORKFLOWS)[number]['id']; const HAS_TEST_SCRIPT = 'node -e \'const f=require("fs");let p;try{p=JSON.parse(f.readFileSync("package.json","utf8"))}catch{process.exit(1)}const t=(p.scripts||{}).test;process.exit(t&&String(t).trim()?0:1)\''; /** - * Installs dependencies with the repository's own package manager (chosen by - * lockfile) and runs its `test` script. Cloud sandboxes ship npm and corepack - * but not pnpm or Yarn, so a bare `npm test` fails for pnpm/Yarn repositories - * whose test script calls the package manager (AgentWorkforce/burn#540). Yarn - * Berry (`.yarnrc.yml`) installs with `--immutable`; Yarn Classic with - * `--frozen-lockfile`. - * - * The step runs under `sh`, and its exit code is the only signal the runner - * has: `f.run` takes a timeout but no retry policy, so anything non-zero is - * retried until the run dies with `retries_exhausted`. A repository with no - * `package.json` therefore killed real runs — every npm subcommand reports - * ENOENT as `errno -2`, which npm returns verbatim as its exit status, and - * `-2 & 0xFF` is 254. Retrying can never create the file, so "there is nothing - * to test" now skips with a message and exit 0; only a genuine install or test - * failure, or a lockfile whose package manager cannot be provided, exits - * non-zero. Every path prints a line, so the journal never records an empty - * `stdout_tail` again. + * The JavaScript default: installs dependencies with the repository's own + * package manager (chosen by lockfile) and runs its `test` script. Cloud + * sandboxes ship npm and corepack but not pnpm or Yarn, so a bare `npm test` + * fails for pnpm/Yarn repositories whose test script calls the package manager + * (AgentWorkforce/burn#540). Yarn Berry (`.yarnrc.yml`) installs with + * `--immutable`; Yarn Classic with `--frozen-lockfile`. `npm ci` runs only with + * a lockfile: the old `npm ci || npm install` chain hid npm ci failures behind + * a second install. */ -export const FLOW_TEST_COMMAND = [ - 'set -e', - 'if [ ! -f package.json ]; then echo "relayflow: no package.json in the repository root; skipping tests." && exit 0; fi', - `if ! ${HAS_TEST_SCRIPT} >/dev/null 2>&1; then echo "relayflow: package.json has no runnable test script; skipping tests." && exit 0; fi`, +const NODE_CHECK_LINES = [ 'if [ -f pnpm-lock.yaml ]; then pm=pnpm; elif [ -f yarn.lock ]; then pm=yarn; elif [ -f bun.lock ] || [ -f bun.lockb ]; then pm=bun; else pm=npm; fi', 'if [ "$pm" = pnpm ] || [ "$pm" = yarn ]; then if ! command -v "$pm" >/dev/null 2>&1; then mkdir -p "$HOME/.local/bin" && { corepack enable --install-directory "$HOME/.local/bin" "$pm" >/dev/null 2>&1 || true; } && PATH="$HOME/.local/bin:$PATH" && export PATH; fi; COREPACK_ENABLE_DOWNLOAD_PROMPT=0 && export COREPACK_ENABLE_DOWNLOAD_PROMPT; fi', - 'if ! command -v "$pm" >/dev/null 2>&1; then echo "relayflow: this repository\'s lockfile requires $pm, which is not installed and could not be provisioned." >&2 && exit 1; fi', + 'if ! command -v "$pm" >/dev/null 2>&1; then echo "relayflow: this repository\'s lockfile requires $pm, which is not installed and could not be provisioned." >&2; exit 1; fi', 'echo "relayflow: installing dependencies and running tests with $pm"', 'if [ "$pm" = pnpm ]; then pnpm install --frozen-lockfile && pnpm test; elif [ "$pm" = yarn ]; then { if [ -f .yarnrc.yml ]; then yarn install --immutable; else yarn install --frozen-lockfile; fi; } && yarn test; elif [ "$pm" = bun ]; then bun install --frozen-lockfile && bun run test; else { if [ -f package-lock.json ]; then npm ci; else npm install; fi; } && npm test; fi', +]; + +const DEFAULT_SCRIPT_HEADER = '# Written by relayflow from the files in this repository. Edit it to match how CI runs the tests.'; + +/** + * Settles how this repository checks itself, as a script at FLOW_CHECK_SCRIPT, + * before any code changes. + * + * The generated flow used to run one hard-coded recipe — `npm ci && npm test` + * behind an npm/pnpm/Yarn/Bun detector — and treat its exit code as the whole + * verdict. That failed both ways in production. AgentWorkforce/cloud run + * acbe30c1 died at the test step, whatever the ticket: cloud's suite asserts + * that `@cloud/core` is built (`npm run -w @cloud/core build`), cloud's CI does + * that before testing, and the recipe did not. A Go, Rust or Python repository + * got "no package.json; skipping tests" and was never tested at all. No fixed + * recipe knows a repository's setup, so the script comes from the repository. + * + * By the time this runs the script may already exist — the author's + * `checkCommand`, a `.relayflow/check.sh` the repository commits, or one the + * discovery agent wrote from its CI configuration — and then it is used as it + * is (`script`). Otherwise the ecosystem default is written into it + * (`default`): a Makefile or justfile `test` target first, because that is the + * repository's own entry point, then Node, Cargo, Go, Python, Ruby, Maven, + * Gradle, .NET and Mix. Writing the default into the file rather than running + * it inline is what lets the repair agent add a missing setup step to it, and + * lets the base-commit comparison run exactly the same recipe. `none` means + * there is nothing to run, and the pull request says so. + * + * One token on stdout, prose on stderr, exit 0 on every path: `f.run` has no + * retry policy, so a non-zero exit is retried until the run dies with + * `retries_exhausted`. + */ +export const FLOW_CHECK_RESOLVE_COMMAND = [ + 'mkdir -p .relayflow', + 'has_test_target() { for f in GNUmakefile makefile Makefile; do if [ -f "$f" ] && grep -Eq \'^test[[:space:]]*:\' "$f"; then return 0; fi; done; return 1; }', + 'eco=; cmd=', + `if [ -s ${FLOW_CHECK_SCRIPT} ]; then eco=script` + + `; elif has_test_target; then eco=make; cmd='make test'` + + `; elif [ -f justfile ] && grep -Eq '^test([[:space:]]|:)' justfile; then eco=just; cmd='just test'` + + `; elif [ -f package.json ] && ${HAS_TEST_SCRIPT} >/dev/null 2>&1; then eco=node` + + `; elif [ -f Cargo.toml ]; then eco=cargo; cmd='cargo test'` + + `; elif [ -f go.mod ]; then eco=go; cmd='go test ./...'` + + `; elif [ -f pyproject.toml ] || [ -f setup.py ] || [ -f setup.cfg ] || [ -f requirements.txt ] || [ -f pytest.ini ] || [ -f tox.ini ]; then eco=python; if [ -f uv.lock ]; then cmd='uv run pytest'; elif [ -f poetry.lock ]; then cmd='poetry install --no-interaction && poetry run pytest'; elif [ -f requirements.txt ]; then cmd='python3 -m pip install -r requirements.txt && python3 -m pytest'; else cmd='python3 -m pytest'; fi` + + `; elif [ -f Gemfile ]; then eco=ruby; if [ -d spec ]; then cmd='bundle install && bundle exec rspec'; else cmd='bundle install && bundle exec rake test'; fi` + + `; elif [ -f pom.xml ]; then eco=maven; if [ -x mvnw ]; then cmd='./mvnw -B test'; else cmd='mvn -B test'; fi` + + `; elif [ -f build.gradle ] || [ -f build.gradle.kts ]; then eco=gradle; if [ -x gradlew ]; then cmd='./gradlew test'; else cmd='gradle test'; fi` + + `; elif [ -n "$(find . -maxdepth 1 \\( -name '*.sln' -o -name '*.csproj' -o -name '*.fsproj' \\) -print 2>/dev/null | head -n 1)" ]; then eco=dotnet; cmd='dotnet test'` + + `; elif [ -f mix.exs ]; then eco=elixir; cmd='mix deps.get && mix test'` + + '; fi', + `if [ "$eco" = script ]; then echo "relayflow: checking with the repository's own ${FLOW_CHECK_SCRIPT}." >&2; echo script` + + `; elif [ -z "$eco" ]; then echo "relayflow: found no way to run this repository's tests: no check command, no test target and no recognised project file." >&2; echo none` + + `; elif [ "$eco" = node ]; then printf '%s\\n' ${[DEFAULT_SCRIPT_HEADER, 'set -e', ...NODE_CHECK_LINES].map(shq).join(' ')} > ${FLOW_CHECK_SCRIPT}; echo "relayflow: wrote the Node default to ${FLOW_CHECK_SCRIPT}." >&2; echo default` + + `; else printf '%s\\n' ${shq(DEFAULT_SCRIPT_HEADER)} 'set -e' "$cmd" > ${FLOW_CHECK_SCRIPT}; echo "relayflow: wrote the $eco default ($cmd) to ${FLOW_CHECK_SCRIPT}." >&2; echo default` + + '; fi', +].join('; '); + +/** + * Runs FLOW_CHECK_SCRIPT and reports `pass`, `fail`, `timeout` or `none`. + * + * The full output goes to a log file, not to stdout: the flow reads one token + * back from `f.run`, and the report and the repair agent read the log. The last + * lines are echoed to stderr so the run journal still shows why a check failed + * — the old step's journal kept a stdout tail that Cloud never surfaced, so a + * failed run said only `retries_exhausted`. + * + * The script runs without the Git configuration Cloud's executor installs for + * its own clone and push (`GIT_CONFIG_COUNT`/`KEY_n`/`VALUE_n` with + * `protocol.allow=never`, `GIT_CONFIG_GLOBAL=/dev/null`). Those reached the + * user's tests: AgentWorkforce/relay run 139d1a46 failed 18 tests, among them + * `sandbox-repo.test.ts` with `fatal: transport 'file' not allowed`, while the + * same branch passed all 3,227 in a clean shell of the same sandbox. A + * repository's tests must see the machine a fresh CI runner would. + * + * `f.run` leases a command for at most 15 minutes and dies past it, so the + * check stops itself first (`timeout`, 14 minutes by default) and reports that + * instead of taking the run down. The caller may set `check_dir` (where to run, + * default here) and `check_out` (the log). + */ +export const FLOW_CHECK_RUN_COMMAND = [ + 'root="$PWD"', + `script="$root/${FLOW_CHECK_SCRIPT}"`, + 'check_dir="${check_dir:-$root}"', + 'check_out="${check_out:-$root/.relayflow/check.log}"', + 'mkdir -p "$(dirname "$check_out")"', + 'if [ ! -s "$script" ]; then echo "relayflow: there is no check script, so no checks ran." > "$check_out"; echo "relayflow: there is no check script, so no checks ran." >&2; echo none' + + '; else limit="${RELAYFLOW_CHECK_TIMEOUT:-840}"; if command -v timeout >/dev/null 2>&1; then guard="timeout $limit"; else guard=; fi' + + '; echo "relayflow: running $script in $check_dir" >&2' + + '; ( cd "$check_dir" && env -u GIT_CONFIG_COUNT -u GIT_CONFIG_GLOBAL -u GIT_CONFIG_NOSYSTEM $guard sh "$script" ) > "$check_out" 2>&1 < /dev/null; status=$?' + + '; tail -n 40 "$check_out" >&2' + + '; if [ "$status" -eq 0 ]; then echo "relayflow: the checks passed." >&2; echo pass' + + '; elif [ -n "$guard" ] && [ "$status" -eq 124 ]; then echo "relayflow: the checks did not finish within ${limit}s." >&2; echo timeout' + + '; else echo "relayflow: the checks failed with exit $status; the full output is in $check_out." >&2; echo fail; fi' + + '; fi', +].join('; '); + +/** + * Runs the same check against the commit the branch started from, in a + * throwaway worktree, and reports its token — or `unknown` when the base + * commit cannot be checked out. + * + * It runs only after the branch has failed and the repair agent could not fix + * it, because it answers the one question an exit code cannot: did this change + * break the checks, or were they failing already? Both production failures + * were the second kind (acbe30c1: a build step the recipe never ran; 139d1a46: + * Cloud's Git configuration leaking into the tests), and both runs died with + * the agents' finished work still in the sandbox. A failure the base commit + * shares is a problem for a person, not a reason to throw the work away. + */ +export const FLOW_BASE_CHECK_COMMAND = [ + 'root="$PWD"', + 'tmp=', + 'if [ -n "$base" ] && git rev-parse --verify --quiet "$base^{commit}" >/dev/null 2>&1 && tmp=$(mktemp -d "${TMPDIR:-/tmp}/relayflow-base.XXXXXX") && git worktree add --detach -q "$tmp/base" "$base" >/dev/null 2>&1; then ' + + 'check_dir="$tmp/base"; check_out="$root/.relayflow/base-check.log"; ' + + FLOW_CHECK_RUN_COMMAND + + '; git worktree remove --force "$tmp/base" >/dev/null 2>&1; git worktree prune >/dev/null 2>&1; rm -rf "$tmp"' + + '; else echo "relayflow: could not check out the base commit to compare against." >&2; if [ -n "$tmp" ]; then rm -rf "$tmp"; fi; echo unknown; fi', +].join('; '); + +/** + * Writes what the checks found to `.relayflow/check-report.md`, and the pull + * request body — summary.md followed by that report — to `.relayflow/pr-body.md`. + * + * The caller sets `check` (the branch's token) and `baseline` (the base + * commit's token, empty when it was not needed, or `revision` when a fixer's + * revision broke checks that had passed). A reviewer reads the verdict, the + * script that ran and the tail of each log on the pull request itself, without + * opening the run journal. + */ +export const FLOW_CHECK_REPORT_COMMAND = [ + 'mkdir -p .relayflow', + 'report=.relayflow/check-report.md', + '{ printf \'## Checks\\n\\n\'' + + '; case "$check" in' + + ` pass) printf '%s\\n' "Relayflow ran this repository's checks (${FLOW_CHECK_SCRIPT}) and they passed." ;;` + + ` none) printf '%s\\n' "Relayflow found no way to run this repository's tests — no check command, no CI test job it could follow, no test target and no recognised project file — so no checks ran. Review the change with that in mind." ;;` + + ' *) case "$baseline" in' + + ` fail|timeout) printf '%s\\n' "**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." ;;` + + ` pass) printf '%s\\n' "**This change breaks checks that pass on the base commit.** The flow tried to repair it and could not. The pull request is a draft so the work is not lost; it is not ready to merge." ;;` + + ` revision) printf '%s\\n' "**The latest revision breaks checks that passed before it.** It was pushed so the work is not lost, and the pull request is now a draft; it is not ready to merge." ;;` + + ` *) printf '%s\\n' "**The checks failed**, and the base commit could not be checked for comparison, so it is not known whether this change caused them. This pull request is a draft until someone looks." ;;` + + ' esac ;;' + + ' esac' + + `; if [ -s ${FLOW_CHECK_SCRIPT} ]; then printf '\\n
    What ran (${FLOW_CHECK_SCRIPT})\\n\\n\`\`\`sh\\n'; cat ${FLOW_CHECK_SCRIPT}; printf '\`\`\`\\n
    \\n'; fi` + + `; if [ "$check" != pass ] && [ "$check" != none ] && [ -s .relayflow/check.log ]; then printf '\\n
    Output on this branch (last 80 lines)\\n\\n\`\`\`\\n'; tail -n 80 .relayflow/check.log; printf '\`\`\`\\n
    \\n'; fi` + + `; if [ "$baseline" = fail ] || [ "$baseline" = timeout ]; then if [ -s .relayflow/base-check.log ]; then printf '\\n
    Output on the base commit (last 80 lines)\\n\\n\`\`\`\\n'; tail -n 80 .relayflow/base-check.log; printf '\`\`\`\\n
    \\n'; fi; fi` + + `; if [ -s .relayflow/repair-notes.md ]; then printf '\\n### What the repair agent found\\n\\n'; cat .relayflow/repair-notes.md; fi` + + '; } > "$report"', + '{ if [ -s summary.md ]; then cat summary.md; printf \'\\n\\n\'; fi; cat "$report"; } > .relayflow/pr-body.md', + 'echo "relayflow: wrote the check report to $report." >&2', + 'echo written', +].join('; '); + +/** + * Marks the pull request as not ready when a revision breaks the checks, and + * posts the check report to it. Every branch exits 0, like + * FLOW_REVIEW_BLOCKED_COMMAND, and for the same reason. + */ +export const FLOW_CHECK_BLOCKED_COMMAND = [ + 'if gh pr ready --undo >/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." >&2; fi', + 'if gh pr comment --body-file .relayflow/check-report.md >/dev/null 2>&1; then echo "relayflow: posted the check report to the pull request."; else echo "relayflow: could not comment on the pull request; .relayflow/check-report.md still holds the report." >&2; fi', +].join('; '); + +/** + * Files the flow and its agents write for each other, which are never part of + * the change. Paths are relative to the repository root. + */ +export const FLOW_WORKING_FILES = [ + 'summary.md', 'plan.md', 'reviewed-plan.md', 'review.md', 'review.clean', 'review-blocked.md', + 'comparison.md', 'prototype-notes.md', '.relayflow', + // The local kit's own files, extracted into the repository root. + 'START-HERE.txt', 'flow-input.json', 'relay-preflight.mjs', 'software-factory.flow.mts', +] as const; + +/** + * Hides the working files from `git add -A` for the rest of the run, through + * the clone's own `.git/info/exclude` — nothing in the repository changes. + * + * Patterns are anchored to the root (`/summary.md`), so a `docs/summary.md` the + * change legitimately adds is still committed, and tracked files are never + * affected: exclusion applies only to untracked paths. The block carries a + * header so it is written once and a person can find and delete it. This is + * prevention only; FLOW_DROP_WORKING_FILES_COMMAND is what guarantees it. + */ +export const FLOW_EXCLUDE_WORKING_FILES_COMMAND = [ + 'x=$(git rev-parse --git-path info/exclude 2>/dev/null)', + `if [ -n "$x" ]; then mkdir -p "$(dirname "$x")" 2>/dev/null; if ! grep -qxF '# relayflow working files' "$x" 2>/dev/null; then printf '%s\\n' '' '# relayflow working files' ${FLOW_WORKING_FILES.map(file => shq(`/${file}${file === '.relayflow' ? '/' : ''}`)).join(' ')} '/.relayflowd/' >> "$x"; fi; echo "relayflow: working files are excluded from commits." >&2; else echo "relayflow: not a git repository, so nothing was excluded." >&2; fi`, + 'echo done', +].join('; '); + +/** + * Removes working files from the branch before it is pushed, when an agent + * committed them. + * + * Both production runs did: Codex committed summary.md into + * AgentWorkforce/cloud (acbe30c1) and AgentWorkforce/relay (139d1a46), so the + * pull-request body would have shipped as a file at the repository root, as it + * did in AgentWorkforce/flows#454. Only paths absent from the base commit are + * removed, so a repository's own summary.md, and any change the agents made to + * it, survives. + * + * The removal is one new commit built from a private index (`read-tree HEAD`, + * `update-index --force-remove`, `write-tree`, `commit-tree`), not + * `git commit`: the real index may hold changes an agent staged and never + * committed, and they must not be swept into this commit. The files stay in + * the working tree, where the pull-request body is read from. It falls back to + * a Relayflow identity only when the clone has none, and exits 0 on every + * path: a branch that keeps a stray file is better than a run that dies here. + */ +export const FLOW_DROP_WORKING_FILES_COMMAND = [ + `paths=${shq(FLOW_WORKING_FILES.join(' '))}`, + 'if [ -z "$base" ] || ! git rev-parse --verify --quiet "$base^{commit}" >/dev/null 2>&1; then echo "relayflow: the base commit is unknown, so working files were left as committed." >&2' + + '; else drop=$(git ls-files -- $paths | while IFS= read -r p; do git cat-file -e "$base:$p" 2>/dev/null || printf \'%s\\n\' "$p"; done)' + + '; if [ -z "$drop" ]; then echo "relayflow: no working files were committed." >&2' + + '; else tmp=$(mktemp -d); msg="Keep relayflow working files out of the change"' + + '; if GIT_INDEX_FILE="$tmp/index" git read-tree HEAD && printf \'%s\\n\' "$drop" | GIT_INDEX_FILE="$tmp/index" git update-index --force-remove --stdin && tree=$(GIT_INDEX_FILE="$tmp/index" git write-tree) && { commit=$(git commit-tree "$tree" -p HEAD -m "$msg" 2>/dev/null) || commit=$(git -c user.name=Relayflow -c user.email=noreply@agentrelay.com commit-tree "$tree" -p HEAD -m "$msg"); } && git update-ref -m "relayflow: drop working files" HEAD "$commit"' + + '; then printf \'%s\\n\' "$drop" | git update-index --force-remove --stdin 2>/dev/null; echo "relayflow: removed working files from the branch: $(printf \'%s \' $drop)" >&2' + + '; else echo "relayflow: could not remove the working files from the branch." >&2; fi' + + '; rm -rf "$tmp"; fi; fi', + 'echo done', ].join('; '); /** @@ -87,7 +305,7 @@ export const FLOW_TEST_COMMAND = [ * publishes only on the strength of a summary.md that is actually there, and * otherwise declines to push. * - * Like FLOW_TEST_COMMAND, every path exits 0. `f.run` has no retry policy, so a + * Like the check commands above, every path exits 0. `f.run` has no retry policy, so a * non-zero exit is retried until the run dies with `retries_exhausted`; a check * that cannot tell must never be the thing that kills the run. */ @@ -148,7 +366,10 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType "'" + value.replace(/'/g, "'\\\\''") + "'"; + const checkScript = ${JSON.stringify(FLOW_CHECK_SCRIPT)}; + if (checkCommand.trim()) { + await f.run("mkdir -p .relayflow && printf '%s\\\\n' 'set -e' " + shellQuote(checkCommand) + " > " + checkScript); + } else if ((await f.run("test -s " + checkScript + " && echo yes || echo no")).trim() !== "yes") { + // No command of yours and none committed to the repository: read how its + // CI tests it. Anything this cannot settle falls back to a default below. + await f.agent("check-discovery", { + ${options('check-discovery', 'builder')} + }); + } + await f.run(${JSON.stringify(FLOW_CHECK_RESOLVE_COMMAND)}); + + // Build and test the change, then open a pull request. // Where this branch started, so the publish step below can tell whether the // agents actually committed anything. const baseCommit = (await f.run("git rev-parse HEAD")).trim(); await f.agent("implementer", { ${options('implementer', 'builder')} });` }); - sections.push({ id: 'checks', code: ` // Scripted checks must pass before publishing the change. - // Install dependencies with the repository's package manager, then run its tests. - const testCommand = ${JSON.stringify(FLOW_TEST_COMMAND)}; - await f.run(testCommand, { timeout: "15m" });` }); + sections.push({ id: 'checks', code: ` // Run the checks. A failure is not the end of the run: the tests are + // how this flow learns what is wrong, so the repair agent reads the output + // and fixes what it can, and whatever still fails is compared against the + // commit this branch started from. Every check step prints one word and + // exits 0; the full output stays in .relayflow/ for the report. + const verdict = (value: string) => ["pass", "fail", "timeout", "none"].includes(value.trim()) ? value.trim() : "fail"; + const verdictOf = (value: string) => /^[a-z]*$/.test(value) ? value : "unknown"; + const broken = (value: string) => value === "fail" || value === "timeout"; + const runChecks = ${JSON.stringify(FLOW_CHECK_RUN_COMMAND)}; + let repairs = 0; + const checkAndRepair = async () => { + let result = verdict(await f.run(runChecks, { timeout: "15m" })); + for (let attempt = 0; attempt < 2 && broken(result); attempt++) { + await f.agent("check-repair-" + (++repairs), { + ${options('check-repair', 'builder')} + }); + result = verdict(await f.run(runChecks, { timeout: "15m" })); + } + return result; + }; + const check = await checkAndRepair(); + // Only a failure pays for the comparison: pass, fail, timeout or unknown. + const baseline = broken(check) + ? (await f.run("base=" + baseCommit + "; " + ${JSON.stringify(FLOW_BASE_CHECK_COMMAND)}, { timeout: "15m" })).trim() + : "";` }); sections.push({ id: 'pull-request', code: ` // Publish the branch and open the pull request without an agent. // Doing no work is a legitimate outcome: a repository with nothing to act on // leaves no commits and no summary.md. Pushing a branch at the base commit // and failing inside "gh pr create" is not the report such a run should // leave, so the check below decides, and anything it cannot vouch for is - // treated as nothing to publish. + // treated as nothing to publish. Working files an agent committed are taken + // out of the branch first, so they are neither published nor counted. + const dropWorkingFiles = ${JSON.stringify(FLOW_DROP_WORKING_FILES_COMMAND)}; + await f.run("base=" + baseCommit + "; " + dropWorkingFiles); const publishCheck = ${JSON.stringify(FLOW_PUBLISH_CHECK_COMMAND)}; const publish = (await f.run("base=" + baseCommit + "; " + publishCheck)).trim(); if (publish !== "publish" && publish !== "no-summary") { @@ -208,7 +471,23 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType; ...`, so a prefix cannot tell them apart. + * `check` may be a function, for a sequence of check results. + */ +function answer(command: string, { publish = 'publish', clean = 'yes', check = 'pass' as string | (() => string), baseline = 'pass' } = {}) { + if (command === FLOW_CHECK_RUN_COMMAND) return typeof check === 'function' ? check() : check; + if (command.endsWith(FLOW_BASE_CHECK_COMMAND)) return baseline; + if (command.endsWith(FLOW_PUBLISH_CHECK_COMMAND)) return publish; + return command.startsWith('test -f') ? clean : ''; +} const draft: FactoryDraft = { ...DEFAULT_FACTORY, sources: ['github'], sourceSettings: { github: { repository: 'acme/app', labels: 'bug, ready' } }, agents: ['claude', 'codex'], workflow: 'traditional', step: 3 }; @@ -78,8 +90,9 @@ describe('local flow starter kit', () => { expect(code).not.toContain('f.done("canceled")'); expect(code).not.toContain('f.done("budget_exceeded")'); // The reason the runtime does lower is used where the flow judges its own - // work. `simple` runs no review, so it has no such verdict to report. - expect(code.includes('f.done("step_failed")')).toBe(workflow !== 'simple'); + // work: every preset when its change breaks checks the base commit + // passes, and the reviewing presets when a review does not pass. + expect(code).toContain('f.done("step_failed")'); // The unshipped reason is named where a reader meets the park, so the // guard is not left looking like an unexplained choice. expect(source).toContain('AgentWorkforce/flows#438'); @@ -223,10 +236,10 @@ describe('local flow starter kit', () => { let finish = ''; await compile(localKitFiles(draft)['software-factory.flow.mts'])({ agent: async (name: string) => { calls.push(name); }, - run: async (command: string) => command.startsWith('base=') ? 'publish' : command.startsWith('test -f') ? 'yes' : '', + run: async (command: string) => answer(command), done: (reason: string) => { finish = reason; }, }, localInput(draft)); - expect(calls).toEqual(['planner', 'plan-reviewer', 'implementer', 'adversary-1', 'adversary-2']); + expect(calls).toEqual(['planner', 'plan-reviewer', 'check-discovery', 'implementer', 'adversary-1', 'adversary-2']); expect(finish).toBe('needs_human'); expect(factorySource(draft)).toContain('return f.done("needs_human")'); expect(factorySource(draft, 'local')).not.toContain('f.human('); @@ -246,7 +259,7 @@ describe('local flow starter kit', () => { try { await compile(localKitFiles(draft)['software-factory.flow.mts'])({ agent: async () => {}, - run: async (command: string) => { commands.push(command); return command.startsWith('base=') ? 'no-commits' : command.startsWith('test -f') ? 'yes' : ''; }, + run: async (command: string) => { commands.push(command); return answer(command, { publish: 'no-commits' }); }, done: (reason: string) => { finish = reason; }, }, localInput(draft)); } finally { console.error = original; } @@ -263,11 +276,11 @@ describe('local flow starter kit', () => { const selected = { ...draft, workflow }; await compile(factorySource(selected, 'local'))({ agent: async () => {}, - run: async (command: string) => { commands.push(command); return command.startsWith('base=') ? 'publish' : command.startsWith('test -f') ? 'yes' : ''; }, + run: async (command: string) => { commands.push(command); return answer(command); }, done: (reason: string) => { finish = reason; }, }, localInput(selected)); expect(finish).toBe('needs_human'); - const testIndex = commands.indexOf(FLOW_TEST_COMMAND); + const testIndex = commands.indexOf(FLOW_CHECK_RUN_COMMAND); const createIndex = commands.findIndex(command => command.startsWith('gh pr create')); expect(testIndex).toBeGreaterThanOrEqual(0); expect(createIndex).toBeGreaterThan(testIndex); @@ -275,43 +288,52 @@ describe('local flow starter kit', () => { } }); - it('does not publish a PR or signal readiness if scripted tests fail', async () => { + it('opens only a draft, never a ready pull request, when a change breaks the checks', async () => { + // It used to throw away the work: the failing step killed the run with the + // agents' commits still unpushed. Now the work is pushed as a draft with the + // output, and the run still ends short of approval. for (const workflow of ['traditional', 'prototype', 'simple'] as const) { const commands: string[] = []; let finish = ''; const selected = { ...draft, workflow }; - await expect(compile(factorySource(selected, 'local'))({ + await compile(factorySource(selected, 'local'))({ agent: async () => {}, - run: async (command: string) => { commands.push(command); if (command === FLOW_TEST_COMMAND) throw Error('tests failed'); return ''; }, + run: async (command: string) => { commands.push(command); return answer(command, { check: 'fail', baseline: 'pass' }); }, done: (reason: string) => { finish = reason; }, - }, localInput(selected))).rejects.toThrow('tests failed'); - expect(commands.some(command => command.startsWith('git push') || command.startsWith('gh pr create'))).toBe(false); - expect(finish).toBe(''); + }, localInput(selected)); + const create = commands.find(command => command.startsWith('gh pr create')) ?? ''; + expect(create).toContain('--draft'); + expect(commands).toContain('git push --set-upstream origin HEAD'); + expect(finish).toBe('step_failed'); } }); - it.each([false, true])('pushes fixer revisions only after passing tests (failure: %s)', async (fail) => { + it.each([false, true])('checks and pushes fixer revisions, drafting the pull request if they break passing checks (failure: %s)', async (fail) => { const calls: string[] = []; let checks = 0; - const run = compile(factorySource(draft, 'local'))({ + let finish = ''; + await compile(factorySource(draft, 'local'))({ agent: async (name: string, options: { task: string }) => { calls.push(name); if (name === 'fixer') expect(options.task).toContain('Commit fixes without pushing'); }, run: async (command: string) => { calls.push(command); - if (command === FLOW_TEST_COMMAND && ++checks === 2 && fail) throw Error('revision failed'); - return command.startsWith('base=') ? 'publish' : command.startsWith('test -f') ? 'no' : ''; + return answer(command, { clean: 'no', check: () => (++checks >= 2 && fail ? 'fail' : 'pass') }); }, - done: () => {}, + done: (reason: string) => { finish = reason; }, }, localInput(draft)); - if (fail) await expect(run).rejects.toThrow('revision failed'); - else await run; const fixer = calls.indexOf('fixer'); expect(fixer).toBeGreaterThan(0); - expect(calls[fixer + 1]).toBe(FLOW_TEST_COMMAND); - if (fail) expect(calls).not.toContain('git push'); - else expect(calls[fixer + 2]).toBe('git push'); + expect(calls[fixer + 1]).toBe(FLOW_CHECK_RUN_COMMAND); + // Pushed either way: the revision is work, and work is never thrown away. + expect(calls.indexOf('git push')).toBeGreaterThan(fixer); + if (fail) { + expect(calls.indexOf(FLOW_CHECK_BLOCKED_COMMAND)).toBeGreaterThan(calls.indexOf('git push')); + expect(finish).toBe('step_failed'); + } else { + expect(calls).not.toContain(FLOW_CHECK_BLOCKED_COMMAND); + } }); it('generates valid local source for every preset with an explicit runtime limit', () => { diff --git a/web/lib/test/flow-onboarding.test.ts b/web/lib/test/flow-onboarding.test.ts index 67bb235..178cb8e 100644 --- a/web/lib/test/flow-onboarding.test.ts +++ b/web/lib/test/flow-onboarding.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import ts from 'typescript'; -import { FLOW_REVIEW_BLOCKED_COMMAND, FLOW_TEST_COMMAND } from '../flow-workflows'; +import { FLOW_BASE_CHECK_COMMAND, FLOW_CHECK_BLOCKED_COMMAND, FLOW_CHECK_RUN_COMMAND, FLOW_DROP_WORKING_FILES_COMMAND, FLOW_PUBLISH_CHECK_COMMAND, FLOW_REVIEW_BLOCKED_COMMAND } from '../flow-workflows'; import { cloudBlockedReason, cloudConnectionsHref, DEFAULT_FACTORY, factorySource, isMarkdownOnly, MARKDOWN_ONLY_CLOUD_NOTE, readFactoryDraft, canContinue, primaryAgent, onboardingPath, accessibleOnboardingStep, type FactoryDraft } from '../flow-onboarding'; import { localInput } from '../flow-local'; @@ -15,16 +15,19 @@ const completed: FactoryDraft = { version: 4, sources: ['github'], sourceSetting const withoutComments = (source: string) => source.split('\n').filter(line => !line.trim().startsWith('//')).join('\n'); /** - * `publish` is what the deterministic publish check reports. That check is the - * only command the flow builds as `base=; ...`, so the mock keys off - * that prefix without having to reproduce the command itself. + * `publish` is what the deterministic publish check reports. `checks` is what + * each run of the repository's checks reports, in order (pass once the list is + * used up), and `baseline` what the base commit's check reports. Three + * commands are built as `base=; ...`, so the mock matches each by the + * command it ends with. */ -async function runFactory(clean: boolean[], _approved = true, issue = matchingIssue, draft = completed, publish = 'publish') { +async function runFactory(clean: boolean[], _approved = true, issue = matchingIssue, draft = completed, publish = 'publish', checks: string[] = [], baseline = 'pass', edit = (source: string) => source) { const calls: string[] = []; const errors: string[] = []; let finish = ''; let index = 0; - const source = factorySource(draft).replace('import { flow } from "@relayflows/surface";', ''); + let checkIndex = 0; + const source = edit(factorySource(draft)).replace('import { flow } from "@relayflows/surface";', ''); const compiled = ts.transpileModule(source, { compilerOptions: { target: ts.ScriptTarget.ES2022, module: ts.ModuleKind.CommonJS } }); const exports: { default?: (ctx: unknown, input: unknown) => Promise } = {}; new Function('exports', 'flow', compiled.outputText)(exports, (_name: string, _options: unknown, body: unknown) => body); @@ -33,7 +36,13 @@ async function runFactory(clean: boolean[], _approved = true, issue = matchingIs try { await exports.default!({ agent: async (name: string, options: { cli: string; cwd?: string }) => { calls.push(`${name}:${options.cli}`); if (name.startsWith('prototype-')) { await Promise.resolve(); calls.push('finished:' + name + ':' + options.cwd); } }, - run: async (command: string) => { calls.push(command); return command.startsWith('base=') ? publish : command.startsWith('test -f') ? (clean[index++] ? 'yes' : 'no') : command.startsWith('mktemp') ? '/tmp/relay-prototypes.test' : command === 'git rev-parse HEAD' ? 'abc123' : ''; }, + run: async (command: string) => { + calls.push(command); + if (command === FLOW_CHECK_RUN_COMMAND) return checks[checkIndex++] ?? 'pass'; + if (command.endsWith(FLOW_BASE_CHECK_COMMAND)) return baseline; + if (command.endsWith(FLOW_PUBLISH_CHECK_COMMAND)) return publish; + return command.startsWith('test -f') ? (clean[index++] ? 'yes' : 'no') : command.startsWith('mktemp') ? '/tmp/relay-prototypes.test' : command === 'git rev-parse HEAD' ? 'abc123' : ''; + }, human: async () => { throw new Error('Interactive human approval is unsupported'); }, done: (reason: string) => { finish = reason; }, }, { issue, approver: 'owner' }); @@ -274,7 +283,7 @@ describe('software factory onboarding', () => { const { calls, finish } = await runFactory([false, true]); expect(calls.filter(call => call.startsWith('adversary-'))).toHaveLength(2); expect(calls).toContain('fixer:claude'); - expect(calls.filter(call => call === FLOW_TEST_COMMAND)).toHaveLength(2); + expect(calls.filter(call => call === FLOW_CHECK_RUN_COMMAND)).toHaveLength(2); expect(calls).not.toContain('human'); expect(finish).toBe('needs_human'); }); @@ -320,7 +329,7 @@ describe('software factory onboarding', () => { const { calls } = await runFactory([true]); const push = calls.indexOf('git push --set-upstream origin HEAD'); const create = calls.findIndex(call => call.startsWith('gh pr create')); - expect(push).toBeGreaterThan(calls.indexOf(FLOW_TEST_COMMAND)); + expect(push).toBeGreaterThan(calls.indexOf(FLOW_CHECK_RUN_COMMAND)); expect(create).toBeGreaterThan(push); expect(calls.indexOf('adversary-1:codex')).toBeGreaterThan(create); }); @@ -354,7 +363,7 @@ describe('software factory onboarding', () => { expect(calls).not.toContain(FLOW_REVIEW_BLOCKED_COMMAND); expect(calls.some(call => call.includes('pr merge'))).toBe(false); expect(factorySource({ ...completed, workflow })).not.toContain('f.human('); - expect(calls).toContain(FLOW_TEST_COMMAND); + expect(calls).toContain(FLOW_CHECK_RUN_COMMAND); expect(calls.some(call => call.startsWith('gh pr create'))).toBe(true); } }); @@ -397,9 +406,106 @@ describe('software factory onboarding', () => { } }); + describe('checks', () => { + const reportCall = (calls: string[]) => calls.find(call => call.startsWith('check=')) ?? ''; + const createCall = (calls: string[]) => calls.find(call => call.startsWith('gh pr create')) ?? ''; + + it('works out how to check the repository before the change, and excludes working files first', async () => { + const { calls } = await runFactory([true, true]); + expect(calls[0]).toContain('# relayflow working files'); + expect(calls.indexOf('check-discovery:claude')).toBeLessThan(calls.indexOf('implementer:claude')); + expect(calls.indexOf('check-discovery:claude')).toBeGreaterThan(calls.indexOf('plan-reviewer:codex')); + }); + + it('uses the author\'s checkCommand instead of discovering one', async () => { + const { calls } = await runFactory([true, true], true, matchingIssue, completed, 'publish', [], 'pass', + source => source.replace('const checkCommand = "";', 'const checkCommand = "npm run build:core && npm test";')); + expect(calls).not.toContain('check-discovery:claude'); + expect(calls).toContain("mkdir -p .relayflow && printf '%s\\n' 'set -e' 'npm run build:core && npm test' > .relayflow/check.sh"); + }); + + it('opens a ready pull request when the checks pass, with the report as its body', async () => { + const { calls, finish } = await runFactory([true, true]); + expect(calls.filter(call => call.startsWith('check-repair'))).toEqual([]); + expect(calls.some(call => call.endsWith(FLOW_BASE_CHECK_COMMAND))).toBe(false); + expect(reportCall(calls)).toMatch(/^check=pass; baseline=; /); + expect(createCall(calls)).toBe('gh pr create --title "Software factory change" --body-file .relayflow/pr-body.md'); + expect(finish).toBe('needs_human'); + }); + + it('repairs missing setup and carries on as if the checks had passed', async () => { + // acbe30c1's shape: the first run fails for want of a build step, the + // repair agent adds it to the check script, and the rerun passes. + const { calls, finish } = await runFactory([true, true], true, matchingIssue, completed, 'publish', ['fail', 'pass']); + expect(calls.filter(call => call.startsWith('check-repair'))).toEqual(['check-repair-1:claude']); + expect(calls.some(call => call.endsWith(FLOW_BASE_CHECK_COMMAND))).toBe(false); + expect(createCall(calls)).not.toContain('--draft'); + expect(calls).toContain('adversary-1:codex'); + expect(finish).toBe('needs_human'); + }); + + it('opens a draft and keeps reviewing when the base commit fails the same checks', async () => { + // 139d1a46's shape: the failure is the environment's, not the change's. + const { calls, finish, errors } = await runFactory([true, true], true, matchingIssue, completed, 'publish', ['fail', 'fail', 'fail'], 'fail'); + expect(calls.filter(call => call.startsWith('check-repair'))).toHaveLength(2); + expect(calls.find(call => call.endsWith(FLOW_BASE_CHECK_COMMAND))).toBe('base=abc123; ' + FLOW_BASE_CHECK_COMMAND); + expect(reportCall(calls)).toMatch(/^check=fail; baseline=fail; /); + expect(createCall(calls)).toContain('--draft'); + expect(calls).toContain('git push --set-upstream origin HEAD'); + expect(calls).toContain('adversary-2:codex'); + expect(finish).toBe('needs_human'); + expect(errors.join('\n')).toContain('not because of this change'); + }); + + it('opens a draft with the output and stops as step_failed when the change broke the checks', async () => { + const { calls, finish, errors } = await runFactory([true, true], true, matchingIssue, completed, 'publish', ['fail', 'fail', 'fail'], 'pass'); + expect(reportCall(calls)).toMatch(/^check=fail; baseline=pass; /); + expect(createCall(calls)).toContain('--draft'); + // The work is pushed, never thrown away; the reviews are not worth running. + expect(calls).toContain('git push --set-upstream origin HEAD'); + expect(calls.some(call => call.startsWith('adversary-'))).toBe(false); + expect(finish).toBe('step_failed'); + expect(errors.join('\n')).toContain('breaks checks that pass on the base commit'); + }); + + it('treats a timeout like a failure and an unrecognised answer as one too', async () => { + const timedOut = await runFactory([true, true], true, matchingIssue, completed, 'publish', ['timeout', 'pass']); + expect(timedOut.calls).toContain('check-repair-1:claude'); + const garbled = await runFactory([true, true], true, matchingIssue, completed, 'publish', ['???', 'pass']); + expect(garbled.calls).toContain('check-repair-1:claude'); + }); + + it('says so, and still opens a ready pull request, when there is nothing to run', async () => { + const { calls, finish } = await runFactory([true, true], true, matchingIssue, completed, 'publish', ['none']); + expect(calls.filter(call => call.startsWith('check-repair'))).toEqual([]); + expect(reportCall(calls)).toMatch(/^check=none; baseline=; /); + expect(createCall(calls)).not.toContain('--draft'); + expect(finish).toBe('needs_human'); + }); + + it('drops committed working files before deciding what to publish, and before every push', async () => { + const { calls } = await runFactory([false, true]); + const drops = calls.flatMap((call, index) => call.endsWith(FLOW_DROP_WORKING_FILES_COMMAND) ? [index] : []); + expect(drops).toHaveLength(2); + expect(drops[0]).toBeLessThan(calls.findIndex(call => call.endsWith(FLOW_PUBLISH_CHECK_COMMAND))); + expect(drops[1]).toBeLessThan(calls.indexOf('git push')); + expect(drops[1]).toBeGreaterThan(calls.indexOf('fixer:claude')); + }); + + it('pushes a review fix that breaks passing checks, then drafts the pull request and stops', async () => { + const { calls, finish } = await runFactory([false, true], true, matchingIssue, completed, 'publish', ['pass', 'fail', 'fail', 'fail']); + expect(calls).toContain('git push'); + expect(calls.indexOf(FLOW_CHECK_BLOCKED_COMMAND)).toBeGreaterThan(calls.indexOf('git push')); + expect(calls.filter(call => call.startsWith('check=')).at(-1)).toMatch(/^check=fail; baseline=revision; /); + expect(calls).not.toContain('adversary-2:codex'); + expect(finish).toBe('step_failed'); + }); + }); + it('simple skips agent reviews but still hands off for human approval and all presets persist', async () => { const { calls, finish } = await runFactory([], true, matchingIssue, { ...completed, workflow: 'simple' }); - expect(calls.filter(call => /:(claude|codex)$/.test(call))).toEqual(['implementer:claude']); + // The only agent besides the implementer works out how to run the checks. + expect(calls.filter(call => /:(claude|codex)$/.test(call))).toEqual(['check-discovery:claude', 'implementer:claude']); expect(calls).not.toContain('human'); expect(finish).toBe('needs_human'); for (const workflow of ['traditional', 'prototype', 'simple'] as const) { diff --git a/web/lib/test/flow-workflows.test.ts b/web/lib/test/flow-workflows.test.ts index cbae136..e37ea27 100644 --- a/web/lib/test/flow-workflows.test.ts +++ b/web/lib/test/flow-workflows.test.ts @@ -3,14 +3,19 @@ import { execFileSync, spawnSync } from 'node:child_process'; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; -import { FLOW_PUBLISH_CHECK_COMMAND, FLOW_REVIEW_BLOCKED_COMMAND, FLOW_TEST_COMMAND } from '../flow-workflows'; +import { + FLOW_BASE_CHECK_COMMAND, FLOW_CHECK_REPORT_COMMAND, FLOW_CHECK_RESOLVE_COMMAND, FLOW_CHECK_RUN_COMMAND, FLOW_CHECK_SCRIPT, + FLOW_DROP_WORKING_FILES_COMMAND, FLOW_EXCLUDE_WORKING_FILES_COMMAND, FLOW_PUBLISH_CHECK_COMMAND, FLOW_REVIEW_BLOCKED_COMMAND, +} from '../flow-workflows'; /** - * The generated test step runs under `sh`, and its exit code is the only thing - * the runner sees. `f.run` has no retry policy, so a non-zero exit is retried - * until the run dies with `retries_exhausted` — that is how a repository with - * no package.json killed production runs against AgentWorkforce/cloud-e2e-sandbox. - * These cases run the real command against real fixtures. + * Every generated check step runs under `sh`, and its exit code is the only + * thing the runner sees besides stdout. `f.run` has no retry policy, so a + * non-zero exit is retried until the run dies with `retries_exhausted` — that + * is how a repository with no package.json killed production runs against + * AgentWorkforce/cloud-e2e-sandbox, and how AgentWorkforce/cloud run acbe30c1 + * died at a test step that never built what cloud's tests need. These cases run + * the real commands against real fixtures and real git repositories. */ const roots: string[] = []; afterAll(() => { for (const root of roots) rmSync(root, { recursive: true, force: true }); }); @@ -18,83 +23,166 @@ afterAll(() => { for (const root of roots) rmSync(root, { recursive: true, force function fixture(files: Record) { const root = mkdtempSync(path.join(tmpdir(), 'flow-test-command-')); roots.push(root); - for (const [name, content] of Object.entries(files)) writeFileSync(path.join(root, name), content); + for (const [name, content] of Object.entries(files)) { + mkdirSync(path.dirname(path.join(root, name)), { recursive: true }); + writeFileSync(path.join(root, name), content); + } return root; } -function runStep(files: Record, env: Record = {}) { - const result = spawnSync('/bin/sh', ['-c', FLOW_TEST_COMMAND], { - cwd: fixture(files), - encoding: 'utf8', - env: { ...process.env, ...env }, - }); - return { code: result.status, stdout: result.stdout, stderr: result.stderr }; +function sh(command: string, cwd: string, env: Record = {}) { + const result = spawnSync('/bin/sh', ['-c', command], { cwd, encoding: 'utf8', env: { ...process.env, ...env } }); + return { code: result.status, token: result.stdout.trim(), stdout: result.stdout, stderr: result.stderr }; } +const read = (root: string, name: string) => existsSync(path.join(root, name)) ? readFileSync(path.join(root, name), 'utf8') : ''; + // A package manager that is genuinely absent: only Node and the base system on PATH. const withoutPackageManagers = { PATH: `${path.dirname(process.execPath)}:/usr/bin:/bin` }; const testScript = (test: string) => JSON.stringify({ name: 'fixture', version: '1.0.0', scripts: { test } }); -describe('FLOW_TEST_COMMAND', () => { - it('skips instead of exiting 254 when the repository has no package.json', () => { - // The exact production failure: every npm subcommand reports ENOENT as errno - // -2, which npm returns as its exit status, and -2 & 0xFF is 254. - const { code, stdout } = runStep({ 'README.md': '# sandbox\n' }); +function resolveIn(files: Record) { + const root = fixture(files); + const result = sh(FLOW_CHECK_RESOLVE_COMMAND, root); + return { ...result, root, script: read(root, FLOW_CHECK_SCRIPT) }; +} + +describe('FLOW_CHECK_RESOLVE_COMMAND', () => { + it('keeps a check script the author, the repository or discovery already wrote', () => { + const { code, token, script } = resolveIn({ [FLOW_CHECK_SCRIPT]: 'npm run build:core\nnpm test\n', 'package.json': testScript('echo x') }); + expect(code).toBe(0); + expect(token).toBe('script'); + // Untouched: this is where cloud's missing `@cloud/core` build step lives. + expect(script).toBe('npm run build:core\nnpm test\n'); + }); + + it('prefers the repository\'s own test target to an ecosystem default', () => { + const { token, script } = resolveIn({ Makefile: 'build:\n\ttrue\ntest: build\n\techo ok\n', 'package.json': testScript('echo x') }); + expect(token).toBe('default'); + expect(script).toContain('make test'); + expect(script).not.toContain('npm'); + }); + + it('writes the Node default, choosing the package manager from the lockfile', () => { + const { token, script } = resolveIn({ 'package.json': testScript('echo x'), 'package-lock.json': '{}' }); + expect(token).toBe('default'); + expect(script.split('\n')[1]).toBe('set -e'); + expect(script).toContain('pnpm install --frozen-lockfile'); + expect(script).toContain('yarn install --immutable'); + expect(script).toContain('yarn install --frozen-lockfile'); + expect(script).toContain('bun install --frozen-lockfile'); + expect(script).toContain('corepack enable --install-directory'); + // npm ci only with a lockfile present; the old `npm ci || npm install` + // chain masked npm ci failures behind a second install. + expect(script).toContain('if [ -f package-lock.json ]; then npm ci; else npm install; fi'); + expect(script).not.toContain('npm ci || npm install'); + }); + + it.each([ + [{ 'Cargo.toml': '[package]\nname = "x"\n' }, 'cargo test'], + [{ 'go.mod': 'module example.com/x\n' }, 'go test ./...'], + [{ 'pyproject.toml': '[project]\nname = "x"\n', 'uv.lock': '' }, 'uv run pytest'], + [{ 'pyproject.toml': '[tool.poetry]\n', 'poetry.lock': '' }, 'poetry install --no-interaction && poetry run pytest'], + [{ 'requirements.txt': 'pytest\n' }, 'python3 -m pip install -r requirements.txt && python3 -m pytest'], + [{ Gemfile: 'source "https://rubygems.org"\n', 'spec/x_spec.rb': '' }, 'bundle install && bundle exec rspec'], + [{ Gemfile: 'source "https://rubygems.org"\n' }, 'bundle install && bundle exec rake test'], + [{ 'pom.xml': '\n' }, 'mvn -B test'], + [{ 'build.gradle.kts': '' }, 'gradle test'], + [{ 'App.csproj': '\n' }, 'dotnet test'], + [{ 'mix.exs': 'defmodule X.MixProject do\nend\n' }, 'mix deps.get && mix test'], + ])('writes a default for a repository that is not JavaScript (%o)', (files, command) => { + // The old step said "no package.json; skipping tests" to every one of these. + const { code, token, script } = resolveIn(files); expect(code).toBe(0); - expect(code).not.toBe(254); - expect(stdout).toContain('no package.json in the repository root; skipping tests.'); + expect(token).toBe('default'); + expect(script.trim().split('\n')).toEqual([expect.stringContaining('# Written by relayflow'), 'set -e', command]); }); - it('skips when package.json declares no runnable test script', () => { + it('reports none, and writes nothing, when there is nothing to run', () => { for (const files of [ + { 'README.md': '# sandbox\n' } as Record, { 'package.json': JSON.stringify({ name: 'fixture', version: '1.0.0' }) }, { 'package.json': testScript(' ') }, { 'package.json': '{ not valid json' }, ]) { - const { code, stdout } = runStep(files); + const { code, token, script, stderr } = resolveIn(files); expect(code).toBe(0); - expect(stdout).toContain('no runnable test script; skipping tests.'); + expect(token).toBe('none'); + expect(script).toBe(''); + expect(stderr).toContain('found no way to run this repository\'s tests'); } }); +}); - it('always reports what it did, so the journal never records an empty stdout_tail', () => { - const cases: Record[] = [{ 'README.md': '#\n' }, { 'package.json': testScript('echo ran') }]; - for (const files of cases) { - expect(runStep(files).stdout.trim()).not.toBe(''); - } +function runChecksIn(root: string, env: Record = {}) { + const result = sh(FLOW_CHECK_RUN_COMMAND, root, env); + return { ...result, log: read(root, '.relayflow/check.log') }; +} + +describe('FLOW_CHECK_RUN_COMMAND', () => { + it('reports pass, fail and none with exit 0, and keeps the output in the log', () => { + const passing = runChecksIn(fixture({ [FLOW_CHECK_SCRIPT]: 'echo suite-ran\n' })); + expect(passing).toMatchObject({ code: 0, token: 'pass' }); + expect(passing.log).toContain('suite-ran'); + + // A genuine failure stays a failure — it is just no longer the end of the run. + const failing = runChecksIn(fixture({ [FLOW_CHECK_SCRIPT]: 'echo broke\nexit 3\n' })); + expect(failing).toMatchObject({ code: 0, token: 'fail' }); + expect(failing.log).toContain('broke'); + // The journal still shows why: the tail and the exit status go to stderr. + expect(failing.stderr).toContain('broke'); + expect(failing.stderr).toContain('exit 3'); + + const nothing = runChecksIn(fixture({ 'README.md': '#\n' })); + expect(nothing).toMatchObject({ code: 0, token: 'none' }); }); - it('still runs and still fails on a real test suite', () => { - const passing = runStep({ 'package.json': testScript('echo suite-ran') }); - expect(passing.code).toBe(0); - expect(passing.stdout).toContain('running tests with npm'); - expect(passing.stdout).toContain('suite-ran'); + it('prints exactly one token on stdout, whatever the tests print', () => { + const { stdout } = runChecksIn(fixture({ [FLOW_CHECK_SCRIPT]: 'echo lots\necho of\necho output\n' })); + expect(stdout.trim().split('\n')).toEqual(['pass']); + }); - // A genuine failure must stay a failure: skipping is only for "nothing to test". - const failing = runStep({ 'package.json': testScript('echo broke && exit 3') }); - expect(failing.code).toBe(3); + it('does not hand Cloud\'s own Git configuration to the repository\'s tests', () => { + // AgentWorkforce/relay run 139d1a46: sandbox-repo.test.ts failed with + // `fatal: transport 'file' not allowed` inside the flow, and passed in a + // clean shell of the same sandbox. Cloud's executor sets these for its own + // clone and push, and they reached the tests. + const cloudGit = { + GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null', + GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'protocol.allow', GIT_CONFIG_VALUE_0: 'never', + }; + const probe = 'git config --get protocol.allow || echo protocol-unset\n'; + // Positive control: the same probe does see the setting when run directly. + expect(sh(probe, fixture({}), cloudGit).stdout).toContain('never'); + const { token, log } = runChecksIn(fixture({ [FLOW_CHECK_SCRIPT]: probe }), cloudGit); + expect(token).toBe('pass'); + expect(log).toContain('protocol-unset'); + expect(log).not.toContain('never'); }); - it('fails once, loudly and legibly, when the lockfile needs a package manager that cannot be provisioned', () => { - const { code, stderr } = runStep( - { 'bun.lockb': '', 'package.json': testScript('echo ok') }, - withoutPackageManagers, - ); - expect(code).toBe(1); - expect(code).not.toBe(254); - expect(stderr).toContain("lockfile requires bun, which is not installed and could not be provisioned."); + it.skipIf(spawnSync('/bin/sh', ['-c', 'command -v timeout']).status !== 0)('stops itself and says timeout before the 15-minute lease would kill the run', () => { + const { code, token } = runChecksIn(fixture({ [FLOW_CHECK_SCRIPT]: 'sleep 5\n' }), { RELAYFLOW_CHECK_TIMEOUT: '1' }); + expect(code).toBe(0); + expect(token).toBe('timeout'); }); - it('selects the package manager from the lockfile and never blind-falls back to npm', () => { - expect(FLOW_TEST_COMMAND).toContain('pnpm install --frozen-lockfile'); - expect(FLOW_TEST_COMMAND).toContain('yarn install --immutable'); - expect(FLOW_TEST_COMMAND).toContain('yarn install --frozen-lockfile'); - expect(FLOW_TEST_COMMAND).toContain('bun install --frozen-lockfile'); - expect(FLOW_TEST_COMMAND).toContain('corepack enable --install-directory'); - // npm ci only with a lockfile present; the old `npm ci || npm install` - // chain masked npm ci failures behind a second install. - expect(FLOW_TEST_COMMAND).toContain('if [ -f package-lock.json ]; then npm ci; else npm install; fi'); - expect(FLOW_TEST_COMMAND).not.toContain('npm ci || npm install'); + it('runs the resolved default end to end, and still fails a failing suite', () => { + const run = (files: Record, env: Record = {}) => { + const root = fixture(files); + expect(sh(FLOW_CHECK_RESOLVE_COMMAND, root).token).toBe('default'); + return runChecksIn(root, env); + }; + const passing = run({ 'package.json': testScript('echo suite-ran') }); + expect(passing.token).toBe('pass'); + expect(passing.log).toContain('running tests with npm'); + expect(passing.log).toContain('suite-ran'); + expect(run({ 'package.json': testScript('echo broke && exit 3') }).token).toBe('fail'); + // The lockfile needs a package manager that cannot be provisioned: a + // failure, legibly explained, and never npm's ENOENT exit status 254. + const bun = run({ 'bun.lockb': '', 'package.json': testScript('echo ok') }, withoutPackageManagers); + expect(bun.code).toBe(0); + expect(bun.token).toBe('fail'); + expect(bun.log).toContain('lockfile requires bun, which is not installed and could not be provisioned.'); }); }); @@ -266,3 +354,179 @@ describe('FLOW_PUBLISH_CHECK_COMMAND', () => { } }); }); + +/** A real repository with one commit per entry; returns each commit's id. */ +function history(commits: Record[]) { + const root = fixture({}); + git(root, 'init', '-q', '.'); + const ids: string[] = []; + for (const files of commits) { + for (const [name, content] of Object.entries(files)) { + mkdirSync(path.dirname(path.join(root, name)), { recursive: true }); + writeFileSync(path.join(root, name), content); + } + git(root, 'add', '-A'); + git(root, 'commit', '-q', '--allow-empty', '-m', `commit ${ids.length}`); + ids.push(git(root, 'rev-parse', 'HEAD').trim()); + } + return { root, ids }; +} +const treeOf = (root: string, ref = 'HEAD') => git(root, 'ls-tree', '-r', '--name-only', ref).trim().split('\n').filter(Boolean); + +describe('FLOW_BASE_CHECK_COMMAND', () => { + // The check reads a file the change edits; the script itself stays in + // .relayflow/, outside the history, exactly as in a real run. + function compare(baseState: string, headState: string) { + const { root, ids } = history([{ state: baseState }, { state: headState }]); + mkdirSync(path.join(root, '.relayflow'), { recursive: true }); + writeFileSync(path.join(root, FLOW_CHECK_SCRIPT), 'echo "state is $(cat state)"\ngrep -q good state\n'); + return { root, base: ids[0] }; + } + + it('tells a change that broke the checks from one that found them broken', () => { + const broke = compare('good', 'bad'); + expect(sh(FLOW_CHECK_RUN_COMMAND, broke.root).token).toBe('fail'); + const regression = sh(`base=${broke.base}; ${FLOW_BASE_CHECK_COMMAND}`, broke.root); + expect(regression).toMatchObject({ code: 0, token: 'pass' }); + + const already = compare('bad', 'bad'); + const preexisting = sh(`base=${already.base}; ${FLOW_BASE_CHECK_COMMAND}`, already.root); + expect(preexisting).toMatchObject({ code: 0, token: 'fail' }); + // The base commit's own output, not the branch's, from its own worktree. + expect(read(already.root, '.relayflow/base-check.log')).toContain('state is bad'); + expect(read(broke.root, '.relayflow/base-check.log')).toContain('state is good'); + }); + + it('cleans up its worktree and leaves the branch alone', () => { + const { root, base } = compare('good', 'bad'); + const head = git(root, 'rev-parse', 'HEAD').trim(); + sh(`base=${base}; ${FLOW_BASE_CHECK_COMMAND}`, root); + expect(git(root, 'worktree', 'list').trim().split('\n')).toHaveLength(1); + expect(git(root, 'rev-parse', 'HEAD').trim()).toBe(head); + expect(read(root, 'state')).toBe('bad'); + }); + + it('says unknown, with exit 0, when the base commit cannot be checked out', () => { + const { root } = compare('good', 'bad'); + for (const base of ['', 'deadbeef'.repeat(5)]) { + expect(sh(`base=${base}; ${FLOW_BASE_CHECK_COMMAND}`, root)).toMatchObject({ code: 0, token: 'unknown' }); + } + }); +}); + +describe('FLOW_EXCLUDE_WORKING_FILES_COMMAND', () => { + it('keeps working files out of `git add -A` without touching the change', () => { + const { root } = history([{ 'README.md': '#\n' }]); + expect(sh(FLOW_EXCLUDE_WORKING_FILES_COMMAND, root).code).toBe(0); + sh(FLOW_EXCLUDE_WORKING_FILES_COMMAND, root); + for (const [name, content] of Object.entries({ 'summary.md': 's', 'plan.md': 'p', [FLOW_CHECK_SCRIPT]: 'true', 'docs/summary.md': 'real docs', 'src.txt': 'change' })) { + mkdirSync(path.dirname(path.join(root, name)), { recursive: true }); + writeFileSync(path.join(root, name), content); + } + git(root, 'add', '-A'); + const staged = git(root, 'diff', '--cached', '--name-only').trim().split('\n'); + expect(staged.sort()).toEqual(['docs/summary.md', 'src.txt']); + // Written once, however many times the flow starts in this clone. + expect(read(root, '.git/info/exclude').match(/# relayflow working files/g)).toHaveLength(1); + }); +}); + +describe('FLOW_DROP_WORKING_FILES_COMMAND', () => { + const drop = (root: string, base: string, env: Record = {}) => sh(`base=${base}; ${FLOW_DROP_WORKING_FILES_COMMAND}`, root, env); + + it('takes the working files an agent committed out of the branch, and keeps the change', () => { + // Both production runs: Codex committed summary.md into cloud (acbe30c1) + // and relay (139d1a46), so the pull-request body would have shipped as a + // file at the repository root. + const { root, ids } = history([{ 'README.md': '#\n' }, { 'summary.md': 'PR body', 'plan.md': 'plan', [FLOW_CHECK_SCRIPT]: 'true', 'src.txt': 'change' }]); + expect(drop(root, ids[0]).code).toBe(0); + expect(treeOf(root).sort()).toEqual(['README.md', 'src.txt']); + // The agent's commit is kept; the removal is a new commit on top of it. + expect(git(root, 'rev-parse', 'HEAD~1').trim()).toBe(ids[1]); + // Still on disk, where the pull-request body is read from, and untracked. + expect(read(root, 'summary.md')).toBe('PR body'); + expect(git(root, 'status', '--porcelain', '--', 'summary.md').trim()).toBe('?? summary.md'); + }); + + it('keeps a summary.md the repository already had, and the agent\'s edit to it', () => { + const { root, ids } = history([{ 'summary.md': 'the project summary' }, { 'summary.md': 'the project summary, updated', 'plan.md': 'plan' }]); + drop(root, ids[0]); + expect(treeOf(root)).toEqual(['summary.md']); + expect(git(root, 'show', 'HEAD:summary.md')).toBe('the project summary, updated'); + }); + + it('adds no commit when there is nothing to drop, or when the base is unknown', () => { + const { root, ids } = history([{ 'README.md': '#\n' }, { 'src.txt': 'change', 'summary.md': 'body' }]); + const clean = history([{ 'README.md': '#\n' }, { 'src.txt': 'change' }]); + expect(drop(clean.root, clean.ids[0]).code).toBe(0); + expect(git(clean.root, 'rev-parse', 'HEAD').trim()).toBe(clean.ids[1]); + for (const base of ['', 'deadbeef'.repeat(5)]) { + expect(drop(root, base).code).toBe(0); + expect(git(root, 'rev-parse', 'HEAD').trim()).toBe(ids[1]); + } + }); + + it('never sweeps staged but uncommitted work into its commit', () => { + const { root, ids } = history([{ 'README.md': '#\n' }, { 'summary.md': 'body', 'src.txt': 'change' }]); + writeFileSync(path.join(root, 'staged.txt'), 'not committed'); + git(root, 'add', 'staged.txt'); + drop(root, ids[0]); + expect(treeOf(root)).not.toContain('staged.txt'); + expect(git(root, 'diff', '--cached', '--name-only').trim()).toBe('staged.txt'); + }); + + it('commits as Relayflow only when the clone has no identity of its own', () => { + const { root, ids } = history([{ 'README.md': '#\n' }, { 'summary.md': 'body', 'src.txt': 'change' }]); + const home = fixture({}); + // No identity anywhere: no config file, no identity variables, and + // user.useConfigOnly so git cannot guess one from the host name. + const env: NodeJS.ProcessEnv = { ...process.env, HOME: home, XDG_CONFIG_HOME: home, GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'user.useConfigOnly', GIT_CONFIG_VALUE_0: 'true' }; + for (const key of Object.keys(env)) if (/^(GIT_(AUTHOR|COMMITTER)_|EMAIL$)/.test(key)) delete env[key]; + const result = spawnSync('/bin/sh', ['-c', `base=${ids[0]}; ${FLOW_DROP_WORKING_FILES_COMMAND}`], { cwd: root, encoding: 'utf8', env }); + expect(result.status).toBe(0); + expect(treeOf(root)).toEqual(['README.md', 'src.txt']); + expect(git(root, 'log', '-1', '--format=%an <%ae>').trim()).toBe('Relayflow '); + }); +}); + +describe('FLOW_CHECK_REPORT_COMMAND', () => { + function report(check: string, baseline: string, files: Record = {}) { + const root = fixture({ 'summary.md': '## What changed\n\nFixed the login bug.\n', [FLOW_CHECK_SCRIPT]: 'set -e\nnpm test\n', '.relayflow/check.log': 'FAIL src/login.test.ts\n', '.relayflow/base-check.log': 'FAIL src/other.test.ts\n', ...files }); + const result = sh(`check=${check}; baseline=${baseline}; ${FLOW_CHECK_REPORT_COMMAND}`, root); + return { ...result, body: read(root, '.relayflow/pr-body.md'), report: read(root, '.relayflow/check-report.md') }; + } + + it('puts the verdict, what ran and the output under the summary', () => { + const passed = report('pass', ''); + expect(passed.code).toBe(0); + expect(passed.body.startsWith('## What changed')).toBe(true); + expect(passed.body).toContain('## Checks'); + expect(passed.body).toContain('and they passed'); + expect(passed.body).toContain('npm test'); + expect(passed.body).not.toContain('FAIL src/login.test.ts'); + + const preexisting = report('fail', 'fail'); + expect(preexisting.body).toContain('fail on the base commit too'); + expect(preexisting.body).toContain('FAIL src/login.test.ts'); + expect(preexisting.body).toContain('FAIL src/other.test.ts'); + + const regression = report('fail', 'pass'); + expect(regression.body).toContain('breaks checks that pass on the base commit'); + expect(regression.body).not.toContain('FAIL src/other.test.ts'); + + expect(report('timeout', 'unknown').body).toContain('could not be checked for comparison'); + expect(report('fail', 'revision').report).toContain('latest revision breaks checks that passed before it'); + expect(report('fail', 'fail', { '.relayflow/repair-notes.md': 'cargo is not installed.\n' }).body).toContain('cargo is not installed.'); + }); + + it('says plainly when nothing could be checked', () => { + const { body } = report('none', '', { [FLOW_CHECK_SCRIPT]: '' }); + expect(body).toContain('no checks ran'); + }); + + it('still writes a report with no summary.md', () => { + const root = fixture({}); + expect(sh(`check=pass; baseline=; ${FLOW_CHECK_REPORT_COMMAND}`, root).code).toBe(0); + expect(read(root, '.relayflow/pr-body.md').startsWith('## Checks')).toBe(true); + }); +}); From 5814e6e8209915b45b7000bf6a9a0be34f22df96 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 18 Sep 2026 13:57:54 -0700 Subject: [PATCH 2/8] fix(flows): look for checks again when the change adds the first test setup Check discovery ran once, before the implementer. On a repository with no way to test itself it resolved "none", and a change that added the first package.json and test script shipped without any check running: dev run 53f1bc98 opened AgentWorkforce/cloud-e2e-sandbox#31 with tests the flow never ran ("no checks ran"). A "none" is now resolved again after the implementer; any other answer is kept, so the base commit and the branch are still checked the same way. Co-Authored-By: Claude Opus 5 (1M context) --- web/lib/flow-workflows.ts | 10 ++++++++-- web/lib/test/flow-onboarding.test.ts | 11 +++++++++++ web/lib/test/flow-workflows.test.ts | 9 +++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/web/lib/flow-workflows.ts b/web/lib/flow-workflows.ts index c68dc2f..9d2591b 100644 --- a/web/lib/flow-workflows.ts +++ b/web/lib/flow-workflows.ts @@ -417,7 +417,8 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType { expect(readFactoryDraft(JSON.stringify({ ...unanswered, workflow: 'prototype' }))?.workflow).toBe('prototype'); }); + it('resolves checks again after the implementer when none were found before it', () => { + for (const workflow of ['simple', 'traditional', 'prototype'] as const) { + const source = factorySource({ ...DEFAULT_FACTORY, sources: ['github'], agents: ['claude'], workflow, step: 3 }); + const implementer = source.indexOf('f.agent("implementer"'); + const again = source.indexOf('if (checkPlan === "none") await f.run(resolveChecks);'); + expect(implementer, workflow).toBeGreaterThan(-1); + expect(again, workflow).toBeGreaterThan(implementer); + expect(source.indexOf('await f.run(runChecks'), workflow).toBeGreaterThan(again); + } + }); + it('rejects corrupt and unsupported persisted drafts', () => { for (const raw of [null, '{', '{}', '{"version":1,"agent":"shell","rounds":3}', '{"version":1,"agent":"claude","rounds":999}']) { expect(readFactoryDraft(raw)).toBeNull(); diff --git a/web/lib/test/flow-workflows.test.ts b/web/lib/test/flow-workflows.test.ts index e37ea27..4268710 100644 --- a/web/lib/test/flow-workflows.test.ts +++ b/web/lib/test/flow-workflows.test.ts @@ -48,6 +48,15 @@ function resolveIn(files: Record) { } describe('FLOW_CHECK_RESOLVE_COMMAND', () => { + it('finds tests a change added to a repository that had none (cloud-e2e-sandbox#31)', () => { + const root = fixture({ 'README.md': '# sandbox\n' }); + expect(sh(FLOW_CHECK_RESOLVE_COMMAND, root).token).toBe('none'); + // The implementer adds the first package.json with a test script. + writeFileSync(path.join(root, 'package.json'), testScript('node --test')); + expect(sh(FLOW_CHECK_RESOLVE_COMMAND, root).token).toBe('default'); + expect(read(root, FLOW_CHECK_SCRIPT)).toContain('test'); + }); + it('keeps a check script the author, the repository or discovery already wrote', () => { const { code, token, script } = resolveIn({ [FLOW_CHECK_SCRIPT]: 'npm run build:core\nnpm test\n', 'package.json': testScript('echo x') }); expect(code).toBe(0); From af387c41569cdc8eb3bb1e0689096ebe40bd90f8 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 18 Sep 2026 15:03:14 -0700 Subject: [PATCH 3/8] feat(flows): give generated flows a two-hour wall-clock budget A software-factory run against a repository the size of AgentWorkforce/cloud spends most of an hour in the implementer before its CI-equivalent checks even start (dev run af3069c9: implementer still working at 39 minutes of 60, checks not started). One hour leaves no room for checks, repair and a base comparison. Cloud's own run ceiling is being raised to match in a separate AgentWorkforce/cloud PR; until that lands the hosted run is still cut at its ~55-minute cloud limit. Co-Authored-By: Claude Opus 5 (1M context) --- web/app/flows/onboarding/RunOptions.tsx | 2 +- web/lib/flow-local.ts | 2 +- web/lib/flow-onboarding.ts | 2 +- web/lib/test/flow-local.test.ts | 2 +- web/lib/test/flow-onboarding.test.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/web/app/flows/onboarding/RunOptions.tsx b/web/app/flows/onboarding/RunOptions.tsx index 626ba1a..0907d5e 100644 --- a/web/app/flows/onboarding/RunOptions.tsx +++ b/web/app/flows/onboarding/RunOptions.tsx @@ -79,7 +79,7 @@ export function RunOptions({ draft, chosen, setDestination, onTrack, getJourneyI
    onTrack('help_toggled', { section: 'local_requirements', open: event.currentTarget.open })}>Requirements and local behavior

    Node.js 22.18+, macOS (Apple silicon) or Linux (x64), your selected coding agents installed and signed in, and GitHub CLI authenticated. Start with a clean Git repository whose tests run the way its CI runs them; the flow reads your CI configuration to find the command, or you can set checkCommand in the flow.

    -

    The local version uses a one-hour runtime limit, not a dollar cap. Your coding agent’s usage charges still apply. Every preset stops at “needs_human” for you to review and merge the PR in GitHub.

    +

    The local version uses a two-hour runtime limit, not a dollar cap. Your coding agent’s usage charges still apply. Every preset stops at “needs_human” for you to review and merge the PR in GitHub.

    In GitHub, require approving reviews and passing CI checks in your target branch’s rules. A repository administrator needs to configure these protections.

    This runs one ticket. Automatic triggers from your issue tracker require a separate connection.

    diff --git a/web/lib/flow-local.ts b/web/lib/flow-local.ts index 3527f21..be8f4df 100644 --- a/web/lib/flow-local.ts +++ b/web/lib/flow-local.ts @@ -446,7 +446,7 @@ If the agents commit nothing — a ticket with nothing to do in this repository If the checks fail, a repair agent reads the output and fixes missing setup or its own bugs, never by weakening tests. Whatever still fails is compared with the commit the branch started from, and the pull request opens as a draft with both outputs in its body: the work is never thrown away. A change that breaks checks which pass on the starting commit ends as step_failed (exit code 1). Working files (summary.md, plans, reviews, .relayflow/) are kept out of the commits through .git/info/exclude, and removed from the branch before pushing if an agent committed them anyway. Local runtime behavior -This local version uses a one-hour wall-clock budget. Model usage is billed by your coding-agent provider; this is not a dollar cap. +This local version uses a two-hour wall-clock budget. Model usage is billed by your coding-agent provider; this is not a dollar cap. Every preset reports needs_human (exit code 3) after its checks and any agent reviews pass. This is the intended manual approval stop, not a failed run. Review and merge the PR in GitHub; the flow never merges automatically and does not resume automatically after approval. A run whose adversarial review does not pass ends as step_failed (exit code 1) instead. Its findings are written to review-blocked.md and posted to the pull request, and the pull request is left as a draft so it cannot be merged by accident. ${draft.workflow === 'prototype' ? 'Prototype worktrees remain available under the generated temporary directory for inspection.' : ''} diff --git a/web/lib/flow-onboarding.ts b/web/lib/flow-onboarding.ts index a1abd5e..b8bce45 100644 --- a/web/lib/flow-onboarding.ts +++ b/web/lib/flow-onboarding.ts @@ -102,7 +102,7 @@ export function factoryCodeSections(draft: FactoryDraft, target: 'cloud' | 'loca // such a step runs unmetered, so a dollar cap cannot bound it. Wall-clock is // enforced on every step regardless of pricing, which is why it stays the // default here; `{ dollars, wallclock }` together is also valid. - const budget = '{ wallclock: "1h" }'; + const budget = '{ wallclock: "2h" }'; if (!draft.sources.length) return [{ id: 'empty', code: `import { flow } from "@relayflows/surface"; export default flow("software-factory", diff --git a/web/lib/test/flow-local.test.ts b/web/lib/test/flow-local.test.ts index a3ef154..4809bb5 100644 --- a/web/lib/test/flow-local.test.ts +++ b/web/lib/test/flow-local.test.ts @@ -341,7 +341,7 @@ describe('local flow starter kit', () => { const source = factorySource({ ...draft, workflow }, 'local'); const file = ts.createSourceFile('local.flow.ts', source, ts.ScriptTarget.ES2022, true); expect((file as unknown as { parseDiagnostics: unknown[] }).parseDiagnostics).toEqual([]); - expect(source).toContain('wallclock: "1h"'); + expect(source).toContain('wallclock: "2h"'); expect(source).not.toContain('$8/run'); expect(source).not.toContain('pr merge'); } diff --git a/web/lib/test/flow-onboarding.test.ts b/web/lib/test/flow-onboarding.test.ts index 46160c6..cd7c390 100644 --- a/web/lib/test/flow-onboarding.test.ts +++ b/web/lib/test/flow-onboarding.test.ts @@ -217,7 +217,7 @@ describe('software factory onboarding', () => { it('gives Cloud flows a wall-clock budget so unpriced agents are never refused', () => { for (const agents of [['claude', 'codex'], ['codex'], ['claude']] as FactoryDraft['agents'][]) { const source = factorySource({ ...completed, agents }); - expect(source).toContain('{ budget: { wallclock: "1h" } }'); + expect(source).toContain('{ budget: { wallclock: "2h" } }'); expect(source).not.toMatch(/budget: "\$\d/); } }); From 10f77395cdd5bef1d77e4f7511ec8887eaa48c5d Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 18 Sep 2026 16:09:12 -0700 Subject: [PATCH 4/8] feat(flows): GitLab as a ticket source and repository host in onboarding Adds GitLab to the flows onboarding so a GitLab flow can be set up, against the contract of AgentWorkforce/cloud#3800/#3801: - GitLab source with `project` ("namespace/project") and `labels`, the fields Cloud's deploy handoff reads from sourceSettings.gitlab; Cloud makes the project the deploy target when no GitHub source is chosen, and `repositoryHost` mirrors that rule for the preview and picker icons. - The generated flow opens its change through `relayflow-open-change` when Cloud puts it on PATH (gh pr create on GitHub, a merge request on GitLab) and falls back to `gh pr create` for local runs. - Local kit: the preflight stops a GitLab origin before any agent runs, pointing at the Cloud deploy, since a local run only has gh. Co-Authored-By: Claude Opus 5 (1M context) --- web/app/flows/onboarding/RunOptions.tsx | 2 +- web/app/flows/onboarding/SourcePicker.tsx | 4 +- web/app/flows/onboarding/WorkflowPicker.tsx | 6 +- .../flows/onboarding/onboarding.module.css | 1 + web/lib/flow-local.ts | 14 +++- web/lib/flow-preview.ts | 4 +- web/lib/flow-sources.ts | 16 ++++ web/lib/flow-workflows.ts | 17 +++- web/lib/test/flow-gitlab-source.test.ts | 77 +++++++++++++++++++ web/lib/test/flow-local.test.ts | 20 ++++- web/lib/test/flow-onboarding.test.ts | 16 ++-- web/lib/test/flow-workflows.test.ts | 36 ++++++++- 12 files changed, 189 insertions(+), 24 deletions(-) create mode 100644 web/lib/test/flow-gitlab-source.test.ts diff --git a/web/app/flows/onboarding/RunOptions.tsx b/web/app/flows/onboarding/RunOptions.tsx index 0907d5e..d96e237 100644 --- a/web/app/flows/onboarding/RunOptions.tsx +++ b/web/app/flows/onboarding/RunOptions.tsx @@ -78,7 +78,7 @@ export function RunOptions({ draft, chosen, setDestination, onTrack, getJourneyI
  • Check and run on a new branch{command(LOCAL_RUN, 'run commands')}
    • onTrack('help_toggled', { section: 'local_requirements', open: event.currentTarget.open })}>Requirements and local behavior -

    Node.js 22.18+, macOS (Apple silicon) or Linux (x64), your selected coding agents installed and signed in, and GitHub CLI authenticated. Start with a clean Git repository whose tests run the way its CI runs them; the flow reads your CI configuration to find the command, or you can set checkCommand in the flow.

    +

    Node.js 22.18+, macOS (Apple silicon) or Linux (x64), your selected coding agents installed and signed in, and GitHub CLI authenticated (GitLab repositories need the Cloud deploy: a local run opens its pull request with GitHub CLI). Start with a clean Git repository whose tests run the way its CI runs them; the flow reads your CI configuration to find the command, or you can set checkCommand in the flow.

    The local version uses a two-hour runtime limit, not a dollar cap. Your coding agent’s usage charges still apply. Every preset stops at “needs_human” for you to review and merge the PR in GitHub.

    In GitHub, require approving reviews and passing CI checks in your target branch’s rules. A repository administrator needs to configure these protections.

    This runs one ticket. Automatic triggers from your issue tracker require a separate connection.

    diff --git a/web/app/flows/onboarding/SourcePicker.tsx b/web/app/flows/onboarding/SourcePicker.tsx index 108ec8e..4a13946 100644 --- a/web/app/flows/onboarding/SourcePicker.tsx +++ b/web/app/flows/onboarding/SourcePicker.tsx @@ -2,7 +2,7 @@ import { useState } from 'react'; import { ListFilter, ChevronDown } from 'lucide-react'; -import { SiGithub, SiLinear, SiShortcut, SiJira, SiMarkdown } from 'react-icons/si'; +import { SiGithub, SiGitlab, SiLinear, SiShortcut, SiJira, SiMarkdown } from 'react-icons/si'; import { ISSUE_SOURCES, sourceLabel, type IssueSourceId, type SourceSettings } from '../../../lib/flow-sources'; import type { FactoryDraft } from '../../../lib/flow-onboarding'; import type { FlowTrack } from '../../../lib/flow-analytics'; @@ -10,7 +10,7 @@ import s from './onboarding.module.css'; export function SourceIcon({ id }: { id: IssueSourceId }) { if (id === 'slack') return ; - const Icon = { github: SiGithub, linear: SiLinear, shortcut: SiShortcut, jira: SiJira, markdown: SiMarkdown }[id]; + const Icon = { github: SiGithub, gitlab: SiGitlab, linear: SiLinear, shortcut: SiShortcut, jira: SiJira, markdown: SiMarkdown }[id]; return