From 3b23cf3714644a832ab51dcfe630409535a3ed8c Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 4 Aug 2026 08:52:11 +0200 Subject: [PATCH] fix: retain snapshots for removed workspaces --- backend/src/routes/sessions.ts | 19 +++++++++++-- backend/src/workspaces/registry.ts | 38 +++++++++++++++++++++++++ backend/test/routes-sessions.test.ts | 35 +++++++++++++++++++++++ backend/test/workspace-registry.test.ts | 27 ++++++++++++++++++ 4 files changed, 116 insertions(+), 3 deletions(-) diff --git a/backend/src/routes/sessions.ts b/backend/src/routes/sessions.ts index afe2b7b8..9b0163c6 100644 --- a/backend/src/routes/sessions.ts +++ b/backend/src/routes/sessions.ts @@ -72,6 +72,15 @@ export function sessionRoutes( return typeof runner.withPrincipal === "function" ? runner.withPrincipal(principal) : runner; }; + /** Include retained historical descriptors so removed workspaces remain resumable. */ + const sessionRevisions = async () => { + const registry = d.workspaceRegistry as Partial; + if (typeof registry.listRetainedSnapshots === "function") { + return await registry.listRetainedSnapshots(); + } + return await d.workspaceRegistry.list(); + }; + const isNotFound = (error: unknown) => /not found|non trovata|inesistente|404/i.test(error instanceof Error ? error.message : String(error)); @@ -107,7 +116,7 @@ export function sessionRoutes( }; let revisions: Awaited>; try { - revisions = await d.workspaceRegistry.list(); + revisions = await sessionRevisions(); } catch (registryError) { // Sessions created before revision pinning still live under the installation's legacy // default config. Keep that compatibility path available when a fresh installation has @@ -394,11 +403,15 @@ export function sessionRoutes( // Admin RLS is deliberately disabled for a normal 'mine' listing. const scopedPrincipal = scope === "mine" ? { ...principal, isAdmin: false } : principal; const runner = runnerFor(scopedPrincipal); - const revisions = await d.workspaceRegistry.list(); + const revisions = await sessionRevisions(); const lists = await Promise.all(revisions .filter((revision) => revision.state === "operational") .map((revision) => runner.sessionList(revision.snapshotPath) as Promise)); - const list = lists.flat(); + const sessions = new Map(); + for (const row of lists.flat()) { + if (!sessions.has(row.id)) sessions.set(row.id, row); + } + const list = [...sessions.values()]; // Only an administrator-visible complete list (or the single local principal) is safe // input for retention. A remote per-user view can never discard another principal's pin. const reconcileSnapshotRetention = (d.workspaceRegistry as Partial).reconcileSnapshotRetention; diff --git a/backend/src/workspaces/registry.ts b/backend/src/workspaces/registry.ts index a9c6a8b0..fa983371 100644 --- a/backend/src/workspaces/registry.ts +++ b/backend/src/workspaces/registry.ts @@ -141,6 +141,31 @@ export class WorkspaceRegistry { return (await this.activeState()).revisions; } + /** + * List every intact retained snapshot, current snapshots first. Session discovery and + * retention use this rather than only the active revision so removing a workspace from + * Git cannot strand a resumable session that still pins one of its older descriptors. + */ + async listRetainedSnapshots(): Promise { + await this.repository.ensureLayout(); + return await this.lock.run(async () => { + try { + const active = await this.activeState(); + const revisions = [...active.revisions]; + const entries = await readdir(this.repository.snapshotsPath, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory() || entry.isSymbolicLink() || !/^[0-9a-f]{40}$/.test(entry.name)) continue; + if (entry.name === active.head) continue; + const state = await this.snapshotState(entry.name); + revisions.push(...state.revisions.filter((revision) => revision.state === "operational")); + } + return revisions; + } catch (error) { + throw workspaceError(error); + } + }); + } + async read(id: string): Promise<{ workspace: WorkspaceDescriptor; revision: WorkspaceRevision }> { const state = await this.activeState(); const revision = state.revisions.find((candidate) => candidate.id === id); @@ -550,6 +575,19 @@ export class WorkspaceRegistry { return JSON.parse(await readFile(path, "utf8")); } + private async snapshotState(head: string): Promise { + const manifest = await this.readSnapshotManifest(safeCommit(head)); + if (this.isLegacySnapshotManifest(manifest)) { + const state = await this.deriveStateFromLegacyRevisions(manifest); + await this.migrateLegacySnapshotManifest(state, manifest); + return state; + } + const state = manifest as ActiveState; + this.assertActiveState(state); + await this.assertSnapshotIntegrity(state); + return state; + } + private async assertSnapshotIntegrity(state: ActiveState): Promise { const directory = join(this.repository.snapshotsPath, state.head); try { diff --git a/backend/test/routes-sessions.test.ts b/backend/test/routes-sessions.test.ts index a1d0697a..eef17d4e 100644 --- a/backend/test/routes-sessions.test.ts +++ b/backend/test/routes-sessions.test.ts @@ -135,6 +135,41 @@ test("an administrator session listing retains revisions referenced by resumable expect(retained).toHaveBeenCalledWith([retainedRevision]); }); +test("retention scans a removed workspace's retained snapshot", async () => { + const retained = vi.fn(async () => {}); + const removedRevision = "e".repeat(40); + const activeSnapshot = "/registry/snapshots/a/other.yaml"; + const removedSnapshot = "/registry/snapshots/e/removed.yaml"; + const listRetainedSnapshots = vi.fn(async () => [ + { id: "other", commit: "a".repeat(40), state: "operational", snapshotPath: activeSnapshot }, + { id: "removed", commit: removedRevision, state: "operational", snapshotPath: removedSnapshot }, + ]); + const app = buildApp(loadConfig({ AUTH_MODE: "upstream", THT_HARNESS_DIR: "../harness" }), { + thtRunner: { + withPrincipal: () => ({ + sessionList: async (snapshotPath: string) => snapshotPath === removedSnapshot + ? [{ id: "resumable", status: "closed", archived: false, workspace_revision: removedRevision }] + : [], + }), + } as any, + workspaceRegistry: { + list: async () => [{ id: "other", commit: "a".repeat(40), state: "operational", snapshotPath: activeSnapshot }], + listRetainedSnapshots, + reconcileSnapshotRetention: retained, + } as any, + }); + + const response = await app.inject({ + method: "GET", url: "/sessions?scope=all", + headers: { ...aliceHeaders, "x-thoth-is-admin": "1" }, + }); + + expect(response.statusCode).toBe(200); + expect(listRetainedSnapshots).toHaveBeenCalledOnce(); + expect(retained).toHaveBeenCalledWith([removedRevision]); + expect(response.json()).toEqual([expect.objectContaining({ id: "resumable" })]); +}); + test("the single local installation listing reconciles its resumable workspace pins", async () => { const retained = vi.fn(async () => {}); const retainedRevision = "d".repeat(40); diff --git a/backend/test/workspace-registry.test.ts b/backend/test/workspace-registry.test.ts index eafc48eb..e3384b53 100644 --- a/backend/test/workspace-registry.test.ts +++ b/backend/test/workspace-registry.test.ts @@ -522,6 +522,33 @@ test("retains a historical snapshot while a resumable manifest still references expect(existsSync(registry.snapshotPath(currentCommit, "psd-clinical"))).toBe(true); }); +test("lists operational descriptors retained after their workspace was removed from the active revision", async () => { + const remote = await fixture(); + const root = join(remote.root, "registry"); + const registry = new WorkspaceRegistry(config(root, remote.remote)); + await registry.bootstrap(); + + writeFileSync(join(remote.source, "workspaces", "archive-only.yaml"), validYaml.replace( + "id: psd-clinical", "id: archive-only", + )); + await git(remote.source, ["add", "workspaces/archive-only.yaml"]); + await git(remote.source, ["commit", "-m", "Add retained workspace"]); + await git(remote.source, ["push", "origin", "main"]); + await registry.pull(); + + rmSync(join(remote.source, "workspaces", "psd-clinical.yaml")); + await git(remote.source, ["add", "-u"]); + await git(remote.source, ["commit", "-m", "Remove original workspace"]); + await git(remote.source, ["push", "origin", "main"]); + await registry.pull(); + + const retained = await registry.listRetainedSnapshots(); + expect(retained).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: "psd-clinical", commit: remote.initialCommit, state: "operational" }), + expect.objectContaining({ id: "archive-only", state: "operational" }), + ])); +}); + test("does not bypass an existing live advisory repository lock", async () => { const remote = await fixture(); const root = join(remote.root, "registry");