[mark-brannan]

Runs the shared reusable nonempty-diff workflow (mark-brannan/.github#25) as a job in the ci-gate needs, so the gate already requires it and no ruleset entry is needed. It refuses to call a PR green when its tree is identical to its base at the merge base (see mark-brannan/dotfiles#196/#198: an empty PR merged and was closed as done, having landed nothing).

Merge mark-brannan/.github#25 first; until then the nonempty job cannot resolve the reusable workflow and this PR is red.

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 51b37c94-ab75-4ece-8853-f02e58d8132c

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Claude finished @mark-brannan's task in 2m 5s —— View job

---

Independent review

• Gather context (diff, repo layout, CLAUDE.md/AGENTS.md — neither exists in this repo)
• Review the workflow YAML for correctness
• Check for credentials/secrets in diff — none found
• Run repo verification commands
• Post inline findings and summary

What I checked:

• The entire diff (.github/workflows/nonempty-diff.yml, +7/-0) — a thin caller of mark-brannan/.github/.github/workflows/nonempty-diff.yml@main, matching the PR description.
• No CLAUDE.md or AGENTS.md exist anywhere in this repo, so there were no repo conventions of that kind to check against.
• Compared against this repo's other two reusable-workflow callers (test.yml's ci-gate job, secret-scan.yml's secret-scan job) for consistency.
• Scanned the diff for credential-like strings — none present; it's a bare workflow reference to a same-owner public repo.

What I ran:

• npm test (node --test) — 58/58 passed. Not exercised by this diff, but green.
• npm install, npm ci, and gh api (to fetch the called workflow's source and confirm its permission requirements) all required network access, which is blocked in this sandbox — consistent with this repo's own test.yml comment that tests "must pass with the network unavailable." I could not independently verify what permissions the upstream nonempty-diff.yml actually needs.

Finding (posted inline on line 1-7): the nonempty job has no permissions: block, unlike this repo's other two reusable-workflow callers, which both set one explicitly with a comment explaining the choice (ci-gate: permissions: {}; secret-scan: permissions: contents: read). Without it, the job inherits the repo/org default GITHUB_TOKEN permissions rather than the minimum it needs. Not a functional bug, but a real inconsistency with an established, deliberate pattern in this repo.

Everything else checks out: the workflow is minimal and does what the PR describes, the "land unwired, ruleset in a follow-up" sequencing is a reasonable and explicitly stated plan, and nothing in the diff touches credentials or secrets.

[claude]

Claude finished @mark-brannan's task in 53s —— View job

---

Independent review

• Gather context (diff vs origin/main, both commits; no CLAUDE.md/AGENTS.md in this repo)
• Review the workflow YAML for correctness
• Check for credentials/secrets in diff — none found
• Run repo verification commands
• Post findings

What changed since the last review round: the second commit (b2cb1db) removed the standalone .github/workflows/nonempty-diff.yml caller and folded it into test.yml as a new nonempty job, added to ci-gate's needs. Net diff is now entirely within test.yml (+7/-1).

This resolves the earlier permissions finding. The prior round (mine and CodeQL's) flagged the standalone caller for having no permissions: block. Rather than bolting on a redundant job-level block, this fold puts nonempty under test.yml's existing top-level permissions: contents: read (lines 7-9) — the same umbrella the test job already relies on without its own override. That's tighter than the standalone file was, and consistent with the one other job here (test) that inherits rather than restates. ci-gate's explicit permissions: {} is a separate case — an actual downgrade from the default, not a redundant restatement — so the inconsistency I flagged before is gone, not just moved.

Checked:

• git diff origin/main...HEAD — full diff is the seven added/changed lines in test.yml.
• No stray references to the deleted nonempty-diff.yml path anywhere else in the repo.
• nonempty has no needs, so it runs independently of the test matrix and doesn't block or get blocked by it — ci-gate still correctly gates on both via if: always().
• Compared against secret-scan.yml's caller pattern (job-level permissions: contents: read restated even though redundant, with a comment on why) — the new nonempty job differs by not restating, but that's defensible here since it truly adds nothing beyond the top-level default, unlike secret-scan's case where the file's top-level and job-level permissions could plausibly diverge in the future.

Ran:

• npm test (node --test) — 58/58 passed.
• npm ci — fails, as expected: this repo ships no package-lock.json and declares no runtime dependencies (comment in test.yml: "coast-wright has no runtime dependencies... tests must pass with the network unavailable"), matching why test.yml itself has no install step.
• npm run build — no build script exists (only build:demo, unrelated and not invoked by this workflow).
• Could not fetch mark-brannan/.github/.github/workflows/nonempty-diff.yml@main's source (network blocked in this sandbox) to independently confirm it needs no more than contents: read — same limitation as last round, noted here again since it's the one thing in this diff I can't verify directly.

No blocking findings. The permissions concern from the previous round is fixed, not sidestepped.

[claude]

Claude finished @mark-brannan's task in 4m 7s —— View job

---

Independent review — round 3 (commit 880983f)

• Gather context (git diff origin/main...HEAD, all 3 commits; no CLAUDE.md/AGENTS.md in this repo)
• Review the workflow YAML for correctness
• Check for credentials/secrets in diff — none found
• Run repo verification commands
• Post inline finding + summary

What changed since the last round: a third commit (880983f) adds an explicit permissions: {} to the nonempty job, which was meant to close out the CodeQL/prior-round finding that it had no permissions block at all. Net diff is unchanged in shape: +9/-1, entirely in .github/workflows/test.yml.

Checked:

• Full diff vs origin/main — the nonempty job + its new permissions line, and ci-gate's needs updated to include it.
• No credential-like strings anywhere in the diff.
• Still no CLAUDE.md/AGENTS.md in this repo.

Ran:

• npm test (node --test) — 58/58 passed.
• npm ci — fails as expected: no package-lock.json shipped, matching test.yml's own comment that this repo has no runtime dependencies and tests must pass offline.
• npm run build — no build script exists (only unrelated build:demo).

Finding, posted inline on test.yml lines 29-32: permissions: {} is a real downgrade from what round 2 evaluated (implicit inheritance of the file's top-level contents: read), and its comment reuses ci-gate's "reads nothing, writes nothing" justification without adjusting it. ci-gate genuinely touches nothing — it only evaluates the JSON already in needs. A diff check is a different shape of job: to compare the PR head against its merge base it plausibly needs actions/checkout or an equivalent git operation, and a GITHUB_TOKEN scoped to contents: none is known to break actions/checkout's auth step even for public repos. I couldn't confirm this against the actual reusable workflow: mark-brannan/.github/.github/workflows/nonempty-diff.yml@main doesn't exist yet (mark-brannan/.github#25 is still open) — which is exactly why this PR's own test workflow run is a startup_failure (run 34883785830) rather than a job-level pass/fail, consistent with the PR description's explicit "merge #25 first" note. Not a blocker given that stated sequencing, but worth confirming the nonempty job itself goes green — not just that ci-gate does — on the first real run after #25 merges.

(Also flagging: gh pr comment for the top-level summary is blocked in this sandbox — the public-issue-guard hook can't load its private-terms denylist because the continuity state repo isn't checked out here. The inline comment above went through since it's a separate tool path; this comment carries the full review.)

Everything else from the prior two rounds still holds: minimal, correctly-scoped change, no credentials, consistent in intent with the repo's other reusable-workflow callers.

[claude]

Claude finished @mark-brannan's task in 50s —— View job

---

Independent review — round 4 (commit 00dd295)

• Gather context (git diff origin/main...HEAD, all 4 commits)
• Review the permissions fix for correctness
• Check for credentials/secrets in diff — none found
• Run repo verification commands
• Post findings

What changed: commit 00dd295 replaces the nonempty job's permissions: {} (added in round 3) with permissions: { contents: read }, and updates the explanatory comment to say the reusable workflow needs to check out the repo to diff against base. This is exactly the fix the round-3 finding asked for.

This resolves the round-3 finding. I don't have to hand-trace it this time — this PR's own CI ran with the fix in place, and I pulled the actual result:

• test workflow run 34884563879 is success.
• Critically, the nonempty / nonempty job itself is success (not just ci-gate / gate) — mark-brannan/.github#25 has merged, the reusable workflow resolves, and contents: read is sufficient for its checkout step. That was the specific thing round 3 flagged as unverified and asked to be confirmed once docs: finish the GeoJSON-first switch in demo, CONTRIBUTING, package #25 landed.

Checked:

• Full diff vs origin/main — 9 lines changed, all in .github/workflows/test.yml (permissions block + comment).
• No credential-like strings in the diff.
• Still no CLAUDE.md/AGENTS.md in this repo.

Ran:

• npm test (node --test) — 58/58 passed.
• npm ci — fails as expected (no package-lock.json shipped; repo has no runtime dependencies, matching test.yml's own comment that tests must pass offline).
• npm run build — no build script exists (only unrelated build:demo).
• Fetched this PR's live CI status and the test run's per-job results via the GitHub API (see above) rather than relying on the sandbox's blocked network access.

No blocking findings. All three prior review rounds' concerns (missing permissions block → CodeQL/round 1, permissions: {} breaking checkout → round 3) are resolved and now verified against a real green run of the nonempty job itself.
· branch add-nonempty-diff-check