From 07975008536b1174a81d890eae4088d5b017c707 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Thu, 10 Sep 2026 22:44:08 +0200 Subject: [PATCH 1/2] docs(examples): nightly-implementer design artifact for flows-driven dogfooding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filed as intent for how flows should orchestrate its own implementation work overnight — instead of the shell-spawn agents pattern that stood in tonight. Every gate that lets code advance toward merge is a deterministic step reading typed JSON; the agents produce blockers, reasons, and diffs but never adjudicate their own work. Per-issue implementer: pinned worktree → typed llm plan → bounded repair loop (impl → scope gate → verify gate → three-lens review swarm → deterministic aggregate verdict) → needs_human on exhausted budget or relayfile-writeback PR-open (surfaces.external, effect.record/confirm, filename-as-idempotency-key). Batch driver fans out via Promise.all with a mechanical summary at the end. Not runnable today. Uses primitives being added in the current launch wave (flows#273 f.llm, flows#275 declarative binding, flows#284 immutable acceptance) and needs cloud#3534's running-stall sweep to survive a stranded sub-run. The README maps every gate to the drive-local defect class it exists to catch (flows#242 / #271 / #284) and the shakedown evidence at evidence/shakedown-0910 on the shakedown/v2-launch-0910 branch. Companion scripts (scope-gate, verify, aggregate-review, report-blocked, report-batch) are trivial jq/vitest/tsc pipelines — no LLM interpretation, kept mechanical so the gate boolean cannot be argued past. Every one syntax-checked with bash -n. Session-Id: efeda5df-9b7c-48d4-b2ce-957f5bef0a82 --- examples/nightly-implementer/README.md | 114 ++++++ .../nightly-implementer/nightly-batch.flow.ts | 78 ++++ .../nightly-implementer.flow.ts | 342 ++++++++++++++++++ .../scripts/aggregate-review.sh | 44 +++ .../scripts/report-batch.sh | 24 ++ .../scripts/report-blocked.sh | 50 +++ .../nightly-implementer/scripts/scope-gate.sh | 37 ++ .../nightly-implementer/scripts/verify.sh | 50 +++ 8 files changed, 739 insertions(+) create mode 100644 examples/nightly-implementer/README.md create mode 100644 examples/nightly-implementer/nightly-batch.flow.ts create mode 100644 examples/nightly-implementer/nightly-implementer.flow.ts create mode 100755 examples/nightly-implementer/scripts/aggregate-review.sh create mode 100755 examples/nightly-implementer/scripts/report-batch.sh create mode 100755 examples/nightly-implementer/scripts/report-blocked.sh create mode 100755 examples/nightly-implementer/scripts/scope-gate.sh create mode 100755 examples/nightly-implementer/scripts/verify.sh diff --git a/examples/nightly-implementer/README.md b/examples/nightly-implementer/README.md new file mode 100644 index 000000000..6b94a479c --- /dev/null +++ b/examples/nightly-implementer/README.md @@ -0,0 +1,114 @@ +# nightly-implementer + +Design artifact for how flows should orchestrate its own implementation +work overnight. Filed on 2026-09-10 as a target shape; the primitives it +uses land in the same launch wave (flows#273 `f.llm`, flows#275 +declarative value binding, structured `output` schemas on agent steps). +Until those close, treat this as intent rather than a runnable script. + +## What the shape proves + +Every decision that lets code advance toward merge is a **deterministic +step reading typed JSON**. The agents produce blockers, reasons, and diffs; +they never adjudicate their own work. That's the whole point. + +Stage by stage: + +1. **`worktree` — deterministic setup**. Checks out a fresh pinned base, + emits `{path, baseSha}`. `json_schema` verification asserts both fields. +2. **`plan` — `f.llm` with typed output**. Compiles to a `json_schema` + verification so a hallucinated plan that doesn't parse as + `{files, testFiles, risks}` fails the step, doesn't bypass it. +3. **Repair loop, bounded to 3 iterations**. Each pass has: + - **`impl` — agent step**. Typed `output` schema (`{sha, filesTouched}`). + Prior-iteration blockers arrive via declarative binding as + `input.blockers`. + - **`scope-gate` — deterministic**. Compares actually-touched files + against the declared allowlist, emits `{scopeOk}`. This is the exact + shape flows#242 / #271 needed and the class fix #284 tracks. + - **`verify` — deterministic**. `tsc --noEmit` + `vitest --reporter=json`. + Verification asserts `numFailed === 0 AND tscOk === true`. An agent + cannot bypass this by asserting success — the JSON is emitted by the + tools themselves. + - **Review swarm — three lenses, parallel**. Correctness / regression / + maintainability. Different providers so shared-model bias cannot + quorum. Each is told "default to `blocked=true`" — the + [review-fix-signoff](../../.claude/skills/review-fix-signoff-loop) + covenant. No lens sees another's verdict. + - **`aggregate-review` — deterministic**. Combines three lens verdicts + with the verify gate result into `{approved, blockers}`. Approve + requires ALL FOUR green. Any lens without `reasons[]` alongside + `blocked=true` is treated as blocked-with-no-reasons and still blocks. +4. **`report-blocked` — needs_human hand-off**. When the repair loop + exhausts its budget without unanimous approval, the run terminates in + `needs_human` (RFC-0001 gate 5, flows#251), not `step_failed`. The + iteration count and every accumulated blocker survive on the outcome + for a human tail. +5. **`open-pr` — agent step declaring `surfaces.external`**. Drops a JSON + file at the relayfile mount's PR-creation path. Gate 6 elects one + attempt via `effect.record`, journals the write, confirms via + `effect.confirm`. Filename at the mount is the idempotency key — a + retried attempt writes the same name and the GitHub adapter refuses + the duplicate. **This is NOT `gh pr create` via a `deterministic` + step**: shelling that out carries none of the effect-journal or + idempotency guarantees. + +## What the shape avoids + +- **Agent-adjudicated gates**. Every merge-forward decision is a + deterministic step reading typed JSON. No LLM rationalizes its way past + a boolean. +- **Single-reviewer rubber-stamping**. Three lenses, distinct angles, + distinct providers, distinct prompts. +- **Silent stalls**. `MAX_REPAIR_ITERATIONS` is finite; exhaustion is a + distinct outcome (`needs_human`) from either success or `step_failed`. +- **Shell-escape hatch for PR opening**. Every writeback goes through the + journaled effect protocol with filename-as-idempotency-key. +- **Free-form verification**. Every `verification:` is `json_schema` over + a shape the producing tool emits directly, or `output_contains` against + a specific literal. + +## What still needs to land + +- **flows#273** — `f.llm` in the TS surface with typed `output`. +- **flows#275** — declarative value binding (the `{step, path}` selector + shape this file uses in every `input:`). +- **flows#274** — YAML agent local-worker path (if the batch driver is + ever re-authored in YAML). +- **flows#284** — first-class immutable-acceptance-inputs. The scope gate + here approximates it as a deterministic step; a proper primitive would + make the guarantee unbypassable at the schema level rather than at the + script level. +- **cloud#3534** — running-stall sweep. Necessary for the batch driver + to survive a stranded sub-run without keeping the run row in `running` + forever. + +Once those five close, `flows run --local-agent nightly-batch.flow.ts` runs +this shape end-to-end. + +## How to run once primitives land + +``` +export RELAYCAST_WORKSPACE_KEY=rk_live_... # observer URL support +export RELAYFILE_MOUNT=/relayfile # PR writeback path root + +# One-shot batch of five issues: +flows run --local-agent examples/nightly-implementer/nightly-batch.flow.ts \ + --input '{"issues":[ + {"repo":"AgentWorkforce/flows","issue":XXX,"briefPath":"...","declaredFiles":[...]}, + ... + ]}' +``` + +The `Observer:` line on stdout is the shareable live view (flows#269 / +#286). The final `RUN completed` carries the aggregate summary +including per-issue outcomes. + +## Provenance + +Shape informed by the [review-fix-signoff-loop](../../.claude/skills/review-fix-signoff-loop), +[relay-80-100-workflow](../../.claude/skills/relay-80-100-workflow), and +[writeback-as-files](../../.claude/skills/writeback-as-files) skills — plus +the concrete drive-local defect class documented at flows#242 / #271 / +#284 and the 2026-09-10 shakedown evidence at +`evidence/shakedown-0910/` on branch `shakedown/v2-launch-0910`. diff --git a/examples/nightly-implementer/nightly-batch.flow.ts b/examples/nightly-implementer/nightly-batch.flow.ts new file mode 100644 index 000000000..1c192deed --- /dev/null +++ b/examples/nightly-implementer/nightly-batch.flow.ts @@ -0,0 +1,78 @@ +// Top-level batch driver for the nightly implementer. +// +// Fan-out across N per-issue implementer runs. `Promise.all` IS the +// parallelism primitive per RFC-0001 (flows#251) — no separate `parallel:` +// declaration, no dependency-string sugar. A worker that dies inside one +// implementer does not stall the others; the runner reports per-branch +// completion reasons and the aggregate outcome names the survivors. + +import { flow, f } from '@relayflows/surface'; +import implementIssue from './nightly-implementer.flow'; + +interface IssueBrief { + repo: string; + issue: number; + briefPath: string; + declaredFiles: string[]; +} + +interface BatchInput { + issues: IssueBrief[]; +} + +export default flow(async (f, input: BatchInput) => { + + // A single implementer's failure must not sink the batch. The catch + // folds it into a structured per-issue outcome the aggregator reads. + // The runner still marks the sub-run as failed; this pattern only + // controls what the enclosing run reports. + const outcomes = await Promise.all( + input.issues.map(async (brief) => { + try { + const result = await implementIssue(f, brief); + return { + issue: brief.issue, + repo: brief.repo, + outcome: 'delivered' as const, + prUrl: (result as { prUrl?: string }).prUrl ?? null, + }; + } catch (error) { + return { + issue: brief.issue, + repo: brief.repo, + outcome: 'failed' as const, + error: error instanceof Error ? error.message : String(error), + }; + } + }), + ); + + // Deterministic summary — a single JSON payload the operator sees on + // wake, including a bounded diagnostic per outcome. No agent adjudicates + // this either: the aggregate is mechanical, so a human reading the run + // report can trust its counts. + return await f.deterministic({ + id: 'summary', + command: + `./scripts/report-batch.sh --outcomes "$OUTCOMES"`, + input: { OUTCOMES: JSON.stringify(outcomes) }, + verification: { + type: 'json_schema', + schema: { + type: 'object', + required: ['delivered', 'failed', 'items'], + properties: { + delivered: { type: 'number' }, + failed: { type: 'number' }, + items: { + type: 'array', + items: { + type: 'object', + required: ['issue', 'outcome'], + }, + }, + }, + }, + }, + }); +}); diff --git a/examples/nightly-implementer/nightly-implementer.flow.ts b/examples/nightly-implementer/nightly-implementer.flow.ts new file mode 100644 index 000000000..f8793f6d0 --- /dev/null +++ b/examples/nightly-implementer/nightly-implementer.flow.ts @@ -0,0 +1,342 @@ +// Per-issue implementer flow with real gates. +// +// This is a DESIGN ARTIFACT. It uses primitives that are landing in the +// 2026-09-10 launch wave (`f.llm`, structured value binding, `output` +// schema on agent steps — flows#273/#275). Until those close, treat this +// as target shape rather than a runnable script. +// +// Design intent (against the drive-local class of bugs — flows#271, #284): +// nothing that decides whether code moves toward merge is agent-adjudicated. +// The agents produce structured blockers/reasons; the aggregation gates that +// let a change advance are deterministic steps reading typed JSON outputs. +// +// The PR-open step is an `agent` step declaring `surfaces.external`, not a +// `deterministic` shelling out to `gh pr create`. Gate 6 elects one attempt +// via `effect.record` + confirms via `effect.confirm`; the filename at the +// relayfile mount is the idempotency key so a retry cannot open two PRs. + +import { flow, f } from '@relayflows/surface'; + +interface ImplementerInput { + /** `AgentWorkforce/flows` or `AgentWorkforce/cloud`. Kept as one string so + * the relayfile mount path derivation stays a single interpolation. */ + repo: string; + /** GitHub issue number the run must close. Journaled with the step, so a + * crash-and-resume rehydrates the correct issue. */ + issue: number; + /** Path to the .md brief the implementation agent reads. Located inside + * the run's worktree so the pinned base commit fixes what the agent sees. */ + briefPath: string; + /** Files the implementation is ALLOWED to touch. Enforced by the scope + * gate before any reviewer sees the diff — the exact drive-local defect + * #242/#271 exists to catch. */ + declaredFiles: string[]; +} + +// Structured verdict every review lens returns. Deterministic aggregation +// reads `blocked` — a lens setting `blocked=true` without concrete reasons +// is treated the same as a schema failure and blocks the merge. +const LENS_VERDICT_SCHEMA = { + type: 'object' as const, + required: ['blocked', 'reasons'], + properties: { + blocked: { type: 'boolean' }, + reasons: { type: 'array', items: { type: 'string' } }, + }, + additionalProperties: false, +}; + +// Hard ceiling on repair iterations. Beyond this the run terminates in +// `needs_human` (RFC-0001 gate 5), not `step_failed` — the outcomes are +// meaningfully different for an operator triaging the drive-cloud queue. +const MAX_REPAIR_ITERATIONS = 3; + +export default flow(async (f, input: ImplementerInput) => { + + // 1. DETERMINISTIC SETUP — pinned base worktree, verified fresh state. + // + // The base commit is pinned by `output.baseSha` so every downstream step + // that references the worktree observes the same tree. A crash resume + // does NOT re-checkout: the runner materializes the same commit from + // its journal, matching gate-1's covenant. + const worktree = await f.deterministic({ + id: 'worktree', + command: ` + set -euo pipefail + dir=/tmp/impl-${input.issue} + rm -rf "$dir" + git worktree add "$dir" origin/main + cd "$dir" + printf '{"path":"%s","baseSha":"%s"}\n' "$dir" "$(git rev-parse HEAD)" + `, + verification: { + type: 'json_schema', + schema: { + type: 'object', + required: ['path', 'baseSha'], + properties: { path: { type: 'string' }, baseSha: { type: 'string' } }, + additionalProperties: false, + }, + }, + }); + + // 2. TYPED PLAN — llm output validated against a schema. + // + // The `output` sugar compiles to a `json_schema` verification (per + // spec.ts:LlmStepSpec). A hallucinated plan that doesn't parse as + // {files, testFiles, risks} fails the step, doesn't bypass it. + const plan = await f.llm({ + id: 'plan', + prompt: `Read ${input.briefPath} and produce an implementation plan.\n` + + `Include: exactly the files you will touch, the test files you\n` + + `will add or modify, and the risks worth naming to a reviewer.`, + output: { + type: 'object', + required: ['files', 'testFiles', 'risks'], + properties: { + files: { type: 'array', items: { type: 'string' } }, + testFiles: { type: 'array', items: { type: 'string' } }, + risks: { type: 'array', items: { type: 'string' } }, + }, + additionalProperties: false, + }, + }); + + // 3-7. REPAIR LOOP — bounded, fresh-context each iteration. + // + // Each pass produces an implementation, mechanical scope + verify gates, + // three independent review lenses, and a deterministic aggregate verdict. + // A blocked verdict feeds its `reasons` into the next iteration's impl + // agent as `input.blockers` — same shape as gate 2's `wake_context` per + // RFC-0001 (flows#251) so a resume observes the same replay input. + let impl: unknown = null; + let verdict: { approved: boolean; blockers: string[] } | null = null; + let iteration = 0; + + while (iteration < MAX_REPAIR_ITERATIONS) { + iteration += 1; + + // 3. IMPLEMENTATION — declared output schema; blockers from prior verdict. + impl = await f.agent({ + id: `impl-${iteration}`, + cli: 'codex', + instruction: + `Iteration ${iteration}. Execute the plan against the pinned worktree. ` + + `Touch ONLY files listed in the plan. If prior blockers are supplied, ` + + `address them without widening scope. Emit {sha, filesTouched} on success.`, + input: { + plan: { step: 'plan' }, + cwd: { step: 'worktree', path: ['path'] }, + blockers: verdict ? { step: `aggregate-${iteration - 1}`, path: ['blockers'] } : [], + }, + output: { + type: 'object', + required: ['sha', 'filesTouched'], + properties: { + sha: { type: 'string' }, + filesTouched: { type: 'array', items: { type: 'string' } }, + }, + additionalProperties: false, + }, + }); + + // 4. SCOPE GATE — deterministic, mechanical, unbypassable. + // + // Compares the sorted file list actually touched against the declared + // allowed set. Runs BEFORE any reviewer sees the diff so a scope + // violation short-circuits without spending review budget. This is + // exactly the shape #242/#271 needed and the class fix #284 tracks. + await f.deterministic({ + id: `scope-${iteration}`, + command: `./scripts/scope-gate.sh "$WORKTREE" '${JSON.stringify(input.declaredFiles)}'`, + input: { WORKTREE: { step: 'worktree', path: ['path'] } }, + verification: { + type: 'json_schema', + schema: { + type: 'object', + required: ['scopeOk'], + properties: { scopeOk: { const: true } }, + }, + }, + }); + + // 5. VERIFY GATE — deterministic: tsc + tests, JSON output, no free-form. + // + // Emits vitest's --reporter=json + `tsc --noEmit` exit-status folded + // into one JSON blob. The verification asserts numFailed === 0 AND + // tscOk === true. An agent cannot pass this by asserting success — + // the JSON is produced by the tools themselves. + const verify = await f.deterministic({ + id: `verify-${iteration}`, + command: `./scripts/verify.sh "$WORKTREE"`, + input: { WORKTREE: { step: 'worktree', path: ['path'] } }, + verification: { + type: 'json_schema', + schema: { + type: 'object', + required: ['numPassed', 'numFailed', 'tscOk'], + properties: { + numPassed: { type: 'number' }, + numFailed: { const: 0 }, + tscOk: { const: true }, + }, + }, + }, + }); + + // 6. REVIEW SWARM — three independent lenses, parallel. + // + // Each lens has: + // - a different provider (so a shared model bias cannot quorum), + // - a specific angle (correctness / regression-risk / maintainability), + // - a `default to blocked=true` instruction (the pattern that catches + // plausible-but-wrong; the review-fix-signoff skill's covenant). + // No lens sees another lens's verdict — the aggregation happens in the + // deterministic step below, not inside any agent's context. + const [correctness, regression, maintainability] = await Promise.all([ + f.agent({ + id: `correctness-${iteration}`, + cli: 'claude', + instruction: + `You are the CORRECTNESS lens. Default blocked=true. ` + + `Approve only if the implementation solves issue #${input.issue} ` + + `as stated in its brief. Read plan, diff, and tests. Refute where you can. ` + + `Cite specific lines when blocking.`, + input: { + plan: { step: 'plan' }, + impl: { step: `impl-${iteration}` }, + verify: { step: `verify-${iteration}` }, + }, + output: LENS_VERDICT_SCHEMA, + }), + f.agent({ + id: `regression-${iteration}`, + cli: 'codex', + instruction: + `You are the REGRESSION lens. Default blocked=true. ` + + `Find what this change could BREAK that isn't tested. Look at ` + + `neighboring code, callers, downstream. Cite one concrete example ` + + `per blocker.`, + input: { + plan: { step: 'plan' }, + impl: { step: `impl-${iteration}` }, + }, + output: LENS_VERDICT_SCHEMA, + }), + f.agent({ + id: `maintainability-${iteration}`, + cli: 'claude', + instruction: + `You are the MAINTAINABILITY lens. Default blocked=true. ` + + `Comments correct? Names honest? Dead-code residue from earlier ` + + `attempts? Any over-abstraction? An identifier lying about what it ` + + `describes is a blocker.`, + input: { impl: { step: `impl-${iteration}` } }, + output: LENS_VERDICT_SCHEMA, + }), + ]); + + // 7. AGGREGATE — deterministic, not agent-adjudicated. + // + // Combines three lens verdicts, the scope gate result, and the verify + // gate result into one decision. Approve requires ALL five to green. + // The shell script is trivial (jq over its inputs); keeping it here + // rather than an agent step makes the approval mechanically auditable — + // no LLM can rationalize its way past a boolean. + verdict = await f.deterministic({ + id: `aggregate-${iteration}`, + command: `./scripts/aggregate-review.sh`, + input: { + correctness: { step: `correctness-${iteration}` }, + regression: { step: `regression-${iteration}` }, + maintainability: { step: `maintainability-${iteration}` }, + verify: { step: `verify-${iteration}` }, + }, + verification: { + type: 'json_schema', + schema: { + type: 'object', + required: ['approved', 'blockers'], + properties: { + approved: { type: 'boolean' }, + blockers: { type: 'array', items: { type: 'string' } }, + }, + additionalProperties: false, + }, + }, + }); + + if (verdict.approved) break; + } + + // 8. BOUNDED FAILURE — never a silent stall. If review never converged, + // surface `needs_human` with the iteration count and every accumulated + // blocker. RFC-0001 gate 5 language, not a bare step_failed. + if (!verdict || !verdict.approved) { + return await f.deterministic({ + id: 'report-blocked', + command: + `./scripts/report-blocked.sh --issue ${input.issue} ` + + `--iterations ${iteration} --blockers "$BLOCKERS"`, + input: { BLOCKERS: { step: `aggregate-${iteration}`, path: ['blockers'] } }, + // The reported outcome should surface as needs_human, not step_failed. + // The script exits with an inspection-friendly diagnostic and a + // non-zero code the runner recognizes as gate-5 hand-off. + verification: { + type: 'json_schema', + schema: { + type: 'object', + required: ['outcome', 'inspectionUrl'], + properties: { outcome: { const: 'needs_human' } }, + }, + }, + }); + } + + // 9. PR OPEN — relayfile writeback, not `gh pr create`. + // + // Declares `surfaces.external` naming the mount path. The runner elects + // one attempt via `effect.record`, journals the write, then closes with + // `effect.confirm` (kernel DAEMON-LIFECYCLE.md gate 6). Filename at the + // mount is the idempotency key — a retried attempt writes the same + // filename and the GitHub adapter refuses the duplicate rather than + // opening a second PR. `gh pr create` carries none of that. + // + // The instruction is deliberately mechanical — the agent's job is to + // format the JSON body from prior step outputs and drop it at the + // declared path. No creative interpretation. + return await f.agent({ + id: 'open-pr', + cli: 'codex', + instruction: + `Drop a PR-creation JSON at the relayfile mount path declared in ` + + `surfaces.external. Filename must be "impl-${input.issue}.json" — ` + + `the adapter uses it as the idempotency key. Fields required by the ` + + `adapter: {title, head, base, body, idempotency_key}. Set head to ` + + `the branch pushed by the impl step (from impl.sha). Body should ` + + `include the plan's risks section and link to the reviewer verdicts. ` + + `Emit {prUrl} once the writeback is confirmed by the adapter.`, + input: { + impl: { step: `impl-${iteration}` }, + plan: { step: 'plan' }, + correctness: { step: `correctness-${iteration}` }, + regression: { step: `regression-${iteration}` }, + maintainability: { step: `maintainability-${iteration}` }, + issue: input.issue, + }, + surfaces: { + external: [ + // Canonical writeback path per relayfile's GitHub adapter. The + // exact prefix is workspace-dependent; ${RELAYFILE_MOUNT} is the + // ambient env the runner sets when a mount is attached. + `github/${input.repo}/pulls/create`, + ], + }, + output: { + type: 'object', + required: ['prUrl'], + properties: { prUrl: { type: 'string' } }, + additionalProperties: false, + }, + }); +}); diff --git a/examples/nightly-implementer/scripts/aggregate-review.sh b/examples/nightly-implementer/scripts/aggregate-review.sh new file mode 100755 index 000000000..fd571de94 --- /dev/null +++ b/examples/nightly-implementer/scripts/aggregate-review.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# aggregate-review.sh — deterministic verdict combiner. +# +# Reads the three lens verdicts and the verify gate result from FLOWS_INPUT +# (the JSON env the runner sets from a step's declared `input:` bindings). +# Emits {approved: bool, blockers: string[]} on stdout. +# +# Approved requires ALL FOUR of: +# correctness.blocked === false +# regression.blocked === false +# maintainability.blocked === false +# verify.numFailed === 0 +# +# A missing field, an unexpected shape, or ANY blocked=true from a lens +# fails the aggregate. The script is deliberately dumb — no LLM interpretation, +# no partial credit. This is the mechanical clearance the drive-local class +# fix (flows#284) needs so no accumulated reviewer bias can approve broken code. +set -euo pipefail + +input="${FLOWS_INPUT:-{}}" + +blocked_count=0 +blockers=$(jq -c --argjson zero 0 ' + def lens($name; $obj): + if ($obj.blocked // true) then + ($obj.reasons // ["\($name): blocked with no reasons — treat as blocked"]) + | map("[\($name)] \(.)") + else [] end; + ( lens("correctness"; .correctness) + + lens("regression"; .regression) + + lens("maintainability"; .maintainability) + + (if (.verify.numFailed // 1) > 0 then + ["[verify] \((.verify.numFailed // 0)) test(s) failing"] + else [] end) + ) +' <<<"$input") + +count=$(jq 'length' <<<"$blockers") + +if [ "$count" = 0 ]; then + jq -n '{approved: true, blockers: []}' +else + jq -n --argjson blockers "$blockers" '{approved: false, blockers: $blockers}' +fi diff --git a/examples/nightly-implementer/scripts/report-batch.sh b/examples/nightly-implementer/scripts/report-batch.sh new file mode 100755 index 000000000..7f9a31b9c --- /dev/null +++ b/examples/nightly-implementer/scripts/report-batch.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# report-batch.sh — deterministic batch summary. +# +# Reads the per-issue outcomes from --outcomes (a JSON array produced by +# the batch flow) and emits {delivered, failed, items} on stdout. Counts +# are mechanical: no LLM adjudicates which run "really" succeeded. +set -euo pipefail + +outcomes_json="" + +while [ $# -gt 0 ]; do + case "$1" in + --outcomes) outcomes_json="$2"; shift 2 ;; + *) echo "unknown arg: $1" >&2; exit 2 ;; + esac +done + +[ -n "$outcomes_json" ] || { echo "--outcomes is required" >&2; exit 2; } + +jq -n --argjson items "$outcomes_json" '{ + delivered: ($items | map(select(.outcome == "delivered")) | length), + failed: ($items | map(select(.outcome == "failed")) | length), + items: $items +}' diff --git a/examples/nightly-implementer/scripts/report-blocked.sh b/examples/nightly-implementer/scripts/report-blocked.sh new file mode 100755 index 000000000..b02fc988a --- /dev/null +++ b/examples/nightly-implementer/scripts/report-blocked.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# report-blocked.sh — needs_human hand-off for the bounded review loop. +# +# Called by the top-level flow when the repair loop exhausted its budget +# without a green verdict. Emits {outcome: "needs_human", inspectionUrl, +# summary} on stdout — the parent verification asserts outcome === "needs_human" +# so the runner surfaces this as gate-5 hand-off (RFC-0001 flows#251), NOT +# as a bare step_failed. The distinction matters to a human triaging the +# drive queue: needs_human means "we did the work, the reviewers refused"; +# step_failed means "we couldn't do the work at all". +set -euo pipefail + +issue="" +iterations="" +blockers="" + +while [ $# -gt 0 ]; do + case "$1" in + --issue) issue="$2"; shift 2 ;; + --iterations) iterations="$2"; shift 2 ;; + --blockers) blockers="$2"; shift 2 ;; + *) echo "unknown arg: $1" >&2; exit 2 ;; + esac +done + +[ -n "$issue" ] || { echo "--issue is required" >&2; exit 2; } +[ -n "$iterations" ] || { echo "--iterations is required" >&2; exit 2; } + +inspection_url="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/AgentWorkforce/flows/issues/${issue}" + +# Blockers arrive as a JSON array (produced by aggregate-review.sh). Fold +# them into a single ordered list per iteration, preserved literally so +# the human sees what each lens actually said — no LLM summarization. +blockers_json="${blockers:-[]}" + +jq -n \ + --arg issue "$issue" \ + --arg iterations "$iterations" \ + --arg url "$inspection_url" \ + --argjson blockers "$blockers_json" \ + '{outcome: "needs_human", + issue: $issue, + iterations: ($iterations | tonumber), + inspectionUrl: $url, + summary: "\($iterations) implementation iterations exhausted without unanimous review approval; last-iteration blockers preserved verbatim below.", + blockers: $blockers}' + +# Non-zero exit lets the runner classify this outcome as blocked. The +# structured stdout is journaled either way. +exit 3 diff --git a/examples/nightly-implementer/scripts/scope-gate.sh b/examples/nightly-implementer/scripts/scope-gate.sh new file mode 100755 index 000000000..7ff86b63d --- /dev/null +++ b/examples/nightly-implementer/scripts/scope-gate.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# scope-gate.sh — deterministic scope-enforcement gate. +# +# Emits {scopeOk: true} on stdout when the implementation touched EXACTLY +# the declared files. Emits {scopeOk: false, touchedOutsideScope: [...]} +# and exits non-zero otherwise. The parent step's `json_schema` +# verification asserts scopeOk === true, so this script's exit code is a +# safety belt — the schema is what actually blocks the merge. +# +# This is the mechanical gate that would have caught the flows#242 case +# where an implementation "verified" while its work sat outside scope. +set -euo pipefail + +worktree="${1:?usage: scope-gate.sh WORKTREE_PATH DECLARED_FILES_JSON}" +declared_json="${2:?usage: scope-gate.sh WORKTREE_PATH DECLARED_FILES_JSON}" + +cd "$worktree" + +# Merge-base diff — what THIS branch changed, not what main also has. +touched_sorted=$(git diff --name-only origin/main | sort -u) +declared_sorted=$(printf '%s' "$declared_json" | jq -r '.[]' | sort -u) + +outside=$(comm -23 <(printf '%s\n' "$touched_sorted") <(printf '%s\n' "$declared_sorted")) +missing=$(comm -13 <(printf '%s\n' "$touched_sorted") <(printf '%s\n' "$declared_sorted")) + +if [ -z "$outside" ] && [ -z "$missing" ]; then + printf '{"scopeOk":true}\n' + exit 0 +fi + +jq -n \ + --arg outside "$outside" \ + --arg missing "$missing" \ + '{scopeOk: false, + touchedOutsideScope: ($outside | split("\n") | map(select(length > 0))), + declaredButNotTouched: ($missing | split("\n") | map(select(length > 0)))}' +exit 1 diff --git a/examples/nightly-implementer/scripts/verify.sh b/examples/nightly-implementer/scripts/verify.sh new file mode 100755 index 000000000..00d99bdf4 --- /dev/null +++ b/examples/nightly-implementer/scripts/verify.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# verify.sh — deterministic tsc + vitest gate. +# +# Emits {numPassed, numFailed, tscOk} on stdout. The parent step's +# json_schema verification requires numFailed === 0 AND tscOk === true. +# An agent cannot pass this by asserting success — the JSON is produced +# by the tools themselves. +# +# tsc runs first because a type error is cheaper to surface than a test +# failure and often produces the same defect. If tsc fails, vitest is +# skipped and both counts are reported as 0 alongside tscOk=false, so +# the parent verification sees "not-all-green" clearly. +set -euo pipefail + +worktree="${1:?usage: verify.sh WORKTREE_PATH}" +cd "$worktree" + +# Prefer the workspace's SDK dir if present; the design flow can be +# extended to accept a package path parameter later. +sdk="packages/sdk" +[ -d "$sdk" ] || sdk="." + +# Capture tsc separately so a compile error is a distinct signal from +# a test failure. Both go to stderr for a human tail; the JSON is stdout. +if (cd "$sdk" && npx tsc --noEmit 2>&1 >&2); then + tsc_ok=true +else + tsc_ok=false +fi + +if [ "$tsc_ok" = false ]; then + jq -n '{numPassed: 0, numFailed: 0, tscOk: false}' + exit 1 +fi + +# vitest --reporter=json includes numTotalTests / numFailedTests at the top. +# --outputFile is picked over stdout so tests using console.log don't +# corrupt the JSON payload. +report=$(mktemp) +trap 'rm -f "$report"' EXIT +(cd "$sdk" && npx vitest run --reporter=json --outputFile="$report" 2>&1 >&2) || true + +num_passed=$(jq '.numPassedTests // 0' "$report") +num_failed=$(jq '.numFailedTests // 0' "$report") + +jq -n \ + --argjson passed "$num_passed" \ + --argjson failed "$num_failed" \ + '{numPassed: $passed, numFailed: $failed, tscOk: true}' +[ "$num_failed" = 0 ] || exit 1 From 6f398ed194e233c45ac28a5552d29dd89aea5f64 Mon Sep 17 00:00:00 2001 From: Miya Date: Thu, 17 Sep 2026 09:38:41 +0200 Subject: [PATCH 2/2] fix(examples): close nightly implementer review gaps Session-Id: 01a09c40-ce3b-7f11-a7df-b6b7ccab6fd9 --- examples/nightly-implementer/README.md | 11 ++- .../nightly-implementer/nightly-batch.flow.ts | 27 +++++- .../nightly-implementer.flow.ts | 16 ++-- .../scripts/aggregate-review.sh | 15 ++-- .../scripts/report-batch.sh | 3 +- .../scripts/report-blocked.sh | 5 +- .../nightly-implementer/scripts/verify.sh | 4 +- examples/nightly-implementer/tests/verify.sh | 86 +++++++++++++++++++ 8 files changed, 145 insertions(+), 22 deletions(-) create mode 100644 examples/nightly-implementer/tests/verify.sh diff --git a/examples/nightly-implementer/README.md b/examples/nightly-implementer/README.md index 6b94a479c..877a34e2a 100644 --- a/examples/nightly-implementer/README.md +++ b/examples/nightly-implementer/README.md @@ -43,7 +43,7 @@ Stage by stage: exhausts its budget without unanimous approval, the run terminates in `needs_human` (RFC-0001 gate 5, flows#251), not `step_failed`. The iteration count and every accumulated blocker survive on the outcome - for a human tail. + for a human tail. The batch reports these separately from delivered PRs. 5. **`open-pr` — agent step declaring `surfaces.external`**. Drops a JSON file at the relayfile mount's PR-creation path. Gate 6 elects one attempt via `effect.record`, journals the write, confirms via @@ -104,6 +104,15 @@ The `Observer:` line on stdout is the shareable live view (flows#269 / #286). The final `RUN completed` carries the aggregate summary including per-issue outcomes. +## Verify the deterministic pieces + +The future surface primitives prevent an end-to-end run today, but the shell +gates and their flow wiring are pinned locally: + +``` +bash examples/nightly-implementer/tests/verify.sh +``` + ## Provenance Shape informed by the [review-fix-signoff-loop](../../.claude/skills/review-fix-signoff-loop), diff --git a/examples/nightly-implementer/nightly-batch.flow.ts b/examples/nightly-implementer/nightly-batch.flow.ts index 1c192deed..b41f3bff4 100644 --- a/examples/nightly-implementer/nightly-batch.flow.ts +++ b/examples/nightly-implementer/nightly-batch.flow.ts @@ -20,6 +20,16 @@ interface BatchInput { issues: IssueBrief[]; } +interface NeedsHumanResult { + outcome: 'needs_human'; + inspectionUrl?: string; +} + +function isNeedsHuman(result: unknown): result is NeedsHumanResult { + return typeof result === 'object' && result !== null && + (result as { outcome?: unknown }).outcome === 'needs_human'; +} + export default flow(async (f, input: BatchInput) => { // A single implementer's failure must not sink the batch. The catch @@ -30,6 +40,14 @@ export default flow(async (f, input: BatchInput) => { input.issues.map(async (brief) => { try { const result = await implementIssue(f, brief); + if (isNeedsHuman(result)) { + return { + issue: brief.issue, + repo: brief.repo, + outcome: 'needs_human' as const, + inspectionUrl: result.inspectionUrl ?? null, + }; + } return { issue: brief.issue, repo: brief.repo, @@ -60,10 +78,11 @@ export default flow(async (f, input: BatchInput) => { type: 'json_schema', schema: { type: 'object', - required: ['delivered', 'failed', 'items'], - properties: { - delivered: { type: 'number' }, - failed: { type: 'number' }, + required: ['delivered', 'needsHuman', 'failed', 'items'], + properties: { + delivered: { type: 'number' }, + needsHuman: { type: 'number' }, + failed: { type: 'number' }, items: { type: 'array', items: { diff --git a/examples/nightly-implementer/nightly-implementer.flow.ts b/examples/nightly-implementer/nightly-implementer.flow.ts index f8793f6d0..8969c215c 100644 --- a/examples/nightly-implementer/nightly-implementer.flow.ts +++ b/examples/nightly-implementer/nightly-implementer.flow.ts @@ -56,15 +56,14 @@ export default flow(async (f, input: ImplementerInput) => { // 1. DETERMINISTIC SETUP — pinned base worktree, verified fresh state. // // The base commit is pinned by `output.baseSha` so every downstream step - // that references the worktree observes the same tree. A crash resume - // does NOT re-checkout: the runner materializes the same commit from - // its journal, matching gate-1's covenant. + // that references the worktree observes the same tree. Each execution gets + // a new atomically-created directory: retries never delete or reuse a + // registered worktree from a prior attempt. const worktree = await f.deterministic({ id: 'worktree', command: ` set -euo pipefail - dir=/tmp/impl-${input.issue} - rm -rf "$dir" + dir=$(mktemp -d "${TMPDIR:-/tmp}/relayflows-impl.XXXXXX") git worktree add "$dir" origin/main cd "$dir" printf '{"path":"%s","baseSha":"%s"}\n' "$dir" "$(git rev-parse HEAD)" @@ -276,9 +275,12 @@ export default flow(async (f, input: ImplementerInput) => { return await f.deterministic({ id: 'report-blocked', command: - `./scripts/report-blocked.sh --issue ${input.issue} ` + + `./scripts/report-blocked.sh --repo "$REPOSITORY" --issue ${input.issue} ` + `--iterations ${iteration} --blockers "$BLOCKERS"`, - input: { BLOCKERS: { step: `aggregate-${iteration}`, path: ['blockers'] } }, + input: { + REPOSITORY: input.repo, + BLOCKERS: { step: `aggregate-${iteration}`, path: ['blockers'] }, + }, // The reported outcome should surface as needs_human, not step_failed. // The script exits with an inspection-friendly diagnostic and a // non-zero code the runner recognizes as gate-5 hand-off. diff --git a/examples/nightly-implementer/scripts/aggregate-review.sh b/examples/nightly-implementer/scripts/aggregate-review.sh index fd571de94..7cb154419 100755 --- a/examples/nightly-implementer/scripts/aggregate-review.sh +++ b/examples/nightly-implementer/scripts/aggregate-review.sh @@ -17,14 +17,17 @@ # fix (flows#284) needs so no accumulated reviewer bias can approve broken code. set -euo pipefail -input="${FLOWS_INPUT:-{}}" +input="${FLOWS_INPUT:-}" +[ -n "$input" ] || input='{}' -blocked_count=0 -blockers=$(jq -c --argjson zero 0 ' +blockers=$(jq -c ' def lens($name; $obj): - if ($obj.blocked // true) then - ($obj.reasons // ["\($name): blocked with no reasons — treat as blocked"]) - | map("[\($name)] \(.)") + if ($obj.blocked != false) then + if (($obj.reasons // []) | type != "array" or length == 0) then + ["[\($name)] blocked with no reasons — treat as blocked"] + else + $obj.reasons | map("[\($name)] \(.)") + end else [] end; ( lens("correctness"; .correctness) + lens("regression"; .regression) diff --git a/examples/nightly-implementer/scripts/report-batch.sh b/examples/nightly-implementer/scripts/report-batch.sh index 7f9a31b9c..620342ce8 100755 --- a/examples/nightly-implementer/scripts/report-batch.sh +++ b/examples/nightly-implementer/scripts/report-batch.sh @@ -2,7 +2,7 @@ # report-batch.sh — deterministic batch summary. # # Reads the per-issue outcomes from --outcomes (a JSON array produced by -# the batch flow) and emits {delivered, failed, items} on stdout. Counts +# the batch flow) and emits {delivered, needsHuman, failed, items} on stdout. Counts # are mechanical: no LLM adjudicates which run "really" succeeded. set -euo pipefail @@ -19,6 +19,7 @@ done jq -n --argjson items "$outcomes_json" '{ delivered: ($items | map(select(.outcome == "delivered")) | length), + needsHuman: ($items | map(select(.outcome == "needs_human")) | length), failed: ($items | map(select(.outcome == "failed")) | length), items: $items }' diff --git a/examples/nightly-implementer/scripts/report-blocked.sh b/examples/nightly-implementer/scripts/report-blocked.sh index b02fc988a..1ec09530d 100755 --- a/examples/nightly-implementer/scripts/report-blocked.sh +++ b/examples/nightly-implementer/scripts/report-blocked.sh @@ -11,11 +11,13 @@ set -euo pipefail issue="" +repo="" iterations="" blockers="" while [ $# -gt 0 ]; do case "$1" in + --repo) repo="$2"; shift 2 ;; --issue) issue="$2"; shift 2 ;; --iterations) iterations="$2"; shift 2 ;; --blockers) blockers="$2"; shift 2 ;; @@ -24,9 +26,10 @@ while [ $# -gt 0 ]; do done [ -n "$issue" ] || { echo "--issue is required" >&2; exit 2; } +[ -n "$repo" ] || { echo "--repo is required" >&2; exit 2; } [ -n "$iterations" ] || { echo "--iterations is required" >&2; exit 2; } -inspection_url="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/AgentWorkforce/flows/issues/${issue}" +inspection_url="/${repo}/issues/${issue}" # Blockers arrive as a JSON array (produced by aggregate-review.sh). Fold # them into a single ordered list per iteration, preserved literally so diff --git a/examples/nightly-implementer/scripts/verify.sh b/examples/nightly-implementer/scripts/verify.sh index 00d99bdf4..ab0140b82 100755 --- a/examples/nightly-implementer/scripts/verify.sh +++ b/examples/nightly-implementer/scripts/verify.sh @@ -22,7 +22,7 @@ sdk="packages/sdk" # Capture tsc separately so a compile error is a distinct signal from # a test failure. Both go to stderr for a human tail; the JSON is stdout. -if (cd "$sdk" && npx tsc --noEmit 2>&1 >&2); then +if (cd "$sdk" && npx tsc --noEmit >&2); then tsc_ok=true else tsc_ok=false @@ -38,7 +38,7 @@ fi # corrupt the JSON payload. report=$(mktemp) trap 'rm -f "$report"' EXIT -(cd "$sdk" && npx vitest run --reporter=json --outputFile="$report" 2>&1 >&2) || true +(cd "$sdk" && npx vitest run --reporter=json --outputFile="$report" >&2) || true num_passed=$(jq '.numPassedTests // 0' "$report") num_failed=$(jq '.numFailedTests // 0' "$report") diff --git a/examples/nightly-implementer/tests/verify.sh b/examples/nightly-implementer/tests/verify.sh new file mode 100644 index 000000000..0645553f9 --- /dev/null +++ b/examples/nightly-implementer/tests/verify.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# Focused regression checks for the executable nightly-implementer gates. +set -euo pipefail + +root=$(cd "$(dirname "$0")/.." && pwd) +tmp=$(mktemp -d "${TMPDIR:-/tmp}/nightly-implementer-test.XXXXXX") +trap 'rm -rf "$tmp"' EXIT + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +test_empty_blocker_reasons_fail_closed() { + local output + output=$(FLOWS_INPUT='{"correctness":{"blocked":true,"reasons":[]},"regression":{"blocked":false,"reasons":[]},"maintainability":{"blocked":false,"reasons":[]},"verify":{"numFailed":0}}' \ + "$root/scripts/aggregate-review.sh") + jq -e '.approved == false and (.blockers | length == 1) and (.blockers[0] | contains("blocked with no reasons"))' \ + <<<"$output" >/dev/null || fail 'empty blocker reasons approved the aggregate' +} + +test_diagnostics_do_not_corrupt_json() { + local fixture="$tmp/verify-fixture" + mkdir -p "$fixture/packages/sdk" "$tmp/bin" + cat > "$tmp/bin/npx" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail +case "$1" in + tsc) + echo 'tsc diagnostic on stdout' + ;; + vitest) + echo 'vitest diagnostic on stdout' + for arg in "$@"; do + case "$arg" in + --outputFile=*) printf '{"numPassedTests":2,"numFailedTests":0}\n' > "${arg#--outputFile=}" ;; + esac + done + ;; + *) exit 2 ;; +esac +EOF + chmod +x "$tmp/bin/npx" + PATH="$tmp/bin:$PATH" "$root/scripts/verify.sh" "$fixture" > "$tmp/verify.json" 2> "$tmp/verify.stderr" + jq -e '. == {numPassed: 2, numFailed: 0, tscOk: true}' "$tmp/verify.json" >/dev/null || + fail 'verify stdout was not its JSON payload' + ! grep -q 'diagnostic on stdout' "$tmp/verify.json" || + fail 'tool diagnostics leaked onto verify stdout' +} + +test_retry_worktrees_are_unique_and_non_destructive() { + rg -F 'dir=$(mktemp -d "${TMPDIR:-/tmp}/relayflows-impl.XXXXXX")' "$root/nightly-implementer.flow.ts" >/dev/null || + fail 'worktree setup does not use an atomic unique directory' + ! rg -F 'rm -rf "$dir"' "$root/nightly-implementer.flow.ts" >/dev/null || + fail 'worktree setup deletes an existing attempt directory' +} + +test_needs_human_is_not_delivered() { + local output + output=$("$root/scripts/report-batch.sh" --outcomes '[{"issue":7,"repo":"AgentWorkforce/flows","outcome":"needs_human"}]') + jq -e '.delivered == 0 and .needsHuman == 1 and .failed == 0' <<<"$output" >/dev/null || + fail 'needs_human was counted as delivered' + rg -F 'if (isNeedsHuman(result))' "$root/nightly-batch.flow.ts" >/dev/null || + fail 'batch flow does not preserve needs_human outcomes' +} + +test_blocked_url_uses_configured_repository() { + local output status + set +e + output=$("$root/scripts/report-blocked.sh" --repo AgentWorkforce/cloud --issue 3534 --iterations 3 --blockers '[]') + status=$? + set -e + [ "$status" -eq 3 ] || fail "report-blocked exited $status, expected 3" + jq -e '.inspectionUrl == "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/AgentWorkforce/cloud/issues/3534"' <<<"$output" >/dev/null || + fail 'blocked URL did not use the configured repository' + rg -F -- '--repo "$REPOSITORY"' "$root/nightly-implementer.flow.ts" >/dev/null || + fail 'flow does not pass the configured repository to report-blocked' +} + +test_empty_blocker_reasons_fail_closed +test_diagnostics_do_not_corrupt_json +test_retry_worktrees_are_unique_and_non_destructive +test_needs_human_is_not_delivered +test_blocked_url_uses_configured_repository + +echo 'nightly-implementer focused verification: PASS'