test: restore task 2 review coverage
This commit is contained in:
@@ -97,3 +97,69 @@ Concerns
|
|||||||
|
|
||||||
- No code concerns remaining for Task 2.
|
- 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.
|
- 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.
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ import { buildApp } from "../src/app.js";
|
|||||||
import { loadConfig } from "../src/config.js";
|
import { loadConfig } from "../src/config.js";
|
||||||
import { WorkspaceRegistryError } from "../src/workspaces/git-repository.js";
|
import { WorkspaceRegistryError } from "../src/workspaces/git-repository.js";
|
||||||
import type { WorkspaceRegistry, WorkspaceRevision } from "../src/workspaces/registry.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 = {
|
const workspace: CanonicalWorkspace = {
|
||||||
workspace: {
|
workspace: {
|
||||||
@@ -39,6 +39,61 @@ const workspace: CanonicalWorkspace = {
|
|||||||
llm_policy: { allowed: ["zai/glm-5.2"] },
|
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 = {
|
const revision: WorkspaceRevision = {
|
||||||
id: workspace.workspace.id,
|
id: workspace.workspace.id,
|
||||||
commit: "a".repeat(40),
|
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 app = appFor(registryFake(), diagnose);
|
||||||
|
|
||||||
const validate = await app.inject({ method: "POST", url: "/workspaces/validate", payload: { workspace } });
|
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.statusCode).toBe(200);
|
||||||
expect(validate.json()).toMatchObject({ workspace });
|
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.statusCode).toBe(400);
|
||||||
expect(testResult.json()).toMatchObject({ code: "workspace_invalid" });
|
expect(testResult.json()).toMatchObject({ code: "workspace_invalid" });
|
||||||
expect(diagnose).not.toHaveBeenCalled();
|
expect(diagnose).not.toHaveBeenCalled();
|
||||||
|
|||||||
@@ -139,6 +139,18 @@ function legacyV1Yaml(source = validYaml): string {
|
|||||||
.replace("schema_version: 3", "schema_version: 1");
|
.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 runFile = promisify(execFile);
|
||||||
const temporaryRoots: string[] = [];
|
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 () => {
|
test("lists operational descriptors retained after their workspace was removed from the active revision", async () => {
|
||||||
const remote = await fixture();
|
const remote = await fixture();
|
||||||
const root = join(remote.root, "registry");
|
const root = join(remote.root, "registry");
|
||||||
|
|||||||
Reference in New Issue
Block a user