feat(runtime): complete Bun-only runtime custody - #22
Conversation
- pin Bun 1.3.14 across packageManager, bun.lock, CI, and the runtime lock - author skill-a (ESM) and skill-b (CJS) with pure-JS deps proving ESM/CJS and conditional-export dependency boundaries - add a fixed Bun.build wrapper: external staging, typed dependency admission and closure-escape rejection, digest-named bundles - generate third-party notices and a bundle inventory from the frozen lock; enforce closure at package admission - preflight: restore the spike launcher and gate custody launcher rendering off until the runtime engine exists - corrections: README acquisition claim, release-impact payload paths, ADR 0006 publisher-vouched premise Cold and warm builds produce byte-identical packages (e858a19a).
Restore the R5 closure-escape contract and harden every guard the U1 review found silently passable: - Reject relative and absolute imports that resolve outside the repository with the same realpath boundary check as bare specifiers. - Require every require()/import() call site in bundle text to be a single immediately-closed string literal, closing the concatenated and member-access escape shapes. - Constrain inventory bundle paths to the digest-derived name inside plugin/runtime and pin the notices path. - Classify packages/ sources, bun.lock, and bunfig.toml as installable payload in the release-impact gate. - Broaden the CI committed-vs-rebuilt guard from hello-world.js to the whole plugin/ payload. - Await the bundle rejection assertion so six negatives cannot pass vacuously; convert a throwing Bun.build into the typed bundler-failure rejection. - Add focused negatives for missing-entry, bundler-failure, trusted-dependencies, and store-missing. - Cover every runtime lock and skill catalog validation branch with a mutation fixture per guard. - Make the Bun pin gate fail when a Bun-invoking workflow sets up Bun without a pin.
…ache (U2) One POSIX stage-zero engine owns run, repair preview, and repair --apply: - run is custody-read-only: it revalidates lock, catalog, and inventory relationships, reverifies the cached Bun executable and the selected digest-named bundle, then passes the bundle's stdio and exit status through unchanged; missing and corrupt states return typed BUN_MISSING and REPAIR_REQUIRED envelopes without mutating anything. - repair --apply is the sole acquisition path: it downloads only the lock-selected official asset with frozen timeout, redirect, and size bounds, verifies archive and executable bytes and digests before granting execute permission, probes the exact locked version, and publishes by same-filesystem atomic rename under a per-blob writer lock with dead-writer-only reclaim. - Custody results are one versioned JSON control object on stdout with typed codes, actual side effects, and one next safe action, using exit classes 20/21/22/23; stderr carries diagnostics. - Ambient control surfaces are suppressed: custody ignores caller PATH and env, and the launched bundle runs with Bun option injection, auto-install, .env loading, and telemetry disabled. - bundle-inventory.sh is a new build-owned shell projection of the bundle inventory so the engine never parses JSON; validateBundleClosure fails on a missing or stale projection. - 30 new focused tests cover the plan's negative suite: unsupported platform, missing host tools, archive and executable size/hash/version mismatches, ambiguous members, byte-cap overrun, unsafe cache roots, hostile environments, concurrent and interrupted repairs, killed-writer reclaim, live-writer refusal, tamper, denied approval, and cold offline retry.
…wner - A thin runtime-custody SKILL routes agents from a custody JSON envelope through read-only repair preview, workflow-owned human approval, repair --apply, and retry, naming runtime-exec as the behavior owner instead of restating its contract. - skill-a and skill-b SKILLs now disclose they are not yet invocable; a gate-coupled test requires the caveat while launchers do not reference runtime-exec and requires its removal when activation flips, so the docs cannot drift from the launcher gate. - scripts/prove-runtime-custody.ts is the single U2 proof owner: it runs the exhaustive custody negative suite, then proves the real payload returns one read-only BUN_MISSING envelope and repair preview against an isolated empty store with no network use. - prove:runtime-custody joins prove:all and runs in the four-target compatibility matrix so each OS family carries the concurrency and interruption proof.
Export shellQuote and compareCodeUnits from runtime-custody-config and reuse them in the bundle-inventory projection renderer instead of duplicating both; hoist the custody test file's inline node:fs requires into its static import.
Address the multi-lens review of the runtime-exec engine, including a cross-model adversarial pass: - P0: neutralize caller-cwd bunfig.toml discovery at launch by pointing Bun at an owner-only empty --config, so a top-level preload in the caller's directory can no longer execute code inside the verified runtime. The AE9 test now plants a top-level preload (the shape Bun honors) and asserts it never runs. - P1: a repair killed between the lock mkdir and its record write no longer strands every future repair. The record is published by atomic rename so a contender never reads a partial record, and a recordless lock older than a grace window is treated as a dead creator and reclaimed. - Require every cache-directory component to be a directory, returning a typed CACHE_ROOT_UNSAFE envelope instead of crashing under set -eu when XDG_CACHE_HOME points at a regular file. - Pass curl -q so a caller .curlrc cannot corrupt the JSON control object or alter custody transport. - Remove the just-published bytes when post-publish re-verification fails, so a failed apply never leaves an unverified blob at the trusted path. - Add a within-max-filesize archive-size-mismatch test so verify_archive's own size guard is exercised, and gate the honest-SKILL caveat test on the canonical launcher-render state instead of launcher text. The nextAction-bypass finding was dropped in validation: the envelope strings already say "approve", so agents get the approval signal.
GitHub Actions
Inactive
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change replaces QuickJS with a plugin-managed Bun runtime. It adds closed workspace bundles, generated launchers and inventories, verified repair flows, candidate-based packaging, and expanded release and CI validation. ChangesBun runtime distribution and custody
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SkillLauncher
participant runtime-exec
participant RuntimeCache
participant Bun
participant SkillBundle
SkillLauncher->>runtime-exec: run skill-a -- args
runtime-exec->>RuntimeCache: validate or repair locked Bun runtime
runtime-exec->>Bun: launch verified runtime
Bun->>SkillBundle: execute digest-identified bundle
SkillBundle-->>SkillLauncher: JSON output and exit status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (9)
plugin/runtime/runtime-exec (1)
438-438: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
$tool_awkfor consistency with the sanitized tool policy.Lines 41-43 state that custody resolves fixed absolute tool locations and never trusts the caller PATH.
require_apply_toolsresolvesawkintotool_awkat line 178, but line 438 calls bareawk. The call still resolves inside the sanitized directories becausePATHwas replaced at line 46, so there is no current defect. The inconsistency invites a later change that reorders PATH handling.♻️ Proposed change
- _lig_min=$(awk "BEGIN{printf \"%.6f\", $RECORDLESS_LOCK_GRACE_SECS/60}" 2>/dev/null) || return 1 + _lig_min=$("$tool_awk" "BEGIN{printf \"%.6f\", $RECORDLESS_LOCK_GRACE_SECS/60}" 2>/dev/null) || return 1🤖 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 `@plugin/runtime/runtime-exec` at line 438, Replace the bare awk invocation in the _lig_min calculation with the resolved $tool_awk variable, preserving the existing formatting, error redirection, and return-on-failure behavior.scripts/build.test.ts (1)
651-670: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffThis test mutates a checked-in artifact in the working tree.
Line 658 overwrites the real
plugin/runtime/<bundle>.js, and thefinallyblock restores it. The restore does not run if the process is terminated, for example on a test timeout, aSIGINT, or a runner crash. The repository is then left with a tampered bundle that still carries a valid inventory entry, andbun run buildoutput would differ silently until someone notices.The other tests in this file already build a temporary root through
temporaryDirectoryandcopyPluginPayload. Prefer the same approach here: copy the payload to a temporary root, tamper there, and run packaging against that root.This test also couples to the deterministic-build test at lines 477-496 through shared repository state.
🤖 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 `@scripts/build.test.ts` around lines 651 - 670, Update the test around “packaging admission fails on a stale bundle before any archive is produced” to use temporaryDirectory and copyPluginPayload, copying the plugin payload into an isolated temporary root before tampering with the bundle. Run scripts/package.ts with that temporary root as its working directory and preserve the stale-bundle assertion, eliminating mutation of checked-in artifacts and coupling to shared repository state.runtime/skill-catalog.json (1)
8-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument
entryas metadata for workspace skills.Keep
entrybecause the catalog schema and generated projection require it. State that workspace skills execute the digest-named bundle fromruntime_inventory_select_bundle, notentry.🤖 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 `@runtime/skill-catalog.json` around lines 8 - 17, Update the workspace skill catalog documentation around the skill-a and skill-b entries to describe entry as required metadata for the catalog schema and generated projection. Explicitly state that workspace skills execute the digest-named bundle selected by runtime_inventory_select_bundle, not the entry path.docs/adr/0005-shared-runtime-custody.md (1)
36-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the ADR names with the shipped artifacts.
Line 40 names the registry
catalog.json. The repository shipsruntime/skill-catalog.json. Line 56 documents the public interface asrun,doctor [profile], andrepair [profile]. The engine in this PR implementsrun,repairpreview, andrepair --apply, with nodoctorcommand.If the ADR records the original decision verbatim, add a short note that productionization renamed the registry and replaced
doctorwith read-onlyrun. Readers otherwise treat this ADR as the current custody contract.🤖 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 `@docs/adr/0005-shared-runtime-custody.md` around lines 36 - 57, Update the ADR’s artifact names and public interface to match production: reference runtime/skill-catalog.json instead of catalog.json, and document run, repair preview, and repair --apply rather than doctor. Add a brief note distinguishing the original decision from the productionized custody contract, including that doctor was replaced by read-only run.scripts/build.ts (3)
121-186: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueState the limits of the text-based closure scan.
collectModuleSpecifiersandvalidateBundleTextscan bundle text with regular expressions. They do not parse the bundle. Two consequences follow.String literals and comments produce false rejections. The pattern at line 123 matches any
fromfollowed by a quoted string, so bundled prose such as"ported from \"lodash\""raisesbare-specifier. Therequire(scan at line 167 behaves the same way. Both fail closed, so they block a build rather than admit an escape.Indirect loads are not detected.
eval,Function, andcreateRequirereach outside the artifact and match none of these patterns. The doc comment at line 142 says the check rejects text that "still reaches outside the closed artifact at runtime", which claims more than the scan proves.Narrow the doc comment to what the scan covers. If you want a real closure proof, parse the bundle and inspect import and require call sites on the AST instead.
🤖 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 `@scripts/build.ts` around lines 121 - 186, The doc comment for validateBundleText overstates the guarantee provided by the regex-based scan. Narrow it to validating detectable static import/require specifiers and computed import/require call shapes in bundle text, explicitly avoiding a claim that all runtime escapes are detected; leave collectModuleSpecifiers and the validation logic unchanged.
213-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBound the entrypoint to the workspace and type the manifest read.
Two gaps sit ahead of the resolution hooks.
Line 215 reads the workspace
package.jsonwithout a guard. If the catalog names a workspace directory that does not exist, this throws a rawENOENT. The JSDoc at line 200 states that this function throwsBundleValidationErroron failure.Line 216 builds
entryPointfromworkspaceManifest.mainand checks only that the file exists. Amainvalue such as../other-package/index.jspasses. Every import is bounded torealRootby the hooks below, but the entrypoint itself is not bounded.Both inputs are review-owned, so this is hardening rather than an exploit. The check is cheap and matches the bound the rest of the function enforces.
🛡️ Proposed guard for the manifest read and the entrypoint bound
const realRoot = realpathSync(repositoryRoot) const workspaceRoot = join(realRoot, workspace) - const workspaceManifest = JSON.parse(readFileSync(join(workspaceRoot, "package.json"), "utf8")) + const manifestPath = join(workspaceRoot, "package.json") + if (!existsSync(manifestPath)) { + throw new BundleValidationError( + skillId, + "missing-entry", + `workspace ${workspace} has no package.json`, + ) + } + const workspaceManifest = JSON.parse(readFileSync(manifestPath, "utf8")) const entryPoint = join(workspaceRoot, String(workspaceManifest.main ?? "")) if (!workspaceManifest.main || !existsSync(entryPoint)) { throw new BundleValidationError( skillId, "missing-entry", `workspace ${workspace} does not declare an existing "main" entry`, ) } + if (!isInsideDirectory(realpathSync(entryPoint), realpathSync(workspaceRoot))) { + throw new BundleValidationError( + skillId, + "parent-resolution", + `workspace ${workspace} "main" resolves outside the workspace`, + ) + }🤖 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 `@scripts/build.ts` around lines 213 - 223, Update the manifest-loading flow in the build function to catch missing or unreadable workspace package.json files and rethrow them as BundleValidationError, preserving the documented failure type. Type the parsed workspace manifest instead of leaving JSON data untyped, and validate the resolved entryPoint remains within workspaceRoot before accepting it, while retaining the existing main declaration and existence checks.
535-555: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueQuote the generated
casepattern.Lines 541-543 pass every value through
shellQuote. Line 540 interpolatesskillIdinto thecasepattern unquoted. A skill id containing*,?, or[therefore becomes a glob pattern and can match a different skill id.The custody engine verifies the bundle digest after selection, so a wrong match fails closed rather than running the wrong bundle.
managedBundlePatternat line 19 also constrains the shape of a bundle filename. This is consistency hardening.Apply
shellQuoteto the pattern so all four interpolations follow the same rule.🔒️ Proposed fix for the case pattern
.map((skillId) => { const record = bundles[skillId] - return ` ${skillId}) + return ` ${shellQuote(skillId)}) RUNTIME_BUNDLE_PATH=${shellQuote(record.path)}Regenerate the checked-in
plugin/runtime/bundle-inventory.shafter this change, becausevalidateBundleClosurebyte-compares it at line 718.🤖 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 `@scripts/build.ts` around lines 535 - 555, Update renderBundleInventoryProjection to pass skillId through shellQuote when interpolating the generated case pattern, matching the quoting used for path, bytes, and sha256. Then regenerate the checked-in bundle-inventory.sh so it remains byte-identical to validateBundleClosure’s generated output.scripts/runtime-custody-config.ts (1)
225-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse one loader for lock and catalog.
renderRuntimeCustodyFilesrepeats the load-and-validate sequence already implemented inloadSkillCatalog. Extract a privateloadCustodySources(root)that returns{ lock, catalog }after validation, then call it from both functions. This keeps a single validation order and prevents drift when a new source file joins the contract.♻️ Proposed refactor
+function loadCustodySources(root: string): { lock: RuntimeLock; catalog: SkillCatalog } { + const lock = loadJson<RuntimeLock>(join(root, "runtime", "runtime.lock.json")) + const catalog = loadJson<SkillCatalog>(join(root, "runtime", "skill-catalog.json")) + validateRuntimeLock(lock) + validateSkillCatalog(catalog, lock) + return { lock, catalog } +}export function renderRuntimeCustodyFiles(root: string): GeneratedFile[] { - const lock = loadJson<RuntimeLock>(join(root, "runtime", "runtime.lock.json")) - const catalog = loadJson<SkillCatalog>(join(root, "runtime", "skill-catalog.json")) - validateRuntimeLock(lock) - validateSkillCatalog(catalog, lock) + const { lock, catalog } = loadCustodySources(root)🤖 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 `@scripts/runtime-custody-config.ts` around lines 225 - 229, Extract a private loadCustodySources(root) helper that loads the runtime lock and skill catalog, validates them in the existing order, and returns both values. Update renderRuntimeCustodyFiles and loadSkillCatalog to use this helper, removing their duplicated loading and validation logic while preserving their current behavior.scripts/runtime-custody-generation.test.ts (1)
108-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKey the launcher assertion on the same gate value.
The SKILL.md check below derives
launcherGateActivefromrenderRuntimeCustodyFiles. This assertion instead hardcodes the inactive expectation. When the gate flips to active, this test fails for the right reason but requires a separate manual edit. Derive both expectations from the same value so activation is a one-line change in the generator.♻️ Proposed refactor
+ const { renderRuntimeCustodyFiles } = await import("./runtime-custody-config") + const generated = renderRuntimeCustodyFiles(new URL("..", import.meta.url).pathname) + const launcherGateActive = generated.some((file) => file.path.startsWith("plugin/bin/")) const launcher = await Bun.file(new URL("../plugin/bin/hello-world", import.meta.url)).text() - expect(launcher).not.toContain("runtime-exec") + expect(`hello-world targets runtime-exec ${launcher.includes("runtime-exec")}`).toBe( + `hello-world targets runtime-exec ${launcherGateActive}`, + )Then remove the duplicate
renderRuntimeCustodyFilesimport andlauncherGateActivecomputation at Lines 119-121.🤖 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 `@scripts/runtime-custody-generation.test.ts` around lines 108 - 111, Update the launcher assertion in the runtime custody generation test to derive its expected runtime-exec presence from the existing renderRuntimeCustodyFiles gate, reusing the existing launcherGateActive value from the SKILL.md check. Remove the duplicate renderRuntimeCustodyFiles import and launcherGateActive computation, and preserve the assertion’s inactive behavior.
🤖 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 `@plugin/runtime/runtime-exec`:
- Around line 806-813: Update plugin/runtime/runtime-exec lines 806-813 to
truncate and recreate empty-bunfig.toml unconditionally before launch, rather
than reusing an existing regular file. At plugin/runtime/runtime-exec line 831,
verify whether --config suppresses both global bunfig locations; if not, unset
XDG_CONFIG_HOME or otherwise neutralize those global configuration inputs before
launching the verified runtime.
- Around line 94-101: Validate RUNTIME_LOCK_VERSION in select_locked_asset
alongside the other projected lock fields, rejecting values containing
characters that would break the JSON envelope, particularly double quotes or
backslashes. Keep runtime_extra unchanged and preserve the existing typed
failure behavior used by the other lock-field checks.
- Around line 531-547: Restrict validate_asset_url to official https URLs by
default, and allow file:// URLs only when an explicit test-only opt-in variable
is enabled. Update the related download_archive protocol selection and
validation flow so production locks cannot use local files, while preserving
file-based fixture acquisition for tests.
In `@plugin/skills/skill-a/SKILL.md`:
- Around line 8-12: Replace the direct bundle-execution instructions in
plugin/skills/skill-a/SKILL.md:8-12 with an invocation of runtime-exec run
skill-a. Apply the corresponding change in plugin/skills/skill-b/SKILL.md:8-12
using runtime-exec run skill-b, preserving the instruction to report the JSON
result.
In `@scripts/build.ts`:
- Around line 430-445: The admission scan in the lockfile loop should process
only packages reachable through the workspace skill bundle graph, reusing that
graph’s known package set instead of iterating every lock entry. In the same
loop, validate that each package identity contains an “@” separator before
slicing; otherwise throw the established typed DependencyAdmissionError, while
preserving the existing workspace filtering and sorting behavior.
- Around line 400-407: The readLicenseText function selects license files from
unsorted filesystem entries, making the chosen text non-deterministic. Sort the
readdirSync(directory) results with the existing compareCodeUnits helper before
applying the license filename filter, while preserving the current matching and
read behavior.
- Around line 776-797: Update the install result handling around Bun.spawnSync
so the build continues only when install.exitCode === 0; when it is null or
otherwise nonzero, report the terminating signal when available and exit
nonzero. In the final output around buildWorkspaceBundles and
validateBundleClosure, remove the standalone helloWorldPath console.log and
include helloWorldPath as a field in the single JSON object, after confirming no
caller relies on the separate first stdout line.
- Around line 364-374: Update parseFrozenLock to parse bun.lock with a
string-aware JSONC parser that supports comments and trailing commas without
modifying comma sequences inside quoted values. Catch parser failures and
rethrow them as a suitable DependencyAdmissionError while preserving the
existing missing-lock error; add coverage for comments, trailing commas, and
quoted comma sequences.
- Around line 421-428: Update admitDependencyClosure to enumerate the root
manifest’s workspaces and read each workspace package.json, rejecting any
manifest that declares trustedDependencies with DependencyAdmissionError before
dependency admission proceeds. Preserve the existing root-manifest validation
and apply the same rejection to every resolved workspace member.
- Around line 241-279: Keep the onResolve handler focused on allowed specifiers
and unresolved-import violations, removing its Bun.resolveSync-based
native-addon and repository-containment checks. Add equivalent validation in an
onLoad handler using args.path, which is the bundler’s actual resolved path, and
preserve the existing native-addon, parent-resolution, and external/violation
behavior there.
In `@scripts/generate.ts`:
- Around line 36-44: Compute the drift list only when check is enabled: update
the flow around checkGeneratedFiles and checkRuntimeCustodyFiles so generation
mode skips both calls and proceeds to write missing files, while check mode
retains the existing drift validation and failure behavior.
In `@scripts/runtime-custody-config.ts`:
- Around line 62-64: Update loadJson and the profile/skill shape validation
around the visible Object.keys checks so missing or non-object profiles and
catalog.skills values are rejected by the existing closed-contract validation
rather than causing a TypeError. Add an object guard before key inspection and
preserve the typed contract error message for both runtime.lock.json profiles
and catalog.skills.
In `@scripts/runtime-custody-exec.test.ts`:
- Around line 970-977: Update the credential-bearing URL test around runEngine
to also assert that apply.stderr does not contain the secret value, alongside
the existing stdout assertion, ensuring rejected credentials are absent from
both output streams.
- Around line 867-874: Update the wrong-archive-hash test to assert that
fixture.blobPath does not exist instead of constructing a path from the
overridden archiveSha256. Keep the existing exit-code, envelope-code, and
staging-directory assertions unchanged.
---
Nitpick comments:
In `@docs/adr/0005-shared-runtime-custody.md`:
- Around line 36-57: Update the ADR’s artifact names and public interface to
match production: reference runtime/skill-catalog.json instead of catalog.json,
and document run, repair preview, and repair --apply rather than doctor. Add a
brief note distinguishing the original decision from the productionized custody
contract, including that doctor was replaced by read-only run.
In `@plugin/runtime/runtime-exec`:
- Line 438: Replace the bare awk invocation in the _lig_min calculation with the
resolved $tool_awk variable, preserving the existing formatting, error
redirection, and return-on-failure behavior.
In `@runtime/skill-catalog.json`:
- Around line 8-17: Update the workspace skill catalog documentation around the
skill-a and skill-b entries to describe entry as required metadata for the
catalog schema and generated projection. Explicitly state that workspace skills
execute the digest-named bundle selected by runtime_inventory_select_bundle, not
the entry path.
In `@scripts/build.test.ts`:
- Around line 651-670: Update the test around “packaging admission fails on a
stale bundle before any archive is produced” to use temporaryDirectory and
copyPluginPayload, copying the plugin payload into an isolated temporary root
before tampering with the bundle. Run scripts/package.ts with that temporary
root as its working directory and preserve the stale-bundle assertion,
eliminating mutation of checked-in artifacts and coupling to shared repository
state.
In `@scripts/build.ts`:
- Around line 121-186: The doc comment for validateBundleText overstates the
guarantee provided by the regex-based scan. Narrow it to validating detectable
static import/require specifiers and computed import/require call shapes in
bundle text, explicitly avoiding a claim that all runtime escapes are detected;
leave collectModuleSpecifiers and the validation logic unchanged.
- Around line 213-223: Update the manifest-loading flow in the build function to
catch missing or unreadable workspace package.json files and rethrow them as
BundleValidationError, preserving the documented failure type. Type the parsed
workspace manifest instead of leaving JSON data untyped, and validate the
resolved entryPoint remains within workspaceRoot before accepting it, while
retaining the existing main declaration and existence checks.
- Around line 535-555: Update renderBundleInventoryProjection to pass skillId
through shellQuote when interpolating the generated case pattern, matching the
quoting used for path, bytes, and sha256. Then regenerate the checked-in
bundle-inventory.sh so it remains byte-identical to validateBundleClosure’s
generated output.
In `@scripts/runtime-custody-config.ts`:
- Around line 225-229: Extract a private loadCustodySources(root) helper that
loads the runtime lock and skill catalog, validates them in the existing order,
and returns both values. Update renderRuntimeCustodyFiles and loadSkillCatalog
to use this helper, removing their duplicated loading and validation logic while
preserving their current behavior.
In `@scripts/runtime-custody-generation.test.ts`:
- Around line 108-111: Update the launcher assertion in the runtime custody
generation test to derive its expected runtime-exec presence from the existing
renderRuntimeCustodyFiles gate, reusing the existing launcherGateActive value
from the SKILL.md check. Remove the duplicate renderRuntimeCustodyFiles import
and launcherGateActive computation, and preserve the assertion’s inactive
behavior.
🪄 Autofix
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 Plus
Run ID: 4328bf3e-958c-46a3-afcb-414e7c3bcb50
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (36)
.github/workflows/plugin-ci.ymlREADME.mdbunfig.tomldocs/adr/0004-category-3-runtime-distribution.mddocs/adr/0005-shared-runtime-custody.mddocs/adr/0006-single-bun-runtime-tier.mddocs/adr/0007-workspace-authoring-bundled-distribution.mdpackage.jsonpackages/skill-a/package.jsonpackages/skill-a/src/main.jspackages/skill-b/package.jsonpackages/skill-b/src/main.cjsplugin/THIRD-PARTY-NOTICES.mdplugin/runtime/bundle-inventory.jsonplugin/runtime/bundle-inventory.shplugin/runtime/runtime-execplugin/runtime/runtime-lock.shplugin/runtime/skill-a-27cb243179e5c93d.jsplugin/runtime/skill-b-535431fb5dd1ed0a.jsplugin/runtime/skill-catalog.shplugin/skills/runtime-custody/SKILL.mdplugin/skills/skill-a/SKILL.mdplugin/skills/skill-b/SKILL.mdruntime/runtime.lock.jsonruntime/skill-catalog.jsonscripts/build.test.tsscripts/build.tsscripts/generate.tsscripts/package.tsscripts/prove-runtime-custody.tsscripts/release-impact.test.tsscripts/release-impact.tsscripts/runtime-custody-config.test.tsscripts/runtime-custody-config.tsscripts/runtime-custody-exec.test.tsscripts/runtime-custody-generation.test.ts
Resolve four P1s and three validated P2s from the independent code review of the shared Bun runtime custody feature (U1+U2). runtime-exec: - Use an integer -mmin grace check for recordless lock reclaim. The fractional argument was rejected by BSD/macOS find, silently defeating reclaim and stranding every future repair in LOCK_HELD after a writer died between mkdir and its record publish. Drops the bare awk dependency. - Stage the launch-time empty bunfig in a per-process temp dir instead of the custody store, so run no longer mutates custody state and a valid blob on a read-only cache still launches. - Guard the do_apply store mkdir with a typed CACHE_ROOT_UNSAFE envelope; a non-writable owned cache previously died under set -eu with no control object, breaking the one-envelope contract. - Restore the caller umask before exec so a launched skill's files are not forced to the custody 077 mask. - Make stale-lock reclaim atomic (rename-then-remove) so two contenders cannot both reclaim and delete a live writer's fresh lock. - Name the human approver in every approval-gated nextAction so an agent following only the JSON channel cannot self-approve repair --apply. build.ts: - Reject a workspace "main" that resolves outside the workspace; the closed-resolution plugin bounds imported modules but never the entrypoint itself. - Replace three inline comparators with the already-imported compareCodeUnits. Tests: add recordless-grace reclaim (fresh live / aged reclaimed), read-only-cache launch, caller-umask restore, and entrypoint-escape regression coverage.
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
scripts/build.ts (1)
803-813: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unreachable
runtime/${skillId}-MISSING.jssentinel.
validateBundleClosure(root)runs at Line 786. It already throwsbundle closure: missing bundle mapping for ${skillId}wheninventory.bundles[skillId]is absent. The fallback at Line 808 is therefore unreachable, and a synthetic path inrequiredwould report a misleadingmissing runtime/<id>-MISSING.jserror if it ever executed. Read the record directly and let the earlier closure check own the missing-mapping failure.♻️ Proposed simplification
for (const [skillId, skill] of Object.entries(catalog.skills)) { required.push(`bin/${skillId}`, `skills/${skillId}/SKILL.md`) - required.push( - skill.workspace === undefined - ? skill.entry - : bundleInventory.bundles[skillId]?.path ?? `runtime/${skillId}-MISSING.js`, - ) + const record = bundleInventory.bundles[skillId] + if (record === undefined) { + throw new Error(`Bun payload closure: missing bundle mapping for ${skillId}`) + } + required.push(skill.workspace === undefined ? skill.entry : record.path) }🤖 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 `@scripts/build.ts` around lines 803 - 813, Remove the `runtime/${skillId}-MISSING.js` fallback in the required-path construction within the `catalog.skills` loop. After the workspace check, read `bundleInventory.bundles[skillId].path` directly so `validateBundleClosure` remains responsible for reporting missing bundle mappings.scripts/runtime-custody-generation.test.ts (1)
119-119: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the launcher count from the catalog.
Line 119 hard-codes
3. The loop above it already iteratesObject.keys(catalog.skills). When a fourth skill is added, this assertion fails with a count mismatch that does not name the missing launcher. Compare against the catalog size.♻️ Proposed change
- expect(generated.filter((file) => file.path.startsWith("plugin/bin/")).length).toBe(3) + expect(generated.filter((file) => file.path.startsWith("plugin/bin/")).map((file) => file.path)).toEqual( + Object.keys(catalog.skills) + .sort() + .map((skillId) => `plugin/bin/${skillId}`), + )🤖 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 `@scripts/runtime-custody-generation.test.ts` at line 119, Update the launcher-count assertion in the runtime custody generation test to compare against the size of the skills catalog, reusing the catalog referenced by the preceding Object.keys(catalog.skills) iteration instead of hard-coding 3. Keep the existing plugin/bin filtering behavior 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/workflows/release.yml:
- Line 352: Update the release workflow step around prove:runtime-custody to
retain that custody test and additionally run prove:runtime-platform for the
current matrix target, passing the target value through the workflow’s existing
matrix context so the locked Bun asset is acquired and executed per target.
In `@plugin/bin/skill-a`:
- Around line 1-5: Update plugin/bin/skill-a lines 1-5 and plugin/bin/skill-b
lines 1-5 so these launchers are excluded from agent discovery, rather than
merely relying on defaultEnabled: false. Apply the exclusion consistently to
both skill entry points while preserving their existing runtime execution
behavior.
In `@scripts/harness-install-codex.ts`:
- Around line 247-251: Update the activation metadata produced by
proveCodexFixtureCopy so fixture-copy mode does not claim unverified plugin or
lifecycle-hook facts. Mark activation as unverified, or omit the activation
object entirely, while preserving the existing verified activation output for
native installation paths.
In `@scripts/prove-distribution.ts`:
- Around line 151-166: Resolve the U3 scope conflict by removing the activation
assertions in the loop over skill IDs, including the launcher runtime-exec check
and active packaged invocation checks, and remove the corresponding Bun
required-runtime metadata. Keep U3 inactive unless the approved PR scope and
release plan are explicitly updated to include it.
In `@scripts/prove-harness-install.ts`:
- Around line 1080-1084: Update the payloadHash loop to length-prefix both each
relativePath and its corresponding file contents before hashing them, using
unambiguous encoded byte lengths and preserving the existing file iteration
order.
In `@scripts/runtime-custody-exec.test.ts`:
- Around line 786-805: Update the test “run restores the caller umask for the
launched skill” to keep asserting that the launched skill’s umask equals
callerUmask, and remove the additional assertion requiring it not to equal
“0077”.
In `@scripts/runtime-custody-generation.test.ts`:
- Line 110: Replace the URL.pathname conversion in the renderRuntimeCustodyFiles
call with fileURLToPath, importing it from the appropriate Node URL module. Pass
the converted filesystem path so spaces, non-ASCII characters, and Windows drive
paths are handled correctly.
---
Nitpick comments:
In `@scripts/build.ts`:
- Around line 803-813: Remove the `runtime/${skillId}-MISSING.js` fallback in
the required-path construction within the `catalog.skills` loop. After the
workspace check, read `bundleInventory.bundles[skillId].path` directly so
`validateBundleClosure` remains responsible for reporting missing bundle
mappings.
In `@scripts/runtime-custody-generation.test.ts`:
- Line 119: Update the launcher-count assertion in the runtime custody
generation test to compare against the size of the skills catalog, reusing the
catalog referenced by the preceding Object.keys(catalog.skills) iteration
instead of hard-coding 3. Keep the existing plugin/bin filtering behavior
unchanged.
🪄 Autofix
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 Plus
Run ID: e7e42e85-5552-49a0-8281-10bf0d6121af
📒 Files selected for processing (60)
.claude-plugin/marketplace.json.github/release-please-config.json.github/workflows/plugin-ci.yml.github/workflows/release.ymlREADME.mddocs/adr/0002-bun-authoring-quickjs-runtime.mddocs/adr/0005-shared-runtime-custody.mddocs/adr/0006-single-bun-runtime-tier.mddocs/adr/0007-workspace-authoring-bundled-distribution.mdpackage.jsonplugin.config.jsonplugin/.claude-plugin/plugin.jsonplugin/.codex-plugin/plugin.jsonplugin/QUICKJS-LICENSEplugin/bin/hello-worldplugin/bin/skill-aplugin/bin/skill-bplugin/hooks/claude/hooks.jsonplugin/hooks/codex/hooks.jsonplugin/runtime/bundle-inventory.jsonplugin/runtime/bundle-inventory.shplugin/runtime/hello-world.jsplugin/runtime/qjs-darwin-arm64plugin/runtime/qjs-darwin-x86_64plugin/runtime/qjs-linux-aarch64plugin/runtime/qjs-linux-x86_64plugin/runtime/quickjs-assets.jsonplugin/runtime/runtime-execplugin/skills/hello-world/SKILL.mdplugin/skills/skill-a/SKILL.mdplugin/skills/skill-b/SKILL.mdruntime/src/bun-proof-adapter.tsruntime/src/portable-command.test.tsruntime/src/portable-command.tsruntime/src/quickjs-adapter.tsscripts/build.test.tsscripts/build.tsscripts/dev.tsscripts/harness-install-codex.test.tsscripts/harness-install-codex.tsscripts/init.test.tsscripts/init.tsscripts/package.tsscripts/plugin-config.tsscripts/plugin-manifest-contract.test.tsscripts/prove-distribution.tsscripts/prove-dx.tsscripts/prove-harness-install.test.tsscripts/prove-harness-install.tsscripts/prove-quickjs-ci.tsscripts/quickjs-spike.tsscripts/release-impact.test.tsscripts/release-projection.test.tsscripts/release-projection.tsscripts/release-validate.test.tsscripts/release-validate.tsscripts/runtime-custody-config.test.tsscripts/runtime-custody-config.tsscripts/runtime-custody-exec.test.tsscripts/runtime-custody-generation.test.ts
💤 Files with no reviewable changes (13)
- plugin/QUICKJS-LICENSE
- scripts/dev.ts
- plugin/hooks/claude/hooks.json
- scripts/quickjs-spike.ts
- scripts/release-validate.test.ts
- plugin/runtime/quickjs-assets.json
- scripts/prove-quickjs-ci.ts
- .github/workflows/plugin-ci.yml
- scripts/plugin-config.ts
- plugin/hooks/codex/hooks.json
- scripts/release-projection.ts
- runtime/src/quickjs-adapter.ts
- scripts/init.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- plugin/skills/skill-b/SKILL.md
- plugin/skills/skill-a/SKILL.md
- plugin/runtime/bundle-inventory.json
- docs/adr/0007-workspace-authoring-bundled-distribution.md
- docs/adr/0006-single-bun-runtime-tier.md
- scripts/package.ts
- plugin/runtime/runtime-exec
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
scripts/prove-harness-install.ts (2)
1155-1171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one control-envelope reader between the two proofs.
requireRuntimeControlduplicatesrequireEnvelopeinscripts/prove-runtime-platform.tslines 104-120, including theschemaVersion !== 1check, the single-line stdout rule, and the error text shape. The twoRuntimeControlEnvelopeandControlEnvelopeinterfaces also overlap. If the envelope schema version increments, both copies must change.Extract the type and the reader into a shared module and import it in both proofs.
🤖 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 `@scripts/prove-harness-install.ts` around lines 1155 - 1171, Extract the shared control-envelope type and reader logic from requireRuntimeControl and requireEnvelope into a common module, preserving the schemaVersion check, single-line stdout validation, exit-code handling, and existing error-message shape. Update both proofs to import and use the shared type and reader, removing their duplicated interfaces and local implementations.
1243-1247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResolve the target before the journey runs.
nativeRuntimeTarget()throws on an unsupported platform or architecture. Line 1246 calls it only after the full repair and retry journey mutated the isolated cache. On an unsupported host the proof performs all the work and then fails on a precondition.Call
nativeRuntimeTarget()at the start ofproveNativeRuntimeJourneyand reuse the value.🤖 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 `@scripts/prove-harness-install.ts` around lines 1243 - 1247, Update proveNativeRuntimeJourney to call nativeRuntimeTarget() before any repair or retry journey work, store the resolved target, and reuse that value in the returned target field instead of invoking nativeRuntimeTarget() after the journey.scripts/prove-harness-install.test.ts (1)
89-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the acknowledgement test independent of installed native CLIs.
proveHarnessInstallchecksrequireNativebefore it checksfixtureAcknowledged. On a host withoutclaudeorcodex, this test throwsnative harness CLIs are required; missing: ...and fails with a misleading message, even though the acknowledgement guard is the subject.
qualifyRuntimeJourney: truealone reaches the guard atscripts/prove-harness-install.tsline 1406. DroprequireNative.♻️ Proposed change
expect(() => proveHarnessInstall(root, { - requireNative: true, qualifyRuntimeJourney: true, }), ).toThrow("requires --fixture-acknowledged")🤖 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 `@scripts/prove-harness-install.test.ts` around lines 89 - 95, Update the test “native runtime qualification requires explicit fixture acknowledgement” to remove requireNative: true from the proveHarnessInstall options, leaving qualifyRuntimeJourney: true so the test reaches and validates the fixtureAcknowledged guard independently of installed native CLIs.scripts/prove-runtime-platform.test.ts (1)
55-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPut the expected job name in the test table.
Line 60 derives the job name with
"name: Build candidate once".replace("candidate", ...)and apath.includes("release")branch.String.prototype.replacewith a string pattern replaces only the first match, so the result is correct, but the intent is hard to read and the branch re-derives information the table already carries.Add the job name as a fourth tuple element.
♻️ Proposed refactor
test.each([ - ["plugin CI", ".github/workflows/plugin-ci.yml", "runtime-candidate-${{ github.sha }}"], - ["release", ".github/workflows/release.yml", "release-platform-candidate-${{ github.run_id }}"], -] as const)("%s builds once and proves the same candidate on every target", (_name, path, artifact) => { + [ + "plugin CI", + ".github/workflows/plugin-ci.yml", + "runtime-candidate-${{ github.sha }}", + "name: Build candidate once", + ], + [ + "release", + ".github/workflows/release.yml", + "release-platform-candidate-${{ github.run_id }}", + "name: Build release candidate once", + ], +] as const)("%s builds once and proves the same candidate on every target", (_name, path, artifact, jobName) => { const workflow = readFileSync(resolve(root, path), "utf8") - expect(workflow).toContain("name: Build candidate once".replace("candidate", path.includes("release") ? "release candidate" : "candidate")) + expect(workflow).toContain(jobName) expect(workflow).toContain(artifact)🤖 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 `@scripts/prove-runtime-platform.test.ts` around lines 55 - 61, Add the expected workflow job name as a fourth element in each tuple passed to the test.each table, then destructure it in the test callback and assert workflow content directly against that value. Remove the path.includes("release") branch and the derived String.replace expression while preserving the existing artifact assertions.
🤖 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/plugin-ci.yml:
- Around line 19-31: Update the checkout step in the candidate job to set
persist-credentials to false in its with configuration, while preserving the
existing pinned checkout action and other workflow settings.
In @.github/workflows/release.yml:
- Around line 328-336: Update the candidate job to declare least-privilege
permissions matching compatibility and package, specifically actions: read and
contents: read. Configure its actions/checkout step to avoid persisting
credentials in .git/config before bun run package executes.
In `@scripts/prove-runtime-platform.ts`:
- Around line 196-204: Update the runLauncher setup used by the cold-run
assertion so HOME points to a sibling directory within isolationRoot, while
XDG_CACHE_HOME continues pointing to cacheRoot. Keep the readdirSync(cacheRoot)
emptiness check focused solely on the isolated cache and preserve the existing
sideEffects assertion.
- Around line 78-102: Update regularFiles() so its returned paths use the same
global name comparator as pluginPayloadInventory(), rather than relying on
depth-first traversal order. Ensure payloadDigest() hashes runtime.lock.json and
nested runtime paths in canonical inventory order, or reuse
pluginPayloadInventory() directly.
---
Nitpick comments:
In `@scripts/prove-harness-install.test.ts`:
- Around line 89-95: Update the test “native runtime qualification requires
explicit fixture acknowledgement” to remove requireNative: true from the
proveHarnessInstall options, leaving qualifyRuntimeJourney: true so the test
reaches and validates the fixtureAcknowledged guard independently of installed
native CLIs.
In `@scripts/prove-harness-install.ts`:
- Around line 1155-1171: Extract the shared control-envelope type and reader
logic from requireRuntimeControl and requireEnvelope into a common module,
preserving the schemaVersion check, single-line stdout validation, exit-code
handling, and existing error-message shape. Update both proofs to import and use
the shared type and reader, removing their duplicated interfaces and local
implementations.
- Around line 1243-1247: Update proveNativeRuntimeJourney to call
nativeRuntimeTarget() before any repair or retry journey work, store the
resolved target, and reuse that value in the returned target field instead of
invoking nativeRuntimeTarget() after the journey.
In `@scripts/prove-runtime-platform.test.ts`:
- Around line 55-61: Add the expected workflow job name as a fourth element in
each tuple passed to the test.each table, then destructure it in the test
callback and assert workflow content directly against that value. Remove the
path.includes("release") branch and the derived String.replace expression while
preserving the existing artifact assertions.
🪄 Autofix
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 Plus
Run ID: 44348c91-8741-4a39-86f6-6264af1239ce
📒 Files selected for processing (13)
.github/workflows/plugin-ci.yml.github/workflows/release.ymlREADME.mdpackage.jsonscripts/init.test.tsscripts/package.tsscripts/prove-distribution.tsscripts/prove-harness-install.test.tsscripts/prove-harness-install.tsscripts/prove-runtime-platform.test.tsscripts/prove-runtime-platform.tsscripts/release-validate.test.tsscripts/release-validate.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- scripts/release-validate.test.ts
- package.json
- scripts/init.test.ts
- README.md
- scripts/package.ts
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@plugin/runtime/runtime-lock.sh`:
- Around line 17-24: Before finalizing the Bun v1.3.14 asset entries in the
runtime lock configuration, validate the Linux and Darwin baseline archives and
custody bundle on representative no-AVX/no-AVX2 and Ivy Bridge hosts. If
compatibility is required, replace the pinned release and corresponding
checksums and sizes with assets that pass; otherwise document the unsupported
CPUs and enforce their rejection.
In `@scripts/prove-harness-install.test.ts`:
- Around line 49-55: Update the git commit command in the fixture setup loop to
include the repository-local configuration override -c commit.gpgSign=false
before the commit action, while leaving the git init and git add commands
unchanged.
In `@scripts/runtime-custody-config.ts`:
- Around line 72-73: Validate profile.assets before the Object.keys comparison,
rejecting absent, null, or otherwise non-object values with the existing
runtime-lock validation error; only inspect and compare its keys after this
validation succeeds.
🪄 Autofix
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 Plus
Run ID: 4f460359-71d7-452d-b995-4e9e0951a984
📒 Files selected for processing (17)
.fallowrc.jsonREADME.mdpackage.jsonplugin/bin/hello-worldplugin/bin/skill-aplugin/bin/skill-bplugin/runtime/runtime-execplugin/runtime/runtime-lock.shruntime/runtime.lock.jsonscripts/build.test.tsscripts/build.tsscripts/prove-harness-install.test.tsscripts/prove-harness-install.tsscripts/prove-runtime-platform.tsscripts/runtime-custody-config.test.tsscripts/runtime-custody-config.tsscripts/runtime-custody-exec.test.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- package.json
- plugin/bin/hello-world
- plugin/bin/skill-b
- README.md
- runtime/runtime.lock.json
- plugin/runtime/runtime-exec
- scripts/build.test.ts
- plugin/bin/skill-a
- scripts/prove-runtime-platform.ts
- scripts/runtime-custody-exec.test.ts
- scripts/prove-harness-install.ts
- scripts/build.ts
Harden dependency admission, runtime transport, proof framing, and candidate workflow isolation while keeping the run/repair surface unchanged.
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/prove-runtime-platform.test.ts (1)
88-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer a behavioral assertion over source-text matching.
This test asserts literal source lines of
scripts/prove-runtime-platform.ts. Any harmless rename, such ashomeRoottoisolatedHome, breaks the test without changing behavior. Export the isolation environment builder and assert thatHOMEis outsideXDG_CACHE_HOME.🤖 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 `@scripts/prove-runtime-platform.test.ts` around lines 88 - 94, Replace the source-text matching in the test around “platform proof isolates HOME” with a behavioral assertion: export the isolation environment builder from prove-runtime-platform.ts, invoke it in the test, and verify that the returned HOME path is distinct from and outside the XDG_CACHE_HOME path while preserving the cache emptiness assertion.scripts/build.test.ts (1)
421-455: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for a workspace-to-workspace dependency.
Every fixture resolves dependencies by exact
name@versionidentity. Theworkspace:traversal branch inadmitDependencyClosurenever runs, so its resolution behavior stays unproven. Add a fixture where one catalog workspace depends on a sibling workspace package, using the reference form thatbun installwrites.🤖 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 `@scripts/build.test.ts` around lines 421 - 455, Extend the dependency-admission tests around admitDependencyClosure with a fixture containing two workspace packages, where one production dependency references the sibling using Bun’s generated workspace: name@version form. Populate the corresponding lockfile entries and package-store metadata, then assert the sibling is included in the admitted closure to exercise workspace traversal.
🤖 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 `@scripts/proof-control-envelope.ts`:
- Around line 21-27: Update the envelope parsing and validation in the
proof-control helper to parse JSON as unknown, reject null or non-object values,
and validate ok, sideEffects, nextAction, and the optional runtime field before
returning. Ensure sideEffects and nextAction are string arrays as required by
ProofControlEnvelope, preserve the existing schemaVersion and code checks, and
only then return the validated envelope.
---
Nitpick comments:
In `@scripts/build.test.ts`:
- Around line 421-455: Extend the dependency-admission tests around
admitDependencyClosure with a fixture containing two workspace packages, where
one production dependency references the sibling using Bun’s generated
workspace: name@version form. Populate the corresponding lockfile entries and
package-store metadata, then assert the sibling is included in the admitted
closure to exercise workspace traversal.
In `@scripts/prove-runtime-platform.test.ts`:
- Around line 88-94: Replace the source-text matching in the test around
“platform proof isolates HOME” with a behavioral assertion: export the isolation
environment builder from prove-runtime-platform.ts, invoke it in the test, and
verify that the returned HOME path is distinct from and outside the
XDG_CACHE_HOME path while preserving the cache emptiness assertion.
🪄 Autofix
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 Plus
Run ID: 6f5a2520-b5a6-4785-abd2-d98419e10706
📒 Files selected for processing (24)
.github/workflows/plugin-ci.yml.github/workflows/release.ymlREADME.mddocs/adr/0007-workspace-authoring-bundled-distribution.mdplugin/runtime/bundle-inventory.shplugin/runtime/runtime-execscripts/build.test.tsscripts/build.tsscripts/generate.tsscripts/harness-install-codex.tsscripts/package.tsscripts/plugin-files.test.tsscripts/plugin-files.tsscripts/proof-control-envelope.tsscripts/prove-distribution.tsscripts/prove-harness-install.test.tsscripts/prove-harness-install.tsscripts/prove-runtime-platform.test.tsscripts/prove-runtime-platform.tsscripts/release-validate.test.tsscripts/runtime-custody-config.test.tsscripts/runtime-custody-config.tsscripts/runtime-custody-exec.test.tsscripts/runtime-custody-generation.test.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- scripts/generate.ts
- scripts/harness-install-codex.ts
- scripts/package.ts
- docs/adr/0007-workspace-authoring-bundled-distribution.md
- plugin/runtime/bundle-inventory.sh
- scripts/release-validate.test.ts
- README.md
- .github/workflows/release.yml
- scripts/runtime-custody-generation.test.ts
- scripts/prove-distribution.ts
- .github/workflows/plugin-ci.yml
- scripts/runtime-custody-config.ts
- scripts/prove-runtime-platform.ts
- plugin/runtime/runtime-exec
- scripts/prove-harness-install.ts
GitHub Actions
Inactive
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b726d1667
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
GitHub Actions
Inactive
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99dd9b7fae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the exact-head Codex P2 on
Exact proof: 457 general tests passed (3 declared manual release-receipt skips), 61 runtime-custody tests passed, deterministic distribution, and native Claude/Codex install mechanics. @codex review The unchanged-head gate resets again: substantive exact-head completion from both reviewers, all hosted checks terminal green, and zero credible unresolved P0/P1/P2 or actionable threads. |
GitHub Actions
Inactive
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested_sha='1847b7f728c29bf6587dcfeb5fe7cfe36cb6b1cd'
repo='myagentdojo/agent-plugin-template'
pr_number='22'
echo '== Exact checkout and requested commit =='
git rev-parse HEAD
git status --short
git cat-file -t "$requested_sha"
git show --no-ext-diff --format=fuller --no-renames --stat "$requested_sha"
echo '== Exact commit diff =='
git diff --no-ext-diff --no-renames --unified=70 "${requested_sha}^" "$requested_sha" -- \
scripts/build.ts \
scripts/build.test.ts
echo '== Source outlines =='
ast-grep outline scripts/build.ts --items all --view expanded
ast-grep outline scripts/build.test.ts --items all --view expanded
echo '== Lexer, statement-block, and loader-validation references =='
rg -n -C 8 --pcre2 \
'executableCodeMask|validateBundleText|statementBlock|statement.*block|try|catch|finally|else|do|brace|regex|import\\.meta|globalThis|require' \
scripts/build.ts scripts/build.test.ts
echo '== Relevant implementation =='
sed -n '110,560p' scripts/build.ts | cat -n
echo '== Relevant regression coverage =='
sed -n '100,490p' scripts/build.test.ts | cat -n
echo '== PR head, hosted checks, and reviews =='
gh pr view "$pr_number" --repo "$repo" \
--json headRefOid,statusCheckRollup,reviews \
--jq '{headRefOid, checks: [.statusCheckRollup[] | {name, status, conclusion, detailsUrl}], reviews: [.reviews[] | {author: .author.login, state, commit: .commit.oid}]}'
echo '== Unresolved review threads =='
gh api graphql \
-F owner='myagentdojo' \
-F name='agent-plugin-template' \
-F number="$pr_number" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
comments(first: 1) {
nodes {
author { login }
body
commit { oid }
path
line
}
}
}
}
}
}
}' \
--jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved | not) | {author: .comments.nodes[0].author.login, commit: .comments.nodes[0].commit.oid, path: .comments.nodes[0].path, line: .comments.nodes[0].line, body: .comments.nodes[0].body}]'Length of output: 188345
The statement-block update is correct in the reviewed paths. The new tests cover I found no credible unresolved P0, P1, or P2 finding. GitHub reports no unresolved review threads. The exact SHA is the current PR head. The hosted gate is not terminal. All four compatibility jobs are still in progress. ✏️ Learnings added
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1847b7f728
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/build.ts (1)
1153-1326: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the unmatched closing brace.
The extra brace makes
scripts/build.tssyntactically invalid and prevents build commands that import it.🤖 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 `@scripts/build.ts` around lines 1153 - 1326, Remove the unmatched closing brace immediately before return admitted in admitDependencyClosure, preserving the function’s intended single closing brace after the return statement so scripts/build.ts parses correctly.
🤖 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 `@scripts/build.ts`:
- Around line 282-283: Update the scanner logic around
pendingDirectStatementBlock to clear it when the current token is a colon,
preventing try/catch/do/else/finally property values from affecting following
division-expression parsing. Add a regression test covering valid input where
such a property is followed by a division expression and validateBundleText must
detect __require(target).
---
Outside diff comments:
In `@scripts/build.ts`:
- Around line 1153-1326: Remove the unmatched closing brace immediately before
return admitted in admitDependencyClosure, preserving the function’s intended
single closing brace after the return statement so scripts/build.ts parses
correctly.
🪄 Autofix
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 Plus
Run ID: 9b4c2f82-3435-468f-a224-eb51cb12564c
📒 Files selected for processing (2)
scripts/build.test.tsscripts/build.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/build.test.ts
|
Addressed the exact-head Codex P2 on
Exact proof: 458 general tests passed (3 declared manual release-receipt skips), 61 runtime-custody tests passed, deterministic distribution, and native Claude/Codex install mechanics. @codex review The unchanged-head gate resets: both reviewers substantive on this exact SHA, hosted checks terminal green, and zero credible unresolved P0/P1/P2 or actionable threads. |
GitHub Actions
Inactive
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested_sha='ffd35da6a04ab8c0f8216a4894f070605f224155'
repo='myagentdojo/agent-plugin-template'
pr_number='22'
echo '== exact checkout and requested object =='
git rev-parse HEAD
git status --short
git cat-file -t "$requested_sha"
git show --no-ext-diff --format=fuller --no-renames --stat "$requested_sha"
echo '== exact commit diff =='
git diff --no-ext-diff --no-renames --unified=70 "${requested_sha}^" "$requested_sha" -- \
scripts/build.ts \
scripts/build.test.ts
echo '== source outlines =='
ast-grep outline scripts/build.ts --items all --view expanded
ast-grep outline scripts/build.test.ts --items all --view expanded
echo '== lexer and validation references =='
rg -n -C 8 --pcre2 \
'executableCodeMask|validateBundleText|statementBoundary|statement.*boundary|label|brace|statementBlock|regex|import\\.meta|globalThis|require' \
scripts/build.ts scripts/build.test.ts
echo '== relevant implementation =='
sed -n '110,590p' scripts/build.ts | cat -n
echo '== relevant regression coverage =='
sed -n '100,540p' scripts/build.test.ts | cat -n
echo '== PR head, hosted checks, and reviews =='
gh pr view "$pr_number" --repo "$repo" \
--json headRefOid,statusCheckRollup,reviews \
--jq '{headRefOid, checks: [.statusCheckRollup[] | {name, status, conclusion, detailsUrl}], reviews: [.reviews[] | {author: .author.login, state, commit: .commit.oid}]}'
echo '== unresolved review threads =='
gh api graphql \
-F owner='myagentdojo' \
-F name='agent-plugin-template' \
-F number="$pr_number" \
-f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
comments(first: 1) {
nodes {
author { login }
body
commit { oid }
path
line
}
}
}
}
}
}
}' \
--jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved | not) | {author: .comments.nodes[0].author.login, commit: .comments.nodes[0].commit.oid, path: .comments.nodes[0].path, line: .comments.nodes[0].line, body: .comments.nodes[0].body}]'Length of output: 126272
The labeled-block update is correct in the reviewed paths. The nested object-property case remains protected. The inner brace does not become a statement block in I found no credible unresolved P0, P1, or P2 finding. GitHub reports no unresolved review threads. The exact SHA is the current PR head. The hosted gate is not terminal. ✏️ Learnings added
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffd35da6a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
myagentdojo
changed the title
nathanvale
left a comment
nathanvale
left a comment
There was a problem hiding this comment.
Submitting the existing pending review comment so current feedback can be resolved.
validateBundleText is the closed-bundle security gate; two prior
unresolved review threads found ways to smuggle dynamic code past it.
1. executableCodeMask left pendingDirectStatementBlock set after a
direct-statement keyword used as an object property key (`{ try: {}
/ __require(target) / 1 }`), masking the following division-shaped
`/ __require(target) /` as a regex literal and hiding the live
require. Clear the flag in the default-character branch.
2. The dynamic-code-generation check matched only the bare eval/Function
identifiers, so `(() => {}).constructor("p", "return import(p)")`
reached the Function constructor via `.constructor` with no match.
Extend the scan to reject a call on `.constructor` while still
allowing non-invoked `.prototype.constructor` reads.
Adds a regression test per bypass (property-key division across all
directStatementBlockKeywords; constructor-based code generation).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
|
Addressed the two exact-head scanner findings on
Exact local proof: 95/95 focused build tests, 521 full-suite passes, 3 declared manual/live receipt skips, 0 failures, generated files current, and deterministic build clean. CodeRabbit completed the exact-head incremental review with no actionable comments; all review threads are resolved. @codex review Gate: substantive Codex review of this exact unchanged SHA, hosted checks terminal green, and zero credible unresolved P0/P1/P2 or actionable threads. A new push resets the gate. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f0c9503c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
GitHub Actions
Inactive
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dda02648f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…oc (#42) * docs: list capability-tour in the architecture tree The "Architecture at a glance" tree enumerated four skills; plugin/skills/ has five. capability-tour shipped in #34 without updating the diagram. The line is an exhaustive list, not shorthand: e927c24 (#22) expanded it from one entry to four when skills were added, and the sibling bin/ line enumerates all three files in plugin/bin/. The same README already depends on capability-tour as an existing feature further down. * docs: list prove:harness-install and guard the skill inventory Two findings from a full doc-surface audit. README's proof-command list omitted `prove:harness-install`, the only prove:* script missing from it. It runs inside `prove:all` and is the named authority in docs/native-capability-qualification.md for package, declaration, and installed-byte proof. capability-tour/SKILL.md enumerates the portable skills in prose with no generation step behind it, so a new skill silently falsifies the line. That is the same drift that left capability-tour out of README's architecture tree until 24102bc. Adds a test deriving the expected inventory from plugin/skills/ and runtime/skill-catalog.json. Sensitivity proven: planting a sixth skill directory turns the guard RED on the missing name; removing it returns GREEN. * docs: declare what verifies each document Maps each doc to the artifact that settles its claims, so a docs audit reads release.yml when checking publishing.md instead of hoping a scanner finds it. Four docs specify workflow behaviour in roughly 130 assertions between them; without the map those go unread. Also declares what this repo cannot check about itself. installing.md is ~90 claims about external Claude and Codex CLI surface, native-capability- qualification.md rests on human receipts, canary-qualification.md needs a hosted run. Scanning those returns nothing whether they are accurate or wrong, so they are marked unverifiable and reported rather than silently passing. Superseded ADRs 0002 and 0004 and docs/plans/ are frozen - they record history correctly. Consumed by the docs-drift skill. Every path verified to resolve. * test: guard the README architecture tree against skill and launcher drift The tree enumerates every skill and launcher by hand with nothing generating those lines, which is how capability-tour stayed missing between #34 and 24102bc. Derives both expected lists from plugin/skills/ and plugin/bin/. Closes the same defect class the capability-tour inventory guard covers in e4c6ea4; the README half was left open there. Sensitivity proven both ways: planting a sixth skill directory turns it RED on the skills line, planting a fourth launcher turns it RED on the bin line, and removing each returns GREEN. * Address PR review feedback (#42) - Parse the skill inventory instead of probing for each name. The containment loop passed while the prose named a skill that does not ship, and passed with a model-only skill inside the Bun-backed prefix; both reproduced before the change and both now fail. Also asserts every catalog entry ships, which is drift in the other direction. - Add plugin/skills/capability-tour/SKILL.md and scripts/prove-harness-install.ts to the README manifest entry. The architecture tree claims capability-tour ships and the proof list describes what prove:harness-install checks; neither package.json nor runtime.lock.json can settle those. --------- Co-authored-by: Nathan Vale <hi@nathanvale.com>
2 participants
Summary
Completes the Shared Bun Runtime Custody plan from authoring through packaged
qualification. Portable plugin skills can now use npm dependencies without
requiring users to install or pin Bun. A direct run stays read-only; when the
verified runtime is missing or corrupt, the agent previews repair, asks the
user, applies the approved repair, and retries the skill.
This PR implements all four planned units. It replaces the unconsumed QuickJS
spike with one Bun-only path and removes runtime hooks and prewarm setup.
Delivered plan
node_modules.run, read-only repair preview, and human-approvedrepair --applyover a private verified cache.User flow
mutation.
repair --applyacquires only the locked official asset,verifies it, publishes it atomically, and retries the original skill.
There is no installation command, lifecycle hook, prewarm step, or requirement
for a user-managed Bun version.
Trust boundaries
1.3.14is pinned across the package manager, lockfile, runtime lock,CI, and the four reviewed platform assets.
dynamic code generation, undeclared peers, native addons, lifecycle scripts,
external assets, and closure escapes fail package admission.
runnever acquires or repairs.repair --applyis the only mutation pathand expresses explicit mutation intent; the calling workflow owns the human
approval receipt.
digest, exact version output, and same-filesystem atomic publication.
symlinked state, hostile configuration, and cleanup failures fail closed with
a typed control envelope.
PATH, Bun option injection, automaticinstall,
.envloading, telemetry, and caller-cwd Bun configuration.Review outcome
Adversarial review found and closed a critical caller-cwd
bunfig.tomlpreloadpath that could execute untrusted code inside the verified runtime. Subsequent
review rounds hardened dependency admission, loader detection, peer and alias
resolution, archive handling, cache publication, writer identity, stale-lock
recovery, and cleanup envelopes.
Codex and CodeRabbit completed substantive reviews on exact head
ffd35da6a04ab8c0f8216a4894f070605f224155. No credible unresolved P0, P1, orP2 finding or actionable review thread remains.
Validation
bun run prove:allpasses on the exact head.without
node_modules.Darwin arm64.
mechanics pass.
Remaining release qualification
The implementation is complete. Release still requires candidate-bound human
receipts for a fresh Claude task, a fresh Codex task, and the Codex Desktop
approved repair/retry flow. Private remote transport also remains
credential-bound and is not claimed by the hermetic proof. These are release
qualification boundaries, not follow-up implementation units.
New concepts
Agent-native runtime custody
Runtime custody separates diagnosis from mutation. A normal skill run can only
verify and execute. Repair is a distinct preview → human approval → apply →
retry flow. This keeps first use setup-free while preventing an agent from
silently downloading an executable during ordinary execution.
Use this pattern when a portable plugin needs one shared native runtime that
may be absent on a user's machine. Do not use it when the host already provides
the runtime or when every invocation needs an isolated runtime identity.
Dependency-closed bundle distribution
Skills are authored in a normal Bun workspace with ESM, CJS, and npm
dependencies, but distribution contains only reviewed, digest-named ESM
artifacts. Package admission proves that no unresolved dependency or runtime
loader can escape the bundle closure.
This gives authors workspace ergonomics without turning the plugin into a
general bundler framework. It is intentionally unsuitable for native addons,
dynamic runtime imports, bundler plugins, or assets that must remain external.
Summary by CodeRabbit
skill-aandskill-blaunchers with offline execution and JSON results.