diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b244b2d..fbfb52c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,7 @@ Keep the ownership boundary strict: - Pi owns agent execution and session lifecycle. - `pi-subagents` owns child orchestration, worktrees, missions, schedules, resume, and external-agent runners. -- ForgeFlow may add deterministic engineering invariants and acceptance gates - only when those policies are not already Pi primitives. +- ForgeFlow may add deterministic engineering invariants only when the policy + is not already owned by Pi or an installed plugin. - Do not add a ForgeFlow database, generic provider router, scheduler, worktree manager, or durable execution engine. diff --git a/README.md b/README.md index 20284ab..efc6763 100644 --- a/README.md +++ b/README.md @@ -1,65 +1,36 @@ # ForgeFlow -> Thin software-engineering governance for Pi Agent. +> Thin software-engineering policy for Pi Agent. -ForgeFlow is a Pi package, not a coding-agent runtime. Pi owns execution and -`pi-subagents` owns delegation, child lifecycle, worktree isolation, missions, -schedules, resume, background execution, and external-agent runners. +ForgeFlow is a Pi package, not an agent runtime or workflow engine. Pi and its +plugins own execution. ForgeFlow keeps only policy that is useful across those +plugins. -ForgeFlow adds only engineering policy that is not already a Pi primitive. +## What ForgeFlow owns -## Architecture +- stable logical model roles: + `forgeflow/planner`, `worker`, `reviewer`, `scout`, and `oracle`; +- deterministic engineering-invariant preflight; +- the one-writer-per-worktree rule; +- policy reminders that bind delivery evidence to the current candidate. -```text -Pi Agent - | - +-- pi-subagents - | +-- worker / reviewer / scout / oracle - | +-- worktrees / missions / schedules / resume - | +-- external-cli / external-job agents - | - +-- ForgeFlow - +-- stable logical model roles via Pi virtual models - +-- deterministic invariant preflight - +-- one-writer policy - +-- independent review workflow - +-- exact-head GitHub acceptance -``` - -The governing rule is: - -> Agent success is evidence, not engineering acceptance. - -ForgeFlow deliberately does **not** implement sessions, provider/channel routing, -generic agent execution, durable workflow state, a second worktree manager, or -a second scheduler. It can map stable engineering roles such as `forgeflow/worker` -to physical Pi models, while credentials, endpoints, channel weights, quotas, and -transport remain owned by Pi/provider infrastructure. - -See [`docs/model-policy.md`](./docs/model-policy.md) for the logical-model policy and provider boundary. - -## Workflows - -### `forgeflow.review` - -Runs a fresh Pi reviewer on the stable `forgeflow/reviewer` virtual model, -persists the full report under `.pi/subagents/`, and uses a Pi typed gate to -validate the reviewer's canonical final merge verdict. Malformed or missing -verdicts fail closed. - -### `forgeflow.accept` +## What ForgeFlow does not own -Binds final acceptance to one exact committed candidate: +Pi and installed plugins own child execution, review loops, acceptance gates, +worktrees, missions, schedules, resume, background jobs, external-agent runners, +and pull-request gating. -1. the local worktree must be clean and at the requested HEAD; -2. a fresh independent reviewer must return a clean typed verdict; -3. the local HEAD must remain unchanged during review; -4. the authoritative open PR must still target the expected base and exact HEAD; -5. every configured required GitHub check must have a successful terminal result. +Use `pi-subagents` for delegation, reviewers, and runtime acceptance evidence. +When `pi-gauntlet` is installed, use its `gatekeep-pr` skill for exact-head PR +verification and merge safety. ForgeFlow deliberately does not wrap or duplicate +those surfaces. -A new push changes the candidate and invalidates prior acceptance evidence. +Provider infrastructure remains below Pi model selection. ForgeFlow logical roles +may resolve to physical models, while endpoint selection, credentials, channel +health, weights, quotas, and transport belong to the provider layer. -See [`docs/architecture.md`](./docs/architecture.md) for the ownership boundary. +See [`docs/model-policy.md`](./docs/model-policy.md) for logical-model policy and +[`docs/architecture.md`](./docs/architecture.md) for the ownership boundary. ## Development diff --git a/docs/architecture.md b/docs/architecture.md index 3c947bc..fa09c58 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1,83 +1,82 @@ # ForgeFlow architecture -ForgeFlow is a thin governance package for Pi Agent. +ForgeFlow is a thin policy package for Pi Agent. ## Ownership -### Pi / pi-subagents own +### Pi and installed plugins own -- physical model and provider execution; -- virtual-model dispatch mechanics and subagent model resolution; -- parent and child sessions; -- subagent dispatch and lifecycle; -- worktree isolation; -- missions, schedules, background work, resume, and retained children; -- external CLI/job agent runners; -- workflow receipts and runtime artifacts. +- physical model/provider execution and virtual-model dispatch mechanics; +- parent/child sessions and subagent lifecycle; +- worktree isolation, missions, schedules, background work, resume, and retained children; +- external CLI/job runners; +- reviewer execution, review loops, and runtime acceptance evidence; +- pull-request verification, CI evidence collection, freshness checks, and merge operations. + +`pi-subagents` is the primary child/runtime primitive. When installed, +`pi-gauntlet` provides higher-level engineering workflows such as `gatekeep-pr`. +ForgeFlow does not wrap those capabilities in a second workflow layer. ### ForgeFlow owns -- stable engineering model roles (`forgeflow/planner`, `worker`, `reviewer`, `scout`, `oracle`) and their thin operator-controlled role policy; +- stable engineering model roles (`forgeflow/planner`, `worker`, `reviewer`, `scout`, `oracle`); - a deterministic engineering-invariant preflight; - the one-writer-per-worktree governance rule; -- the trusted `forgeflow.review` resource; -- the trusted `forgeflow.accept` exact-head acceptance resource; -- deterministic parsing of reviewer verdicts and GitHub check evidence. +- policy text requiring evidence to remain bound to the current candidate. -If a new feature needs generic execution, scheduling, workflow state, provider/channel routing, worktrees, -or resume, it belongs in Pi/pi-subagents or the provider layer rather than ForgeFlow. ForgeFlow may select a physical model for an engineering role through Pi's native virtual-model primitive, but it must not own API transport, credentials, channel weights, quota routing, or a second model runtime. +If a feature needs generic execution, review orchestration, PR acceptance, +scheduling, durable workflow state, provider/channel routing, worktrees, or resume, +it belongs in Pi, an existing plugin, or the provider layer rather than ForgeFlow. ## Model policy boundary -ForgeFlow registers stable logical roles as Pi virtual models. Their physical model mapping is read from operator policy rather than hard-coded into workflows or subagent definitions. For native pi-subagents children, ForgeFlow registers its own extension as a required child extension so those virtual roles are present even though local foreground children intentionally skip parent ambient-extension discovery. New user/direct requests resolve the current mapping, while continuation/retry requests stay on the physical model already handling the turn to preserve prompt-cache and reasoning-signature continuity. +ForgeFlow registers stable logical roles as Pi virtual models. Their physical model +mapping is read from operator policy rather than hard-coded into workflows or agent +definitions. + +For native pi-subagents children, ForgeFlow registers its own extension as a +required child extension. Local foreground children intentionally skip parent +ambient-extension discovery, so this keeps the `forgeflow/*` roles available in +foreground, detached, nested, and recovery child sessions without hard-coding an +installation path in operator profile settings. + +New user/direct requests resolve the current role mapping. Continuation/retry +requests stay on the physical model already handling the turn to preserve cache and +reasoning-signature continuity. -Project-local `.pi/forgeflow-models.json` is considered only when Pi reports the project trusted. User-level policy under `~/.pi/forgeflow-models.json` remains available for untrusted projects. `FORGEFLOW_MODEL_POLICY` is an explicit operator override. +Project-local `.pi/forgeflow-models.json` is considered only when Pi reports the +project trusted. User-level policy under `~/.pi/forgeflow-models.json` remains +available for untrusted projects. `FORGEFLOW_MODEL_POLICY` is an explicit operator +override. -Provider gateways such as LiteLLM/New API/Bifrost sit below this boundary: ForgeFlow chooses the physical model; the provider plane chooses endpoint/channel/key for that already-selected model. This avoids double semantic routing. +Provider gateways such as LiteLLM sit below this boundary: ForgeFlow chooses a +logical role and Pi resolves its physical model; the provider plane chooses the +endpoint/channel/key for that already-selected physical model. See `docs/model-policy.md` for the v1 schema and role mapping. ## Invariant preflight -`extension/invariants.js` is the retained learning layer. It contains a small -stable catalog covering owner validation, product time, identity ownership, +`extension/invariants.js` is the retained engineering-policy layer. It contains a +small stable catalog covering owner validation, product time, identity ownership, lifecycle/tombstones, retry/idempotency, path parity, preflight-before-mutation, stable ordering, host-owned facts, and single-truth cutovers. The preflight is deterministic and adds no model call or durable workflow state. -## Independent review - -`forgeflow.review` launches the builtin Pi reviewer agent with fresh context and -explicitly selects the stable `forgeflow/reviewer` virtual model. The reviewer is -read-only. Its full report is persisted to an absolute path under `.pi/subagents/`. - -A Pi typed gate executes `scripts/review-verdict.mjs` against that same report. -Only Pi's canonical merge-verdict forms are accepted: - -- `Merge verdict: BLOCK|OK|OK with notes` -- the builtin reviewer's Markdown-list form of the same line. - -Anything else fails closed. - -## Exact-head acceptance - -`forgeflow.accept` performs four independent gates in order: +## Review and delivery -1. **HEAD before review** — the worktree is clean and equals `expectedHead`. -2. **Fresh review** — the Pi reviewer running as `forgeflow/reviewer` must return a clean typed verdict. -3. **HEAD after review** — review must not have changed the candidate. -4. **GitHub exact-head** — the PR is open, targets the expected base, still - points at `expectedHead`, and every configured required check resolves to - the newest unambiguous terminal `success`. +ForgeFlow does not implement a reviewer runtime or PR acceptance workflow. -The reviewer evaluates code/contracts/repository-local evidence. Live GitHub CI -state is deliberately owned by the final host gate, avoiding a circular -requirement for a read-only reviewer to prove external check-run state. +Use the reviewer and acceptance primitives already supplied by `pi-subagents`. +For GitHub PR delivery, `pi-gauntlet`'s `gatekeep-pr` skill (when installed) +already handles exact-head CI evidence, head freshness, review, compare-and-swap, +and head-matched merge execution. Adding a second ForgeFlow implementation would +create two sources of truth for the same gate. ## Retired architecture -The former Python/LangGraph/Open SWE control plane, its provider routing, -attempt ledger, custom agent wrappers, Docker sandbox deployment, supervisors, -and GCP systemd units were removed during the Pi-native cutover. Git history is -the audit record for that retired implementation. +The former Python/LangGraph/Open SWE control plane, provider routing, attempt ledger, +custom agent wrappers, Docker sandbox deployment, supervisors, and GCP systemd units +were removed during the Pi-native cutover. Git history is the audit record for that +retired implementation. diff --git a/docs/model-policy.md b/docs/model-policy.md index 0ac2a07..8209de3 100644 --- a/docs/model-policy.md +++ b/docs/model-policy.md @@ -45,7 +45,7 @@ The pinned `pi-subagents@0.75.0` includes the child-runtime fixes required for P } ``` -The parent Pi session can select `forgeflow/planner` when the parent is acting as the planning/orchestration role. ForgeFlow's trusted `forgeflow.review` and `forgeflow.accept` workflows explicitly request `forgeflow/reviewer`, so the workflow owns the stable reviewer role while operator policy remains free to change the physical reviewer model without editing workflow code. +The parent Pi session can select `forgeflow/planner` when acting as the planning/orchestration role. Reviewer and acceptance plugins should select `forgeflow/reviewer` when they need the stable reviewer role; operator policy remains free to change the physical reviewer model without editing plugin workflows or agent definitions. `pi-subagents@0.75.0` contains the upstream child-runtime fixes for queued virtual-model registration and logical-selection verification (#2636 and #2638). ForgeFlow also registers its own extension as a required native-child extension for each Pi session, because local foreground children intentionally do not load the parent's ambient extensions. This makes the `forgeflow/*` virtual models available in foreground, detached, nested, and recovery child sessions without hard-coding an installation path in operator profile settings. External CLI runners remain excluded by pi-subagents. ForgeFlow therefore does not carry a virtual-child compatibility shim. diff --git a/extension/acceptance-workflow.js b/extension/acceptance-workflow.js deleted file mode 100644 index 9bf8896..0000000 --- a/extension/acceptance-workflow.js +++ /dev/null @@ -1,202 +0,0 @@ -import { resolve } from "node:path"; -import { fileURLToPath } from "node:url"; - -import { renderPreflight } from "./invariants.js"; -import { reviewContractTask, reviewVerdictGate } from "./review-workflow.js"; - -const HELPER_PATH = fileURLToPath(new URL("../scripts/github-acceptance.mjs", import.meta.url)); -export const CLEAN_HEAD_COMMAND = - "git diff --quiet && git diff --cached --quiet && test -z \"$(git ls-files --others --exclude-standard | grep -v '^.pi/subagents/' || true)\" && git rev-parse HEAD"; - -function shellQuote(value) { - return `'${String(value).replaceAll("'", "'\\''")}'`; -} - -function validateArgs(args) { - const allowed = new Set(["task", "owner", "repo", "pr", "expectedHead", "baseRef", "requiredChecks"]); - if (Object.keys(args).some((key) => !allowed.has(key))) { - return { error: "forgeflow.accept received unsupported fields." }; - } - - const task = typeof args.task === "string" ? args.task.trim() : ""; - const owner = typeof args.owner === "string" ? args.owner.trim() : ""; - const repo = typeof args.repo === "string" ? args.repo.trim() : ""; - const pr = args.pr; - const expectedHead = typeof args.expectedHead === "string" ? args.expectedHead.trim() : ""; - const baseRef = typeof args.baseRef === "string" ? args.baseRef.trim() : ""; - const requiredChecks = Array.isArray(args.requiredChecks) - ? args.requiredChecks.map((name) => (typeof name === "string" ? name.trim() : name)) - : null; - - if (!task || task.length > 12000) return { error: "task must contain 1-12000 characters." }; - if (!/^[A-Za-z0-9_.-]{1,100}$/.test(owner)) return { error: "owner is invalid." }; - if (!/^[A-Za-z0-9_.-]{1,100}$/.test(repo)) return { error: "repo is invalid." }; - if (!Number.isSafeInteger(pr) || pr <= 0) return { error: "pr must be a positive integer." }; - if (!/^[0-9a-f]{40,64}$/i.test(expectedHead)) { - return { error: "expectedHead must be a full hexadecimal commit id." }; - } - if ( - !baseRef || - baseRef.length > 200 || - /[\\~^:?*\[\]\s]/.test(baseRef) || - baseRef.includes("..") || - baseRef.includes("@{") || - baseRef.startsWith("/") || - baseRef.endsWith("/") || - baseRef.includes("//") - ) { - return { error: "baseRef is invalid." }; - } - if ( - !requiredChecks || - requiredChecks.length === 0 || - requiredChecks.length > 64 || - new Set(requiredChecks).size !== requiredChecks.length || - requiredChecks.some( - (name) => typeof name !== "string" || !name || name.length > 200 || /[\r\n\0]/.test(name) - ) - ) { - return { error: "requiredChecks must contain 1-64 unique check names." }; - } - - return { - task, - owner, - repo, - pr, - expectedHead: expectedHead.toLowerCase(), - baseRef, - requiredChecks - }; -} - -export function createAcceptanceWorkflowDefinition(repoRoot) { - if (typeof repoRoot !== "string" || !repoRoot) { - throw new Error("ForgeFlow acceptance workflow requires the session cwd."); - } - - return { - name: "forgeflow.accept", - version: 4, - resolve(args) { - const validated = validateArgs(args); - if ("error" in validated) return validated; - - const { task, owner, repo, pr, expectedHead, baseRef, requiredChecks } = validated; - const githubCommand = [ - shellQuote(process.execPath), - shellQuote(HELPER_PATH), - "--owner", - shellQuote(owner), - "--repo", - shellQuote(repo), - "--pr", - shellQuote(String(pr)), - "--expected-head", - shellQuote(expectedHead), - "--base-ref", - shellQuote(baseRef), - "--required-checks-json", - shellQuote(JSON.stringify(requiredChecks)) - ].join(" "); - - const reviewTask = reviewContractTask([ - "You are the final independent ForgeFlow reviewer for an exact committed candidate.", - "Review the current repository at the exact checked-out HEAD against the operator task and owner contracts.", - "Do not mutate files. Review code, contracts, tests, and repository-local evidence only.", - "Do not block merely because live GitHub check-run evidence is unavailable to the reviewer: the enclosing trusted workflow performs the authoritative GitHub exact-head CI gate after this review.", - "Use BLOCK when another code change is required before delivery.", - "", - "Operator task:", - task, - "", - renderPreflight(task).join("\n") - ]); - const reviewPath = resolve(repoRoot, `.pi/subagents/forgeflow-final-review-${expectedHead}.md`); - const reviewGate = reviewVerdictGate(reviewPath); - - const headParams = { - kind: "command", - command: CLEAN_HEAD_COMMAND, - timeoutMs: 15000, - role: "gate", - provider: "git" - }; - const githubParams = { - kind: "command", - command: githubCommand, - timeoutMs: 60000, - role: "ci", - provider: "github" - }; - - return { - hostCommands: [ - { key: "head-before-review", command: CLEAN_HEAD_COMMAND }, - { key: "head-after-review", command: CLEAN_HEAD_COMMAND }, - { key: "github-exact-head", command: githubCommand } - ], - script: ` - const expectedHead = ${JSON.stringify(expectedHead)}; - const requestedReviewPath = ${JSON.stringify(reviewPath)}; - - const before = await runs.host("head-before-review", ${JSON.stringify(headParams)}); - if (!before.ok) throw new Error("ForgeFlow requires a clean committed candidate before final review"); - const reviewedHead = before.stdout.trim().toLowerCase(); - if (reviewedHead !== expectedHead) { - throw new Error("ForgeFlow local HEAD does not match expectedHead"); - } - - const review = await runs.run("forgeflow-final-review", { - label: "Review exact ForgeFlow head", - agent: "reviewer", - model: "forgeflow/reviewer", - context: "fresh", - agentContract: { version: 1 }, - task: ${JSON.stringify(reviewTask)}, - output: requestedReviewPath, - outputMode: "file-only", - gate: ${JSON.stringify(reviewGate)} - }); - if (!review.ok || !review.structuredOutput) { - throw new Error("ForgeFlow final reviewer or verdict gate failed"); - } - - const reviewPath = requestedReviewPath; - - if (review.structuredOutput.status === "blocked") { - throw new Error("ForgeFlow final reviewer blocked the candidate; see " + reviewPath); - } - if (review.structuredOutput.status !== "clean") { - throw new Error("ForgeFlow final reviewer verdict contract returned an invalid state"); - } - - const after = await runs.host("head-after-review", ${JSON.stringify(headParams)}); - if (!after.ok || after.stdout.trim().toLowerCase() !== expectedHead) { - throw new Error("ForgeFlow candidate changed during final review"); - } - - const github = await runs.host("github-exact-head", ${JSON.stringify(githubParams)}); - if (!github.ok) throw new Error("ForgeFlow GitHub acceptance command failed"); - - let evidence; - try { - evidence = JSON.parse(github.stdout); - } catch { - throw new Error("ForgeFlow GitHub acceptance returned invalid JSON"); - } - if (evidence.status !== "accepted") { - throw new Error("ForgeFlow exact-head acceptance blocked: " + String(evidence.reason || evidence.status)); - } - - return { - status: "accepted", - reviewedHead: expectedHead, - review: { ...review.structuredOutput, reportPath: reviewPath }, - github: evidence - }; - ` - }; - } - }; -} diff --git a/extension/github-acceptance.js b/extension/github-acceptance.js deleted file mode 100644 index 4b23c33..0000000 --- a/extension/github-acceptance.js +++ /dev/null @@ -1,236 +0,0 @@ -const PASS_CONCLUSIONS = new Set(["success"]); -const TERMINAL_NON_SUCCESS_CONCLUSIONS = new Set(["neutral", "skipped"]); -const FAIL_CONCLUSIONS = new Set([ - "failure", - "cancelled", - "timed_out", - "action_required", - "startup_failure", - "stale" -]); - -function text(value) { - return typeof value === "string" ? value : ""; -} - -function integer(value) { - return Number.isSafeInteger(value) && value >= 0 ? value : -1; -} - -function checkRank(check) { - return [ - text(check?.completed_at) || text(check?.started_at) || text(check?.created_at), - integer(check?.id) - ]; -} - -function statusRank(status) { - return [text(status?.updated_at) || text(status?.created_at), integer(status?.id)]; -} - -function compareRank(left, right) { - if (left[0] !== right[0]) return left[0] < right[0] ? -1 : 1; - return left[1] - right[1]; -} - -function sameRank(left, right) { - return left[0] === right[0] && left[1] === right[1]; -} - -function normalizeLatest(rows, { key, rank, signature, ambiguousPrefix }) { - const grouped = new Map(); - for (const row of rows) { - const name = text(row?.[key]); - if (!name) continue; - const existing = grouped.get(name) ?? []; - existing.push(row); - grouped.set(name, existing); - } - - const result = new Map(); - for (const [name, candidates] of grouped) { - if (candidates.length === 1) { - result.set(name, candidates[0]); - continue; - } - - const ranked = [...candidates].sort((left, right) => - compareRank(rank(right), rank(left)) - ); - const topRank = rank(ranked[0]); - const tied = ranked.filter((row) => sameRank(rank(row), topRank)); - const signatures = new Set(tied.map(signature)); - if ((topRank[0] === "" && topRank[1] === -1) || signatures.size > 1) { - return { error: `${ambiguousPrefix}:${name}` }; - } - result.set(name, ranked[0]); - } - - return { rows: result }; -} - -function normalizeChecks(checkRuns) { - return normalizeLatest(checkRuns, { - key: "name", - rank: checkRank, - signature: (row) => `${text(row?.status)}\0${text(row?.conclusion)}`, - ambiguousPrefix: "AMBIGUOUS_CHECK_NAME" - }); -} - -function normalizeStatuses(statuses) { - return normalizeLatest(statuses, { - key: "context", - rank: statusRank, - signature: (row) => text(row?.state), - ambiguousPrefix: "AMBIGUOUS_STATUS_CONTEXT" - }); -} - -function evaluateRequiredCheck(name, check, status) { - let pending = false; - let unresolved = false; - - if (check) { - const checkStatus = text(check.status).toLowerCase(); - const conclusion = text(check.conclusion).toLowerCase(); - if (checkStatus !== "completed" || !conclusion) { - pending = true; - } else if (FAIL_CONCLUSIONS.has(conclusion)) { - return { status: "blocked", reason: "CI_FAILED", requiredCheck: name }; - } else if (TERMINAL_NON_SUCCESS_CONCLUSIONS.has(conclusion)) { - return { status: "blocked", reason: "CI_NOT_SUCCESSFUL", requiredCheck: name }; - } else if (!PASS_CONCLUSIONS.has(conclusion)) { - unresolved = true; - } - } - - if (status) { - const state = text(status.state).toLowerCase(); - if (state === "pending") { - pending = true; - } else if (state === "failure" || state === "error") { - return { status: "blocked", reason: "CI_FAILED", requiredCheck: name }; - } else if (state !== "success") { - unresolved = true; - } - } - - if (pending) return { status: "pending", reason: "CI_PENDING", requiredCheck: name }; - if (unresolved) { - return { - status: "blocked", - reason: "CI_STATE_UNRECOGNIZED", - requiredCheck: name - }; - } - return null; -} - -function validateExpectedIdentity(expectedHead, baseRef) { - if (!/^[0-9a-f]{40,64}$/i.test(expectedHead ?? "")) { - return { status: "blocked", reason: "EXPECTED_HEAD_INVALID" }; - } - if (typeof baseRef !== "string" || !baseRef) { - return { status: "blocked", reason: "EXPECTED_BASE_INVALID" }; - } - return null; -} - -function evaluatePullRequestIdentityValidated({ pullRequest, expectedHead, baseRef }) { - const prHead = text(pullRequest?.head?.sha); - if (!prHead) { - return { status: "blocked", reason: "PR_HEAD_MISSING" }; - } - if (text(pullRequest?.state).toLowerCase() !== "open") { - return { status: "blocked", reason: "PR_NOT_OPEN", headSha: prHead }; - } - - const observedBase = text(pullRequest?.base?.ref); - if (observedBase !== baseRef) { - return { - status: "blocked", - reason: "PR_BASE_MISMATCH", - headSha: prHead, - expectedBase: baseRef, - observedBase - }; - } - - if (prHead.toLowerCase() !== expectedHead.toLowerCase()) { - return { - status: "blocked", - reason: "PR_HEAD_MISMATCH", - headSha: prHead, - expectedHead - }; - } - - return { status: "matched", headSha: prHead }; -} - -export function evaluatePullRequestIdentity({ pullRequest, expectedHead, baseRef }) { - const invalid = validateExpectedIdentity(expectedHead, baseRef); - if (invalid) return invalid; - return evaluatePullRequestIdentityValidated({ pullRequest, expectedHead, baseRef }); -} - -export function evaluateGithubAcceptance({ - pullRequest, - expectedHead, - baseRef, - requiredChecks, - checkRuns = [], - statuses = [] -}) { - const invalid = validateExpectedIdentity(expectedHead, baseRef); - if (invalid) return invalid; - - if ( - !Array.isArray(requiredChecks) || - requiredChecks.length === 0 || - new Set(requiredChecks).size !== requiredChecks.length || - requiredChecks.some((name) => typeof name !== "string" || !name) - ) { - return { status: "blocked", reason: "REQUIRED_CHECK_POLICY_MISSING" }; - } - - const identity = evaluatePullRequestIdentityValidated({ - pullRequest, - expectedHead, - baseRef - }); - if (identity.status !== "matched") return identity; - const prHead = identity.headSha; - - const normalizedChecks = normalizeChecks(checkRuns); - if (normalizedChecks.error) { - return { status: "blocked", reason: normalizedChecks.error, headSha: prHead }; - } - const normalizedStatuses = normalizeStatuses(statuses); - if (normalizedStatuses.error) { - return { status: "blocked", reason: normalizedStatuses.error, headSha: prHead }; - } - - for (const required of requiredChecks) { - const check = normalizedChecks.rows.get(required); - const status = normalizedStatuses.rows.get(required); - if (!check && !status) { - return { - status: "blocked", - reason: "MISSING_REQUIRED_CHECK", - requiredCheck: required, - headSha: prHead - }; - } - const decision = evaluateRequiredCheck(required, check, status); - if (decision) return { ...decision, headSha: prHead }; - } - - return { - status: "accepted", - reason: null, - headSha: prHead, - requiredChecks: [...requiredChecks] - }; -} diff --git a/extension/index.js b/extension/index.js index fe0165a..49d9feb 100644 --- a/extension/index.js +++ b/extension/index.js @@ -1,21 +1,18 @@ import { fileURLToPath } from "node:url"; import { registerRequiredChildExtensions } from "pi-subagents/required-child-extensions"; -import { registerWorkflowResource } from "pi-subagents/workflow-resources"; import { renderPreflight } from "./invariants.js"; -import { createAcceptanceWorkflowDefinition } from "./acceptance-workflow.js"; -import { createReviewWorkflowDefinition } from "./review-workflow.js"; import { registerForgeFlowVirtualModels } from "./model-policy.js"; const FORGEFLOW_EXTENSION_PATH = fileURLToPath(import.meta.url); const CORE_POLICY = [ - "ForgeFlow is a thin engineering-governance extension for Pi; Pi and pi-subagents own agent execution, sessions, delegation, worktrees, missions, schedules, and resume.", + "ForgeFlow is a thin engineering-governance extension for Pi; Pi and installed plugins own execution, sessions, delegation, review loops, acceptance gates, worktrees, missions, schedules, and resume.", "ForgeFlow may define stable logical model roles, but Pi owns virtual-model dispatch and the provider layer owns channel, credential, quota, and transport routing.", - "Do not create a second agent runtime, workflow database, provider gateway, or duplicate subagent scheduler inside ForgeFlow.", + "Do not create a second agent runtime, workflow database, provider gateway, reviewer runtime, PR gate, or duplicate subagent scheduler inside ForgeFlow.", "Keep one mutation writer per working tree. Independent reviewers must not mutate the candidate under review.", - "Agent completion is evidence, not authority. For PR delivery, final acceptance must be tied to the authoritative current head and must invalidate stale CI/review evidence after every new push.", - "Prefer existing Pi/pi-subagents capabilities over ForgeFlow-specific infrastructure. Add extension code only for engineering policy that Pi does not already own." + "Agent completion is evidence, not authority. For PR delivery, final acceptance must be tied to the authoritative current head and stale evidence must be invalidated after every new push.", + "Prefer existing Pi/plugin capabilities over ForgeFlow-specific infrastructure. Add extension code only for engineering policy that Pi and installed plugins do not already own." ]; export function buildForgeFlowPromptSection(prompt) { @@ -26,8 +23,6 @@ export default function registerForgeFlow(pi) { registerForgeFlowVirtualModels(pi); let requiredChildRegistration; - let reviewRegistration; - let acceptanceRegistration; pi.on("before_agent_start", (event) => { event.systemPromptOptions.sections.forgeflow_policy = buildForgeFlowPromptSection(event.prompt); @@ -35,31 +30,15 @@ export default function registerForgeFlow(pi) { pi.on("session_start", (_event, ctx) => { requiredChildRegistration?.dispose(); - reviewRegistration?.dispose(); - acceptanceRegistration?.dispose(); - const sessionId = ctx.sessionManager.getSessionId(); - const sessionCwd = ctx.cwd; requiredChildRegistration = registerRequiredChildExtensions({ - sessionId, + sessionId: ctx.sessionManager.getSessionId(), extensions: [{ id: "forgeflow", path: FORGEFLOW_EXTENSION_PATH }] }); - reviewRegistration = registerWorkflowResource({ - sessionId, - definition: createReviewWorkflowDefinition(sessionCwd) - }); - acceptanceRegistration = registerWorkflowResource({ - sessionId, - definition: createAcceptanceWorkflowDefinition(sessionCwd) - }); }); pi.on("session_shutdown", () => { requiredChildRegistration?.dispose(); - reviewRegistration?.dispose(); - acceptanceRegistration?.dispose(); requiredChildRegistration = undefined; - reviewRegistration = undefined; - acceptanceRegistration = undefined; }); } diff --git a/extension/review-verdict.js b/extension/review-verdict.js deleted file mode 100644 index 31fbea1..0000000 --- a/extension/review-verdict.js +++ /dev/null @@ -1,45 +0,0 @@ -export const REVIEW_VERDICTS = Object.freeze(["BLOCK", "OK", "OK with notes"]); -const VERDICT_PREFIXES = Object.freeze(["Merge verdict: ", "- Merge verdict: "]); - -export function parseReviewVerdict(report) { - if (typeof report !== "string") { - return { status: "invalid", reason: "REVIEW_REPORT_NOT_TEXT" }; - } - - if (report.trim().length === 0) { - return { status: "invalid", reason: "REVIEW_REPORT_EMPTY" }; - } - - const lines = report - .replaceAll("\r\n", "\n") - .split("\n") - .filter((line) => line.trim().length > 0); - - const lastLine = lines.at(-1); - if (!lastLine) { - return { status: "invalid", reason: "REVIEW_REPORT_EMPTY" }; - } - - const prefix = VERDICT_PREFIXES.find((candidate) => lastLine.startsWith(candidate)); - if (!prefix) { - return { - status: "invalid", - reason: "REVIEW_VERDICT_MISSING", - lastLine - }; - } - - const verdict = lastLine.slice(prefix.length); - if (!REVIEW_VERDICTS.includes(verdict)) { - return { - status: "invalid", - reason: "REVIEW_VERDICT_INVALID", - lastLine - }; - } - - return { - status: verdict === "BLOCK" ? "blocked" : "clean", - verdict - }; -} diff --git a/extension/review-workflow.js b/extension/review-workflow.js deleted file mode 100644 index 10c9b54..0000000 --- a/extension/review-workflow.js +++ /dev/null @@ -1,120 +0,0 @@ -import { randomUUID } from "node:crypto"; -import { resolve } from "node:path"; -import { fileURLToPath } from "node:url"; - -import { renderPreflight } from "./invariants.js"; - -const MAX_TASK_LENGTH = 12000; -const VERDICT_HELPER_PATH = fileURLToPath(new URL("../scripts/review-verdict.mjs", import.meta.url)); - -export const REVIEW_VERDICT_SCHEMA = Object.freeze({ - type: "object", - additionalProperties: false, - properties: { - status: { enum: ["clean", "blocked"] }, - verdict: { enum: ["BLOCK", "OK", "OK with notes"] } - }, - required: ["status", "verdict"] -}); - -function shellQuote(value) { - return `'${String(value).replaceAll("'", "'\\''")}'`; -} - -function validateTask(args) { - const keys = Object.keys(args); - if (keys.some((key) => key !== "task")) { - return { error: "forgeflow.review supports only the task field." }; - } - if (typeof args.task !== "string") { - return { error: "task must be a string." }; - } - const task = args.task.trim(); - if (!task || task.length > MAX_TASK_LENGTH) { - return { error: `task must contain 1-${MAX_TASK_LENGTH} characters.` }; - } - return { task }; -} - -export function reviewContractTask(lines) { - return [ - ...lines, - "", - "Use your normal reviewer report format for findings and evidence.", - "End with Pi's standard merge verdict contract: Merge verdict: BLOCK, OK, or OK with notes.", - "The builtin reviewer may render that final verdict as its final Markdown list item.", - "Do not add any text after the final merge verdict." - ].join("\n"); -} - -export function reviewVerdictCommand(reportPath) { - return [ - shellQuote(process.execPath), - shellQuote(VERDICT_HELPER_PATH), - "--report", - shellQuote(reportPath) - ].join(" "); -} - -export function reviewVerdictGate(reportPath) { - return { - command: reviewVerdictCommand(reportPath), - output: "json", - schema: REVIEW_VERDICT_SCHEMA, - timeoutMs: 15000 - }; -} - -export function createReviewWorkflowDefinition(repoRoot) { - if (typeof repoRoot !== "string" || !repoRoot) { - throw new Error("ForgeFlow review workflow requires the session cwd."); - } - - return { - name: "forgeflow.review", - version: 4, - resolve(args) { - const validated = validateTask(args); - if ("error" in validated) return validated; - - const task = validated.task; - const preflight = renderPreflight(task).join("\n"); - const reviewTask = reviewContractTask([ - "You are the independent ForgeFlow reviewer.", - "Review the current repository diff against the operator task and the existing owner contracts.", - "Do not mutate files.", - "A child or implementation agent claiming success is not acceptance.", - "Report only evidence-backed findings. Use BLOCK when any finding requires another code change before delivery.", - "", - "Operator task:", - task, - "", - preflight - ]); - const reportPath = resolve(repoRoot, `.pi/subagents/forgeflow-review-${randomUUID()}.md`); - const gate = reviewVerdictGate(reportPath); - - return { - script: ` - const requestedReportPath = ${JSON.stringify(reportPath)}; - const review = await runs.run("forgeflow-review", { - label: "Review ForgeFlow candidate", - agent: "reviewer", - model: "forgeflow/reviewer", - context: "fresh", - agentContract: { version: 1 }, - task: ${JSON.stringify(reviewTask)}, - output: requestedReportPath, - outputMode: "file-only", - gate: ${JSON.stringify(gate)} - }); - if (!review.ok || !review.structuredOutput) { - throw new Error("ForgeFlow independent reviewer or verdict gate failed"); - } - - return { ...review.structuredOutput, reportPath: requestedReportPath }; - ` - }; - } - }; -} diff --git a/package.json b/package.json index 3dbdd33..0dfb822 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "Thin software-engineering governance for Pi Agent and pi-subagents.", "type": "module", "license": "MIT", - "files": ["extension", "scripts", "skills", "docs/architecture.md", "docs/model-policy.md", "README.md", "LICENSE"], + "files": ["extension", "skills", "docs/architecture.md", "docs/model-policy.md", "README.md", "LICENSE"], "keywords": [ "pi-package", "pi", @@ -35,6 +35,6 @@ }, "scripts": { "test": "node --test tests/*.test.js", - "check": "npm test && node --check extension/index.js && node --check extension/model-policy.js && node --check extension/invariants.js && node --check extension/review-workflow.js && node --check extension/review-verdict.js && node --check extension/github-acceptance.js && node --check extension/acceptance-workflow.js && node --check scripts/review-verdict.mjs && node --check scripts/github-acceptance.mjs" + "check": "npm test && node --check extension/index.js && node --check extension/model-policy.js && node --check extension/invariants.js" } } diff --git a/scripts/github-acceptance.mjs b/scripts/github-acceptance.mjs deleted file mode 100644 index 0a965a0..0000000 --- a/scripts/github-acceptance.mjs +++ /dev/null @@ -1,158 +0,0 @@ -#!/usr/bin/env node - -import { execFileSync } from "node:child_process"; -import { - evaluateGithubAcceptance, - evaluatePullRequestIdentity -} from "../extension/github-acceptance.js"; - -function fail(message) { - process.stderr.write(`${message}\n`); - process.exit(2); -} - -function parseArgs(argv) { - const result = {}; - if (argv.length % 2 !== 0) fail("Invalid arguments."); - for (let index = 0; index < argv.length; index += 2) { - const key = argv[index]; - const value = argv[index + 1]; - if (!key?.startsWith("--") || value === undefined) fail("Invalid arguments."); - const name = key.slice(2); - if (!name || Object.hasOwn(result, name)) fail("Invalid arguments."); - result[name] = value; - } - return result; -} - -function ghJson(args) { - let stdout; - try { - stdout = execFileSync("gh", args, { - encoding: "utf8", - stdio: ["ignore", "pipe", "pipe"], - maxBuffer: 2 * 1024 * 1024 - }); - } catch (error) { - const stderr = typeof error?.stderr === "string" ? error.stderr.trim() : ""; - fail(`GitHub CLI request failed${stderr ? `: ${stderr}` : "."}`); - } - try { - return JSON.parse(stdout); - } catch { - fail("GitHub CLI returned invalid JSON."); - } -} - -const args = parseArgs(process.argv.slice(2)); -const allowed = new Set(["owner", "repo", "pr", "expected-head", "base-ref", "required-checks-json"]); -if (Object.keys(args).some((key) => !allowed.has(key))) fail("Invalid arguments."); - -const owner = args.owner; -const repo = args.repo; -const pr = Number(args.pr); -const expectedHead = args["expected-head"]; -const baseRef = args["base-ref"]; -let requiredChecks; - -if (!/^[A-Za-z0-9_.-]{1,100}$/.test(owner ?? "")) fail("Invalid owner."); -if (!/^[A-Za-z0-9_.-]{1,100}$/.test(repo ?? "")) fail("Invalid repo."); -if (!Number.isSafeInteger(pr) || pr <= 0) fail("Invalid pull request number."); -if (!/^[0-9a-f]{40,64}$/i.test(expectedHead ?? "")) fail("Invalid expected head."); -if ( - typeof baseRef !== "string" || - !baseRef || - baseRef.length > 200 || - /[\\~^:?*\[\]\s]/.test(baseRef) || - baseRef.includes("..") || - baseRef.includes("@{") || - baseRef.startsWith("/") || - baseRef.endsWith("/") || - baseRef.includes("//") -) { - fail("Invalid base ref."); -} -try { - requiredChecks = JSON.parse(args["required-checks-json"]); -} catch { - fail("Invalid required checks."); -} -if ( - !Array.isArray(requiredChecks) || - requiredChecks.length === 0 || - requiredChecks.length > 64 || - requiredChecks.some( - (name) => typeof name !== "string" || !name.trim() || name.length > 200 || /[\r\n\0]/.test(name) - ) -) { - fail("Invalid required checks."); -} -requiredChecks = requiredChecks.map((name) => name.trim()); -if (new Set(requiredChecks).size !== requiredChecks.length) fail("Invalid required checks."); - -const PAGE_SIZE = 100; -const MAX_PAGES = 20; - -function checkRunsForHead() { - const rows = []; - for (let page = 1; page <= MAX_PAGES; page += 1) { - const payload = ghJson([ - "api", - "-H", - "Accept: application/vnd.github+json", - `repos/${owner}/${repo}/commits/${expectedHead}/check-runs?per_page=${PAGE_SIZE}&page=${page}` - ]); - const pageRows = Array.isArray(payload?.check_runs) ? payload.check_runs : []; - rows.push(...pageRows); - const total = Number(payload?.total_count); - if ((Number.isSafeInteger(total) && rows.length >= total) || pageRows.length < PAGE_SIZE) { - return rows; - } - } - fail("GitHub check-run evidence exceeds the supported pagination limit."); -} - -function statusesForHead() { - const rows = []; - for (let page = 1; page <= MAX_PAGES; page += 1) { - const payload = ghJson([ - "api", - "-H", - "Accept: application/vnd.github+json", - `repos/${owner}/${repo}/commits/${expectedHead}/statuses?per_page=${PAGE_SIZE}&page=${page}` - ]); - if (!Array.isArray(payload)) fail("GitHub commit-status response was not an array."); - rows.push(...payload); - if (payload.length < PAGE_SIZE) return rows; - } - fail("GitHub commit-status evidence exceeds the supported pagination limit."); -} - -const initialPullRequest = ghJson(["api", `repos/${owner}/${repo}/pulls/${pr}`]); -const initialIdentity = evaluatePullRequestIdentity({ - pullRequest: initialPullRequest, - expectedHead, - baseRef -}); -if (initialIdentity.status !== "matched") { - process.stdout.write(`${JSON.stringify(initialIdentity)}\n`); - process.exit(0); -} - -const checkRuns = checkRunsForHead(); -const statuses = statusesForHead(); - -// Re-read the authoritative PR after collecting CI evidence. A force-push, -// close, or retarget during those API calls invalidates this acceptance turn. -const finalPullRequest = ghJson(["api", `repos/${owner}/${repo}/pulls/${pr}`]); - -const result = evaluateGithubAcceptance({ - pullRequest: finalPullRequest, - expectedHead, - baseRef, - requiredChecks, - checkRuns, - statuses -}); - -process.stdout.write(`${JSON.stringify(result)}\n`); diff --git a/scripts/review-verdict.mjs b/scripts/review-verdict.mjs deleted file mode 100644 index d1587ff..0000000 --- a/scripts/review-verdict.mjs +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/env node - -import { readFileSync } from "node:fs"; -import { isAbsolute, relative, resolve } from "node:path"; - -import { parseReviewVerdict } from "../extension/review-verdict.js"; - -function fail(reason) { - process.stdout.write(`${JSON.stringify({ status: "invalid", reason })}\n`); - process.exit(2); -} - -const argv = process.argv.slice(2); -if (argv.length !== 2 || argv[0] !== "--report") fail("INVALID_ARGUMENTS"); - -const report = argv[1]; -if ( - typeof report !== "string" || - !report || - report.length > 1024 || - report.includes("\0") -) { - fail("INVALID_REPORT_PATH"); -} - -const cwd = process.cwd(); -const allowedRoot = resolve(cwd, ".pi/subagents"); -const absolute = isAbsolute(report) ? resolve(report) : resolve(cwd, report); -const rel = relative(allowedRoot, absolute); -if (!rel || rel.startsWith("..") || isAbsolute(rel)) fail("REPORT_PATH_OUTSIDE_REVIEW_ROOT"); - -let content; -try { - content = readFileSync(absolute, "utf8"); -} catch { - fail("REVIEW_REPORT_UNREADABLE"); -} - -if (Buffer.byteLength(content, "utf8") > 256 * 1024) fail("REVIEW_REPORT_TOO_LARGE"); - -const result = parseReviewVerdict(content); -process.stdout.write(`${JSON.stringify(result)}\n`); -if (result.status === "invalid") process.exit(2); diff --git a/skills/forgeflow/SKILL.md b/skills/forgeflow/SKILL.md index 3210691..688a8ec 100644 --- a/skills/forgeflow/SKILL.md +++ b/skills/forgeflow/SKILL.md @@ -1,66 +1,46 @@ --- name: forgeflow description: | - Pi-native software-engineering governance: preserve one writer per worktree, - use pi-subagents for delegation, require independent review where useful, and - treat exact-revision repository evidence as stronger than agent success text. + Thin Pi-native engineering policy: preserve one writer per worktree, use stable + logical model roles, apply deterministic engineering invariants, and reuse + installed Pi plugins instead of creating a second execution or review runtime. --- # ForgeFlow ForgeFlow is policy for Pi, not another agent runtime. -Use Pi as the parent execution kernel and `pi-subagents` for child agents, -missions, schedules, worktree isolation, resume, background execution, external -CLI agents, and ordinary acceptance evidence. Do not recreate those mechanisms -inside ForgeFlow. +Use Pi as the execution kernel. Reuse installed plugins for delegation, review, +acceptance, worktrees, missions, schedules, resume, and PR gating. Do not recreate +those mechanisms inside ForgeFlow. -## Core execution rules +## Core rules - Keep one mutation writer per working tree. -- Use fresh-context reviewers for independent review. Reviewers do not mutate the - candidate they are judging. +- Use fresh-context reviewers when independent review is useful; reviewers do not + mutate the candidate they judge. - Treat worker success, reviewer prose, and external receipts as evidence rather than final authority. -- When delivery uses a pull request, bind final acceptance to the current - authoritative head. A new push invalidates CI and review evidence for the old - head. +- Bind delivery evidence to the current candidate. A new push invalidates evidence + for the old head. - Prefer deterministic repository checks over model claims. -- Repair the same candidate/worktree/PR when possible; do not silently fork a - second implementation truth. -- Reuse Pi and pi-subagents primitives before adding ForgeFlow code. -- Refer to model roles through `forgeflow/planner`, `forgeflow/worker`, `forgeflow/reviewer`, `forgeflow/scout`, and `forgeflow/oracle` rather than hard-coding fast-moving physical model IDs into workflows. -- Keep model selection separate from provider/channel selection: ForgeFlow role policy may choose the physical model through Pi virtual models; provider gateways own endpoint, key, quota, weight, health, and transport selection. - -## Review - -Use the trusted `forgeflow.review` workflow when an independent review is -needed before final delivery. The workflow selects the stable `forgeflow/reviewer` -virtual model; Pi's read-only reviewer writes its full evidence-backed report to -a managed artifact and must end with Pi's standard merge-verdict -contract: `Merge verdict: BLOCK|OK|OK with notes`, either as the plain final line -or the builtin reviewer's final Markdown list item. ForgeFlow deterministically -validates only those canonical forms and returns the artifact path plus a `clean` -or `blocked` state. Missing or malformed verdicts fail closed. The reviewer report remains review -evidence rather than final repository acceptance. - -## Final pull-request acceptance - -Use the trusted `forgeflow.accept` workflow for a committed pull-request -candidate. Supply the task, repository owner/name, pull-request number, expected -base branch, full expected head commit, and the repository's explicit required -check names. Missing required-check policy fails closed rather than guessing from -whatever checks happened to start. - -The workflow fails closed unless: - -- the local working tree is clean and its HEAD equals the expected commit before - review; -- a fresh read-only reviewer returns a clean structured verdict; -- the local candidate is still clean and on the same HEAD after review; -- GitHub reports the pull request open at that exact head; and -- the observed GitHub check-runs and commit statuses for that head are terminal - and successful. - -Any new push requires a new acceptance run for the new head. Do not reuse an -older reviewer or CI result. +- Repair the same candidate/worktree/PR when possible; do not fork a second + implementation truth without an explicit reason. +- Refer to engineering roles through `forgeflow/planner`, `forgeflow/worker`, + `forgeflow/reviewer`, `forgeflow/scout`, and `forgeflow/oracle` instead of + hard-coding fast-moving physical model IDs. +- Keep model selection separate from provider/channel selection. Provider + infrastructure owns endpoints, credentials, channel health, weights, quotas, and + transport. + +## Reuse plugin surfaces + +Use `pi-subagents` for child execution, reviewer runs, review loops, typed gates, +and runtime acceptance evidence. + +When `pi-gauntlet` is installed, use its `gatekeep-pr` skill for pull-request +verification and merge gating. It owns exact-head CI evidence, freshness checks, +review orchestration, and head-matched merge safety. + +Do not add ForgeFlow-specific wrappers around those plugin surfaces unless a +concrete missing policy cannot be expressed through the existing plugin contract. diff --git a/tests/acceptance-workflow.test.js b/tests/acceptance-workflow.test.js deleted file mode 100644 index 0afdc45..0000000 --- a/tests/acceptance-workflow.test.js +++ /dev/null @@ -1,115 +0,0 @@ -import assert from "node:assert/strict"; -import test from "node:test"; - -import { - CLEAN_HEAD_COMMAND, - createAcceptanceWorkflowDefinition -} from "../extension/acceptance-workflow.js"; - -const HEAD = "a".repeat(40); -const ROOT = process.cwd(); - -function validArgs(overrides = {}) { - return { - task: "Ship the retry fix", - owner: "BakerSean168", - repo: "forgeflow", - pr: 106, - expectedHead: HEAD, - baseRef: "main", - requiredChecks: ["verify"], - ...overrides - }; -} - -test("acceptance resource validates public identity and CI policy fields", () => { - const definition = createAcceptanceWorkflowDefinition(ROOT); - - assert.deepEqual(definition.resolve({ ...validArgs(), command: "echo nope" }), { - error: "forgeflow.accept received unsupported fields." - }); - assert.deepEqual(definition.resolve(validArgs({ owner: "bad owner" })), { - error: "owner is invalid." - }); - assert.deepEqual(definition.resolve(validArgs({ pr: 0 })), { - error: "pr must be a positive integer." - }); - assert.deepEqual(definition.resolve(validArgs({ expectedHead: "abc" })), { - error: "expectedHead must be a full hexadecimal commit id." - }); - assert.deepEqual(definition.resolve(validArgs({ baseRef: "../main" })), { - error: "baseRef is invalid." - }); - assert.deepEqual(definition.resolve(validArgs({ requiredChecks: [] })), { - error: "requiredChecks must contain 1-64 unique check names." - }); - assert.deepEqual(definition.resolve(validArgs({ requiredChecks: ["verify", " verify "] })), { - error: "requiredChecks must contain 1-64 unique check names." - }); -}); - -test("acceptance binds Pi typed reviewer gate and GitHub evidence to one exact committed head", () => { - const definition = createAcceptanceWorkflowDefinition(ROOT); - const result = definition.resolve(validArgs()); - - assert.equal(definition.version, 4); - assert.equal("error" in result, false); - assert.equal(result.hostCommands.length, 3); - assert.deepEqual( - result.hostCommands.map((entry) => entry.key), - ["head-before-review", "head-after-review", "github-exact-head"] - ); - assert.equal(result.hostCommands[0].command, CLEAN_HEAD_COMMAND); - assert.equal(result.hostCommands[1].command, CLEAN_HEAD_COMMAND); - assert.match(result.hostCommands[2].command, /github-acceptance\.mjs/); - assert.match(result.hostCommands[2].command, /--expected-head 'a{40}'/); - assert.match(result.hostCommands[2].command, /--base-ref 'main'/); - assert.match(result.hostCommands[2].command, /--required-checks-json '\["verify"\]'/); - - assert.match(result.script, /context: "fresh"/); - assert.match(result.script, /agent: "reviewer"/); - assert.match(result.script, /model: "forgeflow\/reviewer"/); - assert.match(result.script, /agentContract: \{ version: 1 \}/); - assert.match(result.script, /outputMode: "file-only"/); - assert.match(result.script, /standard merge verdict contract/); - assert.match(result.script, /authoritative GitHub exact-head CI gate after this review/); - assert.match(result.script, /review-verdict\.mjs/); - assert.match(result.script, /"output":"json"/); - assert.match(result.script, /head-before-review/); - assert.match(result.script, /head-after-review/); - assert.match(result.script, /github-exact-head/); - assert.match(result.script, /const reviewPath = requestedReviewPath/); - assert.match(result.script, /review\.structuredOutput\.status === "blocked"/); - assert.match(result.script, /evidence\.status !== "accepted"/); - assert.doesNotMatch(result.script, /outputPathMapping/); - assert.doesNotMatch(result.script, /artifactPaths/); - assert.doesNotMatch(result.script, /outputReference/); - assert.doesNotMatch(result.script, /outputSchema/); - - const reviewIndex = result.script.indexOf('await runs.run("forgeflow-final-review"'); - const headAfterIndex = result.script.indexOf('await runs.host("head-after-review"'); - const githubIndex = result.script.indexOf('await runs.host("github-exact-head"'); - assert.ok(reviewIndex >= 0); - assert.ok(headAfterIndex > reviewIndex); - assert.ok(githubIndex > headAfterIndex); -}); - -test("task text cannot widen host or typed gate command authority", () => { - const definition = createAcceptanceWorkflowDefinition(ROOT); - const injectedTask = "Review this; rm -rf /; $(touch /tmp/nope)"; - const result = definition.resolve(validArgs({ task: injectedTask })); - - assert.equal("error" in result, false); - for (const grant of result.hostCommands) { - assert.equal(grant.command.includes(injectedTask), false); - assert.equal(grant.command.includes("rm -rf"), false); - assert.equal(grant.command.includes("touch /tmp/nope"), false); - } - - const gateIndex = result.script.indexOf("review-verdict.mjs"); - assert.notEqual(gateIndex, -1); - const gateWindow = result.script.slice(gateIndex, gateIndex + 1200); - assert.equal(gateWindow.includes("rm -rf"), false); - assert.equal(gateWindow.includes("touch /tmp/nope"), false); - assert.match(result.script, /rm -rf/); -}); diff --git a/tests/architecture-boundary.test.js b/tests/architecture-boundary.test.js index 2f07cd7..672b818 100644 --- a/tests/architecture-boundary.test.js +++ b/tests/architecture-boundary.test.js @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { execFileSync } from "node:child_process"; -import { readFileSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import test from "node:test"; const FORBIDDEN_PREFIXES = [ @@ -17,6 +17,15 @@ const FORBIDDEN_FILES = new Set([ "deploy/gcp-dev/open-swe-codex-broker.service.in" ]); +const PLUGIN_OWNED_RUNTIME_FILES = [ + "extension/review-workflow.js", + "extension/review-verdict.js", + "extension/acceptance-workflow.js", + "extension/github-acceptance.js", + "scripts/review-verdict.mjs", + "scripts/github-acceptance.mjs" +]; + function trackedFiles() { return execFileSync("git", ["ls-files"], { encoding: "utf8" }) .split("\n") @@ -37,7 +46,13 @@ test("repository does not reintroduce the retired Python/Open SWE control plane" } }); -test("Pi package has one explicit runtime entrypoint and one execution dependency", () => { +test("review and PR acceptance remain plugin-owned", () => { + for (const path of PLUGIN_OWNED_RUNTIME_FILES) { + assert.equal(existsSync(path), false, path); + } +}); + +test("Pi package has one explicit runtime entrypoint and one plugin dependency", () => { const packageJson = JSON.parse(readFileSync("package.json", "utf8")); assert.deepEqual(packageJson.pi.extensions, ["./extension/index.js"]); @@ -46,6 +61,6 @@ test("Pi package has one explicit runtime entrypoint and one execution dependenc assert.equal(packageJson.peerDependencies["@earendil-works/pi-coding-agent"], ">=0.99.0"); assert.deepEqual( new Set(packageJson.files), - new Set(["extension", "scripts", "skills", "docs/architecture.md", "docs/model-policy.md", "README.md", "LICENSE"]) + new Set(["extension", "skills", "docs/architecture.md", "docs/model-policy.md", "README.md", "LICENSE"]) ); }); diff --git a/tests/github-acceptance-helper.test.js b/tests/github-acceptance-helper.test.js deleted file mode 100644 index 41e55f4..0000000 --- a/tests/github-acceptance-helper.test.js +++ /dev/null @@ -1,109 +0,0 @@ -import assert from "node:assert/strict"; -import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { join, resolve } from "node:path"; -import { spawnSync } from "node:child_process"; -import test from "node:test"; - -const EXPECTED_HEAD = "a".repeat(40); - -function runHelper(finalHead) { - const dir = mkdtempSync(join(tmpdir(), "forgeflow-gh-helper-")); - const state = join(dir, "state"); - const fakeGh = join(dir, "gh"); - - writeFileSync( - fakeGh, - `#!/bin/sh -set -eu -last="" -for arg in "$@"; do last="$arg"; done - -case "$last" in - repos/BakerSean168/forgeflow/pulls/107) - count=0 - if [ -f "$FAKE_GH_STATE" ]; then count=$(cat "$FAKE_GH_STATE"); fi - count=$((count + 1)) - printf '%s' "$count" > "$FAKE_GH_STATE" - if [ "$count" -eq 1 ]; then head="$FAKE_EXPECTED_HEAD"; else head="$FAKE_FINAL_HEAD"; fi - printf '{"state":"open","head":{"sha":"%s"},"base":{"ref":"main"}}\\n' "$head" - ;; - *"/check-runs?per_page=100&page=1") - printf '{"total_count":1,"check_runs":[{"id":1,"name":"verify","status":"completed","conclusion":"success"}]}\\n' - ;; - *"/statuses?per_page=100&page=1") - printf '[]\\n' - ;; - *) - echo "unexpected gh call: $*" >&2 - exit 9 - ;; -esac -`, - "utf8" - ); - chmodSync(fakeGh, 0o755); - - const helper = resolve("scripts/github-acceptance.mjs"); - const result = spawnSync( - process.execPath, - [ - helper, - "--owner", - "BakerSean168", - "--repo", - "forgeflow", - "--pr", - "107", - "--expected-head", - EXPECTED_HEAD, - "--base-ref", - "main", - "--required-checks-json", - '["verify"]' - ], - { - cwd: process.cwd(), - encoding: "utf8", - env: { - ...process.env, - PATH: `${dir}:${process.env.PATH ?? ""}`, - FAKE_GH_STATE: state, - FAKE_EXPECTED_HEAD: EXPECTED_HEAD, - FAKE_FINAL_HEAD: finalHead - } - } - ); - - try { - assert.equal(result.status, 0, result.stderr); - return { - output: JSON.parse(result.stdout), - pullRequestReads: Number(readFileSync(state, "utf8")) - }; - } finally { - rmSync(dir, { recursive: true, force: true }); - } -} - -test("helper rechecks the authoritative PR after collecting CI evidence", () => { - const result = runHelper("b".repeat(40)); - - assert.equal(result.pullRequestReads, 2); - assert.equal(result.output.status, "blocked"); - assert.equal(result.output.reason, "PR_HEAD_MISMATCH"); - assert.equal(result.output.expectedHead, EXPECTED_HEAD); - assert.equal(result.output.headSha, "b".repeat(40)); -}); - -test("helper accepts only when the final PR identity still matches the checked head", () => { - const result = runHelper(EXPECTED_HEAD); - - assert.equal(result.pullRequestReads, 2); - assert.deepEqual(result.output, { - status: "accepted", - reason: null, - headSha: EXPECTED_HEAD, - requiredChecks: ["verify"] - }); -}); diff --git a/tests/github-acceptance.test.js b/tests/github-acceptance.test.js deleted file mode 100644 index 169518c..0000000 --- a/tests/github-acceptance.test.js +++ /dev/null @@ -1,176 +0,0 @@ -import assert from "node:assert/strict"; -import test from "node:test"; - -import { evaluateGithubAcceptance } from "../extension/github-acceptance.js"; - -const HEAD = "a".repeat(40); -const BASE = "main"; -const REQUIRED = ["verify"]; - -function pr(overrides = {}) { - return { - state: "open", - head: { sha: HEAD }, - base: { ref: BASE }, - ...overrides - }; -} - -function evaluate(overrides = {}) { - return evaluateGithubAcceptance({ - pullRequest: pr(), - expectedHead: HEAD, - baseRef: BASE, - requiredChecks: REQUIRED, - checkRuns: [{ id: 1, name: "verify", status: "completed", conclusion: "success" }], - statuses: [], - ...overrides - }); -} - -test("accepts only an open PR at the expected base/head with required CI passing", () => { - const result = evaluate(); - - assert.deepEqual(result, { - status: "accepted", - reason: null, - headSha: HEAD, - requiredChecks: REQUIRED - }); -}); - -test("blocks when the authoritative PR head differs from the reviewed head", () => { - const result = evaluate({ - pullRequest: pr({ head: { sha: "b".repeat(40) } }) - }); - - assert.equal(result.status, "blocked"); - assert.equal(result.reason, "PR_HEAD_MISMATCH"); - assert.equal(result.expectedHead, HEAD); -}); - -test("blocks when the pull request is retargeted to another base", () => { - const result = evaluate({ - pullRequest: pr({ base: { ref: "release" } }) - }); - - assert.equal(result.status, "blocked"); - assert.equal(result.reason, "PR_BASE_MISMATCH"); - assert.equal(result.expectedBase, BASE); - assert.equal(result.observedBase, "release"); -}); - -test("reports pending when a required CI signal is still pending", () => { - const result = evaluate({ - checkRuns: [{ id: 1, name: "verify", status: "in_progress", conclusion: null }] - }); - - assert.equal(result.status, "pending"); - assert.equal(result.reason, "CI_PENDING"); - assert.equal(result.requiredCheck, "verify"); -}); - -test("blocks when a required CI signal fails", () => { - const result = evaluate({ - checkRuns: [{ id: 1, name: "verify", status: "completed", conclusion: "failure" }] - }); - - assert.equal(result.status, "blocked"); - assert.equal(result.reason, "CI_FAILED"); - assert.equal(result.requiredCheck, "verify"); -}); - -test("fails closed when required CI policy is missing", () => { - const result = evaluate({ requiredChecks: [] }); - - assert.deepEqual(result, { - status: "blocked", - reason: "REQUIRED_CHECK_POLICY_MISSING" - }); -}); - -test("fails closed when a required check has no evidence", () => { - const result = evaluate({ checkRuns: [], statuses: [] }); - - assert.equal(result.status, "blocked"); - assert.equal(result.reason, "MISSING_REQUIRED_CHECK"); - assert.equal(result.requiredCheck, "verify"); -}); - -test("fails closed on unrecognized required CI conclusions", () => { - const result = evaluate({ - checkRuns: [{ id: 1, name: "verify", status: "completed", conclusion: "mystery" }] - }); - - assert.equal(result.status, "blocked"); - assert.equal(result.reason, "CI_STATE_UNRECOGNIZED"); -}); - -for (const conclusion of ["neutral", "skipped"]) { - test(`blocks terminal non-success required check conclusion: ${conclusion}`, () => { - const result = evaluate({ - checkRuns: [{ id: 1, name: "verify", status: "completed", conclusion }] - }); - - assert.equal(result.status, "blocked"); - assert.equal(result.reason, "CI_NOT_SUCCESSFUL"); - assert.equal(result.requiredCheck, "verify"); - }); -} - -test("uses the newest rerun for a duplicated check name", () => { - const result = evaluate({ - checkRuns: [ - { - id: 10, - name: "verify", - status: "completed", - conclusion: "failure", - completed_at: "2026-10-02T01:00:00Z" - }, - { - id: 11, - name: "verify", - status: "completed", - conclusion: "success", - completed_at: "2026-10-02T01:05:00Z" - } - ] - }); - - assert.equal(result.status, "accepted"); -}); - -test("fails closed when duplicate check evidence cannot be ordered", () => { - const result = evaluate({ - checkRuns: [ - { name: "verify", status: "completed", conclusion: "failure" }, - { name: "verify", status: "completed", conclusion: "success" } - ] - }); - - assert.equal(result.status, "blocked"); - assert.equal(result.reason, "AMBIGUOUS_CHECK_NAME:verify"); -}); - -test("uses the newest legacy commit status for a duplicated context", () => { - const result = evaluate({ - checkRuns: [], - statuses: [ - { - id: 20, - context: "verify", - state: "failure", - updated_at: "2026-10-02T01:00:00Z" - }, - { - id: 21, - context: "verify", - state: "success", - updated_at: "2026-10-02T01:05:00Z" - } - ] - }); - - assert.equal(result.status, "accepted"); -}); diff --git a/tests/invariants.test.js b/tests/invariants.test.js index f54f1b6..bf27e09 100644 --- a/tests/invariants.test.js +++ b/tests/invariants.test.js @@ -32,7 +32,7 @@ test("trigger matching uses terms rather than substrings", () => { test("prompt section declares Pi ownership and deterministic preflight", () => { const section = buildForgeFlowPromptSection("Migrate a legacy importer with retry semantics"); - assert.match(section, /Pi and pi-subagents own agent execution/); + assert.match(section, /Pi and installed plugins own execution/); assert.match(section, /Do not create a second agent runtime/); assert.match(section, /INV-CUTOVER-001/); assert.match(section, /INV-REPLAY-001/); diff --git a/tests/review-verdict.test.js b/tests/review-verdict.test.js deleted file mode 100644 index c16ee06..0000000 --- a/tests/review-verdict.test.js +++ /dev/null @@ -1,87 +0,0 @@ -import assert from "node:assert/strict"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; -import { resolve } from "node:path"; -import { spawnSync } from "node:child_process"; -import test from "node:test"; - -import { parseReviewVerdict } from "../extension/review-verdict.js"; - -for (const prefix of ["Merge verdict: ", "- Merge verdict: "]) { - for (const [verdict, status] of [ - ["BLOCK", "blocked"], - ["OK", "clean"], - ["OK with notes", "clean"] - ]) { - const line = `${prefix}${verdict}`; - test(`parses Pi canonical reviewer verdict: ${line}`, () => { - assert.deepEqual(parseReviewVerdict(`## Review\n- evidence\n\n${line}\n`), { - status, - verdict - }); - }); - } -} - -test("fails closed when the verdict is not the final non-empty line", () => { - const result = parseReviewVerdict("Merge verdict: OK\nextra prose"); - assert.equal(result.status, "invalid"); - assert.equal(result.reason, "REVIEW_VERDICT_MISSING"); -}); - -test("fails closed on decoration, whitespace, noncanonical shape, or unknown spelling", () => { - for (const report of [ - "**Merge verdict:** OK", - "-- Merge verdict: OK", - "* Merge verdict: OK", - "Merge verdict: CLEAN", - "- Merge verdict: ok", - " Merge verdict: OK", - "Merge verdict: OK ", - "- Merge verdict: OK " - ]) { - assert.equal(parseReviewVerdict(report).status, "invalid", JSON.stringify(report)); - } -}); - -test("ignores whitespace-only lines after an exact canonical verdict", () => { - assert.deepEqual(parseReviewVerdict("review body\nMerge verdict: OK\n \t\n"), { - status: "clean", - verdict: "OK" - }); -}); - -test("fails closed on empty or non-text reviewer output", () => { - assert.deepEqual(parseReviewVerdict(" \n "), { - status: "invalid", - reason: "REVIEW_REPORT_EMPTY" - }); - assert.deepEqual(parseReviewVerdict(null), { - status: "invalid", - reason: "REVIEW_REPORT_NOT_TEXT" - }); -}); - -test("review verdict helper reads the absolute reviewer artifact under .pi/subagents", () => { - const root = process.cwd(); - const reviewRoot = resolve(root, ".pi/subagents"); - mkdirSync(reviewRoot, { recursive: true }); - const dir = mkdtempSync(resolve(reviewRoot, "verdict-test-")); - const report = resolve(dir, "review.md"); - writeFileSync(report, "## Review\nNo issues found.\nMerge verdict: OK\n", "utf8"); - - try { - const helper = resolve(root, "scripts/review-verdict.mjs"); - const result = spawnSync(process.execPath, [helper, "--report", report], { - cwd: root, - encoding: "utf8" - }); - - assert.equal(result.status, 0, result.stderr); - assert.deepEqual(JSON.parse(result.stdout), { - status: "clean", - verdict: "OK" - }); - } finally { - rmSync(dir, { recursive: true, force: true }); - } -}); diff --git a/tests/review-workflow.test.js b/tests/review-workflow.test.js deleted file mode 100644 index 3125258..0000000 --- a/tests/review-workflow.test.js +++ /dev/null @@ -1,51 +0,0 @@ -import assert from "node:assert/strict"; -import test from "node:test"; - -import { createReviewWorkflowDefinition } from "../extension/review-workflow.js"; - -const ROOT = process.cwd(); - -test("review resource rejects unknown public arguments", () => { - const definition = createReviewWorkflowDefinition(ROOT); - const result = definition.resolve({ task: "Review this", command: "rm -rf /" }); - assert.deepEqual(result, { error: "forgeflow.review supports only the task field." }); -}); - -test("review resource persists reviewer prose and uses Pi typed gate for deterministic verdict", () => { - const definition = createReviewWorkflowDefinition(ROOT); - const result = definition.resolve({ task: "Review the retry implementation" }); - - assert.equal(definition.version, 4); - assert.equal("error" in result, false); - assert.equal(result.hostCommands, undefined); - assert.match(result.script, /agent: "reviewer"/); - assert.match(result.script, /model: "forgeflow\/reviewer"/); - assert.match(result.script, /context: "fresh"/); - assert.match(result.script, /agentContract: \{ version: 1 \}/); - assert.match(result.script, /outputMode: "file-only"/); - assert.match(result.script, /standard merge verdict contract/); - assert.match(result.script, /review-verdict\.mjs/); - assert.match(result.script, /"output":"json"/); - assert.match(result.script, /structuredOutput/); - assert.match(result.script, /reportPath: requestedReportPath/); - assert.doesNotMatch(result.script, /outputPathMapping/); - assert.doesNotMatch(result.script, /artifactPaths/); - assert.doesNotMatch(result.script, /outputReference/); - assert.match(result.script, /INV-REPLAY-001/); - assert.doesNotMatch(result.script, /outputSchema/); - assert.doesNotMatch(result.script, /runs\.host/); -}); - -test("review task cannot widen typed gate command authority", () => { - const definition = createReviewWorkflowDefinition(ROOT); - const task = "Review this; rm -rf /; $(touch /tmp/nope)"; - const result = definition.resolve({ task }); - - assert.equal("error" in result, false); - const gateIndex = result.script.indexOf("review-verdict.mjs"); - assert.notEqual(gateIndex, -1); - const gateWindow = result.script.slice(gateIndex, gateIndex + 1200); - assert.equal(gateWindow.includes("rm -rf"), false); - assert.equal(gateWindow.includes("touch /tmp/nope"), false); - assert.match(result.script, /rm -rf/); -});