Fimeg/letta-code
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

refactor: tighten codex toolset parity without freeform (#1014)

Browse Source
This commit is contained in:
Charles Packer
2026-02-18 12:55:09 -08:00
committed by GitHub
parent a1637b313a
commit 4e1827ebc1
17 changed files with 336 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files

175
src/tools/impl/ApplyPatch.ts
View File

@@ -32,7 +32,7 @@ interface Hunk {
}
/**
* Simple ApplyPatch implementation compatible with the Letta/Codex apply_patch tool format.
* ApplyPatch implementation compatible with the Letta/Codex apply_patch JSON tool format.
*
* Supports:
* - *** Add File: path
@@ -48,13 +48,21 @@ export async function apply_patch(
const { input } = args;
const lines = input.split(/\r?\n/);
if (lines[0]?.trim() !== "*** Begin Patch") {
const beginIndex = lines.findIndex(
(line) => line.trim() === "*** Begin Patch",
);
if (beginIndex !== 0) {
throw new Error('Patch must start with "*** Begin Patch"');
}
const endIndex = lines.lastIndexOf("*** End Patch");
const endIndex = lines.findIndex((line) => line.trim() === "*** End Patch");
if (endIndex === -1) {
throw new Error('Patch must end with "*** End Patch"');
}
for (let tail = endIndex + 1; tail < lines.length; tail += 1) {
if ((lines[tail] ?? "").trim().length > 0) {
throw new Error("Unexpected content after *** End Patch");
}
}
const ops: FileOperation[] = [];
let i = 1;
@@ -68,22 +76,34 @@ export async function apply_patch(
if (line.startsWith("*** Add File:")) {
const filePath = line.replace("*** Add File:", "").trim();
assertRelativePatchPath(filePath, "Add File");
i += 1;
const contentLines: string[] = [];
while (i < endIndex) {
const raw = lines[i];
if (raw === undefined || raw.startsWith("*** ")) break;
if (raw.startsWith("+")) {
contentLines.push(raw.slice(1));
if (raw === undefined || raw.startsWith("*** ")) {
break;
}
if (!raw.startsWith("+")) {
throw new Error(
`Invalid Add File line at ${i + 1}: expected '+' prefix`,
);
}
contentLines.push(raw.slice(1));
i += 1;
}
if (contentLines.length === 0) {
throw new Error(
`Add File for ${filePath} must include at least one + line`,
);
}
ops.push({ kind: "add", path: filePath, contentLines });
continue;
}
if (line.startsWith("*** Update File:")) {
const fromPath = line.replace("*** Update File:", "").trim();
assertRelativePatchPath(fromPath, "Update File");
i += 1;
let toPath: string | undefined;
@@ -91,6 +111,7 @@ export async function apply_patch(
const moveLine = lines[i];
if (moveLine?.startsWith("*** Move to:")) {
toPath = moveLine.replace("*** Move to:", "").trim();
assertRelativePatchPath(toPath, "Move to");
i += 1;
}
}
@@ -98,7 +119,9 @@ export async function apply_patch(
const hunks: Hunk[] = [];
while (i < endIndex) {
const hLine = lines[i];
if (hLine === undefined || hLine.startsWith("*** ")) break;
if (hLine === undefined || hLine.startsWith("*** ")) {
break;
}
if (hLine.startsWith("@@")) {
// Start of a new hunk
i += 1;
@@ -108,6 +131,10 @@ export async function apply_patch(
if (l === undefined || l.startsWith("@@") || l.startsWith("*** ")) {
break;
}
if (l === "*** End of File") {
i += 1;
break;
}
if (
l.startsWith(" ") ||
l.startsWith("+") ||
@@ -115,14 +142,19 @@ export async function apply_patch(
l === ""
) {
hunkLines.push(l);
} else {
throw new Error(
`Invalid hunk line at ${i + 1}: expected one of ' ', '+', '-'`,
);
}
i += 1;
}
hunks.push({ lines: hunkLines });
continue;
}
// Skip stray lines until next header/hunk
i += 1;
throw new Error(
`Invalid Update File body at ${i + 1}: expected '@@' hunk header`,
);
}
if (hunks.length === 0) {
@@ -135,21 +167,25 @@ export async function apply_patch(
if (line.startsWith("*** Delete File:")) {
const filePath = line.replace("*** Delete File:", "").trim();
assertRelativePatchPath(filePath, "Delete File");
ops.push({ kind: "delete", path: filePath });
i += 1;
continue;
}
// Unknown directive; skip
i += 1;
throw new Error(`Unknown patch directive at line ${i + 1}: ${line}`);
}
const cwd = process.cwd();
const cwd = process.env.USER_CWD || process.cwd();
const pendingWrites = new Map<string, string>();
const pendingDeletes = new Set<string>();
// Helper to get current content (including prior ops in this patch)
const loadFile = async (relativePath: string): Promise<string> => {
const abs = path.resolve(cwd, relativePath);
if (pendingDeletes.has(abs)) {
throw new Error(`File not found for update: ${relativePath}`);
}
const cached = pendingWrites.get(abs);
if (cached !== undefined) return cached;
@@ -175,6 +211,12 @@ export async function apply_patch(
for (const op of ops) {
if (op.kind === "add") {
const abs = path.resolve(cwd, op.path);
if (pendingWrites.has(abs)) {
throw new Error(`File already added/updated in patch: ${op.path}`);
}
if (!(await isMissing(abs))) {
throw new Error(`Cannot Add File that already exists: ${op.path}`);
}
const content = op.contentLines.join("\n");
pendingWrites.set(abs, content);
} else if (op.kind === "update") {
@@ -182,46 +224,20 @@ export async function apply_patch(
let content = await loadFile(currentPath);
for (const hunk of op.hunks) {
const { oldChunk, newChunk } = buildOldNewChunks(hunk.lines);
if (!oldChunk) {
continue;
}
const idx = content.indexOf(oldChunk);
if (idx === -1) {
throw new Error(
`Failed to apply hunk to ${currentPath}: context not found`,
);
}
content =
content.slice(0, idx) +
newChunk +
content.slice(idx + oldChunk.length);
content = applyHunk(content, hunk.lines, currentPath);
}
const targetPath = op.toPath ?? op.fromPath;
saveFile(targetPath, content);
// If file was renamed, also clear the old path so we don't write both
if (op.toPath && op.toPath !== op.fromPath) {
const oldAbs = path.resolve(cwd, op.fromPath);
if (pendingWrites.has(oldAbs)) {
pendingWrites.delete(oldAbs);
}
pendingWrites.delete(oldAbs);
pendingDeletes.add(oldAbs);
}
}
}
// Apply deletes on disk
for (const op of ops) {
if (op.kind === "delete") {
} else if (op.kind === "delete") {
const abs = path.resolve(cwd, op.path);
try {
await fs.unlink(abs);
} catch (error) {
const err = error as NodeJS.ErrnoException;
if (err.code !== "ENOENT") {
throw err;
}
}
pendingWrites.delete(abs);
pendingDeletes.add(abs);
}
}
@@ -232,11 +248,82 @@ export async function apply_patch(
await fs.writeFile(absPath, content, "utf8");
}
// Flush deletes after writes (for move semantics)
for (const absPath of pendingDeletes) {
if (pendingWrites.has(absPath)) continue;
if (await isMissing(absPath)) continue;
await fs.unlink(absPath);
}
return {
message: "Patch applied successfully",
};
}
function assertRelativePatchPath(patchPath: string, operation: string): void {
if (!patchPath) {
throw new Error(`${operation} path cannot be empty`);
}
if (path.isAbsolute(patchPath)) {
throw new Error(
`${operation} path must be relative (absolute paths are not allowed): ${patchPath}`,
);
}
}
async function isMissing(filePath: string): Promise<boolean> {
try {
await fs.access(filePath);
return false;
} catch {
return true;
}
}
function applyHunk(
content: string,
hunkLines: string[],
filePath: string,
): string {
const { oldChunk, newChunk } = buildOldNewChunks(hunkLines);
if (oldChunk.length === 0) {
throw new Error(
`Failed to apply hunk to ${filePath}: hunk has no anchor/context`,
);
}
const index = content.indexOf(oldChunk);
if (index !== -1) {
return (
content.slice(0, index) +
newChunk +
content.slice(index + oldChunk.length)
);
}
// Handle files that omit trailing newline
if (oldChunk.endsWith("\n")) {
const oldWithoutTrailingNewline = oldChunk.slice(0, -1);
const indexWithoutTrailingNewline = content.indexOf(
oldWithoutTrailingNewline,
);
if (indexWithoutTrailingNewline !== -1) {
const replacement = newChunk.endsWith("\n")
? newChunk.slice(0, -1)
: newChunk;
return (
content.slice(0, indexWithoutTrailingNewline) +
replacement +
content.slice(
indexWithoutTrailingNewline + oldWithoutTrailingNewline.length,
)
);
}
}
throw new Error(`Failed to apply hunk to ${filePath}: context not found`);
}
function buildOldNewChunks(lines: string[]): {
oldChunk: string;
newChunk: string;

9
src/tools/impl/ShellCommand.ts
View File

@@ -6,9 +6,11 @@ import { validateRequiredParams } from "./validation.js";
interface ShellCommandArgs {
command: string;
workdir?: string;
login?: boolean;
timeout_ms?: number;
with_escalated_permissions?: boolean;
sandbox_permissions?: "use_default" | "require_escalated";
justification?: string;
prefix_rule?: string[];
signal?: AbortSignal;
onOutput?: (chunk: string, stream: "stdout" | "stderr") => void;
}
@@ -31,13 +33,13 @@ export async function shell_command(
const {
command,
workdir,
login = true,
timeout_ms,
with_escalated_permissions,
justification,
signal,
onOutput,
} = args;
const launchers = buildShellLaunchers(command);
const launchers = buildShellLaunchers(command, { login });
if (launchers.length === 0) {
throw new Error("Command must be a non-empty string");
}
@@ -51,7 +53,6 @@ export async function shell_command(
command: launcher,
workdir,
timeout_ms,
with_escalated_permissions,
justification,
signal,
onOutput,

78
src/tools/impl/shellLaunchers.ts
View File

@@ -1,4 +1,7 @@
const SEP = "\u0000";
type ShellLaunchOptions = {
login?: boolean;
};
function pushUnique(
list: string[][],
@@ -53,7 +56,16 @@ function windowsLaunchers(command: string): string[][] {
return launchers;
}
function unixLaunchers(command: string): string[][] {
function shellCommandFlag(shellName: string, login: boolean): string {
if (!login) return "-c";
const normalized = shellName.replace(/\\/g, "/").toLowerCase();
if (normalized.includes("bash") || normalized.includes("zsh")) {
return "-lc";
}
return "-c";
}
function unixLaunchers(command: string, login: boolean): string[][] {
const trimmed = command.trim();
if (!trimmed) return [];
const launchers: string[][] = [];
@@ -62,42 +74,50 @@ function unixLaunchers(command: string): string[][] {
// On macOS, ALWAYS prefer zsh first due to bash 3.2's HEREDOC parsing bug
// with odd numbers of apostrophes. This takes precedence over $SHELL.
if (process.platform === "darwin") {
pushUnique(launchers, seen, ["/bin/zsh", "-c", trimmed]);
pushUnique(launchers, seen, [
"/bin/zsh",
shellCommandFlag("/bin/zsh", login),
trimmed,
]);
}
// Try user's preferred shell from $SHELL environment variable
// Use -c (non-login) to avoid profile sourcing that can hang on CI
// Use login semantics only when explicitly requested.
const envShell = process.env.SHELL?.trim();
if (envShell) {
pushUnique(launchers, seen, [envShell, "-c", trimmed]);
pushUnique(launchers, seen, [
envShell,
shellCommandFlag(envShell, login),
trimmed,
]);
}
// Fallback defaults - prefer simple "bash" PATH lookup first (like original code)
// then absolute paths. Use -c (non-login shell) to avoid profile sourcing.
// Fallback defaults - prefer simple "bash" PATH lookup first (like original code),
// then absolute paths.
const defaults: string[][] =
process.platform === "darwin"
? [
["/bin/zsh", "-c", trimmed],
["bash", "-c", trimmed], // PATH lookup, like original
["/bin/bash", "-c", trimmed],
["/usr/bin/bash", "-c", trimmed],
["/bin/sh", "-c", trimmed],
["/bin/ash", "-c", trimmed],
["/usr/bin/env", "zsh", "-c", trimmed],
["/usr/bin/env", "bash", "-c", trimmed],
["/usr/bin/env", "sh", "-c", trimmed],
["/usr/bin/env", "ash", "-c", trimmed],
["/bin/zsh", shellCommandFlag("/bin/zsh", login), trimmed],
["bash", shellCommandFlag("bash", login), trimmed], // PATH lookup, like original
["/bin/bash", shellCommandFlag("/bin/bash", login), trimmed],
["/usr/bin/bash", shellCommandFlag("/usr/bin/bash", login), trimmed],
["/bin/sh", shellCommandFlag("/bin/sh", login), trimmed],
["/bin/ash", shellCommandFlag("/bin/ash", login), trimmed],
["/usr/bin/env", "zsh", shellCommandFlag("zsh", login), trimmed],
["/usr/bin/env", "bash", shellCommandFlag("bash", login), trimmed],
["/usr/bin/env", "sh", shellCommandFlag("sh", login), trimmed],
["/usr/bin/env", "ash", shellCommandFlag("ash", login), trimmed],
]
: [
["/bin/bash", "-c", trimmed],
["/usr/bin/bash", "-c", trimmed],
["/bin/zsh", "-c", trimmed],
["/bin/sh", "-c", trimmed],
["/bin/ash", "-c", trimmed],
["/usr/bin/env", "bash", "-c", trimmed],
["/usr/bin/env", "zsh", "-c", trimmed],
["/usr/bin/env", "sh", "-c", trimmed],
["/usr/bin/env", "ash", "-c", trimmed],
["/bin/bash", shellCommandFlag("/bin/bash", login), trimmed],
["/usr/bin/bash", shellCommandFlag("/usr/bin/bash", login), trimmed],
["/bin/zsh", shellCommandFlag("/bin/zsh", login), trimmed],
["/bin/sh", shellCommandFlag("/bin/sh", login), trimmed],
["/bin/ash", shellCommandFlag("/bin/ash", login), trimmed],
["/usr/bin/env", "bash", shellCommandFlag("bash", login), trimmed],
["/usr/bin/env", "zsh", shellCommandFlag("zsh", login), trimmed],
["/usr/bin/env", "sh", shellCommandFlag("sh", login), trimmed],
["/usr/bin/env", "ash", shellCommandFlag("ash", login), trimmed],
];
for (const entry of defaults) {
pushUnique(launchers, seen, entry);
@@ -105,8 +125,12 @@ function unixLaunchers(command: string): string[][] {
return launchers;
}
export function buildShellLaunchers(command: string): string[][] {
export function buildShellLaunchers(
command: string,
options?: ShellLaunchOptions,
): string[][] {
const login = options?.login ?? false;
return process.platform === "win32"
? windowsLaunchers(command)
: unixLaunchers(command);
: unixLaunchers(command, login);
}