Skip to content

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
openclaw:mainfrom
electricsheephq:feat/plan-mode-1.0-followup
Closed

feat(plan-mode): 1.0 release follow-up β€” hardening + Telegram attachment + config + security + DX (stacks on #68939)#69324
100yenadmin wants to merge 166 commits into
openclaw:mainfrom
electricsheephq:feat/plan-mode-1.0-followup

Conversation

@100yenadmin

Copy link
Copy Markdown
Contributor

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 β€” 2ed8258eb5 through a08adb0813 β€” are the actual delta this PR lands.

What's shipped

Commit Scope
2ed8258eb5 C1 rollout hardening β€” PLAN_APPROVAL_EXPIRED error code (Bug B stale-card auto-dismiss) + 41 plan-title XSS adversarial tests (R3) + PlanPersistStorageError disk-full classifier (R4) + 3 multi-channel approval dedup tests (R5). Swift protocol bindings regenerated.
aae864e5cc C2 Telegram PR-14 re-wire β€” live markdown attachment delivery via the public openclaw/plugin-sdk/telegram.sendDocumentTelegram facade. Auto thread-ID parsing via the SDK's parseTelegramTarget. Un-skipped failure-branch test + new topic-scoped target test.
49d5d4ac1a C3a autoEnableFor runtime β€” pure evaluateAutoEnableForMatch helper with compiled-regex cache (14 tests) + cron-side wiring at src/cron/isolated-agent/run.ts (4 integration tests). Respects user-toggled /plan off.
dcb9f0ec7d C4 security hardening β€” shell-escape layered defense in accept-edits-gate.ts (26 adversarial fixtures covering env-var indirection, subshells, quote concatenation, hex/octal byte escapes) + approvalRunId silent-bypass guard in plan-snapshot-persister.ts.
c0aaabb1f1 C7 debug log correlation β€” optional approvalRunId + approvalId on 7 debug event kinds + wired through sessions-patch.ts approval emitters.
a08adb0813 C8 operator runbook + docs — new docs/plans/PLAN-MODE-OPERATOR-RUNBOOK.md covering 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)

Item Why deferred
C3b approvalTimeoutSeconds runtime Needs cron protocol surface additions (new payload kind or field) + scheduler + cleanup paths. Cross-surface change deserves its own commit.
C4.2 retry re-hydration on empty-response Needs new SessionEntry.lastConsumedInjections field + consumePendingAgentInjections modifications + retry-logic re-inject.
C5 Telegram inline-keyboard Accept/Revise Needs grammy callback_query handler registration + TelegramDocumentOpts.inlineButtons extension + plan-bridge button construction.
C6 Slack Block Kit cards Needs new render format + action-id handler + Revise modal. Similar scope to C5.
C7b /plan self-test harness New synthetic-flow validator + CLI subcommand + assertions.

Verification (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 test on every touched test file β€” all green.
  • Pre-existing build issue noted in C2: pnpm build fails in the Discord extension runtime-staging script with discord-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:

  1. Land feat(plan-mode): full rollout + iter-1/2/3 hardening (consolidates PRs A/B/C/D/E/F/7/8/10/11)Β #68939 to main first.
  2. This branch rebases onto main (fast-forward, no conflicts expected β€” my 6 commits are purely additive on top of 906eb68403).
  3. Mark this PR ready-for-review.

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

Related

  • Parent PR: #68939
  • Design doc (updated): docs/plans/PLAN-MODE-ARCHITECTURE.md
  • Operator runbook (new): docs/plans/PLAN-MODE-OPERATOR-RUNBOOK.md

Eva 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)
Eva 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).
@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 extensions: openai size: XL labels Apr 20, 2026
@100yenadmin

Copy link
Copy Markdown
Contributor Author

Stacked for testing purposes given size. Closing to re-divide now that it's achieved 1.0 state.

@100yenadmin

Copy link
Copy Markdown
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.

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 extensions: openai gateway Gateway runtime size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant