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

fix: always show detailed plan mode denial message (#386)

Browse Source 
Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
Charles Packer
2025-12-24 14:59:29 -08:00
committed by GitHub
parent 552a006e97
commit a4fa3023c1
3 changed files with 24 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/cli/App.tsx
View File

@@ -1222,10 +1222,13 @@ export default function App({
// Create denial results for auto-denied tools and update buffers
const autoDeniedResults = autoDenied.map((ac) => {
const reason =
"matchedRule" in ac.permission && ac.permission.matchedRule
// Prefer the detailed reason over the short matchedRule name
// (e.g., reason contains plan file path info, matchedRule is just "plan mode")
const reason = ac.permission.reason
? `Permission denied: ${ac.permission.reason}`
: "matchedRule" in ac.permission && ac.permission.matchedRule
? `Permission denied by rule: ${ac.permission.matchedRule}`
: `Permission denied: ${ac.permission.reason || "Unknown reason"}`;
: "Permission denied: Unknown reason";
// Update buffers with tool rejection for UI
onChunk(buffersRef.current, {
@@ -3623,10 +3626,12 @@ DO NOT respond to these messages or otherwise consider them in your response unl
// Create denial results for auto-denied and update UI
const autoDeniedResults = autoDenied.map((ac) => {
const reason =
"matchedRule" in ac.permission && ac.permission.matchedRule
// Prefer the detailed reason over the short matchedRule name
const reason = ac.permission.reason
? `Permission denied: ${ac.permission.reason}`
: "matchedRule" in ac.permission && ac.permission.matchedRule
? `Permission denied by rule: ${ac.permission.matchedRule}`
: `Permission denied: ${ac.permission.reason || "Unknown"}`;
: "Permission denied: Unknown";
// Update buffers with denial for UI
onChunk(buffersRef.current, {
@@ -3704,10 +3709,12 @@ DO NOT respond to these messages or otherwise consider them in your response unl
// Create denial reasons for auto-denied and update UI
const autoDeniedWithReasons = autoDenied.map((ac) => {
const reason =
"matchedRule" in ac.permission && ac.permission.matchedRule
// Prefer the detailed reason over the short matchedRule name
const reason = ac.permission.reason
? `Permission denied: ${ac.permission.reason}`
: "matchedRule" in ac.permission && ac.permission.matchedRule
? `Permission denied by rule: ${ac.permission.matchedRule}`
: `Permission denied: ${ac.permission.reason || "Unknown"}`;
: "Permission denied: Unknown";
// Update buffers with denial for UI
onChunk(buffersRef.current, {

11
src/permissions/checker.ts
View File

@@ -120,12 +120,11 @@ export function checkPermission(
let reason = `Permission mode: ${currentMode}`;
if (currentMode === "plan" && modeOverride === "deny") {
const planFilePath = permissionMode.getPlanFilePath();
if (planFilePath) {
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}. ` +
`Use ExitPlanMode when your plan is ready for user approval.`;
}
// planFilePath should always be set when in plan mode - they're set together
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)"}. ` +
`Use ExitPlanMode when your plan is ready for user approval.`;
}
return {
decision: modeOverride,

5
src/tests/permissions-mode.test.ts
View File

@@ -273,7 +273,8 @@ test("plan mode - denies Write", () => {
expect(result.decision).toBe("deny");
expect(result.matchedRule).toBe("plan mode");
expect(result.reason).toBe("Permission mode: plan");
// Reason now includes detailed guidance (planFilePath not set in test, so shows error fallback)
expect(result.reason).toContain("Plan mode is active");
});
test("plan mode - denies Bash", () => {
@@ -362,7 +363,7 @@ test("Permission mode takes precedence over CLI allowedTools", () => {
// Permission mode denies take precedence over CLI allowedTools
expect(result.decision).toBe("deny");
expect(result.reason).toBe("Permission mode: plan");
expect(result.reason).toContain("Plan mode is active");
// Clean up
cliPermissions.clear();