[thomhurst]

Problem

1. No home for agent evidence. Agent guidance said to keep evidence "outside the worktree" without naming a place, so logs, PR-body drafts and validation folders accumulated loose in the repository's parent directory, where no cleanup looks.
2. Lock markers leak into new worktrees. git worktree add copies the source checkout's config.worktree into the new worktree. A stale agent.lockName in the main checkout, or in any worktree used as a source, therefore reached every new worktree, and implicit status/renew/release resolved the wrong Redis lock.

Changes

• scripts/WorktreeCleanup.ps1: Get-AgentEvidenceRoot (<main checkout>-evidence) and Remove-StaleAgentEvidence, which removes each top-level entry whose newest write is older than the retention window. Links are skipped; directory junctions inside an entry are detached before deletion.
• scripts/Remove-MergedWorktrees.ps1: new -EvidenceRetentionDays (default 7, range 1 to 3650); the sweep runs the evidence cleanup (honouring -WhatIf) and reports the count in the summary line.
• scripts/AgentLocks.ps1: Set-WorktreeLockMarker refuses the main checkout (git-dir equals git-common-dir) and writes agent.lockWorktree with the worktree's full path next to agent.lockName. Get-WorktreeLockMarker trusts a marker only when agent.lockWorktree matches the current worktree. Markers without agent.lockWorktree are ignored; no compatibility path.
• .claude/skills/issue-pr-loop/SKILL.md: one rule for where agent output goes (evidence root, worktrees for throwaway checkouts, shared toolchain directory, never the parent directory or the main checkout) and the 7-day retention.
• Tests: new scripts/Test-AgentEvidenceCleanup.ps1 (no Docker); scripts/Test-AgentLocks.ps1 gains an inherited-marker case and a main-checkout case.

Validation

• pwsh scripts/Test-AgentEvidenceCleanup.ps1: passed.
• pwsh scripts/Test-WorktreeCleanup.ps1: passed.
• pwsh scripts/Remove-MergedWorktrees.ps1 -WhatIf: summary line includes the evidence count.
• All edited scripts parse cleanly with the PowerShell parser.
• pwsh scripts/Test-AgentLocks.ps1: not run to completion locally. It needs Docker for Redis and Docker Desktop failed to start on the test machine (Error response from daemon: Docker Desktop is unable to start). The marker functions were checked separately without Redis against a temp repository: the main checkout got no marker, a worktree's own marker resolved, and a worktree created from it inherited the copied agent.lockName but did not resolve it.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0ce4c39f-06c2-48bb-9fee-5dc367c41883

📥 Commits

Reviewing files that changed from the base of the PR and between 136fc4f and 7cd1277.

📒 Files selected for processing: 6

• .claude/skills/issue-pr-loop/SKILL.md
• scripts/AgentLocks.ps1
• scripts/Remove-MergedWorktrees.ps1
• scripts/Test-AgentEvidenceCleanup.ps1
• scripts/Test-AgentLocks.ps1
• scripts/WorktreeCleanup.ps1

+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Most software today is very much like an Egyptian pyramid with millions of bricks piled on top of each other, with no structural integrity, but just done by brute force and thousands of slaves. - Alan Kay |
+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Review

Overall this is a solid, well-scoped change. The agent.lockWorktree ownership check is a better fix for the inherited-marker problem than trying to scrub config.worktree after git worktree add. Refusing to write a marker in the main checkout also removes the source of the leak. The evidence sweep skips links and detaches junctions before deleting, which is the right level of care for a destructive cleanup. I found no blocking issues, and there were no earlier review threads to check (only CodeRabbit's in-progress summary).

Non-blocking suggestions:

1. Stale-by-mtime can delete evidence for a live item. Remove-StaleAgentEvidence judges age only by the newest write under an entry. A long-running item that has been quiet for more than 7 days, for example a PR waiting on review, loses its logs and PR-body drafts while it still holds its Redis lock. The sweep already knows about locks and PRs, so it could skip entries whose name (issue-N or pr-N) matches an open PR or a held lock. That would make retention follow item state rather than a clock. If you'd rather keep it simple, say in the SKILL.md rule that evidence is disposable after 7 days of inactivity.

2. Pull the marker-path logic into one place. Get-MarkerPath is defined after its first use. That works in PowerShell because the call happens at runtime, but moving it above Get-WorktreeLockMarker reads better. The comparison also relies on -ne, which is case-insensitive. That's fine on Windows, but on Linux it would treat paths differing only by case as the same worktree. Using -ceq when not on Windows would be stricter, though the risk is very low.

3. Silent failure of the agent.lockWorktree write. The second git config --worktree call discards its exit code. If it fails after agent.lockName was written, you get a marker that is never trusted. That is a safe failure, but status, renew and release will then silently require an explicit -LockName. Emit a warning on non-zero exit, or write lockWorktree first so a half-written marker never exists.

4. The new test isn't wired into CI. Test-AgentEvidenceCleanup.ps1 needs only PowerShell, so it is cheap to run in the workflow or a pre-merge check. As written it only runs when someone remembers to. The PR description also notes that Test-AgentLocks.ps1 was not run end to end locally. Please confirm it passes in an environment with Docker before merging, since the inherited-marker and main-checkout cases are only covered there.

5. Minor: the summary text "stale evidence entr(ies)" is awkward. "stale evidence entries" reads fine for any count.

Nice work on the tests for the dry-run and -WhatIf paths and the missing-root case.

[thomhurst]

Pushed directly to main as 7cd1277.

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The changes affect the agent work-item workflow and its local cleanup sweep.

Fix the Linux path comparison before merging so copied markers cannot select another worktree's lock.

Findings

1.  Different worktrees share a marker ▶
2.  Failed scans allow evidence deletion ▶
3.  Link safeguards lack tests ▶

Summary

The PR gives agent evidence a shared home and adds age-based cleanup. It also ties lock markers to the worktree that wrote them.

• Agent evidence now has a home and expires after a set time.
• Lock markers now apply only to the worktree that owns them.

_{Reviews (1) · Last reviewed commit: "fix(agents): give agent output a well-kn..." · Reviewed by Greptile}

[greptile-apps]

Different worktrees share a marker

Get-WorktreeLockMarker compares paths with -ne, which ignores case even on Linux. If /work/task inherits a marker from /work/Task, this check accepts it despite those being separate directories. Implicit status, renew, or release then targets the first worktree's lock. Compare paths case-sensitively outside Windows, as Test-SameNativePath already does.

[greptile-apps]

Failed scans allow evidence deletion

The age scan suppresses errors and treats the files it could read as the complete entry. If a nested directory cannot be listed, its recent files are omitted. The sweep can then delete readable files from an entry that should be kept. Skip the entry and log a warning whenever its age scan is incomplete.

[greptile-apps]

Link safeguards lack tests

The new cleanup test never creates a link, so it cannot catch changes that break the sweep's link safeguards. Add a top-level linked entry and a stale folder containing a junction or symlink to an outside directory. Assert that the linked entry is skipped and that cleanup leaves the outside files unchanged.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!