Skip to content

Commit 852d4a4

Browse files
ualtinokalfonso-aft
andcommitted
fix(delete): coerce recursive at the boundary so a stringified "true" is honored
aft_delete rejected a directory ("is a directory. Pass recursive: true.") even when the agent passed recursive: true, reproduced across multiple agents. Root cause: the plugin read the flag with a strict `args.recursive === true`, but hosts deliver a boolean-typed arg as the model's raw emitted value despite the declared schema — and models non-deterministically emit `true` as the string "true" (the same host behavior `coerceStringArray` already absorbs for `files`, documented in coerce.ts). A stringified "true" then read as false and the flag was silently dropped, so Rust correctly refused the directory. Adds a shared `coerceBoolean` to @cortexkit/aft-bridge (true/"true"/1/"1" → true; everything else → false, a deliberately tight truthy set so a destructive gate can't be flipped by an arbitrary truthy value) and uses it for `recursive` in both the OpenCode and Pi delete paths. Real booleans pass through unchanged (hoisted-tools 62/0, fs 4/0). A follow-up sweeps the same strict-=== true class on the other agent-facing boolean args (dryRun, replaceAll, background/pty, files, callgraph). Co-authored-by: Alfonso <289616620+alfonso-aft@users.noreply.github.com>
1 parent f0d40a7 commit 852d4a4

5 files changed

Lines changed: 69 additions & 5 deletions

File tree

packages/aft-bridge/src/__tests__/coerce.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,33 @@
11
import { describe, expect, test } from "bun:test";
2-
import { coerceStringArray } from "../coerce.js";
2+
import { coerceBoolean, coerceStringArray } from "../coerce.js";
3+
4+
describe("coerceBoolean", () => {
5+
test("passes real booleans through", () => {
6+
expect(coerceBoolean(true)).toBe(true);
7+
expect(coerceBoolean(false)).toBe(false);
8+
});
9+
10+
test("coerces the stringified booleans models emit (the recursive bug)", () => {
11+
expect(coerceBoolean("true")).toBe(true);
12+
expect(coerceBoolean("TRUE")).toBe(true);
13+
expect(coerceBoolean(" true ")).toBe(true);
14+
expect(coerceBoolean("1")).toBe(true);
15+
expect(coerceBoolean(1)).toBe(true);
16+
});
17+
18+
test("treats everything else as false (tight truthy set for safety gates)", () => {
19+
expect(coerceBoolean("false")).toBe(false);
20+
expect(coerceBoolean("0")).toBe(false);
21+
expect(coerceBoolean(0)).toBe(false);
22+
expect(coerceBoolean(2)).toBe(false);
23+
expect(coerceBoolean("yes")).toBe(false);
24+
expect(coerceBoolean("")).toBe(false);
25+
expect(coerceBoolean(undefined)).toBe(false);
26+
expect(coerceBoolean(null)).toBe(false);
27+
expect(coerceBoolean({})).toBe(false);
28+
expect(coerceBoolean([])).toBe(false);
29+
});
30+
});
331

432
describe("coerceStringArray", () => {
533
test("passes a real string array through, dropping empties + non-strings", () => {

packages/aft-bridge/src/coerce.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,35 @@ export function coerceStringArray(value: unknown): string[] {
4343
return [];
4444
}
4545

46+
/**
47+
* Coerce a tool argument that is contractually a boolean into a real boolean,
48+
* tolerating the shapes models send in practice.
49+
*
50+
* Like array and integer params (see `coerceStringArray` / `coerceOptionalInt`),
51+
* hosts deliver a boolean-typed param as the model's raw emitted value WITHOUT
52+
* coercing it to the declared schema type — and models non-deterministically
53+
* emit `true` as the string `"true"` (or `1` / `"1"`). A strict `args.x === true`
54+
* check then reads a stringified `"true"` as false, silently dropping the flag.
55+
* This bit `aft_delete`'s `recursive`: an agent passing `recursive: true` got
56+
* "is a directory, pass recursive: true" because the wire value was `"true"`.
57+
*
58+
* Conservative by design: only values that UNAMBIGUOUSLY mean true coerce to
59+
* true (`true`, case-insensitive `"true"`, `1`, `"1"`). Everything else —
60+
* `false`, `"false"`, `0`, `""`, `null`, `undefined`, objects — is `false`. A
61+
* false-negative just re-surfaces the original "pass the flag" error (safe); a
62+
* false-positive on a destructive gate like `recursive` would not be, so the
63+
* truthy set is kept tight rather than accepting arbitrary truthy values.
64+
*/
65+
export function coerceBoolean(value: unknown): boolean {
66+
if (typeof value === "boolean") return value;
67+
if (typeof value === "number") return value === 1;
68+
if (typeof value === "string") {
69+
const normalized = value.trim().toLowerCase();
70+
return normalized === "true" || normalized === "1";
71+
}
72+
return false;
73+
}
74+
4675
/**
4776
* Runtime coercion for agent-friendly sentinel handling.
4877
*

packages/aft-bridge/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export {
4444
// --- aft_callgraph flat formatter (shared by both plugin hosts) ---
4545
export type { CallgraphTheme } from "./callgraph-format.js";
4646
export { formatCallgraphSections, PLAIN_CALLGRAPH_THEME } from "./callgraph-format.js";
47-
export { coerceOptionalInt, coerceStringArray, isEmptyParam } from "./coerce.js";
47+
export { coerceBoolean, coerceOptionalInt, coerceStringArray, isEmptyParam } from "./coerce.js";
4848
export { LONG_RUNNING_COMMAND_TIMEOUT_MS, timeoutForCommand } from "./command-timeouts.js";
4949
// --- config tiers ---
5050
export type { ConfigTier } from "./config-tiers.js";

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import * as fs from "node:fs";
1313
import * as path from "node:path";
1414
import {
15+
coerceBoolean,
1516
coerceStringArray,
1617
formatEditSummary,
1718
formatReadFooter as formatSharedReadFooter,
@@ -1683,7 +1684,11 @@ function createDeleteTool(ctx: PluginContext): ToolDefinition {
16831684
if (inputs.length === 0) {
16841685
throw new Error("delete: `files` must be a non-empty array of paths");
16851686
}
1686-
const recursive = args.recursive === true;
1687+
// Coerce at the boundary: hosts deliver this boolean as the model's raw
1688+
// emitted value (e.g. the string "true") despite the declared schema, same
1689+
// as `files` above. A strict `=== true` then drops a stringified flag and
1690+
// an agent's `recursive: true` is silently lost (see coerceBoolean).
1691+
const recursive = coerceBoolean(args.recursive);
16871692
const projectRoot = await resolveProjectRoot(ctx, context);
16881693
const absolutePaths = inputs.map((f) => resolvePathFromProjectRoot(projectRoot, f));
16891694

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Both go through Rust so backups and checkpoint rollback work the same way.
44
*/
55

6-
import { coerceStringArray } from "@cortexkit/aft-bridge";
6+
import { coerceBoolean, coerceStringArray } from "@cortexkit/aft-bridge";
77
import type { AgentToolResult, ExtensionAPI, Theme } from "@earendil-works/pi-coding-agent";
88
import { type Static, Type } from "typebox";
99
import type { PluginContext } from "../types.js";
@@ -158,7 +158,9 @@ export function registerFsTools(pi: ExtensionAPI, ctx: PluginContext, surface: F
158158
"delete_file",
159159
{
160160
files,
161-
recursive: params.recursive === true,
161+
// Coerce at the boundary, like `files`: a stringified "true" from the
162+
// model must not silently drop the flag (see coerceBoolean).
163+
recursive: coerceBoolean(params.recursive),
162164
},
163165
extCtx,
164166
);

0 commit comments

Comments
 (0)