Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/core/agent_loop_v2.ail
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,15 @@ func tool_messages_to_result_jsons(msgs: [Message]) -> [Json] {
Ok(j) => j,
Err(_) => jo([])
};
let stdout_str = json_field_str(inner, "stdout", "");
-- WriteFile/EditFile results carry their unified diff under a `diff`
-- key (tool_result_item_to_json), not `stdout`. The TUI's edit-diff
-- renderer (ui.ts parseUnifiedDiff) reads the diff out of `stdout`,
-- so forward `diff` into `stdout` when there's no real stdout. Without
-- this, edit-tool rows show no diff (issue #19: TUI edit diff disabled
-- after the upstream AILANG port).
let stdout_raw = json_field_str(inner, "stdout", "");
let diff_str = json_field_str(inner, "diff", "");
let stdout_str = if stdout_raw == "" then diff_str else stdout_raw;
let stderr_str = json_field_str(inner, "stderr", "");
-- If the payload flags itself as an error (policy_denied_message
-- or delegated_deferred_message both set `error: true`), drive
Expand Down
2 changes: 1 addition & 1 deletion src/tui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"scripts": {
"build": "tsc",
"dev": "tsc --watch",
"test": "bun node_modules/.bin/jest --testPathPattern='src/.*\\.test\\.ts'"
"test": "NODE_OPTIONS=--experimental-vm-modules node node_modules/.bin/jest --testPathPattern='src/.*\\.test\\.ts'"
},
"dependencies": {
"@mariozechner/pi-tui": "^0.64.0",
Expand Down
17 changes: 16 additions & 1 deletion src/tui/src/scratchpad/loopback.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,24 @@
import { afterEach, describe, expect, it } from "@jest/globals";
import { execFileSync } from "child_process";
import * as fs from "fs";
import * as os from "os";
import * as path from "path";
import { startLoopbackServer, type LoopbackServer } from "./loopback.js";

// The search path shells out to ripgrep. Skip the integration test in
// environments where `rg` is not a spawnable binary (e.g. CI images without
// ripgrep installed) so the suite stays green there while still exercising
// the real path wherever ripgrep is available.
function ripgrepAvailable(): boolean {
try {
execFileSync("rg", ["--version"], { stdio: "ignore" });
return true;
} catch {
return false;
}
}
const itWithRg = ripgrepAvailable() ? it : it.skip;

describe("scratchpad loopback server", () => {
let tempDir = "";
let server: LoopbackServer | undefined;
Expand All @@ -15,7 +30,7 @@ describe("scratchpad loopback server", () => {
tempDir = "";
});

it("returns structured JSON for tool.search", async () => {
itWithRg("returns structured JSON for tool.search", async () => {
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "motoko-loopback-"));
fs.mkdirSync(path.join(tempDir, "src"), { recursive: true });
fs.writeFileSync(path.join(tempDir, "src", "sample.ts"), "const x = 'WebSocket';\n", "utf8");
Expand Down
14 changes: 11 additions & 3 deletions src/tui/src/scratchpad/loopback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,20 @@ export async function startLoopbackServer(opts: LoopbackResolverOptions): Promis
});
return result(reqId, 0, parseRipgrepMatches(opts.workdir, pattern, out).slice(0, 50 * 1024));
} catch (e: any) {
const code = typeof e.status === "number" ? e.status : 1;
// rg exits 1 when it ran successfully but found no matches — treat
// that as a success with an empty match set. A spawn failure
// (notably ENOENT when ripgrep isn't installed) has no numeric
// status; surface it as an error instead of silently reporting
// "0 matches", which would mask a missing-ripgrep environment.
const status = typeof e.status === "number" ? e.status : null;
const stdout = String(e.stdout ?? "");
if (status === null) {
return result(reqId, 127, "", `ripgrep (rg) could not be run: ${String(e.code ?? e.message ?? e)}`);
}
return result(
reqId,
code === 1 ? 0 : code,
code === 1 ? parseRipgrepMatches(opts.workdir, pattern, stdout).slice(0, 50 * 1024) : stdout.slice(0, 50 * 1024),
status === 1 ? 0 : status,
status === 1 ? parseRipgrepMatches(opts.workdir, pattern, stdout).slice(0, 50 * 1024) : stdout.slice(0, 50 * 1024),
String(e.stderr ?? "").slice(0, 4000),
);
}
Expand Down
108 changes: 33 additions & 75 deletions src/tui/test/path-guard.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Adversarial smoke for the path-doubling guard added in commit 5130eb7.
// Run: bun test/path-guard.test.ts
//
// The dispatcher's resolvePath should reject:
// 1. Paths that start with cwd-without-leading-slash (the doubling case)
Expand All @@ -8,6 +7,7 @@
// And accept:
// 3. Plain relative paths (the happy path)

import { describe, expect, it } from "@jest/globals";
import path from "node:path";

// Inline copy of resolvePath logic from src/ohMyPi/dispatcher.ts for testing
Expand All @@ -26,80 +26,38 @@ function resolvePath(session: { cwd: string }, p: string): string {
return path.resolve(cwd, p);
}

let pass = 0;
let fail = 0;

function check(name: string, fn: () => void) {
try {
fn();
console.log(` ✓ ${name}`);
pass++;
} catch (e: any) {
console.log(` ✗ ${name}: ${e?.message ?? e}`);
fail++;
}
}

const SESSION = { cwd: "/Users/mark/dev/sunholo/motoko_explore/runs/foo/motoko_agent" };

// 1. The exact regression case: absolute-without-leading-slash mirrors cwd.
check("rejects 'Users/mark/.../foo' (the doubling regression)", () => {
let threw = false;
try {
resolvePath(SESSION, "Users/mark/dev/sunholo/motoko_explore/runs/foo/motoko_agent/src/foo.ail");
} catch (e: any) {
if (!e.message.includes("missing leading slash")) {
throw new Error(`wrong error message: ${e.message}`);
}
threw = true;
}
if (!threw) throw new Error("expected rejection, got silent acceptance");
});

// 2. Plain absolute path — already-existing guard, regression check.
check("rejects '/etc/passwd' (already-absolute)", () => {
let threw = false;
try {
resolvePath(SESSION, "/etc/passwd");
} catch (e: any) {
if (!e.message.includes("absolute paths are not allowed")) {
throw new Error(`wrong error message: ${e.message}`);
}
threw = true;
}
if (!threw) throw new Error("expected rejection, got silent acceptance");
});

// 3. Happy path: a normal relative path resolves under cwd.
check("accepts 'src/foo.ail' (relative — happy path)", () => {
const out = resolvePath(SESSION, "src/foo.ail");
if (out !== `${SESSION.cwd}/src/foo.ail`) {
throw new Error(`unexpected resolution: ${out}`);
}
});

// 4. Happy path: nested relative path resolves correctly.
check("accepts 'src/core/ext/mcp/mcp.ail' (relative, deep)", () => {
const out = resolvePath(SESSION, "src/core/ext/mcp/mcp.ail");
if (out !== `${SESSION.cwd}/src/core/ext/mcp/mcp.ail`) {
throw new Error(`unexpected resolution: ${out}`);
}
});

// 5. Edge case: path with the bare-cwd as a substring but NOT prefix should be allowed.
check("accepts 'src/Users/mark/.../foo' (cwd-substring, not prefix)", () => {
// Path containing "Users/mark/..." but not starting with the bare cwd.
const out = resolvePath(SESSION, "src/Users/mark/file.txt");
if (!out.endsWith("src/Users/mark/file.txt")) {
throw new Error(`unexpected resolution: ${out}`);
}
describe("path-doubling guard (resolvePath)", () => {
// 1. The exact regression case: absolute-without-leading-slash mirrors cwd.
it("rejects 'Users/mark/.../foo' (the doubling regression)", () => {
expect(() =>
resolvePath(SESSION, "Users/mark/dev/sunholo/motoko_explore/runs/foo/motoko_agent/src/foo.ail"),
).toThrow("missing leading slash");
});

// 2. Plain absolute path — already-existing guard, regression check.
it("rejects '/etc/passwd' (already-absolute)", () => {
expect(() => resolvePath(SESSION, "/etc/passwd")).toThrow("absolute paths are not allowed");
});

// 3. Happy path: a normal relative path resolves under cwd.
it("accepts 'src/foo.ail' (relative — happy path)", () => {
expect(resolvePath(SESSION, "src/foo.ail")).toBe(`${SESSION.cwd}/src/foo.ail`);
});

// 4. Happy path: nested relative path resolves correctly.
it("accepts 'src/core/ext/mcp/mcp.ail' (relative, deep)", () => {
expect(resolvePath(SESSION, "src/core/ext/mcp/mcp.ail")).toBe(`${SESSION.cwd}/src/core/ext/mcp/mcp.ail`);
});

// 5. Edge case: path with the bare-cwd as a substring but NOT prefix should be allowed.
it("accepts 'src/Users/mark/.../foo' (cwd-substring, not prefix)", () => {
expect(resolvePath(SESSION, "src/Users/mark/file.txt").endsWith("src/Users/mark/file.txt")).toBe(true);
});

// 6. Edge case: empty string returns empty (current behaviour).
it("returns '' for empty path (no-op short-circuit)", () => {
expect(resolvePath(SESSION, "")).toBe("");
});
});

// 6. Edge case: empty string returns empty (current behaviour).
check("returns '' for empty path (no-op short-circuit)", () => {
const out = resolvePath(SESSION, "");
if (out !== "") throw new Error(`expected '', got '${out}'`);
});

console.log(`\nResult: ${pass} passed, ${fail} failed`);
process.exit(fail === 0 ? 0 : 1);
Loading