Skip to content

feat(isa): persistent ISA backlog index — work stays findable after it leaves the push window - #1714

Open
anikinsasha wants to merge 9 commits into
danielmiessler:mainfrom
anikinsasha:feat/isa-backlog-index
Open

feat(isa): persistent ISA backlog index — work stays findable after it leaves the push window#1714
anikinsasha wants to merge 9 commits into
danielmiessler:mainfrom
anikinsasha:feat/isa-backlog-index

Conversation

@anikinsasha

@anikinsasha anikinsasha commented Jul 31, 2026

Copy link
Copy Markdown

The problem

An ISA is the memory of a piece of work. Today it goes invisible in two steps: it drops off the SessionStart block at 48h, then out of work.json at 7d, where the TTL deletes the row. After that the work exists only as bytes on disk that nothing tells you about.

That is freshness governing what exists, not just what is pushed. Those are different things, and only one of them should have a window.

What this adds

MEMORY/STATE/isa-index.json — an append-only index of every ISA the instance owns, across both WORK trees and the persistent tool ISAs under LIFEOS/TOOLS/ — plus a Stalled ISAs block in LoadContext that surfaces the unfinished ones long after they leave the push window.

The index keeps everything forever. What the block shows is bounded by stalledMaxAgeDays (30 days by default, 0 for genuinely any age), and the heading states whichever window is in force. Bounding the display is fine; bounding the memory is the thing this exists to prevent.

Three properties are load-bearing:

  • No entry is ever removed. Not on TTL, not on cap eviction, not when the artifact leaves disk. A deleted artifact is tombstoned (missing: true) and un-marks itself if the file returns.
  • There is deliberately no retention knob. Every display window is configurable; the index itself is not, because a retention knob on memory is the defect this removes.
  • work.json rows expire INTO the index, never into the void, and the delete is deferred if the archive write fails. A row never leaves memory because a write lost a race.

Knob-gated, conservative by default

Everything is governed by an isaPickup block in settings.system.json, so you get defaults and each principal decides whether to mandate more:

Knob Default
enabled true. Switches off this index — every write path and the Stalled ISAs block. It does not govern the Recent Sessions block, which predates the index. With no index to archive into, work.json rows are then dropped on expiry rather than kept.
stalledMaxAgeDays 30 (0 = index-forever)
stalledDisplayLimit 5 (a display cap, never a data cap)
recentWorkWindowHours / recentWorkLimit 48 / 8
strandedAfterDays / reconcileMaxAgeDays 7 / 30
workJson.{nativeStartingHours,completeHours,defaultDays,capRows} 4 / 24 / 7 / 50

Verified end to end against a synthetic root: with stock defaults a 7-month-old ISA is held back; at stalledMaxAgeDays: 0 it surfaces; at enabled: false the block disappears entirely.

Three defects in main this also fixes

These are independent of the feature and worth reviewing on their own:

  1. LoadContext scanned only one WORK tree. Sessions live in both ~/.claude/MEMORY/WORK and <LIFEOS_DIR>/MEMORY/WORK; scanning one starved the block of exactly the work it exists to surface. It now scans both, keyed so identical directory names in the two trees cannot collide.
  2. "Done" meant two different things on one screen. The Recent Sessions filter hardcoded status === 'COMPLETED' while the terminal vocabulary elsewhere is fuller (complete/completed/closed/done/abandoned/archived). A session marked CLOSED kept occupying a Recent slot forever. Both paths now route through TERMINAL_PHASES.
  3. IsaReconcile --abandon-old-verify rewrote stranded ISAs to phase: complete. That erased the exact backlog signal this index exists to carry — a reconciler silently marking unfinished work as finished. Retired: it now prints a notice and proceeds, and stranded ISAs are flagged in the index instead. It was wired to nothing, so a hard failure would only punish someone typing the old command.

The sweep never writes an ISA file

Stated precisely, because a vaguer version of this claim was wrong once already. Three independent legs:

  1. File. IsaReconcile.ts contains no write call — no writeFileSync, no writeFrontmatterField.
  2. Call graph, at the write. The only ISA write reachable from the sweep is syncToWorkJson's v6.9.0 Resume After Complete write-back, and the new SyncOptions.artifactReadOnly suppresses it beside the write itself, not from a gate in another file. It suppresses the whole resume, not just the write: no phase flip, no iteration bump, no rework event. An observer observes.
  3. Call graph, by construction. That branch is in fact unreachable from --fix even without the flag: the resume arms only when the registry phase and the frontmatter phase are both exactly complete, and that is precisely the state classify() calls in-sync, which the sync gate skips. Verified empirically against a flag-stripped copy across the whole reachable classify space (in-sync / drift / orphan, matching and mismatching case): zero ISA bytes changed in every case, including the three that did call syncToWorkJson.
  4. Destination. The index writer's output path is operator-configurable, so the claim was refutable by pointing LIFEOS_ISA_INDEX_PATH at an ISA.md and letting the publish rename over it. The destination is now validated rather than merely described: it must be a .json file and must not be an artifact filename.

Leg 3 is why the flag is defense in depth rather than the load-bearing guard, and exactly why it is worth keeping — leg 3 is an emergent property of a classifier, leg 2 is a property of the callee that owns the write.

Design notes

  • Indexing is not gated on --fix. --audit means "no writes to work.json or ISA frontmatter"; append-only memory capture is not a mutation of state anyone reasons about. --no-index opts out.
  • Concurrency — one writer at a time, because nobody ever removes an existing lock. A mkdir lock plus tmp/rename publish. mkdir is the exclusive claim, and it is confirmed by reading our own owner token back out of the directory — a claim that does not read back is not a claim, and proceeding on one would be the only remaining way to end up with two writers. If the lock already exists we defer, unconditionally: whoever owns it, however old it is, alive or dead or unidentifiable. No age check, no liveness check as a decision input. A lock is removed by exactly two actors — the owner that created it, and a human running rm. There is no third, so no interleaving of this module against itself can produce a second writer. Deferral is safe by construction: callers read false as keep-my-rows, and the one caller that cannot tolerate a skip (TTL eviction) checks that boolean before deleting. Readers never see a partial file. A corrupt index is quarantined aside rather than silently wiped.
  • Tolerant read path. A large share of real ISAs put the H1 above the frontmatter block, which the strict parser reads as null. Skipping those would rebuild the very invisibility this removes, so the index uses a separate tolerant reader with derived fallbacks (directory name, H1, mtime, ISC counts). The strict parser is unchanged for every writer.
  • Tool ISAs are discovered by presence, not registration: any LIFEOS/TOOLS/<tool>/ISA.md. They are index-only and never become work.json rows, since work.json is a per-session view and a tool ISA is not a session.
  • Keys are the artifact's directory relative to ~/.claude, so a PRD.mdISA.md rename stays one entry rather than splitting into two.

Notes for reviewers

  • The sweep is registered async at SessionStart (30s timeout) so it stays off the blocking path. A session may therefore render the previous sweep's index; the live upsert in ISASync keeps actively-worked ISAs current regardless. This is memory, not telemetry.
  • Cost, stated plainly rather than as "cheap": the blocking read is one JSON parse of a file that grows without bound by design, and the display cap is applied after the parse, so it bounds output rather than work. The async sweep walks every owned ISA. Both are inexpensive at realistic sizes — order 10ms at a few thousand entries, ~1.3 MB after five years at ~350 ISAs/year — but neither is constant-time. The index is written compact rather than pretty-printed, which roughly halves both. If it ever does matter, the fix is a compact push-view beside the full index, never a retention window.
  • No tests are included: there is no hook-test harness in LifeOS/install/ to add them to. A suite exists on my side and I am happy to contribute it separately if you want one.

Review pass (second commit)

The published diff was reviewed and one critical correctness bug was found and fixed, along with eight places where code and prose disagreed. Detail is in the commit message; the headline:

archiveWorkRows() skipped any non-placeholder row whose session.isa did not resolve, then returned true once no entries remained — and the caller reads true as permission to delete every row in the batch. Pathless rows were therefore deleted unarchived, and a mixed batch archived the path-bearing subset while dropping the rest. That directly contradicted this PR's central promise. The function is now total: a row's memory lives on the row, not the file, so a pathless row is archived under a namespaced key and tombstoned, and a shortfall between memory-carrying rows and produced entries returns false (defer) rather than true.

The remaining eight were either fixed in code (quarantine-failure overwrite, entries-array validation, index destination aliasing, phase:/status: precedence, enabled not gating every write path, the any age heading, the --quiet stderr leak) or made honest in the text (what enabled actually governs, what the SessionStart read actually costs). One unrelated getSessionAgents change that had ridden in on the branch was reverted to upstream rather than fixed here — it belongs to its own change.

Concurrency: how the lock ended up here

Several review rounds went at the write path, and the shape of the mistake is more useful than any individual fix. Every round found the same organ misbehaving in a new way: this module removing a lock it did not create. First the rule was "take it if it looks old", which cannot tell a writer that is stopped — SIGSTOP, a suspended laptop, a stalled network mount, a long GC pause — from one that is dead; the machinery built to survive that mistake grew to generation-named archives, a regression-repair pass and a pruner. Then the rule became "take it if the owner is provably dead", which is better and still wrong: two contenders can prove the same corpse and displace each other, putting a wrongly-captured lock back resurrects a directory another writer had moved on from, and kill(pid, 0) simply lies across pid namespaces and a relocated index path. Each answer was a narrower version of the same idea, and each one produced its own criticals.

So the mechanism is gone rather than refined. The rule is now total: if the lock exists, defer. The cost is real and is stated rather than hidden — a writer that dies holding the lock wedges every later index write until a person clears it. Nothing is lost while it is wedged (writers defer, and work.json rows that should have expired are kept rather than archived), but the surface silently stops updating, and a designed trade with a silent failure mode is just a silent failure mode. So the condition is surfaced where the human who must act will see it: the SessionStart ACTIVE WORK banner renders a notice naming how long the lock has been held, who holds it, what is accumulating meanwhile, and the exact command to clear it. It renders even when there is nothing else to show, because an empty banner is exactly what a wedged index looks like from outside. Nothing auto-escalates and nothing auto-clears.

📋 ACTIVE WORK:

  ── ⚠️  ISA INDEX: WRITES ARE BLOCKED ──

  🔒 The index lock has been held for 3h by pid 1 (appears alive).
     Nothing is lost while it is held: every writer defers, and work.json
     rows that should have expired are KEPT instead of archived. What does
     degrade is that the index stops gaining entries, so the backlog above
     goes stale and newly-finished work stops appearing in it.
     This will NOT clear itself — no code here removes a lock it does not
     own. If that process is gone or is not an ISA index writer, clear it:
       rm -rf <path>/isa-index.json.lock

What remains is a residual-window list re-derived for this protocol rather than edited: three windows (quarantine, commit, release), each reachable concurrently only if a human removes a live lock by hand. Acquire contributes none, because a failed claim leaves the directory exactly as it found it. The claim is exactly that and nowhere stronger: no lock is ever removed except by its owner or a human; this code cannot create a second writer. That is not "races are impossible" — an operator with rm can still make one, and the three windows say what it would cost.

…t leaves the push window

An ISA is the memory of a piece of work, but today it goes invisible in two
steps: off the SessionStart block at 48h, out of work.json at 7d — where the
TTL *deletes* the row. After that the work exists only as bytes on disk that
nobody is told about. Freshness was governing what EXISTS, not just what is
pushed.

This adds MEMORY/STATE/isa-index.json: an append-only index of every ISA the
instance owns, and a 'Stalled ISAs' block that surfaces the unfinished ones at
any age. Knob-gated throughout — 'isaPickup' in settings.system.json ships
conservative defaults (30d display ceiling, 5 rows) and 'enabled: false' turns
the whole surface off.

Three properties are load-bearing:
  - No entry is ever removed. Not on TTL, not on cap eviction, not when the
    artifact leaves disk (it is tombstoned 'missing' and un-marks itself if the
    file returns). There is deliberately NO retention knob: a retention knob on
    memory is the defect this removes.
  - work.json rows expire INTO the index, never into the void, and the delete
    is DEFERRED if the archive write fails.
  - The sweep never writes an ISA file. IsaReconcile contains no write call,
    and the one call-graph path that could reach one (syncToWorkJson's v6.9.0
    Resume After Complete write-back) is suppressed at the write itself by the
    new SyncOptions.artifactReadOnly.

Also fixed, both prerequisites of the above:
  - LoadContext scanned only one WORK tree, starving the block of exactly the
    work it exists to surface. It now scans both.
  - 'Recent Sessions' hardcoded status === 'COMPLETED' while the backlog used a
    fuller terminal vocabulary, so 'done' meant two things on one screen. Both
    now route through TERMINAL_PHASES.
  - IsaReconcile's --abandon-old-verify rewrote stranded ISAs to phase:complete,
    erasing the exact backlog signal this index exists to carry. Retired: it
    prints a notice and proceeds, and stranded ISAs are flagged in the index
    instead. It was wired to nothing, so a hard failure would only punish a
    human typing the old command.

Indexing is not gated on --fix: '--audit' means no writes to work.json or ISA
frontmatter, and append-only memory capture is not a mutation of state anyone
reasons about. '--no-index' opts out.

Concurrency: mkdir lock with stale takeover plus tmp/rename publish. Readers
never see a partial file; on contention a write is skipped, never partial, and
the one caller that cannot tolerate a skip checks the boolean return.

Tolerant read path: a large share of real ISAs put the H1 above the frontmatter
block, which the strict parser reads as null. Skipping those would rebuild the
very invisibility this removes, so the index uses a separate tolerant reader
with derived fallbacks. The strict parser is unchanged for every writer.
…ery claim in this PR true

Review pass on the published diff. One critical correctness bug, and eight
places where the code and the prose disagreed.

CRITICAL — archiveWorkRows could delete memory unarchived.
It skipped any non-placeholder row whose session.isa did not resolve, then
returned true once no entries remained; the caller reads true as permission to
delete EVERY row in the batch. A pathless row was therefore deleted without
being archived, and a mixed batch archived the path-bearing subset while
dropping the rest — the exact "no path deletes memory into the void" promise
this PR makes.

Fixed by making the function total: a row's memory (slug, title, phase,
criteria, timestamps) lives on the ROW, not the file, so a pathless row is now
archived under a namespaced work-expiry key and tombstoned rather than
discarded. Deferring instead would have been worse: an absent path is permanent,
not transient, so eviction would block forever and work.json would grow without
bound. Entry count is now checked against the number of memory-carrying rows,
so a future edit that drops a row degrades to "defer the eviction", never to
"delete it anyway".

Also fixed:

- The append-only guarantee failed when the corrupt-index quarantine rename
  failed: the rebuild was published over unrecoverable bytes anyway. Quarantine
  now reports success, and the writer aborts when it cannot preserve the file.
  An entries ARRAY is also treated as corrupt (typeof [] === 'object' let it
  through as a valid map).

- "The sweep never writes an ISA file" was refutable through the configurable
  index destination: pointing LIFEOS_ISA_INDEX_PATH at an ISA.md made the index
  writer rename over it. The destination is now validated, not just described.

- Field precedence was inverted against this PR's own documented contract:
  LoadContext read status: first and phase: only as a fallback, so
  phase: complete with a stale open status stayed pushed, while phase: build
  with status: CLOSED was hidden. Phase first everywhere now, matching
  normalizePhase(phase || status).

- enabled: false did not stop all index writes (TTL/cap archiving still wrote).
  It now gates every write path. The claim that it disables the "whole surface"
  was false either way — it does not govern the Recent Sessions block, which
  predates the index — so the text now says what it actually does, including
  that turning it off means work.json rows are dropped rather than archived.

- The stalled heading and the docs claimed "any age" while the stock default
  hides anything older than 30 days. The heading now states the window actually
  in force, and the docs distinguish the display bound from the index.

- SessionStart cost is stated honestly rather than as "cheap": one JSON parse of
  a file that grows without bound, with the display cap applied after the parse.
  The index is written compact rather than pretty-printed (roughly half the
  bytes and parse cost), and the lock-contention notice no longer prints under
  --quiet, which is how the SessionStart registration runs.

- Removed an unrelated getSessionAgents change that rode in on this branch: it
  marked partial attestation rows completed. That belongs to its own change, not
  this one, and is reverted to the upstream implementation here.

- Stripped instance-specific provenance (internal task identifiers, dated
  private audit references, local dashboard names) from the code and docs this
  PR carries.
@anikinsasha
anikinsasha marked this pull request as draft July 31, 2026 14:31
…keys, not entries

Second review pass. One critical of the same class as the last one, one
concurrency fix, one port artifact, three claims made honest.

CRITICAL — the totality guard could still authorize deleting a row.
The guard counted entries PRODUCED, but the index stores by KEY and
indexKeyForArtifact keys by DIRECTORY. Two rows whose artifacts sit in one
directory — an ISA.md and a PRD.md side by side, common in real trees — produced
two entries that merged into ONE on write. The count matched, the write
succeeded, and the caller deleted both rows with one row's memory retained.
Same shape as the pathless-row bug it replaced: a guard checking a proxy instead
of the property.

Fixed at the class rather than the instance. Keys are disambiguated before the
write, so two sessions sharing a directory cannot collapse into one entry, and
the guard now counts DISTINCT KEYS. Anything that still collapses fails closed
and the caller keeps its rows. Falsifier is now a test: an eviction batch of two
rows in one directory asserts either both are retrievable or nothing is deleted.

Lock ownership is now verified rather than assumed.
Reported as a size problem: an index designed to grow without bound, protected
by a fixed 10s stale window. Measured, that premise does not hold — the whole
locked operation is ~1.2s at 400k entries / 193MB, three orders of magnitude
past any realistic index. But the race is real for a different reason: a writer
does not out-run the window by being slow, it out-runs it by being STOPPED (a
suspended laptop, a stalled network filesystem, a long pause), and no
size-derived threshold fixes that. So the lock now carries an owner token that
is re-checked immediately before the rename. If it was taken over mid-write, the
merge is discarded rather than published over the other writer's, and the caller
defers. Staleness is measured from the token so a heartbeat can defer takeover
across a long phase.

- The published file defined resolveRowIsaPath TWICE, verbatim — a port artifact
  that made the diff not cleanly type-checkable. Removed, and a real tsc pass
  over every file this PR carries is now part of the pre-push checks (it reports
  TS2393 on exactly this defect, which bun build does not).

- work-expiry: is now a reserved namespace: a real artifact directory of that
  literal name is escaped rather than allowed to mint a colliding key. The
  synthetic key is also unique per INCARNATION rather than per slug, so a later
  pathless row reusing a slug no longer merges over the archived memory of an
  earlier one.

- The retention invariant is stated with its exceptions everywhere it appears,
  rather than as a universal that the code does not implement: placeholder
  (native/starting) rows carry no memory, and with the index disabled there is
  nothing to archive into.

- Permanent index failures are distinguished from transient ones. An invalid
  configured destination, a read-only volume, or corruption that cannot be
  quarantined never self-heals, so work.json defers forever and grows. The
  comment claiming "the next sync clears the backlog" was false for those; the
  cost is now stated plainly and permanent causes are logged distinctly.
  Unbounded growth is recoverable; deleted memory is not.
…de the lock

Third review pass, and the third finding of one class. Rounds 1-3 each added a
guard: none, then a count of entries produced, then a count of distinct keys.
All three measured a proxy, and the last one computed it against an UNLOCKED
snapshot. Nothing evaluated before the lock can establish a property about what
the index contains after it. This commit moves the boundary rather than adding a
fourth predicate.

CRITICAL 1 — key selection ran before the lock.
archiveWorkRows read the index unlocked, built the claimed map, selected keys and
ran the distinct-key guard, then handed entries to a merge that was
identity-blind: mergeEntry spreads incoming over existing, so an entry at a
contested key was replaced outright, slug and all. Two concurrent one-row
archives for different slugs in one directory both saw the key unclaimed, both
passed, both returned true, and the second merge erased the first.

Now there is ONE serialized transaction. Under the lock: read, select keys
against THAT read, merge identity-safely, verify, publish. archiveWorkRows is a
caller that hands rows in; its plan runs inside the critical section. The merge
is no longer identity-blind — a work-expiry entry may never replace, nor be
replaced by, an entry carrying a different slug: the incoming archive re-keys to
its incarnation key, or a resident archive is moved aside to its own before a
fresh observation takes the directory key, and an unresolvable collision fails
closed. Sweep/sync entries keep directory identity, because for an observation
of an artifact a changed slug is an update, not a collision.

The guarantee is now a post-merge check of the actual property: every entry the
transaction was asked to write is retrievable under a key carrying its identity.
The distinct-key count survives only as a cheap early abort.

CRITICAL 2 — the ownership check did not guard the commit.
It ran, then touchLock, then publish did serialize + write + rename — so the
expensive phase and the commit itself sat after the last check. Worse, touchLock
wrote unconditionally, so a writer resuming after a legitimate takeover stamped
its token over the new owner's: the new owner then failed its own check and
aborted while the stale writer proceeded. The mechanism inverted its own
fail-closed direction.

The check now sits inside publish, between writeFileSync and renameSync — the
rename is the commit and the last instruction that can destroy another writer's
index. touchLock verifies before writing and can no longer steal. releaseLock
removes the lock only if it is still ours, so a takeover's lock is not handed to
a third writer. Two residual windows remain, both documented in place rather
than claimed closed: verify-then-write in touchLock, and check-then-rename in
publish. Neither closes without a filesystem compare-and-swap.

