From d630cac3ab0860c967dcdc008509e4e010da7bfe Mon Sep 17 00:00:00 2001 From: mptyl Date: Sat, 27 Jun 2026 21:31:35 +0200 Subject: [PATCH] feat(task-10): SQL routes + /workspaces + /models + fix sql preview path bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harness: - preview_cmd FILE positional arg made optional; when omitted with --session, path is derived via _session_sql_file (mirrors export_cmd) — fixes the deferred Task-5 bug where the backend passed sessions//sql_final.sql relative to harnessDir, which broke for workspace-dependent paths. - New pytest: test_preview_session_no_file_resolves_sql_final Backend: - ThtRunner.sqlPreview: drop positional file arg; use --session only - New routes/sql.ts: POST /sessions/:id/sql/preview + /export - New routes/meta.ts: GET /workspaces (yaml scan) + GET /models (injectable seam + graceful fallback to {models:[]}) - app.ts: register sqlRoutes + metaRoutes; add listModels to BuildAppDeps - tht-runner.test.ts: add sqlPreview argv assertion (no file path) - test/routes-sql-meta.test.ts: 9 tests (sql preview/export + meta routes) Tests: harness 233 passed; backend 29 passed; build clean. Co-Authored-By: Claude Sonnet 4.6 --- backend/src/app.ts | 5 + backend/src/routes/meta.ts | 59 +++++++++ backend/src/routes/sql.ts | 25 ++++ backend/src/tht/tht-runner.ts | 4 +- backend/test/routes-sql-meta.test.ts | 158 +++++++++++++++++++++++++ backend/test/tht-runner.test.ts | 25 ++++ harness/tests/test_sql_preview_json.py | 38 ++++++ harness/tht/cli/sql_cmd.py | 20 +++- 8 files changed, 330 insertions(+), 4 deletions(-) create mode 100644 backend/src/routes/meta.ts create mode 100644 backend/src/routes/sql.ts create mode 100644 backend/test/routes-sql-meta.test.ts diff --git a/backend/src/app.ts b/backend/src/app.ts index c8a20d0f..e5de0404 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -5,10 +5,13 @@ import { PiProcessManager } from "./pi/pi-process-manager.js"; import { SseHub } from "./sse/sse-hub.js"; import { authPreHandler } from "./auth/auth.js"; import { sessionRoutes } from "./routes/sessions.js"; +import { sqlRoutes } from "./routes/sql.js"; +import { metaRoutes, type ListModelsFn } from "./routes/meta.js"; export interface BuildAppDeps { thtRunner?: ThtRunner; spawnFn?: () => any; + listModels?: ListModelsFn; } export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstance { @@ -25,6 +28,8 @@ export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstanc app.addHook("preHandler", authPreHandler(config.authMode)); app.get("/health", async () => ({ status: "ok" })); sessionRoutes(app, { mgr, tht: tht as ThtRunner, hub }); + sqlRoutes(app, { tht: tht as ThtRunner }); + metaRoutes(app, { harnessDir: config.harnessDir, listModels: deps?.listModels }); return app; } diff --git a/backend/src/routes/meta.ts b/backend/src/routes/meta.ts new file mode 100644 index 00000000..be8dec79 --- /dev/null +++ b/backend/src/routes/meta.ts @@ -0,0 +1,59 @@ +import { readdirSync } from "node:fs"; +import { join } from "node:path"; +import type { FastifyInstance } from "fastify"; + +export type ListModelsFn = () => Promise; + +/** + * List YAML workspace configs found in /workspaces/*.yaml. + * Returns [{name, file}] — no database credentials or secrets. + */ +function listWorkspaces(harnessDir: string): { name: string; file: string }[] { + const dir = join(harnessDir, "workspaces"); + let entries: string[]; + try { + entries = readdirSync(dir); + } catch { + return []; + } + return entries + .filter((f) => f.endsWith(".yaml") || f.endsWith(".yml")) + .map((f) => ({ + name: f.replace(/\.ya?ml$/, ""), + file: f, + })); +} + +export function metaRoutes( + app: FastifyInstance, + deps: { harnessDir: string; listModels?: ListModelsFn }, +): void { + app.get("/workspaces", async () => { + return listWorkspaces(deps.harnessDir); + }); + + app.get("/models", async (_req, reply) => { + const fn = deps.listModels ?? defaultListModels; + try { + const models = await fn(); + return { models }; + } catch { + // Graceful fallback: Pi may not be running; don't crash the server. + return { models: [] }; + } + }); +} + +/** + * Default implementation: spawns a short-lived `tht` invocation that asks a Pi + * process for available models via get_available_models. This is intentionally + * behind the injectable seam so tests can stub it without spawning real processes. + * + * In the MVP we return an empty list — the real spawn path can be wired in later + * once a Pi-side "list models" RPC stabilises. + */ +async function defaultListModels(): Promise { + // Real ephemeral Pi spawn left for a follow-up task. + // Returning [] here triggers the graceful fallback seen by clients. + return []; +} diff --git a/backend/src/routes/sql.ts b/backend/src/routes/sql.ts new file mode 100644 index 00000000..8b2970b1 --- /dev/null +++ b/backend/src/routes/sql.ts @@ -0,0 +1,25 @@ +import type { FastifyInstance } from "fastify"; +import type { ThtRunner } from "../tht/tht-runner.js"; + +export function sqlRoutes(app: FastifyInstance, deps: { tht: ThtRunner }): void { + app.post("/sessions/:id/sql/preview", async (req, reply) => { + const id = (req.params as any).id as string; + const { limit, offset } = (req.body as any) ?? {}; + try { + const result = await deps.tht.sqlPreview(id, { limit, offset }); + return result; + } catch (err: any) { + return reply.code(500).send({ error: err.message ?? String(err) }); + } + }); + + app.post("/sessions/:id/sql/export", async (req, reply) => { + const id = (req.params as any).id as string; + try { + const result = await deps.tht.sqlExport(id); + return result; + } catch (err: any) { + return reply.code(500).send({ error: err.message ?? String(err) }); + } + }); +} diff --git a/backend/src/tht/tht-runner.ts b/backend/src/tht/tht-runner.ts index 04fba387..c18b0caf 100644 --- a/backend/src/tht/tht-runner.ts +++ b/backend/src/tht/tht-runner.ts @@ -82,7 +82,9 @@ export class ThtRunner { } sqlPreview(id: string, p: { limit?: number; offset?: number }) { - const a = ["sql", "preview", `sessions/${id}/sql_final.sql`, "--session", id, "--json"]; + // No positional FILE: the harness resolves sql_final.sql from the session + // via _session_sql_file(cfg, session_id), which respects the workspace path. + const a = ["sql", "preview", "--session", id, "--json"]; if (p.limit != null) a.push("--limit", String(p.limit)); if (p.offset != null) a.push("--offset", String(p.offset)); return this.json<{ diff --git a/backend/test/routes-sql-meta.test.ts b/backend/test/routes-sql-meta.test.ts new file mode 100644 index 00000000..5b6ed253 --- /dev/null +++ b/backend/test/routes-sql-meta.test.ts @@ -0,0 +1,158 @@ +import { test, expect } from "vitest"; +import { buildApp } from "../src/app.js"; +import { loadConfig } from "../src/config.js"; + +// --------------------------------------------------------------------------- +// SQL routes +// --------------------------------------------------------------------------- + +test("POST /sessions/:id/sql/preview returns rows from injected thtRunner stub", async () => { + const previewResult = { + columns: ["a"], + rows: [[1]], + execution_ms: 2, + truncated: false, + }; + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: { + sqlPreview: async () => previewResult, + } as any, + }); + + const res = await app.inject({ + method: "POST", + url: "/sessions/s1/sql/preview", + payload: { limit: 10, offset: 0 }, + }); + + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual(previewResult); +}); + +test("POST /sessions/:id/sql/preview passes limit and offset to thtRunner", async () => { + let captured: any; + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: { + sqlPreview: async (id: string, p: any) => { + captured = { id, ...p }; + return { columns: [], rows: [], execution_ms: 0, truncated: false }; + }, + } as any, + }); + + await app.inject({ + method: "POST", + url: "/sessions/abc/sql/preview", + payload: { limit: 25, offset: 50 }, + }); + + expect(captured.id).toBe("abc"); + expect(captured.limit).toBe(25); + expect(captured.offset).toBe(50); +}); + +test("POST /sessions/:id/sql/export returns {path} from injected thtRunner stub", async () => { + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: { + sqlExport: async (_id: string) => ({ path: "/tmp/export.csv" }), + } as any, + }); + + const res = await app.inject({ + method: "POST", + url: "/sessions/s1/sql/export", + payload: {}, + }); + + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ path: "/tmp/export.csv" }); +}); + +test("POST /sessions/:id/sql/preview returns 500 when thtRunner throws", async () => { + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: { + sqlPreview: async () => { throw new Error("tht boom"); }, + } as any, + }); + + const res = await app.inject({ + method: "POST", + url: "/sessions/s1/sql/preview", + payload: {}, + }); + + expect(res.statusCode).toBe(500); + expect(res.json()).toMatchObject({ error: /boom/ }); +}); + +// --------------------------------------------------------------------------- +// Meta routes +// --------------------------------------------------------------------------- + +test("GET /workspaces lists yaml files from ../harness/workspaces", async () => { + // The real ../harness/workspaces directory contains *.yaml files. + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: {} as any, + }); + + const res = await app.inject({ method: "GET", url: "/workspaces" }); + + expect(res.statusCode).toBe(200); + const body = res.json() as { name: string; file: string }[]; + expect(Array.isArray(body)).toBe(true); + // ../harness/workspaces has at least one yaml (tht-test.yaml / tht.example.yaml) + expect(body.length).toBeGreaterThan(0); + for (const w of body) { + expect(typeof w.name).toBe("string"); + expect(w.name).not.toContain(".yaml"); // name strips extension + expect(w.file).toMatch(/\.ya?ml$/); + } +}); + +test("GET /workspaces returns [] when harnessDir has no workspaces subdir", async () => { + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "/nonexistent-harness-dir" }), { + thtRunner: {} as any, + }); + + const res = await app.inject({ method: "GET", url: "/workspaces" }); + + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual([]); +}); + +test("GET /models returns {models:[...]} from injected listModels stub", async () => { + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: {} as any, + listModels: async () => ["claude-opus-4", "claude-sonnet-4-5"], + }); + + const res = await app.inject({ method: "GET", url: "/models" }); + + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ models: ["claude-opus-4", "claude-sonnet-4-5"] }); +}); + +test("GET /models returns {models:[]} when listModels throws (graceful fallback)", async () => { + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: {} as any, + listModels: async () => { throw new Error("Pi not running"); }, + }); + + const res = await app.inject({ method: "GET", url: "/models" }); + + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ models: [] }); +}); + +test("GET /models with no listModels injected falls back to {models:[]}", async () => { + // No listModels dep → defaultListModels → returns [] → {models:[]} + const app = buildApp(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + thtRunner: {} as any, + // listModels intentionally omitted + }); + + const res = await app.inject({ method: "GET", url: "/models" }); + + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ models: [] }); +}); diff --git a/backend/test/tht-runner.test.ts b/backend/test/tht-runner.test.ts index ef28cbf3..8c692f13 100644 --- a/backend/test/tht-runner.test.ts +++ b/backend/test/tht-runner.test.ts @@ -39,3 +39,28 @@ test("sessionNew with missing workspace file falls back to default configPath ar expect(bin).toBe("tht"); expect(argv).toEqual(["-c", "config/tht.yaml", "session", "new", "q", "--json"]); }); + +test("sqlPreview argv has no positional file — uses --session to resolve path", async () => { + // The harness preview_cmd now resolves sql_final.sql from the session workspace; + // the backend must NOT pass a sessions//sql_final.sql positional arg. + (spawn as any).mockClear(); + const r = new ThtRunner({ thtBin: "tht", harnessDir: "/nope", configPath: "config/tht.yaml" }); + // stub json() via run() — just need spawn call captured + r.run = async () => ({ code: 0, stdout: '{"columns":[],"rows":[],"execution_ms":1,"truncated":false}', stderr: "" }); + await r.sqlPreview("ses1", { limit: 10, offset: 5 }); + // Verify via the patched run — we stub run() so spawn isn't called again. + // Instead confirm directly that sqlPreview builds the right args by inspecting run calls. + // We swap back to a spy on run itself. + const runSpy = vi.fn().mockResolvedValue({ + code: 0, + stdout: '{"columns":["c"],"rows":[[1]],"execution_ms":2,"truncated":false}', + stderr: "", + }); + const r2 = new ThtRunner({ thtBin: "tht", harnessDir: "/nope", configPath: "config/tht.yaml" }); + r2.run = runSpy; + await r2.sqlPreview("ses2", { limit: 20, offset: 0 }); + const [calledArgs] = runSpy.mock.calls[0]; + // Must NOT include any positional file path before --session + expect(calledArgs).toEqual(["sql", "preview", "--session", "ses2", "--json", "--limit", "20", "--offset", "0"]); + expect(calledArgs).not.toContain("sessions/ses2/sql_final.sql"); +}); diff --git a/harness/tests/test_sql_preview_json.py b/harness/tests/test_sql_preview_json.py index f26371e8..a19b34c7 100644 --- a/harness/tests/test_sql_preview_json.py +++ b/harness/tests/test_sql_preview_json.py @@ -107,6 +107,44 @@ def test_do_run_offset_zero_path_unchanged(monkeypatch): assert captured["limit"] == 2 +def test_preview_session_no_file_resolves_sql_final(monkeypatch, tmp_path, capsys): + """--session without a positional FILE resolves sql_final.sql via _session_sql_file.""" + from types import SimpleNamespace + + from tht.cli import sql_cmd + + sql_file = tmp_path / "sql_final.sql" + sql_file.write_text("SELECT session_resolved") + + # Patch _session_sql_file to return our tmp file (no real workspace/DB needed). + monkeypatch.setattr(sql_cmd, "_session_sql_file", lambda cfg, sid: sql_file) + + captured_sql = {} + + def fake_do_run(cfg, sql, *, limit, offset=0): + captured_sql["sql"] = sql + return SimpleNamespace(columns=["x"], rows=[[42]], execution_ms=1, truncated=False) + + monkeypatch.setattr(sql_cmd, "do_run", fake_do_run) + monkeypatch.setattr(sql_cmd, "validate_or_exit", lambda *a, **k: SimpleNamespace(ast=None)) + monkeypatch.setattr(sql_cmd, "require_action", lambda *a, **k: None) + monkeypatch.setattr( + sql_cmd, + "_load_config_or_exit", + lambda *a, **k: SimpleNamespace(execution=SimpleNamespace(max_preview_rows=100)), + ) + + # Call with file=None, session="abc123" — should NOT raise. + sql_cmd.preview_cmd(file=None, limit=None, offset=0, session="abc123", json_out=True, config=None) + + out = capsys.readouterr().out.strip() + data = json.loads(out) + assert data["columns"] == ["x"] + assert data["rows"] == [[42]] + # Confirm the SQL came from the file, not a None read. + assert captured_sql["sql"] == "SELECT session_resolved" + + def test_preview_json_pure_stdout(monkeypatch, tmp_path, capsys): from types import SimpleNamespace diff --git a/harness/tht/cli/sql_cmd.py b/harness/tht/cli/sql_cmd.py index 40c5f6b6..ddf74bab 100644 --- a/harness/tht/cli/sql_cmd.py +++ b/harness/tht/cli/sql_cmd.py @@ -183,19 +183,33 @@ def explain_cmd( @sql_app.command("preview") def preview_cmd( - file: Path = typer.Argument(...), + file: Path = typer.Argument(None, help="File SQL da eseguire. Opzionale se --session è dato."), limit: int = typer.Option(None, "--limit", help="Default: execution.max_preview_rows."), offset: int = typer.Option(0, "--offset", help="Riga di partenza (0-based) per il paging."), session: str = typer.Option(None, "--session"), json_out: bool = typer.Option(False, "--json", help="Output JSON puro per il backend (sopprime tabella rich)."), config: Path = CONFIG_OPT, ) -> None: - """Esecuzione controllata con LIMIT iniettato; aggregati mostrati per interi.""" + """Esecuzione controllata con LIMIT iniettato; aggregati mostrati per interi. + + Se FILE è omesso e --session è fornito, il file viene risolto automaticamente + come /sessions//sql_final.sql (tramite _session_sql_file). + """ from tht.execute import ExecutionError cfg = _load_config_or_exit(config) require_action(cfg, "preview") - sql = _read_sql(file) + if file is None: + if session is None: + typer.secho( + "ERRORE: specificare FILE oppure --session.", + fg=typer.colors.RED, err=True, + ) + raise typer.Exit(code=1) + resolved = _session_sql_file(cfg, session) + sql = resolved.read_text() + else: + sql = _read_sql(file) check = validate_or_exit(cfg, sql, session) effective_limit = limit if limit is not None else cfg.execution.max_preview_rows try: