[jasondeng1997]

Problem

CodeService.index() writes one generation through two resources: the graph store commits rows, and GenerationCache.publish() renames staging-* to generation-*. Those two cannot be atomic. Today's recovery is single-directional — it starts from a directory that exists:

• index() removes leftover staging-* directories and calls store.remove() on each;
• collect() globs generation-* and keeps whatever current.json names.

A commit whose directory is gone has no name left to be found by. Examples: the cache directory was wiped or restored elsewhere, a staging directory was pruned by another process, or a seekdb commit landed and the rename never did. The pc_code_generations / pc_code_nodes / pc_code_edges rows then stay forever and keep counting toward code_cache_limit_exceeded — size() sums them by binding, and nothing ever revisits them.

Change

The publication attempt is named before the store writes a row, and the cache keeps that name.

• GenerationCache.begin_publication(staging) mints the identifier and records {directory, state: "pending"}. CodeService._build passes it down as store.create(..., generation_id=...).
• publish() records state: "published" together with the final generation-* name, immediately after the rename and before anything reads current.json.
• GenerationCache.recover(), called at the top of index() under the build lock, deletes rows for every record whose directory no longer exists, through the new GraphStore.remove_generation(generation_id, binding).
• collect() drops a record once the directory it names is removed. The journal keeps a bounded window (64 attempts).

The seekdb store now accepts a caller-supplied identifier (validated as 32 hex characters) instead of always minting its own. The SQLite store ignores the identifier, because its rows live inside the directory that would have carried the name; remove_generation() is a no-op there for the same reason.

create()'s new argument is keyword-only with a default, so existing callers (including the seekdb e2e test) are unchanged.

Verification

pytest tests/builtin/test_code_publication.py   # 4 passed
pytest tests/builtin/test_native_code.py        # 63 passed
ruff check / ruff format --check / ty check     # clean

test_a_generation_directory_removed_out_of_band_is_reclaimed_by_name and test_a_pending_attempt_without_its_staging_directory_is_reclaimed both fail when recover() is reduced to a no-op — that is what they pin. The other two assert that a live generation is never swept and that the record ends up on the final generation-* name.

Non-goals

• No cross-resource transaction. This keeps the bound RFC 1430 states: at-most-one commit per window, plus a record of which one it was.
• No change to collect()'s retained set, reader leases, or the manifest format.
• No Ledger or multi-worker topology; this does not revive feat: add database-backed distributed work runtime #1446.

Follow-up, deliberately out of scope here: ScopeInvocation-side commits could express a before image through the same record once artifact processing needs it.

[Copilot]

🟡 Changes recommended

Recovery can delete live generation data, lose cleanup handles, and block retries after interrupted initialization.

5 open issues

• Evicting cleanup handles prevents reclamation of retained generations · New
• Crash window can reclaim live generation after staging rename · New
• Clear drops missing-generation records before SeekDB rows are reclaimed · New
• Missing-table recovery fails because connection masks MySQL error 1146 · New
• SeekDB interruption wrapper rejects new create keyword arguments · New

What changed in this PR

This PR adds publication tracking to native code indexing so SeekDB generations can be reclaimed after their directories disappear.

Changes:

• Record generation identifiers before storage writes and recover missing directories during indexing.
• Extend graph stores with caller-supplied identifiers and identifier-based removal.
• Add publication and recovery tests.

File	Description
tests/​builtin/​test_code_publication.py	Tests publication records and missing-directory recovery.
src/​powercontext/​builtin/​code/​store.py	Adds compatible SQLite methods.
src/​powercontext/​builtin/​code/​service.py	Connects indexing to publication tracking and recovery.
src/​powercontext/​builtin/​code/​seekdb.py	Supports named generations and identifier-based deletion.
src/​powercontext/​builtin/​code/​graph.py	Extends the graph-store protocol.
src/​powercontext/​builtin/​code/​cache.py	Adds the publication journal and cleanup logic.

_{Review effort: Balanced}

[hidb4ai]

[P2] Confirmed on d243cc15 with real embedded SeekDB. Publishing 65 generations while retaining their reader leases leaves 65 generation directories but only 64 journal entries. The oldest identifier is already absent from the journal. Removing that generation's directory and invoking recovery leaves its pc_code_generations row untouched. The window must not discard the only cleanup handle for a generation that can still require reclamation; retain it until cleanup succeeds or provide another reliable reconciliation path.

[jasondeng1997]

Fixed in ccec4880. The cap no longer retires records by age.

_write_publications now only considers entries whose directory is gone. Evicting one of those would discard the only handle to rows that still exist, so those are recalled — remove_generation is called and the record is dropped only after the delete — and records whose directory still exists are never touched. If a binding keeps more than PUBLICATION_WINDOW generations alive through reader leases, the count stands above the bound rather than losing a handle. The constant is documented as the normal size, not a hard ceiling.

Test: test_the_record_window_never_drops_a_generation_that_still_exists builds 70 generations with directories plus 6 stale records; it asserts the 6 stale ones are reclaimed and all 70 survive. Fails on the previous commit.

[jasondeng1997]

Fixed in ccec4880, and your 65-generation case is the reason the fix is shaped this way.

The window was capping by age across all records, so a generation that collect() could not retire — a reader holding it keeps the directory alive — lost its record while it still needed one. Capping is now scoped: _write_publications only retires entries whose directory is gone, and retires them by recalling the commit rather than forgetting it. Records for existing directories are never dropped, so a binding holding 65 pinned generations carries 65 records and the excess is allowed to stand.

Test: test_the_record_window_never_drops_a_generation_that_still_exists builds 70 live directories plus 6 stale records and asserts the stale ones are reclaimed while all 70 survive. Fails on the previous commit.

[hidb4ai]

[P1] Confirmed on d243cc15 with real embedded SeekDB 1.4.0.post1. Injecting an OSError from complete_publication() after current.json is replaced leaves the journal pointing at the vanished staging directory. Start a symbols query and pause it after it has pinned the generation; run index(), then resume the query. It returns status="ok", the same fingerprint, and items=[], although run was present before recovery. recover() deletes the live generation's database rows without respecting its reader lease. Recovery must reconcile the staging/final identity and preserve data referenced by active readers.

[AlexStocks]

补一条与真实 seekdb 无关的确定性复现，以及一个能把窗口两侧都关掉的改法。

复现（无需数据库，直接构造缓存状态后调用 GenerationCache.recover()；probes/probe_live_reclaim.py，输出 evidence/probe_live_reclaim.out，WSL/Python 3.13）：

control: reclaimed=0 rows=[] live=['generation-cccc…']                       # 记录已指向 generation-*
crash:   reclaimed=1 rows=[('aaaa…', 'bbbb…')] still_current='generation-cccc…' exists=True
crash:   reclaimed id == live generation id -> True                          # 回收的正是仍被 current.json 引用的 id

即：recover() 的判据只有"记录里的目录是否存在"，它不核对 pointer()，所以在 rename 已发生、current.json 已替换、journal 未更新的状态下，会按 attempt 删除 live generation 的 pc_code_nodes/pc_code_edges/pc_code_generations 行（SQLite 后端因行随目录走，表现为无害；seekdb 后端即上述 P1）。

改法上，"把 journal 更新挪到 current.json 之前"仍留着 rename→journal 的窗口；把窗口两侧都关掉的最小改动是：:303 已经算出最终目录名，因此在 os.replace(staging, directory) 之前就写入 {directory: generation-…, state: "published"}。

• rename 前被杀：目录不存在 → recover() 回收 create() 已提交的行，语义正确；
• rename 后被杀：目录存在 → 记录保留，collect()/_forget_collected() 负责后续收敛。

若不便调整顺序，则 recover() 在回收前必须核对 pointer()["current"] / ["previous"]（并保留读者租约持有的 generation），跳过仍被引用的 id。

[jasondeng1997]

Fixed in ccec4880, and the fix is not to move the update earlier — it is to name the final directory before it exists.

publish() now writes {directory: generation-…, state: "published"} before os.replace. That leaves exactly one possible inconsistency, in the harmless direction: the record names a directory that was never created, and the rows it would recall are the ones this attempt committed. Renaming first and updating after can only be wrong in the direction you describe, where the record survives and the rows do not.

Two windows under the new order:

• killed before the rename: directory missing, rows committed → reclaimed. Correct.
• killed after the rename: directory exists → record kept, so current.json's generation is never a candidate.

recover() also stops trusting the directory name alone: _retained_identifiers() reads the manifest of every generation the pointer reaches and refuses to reclaim an identifier that one of them names. That protects a record written by an older writer, which is the shape you tested.

Test: test_recovery_cannot_reclaim_rows_a_reachable_generation_still_owns reconstructs that old state exactly — record pointing at a staging name that is gone, current.json pointing at the live generation — and asserts reclaimed == 0. Fails on the previous commit.

[jasondeng1997]

Reproduced from your description and fixed in ccec4880. Thank you for the concrete probe — pinning a reader and resuming it after index() is what makes the data loss visible rather than inferred.

Root cause is the ordering of the two writes, not the reader lease itself. publish() renamed the staging directory to generation-*, replaced current.json, and only then updated the record. A crash from complete_publication() left the record naming the staging directory that no longer existed, so recover() concluded "abandoned" and deleted rows belonging to the generation current.json was serving.

Two changes, because the ordering fix alone does not repair a record written by an older build:

1. the record now names the final directory before os.replace, so the window is one-sided;
2. recover() reads the manifest of every generation the pointer reaches and refuses to reclaim an identifier it finds there.

The second is what protects the state you constructed: a record pointing at staging-… while the live generation's manifest holds that same identifier.

Test: test_recovery_cannot_reclaim_rows_a_reachable_generation_still_owns reconstructs your crash state from current.json plus a hand-edited journal and asserts nothing is reclaimed. Fails on the previous commit.

[jasondeng1997]

Your repro is now a test, and your suggested ordering is what the fix does.

publish() writes the record naming generation-… before os.replace. The window is then one-sided:

• killed before the rename → directory absent → recover() recalls the rows create() committed. Correct.
• killed after the rename → directory present → the record survives and collect() / recover() converge later.

I took the second half of your suggestion as well, because the ordering fix cannot repair a journal written by an earlier build: recover() now reads the manifest of every generation the pointer reaches and skips an identifier it finds there. A record still pointing at a vanished staging name therefore no longer decides a live commit.

test_recovery_cannot_reclaim_rows_a_reachable_generation_still_owns is your probe as a deterministic test, no database involved: control unchanged, then the record repointed at staging-renamed-away while current.json still reaches the generation — reclaimed is asserted 0. It fails on the previous commit with reclaimed == 1.

[AlexStocks]

Confirmed on 1072176e: the age-based cap is gone, but the replacement predicate still decides by "the recorded directory is gone" — the inference recover() was just taught to distrust. Two consequences, both reproducible without a database.

1. recover() bypasses its own retained guard when the journal is over the window

recover() computes retained for the abandoned pass and then hands the surviving records to _write_publications() because len(records) > PUBLICATION_WINDOW (cache.py:250-251). That call has no retained set, so a record whose directory is gone but whose identifier a reachable generation still owns is recalled anyway — the exact state the new docstring says must be preserved.

probes/probe_window_recall.py (stubbed store records the remove_generation calls; the generation named by current.json has graph.id = 1111…):

A. journal under the window (5 pinned records)
  recover(): returned=0 recalls=[] kept=True
B. journal over the window (69 pinned records)
  recover(): returned=0 recalls=['11111111111111111111111111111111'] kept=False

The mutation check shows which pass is guarded: deleting attempt not in retained from cache.py:245 fails test_recovery_cannot_reclaim_rows_a_reachable_generation_still_owns (1 failed, 8 passed), so the abandoned pass has a regression guard and the window pass has none.

2. The record written before the rename is the first window casualty, and on seekDB that deletes the rows it is about to publish

publish() now records {"directory": directory.name, "state": "published"} before os.replace() (cache.py:346-352). At that instant the recorded directory cannot exist yet, so the fresh record is the only "stale" candidate whenever every older record still has its directory — the pinned-by-readers state the PUBLICATION_WINDOW comment allows to stand above the bound. excess = len(records) - 64 is then >= len(stale), stale[:excess] takes the fresh record, and remove_generation(attempt) runs before the rename.

probes/probe_publish_window.py (real GenerationCache, real shared reader locks on the pinned generations, stubbed store):

A. journal at the window (63 pinned + 1 new attempt)
  records before publish=64 pinned=63
  remove_generation calls=[]
  the published generation's rows were recalled: False
B. journal over the window (70 pinned + 1 new attempt)
  records before publish=71 pinned=70
  remove_generation calls=['82444b4469dd44698269b7fcef237953']
  published generation=generation-3f52cf5dc6fb406b83c348af50d7069f attempt=82444b4469dd44698269b7fcef237953
  the published generation's rows were recalled: True

SQLite is unaffected (remove_generation is a no-op; rows live inside the generation directory). On seekDB it becomes DELETE FROM pc_code_nodes/edges WHERE generation_id = %s (seekdb.py:425-430) against rows CodeService wrote between begin_publication() and publish(..., attempt=publication) (service.py:159-177), and os.replace() then makes that emptied generation current. The journal only reaches 65+ records when records cannot be settled, so the state is narrow — but it is the state this new ordering creates for itself, and it fails silently because manifest still claims the nodes.

Smallest change that closes both: hand _write_publications() the identifiers recover() already computes and exclude them from the trim candidates — e.g. retained = self._retained_identifiers() | {attempt} passed from begin_publication()/publish(), with stale filtered by attempt not in retained. For coverage, pin N generations and assert publish(..., attempt=…) leaves that attempt's rows alone; a second case with len(records) > PUBLICATION_WINDOW plus one moved-away directory covers the recover() hole.

[hidb4ai]

[P1] Independently confirmed on 1072176e with real embedded SeekDB, normal CodeService.index(full=True) calls, the unchanged 64-entry window, and real shared reader locks. Keeping the first 64 generations pinned makes the 65th index return status="ready" even though its identifier has no rows left in pc_code_generations or pc_code_nodes. The subsequent public symbols query fails with index_integrity_failed.

The new record names the final directory before the rename, so _write_publications() treats that still-missing directory as stale and deletes the rows just committed for publication. The first 64 indexes remain readable; no injected publication failure is needed. Please protect the currently publishing attempt from window reclamation and apply the same live-generation safety checks to every reclamation path.

[hidb4ai]

[P2] Confirmed on d243cc15 with real embedded SeekDB. After indexing, remove the current generation directory out of band, call CodeService.clear(), then index again. clear() reports success and empties the publication journal, but the original identifier still has a row in pc_code_generations after the subsequent index. Its graph payload therefore remains charged to the binding with no recovery handle. Preserve missing-directory records until their database cleanup succeeds; directory absence alone does not establish that collection reclaimed the commit.

[jasondeng1997]

Fixed in ccec4880. _forget_collected() is gone.

You are right that the criterion was inverted: directory absence was treated as proof that collection had reclaimed the commit, when the record is precisely the thing that still knows how to. Collection no longer touches the journal at all; recover() reclaims the rows and drops the record only after remove_generation returns. clear() now calls recover() after unpublishing, so the case you describe — directory removed out of band, then clear() before any index() — settles the commit instead of erasing the handle.

Test: test_clear_settles_a_record_whose_directory_went_missing removes the current generation directory out of band and then clears; it asserts the identifier was recalled by name and the journal is empty. Fails on the previous commit.

[jasondeng1997]

Fixed in ccec4880. _forget_collected() is removed and clear() now runs recover().

Directory absence tells you a directory is gone; it does not tell you who owns the rows that outlived it. The record was being dropped on that inference, which is why clear() reported success while the identifier still had a pc_code_generations row. Now the record is dropped only after remove_generation returns, and clear() settles the journal after unpublishing — which is the last point where the binding still has a name.

Test: test_clear_settles_a_record_whose_directory_went_missing follows your sequence — index, remove the current generation directory out of band, clear() — and asserts the identifier was recalled by name and the journal is empty. Fails on the previous commit.

[hidb4ai]

