Skip to content

Implement repo-backed source foundation - #181

Merged
kentcdodds merged 42 commits into
mainfrom
cursor/repo-backed-source-23f1
Apr 17, 2026
Merged

kentcdodds merged 42 commits into
mainfrom
cursor/repo-backed-source-23f1

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Apr 16, 2026 •

Copy link
Copy Markdown
Member

Summary

  • add the Artifacts-backed source/session data model and worker bindings
  • add the repo-backed repo_* capability surface for opening sessions, reading/searching files, summarizing trees, applying transactional edits, running Worker-native checks, and rebase/publish lifecycle operations
  • cut jobs and skills onto repo-backed source/session behavior and make jobs first-class published search entities
  • make saved app writes source-aware and cut the main saved-app read/runtime entry points over to prefer repo-backed app source when available
  • add source-sync helpers so save/update flows can mirror current source into repo-backed artifacts and record a published commit before the post-merge migration/backfill runs
  • add a job reindex maintenance hook so the post-merge projection cleanup has a concrete endpoint
  • lazy-load the worker-bundler checks path and job execution runtime so the branch remains stable under the existing node/workers test suite despite the upstream gojs / esbuild.wasm package bug

Testing

  • npm run typecheck
  • npx vitest run packages/worker/src/mcp/context.node.test.ts packages/worker/src/jobs/service.node.test.ts packages/worker/src/oauth-handlers-skill-runner.workers.test.ts
  • npx vitest run packages/worker/src/jobs/service.node.test.ts packages/worker/src/mcp/run-codemode-registry.node.test.ts packages/worker/src/mcp/tools/search.node.test.ts packages/worker/src/mcp/capabilities/meta/meta-list-capabilities.node.test.ts packages/worker/src/mcp/capabilities/build-capability-registry.workers.test.ts
  • npx vitest run packages/worker/src/mcp/tools/search.node.test.ts packages/worker/src/mcp/capabilities/unified-search.workers.test.ts
  • npx vitest run packages/worker/src/mcp/capabilities/apps/ui-save-app.node.test.ts packages/worker/src/mcp/observability.workers.test.ts packages/worker/src/mcp/capabilities/apps/saved-app-capabilities.node.test.ts packages/worker/src/app/saved-ui-hosted-html.node.test.ts
  • npx vitest run packages/worker/src/storage-runner.workers.test.ts packages/worker/src/mcp/capabilities/build-capability-registry.workers.test.ts
  • npm run preview:e2e
  • repeated git push runs exercised the branch through the repo’s pre-push test/e2e hooks on each stable checkpoint

Notes

  • this branch now implements the full first phase up to the point of the actual post-merge migration/backfill execution
  • existing save/update flows can seed repo-backed artifacts immediately in real runtimes, but the one-time migration/backfill over already-saved data still remains intentionally post-merge and is tracked in issue Complete repo-backed source phased migration #183
  • some legacy inline fields remain as compatibility caches until that post-merge migration runs; they are no longer the only path for the major workflows implemented here
  • the installed @cloudflare/worker-bundler package still has a broken eager import path in this environment (gojs through esbuild.wasm), so the repo checks path intentionally lazy-loads it to avoid breaking unrelated imports while the migration continues
Open in Web Open in Cursor 

Summary by CodeRabbit

  • New Features

    • Full repo-backed workflows: open/manage sessions, browse tree, read/write files, search, apply patches, run checks, rebase/publish, and discard sessions.
    • Repo-backed saved entities: skills, apps, jobs can be stored in repos and synced; saved UIs resolve repo-hosted client/server code and parameters.
    • Jobs: repo-backed execution, vector embeddings, unified search integration, and a maintenance endpoint to reindex job vectors.
  • Chores

    • DB migrations, durable object and worker config, artifact namespace, repo tooling, schema/type updates, and test adjustments.

@coderabbitai

coderabbitai Bot commented Apr 16, 2026 •

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds repository-backed artifact/session infrastructure and MCP repo capabilities; persists and syncs entity sources; enables repo-backed execution for apps/skills/jobs; integrates jobs into vector search and unified search; introduces DB migrations, Durable Object repo sessions, maintenance reindex endpoint, and related types/tests/tooling.

Changes

Cohort / File(s) Summary
Shared schemas & caller context
packages/shared/src/chat.ts, packages/worker/src/mcp/context.ts, packages/worker/src/mcp/context.node.test.ts
Add mcpRepoContextSchema and exported McpRepoContext; propagate repoContext into MCP caller context and tests.
DB migrations
packages/worker/migrations/0023-entity-sources.sql, packages/worker/migrations/0024-repo-sessions-and-source-columns.sql
Create entity_sources and repo_sessions tables; add source_id (and published_commit where noted) columns and indexes; add source_id to jobs/mcp_skills/ui_artifacts.
Repo types, manifests & templates
packages/worker/src/repo/types.ts, packages/worker/src/repo/manifest.ts, packages/worker/src/repo/source-templates.ts
New repo/session/entity Zod schemas and TS types, manifest parsing/projection helpers, and source-file template builders for skill/app/job artifacts.
Repo Durable Object & RPC
packages/worker/src/repo/repo-session-do.ts, packages/worker/src/repo/repo-session-rpc.ts, packages/worker/src/repo/repo-sessions.ts, packages/worker/worker-configuration.d.ts, packages/worker/wrangler.jsonc
Add RepoSession Durable Object implementation, RPC factory, DB CRUD for sessions, DO binding registration and runtime type updates.
Entity sources & source service/sync
packages/worker/src/repo/entity-sources.ts, packages/worker/src/repo/source-service.ts, packages/worker/src/repo/source-sync.ts
CRUD/upsert/delete for entity_sources; ensure/create artifact repos; syncArtifactSourceSnapshot to apply files and publish, returning published commit.
Artifacts binding & utilities
packages/worker/src/repo/artifacts.ts
Types and helpers for ARTIFACTS binding, repo id builders, token parsing, authenticated remote builder, and repo resolution helpers.
Repo checks & session tooling
packages/worker/src/repo/checks.ts, packages/worker/src/repo/repo-session-search.ts, packages/worker/src/repo/repo-session-tree.ts
Repo-level checks (manifest/deps/bundle/typecheck/lint), workspace search, match-context extraction, and tree/result mapping helpers.
Repo MCP capabilities & shared schemas
packages/worker/src/mcp/capabilities/repo/*, packages/worker/src/mcp/capabilities/repo/repo-shared.ts, packages/worker/src/mcp/capabilities/builtin-domains.ts, packages/worker/src/mcp/capabilities/domain-metadata.ts, packages/worker/src/mcp/capabilities/repo/index.ts
New repo capability domain and capabilities (open/get/publish/rebase/discard session, read/write file, apply patch, tree, search, run/get-check status) plus shared repo schemas and domain registration.
Saved UI / app resolution & persistence
packages/worker/src/repo/app-source.ts, packages/worker/src/app/handlers/saved-ui.ts, packages/worker/src/mcp/app-runner.ts, packages/worker/src/mcp/generated-ui-api.ts, packages/worker/src/mcp/capabilities/apps/*, packages/worker/src/mcp/ui-artifacts-repo.ts, packages/worker/src/mcp/ui-artifacts-types.ts
Resolve saved app sources from repo when sourceId present; return resolved client/server code/parameters; persist sourceId on artifacts; add sync/rollback handling and test ARTIFACTS mocks.
Skills & Jobs: repo-backed execution and persistence
packages/worker/src/mcp/skills/run-saved-skill.ts, packages/worker/src/mcp/skills/mcp-skills-repo.ts, packages/worker/src/mcp/skills/mcp-skills-types.ts, packages/worker/src/mcp/capabilities/meta/meta-save-skill.ts, packages/worker/src/jobs/service.ts, packages/worker/src/jobs/repo.ts, packages/worker/src/jobs/types.ts
Support repo-backed skills/jobs: branch execution to repo flow, persist source_id/published_commit, call ensureEntitySource and syncArtifactSourceSnapshot, update persistence and rollback handling.
Job vectorization & unified search integration
packages/worker/src/mcp/jobs-embed.ts, packages/worker/src/mcp/jobs-vectorize.ts, packages/worker/src/mcp/capabilities/unified-search.ts, packages/worker/src/mcp/tools/search-format.ts, packages/worker/src/mcp/tools/search.ts, packages/worker/src/mcp/tools/search.node.test.ts
Build job embed text, upsert/delete job vectors, add job hit types, integrate jobs into unified search ranking (lexical+vector fusion), and adapt search formatting and loaders to include jobs.
Agent & search tooling changes
packages/worker/src/agent-turn/tools.ts, packages/worker/src/mcp/tools/search.ts
Agent search tool now loads jobs (listJobs) into optional search rows and forwards jobRows into unified search.
Job maintenance / reindex endpoint
packages/worker/src/job-maintenance.ts, packages/worker/src/jobs/job-reindex.ts, packages/worker/src/index.ts
Add POST /__maintenance/reindex-jobs (Bearer secret auth) and reindexJobVectors to rebuild/upsert job embeddings; re-export RepoSession.
Executor & codemode options
packages/worker/src/mcp/executor.ts, packages/worker/src/mcp/run-codemode-registry.ts
createExecuteExecutor accepts modules map; runCodemodeWithRegistry forwards executorModules into the executor.
Persistence updates (skills, ui_artifacts, jobs)
packages/worker/src/mcp/skills/mcp-skills-repo.ts, packages/worker/src/mcp/skills/mcp-skills-types.ts, packages/worker/src/mcp/ui-artifacts-repo.ts, packages/worker/src/mcp/ui-artifacts-types.ts, packages/worker/src/jobs/repo.ts
Add source_id to skill/ui_artifacts persistence and types; jobs schema extended with source_id/published_commit and nullable code; SQL read/write updated.
Tests & mocks
various packages/worker/src/.../*.test.ts
Many tests updated for new persisted columns, repo-backed flows, ARTIFACTS mock, repoContext propagation, vector deletion/reindex behaviors, and session discard checks.
Deps & worker config
packages/worker/package.json, packages/worker/wrangler.jsonc, packages/worker/worker-configuration.d.ts
Add @cloudflare/shell and @cloudflare/worker-bundler; add ARTIFACTS binding and REPO_SESSION DO; update generated worker types (durable namespaces, cache API, resource limits, AI overloads).

Sequence Diagram(s)

sequenceDiagram
    rect rgba(200,200,255,0.5)
    actor User
    participant JobSvc as Job Service
    participant RepoRPC as RepoSession RPC
    participant Shell as Cloudflare Shell (Workspace)
    participant AppDB as D1 Database
    participant AI as AI / Executor
    end

    User->>JobSvc: trigger job run (repo-backed)
    JobSvc->>RepoRPC: openSession({ sourceId, userId, ... })
    RepoRPC->>Shell: fork/clone repo, init workspace
    RepoRPC->>AppDB: upsert repo_sessions
    RepoRPC-->>JobSvc: sessionId

    JobSvc->>RepoRPC: read entrypoint / runChecks
    RepoRPC->>Shell: read files, run checks, compute tree/hash
    RepoRPC->>AppDB: persist check results
    RepoRPC-->>JobSvc: manifest + file content

    JobSvc->>AI: run codemode with entrypoint + repoContext
    AI-->>JobSvc: execution result
    JobSvc-->>User: ExecuteResult
Loading
sequenceDiagram
    rect rgba(200,255,200,0.5)
    actor User
    participant UISave as ui_save_app Handler
    participant EntitySvc as Entity Source Service
    participant Artifacts as ARTIFACTS Binding
    participant RepoRPC as RepoSession RPC
    participant AppDB as D1 Database
    end

    User->>UISave: save app with source intent
    UISave->>EntitySvc: ensureEntitySource({ userId, entityKind:'app', entityId })
    EntitySvc->>AppDB: query entity_sources
    alt exists
        EntitySvc-->>UISave: return existing source
    else create
        EntitySvc->>Artifacts: create/fork repo
        Artifacts-->>EntitySvc: repo handle
        EntitySvc->>AppDB: insert entity_sources
        EntitySvc-->>UISave: created source
    end
    UISave->>AppDB: insert/update ui_artifact (with sourceId)
    UISave->>RepoRPC: openSession({ sourceId, userId, ... })
    RepoRPC->>Shell: apply edits (client/server/kody.json)
    RepoRPC->>RepoRPC: publishSession -> push & update entity_sources.published_commit
    RepoRPC-->>UISave: published commit
    UISave->>AppDB: update ui_artifact.published_commit
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Poem

🐰 I nibbled through commits and hopped on branches new,

Opened sessions, stitched manifests, and wrote a sync or two.
I leapt through vectors, seeds of jobs aligned,
And left a trail of carrots so the repo blooms in kind.
Hooray — a cozy burrow for artifacts and glue!

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/repo-backed-source-23f1

@kentcdodds
kentcdodds marked this pull request as ready for review April 16, 2026 22:58
Comment thread packages/worker/src/jobs/service.ts Outdated
Comment thread packages/worker/src/mcp/capabilities/unified-search.ts
Comment thread packages/worker/src/repo/app-source.ts
@github-actions

github-actions Bot commented Apr 16, 2026 •

Copy link
Copy Markdown
Contributor

🔎 Preview deployed: https://kody-pr-181.kentcdodds.workers.dev

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

Mocks:

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Comment thread packages/worker/src/jobs/repo.ts
Comment thread packages/worker/src/mcp/capabilities/repo/repo-open-session.ts
Comment thread packages/worker/src/jobs/job-reindex.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 13

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
packages/worker/src/mcp/capabilities/meta/meta-save-skill.ts (1)

199-241: ⚠️ Potential issue | 🟡 Minor

Add deleteEntitySource call to rollback path for new skill insert failures.

When a new skill insert succeeds but upsertSkillVector fails, the recovery path deletes the skill via deleteMcpSkill but leaves the entity_sources row created by ensureEntitySource. On retry, a new UUID is generated for the skill (line 127: crypto.randomUUID()), so ensureEntitySource won't reuse the orphaned row—it will create another one, leaking both the old entity_sources entry and its synced artifacts in the repo. Import deleteEntitySource and call it in the else block (line 235) before deleteMcpSkill:

} else {
+  await deleteEntitySource(ctx.env.APP_DB, {
+    id: source.id,
+    userId: user.userId,
+  })
   await deleteMcpSkill(
     ctx.env.APP_DB,
     user.userId,
     prep.rowPayload.name,
   )
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/meta/meta-save-skill.ts` around lines
199 - 241, The rollback path after a failed upsertSkillVector leaves an orphaned
entity_sources row created earlier by ensureEntitySource because retries
generate a new skill id via crypto.randomUUID(); to fix, import
deleteEntitySource and invoke deleteEntitySource(ctx.env.APP_DB, user.userId,
prep.rowPayload.source_id or the source id used) in the else branch just before
calling deleteMcpSkill in meta-save-skill.ts so the entity_sources row is
removed alongside the MCP skill; ensure the call uses the same DB env and userId
as deleteMcpSkill and matches the source identifier that ensureEntitySource
created.
packages/worker/src/mcp/capabilities/apps/ui-save-app.ts (1)

188-274: ⚠️ Potential issue | 🟠 Major

Runner/vector failures leave the published source snapshot orphaned.

The update path publishes the source snapshot (syncArtifactSourceSnapshot) before configureSavedAppRunner runs in saveAndIndexApp. If runner configuration fails, the artifact row is rolled back to existingApp values, but the repo commit recorded by syncArtifactSourceSnapshot is not rolled back — the source now advertises a commit whose corresponding DB state was reverted. The insert path has the same issue: on failure we delete the artifact row and runner but leave the newly-synced source/commit behind.

Given the PR's goal of making saved writes source-aware, consider either (a) reordering so runner+vector configuration happen before source sync/publish, or (b) tracking the synced commit and explicitly compensating (discard/rewind) on the error paths.

Also applies to: 340-358, 398-412

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/apps/ui-save-app.ts` around lines 188 -
274, The source snapshot commit produced by syncArtifactSourceSnapshot can
remain published when subsequent steps (configureSavedAppRunner,
upsertUiArtifactVector) fail, leaving an orphaned repo commit; fix by either
moving syncArtifactSourceSnapshot to run after
configureSavedAppRunner/upsertUiArtifactVector or by capturing the commit id
returned by syncArtifactSourceSnapshot and performing a compensating cleanup on
error paths (the catch blocks that currently call deleteSavedAppRunner and
deleteUiArtifact/deleteUiArtifactVector); specifically update the flows around
configureSavedAppRunner, deleteSavedAppRunner, upsertUiArtifactVector, and
deleteUiArtifactVector to either reorder operations or call a rollback API to
discard/rewind the synced commit when runner/vector setup fails.
packages/worker/src/jobs/service.ts (1)

96-122: ⚠️ Potential issue | 🟡 Minor

Misleading error message when both code and sourceId are absent.

Line 115 still reads 'Jobs require code.', but the type surface now accepts repo-backed jobs via sourceId. The check itself is correct; the user-facing message should reflect the new option so callers don't think code is the only valid update path.

🐛 Proposed fix
-	if (!nextCode && !nextSourceId) {
-		throw new Error('Jobs require code.')
-	}
+	if (!nextCode && !nextSourceId) {
+		throw new Error('Jobs require either code or a sourceId.')
+	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/jobs/service.ts` around lines 96 - 122, The error thrown
in resolveUpdatedShape is misleading: when both nextCode and nextSourceId are
absent it currently throws "Jobs require code." Update the thrown Error message
in resolveUpdatedShape to mention that either code or sourceId is required
(e.g., "Jobs require either code or sourceId.") so callers understand
repo-backed jobs via sourceId are valid; locate the check that uses nextCode and
nextSourceId and replace the message accordingly.
packages/worker/src/mcp/tools/search.ts (1)

552-558: ⚠️ Potential issue | 🟡 Minor

entity schema .describe(...) text is missing job.

The tool's prose description (Lines 127-138) was updated to announce job, but the Zod schema's .describe(...) on Line 557 still lists only capability, skill, app, secret, value, or connector. Clients that introspect the input schema will see an inconsistent contract.

🐛 Proposed fix
-					'Optional exact entity reference in the format "{id}:{type}" where type is capability, skill, app, secret, value, or connector.',
+					'Optional exact entity reference in the format "{id}:{type}" where type is capability, skill, app, job, secret, value, or connector.',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/tools/search.ts` around lines 552 - 558, The Zod
schema for the entity field (the z.string().min(1).optional() chain for the
entity property) has an outdated .describe(...) text that omits "job"; update
the .describe(...) string on the entity schema to list "job" among the allowed
types (e.g., "capability, skill, app, secret, value, connector, or job") so the
schema prose matches the updated tool description and consumers that introspect
the schema see a consistent contract.
packages/worker/src/mcp/tools/search-format.ts (1)

278-282: ⚠️ Potential issue | 🟡 Minor

Error message missing job in the accepted-types list.

Line 295 was updated to include job, but the format-error on Line 280 (thrown when : is missing/misplaced) still lists only capability, skill, app, secret, value, or connector. For consistency with the type union and the other error message, add job here too.

🐛 Proposed fix
-			'Entity must use the format "{id}:{type}" where type is capability, skill, app, secret, value, or connector.',
+			'Entity must use the format "{id}:{type}" where type is capability, skill, app, job, secret, value, or connector.',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/tools/search-format.ts` around lines 278 - 282, The
thrown Error inside the conditional "if (separator <= 0 || separator ===
trimmed.length - 1)" in search-format.ts lists accepted types but omits "job";
update that error string to match the updated union and other error messages by
adding "job" to the list so it reads something like '{id}:{type}' where type is
capability, skill, app, secret, value, connector, or job.
🟡 Minor comments (19)
packages/worker/wrangler.jsonc-8-13 (1)

8-13: ⚠️ Potential issue | 🟡 Minor

Schema is valid, but consider moving artifacts into each env block for consistency.

The artifacts binding schema is correct per Cloudflare documentation (binding, namespace fields, "default" namespace value all valid). However, all other bindings in this config (ai, worker_loaders, assets, durable_objects, d1_databases, kv_namespaces, vectorize) are defined within env.production, env.preview, and env.test blocks. Placing artifacts at the top-level while everything else is environment-specific breaks the architectural pattern and may cause confusion during maintenance. Move the artifacts binding into each environment block for parity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/wrangler.jsonc` around lines 8 - 13, Move the top-level
"artifacts" binding into each environment block so it matches the pattern used
for other bindings; remove the top-level "artifacts" array and add an
"artifacts" entry with the same object ({ "binding": "ARTIFACTS", "namespace":
"default" }) inside env.production, env.preview, and env.test (matching the
existing structure used for ai, worker_loaders, assets, durable_objects,
d1_databases, kv_namespaces, vectorize) so each env contains its own artifacts
binding.
packages/worker/src/agent-turn/tools.ts-65-69 (1)

65-69: ⚠️ Potential issue | 🟡 Minor

Add a test for loadJobs failure to match coverage for other loaders.

The loadOptionalSearchRows function properly isolates loadJobs errors using Promise.allSettled() and the warning pattern (lines 294–302 of search.ts), matching the error handling for skills, artifacts, and values. However, the test file lacks a symmetric test for loadJobs failure—tests exist for skills (line 43), apps (line 76), and values (line 96), but not for jobs. Add a test case to verify that a loadJobs rejection generates the "Saved jobs are temporarily unavailable" warning and doesn't poison the search result.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/agent-turn/tools.ts` around lines 65 - 69, Add a unit
test that simulates `listJobs` (used by `loadJobs` inside
`loadOptionalSearchRows`) rejecting and asserts that the rejection produces the
warning message "Saved jobs are temporarily unavailable" and does not break the
overall search result; mock `listJobs` to return a rejected promise, call
`loadOptionalSearchRows` (or the search helper used by existing tests for
skills/apps/values), then assert the warning was recorded (or the warnings array
contains that string) and the search payload is still returned/usable. Ensure
the test mirrors the pattern and assertions used in the existing
skills/apps/values test cases so coverage and behavior are symmetric.
packages/worker/src/job-maintenance.ts-20-22 (1)

20-22: ⚠️ Potential issue | 🟡 Minor

Minor: consider constant-time token comparison.

Bearer token is compared with !==, which is not constant-time. For a maintenance endpoint behind a shared secret this is typically fine on Workers, but a timing-safe comparison (e.g., hashing both sides and comparing, or subtle/crypto-based) is a cheap hardening win if this endpoint is reachable from the public internet.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/job-maintenance.ts` around lines 20 - 22, The bearer
token comparison using `token !== secret` is susceptible to timing attacks;
replace it with a constant-time comparison: compute a fixed-length digest for
both `token` and `secret` (e.g., SHA-256 via Web Crypto `crypto.subtle.digest`)
and compare the resulting ArrayBuffers/Uint8Arrays byte-by-byte in constant time
(or use a timing-safe helper like `timingSafeEqual`) before returning `new
Response('Unauthorized', { status: 401 })` when they do not match; update the
check in the `job-maintenance.ts` handler where `token` and `secret` are
compared.
packages/worker/src/mcp/generated-ui-api.ts-145-149 (1)

145-149: ⚠️ Potential issue | 🟡 Minor

Unhandled failures from resolveSavedAppSource.

This call is outside the existing try/catch (lines 151–169), so any failure in source resolution (e.g., transient repo/RPC error) will bubble up as an unhandled router 500 instead of a structured JSON error. Previously, parameter parsing errors were reported via a 400 JSON response; consider wrapping this call similarly so clients receive a useful payload, and distinguish between "can't load source" (likely 502/500) and "bad params" (400).

♻️ Suggested handling
-			const resolvedApp = await resolveSavedAppSource({
-				env,
-				baseUrl: getAppBaseUrl({ env, requestUrl: request.url }),
-				artifact: app,
-			})
+			let resolvedApp
+			try {
+				resolvedApp = await resolveSavedAppSource({
+					env,
+					baseUrl: getAppBaseUrl({ env, requestUrl: request.url }),
+					artifact: app,
+				})
+			} catch (error) {
+				return jsonResponse(
+					{
+						error:
+							error instanceof Error
+								? error.message
+								: 'Unable to load saved UI source.',
+					},
+					502,
+				)
+			}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/generated-ui-api.ts` around lines 145 - 149, The call
to resolveSavedAppSource is unwrapped and can throw, causing unhandled 500s;
wrap the resolveSavedAppSource({... baseUrl: getAppBaseUrl({ env, requestUrl:
request.url }), artifact: app }) invocation in a try/catch like the existing
block handling parameter parsing, and return a structured JSON error instead of
letting it bubble: catch parameter/validation errors and respond with a 400 JSON
payload, and for source loading or RPC/repo failures respond with a 502/500 JSON
payload (include a clear message and any safe error details), matching the
existing response shape used elsewhere in this file.
packages/worker/src/mcp/capabilities/meta/meta-save-skill.ts-190-197 (1)

190-197: ⚠️ Potential issue | 🟡 Minor

Non-atomic update sequence across entity_sources and skills.

If updateEntitySource succeeds but the subsequent vector upsert fails and triggers the rollback branch below, the entity source's publishedCommit/indexedCommit will have moved forward while the skill data has been reverted (or deleted). On next save/re-run, syncArtifactSourceSnapshot may think the source is already current and skip re-publishing. Consider moving the updateEntitySource call after the successful upsertSkillVector, or otherwise making the commit pointer update conditional on the full flow succeeding.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/meta/meta-save-skill.ts` around lines
190 - 197, The update to entity_sources is happening before the vector upsert,
creating a non-atomic state if upsertSkillVector fails; move the
updateEntitySource call (the publishedCommit/indexedCommit write) to after a
successful upsertSkillVector, or wrap both operations in a single
transactional/conditional flow so the commit pointer is only advanced when
upsertSkillVector completes successfully (ensure references to
updateEntitySource, upsertSkillVector and the rollback
branch/syncArtifactSourceSnapshot logic are updated accordingly).
packages/worker/src/mcp/capabilities/repo/repo-run-checks.ts-17-19 (1)

17-19: ⚠️ Potential issue | 🟡 Minor

readOnly: true likely misrepresents repo_run_checks.

Running the Worker-native validation pipeline persists check-run metadata (tree_hash, results, checked_at) on the session — that's precisely what repo_get_check_status reads back. This capability also performs meaningful compute, so marking it readOnly: true and idempotent: true will cause callers/routers that rely on capability flags (e.g., for caching, retry semantics, or gating in higher-level agents) to treat it as a free-side-effect query.

Suggest readOnly: false; idempotent is debatable but at minimum the read-only claim is incorrect.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/repo/repo-run-checks.ts` around lines 17
- 19, The capability metadata for repo_run_checks incorrectly declares readOnly:
true; update the capability descriptor for repo_run_checks to set readOnly:
false (leave idempotent as-is or discuss separately) so callers/routers don't
treat this operation as a no-side-effect query—locate the capability object
named repo_run_checks in repo-run-checks.ts and change the readOnly flag to
false while keeping destructive and idempotent settings consistent with current
semantics.
packages/worker/src/mcp/capabilities/repo/repo-open-session.ts-30-41 (1)

30-41: ⚠️ Potential issue | 🟡 Minor

Existing-session resume ignores args.source_id mismatch.

If the caller passes source_id=A with conversation_id=C but the active session bound to that conversation was opened for source_id=B, this silently returns the session for B and drops A on the floor. That's easy to misuse when a client rotates between sources within the same conversation. Consider either (a) validating existingSession.source_id === args.source_id and erroring on mismatch, or (b) explicitly documenting in the capability description that conversation-bound resume takes precedence over source_id.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/repo/repo-open-session.ts` around lines
30 - 41, The current resume logic returns an existing session found by
getActiveRepoSessionByConversation without checking args.source_id, allowing a
session opened for a different source to be returned; update the logic in
repo-open-session.ts to compare existingSession.source_id with args.source_id
and, if they differ, reject (throw an error or return an explicit RPC error)
instead of silently returning the mismatched session; ensure the check occurs
before calling repoSessionRpc(...).getSessionInfo and include existingSession,
args.source_id, and conversation id in the error context for clarity.
packages/worker/src/mcp/capabilities/apps/ui-save-app.ts-164-171 (1)

164-171: ⚠️ Potential issue | 🟡 Minor

ensureEntitySource runs before the update-path existence check.

When isUpdate is true and the artifact doesn't belong to the user, we throw at line 293, but ensureEntitySource has already created a new entity_sources row (or bound a fresh one). Moving ensureEntitySource into each branch — after the existence check for updates, and where the new appId is committed for creates — avoids leaving orphan entity-source rows on auth/lookup failures.

Also applies to: 286-294

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/apps/ui-save-app.ts` around lines 164 -
171, ensureEntitySource is being called unconditionally before verifying update
permissions, which creates/binds an entity_sources row even when the subsequent
existence/auth checks fail; move the ensureEntitySource call into each branch so
it runs only after checks pass: for the update path (when isUpdate is true)
first verify the artifact/app belongs to the user (the same logic that currently
throws in the update branch), then call ensureEntitySource({ db: ctx.env.APP_DB,
env: ctx.env, userId: user.userId, entityKind: 'app', entityId: appId,
sourceRoot: '/' }); for the create path call ensureEntitySource only after the
new appId has been committed/created; update references to ensureEntitySource in
the save flow (including the code around the current isUpdate check and the
create-commit path) so orphan entity_sources rows are not created on auth/lookup
failure.
packages/worker/src/mcp/capabilities/repo/repo-tree.ts-85-97 (1)

85-97: ⚠️ Potential issue | 🟡 Minor

total_bytes aggregates sizes for directories and symlinks.

totals.total_bytes += current.size runs unconditionally, so any non-zero size reported for a directory or symlink by the RPC contributes to the total. If directories ever report an aggregate size (e.g. recursive total or a link target size) this will double-count or produce a misleading byte total. Safer to only accumulate file sizes.

🛠️ Proposed fix
 		totals.max_depth = Math.max(totals.max_depth, depth)
-		totals.total_bytes += current.size
 		switch (current.type) {
 			case 'file':
 				totals.files += 1
+				totals.total_bytes += current.size
 				break
 			case 'directory':
 				totals.directories += 1
 				break
 			case 'symlink':
 				totals.symlinks += 1
 				break
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/repo/repo-tree.ts` around lines 85 - 97,
The totals.total_bytes is being increased for every node type, causing
directories or symlinks with non-zero size to be incorrectly counted; modify the
aggregation so totals.total_bytes += current.size only runs for file nodes
(i.e., inside the 'file' case or guarded by if (current.type === 'file')),
leaving totals.files, totals.directories, and totals.symlinks updates unchanged
(reference the totals and current variables and the switch on current.type in
repo-tree.ts).
packages/worker/src/mcp/ui-artifacts-repo.ts-27-28 (1)

27-28: ⚠️ Potential issue | 🟡 Minor

Inconsistent null handling for clientCode: type says string, but writes allow null and reads coerce to empty string.

The UiArtifactRow.clientCode type is declared as string (non-nullable), yet INSERT (line 28) and UPDATE (line 126) both write clientCode ?? null, allowing null values into the database. On read, mapRow coerces null back to an empty string (String(row['client_code'] ?? '')), creating an asymmetry: saving null → storing null → reading back as ''.

This violates the type contract and silently hides missing values. Consumers expecting a string per the type will see empty strings instead of being able to distinguish between "not set" and "legitimately empty".

Align by either:

  • Changing UiArtifactRow.clientCode to string | null and having mapRow return null (so callers can fall through to repo-backed source resolution), or
  • Keeping it non-nullable and rejecting null writes in INSERT/UPDATE.

Also applies to lines 126 and 178.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/ui-artifacts-repo.ts` around lines 27 - 28,
UiArtifactRow.clientCode should be nullable to match the DB writes and reads:
change the UiArtifactRow type to clientCode: string | null, update mapRow in
ui-artifacts-repo.ts to return row['client_code'] ?? null (do not coerce to an
empty string), and keep the INSERT/UPDATE code (the places currently using
clientCode ?? null) as-is so nulls are persisted; also audit callers of
UiArtifactRow to handle string | null accordingly (particularly code paths that
rely on repo-backed source resolution).
packages/worker/src/repo/source-sync.ts-12-20 (1)

12-20: ⚠️ Potential issue | 🟡 Minor

canSyncArtifactSource should also verify APP_DB.

The guard returns true as long as ARTIFACTS and REPO_SESSION are bound, but line 31 unconditionally reads input.env.APP_DB. In any environment missing the DB binding, this will throw rather than fall back to the return null semantics the rest of the guard promises.

♻️ Proposed fix
 function canSyncArtifactSource(env: Env) {
 	return (
 		typeof (env as Env & { ARTIFACTS?: unknown }).ARTIFACTS === 'object' &&
 		(env as Env & { ARTIFACTS?: unknown }).ARTIFACTS != null &&
 		typeof (env as Env & { REPO_SESSION?: unknown }).REPO_SESSION ===
 			'object' &&
-		(env as Env & { REPO_SESSION?: unknown }).REPO_SESSION != null
+		(env as Env & { REPO_SESSION?: unknown }).REPO_SESSION != null &&
+		(env as Env & { APP_DB?: unknown }).APP_DB != null
 	)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/repo/source-sync.ts` around lines 12 - 20,
canSyncArtifactSource currently only checks ARTIFACTS and REPO_SESSION but code
later reads input.env.APP_DB unconditionally; update canSyncArtifactSource to
also verify APP_DB is present and non-null (e.g. check typeof (env as Env & {
APP_DB?: unknown }).APP_DB === 'object' && (env as Env & { APP_DB?: unknown
}).APP_DB != null) so callers can safely access input.env.APP_DB without
throwing; use the same casting pattern used for ARTIFACTS and REPO_SESSION to
locate the change in the function.
packages/worker/src/mcp/capabilities/repo/repo-publish-session.ts-49-56 (1)

49-56: ⚠️ Potential issue | 🟡 Minor

Remove unnecessary empty-string fallback; RPC contract guarantees non-null.

When status === 'ok', the type definition guarantees publishedCommit is a string (non-nullable). The ?? '' is dead code since RepoSessionPublishResult explicitly types it as non-null for the ok variant. Either remove the coalescing operator or add an explicit assertion if the contract changes in future.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/repo/repo-publish-session.ts` around
lines 49 - 56, Remove the unnecessary fallback when constructing the success
return: the RPC contract's ok variant (RepoSessionPublishResult) guarantees
publishedCommit is a non-null string, so drop the "?? ''" coalescing on
result.publishedCommit and return result.publishedCommit directly (or, if you
prefer to be explicit, use a non-null assertion on result.publishedCommit).
Update the return in the branch checking result.status === 'ok' to use
result.publishedCommit as-is and ensure types still satisfy the function's
return shape.
packages/worker/src/repo/checks.ts-146-150 (1)

146-150: ⚠️ Potential issue | 🟡 Minor

Lint check unconditionally reports "passed".

The 'lint' result is hard-coded ok: true with a placeholder message. As-is, operators looking at check output can't distinguish "lint is intentionally unimplemented" from "lint actually passed", and the aggregate ok on Line 162 will never reflect lint failures. At minimum, add a TODO/issue reference; ideally flip the message to something like Lint check is not yet implemented. so it's obvious in any UI.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/repo/checks.ts` around lines 146 - 150, The lint result
currently pushed via results.push({ kind: 'lint', ok: true, ... }) is a
hard-coded placeholder; change it to clearly indicate the check is unimplemented
by setting ok: false (or at least not true) and updating the message to
something like "Lint check is not yet implemented." also add a TODO or issue
reference string in the message so operators can tell this is intentional;
locate the results.push call for kind 'lint' and update the ok and message
fields so the aggregate ok calculation that consumes results reflects the
unimplemented state.
packages/worker/src/repo/entity-sources.ts-97-133 (1)

97-133: ⚠️ Potential issue | 🟡 Minor

The .bind() arity is correct, but add test coverage for partial updates.

The dynamic SET clause construction properly matches the bind arguments: values contains exactly one entry per SET placeholder (from the conditional add() calls plus the always-run updated_at), and the two WHERE placeholders (id and user_id) are correctly appended. However, this function currently has no test coverage. Since production code exercises partial updates with different field combinations (see usages in source-service.ts, meta-save-skill.ts, and repo-session-do.ts), add at least one test verifying a single-field partial update to prevent regressions if fields are added or removed later.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/repo/entity-sources.ts` around lines 97 - 133, Add a
unit/integration test that exercises updateEntitySource with a partial update
(e.g., only publishedCommit or only manifestPath) to ensure the dynamic SET
clause and bind ordering are correct; specifically, call updateEntitySource(...)
with a single optional field changed and verify the DB row is updated (result
true and the specific column changed while others remain unchanged). Target the
updateEntitySource function in packages/worker/src/repo/entity-sources.ts and
mirror realistic call patterns similar to usages in source-service.ts,
meta-save-skill.ts, or repo-session-do.ts; seed a test row, invoke
updateEntitySource with just one field, then read the row back to assert only
that field and updated_at changed.
packages/worker/src/repo/source-templates.ts-50-51 (1)

50-51: ⚠️ Potential issue | 🟡 Minor

Duplicating job.name as both title and description.

description: input.job.name duplicates the title. manifestBaseSchema.description requires min(1), and consumers use both fields independently for search/ranking, so they'll double-weight the same string. Consider using scheduleSummary or a stored job description field.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/repo/source-templates.ts` around lines 50 - 51, The
manifest template currently sets both title and description to the same value
(title: input.job.name, description: input.job.name), which duplicates content
and skews search/ranking; update the description field in the source-templates
generation (where manifestBaseSchema is populated) to use a different source
such as input.job.scheduleSummary or a dedicated job description field instead
of input.job.name, ensuring manifestBaseSchema.description still meets min(1)
validation and consumers receive distinct title and description values.
packages/worker/src/mcp/capabilities/unified-search.ts-388-396 (1)

388-396: ⚠️ Potential issue | 🟡 Minor

Job description is hardcoded to row.name — drop it or source from a real field.

Both buildJobEmbedDoc (line 391) and rowToJobHit (line 516) emit description: row.name. As a result:

  • The embed doc and hit artifacts contain the job name twice, so lexical/embed ranking is effectively double-weighted on name.
  • The rendered hit shows identical title and description to users/LLM callers.

If JobView has no real description field, consider using scheduleSummary as the description fallback (it's already the most meaningful secondary text) or leaving it as an empty string instead of duplicating the name.

♻️ Proposed change
 function buildJobEmbedDoc(row: JobView): string {
 	return buildJobEmbedText({
 		name: row.name,
-		description: row.name,
+		description: row.scheduleSummary ?? '',
 		scheduleSummary: row.scheduleSummary,
 		sourceId: row.sourceId,
 		publishedCommit: row.publishedCommit,
 	})
 }
@@
 	return {
 		type: 'job',
 		jobId: row.id,
 		domain: 'jobs',
 		title: row.name,
-		description: row.name,
+		description: row.scheduleSummary ?? '',
 		scheduleSummary: row.scheduleSummary,

Also applies to: 505-526

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/unified-search.ts` around lines 388 -
396, The description field is incorrectly duplicated from the job name in
buildJobEmbedDoc and rowToJobHit; update both places (functions buildJobEmbedDoc
and rowToJobHit dealing with JobView) to stop setting description: row.name and
instead set description to a meaningful fallback such as row.scheduleSummary (or
'' if scheduleSummary is missing). Ensure the embed/doc and hit artifacts use
description: row.scheduleSummary ?? '' (or equivalent) so the title and
description are not identical.
packages/worker/src/repo/repo-session-do.ts-541-586 (1)

541-586: ⚠️ Potential issue | 🟡 Minor

truncated signal can be wrong.

searchInText breaks early on each file when matches.length >= input.maxMatches, but never returns a signal that it hit the cap. At the caller:

truncated: totalMatches > maxMatches
  • If a single file had exactly maxMatches + k real matches but we stopped at maxMatches, and no other file matched, totalMatches === maxMatches → truncated = false even though we dropped hits.
  • Conversely, totalMatches > maxMatches only reflects aggregate volume, not the per-file truncation that actually occurred.

Make searchInText return a boolean truncated (or the pre-cap count), and OR-reduce it across files.

🛠️ Suggested shape
-		const matches = searchInText({ ... })
+		const { matches, truncated: fileTruncated } = searchInText({ ... })
 		if (matches.length === 0) continue
+		anyFileTruncated ||= fileTruncated
 		...
 		totalMatches += matches.length
 	}
 	...
-		truncated: totalMatches > maxMatches,
+		truncated: anyFileTruncated,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/repo/repo-session-do.ts` around lines 541 - 586, search
truncation is currently unnoticed because searchInText stops per-file at
maxMatches but doesn't report that, so change searchInText to return both the
matches array and a boolean truncated flag (or the raw pre-cap count); then in
repo-session-do.ts where you call searchInText (the block that populates
matches, matchMap, totalMatches and uses maxMatches), capture the per-file
truncated value and OR-reduce it into an overall truncated variable (e.g., let
truncated = false; truncated ||= fileTruncated), and use that final truncated in
the returned object instead of computing truncated: totalMatches > maxMatches;
keep existing names (searchInText, matches, maxMatches, totalMatches, matchMap)
so the change is easy to locate.
packages/worker/src/repo/artifacts.ts-125-131 (1)

125-131: ⚠️ Potential issue | 🟡 Minor

buildAuthenticatedArtifactsRemote silently corrupts non-https remotes.

input.remote.slice('https://'.length) unconditionally strips 8 characters. If remote is http://... or lacks a scheme (e.g., a relative/local test URL), the result is a malformed URL like https://x:TOKEN@ttp://host/... and downstream git clone/push operations will fail in confusing ways.

🛠️ Proposed hardening
 export function buildAuthenticatedArtifactsRemote(input: {
 	remote: string
 	token: string
 }) {
 	const tokenSecret = parseArtifactTokenSecret(input.token)
-	return `https://x:${tokenSecret}@${input.remote.slice('https://'.length)}`
+	if (!input.remote.startsWith('https://')) {
+		throw new Error(`Artifact remote must use https://, got: ${input.remote}`)
+	}
+	return `https://x:${tokenSecret}@${input.remote.slice('https://'.length)}`
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/repo/artifacts.ts` around lines 125 - 131, The function
buildAuthenticatedArtifactsRemote currently blindly slices off 8 characters
which corrupts non-https remotes; instead, parse the token with
parseArtifactTokenSecret(token) into tokenSecret and construct the authenticated
remote robustly (use the URL parser or conditional logic) so you only
strip/insert the auth part when the scheme is present: for a proper URL create a
URL object from input.remote, set url.username = 'x' and url.password =
tokenSecret and return url.toString(), and for schemeless strings prepend
"https://x:tokenSecret@" or otherwise handle "http://" by replacing its scheme
with https and inserting the credentials. Ensure you update the return from
buildAuthenticatedArtifactsRemote and keep parseArtifactTokenSecret referenced.
packages/worker/src/mcp/capabilities/repo/repo-shared.ts-267-276 (1)

267-276: ⚠️ Potential issue | 🟡 Minor

repoRunChecksOutputSchema intentionally omits runId, treeHash, and checkedAt.

The RepoSessionCheckRun type returns these fields, but the handler explicitly strips them and only returns ok, results, and manifest. If these fields are valuable to callers, consider adding them to the schema and handler return value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/mcp/capabilities/repo/repo-shared.ts` around lines 267 -
276, repoRunChecksOutputSchema currently only validates ok, results, and
manifest while RepoSessionCheckRun includes runId, treeHash, and checkedAt;
update repoRunChecksOutputSchema to include runId (string), treeHash (string),
and checkedAt (string or Date ISO) with the same types used by
RepoSessionCheckRun and adjust the handler that returns the check run to include
these fields in its response so the runtime return shape matches the schema and
RepoSessionCheckRun type.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7bae31ae-94a8-47cb-b8ce-48e474caefbf

📥 Commits

Reviewing files that changed from the base of the PR and between 87a495a and 1808a6a.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (69)
  • packages/shared/src/chat.ts
  • packages/worker/migrations/0023-entity-sources.sql
  • packages/worker/migrations/0024-repo-sessions-and-source-columns.sql
  • packages/worker/package.json
  • packages/worker/src/agent-turn/tools.ts
  • packages/worker/src/app/handlers/saved-ui.ts
  • packages/worker/src/index.ts
  • packages/worker/src/job-maintenance.ts
  • packages/worker/src/jobs/job-reindex.ts
  • packages/worker/src/jobs/repo.ts
  • packages/worker/src/jobs/service.node.test.ts
  • packages/worker/src/jobs/service.ts
  • packages/worker/src/jobs/types.ts
  • packages/worker/src/mcp/app-runner.ts
  • packages/worker/src/mcp/capabilities/apps/ui-get-app.ts
  • packages/worker/src/mcp/capabilities/apps/ui-load-app-source.ts
  • packages/worker/src/mcp/capabilities/apps/ui-save-app.node.test.ts
  • packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
  • packages/worker/src/mcp/capabilities/builtin-domains.ts
  • packages/worker/src/mcp/capabilities/domain-metadata.ts
  • packages/worker/src/mcp/capabilities/jobs/shared.ts
  • packages/worker/src/mcp/capabilities/meta/meta-save-skill.ts
  • packages/worker/src/mcp/capabilities/repo/domain.ts
  • packages/worker/src/mcp/capabilities/repo/index.ts
  • packages/worker/src/mcp/capabilities/repo/repo-apply-patch.ts
  • packages/worker/src/mcp/capabilities/repo/repo-discard-session.ts
  • packages/worker/src/mcp/capabilities/repo/repo-get-check-status.ts
  • packages/worker/src/mcp/capabilities/repo/repo-get-session.ts
  • packages/worker/src/mcp/capabilities/repo/repo-open-session.ts
  • packages/worker/src/mcp/capabilities/repo/repo-publish-session.ts
  • packages/worker/src/mcp/capabilities/repo/repo-read-file.ts
  • packages/worker/src/mcp/capabilities/repo/repo-rebase-session.ts
  • packages/worker/src/mcp/capabilities/repo/repo-run-checks.ts
  • packages/worker/src/mcp/capabilities/repo/repo-search.ts
  • packages/worker/src/mcp/capabilities/repo/repo-shared.ts
  • packages/worker/src/mcp/capabilities/repo/repo-tree.ts
  • packages/worker/src/mcp/capabilities/repo/repo-write-file.ts
  • packages/worker/src/mcp/capabilities/unified-search.ts
  • packages/worker/src/mcp/context.node.test.ts
  • packages/worker/src/mcp/context.ts
  • packages/worker/src/mcp/executor.ts
  • packages/worker/src/mcp/generated-ui-api.ts
  • packages/worker/src/mcp/jobs-embed.ts
  • packages/worker/src/mcp/jobs-vectorize.ts
  • packages/worker/src/mcp/observability.workers.test.ts
  • packages/worker/src/mcp/run-codemode-registry.ts
  • packages/worker/src/mcp/skills/mcp-skills-repo.ts
  • packages/worker/src/mcp/skills/mcp-skills-types.ts
  • packages/worker/src/mcp/skills/run-saved-skill.ts
  • packages/worker/src/mcp/tools/search-format.ts
  • packages/worker/src/mcp/tools/search.node.test.ts
  • packages/worker/src/mcp/tools/search.ts
  • packages/worker/src/mcp/ui-artifacts-repo.ts
  • packages/worker/src/mcp/ui-artifacts-types.ts
  • packages/worker/src/oauth-handlers-skill-runner.workers.test.ts
  • packages/worker/src/oauth-handlers.ts
  • packages/worker/src/repo/app-source.ts
  • packages/worker/src/repo/artifacts.ts
  • packages/worker/src/repo/checks.ts
  • packages/worker/src/repo/entity-sources.ts
  • packages/worker/src/repo/manifest.ts
  • packages/worker/src/repo/repo-session-do.ts
  • packages/worker/src/repo/repo-sessions.ts
  • packages/worker/src/repo/source-service.ts
  • packages/worker/src/repo/source-sync.ts
  • packages/worker/src/repo/source-templates.ts
  • packages/worker/src/repo/types.ts
  • packages/worker/worker-configuration.d.ts
  • packages/worker/wrangler.jsonc

Comment on lines +45 to +53
const texts = batch.map(({ job }) =>
buildJobEmbedText({
name: job.name,
description: job.name,
scheduleSummary: job.scheduleSummary,
sourceId: job.sourceId,
publishedCommit: job.publishedCommit,
}),
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm buildJobEmbedText's expected inputs and how the primary write path calls it.
fd -t f 'jobs-embed.ts' | xargs -I{} sed -n '1,120p' {}
rg -nP -C3 '\bbuildJobEmbedText\s*\('

Repository: kentcdodds/kody

Length of output: 3613


🏁 Script executed:

#!/bin/bash
# Check the Job type definition to see if description field exists
rg -nP 'type Job|interface Job' --max-count=10
rg -nP '\bjob\.description\b'
# Also check if any entity has a description field
rg -nP '\bdescription:' packages/worker/src/jobs/types.ts -A2 -B2

Repository: kentcdodds/kody

Length of output: 3031


description is wired to job.name — embedding input is wrong.

description: job.name duplicates the name into the description slot. Job objects have a description field that should be used instead. This corrupts embeddings across all reindex, create, and update paths, causing search quality to degrade silently.

🐛 Proposed fix
 		const texts = batch.map(({ job }) =>
 			buildJobEmbedText({
 				name: job.name,
-				description: job.name,
+				description: job.description,
 				scheduleSummary: job.scheduleSummary,
 				sourceId: job.sourceId,
 				publishedCommit: job.publishedCommit,
 			}),
 		)

Note: The same bug exists in packages/worker/src/jobs/service.ts (lines 196 and 315) and packages/worker/src/mcp/capabilities/unified-search.ts (line 391). All call sites pass entity name to description instead of the actual description field.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/worker/src/jobs/job-reindex.ts` around lines 45 - 53, The embedding
input incorrectly sets description: job.name when calling buildJobEmbedText in
job-reindex.ts; change it to description: job.description so the actual job
description is embedded (also search for other call sites like the
buildJobEmbedText calls in jobs/service.ts and
mcp/capabilities/unified-search.ts where description is similarly passed as
job.name and fix them to use job.description). Ensure you update every
invocation of buildJobEmbedText that currently uses job.name for the description
parameter so embeddings reflect the true description field.

Comment thread packages/worker/src/jobs/repo.ts Outdated
Comment thread packages/worker/src/jobs/service.ts
Comment thread packages/worker/src/jobs/service.ts
Comment thread packages/worker/src/mcp/capabilities/apps/ui-save-app.ts Outdated
Comment thread packages/worker/src/repo/repo-session-do.ts Outdated
Comment thread packages/worker/src/repo/source-service.ts
Comment thread packages/worker/src/repo/source-sync.ts Outdated
Comment thread packages/worker/worker-configuration.d.ts
Comment thread packages/worker/worker-configuration.d.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Comment thread packages/worker/src/jobs/service.ts
Comment thread packages/worker/src/mcp/capabilities/repo/domain.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Comment thread packages/worker/src/repo/app-source.ts Outdated
Comment thread packages/worker/src/mcp/capabilities/repo/repo-apply-patch.ts Outdated
Comment thread packages/worker/src/mcp/capabilities/meta/meta-save-skill.ts Outdated
Comment thread packages/worker/src/mcp/capabilities/apps/ui-save-app.ts
Comment thread packages/worker/src/repo/repo-session-do.ts
Move repo session search, RPC typing, and shared session result types into focused modules so future changes stay local and easier to review.

Made-with: Cursor
Move repo session path, tree, and session-info shaping into a focused helper module so the session DO stays centered on orchestration.

Made-with: Cursor

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/worker/src/jobs/service.ts`:
- Around line 253-272: The updateJob flow allows swapping sourceId without
validation because ensureEntitySource is only called when shape.sourceId &&
!existing.sourceId; change this to run whenever the source is changing
(shape.sourceId && shape.sourceId !== existing.sourceId) or alternatively reject
updates that attempt to change an existing source; update the conditional that
calls ensureEntitySource (referencing ensureEntitySource, shape.sourceId,
existing.sourceId) so ensuredSource is produced for changes and ensure
updated.sourceId (and subsequent calls to syncArtifactSourceSnapshot and
upsertJobVector) only use a vetted ensuredSource or the original
existing.sourceId.

In `@packages/worker/src/repo/repo-session-do.ts`:
- Around line 636-640: runRepoChecks is being called with
repoSessionWorkspacePrefix which ignores individual source.source_root and
causes snapshot keys to be misaligned; resolve source.source_root against the
workspace prefix (the same way manifestPath is resolved earlier) to compute an
effective sourceRoot and pass that to runRepoChecks instead of
repoSessionWorkspacePrefix. Locate where runRepoChecks is invoked (the call
using manifestPath and repoSessionWorkspacePrefix) and replace the sourceRoot
argument with the resolved path (join/resolve workspace prefix +
source.source_root, handle leading/trailing slashes) so
snapshot.read('package.json') and other reads use the correct relative keys.
- Around line 283-352: In openSession, enforce ownership in both branches: after
retrieving sessionRow from getRepoSessionById, validate sessionRow.user_id ===
input.userId and throw if not before calling
resolveSessionRepo/ensureArtifactRepoRemote/initialize/toRepoSessionInfoResult;
and in the create branch after getEntitySourceById but before
forking/insertRepoSession, validate source.user_id === input.userId and throw if
not to prevent cross-user forks; update the checks adjacent to
getRepoSessionById and getEntitySourceById in the openSession flow.

In `@packages/worker/src/repo/repo-session-search.ts`:
- Around line 155-189: The current code uses maxMatches (from
normalizeSearchLimit(input.limit)) as a per-file cap because maxMatches is
computed once and passed into searchInText for every file; to make limit a
global budget, compute remaining = normalizeSearchLimit(input.limit) before the
files loop, pass remaining (not maxMatches) into searchInText for each file,
after adding matches decrement remaining by the number of matches actually
appended and increment totalMatches, set truncated true if result.truncated OR
if remaining reaches 0, and break out of the outer files loop when remaining <=
0; adjust uses of maxMatches/remaining and references to searchInText, matchMap,
truncated, totalMatches, and the files loop accordingly (alternatively, if you
meant a per-file limit, rename maxMatches to perFileLimit and document it).

In `@packages/worker/src/repo/repo-session-tree.ts`:
- Around line 46-50: The check in resolveRepoWorkspacePath uses
trimmed.startsWith(workspacePrefix) which misclassifies sibling paths (e.g.,
'/session-backup'); update the guard to treat an exact match (trimmed ===
workspacePrefix) or a proper directory prefix
(trimmed.startsWith(workspacePrefix + '/')) so only true children are returned
unchanged; otherwise normalize by returning
`${workspacePrefix}/${trimmed.replace(/^\/+/, '')}`. Ensure you modify the
conditional in resolveRepoWorkspacePath to use these two precise checks instead
of the loose startsWith.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 73a1f3e1-b709-422a-b264-d7a760ffe1a8

📥 Commits

Reviewing files that changed from the base of the PR and between 6fd87ea and 688097d.

📒 Files selected for processing (6)
  • packages/worker/src/jobs/service.ts
  • packages/worker/src/repo/repo-session-do.ts
  • packages/worker/src/repo/repo-session-rpc.ts
  • packages/worker/src/repo/repo-session-search.ts
  • packages/worker/src/repo/repo-session-tree.ts
  • packages/worker/src/repo/types.ts

Comment thread packages/worker/src/jobs/service.ts
Comment thread packages/worker/src/repo/repo-session-do.ts
Comment thread packages/worker/src/repo/repo-session-do.ts
Comment thread packages/worker/src/repo/repo-session-search.ts Outdated
Comment thread packages/worker/src/repo/repo-session-tree.ts Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 4 total unresolved issues (including 2 from previous reviews).

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Deterministic session ID enables concurrent request race condition
    • Generated a unique repo session ID per resolve call to prevent shared sessions and discard races.
  • ✅ Fixed: Job storageId throw on null breaks existing rows without it
    • Fallback to a derived job storage ID when rows lack storage_id to avoid crashes on legacy data.
Preview (ad414e84ac)
diff --git a/package-lock.json b/package-lock.json
--- a/package-lock.json
+++ b/package-lock.json
@@ -678,7 +678,6 @@
       "resolved": "https://registry.npmjs.org/@cloudflare/shell/-/shell-0.3.2.tgz",
       "integrity": "sha512-2UYlhp308IxKWutGcdOJ8+QTR2J1sck108JJ0AEDYG/aNaapKSCrm0Rvd3EFAfqAPtzsqtFqDOKU/4LtpCF8dw==",
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "@cloudflare/codemode": "^0.3.4",
         "isomorphic-git": "^1.37.5"
@@ -1212,6 +1211,38 @@
         "url": "https://github.com/sponsors/colinhacks"
       }
     },
+    "node_modules/@cloudflare/worker-bundler": {
+      "version": "0.1.1",
+      "resolved": "https://registry.npmjs.org/@cloudflare/worker-bundler/-/worker-bundler-0.1.1.tgz",
+      "integrity": "sha512-pwEgMFk7RORJZD7sg0GgnW47zATPAFCJocfiiGCEJ6qjeD+QFENwnp495Nudim1LkPkt0EsERQykNfV/c3gaaA==",
+      "license": "MIT",
+      "dependencies": {
+        "@typescript/vfs": "^1.6.4",
+        "es-module-lexer": "^2.0.0",
+        "esbuild-wasm": "0.28.0",
+        "resolve.exports": "^2.0.3",
+        "semver": "^7.7.4",
+        "smol-toml": "^1.6.1",
+        "sucrase": "^3.35.1",
+        "typescript": "^6.0.2"
+      },
+      "engines": {
+        "node": ">=22"
+      }
+    },
+    "node_modules/@cloudflare/worker-bundler/node_modules/typescript": {
+      "version": "6.0.2",
+      "resolved": "https://registry.npmjs.org/typescript/-/typescript-6.0.2.tgz",
+      "integrity": "sha512-bGdAIrZ0wiGDo5l8c++HWtbaNCWTS4UTv7RaTH/ThVIgjkveJt83m74bBHMJkuCbslY8ixgLBVZJIOiQlQTjfQ==",
+      "license": "Apache-2.0",
+      "bin": {
+        "tsc": "bin/tsc",
+        "tsserver": "bin/tsserver"
+      },
+      "engines": {
+        "node": ">=14.17"
+      }
+    },
     "node_modules/@cloudflare/workerd-darwin-64": {
       "version": "1.20260415.1",
       "resolved": "https://registry.npmjs.org/@cloudflare/workerd-darwin-64/-/workerd-darwin-64-1.20260415.1.tgz",
@@ -1412,6 +1443,7 @@
       "cpu": [
         "ppc64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1428,6 +1460,7 @@
       "cpu": [
         "arm"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1444,6 +1477,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1460,6 +1494,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1493,6 +1528,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1509,6 +1545,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1525,6 +1562,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1541,6 +1579,7 @@
       "cpu": [
         "arm"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1557,6 +1596,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1573,6 +1613,7 @@
       "cpu": [
         "ia32"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1589,6 +1630,7 @@
       "cpu": [
         "loong64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1605,6 +1647,7 @@
       "cpu": [
         "mips64el"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1621,6 +1664,7 @@
       "cpu": [
         "ppc64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1637,6 +1681,7 @@
       "cpu": [
         "riscv64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1653,6 +1698,7 @@
       "cpu": [
         "s390x"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1669,6 +1715,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1685,6 +1732,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1701,6 +1749,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1717,6 +1766,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1733,6 +1783,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1749,6 +1800,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1765,6 +1817,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1781,6 +1834,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1797,6 +1851,7 @@
       "cpu": [
         "ia32"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -1813,6 +1868,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -4088,9 +4144,6 @@
         "arm64"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -4108,9 +4161,6 @@
         "arm64"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -4128,9 +4178,6 @@
         "ppc64"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -4148,9 +4195,6 @@
         "riscv64"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -4168,9 +4212,6 @@
         "riscv64"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -4188,9 +4229,6 @@
         "s390x"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -4208,9 +4246,6 @@
         "x64"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -4228,9 +4263,6 @@
         "x64"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5577,9 +5609,6 @@
       "cpu": [
         "arm64"
       ],
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5596,9 +5625,6 @@
       "cpu": [
         "arm64"
       ],
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5615,9 +5641,6 @@
       "cpu": [
         "ppc64"
       ],
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5634,9 +5657,6 @@
       "cpu": [
         "s390x"
       ],
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5653,9 +5673,6 @@
       "cpu": [
         "x64"
       ],
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5672,9 +5689,6 @@
       "cpu": [
         "x64"
       ],
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5904,9 +5918,6 @@
         "arm"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5921,9 +5932,6 @@
         "arm"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5937,9 +5945,6 @@
       "cpu": [
         "arm64"
       ],
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5954,9 +5959,6 @@
         "arm64"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5971,9 +5973,6 @@
         "loong64"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -5988,9 +5987,6 @@
         "loong64"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -6005,9 +6001,6 @@
         "ppc64"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -6022,9 +6015,6 @@
         "ppc64"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -6039,9 +6029,6 @@
         "riscv64"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -6056,9 +6043,6 @@
         "riscv64"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -6073,9 +6057,6 @@
         "s390x"
       ],
       "dev": true,
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -6089,9 +6070,6 @@
       "cpu": [
         "x64"
       ],
-      "libc": [
-        "glibc"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -6106,9 +6084,6 @@
         "x64"
       ],
       "dev": true,
-      "libc": [
-        "musl"
-      ],
       "license": "MIT",
       "optional": true,
       "os": [
@@ -8086,7 +8061,6 @@
     },
     "node_modules/@typescript/vfs": {
       "version": "1.6.4",
-      "dev": true,
       "license": "MIT",
       "dependencies": {
         "debug": "^4.4.3"
@@ -9257,7 +9231,6 @@
       "resolved": "https://registry.npmjs.org/abort-controller/-/abort-controller-3.0.0.tgz",
       "integrity": "sha512-h8lQ8tacZYnR3vNQTgibj+tODHI5/+l06Au2Pcriv/Gmet0eaj4TwWH41sO9wnHDiQsEj19q0drzdWdeAHtweg==",
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "event-target-shim": "^5.0.0"
       },
@@ -9585,6 +9558,12 @@
         "node": ">=14"
       }
     },
+    "node_modules/any-promise": {
+      "version": "1.3.0",
+      "resolved": "https://registry.npmjs.org/any-promise/-/any-promise-1.3.0.tgz",
+      "integrity": "sha512-7UvmKalWRt1wgjL1RrGxoSJW/0QZFIegpeGvZG9kjp8vrRu55XTHbwnqq2GpXm9uLbcuhxm3IqX9OB4MZR1b2A==",
+      "license": "MIT"
+    },
     "node_modules/anymatch": {
       "version": "3.1.3",
       "dev": true,
@@ -9791,8 +9770,7 @@
       "version": "1.4.1",
       "resolved": "https://registry.npmjs.org/async-lock/-/async-lock-1.4.1.tgz",
       "integrity": "sha512-Az2ZTpuytrtqENulXwO3GGv1Bztugx6TT37NIo7imr/Qo0gsYiGtSdBa2B6fsXhTpVZDNfu1Qn3pk531e3q+nQ==",
-      "license": "MIT",
-      "peer": true
+      "license": "MIT"
     },
     "node_modules/asynckit": {
       "version": "0.4.0",
@@ -10250,8 +10228,7 @@
       "version": "2.0.1",
       "resolved": "https://registry.npmjs.org/clean-git-ref/-/clean-git-ref-2.0.1.tgz",
       "integrity": "sha512-bLSptAy2P0s6hU4PzuIMKmMJJSE6gLXGH1cntDu7bWJUksvuM+7ReOK61mozULErYvP6a15rnYl0zFDef+pyPw==",
-      "license": "Apache-2.0",
-      "peer": true
+      "license": "Apache-2.0"
     },
     "node_modules/cli-cursor": {
       "version": "3.1.0",
@@ -10650,7 +10627,6 @@
       "resolved": "https://registry.npmjs.org/crc-32/-/crc-32-1.2.2.tgz",
       "integrity": "sha512-ROmzCKrTnOwybPcJApAA6WBWij23HVfGVNKqqrZpuyZOHqK2CwHSvpGuyt/UNNvaIjEd8X5IFGp4Mh+Ie1IHJQ==",
       "license": "Apache-2.0",
-      "peer": true,
       "bin": {
         "crc32": "bin/crc32.njs"
       },
@@ -11278,7 +11254,6 @@
       "resolved": "https://registry.npmjs.org/decompress-response/-/decompress-response-6.0.0.tgz",
       "integrity": "sha512-aW35yZM6Bb/4oJlZncMH2LCoZtJXTRxES17vE3hoRiowU2kWHaJKFkSBDnDR+cm9J+9QhXmREyIfv0pji9ejCQ==",
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "mimic-response": "^3.1.0"
       },
@@ -11443,8 +11418,7 @@
       "version": "0.0.3",
       "resolved": "https://registry.npmjs.org/diff3/-/diff3-0.0.3.tgz",
       "integrity": "sha512-iSq8ngPOt0K53A6eVr4d5Kn6GNrM2nQZtC740pzIriHtn4pOQ2lyzEXQMBeVcWERN0ye7fhBsk9PbLLQOnUx/g==",
-      "license": "MIT",
-      "peer": true
+      "license": "MIT"
     },
     "node_modules/dns-equal": {
       "version": "1.0.0",
@@ -11826,7 +11800,6 @@
     },
     "node_modules/es-module-lexer": {
       "version": "2.0.0",
-      "dev": true,
       "license": "MIT"
     },
     "node_modules/es-object-atoms": {
@@ -11884,7 +11857,7 @@
       "version": "0.27.7",
       "resolved": "https://registry.npmjs.org/esbuild/-/esbuild-0.27.7.tgz",
       "integrity": "sha512-IxpibTjyVnmrIQo5aqNpCgoACA/dTKLTlhMHihVHhdkxKyPO1uBBthumT0rdHmcsk9uMonIWS0m4FljWzILh3w==",
-      "devOptional": true,
+      "dev": true,
       "hasInstallScript": true,
       "license": "MIT",
       "bin": {
@@ -11922,6 +11895,18 @@
         "@esbuild/win32-x64": "0.27.7"
       }
     },
+    "node_modules/esbuild-wasm": {
+      "version": "0.28.0",
+      "resolved": "https://registry.npmjs.org/esbuild-wasm/-/esbuild-wasm-0.28.0.tgz",
+      "integrity": "sha512-5TRVKExcEmeMkccIZMzUq+Az6X2RoMAJyfl6SMMO1dMVhmvt0I2mx7gAb6zYi42n4d1ETcatFXazGKzA+aW7fg==",
+      "license": "MIT",
+      "bin": {
+        "esbuild": "bin/esbuild"
+      },
+      "engines": {
+        "node": ">=18"
+      }
+    },
     "node_modules/esbuild/node_modules/@esbuild/darwin-arm64": {
       "version": "0.27.7",
       "resolved": "https://registry.npmjs.org/@esbuild/darwin-arm64/-/darwin-arm64-0.27.7.tgz",
@@ -11929,6 +11914,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MIT",
       "optional": true,
       "os": [
@@ -12415,7 +12401,6 @@
       "resolved": "https://registry.npmjs.org/event-target-shim/-/event-target-shim-5.0.1.tgz",
       "integrity": "sha512-i/2XbnSz/uxRCU6+NdVJgKWDTM427+MqYbkQzD321DuCQJUqOuJKIA0IM2+W2xtYHdKOmZ4dR6fExsd4SXL+WQ==",
       "license": "MIT",
-      "peer": true,
       "engines": {
         "node": ">=6"
       }
@@ -12430,7 +12415,6 @@
       "resolved": "https://registry.npmjs.org/events/-/events-3.3.0.tgz",
       "integrity": "sha512-mQw+2fkQbALzQ7V0MY0IqdnXNOeTtP4r0lN9z7AAawCXgqea7bDii20AYrIBrFd/Hx0M2Ocz6S111CaFkUcb0Q==",
       "license": "MIT",
-      "peer": true,
       "engines": {
         "node": ">=0.8.x"
       }
@@ -12893,6 +12877,7 @@
       "version": "2.3.3",
       "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz",
       "integrity": "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==",
+      "dev": true,
       "hasInstallScript": true,
       "license": "MIT",
       "optional": true,
@@ -14086,7 +14071,6 @@
       "resolved": "https://registry.npmjs.org/isomorphic-git/-/isomorphic-git-1.37.5.tgz",
       "integrity": "sha512-wek54c5uFvd3WsxewLWt6h0GXKWQh0P8rRXns9bN1rHNjcgCb3+0lmyAsP594NeTtQFeCJQVS9b0kjbkD1l5qg==",
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "async-lock": "^1.4.1",
         "clean-git-ref": "^2.0.1",
@@ -14126,7 +14110,6 @@
         }
       ],
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "base64-js": "^1.3.1",
         "ieee754": "^1.2.1"
@@ -14137,7 +14120,6 @@
       "resolved": "https://registry.npmjs.org/ignore/-/ignore-5.3.2.tgz",
       "integrity": "sha512-hsBTNUqQTDwkWtcdYI2i06Y/nUBEsNEDJKjWdigLvegy8kDuJAS8uRlpkkcQpyEXL0Z/pjDy5HBmMjRCJ2gq+g==",
       "license": "MIT",
-      "peer": true,
       "engines": {
         "node": ">= 4"
       }
@@ -14147,7 +14129,6 @@
       "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-4.7.0.tgz",
       "integrity": "sha512-oIGGmcpTLwPga8Bn6/Z75SVaH1z5dUut2ibSyAMVhmUggWpmDn2dapB0n7f8nwaSiRtepAsfJyfXIO5DCVAODg==",
       "license": "MIT",
-      "peer": true,
       "dependencies": {
         "abort-controller": "^3.0.0",
         "buffer": "^6.0.3",
@@ -14207,7 +14188,7 @@
     },
     "node_modules/jiti": {
       "version": "2.6.1",
-      "devOptional": true,
+      "dev": true,
       "license": "MIT",
       "bin": {
         "jiti": "lib/jiti-cli.mjs"
@@ -14531,6 +14512,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14551,6 +14533,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14571,6 +14554,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14591,6 +14575,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14611,6 +14596,7 @@
       "cpu": [
         "arm"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14631,6 +14617,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14651,6 +14638,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14671,6 +14659,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14691,6 +14680,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14711,6 +14701,7 @@
       "cpu": [
         "arm64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -14731,6 +14722,7 @@
       "cpu": [
         "x64"
       ],
+      "dev": true,
       "license": "MPL-2.0",
       "optional": true,
       "os": [
@@ -16056,7 +16048,6 @@
       "resolved": "https://registry.npmjs.org/mimic-response/-/mimic-response-3.1.0.tgz",
       "integrity": "sha512-z0yWI+4FDrrweS8Zmt4Ej5HdJmky15+L2e6Wgn3+iK5fWzb6T3fhNFq2+MeTRb064c6Wr4N/wv0DzQTjNzHNGQ==",
       "license": "MIT",
-      "peer": true,
       "engines": {
         "node": ">=10"
       },
... diff truncated: showing 800 of 8639 lines

You can send follow-ups to the cloud agent here.

Comment thread packages/worker/src/repo/app-source.ts
Comment thread packages/worker/src/jobs/repo.ts Outdated
Comment thread packages/worker/src/mcp/capabilities/apps/ui-save-app.ts Outdated
publishedCommit?: string | null
}) {
return typeof input.sourceId === 'string' && input.sourceId.length > 0
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unused publishedCommit parameter in hasRepoBackedJobSource

Low Severity

hasRepoBackedJobSource declares publishedCommit in its input type but never reads it. The function only checks sourceId. This is misleading since callers pass objects containing both fields, giving the impression that publishedCommit participates in the decision.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7d96125. Configure here.

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Comment thread packages/worker/src/job-maintenance.ts Outdated
Comment thread packages/worker/src/repo/source-sync.ts
Comment thread packages/worker/src/jobs/service.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Comment thread packages/worker/src/mcp/capabilities/meta/meta-save-skill.ts

const savedAppSourceCache = new Map<string, ResolvedSavedAppSource>()
const savedAppSourceCacheLimit = 100

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Module-level Map cache leaks memory across Worker requests

Low Severity

savedAppSourceCache is a module-level Map with a 100-entry LRU cap. In Cloudflare Workers, module-scoped state persists across requests within the same isolate. While the cap prevents unbounded growth, the cached ResolvedSavedAppSource objects contain full clientCode and serverCode strings that can be large, and the cache is never cleared between requests. Under steady traffic to many different apps, this memory pressure persists for the lifetime of the isolate.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit e544738. Configure here.

@kentcdodds
kentcdodds merged commit 8d2a07d into main Apr 17, 2026
9 checks passed
@kentcdodds
kentcdodds deleted the cursor/repo-backed-source-23f1 branch April 17, 2026 03:19

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 4 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.

Reviewed by Cursor Bugbot for commit ddb50b0. Configure here.

.catch(() => {
// Best effort only; ensure new sessions pick up the latest commit.
})
session = await sessionClient.openSession(openSessionInput)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Stale session reused when discard silently fails

Medium Severity

In runRepoBackedJob, when base_commit differs from published_commit, discardSession is called with .catch(() => {}), swallowing all errors. If the discard fails (e.g., D1 delete fails), the subsequent openSession finds the old session row still in the database and reuses it with the stale base_commit, causing the job to execute against outdated code instead of the latest published commit.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ddb50b0. Configure here.

/\bmodule\.exports\b/.test(code) ||
/\bexports\.[A-Za-z_$][\w$]*/.test(code)
)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Module-style regex falsely matches content inside template literals

Low Severity

hasModuleStyleCodemodeEntrypoint uses /^\s*export\s+/m which, due to the multiline flag, matches any line starting with export — including lines inside template literals or multi-line strings. A legitimate async function snippet containing a template literal with an export-prefixed line would be incorrectly rejected as a module-style entrypoint, blocking execution of valid repo-backed job code.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ddb50b0. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants