__testing export consumed in production code

buildStoredUserDeliveryPayload is invoked via subagentAnnounceTesting in three production paths (lines 188, 248, and via subagent-registry-lifecycle.ts). The __testing export is conventionally test-only; importing it in production code bypasses that boundary and means tree-shakers/bundlers cannot safely strip test helpers, and any future refactoring of the testing surface will silently break production behavior.

Export buildStoredUserDeliveryPayload as a proper named export from subagent-announce.ts and import it directly instead of routing through __testing.

This is a comment left during a code review.
Path: src/agents/subagent-registry-lifecycle.ts
Line: 16

Comment:
**`__testing` export consumed in production code**

`buildStoredUserDeliveryPayload` is invoked via `subagentAnnounceTesting` in three production paths (lines 188, 248, and via `subagent-registry-lifecycle.ts`). The `__testing` export is conventionally test-only; importing it in production code bypasses that boundary and means tree-shakers/bundlers cannot safely strip test helpers, and any future refactoring of the testing surface will silently break production behavior.

Export `buildStoredUserDeliveryPayload` as a proper named export from `subagent-announce.ts` and import it directly instead of routing through `__testing`.

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

Double normalization of delivery text

deliverReplies normalizes payload.text via normalizeIMessageDeliveryText on line 123 (when building reply.text) and then chunkIMessageText normalizes the same, already-normalized value a second time (line 59). Since normalizeIMessageDeliveryText is idempotent this is functionally harmless, but it's redundant and would mask any future non-idempotency introduced by sanitization or fence-closing changes.

Consider removing the normalizeIMessageDeliveryText call inside chunkIMessageText and making it the caller's responsibility, or conversely, not pre-normalizing in deliverReplies and letting the chunker own normalization exclusively.

This is a comment left during a code review.
Path: extensions/imessage/src/monitor/deliver.ts
Line: 58-100

Comment:
**Double normalization of delivery text**

`deliverReplies` normalizes `payload.text` via `normalizeIMessageDeliveryText` on line 123 (when building `reply.text`) and then `chunkIMessageText` normalizes the same, already-normalized value a second time (line 59). Since `normalizeIMessageDeliveryText` is idempotent this is functionally harmless, but it's redundant and would mask any future non-idempotency introduced by sanitization or fence-closing changes.

Consider removing the `normalizeIMessageDeliveryText` call inside `chunkIMessageText` and making it the caller's responsibility, or conversely, not pre-normalizing in `deliverReplies` and letting the chunker own normalization exclusively.

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

Duplicated regex constants across modules

OPENCLAW_INTERNAL_BLOCK_RE and OPENCLAW_INTERNAL_LINE_RE are defined verbatim in both subagent-announce.ts and sanitize-outbound.ts. If the pattern ever needs updating, both copies must be changed in sync. Consider extracting these into a shared constant (e.g. in a subagent-text-patterns.ts internal helper) so there is a single source of truth.

This is a comment left during a code review.
Path: src/agents/subagent-announce.ts
Line: 99-106

Comment:
**Duplicated regex constants across modules**

`OPENCLAW_INTERNAL_BLOCK_RE` and `OPENCLAW_INTERNAL_LINE_RE` are defined verbatim in both `subagent-announce.ts` and `sanitize-outbound.ts`. If the pattern ever needs updating, both copies must be changed in sync. Consider extracting these into a shared constant (e.g. in a `subagent-text-patterns.ts` internal helper) so there is a single source of truth.

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

Derive user payload from chosen completion findings

When descendant completions are present, findings is intentionally set from childCompletionFindings, but this code still prefers persistedUserDeliveryPayload (captured earlier from roundOneReply) for userDeliveryPayload. In completion mode for non-subagent requesters, buildDirectUserCompletionPrompt then uses that stale payload, so users can receive an outdated summary instead of the descendant-derived result that this flow selected as authoritative. This is reproducible whenever a run has both a frozen round-one reply and direct-child completion findings.

Useful? React with 👍 / 👎.

Release delivery claim when dispatch aborts early

When announceId is provided and the signal is already aborted, this early return exits before the finally block that clears or finalizes the persisted claim. Because the claim was already created earlier in the function, the run is left in deliveryClaim.state = "claimed" until lease expiry, and subsequent retries for the same announce can be rejected as delivery-already-in-flight without attempting queue/direct delivery.

Useful? React with 👍 / 👎.

Skip per-chunk fence normalization in iMessage send

This normalizes (and potentially auto-closes) code fences on every sendMessageIMessage call, but chunked monitor delivery already normalizes before splitting. If a fenced block is longer than textLimit, individual chunks can have an odd fence count, so this line appends extra closing fences per chunk and corrupts the rendered code output for long responses.

Useful? React with 👍 / 👎.