Skip to content

fix: drop references to the removed docs/topics tree - #660

Merged
kyle-sexton merged 3 commits into
mainfrom
fix/docs-topics-stragglers
Oct 3, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
fix/docs-topics-stragglers

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

No related issue: main has been red since c11c7ec (#650); this restores it.

Summary

#650 deleted docs/topics/ while #656 added two AGENTS.md links into it, and two tests still read files there. lychee-offline and selector-contract fail on main and on every open PR.

Fix

  • AGENTS.md: the two Code Review Rules lines that linked into docs/topics/ now state their rule inline.
  • .github/scripts/ci-fanout-consolidation.test.cjs: drop the ADR read and the test that asserted the ADR's text.
  • .github/scripts/claude-review-plugin-path.test.cjs: drop the test that asserted the V2 architecture doc's text.

Verification

  • node --test .github/scripts/*.test.cjs: 192 pass, 0 fail.
  • git grep docs/topics finds only the standards-managed .github/actionlint.yaml comment that chore: remove docs/topics #650 already noted.

Related

🤖 Generated with Claude Code

#650 removed docs/topics while #656 added two AGENTS.md links into it, and
two tests still read its files, so main went red on lychee-offline and
selector-contract. State the two review rules inline and delete the tests
that only asserted the removed documents' content.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 3, 2026 19:05
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-03T19:06:42.022198Z 15557a0 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

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


Code review complete

  • Gather context (diff, changed files, PR body)
  • Review AGENTS.md changes
  • Review test file changes (ci-fanout-consolidation.test.cjs, claude-review-plugin-path.test.cjs)
  • Verify claims (grep for docs/topics)
  • Post findings

Summary

The fix itself is correct and minimal: it removes the two now-dangling docs/topics/... links from AGENTS.md and the two test assertions that read files under the deleted docs/topics/ tree, matching exactly what #650 broke. I independently confirmed via git grep/Grep that the only remaining docs/topics reference repo-wide is the already-noted comment in .github/actionlint.yaml, and that neither removed doc path (ci-fanout-consolidation/ADR.md, claude-review-lanes/V2-PLUGIN-ARCHITECTURE.md) nor local-lane-guards.md is referenced anywhere else. Both edited test files end cleanly with no leftover unused imports or dangling syntax.

I could not run node --test .github/scripts/*.test.cjs myself in this sandbox (command execution was blocked), so the PR's "192 pass, 0 fail" claim is author-reported and unverified by me — the static review above is what I was able to confirm independently.

One non-blocking documentation-consistency finding posted inline on AGENTS.md (lines 13-14): the file's own preamble ("the linked file states it in full") no longer holds for the two rewritten bullets, which now state their rule inline with no link. A home for at least one of them already exists in README.md.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

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


Security review: no issues found

  • Gather context (PR diff, changed files)
  • Review changed files for security-relevant issues
  • Check AGENTS.md instruction-surface deletions against protected classes
  • Post findings

Scope: AGENTS.md, .github/scripts/ci-fanout-consolidation.test.cjs, .github/scripts/claude-review-plugin-path.test.cjs — all consistent with the already-merged docs/topics removal in #650.

Instruction-surface check (AGENTS.md): the two edited lines each drop a markdown link into the now-deleted docs/topics/ tree, but the rule statement itself is unchanged word-for-word in both cases — "Local-lane guard wrappers keep parity with the standards component" and "Lanes consolidate as composite actions; ci-status stays the single required check." No enforcement content, scope, or exception was narrowed; only a dead pointer to a file #650 already deleted was removed. This does not match a protected-class removal — nothing a hook, allow/deny rule, or validator relied on changed. Note: docs/conventions/instruction-exception-register/README.md is not present in this checkout, so the protected-class list itself was not consulted directly; the consequence-based check above stands in for it.

Test deletions: both removed test(...) blocks only asserted string content of docs files that no longer exist (the ADR and the V2 architecture doc) — no behavioral or security assertions were removed, and no fs.readFileSync calls were left dangling on paths outside the repo tree.

No CRITICAL, IMPORTANT, or SUGGESTION findings. This is a straightforward doc/test cleanup PR with no security-relevant surface.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

claude-security-review has reviewed this pull request through 1b6e287; a later push is reviewed from there.

Comment thread AGENTS.md
@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

claude-review has reviewed this pull request through 1b6e287; a later push is reviewed from there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

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


Incremental code review (AGENTS.md only)

  • Read .claude-lane/incremental.diff (the only change since 15557a0)
  • Read current AGENTS.md
  • Check whether the change resolves the inconsistency flagged in the prior review
  • Post findings

Summary

Scope for this pass: only the line changed since 15557a0 — AGENTS.md:3, where 8cdfe5c reworded the Code Review Rules preamble from "the linked file states it in full" to "a linked file states it in full."

One finding posted inline on AGENTS.md:3: the article swap doesn't actually close the gap the prior review raised. The preamble still implies every line has a linked file that states its rule in full, but the two bullets on lines 13-14 carry no link at all, so neither "the" nor "a" linked file exists for them. A real fix is still either linking line 13 to README.md's existing local-lane-guards paragraph, or explicitly carving out inline-only bullets in the preamble's wording.

Not blocking — the PR's actual goal (unbreaking lychee-offline/selector-contract) is unaffected by this wording.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

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


Security review: no issues found

  • Gather context (incremental diff since 15557a0, scoped to AGENTS.md)
  • Run /review:security-review per the lane's incremental scope
  • Post findings

Scope: this lane already reviewed the PR through 15557a0. The only change since then is one line in AGENTS.md:

-Each line names a rule CI does not enforce; the linked file states it in full.
+Each line names a rule CI does not enforce; a linked file states it in full.

Instruction-surface check: this sentence is the file's own meta-description of its bullet-list convention, not a standing instruction that itself gates an action elsewhere in the tree (no hook, allow/deny rule, or validator keys off its wording). Changing "the" to "a" only acknowledges what the prior commit (15557a0, already reviewed) had already done — stating two rules inline instead of via a link — and was the exact fix the earlier review comment on this PR suggested ("either the preamble needs to be loosened to admit inline-only rules, or that statement needs a new home"). No rule's enforcement, scope, or exception changed; no rule text was removed or softened. This does not match a protected-class removal under the instruction-exception-register lens. Note: docs/conventions/instruction-exception-register/README.md is not present in this checkout, so the protected-class list itself was not consulted directly; the consequence-based check above stands in for it.

No CRITICAL, IMPORTANT, or SUGGESTION findings.

Comment thread AGENTS.md Outdated
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

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


Security review: no issues found

  • Read .claude-lane/incremental.diff (the only change since 8cdfe5c)
  • Run /review:security-review per the lane's incremental scope
  • Post findings

Scope: this lane already reviewed the PR through 8cdfe5c. The only change since then is the Code Review Rules preamble in AGENTS.md:

-Each line names a rule CI does not enforce; a linked file states it in full.
+Each line names a rule CI does not enforce. A line with a link points at the file that states the
+rule in full; a line without one states the whole rule itself.

Instruction-surface check: this sentence is the file's own meta-description of its bullet-list convention — not itself a standing instruction that gates an action elsewhere in the tree (no hook, allow/deny rule, or validator keys off its wording, consistent with what the two prior reviews on this PR already found for the earlier "the"→"a" wording tweak). This revision doesn't remove, narrow, or soften any rule's enforcement, scope, or exception: it only reconciles the preamble with what 15557a0 had already done two commits earlier (stating the two guard/consolidation rules inline instead of via a now-deleted docs/topics/ link). That inline content is unchanged word-for-word across this whole PR. This is in fact the fix both prior review comments on this PR requested — explicitly admitting inline-only bullets rather than relying on an article swap to imply it — so it closes the gap rather than opening one. No protected-class removal under the instruction-exception-register lens. Note: docs/conventions/instruction-exception-register/README.md is not present in this checkout, so the protected-class list itself was not consulted directly; the consequence-based check above stands in for it.

No CRITICAL, IMPORTANT, or SUGGESTION findings.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

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


Incremental code review (AGENTS.md only)

  • Read .claude-lane/incremental.diff (the only change since 8cdfe5c)
  • Read current AGENTS.md
  • Check whether the change resolves the inconsistency flagged in the two prior reviews
  • Post findings

Summary

Scope for this pass: the one change since 8cdfe5c — AGENTS.md:3-4, where 1b6e287 rewrote the preamble from:

Each line names a rule CI does not enforce; a linked file states it in full.

to:

Each line names a rule CI does not enforce. A line with a link points at the file that states the
rule in full; a line without one states the whole rule itself.

This closes the gap both prior reviews on this PR raised: it explicitly carves out inline-only bullets rather than implying every line has a backing linked file. The two unlinked bullets this preamble now has to cover — AGENTS.md:14 ("Local-lane guard wrappers keep parity with the standards component.") and AGENTS.md:15 ("Lanes consolidate as composite actions; ci-status stays the single required check.") — each state their rule wholly inline with no link, matching the new wording exactly.

No findings on this increment. The earlier suggestion to instead link line 14 to README's local-lane-guards paragraph was one of two valid options the prior review offered; the author took the other (loosen the preamble), which is internally consistent and resolves the issue.

@kyle-sexton
kyle-sexton merged commit adc1624 into main Oct 3, 2026
27 checks passed
@kyle-sexton
kyle-sexton deleted the fix/docs-topics-stragglers branch October 3, 2026 19:23
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.

1 participant