Skip to content

fix(links): repair online link-check failures - #224

Merged
kyle-sexton merged 2 commits into
mainfrom
agent/issue-208-link-check
Jul 21, 2026
Merged

fix(links): repair online link-check failures#224
kyle-sexton merged 2 commits into
mainfrom
agent/issue-208-link-check

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

  • keep all component fixtures in Lychee's dedicated offline contract lane so deliberately invalid boundary examples cannot enter the scheduled online scan
  • update stale official documentation paths and fragments while preserving each citation's original meaning
  • preserve the semantically exact W3C time-zone citation behind a URL-specific bot-throttling exception, with a sibling-path regression assertion
  • repair the additional stale actions/checkout fragment exposed by the bounded production scan

Root cause

The online workflow excluded only fixtures/bad, but components/lychee/fixtures/good/Exclusions.md intentionally contains invalid sibling URLs used by the offline --dump boundary test. Several external authorities had also renamed pages or headings. The W3C time-zone page remains the correct source, but the scheduled bot received HTTP 429 responses.

Research

Replacements and exceptions were checked against current primary sources:

Verification

  • bash harness/shell/run-tests.sh components/lychee/lychee.test.sh — pass
  • bash harness/shell/run-tests.sh components/editorconfig-checker/editorconfig-checker.test.sh — pass
  • all README validation commands — pass
  • bash distribution/sync-manifest.sh validate — pass (29 components, 8 targets)
  • full pre-commit hook — pass (EditorConfig, markdownlint, shellcheck, typos, gitleaks)
  • corrected actions/checkout URL through Lychee stdin — pass (1/1)
  • production-shaped online scan — attempted with a 90-second cap; the initial run found the now-fixed checkout fragment, and the post-fix run reached the cap without emitting a link failure. CI remains the authoritative Linux/network execution.
  • cross-document reconciliation — complete; no stale forms remain and no contradicting guidance was found

Distribution impact

lychee.toml is a managed component. After merge, the normal distribution workflow should propagate the exact W3C exception to the manifest's downstream targets; no downstream copy was edited directly.

Related

Closes #208

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 36s —— View job


Review: fix(links): repair online link-check failures

  • Gather context (REVIEW.md, CLAUDE.md, PR diff)
  • Review .github/workflows/link-check.yml changes
  • Review lychee.toml changes
  • Review fixture and test changes (components/lychee/)
  • Review documentation URL updates (conventions, distribution, harness)
  • Verify cross-document consistency
  • Post final review

The change is well-scoped and correct. No blocking findings. Notes below.

Workflow exclusion fix

link-check.yml L35: The expansion from components/[^/]+/fixtures/bad to components/[^/]+/fixtures correctly addresses the root cause. fixtures/good/Exclusions.md intentionally contains bot-blocked and 429-returning URLs used to test exclusion boundaries — these have no business being input to the online network scanner. The regex [^/]+ is correct here since --exclude-path takes a regex, not a glob. The updated comment accurately describes the intent.

W3C exclusion

lychee.toml L32: The regex '^https?://www\.w3\.org/International/wiki/WorkingWithTimeZones/?([?#].*)?$' is correctly scoped — it targets only the specific page that returns HTTP 429, while leaving the parent https://www.w3.org/International/ and other W3C paths subject to checking. The fixture (Exclusions.md) documents both directions of the boundary, and the two new assertions in lychee.test.sh verify both sides: the excluded URL is not dumped, and the sibling path https://www.w3.org/International/ still appears.

URL replacements

All seven citation replacements look correct:

File Old target New target Assessment
components/lychee/README.md /guides/config/#excluding-links /recipes/excluding-links/ Correct — lychee reorganized docs
conventions/engineering/concise-prose.md #use_helper_words_and_optional_words #use-helper-words-and-optional-words Correct anchor fragment (underscores → hyphens)
components/runner-policy/THREAT-MODEL.md #set-the-permissions-of-the-github_token-for-a-repository #set-the-permissions-of-the-github_token-for-your-repository Correct anchor fragment ("a" → "your")
distribution/REVIEW-CREDENTIAL.md setting-up-a-github-app/choosing-permissions-for-github-apps registering-a-github-app/choosing-permissions-for-a-github-app Correct path (GitHub reorganized app docs)
distribution/REVIEW-CREDENTIAL.md #checkout-a-different-repository #checkout-multiple-repos-private Correct anchor for the multi-repo private example
harness/packages/README.md /files/package.json/#name /cli/v11/configuring-npm/package-json/#name Correct — versioned CLI path
conventions/engineering/deterministic-artifact-scaffolding.md openai.com/index/introducing-structured-outputs-in-the-api/ developers.openai.com/api/docs/guides/structured-outputs Correct — old URL was a blog post, now points to official API docs

🟡 Nit — prose update in deterministic-artifact-scaffolding.md goes slightly beyond the stated scope

deterministic-artifact-scaffolding.md L6–8: The PR description says changes "preserve each citation's original meaning," but the prose around the OpenAI link does more than update the URL — it adds the phrase "recommends evals for the latter" which wasn't in the original text. The substance of the convention rule is unchanged, and the new characterization is accurate to the current docs, so this is not a problem. Just noting it as a minor deviation from the stated scope.

Security checklist (REVIEW.md)

  • No credentials or secrets introduced ✓
  • No object-level authorization surface touched ✓
  • No injection-prone process invocations ✓
  • No tenant data access paths ✓

Overall: Clean, well-tested maintenance PR. The test pairing (fixture boundary + both assert_not_contains / assert_contains directions) follows the existing pattern correctly.

@kyle-sexton
kyle-sexton force-pushed the agent/issue-208-link-check branch from 9266087 to 9456c9a Compare July 21, 2026 03:14
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 48s —— View job


Review: fix(links): repair online link-check failures

  • Gather context (REVIEW.md, PR diff)
  • Review workflow exclusion change
  • Review lychee.toml regex addition
  • Review fixture and test changes
  • Review URL citation updates (7 files)
  • Post final review

No blocking findings. Notes below.

Workflow exclusion (link-check.yml)

link-check.yml L35: Broadening components/[^/]+/fixtures/badcomponents/[^/]+/fixtures is correct. Since --exclude-path takes a regex (not a glob), this acts as a substring match against full paths; the [^/]+ segment correctly restricts to a single component-name level. Both fixtures/bad and fixtures/good directories now stay out of the online scan, which is the right policy — fixture content exists to exercise the offline lychee configuration, not to be a live link target.

W3C exclusion (lychee.toml)

lychee.toml L32: The added regex '^https?://www\.w3\.org/International/wiki/WorkingWithTimeZones/?([?#].*)?$' is correctly scoped — anchored with $, dots escaped, optional trailing slash, optional query/fragment. It targets only the throttled page; the broader https://www.w3.org/International/ domain remains subject to live checking.

Fixture documentation (Exclusions.md) and tests (lychee.test.sh)

Both directions of the W3C boundary are documented in the fixture and exercised in the test suite: the excluded URL appears under assert_not_contains, and the https://www.w3.org/International/ sibling appears under assert_contains. This is consistent with how all other entries in this file are structured.

URL citation replacements

All seven citation updates are correct:

File Change Assessment
components/lychee/README.md /guides/config/#excluding-links/recipes/excluding-links/ Lychee doc reorganization ✓
conventions/engineering/concise-prose.md #use_helper_words_and_optional_words#use-helper-words-and-optional-words Anchor underscore→hyphen ✓
components/runner-policy/THREAT-MODEL.md …-for-a-repository…-for-your-repository Anchor "a"→"your" ✓
distribution/REVIEW-CREDENTIAL.md setting-up-a-github-app/…registering-a-github-app/… GitHub App docs path update ✓
distribution/REVIEW-CREDENTIAL.md #checkout-a-different-repository#checkout-multiple-repos-private Correct anchor for multi-repo private example ✓
harness/packages/README.md /files/package.json/#name/cli/v11/configuring-npm/package-json/#name Versioned npm CLI path ✓
conventions/engineering/deterministic-artifact-scaffolding.md Blog URL → developers.openai.com/api/docs/guides/structured-outputs Old URL was a blog post; now points to official API guide ✓