[P2] Confirmed on d243cc15 with real embedded SeekDB for a directly constructed CodeService using SeekDBGraphStore. Leave a pending publication record before graph-table initialization and remove its staging directory. Two successive index() calls both fail with CodeError("code_storage_unavailable"), caused by MySQL 1146 for the missing pc_code_edges table. Recovery runs before create() can initialize the tables. connection() converts the MySQL exception on context exit, so this outer handler cannot suppress it. Handle the absent-table case before that conversion so an interrupted first build remains retryable.

[jasondeng1997]

Fixed in ccec4880. The handler moved inside the connection context.

You are right that the outer except pymysql.MySQLError was unreachable: connection() raises CodeError from its own except, so remove_generation never saw 1146 — it propagated as code_storage_unavailable and recovery failed before create() could create the tables. The try/except is now inside with self.connection(...), so the missing table is consumed before the context manager converts anything, and the interrupted first build stays retryable.

Test: test_a_commit_can_be_recalled_before_its_tables_exist drives the real connection() against a fake pymysql.connect whose cursor raises ProgrammingError(1146). On the previous commit it fails with CodeError: code_storage_unavailable, which is the exact symptom you reported.

[jasondeng1997]

Fixed in ccec4880. Your diagnosis of the conversion order was exactly right.

connection() raises CodeError from its own except pymysql.MySQLError, so remove_generation's outer handler was unreachable — 1146 surfaced as code_storage_unavailable and both retries died in recovery before create() could initialize the tables. The try is now inside the with self.connection(...) block, so the missing table is consumed before the context manager converts anything.

Test: test_a_commit_can_be_recalled_before_its_tables_exist drives the real connection() against a fake pymysql.connect whose cursor raises ProgrammingError(1146), and asserts remove_generation returns instead of raising. Fails on the previous commit with code_storage_unavailable — the same error you saw.

[jasondeng1997]

Fixed in 5d26c4c9, before this round of review. interrupted_create in tests/e2e/test_native_code_seekdb.py now takes *args, **kwargs and forwards both, so the generation_id keyword reaches create() and the subprocess still stops at its pause marker. code-seekdb (3.11) and (3.14) both pass on ccec4880.

[Copilot]

🔵 Needs a closer look

Interrupted initialization can block indexing retries, and full-cache deletion remains outside the implemented recovery coverage.

5 open issues

• SeekDB interruption wrapper rejects new create keyword arguments
• Missing-table recovery fails because connection masks MySQL error 1146
• Clear drops missing-generation records before SeekDB rows are reclaimed
• Crash window can reclaim live generation after staging rename
• Evicting cleanup handles prevents reclamation of retained generations

2 previously missed issues

In code that hasn't changed since last review

Preserve recovery metadata when wiping the cache

src/​powercontext/​builtin/​code/​cache.py:208

Wiping the cache, one of the failure scenarios named in the PR, also deletes publication.json. On the next index(), the binding directory is recreated, this returns an empty journal, and recovery leaves the old SeekDB rows counting toward the cache limit. To cover a full cache wipe, keep recovery metadata outside the disposable cache or reconcile stored generations by binding. Otherwise, narrow the recovery claim to lost generation/staging directories with an intact journal.

Handle missing-table errors before connection error conversion

src/​powercontext/​builtin/​code/​seekdb.py:434

connection() converts pymysql.MySQLError into CodeError before this handler runs, so it cannot suppress missing-table error 1146. With a directly constructed CodeService, interruption after begin_publication() but before table initialization leaves a pending record. Subsequent index() calls then fail in recovery before create() can initialize the tables. Catch error 1146 inside the connection context so this interrupted first build remains retryable.

_{Review effort: Balanced}

[jasondeng1997]

CI state on d243cc15 (40/41 completed):

check	result	note
code-seekdb (3.11) / (3.14)	pass	failed on the first commit — the interrupted-build harness patched create(self, *args) and could not forward the new keyword argument. Fixed in the second commit.
windows	pass	first run failed in real_server.HarnessFailure: Desktop isolated fixture failed (installed_element_timeout); nothing in this PR touches the desktop fixture.
validate	fail	fails identically on the base commit. master @ e4efb39b: 2 failed, 269 passed, both at e2e/bub/tests/test_harbor_job_config.py:874; this branch reports the same two counts and the same location. It compares generated guidance text, which this PR does not touch — left alone rather than widening the diff.
Atomic Memory clean migration (OceanBase)	still running	real-database migration job, >30 min at the time of writing.

Everything else is green, including the unit / e2e matrices and both code-seekdb legs.

[hidb4ai]

Reviewed d243cc15. I independently reproduced four recovery defects with real embedded SeekDB and added the concrete results to the existing threads:

• P1: Recovery deletes a generation held by an active reader. The in-flight query returns status="ok" and an empty result for the same fingerprint.
• P2: Clear loses cleanup handles for missing directories.
• P2: The 64-entry window evicts handles for retained generations.
• P2: Recovery prevents retry after interrupted first-time initialization, on the directly constructed CodeService path.

These recovery failures should be addressed before merging.

[Copilot]

🟡 Changes recommended

Previously confirmed recovery defects remain unresolved, including deletion of live generation data and loss of cleanup identifiers.

5 open issues

• SeekDB interruption wrapper rejects new create keyword arguments
• Missing-table recovery fails because connection masks MySQL error 1146
• Clear drops missing-generation records before SeekDB rows are reclaimed
• Crash window can reclaim live generation after staging rename
• Evicting cleanup handles prevents reclamation of retained generations

_{Review effort: Balanced}

[jasondeng1997]

Rebased onto fe50f894. validate is green now — I want to correct my earlier note about it.

When I first looked, validate failed on this branch and on the base commit e4efb39b, with two tests at e2e/bub/tests/test_harbor_job_config.py:874. I reported it as a pre-existing failure that was not mine to touch. The diagnosis was too shallow: I compared against the base commit but never re-checked whether master had moved past it.

It had. The failure was a real bug, and #2090 fixed it in a8383b4d:

#2043 added a __POWERCONTEXT_SCOPE_SCRIPT_JSON__ placeholder to the Kimi Code skill that setup replaces with the installed scope.mjs path. The ON arm's installer, added in #2038, only filled the Scope placeholder, so its guidance kept the raw marker and no longer matched setup.

So the guidance text differed in exactly one line — node "<abs>/scripts/scope.mjs" on the setup side versus the raw __POWERCONTEXT_SCOPE_SCRIPT_JSON__ marker on the simulated-install side. Nothing in builtin/code was involved.

What changed in this PR: only the base. Two commits, same diff as before, +286/-8 over six files, no conflict in the rebase. git log upstream/master -6 -- src/powercontext/builtin/code/ shows no upstream activity in that directory since e4efb39b, so no code changed underneath.

Re-verified locally on the rebased tree:

check	result
tests/builtin/test_code_publication.py + tests/builtin/test_native_code.py	67 passed
ruff check / ruff format --check	clean
ty check	clean
validate / code-seekdb (3.11) / code-seekdb (3.14) on CI	success

The rest of the matrix is still running. The lesson I am taking from this: "red on the base commit" is not a conclusion until the base commit is the current head. validate was red on e4efb39b; it was green on fe50f894 the whole time.

[AlexStocks]

[P2] 四条新用例没有断言"记录键 == 提交行的 generation_id"，seekdb 侧的关键不变量无回归保护

RecordingStore 只覆写 remove_generation 并记录参数（:41-42），create 仍走 SQLiteGraphStore，而 SQLite 实现把 generation_id 直接丢弃（store.py：del generation_id）。因此这四条用例验证的是"记录键被传给回收入口"，而不是"行就是以该键提交"。

后果是可静默失败：若 SeekDBGraphStore.create 忽略 generation_id 继续 uuid4()（或 len != 32 / 非 hex 校验分支写错），remove_generation 会执行 DELETE ... WHERE generation_id = <无人使用的 id>，删 0 行且不抛错，本 PR 要修的泄漏原样存在，全部新用例仍然通过。tests/e2e/test_native_code_seekdb.py 本次只把 interrupted_create 改成转发 kwargs，也没有断言 graph.id == attempt。本 head 全仓（grep -rn "generation_id" tests/）没有把调用方传入的 id 与提交行 id 做比较的断言。

