Files
oh-my-pi/packages/coding-agent/test/edit-patch-unchanged-error.test.ts
T
oldschoola 96aad47eb6 fix(coding-agent): address PR #1388 review feedback
Refactor:
- session.ts: extract mapDebugpyMissingModule helper; replace the duplicated
  inline check in launch/attach catch blocks. Add jsdoc on DapStartRequestFailure.settled
  documenting per-call ownership and how throwPreferredDapStartError consumes it.
- path-utils.ts: replace the no-op keepOpaqueResourceUri branch with an
  OPAQUE_RESOURCE_SCHEMES Set so the structure carries the intent. Functionally
  equivalent; new opaque schemes become a one-line Set change.

Tests:
- dap-launch-failures: cover the debugpy stderr -> 'pip install debugpy'
  rewrite for launch and attach, plus a negative case (non-debugpy adapter
  with the substring in stderr is left untouched).
- dap-launch-failures: model the delayed-launch-failure case the new
  settled-race in throwPreferredDapStartError defends against. FakeDapClient
  gains optional launchErrorDelayMs/attachErrorDelayMs.
- dap-launch-failures (DebugTool): assert adapter:'debugpy' early-throw
  surfaces 'python not found in PATH' on both launch and attach when
  selectLaunchAdapter/selectAttachAdapter return null, and the unspecified-adapter
  path still falls back to the generic 'No debugger adapter' error.
- find.ts: export validateFindPathInputs and pin the new backslash-escape
  semantics (\, no longer trips the comma-joined heuristic) plus the
  existing brace-expansion and rejection paths.
- patch.ts: cover the post-write verification error message. The user-facing
  ToolError must contain the caller-supplied relative path and not the
  absolute resolvedPath (which still lives in the structured context for
  log correlation).
- split-internal-url-sel: reword two mcp:// test comments that described a
  'peeler refuses' guard that doesn't exist; rename the tests to reflect the
  actual opaque-scheme rule.
2026-05-26 01:54:09 -07:00

113 lines
3.8 KiB
TypeScript

import { afterEach, beforeEach, describe, expect, test } 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 { DEFAULT_FUZZY_THRESHOLD, executePatchSingle } from "@oh-my-pi/pi-coding-agent/edit";
import type { FileDiagnosticsResult } from "@oh-my-pi/pi-coding-agent/lsp";
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
function makeSession(cwd: string): ToolSession {
return {
cwd,
hasUI: false,
getSessionFile: () => null,
getSessionSpawns: () => "*",
enableLsp: false,
settings: Settings.isolated({ "edit.mode": "patch" }),
getArtifactsDir: () => null,
getSessionId: () => null,
getPlanModeState: () => undefined,
} as unknown as ToolSession;
}
const noopBeginDeferred = (_p: string) => ({
onDeferredDiagnostics: () => {},
signal: new AbortController().signal,
finalize: () => {},
});
/**
* Simulates an LSP host integration that claims success without persisting the
* write — the failure mode the post-write verification block in `patch.ts` is
* defending against. Unlike `writethroughNoop`, this really doesn't touch the
* filesystem.
*/
async function silentlySwallowingWritethrough(): Promise<FileDiagnosticsResult | undefined> {
return undefined;
}
let tempDir: string;
beforeEach(async () => {
resetSettingsForTest();
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-patch-unchanged-"));
await Settings.init({ inMemory: true, cwd: tempDir });
});
afterEach(async () => {
resetSettingsForTest();
await fs.rm(tempDir, { recursive: true, force: true });
});
describe("executePatchSingle — post-write verification error path", () => {
test("error message contains the caller-supplied relative path and not the absolute resolvedPath", async () => {
const relPath = "deep/nested/foo.txt";
await fs.mkdir(path.join(tempDir, "deep", "nested"), { recursive: true });
await fs.writeFile(path.join(tempDir, relPath), "a\n");
let caught: Error | undefined;
try {
await executePatchSingle({
session: makeSession(tempDir),
path: relPath,
params: { op: "update", diff: "@@\n-a\n+b" },
allowFuzzy: true,
fuzzyThreshold: DEFAULT_FUZZY_THRESHOLD,
writethrough: silentlySwallowingWritethrough,
beginDeferredDiagnosticsForPath: noopBeginDeferred,
});
} catch (err) {
caught = err as Error;
}
expect(caught).toBeInstanceOf(Error);
const message = caught?.message ?? "";
// The relative path supplied by the caller must appear in the
// user-facing error — it's what the outer composer in `executeSinglePathEntries`
// uses in its `Error editing ${path}: …` wrapper.
expect(message).toContain(relPath);
// The absolute resolved path must NOT appear in the user-facing
// message — leaking it embeds `$HOME`/`os.tmpdir()` in the TUI and
// double-embeds the path when the outer composer prepends its own.
// resolvedPath still lives in the structured `context` metadata.
expect(message).not.toContain(tempDir);
});
test("ToolError still carries the absolute resolvedPath in its structured context for log correlation", async () => {
const relPath = "foo.txt";
await fs.writeFile(path.join(tempDir, relPath), "a\n");
let caught: Error | undefined;
try {
await executePatchSingle({
session: makeSession(tempDir),
path: relPath,
params: { op: "update", diff: "@@\n-a\n+b" },
allowFuzzy: true,
fuzzyThreshold: DEFAULT_FUZZY_THRESHOLD,
writethrough: silentlySwallowingWritethrough,
beginDeferredDiagnosticsForPath: noopBeginDeferred,
});
} catch (err) {
caught = err as Error;
}
expect(caught).toBeInstanceOf(Error);
const context = (caught as Error & { context?: { path?: string } }).context;
expect(context?.path).toBe(path.join(tempDir, relPath));
});
});