Improve link-check failure DevEx; rename refcache to link-cache#10979
Open
chalin wants to merge 24 commits into
Open
Improve link-check failure DevEx; rename refcache to link-cache#10979chalin wants to merge 24 commits into
chalin wants to merge 24 commits into
Conversation
github-actions
Bot
added
dependencies
otelbot-docs
Bot
added
missing:docs-approval
Pull request dashboard statusStatus last refreshed: 2026-07-26 01:00:52 UTC.
This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. |
Contributor
Author
|
Red/green proof of the
|
chalin
force-pushed
the
chalin-m24-link-check-ux-2026-0722
branch
from
July 24, 2026 13:35
36c81d8 to
f29e6f5
Compare
chalin
marked this pull request as ready for review
July 24, 2026 13:45
chalin
force-pushed
the
chalin-m24-link-check-ux-2026-0722
branch
from
July 24, 2026 16:07
b91360d to
c1f55c2
Compare
chalin
marked this pull request as draft
July 24, 2026 17:18
chalin
changed the title
chalin
marked this pull request as ready for review
July 24, 2026 19:42
The npm scripts, workflows, skills, and docs said `refcache` where they
meant the Lychee link cache (`.lycheecache`). Align naming with the
link-cache package that now provides the tooling:
- npm scripts: `fix:refcache` -> `fix:link-cache`,
`fix:refcache:refresh` -> `fix:link-cache:refresh`, and the `refcache`
passthrough -> `link-cache`
- `/fix:refcache` PR comments still work: pr-fix maps them to
`fix:link-cache` with an INFO notice, following the `/fix:all` compat
precedent
- Skills: `refresh-refcache-pr-fix` -> `refresh-link-cache-pr-fix`,
`resolve-refcache-conflicts` -> `resolve-link-cache-conflicts`; the
site skill page gets a Hugo alias for the old URL
Kept, deliberately: the `refcache` bin name (provided by the link-cache
package), the `refcache-refresh.yml` workflow filename and its
`otelbot/refcache-refresh` branch, and the `{#refcache}` anchor IDs
(URL stability).
When a PR added external links without committing the refreshed .lycheecache, the CHECK LINKS job failed with only a bare `git diff --exit-code` and a hint buried in the step log — the check run page and PR view gave no clue. Contributors read it as a broken link check (open-telemetry#10910). - Emit a `::error` annotation and a step-summary block that name the problem (stale committed cache) and both fixes: comment `/fix:link-cache`, or run `npm run check:links` and commit the result. - Rename the job to `CHECK LINKS and CACHE` so the failing check's name points at the cache, and give the pr-checks.md section an explicit `{#build-and-check-links}` anchor so its URL survives the heading rename. - Tell contributors to run `npm run check:links` before submitting link-adding PRs (link-checking.md), absorbing open-telemetry#10934. Step logic proven locally both ways: a planted one-line cache miss produces the annotation + summary and exit 1; a clean cache passes.
A successful `npm run check:links` quietly rewrites the committed .lycheecache; contributors who don't know to look for it push their PR without the cache change and hit the stale-cache CI failure. Route `check:links` through a small wrapper (scripts/lychee/check-report) that runs the `lychee-norm-cache` bin as before and, when a passing run leaves .lycheecache modified, prints a loud end-of-run notice to commit the cache change. Failing runs keep their exit status. CI is unaffected: the `CHECK LINKS and CACHE` job calls `_check:links` directly and reports cache staleness itself. Proven end to end with a planted one-line cache miss (notice printed, exit 0) and a clean cache (no notice).
A failing `npm run check:links` (or bot `/fix:link-cache` run) ends with lychee's raw output; contributors then reach for cache-side fixes when the real problem is genuinely unreachable links (e.g. PR open-telemetry#10940). Teach the check-report wrapper to parse failures out of the lychee output and end the run with a report that names each dead link and says that nothing cache-side will fix it — pointing at the real fixes: repair or remove the link, or append `?link-check=no` for a URL that blocks checkers. In CI, the report also lands in the workflow step summary. Proven end to end against a planted page with real 404/410 links (report printed, exit status preserved, step summary written) and a clean run (no report).
- pr-actions runs the resolved script on the PR head; heads cut before the rename only have fix:refcache, so mapping the directive to fix:link-cache broke them - post-rename package.json forwards fix:refcache to fix:link-cache, and the base _list:fix:* filter hides the alias from fix:all-style aggregates - a new test guards the directive-to-alias pair
- process.exit() can drop buffered stdout when piped, truncating large dead-links reports under npm or tee
- TIMEOUT, ERROR, and 5xx results can clear on a rerun; say so before pointing at link fixes
- the renamed SKILL.md blob/main URLs only resolve once this PR merges; carrying the two cache entries over keeps CHECK LINKS green in the meantime, via the same trusted-verified mechanism the committed cache already provides
- Restores the two-check pattern from the htmltest era: `CHECK LINKS` answers "are the links valid?" and a new `CACHE updates committed?` job answers "is the committed cache fresh?", so a failing check names its own cause at a glance. - `CHECK LINKS` uploads the refreshed `.lycheecache` as an artifact; the gate job downloads it and diffs it against the committed cache (a missing artifact fails the job — no false green). - Moves the stale-cache guidance in pr-checks.md to its own section (`#cache-updates-committed`) and points the workflow's failure annotation, step summary, and docs cross-references there.
Deliberately stales .lycheecache so CACHE updates committed? fails on CI; to be reverted once the red run is captured.
This reverts commit d024a22.
- Corrects the PR-comment notice and code comments to cite the actual workflow job name, `CACHE updates committed?` (was `CHECK LINKS and CACHE`, a leftover from an earlier design). - Adds wiring-drift guard tests: the job name cited by the notice and the pr-checks anchors cited by workflow/report links must exist in check-links.yml and pr-checks.md. - Gives the `Handling valid external links` heading an explicit id so the anchor guard can verify it (slug unchanged). - Trims a comment in the pr-fix test that restated the compat rationale already documented in index.mjs; clarifies the check:links speed claim in link-checking.md.
- Switches the check:links wrapper from spawnSync to an async spawn that forwards stdout/stderr as they arrive, so the minutes-long run shows live progress instead of buffering everything until exit (lychee runs with no_progress, so per-error lines are the only liveness signal). - Still accumulates the output for the cache-notice scan on failure.
- Drops the duplicated job display name (its home is the job's own `name:` field) and the restated downstream comparison logic (readable in the check-link-cache job itself) - Keeps the one non-obvious fact: the link check refreshes the cache
- The artifact handoff is already stated structurally (artifact name + needs), and the cache refresh is presupposed by the downstream diff
The forwarding alias now echoes a deprecation warning to stderr before running fix:link-cache, so devs invoking the old name by habit learn of the rename. The alias-guard test pins the warning along with the forwarding.
The parser's info messages (/fix:refcache deprecation, /fix:all compat) only reached the extract step's run log, which nobody reads. The report job now re-derives the notice from the comment body via the trusted default-branch parser — same pattern as the hint, and like it a parser constant, never comment text — and patch-report appends it to the outcome comment as a final paragraph.
Both were caller-supplied trusted prose for the outcome comment, differing only in when the caller passes them — a decision that belongs at the parse site, not in the builder's API. The report step now derives a single note with one expression: a parsed directive may carry a compat notice, an unparseable one gets the phrasing hint. The note is always appended as the comment's final paragraph, which moves the hint out of mid-sentence and also surfaces it on outcomes that previously dropped it (e.g. unparseable directive on a closed PR).
- check-report/cli.mjs: correct header comment (CI does use the wrapper via check:links), handle spawn errors, clarify cache-diff comment - check-report/index.mjs: unify dash style in report prose - patch-report/index.test.mjs: drop assert subsumed by endsWith check - ci-workflows.md: document the /fix:refcache compat mapping; name refcache-refresh instead of a dangling 'it' - pr-checks.md: /fix runs 'npm run fix', not fix:all - link-checking.md: defer recovery steps to pr-checks.md; note that the site build dominates check:links run time - content-module-patches.md: realign comment column
One home per fact: - check-links.yml: step summary defers stale-cache remedies to pr-checks.md#cache-updates-committed; the ::error annotation keeps the one runtime copy - refcache-refresh.yml: home the rename-boundary decision (workflow and branch keep historical refcache names) in the header comment - link-checking.md: drop the fix-and-test:all composition paragraph (npm-scripts.md footnote owns it); link the table's Housekeeping row - pr-fix/index.test.mjs: assert forwarding+warning properties instead of hardcoding package.json's script string Leanness: - check-report/index.mjs: comments state contracts, not the strings below - check-report/index.test.mjs: wiring guard points at cli.mjs for the why - pr-actions.yml: note comment keeps only the trust rationale - link-checking.md: drop aside restating the cache-scope paragraph Nits: /fix list sort order (pull-requests.md); DIRECTIVE_HINT doc says --note, not hint (pr-fix/index.mjs)
The Markdown summary carries the remedy menu (links render there); the plain-text ::error annotation shrinks to cause + pointer.
chalin
force-pushed
the
chalin-m24-link-check-ux-2026-0722
branch
from
July 24, 2026 19:43
a0500ca to
01385e2
Compare
chalin
enabled auto-merge
July 24, 2026 20:26
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
1 participant
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.
refcache-refresh.ymlworkflow andotelbot/refcache-refreshbranch names (operationally entrenched), the upstreamrefcache/lychee-norm-cachebin names, and{#refcache}anchors (inbound links)./fix:refcacheworking on both pre- and post-rename PR heads (forwards post-rename), with deprecation notices in the bot reply and on local runs of the kept npm alias..lycheecachein place, so CHECK LINKS stays green here even though the renamedblob/mainpaths only resolve at merge.CHECK LINKSinto a newCACHE updates committed?job — each red check names its own cause, restoring the at-a-glance clarity of the htmltest-eraREFCACHE updates?gate.check:linksruns with a commit-the-cache notice when the run refreshed.lycheecache, instead of printing nothing actionable.