M4B (PR #39) follow-up: 7 bot-flagged bugs + cargo fmt CI red

[Madreag]

M4B (PR #39) merged with 7 actionable bot findings deferred to follow-up. The PR also merged with red CI (workspace-wide cargo fmt drift in pre-existing files outside M4B's scope — see #cargo-fmt below).

Bot findings to address

P1 (Codex blocker bugs)

1. read_quicksave silently allows missing checksum sidecar

• File: game/crates/cf-save/src/quicksave.rs::read_quicksave
• Issue: When the .cfsave.checksum sidecar is absent (e.g., attacker deletes it), the loader treats expected_checksum as None and skips verification. Defeats the entire "Save corruption is detectable" promise.
• Fix sketch: Require the sidecar; return new SaveError::MissingChecksum { path } variant when absent or empty. Add SaveError::Io { path, source } for clean filesystem errors so they don't masquerade as MigrationFailed.
• Tests to add:
  • read_refuses_when_checksum_sidecar_is_missing
  • read_refuses_when_checksum_sidecar_is_empty
  • missing_save_file_returns_io_error_not_migration_failed

2. Chain encoder advances BEFORE backpressure drop handling

• File: game/crates/cf-replay/src/lib.rs::record_with_cosmetic
• Issue: When capacity is hit and an event is dropped or a cosmetic is evicted, the encoder has already advanced the chain state. The on-disk events.jsonl no longer matches the chain — verification reports false tampered on legitimate bundles under recorder pressure.
• Two viable fixes:
  • (a) Defensive guard (small): Recorder::enable_chain_mode panics when capacity is non-zero. Chain mode = unlimited capacity by contract. ~10 LOC.
  • (b) Real refactor (correct but larger): Defer chain computation to write time. Remove eager encoding from record_with_cosmetic; have cf-replay::write_run_bundle walk the final retained events and stamp prev_event_hash + chained_hash_hex over them. ~80-100 LOC + test updates.
• Recommendation: ship (a) first as a follow-up PR (zero risk); land (b) when M11/M12 refactor settles.

P2 (Codex medium bugs)

3. cfctl save migrate <path> ignores the file path argument

• File: game/crates/cfctl/src/main.rs::cmd_save_migrate
• Issue: Always migrates quicksave.cfsave in path.parent(), regardless of what <path>.cfsave the user supplied. In a multi-save directory it migrates the WRONG file while printing a misleading success envelope.
• Fix sketch: Read the exact file at path, deserialize as WorldSave, call migration::migrate(blob, target) directly, write back to the same path with a fresh sidecar checksum. Refuse non-current --to <version> until partial-migration handlers ship.

4. Ledger chain doesn't bind event_id into hash material

• File: game/crates/cf-save/src/ledger_chain.rs::Encoder::compute_hash
• Issue: Accepts event_id but hash is only (prev_hash, payload). Rewriting an event_id field in events.jsonl passes verify_chain unchanged, even though event_id is part of replay/audit semantics (M10 cause-chain walker, M4 envelope schema).
• Fix sketch: Bind event_id into the hash material: prev_hash || '|' || event_id || '|' || canonical_payload. Add regression test: verify_rewriting_event_id_returns_tampered.

Cursor (medium/low bugs)

5. cf-headless save migrate --to <version> flag is broken

• File: game/crates/cf-headless/src/save_migrate.rs::run_migrate
• Issue: Same root cause as Refresh Rust toolchain, CI actions, and direct crates #3 — cf_save::quicksave::read_quicksave always migrates to CURRENT_SAVE_SCHEMA_VERSION internally, so the post-check outcome.save.schema_version != requested always fails for any non-current target. The flag accepts every input then rejects every value.
• Fix sketch: Same shape as Refresh Rust toolchain, CI actions, and direct crates #3 — read the file directly, call migration::migrate(blob, target) with the parsed --to value, write back. Reject targets newer than the current build's schema.

6. cf-mod save validate reports blake3_hex: Some("") when sidecar missing

• File: game/crates/cf-mod/src/save_validate.rs (lines 101-117)
• Issue: When the .checksum sidecar is absent, expected_checksum.unwrap_or_default() yields an empty string. The audit envelope reports blake3_hex: Some("") — a nominally-present-but-meaningless hash. The actual canonical-JSON BLAKE3 of the (possibly migrated) save is never computed.
• Fix sketch: When sidecar missing, compute outcome.blob.checksum_hex()? and report THAT in the envelope. Distinguish in the report between "verified against sidecar" and "computed (no sidecar)".

Devin (1 bug + 7 informational; only the bug requires action)

7. Viewer migration banner hardcodes (handler: v1_to_v2)

• File: game/crates/cf-tools-replay-viewer/src/viewer.rs (lines 121-125)
• Issue: When recorded save_schema_version differs from current, banner unconditionally prints (handler: v1_to_v2). If a v2.0.0 bundle is opened by a v3.0.0 build (with a v2_to_v3 handler), it would incorrectly display Replay migrated from v2.0.0 -> v3.0.0 (handler: v1_to_v2).
• Fix sketch: Either omit the parenthetical entirely OR call cf_save::migration::migrate(blob, current) on a stub deserialized from the bundle and surface the actual outcome.handler_chain in the banner.

Informational analyses (NO action required)

Devin also flagged 7 informational findings; all are benign and intentional per the M4B design:

• fire_quickload pattern-match is safe because SaveSchemaVersion derives Copy
• is_multiple_of(cadence) is guarded by the early cadence == 0 return
• WorldSave::deserialize checksum verifies re-serialized canonical form (intentional, deterministic via BTreeMap key ordering + DR-052 float rules)
• Quicksave checksum sidecar is not atomically written (acceptable trade-off; Land M0 native engine bootstrap (29-crate workspace, cf-control + cfctl + cf-replay) #1 above forces refusal-on-missing so a half-write is detected)
• delta::diff doesn't recurse into nested objects within arrays (intentional flat replacement; bench still hit 12.25x)
• emit_m4b_snapshot_for_tick multi-lock pattern is TOCTOU-safe because drive_tick is single-threaded
• save_validate.rs checksum sidecar path construction is correct for both .cfsave and no-extension inputs

CI status (red on PR #39)

All 3 platforms (ubuntu / macos / windows) failed at cargo fmt --check. Diff is in pre-existing files outside M4B scope (cf-actor/src/gib.rs, cf-actor/src/sim.rs) AND new M4B code (cf-app/src/main.rs, etc.). Single-line fix: cd game && cargo fmt --all then commit + push. Should land as the same follow-up PR that addresses the 7 findings above.

Recommended ordering for the follow-up PR(s)

1. Tiny unblock PR — cargo fmt --all only. Gets CI green for main.
2. Bot-findings PR — fixes Land M0 native engine bootstrap (29-crate workspace, cf-control + cfctl + cf-replay) #1, M1: Actor Controller And Sim Core #2(a), Refresh Rust toolchain, CI actions, and direct crates #3, Add dependency drift reporting and refresh README #4, M1.5: Micro Breach Fun Slice #5, T-CAPTURE: cf-capture frame readback + grid composer + cf-e2e wiring (BP2+ Roadmap V2 side track) #6, T-RELEASE: per-BP cross-platform GitHub Releases (BP1+ side track) #7. ~250-400 LOC.

References

• PR: M4B: save format versioning + delta-encoded snapshots + tamper-evident ledger chain #39
• Original spec: specs/done/M4B.md
• Failed CI run: https://github.com/Madreag/corefall/actions/runs/25978175064