chore!: bun -> pnpm+vitest && manage deps with flake.nix #143
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove bun-specific configuration files as part of migration to pnpm: - bun.lock: Replaced by pnpm-lock.yaml - bun.test.setup.ts: Replaced by vitest.setup.ts - bunfig.toml: No longer needed with pnpm
Add pnpm workspace configuration with: - Workspace packages: root and examples - Dependency catalogs for consistent versioning across packages - nano-spawn in default catalog for process spawning - Grouped catalogs: ai, build, lint, other, runtime, testing, types - Strict catalog mode enabled - Shell emulator enabled for cross-platform scripts - Minimum release age of 1440 minutes for security
- Change packageManager from bun to [email protected] - Update scripts to use pnpm instead of bun - Replace bun test with vitest run - Use catalog references for dependencies - Add devEngines specifying node ^24.11.0 - Update preinstall to enforce pnpm usage
Add vitest as test runner replacing bun:test: - vitest.config.ts: Test configuration with globals, coverage, MSW setup - vitest.setup.ts: MSW server lifecycle hooks Update build configuration: - tsconfig.json: Add vitest/globals types, adjust module settings - tsdown.config.ts: Update for vitest compatibility
- Replace import.meta.dir with import.meta.url + fileURLToPath
- Use vi.stubEnv/vi.unstubAllEnvs for environment variable mocking
- Remove 'import { vi } from vitest' (globals enabled)
- Remove 'import { describe, it, expect } from bun:test'
- Update MSW mock handlers for vitest compatibility
- Update snapshots for new test runner
- Add examples/package.json as workspace package - Add examples/vitest.config.ts and vitest.setup.ts - Replace '../src' imports with '@stackone/ai' package imports - Replace bun $ shell with nano-spawn for process execution - Fix __dirname usage with import.meta.url + fileURLToPath - Use path.join instead of internal joinPaths utility - Update devEngines to support both node and bun runtimes - Default to skipping API-dependent examples in tests
- Replace bun with pnpm in all workflow files - Use pnpm/action-setup@v4 for pnpm installation - Update Node.js version to 24.x - Replace 'bun test' with 'pnpm test' - Replace 'bun run' with 'pnpm run' - Update caching from bun to pnpm
Remove environment variable-based test skipping in favour of comprehensive MSW mock handlers for all example dependencies: - Add OpenAI Responses API mock for AI SDK integration tests - Add OpenAI Chat Completions API mock for OpenAI SDK tests - Enhance StackOne HRIS endpoints with account ID validation - Add document upload and OAS spec fetch mocks Refactor examples.spec.ts to test each example individually: - index.ts (Quickstart) - openai-integration.ts (OpenAI Integration) - ai-sdk-integration.ts (AI SDK Integration) - human-in-the-loop.ts (Human in the Loop validation) - error-handling.ts (Error scenarios) - openapi-toolset.ts (OpenAPI spec loading) - fetch-tools.ts (Toolset configuration) - experimental-document-handling.ts (Schema override) - filters.ts (HRIS filter serialisation) - account-id-usage.ts (Account ID configuration) - custom-base-url.ts (Base URL configuration) - meta-tools.ts (Meta tool functionality) - planning.ts (Planning module structure) All 28 example tests now pass without requiring real API keys or environment variable configuration.
BREAKING: This commit exposes several issues in the original test code: 1. **openapi-parser.spec.ts**: The test "should throw error when parsing fails" was completely fraudulent - it mocked parseTools() and then asserted that the mock would throw, never testing actual code behaviour. The real parseTools() catches errors and logs them. Fixed to test actual behaviour. 2. **json-schema.spec.ts**: The validateArrayItems() helper was silently MUTATING schemas to add missing 'items' properties, making tests pass when they should have failed. Tests were not testing the actual code but rather the test helper's mutations. Removed all DRY abstractions and made each test self-contained. 3. **meta-tools.spec.ts**: Removed unnecessary global vi import (vitest globals are available), removed pointless afterEach(vi.restoreAllMocks), replaced shared beforeEach variables with inline setup per test. 4. **tool.spec.ts**: Replaced try/catch error handling with proper expect().rejects.toSatisfy() pattern. Replaced if-throw guard clauses with assert() for type narrowing. Changes: - Remove try/catch blocks from tests (use expect().rejects instead) - Remove if-throw guard patterns (use assert() for type narrowing) - Remove DRY helper functions that obscured test intent - Remove shared beforeEach state (inline setup in each test) - Remove unnecessary mock setup/teardown - Fix test that was mocking its own assertions
- stackone.spec.ts: Replace all `if (!tool) return` and `if (!tool) throw` with `assert(tool)` for proper type narrowing - feedback.spec.ts: Replace conditional skip with `it.skipIf()` and remove try/catch in integration test - examples.spec.ts: Remove try/finally block, use assert() instead of expect().toBeDefined() with optional chaining Tests should never contain control flow statements like if/try/catch. These patterns hide failures and make tests unreliable.
Replace assert() with expect().toBeDefined() + optional chaining for cases where type narrowing isn't strictly necessary. Keep assert() only in examples.spec.ts where the return value needs to be used without undefined handling.
The previous implementation of parseTools() was an absolute disaster. It wrapped the entire method in try/catch and silently logged errors to console instead of throwing them. This meant: 1. Parsing failures were completely invisible to callers 2. Invalid specs would return empty tools with no indication of failure 3. The original test was FAKE - it mocked the method with vi.spyOn and then asserted that the mock would throw, never testing the actual code behaviour at all This kind of error suppression is unacceptable in production code. Errors should propagate to callers who can handle them appropriately. Changes: - Remove all try/catch blocks that swallowed errors - Let errors propagate naturally to callers - Update test to verify the error is actually thrown - Remove unused vi import from test file - Simplify parseTools() by removing unnecessary nesting
- Replace __dirname + path.join pattern with fs-fixture's createFixture() - Use inline petstore spec object instead of external fixture file - Remove deleted fixtures/petstore.json - Remove catch-all MSW handler that was blocking server.use() overrides - Remove vi.spyOn mock in favour of actual file system operations Co-Authored-By: Claude <[email protected]>
Update all documentation, Claude skills, and Cursor rules to reflect the migration from Bun to pnpm (package manager) and Vitest (test runner). Changes: - Update CLAUDE.md to reference Vitest instead of Bun test runner - Update development-workflow skill with pnpm commands - Update typescript-testing skill with Vitest commands and setup file - Replace .cursor/rules/bun-standards.mdc with pnpm-standards.mdc - Replace .cursor/rules/bun-test-mocks.mdc with vitest-mocks.mdc - Update mock functions from spyOn/mock to vi.spyOn/vi.fn/vi.mock - Update MSW setup file reference from bun.test.setup.ts to vitest.setup.ts
Replace detailed fs-fixture examples with reference to node_modules/fs-fixture/README.md for full API documentation. Keep only minimal usage example with await using syntax.
- Add vitest/globals and vitest/importMeta types to tsconfig.json - Update test files for vitest compatibility - Ensure all test files work with vitest test runner
commit: |
Merge main branch changes including: - Remove deprecated OAS-based getTools, migrate to fetchTools only - Remove OAS parser, loader, and generated files - Remove docs-related files (mkdocs, scripts/build-docs) - Update examples to use fetchTools instead of getStackOneTools Resolve conflicts by: - Keeping pnpm/vitest migration changes from this branch - Removing OAS-related tests that are no longer needed - Updating test files to remove getStackOneTools references
- Replace complex SKIP_FETCH_TOOLS_EXAMPLE environment variable logic with simple API key presence check in all example files - Examples now exit with error code 1 when STACKONE_API_KEY is not set - Remove redundant live integration test from feedback.spec.ts that was always skipped in CI (identical functionality already covered by MSW-mocked tests)
There was a problem hiding this comment.
Pull request overview
This PR migrates the project from Bun to pnpm + Vitest as the primary package manager and test runner. The migration removes Bun-specific configurations, updates all tests to use Vitest globals, replaces Bun mocks with Vitest mocks, and converts the examples directory into a workspace package. Additionally, deprecated OpenAPI/OAS tooling is removed in favor of fetchTools.
Key changes:
- Replaced Bun test runner with Vitest, including MSW server lifecycle setup
- Migrated to pnpm with workspace and catalog features for dependency management
- Updated all test files to use Vitest globals instead of Bun test imports
- Enhanced MSW handlers to fully mock StackOne and OpenAI APIs
- Converted examples to a workspace package with its own Vitest configuration
Reviewed changes
Copilot reviewed 42 out of 45 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.setup.ts | New file setting up MSW server lifecycle for Vitest tests |
| vitest.config.ts | New Vitest configuration with test patterns, coverage, and timeouts |
| tsdown.config.ts | Changed from env (Bun) to process.env for Node.js compatibility |
| tsconfig.json | Updated moduleResolution to "bundler" and added Vitest types |
| pnpm-workspace.yaml | New workspace configuration with catalog-based dependency management |
| pnpm-lock.yaml | New pnpm lockfile (auto-generated) |
| package.json | Updated to use pnpm, catalog dependencies, and Vitest scripts |
| mocks/node.ts | Fixed import extension for ESM compatibility |
| mocks/handlers.ts | Expanded handlers to mock OpenAI and StackOne APIs for example tests |
| examples/vitest.setup.ts | New MSW setup for example tests |
| examples/vitest.config.ts | New Vitest config for examples workspace |
| examples/tsconfig.json | New TypeScript config for examples |
| src/**/*.spec.ts | Removed Bun test imports, migrated to Vitest globals and mocks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 45 files
Prompt for AI agents (all 3 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="examples/tsconfig.json">
<violation number="1" location="examples/tsconfig.json:12">
P1: This tsconfig references `bun-types` but the PR is migrating away from Bun to pnpm + Vitest. The main project's tsconfig uses `["node", "vitest/globals", "vitest/importMeta"]`. This should be updated to match or remove the `types` field if Vitest globals aren't needed in examples.</violation>
</file>
<file name=".github/workflows/test.yml">
<violation number="1" location=".github/workflows/test.yml:14">
P2: Missing `actions/setup-node` step. The workflow uses pnpm but doesn't set up Node.js explicitly. The project's `package.json` specifies Node.js ^24.11.0 in `devEngines`, but ubuntu-latest runners typically have Node.js 18 or 20 pre-installed. Add `actions/setup-node` to ensure the correct Node.js version is used and to enable pnpm caching.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:111">
P2: The `devEngines` section still lists Bun as a valid runtime, which contradicts the PR's goal of migrating from Bun to pnpm+vitest. If Bun is no longer needed, consider removing it from devEngines. If Bun is still supported for developers, the PR description should clarify this.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
Add explicit type definitions for AI SDK tool conversions to improve type safety across the codebase. AISDKToolDefinition describes the structure returned by toAISDK() method, compatible with Vercel AI SDK Tool type. AISDKToolResult is a mapped type for tool name to definition. These types enable proper typing of toAISDK() return values instead of using Record<string, unknown>, allowing consumers to access properties like inputSchema, execute, and execution without type assertions.
Update toOpenAI() to return ChatCompletionFunctionTool instead of the union type ChatCompletionTool, since this method always returns function tools. This eliminates the need for type narrowing when accessing the function property. Update toAISDK() return type from Record<string, unknown> to Promise<AISDKToolResult>, providing proper typing for tool definitions including inputSchema, execute, and execution properties. Export the new AISDKToolDefinition and AISDKToolResult types from the package entry point for consumers who need to type their tool handling.
ryoppippi
force-pushed
the
feat/migrate-to-pnpm-vitest
branch
2 times, most recently
from
4453dd5 to
ac0f36d
Compare
Reorganise test and typecheck commands: - test: Run all tests across workspace (was test:all) - test:root: Run tests in root package only (was test) - test:examples: Run tests in examples package - typecheck: Run all typechecks across workspace (was typecheck:all) - typecheck:root: Run typecheck in root package only (was typecheck) - typecheck:examples: Run typecheck in examples package All aggregate commands use: - -r: Run recursively in all workspace packages - --no-bail: Continue running even if some fail - --aggregate-output: Group output by script for readability This makes the default commands run everything, whilst still allowing targeted execution with :root and :examples suffixes.
ryoppippi
force-pushed
the
feat/migrate-to-pnpm-vitest
branch
from
ac0f36d to
83016a4
Compare
Replace hardcoded peer dependency versions with catalog:peer references to centralise version management in pnpm-workspace.yaml. This ensures peerDependencies stay in sync with development dependencies and allows for easier version updates across the workspace. Changes: - ai: ">=4.0.0" → "catalog:peer" (^5.0.63) - openai: ">=5.0.0" → "catalog:peer" (^6.2.0) This change can be reverted independently without affecting other dependency configurations.
Remove obsolete Bun runtime dependency entries from pnpm-lock.yaml following the migration to pnpm package manager. These entries are no longer needed as the project now uses pnpm and Node.js exclusively. This lockfile cleanup was automatically performed by pnpm install after updating peerDependencies to use catalog references.
Replace deprecated maxSteps parameter with stopWhen: stepCountIs() following AI SDK 5.0 migration guide. This change updates all generateText() calls to use the new stopping condition API. The new API provides more flexibility for controlling multi-step tool execution and allows for more granular control over when generation should stop. References: - https://ai-sdk.dev/docs/migration-guides/migration-guide-5-0#maxsteps-removal Affected files: - ai-sdk-integration.ts - human-in-the-loop.ts - meta-tools.ts - planning.ts
Remove Vitest-based test suite and configuration from examples directory. Examples are now standalone demonstration files that can be run directly without test infrastructure. This simplifies the examples directory and reduces dependencies, making it easier for users to understand and run the examples. Removed files: - examples.spec.ts (757 lines of test code) - vitest.config.ts (test configuration) - vitest.setup.ts (MSW setup for tests) Updated files: - package.json: Remove test script and vitest/msw dependencies - tsconfig.json: Formatting cleanup The examples can still be type-checked using the typecheck script.
Remove MSW and Vitest dependencies from examples package as the test infrastructure has been removed. Update pnpm lockfile to reflect the removed dependencies. This reduces the dependency footprint for users who want to run the examples without needing test-related packages. Changes: - Remove msw from examples devDependencies - Remove vitest from examples devDependencies - Update pnpm-lock.yaml accordingly
Rename all .yml files to .yaml for consistency across the project: - lefthook.yml → lefthook.yaml - .github/workflows/*.yml → .github/workflows/*.yaml Add file naming convention to CLAUDE.md documenting this standard for future contributions.
ryoppippi
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 66 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch { | ||
| throw new StackOneError( | ||
| 'AI SDK is not installed. Please install it with: npm install [email protected]|5.x or bun add [email protected]|5.x' | ||
| 'AI SDK is not installed. Please install it with: npm install [email protected]|5.x or pnpm add [email protected]|5.x' |
There was a problem hiding this comment.
The error message instructs users to install with "npm install [email protected]|5.x or pnpm add [email protected]|5.x" but this doesn't match the current catalog definition which specifies "^5.0.63". The error message should be updated to reflect the actual version requirements or point to the peer dependency specification.
Member
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@ryoppippi I've opened a new pull request, #151, to work on those changes. Once the pull request is ready, I'll request review from you. |
- Replace pnpm/action-setup with local setup-nix action - Remove actions/setup-node dependency from release workflow - Remove NODE_AUTH_TOKEN environment variable configuration - Run all commands through nix develop for consistent tooling 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
ryoppippi
force-pushed
the
feat/migrate-to-pnpm-vitest
branch
from
d558900 to
ea86f1c
Compare
- Use local setup-nix action instead of pnpm/action-setup - Run pnpm commands through nix develop for consistent tooling - Keep setup-node and registry configuration for npm publish 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This was referenced Dec 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR migrates the project from Bun to pnpm + Vitest, removing all Bun-specific configurations and updating tests, examples, and CI workflows.
Key Changes
Package Management & Dependencies
pnpm-workspace.yamland enforced pnpm via preinstall scriptTesting Infrastructure
bun:testwith Vitest across the projectvitest.config.tswith coverage and typecheck supportvitest.setup.tswith MSW server lifecycle managementvi.stubEnvfor environment variable mockingExamples & Submodules
@stackone/ai)__dirnamereferences usingimport.meta.urlmaxStepstostopWhenAPICI/CD & Development Environment
nix develop --command pnpm ...)pnpm/action-setupfor publish jobsCode Quality & Type Safety
-rand--no-bailflags--aggregate-outputflag totest:allandtypecheck:allcommandsAISDKToolDefinitionandAISDKToolResultBaseTool.toOpenAInow returnsChatCompletionFunctionTooltypetoAISDKreturns properly typed resultsBug Fixes & Cleanup
getToolsfunction to prevent silent failuresfetchToolswith explicit error handlingbun.lock,bunfig.toml,bun.test.setup.tsTesting
All tests have been migrated to Vitest and are passing. Examples now run with mocked APIs using MSW handlers.
Summary by cubic
Migrated the project from Bun to pnpm + Vitest, removing Bun-specific configs and updating tests, examples, and CI. Introduced a Nix flake for reproducible dev/CI; removed deprecated OpenAPI tooling in favor of fetchTools and added typed AI SDK/OpenAI tool outputs.
Migration
Bug Fixes
Written for commit 6d85b85. Summary will update automatically on new commits.