DOC-6951 Detect missing aliases after merge and open a fix PR - #3770
Open
andy-stark-redis wants to merge 9 commits into
Open
DOC-6951 Detect missing aliases after merge and open a fix PR#3770andy-stark-redis wants to merge 9 commits into
andy-stark-redis wants to merge 9 commits into
Conversation
Contributor
🧠 Redis MemoryFound 6 related items from repository history:
Memory updated at 49025d2 |
Contributor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
🧠 Redis MemoryFound 5 related items from repository history: Memory updated at 4fda01f |
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 9bd76a0 |
andy-stark-redis
force-pushed
the
DOC-6951-alias-workflow
branch
from
August 7, 2026 15:57
9bd76a0 to
4b5b62a
Compare
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 4b5b62a |
Automates remembering to run the alias scanner, which is the only thing item C was ever about. The scan itself has been available as make check_aliases since the tooling landed. Post-merge on purpose, rather than the pull_request check originally planned. A PR-time check has to choose between being ignorable and being intrusive, and neither is worth it here: the fix is always the same mechanical edit, so author intent almost never matters, and the cases where it might -- a page retired rather than moved -- are already the ones the scanner refuses to touch. Running on every push to main instead costs about three seconds, needs no Hugo build, and keeps the window where an old URL 404s down to minutes rather than the days a scheduled-only sweep implies. Nobody sees anything on their own PR. The branch is regenerated from main on every run and force-pushed, so the PR always means the same thing: current main plus every alias currently missing. That makes repeated runs idempotent and stops a stale half-fix accumulating on a long-lived bot branch. An earlier draft tried to reuse and extend the existing fix branch, which meant switching branches with a dirty working tree -- fragile for no benefit. fetch-depth: 0 is load-bearing rather than tidy. The scanner reads git rename records, and in a shallow clone it finds none, reports zero moves and exits 0. Verified against a --depth 1 clone: a permanent green tick that never examines anything, which is the worst possible failure for a check like this. No other workflow in the repo sets it, and repo-memory.yml has already been caught by the same default once, needing fetch-depth: 2. Uses gh pr list --head rather than the gh search prs idiom the sync workflows use, because search goes through an eventually-consistent index and can miss a PR opened moments earlier, which would produce duplicates. Dry-run in a fresh clone: the scan and the exact git sequence produce a 202-file, 430-insertion commit on the fix branch, matching the backfill in #3769 line for line. Learned: checkout defaults to a depth-1 clone, in which this scanner silently passes; any history-reading check needs fetch-depth 0 and a test that it fails when it should Constraint: the fix branch is rebuilt from main and force-pushed every run, never extended, so the PR always represents main plus all currently-missing aliases Rejected: a pull_request check with annotations | intent almost never matters for a mechanical alias, and the cases where it would are already never auto-fixed Directive: merge this after the backfill in #3769, or its first run opens a competing PR containing the same 256 aliases Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five Bugbot findings, all valid, and one of them was a certainty rather than a risk. The PR body embedded the whole scanner report, and a full report runs to 117,912 characters against GitHub's 65,536 limit for a body. So gh pr create would have failed on the very first run, after the branch had already been force-pushed -- leaving a pushed branch and no PR. The report is now trimmed to 40,000 characters with a note saying so, which puts the body around 41,000, and the untrimmed version stays in the run log. Two findings were about concurrency. Overlapping runs both force-push the same branch, so a slower run built from an older main could replace a newer commit; and both could pass the gh pr list check before either created a PR, so the loser would fail under set -e and redden a run on main. There is now a concurrency group that cancels an in-flight older run, since the newest run is always the one whose answer we want, and PR creation tolerates losing the race when a PR turns out to exist. A run cancelled between the push and the create is self-healing: the next one force-pushes again and finds no open PR. The checkout now pins ref: main. A manual dispatch from another branch would otherwise have scanned that branch while still opening a PR against main, so the head would have carried unrelated commits. The last one is the same shape as a defect Bugbot found in the scanner earlier, which I had fixed only halfway. --fix can decline to edit a file, and I had made that visible in the exit code but only under --fail, which the workflow did not pass. So a refused file left the working tree clean for that page and the run printed that there was nothing to do. The workflow now passes --fail, captures the status through the tee with PIPESTATUS, and errors when the fixer declined something it could not put in a PR -- a red run being the only channel anyone would notice for a case with nothing to review. Verified the exit-code contract the workflow now depends on: --fix --fail exits 0 when nothing was skipped and 1 when something was, and --fail without --fix exits 1 while gaps remain. Both branches of the new shell gate were exercised directly. The skipped-file path inside the scanner itself is currently unreachable with this corpus, so that half is reasoned from the code rather than observed. Learned: the PR-body size limit turned a design that looked fine into one that could never have worked once, and only measuring the artifact showed it -- 117,912 characters against a 65,536 cap Constraint: the fix PR body must stay under GitHub's 65,536-character limit, so the scanner report is trimmed and the run log holds the full version Constraint: the workflow always checks out main, whatever ref triggered it, because the fix branch and the PR base both assume main Gaps: the scanner's own skipped-file exit path is unexercised, since no file in this corpus is refused any more Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two more Bugbot findings, both about the bot PR drifting from what it claims. The description was written once and never rewritten. A later run force-pushed a new commit to the branch, so the diff moved but the body kept the previous run's scanner report -- including its skip and collision notes, which are exactly what a reviewer reads to decide. Both the already-open and the lost-the-race paths now refresh it with gh pr edit, so the body always describes the commit beneath it. The other is the case where nothing is missing any more. The clean-scan path exited without looking for an open PR, so if the aliases reached main some other way -- by hand, or inside someone else's PR -- the bot's PR stayed open with a diff that had become redundant, inviting someone to merge a stale set of edits weeks later. It is now commented and closed with its branch deleted, and the next run opens a fresh one if anything is missing again. Restructuring for that meant looking up the open PR once at the top rather than just before creating one, which also removed a duplicated gh pr list call. All five paths of the step were exercised as shell: clean with no PR, clean with a PR, changes with no PR, changes with a PR, and the refused-file error. The GitHub calls themselves are still unexercised until this is on main. Learned: a bot PR has a lifecycle, not just a creation -- the interesting bugs were in refreshing it and retiring it, neither of which the first version considered Constraint: whenever the fix branch is force-pushed the PR description is rewritten, because it embeds a scanner report that a reviewer uses to make decisions Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The open PR was looked up once at the top of the step and then acted on minutes later. Someone can merge or close it inside that window, and closing an already-merged PR is an error, so set -e would have reddened a run on main for something entirely harmless -- or, on the other path, quietly rewritten the description of a PR nobody will read again. The state is now confirmed immediately before each use, and anything other than OPEN is treated as "there is no PR", which lets the ordinary paths take over: open a fresh one, or do nothing. The gh calls stay tolerant of failure anyway, because checking the state narrows the race window without closing it, and a failed comment on a PR that has just been merged is not worth a red run. The companion change to the scanner means exit 2 now identifies a failed scan, so the step's comment no longer implies that a non-zero exit can only mean skipped files. Verified as shell across all four PR states -- open, merged, closed, absent -- with set -e active, confirming the fall-through does not abort, and that a failing tolerant call leaves the run going. Learned: a status read at the start of a job is a guess by the time the job acts on it, and "closed" needs to route into the no-PR path rather than into an error Constraint: confirm the fix PR is still open immediately before commenting, closing or editing it, and keep those calls non-fatal regardless Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ntradicting the diff Two Bugbot findings on the workflow, one of which could have touched somebody else's pull request. gh pr list --head matches on branch name alone, across forks. A contributor whose branch happened to be called auto/missing-aliases would have matched, and this job goes on from that lookup to comment on, close with --delete-branch, or rewrite the description of whatever it found. Filtering on isCrossRepository restricts it to pull requests whose head is in this repository. The lookup existed in two places, the initial one and the lost-the-race fallback, and only one would have been fixed if I had patched them where they sat, so it is now a single function used by both. The other is a wording problem with a real consequence. report() runs before apply_fixes(), so in --fix mode the report describes gaps the same run is about to close -- and that report is embedded verbatim in the pull request the automation opens. A reviewer reading "add this alias to that file", next to a diff that already contains it, goes looking for work that is done. The report now knows whether a fix follows and says "added" rather than "add", drops the instruction to run the fixer, and words its annotations to match. Covered by a test that captures the log in both modes, since this is the kind of thing that reads fine to whoever wrote it. Also rebased onto main now that the scanner has landed, so this PR is four workflow commits rather than eleven. Learned: a lookup by branch name reaches across forks, so automation that closes or rewrites what it finds needs to check the head repository, not just the branch Constraint: the bot PR lookup filters on isCrossRepository so it can only ever act on a pull request whose head is in this repository Constraint: in --fix mode the report describes what was done rather than what to do, because it is published verbatim in the pull request the fix opens Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
andy-stark-redis
force-pushed
the
DOC-6951-alias-workflow
branch
from
August 10, 2026 09:48
4b5b62a to
a4be80e
Compare
A one-line wrap missed when the exit-code constants were added. No behavior change. Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two Bugbot findings, the first of them a correction to my own previous fix. I had changed the report to say "added" in --fix mode, because saying "add this alias" beside a diff that already contains it sends a reviewer after finished work. But --fix can decline a file, so "added" was then a lie in the other direction, and a gap that still needs a person would have read as closed. The honest fix is not better wording but better ordering: --fix now runs first and the report is told which files it declined, so each line says added, or COULD NOT add and fix by hand. Neither version of the wording can be right without that, which is why the first attempt was bound to be wrong whichever phrasing I picked. The second is the last untolerated call on a PR number. The lost-the-race path edited the winner's description without allowing for that PR being merged or closed in the meantime, so set -e could have reddened a run on main after the branch was already pushed. Every gh call that mutates a pull request here is now failure-tolerant, which I checked by auditing all four rather than the one that was reported. Learned: the first fix was unfalsifiable as worded -- a report written before the fix runs cannot describe the fix, so no phrasing was going to be accurate and the ordering had to change Constraint: --fix runs before the report, and the report is given the set of files the fixer declined, so it can distinguish an alias that was added from one that still needs a person Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…PR step The refused-file mechanism was unreachable, and it failed in the worst direction. Actions runs `run` blocks with `bash -eo pipefail`. `set -uo pipefail` sets -u and pipefail but does not undo the -e, so the deliberate exit 1 that --fix --fail uses to report a declined file aborted the step on the spot: PIPESTATUS was never read, the `skipped` output was never written, the next step never ran, and every alias the run had successfully fixed was thrown away with no pull request opened. So the one scenario the plumbing existed to surface was the one scenario that discarded the work. errexit now comes off around the pipeline and back on afterwards. Verified under `bash --noprofile --norc -eo pipefail` across all three exits: 0 and 1 both reach the PR step with the status captured, and 2 still stops the run with exit 2. Also guarded the two command substitutions that assign a PR number. Under -e a transient API failure there would redden a run on main; treating it as "no PR" means the worst case is a duplicate that the next run's force push folds back together. Worth noting what this says about the earlier fixes in this PR. The exit-code contract, the --fail flag and the skipped plumbing were all correct and all dead, because the shell aborted before any of them ran. Nothing in the workflow file itself hinted at that -- it needed knowing what shell Actions supplies. Learned: Actions supplies `bash -eo pipefail`, and `set -uo pipefail` reads like it replaces those flags while only adding to them, so a deliberate non-zero exit dies before its status can be captured Constraint: disable errexit around any pipeline whose exit code is then inspected, because the step's default shell aborts on failure before the next line runs Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 218810c. Configure here.
Three Bugbot findings, and two of them are consequences of my own earlier change in this PR rather than of the original design. Reordering --fix to run before the report moved the useful part of the output to the end: the run now opens with a line per alias written, which the diff already shows, and closes with the summary and the categories needing a human decision. The body was still keeping the first 40,000 characters, so on a large report it preserved the noise and cut off everything a reviewer actually needs. Demonstrated on a synthetic report: head -c kept neither the summary nor the collision section, tail -c keeps both. It now trims from the front and says so. The summary line still described the pre-fix state. Every detail line said "added to" while the headline above them said "missing an alias" -- and the headline is what anyone skims. It now reports aliases added and aliases that could not be added, which matches the lines beneath it in all three modes, with a test pinning that. The third is a real gap in the close logic. When the tree is clean because the fixer declined every remaining file, the step errored before reaching the stale-PR path, so an open bot PR whose edits had already landed stayed open. That is the stale-PR case the close logic exists for, whether or not other gaps remain. It now closes the PR -- with wording that says gaps remain rather than implying everything is resolved -- and then fails. Learned: reordering the fix ahead of the report changed which end of the output mattered, and the trim that had been correct became exactly backwards; a change of order is a change to everything downstream that assumed it Constraint: the fix PR body keeps the tail of the report, because --fix runs first and the summary and human-decision sections come last Ticket: DOC-6951 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dwdougherty
reviewed
Aug 10, 2026
dwdougherty
left a comment
dwdougherty
left a comment
Collaborator
There was a problem hiding this comment.
Looks okay to me, but I'll leave the final approval to @paoloredis.
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
2 participants
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.
Automates running the alias scanner — item C of DOC-6951. One new workflow file, nothing else.
Post-merge, not a PR check
Deliberately nothing appears on anyone's PR — no check, no annotation, no comment. It runs on push to
maininstead.A PR-time check has to pick between being ignorable and being intrusive, and neither pays here: the fix is always the same mechanical edit, so author intent almost never matters — and the cases where it might, such as a page retired rather than moved, are exactly the ones the scanner already refuses to touch.
Running per-push costs ~3s and needs no Hugo build, so the window where an old URL 404s is minutes rather than the days a scheduled-only sweep would imply.
mainworkflow_dispatchThe PR it opens
Always means one thing: current main plus every alias currently missing. The bot branch is regenerated from
mainand force-pushed each run, never extended, so repeated runs are idempotent and a stale half-fix can't accumulate.Cases needing judgement are reported in the PR body rather than changed — an old URL that's a live page, a URL another page already claims, or a move onto a draft. Closing the PR is a fine answer when a page was retired on purpose; the scanner will simply propose it again if the dead URL is still reachable, and nothing depends on it.
fetch-depth: 0is load-bearingVerified against a
--depth 1clone: the scanner finds no rename records, reports zero moves and exits 0. A shallow checkout would give a permanent green tick that never examines anything — the worst failure mode for a check like this.No other workflow in the repo sets it, and
repo-memory.ymlhas already been caught by the same default once, needingfetch-depth: 2.Two deviations from the house pattern, both deliberate
gh pr list --headinstead of thegh search prsidiom the seven sync workflows use. Search goes through an eventually-consistent index and can miss a PR opened moments earlier, which would produce duplicates.--headis an exact lookup.--fail. The PR is the signal; a red tick onmainwould be noise for something no build depends on.Review round (
4fda01f09)Five Bugbot findings, all valid. One was a certainty, not a risk: the PR body embedded the whole scanner report, and a full report is 117,912 characters against GitHub's 65,536 limit — so
gh pr createwould have failed on the very first run, after force-pushing the branch. The report is now trimmed to 40,000 chars with a note, putting the body near 41,000; the untrimmed version stays in the run log.concurrencygroup, cancelling in-flight older runsref: mainregardless of trigger--fail, capture status viaPIPESTATUS, error when the fixer declined something with nothing to reviewThat last one is the same defect Bugbot found in the scanner earlier and I'd only half-fixed: I made a refused file visible in the exit code, but only under
--fail, which this workflow wasn't passing.Second review round (
9bd76a07a)Two further findings, both about the bot PR drifting from what it claims:
gh pr editon both the already-open and lost-the-race paths, so the body always describes the commit beneath itThe second matters because the diff becomes redundant, not wrong: if the aliases reach
mainsome other way, a stale bot PR sits there inviting someone to merge it weeks later.All five control paths of the final step were exercised as shell — clean/no PR, clean/with PR, changes/no PR, changes/with PR, and the refused-file error.
Third review round (
b72cfc125on #3767,4b5b62acbhere)>1as a hard errorThe second was a TOCTOU: the PR was looked up at the top of the step and acted on minutes later. Closing an already-merged PR is an error, so
set -ewould have reddened a run onmainfor something harmless. Verified as shell across all four states — open, merged, closed, absent — confirming the fall-through routes into the no-PR path rather than aborting.Verification
Dry-run in a fresh full clone: the scan plus the exact git sequence produces a commit on the fix branch matching #3769. YAML parses, permissions are
contents: write+pull-requests: writescoped to the single job with acontents: readdefault. The exit-code contract the workflow now relies on was checked directly —--fix --failexits 0 with nothing skipped and 1 when something is,--failalone exits 1 while gaps remain — and both branches of the new shell gate were exercised.Not verifiable until it's on
main: the trigger, theconcurrencybehaviour, andGITHUB_TOKENhaving enough scope to push the branch and open the PR. Also unexercised: the scanner's own skipped-file exit path, since no file in this corpus is refused any more — that half is reasoned from the code, not observed. Worth watching the first run.🤖 Generated with Claude Code
Note
Medium Risk
New automation on
maincan mass-editcontent/frontmatter via a bot branch; safeguards are PR-based review and narrow alias-only edits, but a scanner mistake could propose wrong redirects.Overview
Adds a post-merge GitHub Actions workflow (
alias_check) that runs on pushes tomain, a monthly cron, and manual dispatch. It full-history-checkoutsmain, runsbuild/check_missing_aliases.py --all --fix --fail, and force-pushes anauto/missing-aliasesbranch to open or refresh a bot PR whencontent/changes—or closes stale bot PRs when nothing is missing.The workflow handles races and API edge cases: concurrency cancellation,
gh pr list --headwith same-repo filtering, re-checking PR state before close/edit, trimmed scanner output in the PR body, and treating scanner exit>1as a hard failure while exit1means some aliases could not be auto-applied.Scanner/report changes:
main()now applies fixes before reporting so logs and the embedded PR report describe outcomes (added to/COULD NOT add) instead of telling reviewers to add aliases already in the diff. Summary counts match that mode when--fixis used.Tests lock in the new report wording and headline counts.
Reviewed by Cursor Bugbot for commit 49025d2. Bugbot is set up for automated code reviews on this repo. Configure here.