Skip to content

docs: add Code Review Rules section for Codex review - #656

Merged
kyle-sexton merged 2 commits into
mainfrom
docs/code-review-rules
Oct 3, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
docs/code-review-rules

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

No related issue: rollout item for melodic-software/standards#656 (closed centrally)

Summary

Adds a pointer-only ## Code Review Rules section to the root AGENTS.md, so the Codex PR reviewer gets this repository's review rules. Shape follows components/code-review-rules/README.md in melodic-software/standards: the inherited block (managed REVIEW.md pointer, since this repository receives the synced REVIEW.md), then six repository-specific pointer lines.

Fix

AGENTS.md was empty (blanked by the unhobble bare-baseline reset in #403). It now holds only this section. CLAUDE.md stays empty and does not import AGENTS.md, so Claude Code sessions load nothing new; the section reaches Codex.

Per-repository lines, each a short rule name followed by a [rule](...) link to the file that owns it, none restating it:

  • Claude lane security model: the SECURITY MODEL headers of claude-review.yml and claude-security-review.yml.
  • Claude lane checks stay advisory: README, Claude lanes contract.
  • Configurable, not forkable: README, Contract.
  • Policy authored in standards; fixtures/ configs only exercise contracts: README, Policy ownership.
  • Local-lane guard parity with the standards component: docs/topics/local-lane-guards.md.
  • Composite-action consolidation, single ci-status: CI fan-out ADR, Decisions locked.

Left out because CI already checks them: SHA pinning (zizmor), PR title and body contract (ci-status), markdown and link lint.

Verification

  • code-review-rules.sh file --root <worktree> (standards origin/main): OK.
  • markdownlint-cli2 with the repository's .markdownlint-cli2.jsonc: 0 issues.
  • lychee --offline --include-fragments on AGENTS.md: 8 of 8 links OK, anchors included.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 3, 2026 02:05
Give the Codex PR reviewer this repository's review rules through a
pointer-only "## Code Review Rules" section in the root AGENTS.md, per the
melodic-software/standards code-review-rules convention (standards#656).
Each line names a rule CI does not enforce and links the file that owns it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop the security-model parenthetical, which named a rule the SECURITY
MODEL headers do not state, and the paraphrased qualifiers on the other
lines. Each per-repository line now uses the convention's slot form and
ends at its link.

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 06:07
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

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


Security review: no issues found

  • Gather context (PR diff, AGENTS.md change)
  • Check instruction-exception register / instruction-surface lens
  • Review changed content for security-relevant issues
  • Post findings

Scope: This PR touches a single file, AGENTS.md, adding a 16-line ## Code Review Rules section. Verified via git diff origin/main...HEAD -- AGENTS.md and git show origin/main:AGENTS.md that the file was previously empty (e69de29, blanked by the earlier "unhobble" reset per the PR description) — this change is purely additive, not a deletion, narrowing, or softening of any existing instruction.

Instruction-surface lens: Since nothing is being removed, the instruction-exception-register check (for deleted/weakened guardrails) doesn't apply here — there's no prior rule being taken away for a hostile context to exploit. For completeness: docs/conventions/instruction-exception-register/README.md is not present in this checkout, consistent with the skill's documented fallback.

Content reviewed: All six repo-specific lines are pointer-only (rule name + link), none restate executable policy inline, and none introduce secrets, credentials, executable code, or trust-boundary changes. CLAUDE.md remains empty and does not import AGENTS.md, so this addition reaches the Codex reviewer lane only, as described in the PR body — no change to Claude Code's own instruction surface.

Conclusion: No security-relevant surface in this diff (skip-gate condition 2) — docs-only, additive, no code/logic/auth/injection path. No CRITICAL / IMPORTANT / SUGGESTION findings.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

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


Code review — docs/code-review-rules (#656)

  • Gather context (diff vs origin/main, read AGENTS.md, README.md, REVIEW.md, ADR, workflow files)
  • Verify each link target and anchor resolves
  • Verify each pointer line accurately represents the content it links to
  • Check consistency with repo conventions (PR body contract, markdownlint config, CLAUDE.md non-import claim)
  • Post findings

Scope: single-file change, AGENTS.md (+16/-0), adding a pointer-only ## Code Review Rules section for the Codex reviewer.

Findings: none. This is a trivial, well-verified documentation change with no functional code and no meaningful defect surface. Specifics checked:

  • All six link targets exist (REVIEW.md, claude-review.yml, claude-security-review.yml, README.md, docs/topics/local-lane-guards.md, docs/topics/ci-fanout-consolidation/ADR.md).
  • All anchors resolve correctly, including the em-dash heading README.md#claude-lanes--shared-consumption-contract (README.md#L31 already uses the identical anchor in its own TOC).
  • Each pointer line's claim matches its target's actual content: SECURITY MODEL headers exist verbatim in both workflow files (claude-review.yml#L25, claude-security-review.yml#L28); the "advisory" claim is backed by README.md's "Claude lanes" section and its "do not make either check required anywhere else" line; "Configurable, not forkable" and "Policy ownership" map to their named ## Contract / ## Policy ownership and action inputs sections; the ADR's ## Decisions locked §2–3 back the composite-action/ci-status claim; docs/topics/local-lane-guards.md backs the local-lane-guard parity claim.
  • CLAUDE.md is confirmed empty (0 bytes), consistent with the PR's claim that it doesn't import AGENTS.md and Claude Code sessions pick up nothing new from this change.
  • PR body satisfies the repo's PR body contract (No related issue: <reason> opener, all four required sections non-empty).

Note: the PR's listed markdownlint/lychee verification output is author-claimed and was not independently re-run here; I instead checked the underlying claims directly (anchors, file existence, .markdownlint-cli2.jsonc config confirming MD041 is disabled so a non-H1 first heading is not a lint issue).

@chatgpt-codex-connector

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-03T06:08:36.913968Z f62a7a7 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.

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

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

@kyle-sexton
kyle-sexton merged commit 2f62f6d into main Oct 3, 2026
48 checks passed
@kyle-sexton
kyle-sexton deleted the docs/code-review-rules branch October 3, 2026 06:08
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

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

kyle-sexton added a commit that referenced this pull request Oct 3, 2026
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 #650 already noted.

## Related

- Unblocks #659.
- Cause: #650 and #656 merged in parallel.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <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.

1 participant