mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 12:40:32 +00:00
fix: offer bypass alongside auto in plan approval (#1164)
Supersedes #1140 by @s-cork. Their commit is kept as-is (rebased onto `main`), plus one follow-up commit. The fork is org-owned, so maintainers can't push to it. ## Problem ExitPlanMode always led with Auto and dropped bypass, even when the session advertised `bypassPermissions`. Clients that deliberately run in full-access mode lost that choice at plan approval. Claude Code itself offers one elevated choice and prefers bypass over Auto. It can do that because the CLI only offers bypass to users who launched with it. The adapter advertises bypass for every non-root session, so copying that priority would hide Auto for almost everyone. ## Change - Offer both Auto and bypass whenever the session advertises them (@s-cork's commit). - Follow-up: remember the mode the session left when it entered plan mode (`SessionModeManager.syncConfig`, which every mode change goes through, including plan approval since #1163). If that was `bypassPermissions`, bypass comes first and is also what the clear-context option uses. Otherwise Auto leads, as before. A remembered bypass mode is ignored if the session no longer advertises it. This also fills the gap where the clear-context choice only ever came in the Auto flavour. Partially addresses #1077. ## Validation - `npm run build`, `npm run check` and `npm run test:run` pass. - New tests cover the pre-plan mode tracking and the option order with and without a pre-plan bypass mode, including the clear-context row. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: stu <stucork@gmail.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -822,6 +822,8 @@ export type Session = {
|
||||
* (see resumedFirstResultModelUsage). */
|
||||
lastModelUsageReading?: ModelTokenTally;
|
||||
modes: SessionModeState;
|
||||
/** The mode the session left when it entered plan mode, if it is in plan. */
|
||||
prePlanMode?: string;
|
||||
models: SessionModelState;
|
||||
modelInfos: ModelInfo[];
|
||||
/** Prevents the model-specific Auto fallback from spamming the transcript. */
|
||||
@@ -7248,6 +7250,7 @@ export class ClaudeAcpAgent {
|
||||
allowPersistentOptions: !noPersistentRule,
|
||||
defaultToNo,
|
||||
availableModes: this.sessionModes.availableModeIds(session.modes),
|
||||
prePlanMode: session.prePlanMode,
|
||||
contextUsedPercent:
|
||||
session.contextUsedTokens === undefined || session.contextWindowSize <= 0
|
||||
? undefined
|
||||
|
||||
@@ -27,6 +27,8 @@ export interface PermissionOptionContext {
|
||||
* approvable by a stray keystroke, so the decline option leads. */
|
||||
defaultToNo?: boolean;
|
||||
availableModes?: readonly string[];
|
||||
/** The mode the session left when it entered plan mode. */
|
||||
prePlanMode?: string;
|
||||
contextUsedPercent?: number;
|
||||
}
|
||||
|
||||
|
||||
@@ -60,47 +60,55 @@ export function buildExitPlanModePermissionOptions(
|
||||
): PermissionOption[] {
|
||||
const modes = new Set(context.availableModes);
|
||||
const options: PermissionOption[] = [];
|
||||
const elevatedMode = modes.has("auto")
|
||||
? "auto"
|
||||
: modes.has("bypassPermissions")
|
||||
? "bypassPermissions"
|
||||
: "acceptEdits";
|
||||
// Claude Code offers bypass only when the user launched with it, so it can
|
||||
// prefer bypass outright. The adapter advertises bypass by default, so Auto
|
||||
// leads unless the session was in bypass before entering plan mode. Both stay
|
||||
// selectable: they have different permission semantics.
|
||||
const elevatedModes = (
|
||||
context.prePlanMode === "bypassPermissions"
|
||||
? (["bypassPermissions", "auto"] as const)
|
||||
: (["auto", "bypassPermissions"] as const)
|
||||
).filter((mode) => modes.has(mode));
|
||||
const preferredMode = elevatedModes[0] ?? "acceptEdits";
|
||||
if (plainString(context.input.plan)) {
|
||||
const usage =
|
||||
context.contextUsedPercent === undefined ? "" : ` (${context.contextUsedPercent}% used)`;
|
||||
if (elevatedMode === "auto") {
|
||||
options.push({
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanClearAuto,
|
||||
name: `Yes, clear context${usage} and use auto mode`,
|
||||
kind: "allow_always",
|
||||
});
|
||||
} else if (elevatedMode === "bypassPermissions") {
|
||||
options.push({
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanClearBypass,
|
||||
name: `Yes, clear context${usage} and bypass permissions`,
|
||||
kind: "allow_always",
|
||||
});
|
||||
} else {
|
||||
options.push({
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanClearAcceptEdits,
|
||||
name: `Yes, clear context${usage} and auto-accept edits`,
|
||||
kind: "allow_always",
|
||||
});
|
||||
}
|
||||
options.push(
|
||||
preferredMode === "auto"
|
||||
? {
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanClearAuto,
|
||||
name: `Yes, clear context${usage} and use auto mode`,
|
||||
kind: "allow_always",
|
||||
}
|
||||
: preferredMode === "bypassPermissions"
|
||||
? {
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanClearBypass,
|
||||
name: `Yes, clear context${usage} and bypass permissions`,
|
||||
kind: "allow_always",
|
||||
}
|
||||
: {
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanClearAcceptEdits,
|
||||
name: `Yes, clear context${usage} and auto-accept edits`,
|
||||
kind: "allow_always",
|
||||
},
|
||||
);
|
||||
}
|
||||
if (elevatedMode === "auto") {
|
||||
options.push({
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanAuto,
|
||||
name: "Yes, and use auto mode",
|
||||
kind: "allow_always",
|
||||
});
|
||||
} else if (elevatedMode === "bypassPermissions") {
|
||||
options.push({
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanBypass,
|
||||
name: "Yes, and bypass permissions",
|
||||
kind: "allow_always",
|
||||
});
|
||||
} else {
|
||||
for (const mode of elevatedModes) {
|
||||
options.push(
|
||||
mode === "auto"
|
||||
? {
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanAuto,
|
||||
name: "Yes, and use auto mode",
|
||||
kind: "allow_always",
|
||||
}
|
||||
: {
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanBypass,
|
||||
name: "Yes, and bypass permissions",
|
||||
kind: "allow_always",
|
||||
},
|
||||
);
|
||||
}
|
||||
if (elevatedModes.length === 0) {
|
||||
options.push({
|
||||
optionId: PERMISSION_OPTION_ID.exitPlanAcceptEdits,
|
||||
name: "Yes, auto-accept edits",
|
||||
|
||||
@@ -42,6 +42,8 @@ export type SessionMode = {
|
||||
autoModeFallbackWarningShown?: boolean;
|
||||
/** Initial mode fallback is reported after session/new, on the first prompt. */
|
||||
autoModeFallbackWarningPending?: boolean;
|
||||
/** The mode the session left when it entered plan mode, if it is in plan. */
|
||||
prePlanMode?: string;
|
||||
};
|
||||
|
||||
export type SessionModeManagerOptions<S extends SessionMode> = {
|
||||
@@ -132,6 +134,12 @@ export class SessionModeManager<S extends SessionMode> {
|
||||
}
|
||||
|
||||
syncConfig(session: ModeConfigSession, mode: string): void {
|
||||
const previousMode = session.modes.currentModeId;
|
||||
if (mode !== "plan") {
|
||||
session.prePlanMode = undefined;
|
||||
} else if (previousMode !== "plan") {
|
||||
session.prePlanMode = previousMode;
|
||||
}
|
||||
session.modes = { ...session.modes, currentModeId: mode };
|
||||
session.configOptions = session.configOptions.map((option) =>
|
||||
option.id === MODE_CONFIG_ID && typeof option.currentValue === "string"
|
||||
|
||||
@@ -17,6 +17,7 @@ describe("Claude permission options and response mapping", () => {
|
||||
allowPersistentOptions = true,
|
||||
availableModes: readonly string[] = [],
|
||||
contextUsedPercent?: number,
|
||||
prePlanMode?: string,
|
||||
) =>
|
||||
buildClaudePermissionOptions({
|
||||
toolName,
|
||||
@@ -27,6 +28,7 @@ describe("Claude permission options and response mapping", () => {
|
||||
allowPersistentOptions,
|
||||
availableModes,
|
||||
contextUsedPercent,
|
||||
prePlanMode,
|
||||
});
|
||||
|
||||
it("leads with the reject options when the CLI defaults the ask to no", () => {
|
||||
@@ -485,7 +487,7 @@ describe("Claude permission options and response mapping", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("offers only the highest-priority elevated ExitPlanMode choice", () => {
|
||||
it("offers bypass alongside Auto when both modes are available", () => {
|
||||
const options = build("ExitPlanMode", undefined, {}, undefined, true, [
|
||||
"auto",
|
||||
"default",
|
||||
@@ -495,11 +497,56 @@ describe("Claude permission options and response mapping", () => {
|
||||
expect(options).toMatchObject([
|
||||
{ optionId: PERMISSION_OPTION_ID.exitPlanDefault, name: "Yes, manually approve edits" },
|
||||
{ optionId: PERMISSION_OPTION_ID.exitPlanAuto, name: "Yes, and use auto mode" },
|
||||
{ optionId: PERMISSION_OPTION_ID.exitPlanBypass, name: "Yes, and bypass permissions" },
|
||||
{ optionId: PERMISSION_OPTION_ID.reject, name: "No, keep planning" },
|
||||
]);
|
||||
expect(options[2]?._meta).toBeUndefined();
|
||||
});
|
||||
|
||||
it("leads with bypass when the session was in bypass before entering plan mode", () => {
|
||||
const modes = ["auto", "default", "acceptEdits", "bypassPermissions"];
|
||||
expect(
|
||||
build("ExitPlanMode", undefined, {}, undefined, true, modes, undefined, "bypassPermissions"),
|
||||
).toMatchObject([
|
||||
{ optionId: PERMISSION_OPTION_ID.exitPlanDefault, name: "Yes, manually approve edits" },
|
||||
{ optionId: PERMISSION_OPTION_ID.exitPlanBypass, name: "Yes, and bypass permissions" },
|
||||
{ optionId: PERMISSION_OPTION_ID.exitPlanAuto, name: "Yes, and use auto mode" },
|
||||
{ optionId: PERMISSION_OPTION_ID.reject, name: "No, keep planning" },
|
||||
]);
|
||||
expect(
|
||||
build(
|
||||
"ExitPlanMode",
|
||||
undefined,
|
||||
{ plan: "Implement it" },
|
||||
undefined,
|
||||
true,
|
||||
modes,
|
||||
73,
|
||||
"bypassPermissions",
|
||||
).map((option) => option.optionId),
|
||||
).toEqual(expect.arrayContaining([PERMISSION_OPTION_ID.exitPlanClearBypass]));
|
||||
});
|
||||
|
||||
it("ignores a pre-plan bypass mode the session no longer advertises", () => {
|
||||
expect(
|
||||
build(
|
||||
"ExitPlanMode",
|
||||
undefined,
|
||||
{ plan: "Implement it" },
|
||||
undefined,
|
||||
true,
|
||||
["auto", "acceptEdits"],
|
||||
73,
|
||||
"bypassPermissions",
|
||||
).map((option) => option.optionId),
|
||||
).toEqual([
|
||||
PERMISSION_OPTION_ID.exitPlanDefault,
|
||||
PERMISSION_OPTION_ID.exitPlanClearAuto,
|
||||
PERMISSION_OPTION_ID.exitPlanAuto,
|
||||
PERMISSION_OPTION_ID.reject,
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
[
|
||||
["bypassPermissions", "acceptEdits"],
|
||||
|
||||
@@ -201,6 +201,25 @@ describe("session mode", () => {
|
||||
expect(session.configOptions[0].currentValue).toBe("plan");
|
||||
});
|
||||
|
||||
it("remembers the mode the session left when it entered plan mode", () => {
|
||||
const session = {
|
||||
...createSession(),
|
||||
configOptions: [SessionModeManager.configOption(createSession().modes)],
|
||||
};
|
||||
const { manager } = createManager(session);
|
||||
|
||||
manager.syncConfig(session, "bypassPermissions");
|
||||
manager.syncConfig(session, "plan");
|
||||
expect(session.prePlanMode).toBe("bypassPermissions");
|
||||
|
||||
// A repeated plan sync (e.g. EnterPlanMode while already planning) keeps it.
|
||||
manager.syncConfig(session, "plan");
|
||||
expect(session.prePlanMode).toBe("bypassPermissions");
|
||||
|
||||
manager.syncConfig(session, "auto");
|
||||
expect(session.prePlanMode).toBeUndefined();
|
||||
});
|
||||
|
||||
it("reconciles Auto after a model switch", async () => {
|
||||
const session = createSession();
|
||||
session.modes.currentModeId = "auto";
|
||||
|
||||
Reference in New Issue
Block a user