Skip to content

ci: gate paybot-sdk on npm test (closes #4)#35

Merged
RBKunnela merged 1 commit into
mainfrom
ci/add-test-gate
May 22, 2026
Merged

ci: gate paybot-sdk on npm test (closes #4)#35
RBKunnela merged 1 commit into
mainfrom
ci/add-test-gate

Conversation

@RBKunnela
Copy link
Copy Markdown
Owner

@RBKunnela RBKunnela commented May 22, 2026

Summary

Gap: paybot-sdk auto-publishes to npm on every push to main, but CI runs only npm cinpm run type-checknpm run build. The test suite never executes in CI, so code can ship to consumers without its own tests ever running.

Fix: Wire npm test (vitest) into the existing build matrix as a new step between type-check and build, across the Node 18 + Node 20 matrix entries. Three-line additive change in .github/workflows/ci.yml.

- name: Run tests
  run: npm test

Tasks closed

Discovered during Phase 1, split into separate tasks

Phase 1 discovery revealed that wiring npm run lint in the same PR is not viable today: lint surfaces a real pre-existing semantic bug in src/x402-v2.ts:251 (no-dupe-else-if). The fix is not a chore — it requires deciding what dual-mode in signPaymentV2 should actually emit. Routing this through @devops as part of a CI-wiring PR violates scope discipline. Coverage gate is a third, separable concern. Filed as:

Local verification

$ npm test
> paybot-sdk@0.3.0 test
> vitest run

 ✓ tests/crypto.test.ts (3 tests)
 ✓ tests/errors.test.ts (7 tests)
 ✓ tests/networks.test.ts (11 tests)
 ✓ tests/middleware.test.ts (15 tests)
 ✓ tests/x402-handler.test.ts (10 tests)
 ✓ tests/auth.test.ts (13 tests)
 ✓ tests/client.test.ts (43 tests)

 Test Files  7 passed (7)
      Tests  102 passed (102)
   Duration  1.72s

vitest.config.ts is scoped to include: ['tests/**/*.test.ts'] — no risk of picking up stray *.test.ts files under node_modules/. Coverage thresholds (80/80/70/80) are configured but fire only under --coverage, not under npm test, so this PR does NOT introduce a coverage gate. Coverage gating is Task #15.

Required status checks impact

None. The new step runs INSIDE the existing build (18) + build (20) matrix entries, so context names stay identical:

  • Existing: build (18), build (20), Analyze (javascript-typescript), scan / osv-scan
  • Post-merge: same four contexts, but build (18) + build (20) now have substance (test execution) beyond compile-only validation.

required_status_checks.contexts in branch protection requires no change. The check names emitted by GitHub Actions are matrix-derived and do not include step names.

Chain governance

5th application of automated-pr-merge-authority.md. Pre-approval pattern in effect: this PR was routed by @aiox-master (Orion) after a Phase 1 discovery escalation where Branch B was hit (tests pass, lint fails on bug). Orion's Option 1 ruling: ship test-only now, lint deferred to Task #14.

Chain composition for this PR (chore CI carve-out, paybot-core PR #3 precedent):

  • @sm, @po, @dev — SKIP (no story, no AC, operator-routed Phase 1 by Orion)
  • @aiox-master (Orion) — ROUTED (this comment is the routing artefact; full Phase 1 discovery report lives in .aios/handoffs/ and aios-devops memory)
  • @qa (Quinn) — pending PASS on adapted 12-check matrix
  • @devops (Gage) — pending merge after @qa PASS

Per the pre-approval pattern, @qa PASS will route directly to @devops for merge without re-confirming with operator.

Anti-patterns avoided

N/A — clean additive 3-line change. No new actions introduced, no new permissions required, no new contexts emitted, no SHA-pinning needed (no new uses: directives). No coverage gate snuck in. No bundled lint wire-up that would have failed on the unrelated pre-existing bug.

Phase 1 escalation transparency

The lint failure could have been silently ignored or masked with continue-on-error: true. Both are anti-patterns: the first hides the gap, the second adds noise to CI without enforcement. Escalating to Orion with options and accepting the split into three tasks is the disciplined path. Audit trail of that escalation is preserved in aios-devops memory at project_paybot_sdk_task4_phase1_20260522.md in AIOX-Enterprise.

Summary by CodeRabbit

  • Tests
    • Automated testing is now integrated into the continuous integration workflow, validating code changes before each build to ensure quality and stability in releases.

Review Change Stack

Wire vitest run into the build matrix between type-check and build,
across Node 18 + 20. paybot-sdk auto-publishes to npm on every push
to main; before this change CI never ran the test suite, so code
could ship to customers without its own tests ever executing.

Test suite: 102 tests / 7 files in tests/, vitest.config.ts scoped
to tests/**/*.test.ts. Local: 102/102 pass in 1.72s.

Not in this PR (deliberately split per Orion routing during Phase 1
discovery):
- Task #14: dual-mode dead-code bug in src/x402-v2.ts:251
  (no-dupe-else-if surfaces it; requires semantic decision on what
  dual-mode should emit — separate @dev story)
- Task #15: coverage gate at 80% threshold + tests for x402-v2.ts
  and payment-engine.ts (currently 0% covered, 805 LOC combined)
- Task #16: coverage/ gitignore hygiene

No change to required_status_checks contexts: the new step runs
inside the existing build (18) + build (20) matrix entries, adding
substance without adding new context names.

5th application of automated-pr-merge-authority.md.
@gemini-code-assist
Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 640a6b00-05ca-4f57-b3ba-0f1a84b2c939

📥 Commits

Reviewing files that changed from the base of the PR and between e129fe4 and 4aab22f.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

Walkthrough

The CI workflow adds a test execution step to the build job. The npm test command now runs after type checking and before the build step, ensuring test failures block compilation without requiring manual test invocation.

Changes

CI Test Execution

Layer / File(s) Summary
Add test step to CI build
.github/workflows/ci.yml
New "Run tests" step executes npm test between "Type check" and "Build" steps in the CI workflow.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately reflects the main change: adding an npm test gate to the CI workflow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/add-test-gate

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@RBKunnela
Copy link
Copy Markdown
Owner Author

Merge authorization per .claude/rules/automated-pr-merge-authority.md (5th application, paybot-sdk track):

  • @sm/@po/@dev: SKIP (chore-style CI infra, no story/AC/app code)
  • @qa: PASS — 12-check adapted matrix, 2 passes
    • 4/4 required CI green
    • 102/102 tests pass in build (18) + build (20) (zero flake risk)
    • No new required_status_checks context (matrix-derived names unchanged)
  • @aiox-master (Orion): Chain routing approved
  • @devops: Executing merge under operator pre-approval pattern (feedback_qa_pass_auto_merge.md)

Closes Task #4. Out-of-scope discoveries split:

Precedent chain (today, 2026-05-22):

This is the 7th merged PR in the paybot ecosystem in a single day.

@RBKunnela RBKunnela merged commit b4de4e7 into main May 22, 2026
8 checks passed
@RBKunnela RBKunnela deleted the ci/add-test-gate branch May 22, 2026 19:32
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