Skip to content

check-readme-links only reads README.md, so a broken link in CONTRIBUTING.md passes every gate #352

Description

@vyncint

Today.github/scripts/check-readme-links.sh is the only link gate, and
ci.yml runs it as .github/scripts/check-readme-links.sh with no argument,
so it reads README.md and nothing else. Measured on #351: a link to
https://github.com/vyncint/temlens/labels/good%20first%20issue (missing the
r) landed in CONTRIBUTING.md and all sixteen checks reported success. The
URL 404s; curl -o /dev/null -w '%{http_code}' returns 404 for it and 200
for the correct spelling.

Two separate reasons it was missed, and both still stand:

  1. The file is out of range. Only README.md is read.
  2. Even pointed at the file, the second half would not have caught it. That
    half resolves paths under https://github.com/vyncint/termlens/blob/main/
    against the working tree; a …/labels/… URL, and anything on a different
    host or a misspelled one, names no path and is simply never examined.

Why it is worth fixing — the broken link was the one thing that section
existed to provide, and CONTRIBUTING is the file a first-time contributor
reads. It is also the file most likely to accumulate link rot, since it points
at scripts and workflows that get renamed.

Fix — the two halves of the script want separating, because they do not
generalise together:

  • The absolute-only rule is README-specific on purpose. It exists because
    crates/termlens/Cargo.toml sets readme = "../../README.md" and crates.io
    rewrites relative links against the crate directory. CONTRIBUTING.md is not
    packaged and legitimately uses relative links today (AGENTS.md,
    docs/RELEASING.md, docs/STABILITY.md, .github/scripts/check-dco.sh,
    .github/scripts/check-no-ai-attribution.sh, .github/workflows/ci.yml).
    Pointing the current script at CONTRIBUTING would fail on all six, which is
    why docs : add contibutor getting started guide #351 did not just do it.
  • The target exists rule generalises fine, and should run over CONTRIBUTING
    too, resolving relative links against the repository root and absolute
    blob/main/ links as it does now.

That leaves the actual hole: a URL naming no in-repo path, which today is
checked by nobody. Options, cheapest first:

  • Verify that any github.com/vyncint/<repo> URL names a repository that
    exists in the org, offline, from a small allow-list. Catches temlens
    exactly, needs no network, stays deterministic.
  • A real link check over external URLs. Catches more, but it is online and
    flaky, and the existing script's header argues deliberately against making
    the gate depend on GitHub answering. If it lands, it belongs in a scheduled
    workflow, not in the PR gate.

The first is probably enough. Whichever is chosen, break it on purpose first
and show it failing, per CONTRIBUTING.

Done when

  • A misspelled github.com/vyncint/temlens link in CONTRIBUTING.md
    fails CI, demonstrated by a deliberate failing run.
  • A renamed or deleted in-repo target linked from CONTRIBUTING.md fails.
  • The README's absolute-only rule still applies to README.md alone, and
    CONTRIBUTING's existing relative links still pass.
  • The reason the rules differ is written down where the next reader finds
    it, rather than inferred from the script.

A good first contribution: shell only, no Rust. Start with
CONTRIBUTING.md
— §1 lists every gate you can run locally. Commits are Conventional Commits
and need git commit -s (DCO). Happy to review a draft PR early.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationgood first issueGood for newcomers

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions