Notify merge-queue when workers complete PRs

[dlorenc]

Summary

• Fixes issue Merge queue not always notified when workers complete PRs #16: Merge queue not always notified when workers complete PRs
• When workers call multiclaude agent complete, the daemon now notifies both supervisor AND merge-queue
• This enables the merge-queue to immediately check for new PRs instead of waiting for 2-minute polling intervals

Investigation Findings

The root cause was in internal/daemon/daemon.go:handleCompleteAgent:

• Only the supervisor was being notified when workers completed
• The merge-queue relied entirely on periodic wake messages (every 2 minutes) to discover new PRs
• This caused delays between PR creation and merge-queue processing

Changes

1. daemon.go: Modified handleCompleteAgent to send completion notifications to both supervisor and merge-queue
2. worker.md: Updated prompt to reflect that merge-queue is also notified
3. merge-queue.md: Added new "Worker Completion Notifications" section explaining the notification behavior

Test plan

• All existing tests pass (go test ./...)
• Manual test: Create a worker, complete it with multiclaude agent complete, verify merge-queue receives notification

🤖 Generated with Claude Code

[whitmo]

Review: PR #18 — pr-triage

Scope Discrepancy (Critical)

Claimed: "Merges 5 clean PRs from upstream (#342, #334, #336, #340, #335)" with "659 lines of new tests"

Actual: 99 commits, 91 files changed, 17,183 insertions, 5,535 deletions across the entire upstream history since the fork diverged.

Only 7 of the 99 commits (5 merge commits + the original 5 source commits + 1 test commit) relate to the claimed PRs. The other 92 commits include:

• Full CLI restructuring (noun-group refactors for repo, worker, agent, message)
• Fork-aware workflow implementation (#267, #274, #275, #276)
• Agent definitions infrastructure (#236, #237, #238, #239, #240)
• Configurable agents feature (#247)
• Task management support (#313)
• CI pipeline changes (golangci-lint, gofmt checks)
• Repo lifecycle / hibernate / status commands (#315, #316, #317)
• Dozens of test/refactoring commits

Verdict: The PR description is severely misleading. It represents a full upstream sync disguised as a focused 5-PR triage.

---

Test Results

• Passes: go build ./cmd/multiclaude ✅
• Passes: 24/25 test packages ✅
• Fails: pkg/tmux — 2 flaky timing-dependent tests (TestSendKeys, TestSendKeysLiteralWithEnter)
  • These appear to be pre-existing flaky tests, not introduced by this PR

---

Code Quality (Gemini + Claude Review)

Positives:

• periodicLoop helper — good DRY refactoring for daemon loops
• startAgentWithConfig — consolidates agent startup, reduces duplication
• socket.SuccessResponse / socket.ErrorResponse — cleaner response handling
• appendToSliceMap — eliminates repeated nil-check boilerplate
• Error message updates correctly reference new CLI noun groups (multiclaude repo init vs multiclaude init)
• New structured error constructors (RepoNotFound, RepoAlreadyExists, WorkspaceAlreadyExists, etc.)

Concerns:

1. Fragile agent type derivation — handleSpawnAgent infers AgentTypeReview from strings.Contains(agentName, "review"). Naming convention as type system is brittle.
2. Removed crash notification — The diff removes notifySupervisorOfCrash for workers/review agents. Transient agents are now fire-and-forget without supervisor notification. Impact unclear.
3. Removed selective wake — agentHasWork logic removed; all agents get woken uniformly. Could waste resources.
4. Removed branch/config cleanup — DeleteBranch and RemoveAll(agentConfigDir) were removed from agent cleanup. Risk of orphaned branches and config directories accumulating.
5. PID=0 in test mode — MULTICLAUDE_TEST_MODE=1 skips Claude startup, leaving PID at 0. Code paths checking isProcessAlive(agent.PID) may behave unexpectedly.

---

Security

No obvious security issues. No credential handling changes, no user input injection vectors observed in the key files.

---

Recommendation

Request Changes — This PR should not be merged as-is:

1. Split the PR — The upstream sync and the 5 targeted PRs should be separate operations. Either:
   • Rebase onto upstream main first (separate PR), then cherry-pick the 5 PRs
   • Or clearly label this as "full upstream sync" in the description
2. Fix the description — Currently misleading about scope
3. Address the 5 code concerns above (especially the removed crash notification and cleanup logic)

The underlying code changes are mostly good quality refactoring, but the review surface is unreasonably large for what's described as a 5-PR merge.

---

Review by swift-badger (Claude) + Gemini second opinion
🤖 Generated with Claude Code

[whitmo]

Review: PR #18 — pr-triage

Scope Discrepancy (Critical)

Claimed: Merges 5 clean PRs from upstream (#342, #334, #336, #340, #335) with 659 lines of new tests

Actual: 99 commits, 91 files changed, 17,183 insertions, 5,535 deletions across the entire upstream history since the fork diverged.

Only 7 of the 99 commits relate to the claimed PRs. The other 92 include full CLI restructuring, fork-aware workflow, agent definitions infrastructure, task management, CI pipeline changes, and dozens of test/refactoring commits.

Verdict: PR description is severely misleading. This is a full upstream sync disguised as a focused 5-PR triage.

---

Test Results

• Build: go build passes
• Tests: 24/25 packages pass
• Fails: pkg/tmux — 2 flaky timing tests (pre-existing, not from this PR)

---

Code Quality (Gemini + Claude Review)

Positives:

• periodicLoop helper — good DRY refactoring for daemon loops
• startAgentWithConfig — consolidates agent startup
• socket.SuccessResponse/ErrorResponse — cleaner response handling
• Error messages correctly reference new CLI noun groups
• New structured error constructors

Concerns:

1. Fragile agent type derivation — handleSpawnAgent infers AgentTypeReview from strings.Contains(agentName, "review")
2. Removed crash notification — notifySupervisorOfCrash removed for workers/review agents
3. Removed selective wake — agentHasWork logic removed; all agents woken uniformly
4. Removed branch/config cleanup — DeleteBranch and RemoveAll(agentConfigDir) removed from agent cleanup
5. PID=0 in test mode — MULTICLAUDE_TEST_MODE=1 leaves PID at 0

---

Recommendation: Request Changes

1. Split the PR — upstream sync and 5 targeted PRs should be separate
2. Fix the description — currently misleading about scope
3. Address the 5 code concerns above

The code changes are mostly good quality refactoring, but review surface is unreasonably large for what is described as a 5-PR merge.

Review by swift-badger (Claude) + Gemini second opinion