feat: bun first-class support - #784
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 87834cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Bun runtime and SQLite support packages, Bun-aware template generation and testing, a Bun verifier app, and CI, packaging, deploy, and type-resolution updates for Bun-focused builds. ChangesBun first-class support
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Closed
yasserf
force-pushed
the
bun-first-class-support
branch
from
June 26, 2026 13:26
7306a3f to
00ea509
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (3)
packages/runtimes/bun-server/package.json (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin
@types/bunto a supported range.Using
"latest"here makes the package's type surface non-reproducible and can break builds when Bun's ambient types change independently of the runtime you support. Prefer a concrete or semver-bounded version aligned with the Bun baseline inengines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/runtimes/bun-server/package.json` at line 22, The `@types/bun` dependency is currently unpinned, which makes the bun-server type surface non-reproducible and can break builds when ambient types change. Update the package.json entry for `@types/bun` to a concrete semver-bounded version that matches the Bun baseline declared in engines, and keep it aligned with the runtime supported by this package.verifiers/bun-server/src/services.ts (1)
18-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse injected services before creating new logger/schema instances.
This factory already preserves
existingServices.variablesandexistingServices.secrets, but it always replacesloggerandschema. SincepikkuServicesmerges injected services with the factory result, overriding them here makes verifier wiring less composable and can leave the schema using a different logger instance than the rest of the graph.Suggested change
- const logger = new ConsoleLogger() - const schema = new CFWorkerSchemaService(logger) + const logger = existingServices?.logger || new ConsoleLogger() + const schema = + existingServices?.schema || new CFWorkerSchemaService(logger)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@verifiers/bun-server/src/services.ts` around lines 18 - 19, The services factory is always creating a new ConsoleLogger and CFWorkerSchemaService instead of reusing injected ones, which breaks composability and can desync the schema from the rest of the service graph. Update the factory that builds the bun-server services to prefer any provided logger and schema from existingServices before instantiating new ones, just like it already does for variables and secrets. Keep the wiring consistent so pikkuServices can merge injected services without these defaults overwriting them.package.json (1)
48-48: 📐 Maintainability & Code Quality | 🔵 TrivialAdd
packages/runtimes/bun-server/srcto the roottest:bunscript.The current
test:bundefinition atpackage.json:48includespackages/services/kysely-bun-sqlite/srcbut omitspackages/runtimes/bun-server/src. While individual package tests are covered by the CI workflow loop overrun-tests.sh, the convenience commandtest:bunfails to exercise the primary Bun-specific runtime source.Current `test:bun` definition
"test:bun": "bun test packages/core/src packages/openapi-parser/src packages/openapi-to-zod-schema/src packages/pikku/src packages/runtimes/cloudflare/src packages/schedule/src packages/services/kysely-bun-sqlite/src",Include
packages/runtimes/bun-server/srcto ensure local Bun test runs cover the new runtime implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 48, The root test:bun script is missing the Bun server runtime source, so local Bun test runs do not cover the primary runtime implementation. Update the package.json test:bun command to include packages/runtimes/bun-server/src alongside the existing paths, keeping the rest of the script unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/actions/setup/action.yml:
- Around line 21-23: The setup-bun step currently uses a floating Bun version,
which makes the workflow non-reproducible. Update the `oven-sh/setup-bun@v2`
configuration in the action to use a pinned Bun release or a version file
instead of `bun-version: latest`, and keep the chosen version consistent with
local development by adjusting the `with` block accordingly.
In @.github/workflows/develop.yml:
- Around line 244-245: Remove the invalid matrix exclusion for nextjs-full in
the workflow matrix configuration, since matrix.template never includes that
value and the exclude entry in the workflow is dead config. Update the matrix
exclude section in develop.yml to only reference actual template values so
actionlint stops flagging the impossible match.
- Around line 251-253: The Test template workflow step passes github.ref_name
directly into the shell, which can allow command injection from crafted ref
names. Move the matrix.template, github.ref_name, and matrix.package-manager
values into env on this step and update the script invocation in the workflow to
reference quoted environment variables instead of inline interpolation, keeping
the fix localized to the Test template job.
In `@packages/cli/build.sh`:
- Around line 29-42: The bootstrap manifest in build.sh still uses floating
latest versions for `@pikku/auth-js` and the `@pikku/better-auth` override, which
makes bootstrapping non-reproducible. Update the package.json generation in the
bootstrap step so these dependencies are pinned to fixed versions alongside
`@pikku/cli` and keep the install flow unchanged.
- Around line 105-110: The schema lookup in build.sh can still terminate early
under set -euo pipefail because the find | head substitution fails before the
warning branch runs. Update the schema_src assignment and fallback around the
existing find, head, cp, and echo logic so the script first guards whether
.pikku/schemas exists (or otherwise suppresses the non-zero lookup failure) and
only then attempts to locate PikkuCLIConfig.schema.json; keep the current
warning path as the intended fallback when no schema is present.
In `@packages/core/package.json`:
- Line 63: Move `@types/json-schema` out of devDependencies and into dependencies
in package.json so downstream TypeScript consumers can resolve the public
JSONSchema7 type used by meta-service. Update the dependency section
accordingly, keeping the package version the same, and ensure the library’s
published manifest exposes the type definitions needed by the public API
surface.
In `@packages/create/src/index.ts`:
- Around line 114-118: The bun template entry is not being used to default the
package manager, so create-pikku with --template bun still falls back to npm.
Update the package-manager selection logic in run() (and any helper it uses) to
recognize the bun template and automatically choose bun when template is bun and
--package-manager is omitted, keeping the template and install behavior aligned.
In `@packages/create/src/utils.ts`:
- Around line 470-473: The packageManager assignment in the package creation
helper uses an unversioned Bun value, which does not match the required
<name>@<version> format. Update the bun branch in utils.ts where
packageJson.packageManager is set, and make it use a versioned Bun identifier
consistent with the Yarn case and the project’s supported Bun version. Keep the
existing packageManager handling logic intact, only change the Bun value to
include the version suffix.
In `@packages/runtimes/bun-server/run-tests.sh`:
- Line 25: The test file discovery in run-tests.sh is vulnerable to shell
splitting because the files array is populated directly from find output. Update
the logic around the files assignment to use find with -print0 and read the
results with mapfile so each test path remains a single array element even when
it contains whitespace or newlines.
In `@packages/runtimes/bun-server/src/pikku-bun-server.ts`:
- Around line 131-133: The binary frame handling in pikku-bun-server is
re-wrapping the full backing buffer when message is a Uint8Array, which can
include bytes outside the received frame. Update the bytes creation logic in the
binary message path to preserve the exact slice represented by the incoming
Uint8Array or ArrayBuffer, and avoid using message.buffer directly for
typed-array views. Keep the fix localized around the message-to-bytes conversion
used before channelHandler.binaryMessage().
In `@packages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.test.ts`:
- Around line 145-148: The test is bypassing the adapter’s iterator path by
calling `db.prepare()` and casting to `any`, so `BunSqliteStatement.iterate()`
is never actually verified. Update the test to prepare the query through `new
BunSqliteDatabase(db)` and call `iterate()` on the resulting
`BunSqliteStatement` so the adapter’s `iterate(parameters)` implementation is
exercised end-to-end. Keep the assertion behavior the same, but remove the
direct Bun statement cast to ensure this test would fail if
`BunSqliteStatement.iterate()` regressed.
In `@packages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.ts`:
- Around line 12-19: The BunSqliteStatement implementation is marking every
statement as a reader, which makes Kysely route writes through all() instead of
run(). Update the reader property on BunSqliteStatement to derive its value from
the underlying Statement metadata rather than hardcoding true, so SELECTs still
use all() while INSERT/UPDATE/DDL statements use run() and preserve mutation
metadata like insertId and rowsAffected.
In `@packages/services/kysely-bun-sqlite/src/coercion-plugin.ts`:
- Around line 44-57: The global flattening in buildGlobalMap makes shared column
names overwrite each other, so joined or aliased results can be coerced with the
wrong ColumnKind. Update the coercion lookup used by CoercionPlugin to detect
ambiguous column names across tables and avoid assigning a kind when the same
col or snakeToCamel(col) maps to different kinds; keep those entries uncoerced
instead of letting the last table win. Apply the same ambiguity handling
wherever the global lookup is consumed so the plugin only coerces unambiguous
columns.
In `@scripts/test-template.sh`:
- Around line 141-205: The non-Yarn path in scripts/test-template.sh currently
writes `@pikku/`* overrides into package.json via pkg.overrides inside the inline
Node script, but pnpm does not honor that location. Update the branch that
patches package.json so it either explicitly rejects pnpm before making the
changes, or moves the override-writing logic to pnpm-workspace.yaml instead; use
the existing localPaths mapping and the pkg.overrides assignment as the main
location to adjust.
In `@templates/bun/src/start.ts`:
- Around line 11-23: The scheduler is being created after
createSingletonServices() runs, so schedulerService stays undefined in the
service graph even though scheduled support is expected. Instantiate the
InMemorySchedulerService before calling createSingletonServices(), pass it
through the config/existing services path that createSingletonServices() and
services.ts use for schedulerService, and then reuse that same instance when
starting the app so the singleton graph and runtime scheduler stay in sync.
In `@templates/bun/tsconfig.json`:
- Around line 3-8: The Bun template tsconfig is overriding ambient types by
setting compilerOptions.types to only Node, which drops Bun globals from
type-checking. Update the tsconfig in the Bun template so the
compilerOptions.types list includes Bun alongside Node, keeping the existing
settings intact. Use the tsconfig.json compilerOptions block to locate and
adjust the types configuration.
In `@verifiers/bun-server/src/start.ts`:
- Line 7: The verifier port is hardcoded in the start logic, which makes
parallel or conflicting runs brittle; update the startup flow around the PORT
constant in start.ts so bootstrap can override it instead of always using 7979.
Use the existing start/bootstrap entrypoint to read an injected port value or
environment override, and fall back to the current default only when no override
is provided.
---
Nitpick comments:
In `@package.json`:
- Line 48: The root test:bun script is missing the Bun server runtime source, so
local Bun test runs do not cover the primary runtime implementation. Update the
package.json test:bun command to include packages/runtimes/bun-server/src
alongside the existing paths, keeping the rest of the script unchanged.
In `@packages/runtimes/bun-server/package.json`:
- Line 22: The `@types/bun` dependency is currently unpinned, which makes the
bun-server type surface non-reproducible and can break builds when ambient types
change. Update the package.json entry for `@types/bun` to a concrete
semver-bounded version that matches the Bun baseline declared in engines, and
keep it aligned with the runtime supported by this package.
In `@verifiers/bun-server/src/services.ts`:
- Around line 18-19: The services factory is always creating a new ConsoleLogger
and CFWorkerSchemaService instead of reusing injected ones, which breaks
composability and can desync the schema from the rest of the service graph.
Update the factory that builds the bun-server services to prefer any provided
logger and schema from existingServices before instantiating new ones, just like
it already does for variables and secrets. Keep the wiring consistent so
pikkuServices can merge injected services without these defaults overwriting
them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e35c387-fff9-4efe-ab29-e02b7231630c
⛔ Files ignored due to path filters (3)
.claude/scheduled_tasks.lockis excluded by!**/*.lockbun.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (46)
.changeset/bun-first-class-support.md.github/actions/build/action.yml.github/actions/setup/action.yml.github/workflows/develop.yml.gitignorepackage.jsonpackages/cli/build.shpackages/core/package.jsonpackages/core/src/dev/hot-reload.test.tspackages/core/src/wirings/oauth2/oauth2-client.test.tspackages/create/src/index.tspackages/create/src/utils.tspackages/cucumber/tsconfig.jsonpackages/runtimes/bun-server/package.jsonpackages/runtimes/bun-server/run-tests.shpackages/runtimes/bun-server/src/bun-event-hub-service.tspackages/runtimes/bun-server/src/index.tspackages/runtimes/bun-server/src/pikku-bun-server.tspackages/runtimes/bun-server/tsconfig.jsonpackages/runtimes/modelcontextprotocol/package.jsonpackages/schedule/package.jsonpackages/services/kysely-bun-sqlite/package.jsonpackages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.test.tspackages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.tspackages/services/kysely-bun-sqlite/src/coercion-plugin.tspackages/services/kysely-bun-sqlite/src/create-bun-sqlite-kysely.tspackages/services/kysely-bun-sqlite/src/index.tspackages/services/kysely-bun-sqlite/tsconfig.jsonscripts/test-template.shtemplates/bun/package.jsontemplates/bun/src/start.tstemplates/bun/tsconfig.jsontsconfig.jsonverifiers/bun-server/package.jsonverifiers/bun-server/pikku.config.jsonverifiers/bun-server/src/functions/echo.channel.tsverifiers/bun-server/src/functions/echo.channel.wiring.tsverifiers/bun-server/src/functions/greeting.function.tsverifiers/bun-server/src/functions/greeting.wiring.tsverifiers/bun-server/src/services.tsverifiers/bun-server/src/start.tsverifiers/bun-server/tsconfig.jsonverifiers/bun-server/types/application-types.d.tsverifiers/gateway/tsconfig.jsonverifiers/middleware-and-permissions/tsconfig.jsonverifiers/treeshaking/tsconfig.json
Comment on lines
+21
to
+23
| - uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version: latest |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
oven-sh/setup-bun pin specific bun version best practices
💡 Result:
To pin a specific version of Bun using the oven-sh/setup-bun GitHub Action, you have three primary methods. Best practice involves either explicitly defining the version in your workflow file or using a version file to keep your CI configuration synchronized with your development environment [1][2]. 1. Explicit Input (Recommended for consistency): You can specify the version directly in your workflow file using the bun-version input [1][3]. This is the most straightforward way to ensure every pipeline run uses an identical, known-good version. - uses: oven-sh/setup-bun@v2 with: bun-version: 1.3.3 2. Using a Version File (Best for project synchronization): To keep your CI in sync with the version used locally (e.g., if you use.bun-version or.tool-versions), point the action to that file. This prevents drift between local development and CI/CD [1][2]. - uses: oven-sh/setup-bun@v2 with: bun-version-file:.bun-version 3. Automatic Resolution (Default Behavior): If you do not provide a version, the action attempts to resolve the version automatically in this order [1][2]: - The packageManager field in package.json (e.g., "packageManager": "bun@1.0.25"). - The engines.bun field in package.json. - If neither is present, it defaults to latest. For production or critical pipelines, explicitly pinning via bun-version or a version file is recommended to avoid unexpected behavior from new Bun releases [1]. You can also use special tags like latest or canary when specific pinning is not required [1][4].
Citations:
- 1: https://github.com/oven-sh/setup-bun
- 2: https://github.com/marketplace/actions/setup-bun
- 3: https://bun.com/docs/guides/runtime/cicd
- 4: https://github.com/oven-sh/setup-bun/releases
Pin the Bun toolchain version.
Using bun-version: latest makes CI non-reproducible; a new Bun release can break the workflow without any code change. Follow the official oven-sh/setup-bun best practices by either:
- Explicitly specifying a fixed version (e.g.,
bun-version: '1.3.3'), or - Using a version file (e.g.,
bun-version-file: .bun-version) to keep CI in sync with local development.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/actions/setup/action.yml around lines 21 - 23, The setup-bun step
currently uses a floating Bun version, which makes the workflow
non-reproducible. Update the `oven-sh/setup-bun@v2` configuration in the action
to use a pinned Bun release or a version file instead of `bun-version: latest`,
and keep the chosen version consistent with local development by adjusting the
`with` block accordingly.
yasserf
force-pushed
the
bun-first-class-support
branch
3 times, most recently
from
June 26, 2026 14:30
cf1b054 to
c625f0e
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
yasserf
force-pushed
the
bun-first-class-support
branch
2 times, most recently
from
June 26, 2026 15:35
5b14eba to
98a5b02
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/develop.yml:
- Line 247: Update the checkout step in the workflow to avoid the floating
actions/checkout@v4 reference and disable saved Git credentials. Locate the
actions/checkout usage in the develop workflow and replace it with a SHA-pinned
v4 release while adding persist-credentials: false under its with block, since
the downstream test and coverage steps do not need git auth.
In `@scripts/test-template.sh`:
- Around line 33-44: The template script currently treats every non-yarn
PACKAGE_MANAGER as Bun, which can send npm/pnpm/typos into the Bun-specific
setup path and fail later with unclear errors. Update the PACKAGE_MANAGER
handling in scripts/test-template.sh to explicitly validate the value before the
branching logic in the main template flow, and reject anything other than
supported managers with a clear log_error/exit path. Use the existing
PACKAGE_MANAGER variable and the downstream branching block that distinguishes
yarn vs the Bun-specific section to place the guard so unsupported managers fail
fast.
In `@templates/functions/run-tests.sh`:
- Line 151: The server launch in run-tests.sh is executing user-provided
SERVER_CMD through bash -c, which enables arbitrary shell interpretation. Update
the startup logic around SERVER_CMD and the server execution block to invoke the
command directly as arguments instead of shell evaluation, or add strict
allow-list validation before any execution. Use the SERVER_CMD assignment from
the --server argument and the server launch path near SERVER_PID to keep the fix
localized.
In `@verifiers/bun-server/src/start.ts`:
- Around line 62-84: The WebSocket in the bun server verifier is not being
closed on timeout and error paths, which can leak connections during failures.
Update the promise setup in start.ts so the ws created in the WebSocket
onConnect check is explicitly closed before rejecting in both the setTimeout
handler and the ws.on('error') handler, matching the existing cleanup already
done in the message path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c863f16b-6285-41b1-9b97-744ea72b5220
⛔ Files ignored due to path filters (2)
.claude/scheduled_tasks.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (50)
.changeset/bun-first-class-support.md.github/actions/build/action.yml.github/actions/setup/action.yml.github/workflows/develop.yml.github/workflows/main.yml.gitignorepackage.jsonpackages/cli/build.shpackages/core/package.jsonpackages/core/src/dev/hot-reload.test.tspackages/core/src/wirings/oauth2/oauth2-client.test.tspackages/create/src/index.tspackages/create/src/utils.tspackages/cucumber/tsconfig.jsonpackages/runtimes/bun-server/package.jsonpackages/runtimes/bun-server/run-tests.shpackages/runtimes/bun-server/src/bun-event-hub-service.tspackages/runtimes/bun-server/src/index.tspackages/runtimes/bun-server/src/pikku-bun-server.tspackages/runtimes/bun-server/tsconfig.jsonpackages/runtimes/modelcontextprotocol/package.jsonpackages/schedule/package.jsonpackages/services/kysely-bun-sqlite/package.jsonpackages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.test.tspackages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.tspackages/services/kysely-bun-sqlite/src/coercion-plugin.tspackages/services/kysely-bun-sqlite/src/create-bun-sqlite-kysely.tspackages/services/kysely-bun-sqlite/src/index.tspackages/services/kysely-bun-sqlite/tsconfig.jsonscripts/test-template.shtemplates/bun/package.jsontemplates/bun/src/start.tstemplates/bun/tsconfig.jsontemplates/functions/run-tests.shtemplates/workflows-bullmq/package.jsontemplates/workflows-pg-boss/package.jsontsconfig.jsonverifiers/bun-server/package.jsonverifiers/bun-server/pikku.config.jsonverifiers/bun-server/src/functions/echo.channel.tsverifiers/bun-server/src/functions/echo.channel.wiring.tsverifiers/bun-server/src/functions/greeting.function.tsverifiers/bun-server/src/functions/greeting.wiring.tsverifiers/bun-server/src/services.tsverifiers/bun-server/src/start.tsverifiers/bun-server/tsconfig.jsonverifiers/bun-server/types/application-types.d.tsverifiers/gateway/tsconfig.jsonverifiers/middleware-and-permissions/tsconfig.jsonverifiers/treeshaking/tsconfig.json
✅ Files skipped from review due to trivial changes (17)
- packages/runtimes/bun-server/tsconfig.json
- templates/bun/src/start.ts
- verifiers/bun-server/src/functions/echo.channel.wiring.ts
- packages/runtimes/bun-server/package.json
- packages/services/kysely-bun-sqlite/tsconfig.json
- packages/services/kysely-bun-sqlite/src/index.ts
- verifiers/bun-server/types/application-types.d.ts
- templates/bun/tsconfig.json
- tsconfig.json
- verifiers/bun-server/tsconfig.json
- verifiers/gateway/tsconfig.json
- .changeset/bun-first-class-support.md
- templates/workflows-bullmq/package.json
- verifiers/middleware-and-permissions/tsconfig.json
- .github/actions/build/action.yml
- packages/core/package.json
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (26)
- packages/create/src/utils.ts
- verifiers/treeshaking/tsconfig.json
- templates/bun/package.json
- verifiers/bun-server/src/functions/greeting.wiring.ts
- verifiers/bun-server/package.json
- packages/services/kysely-bun-sqlite/package.json
- verifiers/bun-server/src/functions/greeting.function.ts
- verifiers/bun-server/pikku.config.json
- packages/runtimes/modelcontextprotocol/package.json
- packages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.ts
- .github/actions/setup/action.yml
- packages/runtimes/bun-server/src/index.ts
- verifiers/bun-server/src/services.ts
- verifiers/bun-server/src/functions/echo.channel.ts
- packages/runtimes/bun-server/src/bun-event-hub-service.ts
- packages/cucumber/tsconfig.json
- packages/core/src/dev/hot-reload.test.ts
- packages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.test.ts
- packages/create/src/index.ts
- package.json
- packages/services/kysely-bun-sqlite/src/coercion-plugin.ts
- packages/services/kysely-bun-sqlite/src/create-bun-sqlite-kysely.ts
- packages/core/src/wirings/oauth2/oauth2-client.test.ts
- packages/schedule/package.json
- packages/cli/build.sh
- packages/runtimes/bun-server/src/pikku-bun-server.ts
| - package-manager: bun | ||
| template: nextjs-full | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/develop.yml | sed -n '240,260p'Repository: pikkujs/pikku
Length of output: 1197
🏁 Script executed:
cat -n .github/workflows/develop.yml | sed -n '200,247p'Repository: pikkujs/pikku
Length of output: 1776
🏁 Script executed:
cat -n .github/workflows/develop.yml | sed -n '247,280p'Repository: pikkujs/pikku
Length of output: 2010
🏁 Script executed:
cat -n .github/actions/setup/action.ymlRepository: pikkujs/pikku
Length of output: 1126
Pin actions/checkout to a SHA and disable persisted credentials.
The workflow uses the floating tag actions/checkout@v4. Since subsequent steps (running tests, generating coverage, uploading to Codecov) do not perform any git operations (e.g., push, pull, or PR creation), the default persist-credentials: true is unnecessary and poses a supply-chain risk.
Update the step to pin the action to a specific SHA and explicitly set persist-credentials: false:
Proposed change
- uses: actions/checkout@<sha-pinned-v4>
with:
persist-credentials: false🧰 Tools
🪛 zizmor (1.26.1)
[warning] 247-247: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 247-247: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/develop.yml at line 247, Update the checkout step in the
workflow to avoid the floating actions/checkout@v4 reference and disable saved
Git credentials. Locate the actions/checkout usage in the develop workflow and
replace it with a SHA-pinned v4 release while adding persist-credentials: false
under its with block, since the downstream test and coverage steps do not need
git auth.
Source: Linters/SAST tools
| echo "Skipping server start (--no-start), assuming external server at $HELLO_WORLD_URL_PREFIX" | ||
| else | ||
| echo "Starting server: $SERVER_CMD" | ||
| bash -c "$SERVER_CMD" & SERVER_PID=$! |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect all SERVER_CMD assignments and the bash -c call:"
rg -n -C3 '\bSERVER_CMD\b|bash -c "\$SERVER_CMD"' templates/functions/run-tests.shRepository: pikkujs/pikku
Length of output: 881
Remove bash -c indirection; SERVER_CMD accepts arbitrary user input.
SERVER_CMD is assigned directly from the --server argument (line 50). Executing this string via bash -c (line 151) executes arbitrary shell code provided by the user. Refactor to execute arguments directly or validate against an allow-list before execution.
- bash -c "$SERVER_CMD" & SERVER_PID=$!
+ eval "$SERVER_CMD" & SERVER_PID=$!🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/functions/run-tests.sh` at line 151, The server launch in
run-tests.sh is executing user-provided SERVER_CMD through bash -c, which
enables arbitrary shell interpretation. Update the startup logic around
SERVER_CMD and the server execution block to invoke the command directly as
arguments instead of shell evaluation, or add strict allow-list validation
before any execution. Use the SERVER_CMD assignment from the --server argument
and the server launch path near SERVER_PID to keep the fix localized.
Source: Linters/SAST tools
yasserf
force-pushed
the
bun-first-class-support
branch
from
June 26, 2026 16:19
98a5b02 to
20a7af8
Compare
Adds first-class Bun support, rebuilt cleanly on top of latest main so it carries only Bun-related changes (the earlier branch had accumulated unrelated changesets, docs, and a stale db-codegen snapshot already superseded in main). - @pikku/bun-server: Bun HTTP + WebSocket runtime with health checks and graceful shutdown. - @pikku/kysely-bun-sqlite: Bun-native SQLite Kysely adapter + coercion plugin. - templates/bun + verifiers/bun-server: Bun starter template and verifier. - CI: Bun added to the template matrix; setup-bun + dist .d.ts in the build artifact for Bun file: deps. - create: --skip-install so the CI harness can patch deps before installing. - test-template.sh: Bun package-manager path. Its file: patch now scans templates/ too, forces every @pikku/* to the in-repo copy via bun overrides, and de-duplicates deps across dependency types (a package in both dependencies and devDependencies made bun emit a broken self-referential symlink, breaking tsc/runtime resolution). - tsconfig/skipLibCheck and node:test-mock shims so non-Bun verifiers and core tests tolerate bun-types / Bun's node:test. Also untracks .claude/scheduled_tasks.lock and gitignores it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yasserf
force-pushed
the
bun-first-class-support
branch
from
June 26, 2026 16:39
20a7af8 to
8276505
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/main.yml (1)
77-114: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winScope this job's token permissions.
This job only needs to read the repository and execute tests, so relying on default
GITHUB_TOKENpermissions is unnecessarily broad. Add a minimal job-levelpermissionsblock.Suggested change
verifiers: name: Verifiers (${{ matrix.verifier }}, ${{ matrix.package-manager }}) + permissions: + contents: read runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/main.yml around lines 77 - 114, The verifiers job is using the default broad GITHUB_TOKEN permissions even though it only checks out code and runs tests. Add a minimal job-level permissions block to the verifiers job in the workflow, granting only repository read access, and keep the existing matrix/steps intact for the verifiers job configuration.Source: Linters/SAST tools
.github/workflows/develop.yml (1)
90-123: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winScope this job's token permissions.
This verifier job only checks out the repo and runs local tests, so inheriting the default
GITHUB_TOKENpermissions is broader than necessary. Add a job-levelpermissionsblock with the minimum read access it needs.Suggested change
verifiers: name: Verifiers (${{ matrix.verifier }}, ${{ matrix.package-manager }}) + permissions: + contents: read runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/develop.yml around lines 90 - 123, The verifiers job currently inherits the default GITHUB_TOKEN permissions even though it only checks out code and runs local tests. Add a job-level permissions block to the verifiers job in develop.yml with the minimum required read-only access, and keep the change scoped to that job so the matrix steps using actions/checkout and ./.github/actions/setup still work without broader token privileges.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/main.yml:
- Around line 108-113: Update the workflow step sequence in the main job to
harden supply-chain security: the existing actions/checkout and
oven-sh/setup-bun usages are still tied to mutable tags, so pin both to fixed
commit SHAs and add persist-credentials: false to the checkout step. Keep the
same setup behavior in the workflow, but reference the actions/checkout and
setup-bun entries directly so they can be located even if the YAML shifts.
In `@packages/runtimes/bun-server/src/pikku-bun-server.test.ts`:
- Around line 8-9: The bun server integration tests are hardcoded to fixed
ports, which can collide with other processes or parallel test shards. Update
the test setup in pikku-bun-server.test.ts to bind the server to ephemeral ports
chosen by the OS, then read the actual bound port from the server/listener
before issuing fetch requests. Apply the same pattern throughout the affected
test cases and keep using the HEALTH path constant for the readiness check.
---
Outside diff comments:
In @.github/workflows/develop.yml:
- Around line 90-123: The verifiers job currently inherits the default
GITHUB_TOKEN permissions even though it only checks out code and runs local
tests. Add a job-level permissions block to the verifiers job in develop.yml
with the minimum required read-only access, and keep the change scoped to that
job so the matrix steps using actions/checkout and ./.github/actions/setup still
work without broader token privileges.
In @.github/workflows/main.yml:
- Around line 77-114: The verifiers job is using the default broad GITHUB_TOKEN
permissions even though it only checks out code and runs tests. Add a minimal
job-level permissions block to the verifiers job in the workflow, granting only
repository read access, and keep the existing matrix/steps intact for the
verifiers job configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40087c04-e013-4208-be46-863e3da26012
⛔ Files ignored due to path filters (2)
.claude/scheduled_tasks.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (55)
.changeset/bun-first-class-support.md.github/actions/build/action.yml.github/actions/setup/action.yml.github/workflows/develop.yml.github/workflows/main.yml.gitignorepackage.jsonpackages/cli/build.shpackages/core/package.jsonpackages/core/src/dev/hot-reload.test.tspackages/core/src/wirings/oauth2/oauth2-client.test.tspackages/create/src/index.tspackages/create/src/utils.tspackages/cucumber/tsconfig.jsonpackages/runtimes/bun-server/package.jsonpackages/runtimes/bun-server/run-tests.shpackages/runtimes/bun-server/src/bun-event-hub-service.test.tspackages/runtimes/bun-server/src/bun-event-hub-service.tspackages/runtimes/bun-server/src/index.tspackages/runtimes/bun-server/src/pikku-bun-server.test.tspackages/runtimes/bun-server/src/pikku-bun-server.tspackages/runtimes/bun-server/tsconfig.jsonpackages/runtimes/modelcontextprotocol/package.jsonpackages/schedule/package.jsonpackages/services/kysely-bun-sqlite/package.jsonpackages/services/kysely-bun-sqlite/run-tests.shpackages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.test.tspackages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.tspackages/services/kysely-bun-sqlite/src/coercion-plugin.test.tspackages/services/kysely-bun-sqlite/src/coercion-plugin.tspackages/services/kysely-bun-sqlite/src/create-bun-sqlite-kysely.test.tspackages/services/kysely-bun-sqlite/src/create-bun-sqlite-kysely.tspackages/services/kysely-bun-sqlite/src/index.tspackages/services/kysely-bun-sqlite/tsconfig.jsonscripts/test-template.shtemplates/bun/package.jsontemplates/bun/src/start.tstemplates/bun/tsconfig.jsontemplates/functions/run-tests.shtemplates/workflows-bullmq/package.jsontemplates/workflows-pg-boss/package.jsontsconfig.jsonverifiers/bun-server/package.jsonverifiers/bun-server/pikku.config.jsonverifiers/bun-server/src/functions/echo.channel.tsverifiers/bun-server/src/functions/echo.channel.wiring.tsverifiers/bun-server/src/functions/greeting.function.tsverifiers/bun-server/src/functions/greeting.wiring.tsverifiers/bun-server/src/services.tsverifiers/bun-server/src/start.tsverifiers/bun-server/tsconfig.jsonverifiers/bun-server/types/application-types.d.tsverifiers/gateway/tsconfig.jsonverifiers/middleware-and-permissions/tsconfig.jsonverifiers/treeshaking/tsconfig.json
✅ Files skipped from review due to trivial changes (10)
- verifiers/bun-server/pikku.config.json
- templates/workflows-pg-boss/package.json
- packages/schedule/package.json
- packages/services/kysely-bun-sqlite/src/index.ts
- .changeset/bun-first-class-support.md
- verifiers/bun-server/package.json
- verifiers/middleware-and-permissions/tsconfig.json
- verifiers/bun-server/types/application-types.d.ts
- verifiers/treeshaking/tsconfig.json
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (31)
- tsconfig.json
- verifiers/bun-server/src/functions/echo.channel.wiring.ts
- templates/bun/package.json
- templates/workflows-bullmq/package.json
- .github/actions/build/action.yml
- packages/runtimes/bun-server/package.json
- verifiers/bun-server/src/functions/echo.channel.ts
- packages/create/src/utils.ts
- packages/runtimes/bun-server/tsconfig.json
- packages/services/kysely-bun-sqlite/tsconfig.json
- templates/bun/tsconfig.json
- verifiers/bun-server/tsconfig.json
- verifiers/bun-server/src/functions/greeting.function.ts
- .github/actions/setup/action.yml
- packages/services/kysely-bun-sqlite/package.json
- packages/runtimes/bun-server/src/index.ts
- verifiers/bun-server/src/services.ts
- packages/services/kysely-bun-sqlite/src/create-bun-sqlite-kysely.ts
- verifiers/bun-server/src/functions/greeting.wiring.ts
- packages/runtimes/modelcontextprotocol/package.json
- packages/services/kysely-bun-sqlite/src/bun-sqlite-adapter.ts
- verifiers/gateway/tsconfig.json
- package.json
- packages/services/kysely-bun-sqlite/src/coercion-plugin.ts
- templates/bun/src/start.ts
- packages/cucumber/tsconfig.json
- packages/core/src/dev/hot-reload.test.ts
- packages/create/src/index.ts
- packages/runtimes/bun-server/src/bun-event-hub-service.ts
- packages/core/src/wirings/oauth2/oauth2-client.test.ts
- packages/runtimes/bun-server/src/pikku-bun-server.ts
Comment on lines
108
to
113
| - uses: actions/checkout@v4 | ||
| - uses: ./.github/actions/setup | ||
| - uses: oven-sh/setup-bun@v2 | ||
| if: matrix.verifier == 'binary' | ||
| with: | ||
| bun-version: latest |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the specific lines to confirm context and line numbers
sed -n '105,120p' .github/workflows/main.ymlRepository: pikkujs/pikku
Length of output: 702
🌐 Web query:
best pinned SHA for actions/checkout@v4 and oven-sh/setup-bun@v2 security best practices 2026
💡 Result:
In 2026, the industry standard and recommended security best practice for using GitHub Actions is to pin all actions to their full-length (40-character) commit SHA [1][2][3]. Pinning by tag (e.g., @v4) or branch (e.g., @main) is considered insecure because these references are mutable and can be updated by bad actors to inject malicious code [1][4][2][5]. As of June 26, 2026, the following SHAs are recommended for the latest stable versions of your requested actions: 1. actions/checkout: The latest commit for v4 as of June 16, 2026, is d914b262ffc244530a203ab40decab34c3abf34d [6]. Usage: - uses: actions/checkout@d914b26 # v4 2. oven-sh/setup-bun: The latest release is v2.2.0 (published March 14, 2026) [7][8]. The commit SHA for this release is 0c5077e [9]. Usage: - uses: oven-sh/setup-bun@0c5077e # v2.2.0 Security Implementation Best Practices: - Always use the full 40-character SHA [1][3]. - Include the original tag name as a comment (e.g., # v4.x.x) to maintain readability and facilitate easier identification for automated dependency update tools like Renovate or Dependabot [2][5]. - Enable GitHub's organization-level policy to enforce SHA pinning [1][3]. When enabled, this policy causes any workflow using a non-pinned action to fail, protecting your CI/CD pipeline from supply chain attacks [4][3]. - Use automated tools like Renovate (with the helpers:pinGitHubActionDigests preset) to keep your pinned SHAs up to date automatically, mitigating the maintenance overhead of manual pinning [2][5].
Citations:
- 1: https://docs.github.com/en/actions/reference/security/secure-use
- 2: https://architecture.lullabot.com/adr/20260505-pin-github-actions-to-hash/
- 3: https://github.blog/changelog/2025-08-15-github-actions-policy-now-supports-blocking-and-sha-pinning-actions/
- 4: https://www.romainlespinasse.dev/posts/github-actions-commit-sha-pinning/
- 5: https://safeguard.sh/resources/blog/github-actions-pinning-sha-vs-tag
- 6: actions/checkout@d914b26
- 7: https://github.com/oven-sh/setup-bun
- 8: https://github.com/oven-sh/setup-bun/releases/tag/v2.2.0
- 9: oven-sh/setup-bun@v2.1.3...v2.2.0
🏁 Script executed:
#!/bin/bash
# Fetch the full commit SHA for oven-sh/setup-bun v2.2.0
curl -s https://api.github.com/repos/oven-sh/setup-bun/git/refs/tags/v2.2.0 | grep '"sha"' | cut -d'"' -f4Repository: pikkujs/pikku
Length of output: 192
🌐 Web query:
full commit SHA for oven-sh/setup-bun v2.2.0
💡 Result:
The full commit SHA for the oven-sh/setup-bun v2.2.0 release is 0c5077e51419868618aeaa5fe8019c62421857d6 [1][2]. This release was published on March 14, 2026 [3][4].
Citations:
- 1: oven-sh/setup-bun@0c5077e
- 2: oven-sh/setup-bun@v2.1.3...v2.2.0
- 3: https://github.com/oven-sh/setup-bun/releases/tag/v2.2.0
- 4: release: v2.2.0 oven-sh/setup-bun#177
Pin actions to specific SHAs and disable persisted Git credentials.
actions/checkout and oven-sh/setup-bun currently use mutable tags (v4, v2), exposing the workflow to supply chain risks if these tags are updated. Pin them to their latest stable commit SHAs and add persist-credentials: false to the checkout step to prevent accidental credential leakage.
- uses: actions/checkout@d914b262ffc244530a203ab40decab34c3abf34d # v4
with:
persist-credentials: false
- uses: ./.github/actions/setup
- uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0
if: matrix.verifier == 'binary'
with:
bun-version: latestThis ensures the workflow executes exactly the code version intended at the time of the policy, preventing unforeseen breakages or malicious injections via tag updates.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 108-108: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 108-108: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 110-110: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/main.yml around lines 108 - 113, Update the workflow step
sequence in the main job to harden supply-chain security: the existing
actions/checkout and oven-sh/setup-bun usages are still tied to mutable tags, so
pin both to fixed commit SHAs and add persist-credentials: false to the checkout
step. Keep the same setup behavior in the workflow, but reference the
actions/checkout and setup-bun entries directly so they can be located even if
the YAML shifts.
Source: Linters/SAST tools
Comment on lines
+8
to
+9
| const PORT = 47817 | ||
| const HEALTH = '/__health' |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use ephemeral ports in these integration tests.
Binding the suite to 47817/47818 makes it fail whenever those ports are already in use or another shard runs the same test concurrently. Let the OS choose a free port and read the bound port back before calling fetch.
Also applies to: 31-37, 45-58
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/runtimes/bun-server/src/pikku-bun-server.test.ts` around lines 8 -
9, The bun server integration tests are hardcoded to fixed ports, which can
collide with other processes or parallel test shards. Update the test setup in
pikku-bun-server.test.ts to bind the server to ephemeral ports chosen by the OS,
then read the actual bound port from the server/listener before issuing fetch
requests. Apply the same pattern throughout the affected test cases and keep
using the HEALTH path constant for the readiness check.
- @pikku/cli: add --runtime node|bun to deploy plan/apply (default node) - @pikku/deploy-standalone: bun entry (PikkuBunServer, native WS) compiled to a self-contained executable via bun build --compile; node path unchanged; drop unused @yao-pkg/pkg dep + stale type shim (pkg path dropped in #489) - @pikku/bun-server: PikkuBunServer accepts an injectable eventHub so functions and the WebSocket transport share one hub (fixes channel broadcast on bun) - templates/bun + standalone bun entry wire the shared eventHub - verifiers/deploy-standalone: bun verifier incl. HTTP->WS broadcast test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/runtimes/bun-server/src/pikku-bun-server.ts (1)
88-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle
Upgradeheader values case-insensitively.A client sending
Upgrade: WebSocketwill miss this branch and be routed as a normal HTTP request instead of upgrading.Proposed fix
- if (req.headers.get('upgrade') === 'websocket') { + if (req.headers.get('upgrade')?.toLowerCase() === 'websocket') {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/runtimes/bun-server/src/pikku-bun-server.ts` at line 88, The websocket upgrade check in pikku-bun-server should treat the Upgrade header value case-insensitively, since a value like WebSocket currently bypasses the branch in the request handling flow. Update the conditional in the request path that inspects req.headers.get('upgrade') so it normalizes the header value before comparison, and keep the logic localized around the websocket upgrade branch in pikku-bun-server.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/functions/commands/deploy-apply.ts`:
- Around line 238-242: `deployApply` is resolving the provider runtime from the
current flags instead of preserving the planned runtime for `--from-plan`, which
can cause a Bun plan to be applied as a Node bundle. Update the
`deploy-apply.ts` flow in `deployApply` to read or carry forward the runtime
from the plan output (or require an explicit matching runtime when `fromPlan` is
set) before calling `resolveProvider`, so the provider used for apply matches
the planned standalone artifact.
In `@packages/deploy/deploy-standalone/src/adapter.ts`:
- Around line 62-70: The StandaloneProviderAdapter constructor currently
defaults any unknown runtime to node, which hides typos from the CLI/provider
boundary. Update the StandaloneProviderAdapter initialization and related
runtime handling in adapter.ts so only supported values are accepted, and throw
an explicit error for anything other than 'node' or 'bun'. Keep the branching in
generateEntrySource and the helper methods aligned with this validation so
unsupported runtimes fail fast instead of falling through to
generateNodeEntrySource.
In `@verifiers/deploy-standalone/test-deploy-bun.ts`:
- Around line 117-118: The deploy verifier is using a bind address as the client
probe target, which can cause flaky checks; keep the server bound with the
existing 0.0.0.0 setting, but change the probe URL/host in the deploy flow to
use 127.0.0.1 instead. Update the client-side checks in test-deploy-bun.ts
wherever the probe target is constructed so the affected verification steps
reference loopback while preserving the server bind behavior.
- Around line 13-14: The verifier setup still relies on shell-based execution,
which should be removed in favor of safer direct process/file APIs. Update the
setup logic in the verifier entrypoint that uses execSync with PIKKU_BIN so it
runs through execFileSync('node', [PIKKU_BIN, ...]) instead of building a shell
string, and replace any rm -rf-style cleanup with rmSync. Use the existing
execFileSync import and the verifier setup symbols around PIKKU_BIN to locate
and adjust the affected code.
- Around line 78-111: The entry-generation assertions in test-deploy-bun are
being scheduled without waiting for them to finish, so the failure state can be
printed before these checks update it. Update the checks around check(...) in
test-deploy-bun so the assertion flow is awaited or otherwise serialized,
ensuring the entry validation for PikkuBunServer, `@pikku/bun-server`, and the
bootstrap/main() checks complete before failures are reported.
---
Outside diff comments:
In `@packages/runtimes/bun-server/src/pikku-bun-server.ts`:
- Line 88: The websocket upgrade check in pikku-bun-server should treat the
Upgrade header value case-insensitively, since a value like WebSocket currently
bypasses the branch in the request handling flow. Update the conditional in the
request path that inspects req.headers.get('upgrade') so it normalizes the
header value before comparison, and keep the logic localized around the
websocket upgrade branch in pikku-bun-server.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91020a12-8efb-4db9-8cab-24b92e014e91
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
.changeset/standalone-bun-runtime.mdpackages/cli/src/cli.wiring.tspackages/cli/src/functions/commands/deploy-apply.tspackages/cli/src/functions/commands/deploy-plan.tspackages/deploy/deploy-standalone/package.jsonpackages/deploy/deploy-standalone/src/adapter.tspackages/deploy/deploy-standalone/src/index.tspackages/deploy/deploy-standalone/src/yao-pkg.d.tspackages/runtimes/bun-server/src/pikku-bun-server.tstemplates/bun/src/start.tsverifiers/deploy-standalone/package.jsonverifiers/deploy-standalone/test-deploy-bun.ts
💤 Files with no reviewable changes (2)
- packages/deploy/deploy-standalone/package.json
- packages/deploy/deploy-standalone/src/yao-pkg.d.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/standalone-bun-runtime.md
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/bun/src/start.ts
Comment on lines
238
to
+242
| func: async ({ logger, config, getInspectorState }, data) => { | ||
| const projectDir = config.rootDir | ||
| const provider = await resolveProvider(config, data?.provider) | ||
| const provider = await resolveProvider(config, data?.provider, { | ||
| runtime: data?.runtime, | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve the planned runtime for --from-plan.
deploy plan --runtime bun writes a Bun entry into the same standalone deploy directory, but deploy apply --from-plan resolves a fresh provider from the current flag/default. If --runtime bun is omitted on apply, this defaults to node and skips the Bun compile step, producing an artifact that contains a Bun server entry but is packaged as node bundle.js.
Persist/read the planned runtime, infer it from the plan output, or require an explicit matching runtime before resolving the provider for fromPlan.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/functions/commands/deploy-apply.ts` around lines 238 - 242,
`deployApply` is resolving the provider runtime from the current flags instead
of preserving the planned runtime for `--from-plan`, which can cause a Bun plan
to be applied as a Node bundle. Update the `deploy-apply.ts` flow in
`deployApply` to read or carry forward the runtime from the plan output (or
require an explicit matching runtime when `fromPlan` is set) before calling
`resolveProvider`, so the provider used for apply matches the planned standalone
artifact.
Comment on lines
+62
to
+70
| constructor(options: StandaloneProviderAdapterOptions = {}) { | ||
| this.runtime = options.runtime ?? 'node' | ||
| } | ||
|
|
||
| generateEntrySource(ctx: EntryGenerationContext): string { | ||
| if (this.runtime === 'bun') { | ||
| return this.generateBunEntrySource(ctx) | ||
| } | ||
| return this.generateNodeEntrySource(ctx) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject unsupported runtimes instead of silently selecting node.
runtime crosses a dynamic CLI/provider boundary, so values like --runtime buun can reach this constructor. Because only 'bun' branches, any typo currently builds the node entry instead of failing.
Proposed fix
constructor(options: StandaloneProviderAdapterOptions = {}) {
- this.runtime = options.runtime ?? 'node'
+ const runtime = options.runtime ?? 'node'
+ if (runtime !== 'node' && runtime !== 'bun') {
+ throw new Error(
+ `Unsupported standalone runtime: ${runtime}. Expected "node" or "bun".`
+ )
+ }
+ this.runtime = runtime
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| constructor(options: StandaloneProviderAdapterOptions = {}) { | |
| this.runtime = options.runtime ?? 'node' | |
| } | |
| generateEntrySource(ctx: EntryGenerationContext): string { | |
| if (this.runtime === 'bun') { | |
| return this.generateBunEntrySource(ctx) | |
| } | |
| return this.generateNodeEntrySource(ctx) | |
| constructor(options: StandaloneProviderAdapterOptions = {}) { | |
| const runtime = options.runtime ?? 'node' | |
| if (runtime !== 'node' && runtime !== 'bun') { | |
| throw new Error( | |
| `Unsupported standalone runtime: ${runtime}. Expected "node" or "bun".` | |
| ) | |
| } | |
| this.runtime = runtime | |
| } | |
| generateEntrySource(ctx: EntryGenerationContext): string { | |
| if (this.runtime === 'bun') { | |
| return this.generateBunEntrySource(ctx) | |
| } | |
| return this.generateNodeEntrySource(ctx) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/deploy/deploy-standalone/src/adapter.ts` around lines 62 - 70, The
StandaloneProviderAdapter constructor currently defaults any unknown runtime to
node, which hides typos from the CLI/provider boundary. Update the
StandaloneProviderAdapter initialization and related runtime handling in
adapter.ts so only supported values are accepted, and throw an explicit error
for anything other than 'node' or 'bun'. Keep the branching in
generateEntrySource and the helper methods aligned with this validation so
unsupported runtimes fail fast instead of falling through to
generateNodeEntrySource.
When the CLI itself runs under bun (e.g. the compiled brew binary), `pikku dev` now serves over @pikku/bun-server (native Bun.serve WebSockets) instead of the node http server + ws package. The bun runtime is dynamically imported and gated on `typeof Bun !== 'undefined'`, so a node-run CLI is unaffected. The dev server shares one BunEventHubService between the singleton services and the WS transport so channel broadcasts reach connected sockets. Adds the dev-bun verifier (fail-before/pass-after) and wires it into both CI verifier matrices. Also fixes bun:sqlite binding types in sqlite-runtime-bun, which @pikku/bun-server's bun types now surface in the CLI build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses CodeRabbit review on PR #784: - pikku-bun-server: match the `Upgrade: websocket` token case-insensitively (RFC 6455 §4.2.1) so a capitalized value no longer bypasses the WS upgrade branch and 500s. - deploy-standalone bun verifier: await the entry-generation checks so results settle before the final report; probe over loopback (127.0.0.1) instead of 0.0.0.0; replace shell `execSync`/`rm -rf` with `execFileSync`/`rmSync`. - Add a raw-socket handshake check (capitalized `Upgrade: WebSocket` -> 101) giving fail-before/pass-after coverage for the header fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
verifiers/dev-bun/package.json (1)
9-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare
tsxas a devDependency.The
testscript runsnpx tsx test-dev-bun.ts, buttsxis not indevDependencies.npxwill then attempt to fetch it on demand, which is non-deterministic and fails in offline/locked CI environments. Add it explicitly so the verifier runs against a pinned version.♻️ Proposed fix
"devDependencies": { "`@pikku/cli`": "workspace:*", "`@types/node`": "^24", + "tsx": "^4", "typescript": "^5.9" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@verifiers/dev-bun/package.json` around lines 9 - 15, The test script in the package.json for the dev-bun verifier depends on tsx, but it is not declared in devDependencies. Add tsx to devDependencies alongside `@pikku/cli`, `@types/node`, and typescript so the test command in the package remains deterministic and does not rely on npx downloading it at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@verifiers/dev-bun/test-dev-bun.ts`:
- Around line 39-44: The `test-dev-bun.ts` call site that runs `PIKKU_BIN` is
still using shell-based `execSync` with string interpolation, which can break on
paths containing spaces and triggers command-injection warnings. Replace that
invocation with `execFileSync` (or the same safer arg-array pattern used in
`bootDev`) so `node` is executed with `PIKKU_BIN` passed as a separate argument,
and keep the existing cwd/stdio/timeout behavior intact.
---
Nitpick comments:
In `@verifiers/dev-bun/package.json`:
- Around line 9-15: The test script in the package.json for the dev-bun verifier
depends on tsx, but it is not declared in devDependencies. Add tsx to
devDependencies alongside `@pikku/cli`, `@types/node`, and typescript so the test
command in the package remains deterministic and does not rely on npx
downloading it at runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42af8dd7-f82c-4255-a57c-981e378c2547
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.changeset/dev-bun-runtime.md.github/workflows/develop.yml.github/workflows/main.ymlpackages/cli/package.jsonpackages/cli/src/functions/commands/dev.tspackages/cli/src/functions/db/sqlite/sqlite-runtime-bun.tspackages/runtimes/bun-server/src/pikku-bun-server.tsverifiers/deploy-standalone/test-deploy-bun.tsverifiers/dev-bun/package.jsonverifiers/dev-bun/test-dev-bun.ts
✅ Files skipped from review due to trivial changes (2)
- .changeset/dev-bun-runtime.md
- packages/cli/src/functions/db/sqlite/sqlite-runtime-bun.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/main.yml
- .github/workflows/develop.yml
- packages/runtimes/bun-server/src/pikku-bun-server.ts
- Changesets: bump @pikku/cli, @pikku/bun-server, @pikku/deploy-standalone at patch (not minor) — additive/fix changes in the 0.12 line. - dev-bun verifier: replace shell execSync/rm -rf with execFileSync/rmSync, and declare tsx as a devDependency for deterministic runs (CodeRabbit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
yasserf
added a commit
that referenced
this pull request
Jun 27, 2026
Addresses CodeRabbit review on PR #784: - pikku-bun-server: match the `Upgrade: websocket` token case-insensitively (RFC 6455 §4.2.1) so a capitalized value no longer bypasses the WS upgrade branch and 500s. - deploy-standalone bun verifier: await the entry-generation checks so results settle before the final report; probe over loopback (127.0.0.1) instead of 0.0.0.0; replace shell `execSync`/`rm -rf` with `execFileSync`/`rmSync`. - Add a raw-socket handshake check (capitalized `Upgrade: WebSocket` -> 101) giving fail-before/pass-after coverage for the header fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Adds first-class Bun support. Rebuilt cleanly on top of latest
main, carrying only Bun-related changes — supersedes #662, which had accumulated unrelated changesets, docs, and a stale db-codegen snapshot already superseded in main (plus stray deletions of e2e impersonation tests).What's included
@pikku/bun-server— Bun HTTP + WebSocket runtime with health checks and graceful shutdown.@pikku/kysely-bun-sqlite— Bun-native SQLite Kysely adapter + result-coercion plugin + helper.templates/bun+verifiers/bun-server— Bun starter template and verifier.setup-bunand dist.d.tsfiles included in the build artifact for Bunfile:deps.create—--skip-installso the CI harness can patch deps before installing.scripts/test-template.sh— Bun package-manager path. Itsfile:patch now:templates/as well aspackages/(so workspace-only deps like@pikku/templates-function-addonget a local path instead of an unresolvableworkspace:*);overridesforcing every@pikku/*(including transitive^0.12.xdeps not yet on npm) to the in-repo copy — the equivalent of yarn's globallink --all;dependenciesanddevDependencies, which made bun'sfile:linker emit a broken self-referential symlink (package.json -> package.json) that broke tsc/runtime resolution.skipLibCheckandnode:test-mock shims so non-Bun verifiers and core tests toleratebun-types/ Bun'snode:test..claude/scheduled_tasks.lock.Verification
The bun-install failure (
@pikku/<pkg>@workspace:* failed to resolve) that broke every Bun template job was reproduced locally and fixed; a scaffolded Bun app now gets throughbun install→pikkucodegen →tsc. Validating the full template matrix in CI on this branch.🤖 Generated with Claude Code
Summary by CodeRabbit
--skip-install), and Bun verifiers.--runtime bunfor standalone deployments (generates a self-contained executable) and Bun-nativepikku devwhen running under Bun.test:bunscript.