Skip to content

Fix tool denials and restore build validation in release docs workflow #1114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danbarr merged 2 commits into from
Aug 24, 2026
Merged

Fix tool denials and restore build validation in release docs workflow #1114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e344aaa
Fix tool denials and restore build validation
danbarr Aug 24, 2026
90bac5c
Address Copilot review on the tool-permission fixes
danbarr Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 33 additions & 8 deletions .claude/skills/upstream-release-docs/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ Analyze a new release of an upstream project and update the documentation site t

## Core Principle

**Verify everything against source code at the release tag.** Never trust release notes, PR descriptions, PR review comments, or issue descriptions at face value. Always check actual source code using:
**Verify everything against source code at the release tag.** Never trust release notes, PR descriptions, PR review comments, or issue descriptions at face value. Always check the actual source code.

```bash
gh api repos/<OWNER>/<REPO>/contents/<PATH>?ref=<TAG>
```
How you reach that source depends on what the caller gave you:

- **A local clone at the tag** (an automated caller usually provides one, and names its path in the invocation): read it with the Read, Grep, and Glob tools. Prefer this; it costs no API quota. See [Reading the upstream clone](#reading-the-upstream-clone) for the constraints.
- **No clone:** fetch individual files from the API, and decode the base64 response.

```bash
gh api repos/<OWNER>/<REPO>/contents/<PATH>?ref=<TAG>
```

Claims from any human-written source (release notes, PR bodies, review comments) may be inaccurate, outdated, or aspirational. The source code at the tag is the single source of truth.

Expand Down Expand Up @@ -52,6 +57,17 @@ When Phase 2 step 4 would normally ask the user for a major feature's "why", ins
3. Write the "why"/consumer narrative directly into the relevant page using what you learned, translated into reader-facing language rather than the PR's engineering shorthand. This is best-effort; reviewers refine it later.
4. Defer to `GAPS.md` only when the rationale demonstrably cannot be derived from available sources: the PR points to an internal design doc you cannot access, multiple plausible consumer narratives exist and choosing one would mislead readers, or a release timeline or commitment needs product-team confirmation.

### Reading the upstream clone

When a caller provides a local clone of the upstream repo at the release tag, read it with the **Read, Grep, and Glob tools**. Prefer it over `gh api contents?ref=<tag>`: it is already at the tag and costs no API quota.

Do not reach for Bash to explore it. An automated caller typically clones to a scratch directory outside the session's working directory, so `ls`, `find`, and `grep` run through Bash are refused there, and `git -C <path>` is refused everywhere because it can execute untrusted hooks from the target repo. Read, Grep, and Glob have no such restriction and are the right tools regardless.
Comment thread
danbarr marked this conversation as resolved.

Two consequences worth internalizing, because working around them by retrying Bash variants wastes turns and never succeeds:

- **Don't try to run `git log` on the clone.** When the caller supplies `.release-meta.json`, its `commits` array is the release range's commit list. Otherwise derive the range from the release notes and `gh api`.
- **Don't append `2>&1` or chain with `&&`** on any Bash call. That splits the command into parts that no longer match the caller's tool allowlist, so the call is denied even when the underlying command is permitted.

### Artifacts (unattended mode only, written at repo root)

These files are read by the automated caller and spliced into the PR body. The filenames and the repo-root location are a contract with the caller; do not rename or relocate them.
Expand Down Expand Up @@ -87,10 +103,21 @@ Read `.release-meta.json` first (the caller writes it before invoking you):
"new_tag": "v0.43.0",
"owner": "jerm-dro",
"owner_source": "merged release PR stacklok/toolhive#6333",
"contributors": ["alice", "bob", "carol"]
"contributors": ["alice", "bob", "carol"],
"commits": [
{
"sha": "8343851e",
"subject": "Push skills unsigned until keyless signing lands (#6334)",
"author": "alice"
}
]
}
```

`commits` is the commit list for the release range: use it instead of trying to run `git log` against the upstream clone, which is refused (see [Reading the upstream clone](#reading-the-upstream-clone)). It is also what tells you which commits belong to which contributor for the classification below.

When `commits_truncated` is `true`, the range exceeded what the caller could fetch in one request, so both `commits` and `contributors` are partial. Treat the release notes as the authoritative list of changes for that run, and say in `SUMMARY.md` that the commit list was truncated so a reviewer knows the classification may have missed someone.

Classify **every** login in `contributors` as docs-facing or not, and write `REVIEWERS.json` at the repo root:

```json
Expand Down Expand Up @@ -192,14 +219,12 @@ For each PR identified in Phase 1 (skip internal/infra unless user requests):

5. **Check related repositories**: components often span multiple repos. For example, a server's CRD/operator may live in a different repo than the server itself. When a release changes config structures, API surfaces, or deployment models, check whether related repos (operators, CLIs, client libraries) have also released changes that affect the documentation. Ask the user which repos are related if unclear (in unattended mode, infer related repos from the release notes and proceed best-effort).

6. **Read the actual source code at the release tag** to verify every claim made in the PR description:
6. **Read the actual source code at the release tag** to verify every claim made in the PR description. Use the local clone when the caller provided one, per [Core Principle](#core-principle); otherwise fetch the file from the API and decode the base64 response:

```bash
gh api repos/<OWNER>/<REPO>/contents/<PATH>?ref=<TAG>
```

The response is base64-encoded; decode it to read the content.

7. Note discrepancies between PR descriptions and actual code. Trust the code.

**A PR body describes the moment it was written, not the release.** This is a distinct failure from a PR body simply being wrong: the body was accurate when opened, then later commits, review changes, or a follow-up PR in the same release moved past it. `stacklok/docs-website#1037` documented a field as "not wired up" because an upstream PR note said so; by the time the release shipped it was wired up, and the note was months stale. A caveat, limitation, or "not yet supported" claim in a PR body is the single least trustworthy kind of statement you will read, because it is a claim about absence, and absence is exactly what a later commit silently fixes. Before writing any limitation into the docs, verify in source at the tag that the limitation still holds. If you cannot confirm it, leave it out rather than repeating it.
Expand Down
126 changes: 106 additions & 20 deletions .github/workflows/upstream-release-docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,26 @@ jobs:
if: steps.cache.outputs.cache-hit != 'true'
run: npm ci

# `npm run build` needs `thv` on PATH: mcp-metadata-plugin
# shells out to `thv registry info`, and docusaurus.config.ts
# sets its failOnError whenever CI=true. Same script Vercel's
# buildCommand runs. Required, not soft-failed, so a broken
# install fails here rather than mid-skill.
- name: Install ToolHive CLI (for build-time MCP metadata)
run: |
./scripts/install-thv.sh
# The script falls back to ~/.local/bin when /usr/local/bin
# isn't writable, and exports PATH only in its own shell.
# Persist that so later steps see it, then assert, so a miss
# fails here instead of as a confusing build error mid-skill.
echo "$HOME/.local/bin" >> "$GITHUB_PATH"
export PATH="$HOME/.local/bin:$PATH"
command -v thv >/dev/null || {
echo "::error::thv not on PATH after install-thv.sh"
exit 1
}
thv version

- name: Set up Git
run: |
git config --global user.name "github-actions[bot]"
Expand Down Expand Up @@ -472,14 +492,21 @@ jobs:
printf '%s\n' "$1" | grep -qE '(\[bot\]$|^app/|^github-actions|^stacklokbot$|^dependabot|^renovate|^copilot)'
}

# ---------- contributor candidates ----------
if COMPARE=$(gh api "repos/$REPO/compare/$PREV...$NEW" \
--jq '[.commits[].author.login? // empty] | unique | .[]' 2>/dev/null); then
# ---------- compare the release range ----------
# One fetch, two consumers: the contributor logins below and
# the commit list handed to the skill, which cannot run
# `git log` against the clone and needs it as data.
if COMPARE_JSON=$(gh api "repos/$REPO/compare/$PREV...$NEW" 2>/dev/null); then
Comment thread
danbarr marked this conversation as resolved.
COMPARE_OK=true
else
COMPARE=""
COMPARE_JSON=""
COMPARE_OK=false
fi
COMPARE=""
if [ -n "$COMPARE_JSON" ]; then
COMPARE=$(printf '%s' "$COMPARE_JSON" \
| jq -r '[.commits[].author.login? // empty] | unique | .[]' 2>/dev/null || true)
fi
CANDIDATES=""
while IFS= read -r login; do
[ -z "$login" ] && continue
Expand Down Expand Up @@ -542,20 +569,47 @@ jobs:
# ---------- handoff file for the skill ----------
# The skill reads .release-meta.json to learn who the owner
# is and which contributors it must classify. Gitignored.
# One entry per commit in the range: short sha, subject,
# author login. Empty when the compare failed.
COMMITS_JSON='[]'
COMMITS_TRUNCATED=false
if [ -n "$COMPARE_JSON" ]; then
COMMITS_JSON=$(printf '%s' "$COMPARE_JSON" | jq -c '
[ .commits[]
| { sha: (.sha[0:8]),
subject: (.commit.message | split("\n")[0]),
author: (.author.login? // .commit.author.name? // null) } ]' \
2>/dev/null || echo '[]')
# The compare endpoint caps at 250 commits. Past that both
# the commit list and the contributor list are partial, and
# the skill treats commits as the complete range, so say so
# rather than handing over silently-short data.
TOTAL=$(printf '%s' "$COMPARE_JSON" | jq -r '.total_commits // 0' 2>/dev/null || echo 0)
RETURNED=$(printf '%s' "$COMPARE_JSON" | jq -r '.commits | length' 2>/dev/null || echo 0)
if [ "$TOTAL" -gt "$RETURNED" ]; then
COMMITS_TRUNCATED=true
echo "::warning::Compare returned $RETURNED of $TOTAL commits; commit and contributor lists are partial."
fi
fi

jq -n \
--arg owner "$OWNER" \
--arg owner_source "$OWNER_SOURCE" \
--arg repo "$REPO" \
--arg prev "$PREV" \
--arg new "$NEW" \
--arg candidates "$CANDIDATES" \
--argjson commits "$COMMITS_JSON" \
--argjson commits_truncated "$COMMITS_TRUNCATED" \
'{
repo: $repo,
prev_tag: $prev,
new_tag: $new,
owner: (if $owner == "" then null else $owner end),
owner_source: (if $owner_source == "" then null else $owner_source end),
contributors: ($candidates | split("\n") | map(select(length > 0)))
contributors: ($candidates | split("\n") | map(select(length > 0))),
commits: $commits,
commits_truncated: $commits_truncated
}' > .release-meta.json

{
Expand Down Expand Up @@ -683,7 +737,12 @@ jobs:
claude_args: |
--model claude-opus-4-7
--max-turns 1000
--allowedTools "Bash(gh:*),Bash(npm run build),Bash(npm run prettier),Bash(npm run prettier:fix),Bash(npm run eslint),Bash(npm run eslint:fix)"
# Exact entries for the npm scripts, because they already
# act on the whole repo (`prettier . --write`,
# `eslint . --fix`) and so never need a file list. The
# prompt says to run them bare. Git entries are read-only
# inspection; the action's defaults cover the write side.
--allowedTools "Bash(gh:*),Bash(npm run build),Bash(npm run prettier),Bash(npm run prettier:fix),Bash(npm run eslint),Bash(npm run eslint:fix),Bash(git log:*),Bash(git diff:*),Bash(git show:*),Bash(git check-ignore:*)"
prompt: |
You are running in GitHub Actions with no interactive user.

Expand Down Expand Up @@ -712,22 +771,40 @@ jobs:
back to requesting a review from every contributor, which
is the noisy behavior we are trying to fix.

Prefer reading source code from the local clone at
${{ steps.clone.outputs.scratch_dir }} instead of
Read source from the local clone at
${{ steps.clone.outputs.scratch_dir }} rather than
`gh api contents?ref=<tag>` -- it's already at the tag and
doesn't consume API quota. Do not hand-edit auto-generated
doesn't consume API quota. Use the **Read, Grep, and Glob
tools** for it, not Bash: the clone sits outside this
session's working directory, so `ls`, `find`, and `grep`
run through Bash are refused there, and `git -C <path>` is
refused everywhere because it can execute untrusted hooks.
The commit list you would otherwise get from `git log` is
already in `.release-meta.json` under `commits`, one entry
per commit with its sha, subject, and author.

Do not append `2>&1` or chain commands with `&&` on Bash
calls. Doing so splits the command into parts that no
longer match the tool allowlist, and the call is denied
even when the underlying command is permitted.

Do not hand-edit auto-generated
reference under static/api-specs/, docs/toolhive/reference/cli/,
or any CRD pages dirs (${{ steps.crd_pages.outputs.dirs }}) --
earlier workflow steps sync those from release assets.

Project dependencies are already installed by an earlier
workflow step; node_modules is present in the working tree.
Run `npm run prettier:fix` and `npm run eslint:fix`
directly to format and lint your edits. Do NOT run
`npm ci` or `npm install` -- they are unnecessary here and
not in the allow-list; if a lint command appears to require
a fresh install, it does not. Do not defer formatting to
CI: the lint and format CI checks do not run on commits
with NO arguments to format and lint your edits. Both
scripts already act on the whole repo, so a file list adds
nothing and takes the command outside the allow-list. Do
NOT run `npm ci` or `npm install` -- they are unnecessary
here; if a lint command appears to require a fresh install,
it does not. Do not append `2>&1` or chain with `&&`: that
splits the command into parts the allowlist no longer
matches and the call is denied. Do not defer formatting to
CI, since the lint and format checks do not run on commits
pushed by this bot account. Format your edits in this
session.

Expand Down Expand Up @@ -805,7 +882,12 @@ jobs:
claude_args: |
--model claude-opus-4-7
--max-turns 200
--allowedTools "Bash(gh:*),Bash(npm run build),Bash(npm run prettier),Bash(npm run prettier:fix),Bash(npm run eslint),Bash(npm run eslint:fix)"
# Exact entries for the npm scripts, because they already
# act on the whole repo (`prettier . --write`,
# `eslint . --fix`) and so never need a file list. The
# prompt says to run them bare. Git entries are read-only
# inspection; the action's defaults cover the write side.
--allowedTools "Bash(gh:*),Bash(npm run build),Bash(npm run prettier),Bash(npm run prettier:fix),Bash(npm run eslint),Bash(npm run eslint:fix),Bash(git log:*),Bash(git diff:*),Bash(git show:*),Bash(git check-ignore:*)"
prompt: |
You are running in GitHub Actions with no interactive user. Follow
these steps exactly and do NOT ask clarifying questions -- proceed
Expand All @@ -824,11 +906,15 @@ jobs:
Project dependencies are already installed by an earlier
workflow step; node_modules is present in the working tree.
Run `npm run prettier:fix` and `npm run eslint:fix`
directly to format and lint your edits. Do NOT run
`npm ci` or `npm install` -- they are unnecessary here and
not in the allow-list; if a lint command appears to require
a fresh install, it does not. Do not defer formatting to
CI: the lint and format CI checks do not run on commits
with NO arguments to format and lint your edits. Both
scripts already act on the whole repo, so a file list adds
nothing and takes the command outside the allow-list. Do
NOT run `npm ci` or `npm install` -- they are unnecessary
here; if a lint command appears to require a fresh install,
it does not. Do not append `2>&1` or chain with `&&`: that
splits the command into parts the allowlist no longer
matches and the call is denied. Do not defer formatting to
CI, since the lint and format checks do not run on commits
pushed by this bot account. Format your edits in this
session.

Expand Down