Skip to content

ci: address coderabbit review on PR #1 — perms, sha-pin, spdx, actionlint#7

Merged
sebyx07 merged 3 commits into
mainfrom
ci/coderabbit-fixes-pr1
May 17, 2026
Merged

ci: address coderabbit review on PR #1 — perms, sha-pin, spdx, actionlint#7
sebyx07 merged 3 commits into
mainfrom
ci/coderabbit-fixes-pr1

Conversation

@sebyx07

@sebyx07 sebyx07 commented May 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses all 4 CodeRabbit threads from #1 (which was admin-merged before review was processed — this PR closes the loop).

Thread Severity Status
Narrow GITHUB_TOKEN default to read-only Major ✅ Fixed
Pin all 3rd-party actions to commit SHAs Major ✅ Fixed (29 refs across ci.yml + labeler.yml)
SPDX check enforces header location (top 15 lines) Major ✅ Fixed
Register Blacksmith labels in .github/actionlint.yaml Minor ✅ Fixed

Test plan

Spec refs

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Pinned CI/CD actions to fixed commits for improved stability.
    • Tightened workflow permissions to read-only where possible.
    • Made license-header checks stricter to ensure SPDX identifiers appear near file starts.
    • Registered specific self-hosted runner labels to avoid false warnings.
    • Simplified internal tooling guidance and updated governance references from 12 to 15 laws.

Review Change Stack

…lint

Four threads from PR #1 review, all accepted:

1. Narrow GITHUB_TOKEN default to read-only (Major)
   Dropped workflow-level pull-requests:write + checks:write. No job
   currently needs write — artifact upload uses contents:read.

2. Pin all third-party actions to commit SHAs (Major)
   29 action references across ci.yml + labeler.yml now pin to 40-char
   SHAs with trailing `# vX.Y.Z` comments for human readability.
   Dependabot's github-actions ecosystem will update both SHA + comment.

3. SPDX check must enforce header location, not just token presence (Major)
   Switched `grep "$f"` to `head -n 15 "$f" | grep` — header must be
   near top-of-file per Law 7 intent, not buried somewhere in the blob.

4. Register custom Blacksmith labels in actionlint.yaml (Minor)
   Added .github/actionlint.yaml registering blacksmith-{2,4,8,16}vcpu-
   ubuntu-{2204,2404} so actionlint (used by CodeRabbit) stops flagging
   `runs-on: blacksmith-*` as unknown.

Verified locally:
- python yaml.safe_load passes on all 3 YAML files
- All resolved SHAs verified via `gh api repos/.../commits/<sha>`

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cdb5fd19-a2be-4ca9-9d30-0c951f5162bb

📥 Commits

Reviewing files that changed from the base of the PR and between 00bcfff and a79e336.

📒 Files selected for processing (2)
  • .coderabbit.yaml
  • CLAUDE.md

Walkthrough

This PR hardens GitHub Actions security and reproducibility by establishing actionlint runner configuration, tightening CI workflow permissions to read-only defaults, pinning action references to commit SHAs across jobs, and restricting SPDX header detection to the first 15 lines of files.

Changes

GitHub Actions Security and Configuration

Layer / File(s) Summary
Actionlint runner configuration
.github/actionlint.yaml
Populate actionlint configuration with SPDX/copyright headers and register Blacksmith-managed self-hosted runner labels (Ubuntu 2204/2404 with various CPU sizes) to suppress unknown-label warnings.
CI workflow permissions and documentation
.github/workflows/ci.yml
Tighten workflow-level permissions from broader write access to contents: read only, and update comments documenting the shift to pinned commit SHAs for action setup.
Rust job action version pinning
.github/workflows/ci.yml
Pin all action versions by commit SHA across Rust jobs: actions/checkout, actions-rust-lang/setup-rust-toolchain, useblacksmith/rust-cache, taiki-e/install-action, and actions/upload-artifact.
Bun and docs job action pinning with SPDX refinement
.github/workflows/ci.yml
Pin actions/checkout, oven-sh/setup-bun, and useblacksmith/cache to commit SHAs in Bun jobs. Update SPDX verification script to require SPDX-License-Identifier: within the first 15 lines only.
Labeler workflow action pinning
.github/workflows/labeler.yml
Pin actions/labeler from @v5 to specific commit SHA for v5.0.0.
CodeRabbit tone_instructions update
.coderabbit.yaml, CLAUDE.md
Replace verbose tone_instructions block with a condensed authority-and-citation instruction set and update references from “12 Laws” to “15 Laws” in policy/routing entries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Actions pinned, permissions tight,
Blacksmith runners now in sight,
SPDX checks scan just the start,
Reproducible CI—a rabbit's art!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main changes: CI fixes responding to CodeRabbit review, covering permissions narrowing, SHA-pinning, SPDX enforcement, and actionlint configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/coderabbit-fixes-pr1

Comment @coderabbitai help to get the list of available commands and usage tips.

CR flagged on PR #7: tone_instructions was 1286 chars; CR's schema caps
it at 250 and was falling back to default settings.

Compressed to 230 chars — keeps authority order + reject-violators-cite-Law
+ "concrete diffs, skip nits". The detailed per-Law rules (panic!/unwrap,
string errors, println!, unsafe SAFETY:, SPDX, ADR-for-architecture)
already live in CLAUDE.md and docs/architecture/01-principles.md where
they belong; CR reads CLAUDE.md anyway.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.coderabbit.yaml:
- Line 18: The file's governance counts are inconsistent: the tone_instructions
entry declares "15 Laws" while other places enforce "12 Laws"; update all
law-count references so they match the intended number (either change
tone_instructions to "12 Laws" or change the other law-count entries to "15
Laws") — locate the keys/values named tone_instructions and the law count fields
(the repeated "Laws" or numeric law-count entries present in the file) and make
them identical across the file, and ensure any accompanying text (e.g., "15
Laws" phrasing) and enforcement lists (the actual law entries) reflect the same
count.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5d8efa0d-a96d-4599-bdd6-031c06eb2b0e

📥 Commits

Reviewing files that changed from the base of the PR and between dcce61e and 00bcfff.

📒 Files selected for processing (1)
  • .coderabbit.yaml

Comment thread .coderabbit.yaml
CR caught a real inconsistency on PR #7: tone_instructions said
"15 Laws" but other places still referenced "12 Laws". Source of
truth is docs/architecture/01-principles.md, which has 15 Laws
(Laws 13–15 ratified via ADR-0010 per docs/INTEGRATION-REPORT.md).

Rejected CR's specific diff (would have downgraded to 12); applied
the spirit of the finding — bumped all stale 12-references to 15:
  - .coderabbit.yaml line 7  (header comment)
  - .coderabbit.yaml line 15 (tone block comment)
  - .coderabbit.yaml line 110 (architecture path-instructions)
  - CLAUDE.md line 72 (principle-keeper row)
  - CLAUDE.md line 277 (CR tools summary)

CLAUDE.md table at line 14 already enumerated all 15 Laws — no body
changes needed there.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sebyx07 sebyx07 merged commit 50548fa into main May 17, 2026
11 checks passed
@sebyx07 sebyx07 deleted the ci/coderabbit-fixes-pr1 branch May 17, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant