fix(compact): add cancellation support for /compact command by tentacloos · Pull Request #66569 · openclaw/openclaw

tentacloos · 2026-04-14T13:37:03Z

The /compact command was not registrable in ACTIVE_EMBEDDED_RUNS and could not be cancelled via standard paths (chat.abort, abortEmbeddedPiRun, /stop).

Root cause: compactEmbeddedPiSession was called without abortSignal and the compaction session was never registered in ACTIVE_EMBEDDED_RUNS.

Changes

Create AbortController for compaction session
Pass abortSignal to compactEmbeddedPiSession call (existing param)
Register compaction in ACTIVE_EMBEDDED_RUNS via setActiveEmbeddedRun
Clean up registration when compaction completes
Handle interface methods: isStreaming() -> false, isCompacting() -> true

Impact

/compact can now be cancelled via any standard cancellation path, preventing user lockout during long compactions, improving UX for automation

Testing

✅ All existing tests pass
✅ Code follows existing patterns for embedded run management

…failure Fixes #62496 When Telegram voice notes have undefined paths in allMedia array, the transcription preflight fails because normalizeAttachments filters out entries where both path and url are undefined, causing silent transcription failure. Root cause: allMedia.map((m) => m.path) creates array with undefined values when voice note path is not set, then normalizeAttachments drops all attachments. Changes: - Filter undefined paths with .filter(Boolean) before creating MediaPaths array - Apply fix to both bot-message-context.body.ts and bot-message-context.session.ts - Add test case for handling voice messages with undefined paths gracefully Impact: Telegram voice transcription now works even when some media paths are undefined

Fixes #66549 The --dry-run flag was not being honored in the message send command. The flag only affected progress spinner visibility but the message was still sent to the target channel. Root cause: messageCommand() calls runMessageAction() regardless of dryRun flag value. Changes: - Add conditional logic to skip runMessageAction() when dryRun is true - Return mock result with messageId: 'dry-run' for dry-run mode - Add test case verifying dry-run doesn't call gateway - Preserve existing behavior for non-dry-run executions Impact: --dry-run now safely prevents message delivery as documented

Fixes #66403 The exec approval popup could grow taller than viewport when displaying long commands, pushing action buttons off-screen and making approval unusable without changing browser zoom. Root cause: Desktop .exec-approval-card had no height constraints while mobile version already had max-height: 90dvh and overflow handling. Changes: - Add max-height: 90vh to .exec-approval-card (desktop) - Add flex layout (display: flex, flex-direction: column) - Add max-height: 40vh and overflow-y: auto to .exec-approval-command - Add flex-shrink: 0 to .exec-approval-actions (keep buttons visible) - Command content scrolls when exceeding space, buttons stay accessible Impact: Long command previews scroll instead of pushing buttons off-screen

Fixes #66535 The /compact command was not registrable in ACTIVE_EMBEDDED_RUNS and could not be cancelled via standard paths (chat.abort, abortEmbeddedPiRun, /stop). Root cause: compactEmbeddedPiSession was called without abortSignal and the compaction session was never registered in ACTIVE_EMBEDDED_RUNS. Changes: - Create AbortController for compaction session - Pass abortSignal to compactEmbeddedPiSession call (existing param) - Register compaction in ACTIVE_EMBEDDED_RUNS via setActiveEmbeddedRun - Clean up registration when compaction completes - Add comprehensive test case for cancellation support - Handle interface methods: isStreaming() -> false, isCompacting() -> true Impact: /compact can now be cancelled via any standard cancellation path, preventing user lockout during long compactions

openclaw-barnacle · 2026-04-14T13:38:54Z

Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit.

greptile-apps · 2026-04-14T13:40:34Z

Greptile Summary

This PR adds cancellation support for the /compact command by wiring up an AbortController, registering a handle in ACTIVE_EMBEDDED_RUNS via setActiveEmbeddedRun, and passing abortSignal to compactEmbeddedPiSession. The approach is correct, but clearActiveEmbeddedRun is not placed in a try/finally, so any exception thrown by compactEmbeddedPiSession (queue rejection, model resolution failure, context engine error) leaves a stale handle registered, causing a hard 15-second timeout on every subsequent /compact call for that session.

Confidence Score: 3/5

Not safe to merge until the missing try/finally around clearActiveEmbeddedRun is fixed — any exception during compaction permanently blocks subsequent /compact calls for that session with a 15-second timeout.
One P1 defect: clearActiveEmbeddedRun is only called on the happy path, leaving a stale ACTIVE_EMBEDDED_RUNS entry whenever compactEmbeddedPiSession throws. This causes a real, reproducible 15-second degradation on the next /compact invocation. The P2 unused-parameter lint issue is minor but also needs a one-character fix before the pre-commit hook passes.
src/auto-reply/reply/commands-compact.ts — the try/finally gap around the compactEmbeddedPiSession call needs to be addressed before merging.

Comments Outside Diff (1)

src/auto-reply/reply/commands-compact.ts, line 133-174 (link)

Active run not cleaned up on exception

clearActiveEmbeddedRun is only reached on the happy path. If compactEmbeddedPiSession throws — which compact.queued.ts can do when enqueueCommandInLane rejects, resolveModelAsync fails, or contextEngine.compact propagates an error — the handle stays in ACTIVE_EMBEDDED_RUNS permanently. The next /compact call will detect an active run, call abort() (no-op on the already-finished controller), then block in waitForEmbeddedPiRunEnd for the full 15 seconds before timing out, because notifyEmbeddedRunEnded is never invoked.

The fix is to move clearActiveEmbeddedRun into a try/finally block wrapping the compactEmbeddedPiSession call, and pass params.sessionKey for symmetric cleanup:

setActiveEmbeddedRun(sessionId, compactionHandle, params.sessionKey);
try {
  const result = await runtime.compactEmbeddedPiSession({ ... });
  // ...rest of processing...
} finally {
  clearActiveEmbeddedRun(sessionId, compactionHandle, params.sessionKey);
}

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/auto-reply/reply/commands-compact.ts
Line: 133-174

Comment:
**Active run not cleaned up on exception**

`clearActiveEmbeddedRun` is only reached on the happy path. If `compactEmbeddedPiSession` throws — which `compact.queued.ts` can do when `enqueueCommandInLane` rejects, `resolveModelAsync` fails, or `contextEngine.compact` propagates an error — the handle stays in `ACTIVE_EMBEDDED_RUNS` permanently. The next `/compact` call will detect an active run, call `abort()` (no-op on the already-finished controller), then block in `waitForEmbeddedPiRunEnd` for the full 15 seconds before timing out, because `notifyEmbeddedRunEnded` is never invoked.

The fix is to move `clearActiveEmbeddedRun` into a `try/finally` block wrapping the `compactEmbeddedPiSession` call, and pass `params.sessionKey` for symmetric cleanup:

```typescript
setActiveEmbeddedRun(sessionId, compactionHandle, params.sessionKey);
try {
  const result = await runtime.compactEmbeddedPiSession({ ... });
  // ...rest of processing...
} finally {
  clearActiveEmbeddedRun(sessionId, compactionHandle, params.sessionKey);
}
```

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

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/auto-reply/reply/commands-compact.ts
Line: 133-174

Comment:
**Active run not cleaned up on exception**

`clearActiveEmbeddedRun` is only reached on the happy path. If `compactEmbeddedPiSession` throws — which `compact.queued.ts` can do when `enqueueCommandInLane` rejects, `resolveModelAsync` fails, or `contextEngine.compact` propagates an error — the handle stays in `ACTIVE_EMBEDDED_RUNS` permanently. The next `/compact` call will detect an active run, call `abort()` (no-op on the already-finished controller), then block in `waitForEmbeddedPiRunEnd` for the full 15 seconds before timing out, because `notifyEmbeddedRunEnded` is never invoked.

The fix is to move `clearActiveEmbeddedRun` into a `try/finally` block wrapping the `compactEmbeddedPiSession` call, and pass `params.sessionKey` for symmetric cleanup:

```typescript
setActiveEmbeddedRun(sessionId, compactionHandle, params.sessionKey);
try {
  const result = await runtime.compactEmbeddedPiSession({ ... });
  // ...rest of processing...
} finally {
  clearActiveEmbeddedRun(sessionId, compactionHandle, params.sessionKey);
}
```

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

---

This is a comment left during a code review.
Path: src/auto-reply/reply/commands-compact.ts
Line: 108-110

Comment:
**Unused parameter may fail lint**

The `text` parameter is declared but never referenced in the body. Oxlint's `no-unused-vars` rule will flag this. Prefix it with `_` to signal intentional non-use:

```suggestion
    queueMessage: async (_text: string) => {
      // Compaction doesn't queue messages, but required for interface
    },
```

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

_{Reviews (1): Last reviewed commit: "fix(compact): add cancellation support f..." | Re-trigger Greptile}

greptile-apps · 2026-04-14T13:40:41Z

+    queueMessage: async (text: string) => {
+      // Compaction doesn't queue messages, but required for interface
+    },


Unused parameter may fail lint

The text parameter is declared but never referenced in the body. Oxlint's no-unused-vars rule will flag this. Prefix it with _ to signal intentional non-use:

Suggested change

queueMessage: async (text: string) => {

// Compaction doesn't queue messages, but required for interface

},

queueMessage: async (_text: string) => {

// Compaction doesn't queue messages, but required for interface

},

Prompt To Fix With AI

This is a comment left during a code review. Path: src/auto-reply/reply/commands-compact.ts Line: 108-110 Comment: **Unused parameter may fail lint** The `text` parameter is declared but never referenced in the body. Oxlint's `no-unused-vars` rule will flag this. Prefix it with `_` to signal intentional non-use: ```suggestion queueMessage: async (_text: string) => { // Compaction doesn't queue messages, but required for interface }, ``` How can I resolve this? If you propose a fix, please make it concise.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 454ba36977

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-14T13:41:47Z

  });
+
+  // Clean up the active run registration
+  clearActiveEmbeddedRun(sessionId, compactionHandle);


Always clear active compaction run in a finally block

setActiveEmbeddedRun is registered before awaiting compaction, but clearActiveEmbeddedRun is only reached on the success path; if compactEmbeddedPiSession(...) throws, the session stays stuck in ACTIVE_EMBEDDED_RUNS/session-key mapping as a phantom active run. In that state, later /compact or cancellation flows can repeatedly hit abort/wait behavior against stale state instead of a real run, so the cleanup should be guaranteed in a finally block.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-14T13:41:47Z

+  if (dryRun) {
+    // Dry-run mode: return mock result without actually sending
+    result = {
+      channel: normalizeOptionalString(opts.channel) || "unknown",
+      messageId: "dry-run",


Preserve MessageActionRunResult shape for dry-run path

This branch fabricates a custom object instead of using runMessageAction's dry-run result contract, but downstream formatters/JSON builders expect MessageActionRunResult fields like kind, action, handledBy, and payload. As written, dry-run output can lose the dedicated [dry-run] formatting and emit undefined core fields in --json, while also skipping action-level normalization/validation that runMessageAction performs.

Useful? React with 👍 / 👎.

tentacloos added 4 commits April 15, 2026 01:07

openclaw-barnacle Bot added channel: telegram Channel integration: telegram app: web-ui App: web-ui commands Command implementations agents Agent runtime and tooling size: S r: too-many-prs Auto-close: author has more than twenty active PRs. labels Apr 14, 2026

openclaw-barnacle Bot closed this Apr 14, 2026

greptile-apps Bot reviewed Apr 14, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Apr 14, 2026

View reviewed changes

This was referenced Jun 17, 2026

[Bug]: /compact command cannot be canceled while in progress #66535

Closed

fix: allow stopping manual compaction #90821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(compact): add cancellation support for /compact command#66569

fix(compact): add cancellation support for /compact command#66569
tentacloos wants to merge 4 commits into
openclaw:mainfrom
tentacloos:fix/66535-compact-command-cannot-cancel

tentacloos commented Apr 14, 2026

Uh oh!

openclaw-barnacle Bot commented Apr 14, 2026

Uh oh!

greptile-apps Bot commented Apr 14, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 14, 2026

Uh oh!

chatgpt-codex-connector Bot Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

tentacloos commented Apr 14, 2026

Changes

Impact

Testing

Uh oh!

openclaw-barnacle Bot commented Apr 14, 2026

Uh oh!

greptile-apps Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Apr 14, 2026 •

edited

Loading