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---
This commit is contained in:
Jon Chery
@@ -1,11 +1,11 @@
|
|||||||
{
|
{
|
||||||
"phase": 0,
|
"phase": 0,
|
||||||
"stage": "clarify",
|
"stage": "research",
|
||||||
"milestone": "v0.7",
|
"milestone": "v0.7",
|
||||||
"milestone_slug": "hardening-completion",
|
"milestone_slug": "hardening-completion",
|
||||||
"phase_role": "pre_execution",
|
"phase_role": "pre_execution",
|
||||||
"attempts": 0,
|
"attempts": 0,
|
||||||
"updated_at": "2026-08-03T20:35:00Z",
|
"updated_at": "2026-08-03T20:45:00Z",
|
||||||
"milestone_complete": false,
|
"milestone_complete": false,
|
||||||
"next_milestone": null
|
"next_milestone": null
|
||||||
}
|
}
|
||||||
+16
-12
@@ -2,26 +2,30 @@
|
|||||||
active_personas:
|
active_personas:
|
||||||
- lead-developer
|
- lead-developer
|
||||||
- backend-engineer
|
- backend-engineer
|
||||||
- cli-engineer
|
|
||||||
- data-engineer
|
- data-engineer
|
||||||
- security-engineer
|
|
||||||
deactivated_personas:
|
deactivated_personas:
|
||||||
|
- cli-engineer
|
||||||
|
- security-engineer
|
||||||
- devops-engineer
|
- devops-engineer
|
||||||
- network-engineer
|
- network-engineer
|
||||||
- frontend-engineer
|
- frontend-engineer
|
||||||
phase_specific: []
|
phase_specific: []
|
||||||
reason: |
|
reason: |
|
||||||
Orca v0.6 is a bootstrap-ergonomics + heterogeneous-nodes milestone.
|
Orca v0.7 is an NFR hardening & completion milestone. The work is CLI
|
||||||
The work is schema (migration 0006), security (SSH keygen, TOFU,
|
registration (cert command), a new internal/config package, test
|
||||||
sudoers, PVE role), CLI (init full bootstrap, node join --type proxmox,
|
coverage uplift across engine/transport/proxmox/audit, and an opt-in
|
||||||
doctor os/proxmox), and backend orchestration (proxmox SSH bootstrap
|
pprof endpoint on the daemon. No schema changes, no new security
|
||||||
sequence). No devops (no install/docker/release), no network (no
|
surface, no packaging/distribution, no UI.
|
||||||
transport/mTLS), no frontend (no UI).
|
|
||||||
|
|
||||||
Roster changes vs v0.5:
|
Roster changes vs v0.6:
|
||||||
- data-engineer: REACTIVATED — owns migration 0006 + NodeRepo schema extension.
|
- data-engineer: RETAINED — owns cert_repo tests + store coverage.
|
||||||
- security-engineer: REACTIVATED — owns SSH keygen, TOFU host-key, sudoers, PVE role.
|
- security-engineer: DEACTIVATED — v0.7 adds no new security surface
|
||||||
- devops-engineer: DEACTIVATED — v0.6 has no packaging/distribution 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
|
# Personas: Orca
|
||||||
|
|||||||
@@ -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 <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.
|
||||||
Reference in New Issue
Block a user