feat(task-10): SQL routes + /workspaces + /models + fix sql preview path bug
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/<id>/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 <noreply@anthropic.com>
This commit is contained in:
@@ -5,10 +5,13 @@ import { PiProcessManager } from "./pi/pi-process-manager.js";
|
|||||||
import { SseHub } from "./sse/sse-hub.js";
|
import { SseHub } from "./sse/sse-hub.js";
|
||||||
import { authPreHandler } from "./auth/auth.js";
|
import { authPreHandler } from "./auth/auth.js";
|
||||||
import { sessionRoutes } from "./routes/sessions.js";
|
import { sessionRoutes } from "./routes/sessions.js";
|
||||||
|
import { sqlRoutes } from "./routes/sql.js";
|
||||||
|
import { metaRoutes, type ListModelsFn } from "./routes/meta.js";
|
||||||
|
|
||||||
export interface BuildAppDeps {
|
export interface BuildAppDeps {
|
||||||
thtRunner?: ThtRunner;
|
thtRunner?: ThtRunner;
|
||||||
spawnFn?: () => any;
|
spawnFn?: () => any;
|
||||||
|
listModels?: ListModelsFn;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstance {
|
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.addHook("preHandler", authPreHandler(config.authMode));
|
||||||
app.get("/health", async () => ({ status: "ok" }));
|
app.get("/health", async () => ({ status: "ok" }));
|
||||||
sessionRoutes(app, { mgr, tht: tht as ThtRunner, hub });
|
sessionRoutes(app, { mgr, tht: tht as ThtRunner, hub });
|
||||||
|
sqlRoutes(app, { tht: tht as ThtRunner });
|
||||||
|
metaRoutes(app, { harnessDir: config.harnessDir, listModels: deps?.listModels });
|
||||||
|
|
||||||
return app;
|
return app;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,59 @@
|
|||||||
|
import { readdirSync } from "node:fs";
|
||||||
|
import { join } from "node:path";
|
||||||
|
import type { FastifyInstance } from "fastify";
|
||||||
|
|
||||||
|
export type ListModelsFn = () => Promise<string[]>;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* List YAML workspace configs found in <harnessDir>/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<string[]> {
|
||||||
|
// Real ephemeral Pi spawn left for a follow-up task.
|
||||||
|
// Returning [] here triggers the graceful fallback seen by clients.
|
||||||
|
return [];
|
||||||
|
}
|
||||||
@@ -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) });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -82,7 +82,9 @@ export class ThtRunner {
|
|||||||
}
|
}
|
||||||
|
|
||||||
sqlPreview(id: string, p: { limit?: number; offset?: number }) {
|
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.limit != null) a.push("--limit", String(p.limit));
|
||||||
if (p.offset != null) a.push("--offset", String(p.offset));
|
if (p.offset != null) a.push("--offset", String(p.offset));
|
||||||
return this.json<{
|
return this.json<{
|
||||||
|
|||||||
@@ -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: [] });
|
||||||
|
});
|
||||||
@@ -39,3 +39,28 @@ test("sessionNew with missing workspace file falls back to default configPath ar
|
|||||||
expect(bin).toBe("tht");
|
expect(bin).toBe("tht");
|
||||||
expect(argv).toEqual(["-c", "config/tht.yaml", "session", "new", "q", "--json"]);
|
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/<id>/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");
|
||||||
|
});
|
||||||
|
|||||||
@@ -107,6 +107,44 @@ def test_do_run_offset_zero_path_unchanged(monkeypatch):
|
|||||||
assert captured["limit"] == 2
|
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):
|
def test_preview_json_pure_stdout(monkeypatch, tmp_path, capsys):
|
||||||
from types import SimpleNamespace
|
from types import SimpleNamespace
|
||||||
|
|
||||||
|
|||||||
@@ -183,19 +183,33 @@ def explain_cmd(
|
|||||||
|
|
||||||
@sql_app.command("preview")
|
@sql_app.command("preview")
|
||||||
def preview_cmd(
|
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."),
|
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."),
|
offset: int = typer.Option(0, "--offset", help="Riga di partenza (0-based) per il paging."),
|
||||||
session: str = typer.Option(None, "--session"),
|
session: str = typer.Option(None, "--session"),
|
||||||
json_out: bool = typer.Option(False, "--json", help="Output JSON puro per il backend (sopprime tabella rich)."),
|
json_out: bool = typer.Option(False, "--json", help="Output JSON puro per il backend (sopprime tabella rich)."),
|
||||||
config: Path = CONFIG_OPT,
|
config: Path = CONFIG_OPT,
|
||||||
) -> None:
|
) -> 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 <workspace>/sessions/<session>/sql_final.sql (tramite _session_sql_file).
|
||||||
|
"""
|
||||||
from tht.execute import ExecutionError
|
from tht.execute import ExecutionError
|
||||||
|
|
||||||
cfg = _load_config_or_exit(config)
|
cfg = _load_config_or_exit(config)
|
||||||
require_action(cfg, "preview")
|
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)
|
check = validate_or_exit(cfg, sql, session)
|
||||||
effective_limit = limit if limit is not None else cfg.execution.max_preview_rows
|
effective_limit = limit if limit is not None else cfg.execution.max_preview_rows
|
||||||
try:
|
try:
|
||||||
|
|||||||
Reference in New Issue
Block a user