Skip to content

feat(agents): let read-only CR agents use git worktrees for parallel review#728

Merged
pekral merged 2 commits into
masterfrom
feat/cr-agents-parallel-review-worktrees
Jun 29, 2026
Merged

feat(agents): let read-only CR agents use git worktrees for parallel review#728
pekral merged 2 commits into
masterfrom
feat/cr-agents-parallel-review-worktrees

Conversation

@pekral

@pekral pekral commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

Refines the agent worktree policy so the read-only code-review agents (argos, athena) may opt into an isolated git worktree to run their parallel CR pass when isolation is genuinely needed, and makes daidalos clean every CR worktree up after the run / merge so the repository and codebase stay clean.

This is a deliberate, scoped relaxation of the earlier "git worktrees are forbidden across the whole workflow" rule — not a blanket reversal:

  • Writing path unchanged. talos (and every writing daidalos run) still never uses a worktree and still serialises on the single shared working tree via the write-lock. There is no isolated-worktree escape for the writing path.
  • CR pass gains the opt-in. Because argos / athena are read-only, an isolated checkout lets the two parallel CR passes review a stable diff without contending with the writing path or each other. This is exactly the explicit-request opt-in of @rules/git/general.mdc Worktrees / Workspaces, granted to the CR pass only.
  • Daidalos owns cleanup. Each CR agent records any worktree path in its handoff; daidalos removes them in step 7 (git worktree remove + git worktree prune, never --force, skipping the active/dirty tree), and after a merge confirms @skills/merge-github-pr/SKILL.md left no orphaned branch / worktree / temp file behind. A CR agent run standalone removes its own worktree.

Changes

  • agents/daidalos.mdConcurrency & the working-tree write-lock section retitled and rewritten (writing path stays on the shared tree, read-only CR may isolate); step 6 instructs the CR dispatch prompts to grant the worktree opt-in; step 7 gains explicit worktree + post-merge cleanup.
  • agents/argos.md, agents/athena.md — new Parallel review worktree (optional) section + a Worktree: handoff field.
  • docs/agents.md — concurrency narrative updated to match (writing path never uses worktrees; CR pass may isolate; daidalos cleans up).
  • tests/Installer/AgentsTest.php — worktree pinning test rewritten to the new policy, plus a new test asserting both CR agents document the opt-in worktree and hand it back for cleanup.
  • CHANGELOG.md — Unreleased entry.

Testing

  • composer build — green (install + fix + full check: skill-check, normalize, phpcs, pint, rector, phpstan, security-audit, test:coverage). 324 tests / 1856 assertions pass, 100% coverage.
  • vendor/bin/pest tests/Installer/AgentsTest.php — 29 passed.

Notes

  • No tracker issue — this is a free-form request implemented directly.
  • Change is confined to agent prose, docs, the pinning test, and the changelog — no production PHP and no security surface, so the inline security review is a no-op.

…review

argos and athena may now opt into an isolated read-only git worktree to
run their parallel CR pass when isolation is needed; daidalos cleans every
CR worktree up after the run/merge so the repository stays clean.

The writing path (talos) still never uses worktrees and concurrent writers
still serialise on the single shared working tree via the write-lock — only
the read-only CR pass gains the explicit-request opt-in of
@rules/git/general.mdc Worktrees / Workspaces, which carries no write-lock
and never contends with the writing path.

Documented in agents/daidalos.md, agents/argos.md, agents/athena.md
Parallel review worktree, mirrored in docs/agents.md, with the worktree
pinning test in tests/Installer/AgentsTest.php updated to the new policy.
@pekral

pekral commented Jun 29, 2026

Copy link
Copy Markdown
Owner Author

Technical report

Code review (self-check): clean. The change is confined to agent prose (daidalos/argos/athena), docs/agents.md, the worktree pinning test, and CHANGELOG.md. The core invariant is preserved — the writing path (talos) still never uses worktrees and concurrent writers still serialise on the shared tree via the write-lock; only the read-only CR pass gains the worktree opt-in, and daidalos owns its cleanup. No production PHP touched.

Security review: no-op. No code path, input handling, external request, or secret surface is added or modified; the worktree creation is git worktree add of a read-only review checkout (no edits/commits/pushes there) and cleanup uses git worktree remove/prune without --force and skips the active/dirty tree.

Verification: composer build green — 324 tests / 1856 assertions, 100% coverage; vendor/bin/pest tests/Installer/AgentsTest.php 29 passed.

…pt-in

Two restatements still said worktrees were forbidden workflow-wide, which
contradicted the new read-only CR worktree opt-in in the same file. Scope
both to the writing path, consistent with docs/agents.md.

Self-CR (PR #728): resolves 2 Moderate consistency findings.
@pekral

pekral commented Jun 29, 2026

Copy link
Copy Markdown
Owner Author

CR status — converged

Loop: converged on iteration 1 — 0 Critical / 0 Moderate / 0 Minor.
Reviewer threads: none open (0 unresolved).
Reviewer comments: 0/0 fulfilled (no line-anchored reviewer threads on this PR).

Resolved items

  • Leftover "worktrees are forbidden" prose contradicted the new CR worktree opt-in (agents/daidalos.md:56)

    • Why: the Multiple top-level runs paragraph still stated worktrees were forbidden workflow-wide.
    • Reason: the policy relaxation was applied at the section head but a secondary restatement was left absolute, so the agent file carried two conflicting instructions.
    • Solution: scoped the claim to the writing path — "Because the writing path never uses worktrees, every writing run shares one git working tree".
  • Blocked remediation still said "git worktrees are forbidden in this workflow" (agents/daidalos.md:86, step 5)

    • Why: the step-5 lock-contention message asserted a workflow-wide worktree ban that no longer holds.
    • Reason: same leftover absolute wording; it also diverged from the already-corrected mirror in docs/agents.md:144.
    • Solution: reworded to "the writing path takes no worktree, so there is no parallel-isolation escape", matching the docs mirror.

Verification: composer build green — 324 tests / 1856 assertions, 100% coverage; vendor/bin/pest tests/Installer/AgentsTest.php 29 passed.

@pekral
pekral marked this pull request as ready for review June 29, 2026 15:13
@pekral
pekral merged commit 8123df3 into master Jun 29, 2026
1 check passed
@pekral
pekral deleted the feat/cr-agents-parallel-review-worktrees branch June 29, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant