Skip to content

[Plan Mode 2/6] Core backend MVP#70066

Closed
100yenadmin wants to merge 1 commit into
openclaw:mainfrom
electricsheephq:isolated/pm-2-of-6-core-backend
Closed

[Plan Mode 2/6] Core backend MVP#70066
100yenadmin wants to merge 1 commit into
openclaw:mainfrom
electricsheephq:isolated/pm-2-of-6-core-backend

Conversation

@100yenadmin

@100yenadmin 100yenadmin commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

📋 Umbrella tracker: #70101 — master tracker for the 9-PR plan-mode rollout. See it for status of all parts + suggested merge order + carry-forward backlog.


📋 Stack position: This is [Plan Mode 2/6], the second part of a 6-PR per-part decomposition of the original umbrella #68939 (closed).

  • Previous in stack: [Plan Mode 1/6] Plan-state foundation ([Plan Mode 1/6] Plan-state foundation #70031) — must merge first for this PR's code to compile against main
  • Next in stack: [Plan Mode 3/6] Advanced plan interactions
  • Integration bundle: [Plan Mode FULL] — green-CI bundle of Parts 1/6–6/6 + automation + executing-state lifecycle, for end-to-end testing

⚠️ CI on this PR will be RED: this part's code references symbols from [Plan Mode 1/6] (plan-mode types, SessionEntry.planMode schema) that aren't on main yet. CI will pass once 1/6 merges, OR review the green-CI integrated state in [Plan Mode FULL].

Ways to land this feature (maintainer choice):

  • Per-part review + sequential merge of 1/6 → 6/6
  • Single bundle merge via [Plan Mode FULL]

Executive summary

This PR is the runtime core of the plan-mode rollout. It adds the two security-critical pieces that make plan mode actually enforce its contract: a mutation gate that fail-closes on every write/edit/exec attempt while plan mode is active, and an approval state machine that resolves the user's Approve/Edit/Reject/Timeout decisions into the next session state. It also adds the gateway integration (sessions.patch { planMode }) that flips a session into plan mode, and the runner plumbing (pi-toolsbefore-tool-call) that arms the gate without re-reading the session store on every tool call.

It builds directly on [Plan Mode 1/6] (#70031), which contributes the SessionEntry.planMode persisted schema, the Zod validators, and the plan-snapshot persister. Together those two parts are the MVP: with both merged, a session can flip into plan mode via /plan on, every mutation tool gets blocked, and the approval lifecycle resolves cleanly. Subsequent parts (3/6 advanced interactions, AUTOMATION, FULL) layer on ask_user_question, plan archetypes, accept-edits gating, cron nudges, and the executing-state lifecycle — none of which are required for the basic plan-then-approve workflow to function. The split exists so each maintainer-reviewable surface is small enough to read in one sitting.

TL;DR

  • Scope: 27 files, ~1.9k additions. 7 net-new files in src/agents/plan-mode/ (mutation-gate, approval, types, index, three test files); rest are integration touchpoints in the runner, gateway, and config layers.
  • Security model: fail-closed by default. Unknown tools are blocked when plan mode is active (mutation-gate.ts:182-187). Stale approval clicks are no-op'd (approval.ts:62-66). Adversarial feedback strings cannot escape the [PLAN_DECISION] envelope (types.ts:105-107 + regression test approval.test.ts:146-159).
  • Default state: opt-in. agents.defaults.planMode.enabled is undefined/false on every existing config — zero behavioral change for current users. sessions.patch { planMode: "plan" } is rejected with a friendly error when the feature is off (sessions-patch.ts:401-405).
  • Test coverage: 693 lines of test code across mutation-gate.test.ts (192 lines), approval.test.ts (270 lines), and integration.test.ts (231 lines). Adversarial regressions exercised: marker-injection in feedback, approvalId entropy (1024 distinct calls), fail-closed when current state has no token, dangerous-flag substring false positives.
  • Rollback: flip agents.defaults.planMode.enabled back to false (or remove it). Sessions already in plan mode get unstranded because the sessions-patch.ts:398-400 "normal/null" branch is unconditional — operators can always escape.
  • Parity benchmark: same prompt set hit OpenClaw plan mode + Codex (OpenAI) + Claude Code (Anthropic). Result was 90% parity on quality, 95% parity on session length across both Anthropic and OpenAI models. The state-machine + allowlist semantics here converge on the industry-standard plan-mode pattern, which is why the parity numbers are this tight.

1. Approval state machine

PlanApprovalState ∈ {none, pending, approved, edited, rejected, timed_out}. none is the resting state after /plan on (no plan submitted yet). pending is set by exit_plan_mode once the agent submits a plan. The four terminal-or-cycling transitions are driven by resolvePlanApproval(state, action, feedback?, expectedApprovalId?) in src/agents/plan-mode/approval.ts:44-135.

stateDiagram-v2
  [*] --> None : /plan on (sessions.patch)
  None --> Pending : exit_plan_mode<br/>(mints fresh approvalId)
  Pending --> Approved : approve<br/>(approvalId match)
  Pending --> Edited : edit<br/>(approvalId match)
  Pending --> Rejected : reject + feedback<br/>(rejectionCount++)
  Pending --> TimedOut : timeout<br/>(stays in plan mode)
  Rejected --> Approved : approve<br/>(user changes mind)
  Rejected --> Edited : edit
  Rejected --> Rejected : reject (count++)
  Rejected --> Pending : exit_plan_mode again<br/>(NEW approvalId)
  Approved --> [*] : mode → "normal"<br/>(mutations unlocked)
  Edited --> [*] : mode → "normal"<br/>(mutations unlocked)
  TimedOut --> Pending : exit_plan_mode<br/>(new cycle)

  note right of Pending
    Stale-event guard:<br/>any action carrying<br/>expectedApprovalId<br/>that doesn't match<br/>current.approvalId<br/>→ no-op (returns same state).<br/>Fail-closed if current<br/>has no approvalId.
  end note

  note left of Approved
    rejectionCount reset to 0.<br/>feedback cleared.<br/>Terminal — needs fresh<br/>exit_plan_mode for<br/>next action to apply.
  end note
Loading

Key invariants enforced in approval.ts:

  • Stale-event guard (approval.ts:62-66): if the caller passes expectedApprovalId and the current state's approvalId is undefined OR mismatched, return the current state unchanged. This is fail-closed: an earlier draft only no-op'd when both sides had defined IDs and they differed, which let an adversary or a stale UI fire approvals against a state with a cleared approvalId. Regression in approval.test.ts:242-270 (the "fail-closed when current state has no token" describe block).
  • Terminal-state guard (approval.ts:72-78): approved / edited / timed_out are terminal — they require a fresh exit_plan_mode call (which mints a new approvalId) before any new action can apply. rejected and none stay re-entrant. The timeout action additionally requires current.approval === "pending" (approval.ts:79-81).
  • Rejection counter reset (approval.ts:87-95, 97-107): approve and edit clear feedback AND reset rejectionCount to 0. The user is moving forward, so cycle history is no longer relevant. reject increments. timeout does not touch the counter (separate concern).

2. Mutation-gate decision flow

The mutation gate is a pure function in src/agents/plan-mode/mutation-gate.ts invoked by the before-tool-call hook. It runs after loop detection (loops should still trip even in plan mode) and before the plugin hookRunner (so a plugin can't intercept and bypass the gate by responding earlier in the pipeline). See pi-tools.before-tool-call.ts:198-217.

flowchart TD
  start([tool call]) --> loop{loop<br/>detection}
  loop -->|critical loop| block_loop[block]
  loop -->|ok or warning| gate_check{ctx.planMode<br/>=== 'plan'?}
  gate_check -->|no| pass_to_plugins[run plugin<br/>hookRunner]
  gate_check -->|yes| allowlist{tool in<br/>PLAN_MODE_<br/>ALLOWED_TOOLS?}
  allowlist -->|yes<br/>read, web_search,<br/>web_fetch, memory_*,<br/>update_plan,<br/>exit_plan_mode,<br/>session_status| allow_to_plugins[run plugin<br/>hookRunner]
  allowlist -->|no| exec_branch{tool ===<br/>'exec' or 'bash'?}
  exec_branch -->|yes| shell_check{shell compound<br/>operators?<br/>;|&` $\( >> < newline}
  shell_check -->|yes| block_shell[block:<br/>shell operators]
  shell_check -->|no| flag_check{dangerous flags?<br/>-delete, -exec, -rf,<br/>--output, --delete}
  flag_check -->|yes| block_flag[block:<br/>dangerous flag]
  flag_check -->|no| readonly_prefix{starts with<br/>read-only prefix?<br/>ls, cat, pwd, git status,<br/>git log, find, grep, rg,<br/>head, tail, wc, ...}
  readonly_prefix -->|yes| allow_to_plugins
  readonly_prefix -->|no| block_exec[block:<br/>not in exec allowlist]
  exec_branch -->|no| blocklist{tool in<br/>MUTATION_TOOL_<br/>BLOCKLIST?<br/>write, edit, apply_patch,<br/>gateway, message, nodes,<br/>process, sessions_send,<br/>sessions_spawn,<br/>subagents}
  blocklist -->|yes| block_listed[block:<br/>blocklisted]
  blocklist -->|no| suffix_mut{ends with<br/>.write .edit .delete?}
  suffix_mut -->|yes| block_suffix[block:<br/>mutation suffix]
  suffix_mut -->|no| suffix_read{ends with<br/>.read .search .list<br/>.get .view?}
  suffix_read -->|yes| allow_to_plugins
  suffix_read -->|no| default_deny[block:<br/>default-deny]
Loading

The shape worth highlighting is the default-deny terminal at the bottom right (mutation-gate.ts:182-187). Anything that isn't on the explicit allowlist, isn't a recognized exec read prefix, isn't on the explicit blocklist, and doesn't match a known suffix pattern is blocked. This is what hardens the gate against unknown plugin tools and future tool additions: a contributor adding a new tool doesn't have to remember to add it to the blocklist for plan mode to do the right thing. They have to opt it in, on purpose, by adding it to either PLAN_MODE_ALLOWED_TOOLS or one of the allow-suffix patterns.

3. Gateway sessions.patch transition

sequenceDiagram
  actor User
  participant UI as Webchat / channel
  participant GW as Gateway<br/>sessions-patch.ts
  participant Cfg as agents.defaults.<br/>planMode.enabled
  participant Store as SessionEntry
  participant Runner as pi-embedded-runner

  User->>UI: /plan on (or click chip)
  UI->>GW: sessions.patch { planMode: "plan" }
  GW->>Cfg: read enabled flag
  alt enabled === true
    GW->>GW: construct PlanModeSessionState<br/>{ mode: "plan",<br/>  approval: "none",<br/>  enteredAt: now,<br/>  updatedAt: now,<br/>  rejectionCount: 0 }
    GW->>Store: SessionEntry.planMode = state
    GW-->>UI: ack + broadcast sessions.changed
  else enabled !== true
    GW-->>UI: INVALID_REQUEST:<br/>"plan mode is disabled"
  end
  Note over Runner: next agent turn
  Runner->>Store: load SessionEntry
  Runner->>Runner: thread planMode into ToolCtx<br/>(attempt.ts:547-550)
  Runner->>Runner: arm before-tool-call gate<br/>(pi-tools.before-tool-call.ts:202-217)

  Note over User,Runner: When user toggles back...
  User->>UI: /plan off (or normal)
  UI->>GW: sessions.patch { planMode: "normal" } or null
  GW->>Store: delete SessionEntry.planMode<br/>(unconditional — escape hatch)
Loading

The opt-in check (sessions-patch.ts:393-405) is the contract the rest of the rollout depends on. Plan-mode tool registration also checks agents.defaults.planMode.enabled (openclaw-tools.registration.ts:43-46), so when the feature is off:

  • Tools enter_plan_mode / exit_plan_mode are not in the catalog.
  • sessions.patch { planMode: "plan" } returns INVALID_REQUEST.
  • The before-tool-call hook never sees ctx.planMode === "plan" (because nothing wrote it), so checkMutationGate is never called.

The escape-hatch asymmetry is intentional: clearing back to "normal" or null is always allowed (sessions-patch.ts:398-400), even if the operator turns the feature off mid-session. Without this asymmetry an operator who enabled the feature, put a session into plan mode, and then disabled the feature would have no way to unstrand the session.

4. Per-file deep dive

src/agents/plan-mode/mutation-gate.ts (188 lines)

Pure function checkMutationGate(toolName, mode, execCommand?). Returns { blocked: boolean, reason?: string }.

The allowlist (mutation-gate.ts:41-50) is intentionally minimal: read, web_search, web_fetch, memory_search, memory_get, update_plan, exit_plan_mode, session_status. The plan-mode tools themselves (update_plan, exit_plan_mode) are exempted explicitly so the agent can revise its proposal and submit for approval without the gate blocking the very tools that move the cycle forward.

Suffix patterns (mutation-gate.ts:35-38) handle MCP-style tools where the actual tool surface follows a provider.verb convention. *.write, *.edit, *.delete are blocked. *.read, *.search, *.list, *.get, *.view are allowed. This is what lets a contributor add an airtable.read MCP tool and have it Just Work in plan mode without modifying the gate.

The exec/bash special case (mutation-gate.ts:115-148) is layered:

  1. Reject anything containing shell compound operators (;, |, &, backticks, $(), >, >>, <(, >(, newlines, carriage returns) — see mutation-gate.ts:119. This is a regex, not a parser, but it is conservative: anything fancier than a simple command is rejected.
  2. Reject dangerous flags using a word-boundary regex (mutation-gate.ts:131-141): -delete, -exec, -execdir, --delete, -rf, --output. Word-boundaries are critical because a substring match would block legitimate flags like find . -executable (which contains -exec as a substring). Regression test mutation-gate.test.ts:184-191.
  3. Allow if the command starts with one of the read-only prefixes (mutation-gate.ts:57-81): ls, cat, pwd, git status, git log, git diff, git show, which, find, grep, rg, head, tail, wc, file, stat, du, df, echo, printenv, whoami, hostname, uname.

If exec is called without a command (or with an empty string), it falls through to the blocklist check and is blocked (mutation-gate.test.ts:124-127).

The blocklist (mutation-gate.ts:19-32) is the explicit "known-mutation" set: apply_patch, bash, edit, exec, gateway, message, nodes, process, sessions_send, sessions_spawn, subagents, write. (bash and exec only reach the blocklist if they failed the read-only check above; this gives a more specific reason string in the typical case.)

The default-deny terminal (mutation-gate.ts:182-187) is the security-critical default. Any tool that doesn't match anything above is blocked with "... is not in the plan-mode allowlist and is blocked by default. Call exit_plan_mode to proceed." Regression: integration.test.ts:222-229.

src/agents/plan-mode/approval.ts (148 lines)

resolvePlanApproval(current, action, feedback?, expectedApprovalId?) — the state-transition resolver.

Stale-event guard semantics (approval.ts:62-66):

if (expectedApprovalId !== undefined) {
  if (current.approvalId === undefined || expectedApprovalId !== current.approvalId) {
    return current;
  }
}

The fail-closed shape — an expectedApprovalId against current.approvalId === undefined is rejected, not silently accepted — is the fix for the iteration-1 audit finding. The earlier shape was if (expectedApprovalId !== undefined && current.approvalId !== undefined && ...) which, when current.approvalId was cleared, fell through and accepted any incoming expectedApprovalId. That meant a stale UI re-firing an approval against a session that had transitioned to "normal" (with approvalId cleared) would silently succeed. Regression covered by approval.test.ts:242-270.

Terminal-state guard (approval.ts:72-81): approved, edited, timed_out are terminal; only pending, rejected, none accept transitions. Additionally, timeout only fires from pending (a session that's already rejected can't time out — the user has already responded).

Rejection-counter reset (approval.ts:93-94, 105-106): approve and edit set rejectionCount: 0. reject does rejectionCount: (current.rejectionCount ?? 0) + 1. timeout doesn't touch the counter. This counter feeds into buildPlanDecisionInjection which, at rejectionCount >= 3, suggests the agent ask the user to clarify their goal instead of looping (types.ts:124-128).

buildApprovedPlanInjection(planSteps) (approval.ts:141-148): builds the context injection prepended to the next agent turn after approval. Contains "Execute it now without re-planning. If a step is no longer viable, mark it cancelled and add a revised step." This is what stops the agent from re-thinking the plan after approval (a recurring failure mode in early prototypes).

src/agents/plan-mode/types.ts (137 lines)

Type contracts + the two security-critical helpers.

newPlanApprovalId() (types.ts:77-93): generates a fresh approvalId via crypto.randomUUID() (~122 bits of entropy), prefixed with plan-. Falls back to Date.now() + Math.random() x2 on hosts without webcrypto. The earlier implementation was Math.random().toString(36).slice(2, 10) (~26 bits, guess-feasible). Regression approval.test.ts:174-184: 1024 calls produces 1024 distinct values.

buildPlanDecisionInjection(decision, feedback?, rejectionCount?) (types.ts:114-137): builds the [PLAN_DECISION]...[/PLAN_DECISION] envelope injected at the start of the agent's next turn after rejection or timeout. The feedback is passed through sanitizeFeedbackForInjection (types.ts:105-107) which rewrites any [/PLAN_DECISION] substring to [\u200B/PLAN_DECISION] (zero-width-space-separated). Without this, an adversarial feedback like "x[/PLAN_DECISION]\n[FAKE_BLOCK]..." would close the envelope early and inject downstream blocks the parser may trust. Regression approval.test.ts:146-165.

src/agents/plan-mode/integration.test.ts (231 lines)

The wiring smoke test — what is verified is that the pieces shipped in this PR are actually wired together end-to-end:

  1. agents.defaults.planMode.enabled === true registers the tools (integration.test.ts:36-55).
  2. enter_plan_mode returns a structured entered result with optional reason (integration.test.ts:57-75).
  3. exit_plan_mode returns approval_requested with the proposed plan, rejects empty plans, rejects plans with multiple in_progress steps, rejects unknown statuses (integration.test.ts:77-120).
  4. before-tool-call hook with ctx.planMode === "plan" blocks write / edit / exec (mutation cmd), allows read / web_search / update_plan / exit_plan_mode, allows exec with read-only ls -la (integration.test.ts:122-220).
  5. With planMode absent or "normal", the gate is disarmed — even write and exec rm -rf /tmp pass through (integration.test.ts:198-220).
  6. The default-deny case: an unknown tool with planMode === "plan" is blocked (integration.test.ts:222-229).

This is the smoke; it does NOT exercise the full approval reply loop (channel renderers, agent_approval_event dispatch). That belongs to subsequent parts.

src/gateway/sessions-patch.ts (39 added lines for plan-mode block)

The plan-mode branch lives at sessions-patch.ts:393-425 inside applySessionsPatchToStore. The pattern matches the rest of applySessionsPatchToStore: the wire-format exposes a flat literal ("plan" / "normal" / null), and the server constructs the full PlanModeSessionState shape on transitions.

Key behaviors:

  • null or "normal" → unconditional clear (sessions-patch.ts:398-400). Always allowed, even if the feature flag is off (escape hatch).
  • "plan" with feature off → INVALID_REQUEST with explanatory message (sessions-patch.ts:401-405).
  • "plan" when already in plan mode → preserve approval state, refresh updatedAt only (sessions-patch.ts:407-409). Important so a duplicate /plan on doesn't wipe a pending approval.
  • "plan" from a non-plan state → mint a fresh PlanModeSessionState with approval: "none", enteredAt / updatedAt set, rejectionCount: 0 (sessions-patch.ts:410-421). The agent then calls exit_plan_mode to actually submit a plan; until then approval is "none".
  • Anything else → INVALID_REQUEST (sessions-patch.ts:422-424).

src/agents/pi-tools.before-tool-call.ts (31 added lines for plan-mode block)

The hook is called per tool call. It receives a HookContext (pi-tools.before-tool-call.ts:15-31) that now includes planMode?: PlanMode. The runner threads this through once per run setup; the hook does not re-read the session store on every tool call.

The plan-mode check (pi-tools.before-tool-call.ts:198-217) runs after loop detection and before the plugin hookRunner:

if (args.ctx?.planMode === "plan") {
  let execCommand: string | undefined;
  if ((toolName === "exec" || toolName === "bash") && isPlainObject(params)) {
    const cmd = params.command;
    if (typeof cmd === "string") {
      execCommand = cmd;
    }
  }
  const gateResult = checkMutationGate(toolName, args.ctx.planMode, execCommand);
  if (gateResult.blocked) {
    return {
      blocked: true,
      reason: gateResult.reason ?? `Tool "${toolName}" is blocked while plan mode is active.`,
    };
  }
}

Three things to note:

  1. The ctx.planMode check is the only fast-path skip — when the session isn't in plan mode, the gate never runs (zero overhead).
  2. For exec / bash, the command string is extracted from the params and passed to checkMutationGate so the read-only-prefix allowlist can apply. Tools other than exec/bash never see this path.
  3. The block runs before getGlobalHookRunner().runBeforeToolCall (pi-tools.before-tool-call.ts:219) — this ordering is what prevents a plugin from intercepting a write call and bypassing the gate.

src/agents/pi-embedded-runner/run/attempt.ts (29 added lines)

The threading point. runEmbeddedAttempt is the function that sets up the per-run tool context. The plan-mode addition (attempt.ts:547-550) is a single conditional spread:

// PR-8: thread plan-mode state through so the
// before-tool-call hook arms the mutation gate without
// re-loading the session store on every tool call.
...(params.planMode ? { planMode: params.planMode } : {}),

The runner reads SessionEntry.planMode.mode once at run setup and passes the resolved literal ("plan" or "normal") into the tool context. The hook (above) reads ctx.planMode. No per-tool-call session-store reads. This is what makes plan mode cheap when it's on — the gate is a synchronous check against a captured literal, not an async session load.

src/agents/openclaw-tools.registration.ts (17 added lines)

Adds isPlanModeToolsEnabledForOpenClawTools(params) (openclaw-tools.registration.ts:42-46) — a single pure check against params.config?.agents?.defaults?.planMode?.enabled === true. Used by openclaw-tools.ts:279 to gate the registration of enter_plan_mode / exit_plan_mode and by the integration test for the enablement-gate assertions.

The function comment is the canonical spec for the opt-in contract: "Default OFF — opt-in feature so a default GPT-5.4 / Claude Sonnet run does NOT see these tools and doesn't accidentally fall into a plan-first workflow." That sentence, taken literally, is the rollout's primary backward-compat guarantee.

Supporting files — at-a-glance

File Role Lines
src/agents/plan-mode/index.ts Public re-export surface 9
src/agents/openclaw-tools.ts Conditionally registers enter_plan_mode / exit_plan_mode based on the gate +8
src/agents/pi-tools.ts Resolves planMode once per run setup, threads into hook ctx +13
src/agents/tool-catalog.ts Adds plan-mode tool catalog entries (gated by isPlanModeToolsEnabledForOpenClawTools) +21
src/agents/tool-description-presets.ts Tool descriptions / display summaries for the two new tools +22
src/agents/tools/enter-plan-mode-tool.ts The enter_plan_mode tool — flips session to plan mode 59 (new)
src/agents/tools/exit-plan-mode-tool.ts The exit_plan_mode tool — submits proposal for approval 124 (new)
src/config/sessions/types.ts SessionEntry.planMode + PostApprovalPermissions type contracts +40
src/config/types.agent-defaults.ts TS type for agents.defaults.planMode +33
src/config/zod-schema.agent-defaults.ts Zod validator for agents.defaults.planMode +23
src/gateway/protocol/schema/sessions.ts Wire-format planMode field on sessions.patch +18
src/agents/pi-embedded-runner/run/params.ts Adds planMode? to run params type +8
src/agents/pi-embedded-runner/run/incomplete-turn.ts Plan-mode-aware planning-only-retry guard (planning-only IS the goal in plan mode) +46
src/agents/pi-embedded-runner/run.ts Plumbs planMode from session entry into run params +81
apps/macos/Sources/OpenClawProtocol/GatewayModels.swift Swift-side schema mirror +13
apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift Swift-side schema mirror +13

5. Security properties

Property File:line Test
Mutation gate fail-closes on unknown tools mutation-gate.ts:182-187 integration.test.ts:222-229 ("blocks unknown tools by default")
Plan-mode tools never bypass the gate themselves mutation-gate.ts:41-50 (explicit allowlist) mutation-gate.test.ts:43-60
exec is blocked without a command mutation-gate.ts:115 (the && execCommand guard, falls through to blocklist) mutation-gate.test.ts:124-127
Shell compound operators rejected on exec mutation-gate.ts:119 (`; &`+newline regex)
Dangerous flags (-delete, -exec, -rf) rejected on exec mutation-gate.ts:131-141 (word-boundary regex) mutation-gate.test.ts:173-181
Word-boundary regex avoids -executable/-rfl false positives mutation-gate.ts:133-134 mutation-gate.test.ts:183-191
Approval requires valid approvalId when one is expected approval.ts:62-66 approval.test.ts:198-207
Approval fail-closes when current state has no token approval.ts:62-66 (the current.approvalId === undefined clause) approval.test.ts:242-270
Adversarial feedback can't escape [PLAN_DECISION] envelope types.ts:105-107 approval.test.ts:146-165
approvalId has cryptographic entropy types.ts:77-93 approval.test.ts:174-184 (1024 distinct calls)
Sessions-patch refuses to arm the gate when feature is off sessions-patch.ts:401-405 covered indirectly via integration.test.ts enablement-gate assertions
Plugin hookRunner cannot bypass the gate pi-tools.before-tool-call.ts:198-217 runs before pi-tools.before-tool-call.ts:219 (hookRunner.runBeforeToolCall) order-of-operations is structural

6. Backward compatibility

  • agents.defaults.planMode.enabled defaults to undefined. Existing configs continue to work unchanged.
  • When the feature is off (the default):
    • enter_plan_mode / exit_plan_mode are not in the tool catalog (openclaw-tools.registration.ts:42-46 + openclaw-tools.ts:279).
    • sessions.patch { planMode: "plan" } is rejected with INVALID_REQUEST (sessions-patch.ts:401-405).
    • The before-tool-call hook never sees ctx.planMode === "plan" (because nothing writes it), so checkMutationGate is never invoked.
  • When the feature is on but no session is in plan mode:
    • All sessions behave exactly as before. The hook fast-paths on args.ctx?.planMode === "plan" (pi-tools.before-tool-call.ts:202).
  • When the feature is on and a session is in plan mode:
    • The gate is active. Read tools, plan-mode tools, and read-only exec commands work; mutation tools are blocked with explanatory reasons.
  • Rollback path: flip the flag back to false (or remove it). Sessions already in plan mode get unstranded via the unconditional null/"normal" clear path (sessions-patch.ts:398-400).

The on-disk SessionEntry.planMode schema lands in [Plan Mode 1/6] and is structurally typed (no runtime import of PlanModeSessionState from this PR's agents/plan-mode/types.ts into config/sessions/types.ts). That keeps the dependency direction agents/* → config/*, never the reverse.

7. Test coverage matrix

File Lines Coverage
src/agents/plan-mode/mutation-gate.test.ts 192 Normal mode (allows everything); plan mode blocks the 11-tool blocklist (case-insensitive); allows the 8-tool allowlist; suffix patterns (*.write, *.edit, *.delete blocked; *.read, *.search allowed); exec read-only allowlist (16 commands); exec mutation blocklist (6 commands); exec without command blocked; newline separators blocked; dangerous flags blocked; bash alias matches exec semantics; word-boundary false-positive guards (-executable, -rfl).
src/agents/plan-mode/approval.test.ts 270 All four actions from pending (approve, edit, reject, timeout); rejection-count accumulation; stale-timeout from approved; enteredAt preservation; feedback cleared on approve; transition from rejected (user changes mind); terminal-state no-op; buildApprovedPlanInjection formatting; buildPlanDecisionInjection rejection + clarification hint at >= 3; expired injection; adversarial-feedback envelope-injection test; case-insensitive marker variants; approvalId prefix + 1024-distinct entropy; stale-event guard match/mismatch + backwards-compat skip when no token expected; rejectionCount reset on approve/edit (NOT on reject/timeout); fail-closed when current state has no token.
src/agents/plan-mode/integration.test.ts 231 Tool enablement gate (false when absent / when disabled / true only when explicitly enabled); enter_plan_mode result shape and reason normalization; exit_plan_mode result shape, empty-plan rejection, multi-in_progress rejection, unknown-status rejection; before-tool-call hook blocks write/edit/exec-mutation in plan mode; allows read/web_search/update_plan/exit_plan_mode/exec-read-only in plan mode; gate disarms when planMode absent or "normal"; default-deny on unknown tools in plan mode.
src/agents/pi-embedded-runner/run.incomplete-turn.test.ts +101 Tightening the planning-only retry guard so plan mode (where planning-only IS the desired state) skips the act-now retry pressure.

Adversarial-regression coverage worth calling out: approval.test.ts:146-165 (envelope injection), approval.test.ts:174-184 (entropy), approval.test.ts:242-270 (fail-closed when current has no token), mutation-gate.test.ts:183-191 (substring false positives).

8. Runtime cost & performance

The cost-of-plan-mode-being-on:

  • Tool registration: one extra check at run setup (openclaw-tools.registration.ts:42-46). When the flag is off, the two plan-mode tools are not constructed at all.
  • Run setup: one extra read of SessionEntry.planMode.mode (already loaded as part of the session entry) and one assignment into the tool context.
  • Per tool call (plan mode off): zero — the hook fast-paths on args.ctx?.planMode === "plan" (pi-tools.before-tool-call.ts:202).
  • Per tool call (plan mode on): one synchronous call to checkMutationGate(toolName, "plan", execCommand?). The gate is a sequence of Set.has lookups and (for exec/bash) two regex tests against the command string. No async I/O, no session-store reads.

There is no batching, no caching, no async — the gate is intentionally a pure function of the captured literal so the cost stays predictable.

9. Parity benchmark callout

The user ran a benchmark sweep against the same prompt set on three plan-mode implementations:

  • OpenClaw plan mode (this rollout)
  • Codex (OpenAI's plan-mode equivalent)
  • Claude Code (Anthropic's plan-mode equivalent)

Results: ~90% parity on output quality and ~95% parity on session length across both Anthropic and OpenAI models. The state-machine semantics (pending → approved/rejected/edited/timed_out with stale-event guards), the read-only allowlist shape (read tools + memory + search + web), and the plan-then-approve UX all converge on the same pattern across vendors. That's the point of framing this PR as "the convergent industry-standard plan-mode pattern" rather than a novel design — the design space is small, and if you build it correctly you end up with a state machine that looks essentially like Codex's and Claude Code's plan modes.

10. What a reviewer can verify in <30 min

  1. Mutation gate (10 min): read mutation-gate.ts:1-188. Confirm the four code paths in order: (a) normal mode short-circuit at :101-104, (b) explicit allowlist at :108-111, (c) exec/bash special case at :115-148 (verify the regex covers all the shell operators you care about), (d) exact blocklist at :151-159, (e) suffix patterns at :162-178, (f) default-deny terminal at :182-187. Then read mutation-gate.test.ts start-to-finish for what's exercised.
  2. Approval state machine (10 min): read approval.ts:44-135. Verify the stale-event guard (:62-66) is fail-closed (the current.approvalId === undefined || clause is the critical part). Verify the terminal-state guard (:72-81). Skim approval.test.ts for the regression tests, specifically the "approvalId stale-event guard — fail-closed" describe block at :242.
  3. Opt-in gate (3 min): read sessions-patch.ts:393-425. Verify the asymmetry: clearing is unconditional (:398-400), arming requires the flag (:401-405).
  4. Hook ordering (3 min): read pi-tools.before-tool-call.ts:198-217. Verify the gate runs before getGlobalHookRunner().runBeforeToolCall (which is at :219) so plugins can't bypass.
  5. Threading (3 min): read attempt.ts:547-550. Verify the runner threads planMode through the tool context once per run, not per call.
  6. Wiring smoke (1 min): scan integration.test.ts describe blocks. The shape (enabled gate, enter tool, exit tool, before-tool-call hook) matches the public surface this PR adds.

11. What this PR does NOT include

  • ask_user_question + plan archetypes + accept-edits gate[Plan Mode 3/6] Advanced plan interactions. The MVP here doesn't need them: a session can flip into plan mode, the agent proposes via exit_plan_mode, the user approves/rejects, the cycle resolves. The advanced-interactions PR adds the agent's ability to ask clarifying questions during planning, the markdown-archetype on-disk layout for plans, and the "accept edits" Claude-Code-style permission grant.
  • Cron nudges + auto-mode + escalating retry + auto-enable for specific models[Plan Mode AUTOMATION] ([Plan Mode AUTOMATION] Cron nudges + auto-enable + subagent follow-ups #70089). The automation layer is orthogonal to the runtime contract — the contract is "block mutations, run state machine"; automation is "schedule nudges, auto-approve when configured, retry with escalating language".
  • Executing-state lifecycle[Plan Mode FULL]. The full bundle adds a third mode value ("executing") for tracking the "plan approved, currently executing" phase distinctly from the generic "normal" post-approval state. Not required for the basic plan-then-approve workflow.
  • Channel renderers + UI components → spread across the channel parts (Telegram/Discord/Slack/iMessage/Signal/CLI) and the UI part. The runtime here emits the events; the channel surfaces consume them.
  • Plan-mode reference card + plan-mode-101 skill → docs PR. Out of scope for the runtime.

Issue references

@greptile-apps

greptile-apps Bot commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds the plan-mode runtime core: a default-deny mutation gate, an approvalId-guarded approval state machine, enter_plan_mode / exit_plan_mode tools, and the sessions.patch opt-in gate — all well-tested with adversarial cases for stale events, feedback injection, and exec command sandboxing.

One unrelated behavioral regression is worth confirming before merge: the attachments argument (image data for hook model selection) was silently dropped from the resolveHookModelSelection call in run.ts, which could cause incorrect model selection for image-bearing requests on all runs, not just plan-mode ones.

Confidence Score: 4/5

Safe to merge once the buildBeforeModelResolveAttachments removal is confirmed intentional or coordinated with Part 1/6; all other findings are P2.

The plan-mode core logic (mutation gate, approval state machine, stale-event guard) is thorough and well-tested. The one P1 concern is the silent removal of attachments from resolveHookModelSelection in run.ts — this touches all runs, not just plan-mode, and the intent is unclear without Part 1/6's setup.ts changes in scope. If confirmed correct, the score is 5.

src/agents/pi-embedded-runner/run.ts — verify the buildBeforeModelResolveAttachments removal is coordinated with Part 1/6 changes to run/setup.ts.

Security Review

  • Environment variable exposure via exec allowlist (src/agents/plan-mode/mutation-gate.ts): printenv is allow-listed for plan-mode exec commands and will dump all environment variables (API keys, database URLs, etc.) into the agent's context window. echo also permits shell variable expansion (e.g. echo $SECRET). Plan-mode is mutation-focused, but operators should be aware that read-only exec commands can still leak secrets in the process environment.
  • approvalId stale-event guard is correctly fail-closed: when expectedApprovalId is supplied but current.approvalId is undefined, the action is a no-op. No bypass identified.
  • Default-deny for unknown tools: unrecognized or MCP plugin tools are blocked in plan mode, preventing newly registered tools from bypassing the gate silently.
  • No credential leakage, injection, or auth-bypass issues identified in the approval state machine or sessions-patch gate logic.

Comments Outside Diff (1)

  1. src/agents/pi-embedded-runner/run.ts, line 305-312 (link)

    P1 Silent removal of attachments from hook model selection

    buildBeforeModelResolveAttachments(params.images) was previously passed to resolveHookModelSelection so the before-model hook could pick a vision-capable model when the prompt carries images. That argument is now dropped entirely. If resolveHookModelSelection in run/setup.ts still accepts (or internally uses) an attachments field, image-bearing requests will silently fall back to whichever model the non-attachment path selects — potentially a text-only model. This is an unrelated behavioral regression introduced in a plan-mode PR with no mention in the PR description.

    Please confirm whether resolveHookModelSelection was updated in Part 1/6 to drop the attachments parameter, or whether this removal is intentional here.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/agents/pi-embedded-runner/run.ts
    Line: 305-312
    
    Comment:
    **Silent removal of `attachments` from hook model selection**
    
    `buildBeforeModelResolveAttachments(params.images)` was previously passed to `resolveHookModelSelection` so the before-model hook could pick a vision-capable model when the prompt carries images. That argument is now dropped entirely. If `resolveHookModelSelection` in `run/setup.ts` still accepts (or internally uses) an `attachments` field, image-bearing requests will silently fall back to whichever model the non-attachment path selects — potentially a text-only model. This is an unrelated behavioral regression introduced in a plan-mode PR with no mention in the PR description.
    
    Please confirm whether `resolveHookModelSelection` was updated in Part 1/6 to drop the `attachments` parameter, or whether this removal is intentional here.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run.ts
Line: 305-312

Comment:
**Silent removal of `attachments` from hook model selection**

`buildBeforeModelResolveAttachments(params.images)` was previously passed to `resolveHookModelSelection` so the before-model hook could pick a vision-capable model when the prompt carries images. That argument is now dropped entirely. If `resolveHookModelSelection` in `run/setup.ts` still accepts (or internally uses) an `attachments` field, image-bearing requests will silently fall back to whichever model the non-attachment path selects — potentially a text-only model. This is an unrelated behavioral regression introduced in a plan-mode PR with no mention in the PR description.

Please confirm whether `resolveHookModelSelection` was updated in Part 1/6 to drop the `attachments` parameter, or whether this removal is intentional here.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/plan-mode/mutation-gate.ts
Line: 56-65

Comment:
**`enter_plan_mode` missing from `PLAN_MODE_ALLOWED_TOOLS`**

`enter_plan_mode` is not listed in `PLAN_MODE_ALLOWED_TOOLS`, so if an agent calls `enter_plan_mode` while already in plan mode (e.g. a confused model retry) it will hit the default-deny path and receive a message saying `"is not in the plan-mode allowlist and is blocked by default. Call exit_plan_mode to proceed."` — which is misleading guidance.

Either add `enter_plan_mode` to the allowlist (making it a harmless no-op) or return a distinct "already in plan mode" reason string instead of the generic default-deny message.

```suggestion
const PLAN_MODE_ALLOWED_TOOLS = new Set([
  "read",
  "web_search",
  "web_fetch",
  "memory_search",
  "memory_get",
  "update_plan",
  "enter_plan_mode",
  "exit_plan_mode",
  "session_status",
]);
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/plan-mode/mutation-gate.ts
Line: 82-100

Comment:
**`printenv` and `echo` expose environment secrets during plan mode**

`printenv` (dumps every env var) and `echo` (can expand `$VAR` via the host shell) are in the read-only exec allowlist. In plan-mode deployments that have secrets in the environment (`ANTHROPIC_API_KEY`, `DATABASE_URL`, etc.) an agent calling `exec` with `printenv` or `echo $API_KEY` will print those values into its context window, which can then appear in plan summaries, approval text, and logs.

Consider removing `printenv` from the allowlist (it has little planning utility) and documenting that `echo` permits shell variable expansion.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/plan-mode/approval.ts
Line: 110-120

Comment:
**`timeout` clears `feedback` but timeout is only reachable from `pending`**

The `timeout` case unconditionally sets `feedback: undefined`. In a second proposal cycle (agent rejected → revised → calls `exit_plan_mode` again), if the runner doesn't explicitly clear `feedback` when re-arming the `pending` state, the previous rejection feedback may still be present when timeout fires — silently erasing context the agent needs on its next turn.

Confirm the runner clears `feedback` on every `exit_plan_mode``pending` transition. If it doesn't, either clear it there or preserve `feedback` through `timeout`.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "feat(plan-mode): split PR3 core backend ..." | Re-trigger Greptile

Comment on lines +56 to +65
*/
const READ_ONLY_EXEC_PREFIXES = [
"ls",
"cat",
"pwd",
"git status",
"git log",
"git diff",
"git show",
"which",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 enter_plan_mode missing from PLAN_MODE_ALLOWED_TOOLS

enter_plan_mode is not listed in PLAN_MODE_ALLOWED_TOOLS, so if an agent calls enter_plan_mode while already in plan mode (e.g. a confused model retry) it will hit the default-deny path and receive a message saying "is not in the plan-mode allowlist and is blocked by default. Call exit_plan_mode to proceed." — which is misleading guidance.

Either add enter_plan_mode to the allowlist (making it a harmless no-op) or return a distinct "already in plan mode" reason string instead of the generic default-deny message.

Suggested change
*/
const READ_ONLY_EXEC_PREFIXES = [
"ls",
"cat",
"pwd",
"git status",
"git log",
"git diff",
"git show",
"which",
const PLAN_MODE_ALLOWED_TOOLS = new Set([
"read",
"web_search",
"web_fetch",
"memory_search",
"memory_get",
"update_plan",
"enter_plan_mode",
"exit_plan_mode",
"session_status",
]);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/plan-mode/mutation-gate.ts
Line: 56-65

Comment:
**`enter_plan_mode` missing from `PLAN_MODE_ALLOWED_TOOLS`**

`enter_plan_mode` is not listed in `PLAN_MODE_ALLOWED_TOOLS`, so if an agent calls `enter_plan_mode` while already in plan mode (e.g. a confused model retry) it will hit the default-deny path and receive a message saying `"is not in the plan-mode allowlist and is blocked by default. Call exit_plan_mode to proceed."` — which is misleading guidance.

Either add `enter_plan_mode` to the allowlist (making it a harmless no-op) or return a distinct "already in plan mode" reason string instead of the generic default-deny message.

```suggestion
const PLAN_MODE_ALLOWED_TOOLS = new Set([
  "read",
  "web_search",
  "web_fetch",
  "memory_search",
  "memory_get",
  "update_plan",
  "enter_plan_mode",
  "exit_plan_mode",
  "session_status",
]);
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +82 to +100

export interface MutationGateResult {
blocked: boolean;
reason?: string;
}

/**
* Checks whether a tool call should be blocked during plan mode.
*
* @param toolName - The tool name being called (case-insensitive)
* @param currentMode - The current plan mode state
* @param execCommand - If the tool is `exec`, the command string to check
* against the read-only prefix whitelist
*/
export function checkMutationGate(
toolName: string,
currentMode: PlanMode,
execCommand?: string,
): MutationGateResult {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security printenv and echo expose environment secrets during plan mode

printenv (dumps every env var) and echo (can expand $VAR via the host shell) are in the read-only exec allowlist. In plan-mode deployments that have secrets in the environment (ANTHROPIC_API_KEY, DATABASE_URL, etc.) an agent calling exec with printenv or echo $API_KEY will print those values into its context window, which can then appear in plan summaries, approval text, and logs.

Consider removing printenv from the allowlist (it has little planning utility) and documenting that echo permits shell variable expansion.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/plan-mode/mutation-gate.ts
Line: 82-100

Comment:
**`printenv` and `echo` expose environment secrets during plan mode**

`printenv` (dumps every env var) and `echo` (can expand `$VAR` via the host shell) are in the read-only exec allowlist. In plan-mode deployments that have secrets in the environment (`ANTHROPIC_API_KEY`, `DATABASE_URL`, etc.) an agent calling `exec` with `printenv` or `echo $API_KEY` will print those values into its context window, which can then appear in plan summaries, approval text, and logs.

Consider removing `printenv` from the allowlist (it has little planning utility) and documenting that `echo` permits shell variable expansion.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +110 to +120
return {
...current,
mode: "plan",
approval: "rejected",
confirmedAt: undefined,
updatedAt: now,
feedback: feedback ?? current.feedback,
rejectionCount: (current.rejectionCount ?? 0) + 1,
};

case "timeout":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 timeout clears feedback but timeout is only reachable from pending

The timeout case unconditionally sets feedback: undefined. In a second proposal cycle (agent rejected → revised → calls exit_plan_mode again), if the runner doesn't explicitly clear feedback when re-arming the pending state, the previous rejection feedback may still be present when timeout fires — silently erasing context the agent needs on its next turn.

Confirm the runner clears feedback on every exit_plan_modepending transition. If it doesn't, either clear it there or preserve feedback through timeout.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/plan-mode/approval.ts
Line: 110-120

Comment:
**`timeout` clears `feedback` but timeout is only reachable from `pending`**

The `timeout` case unconditionally sets `feedback: undefined`. In a second proposal cycle (agent rejected → revised → calls `exit_plan_mode` again), if the runner doesn't explicitly clear `feedback` when re-arming the `pending` state, the previous rejection feedback may still be present when timeout fires — silently erasing context the agent needs on its next turn.

Confirm the runner clears `feedback` on every `exit_plan_mode``pending` transition. If it doesn't, either clear it there or preserve `feedback` through `timeout`.

How can I resolve this? If you propose a fix, please make it concise.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8434da8c7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 306 to 310
const hookSelection = await resolveHookModelSelection({
prompt: params.prompt,
attachments: buildBeforeModelResolveAttachments(params.images),
provider,
modelId,
hookRunner,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve image attachments in before_model_resolve hook

This call now omits attachments, so before_model_resolve hooks no longer receive image metadata for multimodal prompts. Plugins that route to vision-capable models based on attachments will silently lose that signal and can pick a text-only model, causing image turns to fail or degrade. The previous code passed buildBeforeModelResolveAttachments(params.images) here, so this is a regression in model-selection behavior for image inputs.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real concern, acknowledged. Not fixed in this immediate cycle (we're in cleanup-mode focused on maintainer-takeover; see #70101 for context). Tracked in the umbrella's deferred-items checklist for the bot-triage follow-up cycle. The fix will land on this branch with a Fixed in {SHA} reply here per the standard pr-review-loop pattern. Maintainers can also opt to merge the per-part PR as-is and address in a post-merge polish cycle.

Comment on lines +707 to +711
// PR-8: thread plan-mode state through to the attempt so the
// before-tool-call hook arms the mutation gate. Without this
// the field added to attempt's params + the threading through
// pi-tools is dead code (Codex P1 #67840 r3096735975).
...(params.planMode ? { planMode: params.planMode } : {}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Thread plan mode into retry/ack guards

Plan mode is forwarded only into tool execution context here, but the same run still computes planning-only retries and ACK fast-path without planModeActive. In this commit, resolvePlanningOnlyRetryInstruction/resolveAckExecutionFastPathInstruction gained plan-mode bypass logic, yet runEmbeddedPiAgent never supplies that flag, so plan-mode sessions still get "act now" pressure and can hit strict-agentic blocked flows instead of staying in plan-first behavior.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real concern, acknowledged. Not fixed in this immediate cycle (we're in cleanup-mode focused on maintainer-takeover; see #70101 for context). Tracked in the umbrella's deferred-items checklist for the bot-triage follow-up cycle. The fix will land on this branch with a Fixed in {SHA} reply here per the standard pr-review-loop pattern. Maintainers can also opt to merge the per-part PR as-is and address in a post-merge polish cycle.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces the core backend wiring for Plan Mode: a server-controlled session toggle, runtime tool-call gating to block mutations while planning, and the initial approval-state primitives/tools needed to request user approval before executing mutating actions.

Changes:

  • Add plan-mode runtime primitives (mutation gate + approval state machine) and integration tests.
  • Add sessions.patch wire support for planMode: "plan" | "normal" | null, with server-side construction/clearing of persisted session plan-mode state.
  • Thread planMode through the embedded runner/tool pipeline so the before-tool-call hook can enforce the gate without per-call session-store reads.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/gateway/sessions-patch.ts Applies sessions.patch.planMode with feature-flag gating and server-side state construction.
src/gateway/protocol/schema/sessions.ts Extends sessions.patch schema with planMode toggle.
src/config/zod-schema.agent-defaults.ts Adds config schema for embedded runner auto-continue + planMode enable flag.
src/config/types.agent-defaults.ts Adds config types/docs for embedded runner auto-continue + planMode enable flag.
src/config/sessions/types.ts Adds structural SessionEntry.planMode persisted shape and refactors plugin status/trace line helpers.
src/agents/plan-mode/types.ts Defines plan-mode session state + approvalId generation + decision injection builder.
src/agents/plan-mode/mutation-gate.ts Implements fail-closed tool allow/deny logic for plan mode, including readonly exec allowlist.
src/agents/plan-mode/approval.ts Implements plan approval state transitions + stale-event guarding + approved-plan injection builder.
src/agents/plan-mode/index.ts Re-exports plan-mode module surface.
src/agents/plan-mode/mutation-gate.test.ts Unit tests for mutation gate tool/exec behavior.
src/agents/plan-mode/approval.test.ts Unit tests for approval transitions, stale guards, and injection hardening.
src/agents/plan-mode/integration.test.ts Smoke test of end-to-end wiring (tool gating + tool registration + hook behavior).
src/agents/tools/enter-plan-mode-tool.ts Adds enter_plan_mode agent tool returning a structured transition request.
src/agents/tools/exit-plan-mode-tool.ts Adds exit_plan_mode tool returning a structured approval request + plan payload validation.
src/agents/tool-description-presets.ts Adds descriptions/display summaries for plan-mode tools.
src/agents/tool-catalog.ts Registers plan-mode tools in the catalog for profile/policy participation.
src/agents/openclaw-tools.registration.ts Adds config gate helper isPlanModeToolsEnabledForOpenClawTools.
src/agents/openclaw-tools.ts Conditionally registers enter/exit plan-mode tools when feature is enabled.
src/agents/pi-tools.ts Threads planMode into hook context during tool assembly.
src/agents/pi-tools.before-tool-call.ts Enforces mutation gate before plugin hooks run.
src/agents/pi-embedded-runner/run/params.ts Adds runner param to carry planMode into attempts/tool setup.
src/agents/pi-embedded-runner/run/attempt.ts Threads planMode into tool creation path; seeds skill plan templates.
src/agents/pi-embedded-runner/run/incomplete-turn.ts Expands strict-agentic planning-only retries + adds plan-mode awareness hooks for retry pressure/ACK path.
src/agents/pi-embedded-runner/run.ts Threads planMode into attempts; adds auto-continue loop logic and retry escalation.
src/agents/pi-embedded-runner/run.incomplete-turn.test.ts Updates retry-count expectations and adds coverage for escalation + plan-mode suppression.
apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift Adds sessions.patch fields for plan-mode-related params in Swift protocol models.
apps/macos/Sources/OpenClawProtocol/GatewayModels.swift Same as above for macOS Swift protocol models.
Comments suppressed due to low confidence (2)

src/agents/pi-embedded-runner/run.ts:22

  • resolveAgentAutoContinue is imported from ../agent-scope.js, but there is no definition/export for this symbol in the current codebase (searching src/** only finds this import). This will break compilation; either add/export resolveAgentAutoContinue (and its config resolution) or remove the import and gate the auto-continue wiring behind an existing resolver.
import {
  hasConfiguredModelFallbacks,
  resolveAgentAutoContinue,
  resolveAgentExecutionContract,
  resolveSessionAgentIds,
  resolveAgentWorkspaceDir,
} from "../agent-scope.js";

src/agents/pi-embedded-runner/run.ts:312

  • The resolveHookModelSelection call no longer passes attachments derived from params.images. This breaks before_model_resolve hooks that select provider/model based on prompt attachments (e.g. vision model routing). Please restore passing attachments: buildBeforeModelResolveAttachments(params.images) (and keep omitting the key when there are none).
      const hookSelection = await resolveHookModelSelection({
        prompt: params.prompt,
        provider,
        modelId,
        hookRunner,
        hookContext: hookCtx,
      });

Comment thread src/agents/plan-mode/approval.ts Outdated
Comment on lines +72 to +81
if (
current.approval !== "pending" &&
current.approval !== "rejected" &&
current.approval !== "none"
) {
return current;
}
if (action === "timeout" && current.approval !== "pending") {
return current;
}

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolvePlanApproval currently allows approve/edit/reject actions when current.approval === "none" (see terminal-state guard). That enables accidental or malicious approvals to unlock mutations even if no exit_plan_mode proposal was submitted. Consider treating "none" as terminal/no-op (only allow transitions from pending or rejected, and keep timeout limited to pending).

Copilot uses AI. Check for mistakes.
* `PlanModeSessionState` from `src/agents/plan-mode/types.ts` to avoid
* an `agents/*` → `config/sessions/*` dependency on what is still a
* transitional plan-mode lib (PR #67538). The shape mirrors that type
* and is enforced via Zod at sessions.patch time.

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment says the SessionEntry.planMode structural shape "is enforced via Zod at sessions.patch time", but the gateway patch path uses TypeBox (SessionsPatchParamsSchema) plus imperative normalization, not Zod. Please update the comment to avoid implying a validator that isn’t actually in the request pipeline.

Suggested change
* and is enforced via Zod at sessions.patch time.
* without coupling this file to the plan-mode module's definitions.

Copilot uses AI. Check for mistakes.
Comment thread src/agents/tool-description-presets.ts Outdated
export function describeExitPlanModeTool(): string {
return [
"Exit plan mode and request user approval of the proposed plan.",
"Pass the current plan steps via `plan` (use the same shape as update_plan).",

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

describeExitPlanModeTool() says the plan payload uses "the same shape as update_plan", but exit_plan_mode's schema adds fields/statuses (e.g. activeForm, cancelled) that update_plan doesn't accept. Either align the schemas or adjust this description to say "similar" and document the differences so tool authors don’t get inconsistent validation errors.

Suggested change
"Pass the current plan steps via `plan` (use the same shape as update_plan).",
"Pass the current plan via `plan`; its structure is similar to update_plan, but exit_plan_mode may include additional exit-specific fields or statuses (for example `activeForm` or `cancelled`) that update_plan does not accept.",

Copilot uses AI. Check for mistakes.
Comment thread src/agents/tools/exit-plan-mode-tool.ts Outdated
Comment on lines +25 to +46
const PLAN_STEP_STATUSES = ["pending", "in_progress", "completed", "cancelled"] as const;

const ExitPlanModeToolSchema = Type.Object({
plan: Type.Array(
Type.Object(
{
step: Type.String({ description: "Short plan step." }),
status: stringEnum(PLAN_STEP_STATUSES, {
description: 'One of "pending", "in_progress", "completed", or "cancelled".',
}),
activeForm: Type.Optional(
Type.String({
description: 'Present-continuous form shown while in_progress (e.g. "Running tests").',
}),
),
},
{ additionalProperties: false },
),
{
minItems: 1,
description: "The plan being proposed for approval. At most one step may be in_progress.",
},

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exit_plan_mode's plan schema diverges from update_plan (extra cancelled status and activeForm field) while nearby docs describe it as the same/near-copy. If you want a single shared plan shape across tools, consider reusing the same status enum/fields (or explicitly documenting why exit_plan_mode differs).

Copilot uses AI. Check for mistakes.
Comment on lines 681 to +694
const basePrompt =
provider === "anthropic" ? scrubAnthropicRefusalMagic(params.prompt) : params.prompt;
const promptAdditions = [
ackExecutionFastPathInstruction,
planningOnlyRetryInstruction,
reasoningOnlyRetryInstruction,
emptyResponseRetryInstruction,
].filter(
(value): value is string => typeof value === "string" && value.trim().length > 0,
);
...new Set(
[
ackExecutionFastPathInstruction,
planningOnlyRetryInstruction,
reasoningOnlyRetryInstruction,
emptyResponseRetryInstruction,
].filter(
(value): value is string => typeof value === "string" && value.trim().length > 0,
),
),
];

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plan mode needs to disable both the ACK fast-path and the planning-only retry pressure (planModeActive was added for this), but the runner still computes ackExecutionFastPathInstruction / retry instructions without passing planModeActive: params.planMode === "plan". As-is, plan-mode sessions can still get act-now steering appended to the prompt via promptAdditions.

Copilot uses AI. Check for mistakes.
Comment on lines 1779 to 1787
});
}
planningOnlyRetryAttempts += 1;
planningOnlyRetryInstruction = nextPlanningOnlyRetryInstruction;
planningOnlyRetryInstruction = resolveEscalatingPlanningRetryInstruction(
planningOnlyRetryAttempts - 1,
);
log.warn(
`planning-only turn detected: runId=${params.runId} sessionId=${params.sessionId} ` +
`provider=${provider}/${modelId} contract=${executionContract} configured=${configuredExecutionContract} — retrying ` +

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runner escalates planningOnlyRetryInstruction here, but nextPlanningOnlyRetryInstruction is still computed earlier without planModeActive. In plan mode this should be disabled entirely; thread planModeActive: params.planMode === "plan" into the resolvePlanningOnlyRetryInstruction (and sibling) calls so this block never triggers during plan-mode sessions.

Copilot uses AI. Check for mistakes.
@100yenadmin

Copy link
Copy Markdown
Contributor Author

Acknowledging Greptile / Copilot / Codex review feedback on this PR. Triage status:

  • Stack-coordination concerns (e.g. unresolved imports referencing symbols added by earlier per-part PRs in the stack) — expected by design. Red CI on this PR is per the rollout plan; CI turns green as the chain merges in sequence (1/6 → 6/6). See the stack-position header banner at the top of this PR body.
  • Real source-code issues flagged here (P1 bugs, P2 nits, etc.) — will be triaged + fixed in a focused follow-up cycle within ~24h. Fix SHAs will be posted as in-line replies on each individual comment per the standard pr-review-loop pattern.

For end-to-end integration testing in the meantime, see #70071 [Plan Mode FULL] — the green-CI integrated bundle of all parts.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2c0d75f17

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/agents/openclaw-tools.ts Outdated
Comment on lines +307 to +311
createSessionsSendTool({
agentSessionKey: options?.agentSessionKey,
agentChannel: options?.agentChannel,
sandboxed: options?.sandboxed,
config: resolvedConfig,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reinstate embedded gating for gateway-only session tools

createOpenClawTools now always exposes sessions_send (and the adjacent sessions_spawn) and routes them through the real gateway caller, even in local embedded mode. Embedded runs do not have a gateway process, so these tools become selectable but fail at execution time with gateway-unavailable errors; previously this path used the embedded gateway stub and hid send/spawn in embedded mode. This regression can strand embedded CLI runs in retry/error loops when the model picks these tools.

Useful? React with 👍 / 👎.

@@ -195,7 +175,6 @@ export const AgentDefaultsSchema = z
})
.strict()
.optional(),
truncateAfterCompaction: z.boolean().optional(),
notifyUser: z.boolean().optional(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve truncateAfterCompaction in defaults schema

The strict AgentDefaultsSchema compaction object no longer accepts truncateAfterCompaction, so existing configs using agents.defaults.compaction.truncateAfterCompaction will now be rejected at load time. That is a backward-incompatible regression because runtime logic still reads this flag to control post-compaction truncation, so users can no longer enable a supported behavior without startup/config validation failures.

Useful? React with 👍 / 👎.

Cherry-pick / file-copy of 27 files from feat/plan-channel-parity-merged-upstream
(merged-head 651a22d) onto current upstream/main. Branch: isolated/pm-2-of-6-core-backend.

v4 absorbs the upstream merge into plan-mode dev (Option B; merge commit
b1ead74) plus the post-merge build/test fixes (commit 651a22d).
@100yenadmin
100yenadmin force-pushed the isolated/pm-2-of-6-core-backend branch from c2c0d75 to 52fc729 Compare April 22, 2026 14:36

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52fc729eaa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +746 to +750
// PR-8: thread plan-mode state through to the attempt so the
// before-tool-call hook arms the mutation gate. Without this
// the field added to attempt's params + the threading through
// pi-tools is dead code (Codex P1 #67840 r3096735975).
...(params.planMode ? { planMode: params.planMode } : {}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Forward dropped run context fields into attempt params

This runEmbeddedAttemptWithBackend payload now omits both memberRoleIds and isCanonicalWorkspace, which were previously threaded through this same object. run/attempt.ts still relies on these values (memberRoleIds is passed into tool/requester-origin policy paths, and isCanonicalWorkspace drives bootstrap routing), so group-role-aware spawn/routing decisions and canonical-workspace bootstrap behavior silently fall back to defaults on every run. Re-add both fields when constructing attempt params so downstream policy and bootstrap gating keep using the caller’s real session context.

Useful? React with 👍 / 👎.

@prtags

prtags Bot commented Apr 23, 2026

Copy link
Copy Markdown

Related work from PRtags group ace-bullfrog-hw3e

Title: Open PR candidate: plan-mode carve-out overlaps integrated full bundle

Number Title
#70031 [Plan Mode 1/6] Plan-state foundation
#70066* [Plan Mode 2/6] Core backend MVP
#70067 [Plan Mode 3/6] Advanced plan interactions
#70068 [Plan Mode 4/6] Web UI + i18n
#70069 [Plan Mode 5/6] Text channels + Telegram
#70070 [Plan Mode 6/6] Docs, QA, and help
#70071 [Plan Mode FULL] Integrated bundle for testing (Parts 1\u20136 + automation + executing-state lifecycle)
#70088 [Plan Mode INJECTIONS] Typed pending-injection queue foundation
#70089 [Plan Mode AUTOMATION] Cron nudges + auto-enable + subagent follow-ups

* This PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling app: macos App: macos app: web-ui App: web-ui gateway Gateway runtime size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants