feat: implemented dynamic coercion for legacy tool argument aliases

- Added `normalizeSingleStringField` to dynamically map misplaced string inputs to required schema fields for single-argument tools. - Integrated argument normalization into `validateToolArguments` to handle model-specific variations in JSON payloads during validation passes. - Updated `coding-agent` streaming and rendering components to recognize `_input` as a legacy alias for `input` across various UI paths and logic flows. - Refactored `hashlineEditParamsSchema` to strictly enforce the `input` field while maintaining support for legacy aliases via runtime coercion rather than schema definition. - Corrected unit tests to reflect that `_input` is rejected by the strict schema but handled gracefully by the validation layer.
2026-07-02 23:32:23 +02:00
parent cc1990a2ad
commit 2c8daf0578
13 changed files with 169 additions and 43 deletions
@@ -6,19 +6,16 @@

 - Added support for overlaying Anthropic per-tier usage limits onto cached usage reports
 - Improved Anthropic credential ranking by using drain-rate pressure for weekly usage limits
-
 - Added Claude Fable weekly usage tracking to the Anthropic usage provider: the OAuth usage endpoint's new generic `limits` array is parsed for model-scoped weekly windows (`kind: "weekly_scoped"`), so `omp usage`, `/usage`, and usage history now surface a separate `Claude 7 Day (Fable)` row (id `anthropic:7d:fable`) alongside the shared 5h/7d windows — mirroring how Codex Spark rows are tracked. The `session`/`weekly_all` entries also backfill the shared 5h/7d windows if the legacy `five_hour`/`seven_day` buckets ever go null (as `seven_day_opus`/`seven_day_sonnet` already have).
 - Added `scopeLimits`/`blockScope` to the Anthropic credential-ranking strategy so an exhausted Fable/Mythos weekly cap no longer blocks the whole OAuth credential: credential-wide exhaustion gating now considers only shared umbrella windows plus the requested model family's own scoped cap, and reactive usage-limit blocks for Fable/Mythos requests land in a per-tier backoff scope (mirroring the Antigravity per-counter precedent).

 ### Fixed

+- Improved robustness of single-argument tool calls by automatically remapping mislabeled string arguments
+
 - Fixed Anthropic OAuth usage reporting to stop retrying on 429 rate-limit errors
 - Fixed usage cache to correctly persist null values during cold-start failure backoff windows
