Repository navigation
ci: make release workflow Reborn compile-only - #6188
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
github-actions
Bot
added
size: XS
📝 WalkthroughSummary by CodeRabbit
WalkthroughRelease CI disables the legacy release and Docker publication paths with impossible repository guards. Smoke tests validate workflow wiring, and Code Style now selects and propagates Reborn CLI workflow checks. Documentation records the policy and rollback conditions. ChangesReborn release validation
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a changelog entry and a smoke test to verify that the release CI workflow temporarily skips Docker image builds and publication while keeping independent Docker workflow runs active. The reviewer suggested a more robust approach for extracting the Docker job block in the test to prevent fragility when the workflow file is modified.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | 82ee5c5879de |
Head: 82ee5c5879de564c31ccafaf068be9fa4078fd97
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
The release caller is correctly disabled, but the new regression test is not wired to run for future workflow-only changes, so the policy can regress before merge without failing required checks.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Run this contract test for workflow-only changes
Location: crates/ironclaw_reborn_cli/tests/smoke.rs:158-180
This test lives in the Reborn CLI smoke suite, but the has_reborn_cli path gate in .github/workflows/code_style.yml excludes both .github/workflows/release.yml and .github/workflows/docker.yml. The Reborn test classifier likewise returns has_reborn_tests=false for either workflow path. Consequently, a later PR or merge-group diff that only re-enables this caller or disables the independent entry points will not run this contract; it runs only after landing, when push runs force all tests. Add these workflow paths to the smoke-test CI scope (with classifier coverage), or move the assertion into an always-run workflow validation job.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 476-477: Update the job guard for update-registry-checksums by
removing the constant if: ${{ false }} expression, or replace it with a valid
non-constant condition; preserve needs: host and ensure the job is not
incorrectly gated on docker-image.
In `@crates/ironclaw_reborn_cli/tests/smoke.rs`:
- Around line 170-177: Update the release CI assertion in the smoke test to
match the lint-compliant false condition used by release.yml, replacing the
current `if: ${{ false }}` string while preserving the existing checks for the
Docker caller, release flag, dependency, and inherited secrets.
🪄 Autofix (Beta)
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: 2e21fb15-9e0e-4a41-b424-0a4225fa397e
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
📒 Files selected for processing (3)
.github/workflows/README.md.github/workflows/release.ymlcrates/ironclaw_reborn_cli/tests/smoke.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/ironclaw_reborn_cli/tests/smoke.rs`:
- Around line 4298-4323: Update the smoke-test scope classifier used for the
ironclaw_reborn_cli test suite to include .github/workflows/release.yml and
.github/workflows/docker.yml in its curated allowlist, ensuring workflow-only
changes run
release_ci_skips_docker_publish_without_disabling_independent_docker_runs.
Alternatively, move this validation into a CI check that is guaranteed to run
for workflow changes.
🪄 Autofix (Beta)
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: d6c7285f-079b-46b7-afbb-80c07b81670e
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
📒 Files selected for processing (2)
.github/workflows/README.mdcrates/ironclaw_reborn_cli/tests/smoke.rs
|
🚅 Deployed to the ironclaw-pr-6188 environment in ironclaw-ci-preview
|
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.59% — 313152 / 365860 lines Per-crate breakdown (65 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
github-actions
Bot
added
size: S
|
Review follow-up for head
Validation: focused contract test, rustfmt, YAML parse, actionlint 1.7.12 on all three relevant workflows, and @ironloopai review |
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 0 | 0 | eb4f4efc993d |
Head: eb4f4efc993dbf152817652f8369a9f5094df507
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Approved. This is a focused five-file CI-policy change that disables only the tag-driven release Docker caller. The independent manual and hourly Docker entry points remain unchanged, no downstream release job depends on the skipped caller, and the new smoke contract plus Code Style routing cover the changed policy. No concrete correctness, security, or test-coverage issue was found.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
|
Besides docker image, do we need to skip other jobs like build-wasm-extensions in the release workflow? |
|
@think-in-universe I tested this with a real tag push in a personal fork. Evidence:
Observed tag behavior:
So the current #6188 guard is working as intended for Docker. build-wasm-extensions is not a Docker dependency; it feeds WASM bundles/checksums into the existing GitHub Release path. If the intended tag behavior is to keep the existing non-Docker release artifacts, I would leave WASM and the rest of the cargo-dist DAG unchanged. If the intended behavior is Reborn compile-only validation, then yes, we should skip the entire legacy plan -> WASM/local/global -> host -> checksum/announce chain, not only build-wasm-extensions. Skipping WASM alone still allows build-local and host to publish the legacy release. #6188 itself does not contain the Reborn compile job, so I have not expanded its scope without confirmation. |
hanakannzashi
changed the title
|
Follow-up after the release-scope clarification: the intended tag path is Reborn-only, so I updated head
I converted this PR to Draft because it must merge last: #6185 first, then update/merge #6176 with the canonical Local validation passed: full Reborn CLI smoke suite (103 tests), focused workflow contract, rustfmt, all-target/all-feature Clippy with warnings denied, YAML parse, actionlint, pre-commit safety, and diff check. The previous fork tag run validated only Docker skipping. I will run a new fork tag test on the final combined head before marking this ready, verifying that only the Reborn matrix runs and that no GitHub Release is created. @ironloopai review |
There was a problem hiding this comment.
⏭️ IronLoop Review Declined: reviewer
Review at a glance
| Disposition | Head |
|---|---|
| ⏭️ Review declined | 8a796809e24b |
Head: 8a796809e24b11e99c23fd6a5ed6e929bf5085e1
Reason: The required stacked-PR context is unavailable; reviewing this layer alone cannot verify that a release tag retains the intended seven-platform Reborn compile path.
Next: Rebase this PR onto the merged #6185 and #6176 changes so release.yml contains the Reborn compile matrix, then run the required fork tag test against that combined head and request review again.
Run details
Status: Current
Trustworthy review produced: no
Summary
Skipped: this draft layer disables every currently present release job, while its required Reborn compile matrix (#6176) is not included in the supplied base-to-head comparison.
|
The required combined release-path proof is now complete: fork Release run 29668508858 used current #6176 (rebased after #6185, canonical Results:
This PR correctly remains Draft until #6176 merges. Then it still needs a rebase onto that result and final local contract/static validation before review; the whole old #6188 branch should not be merged/cherry-picked into #6176 because it predates #6185 and would regress canonical paths. |
8a79680 to
6be18b3
Compare
|
Final post-#6176 update is complete on head
The PR is now ready for review and does not depend on #6122. @ironloopai review |
There was a problem hiding this comment.
⚠️ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| 0 | 0 | 0 | 6be18b3a15fe |
Head: 6be18b3a15fe7284b9c61896f07ed9b15588024a
Next: Human review or validation is required before merging.
Run details
Status: Current
Needs human: no
Needs validation: yes
Summary
Static review found no actionable correctness or security defects in this focused six-file CI/documentation change. The legacy release DAG is disabled at its root while the Reborn seven-target compile caller remains active, and the Code Style roll-up now propagates workflow-only smoke failures.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 0 | 0 | 6be18b3a15fe |
Head: 6be18b3a15fe7284b9c61896f07ed9b15588024a
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Approved. The focused six-file CI change correctly makes the legacy release DAG unreachable from its disabled plan root, preserves the active Reborn compile caller, keeps Docker release publishing independently disabled, and wires workflow-only changes into the required smoke-result roll-up.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/code_style.yml (1)
278-280: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant
corepack enable pnpmstep.
corepack enable pnpmis already executed prior tosetup-node(e.g., lines 268-270) to ensure thepnpmbinary is available for cache hashing. Repeating it immediately after is dead logic.
.github/workflows/code_style.yml#L278-L280: remove the redundantEnable pnpmstep in theclippyjob..github/workflows/code_style.yml#L334-L336: remove the redundantEnable pnpmstep in theclippy-windowsjob.🤖 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 @.github/workflows/code_style.yml around lines 278 - 280, Remove the redundant “Enable pnpm” step from the clippy job in .github/workflows/code_style.yml lines 278-280 and from the clippy-windows job at lines 334-336; retain the earlier Corepack setup before setup-node in both jobs.
🤖 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.
Outside diff comments:
In @.github/workflows/code_style.yml:
- Around line 278-280: Remove the redundant “Enable pnpm” step from the clippy
job in .github/workflows/code_style.yml lines 278-280 and from the
clippy-windows job at lines 334-336; retain the earlier Corepack setup before
setup-node in both jobs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aa9c1e1a-67ae-4544-86c3-30d62a6f41b1
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!CHANGELOG.md
📒 Files selected for processing (3)
.github/workflows/README.md.github/workflows/code_style.yml.github/workflows/release.yml
|
@claude review |
Code Review: PR #6188Found 6 issues across security, architecture, and test quality: 🔴 HIGH Priority[HIGH:75] Logic error in code_style.yml rollup gate (lines 479-484) The conditional exit structure is inverted: if [[ "${{ needs.changes.outputs.has_code }}" == "false" ]]; then
if [[ "${{ needs.changes.outputs.has_reborn_cli }}" == "true" && "${{ needs.reborn-cli-smoke.result }}" != "success" ]]; then
exit 1
fi
echo "No code changes — style checks skipped correctly"
exit 0
fiWhen a PR touches only workflow files and Reborn CLI smoke fails, this job will Suggested fix: Move the reborn-cli smoke check outside the [HIGH:95] Test belongs in architecture tier, not smoke suite
This test should move to a dedicated CI contract validator (e.g., a GitHub Actions workflow job with YAML schema validation, or an architecture-tier test). [HIGH:85] Test has incomplete dependency DAG verification The test verifies that legacy jobs depend on build-global-artifacts:
needs: [plan, reborn-binary-compile]The test would pass. Add a negative assertion: 🟡 MEDIUM Priority[CRITICAL:95] Test uses fragile substring matching for YAML structure The assertion [MEDIUM:80] Test-first discipline not followed Per CLAUDE.md §Testing Discipline, infrastructure changes should start with a failing test, not add the test post-implementation. There's no evidence this test was written first and allowed to fail before the workflow changes were made. [MEDIUM:75] No regression test marked; missing commit context This is a high-risk change (disabling release artifact publication paths) but carries no SummaryThe PR's intent is clear and the core workflow changes are sound (disabling legacy paths with impossible guards, keeping architecture visible for rollback). The issues are organizational: the test should move to the architecture tier, the logic gate in code_style needs restructuring, and the test suite should use YAML parsing instead of string matching. Recommend: Fix the code_style.yml logic gate (high priority), move the test to architecture tier, and consider YAML parsing for robustness. |
This branch was successfully deployed
2 participants
Summary
planroot so all dependent legacy build, host, checksum, and announcement jobs skip.docker.ymlremain available.reborn-binary-compilepath as the only active release-tag job.Dependency state
#6185 and #6176 are merged. This branch was rebuilt as one commit directly on #6176's merge result (
5d3dfcc1c) and preserves the reusable seven-target Reborn compile workflow plus the host dependency/success gate.This work does not depend on #6122.
The PR remains Draft only while the final fork tag run and refreshed CI/review complete.
Change Type
Linked Issue
Refs #6160
Validation
cargo +1.96.0 fmt --all -- --checkcargo +1.96.0 test -p ironclaw_reborn_cli(368 passed: 257 unit + 5 extension + 106 smoke)cargo +1.96.0 clippy --workspace --all-targets --all-features -- -D warningscargo +1.96.0 test -p ironclaw_architecture(66 passed)release.yml,docker.yml,code_style.yml, andreborn-release-compile.ymlscripts/pre-commit-safety.shgit diff --checkironclaw-v0.30.1-rc.4resolved to PR head6be18b3a1; all seven compile/native-smoke jobs succeeded, both musl portability checks passed, exactly seven non-emptyreborn-compile-*artifacts were uploaded, all eight legacy/publish jobs skipped, and no rc.4 GitHub Release or checksum PR was created.The earlier fork release run 29610386170 validated only the superseded Docker-only policy.
Security Impact
The disabled legacy root prevents the tag path from reaching legacy artifact publication, GitHub Release creation, registry checksum updates, announcements, or their associated publishing credentials. The explicit Docker guard prevents release-path image publication. Independent manual/scheduled Docker workflow permissions are unchanged.
Reborn Trust-Boundary Checklist
N/A — this changes release CI policy only and does not alter runtime, persistence, ingress, capability, or product security boundaries.
Database Impact
None.
Blast Radius
Release tags run only the seven-platform Reborn compile matrix. They produce short-lived Actions evidence artifacts, but no legacy binaries, WASM bundles, Docker images, GitHub Release, permanent downloadable release assets, registry checksum update, or announcement. Manual/hourly
docker.ymlentry points are unaffected.Rollback Plan
Remove
plan's impossiblegithub.repository == ''guard to re-enable the non-Docker legacy release DAG; the workflow trigger is already tag-only. Restoredocker-image.ifto${{ always() && needs.host.result == 'success' }}separately only when release image publication is intentionally re-enabled. No schema or persisted-state migration is involved.Review track: C (CI/release)