Repository navigation
feat: add an action to notify Pubgrade from CI - #115
Merged
Merged
Conversation
Add the composite action actions/notify, which registers a prebuilt
image as a build of a repository (POST /repositories/{id}/builds), so
that other repositories can notify Pubgrade from their release
workflows with a few lines of YAML instead of an inline step. Callers
pass the URL, the repository ID and the token as inputs (the token from
a secret); the action masks the token, sends it in the header via
curl's stdin only, takes HTTPS only unless allow-http is set, derives
head_commit from the run's tag or branch and commit unless given, and
outputs the build ID. Invalid inputs always fail; failed requests fail
unless fail-on-error is false. Only requests that didn't reach Pubgrade
(no connection, 502, 503, 504) are retried, so that a build is never
registered twice.
Pubgrade takes an image with a tag and no digest, so the input image
takes no digest either (the issue preferred one).
Test the script against a stub of Pubgrade (tests/test_notify_action.py)
and the action itself in the new CI job "Notify action" against
Pubgrade from the branch's image, which the gate now needs. The start
of the app moves from the smoke job to tests/start.sh, shared by both
jobs. Document the action in the usage and development docs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Repository and build identifiers may contain '.' and '~' (the characters that pubgrade/charsets.py allows), which the action rejected. Check the token against the same characters, so that it can't break out of the quotes in curl's configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A 502 or 504 may come after Pubgrade registered the build (e.g. a proxy's timeout while Pubgrade notifies the subscriptions), so the action no longer retries them; it still retries when it can't connect and on a 503. Escape the messages of workflow commands, so that Pubgrade's error text stays one annotation. In a run for a tag, git-commit now needs git-branch, instead of sending the tag as branch. Test the warning paths, error bodies without JSON or detail, and curl errors that aren't retried; rename the CI job to notify, give its checks error messages, document pull request runs, and rewrap the comments and docs that the first commit left too long. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mask every line of the token before checking it, as the runner masks secrets, so that an invalid token with line breaks isn't printed. Document which failures are retried (refused connections, unresolved host names, 503) and that proxies like Envoy may answer 503 after Pubgrade received the request. Fix the default of git-branch in the action's description, and show the body when the CI job's build check can't fetch the build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideAPIDefine the composite action’s inputs, output, and invocation contract for callers registering a prebuilt image with Pubgrade.
Uses of the APIExercise the composite action against a live branch-built Pubgrade instance and make notification checks prerequisites for publishing and the CI gate.
ImplementationImplement input validation, Git metadata selection, authenticated build registration, retry behavior, and workflow outputs for the notify action.
Extract shared Docker startup and readiness logic for running Pubgrade with MongoDB in CI.
DocumentationDocument how callers configure the action and its inputs, outputs, retry behavior, and development and CI coverage.
TestsTest the notify script’s request payload, credential handling, validation, Git metadata, errors, and retry behavior against a local stub.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #115 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 18 18
Lines 1545 1545
=========================================
Hits 1545 1545 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
uniqueg
marked this pull request as ready for review
November 13, 2026 13:22
Linked issuesChecked at commit 35f3e66 #88 feat: reusable action to notify Pubgrade from CI · 7 of 9 met
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 9 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="actions/notify/notify.sh" line_range="142" />
<code_context>
+ 0:200)
+ break
+ ;;
+ 6:* | 7:* | 0:503)
+ if [[ "${status}" -eq 0 ]]; then
+ reason="Pubgrade answered ${code}"
</code_context>
<issue_to_address>
**Duplicate builds and notifications**
When a proxy returns 503 after Pubgrade has accepted and registered the POST, `notify.sh` retries the non-idempotent POST, and each `register_builds` call creates a fresh build ID and notifies subscribers again. The repository gains duplicate builds and deployments receive duplicate updates.
Make the POST idempotent or avoid retrying when Pubgrade may already have accepted it.
</issue_to_address>
### Comment 2
<location path="actions/notify/notify.sh" line_range="133" />
<code_context>
+ status=0
+ code="$(curl --proto "${protocols}" --silent --show-error \
+ --connect-timeout 10 --max-time 60 \
+ --request POST "${url%/}/repositories/${repository_id}/builds" \
+ --header 'Content-Type: application/json' \
+ --data "${body}" \
</code_context>
<issue_to_address>
**Older builds can overwrite newer deployments**
When two CI runs publish images for the same subscribed ref concurrently and the older notification reaches the subscriber last, pubgrade handles the two POSTs independently and can deliver the older image after the newer one; subscribers receive only the image name and tag, with no commit or sequence to reject a stale notification, so a deployment ends on the older image.
Serialize notifications for each repository/ref, or include a monotonic version that subscribers can use to reject stale notifications.
</issue_to_address>
### Comment 3
<location path="actions/notify/notify.sh" line_range="113-114" />
<code_context>
+body="$(jq --null-input --compact-output \
+ --arg name "${image}" --arg type "${build_type}" \
+ --argjson head "${head_commit}" \
+ '{images: [{name: $name}], head_commit: $head}
+ + if $type == "" then {} else {build_type: $type} end')"
+
+response="$(mktemp)"
</code_context>
<issue_to_address>
**Action fails when Pubgrade builds images**
When pubgrade is configured with `endpoints.builds.enabled` set to `true`, `register_builds` uses its native-build path when Pubgrade builds are enabled, and `start_build` then requires `location` and `dockerhub_token`; this action sends neither, so Pubgrade rejects the request and the caller does not register the prebuilt image.
Document and enforce that the action requires Pubgrade builds to be disabled, or add API support for registering prebuilt images when builds are enabled.
</issue_to_address>
### Comment 4
<location path="actions/notify/notify.sh" line_range="159" />
<code_context>
+ # Pubgrade's errors are problem details (`title`, `detail`)
+ detail="$(jq -r '[.title, .detail] | map(select(. != null)) | join(": ")' \
+ "${response}" 2> /dev/null | head -c 500 || true)"
+ give_up "Pubgrade answered ${code}${detail:+: ${detail}}"
+ ;;
+ *)
</code_context>
<issue_to_address>
**Invalid requests can succeed**
When `fail-on-error` is `false` and Pubgrade rejects a malformed image, undeclared build type, or other invalid request field, `give_up` emits a warning and exits 0 for Pubgrade rejections, so invalid values the script leaves to Pubgrade—such as a malformed image or undeclared build type—make the action succeed without registering a build, despite its promise that invalid inputs always fail.
Validate request fields locally or make known validation rejections fail regardless of `fail-on-error`.
</issue_to_address>
### Comment 5
<location path="actions/notify/notify.sh" line_range="72" />
<code_context>
+# so it starts with a letter or digit (no `.` or `..`); the token goes into
+# curl's configuration in quotes, so it has no quotes, backslashes or line
+# breaks (and isn't printed)
+if [[ ! "${repository_id}" =~ ^[A-Za-z0-9][A-Za-z0-9._~-]*$ ]]; then
+ fail "Invalid input 'repository-id': letters, digits, '.', '_', '~' and '-' only, starting with a letter or digit"
+fi
</code_context>
<issue_to_address>
**Impossible identifiers are accepted**
When a caller supplies an uppercase letter, `_` or `~` in a repository ID, or `_` in a token, and sets `fail-on-error` to `false`, the local regexes accept characters Pubgrade cannot generate for these values, so the request reaches Pubgrade and is rejected. `give_up` exits successfully, and the workflow reports success without registering a build.
Restrict the repository ID and token regexes to the characters Pubgrade can generate for each value.
Also at `actions/notify/notify.sh:75`.
</issue_to_address>
### Comment 6
<location path="actions/notify/notify.sh" line_range="80" />
<code_context>
+fi
+# Pubgrade notifies subscribers of the image's tag, so it needs a tag and
+# takes no digest; it checks the full reference itself
+if [[ "${image}" == *@* || ! "${image##*/}" =~ :. ]]; then
+ fail "Input 'image' needs a tag and no digest, e.g. 'org/image:1.0.0'"
+fi
</code_context>
<issue_to_address>
**Invalid tagged images succeed**
When `fail-on-error` is `false` and an image has a tag-shaped suffix but fails Pubgrade’s image-reference validation, the shallow check accepts the image, Pubgrade rejects it, and `give_up` exits successfully with a warning. The caller sees a successful step for an invalid image input.
Validate the full image reference locally before sending the request.
</issue_to_address>
### Comment 7
<location path="actions/notify/notify.sh" line_range="89-90" />
<code_context>
+ || fail "Input 'url' must start with https:// or http://"
+else
+ protocols="=https"
+ [[ "${url}" =~ ^https:// ]] \
+ || fail "Input 'url' must start with https:// (or set 'allow-http')"
+fi
+
</code_context>
<issue_to_address>
**Malformed URLs only warn**
When `fail-on-error` is `false` and `url` has the required scheme but is otherwise malformed, the scheme check accepts malformed values such as `https://bad host`, so `curl` rejects the request and `give_up` exits successfully with a warning when `fail-on-error` is false; the caller sees an invalid input treated as success.
Validate that `url` is a well-formed base URL before making the request, and fail that validation regardless of `fail-on-error`.
</issue_to_address>
### Comment 8
<location path="actions/notify/notify.sh" line_range="142" />
<code_context>
+ 0:200)
+ break
+ ;;
+ 6:* | 7:* | 0:503)
+ if [[ "${status}" -eq 0 ]]; then
+ reason="Pubgrade answered ${code}"
</code_context>
<issue_to_address>
**Other 5xx retries**
The issue asks for retries on 5xx responses, but the action retries only 503 responses (as well as connection failures). Other 5xx responses are not retried.
</issue_to_address>
### Comment 9
<location path="actions/notify/notify.sh" line_range="80-81" />
<code_context>
+fi
+# Pubgrade notifies subscribers of the image's tag, so it needs a tag and
+# takes no digest; it checks the full reference itself
+if [[ "${image}" == *@* || ! "${image##*/}" =~ :. ]]; then
+ fail "Input 'image' needs a tag and no digest, e.g. 'org/image:1.0.0'"
+fi
+if [[ "${allow_http}" == "true" ]]; then
</code_context>
<issue_to_address>
**Digest image references**
The issue prefers an image input with a digest, but this validation rejects digests and requires a tag. The PR explains that Pubgrade currently accepts only tagged images, so this request remains unmet.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 9 findings to address first, and if the action sends the wrong image or Git reference, Pubgrade can persist that build and notify subscribed deployments; those notifications or any resulting rollout cannot be undone by reverting this change. The action also sends a repository access token in an authenticated request, though it masks the token and avoids placing it on the command line.
Blocking findings: actions/notify/notify.sh:142, actions/notify/notify.sh:133, actions/notify/notify.sh:114, actions/notify/notify.sh:159, actions/notify/notify.sh:72, and 4 more
A 503 from a proxy may also come after Pubgrade received the request (e.g. Envoy on an upstream reset), so the action retries only when it can't connect at all. Reject '_' in repository IDs and tokens, which Pubgrade never generates, and fail on URLs that curl can't parse, whatever fail-on-error says. Document that the action needs builds switched off, and which inputs it checks itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the composite action
actions/notify, which notifies Pubgrade of a new image from CI: it registers a prebuilt image as a build of a repository (POST /repositories/{id}/builds), and Pubgrade then notifies the repository's subscriptions. Callers reference it aselixir-cloud-aai/Pubgrade/actions/notify@<commit> # <version>, as decided in #88.url,repository-id,tokenandimage(required);build-type,git-tag,git-branch,git-commit,fail-on-error(defaulttrue) andallow-http(defaultfalse). Output:build-id.allow-httpis set. All inputs reach the script through env, never interpolated into it, and the messages it prints are escaped for the runner.fail-on-errorsays (required inputs, a URL curl cannot parse, the formats of the ID, the token and the image,git-commitin a tag run). The ID and the token must use the characters Pubgrade generates (pubgrade/charsets.py), and the image needs a tag.head_commit: taken from the run's tag, or else from its branch and commit, unless thegit-*inputs set it. In a run for a tag,git-commitneedsgit-branch.IMAGE_WITH_TAGinbuilds.py), soimagetakesname:tag.notify/ "Notify action" runs the action, as callers do, against Pubgrade built from the branch's image. It covers a registered build, a wrong token that fails, and a wrong token withfail-on-error: falsethat only warns.CI passedandpublishneed the new job. The app start moves from the smoke job totests/start.sh, which both jobs use.docs/usage.md(usage, inputs, retries, runs for pull requests), and notes indocs/development.md(tests, CI).The action needs Pubgrade with builds switched off (the default). Adoption in tesk-api is tracked in elixir-cloud-aai/tesk-api#157 and needs a Pubgrade release first.
Verification
pytest --cov: 756 passed, 100% coverage (41 of the tests are intests/test_notify_action.py, which runs the script against a stub of Pubgrade).pre-commit run --all-filespassed: ruff, mypy, actionlint, gitleaks and the file checks.mkdocs build --strictpassed.notify.shran against a real Pubgrade started withtests/start.sh(local image build):SUCCEEDED, the image and the branch and commit as sent.::error::Pubgrade answered 401: Unauthorized: …and a failed step.fail-on-error=falsegave a warning and exit 0.notifyrepeats these checks with the final version.adversarial-reviewerandquality-reviewerran on the branch. The fixed findings were 502/504 retries, message escaping,git-commitin tag runs, test gaps, the job id, CI check messages, wrapping and docs. A second adversarial round confirmed these fixes; its low findings are fixed too: each line of the token is masked, the retried failures are documented precisely (incl. the remaining risk of 503s from proxies like Envoy), and the git-branch description is corrected._rejected in IDs and tokens, unparsable URLs fail, builds-off requirement and checked inputs documented. The others were answered with reasons (see below); all threads are resolved.Declined review findings
tests/notify.shscript (quality, optional): the job runs the action withuses:, as callers do, which a script can't do. Its checks now print error messages instead.action.ymlto avoid duplicating the docs (quality, low): GitHub and the Marketplace show these descriptions on their own, so they stay complete.image(Sourcery, from the issue): Pubgrade's API takes tags only.fail-on-errorwhen Pubgrade rejects the request (Sourcery):fail-on-error: falseis meant to let a release go on whatever Pubgrade answers; the step still warns.Open questions
None.
Closes #88
🤖 Generated with Claude Code