Skip to content

Code review · D1 (agent and skill frontmatter check)

Range 98d74c2..aee3798 on branch autopilot/D1. Test command: python3 -m unittest tools/claude/test_check_frontmatter.py && python3 tools/claude/check_frontmatter.py.

CODE  step=D1  review=approve b/M/m=0/0/2  security=approve b/M/m=0/0/3  rounds=1  simplify=nochange -0/+0  tests=pass
  • Pass 1 on 98d74c2..510a3a9: code-reviewer approve (2 minors), security-reviewer approve (3 minors).
  • Fixes in aee3798 for CR-1, CR-2 and SEC-D1-01, then pass 2 on 510a3a9..aee3798: both approve, no findings.
  • code-simplifier on 98d74c2..aee3798: no change; it judged the code already minimal.

Findings: code-reviewer

CR-D1-01 · minor · Agents in subfolders were not checked

  • Where: tools/claude/check_frontmatter.py (agent glob)
  • Evidence: the checker used agents/*.md; the sub-agents docs say Claude Code scans .claude/agents/ recursively (sub-agents).
  • Problem: a broken agent in agents/<sub>/ would pass CI.
  • Proposal: rglob("*.md") and one test with an agent in a subfolder.
  • Found by: code-reviewer
  • Status: applied (aee3798)

CR-D1-02 · minor · A skill with no description is a repository rule, not a load failure

  • Where: tools/claude/check_frontmatter.py docstring; dev-environment-and-testing.md §11, claude-config.yml row
  • Evidence: the skills docs say every field is optional, and a skill with no description uses the first line of its body (skills).
  • Problem: the row presented the rule as a reason the skill would not load.
  • Proposal: keep the rule and say that it is a repository rule.
  • Found by: code-reviewer
  • Status: applied (aee3798, §11 row)

Findings: security-reviewer

SEC-D1-01 · minor · Checkout kept the job token in .git/config

  • Where: .github/workflows/claude-config.yml, checkout step
  • Evidence: actions/checkout writes the token unless persist-credentials: false; the next steps run Python from the PR branch.
  • Problem: an avoidable exposure, low impact (contents: read).
  • Proposal: persist-credentials: false.
  • Found by: security-reviewer
  • Status: applied (aee3798)

SEC-D1-02 · minor · PyYAML pinned by version, not by hash

  • Where: .github/workflows/claude-config.yml, pip install --break-system-packages pyyaml==6.0.3
  • Evidence: a version pin does not stop a different artefact being served; §11 says nothing yet about pip installs in workflows.
  • Problem: supply-chain hardening gap, shared with docs.yml (which installs pyyaml unpinned).
  • Proposal: a hash-pinned requirements file with --require-hashes --only-binary=:all:, or the uv lock once it exists.
  • Found by: security-reviewer
  • Status: open, for D4 (dependency audit and workflow conventions of §11)

SEC-D1-03 · minor · Reviewer agents' read-only promise is not checked

  • Where: tools/claude/check_frontmatter.py; CLAUDE.md ("Reviewers are read-only")
  • Evidence: the checker accepts any value of tools and permissionMode, so Edit on a reviewer or permissionMode: bypassPermissions would pass.
  • Problem: nothing enforces the read-only rule for agents fed untrusted input (diffs, web pages, PR bodies).
  • Proposal: a small policy table rejecting Edit, Write, NotebookEdit and bypassPermissions / acceptEdits for the read-only reviewers, with one failing fixture per rule.
  • Found by: security-reviewer
  • Status: open, question for Thomas (out of D1's scope: frontmatter validity, not policy)