# MFD Monitor Agent Completion Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [x]`) syntax for tracking.

**Goal:** Complete the host monitoring agent behavior required by `AGENTS.md` while preserving the current single-binary, cached-snapshot architecture.

**Architecture:** Keep the current packages and add test seams only where needed. Linux collectors are completed with parser-level tests, service installation becomes testable through unit rendering and command injection, reporter retry/backoff stays isolated from HTTP and collection, and Windows service behavior remains build-tagged.

**Tech Stack:** Go 1.22, standard library HTTP/config/process APIs, Linux `/proc`, systemd, Windows Service build tags, `go test`.

---

## File Structure

- Modify `cmd/agent/main.go`: preserve config path for service installation and add comments.
- Modify `internal/status/types.go`: add required comments for API-visible JSON model fields and types.
- Modify `internal/config/config.go`: add comments and preserve existing defaults/env behavior.
- Modify `internal/snapshot/snapshot.go`: add comments and tests for timeout/partial behavior.
- Modify `internal/httpapi/server.go`: add comments and tests for not-ready/method behavior.
- Modify `internal/reporter/reporter.go`: add retry/backoff and tests.
- Modify `internal/collector/collector.go`: add comments.
- Modify `internal/collector/collector_linux.go`: complete Linux CPU/process/TCP partial behavior and add parser seams.
- Modify `internal/collector/collector_other.go`: add comments and explicit unsupported behavior.
- Create `internal/collector/collector_linux_test.go`: parser and degradation tests.
- Modify `internal/service/service.go`: add comments and extend install options if needed.
- Modify `internal/service/service_linux.go`: root guard, systemd unit rendering, config path propagation, command seams.
- Create `internal/service/service_linux_test.go`: root guard and unit rendering tests.
- Modify `internal/service/service_windows.go`: document Windows service behavior and improve no-NSSM path where compile-testable.
- Modify `internal/service/service_other.go`: comments.

## Task 1: Comment Coverage Baseline

**Files:**
- Modify: `internal/status/types.go`
- Modify: `internal/config/config.go`
- Modify: `internal/snapshot/snapshot.go`
- Modify: `internal/httpapi/server.go`
- Modify: `internal/reporter/reporter.go`
- Modify: `internal/collector/collector.go`
- Modify: `internal/collector/collector_linux.go`
- Modify: `internal/collector/collector_other.go`
- Modify: `internal/service/service.go`
- Modify: `internal/service/service_linux.go`
- Modify: `internal/service/service_windows.go`
- Modify: `internal/service/service_other.go`

- [x] **Step 1: Run comment lint proxy**

Run: `go test ./...`

Expected: PASS. This is the behavioral baseline before comments.

- [x] **Step 2: Add comments**

Add concise comments above every package-level type, struct, interface, function, and method. For API structs in `status`, include JSON/API meaning and units where relevant, for example:

```go
// HostStatus is the full cached host snapshot returned by /status and posted by the reporter.
type HostStatus struct {
	// Agent contains agent process metadata and the latest snapshot status.
	Agent AgentInfo `json:"agent"`
}
```

- [x] **Step 3: Verify comments did not change behavior**

Run: `go test ./...`

Expected: PASS.

## Task 2: HTTP Cache Edge Tests

**Files:**
- Modify: `internal/httpapi/server_test.go`
- Modify: `internal/httpapi/server.go`

- [x] **Step 1: Write failing tests**

Add tests named:

```go
func TestStatusReturnsServiceUnavailableBeforeSnapshot(t *testing.T) {}
func TestHealthReturnsStartingBeforeSnapshot(t *testing.T) {}
func TestStatusRejectsNonGET(t *testing.T) {}
func TestHealthRejectsNonGET(t *testing.T) {}
```

Assert `/status` returns `503` when `StaticProvider{OK:false}`, `/health` returns JSON status `starting`, and POST requests return `405`.

- [x] **Step 2: Verify red or existing green**

Run: `go test ./internal/httpapi -run 'Test(StatusReturnsServiceUnavailableBeforeSnapshot|HealthReturnsStartingBeforeSnapshot|StatusRejectsNonGET|HealthRejectsNonGET)'`

Expected: PASS if behavior already exists, otherwise FAIL with the missing behavior.

- [x] **Step 3: Implement only missing behavior**

