feat(v1.5): MCP integration — readonly stdio server (phases 16-19)#18
Merged
Conversation
Plan MCP Server Foundation & Write Safety (SRV-02, SRV-03, SEC-02): - 16-01 (wave 1): atomic temp+rename policy writer with chmod 0644 (SEC-02) - 16-02 (wave 1): go-sdk v1.6.1 dependency legitimacy gate + install - 16-03 (wave 2): cpg mcp stdio skeleton (IOTransport capture+swap, zapslog stderr bridge, SilenceUsage/Errors) + in-memory purity harness Decisions: diffOut handled structurally (no pkg/hubble change); IOTransport used instead of StdioTransport per research lazy-capture finding.
- 16-03 Task 2: split stdout-purity harness into two independent in-memory transport pairs so no transport half is Connect-ed twice (go-sdk one-Connect-per-half contract; avoids -race hang/flake) - 16-03: add Handoff to Phase 17 note + SUMMARY obligation so the deferred D-02 mcpModeStdout() wiring is not silently dropped - 16-RESEARCH.md: retitle Open Questions -> (RESOLVED) with inline RESOLVED markers (Q1->16-03 objective, Q2->16-01 Task 1 step 4, Q3->research recommendation) + correction note under the harness skeleton
- Replace os.WriteFile(path, data, 0644) with same-dir CreateTemp -> Write -> Close -> Chmod(0644) -> Rename - Mirrors pkg/evidence/writer.go's unprefixed error convention - Explicit chmod preserves the 0644 contract (CreateTemp defaults to 0600); TestWriter_FilePermissions stays green
… writer - TestWriter_AtomicNoLeftoverTempFiles: asserts no .tmp-* entries remain in the namespace dir after a write, and the file unmarshals as valid CNP - TestWriter_ConcurrentReaderNeverSeesPartialFile: writer/reader goroutines race under -race; writer varies dest port per iteration so every call forces a real temp+rename, reader asserts every successful read unmarshals cleanly (never partial/truncated)
- go get github.com/modelcontextprotocol/go-sdk/mcp@v1.6.1 (operator-approved at Task 1 checkpoint) - adds transitive deps: google/jsonschema-go v0.4.3, segmentio/asm v1.1.3, segmentio/encoding v0.5.4, yosida95/uritemplate/v3 v3.0.2 - bumps golang.org/x/oauth2 v0.34.0 -> v0.35.0 (inert transitive; cpg is stdio-only, no HTTP transport) - require line lands as // indirect (no importer yet); go mod tidy deliberately deferred to plan 16-03
- SEC-02 landed: pkg/output/writer.go writes via same-dir temp+rename - REQUIREMENTS.md: mark SEC-02 complete
- cmd/cpg/mcp.go: newMCPCmd/runMCPServer split, mcp.IOTransport captures the real stdin/stdout BEFORE the os.Stdout=os.Stderr backstop swap (D-01); never mcp.StdioTransport (lazy-capture trap would bind the wire to stderr) - noopCloseWriter protects the real stdout fd from IOTransport's Close() - SilenceUsage/SilenceErrors set on the mcp command only (D-03) - bridgedSlogLogger bridges go-sdk logs into the existing zap stderr stream via zap/exp/zapslog (SRV-03) - mcpModeStdout() pins the MCP-mode human-output seam to os.Stderr (D-02); diffOut needs no wiring since MCP mode never sets DryRun:true this phase - zero tools registered (composition root discipline for SEC-01/Phase 19) - register `mcp` in main.go rootCmd Dependency correction: zap/exp/zapslog is NOT bundled inside go.uber.org/zap v1.27.1 as 16-CONTEXT.md/16-RESEARCH.md claimed. It is the independently versioned go.uber.org/zap/exp module (v0.3.0, requires zap>=v1.26.0), added via `go get go.uber.org/zap/exp/zapslog@v0.3.0` + `go mod tidy` (operator-approved at the human-verify checkpoint). go-sdk v1.6.1 promoted from indirect to direct now that mcp.go imports it.
- cmd/cpg/mcp_harness_test.go: TestMCPStdoutPurity drives two independent
mcp.NewInMemoryTransports() pairs sharing one os.Stdout os.Pipe capture
- Session A: mcp.NewClient(...).Connect performs the initialize
handshake automatically; ListTools asserts the empty-tools response
- Session B: raw Connection (no automatic initialize) writes an unknown
method and asserts a well-formed JSON-RPC error Response comes back,
not a Go transport error
- each transport half is Connect-ed exactly once, honoring go-sdk's
Transport.Connect "called exactly once" contract; shutdown is via
context cancellation, mirroring go-sdk's own TestServerRunContextCancel
- final assertion: zero bytes leaked to the real os.Stdout across both
sessions (D-06)
- startInMemoryMCPSession helper factored out so Phases 17-19 extend this
file with one more call per new tool scenario instead of reinventing it
go test ./cmd/cpg/... -run TestMCPStdoutPurity -race -count=10 -v: stable,
zero data races.
- cmd/cpg/mcp_test.go, three deterministic unit tests:
- TestMCPModeStdoutNeverDefaultsToRealStdout (D-05 seam audit): pins
mcpModeStdout() == os.Stderr, != os.Stdout, non-nil
- TestMCPCobraFlagErrorStaysOffStdout (D-04 scenario 4 / D-03): a
flag-parse error on `cpg mcp` never reaches the real stdout
- TestMCPLoggingBridgesToZapStderr (SRV-03): a message logged via
bridgedSlogLogger() lands in cpg's existing zap stream exactly once
go test ./cmd/cpg/... -race and go test ./... -race (full 10-package
project suite) both green. golangci-lint run ./cmd/cpg/...: zero issues in
mcp.go/mcp_test.go/mcp_harness_test.go (6 pre-existing errcheck issues in
explain_render.go are untouched LINT-01 debt, out of scope).
TestMCPModeStdoutNeverDefaultsToRealStdout compared mcpModeStdout() against the live os.Stdout global with testify deep-equality. Other tests in the package legitimately swap os.Stdout (purity pipe capture), making the assertion order- and timing-sensitive — it flaked under -count>1 and occasionally in full-suite -race runs. Pin the contract with pointer identity instead: NotSame against the process's real stdout captured at package-var init (before any test mutates globals), Same against the current os.Stderr. The real-stdio aliasing net remains Phase 19's subprocess e2e (SRV-04).
… subprocess/tee harness, drop-flow fixtures - fakeRelay implements observerpb.ObserverServer.GetFlows only, signaling started (before sending flows) and cancelled (on stream ctx.Done()) via a mutex-guarded snapshot() -- the interface Plan 04's ungraceful variant will consume, built now per the interface-first ordering - buildE2EBinary builds ./cmd/cpg once per test-binary run via `go build -race`, guarded by sync.Once (D-05) - startE2ESubprocess/e2eSession wire mcp.IOTransport over a real exec.Cmd's stdin/stdout pipes with an io.TeeReader byte buffer for the stdout-purity re-validation -- never mcp.CommandTransport - buildPolicyDeniedFlow/buildInfraClassDropFlow set Verdict/DropReasonDesc on the base testdata fixtures so they actually flow through the real classifier (Pitfall 4)
- go mod tidy moves golang.org/x/tools v0.44.0 from the indirect block to the direct require block, now that mcp_audit_test.go imports go/packages, go/ssa, go/ssa/ssautil, go/callgraph, go/callgraph/rta directly - go.sum diff is empty: zero new module hashes, confirming this is a promotion of an already-resolved transitive dependency, not a new download (RESEARCH.md Package Legitimacy Audit) - go build ./... and the SEC-01 audit test both verified green under -race (62.65s, within the ~45-76s budget); full-suite regression (go test ./... -race) stays green across all 12 packages
…a/annotation assertions - TestMCPE2EGracefulLifecycle drives the full D-07 sequence over a real -race cpg mcp subprocess against the fake relay: initialize -> tools/list -> start_session -> get_status -> all 5 query tools mid-capture -> stop_session -> get_cluster_health post-stop -> close stdin -> exit 0 - SRV-01: serverInfo name/version, exactly 8 tools with descriptions + inputSchema + outputSchema, D-10 annotation truth per tool family (5 query tools assert OpenWorldHint=false; session tools never assert it), dropclass enum scoped to list_dropped_flows only - Mid-capture query-tool assertions poll via require.Eventually (aggregator flush_interval race), then assert a real policy file under DeriveSessionPaths(tmp_dir) (Pitfall 4 proof) - stdout byte-purity re-validated line-by-line on real stdio (D-05, redeems 16-CONTEXT D-06); testing.Short() skip - Rule 1 fix: replaced the two exec.Cmd-fed bytes.Buffer fields (stderr, rawTee) with a mutex-guarded syncBuffer -- go test -race caught a genuine data race between the background copy/read goroutines and diagnostic reads on the test goroutine
Add TestMCPE2EUngracefulDisconnect to cmd/cpg/mcp_e2e_test.go, reusing Plan 02's fake relay + subprocess/tee harness + fixtures with zero infra edits. Drives start_session -> get_status against the fake relay, then abruptly closes stdin with NO stop_session call, asserting the SESS-05 bounded cleanup fan-out fires: process self-exit within a 10s cap, tmp_dir removed (NoDirExists), and the fake relay's GetFlows stream context cancelled -- proving the fan-out that would close a real port-forward fired here (D-09 mapping documented in-test). - Rule 1 auto-fix: waitStarted alone (Pitfall 3 sync) is necessary but not sufficient -- disconnecting immediately after it races the relay's own fixture-send loop (empirically reproduced: flows_seen=0, 3ms session duration, cancelled stayed false 5s post-exit). Added a second synchronization gate: wait for the POLICY_DENIED fixture to land as a real policy file before disconnecting, proving the relay's send loop already completed. - Verified stable under rtk proxy go test ./cmd/cpg/... -run TestMCPE2EUngracefulDisconnect -race -count=5 (5/5 pass), both e2e variants together, full cmd/cpg package, and full module regression (go test ./... -count=1 -race) -- all green. go vet and golangci-lint clean.
…truncate os.* mutators
…ig-load exception
… bootstrap + open decision on cpg-driven mutation)
…skills/agents) and link from PROJECT.md
govulncheck flagged GO-2026-5970 (infinite loop on invalid input in x/text) as reachable via session.Manager.Stop -> norm.Form.*. Minimal MVS bump pulls x/term v0.44.0 and x/tools v0.47.0 along; full -race suite (610 tests) and the SEC-01 SSA audit verified green against the new x/tools.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
v1.5 MCP Integration — exposes cpg as a readonly MCP server over stdio (
cpg mcp): an LLM harness spawns the process, drives one live Hubble capture session at a time, and reads dropped flows, generated CiliumNetworkPolicy YAML, per-rule evidence, and cluster health through 8 tools. cpg stays deterministic; the LLM brings the judgment. No mutation surface: applying policies remains a human act.What shipped (phases 16–19)
os.Stdout→stderrbackstop), unified stderr logging (zap + go-sdk via zapslog), atomic temp+rename policy writerstart_session/get_status/stop_session; single-slot manager (capturing → stopped → gone), retained stopped session, idempotent stop, bounded cleanup fan-out on every exit path (stdin EOF, crash, SIGTERM)list_dropped_flows(composed sampled/aggregated view),list_policies+get_policy,get_evidence(byte-identical tocpg explain --output jsonvia promotedpkg/explain),get_cluster_health; pagination contract,structuredContent+outputSchema+ truthful annotations on all tools-race(graceful lifecycle incl. 8-tool handshake/schema proof + stdout byte-purity, and ungraceful-disconnect variant proving bounded self-exit + tmpdir cleanup), README## MCP Server (cpg mcp)section (harnessenvconfig, secrets posture, exec-credential-plugin caveat)Requirements
All 18 v1.5 requirements complete: SRV-01..04, SESS-01..06, QRY-01..05, SEC-01..03 (traceability in
.planning/REQUIREMENTS.md).Verification
-race— full suite green.planning/phases/16..19-*🤖 Generated with Claude Code
https://claude.ai/code/session_01PXgHGRAtNrXfcpQoALxEod