diff --git a/AGENTS.md b/AGENTS.md index d8e5b0b..29a2f68 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,6 +25,11 @@ The owner approved the bounded verifier recovery repair on 2026-09-21. Its authority and acceptance are in `product/verifier-recovery.md`. Use the native maintainer route; preserve downstream task and candidate identities. +The owner approved the GitHub Codex review follow-through on 2026-09-21. Its +authority and acceptance are in `product/github-codex-review-follow-through.md`. +Use the native maintainer route and preserve the existing local and GitHub +approval policies. + ## Operating Mill Mill turns approved repository-native product intent into a bounded, tested, @@ -195,6 +200,12 @@ two-step plan/apply wrapper, never as implicit push authority. Preserve every finding; only P0/P1 require repair under that policy. Legacy reviews retain their original blocking rules. Required GitHub approval remains separate. See `docs/review-policy.md`. +- `propose.reviewPolicy.mode: github_codex_required` requires the named GitHub + Codex actor's completed summary on the exact PR head. If marking a draft ready + triggers review, execute and verify readiness only. Observe review completion, + then approve a new merge plan bound to the exact review and feedback snapshot. + Running, stale, malformed, unclassified, or P0/P1 feedback blocks. Never treat + readiness approval as merge approval. - Before any remote attempt, stale full-diff scope may use attended `review --refresh --base `. Preserve candidate, native validation, prior receipts, deadline and remaining review budget; do not move diff --git a/CHANGELOG.md b/CHANGELOG.md index 03c7256..4bdb582 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,24 @@ All notable changes follow Keep a Changelog and Semantic Versioning. ## [Unreleased] +## [0.9.0] - 2026-09-21 + +### Added + +- Add a GitHub Codex review mode that requires exact-head completion, retains + hosted feedback, and binds that evidence into attended merge approval. +- Split draft readiness from merge when readiness starts hosted review. + +### Fixed + +- Recheck required hosted review and feedback during post-merge closure. +- Reject hosted-review drift between merge planning and execution. + +### Changed + +- Shorten the installation command and rewrite the README around the first + reviewed pull request, trust boundary, supported scope, and recovery path. + ## [0.8.1] - 2026-09-21 ### Fixed diff --git a/README.md b/README.md index 7de6fd4..bc3cece 100644 --- a/README.md +++ b/README.md @@ -2,210 +2,47 @@ Mill is a local-first software factory for new and existing codebases. It turns approved product intent into bounded, tested, reviewed pull requests with -repository-native evidence and explicit human approval for delivery and merge. - -It is designed for a founder or small team that wants to use coding agents -without handing one agent an open-ended ticket, production credentials, and the -power to judge its own work. Product truth stays in the repository. Codex writes -inside a disposable worktree. Native tests and scenarios verify the committed -candidate. A separate read-only pass reviews that exact commit. Only the -attended shipper can use your GitHub identity. Draft-only is the default; -repositories may explicitly enable a separately approved, exact-plan merge. - -Release channels and exact artifact evidence are facts of a particular -publication. Read the -[latest GitHub release](https://github.com/davidahmann/mill/releases/latest), -its attached `release-evidence-final.json`, and -`npm view @davidahmann/mill dist-tags --json` before selecting a version. GitHub -Latest and npm `latest` are separate provider pointers. The `v0.1.5` genesis -release remains the historical trust root; failed release candidates remain -preserved evidence. +repository-owned evidence and explicit human approval for delivery and merge. + +Mill wraps the coding agent you already use. The builder works in a disposable +Git worktree. Declared checks run against the committed candidate in a +no-network OCI verifier. A separate read-only pass reviews the complete diff. +Only the attended shipper can use your GitHub identity. ## Why Mill -Coding is only one part of software delivery. The difficult failure modes live -between a PRD and a merged change: ambiguous intent, changing architecture, weak -acceptance tests, context drift, cumulative regressions, credential leakage, -interrupted side effects, noisy review, and artifacts that cannot be -reconstructed later. - -Mill makes those boundaries explicit: - -- approved outcomes, invariants, scenarios, impacts, and tasks are versioned in - Git; -- each run is bound to an exact base, authority closure, worker profile, budget, - candidate commit, validation result, and review result; -- product code remains independently buildable and testable without Mill; -- the builder cannot push, merge, deploy, or rewrite the oracle that certifies - its own candidate; -- GitHub mutations are separately planned, approved, journaled, and reconciled; -- one complete review is repaired systemically instead of creating micro-PR - churn; -- longitudinal qualification checks whether accepted behavior survives a - sequence of dependent changes; -- releases preserve and publish one independently reproduced tarball rather than - rebuilding at publication time. - -Mill is a small delivery control plane around the coding agent you already use. -It makes scope, evidence, and external actions inspectable. - -## Capability status - -| Capability | Status | Evidence and limit | -| -------------------------------------------- | ------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------- | -| Core Node/npm delivery path | Qualified for the release-specific support tuple | See the release evidence attached to the selected version. | -| Proposal assessment | Shipped and exercised | `init propose` is read-only; approval remains outside Mill. | -| Cancellation, diagnostics, and state upgrade | Shipped and tested | Diagnostics do not create or upgrade state. A mutating command takes the writer lease and preserves a pre-upgrade backup. | -| Retained verifier reports | Shipped and exercised | Paths and limits are task-controlled. Mill lists descriptors, not report bytes. | -| Shallow pnpm workspace | Exercised | One pinned [OCI fixture](docs/development.md#pnpm-workspace-oci-canary) only. It is not general pnpm or native-package support. | -| Synthetic provider migration replay | Exercised | [Private disposable fixtures](docs/canaries/mrev-maintenance-replays.md), not live provider behavior, customer acceptance, or demand evidence. | - -## What it can do - -For its one qualified shape, Mill can: - -1. inspect a PRD, source manifest, structured product proposal, and repository - without executing repository code; -2. freeze an approved product contract, stable invariants, scenarios, and - per-change impact; -3. create a repository from the bundled web recipe or plan a compatible adoption - without overwriting existing truth; -4. prepare exact npm dependencies as a separate attended network effect; -5. run one approved task through Codex build, OCI validation, exact-candidate - review, and one bounded repair generation; -6. plan and open one draft GitHub PR through the operator's own `gh` session; -7. observe exact-head CI and review, optionally execute a separately approved - attended merge, then verify resulting-main closure; -8. back up, restore, purge, reconcile, cancel, detach, and export a redacted - support bundle through explicit commands; -9. audit the exact repository candidate and validate a public-alpha - qualification record; -10. derive bounded, source-revision-bound TypeScript repository evidence from a - clean Git root without executing its code; -11. combine inspected planning drafts into a read-only proposal summary, then - compile an operator-supplied follow-up change request and approved impacts - into dependency-checked task packets and an outcome plan; -12. project a durable run into a versioned, read-only continuation packet that - names the next attended safe action without taking it; -13. report the built-in builder's trusted-host boundary and reject an - unqualified request for isolated execution rather than silently claiming - containment; and -14. report redacted aggregate lifecycle counts with `millctl stats`, plus a - repository-owned development-evidence ledger with `millctl report`. - -Mill does not autonomously research the web or turn prose into approved product -intent. The operator supplies the structured drafts that Mill assesses and -freezes. - -## Repository playbooks, run timelines and outcomes - -The source branch can index small repository-owned playbooks, search compact -metadata, and bind a selected set into a task's frozen context. A playbook can -carry reusable migration knowledge or a repository procedure, while the task's -approved acceptance criteria and native checks remain authoritative. The -selection is digest-pinned and recorded in the context manifest; stale or -mismatched index or playbook bytes block admission. See -[repository playbooks](docs/playbooks.md). - -`millctl --json timeline` projects the selected durable run and its append-only -events into a schema-checked timeline. It includes lifecycle facts only, never -event payloads, and marks malformed, discontinuous, forbidden, or stale state -evidence as inconsistent. See [run timelines](docs/run-timeline.md). - -`millctl --json outcome` projects the same durable run into a redacted outcome -record. It binds local lifecycle, validation, review, delivery and measured -usage facts, and summarizes adaptation provider/configuration matrix evidence -when present. It reports owner acceptance as `not_recorded`; checks, review and -delivery cannot certify that decision. Invalid, stale or cross-candidate records -block the projection. See [run outcomes](docs/run-outcome.md). - -These capabilities remain outside the qualified public-alpha support claim until -separately qualified. They do not grant a builder authority to change acceptance -criteria, deliver, merge, or release. - -The source also contains a constrained pnpm workspace path. A real local OCI -canary exercised Node 24, pnpm 10.23.0, a shallow workspace, a service, CLI, -SQLite scratch state, an offline verifier, a retained report, and cleanup under -failure, timeout, and cancellation. It rejects install hooks, native-build -allowlists, and arbitrary layouts. That evidence is limited to the pinned -fixture; the public alpha does not support general pnpm workspaces. - -## Supported shape - -The first recipe is intentionally exact: - -- Node.js 24.18.1 and npm 11.16.0 inside the verifier image; -- TypeScript 6.0.3; -- Next.js 16.3.4 and React 19.2.8; -- Playwright 1.62.1; -- GitHub as the only forge; -- Codex CLI with the operator's existing login; -- Docker-compatible OCI verification; -- macOS arm64 as the first candidate host tuple. - -Mill itself is developed with Node.js 24.20.0 and npm 11.19.0. Exact support is -published in each release qualification record, not inferred from nearby -versions. All other stacks, operating systems, architectures, forges, models, -and worker harnesses are experimental or unsupported until independently -qualified. Experimental Node package-manager adoption is separate from this -qualified web recipe. +A coding agent can produce a patch, but production delivery also depends on +clear scope, stable acceptance checks, complete review, and recoverable external +effects. Mill records those boundaries in the repository and binds each decision +to the exact candidate it governs. + +This keeps product approval, implementation, verification, review, and shipping +with separate principals while preserving the repository's native commands and +GitHub workflow. ## Install -Inspect the available channels, choose a released version, and pin that exact -version with lifecycle scripts disabled. Do not install a floating tag into an -existing repository. +Pin a released version in the repository that will use Mill: ```sh -npm view @davidahmann/mill dist-tags --json -npm install --save-dev --save-exact --ignore-scripts @davidahmann/mill@0.8.1 +npm i -D -E --ignore-scripts @davidahmann/mill@0.9.0 npx --no-install millctl --version ``` -The example selects this source candidate's version, `0.8.1`. Use it after the -linked release's final evidence confirms publication; an unpublished source -candidate is not an installable release. See the -[v0.8.1 scope](docs/releases/v0.8.1.md) and -[planning evidence rules](docs/planning.md#what-passing-evidence-means). +The exact pin prevents a later release from changing delivery behavior without +review. `--ignore-scripts` prevents package lifecycle code from running during +installation. Before adopting a release, inspect its +[GitHub evidence](https://github.com/davidahmann/mill/releases/latest) and npm +channel with `npm view @davidahmann/mill dist-tags --json`. -To develop Mill itself from a clean source checkout: +## Quick start -```sh -git clone https://github.com/davidahmann/mill.git -cd mill -asdf install -node_bin_dir=$(dirname "$(asdf which node)") -PATH="$node_bin_dir:$PATH" "$node_bin_dir/npm" ci --ignore-scripts -PATH="$node_bin_dir:$PATH" "$node_bin_dir/npm" run check -PATH="$node_bin_dir:$PATH" "$node_bin_dir/npm" run build -node dist/cli.js --version -``` - -Downstream repositories should pin the exact selected version so a later release -cannot alter their delivery machinery implicitly. Updating an existing Mill -installation is deliberate: preserve state and repository snapshots, inspect the -release evidence, and requalify the changed toolchain. There is no automatic -upgrade or proven general downgrade path for operational state. - -### Policy starters for downstream repositories - -The packed source includes reviewed starters for a Node/npm repository under -`policy-starters/node-npm/`. They provide a documentation check, an agent-policy -addendum, Dependabot grouping, and a DCO workflow that verifies an eligible -Dependabot commit without running pull-request code under `pull_request_target`. -Copy them only through a repository review. Add the `dco` check to the target -branch-protection settings through that repository's own approved process. +Mill's shortest useful path is: inspect the repository, approve the proposed +authority files, run one bounded task, then approve draft delivery. -## One successful path +### 1. Inspect -The first useful outcome is a reviewed draft PR. Follow this route: inspect the -repository, approve the proposal and task materials, then run one attended -delivery through review and draft delivery. The detailed commands below expose -the same boundaries when you need to inspect them separately. - -### 1. Inspect the repository - -These commands are read-only and do not execute repository code: +These commands read files without executing repository code: ```sh npx --no-install millctl doctor --mode inspect @@ -213,17 +50,11 @@ npx --no-install millctl inspect --prd product/PRD.md npx --no-install millctl adopt --scan-only ``` -### 2. Approve the proposal - -Use `init propose` to read the PRD and planning drafts together. It returns a -reviewable summary and compiled task drafts without writing authority files. -Review its contradictions, assumptions, questions, and digests. Commit and -approve the product contract, scenarios, impact, and task packet through the -repository's normal review process. Mill does not turn prose into approval or -acceptance criteria on its own. +Use `init propose` when the repository already contains a PRD, source manifest, +product proposal, contract, scenarios, impact, and change request: ```sh -millctl --json init propose \ +npx --no-install millctl --json init propose \ --prd product/PRD.md \ --sources product/sources.yaml \ --proposal product/proposal.yaml \ @@ -233,416 +64,202 @@ millctl --json init propose \ --request product/change-request.yaml ``` -### 3. Get a reviewed draft PR +The command reports contradictions, assumptions, questions, and digests. It does +not write authority files or approve product intent. -Once one outcome is approved and ready, start its attended lifecycle. It selects -that outcome or resumes its existing run, verifies the committed candidate, and -keeps all delivery effects separate: +### 2. Approve one bounded task -```sh -millctl --json start --prd product/PRD.md --attended -millctl --json status -millctl --json stats -millctl --json report -``` - -`start` checks authority before dependency or model spend. `status` explains the -selected run; `stats` gives lifecycle counters; `report` adds redacted outcome -and measured-usage aggregates. When `start` reports a reviewed candidate, plan -and make the separately proposed draft delivery effect: - -```sh -millctl --json pr plan --task product/tasks/TASK.yaml --run -millctl --json pr open --task product/tasks/TASK.yaml --run \ - --approve sha256: --attended -``` +Review and commit the product contract, scenarios, impact manifest, and task +packet through the repository's normal process. The repository owns these files. +Mill checks their exact digests before spending dependency or model budget. -Mill never treats these commands as approval to mark ready, merge, deploy, or -release. See the [glossary](docs/glossary.md) before creating or reviewing -authority files. +The [planning guide](docs/planning.md) defines the contracts. The +[glossary](docs/glossary.md) explains Mill's terms. -Repositories may approve `review: { blocking: p0_p1 }` in `mill.yaml` before -admitting a run. Mill retains every finding; P0/P1 block and standalone P2/P3 -remain advisory. Existing receiptless reviews keep their previous rules. See -[review policy and preparation](docs/review-policy.md). Changes outside admitted -runs use the [maintainer review route](docs/maintainer-review.md). - -## Expert reference - -### Discover a TypeScript repository - -The remaining examples use `millctl` as shorthand for the installed executable; -use `npx --no-install millctl` when it is installed locally. Supply your actual -PRD and approved authority paths; installing the CLI does not create them. - -Static discovery is a separate read-only operation. It requires a clean Git -repository root and rejects sensitive paths, symbolic links, unsafe Git -configuration, and incomplete source traversal. It parses local TypeScript and -JavaScript imports only after each regular file's physical bytes match its blob -in the captured `HEAD` tree. The same check protects parsed package metadata. It -reports source locations, inventories test files, and gives conservative -importer leads for named paths. Nonliteral module loads and option-bearing test -commands remain explicit unknowns. It never installs dependencies, runs the -target's code, or claims executed coverage or delivery authority. +### 3. Build, verify, and review ```sh -millctl --json discover /absolute/path/to/repository --changed src/service.ts +npx --no-install millctl --json start --prd product/PRD.md --attended +npx --no-install millctl --json status ``` -The first increment is a deterministic TypeScript extractor, not arbitrary-stack -support, a graph database, a watcher, or an AI-generated knowledge graph. Read -[`product/brownfield-discovery.md`](product/brownfield-discovery.md) and -[`docs/canaries/brownfield-discovery.md`](docs/canaries/brownfield-discovery.md) -for the exact boundary and real-fixture evidence. - -For a source-backed specification, supply the PRD, source manifest, and -structured proposal: +`millctl start` selects one approved ready outcome or resumes its sole active +run. The long form exposes each boundary separately: ```sh -millctl --json plan specification \ - --prd product/PRD.md \ - --sources product/sources.yaml \ - --proposal product/proposal.yaml +npx --no-install millctl --json qualify --baseline \ + --task product/tasks/TASK.yaml +npx --no-install millctl --json run \ + --task product/tasks/TASK.yaml \ + --approve sha256: --attended +npx --no-install millctl --json verify \ + --task product/tasks/TASK.yaml --run +npx --no-install millctl --json review \ + --task product/tasks/TASK.yaml --run ``` -Review the returned contradictions, assumptions, questions, and exact proposal -digest. Approval freezes that proposal; it does not grant repository writes. - -### Create a repository +The builder cannot edit task authority or the declared controls that certify its +change. Verification and review bind to the same candidate commit. -Preview the exact file plan first, then apply the same plan attended: +### 4. Approve a draft pull request ```sh -millctl --json new my-product --dry-run \ - --prd product/PRD.md --sources product/sources.yaml \ - --proposal product/proposal.yaml --approve-product sha256: \ - --repository-id --approved-by \ - --approved-at --author-name --author-email - -millctl --json new my-product --apply --attended \ - --prd product/PRD.md --sources product/sources.yaml \ - --proposal product/proposal.yaml --approve-product sha256: \ - --approve-plan sha256: --repository-id \ - --approved-by --approved-at \ - --author-name --author-email +npx --no-install millctl --json pr plan \ + --task product/tasks/TASK.yaml --run +npx --no-install millctl --json pr open \ + --task product/tasks/TASK.yaml --run \ + --approve sha256: --attended ``` -Greenfield apply stages and runs the complete native recipe gate before the -target becomes a Git repository. It never replaces an existing path. - -### Adopt a compatible repository +Draft delivery is a separate external effect. It does not authorize readiness, +merge, deployment, or release. -Use the same two-step boundary with `adopt --plan` and then -`adopt --apply --attended`. Adoption supports only the exact recipe-compatible -Node/Next.js shape. It keeps the operator checkout unchanged, writes an isolated -branch, and blocks on conflicting product truth, drifted native oracle files, -unsafe Git state, symlinks, credential-like files, or incompatible versions. +## Review and merge policy -Prepare dependencies separately because it is the only recipe step that needs -registry network access: +Repositories choose one GitHub review mode in `mill.yaml`: -```sh -millctl --json dependencies prepare --attended -``` +- `local_only` requires Mill's exact-candidate local review. +- `github_required` also requires an `APPROVED` GitHub review from every named + reviewer on the exact PR head. +- `github_codex_required` also requires the GitHub Codex review summary to be + complete for the exact PR head. When marking a draft ready starts that review, + Mill stops after readiness. The operator observes completion, approves a new + merge plan bound to that review evidence, then merges. -Later verification has no network and receives read-only source. +An approved top-level `review: { blocking: p0_p1 }` keeps standalone P2/P3 +findings advisory while retaining them in evidence. P0, P1, unclassified +feedback, missing completion, stale feedback, failed checks, and review drift +block the applicable phase. Read [review policy](docs/review-policy.md) before +changing this boundary. -### Deliver one approved task - -The downstream repository owns `mill.yaml`, `product/contract.yaml`, -`quality/scenarios.yaml`, an approved impact manifest, and a version 2 task. -Qualify the unchanged base, then use the returned digest once: +Attended merge is opt-in. It requires producer-bound checks, strict up-to-date +branch protection, allowed operator and merger identities, and an exact approval +plan: ```sh -millctl --json qualify --baseline --task product/tasks/TASK.yaml -millctl --json run --task product/tasks/TASK.yaml \ - --approve sha256: --attended -millctl --json verify --task product/tasks/TASK.yaml --run -millctl --json review --task product/tasks/TASK.yaml --run +npx --no-install millctl --json pr observe \ + --task product/tasks/TASK.yaml --run +npx --no-install millctl --json pr merge-plan \ + --task product/tasks/TASK.yaml --run --method squash +npx --no-install millctl --json pr merge \ + --task product/tasks/TASK.yaml --run \ + --approve sha256: --attended +npx --no-install millctl --json pr finalize \ + --task product/tasks/TASK.yaml --run ``` -The shorter resumable path is: +Mill rereads checks, review, feedback, identities, head, base, and candidate +tree before remote effects. Finalization separately verifies the merge and +required checks on resulting `main`. See [attended approvals](docs/approvals.md) +and [repository settings](docs/repository-settings.md). + +## What Mill enforces + +- Product authority, stable invariants, scenarios, impacts, and tasks are + versioned in Git. +- Each run binds the base, configuration, context, budget, candidate, + validation, review, and delivery evidence by digest. +- The builder cannot push, merge, deploy, or rewrite its acceptance oracle. +- GitHub effects are planned, approved, journaled, read back, and reconciled. +- Native checks remain the repository's source of behavioral evidence. +- Releases publish one independently reproduced tarball with provenance and + registry readback. + +The supported public interface is `millctl --json` plus the published JSON +schemas. Mill does not expose a supported package-root JavaScript API. + +## Trust boundary + +| Principal | May do | Cannot do | +| ---------------- | ------------------------------------------------------------------------ | ---------------------------------------------------------------- | +| Builder | Edit approved paths in a disposable worktree | Push, merge, deploy, or change authority and oracles | +| Verifier | Run declared commands in bounded no-network OCI | Write candidate source or use forge credentials | +| Reviewer | Read the exact committed candidate | Execute or edit candidate code | +| Attended shipper | Push/open a draft; execute a separately approved readiness or merge plan | Change the candidate, self-approve, bypass protection, or deploy | + +The Codex builder runs on the trusted host with a workspace-write sandbox. This +is not containment against hostile source, host files, keychains, processes, or +network access. Use Mill only with repositories you trust. Candidate +verification uses a pre-pulled digest-pinned OCI image with no network, +read-only source, bounded resources, deadlines, and explicit cleanup. Read the +[architecture](architecture/ARCHITECTURE.md) for the full boundary. + +GitHub operations use the operator's `gh` session by default. A repository can +instead name the fixed `MILL_GITHUB_TOKEN` environment variable for a reviewed, +repository-scoped token. Mill does not store credentials or attest their +permissions. See [delivery access](docs/delivery-access.md). + +## Supported scope + +The qualified support tuple belongs to each release. The current core path is a +single local Node/npm repository using GitHub, Codex CLI, and a +Docker-compatible OCI runtime on macOS arm64. The repository also contains +narrower exercised paths for shallow pnpm workspaces and synthetic +integration-adaptation replays; those do not establish general pnpm, +native-package, live-provider, customer, or market compatibility. + +Mill is local and attended. It supports one repository, outcome, and writer at a +time. It has no hosted control plane, daemon, scheduler, fleet, parallel agent +runtime, deployment authority, autonomous research, or model-authored acceptance +authority. + +## Recovery and evidence + +Every run has durable state and append-only lifecycle events. Inspect before +acting after interruption: ```sh -millctl --json start --prd product/PRD.md --attended +npx --no-install millctl --json status --run +npx --no-install millctl --json continuation --run +npx --no-install millctl --json timeline --run +npx --no-install millctl --json pr reconcile \ + --task product/tasks/TASK.yaml --run +npx --no-install millctl --json pr merge-reconcile \ + --task product/tasks/TASK.yaml --run ``` -`millctl start` selects exactly one approved ready outcome or resumes its sole -existing lifecycle. It checks authority before dependency or model spend. - -### Open a reviewed draft PR +Mill does not retry an uncertain external effect without authoritative readback. +Cancellation preserves unresolved receipts. The +[approval guide](docs/approvals.md#interruptions), +[OCI recovery guide](docs/oci-recovery.md), and +[verifier recovery guide](docs/verifier-recovery.md) describe the supported +routes. -Raise `trustCeiling` to `propose` only after configuring the exact GitHub -repository node ID, branch, allowed operator and merger, checks, review policy, -and approval TTL in `mill.yaml`. +Use these read-only views for local operating evidence: ```sh -millctl --json pr plan --task product/tasks/TASK.yaml --run -millctl --json pr open --task product/tasks/TASK.yaml --run \ - --approve sha256: --attended -millctl --json pr observe --task product/tasks/TASK.yaml --run -# By default, a human marks ready and merges in GitHub. -millctl --json pr finalize --task product/tasks/TASK.yaml --run +npx --no-install millctl --json stats +npx --no-install millctl --json report +npx --no-install millctl --json outcome --run ``` -Or use `millctl ship --draft` twice: first to return the proposal, then with its -exact digest and `--attended` to perform it. Draft delivery never implicitly -authorizes readiness or merge. - -### Approve a merge from your work surface +They report lifecycle and measured usage without inferring owner acceptance or +commercial value. -With `propose.attendedMerge: true`, producer-bound required checks, strict -up-to-date protection enforced for administrators, and no bypass-role grants, -the attending operator can inspect and approve: +## Reference -```sh -millctl --json pr merge-plan --task product/tasks/TASK.yaml --run --method squash -millctl --json pr merge --task product/tasks/TASK.yaml --run \ - --approve sha256: --attended -millctl --json pr finalize --task product/tasks/TASK.yaml --run -``` +- [Planning and authority](docs/planning.md) +- [Brownfield discovery and adoption](docs/brownfield.md) +- [Repository playbooks](docs/playbooks.md) +- [Development evidence](docs/report.md) +- [Run timelines](docs/run-timeline.md) and [outcomes](docs/run-outcome.md) +- [Development guide](docs/development.md) +- [Release runbook](docs/release.md) +- [Product requirements](product/PRD.md) +- [Agent operating contract](AGENTS.md) -The plan binds PR/head/base, exact tree, actor, method, policy and expiry. A -chat host can submit the operator's approval through this CLI; Mill does not -authenticate arbitrary chat messages or let the builder approve its own work. -See [attended approvals and recovery](docs/approvals.md). - -Mill checks classic branch-protection enforcement and exact check-producer -bindings. The operator must separately inspect bypass-role/ruleset grants; do -not infer a complete permissions audit from a successful merge preflight. - -### Compile follow-up work and adopt native brownfield commands - -`plan tasks --request product/change.yaml` deterministically compiles an -operator-supplied PRD/plan/bug/review change request and approved impact -manifests into version-2 tasks and a dependency-checked outcome plan. Its -separate `--apply --approve sha256: --attended` step writes an isolated -worktree. It does not infer approval or acceptance tests from prose. See -[planning](docs/planning.md). - -`adopt-native --config adoption.yaml` offers experimental Node ESM/npm adoption -that adds only `mill.yaml` and `mill.lock`, preserving existing code and native -scripts. Apply requires the exact digest and attendance; dependency preparation, -baseline qualification and task execution remain separate. See -[brownfield scope and qualification](docs/brownfield.md). - -### Pull-request and resulting-main checks - -The source implementation keeps `propose.requiredChecks` as the complete gate -for the exact pull-request head. Optional `propose.postMergeRequiredChecks` -selects a nonempty subset of those names for resulting-main readback only; -omitting it retains the full `requiredChecks` list for both phases. New delivery -plans approval-bind and persist both effective lists. Missing or pending -required checks prevent completion; skipped or failed required checks fail the -phase that requires them. - -In this repository, `dependency-review` runs only for pull requests, while -`validate` and `codeql` also run on pushes to `main`. The maintainer-prepared -`mill.yaml` requires all three at the exact PR head and explicitly selects -`[validate, codeql]` for resulting-main readback. - -New delivery records identify the policy source as `configured` or -`implicit_default`. A historical record with a full defaulted list and no -provenance can bind the configured subset once only if its exact reviewed -candidate proves omission of the optional policy and authoritative merge -readback establishes every other delivery identity. That compatibility path -cannot relax a new delivery. See -[repository settings](docs/repository-settings.md) and the -[migration record](docs/canaries/post-merge-default-policy-migration.md) for the -conditions and outstanding live evidence for delivery -`01801a1b-58f9-480f-8cee-54ea2bbeabb2`. Human readiness, merge authority, and -exact candidate/tree checks remain required. - -## Public interface - -The supported programmatic surface is the `millctl` CLI, its versioned JSON -result envelope, and the published JSON schemas. Shell automation should invoke -the CLI with `--json` and check `ok`, `status`, and `reasons`; it should not -parse the human-readable formatter. - -Mill does not expose a direct JavaScript library API. Build integrations on the -CLI and schemas until Mill publishes a separately supported library contract. - -## Trust model - -Mill separates four principals: - -| Principal | May do | Cannot do | -| ---------------- | ------------------------------------------------------------- | ---------------------------------------------------------------- | -| Builder | Edit approved paths in a disposable worktree | Push, merge, deploy, change authority or oracles | -| Verifier | Run declared commands in bounded no-network OCI | Write candidate source or use forge credentials | -| Reviewer | Read the exact committed candidate | Execute or edit code | -| Attended shipper | Push/open a draft; separately approved opt-in readiness/merge | Change the candidate, self-approve, bypass protection, or deploy | - -Codex uses your existing Codex CLI session and therefore your own provider -billing. GitHub operations use your existing `gh` session by default. A -repository can instead declare the fixed `MILL_GITHUB_TOKEN` environment name -for a reviewed fine-grained token. Another maintainer can clone Mill and use -their own Codex and GitHub accounts after the downstream repo explicitly allows -their identity. Mill stores neither credential. - -Use a separate, repository-scoped GitHub identity where the risk warrants it. -The [delivery credential guide](docs/delivery-access.md) describes the required -review and current limits. Mill does not inspect or attest the permissions of -the `gh` session it invokes. - -The Codex worker runs on the trusted host with a workspace-write sandbox. This -is not containment against hostile code, host files, keychains, processes, or -network access. Native candidate verification is the stronger boundary: a -pre-pulled digest-pinned OCI image, no network, read-only source/root, dropped -capabilities, deadlines, bounded output and resources, and explicit cleanup. -Deadlines require a live controller. After abrupt controller death, daemon-owned -containers can remain until [OCI recovery](docs/oci-recovery.md) removes them; -unresolved ownership blocks retry and state cleanup. Do not use this alpha with -hostile repositories or sensitive source. - -`millctl isolation --request trusted-host` reports the exact built-in boundary. -`millctl isolation --request isolated` fails closed because no isolated builder -adapter has been qualified; `run` and `resume` accept the same `--isolation` -option and will not silently fall back. See the -[reliability boundary](architecture/reliability-brownfield-foundation.md). - -## Recovery - -Every run has durable state and an append-only event history. If a controller is -interrupted, inspect before acting: +## Develop Mill ```sh -millctl --json status --run -millctl --json continuation --run -millctl --json timeline --run -millctl --json stats -millctl --json report -millctl --json resume --task product/tasks/TASK.yaml --run -millctl --json cancel --run -millctl --json pr reconcile --task product/tasks/TASK.yaml --run -``` - -Mill never signals a process solely from a stored PID and never retries an -uncertain external effect without authoritative readback. Use explicit local -recovery for state and diagnostics: - -```sh -millctl --json state backup -millctl --json state restore --from /absolute/path/returned-backup.sqlite3 -millctl --json state purge --confirm -millctl --json support-bundle --run -millctl --json detach plan -``` - -`status` remains compact: it does not reveal the worktree, raw worker context, -validation/review data, logs, private commit trailers, or credentials. Its -`continuation` projection binds task/base/candidate/configuration identities, -observed interruption/effect uncertainty, measured resource fields, and the next -attended action without performing it. It reports provider-measured input, -output, and cache-input tokens when present, partial fields when a completed -call omitted them, and `unavailable` currency cost rather than an estimate. - -`timeline` is a separate read-only diagnostic view. It shows event sequence, -timestamp, event type and state transitions, never event payloads. It blocks if -the recorded sequence, lifecycle contract or durable run status disagree; it -cannot repair state or authorize the next action. - -An unresolved push, PR, readiness or merge blocks repair, new delivery and state -purge/restore even if the enclosing run says `blocked` or `cancelled`. -Cancellation records intent without discarding the receipt. Use -`pr merge-reconcile` for readiness/merge; a confirmed merge then requires -`pr finalize` and green main checks before cleanup. See -[approval recovery](docs/approvals.md#interruptions). - -For an eligible pre-command verifier infrastructure failure, inspect the -[candidate recovery procedure](docs/verifier-recovery.md). It preserves the -original run and can grant one separately approved verification/review window -after its original deadline expires. The unchanged candidate must have a -committed version pin. Tests and independent review remain required. - -Before any remote attempt, stale review scope can use -`review --task product/tasks/TASK.yaml --run --refresh --base --attended`. -This preserves the candidate and remaining review budget, invalidates the -unexecuted delivery plan and does not move frozen Git refs. Plan delivery again -after the fresh review passes. - -For generated authority, `state reconcile-plans` verifies the exact committed -files. A failed plan can instead be explicitly discontinued with -`state abandon-plan --approve --attended`, after -preserving partial output in a clean commit on its recorded branch. Abandonment -retains evidence and does not certify successful apply. See -[plan recovery](docs/planning.md#recovery). - -Restore validates the database before atomic replacement and quarantines newer -unreferenced worktrees. Detach is plan-only; the operator performs the reviewed -removal. A generated/adopted repo must continue to build and test natively after -Mill is removed. - -Run purge from a surviving original checkout, never a worktree scheduled for -deletion. Preserve an external state backup and committed candidate branches -first. The known purge-from-a-deletable-worktree P2 can leave cleanup -incomplete; it does not grant permission to discard unresolved effects or -foreign files. - -## Audit and qualification - -`millctl audit` is a bounded, read-only milestone check for Mill's selected -recipe and release path. It requires a clean exact Git candidate and reports -product, code, UX, accessibility, security, dependency, architecture, -operations, and release checks in a schema-valid JSON envelope. These checks are -labelled `assurance: structural`: they inspect contracts and hooks, not executed -security, accessibility or business behavior. Native command results, realistic -scenarios and release canaries are separate evidence. - -```sh -millctl --json --cwd . audit -millctl --json --cwd . qualify public-alpha \ - --file /absolute/path/qualification.json +git clone https://github.com/davidahmann/mill.git +cd mill +asdf install +node_bin_dir=$(dirname "$(asdf which node)") +PATH="$node_bin_dir:$PATH" "$node_bin_dir/npm" ci --ignore-scripts +PATH="$node_bin_dir:$PATH" MILL_GIT_PATH=/opt/homebrew/bin/git \ + "$node_bin_dir/npm" run check ``` -Public-alpha qualification requires at least five dependent accepted changes, -item-level new-behavior and preservation evidence, a rejected and recovered -seeded-fault branch, a current exact support tuple, every required packed and -integration canary, and all nine audits. A later success cannot conceal an -earlier unresolved preservation failure. - -Every qualified release also requires two independent clean builds from the -exact annotated tag, canonical content equality, a preserved tarball, SBOM, -trusted npm OIDC publication, provenance, registry reinstallation, and GitHub -Release readback. See [the release runbook](docs/release.md). - -## Troubleshooting - -- `WRONG_MILL_VERSION`: run the exact package version in `mill.lock`; Mill does - not silently delegate to another version. -- `BASE_REF_DRIFT` or context drift: stop, review the new repository state, and - requalify. Never reuse the old approval digest. -- missing OCI image: pull the exact digest explicitly outside Mill, then rerun - readiness. Mill never pulls implicitly. -- provider login failure: run `codex login` as the operator; do not pass a token - through the task or repository. -- GitHub identity or destination mismatch: correct `mill.yaml` or log in with - the explicitly allowed `gh` identity. Do not weaken the binding. -- `effect_unknown`: run read-only reconciliation. Do not retry push or PR - creation until absence or success is authoritative. -- active-run conflict: resume or safely terminalize the existing run; do not - start a second writer. - Use [GitHub Issues](https://github.com/davidahmann/mill/issues) for reproducible -defects and private vulnerability reporting for security issues. Support is -best-effort with no SLA. - -## Limitations - -- local and attended only; -- one repository, outcome, and writer at a time; -- one exact web recipe and compatible adoption shape; -- operator-supplied structured proposal, not autonomous planning research; -- Codex and GitHub through the operator's existing sessions; -- no hostile-host containment for the coding agent; -- no daemon, hosted control plane, scheduler, fleet, or parallel agents; -- no automatic readiness, merge, deployment, repository provisioning, or issue - synchronization; -- no general migration engine, automatic upgrade/rollback, or automatic detach; -- no self-improvement loop or model-authored acceptance authority. - -For system detail, read the [product requirements](product/PRD.md), -[architecture](architecture/ARCHITECTURE.md), -[development guide](docs/development.md), [workflow](WORKFLOW.md), and -[agent operating contract](AGENTS.md). +defects and private vulnerability reporting for security issues. Support is best +effort with no SLA. diff --git a/architecture/ARCHITECTURE.md b/architecture/ARCHITECTURE.md index 667ef57..d86cf77 100644 --- a/architecture/ARCHITECTURE.md +++ b/architecture/ARCHITECTURE.md @@ -48,8 +48,9 @@ exact human-authored task + product/scenario/policy digests -> reviewed local candidate or one repair-and-revalidate cycle -> exact actor/repository/remote proposal digest -> expected-head push + immutable-marker draft PR - -> exact-head CI and optional GitHub-review observation - -> human readiness and merge + -> exact-head CI and configured GitHub-review observation + -> separately approved readiness when hosted review starts there + -> completed exact-head hosted review and separately approved merge -> exact merge/tree/default-branch check readback and closure ``` diff --git a/docs/approvals.md b/docs/approvals.md index 0cbd08d..8ef9b1b 100644 --- a/docs/approvals.md +++ b/docs/approvals.md @@ -46,6 +46,18 @@ method and readiness action. `merge` and `squash` are the available request methods; squash requires the existing `linear_tree_preserving` policy. The operator is reauthenticated through the configured local `gh` session. +With `github_codex_required`, a draft can still be in `awaiting_ci` after its +checks pass because hosted review has not started. `pr merge-plan` may then +produce a readiness-only plan. Applying it marks the exact PR ready, verifies +that effect, and stops. Wait for GitHub Codex, run `pr observe`, address any +P0/P1 or unclassified feedback, and request a new merge plan. That second plan +includes the digest of the completed exact-head review and feedback snapshot. +Mill rereads the snapshot immediately before merge and rejects drift. + +Do not reuse the readiness approval for merge. A `ready_verified` receipt is +evidence that the PR left draft state; it is not hosted-review completion or +merge authority. + A trusted chat integration must authenticate its operator and obtain their approval of this exact displayed plan before invoking the CLI. The local `attended_operator` receipt is not a signed chat event, an identity federation @@ -53,10 +65,11 @@ service, or proof that an arbitrary message was human-authored. Keep this capability outside all model-controlled builder/reviewer tool bundles. Mill rechecks policy, native validation, full-diff review, current GitHub -review, feedback and CI before effects. GitHub's merge API compares the exact PR -head; it does not offer an atomic base-SHA comparison. Strict branch protection -and fresh base checks constrain that race, and exact merged-tree readback is -still required. Do not interpret an API success as verified lifecycle closure. +review, feedback and CI before effects and during post-merge closure. GitHub's +merge API compares the exact PR head; it does not offer an atomic base-SHA +comparison. Strict branch protection and fresh base checks constrain that race, +and exact merged-tree readback is still required. Do not interpret an API +success as verified lifecycle closure. Draft planning, push and PR creation compare the locally reviewed merge-base diff with GitHub's authoritative base SHA. An unpushed preparation commit on diff --git a/docs/releases/v0.9.0.md b/docs/releases/v0.9.0.md new file mode 100644 index 0000000..bcada9d --- /dev/null +++ b/docs/releases/v0.9.0.md @@ -0,0 +1,22 @@ +# Mill 0.9.0 + +This source candidate adds an explicit GitHub Codex review policy. It recognizes +the hosted review summary only when it reports completion for the exact pull +request head, evaluates the named actor's current-head feedback, and binds that +review snapshot into merge approval. + +Because GitHub Codex starts after a draft becomes ready, Mill now supports a +readiness-only attended plan for that policy. It verifies readiness and stops. +The operator must observe completed hosted review and approve a second exact +plan before merge. P0/P1 and unclassified feedback block under the approved +P0/P1 policy; standalone P2/P3 remains recorded and advisory. Post-merge +readback checks the same policy before closure. + +The README now leads with a short exact installation command and one successful +path from repository inspection to a reviewed draft pull request. Detailed +policy, recovery, and release procedures remain in focused documents. + +Publication and npm/GitHub channel pointers are established by the protected +workflow's final release evidence. This source record grants no new stack, +customer-compatibility, provider-behavior, deployment, or unattended-operation +claim. diff --git a/docs/repository-settings.md b/docs/repository-settings.md index dba5c99..a1cee7f 100644 --- a/docs/repository-settings.md +++ b/docs/repository-settings.md @@ -18,9 +18,12 @@ After the Wave 1 checks have run at least once, configure: - maintainer bypass allowed only for emergencies and recorded as repair/audit intake. -`CODEOWNERS` routes ownership but must not deadlock the sole maintainer. GitHub -Codex review is optional repository policy in v1; frozen local review and -required machine checks are portable and mandatory. +`CODEOWNERS` routes ownership but must not deadlock the sole maintainer. Frozen +local review and required machine checks remain mandatory. A downstream +repository can additionally select `github_required` for ordinary GitHub +approvals or `github_codex_required` for GitHub Codex completion and feedback. +The latter uses a separate readiness approval because the hosted review begins +after draft state ends. See [review policy](review-policy.md). ## Check names and workflow events diff --git a/docs/review-policy.md b/docs/review-policy.md index e82ac6f..efe0fa4 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -22,6 +22,42 @@ policy was frozen for that delivery. Unclassified feedback blocks. An explicit required GitHub approval remains required; advisory findings do not turn a rejection or missing approval into approval. +## GitHub review modes + +`propose.reviewPolicy` has three modes: + +- `local_only` uses Mill's required exact-candidate local review and does not + wait for a GitHub reviewer. +- `github_required` additionally requires an `APPROVED` review from every named + login on the exact PR head. +- `github_codex_required` additionally requires the GitHub Codex summary comment + from every named login to report `Completed` for the exact PR head. GitHub + Codex posts findings as review comments rather than an approving review, so + this mode treats its exact-head summary as completion evidence and evaluates + its current-head feedback separately. + +GitHub reports the Codex App login with its bot suffix: + +```yaml +reviewPolicy: + mode: github_codex_required + requiredReviewerLogins: ["chatgpt-codex-connector[bot]"] +``` + +Confirm the login from the target repository's API before freezing policy. Mill +compares it exactly. + +For `github_codex_required`, a running, missing, malformed, or stale summary is +not completion. P0/P1 and unclassified feedback block when the frozen review +policy is `p0_p1`; standalone P2/P3 remains recorded and advisory. Mill binds +the complete required-actor review and feedback snapshot into the merge plan. +Any change before merge invalidates that approval. + +GitHub Codex begins its hosted review after a draft is marked ready. With +attended merge enabled, Mill therefore uses two approvals: the first may only +mark an exact, green draft ready; after `pr observe` records completed review, +the second may authorize merge. The first approval cannot merge the PR. + Changing policy does not repair an existing blocked run. Preserve its evidence and use a separately approved admission when its configuration, deadline or budget no longer permits continuation. Do not edit a stored receipt, reset a diff --git a/mill.yaml b/mill.yaml index 9241fc1..8dc71e7 100644 --- a/mill.yaml +++ b/mill.yaml @@ -37,8 +37,8 @@ propose: pullRequestEvent: pull_request postMergeEvent: push reviewPolicy: - mode: local_only - requiredReviewerLogins: [] + mode: github_codex_required + requiredReviewerLogins: ["chatgpt-codex-connector[bot]"] allowedMergeMethods: [linear_tree_preserving] approvalTtlSeconds: 900 pollTimeoutSeconds: 600 diff --git a/package-lock.json b/package-lock.json index 62a922c..d6a5cb4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@davidahmann/mill", - "version": "0.8.1", + "version": "0.9.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@davidahmann/mill", - "version": "0.8.1", + "version": "0.9.0", "bundleDependencies": [ "typescript" ], diff --git a/package.json b/package.json index 21bc35a..a54a926 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@davidahmann/mill", - "version": "0.8.1", + "version": "0.9.0", "description": "Local-first software factory for new and existing codebases. Turns approved product intent into tested, reviewed PRs with repo-native evidence and explicit human approval for delivery and merge.", "license": "Apache-2.0", "author": "David Ahmann", diff --git a/product/github-codex-review-follow-through.md b/product/github-codex-review-follow-through.md new file mode 100644 index 0000000..9472312 --- /dev/null +++ b/product/github-codex-review-follow-through.md @@ -0,0 +1,39 @@ +# GitHub Codex review follow-through + +Status: approved by David Ahmann on 2026-09-21 in the maintainer work session. +Route: native maintainer implementation, exact-candidate review, qualified +release, and downstream correction through the released version. + +The T-021 preparation PR in Telryn received two P1 comments from GitHub Codex +after it was merged. Mill's configured `local_only` policy behaved as declared, +but the operator did not wait for or reconcile the hosted review. The local +review also missed those oracle and cleanup gaps. + +Add a distinct GitHub Codex review policy without changing `local_only` or +`github_required` semantics. It must: + +- require the Codex review summary to report completion for the exact current PR + head before merge; +- retain current-head GitHub Codex feedback and block P0/P1 while leaving + standalone P2/P3 advisory under an approved P0/P1 policy; +- keep a draft PR's readiness effect separate when marking it ready triggers a + new hosted review; +- bind the completed hosted-review observation into the merge plan and recheck + it immediately before the merge effect; +- reject running, stale, malformed, unclassified, or late blocking feedback; +- surface post-merge review drift rather than closing over it; and +- preserve existing review policies and historical delivery records. + +Tests must cover completed, running and stale Codex summaries, clean and +blocking feedback, advisory feedback, draft readiness, review drift between plan +and merge, and post-merge blocking feedback. + +Ship this as the next qualified Mill release, repin Telryn, and repair only the +two P1 gaps from Telryn PR #18 through a separately recorded correction. Do not +rewrite T-021's historical packet or receipt, implement either standalone P2, +expand the prototype, publish Telryn, or claim customer/provider qualification. + +The owner also approved installation and README simplification in the same +release. Lead with one short, exact, lifecycle-script-disabled npm install and +the path to a reviewed draft PR. Keep the trust boundary and material limits in +the README; move detailed procedures behind focused documentation links. diff --git a/schemas/delivery-record.schema.json b/schemas/delivery-record.schema.json index 16a4ac8..387d929 100644 --- a/schemas/delivery-record.schema.json +++ b/schemas/delivery-record.schema.json @@ -111,7 +111,10 @@ "reviewPolicy": { "type": "object", "properties": { - "mode": { "type": "string", "enum": ["local_only", "github_required"] }, + "mode": { + "type": "string", + "enum": ["local_only", "github_required", "github_codex_required"] + }, "requiredReviewerLogins": { "type": "array", "items": { "type": "string", "minLength": 1 } @@ -122,7 +125,9 @@ "allOf": [ { "if": { - "properties": { "mode": { "const": "github_required" } }, + "properties": { + "mode": { "enum": ["github_required", "github_codex_required"] } + }, "required": ["mode"] }, "then": { @@ -219,6 +224,14 @@ "type": "string", "pattern": "^sha256:[a-f0-9]{64}$" }, + "reviewEvidenceDigest": { + "type": "string", + "pattern": "^sha256:[a-f0-9]{64}$" + }, + "reviewCompletionDigest": { + "type": "string", + "pattern": "^sha256:[a-f0-9]{64}$" + }, "method": { "type": "string", "enum": ["merge", "squash"] }, "markReady": { "type": "boolean" }, "expiresAt": { diff --git a/schemas/mill-config.schema.json b/schemas/mill-config.schema.json index c622f1a..288fa4e 100644 --- a/schemas/mill-config.schema.json +++ b/schemas/mill-config.schema.json @@ -203,7 +203,7 @@ "properties": { "mode": { "type": "string", - "enum": ["local_only", "github_required"] + "enum": ["local_only", "github_required", "github_codex_required"] }, "requiredReviewerLogins": { "type": "array", @@ -215,7 +215,11 @@ "allOf": [ { "if": { - "properties": { "mode": { "const": "github_required" } }, + "properties": { + "mode": { + "enum": ["github_required", "github_codex_required"] + } + }, "required": ["mode"] }, "then": { diff --git a/src/contracts/schemas.ts b/src/contracts/schemas.ts index fde3088..05e5820 100644 --- a/src/contracts/schemas.ts +++ b/src/contracts/schemas.ts @@ -405,19 +405,18 @@ const repositoryMountDirectorySchema = z const githubReviewPolicySchema = z .strictObject({ - mode: z.enum(["local_only", "github_required"]), + mode: z.enum(["local_only", "github_required", "github_codex_required"]), requiredReviewerLogins: z.array(z.string().min(1)), }) .superRefine((policy, context) => { if ( - policy.mode === "github_required" && + policy.mode !== "local_only" && policy.requiredReviewerLogins.length === 0 ) { context.addIssue({ code: "custom", path: ["requiredReviewerLogins"], - message: - "github_required review policy needs at least one reviewer login", + message: "remote review policy needs at least one reviewer login", }); } }) @@ -425,7 +424,11 @@ const githubReviewPolicySchema = z allOf: [ { if: { - properties: { mode: { const: "github_required" } }, + properties: { + mode: { + enum: ["github_required", "github_codex_required"], + }, + }, required: ["mode"], }, then: { @@ -1327,6 +1330,8 @@ export const mergeApprovalPlanSchema = z.strictObject({ actorLogin: z.string().min(1), actorId: z.number().int().positive(), policyDigest: digestSchema, + reviewEvidenceDigest: digestSchema.optional(), + reviewCompletionDigest: digestSchema.optional(), method: z.enum(["merge", "squash"]), markReady: z.boolean(), expiresAt: z.iso.datetime(), diff --git a/src/runtime/delivery.ts b/src/runtime/delivery.ts index 380bf1e..55c39ff 100644 --- a/src/runtime/delivery.ts +++ b/src/runtime/delivery.ts @@ -8,7 +8,7 @@ import { } from "./effect-boundary.js"; import { parse as parseYaml } from "yaml"; -import { canonicalDigest } from "../contracts/canonical.js"; +import { canonicalDigest, type JsonValue } from "../contracts/canonical.js"; import { deliveryRecordSchema, millConfigSchema, @@ -713,7 +713,7 @@ export function actionableFeedback( candidateCommit: string, blocking?: "p0_p1", ): GitHubFeedback[] { - if (reviewPolicy.mode !== "github_required") return []; + if (reviewPolicy.mode === "local_only") return []; return observation.feedback.filter( (item) => item.commitId === candidateCommit && @@ -729,17 +729,92 @@ export function reviewsPassed( candidateCommit: string, ): boolean { if (reviewPolicy.mode === "local_only") return true; + const requiredState = + reviewPolicy.mode === "github_codex_required" + ? "CODEX_COMPLETED" + : "APPROVED"; return reviewPolicy.requiredReviewerLogins.every((login) => { const latest = observation.reviews .filter( (review) => - review.actorLogin === login && review.commitId === candidateCommit, + review.actorLogin === login && + (reviewPolicy.mode === "github_codex_required" + ? review.state.startsWith("CODEX_") + : !review.state.startsWith("CODEX_")), ) .at(-1); - return latest?.state === "APPROVED"; + return ( + latest?.state === requiredState && latest.commitId === candidateCommit + ); }); } +export function githubReviewEvidenceDigest( + observation: Pick, + reviewPolicy: DeliveryRecord["reviewPolicy"], + candidateCommit: string, +): string | undefined { + if (reviewPolicy.mode === "local_only") return undefined; + const requiredActors = new Set(reviewPolicy.requiredReviewerLogins); + const reviews = observation.reviews + .filter( + (review) => + requiredActors.has(review.actorLogin) && + review.commitId === candidateCommit, + ) + .toSorted((left, right) => left.id.localeCompare(right.id)); + const feedback = observation.feedback + .filter( + (item) => + requiredActors.has(item.actorLogin) && + item.commitId === candidateCommit, + ) + .toSorted((left, right) => left.id.localeCompare(right.id)); + return canonicalDigest( + JSON.parse( + JSON.stringify({ + schemaVersion: "1", + candidateCommit, + reviewPolicy, + reviews, + feedback, + }), + ) as JsonValue, + ); +} + +export function githubReviewCompletionDigest( + observation: Pick, + reviewPolicy: DeliveryRecord["reviewPolicy"], + candidateCommit: string, +): string | undefined { + if (reviewPolicy.mode === "local_only") return undefined; + const completions = reviewPolicy.requiredReviewerLogins.map((actorLogin) => { + const review = observation.reviews + .filter( + (item) => + item.actorLogin === actorLogin && + (reviewPolicy.mode === "github_codex_required" + ? item.state.startsWith("CODEX_") + : !item.state.startsWith("CODEX_")), + ) + .at(-1); + return review === undefined + ? { actorLogin, review: null } + : { actorLogin, review }; + }); + return canonicalDigest( + JSON.parse( + JSON.stringify({ + schemaVersion: "1", + candidateCommit, + reviewPolicy, + completions, + }), + ) as JsonValue, + ); +} + function feedbackAsReview( candidateCommit: string, feedback: readonly GitHubFeedback[], @@ -1711,7 +1786,10 @@ export async function finalizeDraftPr(input: { run.status !== "merged" && run.status !== "post_merge_verified" && !( - run.status === "blocked" && run.blockCode === "POST_MERGE_CHECKS_FAILED" + run.status === "blocked" && + ["POST_MERGE_CHECKS_FAILED", "POST_MERGE_REVIEW_BLOCKED"].includes( + run.blockCode ?? "", + ) ) ) { throw new MillError( @@ -1842,6 +1920,8 @@ export async function finalizeDraftPr(input: { }, observation: { mergeChecks: observation.mergeChecks, + reviews: observation.reviews, + feedback: observation.feedback, observedAt: new Date().toISOString(), }, lastErrorCode: null, @@ -1852,6 +1932,37 @@ export async function finalizeDraftPr(input: { if (run.status !== "merged" && run.status !== "post_merge_verified") { run = store.transition(run.id, "merged", "delivery.merged"); } + if ( + !reviewsPassed( + observation, + delivery.reviewPolicy, + delivery.candidateCommit, + ) || + actionableFeedback( + observation, + delivery.reviewPolicy, + delivery.candidateCommit, + delivery.reviewBlocking, + ).length !== 0 + ) { + delivery = persistDelivery( + store, + run.id, + { + ...delivery, + state: "blocked", + lastErrorCode: "POST_MERGE_REVIEW_BLOCKED", + }, + "delivery.post_merge_review_blocked", + ); + run = setRunBlocker( + store, + run, + "POST_MERGE_REVIEW_BLOCKED", + "delivery.blocked", + ); + return { run: publicRunRecord(run), delivery }; + } const checks = checkDecision( delivery.postMergeRequiredChecks ?? delivery.requiredChecks, observation.mergeChecks, diff --git a/src/runtime/github.ts b/src/runtime/github.ts index dfe30d8..17f9337 100644 --- a/src/runtime/github.ts +++ b/src/runtime/github.ts @@ -1,6 +1,7 @@ import path from "node:path"; import type { MillConfig } from "./inputs.js"; +import { canonicalDigest, type JsonValue } from "../contracts/canonical.js"; import { findTrustedExecutable } from "../doctor.js"; import { ExitCode, MillError } from "../errors.js"; import { runProcess } from "./process.js"; @@ -52,6 +53,7 @@ export interface GitHubReview { commitId: string | null; body: string; url: string; + revision?: string; } export interface GitHubFeedback { @@ -296,6 +298,94 @@ function priority(body: string): GitHubFeedback["priority"] { ); } +function codexSummaryReview( + value: unknown, + headSha: string, + pullRequestCommits: readonly string[], +): GitHubReview | null { + const item = object(value, "issue comment"); + const user = object(item.user, "issue comment actor"); + const body = typeof item.body === "string" ? item.body : ""; + if (!body.includes("")) return null; + const rows = body + .split("\n") + .filter((line) => /^\|\s*[^|]*\*\*Code Review\*\*[^|]*\|/iu.test(line)); + const row = rows.length === 1 ? rows[0] : undefined; + const cells = + row === undefined + ? null + : /^\|\s*📝\s*\*\*Code Review\*\*\s*\|\s*([^|]*?)\s*\|\s*([^|]*?)\s*\|\s*[^|]*\|\s*$/iu.exec( + row, + ); + const status = cells?.[1] ?? ""; + const commit = cells?.[2] ?? ""; + const commitMatch = /^`([a-f0-9]{7,40})`$/iu.exec(commit); + const commitPrefix = (commitMatch?.[1] ?? "").toLowerCase(); + const completed = status === "✅ **Completed**"; + const running = status === "🔄 **Running**"; + const state = + cells !== null && commitMatch !== null && completed !== running && completed + ? "CODEX_COMPLETED" + : cells !== null && + commitMatch !== null && + completed !== running && + running + ? "CODEX_RUNNING" + : "CODEX_INVALID"; + const matchingCommits = pullRequestCommits.filter((commit) => + commit.startsWith(commitPrefix), + ); + return { + id: `codex-summary-${integer(item.id, "issue comment ID")}`, + actorLogin: text(user.login, "issue comment actor login"), + state, + commitId: + state === "CODEX_INVALID" + ? headSha + : matchingCommits.length === 1 + ? (matchingCommits[0] ?? null) + : null, + body: "", + url: text(item.html_url, "issue comment URL"), + revision: canonicalDigest({ + updatedAt: text(item.updated_at, "issue comment update timestamp"), + body, + }), + }; +} + +function withoutCodexReviewEnvelope(body: string): string { + if ( + !body.includes("### 💡 Codex Review") || + !body.includes("**Reviewed commit:**") || + !body.includes("Codex can also answer questions or update the PR") + ) + return body; + let insideAbout = false; + const substantive: string[] = []; + for (const line of body.split("\n")) { + const value = line.trim(); + if (value.startsWith("
") && value.includes("About Codex")) { + insideAbout = true; + continue; + } + if (insideAbout) { + if (value === "
") insideAbout = false; + continue; + } + if ( + value === "### 💡 Codex Review" || + value === + "Here are some automated review suggestions for this pull request." || + value.startsWith("**Reviewed commit:**") || + value.includes("Codex can also answer questions or update the PR") + ) + continue; + substantive.push(line); + } + return substantive.join("\n").trim(); +} + function parseChecks(checkValue: unknown, statusValue: unknown): GitHubCheck[] { const checksObject = object(checkValue, "check runs"); const checkRuns = Array.isArray(checksObject.check_runs) @@ -709,9 +799,11 @@ class GhGitHubAdapter implements GitHubAdapter { const [ branchSha, checkValue, + pullRequestCommitsValue, statusValue, - reviewsValue, - commentsValue, + initialReviewsValue, + initialCommentsValue, + issueCommentsValue, defaultRefValue, ] = await Promise.all([ this.readBranch({ @@ -731,6 +823,19 @@ class GhGitHubAdapter implements GitHubAdapter { ], lifecycle, ), + input.config.reviewPolicy.mode === "github_codex_required" + ? this.#ghJson( + [ + "api", + "--hostname", + input.config.host, + "--paginate", + "--slurp", + `${prefix}/pulls/${input.pullRequestNumber}/commits?per_page=100`, + ], + lifecycle, + ) + : Promise.resolve([[]]), this.#ghJson( [ "api", @@ -764,6 +869,19 @@ class GhGitHubAdapter implements GitHubAdapter { ], lifecycle, ), + input.config.reviewPolicy.mode === "github_codex_required" + ? this.#ghJson( + [ + "api", + "--hostname", + input.config.host, + "--paginate", + "--slurp", + `${prefix}/issues/${input.pullRequestNumber}/comments?per_page=100`, + ], + lifecycle, + ) + : Promise.resolve([[]]), this.#ghJson( [ "api", @@ -774,6 +892,8 @@ class GhGitHubAdapter implements GitHubAdapter { lifecycle, ), ]); + let reviewsValue = initialReviewsValue; + let commentsValue = initialCommentsValue; const checks = await this.#bindCheckProducers( parseChecks( paginatedObjectCollection(checkValue, "check_runs", "check runs"), @@ -782,8 +902,8 @@ class GhGitHubAdapter implements GitHubAdapter { input.config, lifecycle, ); - const reviews = paginatedArray(reviewsValue, "reviews").map( - (raw): GitHubReview => { + const parseProviderReviews = (value: unknown) => + paginatedArray(value, "reviews").map((raw): GitHubReview => { const item = object(raw, "review"); const user = object(item.user, "review actor"); return { @@ -797,8 +917,79 @@ class GhGitHubAdapter implements GitHubAdapter { body: typeof item.body === "string" ? item.body : "", url: text(item.html_url, "review URL"), }; - }, + }); + const pullRequestCommits = paginatedArray( + pullRequestCommitsValue, + "pull request commits", + ).map((value) => + assertSha( + text(object(value, "pull request commit").sha, "commit SHA"), + "pull request commit SHA", + ), ); + const parseCodexSummaries = (value: unknown) => + paginatedArray(value, "issue comments").flatMap((raw): GitHubReview[] => { + const review = codexSummaryReview( + raw, + pullRequest.headSha, + pullRequestCommits, + ); + return review === null ? [] : [review]; + }); + let codexSummaries = parseCodexSummaries(issueCommentsValue); + if ( + input.config.reviewPolicy.mode === "github_codex_required" && + codexSummaries.some((review) => review.state === "CODEX_COMPLETED") + ) { + [reviewsValue, commentsValue] = await Promise.all([ + this.#ghJson( + [ + "api", + "--hostname", + input.config.host, + "--paginate", + "--slurp", + `${prefix}/pulls/${input.pullRequestNumber}/reviews?per_page=100`, + ], + lifecycle, + ), + this.#ghJson( + [ + "api", + "--hostname", + input.config.host, + "--paginate", + "--slurp", + `${prefix}/pulls/${input.pullRequestNumber}/comments?per_page=100`, + ], + lifecycle, + ), + ]); + const verifiedIssueCommentsValue = await this.#ghJson( + [ + "api", + "--hostname", + input.config.host, + "--paginate", + "--slurp", + `${prefix}/issues/${input.pullRequestNumber}/comments?per_page=100`, + ], + lifecycle, + ); + const verifiedSummaries = parseCodexSummaries(verifiedIssueCommentsValue); + const stable = + canonicalDigest(codexSummaries as unknown as JsonValue) === + canonicalDigest(verifiedSummaries as unknown as JsonValue); + codexSummaries = stable + ? verifiedSummaries + : verifiedSummaries.map((review) => ({ + ...review, + state: "CODEX_INVALID", + commitId: pullRequest.headSha, + })); + } + const providerReviews = parseProviderReviews(reviewsValue); + const reviews = [...providerReviews, ...codexSummaries]; const inlineFeedback = paginatedArray(commentsValue, "review comments").map( (raw): GitHubFeedback => { const item = object(raw, "review comment"); @@ -816,28 +1007,36 @@ class GhGitHubAdapter implements GitHubAdapter { }; }, ); - const reviewFeedback = reviews.flatMap((review): GitHubFeedback[] => { - const reviewPriority = priority(review.body); - if ( - review.body.trim().length === 0 || - review.commitId === null || - review.state === "APPROVED" - ) { - return []; - } - return [ - { - id: `review-${review.id}`, - actorLogin: review.actorLogin, - priority: reviewPriority, - body: review.body, - path: null, - line: null, - url: review.url, - commitId: review.commitId, - }, - ]; - }); + const codexSummaryActors = new Set( + codexSummaries.map((review) => review.actorLogin), + ); + const reviewFeedback = providerReviews.flatMap( + (review): GitHubFeedback[] => { + const reviewBody = codexSummaryActors.has(review.actorLogin) + ? withoutCodexReviewEnvelope(review.body) + : review.body; + const reviewPriority = priority(reviewBody); + if ( + reviewBody.length === 0 || + review.commitId === null || + review.state === "APPROVED" + ) { + return []; + } + return [ + { + id: `review-${review.id}`, + actorLogin: review.actorLogin, + priority: reviewPriority, + body: reviewBody, + path: null, + line: null, + url: review.url, + commitId: review.commitId, + }, + ]; + }, + ); const feedback = [...reviewFeedback, ...inlineFeedback]; const defaultBranchHead = assertSha( object( diff --git a/src/runtime/merge.ts b/src/runtime/merge.ts index ac02cff..1409718 100644 --- a/src/runtime/merge.ts +++ b/src/runtime/merge.ts @@ -1,6 +1,6 @@ import { blockingReviewFindings } from "./review-policy.js"; import type { z } from "zod"; -import { canonicalDigest } from "../contracts/canonical.js"; +import { canonicalDigest, type JsonValue } from "../contracts/canonical.js"; import { assessImpactManifest } from "../planning/impact.js"; import { deliveryRecordSchema, @@ -12,6 +12,8 @@ import { ExitCode, MillError } from "../errors.js"; import { actionableFeedback, checkDecision, + githubReviewCompletionDigest, + githubReviewEvidenceDigest, reviewsPassed, type DeliveryRecord, } from "./delivery.js"; @@ -28,7 +30,7 @@ import { commonGitDirectory, repositoryRemoteUrl, } from "./repository.js"; -import { acquireWriterLease, StateStore } from "./state.js"; +import { acquireWriterLease, StateStore, type RunStatus } from "./state.js"; type MergePlan = z.infer; type Approval = NonNullable; @@ -44,6 +46,7 @@ interface MergeContext { adapter: GitHubAdapter; inputs: RuntimeInputs; config: ProposeConfig; + runStatus: RunStatus; save(value: Approval): void; } @@ -133,7 +136,11 @@ async function withMergeContext( ); if ( !readback && - (run.status !== "awaiting_human" || + ((run.status !== "awaiting_human" && + !( + run.status === "awaiting_ci" && + config.reviewPolicy.mode === "github_codex_required" + )) || run.cancelRequested || run.configDigest !== inputs.configDigest || run.taskDigest !== inputs.taskDigest) @@ -148,6 +155,7 @@ async function withMergeContext( delivery, inputs, config, + runStatus: run.status, adapter: input.adapter ?? createGitHubAdapter(input.root), save(value) { delivery.mergeApproval = value; @@ -173,6 +181,7 @@ async function preflight( input: MergeInput, context: MergeContext, limit?: number, + options: { allowCodexReadiness?: boolean } = {}, ) { const { inputs, config, delivery, adapter, store } = context; assertCurrentAuthority(inputs); @@ -275,28 +284,57 @@ async function preflight( "The open PR identity or head changed.", ExitCode.configuration, ); - if ( + const checksPassed = checkDecision( config.requiredChecks, observation.checks, config.checkProducers, "pull_request", candidate.commit, - ).status !== "passed" || - !reviewsPassed(observation, config.reviewPolicy, candidate.commit) || - actionableFeedback( - observation, - config.reviewPolicy, - candidate.commit, - delivery.reviewBlocking, - ).length !== 0 + ).status === "passed"; + const reviewReady = reviewsPassed( + observation, + config.reviewPolicy, + candidate.commit, + ); + const blockingFeedback = actionableFeedback( + observation, + config.reviewPolicy, + candidate.commit, + delivery.reviewBlocking, + ); + const readinessOnly = + options.allowCodexReadiness === true && + pull.draft && + config.reviewPolicy.mode === "github_codex_required"; + if ( + !checksPassed || + blockingFeedback.length !== 0 || + (!reviewReady && !readinessOnly) ) throw new MillError( "MERGE_CHECKS_NOT_GREEN", "Current producer-bound checks and review must pass before any merge effect.", ExitCode.configuration, ); - return { config, binding, candidate, observation, deadlineMs }; + return { + config, + binding, + candidate, + observation, + deadlineMs, + reviewReady, + reviewCompletionDigest: githubReviewCompletionDigest( + observation, + config.reviewPolicy, + candidate.commit, + ), + reviewEvidenceDigest: githubReviewEvidenceDigest( + observation, + config.reviewPolicy, + candidate.commit, + ), + }; } export async function planMerge( @@ -314,7 +352,31 @@ export async function planMerge( "An attempted merge plan must be reconciled, not overwritten.", ExitCode.configuration, ); - const current = await preflight(input, context); + const current = await preflight(input, context, undefined, { + allowCodexReadiness: true, + }); + if ( + context.runStatus === "awaiting_ci" && + !current.observation.pullRequest.draft + ) + throw new MillError( + "MERGE_NOT_READY", + "Observe the completed hosted review before planning the final merge.", + ExitCode.configuration, + ); + if ( + current.config.reviewPolicy.mode === "github_codex_required" && + !current.observation.pullRequest.draft && + prior?.state === "ready_verified" && + prior.plan.markReady && + (prior.plan.reviewCompletionDigest === undefined || + prior.plan.reviewCompletionDigest === current.reviewCompletionDigest) + ) + throw new MillError( + "MERGE_NOT_READY", + "The hosted review must complete again after the pull request becomes ready.", + ExitCode.configuration, + ); if ( !current.config.allowedMergeMethods.includes( input.method === "squash" ? "linear_tree_preserving" : "merge", @@ -336,6 +398,12 @@ export async function planMerge( actorLogin: current.binding.actorLogin, actorId: current.binding.actorId, policyDigest: context.inputs.configDigest, + ...(current.reviewEvidenceDigest === undefined + ? {} + : { reviewEvidenceDigest: current.reviewEvidenceDigest }), + ...(current.reviewCompletionDigest === undefined + ? {} + : { reviewCompletionDigest: current.reviewCompletionDigest }), method: input.method, markReady: current.observation.pullRequest.draft, expiresAt: new Date( @@ -347,7 +415,7 @@ export async function planMerge( }); const approval = { plan, - digest: canonicalDigest(plan), + digest: canonicalDigest(JSON.parse(JSON.stringify(plan)) as JsonValue), state: "planned" as const, }; context.save(approval); @@ -380,7 +448,9 @@ export async function applyMerge( if ( approval?.state !== "planned" || approval.digest !== input.approvalDigest || - canonicalDigest(approval.plan) !== input.approvalDigest || + canonicalDigest( + JSON.parse(JSON.stringify(approval.plan)) as JsonValue, + ) !== input.approvalDigest || Date.parse(approval.plan.expiresAt) <= Date.now() ) throw new MillError( @@ -389,12 +459,20 @@ export async function applyMerge( ExitCode.configuration, ); const plan = approval.plan; + if (context.runStatus === "awaiting_ci" && !plan.markReady) + throw new MillError( + "MERGE_NOT_READY", + "Observe the completed hosted review before applying the final merge.", + ExitCode.configuration, + ); const effectDeadline = Math.min( Date.now() + context.config.pollTimeoutSeconds * 1000, Date.parse(plan.expiresAt), authorityDeadline(context.inputs), ); - const current = await preflight(input, context, effectDeadline); + const current = await preflight(input, context, effectDeadline, { + allowCodexReadiness: plan.markReady, + }); if ( !readbackMatches(current.observation, plan) || current.observation.defaultBranchHead !== plan.baseCommit || @@ -402,6 +480,10 @@ export async function applyMerge( current.binding.actorId !== plan.actorId || current.binding.actorLogin !== plan.actorLogin || context.inputs.configDigest !== plan.policyDigest || + (plan.reviewEvidenceDigest !== undefined && + current.reviewEvidenceDigest !== plan.reviewEvidenceDigest) || + (plan.reviewCompletionDigest !== undefined && + current.reviewCompletionDigest !== plan.reviewCompletionDigest) || current.observation.pullRequest.draft !== plan.markReady ) throw new MillError( @@ -410,7 +492,7 @@ export async function applyMerge( ExitCode.configuration, ); if ( - context.adapter.mergeExact === undefined || + (!plan.markReady && context.adapter.mergeExact === undefined) || (plan.markReady && context.adapter.markReady === undefined) ) throw new MillError( @@ -471,10 +553,22 @@ export async function applyMerge( ExitCode.temporary, ); save("ready_verified"); + if (current.config.reviewPolicy.mode === "github_codex_required") { + const run = context.store.getRun(input.runId); + if (run.status !== "awaiting_ci") + context.store.transition( + run.id, + "awaiting_ci", + "delivery.hosted_review_retriggered", + ); + return approval; + } } const fresh = await preflight(input, context, effectDeadline); if ( fresh.observation.defaultBranchHead !== plan.baseCommit || + fresh.reviewEvidenceDigest !== plan.reviewEvidenceDigest || + fresh.reviewCompletionDigest !== plan.reviewCompletionDigest || Date.now() >= effectDeadline || cancellationRequested() ) @@ -484,6 +578,12 @@ export async function applyMerge( ExitCode.configuration, ); assertCurrentAuthority(context.inputs); + if (context.adapter.mergeExact === undefined) + throw new MillError( + "MERGE_ADAPTER_UNAVAILABLE", + "The configured adapter does not implement attended merge.", + ExitCode.unavailable, + ); save("merge_started"); await context.adapter.mergeExact({ config: current.config, diff --git a/src/runtime/outcome.ts b/src/runtime/outcome.ts index 776b9f6..852708b 100644 --- a/src/runtime/outcome.ts +++ b/src/runtime/outcome.ts @@ -425,6 +425,10 @@ function recordedReviewsPass( ): boolean { if (delivery.reviewPolicy.mode === "local_only") return true; if (!Array.isArray(value)) return false; + const requiredState = + delivery.reviewPolicy.mode === "github_codex_required" + ? "CODEX_COMPLETED" + : "APPROVED"; return delivery.reviewPolicy.requiredReviewerLogins.every((login) => { let latest: string | undefined; for (const entry of value) { @@ -438,11 +442,16 @@ function recordedReviewsPass( return false; if ( review.actorLogin === login && - review.commitId === delivery.candidateCommit + (delivery.reviewPolicy.mode === "github_codex_required" + ? review.state.startsWith("CODEX_") + : !review.state.startsWith("CODEX_")) ) - latest = review.state; + latest = + review.commitId === delivery.candidateCommit + ? review.state + : undefined; } - return latest === "APPROVED"; + return latest === requiredState; }); } diff --git a/src/runtime/state.ts b/src/runtime/state.ts index c73799c..1a7a352 100644 --- a/src/runtime/state.ts +++ b/src/runtime/state.ts @@ -310,7 +310,14 @@ const transitions: Readonly> = { "failed", "stale", ], - awaiting_human: ["merged", "blocked", "cancelled", "failed", "stale"], + awaiting_human: [ + "awaiting_ci", + "merged", + "blocked", + "cancelled", + "failed", + "stale", + ], merged: ["post_merge_verified", "blocked", "failed", "stale"], post_merge_verified: ["closed", "blocked", "failed", "stale"], closed: [], diff --git a/src/version.ts b/src/version.ts index 02a557b..d89e229 100644 --- a/src/version.ts +++ b/src/version.ts @@ -1,3 +1,3 @@ export const MILL_PACKAGE = "@davidahmann/mill"; -export const MILL_VERSION = "0.8.1"; +export const MILL_VERSION = "0.9.0"; export const RESULT_SCHEMA_VERSION = "1"; diff --git a/test/runtime-delivery.test.ts b/test/runtime-delivery.test.ts index 02342e8..b27f2c8 100644 --- a/test/runtime-delivery.test.ts +++ b/test/runtime-delivery.test.ts @@ -25,6 +25,7 @@ import { planDraftPr, reconcileDraftPr, reviewsPassed, + type DeliveryRecord, } from "../src/runtime/delivery.js"; import type { GitHubAdapter, @@ -456,6 +457,7 @@ async function reviewedFixture( adaptationExpiresAt?: string; impactExpiresAt?: string; githubReviewer?: string; + githubReviewMode?: "github_required" | "github_codex_required"; requiredChecks?: readonly string[]; postMergeRequiredChecks?: readonly string[]; prepare?: ( @@ -480,7 +482,12 @@ async function reviewedFixture( ...(options.attendedMerge === true ? { attendedMerge: true } : {}), ...(options.githubReviewer === undefined ? {} - : { githubReviewer: options.githubReviewer }), + : { + githubReviewer: options.githubReviewer, + ...(options.githubReviewMode === undefined + ? {} + : { githubReviewMode: options.githubReviewMode }), + }), }); if ( options.requiredChecks !== undefined || @@ -558,6 +565,218 @@ async function planAndOpen(input: { } describe("exact-candidate GitHub draft delivery", () => { + it("gates GitHub Codex delivery across readiness, exact review evidence, and post-merge readback", async () => { + const { fixture, runId, candidateCommit, candidateTree } = + await reviewedFixture({ + attendedMerge: true, + githubReviewer: "chatgpt-codex-connector[bot]", + githubReviewMode: "github_codex_required", + reviewBlocking: "p0_p1", + }); + class CodexMergeGitHub extends FakeGitHub { + readyCalls = 0; + mergeCalls = 0; + observeCalls = 0; + changeFeedbackOn = 0; + async strictChecks() { + await Promise.resolve(); + return true; + } + async markReady() { + await Promise.resolve(); + this.readyCalls++; + if (this.pullRequest === null) throw new Error("missing fake PR"); + this.pullRequest = { ...this.pullRequest, draft: false }; + } + async mergeExact() { + await Promise.resolve(); + this.mergeCalls++; + this.merge(candidateTree); + } + override async observe() { + this.observeCalls++; + if (this.observeCalls === this.changeFeedbackOn) + this.feedback = [advisory("[P2] Changed during final preflight")]; + return super.observe(); + } + } + const adapter = new CodexMergeGitHub( + (await git(fixture.root, ["rev-parse", "main"])).stdout.trim(), + ); + const input = { + root: fixture.root, + taskPath: fixture.taskPath, + runId, + adapter, + }; + const review = ( + state: "CODEX_RUNNING" | "CODEX_COMPLETED", + ): GitHubReview => ({ + id: `summary-${state}`, + actorLogin: "chatgpt-codex-connector[bot]", + state, + commitId: candidateCommit, + body: "", + url: "https://github.com/example/app/pull/41#issuecomment-1", + }); + const advisory = (body = "[P2] Optional cleanup"): GitHubFeedback => ({ + id: "advisory-1", + actorLogin: "chatgpt-codex-connector[bot]", + priority: "P2", + body, + path: "src/value.js", + line: 1, + url: "https://github.com/example/app/pull/41#discussion-1", + commitId: candidateCommit, + }); + const check = { + ...completedCheck("success"), + appId: 15368, + workflowPath: ".github/workflows/ci.yml", + event: "pull_request", + headSha: candidateCommit, + }; + try { + await planAndOpen({ fixture, runId, adapter }); + adapter.checks = [check]; + adapter.reviews = [review("CODEX_COMPLETED")]; + adapter.feedback = [advisory()]; + expect((await observeDraftPr(input)).run.status).toBe("awaiting_human"); + + const readiness = await planMerge({ ...input, method: "squash" }); + expect(readiness.plan.markReady).toBe(true); + expect(readiness.plan.reviewEvidenceDigest).toMatch(/^sha256:/u); + expect(readiness.plan.reviewCompletionDigest).toMatch(/^sha256:/u); + expect( + ( + await applyMerge({ + ...input, + approvalDigest: readiness.digest, + attended: true, + }) + ).state, + ).toBe("ready_verified"); + expect(adapter.readyCalls).toBe(1); + expect(adapter.mergeCalls).toBe(0); + + await expect( + planMerge({ ...input, method: "squash" }), + ).rejects.toMatchObject({ code: "MERGE_NOT_READY" }); + adapter.feedback = [advisory("[P2] changed after readiness")]; + await expect( + planMerge({ ...input, method: "squash" }), + ).rejects.toMatchObject({ code: "MERGE_NOT_READY" }); + adapter.reviews = [ + { ...review("CODEX_RUNNING"), id: "summary-after-ready-running" }, + ]; + expect((await observeDraftPr(input)).run.status).toBe("awaiting_ci"); + adapter.reviews = [ + { ...review("CODEX_COMPLETED"), id: "summary-after-ready-completed" }, + ]; + await expect( + planMerge({ ...input, method: "squash" }), + ).rejects.toMatchObject({ code: "MERGE_NOT_READY" }); + expect((await observeDraftPr(input)).run.status).toBe("awaiting_human"); + const merge = await planMerge({ ...input, method: "squash" }); + expect(merge.plan.markReady).toBe(false); + expect(merge.plan.reviewEvidenceDigest).toMatch(/^sha256:/u); + adapter.feedback = [advisory("[P2] Changed after approval")]; + await expect( + applyMerge({ + ...input, + approvalDigest: merge.digest, + attended: true, + }), + ).rejects.toMatchObject({ code: "MERGE_PLAN_STALE" }); + + const finalDrift = await planMerge({ ...input, method: "squash" }); + adapter.changeFeedbackOn = adapter.observeCalls + 2; + await expect( + applyMerge({ + ...input, + approvalDigest: finalDrift.digest, + attended: true, + }), + ).rejects.toMatchObject({ code: "MERGE_PLAN_STALE" }); + adapter.changeFeedbackOn = 0; + const fresh = await planMerge({ ...input, method: "squash" }); + expect( + ( + await applyMerge({ + ...input, + approvalDigest: fresh.digest, + attended: true, + }) + ).state, + ).toBe("merged"); + expect(adapter.mergeCalls).toBe(1); + adapter.feedback = [{ ...advisory(), id: "blocking-1", priority: "P1" }]; + adapter.mergeChecks = [ + { ...check, event: "push", headSha: "c".repeat(40) }, + ]; + expect((await finalizeDraftPr(input)).run).toMatchObject({ + status: "blocked", + blockCode: "POST_MERGE_REVIEW_BLOCKED", + }); + adapter.feedback = [advisory()]; + expect((await finalizeDraftPr(input)).run.status).toBe("closed"); + } finally { + await fixture.cleanup(); + } + }); + + it("blocks completed GitHub Codex review when exact-head P1 feedback remains", async () => { + const { fixture, runId, candidateCommit } = await reviewedFixture({ + githubReviewer: "chatgpt-codex-connector[bot]", + githubReviewMode: "github_codex_required", + reviewBlocking: "p0_p1", + }); + const adapter = new FakeGitHub( + (await git(fixture.root, ["rev-parse", "main"])).stdout.trim(), + ); + try { + await planAndOpen({ fixture, runId, adapter }); + adapter.checks = [completedCheck("success")]; + adapter.reviews = [ + { + id: "codex-complete", + actorLogin: "chatgpt-codex-connector[bot]", + state: "CODEX_COMPLETED", + commitId: candidateCommit, + body: "", + url: "https://github.com/example/app/pull/41#issuecomment-1", + }, + ]; + adapter.feedback = [ + { + id: "p1", + actorLogin: "chatgpt-codex-connector[bot]", + priority: "P1", + body: "[P1] Correctness gap", + path: "src/value.js", + line: 1, + url: "https://github.com/example/app/pull/41#discussion-1", + commitId: candidateCommit, + }, + ]; + expect( + ( + await observeDraftPr({ + root: fixture.root, + taskPath: fixture.taskPath, + runId, + adapter, + }) + ).run, + ).toMatchObject({ + status: "blocked", + blockCode: "REMOTE_REVIEW_FINDINGS", + }); + } finally { + await fixture.cleanup(); + } + }); + it.each([ ["push", "before", false], ["push", "after", false], @@ -2850,6 +3069,112 @@ describe("exact-candidate GitHub draft delivery", () => { candidateCommit, ), ).toBe(true); + expect( + reviewsPassed( + { + ...observation, + reviews: [ + { + ...firstReview, + state: "APPROVED", + body: "", + }, + { + ...firstReview, + id: "codex-summary", + state: "CODEX_COMPLETED", + body: "", + }, + ], + }, + policy, + candidateCommit, + ), + ).toBe(true); + }); + + it("requires exact-head GitHub Codex completion and blocks only configured priorities", () => { + const candidateCommit = "a".repeat(40); + const policy = { + mode: "github_codex_required", + requiredReviewerLogins: ["chatgpt-codex-connector[bot]"], + } as unknown as DeliveryRecord["reviewPolicy"]; + const observation = { + reviews: [ + { + id: "codex-summary", + actorLogin: "chatgpt-codex-connector[bot]", + state: "CODEX_COMPLETED", + commitId: candidateCommit, + body: "", + url: "https://github.com/example/app/pull/41#issuecomment-1", + }, + ], + feedback: [], + } as Pick as GitHubObservation; + + expect(reviewsPassed(observation, policy, candidateCommit)).toBe(true); + const completedReview = observation.reviews.at(0); + if (completedReview === undefined) + throw new Error("missing completed Codex review"); + expect( + reviewsPassed( + { + ...observation, + reviews: [ + ...observation.reviews, + { + ...completedReview, + id: "codex-invalid", + state: "CODEX_INVALID", + }, + ], + }, + policy, + candidateCommit, + ), + ).toBe(false); + expect( + reviewsPassed( + { + ...observation, + reviews: observation.reviews.map((review) => ({ + ...review, + state: "CODEX_RUNNING", + })), + }, + policy, + candidateCommit, + ), + ).toBe(false); + expect(reviewsPassed(observation, policy, "b".repeat(40))).toBe(false); + + const feedback = (priority: "P1" | "P2"): GitHubFeedback => ({ + id: priority, + actorLogin: "chatgpt-codex-connector[bot]", + priority, + body: `[${priority}] finding`, + path: "src/index.ts", + line: 1, + url: `https://github.com/example/app/pull/41#discussion_${priority}`, + commitId: candidateCommit, + }); + expect( + actionableFeedback( + { ...observation, feedback: [feedback("P1")] }, + policy, + candidateCommit, + "p0_p1", + ), + ).toHaveLength(1); + expect( + actionableFeedback( + { ...observation, feedback: [feedback("P2")] }, + policy, + candidateCommit, + "p0_p1", + ), + ).toHaveLength(0); }); it("fails closed on PR drift and post-merge evidence until every identity settles", async () => { diff --git a/test/runtime-fixture.ts b/test/runtime-fixture.ts index f666038..00b9ed6 100644 --- a/test/runtime-fixture.ts +++ b/test/runtime-fixture.ts @@ -80,6 +80,7 @@ export async function runtimeFixture( impactExpiresAt?: string; nativeRepair?: boolean; githubReviewer?: string; + githubReviewMode?: "github_required" | "github_codex_required"; reviewerCacheInputTokens?: number; } = {}, ): Promise<{ @@ -234,9 +235,13 @@ scenarios: ), ]); const reviewMode = - options.githubReviewer === undefined ? "local_only" : "github_required"; + options.githubReviewer === undefined + ? "local_only" + : (options.githubReviewMode ?? "github_required"); const reviewers = - options.githubReviewer === undefined ? "[]" : `[${options.githubReviewer}]`; + options.githubReviewer === undefined + ? "[]" + : JSON.stringify([options.githubReviewer]); const proposalConfiguration = options.propose === true ? `propose: diff --git a/test/runtime-github.test.ts b/test/runtime-github.test.ts index d62e761..c373a8e 100644 --- a/test/runtime-github.test.ts +++ b/test/runtime-github.test.ts @@ -36,8 +36,8 @@ const config: ProposeConfig = { allowedMergerLogins: ["operator"], requiredChecks: ["validate"], reviewPolicy: { - mode: "github_required", - requiredReviewerLogins: ["codex-review"], + mode: "github_codex_required", + requiredReviewerLogins: ["chatgpt-codex-connector[bot]"], }, allowedMergeMethods: ["linear_tree_preserving"], approvalTtlSeconds: 900, @@ -49,15 +49,41 @@ describe("GitHub CLI adapter", () => { const repository = await temporaryDirectory("mill-github-repository-"); const tools = await temporaryDirectory("mill-github-tools-"); const gh = path.join(tools.path, "gh"); + const completedSummary = ` +| Review | Status | Commit | Review trigger | +| --- | --- | --- | --- | +| 📝 **Code Review** | ✅ **Completed** | \`${sha.slice(0, 7)}\` | PR opened |`; + const fullReviewEnvelope = ` +### 💡 Codex Review + +Here are some automated review suggestions for this pull request. + +**Reviewed commit:** \`${sha.slice(0, 10)}\` + +
ℹī¸ About Codex in GitHub +
+ +[Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you +- Open a pull request for review +- Mark a draft as ready +- Comment "@codex review". + +If Codex has suggestions, it will comment; otherwise it will react with 👍. + +Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". + +
`; try { await writeFile( gh, `#!${process.execPath} import {appendFileSync,existsSync,readFileSync} from "node:fs"; const modeUrl=new URL("./mode.json",import.meta.url);const mode=existsSync(modeUrl)?JSON.parse(readFileSync(modeUrl,"utf8")):{}; -appendFileSync(new URL("./calls.log",import.meta.url),JSON.stringify(process.argv.slice(2))+"\\n"); +const callsUrl=new URL("./calls.log",import.meta.url);const priorCalls=existsSync(callsUrl)?readFileSync(callsUrl,"utf8").trim().split("\\n").filter(Boolean).map(JSON.parse):[]; +appendFileSync(callsUrl,JSON.stringify(process.argv.slice(2))+"\\n"); if(process.env.GH_TOKEN!==undefined)appendFileSync(new URL("./token.log",import.meta.url),process.env.GH_TOKEN==="scoped-token"?"present\\n":"unexpected\\n"); const args=process.argv.slice(2);const endpoint=args.includes("graphql")?"graphql":args.find((value)=>value.startsWith("repos/"))??args.at(-1)??""; +const endpointCall=priorCalls.filter((call)=>{const priorEndpoint=call.includes("graphql")?"graphql":call.find((value)=>value.startsWith("repos/"))??call.at(-1)??"";return priorEndpoint===endpoint;}).length; const pull={number:41,node_id:"PR_example",html_url:"https://github.com/example/app/pull/41",state:"open",draft:true,body:"",head:{ref:"mill/task",sha:"${sha}"},base:{ref:"main"},merged:false,merge_commit_sha:null,merged_by:null,merged_at:null}; const listedPull={...pull};delete listedPull.merged;delete listedPull.merged_by;delete listedPull.merged_at; if(endpoint==="user")console.log(JSON.stringify({login:"operator",id:7})); @@ -67,6 +93,7 @@ else if(endpoint.includes("/git/ref/heads/"))console.log(JSON.stringify({object: else if(endpoint.includes("/pulls?"))console.log(JSON.stringify([[listedPull]])); else if(args.includes("--method")&&endpoint==="repos/example/app/pulls")console.log(JSON.stringify(pull)); else if(endpoint.endsWith("/pulls/41"))console.log(JSON.stringify(pull)); +else if(endpoint.includes("/pulls/41/commits?"))console.log(JSON.stringify([((mode.commits??["${sha}"]).map((sha)=>({sha}))) ])); else if(endpoint.includes("/check-runs"))console.log(JSON.stringify([{check_runs:[{id:101,name:"validate",status:"completed",conclusion:"success",app:{id:15368},head_sha:"${sha}",details_url:"https://github.com/example/app/actions/runs/50/job/101",...mode.check}]}])); else if(endpoint.endsWith("/actions/jobs/101"))console.log(JSON.stringify({id:101,run_id:50,head_sha:"${sha}",check_run_url:"https://api.github.com/repos/example/app/check-runs/101",...mode.job})); else if(endpoint.endsWith("/actions/runs/50"))console.log(JSON.stringify({head_sha:"${sha}",repository:{node_id:"R_example"},path:".github/workflows/ci.yml",event:"pull_request",...mode.run})); @@ -74,8 +101,9 @@ else if(endpoint.endsWith("/protection"))console.log(JSON.stringify({enforce_adm else if(endpoint==="graphql")console.log(JSON.stringify(mode.ready??{data:{markPullRequestReadyForReview:{pullRequest:{id:"PR_example",isDraft:false}}}})); else if(endpoint.endsWith("/pulls/41/merge"))console.log(JSON.stringify({merged:mode.merged??true})); else if(endpoint.includes("/status?"))console.log(JSON.stringify([{statuses:[{state:"pending",context:"legacy"}]}])) -else if(endpoint.includes("/reviews?"))console.log(JSON.stringify([[{id:11,user:{login:"codex-review"},state:"COMMENTED",commit_id:"${sha}",body:"Top-level concern without a priority label",html_url:"https://github.com/example/app/pull/41#pullrequestreview-11",...mode.review}]])); -else if(endpoint.includes("/comments?"))console.log(JSON.stringify([[{id:12,user:{login:"codex-review"},body:"[P2] clarify edge case",path:"src/index.ts",line:4,html_url:"https://github.com/example/app/pull/41#discussion_r12",commit_id:"${sha}"}]])); +else if(endpoint.includes("/reviews?"))console.log(JSON.stringify([[{id:11,user:{login:"chatgpt-codex-connector[bot]"},state:"COMMENTED",commit_id:"${sha}",body:"Top-level concern without a priority label",html_url:"https://github.com/example/app/pull/41#pullrequestreview-11",...mode.review}]])); +else if(endpoint.includes("/issues/41/comments?")){const bodies=mode.issueBodies??[mode.issueBody??${JSON.stringify(completedSummary)}];const updatedAt=(mode.issueUpdatedAts??[])[endpointCall]??"2026-09-21T12:00:00Z";console.log(JSON.stringify([bodies.map((body,index)=>({id:13+index,user:{login:"chatgpt-codex-connector[bot]"},body,updated_at:updatedAt,html_url:"https://github.com/example/app/pull/41#issuecomment-"+(13+index)}))]));} +else if(endpoint.includes("/comments?")){const body=mode.interleave&&endpointCall>0?"[P1] published with completion":"[P2] clarify edge case";console.log(JSON.stringify([[{id:12,user:{login:"chatgpt-codex-connector[bot]"},body,path:"src/index.ts",line:4,html_url:"https://github.com/example/app/pull/41#discussion_r12",commit_id:"${sha}"}]]));} else process.exit(2); `, { mode: 0o755 }, @@ -152,11 +180,18 @@ else process.exit(2); reviews: [ { id: "11", - actorLogin: "codex-review", + actorLogin: "chatgpt-codex-connector[bot]", state: "COMMENTED", commitId: sha, body: "Top-level concern without a priority label", }, + { + id: "codex-summary-13", + actorLogin: "chatgpt-codex-connector[bot]", + state: "CODEX_COMPLETED", + commitId: sha, + body: "", + }, ], feedback: [ { priority: "unclassified", commitId: sha, path: null }, @@ -174,9 +209,271 @@ else process.exit(2); deadlineMs: Date.now() + 10_000, }), ).resolves.toMatchObject({ - reviews: [{ state: "APPROVED", body: "LGTM" }], + reviews: [ + { state: "APPROVED", body: "LGTM" }, + { state: "CODEX_COMPLETED", commitId: sha }, + ], feedback: [{ priority: "P2", path: "src/index.ts" }], }); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + issueBody: completedSummary + .replace("✅ **Completed**", "🔄 **Running**") + .replace(sha.slice(0, 7), "b".repeat(7)), + }), + ); + await expect( + adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }), + ).resolves.toMatchObject({ + reviews: [ + { state: "COMMENTED", commitId: sha }, + { state: "CODEX_RUNNING", commitId: null }, + ], + }); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + commits: [sha, `${sha.slice(0, 7)}${"b".repeat(33)}`], + }), + ); + await expect( + adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }), + ).resolves.toMatchObject({ + reviews: [ + { state: "COMMENTED", commitId: sha }, + { state: "CODEX_COMPLETED", commitId: null }, + ], + }); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + issueBody: completedSummary.replace( + "✅ **Completed**", + "✅ **Completed** 🔄 **Running**", + ), + }), + ); + await expect( + adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }), + ).resolves.toMatchObject({ + reviews: [ + { state: "COMMENTED", commitId: sha }, + { state: "CODEX_INVALID", commitId: sha }, + ], + }); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + issueBody: `${completedSummary}\n| 📝 **Code Review** | 🔄 **Running** | malformed | retried |`, + }), + ); + await expect( + adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }), + ).resolves.toMatchObject({ + reviews: [ + { state: "COMMENTED", commitId: sha }, + { state: "CODEX_INVALID", commitId: sha }, + ], + }); + const callsBeforeInterleave = await readFile( + path.join(tools.path, "calls.log"), + "utf8", + ); + await writeFile(path.join(tools.path, "calls.log"), ""); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ interleave: true }), + ); + const interleaved = await adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }); + expect(interleaved.reviews).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + state: "CODEX_COMPLETED", + commitId: sha, + }), + ]), + ); + expect(interleaved.feedback).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + priority: "P1", + body: "[P1] published with completion", + }), + ]), + ); + const interleaveCalls = await readFile( + path.join(tools.path, "calls.log"), + "utf8", + ); + await writeFile( + path.join(tools.path, "calls.log"), + `${callsBeforeInterleave}${interleaveCalls}`, + ); + const callsBeforeRevisionCheck = await readFile( + path.join(tools.path, "calls.log"), + "utf8", + ); + await writeFile(path.join(tools.path, "calls.log"), ""); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + issueUpdatedAts: ["2026-09-21T12:00:00Z", "2026-09-21T12:00:01Z"], + }), + ); + await expect( + adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }), + ).resolves.toMatchObject({ + reviews: [ + { state: "COMMENTED", commitId: sha }, + { state: "CODEX_INVALID", commitId: sha }, + ], + }); + const revisionCheckCalls = await readFile( + path.join(tools.path, "calls.log"), + "utf8", + ); + await writeFile( + path.join(tools.path, "calls.log"), + `${callsBeforeRevisionCheck}${revisionCheckCalls}`, + ); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + issueBody: `${completedSummary}\n| 📝 **Code Review** | ✅ **Completed** | \`${sha.slice(0, 7)}\` | retried |`, + }), + ); + await expect( + adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }), + ).resolves.toMatchObject({ + reviews: [ + { state: "COMMENTED", commitId: sha }, + { state: "CODEX_INVALID", commitId: sha }, + ], + }); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + review: { + body: fullReviewEnvelope, + }, + }), + ); + await expect( + adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }), + ).resolves.toMatchObject({ + reviews: [ + { state: "COMMENTED", commitId: sha }, + { state: "CODEX_COMPLETED", commitId: sha }, + ], + feedback: [{ priority: "P2", path: "src/index.ts" }], + }); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + issueBody: + "\n| 📝 **Code Review** | unknown | malformed |", + }), + ); + const malformed = await adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }); + expect( + malformed.reviews.some((review) => review.state.startsWith("CODEX_")), + ).toBe(true); + expect(malformed.reviews).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + state: "CODEX_INVALID", + commitId: sha, + }), + ]), + ); + expect(malformed.feedback).toEqual( + expect.arrayContaining([ + expect.objectContaining({ priority: "unclassified" }), + ]), + ); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + issueBodies: [ + completedSummary, + "\n| 📝 **Code Review** | unknown | malformed |", + ], + }), + ); + await expect( + adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }), + ).resolves.toMatchObject({ + reviews: [ + { state: "COMMENTED", commitId: sha }, + { state: "CODEX_COMPLETED", commitId: sha }, + { state: "CODEX_INVALID", commitId: sha }, + ], + }); + await writeFile( + path.join(tools.path, "mode.json"), + JSON.stringify({ + review: { + body: fullReviewEnvelope.replace( + "
", + "[P1] retain this finding\n\n
", + ), + }, + }), + ); + const substantive = await adapter.observe({ + config, + pullRequestNumber: 41, + deadlineMs: Date.now() + 10_000, + }); + expect(substantive.feedback).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + priority: "P1", + body: "[P1] retain this finding", + }), + ]), + ); await writeFile(path.join(tools.path, "mode.json"), "{}"); const calls = await readFile(path.join(tools.path, "calls.log"), "utf8"); const producerConfig: ProposeConfig = { @@ -470,6 +767,7 @@ else process.exit(2); const args=process.argv.slice(2);const endpoint=args.find((value)=>value.startsWith("repos/"))??args.at(-1)??""; const pull={number:41,node_id:"PR_example",html_url:"https://github.com/example/app/pull/41",state:"closed",draft:false,body:"marker",head:{ref:"mill/task",sha:"${sha}"},base:{ref:"main"},merged:true,merge_commit_sha:"${mergeSha}",merged_by:{login:"operator"},merged_at:"2026-09-01T17:00:00.000Z"}; if(endpoint.endsWith("/pulls/41"))console.log(JSON.stringify(pull)); +else if(endpoint.includes("/pulls/41/commits?"))console.log(JSON.stringify([[{sha:"${sha}"}]])); else if(endpoint.includes("/git/ref/heads/mill")){console.error("HTTP 404");process.exit(1)} else if(endpoint.includes("/git/ref/heads/main"))console.log(JSON.stringify({object:{sha:"${mergeSha}"}})); else if(endpoint.includes("/check-runs"))console.log(JSON.stringify([{check_runs:[{name:"validate",status:"completed",conclusion:"success"}]}])); @@ -522,6 +820,7 @@ else if(endpoint==="repos/example/app")console.log(JSON.stringify(mode==="bad-re else if(endpoint.includes("/git/ref/heads/"))console.log(JSON.stringify({object:{sha:mode==="bad-sha"?"bad":"${sha}"}})); else if(endpoint.includes("/pulls?"))console.log(JSON.stringify(mode==="bad-pages"?{}:mode==="bad-page"?[{}]:[[listedPull]])); else if(endpoint.endsWith("/pulls/41"))console.log(JSON.stringify(pull)); +else if(endpoint.includes("/pulls/41/commits?"))console.log(JSON.stringify([[{sha:"${sha}"}]])); else if(endpoint.includes("/check-runs"))console.log(JSON.stringify(mode==="bad-check-pages"?{}:mode==="bad-check-page"?[{}]:[{check_runs:[]}])) else if(endpoint.includes("/status?"))console.log(JSON.stringify([{statuses:[]}])) else if(endpoint.includes("/reviews?"))console.log(JSON.stringify([[]])); diff --git a/test/runtime-outcome.test.ts b/test/runtime-outcome.test.ts index 965f54c..c482ec5 100644 --- a/test/runtime-outcome.test.ts +++ b/test/runtime-outcome.test.ts @@ -859,12 +859,70 @@ describe("run outcome projection", () => { state: "APPROVED", commitId: candidateCommit, }, + { + actorLogin: "reviewer", + state: "CODEX_COMPLETED", + commitId: candidateCommit, + }, ], feedback: [], }; ready.deliveryJson = JSON.stringify(delivery); expect(outcome(ready).integrity.status).toBe("consistent"); + const codexReady = run(); + codexReady.status = "awaiting_human"; + const codexDelivery = awaitingHumanDelivery(); + codexDelivery.reviewPolicy = { + mode: "github_codex_required", + requiredReviewerLogins: ["chatgpt-codex-connector[bot]"], + }; + codexDelivery.observation = { + headSha: candidateCommit, + branchSha: candidateCommit, + checks: [ + { + name: "validate", + status: "completed", + conclusion: "success", + headSha: candidateCommit, + }, + ], + reviews: [ + { + actorLogin: "chatgpt-codex-connector[bot]", + state: "APPROVED", + commitId: candidateCommit, + }, + { + actorLogin: "chatgpt-codex-connector[bot]", + state: "CODEX_COMPLETED", + commitId: candidateCommit, + }, + ], + feedback: [], + }; + codexReady.deliveryJson = JSON.stringify(codexDelivery); + expect(outcome(codexReady).integrity.status).toBe("consistent"); + const completedCodexObservation = codexDelivery.observation as Record< + string, + unknown + >; + codexDelivery.observation = { + ...completedCodexObservation, + reviews: [ + { + actorLogin: "chatgpt-codex-connector[bot]", + state: "CODEX_RUNNING", + commitId: candidateCommit, + }, + ], + }; + codexReady.deliveryJson = JSON.stringify(codexDelivery); + expect(outcome(codexReady).integrity.reasons).toContainEqual( + expect.objectContaining({ code: "OUTCOME_DELIVERY_RECEIPT_MISMATCH" }), + ); + delivery.observation = { headSha: candidateCommit, branchSha: candidateCommit, diff --git a/test/schemas.test.ts b/test/schemas.test.ts index 6c47aa5..3b0e778 100644 --- a/test/schemas.test.ts +++ b/test/schemas.test.ts @@ -1142,6 +1142,34 @@ describe("compact schemas", () => { expect( contractSchemas.millConfig.safeParse(emptyRequiredReview).success, ).toBe(false); + const codexReview = { + ...localReview, + propose: { + ...localReview.propose, + reviewPolicy: { + mode: "github_codex_required", + requiredReviewerLogins: ["chatgpt-codex-connector[bot]"], + }, + }, + } as const; + expect(validate(codexReview)).toBe(true); + expect(contractSchemas.millConfig.safeParse(codexReview).success).toBe( + true, + ); + const emptyCodexReview = { + ...codexReview, + propose: { + ...codexReview.propose, + reviewPolicy: { + mode: "github_codex_required", + requiredReviewerLogins: [], + }, + }, + } as const; + expect(validate(emptyCodexReview)).toBe(false); + expect(contractSchemas.millConfig.safeParse(emptyCodexReview).success).toBe( + false, + ); const deliveryValidate = ajv.compile( JSON.parse( await readFile(