Clean dirty default agent workdirs

[wwind123]

Summary

• clean dirty tool-owned default agent checkouts with git reset/clean before reuse
• keep explicit git workdirs conservative by refusing dirty state
• add scratch-file guidance to coder/reviewer prompts and document disposable default checkouts

Fixes #47

Tests

• python -m pytest

[wwind123]

Implemented issue #47 and opened PR #49.

Tests run:
python -m pytest -> 118 passed

-- OpenAI Codex

[wwind123]

I have a complete picture of the PR. Let me now write the review.

---

PR #49 — Clean dirty default agent workdirs

Overview: This PR differentiates between tool-owned default checkouts (auto-cleaned on dirty) and user-supplied explicit checkouts (still rejected on dirty). It also adds SCRATCH_FILE_GUIDANCE to every agent prompt to discourage agents from leaving temp files in the repo worktree.

config.py

ensure_explicit_workdir — logic is correct. The .git existence check before running rev-parse is sensible defensive programming; it avoids triggering the dirty check for plain directories that happen to live inside a parent git repo. The double-check (.git exists AND rev-parse returns "true") guards against a corrupt .git entry. check=False on rev-parse makes the fallthrough safe.

ensure_temp_checkout dirty branch — git reset --hard (no ref) resets to HEAD; combined with git clean -fd this correctly removes all staged/unstaged tracked changes and untracked files/dirs. Ignored files (.gitignore) are intentionally preserved — fine for build caches. The remote-URL check happens before the dirty check, so a default workdir pointing at the wrong repo still fails clearly. ✓

ensure_agent_workdirs — correctly routes auto dirs to ensure_temp_checkout and explicit dirs to the new ensure_explicit_workdir.

tests/test_agent_loop.py

• test_dirty_existing_auto_agent_dir_is_cleaned_before_reuse correctly wires capsys, verifies reset --hard and clean -fd appear in commands, verifies the log message, and verifies the agent ultimately ran. The FakeRunner returns dirty status for every git status call throughout the run, but ensure_temp_checkout only checks status once before cleaning and doesn't re-check after — so the fixed-value stub is valid.
• test_dirty_explicit_agent_dir_still_fails_clearly correctly relies on make_config's create_dirs=True default to pre-create codex_dir, then manually adds .git/ to trigger the dirty-check path. It asserts neither reset nor clean ran, and that codex exec was never invoked. ✓
• test_agent_prompts_keep_scratch_files_outside_repo_worktree covers all six prompt builders, including build_task_clarification_prompt. ✓

prompts.py

SCRATCH_FILE_GUIDANCE is a single constant injected at each prompt build site — correct. The /tmp/coding-review-agent-loop/scratch/ path is a hardcoded suggestion; /tmp works on Linux and macOS, and this is guidance text not executed code, so portability is not a concern.

docs/local_agent_loop.md

Accurately describes the new split between auto (disposable, cleaned) and explicit (conservative, fails on dirty).

No blocking issues found.

Same-PR follow-ups

None that rise to the bar of "needs fixing before merge."

Future follow-ups

• ensure_explicit_workdir does not validate that the explicit path's git remote matches config.repo (unlike ensure_temp_checkout which calls _looks_like_repo_remote). For a future PR, adding the same remote check to the explicit-workdir path would give a clearer error if a user accidentally passes the wrong checkout.

-- Anthropic Claude

[wwind123]

The PR successfully implements the cleaning of dirty default agent workdirs, improving the usability of the tool by avoiding manual cleanup steps for tool-managed checkouts. It also correctly maintains a conservative approach for user-supplied explicit workdirs.

Analysis

• ensure_temp_checkout: The addition of git reset --hard and git clean -fd when a default workdir is dirty is a welcome change. It ensures that the agent always starts with a clean slate without requiring user intervention for directories that are explicitly managed by the tool.
• ensure_explicit_workdir: Correctly separates the logic for explicit directories, ensuring that the tool does not accidentally wipe out user-managed work. The check for .git existence and the use of rev-parse --is-inside-work-tree provide a safe way to detect git repos before checking their status.
• SCRATCH_FILE_GUIDANCE: Providing explicit guidance to agents about where to put temporary files is a proactive way to keep the worktree clean and reduce the likelihood of "dirty" states in the first place.
• Test Coverage: New tests in tests/test_agent_loop.py effectively verify both the auto-cleaning of default dirs and the continued blocking of dirty explicit dirs. The prompt test ensures the guidance is present in all relevant contexts.

Future follow-ups

• Remote validation for explicit workdirs: As noted by the other reviewer, ensure_explicit_workdir could be enhanced to verify that the git remote of the explicit path matches the target config.repo, providing an even better safety net for users.

I have verified the changes and confirmed that all 118 tests pass.

-- Google Gemini

[wwind123]

Created approved-review future follow-up issues for PR #49:

• Follow up future review note: ensure_explicit_workdir does not validate that the explicit path's git remote matches `c #50
• Follow up future review note: **Remote validation for explicit workdirs**: As noted by the other reviewer, `ensure_expli #51

These were mentioned in approved reviews as future work and did not block merge readiness.

-- coding-review-agent-loop

[wwind123]

The two future follow-up issues created from this PR (#50 and #51) are duplicates. I closed #51 as a duplicate of #50.

Also, this looks like a small localized hardening item rather than major independent future work: ensure_explicit_workdir can reuse the existing remote validation pattern from default temp checkouts. If PR #49 is still open for changes, this would fit better as a same-PR follow-up than as a separate future issue.

-- OpenAI Codex

[wwind123]

Closing this PR as superseded.

The useful pieces from PR #49 have already landed on current main through later merged work, especially PR #55 and follow-up merges:

• Dirty tool-owned default temp workdirs are cleaned only when dirty.
• Explicit user-provided workdirs still fail safely when dirty.
• Explicit workdirs now validate that their origin remote matches config.repo, which was the follow-up raised in this PR's reviews.
• Scratch-file prompt guidance is present for agents.
• Docs and tests cover the current behavior.

This PR is now conflicting and older than the implementation on main, so merging it would be redundant and risks regressing newer fixes.

-- OpenAI Codex

[wwind123]

Closed as superseded by later merged work on main.\n\n-- OpenAI Codex