建议：在测试替身里同时记录 create 收到的 generation_id（子类可在 del 之前记录再调 super()），并断言 store.created == store.reclaimed；或让 seekdb e2e 断言 manifest["graph"]["id"] 等于 publications() 的键。这样"名字 → 行"的对应关系才有回归保护。

[jasondeng1997]

Fair criticism, and it was the most dangerous gap in the batch: a name that nothing was committed under makes recall a silent no-op, so all four new tests could pass while the leak they describe survived.

Fixed in ccec4880. Three assertions now cover the correspondence:

• the test double records what create() actually receives (store.created) and the existing recall test asserts store.created == [attempt], so the name handed to the store is the name used to delete;
• test_the_recorded_name_is_the_identifier_the_store_committed reads manifest.json back and asserts manifest["graph"]["id"] == attempt, which is the assertion you proposed;
• tests/e2e/test_native_code_seekdb.py::test_a_record_names_the_row_the_commit_wrote asserts the committed row identifier appears in publications() on real embedded SeekDB.

I added a NamedStore double rather than asserting against SQLiteGraphStore, precisely because the SQLite implementation discards the name — asserting there would have encoded the wrong invariant. The double keeps the manifest-shaped reference so the assertion is about the correspondence and not about a backend's storage layout.

[hidb4ai]

Re-reviewed 5d26c4c9 against fe50f894. All seven changed files are byte-for-byte identical to d243cc15; the recovery implementation and its tests are unchanged.

I replayed the original probes with real embedded SeekDB on this head. All four failures remain:

• P1: Recovery deletes a generation held by an active reader. After the publication-journal write fails and indexing runs again, an in-flight symbols query returns status="ok", the same fingerprint, and items=[] instead of the previously visible run symbol.
• P2: Clear discards the only cleanup handle. Removing a generation directory, calling clear(), then indexing again leaves the old database row present while its journal entry is gone.
• P2: The 64-entry window evicts handles for retained generations. With 65 reader-retained generations, only 64 records survive; recovery cannot reclaim the oldest identifier after its directory disappears.
• P2: Interrupted first-time initialization remains unretryable, for a directly constructed CodeService using SeekDBGraphStore. Two successive index() calls still fail with code_storage_unavailable, caused by missing-table error 1146, because connection() converts the exception before the outer handler can catch it.

Please address these recovery failures before merging. Recovery must preserve generations held by readers, cleanup handles must remain available until database cleanup succeeds, and missing-table handling must run before exception conversion.

[jasondeng1997]

All eleven threads are answered individually. ccec4880 addresses the four defects, and the batch had one shape: the record was being treated as bookkeeping about directories when it is the only handle to rows that can outlive the directory that carried them.

#	defect	where it was fixed
P1	recovery deleted a live generation	the record names the final directory before os.replace; recover() also refuses an identifier a reachable generation's manifest names
P2	clear() forgot the handle instead of settling it	_forget_collected() deleted; recover() recalls, then forgets; clear() runs it after unpublishing
P2	the record window evicted live handles	capping only retires entries whose directory is gone, and retires them by recalling the commit
P2	1146 could not be caught outside connection()	handled inside the connection context, before the conversion to CodeError

The P1 fix is two changes because the ordering alone cannot repair a journal written by an earlier build. Ordering narrows the window to one harmless direction — a record naming a directory that was never created, whose rows are the ones that attempt committed. The manifest check is what covers the state that was actually reported: a record naming staging-… while current.json reaches the generation that owns those rows.

Red/green, not just green. Stashing the source changes and running the new file gives:

FAILED test_recovery_cannot_reclaim_rows_a_reachable_generation_still_owns  <- P1, reclaimed == 1
FAILED test_clear_settles_a_record_whose_directory_went_missing             <- P2, reclaimed == []
FAILED test_the_record_window_never_drops_a_generation_that_still_exists    <- P2, record evicted
FAILED test_a_commit_can_be_recalled_before_its_tables_exist                <- P2, CodeError: code_storage_unavailable
4 failed, 5 passed

All four pass on ccec4880; the file is 9 passed, tests/builtin/test_native_code.py is 63 passed, ruff / ruff format / ty clean.

