fix(maintenance): gate reindex on durable schema currency (#3851) - #3851
fix(maintenance): gate reindex on durable schema currency (#3851)#3851Sinity wants to merge 1 commit 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>
|
Warning Review limit reached
Next review available in: 57 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 (5)
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: f2ae88e2d9
ℹ️ 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".
| log_mapped_bytes_budget_check(logger, check_mapped_bytes_budget_against_cgroup_limit()) | ||
| validate_rebuild_index_request(request) | ||
| root = request.archive_root | ||
| require_rebuild_schema_currency(root) |
There was a problem hiding this comment.
When a newer package migrates source.db or user.db after this probe succeeds but before this process acquires OwnedArchiveLocation, the rebuild continues using the now-mismatched durable schema. This is possible because, unlike the provenance receipt, schema currency is never revalidated under ownership; a migration that only changes schema/version also is not guaranteed to affect the later source-evidence snapshot. Repeat this check immediately after ownership acquisition so the schema cannot change between validation and replay.
Useful? React with 👍 / 👎.
| for tier in (ArchiveTier.SOURCE, ArchiveTier.USER): | ||
| probe = probe_archive_tier(tier, root / f"{tier.value}.db") |
There was a problem hiding this comment.
The existing _init_empty_source() helper in tests/unit/maintenance/test_rebuild_index_ownership.py creates only source.db, but six tests using it invoke the rebuild and expect ownership, liveness, or successful empty-source behavior. This loop now classifies the absent user.db as blocking first, so those tests instead raise RebuildSchemaCurrencyError; update the fixture to create the user tier, or preserve source-only rebuild support by not treating a missing user tier as a version mismatch. The commit's two -k verification commands did not cover these affected cases.
AGENTS.md reference: AGENTS.md:L325-L328
Useful? React with 👍 / 👎.
| log_mapped_bytes_budget_check(logger, check_mapped_bytes_budget_against_cgroup_limit()) | ||
| validate_rebuild_index_request(request) | ||
| root = request.archive_root | ||
| require_rebuild_schema_currency(root) |
There was a problem hiding this comment.
For local CLI execution when source.db.raw_sessions is empty, _rebuild_index.py returns an empty-source success before calling rebuild_index_from_source_sync, so this newly added guard never runs. The same v28 fixture used by the new ownership test therefore remains accepted through the actual polylogue ops maintenance rebuild-index command whenever it has no raw rows, making the surface disagree with the shared implementation and the documented execution guarantee; invoke the shared preflight before the CLI's raw-count shortcut.
AGENTS.md reference: AGENTS.md:L37-L40
Useful? React with 👍 / 👎.
| if preflight: | ||
| from polylogue.maintenance.rebuild_index import rebuild_schema_currency_preflight | ||
|
|
||
| payload = rebuild_schema_currency_preflight(root) |
There was a problem hiding this comment.
When --preflight is combined with --daemon, this branch silently ignores both use_daemon and daemon_url and probes the client's local archive_root() instead. If the CLI is controlling a daemon with a different archive root, it can report ready for the wrong databases—or block because of an unrelated local archive—while the subsequent daemon rebuild targets another root. Either reject this option combination like --daemon --plan, or expose and call a daemon-side preflight endpoint.
Useful? React with 👍 / 👎.
| for tier in (ArchiveTier.SOURCE, ArchiveTier.USER): | ||
| probe = probe_archive_tier(tier, root / f"{tier.value}.db") |
There was a problem hiding this comment.
The canonical durable migration set already includes ArchiveTier.AUDIT (storage/sqlite/migration_runner.py:34), and the archive's tier-schema verifier treats a missing or mismatched audit.db as an error (maintenance/archive_verification.py:379-405). Because this loop checks only source and user, the new diagnostic can nevertheless report ready while a required durable tier is absent or belongs to another package schema, contradicting its durable-currency result and the runbook's requirement to clear durable-tier mismatches. Derive these checks from the canonical durable-tier set, excluding only tiers for which rebuild currency is intentionally irrelevant.
Useful? React with 👍 / 👎.
|
Superseded by the stacked hardening PR #3856, which carries this implementation plus the later audit, post-lock, daemon-bulk, empty-source, and HTTP-diagnostic fixes. The durable migration and live transition remain tracked by the named Beads successors. |
1 participant
Summary
Add a durable schema-currency preflight to the managed index rebuild route and expose the same read-only diagnostic through
rebuild-index --preflight.Problem
The live source tier reached v28 while the installed package expected v24 and the current package expects v29. The rebuild operation previously accepted a source tier whose durable schema did not match the parsing package. Source v28 is a valid predecessor of v29, but it must be migrated with the selected v29 package before that package rebuilds index.db.
Solution
The shared rebuild implementation probes source.db and user.db before provenance receipt validation, archive ownership, lease acquisition, or candidate creation. index.db is deliberately exempt because replacing it is the rebuild operation's purpose. The CLI preflight emits structured JSON, and the maintenance guide records the ordered migration, deployment, preflight, and managed rebuild sequence.
Verification
direnv exec . devtools test tests/unit/maintenance/test_rebuild_index_ownership.py -k schema_behind_runtime:1 passed, 6 deselected.direnv exec . devtools test tests/unit/cli/test_archive_maintenance_cli.py -k durable_schema_currency:1 passed, 62 deselected.direnv exec . devtools render all: completed without generated changes.Acceptance criteria
f2ae88e2d918510cf1fcfff4afb5c69680d9eb27; the whole bead remains partial.Remaining live migration, deployment, and managed rebuild work is carried by
polylogue-a7gmk.