Refactor Telegram turn handling for direct DO execution and faster follow-ups#104
Closed
punitarani wants to merge 5 commits intomainfrom
Closed
Refactor Telegram turn handling for direct DO execution and faster follow-ups#104punitarani wants to merge 5 commits intomainfrom
punitarani wants to merge 5 commits intomainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Resolve conflict in packages/env/src/workers.ts: remove TELEGRAM_QUEUE and TELEGRAM_DLQ bindings (queue ingress replaced by direct DO path) and deduplicate CLOUDFLARE_AI_GATEWAY_ID / ATTACHMENTS_BUCKET declarations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ConversationSession, and workflowTesting
bun run --filter @amby/api testbun run --filter @amby/api typecheckbun run --filter @amby/channels testGreptile Summary
This PR replaces the Cloudflare Queue-based Telegram ingress with a direct webhook→Chat SDK→ConversationSession DO path, and introduces a substantial new turn-management architecture: adaptive debounce deadlines, execution tokens for atomic first-outbound claiming, supersession detection for pre-reply corrections, and a
TelegramDeliveryControllerabstraction that gates all outbound Telegram messages behind a DO-verified claim. The agent-execution workflow is also refactored to use the delivery controller and to explicitly avoid retrying the agent-loop step (preventing duplicate user-visible messages).Key changes:
consumer.tsdeleted,wrangler.tomlqueue bindings removed) — messages now ingested synchronously at webhook time viachat-sdk.tsconversation-session-state.ts(new) — pure state-machine helpers for debounce scheduling, supersession, execution tokens, and outbound claiming, with good unit test coverageconversation-session.ts— DO updated to use new state helpers; addsclaimFirstOutboundRPC; P1 bug:completeExecutionsets state to"debouncing"on rerun but never callsthis.ctx.storage.setAlarm(), so follow-up messages queued during a running workflow are silently lost when the workflow finishestelegram-delivery.ts(new) — delivery controller with suppression, streaming draft management, and first-outbound gating; well-testedagent-execution.ts— retries removed from agent-loop step, execution token threaded through, blocked-user check moved from early webhook path into workflowConfidence Score: 4/5
Not safe to merge until the missing alarm scheduling in completeExecution is fixed; without it, follow-up messages queued during an active workflow are silently dropped.
One P1 bug: completeExecution transitions state to debouncing via completeExecutionState but never calls this.ctx.storage.setAlarm(), so the rerun path is broken. All other findings are P2. The new architecture, state-machine extraction, and test coverage are well-designed; a single targeted fix unblocks merge.
apps/api/src/durable-objects/conversation-session.ts — completeExecution method needs to schedule the Cloudflare alarm when shouldRerun is true
Important Files Changed
Sequence Diagram
sequenceDiagram participant TG as Telegram Webhook participant SDK as Chat SDK participant DO as ConversationSession DO participant WF as AgentExecutionWorkflow participant DLV as TelegramDeliveryController TG->>SDK: incoming message SDK->>DO: ingestMessage(payload) DO->>DO: buffer + scheduleDebounce (setAlarm) alt follow-up arrives while processing TG->>SDK: follow-up message SDK->>DO: ingestMessage(payload) DO->>DO: handleProcessingFollowUpState (may set supersededAt) end DO-->>DO: alarm fires DO->>DO: beginProcessingState with executionToken DO->>WF: create workflow(chatId, messages, executionToken) WF->>DO: claimFirstOutbound(executionToken) DO-->>WF: ClaimFirstOutboundResult WF->>DLV: flushStreamText / finalizeResponse DLV->>TG: post or edit message WF->>DO: completeExecution(executionToken, outcome) DO->>DO: completeExecutionState alt shouldRerun true Note over DO: state set to debouncing but setAlarm not called else shouldRerun false DO->>DO: status = idle endComments Outside Diff (1)
apps/api/src/durable-objects/conversation-session.ts, line 298-307 (link)completeExecutioncompleteExecutionStatecallsscheduleDebounceStatewhenshouldRerunistrue, which setsstate.status = "debouncing"andstate.debounceDeadlineAt— but it only mutates in-memory state. The actual Cloudflare alarm (this.ctx.storage.setAlarm(deadline)) is never scheduled here.This means that when a workflow finishes and there are buffered follow-up messages, the DO sits in
"debouncing"state forever without the alarm ever firing. Those messages are silently dropped.The
alarm()handler also returns early for any non-"debouncing"status, so there's no fallback path.Reviews (1): Last reviewed commit: "Refresh Telegram docs and remove exec pl..." | Re-trigger Greptile