Add cross-agent messaging and concurrent phase execution

.egg-state/agent-outputs/1027-risk_analyst-output.json

@@ -0,0 +1,104 @@
+{
+  "issue": 1027,
+  "phase": "plan",
+  "agent": "risk_analyst",
+  "revision": 2,
+
+  "title": "Risk assessment: Cross-agent communication and concurrent phase execution (revision 2)",
+
+  "summary": "Updated risk assessment following plan review feedback and architect revision 2. The architect addressed all three blocking review items: (1) corrected orchestrator/app.py → api.py references, (2) added container failure behavior (LOG → NOTIFY → ESCALATE via HITL), (3) reordered Phase 1 tasks with dependency note. However, I found a residual issue: the plan YAML manifest still lists orchestrator/app.py in TASK-1-3 files (plan line 288), contradicting the prose and architect output. This must be fixed. The 12 risks from revision 1 remain valid. I add one new finding: the container failure escalation design (HITL with Retry/Continue/Abort) is well-specified but creates a dependency on HITL infrastructure being available during concurrent execution — which is a new Phase 2 prerequisite. Overall risk rating remains MEDIUM-HIGH. Recommendation: PROCEED WITH CAUTION after fixing the residual YAML path and the two prerequisite fixes (signal handler locking, worktree API integration).",
+
+  "review_feedback_validation": [
+    {
+      "feedback_id": 1,
+      "feedback": "BLOCKING — Wrong file path: TASK-1-3 lists orchestrator/app.py. The Flask application is defined in orchestrator/api.py.",
+      "architect_resolution": "Architect claims all references corrected to orchestrator/api.py. Verified: architect output (1027-architect-output.json) consistently uses api.py in files_inventory and architecture_design sections.",
+      "validation": "PARTIALLY RESOLVED. The architect output is correct, but the plan YAML (1027-plan.md line 288) still lists 'orchestrator/app.py' under TASK-1-3 files. Confirmed: orchestrator/app.py does NOT exist; the Flask app is defined in orchestrator/api.py. The plan YAML must be updated to match.",
+      "residual_action": "Fix TASK-1-3 files list in plan YAML: change orchestrator/app.py → orchestrator/api.py"
+    },
+    {
+      "feedback_id": 2,
+      "feedback": "BLOCKING — TASK-2-2 missing failure behavior: Must specify what ConcurrentPhaseExecutor does when a container fails.",
+      "architect_resolution": "Architect added container_failure_behavior to component_3_concurrent_executor: three-step escalation (LOG → NOTIFY → ESCALATE via HITL with Retry/Continue/Abort options). Phase continues with remaining agents while human decides.",
+      "validation": "RESOLVED. The failure behavior is well-specified and follows a reasonable pattern. The three HITL options (Retry, Continue without, Abort) cover the key scenarios. The design that remaining agents continue while awaiting human input is appropriate — it avoids blocking the entire phase on a single failure. One concern: this creates a dependency on HITL infrastructure being available during concurrent execution. The existing DecisionQueue (orchestrator/decision_queue.py) supports this, but the ConcurrentPhaseExecutor must integrate with it correctly.",
+      "residual_action": "None. But TASK-2-2 acceptance criteria should explicitly mention: 'Container failure triggers HITL decision creation via DecisionQueue; test with mocked HITL resolution.'"
+    },
+    {
+      "feedback_id": 3,
+      "feedback": "BLOCKING — Phase 1 task ordering: TASK-1-3 must come before TASK-1-2.",
+      "architect_resolution": "Architect reordered Phase 1 tasks in the prose description and added task_ordering_note: models+events+registration → message store → route handlers → types → client → CLI → tests.",
+      "validation": "PARTIALLY RESOLVED. The architect output correctly reorders tasks in the prose and adds the ordering note. However, the plan YAML (1027-plan.md) still lists tasks in the original order: TASK-1-1 (message_store) → TASK-1-2 (routes) → TASK-1-3 (models/events). The YAML ordering contradicts the architect's note. Since coder agents typically follow YAML task order, TASK-1-3 should be listed first in the YAML or the dependency should be explicitly stated in each task's description.",
+      "residual_action": "Either reorder YAML tasks to TASK-1-3 → TASK-1-1 → TASK-1-2 → TASK-1-4 → TASK-1-5 → TASK-1-6, or add explicit 'depends_on: TASK-1-3' to TASK-1-1 and TASK-1-2."
+    }
+  ],
+
+  "new_findings_since_revision_1": [
+    {
+      "id": "NF-1",
+      "title": "Plan YAML still contains orchestrator/app.py",
+      "description": "The plan file (1027-plan.md line 288) lists orchestrator/app.py in TASK-1-3 files. This file does not exist. The Flask app is in orchestrator/api.py. The architect output was corrected but the plan YAML was not updated to match.",
+      "severity": "blocking",
+      "action": "Update plan YAML TASK-1-3 files: orchestrator/app.py → orchestrator/api.py"
+    },
+    {
+      "id": "NF-2",
+      "title": "Plan YAML task ordering does not reflect architect's reordering",
+      "description": "The architect's revision 2 specifies task order: models+events+registration (TASK-1-3) first, then message store (TASK-1-1), then routes (TASK-1-2). The plan YAML still has TASK-1-1 → TASK-1-2 → TASK-1-3 ordering. Coder agents following the YAML sequentially will implement message_store.py before the Message model it depends on.",
+      "severity": "blocking",
+      "action": "Reorder YAML or add explicit dependency annotations"
+    },
+    {
+      "id": "NF-3",
+      "title": "Container failure HITL escalation requires DecisionQueue integration in Phase 2",
+      "description": "The architect's new container_failure_behavior specifies HITL decision creation with Retry/Continue/Abort options. This requires the ConcurrentPhaseExecutor to integrate with the existing DecisionQueue (orchestrator/decision_queue.py). The Phase 2 task list (TASK-2-2) mentions 'failures detected' in acceptance criteria but does not explicitly mention HITL decision creation or DecisionQueue integration.",
+      "severity": "non-blocking",
+      "action": "Update TASK-2-2 acceptance criteria to include: 'Container failure creates HITL decision via DecisionQueue with Retry/Continue/Abort options; test HITL integration with mocked decision resolution.'"
+    }
+  ],
+
+  "risks_update": {
+    "note": "All 12 risks from revision 1 remain valid. The architect's revision 2 does not change the fundamental risk profile. The container_failure_behavior addition (review feedback #2) is a positive change that partially mitigates R-3 (resource exhaustion) by providing human escalation for failed containers. Below I summarize changes to risk assessments based on revision 2.",
+    "updated_risks": [
+      {
+        "id": "R-1",
+        "title": "Signal handler race conditions amplified by concurrent agents",
+        "change": "No change. Confirmed: signals.py has ZERO uses of get_pipeline_state_lock(). All four signal handlers (complete, progress, error, heartbeat) operate without locks. The lock IS used extensively in routes/pipelines.py (40+ usages) and decision_queue.py, confirming it's the intended pattern for state mutations. Signal handlers are the outlier.",
+        "severity": "HIGH — PREREQUISITE FIX REQUIRED"
+      },
+      {
+        "id": "R-2",
+        "title": "Per-agent worktree infrastructure not wired into orchestrator",
+        "change": "No change. Confirmed: the TODO at orchestrator/routes/pipelines.py:3942-3946 is present verbatim. The architect's revision 2 does not add worktree API integration as an explicit task.",
+        "severity": "HIGH — PREREQUISITE FIX REQUIRED"
+      },
+      {
+        "id": "R-3",
+        "title": "Resource exhaustion from concurrent container spawning",
+        "change": "Slightly mitigated by the new container_failure_behavior. HITL escalation on container failure (OOM kill) gives humans the option to reduce concurrency or abort. But the initial spawn still risks resource exhaustion. Recommendation unchanged: set max_concurrent_agents default to 4.",
+        "severity": "HIGH (unchanged)"
+      },
+      {
+        "id": "R-5",
+        "title": "Consensus deadlock from agent state oscillation",
+        "change": "No change. The consensus protocol design is unchanged in revision 2.",
+        "severity": "MEDIUM (unchanged)"
+      }
+    ],
+    "unchanged_risks": ["R-4 (session enumeration)", "R-6 (handoff incompatibility)", "R-7 (polling latency)", "R-8 (message injection)", "R-9 (Flask bottleneck)", "R-10 (resource leaks)", "R-11 (test suite breakage)", "R-12 (LLM collaboration effectiveness)"]
+  },
+
+  "cross_cutting_concerns_update": {
+    "note": "All 4 cross-cutting concerns from revision 1 remain valid. Adding one new concern.",
+    "new_concerns": [
+      {
+        "id": "CC-5",
+        "title": "HITL responsiveness assumption in concurrent mode",
+        "description": "The container_failure_behavior design (revision 2) assumes a human is available to respond to HITL decisions during concurrent execution. In concurrent mode, all agents are running simultaneously and the phase continues with remaining agents while awaiting human input. If the human doesn't respond within a reasonable window, the remaining agents may complete their work while the failed agent is stuck. When the human eventually responds with 'Retry', the retried agent may need to redo significant work to catch up with the other agents.",
+        "recommendation": "Add a timeout for the container failure HITL decision. If no human response within 10 minutes, default to 'Continue without agent' (the safest automatic choice — doesn't retry and doesn't abort). Document this auto-resolution behavior."
+      }
+    ]
+  },
+
+  "overall_risk_rating": "MEDIUM-HIGH",
+  "recommendation": "PROCEED WITH CAUTION after resolving the following items: (1) Fix plan YAML: orchestrator/app.py → orchestrator/api.py in TASK-1-3 files list. (2) Fix plan YAML task ordering to match architect's dependency chain. (3) Add prerequisite phase-0 for signal handler locking fix and worktree API integration. (4) Security review of message API before implementation. The architecture is sound, the phased approach limits blast radius, and the opt-in flag provides clean rollback. The messaging system (Phase 1) remains independently valuable and lower risk — ship it first."
+}

.egg-state/checks/1027-implement-results.json

@@ -0,0 +1,20 @@
+{
+  "all_passed": true,
+  "checks": [
+    {
+      "name": "lint",
+      "passed": true,
+      "output": "==> Ruff check...\nAll checks passed!\n==> Ruff format check...\n442 files already formatted\n==> Mypy...\nSuccess: no issues found in 120 source files\n==> Shellcheck...\n==> Custom checks...\nOK: All checks passed"
+    },
+    {
+      "name": "test",
+      "passed": true,
+      "output": "7491 passed, 87 skipped, 4 warnings in 99.69s (0:01:39)"
+    },
+    {
+      "name": "security",
+      "passed": true,
+      "output": "==> Running security scan...\nSKIP: bandit not installed"
+    }
+  ]
+}