fix(backend): make data root config authoritative
This commit is contained in:
@@ -35,6 +35,7 @@ export function createPiModelLister(cfg: AppConfig, opts: Opts = {}): () => Prom
|
|||||||
if (cache && now() - cache.at < ttlMs) return cache.models;
|
if (cache && now() - cache.at < ttlMs) return cache.models;
|
||||||
|
|
||||||
const env: NodeJS.ProcessEnv = { ...process.env };
|
const env: NodeJS.ProcessEnv = { ...process.env };
|
||||||
|
delete env.THT_DATA_ROOT;
|
||||||
if (cfg.dataRoot !== undefined) env.THT_DATA_ROOT = cfg.dataRoot;
|
if (cfg.dataRoot !== undefined) env.THT_DATA_ROOT = cfg.dataRoot;
|
||||||
const child = spawnFn(cfg.piBin, ["--mode", "rpc"], { cwd: cfg.harnessDir, env });
|
const child = spawnFn(cfg.piBin, ["--mode", "rpc"], { cwd: cfg.harnessDir, env });
|
||||||
child.stderr.resume();
|
child.stderr.resume();
|
||||||
|
|||||||
@@ -35,6 +35,7 @@ export class PiProcessManager {
|
|||||||
THT_SESSION: sessionId,
|
THT_SESSION: sessionId,
|
||||||
THT_AUTHOR: author,
|
THT_AUTHOR: author,
|
||||||
};
|
};
|
||||||
|
delete env.THT_DATA_ROOT;
|
||||||
if (this.cfg.dataRoot !== undefined) env.THT_DATA_ROOT = this.cfg.dataRoot;
|
if (this.cfg.dataRoot !== undefined) env.THT_DATA_ROOT = this.cfg.dataRoot;
|
||||||
// pi 0.73 removed `--approve`: rpc mode is headless and its argv is intentionally minimal.
|
// pi 0.73 removed `--approve`: rpc mode is headless and its argv is intentionally minimal.
|
||||||
const child = spawnFn(this.cfg.piBin, ["--mode", "rpc"], {
|
const child = spawnFn(this.cfg.piBin, ["--mode", "rpc"], {
|
||||||
|
|||||||
@@ -63,6 +63,7 @@ export class ThtRunner {
|
|||||||
run(args: string[], workspace?: string): Promise<{ code: number; stdout: string; stderr: string }> {
|
run(args: string[], workspace?: string): Promise<{ code: number; stdout: string; stderr: string }> {
|
||||||
return new Promise((resolve) => {
|
return new Promise((resolve) => {
|
||||||
const env: NodeJS.ProcessEnv = { ...process.env };
|
const env: NodeJS.ProcessEnv = { ...process.env };
|
||||||
|
delete env.THT_DATA_ROOT;
|
||||||
if (this.cfg.dataRoot !== undefined) env.THT_DATA_ROOT = this.cfg.dataRoot;
|
if (this.cfg.dataRoot !== undefined) env.THT_DATA_ROOT = this.cfg.dataRoot;
|
||||||
const ch = spawn(this.cfg.thtBin, this.buildArgv(args, workspace), {
|
const ch = spawn(this.cfg.thtBin, this.buildArgv(args, workspace), {
|
||||||
cwd: this.cfg.harnessDir,
|
cwd: this.cfg.harnessDir,
|
||||||
|
|||||||
@@ -81,3 +81,29 @@ test("production model-list spawn preserves PATH and passes the portable data ro
|
|||||||
rmSync(path.dirname(script), { recursive: true, force: true });
|
rmSync(path.dirname(script), { recursive: true, force: true });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("model-list spawn omits ambient THT_DATA_ROOT when config does not provide one", async () => {
|
||||||
|
const script = scriptWith([]);
|
||||||
|
const calls: any[][] = [];
|
||||||
|
const previousDataRoot = process.env.THT_DATA_ROOT;
|
||||||
|
const previousCredential = process.env.PI_PROVIDER_API_KEY;
|
||||||
|
process.env.THT_DATA_ROOT = "/ambient-must-not-leak";
|
||||||
|
process.env.PI_PROVIDER_API_KEY = "still-inherited";
|
||||||
|
try {
|
||||||
|
const lister = createPiModelLister(loadConfig({ PI_BIN: "/usr/local/bin/pi" }), {
|
||||||
|
spawnFn: (...args: any[]) => {
|
||||||
|
calls.push(args);
|
||||||
|
return spawn("node", [FAKE, script]) as any;
|
||||||
|
},
|
||||||
|
});
|
||||||
|
await lister();
|
||||||
|
expect(calls[0][2].env).not.toHaveProperty("THT_DATA_ROOT");
|
||||||
|
expect(calls[0][2].env.PI_PROVIDER_API_KEY).toBe("still-inherited");
|
||||||
|
} finally {
|
||||||
|
if (previousDataRoot === undefined) delete process.env.THT_DATA_ROOT;
|
||||||
|
else process.env.THT_DATA_ROOT = previousDataRoot;
|
||||||
|
if (previousCredential === undefined) delete process.env.PI_PROVIDER_API_KEY;
|
||||||
|
else process.env.PI_PROVIDER_API_KEY = previousCredential;
|
||||||
|
rmSync(path.dirname(script), { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
@@ -160,3 +160,23 @@ test("production spawn uses explicit Pi path and passes portable data root witho
|
|||||||
vi.unstubAllEnvs();
|
vi.unstubAllEnvs();
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("session Pi spawn omits ambient THT_DATA_ROOT when config does not provide one", async () => {
|
||||||
|
vi.stubEnv("THT_DATA_ROOT", "/ambient-must-not-leak");
|
||||||
|
vi.stubEnv("PI_PROVIDER_API_KEY", "still-inherited");
|
||||||
|
const calls: any[][] = [];
|
||||||
|
const child = recordingChild();
|
||||||
|
child.stderr.resume = () => {};
|
||||||
|
const mgr = new PiProcessManager(loadConfig({ PI_BIN: "/usr/local/bin/pi" }), {
|
||||||
|
spawnFn: (...args: any[]) => { calls.push(args); return child as any; },
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
await mgr.spawnFor("no-data-root", {});
|
||||||
|
expect(calls[0][2].env).not.toHaveProperty("THT_DATA_ROOT");
|
||||||
|
expect(calls[0][2].env.PI_PROVIDER_API_KEY).toBe("still-inherited");
|
||||||
|
} finally {
|
||||||
|
mgr.teardown("no-data-root");
|
||||||
|
vi.unstubAllEnvs();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
@@ -24,6 +24,54 @@ test("sessionNew parses id from JSON", async () => {
|
|||||||
expect(await r.sessionNew({ question: "q" })).toEqual({ id: "2026-06-27-100000-x" });
|
expect(await r.sessionNew({ question: "q" })).toEqual({ id: "2026-06-27-100000-x" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("run passes configured THT_DATA_ROOT and preserves the remaining environment", async () => {
|
||||||
|
const previousDataRoot = process.env.THT_DATA_ROOT;
|
||||||
|
const previousCa = process.env.NODE_EXTRA_CA_CERTS;
|
||||||
|
process.env.THT_DATA_ROOT = "/ambient";
|
||||||
|
process.env.NODE_EXTRA_CA_CERTS = "/certs/company-ca.pem";
|
||||||
|
try {
|
||||||
|
(spawn as any).mockClear();
|
||||||
|
const r = new ThtRunner({
|
||||||
|
thtBin: "/opt/venv/bin/tht",
|
||||||
|
harnessDir: "/app/harness",
|
||||||
|
configPath: "config/tht.yaml",
|
||||||
|
dataRoot: "/configured",
|
||||||
|
});
|
||||||
|
await r.run(["session", "list", "--json"]);
|
||||||
|
const [bin, , options] = (spawn as any).mock.calls[0];
|
||||||
|
expect(bin).toBe("/opt/venv/bin/tht");
|
||||||
|
expect(options.env).toMatchObject({
|
||||||
|
THT_DATA_ROOT: "/configured",
|
||||||
|
NODE_EXTRA_CA_CERTS: "/certs/company-ca.pem",
|
||||||
|
});
|
||||||
|
} finally {
|
||||||
|
if (previousDataRoot === undefined) delete process.env.THT_DATA_ROOT;
|
||||||
|
else process.env.THT_DATA_ROOT = previousDataRoot;
|
||||||
|
if (previousCa === undefined) delete process.env.NODE_EXTRA_CA_CERTS;
|
||||||
|
else process.env.NODE_EXTRA_CA_CERTS = previousCa;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test("run omits ambient THT_DATA_ROOT when config does not provide one", async () => {
|
||||||
|
const previousDataRoot = process.env.THT_DATA_ROOT;
|
||||||
|
const previousCredential = process.env.PI_PROVIDER_API_KEY;
|
||||||
|
process.env.THT_DATA_ROOT = "/ambient-must-not-leak";
|
||||||
|
process.env.PI_PROVIDER_API_KEY = "still-inherited";
|
||||||
|
try {
|
||||||
|
(spawn as any).mockClear();
|
||||||
|
const r = new ThtRunner({ thtBin: "tht", harnessDir: "/h", configPath: "config/tht.yaml" });
|
||||||
|
await r.run(["session", "list", "--json"]);
|
||||||
|
const options = (spawn as any).mock.calls[0][2];
|
||||||
|
expect(options.env).not.toHaveProperty("THT_DATA_ROOT");
|
||||||
|
expect(options.env.PI_PROVIDER_API_KEY).toBe("still-inherited");
|
||||||
|
} finally {
|
||||||
|
if (previousDataRoot === undefined) delete process.env.THT_DATA_ROOT;
|
||||||
|
else process.env.THT_DATA_ROOT = previousDataRoot;
|
||||||
|
if (previousCredential === undefined) delete process.env.PI_PROVIDER_API_KEY;
|
||||||
|
else process.env.PI_PROVIDER_API_KEY = previousCredential;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
test("run with exit != 0 propagates error with stderr", async () => {
|
test("run with exit != 0 propagates error with stderr", async () => {
|
||||||
const r = new ThtRunner({ thtBin: "tht", harnessDir: "/h", configPath: "config/tht.yaml" });
|
const r = new ThtRunner({ thtBin: "tht", harnessDir: "/h", configPath: "config/tht.yaml" });
|
||||||
r.run = async () => ({ code: 1, stdout: "", stderr: "ERRORE: boom" });
|
r.run = async () => ({ code: 1, stdout: "", stderr: "ERRORE: boom" });
|
||||||
|
|||||||
Reference in New Issue
Block a user