Repository navigation
Fix production repo-backed source fallback - #191
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughMigrates the Artifacts integration from a Wrangler binding to a REST-based client using Cloudflare account/API credentials; adds persistence gating that can fail closed, reorders backfill/sync flows to persist state before updating records, and documents repo-backed sources and repo session behavior. Changes
Sequence DiagramsequenceDiagram
participant Backfill as Source Backfill
participant ArtClient as Artifacts REST Client
participant CFAPI as Cloudflare Artifacts API
participant D1 as D1 Database
participant DO as Repo Session Durable Object
Backfill->>ArtClient: getArtifactsBinding(env)
ArtClient->>CFAPI: validate credentials (CLOUDFLARE_ACCOUNT_ID, API_TOKEN)
Backfill->>ArtClient: create/get/list repo, createToken, fork as needed
ArtClient->>CFAPI: POST/GET /repos, /tokens, /repos/{id}/fork
CFAPI-->>ArtClient: repo metadata + token(s)
Backfill->>D1: ensureEntitySource(..., requirePersistence:true)
D1->>D1: check DB.prepare and Artifacts access
alt prerequisites met
D1->>D1: insert/update entity_sources row
D1-->>Backfill: persisted source row
Backfill->>DO: create/update repo session
DO->>CFAPI: fork/sync repo
CFAPI-->>DO: forked repo metadata
Backfill->>D1: updateEntitySource + update artifact record
else missing prerequisites
D1-->>Backfill: throw error (missing requirements)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
kentcdodds
marked this pull request as ready for review
April 17, 2026 16:12
Contributor
|
🔎 Preview deployed: https://kody-pr-191.kentcdodds.workers.dev Worker: Mocks:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/worker/src/repo/source-backfill.ts (1)
488-504: RedundantupdateJobRowcall whensourceIdchanges.When
input.row.record.sourceId !== ensuredSource.id,updateJobRowis invoked twice back-to-back with effectively the samenextRecord: first withoutpublishedCommit(Lines 488-495), and then again immediately withpublishedCommitset (Lines 496-504). The first write is clobbered by the second and only adds an extra D1 round-trip. Consider dropping the guarded call and letting the unconditional call at Lines 496-504 carry both the newsourceIdandpublishedCommit.♻️ Proposed simplification
- if (input.row.record.sourceId !== ensuredSource.id) { - await updateJobRow({ - db: input.env.APP_DB, - userId: input.userId, - job: nextRecord, - callerContextJson: input.row.callerContextJson, - }) - } await updateJobRow({ db: input.env.APP_DB, userId: input.userId, job: { ...nextRecord, publishedCommit, }, callerContextJson: input.row.callerContextJson, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/repo/source-backfill.ts` around lines 488 - 504, The code currently calls updateJobRow twice when input.row.record.sourceId !== ensuredSource.id, which is redundant; remove the guarded call that writes nextRecord without publishedCommit (the conditional updateJobRow block that checks input.row.record.sourceId !== ensuredSource.id) and keep the unconditional updateJobRow that writes {...nextRecord, publishedCommit} so the single write includes both the new sourceId (from nextRecord/ensuredSource) and publishedCommit; updateJobRow, nextRecord, input.row.record.sourceId, ensuredSource.id, and publishedCommit are the identifiers to locate and adjust.packages/worker/src/repo/artifacts.node.test.ts (1)
17-31: Cryptic first-call detection for the 404 path.The
fetchMock.mock.calls.some((call) => call[0] === input && call !== fetchMock.mock.calls[0])guard relies on the current invocation being recorded as the first entry inmock.callsand does reference-identity comparisons on the call tuple. It works for this sequence but is hard to reason about and will break subtly if anotherGET /repos/repo-1happens before the resolve step. A simple request counter (or dedicated flag for the first GET) expresses the intent far more clearly.♻️ Suggested simpler form
- const fetchMock = vi.spyOn(globalThis, 'fetch').mockImplementation( - async (input, init) => { + let getRepoCallCount = 0 + const fetchMock = vi.spyOn(globalThis, 'fetch').mockImplementation( + async (input, init) => { const url = new URL(String(input)) const method = init?.method ?? 'GET' if (method === 'GET' && url.pathname.endsWith('/repos/repo-1')) { - if (!fetchMock.mock.calls.some((call) => call[0] === input && call !== fetchMock.mock.calls[0])) { + getRepoCallCount += 1 + if (getRepoCallCount === 1) { return new Response(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/repo/artifacts.node.test.ts` around lines 17 - 31, The current 404 branch in the test handler uses a fragile reference-identity check on fetchMock.mock.calls to detect the "first" GET /repos/repo-1; replace that with an explicit counter or boolean flag (e.g., add a local let firstGetRepo1 = true or let getRepo1Count = 0) inside the test scope and update it when the handler observes method === 'GET' && url.pathname.endsWith('/repos/repo-1'); then change the condition using fetchMock.mock.calls.some(...) to check the counter/flag (firstGetRepo1 or getRepo1Count > 1) so the intent is clear and robust across multiple calls to the handler.packages/worker/src/repo/source-sync.ts (1)
74-81: DuplicatedhasArtifactsAccesshelper.This helper is identical to
hasArtifactsAccessConfiginpackages/worker/src/repo/source-service.ts. Consider exporting a singlehasArtifactsAccess(env)from./artifacts.ts(or exposingcreateArtifactsRestBinding(env) != nullvia a public check) and reusing it in both call sites to avoid drift in gating semantics.🤖 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 74 - 81, The helper hasArtifactsAccess in source-sync.ts duplicates hasArtifactsAccessConfig in source-service.ts; refactor by centralizing the check in a single exported function in artifacts.ts (e.g., export function hasArtifactsAccess(env) { try { void getArtifactsBinding(env) ; return true } catch { return false } } or export createArtifactsRestBinding(env) !== null as a public check) and update both source-sync.ts and source-service.ts to import and call that shared hasArtifactsAccess instead of keeping two separate implementations (reference symbols: hasArtifactsAccess, hasArtifactsAccessConfig, getArtifactsBinding, createArtifactsRestBinding, artifacts.ts, source-sync.ts, source-service.ts).
🤖 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/repo/artifacts.ts`:
- Around line 322-331: The current check reads envelope?.errors[0]?.message
which will throw if envelope.errors is undefined; update the guard to safely
access the first error message (e.g. use envelope?.errors?.[0]?.message or check
Array.isArray(envelope.errors) && envelope.errors.length) when building message,
keeping the existing fallback `Artifacts API request failed
(${response.status})`; leave the treat404AsNull handling and the return of `{
...envelope, result: null } as ArtifactApiEnvelope<T>` unchanged but ensure the
envelope spread is safe even when errors is missing.
- Around line 186-208: The fork implementation ignores an optional namespace on
its parameter type: update the fork handling in the fork async function to be
consistent with the type by either removing namespace from the Target type or
wiring namespace into the REST call; specifically, if you choose to support
namespaces add the namespace to the POST request (e.g., include target.namespace
in the request body or encode it into the path used by requestArtifactsApi with
basePath and `/repos/${encodeURIComponent(name)}/fork`), otherwise remove
namespace from the fork parameter/interface so the function signature matches
the sent payload (references: fork function, requestArtifactsApi call,
target.name/target.readOnly, basePath, and repoHandle).
- Around line 365-374: The parseArtifactTokenExpiry function currently
fabricates a 1-hour expiry when it cannot parse ?expires= from the token; change
it to return null (or throw) instead of inventing an expiry so callers cannot
silently trust a bad timestamp: update parseArtifactTokenExpiry to return null
on unparseable or missing expires value and update any callers of
parseArtifactTokenExpiry (search for parseArtifactTokenExpiry usages) to handle
null (e.g., trigger a refresh/error path or propagate the failure) so downstream
logic never continues using an assumed-valid expiry.
---
Nitpick comments:
In `@packages/worker/src/repo/artifacts.node.test.ts`:
- Around line 17-31: The current 404 branch in the test handler uses a fragile
reference-identity check on fetchMock.mock.calls to detect the "first" GET
/repos/repo-1; replace that with an explicit counter or boolean flag (e.g., add
a local let firstGetRepo1 = true or let getRepo1Count = 0) inside the test scope
and update it when the handler observes method === 'GET' &&
url.pathname.endsWith('/repos/repo-1'); then change the condition using
fetchMock.mock.calls.some(...) to check the counter/flag (firstGetRepo1 or
getRepo1Count > 1) so the intent is clear and robust across multiple calls to
the handler.
In `@packages/worker/src/repo/source-backfill.ts`:
- Around line 488-504: The code currently calls updateJobRow twice when
input.row.record.sourceId !== ensuredSource.id, which is redundant; remove the
guarded call that writes nextRecord without publishedCommit (the conditional
updateJobRow block that checks input.row.record.sourceId !== ensuredSource.id)
and keep the unconditional updateJobRow that writes {...nextRecord,
publishedCommit} so the single write includes both the new sourceId (from
nextRecord/ensuredSource) and publishedCommit; updateJobRow, nextRecord,
input.row.record.sourceId, ensuredSource.id, and publishedCommit are the
identifiers to locate and adjust.
In `@packages/worker/src/repo/source-sync.ts`:
- Around line 74-81: The helper hasArtifactsAccess in source-sync.ts duplicates
hasArtifactsAccessConfig in source-service.ts; refactor by centralizing the
check in a single exported function in artifacts.ts (e.g., export function
hasArtifactsAccess(env) { try { void getArtifactsBinding(env) ; return true }
catch { return false } } or export createArtifactsRestBinding(env) !== null as a
public check) and update both source-sync.ts and source-service.ts to import and
call that shared hasArtifactsAccess instead of keeping two separate
implementations (reference symbols: hasArtifactsAccess,
hasArtifactsAccessConfig, getArtifactsBinding, createArtifactsRestBinding,
artifacts.ts, source-sync.ts, source-service.ts).
🪄 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: d6d860bc-f8ad-41b2-8936-cbbf1d507e59
📒 Files selected for processing (9)
docs/contributing/architecture/data-storage.mdpackages/worker/src/repo/artifacts.node.test.tspackages/worker/src/repo/artifacts.tspackages/worker/src/repo/source-backfill.node.test.tspackages/worker/src/repo/source-backfill.tspackages/worker/src/repo/source-service.node.test.tspackages/worker/src/repo/source-service.tspackages/worker/src/repo/source-sync.tspackages/worker/wrangler.jsonc
💤 Files with no reviewable changes (1)
- packages/worker/wrangler.jsonc
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Null envelope spread produces incomplete API response object
- Returned a full success envelope for treat-404-as-null responses to avoid spreading a null body.
Preview (6266f29c68)
diff --git a/docs/contributing/architecture/data-storage.md b/docs/contributing/architecture/data-storage.md
--- a/docs/contributing/architecture/data-storage.md
+++ b/docs/contributing/architecture/data-storage.md
@@ -84,3 +84,29 @@
- `JOB_MANAGER` (Durable Objects)
- `STORAGE_RUNNER` (Durable Objects)
- `ASSETS` (static assets bucket)
+
+## Repo-backed sources and Artifacts
+
+Repo-backed saved apps, skills, jobs, and repo editing sessions use Cloudflare
+Artifacts repos plus D1 `entity_sources` / `repo_sessions` rows.
+
+- Primary code lives under `packages/worker/src/repo/`.
+- `entity_sources` stores the durable mapping from `(user_id, entity_kind,
+ entity_id)` to the repo identity and last published commit.
+- `repo_sessions` stores mutable editing forks for repo session Durable Objects.
+
+Production note:
+
+- Released `wrangler` `4.83.0` still warns that the documented Artifacts Worker
+ binding config is unexpected, and production deploy logs show no
+ `env.ARTIFACTS` binding in the deployed Worker binding summary.
+- Because of that deploy-time gap, repo source code uses the documented
+ Artifacts REST API as the single integration path for create/get/token/fork
+ operations.
+- `packages/worker/src/repo/artifacts.ts` builds that REST client from
+ `CLOUDFLARE_ACCOUNT_ID`, `CLOUDFLARE_API_TOKEN`, and optional
+ `CLOUDFLARE_API_BASE_URL` / `ARTIFACTS_NAMESPACE`, which also makes local dev
+ mocking straightforward.
+- Durable repo-source creation paths (`ensureEntitySource(...,
+ requirePersistence: true)`) fail closed when persistence bindings are
+ unavailable so callers do not write orphaned `source_id` references into D1.
diff --git a/packages/worker/src/repo/artifacts.node.test.ts b/packages/worker/src/repo/artifacts.node.test.ts
new file mode 100644
--- /dev/null
+++ b/packages/worker/src/repo/artifacts.node.test.ts
@@ -1,0 +1,161 @@
+import { afterEach, expect, test, vi } from 'vitest'
+
+const {
+ getArtifactsBinding,
+ resolveArtifactSourceRepo,
+} = await import('./artifacts.ts')
+
+afterEach(() => {
+ vi.restoreAllMocks()
+})
+
+test('artifacts REST client supports get, create, token, and fork operations', async () => {
+ const fetchMock = vi.spyOn(globalThis, 'fetch').mockImplementation(
+ async (input, init) => {
+ const url = new URL(String(input))
+ const method = init?.method ?? 'GET'
+ if (method === 'GET' && url.pathname.endsWith('/repos/repo-1')) {
+ if (!fetchMock.mock.calls.some((call) => call[0] === input && call !== fetchMock.mock.calls[0])) {
+ return new Response(
+ JSON.stringify({
+ success: false,
+ result: null,
+ errors: [{ code: 1000, message: 'Repo not found' }],
+ messages: [],
+ }),
+ {
+ status: 404,
+ headers: { 'content-type': 'application/json' },
+ },
+ )
+ }
+ return new Response(
+ JSON.stringify({
+ success: true,
+ result: {
+ id: 'repo_1',
+ name: 'repo-1',
+ description: 'Repo 1',
+ default_branch: 'main',
+ created_at: '2026-04-17T00:00:00.000Z',
+ updated_at: '2026-04-17T00:00:00.000Z',
+ last_push_at: null,
+ source: null,
+ read_only: false,
+ remote: 'https://acct.artifacts.cloudflare.net/git/default/repo-1.git',
+ },
+ errors: [],
+ messages: [],
+ }),
+ {
+ status: 200,
+ headers: { 'content-type': 'application/json' },
+ },
+ )
+ }
+ if (method === 'POST' && url.pathname.endsWith('/repos')) {
+ return new Response(
+ JSON.stringify({
+ success: true,
+ result: {
+ id: 'repo_1',
+ name: 'repo-1',
+ description: null,
+ default_branch: 'main',
+ remote: 'https://acct.artifacts.cloudflare.net/git/default/repo-1.git',
+ token: 'art_v1_create?expires=1760000000',
+ },
+ errors: [],
+ messages: [],
+ }),
+ {
+ status: 200,
+ headers: { 'content-type': 'application/json' },
+ },
+ )
+ }
+ if (method === 'POST' && url.pathname.endsWith('/tokens')) {
+ return new Response(
+ JSON.stringify({
+ success: true,
+ result: {
+ id: 'tok_1',
+ plaintext: 'art_v1_read?expires=1760000100',
+ scope: 'read',
+ expires_at: '2026-10-09T08:55:00.000Z',
+ },
+ errors: [],
+ messages: [],
+ }),
+ {
+ status: 200,
+ headers: { 'content-type': 'application/json' },
+ },
+ )
+ }
+ if (method === 'POST' && url.pathname.endsWith('/repos/repo-1/fork')) {
+ return new Response(
+ JSON.stringify({
+ success: true,
+ result: {
+ id: 'repo_2',
+ name: 'repo-copy',
+ description: null,
+ default_branch: 'main',
+ remote: 'https://acct.artifacts.cloudflare.net/git/default/repo-copy.git',
+ token: 'art_v1_fork?expires=1760000200',
+ },
+ errors: [],
+ messages: [],
+ }),
+ {
+ status: 200,
+ headers: { 'content-type': 'application/json' },
+ },
+ )
+ }
+ throw new Error(`Unexpected fetch: ${method} ${url.pathname}`)
+ },
+ )
+
+ const env = {
+ CLOUDFLARE_ACCOUNT_ID: 'acct',
+ CLOUDFLARE_API_TOKEN: 'token-123',
+ CLOUDFLARE_API_BASE_URL: 'https://api.example.com',
+ } as Env
+
+ const binding = getArtifactsBinding(env)
+ await expect(binding.get('repo-1')).resolves.toEqual({ status: 'not_found' })
+ await expect(binding.create('repo-1')).resolves.toMatchObject({
+ id: 'repo_1',
+ name: 'repo-1',
+ defaultBranch: 'main',
+ remote: 'https://acct.artifacts.cloudflare.net/git/default/repo-1.git',
+ token: 'art_v1_create?expires=1760000000',
+ })
+
+ const repo = await resolveArtifactSourceRepo(env, 'repo-1')
+ await expect(repo.info()).resolves.toMatchObject({
+ id: 'repo_1',
+ name: 'repo-1',
+ defaultBranch: 'main',
+ remote: 'https://acct.artifacts.cloudflare.net/git/default/repo-1.git',
+ })
+ await expect(repo.createToken('read', 120)).resolves.toEqual({
+ id: 'tok_1',
+ plaintext: 'art_v1_read?expires=1760000100',
+ scope: 'read',
+ expiresAt: '2026-10-09T08:55:00.000Z',
+ })
+ await expect(repo.fork({ name: 'repo-copy', readOnly: false })).resolves.toMatchObject(
+ {
+ id: 'repo_2',
+ name: 'repo-copy',
+ defaultBranch: 'main',
+ remote: 'https://acct.artifacts.cloudflare.net/git/default/repo-copy.git',
+ token: 'art_v1_fork?expires=1760000200',
+ },
+ )
+
+ expect(fetchMock).toHaveBeenCalledTimes(6)
+})
diff --git a/packages/worker/src/repo/artifacts.ts b/packages/worker/src/repo/artifacts.ts
--- a/packages/worker/src/repo/artifacts.ts
+++ b/packages/worker/src/repo/artifacts.ts
@@ -1,3 +1,7 @@
+import {
+ CloudflareApiError,
+ createCloudflareRestClient,
+} from '#mcp/cloudflare/cloudflare-rest-client.ts'
import { type EntityKind } from './types.ts'
export type ArtifactToken = {
@@ -48,7 +52,11 @@
export type ArtifactNamespaceBinding = {
create(
name: string,
- opts?: { readOnly?: boolean },
+ opts?: {
+ description?: string
+ readOnly?: boolean
+ setDefaultBranch?: string
+ },
): Promise<{
id: string
name: string
@@ -69,14 +77,24 @@
export function getArtifactsBinding(
env: Env,
): ArtifactNamespaceBinding & Record<string, unknown> {
- const binding = (env as Env & { ARTIFACTS?: ArtifactNamespaceBinding })
- .ARTIFACTS
- if (!binding) {
- throw new Error('ARTIFACTS binding is not configured.')
+ const restBinding = createArtifactsRestBinding(env)
+ if (!restBinding) {
+ throw new Error(
+ 'Cloudflare Artifacts REST access requires CLOUDFLARE_ACCOUNT_ID and CLOUDFLARE_API_TOKEN.',
+ )
}
- return binding
+ return restBinding
}
+export function hasArtifactsAccess(env: Env) {
+ try {
+ void getArtifactsBinding(env)
+ return true
+ } catch {
+ return false
+ }
+}
+
export function getArtifactsNamespace(env: Env) {
return (
(env as Env & { ARTIFACTS_NAMESPACE?: string | undefined })
@@ -84,6 +102,291 @@
)
}
+type ArtifactApiEnvelope<T> = {
+ result: T | null
+ success: boolean
+ errors: Array<{
+ code: number
+ message: string
+ }>
+ messages: Array<{
+ code: number
+ message: string
+ }>
+ result_info?: {
+ cursor?: string
+ count?: number
+ total_count?: number
+ }
+}
+
+type ArtifactRestRepoInfo = {
+ id: string
+ name: string
+ description: string | null
+ default_branch: string
+ created_at: string
+ updated_at: string
+ last_push_at: string | null
+ source: string | null
+ read_only: boolean
+ remote: string
+}
+
+type ArtifactRestCreateRepoResult = {
+ id: string
+ name: string
+ description: string | null
+ default_branch: string
+ remote: string
+ token: string
+}
+
+type ArtifactRestCreateTokenResult = {
+ id: string
+ plaintext: string
+ scope: string
+ expires_at: string
+}
+
+type ArtifactRestForkRepoResult = ArtifactRestCreateRepoResult & {
+ objects?: number
+}
+
+function createArtifactsRestBinding(env: Env) {
+ const accountId = env.CLOUDFLARE_ACCOUNT_ID?.trim()
+ const apiToken = env.CLOUDFLARE_API_TOKEN?.trim()
+ if (!accountId || !apiToken) {
+ return null
+ }
+ const client = createCloudflareRestClient(env)
+ const namespace = getArtifactsNamespace(env)
+ const basePath = `/client/v4/accounts/${accountId}/artifacts/namespaces/${namespace}`
+ const getRepoInfo = async (name: string): Promise<ArtifactRepoInfo | null> => {
+ const response = await requestArtifactsEnvelope<ArtifactRestRepoInfo>(client, {
+ method: 'GET',
+ path: `${basePath}/repos/${encodeURIComponent(name)}`,
+ treat404AsNull: true,
+ })
+ return response.result ? normalizeArtifactRepoInfo(response.result) : null
+ }
+ const repoHandle = (name: string): ArtifactRepoHandle => ({
+ info: async () => await getRepoInfo(name),
+ createToken: async (scope = 'write', ttl = 3600) => {
+ const result = await requestArtifactsApi<ArtifactRestCreateTokenResult>(
+ client,
+ {
+ method: 'POST',
+ path: `${basePath}/tokens`,
+ body: {
+ repo: name,
+ scope,
+ ttl,
+ },
+ },
+ )
+ return {
+ id: result.id,
+ plaintext: result.plaintext,
+ scope: result.scope,
+ expiresAt: result.expires_at,
+ }
+ },
+ fork: async (target) => {
+ const result = await requestArtifactsApi<ArtifactRestForkRepoResult>(
+ client,
+ {
+ method: 'POST',
+ path: `${basePath}/repos/${encodeURIComponent(name)}/fork`,
+ body: {
+ name: target.name,
+ read_only: target.readOnly ?? false,
+ },
+ },
+ )
+ return {
+ id: result.id,
+ name: result.name,
+ description: result.description,
+ defaultBranch: result.default_branch,
+ remote: result.remote,
+ token: result.token,
+ expiresAt: parseArtifactTokenExpiry(result.token),
+ repo: repoHandle(result.name),
+ }
+ },
+ })
+ return {
+ create: async (name, opts) => {
+ const result = await requestArtifactsApi<ArtifactRestCreateRepoResult>(
+ client,
+ {
+ method: 'POST',
+ path: `${basePath}/repos`,
+ body: {
+ name,
+ ...(opts?.description ? { description: opts.description } : {}),
+ ...(opts?.setDefaultBranch
+ ? { default_branch: opts.setDefaultBranch }
+ : {}),
+ ...(opts?.readOnly !== undefined
+ ? { read_only: opts.readOnly }
+ : {}),
+ },
+ },
+ )
+ return {
+ id: result.id,
+ name: result.name,
+ description: result.description,
+ defaultBranch: result.default_branch,
+ remote: result.remote,
+ token: result.token,
+ expiresAt: parseArtifactTokenExpiry(result.token),
+ }
+ },
+ get: async (name) => {
+ const info = await getRepoInfo(name)
+ if (!info) {
+ return { status: 'not_found' as const }
+ }
+ return {
+ status: 'ready' as const,
+ repo: repoHandle(name),
+ }
+ },
+ list: async (opts) => {
+ const query: Record<string, string> = {}
+ if (opts?.limit !== undefined) {
+ query['limit'] = String(opts.limit)
+ }
+ if (opts?.cursor) {
+ query['cursor'] = opts.cursor
+ }
+ const envelope = await requestArtifactsEnvelope<Array<ArtifactRestRepoInfo>>(
+ client,
+ {
+ method: 'GET',
+ path: `${basePath}/repos`,
+ query,
+ },
+ )
+ const repos = (envelope.result ?? []).map((repo) => {
+ const normalized = normalizeArtifactRepoInfo(repo)
+ return {
+ id: normalized.id,
+ name: normalized.name,
+ description: normalized.description,
+ defaultBranch: normalized.defaultBranch,
+ createdAt: normalized.createdAt,
+ updatedAt: normalized.updatedAt,
+ lastPushAt: normalized.lastPushAt,
+ source: normalized.source,
+ readOnly: normalized.readOnly,
+ }
+ })
+ return {
+ repos,
+ total: envelope.result_info?.total_count ?? repos.length,
+ cursor: envelope.result_info?.cursor,
+ }
+ },
+ } satisfies ArtifactNamespaceBinding & Record<string, unknown>
+}
+
+async function requestArtifactsApi<T>(
+ client: ReturnType<typeof createCloudflareRestClient>,
+ input: {
+ method: 'GET' | 'POST' | 'DELETE'
+ path: string
+ query?: Record<string, string>
+ body?: unknown
+ treat404AsNull?: boolean
+ },
+) {
+ const envelope = await requestArtifactsEnvelope<T>(client, input)
+ if (envelope.result == null) {
+ throw new Error(`Artifacts API returned no result for ${input.path}.`)
+ }
+ return envelope.result
+}
+
+async function requestArtifactsEnvelope<T>(
+ client: ReturnType<typeof createCloudflareRestClient>,
+ input: {
+ method: 'GET' | 'POST' | 'DELETE'
+ path: string
+ query?: Record<string, string>
+ body?: unknown
+ treat404AsNull?: boolean
+ },
+) {
+ try {
+ const response = await client.rawRequest({
+ method: input.method,
+ path: input.path,
+ query: input.query,
+ body: input.body,
+ })
+ const envelope = response.body as ArtifactApiEnvelope<T> | null
+ if (!envelope?.success) {
+ const message =
+ envelope?.errors[0]?.message ??
+ `Artifacts API request failed (${response.status}).`
+ if (input.treat404AsNull && response.status === 404) {
+ return {
+ result: null,
+ success: true,
+ errors: [],
+ messages: [],
+ } satisfies ArtifactApiEnvelope<T>
+ }
+ throw new Error(message)
+ }
+ return envelope
+ } catch (error) {
+ if (
+ input.treat404AsNull &&
+ error instanceof CloudflareApiError &&
+ error.status === 404
+ ) {
+ return {
+ result: null,
+ success: true,
+ errors: [],
+ messages: [],
+ } satisfies ArtifactApiEnvelope<T>
+ }
+ throw error
+ }
+}
+
+function normalizeArtifactRepoInfo(repo: ArtifactRestRepoInfo): ArtifactRepoInfo {
+ return {
+ id: repo.id,
+ name: repo.name,
+ description: repo.description,
+ defaultBranch: repo.default_branch,
+ createdAt: repo.created_at,
+ updatedAt: repo.updated_at,
+ lastPushAt: repo.last_push_at,
+ source: repo.source,
+ readOnly: repo.read_only,
+ remote: repo.remote,
+ }
+}
+
+function parseArtifactTokenExpiry(token: string) {
+ const expiresAtSeconds = Number.parseInt(
+ token.split('?expires=')[1] ?? '',
+ 10,
+ )
+ if (Number.isFinite(expiresAtSeconds)) {
+ return new Date(expiresAtSeconds * 1000).toISOString()
+ }
+ return new Date(Date.now() + 3600 * 1000).toISOString()
+}
+
function normalizeRepoNamePart(value: string) {
return value
.trim()
diff --git a/packages/worker/src/repo/source-backfill.node.test.ts b/packages/worker/src/repo/source-backfill.node.test.ts
new file mode 100644
--- /dev/null
+++ b/packages/worker/src/repo/source-backfill.node.test.ts
@@ -1,0 +1,131 @@
+import { expect, test, vi } from 'vitest'
+import { type UiArtifactRow } from '#mcp/ui-artifacts-types.ts'
+
+const mockModule = vi.hoisted(() => ({
+ listUiArtifactsByUserId: vi.fn(),
+ updateUiArtifact: vi.fn(),
+ listMcpSkillsByUserId: vi.fn(),
+ updateMcpSkill: vi.fn(),
+ listJobRowsByUserId: vi.fn(),
+ updateJobRow: vi.fn(),
+ syncSavedAppRunnerFromDb: vi.fn(),
+ reindexUiArtifactVectors: vi.fn(),
+ reindexSkillVectors: vi.fn(),
+ reindexJobVectors: vi.fn(),
+}))
+
+vi.mock('#mcp/ui-artifacts-repo.ts', () => ({
+ listUiArtifactsByUserId: (...args: Array<unknown>) =>
+ mockModule.listUiArtifactsByUserId(...args),
+ updateUiArtifact: (...args: Array<unknown>) =>
+ mockModule.updateUiArtifact(...args),
+}))
+
+vi.mock('#mcp/skills/mcp-skills-repo.ts', () => ({
+ listMcpSkillsByUserId: (...args: Array<unknown>) =>
+ mockModule.listMcpSkillsByUserId(...args),
+ updateMcpSkill: (...args: Array<unknown>) =>
+ mockModule.updateMcpSkill(...args),
+}))
+
+vi.mock('#worker/jobs/repo.ts', () => ({
+ listJobRowsByUserId: (...args: Array<unknown>) =>
+ mockModule.listJobRowsByUserId(...args),
+ updateJobRow: (...args: Array<unknown>) => mockModule.updateJobRow(...args),
+}))
+
+vi.mock('#mcp/app-runner.ts', () => ({
+ syncSavedAppRunnerFromDb: (...args: Array<unknown>) =>
+ mockModule.syncSavedAppRunnerFromDb(...args),
+}))
+
+vi.mock('#mcp/ui-artifact-reindex.ts', () => ({
+ reindexUiArtifactVectors: (...args: Array<unknown>) =>
+ mockModule.reindexUiArtifactVectors(...args),
+}))
+
+vi.mock('#mcp/skills/skill-reindex.ts', () => ({
+ reindexSkillVectors: (...args: Array<unknown>) =>
+ mockModule.reindexSkillVectors(...args),
+}))
+
+vi.mock('#worker/jobs/job-reindex.ts', () => ({
+ reindexJobVectors: (...args: Array<unknown>) =>
+ mockModule.reindexJobVectors(...args),
+}))
+
+const { backfillRepoSources } = await import('./source-backfill.ts')
+
+test('backfill leaves app sourceId unchanged when Artifacts REST credentials are unavailable', async () => {
+ mockModule.listUiArtifactsByUserId.mockReset()
+ mockModule.updateUiArtifact.mockReset()
+ mockModule.listMcpSkillsByUserId.mockReset()
+ mockModule.updateMcpSkill.mockReset()
+ mockModule.listJobRowsByUserId.mockReset()
+ mockModule.updateJobRow.mockReset()
+ mockModule.syncSavedAppRunnerFromDb.mockReset()
+ mockModule.reindexUiArtifactVectors.mockReset()
+ mockModule.reindexSkillVectors.mockReset()
+ mockModule.reindexJobVectors.mockReset()
+
+ const appRow: UiArtifactRow = {
+ id: 'app-1',
+ user_id: 'user-1',
+ title: 'Investigate orphaned source ids',
+ description: 'Debug repro app',
+ sourceId: null,
+ clientCode: '<main>Hello</main>',
+ serverCode: null,
+ serverCodeId: 'server-code-1',
+ parameters: null,
+ hidden: true,
+ created_at: '2026-04-17T00:00:00.000Z',
+ updated_at: '2026-04-17T00:00:00.000Z',
+ }
+
+ mockModule.listUiArtifactsByUserId.mockResolvedValue([appRow])
+ mockModule.listMcpSkillsByUserId.mockResolvedValue([])
+ mockModule.listJobRowsByUserId.mockResolvedValue([])
+
+ const db = {
+ prepare() {
+ return {
+ bind() {
+ return {
+ async first() {
+ return null
+ },
+ }
+ },
+ }
+ },
+ } as unknown as D1Database
+
+ const result = await backfillRepoSources({
+ env: {
+ APP_DB: db,
+ } as Env,
+ userId: 'user-1',
+ baseUrl: 'https://heykody.dev',
+ dryRun: false,
+ includeApps: true,
+ includeSkills: false,
+ includeJobs: false,
+ reindex: false,
+ syncAppRunners: false,
+ })
+
+ expect(result.apps.errors).toBe(1)
+ expect(result.apps.results[0]).toEqual({
+ kind: 'app',
+ id: 'app-1',
+ title: 'Investigate orphaned source ids',
+ status: 'error',
+ reason:
+ 'Repo-backed source persistence requires CLOUDFLARE_ACCOUNT_ID and CLOUDFLARE_API_TOKEN.',
+ sourceId: null,
+ publishedCommit: null,
+ })
+ expect(mockModule.updateUiArtifact).not.toHaveBeenCalled()
+ expect(appRow.sourceId).toBeNull()
+})
diff --git a/packages/worker/src/repo/source-backfill.ts b/packages/worker/src/repo/source-backfill.ts
--- a/packages/worker/src/repo/source-backfill.ts
+++ b/packages/worker/src/repo/source-backfill.ts
@@ -213,12 +213,8 @@
entityKind: 'app',
entityId: input.row.id,
sourceRoot: '/',
+ requirePersistence: true,
})
- if (input.row.sourceId !== ensuredSource.id) {
- await updateUiArtifact(input.env.APP_DB, input.userId, input.row.id, {
- sourceId: ensuredSource.id,
- })
- }
const publishedCommit = await syncArtifactSourceSnapshot({
env: input.env,
userId: input.userId,
@@ -242,6 +238,11 @@
publishedCommit,
indexedCommit: publishedCommit,
})
+ if (input.row.sourceId !== ensuredSource.id) {
+ await updateUiArtifact(input.env.APP_DB, input.userId, input.row.id, {
+ sourceId: ensuredSource.id,
+ })
+ }
if (input.syncAppRunner) {
await syncSavedAppRunnerFromDb({
env: input.env,
@@ -319,27 +320,8 @@
entityKind: 'skill',
entityId: input.row.id,
sourceRoot: '/',
+ requirePersistence: true,
})
- if (input.row.source_id !== ensuredSource.id) {
- await updateMcpSkill(input.env.APP_DB, input.userId, input.row.name, {
- name: input.row.name,
- title: input.row.title,
- description: input.row.description,
- source_id: ensuredSource.id,
- keywords: input.row.keywords,
- code: input.row.code,
- search_text: input.row.search_text,
- uses_capabilities: input.row.uses_capabilities,
- parameters: input.row.parameters,
- collection_name: input.row.collection_name,
- collection_slug: input.row.collection_slug,
- inferred_capabilities: input.row.inferred_capabilities,
- inference_partial: input.row.inference_partial,
- read_only: input.row.read_only,
- idempotent: input.row.idempotent,
- destructive: input.row.destructive,
- })
- }
const publishedCommit = await syncArtifactSourceSnapshot({
env: input.env,
userId: input.userId,
@@ -370,6 +352,26 @@
publishedCommit,
indexedCommit: publishedCommit,
})
+ if (input.row.source_id !== ensuredSource.id) {
+ await updateMcpSkill(input.env.APP_DB, input.userId, input.row.name, {
+ name: input.row.name,
+ title: input.row.title,
+ description: input.row.description,
+ source_id: ensuredSource.id,
+ keywords: input.row.keywords,
+ code: input.row.code,
+ search_text: input.row.search_text,
+ uses_capabilities: input.row.uses_capabilities,
+ parameters: input.row.parameters,
+ collection_name: input.row.collection_name,
+ collection_slug: input.row.collection_slug,
+ inferred_capabilities: input.row.inferred_capabilities,
+ inference_partial: input.row.inference_partial,
+ read_only: input.row.read_only,
+ idempotent: input.row.idempotent,
+ destructive: input.row.destructive,
+ })
+ }
return {
kind: 'skill',
id: input.row.id,
@@ -459,19 +461,12 @@
entityKind: 'job',
entityId: input.row.record.id,
sourceRoot: '/',
+ requirePersistence: true,
})
const nextRecord = {
...input.row.record,
sourceId: ensuredSource.id,
}
- if (input.row.record.sourceId !== ensuredSource.id) {
- await updateJobRow({
- db: input.env.APP_DB,
- userId: input.userId,
- job: nextRecord,
- callerContextJson: input.row.callerContextJson,
- })
- }
const publishedCommit = await syncArtifactSourceSnapshot({
env: input.env,
userId: input.userId,
diff --git a/packages/worker/src/repo/source-service.node.test.ts b/packages/worker/src/repo/source-service.node.test.ts
new file mode 100644
--- /dev/null
+++ b/packages/worker/src/repo/source-service.node.test.ts
@@ -1,0 +1,33 @@
+import { expect, test } from 'vitest'
+
+const { ensureEntitySource } = await import('./source-service.ts')
... diff truncated: showing 800 of 957 linesYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 35c62f1. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
This was referenced Apr 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
APP_DBor Artifacts REST credentials so backfill/save flows cannot write orphanedsource_idreferences into D1artifactsconfig and document the REST-only production pathTesting
npm exec vitest run packages/worker/src/repo/artifacts.node.test.ts packages/worker/src/repo/source-service.node.test.ts packages/worker/src/repo/source-backfill.node.test.ts packages/worker/src/mcp/capabilities/repo/repo-backfill-sources.node.test.ts --project node-unitnpm exec vitest run packages/worker/src/repo/app-source.node.test.ts packages/worker/src/mcp/capabilities/apps/ui-save-app.node.test.ts packages/worker/src/mcp/observability.workers.test.ts --project node-unit --project workers-unitnpm run typecheck && npm run buildProduction evidence
wrangler 4.83.0warning that top-levelartifactsis unexpected and the deployed binding summary omitsenv.ARTIFACTSCLOUDFLARE_ACCOUNT_IDas an env var andCLOUDFLARE_API_TOKENas a Worker secret, which is what the REST-only Artifacts client usesSummary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Documentation