CRITICAL 3 — synthetic keys were not injective.
workExpiryKey fell back to '@nostamp' when a row carried no timestamp, so a
reused slug minted one key across incarnations, and the sanitizer could collapse
distinct stamps. Synthetic keys were also never checked against the existing
index — only against the current batch — so a later archive merged over an
earlier one and reported success.

The discriminator is now a hash of what the row carries (stamps, artifact path,
title, phase, criteria counts, session id). Total: no fallback branch.
Deterministic: re-archiving an incarnation is idempotent. Colliding only on
identical content, which is the same memory. Hex output cannot be mangled by the
sanitizer that broke the previous scheme. Collisions are checked against the
index inside the transaction, and an unresolvable one defers.

WARNING 4 — the reserved-namespace escape was not injective: a directory named
'artifact:work-expiry:x' collided with the escaped form of 'work-expiry:x'. The
escape now escapes itself (escape when the key starts with EITHER prefix), which
is injective over the whole key space; the proof is in the comment and the cases
are tested.

WARNING 5 — three failure paths returned a bare false while isa-utils pointed
the operator at a diagnostic that never printed: parent mkdir failure,
non-EEXIST lock-acquire failure, and write/rename exceptions. All three now
report, and classify permanent (EROFS/EACCES/EPERM/ENOTDIR, plus EEXIST on the
recursive mkdir, which means a non-directory occupies the path) distinctly from
transient.

Adds one clearly-marked test seam (__setAfterLockedRead) so the serialization
boundary can be tested with a real interleave rather than a source assertion.
…y pattern contained it

Fourth review pass, narrower than the last: fixes inside the structure the
transaction rewrite established, no new subsystem.

The headline first, because it is the point. isRetrievable() asked "does SOME
entry with this slug and source exist" when the property is "does THIS row's
memory survive at a key carrying ITS identity". It was proxy danielmiessler#4, sitting inside
the guard built to end the proxies.

FINDING 1 — same-slug incarnations were identity-blind.
placeEntry merged whenever `existing.slug === incoming.slug`. Slugs recur — a
directory name is reused — so incarnation 2 of a slug merged over incarnation
1's memory at the directory key, returned true, and the caller deleted its row.
The exists-by-slug guard passed because slug and source still matched; the
earlier incarnation's own phase, criteria and timestamps were gone.
archiveWorkRows made the same false equation at key selection with
`owner === slug -> reuse dirKey`.

Incarnation now governs every work-expiry conflict, same-slug included. It is an
explicit stored field rather than something recomputed: an incoming archive
merges over a resident ONLY when it IS that resident (same slug AND same
incarnation); any other incarnation re-keys exactly as a different slug does. A
resident archive is moved aside to its own incarnation key before an artifact
observation takes the directory key, so an observation never overwrites session
memory either.

The guard is rebuilt on the property. placeEntry returns the key each entry
actually landed on, and verification checks THAT key holds an entry matching
THAT entry's identity — never a scan for something that looks similar.

This also removed a latent inconsistency: incarnationKeyFor() hashed entry
fields while the plan hashed session fields, so one incarnation had two
discriminators and a relocated archive could not be recognised as itself.

FINDING 2 — folded into 1, plus two paper cuts: the discriminator is now
full-width sha256 (truncation to 64 bits was theoretical at this scale, but
widening costs nothing and retires the argument), and the comment claiming
collisions occur "only on identical content" now says collision-resistant. A
hash collision is not identical content; the old wording claimed more than a
hash can deliver.

FINDING 3 — two undocumented lock windows and one false universal.
releaseLock's docstring claimed "ONLY if still ours", which POSIX cannot
deliver: it is check-then-delete, so a takeover in the gap means we remove the
new owner's lock. acquireLock's stale path was age-check-then-rm-then-mkdir —
two syscalls after the measurement, and a contender could mkdir into the gap.

The steal is narrowed by claiming a stale lock with an atomic renameSync to a
tombstone (exactly one contender can win a rename) before mkdir'ing fresh, and
deleting the tombstone after. Release cannot be narrowed the same way — removal
IS the operation — so it is stated instead. All four windows (steal, heartbeat,
commit, release) are now enumerated in ONE place at the top of the lock section,
each with its width and residual. No docstring claims a guarantee the filesystem
cannot provide: "narrows", never "only if".

FINDING 4 — publish()'s catch named EROFS/EACCES in prose but never inspected
err.code, so the "permanent failures are logged distinctly" claim was false on
exactly that path while acquireLock honoured it. It now branches on errno like
the others.
…mit-race class

Fifth review pass, and the narrowest yet. Both criticals were the same shape:
a destructive rename with a check somewhere in front of it, in a world where
"somewhere in front" can be minutes.

CRITICAL 1 — the quarantine rename was unguarded.
quarantineIfCorrupt judged the file corrupt and renamed it aside with no
ownership re-check in between. A writer that judged, then paused past the stale
threshold while a thief took over, quarantined, published valid rows and let its
caller delete them, would resume and rename that VALID index aside as "corrupt".
publish()'s claim to own "the last instruction that can destroy a published
index" was simply false — this rename came earlier. It now re-checks ownership
immediately before the rename, and the window is in the enumeration.

CRITICAL 2 — the commit window had a durability consequence the comment denied.
The residual was documented as "microseconds of syscall latency" with the
failure mode "the caller defers, never a silent delete". Both were wrong. The
window is bounded in INSTRUCTIONS and unbounded in WALL-CLOCK — SIGSTOP,
suspension, a stalled network filesystem — and in that world a stale writer's
rename destroyed a newer publish whose caller had already deleted its rows.

The commit no longer depends on winning a race. It is two renames: the current
canonical is renamed to a generation-named archive, then tmp is renamed into
place carrying generation N+1 and the writer token. A stale writer therefore
DEMOTES the newer publish instead of destroying it, and leaves evidence — the
canonical's generation is now lower than a surviving archive's. Every
transaction's locked read detects exactly that and folds the archive's entries
back in through the same identity-safe placement path, before planning. This
needs no filesystem compare-and-swap; it needs only that rename is atomic.

Retention: an archive is unlinked only after every entry it holds is verified
present in the canonical index under a key carrying that entry's identity.
Unverified archives are kept, forever if necessary. In steady state the archive
written by the previous commit prunes immediately — an append-only merge
contains everything it superseded — so the directory holds zero or one archive
rather than growing. Pruning is capped per transaction so an accumulation
drains over several runs instead of stalling one.

WARNING 3 — the residual list is now DERIVED rather than remembered: the file
was swept for every rename/rm/unlink/write on shared state and each is listed
with the guard in front of it, its consequence, and the note that widths are
bounded in instructions and unbounded in wall-clock. Seven windows, not four —
the quarantine rename had gone four rounds unlisted, which is what a remembered
list does. Writes to pid-scoped temp and tombstone names are excluded and the
exclusion is justified in place.

WARNING 4 — sameIdentity treated two undefined incarnations as equal, because
`undefined === undefined`, so legacy incarnation-less archives merged in place:
the class the incarnation field was added to close, surviving through the
optionality of the field. Undefined now matches nothing. A work-expiry entry is
also never STORED without an identity — one derived from its own fields is
assigned at placement — so legacy entries acquire an identity on first touch,
identical content still merges, and different content re-keys. Duplication is
accepted; loss is not.

WARNING 5 — the permanent-errno set gains EISDIR, ENAMETOOLONG, ELOOP and
EINVAL, and the claim is reworded to what the code does: a known-permanent
SUBSET is named, and anything unclassified defaults to transient-with-retry.
Failing toward "retry" is the safe direction — it never authorizes a delete.
…age required

An external review pass returned six criticals against the generation-archive
commit protocol added in the previous commit, all verified real. They shared one
cause with the criticals of the three rounds before it: the TIME-BASED stale
steal. A writer paused past the threshold — SIGSTOP, a suspended laptop, a
stalled network mount, a long GC pause — is not dead, and stealing its lock puts
two LIVE writers inside the critical section. Age cannot tell slow from dead,
and no size-derived threshold fixes it, because the pause has nothing to do with
the work.

The previous rounds kept treating the symptom: narrow the window, then make the
commit non-destructive, then repair the damage afterwards. Each answer was
larger than the last, and the final one needed archives, a generation counter, a
regression repair pass and a pruner to survive a second writer it had itself
admitted.

REMOVE THE CAUSE.

Takeover is now gated on the owner PROCESS being dead, not the lock being old.
The token already carried the pid; kill(pid, 0) is the predicate, single-host
premise stated in the header. A live owner is never stolen from at any age.
An owner that cannot be identified — no token, an unparseable token, or a
REUSED pid now held by an unrelated live process — is likewise never stolen
from: it defers, and once the lock is old it warns a human and names the manual
unlock. Code that resolves its own ambiguity by deleting is the defect here.
Deferral was already safe: every caller treats false as keep-my-rows.

