Skip to content

Let GitHub title promoted pull requests, and link digests and ranges - #136

Merged
Peeja merged 13 commits into
mainfrom
claude/happy-bell-cfh5qb-promote-links
Oct 13, 2026
Merged

Peeja merged 13 commits into
mainfrom
claude/happy-bell-cfh5qb-promote-links

Conversation

@Peeja

@Peeja Peeja commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Written by Claude.

Addresses bajtos's comment on #117: #117 (comment)

Problem

In the promotion pull request (#129), each line of a service's change list showed its title twice, and its commit hash was plain text. The script wrote each line as the commit's title, then a bare cross-repository reference (fil-forge/piri#147), then a bare short hash (a6e6377):

  • GitHub renders a bare reference as the title of the pull request it names. Promote dev's images to staging #129's rendered body gives those references the issue-link js-issue-link class, which marks them for that. The line already started with the same title.
  • A bare short hash from another repository isn't autolinked.

Change

In scripts/ci/promote-staging.sh:

  • Change list (changes()): a squash-merged commit, whose title ends in (#123), is now written as only the bare reference to the service's pull request, and GitHub supplies the title. A commit that came from no pull request keeps its own title, in a code span, so nothing in it is read as a reference, a mention or a closing keyword. Each commit hash links to the commit. The list says so when it's empty or when the compare API cut it short.
  • Commit range: links to the service's compare view.
  • Digests (digest_link()): each links to its version's page on ghcr.io, looked up through the packages API, which is read once per service. If the lookup fails, the run logs a warning. If it fails or finds nothing, the digest links to the bump pull request that first pinned it on dev, which names the source commit and the publish run. If no bump on main carried it either, it stays plain text.

Demo

The piri section of the message, trimmed to three of its 13 lines. Before is #129's body now. After is --dry-run output on main at 8041b66, from the Actions run in #137.

Before

sha256:c16a9f9 → sha256:b6f7371

What fil-forge/piri brings over staging's pin, 7ba7d94..d20dcec:

After

sha256:c16a9f9 → sha256:b6f7371

What fil-forge/piri brings over staging's pin, 7ba7d94..d20dcec:

Testing

  • make check-shell passes.
  • Ran --dry-run in Actions, with the fil-forge-bot token and the GitHub CLI that promote-staging.yml uses, in the scratch pull request Scratch: can fil-forge-bot read ghcr.io package versions? #137 (job). Both services' sections came out as the After above, with every digest linked to its ghcr.io version page.
  • Scratch: can fil-forge-bot read ghcr.io package versions? #137's probe job also found that the fil-forge-bot installation lacks the Packages permission: minting a token that asks for permission-packages: read fails with "The permissions requested are not granted to this installation." Its default token and GITHUB_TOKEN still read both packages' versions. Speculation: that is because the packages are public. If either goes private, the lookup fails and the digest falls back to the bump pull request link.
  • Ran the fallback here, where the packages API is out of reach: a digest a bump carried linked to its bump pull request, and an unknown one stayed plain.
  • Ran changes()'s jq on made-up commits: a title from no pull request came out in a code span with its backticks swapped for quotes, a CRLF squash title still became a bare reference, an empty compare said so, and a cut-short one counted what it left out.
  • Ran --dry-run here on main at bb08740 after the round-1 fixes: both services' change lists came out as bare references, and with the packages API out of reach each service logged one warning and fell back to its bump pull requests.
  • Not checked: how a rebuilt Promote dev's images to staging #129 renders. The next push to main after this merges, this merge included, rebuilds it.

✴️

🤖 Generated with Claude Code

https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ


Generated by Claude Code

@Peeja Peeja changed the title Link each promoted commit and pull request explicitly Let GitHub title promoted pull requests, and link digests and ranges Oct 6, 2026
Peeja pushed a commit that referenced this pull request Oct 6, 2026
claude and others added 3 commits October 8, 2026 14:48
The promotion pull request's change list wrote each squash-merged title's
pull request as a bare cross-repository reference and its commit as a bare
short hash. GitHub renders a bare reference as the title of the pull
request it names, which repeated the title each line already starts with,
and does not link a bare hash from another repository. Write both as
explicit Markdown links instead: the reference keeps its owner/repo#N text,
and the hash links to the commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Petra Jaros <peeja@peeja.com>
Each squash-merged commit in a service's change list is now just a bare
reference to its pull request, which GitHub renders as that pull
request's title, instead of the commit's title followed by a reference
GitHub expanded to the same title again. Commits that came from no pull
request keep their own title.

Each digest links to the bump pull request that first pinned it on dev,
which names its source commit and publish run, and each commit range
links to its compare view.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Petra Jaros <peeja@peeja.com>
The packages API maps a digest to its version's page; the bot token can
read it. When the lookup fails or finds nothing, the digest links to the
dev bump pull request as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Petra Jaros <peeja@peeja.com>
@Peeja
Peeja force-pushed the claude/happy-bell-cfh5qb-promote-links branch from ab7102d to 4f21357 Compare October 8, 2026 14:49
@Peeja
Peeja force-pushed the claude/happy-bell-cfh5qb-promote-links branch from 72cd8f8 to 9e01d99 Compare October 8, 2026 15:02
@Peeja
Peeja marked this pull request as ready for review October 9, 2026 18:02
Copilot AI balanced review requested due to automatic review settings October 9, 2026 18:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Peeja commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (1st round, d69107f)

Nothing blocking; two should-fix findings in digest_link, and some nits, most of which predate this PR.

Verified: shellcheck 0.11.0 passes on scripts/ci/*.sh (what make check-shell runs), and the SC2016 disable is justified. Running the new jq on the real piri compare 7ba7d94...d20dcec reproduces the Demo's "After" exactly. A full --dry-run in a scratch clone exited 0, with correct change lists and compare links, and the digest fallback produced bump PRs that match git log -S on dev's versions.env. The digest spliced into the jq filter was already validated as sha256:[0-9a-f]{64}, and REPO goes in through env. promote-staging.yml is unchanged, so permissions are not widened. Not checked: the ghcr.io html_url format and digest-to-version mapping (the packages API returned 403 from the review sandbox), and how a rebuilt #129 renders.

  1. should-fix: a failed ghcr lookup leaves no trace (scripts/ci/promote-staging.sh:96-97). 2>/dev/null drops gh's error and the function quietly links the bump PR instead. If a package goes private or the token changes, every digest falls back with nothing in the log. Print a ::warning:: before falling back.
  2. should-fix: the full package version list is fetched four times per run (:96). digest_link runs twice per moving service and pages through every version each time, on every push to main. The list only grows. Fetch it once per service.
  3. nit: titles that don't end in (#N) are copied raw, so a bare #12 resolves to an infra-nodes issue and @name pings someone (:125-130; predates this PR, but the new comment at :116-120 implies wider coverage).
  4. nit: a CRLF commit message leaves \r on $title, so the \)$ anchor misses (:126).
  5. nit: an empty compare range still prints the heading, compare lists cap at 250 commits silently, and a failed changes() call puts gh's error JSON in the body (:176-181; predates this PR).
  6. nit: nothing tests the script beyond shellcheck; a jq run against canned compare JSON would cover the formatting.

✴️ Claude Code session


Generated by Claude Code

…ay titles

Review round 1 on this PR:

- A failed ghcr.io lookup was silent: gh's error went to /dev/null and the
  digest quietly linked its bump pull request. It now leaves a ::warning::.
- Each run paged through a service's whole version list once per digest,
  and the list only grows. It is now fetched once per moving service, in
  the main loop, since digest_link runs in a subshell that would lose it.
- A commit title that isn't a squash-merge's was copied raw, so a "#12"
  in it pointed at this repository and an "@name" pinged someone. It now
  goes in a code span, so it is never read as a reference, a mention, a
  closing keyword or formatting.
- A CRLF title kept its "\r", so a squash commit's "(#N)" went unseen.
- The change list said nothing when it was empty or when the compare API
  cut it short, and a failed call could leave gh's error JSON in the body.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

The fixes are in cabb076.

  1. Fixed. A failed versions lookup now logs a ::warning:: naming the service and saying its digests fall back to bump pull requests. gh's own error message is no longer discarded.
  2. Fixed. load_versions fetches each moving service's version list once, in the main loop. digest_link looks digests up in that list. It has to run in the main loop because digest_link's callers run it in a subshell, which would drop a cache filled there.
  3. Fixed, differently from the suggestion. Instead of qualifying references and escaping @, a title that isn't a squash-merge's now goes in a code span, with any backticks swapped for single quotes. That stops it being read as a reference, a mention, a closing keyword or formatting. Qualifying #12 would have turned fixes #12 into a cross-repository closing reference.
  4. Fixed: rtrimstr("\r") on the title.
  5. Fixed:
    • An empty compare now writes "No commits: both pins were built from the same commit."
    • A compare cut short writes how many commits it left out.
    • The change list is only written once the call has succeeded, so gh's error JSON can't reach the body.
    • The .. label beside the ... URL stays: ... is right for the URL, and the label is the usual way to write a range.
  6. Stays as is. The repo has no shell test harness, and adding one would widen this PR. Instead I ran the jq against canned compare JSON covering each case above, and ran a full --dry-run.

Checks:

  • shellcheck scripts/ci/*.sh and bash -n pass.
  • The jq produced the expected output on canned JSON: a squash commit, a stray title with backticks, @, * and #12, a CRLF squash commit, an empty compare, and a truncated one.
  • --dry-run on main at bb08740: both change lists came out as bare references. Each service logged one warning and fell back to its bump pull requests, because the packages API isn't reachable from here.

Round 1 had should-fix findings, so a round 2 follows.

✴️ Claude Code session


Generated by Claude Code

Peeja commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (2nd round, cabb076)

cabb076 fixes all five round-1 findings. The new "No commits" line adds one should-fix, and there are two nits.

Verified:

  • shellcheck scripts/ci/*.sh and bash -n pass.
  • promote-staging.yml is untouched. Its runner is ubuntu-latest, so bash 5 supports declare -A.
  • Checked in bash 5.2:
    • if ! VERSIONS[$service]=$(…) takes the warning branch on failure and drops the partial output.
    • ${VERSIONS[$service]-} is safe under set -u.
  • --dry-run exits 0. Each service logged exactly one warning and fell back to its bump PRs.
  • The new jq on the live piri compare 7ba7d94...d20dcec matches the Demo's "After".
  • Canned JSON covered:
    • CRLF squash titles;
    • stray titles containing backticks, #12, @bob, *b* and <img>;
    • "Merge pull request #9" titles;
    • a 300-commit compare cut short;
    • an error body, which the if list=$(…) guard keeps out of the body.

Not run: the ghcr.io lookup (403 here), and how GitHub renders the body.

  1. should-fix: "No commits: both pins were built from the same commit." also prints when dev's pin is behind staging's (scripts/ci/promote-staging.sh:149-150).
    • Evidence: the reversed compare d20dcec...7ba7d94 returns status: "behind", behind_by: 13, total_commits: 0 and no commits. I re-ran that call myself.
    • Effect: a reverted dev bump would produce a promotion that rolls staging back, while its body says nothing changes.
    • Fix: branch on .status.
  2. nit: a null html_url would print as the string null, and the digest would link to it (:102, :113). This probably never happens, because GitHub sets the field.
  3. nit: an empty first line in a commit message renders as two literal backticks (:147). Git refuses empty messages by default.

✴️ Claude Code session


Generated by Claude Code

Review round 2 on this PR: the compare API returns no commits when dev's
pin is behind staging's, as after a reverted dev bump, and the list then
said both pins were built from the same commit while the promotion would
roll staging back. The line now follows the compare's status: identical,
behind (merging rolls staging back) or diverged (merging drops staging's
own commits). A version with no page is skipped rather than linked to
"null", and a commit with an empty title reads "(no title)".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

The fixes are in 994ddd7.

  1. Fixed. The note under the change list now follows the compare's status:

    • identical keeps "both pins were built from the same commit";
    • behind says dev's pin is N commits behind staging's, so merging rolls staging back;
    • diverged says staging's pin also has N commits that dev's lacks, which merging drops.

    The cut-short note is now separate, so it can appear alongside any of these.

  2. Fixed. Versions without an html_url are skipped (select(.html_url)), so a digest falls back instead of linking to null.

  3. Fixed. A commit whose first line is empty now reads "(no title)".

Checks:

  • shellcheck scripts/ci/*.sh and bash -n pass. The apostrophes in the new lines are jq ’ escapes, so shellcheck sees no Unicode quotes.
  • The jq gave the right line for each case:
    • the live reversed compare d20dcec...7ba7d94 gives "No commits forward: dev's pin is 13 commits behind staging's, so merging this rolls staging back";
    • the live 7ba7d94...d20dcec is unchanged;
    • an identical compare, and a canned diverged, cut-short compare with an empty title, both came out as expected.
  • CI is green.

Round 2 had a should-fix, so a round 3 follows.

✴️ Claude Code session


Generated by Claude Code

Review round 3 on this PR: the behind and diverged notes read "1 commits",
and the cut-short note followed the diverged one, where it read as more of
staging's dropped commits rather than more of the list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

Peeja commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Claude reviewer says… (3rd round, 994ddd7)

The round-2 finding is fixed. Nothing blocking or should-fix; two wording nits.

Verified:

  • shellcheck and bash -n pass.
  • Every compare status, against live piri compares:
    • ahead (7ba7d94...d20dcec): 13 bare refs and no status line, matching the PR body's After.
    • behind (d20dcec...7ba7d94): the rollback note, with 13 commits.
    • diverged (a real branch, behind by 126): one title, then the "drops" note. behind_by is the right count, since from is staging's commit.
    • identical: the same-commit note.
  • ’ renders as an apostrophe.
  • An error body never reaches the body: gh exits 1, and jq fails on .commits[] before printing.
  • --dry-run at the head exits 0 and falls back to bump PRs, with one warning per service.
  • Nothing touched bot/promote-staging or Promote dev's images to staging #129.

Not run: the ghcr.io lookup (403 here), and GitHub's rendering.

  1. nit: behind_by: 1 gives "1 commits", in both the behind and diverged notes (scripts/ci/promote-staging.sh:153,155).
  2. nit: the cut-short note prints after the diverged note, where "…and 298 more" reads as more of staging's dropped commits (:151-160).

✴️ Claude Code session


Generated by Claude Code

Peeja commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Claude author says…

Both nits are fixed in 6c08b13:

  1. Plural: the count now reads "1 commit" or "N commits".
  2. Order: the cut-short note now comes straight after the list, before the status note.

Checks:

  • shellcheck scripts/ci/*.sh passes.
  • The jq gave the expected output on the live 7ba7d94...d20dcec (13 lines, unchanged), d20dcec...7ba7d94 (13 commits behind) and d20dcec...d20dcec (identical) compares.
  • A canned diverged, cut-short compare with behind_by: 1 now reads "…and 299 more" and then "1 commit that dev’s lacks".

This round found only nits, so there won't be another.

✴️ Claude Code session


Generated by Claude Code

claude and others added 5 commits October 12, 2026 15:15
Brings in 878acad (Bump piri on dev to sha256:f4e3889, #149).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Brings in fa4384f (Bump ingot on dev to sha256:3158cdb, #150).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Brings in 3638753 (Bump ingot on dev to sha256:e525b38, #151).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Petra Jaros <petra@fil.org>
Claude-Session: https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ
@Peeja
Peeja merged commit d453148 into main Oct 13, 2026
4 checks passed
@Peeja
Peeja deleted the claude/happy-bell-cfh5qb-promote-links branch October 13, 2026 19:23
bajtos added a commit to fil-forge/infra-central that referenced this pull request Oct 15, 2026
* Let GitHub title promoted PRs, and link digests

A promotion pull request listed each squash-merged commit as its title
followed by a cross-repository reference, which GitHub expanded into
the same title again as a link, so every line said everything twice.
Digests, commit hashes and the commit range were plain text.

Port fil-forge/infra-nodes#136, which fixed the same list there:

- A squash-merged commit is a bare reference to its pull request,
  which GitHub renders as that pull request's title. Any other commit
  keeps its own title, in a code span, so nothing in it is read as a
  reference, a mention or a closing keyword.
- Each hash links to its commit, and each range to its compare view.
- Each digest links to its version page on ghcr.io, or to the dev bump
  that carried it when the packages API has no page for it.
- The list says when the source's pin is behind the target's or has
  diverged from it, and when the compare API cut it short.

Unlike infra-nodes, this script pages through the compare API, and
every page repeats the totals and the status. The pages are read as
one array, with jq, so each note is written once. The ghcr.io package
is named after the service's repository rather than its pin key, and
the version list sits in a plain variable rather than an associative
array, so the script still runs under macOS's bash 3.2.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

* docs: name every jq variable in the SC2016 comment

The comment justifying the shellcheck exception listed four of the jq
program's variables and missed $compare, $commits and $behind.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>

---------

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
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.

3 participants