From e4fdbed86487936f52e5699a53526a6399bb4e1d Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 4 Aug 2026 09:25:06 +0200 Subject: [PATCH] fix: harden workspace activation and snapshot retention --- PROJECT_STATE.md | 21 +- README.md | 5 + backend/src/app.ts | 11 + backend/src/routes/sessions.ts | 217 ++++++++++-------- backend/src/workspaces/bindings.ts | 8 + backend/src/workspaces/diagnostics.ts | 14 +- backend/src/workspaces/registry.ts | 140 ++++++++++- backend/test/routes-sessions.test.ts | 95 ++++++++ backend/test/workspace-registry.test.ts | 27 +++ .../test/workspace-runtime-renderer.test.ts | 13 ++ backend/test/workspaces-diagnostics.test.ts | 8 +- docs/install/local-workspace-registry.md | 6 +- docs/install/server-workspace-registry.md | 6 +- docs/workspace-diagnostic-protocol.md | 6 + 14 files changed, 474 insertions(+), 103 deletions(-) diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 7839dc1b..beed187c 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -13,21 +13,28 @@ contents are never stored in Git, API responses, browser storage, diagnostics, or bundles. - **Migration and session safety.** Schema-v2 descriptors are operational; legacy descriptors are visible as `migration_required` until migrated by the documented operator workflow. New sessions - persist workspace ID and immutable Git revision. Resume resolves that historical snapshot, while - retention preserves every revision referenced by an open, closed, or failed unarchived manifest. + acquire a persistent revision lease before readiness and persist workspace ID plus immutable Git + revision. Retention hands that lease off only after an authoritative scan observes the manifest, + so a stale concurrent scan cannot prune the pinned snapshot. Resume resolves that historical + snapshot, while retention preserves every revision referenced by an open, closed, or failed + unarchived manifest. Reconciliation runs only with a complete local installation list or an administrator's complete server list, never from a remote user's partial view. +- **SSH connector boundary.** The current OpenSSH forward is owned by one bounded diagnostic and is + always cleaned up afterward. DWH/vector `ssh_tunnel` bindings therefore return + `workspace_not_activatable`, and new-session creation rejects them before persistence. Direct and + REST runtime connectors remain supported; Git remote access over SSH is unaffected. - **Operator manuals.** Follow [the local manual](docs/install/local-workspace-registry.md) for macOS/Windows/Linux Docker Desktop deployment and [the server manual](docs/install/server-workspace-registry.md) for Gitea-compatible remotes, reverse proxy, migration, backup, and recovery. The release workflow is Git review/push → installation pull → validate → local diagnostic test → browser-local workspace/model/reasoning selection → revision-pinned session. - **Verification recorded for this source branch.** `git diff --check` passed; backend Vitest - **365/365** and TypeScript passed; frontend Vitest **398/398** and TypeScript passed; the - harness document regression passed **10/10**. `./scripts/workspace-registry-smoke.sh`, both - installation-document verifier profiles, and a final unrestricted full harness run remain the - release commands to execute in the deployment environment; the local long-running harness run - was intentionally cancelled before it produced a final result. + **371/371** and TypeScript passed; frontend Vitest **398/398** and TypeScript passed; the + harness document regression passed **10/10**. `./scripts/workspace-registry-smoke.sh` and the + executable installation-manual fixture verifier passed with Docker. A final unrestricted full + harness run remains a release command for the deployment environment; the earlier local + long-running harness run was intentionally cancelled before it produced a final result. ## Session summary redesign — LIVE 2026-07-23 diff --git a/README.md b/README.md index a3d5b19c..db231057 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,11 @@ every revision referenced by an open, closed, or failed unarchived session. It r single local installation list or from a server administrator's complete session list, never from a remote user's partial list. +Connector `ssh_tunnel` bindings are diagnostic-only in this release: their bounded probe always +cleans up the loopback forward and returns `workspace_not_activatable`; session creation is rejected +before persistence. Git registry access over SSH is unaffected. Use direct or REST connector +transport for runtime sessions. + `docker-compose.dev.yml` is deliberately local: both published ports bind to `127.0.0.1`, `THT_SESSION_STORAGE=local`, and `THT_HOME=/data/local-home`. Do not set `THOTH_PUBLIC_EXPOSURE=true` for that profile; the backend rejects that public/local combination diff --git a/backend/src/app.ts b/backend/src/app.ts index df414f81..9b5d935f 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -18,6 +18,8 @@ import { ReadinessManager } from "./runtime/readiness-manager.js"; import { WorkspaceRegistry } from "./workspaces/registry.js"; import { createProductionWorkspaceDiagnoser } from "./workspaces/diagnostics.js"; import { workspaceRoutes, type WorkspaceDiagnoser } from "./routes/workspaces.js"; +import { resolveRuntimeBindings, supportsSessionRuntime } from "./workspaces/bindings.js"; +import type { WorkspaceDescriptor } from "./workspaces/schema.js"; export interface BuildAppDeps { thtRunner?: ThtRunner; @@ -29,6 +31,7 @@ export interface BuildAppDeps { hub?: SseHub; workspaceRegistry?: WorkspaceRegistry; workspaceDiagnoser?: WorkspaceDiagnoser; + workspaceRuntimeSupport?: (workspace: WorkspaceDescriptor) => boolean; } export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstance { @@ -55,6 +58,13 @@ export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstanc const workspaceRegistry = deps?.workspaceRegistry ?? new WorkspaceRegistry(config.workspaceRegistry); const workspaceDiagnoser = deps?.workspaceDiagnoser ?? createProductionWorkspaceDiagnoser(config.workspaceDiagnosticTimeoutMs); + const workspaceRuntimeSupport = deps?.workspaceRuntimeSupport ?? ((workspace: WorkspaceDescriptor) => ( + supportsSessionRuntime(resolveRuntimeBindings( + workspace, + process.env, + config.workspaceRegistry.secretRoots, + )) + )); const readiness = deps?.readiness ?? new ReadinessManager( tht as ThtRunner, Math.round(config.ollamaEnsureTimeoutMs / 1000), @@ -88,6 +98,7 @@ export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstanc mgr, tht: tht as ThtRunner, hub, getSettings, readiness, listModels, workspaceRegistry, dwhPrecheck: config.dwhPrecheck, legacyWorkspaceMode: config.legacyWorkspaceMode, + workspaceRuntimeSupport, }); sqlRoutes(app, { tht: tht as ThtRunner, getSettings }); metaRoutes(app, { harnessDir: config.harnessDir, listModels }); diff --git a/backend/src/routes/sessions.ts b/backend/src/routes/sessions.ts index 9b0163c6..136af41e 100644 --- a/backend/src/routes/sessions.ts +++ b/backend/src/routes/sessions.ts @@ -8,6 +8,7 @@ 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"; +import type { WorkspaceDescriptor } from "../workspaces/schema.js"; const BOOTSTRAP_FAILURE_MESSAGE = "Session startup failed. Check configuration and connectivity, then Resume the session."; @@ -34,6 +35,8 @@ export function sessionRoutes( dwhPrecheck?: boolean; /** Explicit loopback-only compatibility path for old clients that send `workspace`. */ legacyWorkspaceMode?: boolean; + /** Fail-closed installation/runtime transport capability check. */ + workspaceRuntimeSupport: (workspace: WorkspaceDescriptor) => boolean; }, ) { const lifecycleTails = new Map>(); @@ -289,110 +292,144 @@ export function sessionRoutes( code: "workspace_revision_unavailable", }); } - let workspaceConfigPath: string | undefined; - let workspaceId: string | undefined; - let workspaceRevision: string | undefined; - let allowedModels: readonly string[] | undefined; - if (requestedWorkspaceId) { - try { - const resolved = await d.workspaceRegistry.read(requestedWorkspaceId); - if (resolved.revision.state !== "operational") { + let revisionLease: Awaited> | undefined; + let manifestPersisted = false; + try { + let workspaceConfigPath: string | undefined; + let workspaceId: string | undefined; + let workspaceRevision: string | undefined; + let allowedModels: readonly string[] | undefined; + if (requestedWorkspaceId) { + try { + const registry = d.workspaceRegistry as Partial; + const resolved = typeof registry.acquireSessionRevision === "function" + ? await registry.acquireSessionRevision.call(d.workspaceRegistry, requestedWorkspaceId) + : await d.workspaceRegistry.read(requestedWorkspaceId); + if ("markPersisted" in resolved && "abort" in resolved) { + revisionLease = resolved as Awaited>; + } + if (resolved.revision.state !== "operational") { + return reply.code(409).send({ + error: WORKSPACE_REVISION_UNAVAILABLE_MESSAGE, + code: "workspace_revision_unavailable", + }); + } + if (!d.workspaceRuntimeSupport(resolved.workspace)) { + return reply.code(409).send({ + error: "This workspace transport is not available to runtime sessions.", + code: "workspace_not_activatable", + }); + } + 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", }); } - 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(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(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" }); + 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." }); } - } - if (provider && model) { - let available: Awaited>; + // 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(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(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 (provider && model) { + let available: Awaited>; + try { + available = await d.listModels(); + } catch { + return reply.code(503).send({ + error: MODEL_UNAVAILABLE_MESSAGE, + code: "model_unavailable", + }); + } + const selectedAvailable = available.some( + (candidate) => candidate.provider === provider && candidate.id === model, + ); + if (!selectedAvailable) { + return reply.code(503).send({ + error: MODEL_UNAVAILABLE_MESSAGE, + code: "model_unavailable", + }); + } + } + // 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 { - available = await d.listModels(); - } catch { - return reply.code(503).send({ - error: MODEL_UNAVAILABLE_MESSAGE, - code: "model_unavailable", + ({ id } = await runner.sessionNew({ + question: b.question, name: b.name, workspaceConfigPath, + workspaceId, workspaceRevision, provider, model, thinking, + })); + manifestPersisted = true; + if (revisionLease) { + await revisionLease.markPersisted().catch((error: unknown) => { + console.error( + `[session:${id}] revision lease hand-off failed:`, + error instanceof Error ? error.message : "unknown error", + ); + }); + } + } catch { return storageFailure(reply); } + const options = { + provider, model, thinking, + author: principal.displayName ?? principal.subject, + principal, + question: b.question, + }; + let rt: ReturnType | undefined; + try { + rt = d.mgr.createFor(id, options); + 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, workspaceConfigPath).catch((persistenceError: unknown) => { + console.error(`[session:${id}] failSession persistence failed:`, persistenceError); }); + return reply.code(503).send({ error: BOOTSTRAP_FAILURE_MESSAGE }); } - const selectedAvailable = available.some( - (candidate) => candidate.provider === provider && candidate.id === model, + info(id, "Session created"); + bootstrap( + id, rt, runner, workspaceConfigPath, d.mgr.configure(rt, options), + runner.searchPack(b.question, id, workspaceConfigPath), + () => d.mgr.start(id, rt, options), ); - if (!selectedAvailable) { - return reply.code(503).send({ - error: MODEL_UNAVAILABLE_MESSAGE, - code: "model_unavailable", + return { id }; + } finally { + if (revisionLease && !manifestPersisted) { + await revisionLease.abort().catch((error: unknown) => { + console.error( + "[session] revision lease cleanup failed:", + error instanceof Error ? error.message : "unknown error", + ); }); } } - // 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, workspaceConfigPath, - workspaceId, workspaceRevision, provider, model, thinking, - })); - } catch { return storageFailure(reply); } - const options = { - provider, model, thinking, - author: principal.displayName ?? principal.subject, - principal, - question: b.question, - }; - let rt: ReturnType | undefined; - try { - rt = d.mgr.createFor(id, options); - 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, 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, workspaceConfigPath, d.mgr.configure(rt, options), - runner.searchPack(b.question, id, workspaceConfigPath), - () => d.mgr.start(id, rt, options), - ); - return { id }; }); app.get("/sessions", async (req, reply) => { const principal = getPrincipal(req); diff --git a/backend/src/workspaces/bindings.ts b/backend/src/workspaces/bindings.ts index f991960b..deac77b6 100644 --- a/backend/src/workspaces/bindings.ts +++ b/backend/src/workspaces/bindings.ts @@ -143,3 +143,11 @@ export function resolveRuntimeBindings( embedding: resolveBinding(workspace, "EMBEDDING", env, secretRoots), }; } + +/** + * SSH bindings are currently probe-only: diagnostics owns a short-lived tunnel, while the + * session runtime has no tunnel owner. Keep activation fail-closed until that lifecycle exists. + */ +export function supportsSessionRuntime(bindings: RuntimeBindings): boolean { + return bindings.dwh.transport !== "ssh_tunnel" && bindings.vector.transport !== "ssh_tunnel"; +} diff --git a/backend/src/workspaces/diagnostics.ts b/backend/src/workspaces/diagnostics.ts index 945c08bd..aea16f25 100644 --- a/backend/src/workspaces/diagnostics.ts +++ b/backend/src/workspaces/diagnostics.ts @@ -535,7 +535,9 @@ function diagnosticError(code: WorkspaceErrorCode, field?: string): Diagnostic { ? "Installation binding is missing or invalid." : code === "semantic_index_incompatible" ? "Semantic index metadata is incompatible with this workspace." - : "Connector diagnostic failed.", + : code === "workspace_not_activatable" + ? "This transport can be tested, but it is not available to runtime sessions." + : "Connector diagnostic failed.", }; } @@ -851,6 +853,16 @@ export function createWorkspaceDiagnoser( } } + // The concrete SSH adapter deliberately owns only a bounded diagnostic tunnel and closes it + // in `finally`. Until a session runtime owns an equivalent long-lived tunnel, a successful + // probe is connectivity evidence only and must never be advertised as activatable. + if ( + (bindings.dwh.transport === "ssh_tunnel" || bindings.vector.transport === "ssh_tunnel") + && !diagnostics.some((diagnostic) => diagnostic.level === "error") + ) { + diagnostics.push(diagnosticError("workspace_not_activatable")); + } + return { activatable: !diagnostics.some((diagnostic) => diagnostic.level === "error"), diagnostics, diff --git a/backend/src/workspaces/registry.ts b/backend/src/workspaces/registry.ts index fa983371..f47efd38 100644 --- a/backend/src/workspaces/registry.ts +++ b/backend/src/workspaces/registry.ts @@ -28,6 +28,15 @@ export interface WorkspaceRevision { state: "operational" | "migration_required"; } +export interface SessionRevisionLease { + workspace: WorkspaceDescriptor; + revision: WorkspaceRevision; + /** Mark the manifest durable; retention removes the lease only after observing that manifest. */ + markPersisted(): Promise; + /** Remove a lease for a session that failed before its manifest was durable. */ + abort(): Promise; +} + export type PublishWorkspaceRequest = | { action: "create"; workspace: CanonicalWorkspace; baseCommit: string } | { action: "update"; workspace: CanonicalWorkspace; baseCommit: string; baseBlob: string } @@ -56,6 +65,14 @@ interface SnapshotManifest extends ActiveState { files: Record; } +interface RevisionLeaseRecord { + version: 1; + token: string; + workspaceId: string; + commit: string; + state: "creating" | "persisted"; +} + type LegacyWorkspaceRevision = Omit; interface LegacyActiveState { @@ -178,6 +195,56 @@ export class WorkspaceRegistry { } } + /** + * Resolve the active revision and create its cross-process retention lease under the same + * repository lock. The lease bridges the interval before `session_manifest.yaml` is durable. + */ + async acquireSessionRevision(id: string): Promise { + await this.repository.ensureLayout(); + return await this.lock.run(async () => { + const state = await this.activeState(); + const revision = state.revisions.find((candidate) => candidate.id === id); + if (!revision) throw new WorkspaceRegistryError("workspace_invalid", "Workspace is unavailable"); + let workspace: WorkspaceDescriptor; + try { + workspace = parseWorkspaceYaml(await readFile(revision.snapshotPath, "utf8")); + } catch (error) { + throw workspaceError(error); + } + + const token = randomUUID(); + const record: RevisionLeaseRecord = { + version: 1, + token, + workspaceId: id, + commit: revision.commit, + state: "creating", + }; + const path = await this.writeRevisionLease(record, true); + let localState: RevisionLeaseRecord["state"] | "aborted" = "creating"; + + return { + workspace, + revision, + markPersisted: async () => { + if (localState === "persisted") return; + if (localState === "aborted") throw new WorkspaceRegistryError( + "workspace_invalid", "Workspace revision lease is unavailable", + ); + await this.lock.run(async () => { + await this.replaceRevisionLease(path, { ...record, state: "persisted" }); + }); + localState = "persisted"; + }, + abort: async () => { + if (localState !== "creating") return; + await this.lock.run(async () => { await rm(path, { force: true }); }); + localState = "aborted"; + }, + }; + }); + } + /** 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); @@ -195,9 +262,12 @@ export class WorkspaceRegistry { * a partial, per-user list could otherwise remove another user's resumable workspace pin. */ async reconcileSnapshotRetention(referencedCommits: readonly string[]): Promise { - const retained = new Set(referencedCommits.map(safeCommit)); + const manifestReferences = new Set(referencedCommits.map(safeCommit)); + const retained = new Set(manifestReferences); await this.repository.ensureLayout(); await this.lock.run(async () => { + const leases = await this.revisionLeases(); + for (const { record } of leases) retained.add(record.commit); retained.add((await this.activeState()).head); const entries = await readdir(this.repository.snapshotsPath, { withFileTypes: true }); for (const entry of entries) { @@ -210,9 +280,77 @@ export class WorkspaceRegistry { if (!current.isDirectory() || current.isSymbolicLink()) continue; await rm(path, { recursive: true, force: true }); } + // A persisted lease is handed off only when this exact authoritative scan has observed a + // manifest pin for its commit. A stale scan therefore keeps the lease and cannot prune it. + for (const { path, record } of leases) { + if (record.state === "persisted" && manifestReferences.has(record.commit)) { + await rm(path, { force: true }); + } + } }); } + private revisionLeaseDirectory(): string { + return join(this.repository.statePath, "revision-leases"); + } + + private async writeRevisionLease(record: RevisionLeaseRecord, exclusive: boolean): Promise { + const directory = this.revisionLeaseDirectory(); + await mkdir(directory, { recursive: true, mode: 0o700 }); + const path = join(directory, `${record.token}.json`); + await writeFile(path, JSON.stringify(record), { + encoding: "utf8", + mode: 0o600, + flush: true, + ...(exclusive ? { flag: "wx" } : {}), + }); + return path; + } + + private async replaceRevisionLease(path: string, record: RevisionLeaseRecord): Promise { + const staging = `${path}.staging-${randomUUID()}`; + try { + await writeFile(staging, JSON.stringify(record), { + encoding: "utf8", mode: 0o600, flag: "wx", flush: true, + }); + await rename(staging, path); + } catch (error) { + await rm(staging, { force: true }); + throw error; + } + } + + private async revisionLeases(): Promise> { + const directory = this.revisionLeaseDirectory(); + await mkdir(directory, { recursive: true, mode: 0o700 }); + const entries = await readdir(directory, { withFileTypes: true }); + const leases: Array<{ path: string; record: RevisionLeaseRecord }> = []; + for (const entry of entries) { + if (!entry.isFile() || entry.isSymbolicLink() || !/^[0-9a-f-]{36}\.json$/.test(entry.name)) { + throw new WorkspaceRegistryError("workspace_invalid", "Workspace revision lease is invalid"); + } + const path = join(directory, entry.name); + let record: RevisionLeaseRecord; + try { + record = JSON.parse(await readFile(path, "utf8")) as RevisionLeaseRecord; + } catch { + throw new WorkspaceRegistryError("workspace_invalid", "Workspace revision lease is invalid"); + } + if ( + record.version !== 1 + || `${record.token}.json` !== entry.name + || !/^[0-9a-f-]{36}$/.test(record.token) + || !/^[a-z][a-z0-9-]{2,62}$/.test(record.workspaceId) + || !/^[0-9a-f]{40}$/.test(record.commit) + || (record.state !== "creating" && record.state !== "persisted") + ) { + throw new WorkspaceRegistryError("workspace_invalid", "Workspace revision lease is invalid"); + } + leases.push({ path, record }); + } + return leases; + } + /** * 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 eef17d4e..5610c95d 100644 --- a/backend/test/routes-sessions.test.ts +++ b/backend/test/routes-sessions.test.ts @@ -30,6 +30,7 @@ const defaultWorkspaceRegistry = { function buildApp(config: Parameters[0], deps: Record = {}) { return buildRealApp(config, { + workspaceRuntimeSupport: () => true, ...deps, workspaceRegistry: { ...defaultWorkspaceRegistry, ...(deps.workspaceRegistry as object | undefined) }, } as any); @@ -324,6 +325,100 @@ test("creates a session from the active immutable workspace revision", async () })); }); +test("rejects an SSH-only workspace before persisting or starting a session", async () => { + const sessionNew = vi.fn(async () => ({ id: "must-not-exist" })); + const ensure = vi.fn(async () => ({ ok: true })); + const createFor = vi.fn(); + const abort = vi.fn(async () => {}); + const markPersisted = vi.fn(async () => {}); + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: { sessionNew, searchPack: async () => {} } as any, + readiness: { ensure } as any, + mgr: { get: () => undefined, createFor } as any, + getSettings: () => ({ workspace: "ssh-workspace" }) as any, + workspaceRuntimeSupport: vi.fn(() => false), + workspaceRegistry: { + acquireSessionRevision: vi.fn(async () => ({ + workspace: { + workspace: { schema_version: 2, id: "ssh-workspace", name: "SSH", language: "en" }, + dwh: { + engine: "postgres", database: "postgres", schema: "public", + supported_transports: ["ssh_tunnel"], + }, + semantic_index: { + vector_store: { + engine: "pgvector", database: "postgres", schema: "vectors", + collection: "documents", dimensions: 768, distance: "cosine", + supported_transports: ["ssh_tunnel"], + }, + embedding: { + provider: "ollama_compatible", model: "nomic-embed-text", dimensions: 768, + }, + }, + llm_policy: { allowed: ["zai/glm-5.2"] }, + }, + revision: { + id: "ssh-workspace", commit: "a".repeat(40), blob: "b".repeat(40), + snapshotPath: `/data/workspace-registry/snapshots/${"a".repeat(40)}/ssh-workspace.yaml`, + state: "operational", + }, + abort, + markPersisted, + })), + } as any, + }); + + const response = await app.inject({ method: "POST", url: "/sessions", payload: { question: "q" } }); + + expect(response.statusCode).toBe(409); + expect(response.json()).toMatchObject({ code: "workspace_not_activatable" }); + expect(ensure).not.toHaveBeenCalled(); + expect(sessionNew).not.toHaveBeenCalled(); + expect(createFor).not.toHaveBeenCalled(); + expect(abort).toHaveBeenCalledOnce(); + expect(markPersisted).not.toHaveBeenCalled(); +}); + +test("hands a revision lease to retention only after the session manifest is durable", async () => { + const persisted = deferred<{ id: string }>(); + const markPersisted = vi.fn(async () => {}); + const abort = vi.fn(async () => {}); + const acquireSessionRevision = vi.fn(async () => ({ + workspace: { llm_policy: { allowed: ["zai/glm-5.2"] } }, + revision: { + id: "leased", commit: "a".repeat(40), blob: "b".repeat(40), + snapshotPath: `/data/workspace-registry/snapshots/${"a".repeat(40)}/leased.yaml`, + state: "operational", + }, + markPersisted, + abort, + })); + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: { sessionNew: () => persisted.promise, searchPack: async () => {} } as any, + readiness: { ensure: async () => ({ ok: true }) } as any, + mgr: { + get: () => undefined, + createFor: () => ({ bridge: { onClientEvent: () => {} } }), + configure: async () => {}, + start: () => {}, + } as any, + getSettings: () => ({ workspace: "leased", provider: "zai", model: "glm-5.2" }) as any, + listModels: async () => [{ provider: "zai", id: "glm-5.2", name: "GLM", reasoning: true }], + workspaceRuntimeSupport: () => true, + workspaceRegistry: { acquireSessionRevision } as any, + }); + + const request = app.inject({ method: "POST", url: "/sessions", payload: { question: "q" } }); + await new Promise((resolve) => setImmediate(resolve)); + expect(markPersisted).not.toHaveBeenCalled(); + expect(abort).not.toHaveBeenCalled(); + + persisted.resolve({ id: "leased-session" }); + expect((await request).statusCode).toBe(200); + expect(markPersisted).toHaveBeenCalledOnce(); + expect(abort).not.toHaveBeenCalled(); +}); + test("creates a session from the configured default workspace revision when workspaceId is omitted", async () => { const sessionNew = vi.fn(async () => ({ id: "default-pinned" })); const registry = { diff --git a/backend/test/workspace-registry.test.ts b/backend/test/workspace-registry.test.ts index e3384b53..fef26e14 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("a session revision lease survives stale retention scans until its manifest is observed", async () => { + const remote = await fixture(); + const root = join(remote.root, "registry"); + const registry = new WorkspaceRegistry(config(root, remote.remote)); + await registry.bootstrap(); + const lease = await registry.acquireSessionRevision("psd-clinical"); + + writeFileSync(join(remote.source, "workspaces", "psd-clinical.yaml"), validYaml.replace( + "name: Policlinico San Donato", "name: Concurrent revision", + )); + await git(remote.source, ["add", "workspaces/psd-clinical.yaml"]); + await git(remote.source, ["commit", "-m", "Publish while session is starting"]); + await git(remote.source, ["push", "origin", "main"]); + await registry.pull(); + + await registry.reconcileSnapshotRetention([]); + expect(existsSync(registry.snapshotPath(remote.initialCommit, "psd-clinical"))).toBe(true); + + await lease.markPersisted(); + await registry.reconcileSnapshotRetention([]); + expect(existsSync(registry.snapshotPath(remote.initialCommit, "psd-clinical"))).toBe(true); + + await registry.reconcileSnapshotRetention([remote.initialCommit]); + await registry.reconcileSnapshotRetention([]); + expect(existsSync(registry.snapshotPath(remote.initialCommit, "psd-clinical"))).toBe(false); +}); + 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"); diff --git a/backend/test/workspace-runtime-renderer.test.ts b/backend/test/workspace-runtime-renderer.test.ts index c677312b..e45d93cf 100644 --- a/backend/test/workspace-runtime-renderer.test.ts +++ b/backend/test/workspace-runtime-renderer.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "vitest"; import { parse } from "yaml"; import { renderRuntimeConfig, type RuntimeBindings, type RuntimePaths } from "../src/workspaces/runtime-renderer.js"; +import { supportsSessionRuntime } from "../src/workspaces/bindings.js"; import { parseWorkspaceYaml } from "../src/workspaces/schema.js"; const workspace = parseWorkspaceYaml(`workspace: @@ -90,6 +91,18 @@ const directBindings: RuntimeBindings = { }, }; +test("runtime support stays fail-closed for either SSH connector", () => { + expect(supportsSessionRuntime(directBindings)).toBe(true); + expect(supportsSessionRuntime({ + ...directBindings, + dwh: { ...directBindings.dwh, transport: "ssh_tunnel" }, + })).toBe(false); + expect(supportsSessionRuntime({ + ...directBindings, + vector: { ...directBindings.vector, transport: "ssh_tunnel" }, + })).toBe(false); +}); + test("renders a direct PostgreSQL binding to the legacy harness shape", () => { const yaml = renderRuntimeConfig(workspace, directBindings, paths); const rendered = parse(yaml); diff --git a/backend/test/workspaces-diagnostics.test.ts b/backend/test/workspaces-diagnostics.test.ts index cae90433..43f3eaf6 100644 --- a/backend/test/workspaces-diagnostics.test.ts +++ b/backend/test/workspaces-diagnostics.test.ts @@ -336,7 +336,7 @@ llm_policy: })); }); -test("uses a loopback-only SSH tunnel for the bounded connector probe", async () => { +test("does not advertise a probe-only SSH tunnel as usable by runtime sessions", async () => { const adapters = successfulAdapters(); const sshBindings: RuntimeBindings = { ...bindings, @@ -360,7 +360,11 @@ test("uses a loopback-only SSH tunnel for the bounded connector probe", async () const result = await diagnose(adapters)(workspace, sshBindings, { writeProbe: false }); - expect(result.activatable).toBe(true); + expect(result.activatable).toBe(false); + expect(result.diagnostics).toContainEqual(expect.objectContaining({ + level: "error", + code: "workspace_not_activatable", + })); expect(adapters.withSshTunnel).toHaveBeenCalledWith(expect.objectContaining({ localHost: "127.0.0.1", localPort: 0, diff --git a/docs/install/local-workspace-registry.md b/docs/install/local-workspace-registry.md index 03a1223e..989753dc 100644 --- a/docs/install/local-workspace-registry.md +++ b/docs/install/local-workspace-registry.md @@ -111,7 +111,7 @@ THT_WS_PSD_CLINICAL_VECTOR_API_KEY_FILE=/run/secrets/psd-vector-api-key ``` ```dotenv -# SSH tunnel; host-key verification and TLS target name remain mandatory. +# SSH tunnel diagnostic only; runtime sessions are fail-closed in this release. THT_WS_PSD_CLINICAL_DWH_TRANSPORT=ssh_tunnel THT_WS_PSD_CLINICAL_DWH_USER=thoth_reader THT_WS_PSD_CLINICAL_DWH_PASSWORD_FILE=/run/secrets/psd-dwh-reader @@ -128,6 +128,10 @@ Repeat the SSH names for `VECTOR` where needed. REST diagnostics reject a privat rather than weakening TLS; use runtime-trusted HTTPS or verified direct/SSH native TLS. See the [diagnostic protocol](../workspace-diagnostic-protocol.md). +An SSH connector can prove installation reachability, host-key verification, authentication, and +target identity, but it intentionally returns `workspace_not_activatable`; select direct or REST +before creating sessions. Git pull/push over SSH remains fully supported and is independent. + ## Bootstrap, first pull, and diagnostics Copy [the local Compose example](examples/local-compose.workspace-registry.yaml) and exactly one diff --git a/docs/install/server-workspace-registry.md b/docs/install/server-workspace-registry.md index f7be498c..48b28f19 100644 --- a/docs/install/server-workspace-registry.md +++ b/docs/install/server-workspace-registry.md @@ -114,7 +114,7 @@ THT_WS_PSD_CLINICAL_EMBEDDING_BASE_URL=https://embeddings.internal.example ``` ```dotenv -# SSH tunnel requires explicit host-key verification and TLS target identity. +# SSH tunnel diagnostic only; runtime sessions are fail-closed in this release. THT_WS_PSD_CLINICAL_DWH_TRANSPORT=ssh_tunnel THT_WS_PSD_CLINICAL_DWH_USER=thoth_reader THT_WS_PSD_CLINICAL_DWH_PASSWORD_FILE=/run/secrets/psd-dwh-reader @@ -132,6 +132,10 @@ rather than disable verification; use runtime-trusted HTTPS or verified direct/S the [diagnostic protocol](../workspace-diagnostic-protocol.md) for its read-only checks and optional reversible writer probe. +An SSH connector can be tested with strict host-key and target verification, but it intentionally +returns `workspace_not_activatable`; configure direct or REST transport before starting sessions. +The Git registry itself may still use SSH normally. + ## Same-origin reverse proxy, bootstrap, and health Copy [the server Compose example](examples/server-compose.workspace-registry.yaml) plus exactly one diff --git a/docs/workspace-diagnostic-protocol.md b/docs/workspace-diagnostic-protocol.md index 8e620b6e..159ddb43 100644 --- a/docs/workspace-diagnostic-protocol.md +++ b/docs/workspace-diagnostic-protocol.md @@ -227,6 +227,12 @@ The local listener is `127.0.0.1` only. The process is terminated in cleanup aft probe, on timeout, or on failure. There is no accept-new mode, no disabled host-key checking, and no persistent forwarding. +In this release, `ssh_tunnel` is therefore a diagnostic-only connector transport. A successful +probe is followed by `workspace_not_activatable`, and `POST /sessions` rejects the workspace before +persisting a manifest or starting Pi. Use direct PostgreSQL/pgvector or REST for runtime sessions +until the backend owns a tunnel for the full runtime lifecycle. This restriction does not apply to +using SSH as the transport for the workspace Git remote. + ## Reader-only fallback A workspace may be fully valid in Git but non-activatable locally when a required reader binding,