diff --git a/CHANGELOG.md b/CHANGELOG.md index 126bd9b..7097300 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +- `Status Errors`: Failures of the poll loop and of message delivery are classified into a short state with the next step (`invalid token · /telegram-setup`, `rate limited 12s · retrying`, `another poller is active · /telegram-status`, `Telegram unavailable`, `offline · retrying`); unrecognized messages stay verbatim, sanitized and trimmed to 100 characters. A non-idempotent send that hit a transport failure after the request left is reported as `delivery unconfirmed` instead of a definite failure. Impact: the status says what to do instead of showing raw runtime text. +- `Status Persistence`: The poll loop cleared the error status after its retry sleep, before the next attempt, so a slow retry (a 12 s rate-limit wait) showed as `connected`. The reset now happens after the next successful `getUpdates`, and recovery is reported only when `getUpdates` itself had failed, not when an update handler did. Impact: an error stays visible until polling works again. +- `Delivery Counters`: `lib/traffic.ts` counts undelivered messages (`recordFailed`, `onCallFailed`) and the status shows `✘N` next to the sent counter; `/telegram-status` adds `N failed`. `TelegramBridgeApiRuntime` gains `onCallFailed` beside `onCallSucceeded`. +- `Operator Notices`: New leaf-adjacent domain `lib/notices.ts` raises host toasts for owner paired (`onPaired` on the pairing runtime), connection recovered (`onRecovered` on the poll loop) and reply not delivered (throttled to one per 30 s). A failing toast is recorded as a runtime event and never reaches the send, pairing or polling path. +- `Queue Preview`: With turns waiting, the TUI widget adds a second line with the next two summaries in dispatch order (`Queue.formatQueuedTelegramItemsPreview`) and a remainder count; the plain `setStatus` entry keeps one line. +- `Working Message`: While a Telegram-originated turn runs, the omp spinner reads `Answering Telegram · ""` and is restored when the turn settles. `setExtensionWorkingMessage` skips hosts without `ui.setWorkingMessage`, because the text is cosmetic. +- `Widget Sanitization`: Widget lines bypass the host's status sanitizer, so `sanitizeTelegramStatusText` strips control characters and bidi overrides from every Telegram-controlled string (thread name, bot name, queued prompts, error text) before it reaches the terminal. - `Status Line`: The line under the omp editor now shows a state glyph, the bot name, received/sent message counters and the time of the last message (`telegram ● connected · @bot · ⤵12 ⤴11 · ⏱ 14:32`), plus the next step for non-working states (`/telegram-setup`, `send /start to @bot`, `/telegram-connect`). Glyphs follow omp's `symbolPreset`. omp strips ANSI from extension status text, so in the TUI the line is rendered as a `belowEditor` widget that keeps the theme colors; other modes keep `setStatus`. Counters come from the new leaf domain `lib/traffic.ts`: received on every owner message, sent on every successful `sendMessage`/`sendRichMessage`/media call, and `/telegram-status` gains a `- messages:` line. Impact: connection state, traffic and recovery steps are visible without opening `/telegram-status`. - `Pairing`: The poll loop persisted the config object it captured when polling started. The store swaps in a fresh object on every persist, so after the initial sync that stale copy lacked later changes and the next offset commit erased the paired owner: `telegram.json` never kept `allowedUserId`, the bridge re-paired on every message and the status flipped back to `awaiting pairing`. The runner now carries only the poll offset onto the store's current config. Impact: first-time pairing sticks, and another Telegram user can no longer be paired by writing before the owner's next message. - `omp Agent Directory`: Resolve the agent directory from the host's own `getAgentDir()` (registered by the composition root via `setHostAgentDirResolver`) instead of sniffing `execPath`/`argv[1]` for an `omp` prefix. omp installed through Bun runs as `bun …/@oh-my-pi/pi-coding-agent/dist/cli.js`, which matched neither, so with `telegram.json` in `~/.omp/agent` the bridge never connected and wrote its log and bus socket under `~/.pi/agent`. `PI_CODING_AGENT_DIR` still wins; with no host registered the fallback stays `~/.pi/agent`. Impact: a bot token saved by `/telegram-setup` lives in omp's own agent directory and is found again. `lib/paths.ts` stays free of Pi SDK imports, which cost about a second of startup per importing process. diff --git a/README.md b/README.md index 361c3ac..aac9413 100644 --- a/README.md +++ b/README.md @@ -136,15 +136,27 @@ The bridge keeps one live line under the `omp` editor. In the interactive TUI it ```text telegram ● connected · @my_bot · ⤵12 ⤴11 · ⏱ 14:32 -telegram ⟳ active +2 · @my_bot · ⤵14 ⤴11 · ⏱ 14:33 +telegram ⟳ active +3 · @my_bot · ⤵14 ⤴11 · ⏱ 14:33 + next: "second prompt" · "third prompt" · +1 more +telegram ● connected · @my_bot · ⤵12 ⤴10 ✘2 · ⏱ 14:32 telegram ⚠ awaiting pairing · send /start to @my_bot telegram ○ disconnected · /telegram-connect telegram ⦸ not configured · /telegram-setup -telegram ✘ error Unable to connect. Is the computer able to access the url? +telegram ✘ invalid token · /telegram-setup +telegram ⟳ rate limited 12s · retrying +telegram ⟳ offline · retrying +telegram ⚠ another poller is active · /telegram-status +telegram ✘ error ``` -- `⤵N` counts messages received from the paired owner and `⤴N` counts messages the bridge delivered to Telegram (replies, menus and attachments; typing indicators, drafts and edits are not counted). `⏱` is the time of the latest one. Counters live in memory, start at zero when the extension loads, and are shown only while the bridge is up. +- `⤵N` counts messages received from the paired owner and `⤴N` counts messages the bridge delivered to Telegram (replies, menus and attachments; typing indicators, drafts and edits are not counted). `✘N` appears only when some deliveries failed or could not be confirmed. `⏱` is the time of the latest message. Counters live in memory, start at zero when the extension loads, and are shown only while the bridge is up. - The state word is the same ladder as before: `not configured`, `awaiting pairing`, `electing`, `disconnected`, then `active` while a turn runs or is queued, and `connected`, `leader` or `follower` otherwise. +- While turns wait, a second line previews the next two in dispatch order and how many more follow. +- Transport and API failures are classified into a short state with the next step: HTTP 401 (`invalid token`), 429 (`rate limited`), 409 (`another poller is active`), 5xx (`Telegram unavailable`) and network errors (`offline`). Anything unrecognized stays visible verbatim, trimmed to 100 characters. +- An error stays on screen until a poll succeeds again, so a slow retry is never shown as healthy. The API client waits out a rate limit itself, so `rate limited` appears once those waits are exhausted, not at the first 429. +- Toasts: `Telegram paired with user `, `Telegram reconnected`, and `Telegram: reply not delivered ()`. When a send hit a transport failure after the request left, the outcome is unknown and the toast says `reply delivery unconfirmed (check the chat)`. Delivery failure toasts are throttled to one per 30 seconds. +- While a turn that arrived from Telegram runs, the `omp` spinner reads `Answering Telegram · ""`. +- Widget lines bypass the host's text sanitizer, so every Telegram-controlled string shown there (thread name, bot name, queued prompts, error text) is stripped of control characters and bidi overrides. - Messages a follower instance sends through the leader are counted by the leader process, not by the follower. - `/telegram-status` repeats the counters as a `- messages:` line. diff --git a/lib/notices.ts b/lib/notices.ts index 0e1c181..6e0ba3e 100644 --- a/lib/notices.ts +++ b/lib/notices.ts @@ -36,22 +36,26 @@ export interface TelegramNoticeRuntime { } function describeTelegramDeliveryFailure(error: unknown): { - reason: string; + message: string; level: TelegramNoticeLevel; } { const message = error instanceof Error ? error.message : String(error); const summary = summarizeTelegramStatusError(message); + if (summary?.deliveryUnknown) { + return { + message: "Telegram: reply delivery unconfirmed (check the chat)", + level: "warning", + }; + } if (summary) { return { - reason: summary.label, + message: `Telegram: reply not delivered (${summary.label})`, level: summary.kind === "fatal" ? "error" : "warning", }; } const raw = sanitizeTelegramStatusText(message); - return { - reason: raw.length > 80 ? `${raw.slice(0, 79)}…` : raw, - level: "error", - }; + const reason = raw.length > 80 ? `${raw.slice(0, 79)}…` : raw; + return { message: `Telegram: reply not delivered (${reason})`, level: "error" }; } /** @@ -93,8 +97,8 @@ export function createTelegramNoticeRuntime( return; } lastDeliveryNoticeAtMs = nowMs; - const { reason, level } = describeTelegramDeliveryFailure(error); - show(ctx, `Telegram: reply not delivered (${reason})`, level); + const failure = describeTelegramDeliveryFailure(error); + show(ctx, failure.message, failure.level); }, }; } diff --git a/lib/polling.ts b/lib/polling.ts index 96bab1f..9cd3d9d 100644 --- a/lib/polling.ts +++ b/lib/polling.ts @@ -1055,16 +1055,32 @@ export async function runTelegramPollLoop< const admittedUpdates = new Set(); let handledUpdateFailureRethrown = false; let consecutiveGetUpdatesConflicts = 0; - let failing = false; + // An error status is on screen; it stays until polling works again, so a + // slow retry (for example a long rate-limit wait) never shows as healthy. + let errorShown = false; + // The last getUpdates failed, so the next successful one is a recovery. + // Failures of an update handler do not count: the transport kept working. + let transportDown = false; + const showError = (message: string): void => { + errorShown = true; + deps.onErrorStatus(message); + }; while (!deps.signal.aborted) { + let inGetUpdates = false; try { + inGetUpdates = true; const updates = await deps.getUpdates( buildTelegramLongPollRequest(deps.config.lastUpdateId), deps.signal, ); + inGetUpdates = false; consecutiveGetUpdatesConflicts = 0; - if (failing) { - failing = false; + if (errorShown) { + errorShown = false; + deps.onStatusReset(); + } + if (transportDown) { + transportDown = false; deps.onRecovered?.(); } for (const update of updates) { @@ -1095,7 +1111,7 @@ export async function runTelegramPollLoop< throw error; } const message = getTelegramPollingErrorMessage(error); - deps.onErrorStatus( + showError( `skipping Telegram update ${update.update_id} after ${failureCount} failures: ${message}`, ); admittedUpdates.add(update.update_id); @@ -1124,11 +1140,10 @@ export async function runTelegramPollLoop< continue; } consecutiveGetUpdatesConflicts = 0; - failing = true; - deps.onErrorStatus(getTelegramPollingErrorMessage(error)); + if (inGetUpdates) transportDown = true; + showError(getTelegramPollingErrorMessage(error)); await deps.sleep(TELEGRAM_POLLING_RETRY_MS, deps.signal); if (deps.signal.aborted) return; - deps.onStatusReset(); } } } diff --git a/lib/status.ts b/lib/status.ts index ddce3e5..d68b49c 100644 --- a/lib/status.ts +++ b/lib/status.ts @@ -908,6 +908,11 @@ export interface TelegramStatusErrorSummary { hint?: string; /** fatal needs the operator, attention needs a look, retrying heals itself. */ kind: "fatal" | "attention" | "retrying"; + /** + * The failed call may still have reached Telegram: a non-idempotent send hit + * a transport failure after the request left, so the outcome is unknown. + */ + deliveryUnknown?: boolean; } /** @@ -917,6 +922,13 @@ export interface TelegramStatusErrorSummary { export function summarizeTelegramStatusError( message: string, ): TelegramStatusErrorSummary | undefined { + if (/may have committed before transport failed/i.test(message)) { + return { + kind: "attention", + label: "delivery unconfirmed", + deliveryUnknown: true, + }; + } if (/HTTP 401\b|\bUnauthorized\b/i.test(message)) { return { kind: "fatal", label: "invalid token", hint: "/telegram-setup" }; } diff --git a/tests/notices.test.ts b/tests/notices.test.ts index 09eb522..d22a259 100644 --- a/tests/notices.test.ts +++ b/tests/notices.test.ts @@ -61,11 +61,16 @@ test("Delivery failures explain the reason and pick a severity from it", () => { harness.runtime.deliveryFailed(new Error("Telegram API sendMessage failed: HTTP 401: Unauthorized")); harness.advance(TELEGRAM_DELIVERY_FAILED_NOTICE_COOLDOWN_MS); harness.runtime.deliveryFailed(new Error("message is too long")); + harness.advance(TELEGRAM_DELIVERY_FAILED_NOTICE_COOLDOWN_MS); + harness.runtime.deliveryFailed( + new Error("Telegram API sendRichMessage may have committed before transport failed."), + ); assert.deepEqual(harness.notices, [ "warning|session|Telegram: reply not delivered (rate limited 12s)", "warning|session|Telegram: reply not delivered (offline)", "error|session|Telegram: reply not delivered (invalid token)", "error|session|Telegram: reply not delivered (message is too long)", + "warning|session|Telegram: reply delivery unconfirmed (check the chat)", ]); }); diff --git a/tests/polling.test.ts b/tests/polling.test.ts index 1128a0e..cc227cb 100644 --- a/tests/polling.test.ts +++ b/tests/polling.test.ts @@ -667,7 +667,6 @@ test("Poll loop reports recovery once, only after a failed attempt", async () => }; assert.deepEqual(await run(2), [ "status:network down", - "status:ok", "status:network down", "status:ok", "recovered:ctx", @@ -714,6 +713,7 @@ test("Poll loop runner ignores stale-context status failures while retrying", as getUpdates: async () => { calls += 1; if (calls === 1) throw new Error("network down"); + if (calls === 2) return []; throw new DOMException("stop", "AbortError"); }, persistConfig: async () => {}, @@ -739,6 +739,74 @@ test("Poll loop runner ignores stale-context status failures while retrying", as ]); }); +test("Poll loop keeps the error status until a poll succeeds", async () => { + const events: string[] = []; + let calls = 0; + await runTelegramPollLoop({ + ctx: TEST_CONTEXT, + signal: new AbortController().signal, + config: { botToken: "123:abc", lastUpdateId: 1 }, + deleteWebhook: async () => {}, + getUpdates: async () => { + calls += 1; + if (calls <= 2) throw new Error("rate limited"); + if (calls === 3) return []; + throw new DOMException("stop", "AbortError"); + }, + persistConfig: async () => {}, + handleUpdate: async () => {}, + onErrorStatus: (message) => { + events.push(`error:${message}`); + }, + onStatusReset: () => { + events.push("reset"); + }, + sleep: async () => { + events.push("sleep"); + }, + }); + assert.deepEqual(events, [ + "error:rate limited", + "sleep", + "error:rate limited", + "sleep", + "reset", + ]); +}); + +test("Poll loop does not report recovery when only an update handler failed", async () => { + const events: string[] = []; + let calls = 0; + await runTelegramPollLoop({ + ctx: TEST_CONTEXT, + signal: new AbortController().signal, + config: { botToken: "123:abc", lastUpdateId: 1 }, + maxUpdateFailures: 3, + deleteWebhook: async () => {}, + getUpdates: async () => { + calls += 1; + if (calls <= 3) return [{ update_id: 6 }]; + throw new DOMException("stop", "AbortError"); + }, + persistConfig: async () => {}, + handleUpdate: async () => { + throw new Error("handler failed"); + }, + onErrorStatus: (message) => { + events.push(`error:${message}`); + }, + onStatusReset: () => { + events.push("reset"); + }, + onRecovered: () => { + events.push("recovered"); + }, + sleep: async () => {}, + }); + assert.equal(events.includes("recovered"), false, events.join("|")); + assert.ok(events.includes("reset"), events.join("|")); +}); + test("Poll loop initializes lastUpdateId and processes updates", async () => { const handled: number[] = []; const config: { botToken: string; lastUpdateId?: number } = { @@ -995,6 +1063,7 @@ test("Poll loop reports retryable errors and sleeps before retrying", async () = if (calls === 1) { throw new Error("network down"); } + if (calls === 2) return []; throw new DOMException("stop", "AbortError"); }, persistConfig: async () => {}, diff --git a/tests/status.test.ts b/tests/status.test.ts index 49a6f58..cd58999 100644 --- a/tests/status.test.ts +++ b/tests/status.test.ts @@ -388,6 +388,7 @@ test("Error summaries turn transport failures into a short state and next step", ["Telegram API getUpdates failed: HTTP 502: Bad Gateway", "retrying", "Telegram unavailable", "retrying"], ["Unable to connect. Is the computer able to access the url?", "retrying", "offline", "retrying"], ["fetch failed", "retrying", "offline", "retrying"], + ["Telegram API sendRichMessage may have committed before transport failed.", "attention", "delivery unconfirmed", undefined], ["typing failed", undefined, undefined, undefined], ["settingsManager.reload is not a function", undefined, undefined, undefined], ];