Skip to content

ci: add @claude /focus to point human reviewers at the sections that need them - #404

Merged
thecodedrift merged 2 commits into
mainfrom
ci/claude-focus-on-demand
Sep 27, 2026
Merged

thecodedrift merged 2 commits into
mainfrom
ci/claude-focus-on-demand

Conversation

@thecodedrift

Copy link
Copy Markdown
Member

Adds @claude /focus [N], an on-demand sibling of @claude /review. It doesn't look for bugs. It triages the diff for a human reviewer: diffs move a lot of code and human attention is finite, so it picks the N sections (default 3, clamped to 1–10) where human judgement adds the most, and posts each one as an inline review comment.

What it posts

  • Inline, per area, most important first: Focus area K of M: <title>, why it needs a human, 2–4 questions to answer before approving (questions, not findings or suggested code), and related locations to also read.
  • One summary comment: Focus areas: K of N requested, the ranked list with path:line, and a Safe to skim section naming the parts of the diff that need little attention.

It targets architecture and boundaries, contracts (API, CLI output, formats, schemas), persisted or migrated state, trust boundaries, and code that is hard to reason about locally. It skips renames, generated files, moved-but-unchanged code and routine tests. It posts fewer than N rather than padding the list.

Combined with /review's inline trigger, a reviewer can reply @claude /review on a focus thread to pull Claude into that specific area.

Carried over from /review

  • Maintainer-only (author_association gate), PR conversation only.
  • The comment body is tested, never forwarded: only N is extracted, as digits, in a shell regex with a word boundary and horizontal-whitespace separator.
  • Same read-only allowlist and base-prompt correction: no git, no gh api, no contents: write.
  • Checkout of refs/pull/N/head with persist-credentials: false, the pairing that makes Read safe.
  • Existing review threads are fetched by a workflow step into .prior-review.json. An area with an open human thread already has attention, and a re-run doesn't re-post unresolved areas from an earlier /focus.
  • A verify step fails any run that left no bot comment citing this run ID with the Focus areas: marker. It is inline rather than a shared script because the checkout is contributor-authored and the job holds a write token.

Testing

N parsing was checked locally against edge cases: /focus → 3, /focus 5 → 5, /focus 08 → 8, /focus 0 → 1, /focus 123 → 10, /focusing 5 → 3, a number on the next line → 3. Prettier and pnpm lint pass.

This can't be exercised on this PR. issue_comment workflows run from the default branch, so the first real run is after merge. The /review workflow is untouched.

…s that need them

A maintainer-only, on-demand sibling of @claude /review. Instead of looking
for bugs, it triages the diff for a human reviewer: it posts up to N (default
3, clamped to 1..10) inline comments on the architectural, structural, or
hard-to-reason-about sections, each framed as questions to answer before
approving, plus a summary naming what is safe to skim.

It carries over every boundary of the review workflow: the author_association
gate, a comment body that is tested but never forwarded (only N is extracted,
as digits), a read-only allowlist with no git or gh api, the PR-ref checkout
without persisted credentials, and a verify step that fails a run which
posted nothing.
@thecodedrift thecodedrift added the skip-changeset PR intentionally ships no release note (bypasses the changeset requirement) label Sep 27, 2026
@thecodedrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Claude finished @thecodedrift's task in 6m 2s —— View job


Review mode: incremental — read 0 prior review thread(s) before reviewing.

.prior-review.json contains no reviewThreads and no reviews, only the top-level trigger comment (@claude /review) itself. There is nothing previously raised to carry forward — this is effectively the first review of this PR.

  • Read .prior-review.json
  • Read the diff (gh pr diff / Read) — the PR adds one file, .github/workflows/claude-code-focus-on-demand.yml (+388)
  • Reviewed for correctness, security, clarity
  • Posted findings

Summary

This is a careful port of claude-code-review-on-demand.yml's security boundaries (author_association gate, read-only tool allowlist, no contents: write, persist-credentials: false + PR-ref checkout, comment body only tested/never forwarded) to a new /focus command. The N-parsing regex in "Prepare focus context" is correct against all the edge cases named in the PR description — I traced /focus, /focus 5, /focus 08, /focus 0, /focus 123, /focusing 5, and a number on the next line by hand and all match the stated expected outputs (3, 5, 8, 1, 10, 3, 3 respectively). I didn't find a correctness or security bug in the new logic.

[New] One finding, posted inline on line 261: the ~130-line "verify the run actually posted" step is duplicated almost verbatim from the sibling review workflow (only the Focus areas: / Review mode: marker differs, and a couple of diagnostic-only blocks were dropped in this copy). The file's own comment at line 133 warns that the marker string is "a contract... reword it in both places in the same commit, or every run fails" — that warning now spans two files instead of one, so a future fix to the shared bot-comment-detection logic can easily be applied to one copy and silently missed in the other. Suggest factoring it into a composite action shared by both workflows.

Nothing else risen to the level of a finding: permissions, the GraphQL thread-fetch scoping, the checkout ref, and the tool allowlist are all consistent with the already-established sibling pattern, and the PR's own limitation notice (this can't be exercised until merge, since issue_comment runs off the default branch) is accurate and appropriately called out.

Comment thread .github/workflows/claude-code-focus-on-demand.yml
…re not shared

A local composite action loads from the checkout, which in both workflows is
the PR's contributor-authored tree, so sharing the step that way would run
PR-controlled code with the job's write token. Each copy now names the other
so a fix to the detection logic reaches both.
@thecodedrift
thecodedrift merged commit 79584f1 into main Sep 27, 2026
5 checks passed
@thecodedrift
thecodedrift deleted the ci/claude-focus-on-demand branch September 27, 2026 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR intentionally ships no release note (bypasses the changeset requirement)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant