Whitespace-only payloads bypass the empty check

mergeNonErrorTextPayloads uses a bare falsiness check (if (!text)) to skip empty payloads, so a chunk containing only whitespace (e.g. " ") is truthy and gets appended to merged. By contrast, pickLastNonEmptyTextFromPayloads (the function this replaces in the hot path) trims before checking:

const clean = (payloads[i]?.text ?? "").trim();
if (clean) { return clean; }

As a result, whitespace-only chunks that the old code would silently ignore will now appear verbatim in the final merged string and be delivered to the channel. A simple guard matches the existing helper's behaviour:

The unit tests in helpers.test.ts only cover "" (empty string) in the "returns undefined when all payloads have empty text" case; adding a " " (whitespace-only) assertion would catch this gap.

This is a comment left during a code review.
Path: src/cron/isolated-agent/helpers.ts
Line: 22-29

Comment:
**Whitespace-only payloads bypass the empty check**

`mergeNonErrorTextPayloads` uses a bare falsiness check (`if (!text)`) to skip empty payloads, so a chunk containing only whitespace (e.g. `"  "`) is truthy and gets appended to `merged`. By contrast, `pickLastNonEmptyTextFromPayloads` (the function this replaces in the hot path) trims before checking:

```ts
const clean = (payloads[i]?.text ?? "").trim();
if (clean) { return clean; }
```

As a result, whitespace-only chunks that the old code would silently ignore will now appear verbatim in the final merged string and be delivered to the channel. A simple guard matches the existing helper's behaviour:

```suggestion
    const text = typeof payload?.text === "string" ? payload.text.trim() : "";
    if (!text) {
      continue;
    }
    merged += text;
    sawText = true;
```

The unit tests in `helpers.test.ts` only cover `""` (empty string) in the "returns undefined when all payloads have empty text" case; adding a `"   "` (whitespace-only) assertion would catch this gap.

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

Fixed in c576af4 — split the check into raw (untrimmed, used for concatenation) and raw.trim() (used for emptiness check). Whitespace-only payloads are now skipped without altering the content of real payloads. Added test case for whitespace-only filtering.

Text-only deliveryPayload is silently dropped

Before this PR the condition was:

deliveryPayload !== undefined
  ? [deliveryPayload]
  : ...

Now it is:

deliveryPayload !== undefined && deliveryPayloadHasStructuredContent
  ? [deliveryPayload]
  : mergedText
    ? [{ text: mergedText }]
    : ...

When deliveryPayload exists but carries only text (no mediaUrl, mediaUrls, or channelData), the new code discards deliveryPayload entirely and constructs a fresh { text: mergedText } object instead. This is the intended behaviour for the merge fix, and is safe as long as the DeliveryPayload type captures all fields that downstream delivery code cares about.

However, the code comments and the PR description don't acknowledge this side-effect explicitly. If deliveryPayload ever carries extra runtime fields beyond what the TypeScript type declares (e.g. metadata added by a plugin), those fields would be silently dropped. It would be safer to either spread deliveryPayload and override just the text field when structured content is absent, or add an explicit comment noting that the object is intentionally reconstructed:

// When there is no structured content, rebuild the payload with merged text
// so that all non-error text chunks are included verbatim (#13812).
: mergedText
  ? [{ ...deliveryPayload, text: mergedText }]  // spread to preserve any extra fields
  : ...

This is a comment left during a code review.
Path: src/cron/isolated-agent/run.ts
Line: 772-779

Comment:
**Text-only `deliveryPayload` is silently dropped**

Before this PR the condition was:

```ts
deliveryPayload !== undefined
  ? [deliveryPayload]
  : ...
```

Now it is:

```ts
deliveryPayload !== undefined && deliveryPayloadHasStructuredContent
  ? [deliveryPayload]
  : mergedText
    ? [{ text: mergedText }]
    : ...
```

When `deliveryPayload` exists but carries **only text** (no `mediaUrl`, `mediaUrls`, or `channelData`), the new code discards `deliveryPayload` entirely and constructs a fresh `{ text: mergedText }` object instead. This is the intended behaviour for the merge fix, and is safe as long as the `DeliveryPayload` type captures all fields that downstream delivery code cares about.

However, the code comments and the PR description don't acknowledge this side-effect explicitly. If `deliveryPayload` ever carries extra runtime fields beyond what the TypeScript type declares (e.g. metadata added by a plugin), those fields would be silently dropped. It would be safer to either spread `deliveryPayload` and override just the `text` field when structured content is absent, or add an explicit comment noting that the object is intentionally reconstructed:

```ts
// When there is no structured content, rebuild the payload with merged text
// so that all non-error text chunks are included verbatim (#13812).
: mergedText
  ? [{ ...deliveryPayload, text: mergedText }]  // spread to preserve any extra fields
  : ...
```

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

Fixed in c576af4 — now uses { ...deliveryPayload, text: mergedText } to preserve any metadata fields (replyToId, replyToTag, etc.) from the original delivery payload when rebuilding with merged text.

Preserve text payload metadata when merging announce chunks

This branch drops the original deliveryPayload whenever it is text-only and rebuilds it as { text: mergedText }, which strips metadata fields like replyToId, replyToCurrent, and replyToTag from the payload. That changes delivery behavior for cron announce runs where the agent payload intentionally targets a reply/thread: the outbound layer reads payload.replyToId when building send overrides (src/infra/outbound/deliver.ts:713), so these messages will now be posted as standalone messages instead of threaded replies once chunk merging is enabled.

Useful? React with 👍 / 👎.

Fixed in c576af4 — spread operator preserves all original payload fields including replyToId, replyToCurrent, and replyToTag.

Preserve merge metadata from all text chunks

When mergedText is selected, the new payload is built from only deliveryPayload (the last deliverable chunk), so per-chunk metadata from earlier chunks is dropped. In buildEmbeddedRunPayloads each assistant chunk is parsed independently for reply directives (replyToId, replyToTag, replyToCurrent), so a multi-chunk response where only the first chunk carries threading directives will now deliver merged text without those directives and post to the wrong thread/channel context.

Useful? React with 👍 / 👎.