fix(package-apps): take D1 and run-record begin off the warm fetch path - #1410
Merged
Merged
Conversation
A hello-world package app that returns `ok` was ~113ms warm vs ~0ms for keyless packages.invoke of a trivial export. Serve now uses the invoke freshness cache, and the generated worker no longer awaits the run-record begin RPC before user fetch.
kentcdodds
marked this pull request as ready for review
August 12, 2026 21:56
📝 WalkthroughWalkthroughPackage-app serving now uses cached package and invocation-manifest loaders. Warm requests avoid package, source, and manifest storage loads. Fetch and realtime handlers start run recording asynchronously and finish through ChangesPackage-app runtime and serving
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🔵 Low · up to The change improves package-app latency, but the lookup cache needs to distinguish id-based and kody_id-based requests to avoid returning the wrong package record. The PR is otherwise mergeable with explicit owner follow-up on this bounded correctness issue. Sequence Diagram(s)sequenceDiagram
participant PackageAppHandler
participant packageRuntimeRunStart
participant UserCode
participant waitUntil
participant packageRuntimeRunFinish
PackageAppHandler->>packageRuntimeRunStart: start run without awaiting
PackageAppHandler->>UserCode: process fetch or realtime request
PackageAppHandler->>waitUntil: schedule run completion
waitUntil->>packageRuntimeRunStart: await start promise
waitUntil->>packageRuntimeRunFinish: finish resolved run
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
🔎 Preview deployed: https://kody-pr-1410.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/mcp/capabilities/packages/package-app-fetch.ts`:
- Around line 262-277: The cache key passed to
resolveSavedPackageWithFreshnessCache must include the lookup namespace: prefix
packageIdOrKodyId with “package-id” when input.packageId is provided and
“kody-id” when resolving by input.kodyId, while preserving the existing lookup
query 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: 16db9155-2150-44f1-b600-5444d7aac69b
📒 Files selected for processing (10)
docs/contributing/architecture/invocation-overhead-guardrails.mddocs/contributing/architecture/run-records.mdpackages/worker/src/app/handlers/package-app.node.test.tspackages/worker/src/mcp/capabilities/packages/package-app-fetch.tspackages/worker/src/package-invocations/invoke-contract-cache.tspackages/worker/src/package-invocations/module-artifacts.tspackages/worker/src/package-runtime/package-app-serve.node.test.tspackages/worker/src/package-runtime/package-app-serve.tspackages/worker/src/package-runtime/package-app.node.test.tspackages/worker/src/package-runtime/package-app.ts
Comment on lines
+262
to
+277
| const packageIdOrKodyId = input.packageId ?? input.kodyId ?? '' | ||
| return await resolveSavedPackageWithFreshnessCache({ | ||
| userId: input.userId, | ||
| kodyId: input.kodyId ?? '', | ||
| packageIdOrKodyId, | ||
| load: async () => { | ||
| if (input.packageId !== undefined) { | ||
| return await getSavedPackageById(input.db, { | ||
| userId: input.userId, | ||
| packageId: input.packageId, | ||
| }) | ||
| } | ||
| return await getSavedPackageByKodyId(input.db, { | ||
| userId: input.userId, | ||
| kodyId: input.kodyId ?? '', | ||
| }) | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect schema and write paths for constraints that prevent a package ID from
# matching a different package Kody ID for the same user.
rg -n -C 4 'saved_packages|kody_id|package_id|CREATE TABLE' .Repository: kentcdodds/kody
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- cache helper and call sites ---'
rg -n -C 8 'resolveSavedPackageWithFreshnessCache|packageIdOrKodyId' packages/worker/src
printf '%s\n' '--- saved package repository definitions ---'
rg -n -C 10 'getSavedPackageBy(Id|KodyId)|saved_packages' packages/worker/src packages/*/migrations packages/*/src 2>/dev/null | head -n 1200
printf '%s\n' '--- schema constraints and package identifiers ---'
rg -n -C 6 'CREATE TABLE[^;]*(saved_packages|packages)|UNIQUE[^;]*(kody_id|user_id)|kody_id.*UNIQUE|package.*id' packages --glob '*.sql' --glob '*.ts' --glob '*.tsx' | head -n 1600Repository: kentcdodds/kody
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact helper locations ---'
rg -l 'resolveSavedPackageWithFreshnessCache' packages/worker/src
rg -l 'function getSavedPackageBy(Id|KodyId)|export async function getSavedPackageBy(Id|KodyId)' packages/worker/src
printf '%s\n' '--- relevant implementation excerpts ---'
for f in $(rg -l 'resolveSavedPackageWithFreshnessCache' packages/worker/src); do
echo "FILE: $f"
rg -n -C 18 'resolveSavedPackageWithFreshnessCache|packageIdOrKodyId' "$f"
done
for f in $(rg -l 'function getSavedPackageBy(Id|KodyId)|export async function getSavedPackageBy(Id|KodyId)' packages/worker/src); do
echo "FILE: $f"
rg -n -C 16 'getSavedPackageBy(Id|KodyId)' "$f"
done
printf '%s\n' '--- production schema/migrations containing saved_packages ---'
git ls-files | rg '(^|/)(migrations?|schema|.*test-schema).*|\.sql$' | while read -r f; do
if rg -q 'saved_packages' "$f"; then
echo "FILE: $f"
rg -n -C 8 'saved_packages|idx_saved_packages_user_kody_id' "$f"
fi
doneRepository: kentcdodds/kody
Length of output: 18047
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- saved_packages write paths and ID generation ---'
rg -n -C 12 'INSERT INTO saved_packages|UPDATE saved_packages SET|id:\s*.*package|packageId.*random|randomUUID|crypto\.randomUUID|create.*package' packages/worker/src packages/worker/migrations --glob '*.ts' --glob '*.sql' | head -n 1800
printf '%s\n' '--- Kody ID validation and normalization ---'
rg -n -C 10 'kodyId|kody_id|KodyId|kody id' packages/worker/src/package-registry packages/worker/src/community packages/universal packages/shared --glob '*.ts' 2>/dev/null | rg -v 'node_modules' | head -n 1800
printf '%s\n' '--- cache tests and package-app-fetch tests ---'
rg -n -C 14 'saved-package|cache|package_id|kody_id|resolveSavedPackage' packages/worker/src/package-invocations packages/worker/src/mcp/capabilities/packages --glob '*test.ts' --glob '*.ts' | head -n 2200Repository: kentcdodds/kody
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- insertSavedPackage definition and callers ---'
rg -n -C 20 'export (async )?function insertSavedPackage|function insertSavedPackage|insertSavedPackage\(' packages/worker/src --glob '*.ts' --glob '!*.test.ts' | head -n 1000
printf '%s\n' '--- package ID assignments in production code ---'
rg -n -C 8 'const (packageId|id) =|packageId\s*[:=].*crypto|id\s*[:=].*crypto|crypto\.randomUUID\(\)' packages/worker/src --glob '*.ts' --glob '!*.test.ts' | rg -C 4 'package|saved|randomUUID|id' | head -n 1200
printf '%s\n' '--- Kody ID schemas and validators ---'
rg -n -C 14 'kody[_-]?id|kodyId' packages/worker/src/package-registry packages/worker/src/mcp/capabilities/packages packages/worker/src/community --glob '*.ts' --glob '!*.test.ts' | head -n 1600Repository: kentcdodds/kody
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- manifest Kody ID validation ---'
rg -n -C 18 'kody:\s*z|kodyIdSchema|kody.*id|id:\s*z\.(string|literal)|Kody.*Schema' packages/worker/src packages --glob '*.ts' --glob '!*.test.ts' | head -n 1600
printf '%s\n' '--- package ID creation in save flows ---'
rg -n -C 10 'const packageId|let packageId|packageId =' packages/worker/src/mcp/capabilities/packages/save-package.ts packages/worker/src/mcp/capabilities/packages/create-stub-package.ts packages/worker/src/mcp/capabilities/repo/repo-promote-to-package.ts packages/worker/src/package-registry/service.ts
printf '%s\n' '--- relevant cache tests ---'
rg -n -C 20 'resolveSavedPackageWithFreshnessCache|savedPackageCacheKey|clearInvokeContractCachesForTests|packageIdOrKodyId' packages/worker/src --glob '*test.ts' | head -n 1800Repository: kentcdodds/kody
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- authored package schema definition ---'
rg -n -C 25 'authoredPackageJsonSchema|AuthoredPackageJson' packages/worker/src packages --glob '*.ts' --glob '!*.test.ts' | head -n 1400
printf '%s\n' '--- package ID creation around save handlers ---'
sed -n '220,325p' packages/worker/src/mcp/capabilities/packages/save-package.ts
sed -n '60,118p' packages/worker/src/mcp/capabilities/packages/create-stub-package.ts
sed -n '140,165p' packages/worker/src/mcp/capabilities/repo/repo-promote-to-package.tsRepository: kentcdodds/kody
Length of output: 1919
Include the lookup kind in the cache key. id and kody_id use different lookup queries, but the schema does not make their namespaces disjoint. Prefix the key with package-id or kody-id to prevent one lookup from returning another package’s cached record.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/mcp/capabilities/packages/package-app-fetch.ts` around
lines 262 - 277, The cache key passed to resolveSavedPackageWithFreshnessCache
must include the lookup namespace: prefix packageIdOrKodyId with “package-id”
when input.packageId is provided and “kody-id” when resolving by input.kodyId,
while preserving the existing lookup query behavior.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b1e8685. Configure here.
…urce On artifact miss, re-derive kody.app.entry from loaded files and look up the current source row so a freshness-cached manifest cannot poison the previous commit's bundle identity.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Package apps felt slow. A hello-world
fetchthat returnsokconfirmed that: ~113ms warm platform overhead vs ~0ms for keylesspackages.invokeof a trivial export. This PR removes the two host costs that do not belong on the response path.Probe
Hidden private package
perf-app-tiny-a287(package_id6143a543-ef4c-482d-a1c4-a6e236feaa91): default export{ ok: true }, app handlerreturn new Response('ok'). Timed from execute against production:packages.invokepingpackage_app_fetchokUser code is not the bottleneck. Two host costs were on every warm hit:
packageRuntimeRunStartbefore userfetch.beginRunRecordis synchronous and fire-and-forgets therunninginsert; finish already upserts the terminal row. The isolate hop was the cost.Summary
resolveSavedPackage+loadInvokeManifestBySourceId(same freshness/commit caches as keyless invoke). Warm serve now does zero D1/KV loads before dispatch.await, runs userfetch/onRealtimeEvent, andwaitUntils begin-then-finish. Start RPC failures no longer fail the HTTP response.package_app_fetchshares the saved-package freshness cache and validates the path before any D1 lookup.kody.app.entryfrom the loaded files and persists against a freshly loaded source row (same as invoke), so a stale freshness-cached manifest cannot poison the previous commit's bundle identity.Remaining warm cost after this (not in this PR):
APP_LOADER.getper request (stubs are request-bound; worker options are already cached) plus isolateimport()of the user module.Testing
vitestnode-unit for package-app serve/worker, handler,package_app_fetch, and invoke-contract-cacheperf-app-tiny-a287after deploy)System recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@4e2f43a9· Head:189e5312Classification: extends — package-app HTTP now shares the invoke freshness cache and no longer awaits run-record begin on the response path.
Primitives touched
package-appswaitUntil; artifact rebuild persists against the fresh sourcepackage-runtimesaved-packagespackage_app_fetchshares the cache; path validation before D1app-uiSystem map
Warm package-app HTTP now resolves the saved package and manifest through the invoke freshness cache, then runs user
fetchwithout waiting for the run-record begin RPC. Artifact misses rebuild against the current published source, not the cached row.Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Change flow
sequenceDiagram participant Browser participant Serve as servePackageAppRequest participant Cache as invoke freshness cache participant Isolate as PackageAppWorker participant Bridge as packageRuntimeRunStart Browser->>Serve: GET /packages/{kodyId}/ Serve->>Cache: resolveSavedPackage + loadInvokeManifest Note over Cache: warm: zero D1/KV Serve->>Isolate: entrypoint.fetch Isolate-->>Bridge: startRuntimeRun (not awaited) Isolate->>Isolate: user fetch() Isolate-->>Browser: Response Isolate->>Bridge: waitUntil begin then finishBefore / after
Warm hello-world
app_fetch(production probe, before this PR): ~113ms platform overhead vs ~0ms keyless invoke.Warm serve D1/KV (this PR's node test): zero saved-package, source-row, and manifest loads before dispatch.
Artifact miss after republish: rebuild entry and persist identity come from the freshly loaded source, matching invoke.
Invariants
Cache keys still start with the owning
userId. Publish/rebuild keep using uncachedloadPackageManifestBySourceId. A droppedrunninginsert remains harmless because finish upserts the terminal row. Artifact hits may still serve the stale commit for up to the freshness TTL.Summary by CodeRabbit
Performance
Reliability
Tests
Documentation