Skip to content

Commit c79729e

Browse files
rdimitrovclaude
andauthored
Give skill gh access, auto-fix formatting, tighten GAPS.md contract (#774)
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>
1 parent 5a6cd94 commit c79729e

1 file changed

Lines changed: 149 additions & 26 deletions

File tree

.github/workflows/upstream-release-docs.yml

Lines changed: 149 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@ name: Upstream Release Docs
1616
# bundle-upstream-schema.mjs to produce our opinionated reference
1717
# MDX (the CRD tarball and toolhive-core schemas are shipped as
1818
# release assets by stacklok/toolhive#4982)
19-
# 5. Runs the upstream-release-docs skill (3 passes) to produce
20-
# source-verified content edits
21-
# 6. Commits everything to the PR branch, augments the PR body,
19+
# 5. Runs two Claude Opus sessions: a generation pass running the
20+
# upstream-release-docs skill, then a fresh-context editorial
21+
# pass running docs-review over the changed files. Both have
22+
# `gh` CLI access so they can fetch PR descriptions and linked
23+
# issues for product context.
24+
# 6. Auto-fixes prettier/eslint on the skill-touched files,
25+
# commits everything to the PR branch, augments the PR body,
2226
# assigns reviewers from non-bot release contributors
2327
#
2428
# Renovate is configured with rebaseWhen: never + recreateWhen: never
@@ -378,6 +382,15 @@ jobs:
378382
git push origin "HEAD:$HEAD_REF"
379383
fi
380384
385+
# Anchor the "skill touched" set for the autofix step below.
386+
# HEAD at this point is either the refresh commit (if it was
387+
# non-empty) or the PR's pre-workflow tip. Either way, anything
388+
# committed between this SHA and HEAD after the skill runs is
389+
# attributable to the skill.
390+
- name: Capture pre-skill SHA
391+
id: pre_skill
392+
run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
393+
381394
- name: Extract reviewers from release compare
382395
id: reviewers
383396
env:
@@ -460,8 +473,16 @@ jobs:
460473
# Actions Step Summary regardless of trigger.
461474
track_progress: ${{ github.event_name == 'pull_request' }}
462475
display_report: true
476+
# Grant the `gh` CLI so the skill can fetch PR descriptions,
477+
# compare ranges, and view linked issues as it was designed
478+
# to. Without this, Claude Code's default Bash allowlist
479+
# rejects gh and the skill has to work from commit messages
480+
# only, which misses most of the "why" narrative that PR
481+
# authors write at open time. GH_TOKEN for auth is already
482+
# in the job env at the top of this workflow.
463483
claude_args: |
464484
--model claude-opus-4-7
485+
--allowed-tools "Bash(gh:*)"
465486
prompt: |
466487
You are running in GitHub Actions with no interactive user. Follow
467488
these steps exactly and do NOT ask clarifying questions -- proceed
@@ -480,22 +501,69 @@ jobs:
480501
instead of `gh api contents?ref=<tag>` -- it's already at
481502
the tag and doesn't consume API quota.
482503
483-
For Phase 2 step 4 (context on major new features), SKIP
484-
writing the "why"/consumer narrative and append one bullet
485-
per gap to GAPS.md at repo root (create if missing). Each
486-
bullet MUST:
487-
- Name the feature
488-
- Reference the PR that introduced it, using the PR
489-
number you found in Phase 2 deep-dive
490-
- @-mention the PR author by their GitHub handle (skip
491-
this for bot authors like renovate[bot] or
492-
github-actions[bot])
493-
- Describe what context a human needs to supply
494-
Format: `- Feature X (PR #123 by @alice): needs a user
495-
story explaining who this is for and the expected
496-
consumer workflow.`
497-
This routes gaps to the engineer who built the feature
498-
rather than the whole reviewer pool.
504+
For Phase 2 step 4 (context on major new features), do the
505+
following INSTEAD of the skill's default "ask the user"
506+
behavior (there is no interactive user here):
507+
1. Use `gh pr view <NUMBER> --repo ${{ steps.detect.outputs.repo }}
508+
--json title,body,author` to fetch the PR description
509+
for each major new feature. The PR body typically
510+
contains the "why" framing the author wrote when
511+
opening the PR -- motivation, intended consumers,
512+
design decisions.
513+
2. If the PR body references linked issues (look for
514+
"Closes #N", "Fixes #N", "Refs #N"), fetch those
515+
with `gh issue view <N> --repo ${{ steps.detect.outputs.repo }}`
516+
when they are likely to contain product context.
517+
3. Write the "why"/consumer narrative directly in the
518+
relevant docs page using what you learned. This is a
519+
best-effort pass -- reviewers will refine it during
520+
normal review.
521+
4. Only defer to GAPS.md when the rationale demonstrably
522+
cannot be derived from available sources. Genuine
523+
examples: the PR body points to an internal design
524+
doc or product spec you cannot access; multiple
525+
plausible consumer narratives exist and picking one
526+
would mislead readers; a release timeline or
527+
commitment needs product-team confirmation.
528+
529+
GAPS.md contract (only if you genuinely need to defer):
530+
- ONLY include content gaps a human reviewer must fill
531+
in. Do NOT include environment/sandbox limitations
532+
(e.g. "couldn't run npm build") -- those are
533+
infrastructure concerns; the PR's CI handles them.
534+
Do NOT include "not a gap, documented for clarity"
535+
commentary.
536+
- Each entry MUST include a "Helper prompt for local
537+
Claude" block a reviewer can paste verbatim into
538+
their local Claude Code to resolve the gap. The
539+
prompt must reference the specific file(s) to edit,
540+
the PR number that provides context, and the narrow
541+
piece of information the human needs to supply or
542+
confirm.
543+
- Each entry MUST @-mention the PR author (skip this
544+
for bot authors: renovate[bot], github-actions[bot],
545+
stacklokbot).
546+
547+
GAPS.md entry format:
548+
```
549+
### <Feature name> (PR #123 by @alice)
550+
551+
<One paragraph: what's missing, why you couldn't
552+
resolve it from available sources.>
553+
554+
**File(s):** path/to/file.mdx
555+
556+
**Helper prompt for local Claude:**
557+
> <Paste-ready prompt. Self-contained. References the
558+
> specific file(s), the PR number, and the narrow
559+
> piece of info needed. A human should be able to
560+
> paste this into `claude` locally and get a usable
561+
> response without additional context.>
562+
```
563+
564+
Do NOT create GAPS.md if every feature's "why" was
565+
resolvable from PR descriptions. An empty GAPS.md is
566+
worse than no file.
499567
500568
Follow the skill's own guidance on auto-generated reference
501569
files (Phase 4 step 5, Phase 4 step 6) -- do not hand-edit
@@ -527,8 +595,12 @@ jobs:
527595
allowed_bots: 'renovate'
528596
track_progress: ${{ github.event_name == 'pull_request' }}
529597
display_report: true
598+
# gh access parallels skill_gen so the review pass can
599+
# re-verify claims against PR descriptions and linked
600+
# issues if needed.
530601
claude_args: |
531602
--model claude-opus-4-7
603+
--allowed-tools "Bash(gh:*)"
532604
prompt: |
533605
You are running in GitHub Actions with no interactive user. Follow
534606
these steps exactly and do NOT ask clarifying questions -- proceed
@@ -544,17 +616,68 @@ jobs:
544616
--name-only HEAD~1 HEAD` to find them). Apply every
545617
actionable fix per the skill's standard guidance.
546618
547-
If you spot a factual concern, re-verify against source
548-
code in the local clone at
549-
${{ steps.clone.outputs.scratch_dir }} before changing
550-
anything. Don't introduce new claims; only refine what's
551-
already there.
619+
If you spot a factual concern, re-verify against the
620+
local clone at ${{ steps.clone.outputs.scratch_dir }}
621+
first. You also have gh CLI access -- use `gh pr view
622+
<N> --repo ${{ steps.detect.outputs.repo }}` to pull
623+
context from the upstream PR if needed. Don't introduce
624+
new claims; only refine what's already there.
552625
553626
Do NOT re-run /upstream-release-docs. Do NOT touch files
554627
under docs/toolhive/reference/cli/, static/api-specs/,
555628
or docs/toolhive/reference/crds/ — those are
556629
regenerated from release assets and aren't yours to edit.
557630
631+
Do NOT touch GAPS.md or NO_CHANGES.md at the repo root
632+
if they exist -- they're signal files handed off to the
633+
next workflow step, not part of the docs.
634+
635+
# Auto-apply the same formatters the project's pre-commit hook
636+
# runs, scoped to the files the skill touched. The skill's
637+
# sandbox doesn't include npm run prettier/eslint, so without
638+
# this step any formatting drift lands on PR CI as a "Lint and
639+
# format checks" failure requiring a human push. Scope via git
640+
# diff vs the pre-skill SHA so we don't reformat auto-generated
641+
# reference assets (which would fight the generators).
642+
- name: Auto-fix prettier and eslint on skill-touched files
643+
id: autofix
644+
if: always() && steps.skill_gen.conclusion == 'success'
645+
env:
646+
BASELINE_SHA: ${{ steps.pre_skill.outputs.sha }}
647+
run: |
648+
# Files the skill added/modified, excluding the three
649+
# auto-generated reference paths.
650+
CHANGED=$(git diff --name-only "$BASELINE_SHA..HEAD" -- \
651+
':!docs/toolhive/reference/cli/' \
652+
':!docs/toolhive/reference/crds/' \
653+
':!static/api-specs/' \
654+
2>/dev/null | \
655+
grep -E '\.(md|mdx|ts|tsx|js|jsx|mjs|cjs|css|json|jsonc|yaml|yml)$' || true)
656+
if [ -z "$CHANGED" ]; then
657+
echo "No skill-touched files in scope for autofix."
658+
exit 0
659+
fi
660+
echo "Running prettier --write and eslint --fix on:"
661+
echo "$CHANGED"
662+
# xargs -0 with a NUL-delimited list so filenames with
663+
# spaces survive.
664+
printf '%s\n' "$CHANGED" | tr '\n' '\0' | \
665+
xargs -0 npx prettier --write --log-level warn
666+
# eslint --fix against mdx/ts/tsx/js only.
667+
LINT_TARGETS=$(printf '%s\n' "$CHANGED" | \
668+
grep -E '\.(mdx|ts|tsx|js|jsx|mjs|cjs)$' || true)
669+
if [ -n "$LINT_TARGETS" ]; then
670+
printf '%s\n' "$LINT_TARGETS" | tr '\n' '\0' | \
671+
xargs -0 npx eslint --fix --no-error-on-unmatched-pattern || \
672+
echo "::warning::eslint --fix reported non-zero; proceeding."
673+
fi
674+
if git diff --quiet; then
675+
echo "No formatting changes needed."
676+
else
677+
git add -A
678+
git commit -m "Apply prettier and eslint fixups to skill output"
679+
fi
680+
558681
- name: Capture skill signal files
559682
id: signals
560683
run: |
@@ -684,7 +807,7 @@ jobs:
684807
echo ""
685808
echo "## Content additions by upstream-release-docs"
686809
echo ""
687-
echo "Source-verified against \`$REPO\` at tag \`$NEW_TAG\` (was \`$PREV_TAG\`). The \`upstream-release-docs\` and \`docs-review\` skills each ran twice (three total passes) before this update."
810+
echo "Source-verified against \`$REPO\` at tag \`$NEW_TAG\` (was \`$PREV_TAG\`). Two Claude Opus sessions produced this update: a generation pass running the \`upstream-release-docs\` skill over all six phases, then a fresh-context editorial pass running \`docs-review\` over the changed files. Prettier and ESLint auto-fixes were applied afterward."
688811
echo ""
689812
if [ "$COMPARE_OK" != "true" ]; then
690813
echo "> [!WARNING]"
@@ -701,7 +824,7 @@ jobs:
701824
fi
702825
echo "### Review guidance"
703826
echo ""
704-
echo "Machine-generated reference files under \`docs/toolhive/reference/cli/\`, \`static/api-specs/\`, and \`docs/toolhive/reference/crds/\` are synced or regenerated from upstream release assets (separate commit, titled \"Refresh reference assets\") and should be spot-checked only. The \"Add upstream-release-docs content\" commit contains hand-edited prose; review that one for accuracy, not just style. If the \"Gaps needing human context\" section is populated, the skill deferred those sections to a human; fill them in before merging."
827+
echo "Machine-generated reference files under \`docs/toolhive/reference/cli/\`, \`static/api-specs/\`, and \`docs/toolhive/reference/crds/\` are synced or regenerated from upstream release assets (separate commit, titled \"Refresh reference assets\") and should be spot-checked only. Commits authored by the skill contain hand-edited prose; review those for accuracy, not just style. If the \"Gaps needing human context\" section below is populated, each entry includes a **Helper prompt for local Claude** that a reviewer can paste verbatim into their local Claude Code session to resolve the gap. Fill those in before merging."
705828
echo ""
706829
if [ -n "$GAPS_BLOCK" ]; then
707830
echo "$GAPS_BLOCK"

0 commit comments

Comments
 (0)