fix(coding-agent): removed atom sub edits and excluded write from Anthropic strict allowlist

- Removed the atom tool's `sub` verb from its schema, parser, conflict checks, and edit application.
- Updated atom documentation, examples, and tests to reject `sub`-based replacements in favor of `set` replacements.
- Adjusted Anthropic strict-tool handling by dropping `write` from the allowlist and updating alignment expectations.
This commit is contained in:
can1357
2026-04-26 05:23:32 +02:00
parent 52da3674a4
commit 0db51b1dbc
7 changed files with 40 additions and 154 deletions
+1 -2
View File
@@ -13,8 +13,7 @@
### Fixed
- Fixed Anthropic strict tool-call retry logic to also handle 400 errors reporting `schema is too complex` when grammar compilation fails
-
- Fixed Anthropic tool schema compilation failures by keeping the `write` tool out of the strict-tool allowlist when the full coding-agent tool set is active
- Fixed Anthropic 400 `tools.*.custom: For 'object' type, property 'minItems' is not supported` by stripping `minItems` from object-shaped JSON schema nodes (array nodes still keep supported `minItems` values)
- Fixed Anthropic tool schemas that used tuple-style arrays by stripping unsupported `maxItems` and only preserving provider-supported `minItems` values
- Fixed Anthropic and OpenRouter Anthropic tool calls that previously failed with `compiled grammar is too large` by retrying automatically without strict tool schemas and reusing non-strict mode for subsequent requests in the same provider session
+3 -2
View File
@@ -216,7 +216,8 @@ function isAnthropicStrictGrammarTooLargeError(error: unknown): boolean {
if (extractHttpStatusFromError(error) !== 400) return false;
const message = error instanceof Error ? error.message : String(error);
const isStrictGrammarTooLarge = /compiled grammar/i.test(message) && /too large/i.test(message);
const isSchemaCompilationTooComplex = /schema/i.test(message) && /too complex/i.test(message) && /compil/i.test(message);
const isSchemaCompilationTooComplex =
/schema/i.test(message) && /too complex/i.test(message) && /compil/i.test(message);
return /invalid_request_error/i.test(message) && (isStrictGrammarTooLarge || isSchemaCompilationTooComplex);
}
@@ -1754,7 +1755,7 @@ export function convertAnthropicMessages(
}
const ANTHROPIC_UNSUPPORTED_TOOL_SCHEMA_FIELDS = new Set(["maxItems", "patternProperties"]);
const ANTHROPIC_STRICT_TOOL_ALLOWLIST = new Set(["bash", "python", "edit", "find", "write"]);
const ANTHROPIC_STRICT_TOOL_ALLOWLIST = new Set(["bash", "python", "edit", "find"]);
const MAX_ANTHROPIC_STRICT_TOOLS = 20;
const MAX_ANTHROPIC_STRICT_OPTIONAL_PARAMETERS = 24;
const MAX_ANTHROPIC_STRICT_UNION_PARAMETERS = 16;
+5 -5
View File
@@ -437,7 +437,7 @@ describe("Anthropic request fingerprint alignment", () => {
it("marks only the Anthropic strict allowlist strict", async () => {
const tools: Tool[] = [
...(["bash", "python", "edit", "find", "write"] as const).map(name => ({
...(["bash", "python", "edit", "find"] as const).map(name => ({
name,
description: `${name} tool`,
strict: true,
@@ -447,7 +447,7 @@ describe("Anthropic request fingerprint alignment", () => {
required: ["requiredValue"],
} as unknown as TSchema,
})),
...(["grep", "read", "task", "todo_write", "web_search", "ast_grep"] as const).map(name => ({
...(["write", "grep", "read", "task", "todo_write", "web_search", "ast_grep"] as const).map(name => ({
name,
description: `${name} tool`,
strict: true,
@@ -473,11 +473,11 @@ describe("Anthropic request fingerprint alignment", () => {
const strictNames = (payload.tools ?? []).filter(tool => tool.strict === true).map(tool => tool.name);
expect(strictNames).toEqual(["bash", "python", "edit", "find", "write"]);
expect(strictNames).toEqual(["bash", "python", "edit", "find"]);
expect(payload.tools?.find(tool => tool.name === "bash")?.input_schema?.required).toEqual(["requiredValue"]);
});
it("honors strict=false for allowlisted Anthropic tools", async () => {
it("honors strict=false and skips non-allowlisted Anthropic tools", async () => {
const tools: Tool[] = [
{
name: "bash",
@@ -531,7 +531,7 @@ describe("Anthropic request fingerprint alignment", () => {
)) as { tools?: Array<{ name?: string; strict?: boolean }> };
const strictNames = (payload.tools ?? []).filter(tool => tool.strict === true).map(tool => tool.name);
expect(strictNames).toEqual(["python", "write"]);
expect(strictNames).toEqual(["python"]);
});
it("drops fine-grained tool-streaming beta from default Anthropic client options", () => {
@@ -265,8 +265,8 @@ describe("anthropic stream envelope handling", () => {
...context,
tools: [
{
name: "write",
description: "Write a value",
name: "edit",
description: "Edit a value",
strict: true,
parameters: Type.Object({ query: Type.String() }),
},
@@ -321,8 +321,8 @@ describe("anthropic stream envelope handling", () => {
...context,
tools: [
{
name: "write",
description: "Write a value",
name: "edit",
description: "Edit a value",
strict: true,
parameters: Type.Object({ query: Type.String() }),
},
+10 -75
View File
@@ -1,7 +1,7 @@
/**
*
* Flat locator + verb edit mode backed by hashline anchors. Each entry carries
* one shared `loc` selector plus one or more verbs (`pre`, `set`, `post`, `sub`).
* one shared `loc` selector plus one or more verbs (`pre`, `set`, `post`).
* The runtime resolves those verbs into internal anchor-scoped edits and still
* reuses hashline's staleness scheme (`computeLineHash`) verbatim.
*
@@ -9,13 +9,12 @@
* { path, loc: "5th", set: ["..."] }
* { path, loc: "5th", pre: ["..."] }
* { path, loc: "5th", post: ["..."] }
* { path, loc: "5th", sub: ["find", "replace"] }
* { path, loc: "5th", pre: [...], set: [...], post: [...] }
* { path, loc: "^", pre: [...] } // prepend to BOF
* { path, loc: "$", post: [...] } // append to EOF
*
* `set: []` on a single-anchor locator deletes that line. `set:[""]` preserves
* a blank line. Line ranges are not supported; `set` and `sub` cannot coexist
* a blank line. Line ranges are not supported.
* in the same entry.
*
* For deleting or moving files, the agent should use bash.
@@ -66,18 +65,6 @@ export const atomEditSchema = Type.Object(
set: Type.Optional(textSchema),
pre: Type.Optional(textSchema),
post: Type.Optional(textSchema),
sub: Type.Optional(
Type.Array(Type.String(), {
description: "substring search and replace as [find, replace]",
minItems: 2,
maxItems: 2,
examples: [
["||", "??"],
["true", "false"],
["i--", "i++"],
],
}),
),
},
{ additionalProperties: false },
);
@@ -102,7 +89,6 @@ export type AtomEdit =
| { op: "pre"; pos: Anchor; lines: string[] }
| { op: "post"; pos: Anchor; lines: string[] }
| { op: "del"; pos: Anchor }
| { op: "sub"; pos: Anchor; find: string; to: string }
| { op: "append_file"; lines: string[] }
| { op: "prepend_file"; lines: string[] };
@@ -110,7 +96,7 @@ export type AtomEdit =
// Param guards
// ═══════════════════════════════════════════════════════════════════════════
const ATOM_VERB_KEYS = ["set", "pre", "post", "sub"] as const;
const ATOM_VERB_KEYS = ["set", "pre", "post"] as const;
type AtomOptionalKey = "path" | "loc" | (typeof ATOM_VERB_KEYS)[number];
const ATOM_OPTIONAL_KEYS = ["path", "loc", ...ATOM_VERB_KEYS] as const satisfies readonly AtomOptionalKey[];
@@ -227,20 +213,6 @@ function parseLoc(raw: string, editIndex: number): ParsedAtomLoc {
return { kind: "anchor", pos: parseAnchor(raw, "loc") };
}
function parseSubSpec(sub: AtomToolEdit["sub"], editIndex: number): { find: string; withText: string } {
if (!Array.isArray(sub) || sub.length !== 2) {
throw new Error(`Edit ${editIndex}: sub must be a 2-item tuple [find, replace].`);
}
const [find, withText] = sub;
if (typeof find !== "string" || find.length === 0) {
throw new Error("sub requires a non-empty `find` string (the unique substring on the anchored line).");
}
if (typeof withText !== "string") {
throw new Error("sub replacement must be a string.");
}
return { find, withText };
}
function classifyAtomEdit(edit: AtomToolEdit): string {
const entry = stripNullAtomFields(edit);
const verbs = ATOM_VERB_KEYS.filter(k => entry[k] !== undefined);
@@ -255,9 +227,6 @@ function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0): AtomEdit[] {
`Edit ${editIndex}: missing verb. Each entry must include at least one of: ${ATOM_VERB_KEYS.join(", ")}.`,
);
}
if (entry.set !== undefined && entry.sub !== undefined) {
throw new Error(`Edit ${editIndex}: set and sub cannot be used together in the same entry.`);
}
if (typeof entry.loc !== "string") {
throw new Error(`Edit ${editIndex}: missing loc. Use a selector like "160sr", "^", or "$".`);
}
@@ -266,7 +235,7 @@ function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0): AtomEdit[] {
const resolved: AtomEdit[] = [];
if (loc.kind === "bof") {
if (entry.set !== undefined || entry.sub !== undefined || entry.post !== undefined) {
if (entry.set !== undefined || entry.post !== undefined) {
throw new Error(`Edit ${editIndex}: loc "^" only supports pre.`);
}
if (entry.pre !== undefined) {
@@ -276,7 +245,7 @@ function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0): AtomEdit[] {
}
if (loc.kind === "eof") {
if (entry.set !== undefined || entry.sub !== undefined || entry.pre !== undefined) {
if (entry.set !== undefined || entry.pre !== undefined) {
throw new Error(`Edit ${editIndex}: loc "$" only supports post.`);
}
if (entry.post !== undefined) {
@@ -295,10 +264,6 @@ function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0): AtomEdit[] {
resolved.push({ op: "set", pos: loc.pos, lines: hashlineParseText(entry.set) });
}
}
if (entry.sub !== undefined) {
const { find, withText } = parseSubSpec(entry.sub, editIndex);
resolved.push({ op: "sub", pos: loc.pos, find, to: withText });
}
if (entry.post !== undefined) {
resolved.push({ op: "post", pos: loc.pos, lines: hashlineParseText(entry.post) });
}
@@ -315,7 +280,6 @@ function* getAtomAnchors(edit: AtomEdit): Iterable<Anchor> {
case "pre":
case "post":
case "del":
case "sub":
yield edit.pos;
return;
default:
@@ -348,16 +312,16 @@ function validateAtomAnchors(edits: AtomEdit[], fileLines: string[], warnings: s
}
function validateNoConflictingAnchorOps(edits: AtomEdit[]): void {
// For each anchor line, at most one mutating op (set/del/sub).
// For each anchor line, at most one mutating op (set/del).
// `pre`/`post` (insert ops) may coexist with them — they don't mutate the anchor line.
const mutatingPerLine = new Map<number, string>();
for (const edit of edits) {
if (edit.op !== "set" && edit.op !== "del" && edit.op !== "sub") continue;
if (edit.op !== "set" && edit.op !== "del") continue;
const existing = mutatingPerLine.get(edit.pos.line);
if (existing) {
throw new Error(
`Conflicting ops on anchor line ${edit.pos.line}: \`${existing}\` and \`${edit.op}\`. ` +
`At most one of set/del/sub is allowed per anchor.`,
`At most one of set/del is allowed per anchor.`,
);
}
mutatingPerLine.set(edit.pos.line, edit.op);
@@ -368,26 +332,6 @@ function validateNoConflictingAnchorOps(edits: AtomEdit[]): void {
// Apply
// ═══════════════════════════════════════════════════════════════════════════
function applySubToLine(edit: { op: "sub"; pos: Anchor; find: string; to: string }, current: string): string {
const first = current.indexOf(edit.find);
if (first === -1) {
throw new Error(
`sub: substring \`${edit.find}\` not found on line ${edit.pos.line}. ` +
`Current line content: ${JSON.stringify(current)}`,
);
}
const second = current.indexOf(edit.find, first + 1);
if (second !== -1) {
throw new Error(
`sub: substring \`${edit.find}\` occurs more than once on line ${edit.pos.line}; ` +
`use a longer substring that uniquely identifies the target. ` +
`Current line content: ${JSON.stringify(current)}`,
);
}
// `sub`: replace only the matched span; tail preserved.
return current.slice(0, first) + edit.to + current.slice(first + edit.find.length);
}
function maybeAutocorrectEscapedTabIndentation(edits: AtomEdit[], warnings: string[]): void {
const enabled = Bun.env.PI_HASHLINE_AUTOCORRECT_ESCAPED_TABS !== "0";
if (!enabled) return;
@@ -487,7 +431,7 @@ export function applyAtomEdits(
bucket.sort((a, b) => a.idx - b.idx);
const idx = line - 1;
let currentLine = fileLines[idx];
const currentLine = fileLines[idx];
let replacement: string[] = [currentLine];
let replacementSet = false;
let anchorMutated = false;
@@ -513,13 +457,6 @@ export function applyAtomEdits(
replacementSet = true;
anchorMutated = true;
break;
case "sub": {
currentLine = applySubToLine(edit, currentLine);
replacement = currentLine.includes("\n") ? currentLine.split("\n") : [currentLine];
replacementSet = true;
anchorMutated = true;
break;
}
}
}
@@ -535,9 +472,7 @@ export function applyAtomEdits(
if (replacementProducesNoChange) {
const firstEdit = bucket[0]?.edit;
const loc = firstEdit ? `${firstEdit.pos.line}${firstEdit.pos.hash}` : `${line}`;
const reason = bucket.some(b => b.edit.op === "sub")
? "sub produced no change (find/replace already applied or both substrings are equal)"
: "replacement is identical to the current line content";
const reason = "replacement is identical to the current line content";
noopEdits.push({
editIndex: bucket[0]?.idx ?? 0,
loc,
+11 -24
View File
@@ -15,14 +15,11 @@ Verbs:
- `set: ["…"]` — replace the anchor line
- `pre: ["…"]` — insert before the anchor line (or at BOF when `loc:"^"`)
- `post: ["…"]` — insert after the anchor line (or at EOF when `loc:"$"`)
- `sub: [find, replace]` — replace a unique substring on the anchored line; the line tail is preserved automatically
Combination rules:
- On a single-anchor `loc`, you may combine `pre`, **one of** `set` or `sub`, and `post` in the same entry.
- On a single-anchor `loc`, you may combine `pre`, `set`, and `post` in the same entry.
- `set: []` on a single-anchor `loc` deletes that line.
- `set:[""]` is **not** delete — it replaces the line with a blank line.
`sub` is the cheapest op when only part of a line changes. `find` and `replace` should both be the smallest fragment that does the job.
</operations>
<examples>
@@ -45,28 +42,23 @@ All examples below reference the same file:
{{hline 14 "}"}}
```
# Swap an operator with `sub`
# Swap an operator by replacing the line
Original line 4: `const fallback = group.targetFramework || 'All Frameworks';`
`{path:"a.ts",edits:[{loc:{{href 4 "const fallback = group.targetFramework || 'All Frameworks';"}},sub:["||","??"]}]}`
`{path:"a.ts",edits:[{loc:{{href 4 "const fallback = group.targetFramework || 'All Frameworks';"}},set:["const fallback = group.targetFramework ?? 'All Frameworks';"]}]}`
# Flip a literal with `sub`
# Flip a literal by replacing the line
Original line 2: `const timeout = 5000;`
`{path:"a.ts",edits:[{loc:{{href 2 "const timeout = 5000;"}},sub:["5000","30_000"]}]}`
`{path:"a.ts",edits:[{loc:{{href 2 "const timeout = 5000;"}},set:["const timeout = 30_000;"]}]}`
# Negate a condition with `sub`
# Negate a condition by replacing the line
Original line 10: `\tif (x) {`
`{path:"a.ts",edits:[{loc:{{href 10 "\tif (x) {"}},sub:["(x)","(!x)"]}]}`
# Off-by-one with `sub`
For a single-digit/operator nudge, `sub` is the cheapest op. Do **not** rewrite the whole line with `set`.
Original line 2: `const timeout = 5000;`
`{path:"a.ts",edits:[{loc:{{href 2 "const timeout = 5000;"}},sub:["5000","5001"]}]}`
`{path:"a.ts",edits:[{loc:{{href 10 "\tif (x) {"}},set:["\tif (!x) {"]}]}`
# Combine `pre` + `set` + `post` in one entry
`{path:"a.ts",edits:[{loc:{{href 6 "\tlog();"}},pre:["\tvalidate();"],set:["\tlog();"],post:["\tcleanup();"]}]}`
# Replace one whole line with `set`
Use `set` when you're rewriting most of the line, or when `sub` would need a long `find`.
Use `set` to replace the full anchored line, preserving any unchanged surrounding lines yourself.
`{path:"a.ts",edits:[{loc:{{href 3 "const tag = \"DO NOT SHIP\";"}},set:["const tag = \"OK\";"]}]}`
# Replace multiple non-adjacent lines
@@ -87,23 +79,18 @@ Use `set` when you're rewriting most of the line, or when `sub` would need a lon
`{path:"a.ts",edits:[{loc:"$",post:["","export const VERSION = \"1.0.0\";"]}]}`
# Cross-file override inside `loc`
`{path:"a.ts",edits:[{loc:"b.ts:{{href 2 "const timeout = 5000;"}}",sub:["5000","30_000"]}]}`
`{path:"a.ts",edits:[{loc:"b.ts:{{href 2 "const timeout = 5000;"}}",set:["const timeout = 30_000;"]}]}`
</examples>
<critical>
- Make the minimum exact edit.
- Copy the full anchors exactly as shown by `read/grep` (for example `160sr`, not just `sr`).
- `loc` chooses the target. Verbs describe what to do there.
- On a single-anchor `loc`, you may combine `pre`, **one of** `set` or `sub`, and `post`.
- `set` and `sub` cannot appear together in the same entry.
- On a range `loc`, only `set` is allowed.
- On a single-anchor `loc`, you may combine `pre`, `set`, and `post`.
- `loc:"^"` only supports `pre`. `loc:"$"` only supports `post`.
- For `sub`, the first tuple element (`find`) must occur **exactly once on the anchored line**. It never spans newlines.
- Prefer the **smallest** `sub` fragments. On a single line of code, 1–4 chars is usually enough (`"||"`, `"true"`, `"i--"`).
- **Switch to `set` when `sub` gets long.** If `find` would be more than ~half the line, or the replacement would restate most of the line, use `set` instead.
- `set: []` deletes the anchored line. `set:[""]` preserves a blank line.
- Within a single request you may submit edits in any order — the runtime applies them bottom-up so they don't shift each other. After any request that mutates a file, anchors below the mutation are stale on disk; re-read before issuing more edits to that file.
- `set`/`sub`/delete target the current file content only. Do not try to reference old line text after the file has changed.
- `set` operations target the current file content only. Do not try to reference old line text after the file has changed.
- Text content must be literal file content with matching indentation. If the file uses tabs, use real tabs.
- You **MUST NOT** use this tool to reformat or clean up unrelated code.
</critical>
+6 -42
View File
@@ -3,12 +3,14 @@ import {
type AtomEdit,
type AtomToolEdit,
applyAtomEdits,
atomEditSchema,
computeLineHash,
HashlineMismatchError,
resolveAtomEntryPaths,
resolveAtomToolEdit,
} from "@oh-my-pi/pi-coding-agent/edit";
import type { Anchor } from "@oh-my-pi/pi-coding-agent/edit/modes/hashline";
import { Value } from "@sinclair/typebox/value";
function tag(line: number, content: string): Anchor {
return { line, hash: computeLineHash(line, content) };
@@ -83,47 +85,9 @@ describe("applyAtomEdits — pre/post", () => {
});
});
describe("applyAtomEdits — sub", () => {
it("replaces a unique substring", () => {
const content = "const timeout = 5000;";
const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "5000", to: "30_000" }];
const result = applyAtomEdits(content, edits);
expect(result.lines).toBe("const timeout = 30_000;");
});
it("preserves the line tail (trailing semicolon, comma, brace)", () => {
const content = " required: true,";
const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "true", to: "false" }];
const result = applyAtomEdits(content, edits);
expect(result.lines).toBe(" required: false,");
});
it("swaps an operator without restating the surrounding expression", () => {
const content = "\tfor (let i = 0; i < value.length; i--) {";
const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "i--", to: "i++" }];
const result = applyAtomEdits(content, edits);
expect(result.lines).toBe("\tfor (let i = 0; i < value.length; i++) {");
});
it("errors when find is absent", () => {
const content = "abc def";
const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "missing", to: "x" }];
expect(() => applyAtomEdits(content, edits)).toThrow(/not found/);
});
it("errors when find is non-unique", () => {
const content = "abc abc";
const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "abc", to: "Z" }];
expect(() => applyAtomEdits(content, edits)).toThrow(/more than once/);
});
it("rejects conflict with set on same anchor", () => {
const content = "abc";
const edits: AtomEdit[] = [
{ op: "sub", pos: tag(1, "abc"), find: "abc", to: "x" },
{ op: "set", pos: tag(1, "abc"), lines: ["y"] },
];
expect(() => applyAtomEdits(content, edits)).toThrow(/Conflicting ops/);
describe("atom edit schema", () => {
it("rejects sub edits", () => {
expect(Value.Check(atomEditSchema, { loc: "1ab", sub: ["5000", "30_000"] })).toBe(false);
});
});
@@ -175,7 +139,7 @@ describe("resolveAtomToolEdit — loc syntax", () => {
it("ignores null optional verb fields", () => {
const content = "aaa\nbbb\nccc";
const loc = `2${computeLineHash(2, "bbb")}`;
const toolEdit = { loc, pre: null, set: "BBB", post: null, sub: null } as unknown as AtomToolEdit;
const toolEdit = { loc, pre: null, set: "BBB", post: null } as unknown as AtomToolEdit;
const resolved = resolveAtomToolEdit(toolEdit);
expect(resolved).toEqual([{ op: "set", pos: tag(2, "bbb"), lines: ["BBB"] }]);