[Plan Mode 4/8] Automation + subagent follow-ups#70016
Conversation
…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.
|
Too many files changed for review. ( |
openclaw-barnacle
Bot
added
app: macos
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds/extends Plan Mode automation, persistence, and debugging surfaces so plan state can be resumed safely (cron nudges), persisted for UI refresh, and guarded against subagent races.
Changes:
- Introduces plan-mode cron wake-ups (
planCycleId) + heartbeat plan nudge builder tests. - Wires plan snapshot persistence + subagent gate persistence into gateway runtime subscriptions and session rows.
- Adds new plan-mode tools (
enter_plan_mode,plan_mode_status,ask_user_question), plan template seeding from skills, and expanded plan-mode gating/logging.
Reviewed changes
Copilot reviewed 107 out of 110 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/infra/heartbeat-runner.plan-nudge.test.ts | Adds coverage for active-plan nudge prefix and cron suppression guards. |
| src/gateway/session-utils.types.ts | Exposes plan/exec mode state fields on GatewaySessionRow for UI chips. |
| src/gateway/session-utils.ts | Adjusts session store key canonicalization and surfaces plan/exec state in session rows. |
| src/gateway/server-runtime-subscriptions.ts | Wires plan snapshot persister + subagent gate persistence into gateway event subscriptions. |
| src/gateway/server-runtime-handles.ts | Tracks planSnapshotUnsub in mutable server runtime state. |
| src/gateway/server-methods/sessions.ts | Includes plan/exec fields in sessions.changed; removes deleted-agent guard in send path. |
| src/gateway/server-close.ts | Ensures plan snapshot subscriptions are unsubscribed during shutdown. |
| src/gateway/server-close.test.ts | Updates shutdown tests to include new unsubscribe handle and new handler signature usage. |
| src/gateway/protocol/schema/sessions.ts | Adds planMode, planApproval discriminated union, and lastPlanSteps to sessions.patch schema. |
| src/gateway/protocol/schema/error-codes.ts | Adds plan-approval subagent gate error codes. |
| src/gateway/protocol/schema/cron.ts | Adds optional planCycleId to cron agentTurn payload schema. |
| src/gateway/protocol/index.ts | Re-exports ErrorCode union type; removes ChannelsStartParams plumbing. |
| src/gateway/plan-snapshot-persister.test.ts | Pins approvalRunId guard behavior for plan snapshot persistence helpers. |
| src/cron/types.ts | Adds planCycleId to cron agentTurn payload types. |
| src/cron/normalize.ts | Consolidates sessionTarget: "current" resolution logic and clarifies semantics. |
| src/cron/isolated-agent/run.ts | Adds plan-mode autoEnableFor wiring and planCycleId-bound skip guards; threads runSessionId. |
| src/cron/isolated-agent/run.plan-mode.test.ts | Adds tests for planCycleId nudge skipping + autoEnableFor runtime behavior. |
| src/config/zod-schema.ts | Adds skills.limits.maxPlanTemplateSteps config validation. |
| src/config/zod-schema.agent-runtime.ts | Adds per-agent embeddedPi autoContinue + maxIterations overrides. |
| src/config/zod-schema.agent-defaults.ts | Adds embeddedPi autoContinue, maxIterations, and planMode config fields. |
| src/config/types.skills.ts | Documents maxPlanTemplateSteps limits in skills config types. |
| src/config/types.agents.ts | Adds per-agent embeddedPi autoContinue and maxIterations type support. |
| src/config/types.agent-defaults.ts | Documents defaults for embeddedPi overrides and planMode config. |
| src/config/schema.base.generated.ts | Updates generated config schema for compaction notifyUser + new embeddedPi/planMode fields + skill limits. |
| src/commands/status.summary.ts | Switches to resolveFreshSessionTotalTokens usage in status summary. |
| src/commands/sessions.ts | Preserves stale totals in JSON output; uses fresh totals for display calculation. |
| src/auto-reply/reply/fresh-session-entry.ts | Adds skip-cache disk reads for fresh planMode/acceptEdits + fresh session-entry helper. |
| src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts | Updates assertion to expect skip-cache loads due to plan-mode freshness contract. |
| src/agents/tools/update-plan-tool.test.ts | Strengthens assertions for non-empty content and adds close-on-complete + closure gate tests. |
| src/agents/tools/sessions-spawn-tool.ts | Adds plan-mode subagent concurrency cap, parent run tracking, and cleanup keep override in plan mode. |
| src/agents/tools/sessions-spawn-tool.test.ts | Removes “role” forwarding tests; adds plan-mode concurrency cap regression suite. |
| src/agents/tools/plan-mode-status-tool.ts | Adds read-only plan-mode introspection tool with disk-read diagnostics. |
| src/agents/tools/enter-plan-mode-tool.ts | Adds enter-plan-mode tool scaffold. |
| src/agents/tools/cron-tool.ts | Documents “resume after wait” recipe using sessionTarget:"current". |
| src/agents/tools/ask-user-question-tool.ts | Adds ask-user-question tool for approval-card question flow with validation/dedup. |
| src/agents/tools/ask-user-question-tool.test.ts | Adds tests for ask_user_question validation, trimming, determinism, and content. |
| src/agents/tool-display-config.ts | Adds display config for plan-mode tools and fixes ask_user_question detailKeys to match flat shape. |
| src/agents/tool-description-presets.ts | Adds plan-mode tool descriptions, archetype guidance, and strengthened lifecycle text. |
| src/agents/tool-catalog.ts | Registers plan-mode tools in core tool definitions for filtering/catalog exposure. |
| src/agents/test-helpers/fast-openclaw-tools-sessions.ts | Updates update_plan test stub signature and exports step statuses constant. |
| src/agents/subagent-registry.test.ts | Mocks new parent-drain function and adapts outcome shape expectations. |
| src/agents/subagent-registry.steer-restart.test.ts | Adjusts agent-events mocking and adds test for parent openSubagentRunIds remap on steer restart. |
| src/agents/subagent-registry-run-manager.ts | Drains/remaps parent openSubagentRunIds on child completion/kill/steer restarts and simplifies outcome update logic. |
| src/agents/subagent-announce.ts | Adds plan-mode-aware suffix to announce instruction by reading requester planMode. |
| src/agents/skills/workspace.ts | Carries resolved plan templates into skill snapshot for seeded plans in snapshot runs. |
| src/agents/skills/types.ts | Adds skill plan template types and snapshot resolvedPlanTemplates. |
| src/agents/skills/skill-planner.ts | Implements template payload builder with dedup/truncation diagnostics and max step default. |
| src/agents/skills/frontmatter.ts | Parses plan-template from frontmatter (kebab/camel) with content alias and fallback logic. |
| src/agents/skills/frontmatter.test.ts | Adds tests for plan template parsing, aliasing, and fallback behavior. |
| src/agents/plan-mode/types.ts | Adds plan-mode core types, secure approvalId generation, and decision injection builder. |
| src/agents/plan-mode/reference-card.ts | Adds bootstrap-injected plan-mode reference card content. |
| src/agents/plan-mode/plan-nudge-crons.ts | Adds plan-mode nudge cron scheduling/cleanup helpers with validation. |
| src/agents/plan-mode/plan-mode-debug-log.ts | Adds unified plan-mode debug event logger with env/config gating + TTL cache. |
| src/agents/plan-mode/plan-archetype-prompt.ts | Adds decision-complete prompt fragment and plan filename helpers. |
| src/agents/plan-mode/plan-archetype-prompt.test.ts | Adds tests for prompt fragment and filename slug/filename helpers. |
| src/agents/plan-mode/mutation-gate.ts | Adds plan-mode mutation gate logic for tool blocking and read-only exec allowlist. |
| src/agents/plan-mode/mutation-gate.test.ts | Adds mutation gate test coverage for allow/block/suffix and exec whitelist behavior. |
| src/agents/plan-mode/index.ts | Exports plan-mode types and helpers. |
| src/agents/plan-mode/auto-enable.ts | Adds regex-based model matcher for plan-mode autoEnableFor with compiled cache. |
| src/agents/plan-mode/auto-enable.test.ts | Adds tests for auto-enable matcher, malformed patterns, and cache stability. |
| src/agents/plan-mode/approval.ts | Adds approval state resolver with stale-event guard and approval injection builder. |
| src/agents/plan-hydration.ts | Adds post-compaction active-plan hydration formatter. |
| src/agents/plan-hydration.test.ts | Adds tests for hydration formatting and filtering. |
| src/agents/pi-tools.ts | Threads planMode + live accessors into before-tool-call hook for gating + acceptEdits constraints. |
| src/agents/pi-tools.before-tool-call.ts | Enforces mutation gate and acceptEdits constraint gate pre plugin hooks using live mode accessor. |
| src/agents/pi-embedded-runner/skills-runtime.test.ts | Updates snapshot-loading behavior tests for resolvedPlanTemplates backward compat. |
| src/agents/pi-embedded-runner/run/params.ts | Threads planMode + live accessors through embedded runner params interface. |
| src/agents/pi-embedded-runner/run/helpers.ts | Raises retry iteration defaults and adds userMaxIterations override + subagent cap constant. |
| src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test-support.ts | Stubs plan template seeder for workspace spawn test support. |
| src/agents/pi-embedded-runner/run.overflow-compaction.test.ts | Uses config override to keep retry-limit tests fast under higher default iteration caps. |
| src/agents/pi-embedded-runner/run.incomplete-turn.test.ts | Updates strict-agentic retry expectations; adds auto-continue & escalation tests; disables retry/ACK in plan mode. |
| src/agents/pi-embedded-runner/pending-injection.ts | Adds backward-compat shim over typed pending injection queue. |
| src/agents/pi-embedded-runner/pending-injection.test.ts | Adds tests for pending injection consumption & composition behavior. |
| src/agents/openclaw-tools.ts | Registers plan-mode tools and threads runId into update_plan and sessions_spawn. |
| src/agents/openclaw-tools.registration.ts | Adds plan-mode tool gating predicate based on config enable flag. |
| apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift | Updates Swift protocol models for new error codes + plan patch fields; removes ChannelsStartParams. |
| apps/macos/Sources/OpenClawProtocol/GatewayModels.swift | Mirrors Swift protocol model updates for macOS target. |
| const MUTATION_TOOL_BLOCKLIST = new Set([ | ||
| "apply_patch", | ||
| "bash", | ||
| "edit", | ||
| "exec", | ||
| "gateway", | ||
| "message", | ||
| "nodes", | ||
| "process", | ||
| "sessions_send", | ||
| "sessions_spawn", | ||
| "subagents", | ||
| "write", | ||
| ]); |
There was a problem hiding this comment.
The current allow/block lists conflict with the Plan Mode behavior documented elsewhere in this PR: (1) sessions_spawn is blocklisted even though plan mode explicitly relies on spawning research subagents and tracking them for the approval gate; (2) newly added read-only plan-mode tools (enter_plan_mode, plan_mode_status, ask_user_question) are not allowlisted, so they’ll be blocked by the default-deny path when already in plan mode; (3) the cron resume recipe added in cron-tool.ts will also be blocked unless cron is explicitly allowlisted. Recommended fix: treat sessions_spawn and the plan-mode diagnostic/interaction tools as allowed while in plan mode (and add/update tests accordingly).
| const PLAN_MODE_ALLOWED_TOOLS = new Set([ | ||
| "read", | ||
| "web_search", | ||
| "web_fetch", | ||
| "memory_search", | ||
| "memory_get", | ||
| "update_plan", | ||
| "exit_plan_mode", | ||
| "session_status", | ||
| ]); |
There was a problem hiding this comment.
The current allow/block lists conflict with the Plan Mode behavior documented elsewhere in this PR: (1) sessions_spawn is blocklisted even though plan mode explicitly relies on spawning research subagents and tracking them for the approval gate; (2) newly added read-only plan-mode tools (enter_plan_mode, plan_mode_status, ask_user_question) are not allowlisted, so they’ll be blocked by the default-deny path when already in plan mode; (3) the cron resume recipe added in cron-tool.ts will also be blocked unless cron is explicitly allowlisted. Recommended fix: treat sessions_spawn and the plan-mode diagnostic/interaction tools as allowed while in plan mode (and add/update tests accordingly).
| if (!key) { | ||
| return; | ||
| } | ||
| const { cfg, entry, canonicalKey, storePath } = loadSessionEntry(key); | ||
| // Reject sends/steers targeting sessions whose owning agent was deleted (#65524). | ||
| const deletedAgentId = resolveDeletedAgentIdFromSessionKey(cfg, canonicalKey); | ||
| if (deletedAgentId !== null) { | ||
| params.respond( | ||
| false, | ||
| undefined, | ||
| errorShape( | ||
| ErrorCodes.INVALID_REQUEST, | ||
| `Agent "${deletedAgentId}" no longer exists in configuration`, | ||
| ), | ||
| ); | ||
| return; | ||
| } | ||
| const { entry, canonicalKey, storePath } = loadSessionEntry(key); |
There was a problem hiding this comment.
This removes the explicit guard that prevented sending/steering to sessions whose owning agent was deleted from config (#65524). With the current diff, handleSessionSend will proceed for deleted-agent session keys so long as an entry exists, which reintroduces the class of invalid-target behavior the removed check was covering. Suggested fix: reintroduce a deleted-agent ownership check (either locally here, or by having loadSessionEntry/key resolution return an error/flag when the agent is missing) and return INVALID_REQUEST as before.
| const EnterPlanModeToolSchema = Type.Object({ | ||
| reason: Type.Optional( | ||
| Type.String({ | ||
| description: | ||
| "Optional short justification shown alongside the mode-entered event " + | ||
| "(e.g. 'multi-file refactor — surface the plan first').", | ||
| }), | ||
| ), | ||
| }); |
There was a problem hiding this comment.
Two issues worth addressing for consistency with the other new plan-mode tools: (1) the schema doesn’t set { additionalProperties: false }, so arbitrary payload fields would be accepted even though they are ignored; (2) the tool returns empty content, while other tools in this PR explicitly return non-empty text to satisfy the “lossless-claw paired-tool-result” expectation. Consider hardening the schema and returning a short text content message (e.g., “Entered plan mode.”) so downstream renderers/logging don’t drop the result.
| return { | ||
| content: [], | ||
| details: { | ||
| status: "entered" as const, | ||
| mode: "plan" as const, | ||
| ...(reason && reason.length > 0 ? { reason } : {}), | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Two issues worth addressing for consistency with the other new plan-mode tools: (1) the schema doesn’t set { additionalProperties: false }, so arbitrary payload fields would be accepted even though they are ignored; (2) the tool returns empty content, while other tools in this PR explicitly return non-empty text to satisfy the “lossless-claw paired-tool-result” expectation. Consider hardening the schema and returning a short text content message (e.g., “Entered plan mode.”) so downstream renderers/logging don’t drop the result.
| 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 { |
There was a problem hiding this comment.
The dynamic import + sessionKey validation are performed inside the per-interval loop, but both are invariant for the call. This causes extra module-loading overhead and repeated validation work when multiple nudges are scheduled. Suggested fix: move the await import("../../cron/session-target.js") and the single assertSafeCronSessionTargetId(params.sessionKey) check before the loop, so the loop only schedules jobs.
| 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; | ||
| } |
There was a problem hiding this comment.
The dynamic import + sessionKey validation are performed inside the per-interval loop, but both are invariant for the call. This causes extra module-loading overhead and repeated validation work when multiple nudges are scheduled. Suggested fix: move the await import("../../cron/session-target.js") and the single assertSafeCronSessionTargetId(params.sessionKey) check before the loop, so the loop only schedules jobs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6b4d3e69b
ℹ️ 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".
| run({ | ||
| reason: params.reason, | ||
| agentId: params.agentId, | ||
| sessionKey: params.sessionKey, | ||
| heartbeat: params.heartbeat, | ||
| }); |
There was a problem hiding this comment.
This wake dispatch now drops params.heartbeat, so queued wake requests lose their target override before reaching runOnce. That regresses callers that intentionally pass heartbeat: { target: "last" } (for example cron requeue paths in src/cron/service/timer.ts) to keep retries bound to the prior delivery target after a requests-in-flight skip. With the override discarded, retries can fall back to agent defaults and deliver to the wrong destination or not where the triggering flow expects.
Useful? React with 👍 / 👎.
| const feedback = | ||
| action === "reject" | ||
| ? normalizeOptionalString(patch.planApproval.feedback) || undefined | ||
| : undefined; |
There was a problem hiding this comment.
The reject path trims feedback with normalizeOptionalString, which turns whitespace-only input into undefined, but no error is raised afterward. Because the schema only enforces raw minLength: 1, a payload like " " passes validation and lands here, resulting in a rejection that can persist no actionable feedback (resolvePlanApproval falls back to prior/empty feedback). That violates the new “reject requires feedback” contract and can leave the next revision turn without guidance.
Useful? React with 👍 / 👎.
|
Closed for sequential rollout — please disregard cumulative diff. This PR was opened with The fix: sequential rollout. Each Currently open:
Next: when 1/9 merges, This PR's branch is preserved on the fork and will be reopened with a clean diff in its turn. |
Summary
[Plan Mode 4/8] — part of the 8-part decomposition of #68939, stacking on #69449 (Part 1).
Stacks on: previous part (see [Plan Mode 3/8] / [Plan Mode 1/9 #69449]).
Cron-driven plan automation (autoEnableFor pattern matching) + subagent lifecycle improvements + plan-snapshot persister. Stacks on Part 3/8. Includes rebase-fixup commit
d6b4d3e69bupdating exit_plan_mode integration tests for new mandatorytitlearg.Lines: ~7367
Diff note
This PR is opened cross-repo (head:
100yenadmin:restack/68939-pr5-automation-subagents) againstopenclaw: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