Skip to content

skills(running-tend): check the /code-review waiver scan when bumping claude_version - #926

Open
tend-agent wants to merge 2 commits into
mainfrom
skills/bump-waiver-scan-31306655772
Open

skills(running-tend): check the /code-review waiver scan when bumping claude_version#926
tend-agent wants to merge 2 commits into
mainfrom
skills/bump-waiver-scan-31306655772

Conversation

@tend-agent

@tend-agent tend-agent commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

The weekly claude_version bump rule lists the agent paths to skim the CHANGELOG for — first-run onboarding, --model alias resolution, headless -p result events, Stop-hook behavior. It omits the one dependency that can't be settled from the CHANGELOG at all: the /code-review waiver scan.

code_review_notice in generator/src/tend/config.py puts a bare /code-review token into every generated Claude prompt, specifically to trip the Skill tool's disable-model-invocation waiver. The regex that reads that token lives inside the claude binary, and test_default_prompt_unlocks_code_review pins only our side of the contract — it asserts the prompt still carries a token matching the regex we recorded. If the binary's scan changes, that test stays green, no run errors, and every review silently loses its second pass. That is exactly the failure class a version bump can introduce.

This adds slash-command or Skill-tool handling to the CHANGELOG skim list, and a short recipe that checks the two halves against the binary being pinned.

The gap is not hypothetical — it just fired. #918's body reached the right conclusion about the 2.1.223 /code-review entry ("no action needed") from the wrong evidence: it reasoned about which skill the review path invokes rather than about the token in the generated prompt. The checklist it followed didn't point at the real dependency, so nothing prompted a binary-side check. The reviewer caught it and verified separately.

Verification of the recipe, against 2.1.226

The download URL is the path the install actually takes. claude.ai/install.sh 302s to downloads.claude.ai/claude-code-releases/bootstrap.sh, which sets DOWNLOAD_BASE_URL="https://downloads.claude.ai/claude-code-releases" and fetches $DOWNLOAD_BASE_URL/$version/$platform/claude. The downloaded binary's sha256 matches that release's manifest.json entry for linux-x64 (4e9bec11...a55555).

Both greps were run verbatim against the downloaded 2.1.226 binary:

$ grep -aoE '\(\?<!\\\\S\)/\$\{[^}]+\}\(\?=\$\|\\\\s\)' /tmp/claude-2.1.226
(?<!\\S)/${f2(e)}(?=$|\\s)

$ grep -aoE '.{90}"code-review".{40}' /tmp/claude-2.1.226
(VXu,{WORKFLOW_TOOL_NAME:()=>oR,CODE_REVIEW_WORKFLOW_NAME:()=>F_r});var oR="Workflow",F_r="code-review";var KXu="SearchPlugins",YXu="SearchSkil
o=E(()=>{Mt();CVs=require("crypto"),wBd=require("os");sU_=new Set(["verify","pr","commit","code-review","simplify","go"]),aU_=/[/\\]\.claude[/\
"artifact-capabilities",qzo="workshop",xGd="whiteboard",IGd="prototype",Vzo="dataviz",Wde="code-review",HGd="code-walkthrough",RGd="pr-explaine

The first grep's f2(e) is the tell that the scan builds its regex from a runtime argument rather than a constant, and the call site confirms e is the invoked skill's name:

function $8b(e,t){if(t.agentId!==void 0)return!1;let r=new RegExp(`(?<!\\S)/${f2(e)}(?=$|\\s)`);
...
let l=xIn(a,{commandName:o,userTypedThisTurn:$8b(o,t),...});if(l!==null){switch(l.reason){case"disable_model_invocation":{let{sanitizedName:c,skillNameHash:u}=RPe({rawName:o,canonicalName:a.name,

So the half that has to hold is the third hit — the bundled-skill name list, between dataviz and code-walkthrough. The first hit, CODE_REVIEW_WORKFLOW_NAME, names the built-in code-review workflow and is a separate constant; anchoring the check on it would report "still fine" through a skill-side rename. The identifiers (f2, Wde) are minified and will differ between releases, which is why the recipe prints context and asks the reader to identify the hit rather than matching a binding exactly.

@tend-agent tend-agent left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I ran the recipe end-to-end against 2.1.226 (download, both greps). The download URL and the scan-regex grep reproduce exactly as described. The second grep is anchored on the wrong constant.

CODE_REVIEW_WORKFLOW_NAME is exported from the same namespace object as WORKFLOW_TOOL_NAME ("Workflow") — it names the built-in code-review workflow, not the bundled skill whose name the waiver scan interpolates. The name the Skill tool actually matches on comes from a separate var declaration in the bundled-skill name list, sitting between dataviz and code-walkthrough. Both read code-review today, so the grep passes either way — but a skill-side rename doesn't have to move the Workflow constant, and the check would then report "still fine" on precisely the failure the recipe exists to catch.

Printing every binding of the literal with context is both simpler and robust to minifier churn: it surfaces all three (Workflow constant, telemetry set, bundled-skill list) and leaves the reader to identify which is which, which is what the paragraph below the block already tells them to do.

What the scan is fed, in 2.1.226

The scan builds its regex from the invoked skill's name, not from a constant:

function $8b(e,t){if(t.agentId!==void 0)return!1;let r=new RegExp(`(?<!\\S)/${f2(e)}(?=$|\\s)`);

and its call site passes the runtime name alongside it:

xIn(a,{commandName:o,userTypedThisTurn:$8b(...

The three bindings of the literal, from the suggested grep:

(VXu,{WORKFLOW_TOOL_NAME:()=>oR,CODE_REVIEW_WORKFLOW_NAME:()=>F_r});var oR="Workflow",F_r="code-review";var KXu="SearchPlugins",YXu="SearchSkil
o=E(()=>{Mt();CVs=require("crypto"),wBd=require("os");sU_=new Set(["verify","pr","commit","code-review","simplify","go"]),aU_=/[/\\]\.claude[/\
"artifact-capabilities",qzo="workshop",xGd="whiteboard",IGd="prototype",Vzo="dataviz",Wde="code-review",HGd="code-walkthrough",RGd="pr-explaine

Only the third is the bundled-skill name list.

Comment thread .claude/skills/running-tend/SKILL.md Outdated
max-sixty pushed a commit that referenced this pull request Aug 9, 2026
Weekly pin refresh: `claude_version` 2.1.220 → 2.1.226 (npm
`dist-tags.latest`). A stale binary resolves `--model opus`/`sonnet` to
a superseded alias target, so the pin is worth moving even on a run of
quiet releases.

CHANGELOG skim across 2.1.221 → 2.1.226, restricted to the paths this
action exercises (headless `-p`, the credential-injection proxy, plugin
skills, OAuth-token auth):

- **2.1.225 — headless OAuth token.** "Fixed a transient 401 replacing a
long-lived `CLAUDE_CODE_OAUTH_TOKEN` with a stored login's short-lived
token, breaking headless sessions until restart." This is exactly tend's
auth shape; the failure mode was a session that dies partway and can't
recover without a restart the action never does.
- **2.1.222 — HTTPS proxy.** "Fixed the startup connectivity check
hanging and then failing behind an HTTPS proxy; it now uses the same
proxy-aware transport as API requests and times out with a clear
message." Every tend session starts behind `mitmdump`, so this removes a
hang class at the point where the proxy has just come up.
- **2.1.221 — sandbox proxy + non-interactive skills.** "Fixed sandboxed
large uploads failing with TLS errors through the sandbox proxy", and
"Fixed plugin- and org-delivered skills named after terminal-only
built-ins (e.g. `/help`, `/feedback`) being un-invocable in
non-interactive sessions" — tend ships its skills as a plugin and
invokes them from a non-interactive run.
- **2.1.223 — `/review` is now an alias of `/code-review`.** No action
needed, but the reason isn't which skill the review path calls — it's
that `code_review_notice` in `generator/src/tend/config.py` puts a bare
`/code-review` token into *every* generated Claude prompt, to trip the
`Skill` tool's `disable-model-invocation` waiver. The scan reading that
token lives in the binary, so a release touching `/code-review` handling
is precisely the change class that could unhook it with nothing failing
— the run would just lose its second pass. Verified against the 2.1.226
binary rather than the CHANGELOG: the scan still builds its regex from
the command name (`(?<!\S)/${name}(?=$|\s)`), and the canonical name is
still `code-review` — 2.1.223 added `/review` as an alias rather than
renaming it. #926 folds this check into the weekly bump rule so the next
bump doesn't have to re-derive it.
- **2.1.226** is bug fixes and reliability only; **2.1.224** is feature
work on surfaces this action doesn't touch (self-hosted runners, Remote
Control, VS Code).

Nothing in the range changes `-p` result events, `--model` alias
resolution, first-run onboarding, or Stop-hook behavior.

`mitmproxy_version` is already at latest (12.2.3), so `uv_version` stays
put too — per `running-tend`, uv moves with mitmproxy rather than on a
stream of its own.

The pin resolves on the channel the install actually uses, not just npm:
`2.1.226/manifest.json` exists in the release bucket and the downloaded
`linux-x64` binary's sha256 matches its manifest entry. Note that it now
sits ahead of that bucket's own `stable` marker, which currently reads
2.1.220 — the version this PR replaces. That's the rule as written
("track latest"), not a defect here; #927 raises whether `latest` is the
marker we want to keep tracking.

Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
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.

1 participant