fix: retain snapshots for removed workspaces

This commit is contained in:
2026-08-04 08:52:11 +02:00
parent 8b046f9fb2
commit 3b23cf3714
4 changed files with 116 additions and 3 deletions
+16 -3
View File
@@ -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<WorkspaceRegistry>;
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<ReturnType<typeof d.workspaceRegistry.list>>;
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<SessionRow[]>));
const list = lists.flat();
const sessions = new Map<string, SessionRow>();
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<WorkspaceRegistry>).reconcileSnapshotRetention;
+38
View File
@@ -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<WorkspaceRevision[]> {
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<ActiveState> {
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<void> {
const directory = join(this.repository.snapshotsPath, state.head);
try {
+35
View File
@@ -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);
+27
View File
@@ -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");