Keep handler behavior cache-only. Do not add live collection.

- [x] **Step 4: Verify**

Run: `go test ./internal/httpapi`

Expected: PASS.

## Task 3: Reporter Retry And Cancellation

**Files:**
- Modify: `internal/reporter/reporter.go`
- Modify: `internal/reporter/reporter_test.go`

- [x] **Step 1: Write failing tests**

Add tests named:

```go
func TestReportReturnsErrorForHTTPFailure(t *testing.T) {}
func TestRunRetriesFailedReportOnce(t *testing.T) {}
func TestRunStopsDuringRetryDelayWhenContextCanceled(t *testing.T) {}
```

Use `httptest.Server` and a short interval/delay. Count requests with an atomic counter. First request returns `500`, second returns `202`.

- [x] **Step 2: Verify red**

Run: `go test ./internal/reporter -run 'Test(ReportReturnsErrorForHTTPFailure|RunRetriesFailedReportOnce|RunStopsDuringRetryDelayWhenContextCanceled)'`

Expected: retry tests FAIL because `Run` does not retry yet.

- [x] **Step 3: Implement minimal retry**

Add a retry delay parameter through an internal helper while preserving the public `Run` signature:

```go
const defaultRetryDelay = 2 * time.Second

func Run(ctx context.Context, provider Provider, client *Client, interval time.Duration, logger *slog.Logger) {
	runWithRetryDelay(ctx, provider, client, interval, defaultRetryDelay, logger)
}
```

In `runWithRetryDelay`, after a failed report, wait for `retryDelay` or context cancellation, then retry once.

- [x] **Step 4: Verify**

Run: `go test ./internal/reporter`

Expected: PASS.

## Task 4: Linux Collector Parser Coverage

**Files:**
- Create: `internal/collector/collector_linux_test.go`
- Modify: `internal/collector/collector_linux.go`

- [x] **Step 1: Write parser tests**

Add tests named:

```go
func TestParseAddressIPv4(t *testing.T) {}
func TestParseAddressIPv6(t *testing.T) {}
func TestParseProcessStatStartTime(t *testing.T) {}
func TestCPUUsageFromProcStatSamples(t *testing.T) {}
```

Use sample strings from `/proc/net/tcp`, `/proc/net/tcp6`, `/proc/<pid>/stat`, and `/proc/stat`. Expected IPv4 sample `0100007F:1F90` maps to `127.0.0.1:8080`.

- [x] **Step 2: Verify red**

Run: `go test ./internal/collector -run 'Test(ParseAddress|ParseProcessStatStartTime|CPUUsageFromProcStatSamples)'`

Expected: new process stat and CPU usage parser tests FAIL until helpers exist.

- [x] **Step 3: Implement parser helpers**

Add helpers:

```go
func parseProcessStatStartTime(raw string) (int64, error)
func parseCPUTimes(raw string) (idle uint64, total uint64, err error)
func cpuUsagePercent(prevIdle, prevTotal, nextIdle, nextTotal uint64) float64
```

Keep helpers unexported and deterministic.

- [x] **Step 4: Verify**

Run: `go test ./internal/collector`

Expected: PASS.

## Task 5: Linux Collection Fields

**Files:**
- Modify: `internal/collector/collector_linux.go`
- Modify: `internal/collector/collector_linux_test.go`

- [x] **Step 1: Write failing tests**

Add tests for reading process start time from a temporary stat file through a helper:

```go
func TestReadProcessStatReturnsStartTime(t *testing.T) {}
```

Add a CPU usage test around the helper rather than sleeping on real `/proc`.

- [x] **Step 2: Verify red**

Run: `go test ./internal/collector -run 'Test(ReadProcessStatReturnsStartTime|CPUUsage)'`

Expected: FAIL until `readProcess` and `collectResource` use the helpers.

- [x] **Step 3: Implement collection**

Update `readProcess` to set `StartTime`. Update `collectResource` to set `CPU.UsagePercent` through a bounded sample that respects context:

```go
select {
case <-ctx.Done():
	return ctx.Err()
case <-time.After(100 * time.Millisecond):
}
```

- [x] **Step 4: Verify**

Run: `go test ./internal/collector`

Expected: PASS.

## Task 6: TCP Listener Partial Mapping

**Files:**
- Modify: `internal/collector/collector_linux.go`
- Modify: `internal/collector/collector_linux_test.go`

- [x] **Step 1: Write failing test**

Add:

```go
func TestCollectSocketsMarksPartialWhenProcessFDsUnreadable(t *testing.T) {}
```

Use a helper seam around socket inode walking so the test can simulate an unreadable fd directory.

- [x] **Step 2: Verify red**

Run: `go test ./internal/collector -run TestCollectSocketsMarksPartialWhenProcessFDsUnreadable`

Expected: FAIL because unreadable fd directories are currently ignored.

- [x] **Step 3: Implement partial error propagation**

Make inode collection return `(map[string]status.ProcessInfo, error)`. Keep best-effort mappings, but return an error count when pid fd directories cannot be read. `collectSockets` sets `tcp_listeners.status` to `partial` and returns the error while preserving listener items.

- [x] **Step 4: Verify**

Run: `go test ./internal/collector`

Expected: PASS.

## Task 7: Linux Service Install Guard And Unit Rendering

**Files:**
- Modify: `cmd/agent/main.go`
- Modify: `internal/service/service.go`
- Modify: `internal/service/service_linux.go`
- Create: `internal/service/service_linux_test.go`

- [x] **Step 1: Write failing tests**

Add:

```go
func TestRenderSystemdUnitIncludesConfigPath(t *testing.T) {}
func TestLinuxInstallRequiresRoot(t *testing.T) {}
```

Assert the rendered unit includes `ExecStart=/opt/agent run -config /etc/mfd-agent.json` when a config path is supplied.

- [x] **Step 2: Verify red**

Run: `go test ./internal/service -run 'Test(RenderSystemdUnitIncludesConfigPath|LinuxInstallRequiresRoot)'`

Expected: FAIL because config path and root guard seams do not exist.

- [x] **Step 3: Implement install options**

Add a small options struct or extend `Install` input so `cmd/agent` can pass the config path. Add root guard through an injectable `currentUID` function and render the unit through a pure helper:

```go
func renderSystemdUnit(executable string, configPath string, cfg config.Config) string
```

- [x] **Step 4: Verify**

Run: `go test ./internal/service`

Expected: PASS.

## Task 8: Snapshot Timeout And Partial Tests

**Files:**
- Modify: `internal/snapshot/snapshot_test.go`
- Modify: `internal/snapshot/snapshot.go`

- [x] **Step 1: Write tests**

Add:

```go
func TestRunCollectsImmediately(t *testing.T) {}
func TestCollectWithTimeoutMarksCanceledCollectorPartial(t *testing.T) {}
```

Use a collector that waits on `ctx.Done()` and returns `ctx.Err()`.

- [x] **Step 2: Verify**

Run: `go test ./internal/snapshot`

Expected: PASS if current timeout behavior already satisfies this; otherwise FAIL and then implement the minimal fix.

## Task 9: Windows Build-Tagged Service Contracts

**Files:**
- Modify: `internal/service/service_windows.go`
- Optional create: `internal/service/service_windows_test.go`

- [x] **Step 1: Add comments and compile-safe structure**

Ensure Windows service code is build-tagged, documented, and does not reference Linux-only APIs.

- [x] **Step 2: Compile-check Windows**

Run: `GOOS=windows GOARCH=amd64 go test ./...`

Expected: PASS. If network/dependency constraints block a wrapper dependency, document the exact failure and keep Windows behavior explicit.

## Task 10: Final Verification And Audit

**Files:**
- Inspect: `AGENTS.md`
- Inspect: all modified Go files

- [x] **Step 1: Run full tests**

Run: `go test ./...`

Expected: PASS.

- [x] **Step 2: Run Windows compile-check**

Run: `GOOS=windows GOARCH=amd64 go test ./...`

Expected: PASS or documented blocker.

- [x] **Step 3: Audit comments**

Run: `grep -R "^func \\|^type " -n cmd internal`

Expected: every listed package-level function/type has a preceding useful comment.

- [x] **Step 4: Completion checklist**

Map each explicit AGENTS.md requirement to implementation evidence, test evidence, or a documented first-version limitation.

## Git Note

This directory is not currently a Git repository, so plan steps cannot include real commits. Use the test checkpoints above as completion gates.