fix(maintenance): close post-Terra RW acceptance gaps (#3858) - #3858
Conversation
Problem A rebuild accepted a source.db whose user_version lagged the package that would parse and rebuild from it. The live archive reached that state at source v28 while the installed package expected v24 and master expects v29. What changed The rebuild route now checks source.db and user.db before provenance, ownership, or candidate creation. A read-only --preflight option exposes the same structured diagnostic. index.db remains exempt because rebuilding it is the operation's purpose. Compatibility/migration Operators must migrate durable tiers and deploy the matching package before rebuilding index.db. The maintenance runbook records the ordered recovery sequence. Co-Authored-By: Codex <noreply@openai.com>
Problem The initial currency gate checked only source and user tiers, leaving audit, daemon bulk transaction setup, an ownership-acquisition race, and the CLI empty-source path outside the durable schema boundary. The daemon surfaced a currency mismatch as an unstructured 500 response. What changed The gate now derives every durable migration tier from the canonical set, rechecks after archive ownership is acquired, and runs before daemon bulk bookkeeping. The CLI delegates empty sources to the guarded operation and rejects daemon preflight. Currency errors carry the original diagnostic to the daemon HTTP route with conflict semantics. Compatibility/migration Operators must bring source, user, and audit durable tiers to the deployed package versions before a rebuild. No migration or live archive mutation is performed by this change. Co-Authored-By: Codex <noreply@openai.com>
Problem: topology status and method claims lacked a reusable candidate/live census, and unresolved-parent composition had no report-level proof through the archive read seam. What changed: extend lineage-validation with effective topology states, method and cycle-evidence counts, bounded unresolved-parent read checks, candidate index selection, and a receipt digest. Exercise the helper through the production cycle-quarantine writer and mutation fixtures. Compatibility/migration: preserve nullable status for ordinary resolved and unresolved rows; no schema or archive writes are introduced. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: the topology implementation checkpoint must distinguish candidate proof from unavailable live evidence. What changed: record the candidate census results, production-route cycle evidence, read-safety proof, and the exact unexercised live step in the established evidence-report format. Compatibility/migration: this is a read-only evidence record and makes no archive or Beads changes. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: the evidence report was rejected by the generated docs-surface gate because it lacked a registry entry. What changed: register the topology live-proof residue under the evidence tier and regenerate docs/README.md. Compatibility/migration: documentation-only change; no runtime or archive state changes. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: The topology census could pass with hand-built rows, a self-hash without source identity, or an empty unresolved sample.\n\nWhat changed: Build a writer-backed candidate fixture, bind each report to database and SQLite sidecar identities across a held read transaction, and make unresolved-reader sampling report not_observed instead of passing vacuously. Add explicit candidate override and mutation tests.\n\nCompatibility/migration: The nullable raw status contract remains unchanged; effective resolved, unresolved, repaired, and quarantined states are still reported separately. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: topology receipts hashed whole files, emitted variant census shapes, and accepted malformed or contradictory quarantine evidence. Reader composition also treated a quarantined resolved edge as traversable, while the receipt mutation test changed its capture time on every run. What changed: stream snapshot hashing from a transaction-bound read, stabilize the census schema, validate cycle evidence structurally, report contradictory quarantine rows, and exclude quarantined edges from lineage readers and repair traversal. Add fixed-clock receipt reproducibility and production-route mutation tests. Compatibility/migration: no schema or archive data changes. Live census evidence remains explicitly unobserved.
Problem: the rebuild preflight now checks audit.db with every durable tier, but a missing audit tier had no safe operator route. The runbook also named only source and user, so the new gate could block a live upgrade without an executable recovery. What changed: add an explicit --initialize-missing path to migrate-tier under the existing daemon-stop and archive-ownership guards. It refuses any existing path, reports a structured initialization receipt, aligns help and runbook language with the canonical durable tier set, and pins that set in CLI coverage. Verification: devtools test tests/unit/cli/test_archive_maintenance_cli.py -k "migrate_tier_cli or rebuild_index_preflight or rebuild_index_empty_source_still or rebuild_index_rejects_daemon_schema_preflight" (12 passed, 55 deselected). Co-authored-by: Codex <noreply@openai.com>
Problem: a parent-known write normalized away a copied prefix before graph resolution discovered that the asserted parent would close a cycle. Quarantining that edge then prevented composition and silently served only the divergent tail. A deferred BEGIN also hashed WAL sidecars before SQLite established the read snapshot. What changed: preflight the projected parent chain before prefix slicing while leaving persisted quarantine authority in the graph resolver. Exercise sync and async production readers against the writer-created cycle, reject stale parent projections in the census, and force the first SQLite read before snapshot hashing. Verification: devtools test tests/unit/storage/test_topology_cycle_quarantine_live.py tests/unit/devtools/test_lineage_validation.py (16 passed). Co-authored-by: Codex <noreply@openai.com>
Stage a canonical durable-tier database under a private sibling path and publish it with a no-replace hard link. A concurrent creator now wins without having its file opened, modified, or replaced. Verify parent ownership, staged inode stability, and single-link identity. Cover the publish race through the operator CLI and document the no-replace guarantee. Co-Authored-By: Claude <noreply@anthropic.com>
Require quarantine evidence to describe a closed cycle anchored to the stored source and asserted parent. Report walk-budget exhaustion separately instead of accepting it as cycle proof. Hold a second SQLite observer across the census and reject any external commit between the reader snapshot and the two file-set hashes. Add mutations for unrelated cycle JSON, budget exhaustion, and a concurrent WAL commit. Co-Authored-By: Claude <noreply@anthropic.com>
Narrow decoded JSON to a string path only after every shape check succeeds, keeping the hardened evidence validation strict under mypy. Co-Authored-By: Claude <noreply@anthropic.com>
Recheck every durable schema after daemon archive ownership, initialize every durable tier in the status fixture, and supply the required provenance receipt for its daemon transaction. Render local empty-source receipts before replay-only counters and document executable source, user, and audit migration commands. Cover the post-ownership race and exact empty-source plain output. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: a same-UID process could replace the named staging path after initialization and before publication, substituting bytes that were never schema-verified.\n\nWhat changed: copy the initialized database from its still-open descriptor into an anonymous inode, fsync it, and atomically link that exact inode into the absent tier path. Add a CLI route test that replaces the staging name before publication and verifies the canonical database is published while the replacement is untouched.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Problem: the bounded parent walk labeled budget exhaustion as cycle evidence, and the census accepted closed path JSON without verifying its hops against the stored projection. What changed: return a typed walk outcome, quarantine exhaustion as indeterminate while retaining the complete child transcript, and require every evidence hop to match the live parent projection. Production-route tests cover a 1,024-hop acyclic chain and fabricated cycle evidence. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: the post-ownership schema race test reached the helper through a module that imports but does not export it, which violates the strict type surface. What changed: capture the production ownership assertion from its defining archive-identity module while continuing to replace the daemon call site for the race injection. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: copying from a verified named staging file still allowed another same-UID process to mutate that inode in place before publication. What changed: initialize the canonical tier in memory, serialize it directly into an anonymous inode, and atomically publish that exact inode. The CLI route now proves no writable staging name exists before publication. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: a durable migration between daemon transaction resolution and its later page-selection ownership hold could reach receipt and source-page consumption before the shared rebuild engine rejected it. What changed: run the shared durable schema-currency gate immediately after page-selection ownership is proven. A real pass-driver race test advances audit.db at that boundary and makes page selection fail-fast if the recheck is removed. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: the topology census treated any parent pointer on a child with a quarantined edge as stale, even when a separate resolved edge legitimately supported that projection. What changed: count a stale projection only when it points at the quarantined asserted parent without support from a non-quarantined resolved edge. A production writer fixture retains a valid parent while quarantining a cycle-closing alternate assertion. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: the topology census grouped unresolved links without their full identity. Distinct links from one child could multiply stored message counts and incorrectly fail an otherwise safe production-reader sample. What changed: group samples by the complete unresolved edge key, count distinct message identities, and cover multiple unresolved links from the same child. Point the evidence report at the receipt follow-up that owns the remaining live census. Verification: direnv exec . devtools test tests/unit/devtools/test_lineage_validation.py (16 passed). Co-Authored-By: Codex <noreply@openai.com>
Sinity
changed the title
|
Warning Review limit reached
Next review available in: 40 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (22)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d34e663cbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "archive_root": str(config.archive_root), | ||
| "index_db": str(index_db), |
There was a problem hiding this comment.
When --index-db selects a candidate outside the configured --archive-root, all counts and hashes come from that candidate, but the report still labels archive_root as the configured root. _demo_summary then copies only that root and omits index_db and snapshot_identity, so its standalone artifact can appear to prove the live archive while actually describing an inactive candidate. Derive the evidence root from the selected database or propagate the exact database and snapshot identity into every generated artifact.
AGENTS.md reference: AGENTS.md:L477-L482
Useful? React with 👍 / 👎.
| WHERE l.resolved_dst_session_id IS NULL | ||
| AND COALESCE(NULLIF(TRIM(l.status), ''), 'unresolved') = 'unresolved' |
There was a problem hiding this comment.
When a child already has a resolved parent edge and another asserted parent has not been ingested, this query samples the unresolved alternate even though the production reader legitimately composes the resolved parent. The safety predicate consequently sees a non-NULL parent_session_id and additional served messages, marks the sample unsafe, and makes an otherwise valid archive non-citable. Exclude children with a usable resolved edge or evaluate the child's effective edge rather than treating every unresolved alternate as requiring a child-local read.
AGENTS.md reference: AGENTS.md:L100-L112
Useful? React with 👍 / 👎.
| anonymous_flag = getattr(os, "O_TMPFILE", 0) | ||
| if not anonymous_flag: | ||
| raise MigrationError("missing-tier initialization requires anonymous-file publication support") |
There was a problem hiding this comment.
On the documented macOS installation path, os.O_TMPFILE is unavailable, so migrate-tier --initialize-missing always fails here; Linux filesystems without O_TMPFILE support fail at the following os.open as well. Because a missing durable tier blocks the new rebuild currency gate and this command is the documented recovery, affected installations have no way to initialize the tier and proceed with a rebuild. Provide a portable atomic no-replace publication path instead of making the recovery Linux-filesystem-specific.
Useful? React with 👍 / 👎.
| if initialize_missing: | ||
| initialized_version = initialize_missing_durable_tier(path, archive_tier) | ||
| initialized = True |
There was a problem hiding this comment.
If source.db, user.db, or an already-adopted audit.db is absent because it was deleted, misplaced, or corrupted, this generic branch silently creates a current but empty replacement without requiring a restore or explicit destructive consent. The new currency preflight will then report the tier as healthy, masking loss of raw source evidence, user assertions, or audit history and allowing normal operation to continue against the empty tier. Restrict this path to a tier proven never to have been adopted by the archive, and require restoration for established durable tiers.
AGENTS.md reference: AGENTS.md:L114-L122
Useful? React with 👍 / 👎.
| if receipt.status == "empty-source": | ||
| click.echo("No source.db raw_sessions rows found.") | ||
| return |
There was a problem hiding this comment.
On a freshly initialized archive with zero raw_sessions and no POLYLOGUE_SCHEMA_INFERENCE_RECEIPT, the command now enters rebuild_index_from_source_sync, whose provenance validation fails before it reaches the empty-source receipt rendered here. Previously this harmless invocation returned empty-source without requiring a schema-inference receipt; the new regression test only passes because it explicitly installs one. Run the durable-schema currency guard and raw-row count before provenance validation so an empty archive can still report that there is nothing to rebuild.
Useful? React with 👍 / 👎.
1 participant
Summary
This follow-up combines the schema-currency and topology-proof lineages from PRs #3856 and #3850, then repairs the remaining RW acceptance gaps. Rebuild admission now checks every durable tier at each ownership-sensitive boundary, missing durable tiers have an atomic recovery path, topology quarantine preserves child content, and proof receipts bind a stable SQLite snapshot. Ref #3850 and #3856.
Problem
The predecessor branches had concrete execution blockers. Their fixtures omitted the audit tier, the recovery runbook omitted audit initialization, daemon bulk could consume a source page after its last durable-currency check, and local empty-source output assumed replay counters that are absent by design. Cycle quarantine could also run after prefix extraction, so a rejected parent could silently remove copied child content. Finally, the topology census hashed files before proving which SQLite snapshot it read and accepted several ambiguous or fabricated topology evidence shapes.
Solution
The schema lane now enumerates every canonical durable migration tier, rechecks currency after archive ownership and again before daemon page selection, preserves the empty-source CLI receipt, and documents source, user, and audit recovery.
migrate-tier --initialize-missingconstructs the canonical fresh database in memory, copies it directly into an anonymous inode, fsyncs it, and publishes it with atomic no-replace semantics. Existing or concurrently created targets remain untouched.The topology lane classifies cycles before prefix-tail extraction and preserves the full child transcript for quarantined or over-budget parent walks. Stored cycle evidence must close through the asserted edge and match the current projection. Unresolved and quarantined readers remain child-local. The census groups unresolved samples by complete edge identity, distinguishes walk exhaustion from cycle proof, and rejects stale or contradictory projections without penalizing a valid alternate resolved edge.
The receipt route begins a read transaction and establishes its snapshot before the baseline file hash. A second observer checks
data_versionfor concurrent WAL commits, while before and after chunked hashes bindindex.dband its sidecars without whole-file allocation.Five independent adversarial passes reviewed the complete branch. Four produced repairs. The final pass raised only the intentionally empty replay mapping, which the exact PR #3856 review permits the implemented status branch to handle and which the production CLI regression covers.
Acceptance criteria
data_versioncheck, chunked database and sidecar hashes, quiescent and concurrent WAL tests.polylogue-live-operation-receiptsremains open for the approved live census.Verification
devtools test tests/unit/cli/test_archive_maintenance_cli.py -k 'migrate_tier_cli or rebuild_index_preflight or rebuild_index_empty_source_still or rebuild_index_rejects_daemon_schema_preflight': 12 passed, 55 deselected.devtools test tests/unit/cli/test_archive_maintenance_cli.py -k 'migrate_tier_cli_initializes_only_an_absent_durable_tier or migrate_tier_cli_missing_initialization_refuses_an_existing_tier or migrate_tier_cli_missing_initialization_loses_publish_race_without_replacement or migrate_tier_cli_exposes_no_named_staging_inode_before_publication': 4 passed, 66 deselected.devtools test tests/unit/daemon/test_bulk_rebuild_ownership.py -k 'rejects_schema_mismatch_before_transaction_bookkeeping or rechecks_schema_currency_after_ownership': 2 passed, 2 deselected.devtools test tests/unit/daemon/test_bulk_rebuild.py -k daemon_bulk_pass_rechecks_schema_currency_in_page_selection_hold: 1 passed, 8 deselected.devtools test tests/unit/maintenance/test_rebuild_status.py -k falls_back_to_the_daemon_well_known_operation_id_by_default: 1 passed, 5 deselected.devtools test tests/unit/storage/test_topology_cycle_quarantine_live.py tests/unit/devtools/test_lineage_validation.py: 20 passed at6e2996668.devtools test tests/unit/devtools/test_lineage_validation.py: 16 passed after the final multi-edge repair.devtools verify --quick: 24 steps, exit 0 in 105.41 seconds atd34e663cb(20260806T111901Z-quick-119573-0540dbf0).devtools verify --seed-testmon --skip-slow: static and policy steps passed, but the inherited full-suite seed ended with 258 failures, 19,631 passes, 3 skips, and 8 errors across more than 100 files. None of the acceptance files failed. The seed remains incomplete, so the default affected selector could not run.Live receipt still required
No production database was opened or mutated. After merge and deployment,
polylogue-live-operation-receiptsmust run the read-only topology census against the exact active or approved candidate index generation and retain its snapshot-bound receipt.polylogue-a7gmkmust also verify the shipped package and migrate every durable tier to current before the managed rebuild. This PR does not assignpolylogue-9qnzybecause its AC explicitly requires no code changes under that bead.