AntFleet

Disagreement · b1d71d08-openai-0

State file rollback uses non-existent .bak and can cause data loss on write failure

mismatch
repo 6f7fc663·PR #24·reviewed 1 week ago

Primary finding

State file rollback uses non-existent .bak and can cause data loss on write failure

highdata-losshigh
  • skills/fleet-state/SKILL.md:291-306
  • skills/fleet-state/SKILL.md:308
The write flow moves the new file into place before validation, then attempts to restore from a .bak that is never created in this step (and not created anywhere else in the document). If jq empty fails (e.g., interrupted write, disk error), the restore path will also fail due to missing backup, leaving a corrupt or empty state file and losing prior history.

Recommendation

- Create a backup before overwriting: if [ -f memory/topics/fleet-state.json ]; then cp memory/topics/fleet-state.json memory/topics/fleet-state.json.bak; fi - Prefer atomic update: write to $TMP, validate ($ jq empty "$TMP"), then move into place (mv "$TMP" memory/topics/fleet-state.json). Only update the on-disk file after successful validation. - If validation after move is kept, ensure the backup is created before mv and the error path logs FLEET_STATE_STATE_CORRUPT and restores from the created .bak.

Counterpart finding

State write has no `.bak` creation step but recovery path references `memory/topics/fleet-state.json.bak`

mediumdata-losshigh
  • skills/fleet-state/SKILL.md:270-290
  • skills/fleet-state/SKILL.md:292
  • skills/fleet-state/SKILL.md:39
The recovery branch unconditionally `cp memory/topics/fleet-state.json.bak memory/topics/fleet-state.json` but no step ever creates `.bak`. On the first run, or on any run where `.bak` was deleted, the recovery `cp` fails AND the just-written invalid JSON remains in place. The prose says "Keep one `.bak` rolling" but provides no command to do so — operators / LLMs will follow the code block literally and skip backup creation. Result: a single bad `jq` write irreversibly corrupts fleet-state.json, losing the 12-week history.

Recommendation

Before `mv "$TMP" memory/topics/fleet-state.json`, add `cp memory/topics/fleet-state.json memory/topics/fleet-state.json.bak 2>/dev/null || true`. Also validate `$TMP` with `jq empty` *before* the mv, so the corrupt file is never promoted to canonical in the first place.

Why this didn't post

This finding didn't meet AntFleet's unanimous agreement threshold. Both frontier models review every PR independently; only findings they both flag with the same severity and category are posted to the PR. This one fell through.

read the methodology →

From the same review

These findings passed the unanimous gate on the same PR review. The disagreement above was filtered out; the findings below were posted.