-### Fixed
-
 - Fixed cursor-agent persisted transcripts losing tool-call structure by synthesizing `toolCall` content blocks for exec-channel native tools (`bash`/`read`/`write`/`grep`/`ls`/`delete`/`lsp`), so replay pairs each tool result with its call instead of rendering header-less tool output beneath the last assistant text ([#4348](https://github.com/can1357/oh-my-pi/issues/4348)).
-### Fixed
-
 - Fixed OpenAI-compatible streaming usage parsing to prefer non-zero nested cached token counts when root `cached_tokens` is zero ([#4337](https://github.com/can1357/oh-my-pi/issues/4337)).
 - Fixed cursor-agent persisted transcripts losing tool-call structure by synthesizing `toolCall` content blocks for exec-channel native tools (`bash`/`read`/`write`/`grep`/`ls`/`delete`/`lsp`), so replay pairs each tool result with its call instead of rendering header-less tool output beneath the last assistant text ([#4348](https://github.com/can1357/oh-my-pi/issues/4348)). Synthesized blocks carry a new `kCursorExecResolved` symbol marker so the shared agent loop skips executing them a second time.

@@ -1081,6 +1081,54 @@ function normalizeStringEncodedArrayUnions(schema: unknown, value: unknown): { v
 	return { value: changed ? nextValue : valueObject, changed };
 }

+/**
+ * Name of the sole property when a schema declares exactly one required string
+ * field, else `undefined`. Recognizes the closed single-argument tool shape
+ * (`{ type: "object", properties: { X: { type: "string" } }, required: ["X"] }`).
+ */
+function singleRequiredStringKey(schema: unknown): string | undefined {
+	if (!schema || typeof schema !== "object" || Array.isArray(schema)) return undefined;
+	const obj = schema as Record<string, unknown>;
+	if (obj.type !== "object") return undefined;
+	const properties = obj.properties;
+	if (!properties || typeof properties !== "object") return undefined;
+	const keys = Object.keys(properties as Record<string, unknown>);
+	if (keys.length !== 1) return undefined;
+	const key = keys[0];
+	const required = obj.required;
+	if (!Array.isArray(required) || required.length !== 1 || required[0] !== key) return undefined;
+	const propertySchema = (properties as Record<string, unknown>)[key];
+	if (!propertySchema || typeof propertySchema !== "object") return undefined;
+	return (propertySchema as Record<string, unknown>).type === "string" ? key : undefined;
+}
+
+/**
+ * LLM-quirk repair for single-argument tools. When a tool declares exactly one
+ * property — a required string — some providers deliver the payload under a
+ * different key (e.g. the `edit` tool's patch arriving as `input`/`_input`, or
+ * any single-string tool whose argument the model mislabels). When the declared
+ * key is absent but another field holds a string, adopt the first such string
+ * as the declared key so the call validates instead of failing with "<key> was
+ * missing". A present-but-wrong-type value is left alone so its real type error
+ * still surfaces.
+ */
+function normalizeSingleStringField(schema: unknown, value: unknown): { value: unknown; changed: boolean } {
+	const key = singleRequiredStringKey(schema);
+	if (key === undefined) return { value, changed: false };
+	if (typeof value !== "object" || value === null || Array.isArray(value)) return { value, changed: false };
+	const record = value as Record<string, unknown>;
+	if (record[key] !== undefined) return { value, changed: false };
+	for (const candidate in record) {
+		if (candidate === key || !Object.hasOwn(record, candidate)) continue;
+		const candidateValue = record[candidate];
+		if (typeof candidateValue !== "string") continue;
+		const next = { ...record, [key]: candidateValue };
+		delete next[candidate];
+		return { value: next, changed: true };
+	}
+	return { value, changed: false };
+}
+
 // ============================================================================
 // Zod issue → coercion bridge
 // ============================================================================
@@ -1447,6 +1495,14 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[
 		changed = true;
 	}

+	// Single-argument tools (e.g. `edit`): if the model put the lone required
+	// string under a different key, adopt the first string field as that key.
+	const singleStringNorm = normalizeSingleStringField(json, normalizedArgs);
+	if (singleStringNorm.changed) {
+		normalizedArgs = singleStringNorm.value;
+		changed = true;
+	}
+
 	let result = validateContext(ctx, normalizedArgs);
 	if (result.success) return result.value as ToolCall["arguments"];

@@ -1480,6 +1536,14 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): ToolCall[
 			normalizedArgs = stringEncodedArrayNormPass.value;
 		}

+		// Re-run single-string remap: `coerceArgsFromIssues` may have just
+		// unwrapped a JSON-stringified root object, exposing a mislabelled lone
+		// string field the initial pre-pass could not see.
+		const singleStringNormPass = normalizeSingleStringField(json, normalizedArgs);
+		if (singleStringNormPass.changed) {
+			normalizedArgs = singleStringNormPass.value;
+		}
+
 		result = validateContext(ctx, normalizedArgs);
 		if (result.success) return result.value as ToolCall["arguments"];
 	}
@@ -1823,4 +1823,68 @@ describe("Tool argument coercion", () => {
 			expect(result).toEqual({ op: "done", count: 2 });
 		});
 	});
+
+	describe("single-string-field coercion", () => {
+		it("remaps a different string field to the single required string field when target is absent", () => {
+			const tool: Tool = {
+				name: "single-arg",
+				description: "",
+				parameters: z.object({ input: z.string() }),
+			};
+			const result = validateToolArguments(tool, {
+				type: "toolCall",
+				id: "c1",
+				name: "single-arg",
+				arguments: { _input: "hello payload" },
+			}) as { input: string };
+			expect(result).toEqual({ input: "hello payload" });
+		});
+
+		it("leaves arguments alone when the target field is present but has the wrong type", () => {
+			const tool: Tool = {
+				name: "single-arg",
+				description: "",
+				parameters: z.object({ input: z.string() }),
+			};
+			const result = validateToolArguments(tool, {
+				type: "toolCall",
+				id: "c2",
+				name: "single-arg",
+				arguments: { input: 123, _input: "hello" },
+			}) as { input: string };
+			expect(result.input).toBe("123");
+		});
+
+		it("does not remap when the schema has multiple fields", () => {
+			const tool: Tool = {
+				name: "multi-arg",
+				description: "",
+				parameters: z.object({ input: z.string(), path: z.string() }),
+			};
+			expect(() =>
+				validateToolArguments(tool, {
+					type: "toolCall",
+					id: "c3",
+					name: "multi-arg",
+					arguments: { other: "hello", path: "file.ts" },
+				}),
+			).toThrow();
+		});
+
+		it("does not remap when there is no string candidate", () => {
+			const tool: Tool = {
+				name: "single-arg",
+				description: "",
+				parameters: z.object({ input: z.string() }),
+			};
+			expect(() =>
+				validateToolArguments(tool, {
+					type: "toolCall",
+					id: "c4",
+					name: "single-arg",
+					arguments: { other: 123 },
+				}),
+			).toThrow();
+		});
+	});
 });
@@ -4,18 +4,15 @@

 ### Changed

- Defer session_stop extension hooks while agent-owned background jobs are still in progress
+- Removed `_input` as a supported alias for the `edit` tool input field

+- Defer session_stop extension hooks while agent-owned background jobs are still in progress
 - Stop-time settle passes now treat a yield with background async jobs (async `bash`/`task` spawns) owned by the agent — still running or with undelivered results — as a scheduling pause instead of a terminal stop: the incomplete-todo reminder stays silent and the `session_stop` hook pass is deferred, both firing at the settle reached once the session is fully idle. Async-result delivery re-wakes the loop, so neither pass is lost.

 ### Fixed

 - Improved reliability of edits when file snapshots share identical 16-bit hash tags
-### Fixed
-
 - Fixed ACP `terminal/create` sending the bash tool's full shell line in `command` with no `args`, which broke spec-conformant clients that spawn `command`+`args` directly (no implicit shell) — any command containing a space, pipe, `&&`, redirect, or `$(...)` failed with `ENOENT` and the agent silently degraded to read-only tools. The bash tool now wraps the shell line before calling `clientBridge.createTerminal`, reusing the same shell binary + args the local `bash-executor` resolves via `settings.getShellConfig()` (Git Bash / `bash.exe` on Windows, `$SHELL` with `sh` fallback on POSIX) so bash semantics — `$VAR`, `$(...)`, `source`, POSIX quoting, `-l` — are preserved on both platforms. ([#4333](https://github.com/can1357/oh-my-pi/issues/4333))
-### Fixed
-
 - Fixed inference worker subprocesses (TTS, STT, tiny-model, mnemopi embeddings) discarding stderr, which left every unexpected exit — most visibly the local Kokoro TTS worker's recurring `exit code 7` crash loop — undiagnosable from the parent's logs. `createWorkerSubprocess` now pipes stderr without starting a live read while the worker is idle, then drains the stream after `onExit`, emits captured lines to `logger.debug` under an `<exitLabel> stderr` message, and keeps the last 16 KiB in a bounded ring that gets appended to the `Error` surfaced through `onError`. The exit surface is synchronized with the post-exit drain via `SpawnedSubprocess.stderrDrained`, so the full native trace shows up on the `tts: worker error` line without reintroducing event-loop liveness from unref'd workers. ([#4324](https://github.com/can1357/oh-my-pi/issues/4324))

 ## [16.3.2] - 2026-07-02
@@ -1,6 +1,6 @@
 /**
 * Coding-agent runner that drives the hashline {@link Patcher} on behalf of
- * the `edit` tool. Converts a `{input}` tool-call payload into a
+ * the `edit` tool. Converts an `{input}` tool-call payload into a
 * fully-applied patch, wraps the result in the agent's
 * {@link AgentToolResult} shape, and attaches LSP diagnostics + `outputMeta`
 * for the renderer.
@@ -1,19 +1,12 @@
 /**
 * Arktype schema for the `edit` tool's hashline mode payload. The schema is
 * deliberately permissive (allows extra keys) so providers can attach extra
- * keys without rejection; only `input` is required. `_input` is accepted as a
- * provider-emitted alias for `input`.
+ * keys without rejection; only `input` is required.
 */
 import { type } from "arktype";

-const requiredInputSchema = type({ input: "string" });
-const inputAliasSchema = type({ "input?": "string", "_input?": "string" });
+export const hashlineEditParamsSchema = type({
+	input: "string",
+});

-export const hashlineEditParamsSchema = inputAliasSchema
-	.pipe(raw => {
-		if (raw.input !== undefined || raw._input === undefined) return raw;
-		return { ...raw, input: raw._input };
-	})
-	.pipe(requiredInputSchema);
-
-export type HashlineParams = Parameters<typeof hashlineEditParamsSchema.assert>[0];
+export type HashlineParams = typeof hashlineEditParamsSchema.infer;
@@ -114,6 +114,7 @@ interface EditRenderArgs {
 	newText?: string;
 	patch?: string;
 	input?: string;
+	_input?: string;
 	all?: boolean;
 	// Patch mode fields
 	op?: Operation;
@@ -600,10 +601,11 @@ function getHashlineInputRenderSummary(
 	args: EditRenderArgs,
 	editMode: EditMode | undefined,
 ): HashlineInputRenderSummary | undefined {
-	if (editMode !== "hashline" || typeof args.input !== "string") {
+	const input = args.input ?? args._input;
+	if (editMode !== "hashline" || typeof input !== "string") {
 		return undefined;
 	}
-	return { entries: getHashlineInputSections(args.input) };
+	return { entries: getHashlineInputSections(input) };
 }

 function getApplyPatchRenderSummary(
@@ -429,9 +429,19 @@ const patchStrategy: EditStreamingStrategy<PatchArgs> = {

 interface HashlineArgs {
 	input?: string;
+	_input?: string;
 	__partialJson?: string;
 }

+/**
+ * Text payload of a hashline edit call. The public schema declares `input`, but
+ * streaming sees the raw model output before validation coerces aliases, so a
+ * provider that emits the legacy `_input` key still previews correctly.
+ */
+function hashlineEditText(args: HashlineArgs | undefined): string | undefined {
+	return args?.input ?? args?._input;
+}
+
 /**
 * While streaming a free-form text payload (apply_patch envelope, hashline
 * input), trim the trailing partial line so per-character growth of an
@@ -513,7 +523,8 @@ const hashlineStrategy: EditStreamingStrategy<HashlineArgs> = {
 		return args;
 	},
 	async computeDiffPreview(args, ctx) {
-		if (typeof args.input !== "string" || args.input.length === 0) return null;
+		const input = hashlineEditText(args);
+		if (typeof input !== "string" || input.length === 0) return null;
 		// Unlike apply_patch, hashline previews flow through `applyPartialTo`,
 		// whose streaming-tolerant parser (`parsePatchStreaming` → `endStreaming`)
 		// drops a payload-less trailing op and projects a partially-typed payload
@@ -521,7 +532,6 @@ const hashlineStrategy: EditStreamingStrategy<HashlineArgs> = {
 		// would instead strip the sole payload of a single-op `replace`/`insert`
 		// for almost the entire stream, collapsing the preview to "No changes" and
 		// rendering a blank box. Feed the raw in-flight text straight through.
-		const input = args.input;
 		ctx.signal.throwIfAborted();

 		let sections: readonly HashlineInputSection[];
@@ -579,19 +589,19 @@ const hashlineStrategy: EditStreamingStrategy<HashlineArgs> = {
 		return "";
 	},
 	matcherDigest(args) {
-		const input = args?.input;
+		const input = hashlineEditText(args);
 		if (typeof input !== "string") return undefined;
 		// Body rows are `+TEXT`; headers and op lines are grammar, never content.
 		return extractAddedLines(input, false);
 	},
 	matcherPaths(args) {
-		const input = args?.input;
+		const input = hashlineEditText(args);
 		if (typeof input !== "string" || input.length === 0) return undefined;
 		const paths = extractHashlineHeaderPaths(input);
 		return paths.length > 0 ? paths : undefined;
 	},
 	matcherEntries(args) {
-		const input = args?.input;
+		const input = hashlineEditText(args);
 		if (typeof input !== "string" || input.length === 0) return undefined;
 		const entries = splitHashlinePerFile(input);
 		return entries.length > 0 ? entries : undefined;
@@ -60,7 +60,7 @@ export function normalizeToolEventInput(toolName: string, input: Record<string,
 	if (toolName !== "edit" || stringField(input, "path")) return input;

 	// Hashline edit mode: the only authoritative target list is the parsed
-	// `¶PATH#TAG` headers inside `input`/`_input`. Trusting a passthrough
+	// `¶PATH#TAG` headers inside the patch. Trusting a passthrough
 	// `_path` here would let a model-supplied field override the real edit
 	// target and bypass extension gates that allowlist by path.
 	const rawInput = stringField(input, "input") ?? stringField(input, "_input");
@@ -13,7 +13,7 @@ type ToolArgsRevealComponent = {
 // patch/replace `edits[].diff`) still fall through to the throttled parse.
 const STREAMING_STRING_KEYS_BY_TOOL: Record<string, readonly string[]> = {
 	write: ["content"],
-	edit: ["input"],
+	edit: ["input", "_input"],
 	eval: ["code"],
 };

@@ -306,11 +306,7 @@ describe("hashlineEditParamsSchema — payload shape", () => {
 	}

 	it("declares only `input` as the model-facing field", () => {
-		// Create an arktype schema that mirrors hashlineEditParamsSchema structure
-		const testSchema = type({
-			input: "string",
-		});
-		const jsonSchema = getJsonSchema(testSchema) as {
+		const jsonSchema = getJsonSchema(hashlineEditParamsSchema) as {
 			properties?: Record<string, unknown>;
 			required?: string[];
 		};
@@ -327,12 +323,11 @@ describe("hashlineEditParamsSchema — payload shape", () => {
 		expect(result.success).toBe(true);
 	});

-	it("accepts `_input` as a provider-emitted alias for `input`", () => {
+	it("rejects `_input` as an alias for `input`", () => {
 		const result = arkSafeParse(hashlineEditParamsSchema, {
 			_input: `[x.ts]\nINS.HEAD:\n${repl("x")}`,
 		});
-		expect(result.success).toBe(true);
-		if (result.success) expect(result.data.input).toBe(`[x.ts]\nINS.HEAD:\n${repl("x")}`);
+		expect(result.success).toBe(false);
 	});

 	it("still requires `input`", () => {
@@ -2,6 +2,10 @@

 ## [Unreleased]

+### Fixed
+
+- Improved input detection for edit tool summary and body views
+
 ## [16.3.1] - 2026-07-02

 ### Changed
@@ -92,7 +92,7 @@ function fileEntry(d: Record<string, unknown>): FileEntry {
 }

 function Summary({ args, result }: ToolRenderProps): ReactNode {
-	const input = str(args.input);
+	const input = str(args.input) ?? str(args._input);
 	const paths = input ? inputPaths(input) : [];
 	const argPath = str(args.file_path) ?? str(args.path);
 	if (paths.length === 0 && argPath) paths.push(argPath);
@@ -187,7 +187,7 @@ function FileSection({ entry, fallbackPath }: { entry: FileEntry; fallbackPath?:
 }

 function Body({ args, result }: ToolRenderProps): ReactNode {
-	const input = str(args.input);
+	const input = str(args.input) ?? str(args._input);
 	const details = detailsRecord(result);
 	const perFile: FileEntry[] = [];
 	if (details && Array.isArray(details.perFileResults)) {
@@ -232,7 +232,7 @@ function Body({ args, result }: ToolRenderProps): ReactNode {
 				</KvGrid>
 			)}
 			{input !== null && input.length > 0 && <Output text={input} variant="code" maxLines={10} title="input" />}
-			{input === null && args.input !== undefined && <InvalidArg what="input" />}
+			{input === null && (args.input !== undefined || args._input !== undefined) && <InvalidArg what="input" />}
 		</>
 	);
 }