🟡 Nit — prose edit in deterministic-artifact-scaffolding.md slightly exceeds stated scope

deterministic-artifact-scaffolding.md L6: The surrounding prose was reworded — "distinguishes deterministic schema conformance from the correctness of values inside that schema" → "separates schema adherence from generation quality and recommends evals for the latter" — not just the URL. The convention meaning ("a scaffold owns schema conformance, not value correctness") is preserved and the new characterization is accurate to the current docs, so there is no correctness concern. The statement "a scaffold owns the former, not the latter" remains unambiguous in context. This is a minor deviation from the PR's stated scope of URL-only repair.

Security checklist (REVIEW.md)

  • No secrets or credentials introduced ✓
  • No object-level authorization surface touched ✓
  • No injection-prone process invocations ✓
  • No tenant data access paths ✓
  • No atomicity-sensitive multi-record writes ✓

Overall: Clean, well-tested maintenance PR. The root cause fix (fixture exclusion), the bot-throttle exception, and all seven stale URL replacements are correct. Test pairing follows the existing boundary-assertion pattern correctly.

@kyle-sexton
kyle-sexton force-pushed the agent/issue-208-link-check branch from 9456c9a to 796383e Compare July 21, 2026 06:01
@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 2s —— View job


Review: fix(links): repair online link-check failures

  • Gather context (REVIEW.md, CLAUDE.md)
  • Review PR diff
  • Review workflow, lychee.toml, fixtures, tests, and URL updates
  • Post final review

No blocking findings. Two earlier automated reviews on this PR already covered the URL replacements and core logic; I can confirm their assessments and add one nit they didn't raise.

Workflow path-exclusion regex (link-check.yml)

link-check.yml L35: The new pattern '^(\./)?components[\\/][^\\/]+[\\/]fixtures([\\/]|$)' is correctly constructed:

  • ^ anchors to the start — notcomponents/ and nested/components/ cannot match.
  • [^\\/]+ matches exactly one path segment — components/lychee/nested/fixtures cannot match.
  • ([\\/]|$) requires fixtures to be a standalone segment, not a prefix — fixtures-old cannot match.
  • The [\\/] character class is cross-platform; harmless on Linux, useful if this regex is ever applied on Windows.

The 5-case boundary test in lychee.test.sh (intended exclusion + 4 non-exclusion lookalikes) exercises all meaningful edge cases. Pattern verified to match the deployed workflow value exactly.

W3C exclusion (lychee.toml + fixture + tests)

lychee.toml L32: Regex '^https?://www\.w3\.org/International/wiki/WorkingWithTimeZones/?([?#].*)?$' is anchored at both ends with dots escaped and an optional trailing slash — structurally consistent with all other exclude entries. The sibling https://www.w3.org/International/ is untouched by this pattern and confirmed still subject to checking by the new assert_contains assertion.

🟡 Nit — path-boundary test captures only the final lychee exit code

lychee.test.sh L39–50: The for loop runs five lychee --dump invocations inside a $() subshell, and rc=$? captures only the last iteration's exit code. If any of the first four invocations fail (e.g. lychee rejects an input path), those failure codes are silently dropped and the assert_exit … 0 "$rc" assertion would still pass. Since --dump is network-free and all inputs are deterministically constructed in the test itself, this is unlikely to cause a real failure, but it's a coverage gap in the assertion. A future-proof fix would be to collect lychee output differently or run all inputs in a single invocation.

URL replacements confirmed

All seven citation updates in the changed files are correct (Lychee doc reorganization, Google anchor underscores→hyphens, GitHub GITHUB_TOKEN anchor "a"→"your", GitHub App docs path, actions/checkout anchor, npm versioned CLI path, OpenAI blog→official API guide). The prose addition in deterministic-artifact-scaffolding.md ("recommends evals for the latter") was flagged in earlier reviews as a minor scope deviation; it's accurate to the new source and the convention meaning is unchanged.

Security checklist (REVIEW.md)

  • No secrets or credentials introduced ✓
  • No object-level authorization surface touched ✓
  • No injection-prone process invocations ✓
  • No tenant data access paths ✓
  • No atomicity-sensitive multi-record writes ✓

@kyle-sexton
kyle-sexton merged commit 9e0abf0 into main Jul 21, 2026
42 checks passed
@kyle-sexton
kyle-sexton deleted the agent/issue-208-link-check branch July 21, 2026 08:43
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Merged (squash) under the operator's decide-and-log delegation: routine link/docs-hygiene fix in the shared-policy repo with no downstream security or permission posture change; all checks green, review complete (clean verdict, only non-blocking nits), 0 unresolved threads, head unchanged since review (796383e). Reversible + review-backed. This was generated by AI (tower-dispatched merge worker).

kyle-sexton added a commit that referenced this pull request Jul 23, 2026
…ter v0.8.0 selector (#252)

No related issue: enforcer support for merged
melodic-software/ci-workflows v0.8.0 (#224, #225); unblocks the caller
repin + ancillary-events opt-in in melodic-software/medley.

## Related

ci-workflows#224, ci-workflows#225

## Summary

- Adds `optionalBooleanSelectorInputs` to the runner-policy schema and
enforcer so boolean selector opt-ins can be governed; the only accepted
value is the literal `true` (`booleanTrueMap` uses `const: true`).
- Registers `admits-ancillary-events` and the deprecated alias
`admits-comment-events`. These let a caller opt its no-checkout
`issue_comment` / `pull_request_review` / `pull_request_review_comment`
/ `issues` jobs into self-hosted fleet routing (ci-workflows #224,
#225).
- Registers the ci-workflows v0.8.0 `select-runner` revision
`e77f0126b474144708719f99795e44d0ffe2541d`, owner-scoped to
`melodic-software`, so consumers can repin and activate the opt-in.
- The boolean map merges into `exactCanonicalMap`'s optional-input
argument, reusing the existing fail-closed exact-match; a load-time
duplicate-name guard keeps the optional string and boolean maps
disjoint.

## Security posture

The enforcer *permits*, but does not *verify*, the caller's no-checkout
declaration — that premise is trusted under the same job-step review
that backs a local-routing grant. No existing job-level control is
relaxed (statically-read-only-or-grant, privileged-hosted,
container/service, runner-target all still apply); the opt-in only
widens which event types the selector may route self-hosted. Captured as
a new THREAT-MODEL threats-table row and residual-risk bullet, and in
the component README's selector-input contract. Fleet-admission-logic
security review pending per team lead before merge.

## Test plan

- [x] `npm run test:runner-policy` — 238/238 pass (10 new cases: accept
`true`, accept alias, reject `false`, reject quoted `"true"`, reject
unknown input, owner-scoped v0.8.0 accept, unapproved-revision reject,
schema reject non-`true`, duplicate optional-string name, duplicate
canonical name)
- [x] `npm run lint:runner-policy` — Runner policy passed (dogfood with
`GITHUB_REPOSITORY` owner evidence)
- [x] `npm run lint:md` — 0 errors
- [x] `npm run test:packages` — 14/14 pass
- [x] `npm run lint:hooks` — All good
- [x] `npm run test:lefthook-dotnet` — 12/12 pass
- [x] `bash harness/shell/run-tests.sh harness/shell/lib.test.sh` —
passed
- [x] `npx biome check` on changed JS/JSON — no fixes needed
- [x] Independent security review (fresh reviewer, rationale withheld) —
correct and fail-closed, no P1–P3 findings; the two P4 coverage gaps it
raised (quoted-string value, canonical duplicate-name branch) are
addressed here

Do NOT merge — held for the fleet-admission security review.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

Link checker report

1 participant