diff --git a/.ciagent/CHECKPOINT.json b/.ciagent/CHECKPOINT.json index 29e54f8..d997873 100644 --- a/.ciagent/CHECKPOINT.json +++ b/.ciagent/CHECKPOINT.json @@ -1,11 +1,11 @@ { "phase": 0, - "stage": "ideate", + "stage": "plan", "milestone": "v0.7", "milestone_slug": "hardening-completion", "phase_role": "pre_execution", "attempts": 0, - "updated_at": "2026-08-03T20:50:00Z", + "updated_at": "2026-08-03T20:55:00Z", "milestone_complete": false, "next_milestone": null } \ No newline at end of file diff --git a/.ciagent/PLAN_v0.7.md b/.ciagent/PLAN_v0.7.md new file mode 100644 index 0000000..8ce2bcd --- /dev/null +++ b/.ciagent/PLAN_v0.7.md @@ -0,0 +1,250 @@ +# Phase Plans: Orca v0.7 — Hardening & Completion + +All 4 execution phases + final review with vertical-slice structure, wave +ordering, and REQ-ID mapping. v0.7 scope: **Hardening & Completion** — +register the unreachable `orca cert` command, add HCL config file parsing, +uplift test coverage in core packages, and add the long-deferred pprof +endpoint. + +Branching: `phase/01-cert-register`..`phase/05-final-review-ship` on the +`milestone/v0.7-hardening-completion` branch (numbering restarts per +milestone per branch-strategy.md). + +Milestone type: **NFR** (all phases are fix/test/chore; no `feat` phases). +Tags run on the v0.5.x patch line: `v0.5.5` (P0) … `v0.5.10` (P05 = +milestone release). + +--- + +## Phase 1: Register `orca cert` Command Tree + cert_repo Tests (Wave 1) + +**Branch**: `phase/01-cert-register` +**REQ Coverage**: REQ-053 +**Persona leads**: lead-developer (cert registration + smoke test), data-engineer (cert_repo tests) +**Source ideas**: I-401, I-402, I-412 + +### Must-Haves + +#### lead-developer territory +- [ ] `internal/cli/cert.go` — add `init()` that calls `rootCmd.AddCommand(NewCommand(slog.Default()))`. This is the one-line fix that makes the entire `cert ca-init | gen | show | renew | fingerprint` tree reachable. (AD-022) +- [ ] `internal/cli/cert_test.go` (NEW) — regression test asserting `rootCmd.Commands()` contains a child whose `Use == "cert"`; assert each subcommand (`ca-init`, `gen`, `show`, `renew`, `fingerprint`) is present on the cert child. +- [ ] `internal/cli/cert_smoke_test.go` (NEW) — end-to-end smoke test against a temp `ORCA_HOME`: + - [ ] `orca cert ca-init --cn test-ca` → succeeds, `ca.crt` + `ca.key` exist with modes 0644/0600 + - [ ] `orca cert gen --cn test-server --san localhost --san 127.0.0.1` → succeeds, `server.crt` + `server.key` exist with modes 0644/0600 + - [ ] `orca cert show` → outputs PEM with no `PRIVATE KEY` blocks (REQ-035 redaction) + - [ ] `orca cert fingerprint --which ca` → outputs a 64-char hex SHA-256 + - [ ] `orca cert fingerprint --which server` → outputs a 64-char hex SHA-256 + - [ ] `orca cert renew` → succeeds, server cert file mtime updates +- [ ] `internal/cli/root_test.go` — extend the existing root test to assert `orca cert` is in the command tree (belt-and-suspenders with cert_test.go) + +#### data-engineer territory +- [ ] `internal/store/cert_repo_test.go` (NEW) — table-driven tests for `CertRepo`: + - [ ] `Insert` a cert row → `Get` by serial returns matching row + - [ ] `Insert` duplicate `serial_hex` → returns error (UNIQUE constraint, I-107) + - [ ] `List` returns certs ordered by `issued_at desc` + - [ ] Rotation history: Insert 4 certs for the same node → only last N=3 retained (REQ-025); oldest is pruned + - [ ] `GetActive` returns the most-recent cert for a node + - [ ] `Delete` removes a cert by serial + +### Verification + +- `go build ./...` PASS +- `go vet ./...` PASS +- `go test ./internal/cli/... ./internal/store/...` PASS +- `go test -race ./...` PASS +- `./bin/orca cert` → prints help (no longer "unknown command") +- `./bin/orca cert ca-init` on a temp `ORCA_HOME` → succeeds +- `./bin/orca cert show` → no private key material in output (REQ-035) +- cert_repo_test.go covers Insert/Get/List/rotation-prune/duplicate-serial + +--- + +## Phase 2: HCL Config File Parsing (Wave 1) + +**Branch**: `phase/02-config-parser` +**REQ Coverage**: REQ-054 +**Persona leads**: backend-engineer (config package), lead-developer (root command --config flag wiring) +**Source ideas**: I-406, I-408 +**Depends on**: Phase 1 (cert registration lands first so the CLI surface is complete before config extends it) + +### Must-Haves + +#### backend-engineer territory +- [ ] `internal/config/config.go` (NEW package) — `Config` struct with HCL tags: + - [ ] `DBPath string `hcl:"db_path,optional"`` + - [ ] `ListenAddr string `hcl:"listen_addr,optional"`` + - [ ] `CAPath string `hcl:"ca_path,optional"`` + - [ ] `ServerCertPath string `hcl:"server_cert_path,optional"`` + - [ ] `ServerKeyPath string `hcl:"server_key_path,optional"`` + - [ ] `NodeCapacity *CapacityConfig `hcl:"node_capacity,block"` (optional block) + - [ ] `Load(paths ...string) (*Config, error)` — loads the first existing file from `paths` via `hclsimple.Decode` (reuse the jobspec pattern, `internal/jobspec/spec.go:40`); returns a zero-value `Config` if no file exists (no error) + - [ ] `(*Config).MergeOverrides(flags Flags, env Environ) *Config` — applies precedence flag > env > file > default (D-039). Only non-zero flag values override; only set env vars override; file values are the base; missing fields fall back to `certpaths.*` defaults. + - [ ] No package-level state (AD-023). `Load` is a pure function. +- [ ] `internal/config/config_test.go` (NEW) — table-driven tests: + - [ ] Load from a valid HCL file → all fields populated + - [ ] Load from a missing file → zero Config, no error + - [ ] Load from a malformed HCL file → error + - [ ] MergeOverrides: flag wins over env wins over file wins over default (all 4 layers exercised) + - [ ] MergeOverrides: empty flag does NOT override a set env value + - [ ] MergeOverrides: empty env does NOT override a set file value + - [ ] Optional `node_capacity` block parsed correctly + +#### lead-developer territory +- [ ] `internal/cli/root.go` — add `--config string` persistent flag (default `""`). In `PersistentPreRunE`, if `--config` is set, call `config.Load(flag)` and stash the `*Config` in `cmd.Context()` via a context key. If `--config` is empty, `config.Load` is not called (zero overhead; existing flag/env behavior unchanged). +- [ ] `internal/cli/daemon.go` — in the daemon command, if a `*Config` is present in the context, use `cfg.ListenAddr` as the default addr (flag still overrides per D-039). +- [ ] `internal/cli/root_test.go` — extend with `--config ` test: pass a config file, assert the merged values reach the daemon command. +- [ ] `testdata/config.hcl` (NEW) — example config file for tests: + ```hcl + db_path = "/tmp/orca/test.db" + listen_addr = "127.0.0.1:9999" + ca_path = "/tmp/orca/ca.crt" + server_cert_path = "/tmp/orca/server.crt" + server_key_path = "/tmp/orca/server.key" + ``` + +### Verification + +- `go build ./...` PASS +- `go vet ./...` PASS +- `go test ./internal/config/... ./internal/cli/...` PASS +- `go test -race ./...` PASS +- `./bin/orca --config testdata/config.hcl daemon --help` → no error +- Precedence test: flag value overrides config file value for the same key +- No new direct deps (`hashicorp/hcl/v2` already in go.mod) + +--- + +## Phase 3: Test Coverage Uplift (Wave 1) + +**Branch**: `phase/03-coverage-uplift` +**REQ Coverage**: REQ-055 +**Persona leads**: lead-developer (engine/transport/audit tests), data-engineer (store coverage) +**Source ideas**: I-403, I-404, I-405, I-410 +**Depends on**: Phase 1 + Phase 2 (tests build on the now-reachable cert tree + config package) + +### Must-Haves + +#### lead-developer territory — internal/engine +- [ ] `internal/engine/executor_test.go` (NEW) — test `Executor.Start`/`Wait` lifecycle: + - [ ] Start a command (`/bin/echo hello`) → Wait → exit code 0, stdout captured + - [ ] Start a failing command (`/bin/false`) → exit code non-zero + - [ ] Cancel via ctx → process killed, `WaitDelay` honored (REQ-021) + - [ ] Env propagation: `Env=["FOO=bar"]` → child process sees `FOO=bar` +- [ ] `internal/engine/dispatcher_test.go` (NEW) — test `Dispatcher.Submit`/`Dispatch`: + - [ ] Submit a job → dispatched to the correct peer (mock peer client) + - [ ] Idempotency key present → retry on transient failure (mock returns error twice then succeeds) + - [ ] Idempotency key absent → no retry (REQ-037) + - [ ] Bounded queue backpressure: fill the channel → Submit blocks (with timeout assertion) +- [ ] `internal/engine/peer_test.go` (NEW) — test the peer HTTP client: + - [ ] `httptest.NewTLSServer` mock → peer client POSTs a dispatch request + - [ ] TLS handshake failure → structured error with `peer` + `err` fields + +#### lead-developer territory — internal/transport +- [ ] `internal/transport/mtls_test.go` (NEW) — test mTLS handshake: + - [ ] `httptest.NewTLSServer` with a test CA → client with valid cert handshakes OK + - [ ] Client with expired cert → handshake fails with `event=mtls.handshake` log assertion + - [ ] Client with wrong CA → handshake fails +- [ ] `internal/transport/dispatch_test.go` (NEW) — test `Dispatch` RPC: + - [ ] Successful dispatch → 200 OK + - [ ] Dispatch with `X-Orca-Idempotency-Key` → idempotent + - [ ] Dispatch without key → 400 (per REQ-037) +- [ ] `internal/transport/handshake_log_test.go` (NEW) — assert `LogHandshakeOK`/`LogHandshakeFailed` emit the correct slog fields (`event`, `peer`, `cert_fp`, `err`) + +#### lead-developer territory — internal/audit +- [ ] `internal/audit/audit_test.go` (NEW) — test the `Audit` wrapper: + - [ ] `Emit` with `ActionCertIssued` + `ResultSuccess` → `engine.Record` called with correct args (mock `engine.Audit`) + - [ ] `EmitWithErr` → `engine.Record` called with `result=failure` + err in metadata + - [ ] `LogHandshakeOK` → slog output contains `event=mtls.handshake`, `result=ok`, `peer`, `cert_fp` + - [ ] `LogHandshakeFailed` → slog output contains `result=failed` + `err` + - [ ] Nil-safe: `(*Audit)(nil).Emit(...)` → no panic + +#### data-engineer territory — internal/proxmox +- [ ] `internal/proxmox/bootstrap_test.go` — extend the existing test: + - [ ] Mock the `sshDialer` interface (already present at `bootstrap.go:211`) → assert the full bootstrap sequence calls the right shell commands in order (user create, role create, role assign, sudoers drop, pubkey deploy) + - [ ] Idempotent re-run: mock returns "already exists" for user create → bootstrap succeeds without re-creating + - [ ] SSH auth failure → bootstrap returns wrapped error + - [ ] Assert no password is logged (D-031) + +#### CI gate (I-410) +- [ ] `.coreci.yml` — add a `coverage-gate` step in the `test` pipeline that runs `go test -cover ./internal/engine ./internal/transport ./internal/proxmox ./internal/audit` and fails if any package < 50% (AD-025). Use a small shell snippet + `awk`/`grep` to parse coverage percentages. + +### Verification + +- `go build ./...` PASS +- `go test -race ./...` PASS +- `go test -cover ./internal/engine` → ≥ 50% (was 8.3%) +- `go test -cover ./internal/transport` → ≥ 50% (was 26.3%) +- `go test -cover ./internal/proxmox` → ≥ 50% (was 5.1%) +- `go test -cover ./internal/audit` → ≥ 50% (was 0%) +- CI coverage gate step passes +- Any races uncovered by `-race` are fixed in this phase (not deferred) + +--- + +## Phase 4: `--pprof` Opt-in on `orca daemon` (Wave 1) + +**Branch**: `phase/04-pprof-daemon` +**REQ Coverage**: REQ-056 +**Persona leads**: lead-developer (daemon flag + pprof server) +**Source ideas**: I-407, I-409 +**Depends on**: Phase 3 (daemon tests exist; pprof adds a new daemon path) + +### Must-Haves + +#### lead-developer territory +- [ ] `internal/daemon/pprof.go` (NEW) — `StartPprof(addr string, log *slog.Logger) (*http.Server, error)`: + - [ ] Create a dedicated `*http.ServeMux` (NOT `http.DefaultServeMux`) + - [ ] `import _ "net/http/pprof"` → register `pprof.Index`, `pprof.Cmdline`, `pprof.Profile`, `pprof.Symbol`, `pprof.Trace`, `pprof.Handler` on the dedicated mux + - [ ] Return a `*http.Server` listening on `addr` with the dedicated mux + - [ ] Log a WARN: `pprof endpoint exposed unauthenticated on — operator-only, do not expose publicly` + - [ ] Never touch the mTLS daemon listener (AD-024) +- [ ] `internal/daemon/server.go` — add a `pprofAddr string` field to `Options` (default `""` = disabled). In `Start`, if `pprofAddr != ""`, call `StartPprof` and store the `*http.Server` for `Shutdown`. +- [ ] `internal/daemon/pprof_test.go` (NEW) — test: + - [ ] `StartPprof("127.0.0.1:0", ...)` → server starts, GET `/debug/pprof/` returns 200 + - [ ] GET `/debug/pprof/cmdline` returns the cmdline + - [ ] `Shutdown` stops the pprof server + - [ ] The mTLS daemon server (if running) is unaffected by pprof start/stop +- [ ] `internal/cli/daemon.go` — add `--pprof string` flag (default `""` = disabled). Pass it into `daemon.Options.PprofAddr`. Document in `--help`: "enable pprof endpoint on (e.g. :6060); unauthenticated, operator-only". +- [ ] `internal/cli/daemon_test.go` — extend: `--pprof 127.0.0.1:0` → daemon starts with pprof; flag absent → no pprof server. + +### Verification + +- `go build ./...` PASS +- `go vet ./...` PASS +- `go test -race ./internal/daemon/...` PASS +- `./bin/orca daemon --pprof 127.0.0.1:0` (in background) → `curl http://127.0.0.1:/debug/pprof/` returns 200 +- `./bin/orca daemon` (no `--pprof`) → no pprof listener, `/debug/pprof/` not reachable on the daemon port +- pprof mux is separate from the mTLS daemon mux (asserted in test) + +--- + +## Phase 5: Final Review + Ship + Audit (Wave 1) + +**Branch**: `phase/05-final-review-ship` +**REQ Coverage**: all (REQ-053..056) +**Persona leads**: lead-developer (review + audit + ship) + +### Must-Haves + +- [ ] Multi-persona code review across all v0.7 phases (ciagent-review) +- [ ] Audit: reconstruction test (git log matches `.ciagent/` files), branch hygiene, commit discipline (ciagent-audit) +- [ ] Fix any P0 issues found by review; record P1+ in `.ciagent/` for post-hoc +- [ ] Merge `phase/05` → `milestone/v0.7-hardening-completion` +- [ ] Merge `milestone/v0.7` → `main` (rebase-then-fast-forward per config) +- [ ] Tag `v0.5.10` (final phase patch = milestone release) +- [ ] Create Gitea release with full milestone summary (all phases, all REQs) +- [ ] Update `.ciagent/REQUIREMENTS.md` — mark REQ-053..056 complete +- [ ] Update `.ciagent/ROADMAP.md` — mark v0.7 complete +- [ ] Write checkpoint: `{phase: 5, stage: "complete", phase_role: "final", milestone_complete: true}` +- [ ] Clear checkpoint (milestone complete; next run starts a new milestone) + +### Verification + +- `make build` PASS +- `make test` PASS +- `make lint` PASS +- `go vet ./...` PASS +- `git log` on main shows all v0.7 phase commits +- `git tag --list 'v0.5.*'` shows v0.5.5..v0.5.10 +- REQUIREMENTS.md shows REQ-053..056 as Complete +- ROADMAP.md shows v0.7 as COMPLETE \ No newline at end of file