Skip to content

ci: never let the dependency-submission step withhold the GitHub assets - #424

Merged
bernardladenthin merged 1 commit into
mainfrom
claude/guard-dependency-submission-step
Sep 9, 2026
Merged

ci: never let the dependency-submission step withhold the GitHub assets#424
bernardladenthin merged 1 commit into
mainfrom
claude/guard-dependency-submission-step

Conversation

@bernardladenthin

Copy link
Copy Markdown
Owner

One line plus its rationale comment, closing a cross-repo parity gap — but the gap is not cosmetic, and it defeats a guarantee this pipeline was deliberately built to provide.

The guarantee

github-snapshot / github-release-signed run even when the Central publish fails:

if: ${{ !cancelled() && (needs.publish-snapshot.result == 'success' || needs.publish-snapshot.result == 'failure') && ... }}
# Also runs when publish-snapshot FAILED (not when skipped/cancelled): a Central
# publish-poll timeout reds that job after the artifacts were already uploaded —
# the GitHub pre-release assets must not be lost in that case.

Note the precise shape: it tolerates failure, not skipped.

The hole

The dependency-submission step sits in the report job unguarded, and report gates the entire chain:

report FAILS  (third-party informational action)
  └─> check-snapshot        SKIPPED   ← failed need; its if: is a plain event/ref
                                        condition, no always()/!cancelled() escape
       └─> publish-snapshot SKIPPED   ← if: check-snapshot.result == 'success'
            └─> github-snapshot       if: publish-snapshot ∈ {success, failure}
                                      'skipped' matches NEITHER → does not run

So the action having a bad day doesn't merely delay a Central publish — it silently loses the GitHub assets, which is exactly the outcome the if: above exists to prevent. The action submits the dependency graph to GitHub; it says nothing about whether the artifacts are correct, so it has no business gating anything.

Why it was missing here and not in srcmorph

srcmorph got this guard in passing, from ee2ae49 "ci: report unsigned assets without ever withholding them". That commit's actual feature was the unsigned-asset reporting, which was ported here — as 90dd21a, a single clean 68 insertions(+) change that reproduced the feature and did not touch the submission step. The drive-by line never travelled. So this is not a regression and not a design divergence; it is a port that copied the intent and missed an unrelated hardening bundled into the same upstream commit.

Scope — what deliberately stays bare

The convention being restored is narrow: informational third-party steps do not block. Within report, the submission action was the only step violating it —

step before
actions/download-artifact guarded
maven-dependency-submission-action bare
Coveralls guarded
Codecov guarded

checkout and setup-java stay bare because they are infrastructure, not reports.

Verification

All four publish.yml files still parse; report still contains exactly one submission step; and the 5-line block (comment + uses: + flag) now hashes identically in all four repos5cbdaf64e4f4f303.

Companion PRs do the same in BitcoinAddressFinder and streambuffer.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH


Generated by Claude Code

The release pipeline is deliberately built so the GitHub assets land even when
the Central publish fails: github-snapshot / github-release run on
`publish-*.result == 'success' || == 'failure'`, with a comment saying so --
a Central publish-poll timeout must not lose the pre-release assets.

That guarantee has a hole upstream of it. The dependency-submission step sits
in the `report` job unguarded, and `report` gates the whole chain:

  report FAILS
    -> check-snapshot        SKIPPED  (failed need; its if: is a plain
                                       event/ref condition, no always())
      -> publish-snapshot    SKIPPED  (if: check-snapshot.result == 'success')
        -> github-snapshot   does NOT run -- its if: tolerates 'failure' but
                             'skipped' matches neither arm

So a third-party informational action having a bad day does not merely delay a
Central publish: it silently defeats the one thing the pipeline goes out of its
way to guarantee. The action submits the dependency graph to GitHub; it says
nothing about whether the artifacts are correct, so it has no business gating
anything.

srcmorph already had this guard -- added in passing by ee2ae49 ("report unsigned
assets without ever withholding them"), a commit whose *feature* was ported here
as a single clean change that reproduced the feature and not the drive-by line.
The three repos are now textually identical to srcmorph at that step (same
comment, same flag; the 5-line block hashes equal in all four).

Note what stays bare, on purpose: checkout and setup-java are infrastructure,
and streambuffer's PIT steps are gates, not reports. The convention this
restores is narrower -- informational third-party steps do not block -- and the
submission action was the only step in `report` violating it, sitting between a
guarded download-artifact above and guarded Coveralls/Codecov below.

Verified: the four publish.yml files still parse, `report` still has exactly one
submission step, and it now carries continue-on-error in all four.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AnNYn8W1xuVxVJtyL34GyH
@claude

claude Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Summary

Approved — This is a well-reasoned fix for a real pipeline guarantee.

What this fixes

The PR addresses a critical gap in the release pipeline's error handling:

  • The guarantee: GitHub pre-release assets should land even when Central publish fails (gated by if: ... result == 'failure')
  • The hole: The dependency-submission step was unguarded, causing report to fail and cascade to skip downstream jobs
  • The result: If dependency-submission failed, github-snapshot would be skipped (not matching the 'success' or 'failure' conditions expected), silently losing GitHub assets

Quality observations

  1. Correct implementation: The fix is minimal and follows established patterns in the same job (download-artifact, Coveralls, Codecov all have continue-on-error: true)

  2. Well-documented: The added comment clearly explains why this flag is needed and the specific scenario it prevents — excellent for future maintainers

  3. Upstream precedent: srcmorph already carries this guard, so this aligns with cross-repo conventions as documented in CLAUDE.md

  4. No side effects: This change only affects error handling for an informational step; no business logic changes. Successful runs are unaffected.

  5. Consistent scope: The fix narrows the guarantee correctly — informational third-party actions should not block the pipeline

Verification

✅ All informational steps in report now have continue-on-error: true
✅ The downstream github-snapshot and github-release-signed jobs still have their explicit failure-tolerant conditions
✅ File syntax is valid

@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@bernardladenthin
bernardladenthin merged commit 6c64f78 into main Sep 9, 2026
13 of 24 checks passed
@bernardladenthin
bernardladenthin deleted the claude/guard-dependency-submission-step branch September 9, 2026 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants