From 90894176b63800ba45fd0e2dbadf5d38ecbf2da6 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 4 Aug 2026 04:52:06 +0200 Subject: [PATCH] feat: pin sessions to workspace revisions --- .../task-7-report.md | 34 ++++++ backend/src/app.ts | 26 +--- backend/src/routes/sessions.ts | 101 ++++++++++++---- backend/src/routes/settings.ts | 22 ++-- backend/src/settings/settings-store.ts | 5 + backend/src/tht/tht-runner.ts | 29 ++++- backend/src/workspaces/registry.ts | 11 ++ backend/test/routes-sessions.test.ts | 114 +++++++++++++++++- backend/test/routes-settings.test.ts | 23 ++-- harness/tests/test_session_documents.py | 11 +- harness/tht/cli/session_cmd.py | 3 + harness/tht/session/models.py | 2 + harness/tht/session/store.py | 12 +- 13 files changed, 313 insertions(+), 80 deletions(-) create mode 100644 .superpowers/sdd/2026-08-03-git-workspace-registry/task-7-report.md diff --git a/.superpowers/sdd/2026-08-03-git-workspace-registry/task-7-report.md b/.superpowers/sdd/2026-08-03-git-workspace-registry/task-7-report.md new file mode 100644 index 00000000..740b469c --- /dev/null +++ b/.superpowers/sdd/2026-08-03-git-workspace-registry/task-7-report.md @@ -0,0 +1,34 @@ +# Task 7 report — revision-pinned sessions + +## Delivered + +- New-session requests may carry `workspaceId`, provider, model, and thinking. The backend + resolves the active operational registry revision, enforces its LLM policy, and persists the + workspace ID/revision with the selected LLM settings. +- The harness manifest and `tht session new` support the optional, backward-compatible + `workspace_id` and `workspace_revision` fields. +- Resume resolves the manifest's retained snapshot, including after later registry publication. + A missing retained revision returns a sanitized `workspace_revision_unavailable` response. + Legacy manifests retain the prior workspace behavior and are marked with a visible warning on + `GET /sessions/:id`. +- `/settings` is now a non-mutating compatibility endpoint: installation defaults remain + readable, while anonymous workspace/provider/model/thinking selections are no longer written + to backend settings or principal preferences. + +## TDD evidence + +- RED: `npx vitest run test/routes-sessions.test.ts test/routes-settings.test.ts` failed for the + new immutable-snapshot and no-settings-mutation assertions; the manifest test failed because + `new_session_manifest` did not accept workspace revision fields. +- GREEN: `npx vitest run test/tht-runner.test.ts test/routes-sessions.test.ts test/routes-settings.test.ts && npx tsc --noEmit -p .` + completed with 97 passing tests and a clean type check. +- GREEN: `THT_HOME=/private/tmp/thothii-task7-home .venv/bin/pytest tests/test_session_documents.py tests/test_session_mutations.py -q` + completed with 22 passing tests. +- `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` +were fixed; the remaining failures were existing sandbox restrictions on tests that listen on +`127.0.0.1` (`listen EPERM: operation not permitted` in SSE/e2e health tests), not application +assertions. diff --git a/backend/src/app.ts b/backend/src/app.ts index 110ceac1..b02d5126 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -13,7 +13,7 @@ import { sqlRoutes } from "./routes/sql.js"; import { metaRoutes, type ListModelsFn } from "./routes/meta.js"; import { settingsRoutes, effectiveSettings } from "./routes/settings.js"; import { createPiModelLister } from "./pi/list-models.js"; -import { loadSettings, saveSettings, type Settings } from "./settings/settings-store.js"; +import { loadSettings, type Settings } from "./settings/settings-store.js"; import { ReadinessManager } from "./runtime/readiness-manager.js"; import { WorkspaceRegistry } from "./workspaces/registry.js"; import { createProductionWorkspaceDiagnoser } from "./workspaces/diagnostics.js"; @@ -72,28 +72,8 @@ export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstanc }; const getSettings = async (principal: PrincipalContext): Promise => { if (deps?.getSettings) return await deps.getSettings(principal); - const runner = runnerFor(principal); - // The real runner persists preferences through the harness repository. The file fallback - // only keeps older isolated route tests and externally injected runners compatible. - if (typeof runner.preferencesGet === "function") { - const stored = await runner.preferencesGet() as Settings; - if (Object.keys(stored).length === 0) { - const seeded = effectiveSettings(config, loadSettings(config)); - await runner.preferencesSet(seeded); - return seeded; - } - return effectiveSettings(config, stored); - } return effectiveSettings(config, loadSettings(config)); }; - const saveUserSettings = async (principal: PrincipalContext, settings: Settings): Promise => { - const runner = runnerFor(principal); - if (typeof runner.preferencesSet === "function") { - await runner.preferencesSet(settings); - return; - } - saveSettings(config, settings); - }; const authenticate = authPreHandler(config.authMode); app.addHook("preHandler", async (req, reply) => { @@ -105,13 +85,13 @@ export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstanc app.get("/health/dwh", async () => tht.dbPing()); app.get("/me", async (req) => getPrincipal(req)); sessionRoutes(app, { - mgr, tht: tht as ThtRunner, hub, getSettings, readiness, listModels, + mgr, tht: tht as ThtRunner, hub, getSettings, readiness, listModels, workspaceRegistry, dwhPrecheck: config.dwhPrecheck, }); sqlRoutes(app, { tht: tht as ThtRunner, getSettings }); metaRoutes(app, { harnessDir: config.harnessDir, listModels }); workspaceRoutes(app, { registry: workspaceRegistry, config: config.workspaceRegistry, diagnose: workspaceDiagnoser }); - settingsRoutes(app, { cfg: config, listModels, getSettings, saveSettings: saveUserSettings }); + settingsRoutes(app, { cfg: config, listModels, getSettings }); return app; } diff --git a/backend/src/routes/sessions.ts b/backend/src/routes/sessions.ts index f7834422..1783f16b 100644 --- a/backend/src/routes/sessions.ts +++ b/backend/src/routes/sessions.ts @@ -7,6 +7,7 @@ import { getPrincipal } from "../auth/auth.js"; import type { PrincipalContext } from "../auth/principal.js"; import type { ReadinessManager } from "../runtime/readiness-manager.js"; import type { ListModelsFn } from "./meta.js"; +import type { WorkspaceRegistry } from "../workspaces/registry.js"; const BOOTSTRAP_FAILURE_MESSAGE = "Session startup failed. Check configuration and connectivity, then Resume the session."; @@ -18,6 +19,8 @@ const DWH_UNREACHABLE_MESSAGE = "Cannot start a session: the database is unreachable. Check the VPN connection and try again."; const MODEL_UNAVAILABLE_MESSAGE = "Selected model is unavailable. Check Pi authentication and model settings, then try again."; +const WORKSPACE_REVISION_UNAVAILABLE_MESSAGE = + "Session workspace configuration is unavailable. Check configuration and try again."; export function sessionRoutes( app: FastifyInstance, @@ -26,6 +29,7 @@ export function sessionRoutes( getSettings: (principal: PrincipalContext) => Promise; readiness: ReadinessManager; listModels: ListModelsFn; + workspaceRegistry: WorkspaceRegistry; /** Local-only guard: probe DWH reachability before creating a session (run-stack.sh). */ dwhPrecheck?: boolean; }, @@ -195,28 +199,61 @@ export function sessionRoutes( }); app.post("/sessions", async (req, reply) => { - const b = req.body as { question: string; name?: string }; + const b = req.body as { + question: string; name?: string; workspaceId?: string; + provider?: string; model?: string; thinking?: string; + }; const principal = getPrincipal(req); let s: Settings; try { s = await d.getSettings(principal); } catch { return storageFailure(reply); } const runner = runnerFor(principal); + let workspaceConfigPath = s.workspace; + let workspaceId: string | undefined; + let workspaceRevision: string | undefined; + let allowedModels: readonly string[] | undefined; + if (b.workspaceId) { + try { + const resolved = await d.workspaceRegistry.read(b.workspaceId); + if (resolved.revision.state !== "operational") { + return reply.code(409).send({ + error: WORKSPACE_REVISION_UNAVAILABLE_MESSAGE, + code: "workspace_revision_unavailable", + }); + } + workspaceConfigPath = resolved.revision.snapshotPath; + workspaceId = resolved.revision.id; + workspaceRevision = resolved.revision.commit; + allowedModels = resolved.workspace.llm_policy.allowed; + } catch { + return reply.code(409).send({ + error: WORKSPACE_REVISION_UNAVAILABLE_MESSAGE, + code: "workspace_revision_unavailable", + }); + } + } + const provider = b.provider ?? s.provider; + const model = b.model ?? s.model; + const thinking = b.thinking ?? s.thinking; + if (allowedModels && provider && model && !allowedModels.includes(`${provider}/${model}`)) { + return reply.code(400).send({ error: "Selected model is not allowed by this workspace." }); + } // A persisted session is resumable without keeping Pi alive. New work replaces every // runtime owned by this principal, while runtimes belonging to other users remain intact. // Optional chaining preserves the deliberately narrow manager stubs used by route tests. for (const id of d.mgr.teardownForPrincipal?.(principal) ?? []) boundRuntimes.delete(id); - const ensure = await d.readiness.ensure(s.workspace ?? "", principal); + const ensure = await d.readiness.ensure(workspaceConfigPath ?? "", principal); if (!ensure.ok) return reply.code(503).send({ error: READINESS_FAILURE_MESSAGE }); // Local-only: verify the DWH is reachable BEFORE creating the session, so a dropped // VPN surfaces as an up-front alert instead of a session that spawns Pi and then dies // in bootstrap retrieval. `code` lets the client show a specific message. if (d.dwhPrecheck) { - const ping = await runner.dbPing(s.workspace); + const ping = await runner.dbPing(workspaceConfigPath); if (!ping.ok) { console.error(`[dwh-precheck] refusing new session — DWH unreachable: ${ping.detail}`); return reply.code(503).send({ error: DWH_UNREACHABLE_MESSAGE, code: "dwh_unreachable" }); } } - if (s.provider && s.model) { + if (provider && model) { let available: Awaited>; try { available = await d.listModels(); @@ -227,7 +264,7 @@ export function sessionRoutes( }); } const selectedAvailable = available.some( - (candidate) => candidate.provider === s.provider && candidate.id === s.model, + (candidate) => candidate.provider === provider && candidate.id === model, ); if (!selectedAvailable) { return reply.code(503).send({ @@ -236,19 +273,18 @@ export function sessionRoutes( }); } } - // Settings (global) supply workspace/provider/model/thinking. The new-question - // form sends only the question text. `workspace` selects the tht `-c `. + // Browser choices are copied to the persisted manifest together with the immutable + // registry snapshot. The legacy fallback stays available for sessions created before + // the browser-local preference migration. let id: string; try { ({ id } = await runner.sessionNew({ - question: b.question, name: b.name, workspace: s.workspace, - provider: s.provider, model: s.model, thinking: s.thinking, + question: b.question, name: b.name, workspaceConfigPath, + workspaceId, workspaceRevision, provider, model, thinking, })); } catch { return storageFailure(reply); } const options = { - provider: s.provider, - model: s.model, - thinking: s.thinking, + provider, model, thinking, author: principal.displayName ?? principal.subject, principal, question: b.question, @@ -256,22 +292,22 @@ export function sessionRoutes( let rt: ReturnType | undefined; try { rt = d.mgr.createFor(id, options); - bindRuntime(id, rt, runner, s.workspace); + bindRuntime(id, rt, runner, workspaceConfigPath); } catch (error) { if (rt) d.mgr.teardownIfCurrent(id, rt); console.error( `[pi:${id}] runtime construction failed:`, error instanceof Error ? error.message : "unknown error", ); - await runner.failSession(id, s.workspace).catch((persistenceError: unknown) => { + await runner.failSession(id, workspaceConfigPath).catch((persistenceError: unknown) => { console.error(`[session:${id}] failSession persistence failed:`, persistenceError); }); return reply.code(503).send({ error: BOOTSTRAP_FAILURE_MESSAGE }); } info(id, "Session created"); bootstrap( - id, rt, runner, s.workspace, d.mgr.configure(rt, options), - runner.searchPack(b.question, id, s.workspace), + id, rt, runner, workspaceConfigPath, d.mgr.configure(rt, options), + runner.searchPack(b.question, id, workspaceConfigPath), () => d.mgr.start(id, rt, options), ); return { id }; @@ -298,7 +334,10 @@ export function sessionRoutes( try { const settings = await d.getSettings(principal); const manifest = await authorize(principal, (req.params as any).id, settings.workspace); - return manifest ?? reply.code(404).send({ error: "session not found" }); + if (!manifest) return reply.code(404).send({ error: "session not found" }); + 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); } }); app.post("/sessions/:id/response", async (req, reply) => { @@ -339,6 +378,25 @@ export function sessionRoutes( } catch { return storageFailure(reply); } if (!manifest) return reply.code(404).send({ error: "session not found" }); const runner = runnerFor(principal); + const saved = manifest as { + 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", + }); + } + } // Read-only contract FIRST: a finalized/archived session must refuse resume even // when a lingering runtime still looks active — the manifest is the truth. if (manifest?.status === "finalized" || manifest?.archived) { @@ -353,9 +411,8 @@ export function sessionRoutes( return reply.code(200).send({ id, alreadyActive: true }); } } - const ensure = await d.readiness.ensure(settings.workspace ?? "", principal); + const ensure = await d.readiness.ensure(workspaceConfigPath ?? "", principal); if (!ensure.ok) return reply.code(503).send({ error: READINESS_FAILURE_MESSAGE }); - const saved = manifest as { provider?: string; model?: string; thinking?: string } | null; const options = { provider: saved?.provider, model: saved?.model, @@ -368,7 +425,7 @@ export function sessionRoutes( // Reopening is validation, not the transport commit point. Keep the old hub intact if // persistence cannot be reopened. try { - await runner.reopenSession(id, settings.workspace); + await runner.reopenSession(id, workspaceConfigPath); } catch { return reply.code(503).send({ error: RESUME_FAILURE_MESSAGE }); } @@ -394,7 +451,7 @@ export function sessionRoutes( d.mgr.teardownIfCurrent(id, current); } rt = d.mgr.createFor(id, options); - bindRuntime(id, rt, runner, settings.workspace); + bindRuntime(id, rt, runner, workspaceConfigPath); } catch { // A created-but-unbound runtime is not usable. The old hub remains attached because // clear() has not happened yet. @@ -409,7 +466,7 @@ export function sessionRoutes( // immediately before the first event produced by the new Resume. d.hub.clear(id); info(id, "Resuming session"); - bootstrap(id, rt, runner, settings.workspace, d.mgr.configure(rt, options), null, () => d.mgr.start(id, rt, options)); + bootstrap(id, rt, runner, workspaceConfigPath, d.mgr.configure(rt, options), null, () => d.mgr.start(id, rt, options)); return reply.code(200).send({ id, alreadyActive: false }); }); }); diff --git a/backend/src/routes/settings.ts b/backend/src/routes/settings.ts index d6a542ca..31607ad4 100644 --- a/backend/src/routes/settings.ts +++ b/backend/src/routes/settings.ts @@ -1,6 +1,6 @@ import type { FastifyInstance } from "fastify"; import type { AppConfig } from "../config.js"; -import { loadSettings, saveSettings, type Settings } from "../settings/settings-store.js"; +import type { Settings } from "../settings/settings-store.js"; import { listWorkspaces, type ListModelsFn } from "./meta.js"; import { getPrincipal } from "../auth/auth.js"; import type { PrincipalContext } from "../auth/principal.js"; @@ -9,10 +9,10 @@ import type { PrincipalContext } from "../auth/principal.js"; export function effectiveSettings(cfg: AppConfig, stored: Settings): Settings { const workspaces = listWorkspaces(cfg.harnessDir); return { - workspace: stored.workspace ?? (workspaces[0]?.name), - provider: stored.provider ?? cfg.defaults.provider, - model: stored.model ?? cfg.defaults.model, - thinking: stored.thinking ?? cfg.defaults.thinking, + workspace: workspaces[0]?.name ?? stored.workspace, + provider: cfg.defaults.provider ?? stored.provider, + model: cfg.defaults.model ?? stored.model, + thinking: cfg.defaults.thinking ?? stored.thinking, }; } @@ -21,7 +21,6 @@ export function settingsRoutes( deps: { cfg: AppConfig; listModels: ListModelsFn; getSettings: (principal: PrincipalContext) => Promise; - saveSettings: (principal: PrincipalContext, settings: Settings) => Promise; }, ): void { app.get("/settings", async (req, reply) => { @@ -50,15 +49,10 @@ export function settingsRoutes( }); } } - const next: Settings = { - workspace: b.workspace, - provider: b.provider, - model: b.model, - thinking: b.thinking, - }; try { - await deps.saveSettings(getPrincipal(req), next); - return effectiveSettings(deps.cfg, next); + // Retain this endpoint as a validating compatibility surface for older clients, but do + // not write anonymous users' choices to shared server storage. + return await deps.getSettings(getPrincipal(req)); } catch { return reply.code(503).send({ error: "settings storage is unavailable" }); } diff --git a/backend/src/settings/settings-store.ts b/backend/src/settings/settings-store.ts index 7783a40f..e09d417e 100644 --- a/backend/src/settings/settings-store.ts +++ b/backend/src/settings/settings-store.ts @@ -9,6 +9,11 @@ export interface Settings { thinking?: string; } +/** + * Settings files are installation defaults only. Personal workspace/model/thinking choices + * belong to the browser and must never be written back here by request handlers. + */ + /** Read settings from cfg.settingsFile. Returns {} if missing or invalid. */ export function loadSettings(cfg: AppConfig): Settings { try { diff --git a/backend/src/tht/tht-runner.ts b/backend/src/tht/tht-runner.ts index ed112a5c..49909c5e 100644 --- a/backend/src/tht/tht-runner.ts +++ b/backend/src/tht/tht-runner.ts @@ -4,7 +4,7 @@ import { closeSync, constants as fsConstants, existsSync, fchmodSync, fstatSync, fsyncSync, lstatSync, mkdirSync, openSync, readFileSync, readSync, realpathSync, statSync, unlinkSync, writeFileSync, } from "node:fs"; -import { isAbsolute, join } from "node:path"; +import { dirname, isAbsolute, join, relative } from "node:path"; import { clearPrincipalEnvironment, principalEnvironment, type PrincipalContext } from "../auth/principal.js"; import { secretValue, type SecretBundleConfig } from "../config/secret-bundle.js"; @@ -68,7 +68,8 @@ export class ThtRunner { private configArg(workspaceConfigPath?: string): string[] { if (workspaceConfigPath) { if (isAbsolute(workspaceConfigPath)) { - this.assertTrustedRuntimeSnapshot(workspaceConfigPath); + if (this.runtimeSnapshots.has(workspaceConfigPath)) this.assertTrustedRuntimeSnapshot(workspaceConfigPath); + else this.assertWorkspaceSnapshot(workspaceConfigPath); return ["-c", workspaceConfigPath]; } if (workspaceConfigPath.includes("/")) { @@ -83,6 +84,20 @@ export class ThtRunner { return ["-c", this.cfg.configPath]; } + private assertWorkspaceSnapshot(path: string): void { + if (!this.cfg.runtimeSnapshotRoot) throw new Error("workspace snapshot root is not configured"); + const snapshotsRoot = dirname(this.cfg.runtimeSnapshotRoot); + const pathRelative = relative(snapshotsRoot, path); + if ( + pathRelative.startsWith("..") || isAbsolute(pathRelative) + || !/^[0-9a-f]{40}\/[a-z][a-z0-9-]{2,62}\.yaml$/.test(pathRelative) + ) throw new Error("config path is not a trusted runtime snapshot"); + const entry = lstatSync(path); + if (!entry.isFile() || entry.isSymbolicLink()) { + throw new Error("config path is not a trusted runtime snapshot"); + } + } + private runtimeSnapshotDirectory(): string { if (!this.cfg.runtimeSnapshotRoot) throw new Error("runtime snapshot root is not configured"); if (!isAbsolute(this.cfg.runtimeSnapshotRoot)) throw new Error("runtime snapshot root must be absolute"); @@ -230,7 +245,7 @@ export class ThtRunner { let snapshotFd: number | undefined; let ch; try { - snapshotFd = workspaceConfigPath && isAbsolute(workspaceConfigPath) + snapshotFd = workspaceConfigPath && this.runtimeSnapshots.has(workspaceConfigPath) ? this.openTrustedRuntimeSnapshot(workspaceConfigPath) : undefined; ch = spawn( @@ -287,7 +302,11 @@ export class ThtRunner { model?: string; thinking?: string; name?: string; + /** Legacy named-workspace compatibility; pinned sessions use workspaceConfigPath. */ workspace?: string; + workspaceConfigPath?: string; + workspaceId?: string; + workspaceRevision?: string; }) { const a = ["session", "new", o.question]; for (const [f, v] of [ @@ -295,11 +314,13 @@ export class ThtRunner { ["--model", o.model], ["--thinking", o.thinking], ["--name", o.name], + ["--workspace-id", o.workspaceId], + ["--workspace-revision", o.workspaceRevision], ] as const) { if (v) a.push(f, v); } a.push("--json"); - return this.json<{ id: string }>(a, o.workspace); + return this.json<{ id: string }>(a, o.workspaceConfigPath ?? o.workspace); } /** Build and persist the deterministic F1 retrieval pack for a new session. */ diff --git a/backend/src/workspaces/registry.ts b/backend/src/workspaces/registry.ts index fc7e1c7f..2fb3d7ff 100644 --- a/backend/src/workspaces/registry.ts +++ b/backend/src/workspaces/registry.ts @@ -153,6 +153,17 @@ export class WorkspaceRegistry { } } + /** Read a retained immutable snapshot for a session pinned to a historical commit. */ + async readPinned(id: string, commit: string): Promise<{ workspace: WorkspaceDescriptor; workspaceConfigPath: string }> { + const snapshotPath = this.snapshotPath(safeCommit(commit), id); + try { + const source = await readFile(snapshotPath, "utf8"); + return { workspace: parseWorkspaceYaml(source), workspaceConfigPath: snapshotPath }; + } catch (error) { + throw workspaceError(error); + } + } + /** * Publish canonical YAML and derived public documentation as one optimistic Git revision. * The browser never provides paths or generated artifacts; those are derived server-side. diff --git a/backend/test/routes-sessions.test.ts b/backend/test/routes-sessions.test.ts index 9f019541..4499948a 100644 --- a/backend/test/routes-sessions.test.ts +++ b/backend/test/routes-sessions.test.ts @@ -1,4 +1,4 @@ -import { test, expect } from "vitest"; +import { test, expect, vi } from "vitest"; import { spawn as nodeSpawn } from "node:child_process"; import path from "node:path"; import os from "node:os"; @@ -148,6 +148,44 @@ test("new sessions are created through the authenticated principal, not a client expect(principal).toMatchObject({ issuer: "portal", subject: "alice" }); }); +test("creates a session from the active immutable workspace revision", async () => { + const sessionNew = vi.fn(async () => ({ id: "pinned" })); + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: { sessionNew, searchPack: async () => {} } as any, + readiness: { ensure: async () => ({ ok: true }) } as any, + mgr: { + get: () => undefined, + createFor: () => ({ bridge: { onClientEvent: () => {} } }), + configure: async () => {}, + start: () => {}, + } as any, + getSettings: () => ({ 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: vi.fn(async () => ({ + workspace: { + workspace: { schema_version: 2, id: "psd-clinical", name: "PSD", language: "it" }, + dwh: {}, semantic_index: {}, llm_policy: { allowed: ["zai/glm-5.2"] }, + }, + revision: { + id: "psd-clinical", commit: "a".repeat(40), blob: "b".repeat(40), + snapshotPath: "/data/workspace-registry/snapshots/abc/psd-clinical.yaml", state: "operational", + }, + })), + } as any, + }); + + await app.inject({ + method: "POST", url: "/sessions", + payload: { question: "q", workspaceId: "psd-clinical", provider: "zai", model: "glm-5.2", thinking: "low" }, + }); + + expect(sessionNew).toHaveBeenCalledWith(expect.objectContaining({ + workspaceConfigPath: "/data/workspace-registry/snapshots/abc/psd-clinical.yaml", + workspaceId: "psd-clinical", workspaceRevision: "a".repeat(40), + })); +}); + 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 }); @@ -171,7 +209,7 @@ test("POST /sessions usa i settings (workspace/provider/model/thinking) e crea+a }); const created = await app.inject({ method: "POST", url: "/sessions", payload: { question: "q" } }); expect(created.json()).toEqual({ id: "s1" }); - expect(sessionNewArg.workspace).toBe("w"); + expect(sessionNewArg.workspaceConfigPath).toBe("w"); expect(sessionNewArg.provider).toBe("zai"); expect(sessionNewArg.model).toBe("glm-5.2"); expect(sessionNewArg.thinking).toBe("high"); @@ -373,6 +411,78 @@ test("POST /sessions/:id/resume configura Pi con il thinking persistito", async expect(configured.thinking).toBe("medium"); }); +test("POST /sessions/:id/resume uses the manifest's retained workspace revision", async () => { + const reopenSession = vi.fn(async () => {}); + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + mgr: { + get: () => undefined, + createFor: () => ({ bridge: { onClientEvent: () => {} } }), + configure: async () => {}, start: () => {}, + } as any, + thtRunner: { + sessionShow: async () => ({ + status: "open", archived: false, workspace_id: "psd-clinical", workspace_revision: "a".repeat(40), + }), + reopenSession, + } as any, + readiness: { ensure: async () => ({ ok: true }) } as any, + getSettings: () => ({ workspace: "legacy" }) as any, + workspaceRegistry: { + readPinned: vi.fn(async () => ({ + workspace: { workspace: { id: "psd-clinical" } }, + revision: { + id: "psd-clinical", commit: "a".repeat(40), blob: "b".repeat(40), + snapshotPath: "/data/workspace-registry/snapshots/aaaaaaaa/psd-clinical.yaml", state: "operational", + }, + })), + } as any, + }); + + const response = await app.inject({ method: "POST", url: "/sessions/pinned/resume" }); + + expect(response.statusCode).toBe(200); + expect(reopenSession).toHaveBeenCalledWith( + "pinned", "/data/workspace-registry/snapshots/aaaaaaaa/psd-clinical.yaml", + ); +}); + +test("POST /sessions/:id/resume returns a sanitized error when its retained revision is unavailable", async () => { + const rawFailure = "cannot read /data/workspace-registry/snapshots/secret-revision"; + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: { + sessionShow: async () => ({ + status: "open", archived: false, workspace_id: "psd-clinical", workspace_revision: "a".repeat(40), + }), + } as any, + getSettings: () => ({ workspace: "legacy" }) as any, + workspaceRegistry: { readPinned: async () => { throw new Error(rawFailure); } } as any, + }); + + const response = await app.inject({ method: "POST", url: "/sessions/pinned/resume" }); + + expect(response.statusCode).toBe(409); + expect(response.body).not.toContain(rawFailure); + expect(response.json()).toMatchObject({ code: "workspace_revision_unavailable" }); +}); + +test("GET /sessions/:id warns when a legacy manifest has no workspace revision", async () => { + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + mgr: { + get: () => undefined, + createFor: () => ({ bridge: { onClientEvent: () => {} } }), + configure: async () => {}, start: () => {}, + } as any, + thtRunner: { sessionShow: async () => ({ status: "open", archived: false }), reopenSession: async () => {} } as any, + readiness: { ensure: async () => ({ ok: true }) } as any, + getSettings: () => ({ workspace: "legacy" }) as any, + }); + + const response = await app.inject({ method: "GET", url: "/sessions/legacy" }); + + expect(response.statusCode).toBe(200); + expect(response.json()).toMatchObject({ warning: expect.stringMatching(/legacy/i) }); +}); + test("POST /sessions/:id/resume usa il thinking globale se manca nel manifest", async () => { let configured: any; const bridge = { onClientEvent: () => {}, emitClientEvent: () => {} }; diff --git a/backend/test/routes-settings.test.ts b/backend/test/routes-settings.test.ts index f25a4964..27b8d79e 100644 --- a/backend/test/routes-settings.test.ts +++ b/backend/test/routes-settings.test.ts @@ -31,8 +31,8 @@ test("GET /settings returns effective defaults (env provider/model/thinking, fir } }); -test("PUT /settings persists and GET reads it back", async () => { - const { app, dir } = appWithTmpSettings({}, { +test("PUT /settings does not persist personal workspace or LLM choices", async () => { + const { app, dir } = appWithTmpSettings({ PI_PROVIDER: "zai", PI_MODEL: "glm-5.2", PI_THINKING: "medium" }, { listModels: async () => [{ provider: "zai", id: "glm-5.2", name: "GLM 5.2", reasoning: true }], }); try { @@ -42,13 +42,14 @@ test("PUT /settings persists and GET reads it back", async () => { }); expect(put.statusCode).toBe(200); const got = await app.inject({ method: "GET", url: "/settings" }); - expect(got.json()).toMatchObject({ workspace: "psd", provider: "zai", model: "glm-5.2", thinking: "high" }); + expect(got.json()).toMatchObject({ provider: "zai", model: "glm-5.2", thinking: "medium" }); + expect(got.json()).not.toMatchObject({ workspace: "psd", thinking: "high" }); } finally { rmSync(dir, { recursive: true, force: true }); } }); -test("settings are isolated by the authenticated repository principal", async () => { +test("settings no longer read or write principal-specific preferences", async () => { const preferences = new Map(); const runner = { withPrincipal: (principal: any) => ({ @@ -73,11 +74,11 @@ test("settings are isolated by the authenticated repository principal", async () const alice = await app.inject({ method: "GET", url: "/settings", headers: headers("alice") }); const bob = await app.inject({ method: "GET", url: "/settings", headers: headers("bob") }); - expect(alice.json()).toMatchObject({ workspace: "psd", thinking: "high" }); - expect(bob.json()).not.toMatchObject({ workspace: "psd", thinking: "high" }); + expect(alice.json()).toEqual(bob.json()); + expect(preferences.size).toBe(0); }); -test("GET /settings seeds an empty private profile from complete legacy settings once", async () => { +test("GET /settings retains complete legacy installation defaults without seeding a private profile", async () => { let preferences: Record = {}; const writes: Record[] = []; const runner = { @@ -104,13 +105,13 @@ test("GET /settings seeds an empty private profile from complete legacy settings expect(first.statusCode).toBe(200); expect(first.json()).toEqual(expected); expect(second.json()).toEqual(expected); - expect(writes).toEqual([expected]); + expect(writes).toEqual([]); } finally { rmSync(dir, { recursive: true, force: true }); } }); -test("GET /settings does not overwrite an existing private profile with legacy settings", async () => { +test("GET /settings ignores stale private preferences in favor of installation defaults", async () => { const privateSettings = { workspace: "private", provider: "zai", model: "glm-5.2", thinking: "high", }; @@ -130,7 +131,9 @@ test("GET /settings does not overwrite an existing private profile with legacy s const response = await app.inject({ method: "GET", url: "/settings" }); expect(response.statusCode).toBe(200); - expect(response.json()).toEqual(privateSettings); + expect(response.json()).toEqual({ + workspace: "local", provider: "local-qwen", model: "qwen3.6-35b-a3b", thinking: "low", + }); } finally { rmSync(dir, { recursive: true, force: true }); } diff --git a/harness/tests/test_session_documents.py b/harness/tests/test_session_documents.py index a7f8550b..91ea0fb2 100644 --- a/harness/tests/test_session_documents.py +++ b/harness/tests/test_session_documents.py @@ -7,7 +7,7 @@ from typer.testing import CliRunner from tht.config import DatabaseConfig from tht.decisions import DecisionRecord from tht.session.models import SessionSnapshot -from tht.session.store import build_documents, build_snapshot_documents, create_session +from tht.session.store import build_documents, build_snapshot_documents, create_session, new_session_manifest from tht.cli.session_cmd import session_app @@ -18,6 +18,15 @@ def _db(): ) +def test_manifest_persists_workspace_revision(): + manifest = new_session_manifest( + "q", _db(), workspace_id="psd-clinical", workspace_revision="a" * 40 + ) + + assert manifest.workspace_id == "psd-clinical" + assert manifest.workspace_revision == "a" * 40 + + def test_build_documents_always_has_original_question(tmp_path): m = create_session("quante ablazioni nel 2024", _db(), tmp_path) docs = build_documents(m, tmp_path / m.id) diff --git a/harness/tht/cli/session_cmd.py b/harness/tht/cli/session_cmd.py index 79e9f9e4..216c3e89 100644 --- a/harness/tht/cli/session_cmd.py +++ b/harness/tht/cli/session_cmd.py @@ -175,6 +175,8 @@ def new_cmd( provider: str = typer.Option(None, "--provider", help="Provider LLM (es. zai, anthropic)."), model: str = typer.Option(None, "--model", help="Modello LLM (es. glm-5.2)."), thinking: str = typer.Option(None, "--thinking", help="Livello di thinking (es. medium)."), + workspace_id: str = typer.Option(None, "--workspace-id", help="Workspace canonico risolto dal backend."), + workspace_revision: str = typer.Option(None, "--workspace-revision", help="Commit immutabile del workspace."), name: str = typer.Option(None, "--name", help="Nome descrittivo della sessione."), json_out: bool = typer.Option(False, "--json", help="Emetti JSON puro {\"id\": ...} su stdout."), config: Path = CONFIG_OPT, @@ -185,6 +187,7 @@ def new_cmd( cfg = _load_config_or_exit(config) manifest = new_session_manifest(question, cfg.database, provider=provider, model=model, thinking=thinking, + workspace_id=workspace_id, workspace_revision=workspace_revision, name=name or _extract_name(question)) repository = session_repository(cfg) repository.create(manifest) diff --git a/harness/tht/session/models.py b/harness/tht/session/models.py index 011310a6..b61ca5a5 100644 --- a/harness/tht/session/models.py +++ b/harness/tht/session/models.py @@ -117,6 +117,8 @@ class SessionManifest(_YamlModel): provider: str | None = None model: str | None = None thinking: str | None = None + workspace_id: str | None = None + workspace_revision: str | None = None name: str | None = None archived: bool = False group: str | None = None diff --git a/harness/tht/session/store.py b/harness/tht/session/store.py index b72f2b84..7097c81c 100644 --- a/harness/tht/session/store.py +++ b/harness/tht/session/store.py @@ -102,7 +102,8 @@ def create_session( question: str, db: DatabaseConfig, sessions_root: Path, *, author: str | None = None, summary: str | None = None, provider: str | None = None, model: str | None = None, - thinking: str | None = None, name: str | None = None, + thinking: str | None = None, workspace_id: str | None = None, + workspace_revision: str | None = None, name: str | None = None, ) -> SessionManifest: now = datetime.now(UTC) # stamp con ora/min/sec: identifica univocamente sessioni dello stesso giorno @@ -123,7 +124,8 @@ def create_session( database=db.database, schema=db.db_schema, author=who, summary=summary or _summarize(question), updated_at=now, updated_by=who, schema_version=schema_version, - provider=provider, model=model, thinking=thinking, name=name, + provider=provider, model=model, thinking=thinking, workspace_id=workspace_id, + workspace_revision=workspace_revision, name=name, ) session_dir = sessions_root / session_id manifest.to_yaml(session_dir / MANIFEST) @@ -132,7 +134,8 @@ def create_session( def new_session_manifest( - question: str, db: DatabaseConfig, *, provider=None, model=None, thinking=None, name=None + question: str, db: DatabaseConfig, *, provider=None, model=None, thinking=None, + workspace_id=None, workspace_revision=None, name=None, ) -> SessionManifest: """Create an unsaved UUIDv4 manifest for a repository-owned session.""" now = datetime.now(UTC) @@ -140,7 +143,8 @@ def new_session_manifest( id=str(uuid.uuid4()), created_at=now, question=question, database=db.database, schema=db.db_schema, author=current_author(), summary=_summarize(question), updated_at=now, updated_by=current_author(), - provider=provider, model=model, thinking=thinking, name=name, + provider=provider, model=model, thinking=thinking, workspace_id=workspace_id, + workspace_revision=workspace_revision, name=name, )