Files
orca/.ciagent/RESEARCH_v0.7.md
Jon Chery 7c4b603811 docs(P00): v0.7 research findings + persona assessment
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---
2026-08-03 20:29:07 +00:00

8.3 KiB
Raw Permalink Blame History

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.1v0.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 <addr> flag on orca daemon (default disabled). If set, start a second http.Server on <addr> 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 ./<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/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.