Skip to content

[Plan Mode 8/8] Executing-state lifecycle + debug hardening#70020

Closed
100yenadmin wants to merge 30 commits into
openclaw:mainfrom
electricsheephq:restack/68939-pr10-executing-followup
Closed

[Plan Mode 8/8] Executing-state lifecycle + debug hardening#70020
100yenadmin wants to merge 30 commits into
openclaw:mainfrom
electricsheephq:restack/68939-pr10-executing-followup

Conversation

@100yenadmin

Copy link
Copy Markdown
Contributor

Summary

[Plan Mode 8/8] — part of the 8-part decomposition of #68939, stacking on #69449 (Part 1).

Stacks on: previous part (see [Plan Mode 7/8] / [Plan Mode 1/9 #69449]).

Phase-2 work from 24+ hours of live smoke testing: 3-state planMode (plan|executing|normal), executing-phase nudges, plan_mode_status introspection, [PLAN_STATUS] auto-inject preamble, P2.12a/b verify-via-tool discipline, allowlist catch-22 fix (sessions_yield/lcm_grep/lcm_expand_query), 20+ debug commits. Stacks on Part 7/8.

Lines: ~2067

Diff note

This PR is opened cross-repo (head: 100yenadmin:restack/68939-pr10-executing-followup) against openclaw:main. Because GitHub doesn't support stacked-PR bases that point at fork refs, the displayed diff cumulatively includes all previous parts in the stack. Reviewers should focus on the changes specific to this part — see commit list for the new content beyond what previous parts established.

Test status

After post-v2026.4.21 rebase: all targeted plan-mode tests pass (53/53 in unit-fast for approval + integration suites). The full suite has pre-existing vitest workspace project-name conflicts unrelated to this work.

Related

Eva added 30 commits April 22, 2026 12:39
…ion tests (rebase fixup)

Pure rebase fixup. The `exit_plan_mode` tool gained a mandatory `title`
parameter in upstream main between Part 5's original base and current
HEAD. After rebasing Part 5 onto upstream/main, four integration tests
(in `src/agents/plan-mode/integration.test.ts`) were failing with
`ToolInputError: exit_plan_mode requires a title field` instead of
the originally-asserted error messages.

This commit just adds `title: "..."` to the four failing test calls
so they exercise the validation paths they were originally testing
(empty plan, multiple in_progress, unknown status).

No production code changes.
…gh embedded-runner attempt handoff

Root cause for "auto-mode plan approves but agent's tool calls keep getting blocked in plan mode": `runEmbeddedPiAgent` (run.ts:738) spread `params.planMode` into `runEmbeddedAttemptWithBackend` but DROPPED `params.getLatestPlanMode` and `params.getLatestAcceptEdits` on the floor.

Confirmed via `[plan-mode-gate]` diagnostic on EVERY tool call:
  hasGetLatestCallback=no  cachedCtxPlanMode=plan  liveMode=<undefined>  latestPlanMode=plan

Even though `auto-reply/reply/agent-runner-execution.ts:1134` correctly wires the callback into the outer `runEmbeddedPiAgent` call, that callback was lost at this attempt-handoff site — never reached `attempt.ts:614` where it would have been forwarded to `pi-tools` and then to the before-tool-call hook ctx.

Effect: the mutation gate fell back to the cached `ctx.planMode === "plan"` snapshot captured at run-start. That snapshot stayed "plan" for the entire turn even after auto-approve had flipped disk → "normal" and a fresh resume run had started. Every post-approval mutation tool call (write/exec/edit) hit the gate's plan-mode block.

The original "Bug 3+4" live-read fix landed in `agent-runner-execution.ts` was effectively dead wiring because of this gap.

Fix: three matching optional spreads next to the existing `planMode` line, mirroring `attempt.ts:614` on the receiving end.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…ed [PLAN_DECISION]: approved after auto-approve

Bug: in headless / Telegram-only / no-UI auto-mode sessions, after `autoApproveIfEnabled` lands the `sessions.patch { planApproval: { action: "approve" } }`, the agent's main run had already ended and nothing was polling the pending-injection queue. The agent sat idle until the user sent a follow-up message — at which point the queued `[PLAN_DECISION]: approved` finally fired alongside their message. "Auto-mode" silently still required user intervention.

V1 fix attempt: bare `chat.send { message: "continue" }` to kick a fresh agent run — relied on the runtime consuming the queued injection on that run. Live-confirmed not working: the agent's new turn loaded "continue" as the user instruction but didn't dig into the pending-injection queue, reasoned its way to "waiting on approval", sat idle.

V2 fix (this commit): include the canonical `[PLAN_DECISION]: approved` signal IN the synthetic chat.send message itself. The agent sees the decision as its current user instruction — no dependency on injection-queue draining. Mirrors the literal text the persister writes into the queue so the agent's reasoning path is identical.

`deliver: false` keeps the synthetic message out of the user-visible transcript. `idempotencyKey: auto-approve-resume-<approvalId>` makes it safe if the UI ALSO attempts a resume for the same approval (e.g. a connected tab firing its own approval-event handler) — the second call is a no-op.

Failure is non-fatal: if the resume trigger errors out, the queued injection stays on disk and the user's next real message consumes it (same behavior as pre-fix).

Pairs with the run.ts callback-wiring fix in 88ac515034 — without that fix, the resume run still hit the mutation gate because `getLatestPlanMode` was dropped at the attempt handoff.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…ool-call hook

Emits one log line per plan-mode gate evaluation, gated to fire only when `latestPlanMode === "plan"` OR `args.ctx?.planMode === "plan"` so it stays quiet during normal-mode operation.

Fields: tool, sessionKey, cachedCtxPlanMode, liveMode, latestPlanMode, hasGetLatestCallback. Together they distinguish:

  - Stale cached snapshot (cachedCtxPlanMode=plan, liveMode=normal → gate correctly NOT tripped)
  - Disk says still pending (liveMode=plan → gate correctly tripped)
  - Missing live-read wiring (hasGetLatestCallback=no, liveMode=undefined → silent fall-back to cached snapshot, root cause for "auto-mode mutation gate falsely blocks post-approval tool calls")
  - Closure-resolved disk failure (hasGetLatestCallback=yes, liveMode=undefined → resolveLatestPlanModeFromDisk returned undefined; investigate storePath/sessionKey resolution)

This was the diagnostic that nailed the root-cause for the auto-mode wiring fix in 88ac515034 — keeping it in tree so future plan-mode regressions can be triaged the same way (single grep on `[plan-mode-gate]` shows the full causal chain).

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…== true

Two-path popup-suppression for auto-mode plan approvals:

1. `app.ts:buildHydratedPlanApprovalRequest` — return null on hydrate when `row.planMode?.autoApprove === true`, regardless of `pending.approvalId`. Catches reloads / tab-refresh / sessions.list rehydrate.

2. `app-tool-stream.ts:setPlanApprovalRequest` — short-circuit the live event-stream setter when `host.sessionsResult.sessions.find(s => s.key === next.sessionKey).planMode.autoApprove === true`. Catches push events that arrive between hydrate cycles. Required because hydrate-only suppression let the popup briefly flash (sometimes long enough to click) before sessions.changed pushed the auto-approve resolution.

Why suppress instead of click-through-and-dismiss:

- The server-side `autoApproveIfEnabled` (in pi-embedded-subscribe.handlers.tools.ts) already resolves the approval. By the time the client renders the card, the server has often already moved past `approval=pending` → either the agent run is mid-execution OR the resolution event is in flight.
- `sessions.changed` push lag from gateway → client can stretch the client-visible "pending" window for hundreds of ms or longer (especially during high tool-stream activity post-auto-approve).
- A user clicking the briefly-rendered card hits "requires a pending approval (current state: none)" stale-state errors. Reads as "I clicked and nothing happened" — worse UX than no card at all.
- Question approvals (pending.kind === "question") still render unconditionally — auto-approve does not answer questions for the user.

Escape hatch: `/plan auto off` flips the flag, the next hydrate renders the card as a manual fallback.

Pairs with the server-side root-cause fix in 88ac515034 (callback wiring) + b45272ce7c (resume chat.send v2). The trio collectively delivers the "auto-mode = no popup, no clicks, agent just executes" UX.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…ction drives hydrate

Two related state-drift fixes surfaced during Test 2 smoke run:

1. Selector decoupling (app-render.ts:2380-2402)
   Prior guard `if (mode.planMode === "plan")` caused the autoApprove sync patch to fire ONLY when switching INTO a plan mode. Switching FROM "Plan ⚡" TO "Default" (or Bypass, etc.) left `planMode.autoApprove = true` persisted on the session entry. The next agent-driven `enter_plan_mode` then silently armed auto-approve despite the user's explicit selector move away from plan-auto — classic state drift between UI intent and persisted flag.

   User's observation nailed it: "I thought the selector controls a lot of the state. If Autoplan is a separate system outside of the state selector, that creates an issue."

   Fix: drop the guard. Every selector click now syncs autoApprove to `mode.planAutoApprove`:
     - Plan ⚡  → autoEnabled=true
     - Plan    → autoEnabled=false
     - Default → autoEnabled=false (clears stale flag)
     - Bypass  → autoEnabled=false (clears stale flag)

   sessions-patch.ts:787-803 handles the "no active planMode" case gracefully (creates a fresh planMode object with mode:"normal", autoApprove:<value>) so the patch is safe even when the session has no planMode entry.

2. Hydrate gate (app.ts:1405)
   Prior guard `if (row.planMode?.approval !== "pending") return null;` was stale under Eva's PR-3 discriminated-union design where `pendingInteraction.status === "pending"` is the canonical source of truth for "there's a plan pending."

   The persister at `plan-snapshot-persister.ts:299 (persistApprovalMetadata)` writes `pendingInteraction` on the approval event but does NOT write `planMode.approval = "pending"`. A separate `persistPlanApprovalId` in `pi-embedded-subscribe.handlers.tools.ts:115` WOULD write it, but only when `current.mode === "plan"` at the moment — if the prior cycle's close-on-complete left `mode: "normal"` OR a prior auto-approve aborted mid-persist leaving a transitional state, `planMode.approval` stays at "none" even though `pendingInteraction.status === "pending"`.

   My prior hydrate guard then returned null and dismissed the card while the live event-stream path had already rendered it — the "popped up and went away instantly" regression user saw on Test 2 attempt #2.

   Fix: drop the `planMode.approval !== "pending"` check. Trust `pendingInteraction.status` (already gated above at line 1395 "pending.status !== 'pending'"). The autoApprove suppression guard immediately below stays in place.

   When the user clicks Approve, the resolve patch transitions BOTH `pendingInteraction.status` and `planMode.approval` — card disappears post-action regardless.

Together these two changes make the selector the single source of truth (Bug A fix) and fix the popup flash-and-dismiss regression (Bug B fix) without introducing new security issues (the autoApprove and selector-derived-state checks remain in place).

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…an injection

Observed during Test 1 live smoke: after auto-approve, agent executes the plan (spawns subagent, gets result back), but does NOT call update_plan to mark the driving step(s) as completed. She reports "the subagent returned hello world" in prose but leaves `planMode.lastPlanSteps[1]` at in_progress and step[2] at pending — plan is effectively stuck at the final recording step.

Root cause: the existing `[PLAN_DECISION]: approved` injection text told the agent to execute and handle cancellations, but never explicitly told her to record step status on completion. Post-approval nudges don't fire (they're `mode: "plan"`-bound and the session is now in `mode: "normal"`), so the reminder has to be inline in the approval injection itself.

Changes:
1. `src/agents/plan-mode/approval.ts` — `buildApprovedPlanInjection()` now includes:
   - "Do not re-plan unless necessary" (reinforces the existing no-re-planning hint)
   - "Check and record the planned status for each step as you go"
   - "After each step finishes (successful or not), call `update_plan` to mark that step's status as completed or cancelled"
   - "The plan is not done until every step is recorded as completed or cancelled"
   - Header "The approved plan:" before the step list (visual separation)

2. `src/agents/plan-mode/approval.test.ts` — updated the wave-4 byte-stability snapshot at line 136 to match the new text. 39/39 tests pass.

3. `src/agents/pi-embedded-subscribe.handlers.tools.ts` — the auto-mode resume chat.send message (my PR #68939 follow-up v2 fix at b45272ce7c) now mirrors the canonical `buildApprovedPlanInjection` wording so the agent sees consistent instructions regardless of which delivery path (chat.send message vs queued injection) fires first.

This is a SOFT STEER, not a hard gate. The agent can still forget to call update_plan — the semantic gate for "plan complete" lives in the close-on-complete detector (`plan-snapshot-persister.ts`) which auto-transitions mode→normal when all steps hit completed/cancelled. But by reinforcing the instruction in the approval text, we close the common-case stall where the agent finishes sub-operations and goes idle without recording state.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…=pending (Test 2 race fix)

Test 2 (rapid back-to-back plans in auto mode, 3× echo plans) reproduced this reliably on 2026-04-21:
  Plan #1: persist→approve-gate→auto-approve→resume→telegram (clean, ~700ms total)
  Plan #2: persist→telegram→`auto-approve aborted: persisted approval state did not reach pending+approvalId=plan-79c87d84 within 2000ms`

Root cause: two separate persisters write to `planMode` after exit_plan_mode emits the approval event:

  (a) `persistApprovalMetadata` (this file:299) — fires from the approval-event listener. Writes pendingInteraction.status="pending" + approvalId + title + approvalRunId. But did NOT write planMode.approval="pending".

  (b) `persistPlanApprovalId` (`pi-embedded-subscribe.handlers.tools.ts:115`) — fires from the exit_plan_mode tool-end handler. WOULD write planMode.approval="pending", but has guard `if (!current || current.mode !== "plan") return null`.

When plan #2 fires immediately after plan #1's auto-approve+close cycle, the persister race kicks in:
  - Plan #1's close-on-complete or autoApprove preservation left planMode.mode="normal"
  - Eva calls enter_plan_mode for plan #2 → starts persisting mode="plan"
  - Eva immediately calls exit_plan_mode for plan #2 → persistPlanApprovalId reads disk
  - On the rare-but-reproducible race window, the disk read happens BEFORE enter_plan_mode's write commits → guard sees mode!=="plan" → SKIPS without writing approval="pending"
  - autoApproveIfEnabled polls 2s for `approval==="pending"`, never sees it, gives up

Fix: write `approval: "pending"` here in persistApprovalMetadata (gated on params.approvalId being present so we only fire when there's an actual approval to be pending on). This makes persistApprovalMetadata the canonical writer for BOTH the modern pendingInteraction.status signal AND the legacy planMode.approval signal — they now move in lockstep at the same write boundary. persistPlanApprovalId becomes redundant in the happy path but stays as a fallback (and its guard remains as a safety check against accidental approval arming outside plan mode).

Why this is safe:
  - persistApprovalMetadata only fires from the approval-event listener — that event is only emitted when exit_plan_mode succeeds, so we ARE at a moment where approval=pending is the correct state
  - All other readers of planMode.approval handle the "pending" state correctly:
      buildActivePlanNudge (heartbeat-runner.ts:742) → suppresses nudges during pending (correct)
      cron suppression (cron/isolated-agent/run.ts:451) → suppresses during pending (correct)
      sessions-patch.ts approve guard → requires "pending" before allowing approve action (correct — autoApproveIfEnabled now succeeds)
      mutation gate → doesn't read approval, only mode (no impact)
  - 109/109 tests pass across plan-snapshot-persister, sessions-patch, approval, and fresh-session-entry suites

Surfaces: Test 2 (rapid back-to-back) should now pass. Test 1 (single plan) was unaffected by this race because the 2000ms poll window was always met by persistPlanApprovalId before any rapid-fire pressure existed.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…4 read-only exec prefixes (Test 4a UX gap)

Test 4a (subagent-gate happy path) live-reproduced a catch-22: Eva tried `sessions_spawn { agentId: "openai-codex/gpt-5.4" }`, got rejected with "use agents_list to discover valid targets", then `agents_list` was ALSO rejected by the default-deny plan-mode gate. She had to submit her plan with a fabricated subagent result to get past the impasse. After approval she retried with the right agentId and it worked, but the plan-mode investigation experience was broken.

Drove a comprehensive audit of `mutation-gate.ts` against `src/agents/openclaw-tools.ts` + `src/agents/tools/*` + the shell-operator regex. Three categories of additions:

1. PLAN_MODE_ALLOWED_TOOLS — three pure-read additions:
   - `agents_list` (the immediate Test 4a fix — closes the discoverability catch-22)
   - `image` (vision-model analysis of screenshots/diagrams; no workspace mutation)
   - `pdf` (vision-model analysis of PDF documents; no workspace mutation)

2. READ_ONLY_EXEC_PREFIXES — 14 new utilities, two clusters:

   Structured-data + log analysis (closes the "can't tail-pipe-grep in plan mode" gap by giving the agent single-command alternatives):
     sort, uniq, cut, diff, tree, jq, column

   System introspection (pure-read diagnostics):
     env, uptime, date, id, nproc, arch, vm_stat

3. Explicit EXCLUSIONS documented in comments:
   - `awk` / `sed` (even `sed -n`) — both have destructive write modes (`sed -i`) and prefix-match logic can't reliably distinguish read-only invocations. Agent should use `grep`/`cut`/`jq` for the same workflows.
   - `ps`, `lsof`, `top`, `dmesg`, `groups`, `last` — security-sensitive system-info exposure
   - `tts`, `image_generate`, `music_generate`, `video_generate`, `canvas`, `cron`, `gateway`, `message`, `sessions_send`, `subagents` (mutating actions) — correctly stay default-deny

Important diagnostic clarification: the user reported Eva couldn't tail long error logs in plan mode. The shell-operator regex (`/[;|&\`\n\r]|\$\(|>>?|<\(|>\(/`) is working correctly — `tail -n 200 file.log`, `tail -F file.log`, and `tail -n +500 file.log` all pass. What gets blocked is `tail file | grep pattern` (pipe). The fix is providing single-command alternatives via `jq`/`cut`/`grep -A` so the agent doesn't need pipes for the common log-parsing flows.

64/64 mutation-gate tests still pass.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…tion (P2.3)

First commit of the Phase-2 architectural refactor. The 2-state union `"plan" | "normal"` had two distinct meanings collapsed into "normal":
  - (a) "no plan ever touched this session"
  - (b) "plan was approved and is mid-execution"

This conflation broke:
  - UI chip rendering: post-approve sessions reverted to "Default" chip even though autoApprove was still armed for the next cycle (live-observed during Eva's MiniMax/David VM session)
  - Execution-phase nudges: nudge crons gated on `mode === "plan"` correctly skip post-approval, leaving execution stalls (subagent returns + steps not marked complete) with no recovery mechanism
  - Future plan_mode_status introspection accuracy

This commit just WIDENS the type — no transition logic change yet. The approve transition still deletes planMode (P2.4 changes that to write `mode: "executing"`). The mutation gate, nudge suppression, and UI selector still treat the field as 2-state effectively (P2.5 + P2.6 wire the new value through).

Files:
  - src/config/sessions/types.ts — `mode: "plan" | "executing" | "normal"` + lifecycle doc
  - src/agents/plan-mode/types.ts — widened `PlanMode` runtime type + doc
  - src/infra/agent-events.ts — `getLatestPlanMode` callback return type
  - src/agents/pi-tools.ts — wrap-options + callback types
  - src/agents/pi-embedded-runner/run/params.ts — runner params + callback
  - src/agents/subagent-announce.ts — `requesterPlanMode` field + the discriminator at line 521 (only "plan" gets the plan-mode suffix; "executing" behaves like "normal" for announce)
  - src/auto-reply/reply/fresh-session-entry.ts — `LivePlanMode` widened + new return path for "executing" mode reads
  - src/auto-reply/reply/agent-runner-execution.ts — local type narrowing + forward "executing" to `runEmbeddedPiAgent` so the runtime ctx has the precise mode

Migration:
  - src/config/sessions/store-migrations.ts — backfill rule: existing `mode: "normal"` entries with non-empty `lastPlanSteps` AND any pending/in_progress step → set `mode: "executing"`. Idempotent (re-runs no-op because the guard checks `=== "normal"`). Safe failure mode: if heuristic is wrong, only the chip renders differently — no functional impact.

Wire-input validator at sessions-patch.ts:637 KEPT as `"plan" | "normal"` only — `"executing"` is server-derived (set by approve transition in P2.4), not a wire-accepted client input.

Tests: 331/331 pass across plan-mode/, fresh-session-entry, subagent-announce, pi-tools.before-tool-call, sessions/. Typecheck clean.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…dge cleanup from planMode delete (P2.4)

Second commit of the Phase-2 architectural refactor. Builds on P2.3 (daf00a29d4) which widened the type union; this commit wires the actual transition.

Changes:

1. src/agents/plan-mode/approval.ts — `resolvePlanApproval` for `approve` and `edit` actions now returns `mode: "executing"` instead of `mode: "normal"`. The plan was approved AND the agent is executing the steps; conflating these into "normal" (which also means "no plan ever touched this session") was the conceptual error fixed here.

2. src/gateway/sessions-patch.ts — split the post-approval cleanup block at line 1058 into two sequential responsibilities:

   (a) Design-phase nudge cleanup — fires on EITHER `mode === "executing"` OR `mode === "normal"`. Design-phase nudges (the 10/30/60 min crons scheduled at enter_plan_mode) are obsolete the moment a plan is approved, regardless of which post-approval state we land in. Cleanup widened to cover the new transition path.

   (b) planMode-object delete + autoApprove preservation — fires ONLY on `mode === "normal"` (the genuine "close" path). The approve transition (mode → "executing") does NOT delete planMode anymore — it lives on carrying title + lastPlanSteps + cycleId + autoApprove through execution. Close-on-complete in plan-snapshot-persister.ts:696 still fires sessions.patch { planMode: "normal" } when all steps mark completed/cancelled, which goes through the wire path → eventually triggers this delete-block via the carry-forward mechanism.

   Lifecycle now: plan → executing → (close-on-complete) → normal + delete. Pre-P2.4 was: plan → normal + delete (skipping the executing phase entirely).

3. src/agents/plan-mode/approval.test.ts — 2 tests updated:
   - approve test: expect mode === "executing" (was "normal")
   - edit test: same

4. src/gateway/sessions-patch.test.ts — 5 tests updated to match the new contract:
   - "preserves autoApprove across approve transition" — expect mode "executing" (was "normal")
   - "PR-12 Bug A1: nudgeJobIds dropped" — same; key invariant (nudgeJobIds undefined on the post-approve entry) preserved
   - "approve without autoApprove keeps planMode as executing" — was "clears planMode entry"; now planMode lives on as executing object
   - "multi-channel dedup: two approve writes" — second approve now hits the "requires pending approval" guard (INVALID_REQUEST) instead of the "planMode missing" guard (PLAN_APPROVAL_EXPIRED). Both are correct rejection paths; just a different error code based on which guard catches the stale-second-approve.
   - "approve then reject on same approvalId" — same INVALID_REQUEST vs PLAN_APPROVAL_EXPIRED rationale.

95/95 tests pass across approval.test.ts + sessions-patch.test.ts + plan-snapshot-persister.test.ts. Typecheck clean.

NOT YET WIRED (P2.5+):
  - Mutation gate: still only blocks on mode === "plan" — already correct for executing (mutations allowed). Just needs comment update + diagnostic line.
  - Nudge suppression: cron/isolated-agent/run.ts:431 + heartbeat-runner.ts:721 still use 2-state logic. P2.5 widens to handle "executing" appropriately.
  - UI chip: still shows Default for mode === "executing". P2.6 fixes.
  - plan_mode_status: doesn't surface the new state yet. P2.8.
  - No execution-phase nudges yet. P2.9 adds them.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…nize executing state (P2.5)

Third commit of the Phase-2 architectural refactor. After P2.3 (type widening) + P2.4 (approve-transition writes mode: executing), the four runtime callers that read planMode.mode all need to handle the new value correctly. Three are already correct by accident (their existing logic uses `mode !== "plan"` which inclusively covers both "executing" and "normal"); only the diagnostic + acceptEdits gate needed actual logic changes.

Changes:

1. src/agents/plan-mode/mutation-gate.ts — `checkMutationGate` line 183 was already `if (currentMode !== "plan") return blocked: false`. Comment updated to make the inclusive contract explicit: mutations are unblocked in BOTH "executing" (plan approved, agent is mid-execution) AND "normal" (no plan activity). The guard is intentionally future-proof — any new mode value defaults to mutations-allowed unless explicitly added to the gate-armed set.

2. src/cron/isolated-agent/run.ts:431 — `mode !== "plan"` in the design-phase nudge suppression already correctly skips for "executing" + "normal". Comment added clarifying that execution-phase nudges (P2.9) use a separate cron class with their own suppression checking `mode !== "executing"`.

3. src/infra/heartbeat-runner.ts:735 — `buildActivePlanNudge` same `mode !== "plan"` early-return correctly handles "executing". Comment added explaining the design-phase steering (advance the next step / call exit_plan_mode) is irrelevant once approved.

4. src/agents/pi-tools.before-tool-call.ts:271 — TWO logic changes:

   (a) The `[plan-mode-gate]` diagnostic now also fires for `"executing"` mode AND for `args.ctx?.planMode === "executing"`, so operators can trace the full approve→execute handshake in `gateway.err.log`. Pre-P2.5 the diagnostic only fired during plan-design, losing the post-approve handshake visibility.

   (b) The acceptEdits constraint gate at line 295 was checking `latestPlanMode === "normal"` only. Widened to `"normal" || "executing"` so post-approve mid-execution sessions correctly trigger the constraint check (the user's acceptEdits permission is granted at approve time and applies through execution; pre-P2.5 the gate stopped firing the moment we landed in "executing" state, leaving destructive/self-restart/config-change actions unconstrained mid-execution).

86/86 tests pass across mutation-gate, before-tool-call, run.plan-mode, heartbeat-runner.plan-nudge. Typecheck clean.

NOT YET WIRED (P2.6-P2.9):
  - UI chip still shows "Default" for mode === "executing" (P2.6)
  - Auto-approve toggle still in mode menu (P2.7)
  - plan_mode_status introspection doesn't surface executing (P2.8)
  - No execution-phase nudge cron class yet (P2.9)

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…(P2.6)

Fourth commit of the Phase-2 architectural refactor. Now that the server-side transitions write `mode: "executing"` post-approval (P2.4), the UI selector chip needs to recognize the new value so it stays on the plan-mode chip throughout execution instead of reverting to "Default" the moment approval lands.

User's report (live observation during Eva's MiniMax/David VM session): the chip lying about state was the most-confusing part of the broken decision tree — the system showed "Default" while autoApprove was still armed and the agent was actively executing the approved plan.

Changes:

1. `ui/src/ui/chat/mode-switcher.ts:243` — `resolveCurrentMode` widened to recognize BOTH `"plan"` AND `"executing"` as plan-mode-active states. Both resolve to the Plan / Plan ⚡ chip (Plan ⚡ when autoApprove is true). The chip stays on this entry through the entire plan→executing arc; only when close-on-complete deletes planMode does the chip revert to Default/Ask/Accept/Bypass per execSecurity.

2. `ui/src/ui/app-render.ts:2330` — the inline copy of resolveCurrentMode's plan-aware logic (used for the redundant-selector-click short-circuit) widened to match. Pre-P2.6 the short-circuit missed redundant clicks during executing because currentChipMode returned null, letting redundant patches fire on every selector re-click.

3. `ui/src/ui/types.ts:439` — UI's `SessionsListItem.planMode.mode` field type widened from `"plan" | "normal"` to `"plan" | "executing" | "normal"`. Mirror of the server-side widening in src/config/sessions/types.ts (P2.3). Without this widening, app-render.ts:2341 had a type error: comparison appears unintentional because '"normal" | undefined' and '"executing"' have no overlap.

Tests: 57/57 mode-switcher tests pass (the 11 failures present here are PRE-EXISTING jsdom-env infrastructure issues — `document is not defined` in renderModeSwitcher tests — same failures exist without my changes; not a regression). Typecheck clean.

NOT YET WIRED (P2.7+):
  - Auto-approve toggle decoupling: still in mode menu (P2.7)
  - plan_mode_status introspection: doesn't surface executing yet (P2.8)
  - Execution-phase nudge cron class: not added yet (P2.9)

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…ate summary (P2.8)

Fifth commit of the Phase-2 architectural refactor. The agent's self-introspection tool now distinguishes the new "executing" state explicitly so the agent can correctly answer "where are we" questions and operators debugging stuck sessions get accurate signal.

Changes (single file, ~15 lines):

1. New `inExecution: boolean` field on the structured status JSON, alongside the existing `inPlanMode: boolean`. Both can be false (truly idle); one can be true (designing OR executing); never both true (mode is exclusive).

2. New summary-text branch when `inExecution === true`:

   "Executing approved plan (title=...; approval=approved; N step(s) in progress, M pending, K completed)."

   Counts steps by status from `lastPlanSteps` so the agent has an immediate picture of how much execution work remains. Pre-P2.8 the agent would have seen "Not in plan mode (mode=executing)" — accurate but not actionable.

The summary now has THREE branches:
  - `inPlanMode` (mode === "plan") — designing
  - `inExecution` (mode === "executing") — post-approval execution NEW
  - else — truly idle (mode === "normal" OR no planMode entry)

Plus the existing failure branch (`!sessionStoreReadOk`) that fires when the disk read can't be attempted.

Typecheck clean. No test file for this tool exists — the existing introspection contract is enforced by the runtime usage path; the new field is additive (existing consumers ignore unknown fields).

NOT YET WIRED (P2.7 + P2.9):
  - Auto-approve toggle decoupling: still in mode menu (P2.7)
  - Execution-phase nudge cron class: not added yet (P2.9)

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…5 min during executing state (P2.9)

Sixth commit of the Phase-2 architectural refactor. Adds a new cron class that wakes the agent during the post-approval execution phase to nudge it on stalls — the gap that left Eva idle after subagent returns without marking plan steps complete.

The design-phase nudge (10/30/60 min) was always plan-mode-bound (per Eva PR-3 fix #5 cycleId guard). Once approval lands and mode → "executing", design-phase nudges correctly stop firing — but there was NO mechanism to push the agent during execution stalls. This commit adds the missing mechanism.

Architecture:

1. NEW src/agents/plan-mode/plan-execution-nudge-crons.ts (~210 lines)
   - `schedulePlanExecutionNudges()` — sibling to schedulePlanNudges
   - Default intervals: [1, 3, 5] minutes (vs [10, 30, 60] for design)
   - `cleanupPlanExecutionNudges()` — sibling to cleanupPlanNudges
   - Cron name prefix: `plan-execution-nudge:` so telemetry can distinguish
   - Message body: narrower than design-phase ("call update_plan to mark step done if it's complete" vs "advance the next step") — agent's response should be focused on completing the approved plan, not re-planning

2. src/config/sessions/types.ts — added `executionNudgeJobIds?: string[]` to SessionEntry.planMode (sibling to existing nudgeJobIds for design-phase tracking)

3. src/cron/types.ts — added `executionCycleId?: string` to CronAgentTurnPayloadFields. Distinct from planCycleId so the cron-fire-time guard can route to the right suppression check.

4. src/cron/isolated-agent/run.ts — added cron-fire-time suppression for execution-phase nudges. Skips when:
   - mode !== "executing" (plan closed OR reverted to plan-design)
   - cycleId !== payload.executionCycleId (older executing cycle)
   - all lastPlanSteps are completed/cancelled (close-on-complete pending; waking right before auto-close would just generate noise)

5. src/gateway/sessions-patch.ts — TWO new sites:
   (a) Schedule on approve transition (action === "approve" only; "edit" excluded because acceptEdits permission already implies post-approval execution and would compete with self-modification flow). Fire-and-forget after the patch resolves; persists the resulting jobIds back to entry.planMode.executionNudgeJobIds via a follow-up updateSessionStoreEntry call. Race-safe: if close-on-complete fires between schedule and persist, the closure detects the missing planMode + immediately cancels the just-scheduled crons.
   (b) Cleanup on close-on-complete (mode === "normal" only). Sibling block to the design-phase cleanup at line 1146. Strips executionNudgeJobIds from the entry to avoid double-cancel.

Single scheduling site in sessions-patch covers ALL approve paths (manual UI click, autoApproveIfEnabled, Telegram /plan accept, future channel impls) with no per-channel wiring required.

368/368 tests pass across plan-mode/, sessions-patch.test.ts, run.plan-mode.test.ts. Typecheck clean.

NOT YET WIRED:
  - P2.7: Auto-approve toggle decoupling from mode menu
  - P2.10: shorten/delete obsolete design-phase intervals (judgment call)
  - End-to-end smoke test of the full plan → executing → nudge → close lifecycle (deferred until P2.7 lands + final gateway bounce)

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…race-check hardening + append semantics + edit scheduling + migration tightening

Hardening commit landing all CRITICAL findings from 3 adversarial reviewers on commits daf00a29d4 (P2.3) through a706894c51 (P2.9). No new features; all fixes target correctness / race-safety / observability of the existing Phase-2 work.

## Fixes in this commit

### P2.9 critical fixes (Agent 3)

1. **C1+C2 — close-on-complete cleanup wired to correct branch.** The adversarial reviewer caught that close-on-complete fires via plan-snapshot-persister.ts:696 as a RAW `planMode: "normal"` patch (NOT as planApproval), so the original cleanup-block at sessions-patch.ts:1177 (inside the planApproval branch) was NEVER REACHED on the primary lifecycle path. Every successful plan cycle leaked 3 execution-phase cron jobs (self-scrubbed only on fire via deleteAfterRun + guard skip). Fix: mirror the cleanup into the raw `"normal"` branch at sessions-patch.ts:517-540, converging cleanup paths so close-on-complete + /plan off + UI "Default" click all correctly cancel the scheduled crons. Also: do NOT carry forward executionNudgeJobIds into the autoApprove-preserve entry (they were just cancelled; keeping them would orphan them).

2. **C3 — race-check in fire-and-forget closure strengthened.** Original check was only `if (!entry?.planMode)`. Review found that autoApprove-preserve close keeps planMode alive as `mode: "normal"` (so the check misses), and fresh `/plan on` between schedule and persist mints a new cycleId (so the check misses, causing pollution of the new plan-design entry with stale IDs). Widened to a 3-pronged check mirroring design-phase semantics:
   - planMode deleted OR
   - mode !== "executing" OR
   - cycleId !== executionCycleId (scope-match)

3. **C4 — append executionNudgeJobIds instead of overwriting.** Original `executionNudgeJobIds: scheduled.map(...)` silently dropped any prior batch. Now `[...(entry.planMode.executionNudgeJobIds ?? []), ...scheduled.map((s) => s.jobId)]`, mirroring design-phase persistence at pi-embedded-subscribe.handlers.tools.ts:563.

4. **P7 — `action === "edit"` now ALSO schedules execution-phase nudges.** Edit transitions mode → "executing" (approval.ts:113) same as approve, but original guard `if (action === "approve")` excluded it. Users who accept-with-edits got zero stall coverage. Extended to `action === "approve" || action === "edit"`.

### P2.3 migration tightening (Agent 1)

5. **Backfill false-positive fix.** Original rule (mode === "normal" + non-empty lastPlanSteps + any pending/in_progress) could backfill a rejected-then-`/plan off` session to mode: "executing" despite never having been approved. Tightened to also require `approval === "approved" || approval === "edited"`. Without this, plan_mode_status would misreport "Executing approved plan…" for sessions that never had an approved plan.

## NOT fixed in this commit (deferred or accepted)

- **A2 log volume** — `[plan-mode-gate]` firing on every executing-phase tool call is noise-heavy but non-correctness. Deferred to a follow-up: could be debug-level OR first-N-calls-per-cycle gated.
- **A3 idle-threshold guard on exec nudges** — 1-min first interval can fire during a legitimately-long 61-sec tool call. Defer; add an `updatedAt > now-threshold` guard in a follow-up commit if live testing shows spurious wake-ups.
- **A3 test coverage gaps** — none of: migration tests, exec-nudge cron tests, subagent-announce executing-branch test, fresh-session-entry executing return path test. Add in a dedicated test-coverage commit BEFORE upstream cherry-pick. Local smoke test first (gateway bounce) establishes functional correctness; then we add tests.
- **A1 stale doc comments** — references to "planMode gets deleted on approve" in 6 files. Cosmetic but high signal-to-noise. Bundled into a future docs-refresh commit.
- **A1 stuck-executing safety net** — if agent abandons mid-execution, no timeout-driven transition back to normal. Design call; document.

Typecheck clean. 361/361 targeted tests pass (plan-mode, sessions-patch, sessions/).

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
… (P2.9 live smoke-test fix)

CRITICAL Phase-2 smoke-test finding (2026-04-21 ~22:35 GMT+7). Every approve transition since bounce was firing execution-phase nudge scheduling, and ALL cron.add calls were being rejected with:

  errorCode=INVALID_REQUEST
  errorMessage=invalid cron.add params:
    at /payload: unexpected property 'executionCycleId';
    at /payload: must match ...

8 approve cycles in the smoke test = 24 rejected cron.add attempts = ZERO execution-phase nudges actually scheduled. This is why the user observed Eva stalling without nudges firing.

Root cause: P2.9 (a706894c51) added `executionCycleId` field to:
  - CronAgentTurnPayloadFields (src/cron/types.ts) ✓
  - The scheduler + guard code (new plan-execution-nudge-crons.ts, run.ts:481 suppression) ✓

But I MISSED the wire-protocol schema at `src/gateway/protocol/schema/cron.ts:9`. The protocol-level validator has `additionalProperties: false`, so any unknown field gets rejected at cron.add time — the runtime Zod/TypeBox schema is the source of truth for wire validation, NOT the TypeScript types.

Fix: add `executionCycleId: Type.Optional(Type.String())` to `cronAgentTurnPayloadSchema`, sibling to the existing `planCycleId` field. Rebuild + gateway bounce required to pick up the widened validator.

This was discovered live during smoke test — the adversarial review agents (3 of them) didn't catch this because their scope was static code analysis. Live validation against actual cron.add was required. Moving forward the test-coverage commit (todo #5 in plan file) should include a scheduling integration test that actually calls cron.add and asserts the resulting payload round-trips.

Post-fix expected behavior: every approve/edit transition schedules 3 execution-phase nudges (1/3/5 min). Next approve should emit `plan-execution-nudge crons scheduled: sessionKey=... count=3` in the log, and Eva should see a nudge if she stalls for 1+ min during executing.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…9 observability gap)

Live smoke test on 2026-04-21 confirmed the execution-phase nudge fix (37bb87feaf) works end-to-end: 6 crons scheduled, persisted to `executionNudgeJobIds`, appended correctly across approve cycles.

BUT the scheduler's internal `plan-execution-nudge crons scheduled: sessionKey=... count=N` confirmation log was never emitted. Root cause: the fire-and-forget closure in sessions-patch.ts didn't pass a `log` parameter to `schedulePlanExecutionNudges()`, so `params.log?.info?.(...)` was a no-op. The `[gateway] cron: job created` lines from the cron subsystem DID fire, proving the scheduling worked — but the higher-level "N scheduled for this cycle" summary was silent.

Fix: add a dedicated `createSubsystemLogger("gateway/plan-execution-nudge")` module-level logger, thread it to the scheduler as the `log` param. Future smoke tests / operators can grep `gateway/plan-execution-nudge` for the scheduling summary.

This was caught by live validation that didn't have a static-analysis equivalent — the 3 adversarial reviewers all reviewed logic flow but none checked "does the scheduler's internal log line actually reach a sink?" Filing this class of miss as a reminder for future review prompts: include "verify observability logs actually reach a configured sink" in the review checklist.

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.
…ative steps (verify-via-tool discipline)

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.

## Why

Live-validated failure mode (2026-04-22, Eva Baoyu-flyer investigation):
the +1/+3/+5 execution-phase nudges fired correctly but Eva returned
NO_REPLY three times claiming internally "all steps marked complete"
— while plan_mode_status showed 1 in_progress + 3 pending + 2
completed (4 of 6 unaccounted). Agent conflated "did the work" with
"called update_plan on it."

The prior nudge text said "Before returning NO_REPLY, call
plan_mode_status to verify..." which works as a gentle suggestion
but leaves room for the model to skip the tool call if it believes
it already knows the state. Adversarial review #1 round-2 flagged
the closing sentence "If plan_mode_status confirms zero
pending+in_progress steps, close-on-complete fires automatically —
no further action needed" as itself an escape hatch: the model
reads it as permission to NO_REPLY without calling the tool at all,
reinstating the exact failure mode.

## What

Restructures the nudge body as a 4-step imperative checklist:
  (1) Call plan_mode_status — unconditional, do NOT skip even if
      you believe you know the answer
  (2) Based ONLY on what the tool reports, call update_plan
  (3) If blocked on external wait, schedule another resume
  (4) If tool shows zero pending+in_progress, return a brief
      completion summary (NOT silent NO_REPLY)

Explicitly names the failure mode inline: "your internal memory of
which steps you marked via update_plan is unreliable across turns."

## Files

- src/agents/plan-mode/plan-execution-nudge-crons.ts — rewrite the
  message body of schedulePlanExecutionNudges + add comment
  explaining the imperative-step restructuring.
- src/config/sessions/store-migrations.ts — fix pre-existing
  eslint(curly) violation in the P2.3 migration added in 24889c7831
  (surfaced by a recent lint rule change; blocked the commit hook).

## Tests

The nudge text is baked into the cron.add payload at schedule time
— pure string content, no parser contract. Existing
plan-nudge-crons.test.ts structural assertions pass (17/17). Byte-
stability / schema-round-trip test deferred to the test-coverage
commit.

## Related

- Composed with P2.12b (execution-status preamble) to break the
  "trust your internal memory" failure mode at both the nudge
  content level (here) AND the turn-preamble level (P2.12b).
- Adversarial review consolidated fix list: addresses R1/MINOR-2
  (escape-hatch closing sentence).
…ting-mode turns (both user-reply and cron-nudge paths)

LOCAL TESTING ARTIFACT — do NOT push to origin or upstream.

Companion to P2.12a (1411c0d99b). The nudge-text tweak breaks the
NO_REPLY escape hatch in the cron message; this change adds a
turn-preamble system that delivers ground-truth plan state on EVERY
executing-mode turn so the agent never needs to rely on its lossy
internal memory of its own update_plan calls.

Same live failure mode that motivated P2.12a (Eva Baoyu-flyer,
2026-04-22): the agent's internal belief "I already marked all
steps complete" contradicted plan_mode_status's 2-completed,
1-in-progress, 3-pending reality. The model treated the
approved-plan injection text (baked into conversation history at
approve time) as an authoritative state snapshot — but that text
never mutates to reflect subsequent update_plan calls, so the
snapshot goes stale immediately.

New module src/agents/plan-mode/execution-status-injection.ts:

  buildExecutionStatusInjection(sessionKey, { storePath? })
    → reads planMode from disk (skipCache: true)
    → returns a compact [PLAN_STATUS]: preamble with step counts +
      in-progress step title when mode === "executing"
    → returns undefined otherwise (not-executing, no steps, read
      failed — fail-open)
    → all logging via createSubsystemLogger("plan-status-injection")
      (adversarial review #3 M2: logs must reach a sink without
      caller-threaded log-arg dependencies)

  prependExecutionStatusIfExecuting(prompt, sessionKey, options)
    → shared convenience wrapper used by both callers

1. src/auto-reply/reply/agent-runner-execution.ts — the user-reply +
   heartbeat path. Composed ahead of hoistedPendingInjection so the
   agent reads live status before consuming any [PLAN_DECISION] /
   [QUESTION_ANSWER] one-shots. Now passes params.storePath to avoid
   redundant loadConfig() (adversarial review #2 MAJOR 4).

2. src/cron/isolated-agent/run-executor.ts — the cron-nudge path.
   runPrompt() prepends the preamble to its promptText before
   runCliAgent / runEmbeddedPiAgent. Adversarial review #2 MAJOR 1
   flagged this as a SHIP-BLOCKER: without it, the feature never
   fires on the exact Eva stall scenario (+1/+3/+5 nudges hitting
   an otherwise-silent agent).

  [PLAN_STATUS]: Executing approved plan "<truncated title, 120ch>".
  Steps: N/total completed, N in_progress, N pending[, N cancelled][, N unrecognized].
    Current in_progress step: "<truncated step, 140ch>"
    This snapshot is captured at turn-start and may be stale;
    plan_mode_status is authoritative if they disagree. Before
    returning NO_REPLY, call plan_mode_status to verify, then call
    update_plan to mark any finished step "completed" or "cancelled"
    based on what the tool reports. close-on-complete fires
    automatically once pending=0 + in_progress=0.

Adversarial review #1 MAJOR 2 fix: the earlier draft said "Trust
this snapshot over your internal memory" which CONTRADICTED the
P2.12a nudge's "plan_mode_status is authoritative" instruction.
The revised language defers to the tool so preamble and nudge agree.

- Unknown step statuses (e.g. "skipped", "blocked", "completed " with
  trailing whitespace) go into an "unrecognized" bucket — the sum of
  known + unknown always equals total, so the agent never sees a
  partially-accounted step list (R1/MINOR-1 + R3/m1).
- planMode.title truncated to 120 chars to prevent prompt-bloat vector
  on pathological long titles (R2/MAJOR-3).
- buildExecutionStatusInjection accepts optional storePath to skip
  the redundant loadConfig() + resolveStorePath round-trip when the
  caller already has it resolved (R2/MAJOR-4).

22 new tests in execution-status-injection.test.ts covering:
  - non-executing modes return undefined (plan, normal, missing planMode)
  - correct counts for multi-status steps
  - unrecognized bucket for unknown status values + strict-match test
    for trailing-whitespace status
  - in-progress line omission when no in-progress step OR empty step
  - step title + plan title truncation at respective caps
  - fail-open on malformed JSON, missing file, empty/whitespace key
  - canonical "plan_mode_status is authoritative" language in output
  - prependExecutionStatusIfExecuting wrapper composition semantics

Run: 22/22 new tests pass. 330/330 plan-mode tests pass. 7/7
cron plan-mode tests pass. Pre-existing cross-file test pollution
in the broader auto-reply suite is unrelated (same-shape flakiness
on baseline — ACP/Slack/plugin-install tests).

- Integration test asserting the preamble reaches runEmbeddedPiAgent
  (R2/MINOR-7). Low-priority — live smoke test will catch any wiring
  regression via monitor stream.
- Prompt-cache invalidation (R2/MAJOR-2) — every executing-mode turn
  has different step counts → cache-prefix changes. Requires moving
  the preamble to a system-message suffix instead of user-message
  prefix. Deferred as a separate PR because it touches the prompt
  assembly path.
- Fallback re-entrancy (R2/MAJOR-5) — preamble is read pre-fallback
  and reused on retry. Rare edge case; defer or accept.

All three touched files are loaded in-process. A dist/ rebuild +
gateway bounce picks up:
  - execution-status-injection.ts (imported by both callers)
  - agent-runner-execution.ts (user-reply hoist composition)
  - run-executor.ts (cron-nudge prepend)

Next executing-mode turn after the bounce fires the preamble.
…llowlist (live-blocked catch-22 audit)

LOCAL TESTING ARTIFACT — for upstream Part-10 stack on top of #69449's
9-part decomposition. Closes the same class of catch-22 as the
agents_list fix in 4a005791ef but for three more tools live-observed
hitting the default-deny gate during Eva's Baoyu investigation.

## Tools added

- **`sessions_yield`** — agent calls this to suspend itself while
  waiting for spawned subagents to finish. Read-only (only signals
  the runtime to park this turn's promise; no workspace mutation).
  Without allowlist entry, an agent that correctly used `sessions_spawn`
  hits a catch-22: "you can spawn but cannot wait." The error message
  even suggests `exit_plan_mode` which is the wrong escape — the
  agent is mid-investigation, not done.
- **`lcm_grep`** — lossless-claw memory plugin search-by-substring.
  Pure-read. Live-blocked 2026-04-22 01:11 GMT+7 when Eva tried to
  grep prior memory while investigating Baoyu skill install state.
- **`lcm_expand_query`** — lossless-claw memory query expansion.
  Pure-read; surfaces related memory facts.

## Excluded (intentionally NOT added)

- `lcm_backfill_*` (mutates memory store)
- `lcm_rollup` (mutates memory store)
- `lcm_migration_state` (configuration mutation)

## Pre-existing test fix folded in

The `blockedTools` test list had `sessions_spawn` even though
`sessions_spawn` is in the canonical PLAN_MODE_ALLOWED_TOOLS set
(allowed for research-subagent flows). This was a long-standing
test-vs-source drift that surfaced when running tests during this
allowlist audit. Removed `sessions_spawn` from the blocked-tools test
to keep the test in sync with the canonical allowlist.

## Tests

- 64/64 mutation-gate tests pass
- 3 new tests added to the `allowedTools` block covering
  sessions_yield, lcm_grep, lcm_expand_query
…ertion (rebase fixup)

Pure rebase fixup. Between when this byte-stability assertion was
originally written and the post-v2026.4.21 rebase, main moved the
`[PLAN_DECISION]: approved\n\n` prefix out of `buildApprovedPlanInjection`'s
output and into the synthetic-injection composer (the caller). The
contract is functionally equivalent (the agent still receives the
same prepended text) but now the prefix is composed at injection
time rather than baked into this helper's output.

Updated assertion drops the prefix; the rest of the snapshot is
byte-identical to before. Also added a comment explaining the
contract change so future readers understand why the prefix moved.

No production code changes.
Copilot AI review requested due to automatic review settings April 22, 2026 06:20
@greptile-apps

greptile-apps Bot commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Too many files changed for review. (190 files found, 100 file limit)

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: telegram Channel integration: telegram app: macos App: macos app: web-ui App: web-ui gateway Gateway runtime commands Command implementations agents Agent runtime and tooling size: XL labels Apr 22, 2026

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

Note

Copilot was unable to run its full agentic suite in this review.

Implements the final wave of plan-mode lifecycle work: adds executing-state support, hardens debugging/repair paths, and expands tooling for plan approvals and operator diagnostics across channels.

Changes:

  • Adds plan-mode tool surface (enter_plan_mode, plan_mode_status, ask_user_question) plus universal /plan command entrypoints and display/catalog wiring.
  • Introduces executing-phase support (mode "executing", execution-status auto-injection) and strengthens subagent gating (concurrency cap + parent open-set tracking/draining).
  • Adds plan templates for skills (frontmatter parsing + snapshot propagation + payload builder) and improves transport/tool reconstruction logging.

Reviewed changes

Copilot reviewed 95 out of 190 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/auto-reply/commands-registry.shared.ts Adds universal /plan slash command to resolve approvals on all channels.
src/agents/transport-message-transform.ts Adds bounded “missing tool_result” repair placeholder + log-volume/memory caps.
src/agents/tools/update-plan-tool.test.ts Updates expectations for non-empty tool content; adds closure/criteria gate tests.
src/agents/tools/sessions-spawn-tool.ts Adds plan-mode subagent concurrency cap, plan-mode cleanup override, and parent/child run tracking.
src/agents/tools/sessions-spawn-tool.test.ts Removes role-forwarding assertions; adds regression tests for plan-mode concurrency cap.
src/agents/tools/plan-mode-status-tool.ts New read-only tool to introspect plan-mode/executing state + subagent gate/debug state.
src/agents/tools/enter-plan-mode-tool.ts New tool stub for entering plan mode (runner performs the session-state mutation).
src/agents/tools/cron-tool.ts Adds operator recipe docs for “resume after wait” cron usage in current session.
src/agents/tools/ask-user-question-tool.ts New tool to ask multiple-choice question through approval pipeline (plan-mode safe).
src/agents/tools/ask-user-question-tool.test.ts Adds schema/validation/metadata tests for ask_user_question.
src/agents/tool-display-config.ts Adds display config entries for plan-mode tools and fixes ask_user_question detailKeys.
src/agents/tool-description-presets.ts Adds display summaries + detailed descriptions for new plan-mode tools; clarifies update_plan contract.
src/agents/tool-catalog.ts Registers plan-mode tools in core tool catalog behind plan-mode enablement.
src/agents/test-helpers/fast-openclaw-tools-sessions.ts Updates update_plan mock signature + exports PLAN_STEP_STATUSES for tests.
src/agents/subagent-registry.test.ts Updates agent-events mock and normalizes outcome assertions.
src/agents/subagent-registry.steer-restart.test.ts Uses partial actual agent-events module; adds test for parent open-subagent remap on steer restart.
src/agents/subagent-registry-run-manager.ts Drains parent open-subagent sets on completion/kill; replaces open-run IDs on steer restart; outcome equality update.
src/agents/subagent-announce.ts Adds plan-mode-aware suffix to announce instruction when requester is in plan mode.
src/agents/skills/workspace.ts Carries skill plan templates into snapshots so template seeding works with snapshot-backed runs.
src/agents/skills/types.ts Adds skill plan template types + snapshot field for resolved plan templates.
src/agents/skills/skill-planner.ts New module to normalize/dedupe/truncate skill plan templates into an update_plan payload.
src/agents/skills/frontmatter.ts Parses plan templates from frontmatter (kebab/camel + step/content alias).
src/agents/skills/frontmatter.test.ts Adds coverage for planTemplate parsing precedence + alias behavior.
src/agents/plan-mode/types.ts Introduces 3-state PlanMode union and approval-id generation + feedback sanitization.
src/agents/plan-mode/reference-card.ts Adds bootstrap-injected reference card for plan-mode lifecycle and debugging.
src/agents/plan-mode/plan-nudge-crons.ts Adds scheduling/cleanup for plan-mode nudge cron jobs with validation and best-effort semantics.
src/agents/plan-mode/plan-mode-debug-log.ts Adds structured plan-mode debug logger with env/config gating and TTL-cached config read.
src/agents/plan-mode/plan-archetype-prompt.ts Adds plan-mode archetype prompt + helpers to build persisted plan filenames/slugs.
src/agents/plan-mode/plan-archetype-prompt.test.ts Adds tests for prompt fragment presence + filename helper behavior.
src/agents/plan-mode/plan-archetype-bridge.ts Implements plan archetype markdown persistence + Telegram attachment delivery bridge.
src/agents/plan-mode/mutation-gate.test.ts Adds coverage for mutation gate allow/deny lists and exec-read-only whitelist/guards.
src/agents/plan-mode/integration.test.ts Adds plan-mode wiring integration smoke test for enablement + before-tool-call gating.
src/agents/plan-mode/index.ts Exports plan-mode public API types/helpers.
src/agents/plan-mode/execution-status-injection.ts Adds [PLAN_STATUS] executing-mode preamble injection with fail-open disk read.
src/agents/plan-mode/auto-enable.ts Adds regex-based auto-enable matcher with compiled-regex cache and defensive behavior.
src/agents/plan-mode/auto-enable.test.ts Adds tests for auto-enable matcher, malformed patterns, and cache behavior.
src/agents/plan-mode/approval.ts Adds approval state machine with stale-event guard and executing-state transition.
src/agents/plan-hydration.ts Adds post-compaction active-plan hydration injection formatting.
src/agents/plan-hydration.test.ts Adds tests for plan hydration formatting and filtering behavior.
src/agents/pi-tools.ts Threads planMode/live accessors through to before-tool-call hook context.
src/agents/pi-tools.before-tool-call.ts Adds mutation gate + acceptEdits gate enforcement with live planMode lookup and diagnostics.
src/agents/pi-embedded-runner/skills-runtime.test.ts Updates snapshot behavior tests for resolvedPlanTemplates backward-compat.
src/agents/pi-embedded-runner/run/params.ts Adds planMode and live accessors to embedded runner params.
src/agents/pi-embedded-runner/run/helpers.ts Increases retry iteration defaults; adds user override and introduces subagent-specific cap constant.
src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test-support.ts Stubs applySkillPlanTemplateSeed in test support.
src/agents/pi-embedded-runner/run.overflow-compaction.test.ts Sets small per-test maxIterations override to keep CI fast with new defaults.
src/agents/pi-embedded-runner/run.incomplete-turn.test.ts Updates strict-agentic retry counts; adds escalating instructions and plan-mode disablement checks.
src/agents/pi-embedded-runner/pending-injection.ts Adds compatibility shim over new typed pending injection queue.
src/agents/pi-embedded-runner/pending-injection.test.ts Adds tests for composition and once-only consumption semantics.
src/agents/openclaw-tools.ts Registers plan-mode tools behind enable flag; threads runId into update_plan and sessions_spawn.
src/agents/openclaw-tools.registration.ts Adds config gate for plan-mode tool registration.
skills/plan-mode-101/SKILL.md Adds plan-mode reference/self-test skill documentation mirroring reference card.
qa/scenarios/gpt54-plan-mode-default-off.md Adds QA scenario to validate plan-mode default-off behavior with explicit toolCalls checks.
qa/scenarios/gpt54-mandatory-tool-use.md Adds QA scenario ensuring GPT-5.4 uses tools for factual queries.
qa/scenarios/gpt54-injection-scan.md Adds QA scenario validating injection scanner baseline behavior.
qa/scenarios/gpt54-cancelled-status.md Adds QA scenario around cancelled plan-step status behavior.
qa/scenarios/gpt54-act-dont-ask.md Adds QA scenario validating “act on defaults” behavior (exec usage).
extensions/telegram/src/send.ts Adds sendDocumentTelegram with size/caption caps and diag/thread fallback handling.
extensions/telegram/runtime-api.ts Re-exports sendDocumentTelegram (+ opts type) via Telegram runtime API.
docs/tools/slash-commands.md Documents /plan command surface across channels.
docs/plans/PLAN-MODE-OPERATOR-RUNBOOK.md Adds operator runbook for production incident triage/debugging.
docs/concepts/plan-mode.md Adds user-facing plan-mode concept doc and troubleshooting pointers.
apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift Adds plan approval error codes and sessions.patch params for plan-mode fields.
apps/shared/OpenClawKit/Sources/OpenClawKit/Resources/tool-display.json Adds tool display metadata for plan-mode tools in client resources.
apps/macos/Sources/OpenClawProtocol/GatewayModels.swift Mirrors shared Swift protocol updates for macOS target.

Comment on lines +104 to +114
const planModeSuffix =
params.requesterPlanMode === "plan"
? " You are currently in PLAN MODE — do not stop after the user-facing update. Your next action MUST be either: (a) call `exit_plan_mode(title=..., plan=[...])` if this subagent's result completes your investigation, OR (b) continue investigation with another read-only tool call. Trailing chat alone is treated as yielding without acting and will trigger a [PLAN_ACK_ONLY] retry."
: "";
if (params.requesterIsSubagent) {
return `Convert this completion into a concise internal orchestration update for your parent agent in your own words. Keep this internal context private (don't mention system/log/stats/session details or announce type). If this result is duplicate or no update is needed, reply ONLY: ${SILENT_REPLY_TOKEN}.`;
return `Convert this completion into a concise internal orchestration update for your parent agent in your own words. Keep this internal context private (don't mention system/log/stats/session details or announce type). If this result is duplicate or no update is needed, reply ONLY: ${SILENT_REPLY_TOKEN}.${planModeSuffix}`;
}
if (params.expectsCompletionMessage) {
return `A completed ${params.announceType} is ready for user delivery. Convert the result above into your normal assistant voice and send that user-facing update now. Keep this internal context private (don't mention system/log/stats/session details or announce type).`;
return `A completed ${params.announceType} is ready for user delivery. Convert the result above into your normal assistant voice and send that user-facing update now. Keep this internal context private (don't mention system/log/stats/session details or announce type).${planModeSuffix}`;
}
return `A completed ${params.announceType} is ready for user delivery. Convert the result above into your normal assistant voice and send that user-facing update now. Keep this internal context private (don't mention system/log/stats/session details or announce type), and do not copy the internal event text verbatim. Reply ONLY: ${SILENT_REPLY_TOKEN} if this exact result was already delivered to the user in this same turn.`;
return `A completed ${params.announceType} is ready for user delivery. Convert the result above into your normal assistant voice and send that user-facing update now. Keep this internal context private (don't mention system/log/stats/session details or announce type), and do not copy the internal event text verbatim. Reply ONLY: ${SILENT_REPLY_TOKEN} if this exact result was already delivered to the user in this same turn.${planModeSuffix}`;
Comment on lines +67 to +83
export function buildPlanAttachmentCaption(
title: string | undefined,
summary: string | undefined,
): string {
const safeTitle = (title ?? "").trim() || "Plan";
const escTitle = escapeHtml(safeTitle);
const safeSummary = (summary ?? "").trim();
const summaryLine = safeSummary ? `\n${escapeHtml(safeSummary)}` : "";
return [
`<b>${escTitle}</b> — plan submitted for approval. See attached.`,
summaryLine,
"",
"Resolve with: <code>/plan accept</code> | <code>/plan accept edits</code> | <code>/plan revise &lt;feedback&gt;</code>",
]
.filter(Boolean)
.join("\n");
}
Comment on lines +80 to +123
for (const minutes of intervals) {
if (minutes <= 0 || !Number.isFinite(minutes)) {
continue;
}
const fireAtMs = now + Math.floor(minutes * 60_000);
const fireAtIso = new Date(fireAtMs).toISOString();
try {
// The wake-up message intentionally references plan state by
// saying "your plan" — when this fires, the resumed agent turn
// reads SessionEntry.planMode.lastPlanSteps from disk and
// figures out which step to advance. If plan mode has already
// been exited / completed by the time this fires, the
// heartbeat-runner's `buildActivePlanNudge` returns null and
// the turn degrades to standard heartbeat behavior (no-op).
// Live-test iteration 1 Bug 1: `[PLAN_NUDGE]:` prefix matches the
// family of plan-mode synthetic messages so channel renderers
// can identify + future PRs can hide them from user-visible chat.
const message =
`[PLAN_NUDGE]: Plan-nudge wake-up (+${minutes}min): if your plan is still active, ` +
"advance the next step. If you're blocked on an external wait, schedule " +
"another resume via cron sessionTarget:'current'. If the plan is " +
"complete, exit_plan_mode (or update_plan with all steps marked " +
"completed/cancelled to auto-close).";
// Copilot review #68939 (2026-04-19): validate the sessionKey
// against the same constraints the cron service applies in
// `assertSafeCronSessionTargetId` (no `/`, `\`, or `\0`
// characters). Pre-fix, an exotic sessionKey could produce a
// sessionTarget the cron jobs.ts validator rejects, causing
// the plan-nudge schedule call to fail at gateway-side
// assertion time. Belt-and-suspenders here keeps the failure
// local + actionable instead of a generic cron-validator
// error 60 seconds later.
const { assertSafeCronSessionTargetId } = await import("../../cron/session-target.js");
try {
assertSafeCronSessionTargetId(params.sessionKey);
} catch (validationErr) {
params.log?.warn?.(
`plan-nudge schedule skipped: sessionKey "${params.sessionKey}" fails cron sessionTarget validation: ${
validationErr instanceof Error ? validationErr.message : String(validationErr)
}`,
);
continue;
}
const job: Record<string, unknown> = {
Comment on lines +63 to +109
export function buildPlanTemplatePayload(
skillName: string,
template?: SkillPlanTemplateStep[],
options?: BuildPlanTemplateOptions,
): PlanTemplatePayload | null {
if (!template || template.length === 0) {
return null;
}

const maxSteps =
options?.maxSteps && options.maxSteps > 0 ? options.maxSteps : DEFAULT_MAX_PLAN_TEMPLATE_STEPS;

// Dedup by step text — keep first occurrence, record dropped duplicates.
const seen = new Set<string>();
const droppedDuplicates: string[] = [];
const deduped: SkillPlanTemplateStep[] = [];
for (const step of template) {
if (seen.has(step.step)) {
droppedDuplicates.push(step.step);
continue;
}
seen.add(step.step);
deduped.push(step);
}

if (deduped.length === 0) {
return null;
}

// Apply upper bound. Truncation drops the tail, since later steps are
// less likely to be reached anyway and we want the seed to model the
// "first N actions" the agent should take.
const truncated = deduped.length > maxSteps;
const final = truncated ? deduped.slice(0, maxSteps) : deduped;

return {
plan: final.map((t) => ({
step: t.step,
status: "pending" as const,
...(t.activeForm ? { activeForm: t.activeForm } : {}),
})),
explanation: `Auto-populated from skill "${skillName}" plan template.`,
...(droppedDuplicates.length > 0 ? { droppedDuplicates } : {}),
...(truncated ? { truncated: true } : {}),
maxSteps,
};
}
Comment on lines +103 to +132
export async function buildExecutionStatusInjection(
sessionKey: string,
options?: BuildExecutionStatusInjectionOptions,
): Promise<string | undefined> {
try {
if (!sessionKey || sessionKey.trim().length === 0) {
return undefined;
}
let storePath = options?.storePath;
if (!storePath) {
const cfg = loadConfig();
const parsed = parseAgentSessionKey(sessionKey);
storePath = resolveStorePath(
cfg.session?.store,
parsed?.agentId ? { agentId: parsed.agentId } : {},
);
}
const store = loadSessionStore(storePath, { skipCache: true });
const entry: SessionEntry | undefined = store?.[sessionKey];
if (!entry?.planMode) {
return undefined;
}
const planMode = entry.planMode;
if (planMode.mode !== "executing") {
return undefined;
}
const steps = planMode.lastPlanSteps ?? [];
if (steps.length === 0) {
return undefined;
}

@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: 91b373b828

ℹ️ 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".

sessionId: options?.sessionId,
onYield: options?.onYield,
}),
createSessionsSpawnTool({

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 Remove duplicate sessions_spawn registration

This adds a second sessions_spawn tool while the original non-embedded registration still exists earlier in the same tool list, creating duplicate tool names in non-embedded runs. Call sites that resolve tools by first name match (for example src/gateway/tools-invoke-http.ts:243) can execute the first, runId-less variant, which bypasses the new run-context-dependent plan-mode protections (subagent tracking/concurrency gating) that this commit intended to enforce.

Useful? React with 👍 / 👎.

Comment on lines 771 to 774
groupId: params.groupId,
groupChannel: params.groupChannel,
groupSpace: params.groupSpace,
memberRoleIds: params.memberRoleIds,
spawnedBy: params.spawnedBy,

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 to attempt execution

The attempt call no longer forwards memberRoleIds and isCanonicalWorkspace, even though downstream logic still relies on both (attempt.ts uses memberRoleIds when creating coding tools and isCanonicalWorkspace for bootstrap routing). In group/role-scoped sessions this causes role-aware policy resolution to run without requester roles, and in non-canonical workspace runs bootstrap mode is evaluated with the wrong canonicality signal.

Useful? React with 👍 / 👎.

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

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 Badge Preserve image attachments in hook model-selection input

resolveHookModelSelection is now invoked without attachments, so before_model_resolve hooks no longer receive image metadata for multimodal prompts. Hook implementations that switch to a vision-capable model based on attachments will miss that signal and can keep a text-only model, causing failures only when image inputs are present.

Useful? React with 👍 / 👎.

@100yenadmin

Copy link
Copy Markdown
Contributor Author

Closed for sequential rollout — please disregard cumulative diff.

This PR was opened with --base main against the upstream repo. Cross-repo PRs can't reference fork branches as bases, so opening all 8 stack parts simultaneously produced cumulative diffs (each PR included all prior parts, 10k → 30k lines). That's review-hostile and reproduces exactly the problem the original umbrella #68939 had.

The fix: sequential rollout. Each [Plan Mode N/9] PR opens only AFTER the previous one merges to main, so its diff is incremental.

Currently open:

Next: when 1/9 merges, [Plan Mode 2/9] Core backend MVP opens with a clean ~2,000-line diff. Etc.

This PR's branch is preserved on the fork and will be reopened with a clean diff in its turn.

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 channel: telegram Channel integration: telegram commands Command implementations docs Improvements or additions to documentation gateway Gateway runtime size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants