fix(coding-agent): honored boolean env flag overrides over settings

- Made PI_INTENT_TRACING, PI_AUTO_QA, PI_PY, and PI_JS take precedence when set. - Fell back to config when the env flag is unset instead of ORing. - Surfaced PI_PY=0/PI_JS=0 in the disabled-backend error messages.
2026-06-06 16:30:09 +02:00
parent 7ffafae1cc
commit 6efe86c07b
7 changed files with 90 additions and 25 deletions
@@ -2,6 +2,10 @@

 ## [Unreleased]

+### Fixed
+
+- Fixed boolean environment flag overrides that were ORed with settings, so `PI_INTENT_TRACING=0`, `PI_AUTO_QA=0`, and per-backend eval flags now take precedence when present while falling back to config when unset.
+
 ## [15.9.67] - 2026-06-06
 ### Added

@@ -1723,7 +1723,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}

 		const repeatToolDescriptions = settings.get("repeatToolDescriptions");
 		const eagerTasks = settings.get("task.eager");
-		const intentField = settings.get("tools.intentTracing") || $flag("PI_INTENT_TRACING") ? INTENT_FIELD : undefined;
+		const intentField = $flag("PI_INTENT_TRACING", settings.get("tools.intentTracing")) ? INTENT_FIELD : undefined;
 		const rebuildSystemPrompt = async (
 			toolNames: string[],
 			tools: Map<string, AgentTool>,
@@ -1,4 +1,4 @@
-import { $env, $flag } from "@oh-my-pi/pi-utils";
+import { $flag } from "@oh-my-pi/pi-utils";
 import type { ToolSession } from ".";

 export interface EvalBackendsAllowance {
@@ -6,21 +6,6 @@ export interface EvalBackendsAllowance {
 	js: boolean;
 }

-/**
- * Parse PI_PY / PI_JS environment variables. Each is a boolean flag; unset
- * means "not specified, defer to settings". Returns null when neither is set
- * so the caller can fall through to `readEvalBackendsAllowance` per key.
- */
-function getEvalBackendsFromEnv(): EvalBackendsAllowance | null {
-	const pyEnv = $env.PI_PY;
-	const jsEnv = $env.PI_JS;
-	if (pyEnv === undefined && jsEnv === undefined) return null;
-	return {
-		python: pyEnv === undefined ? true : $flag("PI_PY"),
-		js: jsEnv === undefined ? true : $flag("PI_JS"),
-	};
-}
-
 /** Read per-backend allowance from settings (defaults true). */
 export function readEvalBackendsAllowance(session: ToolSession): EvalBackendsAllowance {
 	return {
@@ -34,5 +19,9 @@ export function readEvalBackendsAllowance(session: ToolSession): EvalBackendsAll
 * override the per-key settings; otherwise settings (defaults true) win.
 */
 export function resolveEvalBackends(session: ToolSession): EvalBackendsAllowance {
-	return getEvalBackendsFromEnv() ?? readEvalBackendsAllowance(session);
+	const settings = readEvalBackendsAllowance(session);
+	return {
+		python: $flag("PI_PY", settings.python),
+		js: $flag("PI_JS", settings.js),
+	};
 }
@@ -130,11 +130,12 @@ function timeoutSecondsFromMs(timeoutMs: number): number {
 }

 async function resolveBackend(session: ToolSession, language: EvalLanguage): Promise<ResolvedBackend> {
-	const allowPy = (session.settings.get("eval.py") as boolean | undefined) ?? true;
-	const allowJs = (session.settings.get("eval.js") as boolean | undefined) ?? true;
+	const backends = resolveEvalBackends(session);
+	const allowPy = backends.python;
+	const allowJs = backends.js;

 	if (language === "python") {
-		if (!allowPy) throw new ToolError("Python backend is disabled (eval.py = false).");
+		if (!allowPy) throw new ToolError("Python backend is disabled (PI_PY=0 or eval.py = false).");
 		if (!(await pythonBackend.isAvailable(session))) {
 			throw new ToolError(
 				'Python backend is unavailable in this session. Pass language: "js" or install the python kernel.',
@@ -142,7 +143,7 @@ async function resolveBackend(session: ToolSession, language: EvalLanguage): Pro
 		}
 		return { backend: pythonBackend };
 	}
-	if (!allowJs) throw new ToolError("JavaScript backend is disabled (eval.js = false).");
+	if (!allowJs) throw new ToolError("JavaScript backend is disabled (PI_JS=0 or eval.js = false).");
 	return { backend: jsBackend };
 }

@@ -41,7 +41,7 @@ function buildReportToolIssueParams(activeBuiltinNames: readonly string[]) {
 }

 export function isAutoQaEnabled(settings?: Settings): boolean {
-	return $flag("PI_AUTO_QA") || !!settings?.get("dev.autoqa");
+	return $flag("PI_AUTO_QA", !!settings?.get("dev.autoqa"));
 }

 // ───────────────────────────────────────────────────────────────────────────
@@ -1,10 +1,21 @@
-import { afterEach, describe, expect, it, vi } from "bun:test";
+import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
 import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
 import * as evalIndex from "@oh-my-pi/pi-coding-agent/eval";
 import * as pyKernel from "@oh-my-pi/pi-coding-agent/eval/py/kernel";
 import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
 import { EvalTool } from "@oh-my-pi/pi-coding-agent/tools/eval";
+import { resolveEvalBackends } from "@oh-my-pi/pi-coding-agent/tools/eval-backends";

+let originalPiPy: string | undefined;
+let originalPiJs: string | undefined;
+
+function restoreEnv(name: "PI_PY" | "PI_JS", value: string | undefined): void {
+	if (value === undefined) {
+		delete Bun.env[name];
+		return;
+	}
+	Bun.env[name] = value;
+}
 function makeSession(settings = Settings.isolated()): ToolSession {
 	return {
 		cwd: "/tmp/eval-test",
@@ -29,8 +40,17 @@ const mockResult = {
 };

 describe("EvalTool language dispatch", () => {
+	beforeEach(() => {
+		originalPiPy = Bun.env.PI_PY;
+		originalPiJs = Bun.env.PI_JS;
+		delete Bun.env.PI_PY;
+		delete Bun.env.PI_JS;
+	});
+
 	afterEach(() => {
 		vi.restoreAllMocks();
+		restoreEnv("PI_PY", originalPiPy);
+		restoreEnv("PI_JS", originalPiJs);
 	});

 	it('dispatches to the JS backend when cell.language === "js"', async () => {
@@ -100,4 +120,26 @@ describe("EvalTool language dispatch", () => {
 			}),
 		).rejects.toThrow(/eval\.js = false/);
 	});
+
+	it("uses settings for eval backends whose env flag is unset", () => {
+		Bun.env.PI_PY = "1";
+		const settings = Settings.isolated();
+		settings.set("eval.py", false);
+		settings.set("eval.js", false);
+
+		expect(resolveEvalBackends(makeSession(settings))).toEqual({ python: true, js: false });
+	});
+
+	it("lets PI_JS disable js execution even when eval.js is enabled", async () => {
+		Bun.env.PI_JS = "0";
+		const settings = Settings.isolated();
+		settings.set("eval.js", true);
+		const tool = new EvalTool(makeSession(settings));
+
+		await expect(
+			tool.execute("call-js-env-disabled", {
+				cells: [{ language: "js", code: "const x = 1;" }],
+			}),
+		).rejects.toThrow(/PI_JS=0/);
+	});
 });
@@ -1,7 +1,11 @@
 import { Database } from "bun:sqlite";
 import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
 import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
-import { __resetAutoQaFlushStateForTests, flushGrievances } from "@oh-my-pi/pi-coding-agent/tools/report-tool-issue";
+import {
+	__resetAutoQaFlushStateForTests,
+	flushGrievances,
+	isAutoQaEnabled,
+} from "@oh-my-pi/pi-coding-agent/tools/report-tool-issue";
 import * as piUtils from "@oh-my-pi/pi-utils";
 import { hookFetch } from "@oh-my-pi/pi-utils";

@@ -57,11 +61,23 @@ function pushSettings(overrides: Record<string, unknown> = {}): Settings {
 	});
 }

+let originalPiAutoQa: string | undefined;
+
+function restoreAutoQaEnv(): void {
+	if (originalPiAutoQa === undefined) {
+		delete Bun.env.PI_AUTO_QA;
+		return;
+	}
+	Bun.env.PI_AUTO_QA = originalPiAutoQa;
+}
+
 describe("flushGrievances", () => {
 	let db: Database;

 	beforeEach(() => {
 		__resetAutoQaFlushStateForTests();
+		originalPiAutoQa = Bun.env.PI_AUTO_QA;
+		delete Bun.env.PI_AUTO_QA;
 		vi.spyOn(piUtils, "getInstallId").mockReturnValue("11111111-2222-3333-4444-555555555555");
 		db = openTempDb();
 	});
@@ -69,9 +85,22 @@ describe("flushGrievances", () => {
 	afterEach(() => {
 		vi.restoreAllMocks();
 		__resetAutoQaFlushStateForTests();
+		restoreAutoQaEnv();
 		db.close();
 	});

+	it("lets PI_AUTO_QA=false disable auto QA when the setting is enabled", () => {
+		Bun.env.PI_AUTO_QA = "0";
+
+		expect(isAutoQaEnabled(Settings.isolated({ "dev.autoqa": true }))).toBe(false);
+	});
+
+	it("lets PI_AUTO_QA=true enable auto QA when the setting is disabled", () => {
+		Bun.env.PI_AUTO_QA = "1";
+
+		expect(isAutoQaEnabled(Settings.isolated({ "dev.autoqa": false }))).toBe(true);
+	});
+
 	it("skips network when consent is missing and leaves rows intact", async () => {
 		insertGrievance(db, "find", "weird ordering");
 		const fetchSpy = vi.fn(() => new Response("unexpected", { status: 200 }));