Skip to content

Commit f7e3123

Browse files
karishnukarishnu
committed
fix(space): stabilize artifacts sync and preview deploys
1 parent 645bb5b commit f7e3123

9 files changed

Lines changed: 551 additions & 118 deletions

File tree

‎space/src/space/artifacts-fs.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
* branch.
44
*
55
* It composes two layers:
6-
* - overlay (writable): a real `WorkspaceFileSystem` (SQLite). Holds all
7-
* writes/edits, the `.git` dir, hydrated file contents, and its own
8-
* bookkeeping under `/.afs`.
6+
* - overlay (writable): an in-memory `InMemoryFs`. Holds all writes/edits,
7+
* the `.git` dir, hydrated file contents, and its own bookkeeping under
8+
* `/.afs`. Not durable across DO eviction — Artifacts is the source of truth.
99
* - base (read-only): an immutable snapshot of the imported Artifacts branch
1010
* (`path -> { oid, mode }`) whose blobs live in the overlay's `.git` object
1111
* store after one packfile fetch.
@@ -15,9 +15,9 @@
1515
* directory listing (or `whenFullyMaterialized()`) copies the whole base into
1616
* the overlay. Once fully materialized, the FS behaves as a plain overlay.
1717
*
18-
* When no base source is available (local dev without the ARTIFACTS binding),
19-
* every operation passes straight through to the overlay — an exact
20-
* `WorkspaceFileSystem` equivalent.
18+
* When the base snapshot is empty (an Artifacts repo with no commits yet),
19+
* every operation passes straight through to the overlay — an exact in-memory
20+
* overlay equivalent — until the first commit establishes a base.
2121
*/
2222
import type { FileSystem, FsStat, EntryType } from "@cloudflare/shell"
2323
import type { BaseEntry, BaseSnapshotSource } from "./git-objects"

‎space/src/space/artifacts-sync.ts‎

Lines changed: 163 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
* Artifacts is a git-compatible, versioned remote (see
55
* https://developers.cloudflare.com/artifacts/). Each app gets its own repo,
66
* and the SpaceDO mirrors every commit/deploy there via `git push` so Artifacts
7-
* is the durable source of truth for history — the SpaceDO's local isomorphic-git
8-
* (backed by the Workspace SQLite FS) remains the live working tree used to
9-
* build/serve previews.
7+
* is the durable source of truth — the SpaceDO's local isomorphic-git (backed by
8+
* an in-memory overlay FS) is the live working tree used to build/serve previews
9+
* and is rehydrated from Artifacts on cold start.
1010
*
1111
* All operations here are best-effort: Artifacts is a beta product and may be
1212
* absent in local dev (no binding). Failures are logged and reported via return
@@ -19,6 +19,31 @@ const REMOTE_NAME = "artifacts"
1919
const TOKEN_REFRESH_SKEW_MS = 60_000
2020
/** Requested token lifetime (seconds). Artifacts allows 60s..1y. */
2121
const TOKEN_TTL_SECONDS = 3600
22+
/** Max attempts for a single push before giving up (best-effort). */
23+
const PUSH_MAX_ATTEMPTS = 4
24+
/** Base backoff between push retries; grows linearly per attempt. */
25+
const PUSH_RETRY_BASE_MS = 100
26+
27+
const sleep = (ms: number): Promise<void> => new Promise((r) => setTimeout(r, ms))
28+
29+
/**
30+
* Detect a rejected push that a retry can plausibly resolve.
31+
*
32+
* The push protocol sends the ref's *current* server value (`oldoid`, read
33+
* from a fresh ref advertisement) as the expected old value. If another writer
34+
* updates the ref between that advertisement and the receive-pack, the server
35+
* rejects the update as "stale info"/"stale ref" — even under `force`, because
36+
* the mismatch is detected server-side. Re-running the push re-reads a fresh
37+
* `oldoid`, so a bounded retry converges. isomorphic-git surfaces this as a
38+
* `GitPushError`; treat those (and explicit stale wording) as retryable.
39+
*/
40+
function isRetryablePushError(err: unknown): boolean {
41+
const e = err as { code?: string; message?: string; data?: unknown } | null
42+
if (!e) return false
43+
if (e.code === "GitPushError") return true
44+
const text = String(e.message ?? err)
45+
return /stale|not-fast-forward|rejected|fetch first|failed to update ref/i.test(text)
46+
}
2247

2348
/**
2449
* Artifacts repo names allow alphanumerics, dots, hyphens and underscores.
@@ -35,17 +60,52 @@ export interface ArtifactsSyncLogger {
3560
info?(message: string, ...args: unknown[]): void
3661
}
3762

63+
/**
64+
* Durable persistence for the repo's git remote URL. The URL is only ever
65+
* surfaced as a by-value result of `create()`/`import()`; a `get()` handle
66+
* exposes methods only (its data properties are unreadable across the local-dev
67+
* remote-binding proxy). Cloudflare's guidance is to save the value — so the
68+
* SpaceDO backs this with durable DO storage, which survives eviction even
69+
* though the in-memory working tree does not.
70+
*/
71+
export interface ArtifactsRemoteStore {
72+
read(): Promise<string | null>
73+
write(remoteUrl: string): Promise<void>
74+
}
75+
76+
/** Dispose an RPC stub/result if it is disposable; never throws. */
77+
function disposeQuietly(value: unknown): void {
78+
// `Symbol.dispose` is not in the ES2022 lib we target, so look it up at
79+
// runtime instead of referencing it statically.
80+
const disposeSym = (Symbol as unknown as { dispose?: symbol }).dispose
81+
if (!disposeSym || value == null) return
82+
try {
83+
const fn = (value as Record<symbol, unknown>)[disposeSym]
84+
if (typeof fn === "function") (fn as () => void).call(value)
85+
} catch {
86+
// best-effort cleanup
87+
}
88+
}
89+
3890
export class ArtifactsSync {
3991
private readonly repoName: string
4092
private remoteUrl: string | null = null
4193
private remoteRegistered = false
4294
private token: string | null = null
4395
private tokenExpiresAt = 0
96+
/**
97+
* Serializes pushes within this DO instance. Concurrent pushes to the same
98+
* branch (e.g. a commit's fire-and-forget mirror overlapping a deploy's push,
99+
* or rapid successive commits) race on the ref advertisement and get rejected
100+
* as "stale ref". Chaining pushes eliminates that self-inflicted race.
101+
*/
102+
private pushQueue: Promise<boolean> = Promise.resolve(true)
44103

45104
constructor(
46105
private readonly artifacts: Artifacts,
47106
private readonly git: Git,
48107
repoName: string,
108+
private readonly store?: ArtifactsRemoteStore,
49109
private readonly logger: ArtifactsSyncLogger = console,
50110
) {
51111
this.repoName = sanitizeRepoName(repoName)
@@ -55,34 +115,21 @@ export class ArtifactsSync {
55115
* Ensure the app's Artifacts repo exists and the local git repo has an
56116
* `artifacts` remote pointing at it. Idempotent. Returns false (and logs) if
57117
* the repo could not be ensured — callers should treat sync as unavailable.
118+
*
119+
* Resolution order for the remote URL: in-memory cache -> durable store ->
120+
* provision (create, persist). This means a cold-started DO (empty in-memory
121+
* FS) recovers the remote from durable storage without any `get()` handle.
58122
*/
59123
private async ensureRepo(): Promise<boolean> {
60124
if (this.remoteRegistered && this.remoteUrl) return true
61125

62126
try {
63-
let remote: string
64-
try {
65-
const existing = await this.artifacts.get(this.repoName)
66-
remote = existing.remote
67-
} catch {
68-
// Not found (or not ready) — create it. `create` throws ALREADY_EXISTS
69-
// on a race; fall back to get in that case.
70-
try {
71-
const created = await this.artifacts.create(this.repoName, {
72-
setDefaultBranch: "main",
73-
})
74-
remote = created.remote
75-
// `create` hands back an initial token — reuse it to avoid an extra
76-
// round-trip on the first push.
77-
this.token = created.token
78-
this.tokenExpiresAt = Date.parse(created.tokenExpiresAt) || 0
79-
} catch {
80-
const existing = await this.artifacts.get(this.repoName)
81-
remote = existing.remote
82-
}
83-
}
127+
let remote = this.remoteUrl ?? (await this.store?.read()) ?? null
128+
if (!remote) remote = await this.provisionRemote()
129+
if (!remote) return false
84130

85131
this.remoteUrl = remote
132+
await this.store?.write(remote)
86133
await this.registerRemote(remote)
87134
this.remoteRegistered = true
88135
return true
@@ -92,6 +139,49 @@ export class ArtifactsSync {
92139
}
93140
}
94141

142+
/**
143+
* Obtain the repo's git remote URL, creating the repo if needed.
144+
*
145+
* Only `create()`/`import()` return the `remote` as a by-value RPC result; a
146+
* `get()` handle exposes methods only, and its data properties cannot be read
147+
* across the remote-binding proxy used in local dev (they resolve as method
148+
* calls and throw "does not implement the method"). So we create first and,
149+
* on ALREADY_EXISTS, fall back to reading the handle's `remote` — which works
150+
* with the native binding in production; in local dev it may fail, in which
151+
* case sync degrades to unavailable (best-effort) until a value is persisted.
152+
*/
153+
private async provisionRemote(): Promise<string | null> {
154+
let created: ArtifactsCreateRepoResult | null = null
155+
try {
156+
created = await this.artifacts.create(this.repoName, { setDefaultBranch: "main" })
157+
const remote = await created.remote
158+
try {
159+
// Reuse the initial token to avoid an extra round-trip on first push.
160+
this.token = await created.token
161+
this.tokenExpiresAt = Date.parse(await created.tokenExpiresAt) || 0
162+
} catch {
163+
// Token unreadable here — getWriteToken() will mint one on demand.
164+
}
165+
return remote
166+
} catch {
167+
const repo = await this.artifacts.get(this.repoName).catch(() => null)
168+
if (!repo) return null
169+
try {
170+
return await repo.remote
171+
} catch (e) {
172+
this.logger.warn(
173+
"ArtifactsSync: repo exists but its remote URL is not readable and none is persisted; sync unavailable",
174+
e,
175+
)
176+
return null
177+
} finally {
178+
disposeQuietly(repo)
179+
}
180+
} finally {
181+
if (created) disposeQuietly(created)
182+
}
183+
}
184+
95185
private async registerRemote(url: string): Promise<void> {
96186
try {
97187
await this.git.remote({ add: { name: REMOTE_NAME, url } })
@@ -112,39 +202,73 @@ export class ArtifactsSync {
112202
if (this.token && Date.now() < this.tokenExpiresAt - TOKEN_REFRESH_SKEW_MS) {
113203
return this.token
114204
}
205+
const repo = await this.artifacts.get(this.repoName).catch(() => null)
206+
if (!repo) {
207+
this.logger.warn("ArtifactsSync.getWriteToken failed to get repo handle")
208+
return null
209+
}
115210
try {
116-
const repo = await this.artifacts.get(this.repoName)
211+
// createToken() is a method call whose result is a by-value plain object,
212+
// so its `plaintext`/`expiresAt` fields are directly readable.
117213
const result = await repo.createToken("write", TOKEN_TTL_SECONDS)
118214
this.token = result.plaintext
119215
this.tokenExpiresAt = Date.parse(result.expiresAt) || 0
120216
return this.token
121217
} catch (e) {
122218
this.logger.warn("ArtifactsSync.getWriteToken failed", e)
123219
return null
220+
} finally {
221+
disposeQuietly(repo)
124222
}
125223
}
126224

127225
/**
128226
* Mirror `branch` to Artifacts. Best-effort: returns true on success, false
129227
* (with a logged warning) otherwise. Never throws.
228+
*
229+
* Pushes are serialized per DO instance (see `pushQueue`) so overlapping
230+
* callers can't race each other into a "stale ref" rejection, and each push
231+
* is retried with backoff to absorb races from any other writer.
130232
*/
131233
async push(branch: string): Promise<boolean> {
234+
const run = this.pushQueue.then(
235+
() => this.pushWithRetry(branch),
236+
() => this.pushWithRetry(branch),
237+
)
238+
// Keep the chain alive regardless of this push's outcome.
239+
this.pushQueue = run.catch(() => false)
240+
return run
241+
}
242+
243+
private async pushWithRetry(branch: string): Promise<boolean> {
132244
if (!(await this.ensureRepo())) return false
133-
const token = await this.getWriteToken()
134-
if (!token) return false
135-
try {
136-
await this.git.push({
137-
remote: REMOTE_NAME,
138-
ref: branch,
139-
force: true,
140-
username: "x",
141-
password: token,
142-
})
143-
return true
144-
} catch (e) {
145-
this.logger.warn(`ArtifactsSync.push failed for branch "${branch}"`, e)
146-
return false
245+
246+
for (let attempt = 1; attempt <= PUSH_MAX_ATTEMPTS; attempt++) {
247+
const token = await this.getWriteToken()
248+
if (!token) return false
249+
try {
250+
await this.git.push({
251+
remote: REMOTE_NAME,
252+
ref: branch,
253+
force: true,
254+
username: "x",
255+
password: token,
256+
})
257+
return true
258+
} catch (e) {
259+
const retryable = isRetryablePushError(e)
260+
if (retryable && attempt < PUSH_MAX_ATTEMPTS) {
261+
this.logger.warn(
262+
`ArtifactsSync.push retry ${attempt}/${PUSH_MAX_ATTEMPTS} for branch "${branch}" (stale ref race)`,
263+
)
264+
await sleep(PUSH_RETRY_BASE_MS * attempt)
265+
continue
266+
}
267+
this.logger.warn(`ArtifactsSync.push failed for branch "${branch}"`, e)
268+
return false
269+
}
147270
}
271+
return false
148272
}
149273

150274
/**

‎space/src/space/deploy-engine.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import type { Git } from "@cloudflare/shell/git"
2-
import type { Workspace } from "@cloudflare/shell"
2+
import type { FileSystem } from "@cloudflare/shell"
33
import { createApp, createWorker, type AssetConfig, type Modules } from "@cloudflare/worker-bundler"
44
import { parseWranglerConfig, WranglerConfigError } from "./wrangler-config"
5+
import { globInfos } from "./fileinfo"
56

67
// ─── Deploy Engine ──────────────────────────────────────────────────────────
78

@@ -16,7 +17,7 @@ function jsonResponse(data: unknown, status: number = 200): Response {
1617
export interface DeployContext {
1718
sql: SqlStorage
1819
git: Git
19-
workspace: Workspace
20+
fs: FileSystem
2021
}
2122

2223
export async function handleDeployCommand(
@@ -56,10 +57,10 @@ async function readBranchFiles(
5657
const commitHash = log[0].oid
5758

5859
// Checkout the branch to populate working tree
59-
await ctx.git.checkout({ ref: branch })
60+
await ctx.git.checkout({ ref: branch, force: true })
6061

6162
// Read all files recursively (readDir is non-recursive, glob is)
62-
const allFiles = await ctx.workspace.glob("**/*")
63+
const allFiles = await globInfos(ctx.fs, "**/*")
6364
const files: Record<string, string> = {}
6465

6566
for (const fileInfo of allFiles) {
@@ -69,11 +70,14 @@ async function readBranchFiles(
6970
if (fileInfo.path.startsWith("/.git/") || fileInfo.path === "/.git") continue
7071
if (fileInfo.path.startsWith("/.afs/") || fileInfo.path === "/.afs") continue
7172

72-
const content = await ctx.workspace.readFile(fileInfo.path)
73-
if (content !== null) {
74-
const path = fileInfo.path.startsWith("/") ? fileInfo.path.slice(1) : fileInfo.path
75-
files[path] = content
73+
let content: string
74+
try {
75+
content = await ctx.fs.readFile(fileInfo.path)
76+
} catch {
77+
continue
7678
}
79+
const path = fileInfo.path.startsWith("/") ? fileInfo.path.slice(1) : fileInfo.path
80+
files[path] = content
7781
}
7882

7983
return { commitHash, files }

0 commit comments

Comments
 (0)