Skip to content

feat(adapters): add sendPayload to batch-c (Feishu, IRC, Matrix, Nextcloud-Talk, Nostr, Tlon, Twitch)#30143

Open
nohat wants to merge 6 commits intoopenclaw:mainfrom
nohat:lifecycle/adapter-sendpayload-batch-c-v2
Open

feat(adapters): add sendPayload to batch-c (Feishu, IRC, Matrix, Nextcloud-Talk, Nostr, Tlon, Twitch)#30143
nohat wants to merge 6 commits intoopenclaw:mainfrom
nohat:lifecycle/adapter-sendpayload-batch-c-v2

Conversation

@nohat
Copy link
Contributor

@nohat nohat commented Feb 28, 2026

Part of Message Reliability: Durable SQLite Outbox, Recovery Worker, and Unified sendPayload (#32063)

Summary

  • Problem: Channel adapters lack a unified sendPayload method for the delivery pipeline
  • Why it matters: The outbox-based delivery pipeline (feat(outbound): prefer sendPayload for all payloads when adapter supports it #29997) needs sendPayload per adapter
  • What changed: Added sendPayload to batch-c adapters (Google Chat, Line, RCS, Web). All iterate mediaUrls and delegate to existing sendText/sendMedia
  • What did NOT change: Existing sendText/sendMedia methods unchanged

Change Type (select all)

  • Feature

Scope (select all touched areas)

  • Integrations

Linked Issue/PR

User-visible / Behavior Changes

None — additive internal method only.

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: Any
  • Runtime/container: Node 22+
  • Integration/channel: Google Chat, Line, RCS, Web

Steps

  1. pnpm check passes
  2. Send messages through affected channels — delivery unchanged

Expected

  • No behavior change; new method available for delivery pipeline

Actual

  • Same as expected

Evidence

  • Failing test/log before + passing after (CI green)

Human Verification (required)

  • Verified scenarios: TypeScript compilation, existing test suite
  • Edge cases checked: Media-only, text-only, mixed payloads via sendPayload
  • What you did not verify: Live delivery through all 4 channels

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Failure Recovery (if this breaks)

Risks and Mitigations

None — additive change.

@openclaw-barnacle openclaw-barnacle bot added channel: matrix Channel integration: matrix channel: nextcloud-talk Channel integration: nextcloud-talk channel: nostr Channel integration: nostr channel: tlon Channel integration: tlon channel: feishu Channel integration: feishu channel: twitch Channel integration: twitch channel: irc size: S labels Feb 28, 2026
@nohat nohat force-pushed the lifecycle/adapter-sendpayload-batch-c-v2 branch from 8cf3c40 to d2320ce Compare March 2, 2026 04:09
@nohat nohat marked this pull request as ready for review March 2, 2026 04:39
Copilot AI review requested due to automatic review settings March 2, 2026 04:39
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2320ce828

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +18 to +22
return matrixOutbound.sendMedia!({
...ctx,
text: ctx.payload.text ?? "",
mediaUrl: urls[0],
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Send every Matrix media URL in sendPayload

When payload.channelData is present, delivery uses sendPayload, and this implementation only forwards urls[0] to sendMedia. Any additional entries in payload.mediaUrls are silently dropped, so multi-attachment replies lose content instead of sending one Matrix event per media item (which is how the generic outbound path behaves).

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. Matrix sendPayload now iterates all media URLs, sending one Matrix event per URL (caption on the first, empty text for subsequent). The comment "Matrix API supports one media attachment per event" is preserved — the fix sends multiple events, not multiple attachments per event.

Comment on lines +307 to +310
if (ctx.payload.text) parts.push(ctx.payload.text);
for (const url of urls) parts.push(url);
const combined = parts.join("\n");
return ircPlugin.outbound!.sendText!({ ...ctx, text: combined });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid sending empty IRC text for channelData-only payloads

This path builds combined from payload.text and media URLs, then unconditionally calls sendText; for channelData-only payloads, combined is empty. Outbound delivery explicitly preserves channelData-only payloads, so this now turns metadata-only payloads into hard IRC send failures (sendMessageIrc rejects empty messages) whenever sendPayload is selected.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. IRC sendPayload now delegates to sendMedia for each URL (caption on first, empty for subsequent), preserving the existing \n\nAttachment: <url> formatting. The text-only fallback explicitly passes text: ctx.payload.text ?? "".

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds sendPayload implementations to several outbound channel adapters so the delivery pipeline can call a single adapter entrypoint that routes a ReplyPayload to existing sendText / sendMedia behavior.

Changes:

  • Added sendPayload to Feishu, Twitch, and Tlon outbound adapters to iterate mediaUrls and delegate to existing sendMedia/sendText.
  • Added sendPayload to IRC / Nextcloud Talk / Nostr adapters with URL-to-text handling for media.
  • Added sendPayload to Matrix outbound adapter (currently only sends the first media URL).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
extensions/twitch/src/outbound.ts Adds sendPayload that delegates to existing sendMedia/sendText and iterates media URLs.
extensions/tlon/src/channel.ts Adds sendPayload that delegates to existing sendMedia/sendText and iterates media URLs.
extensions/nostr/src/channel.ts Adds sendPayload that appends media URLs to text and delegates to sendText.
extensions/nextcloud-talk/src/channel.ts Adds sendPayload (currently appends URLs into one message rather than delegating to sendMedia).
extensions/matrix/src/outbound.ts Adds sendPayload (currently drops additional mediaUrls beyond the first).
extensions/irc/src/channel.ts Adds sendPayload (currently appends URLs into one message rather than delegating to sendMedia).
extensions/feishu/src/outbound.ts Adds sendPayload that delegates to existing sendMedia/sendText and iterates media URLs.

Comment on lines +305 to +310
// IRC has no native media — append URLs to text
const parts: string[] = [];
if (ctx.payload.text) parts.push(ctx.payload.text);
for (const url of urls) parts.push(url);
const combined = parts.join("\n");
return ircPlugin.outbound!.sendText!({ ...ctx, text: combined });
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sendPayload implementation bypasses the existing sendMedia behavior (which formats media as ...\n\nAttachment: <url>) and instead concatenates all URLs into a single text message. That’s a behavior change (and can exceed the IRC textChunkLimit once sendPayload is used by the delivery pipeline). Prefer delegating to sendMedia for each URL (caption on first, empty for subsequent) to preserve current formatting and splitting.

Suggested change
// IRC has no native media — append URLs to text
const parts: string[] = [];
if (ctx.payload.text) parts.push(ctx.payload.text);
for (const url of urls) parts.push(url);
const combined = parts.join("\n");
return ircPlugin.outbound!.sendText!({ ...ctx, text: combined });
// If there is no media, just send the text as-is.
if (!urls.length) {
const text = ctx.payload.text ?? "";
return ircPlugin.outbound!.sendText!({ ...ctx, text });
}
// Delegate to sendMedia for each URL to preserve existing formatting.
let lastResult: any;
for (let i = 0; i < urls.length; i++) {
const url = urls[i];
const text = i === 0 ? ctx.payload.text ?? "" : "";
lastResult = await ircPlugin.outbound!.sendMedia!({
...ctx,
text,
mediaUrl: url,
});
}
return lastResult;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. IRC sendPayload now delegates to sendMedia for each URL (caption on first, empty for subsequent), preserving the existing \n\nAttachment: <url> formatting. The text-only fallback explicitly passes text: ctx.payload.text ?? "".

Comment on lines +271 to +276
// Nextcloud Talk has no native media — append URLs to text
const parts: string[] = [];
if (ctx.payload.text) parts.push(ctx.payload.text);
for (const url of urls) parts.push(url);
const combined = parts.join("\n");
return nextcloudTalkPlugin.outbound!.sendText!({ ...ctx, text: combined });
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sendPayload implementation changes media delivery behavior by appending all URLs into a single text message rather than reusing sendMedia (which currently formats as ...\n\nAttachment: <url> and sends one message per attachment). Once the pipeline starts preferring sendPayload, this will alter output formatting and can create overly-long messages. Prefer iterating URLs and delegating to sendMedia (caption on first, empty for subsequent) to preserve existing behavior.

Suggested change
// Nextcloud Talk has no native media — append URLs to text
const parts: string[] = [];
if (ctx.payload.text) parts.push(ctx.payload.text);
for (const url of urls) parts.push(url);
const combined = parts.join("\n");
return nextcloudTalkPlugin.outbound!.sendText!({ ...ctx, text: combined });
// Nextcloud Talk has no native media — reuse sendText/sendMedia behavior
if (!urls.length) {
// No media: behave like a plain text send
return nextcloudTalkPlugin.outbound!.sendText!({
...ctx,
text: ctx.payload.text ?? "",
});
}
let lastResult: Awaited<ReturnType<NonNullable<typeof nextcloudTalkPlugin.outbound>["sendMedia"]>> | undefined;
for (const [index, url] of urls.entries()) {
const caption = index === 0 ? (ctx.payload.text ?? "") : "";
lastResult = await nextcloudTalkPlugin.outbound!.sendMedia!({
...ctx,
text: caption,
mediaUrl: url,
});
}
// Return the result of the last media send
return lastResult!;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. Nextcloud Talk sendPayload now delegates to sendMedia for each URL (caption on first, empty for subsequent), preserving the existing \n\nAttachment: <url> formatting. Same approach as the IRC fix.

Comment on lines +96 to +100
sendPayload: async (ctx) => {
const urls = ctx.payload.mediaUrls?.length
? ctx.payload.mediaUrls
: ctx.payload.mediaUrl
? [ctx.payload.mediaUrl]
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says sendPayload was added for “Google Chat, Line, RCS, Web”, but this diff adds sendPayload for Feishu/IRC/Matrix/Nextcloud Talk/Nostr/Tlon/Twitch. Please update the PR description (or title) so the stated scope matches the actual changed adapters.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct — the PR description was stale. This PR covers Feishu, IRC, Matrix, Nextcloud Talk, Nostr, Tlon, and Twitch.

Comment on lines +17 to +22
// Matrix API supports only one media attachment per message
return matrixOutbound.sendMedia!({
...ctx,
text: ctx.payload.text ?? "",
mediaUrl: urls[0],
});
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendPayload only sends the first media URL (urls[0]) and drops any remaining attachments in payload.mediaUrls. This will change behavior vs the existing delivery path (which sends each media URL as its own message). Update sendPayload to iterate all URLs (caption on the first, empty caption for the rest) and return the last delivery result.

Suggested change
// Matrix API supports only one media attachment per message
return matrixOutbound.sendMedia!({
...ctx,
text: ctx.payload.text ?? "",
mediaUrl: urls[0],
});
// Matrix API supports only one media attachment per message.
// Send each media URL as its own message: caption on the first, empty text for the rest.
let lastResult;
for (let index = 0; index < urls.length; index++) {
const url = urls[index];
const text = index === 0 ? ctx.payload.text ?? "" : "";
// eslint-disable-next-line no-await-in-loop
lastResult = await matrixOutbound.sendMedia!({
...ctx,
text,
mediaUrl: url,
});
}
return lastResult!;

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. Matrix sendPayload now iterates all media URLs, sending one Matrix event per URL (caption on the first, empty text for subsequent). The comment "Matrix API supports one media attachment per event" is preserved — the fix sends multiple events, not multiple attachments per event.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 2, 2026

Greptile Summary

Added sendPayload method to 7 channel adapters (Feishu, IRC, Matrix, Nextcloud-Talk, Nostr, Tlon, Twitch) to support the outbox-based delivery pipeline. The implementations follow two patterns:

  1. Native media support (Feishu, Tlon, Twitch): Send first media with text, then remaining media with empty text
  2. No native media (IRC, Nextcloud-Talk, Nostr): Append media URLs to text body
  3. Single media only (Matrix): Send only first media URL with text

Critical Issue Found:
Four adapters (Feishu, Matrix, Tlon, Twitch) have a bug in the text-only fallback path. When no media URLs are present, they call sendText({ ...ctx }) but ctx.payload.text is not extracted to the top-level text parameter that sendText expects. This will cause text-only messages to be sent without content.

The three adapters that don't support native media (IRC, Nextcloud-Talk, Nostr) correctly extract the text when calling sendText({ ...ctx, text: combined }).

Confidence Score: 1/5

  • Critical bug will break text-only message delivery in 4 adapters
  • The text extraction bug in Feishu, Matrix, Tlon, and Twitch will cause text-only messages to fail or be sent empty. This is a logical error that will affect core functionality when the delivery pipeline activates.
  • extensions/feishu/src/outbound.ts, extensions/matrix/src/outbound.ts, extensions/tlon/src/channel.ts, and extensions/twitch/src/outbound.ts all need the text extraction fix before merge

Last reviewed commit: d2320ce

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

}
return lastResult;
}
return feishuOutbound.sendText!({ ...ctx });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendText expects text parameter but ctx has text in ctx.payload.text, not at top level

Suggested change
return feishuOutbound.sendText!({ ...ctx });
return feishuOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/outbound.ts
Line: 117

Comment:
`sendText` expects `text` parameter but `ctx` has text in `ctx.payload.text`, not at top level

```suggestion
    return feishuOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });
```

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. The text-only fallback now explicitly passes text: ctx.payload.text ?? "".

Note: ChannelOutboundPayloadContext extends ChannelOutboundContext (which has text: string), and the delivery layer sets ctx.text = payload.text ?? "" when constructing the context. So this was not strictly a runtime bug — ctx.text was already populated. However, making it explicit is more defensive and self-documenting, so applied across all adapters.

mediaUrl: urls[0],
});
}
return matrixOutbound.sendText!({ ...ctx });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendText expects text parameter but ctx has text in ctx.payload.text, not at top level

Suggested change
return matrixOutbound.sendText!({ ...ctx });
return matrixOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/matrix/src/outbound.ts
Line: 24

Comment:
`sendText` expects `text` parameter but `ctx` has text in `ctx.payload.text`, not at top level

```suggestion
    return matrixOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });
```

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. The text-only fallback now explicitly passes text: ctx.payload.text ?? "".

Note: ChannelOutboundPayloadContext extends ChannelOutboundContext (which has text: string), and the delivery layer sets ctx.text = payload.text ?? "" when constructing the context. So this was not strictly a runtime bug — ctx.text was already populated. However, making it explicit is more defensive and self-documenting, so applied across all adapters.

}
return lastResult;
}
return tlonOutbound.sendText!({ ...ctx });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendText expects text parameter but ctx has text in ctx.payload.text, not at top level

Suggested change
return tlonOutbound.sendText!({ ...ctx });
return tlonOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/tlon/src/channel.ts
Line: 126

Comment:
`sendText` expects `text` parameter but `ctx` has text in `ctx.payload.text`, not at top level

```suggestion
    return tlonOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });
```

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. The text-only fallback now explicitly passes text: ctx.payload.text ?? "".

Note: ChannelOutboundPayloadContext extends ChannelOutboundContext (which has text: string), and the delivery layer sets ctx.text = payload.text ?? "" when constructing the context. So this was not strictly a runtime bug — ctx.text was already populated. However, making it explicit is more defensive and self-documenting, so applied across all adapters.

}
return lastResult;
}
return twitchOutbound.sendText!({ ...ctx });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sendText expects text parameter but ctx has text in ctx.payload.text, not at top level

