Skip to content

ci: add CI Check 2 — capability matrix drift warning#60

Merged
grdsdev merged 6 commits into
mainfrom
claude/sdk-993-ci-drift-warning-7fc4b5
Jul 15, 2026
Merged

ci: add CI Check 2 — capability matrix drift warning#60
grdsdev merged 6 commits into
mainfrom
claude/sdk-993-ci-drift-warning-7fc4b5

Conversation

@grdsdev

@grdsdev grdsdev commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements SDK-993: a new non-blocking CI check that warns when a capability marked implemented in an SDK's sdk-compliance.yaml has a registered symbol the parser can no longer find, or has no registered symbols at all to verify against.

This is deliberately separate from and additive to Check 1 (api-check.ts/check-api-symbols.ts), which only catches removal within the diff of the current PR and therefore can't see drift that accumulated silently across past PRs — the actual problem this check targets. Check 1's existing blocking behavior is left untouched.

  • src/drift-check.ts — pure detection logic (checkDrift, formatDriftSummary)
  • src/compliance-source-map.ts — maps YAML symbol/feature lines for annotations, using the existing yaml dependency
  • src/check-drift.ts — CLI wrapper, never blocks the build (no process.exit)
  • .github/actions/sdk-compliance-check-drift/action.yml — runs the CLI, then upserts/deletes a sticky PR comment via actions/github-script (comment created only when drift exists, deleted once resolved)
  • Wired into all 4 reusable workflows (javascript/python/swift/dart)
  • docs/capability-matrix.md — new doc, since the ticket's mandated warning message links to it

Findings from the whole-branch review were fixed in a follow-up commit:

  • github-script comment step now swallows errors (fork PRs get a read-only token — write calls would 403 and fail the job, defeating the non-blocking premise)
  • restored contents: read alongside the new pull-requests: write permission (job-level permissions: zeroes unlisted scopes)
  • sanitized annotation text against workflow-command injection

Design spec and implementation plan: docs/superpowers/specs/2026-07-10-ci-check-2-drift-warning-design.md, docs/superpowers/plans/2026-07-10-ci-check-2-drift-warning.md (local only — docs/superpowers/ is gitignored in this repo).

Test plan

  • npm test — 174/174 passing
  • npm run typecheck — clean
  • npm run validate — OK
  • Manual CLI fixture verification (annotation format, summary create/delete lifecycle)
  • YAML syntax validated for the composite action and all 4 workflow files
  • Independent whole-branch review (Opus) + fix pass + re-review, approved ready to merge

End-to-end test against a real SDK repo

Composite-action refs in reusable workflows can't be parameterized with ${{ inputs.* }} (a GitHub Actions restriction — same limitation already applies to Check 1's composite actions), so sdk-compliance-check-drift@main can't be exercised pre-merge without pointing it at a real ref. Used a disposable branch (supabase/sdk@claude/sdk-993-e2e-test, since deleted) to test the full path against supabase-swift#1113:

  • Ran against supabase-swift's real sdk-compliance.yaml and real Swift symbol graph — found genuine drift: 9 registered-but-missing symbols (auth.mfa.enroll, auth.mfa.verify, auth.passkey.*, realtime.subscriptions.broadcast) and 8 implemented entries with no symbols registered at all.
  • ::warning:: annotations printed correctly, pointing at the right compliance file.
  • Sticky PR comment posted correctly on the target PR, matching the annotation content.
  • The job stayed green (non-blocking) — confirms the fork-PR fix (github-script try/catch) actually works, not just in theory.

Test branch reverted to its original state afterward; no residue left in either repo.

🤖 Generated with Claude Code

grdsdev added 6 commits July 10, 2026 10:57
…warning

- Wrap the drift-comment github-script body in try/catch so a read-only
  GITHUB_TOKEN on fork PRs (403 on write calls) can't fail the job; the
  ::warning:: annotations already carry the signal, the sticky comment is
  best-effort.
- Restore contents: read alongside pull-requests: write on the check job's
  permissions block in all 4 validate-sdk-compliance-*.yml workflows, since
  any job-level permissions key zeroes out unlisted scopes and checkout
  needs contents: read.
- Add docs/capability-matrix.md so the drift summary's doc link resolves.
- Sanitize newlines out of featureId/symbol before building ::warning::
  annotation messages to prevent workflow-command injection via YAML.
@grdsdev
grdsdev marked this pull request as ready for review July 10, 2026 15:01
@grdsdev
grdsdev requested review from a team as code owners July 10, 2026 15:01

@QuintinWillison QuintinWillison left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general I would aim to avoid anything in CI that doesn't fail hard, forcing immediate correction. I might be missing something in terms of wider context, but I'm not sure it's clear who's going to listen to the event "CI posts a warning". My fear is that this'll be a source of "warnings noise" that becomes unfixed annoyance rather than forced to be fixed due to it blocking CI green state.

@grdsdev

grdsdev commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

In general I would aim to avoid anything in CI that doesn't fail hard, forcing immediate correction. I might be missing something in terms of wider context, but I'm not sure it's clear who's going to listen to the event "CI posts a warning". My fear is that this'll be a source of "warnings noise" that becomes unfixed annoyance rather than forced to be fixed due to it blocking CI green state.

We can lift to an error once all SDKs are in a good shape. Wdyt?

@grdsdev
grdsdev requested a review from QuintinWillison July 10, 2026 17:09
@grdsdev
grdsdev merged commit 45dba43 into main Jul 15, 2026
3 checks passed
@grdsdev
grdsdev deleted the claude/sdk-993-ci-drift-warning-7fc4b5 branch July 15, 2026 13:34
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