diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e150972d1..9a8d59f21 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -22,6 +22,8 @@ ### Fixed +- Fixed Edit and Write tools failing with `Settings not initialized` in isolated sessions by using each tool session's settings for generated-file guards, with safe schema defaults for standalone guards and inline-image rendering ([#6549](https://github.com/can1357/oh-my-pi/issues/6549)). + - Fixed `todo` calls that omit `op` hard-failing validation ("op must be operation to apply (was missing)"): the tool now validates leniently and infers the op for unambiguous payloads (`list` → `init`, `phase`+`items` → `append`, bare `items` on an empty list → `init`); `op` stays required in the schema, and ambiguous op-less calls surface the schema error as a retryable tool error. - Fixed credential-free web search engines (SearXNG, DuckDuckGo, Google, Startpage, Ecosia, Mojeek, and the Public Web fan-out) returning zero results for queries with `site:` paths (e.g. `site:github.com/owner/repo`) or `inurl:` operators: scraper engines only match `site:` against a bare domain and DuckDuckGo ignores `inurl:` entirely, so such queries silently emptied the result set and fell through to the next provider in the chain. A shared `formatScraperQuery` formatter now structurally demotes path-carrying `site:` and all `inurl:` values to plain search terms (covering OR-grouped and quoted directives) while preserving bare-domain `site:` filters, negated operators, and each engine's supported syntax; the pipeline post-filter still enforces the demoted constraints on returned sources. - Fixed `ast_edit` previews reading like applied edits to the model: the `⟨proposed⟩` badge was TUI-only, so the model-visible result (hashline header + `-`/`+` rows, identical to applied edit output) carried no staged-proposal signal. The preview result now leads with a "Staged as a proposal — files NOT modified yet" notice naming `xd://resolve`/`xd://reject`, the injected resolve reminder names the source tool, and the `ast_edit` tool prompt documents the two-phase flow. diff --git a/packages/coding-agent/src/edit/hashline/filesystem.ts b/packages/coding-agent/src/edit/hashline/filesystem.ts index cfe6f96f5..ca118dd42 100644 --- a/packages/coding-agent/src/edit/hashline/filesystem.ts +++ b/packages/coding-agent/src/edit/hashline/filesystem.ts @@ -121,7 +121,7 @@ export class HashlineFilesystem extends Filesystem { throw error; } // Refuse edits against generated files (lockfiles, models.json, …). - assertEditableFileContent(content, relativePath); + assertEditableFileContent(content, relativePath, this.session.settings); return content; } diff --git a/packages/coding-agent/src/edit/modes/patch.ts b/packages/coding-agent/src/edit/modes/patch.ts index 90a9772e2..68d078421 100644 --- a/packages/coding-agent/src/edit/modes/patch.ts +++ b/packages/coding-agent/src/edit/modes/patch.ts @@ -1816,7 +1816,7 @@ export async function executePatchSingle( const resolvedPath = resolvePlanPath(session, path); const resolvedRename = rename ? resolvePlanPath(session, rename) : undefined; - await assertEditableFile(resolvedPath, path); + await assertEditableFile(resolvedPath, path, session.settings); // Capture pre-edit content so we can verify the write actually hit disk. // `LspFileSystem.writeFile` delegates to a writethrough callback that, in diff --git a/packages/coding-agent/src/session/stream-guards.ts b/packages/coding-agent/src/session/stream-guards.ts index 6f9996d7f..2818b1eed 100644 --- a/packages/coding-agent/src/session/stream-guards.ts +++ b/packages/coding-agent/src/session/stream-guards.ts @@ -185,7 +185,7 @@ export class StreamingEditGuard { #abortForAutoGeneratedPath(toolCall: ToolCall, filePath: string, resolvedPath: string): void { if (this.#lastToolCallId === toolCall.id) return; this.#lastToolCallId = toolCall.id; - void assertEditableFile(resolvedPath, filePath).catch(error => { + void assertEditableFile(resolvedPath, filePath, this.#host.settings).catch(error => { if (!(error instanceof ToolError) || this.#lastToolCallId !== toolCall.id) return; if (!this.#abortTriggered) { this.#abortTriggered = true; diff --git a/packages/coding-agent/src/tools/auto-generated-guard.ts b/packages/coding-agent/src/tools/auto-generated-guard.ts index 807f2197c..ced9f800c 100644 --- a/packages/coding-agent/src/tools/auto-generated-guard.ts +++ b/packages/coding-agent/src/tools/auto-generated-guard.ts @@ -7,7 +7,8 @@ import * as path from "node:path"; import { isEnoent, peekFile } from "@oh-my-pi/pi-utils"; import { LRUCache } from "lru-cache/raw"; -import { settings } from "../config/settings"; +import { settings as globalSettings, isSettingsInitialized, type Settings } from "../config/settings"; +import { getDefault } from "../config/settings-schema"; import { ToolError } from "./tool-errors"; /** @@ -283,15 +284,26 @@ async function getAutoGeneratedMarker(filePath: string): Promise { + if (!shouldBlockAutoGeneratedFiles(activeSettings)) { return; } const pathForDisplay = displayPath ?? absolutePath; @@ -308,9 +320,10 @@ export async function assertEditableFile(absolutePath: string, displayPath?: str * * @param content - File content to check (can be full content or prefix) * @param displayPath - Path to show in error messages + * @param activeSettings - Session settings; falls back to global settings or the schema default */ -export function assertEditableFileContent(content: string, displayPath: string): void { - if (!settings.get("edit.blockAutoGenerated")) { +export function assertEditableFileContent(content: string, displayPath: string, activeSettings?: Settings): void { + if (!shouldBlockAutoGeneratedFiles(activeSettings)) { return; } diff --git a/packages/coding-agent/src/tools/render-utils.ts b/packages/coding-agent/src/tools/render-utils.ts index 9a6e857af..1a59f2764 100644 --- a/packages/coding-agent/src/tools/render-utils.ts +++ b/packages/coding-agent/src/tools/render-utils.ts @@ -13,7 +13,8 @@ import type { Component } from "@oh-my-pi/pi-tui"; import { getKeybindings, replaceTabs, truncateToWidth } from "@oh-my-pi/pi-tui"; import { pluralize } from "@oh-my-pi/pi-utils"; import { formatKeyHints, type KeyId } from "../config/keybindings"; -import { settings } from "../config/settings"; +import { isSettingsInitialized, settings } from "../config/settings"; +import { getDefault } from "../config/settings-schema"; import type { Theme } from "../modes/theme/theme"; import { Hasher } from "../tui/utils"; import { formatDimensionNote, type ResizedImage } from "../utils/image-resize"; @@ -27,8 +28,12 @@ export { replaceTabs, truncateToWidth, wrapTextWithAnsi } from "@oh-my-pi/pi-tui /** Resolve inline image dimension caps from settings and viewport. */ export function resolveImageOptions(): { maxWidthCells: number; maxHeightCells?: number } { - const maxWidthCells = settings.get("tui.maxInlineImageColumns"); - const rowSetting = Math.max(0, settings.get("tui.maxInlineImageRows")); + const activeSettings = isSettingsInitialized() ? settings : undefined; + const maxWidthCells = activeSettings?.get("tui.maxInlineImageColumns") ?? getDefault("tui.maxInlineImageColumns"); + const rowSetting = Math.max( + 0, + activeSettings?.get("tui.maxInlineImageRows") ?? getDefault("tui.maxInlineImageRows"), + ); const viewportRows = process.stdout.rows; const viewportFraction = viewportRows ? Math.floor(viewportRows * 0.6) : 0; let maxHeightCells: number | undefined; diff --git a/packages/coding-agent/src/tools/write.ts b/packages/coding-agent/src/tools/write.ts index cd0a7068a..639fc87cf 100644 --- a/packages/coding-agent/src/tools/write.ts +++ b/packages/coding-agent/src/tools/write.ts @@ -1215,7 +1215,7 @@ export class WriteTool implements AgentTool): string { .join("\n"); } -beforeAll(async () => { - // The edit path's auto-generated-file guard reads the global Settings proxy. - resetSettingsForTest(); - await Settings.init({ inMemory: true, cwd: process.cwd() }); -}); - function createSession(cwd: string, approvedPlan?: { artifactsDir: string; planFilePath: string }): ToolSession { const settings = Settings.isolated(); settings.set("read.summarize.enabled", false); diff --git a/packages/coding-agent/test/streaming-edit-abort.test.ts b/packages/coding-agent/test/streaming-edit-abort.test.ts index dac4fc0df..0a8c27a9c 100644 --- a/packages/coding-agent/test/streaming-edit-abort.test.ts +++ b/packages/coding-agent/test/streaming-edit-abort.test.ts @@ -500,7 +500,7 @@ it("aborts auto-generated file edits as soon as the path is available", async () waitBeforeFirstDelta.resolve(); await promptPromise; - expect(checkSpy).toHaveBeenCalledWith(generatedPath, "generated.ts"); + expect(checkSpy).toHaveBeenCalledWith(generatedPath, "generated.ts", session.settings); expect(abortSpy).toHaveBeenCalled(); expect(abortSignalRef.current?.aborted ?? false).toBe(true); const lastAssistant = lastAssistantMessage(session.state.messages); diff --git a/packages/coding-agent/test/tools/auto-generated-guard.test.ts b/packages/coding-agent/test/tools/auto-generated-guard.test.ts index 0360a8ab1..5e3406fc2 100644 --- a/packages/coding-agent/test/tools/auto-generated-guard.test.ts +++ b/packages/coding-agent/test/tools/auto-generated-guard.test.ts @@ -1,49 +1,75 @@ -import { beforeAll, describe, expect, it } from "bun:test"; +import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { assertEditableFile, assertEditableFileContent } from "@oh-my-pi/pi-coding-agent/tools/auto-generated-guard"; +import { resolveImageOptions } from "@oh-my-pi/pi-coding-agent/tools/render-utils"; import { ToolError } from "@oh-my-pi/pi-coding-agent/tools/tool-errors"; let tempDir: string; +let testSettings: Settings; +const GENERATED_TYPESCRIPT = "// Code generated by sqlc. DO NOT EDIT.\n\nexport const foo = 1;"; beforeAll(async () => { resetSettingsForTest(); tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "auto-gen-guard-")); - await Settings.init({ inMemory: true, cwd: tempDir }); + testSettings = Settings.isolated(); +}); + +afterAll(async () => { + resetSettingsForTest(); + await fs.rm(tempDir, { recursive: true, force: true }); }); describe("assertEditableFileContent", () => { it("detects canonical TypeScript generated header", () => { - const content = "// Code generated by sqlc. DO NOT EDIT.\n\nexport const foo = 1;"; - expect(() => assertEditableFileContent(content, "test.ts")).toThrow(ToolError); + expect(() => assertEditableFileContent(GENERATED_TYPESCRIPT, "test.ts", testSettings)).toThrow(ToolError); }); it("detects @generated marker", () => { const content = "// @generated\n\nexport const foo = 1;"; - expect(() => assertEditableFileContent(content, "test.ts")).toThrow(ToolError); + expect(() => assertEditableFileContent(content, "test.ts", testSettings)).toThrow(ToolError); }); it("detects generated-by marker for Python files", () => { const content = "# Generated by buf\n\nvalue = 1"; - expect(() => assertEditableFileContent(content, "test.py")).toThrow(ToolError); + expect(() => assertEditableFileContent(content, "test.py", testSettings)).toThrow(ToolError); }); it("detects generated-by marker for SQL files", () => { const content = "-- generated by sqlc\n\nselect 1;"; - expect(() => assertEditableFileContent(content, "query.sql")).toThrow(ToolError); + expect(() => assertEditableFileContent(content, "query.sql", testSettings)).toThrow(ToolError); }); it("detects generated markers in leading block comments", () => { const content = "/*\n * Code generated by mockery. DO NOT EDIT.\n */\nexport const foo = 1;"; - expect(() => assertEditableFileContent(content, "test.ts")).toThrow(ToolError); + expect(() => assertEditableFileContent(content, "test.ts", testSettings)).toThrow(ToolError); }); it("detects kysely-codegen generated header", () => { const content = "/**\n * This file was generated by kysely-codegen.\n * Please do not edit it manually.\n */\n\nexport interface Database {}"; - expect(() => assertEditableFileContent(content, "db.ts")).toThrow(ToolError); + expect(() => assertEditableFileContent(content, "db.ts", testSettings)).toThrow(ToolError); + }); + + it("defaults to blocking when global settings are uninitialized", () => { + resetSettingsForTest(); + expect(() => assertEditableFileContent(GENERATED_TYPESCRIPT, "test.ts")).toThrow(ToolError); + }); + + it("honors isolated session settings without global initialization", () => { + const permissiveSettings = Settings.isolated({ "edit.blockAutoGenerated": false }); + expect(() => assertEditableFileContent(GENERATED_TYPESCRIPT, "test.ts", permissiveSettings)).not.toThrow(); + }); +}); + +describe("resolveImageOptions", () => { + it("uses schema defaults when global settings are uninitialized", () => { + resetSettingsForTest(); + const options = resolveImageOptions(); + expect(options.maxWidthCells).toBe(100); + expect(options.maxHeightCells).toBeGreaterThan(0); }); }); @@ -51,17 +77,17 @@ describe("assertEditableFile", () => { it("detects content marker from file prefix", async () => { const filePath = path.join(tempDir, "service.ts"); await Bun.write(filePath, "// Code generated by sqlc. DO NOT EDIT.\nexport const foo = 1;"); - await expect(assertEditableFile(filePath)).rejects.toBeInstanceOf(ToolError); + await expect(assertEditableFile(filePath, undefined, testSettings)).rejects.toBeInstanceOf(ToolError); }); it("allows normal files", async () => { const filePath = path.join(tempDir, "normal.ts"); await Bun.write(filePath, "// Regular source file\nexport const foo = 1;"); - await expect(assertEditableFile(filePath)).resolves.toBeUndefined(); + await expect(assertEditableFile(filePath, undefined, testSettings)).resolves.toBeUndefined(); }); it("handles missing files gracefully", async () => { const filePath = path.join(tempDir, "does-not-exist.ts"); - await expect(assertEditableFile(filePath)).resolves.toBeUndefined(); + await expect(assertEditableFile(filePath, undefined, testSettings)).resolves.toBeUndefined(); }); }); diff --git a/packages/coding-agent/test/write-read-selector-misfire.test.ts b/packages/coding-agent/test/write-read-selector-misfire.test.ts index 8832c0487..917ba7482 100644 --- a/packages/coding-agent/test/write-read-selector-misfire.test.ts +++ b/packages/coding-agent/test/write-read-selector-misfire.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, describe, expect, it } from "bun:test"; +import { describe, expect, it } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -32,12 +32,6 @@ async function makeWorkspace(): Promise { } describe("write refuses read-selector misfires", () => { - beforeAll(async () => { - // assertEditableFile (auto-generated guard) reads the global settings proxy - // when overwriting an existing file. - await Settings.init({ inMemory: true }); - }); - it("fails closed on a missing selector-suffixed target with empty content and points at read()", async () => { const dir = await makeWorkspace(); const write = new WriteTool(session(dir));