fix(plan): avoid first-try plan-file path misses in codex apply_patch (#1187)
This commit is contained in:
@@ -2,7 +2,7 @@
|
||||
|
||||
import { existsSync, readFileSync, renameSync, writeFileSync } from "node:fs";
|
||||
import { homedir, tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { join, relative } from "node:path";
|
||||
import { APIError, APIUserAbortError } from "@letta-ai/letta-client/core/error";
|
||||
import type {
|
||||
AgentState,
|
||||
@@ -672,13 +672,16 @@ function getPlanModeReminder(): string {
|
||||
}
|
||||
|
||||
const planFilePath = permissionMode.getPlanFilePath();
|
||||
const applyPatchRelativePath = planFilePath
|
||||
? relative(process.cwd(), planFilePath).replace(/\\/g, "/")
|
||||
: null;
|
||||
|
||||
// Generate dynamic reminder with plan file path
|
||||
return `${SYSTEM_REMINDER_OPEN}
|
||||
Plan mode is active. The user indicated that they do not want you to execute yet -- you MUST NOT make any edits (with the exception of the plan file mentioned below), run any non-readonly tools (including changing configs or making commits), or otherwise make any changes to the system. This supersedes any other instructions you have received.
|
||||
|
||||
## Plan File Info:
|
||||
${planFilePath ? `No plan file exists yet. You should create your plan at ${planFilePath} using a write tool (e.g. Write, ApplyPatch, etc. depending on your toolset).` : "No plan file path assigned."}
|
||||
${planFilePath ? `No plan file exists yet. You should create your plan at ${planFilePath} using a write tool (e.g. Write, ApplyPatch, etc. depending on your toolset).\n${applyPatchRelativePath ? `If using apply_patch, use this exact relative patch path: ${applyPatchRelativePath}.` : ""}` : "No plan file path assigned."}
|
||||
|
||||
You should build your plan incrementally by writing to or editing this file. NOTE that this is the only file you are allowed to edit - other than this you are only allowed to take READ-ONLY actions.
|
||||
|
||||
@@ -12030,6 +12033,10 @@ ${SYSTEM_REMINDER_CLOSE}
|
||||
|
||||
// Generate plan file path
|
||||
const planFilePath = generatePlanFilePath();
|
||||
const applyPatchRelativePath = relative(
|
||||
process.cwd(),
|
||||
planFilePath,
|
||||
).replace(/\\/g, "/");
|
||||
|
||||
// Toggle plan mode on and store plan file path
|
||||
permissionMode.setMode("plan");
|
||||
@@ -12049,7 +12056,8 @@ In plan mode, you should:
|
||||
|
||||
Remember: DO NOT write or edit any files yet. This is a read-only exploration and planning phase.
|
||||
|
||||
Plan file path: ${planFilePath}`;
|
||||
Plan file path: ${planFilePath}
|
||||
If using apply_patch, use this exact relative patch path: ${applyPatchRelativePath}`;
|
||||
|
||||
const precomputedResult: ToolExecutionResult = {
|
||||
toolReturn,
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// src/permissions/checker.ts
|
||||
// Main permission checking logic
|
||||
|
||||
import { resolve } from "node:path";
|
||||
import { relative, resolve } from "node:path";
|
||||
import { getCurrentAgentId } from "../agent/context";
|
||||
import { runPermissionRequestHooks } from "../hooks";
|
||||
import { canonicalToolName, isShellToolName } from "./canonical";
|
||||
@@ -308,9 +308,15 @@ function checkPermissionForEngine(
|
||||
let reason = `Permission mode: ${currentMode}`;
|
||||
if (currentMode === "plan" && modeOverride === "deny") {
|
||||
const planFilePath = permissionMode.getPlanFilePath();
|
||||
const applyPatchRelativePath = planFilePath
|
||||
? relative(workingDirectory, planFilePath).replace(/\\/g, "/")
|
||||
: null;
|
||||
reason =
|
||||
`Plan mode is active. You can only use read-only tools (Read, Grep, Glob, etc.) and write to the plan file. ` +
|
||||
`Write your plan to: ${planFilePath || "(error: plan file path not configured)"}. ` +
|
||||
(applyPatchRelativePath
|
||||
? `If using apply_patch, use this exact relative path in patch headers: ${applyPatchRelativePath}. `
|
||||
: "") +
|
||||
`Use ExitPlanMode when your plan is ready for user approval.`;
|
||||
}
|
||||
traceEvent(trace, "mode-override", reason);
|
||||
|
||||
@@ -279,6 +279,35 @@ test("plan mode - denies Write", () => {
|
||||
expect(result.reason).toContain("Plan mode is active");
|
||||
});
|
||||
|
||||
test("plan mode deny reason includes exact apply_patch relative path hint", () => {
|
||||
permissionMode.setMode("plan");
|
||||
const workingDirectory = join(homedir(), "dev", "repo");
|
||||
const planPath = join(homedir(), ".letta", "plans", "unit-test-plan.md");
|
||||
const expectedRelativePath = relative(workingDirectory, planPath).replace(
|
||||
/\\/g,
|
||||
"/",
|
||||
);
|
||||
permissionMode.setPlanFilePath(planPath);
|
||||
|
||||
const permissions: PermissionRules = {
|
||||
allow: [],
|
||||
deny: [],
|
||||
ask: [],
|
||||
};
|
||||
|
||||
const result = checkPermission(
|
||||
"Write",
|
||||
{ file_path: "/tmp/test.txt" },
|
||||
permissions,
|
||||
workingDirectory,
|
||||
);
|
||||
|
||||
expect(result.decision).toBe("deny");
|
||||
expect(result.reason).toContain(
|
||||
`If using apply_patch, use this exact relative path in patch headers: ${expectedRelativePath}.`,
|
||||
);
|
||||
});
|
||||
|
||||
test("plan mode - allows Write to plan markdown file", () => {
|
||||
permissionMode.setMode("plan");
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { relative } from "node:path";
|
||||
import { generatePlanFilePath } from "../../cli/helpers/planName";
|
||||
import { permissionMode } from "../../permissions/mode";
|
||||
|
||||
@@ -26,6 +27,10 @@ export async function enter_plan_mode(
|
||||
}
|
||||
|
||||
const planFilePath = permissionMode.getPlanFilePath();
|
||||
const cwd = process.env.USER_CWD || process.cwd();
|
||||
const applyPatchRelativePath = planFilePath
|
||||
? relative(cwd, planFilePath).replace(/\\/g, "/")
|
||||
: null;
|
||||
|
||||
return {
|
||||
message: `Entered plan mode. You should now focus on exploring the codebase and designing an implementation approach.
|
||||
@@ -40,6 +45,7 @@ In plan mode, you should:
|
||||
|
||||
Remember: DO NOT write or edit any files yet. This is a read-only exploration and planning phase.
|
||||
|
||||
Plan file path: ${planFilePath}`,
|
||||
Plan file path: ${planFilePath}
|
||||
${applyPatchRelativePath ? `If using apply_patch, use this exact relative patch path: ${applyPatchRelativePath}` : ""}`,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user