From 27922c67f80dd920cc017d8cc3b26717b7dd4516 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Mon, 6 Jul 2026 11:34:43 -0400 Subject: [PATCH 1/5] feat(issue-filing): harden final report proofs for handoff and mutations (#191) Add assess_issue_filing_final_report and supporting checks for Controller Handoff (issue_filing role), exact issue number/title, full 40-char SHAs, per-mutation capability proof, single-mutation scope statements, and duplicate-search summaries. Workflow skill documents the issue-filing handoff fields. Co-Authored-By: Claude Opus 4.8 (1M context) --- review_proofs.py | 308 +++++++++++++++++++++++++++ skills/llm-project-workflow/SKILL.md | 6 + tests/test_review_proofs.py | 113 ++++++++++ 3 files changed, 427 insertions(+) diff --git a/review_proofs.py b/review_proofs.py index 3356a23..5df0ef8 100644 --- a/review_proofs.py +++ b/review_proofs.py @@ -1297,6 +1297,16 @@ HANDOFF_ROLE_FIELDS = { ("Session authored PR", ("session authored pr", "authored pr")), ("Why continuation allowed", ("why continuation", "continuation allowed")), ), + "issue_filing": ( + ("Issue created or updated", ( + "issue created or updated", + "issue created", + "issue updated", + "created issue", + "updated issue", + )), + ("Related issues", ("related issues",)), + ), } @@ -2135,6 +2145,304 @@ def assess_issue_selection_final_report( } +_SHA_EVIDENCE_LABEL = re.compile( + r"(?:\b(?:old|new)\s+head\b|\bhead\s+sha\b|\bcommit\s+sha\b|\bsha\b)" + r"\s*[:=]?\s*([0-9a-f]+)", + re.IGNORECASE, +) + + +def assess_issue_filing_sha_evidence(report_text: str) -> dict: + """Issue #191: commit evidence must use full 40-hex SHAs.""" + text = report_text or "" + reasons = [] + for match in _SHA_EVIDENCE_LABEL.finditer(text): + sha = match.group(1).strip().lower() + if sha and not _FULL_SHA.match(sha): + reasons.append( + f"abbreviated SHA in evidence ({len(sha)} hex chars); " + "full 40-character SHA required" + ) + return { + "complete": not reasons, + "downgraded": bool(reasons), + "reasons": reasons, + } + + +def assess_issue_filing_issue_reference( + report_text: str, + *, + issue_number: int | None = None, + issue_title: str | None = None, + action: str = "created", +) -> dict: + """Issue #191: reports must cite exact issue number and title.""" + text = (report_text or "").lower() + reasons = [] + if issue_number is None: + reasons.append("issue number proof missing from assessment context") + else: + num_patterns = (f"#{issue_number}", f"issue #{issue_number}", + f"issue {issue_number}") + if not any(p in text for p in num_patterns): + reasons.append( + f"report missing exact issue number #{issue_number}" + ) + if issue_title: + title_fragment = issue_title.strip().lower()[:40] + if title_fragment and title_fragment not in text: + reasons.append("report missing exact issue title") + action = (action or "created").strip().lower() + if action == "created" and "creat" not in text: + reasons.append( + "report must distinguish new issue creation from update" + ) + if action == "updated" and "updat" not in text: + reasons.append( + "report must distinguish issue update from new creation" + ) + return { + "complete": not reasons, + "downgraded": bool(reasons), + "reasons": reasons, + } + + +def assess_issue_filing_mutation_capability( + report_text: str, + mutations: list[str] | None, + resolved_capabilities: dict | None = None, +) -> dict: + """Issue #191: every mutation needs exact capability proof in the report.""" + import task_capability_map + + text = (report_text or "").lower() + reasons = [] + mutation_list = list(mutations or []) + if not mutation_list: + reasons.append("no mutations listed for capability proof") + cap_proof = assess_capability_proof(resolved_capabilities or {}) + if not cap_proof.get("proven"): + reasons.extend(cap_proof.get("reasons") or []) + + for task in mutation_list: + try: + permission = task_capability_map.required_permission(task) + except KeyError: + reasons.append(f"unknown mutation task '{task}'") + continue + if permission.lower() not in text and task.replace("_", " ") not in text: + reasons.append( + f"mutation '{task}' missing exact capability proof " + f"('{permission}')" + ) + if task == "set_issue_labels": + label_proof = ( + "label change", + "labels changed", + "set label", + "label mutation", + "issue labels", + "label:", + ) + if not any(p in text for p in label_proof): + reasons.append( + "label mutation missing label-specific capability proof" + ) + return { + "complete": not reasons, + "downgraded": bool(reasons), + "reasons": reasons, + } + + +def assess_issue_filing_mutation_scope( + report_text: str, + *, + performed_mutations: list[str] | None = None, + forbidden_mutations: list[str] | None = None, +) -> dict: + """Issue #191: single-mutation runs must state scope and absent mutations.""" + text = (report_text or "").lower() + performed = list(performed_mutations or []) + forbidden = list(forbidden_mutations or [ + "label", "comment", "pr", "review", "merge", "close", + ]) + reasons = [] + + if len(performed) == 1: + only_phrases = ("only mutation", "only mutations", "sole mutation") + if not any(p in text for p in only_phrases): + reasons.append( + "single-mutation run must state 'Only mutation(s): ...'" + ) + for absent in forbidden: + if absent == "comment" and performed[0] in ( + "comment_issue", "create_issue", + ): + continue + if absent in text and f"no {absent}" not in text: + if any( + phrase in text + for phrase in ( + f"no {absent}", + f"no {absent}s", + f"without {absent}", + "none performed", + "none.", + ) + ): + continue + if performed == ["create_issue"]: + for check in ("label", "comment", "pr", "review", "merge"): + if check in text and f"no {check}" not in text: + if not any( + n in text + for n in ( + f"no {check}", + f"no {check}s", + "none performed", + "confirm no", + ) + ): + reasons.append( + f"create-only run must confirm no {check} " + "mutations were performed" + ) + + return { + "complete": not reasons, + "downgraded": bool(reasons), + "reasons": reasons, + } + + +def assess_issue_filing_duplicate_summary( + report_text: str, + *, + issues_searched: int | None = None, + closest_matches: list[dict] | None = None, + duplicate_result: dict | None = None, +) -> dict: + """Issue #191: duplicate-check evidence must be summarized in the report.""" + text = (report_text or "").lower() + reasons = [] + dup = duplicate_result or {} + + if issues_searched is not None: + count_tokens = ( + str(issues_searched), + f"{issues_searched} open", + f"{issues_searched} issue", + ) + if not any(t in text for t in count_tokens): + reasons.append( + "duplicate-check summary missing open-issues searched count" + ) + + matches = closest_matches if closest_matches is not None else dup.get("matches") + for match in matches or []: + num = match.get("number") + if num is not None and f"#{num}" not in text and str(num) not in text: + reasons.append( + f"duplicate-check summary missing closest issue #{num}" + ) + break + + justification_phrases = ( + "why new", + "why update", + "rejected update", + "new issue justified", + "not a duplicate", + "no duplicate", + "justified", + "instead of expanding", + ) + if not any(p in text for p in justification_phrases): + reasons.append( + "duplicate-check summary missing why update was rejected or " + "why a new issue was justified" + ) + + dup_proof = issue_duplicate_gate.assess_duplicate_search_proof( + report_text, matches or [] + ) + if not dup_proof.get("valid"): + reasons.extend(dup_proof.get("reasons") or []) + + return { + "complete": not reasons, + "downgraded": bool(reasons), + "reasons": reasons, + } + + +def assess_issue_filing_final_report( + report_text: str, + *, + issue_number: int | None = None, + issue_title: str | None = None, + action: str = "created", + mutations: list[str] | None = None, + resolved_capabilities: dict | None = None, + issues_searched: int | None = None, + closest_matches: list[dict] | None = None, + duplicate_result: dict | None = None, + performed_mutations: list[str] | None = None, +) -> dict: + """Issue #191: composite A-bar for issue-filing final reports.""" + checks = { + "controller_handoff": assess_controller_handoff( + report_text, role="issue_filing" + ), + "issue_reference": assess_issue_filing_issue_reference( + report_text, + issue_number=issue_number, + issue_title=issue_title, + action=action, + ), + "sha_evidence": assess_issue_filing_sha_evidence(report_text), + "mutation_capability": assess_issue_filing_mutation_capability( + report_text, + mutations or performed_mutations, + resolved_capabilities, + ), + "mutation_scope": assess_issue_filing_mutation_scope( + report_text, + performed_mutations=performed_mutations or mutations, + ), + "duplicate_summary": assess_issue_filing_duplicate_summary( + report_text, + issues_searched=issues_searched, + closest_matches=closest_matches, + duplicate_result=duplicate_result, + ), + } + + reasons = [] + downgraded = False + for name, result in checks.items(): + if result.get("verdict") == "missing": + downgraded = True + reasons.extend(result.get("reasons") or []) + elif result.get("downgraded") or not result.get("complete", True): + downgraded = True + reasons.extend( + f"{name}: {r}" for r in (result.get("reasons") or []) + ) + + grade = "A" if not downgraded else "downgraded" + return { + "grade": grade, + "downgraded": downgraded, + "checks": checks, + "reasons": reasons, + "complete": not downgraded, + } + + def assess_duplicate_search_proof(report_text, matches): """#207: reject LLM duplicate summaries that omit known title matches.""" return issue_duplicate_gate.assess_duplicate_search_proof( diff --git a/skills/llm-project-workflow/SKILL.md b/skills/llm-project-workflow/SKILL.md index fcefe8f..cfb723a 100644 --- a/skills/llm-project-workflow/SKILL.md +++ b/skills/llm-project-workflow/SKILL.md @@ -462,6 +462,12 @@ Role-specific fields (append to the compact block): - review/merge tasks: `Selected PR:`, `Reviewer eligibility:`, `Pinned reviewed head:`, `Review decision:`, `Merge result:`, `Linked issue status:`, `Cleanup status:` +- issue-filing tasks (#191): `Issue created or updated:`, `Related issues:`; + body must cite exact issue number/title, duplicate-search summary (issues + searched, closest matches, why update rejected / new issue justified), full + 40-char SHAs when citing commits, exact mutation capability per change, and + `Only mutation(s):` when a single mutation was performed + (`review_proofs.assess_issue_filing_final_report`). - author tasks: `Selected issue:`, `Claim/comment status:`, `PR number opened:`, `No review/merge:` (explicit confirmation) - continuation tasks (#188): `Continuation mode:`, `Existing PR:`, diff --git a/tests/test_review_proofs.py b/tests/test_review_proofs.py index 92bdfc0..7166cca 100644 --- a/tests/test_review_proofs.py +++ b/tests/test_review_proofs.py @@ -33,6 +33,9 @@ from review_proofs import ( # noqa: E402 assess_empty_queue_report, assess_fresh_issue_selection, assess_inventory_completeness, + assess_issue_filing_final_report, + assess_issue_filing_mutation_capability, + assess_issue_filing_sha_evidence, assess_issue_selection_final_report, assess_queue_target_final_report, assess_reviewer_queue_inventory, @@ -1929,5 +1932,115 @@ class TestIssueSelectionContinuation(unittest.TestCase): self.assertEqual(result["grade"], "A") +class TestIssueFilingFinalReport(unittest.TestCase): + """Issue #191: issue-filing runs need A-bar final report proofs.""" + + ISSUE_TITLE = ( + "Implement fail-closed continuation mode for issues already " + "represented by open PRs" + ) + FULL_SHA = "a4060c5de00f2b1c9e88f4f6f0f3f9a7b2c1d0e9" + CAPABILITIES = { + "create_issue": { + "requested_task": "create_issue", + "required_operation_permission": "gitea.issue.create", + "allowed_in_current_session": True, + }, + } + CLOSEST = [{"number": 183, "title": "Harden author-run reporting", "state": "open"}] + + def _good_report(self, *, sha_line=None): + lines = [ + "Created Gitea-Tools #189 — " + f"`{self.ISSUE_TITLE}`", + "Duplicate check: searched 12 open issues.", + "Closest existing: #183 — Harden author-run reporting " + "(update rejected — different scope).", + "Why new issue justified: continuation mode is distinct from #183.", + "Only mutation: issue creation (gitea.issue.create).", + "Confirm no labels, comments, PRs, reviews, merges, or closes.", + "## Controller Handoff", + "- Task: file issue", + "- Repo: Scaled-Tech-Consulting/Gitea-Tools", + "- Role: author", + "- Identity: prgs-author", + "- Issue/PR: #189", + "- Branch/SHA: n/a", + "- Files changed: none", + "- Validation: duplicate gate + create_issue capability resolved", + "- Mutations: create_issue via gitea.issue.create", + "- Workspace mutations: none", + "- Current status: issue created", + "- Blockers: none", + "- Next: implementation", + "- Safety: no review/merge", + "- Issue created or updated: created #189", + "- Related issues: #183, #188", + ] + if sha_line: + lines.insert(4, sha_line) + return "\n".join(lines) + + def test_complete_issue_filing_report_earns_a(self): + result = assess_issue_filing_final_report( + self._good_report(), + issue_number=189, + issue_title=self.ISSUE_TITLE, + action="created", + mutations=["create_issue"], + resolved_capabilities=self.CAPABILITIES, + issues_searched=12, + closest_matches=self.CLOSEST, + performed_mutations=["create_issue"], + ) + self.assertEqual(result["grade"], "A") + self.assertFalse(result["downgraded"]) + + def test_missing_controller_handoff_downgrades(self): + report = self._good_report().replace("## Controller Handoff", "") + result = assess_issue_filing_final_report( + report, + issue_number=189, + issue_title=self.ISSUE_TITLE, + mutations=["create_issue"], + resolved_capabilities=self.CAPABILITIES, + issues_searched=12, + performed_mutations=["create_issue"], + ) + self.assertTrue(result["downgraded"]) + + def test_abbreviated_sha_downgrades(self): + result = assess_issue_filing_sha_evidence( + f"old head: {self.FULL_SHA[:7]}" + ) + self.assertTrue(result["downgraded"]) + + def test_mutation_without_capability_proof_downgrades(self): + report = self._good_report().replace("gitea.issue.create", "allowed") + result = assess_issue_filing_mutation_capability( + report, + ["create_issue"], + self.CAPABILITIES, + ) + self.assertTrue(result["downgraded"]) + + def test_label_mutation_without_label_proof_downgrades(self): + caps = { + "set_issue_labels": { + "requested_task": "set_issue_labels", + "required_operation_permission": "gitea.issue.comment", + "allowed_in_current_session": True, + }, + } + report = "\n".join([ + "Updated labels with gitea.issue.comment capability resolved.", + "Only mutations: set_issue_labels", + ]) + result = assess_issue_filing_mutation_capability( + report, ["set_issue_labels"], caps + ) + self.assertTrue(result["downgraded"]) + + if __name__ == "__main__": unittest.main() -- 2.43.7 From 24d88914243b590da7cb9c787a1e4f6335b8be4f Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Mon, 6 Jul 2026 14:36:18 -0400 Subject: [PATCH 2/5] fix(issue-filing): enforce mutation-scope denials in generic loop (#191) The forbidden-mutation loop in assess_issue_filing_mutation_scope never appended reasons; align aggregation with issue-selection handoff checks. Co-Authored-By: Claude Opus 4.8 (1M context) --- review_proofs.py | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/review_proofs.py b/review_proofs.py index 5df0ef8..c539a0b 100644 --- a/review_proofs.py +++ b/review_proofs.py @@ -2282,18 +2282,22 @@ def assess_issue_filing_mutation_scope( "comment_issue", "create_issue", ): continue - if absent in text and f"no {absent}" not in text: - if any( - phrase in text - for phrase in ( - f"no {absent}", - f"no {absent}s", - f"without {absent}", - "none performed", - "none.", - ) - ): - continue + if absent not in text: + continue + denial_phrases = ( + f"no {absent}", + f"no {absent}s", + f"without {absent}", + "none performed", + "none.", + "confirm no", + ) + if any(phrase in text for phrase in denial_phrases): + continue + reasons.append( + f"single-mutation run mentions '{absent}' without " + f"confirming it was not performed" + ) if performed == ["create_issue"]: for check in ("label", "comment", "pr", "review", "merge"): if check in text and f"no {check}" not in text: @@ -2424,7 +2428,8 @@ def assess_issue_filing_final_report( reasons = [] downgraded = False for name, result in checks.items(): - if result.get("verdict") == "missing": + verdict = result.get("verdict") + if verdict in ("missing", "incomplete"): downgraded = True reasons.extend(result.get("reasons") or []) elif result.get("downgraded") or not result.get("complete", True): -- 2.43.7 From f464716e89c5bcdc957cedc8a42a16b92e8e247a Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Mon, 6 Jul 2026 14:42:22 -0400 Subject: [PATCH 3/5] docs: document agent temp artifact cleanup and gitignore patterns (#261) Add runbook guidance for deleting throwaway _encode/_emit/_inline commit helpers after MCP commit attempts, ignore those patterns in .gitignore, and cross-link the worktree-cleanup template. Closes #261. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 4 ++++ docs/llm-workflow-runbooks.md | 20 +++++++++++++++++++ .../templates/worktree-cleanup.md | 2 ++ 3 files changed, 26 insertions(+) diff --git a/.gitignore b/.gitignore index e6fe104..be618c5 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,7 @@ gitea-mcp*.json .vscode/ graphify-out/ branches/ +# Throwaway agent commit helpers (delete after MCP commit; see docs/llm-workflow-runbooks.md) +/_encode_*.py +/_emit_*.py +/_inline_*.py diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index 1b04cb3..cb19db2 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -370,6 +370,26 @@ 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 `). + +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). diff --git a/skills/llm-project-workflow/templates/worktree-cleanup.md b/skills/llm-project-workflow/templates/worktree-cleanup.md index a9ffa95..9d6a178 100644 --- a/skills/llm-project-workflow/templates/worktree-cleanup.md +++ b/skills/llm-project-workflow/templates/worktree-cleanup.md @@ -20,6 +20,8 @@ Steps: (removes branches/-issue--; 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 --prune; git checkout master; git reset --hard /master ONLY if local master safely matches remote. 7. Confirm main checkout clean and current (git status; 0 0 vs /master). -- 2.43.7 From 850b7c34b681d13d7b4836c7f559ec1288bddee5 Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Mon, 6 Jul 2026 14:42:33 -0400 Subject: [PATCH 4/5] 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) --- .gitignore | 2 +- agent_temp_artifacts.py | 26 ++++++ docs/llm-workflow-runbooks.md | 44 +++++----- gitea_mcp_server.py | 22 ++++- .../templates/worktree-cleanup.md | 2 - tests/test_agent_temp_artifacts.py | 87 +++++++++++++++++++ 6 files changed, 159 insertions(+), 24 deletions(-) create mode 100644 agent_temp_artifacts.py create mode 100644 tests/test_agent_temp_artifacts.py diff --git a/.gitignore b/.gitignore index be618c5..9fd03b9 100644 --- a/.gitignore +++ b/.gitignore @@ -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 diff --git a/agent_temp_artifacts.py b/agent_temp_artifacts.py new file mode 100644 index 0000000..5020521 --- /dev/null +++ b/agent_temp_artifacts.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) \ No newline at end of file diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index cb19db2..25a11a9 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -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 `). - -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). diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index dd1480c..037a844 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -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() diff --git a/skills/llm-project-workflow/templates/worktree-cleanup.md b/skills/llm-project-workflow/templates/worktree-cleanup.md index 9d6a178..a9ffa95 100644 --- a/skills/llm-project-workflow/templates/worktree-cleanup.md +++ b/skills/llm-project-workflow/templates/worktree-cleanup.md @@ -20,8 +20,6 @@ Steps: (removes branches/-issue--; 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 --prune; git checkout master; git reset --hard /master ONLY if local master safely matches remote. 7. Confirm main checkout clean and current (git status; 0 0 vs /master). diff --git a/tests/test_agent_temp_artifacts.py b/tests/test_agent_temp_artifacts.py new file mode 100644 index 0000000..2e14385 --- /dev/null +++ b/tests/test_agent_temp_artifacts.py @@ -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() \ No newline at end of file -- 2.43.7 From 2a544b78d1371925761d16f1c451bb3b2984470e Mon Sep 17 00:00:00 2001 From: Jason Walker <913443@dadeschools.net> Date: Mon, 6 Jul 2026 14:42:59 -0400 Subject: [PATCH 5/5] feat(mcp): support native commit payload preparation without shell (closes #263) --- gitea_mcp_server.py | 111 ++++++++++++++- tests/test_commit_payloads.py | 251 ++++++++++++++++++++++++++++++++++ 2 files changed, 358 insertions(+), 4 deletions(-) create mode 100644 tests/test_commit_payloads.py diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 60e65fb..a1ef73f 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -2275,6 +2275,106 @@ def gitea_get_file( } +def _prepare_commit_payload_files(files: list[dict]) -> tuple[list[dict], list[dict]]: + import base64 + import json + import os + + processed_files = [] + source_proofs = [] + + lock_data = {} + if os.path.exists(ISSUE_LOCK_FILE): + try: + with open(ISSUE_LOCK_FILE, "r", encoding="utf-8") as f: + lock_data = json.load(f) + except Exception: + pass + + locked_worktree = lock_data.get("worktree_path") + if locked_worktree: + locked_worktree = os.path.realpath(locked_worktree) + + for f in files: + f_copy = dict(f) + path = f_copy.get("path", "") + + content_keys = [k for k in ["content", "content_plain", "workspace_path", "local_path"] if k in f_copy] + if len(content_keys) > 1: + raise ValueError( + f"Multiple content sources specified for file '{path}'; provide exactly one of " + f"'content', 'content_plain', 'workspace_path', or 'local_path' (fail closed)" + ) + + source = "none" + if "content_plain" in f_copy: + content_plain = f_copy.pop("content_plain") + if content_plain is not None: + content_bytes = content_plain.encode("utf-8") + f_copy["content"] = base64.b64encode(content_bytes).decode("utf-8") + source = "inline_plain" + elif "workspace_path" in f_copy: + workspace_path = f_copy.pop("workspace_path") + if not locked_worktree: + raise RuntimeError( + f"Issue lock is missing or does not define a worktree path. " + f"Cannot resolve workspace_path '{workspace_path}' (fail closed)." + ) + target_path = os.path.realpath(os.path.abspath(os.path.join(locked_worktree, workspace_path))) + + is_inside = target_path == locked_worktree or target_path.startswith(os.path.join(locked_worktree, "")) + is_tmp = target_path.startswith(os.path.realpath("/tmp") + os.sep) or target_path.startswith("/tmp" + os.sep) + + if not (is_inside or is_tmp): + raise ValueError( + f"workspace_path '{workspace_path}' resolves to '{target_path}' which falls outside " + f"of locked worktree '{locked_worktree}' (fail closed)" + ) + + if not os.path.exists(target_path): + raise FileNotFoundError(f"File not found: {target_path} (fail closed)") + + with open(target_path, "rb") as fh: + file_bytes = fh.read() + f_copy["content"] = base64.b64encode(file_bytes).decode("utf-8") + source = "workspace_path" + elif "local_path" in f_copy: + local_path = f_copy.pop("local_path") + if not locked_worktree: + raise RuntimeError( + f"Issue lock is missing or does not define a worktree path. " + f"Cannot resolve local_path '{local_path}' (fail closed)." + ) + target_path = os.path.realpath(os.path.abspath(local_path)) + + is_inside = target_path == locked_worktree or target_path.startswith(os.path.join(locked_worktree, "")) + is_tmp = target_path.startswith(os.path.realpath("/tmp") + os.sep) or target_path.startswith("/tmp" + os.sep) + + if not (is_inside or is_tmp): + raise ValueError( + f"local_path '{local_path}' resolves to '{target_path}' which falls outside " + f"of locked worktree '{locked_worktree}' (fail closed)" + ) + + if not os.path.exists(target_path): + raise FileNotFoundError(f"File not found: {target_path} (fail closed)") + + with open(target_path, "rb") as fh: + file_bytes = fh.read() + f_copy["content"] = base64.b64encode(file_bytes).decode("utf-8") + source = "local_path" + elif "content" in f_copy: + source = "inline_base64" + + processed_files.append(f_copy) + source_proofs.append({ + "path": path, + "source": source + }) + + return processed_files, source_proofs + + @mcp.tool() def gitea_commit_files( files: list[dict], @@ -2289,7 +2389,7 @@ def gitea_commit_files( """Commit changes to multiple files in a Gitea repository in a single atomic commit. Args: - files: List of file operations. Each file dict must contain 'operation' ('create', 'update', 'delete', 'rename'), 'path', and 'content' (base64 encoded for create/update), and optionally 'sha' (required for update/delete) or 'from_path' (for rename). + files: List of file operations. Each file dict must contain 'operation' ('create', 'update', 'delete', 'rename'), 'path', and one of content payload sources: 'content' (base64), 'content_plain', 'workspace_path', or 'local_path'. message: The commit message. branch: Optional existing branch to start/commit from. new_branch: Optional new branch name to create for this commit. @@ -2325,12 +2425,14 @@ def gitea_commit_files( return blocked verify_preflight_purity(remote) + processed_files, source_proofs = _prepare_commit_payload_files(files) + h, o, r = _resolve(remote, host, org, repo) auth = _auth(h) url = f"{repo_api_url(h, o, r)}/contents" payload = { - "files": files, + "files": processed_files, "message": message, } if branch is not None: @@ -2341,13 +2443,14 @@ def gitea_commit_files( with _audited("commit_files", host=h, remote=remote, org=o, repo=r, target_branch=(new_branch or branch), request_metadata={"message": message, - "paths": [f.get("path") for f in files], - "operations": [f.get("operation") for f in files]}): + "paths": [f.get("path") for f in processed_files], + "operations": [f.get("operation") for f in processed_files]}): data = api_request("POST", url, auth, payload) return { "success": True, "commit": data.get("commit", {}).get("sha", ""), "branch": data.get("branch", {}).get("name", ""), + "content_source_proof": source_proofs, } diff --git a/tests/test_commit_payloads.py b/tests/test_commit_payloads.py new file mode 100644 index 0000000..1b3a80d --- /dev/null +++ b/tests/test_commit_payloads.py @@ -0,0 +1,251 @@ +import json +import os +import tempfile +import unittest +from unittest.mock import MagicMock, patch + +import gitea_config +import mcp_server +import task_capability_map + +CONFIG = { + "version": 2, + "contexts": { + "ctx": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.example.com"}, + } + }, + "profiles": { + "full-author": { + "enabled": True, + "context": "ctx", + "role": "author", + "username": "author-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, + "allowed_operations": [ + "gitea.read", + "gitea.repo.commit", + "gitea.pr.create", + "gitea.branch.push", + ], + "forbidden_operations": [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.review", + ], + "execution_profile": "full-author", + }, + }, +} + + +class TestCommitPayloads(unittest.TestCase): + def setUp(self): + self._remotes = patch.dict(mcp_server.REMOTES, { + "prgs": {"host": "gitea.example.com", "org": "Example-Org", + "repo": "Example-Repo"}, + }) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + + # Setup temp directories and lock files + self._dir = tempfile.TemporaryDirectory() + self.config_path = os.path.join(self._dir.name, "profiles.json") + with open(self.config_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(CONFIG)) + + self.locked_worktree_dir = tempfile.TemporaryDirectory() + self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name) + + self.lock_file_path = "/tmp/gitea_issue_lock.json" + self.lock_data = { + "issue_number": 263, + "branch_name": "feat/issue-263-native-commit-payloads", + "remote": "prgs", + "org": "Example-Org", + "repo": "Example-Repo", + "worktree_path": self.locked_worktree_path, + } + with open(self.lock_file_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(self.lock_data)) + + # Reset preflight status to bypass/pass verification in tests + self.orig_whoami_called = mcp_server._preflight_whoami_called + self.orig_capability_called = mcp_server._preflight_capability_called + mcp_server._preflight_whoami_called = True + mcp_server._preflight_capability_called = True + + def tearDown(self): + self._remotes.stop() + mcp_server._IDENTITY_CACHE.clear() + + mcp_server._preflight_whoami_called = self.orig_whoami_called + mcp_server._preflight_capability_called = self.orig_capability_called + + self._dir.cleanup() + self.locked_worktree_dir.cleanup() + if os.path.exists(self.lock_file_path): + os.remove(self.lock_file_path) + + def _env(self, profile: str) -> dict: + return { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": profile, + "GITEA_TOKEN_AUTHOR": "author-pass", + "GITEA_TEST_PORCELAIN": "", + } + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_content_plain(self, _auth, mock_api): + mock_api.return_value = { + "commit": {"sha": "sha-123"}, + "branch": {"name": "feat/issue-263-native-commit-payloads"} + } + with patch.dict(os.environ, self._env("full-author"), clear=True): + res = mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "hello.txt", + "content_plain": "Hello World!", + } + ], + message="Add hello.txt", + remote="prgs", + ) + print("DEBUG RES IS:", res) + self.assertTrue(res["success"]) + self.assertEqual(res["commit"], "sha-123") + self.assertEqual(res["content_source_proof"][0]["source"], "inline_plain") + + # Verify posted base64 content + post_args = mock_api.call_args[0] + payload = post_args[3] + self.assertEqual(payload["files"][0]["content"], "SGVsbG8gV29ybGQh") + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_workspace_path(self, _auth, mock_api): + mock_api.return_value = { + "commit": {"sha": "sha-456"}, + "branch": {"name": "feat/issue-263-native-commit-payloads"} + } + + # Create a workspace file + file_relative = "src/code.py" + file_abs = os.path.join(self.locked_worktree_path, file_relative) + os.makedirs(os.path.dirname(file_abs), exist_ok=True) + with open(file_abs, "wb") as fh: + fh.write(b"print('hello')") + + with patch.dict(os.environ, self._env("full-author"), clear=True): + res = mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "src/code.py", + "workspace_path": file_relative, + } + ], + message="Add code.py", + remote="prgs", + ) + self.assertTrue(res["success"]) + self.assertEqual(res["content_source_proof"][0]["source"], "workspace_path") + + post_args = mock_api.call_args[0] + payload = post_args[3] + # "print('hello')" in base64 is cHJpbnQoJ2hlbGxvJyk= + self.assertEqual(payload["files"][0]["content"], "cHJpbnQoJ2hlbGxvJyk=") + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_local_path(self, _auth, mock_api): + mock_api.return_value = { + "commit": {"sha": "sha-789"}, + "branch": {"name": "feat/issue-263-native-commit-payloads"} + } + + file_abs = os.path.join(self.locked_worktree_path, "local.bin") + with open(file_abs, "wb") as fh: + fh.write(b"\x00\x01\x02\x03\xff") + + with patch.dict(os.environ, self._env("full-author"), clear=True): + res = mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "local.bin", + "local_path": file_abs, + } + ], + message="Add local.bin", + remote="prgs", + ) + self.assertTrue(res["success"]) + self.assertEqual(res["content_source_proof"][0]["source"], "local_path") + + post_args = mock_api.call_args[0] + payload = post_args[3] + # b"\x00\x01\x02\x03\xff" in base64 is AAECA/8= + self.assertEqual(payload["files"][0]["content"], "AAECA/8=") + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_traversal_blocked(self, _auth, mock_api): + # Remove active lock file to ensure it fails on traversal/invalid locks + os.remove(self.lock_file_path) + + with patch.dict(os.environ, self._env("full-author"), clear=True): + with self.assertRaises(RuntimeError) as ctx: + mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "hack.txt", + "workspace_path": "any.txt", + } + ], + message="Add hack", + remote="prgs", + ) + self.assertIn("Issue lock is missing", str(ctx.exception)) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_outside_scope_blocked(self, _auth, mock_api): + with patch.dict(os.environ, self._env("full-author"), clear=True): + with self.assertRaises(ValueError) as ctx: + mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "hack.txt", + "workspace_path": "../outside.txt", + } + ], + message="Add hack", + remote="prgs", + ) + self.assertIn("falls outside of locked worktree", str(ctx.exception)) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_multiple_sources_blocked(self, _auth, mock_api): + with patch.dict(os.environ, self._env("full-author"), clear=True): + with self.assertRaises(ValueError) as ctx: + mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "hello.txt", + "content_plain": "Hello!", + "content": "SGVsbG8=", + } + ], + message="Add hello", + remote="prgs", + ) + self.assertIn("Multiple content sources specified", str(ctx.exception)) -- 2.43.7