diff --git a/.ciagent/CHECKPOINT.json b/.ciagent/CHECKPOINT.json index 1873c2e..afaafbd 100644 --- a/.ciagent/CHECKPOINT.json +++ b/.ciagent/CHECKPOINT.json @@ -1,11 +1,11 @@ { "phase": 0, - "stage": "clarify", + "stage": "research", "milestone": "v0.7", "milestone_slug": "hardening-completion", "phase_role": "pre_execution", "attempts": 0, - "updated_at": "2026-08-03T20:35:00Z", + "updated_at": "2026-08-03T20:45:00Z", "milestone_complete": false, "next_milestone": null } \ No newline at end of file diff --git a/.ciagent/PERSONAS.md b/.ciagent/PERSONAS.md index 2e60c94..949f4e5 100644 --- a/.ciagent/PERSONAS.md +++ b/.ciagent/PERSONAS.md @@ -2,26 +2,30 @@ active_personas: - lead-developer - backend-engineer - - cli-engineer - data-engineer - - security-engineer deactivated_personas: + - cli-engineer + - security-engineer - devops-engineer - network-engineer - frontend-engineer phase_specific: [] reason: | - Orca v0.6 is a bootstrap-ergonomics + heterogeneous-nodes milestone. - The work is schema (migration 0006), security (SSH keygen, TOFU, - sudoers, PVE role), CLI (init full bootstrap, node join --type proxmox, - doctor os/proxmox), and backend orchestration (proxmox SSH bootstrap - sequence). No devops (no install/docker/release), no network (no - transport/mTLS), no frontend (no UI). + Orca v0.7 is an NFR hardening & completion milestone. The work is CLI + registration (cert command), a new internal/config package, test + coverage uplift across engine/transport/proxmox/audit, and an opt-in + pprof endpoint on the daemon. No schema changes, no new security + surface, no packaging/distribution, no UI. - Roster changes vs v0.5: - - data-engineer: REACTIVATED — owns migration 0006 + NodeRepo schema extension. - - security-engineer: REACTIVATED — owns SSH keygen, TOFU host-key, sudoers, PVE role. - - devops-engineer: DEACTIVATED — v0.6 has no packaging/distribution surface. + Roster changes vs v0.6: + - data-engineer: RETAINED — owns cert_repo tests + store coverage. + - security-engineer: DEACTIVATED — v0.7 adds no new security surface + (pprof is operator-only, addr-gated; cert registration exposes + existing security code, does not add new). + - cli-engineer: DEACTIVATED — merged into lead-developer for v0.7 + (the cert registration is a 1-line AddCommand; config --config flag + is root-command wiring, not a new CLI subsystem). + - devops-engineer: DEACTIVATED — no packaging/distribution in v0.7. --- # Personas: Orca diff --git a/.ciagent/RESEARCH_v0.7.md b/.ciagent/RESEARCH_v0.7.md new file mode 100644 index 0000000..fd36492 --- /dev/null +++ b/.ciagent/RESEARCH_v0.7.md @@ -0,0 +1,161 @@ +# Research: Orca v0.7 — Hardening & Completion + +## 1. Codebase audit findings (RESEARCH stage) + +A full codebase audit surfaced the gaps that define the v0.7 scope. +Each finding is grounded in a specific file/coverage measurement. + +### 1.1 `orca cert` command tree is unreachable (critical) + +- `internal/cli/cert.go:44` exports `NewCommand(log *slog.Logger) + *cobra.Command` which builds the full `cert ca-init | gen | show | + renew | fingerprint` tree (5 subcommands, all implemented, all + spec-compliant per REQ-033/035/036). +- **No file in the repo calls `NewCommand` or registers it on + `rootCmd`.** `grep -rn "rootCmd.AddCommand" internal/cli/` lists + daemon, init, audit, version, job, node, doctor, status — `cert` is + absent. `./bin/orca cert` returns `error: unknown command "cert"`. +- The function is named `NewCommand` (not `newCertCmd`), so it is not + picked up by any init-based registration convention. +- **Impact**: every cert operation the spec promises (REQ-023, REQ-025, + REQ-033, REQ-035, REQ-036) is unreachable from the CLI. Operators + cannot bootstrap a CA, issue a server cert, or rotate one without + hand-crafting calls into the `security` package. This is the single + highest-impact bug in the v0.1–v0.6 line. +- **Fix**: one-line `rootCmd.AddCommand(NewCommand(log))` in + `internal/cli/cert.go` (or a new `init()`), plus a regression test + that asserts `rootCmd.Commands()` contains a child whose `Use == + "cert"`. + +### 1.2 `internal/store/cert_repo.go` has no test file + +- `internal/store/cert_repo.go` exists (the `certs` table from + migration 0004) but `internal/store/cert_repo_test.go` does not. +- Every other repo in `internal/store/` has a `_test.go`: + `node_repo_test.go`, `job_task_repo_test.go`, `capacity_repo_test.go`, + `audit_repo_test.go`, `migrate_test.go`. +- **Fix**: add `cert_repo_test.go` covering Insert/Get/List/rotation + history (N=3 per REQ-025) + serial_hex uniqueness. + +### 1.3 Low test coverage in core packages + +| Package | Coverage | Missing tests for | +|---------|----------|-------------------| +| `internal/engine` | 8.3% | `executor.go`, `dispatcher.go`, `peer.go` (only `scheduler_test.go` exists) | +| `internal/transport` | 26.3% | `mtls.go`, `dispatch.go`, `handshake_log.go` (only `idempotency_test.go` exists) | +| `internal/proxmox` | 5.1% | `bootstrap.go` SSH path (only `bootstrap_test.go` exists, exercises the no-op dry-run) | +| `internal/audit` | no test files | `audit.go` (Emit, EmitWithErr, LogHandshake*) | + +- Target per D-042: 50% floor per package, 70% for new code in P02/P04. +- Strategy: table-driven tests + `httptest.NewTLSServer` for transport; + interface-based mocks for the SSH dialer (already an interface in + `proxmox/bootstrap.go:211` `defaultSSHDialer` with `DialContext`). + +### 1.4 No HCL config file parser + +- D-009 specified `~/.orca/config.hcl` and `/etc/orca/orca.hcl` as + config locations. `find . -name "*.hcl"` returns only testdata + (`testdata/hello.hcl`, `testdata/fail.hcl`) used by jobspec tests. +- The CLI relies entirely on flags + env vars (`ORCA_HOME`, + `ORCA_DB`, `ORCA_PROXMOX_PASSWORD`). There is no `internal/config` + package. +- `internal/jobspec/spec.go:40` already uses + `hclsimple.Decode(filename, data, nil, &spec)` — the exact same + pattern works for a `Config` struct. No new dep required (hashicorp/hcl/v2 + is already a direct dep). +- **Fix**: new `internal/config` package with a `Config` struct (HCL + tags: `db_path`, `listen_addr`, `ca_path`, `server_cert_path`, + `server_key_path`, `node_capacity`), a `Load(paths ...string)` + function, and a `--config` flag on the root command. Precedence per + D-039: flag > env > file > default. + +### 1.5 pprof endpoint (I-308, deferred since v0.2) + +- I-308 was deferred in v0.2 IDEATE ("keep v0.2 lean") and never + revisited. The daemon (`internal/daemon/server.go`) has no pprof + surface today. +- `net/http/pprof` is stdlib — zero new deps. Mount on a separate + `*http.ServeMux` so it never touches the mTLS daemon listener. +- **Fix**: `--pprof ` flag on `orca daemon` (default disabled). + If set, start a second `http.Server` on `` with + `pprof.Index`/`pprof.Cmdline`/etc. registered. Log a WARN that the + endpoint is unauthenticated + operator-only. + +## 2. Prior art & patterns + +### 2.1 HCL config in HashiCorp tools + +Nomad, Consul, and Terraform all use HCL for config with the same +`hclsimple.Decode` + struct-tag pattern. The precedence model (flag > +env > file > default) is the de-facto standard; Viper implements it but +adds a large dep. Orca's `internal/config` will implement the 4-layer +merge by hand (~80 LOC) to stay minimal-deps. + +### 2.2 pprof in Go daemons + +Standard pattern: `import _ "net/http/pprof"` registers handlers on +`http.DefaultServeMux`. Best practice for production daemons is a +**separate listener** (not DefaultServeMux) so pprof is never exposed +on the public port. Orca will use a dedicated `*http.ServeMux` + +`http.Server` on the `--pprof` addr, default disabled. + +### 2.3 Test coverage for concurrent Go + +`internal/engine` (executor, dispatcher) and `internal/transport` +(mtls, dispatch) are concurrent. Coverage strategy: +- `httptest.NewTLSServer` for transport — exercise real TLS handshakes + against an in-process server. +- Interface-based mocks for the SSH dialer (proxmox) and the peer + client (transport) — both already have interface seams. +- `sync.WaitGroup` + channel assertions for executor/dispatcher + lifecycle. +- `-race` is already on in CI (REQ-031) — new tests inherit it. + +## 3. v0.7 Architectural Decisions (AD-022..AD-026) + +| ID | Decision | Rationale | +|----|----------|-----------| +| AD-022 | `orca cert` registered via `init()` in `cert.go` calling `rootCmd.AddCommand(NewCommand(slog.Default()))` | Keeps registration co-located with the command definition; matches the pattern in `daemon.go`/`audit.go` where each command file self-registers. Avoids a central registration function that would drift. | +| AD-023 | `internal/config` package: `Config` struct + `Load(paths ...string) (*Config, error)`; no global singleton | Config is passed explicitly to `daemon.NewServer`, `cli` commands, etc. No package-level state — testable, no init-order surprises. | +| AD-024 | pprof on a separate `*http.Server` + `*http.ServeMux`, default disabled | Never co-mingles with the mTLS daemon listener. Operator opts in via `--pprof :6060`. Matches Go daemon best practice. | +| AD-025 | Coverage floor measured per-package via `go test -cover ./` | No aggregate threshold (aggregates hide low-coverage packages). CI gate added in P03: `go test -cover ./internal/engine ./internal/transport ./internal/proxmox ./internal/audit` and assert each ≥ 50%. | +| AD-026 | No new direct dependencies in v0.7 | `net/http/pprof` (stdlib), `hashicorp/hcl/v2` (already direct). v0.7 preserves the minimal-deps ethos. | + +## 4. PERSONAS assessment + +v0.7 is an NFR milestone touching CLI, config, tests, and daemon. The +default 3-persona roster (lead-developer, backend-engineer, +data-engineer) is sufficient: + +- **lead-developer**: owns P01 (cert registration) + P04 (pprof) — CLI/ + daemon territory. +- **backend-engineer**: owns P02 (config package) — internal/config + + CLI integration. +- **data-engineer**: owns P01 cert_repo tests + P03 store coverage — + `internal/store` territory. +- **lead-developer** also owns P03 engine/transport/proxmox/audit + coverage (test-only phase, no schema changes). + +No new personas needed. No phase-specific personas. Territory +enforcement stays `warn`. See `.ciagent/PERSONAS.md` (updated). + +## 5. Dependencies + +v0.7 adds **zero** new direct dependencies: +- HCL parsing: `hashicorp/hcl/v2` (already direct, used by jobspec). +- pprof: `net/http/pprof` (stdlib). +- Tests: `net/http/httptest` (stdlib), existing interfaces. + +`go.mod` is unchanged by v0.7. + +## 6. Risks + +- **P01 cert registration** may surface latent bugs in the cert + subcommands (they've never been exercised end-to-end). Mitigation: + P01 includes a smoke test that runs `cert ca-init` + `cert gen` + + `cert show` + `cert fingerprint` against a temp `ORCA_HOME`. +- **P02 config precedence** is easy to get wrong (flag/env/file/default + merge order). Mitigation: table-driven test covering all 4 layers. +- **P03 coverage** on concurrent packages may reveal race conditions + (already hidden by the 8.3% coverage). Mitigation: `-race` is on; P03 + fixes any races it uncovers as part of the same phase. \ No newline at end of file