From 06e3c03b5a273fd89aa4e334676501711679e841 Mon Sep 17 00:00:00 2001 From: Oleg-Melnik Date: Wed, 5 Aug 2026 18:25:21 +0300 Subject: [PATCH 1/5] Update AI agent configuration. --- .agents/project.md | 7 +- .agents/skills/README.md | 7 +- .agents/skills/build-engineer/SKILL.md | 18 +- .agents/skills/code-reviewer/SKILL.md | 6 +- .agents/skills/codegen-engineer/SKILL.md | 16 +- .agents/skills/component-engineer/SKILL.md | 10 +- .agents/skills/engineer/SKILL.md | 7 +- .agents/skills/kotlin-engineer/SKILL.md | 59 +- .agents/skills/pair-workflow/SKILL.md | 139 +- .agents/skills/pair-workflow/template.md | 10 + .agents/skills/tester/SKILL.md | 37 +- .agents/workflows/gradle-root.sh | 114 ++ .agents/workflows/pair-test.sh | 1080 ++++++++++++++- .agents/workflows/pair.sh | 1400 +++++++++++++++++++- .claude/commands/pair.md | 23 +- .claude/settings.json | 8 + .gitignore | 3 + AGENTS.md | 71 +- PAIR_AGENTS_RUN_GUIDE.md | 513 +++---- 19 files changed, 3073 insertions(+), 455 deletions(-) create mode 100755 .agents/workflows/gradle-root.sh diff --git a/.agents/project.md b/.agents/project.md index 8d7d863b..4239b866 100644 --- a/.agents/project.md +++ b/.agents/project.md @@ -58,12 +58,15 @@ and GitHub Packages. - `codegen/plugins/`: **separate Gradle project** with ProtoData plugins that generate Kotlin extensions for Protobuf messages. Requires JDK 17, Gradle 9.4.x, and Kotlin 2.3.20, unlike the root project (JDK 11, - Gradle 6.9.4, Kotlin 1.8.20). Applied to consuming projects through the + Gradle 6.9.4, Kotlin Gradle plugin 1.8.22). It is applied to consuming + projects through the [Chords Gradle plugin](https://github.com/SpineEventEngine/Chords-Gradle-plugin). - `pom.xml`, `dependencies.md`: generated dependency/license reports; must be regenerated (not hand-edited) when the version or dependencies change. - `.github/workflows/`: CI for Ubuntu/Windows builds, license-report and version-increment guards, Gradle wrapper validation, and publishing. +- `.agents/workflows/`: local agent workflow drivers and their regression + suites. Gradle group: `io.spine.chords`. Artifact prefix: `spine-chords-`. Preserve package roots such as `io.spine.chords` and `io.spine.money` where already @@ -106,6 +109,8 @@ or Spine versions; such upgrades are dedicated tasks with wide impact. - `AGENTS.md`: repository operating policy for agents. - `.agents/project.md`: project map, architecture notes, documentation ownership, and CI notes. +- `PAIR_AGENTS_RUN_GUIDE.md`: operator guide for the paired-agent issue + workflow. - `core/README.md`: application shell, component model, and core components. - `proto/README.md`: Protobuf-aware components and message forms. - `proto-values/README.md`: supplementary Protobuf messages and extensions. diff --git a/.agents/skills/README.md b/.agents/skills/README.md index cc6a6845..fff2f77d 100644 --- a/.agents/skills/README.md +++ b/.agents/skills/README.md @@ -5,9 +5,10 @@ routing source of truth. - `engineer`: routing skill for mixed or unclear implementation work, and the home of the shared design-restraint policy. -- `kotlin-engineer`: the Kotlin language itself — the 1.8.20 ceiling, - null-safety, `lateinit`, coroutine scoping, and public-type rules. Pairs - with whichever area skill owns the code being changed. +- `kotlin-engineer`: the Kotlin language itself — the root/compiler/library + version split, the separate codegen-plugin toolchain, null-safety, + `lateinit`, coroutine scoping, and public-type rules. Pairs with whichever + area skill owns the code being changed. - `component-engineer`: class-based Compose UI components across `core`, `proto`, and `client` — the component model, input components, message forms, and server-connected components. diff --git a/.agents/skills/build-engineer/SKILL.md b/.agents/skills/build-engineer/SKILL.md index bfbb288a..e367c025 100644 --- a/.agents/skills/build-engineer/SKILL.md +++ b/.agents/skills/build-engineer/SKILL.md @@ -29,9 +29,10 @@ For workflow YAML under `.github/workflows`, use ## Policy -- Two toolchains coexist: the root project uses JDK 11 / Gradle 6.9.4 / - Kotlin 1.8.20; `codegen/plugins` uses JDK 17 / Gradle 9.4.x / Kotlin 2.3.20. - Keep build logic compatible with the owning toolchain. +- Two toolchains coexist: the root project uses JDK 11 / Gradle 6.9.4 and the + compiler/library version split documented in `kotlin-engineer`; + `codegen/plugins` uses JDK 17 / Gradle 9.4.x / Kotlin 2.3.20. Keep build + logic compatible with the owning toolchain. - Dependency coordinates belong in `buildSrc/src/main/kotlin/io/spine/internal/dependency/`, one object per library, following the existing pattern. Do not inline version strings into @@ -41,8 +42,9 @@ For workflow YAML under `.github/workflows`, use - Every PR increments `chordsVersion` in `version.gradle.kts` (`2.0.0-SNAPSHOT.`); CI enforces this with `checkVersionIncrement`. - `pom.xml` and `dependencies.md` are generated by the build (PomGenerator and - LicenseReporter); regenerate them via `./gradlew build` rather than editing, - and include the regenerated files in version-bump changesets. + LicenseReporter); regenerate them with the focused command in `AGENTS.md`, + "Versioning and Reports", rather than editing, and include the regenerated + files in version-bump changesets. - The `config/` submodule is owned by `SpineEventEngine/config`; do not edit its contents here. If shared build logic must change, describe the upstream change instead. @@ -60,9 +62,9 @@ For workflow YAML under `.github/workflows`, use Root build (repository root, JDK 11): ```bash -./gradlew clean build -./gradlew publishToMavenLocal -./gradlew checkVersionIncrement +.agents/workflows/gradle-root.sh clean build +.agents/workflows/gradle-root.sh publishToMavenLocal +.agents/workflows/gradle-root.sh checkVersionIncrement ``` Codegen plugins build (from `codegen/plugins/`, JDK 17): diff --git a/.agents/skills/code-reviewer/SKILL.md b/.agents/skills/code-reviewer/SKILL.md index a50d9d12..7e756542 100644 --- a/.agents/skills/code-reviewer/SKILL.md +++ b/.agents/skills/code-reviewer/SKILL.md @@ -86,6 +86,8 @@ duplicate its steps. get wrong, including which toolchain ceiling governs the file and the sanctioned exceptions to otherwise-standard bans. - Missing or weak tests for changed logic, extensions, or codegen behavior. + For whether an added suite follows local conventions, apply + `.agents/skills/kotlin-jvm-tester/SKILL.md`. - Version-policy misses: `chordsVersion` not incremented, or `pom.xml` / `dependencies.md` not regenerated when required. - Module-ownership violations, leaked state, unjustified reflection, and @@ -110,7 +112,9 @@ duplicate its steps. security-specific analysis. - Documentation and comment findings go to `.agents/skills/docs-reviewer/SKILL.md`. -- Test design or coverage authoring goes to `.agents/skills/tester/SKILL.md`. +- Test design or coverage authoring goes to `.agents/skills/tester/SKILL.md`; + test-suite conventions go to + `.agents/skills/kotlin-jvm-tester/SKILL.md`. ## Skip diff --git a/.agents/skills/codegen-engineer/SKILL.md b/.agents/skills/codegen-engineer/SKILL.md index 0d0269a6..47e9686e 100644 --- a/.agents/skills/codegen-engineer/SKILL.md +++ b/.agents/skills/codegen-engineer/SKILL.md @@ -29,10 +29,10 @@ For components that merely consume generated metadata, prefer ## Policy -- `codegen/plugins` targets JDK 17, Gradle 9.4.x, and Kotlin 2.3.20; the rest - of the repository targets JDK 11, Gradle 6.9.4, and Kotlin 1.8.20. Never mix - the two toolchains in one command or assume APIs from one are available in - the other. +- `codegen/plugins` targets JDK 17, Gradle 9.4.x, and Kotlin 2.3.20; the root + build targets JDK 11 and Gradle 6.9.4, with the Kotlin version split described + in `.agents/skills/kotlin-engineer/SKILL.md`. Never mix the two toolchains in + one command or assume APIs from one are available in the other. - The generated-code contract is consumed by `proto` and `client` and by external projects: changes to `MessageField`/`MessageOneof`/`MessageDef` shapes are public API changes on both the generator and runtime sides and @@ -62,10 +62,10 @@ Codegen plugin changes (from `codegen/plugins/`, JDK 17): Runtime and end-to-end verification (from the repository root, JDK 11): ```bash -./gradlew :runtime:test -./gradlew :codegen-tests:test -./gradlew :proto-values:test -./gradlew clean build +.agents/workflows/gradle-root.sh :runtime:test +.agents/workflows/gradle-root.sh :codegen-tests:test +.agents/workflows/gradle-root.sh :proto-values:test +.agents/workflows/gradle-root.sh clean build ``` The root build republishes codegen plugins to Maven local automatically before diff --git a/.agents/skills/component-engineer/SKILL.md b/.agents/skills/component-engineer/SKILL.md index 5f6cdf41..07ff6101 100644 --- a/.agents/skills/component-engineer/SKILL.md +++ b/.agents/skills/component-engineer/SKILL.md @@ -46,8 +46,8 @@ build logic, use `.agents/skills/build-engineer/SKILL.md`. Compose APIs in use are experimental (`@OptIn(ExperimentalComposeUiApi::class)`); keep such opt-ins localized and documented. -- For the Kotlin language itself — the 1.8.20 ceiling, explicit API mode, - null-safety, `lateinit` in `Props`, and coroutine scoping — follow +- For the Kotlin language itself — the root compiler/library split, explicit + API mode, null-safety, `lateinit` in `Props`, and coroutine scoping — follow `.agents/skills/kotlin-engineer/SKILL.md`. - Match existing KDoc style: `@param` tags for type parameters and constructor-like parameters, backticked identifiers, and wrapped lines @@ -74,9 +74,9 @@ build logic, use `.agents/skills/build-engineer/SKILL.md`. Run the narrowest relevant command first (repository root, JDK 11): ```bash -./gradlew ::test -./gradlew ::check -./gradlew clean build +.agents/workflows/gradle-root.sh ::test +.agents/workflows/gradle-root.sh ::check +.agents/workflows/gradle-root.sh clean build ``` UI rendering and interaction cannot be covered by automated tests here. For diff --git a/.agents/skills/engineer/SKILL.md b/.agents/skills/engineer/SKILL.md index ff37565d..a47dd0a9 100644 --- a/.agents/skills/engineer/SKILL.md +++ b/.agents/skills/engineer/SKILL.md @@ -11,9 +11,10 @@ description: > # Engineering Router `.agents/skills/kotlin-engineer/SKILL.md` applies to *all* of the areas -below — it owns the Kotlin language baseline (the pinned 1.8.20 ceiling, -null-safety, coroutine scoping, public types under explicit API mode). Pair -it with the area skill that owns the code being changed: +below — it owns the Kotlin language baseline (the root compiler/library split, +the separate codegen-plugin toolchain, null-safety, coroutine scoping, and +public types under explicit API mode). Pair it with the area skill that owns +the code being changed: - `.agents/skills/component-engineer/SKILL.md` for class-based Compose UI components in `core`, `proto`, and `client`: the component model, diff --git a/.agents/skills/kotlin-engineer/SKILL.md b/.agents/skills/kotlin-engineer/SKILL.md index 593ffda6..f4c49a85 100644 --- a/.agents/skills/kotlin-engineer/SKILL.md +++ b/.agents/skills/kotlin-engineer/SKILL.md @@ -2,9 +2,10 @@ name: kotlin-engineer description: > Chords Kotlin implementation policy and the pitfalls that recur in review: - the pinned 1.8.20 language ceiling, null-safety and `!!`, `lateinit` in - `Props`, coroutine scoping and cancellation in `client`, and read-only - public types under explicit API mode. Use whenever writing, changing, + the root compiler/library version split and separate codegen-plugin + toolchain, null-safety and `!!`, `lateinit` in `Props`, coroutine scoping + and cancellation in `client`, and read-only public types under explicit API + mode. Use whenever writing, changing, refactoring, or reviewing Kotlin in any module: `.kt`/`.kts` edits, turning Java-style Kotlin idiomatic, anything touching coroutines, cancellation, or `Flow` (concentrated in `client`), and designing a public @@ -42,19 +43,21 @@ Each of these owns its area; this skill stays out of them: ## Toolchain Ceiling **Two ceilings, and which one applies depends on the file you are editing.** -The root build compiles with **Kotlin 1.8.20** — the version of the -`kotlin("jvm")` plugin declared in `buildSrc/build.gradle.kts` — on JVM -target 11. It covers every module in +The root build compiles with the Kotlin Gradle plugin dependency declared as +`kotlinVersion` in `buildSrc/build.gradle.kts` — currently **1.8.22** — on JVM +target 11. The `kotlin("jvm")` declaration earlier in that file is the plugin +used to compile `buildSrc` itself, not the version applied to root modules. The +root toolchain covers every module in `settings.gradle.kts`: `core`, `proto`, `proto-values`, `client`, `runtime` (at `codegen/runtime`), and `codegen-tests` (at `codegen/tests`) — note that both `codegen/` subprojects belong to the *root* build. Only `codegen/plugins` is separate, using **Kotlin 2.3.20** on JDK 17 (its own `kotlinVersion` in `codegen/plugins/buildSrc/build.gradle.kts`). -Everything below about the 1.8.20 ceiling applies to the root build; in -`codegen/plugins` the 2.x language is available. Never carry a construct -from one across to the other because it compiled where you first wrote it. -Both builds enable explicit API mode. +Everything below about the Kotlin 1.8 language ceiling applies to the root +build; in `codegen/plugins` the 2.x language is available. Never carry a +construct from one across to the other because it compiled where you first +wrote it. Both builds enable explicit API mode. Within the root modules: @@ -62,19 +65,30 @@ Within the root modules: `forceProductionDependencies()` in `buildSrc/src/main/kotlin/DependencyResolution.kt` pins `kotlin-stdlib` to the `Kotlin.version` coordinate — currently 1.9.23 — while the compiler - stays at 1.8.20. A 1.9 stdlib *function* can therefore resolve and compile, + stays at 1.8.22. A 1.9 stdlib *function* can therefore resolve and compile, even though a 1.9 *language feature* cannot. **"It compiles" is not evidence that a construct is within the baseline** — check when the API - was introduced, and prefer one that predates 1.8.20. -- Not available at 1.8.20: `data object`, `enumEntries`, the stable `..<` + was introduced, and prefer one that predates Kotlin 1.8.20. +- Not available under the root's 1.8 language ceiling: `data object`, + `enumEntries`, the stable `..<` operator (use `until`), and stable context receivers. - Available and preferred where they fit: sealed interfaces, `@JvmInline value class`, `buildList` / `buildMap`, and `kotlin.time.Duration`. +- **Context receivers, not context parameters.** The root build passes + `-Xcontext-receivers` from `KotlinConfig.setFreeCompilerArgs()`. That enables + the experimental context-receiver syntax under Kotlin 1.8; it does not + enable the later context-parameter syntax. `codegen/plugins` enables + neither feature. - Coroutines are **1.7.3** (`KotlinX.Coroutines.version`), forced across every configuration by the `resolutionStrategy` block in the root `build.gradle.kts`. 1.7 APIs are available. Ignore the unused `Coroutines` object in the same dependency package — nothing imports it, and its version is not what resolves. +- **`failOnVersionConflict()` is enabled.** Adding a dependency that brings a + different version of an already-forced library fails resolution rather than + silently choosing one. Adjust the coordinate in + `buildSrc/src/main/kotlin/io/spine/internal/dependency/` and the force list + when the conflict is real; do not work around it in a module build file. - **Explicit API mode is on in both builds** — each calls `explicitApi()` in its Kotlin block. The compiler enforces exactly two things: an explicit visibility modifier and an explicit return type on @@ -124,9 +138,9 @@ Within the root modules: - **Confine `runBlocking` to a bridge** from a non-suspend API into suspend code. Inside a `suspend` function it is always a bug. - **Expose read-only types from public API** — `List` over `MutableList`, - `StateFlow` over `MutableStateFlow`, and never the mutable backing - property itself. Explicit API mode makes each of these a published - contract. + `StateFlow` over `MutableStateFlow`, `State` over `MutableState`, and never + the mutable backing property itself. Explicit API mode makes each of these a + published contract. - **Immutability first**: `val` over `var`, and `copy()` on a data class rather than mutation. - **Named arguments once a Kotlin call takes three or more parameters**, @@ -164,10 +178,13 @@ Within the root modules: - **No platform-type leak in public API.** A value crossing from Java arrives as `String!`; give the public declaration an explicit nullable or non-null type rather than letting the platform type propagate. -- **No language feature newer than 1.8.20**, and no stdlib API added after - it without a deliberate decision — see "Toolchain Ceiling". +- **No language feature newer than Kotlin 1.8**, and no stdlib API added after + 1.8.20 without a deliberate decision — see "Toolchain Ceiling". - **No new deprecated-API call** without that explicit instruction; use the replacement named in the `@Deprecated` or `ReplaceWith` message. +- **No blanket Detekt suppression.** Suppress the narrowest rule at the + narrowest declaration, matching existing style, and only when that rule is + genuinely wrong about the site. ## Verification @@ -175,9 +192,9 @@ Compile the narrowest module first; the full command set and the JDK constraints live in `AGENTS.md`, "Verification and Quality". ```bash -./gradlew ::compileKotlin -./gradlew ::test -./gradlew detekt +.agents/workflows/gradle-root.sh ::compileKotlin +.agents/workflows/gradle-root.sh ::test +.agents/workflows/gradle-root.sh detekt ``` Detekt runs over these modules — do not introduce new violations, and keep diff --git a/.agents/skills/pair-workflow/SKILL.md b/.agents/skills/pair-workflow/SKILL.md index be1b2195..744ab2fe 100644 --- a/.agents/skills/pair-workflow/SKILL.md +++ b/.agents/skills/pair-workflow/SKILL.md @@ -129,6 +129,14 @@ that case is content, not history — bump `chordsVersion`, regenerate `pom.xml` and `dependencies.md`, and write `## Pull Request` — and the ref and index comparison still runs after your turn. +The branch the run started from is not your concern either way. The driver +creates or continues the task branch at the exact `start_commit` recorded at +setup and targets the immutable `pr_base_branch` (`master` by default). When +that commit carries history outside the target, `base_branch` provides its +human-readable label and the driver adds a `## Reviewer notes` paragraph with +the exact commit boundary. Do not write a second stacking note in +`## Pull Request`. + ## Working Document One file per task at `.agents/work//plan.md`, created from @@ -141,6 +149,13 @@ write down; the transcripts record what it actually did. Read them when a turn produces a surprising result — an agent's own account of its work is not evidence. +`.agents/work//rounds/` holds what each review round was handed. The +driver writes `plan-.md` (the plan text) and `impl-.patch` (the whole +changeset, untracked files included) at the handoff to `agent2`, because +nothing else preserves them: the plan is revised in place and the +implementation is never committed. A round after the first is told where the +previous round's file is, and reviews the delta against it. + ### Frontmatter Frontmatter is the machine-readable state. It is the only place a transition @@ -155,13 +170,23 @@ impl_round: 1 max_rounds: 2 dirty_at_start: no resume_status: none +question_origin: none manual_testing: unknown agent1: claude agent2: codex +claude_model: claude-opus-5 +claude_effort: high +codex_model: gpt-5.6-sol +codex_effort: high issue: https://github.com/SpineEventEngine/Chords/issues/123 issue_number: 123 issue_title: Add keyboard-accessible copy action base_commit: 825c14b +base_branch: master +start_commit: ebd7c8413f01c19e8dc12d48f150e79c50a94c78 +pr_base_branch: master +changeset_digest: none +reviewed_changeset_digest: none updated: 2026-07-31T14:20:00Z ``` @@ -177,16 +202,48 @@ updated: 2026-07-31T14:20:00Z "Termination". - `issue`, `issue_number`, `issue_title` — the GitHub issue snapshot this task came from. Written by the driver at `start`; neither agent changes them. +- `agent1`, `agent2` — the selected command executables. Written by the driver + at `start` and immutable so a resumed task cannot silently change roles. +- `claude_model`, `claude_effort`, `codex_model`, `codex_effort` — the + engine-specific settings selected at setup. Written by the driver and + immutable so a resumed task uses the same models regardless of role swaps. + `(default)` means a direct CLI command omitted that setting; `(custom)` means + an opaque wrapper may hide the engine from the driver; `(unconfigured)` means + both commands identify another engine directly, so this engine is not used. - `dirty_at_start` — whether the caller explicitly included existing worktree changes. Written by the driver and immutable; a dirty start cannot publish. - `resume_status` — where `agent1` continues after its questions are answered. - Set alongside `questions-pending`, and `none` at all other times. + Set alongside `questions-pending`, and `none` at all other times. It must be + the status the asking turn started from. The driver records that status in + its immutable `question_origin` field and requires the two to match when the + answers return, so editing `resume_status` cannot skip a phase. +- `question_origin` — the asking status recorded by the driver. Agents never + edit it; the driver clears it after resuming. - `manual_testing` — `unknown` until `agent1`'s final turn, then `required` or `none`. The driver reads it to decide whether to print `## Manual Testing` when the run finishes. -- `base_commit` — commit the work started from. The driver writes it at setup - and neither agent changes it. `agent2` scopes the implementation review to - `git diff ...HEAD`. +- `base_commit` — the prospective pull request baseline: the merge-base of + `HEAD` with the remote-tracking `pr_base_branch`. It is written by the driver + at setup and changed by neither agent. Commits between `base_commit` and + `start_commit` are inherited branch history, not this task's review scope. +- `base_branch` — branch `HEAD` was on at setup, or the abbreviated commit if it + was detached. Written by the driver and immutable. It labels the starting + point in a stacked pull request even if that branch is later moved, renamed, + or deleted. Neither agent changes it. +- `start_commit` — the exact `HEAD` at setup. Written by the driver and + immutable. Publication creates or continues the task branch at this commit, + uses it as `agent2`'s implementation-review baseline, separates the task's + version bump from an inherited one, and names it as the boundary between + earlier and task commits in a stacked pull request. +- `pr_base_branch` — the pull request target selected at setup (`master` by + default). Written by the driver and immutable so resuming in another shell + cannot silently retarget the reviewed changeset. +- `changeset_digest` — `none` until the run reaches `done`, then a digest of + the reviewed changeset, written by the driver. Publication compares it, so + edits made after the review cannot be swept into a pull request. +- `reviewed_changeset_digest` — the content, type, and mode digest saved when + the implementation is handed to `agent2`. Agent 1 cannot finish if the + implementation differs from this digest after handling the review. - `updated` — UTC timestamp of the turn that just finished. Rewrite the whole frontmatter block on every turn. Update only the fields your @@ -230,14 +287,20 @@ Sections are owned. Write only your own; never edit, reword, delete, or renumber another agent's text. Add sections for a new round rather than overwriting the previous round's. +This is enforced, not merely asked for: the driver opens only the sections the +current state needs and hashes every other section. That protects the other +role's text and the active role's completed prior rounds. `## Log` is the +single shared section; every turn appends exactly one line, and earlier entries +remain verbatim. + | Section | Owner | |--------------------------------------|----------| | `## Issue` | driver — neither agent edits it | | `## Task` | `agent1` | | `## Questions` | `agent1` asks, the user answers | | `## Plan` | `agent1` | -| `## Plan Review` | `agent2` | -| `## Plan Dispositions` | `agent1` | +| `## Plan Review — Round N` | `agent2` | +| `## Plan Dispositions — Round N` | `agent1` | | `## Implementation — Round N` | `agent1` | | `## Implementation Review — Round N` | `agent2` | | `## Implementation Dispositions — Round N` | `agent1` | @@ -335,7 +398,9 @@ still `blocked`; a default cannot substitute for a specification. `Disposition` is `Accepted`, `Rejected`, or `Deferred`. `Rejected` and `Deferred` require a reason in `Notes`; `Accepted` requires how the plan changed. -2. Revise `## Plan` in place to reflect accepted findings. +2. Revise `## Plan` in place to reflect accepted findings. The previous + version is not lost — the driver saved it as `rounds/plan-.md` at the + handoff, which is what the next round compares against. 3. Decide whether the plan needs another look, on the same rule the implementation phase uses: - Verdict `APPROVE` or `APPROVE WITH CHANGES`, or every Must-fix finding @@ -364,9 +429,12 @@ still `blocked`; a default cannot substitute for a specification. completeness rule. 2. Apply accepted findings and re-run the verification that covers them. 3. Decide the next state: - - Verdict `APPROVE` or `APPROVE WITH CHANGES`, or all Must-fix findings - dispositioned `Accepted` and applied → write `## Outcome` and set + - Verdict `APPROVE` or `APPROVE WITH CHANGES`, no implementation change + after review, and all findings dispositioned → write `## Outcome` and set `status: done`, `turn: human`. + - Any accepted finding changes a source file, file type, or executable bit → + increment `impl_round`, set `status: implementation-review-requested`, + `turn: agent2`, and document the change in the new implementation round. - Verdict `REQUEST CHANGES` and `impl_round` < `max_rounds` → increment `impl_round`, set `status: implementation-review-requested`, `turn: agent2`. @@ -414,6 +482,11 @@ Read `AGENTS.md` and the relevant area skill. Review is read-only with respect to the codebase: change no source file, run no verification unless the document asks for it, and write only your own sections of the working document. +The driver fingerprints tracked and untracked file content around your turn and +aborts the run if any of it moved. Fixing what you found is not your turn to +take — a reviewer that edits the implementation has ended the independence the +second opinion exists for. Write the finding; `agent1` applies it. + Review independently. The plan's reasoning is one input, not a conclusion to ratify — you are in this loop because you are a different model. @@ -427,8 +500,10 @@ reserve it for a plan that would build the wrong thing — not for one you would have written differently. On `plan_round` > 1, review the revision and the dispositions for the previous -round. Do not re-raise a finding that was dispositioned `Rejected` unless you -have new evidence, and say what it is. +round. The plan as that round saw it is in `rounds/plan-.md`, and the +prompt names the file; diff against it rather than guessing what moved. Do not +re-raise a finding that was dispositioned `Rejected` unless you have new +evidence, and say what it is. Assess whether the plan solves the stated task, fixes the root cause rather than masking it, respects module ownership and the toolchain constraints, and @@ -442,13 +517,20 @@ findings. ### From `implementation-review-requested` -1. Scope the diff: `git diff ...HEAD` plus uncommitted changes - (`git diff HEAD`, `git ls-files --others --exclude-standard`). The plan - document is context, not the review target. +1. Scope the diff from `start_commit`: `git diff ...HEAD` plus + uncommitted changes (`git diff HEAD`, + `git ls-files --others --exclude-standard`). Commits between `base_commit` + and `start_commit` belong to the branch this run inherited. Read them only + as context; do not raise findings on them or ask `agent1` to edit them. For + a legacy, non-publishing document without `start_commit`, use `HEAD` as the + baseline because agent work remains uncommitted. The plan document is + context, not the review target. 2. On `impl_round` > 1, review only the delta since the previous round and the - dispositions for it. Do not re-raise a finding the previous round - dispositioned `Rejected` unless you have new evidence — say what the new - evidence is. + dispositions for it. The previous round's changeset is in + `rounds/impl-.patch` and the prompt names it; the delta is the + difference between that patch and the diff above. Do not re-raise a finding + the previous round dispositioned `Rejected` unless you have new evidence — + say what the new evidence is. 3. Apply `.agents/skills/code-reviewer/SKILL.md`: its Review Procedure, Review Focus, Skip list, and Output Format, ending with a verdict of `APPROVE`, `APPROVE WITH CHANGES`, or `REQUEST CHANGES`. @@ -490,6 +572,17 @@ had enough. Guards: agent. - Each turn must change `status`. The driver aborts if a turn returns the document unchanged, which means the agent failed rather than advanced. +- A review turn must leave a review: the round's exact section, non-empty, with + one of the three verdicts as its final non-blank line. A round cannot advance + on the previous round's findings or a verdict word mentioned in prose. +- `agent1` cannot leave a phase with findings unanswered. Every finding ID in + the round's review needs exactly one row in that round's dispositions table, + using `Accepted`, `Rejected`, or `Deferred` with a non-empty note. A + `REQUEST CHANGES` verdict closes the forward move outright — from there the + only ways on are another round, `blocked`, or a question. A disposition + cannot overrule a verdict. +- `done` needs a non-empty `## Outcome`. It is the claim that the task is + finished, and it is what the user reads first. - Set `blocked` rather than guessing whenever a decision is the user's: contradictory review findings, an ambiguous requirement, or a workaround that `AGENTS.md` requires confirmation for. @@ -526,6 +619,12 @@ either: cannot build hands off an implementation it never compiled, and the reviewer spends its round saying so instead of reading the code. -Swap roles or narrow what an agent may do by exporting `AGENT1_CMD` and -`AGENT2_CMD`; `pair.sh` with no arguments prints both defaults. Keep the flags -above when you do. The Git tripwire remains active in every mode. +Pass `--swap-agents` or `--sa` to exchange `agent1` and `agent2`. Repeat the +option when resuming the task; the driver checks the choice against the agent +names recorded at start. Select engine settings with `--claude-model`, +`--claude-effort`, `--codex-model`, and `--codex-effort`; the driver records +them at setup, restores them on resume, and keeps them attached to their engine +when roles are swapped. To change a complete command instead, export +`AGENT1_CMD` or `AGENT2_CMD`; `pair.sh` with no arguments prints both defaults. +Keep the flags above when you do. The Git tripwire remains active in every +mode. diff --git a/.agents/skills/pair-workflow/template.md b/.agents/skills/pair-workflow/template.md index d7ff8b00..67cdf222 100644 --- a/.agents/skills/pair-workflow/template.md +++ b/.agents/skills/pair-workflow/template.md @@ -6,13 +6,23 @@ impl_round: 1 max_rounds: 2 dirty_at_start: no resume_status: none +question_origin: none manual_testing: unknown agent1: AGENT1_NAME agent2: AGENT2_NAME +claude_model: CLAUDE_MODEL +claude_effort: CLAUDE_EFFORT +codex_model: CODEX_MODEL +codex_effort: CODEX_EFFORT issue: ISSUE_URL issue_number: ISSUE_NUMBER issue_title: ISSUE_TITLE base_commit: BASE_COMMIT +base_branch: BASE_BRANCH +start_commit: START_COMMIT +pr_base_branch: PR_BASE_BRANCH +changeset_digest: none +reviewed_changeset_digest: none updated: CREATED_AT --- diff --git a/.agents/skills/tester/SKILL.md b/.agents/skills/tester/SKILL.md index eda091f6..4c62d853 100644 --- a/.agents/skills/tester/SKILL.md +++ b/.agents/skills/tester/SKILL.md @@ -40,29 +40,38 @@ description: > Run the smallest useful command while iterating (repository root, JDK 11): ```bash -./gradlew ::test -./gradlew ::test --tests "io.spine.chords.proto.money.MoneyFieldSpec" -./gradlew :codegen-tests:test -./gradlew clean build +.agents/workflows/gradle-root.sh ::test +.agents/workflows/gradle-root.sh ::test \ + --tests "io.spine.chords.proto.money.MoneyFieldSpec" +.agents/workflows/gradle-root.sh :codegen-tests:test +.agents/workflows/gradle-root.sh clean build ``` Module Gradle paths: `core`, `proto`, `proto-values`, `client`, `runtime`, `codegen-tests`. The `codegen/plugins` project verifies separately from `codegen/plugins/` with JDK 17 (`./gradlew build`). -Read the task list, not only the final line. A build whose compile and test -tasks all report `UP-TO-DATE` finished in seconds without compiling or running -anything, and its `BUILD SUCCESSFUL` describes a previous build rather than the -change in the worktree. When that happens, force the work with `--rerun-tasks` -before reporting a result: +Read the task list, not only the final line. Gradle's up-to-date check is +content-based, so `UP-TO-DATE` normally means the task's inputs are unchanged +and its previous result still holds. Do not force a rerun merely to make the +task execute again. + +What `UP-TO-DATE` cannot prove is that the right build and tasks were selected. +Suspect the result, and only then rerun, when tasks that should cover a changed +input remain up to date. Check that root commands ran from the repository root, +that `codegen/plugins` commands ran from that separate build, and that the task +declares the input that changed. Fix the invocation before rerunning; a rerun of +the wrong build is still the wrong build. + +Where a rerun is genuinely required, the root build's Gradle 6.9.4 supports the +whole-graph flag: ```bash -./gradlew ::test --tests "…" --rerun-tasks +.agents/workflows/gradle-root.sh ::test --tests "…" --rerun-tasks ``` -Gradle's up-to-date check is content-based, so an `UP-TO-DATE` task is normally -sound. It is misleading only when it stands in as evidence for a change that -was never built. Report a verification result as green only when the tasks -covering the change actually executed. +Report a verification result as green only when the tasks covering the change +were evaluated in this worktree and the correct build, then either executed or +were legitimately up to date. Follow the git-history and safety policy in `AGENTS.md`. diff --git a/.agents/workflows/gradle-root.sh b/.agents/workflows/gradle-root.sh new file mode 100755 index 00000000..76d5bc97 --- /dev/null +++ b/.agents/workflows/gradle-root.sh @@ -0,0 +1,114 @@ +#!/usr/bin/env bash +# +# Runs a root Gradle command under the JDK the root build requires. +# +# AGENTS.md prescribes `JAVA_HOME="$(jenv prefix)" ./gradlew …` for the root +# project on Apple Silicon. No permission rule can express that command: the +# leading environment assignment and the command substitution mean an allow +# entry for `./gradlew …` never matches it, and `jenv shell 11` does not +# survive into the next tool call. An unattended agent therefore reaches review +# having compiled nothing. This wrapper is the prescribed command behind a +# single allowlistable path. +# +# .agents/workflows/gradle-root.sh :core:check +# .agents/workflows/gradle-root.sh :proto:test --tests "…" +# +# Codegen plugin commands are not this script's business: `codegen/plugins` is +# a separate build on JDK 17 and runs its own `./gradlew` directly. +# +# Environment: +# CHORDS_JDK11_HOME use this JDK instead of asking jEnv for one + +set -euo pipefail + +readonly REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" + +die() { printf 'gradle-root: %s\n' "$1" >&2; exit 1; } +info() { printf 'gradle-root: %s\n' "$1" >&2; } + +[[ $# -gt 0 ]] || die "usage: gradle-root.sh " + +# This path is wildcard-allowlisted for the unattended implementer. Validate +# every argument here so that wildcard cannot be used to reach publishing, +# deployment, signed packaging, an init script, or another executable Gradle +# input. Task abbreviations are intentionally rejected as well. +validate_arguments() { + local expect="" argument + for argument in "$@"; do + if [[ -n "$expect" ]]; then + case "$expect" in + tests) [[ -n "$argument" && "$argument" != -* ]] \ + || die "--tests needs a non-option test filter" ;; + exclude) [[ "$argument" == "applyCodegenPlugins" ]] \ + || die "only applyCodegenPlugins may be excluded" ;; + esac + expect="" + continue + fi + + case "$argument" in + clean|build|check|test|detekt|publishToMavenLocal|\ + publishCodegenPluginsToMavenLocal|generatePom|\ + mergeAllLicenseReports|checkVersionIncrement) ;; + --tests) expect=tests ;; + -x|--exclude-task) expect=exclude ;; + --exclude-task=applyCodegenPlugins|--rerun-tasks|--stacktrace|\ + --info|--warn|--quiet|--no-daemon|--continue) ;; + *) + local task_pattern + task_pattern='^(:[A-Za-z0-9_-]+)+:' + task_pattern+='(test|check|compileKotlin|compileTestKotlin)$' + [[ "$argument" =~ $task_pattern ]] \ + || die "argument '${argument}' is not permitted for "\ +"unattended root verification" + ;; + esac + done + [[ -z "$expect" ]] || die "option at the end of the command needs a value" +} + +validate_arguments "$@" + +# jEnv is a shell function in an interactive shell and does not exist in this +# one, so resolve the executable rather than relying on the function. +jenv_bin="" +for candidate in "$(command -v jenv 2>/dev/null || true)" \ + /opt/homebrew/bin/jenv /usr/local/bin/jenv "${HOME}/.jenv/bin/jenv"; do + [[ -n "$candidate" && -x "$candidate" ]] || continue + jenv_bin="$candidate" + break +done + +java_home="${CHORDS_JDK11_HOME:-}" +if [[ -z "$java_home" && -n "$jenv_bin" ]]; then + java_home="$("$jenv_bin" prefix 11 2>/dev/null || true)" +fi +[[ -n "$java_home" ]] || java_home="${JAVA_HOME:-}" +[[ -n "$java_home" ]] \ + || die "no JDK 11 found. Install one, register it with jEnv ('jenv add …'), "\ +"or set CHORDS_JDK11_HOME" +[[ -x "${java_home}/bin/java" ]] \ + || die "'${java_home}' has no bin/java; it is not a JDK home" + +# Verify rather than trust. AGENTS.md warns that JDK selection here silently +# picks a newer ARM JVM when the expected one is not registered, and the root +# build then fails in ways that look like a code problem. +settings="$("${java_home}/bin/java" -XshowSettings:properties -version 2>&1)" +property() { printf '%s\n' "$settings" | awk -F'= ' -v k="$1" '$0 ~ k { print $2; exit }'; } + +version="$(property 'java\.specification\.version')" +[[ "$version" == "11" ]] \ + || die "'${java_home}' reports Java ${version:-unknown}; the root build needs "\ +"JDK 11 (see the Apple Silicon section of AGENTS.md)" + +arch="$(property 'os\.arch')" +if [[ "$(uname -s)" == "Darwin" && "$arch" != "x86_64" ]]; then + die "'${java_home}' reports os.arch=${arch:-unknown}; Apple Silicon root "\ +"verification must use an x86_64 JDK 11" +elif [[ "$arch" != "x86_64" && "$arch" != "amd64" ]]; then + info "warning: this JDK reports os.arch=${arch}; the root build pins "\ +"platform-specific Protobuf and gRPC tooling that may have no macOS ARM build" +fi + +cd "$REPO_ROOT" +exec env JAVA_HOME="$java_home" ./gradlew "$@" diff --git a/.agents/workflows/pair-test.sh b/.agents/workflows/pair-test.sh index 3ea7220a..5c5e2a0b 100755 --- a/.agents/workflows/pair-test.sh +++ b/.agents/workflows/pair-test.sh @@ -36,8 +36,10 @@ sandbox() { cp "${SUITE_DIR}/../skills/pair-workflow/template.md" "${repo}/.agents/skills/pair-workflow/" printf 'protocol stub\n' > "${repo}/.agents/skills/pair-workflow/SKILL.md" printf 'val chordsVersion: String by extra("2.0.0-SNAPSHOT.1")\n' > "${repo}/version.gradle.kts" - printf 'pom\n' > "${repo}/pom.xml" - printf 'deps\n' > "${repo}/dependencies.md" + printf '\n 2.0.0-SNAPSHOT.1\n \n\n' \ + > "${repo}/pom.xml" + printf '# Dependencies of `o:r:2.0.0-SNAPSHOT.1`\n' > "${repo}/dependencies.md" + printf 'fixture\n' > "${repo}/README.md" printf '.agents/work/\n' > "${repo}/.gitignore" git -C "$repo" init -q -b master @@ -47,6 +49,7 @@ sandbox() { git -C "$repo" commit -qm init git init -q --bare "${SANDBOX}/origin.git" git -C "$repo" remote add origin "${SANDBOX}/origin.git" + git -C "$repo" push -qu origin master # Stub gh: issue metadata from files, and a recorded no-op for pr create. cat > "${SANDBOX}/bin/gh" <<'GH' @@ -75,6 +78,11 @@ case "$1 $2" in "pr view") [[ -n "${STUB_PR_EXISTS:-}" ]] && printf '%s\n' "$STUB_PR_EXISTS" || exit 1 ;; "pr create") echo "${STUB_PR_CREATE_FAILS:+pr create refused}" >&2 [[ -n "${STUB_PR_CREATE_FAILS:-}" ]] && exit 1 + # Record the invocation so tests can assert on the base branch + # and the body without a real GitHub call. + if [[ -n "${STUB_PR_RECORD:-}" ]]; then + printf '%s\n' "$@" > "$STUB_PR_RECORD" + fi printf 'https://github.com/o/r/pull/1\n' ;; *) exit 1 ;; esac @@ -83,6 +91,11 @@ GH cat > "${SANDBOX}/bin/stub-agent" <<'AGENT' #!/usr/bin/env bash set -uo pipefail +if [[ -n "${STUB_AGENT_ARGS_RECORD:-}" ]]; then + printf '%s' "${0##*/}" >> "$STUB_AGENT_ARGS_RECORD" + printf ' <%s>' "$@" >> "$STUB_AGENT_ARGS_RECORD" + printf '\n' >> "$STUB_AGENT_ARGS_RECORD" +fi doc="$(git rev-parse --show-toplevel)/.agents/work/${PAIR_SLUG}/plan.md" fm() { awk -v k="$1" ' @@ -101,15 +114,67 @@ setfm() { { print } ' "$doc" > "$doc.t" && mv "$doc.t" "$doc" } +root() { git rev-parse --show-toplevel; } +# Fills a section that the template already has, or adds one before ## Log, +# which is how a real agent adds a later round's section. +put_section() { + local head="## $1"; shift + local body; body="$(printf '%s\n' "$@")" + BODY="$body" awk -v head="$head" ' + $0 == head && !written { + print; print ""; print ENVIRON["BODY"]; written = 1; next + } + $0 == "## Log" && !written { + print head; print ""; print ENVIRON["BODY"]; print "" + print; written = 1; next + } + { print } + ' "$doc" > "$doc.t" && mv "$doc.t" "$doc" +} +review() { # review + local verdict="${STUB_VERDICT_LINE:-Verdict: ${STUB_VERDICT:-APPROVE}}" + local finding + case "${STUB_FINDING_LAYOUT:-bullet}" in + bullet) finding="- ${2}: something to fix." ;; + numbered) finding="1. ${2}: something to fix." ;; + heading) finding="#### ${2} — something to fix" ;; + table) finding="| ${2} | Must fix | Something to fix. |" ;; + *) exit 1 ;; + esac + if [[ -n "${STUB_PRIOR_REFERENCE:-}" && "$1" == *"Round 2" ]]; then + put_section "$1" "**Must fix**" "" "$finding" "" \ + "P1-99 was addressed in the previous round." "" "$verdict" + else + put_section "$1" "**Must fix**" "" "$finding" "" "$verdict" + fi +} +dispositions() { # dispositions + put_section "$1" "| ID | Disposition | Notes |" "|----|----|----|" \ + "| ${2} | Accepted | Applied. |" +} +# The log is append-only and shared, so every turn adds its own line. +log_line() { + local role=agent1 + case "$1" in + plan-review-requested|implementation-review-requested) role=agent2 ;; + esac + printf '%s %s %s -> %s: stub turn\n' \ + "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$role" "$1" "$2" >> "$doc" +} +if [[ "$(fm status)" == implementation-review-requested \ + && -n "${STUB_REVIEW_PROMPT_RECORD:-}" ]]; then + printf '%s\n' "${1:-}" > "$STUB_REVIEW_PROMPT_RECORD" +fi case "${STUB_MISBEHAVE:-}" in illegal-jump) setfm status done; setfm turn human; exit 0 ;; git-write) git branch "stub-rogue-$$" setfm status plan-review-requested; setfm turn agent2; exit 0 ;; stash) echo x >> version.gradle.kts; git stash -q setfm status plan-review-requested; setfm turn agent2; exit 0 ;; - restage) echo y >> pom.xml; git add pom.xml + restage) echo y >> README.md; git add README.md setfm status plan-review-requested; setfm turn agent2; exit 0 ;; rewrite-meta) setfm dirty_at_start yes ;; + rewrite-model) setfm claude_model tampered ;; round-jump) setfm plan_round 9; setfm status plan-review-requested; setfm turn agent2; exit 0 ;; loop-no-round) # plan loopback without spending a round if [[ "$(fm status)" == plan-reviewed ]]; then @@ -144,20 +209,140 @@ case "${STUB_MISBEHAVE:-}" in setfm status plan-review-requested; setfm turn agent2; exit 0 ;; loop-past-plan) if [[ "$(fm status)" == plan-reviewed ]]; then - setfm plan_round "$(( $(fm plan_round) + 1 ))" + r="$(fm plan_round)" + dispositions "Plan Dispositions — Round ${r}" "P${r}-01" + log_line plan-reviewed plan-review-requested + setfm plan_round "$(( r + 1 ))" setfm status plan-review-requested; setfm turn agent2; exit 0 fi ;; loop-past-impl) if [[ "$(fm status)" == implementation-reviewed ]]; then - setfm impl_round "$(( $(fm impl_round) + 1 ))" + r="$(fm impl_round)" + dispositions "Implementation Dispositions — Round ${r}" "I${r}-01" + log_line implementation-reviewed implementation-review-requested + setfm impl_round "$(( r + 1 ))" setfm status implementation-review-requested; setfm turn agent2; exit 0 fi ;; + # A legitimate send-back: dispositions written, one round spent, then the + # run carries on to `done` through a second review round. + loop-once|wrong-review-dash) + if [[ "$(fm status)" == plan-reviewed && "$(fm plan_round)" == 1 ]]; then + dispositions "Plan Dispositions — Round 1" "P1-01" + log_line plan-reviewed plan-review-requested + setfm plan_round 2 + setfm status plan-review-requested; setfm turn agent2; exit 0 + fi ;; + # agent2 edits the code it is reviewing. + agent2-edits) if [[ "$(fm status)" == *review-requested ]]; then + printf 'tampered\n' >> "$(root)/README.md" + fi ;; + # Retargeting a symlink is a content change even when both targets have the + # same bytes. + agent2-retarget-link) + if [[ "$(fm status)" == *review-requested ]]; then + ln -sfn link-target-b "$(root)/review-link" + fi ;; + # agent1 rewrites the reviewer's section. + edit-review) if [[ "$(fm status)" == plan-reviewed ]]; then + awk '/^## Plan Review/ { print; print "AGENT1 WAS HERE"; next } {print}' \ + "$doc" > "$doc.t" && mv "$doc.t" "$doc" + fi ;; + # An earlier log entry is rewritten rather than appended to. + rewrite-log) if [[ "$(fm status)" == plan-review-requested ]]; then + sed 's/stub turn/rewritten/' "$doc" > "$doc.t" && mv "$doc.t" "$doc" + fi ;; + extend-log) if [[ "$(fm status)" == plan-review-requested ]]; then + review "Plan Review — Round 1" "P1-01" + perl -0pi -e 's/\n\z//' "$doc" + printf ' extended\n' >> "$doc" + setfm status plan-reviewed; setfm turn agent1; exit 0 + fi ;; + omit-log) if [[ "$(fm status)" == plan-requested ]]; then + put_section Task "Implement the issue." "- [ ] It works." + setfm status plan-review-requested; setfm turn agent2; exit 0 + fi ;; + rewrite-prior-owned) + if [[ "$(fm status)" == implementation-reviewed ]]; then + sed 's/Applied\./Rewritten later./' "$doc" > "$doc.t" \ + && mv "$doc.t" "$doc" + r="$(fm impl_round)" + dispositions "Implementation Dispositions — Round ${r}" "I${r}-01" + put_section Outcome "The criterion is met." + log_line implementation-reviewed done + setfm status done; setfm turn human; setfm manual_testing none; exit 0 + fi ;; + tamper-snapshot) + if [[ "$(fm status)" == plan-review-requested ]]; then + printf 'tampered\n' >> "$(dirname "$doc")/rounds/plan-1.md" + review "Plan Review — Round 1" "P1-01" + log_line plan-review-requested plan-reviewed + setfm status plan-reviewed; setfm turn agent1; exit 0 + fi ;; + # A question asked while planning that would resume at a finished run. + bad-resume) awk '/^## Questions$/ { + print; print "" + print "**Q1.** Retry? Default if unanswered: no."; next + } {print}' \ + "$doc" > "$doc.t" && mv "$doc.t" "$doc" + setfm resume_status done + log_line "$(fm status)" questions-pending + setfm status questions-pending; setfm turn human; exit 0 ;; + verdict-in-prose) + if [[ "$(fm status)" == plan-review-requested ]]; then + put_section "Plan Review — Round 1" "The earlier example says APPROVE." + log_line plan-review-requested plan-reviewed + setfm status plan-reviewed; setfm turn agent1; exit 0 + fi ;; + no-verdict) if [[ "$(fm status)" == plan-review-requested ]]; then + put_section "Plan Review — Round 1" "**Must fix**" "" "None." + log_line plan-review-requested plan-reviewed + setfm status plan-reviewed; setfm turn agent1; exit 0 + fi ;; + empty-review) if [[ "$(fm status)" == plan-review-requested ]]; then + log_line plan-review-requested plan-reviewed + setfm status plan-reviewed; setfm turn agent1; exit 0 + fi ;; + no-dispositions) + if [[ "$(fm status)" == plan-reviewed ]]; then + log_line plan-reviewed implementation-review-requested + setfm status implementation-review-requested + setfm turn agent2; exit 0 + fi ;; + prose-disposition) + if [[ "$(fm status)" == plan-reviewed ]]; then + put_section "Plan Dispositions — Round 1" \ + "P1-01 is mentioned, but this is not a disposition row." + printf 'work\n' >> "$(root)/src.txt" + log_line plan-reviewed implementation-review-requested + setfm status implementation-review-requested + setfm turn agent2; exit 0 + fi ;; + duplicate-disposition) + if [[ "$(fm status)" == plan-reviewed ]]; then + put_section "Plan Dispositions — Round 1" \ + "| ID | Disposition | Notes |" \ + "|----|-------------|-------|" \ + "| P1-01 | Accepted | Applied. |" \ + "| P1-01 | Rejected | Duplicate. |" + printf 'work\n' >> "$(root)/src.txt" + log_line plan-reviewed implementation-review-requested + setfm status implementation-review-requested + setfm turn agent2; exit 0 + fi ;; + no-outcome) if [[ "$(fm status)" == implementation-reviewed ]]; then + r="$(fm impl_round)" + dispositions "Implementation Dispositions — Round ${r}" "I${r}-01" + log_line implementation-reviewed done + setfm status done; setfm turn human + setfm manual_testing none; exit 0 + fi ;; dup-q) awk '/^## Questions$/ { print; print ""; print "**Q1.** First?" print "**Q1.** Second?"; next } {print}' \ "$doc" > "$doc.t" && mv "$doc.t" "$doc" setfm resume_status "$(fm status)" + log_line "$(fm status)" questions-pending setfm status questions-pending; setfm turn human; exit 0 ;; ask) awk '/^## Questions$/ { print; print "" @@ -165,7 +350,28 @@ case "${STUB_MISBEHAVE:-}" in } {print}' \ "$doc" > "$doc.t" && mv "$doc.t" "$doc" setfm resume_status "$(fm status)" + log_line "$(fm status)" questions-pending setfm status questions-pending; setfm turn human; exit 0 ;; + final-edit) if [[ "$(fm status)" == implementation-reviewed ]]; then + r="$(fm impl_round)" + dispositions "Implementation Dispositions — Round ${r}" "I${r}-01" + put_section Outcome "The criterion is met." + printf 'unreviewed\n' >> "$(root)/src.txt" + log_line implementation-reviewed done + setfm status done; setfm turn human; setfm manual_testing none; exit 0 + fi ;; + fix-and-rereview) + if [[ "$(fm status)" == implementation-reviewed \ + && "$(fm impl_round)" == 1 ]]; then + dispositions "Implementation Dispositions — Round 1" "I1-01" + put_section "Implementation — Round 2" \ + "Applied I1-01 and re-ran focused verification." + printf 'review fix\n' >> "$(root)/src.txt" + log_line implementation-reviewed implementation-review-requested + setfm impl_round 2 + setfm status implementation-review-requested + setfm turn agent2; exit 0 + fi ;; nothing) exit 0 ;; esac case "$(fm status)" in @@ -173,26 +379,87 @@ case "$(fm status)" in awk '/^## Task$/ { print; print "Implement the issue."; print "- [ ] It works."; next } {print}' "$doc" > "$doc.t" && mv "$doc.t" "$doc" + log_line plan-requested plan-review-requested setfm status plan-review-requested; setfm turn agent2 ;; plan-review-requested) + r="$(fm plan_round)" + heading="Plan Review — Round ${r}" + if [[ "${STUB_MISBEHAVE:-}" == wrong-review-dash && "$r" == 2 ]]; then + heading="Plan Review - Round ${r}" + fi + review "$heading" "P${r}-01" + log_line plan-review-requested plan-reviewed setfm status plan-reviewed; setfm turn agent1 ;; plan-reviewed) - setfm status implementation-review-requested; setfm turn agent2 - printf 'work\n' >> "$(git rev-parse --show-toplevel)/src.txt" ;; + r="$(fm plan_round)" + dispositions "Plan Dispositions — Round ${r}" "P${r}-01" + printf 'work\n' >> "$(root)/src.txt" + # A requested PR's version belongs to the implementation handed to the + # reviewer, not the final disposition-only turn. + if [[ -n "${STUB_BUMP:-}" ]]; then + if [[ "$STUB_BUMP" == fake ]]; then + printf 'b\n' >> "$(root)/version.gradle.kts" + printf 'b\n' >> "$(root)/pom.xml" + printf 'b\n' >> "$(root)/dependencies.md" + else + printf 'val chordsVersion: String by extra("%s")\n' "$STUB_BUMP" \ + > "$(root)/version.gradle.kts" + printf '\n %s\n \n\n' \ + "$STUB_BUMP" > "$(root)/pom.xml" + printf '# Dependencies of `o:r:%s`\n' "$STUB_BUMP" \ + > "$(root)/dependencies.md" + case "${STUB_BAD_REPORT:-}" in + stale-pom) + printf 'stale pom\n' > "$(root)/pom.xml" ;; + misplaced-pom) + printf '%s\n' '' \ + ' 2.0.0-SNAPSHOT.1' \ + ' 2.0.0-SNAPSHOT.2' \ + ' ' '' > "$(root)/pom.xml" ;; + stale-dependencies) + printf '%s\n' \ + '# Dependencies of `o:r:2.0.0-SNAPSHOT.1`' \ + 'new version: 2.0.0-SNAPSHOT.2' \ + > "$(root)/dependencies.md" ;; + partial-dependencies) + printf '%s\n' \ + '# Dependencies of `o:r:2.0.0-SNAPSHOT.2`' \ + '# Dependencies of `o:s:2.0.0-SNAPSHOT.1`' \ + > "$(root)/dependencies.md" ;; + esac + fi + fi + log_line plan-reviewed implementation-review-requested + setfm status implementation-review-requested; setfm turn agent2 ;; implementation-review-requested) + r="$(fm impl_round)" + review "Implementation Review — Round ${r}" "I${r}-01" + log_line implementation-review-requested implementation-reviewed setfm status implementation-reviewed; setfm turn agent1 ;; implementation-reviewed) + r="$(fm impl_round)" + dispositions "Implementation Dispositions — Round ${r}" "I${r}-01" + put_section Outcome "The criterion is met. Verified by the stub." + log_line implementation-reviewed done setfm status done; setfm turn human setfm manual_testing "${STUB_MANUAL:-none}" ;; *) exit 1 ;; esac AGENT chmod +x "${SANDBOX}/bin/gh" "${SANDBOX}/bin/stub-agent" + ln -s stub-agent "${SANDBOX}/bin/stub-agent1" + ln -s stub-agent "${SANDBOX}/bin/stub-agent2" + ln -s stub-agent "${SANDBOX}/bin/claude" + ln -s stub-agent "${SANDBOX}/bin/codex" # Exported here, so sandbox must be called as a plain command: via $( ) the # subshell would swallow every one of these. export PATH="${SANDBOX}/bin:${ORIGINAL_PATH}" - export AGENT1_CMD="${SANDBOX}/bin/stub-agent" AGENT2_CMD="${SANDBOX}/bin/stub-agent" + export AGENT1_CMD="${SANDBOX}/bin/stub-agent1" + export AGENT2_CMD="${SANDBOX}/bin/stub-agent2" export PAIR_SLUG=issue-7 + export STUB_PR_RECORD="${SANDBOX}/pr-create.args" + export STUB_REVIEW_PROMPT_RECORD="${SANDBOX}/implementation-review.prompt" + export STUB_AGENT_ARGS_RECORD="${SANDBOX}/agent-args.log" R="$repo" } @@ -263,6 +530,108 @@ check "first turn writes the required Task" \ "$(grep -q 'Implement the issue' "$R/.agents/work/issue-7/plan.md" && echo 0 || echo 1)" cleanup +# --- agent selection ----------------------------------------------------- +sandbox +run "$R" start 7 --sa +want "short swap option starts a task" 0 +D="$R/.agents/work/issue-7/plan.md" +check "swapped task records agent2 as planner" \ + "$(grep -q "^agent1: ${SANDBOX}/bin/stub-agent2$" "$D" && echo 0 || echo 1)" +check "swapped task records agent1 as reviewer" \ + "$(grep -q "^agent2: ${SANDBOX}/bin/stub-agent1$" "$D" && echo 0 || echo 1)" +run "$R" step 7 --sa +want "short swap option advances one step" 0 +run "$R" 7 +want "swapped task rejects an unswapped resume" 1 "agent selection differs" +run "$R" 7 --sa +want "short swap option resumes the task" 0 "is done" +cleanup + +sandbox +run "$R" 7 --swap-agents +want "long swap option runs a task" 0 "is done" +cleanup + +# --- model and effort selection ------------------------------------------ +sandbox +export AGENT1_CMD="${SANDBOX}/bin/claude --model claude-opus-5 --effort high" +export AGENT2_CMD="${SANDBOX}/bin/codex exec -m gpt-5.6-sol "\ +"-c model_reasoning_effort=\"high\"" +run "$R" start 7 --claude-model default --claude-effort xhigh \ + --codex-model gpt-5.6-terra --codex-effort medium +want "model options start a task" 0 +D="$R/.agents/work/issue-7/plan.md" +check "Claude model selection is recorded" \ + "$(grep -qx 'claude_model: default' "$D" && echo 0 || echo 1)" +check "Claude effort selection is recorded" \ + "$(grep -qx 'claude_effort: xhigh' "$D" && echo 0 || echo 1)" +check "Codex model selection is recorded" \ + "$(grep -qx 'codex_model: gpt-5.6-terra' "$D" && echo 0 || echo 1)" +check "Codex effort selection is recorded" \ + "$(grep -qx 'codex_effort: medium' "$D" && echo 0 || echo 1)" +: > "$STUB_AGENT_ARGS_RECORD" +run "$R" step 7 +want "saved Claude settings resume without repeated options" 0 +check "saved Claude settings reach the CLI" \ + "$(grep -q '^claude .*<--model> .*<--effort> ' \ + "$STUB_AGENT_ARGS_RECORD" && echo 0 || echo 1)" +: > "$STUB_AGENT_ARGS_RECORD" +run "$R" step 7 +want "saved Codex settings resume without repeated options" 0 +check "saved Codex settings reach the CLI" \ + "$(grep -q '^codex .*<-m> .*' \ + "$STUB_AGENT_ARGS_RECORD" && echo 0 || echo 1)" +run "$R" step 7 --claude-model sonnet +want "a resumed task rejects a different model" 1 "claude-model differs" +cleanup + +sandbox +export AGENT1_CMD="${SANDBOX}/bin/claude" +export AGENT2_CMD="${SANDBOX}/bin/codex exec" +run "$R" start 7 --claude-model sonnet +want "direct engines start a model-configured task" 0 +export AGENT1_CMD="${SANDBOX}/bin/stub-agent1" +run "$R" 7 +want "a changed engine wrapper reports the agent selection mismatch" 1 \ + "agent selection differs" +cleanup + +sandbox +export AGENT1_CMD="${SANDBOX}/bin/claude" +export AGENT2_CMD="${SANDBOX}/bin/claude" +run "$R" start 7 +want "a task may use the same direct engine for both roles" 0 +D="$R/.agents/work/issue-7/plan.md" +check "an absent Codex engine is recorded as unconfigured" \ + "$(grep -qx 'codex_model: (unconfigured)' "$D" \ + && grep -qx 'codex_effort: (unconfigured)' "$D" && echo 0 || echo 1)" +run "$R" step 7 +want "unconfigured engine metadata is accepted on resume" 0 +cleanup + +sandbox +export AGENT1_CMD="${SANDBOX}/bin/claude --model claude-opus-5 --effort high" +export AGENT2_CMD="${SANDBOX}/bin/codex exec -m gpt-5.6-sol "\ +"-c model_reasoning_effort=\"high\"" +run "$R" start 7 --sa --claude-model 'opus[1m]' --codex-model gpt-5.6-luna +want "model settings combine with swapped agents" 0 +D="$R/.agents/work/issue-7/plan.md" +check "bracketed Claude model alias stays literal" \ + "$(grep -qxF 'claude_model: opus[1m]' "$D" && echo 0 || echo 1)" +: > "$STUB_AGENT_ARGS_RECORD" +run "$R" step 7 --sa +want "swapped task resumes its engine settings" 0 +check "Codex settings follow Codex into the planner role" \ + "$(grep -q '^codex .*<-m> ' "$STUB_AGENT_ARGS_RECORD" \ + && echo 0 || echo 1)" +: > "$STUB_AGENT_ARGS_RECORD" +run "$R" step 7 --sa +want "swapped Claude reviewer restores its model" 0 +check "bracketed Claude model reaches the CLI literally" \ + "$(grep -qF '<--model> ' "$STUB_AGENT_ARGS_RECORD" \ + && echo 0 || echo 1)" +cleanup + # --- exit codes ----------------------------------------------------------- sandbox; STUB_MANUAL=required run "$R" 7; want "manual testing exits 2" 2 cleanup @@ -289,6 +658,223 @@ want "restaged blob caught" 1 "Git state moved"; cleanup sandbox; STUB_MISBEHAVE=rewrite-meta run "$R" 7 want "driver-owned field edit caught" 1 "protected fields or sections" cleanup +sandbox; STUB_MISBEHAVE=rewrite-model run "$R" 7 +want "model metadata edit caught" 1 "protected fields or sections" +cleanup + +sandbox; run "$R" start 7 >/dev/null +D="$R/.agents/work/issue-7/plan.md" +sed 's/^claude_model: .*/claude_model: bad model/' "$D" > "$D.t" && mv "$D.t" "$D" +run "$R" step 7 +want "malformed saved model metadata is rejected" 1 "without spaces" +check "malformed saved model starts no agent turn" \ + "$([[ ! -d "$R/.agents/work/issue-7/turns" ]] && echo 0 || echo 1)" +cleanup + +# A legacy document may continue through agent turns. Its known historical +# target is backfilled, while unknown starting metadata is required only for +# publication. +sandbox; run "$R" start 7 >/dev/null +D="$R/.agents/work/issue-7/plan.md" +awk '!/^(base_branch|start_commit|pr_base_branch): / && + !/^(claude_model|claude_effort|codex_model|codex_effort): /' \ + "$D" > "$D.t" && mv "$D.t" "$D" +run "$R" step 7 +want "legacy working document may take another non-publishing turn" 0 +check "legacy document gets the historical master target" \ + "$(grep -qx 'pr_base_branch: master' "$D" && echo 0 || echo 1)" +check "legacy non-publishing turn writes a transcript" \ + "$([[ -f "$R/.agents/work/issue-7/turns/01-agent1.log" ]] && echo 0 || echo 1)" +cleanup + +sandbox; run "$R" start 7 >/dev/null +D="$R/.agents/work/issue-7/plan.md" +awk '!/^question_origin: / && + !/^(claude_model|claude_effort|codex_model|codex_effort): /' \ + "$D" > "$D.t" && mv "$D.t" "$D" +STUB_MISBEHAVE=ask run "$R" 7 +want "a legacy document can record a newly raised question" 3 "needs answers" +check "the driver records the origin after legacy metadata backfill" \ + "$(grep -qx 'question_origin: plan-requested' "$D" && echo 0 || echo 1)" +cleanup + +sandbox +STUB_MISBEHAVE=ask run "$R" 7 +D="$R/.agents/work/issue-7/plan.md" +awk '!/^question_origin: / && + !/^(claude_model|claude_effort|codex_model|codex_effort): /' \ + "$D" > "$D.t" && mv "$D.t" "$D" +awk '/^\*\*Q1\./ { print; print ""; print "**A1.** No retry."; next } { print }' \ + "$D" > "$D.t" && mv "$D.t" "$D" +run "$R" 7 +want "a legacy questions-pending document resumes from its saved status" 0 \ + "answers found" +cleanup + +sandbox +STUB_MISBEHAVE=ask run "$R" 7 +D="$R/.agents/work/issue-7/plan.md" +awk '!/^question_origin: /' "$D" > "$D.t" && mv "$D.t" "$D" +awk '/^\*\*Q1\./ { print; print ""; print "**A1.** No retry."; next } { print }' \ + "$D" > "$D.t" && mv "$D.t" "$D" +run "$R" 7 +want "missing question provenance in a current document is refused" 1 \ + "question_origin is missing from current-format" +cleanup + +sandbox; run "$R" start 7 >/dev/null +D="$R/.agents/work/issue-7/plan.md" +awk '!/^(base_branch|start_commit|pr_base_branch): / && + !/^(claude_model|claude_effort|codex_model|codex_effort): /' \ + "$D" > "$D.t" && mv "$D.t" "$D" +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 --cp +want "legacy publication metadata is refused before an agent turn" 1 \ + "predates metadata required by --create-pr" +check "legacy publication refusal writes no transcript" \ + "$([[ ! -d "$R/.agents/work/issue-7/turns" ]] && echo 0 || echo 1)" +cleanup + +sandbox; run "$R" start 7 >/dev/null +D="$R/.agents/work/issue-7/plan.md" +awk '!/^(claude_model|claude_effort|codex_model|codex_effort): /' \ + "$D" > "$D.t" && mv "$D.t" "$D" +run "$R" step 7 --claude-model opus +want "legacy model metadata rejects new model options" 1 "predates model options" +check "legacy model refusal writes no transcript" \ + "$([[ ! -d "$R/.agents/work/issue-7/turns" ]] && echo 0 || echo 1)" +cleanup + +# --- reviewer stays out of the worktree (RR3-01) -------------------------- +sandbox; STUB_MISBEHAVE=agent2-edits run "$R" 7 +want "agent2 editing a source file caught" 1 "changed the code it was reviewing" +cleanup + +sandbox +ln -s missing-target "$R/dangling-link" +run "$R" 7 --allow-dirty +want "a dangling untracked symlink survives reviewer snapshots" 0 "is done" +cleanup + +sandbox +printf 'same content\n' > "$R/link-target-a" +printf 'same content\n' > "$R/link-target-b" +ln -s link-target-a "$R/review-link" +STUB_MISBEHAVE=agent2-retarget-link run "$R" 7 --allow-dirty +want "agent2 retargeting an untracked symlink is caught" 1 \ + "changed the code it was reviewing" +cleanup + +# --- section ownership (RR3-02) ------------------------------------------- +sandbox; STUB_MISBEHAVE=edit-review run "$R" 7 +want "agent1 rewriting the review caught" 1 "section that is closed" +cleanup +sandbox; STUB_MISBEHAVE=rewrite-log run "$R" 7 +want "rewritten log entry caught" 1 "append exactly one" +cleanup +sandbox; STUB_MISBEHAVE=extend-log run "$R" 7 +want "extending the previous log line is refused" 1 "append exactly one" +cleanup +sandbox; STUB_MISBEHAVE=omit-log run "$R" 7 +want "a turn without a new log line is refused" 1 "append exactly one" +cleanup +sandbox; STUB_MISBEHAVE=rewrite-prior-owned run "$R" 7 +want "agent1 cannot rewrite its completed prior-round section" 1 "closed during" +cleanup + +# --- resume target (RR3-03) ----------------------------------------------- +sandbox; STUB_MISBEHAVE=bad-resume run "$R" 7 +want "resume_status past the asking status refused" 1 "resumes where it was asked" +cleanup + +# A value written before this guard existed, or edited in by hand, is still +# refused when the answers come back rather than acted on. +sandbox +STUB_MISBEHAVE=ask run "$R" 7 +D="$R/.agents/work/issue-7/plan.md" +awk '/^resume_status: / { print "resume_status: done"; next } { print }' \ + "$D" > "$D.t" && mv "$D.t" "$D" +awk '/^\*\*Q1\./ { print; print ""; print "**A1.** No retry."; next } { print }' \ + "$D" > "$D.t" && mv "$D.t" "$D" +run "$R" 7; want "resume into a terminal status refused" 1 "question was raised from" +cleanup + +sandbox +STUB_MISBEHAVE=ask run "$R" 7 +D="$R/.agents/work/issue-7/plan.md" +awk '/^resume_status: / { print "resume_status: implementation-reviewed"; next } { print }' \ + "$D" > "$D.t" && mv "$D.t" "$D" +awk '/^\*\*Q1\./ { print; print ""; print "**A1.** No retry."; next } { print }' \ + "$D" > "$D.t" && mv "$D.t" "$D" +run "$R" 7 +want "resume into another allowed but later status refused" 1 "question was raised from" +cleanup + +# --- review results gate advancement (RR3-04) ----------------------------- +sandbox; STUB_MISBEHAVE=empty-review run "$R" 7 +want "advancing on an empty review refused" 1 "has to say what was reviewed" +cleanup +sandbox; STUB_MISBEHAVE=no-verdict run "$R" 7 +want "review without a verdict refused" 1 "states no verdict" +cleanup +sandbox; STUB_MISBEHAVE=verdict-in-prose run "$R" 7 +want "a verdict token in prose does not satisfy the review" 1 "states no verdict" +cleanup +for verdict in 'Verdict: **APPROVE**' '**Verdict:** APPROVE' \ + 'Verdict: `APPROVE`' '**Verdict: APPROVE**' 'Verdict: approve.'; do + sandbox; STUB_VERDICT_LINE="$verdict" run "$R" 7 + want "formatted verdict is accepted: ${verdict}" 0 "is done" + cleanup +done +sandbox; STUB_VERDICT_LINE=$'Verdict: APPROVE\n\n---' run "$R" 7 +want "a horizontal rule may follow the verdict" 0 "is done" +cleanup +for layout in bullet numbered heading table; do + sandbox + STUB_FINDING_LAYOUT="$layout" STUB_MISBEHAVE=no-dispositions run "$R" 7 + want "${layout} findings must be dispositioned" 1 "undispositioned" + cleanup +done +sandbox; STUB_MISBEHAVE=prose-disposition run "$R" 7 +want "mentioning a finding outside a valid table row is refused" 1 "undispositioned" +cleanup +sandbox; STUB_MISBEHAVE=duplicate-disposition run "$R" 7 +want "duplicate disposition rows are refused" 1 "undispositioned" +cleanup +sandbox; STUB_VERDICT="REQUEST CHANGES" run "$R" 7 +want "advancing past REQUEST CHANGES refused" 1 "is not available from here" +cleanup +sandbox; STUB_MISBEHAVE=no-outcome run "$R" 7 +want "done with an empty Outcome refused" 1 "empty ## Outcome" +cleanup + +# --- per-round snapshots (RR3-05) ----------------------------------------- +sandbox; STUB_MISBEHAVE=loop-once run "$R" 7 --mr 2 +want "a second review round reaches done" 0 "is done" +check "the previous plan round was saved for comparison" \ + "$([[ -f "$R/.agents/work/issue-7/rounds/plan-1.md" ]] && echo 0 || echo 1)" +check "the implementation round patch was saved" \ + "$([[ -f "$R/.agents/work/issue-7/rounds/impl-1.patch" ]] && echo 0 || echo 1)" +check "the saved patch carries the untracked implementation file" \ + "$(grep -q 'src.txt' "$R/.agents/work/issue-7/rounds/impl-1.patch" && echo 0 || echo 1)" +cleanup +sandbox; STUB_MISBEHAVE=loop-once STUB_PRIOR_REFERENCE=1 run "$R" 7 --mr 2 +want "a previous-round finding reference is not a new finding" 0 "is done" +cleanup +sandbox; STUB_MISBEHAVE=wrong-review-dash run "$R" 7 --mr 2 +want "a mistyped review heading gets a precise error" 1 \ + "must write exactly one '## Plan Review — Round 2'" +cleanup +sandbox; STUB_MISBEHAVE=tamper-snapshot run "$R" 7 +want "rewriting a driver-owned round snapshot is refused" 1 "review snapshot" +cleanup + +# --- the final state must still be what agent2 reviewed ------------------- +sandbox; STUB_MISBEHAVE=final-edit run "$R" 7 +want "agent1 source edits after review require another round" 1 "another implementation review" +cleanup +sandbox; STUB_MISBEHAVE=fix-and-rereview run "$R" 7 --mr 2 +want "agent1 source fixes finish after another review" 0 "is done" +cleanup # --- stalled turn --------------------------------------------------------- sandbox; STUB_MISBEHAVE=nothing run "$R" 7 @@ -300,6 +886,12 @@ sandbox run "$R" start 7 --slug ../escape; reject "slug traversal rejected" "plain name" run "$R" 7 --mr 0; reject "zero rounds rejected" "positive whole number" run "$R" 7 --max-turns 0; reject "zero turns rejected" "positive whole number" +run "$R" 7 --claude-model 'bad model' +reject "model names with spaces are rejected" "without spaces" +run "$R" 7 --claude-effort ultra +reject "unknown Claude effort is rejected" "must be low, medium, high, xhigh, or max" +run "$R" 7 --codex-effort max +reject "unknown Codex effort is rejected" "must be minimal, low, medium, high, or xhigh" run "$R" https://github.com/other/proj/issues/7 reject "foreign issue URL refused" "but this repository" run "$R" https://example.invalid/o/r/issues/7 @@ -308,11 +900,48 @@ STUB_REPO_URL=unavailable run "$R" https://github.com/o/r/issues/7 want "URL validation fails closed" 1 "cannot resolve" cleanup +# A missing recorded PR target is a setup error, before an agent turn can be +# spent against a baseline that cannot be published. +sandbox +export PR_BASE_BRANCH=missing +run "$R" 7 --cp +want "missing PR target is refused during setup" 1 "origin/missing is unavailable" +check "missing PR target starts no agent turn" \ + "$([[ ! -d "$R/.agents/work/issue-7/turns" ]] && echo 0 || echo 1)" +unset PR_BASE_BRANCH +cleanup + # --- dirty worktree (RS-01) ----------------------------------------------- sandbox; echo scratch > "$R/untracked.txt" run "$R" 7; want "dirty start refused" 1 "uncommitted changes" run "$R" 7 --allow-dirty; want "dirty start with override" 0 "allow-dirty" cleanup +sandbox; echo scratch > "$R/untracked.txt" +run "$R" 7 --allow-dirty --cp +reject "dirty publication flags fail before setup" "cannot be used together" +cleanup +sandbox; echo scratch > "$R/untracked.txt" +run "$R" start 7 --allow-dirty +run "$R" 7 --cp +want "publication of an existing dirty-start run fails before a turn" 1 \ + "publication is disabled" +check "dirty publication refusal writes no transcript" \ + "$([[ ! -d "$R/.agents/work/issue-7/turns" ]] && echo 0 || echo 1)" +cleanup + +# A missing earlier transcript must not make the next turn reuse an existing +# sequence number and overwrite the record that survived. +sandbox; run "$R" start 7 +mkdir -p "$R/.agents/work/issue-7/turns" +printf 'surviving transcript\n' > "$R/.agents/work/issue-7/turns/02-agent1.log" +run "$R" step 7 +want "a transcript gap does not prevent the next turn" 0 +check "a transcript gap allocates after the highest sequence" \ + "$([[ -f "$R/.agents/work/issue-7/turns/03-agent1.log" ]] && echo 0 || echo 1)" +check "the surviving transcript is not overwritten" \ + "$(grep -q '^surviving transcript$' \ + "$R/.agents/work/issue-7/turns/02-agent1.log" && echo 0 || echo 1)" +cleanup # --- execution boundary (RR2-06) ------------------------------------------ sandbox @@ -323,6 +952,12 @@ AGENT1_CMD="${SANDBOX}/bin/stub-agent --dangerously-skip-permissions" \ want "unsafe agent command needs explicit opt-in" 0 "is done" cleanup +sandbox +AGENT2_CMD="${SANDBOX}/bin/stub-agent2 --sandbox danger-full-access" \ + run "$R" 7 --sa +want "swapped unsafe agent command is still refused" 1 "allow-unsafe-agents" +cleanup + # --- answers (RS-06) ------------------------------------------------------ sandbox STUB_MISBEHAVE=ask run "$R" 7 @@ -373,9 +1008,23 @@ pr_section_custom() { next } {print}' "$d" > "$d.t" && mv "$d.t" "$d" } -# A real bump: the version moves, and both generated reports carry the new one. -# The previous fixture only appended a byte to each file, which is exactly the -# stale-report case the publisher is supposed to refuse. +pr_section_with_notes() { + local d="$1/.agents/work/issue-7/plan.md" + awk '/^## Pull Request$/ { + print; print "" + print "### Summary"; print "Does the thing."; print "" + print "### Changes"; print "- Added src.txt"; print "" + print "### Reviewer notes"; print "Agent context."; print "" + print "### Important notes"; print "Keep this last." + next + } {print}' "$d" > "$d.t" && mv "$d.t" "$d" +} +# Reads the argument immediately following an option from the recorded gh +# invocation. PR bodies contain newlines, so whole-file grep is too loose. +recorded_arg_after() { + awk -v option="$2" 'found { print; exit } $0 == option { found = 1 }' "$1" +} +# A real bump moves the project version and both generated reports together. bump() { local v="${2:-2.0.0-SNAPSHOT.2}" printf 'val chordsVersion: String by extra("%s")\n' "$v" > "$1/version.gradle.kts" @@ -383,12 +1032,6 @@ bump() { "$v" > "$1/pom.xml" printf '# Dependencies of `o:r:%s`\n' "$v" > "$1/dependencies.md" } -# Touched but not actually bumped, for the negative case. -fake_bump() { - printf 'b\n' >> "$1/version.gradle.kts" - printf 'b\n' >> "$1/pom.xml" - printf 'b\n' >> "$1/dependencies.md" -} manual_plan() { awk '/^## Manual Testing$/{print; print "" print "### Setup" @@ -436,13 +1079,13 @@ run "$R" 7 --cp; want "PR refused with an empty Changes body" 1 "requires exact" cleanup sandbox; run "$R" 7 >/dev/null; pr_section "$R" -run "$R" 7 --cp; want "PR refused without version and reports" 1 "not in the changeset" +run "$R" 7 --cp; want "PR refused without a version bump" 1 "not in the changeset" current_branch="$(git -C "$R" rev-parse --abbrev-ref HEAD)" check "nothing was committed on refusal" \ "$([[ "$current_branch" == master ]] && echo 0 || echo 1)" cleanup -sandbox; run "$R" 7 >/dev/null; pr_section "$R"; bump "$R" +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" run "$R" 7 --cp; want "PR published from a compliant changeset" 0 "draft pull request" check "version commit uses the required message" \ "$(git -C "$R" log --format=%s | grep -q '^Bump version' && echo 0 || echo 1)" @@ -453,12 +1096,266 @@ want "re-run finds the existing PR" 0 "already open" cleanup # A failed `gh pr create` must leave the push intact and stay retryable. -sandbox; run "$R" 7 >/dev/null; pr_section "$R"; bump "$R" +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" STUB_PR_CREATE_FAILS=1 run "$R" 7 --cp want "failed PR create reports and stops" 1 "gh pr create failed" run "$R" 7 --cp; want "retry after a failed PR create" 0 "draft pull request" cleanup +# --- stacked work --------------------------------------------------------- +# Starting from a branch whose own PR is still open is the ordinary case, not a +# refusal: the task branch is cut from it and the PR still targets master. +stack_on() { # stack_on [version] — creates one commit outside master + git -C "$1" checkout -q -b "$2" + printf 'earlier\n' > "$1/earlier.txt" + if [[ -n "${3:-}" ]]; then + bump "$1" "$3" + fi + git -C "$1" add -A + git -C "$1" commit -qm "Earlier work under review" +} + +sandbox; stack_on "$R" open-pr-branch +parent_head="$(git -C "$R" rev-parse HEAD)" +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 +want "stacking is reported during setup" 0 "starting point 'open-pr-branch'" +check "review prompt construction emits no command errors" \ + "$(printf '%s' "$OUT" | grep -q 'command not found' && echo 1 || echo 0)" +pr_section "$R" +run "$R" 7 --cp; want "PR published from a stacked branch" 0 "draft pull request" +want "stacking is reported to the user" 0 "stacked on 'open-pr-branch'" +recorded_start="$(awk '/^start_commit: / {print $2}' \ + "$R/.agents/work/issue-7/plan.md")" +check "the exact starting commit is recorded" \ + "$([[ "$recorded_start" == "$parent_head" ]] && echo 0 || echo 1)" +check "task branch was cut from the stacked branch" \ + "$(git -C "$R" rev-parse --abbrev-ref HEAD | grep -qx 'a-test-issue' && echo 0 || echo 1)" +check "the stacked branch was not committed onto" \ + "$([[ "$(git -C "$R" rev-list --count open-pr-branch)" -eq 2 ]] && echo 0 || echo 1)" +check "the PR still targets master" \ + "$([[ "$(recorded_arg_after "$STUB_PR_RECORD" --base)" == master ]] \ + && echo 0 || echo 1)" +check "the PR body names the branch it is stacked on" \ + "$(grep -q 'Reviewer notes' "$STUB_PR_RECORD" && + grep -q 'open-pr-branch' "$STUB_PR_RECORD" && + grep -q "${parent_head:0:7}" "$STUB_PR_RECORD" && echo 0 || echo 1)" +check "the agent review patch excludes inherited parent files" \ + "$(grep -q 'earlier.txt' \ + "$R/.agents/work/issue-7/rounds/impl-1.patch" && echo 1 || echo 0)" +check "the reviewer prompt excludes inherited commits from findings" \ + "$(grep -q 'outside this task and must not produce findings' \ + "$STUB_REVIEW_PROMPT_RECORD" && echo 0 || echo 1)" +check "the reviewer prompt names the recorded task boundary" \ + "$(grep -qF "Review only this task's changes from \`${recorded_start}\`" \ + "$STUB_REVIEW_PROMPT_RECORD" && echo 0 || echo 1)" +cleanup + +# An agent may add reviewer context of its own. The driver merges the stacking +# paragraph into that section before any later optional section. +sandbox; stack_on "$R" open-pr-branch +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section_with_notes "$R" +run "$R" 7 --cp; want "agent and driver reviewer notes publish" 0 "draft pull request" +check "the PR body has one Reviewer notes heading" \ + "$([[ "$(grep -c '^## Reviewer notes$' "$STUB_PR_RECORD")" -eq 1 ]] \ + && echo 0 || echo 1)" +check "the stacking paragraph stays inside Reviewer notes" \ + "$(awk ' + $0 == "## Reviewer notes" { notes++; note_line = NR } + /^The workflow started from / { driver_line = NR } + $0 == "## Important notes" { important_line = NR } + END { + exit !(notes == 1 && note_line < driver_line && + driver_line < important_line) + } + ' "$STUB_PR_RECORD" && echo 0 || echo 1)" +check "the agent reviewer context is preserved" \ + "$(grep -q '^Agent context\.$' "$STUB_PR_RECORD" && echo 0 || echo 1)" +check "the driver reviewer note has one blank line on each side" \ + "$(awk ' + { + if (/^The workflow started from /) { + found = 1 + before = (previous == "" && before_previous != "") + getline + after = ($0 == "") + getline + next_heading = ($0 == "## Important notes") + exit !(before && after && next_heading) + } + before_previous = previous + previous = $0 + } + END { if (!found) exit 1 } + ' "$STUB_PR_RECORD" && echo 0 || echo 1)" +cleanup + +# A version inherited from the parent is not this task's required bump. It may +# be in the PR diff against master, but it is absent from the task changeset +# after the recorded starting commit. +sandbox; stack_on "$R" open-pr-branch 2.0.0-SNAPSHOT.2 +run "$R" 7 >/dev/null; pr_section "$R" +run "$R" 7 --cp +want "a parent version bump does not satisfy the task" 1 \ + "not in the changeset after start_commit" +cleanup + +# A run that starts on its existing task branch keeps using that branch, but +# earlier commits outside the PR target still need a reviewer boundary. +sandbox; stack_on "$R" a-test-issue +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +run "$R" 7 --cp; want "an existing task branch publishes" 0 "draft pull request" +want "existing task branch reports inherited history" 0 "stacked on 'a-test-issue'" +check "existing task branch gets a reviewer boundary" \ + "$(grep -q 'Reviewer notes' "$STUB_PR_RECORD" && echo 0 || echo 1)" +cleanup + +# The immutable starting commit, not the later value of its branch ref, owns +# the stack boundary and carried-commit count. +sandbox; stack_on "$R" open-pr-branch +parent_head="$(git -C "$R" rev-parse HEAD)" +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +git -C "$R" checkout -q --detach +git -C "$R" branch -f open-pr-branch master +run "$R" 7 --cp +want "a moved parent ref keeps the recorded stack boundary" 0 \ + "stacked on 'open-pr-branch' at ${parent_head:0:7}" +check "a moved parent ref does not remove the reviewer note" \ + "$(grep -q "${parent_head:0:7}" "$STUB_PR_RECORD" && echo 0 || echo 1)" +cleanup + +# Switching to another commit with the same tree cannot retarget the task +# branch: content digests do not encode ancestry. +sandbox +git -C "$R" checkout -q -b alternate +git -C "$R" commit -qm "Equivalent alternate history" --allow-empty +git -C "$R" checkout -q master +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +git -C "$R" checkout -q alternate +run "$R" 7 --cp +want "publication refuses a different HEAD with the same tree" 1 \ + "HEAD moved from the recorded starting commit" +cleanup + +# A detached start is recorded as a commit label; publication should not expose +# Git's raw "HEAD" sentinel in its branch-creation message. +sandbox +git -C "$R" checkout -q --detach +detached_start="$(git -C "$R" rev-parse --short HEAD)" +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +run "$R" 7 --cp; want "publication from a detached start succeeds" 0 "draft pull request" +want "detached publication names its recorded commit" 0 \ + "created branch 'a-test-issue' from '${detached_start}'" +check "detached publication does not report raw HEAD" \ + "$(printf '%s' "$OUT" | grep -q "from 'HEAD'" && echo 1 || echo 0)" +cleanup + +# The stacked conclusion comes from the recorded base branch, not from the +# branch that happens to be checked out, so a retry reaches it again. +sandbox; stack_on "$R" open-pr-branch +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +STUB_PR_CREATE_FAILS=1 run "$R" 7 --cp >/dev/null +run "$R" 7 --cp; want "retry from the task branch still reports stacking" 0 \ + "stacked on 'open-pr-branch'" +cleanup + +# A parent that merges mid-run moves the merge-base, so the reviewed scope is +# no longer what the pull request would contain. The existing merge-base guard +# refuses that, and stacking does not get to talk it round: the run is redone, +# not published with a stacking note over a stale review. +sandbox; stack_on "$R" open-pr-branch +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +git -C "$R" push -q origin open-pr-branch:master +git -C "$R" fetch -q origin +run "$R" 7 --cp; want "a parent merging mid-run is refused, not published" 1 \ + "is not the pull request's merge-base" +check "nothing was published when the parent merged" \ + "$([[ -f "$STUB_PR_RECORD" ]] && echo 1 || echo 0)" +cleanup + +# Local master moving is not a merge. Only origin/master is, since that is what +# the pull request is opened against. +sandbox; stack_on "$R" open-pr-branch +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +git -C "$R" branch -f master open-pr-branch +run "$R" 7 --cp; want "an unpushed local master does not clear stacking" 0 \ + "stacked on 'open-pr-branch'" +cleanup + +# An unstacked run must not gain a reviewer note it does not need. +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +run "$R" 7 --cp; want "unstacked run publishes" 0 "draft pull request" +check "no stacking reported when cut from master" \ + "$(printf '%s' "$OUT" | grep -q 'stacked on' && echo 1 || echo 0)" +check "no reviewer note when cut from master" \ + "$([[ -f "$STUB_PR_RECORD" ]] && + ! grep -q 'Reviewer notes' "$STUB_PR_RECORD" && echo 0 || echo 1)" +cleanup + +# The configurable PR target is captured at setup, so a later invocation cannot +# silently retarget a finished run by omitting or changing the environment. +sandbox +git -C "$R" branch trunk +git -C "$R" push -qu origin trunk +export PR_BASE_BRANCH=trunk +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +unset PR_BASE_BRANCH +run "$R" 7 --cp; want "a configured PR base publishes" 0 "draft pull request" +check "the configured PR base is recorded in the document" \ + "$(grep -qx 'pr_base_branch: trunk' \ + "$R/.agents/work/issue-7/plan.md" && echo 0 || echo 1)" +check "the recorded PR base reaches gh after the environment is cleared" \ + "$([[ "$(recorded_arg_after "$STUB_PR_RECORD" --base)" == trunk ]] \ + && echo 0 || echo 1)" +cleanup + +# Preserve Git's useful diagnosis when the derived task branch already exists. +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +git -C "$R" branch a-test-issue +run "$R" 7 --cp +want "existing derived branch reports the Git error" 1 "already exists" +cleanup + +# Committing the reviewed work onto the parent branch moves HEAD away from the +# recorded starting point, so publication refuses it before creating a branch. +sandbox; stack_on "$R" open-pr-branch +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +git -C "$R" add -A; git -C "$R" commit -qm "premature" +run "$R" 7 --cp; want "committed work on a foreign branch refused" 1 \ + "HEAD moved from the recorded starting commit" +cleanup + +# --- publication is bound to the reviewed changeset (RR3-06) -------------- +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +bump "$R" 2.0.0-SNAPSHOT.3 +run "$R" 7 --cp; want "post-review edits are not published" 1 "changed since the review" +check "nothing was committed on a digest mismatch" \ + "$([[ "$(git -C "$R" rev-parse --abbrev-ref HEAD)" == master ]] && echo 0 || echo 1)" +cleanup + +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +chmod +x "$R/src.txt" +run "$R" 7 --cp +want "post-review executable-bit changes are not published" 1 "changed since the review" +cleanup + +# The prospective PR baseline is the merge-base with master, not whatever HEAD +# happens to be, so publication can account for every inherited commit. +sandbox +printf 'local master only\n' > "$R/local-master.txt" +git -C "$R" add -A && git -C "$R" commit -qm "Local master commit." +git -C "$R" checkout -q -b earlier-work +printf 'earlier\n' > "$R/earlier.txt" +git -C "$R" add -A && git -C "$R" commit -qm "Earlier commit." +run "$R" start 7 >/dev/null +recorded="$(awk '/^base_commit: / {print $2}' "$R/.agents/work/issue-7/plan.md")" +expected="$(git -C "$R" rev-parse --short \ + "$(git -C "$R" merge-base refs/remotes/origin/master HEAD)")" +check "baseline is the merge-base with origin/master" \ + "$([[ "$recorded" == "$expected" ]] && echo 0 || echo 1)" +check "a local-only master commit remains in the PR changeset" \ + "$([[ "$recorded" != "$(git -C "$R" rev-parse --short master)" ]] && echo 0 || echo 1)" +cleanup + # --- re-review: round accounting (RR2-01, RR2-07) ------------------------- sandbox; STUB_MISBEHAVE=loop-no-round run "$R" 7 want "loopback without spending a round refused" 1 "increase by exactly one" @@ -513,6 +1410,27 @@ want "short fence inside a long fence does not truncate protection" \ 1 "protected fields or sections" cleanup +sandbox +bare_headings='Problem stated here.\n# Task\nIssue task details.\n# Log\n' +bare_headings+='Acceptance criteria: issue headings stay nested and the run completes.' +STUB_ISSUE_BODY="$bare_headings" run "$R" 7 +want "bare issue headings remain nested under the Issue section" 0 "is done" +D="$R/.agents/work/issue-7/plan.md" +check "issue Task and Log headings do not duplicate workflow sections" \ + "$([[ "$(grep -c '^## Task$' "$D")" -eq 1 \ + && "$(grep -c '^## Log$' "$D")" -eq 1 \ + && "$(grep -c '^### Task$' "$D")" -eq 1 \ + && "$(grep -c '^### Log$' "$D")" -eq 1 ]] && echo 0 || echo 1)" +cleanup + +sandbox +bare_hidden='Problem stated here.\n# Task\nHIDDEN_CRITERION\n' +bare_hidden+='Acceptance criteria: content after a bare heading remains protected.' +STUB_ISSUE_BODY="$bare_hidden" STUB_MISBEHAVE=edit-hidden run "$R" 7 +want "rewriting after a bare issue heading is refused" 1 \ + "protected fields or sections" +cleanup + sandbox; STUB_MISBEHAVE=duplicate-task run "$R" 7 want "duplicate protected Task section refused" 1 "protected fields or sections" cleanup @@ -542,58 +1460,59 @@ want "three rounds fit within the derived ceiling" 0 "is done" cleanup # --- re-review: publish gates (RR2-03, RR2-05) ---------------------------- -sandbox; run "$R" 7 >/dev/null; pr_section "$R"; fake_bump "$R" +sandbox; STUB_BUMP=fake run "$R" 7 >/dev/null; pr_section "$R" run "$R" 7 --cp; want "touched but unbumped version refused" 1 "did not increase" cleanup -sandbox; run "$R" 7 >/dev/null; pr_section "$R"; bump "$R" -printf 'stale pom\n' > "$R/pom.xml" -run "$R" 7 --cp; want "stale report refused" 1 "root project version" +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.0 run "$R" 7 >/dev/null; pr_section "$R" +run "$R" 7 --cp; want "lower version refused" 1 "did not increase" cleanup -sandbox; run "$R" 7 >/dev/null; pr_section "$R"; bump "$R" -printf '%s\n' '' ' 2.0.0-SNAPSHOT.1' \ - ' 2.0.0-SNAPSHOT.2' \ - ' ' '' \ - > "$R/pom.xml" +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 STUB_BAD_REPORT=stale-pom \ + run "$R" 7 >/dev/null; pr_section "$R" +run "$R" 7 --cp; want "stale POM refused" 1 "root project version" +cleanup + +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 STUB_BAD_REPORT=misplaced-pom \ + run "$R" 7 >/dev/null; pr_section "$R" run "$R" 7 --cp want "new version elsewhere in a stale POM is refused" 1 "root project version" cleanup -sandbox; run "$R" 7 >/dev/null; pr_section "$R"; bump "$R" -printf '# Dependencies of `o:r:2.0.0-SNAPSHOT.1`\nnew version: 2.0.0-SNAPSHOT.2\n' \ - > "$R/dependencies.md" +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 STUB_BAD_REPORT=stale-dependencies \ + run "$R" 7 >/dev/null; pr_section "$R" run "$R" 7 --cp want "new token with a stale dependency heading is refused" 1 "not every" cleanup -sandbox; run "$R" 7 >/dev/null; pr_section "$R"; bump "$R" -printf '# Dependencies of `o:r:2.0.0-SNAPSHOT.2`\n# Dependencies of `o:s:2.0.0-SNAPSHOT.1`\n' \ - > "$R/dependencies.md" +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 STUB_BAD_REPORT=partial-dependencies \ + run "$R" 7 >/dev/null; pr_section "$R" run "$R" 7 --cp want "partially stale dependency headings are refused" 1 "not every" cleanup -sandbox; STUB_MANUAL=required run "$R" 7 >/dev/null; pr_section "$R"; bump "$R" +sandbox; STUB_MANUAL=required STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" run "$R" 7 --cp; want "required manual testing without a plan is not published" 1 "no usable plan" cleanup -sandbox; STUB_MANUAL=required run "$R" 7 >/dev/null; pr_section "$R"; bump "$R"; manual_plan "$R" +sandbox; STUB_MANUAL=required STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null +pr_section "$R"; manual_plan "$R" run "$R" 7 --cp; want "required manual testing with a plan publishes" 2 "draft pull request" cleanup for missing in long-bullet no-setup no-expected no-covers; do sandbox - STUB_MANUAL=required run "$R" 7 >/dev/null - pr_section "$R"; bump "$R"; manual_plan_variant "$R" "$missing" + STUB_MANUAL=required STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null + pr_section "$R"; manual_plan_variant "$R" "$missing" run "$R" 7 --cp want "manual plan rejects ${missing}" 1 "no usable plan" cleanup done # The first publication commit may succeed before the task commit fails. A -# retry must recognize the committed reports and finish the remaining steps. -sandbox; run "$R" 7 >/dev/null; pr_section "$R"; bump "$R" +# retry must recognize the committed version and reports and finish the +# remaining steps. +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" cat > "$R/.git/hooks/commit-msg" <<'HOOK' #!/usr/bin/env bash grep -q '^Bump version' "$1" @@ -617,6 +1536,12 @@ cleanup # about the symptom rather than the cause. readonly REPO="${SUITE_DIR}/../.." +for option in --claude-model --claude-effort --codex-model --codex-effort; do + check "the /pair command advertises ${option}" \ + "$(grep -qF -- "$option" "${REPO}/.claude/commands/pair.md" \ + && echo 0 || echo 1)" +done + # Codex's workspace-write sandbox excludes gitignored paths, and .agents/work/ # is gitignored by design. Without --add-dir the reviewer reads the document, # forms its findings, and cannot write them down. @@ -626,21 +1551,84 @@ readonly REPO="${SUITE_DIR}/../.." # the default that actually runs had lost it. agent2_default="$(grep -m1 '^AGENT2_CMD=' "$DRIVER")" check "AGENT2_CMD default makes the work root writable" \ - "$(printf '%s' "$agent2_default" | grep -q -- '--add-dir ${WORK_ROOT}' \ + "$(printf '%s' "$agent2_default" | grep -q -- '--add-dir .agents/work' \ && echo 0 || echo 1)" check "the work root is gitignored, which is why --add-dir is needed" \ - "$(git -C "$REPO" check-ignore -q .agents/work && echo 0 || echo 1)" + "$(git -C "$REPO" check-ignore -q .agents/work/ && echo 0 || echo 1)" +check "Claude Code local permissions are gitignored" \ + "$(git -C "$REPO" check-ignore -q .claude/settings.local.json \ + && echo 0 || echo 1)" +check "lock signal traps terminate the driver after releasing the lock" \ + "$(grep -qF -- 'kill -s "$signal" "$$"' "$DRIVER" \ + && grep -qF -- "trap 'handle_signal TERM' TERM" "$DRIVER" \ + && echo 0 || echo 1)" # agent1 runs with --setting-sources project, which loads .claude/settings.json # and nothing else. A verification command missing from it is refused before # the process starts, and the run reviews code that was never compiled. check "project settings exist for agent1 to load" \ "$([[ -f "${REPO}/.claude/settings.json" ]] && echo 0 || echo 1)" -for task in ':core:test' ':client:test' 'detekt'; do - check "project settings allow ${task}" \ - "$(grep -qF -- "./gradlew ${task}" "${REPO}/.claude/settings.json" \ +for command in '.agents/workflows/gradle-root.sh' \ + './gradlew detekt' './gradlew generatePom mergeAllLicenseReports'; do + check "project settings allow ${command}" \ + "$(grep -qF -- "${command}" "${REPO}/.claude/settings.json" \ + 2>/dev/null && echo 0 || echo 1)" +done +for command in 'Bash(git status:*)' 'Bash(git diff:*)' \ + 'Bash(git log:*)' 'Bash(git show:*)'; do + check "project settings allow ${command}" \ + "$(grep -qF -- "$command" "${REPO}/.claude/settings.json" \ 2>/dev/null && echo 0 || echo 1)" done +for skill in build-engineer codegen-engineer component-engineer tester kotlin-engineer; do + check "${skill} routes root Gradle through the wrapper" \ + "$(grep -qF -- '.agents/workflows/gradle-root.sh' \ + "${REPO}/.agents/skills/${skill}/SKILL.md" && echo 0 || echo 1)" +done + +# The root project's JDK 11 selection cannot be expressed as a permission rule: +# `JAVA_HOME="$(jenv prefix)" ./gradlew …` is an environment assignment with a +# command substitution, and `jenv shell 11` does not survive into the next +# command. The wrapper is that flow behind one allowlistable path, so an +# unattended implementation can compile before it is reviewed. +check "the root Gradle wrapper script is executable" \ + "$([[ -x "${SUITE_DIR}/gradle-root.sh" ]] && echo 0 || echo 1)" +check "project settings allow the root Gradle wrapper" \ + "$(grep -qF -- '.agents/workflows/gradle-root.sh' \ + "${REPO}/.claude/settings.json" 2>/dev/null && echo 0 || echo 1)" + +wrapper="${SUITE_DIR}/gradle-root.sh" +wrapper_output="$($wrapper publish 2>&1)"; wrapper_rc=$? +check "the root wrapper refuses external publication" \ + "$([[ "$wrapper_rc" -eq 1 && "$wrapper_output" == *"not permitted"* ]] \ + && echo 0 || echo 1)" +wrapper_output="$($wrapper --init-script /tmp/untrusted.gradle test 2>&1)"; wrapper_rc=$? +check "the root wrapper refuses arbitrary init scripts" \ + "$([[ "$wrapper_rc" -eq 1 && "$wrapper_output" == *"not permitted"* ]] \ + && echo 0 || echo 1)" + +# A fake Java 11 proves that the architecture guard fires before the real +# Gradle wrapper starts. The fake uname confines the macOS branch to this test. +fake_root="$(mktemp -d)" +mkdir -p "${fake_root}/jdk/bin" "${fake_root}/bin" +cat > "${fake_root}/jdk/bin/java" <<'JAVA' +#!/usr/bin/env bash +printf ' java.specification.version = 11\n' +printf ' os.arch = aarch64\n' +JAVA +cat > "${fake_root}/bin/uname" <<'UNAME' +#!/usr/bin/env bash +printf 'Darwin\n' +UNAME +chmod +x "${fake_root}/jdk/bin/java" "${fake_root}/bin/uname" +wrapper_output="$(PATH="${fake_root}/bin:${ORIGINAL_PATH}" \ + CHORDS_JDK11_HOME="${fake_root}/jdk" "$wrapper" :core:test 2>&1)" +wrapper_rc=$? +check "the root wrapper refuses a macOS ARM JDK" \ + "$([[ "$wrapper_rc" -eq 1 && "$wrapper_output" == *"must use an x86_64"* ]] \ + && echo 0 || echo 1)" +rm -rf "$fake_root" + printf '\n%s passed, %s failed\n' "$PASS" "$FAIL" [[ "$FAIL" -eq 0 ]] diff --git a/.agents/workflows/pair.sh b/.agents/workflows/pair.sh index 3e4faef8..eefa7a5f 100755 --- a/.agents/workflows/pair.sh +++ b/.agents/workflows/pair.sh @@ -18,12 +18,15 @@ usage() { cat >&2 <<'USAGE' Driver for the two-agent pair workflow. -A GitHub issue is the only input. It must describe what to do or what is wrong, -and state its acceptance criteria. The worktree must be clean at the start -unless --allow-dirty is given. +A new task starts from a GitHub issue. It must describe what to do or what is +wrong, and state its acceptance criteria. The worktree must be clean at the +start unless --allow-dirty is given. Usage: - pair.sh [--ad] [--mr N] [--cp] + pair.sh [--ad] [--mr N] [--cp] [--sa] [--max-turns N] + [--allow-dirty] [--allow-unsafe-agents] + [--claude-model MODEL] [--claude-effort LEVEL] + [--codex-model MODEL] [--codex-effort LEVEL] Runs the issue to completion: sets up on first call, resumes on later ones. Run the same command again after anything stops it. is a number (123), @@ -34,9 +37,9 @@ Run the same command again after anything stops it. is a number (123), --mr, --max-rounds N review rounds allowed in each phase; N rounds permit at most N - 1 send-backs (default 2) --allow-dirty start even though the worktree has uncommitted - changes. They land in the reviewer's scope and are - reviewed as if the agents wrote them, and PR - publication is refused for the run. + changes. They enter the reviewer's scope as if the + agents wrote them. This option cannot be combined + with --create-pr. --allow-unsafe-agents permit an AGENT1_CMD or AGENT2_CMD that disables approvals or sandboxing. Use only inside an externally isolated, credential-free environment. @@ -46,8 +49,27 @@ Run the same command again after anything stops it. is a number (123), The agents never touch Git either way — the driver does this afterwards, and only on a finished run from a worktree that was clean at the start. + The task branch starts from whatever branch the run + started on, and the PR targets the base recorded at + setup (master by default). + Starting from a branch with commits outside the PR + target is supported: those commits may show up in + this PR, and the PR body identifies their exact + boundary. + --sa, --swap-agents swap the planner/implementer and reviewer commands. + Repeat this option when resuming the same task. + --claude-model MODEL select the Claude Code model for this task. + --claude-effort LEVEL select Claude effort: low, medium, high, xhigh, or + max. + --codex-model MODEL select the Codex model for this task. + --codex-effort LEVEL select Codex reasoning effort: minimal, low, + medium, high, or xhigh. + +Model and effort selections are recorded when the task starts and reused on +resume. A later invocation cannot change them. Less often: + pair.sh run resume a task created with a custom --slug pair.sh step take exactly one turn and stop pair.sh status print the current state, safe during a run pair.sh start set up without running @@ -68,10 +90,12 @@ Environment: --model claude-opus-5 --effort high) AGENT2_CMD reviewer (default: codex exec --sandbox workspace-write - --add-dir /.agents/work --ephemeral + --add-dir .agents/work --ephemeral --ignore-user-config -m gpt-5.6-sol -c model_reasoning_effort="high" -c service_tier="default") + PR_BASE_BRANCH pull request target branch (default: master); its + origin/ ref must exist when a run starts Model and effort are pinned so a review is reproducible. Codex's are passed as flags because --ignore-user-config discards ~/.codex/config.toml by design. @@ -104,15 +128,24 @@ readonly TEMPLATE="${REPO_ROOT}/.agents/skills/pair-workflow/template.md" # --ignore-user-config deliberately discards ~/.codex/config.toml — the run # must not depend on local configuration that differs between machines. # -# --add-dir names WORK_ROOT because Codex's workspace-write sandbox excludes -# gitignored paths from the writable set, and .agents/work/ is gitignored by -# design — the working document is a scratch artifact that is never committed. +# --add-dir names the work directory because Codex's workspace-write sandbox +# excludes gitignored paths from the writable set, and .agents/work/ is +# gitignored by design — the working document is a scratch artifact that is +# never committed. # Without it agent2 can read the document but not write its review, and the # turn ends with the driver aborting on an unmodified document. This widens # the sandbox by exactly one directory inside the repository; it is not a # bypass flag, and unsafe_agent_roles() does not treat it as one. AGENT1_CMD="${AGENT1_CMD:-claude -p --permission-mode acceptEdits --setting-sources project --model claude-opus-5 --effort high}" -AGENT2_CMD="${AGENT2_CMD:-codex exec --sandbox workspace-write --add-dir ${WORK_ROOT} --ephemeral --ignore-user-config -m gpt-5.6-sol -c model_reasoning_effort=\"high\" -c service_tier=\"default\"}" +AGENT2_CMD="${AGENT2_CMD:-codex exec --sandbox workspace-write --add-dir .agents/work --ephemeral --ignore-user-config -m gpt-5.6-sol -c model_reasoning_effort=\"high\" -c service_tier=\"default\"}" + +# Engine-specific selections requested on the command line. They are separate +# from role assignment: --swap-agents exchanges the complete configured +# commands, while these settings continue to identify Claude and Codex. +CLAUDE_MODEL_OPTION="" +CLAUDE_EFFORT_OPTION="" +CODEX_MODEL_OPTION="" +CODEX_EFFORT_OPTION="" readonly DEFAULT_MAX_TURNS=12 @@ -148,10 +181,21 @@ CREATE_PR=0 # accepting that they land in the review scope. ALLOW_DIRTY=0 +# The branch every pull request targets, per "Creating a Pull Request" in +# AGENTS.md. It is also the branch a task is normally cut from — but not the +# only one it may be cut from, see `create_pr`. Overridable for a repository +# whose trunk is named differently; the workflow never infers it from the +# remote, because guessing the target of a PR is not a guess worth making. +PR_BASE_BRANCH="${PR_BASE_BRANCH:-master}" + # Set only when the caller acknowledges that custom agent commands remove the # CLIs' normal execution boundary. ALLOW_UNSAFE_AGENTS=0 +# Set by --swap-agents. The guard makes repeated aliases idempotent rather than +# swapping the commands back to their defaults. +AGENTS_SWAPPED=0 + die() { printf 'pair: %s\n' "$1" >&2; exit 1; } # Same message, but returns instead of exiting. Helpers that may be called from # inside a command substitution must use this: there, `die` ends only the @@ -162,6 +206,256 @@ info() { printf 'pair: %s\n' "$1" >&2; } doc_for() { printf '%s/%s/plan.md' "$WORK_ROOT" "$1"; } +# Returns the executable recorded in the working document for an agent command. +agent_command_name() { printf '%s' "$1" | awk '{print $1}'; } + +# Identifies a directly configured supported CLI. Wrapper commands stay +# customizable through AGENT1_CMD and AGENT2_CMD, but engine-specific options +# cannot be injected into a wrapper without knowing its argument contract. +agent_command_engine() { + local executable + executable="$(agent_command_name "$1")" + case "${executable##*/}" in + claude|codex) printf '%s' "${executable##*/}" ;; + *) printf 'custom' ;; + esac +} + +# Removes quotes used to make Codex configuration values explicit TOML strings. +unquote_setting() { + local value="$1" + value="${value#\"}"; value="${value%\"}" + value="${value#\'}"; value="${value%\'}" + printf '%s' "$value" +} + +# Reads the last model or effort value in one directly configured CLI command. +# Agent command strings already use whitespace-delimited arguments, so this +# mirrors the splitting used when the command runs. +agent_command_setting() { + local cmd="$1" engine="$2" setting="$3" token value="" + local -a words + local i + read -r -a words <<< "$cmd" + for (( i = 0; i < ${#words[@]}; i++ )); do + token="${words[$i]}" + case "${engine}:${setting}:${token}" in + claude:model:--model|codex:model:-m|codex:model:--model) + i=$(( i + 1 )); value="${words[$i]:-}" ;; + claude:model:--model=*|codex:model:--model=*) + value="${token#*=}" ;; + claude:effort:--effort) + i=$(( i + 1 )); value="${words[$i]:-}" ;; + claude:effort:--effort=*) + value="${token#*=}" ;; + codex:effort:-c|codex:effort:--config) + if [[ "${words[$(( i + 1 ))]:-}" == model_reasoning_effort=* ]]; then + i=$(( i + 1 )) + value="${words[$i]#*=}" + fi ;; + codex:effort:--config=model_reasoning_effort=*) + value="${token#--config=model_reasoning_effort=}" ;; + esac + done + unquote_setting "$value" +} + +# Replaces one engine's model and effort flags without disturbing its safety +# flags. Codex rejects repeated --model arguments, so appending an override is +# not sufficient there. +configure_engine_command() { + local cmd="$1" engine="$2" model="$3" effort="$4" token + local -a words kept + local i + read -r -a words <<< "$cmd" + for (( i = 0; i < ${#words[@]}; i++ )); do + token="${words[$i]}" + if [[ -n "$model" ]]; then + case "${engine}:${token}" in + claude:--model|codex:-m|codex:--model) + i=$(( i + 1 )); continue ;; + claude:--model=*|codex:--model=*) continue ;; + esac + fi + if [[ -n "$effort" ]]; then + case "${engine}:${token}" in + claude:--effort) + i=$(( i + 1 )); continue ;; + claude:--effort=*) continue ;; + codex:-c|codex:--config) + if [[ "${words[$(( i + 1 ))]:-}" == model_reasoning_effort=* ]]; then + i=$(( i + 1 )); continue + fi ;; + codex:--config=model_reasoning_effort=*) continue ;; + esac + fi + kept+=("$token") + done + case "$engine" in + claude) + [[ -z "$model" ]] || kept+=(--model "$model") + [[ -z "$effort" ]] || kept+=(--effort "$effort") ;; + codex) + [[ -z "$model" ]] || kept+=(-m "$model") + [[ -z "$effort" ]] \ + || kept+=(-c "model_reasoning_effort=\"${effort}\"") ;; + esac + printf '%s' "${kept[*]}" +} + +# Restricts model values to one shell word. The CLIs remain authoritative for +# whether the installed version, account, and provider support that model. +require_model_value() { + local option="$1" value="$2" + [[ -n "$value" && "$value" =~ ^[A-Za-z0-9][A-Za-z0-9._:/@+%=-]*(\[1m\])?$ ]] \ + || { fail "${option} needs a model name without spaces, got '${value}'"; return 1; } +} + +# Effort names are CLI contracts rather than free-form model identifiers, so a +# typo can be rejected before the workflow creates a document or spends a turn. +require_effort_value() { + local option="$1" value="$2" + case "$option:$value" in + --claude-effort:low|--claude-effort:medium|--claude-effort:high|\ + --claude-effort:xhigh|--claude-effort:max|\ + --codex-effort:minimal|--codex-effort:low|--codex-effort:medium|\ + --codex-effort:high|--codex-effort:xhigh) return 0 ;; + --claude-effort:*) + fail "--claude-effort must be low, medium, high, xhigh, or max; got '${value}'" ;; + *) + fail "--codex-effort must be minimal, low, medium, high, or xhigh; got '${value}'" ;; + esac +} + +# Applies requested engine settings wherever that CLI currently sits. This is +# deliberately independent of agent1/agent2 so --swap-agents is order-neutral. +apply_engine_settings() { + local claude_found=0 codex_found=0 engine + engine="$(agent_command_engine "$AGENT1_CMD")" + case "$engine" in + claude) + claude_found=1 + AGENT1_CMD="$(configure_engine_command "$AGENT1_CMD" claude \ + "$CLAUDE_MODEL_OPTION" "$CLAUDE_EFFORT_OPTION")" ;; + codex) + codex_found=1 + AGENT1_CMD="$(configure_engine_command "$AGENT1_CMD" codex \ + "$CODEX_MODEL_OPTION" "$CODEX_EFFORT_OPTION")" ;; + esac + engine="$(agent_command_engine "$AGENT2_CMD")" + case "$engine" in + claude) + claude_found=1 + AGENT2_CMD="$(configure_engine_command "$AGENT2_CMD" claude \ + "$CLAUDE_MODEL_OPTION" "$CLAUDE_EFFORT_OPTION")" ;; + codex) + codex_found=1 + AGENT2_CMD="$(configure_engine_command "$AGENT2_CMD" codex \ + "$CODEX_MODEL_OPTION" "$CODEX_EFFORT_OPTION")" ;; + esac + if [[ -n "$CLAUDE_MODEL_OPTION$CLAUDE_EFFORT_OPTION" \ + && "$claude_found" -eq 0 ]]; then + die "Claude model or effort options require a direct 'claude' agent command" + fi + if [[ -n "$CODEX_MODEL_OPTION$CODEX_EFFORT_OPTION" \ + && "$codex_found" -eq 0 ]]; then + die "Codex model or effort options require a direct 'codex' agent command" + fi +} + +# Captures the effective setting of a directly configured engine. The markers +# contain characters that model values reject, so valid aliases such as +# Claude's `default` cannot be mistaken for driver metadata. +current_engine_setting() { + local wanted_engine="$1" setting="$2" command engine value opaque=0 + for command in "$AGENT1_CMD" "$AGENT2_CMD"; do + engine="$(agent_command_engine "$command")" + [[ "$engine" != "custom" ]] || opaque=1 + [[ "$engine" == "$wanted_engine" ]] || continue + value="$(agent_command_setting "$command" "$engine" "$setting")" + printf '%s' "${value:-(default)}" + return 0 + done + [[ "$opaque" -eq 0 ]] && printf '(unconfigured)' || printf '(custom)' +} + +# Loads saved selections on resume and rejects an explicit attempt to change +# them. Legacy documents have none of these fields and retain their historical +# AGENT1_CMD/AGENT2_CMD behavior. +prepare_saved_engine_settings() { + local doc="$1" key value current present=0 requested + local claude_model claude_effort codex_model codex_effort + claude_model="$(frontmatter "$doc" claude_model)" + claude_effort="$(frontmatter "$doc" claude_effort)" + codex_model="$(frontmatter "$doc" codex_model)" + codex_effort="$(frontmatter "$doc" codex_effort)" + for value in "$claude_model" "$claude_effort" "$codex_model" "$codex_effort"; do + [[ -z "$value" ]] || present=$(( present + 1 )) + done + if [[ "$present" -eq 0 ]]; then + requested="${CLAUDE_MODEL_OPTION}${CLAUDE_EFFORT_OPTION}" + requested="${requested}${CODEX_MODEL_OPTION}${CODEX_EFFORT_OPTION}" + [[ -z "$requested" ]] \ + || die "this task predates model options; resume without them or start a new task" + return 0 + fi + [[ "$present" -eq 4 ]] \ + || die "model metadata is incomplete in ${doc#"$REPO_ROOT"/}; inspect the document" + + case "$claude_model" in + '(default)'|'(custom)'|'(unconfigured)') ;; + *) require_model_value --claude-model "$claude_model" || exit "$EXIT_ERROR" ;; + esac + case "$claude_effort" in + '(default)'|'(custom)'|'(unconfigured)') ;; + *) require_effort_value --claude-effort "$claude_effort" || exit "$EXIT_ERROR" ;; + esac + case "$codex_model" in + '(default)'|'(custom)'|'(unconfigured)') ;; + *) require_model_value --codex-model "$codex_model" || exit "$EXIT_ERROR" ;; + esac + case "$codex_effort" in + '(default)'|'(custom)'|'(unconfigured)') ;; + *) require_effort_value --codex-effort "$codex_effort" || exit "$EXIT_ERROR" ;; + esac + + for key in claude_model claude_effort codex_model codex_effort; do + case "$key" in + claude_model) value="$claude_model"; current="$CLAUDE_MODEL_OPTION" ;; + claude_effort) value="$claude_effort"; current="$CLAUDE_EFFORT_OPTION" ;; + codex_model) value="$codex_model"; current="$CODEX_MODEL_OPTION" ;; + codex_effort) value="$codex_effort"; current="$CODEX_EFFORT_OPTION" ;; + esac + [[ -z "$current" || "$current" == "$value" ]] \ + || die "${key//_/-} differs from this task ('${current}' requested, "\ +"'${value}' recorded); start a new task to change model settings" + done + + [[ -n "$CLAUDE_MODEL_OPTION" || "$claude_model" == '(default)' \ + || "$claude_model" == '(custom)' || "$claude_model" == '(unconfigured)' ]] \ + || CLAUDE_MODEL_OPTION="$claude_model" + [[ -n "$CLAUDE_EFFORT_OPTION" || "$claude_effort" == '(default)' \ + || "$claude_effort" == '(custom)' || "$claude_effort" == '(unconfigured)' ]] \ + || CLAUDE_EFFORT_OPTION="$claude_effort" + [[ -n "$CODEX_MODEL_OPTION" || "$codex_model" == '(default)' \ + || "$codex_model" == '(custom)' || "$codex_model" == '(unconfigured)' ]] \ + || CODEX_MODEL_OPTION="$codex_model" + [[ -n "$CODEX_EFFORT_OPTION" || "$codex_effort" == '(default)' \ + || "$codex_effort" == '(custom)' || "$codex_effort" == '(unconfigured)' ]] \ + || CODEX_EFFORT_OPTION="$codex_effort" + apply_engine_settings +} + +# Exchanges the complete commands so their models, permissions, and flags move +# with their agents. Calling this more than once in one invocation has no effect. +swap_agent_commands() { + [[ "$AGENTS_SWAPPED" -eq 0 ]] || return 0 + local command="$AGENT1_CMD" + AGENT1_CMD="$AGENT2_CMD" + AGENT2_CMD="$command" + AGENTS_SWAPPED=1 +} + # Reads one frontmatter key from the document's leading `---` block. Only that # block is scanned, so a `status:` line quoted in the body cannot be mistaken # for state. @@ -269,6 +563,15 @@ section() { ' } +# The remote-tracking ref to measure and publish against. A missing ref is a +# setup error: falling back to a local branch would let an entire run finish +# before publication discovers that its recorded target is unavailable. +pr_base_ref() { + local ref="refs/remotes/origin/${PR_BASE_BRANCH}" + git -C "$REPO_ROOT" show-ref --verify --quiet "$ref" || return 1 + printf '%s' "$ref" +} + # Snapshot of everything the workflow forbids an agent from touching: the # checked-out commit and branch, every local branch and tag, every # remote-tracking ref, and the staged index. Remote-tracking refs are in here @@ -286,15 +589,264 @@ git_state() { git -C "$REPO_ROOT" ls-files -v -s } +# Content fingerprint of everything an agent could edit: tracked files as the +# worktree holds them, plus every untracked file Git does not ignore. Compared +# around `agent2`'s turns, which are read-only with respect to the codebase — +# the Git snapshot above cannot see an unstaged edit, and an unstaged edit is +# exactly what a reviewer that "just fixed it" would leave behind. +# +# `.agents/work/` is gitignored, so the working document, the transcripts, and +# the round snapshots — all of which change during a turn by design — are +# excluded by construction rather than by a list that could drift. +worktree_state() { + git -C "$REPO_ROOT" diff HEAD -- + ( + cd "$REPO_ROOT" || exit 1 + git ls-files --others --exclude-standard -z \ + | while IFS= read -r -d '' file; do + local mode object target + if [[ -L "$file" ]]; then + mode=120000 + target="$(readlink "$file")" + object="$(printf '%s' "$target" | git hash-object --stdin)" + else + [[ -x "$file" ]] && mode=100755 || mode=100644 + object="$(git hash-object -- "$file")" + fi + printf '%s %s %s\n' "$mode" "$object" "$file" + done + ) +} + +# Every `## ` heading outside fenced code, in document order. Fenced headings +# are content, exactly as they are for section_raw(). +section_names() { + awk ' + function marker(line, text) { + text = line + sub(/^[[:space:]]*/, "", text) + if (text ~ /^```/) { + match(text, /^`+/) + return substr(text, RSTART, RLENGTH) + } + if (text ~ /^~~~/) { + match(text, /^~+/) + return substr(text, RSTART, RLENGTH) + } + return "" + } + function closes(line, mark, text) { + text = line + sub(/^[[:space:]]*/, "", text) + text = substr(text, length(mark) + 1) + return text ~ /^[[:space:]]*$/ + } + { + mark = marker($0) + if (mark != "") { + if (fence == "") fence = mark + else if (substr(fence, 1, 1) == substr(mark, 1, 1) && + length(mark) >= length(fence) && closes($0, mark)) { + fence = "" + } + } + if (fence == "" && /^## /) { + name = $0 + sub(/^## /, "", name) + print name + } + } + ' "$1" +} + +# Whether the active turn may edit a section. Ownership alone is insufficient: +# it would let an agent rewrite its own completed rounds and alter the audit +# record later. The current state therefore opens only the sections that turn +# actually needs; every other section, including earlier ones by the same role, +# stays byte-identical. +section_is_mutable() { + local status="$1" plan_round="$2" impl_round="$3" name="$4" + local next_impl_round="$(( impl_round + 1 ))" + case "${status}|${name}" in + plan-requested\|Task|plan-requested\|Questions|plan-requested\|Plan) + return 0 ;; + plan-review-requested\|"Plan Review — Round ${plan_round}") + return 0 ;; + plan-reviewed\|Questions|plan-reviewed\|Plan|\ + plan-reviewed\|"Plan Dispositions — Round ${plan_round}"|\ + plan-reviewed\|"Implementation — Round ${impl_round}"|\ + plan-reviewed\|"Pull Request") + return 0 ;; + implementation-review-requested\|"Implementation Review — Round ${impl_round}") + return 0 ;; + implementation-reviewed\|Questions|\ + implementation-reviewed\|"Implementation — Round ${next_impl_round}"|\ + implementation-reviewed\|"Implementation Dispositions — Round ${impl_round}"|\ + implementation-reviewed\|Outcome|implementation-reviewed\|"Pull Request"|\ + implementation-reviewed\|"Manual Testing") + return 0 ;; + esac + return 1 +} + +# Digest of every section closed during this turn. Section names are included, +# so adding or deleting an unauthorized section is caught alongside editing it. +protected_sections() { + local doc="$1" status="$2" plan_round="$3" impl_round="$4" name + section_names "$doc" | while IFS= read -r name; do + if [[ "$name" == "Log" ]]; then continue; fi + if section_is_mutable "$status" "$plan_round" "$impl_round" "$name"; then + continue + fi + printf '%s=%s\n' "$name" "$(section_raw "$doc" "$name" | cksum)" + done +} + +# The non-blank entries in `## Log`, the one section both roles write. Comments +# are dropped so removing the template's placeholder does not read as rewriting +# another agent's entry. +log_entries() { + section "$1" "Log" | awk 'NF' +} + +verify_sections() { + local doc="$1" before="$2" who="$3" status="$4" plan_round="$5" + local impl_round="$6" now + now="$(protected_sections "$doc" "$status" "$plan_round" "$impl_round")" + [[ "$before" == "$now" ]] && return 0 + info "closed sections changed during ${who}'s turn (- before, + after):" + diff <(printf '%s\n' "$before") <(printf '%s\n' "$now") >&2 || true + die "${who} rewrote a section that is closed during '${status}'; "\ +"the document is left as written for you to inspect" +} + +verify_log_appended() { + local doc="$1" before="$2" who="$3" now added + now="$(log_entries "$doc")" + if [[ -z "$before" ]]; then + added="$now" + elif [[ "$now" == "$before"$'\n'* ]]; then + added="${now#"$before"$'\n'}" + else + added="" + fi + # Exactly one non-blank line is required. This rejects no-op turns and text + # appended to the end of the previous entry as well as ordinary rewrites. + [[ -n "$added" && "$added" != *$'\n'* ]] && return 0 + info "## Log before ${who}'s turn (- before, + after):" + diff <(printf '%s\n' "$before") <(printf '%s\n' "$now") >&2 || true + die "${who} must append exactly one new ## Log line without changing "\ +"earlier entries" +} + require_doc() { local doc="$1" [[ -f "$doc" ]] || die "no working document at ${doc} — run 'pair.sh start' first" } +# Adds the default PR target to a document created before that field existed. +# Unlike the starting branch and commit, this value is known: the workflow had +# only one target before the field was introduced. +backfill_pr_base_branch() { + local doc="$1" + [[ -n "$(frontmatter "$doc" pr_base_branch)" ]] && return 0 + + local tmp; tmp="$(mktemp "${doc}.XXXXXX")" + awk ' + NR == 1 && $0 == "---" { inside = 1; print; next } + inside && $0 == "---" { + print "pr_base_branch: master" + inside = 0 + } + { print } + ' "$doc" > "$tmp" && mv "$tmp" "$doc" \ + || { rm -f "$tmp"; die "could not backfill pr_base_branch in ${doc}"; } + info "backfilled pr_base_branch: master in ${doc#"$REPO_ROOT"/}" +} + +# Adds question provenance to a document created before that field existed. +# A document already paused on a question carries its origin in the legacy +# resume field; every other state starts with no active question. +backfill_question_origin() { + local doc="$1" + [[ -n "$(frontmatter "$doc" question_origin)" ]] && return 0 + + local origin=none status resume legacy=1 key + status="$(frontmatter "$doc" status)" + resume="$(frontmatter "$doc" resume_status)" + if [[ "$status" == "questions-pending" ]]; then + for key in claude_model claude_effort codex_model codex_effort; do + [[ -z "$(frontmatter "$doc" "$key")" ]] || legacy=0 + done + [[ "$legacy" -eq 1 ]] \ + || die "question_origin is missing from current-format "\ +"${doc#"$REPO_ROOT"/}; restore its recorded value before resuming" + case "$resume" in + plan-requested|plan-reviewed|implementation-reviewed) origin="$resume" ;; + *) die "cannot recover question_origin in ${doc#"$REPO_ROOT"/}: "\ +"legacy resume_status is '${resume:-empty}'; inspect the document before resuming" ;; + esac + fi + + local tmp; tmp="$(mktemp "${doc}.XXXXXX")" + awk -v origin="$origin" ' + NR == 1 && $0 == "---" { inside = 1; print; next } + inside && $0 == "---" { + print "question_origin: " origin + inside = 0 + } + { print } + ' "$doc" > "$tmp" && mv "$tmp" "$doc" \ + || { rm -f "$tmp"; die "could not backfill question_origin in ${doc}"; } + info "backfilled question_origin: ${origin} in ${doc#"$REPO_ROOT"/}" +} + +# Starting branch metadata matters only to publication. A legacy run may keep +# taking agent turns without it, but --create-pr must fail before the next turn: +# its original HEAD cannot be reconstructed safely after the fact. +validate_run_metadata() { + local doc="$1" + backfill_pr_base_branch "$doc" + backfill_question_origin "$doc" + [[ "$CREATE_PR" -eq 1 ]] || return 0 + + local key missing="" + for key in base_branch start_commit; do + [[ -n "$(frontmatter "$doc" "$key")" ]] \ + || missing="${missing:+${missing}, }${key}" + done + [[ -z "$missing" ]] && return 0 + + local number; number="$(frontmatter "$doc" issue_number)" + die "${doc#"$REPO_ROOT"/} predates metadata required by --create-pr "\ +"(missing: ${missing}); continue without --create-pr, or start a replacement "\ +"with '.agents/workflows/pair.sh start ${number:-} --slug '" +} + # Frontmatter the driver owns. An agent that rewrote these could retarget the # issue, move the review's diff baseline, or clear `dirty_at_start` and make a # worktree that was already dirty publishable. -readonly IMMUTABLE_KEYS="issue issue_number issue_title base_commit dirty_at_start max_rounds" +readonly IMMUTABLE_KEYS="issue issue_number issue_title agent1 agent2 claude_model "\ +"claude_effort codex_model codex_effort base_commit base_branch start_commit "\ +"pr_base_branch dirty_at_start max_rounds changeset_digest "\ +"reviewed_changeset_digest question_origin" + +# Prevents a resumed task from silently assigning its existing plan or review +# to different agents. The selected executables are fixed when `start` creates +# the document, and the caller repeats --swap-agents when that selection was +# swapped. +validate_agent_selection() { + local doc="$1" expected1 expected2 selected1 selected2 + expected1="$(frontmatter "$doc" agent1)" + expected2="$(frontmatter "$doc" agent2)" + selected1="$(agent_command_name "$AGENT1_CMD")" + selected2="$(agent_command_name "$AGENT2_CMD")" + if [[ "$expected1" != "$selected1" || "$expected2" != "$selected2" ]]; then + die "agent selection differs from this task (agent1=${expected1}, "\ +"agent2=${expected2}); use the same agent executables and --swap-agents choice "\ +"that created it" + fi +} immutable_snapshot() { local doc="$1" protect_task="$2" k @@ -376,6 +928,16 @@ release_lock() { [[ -n "$LOCK_DIR" ]] && rmdir "$LOCK_DIR" 2>/dev/null return 0 } + +# Releases the lock, restores the signal's default action, and terminates with +# that signal instead of letting the driver resume without mutual exclusion. +handle_signal() { + local signal="$1" + release_lock + trap - "$signal" + kill -s "$signal" "$$" +} + acquire_lock() { [[ -z "$LOCK_DIR" ]] || return 0 # already held by an outer command local slug="$1" lock dir @@ -385,7 +947,9 @@ acquire_lock() { mkdir "$lock" 2>/dev/null \ || die "another pair.sh is already running for '${slug}' (delete ${lock} if it is stale)" LOCK_DIR="$lock" - trap release_lock EXIT INT TERM + trap release_lock EXIT + trap 'handle_signal INT' INT + trap 'handle_signal TERM' TERM } # Accepts 123, #123, or a full GitHub issue URL, and yields the bare number. @@ -413,7 +977,9 @@ issue_number_from() { [[ -n "$host" && -n "$want" && "$path" == */issues/* ]] \ || { fail "'$1' is not a supported GitHub issue URL"; return 1; } if [[ "$host" != "$here_host" || "$want" != "$here" ]]; then - fail "that URL is for ${host}/${want}, but this repository is ${here_host}/${here}"; return 1 + fail "that URL is for ${host}/${want}, but this repository is "\ +"${here_host}/${here}" + return 1 fi raw="${path#*/issues/}" [[ "$raw" != */* && "$raw" != *\?* && "$raw" != *\#* ]] \ @@ -437,7 +1003,8 @@ require_safe_slug() { # The slug is joined onto WORK_ROOT, so `../` would place the working # document outside .agents/work entirely. [[ "$1" =~ ^[A-Za-z0-9][A-Za-z0-9._-]*$ ]] \ - || { fail "--slug must be a plain name (letters, digits, dot, dash, underscore), got '$1'"; return 1; } + || { fail "--slug must be a plain name (letters, digits, dot, dash, "\ +"underscore), got '$1'"; return 1; } } # A task enters this workflow only as a GitHub issue, so the slug is derived @@ -463,13 +1030,35 @@ resolve_slug() { cmd_start() { local issue_arg="${1:-}"; shift || true [[ -n "$issue_arg" ]] \ - || die "usage: pair.sh start [--slug ] [--max-rounds N]" + || die "usage: pair.sh start [--slug ] "\ +"[--max-rounds N] [--swap-agents] [model and effort options]" local slug="" max_rounds="2" while [[ $# -gt 0 ]]; do case "$1" in --slug) slug="${2:-}"; require_safe_slug "$slug" || exit "$EXIT_ERROR"; shift 2 ;; --allow-dirty) ALLOW_DIRTY=1; shift ;; + --swap-agents|--sa) swap_agent_commands; shift ;; + --claude-model) + CLAUDE_MODEL_OPTION="${2:-}" + require_model_value --claude-model "$CLAUDE_MODEL_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --claude-effort) + CLAUDE_EFFORT_OPTION="${2:-}" + require_effort_value --claude-effort "$CLAUDE_EFFORT_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --codex-model) + CODEX_MODEL_OPTION="${2:-}" + require_model_value --codex-model "$CODEX_MODEL_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --codex-effort) + CODEX_EFFORT_OPTION="${2:-}" + require_effort_value --codex-effort "$CODEX_EFFORT_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; --max-rounds|--mr) max_rounds="${2:-}" require_positive_int --max-rounds "$max_rounds" || exit "$EXIT_ERROR" @@ -479,6 +1068,8 @@ cmd_start() { esac done + apply_engine_settings + command -v gh >/dev/null 2>&1 \ || die "'gh' is not on PATH; it is required to read the issue" command -v jq >/dev/null 2>&1 \ @@ -528,7 +1119,31 @@ cmd_start() { || info "warning: issue #${number} has no obvious acceptance criteria; "\ "agent1 will block if it cannot find them" - local base_commit; base_commit="$(git -C "$REPO_ROOT" rev-parse --short HEAD)" + # Freeze a real remote PR target before spending an agent turn. Local + # branches may lag, carry unpushed commits, or merely hide a typo in + # PR_BASE_BRANCH; none describes what GitHub will compare the PR against. + local base_commit target_ref + target_ref="$(pr_base_ref)" \ + || die "origin/${PR_BASE_BRANCH} is unavailable; fetch it or correct "\ +"PR_BASE_BRANCH before starting the workflow" + base_commit="$(git -C "$REPO_ROOT" merge-base "$target_ref" HEAD)" \ + || die "cannot find a merge-base between HEAD and origin/${PR_BASE_BRANCH}" + base_commit="$(git -C "$REPO_ROOT" rev-parse --short "$base_commit")" + # The branch the run is cut from. `base_commit` cannot stand in for it here: + # it is deliberately the merge-base, so it says nothing about whether the + # starting point carried unmerged work. Recorded now because `create_pr` + # cannot recover it later, once HEAD has moved to the task branch. A + # detached HEAD records the commit, which reads correctly where it is used. + local base_branch; base_branch="$(git -C "$REPO_ROOT" rev-parse --abbrev-ref HEAD)" + [[ "$base_branch" != "HEAD" ]] \ + || base_branch="$(git -C "$REPO_ROOT" rev-parse --short HEAD)" + # Unlike `base_commit`, this is the exact immutable point from which the + # task starts. Publication uses it to distinguish the task's own version + # bump and ancestry from commits inherited from a parent branch. + local start_commit; start_commit="$(git -C "$REPO_ROOT" rev-parse HEAD)" + local carried + carried="$(git -C "$REPO_ROOT" rev-list --count \ + "${target_ref}..${start_commit}")" local now; now="$(date -u +%Y-%m-%dT%H:%M:%SZ)" # A dirty start is refused rather than merely noted. agent2 reviews every @@ -544,14 +1159,19 @@ cmd_start() { fi [[ "$dirty" == "no" ]] \ || info "warning: --allow-dirty — your existing changes are in the "\ -"review scope, and --create-pr will refuse to publish" +"review scope; publication is disabled for this task, so a later "\ +"run --create-pr will be refused" mkdir -p "$(dirname "$doc")" - # Record the agent binaries, not their flags: the names are documentation - # for whoever reads the document later, not something the driver reads back. - local a1 a2 - a1="$(printf '%s' "$AGENT1_CMD" | awk '{print $1}')" - a2="$(printf '%s' "$AGENT2_CMD" | awk '{print $1}')" + # Record executables for role assignment and engine settings independently, + # so swapping roles does not change what --claude-* or --codex-* means. + local a1 a2 claude_model claude_effort codex_model codex_effort + a1="$(agent_command_name "$AGENT1_CMD")" + a2="$(agent_command_name "$AGENT2_CMD")" + claude_model="$(current_engine_setting claude model)" + claude_effort="$(current_engine_setting claude effort)" + codex_model="$(current_engine_setting codex model)" + codex_effort="$(current_engine_setting codex effort)" # The body goes in verbatim from a file rather than through a substitution: # issue text routinely contains backslashes and ampersands, which awk's @@ -561,9 +1181,13 @@ cmd_start() { NUMBER="$number" TITLE="$title" ISSUE="$url" ROUNDS="$max_rounds" \ SLUG="$slug" BASE="$base_commit" NOW="$now" A1="$a1" A2="$a2" DIRTY="$dirty" \ + CLAUDEMODEL="$claude_model" CLAUDEEFFORT="$claude_effort" \ + CODEXMODEL="$codex_model" CODEXEFFORT="$codex_effort" \ + BASEBRANCH="$base_branch" STARTCOMMIT="$start_commit" \ + PRBASE="$PR_BASE_BRANCH" \ awk -v bodyfile="$body_file" ' /ISSUE_BODY/ { - # Demote the issue is own headings by one level so they nest under + # Demote headings from the issue by one level so they nest under # `## Issue` instead of colliding with the document sections that # section ownership is defined over. Lines inside fenced code are # left exactly as written. @@ -588,7 +1212,9 @@ cmd_start() { rest ~ /^[ \t]*$/) fence = "" } } - if (fence == "" && line ~ /^#/) line = "#" line + if (fence == "" && line ~ /^#/) { + line = (line ~ /^##/ ? "#" line : "##" line) + } print line } close(bodyfile) @@ -609,10 +1235,17 @@ cmd_start() { $0 = put($0, "ISSUE_TITLE", ENVIRON["TITLE"]) $0 = put($0, "ISSUE_URL", ENVIRON["ISSUE"]) $0 = put($0, "BASE_COMMIT", ENVIRON["BASE"]) + $0 = put($0, "PR_BASE_BRANCH", ENVIRON["PRBASE"]) + $0 = put($0, "BASE_BRANCH", ENVIRON["BASEBRANCH"]) + $0 = put($0, "START_COMMIT", ENVIRON["STARTCOMMIT"]) $0 = put($0, "CREATED_AT", ENVIRON["NOW"]) $0 = put($0, "TASK_SLUG", ENVIRON["SLUG"]) $0 = put($0, "AGENT1_NAME", ENVIRON["A1"]) $0 = put($0, "AGENT2_NAME", ENVIRON["A2"]) + $0 = put($0, "CLAUDE_MODEL", ENVIRON["CLAUDEMODEL"]) + $0 = put($0, "CLAUDE_EFFORT", ENVIRON["CLAUDEEFFORT"]) + $0 = put($0, "CODEX_MODEL", ENVIRON["CODEXMODEL"]) + $0 = put($0, "CODEX_EFFORT", ENVIRON["CODEXEFFORT"]) if ($0 ~ /^max_rounds: /) $0 = "max_rounds: " ENVIRON["ROUNDS"] if ($0 ~ /^dirty_at_start: /) $0 = "dirty_at_start: " ENVIRON["DIRTY"] print } @@ -622,6 +1255,10 @@ cmd_start() { info "created ${doc#"$REPO_ROOT"/} from issue #${number} at base ${base_commit}" info " ${title}" + [[ "$carried" -eq 0 ]] \ + || info "starting point '${base_branch}' at "\ +"$(git -C "$REPO_ROOT" rev-parse --short "$start_commit") carries ${carried} "\ +"commit(s) not in ${PR_BASE_BRANCH}; they are context, not this task's review scope" [[ "$STARTED_FROM_RUN" -eq 1 ]] \ || info "next: .agents/workflows/pair.sh run ${slug}" } @@ -776,11 +1413,17 @@ pr_body_is_usable() { area = "changes" next } + fence == "" && $0 == "### Reviewer notes" { + reviewer_notes++ + area = "other" + next + } fence == "" && /^### / { area = "other"; next } area == "summary" && /[^[:space:]]/ { summary_text = 1 } area == "changes" && /[^[:space:]]/ { changes_text = 1 } END { - exit !(summary == 1 && changes == 1 && summary_text && changes_text) + exit !(summary == 1 && changes == 1 && summary_text && changes_text && + reviewer_notes <= 1) } ' } @@ -823,6 +1466,70 @@ promote_pr_headings() { ' } +# Adds the driver's stacking paragraph to an agent-written Reviewer notes +# section, or creates that section when the agent did not provide one. Heading +# recognition ignores fenced Markdown examples, matching promote_pr_headings. +merge_reviewer_note() { + local note="$1" + NOTE="$note" awk ' + function marker(line, text) { + text = line + sub(/^[[:space:]]*/, "", text) + if (text ~ /^```/) { + match(text, /^`+/) + return substr(text, RSTART, RLENGTH) + } + if (text ~ /^~~~/) { + match(text, /^~+/) + return substr(text, RSTART, RLENGTH) + } + return "" + } + function closes(line, mark, text) { + text = line + sub(/^[[:space:]]*/, "", text) + text = substr(text, length(mark) + 1) + return text ~ /^[[:space:]]*$/ + } + function emit(line) { + print line + last_blank = (line == "") + } + function add_note() { + if (!last_blank) emit("") + emit(ENVIRON["NOTE"]) + emit("") + inserted = 1 + } + { + mark = marker($0) + if (mark != "") { + if (fence == "") fence = mark + else if (substr(fence, 1, 1) == substr(mark, 1, 1) && + length(mark) >= length(fence) && closes($0, mark)) { + fence = "" + } + } + if (fence == "" && in_notes && /^## / && + $0 != "## Reviewer notes") { + add_note() + in_notes = 0 + } + emit($0) + if (fence == "" && $0 == "## Reviewer notes") in_notes = 1 + } + END { + if (in_notes) { + add_note() + } else if (!inserted) { + if (!last_blank) emit("") + emit("## Reviewer notes") + add_note() + } + } + ' +} + # chordsVersion out of version.gradle.kts, from a file or from stdin, so the # working copy and the base commit's copy can be compared. version_from_stdin() { @@ -852,8 +1559,9 @@ dependency_headings_match() { /^# Dependencies of / { count++ suffix = ":" version "`" - if (length($0) < length(suffix) || - substr($0, length($0) - length(suffix) + 1) != suffix) bad = 1 + if (substr($0, length($0) - length(suffix) + 1) != suffix) { + bad = 1 + } } END { exit !(count > 0 && !bad) } ' "$file" @@ -871,6 +1579,65 @@ changeset_files() { } | sort -u } +# Content, type, and mode digest of the complete prospective changeset. +# Deliberately computed from the working files rather than from a patch, so +# committing does not change it: the digest recorded when the review finished +# must still match on a retry that has already committed part of the work. +changeset_digest() { + local base="$1" file path mode object target + changeset_files "$base" | while IFS= read -r file; do + path="${REPO_ROOT}/${file}" + if [[ -L "$path" ]]; then + mode=120000 + target="$(readlink "$path")" + object="$(printf '%s' "$target" | git -C "$REPO_ROOT" hash-object --stdin)" + printf '%s %s %s\n' "$mode" "$object" "$file" + elif [[ -f "$path" ]]; then + [[ -x "$path" ]] && mode=100755 || mode=100644 + object="$(git -C "$REPO_ROOT" hash-object -- "$file")" + printf '%s %s %s\n' "$mode" "$object" "$file" + else + printf 'deleted %s\n' "$file" + fi + done | cksum | tr -s ' ' '-' +} + +# Fingerprint of driver-owned per-round snapshots. They live in the writable, +# gitignored work directory, so the ordinary worktree guard cannot see them. +# Comparing this around every turn keeps an agent from rewriting the baseline +# that a later reviewer is told to trust. +rounds_state() { + local dir="$1" file + [[ -d "$dir" ]] || return 0 + for file in "$dir"/*; do + if [[ -L "$file" ]]; then + printf '%s=symlink:%s\n' "${file##*/}" "$(readlink "$file")" + elif [[ -f "$file" ]]; then + printf '%s=file:%s\n' "${file##*/}" "$(cksum < "$file")" + fi + done +} + +# The whole changeset as one patch, including files Git does not track yet — +# a new test file is exactly the kind of thing a later round must be able to +# see. `--no-index` exits 1 when it finds a difference, which is the normal +# case here. +changeset_patch() { + local base="$1" file + # A baseline that no longer resolves is create_pr's problem to report; this + # snapshot is a reviewing aid, and failing the handoff over it would abort + # a run that is otherwise fine. + git -C "$REPO_ROOT" cat-file -e "${base}^{commit}" 2>/dev/null || base="HEAD" + git -C "$REPO_ROOT" diff "$base" -- + ( + cd "$REPO_ROOT" || exit 1 + git ls-files --others --exclude-standard -z \ + | while IFS= read -r -d '' file; do + git diff --no-index --binary -- /dev/null "$file" || true + done + ) +} + # Branch name from the issue title: kebab-case, no agent identifiers, per the # branch-naming rule in AGENTS.md. branch_name_from() { @@ -887,6 +1654,8 @@ branch_name_from() { create_pr() { local doc="$1" local number title branch body + command -v gh >/dev/null 2>&1 \ + || die "'gh' is not on PATH; it is required to open the pull request" number="$(frontmatter "$doc" issue_number)" # The title recorded at setup, not a fresh read: a title edited mid-run # would derive a different branch name and strand the branch already @@ -922,18 +1691,67 @@ create_pr() { branch="$(branch_name_from "$title")" [[ -n "$branch" ]] || die "could not derive a branch name from the issue title" - local base changes dirty + local base start target_branch changes task_changes dirty base="$(frontmatter "$doc" base_commit)" git -C "$REPO_ROOT" cat-file -e "${base}^{commit}" 2>/dev/null \ || die "base_commit '${base}' is not available; cannot validate the PR changeset" + start="$(frontmatter "$doc" start_commit)" + git -C "$REPO_ROOT" cat-file -e "${start}^{commit}" 2>/dev/null \ + || die "start_commit '${start}' is not available; cannot validate where the task began" + target_branch="$(frontmatter "$doc" pr_base_branch)" + local base_ref="refs/remotes/origin/${target_branch}" + git -C "$REPO_ROOT" show-ref --verify --quiet "$base_ref" \ + || die "origin/${target_branch} is unavailable; fetch the pull request target and "\ +"start a new workflow run before publishing" + local target_base + target_base="$(git -C "$REPO_ROOT" merge-base "$base_ref" HEAD)" \ + || die "cannot find a merge-base between HEAD and origin/${target_branch}" + [[ "$(git -C "$REPO_ROOT" rev-parse "$base")" == "$target_base" ]] \ + || die "the recorded PR base ${base} is not the pull request's merge-base "\ +"${target_base}; start a new workflow run so the task and PR scopes are recalculated" + + # The first publication attempt must still stand exactly where the run + # started. Once the driver has created the task branch, retries may be ahead + # of that point only on that branch, because earlier publication steps may + # already have committed the reviewed files. + local current current_head + current="$(git -C "$REPO_ROOT" rev-parse --abbrev-ref HEAD)" + current_head="$(git -C "$REPO_ROOT" rev-parse HEAD)" + if [[ "$current" == "$branch" ]]; then + git -C "$REPO_ROOT" merge-base --is-ancestor "$start" HEAD \ + || die "task branch '${branch}' no longer descends from the recorded "\ +"starting commit ${start}; nothing was published" + else + [[ "$current_head" == "$(git -C "$REPO_ROOT" rev-parse "$start")" ]] \ + || die "HEAD moved from the recorded starting commit ${start} to "\ +"${current_head}; return to the starting point or start a new workflow run" + fi + changes="$(changeset_files "$base")" \ || die "could not determine the complete changeset since ${base}" + task_changes="$(changeset_files "$start")" \ + || die "could not determine the task changeset since ${start}" dirty="$(git -C "$REPO_ROOT" status --porcelain)" if [[ -z "$changes" ]]; then info "no changes since ${base}; skipping the pull request" return 0 fi + # Publication is bound to the changeset the review actually finished on. + # `git add -A` below stages whatever is in the worktree, so without this a + # retry after a failed push — or any edit made once the run reported + # `done` — would be committed and published as reviewed work. + local reviewed current_digest + reviewed="$(frontmatter "$doc" changeset_digest)" + current_digest="$(changeset_digest "$start")" + [[ -n "$reviewed" && "$reviewed" != "none" ]] \ + || die "no reviewed changeset is recorded in the document; the driver "\ +"writes it when the run reaches 'done', so this run has nothing to publish" + [[ "$reviewed" == "$current_digest" ]] \ + || die "the changeset has changed since the review finished "\ +"(${reviewed} -> ${current_digest}); nothing was published. Re-run the "\ +"workflow so the new state is reviewed, or open the pull request yourself" + # Everything below is checked before the first Git write, so a run that # cannot produce a policy-compliant PR fails without leaving a branch, # a commit, or a push behind. @@ -944,33 +1762,37 @@ create_pr() { body="$(section "$doc" "Pull Request")" pr_body_is_usable "$doc" \ || die "the ## Pull Request section requires exact, non-empty "\ -"### Summary and ### Changes sections; nothing was published" +"### Summary and ### Changes sections and at most one ### Reviewer notes "\ +"section; nothing was published" body="$(printf '%s\n' "$body" | promote_pr_headings)" - # Every PR must carry a version bump and regenerated reports, enforced by - # CI. Inspect the whole changeset, not only dirty files: a retry may already - # have committed the version and reports successfully. + # Every task must contribute its own version bump and regenerated reports. + # A stacked parent's copies are part of the PR changeset too, so compare + # against the exact starting commit rather than the merge-base with the PR + # target. A retry may already have committed these files successfully. local f absent="" for f in version.gradle.kts pom.xml dependencies.md; do - printf '%s\n' "$changes" | grep -qx "$f" \ + printf '%s\n' "$task_changes" | grep -qx "$f" \ || absent="${absent:+${absent}, }${f}" done [[ -z "$absent" ]] \ - || die "not in the changeset: ${absent}. AGENTS.md requires a version "\ -"bump and regenerated reports in every PR; nothing was published" + || die "not in the changeset after start_commit: ${absent}. AGENTS.md requires a "\ +"version bump and regenerated reports in every PR; nothing was published" - # "The file was touched" is not "the version went up". Compare against - # the commit the run started from, require the documented scheme, and - # validate the generated reports at their exact version-bearing locations. + # "The files were touched" does not prove that they are valid. Compare the + # version against the commit where the task started, require the documented + # scheme, and validate both generated reports at their version-bearing + # locations. local new_v old_v pom_v new_v="$(version_in_file "${REPO_ROOT}/version.gradle.kts")" - old_v="$(git -C "$REPO_ROOT" show "${base}:version.gradle.kts" 2>/dev/null \ + old_v="$(git -C "$REPO_ROOT" show "${start}:version.gradle.kts" 2>/dev/null \ | version_from_stdin || true)" [[ "$new_v" =~ ^2\.0\.0-SNAPSHOT\.[0-9]+$ ]] \ || die "chordsVersion is '${new_v}', which is not the "\ "2.0.0-SNAPSHOT. scheme; nothing was published" [[ "$old_v" =~ ^2\.0\.0-SNAPSHOT\.[0-9]+$ ]] \ - || die "could not read a valid chordsVersion at base commit ${base}; nothing was published" + || die "could not read a valid chordsVersion at start commit ${start}; "\ +"nothing was published" [[ "${new_v##*.}" -gt "${old_v##*.}" ]] \ || die "chordsVersion did not increase (${old_v} -> ${new_v}); "\ "the version-increment check would fail, so nothing was published" @@ -983,21 +1805,59 @@ create_pr() { || die "not every dependencies.md heading carries ${new_v}; "\ "regenerate the reports before publishing" - local current; current="$(git -C "$REPO_ROOT" rev-parse --abbrev-ref HEAD)" + # Stacking is determined from the immutable starting commit, never from the + # current value of a branch ref. The recorded branch remains a useful label + # if it is later moved, renamed, or deleted. The branch label does not + # decide whether earlier commits need a reviewer boundary; ancestry does, + # including when the run continues on an existing task branch. + local start_label stacked_on="" carried=0 start_short + start_label="$(frontmatter "$doc" base_branch)" + stacked_on="$start_label" + start_short="$(git -C "$REPO_ROOT" rev-parse --short "$start")" + if [[ -n "$stacked_on" ]]; then + carried="$(git -C "$REPO_ROOT" rev-list --count \ + "${base_ref}..${start}" 2>/dev/null || printf '0')" + [[ "$carried" -gt 0 ]] || stacked_on="" + else + stacked_on="" + fi + + # Where the task branch starts. Work stacked on earlier commits is the + # ordinary case, not an error: those commits ride along until the PR target + # contains them. A new task branch is cut from the exact HEAD recorded at + # setup; an existing task branch continues there. The PR still targets + # ${target_branch}. + # + # What is still refused is committing onto a branch that is not this + # task's. The new branch always starts at the recorded commit and every + # publication commit lands on it, so the branch the run was started from is + # never written to, whatever it is called. if [[ "$current" == "$branch" ]]; then info "already on '${branch}'" - elif [[ "$current" == "master" ]]; then - [[ -n "$dirty" ]] \ - || die "changes since ${base} are already committed on master; "\ -"move them to '${branch}' before publishing" - git -C "$REPO_ROOT" checkout -b "$branch" >/dev/null 2>&1 \ - || die "could not create branch '${branch}'" - info "created branch '${branch}'" else - die "on branch '${current}', which does not match this task; "\ -"switch to master or to '${branch}' and re-run" + # A clean worktree here means the changeset since ${base} is already + # committed — onto the branch the run started from, which is not this + # task's branch. Moving commits between branches is history rewriting + # by another name, so it is the user's call, not the driver's. + [[ -n "$dirty" ]] \ + || die "changes since ${base} are already committed on "\ +"'${current}'; move them to '${branch}' before publishing" + local checkout_error + if ! checkout_error="$(git -C "$REPO_ROOT" checkout -b \ + "$branch" "$start" 2>&1)"; then + die "could not create branch '${branch}': ${checkout_error}" + fi + info "created branch '${branch}' from '${start_label}' at ${start_short}" fi + # Say plainly what the PR will contain. A reviewer opening a stacked PR + # sees commits nobody in this run wrote, and the person who started the run + # should hear that from the driver rather than discover it on GitHub. + [[ -z "$stacked_on" ]] \ + || info "stacked on '${stacked_on}' at ${start_short}: the pull request "\ +"targets ${target_branch} and carries ${carried} earlier commit(s) that are not "\ +"ancestors of that target" + # Each step is skipped when already done, so a re-run after a failed push # or a failed `gh pr create` resumes instead of concluding there is nothing # left to publish. A clean worktree does not mean the work is published. @@ -1042,11 +1902,26 @@ create_pr() { return 0 fi - # AGENTS.md: draft, assigned to the author, base master, no trailing period - # in the title, no verification detail and no agent attribution in the body. + # A stacked PR shows commits from its parent branch, and a reviewer has no + # way to tell those from this task's work. AGENTS.md allows ## Reviewer + # notes for exactly this: material information the reviewer needs. It says + # nothing about verification and attributes nothing to an agent, so the + # rules on both stay intact. + if [[ -n "$stacked_on" ]]; then + local stacking_note + stacking_note="The workflow started from \`${stacked_on}\` at \ +\`${start_short}\`. That starting point contains ${carried} commit(s) that are \ +not ancestors of \`${target_branch}\`, so this pull request may also show them. \ +Review the task commits after \`${start_short}\`." + body="$(printf '%s\n' "$body" | merge_reviewer_note "$stacking_note")" + fi + + # AGENTS.md: draft, assigned to the author, base ${target_branch}, no + # trailing period in the title, no verification detail and no agent + # attribution in the body. body="${body}"$'\n\n'"Fixes #${number}" local url - url="$(gh pr create --draft --assignee @me --base master \ + url="$(gh pr create --draft --assignee @me --base "$target_branch" \ --title "${title%.}" --body "$body" 2>&1)" \ || die "gh pr create failed (the branch is pushed; re-run to retry just this step): ${url}" info "draft pull request: ${url}" @@ -1086,10 +1961,156 @@ unanswered_questions() { printf '%s' "$missing" } +# Review sections for one phase, in document order — `Plan Review …` or +# `Implementation Review …`, whatever round suffix the reviewer wrote. +phase_sections() { + section_names "$1" | awk -v want="$2" 'index($0, want) == 1' +} + +# The verdict a review section carries. Only the final non-blank line is +# machine-readable; mentions in findings or discussion cannot advance a round. +review_verdict() { + section "$1" "$2" | awk ' + NF { + semantic = $0 + gsub(/[[:space:]]/, "", semantic) + if (semantic !~ /^---+$/ && semantic !~ /^___+$/ && + semantic !~ /^\*\*\*+$/) last = $0 + } + END { + gsub(/[*_`]/, "", last) + sub(/^[[:space:]]+/, "", last) + sub(/[[:space:]]+$/, "", last) + last = toupper(last) + sub(/^VERDICT:[[:space:]]*/, "", last) + sub(/\.[[:space:]]*$/, "", last) + sub(/[[:space:]]+$/, "", last) + if (last == "APPROVE" || last == "APPROVE WITH CHANGES" || + last == "REQUEST CHANGES") print last + } + ' +} + +# Finding IDs declared at the start of review lines: P- for the plan, +# I- for the implementation. Prose and other rounds are ignored. +finding_ids() { + local doc="$1" review="$2" phase="$3" round="$4" + section "$doc" "$review" | awk -v want="${phase}${round}-" ' + { + line = $0 + sub(/^[[:space:]]*/, "", line) + sub(/^[-*][[:space:]]+/, "", line) + sub(/^#+[[:space:]]+/, "", line) + sub(/^[0-9]+[.)][[:space:]]+/, "", line) + sub(/^\|[[:space:]]*/, "", line) + gsub(/^[*_`]+/, "", line) + if (index(line, want) == 1) { + rest = substr(line, length(want) + 1) + if (match(rest, /^[0-9]+/)) { + print want substr(rest, RSTART, RLENGTH) + } + } + } + ' | sort -u +} + +# Valid finding IDs in one exact disposition table. A row is valid only when it +# has an allowed disposition and a non-empty note, and duplicate rows answer +# nothing. Prose mentions do not count. +valid_disposition_ids() { + section "$1" "$2" | awk -F'|' ' + function trim(value) { + sub(/^[[:space:]]+/, "", value) + sub(/[[:space:]]+$/, "", value) + return value + } + /^\|/ { + id = trim($2) + disposition = trim($3) + notes = trim($4) + if (id ~ /^[PI][0-9]+-[0-9]+$/) { + rows[id]++ + if (disposition ~ /^(Accepted|Rejected|Deferred)$/ && notes != "") { + valid[id]++ + } + } + } + END { + for (id in rows) if (rows[id] == 1 && valid[id] == 1) print id + } + ' +} + +# Findings from the round's review that its exact disposition table does not +# answer with one valid row. +undispositioned_findings() { + local doc="$1" review="$2" dispositions="$3" phase="$4" round="$5" + local id valid missing="" + valid="$(valid_disposition_ids "$doc" "$dispositions")" + for id in $(finding_ids "$doc" "$review" "$phase" "$round" || true); do + printf '%s\n' "$valid" | grep -qx "$id" \ + || missing="${missing:+${missing}, }${id}" + done + printf '%s' "$missing" +} + +# A review turn must leave a review. Each round writes its own section, so a +# reviewer cannot advance on the previous round's findings, and that section +# must carry content and one of the three verdicts. Without this the state +# machine accepts an empty review and the second opinion becomes a formality. +require_review() { + local doc="$1" prefix="$2" round="$3" rel_doc="$4" + local names count last="${prefix} — Round ${round}" + names="$(phase_sections "$doc" "$prefix" | awk 'NF')" + count="$(printf '%s' "$names" | grep -c . || true)" + [[ "$count" -eq "$round" ]] \ + || die "agent2 left ${count} '## ${prefix}' section(s) in ${rel_doc} at "\ +"round ${round}; every round records its own review" + [[ "$(section_heading_count "$doc" "$last")" -eq 1 ]] \ + || die "agent2 must write exactly one '## ${last}' section in ${rel_doc}" + [[ -n "$(section "$doc" "$last" | tr -d '[:space:]')" ]] \ + || die "'## ${last}' in ${rel_doc} is empty; a review that advances the "\ +"workflow has to say what was reviewed" + [[ -n "$(review_verdict "$doc" "$last")" ]] \ + || die "'## ${last}' in ${rel_doc} states no verdict; it must end with "\ +"APPROVE, APPROVE WITH CHANGES, or REQUEST CHANGES" +} + +# agent1's side of the same gate. Every finding needs a disposition before its +# phase can be left, and `REQUEST CHANGES` closes the forward move outright: +# the ways on from there are another round, `blocked`, or a question. A +# disposition table cannot overrule the verdict. +require_dispositions() { + local doc="$1" prefix="$2" disp="$3" forward="$4" new_status="$5" + local rel_doc="$6" round="$7" review dispositions missing verdict phase + review="${prefix} — Round ${round}" + dispositions="${disp} — Round ${round}" + [[ "$(section_heading_count "$doc" "$review")" -eq 1 ]] \ + || die "expected exactly one '## ${review}' section in ${rel_doc}" + [[ "$(section_heading_count "$doc" "$dispositions")" -eq 1 ]] \ + || die "agent1 must write exactly one '## ${dispositions}' section in ${rel_doc}" + case "$prefix" in + "Plan Review") phase=P ;; + "Implementation Review") phase=I ;; + *) die "internal: unknown review phase '${prefix}'" ;; + esac + missing="$(undispositioned_findings \ + "$doc" "$review" "$dispositions" "$phase" "$round")" + [[ -z "$missing" ]] \ + || die "agent1 left findings from '## ${review}' undispositioned in "\ +"${rel_doc}: ${missing}; every finding ID needs one valid row in '## ${dispositions}'" + verdict="$(review_verdict "$doc" "$review")" + if [[ "$verdict" == "REQUEST CHANGES" && "$new_status" == "$forward" ]]; then + die "'## ${review}' ends with REQUEST CHANGES, so '${forward}' is not "\ +"available from here; revise and spend a round, or set blocked" + fi + return 0 +} + # Builds the per-turn prompt. Deliberately thin: the protocol lives in the # skill, and every invocation is a cold start that reads it fresh. prompt_for() { - local role="$1" status="$2" rel_doc="$3" + local role="$1" status="$2" rel_doc="$3" previous="${4:-}" cat < "${rounds_dir}/plan-${plan_round}.md" + [[ "$plan_round" -le 1 ]] \ + || previous="${rounds_dir}/plan-$(( plan_round - 1 )).md" ;; + implementation-review-requested) + changeset_patch "$review_base" \ + > "${rounds_dir}/impl-${impl_round}.patch" + set_frontmatter "$doc" \ + "reviewed_changeset_digest=$(changeset_digest "$review_base")" + [[ "$impl_round" -le 1 ]] \ + || previous="${rounds_dir}/impl-$(( impl_round - 1 )).patch" ;; + esac + [[ -z "$previous" || -f "$previous" ]] || previous="" + previous="${previous#"$REPO_ROOT"/}" + local before after git_before git_after immutable_before protect_task=1 # Task is established during plan-requested, including after a question # round-trip. Its heading remains unique then, but its contents become @@ -1276,22 +2360,41 @@ take_turn() { before="$(cksum < "$doc")" git_before="$(git_state)" immutable_before="$(immutable_snapshot "$doc" "$protect_task")" + local sections_before log_before worktree_before="" rounds_before + sections_before="$(protected_sections \ + "$doc" "$status" "$plan_round" "$impl_round")" + log_before="$(log_entries "$doc")" + rounds_before="$(rounds_state "$rounds_dir")" + [[ "$turn" != "agent2" ]] || worktree_before="$(worktree_state)" # Keep a transcript per turn. An unattended run that goes wrong overnight # is otherwise unreconstructable: the document records what an agent chose # to write down, not what it actually did. local turns_dir="$(dirname "$doc")/turns" mkdir -p "$turns_dir" - local n; n="$(find "$turns_dir" -name '*.log' | wc -l | tr -d ' ')" - local log; log="$(printf '%s/%02d-%s.log' "$turns_dir" "$((n + 1))" "$turn")" + local n=0 candidate base sequence + for candidate in "$turns_dir"/*.log; do + [[ -e "$candidate" ]] || continue + base="${candidate##*/}" + sequence="${base%%-*}" + [[ "$sequence" =~ ^[0-9]+$ ]] || continue + if [[ "$sequence" -gt "$n" ]]; then + n="$sequence" + fi + done + n=$(( n + 1 )) + local log; log="$(printf '%s/%02d-%s.log' "$turns_dir" "$n" "$turn")" - # Word splitting on the command is intended: it carries its own flags. - # PIPESTATUS, not $?, because the pipe through tee would otherwise report - # tee's exit code and swallow a failed turn. + # Agent commands carry their own whitespace-delimited flags. Split them + # once into an array so model names such as `opus[1m]` stay literal instead + # of undergoing pathname expansion. PIPESTATUS, not $?, reports the agent + # rather than tee. local rc + local -a command_parts + read -r -a command_parts <<< "$cmd" set +e - # shellcheck disable=SC2086 - (cd "$REPO_ROOT" && $cmd "$(prompt_for "$turn" "$status" "$rel_doc")") 2>&1 | tee "$log" + (cd "$REPO_ROOT" && "${command_parts[@]}" \ + "$(prompt_for "$turn" "$status" "$rel_doc" "$previous")") 2>&1 | tee "$log" rc=${PIPESTATUS[0]} # Restore strict handling for every guard below. An intentional internal # non-zero result must disable it immediately before returning to the @@ -1326,6 +2429,30 @@ take_turn() { "the run can simply be started again" fi + # agent2 reviews the code; it never edits it. Checked only on its turns, + # since changing the worktree is the whole point of agent1's. Like the Git + # comparison above this detects rather than prevents, and it cannot say who + # moved — but a reviewer that edits what it is reviewing has ended the + # independence the second opinion is for, so the run stops either way. + if [[ "$turn" == "agent2" ]]; then + local worktree_after; worktree_after="$(worktree_state)" + if [[ "$worktree_before" != "$worktree_after" ]]; then + info "worktree content changed during ${turn}'s turn (- before, + after):" + diff <(printf '%s\n' "$worktree_before") \ + <(printf '%s\n' "$worktree_after") 2>&1 | head -n 40 >&2 || true + die "${turn} changed the code it was reviewing, which this workflow "\ +"forbids; inspect the worktree before continuing" + fi + fi + + local rounds_after; rounds_after="$(rounds_state "$rounds_dir")" + if [[ "$rounds_before" != "$rounds_after" ]]; then + info "round snapshots changed during ${turn}'s turn (- before, + after):" + diff <(printf '%s\n' "$rounds_before") \ + <(printf '%s\n' "$rounds_after") >&2 || true + die "${turn} rewrote a driver-owned review snapshot; inspect ${rounds_dir}" + fi + after="$(cksum < "$doc")" [[ "$before" != "$after" ]] \ || die "${turn} did not modify ${rel_doc}; aborting instead of looping" @@ -1392,6 +2519,34 @@ questions-pending|human|0|0" # the reviewer's diff scope both trust them. verify_immutable "$doc" "$immutable_before" "$turn" "$protect_task" + # Diagnose a mistyped review heading before section ownership reports it + # as an unexpected protected section. This gate applies only when the + # reviewer claims to have completed the requested review. + case "${status}|${new_status}" in + plan-review-requested\|plan-reviewed) + require_review "$doc" "Plan Review" "$plan_round" "$rel_doc" ;; + implementation-review-requested\|implementation-reviewed) + require_review "$doc" "Implementation Review" \ + "$impl_round" "$rel_doc" ;; + esac + + # Section ownership is what makes the document an audit record rather than + # a shared scratchpad. A turn writes its own sections; the other role's + # must come back byte-identical, and the shared log must only have grown. + verify_sections "$doc" "$sections_before" "$turn" \ + "$status" "$plan_round" "$impl_round" + verify_log_appended "$doc" "$log_before" "$turn" + + # Where a question resumes is decided when it is asked, not when it is + # answered, and it can only be the status the asking turn started from. + if [[ "$new_status" == "questions-pending" ]]; then + local new_resume; new_resume="$(frontmatter "$doc" resume_status)" + [[ "$new_resume" == "$status" ]] \ + || die "${turn} set resume_status to '${new_resume:-empty}' while "\ +"asking from '${status}'; a question resumes where it was asked" + set_frontmatter "$doc" "question_origin=${status}" + fi + # An agent that tries to spend a round after the configured ceiling has # reached the protocol's human-decision point. Convert that attempted # loopback into the documented terminal state instead of accepting it and @@ -1410,6 +2565,50 @@ questions-pending|human|0|0" return "$TURN_NEEDS_YOU" fi + # What the review said gates the move. The driver reads only the three + # fixed verdict tokens and the finding IDs the skill defines — judging the + # findings stays with the agents — but "the reviewer replied" and "agent1 + # answered every finding" are structural, and until now neither was + # required to advance. Skipped when a turn ends at `blocked` or a question: + # those are the states for a review that could not be completed. + case "${new_status}" in + blocked|questions-pending) ;; + *) + case "$status" in + plan-reviewed) + require_dispositions "$doc" "Plan Review" "Plan Dispositions" \ + "implementation-review-requested" "$new_status" "$rel_doc" \ + "$plan_round" ;; + implementation-reviewed) + require_dispositions "$doc" "Implementation Review" \ + "Implementation Dispositions" "done" "$new_status" "$rel_doc" \ + "$impl_round" + # `done` is the claim that the task is finished. An empty + # outcome leaves nobody able to say what shipped. + [[ "$new_status" != "done" \ + || -n "$(section "$doc" "Outcome" | tr -d '[:space:]')" ]] \ + || die "agent1 set 'done' with an empty ## Outcome in "\ +"${rel_doc}; it states what shipped, what was rejected, and the final "\ +"verification result" ;; + esac ;; + esac + + # Bind completion and publication to what agent2 actually reviewed. Agent1 + # may update dispositions and outcome on its final turn, but a source, + # mode, or type change requires another implementation-review round. + if [[ "$new_status" == "done" ]]; then + local reviewed_digest current_digest + reviewed_digest="$(frontmatter "$doc" reviewed_changeset_digest)" + current_digest="$(changeset_digest "$review_base")" + [[ -n "$reviewed_digest" && "$reviewed_digest" != "none" ]] \ + || die "agent1 set 'done' without a driver-recorded implementation review" + [[ "$reviewed_digest" == "$current_digest" ]] \ + || die "agent1 changed the implementation after agent2 reviewed it "\ +"(${reviewed_digest} -> ${current_digest}); increment impl_round and request "\ +"another implementation review instead of setting done" + set_frontmatter "$doc" "changeset_digest=${reviewed_digest}" + fi + info "advanced: ${status} -> ${new_status}" return 0 } @@ -1418,10 +2617,11 @@ questions-pending|human|0|0" unsafe_agent_roles() { local flagged="" case " $AGENT1_CMD " in - *--dangerously-*|*--yolo*|*bypassPermissions*) flagged="agent1" ;; + *--dangerously-*|*--yolo*|*bypassPermissions*|*danger-full-access*) + flagged="agent1" ;; esac case " $AGENT2_CMD " in - *--dangerously-*|*--yolo*|*danger-full-access*) + *--dangerously-*|*--yolo*|*bypassPermissions*|*danger-full-access*) flagged="${flagged:+${flagged} and }agent2" ;; esac printf '%s' "$flagged" @@ -1450,11 +2650,36 @@ cmd_step() { case "$1" in --accept-defaults|--ad) ACCEPT_DEFAULTS=1; shift ;; --allow-unsafe-agents) ALLOW_UNSAFE_AGENTS=1; shift ;; + --swap-agents|--sa) swap_agent_commands; shift ;; + --claude-model) + CLAUDE_MODEL_OPTION="${2:-}" + require_model_value --claude-model "$CLAUDE_MODEL_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --claude-effort) + CLAUDE_EFFORT_OPTION="${2:-}" + require_effort_value --claude-effort "$CLAUDE_EFFORT_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --codex-model) + CODEX_MODEL_OPTION="${2:-}" + require_model_value --codex-model "$CODEX_MODEL_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --codex-effort) + CODEX_EFFORT_OPTION="${2:-}" + require_effort_value --codex-effort "$CODEX_EFFORT_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; *) die "unknown option: $1" ;; esac done + require_doc "$(doc_for "$slug")" acquire_lock "$slug" + validate_run_metadata "$(doc_for "$slug")" + validate_agent_selection "$(doc_for "$slug")" + prepare_saved_engine_settings "$(doc_for "$slug")" validate_agent_permissions local rc ec=0 @@ -1491,10 +2716,35 @@ cmd_run() { --create-pr|--cp) CREATE_PR=1; shift ;; --allow-dirty) ALLOW_DIRTY=1; shift ;; --allow-unsafe-agents) ALLOW_UNSAFE_AGENTS=1; shift ;; + --swap-agents|--sa) swap_agent_commands; shift ;; + --claude-model) + CLAUDE_MODEL_OPTION="${2:-}" + require_model_value --claude-model "$CLAUDE_MODEL_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --claude-effort) + CLAUDE_EFFORT_OPTION="${2:-}" + require_effort_value --claude-effort "$CLAUDE_EFFORT_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --codex-model) + CODEX_MODEL_OPTION="${2:-}" + require_model_value --codex-model "$CODEX_MODEL_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; + --codex-effort) + CODEX_EFFORT_OPTION="${2:-}" + require_effort_value --codex-effort "$CODEX_EFFORT_OPTION" \ + || exit "$EXIT_ERROR" + shift 2 ;; *) die "unknown option: $1" ;; esac done + [[ "$CREATE_PR" -eq 0 || "$ALLOW_DIRTY" -eq 0 ]] \ + || die "--allow-dirty and --create-pr cannot be used together; a run "\ +"that includes pre-existing changes cannot publish them safely" + # Starting is not a separate decision from running — it is the first thing # a run needs. Create the document when it is missing so the common path is # one command, and leave `start` for when you want to read the issue copy @@ -1515,6 +2765,16 @@ cmd_run() { die "--max-rounds only applies when creating the document; '${slug}' already exists" fi + validate_run_metadata "$doc" + validate_agent_selection "$doc" + prepare_saved_engine_settings "$doc" + + if [[ "$CREATE_PR" -eq 1 \ + && "$(frontmatter "$doc" dirty_at_start)" != "no" ]]; then + die "the worktree was already dirty when this run started; publication "\ +"is disabled for this run, so review the result and publish it manually" + fi + # A full run is 1 planning turn + 2 per plan round + 2 per implementation # round + 1 terminal observation. Deriving the ceiling from max_rounds # keeps the loop guard from firing before the protocol's own round limit, diff --git a/.claude/commands/pair.md b/.claude/commands/pair.md index 0e8959e1..b1beda5e 100644 --- a/.claude/commands/pair.md +++ b/.claude/commands/pair.md @@ -2,7 +2,10 @@ description: > Run a GitHub issue through the two-agent pair workflow, relaying its questions, confirmations, and results here in the conversation. -argument-hint: " [--ad] [--mr N] [--cp] [--allow-unsafe-agents]" +argument-hint: >- + [--ad] [--mr N] [--cp] [--sa] [--allow-dirty] + [--claude-model MODEL] [--claude-effort LEVEL] [--codex-model MODEL] + [--codex-effort LEVEL] [--allow-unsafe-agents] [--max-turns N] allowed-tools: >- Read, Edit, AskUserQuestion, Bash(.agents/workflows/pair.sh:*), Bash(git status:*), Bash(git diff:*), Bash(git log:*), @@ -21,14 +24,16 @@ Your one exception is writing the user's answers into `## Questions`. ## Start -1. Read the issue number from `$ARGUMENTS`. Without one, ask for it and stop. +1. Read the issue number or URL from `$ARGUMENTS`. Without one, ask for it and + stop. 2. If `--cp` or `--create-pr` is present, say what it will do when the run finishes — new branch, commit, push, draft PR against `master` — and get a yes before starting. The flag is their instruction, but publishing is worth one confirmation while they are still at the keyboard. 3. If `--allow-unsafe-agents` is present, explain that an agent command removes its CLI approval or sandbox boundary. Confirm that the run is inside an - externally isolated, credential-free environment; otherwise stop. + externally isolated environment with no host mounts or unrelated + credentials; otherwise stop. 4. Start the run in the background, passing `$ARGUMENTS` through unchanged: ```bash @@ -62,16 +67,20 @@ that the change is unverified until they run it. write its `**A.**` line directly under that question, changing nothing else. Then ask the next unanswered question in a new message. Do not restart the driver until every question has an answer. Never invent an answer, answer - on the user's behalf, or bundle multiple questions into one message. + on the user's behalf, or bundle multiple questions into one message. Restart + with the original `$ARGUMENTS` so options such as `--sa` remain in effect, + but remove `--mr N` or `--max-rounds N`: the review limit is already stored, + and the driver accepts that option only while creating the task. - *Blocked.* Explain what blocked it and why, in your own words. If it is an unusable issue, say exactly what the issue is missing and offer to draft that text — but do not edit the issue yourself. If the two agents failed to converge, show both positions and ask how to proceed. **`1` — aborted.** Report the driver's message, say what it means, and propose -the fix. The Git guard tripping is worth flagging loudly: it means an agent -tried to write history, and the repository should be inspected before -continuing. +the fix. The Git guard tripping is worth flagging loudly: Git state changed +during an agent turn, and the repository should be inspected before +continuing. The driver cannot distinguish an agent write from a user's +simultaneous Git operation. ## Throughout diff --git a/.claude/settings.json b/.claude/settings.json index 0f9efd3e..3d43fc6e 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -18,8 +18,16 @@ "Bash(./gradlew generatePom mergeAllLicenseReports:*)", "Bash(./gradlew publishToMavenLocal:*)", "Bash(./gradlew publishCodegenPluginsToMavenLocal:*)", + "Bash(.agents/workflows/gradle-root.sh:*)", "Bash(java -version)", "Bash(java -XshowSettings:properties -version)", + "Bash(jenv version)", + "Bash(jenv versions)", + "Bash(jenv prefix:*)", + "Bash(git status:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + "Bash(git show:*)", "Bash(gh issue view:*)", "Bash(gh issue list:*)", "Bash(gh pr view:*)", diff --git a/.gitignore b/.gitignore index 3d123dba..b8200201 100644 --- a/.gitignore +++ b/.gitignore @@ -101,3 +101,6 @@ build # Pair-workflow scratch documents .agents/work/ + +# Claude Code workstation-specific permission grants. +.claude/settings.local.json diff --git a/AGENTS.md b/AGENTS.md index 19fd5454..632f12fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,14 +51,23 @@ this procedure. 1. **Confirm authorization.** Commit or push only when the current prompt explicitly asks for it, per "Commit and History Safety" above. 2. **Choose the branch.** - - If the current branch is `master`, create a new branch; never commit - directly to `master`. - - If the current branch name does not match the task, ask whether to branch - from the current branch or from `master` before committing. - If the current branch name matches the task, keep using it. + - Otherwise create a new branch from the current `HEAD`, whatever branch + that is. Never commit directly to `master`, and never commit onto a + branch that belongs to a different task — a new branch cut from the + current `HEAD` keeps the work off both. - Name new branches after the task, in the repository's kebab-case style (for example, `dialog-form-dirty-state`); do not include `codex` or other agent-specific identifiers in branches you create. + - **Stacked work is normal.** Starting from a branch whose own pull request + is still under review is the common case, not a mistake: the work depends + on changes that have not merged yet. Branch from it as above and target + `master` anyway (see "Creating a Pull Request"). Until the parent branch + merges, the new pull request also shows that branch's commits; GitHub + stops showing them once it merges. Do not wait for the parent, do not + rebase onto `master` to hide the commits, and do not ask which branch to + cut from — branching from the current `HEAD` is the answer in both the + stacked and the plain case. 3. **Check the version and reports.** Apply "Versioning and Reports" below: inspect the commits and local state, bump `chordsVersion` in `version.gradle.kts` if the changeset has not bumped it yet, and if the @@ -83,7 +92,9 @@ step 6). Creating a PR does not authorize additional verification; follow the verification rule in that section. 1. **Create it as a draft** (`gh pr create --draft`), targeting `master` as the - base branch unless the task specifies otherwise. + base branch unless the task specifies otherwise. This holds for stacked work + too: a branch cut from another unmerged branch still targets `master`, so + the pull request stays mergeable on its own once the parent merges. 2. **Assign it to the authenticated GitHub user** (`--assignee @me`). 3. **Omit a trailing period.** Do not end a pull request title with a period (`.`). @@ -94,6 +105,12 @@ verification rule in that section. verification, testing, build, or check information anywhere in the PR description. Do not add any agent-attribution section such as `Created by `. + - For stacked work, `## Reviewer notes` is material rather than optional: + name the branch and exact commit the work was cut from, explain that the + starting point contains commits outside the target branch, and tell the + reviewer to review the task commits after that boundary. Do not claim the + parent pull request is open or unmerged unless that state was verified. + Without the note, the extra commits read as part of this change. 5. **Link resolved issues.** For each issue the PR implements or fixes, add a GitHub closing keyword in the description (for example, `Fixes #123`) so the issue appears under "Successfully merging this pull request may close these @@ -162,12 +179,11 @@ from the repository root, without running the full build: ```bash find . -path '*/build/reports/dependency-license' -type d -prune \ -exec rm -rf {} + -./gradlew generatePom mergeAllLicenseReports +.agents/workflows/gradle-root.sh generatePom mergeAllLicenseReports ``` -On Apple Silicon workstations, prefix the Gradle command with -`JAVA_HOME="$(jenv prefix)"`; see "Apple Silicon Workstations" under -"Verification and Quality". +The wrapper selects and verifies the required JDK. See "Apple Silicon +Workstations" under "Verification and Quality". The `generatePom` task regenerates `pom.xml`, and `mergeAllLicenseReports` merges the per-module license reports into `dependencies.md`. Deleting the @@ -178,8 +194,8 @@ and the workflow failing. Afterwards, confirm that the `# Dependencies of ...` headings in `dependencies.md` carry the new version, and include both regenerated reports -in the changeset. A full `./gradlew build` regenerates the files as well, but -is unnecessary solely for this purpose. +in the changeset. A full `.agents/workflows/gradle-root.sh build` regenerates +the files as well, but is unnecessary solely for this purpose. Source files carry a copyright header; when modifying a file, keep the header year current (files touched in a given year carry that year). @@ -193,12 +209,13 @@ contracts are affected. Useful root commands (run from the repository root, JDK 11): ```bash -./gradlew ::test -./gradlew ::test --tests "io.spine.chords.proto.money.MoneyFieldSpec" -./gradlew ::check -./gradlew detekt -./gradlew clean build -./gradlew publishToMavenLocal +.agents/workflows/gradle-root.sh ::test +.agents/workflows/gradle-root.sh ::test \ + --tests "io.spine.chords.proto.money.MoneyFieldSpec" +.agents/workflows/gradle-root.sh ::check +.agents/workflows/gradle-root.sh detekt +.agents/workflows/gradle-root.sh clean build +.agents/workflows/gradle-root.sh publishToMavenLocal ``` ### Apple Silicon Workstations @@ -224,6 +241,16 @@ jenv shell 11 JAVA_HOME="$(jenv prefix)" ./gradlew ::check ``` +`.agents/workflows/gradle-root.sh` is that command behind one allowlistable +path. It resolves a registered JDK 11, verifies the version, refuses a +non-x86_64 JVM on macOS, and accepts only routine root verification, +Maven-local publication, and report-generation tasks. Prefer it for unattended +and ordinary root verification: + +```bash +.agents/workflows/gradle-root.sh ::check +``` + This architecture matters because some code generation paths in the pinned Spine toolchain resolve `io.grpc:protoc-gen-grpc-java:1.28.1`. That artifact provides a macOS x86_64 executable but no `osx-aarch_64` executable. Using an @@ -257,9 +284,13 @@ If verification cannot be run, state the reason clearly in the final response. ## Development Conventions - Use JDK 11 for the root project and JDK 17 for `codegen/plugins`. -- The supported environment is deliberately conservative: Kotlin 1.8.20, - Compose Multiplatform 1.5.12, Spine Event Engine 1.9.0, Gradle 6.9.4 for the - root project. Do not assume newer language or library features are available. +- The supported environment is deliberately conservative. The root build uses + the Kotlin Gradle plugin at 1.8.22 and forces production Kotlin libraries to + 1.9.23, while the supported consumer baseline remains Kotlin 1.8.20. Treat + Kotlin 1.8 as the language ceiling and do not introduce post-1.8.20 standard + library APIs without a deliberate compatibility decision. Compose + Multiplatform is 1.5.12, Spine Event Engine is 1.9.0, and root Gradle is + 6.9.4. - Kotlin explicit API mode is enabled: public declarations require explicit `public` modifiers. - Every declaration in project-owned source, including declarations explicitly diff --git a/PAIR_AGENTS_RUN_GUIDE.md b/PAIR_AGENTS_RUN_GUIDE.md index 5808fbc4..e06e423d 100644 --- a/PAIR_AGENTS_RUN_GUIDE.md +++ b/PAIR_AGENTS_RUN_GUIDE.md @@ -1,6 +1,6 @@ # Pair Agents Run Guide -Hand a GitHub issue to two AI agents. One plans and implements it, the other +Hand a GitHub issue to two AI agents. One plans and implements it; the other reviews — the plan before any code is written, and the diff afterward. You get back an uncommitted worktree and a record of what they agreed and disagreed on. @@ -14,48 +14,40 @@ two modules — anything where you want a review before you spend your own time reading a diff. **Poor fits:** an issue that is really a question or a discussion; anything -touching publishing credentials, workflow secrets, or the `config` submodule; -changes you would not let an agent make unattended, since that is what happens -(see [Safety](#safety)). +touching production deployment, infrastructure, credentials, or workflow +secrets; changes you would not let an agent make unattended, since that is +what happens (see [Safety](#safety)). ## Before You Start -Claude Code open in this repository, and four things on your `PATH`: `claude`, -`codex`, `gh` (run `gh auth status`), and `jq`. If one is missing, the run says -so and stops before doing anything. +### Required Tools -Being on `PATH` is not the same as being signed in, and the driver only checks -the former. `claude` and `codex` each hold their own credentials, so either can -be authenticated while the other is not. An unauthenticated CLI produces a turn -that dies immediately having written nothing — `Not logged in · Please run -/login` is the whole transcript. Nothing is lost when that happens: fix the -sign-in and run the same command again, and it resumes from the turn that -failed. +Run from the repository root with these tools installed and on `PATH`: -And an issue that makes two things clear: +- Git (`git`): install with `brew install git`; check with `git --version`. +- Claude Code (`claude`): install with `brew install --cask claude-code`; check + with `claude auth status` and sign in with `claude auth login`. +- Codex CLI (`codex`): install with `brew install --cask codex`; check with + `codex login status` and sign in with `codex login`. +- GitHub CLI (`gh`): install with `brew install gh`; check repository access + with `gh auth status` and sign in with `gh auth login`. +- `jq`: install with `brew install jq`; check with `jq --version`. -- **What to do, or what is wrong** — the functionality to add, or the - misbehavior, concrete enough to act on. -- **Acceptance criteria** — checkable conditions that settle when it is done. +If a check fails, install or sign in to that tool, then run the workflow command +again. An existing run resumes from the failed turn. -Headings do not matter; nothing looks for particular section names. This is the -one thing worth getting right, because the reviewer checks the work against -these criteria. If they are missing, the run stops on the first turn and tells -you what to add rather than inventing them. +### Prepare the GitHub Issue -A feature issue can be this short: +The GitHub issue must make two things clear: -```markdown -Add a copy-to-clipboard button to the validation error panel, so a user can -paste the full message into a bug report. +- **What to do, or what is wrong** — the functionality to add, or the + misbehavior, concrete enough to act on. +- **Acceptance criteria** — checkable conditions that settle when it is done. -Done when: -- Each error entry has a copy control that copies that entry's full text. -- The control is reachable by keyboard. -- A test covers the copied text matching the displayed message. -``` +Headings do not matter. If either part is missing, the first turn stops and +tells you what to add. -## Run It +## Run It with `/pair` In Claude Code, give it an issue number: @@ -63,69 +55,101 @@ In Claude Code, give it an issue number: /pair 150 ``` -That is the whole interface. Claude sets the run up, drives it through plan, -review, implementation, and review, and stays between you and it: when the -agents have a question it asks you here, writes your answer back for them, and -carries on. When the run finishes it reports what happened. You never open the -working file. +An issue URL works too. Claude Code operates the workflow, relaying questions +and the final result in the conversation. -An issue URL works in place of the number. +### Common Options -What you will hear back, in one of four shapes: +| Short form | Full form | Description | +|------------|-----------|-------------| +| `--ad` | `--accept-defaults` | Record and use planner defaults instead of asking questions. | +| `--mr N` | `--max-rounds N` | Set the review limit per phase for a new task; default: `2`. | +| `--cp` | `--create-pr` | Publish a finished run as a draft PR. | +| `--sa` | `--swap-agents` | Swap the implementer and reviewer. | +| — | `--claude-model MODEL` | Select the Claude Code model. | +| — | `--claude-effort LEVEL` | Select the Claude Code effort level. | +| — | `--codex-model MODEL` | Select the Codex model. | +| — | `--codex-effort LEVEL` | Select the Codex reasoning effort. | + +Possible results: - **Done.** Automated tests cover every acceptance criterion. Read the diff, then commit. - **Done, but it needs manual testing.** Claude gives you the plan. Work through it, then commit. -- **It has a question, or it is stuck.** Answer in the conversation, or decide. +- **Question or blocked.** Answer a question in the conversation; a blocked + run needs manual direction. - **Something went wrong.** Claude reports the error and what to do. Nothing is ever committed for you unless you ask — see [Opening a Pull Request](#opening-a-pull-request). -### Walking away +### Walking Away ``` /pair 150 --ad ``` -Stops the run pausing on questions: the planner takes the default it would have -proposed and carries on, recording each assumption for the reviewer to check. -Use it when nobody is watching. `--accept-defaults` is the same flag spelled -out. +The planner takes its proposed defaults instead of pausing for answers and +records every assumption for review. `--accept-defaults` is the long form. -### Without Claude Code +### Swap the Agents -The command is a wrapper. The workflow itself is a shell script you can run -from any terminal, with the same arguments: +The default assignments are: -```bash -.agents/workflows/pair.sh 150 +- `agent1`: Claude, the planner and implementer. +- `agent2`: Codex, the reviewer. + +Reverse them with: + +``` +/pair 150 --sa ``` -You then read its output yourself, and **run the same command again** whenever -something stops it — that is always the next step, and it resumes wherever it -left off. Exit codes are in [Reference](#reference). +Codex becomes the planner and implementer; Claude becomes the reviewer. +`--swap-agents` is the long form. -## When It Needs You +### Choose Models and Effort -Two things can interrupt a run, and Claude brings both to you here. +The defaults are Claude Opus 5 and GPT-5.6 Sol, both at high effort. Override +one or both engines when starting a task: -**Questions.** The planner hit something that changes what it builds and asked -rather than guessed — for example, *"Should the observation retry on failure?"* -Claude puts each question to you as a choice, with the agent's own proposal -marked as the default. It asks one question per message, records that answer, -then asks the next. Once all recorded questions have answers, the run continues -on its own. +``` +/pair 150 \ + --claude-model opus --claude-effort xhigh \ + --codex-model gpt-5.6-sol --codex-effort high +``` -**Blocked.** The task cannot proceed as written: the issue is unusable, the two -agents did not converge within the review rounds, or a call is genuinely yours. -Claude explains which and why. Usually you improve the issue and start again. +The driver records all four settings with the task and restores them on +resume. Start a new task to use different settings. `--swap-agents` changes +the engines' roles, not which settings belong to Claude and Codex. + +#### Possible Values + +- `--claude-model`: `default`, `best`, `opus`, `sonnet`, `haiku`, `opusplan`, + a supported `[1m]` variant such as `opus[1m]`, or a full model or provider + name accepted by Claude Code. See + [Claude Code model configuration](https://code.claude.com/docs/en/model-config). +- `--claude-effort`: `low`, `medium`, `high`, `xhigh`, or `max`. Support varies + by model; Claude Code may use the nearest supported level. +- `--codex-model`: `gpt-5.6-sol`, `gpt-5.6-terra`, `gpt-5.6-luna`, or another + model ID available to the installed Codex CLI and its provider. See + [Codex model selection](https://learn.chatgpt.com/docs/models). +- `--codex-effort`: `minimal`, `low`, `medium`, `high`, or `xhigh`. The pair + driver configures Codex's `model_reasoning_effort`, whose accepted values do + not include the app's Max or Ultra modes. + +Model availability depends on the installed CLI, account, and provider. The +driver rejects malformed names and unsupported effort values before setup; an +unavailable model is reported by its CLI at the first turn. -Running the script directly, both look the same but land in your terminal: the -questions print, and you write an `**A1.**` line under each one in the document -it names, then run the command again. Answering *is* the whole action — leave -one unanswered and it stops again and says which. +## When It Needs You + +- **Question:** Claude asks one question at a time and marks the agent's + proposed default. The run continues after all answers are recorded. +- **Blocked:** the issue is unusable, the agents did not converge, or a human + decision is required. Claude explains the blocker; the workflow does not + resume automatically. ## Opening a Pull Request @@ -136,200 +160,233 @@ and it publishes when it finishes: /pair 150 --cp ``` -Branch, commit, push, and a **draft** PR assigned to you, with `Fixes #150` in -the description. Claude confirms with you before starting a run that will -publish, and reports the PR URL at the end. - -The agents still never touch Git — the driver does this afterwards, once the -run has actually finished. A run that stopped for you or aborted publishes -nothing. - -Two conditions, both checked rather than assumed: +After the reviews finish, the driver creates a branch, commits, pushes, and +opens a **draft** PR assigned to you with `Fixes #150`. The agents never perform +Git writes themselves. An interruption before publication creates no Git +history; if publication fails partway through, the driver reports what +succeeded and a rerun resumes from there. -- **Your worktree must be clean when the run starts.** Otherwise the commit - would sweep up whatever you had in progress — and the reviewer would judge - your unrelated edits against the issue. Any run refuses to start on a dirty - worktree; `--allow-dirty` overrides that, at the cost of both, and disables - publishing for the run. -- **You must be on `master` or on the task's own branch.** On any other branch - it stops rather than committing somewhere you did not intend. +The task branch starts at the exact commit where the run began and targets +`master` by default. If the run started on a branch with commits not yet in +`master`, the driver reports that inherited history during setup, excludes it +from the agents' implementation review, and adds a `## Reviewer notes` section +for the human reviewer. Moving, renaming, or deleting the starting branch does +not change the review boundary. -`--create-pr` is the same flag spelled out. The PR is a draft on purpose: read -the diff before marking it ready. +`--create-pr` is the long form. Read the diff before marking the PR ready. ## When It Finishes -Without `--cp`, nothing was committed — the agents are not allowed near Git, so -what you have is an uncommitted worktree. +Claude reports the final outcome in the conversation and explains what, if +anything, still needs your attention. It also summarizes meaningful +disagreements between the agents and how they were resolved, so normal `/pair` +use does not require reading the workflow's internal documents. -1. Read the summary of what shipped and what was rejected — Claude reports it, - and `## Outcome` in the document holds the same thing. -2. Read the diff. -3. If manual testing was called for, work through the plan. Each step names the - acceptance criterion it covers. -4. Commit, following [`AGENTS.md`](AGENTS.md). +Without `--cp`, the changes remain uncommitted for you to review. With `--cp`, +Claude gives you the draft PR link instead. -Worth a look when something seems off: `## Implementation Dispositions` records -every review finding and whether the implementer accepted or rejected it, with -reasons. That is where the two agents actually disagreed. +1. Read Claude's outcome summary and inspect the diff. +2. If manual testing is required, follow the steps Claude provides. Each step + identifies the acceptance criterion it covers. +3. When you are satisfied with the result, commit the local changes or review + the draft PR before marking it ready. ## Safety -The default commands retain their CLI safety boundaries. Claude runs in -`acceptEdits` mode with project settings only, and Codex runs in its -`workspace-write` sandbox without loading user configuration. A non-interactive -turn stops if it needs an approval those modes cannot grant. Read the diff -before you commit even in this mode. - -If you supply an agent command that contains a known approval or sandbox bypass, -the driver refuses it unless `--allow-unsafe-agents` is present. That override -is only for an externally isolated, credential-free environment; the driver -does not create that environment for you. - -**The issue body is untrusted input.** Anyone who can file an issue can put -text in it. The skill tells both agents to treat `## Issue` as task data rather -than instructions, and to stop and ask if it contains directives. Keep the CLI -boundaries enabled; if you explicitly remove them, run only somewhere -disposable that holds no credentials. - -**They are told not to touch Git, and the run checks afterwards.** No branches, -commits, pushes, or pull requests; every ref and the index are compared after -each turn and the run aborts if anything moved. Be clear about what that is: -a tripwire, not a barrier. It runs after the fact, a change that is undone -again passes it, and effects outside this repository leave no local trace. -It tells you when the rule was broken; it cannot stop the breaking. - -It also cannot tell who did it. The check compares the repository before and -after a turn, and switching branches or committing in another window while a -run is live produces exactly the diff an offending agent would. So do neither -during a run — and if the guard trips and the diff is your own doing, that is -all it is: start the run again and it resumes from the turn that was cut off. - -`--cp` does not loosen any of this. That flag lets the *driver* publish once -the agents have finished. - -## How to Change Models and Efforts for Agents - -Both sides run a pinned model at high effort: **Claude Opus 5** plans and -implements, **GPT-5.6 Sol** reviews. They are pinned rather than left to each -CLI's default because the point of the workflow is that a particular second -model checked the work — a default that shifts under you quietly changes what -the review was worth. - -Each agent is a whole command line, held in an environment variable. To see the -current ones: +Claude runs in `acceptEdits` mode with project settings; Codex uses its +`workspace-write` sandbox without user configuration. The agent instructions +treat issue text as untrusted task data rather than as instructions. The driver +checks that: -```bash -.agents/workflows/pair.sh -``` +- agents do not change Git refs or the index; +- the reviewer does not change source files or saved review snapshots; +- completed document sections and log history remain unchanged; and +- every review has a verdict and every finding has one valid disposition. -**Copy one of those and edit it — do not write a command from scratch.** The -variable replaces the entire default, so anything you leave out is gone: drop -`--ignore-user-config` and your personal Codex config silently comes back; drop -`--permission-mode` and Claude's safety boundary changes; drop `--add-dir` and -the reviewer can no longer write the document the whole workflow runs on. Keep -every flag you are not deliberately changing. +The Git check is a tripwire, not a sandbox: it detects changes after a turn and +cannot identify who made them. Do not modify Git state in another window while +a run is active. -Set it for one run: +`--cp` changes only the driver's final publication step; it does not loosen +agent permissions or review checks. + +## Advanced Mode: Direct Script Usage + +Most users can stop here. Use the workflow driver directly only when Claude +Code is unavailable, you need one-step or status commands for automation, or +you need to customize agent commands, models, or publication settings. + +Run advanced commands from the repository root. + +### Run Directly + +Start or resume the same workflow from a terminal: ```bash -AGENT1_CMD="claude -p --permission-mode acceptEdits --setting-sources project --model sonnet --effort medium" \ - .agents/workflows/pair.sh 150 +.agents/workflows/pair.sh 150 ``` -Export the same line from your shell profile to make it permanent. Through -`/pair`, just say which model or effort you want in the message — a slash -command cannot carry an environment prefix. +If it stops, follow the message and run the command again. Keep saved options +such as `--sa`, but omit `--mr` or `--max-rounds`: the review limit is recorded +at setup, and those forms are accepted only when creating a task. -Swapping the two variables swaps the roles, so the reviewer becomes the planner. +When the script stops for questions, add an `**A.**` line under each +question in `.agents/work/issue-150/plan.md`, using the matching question +number, then run the command again. -### Claude Code — the planner and implementer +### Script Commands -Set with `AGENT1_CMD`, using `--model` and `--effort`. +- `.agents/workflows/pair.sh ` or + `.agents/workflows/pair.sh run ` — set up on the first call and resume + on later calls. +- `.agents/workflows/pair.sh status ` — report the current state; safe + during a run. +- `.agents/workflows/pair.sh step ` — attempt one workflow step, then + stop. +- `.agents/workflows/pair.sh start ` — set up without running. -- `--model` takes an alias for the current model in a family — `opus`, - `sonnet`, `haiku`, `fable` — or a full identifier such as `claude-opus-5`. - An alias follows the latest release; a full identifier stays put. Prefer the - full identifier when you want two runs months apart to be comparable. -- `--effort` takes `low`, `medium`, `high`, `xhigh`, or `max`. +### Script Options -Values are checked locally. A wrong effort prints a warning that lists the valid -values and falls back to the default, so a typo costs you nothing. +- `--ad`, `--accept-defaults` (`run`, `step`) — take proposed defaults instead + of asking. +- `--mr`, `--max-rounds N` (`run`, `start`) — allow up to `N` reviews each of + the plan and implementation. With the default `N=2`, each can be reviewed, + revised once, and reviewed again. Use it only when creating the task. +- `--cp`, `--create-pr` (`run`) — branch, commit, push, and open a draft PR + after a finished run. It is off by default. +- `--sa`, `--swap-agents` (`run`, `start`, `step`) — exchange the configured + planner/implementer and reviewer. Repeat it when resuming the task. +- `--claude-model MODEL`, `--claude-effort LEVEL` (`run`, `start`, `step`) — + select Claude Code's saved model and effort. +- `--codex-model MODEL`, `--codex-effort LEVEL` (`run`, `start`, `step`) — + select Codex's saved model and reasoning effort. +- `--allow-dirty` (`run`, `start`) — include existing worktree changes when + creating the task. It cannot be combined with `--create-pr`. +- `--allow-unsafe-agents` (`run`, `step`) — permit configured commands that + bypass approvals or sandboxing. External isolation is required. +- `--max-turns N` (`run`) — override the derived loop guard. Normally leave it + unset. +- `--slug ` (`start`) — use a working-directory name other than + `issue-`. Resume it with the explicit form + `.agents/workflows/pair.sh run `; the bare issue shortcut accepts + GitHub issues only. + +The driver rejects agent commands that bypass approvals or sandboxing unless +you pass `--allow-unsafe-agents`. Use that option only in a disposable +container or VM whose outer isolation replaces the CLI sandbox, with no host +mounts or unrelated credentials. It is not a shortcut for suppressing +approvals on a development workstation. + +### Exit Codes + +Exit codes for scripting `.agents/workflows/pair.sh run`: + +| Code | Meaning | +|------|---------| +| `0` | Done | +| `1` | Aborted | +| `2` | Done, but needs manual testing | +| `3` | Stopped for you | + +The `step` command uses the same codes, except that `0` means the step +completed without an error, not that the task is done. Check +`.agents/workflows/pair.sh status ` when scripting around `step`. + +### Publication Settings and Guards + +For a repository whose pull requests target another branch, set +`PR_BASE_BRANCH` when creating the run. The driver records that value in the +working document; omitting or changing the environment variable on a later +invocation does not retarget the pull request. + +Before the first agent turn of a publishing run, the driver requires the +remote-tracking PR target to exist and have a merge-base with `HEAD`. Before +its first Git write, it also requires: + +- a clean worktree when the run starts (`--allow-dirty` cannot be combined + with `--create-pr`); +- the changeset uncommitted, unless it is already on the task's own branch — + work committed onto some other branch is left for you to move; +- an `origin/` merge-base matching the recorded PR baseline; +- `HEAD` still at the recorded starting commit on the first publication + attempt, or on the task branch for a retry; +- content, file types, and executable bits identical to the reviewed state; +- a `version.gradle.kts` increase made after the recorded starting commit, + regenerated `pom.xml` and `dependencies.md` reports, and complete `Summary` + and `Changes` sections; and +- an actionable plan when manual testing is required. + +A target-branch update that changes the PR merge-base stops publication; the +run must be repeated against the new scope. + +### Override Complete Agent Commands + +Use the `/pair` model and effort options for ordinary selection. Override a +complete command only to change its executable or other CLI flags. Print both +defaults with: -`--setting-sources project` is what lets the implementer verify its own work. -It loads `.claude/settings.json` and nothing else — not your personal -settings, and not `.claude/settings.local.json`. The Gradle and `java` commands -[`AGENTS.md`](AGENTS.md) prescribes are allowed there for exactly this reason. -A command missing from that file is refused before it starts, and the run -continues to a review of code that was never compiled. If you add a -verification command the workflow should be able to run, add it there rather -than to your local settings. +```bash +.agents/workflows/pair.sh +``` -### Codex — the reviewer +Copy the complete command and change only the intended settings. Without +`--sa`, the variables map to these default roles: -Set with `AGENT2_CMD`, using `-m` for the model and `-c key="value"` for the -rest. +| Default role | Variable | Model and effort flags | +|--------------|----------|------------------------| +| Planner and implementer | `AGENT1_CMD` | `--model`, `--effort` | +| Reviewer | `AGENT2_CMD` | `-m`, `-c model_reasoning_effort=` | -- `-m` takes a model identifier, for example `gpt-5.6-sol`. -- `-c model_reasoning_effort=` takes `minimal`, `low`, `medium`, or `high`. -- `-c service_tier=` takes `default` for standard speed. +Set it for one run: -These are passed as flags rather than read from `~/.codex/config.toml`, because -the workflow runs Codex with `--ignore-user-config` so a review does not change -with local configuration. +```bash +AGENT1_CMD='claude -p --permission-mode acceptEdits --setting-sources project '\ +'--model sonnet --effort medium' \ + .agents/workflows/pair.sh 150 +``` -`--add-dir` is not optional. Codex's sandbox refuses to write gitignored paths, -and `.agents/work/` — where the working document lives — is gitignored on -purpose, because the document is scratch and is never committed. Without that -flag the reviewer reads the plan, forms its findings, and then cannot write -them down; the run ends with `agent2 did not modify … plan.md`. +Export the variable to use it for the current shell session and child +processes. The slash command cannot set environment variables: export them +before starting Claude Code, or run the driver directly with the assignment as +shown above. To exchange the configured roles without rewriting the variables, +pass `--sa` or `--swap-agents`. -**Codex does not check these values locally.** An unrecognised effort is -accepted, echoed in the run header, and then rejected by the API — so a typo -surfaces as a failed reviewer turn rather than as a configuration error. If a -first reviewer turn dies for no obvious reason, check the `reasoning effort` -line in `.agents/work/issue-/turns/02-agent2.log`. +Keep all safety flags from the printed command. In particular: -### One thing to know +- Claude needs `--setting-sources project` to load the verification allowlist + from `.claude/settings.json`. +- Codex needs `--ignore-user-config` for reproducible settings and `--add-dir` + to write the gitignored working document. -The working document records only which CLI ran, not which model or effort. A -run at `minimal` and a run at `high` leave artifacts that look identical -afterwards, so note it yourself if you are comparing runs. +If a model or effort is rejected, read the relevant turn log under +`.agents/work/issue-/turns/`. The document records settings that the +driver can read from direct Claude and Codex commands. An engine hidden by an +opaque wrapper is recorded as `(custom)`; an engine absent because both +commands directly identify the other engine is `(unconfigured)`. Note a +wrapper's hidden settings separately when comparing runs. The `/pair` model +options require a direct `claude` or `codex` command; configure a wrapper's +model internally instead. -## Reference +### Working Files and Legacy Runs Everything for a task lives in `.agents/work/issue-150/` (gitignored): -`plan.md` is the shared document, `turns/*.log` the transcript of each turn. -The document is what an agent chose to write down; the transcripts are what it -actually did. - -- `/pair ` — the normal entry point. Claude Code relays questions and - results in the conversation. -- `pair.sh ` — the same run from a terminal. It sets up on the first call - and resumes on later calls. -- `pair.sh status ` — report current state; safe during a run. -- `pair.sh step ` — take one turn, then stop. -- `pair.sh start ` — set up without running. - -Exit codes, for scripting `pair.sh run`: `0` done · `1` aborted · `2` done but -needs manual testing · `3` stopped for you. `step` uses the same codes, except -that `0` there means "the turn was taken", which may or may not have finished -the task — check `status` if you are scripting around it. - -- `--ad`, `--accept-defaults` (`run`, `step`) — take proposed defaults instead - of asking. -- `--mr`, `--max-rounds N` (`run`, `start`) — allow `N` review rounds in each - phase, and therefore at most `N - 1` send-backs. The default is `2`. -- `--cp`, `--create-pr` (`run`) — branch, commit, push, and open a draft PR - after a finished run. It is off by default. -- `--allow-dirty` (`run`, `start`) — include existing worktree changes in the - review scope. Publication is refused. -- `--allow-unsafe-agents` (`run`, `step`) — permit configured commands that - bypass approvals or sandboxing. External isolation is required. - -To change models or efforts, swap which agent does what, or narrow an agent's -permissions, set `AGENT1_CMD` and `AGENT2_CMD` — see -[How to Change Models and Efforts for Agents](#how-to-change-models-and-efforts-for-agents). +`plan.md` is the shared document, `turns/*.log` contains turn transcripts, and +`rounds/` holds the plan or changeset saved for each review. The document is +what an agent chose to write down; the transcripts are what it actually did. +For a detailed record of disagreements, read `## Plan Dispositions` and +`## Implementation Dispositions` in `plan.md`. + +For working documents created by an older driver, the missing PR target is +backfilled as `master`, and missing question provenance is backfilled before +another turn. If the document is already waiting on a question, the driver +recovers the origin from its saved legacy resume status. A non-publishing run +may continue without the older starting-branch or model fields. New model +options cannot be added to such a run. Publication still requires the +starting-branch fields; if they are missing, continue without `--create-pr` or +start a replacement with +`.agents/workflows/pair.sh start --slug `. + +### Protocol Reference The protocol the agents follow is [`.agents/skills/pair-workflow/SKILL.md`](.agents/skills/pair-workflow/SKILL.md). From 70ffedad2f7874269d05739b6783751d0f49ec2f Mon Sep 17 00:00:00 2001 From: Oleg-Melnik Date: Wed, 5 Aug 2026 18:25:25 +0300 Subject: [PATCH 2/5] =?UTF-8?q?Bump=20version=20=E2=80=94>=20`2.0.0-SNAPSH?= =?UTF-8?q?OT.107`.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dependencies.md | 24 ++++++++++++------------ pom.xml | 2 +- version.gradle.kts | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/dependencies.md b/dependencies.md index b90ffec6..c5e1ab96 100644 --- a/dependencies.md +++ b/dependencies.md @@ -1,6 +1,6 @@ -# Dependencies of `io.spine.chords:spine-chords-client:2.0.0-SNAPSHOT.106` +# Dependencies of `io.spine.chords:spine-chords-client:2.0.0-SNAPSHOT.107` ## Runtime 1. **Group** : cafe.adriel.voyager. **Name** : voyager-core. **Version** : 1.0.1.**No license information found** @@ -1104,12 +1104,12 @@ The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Wed Aug 05 12:36:09 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). +This report was generated on **Wed Aug 05 18:24:24 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). -# Dependencies of `io.spine.chords:spine-chords-codegen-tests:2.0.0-SNAPSHOT.106` +# Dependencies of `io.spine.chords:spine-chords-codegen-tests:2.0.0-SNAPSHOT.107` ## Runtime 1. **Group** : com.google.code.findbugs. **Name** : jsr305. **Version** : 3.0.2. @@ -1899,12 +1899,12 @@ This report was generated on **Wed Aug 05 12:36:09 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Wed Aug 05 12:36:10 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). +This report was generated on **Wed Aug 05 18:24:26 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). -# Dependencies of `io.spine.chords:spine-chords-core:2.0.0-SNAPSHOT.106` +# Dependencies of `io.spine.chords:spine-chords-core:2.0.0-SNAPSHOT.107` ## Runtime 1. **Group** : cafe.adriel.voyager. **Name** : voyager-core. **Version** : 1.0.1. @@ -2938,12 +2938,12 @@ This report was generated on **Wed Aug 05 12:36:10 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Wed Aug 05 12:36:11 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). +This report was generated on **Wed Aug 05 18:24:27 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). -# Dependencies of `io.spine.chords:spine-chords-proto:2.0.0-SNAPSHOT.106` +# Dependencies of `io.spine.chords:spine-chords-proto:2.0.0-SNAPSHOT.107` ## Runtime 1. **Group** : cafe.adriel.voyager. **Name** : voyager-core. **Version** : 1.0.1.**No license information found** @@ -3976,12 +3976,12 @@ This report was generated on **Wed Aug 05 12:36:11 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Wed Aug 05 12:36:12 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). +This report was generated on **Wed Aug 05 18:24:29 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). -# Dependencies of `io.spine.chords:spine-chords-proto-values:2.0.0-SNAPSHOT.106` +# Dependencies of `io.spine.chords:spine-chords-proto-values:2.0.0-SNAPSHOT.107` ## Runtime 1. **Group** : com.google.code.findbugs. **Name** : jsr305. **Version** : 3.0.2. @@ -4775,12 +4775,12 @@ This report was generated on **Wed Aug 05 12:36:12 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Wed Aug 05 12:36:13 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). +This report was generated on **Wed Aug 05 18:24:30 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). -# Dependencies of `io.spine.chords:spine-chords-runtime:2.0.0-SNAPSHOT.106` +# Dependencies of `io.spine.chords:spine-chords-runtime:2.0.0-SNAPSHOT.107` ## Runtime 1. **Group** : com.google.code.findbugs. **Name** : jsr305. **Version** : 3.0.2. @@ -5544,4 +5544,4 @@ This report was generated on **Wed Aug 05 12:36:13 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Wed Aug 05 12:36:14 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). \ No newline at end of file +This report was generated on **Wed Aug 05 18:24:31 EEST 2026** using [Gradle-License-Report plugin](https://github.com/jk1/Gradle-License-Report) by Evgeny Naumenko, licensed under [Apache 2.0 License](https://github.com/jk1/Gradle-License-Report/blob/master/LICENSE). \ No newline at end of file diff --git a/pom.xml b/pom.xml index af0bd7fb..7cd0e8db 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ all modules and does not describe the project structure per-subproject. --> io.spine.chords Chords -2.0.0-SNAPSHOT.106 +2.0.0-SNAPSHOT.107 2015 diff --git a/version.gradle.kts b/version.gradle.kts index 75bce9dc..345beda7 100644 --- a/version.gradle.kts +++ b/version.gradle.kts @@ -27,4 +27,4 @@ /** * The version of all Chords libraries. */ -val chordsVersion: String by extra("2.0.0-SNAPSHOT.106") +val chordsVersion: String by extra("2.0.0-SNAPSHOT.107") From 12f0e3f605040a865878b4a326a5d172931b56ed Mon Sep 17 00:00:00 2001 From: Oleg-Melnik Date: Wed, 5 Aug 2026 18:40:50 +0300 Subject: [PATCH 3/5] Present pair workflow prerequisites as a table. --- PAIR_AGENTS_RUN_GUIDE.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/PAIR_AGENTS_RUN_GUIDE.md b/PAIR_AGENTS_RUN_GUIDE.md index e06e423d..8cd34bd0 100644 --- a/PAIR_AGENTS_RUN_GUIDE.md +++ b/PAIR_AGENTS_RUN_GUIDE.md @@ -24,14 +24,16 @@ what happens (see [Safety](#safety)). Run from the repository root with these tools installed and on `PATH`: -- Git (`git`): install with `brew install git`; check with `git --version`. -- Claude Code (`claude`): install with `brew install --cask claude-code`; check - with `claude auth status` and sign in with `claude auth login`. -- Codex CLI (`codex`): install with `brew install --cask codex`; check with - `codex login status` and sign in with `codex login`. -- GitHub CLI (`gh`): install with `brew install gh`; check repository access - with `gh auth status` and sign in with `gh auth login`. -- `jq`: install with `brew install jq`; check with `jq --version`. +| Tool | Homebrew install | Check | Sign in | +|------|------------------|-------|---------| +| `git` | `brew install git` | `git --version` | — | +| `claude` | `brew install --cask claude-code` | `claude auth status` | `claude auth login` | +| `codex` | `brew install --cask codex` | `codex login status` | `codex login` | +| `gh` | `brew install gh` | `gh auth status` | `gh auth login` | +| `jq` | `brew install jq` | `jq --version` | — | + +Claude Code requires Anthropic authentication, Codex CLI requires OpenAI +authentication, and GitHub CLI must have access to this repository. If a check fails, install or sign in to that tool, then run the workflow command again. An existing run resumes from the failed turn. From 89ba8dab27739889de88c7d5b9a3a6a56f0fb8fe Mon Sep 17 00:00:00 2001 From: Oleg-Melnik Date: Thu, 6 Aug 2026 00:17:03 +0300 Subject: [PATCH 4/5] Update AI-agents config. --- .agents/workflows/pair-test.sh | 58 ++++++++++++++++++++++++++ .agents/workflows/pair.sh | 74 ++++++++++++++++++++++++++++++++++ PAIR_AGENTS_RUN_GUIDE.md | 10 +++-- 3 files changed, 139 insertions(+), 3 deletions(-) diff --git a/.agents/workflows/pair-test.sh b/.agents/workflows/pair-test.sh index 5c5e2a0b..7d3945f7 100755 --- a/.agents/workflows/pair-test.sh +++ b/.agents/workflows/pair-test.sh @@ -958,6 +958,64 @@ AGENT2_CMD="${SANDBOX}/bin/stub-agent2 --sandbox danger-full-access" \ want "swapped unsafe agent command is still refused" 1 "allow-unsafe-agents" cleanup +# --- sandboxed implementer verification access ---------------------------- +# Codex under workspace-write cannot start the root Gradle build: the wrapper +# locks inside the Gradle user home, and gradle.properties forces a forked +# daemon that binds a loopback port. An implementer that cannot build reaches +# review having compiled nothing, and the run ends `blocked` rather than done. +sandbox +export GRADLE_USER_HOME="${SANDBOX}/gradle-home" +mkdir -p "$GRADLE_USER_HOME" +export AGENT1_CMD="${SANDBOX}/bin/codex exec --sandbox workspace-write" +export AGENT2_CMD="${SANDBOX}/bin/claude" +run "$R" start 7 +want "a sandboxed Codex implementer starts a task" 0 +: > "$STUB_AGENT_ARGS_RECORD" +run "$R" step 7 +want "the implementer grant is announced, not silent" 0 "implementer sandbox widened" +check "the implementer can reach the Gradle user home" \ + "$(grep -qF -- "<--add-dir> <${GRADLE_USER_HOME}>" \ + "$STUB_AGENT_ARGS_RECORD" && echo 0 || echo 1)" +check "the implementer sandbox admits the Gradle daemon socket" \ + "$(grep -qF -- '<-c> ' \ + "$STUB_AGENT_ARGS_RECORD" && echo 0 || echo 1)" +unset GRADLE_USER_HOME +cleanup + +# The reviewer never builds, so the widening must not follow Codex into that +# seat — this is what keeps the default configuration unchanged. +sandbox +export GRADLE_USER_HOME="${SANDBOX}/gradle-home" +mkdir -p "$GRADLE_USER_HOME" +export AGENT1_CMD="${SANDBOX}/bin/claude" +export AGENT2_CMD="${SANDBOX}/bin/codex exec --sandbox workspace-write" +run "$R" start 7 +want "a Codex reviewer starts a task" 0 +run "$R" step 7 +want "the Claude implementer takes its turn" 0 +: > "$STUB_AGENT_ARGS_RECORD" +run "$R" step 7 +want "the Codex reviewer takes its turn" 0 +check "a Codex reviewer keeps the narrower sandbox" \ + "$(grep -q 'network_access' "$STUB_AGENT_ARGS_RECORD" && echo 1 || echo 0)" +check "a Codex reviewer is not given the Gradle user home" \ + "$(grep -qF -- "<--add-dir> <${GRADLE_USER_HOME}>" \ + "$STUB_AGENT_ARGS_RECORD" && echo 1 || echo 0)" +unset GRADLE_USER_HOME +cleanup + +# A Gradle user home that does not exist yet is a warning, not an abort: the +# run is still worth taking, it just cannot verify at the end. +sandbox +export GRADLE_USER_HOME="${SANDBOX}/never-populated" +export AGENT1_CMD="${SANDBOX}/bin/codex exec --sandbox workspace-write" +export AGENT2_CMD="${SANDBOX}/bin/claude" +run "$R" 7 +want "a missing Gradle user home warns without failing the run" 0 \ + "no Gradle user home" +unset GRADLE_USER_HOME +cleanup + # --- answers (RS-06) ------------------------------------------------------ sandbox STUB_MISBEHAVE=ask run "$R" 7 diff --git a/.agents/workflows/pair.sh b/.agents/workflows/pair.sh index eefa7a5f..6654b36e 100755 --- a/.agents/workflows/pair.sh +++ b/.agents/workflows/pair.sh @@ -102,6 +102,11 @@ flags because --ignore-user-config discards ~/.codex/config.toml by design. --add-dir is required because that sandbox refuses to write gitignored paths, and the working document lives in one. Keep it when overriding AGENT2_CMD. +A Codex command that holds the implementer seat — where --swap-agents puts the +default reviewer — is additionally given the Gradle user home and sandbox +network access, because the root build cannot start without either. The grant +is announced when it happens and is never extended to the reviewer. + Exit codes (run): 0 done — automated tests cover every acceptance criterion 1 aborted: a guard tripped, or an agent failed @@ -139,6 +144,11 @@ readonly TEMPLATE="${REPO_ROOT}/.agents/skills/pair-workflow/template.md" AGENT1_CMD="${AGENT1_CMD:-claude -p --permission-mode acceptEdits --setting-sources project --model claude-opus-5 --effort high}" AGENT2_CMD="${AGENT2_CMD:-codex exec --sandbox workspace-write --add-dir .agents/work --ephemeral --ignore-user-config -m gpt-5.6-sol -c model_reasoning_effort=\"high\" -c service_tier=\"default\"}" +# Codex's setting for network access inside the workspace-write sandbox. Named +# once because grant_implementer_verification_access() both tests for it and +# appends it, and a typo in either place would be silent. +readonly CODEX_SANDBOX_NETWORK="sandbox_workspace_write.network_access=true" + # Engine-specific selections requested on the command line. They are separate # from role assignment: --swap-agents exchanges the complete configured # commands, while these settings continue to identify Claude and Codex. @@ -2644,6 +2654,68 @@ validate_agent_permissions() { return 0 } +# Where the root build keeps its wrapper distributions, caches, and daemon +# registry. Honors GRADLE_USER_HOME so a machine that relocates it is still +# described accurately. +gradle_user_home() { printf '%s' "${GRADLE_USER_HOME:-${HOME}/.gradle}"; } + +# Gives a sandboxed Codex implementer the two things the root build needs and +# `--sandbox workspace-write` withholds. Both are outside the workspace: +# +# * The wrapper takes a lock inside the Gradle user home +# (wrapper/dists/…/gradle--bin.zip.lck) before it starts anything, +# and the sandbox denies that write. +# * gradle.properties sets `org.gradle.jvmargs`, so Gradle 6.9.4 always runs +# the build in a forked daemon — `--no-daemon` only makes that daemon +# single-use — and the daemon binds a loopback TCP port the sandbox denies. +# Matching the JVM arguments from the client does not avoid the fork, so +# there is no socket-free way to run this build. +# +# Without both, the implementer reaches `## Implementation` having compiled +# nothing and the run ends `blocked` on the verification rule instead of +# `done`. The default configuration never hits this because Claude holds the +# implementer seat; --swap-agents is what moves Codex into it. +# +# Scoped to agent1 deliberately. Network access inside the sandbox is a real +# widening, and the reviewer does not build, so it has no claim on it. +grant_implementer_verification_access() { + [[ "$(agent_command_engine "$AGENT1_CMD")" == codex ]] || return 0 + case " $AGENT1_CMD " in + *" --sandbox workspace-write "*) ;; + *) return 0 ;; + esac + + local home; home="$(gradle_user_home)" + # Agent commands are whitespace-delimited when they are split for + # execution, so a path with a space cannot be passed through as one word. + # Say so rather than appending an argument that would silently truncate. + if [[ "$home" == *[[:space:]]* ]]; then + info "warning: Gradle user home '${home}' contains whitespace; the "\ +"sandboxed implementer cannot be given access to it and will not be able to "\ +"verify" + return 0 + fi + if [[ ! -d "$home" ]]; then + info "warning: no Gradle user home at '${home}'; the sandboxed "\ +"implementer cannot verify until the root build has populated it once" + return 0 + fi + + local granted="" + if [[ " $AGENT1_CMD " != *" --add-dir ${home} "* ]]; then + AGENT1_CMD+=" --add-dir ${home}" + granted="the Gradle user home" + fi + if [[ " $AGENT1_CMD " != *" -c ${CODEX_SANDBOX_NETWORK} "* ]]; then + AGENT1_CMD+=" -c ${CODEX_SANDBOX_NETWORK}" + granted="${granted:+${granted} and }sandbox network access" + fi + [[ -z "$granted" ]] \ + || info "implementer sandbox widened so the root build can start: "\ +"${granted}" + return 0 +} + cmd_step() { local slug; slug="$(resolve_slug "${1:-}")" || exit "$EXIT_ERROR"; shift || true while [[ $# -gt 0 ]]; do @@ -2681,6 +2753,7 @@ cmd_step() { validate_agent_selection "$(doc_for "$slug")" prepare_saved_engine_settings "$(doc_for "$slug")" validate_agent_permissions + grant_implementer_verification_access local rc ec=0 set +e; take_turn "$slug"; rc=$?; set -e @@ -2787,6 +2860,7 @@ cmd_run() { fi validate_agent_permissions + grant_implementer_verification_access local i rc ec for (( i = 1; i <= max_turns; i++ )); do diff --git a/PAIR_AGENTS_RUN_GUIDE.md b/PAIR_AGENTS_RUN_GUIDE.md index 8cd34bd0..538039d0 100644 --- a/PAIR_AGENTS_RUN_GUIDE.md +++ b/PAIR_AGENTS_RUN_GUIDE.md @@ -196,9 +196,13 @@ Claude gives you the draft PR link instead. ## Safety Claude runs in `acceptEdits` mode with project settings; Codex uses its -`workspace-write` sandbox without user configuration. The agent instructions -treat issue text as untrusted task data rather than as instructions. The driver -checks that: +`workspace-write` sandbox without user configuration. When `--sa` moves Codex +into the implementer seat, the driver additionally grants that sandbox the +Gradle user home and network access, and says so at startup: the root build +locks inside the Gradle home and runs in a forked daemon that binds a local +port, so it cannot start without both. The reviewer's sandbox is never widened. +The agent instructions treat issue text as untrusted task data rather than as +instructions. The driver checks that: - agents do not change Git refs or the index; - the reviewer does not change source files or saved review snapshots; From 7bcd0a1ee4cf8b0318ef1e1bbc08435b0baa712a Mon Sep 17 00:00:00 2001 From: Oleg-Melnik Date: Thu, 6 Aug 2026 01:12:16 +0300 Subject: [PATCH 5/5] Update AI-agents config. --- .agents/skills/pair-workflow/SKILL.md | 27 +++++++-- .agents/workflows/pair-test.sh | 64 +++++++++++++++++++++ .agents/workflows/pair.sh | 80 ++++++++++++++++++++++++--- PAIR_AGENTS_RUN_GUIDE.md | 5 ++ 4 files changed, 164 insertions(+), 12 deletions(-) diff --git a/.agents/skills/pair-workflow/SKILL.md b/.agents/skills/pair-workflow/SKILL.md index 744ab2fe..645377b7 100644 --- a/.agents/skills/pair-workflow/SKILL.md +++ b/.agents/skills/pair-workflow/SKILL.md @@ -613,11 +613,28 @@ either: `workspace-write` sandbox excludes gitignored paths, and the working document lives in one by design. Without it the reviewer reads everything, writes nothing, and the driver aborts on an unmodified document. -- **`agent1`'s verification permissions come from `.claude/settings.json`.** - `--setting-sources project` loads that file and nothing else, so a Gradle - command missing from it is refused before the process starts. An agent that - cannot build hands off an implementation it never compiled, and the reviewer - spends its round saying so instead of reading the code. +- **`agent1`'s verification permissions depend on which engine holds the + seat.** Either way, an implementer that cannot build hands off code it never + compiled, and the reviewer spends its round saying so instead of reading it. + - _Claude in the seat_ (the default): permissions come from + `.claude/settings.json`. `--setting-sources project` loads that file and + nothing else, so a Gradle command missing from it is refused before the + process starts. + - _Codex in the seat_ (`--swap-agents`): its `workspace-write` sandbox + blocks the root build outright — the wrapper locks inside the Gradle user + home, and `gradle.properties` forces a forked daemon that binds a loopback + port. The driver adds `--add-dir ` and + `sandbox_workspace_write.network_access=true` to that command and announces + the grant at startup. It applies only to a command that names + `workspace-write` itself, and never to the reviewer. + + The Codex grant is a genuine widening, not a formality: the implementer can + write anywhere under the Gradle user home — which is outside the repository + and shared with every other build on the machine — and can reach the network + from inside the sandbox, so a prompt-injected instruction to exfiltrate is no + longer stopped by the sandbox. The Git tripwire and the review checks are the + remaining guards, and neither is an isolation boundary. Prefer the default + seating unless a run needs Codex as the implementer. Pass `--swap-agents` or `--sa` to exchange `agent1` and `agent2`. Repeat the option when resuming the task; the driver checks the choice against the agent diff --git a/.agents/workflows/pair-test.sh b/.agents/workflows/pair-test.sh index 7d3945f7..95b80625 100755 --- a/.agents/workflows/pair-test.sh +++ b/.agents/workflows/pair-test.sh @@ -982,6 +982,70 @@ check "the implementer sandbox admits the Gradle daemon socket" \ unset GRADLE_USER_HOME cleanup +# Codex accepts one sandbox policy under several spellings. A customized +# command using any of them must be recognized: matching only the long +# separated form leaves the implementer unwidened, and the run then fails the +# root build with nothing in the output naming the cause. +for spelling in "-s workspace-write" "-s=workspace-write" \ + "-sworkspace-write" "--sandbox=workspace-write"; do + sandbox + export GRADLE_USER_HOME="${SANDBOX}/gradle-home" + mkdir -p "$GRADLE_USER_HOME" + export AGENT1_CMD="${SANDBOX}/bin/codex exec ${spelling}" + export AGENT2_CMD="${SANDBOX}/bin/claude" + run "$R" start 7 + : > "$STUB_AGENT_ARGS_RECORD" + run "$R" step 7 + want "'${spelling}' is recognized as workspace-write" 0 \ + "implementer sandbox widened" + check "'${spelling}' reaches the Gradle user home" \ + "$(grep -qF -- "<--add-dir> <${GRADLE_USER_HOME}>" \ + "$STUB_AGENT_ARGS_RECORD" && echo 0 || echo 1)" + unset GRADLE_USER_HOME + cleanup +done + +# The mirror of the above: recognizing spellings must not decay into matching +# the words anywhere in the command. A policy that is not workspace-write has +# no claim on the widening, whether or not the phrase appears elsewhere. +for spelling in "-s read-only" "--sandbox read-only" \ + "-s read-only -c sandbox_workspace_write.network_access=false"; do + sandbox + export GRADLE_USER_HOME="${SANDBOX}/gradle-home" + mkdir -p "$GRADLE_USER_HOME" + export AGENT1_CMD="${SANDBOX}/bin/codex exec ${spelling}" + export AGENT2_CMD="${SANDBOX}/bin/claude" + run "$R" start 7 + : > "$STUB_AGENT_ARGS_RECORD" + run "$R" step 7 + check "'${spelling}' is not widened" \ + "$(grep -qF -- "<--add-dir> <${GRADLE_USER_HOME}>" \ + "$STUB_AGENT_ARGS_RECORD" && echo 1 || echo 0)" + unset GRADLE_USER_HOME + cleanup +done + +# A caller who already granted the access keeps their own spelling: the driver +# must neither duplicate the argument nor claim a widening it did not make. +sandbox +export GRADLE_USER_HOME="${SANDBOX}/gradle-home" +mkdir -p "$GRADLE_USER_HOME" +export AGENT1_CMD="${SANDBOX}/bin/codex exec --sandbox workspace-write \ +--add-dir=${GRADLE_USER_HOME} -c=sandbox_workspace_write.network_access=true" +export AGENT2_CMD="${SANDBOX}/bin/claude" +run "$R" start 7 +: > "$STUB_AGENT_ARGS_RECORD" +run "$R" step 7 +want "an already-granted implementer is not widened again" 0 +check "the caller's own grant is not announced as a widening" \ + "$(printf '%s' "$OUT" | grep -q 'implementer sandbox widened' \ + && echo 1 || echo 0)" +check "the Gradle user home is not passed twice" \ + "$([[ "$(grep -c -- "--add-dir" "$STUB_AGENT_ARGS_RECORD")" -eq 1 ]] \ + && echo 0 || echo 1)" +unset GRADLE_USER_HOME +cleanup + # The reviewer never builds, so the widening must not follow Codex into that # seat — this is what keeps the default configuration unchanged. sandbox diff --git a/.agents/workflows/pair.sh b/.agents/workflows/pair.sh index 6654b36e..637eeb37 100755 --- a/.agents/workflows/pair.sh +++ b/.agents/workflows/pair.sh @@ -270,6 +270,68 @@ agent_command_setting() { unquote_setting "$value" } +# Reads the sandbox policy named by a directly configured Codex command. +# +# Codex spells one policy several ways: `-s` or `--sandbox` with the value as +# the next word, joined by `=`, or attached to the short option, and +# `-c sandbox_mode=…` selects it from configuration instead. Recognizing a +# single literal spelling would leave grant_implementer_verification_access() +# inert for the rest, and the only symptom would be an implementer that cannot +# build — with nothing in the output pointing at the command that caused it. +# +# The command-line option wins over the configuration override, as it does in +# Codex; within each, the last occurrence wins. Prints nothing when the command +# names no policy, which leaves Codex on its own default. +codex_sandbox_mode() { + local cmd="$1" token flag="" config="" + local -a words + local i + read -r -a words <<< "$cmd" + for (( i = 0; i < ${#words[@]}; i++ )); do + token="${words[$i]}" + case "$token" in + -s|--sandbox) + i=$(( i + 1 )); flag="${words[$i]:-}" ;; + -s=*|--sandbox=*) + flag="${token#*=}" ;; + -s?*) + flag="${token#-s}" ;; + -c|--config) + if [[ "${words[$(( i + 1 ))]:-}" == sandbox_mode=* ]]; then + i=$(( i + 1 )) + config="${words[$i]#*=}" + fi ;; + --config=sandbox_mode=*) + config="${token#--config=sandbox_mode=}" ;; + esac + done + unquote_setting "${flag:-$config}" +} + +# Whether a command already passes `option value`, separated or joined by `=`. +# Keeps the verification grant from appending an argument the caller supplied +# themselves, and from announcing a widening that did not happen. +command_passes_option() { + local cmd="$1" option="$2" value="$3" token + local -a words + local i + read -r -a words <<< "$cmd" + for (( i = 0; i < ${#words[@]}; i++ )); do + token="${words[$i]}" + case "$token" in + "$option") + if [[ "${words[$(( i + 1 ))]:-}" == "$value" ]]; then + return 0 + fi ;; + "$option"=*) + if [[ "${token#*=}" == "$value" ]]; then + return 0 + fi ;; + esac + done + return 1 +} + # Replaces one engine's model and effort flags without disturbing its safety # flags. Codex rejects repeated --model arguments, so appending an override is # not sufficient there. @@ -2660,7 +2722,7 @@ validate_agent_permissions() { gradle_user_home() { printf '%s' "${GRADLE_USER_HOME:-${HOME}/.gradle}"; } # Gives a sandboxed Codex implementer the two things the root build needs and -# `--sandbox workspace-write` withholds. Both are outside the workspace: +# the `workspace-write` sandbox withholds. Both are outside the workspace: # # * The wrapper takes a lock inside the Gradle user home # (wrapper/dists/…/gradle--bin.zip.lck) before it starts anything, @@ -2678,12 +2740,14 @@ gradle_user_home() { printf '%s' "${GRADLE_USER_HOME:-${HOME}/.gradle}"; } # # Scoped to agent1 deliberately. Network access inside the sandbox is a real # widening, and the reviewer does not build, so it has no claim on it. +# +# Only a command that names `workspace-write` itself is widened, in any of the +# spellings codex_sandbox_mode() understands. A command that names no policy is +# left alone rather than assumed: extending a sandbox the caller never asked +# for would be a worse failure than the one this repairs. grant_implementer_verification_access() { [[ "$(agent_command_engine "$AGENT1_CMD")" == codex ]] || return 0 - case " $AGENT1_CMD " in - *" --sandbox workspace-write "*) ;; - *) return 0 ;; - esac + [[ "$(codex_sandbox_mode "$AGENT1_CMD")" == workspace-write ]] || return 0 local home; home="$(gradle_user_home)" # Agent commands are whitespace-delimited when they are split for @@ -2702,11 +2766,13 @@ grant_implementer_verification_access() { fi local granted="" - if [[ " $AGENT1_CMD " != *" --add-dir ${home} "* ]]; then + if ! command_passes_option "$AGENT1_CMD" --add-dir "$home"; then AGENT1_CMD+=" --add-dir ${home}" granted="the Gradle user home" fi - if [[ " $AGENT1_CMD " != *" -c ${CODEX_SANDBOX_NETWORK} "* ]]; then + if ! command_passes_option "$AGENT1_CMD" -c "$CODEX_SANDBOX_NETWORK" \ + && ! command_passes_option "$AGENT1_CMD" --config "$CODEX_SANDBOX_NETWORK" + then AGENT1_CMD+=" -c ${CODEX_SANDBOX_NETWORK}" granted="${granted:+${granted} and }sandbox network access" fi diff --git a/PAIR_AGENTS_RUN_GUIDE.md b/PAIR_AGENTS_RUN_GUIDE.md index 538039d0..b1ad8bc1 100644 --- a/PAIR_AGENTS_RUN_GUIDE.md +++ b/PAIR_AGENTS_RUN_GUIDE.md @@ -363,6 +363,11 @@ Keep all safety flags from the printed command. In particular: from `.claude/settings.json`. - Codex needs `--ignore-user-config` for reproducible settings and `--add-dir` to write the gitignored working document. +- A Codex command in the implementer seat receives the build grant only when it + names the `workspace-write` sandbox itself. Every spelling Codex accepts + works — `-s` or `--sandbox`, with the value as the next word or joined by + `=`, and `-c sandbox_mode=` — but a command naming no sandbox is left as it + is rather than widened on its behalf. If a model or effort is rejected, read the relevant turn log under `.agents/work/issue-/turns/`. The document records settings that the