Skip to content

feat(delivery): add @agentworkforce/delivery — unified multi-target messaging#250

Merged
khaliqgant merged 5 commits into
mainfrom
feat/delivery-package
Jun 23, 2026
Merged

feat(delivery): add @agentworkforce/delivery — unified multi-target messaging#250
khaliqgant merged 5 commits into
mainfrom
feat/delivery-package

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jun 23, 2026

Copy link
Copy Markdown
Member

Summary

New @agentworkforce/delivery package that provides a configuration-driven, unified messaging client for agent handlers.

Why

  • Duplication: 4 agent pairs exist as separate Slack/Telegram directories with identical business logic
  • No shared Slack transport: shared/telegram.ts exists but Slack equivalents are copy-pasted across agents
  • Feature drift: Telegram variants have improvements (fetch timeout, pending recovery, fallbacks) Slack variants lack
  • No cross-transport memory sharing: Memory tags differ between variants

API

import { createDelivery, input, list, withTimeout, fetchWithTimeout } from '@agentworkforce/delivery';

const delivery = createDelivery(ctx);

// Blocking mode (waits for receipt)
const heads = await delivery.send(header);
await delivery.send(body, { replyTo: heads });

// Non-blocking parentRef mode (x-reply-radar pattern, zero receipt round-trips)
const heads = await delivery.publish(header);
await delivery.send(body, { replyTo: heads, nonBlocking: true });

Shared helpers

Exports input(), list(), withTimeout(), fetchWithTimeout() — previously copy-pasted across 10+ agent files.

Review in cubic

…essaging

Provides a configuration-driven delivery client that auto-discovers
configured transports (Slack, Telegram) from persona inputs and sends
to all of them through a single API. Supports non-blocking parentRef
threading (zero receipt round-trips, cloud-side ordering) for the
header+threaded-body pattern from x-reply-radar.

Exports shared helpers (input, list, withTimeout, fetchWithTimeout)
that were previously copy-pasted across every agent.

- createDelivery(ctx) — auto-detects SLACK_CHANNEL/TELEGRAM_CHAT
- delivery.send(text, opts?) — blocking mode (waits for receipt)
- delivery.publish(text) — non-blocking parentRef mode
- delivery.send(text, { replyTo, nonBlocking }) — threaded non-blocking
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@khaliqgant, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 52 minutes and 3 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b99c11c4-1615-43c5-9564-5a35d066ed7e

📥 Commits

Reviewing files that changed from the base of the PR and between 84a4c92 and 4f609e1.

📒 Files selected for processing (2)
  • packages/delivery/package.json
  • packages/delivery/src/delivery.ts
📝 Walkthrough

Walkthrough

Introduces a new @agentworkforce/delivery package that discovers configured Slack and Telegram targets from WorkforceCtx and dispatches messages to all applicable transports. Includes public type contracts, persona-input utility helpers, a blocking/non-blocking delivery client factory, and package scaffolding.

Changes

New @agentworkforce/delivery Package

Layer / File(s) Summary
Package scaffolding and entrypoint
packages/delivery/package.json, packages/delivery/tsconfig.json, packages/delivery/src/index.ts
Adds the ESM package manifest with dist-based entrypoints, publish metadata, declared dependencies on @agentworkforce/runtime and @relayfile/relay-helpers, TypeScript build config, and the public re-export entrypoint.
Public types and provider-discovery helpers
packages/delivery/src/types.ts
Defines SlackRef, TelegramRef, MessageRef union, DeliveryResult, DeliveryOptions (with replyTo threading and nonBlocking mode), DeliveryTransports seam, and the DeliveryClient interface; adds resolveDeliveryTargets, slackChannel, and telegramChat helpers reading persona inputs.
Utility helpers
packages/delivery/src/helpers.ts
Adds input() for precedence-based persona input resolution (from resolved inputs, environment, or spec default), list() for comma-separated string splitting, withTimeout() for Promise.race-based timeout rejection, and fetchWithTimeout() for AbortController-backed fetch with safe error swallowing and signal combination.
Delivery client factory and dispatch
packages/delivery/src/delivery.ts
Implements createDelivery factory that instantiates blocking/non-blocking clients per provider; DeliveryClientImpl.send concurrently dispatches to all targets with reply threading by provider and error aggregation; publish delegates to non-blocking send; per-provider methods handle Slack post/messages.write and Telegram blocking/non-blocking paths with ref normalization and receipt validation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 Hop hop, a new package arrives today,
Slack and Telegram get their messages sent away.
Blocking, non-blocking, with timeouts in race,
Refs collected neatly, each target in place.
The rabbit delivers—no carrier pigeon needed! 📦

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: introducing a new @agentworkforce/delivery package for unified multi-target messaging, which matches the changeset's core purpose.
Description check ✅ Passed The description is well-detailed and directly related to the changeset, explaining the motivation (code duplication), API usage, and shared helpers provided by the new package.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/delivery-package

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new '@agentworkforce/delivery' package that provides a delivery client for sending and publishing messages to Slack and Telegram in both blocking and non-blocking modes. Feedback on the implementation highlights several key areas for improvement: ensuring non-blocking clients respect custom transports during testing, handling empty delivery targets consistently in the 'send' method, preventing valid falsy values (like '0' or 'false') from being incorrectly resolved as 'undefined' in the 'input' helper, preserving caller-provided abort signals in 'fetchWithTimeout', and using the 'workspace:*' specifier for internal workspace dependencies in 'package.json'.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/delivery/src/delivery.ts Outdated
Comment on lines +38 to +49
const slackBlocking = transports?.slack ?? (targets.includes('slack')
? slackClient({ writebackTimeoutMs: WRITEBACK_TIMEOUT_MS })
: undefined);
const slackNonBlocking = targets.includes('slack')
? slackClient({ writebackTimeoutMs: 0 })
: undefined;
const telegramBlocking = transports?.telegram ?? (targets.includes('telegram')
? telegramClient({ writebackTimeoutMs: WRITEBACK_TIMEOUT_MS })
: undefined);
const telegramNonBlocking = targets.includes('telegram')
? telegramClient({ writebackTimeoutMs: 0 })
: undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

When custom transports are provided (e.g., mock clients in unit tests), the non-blocking clients (slackNonBlocking and telegramNonBlocking) ignore them and instantiate real clients via slackClient() and telegramClient(). This bypasses test isolation and can cause tests to fail or attempt real network calls.

We should fall back to transports?.slack and transports?.telegram for the non-blocking clients as well.

Suggested change
const slackBlocking = transports?.slack ?? (targets.includes('slack')
? slackClient({ writebackTimeoutMs: WRITEBACK_TIMEOUT_MS })
: undefined);
const slackNonBlocking = targets.includes('slack')
? slackClient({ writebackTimeoutMs: 0 })
: undefined;
const telegramBlocking = transports?.telegram ?? (targets.includes('telegram')
? telegramClient({ writebackTimeoutMs: WRITEBACK_TIMEOUT_MS })
: undefined);
const telegramNonBlocking = targets.includes('telegram')
? telegramClient({ writebackTimeoutMs: 0 })
: undefined;
const slackBlocking = transports?.slack ?? (targets.includes('slack')
? slackClient({ writebackTimeoutMs: WRITEBACK_TIMEOUT_MS })
: undefined);
const slackNonBlocking = transports?.slack ?? (targets.includes('slack')
? slackClient({ writebackTimeoutMs: 0 })
: undefined);
const telegramBlocking = transports?.telegram ?? (targets.includes('telegram')
? telegramClient({ writebackTimeoutMs: WRITEBACK_TIMEOUT_MS })
: undefined);
const telegramNonBlocking = transports?.telegram ?? (targets.includes('telegram')
? telegramClient({ writebackTimeoutMs: 0 })
: undefined);

Comment thread packages/delivery/src/helpers.ts Outdated
Comment on lines +11 to +12
const v = process.env[spec?.env ?? name] ?? ctx.persona.inputs?.[name] ?? spec?.default;
return v && String(v).trim() ? String(v).trim() : undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The input helper's resolution logic uses v && String(v).trim() to check for valid values. If the resolved value v is a valid falsy JSON primitive like 0 or false (e.g., from spec.default), this check will evaluate to false / 0 and return undefined instead of the stringified value ("0" or "false").

We should explicitly check for null and undefined instead.

  const v = process.env[spec?.env ?? name] ?? ctx.persona.inputs?.[name] ?? spec?.default;
  if (v === undefined || v === null) return undefined;
  const trimmed = String(v).trim();
  return trimmed !== '' ? trimmed : undefined;

