Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions docs/contributing/architecture/invocation-overhead-guardrails.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,20 @@ call, not about what user code does inside the call.
[Run records](./run-records.md)), so the durability cost is **one awaited DO
call for claim + run-record begin and one for terminal response + run-record
finish** — no D1 round trips at all.
- **Package-app HTTP** (`servePackageAppRequest` / `app_fetch`) is the same
class of hot path as keyless invoke: a hello-world `fetch` that returns `ok`
should not pay tens of milliseconds of platform overhead on a warm isolate.
The serve path shares the invoke freshness/commit caches, and must not await
the run-record begin RPC before user `fetch`. Finish already upserts a
complete row, so a dropped `running` insert is harmless.

## Per-isolate caches and their staleness bounds

The keyless `packages.invoke` contract check (saved-package row, entity-source
row, manifest, bundle artifact) and per-run registry assembly are cached per
isolate so a warm invoke of an already-warm package+commit performs **zero D1/KV
loads** before dispatch. The caches come in two tiers with different correctness
arguments (see
row, manifest, bundle artifact) and package-app HTTP serve (saved-package row,
entity-source row, manifest) are cached per isolate so a warm call of an
already-warm package+commit performs **zero D1/KV loads** before dispatch. The
caches come in two tiers with different correctness arguments (see
`packages/worker/src/package-invocations/invoke-contract-cache.ts`):

- **Freshness tier** — saved-package row and entity-source row (the row that
Expand All @@ -56,9 +62,10 @@ arguments (see
Rules for touching these paths: publish and rebuild flows must keep using the
uncached row/manifest loaders (`loadPackageManifestBySourceId`,
`loadPackageSourceBySourceId`) so a rebuild can never run against a stale
`published_commit`; every cache key must start with the owning `userId` (per
user isolation is inviolable); and new caches on invocation paths need an
explicit staleness bound documented here.
`published_commit`; package-app HTTP serve must keep using the cached invoke
loaders (`resolveSavedPackage`, `loadInvokeManifestBySourceId`); every cache key
must start with the owning `userId` (per user isolation is inviolable); and new
caches on invocation paths need an explicit staleness bound documented here.

## Watch the percentiles

Expand Down
6 changes: 5 additions & 1 deletion docs/contributing/architecture/run-records.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,11 @@ Rules:
fire-and-forgets `startRun` (optionally via `waitUntil`). It returns a handle
that already carries a minted run id, `startedAt`, persistence mode, and the
full context — or `null` when there is no user / no `RUN_LOG` binding /
missing context.
missing context. Callers that reach begin through a WorkerEntrypoint RPC
(package-app isolates: `packageRuntimeRunStart`) must not await that RPC on
the HTTP/realtime response path — kick it off, run user code, and `waitUntil`
begin-then-finish. A dropped `running` insert is still harmless because finish
upserts the terminal row.
- **`finishRunRecord` awaits the complete-row upsert in one RPC** (`finishRun`),
then replaces logs and enforces retention. A supplied `waitUntil` applies only
to post-terminal observers such as `run.error.recorded`, never to the terminal
Expand Down
24 changes: 12 additions & 12 deletions packages/worker/src/app/handlers/package-app.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const mockModule = vi.hoisted(() => ({
})),
redirectToLogin: vi.fn(() => new Response(null, { status: 302 })),
getAppBaseUrl: vi.fn(() => 'https://example.com'),
getSavedPackageByKodyId: vi.fn(async () => ({
resolveSavedPackage: vi.fn(async () => ({
id: 'package-1',
userId: 'user-1',
name: '@kody/example',
Expand All @@ -43,7 +43,7 @@ const mockModule = vi.hoisted(() => ({
loadPackageSourceBySourceId: vi.fn(async () => {
throw new Error('bundle failed')
}),
loadPackageManifestBySourceId: vi.fn(async () => {
loadInvokeManifestBySourceId: vi.fn(async () => {
throw new Error('manifest load failed')
}),
createPackageAppCallerContext: vi.fn(),
Expand Down Expand Up @@ -77,16 +77,16 @@ vi.mock('#worker/app-base-url.ts', () => ({
getAppBaseUrl: (...args: Array<unknown>) => mockModule.getAppBaseUrl(...args),
}))

vi.mock('#worker/package-registry/repo.ts', () => ({
getSavedPackageByKodyId: (...args: Array<unknown>) =>
mockModule.getSavedPackageByKodyId(...args),
}))

vi.mock('#worker/package-registry/source.ts', () => ({
loadPackageSourceBySourceId: (...args: Array<unknown>) =>
mockModule.loadPackageSourceBySourceId(...args),
loadPackageManifestBySourceId: (...args: Array<unknown>) =>
mockModule.loadPackageManifestBySourceId(...args),
}))

vi.mock('#worker/package-invocations/module-artifacts.ts', () => ({
resolveSavedPackage: (...args: Array<unknown>) =>
mockModule.resolveSavedPackage(...args),
loadInvokeManifestBySourceId: (...args: Array<unknown>) =>
mockModule.loadInvokeManifestBySourceId(...args),
}))

vi.mock('#worker/package-runtime/package-app.ts', () => ({
Expand Down Expand Up @@ -199,7 +199,7 @@ test('handlePackageAppRequest does not report package entrypoint failures to Kod
resetMocks()
consoleError.mockImplementation(() => {})

mockModule.loadPackageManifestBySourceId.mockResolvedValueOnce({
mockModule.loadInvokeManifestBySourceId.mockResolvedValueOnce({
source: {
published_commit: 'commit-1',
manifest_path: 'package.json',
Expand Down Expand Up @@ -273,7 +273,7 @@ test('handlePackageAppRequest routes websocket package paths to realtime session
test('handlePackageAppRequest forwards package code a request stripped of owner credentials', async () => {
resetMocks()

mockModule.loadPackageManifestBySourceId.mockResolvedValueOnce({
mockModule.loadInvokeManifestBySourceId.mockResolvedValueOnce({
source: {
published_commit: 'commit-1',
manifest_path: 'package.json',
Expand Down Expand Up @@ -336,5 +336,5 @@ test('handlePackageAppRequest returns not found when the URL username does not m
)

expect(response.status).toBe(404)
expect(mockModule.getSavedPackageByKodyId).not.toHaveBeenCalled()
expect(mockModule.resolveSavedPackage).not.toHaveBeenCalled()
})
34 changes: 21 additions & 13 deletions packages/worker/src/mcp/capabilities/packages/package-app-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
packageScopeInputDescription,
resolvePackageOwnerContext,
} from '#worker/package-registry/package-owner.ts'
import { resolveSavedPackageWithFreshnessCache } from '#worker/package-invocations/invoke-contract-cache.ts'
import {
getSavedPackageById,
getSavedPackageByKodyId,
Expand Down Expand Up @@ -258,15 +259,22 @@ async function resolveOwnedSavedPackage(input: {
packageId?: string
kodyId?: string
}) {
if (input.packageId !== undefined) {
return await getSavedPackageById(input.db, {
userId: input.userId,
packageId: input.packageId,
})
}
return await getSavedPackageByKodyId(input.db, {
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 ?? '',
})
},
Comment on lines +262 to +277

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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 1600

Repository: 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
done

Repository: 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 2200

Repository: 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 1600

Repository: 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 1800

Repository: 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.ts

Repository: 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.

})
}

Expand Down Expand Up @@ -366,6 +374,11 @@ export const packageAppFetchCapability = defineDomainCapability(
}
assertRequestBodyWithinLimit(args.body)

// Validate the path before any D1 lookup so traversal and encoding
// errors fail closed without warming the saved-package cache.
const restPath = normalizePackageAppFetchPath(args.path)
const restPathOnly = restPath.split(/[?#]/, 1)[0] || '/'

const owner = await resolvePackageOwnerContext(
ctx.env,
user,
Expand Down Expand Up @@ -396,11 +409,6 @@ export const packageAppFetchCapability = defineDomainCapability(
)
}

// The normalized path keeps any query string for the request URL; the
// package path's restPath is path-only (matching what URL parsing
// produced for browser requests).
const restPath = normalizePackageAppFetchPath(args.path)
const restPathOnly = restPath.split(/[?#]/, 1)[0] || '/'
const packageAppOrigin = getPackageAppBaseUrl({ env: ctx.env })
const hostedUrl = resolveHostedPackageAppUrl({
packageAppBaseUrl: packageAppOrigin,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { type PublishedBundleArtifact } from '#worker/package-runtime/published-
import { type EntitySourceRow } from '#worker/repo/types.ts'

/**
* Per-isolate caches for the `packages.invoke` contract check, so a warm
* keyless invoke of an already-warm package+commit performs zero D1/KV loads
* before dispatch (see
* Per-isolate caches for invocation hot paths (`packages.invoke` contract
* check and package-app HTTP serve), so a warm call of an already-warm
* package+commit performs zero D1/KV loads before dispatch (see
* docs/contributing/architecture/invocation-overhead-guardrails.md).
*
* Two tiers with different lifetimes:
Expand Down
3 changes: 2 additions & 1 deletion packages/worker/src/package-invocations/module-artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ export async function resolveSavedPackage(input: {
* contract cache, and the manifest itself from the commit-keyed manifest
* cache. Warm calls perform zero D1/KV loads. Publish and rebuild flows must
* keep using `loadPackageManifestBySourceId`, which always reads the row
* fresh.
* fresh. Package-app HTTP serve is an invocation hot path and uses this
* loader; do not switch it back to the uncached source-row read.
*/
export async function loadInvokeManifestBySourceId(input: {
env: Env
Expand Down
185 changes: 185 additions & 0 deletions packages/worker/src/package-runtime/package-app-serve.node.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
import { expect, test, vi } from 'vitest'
import { servePackageAppRequest } from './package-app-serve.ts'

const mockModule = vi.hoisted(() => ({
getSavedPackageById: vi.fn(),
getSavedPackageByKodyId: vi.fn(),
getEntitySourceById: vi.fn(),
loadPublishedEntityManifest: vi.fn(),
buildPackageAppWorker: vi.fn(),
}))

vi.mock('#worker/package-registry/repo.ts', () => ({
getSavedPackageById: (...args: Array<unknown>) =>
mockModule.getSavedPackageById(...args),
getSavedPackageByKodyId: (...args: Array<unknown>) =>
mockModule.getSavedPackageByKodyId(...args),
}))

vi.mock('#worker/repo/entity-sources.ts', () => ({
getEntitySourceById: (...args: Array<unknown>) =>
mockModule.getEntitySourceById(...args),
}))

vi.mock('#worker/repo/published-source.ts', () => ({
loadPublishedEntityManifest: (...args: Array<unknown>) =>
mockModule.loadPublishedEntityManifest(...args),
loadPublishedEntitySource: vi.fn(),
}))

vi.mock('#worker/package-runtime/package-app.ts', () => ({
createPackageAppCallerContext: async (input: {
user: { userId: string; email: string; displayName: string }
}) => ({
user: input.user,
}),
buildPackageAppWorker: (...args: Array<unknown>) =>
mockModule.buildPackageAppWorker(...args),
}))

const serveLoadMocks = [
['saved package by id (D1)', mockModule.getSavedPackageById],
['saved package by kody id (D1)', mockModule.getSavedPackageByKodyId],
['entity source row (D1)', mockModule.getEntitySourceById],
['published manifest snapshot (KV)', mockModule.loadPublishedEntityManifest],
] as const

function countServeLoads() {
return Object.fromEntries(
serveLoadMocks.map(([label, mock]) => [label, mock.mock.calls.length]),
)
}

function clearServeLoadCounters() {
for (const [, mock] of serveLoadMocks) {
mock.mockClear()
}
}

function createFixture() {
const sourceId = 'source-perf-app'
const savedPackage = {
id: 'pkg-perf-app',
userId: 'user-1',
name: '@kentcdodds/perf-app',
kodyId: 'perf-app',
description: 'Minimal hello-world app',
tags: [],
searchText: null,
sourceId,
hasApp: true,
hidden: true,
isPrivate: true,
createdAt: '2026-08-12T00:00:00.000Z',
updatedAt: '2026-08-12T00:00:00.000Z',
}
const source = {
id: sourceId,
user_id: 'user-1',
entity_kind: 'package' as const,
entity_id: savedPackage.id,
repo_id: 'repo-perf-app',
published_commit: 'commit-1',
indexed_commit: 'commit-1',
manifest_path: 'package.json',
source_root: '/',
last_external_check_at: null,
external_check_until: null,
created_at: '2026-08-12T00:00:00.000Z',
updated_at: '2026-08-12T00:00:00.000Z',
}
const manifestContent = JSON.stringify({
name: '@kentcdodds/perf-app',
private: true,
exports: { '.': './src/index.ts' },
kody: {
id: 'perf-app',
description: 'Minimal hello-world app',
app: { entry: './src/app.ts' },
},
})
return { savedPackage, source, manifestContent }
}

function seedFixture() {
const fixture = createFixture()
mockModule.getSavedPackageById.mockResolvedValue(null)
mockModule.getSavedPackageByKodyId.mockImplementation(
async (_db: unknown, input: { userId: string; kodyId: string }) =>
input.userId === fixture.savedPackage.userId &&
input.kodyId === fixture.savedPackage.kodyId
? fixture.savedPackage
: null,
)
mockModule.getEntitySourceById.mockImplementation(
async (_db: unknown, sourceId: string) =>
sourceId === fixture.source.id ? fixture.source : null,
)
mockModule.loadPublishedEntityManifest.mockImplementation(
async (input: { sourceId: string }) => {
if (input.sourceId !== fixture.source.id) {
throw new Error(`No manifest for ${input.sourceId}`)
}
return { source: fixture.source, content: fixture.manifestContent }
},
)
mockModule.buildPackageAppWorker.mockResolvedValue({
entrypointName: 'PackageAppWorker',
stub: {
getEntrypoint: () => ({
async fetch() {
return new Response('ok')
},
}),
},
})
return fixture
}

async function serveHelloWorld() {
return await servePackageAppRequest({
request: new Request('https://example.com/@kentcdodds/packages/perf-app'),
env: {
APP_DB: {},
BUNDLE_ARTIFACTS_KV: {},
} as Env,
owner: {
userId: 'user-1',
username: 'kentcdodds',
email: 'kent@example.com',
displayName: 'Kent',
},
packagePath: {
username: 'kentcdodds',
kodyId: 'perf-app',
restPath: '/',
mount: 'username-path',
},
})
}

test('a warm package-app serve performs zero D1/KV loads before dispatch', async () => {
seedFixture()

const cold = await serveHelloWorld()
expect(cold.status).toBe(200)
expect(await cold.text()).toBe('ok')
expect(mockModule.getSavedPackageByKodyId).toHaveBeenCalledTimes(1)
expect(mockModule.getEntitySourceById).toHaveBeenCalledTimes(1)
expect(mockModule.loadPublishedEntityManifest).toHaveBeenCalledTimes(1)
expect(mockModule.buildPackageAppWorker).toHaveBeenCalledTimes(1)

clearServeLoadCounters()
mockModule.buildPackageAppWorker.mockClear()
const warm = await serveHelloWorld()

expect(warm.status).toBe(200)
expect(await warm.text()).toBe('ok')
expect(countServeLoads()).toEqual({
'saved package by id (D1)': 0,
'saved package by kody id (D1)': 0,
'entity source row (D1)': 0,
'published manifest snapshot (KV)': 0,
})
expect(mockModule.buildPackageAppWorker).toHaveBeenCalledTimes(1)
})
Loading
Loading