diff --git a/.agents/skills/README.md b/.agents/skills/README.md index fff2f77d..3c0bcb49 100644 --- a/.agents/skills/README.md +++ b/.agents/skills/README.md @@ -12,9 +12,12 @@ routing source of truth. - `component-engineer`: class-based Compose UI components across `core`, `proto`, and `client` — the component model, input components, message forms, and server-connected components. +- `model-engineer`: published model Protobuf declarations and Kotlin model + extensions in `proto-values`, including schema evolution and package and + file organization. - `codegen-engineer`: ProtoData codegen plugins, the codegen runtime, - generated `MessageField`/`MessageOneof`/`MessageDef` contracts, and - Protobuf declarations in `proto-values`. + generated `MessageField`/`MessageOneof`/`MessageDef` contracts, + plugin-internal Protobuf declarations, and codegen correctness tests. - `build-engineer`: root and `codegen/plugins` Gradle builds, `buildSrc` dependency coordinates, publishing wiring, version policy, and generated report regeneration. @@ -23,8 +26,8 @@ routing source of truth. secret exposure review. - `ci-engineer`: GitHub Actions workflow authoring and review for build, guard, validation, and publishing pipelines. -- `code-reviewer`: implementation review for component, codegen, and build - changes. +- `code-reviewer`: implementation review for component, model, codegen, and + build changes. - `tester`: what to cover and how to verify it across all modules, including codegen correctness tests. - `kotlin-jvm-tester`: how a test suite is written — JUnit Jupiter diff --git a/.agents/skills/code-reviewer/SKILL.md b/.agents/skills/code-reviewer/SKILL.md index 7e756542..db9bc41f 100644 --- a/.agents/skills/code-reviewer/SKILL.md +++ b/.agents/skills/code-reviewer/SKILL.md @@ -3,8 +3,8 @@ name: code-reviewer description: > Reviews Chords implementation changes for correctness, regressions, public API breaks, missing tests, and cross-module contract breaks. Use to review - component, codegen, or build diffs. Read-only unless explicitly asked to run - checks. + component, model, codegen, or build diffs. Read-only unless explicitly asked + to run checks. --- # Code Review @@ -29,9 +29,10 @@ unless the task asks for it directly. To judge CI status, read existing results behavior, and contract impact require context beyond the diff hunk. 3. **Trace the owning flow.** For component changes, follow the `Component`/`InputComponent` lifecycle (`beforeComposeContent`, `content`, - `Props` configuration) and the state properties involved. For codegen - changes, follow the generator, the runtime contract, and `codegen/tests` - together. + `Props` configuration) and the state properties involved. For model + changes, follow the Protobuf declarations, Kotlin extensions, generated + accessors, and consumers together. For codegen changes, follow the + generator, the runtime contract, and `codegen/tests` together. 4. **Verify claims against source.** Confirm Gradle task names, module paths, generated API shapes, and toolchain constraints against the relevant build file, README, or workflow. @@ -77,8 +78,20 @@ duplicate its steps. component subclasses consume them. Kotlin explicit API mode applies. - Cross-module contract breaks: `core`/`proto`/`client` layering, the generated `MessageField`/`MessageOneof`/`MessageDef` contract between - `codegen/plugins` and `codegen/runtime`, and Protobuf compatibility in - `proto-values`. + `codegen/plugins` and `codegen/runtime`, and both source API and Protobuf wire + compatibility in `proto-values`. +- Protobuf model changes: apply `.agents/skills/model-engineer/SKILL.md` to + published model declarations and extensions under `proto-values`. Check + changed, deleted, or renumbered field tags and confirm retired numbers and + names are reserved. Inspect every field type, singular/repeated cardinality, + and `oneof` membership change even when its tag is unchanged. Do not treat + rebuilt consumers or passing tests as proof that old wire data remains + compatible. Inspect message renames and Protobuf package changes, including + package changes required by file relocation. Where `(type_url_prefix)` is + set, assess changed type URLs against existing `Any` values and Spine + type registry resolution. Confirm that every intentional incompatibility is + explicitly in scope and that the change reports its external-consumer and + data impact. - Kotlin-language and toolchain violations: apply `.agents/skills/kotlin-engineer/SKILL.md` to the changed Kotlin and report what its rules flag. Read it rather than reviewing from memory — several diff --git a/.agents/skills/code-reviewer/agents/openai.yaml b/.agents/skills/code-reviewer/agents/openai.yaml index 4b775ac4..13174bb9 100644 --- a/.agents/skills/code-reviewer/agents/openai.yaml +++ b/.agents/skills/code-reviewer/agents/openai.yaml @@ -1,6 +1,6 @@ interface: display_name: "Code Reviewer" - short_description: "Review Chords component, codegen, or build changes" + short_description: "Review Chords component, model, codegen, or build changes" default_prompt: > Use $code-reviewer after reading AGENTS.md, README.md, .agents/project.md, and the nearest README for the area being changed to review a scoped diff --git a/.agents/skills/codegen-engineer/SKILL.md b/.agents/skills/codegen-engineer/SKILL.md index 47e9686e..7f001464 100644 --- a/.agents/skills/codegen-engineer/SKILL.md +++ b/.agents/skills/codegen-engineer/SKILL.md @@ -3,27 +3,30 @@ name: codegen-engineer description: > Chords code generation policy. Use for the ProtoData codegen plugins project, the codegen runtime library, generated MessageField/MessageOneof/MessageDef - contracts, codegen correctness tests, and Protobuf declarations with Kotlin - extensions in proto-values. + contracts, plugin-internal Protobuf declarations, codegen correctness tests, + and codegen Gradle wiring. --- # Codegen Engineering ## When to Use -Use this skill for code generation and Protobuf model work: +Use this skill for code generation work: - ProtoData plugins under `codegen/plugins/` (a separate Gradle project). +- Plugin-internal Protobuf declarations under + `codegen/plugins/codegen-plugins/src/main/proto/**`. - The codegen runtime under `codegen/runtime/` (Gradle path `:runtime`): `MessageField`, `MessageOneof`, `MessageDef`, and related runtime types. - Codegen correctness tests under `codegen/tests/` (Gradle path `:codegen-tests`). -- Protobuf declarations and Kotlin extensions in `proto-values`. - The codegen wiring in the root build (`modulesWithChordsCodegen`, `publishCodegenPluginsToMavenLocal`, the `io.spine.chords` Gradle plugin configuration). -For components that merely consume generated metadata, prefer +For published model Protobuf declarations and Kotlin model extensions under +`proto-values`, prefer `.agents/skills/model-engineer/SKILL.md`. For components +that merely consume generated metadata, prefer `.agents/skills/component-engineer/SKILL.md`. For build-only concerns, use `.agents/skills/build-engineer/SKILL.md`. @@ -37,10 +40,9 @@ For components that merely consume generated metadata, prefer external projects: changes to `MessageField`/`MessageOneof`/`MessageDef` shapes are public API changes on both the generator and runtime sides and must stay in sync. -- For Protobuf schema changes in `proto-values`: never delete or renumber - existing fields, reserve retired field numbers and names, and keep package - names consistent with the existing `spine/chords/proto/value/**` structure - under `proto-values/src/main/proto/`. +- Treat `.proto` files under `codegen/tests/src/test/proto/**` as generator + fixtures. Change them only to express a code-generation scenario; published + model declarations under `proto-values` belong to `model-engineer`. - Keep `codegen/plugins/src/main/resources/codegen-workspace` resources consistent with the build logic that copies `buildSrc` and wrapper files into them; that workspace is what the Chords Gradle plugin unpacks in diff --git a/.agents/skills/codegen-engineer/agents/openai.yaml b/.agents/skills/codegen-engineer/agents/openai.yaml index 6e44448f..81dd9020 100644 --- a/.agents/skills/codegen-engineer/agents/openai.yaml +++ b/.agents/skills/codegen-engineer/agents/openai.yaml @@ -1,6 +1,6 @@ interface: display_name: "Codegen Engineer" - short_description: "Implement Chords code generation or Protobuf model changes" + short_description: "Implement Chords code generation changes" default_prompt: > Use $codegen-engineer after reading AGENTS.md, README.md, .agents/project.md, and codegen/plugins/README.md to implement a scoped diff --git a/.agents/skills/component-engineer/SKILL.md b/.agents/skills/component-engineer/SKILL.md index 07ff6101..97dc50b7 100644 --- a/.agents/skills/component-engineer/SKILL.md +++ b/.agents/skills/component-engineer/SKILL.md @@ -23,9 +23,11 @@ Use this skill for UI component and component-infrastructure work: - Server-connected components in `client`: command posting, entity subscriptions, and entity-backed components such as `EntityChooser`. -For generated `MessageField`/`MessageOneof`/`MessageDef` contracts or Protobuf -declarations, prefer `.agents/skills/codegen-engineer/SKILL.md`. For Gradle -build logic, use `.agents/skills/build-engineer/SKILL.md`. +For published model Protobuf declarations and Kotlin model extensions under +`proto-values`, prefer `.agents/skills/model-engineer/SKILL.md`. For generated +`MessageField`/`MessageOneof`/`MessageDef` contracts, prefer +`.agents/skills/codegen-engineer/SKILL.md`. For Gradle build logic, use +`.agents/skills/build-engineer/SKILL.md`. ## Policy diff --git a/.agents/skills/engineer/SKILL.md b/.agents/skills/engineer/SKILL.md index a47dd0a9..42367585 100644 --- a/.agents/skills/engineer/SKILL.md +++ b/.agents/skills/engineer/SKILL.md @@ -3,9 +3,9 @@ name: engineer description: > Routes Chords implementation work to the area-specific engineering skill and carries the design-restraint policy shared by all of them. Use for mixed - component/codegen/build changes or when the owning area is unclear; otherwise - prefer the narrowest specialist skill directly, and follow "Design Restraint" - below in either case. + component/model/codegen/build changes or when the owning area is unclear; + otherwise prefer the narrowest specialist skill directly, and follow + "Design Restraint" below in either case. --- # Engineering Router @@ -20,11 +20,14 @@ the code being changed: components in `core`, `proto`, and `client`: the component model, application shell, input components, message forms, validation display, and server-connected components. +- `.agents/skills/model-engineer/SKILL.md` for published model Protobuf + declarations and Kotlin model extensions under `proto-values`, including + schema evolution and package and file organization. - `.agents/skills/codegen-engineer/SKILL.md` for the `codegen/plugins` ProtoData project, the codegen runtime (`codegen/runtime`), codegen - correctness tests (`codegen/tests`), generated - `MessageField`/`MessageOneof`/`MessageDef` contracts, and Protobuf - declarations in `proto-values`. + correctness tests (`codegen/tests`), plugin-internal Protobuf declarations, + generated `MessageField`/`MessageOneof`/`MessageDef` contracts, and codegen + Gradle wiring. - `.agents/skills/build-engineer/SKILL.md` for root and `codegen/plugins` Gradle build logic, `buildSrc` dependency coordinates, publishing wiring, version policy, generated `pom.xml`/`dependencies.md` reports, and the diff --git a/.agents/skills/kotlin-engineer/SKILL.md b/.agents/skills/kotlin-engineer/SKILL.md index f4c49a85..a413b9cf 100644 --- a/.agents/skills/kotlin-engineer/SKILL.md +++ b/.agents/skills/kotlin-engineer/SKILL.md @@ -27,8 +27,10 @@ Each of these owns its area; this skill stays out of them: - `.agents/skills/component-engineer/SKILL.md` — the component model, `mutableStateOf`-backed state, `PascalCase` composables, module layering, KDoc style, and the Compose 1.5.12 ceiling. -- `.agents/skills/codegen-engineer/SKILL.md` — generated contracts and - Protobuf declarations. +- `.agents/skills/model-engineer/SKILL.md` — published model Protobuf + declarations and Kotlin model extensions under `proto-values`. +- `.agents/skills/codegen-engineer/SKILL.md` — ProtoData plugins and generated + runtime contracts. - `.agents/skills/build-engineer/SKILL.md` — Gradle Kotlin DSL, `buildSrc` coordinates, and publishing. - `.agents/skills/kotlin-jvm-tester/SKILL.md` — how a test suite is diff --git a/.agents/skills/kotlin-jvm-tester/SKILL.md b/.agents/skills/kotlin-jvm-tester/SKILL.md index e080ec62..bac13ac0 100644 --- a/.agents/skills/kotlin-jvm-tester/SKILL.md +++ b/.agents/skills/kotlin-jvm-tester/SKILL.md @@ -28,9 +28,9 @@ restating: body is Kotlin, so its null-safety, coroutine scoping, and language ceiling obey the same rules as production code. - `.agents/skills/engineer/SKILL.md` — the router to the area-specific - engineering skill (`component-engineer`, `codegen-engineer`, - `build-engineer`, …). Use it to find the skill owning the code under - test when you need its API constraints. + engineering skill (`component-engineer`, `model-engineer`, + `codegen-engineer`, `build-engineer`, …). Use it to find the skill owning + the code under test when you need its API constraints. `AGENTS.md` remains authoritative for Git history, versioning, and verification policy. diff --git a/.agents/skills/model-engineer/SKILL.md b/.agents/skills/model-engineer/SKILL.md new file mode 100644 index 00000000..17030f7a --- /dev/null +++ b/.agents/skills/model-engineer/SKILL.md @@ -0,0 +1,88 @@ +--- +name: model-engineer +description: > + Chords Protobuf model and schema policy. Use when adding, changing, + reviewing, or removing project-owned `.proto` declarations and Kotlin model + extensions located in `proto-values`, organizing Protobuf packages, imports, + and files, or evaluating schema-driven generated-accessor effects. Do not + use for ProtoData plugin, generator, or codegen-runtime implementation; use + `codegen-engineer` for those concerns. +--- + +# Model Engineering + +## Scope + +- Own published model Protobuf declarations under + `proto-values/src/main/proto/**`. +- Own Kotlin extensions for project-owned and external Protobuf types under + `proto-values/src/main/kotlin/**`. +- Keep schema declarations, Kotlin extensions, and schema-driven generated + accessors consistent. +- Use `.agents/skills/codegen-engineer/SKILL.md` for ProtoData plugins, + including plugin-internal Protobuf declarations under + `codegen/plugins/codegen-plugins/src/main/proto/**`, and for generator + behavior, codegen-runtime contracts, and test Protobuf files that exist only + as generator fixtures under `codegen/tests/src/test/proto/**`. +- Use `.agents/skills/component-engineer/SKILL.md` for UI components that + consume model types, and `.agents/skills/build-engineer/SKILL.md` for Gradle + wiring. +- Apply `.agents/skills/kotlin-engineer/SKILL.md` to every Kotlin extension. + +## Policy + +- Treat `AGENTS.md` as authoritative. Prefer additive changes for published + APIs. Rely on Chords' experimental status only under the breaking-change + authorization below, and report the compatibility impact. +- Add model declarations and make compatible edits as needed. Make an + incompatible change to an existing message, field, field type, cardinality, + `oneof` membership, or option only when the current task explicitly requires + the break or the user confirms it through the active workflow's prescribed + user-input channel. Do not preserve an obsolete declaration solely for + source compatibility when an intentional breaking change is in scope. +- Treat source compatibility and Protobuf wire compatibility separately. + Recompiling in-repository consumers and passing tests can prove source usage + was updated, but cannot prove that previously serialized or transmitted data + still decodes correctly. +- Evaluate every change to an existing field's type, singular/repeated + cardinality, or `oneof` membership, even when its tag stays unchanged. + Compare Protobuf wire types and value semantics; a shared tag or wire type + alone does not prove compatibility. +- Evaluate message renames and Protobuf package changes, including a package + change required when relocating a file. With `(type_url_prefix)` set, either + change alters the fully qualified message name and type URL, which can break + existing `Any` values and Spine type registry resolution. +- Never reuse a retired field number or name. When deleting a field, reserve + its old number and name in the owning message. When renumbering, reserve the + old number and reserve the old name if the change also retires it. + Renumbering remains wire-incompatible: old data keeps the old tag and does + not populate the field at its new number. Apply the breaking-change + authorization above and report any data or migration consequences. +- Remember that `proto-values` is published and consumed externally even + during the experimental phase. Update all in-repository consumers and tests + for an incompatible source change, and separately evaluate existing wire + data and external consumers. +- Keep published model schemas under + `proto-values/src/main/proto/spine/chords/proto/value/**`. Make each Protobuf + `package` match its directory relative to `proto-values/src/main/proto`, and + keep project-owned import paths aligned with the same structure. Never infer + the Protobuf package from `java_package`, which controls only generated JVM + classes. Before moving an existing file or changing its package, apply the + type-URL compatibility check and breaking-change authorization above. +- Update neighboring Kotlin extensions when a declaration, field, or generated + accessor they expose changes. +- Do not manually edit generated Protobuf or Chords outputs. Change the source + model, or use `codegen-engineer` when the generator itself must change. + +## Verification + +Run the narrowest relevant root command first with JDK 11: + +```bash +.agents/workflows/gradle-root.sh :proto-values:test +.agents/workflows/gradle-root.sh :proto-values:check +.agents/workflows/gradle-root.sh clean build +``` + +Use `:codegen-tests:test` with `codegen-engineer` when a model change exposes +or depends on generator behavior rather than only changing published schema. diff --git a/.agents/skills/model-engineer/agents/openai.yaml b/.agents/skills/model-engineer/agents/openai.yaml new file mode 100644 index 00000000..9c54c498 --- /dev/null +++ b/.agents/skills/model-engineer/agents/openai.yaml @@ -0,0 +1,7 @@ +interface: + display_name: "Model Engineer" + short_description: "Evolve Chords Protobuf models and extensions" + default_prompt: > + Use $model-engineer after reading AGENTS.md, README.md, + .agents/project.md, and proto-values/README.md to make a scoped Chords + Protobuf model change. diff --git a/.agents/skills/pair-workflow/SKILL.md b/.agents/skills/pair-workflow/SKILL.md index 3b2f6b6b..982fa9e2 100644 --- a/.agents/skills/pair-workflow/SKILL.md +++ b/.agents/skills/pair-workflow/SKILL.md @@ -102,21 +102,35 @@ driver talking, not the user. Also, the driver supports explicitly approved unsafe commands for externally isolated runs, where no CLI prompt would stop a stray `git commit` at the moment it happens. -`pair.sh` checks this rather than only asking for it: it snapshots `HEAD`, the -current branch, every ref, and the staged index before each turn, compares them -afterward, and aborts the run on any difference. +`pair.sh` checks this rather than only asking for it. It strictly compares +`HEAD`, the current branch, local refs other than tags, the staged index, the +effective Git configuration, and in-progress operation sentinels before and +after every turn. Any movement aborts and invalidates the run. + +Remote-tracking refs and tags are judged separately because a background fetch +can create, update, or prune them without either agent touching Git. The driver +exports a fresh per-turn Git Trace2 event file to the agent process tree and +writes a turn-unique marker immediately before the agent starts. It correlates +each process's command name with that process's arguments, inspects the trace +after every turn, and aborts for a Git command capable of moving a ref, even +when the final snapshot did not move. If an external ref changed without such a command, the +movement happened outside the agent process, so the driver reports it and +continues. A missing or unreadable trace also aborts rather than guessing. This +provenance check does not rely on commit reachability: a fetched commit may +already have a local branch, and a push may publish an otherwise unreachable +object. That check **detects, it does not prevent**. It runs after your turn has already finished, so it is a tripwire, not a boundary: a write followed by a -restore passes it, an effect outside this repository — a `gh` API call, a PR -opened from a branch that was already pushed — leaves no local trace at all, -and a push it does catch has already reached the remote. It also cannot say -**who** moved: two snapshots taken around a turn look the same whether an agent -wrote to Git or the user switched branches in another window, so a tripped -guard is a fact to investigate, not a verdict against the agent. Nothing here -makes a Git write impossible; the rule above is what keeps it from happening, -and the snapshot is only there to notice when the rule was broken. Do not treat -the absence of an abort as permission. +restore passes it, and an effect outside this repository — a `gh` API call, a +PR opened from a branch that was already pushed — leaves no local ref trace at +all. The agent process can also suppress or rewrite its inherited Trace2 file, +just as it can restore a snapshot, and a push the driver catches has already +reached the remote. For strictly compared local state, two snapshots still +cannot say whether the agent or a person in another window moved it. Nothing +here makes a Git write impossible; the rule above is what keeps it from +happening, and the checks only notice evidence that the rule was broken. Do not +treat the absence of an abort as permission. If a task genuinely cannot proceed without a Git operation, set `status: blocked` and `turn: human` and explain why. Never perform the @@ -143,11 +157,18 @@ One file per task at `.agents/work//plan.md`, created from `.agents/skills/pair-workflow/template.md`. The path is gitignored; the document is a scratch artifact and is never committed. +All slugs share one repository worktree and therefore one driver lock. Each +successful turn records strong Git and worktree digests; another task or +process cannot change the checkout between handoffs and become the next turn's +baseline. The driver excludes `.agents/work/` explicitly from review and +staging even if a task changes the repository's ignore rule. + Alongside it, `.agents/work//turns/NN-.log` holds each turn's -transcript, written by the driver. The document records what an agent chose to -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. +transcript, and the matching `NN-.git-trace.json` holds Git Trace2 events +from the agent process tree. Both are written by the driver. The document +records what an agent chose to write down; the transcripts and traces record +what its process 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 @@ -181,10 +202,19 @@ 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_commit: 825c14b3d392c497d5daaf5f97f28aa11f32279f base_branch: master start_commit: ebd7c8413f01c19e8dc12d48f150e79c50a94c78 pr_base_branch: master +pr_base_tip: 72f2aeb684d2856bbddb773f69e31b7e5c99f645 +task_branch: add-keyboard-accessible-copy-action +github_repo: github.com/SpineEventEngine/Chords +origin_fetch_url: git@github.com:SpineEventEngine/Chords.git +origin_push_url: git@github.com:SpineEventEngine/Chords.git +git_config_state: 178a1a5428fa6a32bfbc8c4272ea8e4d7bb185cf +publication_head: none +expected_git_state: bdc49d34f6f6dcf13c76fbbf8d91c8032d012f23 +expected_worktree_state: 1c44c704e135308a2c30c2a636342f85094bb1e2 changeset_digest: none reviewed_changeset_digest: none updated: 2026-07-31T14:20:00Z @@ -200,6 +230,17 @@ updated: 2026-07-31T14:20:00Z would run until the turn guard cut it off instead of reaching you. - `max_rounds` — the ceiling both counters are measured against; see "Termination". +- `pr_base_tip`, `github_repo`, and the two `origin_*_url` fields pin the remote + identity and target lineage used at setup. The driver refreshes the target + before setup and publication, refuses rewrites, and never lets ambient `gh` + or Git configuration silently retarget the result. +- `task_branch` is derived at setup, including a safe issue-number fallback for + titles that have no ASCII branch characters. Publication validates any local + or remote branch collision before it pushes. +- `git_config_state` pins the effective Git configuration used at setup and + publication. `expected_git_state` and `expected_worktree_state` bind each successful turn + to the exact shared checkout state it produced. Different slugs cannot use + one another's edits as their next baseline. - `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 @@ -225,7 +266,12 @@ updated: 2026-07-31T14:20:00Z - `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. + `start_commit` are inherited branch history, not this task's review scope. If + the target branch advances during the run, publication recomputes the + baseline only when the new merge-base remains between those two recorded + commits — the recorded value stays as the record of where the run began, and + only inherited history may shrink. A merge-base beyond `start_commit` has + absorbed task commits and stops publication. - `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, @@ -238,6 +284,20 @@ updated: 2026-07-31T14:20:00Z - `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. +- `pr_base_tip` — the exact remote target tip fetched at setup. Publication + fetches again and requires this commit to remain an ancestor, so a rewrite + with an unchanged merge-base still stops. +- `task_branch`, `github_repo`, `origin_fetch_url`, `origin_push_url` — the + publication identity derived and checked at setup. The driver passes the + repository and head explicitly to `gh`, and refuses URL or branch collisions. +- `git_config_state` — the effective local, global, and included Git + configuration at setup. Publication refuses a different configuration. +- `publication_head` — `none` before publication, then the last task-branch + commit the driver created. A retry refuses empty or content-equivalent commits + added by another process instead of publishing unreviewed history. +- `expected_git_state`, `expected_worktree_state` — strong driver-owned + digests updated after each successful turn. They prevent cross-task or + between-turn state from being silently adopted. - `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. diff --git a/.agents/skills/pair-workflow/template.md b/.agents/skills/pair-workflow/template.md index 67cdf222..266c3db9 100644 --- a/.agents/skills/pair-workflow/template.md +++ b/.agents/skills/pair-workflow/template.md @@ -21,6 +21,15 @@ base_commit: BASE_COMMIT base_branch: BASE_BRANCH start_commit: START_COMMIT pr_base_branch: PR_BASE_BRANCH +pr_base_tip: PR_BASE_TIP +task_branch: TASK_BRANCH +github_repo: GITHUB_REPO +origin_fetch_url: ORIGIN_FETCH_URL +origin_push_url: ORIGIN_PUSH_URL +git_config_state: GIT_CONFIG_STATE +publication_head: none +expected_git_state: EXPECTED_GIT_STATE +expected_worktree_state: EXPECTED_WORKTREE_STATE 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 4c62d853..30fc7cc8 100644 --- a/.agents/skills/tester/SKILL.md +++ b/.agents/skills/tester/SKILL.md @@ -28,8 +28,10 @@ description: > - Codegen behavior is verified end-to-end in `codegen/tests` (`:codegen-tests`), which runs generation against test Protobuf definitions and asserts on the generated API; add coverage there for generator changes. -- Do not skip codegen-related Gradle tasks when generator behavior, Protobuf - schemas, or generated API contracts are part of the change: rebuild +- For published model Protobuf declarations or Kotlin model extensions in + `proto-values`, run `:proto-values:test` or `:proto-values:check` so the + module generates and compiles accessors against the changed model. +- When generator behavior or generated API contracts change, rebuild `codegen/plugins` and run `:codegen-tests:test` so assertions run against freshly generated code, not outputs left over from a previous build. - Avoid tests that depend on a real Spine server, network resources, or local diff --git a/.agents/workflows/pair-test.sh b/.agents/workflows/pair-test.sh index f3b112d0..5d0393d4 100755 --- a/.agents/workflows/pair-test.sh +++ b/.agents/workflows/pair-test.sh @@ -48,8 +48,13 @@ sandbox() { git -C "$repo" add -A git -C "$repo" commit -qm init git init -q --bare "${SANDBOX}/origin.git" + git -C "${SANDBOX}/origin.git" symbolic-ref HEAD refs/heads/master git -C "$repo" remote add origin "${SANDBOX}/origin.git" git -C "$repo" push -qu origin master + git clone -q "${SANDBOX}/origin.git" "${SANDBOX}/external" + git -C "${SANDBOX}/external" config user.email external@example.com + git -C "${SANDBOX}/external" config user.name External + export STUB_EXTERNAL_REPO="${SANDBOX}/external" # Stub gh: issue metadata from files, and a recorded no-op for pr create. cat > "${SANDBOX}/bin/gh" <<'GH' @@ -75,7 +80,14 @@ case "$1 $2" in else printf 'o/r\n' fi ;; - "pr view") [[ -n "${STUB_PR_EXISTS:-}" ]] && printf '%s\n' "$STUB_PR_EXISTS" || exit 1 ;; + "pr view") + [[ -n "${STUB_PR_EXISTS:-}" ]] || exit 1 + if [[ "$*" == *'@tsv'* ]]; then + printf '%s\t%s\t%s\n' "$STUB_PR_EXISTS" \ + "${STUB_PR_BASE:-master}" "${STUB_PR_HEAD:-a-test-issue}" + else + printf '%s\n' "$STUB_PR_EXISTS" + fi ;; "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 @@ -115,6 +127,19 @@ setfm() { ' "$doc" > "$doc.t" && mv "$doc.t" "$doc" } root() { git rev-parse --show-toplevel; } +# Runs Git outside the agent's inherited Trace2 session, standing in for an +# editor refresh, another checkout, or GitHub changing the remote mid-turn. +outside_git() { env -u GIT_TRACE2_EVENT git "$@"; } +# Advances origin/master in the independent checkout and fetches the result +# into the repository under test, all outside the agent process trace. +external_master_advance() { + local marker="${STUB_EXTERNAL_REPO}/external-$$.txt" + outside_git -C "$STUB_EXTERNAL_REPO" checkout -q master + printf 'external\n' > "$marker" + outside_git -C "$STUB_EXTERNAL_REPO" add "${marker##*/}" + outside_git -C "$STUB_EXTERNAL_REPO" commit -qm "External change" + outside_git -C "$STUB_EXTERNAL_REPO" push -q origin master +} # 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() { @@ -173,6 +198,100 @@ case "${STUB_MISBEHAVE:-}" in setfm status plan-review-requested; setfm turn agent2; exit 0 ;; restage) echo y >> README.md; git add README.md setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + git-write-exit) git branch "stub-rogue-$$"; exit 19 ;; + review-edit-exit) + if [[ "$(fm status)" == plan-review-requested ]]; then + printf 'reviewer edit\n' >> "$(root)/README.md" + exit 19 + fi ;; + rewrite-transcript) + if [[ "$(fm status)" == plan-review-requested ]]; then + printf 'rewritten audit\n' \ + > "$(root)/.agents/work/${PAIR_SLUG}/turns/01-agent1.log" + fi ;; + config-write) git config pair.agentMutation true + setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + trace-read-only) + git tag --format '%(refname)' >/dev/null + git log --oneline -- remote set-url >/dev/null ;; + combined-drop) + git branch -dr origin/obsolete + setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + unignore-work) + if [[ "$(fm status)" == plan-reviewed ]]; then + sed '/[.]agents\/work\//d' "$(root)/.gitignore" \ + > "$(root)/.gitignore.t" + mv "$(root)/.gitignore.t" "$(root)/.gitignore" + fi ;; + nonascii-path) + if [[ "$(fm status)" == plan-reviewed ]]; then + printf 'reviewed\n' > "$(root)/résumé.txt" + fi ;; + codex-ref) # The Codex CLI's own turn bookkeeping, written by the process + # rather than the agent. Once, then this turn proceeds normally, + # so the assertion is that the whole run still finishes. + [[ "$(fm status)" != plan-requested ]] || + git update-ref "refs/codex/turn-diffs/checkpoints/stub-$$" HEAD ;; + fetch-lands) # A fetch arriving from a background process. Once, then this + # turn proceeds normally so the whole run should finish. + if [[ "$(fm status)" == plan-requested ]]; then + git branch --show-current >/dev/null + git remote -v >/dev/null + git tag --list >/dev/null + external_master_advance + outside_git -C "$(root)" fetch -q origin + fi ;; + fetch-known) # The fetched commit already has a local branch, so + # reachability cannot identify its provenance. + if [[ "$(fm status)" == plan-requested ]]; then + outside_git -C "$STUB_EXTERNAL_REPO" fetch -q origin \ + known:refs/remotes/origin/known + outside_git -C "$STUB_EXTERNAL_REPO" push -q origin \ + refs/remotes/origin/known:master + outside_git -C "$(root)" fetch -q origin + fi ;; + fetch-branch) # A newly fetched remote branch is unrelated activity too. + if [[ "$(fm status)" == plan-requested ]]; then + outside_git -C "$STUB_EXTERNAL_REPO" checkout -qb external-branch + printf 'branch\n' > "$STUB_EXTERNAL_REPO/branch.txt" + outside_git -C "$STUB_EXTERNAL_REPO" add branch.txt + outside_git -C "$STUB_EXTERNAL_REPO" commit -qm "External branch" + outside_git -C "$STUB_EXTERNAL_REPO" push -q origin \ + HEAD:external-branch + outside_git -C "$(root)" fetch -q origin + fi ;; + fetch-tag) # Tags can arrive through the same background fetch. + if [[ "$(fm status)" == plan-requested ]]; then + outside_git -C "$STUB_EXTERNAL_REPO" tag external-tag + outside_git -C "$STUB_EXTERNAL_REPO" push -q origin external-tag + outside_git -C "$(root)" fetch -q --tags origin + fi ;; + fetch-prune) # A background prune explains a vanished tracking ref. + if [[ "$(fm status)" == plan-requested ]]; then + outside_git -C "$STUB_EXTERNAL_REPO" push -q origin \ + --delete obsolete + outside_git -C "$(root)" fetch -q --prune origin + fi ;; + agent-fetch) # Even a legitimate remote advance is forbidden when the + # agent itself fetches it. + external_master_advance + git fetch -q origin + setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + remote-push) git push -q origin "HEAD:refs/heads/stub-rogue-$$" + setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + dangling-push) oid="$(git commit-tree -p HEAD -m rogue 'HEAD^{tree}')" + git push -q origin "${oid}:refs/heads/stub-rogue-$$" + setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + push-tag) git push -q origin existing-tag + setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + agent-tag) git tag "stub-rogue-$$" + setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + drop-remote) git update-ref -d refs/remotes/origin/master + setfm status plan-review-requested; setfm turn agent2; exit 0 ;; + missing-trace) external_master_advance + outside_git -C "$(root)" fetch -q origin + : > "$GIT_TRACE2_EVENT" + 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 ;; @@ -534,6 +653,14 @@ check "first turn writes the required Task" \ "$(grep -q 'Implement the issue' "$R/.agents/work/issue-7/plan.md" && echo 0 || echo 1)" cleanup +sandbox +run "$R" 7 >/dev/null +printf 'after done\n' >> "$R/README.md" +run "$R" status 7 +want "status does not report changed completed work as intact" 0 \ + "work: completion invalidated" +cleanup + # --- user-facing status --------------------------------------------------- sandbox export AGENT1_CMD="${SANDBOX}/bin/claude" @@ -683,12 +810,94 @@ want "round jump refused" 1 "requires it to stay unchanged" cleanup # --- git guard (RF-02) ---------------------------------------------------- +sandbox +printf 'private scratch\n' > "$R/preexisting.txt" +git -C "$R" config status.showUntrackedFiles no +run "$R" start 7 +want "dirty-start detection cannot hide untracked files through config" 1 \ + "worktree has uncommitted changes" +cleanup + +# Different slugs still share one worktree. A task that started from the clean +# state must not adopt edits another task produced before its next turn. +sandbox +PAIR_SLUG=issue-7 run "$R" start 7 >/dev/null +PAIR_SLUG=issue-8 run "$R" start 8 >/dev/null +PAIR_SLUG=issue-7 run "$R" step 7 >/dev/null +PAIR_SLUG=issue-7 run "$R" step 7 >/dev/null +PAIR_SLUG=issue-7 run "$R" step 7 >/dev/null +PAIR_SLUG=issue-8 run "$R" step 8 +want "one slug cannot adopt another slug's worktree edits" 1 \ + "worktree content changed between pair-workflow turns" +cleanup + sandbox; STUB_MISBEHAVE=git-write run "$R" 7 want "branch creation caught" 1 "Git state moved"; cleanup sandbox; STUB_MISBEHAVE=stash run "$R" 7 want "git stash caught" 1 "Git state moved"; cleanup sandbox; STUB_MISBEHAVE=restage run "$R" 7 want "restaged blob caught" 1 "Git state moved"; cleanup +sandbox; STUB_MISBEHAVE=git-write-exit run "$R" 7 +want "a nonzero agent cannot bypass the Git guard" 1 "Git state moved"; cleanup +sandbox; STUB_MISBEHAVE=review-edit-exit run "$R" 7 +want "a nonzero reviewer cannot bypass the worktree guard" 1 \ + "worktree content changed"; cleanup +sandbox; STUB_MISBEHAVE=rewrite-transcript run "$R" 7 +want "an agent cannot rewrite an earlier transcript" 1 \ + "rewrote an earlier transcript"; cleanup +sandbox; STUB_MISBEHAVE=config-write run "$R" 7 +want "repository configuration changes are Git-state changes" 1 "Git state moved"; cleanup +sandbox; STUB_MISBEHAVE=trace-read-only run "$R" 7 +want "read-only commands containing ref-writing words stay allowed" 0; cleanup +sandbox +git -C "$R" push -qu origin HEAD:obsolete +git -C "$R" fetch -q origin +STUB_MISBEHAVE=combined-drop run "$R" 7 +want "combined branch flags cannot hide a tracking-ref deletion" 1 \ + "ref-writing Git command"; cleanup +sandbox; STUB_MISBEHAVE=codex-ref run "$R" 7 +want "Codex turn-diff ref does not trip the guard" 0; cleanup +sandbox; STUB_MISBEHAVE=fetch-lands run "$R" 7 +want "a fetch landing mid-turn does not trip the guard" 0; cleanup +sandbox; STUB_MISBEHAVE=fetch-lands run "$R" 7 +want "a landed fetch is still reported" 0 "outside agent1's process"; cleanup +sandbox +git -C "$R" checkout -qb known +printf 'known\n' > "$R/known.txt" +git -C "$R" add known.txt +git -C "$R" commit -qm "Known locally" +git -C "$R" push -qu origin known +git -C "$R" checkout -q master +STUB_MISBEHAVE=fetch-known run "$R" 7 +want "a fetch to an already reachable commit is allowed" 0; cleanup +sandbox; STUB_MISBEHAVE=fetch-branch run "$R" 7 +want "a newly fetched remote branch is allowed" 0; cleanup +sandbox; STUB_MISBEHAVE=fetch-tag run "$R" 7 +want "a newly fetched tag is allowed" 0; cleanup +sandbox +git -C "$R" push -qu origin HEAD:obsolete +STUB_MISBEHAVE=fetch-prune run "$R" 7 +want "a background fetch prune is allowed" 0; cleanup +sandbox; STUB_MISBEHAVE=agent-fetch run "$R" 7 +want "an agent fetch that moves a ref is caught" 1 "ref-writing Git command"; cleanup +sandbox; STUB_MISBEHAVE=remote-push run "$R" 7 +want "a push to origin caught" 1 "ref-writing Git command"; cleanup +sandbox; STUB_MISBEHAVE=dangling-push run "$R" 7 +want "a push of an unreachable commit is caught" 1 "ref-writing Git command"; cleanup +sandbox +git -C "$R" tag existing-tag +git -C "$R" push -q origin existing-tag +STUB_MISBEHAVE=push-tag run "$R" 7 +want "a push with no local ref movement is caught" 1 \ + "ref-writing Git command"; cleanup +sandbox; STUB_MISBEHAVE=agent-tag run "$R" 7 +want "tag creation caught" 1 "ref-writing Git command"; cleanup +sandbox; STUB_MISBEHAVE=drop-remote run "$R" 7 +want "an agent deleting a remote-tracking ref is caught" 1 \ + "ref-writing Git command"; cleanup +sandbox; STUB_MISBEHAVE=missing-trace run "$R" 7 +want "an external-ref change without provenance is caught" 1 \ + "Git trace is missing or unreadable"; cleanup # --- immutable metadata (RF-05) ------------------------------------------- sandbox; STUB_MISBEHAVE=rewrite-meta run "$R" 7 @@ -782,7 +991,8 @@ 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" +want "agent2 editing a source file caught" 1 \ + "worktree content changed during agent2's review" cleanup sandbox @@ -797,7 +1007,7 @@ 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" + "worktree content changed during agent2's review" cleanup # --- section ownership (RR3-02) ------------------------------------------- @@ -941,7 +1151,7 @@ cleanup sandbox export PR_BASE_BRANCH=missing run "$R" 7 --cp -want "missing PR target is refused during setup" 1 "origin/missing is unavailable" +want "missing PR target is refused during setup" 1 "could not refresh origin/missing" check "missing PR target starts no agent turn" \ "$([[ ! -d "$R/.agents/work/issue-7/turns" ]] && echo 0 || echo 1)" unset PR_BASE_BRANCH @@ -979,6 +1189,19 @@ check "the surviving transcript is not overwritten" \ "$R/.agents/work/issue-7/turns/02-agent1.log" && echo 0 || echo 1)" cleanup +# A surviving Trace2 sidecar also owns its sequence number. Reusing it would +# append stale commands and let an old marker stand in for the current turn. +sandbox; run "$R" start 7 +mkdir -p "$R/.agents/work/issue-7/turns" +printf '{"event":"cmd_name","name":"version"}\n' \ + > "$R/.agents/work/issue-7/turns/04-agent1.git-trace.json" +run "$R" step 7 +want "an orphan Git trace does not get reused" 0 +check "an orphan Git trace advances the shared sequence" \ + "$([[ -f "$R/.agents/work/issue-7/turns/05-agent1.git-trace.json" ]] \ + && echo 0 || echo 1)" +cleanup + # --- execution boundary (RR2-06) ------------------------------------------ sandbox AGENT1_CMD="${SANDBOX}/bin/stub-agent --dangerously-skip-permissions" run "$R" 7 @@ -1146,6 +1369,20 @@ check "title with & is written literally" \ "$(grep -q 'Fix A & B' "$D" && echo 0 || echo 1)" cleanup +sandbox +STUB_ISSUE_TITLE='Keep ISSUE_URL literal' run "$R" start 7 +check "placeholder text inside an issue title stays literal" \ + "$(grep -q 'issue_title: Keep ISSUE_URL literal' \ + "$R/.agents/work/issue-7/plan.md" && echo 0 || echo 1)" +cleanup + +sandbox +STUB_ISSUE_TITLE='Поліпшити пошук' run "$R" start 7 +check "a non-ASCII issue title gets a safe branch fallback" \ + "$(grep -qx 'task_branch: issue-7' \ + "$R/.agents/work/issue-7/plan.md" && echo 0 || echo 1)" +cleanup + # --- publish path (RF-06, RF-07, RF-08) ----------------------------------- # Fills the template's existing ## Pull Request section. Appending a second one # would not work: section() reads the first heading it finds. @@ -1221,6 +1458,41 @@ manual_plan_variant() { } {print}' "$d" > "$d.t" && mv "$d.t" "$d" } +# The scratch root is an explicit driver exclusion, not a mutable .gitignore +# convention. A task may change that rule without reviewing or publishing its +# own plans, transcripts, and traces. +sandbox +STUB_BUMP=2.0.0-SNAPSHOT.2 STUB_MISBEHAVE=unignore-work run "$R" 7 >/dev/null +pr_section "$R" +run "$R" 7 --cp +want "removing the work-root ignore rule does not publish scratch artifacts" 0 \ + "draft pull request" +check "the published tree excludes pair-workflow scratch files" \ + "$([[ -z "$(git -C "$R" ls-tree -r --name-only HEAD .agents/work)" ]] \ + && echo 0 || echo 1)" +cleanup + +sandbox +STUB_ISSUE_TITLE='Fix the title...' STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null +pr_section "$R" +run "$R" 7 --cp +want "all trailing periods are removed from a PR title" 0 "draft pull request" +check "the recorded PR title has no trailing period" \ + "$([[ "$(recorded_arg_after "$STUB_PR_RECORD" --title)" == 'Fix the title' ]] \ + && echo 0 || echo 1)" +cleanup + +sandbox +git -C "$R" tag -a unrelated-release -m "Unrelated release" +git -C "$R" config push.followTags true +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null +pr_section "$R" +run "$R" 7 --cp +want "publication neutralizes automatic tag following" 0 "draft pull request" +check "an unrelated annotated tag was not published" \ + "$([[ -z "$(git -C "$R" ls-remote --tags origin)" ]] && echo 0 || echo 1)" +cleanup + sandbox; run "$R" 7 >/dev/null run "$R" 7 --cp; want "PR refused without Summary/Changes" 1 "requires exact" cleanup @@ -1243,6 +1515,13 @@ check "nothing was committed on refusal" \ "$([[ "$current_branch" == master ]] && echo 0 || echo 1)" cleanup +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +git -C "$R" config pair.externalMutation true +run "$R" 7 --cp +want "publication refuses changed Git transport configuration" 1 \ + "effective Git configuration changed" +cleanup + 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" \ @@ -1253,6 +1532,28 @@ STUB_PR_EXISTS=https://github.com/o/r/pull/1 run "$R" 7 --cp want "re-run finds the existing PR" 0 "already open" cleanup +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +STUB_PR_EXISTS=https://github.com/o/r/pull/1 STUB_PR_BASE=staging run "$R" 7 --cp +want "an existing PR on the wrong base is not accepted as published" 1 \ + "different head or base" +cleanup + +sandbox +git -C "$STUB_EXTERNAL_REPO" checkout -qb a-test-issue +printf 'collision\n' > "$STUB_EXTERNAL_REPO/collision.txt" +git -C "$STUB_EXTERNAL_REPO" add collision.txt +git -C "$STUB_EXTERNAL_REPO" commit -qm "Colliding task branch" +git -C "$STUB_EXTERNAL_REPO" push -q origin a-test-issue +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null +pr_section "$R" +run "$R" 7 --cp +want "a remote task-branch collision fails before local publication commits" 1 \ + "contains history outside this task's starting point" +check "remote collision leaves the starting branch untouched" \ + "$([[ "$(git -C "$R" rev-parse --abbrev-ref HEAD)" == master ]] \ + && echo 0 || echo 1)" +cleanup + # A failed `gh pr create` must leave the push intact and stay retryable. 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 @@ -1260,6 +1561,29 @@ 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 +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 >/dev/null +git -C "$R" commit -qm "Unreviewed empty commit" --allow-empty +run "$R" 7 --cp +want "a content-equivalent commit cannot enter a publication retry" 1 \ + "moved beyond the driver's recorded publication head" +cleanup + +# A retry trusts the exact server head, not a potentially stale or unrelated +# local upstream. External commits on the task branch must never enter the PR. +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 >/dev/null +git -C "$STUB_EXTERNAL_REPO" fetch -q origin +git -C "$STUB_EXTERNAL_REPO" checkout -qB a-test-issue origin/a-test-issue +printf 'remote-only\n' > "$STUB_EXTERNAL_REPO/remote-only.txt" +git -C "$STUB_EXTERNAL_REPO" add remote-only.txt +git -C "$STUB_EXTERNAL_REPO" commit -qm "External task-branch commit" +git -C "$STUB_EXTERNAL_REPO" push -q origin a-test-issue +run "$R" 7 --cp +want "an externally advanced task branch is not treated as already pushed" 1 \ + "contains commits outside this reviewed run" +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. @@ -1273,6 +1597,16 @@ stack_on() { # stack_on [version] — creates one commit outside git -C "$1" commit -qm "Earlier work under review" } +# Advances the PR target from another checkout, as an unrelated GitHub change +# would, then refreshes the repository under test. +advance_origin_master() { # advance_origin_master + printf 'unrelated\n' > "$STUB_EXTERNAL_REPO/unrelated.txt" + git -C "$STUB_EXTERNAL_REPO" add unrelated.txt + git -C "$STUB_EXTERNAL_REPO" commit -qm "Unrelated target change" + git -C "$STUB_EXTERNAL_REPO" push -q origin master + git -C "$1" fetch -q origin +} + 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 @@ -1416,17 +1750,73 @@ 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. +# An unrelated target advance shares only the recorded start with the task. +# It changes GitHub state without changing the task or the PR changeset. +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +advance_origin_master "$R" +run "$R" 7 --cp; want "an unrelated target advance still publishes" 0 \ + "draft pull request" +cleanup + +# Publication refreshes the target itself; correctness cannot depend on an IDE +# or the caller fetching between the external update and --create-pr. +sandbox; STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +printf 'not fetched locally\n' > "$STUB_EXTERNAL_REPO/not-fetched.txt" +git -C "$STUB_EXTERNAL_REPO" add not-fetched.txt +git -C "$STUB_EXTERNAL_REPO" commit -qm "Unfetched target change" +git -C "$STUB_EXTERNAL_REPO" push -q origin master +run "$R" 7 --cp +want "publication refreshes a stale local target ref" 0 "draft pull request" +cleanup + +# A parent that merges mid-run moves the merge-base forward. That subtracts +# from the pull request rather than adding to it: the carried commits are in +# the target now, and the task's own reviewed changeset is measured from +# `start_commit` either way. The baseline is recomputed and the run publishes, +# because the merge was nobody-in-the-run's doing and the review still stands. 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" \ +run "$R" 7 --cp; want "a parent merging mid-run publishes from the new base" 0 \ + "draft pull request" +want "the moved baseline is reported" 0 "baseline moves from" +check "the merged parent is no longer carried as stacking" \ + "$(printf '%s' "$OUT" | grep -q 'stacked on' && echo 1 || echo 0)" +cleanup + +# A publication retry may see the target absorb one of the task commits after +# the failed push/create sequence. That is beyond the immutable task boundary, +# so advancing the baseline would silently omit reviewed work from the PR. +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 >/dev/null +recorded_start="$(awk '/^start_commit: / {print $2}' \ + "$R/.agents/work/issue-7/plan.md")" +first_task_commit="$(git -C "$R" rev-list --reverse \ + "${recorded_start}..HEAD" | head -n 1)" +git -C "$R" push -q origin "${first_task_commit}:master" +git -C "$R" fetch -q origin +run "$R" 7 --cp +want "a target that absorbed task commits is refused" 1 \ + "absorbed commits produced by this run" +check "no PR was created after task absorption" \ + "$([[ -f "$STUB_PR_RECORD" ]] && echo 1 || echo 0)" +cleanup + +# Advancing is not the same as being rewritten. A target force-pushed back +# behind the recorded base leaves a scope the driver cannot reconstruct, and +# that still refuses rather than guessing. +sandbox +printf 'later\n' > "$R/later.txt" +git -C "$R" add -A +git -C "$R" commit -qm "Later work on master" +git -C "$R" push -q origin master +rewound="$(git -C "$R" rev-parse --short 'HEAD^')" +STUB_BUMP=2.0.0-SNAPSHOT.2 run "$R" 7 >/dev/null; pr_section "$R" +git -C "$R" push -qf origin "${rewound}:master" +run "$R" 7 --cp; want "a rewritten target is refused, not published" 1 \ + "was rewritten after setup" +check "nothing was published when the target was rewritten" \ "$([[ -f "$STUB_PR_RECORD" ]] && echo 1 || echo 0)" cleanup @@ -1485,15 +1875,26 @@ 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" +run "$R" 7 --cp; want "post-review edits are not published" 1 \ + "worktree changed after the task reached done" 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 STUB_MISBEHAVE=nonascii-path run "$R" 7 >/dev/null +pr_section "$R" +printf 'changed after review\n' > "$R/résumé.txt" +run "$R" 7 --cp +want "non-ASCII path content remains bound to the completed review" 1 \ + "worktree changed after the task reached done" +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" +want "post-review executable-bit changes are not published" 1 \ + "worktree changed after the task reached done" cleanup # The prospective PR baseline is the merge-base with master, not whatever HEAD @@ -1506,7 +1907,7 @@ 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 \ +expected="$(git -C "$R" rev-parse \ "$(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)" @@ -1667,22 +2068,19 @@ for missing in long-bullet no-setup no-expected no-covers; do cleanup done -# The first publication commit may succeed before the task commit fails. A -# retry must recognize the committed version and reports and finish the -# remaining steps. +# Workstation hooks are outside the reviewed changeset and must not run inside +# the driver's publication transaction. 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" +touch "$(git rev-parse --show-toplevel)/hook-ran" +exit 1 HOOK chmod +x "$R/.git/hooks/commit-msg" run "$R" 7 --cp -want "task commit failure stops after version commit" 1 "task commit failed" -check "version commit survives task commit failure" \ - "$(git -C "$R" log --format=%s | grep -q '^Bump version' && echo 0 || echo 1)" -rm -f "$R/.git/hooks/commit-msg" -run "$R" 7 --cp -want "retry after task commit failure publishes" 0 "draft pull request" +want "publication ignores workstation commit hooks" 0 "draft pull request" +check "the workstation hook did not execute" \ + "$([[ ! -e "$R/hook-ran" ]] && echo 0 || echo 1)" cleanup # --- agent execution environment (RF-11) ---------------------------------- diff --git a/.agents/workflows/pair.sh b/.agents/workflows/pair.sh index 505f39d8..f3ee26a3 100755 --- a/.agents/workflows/pair.sh +++ b/.agents/workflows/pair.sh @@ -127,6 +127,50 @@ readonly WORK_ROOT="${REPO_ROOT}/.agents/work" readonly SKILL="${REPO_ROOT}/.agents/skills/pair-workflow/SKILL.md" readonly TEMPLATE="${REPO_ROOT}/.agents/skills/pair-workflow/template.md" +# Repository-redirection variables make `git -C` operate on something other +# than this checkout. +# Refuse them once, before any repository or GitHub state is read or written. +validate_repository_environment() { + local name + for name in GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY \ + GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_COMMON_DIR GIT_NAMESPACE \ + GIT_CONFIG GIT_CONFIG_SYSTEM GIT_CONFIG_GLOBAL GIT_CONFIG_COUNT \ + GIT_QUARANTINE_PATH GIT_PUSH_OPTION_COUNT; do + [[ -z "${!name+x}" ]] \ + || die "${name} is set; unset repository-redirection variables "\ +"before running pair.sh" + done +} + +# A strong, repository-native digest for canonical streams. Unlike POSIX +# `cksum`, this is not a 32-bit tripwire an agent can deliberately collide. +digest_stream() { + git -C "$REPO_ROOT" hash-object --stdin +} + +# Strong digest of the effective local, included, global, and system Git config. +git_config_digest() { + git -C "$REPO_ROOT" config -z --includes --list | digest_stream +} + +# Paths below the driver-owned work root are never implementation input. Keep +# them out even if the task itself edits the mutable .gitignore rule. +declare -ar WORKTREE_PATHS=(-- . ':(exclude).agents/work' ':(exclude).agents/work/**') + +# Git writes performed by the driver must not execute workstation hooks or +# inherit push.followTags. The latter can publish unrelated annotated tags even +# with an explicit branch refspec. +publication_git() { + git -c core.hooksPath=/dev/null -c push.followTags=false \ + -c remote.origin.mirror=false \ + -C "$REPO_ROOT" "$@" +} + +# Ambient GH_REPO must not retarget issue reads or pull-request publication. +github() { + env -u GH_REPO gh "$@" +} + # Models and effort are pinned rather than left to each CLI's default, so a # review is reproducible and the two sides stay the models this workflow was # tuned against. Codex's settings are passed explicitly because @@ -206,7 +250,26 @@ ALLOW_UNSAFE_AGENTS=0 # swapping the commands back to their defaults. AGENTS_SWAPPED=0 -die() { printf 'pair: %s\n' "$1" >&2; exit 1; } +ACTIVE_TURN_DOC="" +POISONING_ACTIVE_TURN=0 +poison_active_turn() { + # Once an agent has started, any guard or process failure makes its partial + # document unsafe to resume as a new baseline. Poison the persisted state so + # a later invocation cannot silently accept that failed turn. + if [[ -n "$ACTIVE_TURN_DOC" && -f "$ACTIVE_TURN_DOC" \ + && "$POISONING_ACTIVE_TURN" -eq 0 ]]; then + POISONING_ACTIVE_TURN=1 + set_frontmatter "$ACTIVE_TURN_DOC" \ + "expected_git_state=invalid-after-failed-turn" \ + "expected_worktree_state=invalid-after-failed-turn" 2>/dev/null || true + fi +} + +die() { + poison_active_turn + 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 # subshell, so validation would report a problem and let the run continue. @@ -216,6 +279,22 @@ info() { printf 'pair: %s\n' "$1" >&2; } doc_for() { printf '%s/%s/plan.md' "$WORK_ROOT" "$1"; } +# Agents can write the ignored work root, so directory symlinks cannot be +# trusted as destinations for plans, snapshots, transcripts, or Trace2 files. +validate_task_directory() { + local slug="$1" dir="${WORK_ROOT}/${slug}" + [[ ! -L "$WORK_ROOT" ]] || die "${WORK_ROOT} must not be a symlink" + [[ ! -L "$dir" ]] || die "${dir} must not be a symlink" +} + +ensure_task_directory() { + local slug="$1" dir="${WORK_ROOT}/${slug}" + validate_task_directory "$slug" + mkdir -p "$WORK_ROOT" + mkdir -p "$dir" + [[ -d "$dir" && ! -L "$dir" ]] || die "could not create a safe task directory at ${dir}" +} + # Returns the executable recorded in the working document for an agent command. agent_command_name() { printf '%s' "$1" | awk '{print $1}'; } @@ -644,50 +723,184 @@ pr_base_ref() { 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 -# because `git push` moves them, which makes a push detectable locally — and a -# PR is detectable in turn, since it needs a push first. +# Exactly one URL must own each direction. Multiple push URLs make one push +# publish to more repositories than the PR metadata can describe. +remote_url() { + local direction="$1" flag="" urls count + [[ "$direction" == "push" ]] && flag="--push" + urls="$(git -C "$REPO_ROOT" remote get-url --all $flag origin 2>/dev/null)" \ + || return 1 + count="$(printf '%s\n' "$urls" | awk 'NF { count++ } END { print count + 0 }')" + [[ "$count" -eq 1 ]] || return 1 + printf '%s' "$urls" +} + +# Refreshes one named remote branch without fetching tags or running hooks. +# The driver calls this before it records or publishes against a target, so an +# IDE's fetch schedule cannot silently define the PR scope. +refresh_remote_branch() { + local branch="$1" + publication_git fetch --no-tags -q origin \ + "+refs/heads/${branch}:refs/remotes/origin/${branch}" +} + +# Stable porcelain that cannot be weakened by status.showUntrackedFiles and +# never treats pair-workflow scratch artifacts as implementation changes. +worktree_porcelain() { + git -C "$REPO_ROOT" status --porcelain=v1 --untracked-files=all \ + "${WORKTREE_PATHS[@]}" +} + +# Snapshot of everything the workflow forbids an agent from touching and whose +# every movement is local: the checked-out commit and branch, every local ref +# except tags, and the staged index. Remote-tracking refs and tags can arrive +# through a background fetch, so external_ref_state() snapshots them and the +# per-turn Git trace identifies whether the agent process ran a command capable +# of moving them. git_state() { git -C "$REPO_ROOT" rev-parse HEAD git -C "$REPO_ROOT" rev-parse --abbrev-ref HEAD - # Every ref, not selected namespaces: `refs/stash` belongs here too, or - # `git stash` could move the user's work out of the tree and pass. - git -C "$REPO_ROOT" for-each-ref --format='%(refname) %(objectname)' + # Every local ref, not selected namespaces: `refs/stash` belongs here too, + # or `git stash` could move the user's work out of the tree and pass. One + # exclusion is `refs/codex/turn-diffs/`, which the Codex CLI writes as its + # own bookkeeping — the process, not the agent obeying its prompt. It lands + # intermittently mid-turn and aborts runs the agents ran correctly. Nothing + # forbidden can hide there: a checkpoint ref neither commits nor pushes nor + # moves work out of the worktree, which is fingerprinted separately anyway. + # The others are `refs/remotes/` and `refs/tags/`, which + # external_ref_state() covers. + # `--exclude` is inert without a positive pattern and matches whole path + # components, so both `refs/` and the slash-free prefixes are load-bearing. + git -C "$REPO_ROOT" for-each-ref --exclude=refs/codex/turn-diffs \ + --exclude=refs/remotes --exclude=refs/tags \ + --format='%(refname) %(objectname)' refs/ # Staged blob ids, modes, and index flags. `--stage` alone misses flags, # so `assume-unchanged` or `skip-worktree` could hide a file from status, # from review, and from later staging while passing this check. git -C "$REPO_ROOT" ls-files -v -s + # Git configuration and operation sentinels can redirect a later fetch or + # push, install filters, or leave the checkout mid-merge without moving a + # ref. They are part of the workflow's control state even though they live + # outside the ordinary worktree. + printf 'effective-config=%s\n' "$(git_config_digest)" + local name path + for name in MERGE_HEAD REBASE_HEAD CHERRY_PICK_HEAD REVERT_HEAD \ + MERGE_MSG SQUASH_MSG; do + path="$(git -C "$REPO_ROOT" rev-parse --git-path "$name")" + [[ -f "$path" ]] \ + && printf '%s=%s\n' "$name" \ + "$(git -C "$REPO_ROOT" hash-object --no-filters "$path")" + done + return 0 } -# 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 - ) +# Compact forms persisted between turns. A task that resumes in a worktree +# changed by another pair run or another process must stop before treating that +# unrelated state as its own baseline. +git_state_digest() { + git_state | digest_stream +} + +# Snapshot of refs whose movement may come from GitHub rather than the agent: +# remote-tracking refs and tags. Include symbolic targets so changing +# `origin/HEAD` is visible even when its object id stays the same. +external_ref_state() { + git -C "$REPO_ROOT" for-each-ref \ + --format='%(refname)%09%(objectname)%09%(symref)' \ + refs/remotes/ refs/tags/ +} + +# Strong, NUL-safe fingerprint of tracked and untracked implementation state. +worktree_state_digest() { + changeset_digest HEAD +} + +# Whether the trace is intact and contains the marker written immediately +# before the agent starts. Without it the turn has no trustworthy Git-command +# provenance, so callers fail closed. +git_trace_is_usable() { + local trace="$1" marker="$2" + [[ -s "$trace" ]] || return 1 + jq -s -e --arg marker "pair.traceMarker=${marker}" ' + . as $events | + any($events[]; + .event == "start" and + ((.argv // []) | index($marker)) != null and + (.sid as $sid | + any($events[]; + .event == "cmd_name" and .sid == $sid and .name == "version" + ) + ) + ) + ' "$trace" >/dev/null 2>&1 +} + +# Git commands in the agent process tree that can move a remote-tracking ref or +# tag. This uses command provenance instead of guessing from commit +# reachability. Commands with both read-only and writing forms are classified +# by their arguments, with ambiguous forms taking the direction that stops. +git_trace_ref_writes() { + local trace="$1" + jq -s -r ' + . as $events | + def after($command): + .argv as $args | + ($args | index($command)) as $at | + if $at == null then [] else $args[($at + 1):] end; + def has($args; $option): + any($args[]; . == $option or startswith($option + "=")); + def short_has($args; $letter): + any($args[]; + type == "string" and + test("^-[^-]*" + $letter) + ); + + $events[] | + select(.event == "start" and (.argv | type == "array")) | + .sid as $sid | + ([ $events[] | + select(.event == "cmd_name" and .sid == $sid) | .name + ][0] // "") as $command | + (after($command)) as $args | + if ($command == "fetch" or $command == "pull" or + $command == "push" or $command == "receive-pack" or + $command == "send-pack") then $command + elif ($command == "branch" and + (short_has($args; "r") or has($args; "--remotes")) and + (short_has($args; "d") or short_has($args; "D") or + has($args; "--delete"))) then "branch" + elif ($command == "remote" and (($args[0] // "") as $sub | + $sub == "add" or $sub == "prune" or $sub == "remove" or + $sub == "rename" or $sub == "rm" or + $sub == "set-branches" or $sub == "set-head" or + $sub == "set-url" or $sub == "update")) then "remote" + elif ($command == "symbolic-ref" and + (short_has($args; "d") or has($args; "--delete") or + ([$args[] | select(startswith("-") | not)] | length) > 1)) + then "symbolic-ref" + elif ($command == "tag" and ( + short_has($args; "d") or has($args; "--delete") or + short_has($args; "f") or has($args; "--force") or + short_has($args; "a") or has($args; "--annotate") or + short_has($args; "s") or has($args; "--sign") or + short_has($args; "u") or has($args; "--local-user") or + short_has($args; "m") or has($args; "--message") or + short_has($args; "F") or has($args; "--file") or + (((short_has($args; "l") or short_has($args; "n") or + short_has($args; "v") or has($args; "--list") or + has($args; "--verify") or has($args; "--contains") or + has($args; "--no-contains") or has($args; "--points-at") or + has($args; "--merged") or has($args; "--no-merged") or + has($args; "--format") or has($args; "--sort") or + has($args; "--column")) | not) and + any($args[]; startswith("-") | not)) + )) then "tag" + elif ($command == "update-ref" and ($args | length) > 0 and + (has($args; "--stdin") or + (any($args[]; startswith("refs/codex/turn-diffs/")) | not))) + then "update-ref" + else empty end + ' "$trace" | sort -u } # Every `## ` heading outside fenced code, in document order. Fenced headings @@ -770,7 +983,7 @@ protected_sections() { if section_is_mutable "$status" "$plan_round" "$impl_round" "$name"; then continue fi - printf '%s=%s\n' "$name" "$(section_raw "$doc" "$name" | cksum)" + printf '%s=%s\n' "$name" "$(section_raw "$doc" "$name" | digest_stream)" done } @@ -873,9 +1086,8 @@ backfill_question_origin() { 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. +# Publication metadata must exist before --create-pr can take another turn; its +# original remote and branch identity cannot be reconstructed safely later. validate_run_metadata() { local doc="$1" backfill_pr_base_branch "$doc" @@ -883,7 +1095,8 @@ validate_run_metadata() { [[ "$CREATE_PR" -eq 1 ]] || return 0 local key missing="" - for key in base_branch start_commit; do + for key in base_branch start_commit pr_base_tip task_branch github_repo \ + origin_fetch_url origin_push_url git_config_state publication_head; do [[ -n "$(frontmatter "$doc" "$key")" ]] \ || missing="${missing:+${missing}, }${key}" done @@ -895,13 +1108,46 @@ validate_run_metadata() { "with '.agents/workflows/pair.sh start ${number:-} --slug '" } +# A slug owns the exact local Git and worktree state produced by its preceding +# successful turn. This closes the gap between turns, where per-turn snapshots +# alone would accept another task's edits as the next baseline. +validate_continuation_state() { + local doc="$1" expected_git expected_worktree current_git current_worktree + expected_git="$(frontmatter "$doc" expected_git_state)" + expected_worktree="$(frontmatter "$doc" expected_worktree_state)" + [[ -n "$expected_git" && -n "$expected_worktree" ]] \ + || die "${doc#"$REPO_ROOT"/} predates between-turn state ownership; start a new run" + current_git="$(git_state_digest)" + current_worktree="$(worktree_state_digest)" + [[ "$expected_git" == "$current_git" ]] \ + || die "Git state changed between pair-workflow turns; inspect the repository and start a new run" + [[ "$expected_worktree" == "$current_worktree" ]] \ + || die "worktree content changed between pair-workflow turns; another task or process may own it" +} + +# Completion remains true only while the implementation is byte-for-byte the +# reviewed one. Publication checks the same invariant, but ordinary status and +# terminal observations must not keep reporting stale work as complete. +validate_done_changeset() { + local doc="$1" reviewed start current + reviewed="$(frontmatter "$doc" changeset_digest)" + start="$(frontmatter "$doc" start_commit)" + [[ -n "$reviewed" && "$reviewed" != "none" && -n "$start" ]] \ + || die "the finished task has no usable reviewed changeset metadata" + current="$(changeset_digest "$start")" + [[ "$reviewed" == "$current" ]] \ + || die "the worktree changed after the task reached done (${reviewed} -> ${current}); re-run review" +} + # 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 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" +"pr_base_branch pr_base_tip task_branch github_repo origin_fetch_url "\ +"origin_push_url dirty_at_start max_rounds changeset_digest "\ +"reviewed_changeset_digest git_config_state publication_head expected_git_state "\ +"expected_worktree_state 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 @@ -929,12 +1175,12 @@ immutable_snapshot() { # agent could rewrite it, it could rewrite the acceptance criteria and then # satisfy them — exactly the property the second opinion exists to prevent. printf 'issue-headings=%s\n' "$(section_heading_count "$doc" "Issue")" - printf 'issue-section=%s\n' "$(section_raw "$doc" "Issue" | cksum)" + printf 'issue-section=%s\n' "$(section_raw "$doc" "Issue" | digest_stream)" # ## Task is agent1's restatement, immutable after its first turn. Before # that turn it is still template scaffolding, so an empty digest is normal. printf 'task-headings=%s\n' "$(section_heading_count "$doc" "Task")" if [[ "$protect_task" -eq 1 ]]; then - printf 'task-section=%s\n' "$(section_raw "$doc" "Task" | cksum)" + printf 'task-section=%s\n' "$(section_raw "$doc" "Task" | digest_stream)" fi } @@ -992,8 +1238,8 @@ set_frontmatter() { ' "$doc" > "$tmp" && mv "$tmp" "$doc" || { rm -f "$tmp"; die "could not update ${doc}"; } } -# Two drivers on one slug would interleave turns and corrupt the document, and -# the failure would look like an agent misbehaving rather than a collision. +# Every task shares one index and worktree. Even different slugs must therefore +# be serialized, or one task can review and commit another task's source edits. # `mkdir` is the atomic primitive here: it succeeds for exactly one caller. LOCK_DIR="" release_lock() { @@ -1005,6 +1251,7 @@ release_lock() { # that signal instead of letting the driver resume without mutual exclusion. handle_signal() { local signal="$1" + poison_active_turn release_lock trap - "$signal" kill -s "$signal" "$$" @@ -1012,12 +1259,13 @@ handle_signal() { acquire_lock() { [[ -z "$LOCK_DIR" ]] || return 0 # already held by an outer command - local slug="$1" lock dir - dir="$(dirname "$(doc_for "$slug")")" - mkdir -p "$dir" - lock="${dir}/.lock" + local slug="$1" lock + lock="$(git -C "$REPO_ROOT" rev-parse --git-path pair-workflow.lock)" + [[ "$lock" == /* ]] || lock="${REPO_ROOT}/${lock}" + mkdir -p "$(dirname "$lock")" mkdir "$lock" 2>/dev/null \ - || die "another pair.sh is already running for '${slug}' (delete ${lock} if it is stale)" + || die "another pair.sh is already using this worktree while '${slug}' was requested; "\ +"delete ${lock} only if no pair.sh process is running" LOCK_DIR="$lock" trap release_lock EXIT trap 'handle_signal INT' INT @@ -1039,8 +1287,8 @@ issue_number_from() { local path="${remainder#*/}" local want="${path%%/issues/*}" local here here_url here_remainder here_host - here="$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null || true)" - here_url="$(gh repo view --json url --jq .url 2>/dev/null || true)" + here="$(github repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null || true)" + here_url="$(github repo view --json url --jq .url 2>/dev/null || true)" [[ -n "$here" && "$here_url" == *://* ]] \ || { fail "cannot resolve the current GitHub repository; "\ "pass an issue number or fix 'gh repo view'"; return 1; } @@ -1151,6 +1399,7 @@ cmd_start() { [[ -n "$slug" ]] || slug="issue-${number}" acquire_lock "$slug" + ensure_task_directory "$slug" local doc; doc="$(doc_for "$slug")" [[ -e "$doc" ]] && die "${doc} already exists; pass --slug or delete it" @@ -1158,8 +1407,19 @@ cmd_start() { # Fetch once, here, so the document is self-contained. Every later turn is # a cold start and must not depend on the issue still being reachable or # unchanged. + local github_repo github_repo_name github_repo_url github_host + github_repo_name="$(github repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null)" \ + || die "cannot resolve the GitHub repository owned by this checkout" + github_repo_url="$(github repo view --json url --jq .url 2>/dev/null)" \ + || die "cannot resolve the GitHub host owned by this checkout" + github_host="${github_repo_url#*://}" + github_host="${github_host%%/*}" + [[ -n "$github_repo_name" && -n "$github_host" ]] \ + || die "the checkout has no GitHub repository identity" + github_repo="${github_host}/${github_repo_name}" local issue_json - issue_json="$(gh issue view "$number" --json number,title,body,url,state 2>/dev/null)" \ + issue_json="$(github issue view "$number" --repo "$github_repo" \ + --json number,title,body,url,state 2>/dev/null)" \ || die "cannot read issue #${number}; check the number, the repository, "\ "and 'gh auth status'" @@ -1168,6 +1428,8 @@ cmd_start() { url="$(printf '%s' "$issue_json" | jq -r '.url')" state="$(printf '%s' "$issue_json" | jq -r '.state')" body="$(printf '%s' "$issue_json" | jq -r '.body // ""')" + [[ -n "$(pr_title_from "$title")" ]] \ + || die "issue #${number} has no usable pull-request title" [[ "$state" == "OPEN" ]] \ || info "warning: issue #${number} is ${state}; continuing anyway" @@ -1194,13 +1456,16 @@ cmd_start() { # 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 + refresh_remote_branch "$PR_BASE_BRANCH" \ + || die "could not refresh origin/${PR_BASE_BRANCH} before setup" + local base_commit target_ref target_tip 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")" + base_commit="$(git -C "$REPO_ROOT" rev-parse "$base_commit")" + target_tip="$(git -C "$REPO_ROOT" rev-parse "$target_ref")" # 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` @@ -1224,7 +1489,7 @@ cmd_start() { # agent1 may edit it. Recorded either way, because once the run begins the # two are indistinguishable. local dirty=no - [[ -z "$(git -C "$REPO_ROOT" status --porcelain)" ]] || dirty=yes + [[ -z "$(worktree_porcelain)" ]] || dirty=yes if [[ "$dirty" == "yes" && "$ALLOW_DIRTY" -eq 0 ]]; then die "the worktree has uncommitted changes; commit or stash them first, "\ "or pass --allow-dirty to review them alongside the agents' work" @@ -1234,7 +1499,18 @@ cmd_start() { "review scope; publication is disabled for this task, so a later "\ "run --create-pr will be refused" - mkdir -p "$(dirname "$doc")" + local task_branch origin_fetch_url origin_push_url config_state + local expected_git expected_worktree + task_branch="$(branch_name_from "$title" "$number")" + [[ -n "$task_branch" ]] || die "could not derive a safe branch name from issue #${number}" + origin_fetch_url="$(remote_url fetch)" \ + || die "origin must have exactly one fetch URL" + origin_push_url="$(remote_url push)" \ + || die "origin must have exactly one push URL" + config_state="$(git_config_digest)" + expected_git="$(git_state_digest)" + expected_worktree="$(worktree_state_digest)" + # 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 @@ -1248,7 +1524,7 @@ cmd_start() { # The body goes in verbatim from a file rather than through a substitution: # issue text routinely contains backslashes and ampersands, which awk's # gsub would silently reinterpret. - local body_file; body_file="$(dirname "$doc")/.issue-body" + local body_file; body_file="$(mktemp "$(dirname "$doc")/.issue-body.XXXXXX")" printf '%s\n' "$body" > "$body_file" NUMBER="$number" TITLE="$title" ISSUE="$url" ROUNDS="$max_rounds" \ @@ -1256,7 +1532,11 @@ cmd_start() { CLAUDEMODEL="$claude_model" CLAUDEEFFORT="$claude_effort" \ CODEXMODEL="$codex_model" CODEXEFFORT="$codex_effort" \ BASEBRANCH="$base_branch" STARTCOMMIT="$start_commit" \ - PRBASE="$PR_BASE_BRANCH" \ + PRBASE="$PR_BASE_BRANCH" PRBASETIP="$target_tip" TASKBRANCH="$task_branch" \ + GITHUBREPO="$github_repo" ORIGINFETCH="$origin_fetch_url" \ + ORIGINPUSH="$origin_push_url" GITCONFIGSTATE="$config_state" \ + EXPECTEDGIT="$expected_git" \ + EXPECTEDWORKTREE="$expected_worktree" \ awk -v bodyfile="$body_file" ' /ISSUE_BODY/ { # Demote headings from the issue by one level so they nest under @@ -1292,32 +1572,51 @@ cmd_start() { close(bodyfile) next } - # Literal replacement, never gsub: in a gsub replacement string `&` - # expands to the matched text and backslashes are escapes, so an issue - # titled "A & B" would render as "A ISSUE_TITLE B". Titles routinely - # contain both characters. - function put(line, ph, val, at, out) { - while ((at = index(line, ph)) > 0) { - out = out substr(line, 1, at - 1) val - line = substr(line, at + length(ph)) + # Replace placeholders from the original line in one pass. Inserted + # values are never scanned again, so a title that literally contains a + # different placeholder token remains literal. + function put(line, at, best, ph, chosen, out) { + while (1) { + best = 0 + chosen = "" + for (ph in value) { + at = index(line, ph) + if (at > 0 && (best == 0 || at < best)) { + best = at + chosen = ph + } + } + if (best == 0) return out line + out = out substr(line, 1, best - 1) value[chosen] + line = substr(line, best + length(chosen)) } - return out line } - { $0 = put($0, "ISSUE_NUMBER", ENVIRON["NUMBER"]) - $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"]) + BEGIN { + value["ISSUE_NUMBER"] = ENVIRON["NUMBER"] + value["ISSUE_TITLE"] = ENVIRON["TITLE"] + value["ISSUE_URL"] = ENVIRON["ISSUE"] + value["BASE_COMMIT"] = ENVIRON["BASE"] + value["PR_BASE_BRANCH"] = ENVIRON["PRBASE"] + value["PR_BASE_TIP"] = ENVIRON["PRBASETIP"] + value["TASK_BRANCH"] = ENVIRON["TASKBRANCH"] + value["GITHUB_REPO"] = ENVIRON["GITHUBREPO"] + value["ORIGIN_FETCH_URL"] = ENVIRON["ORIGINFETCH"] + value["ORIGIN_PUSH_URL"] = ENVIRON["ORIGINPUSH"] + value["GIT_CONFIG_STATE"] = ENVIRON["GITCONFIGSTATE"] + value["EXPECTED_GIT_STATE"] = ENVIRON["EXPECTEDGIT"] + value["EXPECTED_WORKTREE_STATE"] = ENVIRON["EXPECTEDWORKTREE"] + value["BASE_BRANCH"] = ENVIRON["BASEBRANCH"] + value["START_COMMIT"] = ENVIRON["STARTCOMMIT"] + value["CREATED_AT"] = ENVIRON["NOW"] + value["TASK_SLUG"] = ENVIRON["SLUG"] + value["AGENT1_NAME"] = ENVIRON["A1"] + value["AGENT2_NAME"] = ENVIRON["A2"] + value["CLAUDE_MODEL"] = ENVIRON["CLAUDEMODEL"] + value["CLAUDE_EFFORT"] = ENVIRON["CLAUDEEFFORT"] + value["CODEX_MODEL"] = ENVIRON["CODEXMODEL"] + value["CODEX_EFFORT"] = ENVIRON["CODEXEFFORT"] + } + { $0 = put($0) if ($0 ~ /^max_rounds: /) $0 = "max_rounds: " ENVIRON["ROUNDS"] if ($0 ~ /^dirty_at_start: /) $0 = "dirty_at_start: " ENVIRON["DIRTY"] print } @@ -1337,6 +1636,7 @@ cmd_start() { cmd_status() { local slug; slug="$(resolve_slug "${1:-}")" || exit "$EXIT_ERROR" + validate_task_directory "$slug" local doc; doc="$(doc_for "$slug")" require_doc "$doc" local status turn agent1 agent2 work manual @@ -1346,6 +1646,19 @@ cmd_status() { agent2="$(frontmatter "$doc" agent2)" work="$(status_work "$doc" "$status" "$turn")" manual="$(status_manual_testing "$(frontmatter "$doc" manual_testing)")" + if [[ "$status" == "done" ]]; then + local reviewed start current_digest + reviewed="$(frontmatter "$doc" changeset_digest)" + start="$(frontmatter "$doc" start_commit)" + current_digest="" + if [[ -n "$start" ]] && git -C "$REPO_ROOT" cat-file -e \ + "${start}^{commit}" 2>/dev/null; then + current_digest="$(changeset_digest "$start")" + fi + [[ -n "$reviewed" && "$reviewed" != "none" \ + && "$reviewed" == "$current_digest" ]] \ + || work="completion invalidated — worktree differs from the reviewed state" + fi printf 'status: %s\nagents: agent1=%s, agent2=%s\nwork: %s\n' \ "$status" \ "$(status_agent_name "$agent1")" \ @@ -1727,16 +2040,58 @@ dependency_headings_match() { ' "$file" } -# Lists the complete prospective PR changeset, including commits already made -# by an earlier publication attempt and every current index/worktree change. -changeset_files() { +# Lists the complete prospective PR changeset. Comparing the base commit +# directly with the working tree already includes committed, staged, and +# unstaged tracked changes; untracked paths are the only separate set. NUL +# delimiters preserve every valid Git path and avoid core.quotePath aliases. +changeset_files_z() { local base="$1" - { - git -C "$REPO_ROOT" diff --name-only "$base" HEAD - git -C "$REPO_ROOT" diff --name-only - git -C "$REPO_ROOT" diff --cached --name-only - git -C "$REPO_ROOT" ls-files --others --exclude-standard - } | sort -u + LC_ALL=C git -C "$REPO_ROOT" diff --name-only -z "$base" \ + "${WORKTREE_PATHS[@]}" + LC_ALL=C git -C "$REPO_ROOT" ls-files --others --exclude-standard -z \ + "${WORKTREE_PATHS[@]}" +} + +# Human-readable form used only for emptiness and exact ordinary-path checks. +changeset_files() { + local base="$1" file + changeset_files_z "$base" | while IFS= read -r -d '' file; do + printf '%q\n' "$file" + done +} + +# Emits one canonical manifest entry for the current working path. +worktree_manifest_entry() { + local file="$1" path="${REPO_ROOT}/${file}" mode object target entry metadata + if [[ -L "$path" ]]; then + mode=120000 + target="$(readlink "$path")" + object="$(printf '%s' "$target" | git -C "$REPO_ROOT" hash-object --stdin)" + printf '%s %s\0%s\0' "$mode" "$object" "$file" + elif [[ -f "$path" ]]; then + [[ -x "$path" ]] && mode=100755 || mode=100644 + if [[ "$(git -C "$REPO_ROOT" config --bool core.filemode 2>/dev/null || true)" \ + == "false" ]]; then + entry="" + IFS= read -r -d '' entry \ + < <(git -C "$REPO_ROOT" ls-files -s -z -- "$file") || true + metadata="${entry%%$'\t'*}" + [[ "$metadata" != "$entry" ]] && mode="${metadata%% *}" + fi + object="$(git -C "$REPO_ROOT" hash-object --path="$file" -- "$file")" + printf '%s %s\0%s\0' "$mode" "$object" "$file" + else + printf 'deleted\0%s\0' "$file" + fi +} + +# Turns NUL-delimited metadata/path pairs into an order-independent strong +# digest. Per-entry hashes keep newlines and non-ASCII names opaque to sort. +digest_manifest_entries() { + local metadata file + while IFS= read -r -d '' metadata && IFS= read -r -d '' file; do + { printf '%s\0' "$metadata"; printf '%s' "$file"; } | digest_stream + done | LC_ALL=C sort | digest_stream } # Content, type, and mode digest of the complete prospective changeset. @@ -1744,22 +2099,54 @@ changeset_files() { # 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 ' ' '-' + local base="$1" file + changeset_files_z "$base" | while IFS= read -r -d '' file; do + worktree_manifest_entry "$file" + done | digest_manifest_entries +} + +# The same manifest after staging. Comparing it with changeset_digest catches +# clean filters, hooks, and edits that land after review but before a commit. +index_changeset_digest() { + local base="$1" file entry metadata mode object + git -C "$REPO_ROOT" diff --cached --name-only -z "$base" \ + "${WORKTREE_PATHS[@]}" \ + | while IFS= read -r -d '' file; do + entry="" + IFS= read -r -d '' entry \ + < <(git -C "$REPO_ROOT" ls-files -s -z -- "$file") || true + if [[ -z "$entry" ]]; then + printf 'deleted\0%s\0' "$file" + continue + fi + metadata="${entry%%$'\t'*}" + mode="${metadata%% *}" + metadata="${metadata#* }" + object="${metadata%% *}" + printf '%s %s\0%s\0' "$mode" "$object" "$file" + done | digest_manifest_entries +} + +# The reviewed manifest as committed, used immediately before any push. +commit_changeset_digest() { + local base="$1" commit="$2" file entry metadata mode object + git -C "$REPO_ROOT" diff --name-only -z "$base" "$commit" \ + "${WORKTREE_PATHS[@]}" \ + | while IFS= read -r -d '' file; do + entry="" + IFS= read -r -d '' entry \ + < <(git -C "$REPO_ROOT" ls-tree -z "$commit" -- "$file") || true + if [[ -z "$entry" ]]; then + printf 'deleted\0%s\0' "$file" + continue + fi + metadata="${entry%%$'\t'*}" + mode="${metadata%% *}" + metadata="${metadata#* }" + metadata="${metadata#* }" + object="${metadata%% *}" + printf '%s %s\0%s\0' "$mode" "$object" "$file" + done | digest_manifest_entries } # Fingerprint of driver-owned per-round snapshots. They live in the writable, @@ -1773,7 +2160,22 @@ rounds_state() { if [[ -L "$file" ]]; then printf '%s=symlink:%s\n' "${file##*/}" "$(readlink "$file")" elif [[ -f "$file" ]]; then - printf '%s=file:%s\n' "${file##*/}" "$(cksum < "$file")" + printf '%s=file:%s\n' "${file##*/}" "$(digest_stream < "$file")" + fi + done +} + +# Fingerprint of completed audit artifacts. The current log and trace are open +# for this turn and therefore excluded; every earlier artifact is immutable. +turns_state() { + local dir="$1" current_log="$2" current_trace="$3" file + [[ -d "$dir" && ! -L "$dir" ]] || return 0 + for file in "$dir"/*; do + [[ "$file" == "$current_log" || "$file" == "$current_trace" ]] && continue + if [[ -L "$file" ]]; then + printf '%s=symlink:%s\n' "${file##*/}" "$(readlink "$file")" + elif [[ -f "$file" ]]; then + printf '%s=file:%s\n' "${file##*/}" "$(digest_stream < "$file")" fi done } @@ -1788,10 +2190,10 @@ changeset_patch() { # 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" -- + git -C "$REPO_ROOT" diff "$base" "${WORKTREE_PATHS[@]}" ( cd "$REPO_ROOT" || exit 1 - git ls-files --others --exclude-standard -z \ + git ls-files --others --exclude-standard -z "${WORKTREE_PATHS[@]}" \ | while IFS= read -r -d '' file; do git diff --no-index --binary -- /dev/null "$file" || true done @@ -1801,11 +2203,21 @@ changeset_patch() { # Branch name from the issue title: kebab-case, no agent identifiers, per the # branch-naming rule in AGENTS.md. branch_name_from() { - printf '%s' "$1" \ + local title="$1" number="${2:-}" + local branch + branch="$(printf '%s' "$title" \ | tr '[:upper:]' '[:lower:]' \ | sed -e 's/[^a-z0-9]\{1,\}/-/g' -e 's/^-//' -e 's/-$//' \ | cut -c1-50 \ - | sed -e 's/-$//' + | sed -e 's/-$//')" + [[ -n "$branch" ]] || branch="issue-${number}" + printf '%s' "$branch" +} + +# GitHub policy forbids a trailing period; remove surrounding trailing space and +# every terminal period once, both at setup validation and final publication. +pr_title_from() { + printf '%s' "$1" | sed -e 's/[[:space:]]*$//' -e 's/[.]*$//' } # Branches, commits, pushes, and opens a draft PR. Runs only after the workflow @@ -1813,7 +2225,7 @@ branch_name_from() { # user aborts instead of guessing. create_pr() { local doc="$1" - local number title branch body + local number title branch body github_repo origin_fetch origin_push 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)" @@ -1821,7 +2233,8 @@ create_pr() { # would derive a different branch name and strand the branch already # created, and the protocol defines the issue copy as a fixed snapshot. title="$(frontmatter "$doc" issue_title)" - [[ -n "$title" ]] || die "issue_title is missing from the document; it is written at setup" + [[ -n "$title" ]] \ + || die "issue_title is missing from the document; it is written at setup" # Publishing asserts the work is ready for review. An undecided manual # testing field, or `required` with no usable plan, means nobody can say @@ -1848,8 +2261,18 @@ create_pr() { || die "the worktree was already dirty when this run started; "\ "commit or stash your own changes and open the PR yourself" - branch="$(branch_name_from "$title")" - [[ -n "$branch" ]] || die "could not derive a branch name from the issue title" + branch="$(frontmatter "$doc" task_branch)" + [[ -n "$branch" ]] || die "task_branch is missing from the workflow metadata" + github_repo="$(frontmatter "$doc" github_repo)" + [[ -n "$github_repo" ]] || die "github_repo is missing from the workflow metadata" + origin_fetch="$(remote_url fetch)" || die "origin must have exactly one fetch URL" + origin_push="$(remote_url push)" || die "origin must have exactly one push URL" + [[ "$origin_fetch" == "$(frontmatter "$doc" origin_fetch_url)" ]] \ + || die "origin's fetch URL changed during the run; nothing was published" + [[ "$origin_push" == "$(frontmatter "$doc" origin_push_url)" ]] \ + || die "origin's push URL changed during the run; nothing was published" + [[ "$(git_config_digest)" == "$(frontmatter "$doc" git_config_state)" ]] \ + || die "effective Git configuration changed during the run; nothing was published" local base start target_branch changes task_changes dirty base="$(frontmatter "$doc" base_commit)" @@ -1859,39 +2282,104 @@ create_pr() { 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)" + refresh_remote_branch "$target_branch" \ + || die "could not refresh origin/${target_branch} before publication" 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 + local target_base recorded_target_tip current_target_tip + recorded_target_tip="$(frontmatter "$doc" pr_base_tip)" + current_target_tip="$(git -C "$REPO_ROOT" rev-parse "$base_ref")" + git -C "$REPO_ROOT" merge-base --is-ancestor \ + "$recorded_target_tip" "$current_target_tip" \ + || die "origin/${target_branch} was rewritten after setup; start a new workflow run" 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" + target_base="$(git -C "$REPO_ROOT" rev-parse "$target_base")" + if [[ "$(git -C "$REPO_ROOT" rev-parse "$base")" \ + != "$(git -C "$REPO_ROOT" rev-parse "$target_base")" ]]; then + # The baseline is recomputed rather than enforced, because the ordinary + # reason it moves is not this run's doing: someone else's pull request + # merges into the target while this one works, and the merge-base + # advances to absorb commits the run was stacked on. + # + # Nothing the run produced depends on it. The task's own changeset is + # measured from `start_commit`, which is immutable and separately + # checked below, and the reviewed digest is taken from there too. What + # shrinks is only the earlier work riding along in the pull request, + # and the stacking note is derived from `${base_ref}` at this moment, + # so the body already states the smaller truth. Refusing here would + # discard a finished, approved run over a merge nobody in it made. + # + # A merge-base that moved forward but no farther than `start_commit` is + # what that looks like. Moving outside that closed interval means the + # target was rewritten or absorbed commits produced by this run. In + # either case, shrinking the PR baseline would silently remove reviewed + # task changes from the published changeset. + git -C "$REPO_ROOT" merge-base --is-ancestor "$base" "$target_base" \ + || die "the recorded PR base ${base} is not an ancestor of the "\ +"current merge-base ${target_base} with origin/${target_branch}, so that "\ +"branch was rewritten rather than advanced during the run; start a new "\ +"workflow run so the task and PR scopes are recalculated" + git -C "$REPO_ROOT" merge-base --is-ancestor "$target_base" "$start" \ + || die "the current merge-base ${target_base} with "\ +"origin/${target_branch} advanced beyond the recorded start_commit ${start} "\ +"and absorbed commits produced by this run; start a new workflow run so the "\ +"task and PR scopes are recalculated" + info "origin/${target_branch} advanced during the run; the pull "\ +"request baseline moves from ${base} to ${target_base}, which changes how "\ +"many earlier commits ride along and nothing this run produced" + base="$target_base" + fi # 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 + local current current_head publication_head current="$(git -C "$REPO_ROOT" rev-parse --abbrev-ref HEAD)" current_head="$(git -C "$REPO_ROOT" rev-parse HEAD)" + publication_head="$(frontmatter "$doc" publication_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" + if [[ -n "$publication_head" && "$publication_head" != "none" ]]; then + [[ "$current_head" == "$publication_head" ]] \ + || die "task branch '${branch}' moved beyond the driver's recorded publication head" + else + [[ "$current_head" == "$(git -C "$REPO_ROOT" rev-parse "$start")" ]] \ + || die "task branch '${branch}' has commits the driver did not create" + fi else + [[ -z "$publication_head" || "$publication_head" == "none" ]] \ + || die "return to task branch '${branch}' before retrying publication" [[ "$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 + # Fail a remote branch collision before creating local publication commits. + # An existing head is safe only when it is already in the immutable starting + # history and the eventual push can therefore be a fast-forward. + local initial_remote_head + initial_remote_head="$(publication_git ls-remote --heads origin \ + "refs/heads/${branch}" | awk 'NR == 1 { print $1 }')" \ + || die "could not inspect origin/${branch} before publication" + if [[ ( -z "$publication_head" || "$publication_head" == "none" ) \ + && -n "$initial_remote_head" ]]; then + git -C "$REPO_ROOT" cat-file -e "${initial_remote_head}^{commit}" 2>/dev/null \ + && git -C "$REPO_ROOT" merge-base --is-ancestor \ + "$initial_remote_head" "$start" \ + || die "origin/${branch} already contains history outside this task's starting point" + 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)" + dirty="$(worktree_porcelain)" if [[ -z "$changes" ]]; then info "no changes since ${base}; skipping the pull request" return 0 @@ -2003,7 +2491,7 @@ create_pr() { || 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 \ + if ! checkout_error="$(publication_git checkout -b \ "$branch" "$start" 2>&1)"; then die "could not create branch '${branch}': ${checkout_error}" fi @@ -2028,37 +2516,92 @@ create_pr() { # this dedicated commit. if ! git -C "$REPO_ROOT" diff --quiet HEAD -- \ version.gradle.kts pom.xml dependencies.md; then - git -C "$REPO_ROOT" commit -q --only \ + publication_git commit -q --only \ -m "Bump version —> \`${new_v}\`." -- \ version.gradle.kts pom.xml dependencies.md \ || die "the version commit failed" info "committed the version bump and regenerated reports" + set_frontmatter "$doc" \ + "publication_head=$(git -C "$REPO_ROOT" rev-parse HEAD)" else info "version bump and reports are already committed" fi - git -C "$REPO_ROOT" add -A || die "git add failed" + current_digest="$(changeset_digest "$start")" + [[ "$reviewed" == "$current_digest" ]] \ + || die "the changeset moved during publication before staging; nothing was pushed" + # Let Git apply the repository's ignore rules during the broad add. + # Passing an explicit excluded pathspec for the ignored work root makes + # some Git versions reject the command as an attempt to add an ignored + # path. Resetting that exact driver-owned path afterwards also covers a + # reviewed change that deliberately removes its ignore rule. + publication_git add -A -- . || die "git add failed" + publication_git reset -q HEAD -- .agents/work \ + || die "could not exclude pair-workflow scratch files from staging" + local staged_digest + staged_digest="$(index_changeset_digest "$start")" + [[ "$reviewed" == "$staged_digest" ]] \ + || die "the staged tree differs from the reviewed changeset; nothing was pushed" if ! git -C "$REPO_ROOT" diff --cached --quiet; then - git -C "$REPO_ROOT" commit -q -m "${title}" \ + publication_git commit -q -m "${title}" \ || die "the task commit failed" + set_frontmatter "$doc" \ + "publication_head=$(git -C "$REPO_ROOT" rev-parse HEAD)" info "committed onto '${branch}'" fi else info "nothing left to commit; continuing with what is already committed" fi - if ! git -C "$REPO_ROOT" rev-parse --abbrev-ref '@{upstream}' >/dev/null 2>&1 \ - || [[ -n "$(git -C "$REPO_ROOT" log '@{upstream}..HEAD' --oneline)" ]]; then - git -C "$REPO_ROOT" push -q -u origin "$branch" \ + [[ -z "$(worktree_porcelain)" ]] \ + || die "the worktree changed during publication; local commits were not pushed" + current_head="$(git -C "$REPO_ROOT" rev-parse HEAD)" + publication_head="$(frontmatter "$doc" publication_head)" + [[ "$publication_head" == "$current_head" ]] \ + || die "the current task-branch head was not created by this publication run" + local committed_digest + committed_digest="$(commit_changeset_digest "$start" "$current_head")" + [[ "$reviewed" == "$committed_digest" ]] \ + || die "the committed tree differs from the reviewed changeset; nothing was pushed" + [[ "$(git_config_digest)" == "$(frontmatter "$doc" git_config_state)" ]] \ + || die "Git configuration changed during publication; nothing was pushed" + + local remote_head + remote_head="$(publication_git ls-remote --heads origin \ + "refs/heads/${branch}" | awk 'NR == 1 { print $1 }')" \ + || die "could not inspect origin/${branch}; nothing was pushed" + if [[ "$remote_head" != "$current_head" ]]; then + if [[ -n "$remote_head" ]]; then + publication_git fetch --no-tags -q origin \ + "+refs/heads/${branch}:refs/remotes/origin/${branch}" \ + || die "could not inspect the existing origin/${branch}" + git -C "$REPO_ROOT" merge-base --is-ancestor "$remote_head" "$current_head" \ + || die "origin/${branch} contains commits outside this "\ +"reviewed run; refusing to overwrite it" + fi + publication_git push -q origin \ + "refs/heads/${branch}:refs/heads/${branch}" \ || die "git push failed; the commits are local and nothing was published" info "pushed '${branch}'" else info "'${branch}' is already pushed" fi - local existing - existing="$(gh pr view --json url --jq .url 2>/dev/null || true)" + remote_head="$(publication_git ls-remote --heads origin \ + "refs/heads/${branch}" | awk 'NR == 1 { print $1 }')" \ + || die "could not verify origin/${branch} after push" + [[ "$remote_head" == "$current_head" ]] \ + || die "origin/${branch} does not match the reviewed local commit; no PR was opened" + + local existing existing_url existing_base existing_head + existing="$(github pr view "$branch" --repo "$github_repo" \ + --json url,state,baseRefName,headRefName \ + --jq 'select(.state == "OPEN") | [.url, .baseRefName, .headRefName] | @tsv' \ + 2>/dev/null || true)" if [[ -n "$existing" ]]; then - info "pull request already open: ${existing}" + IFS=$'\t' read -r existing_url existing_base existing_head <<< "$existing" + [[ "$existing_base" == "$target_branch" && "$existing_head" == "$branch" ]] \ + || die "an open PR already uses '${branch}' with a different head or base" + info "pull request already open: ${existing_url}" return 0 fi @@ -2080,9 +2623,11 @@ Review the task commits after \`${start_short}\`." # 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 "$target_branch" \ - --title "${title%.}" --body "$body" 2>&1)" \ + local url pr_title + pr_title="$(pr_title_from "$title")" + url="$(github pr create --repo "$github_repo" --draft --assignee @me \ + --base "$target_branch" --head "$branch" \ + --title "$pr_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}" } @@ -2290,9 +2835,9 @@ Do not change Git state. No branch creation or switching, no \`git add\`, no commit, no push, no tag, no rebase/merge/cherry-pick/reset/stash, and no pull request. Read-only Git (\`status\`, \`diff\`, \`log\`, \`show\`) is expected and fine. Leave your work as uncommitted changes in the worktree — that is the -deliverable, and this prompt is not authorization to commit it. Refs and the -index are compared before and after your turn, and the run aborts if they -moved. +deliverable, and this prompt is not authorization to commit it. Local refs and +the index are compared before and after your turn, and ref-writing Git commands +are traced. The run aborts on either violation. If you are agent1 and a question must be answered before you can plan honestly, ask it: write it under ## Questions, set \`resume_status\` to '${status}', set @@ -2376,6 +2921,12 @@ take_turn() { turn="$(frontmatter "$doc" turn)" rel_doc="${doc#"$REPO_ROOT"/}" + if [[ "$status" == "done" ]]; then + validate_done_changeset "$doc" + else + validate_continuation_state "$doc" + fi + # Answering is the whole action: writing the answers into the document is # enough, and the next run picks them up. There is no separate command to # remember. --accept-defaults resumes the same way without answers. @@ -2493,15 +3044,20 @@ take_turn() { # a final agent1 turn may finish only while the worktree still matches it. local rounds_dir previous="" review_base rounds_dir="$(dirname "$doc")/rounds" + [[ ! -L "$rounds_dir" ]] || die "${rounds_dir} must not be a symlink" mkdir -p "$rounds_dir" review_base="$(frontmatter "$doc" start_commit)" [[ -n "$review_base" ]] || review_base=HEAD case "$status" in plan-review-requested) + [[ ! -L "${rounds_dir}/plan-${plan_round}.md" ]] \ + || die "the plan snapshot destination is a symlink" section "$doc" "Plan" > "${rounds_dir}/plan-${plan_round}.md" [[ "$plan_round" -le 1 ]] \ || previous="${rounds_dir}/plan-$(( plan_round - 1 )).md" ;; implementation-review-requested) + [[ ! -L "${rounds_dir}/impl-${impl_round}.patch" ]] \ + || die "the implementation snapshot destination is a symlink" changeset_patch "$review_base" \ > "${rounds_dir}/impl-${impl_round}.patch" set_frontmatter "$doc" \ @@ -2513,27 +3069,31 @@ take_turn() { previous="${previous#"$REPO_ROOT"/}" local before after git_before git_after immutable_before protect_task=1 + local worktree_after_turn + local external_before external_after traced_writes # Task is established during plan-requested, including after a question # round-trip. Its heading remains unique then, but its contents become # immutable only after that planning turn advances. [[ "$status" == "plan-requested" ]] && protect_task=0 - before="$(cksum < "$doc")" + before="$(digest_stream < "$doc")" git_before="$(git_state)" + external_before="$(external_ref_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)" + [[ "$turn" != "agent2" ]] || worktree_before="$(worktree_state_digest)" # 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" + [[ ! -L "$turns_dir" ]] || die "${turns_dir} must not be a symlink" mkdir -p "$turns_dir" local n=0 candidate base sequence - for candidate in "$turns_dir"/*.log; do + for candidate in "$turns_dir"/*.log "$turns_dir"/*.git-trace.json; do [[ -e "$candidate" ]] || continue base="${candidate##*/}" sequence="${base%%-*}" @@ -2544,33 +3104,35 @@ take_turn() { done n=$(( n + 1 )) local log; log="$(printf '%s/%02d-%s.log' "$turns_dir" "$n" "$turn")" + local git_trace="${log%.log}.git-trace.json" + ( set -o noclobber; : > "$log"; : > "$git_trace" ) 2>/dev/null \ + || die "turn ${n} transcript or Git trace already exists; refusing to overwrite it" + local trace_marker="${slug}-${n}-$$-$(date +%s)" + local turns_before; turns_before="$(turns_state "$turns_dir" "$log" "$git_trace")" # 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 rc tee_rc local -a command_parts read -r -a command_parts <<< "$cmd" + ACTIVE_TURN_DOC="$doc" set +e - (cd "$REPO_ROOT" && "${command_parts[@]}" \ - "$(prompt_for "$turn" "$status" "$rel_doc" "$previous")") 2>&1 | tee "$log" - rc=${PIPESTATUS[0]} + ( + cd "$REPO_ROOT" + export GIT_TRACE2_EVENT="$git_trace" + git -c "pair.traceMarker=${trace_marker}" version >/dev/null + "${command_parts[@]}" \ + "$(prompt_for "$turn" "$status" "$rel_doc" "$previous")" + ) 2>&1 | tee "$log" + local -a pipeline_status=("${PIPESTATUS[@]}") + rc=${pipeline_status[0]} + tee_rc=${pipeline_status[1]} # Restore strict handling for every guard below. An intentional internal # non-zero result must disable it immediately before returning to the # caller, which maps that result to a public exit code. set -e - # The transcript is named because the CLI's own error is the diagnosis and - # it goes nowhere else. The two hints cover what actually fails first: a - # CLI that is on PATH but not signed in, and a model identifier that only - # the API rejects. Both surface as a turn that dies immediately having - # written nothing. - [[ "$rc" -eq 0 ]] \ - || die "${turn} exited ${rc}; document left at status '${status}', "\ -"transcript in ${log#"$REPO_ROOT"/}. A turn that fails at once usually means "\ -"the CLI is not authenticated or its model identifier was rejected; the "\ -"transcript says which" - # Checked before the document, because a Git write is the more serious # violation even on a turn that otherwise did its job. # @@ -2580,6 +3142,7 @@ take_turn() { # change and leaves the attribution to whoever reads it. Naming the agent # here would accuse it of a violation the driver has no evidence for. git_after="$(git_state)" + worktree_after_turn="$(worktree_state_digest)" if [[ "$git_before" != "$git_after" ]]; then info "Git state changed during ${turn}'s turn (- before, + after):" diff <(printf '%s\n' "$git_before") <(printf '%s\n' "$git_after") >&2 || true @@ -2589,19 +3152,42 @@ take_turn() { "the run can simply be started again" fi + # The Trace2 marker proves that the sidecar covers the agent process from + # immediately before invocation. Inspect every turn, not only turns whose + # final snapshots differ: a push of an existing tag, or a fetch with no new + # objects, is still forbidden even though it leaves no local movement. + git_trace_is_usable "$git_trace" "$trace_marker" \ + || die "${turn}'s Git trace is missing or unreadable; inspect the "\ +"transcript and repository before continuing" + traced_writes="$(git_trace_ref_writes "$git_trace")" \ + || die "could not inspect ${turn}'s Git trace" + if [[ -n "$traced_writes" ]]; then + info "${turn}'s process ran Git commands capable of changing refs:" + printf '%s\n' "$traced_writes" >&2 + die "the agent process ran a ref-writing Git command, which this "\ +"workflow forbids; inspect the repository and remote before continuing" + fi + + # No ref-writing command appeared in the agent process trace, so any + # remote-tracking ref or tag movement between these snapshots came from an + # editor, background Git process, or GitHub update outside that process. + external_after="$(external_ref_state)" + if [[ "$external_before" != "$external_after" ]]; then + info "remote-tracking refs or tags moved outside ${turn}'s process; "\ +"background Git or GitHub activity may continue without discarding the run:" + diff <(printf '%s\n' "$external_before") \ + <(printf '%s\n' "$external_after") >&2 || true + 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" + if [[ "$worktree_before" != "$worktree_after_turn" ]]; then + die "worktree content changed during ${turn}'s review "\ +"(${worktree_before} -> ${worktree_after_turn}); inspect the worktree before continuing" fi fi @@ -2613,7 +3199,22 @@ take_turn() { die "${turn} rewrote a driver-owned review snapshot; inspect ${rounds_dir}" fi - after="$(cksum < "$doc")" + [[ -f "$log" && ! -L "$log" && -f "$git_trace" && ! -L "$git_trace" ]] \ + || die "the current transcript or Git trace was replaced during ${turn}'s turn" + local turns_after; turns_after="$(turns_state "$turns_dir" "$log" "$git_trace")" + [[ "$turns_before" == "$turns_after" ]] \ + || die "${turn} rewrote an earlier transcript or Git trace; inspect ${turns_dir}" + + # Safety checks run even when the agent or tee failed. Otherwise a reviewer + # could edit source or an agent could move Git state, exit nonzero, and have + # that state accepted as the baseline on the retry. + [[ "$tee_rc" -eq 0 ]] \ + || die "could not write the complete transcript to ${log#"$REPO_ROOT"/}" + [[ "$rc" -eq 0 ]] \ + || die "${turn} exited ${rc}; the failed turn was invalidated. Transcript: "\ +"${log#"$REPO_ROOT"/}" + + after="$(digest_stream < "$doc")" [[ "$before" != "$after" ]] \ || die "${turn} did not modify ${rel_doc}; aborting instead of looping" @@ -2719,6 +3320,13 @@ questions-pending|human|0|0" "resume_status=none" "updated=${blocked_at}" printf '\n%s driver %s -> blocked: review ceiling %s reached; human decision required\n' \ "$blocked_at" "$status" "$max_rounds" >> "$doc" + [[ "$git_after" == "$(git_state)" \ + && "$worktree_after_turn" == "$(worktree_state_digest)" ]] \ + || die "repository state changed while the driver applied the review ceiling" + set_frontmatter "$doc" \ + "expected_git_state=$(git_state_digest)" \ + "expected_worktree_state=$(worktree_state_digest)" + ACTIVE_TURN_DOC="" info "${turn} requested another review beyond max_rounds=${max_rounds}; "\ "task is blocked for a human decision" set +e @@ -2769,6 +3377,18 @@ questions-pending|human|0|0" set_frontmatter "$doc" "changeset_digest=${reviewed_digest}" fi + [[ "$git_after" == "$(git_state)" ]] \ + || die "Git state changed while the driver validated ${turn}'s completed turn" + [[ "$worktree_after_turn" == "$(worktree_state_digest)" ]] \ + || die "worktree content changed while the driver validated ${turn}'s completed turn" + + # Persist the state this successful turn owns before allowing another slug + # or process to become the next baseline. + set_frontmatter "$doc" \ + "expected_git_state=$(git_state_digest)" \ + "expected_worktree_state=$(worktree_state_digest)" + ACTIVE_TURN_DOC="" + info "advanced: ${status} -> ${new_status}" return 0 } @@ -2903,6 +3523,7 @@ cmd_step() { require_doc "$(doc_for "$slug")" acquire_lock "$slug" + ensure_task_directory "$slug" validate_run_metadata "$(doc_for "$slug")" validate_agent_selection "$(doc_for "$slug")" prepare_saved_engine_settings "$(doc_for "$slug")" @@ -2979,6 +3600,7 @@ cmd_run() { # The lock comes before the existence check, not after: otherwise two runs # for one issue can both see no document and both create it. acquire_lock "$slug" + ensure_task_directory "$slug" local doc; doc="$(doc_for "$slug")" if [[ ! -f "$doc" ]]; then @@ -3037,6 +3659,7 @@ cmd_run() { } main() { + validate_repository_environment [[ -f "$SKILL" && -f "$TEMPLATE" ]] || die "run this script from within the repository" case "${1:-}" in start) shift; cmd_start "$@" ;; diff --git a/PAIR_AGENTS_RUN_GUIDE.md b/PAIR_AGENTS_RUN_GUIDE.md index 2a18183a..8dd5e412 100644 --- a/PAIR_AGENTS_RUN_GUIDE.md +++ b/PAIR_AGENTS_RUN_GUIDE.md @@ -168,6 +168,12 @@ 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. +The driver pins the GitHub repository, the single fetch and push URLs, the task +branch, and the current target tip at setup. It refreshes the target again +before publication, disables local hooks and automatic tag following for its +own Git writes, and verifies that the staged, committed, and remote branch tips +all still represent the reviewed changeset before it opens the PR. + 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 @@ -209,9 +215,25 @@ instructions. The driver checks that: - completed document sections and log history remain unchanged; and - every review has a verdict and every finding has one valid disposition. -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. +Only one pair driver may use a worktree at a time, even for different issue +slugs. Each successful turn also records strong Git and worktree digests. A +later turn refuses to start if another task, editor, or process changed that +shared state between handoffs. A failed agent turn is invalidated after all +safety checks run; rerunning cannot silently adopt its partial state. + +The Git check is a tripwire, not a sandbox: it detects changes after a turn. +Local branches, `HEAD`, other local refs, and the index are compared strictly, +so do not modify those in another window while a run is active. + +Remote-tracking refs and tags are handled separately because a background +fetch or GitHub update may create, advance, or prune them. Each agent turn has +a fresh, turn-specific Git Trace2 sidecar covering its process tree. The driver +correlates each command name with its own process arguments and stops for a +command capable of changing refs even when no final ref movement is visible. If refs +move without such a command, it reports the external movement and continues. +Missing or unreadable provenance stops the run rather than guessing. This +distinguishes processes directly instead of inferring a push or fetch from +commit reachability. `--cp` changes only the driver's final publication step; it does not loosen agent permissions or review checks. @@ -309,25 +331,35 @@ For a repository whose pull requests target another branch, set 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: +At setup, the driver fetches the exact PR target without tags, requires it to +have a merge-base with `HEAD`, and records its tip and repository identity. +Before its first publication write, it refreshes that target and 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; +- an `origin/` merge-base that still contains the recorded PR + baseline, whose tip descends from the setup tip, and that has not advanced + beyond the recorded task start; - `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; +- the effective Git configuration and origin fetch/push URLs unchanged; - 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. +A target branch that advances during the run — another pull request merging +into `master`, say — does not stop publication while its new merge-base remains +between the recorded PR baseline and task start. The driver reports the move +and opens the pull request; only inherited commits have left its changeset. A +target rewrite that no longer contains the exact target tip recorded at setup +stops publication, even when the rewritten history retains the same merge-base. +So does a merge-base beyond the task start, because the target has absorbed +commits produced by the run and advancing the baseline would omit reviewed task +work. Either case requires a new run against the new target. ### Override Complete Agent Commands @@ -385,20 +417,18 @@ model internally instead. ### Working Files and Legacy Runs Everything for a task lives in `.agents/work/issue-150/` (gitignored): -`plan.md` is the shared document, `turns/*.log` contains turn transcripts, and +`plan.md` is the shared document, `turns/*.log` contains turn transcripts, +matching `turns/*.git-trace.json` files contain per-turn Git events, 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`. +what an agent chose to write down; the transcripts and traces are what its +process 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 +backfilled as `master`, and recoverable question provenance is backfilled from +the saved legacy resume status. Runs that predate between-turn worktree +ownership or the pinned repository metadata cannot reconstruct those values +safely. Start a replacement with `.agents/workflows/pair.sh start --slug `. ### Protocol Reference diff --git a/dependencies.md b/dependencies.md index 56e423be..f9c48f96 100644 --- a/dependencies.md +++ b/dependencies.md @@ -1,6 +1,6 @@ -# Dependencies of `io.spine.chords:spine-chords-client:2.0.0-SNAPSHOT.108` +# Dependencies of `io.spine.chords:spine-chords-client:2.0.0-SNAPSHOT.109` ## 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 **Thu Aug 06 12:48:28 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 **Fri Aug 07 21:18: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.108` +# Dependencies of `io.spine.chords:spine-chords-codegen-tests:2.0.0-SNAPSHOT.109` ## Runtime 1. **Group** : com.google.code.findbugs. **Name** : jsr305. **Version** : 3.0.2. @@ -1899,12 +1899,12 @@ This report was generated on **Thu Aug 06 12:48:28 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Thu Aug 06 12:48: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). +This report was generated on **Fri Aug 07 21:18: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.108` +# Dependencies of `io.spine.chords:spine-chords-core:2.0.0-SNAPSHOT.109` ## Runtime 1. **Group** : cafe.adriel.voyager. **Name** : voyager-core. **Version** : 1.0.1. @@ -2938,12 +2938,12 @@ This report was generated on **Thu Aug 06 12:48:30 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Thu Aug 06 12:48: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). +This report was generated on **Fri Aug 07 21:18:28 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.108` +# Dependencies of `io.spine.chords:spine-chords-proto:2.0.0-SNAPSHOT.109` ## 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 **Thu Aug 06 12:48:31 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Thu Aug 06 12:48:32 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 **Fri Aug 07 21:18: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.108` +# Dependencies of `io.spine.chords:spine-chords-proto-values:2.0.0-SNAPSHOT.109` ## Runtime 1. **Group** : com.google.code.findbugs. **Name** : jsr305. **Version** : 3.0.2. @@ -4775,12 +4775,12 @@ This report was generated on **Thu Aug 06 12:48:32 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Thu Aug 06 12:48:33 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 **Fri Aug 07 21:18: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.108` +# Dependencies of `io.spine.chords:spine-chords-runtime:2.0.0-SNAPSHOT.109` ## Runtime 1. **Group** : com.google.code.findbugs. **Name** : jsr305. **Version** : 3.0.2. @@ -5544,4 +5544,4 @@ This report was generated on **Thu Aug 06 12:48:33 EEST 2026** using [Gradle-Lic The dependencies distributed under several licenses, are used according their commercial-use-friendly license. -This report was generated on **Thu Aug 06 12:48:34 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 **Fri Aug 07 21:18: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 14beeb69..695babe5 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.108 +2.0.0-SNAPSHOT.109 2015 diff --git a/version.gradle.kts b/version.gradle.kts index cb68b3b6..d1bf6c3e 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.108") +val chordsVersion: String by extra("2.0.0-SNAPSHOT.109")