Skip to content

fix(quarantine): say the all-clear only when the alarm's quarantine is empty, and leave out ledger entries the strategy can't use - #266

Merged
nissa-seru merged 1 commit into
anima-research:mainfrom
nissa-seru:fix/quarantine-false-all-clear
Oct 19, 2026
Merged

nissa-seru merged 1 commit into
anima-research:mainfrom
nissa-seru:fix/quarantine-false-all-clear

Conversation

@nissa-seru

Copy link
Copy Markdown
Contributor

TL;DR: the compression-quarantine alarm's all-clear, "ALL CLEAR — quarantine empty, debt paid", which hosts route to their operators, could be said while the quarantine its alarm had last sounded on still held debt: when the status read failed, when the strategy had moved to another branch whose quarantine was empty, and, on main since #138, when the ledger held an entry with no record. A failed read now says so (unreadable: true on the status) and leaves the alarm as it was. An alarm episode now follows the branch its alarm last sounded on, and only that quarantine reading empty ends it. A ledger entry whose record or generation the strategy can't use is left out of the quarantine and named, so a record-less one no longer breaks L1 compression or the clears; its chunk, if no other entry holds it, is tried again, as after a clear. A ledger, legacy snapshot or checkpoint that holds no list is read as holding nothing, and named.

Follows #138, whose spans made the status's span loop read every entry's record.

Problem

On main at 6a4d01c8, each of these was run with the alarm already sounding on a real refused chunk whose record still stood:

  • Ledger entries with no record. Two claims with no record, each with the exhaustion that targets it, made the status's span loop (new in fix(compression): progress past quarantined chunks; transient failures never spend the ladder; pace the lane #138) read active.record.chunkSourceHash of undefined. The catch returned { count: 0, keys: [], spans: [] } while the projection held three entries, the paid-off sweep warned with the same TypeError, and the alarm printed the all-clear and called the handler with that empty status.
  • A status read that fails. The same empty status, the same all-clear and handler call. When the read worked again, the status showed the record.
  • A move to a branch with no quarantine. After switchBranch to a branch made at the first message, the episode opened on the old branch ended there with the all-clear and the handler call. Back on the old branch, the alarm sounded again.

The record-less entry also made compressing another closed chunk throw the same TypeError, at the quarantine check compressChunkHierarchical makes before it requests an L1 summary. It made a clear of every record throw (reading 'key'), and a clear of its own key throw too (reading 'familyKey', a read #138 added).

How such an entry can arise: no version writes one. The only claims were written by the hardening commit 2ce952df/4c2eab7f, always with their record, and fe844a1d/96fea45f removed that writer the same day, so no release wrote claims. Every exhaustion since carries its own record. It takes a ledger edited by hand or damaged. The other two cases need no edited ledger: a read the store fails, and a host's switchBranch or fork on a resident whose alarm is sounding.

Changes

  • The projection makes an entry only from a record and a generation it can use. projectCompressionQuarantineEvents makes an entry from a checkpoint item, or from an exhaustion with its own record or its claim's, only when that record is an object with a string key and chunkSourceHash (isUsableQuarantineRecord), which name the entry and its span, and its generation is a non-empty string (isUsableGenerationId), which clears and alerts compare by ===. For an exhaustion, the record's key must also be the exhaustion's own, which clears, alerts and checkpoints go by. It reads an outcome list as the objects in it that carry a string outcome. It already skipped a legacy record or a checkpoint item with no key; a claim's record was copied unchecked. A legacy snapshot record without a chunk hash is skipped silently, as one without a key was: the code that wrote that snapshot (23e2b2ba) always wrote the hash, and the ledger replaced it the same day.
  • What leaving an entry out does. It's no longer in the quarantine, so it no longer holds its chunk back. A chunk no other entry holds is tried again: its next compression runs the refusal ladder, which asks the provider for what its family's recorded progress doesn't already answer. A refusal records it afresh, with a usable record the alarm sounds on. That's what a clear does, and the all-clear in between means the same as after a clear: nothing usable is left in the quarantine.
  • An entry left out is named by its key, once per load. [compression-quarantine] on branch 'main', left out ledger entries it can't use (keys=…): they no longer hold their chunks back, no clear reaches them, and they stay in the ledger until a checkpoint rewrites the ledger, which needs 256 events in it ns=default, as a warning, the first time each load reads past each key. The names start over at each initialize, which every open, switchBranch, fork and setStrategy runs. A key that a damaged event names but that stays in the quarantine through another event isn't named, and a checkpoint item whose record has no key, which main already skipped, has none to name. In storage, a left-out entry stays in the ledger, read past, since a clear takes only what's in the quarantine. A checkpoint, which runs after a clear, an alert or an exhaustion once the ledger holds 256 events, rewrites the ledger from the quarantine and leaves it out.
  • A status whose read failed says so. getCompressionQuarantineStatus() returns { count: 0, keys: [], spans: [], unreadable: true } when its read throws. The status still never throws.
  • A stored value or a checkpoint that holds no list holds no entry. A ledger or legacy snapshot whose value isn't a list holds no entry the strategy can use, so every reader, the status included, reads it as holding nothing, as main reads a ledger that isn't a list. The legacy snapshot isn't read beside a ledger like that, which has lost the clears it held. A checkpoint, known by its kind whatever its key, replaces what came before it with its list, so one that holds no list replaces it with nothing, and the events after it apply. A stored null reads as no value at all. [compression-quarantine] on branch 'main', the ledger's checkpoint at index 12 holds no list, so the quarantine reads no entry from it and has lost whatever it held ns=default names each such value or checkpoint, as a warning, once per load.
  • The alarm leaves a failed read alone. It logs ⚠️ quarantine status unreadable; the alarm stays up until a read succeeds: (or stays down, with no episode open) with the read's error, at most once an interval on a clock of its own, and neither ends the episode, moves its repeat, nor calls the handler. The paid-off sweep, which reads the same projection first, now returns quietly when that read fails, so an open episode doesn't also warn at every tick.
  • An alarm episode follows the quarantine its alarm last sounded on. The alarm notes the namespace and branch each alarm line sounds on, so an episode follows the branch its alarm last sounded on. While the strategy reads another branch, or another namespace after setStrategy, whose quarantine is empty, the episode stays up and says so in place of the alarm's repeat, on the same clock: ⚠️ the alarm for branch 'main' stays up while the strategy reads branch 'early', whose quarantine is empty; it ends with the all-clear when the strategy reads branch 'main' empty, with no handler call, naming both namespaces when they differ. When its own quarantine reads empty, it ends with the all-clear and the handler call as before. The store isn't compared, since reopening the same store gives another object.
  • The lines name their namespace. Each line this change adds goes through the strategy's diagnostics (this.diag), so its message ends with ns=<namespace>, before any error it carries, as the strategy's other text lines have since fix(diagnostics): every stderr line about one namespace names it (#239) #245.
  • Docs: CompressionQuarantineStatus.unreadable, when setQuarantineAlarmHandler's handler is and isn't called, and the recall-curve fallback spec's list of a family's ways out of the quarantine, which gains the unusable ledger entry.

What each part is for

The alarm sounds while refused spans stay raw, and says the all-clear once, through the host's handler, when the quarantine its alarm last sounded on is empty. Each part below serves saying it only then.

  • One check where entries are made. Buys: none of the eight functions that read an entry's record (the status, the operator's clear, the clear when a chunk is compressed, the checkpoint, the paid-off sweep, the alert delivery, the compression check and quarantinedChunkSources) meets a record it can't use, with no guard of its own. The key and generation checks keep only entries the clears and checkpoints can find, so the alarm never sounds over one no clear could reach. Costs: such an entry no longer counts, where main counted it in count before fix(compression): progress past quarantined chunks; transient failures never spend the ladder; pace the lane #138, and it releases its chunk, which may cost a fresh refusal ladder. Simpler options: a guard in the status alone, which leaves the compression check and the clears throwing on a record-less entry and the alarm sounding over an entry nobody could clear; a guard at each reader, eight places to keep in step. Cut: a check on a claim's record as well, since the exhaustion that targets the claim checks it.
  • Outcome lists read as the outcomes in them. Buys: an element that isn't an object no longer makes the status fail to read, and one with no string outcome isn't counted as an answer. Costs: one helper, used for exhaustions and checkpoint items alike.
  • A line naming each entry left out, once per load. The projection's other skips say nothing, and this one changes what's counted, so an operator can find what was left out and what becomes of it. Costs: a set of the keys already named, cleared at each load, and a pass that drops keys still in the quarantine. Simpler option: naming each key once per strategy instance, so a host whose instance moves between branches hears a key only on the first branch that reads it.
  • A line naming a stored value that holds no list, once per load. Buys: an operator learns that the quarantine lost what that value held, which reading it as holding nothing would otherwise hide. Costs: a set of the names already said, cleared at each load. A checkpoint is named by its index in the ledger, which every event has and no two share, where its event id can be missing or shared, and converting a stored value to text can throw. Simpler options: the status saying unreadable for it, which keeps the alarm from sounding for as long as the value stays, and nothing rewrites the legacy snapshot; throwing from the read, which would stop initialization, since it reads the quarantine with no catch; reading past a checkpoint that holds no list, which an earlier round of this change did, and which brings back the legacy snapshot's records whose clears the checkpoint writer's redaction removed.
  • unreadable on a status whose read failed. Buys: a host can tell "couldn't read" from "empty", and the alarm reads it. Costs: one optional field on the public status. Simpler options: the alarm catching the read itself, which leaves hosts reading an empty quarantine; an error field, which no host needs, since the alarm logs the error.
  • The alarm leaves a failed read alone. Costs: one branch, a clock for its line, and the status read split so the alarm sees the error. The sweep's quiet return on the same failure keeps an open episode from warning at every tick.
  • An episode that follows its alarm's quarantine. Costs: one field, and a helper that names both namespaces in the stays-up line when they differ, since two branches of one name otherwise read alike. Simpler options: ending the episode on a move with a line, which this change first did, and which says no all-clear when the strategy returns to the old branch and finds it paid, so the host's alert stays up for good; ending it on every reinitialization, which ends it silently even on the same branch.
  • A checkpoint known by its kind. Buys: a checkpoint whose key is damaged still replaces what came before it, as one that holds no list does, so neither leaves the legacy snapshot's cleared records standing once the writer's redaction has removed their clears. The legacy snapshot is left unread beside a ledger that holds no list for the same reason. Costs: the checkpoint branch moves above the check of an event's key, which a checkpoint's reading never used, and one condition in the reader.
  • The lines through this.diag. Buys: each line names its namespace, as the strategy's other text lines have since fix(diagnostics): every stderr line about one namespace names it (#239) #245, so lines from several namespaces in one process can be told apart. Costs: nothing beyond the call. The unreadable line's message holds no value, so the error printed after it can't make it a format.
  • The test file. It drives real refusals through compressChunkHierarchical under a refusing membrane, as the quarantine's other tests do, and moves Date.now past the alarm's interval rather than waiting. Its interval is an hour, so no real pause inside a test reaches it. Each test pins one rule above, or is a control that the all-clear still comes. Costs beyond the tests: a count of the membrane's calls, which shows a released chunk asked for again, a fixture whose ledger and append read the stored ledger and write damage after the real refusal, as setStateJson stores any JSON, and a usable helper that writes a usable exhaustion for the no-list test's phases.
  • The changelog fragment, in changelog.d/, as the repository asks.

Tests

All runs on Linux x86_64 with Node 24.22.0, from a fresh npm ci. CI runs the full suite on this pull request. At this head, 0f72e34d, typecheck and build are clean, and npm test passes 2,216 of 2,216.

  • On main at 6a4d01c8, each case above was run with a probe of the strategy (compressChunkHierarchical refusing a chunk under a refusing membrane, then the alarm): the record-less entries, the failed read, the branch move, a compression of another chunk, and both clears.
  • The new tests, in test/compression-quarantine-all-clear.test.ts, at this head:
    • entries a checkpoint, a claim or an exhaustion carries with a null record, no record, a record with no chunk hash, a key that isn't a string, another entry's key, or a generation that's missing, empty or isn't a string, a null checkpoint item, a null event and an exhaustion whose own key is a number, beside the real refused chunk and a damaged event for its key: the status counts the real record and four usable ones (a claim's, one whose outcomes aren't a list, and an exhaustion and a checkpoint item whose outcome lists mix in what isn't an outcome), in one span, with one answer for each mixed list, and getCompressionDebt() counts the same five; the eleven left-out keys are named in one warning however often the ledger is read, a key damaged later in a line of its own, and all twelve again by the next load's own read; the alarm repeats on what stands with no all-clear; and a second chunk is tried and refused;
    • a legacy snapshot record with no chunk hash is skipped without a line, beside a usable one;
    • when a chunk's only record has lost its chunk hash, the entry is left out, and the debt read counts no quarantine and reads healthy; compressing the chunk calls the summarizer again: the refusal records it afresh, under the same key and hash;
    • a clear by that key and a clear of every record leave a left-out entry in the ledger, and a checkpoint at 256 events rewrites the ledger without it;
    • a failed read gives unreadable: true, one line an interval naming the error, no handler call and no sweep warning, and once a read succeeds the alarm repeats on its own time; an episode outlasts a failed read and ends with one all-clear when the read finds its quarantine empty; with no episode open the line says the alarm stays down; a strategy with no store isn't unreadable;
    • a legacy snapshot that holds no list leaves the ledger's record counted and alarmed; a checkpoint with an empty list clears the entry before it, and one with no key restates its own list; two checkpoints that hold no list, an object and null, clear the entry before them, the entry after them applies, and each is named by its index; a legacy snapshot record a clear took stays cleared under a checkpoint that restates what stood, then under one that holds no list, then under a ledger that holds no list, which reads as empty, the legacy snapshot included, and ends the episode with the all-clear; each is named once within a load however often the store is read, with the branch and the namespace, and again by the next load's own read;
    • a move to a branch whose quarantine is empty keeps the episode up, saying so at the interval and once an interval, with no handler call; back on its own branch within the interval nothing is said, and a clear there with no alarm in between ends the episode with the all-clear;
    • an episode that moves with its alarm to a fork carrying the debt ends with the fork's all-clear;
    • a move to another namespace by setStrategy keeps the episode up the same way, and back in its own namespace a clear ends it with the all-clear;
    • a control: the all-clear and the handler call with count 0 still come once when the debt is paid on the alarm's branch.
  • Red-first: on main's code at 7f963c75, with only the new optional field added to the type so the file compiles, 9 of the 11 fail on what they check: the status is empty with the left-out entries; the legacy and the release tests count a record with no chunk hash; the clear by key throws (reading 'familyKey'); a failed read has no unreadable; the outage test hears the sweep's warning; nothing names a value that holds no list; and the branch and namespace moves say the all-clear. The fork test and the control pass: main's episode follows no branch, and on a fork that carries the debt it goes the same way.
  • Each piece taken out on its own fails a new test, 42 in all: the record check on an exhaustion's record, the key match, the checkpoint item's check, the null check, reading outcomes as a list, keeping only the objects in one, and only those with a string outcome, the generation checks on an exhaustion and on a checkpoint item, and refusing an empty one, the warning, naming each key once, not naming a key still in the quarantine, naming an exhaustion with no generation, unreadable, the alarm's branch for a failed read, saying it once an interval, on its own clock, the sweep's quiet return, the check of the episode's branch and of its namespace, noting the episode's scope, and noting it at every alarm, saying the stays-up line once an interval, naming both namespaces in it, naming a ledger, a legacy snapshot or a checkpoint that holds no list, naming each once, starting both sets of names over at each load, and before initialize reads the quarantine, clearing at a checkpoint, and at one that holds no list, not reading such a checkpoint's value as a list, reading a checkpoint whatever its key, naming each checkpoint by its index, and sending the line that names them through this.diag, the record check's test of the key's type, skipping a null event and an event whose key isn't a string, and leaving the legacy snapshot unread beside a ledger that holds no list.
  • The quarantine's other test files, the 16 that touch it with 583 tests, pass unchanged: 594 of 594 with the new file.

Not verified

  • A resident's store. The tests build a store of 16 messages and refuse its first closed chunk under a refusing membrane. No compile ran against a resident's store.
  • A real store failure. The failed read is a test's projection read that throws, not a store that fails. By reading of chronicle at v0.4.0, a blob whose CRC32 doesn't match fails its read with an error (blobs/storage.rs:149-158), which reaches the status as this case does. A torn log tail is truncated when the store opens (branches/manager.rs:304), so a read after it sees the state before the torn write, with no error.
  • fork() with a failed read or a namespace move. The branch cases were run with switchBranch and fork; the namespace case with setStrategy.
  • A host's handling of unreadable, such as connectome-host's alert row and Health tab. Nothing in a host changes here.
  • One strategy instance on two stores. The episode compares namespace and branch names, not stores, so it would take two stores' quarantines under the same names as one. Not run.
  • A branch paid by another process while the strategy reads another branch. By reading, the episode ends on the first alarm check after the strategy reads that branch again. Not run.

Compatibility

  • Hosts can read one new optional field, unreadable, present only on a status whose read failed, when count is still 0. A host that takes its alert down on count === 0 keeps it up across a failed read only if it also checks unreadable. The alarm's handler stops receiving the all-clears above. While the strategy reads another branch or namespace whose quarantine is empty, the handler hears nothing until it reads the alarm's again. If it never does, as after a rollback to an early branch kept for good, or a deleted branch, the episode stays up, saying so each interval, until a restart ends it with no all-clear.
  • count, keys and getCompressionDebt()'s compressionQuarantineCount no longer include a ledger entry the strategy can't use, and its chunk, if no other entry holds it, is tried again. A ledger holding only such entries no longer makes the debt read degraded by itself.

Out of scope

  • getCompressionDebt(), by reading, still takes a failed quarantine read as 0, and its catch returns healthy, so /healthz can read healthy on a store it can't read.
  • The paid-off sweep, by reading, clears a record whose chunk record is missing even while the chunk's messages stay raw, and it and the clear after a compression carry keys across awaited clears, so a branch switch between them can clear the same key on the newly selected branch.
  • The alarm's idle check, by reading, runs at every tick after an all-clear, or from a start with nothing quarantined, since its clock is then 0.
  • The merge-quarantine alarm, by reading, keeps its episode across a branch move too. Its all-clear, "merge quarantine empty; alarm stands down", is a stderr line with no handler, and says what holds on the branch it reads.
  • The alarm's episode lives in memory, so, by reading, a record cleared before the first alarm check after a restart ends with no all-clear.
  • The legacy snapshot state stays in storage, as before: it's only read, and a checkpoint doesn't rewrite it.
  • Ledger events other than checkpoints with no string key, and checkpoint items whose record has no string key, are still skipped without a line, as before: neither has a key to name.

— Hamish-2699 (Claude Opus 5.5). Oscar-1005 (Claude Opus 5.5) agreed its design, the check where entries are made and the episode that follows its alarm's branch among it. Agnes-1009 found the all-clear on a failed read, Paloma-2644 the one after a branch move, and Kestrel-2409 read the change whole, beside five automated internal reviews.

🤖 Written using Anthropic's Claude Opus 5.5 through the Rookery's tooling.

…s empty, and leave out ledger entries the strategy can't use

The compression-quarantine alarm's all-clear, "ALL CLEAR — quarantine
empty, debt paid", could be said while the quarantine its alarm had last
sounded on still held debt: when the status read failed, when the strategy
had moved to another branch or namespace whose quarantine was empty, and,
since anima-research#138, when the ledger held an entry with no record. Such an entry
also made the quarantine check before each L1 summary request, and both
clears, throw.

- A status whose read fails has `unreadable: true`, with count, keys and
  spans empty. The alarm leaves its episode as it was, saying so once an
  interval on a clock of its own, and the paid-off sweep returns quietly
  when the same read fails.
- An episode follows the namespace and branch its alarm last sounded on.
  While the strategy reads another one whose quarantine is empty, it stays
  up, saying so at the alarm's interval with no handler call. Only its own
  quarantine reading empty ends it with the all-clear.
- The projection makes an entry only from a record with a string key and
  chunk hash and a non-empty string generation, and an exhaustion's record
  must carry the exhaustion's own key. It reads an outcome list as the
  outcomes in it. A ledger entry it leaves out no longer holds its chunk
  back, and a warning names its key, where it has one, once per load.
- A ledger or legacy snapshot whose value isn't a list is read as holding
  nothing, and the legacy snapshot isn't read beside a ledger like that.
  A checkpoint, known by its kind, replaces what came before it with its
  list, so one that holds no list replaces it with nothing. A warning names
  each once per load, a checkpoint by its index in the ledger.
- The change's stderr lines go through this.diag, so each carries
  ` ns=<namespace>`.
- The transient-ladder fragment and the recall-curve fallback spec gain
  the unusable ledger entry among a family's ways out of the quarantine.

test/compression-quarantine-all-clear.test.ts drives real refusals through
compressChunkHierarchical under a refusing membrane.

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

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5 Tier: apex

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

The PR prevents false compression-quarantine all-clears and filters unusable stored entries.

  • The quarantine skips damaged entries and lets their chunks be tried again.
  • A failed quarantine read no longer tells the alarm its debt is paid.
  • Alarm episodes stay tied to the branch and namespace that raised them.

Source and tests were inspected; tests were not run during this review.

Acknowledged by nissa-seru, not raised as findings: malformed lists intentionally count as empty; episodes follow the last alarm’s branch and namespace; store objects are not compared because reopening replaces them. Host health changes and existing sweep races are deferred. Permanently leaving the alarm’s branch can keep its episode open until restart.

Reviews (1) · Last reviewed commit: "fix(quarantine): say the all-clear only ..." · Reviewed by Greptile

@nissa-seru
nissa-seru merged commit 6ba17d6 into anima-research:main Oct 19, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant