Codebase audit: cert command unreachable, no config parser, low coverage (engine 8.3%, transport 26.3%, proxmox 5.1%, audit 0%), pprof deferred. 5 architectural decisions (AD-022..AD-026). Zero new deps. ---ci--- project: orca phase: 0 milestone: v0.7 status: research ---/ci---
8.3 KiB
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:44exportsNewCommand(log *slog.Logger) *cobra.Commandwhich builds the fullcert ca-init | gen | show | renew | fingerprinttree (5 subcommands, all implemented, all spec-compliant per REQ-033/035/036).- No file in the repo calls
NewCommandor registers it onrootCmd.grep -rn "rootCmd.AddCommand" internal/cli/lists daemon, init, audit, version, job, node, doctor, status —certis absent../bin/orca certreturnserror: unknown command "cert". - The function is named
NewCommand(notnewCertCmd), 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
securitypackage. This is the single highest-impact bug in the v0.1–v0.6 line. - Fix: one-line
rootCmd.AddCommand(NewCommand(log))ininternal/cli/cert.go(or a newinit()), plus a regression test that assertsrootCmd.Commands()contains a child whoseUse == "cert".
1.2 internal/store/cert_repo.go has no test file
internal/store/cert_repo.goexists (thecertstable from migration 0004) butinternal/store/cert_repo_test.godoes 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.gocovering 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.NewTLSServerfor transport; interface-based mocks for the SSH dialer (already an interface inproxmox/bootstrap.go:211defaultSSHDialerwithDialContext).
1.4 No HCL config file parser
- D-009 specified
~/.orca/config.hcland/etc/orca/orca.hclas 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 nointernal/configpackage. internal/jobspec/spec.go:40already useshclsimple.Decode(filename, data, nil, &spec)— the exact same pattern works for aConfigstruct. No new dep required (hashicorp/hcl/v2 is already a direct dep).- Fix: new
internal/configpackage with aConfigstruct (HCL tags:db_path,listen_addr,ca_path,server_cert_path,server_key_path,node_capacity), aLoad(paths ...string)function, and a--configflag 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/pprofis stdlib — zero new deps. Mount on a separate*http.ServeMuxso it never touches the mTLS daemon listener.- Fix:
--pprof <addr>flag onorca daemon(default disabled). If set, start a secondhttp.Serveron<addr>withpprof.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.NewTLSServerfor 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.-raceis 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 ./<pkg> |
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/storeterritory. - 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 fingerprintagainst a tempORCA_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:
-raceis on; P03 fixes any races it uncovers as part of the same phase.