Skip to content

fix(ci): run the SDK examples gate 2-wide, not 4-wide - #13

Merged
feng-shiplight merged 1 commit into
mainfrom
feng/workspace
Sep 29, 2026
Merged

feng-shiplight merged 1 commit into
mainfrom
feng/workspace

Conversation

@feng-shiplight

Copy link
Copy Markdown
Contributor

Summary

The E2E — public SDK examples hard gate failed 2 of 3 runs on ubuntu-latest, always on the same example and always the same way:

[basic] page.screenshot: Protocol error (Page.captureScreenshot):
        Unable to capture screenshot
  - waiting for fonts to load...
  - fonts loaded

The page had loaded and Chromium then failed to capture it, so this is a capture-time resource failure rather than a test failure. 7 of 8 examples passed in every run.

Run basic Others
dry run 36497568207 passed 7/7
publish 36500233648 failed 7/7
publish 36502792430 failed 7/7

Code was effectively identical across all three — the only main movement between them was the MCP version bump, which does not touch the SDK.

Why it regressed

The width was 4 on the 8-vCPU self-hosted runner. I kept it at 4 when moving to ubuntu-latest in #12, reasoning that these examples block on live-AI calls rather than CPU. That reasoning was incomplete: four concurrent headed Chromium instances also contend for memory and display surfaces, neither of which is a core count. The review on #12 flagged the width as unverified on this runner class, and the single dry run that passed gave false confidence.

Neither publish attempt shipped anything — the gate runs before npm publish, so @shiplightai/sdk stayed at 0.1.11 and main stayed clean.

What this does not establish

Halving the width trades wall time on that step for a gate that holds. It does not identify the exact constraint: there was no OOM line in any log, so memory, /dev/shm and display surfaces all remain candidates. If 2-wide proves stable, contention is the explanation; if it does not, the explanation is wrong and worth reopening rather than halving again.

Test plan

  • scripts/__tests__/publish-sdk-workflow.test.ts pinned xargs -P 4 and carried the disproven "blocks on live-AI calls, not on cores" claim in a comment. Both corrected.
  • The guard fails against the 4-wide workflow (verified by stashing publish-sdk.yml: 4 pass, 1 fail) and passes at 2-wide.
  • Full scripts guard suite: 144/144.
  • Real verification is the next publish run, which is the only thing that exercises this gate.

🤖 Generated with Claude Code

The gate failed 2 of 3 runs on ubuntu-latest, always on the same example
and always the same way:

  [basic] page.screenshot: Protocol error (Page.captureScreenshot):
          Unable to capture screenshot
    - waiting for fonts to load...
    - fonts loaded

The page had loaded and Chromium then failed to capture it, so this is a
capture-time resource failure rather than a test failure. 7 of 8 examples
passed every time.

The width was 4 on the 8-vCPU self-hosted runner, and I kept it at 4 when
moving to ubuntu-latest, reasoning that these block on live-AI calls rather
than CPU. That reasoning was incomplete: four concurrent headed Chromium
instances also contend for memory and display surfaces, neither of which is
a core count. The review on #12 flagged the width as unverified on this
runner class, and the one dry run that passed gave false confidence.

Halving the width trades wall time on that step for a gate that holds. What
this does not do is identify the exact constraint — there was no OOM in the
logs, so memory, /dev/shm and display surfaces are all still candidates. If
2-wide proves stable, the diagnosis is contention; if it does not, the
explanation is wrong and worth reopening.

The guard in publish-sdk-workflow.test.ts pinned -P 4 and carried the
disproven claim in a comment; both are corrected, and it fails against the
4-wide workflow.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review

Clean, minimal fix. Two files changed, both correctly updated.

What changes

  • .github/workflows/publish-sdk.yml: xargs -P 4 to xargs -P 2 in the SDK examples gate, with an updated comment explaining the Page.captureScreenshot failure mode.
  • scripts/__tests__/publish-sdk-workflow.test.ts: guard regex updated from -P 4 to -P 2; stale "blocks on live-AI calls, not on cores" comment removed.

Findings

No CRITICAL / HIGH / MEDIUM issues.

LOW — none actionable: The remaining comment in the test (github-actions-runner-labels.test.ts holds this for every workflow) still reads cleanly after the stale sentence was removed; no correction needed.

Checklist

  • No literal ::error::/::warning::/::notice:: annotation commands introduced.
  • No untrusted github.event.* interpolation in run: blocks.
  • No secret handling or permission changes.
  • Guard test correctly pins the new value and the PR description confirms it fails against the 4-wide workflow and passes at 2-wide.
  • No application code touched; no i18n or publishing surface affected.

The tradeoff (slower wall time for gate stability) is correctly acknowledged in the PR description. Approved.

@feng-shiplight
feng-shiplight merged commit 7792371 into main Sep 29, 2026
6 checks passed
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