From bbfd518e983520c30bd7e27c06626eb4e812a78f Mon Sep 17 00:00:00 2001 From: agenticode <16611333+agenticode@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:28:15 +0900 Subject: [PATCH] cmd/kilter: wire what-if and closed-loop proposals into the binary pkg/whatif shipped with a full CLI specification and zero reachability. Adds kilter whatif (replay, delta, gate, proposal) and kilter proposals (list, show, reject -- approve is refused). The window is resolved from the history rather than the wall clock, and a window clamped by short history reports the clamp instead of claiming the range it was asked for. Co-authored-by: kording <74226694+kording@users.noreply.github.com> --- cmd/WHATIF-WIRING-FINDINGS.md | 528 +++++++++++++++ cmd/kilter/main.go | 8 + cmd/kilter/proposals.go | 477 ++++++++++++++ cmd/kilter/proposals_test.go | 542 ++++++++++++++++ cmd/kilter/testdata/whatif-bursty.json | 213 +++++++ cmd/kilter/whatif.go | 845 +++++++++++++++++++++++++ cmd/kilter/whatif_test.go | 687 ++++++++++++++++++++ 7 files changed, 3300 insertions(+) create mode 100644 cmd/WHATIF-WIRING-FINDINGS.md create mode 100644 cmd/kilter/proposals.go create mode 100644 cmd/kilter/proposals_test.go create mode 100644 cmd/kilter/testdata/whatif-bursty.json create mode 100644 cmd/kilter/whatif.go create mode 100644 cmd/kilter/whatif_test.go diff --git a/cmd/WHATIF-WIRING-FINDINGS.md b/cmd/WHATIF-WIRING-FINDINGS.md new file mode 100644 index 0000000..ea06174 --- /dev/null +++ b/cmd/WHATIF-WIRING-FINDINGS.md @@ -0,0 +1,528 @@ +# Q3 — wiring: `pkg/whatif` becomes reachable from the binary + +`pkg/whatif` (reasoning-engine unit 5) shipped with a complete CLI and API +specification and **zero reachability**: a user who built `kilter` could not run +a what-if, could not see a proposal, and could not find out that either +existed. That is the third time a unit has shipped its wiring as a FINDINGS +note — PR#29 (j1-wire) and PR#41 (p2-wire2) each cleaned up one round of it. +This closes the third. + +Two commands now do: + +``` +kilter whatif --demo --set = replay → delta → gate → proposal +kilter proposals list | show | reject the proposal record and its audit trail +``` + +**Status:** `gofmt -l ./cmd` empty, `go vet ./...`, `go build ./...`, +`go test -race -count=1 ./cmd/...` and `go test -race -short ./...` all green. +**`go.mod` and `go.sum` are unchanged**; every import added is stdlib or +intra-repo. Nothing under `pkg/**`, `docs/` or `deploy/` was touched. + +| | | +|---|---| +| New production code | 1,322 lines (`cmd/kilter/whatif.go`, `cmd/kilter/proposals.go`) | +| New tests | 1,229 lines, 30 test functions | +| New fixture | `cmd/kilter/testdata/whatif-bursty.json` (5,220 bytes, golden) | +| Core edit | `cmd/kilter/main.go`: two `case` arms, two usage lines, two doc lines | +| Coverage | `cmd/kilter` 45.1 % → **54.2 %** | + +--- + +## 1. What is reachable now + +### 1.1 `kilter whatif` — the counterfactual + +``` +$ kilter whatif --demo bursty --set cpu-headroom=1.05 + +kilter whatif — demo-bursty, bursty trace, 7 days, 2 workloads +window 2026-01-05T00:00:00Z .. 2026-01-12T00:00:00Z horizon 24h0m0s interval 24h0m0s +set: cpu-headroom=1.05 + + what changed + cpu-headroom 1.15 → 1.05 + + baseline 69955a2cdd2004fd + scored 14 decisions 12 refusals 2 (good 0, idle 2) + safety memViolations 0 cpuStarvation 0 oomKills 0 + efficiency oracleGap 92.9% (applied 23.4%) claimed/realized 1.00 + stability flipRate 0.000 flips 0 + regret $2.90 (resource $2.90 + risk $0.00) + + candidate f8e7e12e930bfae8 + … + regret $2.77 (resource $2.77 + risk $0.00) + + delta (candidate − baseline; negative is better everywhere except decisions) + safety memViolations 0 cpuStarvation 0 + regret -$0.13 (-4.4%) resource -$0.13 risk +$0.00 + efficiency oracleGap -4.1 pts (applied -4.8 pts) forgone +$0.00 + behaviour decisions 0 refusals 0 (idle 0) flipRate +0.000 + projected -$0.56/month, extrapolated from 168.0h of history + + Gate + ACCEPTED: the candidate dominates on the terms §4.6 defines + win: regret $2.9026 → $2.7736 + win: oracle gap 92.890% → 88.761% + required regret improvement $0.03 +``` + +Two replays of one recorded history through the **real** `pkg/backtest` +harness, differenced, and put through §4.6's seven dominance rules. `--json` +writes `Result.Encode()` verbatim, which is what +`testdata/whatif-bursty.json` pins. + +Implemented as specified in `pkg/whatif/FINDINGS.md` → "CLI and API surface": +`--from 30d|RFC3339`, `--to`, `--horizon`, `--interval`, `--starvation`, +`--policy`, `--candidate`, repeatable `--set`, `--enforce-refusals`, +`--derive-costs`, `--json`, `--propose`, `--rationale`, and the CI exit code +(spelled `--fail-on-no-improvement`, the name the spec uses). Three flags are +additions and each has a reason below: `--store`, `--author-id`, `--now`. + +### 1.2 `kilter proposals` — the record and its audit trail + +``` +$ kilter proposals list --store ./proposals.json + +kilter proposals — 1 record(s) in ./proposals.json + + ID STATE CLUSTER REGRET PROJECTED/MO CHANGES CREATED AUTHOR + ──────────────── ───── ─────────── ───────────── ──────────── ──────────── ──────────────────── ───────────────── + db58c9f1f46cec54 gated demo-bursty $2.90 → $2.77 -$0.56 cpu-headroom 2026-08-26T12:00:00Z system:kilter-cli + +$ kilter proposals show --store ./proposals.json db58c9f1f46cec54 +proposal db58c9f1f46cec54 — gated +{ … Proposal.Encode() verbatim … } + +audit trail + 2026-08-26T12:00:00Z (new) → draft by system:kilter-cli filed + 2026-08-26T12:00:00Z draft → gated by system:gate gate passed +``` + +`list` prints `Store.List()` / `ListState()` in the order they arrive — already +sorted by `(CreatedAt, ID)` — and does not re-sort. `show` prints +`record.Proposal().Encode()` verbatim plus `record.History()` and +`record.Approval()` when present. `--json` on either emits the package's own +`Record` wire form, which is the same shape `GET /api/v1/proposals[/{id}]` must +serve. + +--- + +## 2. The core change, and the test edits it forced: **none** + +`cmd/kilter/main.go` grew two `case` arms, two lines of `rootUsage` and two +lines of package doc. **No existing test changed, and none needed to.** + +That was checked rather than assumed. The two counts the brief warned about: + +| Assertion | Where | Why it did not move | +|---|---|---| +| `len(env.Plans) == 5`, "one per wired domain" | `cmd/kilter/domains_test.go:352` | counts **domains**, not subcommands; `whatif` and `proposals` register no domain | +| the closed set of `--domain` values | `cmd/kilter/domains_test.go:465-468` | `kilter domains`' subcommands, a different dispatcher | + +Nothing in `cmd/kilter` asserts over `rootUsage` or over the set of top-level +verbs (`grep -rn "rootUsage" cmd/kilter/*_test.go` is empty). So the honest +statement is: this wiring was additive at the dispatcher and the existing suite +is untouched, not loosened. + +--- + +## 3. What refuses, and why + +Five refusals. Each names the seam, exits non-zero, and prints **no** scorecard +— the failure mode this whole file exists to avoid is a number that looks fine. + +### 3.1 `whatif --cluster` — snapshot history is not persisted + +Same seam `kilter backtest --cluster` refuses on: `pkg/store` keeps only the +LATEST snapshot per cluster (`SaveSnapshot`/`LoadSnapshot` are keyed by +cluster, not by time), and `whatif.Scenario.History` is a +`backtest.SnapshotSource`. + +**The refusal matters more here than it does for `backtest`, not less**, and +that is the argument for repeating it rather than deferring to the other +command. `kilter backtest` over an empty replay prints one bad scorecard. +`kilter whatif` prints a *comparison*: two runs over an empty replay agree on +every field, so every delta is `0.00`, `regret $0.00` reads as "perfect", and +the gate's `regret improves by $0.0000, short of the $0.0100 margin` reads as +*"we measured, and the candidate is not better"*. A refusal that produces a +considered-sounding negative verdict is worse than one that produces a +suspicious zero. + +`TestWhatIfLiveHistoryRefusesRatherThanComparingTwoEmptyReplays` asserts the +refusal names `pkg/store`, `SaveSnapshotAt`, `Snapshots(cluster, from, to)` and +`backtest.SnapshotSource`, and that none of `regret`, `Gate`, `delta` or +`oracleGap` is printed. + +**Wired instead:** `--demo ` over `backtest.TraceSpec`, exactly as +`pkg/whatif/FINDINGS.md` → "The snapshot-history seam" suggests, so the output +format, the gate and the exit codes can be integrated against known numbers. +The trace start is `backtestEpoch`, the same package constant `kilter backtest` +uses — a replay window that drifts with the wall clock would additionally make +a proposal's fingerprint (which covers the window) change every night. + +### 3.2 `whatif --auto-tune=apply` — refused by name, on principle + +`pkg/whatif` deferred auto-apply explicitly "not because of budget": apply +needs a **writer**, and a writer is what breaks INV-4's single funnel. The flag +exists — rather than being absent, where `flag provided but not defined` would +teach a reader nothing — and refuses, naming where apply belongs: `pkg/api`, as +a caller that reads a gated proposal, mints an approval as a **configured +operator identity distinct from the tuner**, writes the config, and posts +`applied` with the §4.6 ledger entry. + +It is checked **first, before any replay**, so the output cannot read as though +the request was honoured and merely printed instead of applied. +`TestAutoTuneApplyIsRefusedByName` asserts the refusal text and that no +scorecard is emitted. + +`--auto-tune=propose` is refused too, for a different reason: the nightly loop +is brain wiring (`NewTuner` at brain start, `Run` on the nightly timer with +`historyEnd` from the newest snapshot, once per cluster), not a CLI verb — and +it needs the same snapshot-history seam §3.1 refuses on. `--auto-tune=off` is +the default and a no-op. + +### 3.3 `proposals approve` — a local CLI cannot authenticate a human + +This is the sharpest line in the unit, and the one the spec is most explicit +about: *"the actor must come from the authenticated caller, not from a flag … +authentication is the API layer's job and is the one thing that can undo the +guarantee."* + +`pkg/whatif` made self-approval **unrepresentable**: `Approval` has no exported +fields, no `UnmarshalJSON`, and the only route to one is `Store.Approve`, which +needs an `*Approver`, which `NewApprover` will not build for a non-human actor. +That is a type-system guarantee resting on exactly one input the type system +cannot supply — proof that a human is on the other end. + +A local CLI process has none. The identities within reach — `$USER`, +`os/user.Current`, the uid — describe the **session**, not the presence of a +person, and *everything running in that session inherits them, including unit +8's reasoner*, which is precisely the actor the human-only rule exists to +exclude. An agent with shell access running `kilter proposals approve ` +would mint `Actor{Kind: ActorHuman, ID: $USER}`, and because `sameIdentity` +compares IDs, an agent-authored proposal (`agent:kilter-reasoner`) would clear +the author≠approver check and be approved. **That is self-approval with two +extra steps**, and it converts a structural guarantee into a convention that +`sh -c` walks around. + +So `approve` refuses, points at +`POST /api/v1/proposals/{id}/approvals (authWrite, HUMAN token tier only)`, and +names the thing the CLI *can* do (`reject`). +`TestProposalsApproveIsRefusedByName` additionally asserts the refused command +**did not rewrite the store** and the record is still `gated`. + +### 3.4 `proposals applied` — nothing here writes config or a ledger entry + +`MarkApplied` records, after the fact, that a change landed, and §4.6 requires +a `LedgerEntry` in the same breath (proposal ID, both policy hashes, the +claimed `Delta.ProjectedMonthlyUSD`) — that entry is what later lets the +existing claimed-vs-measured join score the tuner itself. This binary writes +neither, so recording `applied` here would put an unbacked fact into an audit +trail whose whole value is that its facts are backed. + +It is also **unreachable**: only an approved proposal can be applied, and §3.3 +means nothing in the CLI can produce one. A verb that could only ever return +*"proposal X is gated; only an approved proposal can be applied"* is better +replaced by the reason it can never do anything else. + +### 3.5 `--propose` / `proposals *` without `--store PATH` + +`Store.Create` is what runs the gate and mints the ID, so a proposal that is +not stored is a receipt for a document that does not exist. And a *missing* +store file is an error rather than an empty listing, because "0 proposals" and +"you named a file that is not there" render identically and the first reads as +a fact about the fleet. + +Both refusals say the file is a **local artefact** and that the fleet's +proposals belong in `pkg/store`'s bbolt file under a `proposals` bucket. + +### 3.6 One more, which is a genuine mismatch between two commands + +`enforceDecisionRefusals` **in a policy file is refused by name** for +`kilter whatif`, and the reason is worth recording because the same key is +legal for `kilter backtest`: + +* For `kilter backtest` it is part of the **policy** (`cmd/kilter/backtest.go`'s + `policy.EnforceRefusals`), deliberately — there the question is "should we + wire `pkg/decision` in?" and an A/B through `Gate` is the honest way to ask. +* For `kilter whatif` it is part of the **yardstick**: + `whatif.Scenario.EnforceDecisionRefusals` is a *scenario* field shared by both + replays, precisely so the two sides cannot be scored under different rules. + +A policy file that set it would therefore be **silently ignored**, and a +what-if of a policy nobody ran is exactly the artefact this unit exists to +prevent. So the loader refuses it and names `--enforce-refusals`, which applies +to both runs. `TestEnforceRefusalsIsTheYardstickNotThePolicy` covers both flags +and additionally asserts that `--enforce-refusals` reaches **both** sides and +moves **neither** policy hash. + +--- + +## 4. Not letting the CLI become a self-approval path + +Hard rule 5 has two halves, and both are tested. + +### 4.1 The evaluation path cannot be pointed at the policy under test + +`TestTheEvaluationPathCannotBePointedAtThePolicyUnderTest`: + +1. **No flag scores a policy against itself.** `--set cpu-headroom=1.15` (the + shipped value) and `--candidate default` are both refused by + `Scenario.validate` *before anything is replayed*, and the test asserts no + scorecard, delta or gate line is printed. +2. **The yardstick is shared, end to end.** The two most different policies the + envelope allows (p80/1.05× vs p99/1.50× on both dimensions) are run through + the CLI, and `OracleCostUSD`, `Scored`, `Instants`, `Snapshots`, + `MemOOMKills`, `StarvationFactor` and the whole `CostModel` must be + **identical** across the baseline and candidate scorecards, and the oracle + identical across the two invocations. If the evaluation ever started tracing + back to the thing under test, the oracle would move with it. + +There is no CLI-side scoring code at all: every number printed is +`pkg/backtest`'s output or `whatif.Delta`'s arithmetic over two of its +scorecards. + +### 4.2 The proposer cannot supply the verdict + +`Result.Spec` deliberately omits the `GateResult` and `Store.Create` runs +`Decide` itself. `TestTheProposerCannotSupplyTheVerdict` files a candidate the +gate rejects and asserts it lands in `rejected` — and that it is **still +filed**, because a rejected proposal is the record of a question that was asked +and answered, and discarding it would make a loop that ran look like one that +never did. + +### 4.3 The author is never a human, and `--author-id` can only restrict + +`pkg/whatif/FINDINGS.md` says the CLI actor is `{human, }` and, in the same breath, "**never** synthesize a human identity for +an automated caller". Since §3.3 establishes the CLI has no authenticated +identity, the first half is unavailable and the second half decides it: +`--propose` files as `Actor{Kind: ActorSystem, ID: "kilter-cli"}`. + +`--author-id` overrides the **ID only**, and it is a flag while the approver +identity is not, for a reason that is structural rather than stylistic: +`whatif.sameIdentity` compares IDs and **ignores `Kind`** (deliberately — +otherwise `agent:alice` could file what `human:alice` approves). So an operator +who names themselves as author is *blocked* from later approving that proposal +through the authenticated funnel. **Naming yourself can only ever remove a +capability.** A flag whose worst case is a denial is safe in a way a flag that +grants one never is. + +`TestAuthorIDCanOnlyRestrict` proves both directions: `human:Alice` is refused +on the proposal filed by `system:alice` (case-insensitively, across kinds), and +`human:bob` is not. + +`TestNoCLIPathReachesApprovedOrApplied` states the aggregate property: after +any sequence of CLI commands, the store on disk contains no `"approved"`, no +`"applied"`, no `"approval"` and no `"kind": "human"`. + +--- + +## 5. Determinism + +* **No clock in anything computed.** The replay window comes from the history: + `backtestEpoch` is a constant, `--to` defaults to the end of the recorded + history, and a relative `--from 30d` is measured back from the **newest + snapshot**, never `time.Now()` — with the anchor printed, so a reader can see + which it was. `whatif.Scenario` takes no clock at all, so resolving this + window is the CLI's whole job here. +* **`--now` exists and touches nothing computed.** It feeds `CreatedAt` and the + audit transitions and nothing else. The wall clock is read once, at the edge + of the program, and passed inward as a `whatif.Clock`; `pkg/whatif` never + calls `time.Now` itself and errors rather than defaulting. +* **A window past either end of the history is clamped, and the clamp is + reported.** A run claiming a 30-day window over a 7-day trace is a lie by + omission — the same discipline `kilter domains --rds-fixture` uses for its + CloudWatch retention clamp. Asserted by + `TestWhatIfWindowComesFromTheHistoryNotTheWallClock`. +* **Byte-identical across repeated runs in ONE process.** Go randomizes map + iteration on every `range`, so in-process repetition is the real test: + `TestWhatIfOutputIsByteIdenticalAcrossRuns` repeats a noisy 3-workload + comparison six times in text and six in JSON. The same test permutes the + `--set` flags and requires everything except the echoed `set:` line to be + identical — the echo records what the operator typed; nothing computed from + it may. +* **The golden file.** `testdata/whatif-bursty.json` pins `Result.Encode()` + byte for byte, with the same `-update-fixtures` idiom `TestWriteRDSFixture` + and `TestWriteDomainFixtures` use. It additionally asserts the pinned + comparison is an **accepted** one, so a regression in the gate is visible + here rather than silently flipping a rejection to a different rejection. +* **The store file round-trips byte-stably.** `Store.Snapshot()` is + byte-identical for identical state; `TestProposalStoreRoundTripIsByteStable` + loads and rewrites four times and requires no byte to move, then re-files the + identical proposal at a *different* `--now` and asserts the store does not + grow — proposals are content-addressed and `CreatedAt` is outside the + fingerprint, so a nightly loop that re-derives the same candidate is + idempotent rather than a duplicate factory. +* **No map iteration in any output.** `whatifUsage()` walks `whatif.AllAxes`, + `writeScorecard` sorts refusal codes (reused from `backtest.go`), `List()` + arrives sorted and is not re-sorted, and `Result.Changes` is in `AllAxes` + order. `TestWhatIfHelpQuotesTheEnforcedBounds` asserts the help text is + byte-stable across five renders and in `AllAxes` order. +* **No network, no cluster, no credential** on any path, including tests. + +--- + +## 6. Adapters written, and the mismatch that forced each + +### 6.1 `applyAxisSets` — because `Axis.get`/`Axis.set` are unexported + +`whatif.Axis` is exported, `Known()` is exported, `HardBounds()` is exported — +but the projection from an axis onto a policy field is not. So `cmd/` restates +the five-field mapping, which is exactly the kind of duplication that goes +wrong silently: set `MemoryHeadroom` when the caller said `cpu-headroom` and +everything downstream is internally consistent and wrong. + +**The cross-check is `pkg/whatif`'s own projection.** +`TestSetMovesTheAxisPkgWhatifThinksItMoves` runs each axis through the CLI and +asserts `Result.Changes` — computed by `changesBetween`, which uses `Axis.get` +— names the axis the flag named and carries the value the flag carried. Then +all five at once, asserting exactly five moved, in `AllAxes` order. A mis-mapped +field fails the build's tests rather than quietly tuning the wrong knob under +the right name. + +`--set` values are additionally checked against `whatif.HardBounds()` at the +CLI, and the gate re-checks the candidate against the (narrower) declared +envelope. Producer and checker are different code on purpose, which is the +package's own argument for checking the envelope at acceptance and not only at +generation. + +### 6.2 `loadWhatIfPolicy` — reusing `backtest`'s loader, refusing one field + +The policy-file format is `cmd/kilter/backtest.go`'s `policyFile`: pointer +fields overlaid onto package defaults (so absent means default, not zero), +`DisallowUnknownFields`, and a bare number where a duration belongs rejected by +name. Reusing it rather than writing a second format is deliberate — a policy +file should mean the same thing to both commands. + +Detecting whether the file *set* `enforceDecisionRefusals` (§3.6) is done by +loading twice with opposite defaults: if the file pinned the field both agree, +if it left the field out they differ. That is cheaper and less brittle than a +second parser for one boolean, and it is commented as such at the call site. + +### 6.3 The proposal store is a file, and it is not trusted + +`pkg/whatif` owns no file, no bucket and no schema migration by design: +`Snapshot()` and `Load()` move bytes and "cmd/ decides where they live". So +`--store PATH` is a local JSON file — the choice the package explicitly +delegates, and one that touches neither `pkg/store` nor `pkg/api`. + +The file is **not** trusted on the way back in. `whatif.Load` revalidates every +record: the ID must be the fingerprint the contents actually hash to, an +approved record must carry an approval bound to that fingerprint *and* that +verdict granted by a human who is not the author, a gated record must actually +have passed the gate, and unknown fields are rejected. +`TestATamperedProposalStoreDoesNotLoad` runs five concrete edits an attacker +with write access would make — assert `"state": "approved"`, flip the gate +verdict, corrupt the regret numbers, invent a state, splice in an extra record +— and each fails to load with the store named. That property is what makes a +plain file an acceptable home for this artefact at all. + +Writes go to a temp file in the same directory and are renamed over the target: +a store truncated half-way through a write is a store that no longer loads, and +this file is the only record that a proposal was ever gated. Mode `0600`, +because it carries approvals and an audit trail. + +--- + +## 7. What the API-route unit must still do + +Not built here. `pkg/api` and `pkg/store` are another agent's this cycle, and +the six routes plus the nightly tuner wiring are explicitly out of scope. What +this unit learned that whoever builds them will need: + +### 7.1 The six routes + +``` +GET /api/v1/clusters/{id}/whatif?from=&to=&horizon=&candidate= → Result JSON +GET /api/v1/proposals?cluster=&state= → []Record +GET /api/v1/proposals/{id} → Record +POST /api/v1/proposals (authWrite) body: Spec-ish → Record +POST /api/v1/proposals/{id}/approvals (authWrite, HUMAN token tier only) → Record +POST /api/v1/proposals/{id}/rejections (authWrite) → Record +POST /api/v1/proposals/{id}/applied (authWrite, system) → Record +``` + +* **`GET .../whatif` is blocked on the same seam as `--cluster`.** It cannot be + served over the one snapshot `pkg/store` keeps, for the reason in §3.1. It is + blocked on nothing else: `whatif.Scenario` is a struct literal and `Run()` is + one call. +* **The two GETs on `/proposals` are pure projections** of `Store.List()` / + `Get()`. `Record` has `MarshalJSON`, so the wire form is the package's, not a + hand-rolled one — `cmd/kilter/proposals.go` emits it verbatim and asserts as + much with a `var _ json.Marshaler = (*whatif.Record)(nil)`. Lift the + rendering, do not re-derive it. +* **`POST /proposals` must not accept a verdict.** The body is a `Spec`, which + carries scorecards and no `GateResult`; `Store.Create` runs `Decide` itself. + If the handler ever grows a "gate" field in its request body, that is the + regression. +* **`POST .../approvals` is the whole security boundary.** Everything the CLI + refuses in §3.3 is available to an API that authenticates. The token tier + must be one the MCP server (§6) and the reasoner (§5) do not hold, and the + actor must come from the token, never from the body — a `{"by": {"kind": + "human"}}` field in a request body is `--author-id` pointed the dangerous + way. `whatif.NewApprover` is the only constructor; if any handler builds an + `Actor{Kind: ActorHuman}` from request-supplied data, the guarantee is gone. +* **`POST .../applied` owes a `LedgerEntry`** in the same transaction: proposal + ID, both policy hashes, the claimed `Delta.ProjectedMonthlyUSD`. Without it + the claimed-vs-measured join can never score the tuner itself, which is the + only mechanism that would ever catch a tuner that is confidently wrong. + +### 7.2 Persistence + +`Store.Snapshot()` / `whatif.Load()` move the whole store as bytes, and both +are already round-trip and byte-identity tested inside the package. The bbolt +`proposals` bucket is a one-key-per-cluster (or one-blob) write of those bytes; +the store is capped at 1,000 records, so size is bounded by construction. The +CLI's file store is the same bytes — a `--store` file can be read by +`whatif.Load` and vice versa, so the two are interchangeable for testing. + +Call `store.Sweep(clock)` on the brain's existing housekeeping timer so expired +approvals move to `expired` rather than lingering as `approved`. +`MarkApplied` checks expiry independently, so an unswept store is safe, just +untidy. + +### 7.3 Two prerequisites shared with earlier units + +* **Snapshot history** (§3.1) is the same seam `kilter backtest --cluster` and + the explain/why-cost routes are waiting on — `cmd/WIRING-FINDINGS.md` §6.2 + and §6.3. One piece of work unblocks four features. +* **The nightly tuner** needs a `Scenario` pre-filled with `History`, + `Evidence`, `Catalog`, `Scoring` and `Cluster`, run once per cluster, with + `historyEnd` from the newest snapshot — so it needs snapshot history too. + Construct `whatif.NewTuner(cfg, store)` **unconditionally** at brain start so + a disabled-but-invalid config is a startup error rather than a 3am surprise, + and surface `TunerReport.Truncated` wherever the operator looks: a search + that silently covered half the grid reads exactly like one that covered all + of it. + +--- + +## 8. Smaller notes + +* **`--propose` files `Target{Cluster: …}` only.** `Target` carries `Namespace` + and `Class` and they are inside the fingerprint, but nothing narrows the + *replay* to them — `backtest.Harness` scores a whole cluster. Until scoping + the harness lands (a `pkg/backtest` change), a narrowed target would be + documentation printed on top of a cluster-wide measurement, which is why the + tuner only ever files a cluster target and why this command does the same. + No flag pretends otherwise. +* **`--propose` supplies no evidence IDs.** `Spec.EvidenceIDs` is the citation + set, and the CLI has no dossier to cite from — the trace is synthetic. + Passing the trace's own identifiers would be a citation that resolves to + nothing an operator can look up. Left empty rather than filled with + plausible-looking strings. +* **The envelope and tolerance are the shipped defaults.** + `whatif.DefaultEnvelope()` (narrower than the hard bounds on every axis) and + `DefaultTolerance()`. There is no `--envelope` flag: `Envelope.Validate` + errors rather than clamping, so a file-supplied envelope is a config surface + that wants the same "declared once at brain start" treatment the tuner's + does, not a per-invocation flag. +* **`KindAnnotationChange` is not reachable and no flag offers it.** + `Spec.normalize` rejects it with a reason; an annotation proposal's gate is + not a backtest scorecard. +* **Coverage is 54.2 % for `cmd/kilter` as a whole.** The two new files are + exercised end to end; the untested remainder is the pre-existing collectors + and the `brain`/`agent`/`controller` long-running paths. diff --git a/cmd/kilter/main.go b/cmd/kilter/main.go index 5d4473c..885b12a 100644 --- a/cmd/kilter/main.go +++ b/cmd/kilter/main.go @@ -11,6 +11,8 @@ // kilter backtest score a policy against history (falsifiability harness) // kilter explain the full explain payload behind one recommendation // kilter why-cost an additive decomposition of a cluster's cost change +// kilter whatif replay history under a candidate policy and gate the delta +// kilter proposals list, inspect and reject closed-loop policy proposals // kilter version build information package main @@ -47,6 +49,8 @@ Commands: backtest Score a policy against history: oracle gap, regret, safety, gate explain Why the engine would resize a container, with citations why-cost Additive, individually-citable decomposition of a cost change + whatif Replay history under a candidate policy: delta, gate, proposal + proposals Closed-loop policy proposals: list, show, reject (approve is refused) version Print version Run "kilter -h" for command flags. @@ -91,6 +95,10 @@ func main() { err = runExplain(args) case "why-cost": err = runWhyCost(args) + case "whatif": + err = runWhatIf(args) + case "proposals": + err = runProposals(args) case "version", "--version", "-v": fmt.Printf("kilter %s (%s)\n", version, commit) case "help", "-h", "--help": diff --git a/cmd/kilter/proposals.go b/cmd/kilter/proposals.go new file mode 100644 index 0000000..49b233f --- /dev/null +++ b/cmd/kilter/proposals.go @@ -0,0 +1,477 @@ +package main + +import ( + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "os" + "strings" + "time" + + "github.com/agenticode/kilter/pkg/whatif" +) + +// `kilter proposals` — the closed-loop proposal record, made readable. +// +// A proposal is an inert artifact: "this policy scored better than the +// incumbent over this window, here is the evidence, here is the gate's +// verdict". This command lists them, shows one whole, and rejects one. It does +// NOT approve and it does NOT record an apply, and both refusals are by name — +// see proposalsApproveRefusal and proposalsAppliedRefusal. They are the point +// of the command, not gaps in it. +// +// # Where the bytes live +// +// pkg/whatif owns no file, no bucket and no schema migration on purpose: +// Store.Snapshot() and whatif.Load() move bytes and cmd/ decides where they +// go. --store names a local JSON file. That is a LOCAL artifact and says so; +// the fleet's proposals belong in pkg/store's bbolt file under a `proposals` +// bucket, which pkg/api owns and has not built (cmd/WHATIF-WIRING-FINDINGS.md). +// +// The file is not trusted on the way back in. whatif.Load re-validates every +// record: the ID must be the fingerprint the contents actually hash to, an +// approved record must carry an approval bound to that fingerprint and that +// verdict granted by a human who is not the author, and unknown fields are +// rejected. A hand-edited store fails to load rather than loading a lie, which +// is exactly the property that makes a plain file an acceptable home for this. + +const proposalsUsage = `kilter proposals — the closed-loop policy proposals and their audit trail + +Usage: + kilter proposals list --store PATH [--cluster ID] [--state STATE] [--json] + kilter proposals show --store PATH [--json] + kilter proposals reject --store PATH [--reason "..."] [--now RFC3339] + kilter proposals approve --store PATH (refused: see below) + kilter proposals applied --store PATH (refused: see below) + +States: gated | approved | rejected | applied | expired + +Flags: + --store PATH the proposal store file written by kilter whatif --propose + --cluster ID list only proposals for this cluster + --state STATE list only proposals in this state + --json emit the record(s) verbatim instead of the table + --reason TEXT why the proposal is being rejected + --now RFC3339 audit timestamp (default: the wall clock) + +approve and applied are REFUSED by this command, by name. Approving mints a +capability that only an authenticated human may hold, and a local CLI cannot +authenticate one; recording an apply asserts that a config change landed, and +nothing in this binary writes config. Run either to read the full reason. +` + +func runProposals(args []string) error { return runProposalsTo(os.Stdout, args) } + +// runProposalsTo is the testable entry point: everything printed goes to w. +func runProposalsTo(w io.Writer, args []string) error { + if len(args) == 0 { + fmt.Fprint(w, proposalsUsage) + return fmt.Errorf("proposals: a subcommand is required (list|show|reject|approve|applied)") + } + verb, rest := args[0], args[1:] + switch verb { + case "list": + return proposalsList(w, rest) + case "show": + return proposalsShow(w, rest) + case "reject": + return proposalsReject(w, rest) + case "approve": + return proposalsApproveRefusal(rest) + case "applied": + return proposalsAppliedRefusal(rest) + case "help", "-h", "--help": + fmt.Fprint(w, proposalsUsage) + return nil + default: + fmt.Fprint(w, proposalsUsage) + return fmt.Errorf("proposals: unknown subcommand %q", verb) + } +} + +// ---------------------------------------------------------------- list + +func proposalsList(w io.Writer, args []string) error { + fs := flag.NewFlagSet("proposals list", flag.ContinueOnError) + fs.SetOutput(w) + storePath := fs.String("store", "", "proposal store file") + cluster := fs.String("cluster", "", "filter by cluster") + state := fs.String("state", "", "filter by state") + jsonOut := fs.Bool("json", false, "emit the records as JSON") + if err := fs.Parse(args); err != nil { + return err + } + store, err := requireProposalStore(*storePath) + if err != nil { + return err + } + + // List() is already sorted by (CreatedAt, ID). The spec is explicit: print + // in order and do not re-sort — a second sort here would be a second + // definition of "the order proposals are read in". + recs := store.List() + if s := strings.TrimSpace(*state); s != "" { + want := whatif.State(s) + if !knownProposalState(want) { + return fmt.Errorf("proposals list --state %q: unknown state (gated|approved|rejected|applied|expired)", s) + } + recs = store.ListState(want) + } + if c := strings.TrimSpace(*cluster); c != "" { + filtered := make([]*whatif.Record, 0, len(recs)) + for _, r := range recs { + if r.Proposal().Cluster == c { + filtered = append(filtered, r) + } + } + recs = filtered + } + + if *jsonOut { + // A JSON array, always — an empty store is `[]`, never `null`, so a + // consumer cannot confuse "no proposals" with "no answer". + if recs == nil { + recs = []*whatif.Record{} + } + return writeJSON(w, recs) + } + + var b strings.Builder + fmt.Fprintf(&b, "kilter proposals — %d record(s) in %s\n\n", len(recs), *storePath) + if len(recs) == 0 { + b.WriteString(" (none)\n") + _, err := io.WriteString(w, b.String()) + return err + } + t := &table{header: []string{"ID", "STATE", "CLUSTER", "REGRET", "PROJECTED/MO", "CHANGES", "CREATED", "AUTHOR"}} + for _, r := range recs { + p := r.Proposal() + changes := make([]string, 0, len(p.Changes)) + for _, c := range p.Changes { + changes = append(changes, string(c.Axis)) + } + t.add( + r.ID(), + string(r.State()), + p.Cluster, + fmt.Sprintf("%s → %s", usd(p.BaselineRegret), usd(p.CandidateRegret)), + signedUSD(p.Delta.ProjectedMonthlyUSD), + strings.Join(changes, ","), + p.CreatedAt.UTC().Format(time.RFC3339), + p.Author.String(), + ) + } + b.WriteString(t.render(" ")) + _, err = io.WriteString(w, b.String()) + return err +} + +func knownProposalState(s whatif.State) bool { + switch s { + case whatif.StateGated, whatif.StateApproved, whatif.StateRejected, + whatif.StateApplied, whatif.StateExpired: + return true + } + return false +} + +// ---------------------------------------------------------------- show + +func proposalsShow(w io.Writer, args []string) error { + fs := flag.NewFlagSet("proposals show", flag.ContinueOnError) + fs.SetOutput(w) + storePath := fs.String("store", "", "proposal store file") + jsonOut := fs.Bool("json", false, "emit the record as JSON") + if err := fs.Parse(args); err != nil { + return err + } + id := strings.TrimSpace(fs.Arg(0)) + if id == "" { + return fmt.Errorf("proposals show: a proposal id is required") + } + store, err := requireProposalStore(*storePath) + if err != nil { + return err + } + rec, ok := store.Get(id) + if !ok { + return fmt.Errorf("proposals show %s: %w in %s", id, whatif.ErrNotFound, *storePath) + } + if *jsonOut { + // The whole Record — proposal, state, approval and audit history. This + // is the same shape GET /api/v1/proposals/{id} must serve. + return writeJSON(w, rec) + } + + var b strings.Builder + // Proposal().Encode() verbatim, per the spec: the proposal is the + // evidence, and reformatting it here would be a second rendering of one + // artifact that could drift from the one the API serves. + raw, err := rec.Proposal().Encode() + if err != nil { + return err + } + fmt.Fprintf(&b, "proposal %s — %s\n\n", rec.ID(), rec.State()) + b.Write(raw) + + b.WriteString("\naudit trail\n") + for _, tr := range rec.History() { + from := string(tr.From) + if from == "" { + from = "(new)" + } + fmt.Fprintf(&b, " %s %-9s → %-9s by %-24s %s\n", + tr.At.UTC().Format(time.RFC3339), from, tr.To, tr.By.String(), tr.Note) + } + if ap, ok := rec.Approval(); ok { + b.WriteString("\napproval\n") + fmt.Fprintf(&b, " by %s at %s, expires %s\n", + ap.By().String(), ap.At().UTC().Format(time.RFC3339), + ap.ExpiresAt().UTC().Format(time.RFC3339)) + fmt.Fprintf(&b, " bound to proposal %s, verdict %s\n", ap.Fingerprint(), ap.Verdict()) + if n := ap.Note(); n != "" { + fmt.Fprintf(&b, " note: %s\n", n) + } + } + _, err = io.WriteString(w, b.String()) + return err +} + +// ---------------------------------------------------------------- reject + +// proposalsReject is the one write this command performs, and it is safe by +// construction: pkg/whatif lets ANY actor reject, because refusing to make a +// change needs no capability. That asymmetry is why reject is implemented here +// and approve is not — the thing a CLI cannot supply is an authenticated +// human, and rejection does not need one. +// +// The actor is Actor{Kind: ActorSystem, ID: "kilter-cli"} rather than a +// human: the command cannot prove a person ran it, and an audit trail that +// says "human:alice rejected this" when a script did is a worse record than +// one that says where the rejection came from. +func proposalsReject(w io.Writer, args []string) error { + fs := flag.NewFlagSet("proposals reject", flag.ContinueOnError) + fs.SetOutput(w) + storePath := fs.String("store", "", "proposal store file") + reason := fs.String("reason", "", "why the proposal is being rejected") + nowFlag := fs.String("now", "", "audit timestamp as RFC3339 (default: now)") + if err := fs.Parse(args); err != nil { + return err + } + id := strings.TrimSpace(fs.Arg(0)) + if id == "" { + return fmt.Errorf("proposals reject: a proposal id is required") + } + store, err := requireProposalStore(*storePath) + if err != nil { + return err + } + now, err := parseNowFlag(*nowFlag) + if err != nil { + return err + } + by := whatif.Actor{Kind: whatif.ActorSystem, ID: "kilter-cli"} + rec, err := store.Reject(by, id, *reason, whatif.FixedClock(now)) + if err != nil { + return fmt.Errorf("proposals reject %s: %w", id, err) + } + if err := saveProposalStore(*storePath, store); err != nil { + return err + } + fmt.Fprintf(w, "proposal %s is now %s (by %s)\n", rec.ID(), rec.State(), by) + return nil +} + +// ---------------------------------------------------------------- refusals + +// proposalsApproveRefusal is the sharpest line in this wiring. +// +// pkg/whatif made self-approval UNREPRESENTABLE rather than merely forbidden: +// Approval has no exported fields, no UnmarshalJSON, and the only route to one +// is Store.Approve, which needs an *Approver, which NewApprover will not build +// for a non-human actor. That is a type-system guarantee. It rests on exactly +// one thing the type system cannot supply — authentication — and pkg/whatif's +// FINDINGS names it: "the actor must come from the authenticated caller, not +// from a flag ... authentication is the API layer's job and is the one thing +// that can undo the guarantee". +// +// A local CLI has nothing to authenticate with. The identities within reach — +// $USER, os/user.Current, the uid — describe the SESSION, not the presence of +// a person, and everything running in that session inherits them, including +// unit 8's reasoner, which is precisely the actor the human-only rule exists +// to exclude. Minting Actor{Kind: ActorHuman} from any of them would convert a +// structural guarantee into a convention that `sh -c` walks around. +func proposalsApproveRefusal(args []string) error { + id := refusedProposalID("proposals approve", args) + return fmt.Errorf(`proposals approve %s: refused — a local CLI cannot authenticate a human approver. + +whatif.NewApprover requires an Actor{Kind: human}, and pkg/whatif's guarantee is +that self-approval is not a rule that gets checked but a value that cannot be +constructed. The guarantee rests on one input the type system cannot supply: +proof that a human is on the other end. + +This process has none. $USER, os/user.Current and the uid describe the SESSION, +not a person — anything running in that session inherits them, including unit +8's reasoner, which is the exact actor the human-only rule exists to exclude. +Minting Actor{Kind: ActorHuman, ID: $USER} here would turn a type-system +guarantee into a convention that a shell command walks around, and it would do +it silently, in the direction that grants capability. + +Approval belongs behind the authenticated funnel, at a token tier the MCP +server (§6) and the reasoner (§5) do not hold: + + POST /api/v1/proposals/{id}/approvals (authWrite, HUMAN token tier only) + +See pkg/whatif/FINDINGS.md, "Brain wiring (pkg/api)" and "Approval is +structural, not procedural", and cmd/WHATIF-WIRING-FINDINGS.md. + +What this command can do, because it needs no capability: + kilter proposals reject %s --reason "..." --store PATH`, id, id) +} + +// proposalsAppliedRefusal explains why the after-the-fact record is not here. +// +// Two independent reasons, and the second is the one that matters: even if the +// record were harmless, nothing in this binary can produce an APPROVED +// proposal to apply, because approve refuses. A verb that could only ever +// return "proposal X is gated; only an approved proposal can be applied" is +// better replaced by the reason it can never do anything else. +func proposalsAppliedRefusal(args []string) error { + id := refusedProposalID("proposals applied", args) + return fmt.Errorf(`proposals applied %s: refused — nothing in this binary applies a policy change. + +MarkApplied records, AFTER THE FACT, that a config change actually landed, and +§4.6 requires a ledger entry written in the same breath — the proposal ID, both +policy hashes and the claimed Delta.ProjectedMonthlyUSD — because that entry is +what later lets the existing claimed-vs-measured join score the tuner itself. +This command writes no config and no ledger entry, so recording "applied" from +here would put an unbacked fact into an audit trail whose whole value is that +its facts are backed. + +It is also unreachable: only an approved proposal can be applied, and nothing +in this CLI can produce one (kilter proposals approve %s explains why). + +Where it belongs, next to the writer: + POST /api/v1/proposals/{id}/applied (authWrite, system) +plus the LedgerEntry. See pkg/whatif/FINDINGS.md, "Brain wiring (pkg/api)".`, id, id) +} + +// refusedProposalID recovers the id from a refused verb's arguments so the +// refusal can name the proposal the caller meant. The same flags the working +// verbs accept are parsed and discarded, because `--store PATH ` must not +// report PATH as the id — a refusal that quotes the wrong thing back reads as +// a parsing bug rather than as a decision. +func refusedProposalID(verb string, args []string) string { + fs := flag.NewFlagSet(verb, flag.ContinueOnError) + fs.SetOutput(io.Discard) + fs.String("store", "", "proposal store file") + fs.String("note", "", "approver's note") + fs.String("now", "", "audit timestamp as RFC3339") + if err := fs.Parse(args); err == nil { + if id := strings.TrimSpace(fs.Arg(0)); id != "" { + return id + } + } + return "" +} + +// ---------------------------------------------------------------- the file + +// requireProposalStore loads an EXISTING store, or refuses. +// +// A missing file is an error rather than an empty store on purpose: "0 +// proposals" and "you named a file that is not there" render identically as an +// empty list, and the first reads as a fact about the fleet. +func requireProposalStore(path string) (*whatif.Store, error) { + if strings.TrimSpace(path) == "" { + return nil, errors.New(`proposals: --store PATH is required. + +pkg/whatif owns no file by design — Snapshot() and Load() move bytes and cmd/ +decides where they live — so the store has to be named. It is written by +` + "`kilter whatif --propose --store PATH`" + `. + +That file is a LOCAL artifact. The fleet's proposals belong in pkg/store's +bbolt file under a ` + "`proposals`" + ` bucket (pkg/whatif/FINDINGS.md, "Brain +wiring"), which is pkg/api's to add and is not wired yet.`) + } + raw, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("--store: %w", err) + } + store, err := whatif.Load(raw) + if err != nil { + // whatif.Load re-validates every invariant, so this branch is also + // the tamper detector: a hand-edited "state": "approved" fails here. + return nil, fmt.Errorf("--store %s: %w", path, err) + } + return store, nil +} + +// openProposalStore loads the store at path, or returns an empty one when the +// file does not exist yet — the `kilter whatif --propose` case, where creating +// the file is the point. Any other read error is reported: silently starting +// fresh on a permissions error would discard a pending approval. +func openProposalStore(path string) (*whatif.Store, error) { + raw, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return whatif.NewStore(), nil + } + if err != nil { + return nil, fmt.Errorf("--store: %w", err) + } + store, err := whatif.Load(raw) + if err != nil { + return nil, fmt.Errorf("--store %s: %w", path, err) + } + return store, nil +} + +// saveProposalStore writes the store back. +// +// Snapshot() is byte-identical for identical state, so rewriting an unchanged +// store produces an unchanged file. The write goes to a temporary file in the +// same directory and is renamed over the target, because a store truncated +// half-way through a write is a store that no longer loads — and this file is +// the only record that a proposal was ever gated. +func saveProposalStore(path string, store *whatif.Store) error { + raw, err := store.Snapshot() + if err != nil { + return fmt.Errorf("--store %s: %w", path, err) + } + tmp, err := os.CreateTemp(dirOf(path), ".kilter-proposals-*") + if err != nil { + return fmt.Errorf("--store %s: %w", path, err) + } + name := tmp.Name() + defer os.Remove(name) + if _, err := tmp.Write(raw); err != nil { + tmp.Close() + return fmt.Errorf("--store %s: %w", path, err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("--store %s: %w", path, err) + } + // 0600: the file carries approvals and an audit trail. + if err := os.Chmod(name, 0o600); err != nil { + return fmt.Errorf("--store %s: %w", path, err) + } + if err := os.Rename(name, path); err != nil { + return fmt.Errorf("--store %s: %w", path, err) + } + return nil +} + +// dirOf is filepath.Dir with an empty path meaning the working directory. +func dirOf(path string) string { + if i := strings.LastIndexByte(path, '/'); i >= 0 { + return path[:i] + } + return "." +} + +// compile-time assertion that the record shape the CLI prints is the one +// pkg/whatif marshals: Record has MarshalJSON, so writeJSON emits the +// package's own wire form rather than a cmd/-side projection of it. +var _ json.Marshaler = (*whatif.Record)(nil) diff --git a/cmd/kilter/proposals_test.go b/cmd/kilter/proposals_test.go new file mode 100644 index 0000000..94a69c4 --- /dev/null +++ b/cmd/kilter/proposals_test.go @@ -0,0 +1,542 @@ +package main + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/agenticode/kilter/pkg/whatif" +) + +// These tests drive pkg/whatif's REAL Store, state machine and codec through +// the REAL CLI entry point. Every timestamp is supplied with --now, so the +// store's bytes are a function of the arguments alone. + +const proposalNow = "2026-08-26T12:00:00Z" + +func runProposalsOK(t *testing.T, args ...string) string { + t.Helper() + var b strings.Builder + if err := runProposalsTo(&b, args); err != nil { + t.Fatalf("kilter proposals %s: %v\n%s", strings.Join(args, " "), err, b.String()) + } + return b.String() +} + +func runProposalsErr(t *testing.T, args ...string) (string, error) { + t.Helper() + var b strings.Builder + err := runProposalsTo(&b, args) + if err == nil { + t.Fatalf("kilter proposals %s was accepted:\n%s", strings.Join(args, " "), b.String()) + } + return b.String(), err +} + +// fileAProposal runs `kilter whatif --propose` into a fresh store and returns +// the store path and the proposal id. +func fileAProposal(t *testing.T, extra ...string) (string, string) { + t.Helper() + path := filepath.Join(t.TempDir(), "proposals.json") + args := append(append([]string{}, acceptedArgs...), + "--propose", "--store", path, "--now", proposalNow, + "--rationale", "cpu headroom at the bottom of the envelope is cheaper on this trace") + args = append(args, extra...) + out := runWhatIfOK(t, args...) + id := proposalIDFrom(t, out) + return path, id +} + +func proposalIDFrom(t *testing.T, out string) string { + t.Helper() + for _, line := range strings.Split(out, "\n") { + if rest, ok := strings.CutPrefix(line, "filed proposal "); ok { + return strings.Fields(rest)[0] + } + } + t.Fatalf("no proposal id in:\n%s", out) + return "" +} + +// TestWhatIfProposeFilesAGatedProposalTheStoreCanRead is the round trip the +// whole unit exists for: replay → gate → proposal → a record a human reads. +func TestWhatIfProposeFilesAGatedProposalTheStoreCanRead(t *testing.T) { + path, id := fileAProposal(t) + + raw := runProposalsOK(t, "list", "--store", path, "--json") + var recs []json.RawMessage + if err := json.Unmarshal([]byte(raw), &recs); err != nil { + t.Fatalf("decode list: %v\n%s", err, raw) + } + if len(recs) != 1 { + t.Fatalf("listed %d records, want 1", len(recs)) + } + + shown := runProposalsOK(t, "show", "--store", path, "--json", id) + var rec struct { + ID string `json:"id"` + State string `json:"state"` + Proposal whatif.Proposal `json:"proposal"` + Approval json.RawMessage `json:"approval"` + History []struct { + From, To string + By whatif.Actor + } `json:"history"` + } + if err := json.Unmarshal([]byte(shown), &rec); err != nil { + t.Fatalf("decode record: %v\n%s", err, shown) + } + if rec.ID != id { + t.Errorf("record id %q, want %q", rec.ID, id) + } + if rec.State != string(whatif.StateGated) { + t.Errorf("state %q, want gated", rec.State) + } + if rec.Approval != nil { + t.Errorf("a freshly gated proposal carries an approval: %s", rec.Approval) + } + if !rec.Proposal.Gate.Passed { + t.Errorf("a gated proposal whose gate did not pass: %v", rec.Proposal.Gate.Reasons) + } + if rec.Proposal.Kind != whatif.KindPolicyChange { + t.Errorf("kind %q, want %q", rec.Proposal.Kind, whatif.KindPolicyChange) + } + if rec.Proposal.Rationale == "" { + t.Error("--rationale did not reach the proposal") + } + // The audit trail reads as a lifecycle, and the gate is its own actor. + if len(rec.History) != 2 || + rec.History[0].To != string(whatif.StateDraft) || + rec.History[1].To != string(whatif.StateGated) { + t.Fatalf("history = %+v, want (new)→draft→gated", rec.History) + } + if rec.History[1].By.Kind != whatif.ActorSystem { + t.Errorf("the gating transition was attributed to %v, not the system", rec.History[1].By) + } + + // The human form prints the proposal verbatim plus the audit trail. + human := runProposalsOK(t, "show", "--store", path, id) + for _, want := range []string{id, "gated", "audit trail", "→ gated", `"kind": "policy-change"`} { + if !strings.Contains(human, want) { + t.Errorf("show does not print %q:\n%s", want, human) + } + } +} + +// TestTheProposerCannotSupplyTheVerdict. +// +// Result.Spec deliberately omits the GateResult and Store.Create runs Decide +// itself, so a caller hands over evidence and receives a judgment. Through the +// CLI: the verdict on the filed record must be the one the what-if computed, +// and a REJECTED comparison must file as rejected rather than as gated. +func TestTheProposerCannotSupplyTheVerdict(t *testing.T) { + path := filepath.Join(t.TempDir(), "proposals.json") + out := runWhatIfOK(t, append(append([]string{}, rejectedArgs...), + "--propose", "--store", path, "--now", proposalNow)...) + if !strings.Contains(out, "REJECTED") { + t.Fatalf("the rejected case no longer fails the gate:\n%s", out) + } + id := proposalIDFrom(t, out) + if !strings.Contains(out, "the gate rejected it") { + t.Errorf("the filing did not say the gate rejected it:\n%s", out) + } + shown := runProposalsOK(t, "show", "--store", path, "--json", id) + if !strings.Contains(shown, `"state":"rejected"`) && + !strings.Contains(shown, `"state": "rejected"`) { + t.Errorf("a gate-rejected proposal was not filed as rejected:\n%s", shown) + } + // It is still FILED: a rejected proposal is the record of a question that + // was asked and answered, and discarding it would make a tuner that ran + // look like one that never did. + if listed := runProposalsOK(t, "list", "--store", path); !strings.Contains(listed, id) { + t.Errorf("the rejected proposal was not listed:\n%s", listed) + } +} + +// TestProposalsApproveIsRefusedByName. +// +// pkg/whatif made self-approval unrepresentable in the type system. A CLI that +// minted Actor{Kind: ActorHuman} from $USER would reintroduce it, because +// anything in that session inherits the identity — including unit 8's +// reasoner, which is the exact actor the human-only rule exists to exclude. +func TestProposalsApproveIsRefusedByName(t *testing.T) { + path, id := fileAProposal(t) + before, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + + out, err := runProposalsErr(t, "approve", "--store", path, id) + msg := err.Error() + for _, want := range []string{ + id, "refused", "authenticate", "NewApprover", "$USER", + "/api/v1/proposals/{id}/approvals", "HUMAN token tier", + "kilter proposals reject", + } { + if !strings.Contains(msg, want) { + t.Errorf("the refusal does not mention %q:\n%s", want, msg) + } + } + if strings.Contains(out, "approved") { + t.Errorf("approve printed something that reads as success:\n%s", out) + } + // Nothing was written, and the record did not move. + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Error("the refused approve rewrote the store") + } + if shown := runProposalsOK(t, "show", "--store", path, "--json", id); !strings.Contains( + shown, string(whatif.StateGated)) { + t.Errorf("the record moved out of gated:\n%s", shown) + } +} + +// TestProposalsAppliedIsRefusedByName. Recording an apply asserts that a +// config change landed and owes §4.6 a ledger entry; this binary writes +// neither. It is also unreachable, since nothing here can approve. +func TestProposalsAppliedIsRefusedByName(t *testing.T) { + path, id := fileAProposal(t) + _, err := runProposalsErr(t, "applied", "--store", path, id) + msg := err.Error() + for _, want := range []string{ + id, "refused", "LedgerEntry", "claimed-vs-measured", + "/api/v1/proposals/{id}/applied", "only an approved proposal", + } { + if !strings.Contains(msg, want) { + t.Errorf("the refusal does not mention %q:\n%s", want, msg) + } + } +} + +// TestNoCLIPathReachesApprovedOrApplied is the property behind the two +// refusals above, stated directly: INV-4's terminal states are unreachable +// from this binary, whatever sequence of commands is run. +func TestNoCLIPathReachesApprovedOrApplied(t *testing.T) { + path, id := fileAProposal(t) + for _, args := range [][]string{ + {"approve", "--store", path, id}, + {"applied", "--store", path, id}, + {"approve", "--store", path, "--note", "please", id}, + {"list", "--store", path, "--state", "approved"}, + } { + var b strings.Builder + _ = runProposalsTo(&b, args) + if strings.Contains(b.String(), `"state": "approved"`) || + strings.Contains(b.String(), `"state": "applied"`) { + t.Errorf("%v produced an approved/applied record:\n%s", args, b.String()) + } + } + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for _, forbidden := range []string{`"approved"`, `"applied"`, `"approval"`} { + if strings.Contains(string(raw), forbidden) { + t.Errorf("the store on disk contains %s after a CLI-only session:\n%s", forbidden, raw) + } + } + // And the author is never a human, so the audit trail never claims a + // person filed what a command did. + if strings.Contains(string(raw), `"kind":"human"`) || + strings.Contains(string(raw), `"kind": "human"`) { + t.Errorf("the CLI synthesized a human identity:\n%s", raw) + } + _ = id +} + +// TestAuthorIDCanOnlyRestrict. +// +// whatif.sameIdentity compares actor IDs and deliberately IGNORES Kind, so an +// operator who names themselves with --author-id is BLOCKED from later +// approving that proposal through the authenticated funnel. That direction is +// the safe one, which is why the AUTHOR identity is a flag and the APPROVER +// identity is not. +func TestAuthorIDCanOnlyRestrict(t *testing.T) { + path, id := fileAProposal(t, "--author-id", "alice") + shown := runProposalsOK(t, "show", "--store", path, "--json", id) + var rec struct { + Proposal struct { + Author whatif.Actor `json:"author"` + } `json:"proposal"` + } + if err := json.Unmarshal([]byte(shown), &rec); err != nil { + t.Fatal(err) + } + if rec.Proposal.Author.ID != "alice" { + t.Errorf("author id = %q, want alice", rec.Proposal.Author.ID) + } + if rec.Proposal.Author.Kind == whatif.ActorHuman { + t.Error("--author-id minted a human actor; a CLI cannot authenticate one") + } + + // The restriction is real: an approver with the same identity is refused + // by pkg/whatif itself, whatever the kind. + store, err := requireProposalStore(path) + if err != nil { + t.Fatal(err) + } + ap, err := whatif.NewApprover(whatif.Actor{Kind: whatif.ActorHuman, ID: "Alice"}) + if err != nil { + t.Fatal(err) + } + if _, err := store.Approve(ap, id, "mine", whatif.FixedClock(mustTime(t, proposalNow))); err == nil { + t.Fatal("the author approved their own proposal") + } else if !strings.Contains(err.Error(), "cannot be approved by its author") { + t.Errorf("err = %v, want the self-approval refusal", err) + } + // A different human is not blocked — the rule is author≠approver, not + // "nobody may approve". + other, err := whatif.NewApprover(whatif.Actor{Kind: whatif.ActorHuman, ID: "bob"}) + if err != nil { + t.Fatal(err) + } + if _, err := store.Approve(other, id, "reviewed", whatif.FixedClock(mustTime(t, proposalNow))); err != nil { + t.Fatalf("a non-author human could not approve: %v", err) + } +} + +// TestProposalsRejectIsTheOneWriteTheCLIMakes. Any actor may reject: refusing +// to make a change is always safe, so it needs no capability — which is +// exactly why it is here and approve is not. +func TestProposalsRejectIsTheOneWriteTheCLIMakes(t *testing.T) { + path, id := fileAProposal(t) + out := runProposalsOK(t, "reject", "--store", path, "--reason", "not this quarter", + "--now", "2026-08-27T09:00:00Z", id) + if !strings.Contains(out, "rejected") { + t.Errorf("reject did not report the new state:\n%s", out) + } + shown := runProposalsOK(t, "show", "--store", path, id) + if !strings.Contains(shown, "not this quarter") { + t.Errorf("the reason did not reach the audit trail:\n%s", shown) + } + if !strings.Contains(shown, "gated") || !strings.Contains(shown, "rejected") { + t.Errorf("the transition is not in the audit trail:\n%s", shown) + } + // Rejection is terminal: there is no way back. + if _, err := runProposalsErr(t, "reject", "--store", path, id); !strings.Contains( + err.Error(), "cannot go rejected") { + t.Errorf("a second rejection was not refused: %v", err) + } + // And the store still loads, with every invariant revalidated. + if _, err := requireProposalStore(path); err != nil { + t.Errorf("the store no longer loads after a reject: %v", err) + } +} + +// TestATamperedProposalStoreDoesNotLoad. +// +// A file is bytes, and this one is the only record that a proposal was gated. +// whatif.Load revalidates everything on the way in; this asserts the CLI goes +// through that door rather than around it. Without it, "state": "approved" is +// three keystrokes away from a policy change nobody approved. +func TestATamperedProposalStoreDoesNotLoad(t *testing.T) { + path, id := fileAProposal(t) + good, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct{ name, from, to string }{ + {"assert approved", `"state": "gated"`, `"state": "approved"`}, + {"flip the verdict", `"passed": true`, `"passed": false`}, + {"improve the numbers", `"candidateRegretUSD"`, `"candidateRegretUSD_x"`}, + {"invent a state", `"state": "gated"`, `"state": "blessed"`}, + {"add a field", `"records": [`, `"records": [{"override": true},`}, + } { + t.Run(tc.name, func(t *testing.T) { + tampered := strings.Replace(string(good), tc.from, tc.to, 1) + if tampered == string(good) { + t.Fatalf("the edit %q did not apply; the store format changed", tc.from) + } + if err := os.WriteFile(path, []byte(tampered), 0o600); err != nil { + t.Fatal(err) + } + out, err := runProposalsErr(t, "list", "--store", path) + if !strings.Contains(err.Error(), "--store") { + t.Errorf("err = %v, want the store to be named", err) + } + if strings.Contains(out, id) { + t.Errorf("a tampered store still listed a record:\n%s", out) + } + }) + } +} + +// TestProposalStoreRoundTripIsByteStable. Snapshot() is byte-identical for +// identical state, so re-reading and rewriting an unchanged store must not +// move a byte — which is what makes the file diffable and a re-file +// idempotent rather than a duplicate factory. +func TestProposalStoreRoundTripIsByteStable(t *testing.T) { + path, id := fileAProposal(t) + first, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + for i := 0; i < 4; i++ { + store, err := requireProposalStore(path) + if err != nil { + t.Fatal(err) + } + if err := saveProposalStore(path, store); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != string(first) { + t.Fatalf("round trip %d moved bytes", i) + } + } + // Filing the identical proposal again is idempotent: proposals are + // content-addressed, so the same evidence is the same proposal. + args := append(append([]string{}, acceptedArgs...), + "--propose", "--store", path, "--now", "2026-09-01T00:00:00Z", + "--rationale", "cpu headroom at the bottom of the envelope is cheaper on this trace") + again := runWhatIfOK(t, args...) + if got := proposalIDFrom(t, again); got != id { + t.Errorf("re-filing produced a second id %s (was %s); CreatedAt must not be in "+ + "the fingerprint", got, id) + } + listed := runProposalsOK(t, "list", "--store", path, "--json") + var recs []json.RawMessage + if err := json.Unmarshal([]byte(listed), &recs); err != nil { + t.Fatal(err) + } + if len(recs) != 1 { + t.Errorf("re-filing grew the store to %d records", len(recs)) + } +} + +// TestProposalsListIsDeterministicAndFiltered. Store.List is already sorted by +// (CreatedAt, ID); the CLI prints in that order and does not re-sort. +func TestProposalsListIsDeterministicAndFiltered(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "proposals.json") + file := func(demo, set, now string) string { + out := runWhatIfOK(t, "--demo", demo, "--set", set, + "--propose", "--store", path, "--now", now) + return proposalIDFrom(t, out) + } + first := file("bursty", "cpu-headroom=1.05", "2026-08-26T12:00:00Z") + second := file("diurnal", "cpu-headroom=1.05", "2026-08-26T13:00:00Z") + third := file("regime-change", "cpu-headroom=1.30", "2026-08-26T14:00:00Z") + + base := runProposalsOK(t, "list", "--store", path) + for i := 0; i < 5; i++ { + if got := runProposalsOK(t, "list", "--store", path); got != base { + t.Fatalf("list run %d differs", i) + } + } + if a, b, c := strings.Index(base, first), strings.Index(base, second), + strings.Index(base, third); !(a < b && b < c) { + t.Errorf("list is not in (CreatedAt, ID) order: %d %d %d\n%s", a, b, c, base) + } + + // --state and --cluster narrow it, and an unknown state is refused rather + // than silently matching nothing. + gated := runProposalsOK(t, "list", "--store", path, "--state", "gated") + if strings.Contains(gated, third) { + t.Errorf("the rejected proposal was listed as gated:\n%s", gated) + } + if !strings.Contains(gated, first) || !strings.Contains(gated, second) { + t.Errorf("a gated proposal was missing from --state gated:\n%s", gated) + } + byCluster := runProposalsOK(t, "list", "--store", path, "--cluster", "demo-diurnal") + if !strings.Contains(byCluster, second) || strings.Contains(byCluster, first) { + t.Errorf("--cluster did not filter:\n%s", byCluster) + } + if _, err := runProposalsErr(t, "list", "--store", path, "--state", "blessed"); !strings.Contains( + err.Error(), "unknown state") { + t.Errorf("an unknown --state was accepted: %v", err) + } + // An empty result is `[]`, never `null`. + empty := runProposalsOK(t, "list", "--store", path, "--cluster", "nowhere", "--json") + if strings.TrimSpace(empty) != "[]" { + t.Errorf("an empty JSON listing is %q, want []", strings.TrimSpace(empty)) + } +} + +// TestProposalsRefusesAMissingStoreRatherThanShowingNothing. +// +// "0 proposals" and "you named a file that is not there" render identically as +// an empty list, and the first reads as a fact about the fleet. +func TestProposalsRefusesAMissingStoreRatherThanShowingNothing(t *testing.T) { + missing := filepath.Join(t.TempDir(), "nope.json") + for _, args := range [][]string{ + {"list", "--store", missing}, + {"show", "--store", missing, "abc"}, + {"reject", "--store", missing, "abc"}, + } { + out, err := runProposalsErr(t, args...) + if !strings.Contains(err.Error(), "--store") { + t.Errorf("%v: err = %v", args, err) + } + if strings.Contains(out, "(none)") { + t.Errorf("%v printed an empty listing for a missing file:\n%s", args, out) + } + } + // No --store at all names where the brain's proposals belong. + _, err := runProposalsErr(t, "list") + for _, want := range []string{"--store PATH is required", "pkg/store", "proposals", "bucket"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the refusal does not mention %q:\n%s", want, err) + } + } + // --propose without a store refuses rather than printing an id for a + // record nobody kept. + _, werr := runWhatIfErr(t, append(append([]string{}, acceptedArgs...), "--propose")...) + for _, want := range []string{"--store PATH is required", "bbolt", "proposals"} { + if !strings.Contains(werr.Error(), want) { + t.Errorf("the --propose refusal does not mention %q:\n%s", want, werr) + } + } +} + +// TestProposalsUnknownSubcommand. +func TestProposalsUnknownSubcommand(t *testing.T) { + out, err := runProposalsErr(t, "frobnicate") + if !strings.Contains(err.Error(), "unknown subcommand") { + t.Errorf("err = %v", err) + } + if !strings.Contains(out, "kilter proposals list") { + t.Errorf("the usage text was not printed:\n%s", out) + } + if _, err := runProposalsErr(t); !strings.Contains(err.Error(), "subcommand is required") { + t.Errorf("no subcommand: err = %v", err) + } + if _, err := runProposalsErr(t, "show", "--store", "x"); !strings.Contains( + err.Error(), "id is required") { + t.Errorf("show with no id: err = %v", err) + } + // A refused verb still names the proposal the caller meant, not the + // --store value. + path, id := fileAProposal(t) + _, err = runProposalsErr(t, "approve", "--store", path, id) + if !strings.Contains(err.Error(), "approve "+id) { + t.Errorf("the refusal quoted the wrong id:\n%s", err) + } +} + +// TestProposalsShowUnknownID. +func TestProposalsShowUnknownID(t *testing.T) { + path, _ := fileAProposal(t) + if _, err := runProposalsErr(t, "show", "--store", path, "0000000000000000"); !strings.Contains( + err.Error(), "no such proposal") { + t.Errorf("err = %v, want ErrNotFound", err) + } +} + +func mustTime(t *testing.T, s string) time.Time { + t.Helper() + v, err := parseNowFlag(s) + if err != nil { + t.Fatal(err) + } + return v +} diff --git a/cmd/kilter/testdata/whatif-bursty.json b/cmd/kilter/testdata/whatif-bursty.json new file mode 100644 index 0000000..d9eb12b --- /dev/null +++ b/cmd/kilter/testdata/whatif-bursty.json @@ -0,0 +1,213 @@ +{ + "cluster": "demo-bursty", + "window": [ + "2026-01-05T00:00:00Z", + "2026-01-12T00:00:00Z" + ], + "horizonHours": 24, + "baselinePolicy": { + "recommend": { + "CPUPercentile": 0.95, + "CPUHeadroom": 1.15, + "MemoryPercentile": 0.99, + "MemoryHeadroom": 1.2, + "OOMBumpRatio": 1.5, + "MinMilliCPU": 10, + "MinMemoryBytes": 33554432, + "MinSamples": 30, + "MinWindow": 21600000000000, + "MinChangeRatio": 0.1, + "SkipCPUForHPA": true + }, + "plan": { + "MinNodeUtilization": 0.5, + "MinConfidence": 0.6, + "MaxNodeRemovals": 3, + "ApplyRecommendations": true, + "MinClusterHeadroom": 0.1, + "RespectManagedNodes": true, + "DefaultMode": "apply" + }, + "decision": { + "MinSamples": 30, + "MinWindow": 21600000000000, + "BaseSoak": 21600000000000, + "ClassFlipWindow": 86400000000000, + "MinClassStability": 0.7, + "MaxHPAThrashPerHour": 2, + "MaxForecastDivergence": 0.35, + "ActConfidence": 0.6 + } + }, + "candidatePolicy": { + "recommend": { + "CPUPercentile": 0.95, + "CPUHeadroom": 1.05, + "MemoryPercentile": 0.99, + "MemoryHeadroom": 1.2, + "OOMBumpRatio": 1.5, + "MinMilliCPU": 10, + "MinMemoryBytes": 33554432, + "MinSamples": 30, + "MinWindow": 21600000000000, + "MinChangeRatio": 0.1, + "SkipCPUForHPA": true + }, + "plan": { + "MinNodeUtilization": 0.5, + "MinConfidence": 0.6, + "MaxNodeRemovals": 3, + "ApplyRecommendations": true, + "MinClusterHeadroom": 0.1, + "RespectManagedNodes": true, + "DefaultMode": "apply" + }, + "decision": { + "MinSamples": 30, + "MinWindow": 21600000000000, + "BaseSoak": 21600000000000, + "ClassFlipWindow": 86400000000000, + "MinClassStability": 0.7, + "MaxHPAThrashPerHour": 2, + "MaxForecastDivergence": 0.35, + "ActConfidence": 0.6 + } + }, + "changes": [ + { + "axis": "cpu-headroom", + "from": 1.15, + "to": 1.05, + "text": "cpu-headroom 1.15 → 1.05" + } + ], + "baseline": { + "policy": "69955a2cdd2004fd", + "cluster": "demo-bursty", + "window": [ + "2026-01-05T00:00:00Z", + "2026-01-12T00:00:00Z" + ], + "horizonHours": 24, + "decisionIntervalHours": 24, + "starvationFactor": 1, + "snapshots": 2016, + "instants": 7, + "scored": 14, + "decisions": 12, + "refusals": { + "insufficient-history": 2 + }, + "memViolations": 0, + "cpuStarvation": 0, + "memOOMKills": 0, + "oracleGapPct": 92.89006, + "oracleGapPctApplied": 23.4255, + "policyCostUSD": 6.027429, + "oracleCostUSD": 3.1248, + "claimedSavingsUSD": 13.023771, + "realizedSavingsUSD": 13.023771, + "claimedVsRealized": 1, + "forgoneSavingsUSD": 2.2752, + "refusalsGood": 0, + "refusalsIdle": 2, + "flipRate": 0, + "flips": 0, + "resourceRegretUSD": 2.902629, + "riskRegretUSD": 0, + "regretUSD": 2.902629, + "skipped": { + "noHorizon": 1, + "noSnapshot": 0, + "noFutureSamples": 0, + "eventQueryErrors": 0 + }, + "cost": { + "cpuUSDPerCoreHour": 0.024, + "memUSDPerGiBHour": 0.006, + "incidentUSD": 50 + } + }, + "candidate": { + "policy": "f8e7e12e930bfae8", + "cluster": "demo-bursty", + "window": [ + "2026-01-05T00:00:00Z", + "2026-01-12T00:00:00Z" + ], + "horizonHours": 24, + "decisionIntervalHours": 24, + "starvationFactor": 1, + "snapshots": 2016, + "instants": 7, + "scored": 14, + "decisions": 12, + "refusals": { + "insufficient-history": 2 + }, + "memViolations": 0, + "cpuStarvation": 0, + "memOOMKills": 0, + "oracleGapPct": 88.761028, + "oracleGapPctApplied": 18.608296, + "policyCostUSD": 5.898405, + "oracleCostUSD": 3.1248, + "claimedSavingsUSD": 13.152795, + "realizedSavingsUSD": 13.152795, + "claimedVsRealized": 1, + "forgoneSavingsUSD": 2.2752, + "refusalsGood": 0, + "refusalsIdle": 2, + "flipRate": 0, + "flips": 0, + "resourceRegretUSD": 2.773605, + "riskRegretUSD": 0, + "regretUSD": 2.773605, + "skipped": { + "noHorizon": 1, + "noSnapshot": 0, + "noFutureSamples": 0, + "eventQueryErrors": 0 + }, + "cost": { + "cpuUSDPerCoreHour": 0.024, + "memUSDPerGiBHour": 0.006, + "incidentUSD": 50 + } + }, + "delta": { + "memViolations": 0, + "cpuStarvation": 0, + "regretUSD": -0.129024, + "regretPct": -4.445074, + "resourceRegretUSD": -0.129024, + "riskRegretUSD": 0, + "policyCostUSD": -0.129024, + "forgoneSavingsUSD": 0, + "oracleGapPct": -4.129032, + "oracleGapPctApplied": -4.817204, + "decisions": 0, + "refusals": 0, + "refusalsIdle": 0, + "flipRate": 0, + "flips": 0, + "projectedMonthlyUSD": -0.56064, + "windowHours": 168 + }, + "gate": { + "passed": true, + "wins": [ + "regret $2.9026 → $2.7736", + "oracle gap 92.890% → 88.761%" + ], + "requiredRegretImprovementUSD": 0.029026, + "baselinePolicy": "69955a2cdd2004fd", + "candidatePolicy": "f8e7e12e930bfae8", + "tolerance": { + "minRegretImprovementUSD": 0.01, + "minRegretImprovementPct": 1, + "maxFlipRateIncrease": 0, + "maxOracleGapIncreasePct": 0 + } + } +} diff --git a/cmd/kilter/whatif.go b/cmd/kilter/whatif.go new file mode 100644 index 0000000..6ad5b0f --- /dev/null +++ b/cmd/kilter/whatif.go @@ -0,0 +1,845 @@ +package main + +import ( + "flag" + "fmt" + "io" + "os" + "strconv" + "strings" + "time" + + "github.com/agenticode/kilter/pkg/backtest" + "github.com/agenticode/kilter/pkg/whatif" +) + +// `kilter whatif` — the counterfactual, made runnable. +// +// pkg/whatif shipped as reasoning-engine unit 5 with a complete CLI and API +// specification and zero reachability: nothing in the binary called it. This +// file is that specification implemented, not redesigned — the flags, the +// output shape and the exit codes come from pkg/whatif/FINDINGS.md, "CLI and +// API surface: what a later unit must do". +// +// # The three lines this command does not cross +// +// 1. THE DEMO PATH IS WIRED; THE LIVE PATH REFUSES. A what-if replays recorded +// history TWICE, once per policy. pkg/store keeps only the LATEST snapshot +// per cluster, so --cluster has nothing to replay and refuses by name +// (whatifClusterRefusal). The failure mode is worse here than it is for +// `kilter backtest`, because the output is a COMPARISON: two scorecards +// over an empty replay agree on every field, so the delta is all zeros and +// "no strict improvement" reads as a considered verdict rather than as +// "nothing was replayed". +// +// 2. --auto-tune=apply IS NOT IMPLEMENTED, and says so by name. Auto-apply +// needs a writer, and a writer outside INV-4's single funnel is the thing +// the whole unit is built to prevent. See autoTuneRefusal. +// +// 3. THE EVALUATION PATH CANNOT BE POINTED AT THE POLICY UNDER TEST. There is +// no flag that scores the candidate against itself: whatif.Scenario shares +// every yardstick field between the two runs by construction, and a +// candidate equal to the baseline is refused by Scenario.validate before +// anything is replayed. --enforce-refusals is deliberately a SCENARIO knob +// here (shared by both runs) even though `kilter backtest` treats the same +// field as part of the policy — see loadWhatIfPolicy. + +const whatifUsageHead = `kilter whatif — what would this policy have done instead? + +Usage: + kilter whatif --demo --candidate [flags] + kilter whatif --demo --set = [flags] + kilter whatif --cluster ... (refused: see below) + +Two replays of the same recorded history — one under the incumbent policy, one +under the candidate — scored by the same yardstick, differenced, and put +through §4.6's dominance gate. Nothing here computes a quality number of its +own: every figure is pkg/backtest's output or arithmetic over two of its +scorecards. + +Archetypes (--demo), each with a closed-form oracle: + steady every sample at the base level + diurnal half the window at peak + bursty 12 spikes/day + regime-change a level shift at the midpoint, on a decision instant + +History flags: + --demo KIND synthetic archetype to replay + --cluster ID replay a live cluster's own history (refused: see below) + --days N trace length in days (default 7) + --workloads N containers in the trace (default 2) + --noise PCT deterministic jitter, e.g. 0.05 for +/-5% (default 0) + --from 30d|RFC3339 window start; a relative value is measured back from the + NEWEST SNAPSHOT in the history, never from the wall clock + --to RFC3339 window end, exclusive (default: the end of the history) + +Yardstick flags (shared by BOTH runs — changing one changes neither policy): + --horizon DUR how far ahead each decision is scored (default 24h) + --interval DUR spacing of decision instants (default 24h) + --starvation F CPU violation threshold: future p95 > request x F + --incident-usd N price of one violated container-window (default 50) + --derive-costs derive CPU/memory rates from the catalog and the nodes + --catalog PATH pricing catalog JSON (default: embedded) + --enforce-refusals run pkg/decision's refusal predicates in both replays + +Policy flags: + --policy default|PATH the incumbent (default: the shipped policy) + --candidate PATH the policy under test + --set AXIS=VALUE move one axis of the candidate (repeatable) + +Output flags: + --json emit whatif.Result verbatim (byte-stable, CI-diffable) + --fail-on-no-improvement exit non-zero when the gate rejects the candidate + --propose file the result as a proposal (needs --store PATH) + --store PATH the proposal store file (see kilter proposals) + --rationale TEXT why this change is being proposed + --author-id ID the identity to file under (never a human: see below) + --now RFC3339 audit timestamp for --propose (default: the wall clock). + It never touches the replay window, which comes from + the history. + --auto-tune off off|propose|apply — propose and apply are refused by + name; see the output of --auto-tune=apply + +--set axes and the HARD BOUNDS no config, tuner, agent or API caller may widen +(whatif.HardBounds()): +` + +// whatifUsage renders the usage text with the hard bounds appended. The bounds +// are read from whatif.HardBounds() rather than restated, so help text cannot +// drift from the values actually enforced; they are enumerated in +// whatif.AllAxes order, so the text is byte-stable. +func whatifUsage() string { + var b strings.Builder + b.WriteString(whatifUsageHead) + hard := whatif.HardBounds() + for _, a := range whatif.AllAxes { + r := hard[a] + unit := "" + if a == whatif.AxisBaseSoak { + unit = " hours, written with a unit: 8h, 90m" + } + fmt.Fprintf(&b, " %-20s [%g, %g]%s\n", a, r.Min, r.Max, unit) + } + b.WriteString(` +The declared search space (--set is checked against the HARD bounds; the gate +additionally checks the candidate against whatif.DefaultEnvelope, which is +narrower on every axis). +`) + return b.String() +} + +func runWhatIf(args []string) error { return runWhatIfTo(os.Stdout, args) } + +// whatifFlags is the command's input. +type whatifFlags struct { + demo string + cluster string + days int + workloads int + noise float64 + from string + to string + + horizon time.Duration + interval time.Duration + starvation float64 + incidentUSD float64 + deriveCosts bool + catalog string + + policy string + candidate string + sets repeatedFlag + + enforceRefusals bool + jsonOut bool + failOnNoImprove bool + + propose bool + store string + rationale string + authorID string + now string + + autoTune string +} + +// runWhatIfTo is the testable entry point: everything printed goes to w. +func runWhatIfTo(w io.Writer, args []string) error { + fs := flag.NewFlagSet("whatif", flag.ContinueOnError) + fs.SetOutput(w) + var wf whatifFlags + fs.StringVar(&wf.demo, "demo", "", "synthetic archetype (steady|diurnal|bursty|regime-change)") + fs.StringVar(&wf.cluster, "cluster", "", "replay a live cluster's own history") + fs.IntVar(&wf.days, "days", 7, "trace length in days") + fs.IntVar(&wf.workloads, "workloads", 2, "containers in the trace") + fs.Float64Var(&wf.noise, "noise", 0, "deterministic jitter fraction") + fs.StringVar(&wf.from, "from", "", "window start (30d or RFC3339)") + fs.StringVar(&wf.to, "to", "", "window end, exclusive (RFC3339)") + fs.DurationVar(&wf.horizon, "horizon", 24*time.Hour, "scoring horizon") + fs.DurationVar(&wf.interval, "interval", 24*time.Hour, "decision interval") + fs.Float64Var(&wf.starvation, "starvation", 0, "CPU starvation factor (0 = package default)") + fs.Float64Var(&wf.incidentUSD, "incident-usd", 0, "price of one violated container-window") + fs.BoolVar(&wf.deriveCosts, "derive-costs", false, "derive cost rates from the catalog") + fs.StringVar(&wf.catalog, "catalog", "", "pricing catalog JSON") + fs.StringVar(&wf.policy, "policy", "", "incumbent policy triple JSON") + fs.StringVar(&wf.candidate, "candidate", "", "candidate policy triple JSON") + fs.Var(&wf.sets, "set", "move one candidate axis, AXIS=VALUE (repeatable)") + fs.BoolVar(&wf.enforceRefusals, "enforce-refusals", false, "run pkg/decision's refusal predicates in both replays") + fs.BoolVar(&wf.jsonOut, "json", false, "emit whatif.Result as JSON") + fs.BoolVar(&wf.failOnNoImprove, "fail-on-no-improvement", false, "exit non-zero when the gate rejects") + fs.BoolVar(&wf.propose, "propose", false, "file the result as a proposal") + fs.StringVar(&wf.store, "store", "", "proposal store file") + fs.StringVar(&wf.rationale, "rationale", "", "why this change is being proposed") + fs.StringVar(&wf.authorID, "author-id", "", "the identity to file the proposal under") + fs.StringVar(&wf.now, "now", "", "audit timestamp as RFC3339 (default: now)") + fs.StringVar(&wf.autoTune, "auto-tune", "off", "off|propose|apply (propose and apply are refused)") + if err := fs.Parse(args); err != nil { + return err + } + + // The auto-tune refusal is checked FIRST, before any replay: a user who + // asked for auto-apply must not receive a scorecard that reads as though + // the request was honoured and merely printed instead of applied. + if err := autoTuneRefusal(wf.autoTune); err != nil { + return err + } + + switch { + case wf.cluster != "" && wf.demo != "": + return fmt.Errorf("whatif: --demo and --cluster are different sources of history; pass one") + case wf.cluster != "": + return whatifClusterRefusal(wf.cluster) + case wf.demo == "": + fmt.Fprint(w, whatifUsage()) + return fmt.Errorf("whatif: --demo or --cluster is required") + } + if wf.candidate == "" && len(wf.sets) == 0 { + fmt.Fprint(w, whatifUsage()) + return fmt.Errorf("whatif: a what-if needs a candidate: --candidate or --set =") + } + if wf.propose && wf.store == "" { + return whatifProposeRefusal() + } + return runWhatIfDemo(w, &wf) +} + +// whatifClusterRefusal is the honest half of this command. +// +// It names the seam, says what it would take, and exits non-zero. It does NOT +// fall back to the one snapshot pkg/store holds — see the comment on +// backtestLiveRefusal for why a scorecard over an empty replay is the worst +// available outcome, and note that a COMPARISON over an empty replay is worse +// still: a delta of all zeros and a verdict of "no strict improvement" read as +// a measurement that was taken and came back negative. +func whatifClusterRefusal(cluster string) error { + return fmt.Errorf(`whatif --cluster %s: refused — snapshot history is not persisted. + +A what-if replays recorded history TWICE, once per policy. pkg/store keeps only +the LATEST snapshot per cluster (SaveSnapshot/LoadSnapshot are keyed by cluster, +not by time), so there is no history to replay and both runs would score a +single instant. + +Printing that comparison would be worse than printing a single bad scorecard: +two runs over an empty replay agree on every field, so every delta is 0.00, +"regret $0.00" reads as "perfect" and the gate's "regret improves by $0.0000, +short of the $0.0100 margin" reads as "we measured, and the candidate is not +better" — when nothing was measured at all. + +What it needs (pkg/whatif/FINDINGS.md, "The snapshot-history seam"; +pkg/backtest/FINDINGS.md, "Seams this unit needed and did not find", item 1): a +time-keyed snapshot bucket in pkg/store — SaveSnapshotAt(snap) and +Snapshots(cluster, from, to) — plus an adapter implementing +backtest.SnapshotSource, which is the type whatif.Scenario.History already +takes. At a 5-minute cadence a 30-day window is 8,640 snapshots per cluster, so +the realistic shape is keyframe-plus-delta or a reduced replay snapshot. + +Meanwhile: kilter whatif --demo regime-change --set cpu-headroom=1.30`, cluster) +} + +// autoTuneRefusal implements §4.6's mode flag as far as this unit may, which +// is: not at all, by name, with the reason. +// +// The flag exists rather than being silently absent because "flag provided but +// not defined" teaches a reader nothing, and the two refusals below are the +// whole point of the unit. `off` is the default and is a no-op here — the +// nightly loop is a brain, not a CLI. +func autoTuneRefusal(mode string) error { + switch strings.TrimSpace(mode) { + case "", "off": + return nil + case "propose": + return fmt.Errorf(`whatif --auto-tune=propose: refused — the nightly tuner is brain wiring, not a CLI verb. + +§4.6's loop is constructed at brain start (whatif.NewTuner, unconditionally, so +a disabled-but-invalid config is a startup error rather than a 3am surprise) and +Run(basePolicy, scenario, historyEnd, clock) fires on the nightly timer, where +historyEnd is the newest snapshot's timestamp — once per cluster. That is +pkg/api's wiring and it is not built; it also needs the same snapshot-history +seam --cluster refuses on. See cmd/WHATIF-WIRING-FINDINGS.md. + +To ask the same question once, by hand: + kilter whatif --demo --set = --propose --store PATH`) + case "apply": + return fmt.Errorf(`whatif --auto-tune=apply: refused, by name, and not for want of budget. + +§4.6 item 4 offers auto-apply for a proposal that dominates on every metric +inside hard bounds. Auto-apply needs a WRITER, and a writer is exactly what +breaks INV-4's single funnel: gated → approved by a human who is not the author +→ applied. pkg/whatif has no writer on principle ("if a future change adds a +writer here, it has broken the unit"); putting one behind a CLI flag would be +the same mistake one layer up, where it would also be outside the audit trail +the funnel exists to produce. + +When it is built it belongs in pkg/api, as a caller that + (a) reads a gated proposal, + (b) mints an approval as a CONFIGURED OPERATOR IDENTITY distinct from the + tuner — configured, so that a human decided in advance and in writing, + (c) writes the config, + (d) posts applied, with the §4.6 ledger entry in the same breath. +Every step is already representable in pkg/whatif. None of it may live here.`) + default: + return fmt.Errorf("whatif --auto-tune %q: unknown mode (off|propose|apply)", mode) + } +} + +// whatifProposeRefusal explains why --propose needs a named file. +func whatifProposeRefusal() error { + return fmt.Errorf(`whatif --propose: refused — --store PATH is required. + +whatif.Store.Create is what runs the gate and mints the proposal ID, so a +proposal that is not stored is a receipt for a document that does not exist: +` + "`kilter proposals show `" + ` would not find it. pkg/whatif owns no file by +design — Snapshot() and Load() move bytes and cmd/ decides where they live — so +the file has to be named: + + kilter whatif --demo bursty --set cpu-headroom=1.30 --propose --store ./proposals.json + +That file is a LOCAL artifact. The fleet's proposals belong in pkg/store's +bbolt file under a new ` + "`proposals`" + ` bucket (pkg/whatif/FINDINGS.md, "Brain +wiring"), which is pkg/api's to add and is not wired yet.`) +} + +// runWhatIfDemo replays a synthetic trace under two policies. +func runWhatIfDemo(w io.Writer, wf *whatifFlags) error { + kind, err := parseArchetype(wf.demo) + if err != nil { + // The archetype set is shared with `kilter backtest`; the flag name in + // its message is not, so only that is restated. + return fmt.Errorf("whatif %s", strings.TrimPrefix(err.Error(), "backtest ")) + } + // The trace's start is the same FIXED instant `kilter backtest` uses. A + // replay window that drifts with the wall clock makes two runs over one + // configuration disagree, and here it would additionally make a proposal's + // fingerprint — which covers the window — unreproducible. + spec := backtest.TraceSpec{ + Cluster: "demo-" + string(kind), + Kind: kind, + Start: backtestEpoch, + Days: wf.days, + Workloads: wf.workloads, + NoisePct: wf.noise, + } + trace, err := spec.Build() + if err != nil { + return fmt.Errorf("whatif --demo %s: %w", wf.demo, err) + } + store, err := trace.Store() + if err != nil { + return fmt.Errorf("whatif: build evidence store: %w", err) + } + + from, to, notes, err := resolveWhatIfWindow(wf, trace) + if err != nil { + return err + } + + scoring := backtest.DefaultConfig() + scoring.DecisionInterval = wf.interval + if wf.starvation > 0 { + scoring.StarvationFactor = wf.starvation + } + if wf.incidentUSD > 0 { + scoring.Cost.IncidentUSD = wf.incidentUSD + } + catalog, err := loadCatalog(wf.catalog) + if err != nil { + return err + } + if wf.deriveCosts { + if len(trace.Snapshots) == 0 { + return fmt.Errorf("whatif: --derive-costs needs at least one snapshot") + } + cm, err := backtest.CostModelFromCatalog(catalog, trace.Snapshots[len(trace.Snapshots)-1], scoring.Cost) + if err != nil { + return fmt.Errorf("whatif: --derive-costs: %w", err) + } + scoring.Cost = cm + } + + baseline, err := loadWhatIfPolicy(wf.policy, "--policy") + if err != nil { + return err + } + candidate := baseline + if wf.candidate != "" { + candidate, err = loadWhatIfPolicy(wf.candidate, "--candidate") + if err != nil { + return err + } + } + candidate, applied, err := applyAxisSets(candidate, wf.sets) + if err != nil { + return err + } + + scen := whatif.Scenario{ + Cluster: trace.Cluster, + From: from, + To: to, + Horizon: wf.horizon, + Baseline: baseline, + Candidate: candidate, + History: trace.Source(), + Evidence: store, + Catalog: catalog, + Scoring: scoring, + // Shared by both runs: this is a property of the yardstick, not of + // the policy under test. See loadWhatIfPolicy. + EnforceDecisionRefusals: wf.enforceRefusals, + } + // Returned unwrapped: pkg/whatif already prefixes its errors with + // "whatif:", and a second prefix would say it twice. + result, err := scen.Run() + if err != nil { + return err + } + + if wf.jsonOut { + // Verbatim: Result.Encode is the byte-stable, CI-diffable form, and + // is what a golden file pins. + raw, encErr := result.Encode() + if encErr != nil { + return encErr + } + if _, err := w.Write(raw); err != nil { + return err + } + } else { + if err := writeWhatIf(w, wf, kind, trace, notes, applied, result); err != nil { + return err + } + } + + if wf.propose { + if err := proposeWhatIf(w, wf, result); err != nil { + return err + } + } + if wf.failOnNoImprove && !result.Improved() { + return fmt.Errorf("whatif: the candidate policy did not pass the gate") + } + return nil +} + +// resolveWhatIfWindow turns --from/--to into concrete instants. +// +// The anchor is the HISTORY, never time.Now(): `--from 30d` is measured back +// from the newest snapshot in the replayed history, which is the rule +// pkg/backtest's FINDINGS states and pkg/whatif's repeats. whatif.Scenario +// takes no clock at all, so resolving this window is the CLI's whole job here +// — and getting it from a wall clock would make two runs over identical data +// disagree, and a proposal's fingerprint (which covers the window) change +// every night. +// +// A window that reaches past either end of the recorded history is CLAMPED and +// the clamp is reported, for the same reason `kilter domains --rds-fixture` +// reports its window clamp: a run that claims a 30-day window over 7 days of +// history is a lie told by omission. +func resolveWhatIfWindow(wf *whatifFlags, trace *backtest.Trace) (time.Time, time.Time, []string, error) { + newest := trace.Start + for _, s := range trace.Snapshots { + if s.Timestamp.After(newest) { + newest = s.Timestamp + } + } + var notes []string + + to := trace.End + if wf.to != "" { + t, err := time.Parse(time.RFC3339, wf.to) + if err != nil { + return time.Time{}, time.Time{}, nil, fmt.Errorf("whatif --to: %w (RFC3339)", err) + } + to = t.UTC() + } + + from := trace.Start + if wf.from != "" { + if d, ok, err := parseRelativeWindow(wf.from); err != nil { + return time.Time{}, time.Time{}, nil, err + } else if ok { + // Relative to the newest SNAPSHOT, per the spec — not to `to`, + // which the caller may have set past the end of the history. + from = newest.Add(-d) + notes = append(notes, fmt.Sprintf( + "--from %s resolved against the newest snapshot (%s), not the wall clock", + wf.from, newest.UTC().Format(time.RFC3339))) + } else { + t, err := time.Parse(time.RFC3339, wf.from) + if err != nil { + return time.Time{}, time.Time{}, nil, fmt.Errorf( + "whatif --from: %w (RFC3339, or a relative span like 30d)", err) + } + from = t.UTC() + } + } + + if from.Before(trace.Start) { + notes = append(notes, fmt.Sprintf("window start clamped to the start of the history (%s)", + trace.Start.UTC().Format(time.RFC3339))) + from = trace.Start + } + if to.After(trace.End) { + notes = append(notes, fmt.Sprintf("window end clamped to the end of the history (%s)", + trace.End.UTC().Format(time.RFC3339))) + to = trace.End + } + if !to.After(from) { + return time.Time{}, time.Time{}, nil, fmt.Errorf( + "whatif: replay window [%s,%s) is empty or inverted", + from.UTC().Format(time.RFC3339), to.UTC().Format(time.RFC3339)) + } + return from.UTC(), to.UTC(), notes, nil +} + +// parseRelativeWindow reads the `30d` / `72h` form the spec writes for --from. +// Days are spelled out because time.ParseDuration has no day unit, and an +// operator who writes 30d means thirty times twenty-four hours. +func parseRelativeWindow(s string) (time.Duration, bool, error) { + s = strings.TrimSpace(s) + if s == "" { + return 0, false, nil + } + if strings.HasSuffix(s, "d") { + n, err := strconv.ParseFloat(strings.TrimSuffix(s, "d"), 64) + if err != nil { + return 0, false, fmt.Errorf("whatif --from %q: %w", s, err) + } + if !(n > 0) { + return 0, false, fmt.Errorf("whatif --from %q: a relative window must be positive", s) + } + return time.Duration(n * float64(24*time.Hour)), true, nil + } + // An RFC3339 timestamp starts with a four-digit year, so it can never be + // mistaken for a duration; anything ParseDuration accepts is relative. + if d, err := time.ParseDuration(s); err == nil { + if !(d > 0) { + return 0, false, fmt.Errorf("whatif --from %q: a relative window must be positive", s) + } + return d, true, nil + } + return 0, false, nil +} + +// ---------------------------------------------------------------- the policies + +// loadWhatIfPolicy reads a policy triple into whatif.Policy. +// +// It reuses `kilter backtest`'s loader — same file format, same +// pointer-overlay semantics, same rejection of unknown fields — and then +// refuses one field of it, which is the one mismatch between the two commands +// and is worth stating in full: +// +// `enforceDecisionRefusals` is part of the POLICY for `kilter backtest` +// (cmd/kilter/backtest.go's `policy` struct), because there the question is +// "should we wire pkg/decision in?" and an A/B through Gate is the honest way +// to ask it. In a what-if it is part of the YARDSTICK: +// whatif.Scenario.EnforceDecisionRefusals is a scenario field shared by both +// replays, precisely so the two sides cannot be scored under different rules. +// A policy file that set it would therefore be silently ignored, and a what-if +// of a policy nobody ran is exactly the artefact this whole unit exists to +// prevent. So it is refused by name, with the flag that does work. +func loadWhatIfPolicy(path, flagName string) (whatif.Policy, error) { + if path == "" || path == "default" { + return whatif.DefaultPolicy(), nil + } + // Two loads with opposite defaults: if the file PINNED the field, both + // agree; if it left the field out, they differ. Cheaper and less brittle + // than a second parser for one boolean. + withFalse, err := loadPolicy(path, false) + if err != nil { + return whatif.Policy{}, fmt.Errorf("%s: %w", flagName, err) + } + withTrue, err := loadPolicy(path, true) + if err != nil { + return whatif.Policy{}, fmt.Errorf("%s: %w", flagName, err) + } + if withFalse.EnforceRefusals == withTrue.EnforceRefusals { + return whatif.Policy{}, fmt.Errorf( + `%s %s: "enforceDecisionRefusals" is not part of the policy in a what-if. + +whatif.Scenario shares it between BOTH replays (it is a scenario field, not a +policy field), so the two sides are scored under the same rules; a policy file +that set it here would be silently ignored and the answer would describe a +policy nobody ran. Use --enforce-refusals, which applies to both runs, or +`+"`kilter backtest --compare`"+`, where it IS the policy under test.`, flagName, path) + } + return whatif.Policy{ + Rec: withFalse.Rec, + Plan: withFalse.Plan, + Decision: withFalse.Decision, + }, nil +} + +// applyAxisSets applies the repeatable --set overrides to the candidate. +// +// The axis names are exactly whatif.AllAxes and an unknown one is REJECTED +// rather than ignored: silently dropping a knob the caller asked to tune +// produces a what-if whose rationale does not describe what was measured. +// Values are checked against whatif.HardBounds() here — the absolute limits no +// caller may widen — and the gate independently re-checks the candidate +// against the declared envelope, which is narrower. Producer and checker are +// different code on purpose. +// +// whatif.Axis.get/set are unexported, so the five-field projection is restated +// here. TestSetMovesTheAxisPkgWhatifThinksItMoves cross-checks every axis +// against pkg/whatif's own projection (Result.Changes), so a mis-mapped field +// fails the build's tests rather than quietly tuning the wrong knob. +func applyAxisSets(p whatif.Policy, sets []string) (whatif.Policy, []string, error) { + hard := whatif.HardBounds() + var applied []string + for _, raw := range sets { + name, value, ok := strings.Cut(raw, "=") + if !ok { + return whatif.Policy{}, nil, fmt.Errorf("whatif --set %q: expected AXIS=VALUE", raw) + } + axis := whatif.Axis(strings.TrimSpace(name)) + value = strings.TrimSpace(value) + if !axis.Known() { + return whatif.Policy{}, nil, fmt.Errorf("whatif --set %q: unknown axis (known: %s)", + name, strings.Join(axisNames(), ", ")) + } + var num float64 + if axis == whatif.AxisBaseSoak { + d, err := time.ParseDuration(value) + if err != nil { + // A bare number means nanoseconds in Go's encoding, which is + // never what anybody meant — same refusal backtest's setD makes. + if _, numErr := strconv.Atoi(value); numErr == nil { + return whatif.Policy{}, nil, fmt.Errorf( + "whatif --set %s=%q: durations need a unit (\"8h\", \"90m\")", axis, value) + } + return whatif.Policy{}, nil, fmt.Errorf("whatif --set %s: %w", axis, err) + } + num = d.Hours() + p.Decision.BaseSoak = d + } else { + f, err := strconv.ParseFloat(value, 64) + if err != nil { + return whatif.Policy{}, nil, fmt.Errorf("whatif --set %s=%q: %w", axis, value, err) + } + num = f + switch axis { + case whatif.AxisCPUPercentile: + p.Rec.CPUPercentile = f + case whatif.AxisMemoryPercentile: + p.Rec.MemoryPercentile = f + case whatif.AxisCPUHeadroom: + p.Rec.CPUHeadroom = f + case whatif.AxisMemoryHeadroom: + p.Rec.MemoryHeadroom = f + } + } + if r, ok := hard[axis]; ok && !r.Contains(num) { + return whatif.Policy{}, nil, fmt.Errorf( + "whatif --set %s=%s: outside the hard bounds [%g,%g], which no config file, "+ + "tuner, agent or API caller may widen (whatif.HardBounds())", + axis, value, r.Min, r.Max) + } + applied = append(applied, string(axis)+"="+value) + } + return p, applied, nil +} + +// axisNames renders whatif.AllAxes for an error message, in the package's own +// fixed order. +func axisNames() []string { + out := make([]string, 0, len(whatif.AllAxes)) + for _, a := range whatif.AllAxes { + out = append(out, string(a)) + } + return out +} + +// ---------------------------------------------------------------- rendering + +// writeWhatIf renders the human form. Every enumeration is sorted or fixed, so +// two runs over the same trace print the same bytes. +func writeWhatIf(w io.Writer, wf *whatifFlags, kind backtest.TraceKind, trace *backtest.Trace, + notes, applied []string, r *whatif.Result) error { + var b strings.Builder + fmt.Fprintf(&b, "kilter whatif — %s, %s trace, %d days, %d workloads\n", + r.Cluster, kind, wf.days, wf.workloads) + fmt.Fprintf(&b, "window %s .. %s horizon %s interval %s\n", + r.Window[0].UTC().Format(time.RFC3339), r.Window[1].UTC().Format(time.RFC3339), + wf.horizon, wf.interval) + for _, n := range notes { + fmt.Fprintf(&b, "note: %s\n", n) + } + if len(applied) > 0 { + fmt.Fprintf(&b, "set: %s\n", strings.Join(applied, " ")) + } + b.WriteString("\n what changed\n") + if len(r.Changes) == 0 { + b.WriteString(" (no declared axis moved; the policies differ elsewhere)\n") + } + for _, c := range r.Changes { + fmt.Fprintf(&b, " %s\n", c.Text) + } + b.WriteString("\n") + writeScorecard(&b, "baseline", r.BaselineScore) + b.WriteString("\n") + writeScorecard(&b, "candidate", r.CandidateScore) + b.WriteString("\n") + writeDelta(&b, r.Delta) + b.WriteString("\n Gate\n") + if r.Gate.Passed { + b.WriteString(" ACCEPTED: the candidate dominates on the terms §4.6 defines\n") + } else { + b.WriteString(" REJECTED\n") + } + for _, reason := range r.Gate.Reasons { + fmt.Fprintf(&b, " %s\n", reason) + } + for _, win := range r.Gate.Wins { + fmt.Fprintf(&b, " win: %s\n", win) + } + fmt.Fprintf(&b, " required regret improvement %s\n", + usd(r.Gate.RequiredRegretImprovementUSD)) + _, err := io.WriteString(w, b.String()) + return err +} + +// writeDelta renders candidate-minus-baseline. Every field is signed +// explicitly, because the whole value of a delta is that the reader can tell +// which direction it moved without consulting the two scorecards. +func writeDelta(b *strings.Builder, d whatif.Delta) { + b.WriteString(" delta (candidate − baseline; negative is better everywhere except decisions)\n") + fmt.Fprintf(b, " safety memViolations %s cpuStarvation %s\n", + signedInt(d.MemViolations), signedInt(d.CPUStarvation)) + fmt.Fprintf(b, " regret %s (%s) resource %s risk %s\n", + signedUSD(d.RegretUSD), signedPct(d.RegretPct), + signedUSD(d.ResourceRegretUSD), signedUSD(d.RiskRegretUSD)) + fmt.Fprintf(b, " efficiency oracleGap %s pts (applied %s pts) forgone %s\n", + signedFloat(d.OracleGapPct, 1), signedFloat(d.OracleGapPctApplied, 1), + signedUSD(d.ForgoneSavingsUSD)) + fmt.Fprintf(b, " behaviour decisions %s refusals %s (idle %s) flipRate %s\n", + signedInt(d.Decisions), signedInt(d.Refusals), signedInt(d.RefusalsIdle), + signedFloat(d.FlipRate, 3)) + // Labelled a projection everywhere it is printed: it assumes next month + // resembles the window that was replayed, which is exactly the assumption + // a backtest cannot verify. + fmt.Fprintf(b, " projected %s/month, extrapolated from %.1fh of history\n", + signedUSD(d.ProjectedMonthlyUSD), d.WindowHours) +} + +func signedInt(v int) string { + if v > 0 { + return "+" + strconv.Itoa(v) + } + return strconv.Itoa(v) +} + +func signedUSD(v float64) string { + if v < 0 { + return fmt.Sprintf("-$%.2f", -v) + } + return fmt.Sprintf("+$%.2f", v) +} + +func signedPct(v float64) string { + return fmt.Sprintf("%+.1f%%", v) +} + +func signedFloat(v float64, digits int) string { + return fmt.Sprintf("%+.*f", digits, v) +} + +// ---------------------------------------------------------------- --propose + +// proposeWhatIf files the result as a proposal. +// +// Three properties are load-bearing and each is a deliberate choice rather +// than a default: +// +// 1. THE VERDICT IS NOT CARRIED ACROSS. Result.Spec deliberately omits the +// GateResult, and Store.Create runs Decide itself from the scorecards. A +// caller — this one included — hands over evidence and receives a +// judgment; it cannot hand over a judgment. +// +// 2. THE AUTHOR IS NEVER A HUMAN. pkg/whatif's FINDINGS says the CLI actor is +// {human, } — and a local CLI process has no +// authenticated identity to offer. $USER, os/user and the uid are +// properties of the SESSION, which anything running in it inherits, +// including unit 8's reasoner. So the author is filed as +// Actor{Kind: ActorSystem, ID: <--author-id, default "kilter-cli">}. The +// ID matters: whatif.sameIdentity compares IDs and IGNORES Kind, so an +// operator who names themselves here is BLOCKED from later approving this +// proposal through the authenticated funnel. That direction is the safe +// one — naming yourself can only ever remove a capability — which is why +// it is a flag while the approver identity is not. +// +// 3. THE CLOCK IS AN ARGUMENT. --now feeds CreatedAt and the audit trail and +// nothing else; the replay window comes from the history. CreatedAt is +// excluded from the fingerprint, so the proposal ID is the same whether it +// was filed today or in a month. +func proposeWhatIf(w io.Writer, wf *whatifFlags, r *whatif.Result) error { + now, err := parseNowFlag(wf.now) + if err != nil { + return err + } + author := whatif.Actor{Kind: whatif.ActorSystem, ID: "kilter-cli"} + if id := strings.TrimSpace(wf.authorID); id != "" { + author.ID = id + } + spec, err := r.Spec(whatif.Target{Cluster: r.Cluster}, + whatif.DefaultEnvelope(), whatif.DefaultTolerance(), wf.rationale, nil) + if err != nil { + return fmt.Errorf("whatif --propose: %w", err) + } + store, err := openProposalStore(wf.store) + if err != nil { + return err + } + rec, err := store.Create(author, spec, whatif.FixedClock(now)) + if err != nil { + return fmt.Errorf("whatif --propose: %w", err) + } + if err := saveProposalStore(wf.store, store); err != nil { + return err + } + fmt.Fprintf(w, "\nfiled proposal %s (%s) by %s in %s\n", + rec.ID(), rec.State(), author, wf.store) + if rec.State() == whatif.StateRejected { + fmt.Fprintf(w, " the gate rejected it; it is filed anyway, because a rejected proposal is\n"+ + " the record of a question that was asked and answered\n") + } + fmt.Fprintf(w, " kilter proposals show %s --store %s\n", rec.ID(), wf.store) + return nil +} + +// parseNowFlag resolves --now. The wall clock is read here, at the edge of the +// program, and passed inward as a whatif.Clock — pkg/whatif never calls +// time.Now itself, and a caller that forgets a clock gets an error rather than +// a silently unreproducible proposal. +func parseNowFlag(s string) (time.Time, error) { + if strings.TrimSpace(s) == "" { + return time.Now().UTC(), nil + } + t, err := time.Parse(time.RFC3339, s) + if err != nil { + return time.Time{}, fmt.Errorf("--now: %w (RFC3339)", err) + } + return t.UTC(), nil +} diff --git a/cmd/kilter/whatif_test.go b/cmd/kilter/whatif_test.go new file mode 100644 index 0000000..9aca271 --- /dev/null +++ b/cmd/kilter/whatif_test.go @@ -0,0 +1,687 @@ +package main + +import ( + "encoding/json" + "os" + "strings" + "testing" + + "github.com/agenticode/kilter/pkg/whatif" +) + +// These tests drive pkg/whatif's REAL Scenario, gate and store through the +// REAL CLI entry point. The traces are synthetic and their oracles are known +// in closed form, so every number below is reproducible: there is no clock in +// the replay path (backtestEpoch is a constant, and --now only ever reaches +// CreatedAt), and no network anywhere. + +func runWhatIfOK(t *testing.T, args ...string) string { + t.Helper() + var b strings.Builder + if err := runWhatIfTo(&b, args); err != nil { + t.Fatalf("kilter whatif %s: %v\n%s", strings.Join(args, " "), err, b.String()) + } + return b.String() +} + +func runWhatIfErr(t *testing.T, args ...string) (string, error) { + t.Helper() + var b strings.Builder + err := runWhatIfTo(&b, args) + if err == nil { + t.Fatalf("kilter whatif %s was accepted:\n%s", strings.Join(args, " "), b.String()) + } + return b.String(), err +} + +func whatifResult(t *testing.T, args ...string) *whatif.Result { + t.Helper() + raw := runWhatIfOK(t, append(args, "--json")...) + var r whatif.Result + if err := json.Unmarshal([]byte(raw), &r); err != nil { + t.Fatalf("decode result: %v\n%s", err, raw) + } + return &r +} + +// acceptedArgs is a candidate the gate ACCEPTS on the bursty trace: dropping +// CPU headroom to the bottom of the envelope is cheaper here and costs no +// safety. Kept in one place because several tests need a passing result and a +// test that silently started testing a rejection would still pass. +var acceptedArgs = []string{"--demo", "bursty", "--set", "cpu-headroom=1.05"} + +// rejectedArgs is the mirror: raising CPU headroom on the regime-change trace +// buys nothing and costs resource regret. +var rejectedArgs = []string{"--demo", "regime-change", "--set", "cpu-headroom=1.30"} + +// TestWhatIfRunsBothReplaysAndGatesTheDelta is the end-to-end shape: two +// scorecards, the arithmetic between them, and a verdict. +func TestWhatIfRunsBothReplaysAndGatesTheDelta(t *testing.T) { + r := whatifResult(t, acceptedArgs...) + if r.BaselineScore == nil || r.CandidateScore == nil { + t.Fatal("a result must carry both scorecards; a nil one is not evidence") + } + if !r.Gate.Passed { + t.Fatalf("the shipped accepted-case candidate no longer passes: %v", r.Gate.Reasons) + } + if r.BaselineScore.Policy == r.CandidateScore.Policy { + t.Fatal("both scorecards are for the same policy hash") + } + // The delta is arithmetic over the two scorecards and nothing else. + if got, want := r.Delta.RegretUSD, + r.CandidateScore.RegretUSD-r.BaselineScore.RegretUSD; !near(got, want, 1e-5) { + t.Errorf("delta regret %v, want candidate−baseline %v", got, want) + } + if got, want := r.Delta.MemViolations, + r.CandidateScore.MemViolations-r.BaselineScore.MemViolations; got != want { + t.Errorf("delta memViolations %d, want %d", got, want) + } + if len(r.Changes) != 1 || r.Changes[0].Axis != whatif.AxisCPUHeadroom { + t.Errorf("changes = %+v, want exactly cpu-headroom", r.Changes) + } +} + +// TestTheEvaluationPathCannotBePointedAtThePolicyUnderTest. +// +// pkg/whatif's sharpest constraint: if the evaluation can be traced back to +// the policy under test, the number is worthless. Two halves are asserted +// here, because the CLI is the layer that could reintroduce either. +// +// 1. There is no flag that scores a policy against itself. A candidate equal +// to the baseline is refused BEFORE anything is replayed, and no scorecard +// is printed. +// 2. The yardstick is shared. The oracle, the scored set and the ground-truth +// OOM counter are computed from future usage alone, so they must be +// IDENTICAL across the two runs no matter how different the policies are. +// If the evaluation ever started tracing back to the thing under test, the +// oracle would move with it. +func TestTheEvaluationPathCannotBePointedAtThePolicyUnderTest(t *testing.T) { + // 1. A what-if against itself is not a question. + for _, args := range [][]string{ + {"--demo", "steady", "--set", "cpu-headroom=1.15"}, // the shipped value + {"--demo", "steady", "--candidate", "default"}, + } { + out, err := runWhatIfErr(t, args...) + if !strings.Contains(err.Error(), "identical to the baseline") { + t.Errorf("%v: err = %q, want the identical-candidate refusal", args, err) + } + if strings.Contains(out, "regret") || strings.Contains(out, "Gate") { + t.Errorf("%v: a scorecard was printed for a self-comparison:\n%s", args, out) + } + } + + // 2. The two most different policies the envelope allows still share the + // yardstick, through the CLI. + aggressive := whatifResult(t, "--demo", "bursty", "--workloads", "3", + "--set", "cpu-percentile=0.80", "--set", "memory-percentile=0.95", + "--set", "cpu-headroom=1.05", "--set", "memory-headroom=1.05") + conservative := whatifResult(t, "--demo", "bursty", "--workloads", "3", + "--set", "cpu-percentile=0.99", "--set", "memory-percentile=0.999", + "--set", "cpu-headroom=1.50", "--set", "memory-headroom=1.50") + if aggressive.CandidateScore.Policy == conservative.CandidateScore.Policy { + t.Fatal("the two candidates hashed the same; the test proves nothing") + } + for _, r := range []*whatif.Result{aggressive, conservative} { + b, c := r.BaselineScore, r.CandidateScore + if b.OracleCostUSD != c.OracleCostUSD { + t.Errorf("oracle cost moved with the policy: %v vs %v", b.OracleCostUSD, c.OracleCostUSD) + } + if b.Scored != c.Scored || b.Instants != c.Instants || b.Snapshots != c.Snapshots { + t.Errorf("coverage moved with the policy: scored %d/%d instants %d/%d snapshots %d/%d", + b.Scored, c.Scored, b.Instants, c.Instants, b.Snapshots, c.Snapshots) + } + if b.MemOOMKills != c.MemOOMKills { + t.Errorf("ground-truth OOM kills moved with the policy: %d vs %d", + b.MemOOMKills, c.MemOOMKills) + } + if b.StarvationFactor != c.StarvationFactor || b.Cost != c.Cost { + t.Error("the cost model or starvation factor differed between the two runs") + } + } + // And across the two invocations: the oracle is a property of the history. + if aggressive.BaselineScore.OracleCostUSD != conservative.BaselineScore.OracleCostUSD { + t.Error("the oracle differs between two runs over the same trace") + } +} + +// TestSetMovesTheAxisPkgWhatifThinksItMoves. +// +// whatif.Axis.get/set are unexported, so cmd/ restates the five-field +// projection in applyAxisSets. This cross-checks every axis against +// pkg/whatif's OWN projection — Result.Changes is computed by changesBetween, +// which uses Axis.get — so a mis-mapped field (setting memory headroom when +// the caller asked for CPU headroom) fails here rather than quietly tuning the +// wrong knob under the right name. +func TestSetMovesTheAxisPkgWhatifThinksItMoves(t *testing.T) { + for _, tc := range []struct { + axis whatif.Axis + value string + want float64 + }{ + {whatif.AxisCPUPercentile, "0.90", 0.90}, + {whatif.AxisMemoryPercentile, "0.995", 0.995}, + {whatif.AxisCPUHeadroom, "1.25", 1.25}, + {whatif.AxisMemoryHeadroom, "1.35", 1.35}, + {whatif.AxisBaseSoak, "8h", 8}, + } { + t.Run(string(tc.axis), func(t *testing.T) { + r := whatifResult(t, "--demo", "steady", "--set", string(tc.axis)+"="+tc.value) + if len(r.Changes) != 1 { + t.Fatalf("changes = %+v, want exactly one axis to have moved", r.Changes) + } + c := r.Changes[0] + if c.Axis != tc.axis { + t.Fatalf("--set %s moved %s instead", tc.axis, c.Axis) + } + if !near(c.To, tc.want, 1e-9) { + t.Errorf("%s = %v, want %v", tc.axis, c.To, tc.want) + } + }) + } + // Every axis at once still moves exactly five, and no more. + r := whatifResult(t, "--demo", "steady", + "--set", "cpu-percentile=0.90", "--set", "memory-percentile=0.995", + "--set", "cpu-headroom=1.25", "--set", "memory-headroom=1.35", + "--set", "base-soak=8h") + if len(r.Changes) != len(whatif.AllAxes) { + t.Fatalf("moved %d axes, want all %d", len(r.Changes), len(whatif.AllAxes)) + } + for i, c := range r.Changes { + if c.Axis != whatif.AllAxes[i] { + t.Errorf("changes[%d] = %s, want AllAxes order (%s)", i, c.Axis, whatif.AllAxes[i]) + } + } +} + +// TestSetIsRejectedRatherThanIgnored. A knob the caller asked to tune that is +// silently dropped produces a what-if whose rationale does not describe what +// was measured — the same failure a misspelled policy-file key would be. +func TestSetIsRejectedRatherThanIgnored(t *testing.T) { + for _, tc := range []struct{ name, set, want string }{ + {"unknown axis", "gpu-headroom=1.2", "unknown axis"}, + {"typo", "cpu-headrooom=1.2", "unknown axis"}, + {"no equals", "cpu-headroom", "expected AXIS=VALUE"}, + {"not a number", "cpu-headroom=wide", "invalid syntax"}, + {"bare duration", "base-soak=3600", "durations need a unit"}, + {"above hard bound", "cpu-headroom=3.0", "outside the hard bounds"}, + {"below hard bound", "cpu-headroom=0.5", "outside the hard bounds"}, + {"soak past the ceiling", "base-soak=200h", "outside the hard bounds"}, + {"percentile past one", "memory-percentile=1.5", "outside the hard bounds"}, + } { + t.Run(tc.name, func(t *testing.T) { + out, err := runWhatIfErr(t, "--demo", "steady", "--set", tc.set) + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("err = %q, want it to mention %q", err, tc.want) + } + if strings.Contains(out, "regret") { + t.Errorf("a scorecard was printed for a rejected --set:\n%s", out) + } + }) + } +} + +// TestHardBoundsCannotBeWidenedFromTheCommandLine states the property the +// table above exercises: whatif.HardBounds() is the CLI's limit too, and the +// help text quotes the same map rather than a restatement of it. +func TestHardBoundsCannotBeWidenedFromTheCommandLine(t *testing.T) { + usage := whatifUsage() + for axis, r := range whatif.HardBounds() { + if !strings.Contains(usage, string(axis)) { + t.Errorf("--help does not name axis %s", axis) + } + // Just outside each bound must be refused. + for _, v := range []float64{r.Min - 0.01, r.Max + 0.01} { + if axis == whatif.AxisBaseSoak && v < 0 { + continue // a negative duration is a parse error, covered above + } + val := formatSetValue(axis, v) + if _, err := runWhatIfErr(t, "--demo", "steady", "--set", string(axis)+"="+val); err == nil { + t.Errorf("--set %s=%s outside %v was accepted", axis, val, r) + } + } + } + // A caller mutating the returned map does not move the CLI's limits. + stolen := whatif.HardBounds() + stolen[whatif.AxisCPUHeadroom] = whatif.Range{Min: 0, Max: 100} + if _, err := runWhatIfErr(t, "--demo", "steady", "--set", "cpu-headroom=3.0"); err == nil { + t.Error("mutating the HardBounds copy widened the CLI's limits") + } +} + +// formatSetValue renders a --set value in the unit the axis takes. +func formatSetValue(a whatif.Axis, v float64) string { + if a == whatif.AxisBaseSoak { + return strings.TrimSuffix(strconvFormat(v), " ") + "h" + } + return strconvFormat(v) +} + +func strconvFormat(v float64) string { + return strings.TrimRight(strings.TrimRight(jsonNumber(v), "0"), ".") +} + +func jsonNumber(v float64) string { + b, _ := json.Marshal(v) + return string(b) +} + +// TestWhatIfLiveHistoryRefusesRatherThanComparingTwoEmptyReplays. +// +// The comparison makes this refusal MORE important than `kilter backtest`'s, +// not less: two runs over an empty replay agree on every field, so the delta +// is all zeros and the gate's "no strict improvement" reads as a measurement +// that was taken and came back negative. +func TestWhatIfLiveHistoryRefusesRatherThanComparingTwoEmptyReplays(t *testing.T) { + out, err := runWhatIfErr(t, "--cluster", "prod", "--set", "cpu-headroom=1.05") + msg := err.Error() + for _, want := range []string{ + "snapshot history is not persisted", + "pkg/store", + "SaveSnapshotAt", + "Snapshots(cluster, from, to)", + "backtest.SnapshotSource", + "--demo", + } { + if !strings.Contains(msg, want) { + t.Errorf("the refusal does not mention %q:\n%s", want, msg) + } + } + // No scorecard, no delta, no verdict. + for _, forbidden := range []string{"regret", "Gate", "delta", "oracleGap"} { + if strings.Contains(out, forbidden) { + t.Errorf("%q was printed for a cluster with no history:\n%s", forbidden, out) + } + } + // And it refuses rather than quietly preferring one source over the other. + if _, err := runWhatIfErr(t, "--cluster", "prod", "--demo", "steady", + "--set", "cpu-headroom=1.05"); !strings.Contains(err.Error(), "pass one") { + t.Errorf("--cluster with --demo: err = %v", err) + } +} + +// TestAutoTuneApplyIsRefusedByName. +// +// pkg/whatif deferred auto-apply on principle, not for budget: apply needs a +// writer, and the writer is what breaks INV-4's single funnel. A CLI flag that +// quietly implemented it would be the same mistake one layer up, so the flag +// exists and refuses — and it refuses BEFORE any replay, so the output cannot +// read as though the request was honoured and merely printed. +func TestAutoTuneApplyIsRefusedByName(t *testing.T) { + out, err := runWhatIfErr(t, "--demo", "bursty", "--set", "cpu-headroom=1.05", + "--auto-tune=apply") + msg := err.Error() + for _, want := range []string{ + "--auto-tune=apply", "refused", "writer", "INV-4", + "pkg/api", "CONFIGURED OPERATOR IDENTITY", "ledger", + } { + if !strings.Contains(msg, want) { + t.Errorf("the refusal does not mention %q:\n%s", want, msg) + } + } + if strings.Contains(out, "regret") || strings.Contains(out, "Gate") { + t.Errorf("a scorecard was printed for --auto-tune=apply:\n%s", out) + } + + // propose is refused too — the nightly loop is brain wiring, not a verb. + _, err = runWhatIfErr(t, "--demo", "bursty", "--set", "cpu-headroom=1.05", + "--auto-tune=propose") + for _, want := range []string{"NewTuner", "nightly", "historyEnd", "--propose"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("the propose refusal does not mention %q:\n%s", want, err) + } + } + + // off is the default and is a no-op. + if out := runWhatIfOK(t, append(append([]string{}, acceptedArgs...), "--auto-tune=off")...); !strings.Contains(out, "ACCEPTED") { + t.Errorf("--auto-tune=off changed the answer:\n%s", out) + } + if _, err := runWhatIfErr(t, "--demo", "bursty", "--set", "cpu-headroom=1.05", + "--auto-tune=sideways"); !strings.Contains(err.Error(), "unknown mode") { + t.Errorf("an unknown --auto-tune mode was not rejected: %v", err) + } +} + +// TestEnforceRefusalsIsTheYardstickNotThePolicy. +// +// The same field is POLICY for `kilter backtest` and YARDSTICK for +// `kilter whatif`: whatif.Scenario.EnforceDecisionRefusals is shared by both +// replays so the two sides cannot be scored under different rules. A policy +// file that set it would be silently ignored, which is a what-if of a policy +// nobody ran — so it is refused by name. +func TestEnforceRefusalsIsTheYardstickNotThePolicy(t *testing.T) { + for _, flagName := range []string{"--policy", "--candidate"} { + path := writePolicy(t, `{"enforceDecisionRefusals": true, "recommend": {"cpuHeadroom": 1.05}}`) + _, err := runWhatIfErr(t, "--demo", "steady", flagName, path, + "--set", "memory-headroom=1.30") + if !strings.Contains(err.Error(), "not part of the policy in a what-if") { + t.Errorf("%s: err = %q, want the yardstick refusal", flagName, err) + } + if !strings.Contains(err.Error(), "--enforce-refusals") { + t.Errorf("%s: the refusal does not name the flag that works: %q", flagName, err) + } + } + // The flag itself applies to BOTH replays: turning it on must not change + // which policy each scorecard belongs to, and must move both sides. + off := whatifResult(t, "--demo", "regime-change", "--workloads", "3", "--set", "cpu-headroom=1.30") + on := whatifResult(t, "--demo", "regime-change", "--workloads", "3", "--set", "cpu-headroom=1.30", + "--enforce-refusals") + if off.BaselineScore.Policy != on.BaselineScore.Policy || + off.CandidateScore.Policy != on.CandidateScore.Policy { + t.Error("--enforce-refusals changed a policy hash; it is a yardstick knob, not a policy knob") + } + if on.BaselineScore.CPUStarvation == off.BaselineScore.CPUStarvation && + on.CandidateScore.CPUStarvation == off.CandidateScore.CPUStarvation { + t.Skip("the trace no longer starves; the A/B proves nothing") + } + if on.BaselineScore.CPUStarvation != 0 || on.CandidateScore.CPUStarvation != 0 { + t.Errorf("--enforce-refusals reached only one side: starvation baseline %d candidate %d", + on.BaselineScore.CPUStarvation, on.CandidateScore.CPUStarvation) + } +} + +// TestWhatIfWindowComesFromTheHistoryNotTheWallClock. +// +// whatif.Scenario takes no clock at all, so resolving the window is the CLI's +// whole job. A relative --from is measured back from the newest snapshot; a +// window past either end of the history is clamped and the clamp is REPORTED, +// because a run claiming 30 days over 7 days of history is a lie by omission. +func TestWhatIfWindowComesFromTheHistoryNotTheWallClock(t *testing.T) { + full := whatifResult(t, acceptedArgs...) + if got := full.Window[0].UTC().Format("2006-01-02T15:04:05Z"); got != "2026-01-05T00:00:00Z" { + t.Errorf("default window starts at %s, want the trace epoch", got) + } + + short := whatifResult(t, append(append([]string{}, acceptedArgs...), "--from", "3d")...) + if !short.Window[0].After(full.Window[0]) { + t.Errorf("--from 3d did not narrow the window: %v vs %v", short.Window[0], full.Window[0]) + } + if short.BaselineScore.Instants >= full.BaselineScore.Instants { + t.Errorf("--from 3d scored %d instants, not fewer than %d", + short.BaselineScore.Instants, full.BaselineScore.Instants) + } + out := runWhatIfOK(t, append(append([]string{}, acceptedArgs...), "--from", "3d")...) + if !strings.Contains(out, "newest snapshot") || !strings.Contains(out, "not the wall clock") { + t.Errorf("the anchor was not reported:\n%s", out) + } + + // A window wider than the history clamps, loudly, and scores the same as + // the whole history rather than pretending to cover 30 days. + wide := runWhatIfOK(t, append(append([]string{}, acceptedArgs...), "--from", "30d")...) + if !strings.Contains(wide, "clamped to the start of the history") { + t.Errorf("a 30-day window over a 7-day trace did not report a clamp:\n%s", wide) + } + wideR := whatifResult(t, append(append([]string{}, acceptedArgs...), "--from", "30d")...) + if !wideR.Window[0].Equal(full.Window[0]) { + t.Errorf("clamped window start %v, want %v", wideR.Window[0], full.Window[0]) + } + + // An inverted window is refused rather than replayed. + if _, err := runWhatIfErr(t, "--demo", "bursty", "--set", "cpu-headroom=1.05", + "--from", "2026-01-11T00:00:00Z", "--to", "2026-01-06T00:00:00Z"); !strings.Contains( + err.Error(), "empty or inverted") { + t.Errorf("an inverted window was accepted: %v", err) + } + if _, err := runWhatIfErr(t, "--demo", "bursty", "--set", "cpu-headroom=1.05", + "--from", "yesterday"); !strings.Contains(err.Error(), "--from") { + t.Errorf("a bad --from was not reported: %v", err) + } +} + +// TestWhatIfHumanOutputReportsTheNumbersItClaims. +// +// The rendered text is an independent restatement of the JSON, so it can drift +// from it. Every figure the human form prints is checked against the decoded +// result — this is the shape of bug PR#41 found in `kilter backtest`, where a +// percentage was scaled twice and printed 9,445 % for a real 94.5 %. +func TestWhatIfHumanOutputReportsTheNumbersItClaims(t *testing.T) { + for _, args := range [][]string{acceptedArgs, rejectedArgs} { + r := whatifResult(t, args...) + out := runWhatIfOK(t, args...) + + for _, want := range []string{ + r.Cluster, + r.BaselineScore.Policy, + r.CandidateScore.Policy, + signedUSD(r.Delta.RegretUSD), + signedUSD(r.Delta.ResourceRegretUSD), + signedUSD(r.Delta.RiskRegretUSD), + signedUSD(r.Delta.ProjectedMonthlyUSD), + signedInt(r.Delta.MemViolations), + signedInt(r.Delta.CPUStarvation), + signedFloat(r.Delta.OracleGapPct, 1), + usd(r.Gate.RequiredRegretImprovementUSD), + r.Window[0].UTC().Format("2006-01-02T15:04:05Z"), + } { + if !strings.Contains(out, want) { + t.Errorf("%v: output does not contain %q:\n%s", args, want, out) + } + } + // The oracle gap is printed in the units the scorecard uses: it is + // ALREADY scaled by 100 inside pkg/backtest. + if !strings.Contains(out, "oracleGap") { + t.Errorf("%v: no oracle gap in the output", args) + } + verdict := "REJECTED" + if r.Gate.Passed { + verdict = "ACCEPTED" + } + if !strings.Contains(out, verdict) { + t.Errorf("%v: output does not say %s:\n%s", args, verdict, out) + } + for _, reason := range r.Gate.Reasons { + if !strings.Contains(out, reason) { + t.Errorf("%v: gate reason %q was not printed", args, reason) + } + } + // Wins are printed even on a rejection: hiding them makes every + // rejection look alike. + for _, win := range r.Gate.Wins { + if !strings.Contains(out, win) { + t.Errorf("%v: win %q was not printed", args, win) + } + } + // The projection is labelled as one, everywhere it is printed. + if !strings.Contains(out, "projected") { + t.Errorf("%v: the monthly figure is not labelled a projection", args) + } + } +} + +// TestWhatIfSignsAreRenderedSoTheDirectionCannotBeMisread. +func TestWhatIfSignsAreRenderedSoTheDirectionCannotBeMisread(t *testing.T) { + r := whatifResult(t, acceptedArgs...) + if !(r.Delta.RegretUSD < 0) { + t.Fatalf("the accepted case no longer improves regret (%v)", r.Delta.RegretUSD) + } + out := runWhatIfOK(t, acceptedArgs...) + if !strings.Contains(out, "-$") { + t.Errorf("an improvement was not rendered with a minus sign:\n%s", out) + } + rej := whatifResult(t, rejectedArgs...) + if !(rej.Delta.RegretUSD > 0) { + t.Fatalf("the rejected case no longer regresses regret (%v)", rej.Delta.RegretUSD) + } + if !strings.Contains(runWhatIfOK(t, rejectedArgs...), "+$") { + t.Error("a regression was not rendered with a plus sign") + } +} + +// TestFailOnNoImprovementIsTheCIGate. +func TestFailOnNoImprovementIsTheCIGate(t *testing.T) { + // The rejected candidate is reported, and without the flag the command + // still succeeds: a what-if that answers "no" answered the question. + out := runWhatIfOK(t, rejectedArgs...) + if !strings.Contains(out, "REJECTED") { + t.Fatalf("the gate accepted a regression:\n%s", out) + } + rejected := append(append([]string{}, rejectedArgs...), "--fail-on-no-improvement") + out, err := runWhatIfErr(t, rejected...) + if !strings.Contains(err.Error(), "did not pass the gate") { + t.Errorf("err = %v, want the gate failure", err) + } + if !strings.Contains(out, "REJECTED") { + t.Errorf("the reasons were not printed alongside the failure:\n%s", out) + } + // And it does not fire on an accepted candidate. + runWhatIfOK(t, append(append([]string{}, acceptedArgs...), "--fail-on-no-improvement")...) +} + +// TestWhatIfOutputIsByteIdenticalAcrossRuns. Go randomizes map iteration on +// every range, so repeating in ONE process is the real determinism test. +func TestWhatIfOutputIsByteIdenticalAcrossRuns(t *testing.T) { + args := []string{"--demo", "bursty", "--noise", "0.05", "--workloads", "3", + "--set", "cpu-headroom=1.05", "--set", "base-soak=8h"} + base := runWhatIfOK(t, args...) + baseJSON := runWhatIfOK(t, append(append([]string{}, args...), "--json")...) + for i := 0; i < 6; i++ { + if got := runWhatIfOK(t, args...); got != base { + t.Fatalf("text run %d differs", i) + } + if got := runWhatIfOK(t, append(append([]string{}, args...), "--json")...); got != baseJSON { + t.Fatalf("json run %d differs", i) + } + } + // --set order must not change the answer: the same multiset of overrides + // is the same candidate policy. + swapped := runWhatIfOK(t, "--demo", "bursty", "--noise", "0.05", "--workloads", "3", + "--set", "base-soak=8h", "--set", "cpu-headroom=1.05") + if swapped == base { + return // identical rendering including the "set:" echo line + } + // The echo line records the order the operator typed; everything computed + // from it must not. + stripSet := func(s string) string { + var keep []string + for _, line := range strings.Split(s, "\n") { + if !strings.HasPrefix(line, "set: ") { + keep = append(keep, line) + } + } + return strings.Join(keep, "\n") + } + if stripSet(swapped) != stripSet(base) { + t.Error("reordering --set changed the result") + } +} + +// TestWhatIfJSONGolden pins the byte-stable --json form. +// +// Result.Encode() is what `--json` writes verbatim and what a CI job diffs, so +// a change to it is a change to a published interface. Regenerate with: +// +// go test ./cmd/kilter -run TestWhatIfJSONGolden -update-fixtures +func TestWhatIfJSONGolden(t *testing.T) { + got := runWhatIfOK(t, "--demo", "bursty", "--set", "cpu-headroom=1.05", "--json") + path := fixturePath("whatif-bursty.json") + if *updateFixtures { + if err := os.MkdirAll("testdata", 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(got), 0o644); err != nil { + t.Fatal(err) + } + t.Logf("wrote %s (%d bytes)", path, len(got)) + return + } + want, err := os.ReadFile(path) + if err != nil { + t.Fatalf("%v (run with -update-fixtures to create it)", err) + } + if got != string(want) { + t.Errorf("kilter whatif --json drifted from %s.\n"+ + "If this is intentional: go test ./cmd/kilter -run TestWhatIfJSONGolden -update-fixtures\n"+ + "got %d bytes, want %d", path, len(got), len(want)) + } + // The golden is also a round-trip check: it must decode into a Result + // whose gate verdict is the one the golden claims. + var r whatif.Result + if err := json.Unmarshal(want, &r); err != nil { + t.Fatalf("the golden does not decode: %v", err) + } + if !r.Gate.Passed { + t.Error("the golden pins a REJECTED comparison; pin an accepted one, " + + "so a regression in the gate is visible here") + } +} + +// TestWhatIfPolicyFileFailsLoudly — the same contract `kilter backtest` has, +// through this command's loader. +func TestWhatIfPolicyFileFailsLoudly(t *testing.T) { + for _, tc := range []struct{ name, body, want string }{ + {"unknown field", `{"recommend": {"cpuHeadrooom": 1.5}}`, "unknown field"}, + {"bare duration", `{"decision": {"baseSoak": "3600"}}`, "durations need a unit"}, + {"not json", `{`, "unexpected EOF"}, + } { + t.Run(tc.name, func(t *testing.T) { + path := writePolicy(t, tc.body) + out, err := runWhatIfErr(t, "--demo", "steady", "--candidate", path) + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error = %q, want it to mention %q", err, tc.want) + } + if strings.Contains(out, "regret") { + t.Errorf("a comparison was printed for a broken policy file:\n%s", out) + } + }) + } + // A candidate file and --set compose: the file is the base, --set moves it. + path := writePolicy(t, `{"recommend": {"cpuHeadroom": 1.05}}`) + r := whatifResult(t, "--demo", "steady", "--candidate", path, "--set", "memory-headroom=1.30") + axes := map[whatif.Axis]float64{} + for _, c := range r.Changes { + axes[c.Axis] = c.To + } + if !near(axes[whatif.AxisCPUHeadroom], 1.05, 1e-9) { + t.Errorf("the candidate file's cpu-headroom was lost: %+v", r.Changes) + } + if !near(axes[whatif.AxisMemoryHeadroom], 1.30, 1e-9) { + t.Errorf("--set did not apply on top of the candidate file: %+v", r.Changes) + } +} + +// TestWhatIfNeedsACandidate: a what-if with nothing under test is not a +// question, and the usage text is printed rather than an empty comparison. +func TestWhatIfNeedsACandidate(t *testing.T) { + out, err := runWhatIfErr(t, "--demo", "steady") + if !strings.Contains(err.Error(), "needs a candidate") { + t.Errorf("err = %v", err) + } + if !strings.Contains(out, "--set AXIS=VALUE") { + t.Errorf("the usage text was not printed:\n%s", out) + } + if _, err := runWhatIfErr(t); !strings.Contains(err.Error(), "is required") { + t.Errorf("whatif with no source of history: err = %v", err) + } +} + +// TestWhatIfHelpQuotesTheEnforcedBounds: the help text is generated from +// whatif.HardBounds(), in whatif.AllAxes order, so it is byte-stable and +// cannot drift from the values actually enforced. +func TestWhatIfHelpQuotesTheEnforcedBounds(t *testing.T) { + first := whatifUsage() + for i := 0; i < 5; i++ { + if whatifUsage() != first { + t.Fatalf("the usage text is not byte-stable (run %d)", i) + } + } + last := -1 + for _, a := range whatif.AllAxes { + i := strings.Index(first, "\n "+string(a)+" ") + if i < 0 { + t.Fatalf("axis %s is missing from the help text", a) + } + if i < last { + t.Errorf("axis %s is out of AllAxes order in the help text", a) + } + last = i + } + for axis, r := range whatif.HardBounds() { + want := formatBound(r.Min) + ", " + formatBound(r.Max) + if !strings.Contains(first, want) { + t.Errorf("help text does not quote %s's bounds [%s]", axis, want) + } + } +} + +func formatBound(v float64) string { + return strings.TrimSuffix(jsonNumber(v), ".0") +}