docs: agent temp artifact cleanup guidance and detection (#261)
Add runbook section, root-level .gitignore patterns, and detection helper for _encode_/_emit_/_inline_ throwaway scripts. Surface non-blocking warnings from assess_preflight_status and gitea_lock_issue. Closes #261 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
+1
-1
@@ -10,7 +10,7 @@ gitea-mcp*.json
|
||||
.vscode/
|
||||
graphify-out/
|
||||
branches/
|
||||
# Throwaway agent commit helpers (delete after MCP commit; see docs/llm-workflow-runbooks.md)
|
||||
# Throwaway agent commit-encoding helpers (#261) — never commit.
|
||||
/_encode_*.py
|
||||
/_emit_*.py
|
||||
/_inline_*.py
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
"""Detect throwaway agent helper scripts left in the repo root (#261)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import fnmatch
|
||||
|
||||
AGENT_TEMP_BASENAME_PATTERNS = (
|
||||
"_encode_*.py",
|
||||
"_emit_*.py",
|
||||
"_inline_*.py",
|
||||
)
|
||||
|
||||
|
||||
def find_agent_temp_artifacts_from_porcelain(porcelain: str) -> list[str]:
|
||||
"""Return untracked repo-root helper paths matching agent temp patterns."""
|
||||
found: list[str] = []
|
||||
for line in (porcelain or "").splitlines():
|
||||
if not line.startswith("??"):
|
||||
continue
|
||||
path = line[3:].strip()
|
||||
if not path or "/" in path or "\\" in path:
|
||||
continue
|
||||
basename = path.split("/")[-1]
|
||||
if any(fnmatch.fnmatch(basename, pat) for pat in AGENT_TEMP_BASENAME_PATTERNS):
|
||||
found.append(path)
|
||||
return sorted(found)
|
||||
@@ -316,6 +316,30 @@ may edit another issue's branch folder unless explicitly assigned to that issue.
|
||||
No LLM may clean another issue's branch folder unless the PR is merged or closed
|
||||
and cleanup is explicitly part of the task.
|
||||
|
||||
## Agent temp artifact cleanup (#261)
|
||||
|
||||
Failed or aborted MCP commit attempts sometimes leave throwaway helper scripts in
|
||||
the **repository root**. These are not part of any issue scope and pollute
|
||||
`git status`, which can break `gitea_lock_issue` and preflight checks.
|
||||
|
||||
**Patterns (repo root only, untracked):**
|
||||
|
||||
- `_encode_*.py` — base64 payload encoders
|
||||
- `_emit_*.py` — commit payload emitters
|
||||
- `_inline_*.py` — inline encoding helpers
|
||||
|
||||
**Required cleanup (after MCP commit completes or aborts):**
|
||||
|
||||
1. Delete any matching files at the repo root (`rm ./_encode_*.py` etc.).
|
||||
2. Confirm `git status` is clean on the orchestration checkout before
|
||||
`gitea_lock_issue`.
|
||||
3. Prefer native `gitea_commit_files` / gated commit paths — do not leave shell
|
||||
encoding fallbacks behind.
|
||||
|
||||
Root-level matches are listed in `.gitignore` so they never get committed.
|
||||
`gitea_get_runtime_context` and `gitea_lock_issue` surface **warnings** (not
|
||||
hard blocks) when these artifacts are still present.
|
||||
|
||||
Implementation work and review work must use separate branch folders. For
|
||||
example, an implementation branch might live under
|
||||
`branches/fix-issue-123-example`, while a review branch for the resulting PR
|
||||
@@ -370,26 +394,6 @@ git branch -d fix/issue-123-example
|
||||
All three helpers accept `--dry-run` to print the exact commands/paths without
|
||||
touching anything.
|
||||
|
||||
### Agent temp artifact cleanup (#261)
|
||||
|
||||
Failed or exploratory agent runs sometimes leave throwaway helper scripts in the
|
||||
repo root (for example `_encode_commit_payload.py`, `_emit_payload.py`,
|
||||
`_inline_commit.py`). These files are **not** part of any issue scope. They
|
||||
pollute `git status`, break `gitea_lock_issue`, and can trigger preflight
|
||||
fail-closed gates if created before `gitea_whoami`.
|
||||
|
||||
**After every MCP commit attempt** (success or abort):
|
||||
|
||||
1. Delete any root-level `_encode_*.py`, `_emit_*.py`, or `_inline_*.py` helpers
|
||||
the agent created for payload preparation.
|
||||
2. Run `git status --porcelain` in the **author worktree** and confirm no
|
||||
unexpected tracked or untracked files remain before `gitea_lock_issue`.
|
||||
3. Do not commit these helpers; stage only issue-scoped paths (`git add <files>`).
|
||||
|
||||
The repo `.gitignore` ignores these patterns so accidental leftovers stay
|
||||
untracked. Ignored files still block issue-lock if they appear as tracked edits —
|
||||
delete them instead of leaving them in the tree.
|
||||
|
||||
### Create an issue / child issues
|
||||
|
||||
- **Profile:** issue-manager or author (any profile allowed to create issues).
|
||||
|
||||
+21
-1
@@ -260,9 +260,19 @@ def assess_preflight_status() -> dict:
|
||||
"Reviewer profile modified tracked workspace files after capability resolution "
|
||||
f"(offending files: {_format_preflight_files(_preflight_reviewer_violation_files)})"
|
||||
)
|
||||
warnings: list[str] = []
|
||||
agent_artifacts = agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(
|
||||
_get_workspace_porcelain()
|
||||
)
|
||||
if agent_artifacts:
|
||||
warnings.append(
|
||||
"Agent temp artifacts detected at repo root (delete before mutations): "
|
||||
f"{_format_preflight_files(agent_artifacts)}"
|
||||
)
|
||||
return {
|
||||
"preflight_ready": not reasons,
|
||||
"preflight_block_reasons": reasons,
|
||||
"preflight_warnings": warnings,
|
||||
"preflight_whoami_verified": _preflight_whoami_called,
|
||||
"preflight_capability_resolved": _preflight_capability_called,
|
||||
"preflight_whoami_violation_files": list(_preflight_whoami_violation_files),
|
||||
@@ -373,6 +383,7 @@ import role_session_router # noqa: E402
|
||||
import role_namespace_gate # noqa: E402
|
||||
import task_capability_map # noqa: E402
|
||||
import review_proofs # noqa: E402
|
||||
import agent_temp_artifacts
|
||||
import issue_lock_worktree # noqa: E402
|
||||
|
||||
|
||||
@@ -877,7 +888,10 @@ def gitea_lock_issue(
|
||||
except Exception as e:
|
||||
raise RuntimeError(f"Could not write issue lock file: {e}")
|
||||
|
||||
return {
|
||||
agent_artifacts = agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(
|
||||
git_state.get("porcelain_status") or ""
|
||||
)
|
||||
result = {
|
||||
"success": True,
|
||||
"message": (
|
||||
f"Successfully locked issue #{issue_number} to branch '{branch_name}' "
|
||||
@@ -887,6 +901,12 @@ def gitea_lock_issue(
|
||||
"branch_name": branch_name,
|
||||
"worktree_path": resolved_worktree,
|
||||
}
|
||||
if agent_artifacts:
|
||||
result["warnings"] = [
|
||||
"Agent temp artifacts at repo root (delete before implementation): "
|
||||
+ ", ".join(agent_artifacts)
|
||||
]
|
||||
return result
|
||||
|
||||
|
||||
@mcp.tool()
|
||||
|
||||
@@ -20,8 +20,6 @@ Steps:
|
||||
(removes branches/<type>-issue-<n>-<slug>; refuses if dirty; git branch -d is
|
||||
safe-delete only — fails on unmerged.)
|
||||
5. Delete the remote branch if the merge did not already remove it.
|
||||
5b. Delete any throwaway agent helpers (`_encode_*.py`, `_emit_*.py`, `_inline_*.py`)
|
||||
left in the author worktree (see docs/llm-workflow-runbooks.md § Agent temp artifact cleanup).
|
||||
6. From the main checkout: git fetch <remote> --prune; git checkout master;
|
||||
git reset --hard <remote>/master ONLY if local master safely matches remote.
|
||||
7. Confirm main checkout clean and current (git status; 0 0 vs <remote>/master).
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
"""Tests for agent temp artifact detection and preflight warnings (#261)."""
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent))
|
||||
|
||||
import agent_temp_artifacts
|
||||
import mcp_server
|
||||
|
||||
|
||||
class TestAgentTempArtifactPatterns(unittest.TestCase):
|
||||
def test_detects_root_level_encode_emit_inline(self):
|
||||
porcelain = (
|
||||
"?? _encode_commit_payload.py\n"
|
||||
"?? _emit_payload.py\n"
|
||||
"?? _inline_b64.py\n"
|
||||
)
|
||||
self.assertEqual(
|
||||
agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(porcelain),
|
||||
["_emit_payload.py", "_encode_commit_payload.py", "_inline_b64.py"],
|
||||
)
|
||||
|
||||
def test_ignores_nested_and_unrelated_untracked(self):
|
||||
porcelain = (
|
||||
"?? scripts/_encode_x.py\n"
|
||||
"?? README-draft.md\n"
|
||||
" M task_capability_map.py\n"
|
||||
)
|
||||
self.assertEqual(
|
||||
agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(porcelain),
|
||||
[],
|
||||
)
|
||||
|
||||
|
||||
class TestPreflightWarnings(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self._snapshot = (
|
||||
mcp_server._preflight_whoami_called,
|
||||
mcp_server._preflight_capability_called,
|
||||
)
|
||||
mcp_server._preflight_whoami_called = True
|
||||
mcp_server._preflight_capability_called = True
|
||||
|
||||
def tearDown(self):
|
||||
mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = (
|
||||
self._snapshot
|
||||
)
|
||||
|
||||
@patch(
|
||||
"mcp_server._get_workspace_porcelain",
|
||||
return_value="?? _encode_commit_payload.py\n",
|
||||
)
|
||||
def test_assess_preflight_surfaces_warning_not_block(self, _porcelain):
|
||||
status = mcp_server.assess_preflight_status()
|
||||
self.assertTrue(status["preflight_ready"])
|
||||
self.assertEqual(status["preflight_block_reasons"], [])
|
||||
self.assertEqual(len(status["preflight_warnings"]), 1)
|
||||
self.assertIn("_encode_commit_payload.py", status["preflight_warnings"][0])
|
||||
|
||||
|
||||
class TestIssueLockArtifactWarning(unittest.TestCase):
|
||||
@patch("mcp_server.api_get_all", return_value=[])
|
||||
@patch("mcp_server._auth", return_value="token x")
|
||||
@patch("mcp_server._resolve", return_value=("h", "o", "r"))
|
||||
@patch("mcp_server.ISSUE_LOCK_FILE", new_callable=lambda: tempfile.mktemp())
|
||||
@patch("issue_lock_worktree.read_worktree_git_state")
|
||||
def test_lock_success_includes_artifact_warning(self, mock_state, _lock_file, *_mocks):
|
||||
mock_state.return_value = {
|
||||
"current_branch": "master",
|
||||
"porcelain_status": "?? _emit_payload.py\n",
|
||||
}
|
||||
result = mcp_server.gitea_lock_issue(
|
||||
issue_number=261,
|
||||
branch_name="docs/issue-261-agent-artifact-cleanup",
|
||||
remote="prgs",
|
||||
)
|
||||
self.assertTrue(result["success"])
|
||||
self.assertIn("warnings", result)
|
||||
self.assertIn("_emit_payload.py", result["warnings"][0])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user