fix(release): fail before side effects when Trusted Publishing is unready - #30
Conversation
|
Warning Review limit reached
Next review available in: 14 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthrough릴리스 워크플로에 Changes신뢰된 릴리스 식별 경계
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant ReleaseReadiness
participant GitHubEnvironment
participant PyPIAPI
participant BuildRelease
GitHubActions->>ReleaseReadiness: 공식 main 릴리스에서 readiness 실행
ReleaseReadiness->>GitHubEnvironment: pypi 보호 설정과 승인 정책 확인
ReleaseReadiness->>PyPIAPI: RELEASE_VERSION 존재 여부 확인
ReleaseReadiness->>BuildRelease: 검증 성공 후 빌드 실행 허용
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/release.yml:
- Around line 50-53: Update the RELEASE_VERSION validation condition to reject
leading zeros in any numeric component while continuing to accept canonical
MAJOR.MINOR.PATCH versions; constrain each component to either exactly 0 or a
nonzero digit followed by digits, and preserve the existing error and exit
behavior.
In `@tests/test_release_workflow.py`:
- Around line 108-124: The test
test_release_requires_environment_readiness_before_build_or_irreversible_work
must also assert that the release-readiness block validates GITHUB_REF_PROTECTED
and rejects leading-zero versions such as 01.2.3. Add explicit checks for the
corresponding protection-ref condition and version-validation regex or behavior,
while preserving the existing environment reviewer and PyPI checks.
🪄 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: 7625b24f-664a-43c4-b6aa-ae84fca675f6
📒 Files selected for processing (8)
.github/workflows/release.ymlCHANGELOG.mddocs/RELEASE_PROVENANCE.mddocs/TRACEABILITY.mddocs/adr/0008-trusted-release-identity-boundary.mddocs/adr/README.mdtests/test_architecture_documentation.pytests/test_release_workflow.py
|
@coderabbitai review |
|
|
@cwl-noema-review |
|
@cwl-noema-review |
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
263751612acf50c9d332b278cba9dfe0857fbc6f. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/ThreadWeave/actions/runs/31357717822/job/93360434467)
- Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/ThreadWeave/actions/runs/31357717822/job/93360434467)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: release.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: release.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (4 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (4 files)"]
R3 --> V3["docs review"]
Evidence --> S4["CI script: release_contract.py"]
S4 --> I4["review and security gate shell path"]
I4 --> R4["Review risk: CI script: release_contract.py"]
R4 --> V4["bash -n plus Strix self-test"]
Evidence --> S5["Test (4 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test (4 files)"]
R5 --> V5["targeted test run"]
OpenCode Review Overview
--> Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: release.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: release.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (4 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (4 files)"]
R3 --> V3["docs review"]
Evidence --> S4["CI script: release_contract.py"]
S4 --> I4["review and security gate shell path"]
I4 --> R4["Review risk: CI script: release_contract.py"]
R4 --> V4["bash -n plus Strix self-test"]
Evidence --> S5["Test (4 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test (4 files)"]
R5 --> V5["targeted test run"]
|
|
@opencode-agent @coderabbitai review Review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='59eb6d609a6cbf0699e427e913067d8b0c3beb41'
head='4326b18462986b053a7d020acae9600869816395'
printf '%s\n' '== checked-out commit =='
git rev-parse HEAD
printf '%s\n' '== requested commits =='
git show -s --format='%H%n%P%n%s' "$base"
git show -s --format='%H%n%P%n%s' "$head"
printf '%s\n' '== ancestry =='
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor=true'
printf '%s\n' '== commits in requested range =='
git log --format='%H %s' "${base}..${head}"
printf '%s\n' '== changed paths =='
git diff --name-status "$base" "$head"
printf '%s\n' '== diff statistics =='
git diff --stat "$base" "$head"
printf '%s\n' '== Python test outlines =='
fd -t f -e py . tests | sort | xargs -r ast-grep outline --type function --view compact
printf '%s\n' '== workflow outline =='
ast-grep outline .github/workflows/release.yml --view expanded || trueLength of output: 5007 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='59eb6d609a6cbf0699e427e913067d8b0c3beb41'
head='4326b18462986b053a7d020acae9600869816395'
printf '%s\n' '== modified Python symbol map =='
ast-grep outline scripts/ci/release_contract.py --view signatures
ast-grep outline tests/test_release_contract_coverage.py --view signatures
ast-grep outline tests/test_release_readiness_adversarial.py --view signatures
ast-grep outline tests/test_release_workflow.py --view signatures
ast-grep outline tests/test_architecture_documentation.py --view signatures
printf '%s\n' '== workflow and release-contract diff =='
git diff --no-ext-diff --unified=80 "$base" "$head" -- \
.github/workflows/release.yml \
scripts/ci/release_contract.py \
tests/test_release_contract_coverage.py \
tests/test_release_readiness_adversarial.py \
tests/test_release_workflow.py
printf '%s\n' '== current release workflow =='
cat -n .github/workflows/release.yml
printf '%s\n' '== current release contract =='
cat -n scripts/ci/release_contract.pyLength of output: 88370 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='59eb6d609a6cbf0699e427e913067d8b0c3beb41'
head='4326b18462986b053a7d020acae9600869816395'
printf '%s\n' '== release contract test map and sources =='
ast-grep outline tests/test_release_contract.py --view signatures
cat -n tests/test_release_contract.py
printf '%s\n' '== coverage configuration candidates =='
fd -t f -a 'pyproject.toml' -a '.coveragerc' -a 'tox.ini' -a 'setup.cfg' . -x sh -c 'echo "----- $1"; cat -n "$1"' sh {}
printf '%s\n' '== package metadata and package version =='
cat -n pyproject.toml
cat -n src/threadweave/__init__.py
printf '%s\n' '== remaining changed-file diff =='
git diff --no-ext-diff --unified=60 "$base" "$head" -- \
CHANGELOG.md \
docs/RELEASE_PROVENANCE.md \
docs/TRACEABILITY.md \
docs/adr/0008-trusted-release-identity-boundary.md \
docs/adr/README.md \
tests/test_architecture_documentation.py
printf '%s\n' '== references to release coverage and contract test modules =='
rg -n -C 3 --glob '*.yml' --glob '*.yaml' --glob '*.py' \
'test_release_contract(?:_coverage|_readiness_adversarial)?\.py|coverage report|fail-under|release_contract' \
.github scripts testsLength of output: 79234
I verified the exact requested head
|
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Approval sufficiency: Evidence affirmatively supports the PR intent (fail before build/attest/tag/release/publish side effects when Trusted Publishing is unready) — I inspected changed-file evidence for .github/workflows/release.yml (new release-readiness job preceding build-release, needs: release-readiness, fail-closed gh-api/jq/curl checks) and the CodeGraph verbatim current-head source of tests/test_release_workflow.py, plus CHANGELOG.md, docs/RELEASE_PROVENANCE.md, docs/adr/0008-trusted-release-identity-boundary.md, and docs/adr/README.md; current-head authority shows no failed checks, no unresolved threads, and Coverage Result: PASS. Verification posture: Coverage execution evidence reports Result: PASS (supported repository test suites passed; docstring gates passed or advisory) under repo contracts python3 -m pytest tests and coverage report --fail-under=100; no OPENCODE_EXECUTION_RECEIPT exists for the GitHub-hosted release job itself (not executable in this sandbox) — stated as a source limitation. Linter/static: ruff/actionlint are repository gates with no failed checks at current head; workflow YAML shape is enforced by contract tests (harden-runner marker, folded allowed-endpoints: >- scalar, permission counts). TDD/regression: new/updated tests cover the gate — test_release_requires_environment_readiness_before_build_or_irreversible_work, test_release_harden_runner_endpoint_input_is_space_delimited, test_release_separates_build_attestation_tag_release_and_publish_privileges, test_workflow_passes_user_input_via_environment_not_shell_interpolation, the new tests/test_release_readiness_adversarial.py (protected-ref and leading-zero preflight, commit ad0fcce), plus test_release_contract_coverage.py hard-linked-metadata rejection (HEAD commit 4326b18). Coverage: Coverage execution evidence Result: PASS — supported repository test suites passed. Docstring coverage: Coverage execution evidence — configured repository docstring gates passed or advisory. DAG: Mermaid flowchart of head flow — flowchart LR;
G["release-readiness gate (.github/workflows/release.yml)"] --> B["build-release (needs release-readiness)"];
B --> C["attest-release"];
C --> D["tag-release"];
D --> E["github-release"];
E --> F["publish-pypi (environment: pypi)"];
G --> R["Main risk: fail-closed gate blocks release on env/API mismatch"];
G --> V["Verification: test_release_workflow.py + test_release_readiness_adversarial.py, coverage PASS"];
reflects head-side change vs base, where build-release was the first job. PoC/execution: no runtime receipt for the hosted release job; verification rests on trusted Coverage execution evidence PASS plus source trace; no browser/DevTools claims made. DDD/domain: release authority separation extended in docs/RELEASE_PROVENANCE.md (authority items 5-8) and ADR-0008; no domain-model change. CDD/context: the changed context boundary is the release trust boundary — repository-side environment readiness must hold before irreversible side effects (ADR-0008). Similar issues: consistent with prior release hardening (fb7dab5 harden boundaries, 8bf0c3b trusted publishing, 723da96 protected ref + canonical SemVer). Claim/concept check: the PR claim matches workflow ordering, fail-closed exits, tests, and ADR-0008 documentation. Standards search: no new external standards asserted; GitHub environments/Trusted Publishing conventions are referenced in docs with no contradicting evidence in scope. Compatibility/convention: naming review — new job release-readiness (kebab-case, two words, consistent with build-release/tag-release/github-release); environment name pypi follows the documented GitHub Trusted Publishing convention; shell locals reviewer_count, prevent_self_review, protected_branches, environment_json are descriptive snake_case; no DB/API/event identifiers added and no sequential-id exposure. Breaking-change/backcompat: intentional operational change — manual release now requires a pre-created protected pypi environment, protected ref, and canonical SemVer; documented in RELEASE_PROVENANCE.md and ADR-0008; no API/schema backcompat impact. Implementation completeness: the readiness job is fully implemented with fail-closed exits for every branch (protected ref, canonical version, environment existence, reviewer count, self-review, protected branches, published-version refusal); ADR-0008 added and indexed in docs/adr/README.md; CHANGELOG 0.2.0 entry updated; no placeholder bodies. Performance: one extra ~10-minute job with two lightweight HTTP calls; negligible. Developer experience: DX surface is the release/operator flow — clear ::error:: diagnostics before side effects and updated RELEASE_PROVENANCE.md checklist. User experience: UX surface is release/ops reliability (no user-facing UI); fail-before-side-effects reduces broken-release incidents. Visual/DOM: non-web change; the interaction surface reviewed is CLI/workflow output (::error:: annotations and job ordering), not DOM/ARIA. Accessibility/i18n: non-web change; no UI surface; error text is plain ASCII. Supply-chain/license: no new dependencies; harden-runner pinned by full SHA bf7454d0 # v2.20.0; existing pinned actions unchanged; no license changes. Packaging: release pipeline retains python -m build --no-isolation and Trusted Publishing publish; no packaging-contract change. Security/privacy: egress blocked with an allowlist covering api.github.com:443 and pypi.org:443 only; version input reaches shell/URL only via RELEASE_VERSION env with regex validation and quoting (no ${{ inputs.version }} interpolation in run blocks); no username/password/PYPI_API_TOKEN in the workflow; gate fails closed on missing/unreadable environment state.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including .github/workflows/release.yml, CHANGELOG.md, docs/RELEASE_PROVENANCE.md, docs/TRACEABILITY.md, docs/adr/0008-trusted-release-identity-boundary.md, and 6 more.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects .github/workflows/release.yml to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":"tests/test_release_workflow.py","line":177,"hypothesis":"The new release-readiness job's harden-runner egress allowlist omits an endpoint its own steps call (gh api -> api.github.com:443, curl -> pypi.org:443) or uses a literal block scalar, so the gate would fail at runtime or the allowlist would be unverified.","attack_or_counterexample":"Drive the release-readiness job: gh api repos/.../environments/pypi contacts api.github.com:443 and curl https://pypi.org/pypi/threadweave/$RELEASE_VERSION/json contacts pypi.org:443; compare the job's allowlist against the set of hosts actually contacted.","evidence":"Trusted source trace: test_release_harden_runner_endpoint_input_is_space_delimited (CodeGraph verbatim current-head source, tests/test_release_workflow.py:170-191) iterates the (\"release-readiness\", \"build-release\") tuple at tests/test_release_workflow.py:177 and asserts each job has exactly one harden-runner, egress-policy: block, a folded `allowed-endpoints: >-` scalar, and _hardened_endpoints(job) == EXPECTED_RELEASE_ENDPOINTS[\"release-readiness\"] == {api.github.com:443, pypi.org:443, results-receiver.actions.githubusercontent.com:443, *.actions.githubusercontent.com:443, *.blob.core.windows.net:443}; the focused diff of .github/workflows/release.yml shows exactly that allowlist on the readiness job; Coverage execution evidence reports Result: PASS (supported repository test suites passed), so the parity assertion observed a passing outcome; source-line-sha256=a6c3ff50e61c9a24eaae9beaf79f6f55b3437ff697bfd833a52bbc60b614b541","outcome":"falsified"},{"path":".github/workflows/release.yml","line":117,"hypothesis":"build-release (and the downstream attest/tag/github-release/publish chain) can start before or without the release-readiness gate, so the fail-before-side-effects promise is bypassable when the pypi environment is missing or unprotected.","attack_or_counterexample":"Trigger workflow_dispatch on main with a missing pypi environment and attempt to reach build-release; also consider whether job ordering alone (without a needs edge) would let build-release start first.","evidence":"Trusted source trace: the focused diff inserts the release-readiness job before build-release and adds `+ needs: release-readiness` directly under the build-release job header at .github/workflows/release.yml:117-118; test_release_requires_environment_readiness_before_build_or_irreversible_work (CodeGraph verbatim, tests/test_release_workflow.py:108-124) asserts workflow.index(\" release-readiness:\\n\") < workflow.index(\" build-release:\\n\") and \"needs: release-readiness\" in the build block, and the readiness step exits 1 with ::error:: on GITHUB_REF_PROTECTED != true, non-canonical version, gh api environment fetch failure, reviewer_count < 1, prevent_self_review != true, protected_branches != true, or already-published version; Coverage execution evidence reports Result: PASS, so the ordering/needs assertions observed a passing outcome; source-line-sha256=e408c2f1bba8f72a9defa9b3cac33af83ced4c155428c12924586908be7ce381","outcome":"falsified"}],"residual_risk":"GitHub platform details (exact GITHUB_REF_PROTECTED value semantics for workflow_dispatch on protected main, and whether the environments REST response carries prevent_self_review) are not verifiable offline; every plausible mismatch direction fails closed with an ::error:: annotation before build/attest/tag/release/publish side effects, so the residual risk is a possible operational false-failure on the first manual release run rather than a safety defect. The PyPI not-already-published check is inherently TOCTOU; the late publish-pypi job remains the final fail-closed gate. Reviewed-file access limitation: direct reads of /home/runner/work/_temp/opencode-pr-head were denied by the sandbox, so release_contract.py and the remaining test hunks were evaluated via history evidence, Coverage PASS, and CodeGraph blast-radius only."}- Result: APPROVE
- Reason: Fail-closed release-readiness gate is correctly ordered before build and contract-tested; coverage PASS, no failed checks, no unresolved threads at current head.
- Head SHA:
4326b18462986b053a7d020acae9600869816395 - Workflow run: 31448074506
- Workflow attempt: 1
Superseded automated OpenCode change request from a previous head; exact current head 4326b18 has a later OpenCode approval.
Buyer-visible release reliability gap
ThreadWeave's release workflow previously reached build/attestation, annotated tag creation, and GitHub Release creation before the
publish-pypijob exercised thepypienvironment and PyPI Trusted Publisher relationship. Current GitHub documentation requires deployment environments to be created before use and applies required reviewers/self-review/branch restrictions to jobs referencing them. Deferring environment existence/protection checks until the late publishing job therefore risks discovering a preventable repository-side readiness defect only after externally visible release side effects.Test-first change
The release-workflow contract was updated first to require a
release-readinessjob beforebuild-releaseand to requirebuild-releaseto depend on it. The implementation then adds that gate.The readiness gate:
mainrelease dispatches;MAJOR.MINOR.PATCHinput before external lookup;pypienvironment to be pre-created;prevent_self_review=true;This configuration preflight does not pretend to prove the PyPI account-side Trusted Publisher relationship. The
publish-pypijob remains environment-bound, so GitHub still applies the actual required-reviewer approval before the OIDC publication job runs, and issue #17 remains the authority for end-to-end Trusted Publisher/public-artifact proof.Architecture and release records
No long-lived PyPI token, password, manual upload path, model credential, runtime dependency, or package API change is introduced.
Authoritative platform basis
GitHub's current deployment documentation states that environments must be created before workflow use, that required reviewers gate environment jobs, that self-review prevention prevents the deployment initiator from approving, and that deployment branch policies can restrict an environment to protected branches. The REST environment inspection endpoint is a read boundary; creation/update remains administrative.
Acceptance
Merge only after the exact contributor head passes workflow lint, Python 3.10–3.14 CI/coverage/docstrings/package smoke, SAST, Security Scan, current review policy, and zero valid unresolved findings. After integration, issue #17 still remains open until the
pypienvironment and PyPI Trusted Publisher are configured andthreadweave==0.2.0is publicly verified.Summary by CodeRabbit
새로운 기능
문서
테스트