feat(audit): add bounded stable export pages - #64
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head/base review recorded for head 207ceb48de33d599673d7d888523e17b81276866 against base 2820aa36d8dedf7d89d1b745e5728acf3b913d2b.
Two actionable integration findings remain:
-
This parallel stack creates
docs/adr/0010-bounded-checkpoint-audit-export-pagination.md, while already-open predecessor candidate #63 createsdocs/adr/0010-atomic-checkpoint-schema-operator.md. Both currently branch from #62 and also edit AGENTS.md, CLAUDE.md, ARCHITECTURE.md, CHANGELOG.md, andpg_llm_batch/__init__.py. Integrating both as sibling stacks would produce duplicate ADR numbering and avoidable merge conflicts. Serialize this slice after #63 and renumber this ADR to 0011 (including all references/tests), or otherwise establish one explicit non-conflicting order before further writes. -
CI run
31151155188is not exact-source-head evidence. Its quality job checked out synthetic merge commit9bc1190436baacfe45ac326ee67fe13b057a6e44c, not PR head207ceb48de33d599673d7d888523e17b81276866. Do not count that run as exact-head success. Stacking after #63 should inherit the exact-head checkout/verification contract; then regenerate all CI, live PostgreSQL, coverage, package, container, release-acceptance, security, and review evidence on the reconciled exact head/base.
I did not mutate this branch because it appeared during an active independent writer session. Preserve one writer, address these findings test-first, and resolve this review only after the branch is linearly reconciled and fresh exact-head evidence exists.
@coderabbitai review
@opencode-agent @cwl-noema-review review only; do not write the branch, create a repair workflow, mark ready, or merge.
|
Closing as superseded by #65. The #64 sibling stack conflicts with #63 on ADR 0010 and overlapping authoritative files, and its successful CI used the synthetic merge ref rather than the exact source head. #65 is the linearized replacement stacked directly on #63, renumbers the audit-export ADR path to 0011, and carries the test-first contract forward without parallel branch writers. No evidence from #64 is treated as merge evidence for #65. |
Product and acquisition gap
The accepted-save audit trail was queryable only as one newest-first bounded list. Operators that must export retained evidence to separately governed immutable or write-once storage could not traverse longer history without inventing their own pagination contract, and OFFSET-based traversal is vulnerable to duplicate/skip drift when newer rows become visible concurrently.
Strict RED → GREEN → REFACTOR evidence
This draft is stacked on PR #62 exact base
2820aa36d8dedf7d89d1b745e5728acf3b913d2b.RED began at
ce2df9680d767a5c5bd6771c81c7e4e79ec75ea2and was completed at test-only head3be0e4afe133e54e8224fdc5aa3453eb7d3a45b7. The tests specified absent production contracts for a strict positive PostgreSQL BIGINT cursor, immutable strictly descending pages, bounded one-row lookahead, keyset continuation, stable traversal toward older rows, trusted-key revalidation, malformed-driver failure, and package-owned delegation. The RED workflow runs were superseded/cancelled by subsequent commits and are not counted as passing evidence.Production implementation then added
CheckpointAuditPage,validate_checkpoint_audit_cursor(), andlist_audit_event_page*()withcheckpoint_audit_event_id < before_audit_event_id, exact trusted(tenant, consumer, endpoint, batch)row-key revalidation, strict descending identity validation, and a maximum SQL fetch oflimit + 1. A later test-first contract at551858960565650635b7705c8c03c7e004afa54fidentified that direct publicCheckpointAuditEventconstruction could represent an identity above PostgreSQL signed BIGINT even though the database cannot; production now rejects that mismatch. A further RED contract at0986987894c0dc83b1182ad77b94483339b774caproved that a faulty adapter could return an identity at or newer than a continuation cursor despite the SQL predicate; current production revalidates every returned continuation identity and fails closed on that inconsistency.A live least-privilege PostgreSQL regression reads page one, commits a newer accepted-save event in a separate transaction, and proves page two continues strictly toward older identities without duplicating page-one rows or admitting the newer row. Refactoring reduced redundant branch structure without weakening validation. No OFFSET path, migration, database write permission, exporter credential, network destination, scheduled writer, temporary repair workflow, generated coverage database, cache, or build artifact was introduced.
Concurrency and assurance boundary
Keyset pagination prevents later higher-identity inserts from shifting an older continuation window. It does not claim one database snapshot, chronology proof, completeness proof, delivery receipt, cryptographic authentication, or non-repudiation. Identity allocation may contain gaps and commit order can differ from allocation order.
Package-owned page calls use independent database transactions. A host requiring one snapshot across a long export pass must begin its own PostgreSQL
REPEATABLE READor stricter transaction before the first query and repeatedly calllist_audit_event_page_in_transaction()on that same transaction. External immutable/WORM storage, retention, legal hold, export receipts, cryptographic manifests, and reconciliation remain host/operator controls.Authoritative documentation
The contract is synchronized in
AGENTS.md,CLAUDE.md,ARCHITECTURE.md,CHANGELOG.md,docs/checkpoint-audit.md, ADR 0010, anddocs/doctoring/checkpoint-audit-export-pagination.md. Doctoring records APA 7 references to NIST SP 800-53 Rev. 5 AU-9 and PostgreSQL 18 transaction-isolation/concurrency documentation.Dependency and merge boundary
Required order remains
.github#790 -> pg-llm-batch#53 -> #55 -> #56 -> #57 -> #58 -> #59 -> #60 -> #61 -> #62 -> #64.Current exact head is
207ceb48de33d599673d7d888523e17b81276866; stacked base is2820aa36d8dedf7d89d1b745e5728acf3b913d2b. Current-head CI, release acceptance, automated review, and all other required gates must be evaluated only on this exact head; queued, pending, cancelled, skipped-required, absent, predecessor-head, or stale-base evidence is not success. This PR remains draft and must not merge until every prerequisite integrates, it is reconciled onto the integrated protected base, all required exact-head quality/security/coverage/package/review/provenance/release-acceptance gates succeed, unresolved valid findings are zero, and a qualifying independent non-author GitHubAPPROVEDreview exists.