ci: point storyboard runner at @adcp/sdk latest instead of adcp-3.1#961
Conversation
There was a problem hiding this comment.
Correct dist-tag migration — unblocks every storyboard runner. Right shape: one pinned adcp-3.0 leg as the fixed backwards-compat floor, one moving latest leg that tells you when the current line breaks you.
Root cause is traced to the exact upstream commit (509ed55b fix: publish stable releases to latest), not papered over. latest is the AdCP 3.1 GA line, so coverage is equivalent — no leg dropped, no continue-on-error, fail-fast: false retained.
Things I checked
- Every
adcp-3.1reference is gone: env default (ci.yml:12), the matrix (ci.yml:372), and both comment blocks. Grep confirms zero remaining after the diff applies. ADCP_SDK_VERSION: "latest"flows into the non-matrix installs atci.yml:519,736,844(npm install -g @adcp/sdk@${ADCP_SDK_VERSION}) — consistent with the matrix change, no orphaned tag.- Artifact paths and
STORYBOARD_RESULT_PATH(ci.yml:404,411,412) are templated on${{ matrix.adcp-sdk-tag }}→storyboard-result-latest.json; no hard-coded tag. The requiredstoryboard-required-gatechecksneeds.storyboard.result, not tag names, so it adapts automatically. - No CI gate regression — the blocking gate is untouched, both legs survive.
- The schema-drift skip at
ci.yml:280reads the committedsrc/adcp/ADCP_VERSION, notADCP_SDK_VERSION— independent inputs, so this PR does not touch drift-validation behavior. (code-reviewer: sound.dx-expert: ship it.) - CI-only change, no
adcp.*public surface touched, no changeset/version bump expected.
Follow-ups (non-blocking — file as issues)
latestleg's npm cache key is frozen at...-latestwhile the version it resolves drifts. The cache key salts on the tag string (ci.yml:395), so a warm~/.npmhit can serve a stale tarball — the moving target's determinism degrades from "always newest" to "newest, or whatever was cached." Make thelatestleg cache version-aware: resolve the concrete version first (npm view @adcp/sdk@latest version) and fold it into the key, or drop~/.npmcaching for that leg so it always cold-resolves. The pinnedadcp-3.0leg's caching is fine as-is — its resolved version never moves.
The stated tradeoff (a future SDK release could change storyboard behavior without a tag bump here) is correctly characterized and acceptable — for a current-line interop gate, "we want to know early" is the right default.
LGTM. Follow-up noted above.
The @adcp/sdk publish process changed to publish stable releases under the `latest` dist-tag and no longer emits an `adcp-3.1` tag (AdCP 3.1 is the current stable line, shipped as `latest`). The removed `adcp-3.1` tag made every storyboard runner fail with `npm notarget @adcp/sdk@adcp-3.1`. Track `latest` only — the current AdCP 3.1 stable line. The legacy adcp-3.0 compatibility leg is dropped; we no longer ship against the AdCP 3.0 line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ee56619 to
00e1816
Compare
There was a problem hiding this comment.
The dist-tag repoint is correct and unblocks CI — adcp-3.1 is genuinely gone from the registry, so any matrix leg referencing it is dead, and latest always resolves. But the diff and the PR body disagree about the adcp-3.0 leg, and that disagreement is load-bearing for whether this is the right fix. Holding for one answer before approving.
The contradiction
- Diff (
ci.yml:372): matrix goes["adcp-3.0", "adcp-3.1"]→["latest"]— single leg,adcp-3.0dropped. - Commit message: "The legacy adcp-3.0 compatibility leg is dropped." Matches the diff.
- PR body: "Kept the sticky
adcp-3.0tag for backwards-compatibility coverage," and the Tradeoff section leans on "the stickyadcp-3.0leg still provides a fixed backwards-compat gate" as the reason movinglatestis acceptable.
The body's entire risk argument — floating latest is fine because the fixed adcp-3.0 anchor remains — is falsified by its own diff, which deletes that anchor. A reviewer approving on the body's wording approves coverage the diff removes.
Things I checked
storyboard-required-gate(ci.yml:415-427) keys offneeds.storyboard.result, which is matrix-size-agnostic — a single-element matrix still reportssuccess/failurecorrectly. No gate breakage.- Cache key
...-npm-adcp-sdk-${{ matrix.adcp-sdk-tag }}(ci.yml:395) and artifact namestoryboard-result-${{ matrix.adcp-sdk-tag }}(ci.yml:411-412) collapse cleanly to...-latest— no collision. - No dangling
adcp-3.0/adcp-3.1literals remain inci.ymloutside the two edited blocks (env9-12, matrix/comment360-372). - The other storyboard jobs (
501,519,728,736,836,844) already key off theADCP_SDK_VERSIONdefault — so flipping that default tolatestmeans every storyboard job now floats onlatestwith no pinned anchor anywhere.
What flips this to approve
Pick one:
- Restore the anchor: matrix
["adcp-3.0", "latest"]. Minimal blast radius — unblocks on the live tag, keeps a fixed, reproducible backwards-compat gate. Nothing in the incident (a removedadcp-3.1tag) required droppingadcp-3.0; it was collateral from a single-line matrix edit. This is what the PR body already says you intended. - Own the drop: if stopping AdCP-3.0 coverage is a deliberate product-scope call (as the commit asserts: "we no longer ship against the AdCP 3.0 line"), fix the PR body to stop claiming the
adcp-3.0anchor exists, and acknowledge that a required gate now floats entirely onlatest.
That second state is the one to think hard about: a required check with no reproducible anchor means a future @adcp/sdk publish to latest can red-bar every open PR here with zero changes in this repo, and non-deterministically — same commit green Monday, red Wednesday. "We want to know" is right for a signal, wrong for a blocking gate. If you do want the live-tracking signal, a continue-on-error or nightly scheduled leg gets it without making unrelated PRs hostage to an upstream release.
Minor nits (non-blocking)
- Cache key tracks the literal
latest, not a resolved version.ci.yml:395(and501/728/836) key the npm cache on the stringlatest, so after upstream republisheslatestthe runner serves a stale@adcp/sdkuntil eviction — which quietly undercuts the "we want to know immediately" rationale. If fresh-on-publish is the goal, salt the key with a resolved version or date.
Not blocking — this is CI plumbing, no source, schema, or public-API surface touched. One reconciled answer to the question above and I approve. The commit message had it right; the PR body is the one that needs to catch up.
The previous commit collapsed the storyboard matrix to a single ``latest`` leg, which also dropped the ``adcp-3.0`` backwards-compat leg as collateral. ``adcp-3.0`` still resolves on npm (7.11.7) and is the only gate verifying examples/seller_agent.py still interoperates with the AdCP 3.0 runner line. Restore the matrix to ["adcp-3.0", "latest"]: a fixed, reproducible backwards-compat floor plus the moving current-stable line. This makes the diff match the PR body, which always claimed the adcp-3.0 anchor was retained. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Correct minimal fix for a real outage — the adcp-3.1 dist-tag was removed upstream (traced to 509ed55b), so every storyboard job was failing notarget @adcp/sdk@adcp-3.1, not any PR's code. Keeping the sticky adcp-3.0 leg as a fixed backwards-compat floor while latest tracks current stable is the right shape for a matrix gate.
Things I checked
ADCP_SDK_VERSIONdefault flip atci.yml:12(adcp-3.1tolatest) is workflow-globalenv. It feeds three non-matrix install steps (ci.yml:519,:736,:844) plus the matrix leg (:403), all doing the samenpm install -g @adcp/sdk@<tag>. Every consumer was hitting the same removed-tag failure; flipping tolatestfixes them identically. No job branches on the literal stringadcp-3.1.- Matrix change at
ci.yml:372([adcp-3.0, adcp-3.1]to[adcp-3.0, latest]). The matrix value flows into the cache key (:395), env (:403), and artifact paths (:404,:411-412) purely as a string token —latestis mechanically valid in every position. - No dangling references: repo-wide grep finds
adcp-3.1only in the three sites this diff touches.scripts/ci/run_storyboard_reference_seller.shconsumes the version as an opaque install target, so it needs no change. - Comments at
ci.yml:7-8and:364-368updated to match — no staleadcp-3.1prose left claiming it tracks the current 3.1 runner. - Two commits: the first collapsed the matrix to a single
latestleg (droppingadcp-3.0as collateral); the second restoredadcp-3.0. Final diff matches the PR body's claim that the backwards-compat anchor was retained. Worth squashing on merge so the intermediate single-leg state does not land — non-blocking. - Conventional-commit prefix
ci:is correct — no shipped-package surface, no semver signal needed.
Follow-ups (non-blocking — file as issues)
- The npm cache key now drifts under a fixed string. The comment at
ci.yml:496promises the version salts the key so a bump invalidates deterministically. With a fixed tag that held; withlatestthe key suffix is...-adcp-sdk-latestforever while the concrete version behind it floats (9.3.0, then 9.4.0, ...). The npm cache can serve a stale tarball for a tag whose target moved, and the comment now overstates determinism. Either correct that comment or fold a resolvednpm view @adcp/sdk versioninto the cache key. (dx-expertflagged this.) latestin a blocking gate converts an upstream publish into a repo-wide red with no commit here. The PR frames we-want-to-know as desirable — agreed, but a required gate is the wrong place to learn it: a bisect on this repo will finger an innocent PR. The standard resolution is pin a concrete version for the blocking leg plus a separatecontinue-on-errorlatestcanary for early warning. Acceptable to defer; theadcp-3.0floor keeps one reproducible gate in the meantime.
Minor nits (non-blocking)
- Surface the floating-tag footgun in-file, not just the PR body. The tradeoff section in the description is clear, but the next maintainer debugging a red gate reads the comment at
ci.yml:9, not the PR. One sentence there — latest floats, a storyboard failure with no PR change usually means an upstream@adcp/sdkpublish — saves the 2am bisect.
LGTM. Follow-ups noted below.
bokelley
left a comment
There was a problem hiding this comment.
Reviewed the workflow diff and verified current npm dist-tags locally: @adcp/sdk publishes adcp-3.0 and latest, with no adcp-3.1 tag. CI is green.
Why
Every `AdCP storyboard runner` job is failing on `npm error notarget No matching version found for @adcp/sdk@adcp-3.1`. This is not a defect in any PR — the `adcp-3.1` npm dist-tag was removed from the registry.
Root cause (traced in `adcontextprotocol/adcp-client`): commit `509ed55b fix: publish stable releases to latest` (2026-06-26) changed the SDK release process. It previously auto-published every stable release under a protocol-derived tag (`adcp-3.1`); it now publishes stable under `latest` and no longer emits `adcp-3.1`. AdCP 3.1 is the current stable line, shipped as `@adcp/sdk@latest` (currently 9.3.0). The SDK's `NPM-DIST-TAGS.md` now states an `adcp-3.X` compat tag exists only once that line becomes a maintenance line behind a newer stable — so there is intentionally no `adcp-3.1` tag today.
This passed on main as recently as June 20 (when the tag still existed) and started failing for all PRs after June 26. It blocks every PR because the storyboard jobs are required checks.
What changed
Tradeoff
`latest` is a moving target, so a future `@adcp/sdk` release could change storyboard behavior without a tag bump here. That is acceptable (and arguably desirable — we want to know): the sticky `adcp-3.0` leg still provides a fixed backwards-compat gate. If the org later wants a stable `adcp-3.1` compat tag, that is a separate change in the SDK repo's publish process.