Skip to content

feat: add Telegram process-event channel - #2933

Open
bingb0t5 wants to merge 12 commits into
kunchenguid:mainfrom
bingb0t5:fm/fm-telegram-adapter
Open

feat: add Telegram process-event channel#2933
bingb0t5 wants to merge 12 commits into
kunchenguid:mainfrom
bingb0t5:fm/fm-telegram-adapter

Conversation

@bingb0t5

Copy link
Copy Markdown

Intent

Make Telegram a first-class firstmate channel by adding a process-event adapter (bin/fm-procevent-telegram.sh), so the captain's messages wake firstmate within seconds instead of waiting up to five minutes for a check sweep. Inbound Telegram currently runs as a home-local state/telegram-watch.check.sh polled on the watcher's check sweep, so a reply can sit for five minutes. Telegram's getUpdates accepts a timeout parameter and blocks until a message arrives, which is exactly the shape bin/fm-procevent.sh exists to supervise.

Requirements:

  • The adapter is thin: it owns only Telegram-specific canonical source identity, the blocking child's argv, and how to read a completed result. Ownership, durable capture, publication, and restart recovery stay owned by bin/fm-procevent.sh; do not change that script.
  • Commands: arm, source-id, classify, terminal, retire, matching the generic adapter contract bin/fm-procevent.sh calls.
  • terminal must NEVER report terminal (exit 0) - the captain's Telegram channel is permanent and must never retire itself. This is a deliberate, explicit exception documented in the header.
  • answers is deliberately NOT implemented: mapping a Telegram message onto a captain-held decision key is a separate problem, and guessing at it would feed the keyed-answer intake something the captain did not clearly say.
  • The blocking child (poll): one long getUpdates poll per invocation then exits so the runner captures a result and restarts it.
  • Every text message is written to disk under state/telegram-inbox/ (one JSON file per message, mode 0600, matching the existing state/telegram-watch.check.sh convention) BEFORE the offset file advances past it. Telegram permanently deletes updates once getUpdates is called with a higher offset, so if any write in a batch fails, the offset must not advance at all - the whole batch, including already-written messages, is safely re-fetched next time. Proven with a real filesystem write failure (a directory pre-occupying a message's own target path), not a mocked helper.
  • The bot token (~/.config/beanz/telegram.env, mode 600, gitignored, outside the repo) is never logged, printed, or persisted anywhere - not in a repo file, a test fixture, a log line, or the result payload. It reaches curl only through an inline -K - stdin config, never as a literal argv element.
  • An absent or unreadable credential file exits the poll child with status 0 and no output - a deliberate, narrow, and explicitly documented exception to the otherwise-uniform "nonzero exit with empty stdout means nothing to report" contract that the generic runner's own capture rule requires for every other no-signal case. Because arm() itself already refuses to register a source when no readable credential exists, this exit-0 branch is never reached in ordinary operation; it only fires if credentials are removed after a source was already armed, which is documented as an accepted narrow gap rather than silently deviating from the literal spec.
  • Non-text updates (photos, stickers, chat-membership changes) are consumed (folded into the advanced offset) but never produce an inbox file and never wake firstmate - implemented as a nonzero exit with empty stdout, the only shape bin/fm-procevent.sh's runner treats as "no captured result, no wake" regardless of what was printed.
  • The poll is bounded to a chosen long-poll timeout (25s default, well under Telegram's ~50s practical ceiling) with reasoning in the header, so restart cadence stays responsive and the child cannot outlive the runner's expectations.
  • The offset file is state/.telegram-offset, deliberately the same path and convention the existing check-sweep script already uses; sharing it is safe because every message file is keyed by Telegram's own update id, so any overlap during the handoff window is at worst a harmless idempotent redundant write, never lost or duplicated delivery.
  • Do not remove state/telegram-watch.check.sh (home-local runtime state, not repo material; firstmate retires it after this lands) and do not touch the outbound send path or any credential file.
  • Out of scope, not built: a Telegram group, capture-into-the-brain, or reply routing - channel only.

Tests (tests/fm-procevent-telegram.test.sh) exercise only the executable interface (arm/poll/classify/terminal/retire, plus the real bin/fm-procevent.sh runner for one end-to-end arm+reconcile+capture+publish scenario) - never implementation source bytes. curl is replaced by a fake binary on PATH; no test touches the real Telegram API. Every mandated protection was verified by deliberately removing it and confirming the corresponding test went red, then restoring it:

  • write-before-offset-advance: changed the write-failure handler to swallow the OSError instead of aborting the batch -> "a mid-batch write failure exited 0 and would have woken firstmate" went red.
  • token never leaks: removed the post-curl token scrub and interpolated it into the poll's own stdout -> "the token leaked into the adapter's own stdout" went red.
  • missing credential file is silent: added a stdout print before the credential-check exit -> "missing credential file produced output" went red.
  • non-text update never wakes: made the poll print "none" and exit 0 in the no-message tail instead of exiting nonzero silently -> "a non-text-only poll exited 0 and would have woken firstmate" went red.
  • terminal never reports terminal: flipped its return to 0 -> "terminal reported terminal for a real delivered message" went red.
    All five were restored afterward and the suite is green again.

Documentation follows the knowledge-placement decision tree from firstmate-coding-guidelines: exact flags/commands/paths live only in the script's own header and --help. docs/configuration.md and .agents/skills/process-event-sources/SKILL.md each get a minimal pointer (a couple of sentences) naming the adapter, its never-terminal exception, and - for the skill only - how to handle a procevent telegram telegram N wake (read state/telegram-inbox/, act on it, reply on Telegram since the captain is away from the desk, move handled files to state/telegram-inbox/handled/, mirroring what the retiring check-sweep script's own header already told firstmate to do) - neither restates the adapter's own contract.

Validation performed before this run: bin/fm-lint.sh clean (shellcheck 0.11.0 pinned) on the new/changed files; bin/fm-doc-audience-check.sh clean; the full 161-script firstmate test suite run twice - the only two failures in the second, fully-tooled run are pre-existing and independently confirmed (via a clean git-archive extraction of main) to reproduce identically on main and to be unrelated to this branch: a Pi-follow-up duplicate-captain-answer test in tests/fm-calm-pi-extension.test.sh, and a missing-ruby YAML-parsing failure in tests/fm-test-run.test.sh. bin/fm-procevent-telegram.sh arm was demonstrated end-to-end against the real credential file (registered, shown by bin/fm-procevent.sh list, then retired) with no source left armed afterward.

What Changed

  • Add a Telegram process-event adapter that long-polls for captain messages and publishes them through the existing process-event runner.
  • Persist text messages securely before advancing the shared Telegram offset, while consuming non-text updates without waking Firstmate and keeping the channel armed until explicit retirement.
  • Document Telegram wake handling and add executable-interface coverage for adapter commands, failure safeguards, and end-to-end capture and publication.

Risk Assessment

✅ Low: The Telegram adapter is well-bounded, the pending-wake recovery now runs before credential validation, prior source-content-only help assertions were removed, and no remaining source-verifiable correctness or intent-conformance defect was found.

Testing

The supplied baseline had already covered lint, documentation checks, and broad tests; this phase ran the focused Telegram suite plus a manual real-runner delivery using fake curl. Delivery produced a durable private inbox message, advanced the shared offset, published and classified the expected wake, preserved the permanent source, and demonstrated safe legacy overlap without duplicate delivery.

Evidence: End-to-end Telegram delivery transcript

Source: End-to-end Telegram delivery transcript

END-TO-END TELEGRAM DELIVERY EVIDENCE

Published wake:
1787554293	1	check	procevent:telegram:1	check: procevent telegram telegram 1

Durable inbox message (mode 600):
{"update_id": 4101, "date": 1700000200, "chat_id": 555, "text": "captain says test the fast Telegram wake"}

Advanced offset:
4102

Runner-captured result:
message: 1

Classification:
message

Source remains armed after delivery:
SOURCE                       ADAPTER      OWNER      PENDING
telegram                     telegram     none       1
- Outcome: 🔧 1 issue found → auto-fixed ✅ across 2 runs (5m10s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed (5) ✅
  • 🚨 bin/fm-procevent-telegram.sh:319 - The required handoff invariant says overlap with the existing check-sweep is "at worst a harmless idempotent redundant write, never lost or duplicated delivery," but this unconditional os.replace rewrites an update-id file even if another poll already delivered it. Concrete sequence: both pollers fetch update 1001; the adapter writes and wakes; Firstmate handles and moves 1001.json to handled/; the overlapping poll then recreates telegram-inbox/1001.json and reports message: 1, causing the same captain message to be delivered and potentially acted on again. Please confirm whether duplicate delivery during handoff is acceptable; otherwise deduplicate atomically at the inbox persistence boundary, including against handled files, before counting the message as newly delivered.

🔧 Fix: Prevent duplicate Telegram delivery after handoff
1 error still open:

  • 🚨 bin/fm-procevent-telegram.sh:319 - The handoff deduplication is not atomic against the retiring check-sweep because only this adapter observes .delivery.lock. Concrete sequence: this poll writes update 1001 and wakes Firstmate; Firstmate moves it to handled/; then an overlapping invocation of the existing check-sweep, which does not participate in this new lock, rewrites 1001.json and issues its own wake. The required invariant that overlap must never duplicate delivery therefore remains reachable. The sequential handled-file test does not exercise this competing-writer sequence. Please coordinate retirement before arming, or update both producers to use one shared atomic delivery-claim boundary.

🔧 Fix: Require legacy Telegram check retirement before arm
1 error still open:

  • 🚨 bin/fm-procevent-telegram.sh:351 - A successful inbox write can permanently lose its wake if offset persistence fails afterward. Concrete path: poll writes update 1001, write_offset fails (for example .telegram-offset is temporarily a directory), and poll exits nonzero without reporting the message. On retry, 1001 already exists, so it is skipped and messages remains 0; the offset then advances and poll again exits nonzero, leaving the captain's message unwoken indefinitely. A crash after offset advancement but before printing has the same outcome. Persist a pending-delivery marker/count before advancing the offset and clear it only after producing a capturable result, so recovery can report already-written but never-announced messages.

🔧 Fix: Recover Telegram wakes after offset failures
1 error still open:

  • 🚨 bin/fm-procevent-telegram.sh:267 - The pending record is deleted before the poll exits and before fm-procevent.sh durably captures its output, contradicting the requirement to clear it only after printing and exiting 0. Concrete sequence: printf succeeds, rm deletes the marker, and the child or runner dies before fm_procevent_capture; the offset already advanced, so the inbox message is permanently stranded without a wake. Additionally, the unlink is not directory-fsynced, so a power loss can resurrect the marker and duplicate a captured wake. Exact recovery requires an acknowledgement at the runner's durable-capture boundary, or explicit approval to weaken the guarantee because the adapter alone cannot atomically coordinate its exit with its parent.

🔧 Fix: Document Telegram pre-capture crash limitations
2 issues (1 error, 1 warning) still open:

  • 🚨 bin/fm-procevent-telegram.sh:286 - Pending-wake recovery is gated on credentials even though it concerns an already-written message. Concrete sequence: inbox write for update 1001 succeeds, offset persistence fails and leaves .telegram-pending-delivery, then the credential file is removed or becomes unreadable before retry. The retry exits 0 at the credential check without calling report_pending, and subsequent retries keep producing empty captures while the real message remains unwoken. This contradicts the required recovery behavior that the next poll detect the uncleared marker and report the already-written message. Recover a valid pending record before credential validation or any network setup.
  • ⚠️ tests/fm-procevent-telegram.test.sh:65 - The newly added help assertions only search the script's emitted comment text for phrases such as before the runner's, power loss, and Never describe. They do not execute or verify any observable crash-recovery behavior and are source-content-only assertions prohibited by the test-quality rule. Remove these phrase checks; the documented limitation is reviewable documentation, while executable tests should assert actual persisted state and outputs.

🔧 Fix: Recover pending Telegram wakes without credentials
✅ Re-checked - no issues remain.

🔧 **Test** - 1 issue found → auto-fixed ✅
  • 🚨 bin/fm-procevent-telegram.sh:14 - The adapter documentation contradicts the authoritative handoff requirement. It says the legacy check-sweep consumer and the new adapter must never overlap and requires deregistration before arm, while the required contract says sharing state/.telegram-offset during the handoff is safe and overlap is at worst an idempotent redundant write. The legacy home-local watcher is absent from this worktree, so that required overlap behavior also could not be demonstrated end to end. Align the header with the required safe-handoff contract and add executable-interface evidence for overlap if the legacy producer can be represented faithfully.
  • bash tests/fm-procevent-telegram.test.sh
  • Inspected git diff 7b88520c055408a18f1476ecce08be60b2885fc9..628abd878acede37c027fb81df6fc8dc45c7cf66 to map the change to the acceptance criteria.
  • Checked for the legacy state/telegram-watch.check.sh; it is not present in this isolated worktree, preventing a faithful concurrent-handoff exercise.
  • Verified git status --short after testing to confirm no transient worktree artifacts remained.

🔧 Fix: Fix Telegram handoff overlap contract
✅ Re-checked - no issues remain.

  • Inspected git diff 7b88520c055408a18f1476ecce08be60b2885fc9..5f57670eb61f7249761369266ee56c97c694fe5f and the Telegram executable-interface regression scenarios
  • bash tests/fm-procevent-telegram.test.sh
  • Manual fake-Telegram end-to-end run through bin/fm-procevent-telegram.sh arm and bin/fm-procevent.sh reconcile, recording the wake queue, mode-0600 inbox payload, advanced offset, captured result, classification, and still-armed source
  • Verified the evidence transcript does not contain the test bot token and checked git status --short for testing residue
✅ **Document** - passed

✅ No issues found.

⏭️ **Lint** - skipped
  • ⚠️ linter found issues (exit code 1)
✅ **Push** - passed

✅ No issues found.

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge with no concrete unacknowledged defect established in the changed code.

The adapter preserves inbox-before-offset persistence, filters unauthorized and non-text updates, keeps credentials out of durable output, and integrates with the existing runner while explicitly documenting its unavoidable pre-capture handoff limitation.

Reviews (1): Last reviewed commit: "no-mistakes(document): Polish Telegram c..." | Re-trigger Greptile

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Scheduled 3:10am PT 8/24 pass. VISION.md read in full from current main 8fa0505e48a155da78a9aeeb50911719dc558710 (#2939). First look at this unstamped PR. No captain comment authorizing a merge.

VISION (inspected bin/fm-procevent-telegram.sh arm/poll/terminal/retire, credential mode-600 + non-symlink, token via curl -K - stdin not argv, captain-chat filter, inbox 0600 + fsync-before-offset, pending-wake recovery before credential check, terminal never 0, skill wake handling, docs pointer). Per-rule:

  • One captain, one interface: aligns (Telegram talks to firstmate, not to workers; extra delivery surface is opt-in). Treats inbound text as captain input, never instruction (existing skill rule).
  • Authority is explicit and never inferred: aligns as opt-inarm refuses without a readable mode-600 credential; unconfigured homes never register a source. Not default-on.
  • Scripts own the mechanics: aligns (poll, offset, classify are deterministic; answers/autohandle deliberately absent so firstmate still judges).
  • A restart is a non-event: aligns (inbox + offset + pending marker; documented pre-capture crash window is not claimed lossless).
  • Delegation with a spine: aligns (no keyed-answer guess; channel only).
  • The fleet outlives any vendor: mixed — Telegram-specific adapter on the generic procevent runner; credential file is operator-owned ~/.config/beanz/telegram.env, not a firstmate secret store.
  • Scope: aligns (command-layer channel adapter). Extra delivery surface is presentation/convenience and ships as an option.

Class: opt-in. Not default-behavior (must arm; no credential ⇒ no source).

Security (tokens/webhooks, full diff): no .github / workflow secrets. Token never in argv (-K -); scrubbed after curl; never in inbox/offset/stdout (test has a positive control that the stub request did carry it). Credential must be a regular file mode 600. Non-captain chats and non-text updates are consumed without a wake. Inbox writes 0600 + dir fsync before offset advance. set -a; . "$env_file" sources the captain's own 600 file (standard env-file shape; not a repo secret). First-time fork bingb0t5. Full diff reviewed before workflow approval; no security reason to withhold CI.

Workflow-approval this pass (captain gh):

  • 32699208294 CI — approved; now in_progress
  • 32699208264 Require no-mistakes — approved; completed success (GitHub body-compliance only)

CI / NM: HEAD d904e245535ded7234f31ed0380c577ba64f54b0. MERGEABLE / UNSTABLE, ahead 12 / behind 5. Body no-mistakes-pipeline-attestation:v1 matches THIS HEAD (d904e245535ded7234f31ed0380c577ba64f54b0); lint step skipped in the pipeline JSON. GitHub NM SUCCESS is not the SHA match; the body attestation is.

Overlap / HOLD: none of the standing spawn/herdr/lock code files. docs/configuration.md also moves in #2783 and holds #2637/#2804 (3-line pointer here). Skill file is unique to this adapter. Not a spawn/lock hold.

CI is not green yet this pass, so not land-eligible today. Opt-in + matching attestation + no hold-code overlap would be YES once CI is green. Waiting on CI, not the captain. Do not rebase.

Land-eligible: NO. Captain-flag NOW: no.

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.

2 participants