Comment on lines +82 to +83
async send(text: string, opts?: DeliveryOptions): Promise<DeliveryResult> {
const nonBlocking = opts?.nonBlocking === true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If this.targets is empty (i.e., neither Slack nor Telegram is configured), the send method behaves inconsistently:

  • In blocking mode (nonBlocking = false), it returns { ok: true, refs: [] }.
  • In non-blocking mode (nonBlocking = true), it throws an error: Delivery failed to all targets: (with an empty error list) because refs.length > 0 is false.

We should check if this.targets.length === 0 at the beginning of send and throw a clear, consistent error.

Suggested change
async send(text: string, opts?: DeliveryOptions): Promise<DeliveryResult> {
const nonBlocking = opts?.nonBlocking === true;
async send(text: string, opts?: DeliveryOptions): Promise<DeliveryResult> {
if (this.targets.length === 0) {
throw new Error('No delivery targets configured');
}
const nonBlocking = opts?.nonBlocking === true;

Comment on lines +46 to +60
export async function fetchWithTimeout(
url: string,
init: RequestInit = {},
timeoutMs: number = 8_000
): Promise<Response | undefined> {
const controller = new AbortController();
const timer = setTimeout(() => controller.abort(), timeoutMs);
try {
return await fetch(url, { ...init, signal: controller.signal });
} catch {
return undefined;
} finally {
clearTimeout(timer);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

In fetchWithTimeout, if the caller passes a custom AbortSignal in init.signal, it is completely overwritten and ignored by controller.signal.

To support defensive programming and prevent ignoring caller-driven aborts, we should listen to the caller's signal and abort our internal controller if the caller's signal aborts.

export async function fetchWithTimeout(
  url: string,
  init: RequestInit = {},
  timeoutMs: number = 8_000
): Promise<Response | undefined> {
  const controller = new AbortController();
  const timer = setTimeout(() => controller.abort(), timeoutMs);

  if (init.signal) {
    if (init.signal.aborted) {
      controller.abort();
    } else {
      init.signal.addEventListener('abort', () => controller.abort());
    }
  }

  try {
    return await fetch(url, { ...init, signal: controller.signal });
  } catch {
    return undefined;
  } finally {
    clearTimeout(timer);
  }
}

Comment thread packages/delivery/package.json Outdated
"lint": "tsc -p tsconfig.json --noEmit"
},
"dependencies": {
"@agentworkforce/runtime": "^4.0.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

In a pnpm monorepo, internal workspace dependencies should use the "workspace:*" specifier rather than a hardcoded version like "^4.0.0". This ensures consistency across packages and correct resolution during local development and publishing.

Suggested change
"@agentworkforce/runtime": "^4.0.0",
"@agentworkforce/runtime": "workspace:*",

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

Copy link
Copy Markdown

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: d9055eff36

ℹ️ 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 thread packages/delivery/package.json Outdated
"lint": "tsc -p tsconfig.json --noEmit"
},
"dependencies": {
"@agentworkforce/runtime": "^4.0.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep the delivery manifest in sync with the lockfile

The manifest declares @agentworkforce/runtime as ^4.0.0, but the new packages/delivery importer in pnpm-lock.yaml still records the specifier as workspace:* with link:../runtime. pnpm's frozen-lockfile install validates importer specifiers against each package.json, so CI/fresh installs of this commit will fail before build until the lockfile is regenerated or the manifest uses the workspace specifier.

Useful? React with 👍 / 👎.

return {
provider: 'telegram',
chatId: String(result.chatId),
messageId: result.ok ? result.messageId : ''

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve a Telegram parent ref when publishing non-blocking

In the documented publish(header) then send(body, { replyTo: heads, nonBlocking: true }) flow for Telegram, the 0ms writeback path has no receipt, so result.ok is false and this stores an empty messageId. The next Telegram send only sets replyToMessageId when parentRef.messageId is truthy, so the body is posted as a top-level Telegram message instead of under the header; carry the draft result.ref/parentRef or avoid claiming non-blocking Telegram threading.

Useful? React with 👍 / 👎.

Comment thread packages/delivery/src/helpers.ts Outdated
*/
export function input(ctx: WorkforceCtx, name: string): string | undefined {
const spec = ctx.persona.inputSpecs?.[name];
const v = process.env[spec?.env ?? name] ?? ctx.persona.inputs?.[name] ?? spec?.default;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Do not let env vars override resolved persona inputs

When a deployment supplies an input value and the runner environment also contains the same input/env name, ctx.persona.inputs already holds the runtime's agent-value → env → default resolution. Looking at process.env before ctx.persona.inputs reverses that precedence, so handlers using this helper can silently ignore per-agent/deployment overrides whenever an ambient env var is present.

Useful? React with 👍 / 👎.

@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer could not complete review for #250 in AgentWorkforce/workforce.
The review harness exited with code 1.
No review was posted; this needs operator attention.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/delivery/package.json`:
- Line 35: The `@agentworkforce/runtime` dependency in the delivery package.json
is specified as "^4.0.0" but the workspace lockfile records it as "workspace:*",
causing the frozen lockfile install to fail. Change the version specifier for
`@agentworkforce/runtime` from "^4.0.0" to "workspace:*" to properly reference the
internal workspace dependency and align with the lockfile.

In `@packages/delivery/src/delivery.ts`:
- Around line 111-120: Blocking delivery can fail with an empty or silent result
when all target sends return null and no errors are recorded. Update Delivery in
delivery.ts, especially the logic around the ok check and the
error/partial-failure handling in the delivery method, so no-receipt/null
responses are treated as explicit failures rather than leaving errors empty;
this should make the throw from the blocking path include actionable details
instead of an empty message. Also ensure the partial-result case with
refs.length > 0 but fewer than targets is logged or surfaced consistently, using
the existing ctx.log, errors, refs, and nonBlocking flow.
- Around line 38-49: The transport injection seam is incomplete:
slackNonBlocking and telegramNonBlocking variables do not respect the transports
parameter like their blocking counterparts do, meaning injected test mocks are
bypassed for non-blocking sends. Update both slackNonBlocking (lines 41-43) and
telegramNonBlocking (lines 47-49) to check transports first before constructing
fresh clients, using the same pattern as slackBlocking and telegramBlocking, OR
if non-blocking DI is intentionally unsupported, update the documentation at
lines 26-28 and 40-42 to clarify this. Additionally, in non-blocking mode around
line 228 where messageId is set to an empty string when result.ok is false,
ensure the Telegram messageId is properly captured so that subsequent threaded
sends using parentRef will have a valid messageId to thread on, or document this
threading limitation. Finally, at line 119 where the delivery failure error is
constructed, ensure the error message includes actual error details from failed
sends rather than an empty string when all sendSlack and sendTelegram calls
return null without throwing exceptions.
- Around line 216-228: In the Telegram delivery function, when non-blocking mode
is enabled and sendMessage fails (result.ok is false), the returned messageId is
an empty string. This breaks threading on subsequent messages because
parentRef.messageId becomes empty and the condition on line 217 fails to apply
replyToMessageId. Fix this by either implementing an embedded parentRef
mechanism in the message text (similar to the Slack pattern on lines 186-190) to
preserve threading context without relying on messageId, or add a comment
documenting that Telegram non-blocking mode does not support threading.
Additionally, on line 227 where String(result.chatId) is called, add a check to
ensure result.chatId exists before converting it to a string to prevent
returning the literal string 'undefined'.

In `@packages/delivery/src/helpers.ts`:
- Around line 9-12: The input() function has an incorrect precedence order in
the variable assignment. Currently, the environment variable lookup via
process.env is checked first before the explicit persona input in
ctx.persona.inputs, which contradicts the expected precedence. Reorder the
nullish coalescing chain so that ctx.persona.inputs?.[name] is checked before
process.env[spec?.env ?? name], ensuring explicit persona inputs take higher
priority over environment variables. The spec?.default should remain as the
lowest priority fallback.

In `@packages/delivery/src/types.ts`:
- Around line 92-97: The `resolveDeliveryTargets` function and the related
functions at lines 101-104 and 107-109 are currently reading directly from
`ctx.persona.inputs`, which bypasses environment variable and default value
resolution. Replace the direct property access with the shared `input()`
resolver function that properly handles input resolution from all sources
(inputs, env, and defaults). Apply this change to all the transport discovery
and id extraction helper functions mentioned in the comment to ensure they
correctly respect environment variables and default configurations.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 530a17f5-a4ef-47de-9fc5-c8d86e2514c0

📥 Commits

Reviewing files that changed from the base of the PR and between 26b2f39 and d9055ef.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (6)
  • packages/delivery/package.json
  • packages/delivery/src/delivery.ts
  • packages/delivery/src/helpers.ts
  • packages/delivery/src/index.ts
  • packages/delivery/src/types.ts
  • packages/delivery/tsconfig.json

Comment thread packages/delivery/package.json Outdated
Comment thread packages/delivery/src/delivery.ts Outdated
Comment thread packages/delivery/src/delivery.ts
Comment on lines +216 to +228
const result = parentRef?.messageId
? await client.sendMessage(chatId, text, { replyToMessageId: Number(parentRef.messageId) || undefined })
: await client.sendMessage(chatId, text);

if (!nonBlocking && !result.ok) {
this.ctx.log?.('warn', 'delivery.telegram.no-receipt', { chatId });
return null;
}

return {
provider: 'telegram',
chatId: String(result.chatId),
messageId: result.ok ? result.messageId : ''

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP -C4 '\bsendMessage\b|replyToMessageId|messageId|chatId' $(fd -p 'relay-helpers' -t d node_modules 2>/dev/null) 2>/dev/null

Repository: AgentWorkforce/workforce

Length of output: 162


🏁 Script executed:

# First, locate the delivery.ts file and examine the context around lines 216-228
fd -type f -name "delivery.ts" packages/delivery/src/

Repository: AgentWorkforce/workforce

Length of output: 333


🏁 Script executed:

# Locate the delivery.ts file
fd -t f -name "delivery.ts" packages/delivery/src/

Repository: AgentWorkforce/workforce

Length of output: 393


🏁 Script executed:

# Find the delivery.ts file using find command
find . -name "delivery.ts" -path "*/packages/delivery/src/*"

Repository: AgentWorkforce/workforce

Length of output: 198


🏁 Script executed:

# Read the delivery.ts file to see the context around lines 216-228
cat -n ./packages/delivery/src/delivery.ts | sed -n '200,240p'

Repository: AgentWorkforce/workforce

Length of output: 1296


🏁 Script executed:

# Now let's find where the telegram client is defined/imported
grep -n "telegramBlocking\|telegramNonBlocking\|telegram.*Client" ./packages/delivery/src/delivery.ts | head -20

Repository: AgentWorkforce/workforce

Length of output: 729


🏁 Script executed:

# Find where TelegramClient is defined - likely in relay-helpers
find . -name "*.ts" -o -name "*.d.ts" | xargs grep -l "TelegramClient" | head -5

Repository: AgentWorkforce/workforce

Length of output: 231


🏁 Script executed:

# Check the types.ts file for TelegramClient definition
cat -n ./packages/delivery/src/types.ts

Repository: AgentWorkforce/workforce

Length of output: 4884


🏁 Script executed:

# Search for relay-helpers to understand what sendMessage returns
find . -name "relay-helpers" -o -path "*/node_modules/@relayfile/relay-helpers*" 2>/dev/null | head -5

Repository: AgentWorkforce/workforce

Length of output: 162


🏁 Script executed:

# Search for test files or usage of sendMessage in the codebase
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "sendMessage\|sendTelegram" 2>/dev/null | head -5

Repository: AgentWorkforce/workforce

Length of output: 162


🏁 Script executed:

# Let's look at the Slack implementation for comparison
cat -n ./packages/delivery/src/delivery.ts | sed -n '140,200p'

Repository: AgentWorkforce/workforce

Length of output: 2470


🏁 Script executed:

# Let's see the full send method to understand the flow
cat -n ./packages/delivery/src/delivery.ts | sed -n '100,145p'

Repository: AgentWorkforce/workforce

Length of output: 1871


🏁 Script executed:

# Check package.json to see relay-helpers version/repo
find . -name "package.json" -path "*/packages/delivery/*" -exec cat {} \;

Repository: AgentWorkforce/workforce

Length of output: 1112


🏁 Script executed:

# Search for relay-helpers documentation or type definitions in node_modules or package info
grep -r "interface.*TelegramClient\|sendMessage.*return\|type.*sendMessage" . --include="*.ts" --include="*.d.ts" 2>/dev/null | head -20

Repository: AgentWorkforce/workforce

Length of output: 162


🏁 Script executed:

# Let's search for comments or documentation about sendMessage behavior in non-blocking mode
grep -n "writebackTimeoutMs\|sendMessage\|non-blocking\|nonBlocking" ./packages/delivery/src/delivery.ts

Repository: AgentWorkforce/workforce

Length of output: 1558


🌐 Web query:

relay-helpers telegram client sendMessage non-blocking writebackTimeoutMs documentation

💡 Result:

There is no standardized Telegram client API, library, or documentation that defines parameters named "relay-helpers," "sendMessage," or "writebackTimeoutMs" as a unified, interconnected feature set [1][2][3][4][5][6][7]. Based on current technical documentation and repository patterns: 1. Relay Helpers: The term "relay" often refers to custom, project-specific middleware or wrappers designed to forward messages between systems (e.g., Claude Code, local notification services, or HTTP-to-Telegram bridges) [1][3][4][8]. These are typically bespoke implementations rather than a shared, standardized library. 2. writebackTimeoutMs: This parameter does not exist in standard Telegram Bot API documentation [4][9][10]. It is likely a custom configuration option within a specific, private, or project-specific relay implementation intended to manage asynchronous write operations or response timeouts for bridged messages [1][3]. 3. Non-blocking sendMessage: In the context of Telegram clients (such as Telethon or python-telegram-bot), "non-blocking" behavior is typically achieved by using native asynchronous programming patterns (e.g., async/await in Python or JavaScript) rather than a specific parameter named "non-blocking" [5][7][11]. If you are encountering this in a specific codebase, it is likely a proprietary wrapper created to handle rate limits, retries, or asynchronous message queuing specific to that application [2][3]. You should consult the README.md or local docs/ folder within that specific project repository for documentation regarding its internal configuration [1][7].

Citations:


🏁 Script executed:

# Let's look for any TypeScript type definitions or interfaces related to sendMessage result
grep -r "result\.ok\|result\.messageId\|result\.chatId" ./packages/delivery/src/ --include="*.ts"

Repository: AgentWorkforce/workforce

Length of output: 396


Telegram non-blocking threading is broken; parentRef threading relies on the previous messageId which is empty.

In non-blocking mode, sendMessage returns messageId: '' (line 228, when result.ok is false). On the next threaded send, parentRef?.messageId is '' (falsy), so line 217 skips the threading call—Slack avoids this by embedding parentRef in the message body for cloud-side ordering (lines 186–190), but Telegram has no equivalent. The documented behavior (types.ts lines 38–46) claims Telegram supports cloud-side ordering with writebackTimeoutMs: 0, but the implementation only applies this to Slack.

Fix by either:

  1. Implementing an embedded parentRef mechanism for Telegram (mirroring Slack's pattern), or
  2. Documenting that Telegram non-blocking mode does not support threading.

Additionally, String(result.chatId) on line 227 yields 'undefined' if chatId is missing from the result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/delivery/src/delivery.ts` around lines 216 - 228, In the Telegram
delivery function, when non-blocking mode is enabled and sendMessage fails
(result.ok is false), the returned messageId is an empty string. This breaks
threading on subsequent messages because parentRef.messageId becomes empty and
the condition on line 217 fails to apply replyToMessageId. Fix this by either
implementing an embedded parentRef mechanism in the message text (similar to the
Slack pattern on lines 186-190) to preserve threading context without relying on
messageId, or add a comment documenting that Telegram non-blocking mode does not
support threading. Additionally, on line 227 where String(result.chatId) is
called, add a check to ensure result.chatId exists before converting it to a
string to prevent returning the literal string 'undefined'.

Comment thread packages/delivery/src/helpers.ts Outdated
Comment thread packages/delivery/src/types.ts
@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer could not complete review for #250 in AgentWorkforce/workforce.
The review harness exited with code 1.
No review was posted; this needs operator attention.

agent-relay-code Bot added a commit that referenced this pull request Jun 23, 2026
@agent-relay-code

Copy link
Copy Markdown
Contributor

Let me note logic observations. A few behavioral points worth flagging as review comments (not auto-fixing, since they're semantic/judgment):

  1. delivery.ts:166draftRef: result.ref in blocking path. In 0.4.2 post returns ref, so this is correct.
  2. delivery.ts:113 — blocking ok requires refs.length === this.targets.length, but sendSlack/sendTelegram can legitimately return null (no channel configured) which decrements ref count → ok=false even with no errors. This is a behavioral nuance, advisory only.
  3. helpers.ts withTimeout/fetchWithTimeout and input/list are exported but unused internally — dead-ish but part of public API.

None of these are CI-breaking or safety-critical (no lifecycle/reaper/dispatch code). I won't change logic. Let me write up the review.

The review is complete. Here's my summary.


PR #250 Review — feat(delivery): add @agentworkforce/delivery

This PR adds a new, self-contained package @agentworkforce/delivery (unified Slack/Telegram messaging over @relayfile/relay-helpers). Nothing outside the new package is touched except the additive pnpm-lock.yaml entries. I traced the new code against its two dependencies and ran the full CI pipeline.

CI-breaking issue found and fixed (mechanical)

packages/delivery/package.json dependency specifier mismatch — broke pnpm install --frozen-lockfile (the first CI step).

  • The manifest declared "@agentworkforce/runtime": "^4.0.0", but pnpm-lock.yaml records it as specifier: workspace:* / version: link:../runtime. CI runs pnpm install --frozen-lockfile, which aborts with ERR_PNPM_OUTDATED_LOCKFILE on this mismatch — the build never starts.
  • Every other workspace package (cli, deploy, mcp-workforce) declares the runtime dep as "workspace:*". The committed lockfile was already generated against workspace:*.
  • Fix: changed ^4.0.0workspace:* to match the lockfile and repo convention. This is a non-semantic specifier alignment; it does not change resolved versions or runtime behavior. Fixed in packages/delivery/package.json:35. The lockfile required no change.

Verification (ran the way CI does, with the fix in place)

  • pnpm install --frozen-lockfile → passes (was failing before the fix)
  • pnpm -r run build → all 16 packages build, incl. packages/delivery
  • pnpm run lint → passes
  • pnpm run typecheck + typecheck:examples → passes
  • pnpm run test → passes (cli 279/279, etc.; delivery has no tests, exits 0)

I confirmed the relay-helpers API usage against the 0.4.2 version this PR pins (the stale 0.3.41 previously in node_modules lacks Telegram support and the 3-arg post). Under 0.4.2 all usages are valid: SlackClient.post(channel, text, { replyTo }){ channel, ts, ref }; slack.messages.write(...){ path }; telegramClient/TelegramClient.sendMessage(chatId, text, { replyToMessageId }){ ok, chatId, messageId, ref }. WorkforceCtx.persona.inputs and ctx.log(level, msg, attrs) also match.

Addressed comments

  • No bot or human review comments are present in the harness inputs (.workforce/context.json carries no review metadata, and there are no comment files). Nothing to reconcile.

Advisory Notes

These are observations for the author's judgment — I did not change any logic:

  • delivery.ts:111-113 (blocking ok): success requires refs.length === this.targets.length, but sendSlack/sendTelegram can return null for a legitimately-unconfigured channel (e.g. slackChannel(ctx) empty while 'slack' is in targets). That yields ok=false with zero errors. Likely benign given resolveDeliveryTargets keys off the same inputs, but the two resolution paths (resolveDeliveryTargets vs slackChannel/telegramChat) could drift. Consider asserting they stay in sync, or basing ok on attempted-vs-errored rather than targets.length.
  • helpers.ts: input, list, withTimeout, fetchWithTimeout are exported in the public API but unused within the package. Fine if intended as public utilities; otherwise they add surface area.
  • These are out of scope for this PR's "add the package" purpose and would be a follow-up decision, not a change to fold in here.

The only file I modified is packages/delivery/package.json (one-line specifier fix), and it is verified by the full build/lint/typecheck/test run above.

@agent-relay-code

Copy link
Copy Markdown
Contributor

pr-reviewer could not complete review for #250 in AgentWorkforce/workforce.
The review harness exited with code 1.
No review was posted; this needs operator attention.

@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

Review: PR #250@agentworkforce/delivery (unified multi-target messaging)

Summary

This PR adds a new, self-contained package @agentworkforce/delivery (5 source files + package.json + tsconfig) plus the corresponding pnpm-lock.yaml entries for @relayfile/relay-helpers@0.4.2 (and its transitive adapter-core/adapter-linear). It is a library with no in-repo consumers — nothing else in the workspace imports @agentworkforce/delivery, so blast radius is limited to this package and the lockfile.

Verification (ran the way CI does)

CI (.github/workflows/ci.yml) runs: pnpm install --frozen-lockfilepnpm -r buildpnpm run lintpnpm run typecheckpnpm run test. I reproduced all of it end-to-end:

  • pnpm install --frozen-lockfile — clean (lockfile up to date)
  • pnpm -r build — all 16 projects built, including packages/delivery build: Done
  • pnpm run lint — pass
  • pnpm run typecheck (incl. typecheck:examples) — pass
  • pnpm run test — pass (279 CLI tests + others; delivery has 0 tests, exits 0)

I verified the package's API usage against the actually-installed @relayfile/relay-helpers@0.4.2 types (not a stale parent node_modules that contained 0.3.41):

  • SlackClient.post(channel, text, {replyTo}){channel, ts, ref}
  • SlackClient.messages.write(params, body)WritebackResult with .path
  • telegramClient / TelegramClient.sendMessage(chatId, text, {replyToMessageId})TelegramMessageResult {ok, chatId, messageId, ref} ✓ (present in 0.4.2)
  • WorkforceCtx.persona.inputs/inputSpecs and ctx.log(level,msg,attrs) match helpers.ts/types.ts usage ✓

The node --test dist/**/*.test.js glob with no test files exits 0 under sh (node expands the globs), so it is CI-safe.

Addressed comments

  • Prior bot pass (commit 0082cd9 "chore: apply pr-reviewer fixes for feat(delivery): add @agentworkforce/delivery — unified multi-target messaging #250") — Already merged into the head checkout I reviewed; I validated the current tree, which builds/lints/typechecks/tests green. No outstanding regressions from it.
  • No live human reviews or bot review threads were retrievablegh CLI is not available in this environment and .workforce/context.json carries no review/comment payload, so I could not enumerate individual threads. I reviewed the full diff directly against the current checkout instead; no unaddressed actionable finding was found.

Findings

No mechanical (lint/format/typo/import-order) issues to auto-fix — none were present. No source files were edited; working tree is clean.

Logic observations (all acceptable as written, no change made — flagging for human awareness only):

  • send() blocking success is errors.length === 0 && refs.length === targets.length; a target that returns null without throwing (e.g. unconfigured channel, missing receipt) yields {ok:false, refs} without throwing. This is fail-visible (caller must check ok) — correct, not fail-open.
  • helpers.ts input/list are exported but unused in-package; they are public API for downstream persona consumers, so this is intentional.

Advisory Notes

  • None. The PR is in scope (a new isolated package) and I did not fold in any unrelated changes.

The PR builds, lints, typechecks, and tests green locally across the full workspace. I cannot confirm live GitHub CI status or mergeability/conflict state from this environment (no gh/network to GitHub), so whether every required check has completed and passed on the PR and whether GitHub reports it mergeable is unverified here. Because I cannot confirm those gating conditions, I am not printing READY.

…ction

- Fix input() precedence: ctx.persona.inputs before process.env (agent value >
  ctx resolved > env > default). PR feedback caught that the original put
  process.env before ctx.persona.inputs.
- Fix non-blocking transports to respect injected mocks. Previously
  slackNonBlocking/telegramNonBlocking always constructed real clients;
  now they check transports?.slack/telegram first (same as blocking).
- Fix resolveDeliveryTargets/slackChannel/telegramChat to use input()
  helper for proper resolution order instead of direct ctx.persona.inputs
  access.
- Fix fetchWithTimeout to preserve caller-provided abort signal by
  racing our abort timer against the caller's signal.
@khaliqgant khaliqgant force-pushed the feat/delivery-package branch from 0082cd9 to 84a4c92 Compare June 23, 2026 17:02
Ricky Schema Cascade added 3 commits June 23, 2026 19:51
@khaliqgant khaliqgant merged commit e923926 into main Jun 23, 2026
3 checks passed
@khaliqgant khaliqgant deleted the feat/delivery-package branch June 23, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant