feat(plan-mode): 1.0 release follow-up β hardening + Telegram attachment + config + security + DX (stacks on #68939)#69324
Closed
100yenadmin wants to merge 166 commits into
Closed
Conversation
added 30 commits
April 19, 2026 18:28
β¦injection scanning Consolidation of v3 PRs #66371, #66372, #66373, #66374, #66375 into a single review unit. Ports the Hermes Agent GPT-5 prompt stack to OpenClaw with Hermes-verbatim semantics (only mechanical tool-ID substitutions for OpenClaw's catalog: terminalβexec, execute_codeβ code_execution, read_fileβread, search_filesβexec). ## Prompt additions (extensions/openai/prompt-overlay.ts) OPENAI_GPT5_TOOL_ENFORCEMENT β Hermes mandatory_tool_use block: "NEVER answer these from memory β ALWAYS use a tool" + 7 categories. OPENAI_GPT5_EXECUTION_BIAS enhanced with 3 Hermes subsections: - Act, Don't Ask (prompt_builder.py:221-229) - Tool Persistence (prompt_builder.py:198-204) - Verification (prompt_builder.py:238-245) ## Architecture (src/agents/system-prompt-contribution.ts) Add "tool_enforcement" to ProviderSystemPromptSectionId union so providers can inject mandatory tool-use guidance independently of execution_bias. GPT-5 overlay uses sectionOverrides.tool_enforcement. ## Security (src/agents/context-file-injection-scan.ts) Port Hermes's _CONTEXT_THREAT_PATTERNS (prompt_builder.py:36-52): 10 threat patterns + 10 invisible chars. Flagged files are BLOCKED (content replaced with placeholder), matching Hermes behavior. ## Tests - index.test.ts: EXPECTED_GPT5_STABLE_PREFIX helper + 7 assertions updated for tool_enforcement sectionOverride - context-file-injection-scan.test.ts: 17 tests covering all patterns, false-positive avoidance, and BLOCK behavior Part of GPT 5.4 Enhancement v3 sprint. Tracking: #66345. Supersedes: #66371, #66372, #66373, #66374, #66375.
Filename is interpolated into the BLOCKED string. A crafted file path containing brackets or newlines could confuse the model about what content is blocked. Strip ], [, and newlines from the filename.
β¦bosity Additions to the GPT-5.4 prompt overlay (interaction_style, output_contract, execution_bias sections): - Identity Enforcement: prime GPT-5.4 to adopt SOUL.md as primary identity, ban corporate default patterns - Voice Calibration: counter flat/analytical drift, anti-sycophancy reinforcement - Response Length Discipline: 200-word target with 95% confidence self-evaluation gate leveraging thinking mode, file-offload for genuine long-form - Investigation Discipline: prevent partial-findings-then-ask pattern, force parallel tool calls for independent lookups - Plan Confidence Gate: 95%+ β execute without approval, 80-94% β state uncertainty and begin, <80% β iterate privately through research
β¦tterns The prompt_injection pattern only matched single-qualifier forms like 'ignore all instructions' but missed 'ignore all previous instructions' (two qualifiers). Changed to allow zero or more qualifier words between 'ignore' and 'instructions'.
Copilot review: safeFilename only stripped brackets/newlines, leaving bidi overrides, zero-width chars, and other Cc/Cf/Zl/Zp chars that could manipulate the BLOCKED placeholder text. Now uses sanitizeForPromptLiteral() for full Unicode category coverage, then strips brackets separately.
1. Apply sanitizeForPromptLiteral() to file.path in the ## heading of buildProjectContextSection(). This closes the 3x-raised Copilot concern about crafted filenames with control/bidi chars breaking prompt structure. 2. Add || 'unknown' fallback to safeFilename in injection scan placeholder, preventing empty string if filename is entirely control/format chars.
β¦, allowlist, filename strip Prompt overlay (extensions/openai/prompt-overlay.ts): - Plan Confidence Gate carve-out: explicit exception that plan mode goes through approval flow regardless of confidence level Injection scanner (src/agents/context-file-injection-scan.ts): - Multi-line regex flag (im) on translate_execute, exfil_curl, read_secrets patterns. Attacks split across newlines no longer bypass detection - Default allowlist for security docs: SECURITY.md, CONTRIBUTING.md, docs/security/*, qa/scenarios/* β content with injection patterns passes through with optional onAllowlistBypass callback for telemetry - Filename defense-in-depth: strip <, >, & in addition to brackets so a filename like '<!--ignore-->.md' can't embed instruction-like text in the BLOCKED placeholder - New SanitizeContextFileOptions interface with allowlist + bypass callback Tests: 13 new tests (multi-line attack patterns, allowlist for SECURITY/ CONTRIBUTING/docs/security/qa/scenarios, custom allowlist override, regular files still blocked, filename angle/bracket/ampersand stripping, callback fires)
β¦ion + custom-list bug
Two security/correctness fixes from adversarial review of the
context-file injection scanner:
1. **Allowlist Windows-path bypass + traversal bypass (HIGH):**
The default allowlist patterns
/(?:^|\\/)docs\\/security\\//i
/(?:^|\\/)qa\\/scenarios\\//i
only matched forward-slash paths and accepted any path containing
the segment. This let two bypasses slip through:
- Windows: 'docs\\\\security\\\\foo.md' wouldn't match the regex
because backslashes aren't forward-slashes.
- Traversal: 'qa/scenarios/../../etc/passwd' DID match because the
'qa/scenarios/' segment appeared anywhere in the path.
New helper normalizePathForAllowlist():
- Replaces '\\\\' with '/' before matching (cross-platform).
- Rejects any path with a literal '..' SEGMENT (returns null β
never allowlisted, fail-closed). Filenames containing '..' as a
substring (e.g. 'foo..bar.md') are still allowed β only '..'
segments are hostile.
2. **Custom allowlist parameter silently ignored (MEDIUM):**
sanitizeContextFileForInjection accepted options.allowlist but
then called isAllowlistedPath(filename) WITHOUT forwarding the
custom list β so callers' custom allowlists were no-ops. Fixed to
pass the resolved allowlist through.
Tests added (4 new + 1 strengthened, 37 total pass):
- custom allowlist override actually applies (was a void-result no-op
test before)
- empty custom allowlist forces SECURITY.md to be scanned
- Windows backslash path matches default allowlist (regression
guard for the bypass)
- 'qa/scenarios/../../etc/passwd' fails closed
- 'foo..bar.md' (substring, not segment) is still allowed
β¦ist matching Codex P2 (PR #67512 r3096412188): now that custom `allowlist` regexes are honored (per the iter-1 `2b610f6f` fix), callers passing a regex with `g` or `y` flags would see nondeterministic results: `re.test(normalized)` mutates `lastIndex` on stateful flags, so back-to-back scans against the same regex object alternate between allowlisted and blocked. Fix: reset `re.lastIndex = 0` before each `.test()` call. Defensive on non-stateful regexes (no-op there) but eliminates the alternating- outcome failure mode. Test added: 5-iteration loop against `/docs\/security\//g` confirms the same path consistently resolves as allowlisted. 38 tests pass.
β¦e, activeForm, hydration
Ports three Hermes todo-tool features that OpenClaw's update_plan
was missing, plus adds plan hydration for post-compaction recovery.
## cancelled status (Hermes parity)
Add "cancelled" as a fourth plan step status. Hermes uses this to
mark steps that were started but abandoned due to failure, keeping
them visible in history instead of silently dropping them.
Ref: Hermes tools/todo_tool.py Status enum.
## merge mode (Hermes parity)
Add optional merge: boolean parameter (default false). When true,
incoming steps update existing ones by matching step text and new
steps are appended. When false, the entire plan is replaced.
Ref: Hermes tools/todo_tool.py write(todos, merge=False).
## activeForm field (Claude Code TodoWrite parity)
Add optional activeForm: string field on plan steps. Shows the
present-continuous form while in_progress (e.g. "Running tests"
instead of "Run tests"). Small schema addition with disproportionate
UX impact for plan rendering.
## Post-compaction plan hydration (Hermes parity)
New src/agents/plan-hydration.ts helper that formats active
(pending/in_progress) plan items for injection after context
compression. The injected text uses factual phrasing ("Your active
plan was preserved...") to avoid triggering the planning-only retry
guard's promise-language detection.
Ref: Hermes run_agent.py:6754-6756 + tools/todo_tool.py:format_for_injection().
Note: the compaction hook point in compact.ts is not wired in this
PR β that requires a follow-up to integrate with the compaction
checkpoint system (#62146). This PR provides the formatter; the
hook wiring is the next step.
Part of GPT 5.4 Enhancement v3 sprint. Tracking: #66345.
β¦rge mode The execute() third param is an AbortSignal/context, not a plan context, so context?.previousPlan was always undefined and merge mode was a no-op. Fall back to replace until PlanStore (#67542) provides previous-plan lookup.
- Add plan-hydration.test.ts: tests formatPlanForHydration returns null for empty/all-completed/all-cancelled steps, filters out completed and cancelled steps, includes pending and in_progress with correct markers, and validates preserved plan header format - Add update-plan-tool.parity.test.ts: tests cancelled status is accepted in schema, activeForm field is preserved in output, and merge=true with no previousPlan falls back to replace
Now that activeForm is explicitly declared, disallow arbitrary extra keys on plan step objects to catch malformed input early.
β¦escription - Rename _context β _signal in execute() to match actual parameter meaning - Fix activeForm schema description: accepted on any status, rendered for in_progress - Fix plan-hydration JSDoc: 'task list' β 'plan' to match OpenClaw terminology
β¦text.lastPlanSteps
Replaces the no-op if(merge){replace}else{replace} placeholder in
update-plan-tool.ts with a real merge implementation. Storage is keyed
on runId via AgentRunContext.lastPlanSteps so each run sees only its
own plan history.
- src/infra/agent-events.ts: add lastPlanSteps?: PlanStepSnapshot[]
to AgentRunContext (in-memory; PlanStore disk persistence remains
separate scope of #67542).
- src/agents/tools/update-plan-tool.ts:
- export PLAN_STEP_STATUSES + PlanStepStatus union
- accept { runId } factory option
- port mergeSteps from phase4/cross-session-plans:plan-store.ts:204
(in-memory variant β no updatedBy/updatedAt attribution)
- emit agent_plan_event after every update so channel renderers see
the merged result
- src/agents/openclaw-tools.ts: thread runId through to
createUpdatePlanTool factory
- src/agents/pi-tools.ts: pass options.runId to createOpenClawTools
- src/agents/plan-hydration.ts: use the exported PlanStepStatus union
via 'satisfies' for compile-time coupling
- src/agents/test-helpers/fast-openclaw-tools-sessions.ts: update mock
signature + re-export PLAN_STEP_STATUSES
Tests: 9 new merge-mode tests in update-plan-tool.parity.test.ts
covering overlap, append, completed-preservation, cancelled rollback,
runId isolation, persistence, and emission. All 23 update-plan-related
tests pass.
β¦erge + reject duplicate step text Two adversarial fixes from Codex review: P1 r3096162551: Revalidate active-step invariant after merge. readPlanSteps enforces 'at most one in_progress' only on the incoming patch, but merge mode can still create an invalid final plan: if the previous plan has step A as in_progress and the patch marks step B as in_progress (without changing A), the merged result has two active steps β violates the tool's own contract and breaks downstream plan renderers that assume a single active step. Fix: after merge, count in_progress in the merged plan; throw if >1 with a clear remediation hint (mark the prior in_progress step completed/cancelled in the same patch). P2 r3096162555: Reject duplicate step text within a single patch. Merge keys steps by 'step' text, so two patch entries with the same step text would silently rewrite history (second entry clobbers the first, then both collide on the same merge key against the previous plan, rewriting unrelated entries). Better to fail at input time with a message explaining the join-key constraint. Tests: 2 new regression tests in update-plan-tool.parity.test.ts: - merge that would yield two in_progress throws - patch with duplicate step text throws Total: 15 parity tests pass.
Phase 4.2 of the GPT 5.4 parity sprint. Adds a PlanStore class for cross-session task coordination, modeled after Claude Code's Tasks API concept. ## PlanStore (plan-store.ts) - read(namespace): loads plan from ~/.openclaw/plans/<ns>/plan.json - write(namespace, plan): persists plan to disk with auto-mkdir - lock(namespace): file-level exclusive lock with O_EXCL + 10s stale-lock auto-cleanup to prevent deadlocks from crashes - mergeSteps(): merge incoming steps into existing plan by matching step text, appending new steps, preserving existing order. Tracks updatedBy (session key) and updatedAt per step. ## Schema (StoredPlan, StoredPlanStep) - StoredPlanStep: step, status (4 values), activeForm?, updatedBy?, updatedAt? - StoredPlan: namespace, steps[], createdAt, updatedAt ## Tests (8 tests) - Read/write round-trip + nested directory creation - Lock acquisition, release, and concurrent contention - Merge: update by text match, append new, preserve order Default behavior (no namespace configured): plan is session-scoped, PlanStore is not used. Cross-session coordination only activates when planStore.namespace is set in config. Tracking: #66345, #67523.
β¦ errors, lock ownership - Validate namespace to prevent path traversal (reject "..", absolute paths) - Atomic write: write to temp file then rename to prevent mid-write corruption - Fix file handle leak: ensure handle.close() in finally on open errors - Distinguish ENOENT from parse/permission errors in read() instead of swallowing all errors - Verify lock ownership token before unlinking on release to prevent one process from deleting another's lock
Replace O(n) linear scan with a Set when checking whether incoming steps already exist, improving merge performance for large plans.
Three fixes from Copilot review: 1. mergeSteps: only set updatedBy when sessionKey is truthy, avoiding undefined overwriting existing attribution 2. write(): validate plan.namespace matches the namespace argument to prevent accidental mismatches 3. Added tests: path traversal rejection + namespace mismatch in write
β¦g in mergeSteps 1. read(): verify plan.namespace matches requested namespace to catch file corruption or misrouted reads 2. mergeSteps(): deduplicate incoming steps by tracking appended step texts in a Set, preventing duplicate step entries when incoming contains the same step text multiple times
Change from truthy check to explicit undefined check so that plans with no namespace field (backward-compat) pass through, while non-undefined mismatches still throw a corruption error.
Path traversal namespace like '../escape' was caught by the mismatch check (comparing against plan.namespace) before reaching validateNamespace(), producing a confusing error message instead of the security-relevant one. Swap the order so validateNamespace runs first.
Review finding: temp files created without explicit mode inherit umask defaults, which may be more permissive than intended for plan data. Added mode: 0o600 to writeFile call to match repo convention for sensitive data files.
- Add runtime shape validation in read(): verify plan has string namespace and steps array before returning, instead of trusting JSON.parse cast - Normalize dot segments in validateNamespace(): path.normalize() prevents cross-namespace collisions from ./foo vs foo equivalence
- Replace handle.write(lockToken) with handle.writeFile(lockToken) to handle partial writes correctly (write() can return short) - Fix docstring: 'auto-expires after 10s' β 'stale locks cleaned up opportunistically by next lock() caller' (no timer-based expiry)
added 10 commits
April 20, 2026 01:51
β¦+ agent-events mock)
Two more CI failures from the round-2 push (different jobs, same
root cause: nuclear-fix-stack added new exports/test paths that
existing test allowlists / mocks didn't know about):
1. checks-fast-contracts-protocol:
src/plugins/contracts/plugin-sdk-runtime-api-guardrails.test.ts
"keeps runtime api surfaces on an explicit export allowlist"
was failing on `extensions/telegram/runtime-api.ts` β actual
exports had `sendDocumentTelegram` + `TelegramDocumentOpts`
from the PR-14 plan-archetype-bridge work, but the allowlist
in this guardrail test still listed only the original 12 send
helpers. Added both new entries (function + type) to the
allowlist with a comment marker explaining the round.
2. checks-node-agentic-agents-plugins (3 failures):
src/agents/subagent-registry.test.ts seam-flow tests
("loads runtime plugins before emitting killed subagent
ended hooks", "deletes killed delete-mode runs and notifies
deleted cleanup", "removes attachments for killed delete-mode
runs") were failing because the `vi.mock("../infra/agent-
events.js", ...)` mock only exported `onAgentEvent`. The
nuclear-fix-stack subagent concurrency cap commit
(`984b4eebff`) added `drainCompletedSubagentFromParents` β
`subagent-registry-run-manager.ts:431` calls it whenever a
run is marked terminated. Test mock now includes a no-op
`vi.fn()` for the new export; the test focuses on the
registry-side seam and doesn't exercise the parent-set drain.
Both fixes are scoped to the test allowlists/mocks; no production
code changes. tsgo + lint clean; both touched test suites pass
locally.
β¦-card + docs + intro Four Copilot comments on PR #68939 flagged that `/plan self-test` is marked deferred in the slash-command tables but then recommended as a debugging step later in the same surface. Fixed across 3 files so the deferred label is consistent: - src/agents/plan-mode/reference-card.ts β reference-card.ts:56 tweaks the ask_user_question line from 'non-block' to 'clarify; next turn' (Copilot flagged 'non-block' as misleading β the run pauses for the answer arriving as [QUESTION_ANSWER] next turn). Debug-tip line 137 now explicitly points back to the deferred note at line 122 instead of instructing the operator to run the unavailable command. - docs/concepts/plan-mode.md β troubleshooting section line 127 now matches the slash-command table at line 66, pointing operators at the debug log + gate log instead of `/plan self-test`. - src/gateway/sessions-patch.ts β PLAN_MODE_INTRO injection no longer instructs the agent to run `/plan self-test` in its next turn. Comment at line 513 updated to match. Addresses review comments #3107270578, #3107270584 (partial β ack scope correction), #3107313840, #3107365545, #3107365560, #3107365568.
Addresses 4 inline review comments on PR #68939: 1. src/agents/plan-mode/accept-edits-gate.ts: extractApplyPatchTargetPaths now also parses `*** Move File: <src> -> <dst>` targets. Both source and destination paths flow into the protected-path check so a Move touching `~/.openclaw/config.toml` is gated under acceptEdits. Codex P2 #3107291431. 2. src/auto-reply/reply/commands-plan.ts: /plan accept parser now rejects unknown arguments. Pre-fix, `/plan accept editss` (typo) silently landed as a bare approval (allowEdits=false) because any non-'edits' suffix was ignored. Now only `accept`, `accept edit`, `accept edits` are valid; anything else returns a usage error. Codex P1 #3107344436. 3. src/agents/pi-embedded-runner/system-prompt.ts: stripProviderPrefix now uses lastIndexOf('/') so multi-segment IDs like `openai/codex/gpt-5.4` collapse to the trailing model name (`gpt-5.4`) instead of the middle slice (`codex/gpt-5.4`) that wouldn't match the gpt-5 family check. Copilot #3107360348. 4. src/auto-reply/reply/commands-system-prompt.ts: defensive `<provider>/` strip added at the modelId pass-through (mirrors the same strip the callee applies to runtimeInfo.model). Prevents the GPT-5 boot-reorder from missing when params.model arrives pre-qualified. Copilot #3107365533.
β¦+ R3 XSS + R4 disk-full + R5 dedup tests Part 1 of Plan Mode 1.0 follow-up. Lands the 4 not-yet-shipped items from the rollout hardening pack; R1 (subagent drain) and R2 (cron approval-pending guard) were already shipped by lifecycle-hardening commit 906eb68. Bug B β stale approval card auto-dismiss - New error code PLAN_APPROVAL_EXPIRED returned from sessions.patch when the target session no longer has planMode state (e.g. the approval window timed out, /plan off fired, another channel already resolved it, or session compaction lost the state). - Webchat auto-dismisses the card on this code plus several message- substring fallbacks (covers transports that flatten the error to a string). Parallels the existing PLAN_APPROVAL_BLOCKED_BY_SUBAGENTS handling pattern. - Swift bindings regenerated (pnpm protocol:gen:swift). R3 β plan title XSS adversarial regression - 41 new tests across renderPlanWithHeader and renderFullPlanArchetypeMarkdown cover <script>, event handlers, javascript:/data: URLs, mixed-case tags, and path-traversal shaped titles. Pins the existing escapeHtml/escapeMarkdown/neutralizeMentions chain so future refactors don't regress. R4 β disk-full / permission / I/O graceful error - persistPlanArchetypeMarkdown now wraps writeFile in a catch that classifies ENOSPC/EACCES/EIO as a PlanPersistStorageError (typed class) and rethrows. Non-storage errors propagate unchanged. - The bridge catches this distinctive error and emits a [plan-bridge/storage] log line with operator-actionable guidance instead of burying it under the generic plan-bridge failure log. - Plan approval still proceeds (audit artifact loss is non-fatal). - 6 new tests via new _writeFileForTest DI hook (vitest cannot spy on ESM fs/promises namespace). R5 β multi-channel approval dedup - 3 new tests prove that sessions.patch's serial state transition naturally dedups concurrent approve writes from different channels. First write wins; second gets PLAN_APPROVAL_EXPIRED (or the pending-approval error when autoApprove keeps the session in plan mode). Protects against double-injection of [PLAN_DECISION] markers. Docs: updated docs/concepts/plan-mode.md troubleshooting to reference the new error code. Verification: - pnpm tsgo:core, tsgo:core:test β clean for touched surface - pnpm lint β clean for touched surface (5 pre-existing errors in untracked qa-lab and bundled-web-search-fast-path-contract helpers are out of scope) - pnpm test on all touched test files: 41 XSS + 15 persist (9 existing + 6 R4) + sessions-patch (46 existing + 1 Bug B + 3 R5) = all green
β¦very via sendDocumentTelegram SDK facade
Part 2 of Plan Mode 1.0 follow-up. Wires the plan-archetype bridge to
the now-public-facing `sendDocumentTelegram` surface so Telegram
operators receive the rendered markdown as a document attachment on
every exit_plan_mode cycle (not just the caption stub).
Background: commit `906eb68403` (and earlier rebase waves) stubbed
out the Telegram attachment upload with a `// DEFERRED` block after
the upstream plugin-SDK restructure removed the old
`src/plugin-sdk/telegram.ts`. The function is now re-exported via
the public-SDK facade at `src/plugin-sdk/telegram.ts:51` with lazy
loading of the bundled Telegram runtime β the re-wire is a clean
dynamic-import swap.
Bridge changes (src/agents/plan-mode/plan-archetype-bridge.ts):
- Replace the no-op `void caption; void absPath` stub with a live
`await import("../../plugin-sdk/telegram.js")` + `sendDocumentTelegram(dctx.to, absPath, { caption, parseMode: "HTML" })`
call. Dynamic import preserves the cold-path bundle discipline
(plan-bridge import stays cheap; Telegram runtime only loads on
actual exit_plan_mode in a Telegram session).
- Thread-ID handling: `parseTelegramTarget` inside the SDK auto-
extracts `message_thread_id` from the `dctx.to` string (formats:
`chatId`, `chatId:threadId`, `chatId:topic:threadId`) β no
client-side parsing needed. The legacy commented-out
`parseTelegramThreadId` helper is removed.
- Success log now reports `chatId=β¦ msgId=β¦` from the returned
result so operators can verify delivery in gateway logs.
- Failure path unchanged: outer try/catch still swallows network
errors into `log.warn` (fire-and-forget contract preserved β
approval flow never blocks on delivery).
Test changes (src/agents/plan-mode/plan-archetype-bridge.test.ts):
- Un-skip the "Telegram send throws" failure-branch test.
- Rewrote the "Telegram session persists markdown" test to assert
the send actually fires with the right file path + HTML caption.
- Added topic-scoped target passthrough test
(`chatId:topic:threadId` β SDK parses thread ID).
- Multi-cycle test now asserts two sends for two cycles.
Verification:
- pnpm tsgo:core + tsgo:core:test: clean for touched surface
- pnpm lint: clean for touched surface (5 pre-existing errors in
untracked helpers remain out of scope)
- pnpm test src/agents/plan-mode/plan-archetype-bridge.test.ts:
10 tests pass (including the newly un-skipped failure-branch test)
- pnpm build: Discord runtime-staging failure is PRE-EXISTING (not
touched by this commit). Cause is
discord-api-types version constraint `^0.38.47` that the staging
script rejects β unrelated to plan-mode or Telegram. Tracked
outside this commit.
Out of scope for C2 (future follow-ups):
- Telegram inline-keyboard Accept/Revise callback buttons (C5)
- Live-retest against a real Telegram DM + topic group (manual QA)
β¦ into plan mode at cron turn start
Part 3a of Plan Mode 1.0 follow-up. The `agents.defaults.planMode.autoEnableFor`
config field was previously SCHEMA-RESERVED (declared but never read at
runtime). This commit ships the missing wiring.
Helper (src/agents/plan-mode/auto-enable.ts):
- `evaluateAutoEnableForMatch(modelId, patterns)` β pure synchronous
function with compiled-regex cache. Returns true when the model
matches any of the configured patterns. Malformed patterns fail
closed (no match rather than gateway crash). Empty inputs, non-
array/non-string entries all return false.
- 14 unit tests cover: empty inputs, happy-path matches, OR across
multiple patterns, malformed regex handling, empty/null entries,
and implicit cache-stability across repeated calls.
Runtime wire (src/cron/isolated-agent/run.ts):
- Inserted check before the existing plan-cycle-bound nudge guard.
When `cfg.agents.defaults.planMode.enabled === true` AND
`planMode` is completely absent on the session entry (meaning
the session has NEVER been toggled) AND the resolved model matches
any configured pattern, the runner:
1. Mutates the session entry to `{ mode: "plan", approval: "none", ... }`
2. Persists via `persistSessionEntry()`
- Safe-by-default: sessions with `planMode.mode: "normal"` are NOT
re-enabled (respects explicit `/plan off` or post-approval state).
- Uses dynamic import of the helper to keep the cron hot path cold-
load cheap.
Test coverage (src/cron/isolated-agent/run.plan-mode.test.ts):
- 4 integration tests cover: pattern-match triggers auto-enable
(fresh session), user-toggled-off is preserved, feature gate off
blocks auto-enable, non-matching patterns skip.
- Plus 14 helper tests. Total new: 18. Existing cron-nudge tests
still pass (3).
Config type docs (src/config/types.agent-defaults.ts):
- Removed the "SCHEMA-RESERVED β runtime wiring not yet implemented"
warning for `autoEnableFor`. The field is now active; documentation
reflects the user-intent-preserving one-time trigger contract.
Deferred to C3b (separate commit):
- `approvalTimeoutSeconds` runtime wiring. Needs either a new
server-only `timeout` action on the sessions.patch schema or a
gateway-internal bypass RPC + cron scheduling + cleanup β design
decision worth its own commit.
Verification:
- pnpm tsgo:core: clean for touched surface
- pnpm tsgo:core:test: clean for touched surface (pre-existing
moonshot-stream-wrappers error in untracked file remains; unrelated)
- pnpm lint: 5 pre-existing errors in untracked qa-lab/helpers remain
out of scope
- pnpm test src/cron/isolated-agent/run.plan-mode.test.ts +
src/agents/plan-mode/auto-enable.test.ts: 21 pass
β¦ + approvalRunId silent-bypass guard
Part 4 of Plan Mode 1.0 follow-up. Closes two known gaps in the
acceptEdits permission model and the approval-side subagent gate
infrastructure.
C4.1 β Shell-escape layered defense (src/agents/plan-mode/accept-edits-gate.ts)
The existing accept-edits gate prefix-matches destructive verbs
directly (`rm`, `shred`, etc.), SQL statements, and find-family
flags. This catches the 99% case where the destructive command is
directly visible. The 1% gap: shell-escape constructs where the
destructive verb resolves at RUNTIME:
- Env-var indirection: `$RM /tmp/x`, `${RM} /tmp/x`
- Backtick subshell: ```echo rm``` file
- $(...) subshell: `$(echo rm) /tmp/x`
- Quote concatenation: `"r""m" /tmp/x`, `'r''m' /tmp/x`
- Byte escapes: `\x72m /tmp/x`, `\162m /tmp/x`
Under acceptEdits (trusted-plan execution), these are blocked
because the gate cannot track what the shell will expand to.
Rationale: acceptEdits is permission elevation for trusted plan
execution, not cleverness budget. Legitimate post-approval exec
rarely needs runtime destructive-verb expansion β blocking has
near-zero false-positive cost.
Primary defense remains the prompt layer (`buildAcceptEditsPlanInjection`);
this gate is layer-2 defense-in-depth.
Coverage: 26 new adversarial tests (env vars, backticks, subshells,
quote concat, hex/octal escapes) plus 4 false-positive-discipline
tests proving legitimate commands like `$HOME`, `$(date)`,
```date``` still pass unblocked.
C4.4 β approvalRunId silent-bypass guard (src/gateway/plan-snapshot-persister.ts)
`persistApprovalMetadata` previously wrote an empty/missing
`approvalRunId` unconditionally. If a future emitter (or an edge
case we haven't seen) hands us a falsy runId, the write silently
succeeds β then the approval-side subagent gate in sessions-patch.ts
calls `getAgentRunContext("")` and gets undefined, silently
bypassing the concurrency check.
Fix: defensive throw at the top of the function with a diagnostic
message that explains the gate implication. <0.1% probability but
high operator-diagnostic value when it fires (failure-loud beats
fail-silent for security-adjacent state).
Coverage: 3 new unit tests via `__testingPlanSnapshotPersister`
test-only seam (empty string, whitespace, diagnostic message
mentions subagent gate).
Deferred to follow-up commits:
- C4.2 (retry re-hydration of \[PLAN_DECISION\] after empty-response)
requires a new `lastConsumedInjections` SessionEntry field plus
retry-logic changes in incomplete-turn.ts β substantial scope
best handled as its own commit.
- C4.3 (legacy-scalar migration dedup) β audit found the existing
atomic migration already closes this race; no code change needed.
Verification:
- pnpm tsgo:core, tsgo:core:test: clean for touched surface
- pnpm lint: clean for touched surface (5 pre-existing errors in
untracked qa-lab/helpers remain out of scope)
- pnpm test:
- src/agents/plan-mode/accept-edits-gate.test.ts: 66 pass (40
existing + 26 new C4.1 adversarial)
- src/gateway/plan-snapshot-persister.test.ts: 3 pass (new file)
β¦bug log events
Part 7 of Plan Mode 1.0 follow-up. Adds optional correlation fields
to the plan-mode debug event union so operators can trace a single
approval cycle across multiple log lines. Sessions can have many
approvals in their lifetime β sessionKey alone is insufficient for
cycle-level debugging.
Event shape changes (src/agents/plan-mode/plan-mode-debug-log.ts):
- Added optional `approvalRunId?: string` to: state_transition,
gate_decision, synthetic_injection, nudge_event, subagent_event,
approval_event, toast_event
- Added optional `approvalId?: string` to: state_transition,
gate_decision, synthetic_injection, approval_event, toast_event
- Pre-existing emitters keep the current logging shape (both fields
are optional); new emitters populate them when available.
Emitter wiring (src/gateway/sessions-patch.ts):
- Threaded `approvalRunId` + `approvalId` through the two
approval_event emitters (rejected_by_subagent_gate and accepted
paths) and the paired `synthetic_injection` event for
[PLAN_DECISION] approved/edited. Operators can now grep a single
approval cycle via:
tail -F ~/.openclaw/logs/gateway.err.log |
grep '\[plan-mode/' | grep approvalRunId=<id>
Test coverage (src/agents/plan-mode/plan-mode-debug-log.test.ts):
- 2 new tests pin the correlation-field pass-through for
approval_event and synthetic_injection. Existing 17 tests still
pass.
Deferred to future follow-ups:
- /plan self-test harness (synthetic plan-mode flow validator) β
substantial new module + CLI subcommand.
- Full correlation-field wiring across all emitters (nudge crons,
subagent lifecycle, gate decisions) β landed in subsequent commits
as those surfaces are touched.
Verification:
- pnpm tsgo:core: clean for touched surface
- pnpm lint: clean for touched surface (5 pre-existing errors in
untracked qa-lab/helpers remain out of scope)
- pnpm test on sessions-patch + plan-mode-debug-log: 68 pass
β¦.0 follow-up
Part 8 of Plan Mode 1.0 follow-up. Closes the doc loop for the C1-C4
+ C7 commits landed on this branch.
New file: docs/plans/PLAN-MODE-OPERATOR-RUNBOOK.md
- Field guide for operators triaging plan-mode incidents.
- Covers debug log activation (persistent config flag + env var
path), per-approval cycle tracing via the new C7 correlation
fields, and 5 common symptom β fix paths:
1. Approval card stays after click (PLAN_APPROVAL_EXPIRED)
2. Subagent stall blocks approval (BLOCKED_BY_SUBAGENTS /
WAITING_FOR_SUBAGENT_SETTLE)
3. Empty-response stall post-approval (C4.2 deferred)
4. Nudge cron noise during pending approval
5. Plan-mode state lost after compaction / restart
- Plus dedicated sections on shell-escape bypass detection (C4.1),
adversarial XSS regression (C1-R3), disk-full handling (C1-R4),
and cross-channel dedup (C1-R5).
Updates: docs/concepts/plan-mode.md
- Troubleshooting section now links to the operator runbook for
deeper incident triage scenarios beyond the basic PLAN_APPROVAL_EXPIRED
case.
Updates: docs/plans/PLAN-MODE-ARCHITECTURE.md
- New 'Shipped in Plan Mode 1.0 follow-up' section enumerates the
5 landed commits (C1 through C7) with their scope.
- Updated the 'Deferred items' list with post-1.0 status β marks
C4.1 (shell-escape), C4.3 (legacy-scalar dedup verified closed),
C4.4 (approvalRunId guard), and C7 (debug correlation) as
shipped. C4.2 (retry re-hydration) remains deferred.
- New 'Still deferred out of 1.0 follow-up' table for C3b, C4.2,
C5, C6, C7b with rationale for each.
No code changes. Verification: pnpm docs lint passes on the markdown
(Mintlify link rules: root-relative paths, no .md suffix).
Contributor
Author
|
Stacked for testing purposes given size. Closing to re-divide now that it's achieved 1.0 state. |
This was referenced Apr 22, 2026
Contributor
Author
|
Migration map (added 2026-04-22) β for anyone landing here from search: This PR's content (C1-C8 hardening + Eva PR-3 lifecycle work) was verified content-equivalent to material already incorporated in the 8-part decomposition stack starting at #69449. See #68939 for the full migration map. Specifically:
Cherry-picks of all 10 stranded commits onto a fresh Part-9 base produced empty results, confirming content equivalence. |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Draft follow-up to #68939 β stacks on top of the plan-mode umbrella PR. Ships 5 code commits + 1 docs commit that harden the plan-mode 1.0 release.
This PR's diff intentionally overlaps with #68939 until that PR merges (both branches share commits through
906eb68403). The 6 new commits β2ed8258eb5througha08adb0813β are the actual delta this PR lands.What's shipped
2ed8258eb5PLAN_APPROVAL_EXPIREDerror code (Bug B stale-card auto-dismiss) + 41 plan-title XSS adversarial tests (R3) +PlanPersistStorageErrordisk-full classifier (R4) + 3 multi-channel approval dedup tests (R5). Swift protocol bindings regenerated.aae864e5ccopenclaw/plugin-sdk/telegram.sendDocumentTelegramfacade. Auto thread-ID parsing via the SDK'sparseTelegramTarget. Un-skipped failure-branch test + new topic-scoped target test.49d5d4ac1aevaluateAutoEnableForMatchhelper with compiled-regex cache (14 tests) + cron-side wiring atsrc/cron/isolated-agent/run.ts(4 integration tests). Respects user-toggled/plan off.dcb9f0ec7daccept-edits-gate.ts(26 adversarial fixtures covering env-var indirection, subshells, quote concatenation, hex/octal byte escapes) + approvalRunId silent-bypass guard inplan-snapshot-persister.ts.c0aaabb1f1approvalRunId+approvalIdon 7 debug event kinds + wired throughsessions-patch.tsapproval emitters.a08adb0813docs/plans/PLAN-MODE-OPERATOR-RUNBOOK.mdcovering 5 symptomβfix flows + architecture doc updated with shipped/deferred status.Totals: ~1,050 LoC net, ~105 new tests, 95%-confidence gate per commit.
Deferred out of this PR (future focused follow-ups)
approvalTimeoutSecondsruntimeSessionEntry.lastConsumedInjectionsfield +consumePendingAgentInjectionsmodifications + retry-logic re-inject.TelegramDocumentOpts.inlineButtonsextension + plan-bridge button construction./plan self-testharnessVerification (per commit)
pnpm tsgo:core+pnpm tsgo:core:testβ clean for touched surface (pre-existing moonshot-stream-wrappers error in an untracked WIP file remains; unrelated).pnpm lintβ clean for touched surface (5 pre-existing errors in untracked qa-lab/helpers remain out of scope).pnpm teston every touched test file β all green.pnpm buildfails in the Discord extension runtime-staging script withdiscord-api-types must resolve to an exact installed version, got: ^0.38.47. Unrelated to this PR's changes; tracked outside this branch.Sequencing note for maintainers
This PR is opened as DRAFT because #68939 is still in review. Merge order:
mainfirst.main(fast-forward, no conflicts expected β my 6 commits are purely additive on top of906eb68403).Alternatively, cherry-pick commits onto #68939's branch if the maintainer prefers a single umbrella. Each commit is independently verified and cherry-pickable.
Test plan
origin/mainafter feat(plan-mode): full rollout + iter-1/2/3 hardening (consolidates PRs A/B/C/D/E/F/7/8/10/11)Β #68939 merges.pnpm protocol:gen:swift)./plan offmid-approval), verifyPLAN_APPROVAL_EXPIREDauto-dismiss.autoEnableFor: setagents.defaults.planMode.autoEnableFor: ["^openai/gpt-5\\."]+planMode.enabled: true, invoke a fresh GPT-5.x session via cron, verifyplanMode.mode = "plan"at turn start.Related
docs/plans/PLAN-MODE-ARCHITECTURE.mddocs/plans/PLAN-MODE-OPERATOR-RUNBOOK.md