Takeover, when licensed, is an atomic rename to a private tombstone, then
mkdir. Rename is not compare-and-swap, so two contenders can prove the same
dead owner at once and the second would displace the first live lock — the
capture is therefore verified after the fact, restored on mismatch, and the
race costs a deferral rather than a second writer.

With no steal-while-alive there is at most one writer in a transaction, so the
whole apparatus loses its reason to exist and is deleted: generation archives,
the generation and writer header fields, readIndexGeneration,
repairGenerationRegression, pruneGenArchives, PRUNE_BUDGET, the archive path
helpers, and the touchLock heartbeat that existed only to outrun the staleness
clock. Publish is one rename. All six criticals close structurally — there is
no protocol left for them to describe.

readIsaIndex tolerates an index that still carries the retired header fields:
they are ignored on read and dropped by the next publish. Reading an index as
empty over an unrecognized header would discard real memory.

The residual-window enumeration is re-derived for the new protocol rather than
edited: four windows remain, and each is reachable concurrently only by an
operator clearing a live lock by hand, or by the dead-owner race that now fails
closed. The claim is exactly that and nothing stronger.
…sm is gone

An external review pass audited the pid-liveness protocol from the previous
commit and found that the takeover mechanism was still the thing generating
findings: two contenders can prove the same dead owner and displace each other,
the mismatch-restore puts back a lock the other writer had already moved on
from, and the liveness answer itself is false across pid namespaces and a
relocated index path. Each was a defect in the same organ. So the organ is out.

THE PROTOCOL, complete. If the lock directory exists, defer — always,
unconditionally, whoever owns it and however old it is. No age check. No
liveness check as a decision input. A lock is removed by exactly two actors:
the owner that created it, and a human running rm. There is no third. No code
path removes a lock it did not create, so no interleaving of this module
against itself can produce a second writer. That is structural, not a narrowed
race.

ACQUIRE IS NOW TWO HALVES. mkdir proving exclusive at the instant it ran does
not prove the directory is still ours a syscall later, and the token write is
best-effort — it can fail outright, or land after a human removed our lock and
a fresh writer stamped its own. Returning true on mkdir alone would let a writer
proceed believing it owned a lock another process owns, which with takeover gone
was the only remaining path to two writers. So the token is READ BACK, and a
claim that does not confirm is not a claim. On failure we leave the directory
exactly as found — we do not remove it, because "the lock I think I made" is
indistinguishable from "a lock someone made in the gap", and removing on that
guess is the move this protocol abolished.

THE DEFERRAL IS NOW VISIBLE. Never removing a lock means a writer that dies
holding one wedges every later index write until a person acts. Nothing is lost
— writers defer and work.json rows are KEPT rather than archived — but the
surface silently stops updating, and a designed trade with a silent failure mode
is just a silent failure mode. stuckLockNotice() is a pure read that reports a
lock held past the threshold, and LoadContext renders it on the SessionStart
ACTIVE WORK banner, above the backlog it is freezing. It names the duration, the
owner pid and how it appears, WHAT accumulates while deferred, and the exact
clear command. It renders even when there is nothing else to show, because an
empty banner is precisely what a wedged index looks like. Nothing auto-escalates
and nothing auto-clears.

Pid liveness survives only as describeOwner, feeding the notice text. No
decision rides on it, so being wrong about a pid namespace now costs a
misleading sentence instead of memory.

REMOVED: takeOverDeadLock, the tombstone naming, capture-then-verify, and the
warned-once branch in acquire. Zero references remain.

PROSE SWEPT WHOLE. The review found stale window numbers and takeover language
still in quarantineIfCorrupt after the previous round renumbered around them.
The residual enumeration is re-derived rather than edited: three windows
(quarantine, commit, release), correctly numbered, each unreachable concurrently
except by a human removing a live lock — acquire contributes none, because a
failed claim leaves the directory untouched. isa-utils no longer tells the
operator to look for a diagnostic that ordinary contention does not print; it
points at the banner instead.

THE CLAIM, exactly and nowhere stronger: no lock is ever removed except by its
owner or a human; this code cannot create a second writer.
@anikinsasha
anikinsasha marked this pull request as ready for review July 31, 2026 19:26
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