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..0907d5e 100644 --- a/web/app/flows/onboarding/RunOptions.tsx +++ b/web/app/flows/onboarding/RunOptions.tsx @@ -78,8 +78,8 @@ 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.

    -

    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.

    +

    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 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-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..be8f4df 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,9 +443,10 @@ ${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. +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/flow-workflows.ts b/web/lib/flow-workflows.ts index 7b85b8f..321f443 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,274 @@ 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 its arguments in their own process group and stops the group after `$1` seconds, exiting 124. */ +const PERL_LIMITER = + "my $l = shift; my $p = fork; defined $p or exit 127; if (!$p) { setpgrp(0, 0); exec @ARGV or exit 127 } $SIG{ALRM} = sub { kill 'TERM', -$p; sleep 2; kill 'KILL', -$p; exit 124 }; alarm $l; waitpid($p, 0); exit($? & 127 ? 128 + ($? & 127) : $? >> 8)"; + +/** + * 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 (14 minutes by default) and reports `timeout` + * instead of taking the run down. It uses `timeout` or `gtimeout` when one is + * installed and otherwise a small Perl limiter: macOS, the local kit's main + * platform, ships neither `timeout` nor `gtimeout`, and without a limiter a + * hung check ran into the lease and failed the run before anything was pushed. + * Every limiter stops the check's whole process group and exits 124. 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="\${check_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}"; limiter=' + + '; if command -v timeout >/dev/null 2>&1; then limiter=timeout; elif command -v gtimeout >/dev/null 2>&1; then limiter=gtimeout; elif command -v perl >/dev/null 2>&1; then limiter=perl; fi' + + `; run_limited() { case "$limiter" in (timeout|gtimeout) "$limiter" "$limit" "$@" ;; (perl) perl -e ${shq(PERL_LIMITER)} "$limit" "$@" ;; (*) "$@" ;; esac; }` + + '; echo "relayflow: running $script in $check_dir" >&2' + + '; ( cd "$check_dir" && unset GIT_CONFIG_COUNT GIT_CONFIG_GLOBAL GIT_CONFIG_NOSYSTEM && run_limited 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 "$limiter" ] && [ "$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. + * + * Both sides run the branch's recipe: the script is copied aside before the + * base is checked out, because a repository may commit .relayflow/check.sh and + * the checkout would otherwise replace or delete it. + * + * The base commit is checked in the same working tree when it can be: the + * branch's checks ran in a tree the implementer had already built in, and a + * default such as `make test` or `python3 -m pytest` does not install or + * generate anything, so a fresh worktree of the base could fail for missing + * setup while the branch failed on a real test — and a regression would read + * as pre-existing. Ignored build products (dependencies, generated files) stay + * where they are across the checkout. The branch is restored afterwards, by + * name, and anything the base run changed in tracked files is discarded. A + * tree with uncommitted changes to tracked files is never switched; it falls + * back to a throwaway worktree. + */ +export const FLOW_BASE_CHECK_COMMAND = [ + 'root="$PWD"', + 'tmp=', + 'if [ -z "$base" ] || ! git rev-parse --verify --quiet "$base^{commit}" >/dev/null 2>&1; then echo "relayflow: could not check out the base commit to compare against." >&2; echo unknown' + + `; elif git diff --quiet HEAD -- >/dev/null 2>&1 && git diff --cached --quiet >/dev/null 2>&1 && head=$(git rev-parse HEAD) && orig=$(git symbolic-ref -q --short HEAD || git rev-parse HEAD) && recipe=$(mktemp "\${TMPDIR:-/tmp}/relayflow-recipe.XXXXXX") && { [ ! -f "$root/${FLOW_CHECK_SCRIPT}" ] || cp "$root/${FLOW_CHECK_SCRIPT}" "$recipe"; } && git checkout -q --detach "$base" >/dev/null 2>&1; then ` + + 'check_dir="$root"; check_out="$root/.relayflow/base-check.log"; check_script="$recipe"' + + '; token=$( ' + FLOW_CHECK_RUN_COMMAND + ' )' + + '; rm -f "$recipe"' + + '; git checkout -q -f "$orig" >/dev/null 2>&1 || git checkout -q -f "$head" >/dev/null 2>&1' + + '; if [ "$(git rev-parse HEAD 2>/dev/null)" = "$head" ]; then echo "$token"; else echo "relayflow: could not return to $orig after checking the base commit." >&2; echo unknown; fi' + + '; elif if [ -n "${recipe:-}" ]; then rm -f "$recipe"; fi; 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." ;;` + + ` new) printf '%s\\n' "**The checks this change adds fail.** The repository had no checks before this change, so there is no base commit to compare with; the flow tried to repair them 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 +339,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 +400,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')} + }); + } + const resolveChecks = ${JSON.stringify(FLOW_CHECK_RESOLVE_COMMAND)}; + const checkPlan = (await f.run(resolveChecks)).trim(); + + // 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" });` }); + }); + // A repository with no way to test itself may have gained one in this + // change (a first package.json with a test script). Resolving only before + // the change reported "no checks ran" on a pull request that added tests + // (cloud-e2e-sandbox#31), so a "none" is looked at again. + if (checkPlan === "none") await f.run(resolveChecks);` }); + 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. + // Checks this change introduced (nothing to run before it) have no base to + // compare with: the base lacks the files they need and would always fail, + // which read as "pre-existing". Their failure is this change's own. + const baseline = !broken(check) + ? "" + : checkPlan === "none" + ? "new" + : (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 +516,26 @@ 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', () => { @@ -319,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 67bb235..53db066 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' }); @@ -64,6 +73,25 @@ describe('software factory onboarding', () => { expect(readFactoryDraft(JSON.stringify({ ...unanswered, workflow: 'prototype' }))?.workflow).toBe('prototype'); }); + it('treats failing checks the change itself introduced as the change\'s own failure, not pre-existing', () => { + for (const workflow of ['simple', 'traditional', 'prototype'] as const) { + const source = factorySource({ ...DEFAULT_FACTORY, sources: ['github'], agents: ['claude'], workflow, step: 3 }); + expect(source, workflow).toContain('checkPlan === "none"\n ? "new"'); + expect(source, workflow).toContain('(baseline === "pass" || baseline === "new")'); + } + }); + + 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(); @@ -197,7 +225,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/); } }); @@ -274,7 +302,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 +348,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 +382,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 +425,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..4da0684 100644 --- a/web/lib/test/flow-workflows.test.ts +++ b/web/lib/test/flow-workflows.test.ts @@ -1,16 +1,21 @@ import { afterAll, describe, expect, it } from 'vitest'; import { execFileSync, spawnSync } from 'node:child_process'; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, 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,195 @@ 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('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); - expect(code).not.toBe(254); - expect(stdout).toContain('no package.json in the repository root; skipping tests.'); + 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('skips when package.json declares no runnable test script', () => { + 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(token).toBe('default'); + expect(script.trim().split('\n')).toEqual([expect.stringContaining('# Written by relayflow'), 'set -e', command]); + }); + + 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'); } }); +}); + +function runChecksIn(root: string, env: Record = {}) { + const result = sh(FLOW_CHECK_RUN_COMMAND, root, env); + return { ...result, log: read(root, '.relayflow/check.log') }; +} - 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(''); +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('stops a hung check itself where neither timeout nor gtimeout is installed, as on macOS', () => { + // Only the tools the command needs, plus perl: no timeout, no gtimeout. + const bin = fixture({}); + const root0 = () => fixture({}); + for (const tool of ['sh', 'dirname', 'mkdir', 'tail', 'cat', 'sleep', 'perl']) { + const found = spawnSync('/bin/sh', ['-c', `command -v ${tool}`], { encoding: 'utf8' }).stdout.trim(); + if (found) symlinkSync(found, path.join(bin, tool)); } + expect(sh('command -v timeout || command -v gtimeout', root0(), { PATH: bin }).stdout.trim()).toBe(''); + const root = fixture({ [FLOW_CHECK_SCRIPT]: 'sleep 30 &\necho $! > .relayflow/child.pid\nwait\n' }); + const started = Date.now(); + const result = sh(FLOW_CHECK_RUN_COMMAND, root, { PATH: bin, RELAYFLOW_CHECK_TIMEOUT: '1' }); + expect(result).toMatchObject({ code: 0, token: 'timeout' }); + expect(Date.now() - started).toBeLessThan(15_000); + // The whole process group stops, not just the shell running the script. + const child = Number(read(root, '.relayflow/child.pid').trim()); + expect(child).toBeGreaterThan(0); + expect(() => process.kill(child, 0)).toThrow(); }); - 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 +383,228 @@ 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('checks the base commit in the same tree, so build products the branch relied on are there too', () => { + // A default such as `make test` builds nothing: the branch passed its + // build step long ago, in this tree. A fresh worktree of the base would + // fail on the missing build and hide the regression as "pre-existing". + const { root, ids } = history([{ '.gitignore': 'built/\n', state: 'good' }, { state: 'bad' }]); + mkdirSync(path.join(root, '.relayflow'), { recursive: true }); + mkdirSync(path.join(root, 'built'), { recursive: true }); + writeFileSync(path.join(root, 'built/ok'), 'artifact\n'); + writeFileSync(path.join(root, FLOW_CHECK_SCRIPT), 'test -f built/ok || { echo "missing build"; exit 2; }\ngrep -q good state\n'); + const head = git(root, 'rev-parse', 'HEAD').trim(); + const branch = git(root, 'symbolic-ref', '--short', 'HEAD').trim(); + expect(sh(FLOW_CHECK_RUN_COMMAND, root).token).toBe('fail'); + expect(sh(`base=${ids[0]}; ${FLOW_BASE_CHECK_COMMAND}`, root)).toMatchObject({ code: 0, token: 'pass' }); + expect(read(root, '.relayflow/base-check.log')).not.toContain('missing build'); + // Back on the branch, by name, with the change and the build product intact. + expect(git(root, 'rev-parse', 'HEAD').trim()).toBe(head); + expect(git(root, 'symbolic-ref', '--short', 'HEAD').trim()).toBe(branch); + expect(read(root, 'state')).toBe('bad'); + expect(read(root, 'built/ok')).toBe('artifact\n'); + }); + + it('runs the branch recipe against the base even when the repository commits the check script', () => { + // The branch commits .relayflow/check.sh; the base has none. Checking the + // base out in place would delete it and report `none` instead of a verdict. + const { root, ids } = history([ + { state: 'good' }, + { state: 'bad', [FLOW_CHECK_SCRIPT]: 'echo "state is $(cat state)"\ngrep -q good state\n' }, + ]); + const head = git(root, 'rev-parse', 'HEAD').trim(); + expect(sh(`base=${ids[0]}; ${FLOW_BASE_CHECK_COMMAND}`, root)).toMatchObject({ code: 0, token: 'pass' }); + expect(read(root, '.relayflow/base-check.log')).toContain('state is good'); + expect(git(root, 'rev-parse', 'HEAD').trim()).toBe(head); + expect(read(root, FLOW_CHECK_SCRIPT)).toContain('grep -q good state'); + expect(git(root, 'status', '--porcelain', '--untracked-files=no').trim()).toBe(''); + }); + + it('never switches a tree with uncommitted changes; it compares in a throwaway worktree instead', () => { + const { root, base } = compare('good', 'bad'); + writeFileSync(path.join(root, 'state'), 'bad but edited\n'); + const head = git(root, 'rev-parse', 'HEAD').trim(); + expect(sh(`base=${base}; ${FLOW_BASE_CHECK_COMMAND}`, root)).toMatchObject({ code: 0, token: 'pass' }); + expect(git(root, 'rev-parse', 'HEAD').trim()).toBe(head); + expect(read(root, 'state')).toBe('bad but edited\n'); + expect(git(root, 'worktree', 'list').trim().split('\n')).toHaveLength(1); + }); + + 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'); + const introduced = report('fail', 'new'); + expect(introduced.body).toContain('The checks this change adds fail'); + expect(introduced.body).not.toContain('FAIL src/other.test.ts'); + 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); + }); +});