fix(maintenance): harden durable schema currency gate (#3856) - #3856
fix(maintenance): harden durable schema currency gate (#3856)#3856Sinity wants to merge 3 commits into
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>
|
Warning Review limit reached
Next review available in: 2 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 (9)
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 |
Sinity
changed the title
![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: 38d02ade5c
ℹ️ 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".
| """ | ||
| from polylogue.maintenance.rebuild_index import require_rebuild_schema_currency | ||
|
|
||
| require_rebuild_schema_currency(root) |
There was a problem hiding this comment.
This unconditional gate breaks tests/unit/maintenance/test_rebuild_status.py::test_falls_back_to_the_daemon_well_known_operation_id_by_default: its _init_empty_source() creates only source.db, so the new audit/user probes report missing and raise RebuildSchemaCurrencyError before the expected daemon transaction is created. The commit updates similar fixtures but misses this existing caller, leaving the affected unit suite failing.
Useful? React with 👍 / 👎.
| `migrate-tier source` and `migrate-tier user` when the target package requires | ||
| them, then deploy that exact package. Run the preflight above and require a |
There was a problem hiding this comment.
When audit.db is the tier reported as mismatched, following this recovery procedure and migrating only source/user leaves audit unchanged, so the documented preflight remains blocked and the rebuild still cannot run. Since the preceding paragraph explicitly makes audit part of the gate and migrate-tier accepts every canonical durable tier, the recovery sequence must also direct operators to migrate audit when required.
Useful? React with 👍 / 👎.
| """ | ||
| from polylogue.maintenance.rebuild_index import require_rebuild_schema_currency | ||
|
|
||
| require_rebuild_schema_currency(root) |
There was a problem hiding this comment.
If this call waits for an existing archive owner and that owner advances a durable schema before releasing the lock, this pre-lock check observes the old version, but the function acquires ownership and proceeds to transaction resolution without checking currency again. The post-lock provenance validation does not compare the live user/audit versions, so this can create or resume daemon generation bookkeeping under a package that no longer owns the durable schemas; repeat the currency check immediately after assert_owns_archive_location, as the local rebuild path already does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in follow-up PR #3858. Commit 61ba5bf rechecks durable currency immediately after assert_owns_archive_location; commit b28a8f8 adds the later check before page selection and source-row consumption. The focused ownership tests passed with 2 passed and 2 deselected, and the page-selection race passed with 1 passed and 8 deselected.
Problem The rebuild schema-currency gate checks source, user, and audit tiers, while operator text still described only source and user. What changed Align the maintenance recovery instructions and rebuild preflight help with the canonical durable-tier set. Compatibility Documentation and CLI help only. The maintenance behavior is unchanged. Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ac4749772
ℹ️ 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".
| click.echo("No source.db raw_sessions rows found.") | ||
| return | ||
| if plan_only: | ||
| raw_count = _count_source_raw_sessions(root) |
There was a problem hiding this comment.
When a schema-current archive contains no raw rows, the local non-plan path now reaches rebuild_index_from_source_sync, which returns an empty-source receipt with an empty replay mapping. Its serialized payload therefore lacks classified_full_count, replayed_logical_source_count, and quarantined_raw_count, so the default plain-output formatting below raises KeyError instead of reporting the empty archive. Preserve an empty-source branch after the currency guard or render this receipt status without assuming replay counters.
Useful? React with 👍 / 👎.
## Summary Publish the durable Bead record that gives the schema-currency implementation its own execution authority. The code and tests remain in PR #3856; this PR carries only the tracker graph update. ## Problem Polylogue-9qnzy is the live migration and deployment sequence and explicitly excludes code changes. Without a separate implementation Bead, PR #3856 could not carry a truthful scope record or bind its implementation to the graph. ## Solution Add polylogue-dudtn with the reviewed schema-currency implementation scope and make it a blocking prerequisite of polylogue-9qnzy. The Bead records the seven implementation acceptance criteria and leaves live migration, deployment, and operation receipts in their existing Beads. ## Verification The exported `.beads/issues.jsonl` contains the new Bead and the durable blocking edge. The versioned PR-scope carrier below validates against the exact head and committed Bead snapshot. No product code or production data changes are included. Ref polylogue-dudtn. Context: polylogue-9qnzy. <!-- polylogue-pr-scope:v1 { "assigned_beads": [ "polylogue-dudtn" ], "beads_digest": "927c820cb198b222d1d5fb169c7027d81567e9223f2563d6facb4a93913daa95", "dispositions": [ { "bead_id": "polylogue-dudtn", "disposition": "partial", "evidence": [ { "kind": "commit", "ref": "c9d12abcf" }, { "kind": "receipt", "ref": "tracker publication only; implementation is carried by PR #3856" } ], "successors": [ "polylogue-9qnzy" ] } ], "head_sha": "66b1bf3d7cecbbf04b5b886a41a3d82d8c7c436c", "scope_digest": "3213a7c3f8ddcc65fac807b2723cf8749f9331c8efdc6bdf197ad07e24e7ecd7", "version": 1 } -->
## Summary Publish the durable Bead record that gives the schema-currency implementation its own execution authority. The code and tests remain in PR #3856; this PR carries only the tracker graph update. ## Problem Polylogue-9qnzy is the live migration and deployment sequence and explicitly excludes code changes. Without a separate implementation Bead, PR #3856 could not carry a truthful scope record or bind its implementation to the graph. ## Solution Add polylogue-dudtn with the reviewed schema-currency implementation scope and make it a blocking prerequisite of polylogue-9qnzy. The Bead records the seven implementation acceptance criteria and leaves live migration, deployment, and operation receipts in their existing Beads. ## Verification The exported `.beads/issues.jsonl` contains the new Bead and the durable blocking edge. The versioned PR-scope carrier below validates against the exact head and committed Bead snapshot. No product code or production data changes are included. Ref polylogue-dudtn. Context: polylogue-9qnzy. <!-- polylogue-pr-scope:v1 { "assigned_beads": [ "polylogue-dudtn" ], "beads_digest": "927c820cb198b222d1d5fb169c7027d81567e9223f2563d6facb4a93913daa95", "dispositions": [ { "bead_id": "polylogue-dudtn", "disposition": "partial", "evidence": [ { "kind": "commit", "ref": "c9d12abcf" }, { "kind": "receipt", "ref": "tracker publication only; implementation is carried by PR #3856" } ], "successors": [ "polylogue-9qnzy" ] } ], "head_sha": "66b1bf3d7cecbbf04b5b886a41a3d82d8c7c436c", "scope_digest": "3213a7c3f8ddcc65fac807b2723cf8749f9331c8efdc6bdf197ad07e24e7ecd7", "version": 1 } -->
## 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-missing` constructs 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_version` for concurrent WAL commits, while before and after chunked hashes bind `index.db` and 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 | ID | Criterion | Result | Evidence or residue | | --- | --- | --- | --- | | S1 | Gate every canonical durable tier before receipt consumption or candidate creation. | Satisfied | Early local guard, post-ownership local guard, daemon transaction guard, daemon page-selection guard, HTTP guard, and all-tier fixtures. | | S2 | Recheck currency after archive ownership. | Satisfied | Local and daemon ownership-race tests inject a mismatch after ownership and prove refusal before mutation. | | S3 | Preserve empty-source CLI behavior and include audit recovery. | Satisfied | Exact plain-output regression and maintenance commands for source, user, and audit. | | S4 | Initialize only a missing durable tier without exposing replaceable staged bytes. | Satisfied | Canonical in-memory image, anonymous inode, fsync, atomic no-replace link, identity check, directory fsync, and concurrent-creator tests. | | S5 | Avoid production database mutation in this lane. | Satisfied | All runtime tests use temporary archives. No live command was run. | | T1 | Preserve copied child content when a proposed parent is quarantined. | Satisfied | Production writer and sync plus async reader regression; removing the pre-slice classification loses the asserted transcript. | | T2 | Distinguish proven cycles from bounded-walk exhaustion. | Satisfied | Typed writer outcome, projection-backed evidence validation, and a 1,024-hop acyclic fixture. | | T3 | Keep unresolved and quarantined composition child-local. | Satisfied for temporary archives | Production readers plus same-child multi-edge census coverage. Live sampling remains unobserved. | | T4 | Bind topology evidence to the exact stable read snapshot. | Satisfied for temporary WAL fixtures | Snapshot established before hashing, observer `data_version` check, chunked database and sidecar hashes, quiescent and concurrent WAL tests. | | T5 | Record exact live topology evidence before claiming operational completion. | Partial | `polylogue-live-operation-receipts` remains 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 at `6e2996668`. - `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 at `d34e663cb` (`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-receipts` must run the read-only topology census against the exact active or approved candidate index generation and retain its snapshot-bound receipt. `polylogue-a7gmk` must also verify the shipped package and migrate every durable tier to current before the managed rebuild. This PR does not assign `polylogue-9qnzy` because its AC explicitly requires no code changes under that bead. <!-- polylogue-pr-scope:v1 { "assigned_beads": [ "polylogue-topology-live-proof" ], "beads_digest": "ac5775b4d1215a3b38f0070c4e694bef389803aacd4a4d355f4d8173d3ce9b06", "dispositions": [ { "bead_id": "polylogue-topology-live-proof", "disposition": "partial", "evidence": [ { "kind": "commit", "ref": "d34e663cbfb1cd7eb628ab51f48f6574e5c90b1b" }, { "kind": "test", "ref": "tests/unit/devtools/test_lineage_validation.py and tests/unit/storage/test_topology_cycle_quarantine_live.py" }, { "kind": "command", "ref": "devtools verify --quick: 24 steps, exit 0, run 20260806T111901Z-quick-119573-0540dbf0" }, { "kind": "receipt", "ref": "docs/evidence/polylogue-topology-live-proof-2026-08-06.md: synthetic candidate only; live receipt explicitly unobserved" } ], "successors": [ "polylogue-live-operation-receipts" ] } ], "head_sha": "d34e663cbfb1cd7eb628ab51f48f6574e5c90b1b", "scope_digest": "80257ffd894ba2700bce1c520f1e5465e95cef06376bba24ee9039635fe9867b", "version": 1 } --> --------- Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Claude <noreply@anthropic.com>
|
This branch is fully represented in merged PR #3858. The ancestry comparison shows all three #3856 commits reproduced there, plus the durable-tier, ownership, empty-source, daemon, and recovery follow-up repairs. The implementation scope is therefore present in the merged train; live migration remains separately tracked. This stale duplicate PR is being removed from the active frontier. |
1 participant
Summary
Harden the rebuild durable-schema currency boundary across local rebuild, daemon bulk transaction, CLI, and daemon HTTP paths. This PR implements the code slice tracked by polylogue-dudtn and leaves live migration and deployment under polylogue-9qnzy and polylogue-a7gmk.
Problem
The initial gate checked only source and user tiers. Audit could be missing or mismatched, schema could drift after archive ownership acquisition, daemon bulk bookkeeping could start without the check, and the empty-source CLI path returned before validation. The daemon also emitted a generic HTTP 500 instead of the preflight diagnostic.
Solution
Use the canonical
DURABLE_MIGRATION_TIERSset, recheck after ownership acquisition, guard daemon bulk setup before bookkeeping, remove the empty-source bypass, reject--preflight --daemon, return a structured HTTP 409 diagnostic, and document migration of audit.db. Add file-backed regressions for every path and update the operator fixture setup.Verification
Eight focused schema-gate regression tests passed in 1.71s.
devtools verify --quickpassed all 24 steps twice, including the pre-push run. No production migration, reset, rebuild, daemon restart, or archive mutation was performed.Acceptance criteria
DURABLE_MIGRATION_TIERSprobeRef polylogue-dudtn. Context: polylogue-9qnzy.