Skip to content

Commit 9aa29a9

Browse files
ualtinokalfonso-aft
andcommitted
mason: route agent boolean tool args through coerceBoolean
Sweep strict === true reads on files, callgraph, dryRun, pty, background, and replaceAll in OpenCode and Pi plugins so stringified "true" and 1/1 are honored at the runtime boundary. Add dryRun and replaceAll string regression tests. Co-authored-by: Alfonso <289616620+alfonso-aft@users.noreply.github.com>
1 parent 852d4a4 commit 9aa29a9

12 files changed

Lines changed: 139 additions & 16 deletions

File tree

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Unit tests for AST tool argument shaping.
3+
*/
4+
5+
/// <reference path="../bun-test.d.ts" />
6+
7+
import { describe, expect, test } from "bun:test";
8+
import type { BridgePool } from "@cortexkit/aft-bridge";
9+
import type { ToolContext } from "@opencode-ai/plugin";
10+
import { astTools } from "../tools/ast.js";
11+
import type { PluginContext } from "../types.js";
12+
13+
function createSdkContext(directory: string): ToolContext {
14+
return {
15+
sessionID: "test-session",
16+
messageID: "test-message",
17+
agent: "build",
18+
abort: new AbortController().signal,
19+
directory,
20+
worktree: directory,
21+
metadata: () => {},
22+
} as ToolContext;
23+
}
24+
25+
describe("AST tool adapters", () => {
26+
test('ast_grep_replace treats string dryRun "true" as preview (dry_run true)', async () => {
27+
const calls: Array<{ command: string; params: Record<string, unknown> }> = [];
28+
const bridge = {
29+
send: async (command: string, params: Record<string, unknown> = {}) => {
30+
calls.push({ command, params });
31+
return { success: true, dry_run: true, text: "preview" };
32+
},
33+
};
34+
const pool = { getBridge: () => bridge } as unknown as BridgePool;
35+
const ctx: PluginContext = {
36+
pool,
37+
client: {
38+
lsp: { status: async () => ({ data: [] }) },
39+
find: { symbols: async () => ({ data: [] }) },
40+
},
41+
config: {} as PluginContext["config"],
42+
storageDir: "/tmp/aft-test",
43+
};
44+
const tools = astTools(ctx);
45+
const dir = "/tmp/aft-ast-test";
46+
const sdkCtx = createSdkContext(dir);
47+
48+
await tools.ast_grep_replace.execute(
49+
{
50+
pattern: "foo($A)",
51+
rewrite: "bar($A)",
52+
lang: "javascript",
53+
dryRun: "true" as unknown as boolean,
54+
},
55+
sdkCtx,
56+
);
57+
58+
expect(calls).toHaveLength(1);
59+
expect(calls[0]?.command).toBe("ast_replace");
60+
expect(calls[0]?.params.dry_run).toBe(true);
61+
});
62+
});

packages/opencode-plugin/src/__tests__/hoisted-tools.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,29 @@ describe("Hoisted tool execute handlers", () => {
900900
});
901901
});
902902

903+
test('edit forwards string replaceAll "true" to Rust replace_all', async () => {
904+
tmpDir = await makeTempDir();
905+
sdkCtx = createMockSdkContext(tmpDir);
906+
907+
const { calls, tools } = createMockHoistedHarness(async () => ({
908+
success: true,
909+
replacements: 1,
910+
}));
911+
912+
await tools.edit.execute(
913+
{
914+
filePath: "repeated.ts",
915+
oldString: "oldName",
916+
newString: "newName",
917+
replaceAll: "true" as unknown as boolean,
918+
},
919+
sdkCtx,
920+
);
921+
922+
const applyCall = calls.find((c) => c.command === "edit_match" && c.params.preview !== true);
923+
expect(applyCall?.params.replace_all).toBe(true);
924+
});
925+
903926
/// Diff-payload contract: the plugin requests full before/after from Rust
904927
/// (include_diff_content) for UI metadata, but the AGENT-facing result must
905928
/// strip the file content down to counts only. Echoing before/after into the

packages/opencode-plugin/src/tools/ast.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { tool } from "@opencode-ai/plugin";
88

99
const z = tool.schema;
1010

11+
import { coerceBoolean } from "@cortexkit/aft-bridge";
1112
import type { ToolDefinition } from "@opencode-ai/plugin";
1213
import type { PluginContext } from "../types.js";
1314
import { callBridge, isEmptyParam, optionalInt, resolvePathArg } from "./_shared.js";
@@ -226,7 +227,8 @@ export function astTools(ctx: PluginContext): Record<string, ToolDefinition> {
226227
dryRun: z.boolean().optional().describe("Preview changes without applying (default: false)"),
227228
},
228229
execute: async (args, context): Promise<string> => {
229-
const isDryRun = args.dryRun === true;
230+
// Coerce at the boundary: dryRun "true" must stay preview-only (coerceBoolean).
231+
const isDryRun = coerceBoolean(args.dryRun);
230232
const paths = await resolveAstPaths(ctx, context, args.paths);
231233

232234
const externalDenied = await checkAstPathsPermission(ctx, context, paths);
@@ -269,7 +271,9 @@ export function astTools(ctx: PluginContext): Record<string, ToolDefinition> {
269271
// Use isEmptyParam — see ast_search above for rationale.
270272
if (!isEmptyParam(paths)) params.paths = paths;
271273
if (!isEmptyParam(args.globs)) params.globs = args.globs;
272-
params.dry_run = args.dryRun === true;
274+
// Normalize dryRun at the plugin-to-bridge boundary with coerceBoolean so
275+
// a string "true" still sets dry_run to a real boolean and the replace stays preview-only.
276+
params.dry_run = coerceBoolean(args.dryRun);
273277
const response = await callBridge(ctx, context, "ast_replace", params);
274278

275279
// Error response (e.g. invalid pattern)

packages/opencode-plugin/src/tools/bash.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
appendPipeStripNote,
33
type BridgeRequestOptions,
4+
coerceBoolean,
45
formatForegroundResult,
56
formatSeconds,
67
isTerminalStatus,
@@ -255,11 +256,13 @@ export function createBashTool(
255256
// command still runs to completion, just inline.
256257
const isSubagent = await resolveIsSubagent(ctx.client, context.sessionID, context.directory);
257258
const backgroundDisabled = !bashCfg.background;
258-
const requestedPty = !backgroundDisabled && args.pty === true;
259+
// Coerce at the boundary: stringified pty/background flags (coerceBoolean).
260+
const requestedPty = !backgroundDisabled && coerceBoolean(args.pty);
259261
// pty:true silently implies background:true (Rust bash.rs handles the
260262
// auto-promote). Agents don't need to set both flags. When background is
261263
// disabled, those args are omitted from the schema and defensively ignored.
262-
const requestedBackground = !backgroundDisabled && (args.background === true || requestedPty);
264+
const requestedBackground =
265+
!backgroundDisabled && (coerceBoolean(args.background) || requestedPty);
263266
// ptyRows/ptyCols are silently ignored when pty is false so agents
264267
// that defensively pass them on normal bash calls don't get stuck in
265268
// a retry loop. pty: true silently implies background: true (Rust

packages/opencode-plugin/src/tools/bash_watch.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as fs from "node:fs/promises";
22
import {
33
type BridgeRequestOptions,
4+
coerceBoolean,
45
isBridgeTransportTimeout,
56
isTerminalStatus,
67
sleep,
@@ -77,7 +78,8 @@ export function createBashWatchTool(ctx: PluginContext): ToolDefinition {
7778
},
7879
execute: async (args, context) => {
7980
const taskId = args.taskId as string;
80-
const requestedAsync = args.background === true;
81+
// Coerce at the boundary: stringified background must enable async mode (coerceBoolean).
82+
const requestedAsync = coerceBoolean(args.background);
8183
const waitFor = parseWaitPattern(args.pattern);
8284
const bashCfg = resolveBashConfig(ctx.config);
8385
const isSubagent = await resolveIsSubagent(ctx.client, context.sessionID, context.directory);

packages/opencode-plugin/src/tools/hoisted.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,8 @@ function createEditTool(ctx: PluginContext, writeToolName = "write"): ToolDefini
899899
command = "edit_match";
900900
params.match = args.oldString;
901901
params.replacement = args.newString ?? "";
902-
if (args.replaceAll !== undefined) params.replace_all = args.replaceAll;
902+
// Coerce at the boundary: stringified replaceAll must forward true (coerceBoolean).
903+
if (coerceBoolean(args.replaceAll)) params.replace_all = true;
903904
if (args.occurrence !== undefined) params.occurrence = args.occurrence;
904905
} else {
905906
// No mode-selecting parameter matched. We deliberately do NOT fall

packages/opencode-plugin/src/tools/reading.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
coerceBoolean,
23
formatZoomMultiTargetResult,
34
formatZoomText,
45
unwrapRustZoomBatchEnvelope,
@@ -82,7 +83,8 @@ export function readingTools(ctx: PluginContext): Record<string, ToolDefinition>
8283
},
8384
execute: async (args, context): Promise<string> => {
8485
const target = args.target;
85-
const filesMode = args.files === true;
86+
// Coerce at the boundary: stringified "true" must enable files mode (coerceBoolean).
87+
const filesMode = coerceBoolean(args.files);
8688
const hasUrl =
8789
typeof target === "string" &&
8890
(target.startsWith("http://") || target.startsWith("https://"));
@@ -293,7 +295,8 @@ export function readingTools(ctx: PluginContext): Record<string, ToolDefinition>
293295
const hasUrl = !isEmptyParam(args.url);
294296
const hasTargets = hasTargetsProvided(args.targets);
295297
const hasSymbols = !isEmptyParam(args.symbols);
296-
const wantCallgraph = args.callgraph === true;
298+
// Coerce at the boundary: stringified "true" must request callgraph (coerceBoolean).
299+
const wantCallgraph = coerceBoolean(args.callgraph);
297300

298301
// TUI title + scalar metadata for the tool-call header. OpenCode's UI
299302
// only auto-renders SCALAR args (strings, numbers, booleans) — arrays

packages/pi-plugin/src/__tests__/ast.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,20 @@ describe("AST tool adapters", () => {
7474
dry_run: true,
7575
});
7676
});
77+
78+
test('ast_grep_replace treats string dryRun "true" as preview (dry_run true)', async () => {
79+
const { api, tools } = makeMockApi();
80+
const { bridge, calls } = makeMockBridge(() => ({ success: true, text: "preview" }));
81+
registerAstTools(api, makePluginContext(bridge), { astSearch: false, astReplace: true });
82+
83+
await executeTool(tools.get("ast_grep_replace")!, {
84+
pattern: "foo($A)",
85+
rewrite: "bar($A)",
86+
lang: "javascript",
87+
dryRun: "true" as unknown as boolean,
88+
});
89+
90+
expect(calls[0].command).toBe("ast_replace");
91+
expect(calls[0].params.dry_run).toBe(true);
92+
});
7793
});

packages/pi-plugin/src/tools/ast.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* 8 languages: typescript, tsx, javascript, python, rust, go, pascal, r.
44
*/
55

6+
import { coerceBoolean } from "@cortexkit/aft-bridge";
67
import { StringEnum } from "@earendil-works/pi-ai";
78
import type {
89
AgentToolResult,
@@ -347,8 +348,8 @@ export function registerAstTools(pi: ExtensionAPI, ctx: PluginContext, surface:
347348
// Use isEmptyParam — see ast_search above for rationale.
348349
if (!isEmptyParam(paths)) req.paths = paths;
349350
if (!isEmptyParam(params.globs)) req.globs = params.globs;
350-
// Rust ast_replace defaults to dry_run=true; apply by default to match description.
351-
req.dry_run = params.dryRun === true;
351+
// Coerce at the boundary: dryRun "true" must stay preview-only (coerceBoolean).
352+
req.dry_run = coerceBoolean(params.dryRun);
352353
const response = await callBridge(bridge, "ast_replace", req, extCtx);
353354
return textResult((response.text as string | undefined) ?? JSON.stringify(response));
354355
},

packages/pi-plugin/src/tools/bash.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
appendPipeStripNote,
44
type BinaryBridge,
55
type BridgeRequestOptions,
6+
coerceBoolean,
67
formatForegroundResult,
78
formatSeconds,
89
isBridgeTransportTimeout,
@@ -380,9 +381,10 @@ DO NOT use bash for code search or code exploration. If you are about to run gre
380381
const ptyCols = backgroundDisabled
381382
? undefined
382383
: coerceOptionalInt(params.ptyCols, "ptyCols", 1, 140);
383-
const requestedPty = !backgroundDisabled && params.pty === true;
384+
// Coerce at the boundary: stringified pty/background flags (coerceBoolean).
385+
const requestedPty = !backgroundDisabled && coerceBoolean(params.pty);
384386
const effectiveBackground =
385-
!backgroundDisabled && (params.background === true || requestedPty);
387+
!backgroundDisabled && (coerceBoolean(params.background) || requestedPty);
386388
// Hard-kill timeout sent to the bridge. For an EXPLICIT background task a
387389
// small `timeout` is a legitimate kill cap, so honor it verbatim. For the
388390
// FOREGROUND auto-promote path a `timeout` below the foreground wait
@@ -665,7 +667,8 @@ export function createBashWatchTool(ctx: PluginContext) {
665667
) {
666668
const bridge = bridgeFor(ctx, extCtx.cwd);
667669
const waitFor = parseWaitPattern(params.pattern);
668-
if (params.background === true) {
670+
// Coerce at the boundary: stringified background must enable async mode (coerceBoolean).
671+
if (coerceBoolean(params.background)) {
669672
if (!waitFor) {
670673
throw new Error(
671674
"invalid_request: Use auto-reminder; bash_watch without pattern in async mode is redundant",

0 commit comments

Comments
 (0)