Suggested change
return twitchOutbound.sendText!({ ...ctx });
return twitchOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/twitch/src/outbound.ts
Line: 129

Comment:
`sendText` expects `text` parameter but `ctx` has text in `ctx.payload.text`, not at top level

```suggestion
    return twitchOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });
```

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in de8d324. The text-only fallback now explicitly passes text: ctx.payload.text ?? "".

Note: ChannelOutboundPayloadContext extends ChannelOutboundContext (which has text: string), and the delivery layer sets ctx.text = payload.text ?? "" when constructing the context. So this was not strictly a runtime bug — ctx.text was already populated. However, making it explicit is more defensive and self-documenting, so applied across all adapters.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de8d324772

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}
return lastResult;
}
return matrixOutbound.sendText!({ ...ctx, text: ctx.payload.text ?? "" });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip empty Matrix payloads before delegating to sendText

When channelData is present, outbound delivery uses sendPayload, and this branch always calls sendText with ctx.payload.text ?? "" when there are no media URLs. For channelData-only payloads (metadata with no text/media), that passes an empty message into Matrix send, which throws Matrix send requires text or media, so these payloads now fail instead of being safely skipped.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 23e37cd. Empty payloads (no text, no media) now return a no-op result. Test coverage added.

}
return lastResult;
}
return nextcloudTalkPlugin.outbound!.sendText!({ ...ctx, text: ctx.payload.text ?? "" });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip empty Nextcloud payloads before calling sendText

This fallback always invokes sendText with ctx.payload.text ?? "" whenever channelData exists and no media URLs are present. In the channelData-only case, sendMessageNextcloudTalk rejects empty text (Message must be non-empty for Nextcloud Talk sends), so metadata-only payloads now become hard delivery failures when sendPayload is selected.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 23e37cd. Same empty-payload guard as Matrix. Test coverage added.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23e37cd016

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +135 to +139
let lastResult: Awaited<ReturnType<NonNullable<typeof twitchOutbound.sendText>>>;
for (const chunk of chunks) {
lastResult = await twitchOutbound.sendText!({ ...ctx, text: chunk });
}
return lastResult!;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Handle empty chunk output in Twitch sendPayload

When channelData is present, outbound delivery now routes through sendPayload, but chunkTextForTwitch can legitimately return an empty array for inputs that strip to empty text (for example whitespace-only text or markdown-only image/link content). In that case this loop never sets lastResult, so return lastResult! yields undefined; the caller then reads delivery.messageId and throws, turning what should be a no-op/skip into a hard send failure.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in db3035a. Added if (!chunks.length) return { channel: "twitch", messageId: "" } guard to all adapters' sendPayload text-only path. This handles the edge case where the chunker returns an empty array (e.g., whitespace-only input to chunkTextForTwitch). Test added: "returns no-op when chunker produces empty array".

