Architecture Improvement Report

AXIS — Architectural Improvement Report

Scope: internal/* + cmd/axis. All claims code-verified (go list import graph + source anchors), not doc-derived. Layers L1–L5 per the 5-layer stack. Ordered by impact.

Legend: Problem → Cause → Verify → Fix.

---

1. Layering inversion: L4 execution/safety depend upward on L5

• Problem: The safety gate — the lowest-trust guard in the execution path — imports a Layer-5 advisory aggregate. execution (L4) likewise imports L5 knowledge + events. Advisory surfaces are supposed to be subordinate to the fact/exec plane; here the plane depends on them.
• Cause: safety.Check takes *knowledge.ClusterKnowledge, and knowledge transitively pulls snapshotview/state/git (internal/safety/blocker.go:3-8,20; internal/knowledge/context.go:6-10). execution/guarded.go:20-34 imports both knowledge and events.
• Verify: go list -f '{{.ImportPath}} {{.Imports}}' ./internal/safety ./internal/execution | grep knowledge → both match. Draw the layer graph: edges point L4→L5.
• Fix (surgical): Invert via a narrow consumer-defined interface. Define type ClusterView interface { … } (only the fields safety/execution actually read) in the L4 package, pass a value/struct instead of the concrete knowledge.ClusterKnowledge. The L5 caller (agent/api/daemon) builds knowledge and adapts it to the interface. Removes the L4→L5 import without behavior change.

2. events is a process-global bus with unbounded dispatch

• Problem: Single global event bus = shared mutable SPOF; every listener/webhook callback spawns an unbounded goroutine, panics are swallowed, retries block on time.Sleep. Under event bursts this leaks goroutines and provides no backpressure.
• Cause: Cortex client, listener list, ring buffer, interests, queue, worker (sync.Once), and webhook config are package-level vars (internal/events/events.go:17-25,124-127,194-207,250-255; webhooks.go:15-20). Dispatch fan-out: events.go:162-170, webhooks.go:42-63. mcp further mutates this global listener state per-server (internal/mcp/server.go:34-56).
• Verify: grep -n '^var ' internal/events/*.go; load-test Emit in a loop with a slow listener and watch runtime.NumGoroutine() climb without bound.
• Fix (surgical): Make the bus a type Bus struct{…} instance owned at app root (daemon/CLI), injected where needed. Replace per-callback go with a fixed worker pool draining a bounded channel (drop-oldest or block-with-deadline on full). Make retry sleeps select { case <-time.After: case <-ctx.Done(): }. Keep the package-global as a thin default singleton for compatibility if needed.

3. daemon coordination SPOF: single cache lock serializes all reads

• Problem: The daemon is a coupling hotspot (imports L1/L2/L4/L5) and every snapshot reader contends on one mutex that the refresh path also holds while doing node processing + reconciliation. Read latency spikes during refresh; a slow refresh stalls all consumers (CLI status, API, MCP).
• Cause: Daemon centralizes mutable maps behind a single lock; refresh does node result processing + reservation/state reconcile inside the central path (internal/daemon/daemon.go:79-115,617-739,866-889).
• Verify: grep -n 'mu\.\(Lock\|RLock\)' internal/daemon/daemon.go — confirm reads and RefreshNow share the same lock; benchmark concurrent Snapshot() calls during a RefreshNow.
• Fix (surgical): Store the cached snapshot in an atomic.Pointer[models.ClusterSnapshot]. Refresh builds a new snapshot off-lock and does a single atomic swap; Snapshot()/Meta() become lock-free reads. Keep the mutex only to serialize concurrent refreshes. No API change.

4. Full-file JSON read-modify-write held under lock (state/reservation/skills)

• Problem: Every mutation rewrites the entire JSON store while holding the in-memory mutex across flock + disk I/O. This serializes unrelated mutations and gives O(state-size) write amplification per change — a real bottleneck as reservations/observations grow.
• Cause: state.Update holds the OS lock across read→decode→migrate→mutate→full atomic rewrite (internal/state/state.go:114-153); reservation Load/Save hold l.mu across flock + marshal + write (internal/reservation/persist.go:90-115,136-165); skills.Store.Save rewrites the whole store (internal/skills/skills.go:71-75).
• Verify: grep -n 'mu.Lock' internal/reservation/persist.go and trace that marshal/WriteFileAtomic occur before Unlock; measure Reserve latency vs. ledger size.
• Fix (surgical, low-risk): Narrow the critical section — mutate the in-memory struct under mu, snapshot it, Unlock, then marshal + persist under the file lock only. Optionally debounce/coalesce persistence (dirty flag + single writer goroutine). persist.WriteFileAtomic already does temp+fsync+rename, so durability is preserved.

5. Missing hardening: contextless subprocesses, no-timeout HTTP clients, unvalidated URLs (SSRF)

• Problem: (a) Local fact collection can hang uncancellably; (b) LLM/chat HTTP clients have no timeout, so a stalled provider hangs unless every caller sets a deadline; (c) operator-supplied webhook/MCP-server URLs are dispatched without scheme/host validation → SSRF to link-local/metadata endpoints.
• Cause: (a) exec.Command (not CommandContext) throughout internal/facts/local.go:322-328,456-511,720,800,916,1294,1435,1582,1675, plus a context.Background() git stash pop in execution/guarded.go:1415; (b) &http.Client{} with no Timeout in chat/client.go:42, chat/ollama.go:26, llmrouter/engine.go:117-123; (c) webhook http.NewRequest with no context/validation (events/webhooks.go:23-70,69-86), MCP endpoint construction (mcpclient/connection.go:154-190).
• Verify: grep -rn 'exec.Command(' internal/facts | grep -v Context; grep -rn 'http.Client{' internal | grep -v Timeout; unit-test webhook dispatch with http://169.254.169.254/… and confirm it is currently allowed.
• Fix (surgical): (a) thread the collector ctx into exec.CommandContext; (b) set an explicit Timeout (or http.Client with a transport deadline) on the three clients; (c) add a small validateOutboundURL() (require http/https, reject link-local / loopback / metadata IPs unless explicitly allowlisted) called before webhook + MCP dispatch. Independent, low-blast-radius changes.

---

Secondary (worth tracking, lower priority)

• Library-level panics on the hot path: ui.Select re-panics recovered input panics (internal/ui/select.go:189); llmrouter.Registry.MustRegister panics on registration error (registry.go:50-55). Prefer returning errors from library code.
• Silent error drops on persistence/transport/stream/JSON paths (state/state.go:776, skills/autodiscover.go:53, transport/ssh.go:258,275,318, events/events.go:293,319, daemon/run_stream.go:98-106, daemon/handlers.go:413). At minimum log at debug; several (state save, event append) should propagate.
• mcp.SessionCache single-mutex map serializes all session get/invalidate (internal/mcp/session_cache.go:21-31,103-112) — fine now, revisit if multi-session concurrency grows.