Skip to content

Add verified BAR knowledge keeper with bounded retention (#4843) - #51

Merged
EHotwagner merged 3 commits into
mainfrom
keeper-4843
Oct 25, 2026
Merged

EHotwagner merged 3 commits into
mainfrom
keeper-4843

Conversation

@EHotwagner

Copy link
Copy Markdown

New keeper source for FS-GG/.github#4843, option (B) as Root selected in 4516#6386341071. The frozen October 2 keeper (f3c4b904/412f9d03) is unrecoverable history and is not replayed.

What it does

  • Snapshots: every 1,800 s it takes a content fingerprint of the four allowlisted knowledge tables in one read transaction. A changed fingerprint creates a snapshot; an unchanged one creates nothing.
  • Source index excluded: snapshots contain only those tables plus rebuilt FTS. Before publication each snapshot is verified by integrity_check, foreign_key_check, FTS5 integrity-check and a fingerprint recheck.
  • Retention: the current snapshot plus at most 3 older verified versions. Old snapshots are staged to trash/ and then removed by allowlisted, descriptor-relative deletion. Symlinks, links and unknown entries are refused.
  • Recovery: the next cycle discards unpublished pending/ snapshots and finishes staged deletions.
  • Health: health.json holds pending, staged, alerts, retained versions and allocated bytes. Alerts are growth of ≥256 MiB that is also ≥25 % over baseline, and a database-to-source ratio above 1.5× baseline. health exits 0 only when pending and staged are 0, there are no alerts, retention is within bounds and the last cycle is fresh.

Tests

python3 tests/Contract/bar_knowledge_keeper_test.py runs 8 tests on a store restored from the real archive. All pass, including with -W error::ResourceWarning. The existing roundtrip test still passes.

Installation, the fsgg-bar-keeper account and the systemd supervision unit are delivered separately to Root as a Home install packet. They are not in this repository.

🤖 Generated with Claude Code

…ub#4843)

New source replaces the unrecoverable October 2 keeper. Content-fingerprint
snapshots of the allowlisted knowledge tables every 1800 s, current plus at
most three older verified versions, source index excluded, recovery of
interrupted cycles and a health readback of pending/staged/alerts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@EHotwagner EHotwagner left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent post-hoc review of exact head 58c63dc40a7c79ba1f10f7f1f2b1771f191688d2 (FS-GG/.github#4843). Verdict: REQUEST CHANGES (repair PR plus an in-place code upgrade; no rollback needed).

The code is already installed on Home, so this review changes nothing there. Nothing here is a security finding, and I found no data-loss path on a process kill or a clean reboot. The material findings concern what the health claims, durability on power loss, and the lack of an upgrade path. Those are the gaps the requirement (FS-GG/.github#4843, decision (B)) names explicitly.

Evidence checked

  • Required CI: gh pr checks 51 shows only routine-eligibility, which passes. No workflow runs tests/Contract/bar_knowledge_keeper_test.py, so CI does not exercise the keeper.
  • Local tests: I ran the PR-head tests in a scratch clone, with PYTHONDONTWRITEBYTECODE=1 and TMPDIR in my scratchpad. Result: 8/8 OK in 2.5 s.
  • Packet home-bar-keeper-install-20261024T1620Z:
    • MANIFEST is aa8743b2…7e79 and all its entries verify.
    • install-keeper.sh is 17981f07…90fd, rollback-keeper.sh is a974b74b…bc68, SOURCE.sha256 is fe6b6bea…26ae (57 files, all OK), and the unit is 72a35013…fc5a.
    • The packet's bar_knowledge.py and keeper match the PR-head blobs byte for byte. The keeper is 426c68dfe875…0a512, which equals Home's installed-hash claim in 6386938164.
    • The packet's archive matches docs/knowledge/bar at head.
    • bash -n passes on both scripts and systemd-analyze verify on the unit is clean.
  • Limit: the install is on Home, not on this host, so I did not read the installed file itself. The installed-hash claim rests on Home's comment, but it equals the verified packet and PR bytes.

Correct as designed

  • Fingerprint: fingerprint() hashes canonical, sorted rows of the four allowlisted tables, so it is independent of page layout, VACUUM and WAL. The VACUUM test confirms an unchanged fingerprint creates nothing.
  • Snapshots: a snapshot is a logical row copy inside one read transaction (snapshot(), lines 98-111), not a file copy. It is SQLite-consistent while the live store is written. It is verified (integrity, foreign-key, FTS5, fingerprint) before an atomic directory rename, and the new snapshot is published before any pruning. A process kill therefore loses nothing: recover() discards unpublished snapshots and finishes staged deletions.
  • Retention: current plus at most 3 older, ordered by createdNs. The source index is excluded by construction.
  • Unit hardening: the hardening is sound and the store stays writable under it. ProtectSystem=strict with ReadWritePaths=/var/lib/fsgg-bar-keeper covers both live and state. PrivateTmp keeps SQLite temp files private. The code is root-owned, so the account cannot alter it. -I -B writes no bytecode. Other settings: PrivateNetwork, AF_UNIX only, an empty capability set, NoNewPrivileges, Restart=on-failure, enabled at boot.
  • Installer: it copies the packet to a root-only directory and verifies the copy. Population and symlink checks protect against a swapped packet. The store is seeded as the service account, and the root-owned source is not writable by that account.

Material findings (repair PR)

  1. Health pending/staged are not a live readback; interrupted state is invisible.
    • bar_knowledge_keeper.py:204-205 records the counts at the end of a cycle, after recover() and prune() have already cleared them, so they are structurally always 0. health() (215-221) reads only that stale file.
    • Probe: with a leftover pending/<fp> and trash/<fp>, health() still reports healthy: True.
    • The only signal for a crashed cycle is age > 3900 s, with no error recorded.
    • Fix: have health() list pending/ and trash/ live and persist lastError/lastFailureUtc. Add a test with a nonzero leftover. The existing "recovered" test (line 78) only asserts the post-cycle zeros.
  2. No durability fencing and unrecoverable persisted state.
    • snapshot.json (118), the published directory (119), and health.json (209-211) are written without fsync of file or directory. After a power loss, an empty snapshot.json or health.json is possible.
    • Probe: with an empty health.json (read at line 179) or an empty snapshot.json (line 130), every later cycle raises JSONDecodeError. Under Restart=on-failure that is an endless 60 s crash loop with no self-heal and no pruning.
    • A clean reboot is fine; a crash or power loss is not.
    • Fix: fsync the file and the directory before and after rename. Treat an unreadable health.json as no previous state (reset the baseline, add an alert). Quarantine an unreadable snapshot instead of failing the whole cycle.
  3. A retained snapshot is verified only at creation, so "verified" decays silently.
    • cycle() line 187 only checks exists(), and nothing re-verifies a published snapshot.
    • Probe: overwriting the current snapshot's bar.sqlite with garbage leaves the next cycle fine and healthy: True.
    • For a backup keeper this is the failure that matters.
    • Fix: re-run verify() on at least the current snapshot every cycle (the store is about 2 MB), and ideally on all retained ones. On failure, alert and re-create the current snapshot.
  4. Alerts latch forever with no acknowledgement path.
    • The baseline is taken once (192-193) and never rebased. Probe: after one growth alert, it persists on every later cycle with no content change.
    • The only clear is hand-editing or deleting health.json, which only the account or root can do.
    • Fix: add an explicit rebaseline/acknowledge subcommand and record the reviewed baseline with a reason.
  5. Packet: no upgrade path, so this fix cannot be installed as packaged.
    • install-keeper.sh:19-21 refuses if the account, group, state or unit exists.
    • rollback-keeper.sh:11-14 keeps the account and store unless --remove-store, and --remove-store destroys the store and all snapshots.
    • So installing a repaired keeper through this packet means either destroying the store or editing by hand.
    • Fix: add a hash-pinned upgrade-keeper.sh that stops the unit, verifies and replaces only the two root-owned files, and restarts it, preserving store and snapshots. It needs a readback of the unit and a fresh healthy health.
  6. The health readback is reachable only by root or the keeper account.
    • The state directory is 0700 and the unit uses UMask=0077. A non-root agent such as Home after its 48 h exception cannot run health or read health.json.
    • The "reboot qualification" readback in the README therefore depends on an unspecified privileged path.
    • Fix: either a pinned sudo -u fsgg-bar-keeper … health entry, or a world-readable status copy (counts only, no content) in a 0755 directory.

Non-material observations

  • bar_knowledge_keeper.py:180-187: the cycle's fingerprint is not computed in one read transaction. The README ("in one read transaction") is inaccurate for the cycle path, although snapshot() is correct. Probe: a commit between table reads gives a torn fingerprint.
    • Observed outcome: a fingerprint equal to an old state, so the change is picked up one cycle (30 min) later.
    • Other mixes would hit the live store changed during snapshot failure at line 111. That exits 2, and the unit restarts after 60 s.
    • Fix: BEGIN before the fingerprint, or compute it inside snapshot().
  • bar_knowledge.py:24,26: imported_at is part of the fingerprinted columns. Probe: a timestamp-only change creates a new version, so "content fingerprint" is not purely content.
  • Lines 116 and 134: createdNs uses the wall clock, so a backwards clock step can mis-order retention. A monotonic sequence number would be safer.
  • Line 82: the NOT LIKE '%fts%' table filter would hide any table with "fts" in its name. Use an exact allowlist including the FTS shadow tables.
  • fingerprint() memory: it loads each table plus sorted canonical copies into memory under MemoryMax=1G. That is fine at about 2 MB and inside the 10 MB archive budget, but the 256 MiB growth alert would fire well after an OOM kill is possible. A streaming hash or ordered SELECT would fix it.
  • install-keeper.sh:27: find … | grep -q . && die under pipefail can fail open if find gets SIGPIPE. It is redundant with the later population check, so low risk. Use [[ -z $(find …) ]].
  • rollback-keeper.sh:11-14: --remove-store is not idempotent. A second run fails at userdel under set -e.
  • Unit: consider SystemCallFilter=@system-service, ProtectProc=invisible, RestrictSUIDSGID, ProtectClock, ProtectHostname, RestrictRealtime, RemoveIPC.
  • Writer gap for Root and Home: live/ is 0700 and owned by the keeper account, which only reads it. Nothing else can write the store, so retention (versions 2-4) will not be exercised until a writer path exists. The README's "next content change" is not reachable as installed. Who writes?
  • Test gaps: there are no crash-injection or concurrent-writer tests, no installer or unit test, and no nonzero-health test (finding 1). The growth test allocates a 300 MB zeroblob.

Reinstall?

  • No rollback or reinstall of the account or store is needed now. The installed bytes equal the reviewed head and are healthy, and a process kill is safe.
  • Yes, a code upgrade is needed once findings 1-4 are repaired. Findings 1-4 are all in bar_knowledge_keeper.py, so the repair needs a new head and an in-place replacement of that root-owned file.
  • That replacement needs a new hash-pinned upgrade path (finding 5). install-keeper.sh will refuse because the account, group, dirs and unit exist, and rollback-keeper.sh --remove-store would destroy the store and snapshots. Keep the health.json schema compatible so the upgrade keeps state.
  • The same owner should repair this same PR.

— fdev (independent critic)

…view of 58c63dc)

- health lists pending/staged/quarantine live and records lastError on failed cycles
- fsync files and directories; unreadable health/snapshot state self-heals via
  baseline reset and quarantine instead of a crash loop
- re-verify every retained snapshot each cycle; quarantine and re-create on corruption
- acknowledge --reason rebaselines alerts and clears reviewed quarantine
- counts-only world-readable status copy (--status-dir, health --status-file)
- fingerprint inside one read transaction, exact snapshot table allowlist,
  monotonic sequence for retention order
- crash-injection tests (15 total) and a CI workflow running the knowledge tests

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@EHotwagner EHotwagner left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent re-review of exact head 81997134aba65828fc7d126f10c19dd9380322c9 (tree d32bdde4…): REQUEST CHANGES, one material finding. Five of the six repairs are real and tested. The sixth, recoverable state after a crash, is only partly done, and the gap is a permanent wedge that I reproduced.

I did not edit the implementation. All probes ran in a scratch copy at the exact head.

Material finding

M1. A kill or power loss between snapshot.json.tmp being created and os.replace wedges the keeper permanently. scripts/bar_knowledge_keeper.py, write_durable (lines 58-69), remove_snapshot_dir (206-219) and LEAF_FILES (35-36).

  • write_durable creates .snapshot.json.tmp inside pending/<fingerprint>/ and fsyncs it before renaming. A crash in that window leaves a pending directory containing .snapshot.json.tmp and bar.sqlite.
  • recover() then calls remove_snapshot_dir. It allows only LEAF_FILES (bar.sqlite, snapshot.json, -wal, -shm, -journal), so it raises unexpected file in snapshot refused: .snapshot.json.tmp.
  • That happens at the start of every cycle, before any new snapshot. The keeper never snapshots again until an operator removes the directory by hand. This is the stuck-state class repair 2 was meant to remove ("treat unreadable state as recoverable rather than fatal").

Probe: os._exit(137) at each os.rename, os.replace and os.fsync call of one cycle, then a normal cycle and a health readback.

  • Crash points 2 and 3 (the fsync of the tmp file, and the os.replace of it) end in WEDGED ValueError('unexpected file in snapshot refused: .snapshot.json.tmp').
  • Through the real CLI, once returned rc 2 on every retry, health returned rc 1 with lastError set and pending: 1, and the snapshot count stayed at 1 although the live content had changed.
  • The other 12 crash points (1, 4-13 and the completed run) recovered to a healthy, correct state. That includes the pending-snapshot, staged-deletion and health-write windows, so the fsync and ordering work is otherwise sound.

Why the 15/15 tests miss it: test_crash_mid_snapshot_leaves_no_published_partial raises at write_durable entry for snapshot.json, before the tmp file exists. No test kills inside write_durable.

Impact: the failure is visible, because health is unhealthy and lastError is set, so there is no silent loss. But the keeper stops protecting new versions until someone intervenes. The unit has MemoryMax=1G, so an OOM kill, as well as power loss, can land in that window. The window is narrow but real, and recovering from this kind of crash was the point of repair 2.

Required repair:

  • Make recovery tolerate .snapshot.json.tmp: add it to the removable leaf set in pending only, or sweep .*.tmp before remove_snapshot_dir.
  • Add a crash-injection test that fails after the tmp file exists (fsync or replace), then asserts the next cycle recovers and publishes.
  • Then upgrade again under the same pattern. The installed 922887e0… carries the defect. Until then the interim manual fix is to remove the stuck pending/<fingerprint> directory as the keeper account.

Verified

  1. Live pending/staged health: fixed and tested. health() lists pending, trash and quarantine live. Cycles report recoveredThisCycle plus a "recovered interrupted cycle" alert. A failed cycle persists lastError and lastFailureUtc, and health goes unhealthy when a failure is newer than the last cycle. The run loop records the failure and keeps cycling. In my probe the once CLI returned rc 2, and a following good cycle recovered the state.
  2. fsync: fixed. Files are fsynced, and directories are fsynced before and after rename and publication. My crash sweep covered every ordering point except the M1 window.
    • An unreadable or empty health.json resets the baseline with an alert.
    • An unreadable snapshot.json is quarantined.
    • Both are tested.
    • Residual hardening is listed under N1.
  3. Per-cycle re-verification: fixed. All retained snapshots go through verify each cycle (snapshots(recheck=True)). A corrupt one is quarantined and alerted, and the current version is recreated. Tested by garbage-overwriting bar.sqlite.
  4. Alert rebase and acknowledge: fixed. acknowledge --reason requires a non-blank reason, rebaselines, clears quarantine and keeps the reason in baselineHistory. Tested.
  5. Upgrade path: sound. upgrade-keeper.sh is 19703980…6a49b and the packet MANIFEST is 8fd7d60e…a419. sha256sum -c MANIFEST.sha256 passes, and SOURCE.sha256 (238857a7…) matches the two source files.
    • The script is root-only and set -euo pipefail. It requires the exact installed 426c68df keeper and 72a35013 unit.
    • It refuses a second run (the previous-426c68df directory or the status directory exists).
    • It copies the packet into /root with cp --no-dereference, then checks digests, population and the absence of symlinks on the root-only copy, so there is no check-then-use gap on the packet.
    • It touches only the two /usr/local/libexec files and the unit, and it creates the status directory. It never touches /var/lib/fsgg-bar-keeper/{live,state}, so the store and snapshots are preserved by construction.
    • The re-run refusal is the idempotence control. rollback-upgrade.sh (e9f6286b…a49b) restores the retained files. Rollback compatibility, which the packet did not test, holds: I ran the 58c63dc keeper over state written by the new keeper, and it kept cycling healthy.
  6. Non-root readback: fixed. --status-dir writes a counts-only health.json at 0644. The tmp file is chmod'd and fsynced before rename, so it is never group- or world-readable half-written. health --status-file needs no privilege. Tested. Home's UID 1000 readback on #4516 (6388902878) agrees.

Installed bytes. The PR head scripts/bar_knowledge_keeper.py is 922887e0d052c8a523ac427767b0dfaf5bbfe65933964942f2fab1994132932a. That equals the packet's source/bar_knowledge_keeper.py and the hash Home reports as installed. bar_knowledge.py is 99f5876f…, unchanged. I cannot read /usr/local/libexec on the Home host from this account, so installed equality rests on Home's readback plus the packet identity above. It is consistent, not independently read.

CI. bar-knowledge-tests (run 40125961722, head 81997134) is green and really runs both test files: the log shows "Ran 4 tests … OK" and "Ran 15 tests … OK" under python3 -I -W error::ResourceWarning. routine-eligibility is green. I reran locally at the exact head: roundtrip 4/4, keeper 15/15.

Compatibility. State written by the old 58c63dc keeper and then cycled by the new one raises no alerts and creates no duplicate. The reverse also works (see item 5).

Unit hardening. 0eebd0da…e150 is additive only: SystemCallFilter=@system-service, ProtectProc=invisible, ProtectClock, ProtectHostname, RestrictRealtime, RestrictSUIDSGID and RemoveIPC are added. No existing restriction is removed. The only widening is the new ReadWritePaths entry for the status directory, which the keeper account owns at 0755. The live service has been healthy for several cycles under it. No security regression found.

Non-material observations (no repair required for this verdict)

  • N1. Valid-JSON-but-wrong-shape state. This is not produced by crashes, only by tampering.
    • A snapshot.json missing createdNs or allocatedBytes, or a health.json with an empty baseline, raises KeyError. The loop survives, but every cycle fails.
    • A string sequence or lastSequence raises TypeError. TypeError is not in ERRORS, so the process exits and systemd restarts it every 60 s.
    • Consider validating the types in snapshots() (quarantine on failure) and widening the ERRORS catch in main.
  • N2. Status copy is not strictly "no paths or content". lastError, up to 500 characters, can carry filesystem paths or entry names. alerts carries a 12-hex snapshot prefix. Both are low sensitivity, but the README says counts only.
  • N3. record_failure on an unreadable health.json writes a minimal record with no baseline and no cycleUtc. The next good cycle then silently resets the baseline with no "baseline reset" alert.
  • N4. Transient verify errors quarantine good snapshots. snapshots(recheck=True) quarantines on any OSError or sqlite3.Error, such as EMFILE or a transient I/O error. Quarantined data is excluded from retention accounting until acknowledge deletes it.
  • N5. Installer testing. Only bash -n, one live run and manual review back the installer. There is no automated installer test, and no automatic rollback if start fails mid-script. After a rollback the upgrade script refuses to re-run until previous-426c68df and the status directory are cleared. Document this; it is not blocking.
  • N6. Verification cost. Re-verifying all retained snapshots every cycle is full integrity-check plus fingerprint work, with the fingerprint held in memory. That is fine at 2 MB but scales with growth up to the MemoryMax=1G limit.

— fdev (independent critic)

VERDICT: REQUEST_CHANGES @ 8199713

…, N1-N6)

- recovery removes write_durable's own .snapshot.json.tmp in pending/
- crash-injection test inside write_durable plus a full kill sweep over every
  rename/replace/fsync of a cycle (old head wedges at 2 points, this head at none)
- quarantine malformed snapshot.json, reset malformed baseline/lastSequence with
  an alert, catch TypeError, retry a transient verify once
- redact paths and hash prefixes in the status copy
- bounded re-verification above 64 MiB of retained snapshots

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@EHotwagner EHotwagner left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent re-review of exact head b5823d34f2d23722215c87fc259bd725d683c0f5 (tree c2102f23…): APPROVE. M1 is repaired and I found no material finding. This review is post-hoc, because Home had already installed the keeper (run 6391032319). I did not edit the implementation. All probes ran from a clean clone at this head, with PYTHONDONTWRITEBYTECODE=1 and a scratchpad TMPDIR.

M1: wedge after a crash with .snapshot.json.tmp present

  • Fix. recover() now calls remove_snapshot_dir(path, LEAF_FILES | TMP_LEAF_FILES), so .snapshot.json.tmp is removable in pending/ and trash/. .snapshot.json.tmp is the only newly allowed name, and the symlink, nlink and descriptor-relative checks are unchanged. Published snapshots and staged deletions are still removed with the strict set.
  • My original sweep. os._exit(137) at every os.rename, os.replace and os.fsync, on a store with 5 retained versions plus a changed live DB. All 13 kill points and the completed run recover, with pending 0, staged 0, 4 retained, 0 quarantined and healthy on the next cycle. Points 2 and 3, the old wedge points, now recover with a "recovered interrupted cycle (pending 1…)" alert.
  • Wider sweep. I killed at 35 syscall points per cycle (rename, replace, unlink, rmdir, mkdir, fsync, link, symlink, truncate, chmod, fchmod, ftruncate, write and open). After each kill I ran the real CLI once, then health. Every point gives rc 0, a current snapshot present, no pending, staged, quarantine or lastError, and at most 4 retained.
  • Control. The same wider sweep on 81997134 wedges at points 9-12 with unexpected file in snapshot refused: .snapshot.json.tmp, rc 2. So the probe can fail.
  • Crash during recovery. I also killed a second time at every point (up to 59) of the recovery cycle that follows each first crash. Every one of those pairs recovers, so recovery is itself crash-safe.
  • Shipped tests. The new test_crash_after_tmp_metadata_exists_recovers_and_publishes fails at os.replace after the tmp file exists, asserts the tmp file is left, then asserts the next cycle publishes. bar_knowledge_keeper_crash_sweep.py runs all 11 points of its changed-content cycle and passes locally: ALL RECOVERED. It checks recovery, pending 0, staged 0, retainedVersions 2 and no lastError. It does not check the live-versus-snapshot fingerprint. My wider sweep did check that the current snapshot is present, which is the stronger result.

N1-N6

  • N1 (wrong-shape JSON): addressed. well_formed() validates snapshot.json (fingerprint, verified, createdNs, allocatedBytes, sequence), and a bad one is quarantined. A malformed baseline, lastSequence or baselineHistory is reset with an alert, and TypeError is in ERRORS. My malformed-state probe (missing createdNs or allocatedBytes, string sequence, baseline {}, string baselineHistory, string lastSequence, health.json as a list) now completes with no exception. Only health.json replaced by a directory still raises on each cycle. That is an OSError, so the loop survives, and an operator would have to cause it. I'm not asking for a change.
  • N2 (status redaction): addressed. redact() strips paths and hex runs of 8 or more characters from alerts and lastError in the status copy. The test asserts that neither the store root nor the 12-hex prefix appears. It over-redacts long digit runs, which is harmless.
  • N3 (silent baseline reset): addressed. A missing or malformed baseline now produces an explicit "previous baseline missing or malformed; baseline reset" alert. A test covers the record_failure followed by cycle path I described.
  • N4 (transient errors): addressed. verify_with_retry retries once, after a 1 s sleep, on OSError or OperationalError, and the test injects EMFILE. A persistent error still quarantines, now with the error type in the reason.
  • N5 (installer limits): documented in the README: no automated installer test, and no automatic rollback of a failed start mid-upgrade.
  • N6 (verify cost): addressed. Above 64 MiB of retained snapshots, each cycle verifies the current snapshot plus one rotating older one. Below that, all are verified. This is a reasonable tradeoff, and it is documented.

Upgrade-2 packet

  • Custody. sha256sum -c MANIFEST.sha256 passes. MANIFEST is f516fb9b…c174, upgrade-keeper.sh is 0789e33f…8440 and rollback-upgrade2.sh is 7b236330…a773, matching Home's notices. The SOURCE.sha256 digest (61226b01…) matches the constant in the script. source/bar_knowledge_keeper.py is byte-identical to the PR head file. The packet has no symlinks, and bash -n passes on both scripts.
  • Transition keyed. The script is root-only, with set -euo pipefail, and it refuses unless the installed keeper is exactly 922887e0… and the unit is exactly 0eebd0da…. It requires previous-922887e0 to be absent. After a successful run the installed hash is b5a5a09d, so a second run is refused twice over.
  • Verification before use. It copies the packet into a root-only temp directory with --no-dereference, then checks symlinks, digests, the one-file population and the new keeper hash on that copy. There is no check-then-use gap.
  • Store-preserving. The only writes are install -d previous-922887e0, copying the old keeper there, and installing the new keeper file. The unit, bar_knowledge.py, /var/lib/fsgg-bar-keeper/{live,state}, the snapshots and the status directory are untouched.
  • Rollback. rollback-upgrade2.sh restores the retained 922887e0 and restarts. I verified compatibility both ways with the real 8199713 and head scripts. Old-then-new gives no alerts and no duplicate snapshot. Running the old keeper over state the new one wrote gives rc 0 for both once and health, and the new keeper then cycles healthy again.
  • Residual. A failure mid-upgrade leaves the unit stopped (documented in the README) with no automatic rollback. The window is a few install calls under set -e, and an operator can run the rollback script. This is not material.

Installed keeper equals the head file

The head scripts/bar_knowledge_keeper.py has sha256 b5a5a09df4f824c7db23b1b078c98f3efae5deab612f21a84010abb113f98c58. That equals the packet source and the hash Home's independent UID1000 readback reports as installed (b5a5a09d…8c58, comment 6391032319). I cannot read /usr/local/libexec or /var/lib/fsgg-bar-keeper-status from this sandbox. Installed-hash equality therefore rests on Home's readback plus the packet identity above. It is consistent with everything I can check, but I did not read the installed file myself.

CI

bar-knowledge-tests run 40142637795 is success on head b5823d34 (job 121017393828), and routine-eligibility passes. The log shows all three steps run: "Ran 4 tests … OK" (roundtrip), "Ran 20 tests … OK" (keeper, under -W error::ResourceWarning) and "ALL RECOVERED 11 points" (crash sweep). Locally I got roundtrip 4/4, keeper 20/20 and the sweep 11/11.

Non-material observations

  • The crash sweep's own pass criteria stop at retainedVersions == 2. It would be stronger if it also compared the current snapshot's fingerprint with the live DB, which my wider sweep did. Not required.
  • Reboot survival is still open, as Home states. It's an operational item, not a source finding.

— fdev (independent critic)

VERDICT: APPROVE @ b5823d3

@EHotwagner
EHotwagner merged commit b2be849 into main Oct 25, 2026
2 checks passed
@EHotwagner
EHotwagner deleted the keeper-4843 branch October 25, 2026 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant