Remove the ambient storage binding from package invocation contexts

[kody-bot]

Problem

Final stage of removing legacy ambient-storage-in-package-code. The staged
plan so far: #816 shipped packageStorage() (package-bucket storage that
survives static imports), #817 prescribed it as the one storage rule for
saved-package code and added the advisory check nudge, and #820 escalated
that nudge to a failing repo check so new publishes cannot introduce the
pattern. The removal itself was gated on an audit of what is actually
published.

The gate is verified. An ad hoc artifact audit across ALL production
users — 26 users, 74 published packages, every current published bundle
artifact scanned from BUNDLE_ARTIFACTS_KV — found ZERO remaining
ambient-storage dereferences in invocation-context artifacts. Every flagged
package has been migrated and republished.

Approach

Remove the storageTools wiring that bound ambient storage to the package
bucket in package-invocation contexts. Exactly two sites bound it (the ones
catalogued in #820's PR body):

• packages/worker/src/package-invocations/service.ts — the
  runBundledModuleWithRegistry options for package export AND subscription
  handler runs (one shared call site) no longer pass
  storageTools: { storageId: buildPackageInvocationStorageId(...), writable: true }.
• packages/worker/src/package-retrievers/service.ts — retriever runs no
  longer pass the read-only equivalent.

After removal, ambient storage in package-invocation context is simply
absent: the storage helper prelude is omitted, so storage reaches the
sandbox as undefined, guarded access (if (storage) { ... }) keeps
working, and guard-less access gets the #812/#814 runtime_helper_unbound
structured error whose nextStep already leads with packageStorage() (from
#817). No new error machinery was needed — omitting the binding activates the
existing path.

buildPackageInvocationStorageId / buildPackageRetrieverStorageId remain:
they still feed callerContext.storageContext (package-secret scoping and
approval URLs) and runtime-debug metadata, neither of which is the
kody:runtime ambient binding. Comments at both call sites say so.

Explicitly unchanged

• Ad hoc execute with a caller-bound storageId — the prescribed ambient
  use (mcp/tools/execute.ts, mcp/capabilities/meta/execute.ts).
• Job- and service-scoped ambient buckets
  (jobs/service.ts, package-runtime/package-service.ts) — run-scoped
  scratch space (job:<id>, service:<pkg>:<name>), a distinct feature, not
  the legacy package-bucket binding.
• packageStorage() — now the only way package code reaches its bucket.

Judgment calls

• Retrievers bind nothing ambient. The old writable: false binding
  constrained only the ambient helper: packageStorage() has been writable
  in retriever context since Add packageStorage(): package-bucket storage that survives static imports #816 (retrievers pass packageContext, which
  grants the package id; createPackageStorageKodyTools always builds
  writable tools). So the read-only ambient binding was not a real safety
  boundary, and nothing replaces it — retrievers staying read-mostly is a
  convention, now stated in docs/contributing/packages-and-manifests.md and
  at the call site. packageStorage() in retriever context is covered by the
  existing skills-retriever-style workers tests and the reworked two-package
  test.
• The package-app runtime bridge is a separate surface, not this legacy
  pattern — confirmed, not removed. Apps run in their own worker; their
  storage comes from createRuntime in package-runtime/package-app.ts
  (createStorageProxy(runtimeBridge, packageId)) over host-pinned bridge
  RPCs — not from the per-run storageTools/helper-prelude binding this PR
  removes. Note for a possible follow-up: the app runtime does not provide
  packageStorage() (__kodyPackageStorage is absent from the app runtime
  object), so app code's storage access stays on the app bridge's ambient
  shape; aligning apps with the prescription would mean adding
  packageStorage to the app runtime bridge first.

Docs

docs/use/packages.md — the legacy section is now "Ambient storage in
package code (removed)": the binding no longer exists in invocation contexts,
with the runtime_helper_unbound/packageStorage() remedy pointer, and the
packageStorage() bullets no longer describe ambient storage as bound in the
package's own runtime. docs/use/execute.md and the execute tool's sandbox
text (mcp/tools/execute.ts) state that saved-package invocation runs bind no
ambient storage. docs/contributing/packages-and-manifests.md updates the
prescription bullet (removal shipped, audit cited) and the retriever
read-only line. docs/use/email-primitives.md and the service-pattern guide
anchor follow.

Tests

• package-storage.workers.test.ts (real buildKodyModuleBundle bundles,
  end-to-end):
  • The two-packages test now runs the host package the way invocations do
    post-removal — no storageTools — and asserts typeof storage === 'undefined' in the host package while both packages' packageStorage()
    buckets still resolve correctly.
  • New test: an invocation-context run (packageContext, no storageTools) of
    legacy package code that dereferences ambient storage guard-lessly
    yields the structured runtime_helper_unbound hint with
    helperName: 'storage' and a nextStep that mentions packageStorage()
    before the storageId alternative.
  • All pre-existing packageStorage matrix tests pass unchanged.
• package-invocations/service.node.test.ts — the subscription invocation
  test now asserts storageTools is NOT passed to
  runBundledModuleWithRegistry (was asserting the package-bucket binding).
• The Explain unbound optional kody:runtime helper access in execute errors #812 run-kody-registry test asserting "with storage bound, the
  TypeError stays bare" is unchanged: its bound case models ad hoc execute
  with a caller storageId, which this PR does not touch.

Local gate: npm run typecheck, npm run lint (0 errors, 21 pre-existing
warnings), npm run format:check, and npm run test (319 files / 998 tests)
all pass on main @ 3f4a43b1. The Playwright/MCP E2E halves of
npm run validate were not run locally; CI covers them.

System recap — removes a runtime binding (medium risk, audit-gated)

Mode: recap · Base: main @ 3f4a43b1 · Head: 18afe9af

Classification: removes — package-invocation runs (exports, subscription
handlers, retrievers) stop binding ambient storage; the change is
deliberately audit-gated: zero currently-published invocation-context
artifacts dereference the binding (26 users / 74 packages scanned).

Primitives touched

Primitive	Group	Impact
package-invocations	runtime	removes — export/subscription runs no longer pass storageTools
package-retrievers	runtime	removes — retriever runs no longer pass the read-only storageTools
package-storage	storage	composes — packageStorage() is now the only package-bucket path
mcp-server	surfaces	docs — execute sandbox text states invocation runs bind no ambient storage

System map

Package code reaches its bucket through packageStorage() only; the removed
ambient binding falls back to the existing unbound-helper error machinery, so
hypothetical stragglers get an actionable structured hint instead of a bare
TypeError.

Legend: green = composes · amber = extended by this PR · red = removed
binding · gray = context.

flowchart LR
	invocations["package-invocations<br/>exports + subscriptions"]:::added
	retrievers["package-retrievers<br/>retriever runs"]:::added
	packageStorage["packageStorage()<br/>provenance-granted bucket access"]:::touched
	unboundHint["runtime_helper_unbound<br/>#812/#814 error rewrite"]:::untouched
	jobsServices["jobs / services<br/>run-scoped ambient buckets"]:::untouched
	appBridge["package-app bridge<br/>host-pinned app storage"]:::untouched
	invocations -->|"bucket access now only via"| packageStorage
	retrievers -->|"bucket access now only via"| packageStorage
	invocations -->|"guard-less legacy access"| unboundHint
	classDef touched fill:#1a7f37,color:#fff
	classDef extended fill:#9a6700,color:#fff
	classDef added fill:#cf222e,color:#fff
	classDef untouched fill:#57606a,color:#fff

Loading

Before / after

Legacy package code doing import { storage } from 'kody:runtime' then
storage.get(...), invoked as a package export:

• Before: ambient storage was bound to the package bucket
  (package:<packageId>), identical to packageStorage().
• After: storage is undefined; the run fails with the structured
  runtime_helper_unbound error whose remedy leads with packageStorage()
  (identical bucket, so the fix is a rename). if (storage) { ... } guards
  observe undefined and keep working. Per the completed audit, no published
  artifact takes this path today.

Invariants

Bucket naming, provenance grants, entitlement enforcement, and per-user
isolation are untouched — the diff removes two storageTools inputs, updates
tests and docs, and changes no storage-runner or grant code.
callerContext.storageContext (secret scoping) still carries the package
storage id. Job/service scratch buckets and caller-bound execute storage
behave exactly as before. Repo checks (#820) prevent the removed pattern from
being republished.

[coderabbitai]

Warning

Review limit reached

@kody-bot, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 901c4ece-7460-4e78-b9a2-3b8c992eabf3

📥 Commits

Reviewing files that changed from the base of the PR and between 45ad3a0 and cd62077.

📒 Files selected for processing (10)

• docs/contributing/packages-and-manifests.md
• docs/guides/package-service-pattern.md
• docs/use/email-primitives.md
• docs/use/execute.md
• docs/use/packages.md
• packages/worker/src/mcp/tools/execute.ts
• packages/worker/src/package-invocations/service.node.test.ts
• packages/worker/src/package-invocations/service.ts
• packages/worker/src/package-retrievers/service.ts
• packages/worker/src/package-runtime/package-storage.workers.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch remove-ambient-package-storage-binding

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔎 Preview deployed: https://kody-pr-828.kody-a99.workers.dev

Worker: kody-pr-828
D1: kody-pr-828-db
KV: kody-pr-828-oauth-kv

Mocks:

• cloudflare: https://kody-pr-828-mock-cloudflare.kody-a99.workers.dev/__mocks (kody-pr-828-mock-cloudflare)