On the missing invariant: the identity assertions are added at three levels — the double records the identifier create() receives and the recall test asserts store.created == [attempt], manifest["graph"]["id"] == attempt is read back from disk, and test_a_record_names_the_row_the_commit_wrote checks the committed row against the record on real embedded SeekDB. A NamedStore double carries the manifest-shaped reference; asserting this against SQLiteGraphStore would have encoded the wrong invariant, since that backend discards the name by design.

+540/-10 over seven files. CI is running on ccec4880; I will report the matrix when it settles.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

9 open issues

• As written, remove_generation() will delete rows from pc_code_nodes/pc_code_edges solely by… · New
• publications() currently accepts any dict-valued record and coerces missing/invalid directory… · New
• publications() currently accepts any dict-valued record and coerces missing/invalid directory… · New
• started_at is read from JSON and can be non-numeric (e.g., corrupted journal or unexpected old… · New
• SeekDB interruption wrapper rejects new create keyword arguments
• Missing-table recovery fails because connection masks MySQL error 1146
• Clear drops missing-generation records before SeekDB rows are reclaimed
• Crash window can reclaim live generation after staging rename
• Evicting cleanup handles prevents reclamation of retained generations

_{Review effort: Lite}

[jasondeng1997]

CI is fully green on 1072176e — 43/43 completed, 0 failed. It took three runs, and the two failures in between are worth recording because they were both noise, and both provably so.

All three runs share the identical tree T. The two commits between them are --amend --no-edit rewrites: no file changed, only the SHA, which re-triggers the workflow.

run	head	tree	result	the one failure
1	ccec4880	T	42/43	tests (3.12, e2e) — test_zcode_source_is_processed_and_recalled_in_a_new_session asserted empty stderr and got {"component":"powercontext.zcode","stage":"scope","code":"timeout"}
2	48cc4190	T	42/43	Atomic Memory clean migration (SeekDB) — test_rerun_completes_a_database_left_by_an_earlier_reference_conversion[seekdb] killed by Timeout (0:02:00)
3	1072176e	T	43/43	—

Two properties make these failures attributable to the runner rather than the change:

1. Cross-run swap on the same tree. Each run failed a different job that the other runs passed. If either failure were real, it would reproduce on every run of the same tree; instead each disappeared on the next run.
2. No intersection with the diff. The first failure is a zcode service-chain test importing atomic_memory.extraction, server.factory, persistence.sqlite — none touched by this PR. The second is an atomic-memory migration test hitting a 120-second pytest-timeout; it passed in run 1 on the same tree.

For the first one there is additional corroboration: master (fe50f894, still its head) carries byte-identical zcode test code and its tests (3.12, e2e) passed four times on 2026-10-20.

Final matrix on 1072176e: the full tests matrix (3.11–3.14, unit + e2e), both code-seekdb legs, Acceptance (oceanbase/sqlite), both Atomic Memory clean migration jobs, Recall effort, validate, quality, windows, controlled-cli, packaging and website jobs — all success.

[hidb4ai]

Re-reviewed 1072176e with real embedded SeekDB. The missing-directory clear() case and interrupted first-time initialization retry now succeed.

Two P1 issues remain: window reclamation deletes the generation being published, and recovery deletes a reader-pinned generation once it leaves current/previous, as detailed inline. The first returns a ready index that cannot be queried; the second returns a successful empty query for the same fingerprint. Please address both before merging.

[hidb4ai]

[P1] Preserve reader-pinned generations after they leave current/previous

This set only covers the two generations in current.json, while reader leases can keep older generations alive. On 1072176e with real embedded SeekDB, I reconstructed the old-writer crash state by pointing the current identifier's journal entry at a vanished staging name. Initial recovery correctly skips it. I then paused a public symbols query after it pinned that generation and ran three index(full=True) calls without changing the source. On the third recovery, the identifier is no longer in current/previous, so its database rows are deleted even though the original directory and reader lock still exist. The pending query returns status="ok", the same fingerprint, and items=[] instead of run.

This occurs below the 64-entry window. The control with the journal naming its final directory keeps the rows and returns run. Recovery must reconcile and protect all generations still held by readers before deleting an identifier from an older publication record.