Repository navigation
feat(audit): persist append-only checkpoint acceptance trail - #62
seonghobae wants to merge 50 commits into
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:
📝 WalkthroughWalkthroughPostgreSQL 체크포인트 저장 성공 감사 기능을 추가했습니다. 감사 이벤트는 저장과 같은 트랜잭션에 기록됩니다. 테넌트 격리, append-only 제약, 롤백 제한, 감사 조회 및 PostgreSQL 통합 CI 검증을 구현했습니다. Changes체크포인트 감사 추적
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant 호출자
participant 감사저장소
participant PostgreSQL
호출자->>감사저장소: save()
감사저장소->>PostgreSQL: 체크포인트 저장
감사저장소->>PostgreSQL: checkpoint_save_accepted 삽입
PostgreSQL-->>호출자: 결과 반환 또는 트랜잭션 오류
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review @opencode-agent @cwl-noema-review Perform a review-only inspection of exact stacked head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="f3059450f1f263ed59979ae94fb7bf5b621616be"
head="f765afa7f68391cf0460a8a151667ec3eaff2543"
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf '%s\n' '=== commit ancestry ==='
git merge-base --is-ancestor "$base" "$head" && echo "base_is_ancestor=yes" || echo "base_is_ancestor=no"
git show -s --format='base=%H%nbase_subject=%s' "$base"
git show -s --format='head=%H%nhead_subject=%s' "$head"
printf '%s\n' '=== changed paths ==='
git diff --name-status "$base" "$head"
printf '%s\n' '=== relevant source candidates ==='
fd -t f . | rg -i '(audit|checkpoint|migration|postgres|docker|workflow|ci|operator|architecture|adr|changelog|agents|claude|doctor)'
printf '%s\n' '=== Python structure ==='
fd -e py . | xargs -r ast-grep outline --items all --type class,function --match 'Audit|Checkpoint|audit|checkpoint' --view compact
printf '%s\n' '=== SQL and workflow excerpts ==='
git diff --unified=80 "$base" "$head" -- \
'*.sql' \
'*.yml' \
'*.yaml' \
'*audit*' \
'*checkpoint*' \
'README.md' \
'CHANGELOG.md' \
'AGENTS.md' \
'CLAUDE.md' \
'ARCHITECTURE.md' \
'docs/**' \
':!*.pyc'Length of output: 5853 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="f3059450f1f263ed59979ae94fb7bf5b621616be"
head="f765afa7f68391cf0460a8a151667ec3eaff2543"
printf '%s\n' '=== checked-out and requested tree identities ==='
git rev-parse HEAD
git rev-parse "$head^{tree}"
git merge-base "$base" "$head"
git diff --stat "$base" "$head"
printf '%s\n' '=== audit and checkpoint module outlines ==='
ast-grep outline pg_llm_batch/checkpoint_audit.py --items all --view signatures
ast-grep outline pg_llm_batch/checkpoint_store.py --items all --view signatures
printf '%s\n' '=== audit store implementation ==='
git show "$head:pg_llm_batch/checkpoint_audit.py"
printf '%s\n' '=== checkpoint store transaction implementation ==='
git show "$head:pg_llm_batch/checkpoint_store.py" | sed -n '1,460p'
printf '%s\n' '=== package migration ==='
git show "$head:pg_llm_batch/migrations/0008_result_checkpoint_audit_events.sql"
printf '%s\n' '=== rollback migration ==='
git show "$head:pg_llm_batch/migrations/rollback/0008_result_checkpoint_audit_events.sql"
printf '%s\n' '=== package/Docker migration identity ==='
git show "$head:pg_llm_batch/migrations/0008_result_checkpoint_audit_events.sql" > /tmp/package-audit-migration.sql
git show "$head:docker/postgres/init/04_result_checkpoint_audit_events.sql" > /tmp/docker-audit-migration.sql
sha256sum /tmp/package-audit-migration.sql /tmp/docker-audit-migration.sql
cmp -s /tmp/package-audit-migration.sql /tmp/docker-audit-migration.sql && echo 'migration_bytes_identical=yes' || echo 'migration_bytes_identical=no'
printf '%s\n' '=== public export and changed test outlines ==='
git show "$head:pg_llm_batch/__init__.py"
for f in tests/test_checkpoint_audit_contract.py tests/test_checkpoint_audit_store.py tests/test_checkpoint_audit_integration.py tests/test_checkpoint_audit_ci_contract.py tests/test_dependency_refresh_contract.py; do
echo "--- $f"
ast-grep outline "$f" --items all --view signatures
done
printf '%s\n' '=== CI audit-related sections ==='
git show "$head:.github/workflows/ci.yml" | rg -n -C 18 -i 'audit|postgres|setup-uv|checkpoint'Length of output: 47909 🏁 Script executed: #!/bin/bash
set -euo pipefail
head="f765afa7f68391cf0460a8a151667ec3eaff2543"
show() {
printf '\n=== %s ===\n' "$1"
git show "$head:$1"
}
show tests/test_checkpoint_audit_integration.py
show tests/test_checkpoint_audit_store.py
show tests/test_checkpoint_audit_contract.py
show tests/test_checkpoint_audit_ci_contract.py
show pg_llm_batch/db.py
show pg_llm_batch/migrations/0007_result_stream_checkpoints.sql
show docker/postgres/Dockerfile
show docs/adr/0009-append-only-checkpoint-audit-trail.md
show docs/checkpoint-audit.md
show docs/doctoring/checkpoint-audit-trail.md
printf '%s\n' '=== migration-related references and privileges ==='
rg -n -C 4 \
'llm_result_checkpoint_audit_events|reject_checkpoint_audit_mutation|GRANT|REVOKE|tenant_scope|FORCE ROW|clock_timestamp|CURRENT_TIMESTAMP|NOW\(\)' \
pg_llm_batch docker tests docs AGENTS.md ARCHITECTURE.md CLAUDE.md CHANGELOG.md
printf '%s\n' '=== APA-style reference locations ==='
rg -n -C 6 -i 'NIST|OWASP|SP 800-53|APA|References|PostgreSQL 18|trigger|current time' \
AGENTS.md ARCHITECTURE.md CHANGELOG.md CLAUDE.md docsLength of output: 201616 🏁 Script executed: #!/bin/bash
set -euo pipefail
head="f765afa7f68391cf0460a8a151667ec3eaff2543"
for spec in \
'tests/test_checkpoint_audit_integration.py:1,280' \
'tests/test_checkpoint_audit_store.py:1,380' \
'tests/test_checkpoint_audit_contract.py:1,140' \
'pg_llm_batch/db.py:1,180' \
'pg_llm_batch/migrations/0007_result_stream_checkpoints.sql:1,260' \
'docker/postgres/Dockerfile:1,160' \
'.github/workflows/ci.yml:1,155' \
'docs/checkpoint-audit.md:1,150'; do
file="${spec%%:*}"
ranges="${spec#*:}"
start="${ranges%,*}"
end="${ranges#*,}"
printf '\n=== %s lines %s-%s ===\n' "$file" "$start" "$end"
git show "$head:$file" | nl -ba | sed -n "${start},${end}p"
done
printf '%s\n' '=== audit-specific SQL test assertions ==='
git show "$head:tests/test_checkpoint_audit_contract.py" | nl -ba
printf '%s\n' '=== audit-specific changed diff with line numbers ==='
git diff --unified=12 f3059450f1f263ed59979ae94fb7bf5b621616be "$head" -- \
pg_llm_batch/checkpoint_audit.py \
pg_llm_batch/migrations/0008_result_checkpoint_audit_events.sql \
pg_llm_batch/migrations/rollback/0008_result_checkpoint_audit_events.sql \
tests/test_checkpoint_audit_integration.py \
.github/workflows/ci.ymlLength of output: 1622 🤖 web_search executed:
Length of output: 10208 🏁 Script executed: #!/bin/bash
set -euo pipefail
head="f765afa7f68391cf0460a8a151667ec3eaff2543"
show_range() {
file="$1"
start="$2"
end="$3"
printf '\n=== %s lines %s-%s ===\n' "$file" "$start" "$end"
git show "$head:$file" | sed -n "${start},${end}p"
}
show_range tests/test_checkpoint_audit_integration.py 1 280
show_range tests/test_checkpoint_audit_store.py 1 380
show_range tests/test_checkpoint_audit_contract.py 1 140
show_range pg_llm_batch/db.py 1 180
show_range pg_llm_batch/migrations/0007_result_stream_checkpoints.sql 1 260
show_range docker/postgres/Dockerfile 1 160
show_range .github/workflows/ci.yml 1 155
show_range docs/checkpoint-audit.md 1 150
printf '%s\n' '=== audit-specific changed diff ==='
git diff --unified=12 f3059450f1f263ed59979ae94fb7bf5b621616be "$head" -- \
pg_llm_batch/checkpoint_audit.py \
pg_llm_batch/migrations/0008_result_checkpoint_audit_events.sql \
pg_llm_batch/migrations/rollback/0008_result_checkpoint_audit_events.sql \
tests/test_checkpoint_audit_integration.py \
.github/workflows/ci.ymlLength of output: 80201
I found one least-privilege issue.
I found no additional defect in the reviewed same-transaction write sequence, forced-RLS policy, append-only trigger behavior, non-empty rollback refusal, The PostgreSQL authority supports the documented semantics: The package and Docker audit migrations have identical SHA-256 values. The reviewed head is exact. The requested predecessor tip is not an ancestor of this head. Their common merge base is ✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/doctoring/checkpoint-audit-trail.md`:
- Around line 114-119: 분리된 역할별 권한 계약과 mutation-probe 검증을 반영하십시오. 통합 테스트의 애플리케이션
역할에는 체크포인트 테이블의 SELECT·INSERT·UPDATE와 감사 테이블의 SELECT·INSERT만 부여하고, 감사 identity
sequence 권한만 허용하십시오. 감사 테이블의 UPDATE·DELETE·TRUNCATE 및 `55000` 실패 검증은 별도의
mutation-probe 역할로 수행하며, 문서의 권한 설명과 테스트 설정을 동일하게 갱신하십시오.
In `@tests/test_checkpoint_audit_ci_contract.py`:
- Around line 9-21: Replace the raw text checks in
tests/test_checkpoint_audit_ci_contract.py:9-21 with parsed YAML assertions
scoped to the checkpoint-audit-integration job, verifying its PostgreSQL
service, matching DSN, persist-credentials: false, and integration test command
within the appropriate job or step. Also update
tests/test_dependency_refresh_contract.py:11-19 to inspect each
astral-sh/setup-uv step independently and require version "0.12.1" and
prune-cache: true in that same step scope.
In `@tests/test_checkpoint_audit_integration.py`:
- Around line 92-110: Update the role grants in the checkpoint audit setup
around database_admin and cursor.execute: grant the normal application role only
SELECT and INSERT on llm_result_checkpoint_audit_events, plus the minimum
USAGE/SELECT permissions on the specific audit sequence it requires; remove
UPDATE, DELETE, TRUNCATE, and the blanket grant on all public-schema sequences.
Use a separate temporary mutation-probe role for validating append-only trigger
rejection.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c15dfcca-11b3-431c-a546-58f66f88280f
📒 Files selected for processing (21)
.github/workflows/ci.ymlAGENTS.mdARCHITECTURE.mdCHANGELOG.mdCLAUDE.mddocker/postgres/Dockerfiledocker/postgres/init/04_result_checkpoint_audit_events.sqldocs/adr/0009-append-only-checkpoint-audit-trail.mddocs/checkpoint-audit.mddocs/checkpoint-observability.mddocs/doctoring/checkpoint-audit-trail.mdpg_llm_batch/__init__.pypg_llm_batch/checkpoint_audit.pypg_llm_batch/migrations/0008_result_checkpoint_audit_events.sqlpg_llm_batch/migrations/rollback/0008_result_checkpoint_audit_events.sqltests/test_checkpoint_audit_ci_contract.pytests/test_checkpoint_audit_contract.pytests/test_checkpoint_audit_integration.pytests/test_checkpoint_audit_store.pytests/test_checkpoint_telemetry_concurrency.pytests/test_dependency_refresh_contract.py
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
@opencode-agent @cwl-noema-review
Review-only request for exact head 2820aa36d8dedf7d89d1b745e5728acf3b913d2b against stacked base branch agent/checkpoint-opentelemetry-observability. Do not mutate the branch, create a repair workflow, mark ready, or merge. Independently verify the separated application/mutation-probe PostgreSQL roles, exact identity-sequence grant, tenant/RLS and append-only boundaries, scoped CI contracts, migration/rollback safety, 100% production statement/branch and docstring evidence, reproducible-package acceptance, and absence of generated artifacts. Treat the current draft stack and missing independent non-author approval as blocking merge boundaries.
|
Exact-current-base ancestry audit (no branch mutation): source head The two base-only predecessor corrections are already byte-identical on this source head: Do not reuse the current CI/Release Acceptance evidence after predecessor ancestry reconciliation. Reconcile through one reviewed writer path in dependency order, then regenerate exact-head/exact-base quality, security, review, and release evidence. No temporary workflow, duplicate repair path, force update, merge, or protection bypass was created in this audit. |
|
Current head/base audit after dependency-order reconciliation:
Older |
|
Superseded by #79. Replacement head |
1 participant
Commercial and acquisition gap
Durable checkpoint persistence and best-effort OpenTelemetry observability do not by themselves provide durable application audit evidence. This bounded stacked slice adds package-owned, tenant-isolated accepted-save evidence without claiming cryptographic non-repudiation or administrator-proof tamper resistance.
Implemented bounded vertical slice
AuditedPostgresBatchResultCheckpointStoreon top of the durable PostgreSQL checkpoint store;checkpoint_save_acceptedevent after every successful save call, including exact idempotent repeats, in the same PostgreSQL transaction as checkpoint persistence;CheckpointAuditEventand newest-first bounded reads with a non-coercive 1..1,000 limit;llm_result_checkpoint_audit_eventswith forced RLS, fixed-value/checkpoint constraints, descriptive snake_case indexes/policy/triggers, UPDATE/DELETE rejection, and a statement-level TRUNCATE rejection trigger;clock_timestamp()at row insertion rather than transaction-startNOW()/CURRENT_TIMESTAMP;NOSUPERUSER ... NOBYPASSRLSapplication role and a separate mutation-probe role;pg_get_serial_sequence()plusparse_ident();55000, without checkpoint-table or audit-sequence authority;contents: read, checkout credential persistence disabled, and the live verification job non-writing with respect to the repository; andNo temporary or write-capable repair workflow, generated coverage database, build product, cache, version bump, publication, or release authority is included.
Strict RED → GREEN → refactor evidence
8a2deeb14c23d9db97bbad1d4c68837b6c8591cc.3dfab0d6132e523396d2b2e27125aff34d8565e4; CI31138134741failed collection becausepg_llm_batch.checkpoint_auditintentionally did not exist.467f40e7282b6916c7e681636550c7c215db88e2; CI31139284742failed because the permanent live PostgreSQL audit job did not yet exist.31a584c62bab27774c93a18adbbacec22699bc78; CI31139440808exposed invalid protocol parameterization ofCREATE ROLE ... PASSWORD, leading to safe psycopg composable identifiers/literals and fail-safe partial-provision cleanup.9e43420097faabd97deab079c34c5e4e0207eb86,19b39a23dc7e29a467cc6c0d6817f06b5da1e880, and78184f6202d06831800ef3e90db498e9e04e26b5encoded insert-time wall-clock semantics and idempotent migration repair before implementation. Their superseded/cancelled workflow is not counted as success.5dfdecfaf1fb7a4d88be338bd44eddb69814a174; CI31148349909, job92772560159, failed the permanent role-binding contract because the application role still held audit mutation rights and blanket public-schema sequence access.b8d28af14d4ecef3d4497c11c782d13ded25d7d7; CI31148738607proved the live PostgreSQL role split succeeded but exposed two brittle workflow-parser tests. That run is failure evidence only and is not reused as success.2820aa36d8dedf7d89d1b745e5728acf3b913d2bcontains the least-privilege role separation, exact identity-sequence grant, scoped workflow parsing, decoy regressions, authoritative documentation, and changelog repair.Current exact-head evidence
2820aa36d8dedf7d89d1b745e5728acf3b913d2b.8a2deeb14c23d9db97bbad1d4c68837b6c8591cconagent/checkpoint-opentelemetry-observability; the current predecessor branch tip is PR feat(observability): instrument durable checkpoint operations #61 headf3059450f1f263ed59979ae94fb7bf5b621616be. Connector base metadata is not treated as current integrated-base evidence.31148932545: success on the exact current head. Python 3.10/3.12/3.14 unit tests, coverage/docstrings/lint/package, container builds, and the live PostgreSQL checkpoint-audit integration completed successfully.31148932566: success on the exact current head, including reproducible wheel and source distribution verification. This is acceptance evidence only; no version bump or publication is authorized by this stacked draft.APPROVEDreview exists on the current exact head.Dependency and merge boundary
Required order remains:
.github#790 -> pg-llm-batch#53 -> #55 -> #56 -> #57 -> #58 -> #59 -> #60 -> #61 -> this PR.This PR remains a stacked draft. It must not be marked ready or merged until every prerequisite integrates into
main, the branch is reconciled onto the actual integrated base without losing predecessor fixes, and fresh integrated exact-head/exact-base quality, security, dependency, packaging, migration, rollback, live PostgreSQL, container, provenance, supply-chain, release-acceptance, branch-protection, and independent-review gates succeed. Zero unresolved valid findings and a qualifying independent non-author GitHubAPPROVEDreview are mandatory. Staged-base or synthetic merge-result evidence is not reusable as final integrated release evidence.Summary by CodeRabbit
새로운 기능
보안 및 테스트