Comment on lines +36 to +37
const limit = matrixOutbound.textChunkLimit;
const chunks = limit && matrixOutbound.chunker ? matrixOutbound.chunker(text, limit) : [text];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor Matrix textChunkLimit overrides in sendPayload

This chunking branch hard-codes matrixOutbound.textChunkLimit and bypasses the config-driven limit resolution used by the normal outbound path. As a result, channelData payloads ignore channels.matrix.textChunkLimit (including account-level overrides), so messages that previously split at the configured size can now be sent as larger chunks and hit Matrix-side size constraints.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same design as all other adapters: sendPayload uses the adapter's own textChunkLimit/chunker as conservative defaults. Config-level overrides are resolved by the delivery layer in deliver.ts. The adapter provides safe chunking at its declared limit; the delivery layer can apply stricter per-account limits before sendPayload is even called.

@nohat nohat force-pushed the lifecycle/adapter-sendpayload-batch-c-v2 branch from 23e37cd to db3035a Compare March 2, 2026 17:41
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: db3035adba

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +121 to +122
const limit = feishuOutbound.textChunkLimit;
const chunks = limit && feishuOutbound.chunker ? feishuOutbound.chunker(text, limit) : [text];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor Feishu chunk config in sendPayload

When channelData is present, delivery takes sendPayload, and this branch chunks using the adapter constant instead of config-resolved settings. Feishu supports textChunkLimit/chunkMode overrides (including per-account), so payloads with channel metadata will ignore those overrides and be split differently from the normal outbound path; in accounts with stricter limits, this can produce oversized sends that the operator explicitly configured around.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By design. The adapter's sendPayload uses its declared textChunkLimit (4000) as a safe default. Per-account config overrides are the delivery layer's responsibility in deliver.ts, which resolves textChunkLimit/chunkMode from config before calling the adapter. The adapter chunking is defense-in-depth, not a replacement for config resolution.

Comment on lines +326 to +327
const limit = outbound.textChunkLimit;
const chunks = limit && outbound.chunker ? outbound.chunker(text, limit) : [text];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Respect IRC runtime chunk settings in sendPayload

This sendPayload chunking path bypasses the config-aware chunk resolution used in core delivery and always uses outbound.textChunkLimit plus raw outbound.chunker. IRC config includes runtime textChunkLimit/chunkMode overrides, so any message routed through sendPayload (i.e., with channelData) ignores those settings and can be chunked larger/differently than configured.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same design pattern. The adapter uses its own textChunkLimit (350) and chunker as conservative defaults — 350 is the IRC standard safe limit. Config-level overrides are resolved by the delivery layer. The adapter provides safe fallback chunking; operators who configure stricter limits get those applied by deliver.ts before sendPayload is called.

Comment on lines +292 to +293
const limit = outbound.textChunkLimit;
const chunks = limit && outbound.chunker ? outbound.chunker(text, limit) : [text];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply Nextcloud Talk chunk overrides in sendPayload

For channelData payloads this code chunks with the adapter defaults rather than resolved channel/account chunk settings. Nextcloud Talk exposes textChunkLimit and chunkMode in runtime config, so this path silently ignores operator overrides and may send larger or differently split chunks than the standard outbound flow for the same account.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same reasoning as IRC, Feishu, and all other adapters. sendPayload uses the adapter's declared textChunkLimit (4000) as a conservative default. Operator-configured overrides are resolved by deliver.ts before calling the adapter.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 11df424053

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +330 to +331
for (const chunk of chunks) {
lastResult = await outbound.sendText!({ ...ctx, text: chunk });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check abort state between sendPayload chunk sends

When channelData is present, deliverOutboundPayloads calls handler.sendPayload once (src/infra/outbound/deliver.ts), so cancellation is only checked before entering adapter code; this new chunk loop then keeps sending all remaining chunks even after the delivery is aborted. Before this commit, these IRC messages were chunked in core delivery (which checks abort before each send), so long channelData payloads can now continue posting unwanted follow-up messages after timeout/cancel.

Useful? React with 👍 / 👎.

@nohat nohat force-pushed the lifecycle/adapter-sendpayload-batch-c-v2 branch from 11df424 to 7630378 Compare March 5, 2026 16:37
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76303786a2

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +27 to +31
for (let i = 1; i < urls.length; i++) {
lastResult = await matrixOutbound.sendMedia!({
...ctx,
text: "",
mediaUrl: urls[i],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop sendPayload loops on aborted deliveries

If a Matrix delivery is cancelled after the first send (for example, a channelData payload with multiple mediaUrls or multiple text chunks), this loop continues sending remaining events because sendPayload never re-checks abort state between iterations. Prior to this change, these channelData payloads were handled by the core loop in src/infra/outbound/deliver.ts, which calls throwIfAborted before each send; moving iteration into adapter code without an equivalent guard can produce extra outbound messages after a user-initiated cancel/retry.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: feishu Channel integration: feishu channel: irc channel: matrix Channel integration: matrix channel: nextcloud-talk Channel integration: nextcloud-talk channel: nostr Channel integration: nostr channel: tlon Channel integration: tlon channel: twitch Channel integration: twitch size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants