Skip to content

ci: point storyboard runner at @adcp/sdk latest instead of adcp-3.1#961

Merged
bokelley merged 2 commits into
mainfrom
fix-ci-storyboard-sdk-latest-tag
Jun 28, 2026
Merged

ci: point storyboard runner at @adcp/sdk latest instead of adcp-3.1#961
bokelley merged 2 commits into
mainfrom
fix-ci-storyboard-sdk-latest-tag

Conversation

@andybevan-scope3

Copy link
Copy Markdown
Collaborator

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

  • `ADCP_SDK_VERSION` default and the seller_agent.py storyboard matrix leg now track `latest` (which is the AdCP 3.1 GA line) instead of the removed `adcp-3.1` tag.
  • Kept the sticky `adcp-3.0` tag for backwards-compatibility coverage.
  • Updated the stale comments that described `adcp-3.1` as the current-line tag.

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.

aao-ipr-bot[bot]
aao-ipr-bot Bot previously approved these changes Jun 27, 2026

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.1 reference 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 at ci.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 required storyboard-required-gate checks needs.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:280 reads the committed src/adcp/ADCP_VERSION, not ADCP_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)

  • latest leg's npm cache key is frozen at ...-latest while the version it resolves drifts. The cache key salts on the tag string (ci.yml:395), so a warm ~/.npm hit can serve a stale tarball — the moving target's determinism degrades from "always newest" to "newest, or whatever was cached." Make the latest leg cache version-aware: resolve the concrete version first (npm view @adcp/sdk@latest version) and fold it into the key, or drop ~/.npm caching for that leg so it always cold-resolves. The pinned adcp-3.0 leg'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>

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.0 dropped.
  • Commit message: "The legacy adcp-3.0 compatibility leg is dropped." Matches the diff.
  • PR body: "Kept the sticky adcp-3.0 tag for backwards-compatibility coverage," and the Tradeoff section leans on "the sticky adcp-3.0 leg still provides a fixed backwards-compat gate" as the reason moving latest is 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 off needs.storyboard.result, which is matrix-size-agnostic — a single-element matrix still reports success/failure correctly. No gate breakage.
  • Cache key ...-npm-adcp-sdk-${{ matrix.adcp-sdk-tag }} (ci.yml:395) and artifact name storyboard-result-${{ matrix.adcp-sdk-tag }} (ci.yml:411-412) collapse cleanly to ...-latest — no collision.
  • No dangling adcp-3.0 / adcp-3.1 literals remain in ci.yml outside the two edited blocks (env 9-12, matrix/comment 360-372).
  • The other storyboard jobs (501, 519, 728, 736, 836, 844) already key off the ADCP_SDK_VERSION default — so flipping that default to latest means every storyboard job now floats on latest with no pinned anchor anywhere.

What flips this to approve

Pick one:

  1. 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 removed adcp-3.1 tag) required dropping adcp-3.0; it was collateral from a single-line matrix edit. This is what the PR body already says you intended.
  2. 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.0 anchor exists, and acknowledge that a required gate now floats entirely on latest.

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)

  1. Cache key tracks the literal latest, not a resolved version. ci.yml:395 (and 501/728/836) key the npm cache on the string latest, so after upstream republishes latest the runner serves a stale @adcp/sdk until 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>

@aao-ipr-bot aao-ipr-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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_VERSION default flip at ci.yml:12 (adcp-3.1 to latest) is workflow-global env. It feeds three non-matrix install steps (ci.yml:519, :736, :844) plus the matrix leg (:403), all doing the same npm install -g @adcp/sdk@<tag>. Every consumer was hitting the same removed-tag failure; flipping to latest fixes them identically. No job branches on the literal string adcp-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 — latest is mechanically valid in every position.
  • No dangling references: repo-wide grep finds adcp-3.1 only in the three sites this diff touches. scripts/ci/run_storyboard_reference_seller.sh consumes the version as an opaque install target, so it needs no change.
  • Comments at ci.yml:7-8 and :364-368 updated to match — no stale adcp-3.1 prose left claiming it tracks the current 3.1 runner.
  • Two commits: the first collapsed the matrix to a single latest leg (dropping adcp-3.0 as collateral); the second restored adcp-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:496 promises the version salts the key so a bump invalidates deterministically. With a fixed tag that held; with latest the key suffix is ...-adcp-sdk-latest forever 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 resolved npm view @adcp/sdk version into the cache key. (dx-expert flagged this.)
  • latest in 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 separate continue-on-error latest canary for early warning. Acceptable to defer; the adcp-3.0 floor keeps one reproducible gate in the meantime.

Minor nits (non-blocking)

  1. 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/sdk publish — saves the 2am bisect.

LGTM. Follow-ups noted below.

@bokelley bokelley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@bokelley bokelley merged commit 816320b into main Jun 28, 2026
26 checks passed
@bokelley bokelley deleted the fix-ci-storyboard-sdk-latest-tag branch June 28, 2026 04:38
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.

2 participants