From 2573d87a0e313d237dac44ff5a9e076d99b317c9 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 4 Aug 2026 01:10:35 +0200 Subject: [PATCH] fix: recover workspace publication failures --- backend/src/routes/workspaces.ts | 10 ++ backend/src/workspaces/git-repository.ts | 38 +++++- backend/src/workspaces/registry.ts | 18 ++- backend/test/routes-workspaces.test.ts | 4 + backend/test/workspace-registry.test.ts | 148 ++++++++++++++++++++++- 5 files changed, 207 insertions(+), 11 deletions(-) diff --git a/backend/src/routes/workspaces.ts b/backend/src/routes/workspaces.ts index 99d9495a..5f727fd4 100644 --- a/backend/src/routes/workspaces.ts +++ b/backend/src/routes/workspaces.ts @@ -233,12 +233,22 @@ function errorReply(reply: FastifyReply, error: unknown) { const body: Record = { code, message: SAFE_MESSAGES[code] }; if (error instanceof WorkspaceConflictError) { body.fields = error.fields; + body.expected = error.expected; + body.actual = error.actual; if (error.base) body.base = error.base; if (error.local) body.local = error.local; if (error.remote) body.remote = error.remote; } else if (code === "workspace_conflict" && error && typeof error === "object") { const conflict = error as Partial; if (Array.isArray(conflict.fields) && conflict.fields.every((field) => typeof field === "string")) body.fields = conflict.fields; + for (const key of ["expected", "actual"] as const) { + const revision = conflict[key]; + if ( + revision + && typeof revision.commit === "string" && /^[0-9a-f]{40}$/.test(revision.commit) + && (revision.blob === undefined || (typeof revision.blob === "string" && /^[0-9a-f]{40}$/.test(revision.blob))) + ) body[key] = revision; + } for (const key of ["base", "local", "remote"] as const) { if (conflict[key] && isCanonicalWorkspace(conflict[key] as WorkspaceDescriptor)) body[key] = conflict[key]; } diff --git a/backend/src/workspaces/git-repository.ts b/backend/src/workspaces/git-repository.ts index 61db4b99..daca086b 100644 --- a/backend/src/workspaces/git-repository.ts +++ b/backend/src/workspaces/git-repository.ts @@ -172,10 +172,17 @@ export class GitWorkspaceRepository { if (paths.length === 0 || paths.some((path) => !this.isRegistryArtifactPath(path))) { throw new WorkspaceRegistryError("workspace_invalid", "Workspace repository path is invalid"); } - await this.git(["add", "--", ...paths]); - await this.git(["commit", "-m", message]); - await this.git(["push", "origin", `HEAD:${this.config.branch}`]); - return await this.status(); + try { + await this.git(["add", "--", ...paths]); + await this.git(["commit", "-m", message], this.publicationIdentity()); + await this.git(["push", "origin", `HEAD:${this.config.branch}`]); + return await this.status(); + } catch (error) { + // A failed commit leaves staged/working changes; a failed push leaves an ahead commit. + // Restore the last fetched remote revision so the next refresh or explicit retry starts clean. + await this.restoreFailedPublication(); + throw error; + } } private async clone(): Promise { @@ -223,12 +230,31 @@ export class GitWorkspaceRepository { } } - private async git(args: string[]): Promise { + private publicationIdentity(): NodeJS.ProcessEnv { + return { + GIT_AUTHOR_NAME: this.config.gitAuthorName, + GIT_AUTHOR_EMAIL: this.config.gitAuthorEmail, + GIT_COMMITTER_NAME: this.config.gitAuthorName, + GIT_COMMITTER_EMAIL: this.config.gitAuthorEmail, + }; + } + + private async restoreFailedPublication(): Promise { + try { + await this.git(["reset", "--hard", `refs/remotes/origin/${this.config.branch}`]); + await this.git(["clean", "-fd", "--", "workspaces", "workspace-docs"]); + } catch { + // Keep the original sanitized publish failure. A future refresh will surface any recovery + // problem without leaking the Git failure details through the API. + } + } + + private async git(args: string[], env: NodeJS.ProcessEnv = {}): Promise { try { const { stdout } = await execFileAsync( "git", ["-c", `core.hooksPath=${this.hooksPath}`, ...args], - { cwd: this.repoPath, env: { ...process.env, GIT_TERMINAL_PROMPT: "0" } }, + { cwd: this.repoPath, env: { ...process.env, GIT_TERMINAL_PROMPT: "0", ...env } }, ); return stdout; } catch (error) { diff --git a/backend/src/workspaces/registry.ts b/backend/src/workspaces/registry.ts index 4ab35bb7..fc7e1c7f 100644 --- a/backend/src/workspaces/registry.ts +++ b/backend/src/workspaces/registry.ts @@ -36,6 +36,8 @@ export type PublishWorkspaceRequest = export class WorkspaceConflictError extends WorkspaceRegistryError { constructor( readonly fields: string[], + readonly expected: { commit: string; blob?: string }, + readonly actual: { commit: string; blob?: string }, readonly base?: CanonicalWorkspace, readonly local?: CanonicalWorkspace, readonly remote?: CanonicalWorkspace, @@ -168,10 +170,10 @@ export class WorkspaceRegistry { if (request.baseCommit !== status.head || ( request.action !== "create" && existing?.blob !== request.baseBlob )) { - throw await this.conflictFor(request, existing, local); + throw await this.conflictFor(request, status.head!, existing, local); } - if (request.action === "create" && existing) throw await this.conflictFor(request, existing, local); - if (request.action !== "create" && !existing) throw await this.conflictFor(request, existing, local); + if (request.action === "create" && existing) throw await this.conflictFor(request, status.head!, existing, local); + if (request.action !== "create" && !existing) throw await this.conflictFor(request, status.head!, existing, local); const yamlPath = workspacePath(id); const docPaths = this.documentationPaths(id); @@ -205,6 +207,7 @@ export class WorkspaceRegistry { private async conflictFor( request: PublishWorkspaceRequest, + currentCommit: string, existing: WorkspaceRevision | undefined, local: CanonicalWorkspace | undefined, ): Promise { @@ -215,7 +218,14 @@ export class WorkspaceRegistry { const read = await this.read(id); remote = isCanonicalWorkspace(read.workspace) ? read.workspace : undefined; } - return new WorkspaceConflictError(this.changedFields(base, remote), base, local, remote); + return new WorkspaceConflictError( + this.changedFields(base, remote), + { commit: request.baseCommit, ...(request.action === "create" ? {} : { blob: request.baseBlob }) }, + { commit: currentCommit, ...(existing ? { blob: existing.blob } : {}) }, + base, + local, + remote, + ); } private async readSnapshotCanonical(commit: string, id: string): Promise { diff --git a/backend/test/routes-workspaces.test.ts b/backend/test/routes-workspaces.test.ts index efd5e3a7..bb47cfaf 100644 --- a/backend/test/routes-workspaces.test.ts +++ b/backend/test/routes-workspaces.test.ts @@ -194,6 +194,8 @@ test("returns a 409 field conflict instead of overwriting a changed workspace", new WorkspaceRegistryError("workspace_conflict", "Workspace has changed"), { fields: ["semantic_index.embedding.model"], + expected: { commit: "c".repeat(40), blob: "d".repeat(40) }, + actual: { commit: revision.commit, blob: revision.blob }, base: workspace, local: { ...workspace, semantic_index: { ...workspace.semantic_index, embedding: { ...workspace.semantic_index.embedding, model: "local/model" } } }, remote: { ...workspace, semantic_index: { ...workspace.semantic_index, embedding: { ...workspace.semantic_index.embedding, model: "remote/model" } } }, @@ -214,6 +216,8 @@ test("returns a 409 field conflict instead of overwriting a changed workspace", expect(res.json()).toMatchObject({ code: "workspace_conflict", fields: ["semantic_index.embedding.model"], + expected: { commit: "c".repeat(40), blob: "d".repeat(40) }, + actual: { commit: revision.commit, blob: revision.blob }, base: workspace, remote: expect.objectContaining({ semantic_index: expect.objectContaining({ diff --git a/backend/test/workspace-registry.test.ts b/backend/test/workspace-registry.test.ts index 99296b83..6d5fd73a 100644 --- a/backend/test/workspace-registry.test.ts +++ b/backend/test/workspace-registry.test.ts @@ -9,6 +9,7 @@ import { promisify } from "node:util"; import { afterEach, expect, test } from "vitest"; import { WorkspaceRepositoryLock } from "../src/workspaces/git-repository.js"; import { WorkspaceRegistry } from "../src/workspaces/registry.js"; +import { parseWorkspaceYaml, type CanonicalWorkspace } from "../src/workspaces/schema.js"; import type { WorkspaceRegistryConfig } from "../src/workspaces/types.js"; const validYaml = `workspace: @@ -49,6 +50,11 @@ async function git(cwd: string, args: string[]): Promise { await runFile("git", args, { cwd }); } +async function gitOutput(cwd: string, args: string[]): Promise { + const { stdout } = await runFile("git", args, { cwd }); + return stdout.trim(); +} + async function fixture(workspaceSource = validYaml): Promise<{ root: string; remote: string; source: string; initialCommit: string; }> { @@ -71,7 +77,11 @@ async function fixture(workspaceSource = validYaml): Promise<{ return { root, remote, source, initialCommit: stdout.trim() }; } -function config(root: string, remoteUrl: string): WorkspaceRegistryConfig { +function config( + root: string, + remoteUrl: string, + overrides: Partial = {}, +): WorkspaceRegistryConfig { return { root, remoteUrl, @@ -82,6 +92,25 @@ function config(root: string, remoteUrl: string): WorkspaceRegistryConfig { secretRoots: [], maxImportBytes: 1024, maxImportEntries: 1, + ...overrides, + }; +} + +function workspaceWith( + id: string, + changes: Partial> = {}, +): CanonicalWorkspace { + const workspace = parseWorkspaceYaml(validYaml) as CanonicalWorkspace; + return { + ...workspace, + workspace: { ...workspace.workspace, id, name: id, ...changes }, + }; +} + +async function checkoutStatus(checkout: string): Promise<{ porcelain: string; divergence: string }> { + return { + porcelain: await gitOutput(checkout, ["status", "--porcelain"]), + divergence: await gitOutput(checkout, ["rev-list", "--left-right", "--count", "HEAD...@{upstream}"]), }; } @@ -134,6 +163,123 @@ test("bootstraps a checkout and activates a validated immutable snapshot", async }); }); +test("publishes create, update, and delete with the configured Git author identity", async () => { + const remote = await fixture(); + const registry = new WorkspaceRegistry(config(join(remote.root, "registry"), remote.remote, { + gitAuthorName: "Configured Workspace Publisher", + gitAuthorEmail: "publisher@example.invalid", + })); + await registry.bootstrap(); + const createdWorkspace = workspaceWith("research-registry", { name: "Research registry" }); + + const created = await registry.publish({ + action: "create", + workspace: createdWorkspace, + baseCommit: remote.initialCommit, + }); + + expect(created).toMatchObject({ id: "research-registry", commit: expect.stringMatching(/^[0-9a-f]{40}$/) }); + expect(await gitOutput(remote.root, ["--git-dir", remote.remote, "log", "-1", "--format=%an <%ae>"])).toBe( + "Configured Workspace Publisher ", + ); + await expect(runFile("git", ["--git-dir", remote.remote, "cat-file", "-e", "HEAD:workspace-docs/research-registry/README.md"], { + cwd: remote.root, + })).resolves.toBeDefined(); + + const updated = await registry.publish({ + action: "update", + workspace: workspaceWith("research-registry", { description: "Updated workspace description" }), + baseCommit: created!.commit, + baseBlob: created!.blob, + }); + + expect(updated).toMatchObject({ id: "research-registry" }); + expect(await gitOutput(remote.root, ["--git-dir", remote.remote, "show", "HEAD:workspaces/research-registry.yaml"])).toContain( + "description: Updated workspace description", + ); + + await expect(registry.publish({ + action: "delete", + id: "research-registry", + baseCommit: updated!.commit, + baseBlob: updated!.blob, + })).resolves.toBeUndefined(); + await expect(runFile("git", ["--git-dir", remote.remote, "cat-file", "-e", "HEAD:workspaces/research-registry.yaml"], { + cwd: remote.root, + })).rejects.toBeDefined(); +}); + +test("reports stale publish conflicts with expected and actual revisions", async () => { + const remote = await fixture(); + const root = join(remote.root, "registry"); + const registry = new WorkspaceRegistry(config(root, remote.remote)); + await registry.bootstrap(); + const initial = await registry.read("psd-clinical"); + writeFileSync(join(remote.source, "workspaces", "psd-clinical.yaml"), validYaml.replace( + "model: nomic-embed-text-v2-moe", "model: mxbai-embed-large", + )); + await git(remote.source, ["add", "workspaces/psd-clinical.yaml"]); + await git(remote.source, ["commit", "-m", "Change embedding model"]); + await git(remote.source, ["push", "origin", "main"]); + const actualCommit = await gitOutput(remote.source, ["rev-parse", "HEAD"]); + const actualBlob = await gitOutput(remote.source, ["rev-parse", "HEAD:workspaces/psd-clinical.yaml"]); + + await expect(registry.publish({ + action: "update", + workspace: workspaceWith("psd-clinical", { description: "Local stale change" }), + baseCommit: initial.revision.commit, + baseBlob: initial.revision.blob, + })).rejects.toMatchObject({ + code: "workspace_conflict", + fields: ["semantic_index.embedding.model"], + expected: { commit: initial.revision.commit, blob: initial.revision.blob }, + actual: { commit: actualCommit, blob: actualBlob }, + }); +}); + +test("restores a clean checkout after a failed commit and retries publication", async () => { + const remote = await fixture(); + const root = join(remote.root, "registry"); + const registry = new WorkspaceRegistry(config(root, remote.remote)); + await registry.bootstrap(); + const objects = join(root, "repo", ".git", "objects"); + chmodSync(objects, 0o500); + const request = { + action: "create" as const, + workspace: workspaceWith("commit-recovery"), + baseCommit: remote.initialCommit, + }; + + try { + await expect(registry.publish(request)).rejects.toMatchObject({ code: "git_unavailable" }); + } finally { + chmodSync(objects, 0o700); + } + expect(await checkoutStatus(join(root, "repo"))).toEqual({ porcelain: "", divergence: "0\t0" }); + await expect(registry.pull()).resolves.toMatchObject({ head: remote.initialCommit }); + await expect(registry.publish(request)).resolves.toMatchObject({ id: "commit-recovery" }); +}); + +test("resets an ahead checkout after a rejected push and retries publication", async () => { + const remote = await fixture(); + const root = join(remote.root, "registry"); + const registry = new WorkspaceRegistry(config(root, remote.remote)); + await registry.bootstrap(); + const hook = join(remote.remote, "hooks", "pre-receive"); + writeFileSync(hook, "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + const request = { + action: "create" as const, + workspace: workspaceWith("push-recovery"), + baseCommit: remote.initialCommit, + }; + + await expect(registry.publish(request)).rejects.toMatchObject({ code: "git_push_rejected" }); + expect(await checkoutStatus(join(root, "repo"))).toEqual({ porcelain: "", divergence: "0\t0" }); + rmSync(hook); + await expect(registry.pull()).resolves.toMatchObject({ head: remote.initialCommit }); + await expect(registry.publish(request)).resolves.toMatchObject({ id: "push-recovery" }); +}); + test("lists a v1 descriptor in migration-required state without rendering operational artifacts", async () => { const legacyYaml = validYaml.replace( " database: postgres\n schema: vectors\n",