Architecture Improvement Report

AXIS — Architectural Improvement Report

Scope: internal/* + cmd/axis. All statuses below are verified against main source, not copied from the original proposals.

Legend: Problem → Cause → Verify → Shipped status.

---

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

Status: RESOLVED / merged in PR #225.

• Problem: internal/safety and internal/execution imported advisory-layer packages from internal/knowledge and internal/events.
• Cause: safety.Check accepted *knowledge.ClusterKnowledge; guarded execution emitted events directly and built execution context directly.
• Verify: internal/safety/blocker.go now imports internal/models, and internal/execution/guarded.go contains no events or knowledge imports. go list -deps ./internal/safety and ./internal/execution show no such upward dependencies.
• Shipped: safety.Check takes *models.ClusterSnapshot directly. Execution defines the consumer-side ExecutionEventSink interface and ExecutionContextBuilder function type on GuardedExecutionRequest. L5 callers (agent, api, daemon, and cmd/axis) inject events.GuardedExecutionSink{} and knowledge.ExecutionContextJSON. This is intentionally not the originally proposed single ClusterView interface.
• reservation no longer imports events; reservation lifecycle events use a package-neutral EventEmitter callback configured by the events package.

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

Status: RESOLVED / merged in PR #223.

• Problem: Listener callbacks previously created an unbounded goroutine fan-out during event bursts.
• Cause: Dispatch was performed independently for each listener/event.
• Verify: internal/events/events.go defines dispatchWorkers = 8, dispatchQueueSize = 256, a buffered dispatchQueue, and fixed workers. enqueueDispatch drops the oldest queued advisory job when full rather than blocking the event producer.
• Shipped: The process-global compatibility bus remains, but listener dispatch now uses the bounded eight-worker pool and buffered queue. Listener callbacks remain advisory and are isolated from the execution critical path.

3. Daemon coordination SPOF: snapshot readers contended on the refresh lock

Status: RESOLVED / merged in PR #223.

• Problem: Snapshot and metadata readers could block behind refresh work protected by the daemon mutex.
• Cause: Readers accessed mutable cached state under d.mu.
• Verify: internal/daemon/daemon.go defines atomic.Pointer[models.ClusterSnapshot] and an atomic immutable metadata publication pointer. Snapshot() and Meta() load published state without acquiring d.mu; refresh writers continue to serialize state mutation.
• Shipped: Refresh publishes cloned snapshots and immutable metadata atomically. Snapshot() clones the loaded snapshot for caller isolation, while Meta() composes metadata from atomic state plus concurrency-safe mesh, ledger, and pending-latency reads.

4. Full-file persistence work held the reservation mutex during I/O

Status: RESOLVED / merged in PR #223.

• Problem: Reservation persistence could hold the in-memory ledger mutex while marshaling and writing the complete JSON file.
• Cause: The persistence critical section combined in-memory mutation, serialization, and disk I/O.
• Verify: internal/reservation/persist.go snapshots entries under l.mu.RLock() and releases that mutex before writeSnapshot; the file lock still serializes persistence and persist.WriteFileAtomic preserves atomic replacement.
• Shipped: Reservation state is copied while protected, then marshaled and atomically persisted under the file lock with the in-memory mutex released. The narrowed lock scope is covered by internal/reservation/persist_lock_test.go.

5. Missing hardening: subprocess cancellation, HTTP lifecycle, and URL validation

Status: RESOLVED / merged in PR #223.

• Problem: Fact-collector subprocesses lacked caller-context cancellation; streaming/provider HTTP operations needed explicit lifecycle handling; and user-influenced webhook/MCP URLs lacked basic outbound validation.
• Verify: Fact collection uses exec.CommandContext throughout the local collector, and guarded stash restoration uses the caller context. Webhook and MCP URL setup calls netutil.ValidateOutboundURL.
• Shipped: The streaming chat and Ollama clients intentionally retain Timeout: 0; their request contexts control cancellation so a healthy long-running stream is not cut off by a short client timeout. The current main source also uses a context-bounded http.Client{} for the llmrouter classification request (Classify applies its configured context budget), rather than a client-level 30-second timeout.
• netutil.ValidateOutboundURL requires http or https and a non-empty host. In the current shipped source it also resolves hosts and rejects loopback, private, link-local, and unspecified addresses unless explicitly allowlisted with netutil.AllowInternalHost. This preserves explicit self-hosted/private deployment opt-in; it is not accurate to describe the shipped implementation as scheme/host-only.
• The hardening remains intentionally minimal: no general outbound-host allowlist is imposed beyond the explicit internal-host opt-in.

---

Other wiki documentation checked

• L2-snapshot.puml had stale “single cache lock” wording; it now describes atomic snapshot/metadata publication and lock-free reads.
• L5-advisory.puml had stale “unbounded callback goroutines” wording; it now describes the fixed worker pool and bounded dispatch queue.
• Event-Bus-and-Webhooks.md contains no clear contradiction with the shipped bounded dispatch implementation.