From f7b9f3276bc6abf1ac10b8cbb30ed0139701fae6 Mon Sep 17 00:00:00 2001 From: mptyl Date: Sat, 8 Aug 2026 17:10:15 +0200 Subject: [PATCH] test: restore task 2 review coverage --- .../task-2-report.md | 66 +++++++++++++ backend/test/routes-workspaces.test.ts | 98 ++++++++++++++++++- backend/test/workspace-registry.test.ts | 25 +++++ 3 files changed, 187 insertions(+), 2 deletions(-) diff --git a/.superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-2-report.md b/.superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-2-report.md index 94a1a113..9f88efee 100644 --- a/.superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-2-report.md +++ b/.superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-2-report.md @@ -97,3 +97,69 @@ Concerns - No code concerns remaining for Task 2. - One deliberate scope exception: a route test was updated to align with the already-established Task 1 / Task 3 fail-closed contract. + +Fix round 1 + +Scope + +- Restored meaningful route-level diagnoser coverage without reopening schema-v3 semantic runtime paths. +- Added direct schema-v2 registry coverage for `migration_required` listing and lease rejection. + +Covering test files + +- `backend/test/routes-workspaces.test.ts` +- `backend/test/workspace-registry.test.ts` + +RED command and output + +Command: + +```bash +cd backend +npx vitest run test/routes-workspaces.test.ts test/workspace-registry.test.ts +``` + +Observed result on top of `76bc94d` after adding the restored/new assertions: + +- 2 files passed +- 37 tests passed +- 0 failures + +Why no RED appeared: + +- The review items exposed missing/weakened coverage, not a production behavior bug. +- `/workspaces/:id/test` already reaches the diagnoser for resolvable legacy v2 descriptors. +- Schema-v3 `/workspaces/:id/test` already fails closed before diagnoser entry. +- Schema-v2 descriptors were already listed as `migration_required` and already rejected by `acquireSessionRevision()`. + +GREEN command and output + +Command: + +```bash +cd backend +npx vitest run test/routes-workspaces.test.ts test/workspace-registry.test.ts +npx tsc --noEmit -p . +``` + +Fresh results: + +- covering tests: 2 files passed, 37 tests passed +- backend typecheck: passed + +Changed files + +- `backend/test/routes-workspaces.test.ts` +- `backend/test/workspace-registry.test.ts` +- `.superpowers/sdd/2026-08-08-internal-qdrant-ollama/task-2-report.md` + +What changed + +- Split route coverage so `POST /workspaces/validate` still checks canonical validation independently. +- Restored route-level diagnoser coverage through a migration-required schema-v2 descriptor with resolvable legacy bindings. +- Added an explicit schema-v3 fail-closed regression for `POST /workspaces/:id/test`. +- Added a direct schema-v2 registry regression proving `list()` returns `migration_required` and `acquireSessionRevision()` rejects it. + +Concerns + +- No production concerns. This round only tightened coverage and corrected the weakened test expectation. diff --git a/backend/test/routes-workspaces.test.ts b/backend/test/routes-workspaces.test.ts index 916c898d..c339d4d9 100644 --- a/backend/test/routes-workspaces.test.ts +++ b/backend/test/routes-workspaces.test.ts @@ -7,7 +7,7 @@ import { buildApp } from "../src/app.js"; import { loadConfig } from "../src/config.js"; import { WorkspaceRegistryError } from "../src/workspaces/git-repository.js"; import type { WorkspaceRegistry, WorkspaceRevision } from "../src/workspaces/registry.js"; -import { renderWorkspaceDocs, serializeWorkspaceYaml, type CanonicalWorkspace } from "../src/workspaces/schema.js"; +import { renderWorkspaceDocs, serializeWorkspaceYaml, type CanonicalWorkspace, type WorkspaceV2 } from "../src/workspaces/schema.js"; const workspace: CanonicalWorkspace = { workspace: { @@ -39,6 +39,61 @@ const workspace: CanonicalWorkspace = { llm_policy: { allowed: ["zai/glm-5.2"] }, }; +const workspaceV2: WorkspaceV2 = { + workspace: { + schema_version: 2, + id: "psd-clinical", + name: "Policlinico San Donato", + description: "Clinical analytics workspace", + language: "it", + }, + dwh: { + engine: "postgres", + database: "warehouse", + schema: "datawarehouse", + supported_transports: ["rest_api"], + }, + semantic_index: { + vector_store: { + engine: "pgvector", + database: "warehouse", + schema: "vectors", + collection: "clinical_documents", + dimensions: 768, + distance: "cosine", + supported_transports: ["rest_api"], + }, + embedding: { + provider: "ollama_compatible", + model: "nomic-embed-text-v2-moe", + dimensions: 768, + }, + }, + diagnostics: { + dwh_rest: { + method: "GET", + path: "/health", + auth: "none", + response: { database: "database", schema: "schema" }, + }, + vector_rest: { + metadata: { + method: "GET", + path: "/metadata", + auth: "none", + response: { collection: "collection", dimensions: "dimensions", distance: "distance" }, + }, + }, + embedding: { + method: "GET", + path: "/models", + auth: "none", + response: { model: "model", dimensions: "dimensions" }, + }, + }, + llm_policy: { allowed: ["zai/glm-5.2"] }, +}; + const revision: WorkspaceRevision = { id: workspace.workspace.id, commit: "a".repeat(40), @@ -177,10 +232,49 @@ test("validates a canonical workspace and runs the injected installation diagnos const app = appFor(registryFake(), diagnose); const validate = await app.inject({ method: "POST", url: "/workspaces/validate", payload: { workspace } }); - const testResult = await app.inject({ method: "POST", url: "/workspaces/psd-clinical/test", payload: {} }); expect(validate.statusCode).toBe(200); expect(validate.json()).toMatchObject({ workspace }); + expect(diagnose).not.toHaveBeenCalled(); +}); + +test("runs the injected installation diagnostic for a migration-required v2 workspace when legacy bindings resolve", async () => { + const diagnose = vi.fn(async () => ({ + activatable: false, + diagnostics: [{ level: "error" as const, code: "binding_missing" as const, field: "THT_WS_PSD_CLINICAL_VECTOR_BASE_URL", message: "Installation binding is missing or invalid." }], + })); + const registry = registryFake({ + read: vi.fn(async () => ({ workspace: workspaceV2, revision: { ...revision, state: "migration_required" as const } })), + }); + const app = appFor(registry, diagnose); + + const originalEnv = { ...process.env }; + process.env.THT_WS_PSD_CLINICAL_DWH_TRANSPORT = "rest_api"; + process.env.THT_WS_PSD_CLINICAL_DWH_BASE_URL = "https://dwh.example.test"; + process.env.THT_WS_PSD_CLINICAL_VECTOR_TRANSPORT = "rest_api"; + process.env.THT_WS_PSD_CLINICAL_VECTOR_BASE_URL = "https://vector.example.test"; + process.env.THT_WS_PSD_CLINICAL_EMBEDDING_BASE_URL = "https://embedding.example.test"; + try { + const testResult = await app.inject({ method: "POST", url: "/workspaces/psd-clinical/test", payload: {} }); + + expect(testResult.statusCode).toBe(200); + expect(testResult.json()).toMatchObject({ activatable: false, diagnostics: [{ code: "binding_missing" }] }); + expect(diagnose).toHaveBeenCalledWith(workspaceV2, expect.objectContaining({ + dwh: expect.objectContaining({ transport: "rest_api", missing: [] }), + vector: expect.objectContaining({ transport: "rest_api", missing: [] }), + embedding: expect.objectContaining({ transport: "rest_api", missing: [] }), + }), { writeProbe: false }); + } finally { + process.env = originalEnv; + } +}); + +test("fails closed for /workspaces/:id/test on a schema v3 workspace before the internal runtime lands", async () => { + const diagnose = vi.fn(async () => ({ activatable: true, diagnostics: [] })); + const app = appFor(registryFake(), diagnose); + + const testResult = await app.inject({ method: "POST", url: "/workspaces/psd-clinical/test", payload: {} }); + expect(testResult.statusCode).toBe(400); expect(testResult.json()).toMatchObject({ code: "workspace_invalid" }); expect(diagnose).not.toHaveBeenCalled(); diff --git a/backend/test/workspace-registry.test.ts b/backend/test/workspace-registry.test.ts index 28b6dda1..5835bbce 100644 --- a/backend/test/workspace-registry.test.ts +++ b/backend/test/workspace-registry.test.ts @@ -139,6 +139,18 @@ function legacyV1Yaml(source = validYaml): string { .replace("schema_version: 3", "schema_version: 1"); } +function legacyV2Yaml(source = validYaml): string { + return source + .replace(" engine: qdrant\n", " engine: pgvector\n database: postgres\n schema: vectors\n") + .replace(" collection: psd-clinical\n", " collection: psd_clinical\n") + .replace(" dimensions: 1024", " dimensions: 768") + .replace(" provider: ollama_internal", " provider: ollama_compatible") + .replace(" model: qwen3-embedding:0.6b", " model: nomic-embed-text-v2-moe") + .replace(" dimensions: 1024", " dimensions: 768") + .replace("distance: cosine\n", "distance: cosine\n supported_transports: [pgvector_direct]\n") + .replace("schema_version: 3", "schema_version: 2"); +} + const runFile = promisify(execFile); const temporaryRoots: string[] = []; @@ -639,6 +651,19 @@ test("does not acquire a session revision lease for a migration_required workspa }); }); +test("lists a schema v2 descriptor as migration_required and refuses to acquire it", async () => { + const remote = await fixture(legacyV2Yaml()); + const registry = new WorkspaceRegistry(config(join(remote.root, "registry"), remote.remote)); + await registry.bootstrap(); + + await expect(registry.list()).resolves.toMatchObject([ + { id: "psd-clinical", state: "migration_required" }, + ]); + await expect(registry.acquireSessionRevision("psd-clinical")).rejects.toMatchObject({ + code: "workspace_invalid", + }); +}); + 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");