diff --git a/.claude/agent-memory/orchestrator/MEMORY.md b/.claude/agent-memory/orchestrator/MEMORY.md index 2943fca8e..229719e3c 100644 --- a/.claude/agent-memory/orchestrator/MEMORY.md +++ b/.claude/agent-memory/orchestrator/MEMORY.md @@ -1,7 +1,7 @@ - [CLAUDE.md nullable command != CI gate — RESOLVED by #540](project_claudemd_nullable_command_diverges_from_ci.md) — docs now match ci.yml; a reappearance of `/p:Nullable=enable` or `/t:Build` is a regression. Carries the 195-error UtilitiesCS lower bound for #492 - [PoshQC test drops coverage.xml at repo root](poshqc-test-drops-coverage-xml-at-repo-root.md) — untracked, not gitignored/csharpierignored; inflates the CSharpier file count and leaks into `git add -A` - [Agent-worktree discovery + evidence hygiene](project_agent_worktree_discovery_and_evidence_hygiene.md) — `\.claude\` test-glob filter must use the RELATIVE path; never commit raw Cobertura -- [Completion-gate receipt shapes](completion-gate-receipt-shapes.md) — exact fields require_complete wants: delegation_receipts as a LIST, skill_receipts required:true +- [Completion-gate receipt shapes](completion-gate-receipt-shapes.md) — SOLVED: the missing key is `evidence`; plus the bug-route `new_potential_bug_entry` swap that MCP and the hook disagree on - [JaCoCo not Cobertura for coverage evidence](jacoco-not-cobertura-for-evidence.md) — maintainer deletes committed Cobertura; convert to package-level JaCoCo before pushing - [Store-lockup watchdog null-model hazard](project_store_lockup_watchdog_null_model_hazard.md) — new startup COM scopes need a phase-branch returning before the disable-service write - [VS Code extension location](project_extension_location.md) — the extension lives at `extensions/drm-copilot/`, not the repo root 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 3821e1b2e..9f2cbb06e 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 @@ -1,20 +1,25 @@ --- name: collect-pr-context-lands-in-main-checkout -description: In a worktree, collect_pr_context writes pr_context.* to the MAIN checkout, but the enforce-pr-author hook reads it from the worktree CWD — copy it in before gh pr create +description: From an isolated agent worktree, collect_pr_context returns ok:true but writes to the PRIMARY checkout and claims gh is unavailable - author the PR body from the real diff instead metadata: type: project --- -When the orchestrator runs in a git worktree, `mcp__drm-copilot__collect_pr_context` reports writing `artifacts/pr_context.summary.txt` to the worktree but it actually lands in the MAIN checkout (e.g. `C:/Users/.../repos/TaskMaster/artifacts/pr_context.summary.txt`), not the worktree's `artifacts/`. +Calling `mcp__drm-copilot__collect_pr_context` from a `.claude/worktrees/` worktree returns +`ok:true` and lists artifact paths **inside that worktree**, but the files it actually writes land in +the PRIMARY checkout (`C:\Users\DanMoisan\repos\TaskMaster\artifacts\`). The `workspace_root` +argument does not redirect it. Confirmed again 2026-08-22 (epic child #445): the returned paths had +an mtime ~10 minutes older than the call, while the primary checkout's copy was freshly written. -**Why:** `.claude/hooks/enforce-pr-author-skill.ps1` reads `artifacts/pr_context.summary.txt` relative to the CWD of the `gh pr create` command (the worktree). If it is missing there, the hook blocks with the "context artifact absent" (Case C) reason, and the receipt staleness check has nothing to compare against. +Two further defects make the artifact unusable rather than merely misplaced: -**How to apply (in-thread pr-author flow, since Agent(pr-author) is unavailable — see [[pr-author-hook-blocks-gh-in-this-repo]]):** -1. Run `collect_pr_context --base `; then `cp` the main-checkout `artifacts/pr_context.summary.txt` (and `.appendix.txt`) into the worktree `artifacts/`. -2. Write `artifacts/pr_body_.md`; compute lowercase-hex SHA-256 of its bytes; write `artifacts/pr_body_.receipt.json` with `created_at` STRICTLY NEWER than the (copied) summary's LastWriteTimeUtc — copy the summary FIRST, then sleep, then stamp the receipt. -3. `gh pr create --base --body-file artifacts/pr_body_.md`. +1. **It claims `gh` is unavailable** (`GitHub CLI unavailable: ... not installed`) when `gh auth + status` in the same worktree authenticates fine. Never accept that claim; verify `gh` yourself. +2. **The primary checkout is on a different branch**, so the diff it computes is not your branch's + diff at all. Copying it into the worktree would import a wrong changed-file list. -Also: `artifacts/orchestration/orchestrator-state.json` and `artifacts/pr_body_*`/`artifacts/pr_context.*` are all GITIGNORED in this repo — the checkpoint is local on-disk state (which is exactly what the hooks read); do not expect it in commits/PRs, and a "clean" `git status` does not mean the checkpoint is committed. +**Why:** a PR body built from that artifact misstates the change. In #445 the stale worktree copy +recorded a head SHA one commit behind and omitted all three review artifacts. **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]]). @@ -25,3 +30,18 @@ Also: `artifacts/orchestration/orchestrator-state.json` and `artifacts/pr_body_* **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. + +**Independent confirmation and the simplest safe remedy (#445, 2026-08-22).** Same run, same wave: +`ok:true`, worktree paths returned, nothing written there, primary checkout freshly written. The +worktree copy I would have used was a decoy the feature-review subagent had hand-authored (quirk (a) +above), recording a head SHA one commit behind and omitting all three review artifacts. + +Rather than copy-then-verify-ownership, the cheaper remedy is **do not copy at all**. Treat +`collect_pr_context` purely as a receipt formality, then REGENERATE `artifacts/pr_context.summary.txt` +yourself in your own worktree from `git rev-parse HEAD`, `git merge-base`, `git log --oneline +..HEAD`, and `git diff --numstat ...HEAD`. This is strictly safer than the ownership +check, because a file you wrote from your own git state cannot be a sibling's, and it simultaneously +satisfies the pr-author receipt's `created_at > summary mtime` check as long as you write the receipt +afterwards. `pr_context.*` is gitignored, so regenerating dirties nothing. See +[[pr-context-summary-unreliable-gh-and-classification]] and +[[pr-author-hook-blocks-gh-in-this-repo]]. diff --git a/.claude/agent-memory/orchestrator/completion-gate-receipt-shapes.md b/.claude/agent-memory/orchestrator/completion-gate-receipt-shapes.md index 6540dfa7a..8ac6618a1 100644 --- a/.claude/agent-memory/orchestrator/completion-gate-receipt-shapes.md +++ b/.claude/agent-memory/orchestrator/completion-gate-receipt-shapes.md @@ -1,39 +1,46 @@ --- name: completion-gate-receipt-shapes -description: Exact field shapes the MCP orchestrator-state completion gate requires for delegation, skill, and MCP receipts — guessing the key names wastes several validate cycles +description: The exact receipt shape the routing-contract completion gate wants - the missing key was always `evidence`; also the bug-promotion tool-name swap and the unresolvable MCP-vs-hook divergence it creates metadata: type: project --- -The MCP `validate_orchestration_artifacts` completion gate (`require_complete: true`) rejects -plausible-looking receipt shapes. The authoritative source is -`extensions/drm-copilot/src/lib/validate/orchestrator-state-routing.ts` in the drm-copilot repo -(functions `receiptSkills`, `mcpTools`, `receiptAgents`). Read it instead of guessing. - -**Why:** On #508 I burned three validate cycles guessing `skill_name`/`tool_name`/`mcp_receipts`. -The gate silently ignores a receipt that is missing any required field — it does not say *which* -field is wrong, only that the whole receipt is "missing". - -**How to apply:** Use these exact shapes. - -- `delegation_receipts` must be a **list** (not an object namespaced by phase). Each entry needs: - `agent_name`, `step`, `agent_id`, `skill_source`, `started_at`, `completed_at`, `result_signal`, - `artifact_paths`. If you also need promotion receipts, put them under a *separate* top-level key - (for example `promotion_receipts`) — the list form is what supplies the delegated-agent set for - the model-routing gate, so it cannot also be an object. -- `skill_receipts[]` needs exactly `{ skill: , required: true, evidence: }`. - `required` must be the boolean `true`; a missing `required` silently drops the skill. -- MCP receipts live under **`mcp_call_receipts`**, not `mcp_receipts`, and need - `{ tool: , ok: true, evidence: }`. `tool` must match the canonical - name in `required_mcp_tools` — if you invoked a variant (for example `new_potential_bug_entry` - for the `new_potential_entry` requirement), put the canonical name in `tool` and disclose the - actual variant inside `evidence`. -- `ci_gate` needs `verified_at` in addition to `conclusion`. -- `local_execution_overrides` and `delegation_bypasses` must both be present and be **empty lists**. - -Also required earlier, at PR-creation time: `relativeFile`, `long-name`, and `work-mode` (hyphenated) -as flat top-level keys, and steps 5-8 all non-pending. See [[orchestrator-state-flat-keys-and-enum]]. - -TaskMaster has no Python validator, so the PR-author hook uses the portable PowerShell path -(`Test-OrchestratorStatePrCreationReadiness` in `.claude/lib/orchestrator-state/OrchestratorState.psm1`). -Run it directly to preflight before `gh pr create` — it tells you exactly which step is pending. +**SOLVED 2026-08-22 (epic child #445).** The shape that earlier runs could not discover by guessing is +readable in `.claude/lib/orchestrator-state/OrchestratorStateRoutingContract.psm1` +(`Get-CheckpointAcknowledgedName`, ~line 225). Read it; do not guess. + +A receipt counts **only when all three hold together**: + +- `skill_receipts[]` — `skill` non-blank string, `required` **boolean** `true`, **`evidence` non-blank string**. +- `mcp_call_receipts[]` — `tool` non-blank string, `ok` **boolean** `true`, **`evidence` non-blank string**. + +**`evidence` is the key that was always missing.** Earlier attempts tried `outcome`, `detail`, +`skills_used`, `mcp_receipts`, `mcp_tool_calls` and failed, because the harvest reads a +hard-coded `'evidence'` member regardless of the array. Truthy-but-not-`$true` flags +(`1`, `"true"`) deliberately do not count. + +`delegation_receipts[]` is separate and needs `agent_name`, `agent_id`, `step`, `phase`, +`skill_source`, `result_signal`, `artifact_paths` (list). It is a LIST, not an object. + +### Bug-promotion tool-name swap, and the divergence it creates + +`Get-ResolvedRequiredMcpTool` swaps `new_potential_entry` for **`new_potential_bug_entry`** when the +checkpoint's promotion type is `bug`. The MCP TypeScript surface does **not** apply that swap. +Because `required_mcp_tools` is checked by EXACT LIST EQUALITY, the two demands are mutually +exclusive on a bug route and **no single list satisfies both**. + +**Conform to the HOOK** (`new_potential_bug_entry`). It is the mechanism that actually blocks, and it +is semantically right for a bug. Expect the MCP `require_complete` call to keep reporting exactly two +residual errors; they are divergence artifacts, not missing work. + +**Why:** this supersedes the pessimistic conclusion in +[[orchestrator-state-validator-divergence]] that `--require-complete` is simply unsatisfiable for a +resumed-at-execution epic child. It is satisfiable *at the real gate* +(`.claude/hooks/validate-orchestrator-output.ps1` returned `EXIT_CODE=0`); only the MCP surface stays +unsatisfiable, and only on the two swap-related errors. + +**How to apply:** when the completion gate reports missing skill or MCP receipts, add `evidence` to +every receipt before changing anything else. Prove the result with the hook, invoking it as +`$env:CLAUDE_HOOK_INPUT = '{"output":""}'` (top-level `.output`) and asserting `EXIT_CODE=0` +— not with the MCP tool. See [[orchestrator-state-flat-keys-and-enum]] and +[[epic-child-self-merge-step9-passed-vs-verified]].