feat(release): rebuild reproducible evidence on current main - #145
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthrough릴리스 수용 워크플로와 Changes릴리스 증거
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant ExactHead
participant UVBuild
participant ReleaseEvidence
participant ArtifactStore
GitHubActions->>ExactHead: checkout exact PR head
ExactHead->>UVBuild: build wheel and sdist in two clean trees
UVBuild->>ReleaseEvidence: provide two artifact sets
ReleaseEvidence->>ReleaseEvidence: verify identity, size, and SHA-256
ReleaseEvidence->>ReleaseEvidence: write manifest atomically
ReleaseEvidence->>ArtifactStore: retain manifest for 14 days
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
tests/test_release_artifact_dirfd.py (2)
149-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Path.open몽키패치는 사용되지 않습니다. 제거를 고려하십시오.
pg_llm_batch/release_evidence.py는 아티팩트를 오직os.open(..., dir_fd=...)로만 엽니다.Path.open경로는 호출되지 않으므로racing_path_open은 절대 실행되지 않습니다. 이 스캐폴딩은 구현이 여전히 경로명 기반 열기를 사용한다는 잘못된 인상을 줍니다.두 테스트에서
original_path_open,racing_path_open, 해당monkeypatch.setattr(Path, "open", ...)호출을 제거하면 의도가 명확해집니다.Also applies to: 160-163, 176-176, 197-197, 207-210, 223-223
🤖 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/test_release_artifact_dirfd.py` at line 149, Remove the unused Path.open monkeypatch scaffolding from both tests, including original_path_open, racing_path_open, and each monkeypatch.setattr(Path, "open", ...) call. Keep the existing os.open dir_fd race-testing logic and assertions unchanged.
328-333: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
raising=False를 제거하십시오.
_SECURE_ARTIFACT_FLAGS_AVAILABLE는 모듈에 실제로 존재합니다.raising=False는 향후 상수 이름이 바뀌어도 테스트가 조용히 통과하게 만듭니다. 기본값인raising=True를 사용하면 이름 변경이 즉시 드러납니다. 같은 파일의 다른 테스트(tests/test_release_evidence_dirfd.py75행)는 이미 기본값을 사용합니다.♻️ 제안 수정
monkeypatch.setattr( release_evidence, "_SECURE_ARTIFACT_FLAGS_AVAILABLE", False, - raising=False, )🤖 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/test_release_artifact_dirfd.py` around lines 328 - 333, Remove the explicit raising=False argument from the monkeypatch.setattr call targeting release_evidence._SECURE_ARTIFACT_FLAGS_AVAILABLE, relying on the default raising=True so renamed or missing constants cause the test to fail immediately.tests/test_release_artifact_identity_races.py (1)
14-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value픽스처와 상수가
tests/test_release_artifact_dirfd.py와 중복됩니다.
DISTRIBUTION,VERSION,COMMIT,SOURCE_DATE_EPOCH,WHEEL,SDIST,_write_release,_verify가 두 파일에 거의 동일하게 존재합니다. 유일한 차이는_write_release의parents=True여부입니다. 버전 또는 배포 이름이 바뀌면 두 파일을 함께 수정해야 합니다.공유
conftest.py또는 테스트 헬퍼 모듈로 이동하면 한 곳에서 관리됩니다. 이 변경은 선택 사항입니다.🤖 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/test_release_artifact_identity_races.py` around lines 14 - 38, Optionally consolidate the duplicated release constants and helper functions DISTRIBUTION, VERSION, COMMIT, SOURCE_DATE_EPOCH, WHEEL, SDIST, _write_release, and _verify from the two test modules into a shared conftest.py or test helper module. Reuse the shared definitions in both tests, preserving the only behavioral difference: _write_release must retain its required directory-creation semantics, including parents=True where needed.pg_llm_batch/release_evidence.py (1)
114-140: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value두 개의 디스크립터 순회 함수가 거의 동일합니다. 공통 헬퍼로 통합하는 방법을 고려하십시오.
_open_release_directory와_open_manifest_parent는 앵커 열기, 컴포넌트 반복, 이전 디스크립터 닫기, 실패 시 정리라는 동일한 구조를 반복합니다. 차이는os.mkdir생성 단계와 오류 메시지뿐입니다. 보안 순회 로직이 한 곳에만 존재하면 향후 한쪽만 수정되는 위험이 사라집니다.예:
_walk_directory_descriptor(anchor, parts, *, create_mode: int | None, missing_message: str) -> int형태의 내부 헬퍼를 추가하고, 두 함수는 이를 호출하도록 변경하십시오.이 변경은 선택 사항입니다. 현재 동작에는 문제가 없습니다.
Also applies to: 334-369
🤖 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 `@pg_llm_batch/release_evidence.py` around lines 114 - 140, Optionally consolidate the duplicated descriptor-walking logic from _open_release_directory and _open_manifest_parent into a shared internal helper, such as _walk_directory_descriptor, that opens the anchor, iterates components, optionally creates missing directories, closes replaced descriptors, and cleans up on failure. Preserve each caller’s existing creation behavior and error messages while routing both through the common security traversal path.
🤖 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/adr/0003-reproducible-release-evidence.md`:
- Around line 67-68: Update item 12 in the ADR to describe the implementation
order used by _write_manifest_payload: fsync the file, perform the
descriptor-relative os.rename() atomic replacement, then fsync the final parent
directory. Keep the existing payload-writing and synchronization details while
correcting only this ordering.
In `@docs/doctoring/release-artifact-descriptor-verification.md`:
- Around line 50-51: Update step 10 in the release artifact descriptor
verification document to require comparison of the complete initial entry
snapshots, not only the bounded name tuple. Describe that the snapshots include
device, inode, file type, size, mtime, and ctime, matching the behavior of
_scan_release_entries and _artifact_records so inode replacement is detected.
In `@docs/superpowers/plans/2026-08-06-release-evidence-dirfd-hardening.md`:
- Around line 24-26: Update
docs/superpowers/plans/2026-08-06-release-evidence-dirfd-hardening.md lines
24-26 to use protected commit 00ed6aabb82c1754f8b14fa85929cac56f68402b as the
stack base, and revise the PR numbers and dependency order at lines 125-126 from
.github#790 -> `#53` -> `#55` -> `#56` to match the current integrated state; update
docs/superpowers/specs/2026-08-06-release-evidence-dirfd-hardening-design.md
line 5 similarly so its Dependency entry references the current base commit and
PR information rather than PR `#55`.
In
`@docs/superpowers/specs/2026-08-06-release-evidence-dirfd-hardening-design.md`:
- Around line 34-36: Update item 5 to explicitly permit destinations that are
absent or existing regular files, while continuing to reject every existing
non-regular destination; keep the wording aligned with the behavior of
_validate_manifest_destination and the documented “absent or a regular file”
contract.
In `@pg_llm_batch/config.py`:
- Around line 106-112: Update the boolean conversion branch in the configuration
parser to avoid using bool(raw) for unrecognized non-empty strings. After
normalizing the input, treat invalid boolean values such as “maybe” or “false ”
according to the function docstring’s “fall back on error” contract, returning
the configured default or rejecting them during set, while preserving recognized
true and false values.
- Around line 204-206: Update the set and get methods to use the same
normalized, typed value for both database persistence and cache storage, so
string inputs such as false produce consistent typed results across instances.
Ensure dict and list values cannot share mutable references with callers by
returning defensive copies from get, or reject mutable values consistently.
Preserve the existing default fallback behavior.
In `@tests/test_release_artifact_dirfd.py`:
- Around line 233-256: Update racing_read in
test_verifier_refuses_in_place_mutation_during_streaming_hash to replace the
artifact contents with data whose length differs from the original, ensuring the
verifier detects the mutation through file-size metadata without relying on
timestamp precision.
In `@tests/test_release_evidence_dirfd.py`:
- Around line 250-264: Update the test around write_release_manifest to
monkeypatch and record os.close calls, then assert the descriptor captured by
failing_fdopen was closed. Remove the os.fstat-based assertion so validation
does not depend on descriptor-number reuse, while preserving the temporary-file
cleanup assertion.
In `@tests/test_release_evidence_documentation.py`:
- Around line 22-27: Update
test_release_evidence_documents_exact_build_toolchain to load uv.toml
required-version and assert the documentation uses uv 0.12.3, using the existing
tomllib/tomli fallback pattern for Python 3.10 compatibility. Update the
documented uv version expectations to 0.12.3 while preserving the existing
uv_build==0.12.1 and lockfile wording assertions.
---
Nitpick comments:
In `@pg_llm_batch/release_evidence.py`:
- Around line 114-140: Optionally consolidate the duplicated descriptor-walking
logic from _open_release_directory and _open_manifest_parent into a shared
internal helper, such as _walk_directory_descriptor, that opens the anchor,
iterates components, optionally creates missing directories, closes replaced
descriptors, and cleans up on failure. Preserve each caller’s existing creation
behavior and error messages while routing both through the common security
traversal path.
In `@tests/test_release_artifact_dirfd.py`:
- Line 149: Remove the unused Path.open monkeypatch scaffolding from both tests,
including original_path_open, racing_path_open, and each
monkeypatch.setattr(Path, "open", ...) call. Keep the existing os.open dir_fd
race-testing logic and assertions unchanged.
- Around line 328-333: Remove the explicit raising=False argument from the
monkeypatch.setattr call targeting
release_evidence._SECURE_ARTIFACT_FLAGS_AVAILABLE, relying on the default
raising=True so renamed or missing constants cause the test to fail immediately.
In `@tests/test_release_artifact_identity_races.py`:
- Around line 14-38: Optionally consolidate the duplicated release constants and
helper functions DISTRIBUTION, VERSION, COMMIT, SOURCE_DATE_EPOCH, WHEEL, SDIST,
_write_release, and _verify from the two test modules into a shared conftest.py
or test helper module. Reuse the shared definitions in both tests, preserving
the only behavioral difference: _write_release must retain its required
directory-creation semantics, including parents=True where needed.
🪄 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: 21f0af94-434d-4c32-ba45-4e31c9bd4550
📒 Files selected for processing (27)
.github/workflows/release-acceptance.ymlCHANGELOG.mdDockerfiledocs/adr/0003-reproducible-release-evidence.mddocs/adr/0004-descriptor-pinned-release-artifact-verification.mddocs/doctoring/durable-lifecycle-failure-evidence.mddocs/doctoring/release-artifact-descriptor-verification.mddocs/doctoring/reproducible-release-evidence.mddocs/superpowers/plans/2026-08-06-release-evidence-dirfd-hardening.mddocs/superpowers/specs/2026-08-06-release-evidence-dirfd-hardening-design.mdpg_llm_batch/config.pypg_llm_batch/durable_client.pypg_llm_batch/release_evidence.pypyproject.tomltests/test_config_boolean_fallback.pytests/test_config_collection_type_fallback.pytests/test_container_packaging_contract.pytests/test_lifecycle_failure_confidentiality.pytests/test_packaging_metadata.pytests/test_release_acceptance_workflow.pytests/test_release_artifact_dirfd.pytests/test_release_artifact_dirfd_documentation.pytests/test_release_artifact_identity_races.pytests/test_release_evidence.pytests/test_release_evidence_dirfd.pytests/test_release_evidence_dirfd_documentation.pytests/test_release_evidence_documentation.py
💤 Files with no reviewable changes (4)
- tests/test_config_boolean_fallback.py
- docs/doctoring/durable-lifecycle-failure-evidence.md
- tests/test_config_collection_type_fallback.py
- tests/test_lifecycle_failure_confidentiality.py
Controlled current-main replacement for #57
The original release-evidence PR #57 remained stacked on the now-obsolete tenant branch. Protected main has since integrated the tenant lifecycle replacement and substantial independent hardening. Fresh path comparison proved #57's unique semantic delta is 26 paths; only
.github/workflows/ci.yml,AGENTS.md,ARCHITECTURE.md,CHANGELOG.md, andCLAUDE.mdoverlap protected-main movement.This replacement starts from exact protected
main00ed6aabb82c1754f8b14fa85929cac56f68402band replays the exact #57 result blobs for the other 21 non-overlapping release-evidence paths. The five overlapping current-main files are deliberately retained unchanged in the initial replay so exact-head CI and documentation/workflow contracts can identify the narrow composition actually required.The preserved implementation includes deterministic dual clean builds, descriptor-relative/no-follow manifest publication, held-descriptor artifact identity checks, same-name inode/in-place mutation defenses, bounded enumeration and hashing, the release-acceptance workflow, packaging metadata, and focused security/reproducibility tests and doctoring.
No #57 checks, reviews, approvals, generated merge, or stale-base evidence transfers. No force-push, destructive rebase, conflict-side selection, gate weakening, package publication, release authority, or central dependency workaround is introduced.
Replacement proof
00ed6aabb82c1754f8b14fa85929cac56f68402ba25e35dd51536f8c2b8f14298746c91df03fb414Merge boundary
Keep Draft until exact-head CI/security/coverage/package/reproducibility/release-acceptance and required central workflows are terminal-success, current-main overlap composition is resolved test-first, review threads are clear, and live rules are satisfied. Merge only on an unchanged exact head. Close #57 as superseded only after this replacement is coherent and protected-main integration is complete; then rebuild downstream #58 and successors in dependency order.
Summary by CodeRabbit
새 기능
개선 사항
문서