fix(maintenance): reconcile scoped cursor authority - #3867
Conversation
Problem: live ingestion fails closed when a cursor is ahead of accepted byte authority, leaving no scoped recovery route. What changed: add digest-bound dry-run planning and full-evidence backup validation; route apply through LiveBatchProcessor's normal full-ingest path under a context-local single-use path and frontier authorization; emit atomic plan and receipt evidence without direct cursor or accepted-head writes. Compatibility/migration: dry-run is default and resolves only /realm/db/polylogue; apply requires the daemon stopped, an immutable plan, a verified full_evidence backup, and a new receipt. Co-Authored-By: Claude <noreply@anthropic.com>
Problem: the scoped reconciliation route rejected the real incomparable cursor population and did not model the typed deferred outcome. What changed: preserve incomparable rows while binding the one true ahead row, make plans immutable, validate the unchanged gap census, and expand behavior tests for the scoped authorization and backup boundary. Compatibility/migration: the command remains dry-run by default and production apply remains backup-gated. Ref polylogue-cursor-authority-reconcile-implementation Co-Authored-By: Claude <noreply@anthropic.com>
Problem: cursor-authority reconciliation trusted racy file observations, fixed receipt counts, and incomplete recovery and backup evidence.\n\nWhat changed: make source and tier observations descriptor-safe, scope recovery to later completed attempts, validate typed frontier and backup data, publish refuse-existing receipts atomically, and record observed, deferred, and failed outcomes with redacted projections. Extend the focused maintenance, CLI, and retention tests.\n\nCompatibility/migration: the route remains daemon-stopped, backup-gated, and limited to the normal scoped live-ingest path.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Problem: post-ingest receipt construction could fail while reporting the original reconciliation failure, and recovery variables shadowed the normal post-state.\n\nWhat changed: make failure receipts tolerant of unavailable post-state evidence, label recovery observations explicitly, and tighten typed test assertions.\n\nCompatibility/migration: no archive schema or mutation route changes.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
Problem: cursor-authority reconciliation could observe recovery outside archive ownership, fingerprint only SQLite main files, accept untyped deferrals, and persist private path and identity material.\n\nWhat changed: bind the scoped authorization to the exact plan and path, route its one-use maintenance flow through full ingest, resolve the active index generation, attest durable backups, hash effective SQLite images, require healthy sibling projections, bind recovery attempts to planning events, and redact receipts and metrics.\n\nCompatibility/migration: ordinary live ingestion remains unchanged and no direct cursor or accepted-head writes are introduced.
Problem: the scoped authorization return path used an implicit empty return and token truthiness, which strict mypy rejected.\n\nWhat changed: return the optional authorization explicitly and pass the consumed typed token directly; tighten the backup validation test's result narrowing.\n\nCompatibility/migration: runtime routing and ordinary ingestion semantics are unchanged.
|
Warning Review limit reached
Next review available in: 10 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 |
![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: 6588c6c56f
ℹ️ 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".
| for key in ("size_bytes", "sha256", "user_version"): | ||
| if artifact.get(key) != expected_tier.get(key): | ||
| raise CursorAuthorityReconciliationError(f"backup {tier} fingerprint does not match the plan") |
There was a problem hiding this comment.
A real full_evidence backup cannot reliably pass this check: polylogue/daemon/backup.py:_sqlite_source_fingerprint records the SHA-256 of the checkpointed live database file, whereas the plan's tier_fingerprints come from _sqlite_snapshot, which hashes a database produced through SQLite's backup API. Those byte images commonly have different SQLite header values even when their logical contents are identical, so a legitimate freshly verified backup is rejected here before the subsequent snapshot-to-snapshot comparison can validate it.
Useful? React with 👍 / 👎.
| planning_bindings[str(attempt_id)] = ( | ||
| payload.get("cursor_authority_plan_digest") == plan_digest | ||
| and payload.get("cursor_authority_path_digest") == path_digest | ||
| ) |
There was a problem hiding this comment.
During a real authorized ingest, begin_ingest_attempt first records an unbound planning event and the new _record_attempt_progress call then records a bound one. Because events are read newest-first but every row overwrites planning_bindings[attempt_id], an older unbound event processed later changes a valid True back to False whenever the two events have distinct timestamps. _normal_ingest then fails to find the completed recovery attempt after it has already mutated the archive, producing a failed receipt instead of the required reconciliation evidence.
Useful? React with 👍 / 👎.
| return raw_frontier_integrity_projection( | ||
| root, | ||
| raw_materialization_readiness_snapshot(root), | ||
| sample_limit=100, | ||
| ) |
There was a problem hiding this comment.
When .index-active-pointer selects a promoted generation, this helper still delegates to storage.raw_retention.raw_frontier_integrity_projection, which opens archive_root/index.db rather than ArchiveLocation.resolve(root).active_index_path. Planning and post-ingest validation therefore inspect the stale shadow index while _normal_ingest writes the active generation; a successful reconciliation can still appear cursor-ahead and be reported as failed, or planning can be rejected entirely when only the active generation exists.
Useful? React with 👍 / 👎.
| if not (root / "blob").is_dir() or not (root / "blob-inventory.json").is_file(): | ||
| raise CursorAuthorityReconciliationError("backup lacks blob rollback evidence") |
There was a problem hiding this comment.
If a verified backup's blob or blob-inventory.json is modified or truncated after verification-receipt.json was issued, validation still accepts it because this only checks that the paths exist. verify_verification_receipt authenticates the old receipt but does not compare the current artifacts with the receipt's signed artifact_inventory, blobs, or blob_inventory_root_sha256; only the SQLite tiers are subsequently rehashed. The command can therefore perform the repair with backup evidence that no longer contains the blob data needed for rollback.
Useful? React with 👍 / 👎.
| def _build_plan(root: Path, source_path: Path, *, require_candidate: bool = True) -> dict[str, object]: | ||
| tiers = _tier_snapshots(root) | ||
| projection = _projection_for(root) |
There was a problem hiding this comment.
When any required tier has pre-existing corruption that still permits SQLite backup and the specific projection queries to run, _sqlite_snapshot records a non-ok quick_check result but _build_plan accepts it without validation. The repair can consequently write to an already-corrupt source, index, ops, or audit database and only discover the condition in the post-ingest _quick_checks, after mutation has occurred. Planning should fail when any captured tier snapshot does not report exactly ['ok'].
Useful? React with 👍 / 👎.
| if receipt.exists(): | ||
| raise CursorAuthorityReconciliationError(f"output path already exists: {receipt}") |
There was a problem hiding this comment.
If --receipt names an unwritable parent, a path whose parent cannot be created, or a filesystem without enough space, the only preflight is this existence check. The normal ingest then mutates the archive before _write_atomic_json is first attempted at the end; that write can fail outside the ingest exception handler, leaving a completed reconciliation with no success or failure receipt. The destination should be reserved or otherwise proven writable before entering _normal_ingest so ordinary path and permission errors fail before archive changes.
Useful? React with 👍 / 👎.
|
This branch is superseded by merged PR #3860. Its commits are an older prefix of the merged cursor-authority implementation, so it has no unique remaining scope. |
|
Closed as superseded by merged PR #3860; no unique implementation remains on this branch. |
1 participant
Summary
Add the scoped cursor-authority reconciliation path that lets an operator authorize one exact source frontier for live ingestion while keeping ordinary live routes fail-closed. The change carries typed receipts through maintenance and full ingest, preserves failed reconciliation evidence, and protects the active-index pointer boundary.
Problem
A cursor or source frontier can be ahead of the durable raw authority, and the live watcher must not guess whether the source or index is authoritative. Before this change, the recovery decision was not represented as an exact, consumable receipt across the maintenance and live-ingest paths.
Solution
Add the cursor-authority reconciliation service and maintenance command, thread typed authorization through live batch selection and full-ingest routing, retain typed failure receipts, and assert that scoped authorization is consumed only for the selected source path and frontier. The active-index test keeps writes on the selected generation and leaves the shadow index unchanged.
This PR is implementation-complete and live-proof-pending. It does not apply a reconciliation plan to the production archive.
Verification
direnv exec /realm/worktrees/polylogue-cursor-authority-reconcile zsh -c 'cd /realm/worktrees/polylogue-cursor-authority-reconcile && PATH="$PWD/.venv/bin:$PATH" devtools test tests/unit/maintenance/test_cursor_authority_reconcile.py tests/unit/sources/test_live_watcher.py tests/unit/cli/test_archive_maintenance_cli.py tests/unit/storage/test_raw_retention.py': 273 passed; 4 live-watcher failures reproduced on the cleanorigin/masterworktree as well.direnv exec /realm/worktrees/polylogue-cursor-authority-reconcile zsh -c 'cd /realm/worktrees/polylogue-cursor-authority-reconcile && PATH="$PWD/.venv/bin:$PATH" devtools verify --quick': all 24 steps passed.test_live_full_ingest_preserves_complete_workflow_journal_revisions,test_active_index_pointer_keeps_shadow_index_unmodified,test_live_full_ingest_over_ambiguous_membership_preserves_durable_debt_without_retry, andtest_live_append_atof_shared_file_multi_session_boundary_retains_all_events.