fix(audit): restore adopted durable tiers - #3947
Conversation
📝 WalkthroughWalkthroughThe change adds verified backup validation, authenticated audit-tier adoption and restoration, cross-database continuity coordination, archive-bound mutation execution, startup reconciliation, migration metadata, and extensive integrity and recovery coverage. ChangesAudit continuity and archive mutation control
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant migrate_tier_command
participant BackupValidator
participant AuditOperations
participant StartupBootstrap
Operator->>migrate_tier_command: select audit adoption or restoration
migrate_tier_command->>BackupValidator: validate full-evidence backup manifest
BackupValidator-->>migrate_tier_command: return validated evidence
migrate_tier_command->>AuditOperations: publish adoption or restoration receipt
AuditOperations-->>migrate_tier_command: return audit state and receipt path
StartupBootstrap->>AuditOperations: validate receipt and reconcile continuity
AuditOperations-->>StartupBootstrap: return reconciled audit state
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/storage/test_durable_change_train.py (1)
1672-1723: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winThe new test absorbed the body of the preceding test.
Lines 1722-1723 are unchanged lines that belonged to
test_fresh_archive_bootstrap_receipt_allows_repeat_startup. The new test is inserted above them, so two effects follow:
test_fresh_archive_bootstrap_receipt_allows_repeat_startupnow ends at Line 1675. It callsinitialize_active_archive_root(tmp_path)once and asserts nothing. It no longer tests repeat startup.test_audit_adoption_receipt_survives_startup_preflightnow ends withreconcile_durable_change_train_startup(tmp_path)andinitialize_active_archive_root(tmp_path). Both operate ontmp_path, not onarchive_root, and both run after the tampered-receipt assertion. Neither statement belongs to audit adoption.Move Lines 1722-1723 back into the preceding test.
🐛 Proposed fix
def test_fresh_archive_bootstrap_receipt_allows_repeat_startup(tmp_path: Path) -> None: from polylogue.storage.sqlite.archive_tiers.bootstrap import initialize_active_archive_root initialize_active_archive_root(tmp_path) + assert reconcile_durable_change_train_startup(tmp_path) == () + initialize_active_archive_root(tmp_path) def test_audit_adoption_receipt_survives_startup_preflight(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:assert reconcile_durable_change_train_startup(archive_root) == () receipt.write_text("tampered", encoding="utf-8") with pytest.raises(MigrationError, match="invalid audit adoption receipt"): reconcile_durable_change_train_startup(archive_root) - assert reconcile_durable_change_train_startup(tmp_path) == () - initialize_active_archive_root(tmp_path)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/storage/test_durable_change_train.py` around lines 1672 - 1723, Restore the repeat-startup assertion to test_fresh_archive_bootstrap_receipt_allows_repeat_startup by calling initialize_active_archive_root(tmp_path) a second time there. Remove the trailing initialize_active_archive_root(tmp_path) from test_audit_adoption_receipt_survives_startup_preflight, leaving that test focused on archive_root receipt reconciliation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@polylogue/operations/durable_change_train.py`:
- Around line 448-454: Explicitly close both SQLite connections used by
_audit_schema_inventory_sha256 and the nearby read-only validation block instead
of relying on sqlite3.Connection context-manager scope; wrap each connection
with contextlib.closing or close it in a finally block, preserving the existing
transaction behavior and validation logic.
- Around line 441-445: Remove the local _canonical_json_sha256 definition and
import the existing shared helper from its canonical module. Update callers in
this module, including _write_immutable_audit_adoption_receipt, to reuse that
imported symbol while preserving the current receipt checksum behavior.
- Around line 588-622: Update adopt_missing_audit_tier and its receipt
validation flow so an immutable adoption receipt can represent a pending
publication when audit.db is absent, then resume or complete publication safely
on startup instead of treating the receipt as terminal. Preserve immutability by
allowing only the intended recovery transition, and add coverage for
interruption between _write_immutable_audit_adoption_receipt and os.link
followed by successful startup recovery.
In `@polylogue/storage/sqlite/migration_runner.py`:
- Around line 897-904: Update the retained-artifact validation around
_validated_receipt_artifacts to compare each retained tier’s artifact path with
its corresponding live tier path using samefile, including when backup_root
equals archive_root. Reject any artifact that aliases the live archive before
accepting it as backup evidence, while preserving validation for all retained
tiers.
In `@tests/unit/storage/test_durable_change_train.py`:
- Around line 1686-1687: Update the test setup around the relevant test to use
the shared workspace_env fixture for configuring POLYLOGUE_ARCHIVE_ROOT and
XDG_STATE_HOME, replacing the direct monkeypatch.setenv calls while preserving
the existing archive_root and temporary state directory values.
---
Outside diff comments:
In `@tests/unit/storage/test_durable_change_train.py`:
- Around line 1672-1723: Restore the repeat-startup assertion to
test_fresh_archive_bootstrap_receipt_allows_repeat_startup by calling
initialize_active_archive_root(tmp_path) a second time there. Remove the
trailing initialize_active_archive_root(tmp_path) from
test_audit_adoption_receipt_survives_startup_preflight, leaving that test
focused on archive_root receipt reconciliation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 816de976-d876-4753-ac2f-5a5276d49a20
📒 Files selected for processing (6)
polylogue/cli/commands/maintenance/_migrate_tier.pypolylogue/operations/durable_change_train.pypolylogue/storage/sqlite/durable_change_train.pypolylogue/storage/sqlite/migration_runner.pytests/unit/cli/test_archive_maintenance_cli.pytests/unit/storage/test_durable_change_train.py
![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: 48b52e7f12
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9c48f860f
ℹ️ 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".
b9c48f8 to
b6cf043
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6cf04322d
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a7fca2bcf
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a37de44537
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9ee6566e5
ℹ️ 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".
Sinity
changed the title
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@polylogue/cli/commands/maintenance/_migrate_tier.py`:
- Around line 70-89: The migrate_tier_command audit workflow must use the
query-first target syntax instead of accepting a bare positional tier; require
find, a quoted query expression, or field syntax before the action while
preserving existing migration options. Update docs/maintenance.md at line 731 to
replace the positional audit invocation with the corresponding query-first
command.
In `@polylogue/operations/durable_change_train.py`:
- Around line 802-808: Remove the st_nlink != 1 condition from the
restore-record validation in _audit_restore_records, while preserving the
existing regular-file, ownership, and mode checks. This allows valid records
with a leftover temporary hard-link after an interrupted
_write_immutable_audit_adoption_receipt publication to load during startup.
- Around line 705-711: Update _audit_live_metadata to percent-encode the
interpolated audit_path before constructing the SQLite file: URI, while
preserving the read-only mode=ro query parameter and existing PRAGMA validation
behavior. Apply the same safety treatment to any other directly interpolated
SQLite file: URIs identified in the surrounding implementation.
In `@polylogue/storage/sqlite/migration_runner.py`:
- Around line 992-1015: Update
validate_full_evidence_backup_for_adopted_audit_restore and its per-tier
validation flow to reject retained artifacts that resolve to the live archive
tier files, matching the samefile alias check used by the adoption loop. Ensure
the check runs before comparing fingerprints and catches backup_root equal to
archive_root, while preserving existing validation for legitimate independent
backup artifacts.
- Around line 918-929: In the audit adoption validation flow, move the
live_path.is_file() check before the artifact_path.samefile(live_path) alias
comparison so missing live tiers raise the existing “audit adoption live tier is
missing” MigrationError instead of being caught as comparison failures. Keep the
fingerprint and alias validations unchanged after confirming the live tier
exists.
In `@tests/unit/cli/test_archive_maintenance_cli.py`:
- Around line 3561-3569: Update the foreign-database setup in the
publication-failure branch to resolve destination through dst_dir_fd rather than
root / str(destination). Open the destination using the directory descriptor,
following the existing pattern near the other fake setup, then initialize it
with initialize_archive_tier and retain the ArchiveTier.AUDIT configuration and
application_id assignment.
In `@tests/unit/storage/test_durable_change_train.py`:
- Around line 1856-1896: Update
test_adopted_audit_restore_rejects_untrusted_or_stale_backup to parameterize
each tamper case with its expected MigrationError message and match that
specific message instead of the shared alternation. Ensure the stale-source
setup changes only the authority state relevant to its intended guard, avoiding
source-tier fingerprint invalidation so the test verifies the authority check
rather than artifact comparison.
- Around line 1745-1750: Replace the sqlite3.Connection context-manager usage in
the affected test setup blocks, including the shown mutation and the matching
blocks near the referenced locations, with contextlib.closing so each connection
is explicitly closed after committing. Ensure the connection is closed before
reconcile_durable_change_train_startup or other audit-tier validation runs,
while preserving the existing SQL mutations.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e9ad3263-d544-420b-8250-188b65f1354d
📒 Files selected for processing (10)
docs/maintenance.mdpolylogue/cli/commands/maintenance/_migrate_tier.pypolylogue/daemon/backup.pypolylogue/operations/durable_change_train.pypolylogue/storage/sqlite/archive_tiers/bootstrap.pypolylogue/storage/sqlite/durable_change_train.pypolylogue/storage/sqlite/migration_runner.pytests/unit/cli/test_archive_maintenance_cli.pytests/unit/daemon/test_backup.pytests/unit/storage/test_durable_change_train.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bb34200d9
ℹ️ 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".
|
Scope: I own the audit-continuity completion lane on |
5bb3420 to
f0bff90
Compare
|
@codex Please review exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0bff906e4
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dd3979f2b
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a240629a2
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 29
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/internals.md`:
- Around line 688-691: Update the durable-tiers justification sentence in the
documentation to explain why audit.db is durable, in addition to user.db’s
irreplaceable human assertions. Keep the existing migration description and
source.db context unchanged, ensuring the rationale covers all three listed
tiers.
In `@polylogue/cli/archive_query.py`:
- Around line 2143-2159: Move the bound preparation block centered on
runtime_operation_binding, OperationExecutor.for_archive_root, and
prepare_bound_for_archive below the dry_run, zero-target, and
confirmation-return gates in the command flow. Ensure dry runs, empty
session_ids, and declined confirmations exit without invoking
prepare_bound_for_archive; invoke it immediately before the mutation
authorization/execution path once the command has decided to proceed.
In `@polylogue/cli/commands/excise.py`:
- Around line 316-322: All four destructive authorization routes must preserve
the confirmation strength collected by their surfaces: in
polylogue/cli/commands/excise.py lines 316-322, pass the strength from the --yes
gate; in polylogue/api/archive.py lines 6590-6599, pass
confirmation_strength="confirm_flag" or update the delete_session_safe contract;
in polylogue/cli/archive_query.py lines 2212-2213, pass the strength from
--force or interactive confirmation; and in polylogue/cli/commands/reset.py
lines 223-229, pass the strength from reset_command’s --yes gate. Update each
authorize_bound call so durable authorization records the collected strength
rather than the actuator default.
In `@polylogue/cli/commands/maintenance/_migrate_tier.py`:
- Around line 111-132: Move the migration flag conflict validation and the
adopt/restore tier and backup-manifest checks out of the post-ownership flow and
into the existing try block before acquire_durable_archive_ownership. Preserve
the current MigrationError exceptions and messages so validation occurs before
archive ownership without changing --output-format json behavior.
- Around line 194-195: Define a dedicated Pydantic output payload model for the
migrate-tier result, including adoption_receipt and restore_receipt with
nullable string types, and use it instead of the untyped dictionary in the
migrate-tier command. Register the model in
devtools/render_cli_output_schemas.py and generate its corresponding artifact
under docs/schemas/cli-output/, ensuring the command references the typed schema
rather than generic machine-success.
In `@polylogue/cli/commands/maintenance/_raw_identity.py`:
- Around line 233-242: Update the exception handling around the binding and
execution flow in the relevant command function to include
BindingValidationError in the existing try-block except tuple, preserving
conversion of failures into click.ClickException alongside the other handled
errors.
In `@polylogue/operations/audit.py`:
- Around line 403-413: Update _connection() to commit standalone audit
transactions after successful audit operations and roll them back when an error
occurs before closing the connection. Preserve coordinated-connection behavior,
and ensure the transaction handling applies only when _coordinated_connection is
unavailable.
In `@polylogue/operations/bindings.py`:
- Around line 117-157: Replace the shared fallback in the executor-routed
binding flow with explicit target_authority policies for each legacy operation,
preserving each route’s own required capability and allowed surfaces, including
session deletion, excision, identity reset, and raw-authority recovery. Update
all remaining legacy call sites/specifications so their target authority
metadata is declared, then remove the broad legacy-runtime fallback from the
binding logic once no compatibility rows remain.
- Around line 109-114: Update runtime_operation_binding to cache the operation
lookup map produced by build_runtime_operation_catalog().by_name(), reusing the
cached map across calls instead of rebuilding the catalog for every mutation.
Preserve the existing operation-name validation and spec lookup behavior.
In `@polylogue/operations/durable_change_train.py`:
- Around line 1414-1417: Update the retry logic around the published assignment
and subsequent image validation so rebind_already_committed does not bypass
verifying audit.db against artifact_sha256 (and size where applicable). Always
compute and validate the current audit image digest, and fall back to the
artifact copy when it does not match; retain rebind_already_committed only to
prevent the duplicate seed_or_rebind call at the later continuity step.
In `@polylogue/operations/mutation_transaction.py`:
- Around line 607-630: Update prepare_bound_for_archive to reuse its single
binding.actuator.prepare(args) result instead of preparing twice. Extend the
binding path, including prepare_bound as needed, to accept and propagate the
resolved plan while preserving existing behavior for callers without one, so
parameter_digest and the execution preview are derived from the same plan.
- Around line 557-572: Remove the per-mutation audit.reconcile_continuity() call
from OperationExecutor.for_archive_root. Move continuity reconciliation to the
archive startup or recovery flow, or implement a cheap no-pending fast path in
AuditRepository so ordinary facade construction avoids opening and locking both
databases while preserving reconciliation for pending-command recovery.
- Line 820: Update compute_typed_plan_hash and its callers to include
plan.context in the hashed inputs, ensuring execute_bound’s rebuilt plans
receive distinct plan_hash values when actuator context changes while targets
remain unchanged.
In `@polylogue/storage/sqlite/archive_tiers/audit.py`:
- Around line 228-230: The genesis continuity head must have one shared
definition and parity coverage. Define and document a shared constant,
interpolate it into AUDIT_DDL at
polylogue/storage/sqlite/archive_tiers/audit.py:228-230 and SOURCE_DDL at
polylogue/storage/sqlite/archive_tiers/source.py:879-882, while retaining the
migration literal in
polylogue/storage/sqlite/migrations/audit/002_audit_continuity_head.sql:8-10 and
adding tests asserting it matches the constant; apply the same parity test to
032_audit_continuity_control.sql.
In `@polylogue/storage/sqlite/archive_tiers/bootstrap.py`:
- Around line 404-417: Extract the duplicated startup classification into one
local helper defined after has_pending_bootstrap, encapsulating both
durable_tier_exists and pre_marker_adoption rules. Replace the initial
classifications and the post-recovery recomputation with calls to that helper so
all bootstrap-marker decisions use the same logic.
- Around line 314-325: Update the archive probe around
initialize_active_archive_root to wrap the read-only sqlite connection with
contextlib.closing so it is explicitly closed, and extend the exception handling
to catch OSError from resolve(strict=True) alongside sqlite3.DatabaseError.
Preserve the existing False result for missing, invalid, or schema-incompatible
source databases.
- Around line 434-440: Remove the unconditional reconcile_continuity() call from
the per-open bootstrap path guarded by _source_has_audit_continuity_control.
Ensure continuity reconciliation runs through a one-time or consolidated
initialization mechanism instead, while preserving reconciliation before runtime
mutation writes.
In `@polylogue/storage/sqlite/audit_continuity.py`:
- Around line 198-218: Update has_committed_mutation to catch only
sqlite3.OperationalError cases indicating a missing audit-continuity table and
return False for those; propagate every other sqlite3.DatabaseError as
AuditContinuityError, matching the error-handling rule used by is_available.
Preserve the existing row validation and committed-mutation comparison behavior.
- Around line 354-387: The _abort_prepared method must hold the audit write lock
across both head validation and source cleanup. Start an IMMEDIATE transaction
on the audit database before reading audit_continuity_head, keep that
transaction open while validating the current head and clearing the prepared
command in the source database, and commit only after the source abort succeeds;
ensure all early-return and error paths release the audit transaction.
In `@polylogue/storage/sqlite/durable_change_train.py`:
- Around line 334-337: Add the module-level _is_audit_continuity_receipt(path)
helper in durable_change_train.py and replace the duplicated inline
audit-receipt predicates at lines 334-337, 370-373, 517-520, 1984-1985, and
2318-2323 with calls to it. Also apply the helper to the
manifest_root.glob(f"{tier.value}-*.json") result at durable_change_train.py
lines 69-73 so AUDIT adoption excludes continuity receipts consistently.
- Around line 334-337: Define the module-level _is_audit_continuity_receipt
helper with shared receipt-name and restore-prefix constants, then replace the
duplicated inline predicates in the discovery checks around marker_path,
including the inverted check near line 1984, with calls to this helper. Preserve
each site’s existing any/all logic while centralizing the receipt
classification.
- Around line 1466-1474: Update the AuditRepository.reconcile_continuity adapter
in the durable change-train behavior-proof path to execute against an isolated
staging archive root rather than the live archive_root, preserving the live root
from mutations while recording proofs. Keep the existing runtime_probe path
unchanged, and ensure staging setup and cleanup are handled by the surrounding
proof flow.
In `@polylogue/storage/sqlite/migration_runner.py`:
- Around line 1052-1056: Use contextlib.closing around the read-only sqlite3
connection in _validate_source_continuity_rebind_delta at
polylogue/storage/sqlite/migration_runner.py lines 1052-1056 so both live and
attached backup handles close reliably. Also wrap both source.db writer
connections in closing at tests/unit/storage/test_durable_change_train.py lines
1883-1896, ensuring each connection is released before
restore_adopted_audit_tier reads the database.
- Around line 1069-1080: Update the restore validation loop around the
live_count/backup_count checks to compare row multiplicities rather than relying
on EXCEPT set comparison. Build the GROUP BY column list from PRAGMA table_info
for each table, then compare grouped rows and their COUNT(*) values in both
main-versus-backup_source directions; do not use GROUP BY * because SQLite
rejects it. Preserve the existing MigrationError when any grouped multiplicity
differs.
In `@polylogue/storage/sqlite/migrations/source/032_audit_continuity_control.sql`:
- Around line 15-18: In the migration/runtime schema validation tests, add a
parity test that extracts every generation-0 genesis digest from both migrations
and both runtime DDL definitions, then asserts all extracted values are present
and identical. Anchor the test to the existing audit_continuity_control seed and
its corresponding source seed definitions, without changing the seed values.
In `@tests/unit/operations/test_operation_audit.py`:
- Around line 67-89: Extend
test_typed_domain_receipt_replays_after_source_prepare_crash to assert the
persisted receipt JSON in the audit record, first confirming and using the audit
schema’s receipt column name. Verify the typed batch and Pydantic outcome
serialize correctly, while the private _cached_bytes field is absent from
durable JSON; retain the existing run-status and pending-command assertions.
In `@tests/unit/storage/test_audit_continuity.py`:
- Around line 55-65: Update test_crash_before_source_prepare_leaves_no_command
to inspect durable audit state after reconcile, matching the direct-state
assertions used by the sibling tests. Assert that no command remains pending,
while preserving the existing crash expectation and reconcile call.
- Around line 123-130: Explicitly close the SQLite connections used in the audit
continuity tests instead of relying on the connection context manager. Update
both the source.db query before coordinator.execute and the audit.db query
before seed_or_rebind to use closing or an equivalent explicit close, preserving
the existing assertions and transaction scoping.
In `@tests/unit/storage/test_durable_change_train.py`:
- Around line 2174-2180: Update the swap_after_validation stub to accept
arbitrary keyword arguments and forward them to real_validate, preserving the
existing path and archive_root arguments so restore_validation_kwargs such as
allow_source_continuity_rebind are supported.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9624dca6-dd20-4c5f-a1e5-7971b86813e0
📒 Files selected for processing (38)
devtools/verify_schema_upgrade_lane.pydocs/internals.mddocs/maintenance.mdpolylogue/annotations/importer.pypolylogue/api/archive.pypolylogue/api/ingest.pypolylogue/cli/archive_query.pypolylogue/cli/commands/excise.pypolylogue/cli/commands/maintenance/_migrate_tier.pypolylogue/cli/commands/maintenance/_raw_identity.pypolylogue/cli/commands/reset.pypolylogue/maintenance/raw_authority_recovery.pypolylogue/operations/audit.pypolylogue/operations/bindings.pypolylogue/operations/durable_change_train.pypolylogue/operations/mutation_transaction.pypolylogue/operations/specs.pypolylogue/storage/sqlite/archive_tiers/audit.pypolylogue/storage/sqlite/archive_tiers/bootstrap.pypolylogue/storage/sqlite/archive_tiers/source.pypolylogue/storage/sqlite/audit_continuity.pypolylogue/storage/sqlite/durable_change_train.pypolylogue/storage/sqlite/migration_runner.pypolylogue/storage/sqlite/migrations/audit/002.train.jsonpolylogue/storage/sqlite/migrations/audit/002_audit_continuity_head.sqlpolylogue/storage/sqlite/migrations/source/032.train.jsonpolylogue/storage/sqlite/migrations/source/032_audit_continuity_control.sqltests/unit/annotations/test_importer.pytests/unit/api/test_facade_contracts.pytests/unit/api/test_operation_executor_routes.pytests/unit/cli/test_archive_maintenance_cli.pytests/unit/daemon/test_backup.pytests/unit/maintenance/test_raw_authority_reset.pytests/unit/operations/test_mutation_actuators.pytests/unit/operations/test_mutations.pytests/unit/operations/test_operation_audit.pytests/unit/storage/test_audit_continuity.pytests/unit/storage/test_durable_change_train.py
|
Exact head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b719a3677a
ℹ️ 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".
Problem: established production archives that predate audit.db cannot use the\nfresh-root-only initializer, leaving current runtimes unable to start.\n\nWhat changed: add an explicit backup-bound audit adoption route that retains\nthe existing atomic publisher, records an immutable durable-change-train\nreceipt, and validates that receipt during startup reconciliation.\n\nCompatibility: the existing --initialize-missing route remains restricted\nto unadopted archive roots.
Make the audit adoption receipt recoverable after a pre-publication interruption while preserving its initial-image proof. Validate later audit state structurally, tighten full-evidence admission, and stop ordinary bootstrap from bypassing adoption.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20d787c09e
ℹ️ 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".
|
I am addressing the five exact-head Codex findings against |
Problem: established archives could recreate a missing audit tier before\nsource v32, migrate-tier JSON errors violated their published schema, and\nexecutor construction could abandon live work.\n\nWhat changed: fail closed from durable bootstrap evidence, publish a typed\nsuccess/error result union, report the recovered audit version, track live\nprocess ownership on attempts, and defer excise executor construction until\nconfirmation.\n\nCompatibility: existing excise JSON continues to expose its durable assertion\nreceipt while the audited operation receipt remains available in plain output.
Parse Linux proc stat records after their parenthesized command field so PID reuse checks compare the actual start time. Cover a reused PID whose process name contains spaces.
There was a problem hiding this comment.
💡 Codex Review
polylogue/polylogue/operations/audit.py
Lines 804 to 807 in 283e39d
Derive run capabilities from the persisted authorization
When a valid bearer token is supplied in a reconstructed or modified MutationAuthorization, _consume_authorization verifies only its actor, surface, and plan hash, then copies the caller-provided role and capabilities into the durable run. A token holder can therefore alter those fields without changing the authorized domain effect, leaving the append-only journal falsely claiming capabilities or a role that the corresponding operation_authorizations row never granted; load these values from the persisted authorization and capability rows, or require an exact match before consuming the token.
AGENTS.md reference: AGENTS.md:L122-L122
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf44ef5f90
ℹ️ 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".
| operation_id, | ||
| 0 if preview.plan.targets else None, | ||
| str(row[0]), | ||
| self._attempt_owner_id, |
There was a problem hiding this comment.
When a process dies after preparing consume_authorization_and_start in source.db but before the audit-side insert commits, the next executor replays that command and this field is populated from the new repository's current-process owner. recover_abandoned_attempts then treats the reconstructed attempt as live even though no actuator will execute it, leaving the authorization consumed and the run permanently running for the lifetime of the recovery process. Fresh evidence beyond the process-liveness fix is that the consume continuity payload omits the original worker identity, so replay manufactures a live owner; persist the original owner or classify a replayed pre-effect attempt as recoverable rather than adopting it.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
Problem: recovery treated missing or unverifiable ownership as proof of process death, and a pre-publication migrate-tier failure could not serialize its nullable cleanup target.\n\nWhat changed: classify attempt ownership as live, dead, or unknown and recover only confirmed-dead owners; declare the recovery target nullable and validate the real CLI error against its rendered schema.\n\nCompatibility: legacy and externally-owned running attempts remain recoverable until an operator can establish their outcome.
|
@codex review Please review exact pushed head |
Sinity
changed the title
Problem: a symlinked audit tier could redirect authority outside its archive root, and confirmed lifecycle requests could bypass the established-archive audit gate.\n\nWhat changed: require a regular audit leaf before bootstrap or audit access, and bootstrap the complete archive before a confirmed mirror or primary lifecycle request.\n\nCompatibility: confirmed mirror and primary requests now fail closed when an established archive lacks audit authority.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 245e7dd380
ℹ️ 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".
| "SELECT operation_id, worker_id FROM operation_attempts WHERE state = 'running' ORDER BY operation_id" | ||
| ).fetchall() | ||
| operation_ids = tuple( | ||
| str(row[0]) for row in rows if _attempt_owner_liveness(cast(str | None, row[1])) == "dead" |
There was a problem hiding this comment.
When restart reconciliation replays a pending consume_authorization_and_start, _consume_authorization still records the newly constructed repository's current-process owner, and this dead-only filter consequently treats that reconstructed attempt as live. No actuator resumes after the replay, so the authorization and run remain consumed/running until that recovery process exits—potentially indefinitely in a daemon. Fresh evidence beyond the earlier thread is that this head changes only how unproven owners are filtered; it still does not persist the original worker identity or classify a replayed pre-effect attempt as abandoned.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ef97f861f
ℹ️ 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".
| audit_path = live_path.parent / "audit.db" | ||
| with closing( | ||
| sqlite3.connect(f"{audit_path.resolve(strict=True).as_uri()}?mode=ro", uri=True) | ||
| ) as audit: |
There was a problem hiding this comment.
When a restore crashes after seed_or_rebind promotes the source head but before the committed restore record is published, a later retry enters this source_head != backup_head branch; if audit.db is now missing or unreadable—the exact failure this command must recover—the read-only open fails before the verified artifact is republished, permanently blocking that restore generation. Fresh evidence beyond the prior pending-rebind comment is that this is the already-promoted source-head path, not the pending-command path that was reordered; authenticate the operation from its immutable prepared record/source evidence or publish the verified audit image before consulting its head.
AGENTS.md reference: AGENTS.md:L121-L122
Useful? React with 👍 / 👎.
| except sqlite3.OperationalError as exc: | ||
| if "no such table" in str(exc).lower(): | ||
| return False |
There was a problem hiding this comment.
When either database still reports the current schema version but its continuity table has been lost or damaged, this treats no such table as the supported one-sided migration window. The audit decorator consequently falls back to standalone commits, so mutations continue changing the append-only journal without advancing or checking the source continuity head; normal bootstrap also accepts a same-version database without recreating the missing table. Distinguish genuinely older one-sided schemas from a missing table at a version where that table is mandatory, and raise AuditContinuityError for the latter.
AGENTS.md reference: AGENTS.md:L121-L122
Useful? React with 👍 / 👎.
| try: | ||
| metadata = self.path.lstat() | ||
| except FileNotFoundError as exc: | ||
| raise RuntimeError(f"audit tier is missing or uninitialized: {self.path}") from exc | ||
| except OSError as exc: | ||
| raise RuntimeError(f"cannot inspect audit tier leaf: {self.path}") from exc | ||
| if not stat.S_ISREG(metadata.st_mode): | ||
| raise RuntimeError(f"audit tier must be an archive-owned regular file: {self.path}") |
There was a problem hiding this comment.
When audit.db is a hard link to a backup or another archive's journal, lstat() still reports a regular file and this guard permits all subsequent writes. Those writes mutate the other pathname's supposedly independent append-only authority while only this archive's source.db continuity head advances, invalidating the backup or leaving the second archive with a regressed head. Require the audit leaf to be owned by the effective user and have exactly one link, matching the invariant already enforced for initialized and restore-staged durable images.
AGENTS.md reference: AGENTS.md:L121-L122
Useful? React with 👍 / 👎.
| INSERT INTO audit_continuity_head( | ||
| singleton, generation, head_sha256, mutation_id, advanced_at_ms | ||
| ) VALUES (1, 0, '3230fdd585a4fd2d71b7d720bcfe5d697ff120fdb32aecde394e89d407c7198f', NULL, 0); |
There was a problem hiding this comment.
When a populated audit v1 archive is upgraded, this migration gives it the same constant genesis head as an empty fresh audit database, and source migration 032 installs that same constant. Before the first coordinated mutation—and throughout either supported one-sided compatibility window, where standalone audit writes do not advance this head—replacing the upgraded journal with another v2/genesis image therefore passes source/audit reconciliation even if all preexisting previews, authorizations, runs, and receipts disappeared. Seed both halves with a value authenticated from the existing audit authority/content rather than a universal genesis constant.
AGENTS.md reference: AGENTS.md:L121-L122
Useful? React with 👍 / 👎.
|
@codex review the exact pushed head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52b4e04832
ℹ️ 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".
Problem: lifecycle replays, failed receipts, mutable preview payloads, and\nplatform-specific audit leaf access could make durable audit evidence diverge\nfrom the real domain action.\n\nWhat changed: record lifecycle replays as already satisfied, preserve failed\nattempt states, validate typed preview data before consumption, and harden\nportable descriptor-bound audit leaf ownership and cleanup.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e749f078f8
ℹ️ 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".
| """Issue a random one-time token bound to the persisted preview.""" | ||
|
|
||
| binding.validate() | ||
| validate_mutation_plan_integrity(preview.plan) |
There was a problem hiding this comment.
When a caller reconstructs a preview with dataclasses.replace(preview.plan, expires_at_ms=<future>), this new integrity check still passes because expires_at_ms is excluded from the recomputed hash. authorize_bound then tests and persists the substituted expiry, while _persist_authorization reads but never compares the original operation_previews.expires_at_ms, allowing an expired durable preview to mint a live authorization; validate this envelope field against the persisted preview before issuing the token.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
| alias = descriptor_alias_path(self._directory_fd) | ||
| if alias is None: | ||
| raise AuditLeafError(f"cannot access audit tier through a verified descriptor: {self._archive_root}") | ||
| return alias / self._filename |
There was a problem hiding this comment.
On the documented Homebrew macOS environment, /dev/fd/<n> duplicates the exact descriptor rather than providing Linux /proc/self/fd-style directory traversal, so appending /audit.db here is not a usable path (and the identity probe may reject the alias first). With no /proc/self/fd fallback on macOS, every audit-backed mutation fails before opening the authority; the Linux-only test passes because /dev/fd resolves to procfs there, so this needs a Darwin-capable descriptor strategy rather than treating the alias as a directory. Fresh evidence beyond the earlier portability thread is that the replacement now appends the leaf name to the descriptor alias.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
| except AuditLeafError: | ||
| self.close() | ||
| raise |
There was a problem hiding this comment.
If audit.db is replaced between _lstat_leaf_metadata() and _open_leaf_metadata() by a hard-linked or foreign-owned regular file, the first validation succeeds but the second _validate(metadata) at line 55 raises after this new handler has ended, leaving _directory_fd open. Repeating that race against authority requests can still exhaust the daemon's descriptors; fresh evidence beyond the resolved malformed-leaf thread is this separate post-open validation path, which should be covered by the same cleanup.
Useful? React with 👍 / 👎.
| if metadata.st_uid != os.geteuid(): | ||
| raise AuditLeafError( | ||
| f"audit tier must be owned by the current effective user: {self._archive_root / self._filename}" | ||
| ) |
There was a problem hiding this comment.
When a legacy or operator-created archive is group/world writable, matching st_uid still permits another principal to modify an audit.db whose mode grants group/other writes, and a writable archive directory also permits planting unvalidated audit.db-wal, audit.db-shm, or rollback-journal paths that SQLite opens alongside this validated main leaf. Normal mutation construction does not enforce the stricter root permissions used by durable maintenance, so checking only UID and link count does not make the complete SQLite authority single-writer; reject non-private leaf/root modes or independently validate every writable sidecar before opening SQLite.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
| preview: MutationPreview, | ||
| authorization: MutationAuthorization, | ||
| ) -> str | None: | ||
| validate_mutation_plan_integrity(preview.plan) |
There was a problem hiding this comment.
When a caller uses dataclasses.replace on an issued MutationAuthorization while retaining its token, actor, surface, and plan hash, this new validation accepts it because it checks only preview.plan. _consume_authorization verifies only those retained fields and then copies the caller-supplied role and capabilities into operation_runs and operation_run_capabilities, so the append-only run can claim authority evidence that contradicts its linked durable authorization; populate run evidence from the matched authorization row and operation_authorization_capabilities, or compare every supplied field before consumption.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
Preserve original attempt ownership during crash replay, fail closed on damaged current continuity schemas, authenticate populated legacy journals before binding them, and restore adopted audit images before consulting their heads. Harden descriptor-pinned main and sidecar access while keeping partial durable-train images explicitly non-applicable.
Problem: a promoted restore retry admitted unrelated higher source heads, and audit sidecars could change between validation and the first writer transaction.\n\nWhat changed: derive restore rebind targets from immutable prepared evidence, repair only the matching promoted audit head, and pin verified WAL/SHM sidecars before a writer is exposed. Read-only continuity inspection no longer creates sidecars, and invalid pre-continuity entries fail closed.
Use locking-aware read-only SQLite connections so continuity validation sees committed WAL state. Route source continuity access through no-follow descriptor-anchored opens and treat only literal absence as standby.
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Restore durable audit-tier adoption and continuity without weakening filesystem or SQLite authority. The branch now authenticates restore/rebind retries, reads committed WAL state, preserves descriptor-level diagnostics, and keeps WAL mode stable across concurrent readers and writers.
Problem
Restore adoption could authenticate the wrong bytes or strand continuity state; read-only authority checks could ignore committed WAL content; wrapper errors hid the exact rejected leaf; and toggling audit.db back to DELETE mode after every writer let a valid long-lived reader block the next writer before its audit transaction, leaving a prepared source command wedged.
Solution
Verification
Current exact pushed head:
51262f6e99d30ba14b19029d87badd624076a051.devtools verify --quickat the current head: all 25 checks passed in 130.19s.Anti-vacuity: the tests open the production verified audit reader, retain an active snapshot, then open and commit through the production verified writer. Reintroducing DELETE-mode cleanup or manual sidecar unlinking fails that route.
Bead disposition matrix
test:audit-continuity,command:devtools verify --quickNo Bead records are changed.
Changelog
Internal durable-audit correctness; no user-facing changelog entry.