Skip to content

Commit 786cb46

Browse files
NagyViktNagyViktclaude
authored
fix(gx): teach gx cleanup the state-file allowlist + log why a dirty worktree was kept (#550)
`is_clean_worktree` in templates/scripts/agent-worktree-prune.sh previously excluded exactly one path (.omx/state/agent-file-locks.json) from its dirty check, so any worktree with .omc/state/*.json or apps/logs/*.log or .dev-ports.json activity got tagged dirty and skipped, even though those globs are non-authoritative per the contract that PRs #546/#547 established. The skip-dirty log was also a dead end: it told you a worktree was preserved but never which files made it dirty, forcing per-worktree cd + git status. This change: - Generalizes the exclude list to the shared agent-state-file glob set (.omc/**, .omx/state/**, .dev-ports.json, apps/logs/**, .agents/settings.local.json, .codex/state/**, .claude/state/**), overridable via GUARDEX_PRUNE_STATE_EXCLUDE_GLOBS. Worktrees with only state-file dirt now auto-prune; worktrees with real code changes outside the allowlist still skip. - Adds summarize_worktree_dirt() and wires it into the skip-dirty branch of process_entry. Each preserved worktree now logs up to 3 modified + 3 untracked paths (with "(+N more)" tail), indented under the existing Skipping line. The summary honors the same state-file excludes so it only surfaces real work. Verified live against the primary repo: 2 of the locally-stale agent worktrees skipped with their real code dirt (.github/workflows/cr.yml, src/cockpit/*) listed in the indented summary; previously they were silent skips with no diagnostic. Only templates/scripts/agent-worktree-prune.sh is edited; scripts/agent-worktree-prune.sh is a symlink to it (PR #548) and inherits the fix transparently. OpenSpec change at openspec/changes/agent-claude-fix-cleanup-orphan-worktree-prune-2026-05-11-12-19/. Co-authored-by: NagyVikt <nagy.viktordp@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 89e6c49 commit 786cb46

5 files changed

Lines changed: 174 additions & 3 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-05-11
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Proposal: make `gx cleanup` dirt-aware of state-file globs + log why a worktree was kept
2+
3+
## Problem
4+
5+
Two related shortcomings in `templates/scripts/agent-worktree-prune.sh` made `gx cleanup` less useful than it should be:
6+
7+
1. **State-file dirt was misclassified as real work.** `is_clean_worktree()` excluded exactly one path from its dirty check — `.omx/state/agent-file-locks.json` — and treated everything else as if it were authoritative content. A worktree with nothing dirty but agent state (`.omc/state/*.json`, `.omx/state/*.json`, `apps/logs/*.log`, `.dev-ports.json`) was therefore tagged "dirty" and skipped, leaving orphans on disk that the user had to delete manually. This contradicts the allowlist established by PRs #546 / #547.
8+
9+
2. **The skip-dirty log was opaque.** When a worktree was preserved, the script printed `Skipping dirty worktree (<reason>): <path>` and nothing else. With multiple stale worktrees accumulated (8 locally at audit start), per-worktree `cd + git status` was the slow path.
10+
11+
## Approach
12+
13+
One file touched: `templates/scripts/agent-worktree-prune.sh` (the runtime canonical script; `scripts/agent-worktree-prune.sh` is a symlink to it as of PR #548).
14+
15+
### 1. Expand the cleanliness check to honor the shared state-file allowlist
16+
17+
Introduce `WORKTREE_STATE_EXCLUDE_GLOBS_DEFAULT` with the same colon-separated list used by PR #546 (`GUARDEX_AUTO_TRANSFER_EXCLUDE`) and PR #547 (`GUARDEX_FINISH_AUTO_RESOLVE_SAFE_GLOBS`): `.omc/**:.omx/state/**:.dev-ports.json:apps/logs/**:.agents/settings.local.json:.codex/state/**:.claude/state/**`. Convert to `:(exclude,glob)<pattern>` pathspec args once at startup, pass them to all three `is_clean_worktree` subchecks (working-tree diff, cached diff, untracked enumeration).
18+
19+
A worktree whose only deltas are agent state files is now considered clean and gets pruned. Override via `GUARDEX_PRUNE_STATE_EXCLUDE_GLOBS`.
20+
21+
### 2. Add `summarize_worktree_dirt()`, wire it into the skip-dirty branch
22+
23+
When the prune loop skips a dirty worktree, the helper prints up to 3 modified-tracked paths and up to 3 untracked paths (with `(+N more)` tail), using the same state-file excludes so the summary only surfaces work the user cares about. Indented under the existing `Skipping dirty worktree` line.
24+
25+
## Compatibility
26+
27+
- Worktrees with state-file-only dirt are now auto-pruned. `.omc/` and `.omx/state/**` are gitignored per CLAUDE.md and have no authoritative value.
28+
- Worktrees with real code dirt remain skipped; the new diagnostic surfaces which files.
29+
- Env override (`GUARDEX_PRUNE_STATE_EXCLUDE_GLOBS=`) restores strict behavior.
30+
- No CLI surface change.
31+
32+
## Risks
33+
34+
- A user storing local-only notes under `.omc/` would see them deleted at prune. Mitigated by CLAUDE.md's "Never stage or commit: .omc/state/**, .omx/state/**" rule — those paths were always agent-only.
35+
- `summarize_worktree_dirt` runs `git diff` / `git ls-files` twice each per skipped worktree (paths + count). Negligible at typical counts.
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Spec Delta: gx-cleanup worktree prune
2+
3+
## ADDED Requirements
4+
5+
### Requirement: `gx cleanup` MUST treat agent state-file dirt as clean for prune purposes
6+
7+
The `is_clean_worktree` helper in `templates/scripts/agent-worktree-prune.sh` MUST exclude the canonical agent state-file glob list (`.omc/**`, `.omx/state/**`, `.dev-ports.json`, `apps/logs/**`, `.agents/settings.local.json`, `.codex/state/**`, `.claude/state/**`) from all three of its subchecks (working-tree diff, cached diff, untracked-files enumeration). A worktree whose only deltas relative to its HEAD match these globs MUST be classified as clean and become eligible for automatic prune. The exclude list MUST be overridable via the `GUARDEX_PRUNE_STATE_EXCLUDE_GLOBS` environment variable.
8+
9+
#### Scenario: Worktree with only state-file dirt is pruned
10+
11+
- **GIVEN** an orphan agent worktree under `.omc/agent-worktrees/` whose only working-tree changes are modifications to `.omc/state/some.json` and an untracked `apps/logs/run.log`
12+
- **WHEN** `gx cleanup --base main` runs
13+
- **THEN** the worktree is classified as clean
14+
- **AND** the worktree is removed (`removed_worktrees` counter increments)
15+
16+
#### Scenario: Worktree with real code dirt is still skipped
17+
18+
- **GIVEN** an orphan agent worktree with a modified tracked file outside the state-file allowlist (e.g. `src/main.js`)
19+
- **WHEN** `gx cleanup --base main` runs
20+
- **THEN** the worktree is skipped as dirty (`skipped_dirty` counter increments)
21+
22+
#### Scenario: Override restores strict cleanliness
23+
24+
- **GIVEN** `GUARDEX_PRUNE_STATE_EXCLUDE_GLOBS=` (empty) is exported
25+
- **AND** an orphan worktree with only state-file dirt
26+
- **WHEN** `gx cleanup --base main` runs
27+
- **THEN** the worktree is skipped as dirty (no globs excluded; pre-fix behavior preserved)
28+
29+
### Requirement: `gx cleanup` MUST log what's dirty when it skips a worktree
30+
31+
When a worktree is preserved because of dirty content (i.e., `Skipping dirty worktree (<reason>)` is printed), the script MUST also print a summary of up to 3 modified-tracked paths and up to 3 untracked paths, indented under the skip line. If more than 3 of either exist, a `(+N more)` line MUST be appended. The summary MUST honor the same state-file allowlist so it only surfaces real work.
32+
33+
#### Scenario: Dirt summary appears under skip-dirty line
34+
35+
- **GIVEN** an orphan worktree with `src/foo.js` modified and an untracked `src/new-feature.ts`
36+
- **WHEN** `gx cleanup --base main` runs and skips the worktree as dirty
37+
- **THEN** the log includes `Skipping dirty worktree (<reason>): <path>`
38+
- **AND** the log includes an indented line ` modified: src/foo.js`
39+
- **AND** the log includes an indented line ` untracked: src/new-feature.ts`
40+
41+
#### Scenario: Truncated summary with N-more tail
42+
43+
- **GIVEN** an orphan worktree with 8 modified-tracked files outside the state-file allowlist
44+
- **WHEN** `gx cleanup --base main` runs and skips the worktree as dirty
45+
- **THEN** the log shows 3 `modified:` lines
46+
- **AND** a single ` modified: (+5 more)` line follows
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Tasks
2+
3+
## 1. Spec
4+
5+
- [x] 1.1 Capture problem + approach in `proposal.md`.
6+
- [x] 1.2 Add ADDED requirements + scenarios to `specs/gitguardex-agent-lifecycle/spec.md`.
7+
8+
## 2. Implementation
9+
10+
- [x] 2.1 `templates/scripts/agent-worktree-prune.sh`: add `WORKTREE_STATE_EXCLUDE_GLOBS_DEFAULT` + override env var.
11+
- [x] 2.2 `templates/scripts/agent-worktree-prune.sh`: add `build_state_exclude_pathspec_args` helper and capture into `STATE_EXCLUDE_PATHSPEC_ARGS` array once.
12+
- [x] 2.3 `templates/scripts/agent-worktree-prune.sh`: expand `is_clean_worktree` to apply state-exclude pathspecs to all three subchecks (working-tree diff, cached diff, untracked-files).
13+
- [x] 2.4 `templates/scripts/agent-worktree-prune.sh`: add `summarize_worktree_dirt` helper printing top 3 modified + top 3 untracked with `(+N more)` tail.
14+
- [x] 2.5 `templates/scripts/agent-worktree-prune.sh`: call `summarize_worktree_dirt` in the skip-dirty branch of `process_entry`.
15+
16+
## 3. Verification
17+
18+
- [x] 3.1 `bash -n` clean on the edited script.
19+
- [x] 3.2 Live run: invoke the new script against the primary repo. 2 of the local stale worktrees correctly skipped as dirty, each with a modified-files summary listing real cockpit code (`.github/workflows/cr.yml`, `src/cockpit/control.js`, etc.) — proves the dirt summary works and the state-glob exclude doesn't mis-classify real work.
20+
- [x] 3.3 Confirm symlink: `scripts/agent-worktree-prune.sh` (PR #548 symlink) resolves to the edited file.
21+
22+
## 4. Cleanup
23+
24+
- [ ] 4.1 Commit on `agent/claude/fix-cleanup-orphan-worktree-prune-2026-05-11-12-19`.
25+
- [ ] 4.2 Push and open PR against `main`.
26+
- [ ] 4.3 PR merged (record URL + MERGED state).
27+
- [ ] 4.4 Sandbox worktree pruned via `gx branch finish --cleanup`.

templates/scripts/agent-worktree-prune.sh

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,71 @@ branch_has_worktree() {
249249
git -C "$repo_root" worktree list --porcelain | grep -q "^branch refs/heads/${branch}$"
250250
}
251251

252+
# Globs treated as agent state, not real work. Worktrees whose only "dirty"
253+
# content matches these are considered clean for prune purposes. Mirrors the
254+
# auto-transfer + auto-resolve allowlist from PRs #546/#547 (state-file globs
255+
# never carry authoritative content out of an agent branch).
256+
WORKTREE_STATE_EXCLUDE_GLOBS_DEFAULT='.omc/**:.omx/state/**:.dev-ports.json:apps/logs/**:.agents/settings.local.json:.codex/state/**:.claude/state/**'
257+
WORKTREE_STATE_EXCLUDE_GLOBS_RAW="${GUARDEX_PRUNE_STATE_EXCLUDE_GLOBS-$WORKTREE_STATE_EXCLUDE_GLOBS_DEFAULT}"
258+
259+
build_state_exclude_pathspec_args() {
260+
# Emit one ':(exclude,glob)<pat>' arg per non-empty pattern.
261+
if [[ -z "$WORKTREE_STATE_EXCLUDE_GLOBS_RAW" ]]; then
262+
return 0
263+
fi
264+
local -a globs=()
265+
IFS=':' read -ra globs <<< "$WORKTREE_STATE_EXCLUDE_GLOBS_RAW"
266+
local pattern
267+
for pattern in "${globs[@]}"; do
268+
[[ -z "$pattern" ]] && continue
269+
printf '%s\0' ":(exclude,glob)${pattern}"
270+
done
271+
}
272+
273+
# Capture the state-exclude pathspecs once; reused by is_clean_worktree and
274+
# the dirt-summary logger below.
275+
STATE_EXCLUDE_PATHSPEC_ARGS=()
276+
while IFS= read -r -d '' arg; do
277+
STATE_EXCLUDE_PATHSPEC_ARGS+=("$arg")
278+
done < <(build_state_exclude_pathspec_args)
279+
252280
is_clean_worktree() {
253281
local wt="$1"
254-
git -C "$wt" diff --quiet -- . ":(exclude).omx/state/agent-file-locks.json" \
255-
&& git -C "$wt" diff --cached --quiet -- . ":(exclude).omx/state/agent-file-locks.json" \
256-
&& [[ -z "$(git -C "$wt" ls-files --others --exclude-standard)" ]]
282+
git -C "$wt" diff --quiet -- . "${STATE_EXCLUDE_PATHSPEC_ARGS[@]}" \
283+
&& git -C "$wt" diff --cached --quiet -- . "${STATE_EXCLUDE_PATHSPEC_ARGS[@]}" \
284+
&& [[ -z "$(git -C "$wt" ls-files --others --exclude-standard -- . "${STATE_EXCLUDE_PATHSPEC_ARGS[@]}")" ]]
285+
}
286+
287+
# Returns a short, human-readable summary of why a worktree is considered dirty:
288+
# up to 3 modified-tracked paths + up to 3 untracked paths + a "(+N more)" tail.
289+
# Used to surface actionable context next to "Skipping dirty worktree" log lines
290+
# (previously gave no clue what was actually dirty).
291+
summarize_worktree_dirt() {
292+
local wt="$1"
293+
local modified_paths untracked_paths
294+
modified_paths="$(git -C "$wt" diff --name-only --diff-filter=AMDR -- . "${STATE_EXCLUDE_PATHSPEC_ARGS[@]}" 2>/dev/null | head -3)"
295+
local modified_count
296+
modified_count="$(git -C "$wt" diff --name-only --diff-filter=AMDR -- . "${STATE_EXCLUDE_PATHSPEC_ARGS[@]}" 2>/dev/null | wc -l | tr -d ' ')"
297+
untracked_paths="$(git -C "$wt" ls-files --others --exclude-standard -- . "${STATE_EXCLUDE_PATHSPEC_ARGS[@]}" 2>/dev/null | head -3)"
298+
local untracked_count
299+
untracked_count="$(git -C "$wt" ls-files --others --exclude-standard -- . "${STATE_EXCLUDE_PATHSPEC_ARGS[@]}" 2>/dev/null | wc -l | tr -d ' ')"
300+
301+
if [[ -n "$modified_paths" ]]; then
302+
while IFS= read -r p; do
303+
[[ -n "$p" ]] && echo " modified: ${p}"
304+
done <<< "$modified_paths"
305+
if [[ "$modified_count" -gt 3 ]]; then
306+
echo " modified: (+$((modified_count - 3)) more)"
307+
fi
308+
fi
309+
if [[ -n "$untracked_paths" ]]; then
310+
while IFS= read -r p; do
311+
[[ -n "$p" ]] && echo " untracked: ${p}"
312+
done <<< "$untracked_paths"
313+
if [[ "$untracked_count" -gt 3 ]]; then
314+
echo " untracked: (+$((untracked_count - 3)) more)"
315+
fi
316+
fi
257317
}
258318

259319
resolve_worktree_common_dir() {
@@ -482,6 +542,7 @@ process_entry() {
482542
if [[ "$FORCE_DIRTY" -ne 1 ]] && ! is_clean_worktree "$wt"; then
483543
skipped_dirty=$((skipped_dirty + 1))
484544
echo "[agent-worktree-prune] Skipping dirty worktree (${remove_reason}): ${wt}"
545+
summarize_worktree_dirt "$wt"
485546
return
486547
fi
487548

0 commit comments

Comments
 (0)