Skip to content

Give skill gh access, auto-fix formatting, tighten GAPS.md#774

Merged
rdimitrov merged 1 commit intomainfrom
harden-skill-gh-access-and-autofix
Apr 21, 2026
Merged

Give skill gh access, auto-fix formatting, tighten GAPS.md#774
rdimitrov merged 1 commit intomainfrom
harden-skill-gh-access-and-autofix

Conversation

@rdimitrov
Copy link
Copy Markdown
Member

Four hardenings to make upstream-release-docs PRs land-ready without human pre-merge cleanup. Triggered by gaps surfaced in the first full end-to-end run on PR #759.

1. Grant the skill gh CLI access

The skill is designed (see .claude/skills/upstream-release-docs/SKILL.md) to use gh release view, gh pr view, and gh api compare throughout its phases. Claude Code's default Bash allowlist inside claude-code-action was blocking gh, so the skill fell back to working from commit messages and the local clone only. This missed most of the "why" narrative that PR authors write at open time.

Fix: add --allowed-tools "Bash(gh:*)" to claude_args on both the skill_gen and skill_review invocations. GH_TOKEN is already in the job env.

2. Rewrite the Phase 2 step 4 instruction

Previously we instructed the skill to skip writing the "why"/consumer narrative and route every major-feature gap into GAPS.md. That guaranteed a "Gaps needing human context" section on every substantive release, even when the information was fetchable.

Now: fetch PR descriptions via gh pr view, write a best-effort narrative directly in the docs, and defer to GAPS.md only when rationale demonstrably cannot be derived from available sources (internal design doc, multiple plausible consumer narratives, release-timeline confirmation needed from product team, etc.).

Reviewers refine "why" prose in normal review — that's the standard path for any PR, and it's lower-friction than requiring pre-merge context collection.

3. Tighten the GAPS.md contract

The v0.23.1 run produced a GAPS.md that included:

  • Legitimate content gaps (product motivation for v1beta1 graduation) ✓
  • Environment meta: "npm run build blocked in this sandbox" — not a content gap; PR CI handles build validation
  • Not-a-gap commentary: "The Cedar fix is already documented elsewhere" — color, not actionable

Explicitly exclude the latter two categories from GAPS.md. Require no GAPS.md at all when nothing is genuinely deferred.

Every GAPS.md entry must now include a paste-ready Helper prompt for local Claude block:

### v1alpha1 removal timeline (PR #4849 by @ChrisJBurns)

<paragraph on what's missing>

**File(s):** docs/toolhive/guides-k8s/migrate-to-v1beta1.mdx

**Helper prompt for local Claude:**
> Please update the "Removal timeline" section of
> docs/toolhive/guides-k8s/migrate-to-v1beta1.mdx with the
> planned version for v1alpha1 removal. PR #4849 did not
> commit to a specific release.

A reviewer resolves the gap by pasting the prompt into their local claude session. No reverse-engineering "what to ask" required.

4. Auto-fix prettier/eslint after the skill runs

npx prettier --write and npx eslint --fix aren't in the skill's sandbox, so formatting drift was landing on PR CI. On PR #759 this failed the "Lint and format checks" job on a single file until a human pushed a prettier fix.

Added an autofix step after skill_review that:

  • Scopes to files the skill touched via git diff --name-only <pre-skill-sha>..HEAD
  • Excludes auto-generated reference paths (docs/toolhive/reference/cli/, docs/toolhive/reference/crds/, static/api-specs/) so we don't fight generators
  • Runs prettier on all supported extensions, eslint on .mdx/.ts/.tsx/.js/.jsx
  • Commits the fixups as a separate commit ("Apply prettier and eslint fixups to skill output") if anything changed — clean history shows skill → formatter

Matches what the project's pre-commit hook does for human commits.

Also updated

Testing

The real test is the next Renovate-opened upstream PR (or a manual workflow_dispatch). Expected outcomes for that run:

  • No "No gh access" gap message in GAPS.md
  • The "why" narrative for major features populated directly in the docs rather than deferred
  • If GAPS.md is present, every entry includes a Helper prompt
  • No Lint and format checks failure on the resulting PR

Four concrete hardenings to make upstream-release-docs PRs
land-ready without human pre-merge cleanup:

1. Grant `gh` CLI to both skill invocations via claude_args
   `--allowed-tools "Bash(gh:*)"`. GH_TOKEN is already in the job
   env. Without this, Claude Code's default Bash allowlist blocks
   gh and the skill has to work from commit messages only, missing
   most of the "why" narrative that PR authors write at open time.
   The skill is already designed (SKILL.md) to use gh release view,
   gh pr view, gh api compare — we're just unblocking it.

2. Rewrite the Phase 2 step 4 instruction. Previously: "SKIP
   writing the why/consumer narrative and append to GAPS.md" —
   which guaranteed a "Gaps needing human context" section on
   every substantive release. Now: fetch PR descriptions via gh,
   write best-effort narrative directly in the docs, defer to
   GAPS.md only when rationale demonstrably cannot be derived
   from available sources.

3. Tighten the GAPS.md contract. Explicitly exclude environment
   limitations (the skill's v0.23.1 run flagged "npm run build
   blocked" as a gap — that's not a content gap, PR CI handles
   it) and "not a gap" commentary. Require each entry to include
   a paste-ready "Helper prompt for local Claude" block so a
   reviewer can resolve the gap by pasting into their local
   Claude Code rather than reverse-engineering what to ask.
   Require no GAPS.md file at all when nothing is genuinely
   deferred.

4. Add an auto-fix step after skill_review that runs
   `npx prettier --write` and `npx eslint --fix` against
   skill-touched files (scoped by git diff vs a pre-skill SHA,
   excluding auto-generated reference paths). The skill sandbox
   doesn't include these, so formatting drift was landing on PR
   CI — v0.23.1 failed the Lint and format checks job on one
   file. This matches the project's pre-commit hook behavior for
   human contributions.

Also updated:
  - Top-of-file comment block (was "Runs the upstream-release-docs
    skill (3 passes)").
  - The PR body template text at the previous "each ran twice
    (three total passes)" line — stale since PR #764 split the
    multi-pass inline design into two separate claude-code-action
    invocations. Now describes the two-Opus-session architecture.
  - The "fill them in before merging" review-guidance text now
    points reviewers to the Helper prompt blocks.
  - skill_review prompt now explicitly tells the editorial pass
    not to touch GAPS.md / NO_CHANGES.md signal files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 21, 2026 22:36
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs-website Ready Ready Preview, Comment Apr 21, 2026 10:36pm

Request Review

@rdimitrov rdimitrov merged commit c79729e into main Apr 21, 2026
6 checks passed
@rdimitrov rdimitrov deleted the harden-skill-gh-access-and-autofix branch April 21, 2026 22:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the upstream-release-docs GitHub Actions workflow so Renovate-driven upstream release PRs land with fewer manual fixups, by enabling richer upstream context retrieval and enforcing post-skill formatting.

Changes:

  • Grants the Claude skill sessions gh CLI access so they can fetch upstream PR/issue context for “why” narratives.
  • Updates the Phase 2 instructions to prefer writing best-effort rationale directly into docs and only create GAPS.md when truly unavoidable (with paste-ready helper prompts).
  • Adds a scoped Prettier/ESLint autofix step for files touched by the skill and updates PR-body guidance text accordingly.

Comment on lines +650 to +655
CHANGED=$(git diff --name-only "$BASELINE_SHA..HEAD" -- \
':!docs/toolhive/reference/cli/' \
':!docs/toolhive/reference/crds/' \
':!static/api-specs/' \
2>/dev/null | \
grep -E '\.(md|mdx|ts|tsx|js|jsx|mjs|cjs|css|json|jsonc|yaml|yml)$' || true)
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

In the autofix step, git diff --name-only ... -- ':!docs/... uses only negative pathspecs. In git, exclude pathspecs need at least one positive pathspec (commonly .) to match against; otherwise the result set can be empty and the formatter would never run. Add an explicit include pathspec (e.g., -- . ':!docs/...), or switch to filtering out excluded prefixes after collecting the full changed-file list.

Copilot uses AI. Check for mistakes.
Comment on lines 614 to 617
Run /docs-review over every file the previous commit
changed (use `git show --name-only HEAD` or `git diff
--name-only HEAD~1 HEAD` to find them). Apply every
actionable fix per the skill's standard guidance.
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

The skill_review prompt tells Claude to find changed files via git show --name-only HEAD / git diff --name-only HEAD~1 HEAD, but at this point in the workflow there hasn't been a post-skill commit yet (the commit happens later in the separate Commit and push step). Those commands will reflect only the last committed refresh/pre-workflow state and can cause docs-review to miss the actual skill output. Update the instructions to derive the file list from the working tree (e.g., git diff --name-only against HEAD, or git status --porcelain), or explicitly commit skill output before running skill_review.

Copilot uses AI. Check for mistakes.
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.

3 participants