mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

feat: pin sessions to workspace revisions

Browse Source
This commit is contained in:
marcopan
2026-08-04 05:13:39 +02:00
parent 301db4bd85
commit bc39730b58
3 changed files with 234 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.superpowers/sdd/2026-08-03-git-workspace-registry/task-7-report.md
+23
View File
@@ -26,6 +26,29 @@
completed with 22 passing tests.
- `git diff --check` completed cleanly.
## Review fixes — round 2
- Lifecycle authorization no longer selects the installation-default workspace. The backend now
finds each session by querying every operational registry snapshot with the authenticated
principal, preserving RLS ownership concealment.
- After locating the manifest, durable pinned sessions resolve their retained descriptor before
any lifecycle mutation/reopen. Legacy sessions continue using the locating registry snapshot.
- Session listing aggregates the owner-visible rows from all operational registry snapshots;
detail, response, steer, resume, events, documents, and lifecycle mutations use the same
server-side locator. No route depends on browser-local workspace state.
### Round 2 verification
- RED: the new cross-workspace route integration test created a B session while installation
default A was selected, then demonstrated that `GET /sessions` returned an empty list.
- GREEN: `npx vitest run test/routes-sessions.test.ts test/tht-runner.test.ts test/routes-settings.test.ts && npx tsc --noEmit -p .`
— 101 tests passed with a clean type check. The integration test covers create B, list, detail,
response, and resume through B's pinned descriptor while default A remains configured.
- Full backend suite: 342 tests passed. The remaining 7 tests require binding `127.0.0.1` and
fail in this sandbox with `listen EPERM: operation not permitted`; no application assertion
failed. The focused typecheck above passed.
- `git diff --check` completed cleanly.
## Verification note
The unscoped backend suite was also run. The Task 7 code regressions in `test/tht-runner.test.ts`
backend/src/routes/sessions.ts
+111 -79
View File
@@ -73,22 +73,64 @@ export function sessionRoutes(
const isNotFound = (error: unknown) =>
/not found|non trovata|inesistente|404/i.test(error instanceof Error ? error.message : String(error));
/** RLS makes a foreign session indistinguishable from a missing one. */
const authorize = async (principal: PrincipalContext, id: string, workspace?: string): Promise<any | undefined> => {
type LocatedSession = { manifest: any; workspaceConfigPath: string };
const workspaceRevisionUnavailable = () => Object.assign(
new Error("workspace revision unavailable"), { code: "workspace_revision_unavailable" },
);
const unavailableWorkspaceReply = (reply: any) => reply.code(409).send({
error: WORKSPACE_REVISION_UNAVAILABLE_MESSAGE,
code: "workspace_revision_unavailable",
});
/**
* Find a session by asking every active registry snapshot, never by using the installation
* default. `tht` applies RLS for the supplied principal, so a foreign ID remains a 404.
*/
const locateSession = async (principal: PrincipalContext, id: string): Promise<LocatedSession | undefined> => {
const runner = runnerFor(principal);
// Dependency-injected runners in legacy route tests may model only the mutation under test.
if (typeof runner.sessionShow !== "function") return {
manifest: {}, workspaceConfigPath: (await d.workspaceRegistry.list())[0]?.snapshotPath ?? "",
};
const revisions = await d.workspaceRegistry.list();
for (const revision of revisions) {
if (revision.state !== "operational") continue;
try {
const manifest = await runner.sessionShow(id, revision.snapshotPath);
if (manifest) return { manifest, workspaceConfigPath: revision.snapshotPath };
} catch (error) {
if (isNotFound(error)) continue;
throw error;
}
}
return undefined;
};
/** Read the durable pinned descriptor only after the owner-visible manifest is located. */
const resolveSessionWorkspace = async (located: LocatedSession): Promise<LocatedSession> => {
const saved = located.manifest as { workspace_id?: string; workspace_revision?: string };
if (!saved.workspace_id || !saved.workspace_revision) return located;
try {
const runner = runnerFor(principal);
// Dependency-injected runners in legacy route tests may model only the mutation under
// test. Production ThtRunner always exposes sessionShow; keep that test seam harmless.
if (typeof runner.sessionShow !== "function") return {};
const manifest = await runner.sessionShow(id, workspace);
return manifest ?? undefined;
} catch (error) {
if (isNotFound(error)) return undefined;
throw error;
const pinned = await d.workspaceRegistry.readPinned(saved.workspace_id, saved.workspace_revision);
return { ...located, workspaceConfigPath: pinned.workspaceConfigPath ?? (pinned as any).revision?.snapshotPath };
} catch {
throw workspaceRevisionUnavailable();
}
};
/** RLS makes a foreign session indistinguishable from a missing one. */
const authorize = async (principal: PrincipalContext, id: string): Promise<LocatedSession | undefined> => {
const located = await locateSession(principal, id);
return located && await resolveSessionWorkspace(located);
};
const storageFailure = (reply: any) => reply.code(503).send({ error: "session storage is unavailable" });
const lifecycleFailure = (reply: any, error: unknown) =>
(error as { code?: string } | undefined)?.code === "workspace_revision_unavailable"
? unavailableWorkspaceReply(reply)
: storageFailure(reply);
const releaseIfFinalized = async (
id: string, rt: ReturnType<PiProcessManager["createFor"]>,
@@ -323,10 +365,14 @@ export function sessionRoutes(
if (scope !== "mine" && scope !== "all") return reply.code(400).send({ error: "scope must be mine or all" });
if (scope === "all" && !principal.isAdmin) return reply.code(403).send({ error: "admin scope required" });
try {
const settings = await d.getSettings(principal);
// Admin RLS is deliberately disabled for a normal 'mine' listing.
const scopedPrincipal = scope === "mine" ? { ...principal, isAdmin: false } : principal;
const list: SessionRow[] = await runnerFor(scopedPrincipal).sessionList(settings.workspace);
const runner = runnerFor(scopedPrincipal);
const revisions = await d.workspaceRegistry.list();
const lists = await Promise.all(revisions
.filter((revision) => revision.state === "operational")
.map((revision) => runner.sessionList(revision.snapshotPath) as Promise<SessionRow[]>));
const list = lists.flat();
// Annotate each row with whether a live Pi runtime is currently bound. The client
// opens an `active` session straight into its live view (reconnecting to its pending
// gate), while a cold session keeps its explicit Resume affordance — so a mere click
@@ -337,21 +383,20 @@ export function sessionRoutes(
app.get("/sessions/:id", async (req, reply) => {
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
const manifest = await authorize(principal, (req.params as any).id, settings.workspace);
if (!manifest) return reply.code(404).send({ error: "session not found" });
const session = await authorize(principal, (req.params as any).id);
if (!session) return reply.code(404).send({ error: "session not found" });
const manifest = session.manifest;
return (!manifest.workspace_id || !manifest.workspace_revision)
? { ...manifest, warning: "Legacy session: this session is not pinned to a workspace revision." }
: manifest;
} catch { return storageFailure(reply); }
} catch (error) { return lifecycleFailure(reply, error); }
});
app.post("/sessions/:id/response", async (req, reply) => {
const id = (req.params as any).id;
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
} catch { return storageFailure(reply); }
if (!await authorize(principal, id)) return reply.code(404).send({ error: "session not found" });
} catch (error) { return lifecycleFailure(reply, error); }
const rt = d.mgr.get(id);
if (!rt) return reply.code(404).send({ error: "sessione non attiva" });
if (!rt.bridge.respond((req.body as any).ui_response)) {
@@ -363,9 +408,8 @@ export function sessionRoutes(
const id = (req.params as any).id;
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
} catch { return storageFailure(reply); }
if (!await authorize(principal, id)) return reply.code(404).send({ error: "session not found" });
} catch (error) { return lifecycleFailure(reply, error); }
const rt = d.mgr.get(id);
if (!rt) return reply.code(404).send({ error: "sessione non attiva" });
rt.bridge.steer((req.body as any).text);
@@ -376,12 +420,12 @@ export function sessionRoutes(
const principal = getPrincipal(req);
return withSessionLifecycle(id, async () => {
let settings: Settings;
let manifest: any;
let located: LocatedSession | undefined;
try {
settings = await d.getSettings(principal);
manifest = await authorize(principal, id, settings.workspace);
located = await locateSession(principal, id);
} catch { return storageFailure(reply); }
if (!manifest) return reply.code(404).send({ error: "session not found" });
if (!located) return reply.code(404).send({ error: "session not found" });
const manifest = located.manifest;
const runner = runnerFor(principal);
// Read-only contract FIRST: finalized or archived sessions never attempt compatibility
// resolution, even when their historical snapshot was subsequently pruned.
@@ -392,21 +436,10 @@ export function sessionRoutes(
provider?: string; model?: string; thinking?: string;
workspace_id?: string; workspace_revision?: string;
};
let workspaceConfigPath = settings.workspace;
const warning = !saved.workspace_id || !saved.workspace_revision
? "Legacy session: this session is not pinned to a workspace revision."
: undefined;
if (!warning) {
try {
const pinned = await d.workspaceRegistry.readPinned(saved.workspace_id!, saved.workspace_revision!);
workspaceConfigPath = pinned.workspaceConfigPath ?? (pinned as any).revision?.snapshotPath;
} catch {
return reply.code(409).send({
error: WORKSPACE_REVISION_UNAVAILABLE_MESSAGE,
code: "workspace_revision_unavailable",
});
}
}
let workspaceConfigPath: string;
try { workspaceConfigPath = (await resolveSessionWorkspace(located)).workspaceConfigPath; }
catch { return unavailableWorkspaceReply(reply); }
try { settings = await d.getSettings(principal); } catch { return storageFailure(reply); }
// This check belongs inside the per-session lock: a preceding cold Resume may have
// installed a running runtime while this request was waiting.
const existing = d.mgr.get(id);
@@ -479,20 +512,20 @@ export function sessionRoutes(
const id = (req.params as { id: string }).id;
const principal = getPrincipal(req);
return withSessionLifecycle(id, async () => {
let settings: Settings;
let session: LocatedSession | undefined;
try {
settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
} catch { return storageFailure(reply); }
session = await authorize(principal, id);
if (!session) return reply.code(404).send({ error: "session not found" });
} catch (error) { return lifecycleFailure(reply, error); }
// Invalidate the live generation before persistence can yield. Otherwise its deferred
// bootstrap may start Pi while Close is already in progress.
const current = d.mgr.get(id);
boundRuntimes.delete(id);
if (current) d.mgr.teardownIfCurrent(id, current);
try {
await runnerFor(principal).closeSession(id, settings.workspace);
} catch {
return storageFailure(reply);
await runnerFor(principal).closeSession(id, session.workspaceConfigPath);
} catch (error) {
return lifecycleFailure(reply, error);
} finally {
// clear, NOT forget: a closed session can be reopened, and the per-session seq
// monotonicity is what keeps a browser's old cursor detectable. The buffer is
@@ -506,9 +539,8 @@ export function sessionRoutes(
const id = (req.params as any).id;
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
} catch { return storageFailure(reply); }
if (!await authorize(principal, id)) return reply.code(404).send({ error: "session not found" });
} catch (error) { return lifecycleFailure(reply, error); }
const rt = d.mgr.get(id);
// Add CORS headers manually: reply.raw.writeHead bypasses Fastify's onSend hooks
// (where @fastify/cors injects headers), so we must set them explicitly here.
@@ -543,58 +575,58 @@ export function sessionRoutes(
const id = (req.params as any).id;
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).setName(id, (req.body as any).name, settings.workspace);
} catch { return storageFailure(reply); }
const session = await authorize(principal, id);
if (!session) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).setName(id, (req.body as any).name, session.workspaceConfigPath);
} catch (error) { return lifecycleFailure(reply, error); }
return reply.code(204).send();
});
app.post("/sessions/:id/group", async (req, reply) => {
const id = (req.params as any).id;
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).setGroup(id, (req.body as any).group, settings.workspace);
} catch { return storageFailure(reply); }
const session = await authorize(principal, id);
if (!session) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).setGroup(id, (req.body as any).group, session.workspaceConfigPath);
} catch (error) { return lifecycleFailure(reply, error); }
return reply.code(204).send();
});
app.post("/sessions/:id/archive", async (req, reply) => {
const id = (req.params as any).id;
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).archive(id, settings.workspace);
} catch { return storageFailure(reply); }
const session = await authorize(principal, id);
if (!session) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).archive(id, session.workspaceConfigPath);
} catch (error) { return lifecycleFailure(reply, error); }
return reply.code(204).send();
});
app.post("/sessions/:id/unarchive", async (req, reply) => {
const id = (req.params as any).id;
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).unarchive(id, settings.workspace);
} catch { return storageFailure(reply); }
const session = await authorize(principal, id);
if (!session) return reply.code(404).send({ error: "session not found" });
await runnerFor(principal).unarchive(id, session.workspaceConfigPath);
} catch (error) { return lifecycleFailure(reply, error); }
return reply.code(204).send();
});
app.delete("/sessions/:id", async (req, reply) => {
const id = (req.params as any).id;
const principal = getPrincipal(req);
return withSessionLifecycle(id, async () => {
let settings: Settings;
let session: LocatedSession | undefined;
try {
settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
} catch { return storageFailure(reply); }
session = await authorize(principal, id);
if (!session) return reply.code(404).send({ error: "session not found" });
} catch (error) { return lifecycleFailure(reply, error); }
const current = d.mgr.get(id);
boundRuntimes.delete(id);
if (current) d.mgr.teardownIfCurrent(id, current);
try {
await runnerFor(principal).deleteSession(id, settings.workspace);
} catch {
return storageFailure(reply);
await runnerFor(principal).deleteSession(id, session.workspaceConfigPath);
} catch (error) {
return lifecycleFailure(reply, error);
}
d.hub.forget(id);
return reply.code(204).send();
@@ -604,9 +636,9 @@ export function sessionRoutes(
const id = (req.params as any).id;
const principal = getPrincipal(req);
try {
const settings = await d.getSettings(principal);
if (!await authorize(principal, id, settings.workspace)) return reply.code(404).send({ error: "session not found" });
return await runnerFor(principal).documents(id, settings.workspace);
} catch { return storageFailure(reply); }
const session = await authorize(principal, id);
if (!session) return reply.code(404).send({ error: "session not found" });
return await runnerFor(principal).documents(id, session.workspaceConfigPath);
} catch (error) { return lifecycleFailure(reply, error); }
});
}
backend/test/routes-sessions.test.ts
+100 -4
View File
@@ -11,6 +11,10 @@ const FAKE = path.resolve("../harness/tests/fake_pi/fake_pi_rpc.mjs");
const SCRIPT = path.resolve("../harness/tests/fake_pi/scripts/f1_disambiguation.json");
const defaultWorkspaceRegistry = {
list: vi.fn(async () => [{
id: "default", commit: "e".repeat(40), blob: "f".repeat(40),
snapshotPath: `/data/workspace-registry/snapshots/${"e".repeat(40)}/default.yaml`, state: "operational",
}]),
read: vi.fn(async (id: string) => ({
workspace: {
llm_policy: {
@@ -25,7 +29,10 @@ const defaultWorkspaceRegistry = {
};
function buildApp(config: Parameters<typeof buildRealApp>[0], deps: Record<string, unknown> = {}) {
return buildRealApp(config, { workspaceRegistry: defaultWorkspaceRegistry as any, ...deps } as any);
return buildRealApp(config, {
...deps,
workspaceRegistry: { ...defaultWorkspaceRegistry, ...(deps.workspaceRegistry as object | undefined) },
} as any);
}
function mutApp(thtRunner: any) {
@@ -233,6 +240,90 @@ test("creates a session from the configured default workspace revision when work
}));
});
test("session lifecycle locates a B session when installation default is A", async () => {
const aPath = "/registry/snapshots/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/a-workspace.yaml";
const bPath = "/registry/snapshots/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb/b-workspace.yaml";
const bPinnedPath = "/registry/snapshots/cccccccccccccccccccccccccccccccccccccccc/b-workspace.yaml";
const bManifest = {
id: "session-b", status: "open", archived: false,
workspace_id: "b-workspace", workspace_revision: "c".repeat(40),
provider: "zai", model: "glm-5.2", thinking: "low",
};
const calls: string[] = [];
let active: any;
const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), {
thtRunner: {
sessionNew: async (input: any) => {
calls.push(`new:${input.workspaceConfigPath}`);
return { id: "session-b" };
},
searchPack: async () => {},
sessionList: async (workspace: string) => {
calls.push(`list:${workspace}`);
return workspace === bPath ? [{ id: "session-b", status: "open", question: "B question" }] : [];
},
sessionShow: async (id: string, workspace: string) => {
calls.push(`show:${workspace}`);
if (id === "session-b" && workspace === bPath) return bManifest;
throw new Error("session not found");
},
reopenSession: async (id: string, workspace: string) => {
calls.push(`reopen:${workspace}`);
expect(id).toBe("session-b");
},
} as any,
readiness: { ensure: async () => ({ ok: true }) } as any,
mgr: {
get: () => active,
createFor: () => {
active = { bridge: { onClientEvent: () => {}, respond: () => true, turnState: () => "idle" } };
return active;
},
configure: async () => {}, start: () => {},
teardownForPrincipal: () => [],
teardownIfCurrent: (_id: string, expected: any) => {
if (active !== expected) return false;
active = undefined;
return true;
},
} as any,
getSettings: () => ({ workspace: "a-workspace", provider: "zai", model: "glm-5.2", thinking: "low" }) as any,
listModels: async () => [{ provider: "zai", id: "glm-5.2", name: "GLM 5.2", reasoning: true }],
workspaceRegistry: {
read: async (id: string) => ({
workspace: { llm_policy: { allowed: ["zai/glm-5.2"] } },
revision: { id, commit: "b".repeat(40), blob: "d".repeat(40), snapshotPath: bPath, state: "operational" },
}),
list: async () => [
{ id: "a-workspace", commit: "a".repeat(40), blob: "a".repeat(40), snapshotPath: aPath, state: "operational" },
{ id: "b-workspace", commit: "b".repeat(40), blob: "b".repeat(40), snapshotPath: bPath, state: "operational" },
],
readPinned: vi.fn(async (id: string, revision: string) => {
expect([id, revision]).toEqual(["b-workspace", "c".repeat(40)]);
return { workspace: { llm_policy: { allowed: ["zai/glm-5.2"] } }, workspaceConfigPath: bPinnedPath };
}),
} as any,
});
expect((await app.inject({ method: "POST", url: "/sessions", payload: {
question: "B question", workspaceId: "b-workspace", provider: "zai", model: "glm-5.2", thinking: "low",
} })).statusCode).toBe(200);
expect((await app.inject({ method: "GET", url: "/sessions" })).json()).toEqual([
expect.objectContaining({ id: "session-b", active: true }),
]);
expect((await app.inject({ method: "GET", url: "/sessions/session-b" })).json()).toMatchObject(bManifest);
expect((await app.inject({ method: "POST", url: "/sessions/session-b/response", payload: { ui_response: {} } })).statusCode)
.toBe(204);
active = undefined;
expect((await app.inject({ method: "POST", url: "/sessions/session-b/resume" })).json())
.toEqual({ id: "session-b", alreadyActive: false });
expect(calls).toContain(`new:${bPath}`);
expect(calls).toContain(`list:${bPath}`);
expect(calls).toContain(`show:${bPath}`);
expect(calls).toContain(`reopen:${bPinnedPath}`);
});
test("POST /sessions usa i settings (workspace/provider/model/thinking) e crea+avvia", async () => {
const modelKey = path.join(os.tmpdir(), `thoth-model-key-${process.pid}`);
writeFileSync(modelKey, "test-model-key", { mode: 0o600 });
@@ -1707,8 +1798,9 @@ test("POST /sessions/:id/rename calls setName", async () => {
expect(arg).toEqual({ id: "s1", name: "N" });
});
test("rename authorizes and mutates through the same selected workspace", async () => {
test("rename authorizes and mutates through the same registry snapshot", async () => {
const workspaces: string[] = [];
const tenantPath = "/registry/snapshots/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/tenant-a.yaml";
const app = buildApp(loadConfig({ AUTH_MODE: "upstream", THT_HARNESS_DIR: "../harness" }), {
thtRunner: {
withPrincipal: () => ({
@@ -1716,7 +1808,11 @@ test("rename authorizes and mutates through the same selected workspace", async
setName: async (_id: string, _name: string, workspace: string) => { workspaces.push(`set:${workspace}`); },
}),
} as any,
getSettings: () => ({ workspace: "tenant-a" }) as any,
workspaceRegistry: {
list: async () => [{
id: "tenant-a", commit: "a".repeat(40), blob: "b".repeat(40), snapshotPath: tenantPath, state: "operational",
}],
} as any,
});
const res = await app.inject({
method: "POST", url: "/sessions/s1/rename", payload: { name: "N" },
@@ -1725,7 +1821,7 @@ test("rename authorizes and mutates through the same selected workspace", async
},
});
expect(res.statusCode).toBe(204);
expect(workspaces).toEqual(["show:tenant-a", "set:tenant-a"]);
expect(workspaces).toEqual([`show:${tenantPath}`, `set:${tenantPath}`]);
});
test("POST /sessions/:id/group calls setGroup", async () => {