sots-engine/docs/harness-audit.md

23 KiB
Raw Blame History

Harness audit: undeclared side effects

Status: framework change landed, host tests green (31/31). Nothing has been run on the VM. Five hooks changed their region set and need their golden traces recaptured (see §5).

B3 shipped a result that read as evidence and was not: 13 of 15 compares clean, zero divergences on the hook that matters — while replace mode wrote an autosave that differed from the oracle by one EVENT_RESEARCH_OVERBUDGET entry. The event is appended to the owner's event list. The event list was never a declared region. The diff could not see it, so it said nothing, and "said nothing" was read as "found nothing".

That is not a B3 bug. It is a property of the harness: a declared region is a check, and everything else is invisible. Every hook written so far had it. This document is the audit, the mechanism that makes the blind spot visible, and what still cannot be checked.


1. The audit table

Ranked by how likely each undeclared effect is to hide a real divergence: how directly it feeds simulation state, whether any layer of the harness could currently notice it, and how often it fires.

# hook undeclared side effect of the original why it can hide a divergence now
1 TechTree::ProcessResearch (B3) posts EVENT_RESEARCH_OVERBUDGET on the owner's EventStorage (ServerPlayer+0x29c), bumping EvNxID and the turn's list the known defect. Same branch that sets the modelled node.flag = 2; serialized into the save, so it changes the oracle declared as region events; ours still does not post it, so it now diverges loudly instead of passing
2 ServerPlayer::OnTechResearched (B2) posts EVENT_RESEARCH_COMPLETE / _UNDERBUDGET / _TEMPERANCE on the same EventStorage identical shape to #1, and worse: B2 has no replace-mode oracle behind it — docs/B2.md gotcha 4 says a changed save hash on a completion turn is expected, so nothing would have flagged it guard region player reports it
3 ServerSystem::ProcessTurn (B4) the addiction sweep constructs MoraleEvents and appends them to the system's capped history same shape again. Worse still: sim::ProcessColonyTurn computes the morale events, and DescribeMoraleEvents exists — but the hook never calls it, so they are dropped on the floor: not compared, not even logged guard region system reports it; the dead describer is called out below
4 StrategyServer::ProcessFleetMovement (B4) OnFleetArrived posts EVENT_FLEET_ARRIVED; the pass also writes FPdpos into every fleet and clears flags 0x2 and 0x100 on every fleet fourth instance of the same class. No replace mode for this hook either, so neither layer could see it coverage notes only (a guard would have to span every fleet; see §6)
5 StrategyServer::ProcessFleetMovement (B4) ours re-reads the LIVE fleet list after the original has run breaks the compare invariant that ours never touches game memory, and makes the verdict partly self-fulfilling: our gate-traffic total is computed from the original's own post-move state coverage note, risk high
6 ServerPlayer::ComputeBudget (B1) ServerSystem::ComputeOutput repairs damaged ships in orbit; replace mode runs the original a second time on a scratch Budget to harvest six unmodelled slots a real double-effect per turn in replace mode, on objects no region reaches. B1's oracle held, so it is either benign or was not exercised — unknown, not cleared coverage note, risk high; unverifiable from inside this hook
7 ServerPlayer::OnTechResearched (B2) writes other objects: every owned system's AI flag, the arcology civilian-cap re-evaluation, the addiction cure, the plague clear across systems and ships writes through pointers the region model cannot follow; all of it is simulation state coverage note; out of reach of any guard on the player
8 TechTree::ProcessResearch (B3) SetResearched: turn/order stamps, the child-unlock cascade, the recursive research of zero-cost children, and the owner's tech-effect callback (which consumes one RNG word) the extra draw desynchronises the generator, which is a declared region — so it surfaces, but as an unexplained RNG divergence rather than as its cause coverage note + guard tree_header
9 TechTree::ProcessResearch (B3) bumps the tree's completion-order counter at TechTree+0x20 the per-node order word is compared but the counter it is read from was not a region — a compare could match every node and still be running on a different counter guard region tree_header
10 StrategyServer::MoveFleet (B4) departure cancels every still-acting ship and calls FleetDeparts (system ownership bits); arrival dispatches SEFleetArrived and one of three arrival handlers the declared boundary, but "expected to differ" was never measured: an arriving call was simply clean guard region fleet (the fleet's own words only)
11 ServerPlayer::ComputeBudget (B1) the over-budget int at Budget+0x64 the original writes it. B1 captured it only as an argument (tail_before), and args are never compared — so it was neither checked nor watched guard region budget_object
12 WeaponDictionary::Init / SectionDictionary::SectionDictionary (M2) LoadWeapon / LoadSection register every definition's name with the string table and resolve requires against the live TechTree ours delegates to those same functions after the original has run, so a compare run performs every registration a second time. This is the leading hypothesis for the section hook's compare-mode crash coverage notes, risk high; compare mode for the section hook remains unsafe
13 ServerPlayer::OnTechResearched (B2) the pending plague-cure roll draws exactly one RNG word, unconditionally it is the extra draw B3 measured. ours clears the guarded words and reports rather than rolling — correct, but it means the generator is not verified across this hook coverage note
14 ServerSystem::ProcessTurn (B4) every callee: plague, imperial/civilian growth, the resource debit, in-orbit refuel, slaves, rebellion, the build queue (creates ships, bumps per-player ShipRecords, raises SEBuildCompleted) the declared input boundary. Large, and previously invisible: a clean compare on ten words implied nothing about the other 700 bytes of the system guard region system measures how much moves
15 ServerPlayer::ComputeBudget (B1) six slots (1, 2, 3, 4, 7, 11) are copied out of the original's own output back into the same slots they match by construction and prove nothing, but they were counted in the headline "4437 compares, 0 divergences" coverage note, risk high
16 Mars::GlobalConsts::LoadFile (M1) erases each consumed key from the caller's std::map (a red-black tree unlink + free) the erase is what implements first-occurrence-wins. The effect is modelled in game::config::apply; the container is not, and the map is a LoadAll temporary that no region can reach coverage note
17 ServerPlayer::OnTechResearched (B2) allocates or frees the node-bore block at ServerPlayer+0x308 ours has no allocator the game's runtime could free, so replace mode calls the game's own updater — meaning replace mode never exercises our node-bore selection at all coverage note; region node_bore exists only when the block already does
18 StrategyServer::MoveFleet (B4) a node-line waypoint's step comes from the stutter profile; a missed probabilistic jump scatters the fleet in a random direction (second draw) NodeLineStep / BuildStutterSegments are written and unit-tested but not wired in: the hook steps every waypoint type as speed × dt, so a node-line leg is knowingly mis-stepped coverage note (a declared gap, but it was only in prose)
19 ServerSystem::ProcessTurn (B4) ApplyInfraBonus / ApplyPopBonus read the owner's home-system id; the build queue writes the owning ServerPlayer writes through a pointer to another object coverage note
20 StrategyServer::ProcessFleetMovement (B4) the original accumulates by player->index but writes back by the player's position in the server vector, into a fixed 32-int array with no bounds check a latent bug in the original that ours reproduces only while index == position; the reference save never separates them coverage note
21 StrategyServer::ProcessFleetMovement (B4) PassSchedule() is never called by the hook and FleetSummary::targetFleetId / relation are never filled the header claims ours predicts the call order "so a trace can be checked against it". It does not. DescribeGateTraffic and DescribeFleetStep are dead too coverage note
22 M1 / M2 / B3 log output (three GlobalConsts messages, missing-manifest lines, the research completion line) not simulation state; recorded for completeness coverage notes, risk low
23 M1 / M2 heap traffic: M1's long-string leak per compare call; M2's 123 WeaponDef (0x278 B) and 885 SectionDef (0x3d8 B) allocations, both leaked per compare run start-up only, but they are real allocator effects a save-hash oracle cannot see coverage notes, risk low

Things I could not settle offline, and did not guess:

  • Whether the double ComputeOutput in B1 replace mode (#6) actually double-repairs ships. It needs a hook on ComputeOutputFromRates, or a VM run with a damaged ship in orbit.
  • The exact stride of EventStorage::TurnEvents, so the events region reports the outer vector's byte span (turns_bytes) rather than an element count. EvNxID is the field that actually carries the signal.
  • Whether the section dictionary's LoadSection appends to the dictionary's own vector (docs/M2.md raises it as a hypothesis). Still a hypothesis; recorded as risk: high.
  • ServerPlayer / ServerSystem / StarFleet sizes come from the recovered object table in sots-re/findings/, not from the instruction stream. The guard spans use them, and each guard is behind a readable() probe so an over-long span degrades to "no guard" rather than to a fault.

2. The mechanism

Four changes, chosen so the failure mode is noisy rather than silent, and so keeping them correct does not depend on anyone remembering to.

2.1 Coverage — a compile-time-required admission (the honesty layer)

Every descriptor must define

static void coverage(Coverage& c);

and either list what it does not model or explicitly claim it models everything:

c.unmodelled("posts EVENT_RESEARCH_OVERBUDGET on the owner's EventStorage",
             Risk::High, "the message text is composed from the tech name",
             "region:events");
c.complete("Fill writes buf[0..n) and nothing else");   // the other option

Hook<D> carries a static_assert on it, so a hook without a coverage statement does not compile. The notes go into meta.hooks.<H>.coverage, and tracecmp.py prints them under every report — including a clean one. A passing run now always ends with a paragraph saying what it did not check.

Three states: complete, partial (the honest normal case — all nine game hooks), and unstated. unstated is unreachable in new code and is what an old log reads as; the tracer logs it to shim.log at start-up and tracecmp.py warns per hook.

Why this and not just documentation: the boundary prose already existed — B2's header lists its four unmodelled effects almost exactly. It was in a comment, so it never reached the report a human actually reads after a run.

2.2 Guard regions — the part that finds what nobody wrote down (the detection layer)

Region gains a kind:

  • Result — the classic region: snapshotted, copied to scratch, handed to ours, diffed. This is the verdict.
  • Guard — a coarse span (usually the whole object). Snapshotted before and after the original only. Never copied to scratch, never given to ours, never diffed. After the call, every byte run inside it that moved and that no Result region covers is emitted as an undeclared write.

That masking step is what makes it usable: a guard over the whole ServerPlayer during B2 reports the event storage and the plague words while staying quiet about the thirteen field groups the descriptor already checks. A guard is a few hundred bytes of memcpy — cheap enough to put over every object a hook can reach.

Why not a checksum: a hash tells you something moved. A masked byte-run diff tells you where, which is the difference between "B2 diverges somewhere" and "B2 wrote player+0x2b0, which is EvNxID".

Why guards do not fail the run by default: on a hook that admits its boundary, guard hits are the expected consequence of that boundary, and treating them as failures would train people to ignore them. They are always reported. They fail the run in exactly one case — the one that matters:

a hook whose coverage.state is complete while a guard caught an undeclared write has been proved wrong about its own model. tracecmp.py counts that as a divergence (exit 1).

So a hook can only produce a clean, quiet pass by claiming completeness — and if that claim is false, the guard turns it into a failure. --strict-coverage widens failure to any undeclared write or any unstated hook, for a gate that wants zero tolerance.

Scratch is built from Result regions only, so a guard never occupies a scratch slot and existing descriptors' recorded indices keep working — provided guards are pushed last. regions_well_ordered() checks that on every call and turns a violation into an err, so the footgun cannot fire quietly either.

2.3 replace mode emits records

replace used to log nothing, which is precisely why B3's divergence could only be found by hashing a 609 KB autosave. It now writes one record per call — the same regions, measured around ours — plus its own guard findings. Consequences:

  • a replace run is inspectable and --replay-able instead of pass/fail on a save hash;
  • guards in replace mode report what ours writes outside the declared regions;
  • comparing the two, tracecmp.py names the asymmetry directly: ONLY the original writes these: player+0x2b0:4. That line is the B3 bug, printed.

2.4 tracecmp.py coverage reporting

Every report now ends with a coverage section: per hook, the verdict, the regions actually compared (derived from the records, not from a claim), the guards, the undeclared writes they caught with byte spans, and the descriptor's unmodelled list. --json-out carries the same under hooks.<H>.coverage plus top-level coverage_unstated / coverage_contradicted and totals.{guarded_calls, undeclared_calls, undeclared_writes, coverage_unstated, coverage_contradicted}. Exit codes are unchanged (0 / 1 / 2).

What I rejected

  • A checksum-only guard. Detects, but does not localise; a moved hash on a 1 KB object is barely more actionable than no signal at all.
  • A replace-mode assertion against a compare-mode snapshot. Attractive, but the two runs are different game sessions with different call ids and different RNG positions; matching them up is the save-file oracle's job, and it already exists. Emitting replace records plus the guard-span asymmetry gets most of the value with none of the plumbing.
  • Declaring the event list as a Result region on B2 and B4 as well. It would turn every completion and every colony turn into a divergence for a reason already known, drowning real signal. B3 gets a Result region because closing that gap is B3's remaining work; the others get guards, which report without polluting the verdict.

3. What changed

Framework (src/shim/trace/)

  • emitter.h/.cpp: Risk, CoverageNote, Coverage, HookMeta, UndeclaredWrite; record fields has_coverage / guards / undeclared / undeclared_total; emission of meta.hooks.<H>.coverage and the record's coverage block.
  • tracer.h/.cpp: Region::Kind; undeclared_writes() (masked byte-run diff); regions_well_ordered(); register_hook(name, policy, coverage); unstated_hooks().
  • hook.h: the static_assert; Result/Guard split in run<>; run_replace(); the region-order check.
  • selftest.{h,cpp}: Blob + FillCounted + FillGuardHook / FillGuardLyingHook — the B3 shape in miniature (a Result region ours reproduces exactly, next to a counter only the original writes).
  • main.cpp: logs COVERAGE: for any hook registered without a statement.

Descriptors — coverage() on all nine, plus:

hook region change
TechTree::ProcessResearch + Result events (owner+0x29c, 0x1c, {turns_bytes, next_id, vec_*}); + Guard player (owner, 0x3e0); + Guard tree_header (tree, 0x24)
ServerPlayer::OnTechResearched + Guard player (self, 0x3e0)
ServerPlayer::ComputeBudget + Guard budget_object (budget, 0x68)
ServerSystem::ProcessTurn + Guard system (self, 0x2d8)
StrategyServer::MoveFleet + Guard fleet (fleet, 0x120)
the other four coverage notes only; regions unchanged

Harness (sots-re/verify/harness/compare/)

  • tracecmp.py: coverage in RECORD_KEYS; validate_coverage() / validate_meta_coverage(); per-hook coverage accounting; the coverage report section including the compare-vs-replace asymmetry line; --strict-coverage; contradicted claims count as divergences.
  • mkfixture.py: coverage in KEY_ORDER; coverage() / guard_block() helpers; default coverage in meta(); three new fixture logs (coverage_guarded, coverage_lying, coverage_unstated).
  • test_tracecmp.py: a CoverageTest class (8 cases) — meta coverage reaching the report, guard findings reported without failing an honest hook, --strict-coverage, a false completeness claim exiting 1, unstated logs being called out, block validation, an invalid block failing the record, and the compare-vs-replace asymmetry line.
  • TRACE_FORMAT.md: coverage in the record table and the key order, coverage in the meta example, new section 8.

Tests: tests/shim_trace/test_coverage.cpp (new ctest shim_trace_coverage) instantiates every descriptor's Hook<> on the host — which is what fires the static_assert, since the descriptors otherwise only reach a compiler on the MinGW cross build — and checks that every hook states its coverage, that no note is empty, and that the guard machinery reports the undeclared write, stays quiet when there is none, and counts honestly when it truncates. test_emitter.cpp and test_hook.cpp updated for the new meta/record shape and for replace mode now emitting.

ctest: 31/31 green (was 30/30; +shim_trace_coverage). test_tracecmp.py: 38 tests, 2 failures — both pre-existing and unrelated (OracleBridgeTest::test_per_kind_output and ::test_cli_and_replay_round_trip, in oracle_parsers.py's bare-item key and duplicate-id error count; failing identically before this change).


4. What a report looks like now

- calls: 12  compared: 12  diverged: 0  invalid records: 0  warnings: 0
- coverage: 12 guarded call(s), 4 undeclared write(s) in 4 call(s); 0 hook(s) unstated, 0 contradicted

### coverage

| hook | verdict | compared regions | guards | undeclared writes | unmodelled |
|---|---|---|---|---|---|
| Game::TechTree::ProcessResearch | partial | events, node[0], … +214 | player, tree_header | 4 in 4 call(s) | 5 |

#### Game::TechTree::ProcessResearch — not checked by this run
- (high) posts EVENT_RESEARCH_OVERBUDGET on the owner's EventStorage …  [region:events]
- guard hits in compare mode: player+0x2b0:4
- ONLY the original writes these: player+0x2b0:4

The last three lines are the B3 defect, stated by the harness, on a run that reports zero divergences elsewhere.


5. Golden traces that need recapturing

A region-set change invalidates a golden trace (docs/M2.md made this mistake once already). These five need a fresh capture on the VM before their logs mean anything:

hook golden why
Game::TechTree::ProcessResearch b3-trace-golden.jsonl new events side entry + two guards; expect new divergences on over-budget and completion calls — they are the defect becoming visible, not a regression
Game::ServerPlayer::ComputeBudget b1-trace-golden.jsonl new coverage block per record (side entries unchanged, so the diff verdict should be identical: 0 divergences)
Game::ServerPlayer::OnTechResearched none yet (B2 never captured) capture with the guard in place from the start
Game::ServerSystem::ProcessTurn none yet (B4 never captured) ditto
Game::StrategyServer::MoveFleet none yet (B4 never captured) ditto

m1-trace-golden.jsonl and m2-trace-golden.jsonl keep their side entries; only their meta line grows a coverage object. They do not need re-capturing to stay valid, but any strict byte-comparison against them will differ on line 1.

First VM run to do: B3 in compare mode on the reference save, --json-out to verify/results/compare/. Two things to read: whether side.events.after.v.next_id diverges on exactly the over-budget call (it should — that is the mechanism working), and what player / tree_header report as undeclared writes on a completion call, which is the cheapest available map of what SetResearched actually touches.


6. What still cannot be checked

Honest limits of the mechanism, so nobody reads a clean coverage section as more than it is.

  • Writes to other objects reached through pointers. A guard covers one span. B2's writes to every owned system and every ship, B4's writes to other fleets and to the owning player, and ComputeBudget's ship repair are all outside every guard. Covering them means declaring a guard per reachable object — possible for a bounded set (the owner, the tree), not for "every ship in the empire". These stay coverage notes.
  • Heap allocation and container growth as such. A guard sees a std::vector's three words move; it does not see the elements, and it cannot see a std::map node unlinked from a tree that is not in the span. M1's map erase and M2's definition allocations are structurally out of reach of a before/after byte diff.
  • Log output, file I/O, and anything outside the process's data. Not modelled and not modellable this way.
  • The message text of an event. events proves an event was or was not posted, and which counter moved. It cannot prove the text, which the game composes from the tech name. Closing B3 properly still means calling the game's own event API from ours, the way M2 delegates to LoadWeapon.
  • A guard cannot distinguish "the original wrote X" from "a callee of the original wrote X". It reports the byte, not the writer. That is usually enough to start, and never enough to finish.
  • Anything on a call the hook never sees. An empty trace still proves nothing. The coverage section reports not watched for a hook with no guarded calls, which at least makes the absence explicit.
  • B2 and B4 remain entirely unverified against the game. Every claim in their rows above is read from the disassembly and the RE notes, not measured. The guards are how they will be measured.