From 17a11eb868c387b1cf34d594723ce4c003d9934d Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Sat, 22 Aug 2026 11:52:32 -0400 Subject: [PATCH] chore(agent-memory): record collect_pr_context cross-child contamination (#589) During epic child #449's PR authoring, mcp__drm-copilot__collect_pr_context returned ok:true with worktree artifact paths, wrote nothing there, and wrote instead into the main checkout -- a location shared by every concurrently running epic child. Sibling child #491 overwrote the file in the interval between collection and use, so the copied context described #491's branch, commits, and evidence folder. The contamination was caught by comparing the summary's resolved head SHA against HEAD before authoring. Presence and mtime checks both pass on a sibling's file, so ownership verification is the only reliable guard. Also records two further defects confirmed in the same bundle: a false "GitHub CLI is not installed" report while gh was working in the same session, and an author-asserted autoclose list containing AC-1..AC-16 scraped as issue numbers alongside issues not owned by the child. The tool defect itself is tracked as issue #589. Refs #449, #589 Co-Authored-By: Claude Opus 5 (1M context) --- .../collect-pr-context-lands-in-main-checkout.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.claude/agent-memory/orchestrator/collect-pr-context-lands-in-main-checkout.md b/.claude/agent-memory/orchestrator/collect-pr-context-lands-in-main-checkout.md index 6aa6570aa..3821e1b2e 100644 --- a/.claude/agent-memory/orchestrator/collect-pr-context-lands-in-main-checkout.md +++ b/.claude/agent-memory/orchestrator/collect-pr-context-lands-in-main-checkout.md @@ -19,3 +19,9 @@ Also: `artifacts/orchestration/orchestrator-state.json` and `artifacts/pr_body_* **Refinement (#327, 2026-07-16, dedicated agent worktree):** when the child orchestrator's session cwd IS the feature worktree (the harness gave me an isolated `.claude/worktrees/agent-` worktree and I `git switch -c` the feature branch there), collect_pr_context wrote `pr_context.*` DIRECTLY into that worktree's `artifacts/` (returned paths were the worktree) and the hook read them there — no main-checkout copy step was needed. The main-checkout-landing behavior above applies when session cwd differs from the feature worktree. TWO quirks still bit: (a) collect_pr_context reported a `Head:` SHA one commit BEHIND my true branch tip and a second call did NOT rewrite the file (identical mtime) — do not trust the summary's `Head:` line; the pushed branch tip and GitHub's own base...head diff are authoritative, so it is harmless. (b) The receipt `created_at > summary mtime` check still held because I wrote the receipt after collect ran; no sleep was needed since minutes had elapsed. Child->integration PR #334 merged fine (merge commit 9559c73c) on blocking_count==0 with zero CI ([[project_epic_child_prs_no_ci]]). **Quirk (a) is stronger than "does not rewrite on a second call" (#441, 2026-08-10).** `collect_pr_context` returned `ok:true` with worktree paths, but the on-disk `artifacts/pr_context.summary.txt` was untouched — mtime and content still belonged to a file the feature-review subagent had HAND-AUTHORED an hour earlier, whose own first line read "collect_pr_context MCP tool unavailable in this session" and whose `Base:` was the old pre-change baseline rather than the `--base` I passed. So the tool will report success over a pre-existing file it did not write. Consequences: (1) never read the summary to learn the diff — author the PR body from `git diff ..HEAD --stat` and `git log ..HEAD` ([[pr-context-summary-unreliable-gh-and-classification]]); (2) the receipt's `created_at > summary mtime` check gets *easier*, not harder, because the stale mtime is older — but verify it rather than assuming; (3) `feature-review` also lacks `collect_pr_context`, so a reviewer may leave a hand-authored decoy in `artifacts/` that survives your own collect call. + +**CROSS-CHILD CONTAMINATION (#449, 2026-08-22) — the most dangerous variant, promoted to issue #589.** In an isolated agent worktree the tool returned `ok:true` with worktree paths, wrote NOTHING there, and wrote to the MAIN checkout instead — a location SHARED by every concurrently running epic/parallel child. Sibling child #491 ran its own collect in the gap between my collect and my copy, so the file I copied was ENTIRELY #491's: `Head ref (resolved): bug/quickfiler-test-form1-live-form-491-exec @ bec83397`, and its `Additional context files` listed 14 artifacts from #491's feature folder. Authoring from it would have described the wrong change on my PR, with `ok:true` and a well-formed file as the only signals. + +**How to apply — verify OWNERSHIP, not just presence.** Presence checks and mtime checks both pass on a sibling's file. After copying, assert the summary's `Head ref (resolved)` SHA equals your own `git rev-parse HEAD`, and sanity-check that the feature-folder references in the file are YOURS (I counted 96 for my folder and 0 for #491's). Re-run and re-copy if it does not match. Then author the body from the real `git diff ..HEAD` regardless — the file is only safe as a file LIST, never as a narrative. + +Two more defects confirmed in the same bundle: the summary reported "GitHub CLI (gh) is not installed" while `gh auth status` and `gh issue view` both worked in the same session; and the `author asserted` autoclose list contained `#AC-1`..`#AC-16` (acceptance-criterion IDs scraped as issue numbers) plus three issues that were not mine to close. Never emit `Closes` from that list. Note also that a child PR into an epic integration branch cannot auto-close anything — GitHub only honors closing keywords merging into the DEFAULT branch — so `Refs #NNN` is the correct form and the epic's final integration-to-main PR carries the close.