feat(remote): commonize remote contracts and add iMessage channel - #27
Open
manhnguyen24-dev wants to merge 78 commits into
Open
manhnguyen24-dev wants to merge 78 commits into
manhnguyen24-dev wants to merge 78 commits into
Conversation
…store Foundation for the Telegram remote-control feature (spec: documents/plans/remote-channel-telegram.md, plan: documents/plans/remote-access-telegram-implementation.md). - app/core/credential_store.py: standalone, keyed OS-vault credential store (service/account at construction). Independent of app/conductor/, which keeps its own separate implementation unchanged - evo-conductor depends on Conductor's current behavior and constructor signature. - app/models/remote.py: RemoteConnection (per-installation bot connection, non-secret metadata only) and RemotePairing (connection-scoped principal/ destination binding), both registered in app/models/__init__.py and app/migrations/env.py. - Migration 00000064 creates remote_connections and remote_pairings; SCHEMA_HEAD bumped to match. - Tests: tests/core/test_credential_store.py, tests/remote/test_models.py, and extended tests/core/test_alembic_migrations.py to assert the new tables/columns/FKs/unique constraints at head. tests/conductor/test_client_service.py still passes unmodified, confirming Conductor has zero behavior change. AC-3, AC-4, AC-5, AC-6, AC-17 (partial - schema only; service/route behavior follows in Task 2+).
…n service Adds RemoteSettings (outbound_data_policy/outbound_pii_policy, defaulting to redact/standard) wired into RuntimeSettings and exposed via GET/PUT /api/settings/remote. Adds app/remote/contracts.py with the provider-neutral, connection-aware value types and protocols for the remote-access seam (RemoteAdapterKind, RemoteConnectionState, RemoteErrorClass, RemotePrincipal, RemoteInboundAction, RemoteOutboundMessage, RemoteAdapterStatus, RemoteAdapter, RemoteAdapterFactory) with no Telegram/httpx import anywhere. Adds RemoteConnectionService (create_connection/update_token/set_enabled/ remove/list/get) enforcing the v1 one-connection product limit, validating a bot token before vaulting it, keeping DB transactions free of vault/ network calls with compensating vault cleanup on commit failure, and invalidating a connection's pairing row on bot-identity replacement while leaving it untouched for a same-bot token rotation. Extends OutboundChannel with the "remote" value for later tasks' explicit policy selection. app/conductor/ is untouched.
…ove() Review finding on Task 2: remove() deleted and committed the DB row before attempting the vault delete, so a vault-delete failure permanently stranded the bot token in the OS credential store with no remaining row naming its account key. Reorders to vault-first, DB-second — the same pattern create_connection/update_token already use — so a vault failure now leaves the row intact (safely retryable) instead of orphaning the secret. Updates test_remove_reports_vault_deletion_failure to assert the row survives a vault-delete failure, and adds a retry-after-recovery test proving remove() can be called again once the vault is reachable.
Adds a Telegram-specific HTTP client (getMe/deleteWebhook/getUpdates/ send_text/edit_text/answer_callback/set_commands) and a TelegramAdapter polling lifecycle, isolated from connection_service.py and Conductor and fully testable against httpx.MockTransport. - Bounded pydantic models for the subset of the Bot API wire format used. - Safe error classes (TelegramApiError/TelegramTransportError/ TelegramMalformedResponseError/TelegramCallbackDataError) that never carry the request URL or bot token, backed by tests sweeping every error path. - Adapter mirrors app/conductor/service.py's start/stop shape: one owner task, an asyncio.Event for interruptible backoff/rate-limit sleeps, and task cancellation to interrupt a genuinely in-flight long poll. - Implements the spec's Telegram error taxonomy: webhook conflict cleared once and retried immediately, concurrent-consumer conflict -> bounded backoff in used_elsewhere, invalid token terminal for the current start(), 429 honors retry_after verbatim, transport/unknown failures -> exponential backoff with jitter, and phone-unreachable delivery failures never affect the inbound poll loop. - Never sets parse_mode; enforces the 1-64 byte callback_data contract before sending; deletes any webhook and drops pending updates before the first getUpdates call. 53 tests in tests/remote/telegram, ruff and ty check clean on the touched files.
Code review found that TelegramAdapter.answer_callback called the client with no try/except, while send/edit both catch TelegramApiError and call _record_delivery_failure. The spec's taxonomy groups all three surfaces under one phone-unreachable delivery-status requirement, independent of poll-loop health, so answer_callback now matches send/edit: it records delivery failure/success around the underlying call and re-raises on failure, same as the other two. Adds two adapter tests parallel to the existing send-failure/send-success coverage, proving an unreachable-chat 403 on answerCallbackQuery sets phone_reachable/last_error_class without touching poll-loop state, and a successful answer marks phone_reachable again.
Add PairingService (app/remote/pairing.py): issues single-use pairing tokens (secrets.token_urlsafe(16), 128 bits, 22 chars, Telegram-safe [A-Za-z0-9_-] alphabet, ten-minute monotonic expiry, in-memory only), binds a RemotePrincipal to a RemotePairing row only once token validity, private-chat-ness, non-bot-sender, connection match, and the one- pairing-per-connection limit all pass, and authorizes/deauthorizes inbound principals with no caching lag after unpair (AC-7..AC-10). Design choice: issue_link stays a pure in-memory operation with no DB access; the one-pairing-per-connection limit is enforced in consume() instead, so re-pairing requires an explicit unpair first (matching the spec's separate Connect phone / Unpair phone actions). Every consume() failure returns the identical None and burns the token only on success, so a legitimate retry (e.g. group chat -> private chat) still works. Rate limiting reuses the sliding-window algorithm already proven in app/services/webbridge_pairing_service.py, replicated as a small private helper to avoid a cross-feature import for a generic utility.
PairingService.consume() separated its "no existing pairing" SELECT and its token pop/insert with await points and no lock held across them. Two concurrent consume() calls could both pass the one-pairing-per-connection check before either committed (breaking the v1 limit), or both race past the same since-consumed token toward a duplicate insert (an unhandled IntegrityError instead of the uniform refusal). Add a per-instance asyncio.Lock (same pattern as WebBridgeTicketStore in app/services/webbridge_pairing_service.py) held across the whole existing-pairing-check -> token-pop -> insert sequence, and check tokens.pop()'s return value as defense-in-depth even under the lock. Confirmed the race was real before this fix: running the two new concurrent-consume tests against the pre-fix code hung indefinitely (concurrent unsynchronized use of the same AsyncSession), rather than racing to a clean duplicate-row or IntegrityError outcome.
Adds the authenticated /api/remote/* HTTP surface (connections CRUD, token replace, pairing-links/pairing/status) as thin routes over the existing RemoteConnectionService/PairingService, and app/remote/runtime.py as a process singleton (start/stop/reconcile_connection/status) that lazily constructs a real TelegramAdapter only for an enabled, credentialed connection - every Telegram import stays function-local so an idle installation never pulls app.remote.telegram.* into sys.modules, creates a task, or touches the network (AC-1). Wires remote_runtime.start()/stop() into app/api/app.py's existing optional-service startup/shutdown pattern, alongside Conductor/Scheduler, without touching app/conductor/. Adds RemoteConnectionService.set_label (mirrors set_enabled) so PATCH can rename a connection.
Adds documents/plans/remote-telegram-response-ui.md, a proposed design that completes the accepted remote-channel-telegram.md feature with HTML-formatted messages, a typing-indicator status lifecycle, cross-origin completion notifications, a guided project/prompt picker, and a read-mostly /settings view. Explicitly amends AC-24 (parse mode) and the outbound-redaction portion of AC-32/Non-goals, with a cross-reference note added to the original spec.
Natural-language ingress and current-task behavior (Task 6, complete), plus in-progress work on stream projection (Task 7), gate cards (Task 8), and secondary actions (Task 9) per documents/plans/remote-access-telegram-implementation.md. Committed as-is to give the response-UI plan a stable base to branch from; Tasks 7-9 test coverage is not yet complete.
- render_gate_card: escape label from actions - render_settings_card: escape name in Redaction/Notify buttons - render_prompt_suggestions: escape label from suggestions - Add 3 test cases verifying escaping of HTML chars in dynamic labels
TelegramClient.send_text/edit_text now always send parse_mode="HTML" (AC-24, revised) since callers pass text already rendered through app/remote/formatting.py's HTML-escaping. Adds TelegramClient.send_chat_action and a matching indicate_typing on the RemoteAdapter Protocol and TelegramAdapter, a best-effort "still typing" liveliness primitive for AC-38 that later turn-lifecycle code will call.
… paths Review flagged that indicate_typing's widened except clause (added beyond the brief's TelegramApiError-only sketch) was untested for its TelegramTransportError and TelegramMalformedResponseError branches. Adds coverage for both, and adds debug-level logging on swallowed failures (previously a bare pass) so the high-frequency liveliness call stays diagnosable without warning-level spam.
RemotePairing.notify_scope (default "all") lets a pairing record whether it wants notifications from every session or only ones the phone itself started (AC-42, schema half). RemoteProjection gains an in-memory active-pairing cache (connection_id, destination_id, notify_scope, principal_id) so future notification logic can look up the single v1 pairing without a database query per event; the runtime keeps it in sync on startup restore, on a fresh pairing, and on stop.
RemoteActionService now holds an optional reference to the outbound RemoteProjection (set_projection, wired from RemoteRuntime._start_locked) so _cmd_unpair can call clear_active_pairing() immediately after the pairing row is deleted. Without this, /unpair revoked callback/menu tokens and inbound authorization but left the in-memory active-pairing cache added earlier in this branch stale until the runtime restarted — a regression against AC-10's "immediate revocation" guarantee once that cache is read by future notification logic.
Adds turn_activity.py (queries SessionMessage rows for a turn's tool-call summary, used for the done-card's "N tool calls" line) and the phone-admitted status-message lifecycle on RemoteProjection: begin_phone_turn() sends one status card and starts a repeating native typing indicator; _finalize_turn() builds the final done/error card and edits that same message instead of sending a new one, then stops typing.
Fixes 5 Important issues found in code review of the phone-admitted status lifecycle: - Typing loop now re-checks self._adapter every iteration (not a stale snapshot) and survives a transient indicate_typing error; set_adapter(None) stops every live turn's typing task on shutdown instead of relying on a done/error event that may never arrive. - begin_phone_turn reuses an existing unresolved turn's status message (stop its typing task, edit in place) instead of overwriting turn state and leaking the old typing task and status card. - drain_pending is now serialized with an asyncio.Lock (matching PairingService._consume_lock's precedent), and _finalize_turn only edits a status card that a new _sent_correlations set confirms was actually sent, falling back to a fresh send otherwise so a card is never silently dropped. - The _finalize_turn call inside drain_pending's pump is now wrapped in the same try/except as every other adapter call, so a DB failure there can't abandon the rest of the queue. - A registered-but-never-begun turn's card now falls back to a "Task" title instead of rendering blank. Also addresses the reviewer's quick minor items: redact turn.title before interpolation, factor out the duplicated connection-id lookup, guard begin_phone_turn's create_task with the same RuntimeError pattern used elsewhere, and add coverage for the typing-task cleanup paths.
…vity
The remote channel can start work but can't be trusted with it: permission
cards name only the tool ("Permission requested: shell") while the actual
command sits unused in the event payload; the default `auto` mode never
blocks, so approvals are structurally unreachable; and a running turn shows
nothing about what it's doing.
Amends five accepted contracts — AC-20/AC-22 (tool names allowed in the
opt-in live mode only), AC-28 (session-scoped `always` was never the
permanent grant the restriction assumed), AC-32 and AC-41 (mode, model and
lead agent become remotely settable), AC-38 (throttled live edits) — and
carves `bypass` out as permanently desktop-only, since a phone that can
silently disable every approval prompt is an escalation path, not a feature.
Direct user feedback: /new, /stop, and /actions "didn't mean anything"
without already knowing EvoFlux's task/turn model, and the pairing
confirmation read like a log line ("Connected to EvoFlux on {label}.")
with no next step. Rewrites lead with the free-text behavior first (the
thing users were most confused about), state each command's before/after
in plain terms, and give the pairing card something to do next.
Design and rationale: documents/plans/remote-telegram-copy-improvements.md
A handler exception was logged at warning level with the connection and update id only — no traceback. Live debugging against a real bot had nothing to go on beyond "it failed." logger.exception captures the stack trace; behavior (retry the same update on next poll) is unchanged.
A conversational turn with zero tool calls rendered as an empty-looking "✅ Title · 12s / 0 tool calls" card with no answer anywhere in it — live testing against a real bot showed the app's own thread had the reply, Telegram never got it. A "done" StreamEnvelope carries no text field at all (DoneEvent has only type/metadata), so the fix reads the turn's last persisted assistant message instead, the same source the app itself reads from. Also stopped attaching an always-present "Tool log" button that only ever opened an empty "No tool calls." page for the (most common) tool-free case — now shown only when the turn actually called a tool.
…g sys.modules Confirmed a genuinely flaky (not just theoretically risky) test failure: tests/remote/telegram/test_adapter.py::TestDeliveryIndependentOfPolling failed 40-70% of full-suite runs, always passed alone. Bisected to a raw sys.modules.pop() (untracked by monkeypatch, never restored) that forces every later test to import a second, distinct TelegramApiError class — so a later test's freshly-local `from ... import TelegramApiError` binds to a different class than the one already-imported code actually raises, and pytest.raises(TelegramApiError) silently stops matching. monkeypatch.delitem() instead of the raw pop restores it at teardown. Verified with 8 consecutive full-suite runs (0 failures, was ~40-70%).
Phase 1 (Task 1) of documents/plans/remote-telegram-approvals-implementation.md — pure display-hint derivation, never consulted by any gating decision.
Phase 1 (Task 2) of documents/plans/remote-telegram-approvals-implementation.md — the card builders only; not yet wired into gates.py.
Phase 1 (Task 3, steps 1-5) of documents/plans/remote-telegram-approvals-implementation.md — extracts the permission_asked branch into _on_permission_asked, wired to the new severity/formatting helpers. Every gate card (including question/plan) now carries a correlation_id.
Task 2 of documents/plans/remote-telegram-live-mode-implementation.md.
Task 3 of documents/plans/remote-telegram-live-mode-implementation.md.
…n (AC-55/56) Task 4 of documents/plans/remote-telegram-live-mode-implementation.md. RemoteInboundResult now carries the pairing's response_mode (captured before handle_text's mid-turn rollback expires the ORM instance), and runtime.py passes it into begin_phone_turn.
… (AC-56/57) Task 5 of documents/plans/remote-telegram-live-mode-implementation.md. observe() now feeds tool_call/tool_start/tool_end/thinking into each live-mode turn's LiveActivityWindow and schedules a throttled, budgeted status-card edit via the new shared EditBudget; summary-mode turns are unaffected (AC-20 regression guard). A turn's final done/error card always bypasses the budget, and a Telegram 429 degrades that connection's cadence via retry_after.
…AC-58) Task 6 of documents/plans/remote-telegram-live-mode-implementation.md. Closes the last gap in the bounded command surface — /settings, /health, and /changes have been dispatchable since earlier phases but were never added to Telegram's native "/" command menu.
Task 1 of documents/plans/remote-telegram-providers-implementation.md.
…-53) Task 2 of documents/plans/remote-telegram-providers-implementation.md.
Task 3 of documents/plans/remote-telegram-providers-implementation.md.
Task 4 of documents/plans/remote-telegram-providers-implementation.md. AST-based rather than a raw-text scan, so control.py's own module docstring (documenting which functions it deliberately never calls) isn't mistaken for a real reference.
…ssage (AC-54) Task 1 of documents/plans/remote-telegram-onboarding-implementation.md.
…AC-54) Task 2 of documents/plans/remote-telegram-onboarding-implementation.md.
Task 3 of documents/plans/remote-telegram-onboarding-implementation.md. Closes the last item in the original control-surface spec (AC-45 through AC-58).
These have been dispatchable and in Telegram's own command menu since Phase 2/3, but _HELP_TEXT was never updated to mention them — a paired user typing /help had no way to discover they existed.
Tracks the last 200 message ids this bot sent per destination and, via clear_history(), deletes them through Telegram's deleteMessage API, tolerating individual failures (e.g. a message past Telegram's 48-hour deletion window) rather than aborting the whole clear. Prep for a /clear command.
A deliberate, post-spec addition beyond the original control-surface document's AC-58 command set, requested directly this session. Deletes only messages the bot itself sent (Telegram's own restriction) via the adapter's clear_history(); an adapter without that capability gets a plain "can't clear messages" reply instead of an error.
Adds two new phone-facing commands: /pair (type an 8-digit code shown on
the desktop instead of clicking a deep link, with QR support) and
/history (browse and drill into recent sessions). Both are wired into
the Telegram command menu and /help alongside the existing set.
Fixes four bugs found during live Telegram testing (see
documents/plans/telegram-integration-testing-2026-09.md for the full
root-cause writeup):
- Tool log always showed "unknown: {}" — tool_calls are stored in
OpenAI's nested {"function": {...}} shape, not flat.
- Cost never appeared — UsageEvent.cost is a dict of components, not a
scalar.
- Model name never appeared — it lives in metadata.models, not a
top-level field.
- Error cards dumped raw HTTP details instead of a friendly message.
Also adds a context-window/token/cost usage line to the done card, a
markdown-to-Telegram-HTML renderer for richer responses, and a new
remote_pairings migration (pair_code_hash/pair_code_expires_at).
Audit fixes applied while reviewing this batch before commit:
- tests/remote/telegram/test_adapter.py: a test asserted against an
undefined `status` variable (broken since it was written) — added
the missing `adapter.status()` call.
- app/remote/actions.py: the new /pair success path called
set_active_pairing(pairing) with a single positional argument against
a method requiring four keyword-only strings — would have raised
TypeError on every successful code-pairing. Fixed to match the
existing call pattern in runtime.py.
- app/api/routes/remote.py: revoke_pairing built a RemoteOutboundMessage
with connection_id=str(connection_id) against a UUID-typed field —
removed the unnecessary/incorrect str() coercion.
- app/remote/pairing.py: two SQLAlchemy `.is_not(None)` calls were
suppressed with `# type: ignore[union-attr]`, which this project's
`ty` checker doesn't recognize — switched to `# ty: ignore[...]`.
- app/remote/actions.py: removed an unused, already-stale `CommandName`
Literal type alias (never referenced, and missing half the real
commands) and its now-unused `Literal` import.
- app/remote/telegram/adapter.py: /pair and /history were dispatchable
and in /help but missing from the Telegram command menu itself —
same class of gap fixed for /settings/health/changes earlier tonight.
- Fixed a missing-space typo in /pair's usage message, and a pending
ruff-format diff in gates.py.
Full tests/remote/ suite (all touched and pre-existing tests), ruff,
and ty all pass after these fixes.
…am-control # Conflicts: # app/core/schema_version.py # app/migrations/env.py # app/models/__init__.py # documents/features/README.md
…ode switch The migration that added remote_pairings.response_mode still shipped server_default="summary" after the model's Python-level default was changed to "live" during live testing — the two must agree since a raw insert would otherwise land on the wrong value. Also fixes a stale test (tests/models/test_remote_models.py) that still asserted the old "summary" default; tests/remote/test_inbound.py was already updated to expect "live" when this default changed, but this second, independent assertion was missed.
…nded git mv renames a file's path but not its content — after renaming the four remote_pairings migrations to 00000067-00000070 (to sit after main's independently-numbered 00000064-00000066), I edited each file's revision/down_revision strings but only re-staged one of the four (00000069). The other three (00000067, 00000068, 00000070) kept their pre-rename revision ids in the actual commit despite the working tree looking correct, which broke alembic's revision graph (KeyError: '00000068') the moment a fresh checkout (no stale working tree) tried to resolve it — exactly what happened on the just-synced main checkout. Caught by re-running the full migration test suite there instead of trusting the worktree's already-fixed working tree.
manhnguyen24-dev
changed the title
Keep the remote-channel implementation, plans, feature documentation, migrations, tests, and UI changes in the feature branch while excluding ASDD working artifacts and the optional protocol Dockerfile.
The native iMessage adapter starts in pairing state before a principal exists. Allow the pairing link flow in both pairing and polling states so the first phone can complete authorization.
…management Telegram and iMessage remote interface overhaul: Streaming and media: - Live card edits with heartbeat-based throttling and abort at 10 MiB - Telegram photo/document and iMessage attachment pipeline - Provider-neutral RawAttachment contract (InboundAttachment) - Cumulative token/cache/cost accounting snapshot model Process-card animation: - 10-frame braille spinner with phase labels (thinking/streaming/finalizing) - Model-aware capability icons (capture/retention/vision/reasoning/fallback) - Terminal card deduplication and stale-edit purge Commands and settings: - /status dashboard: connection, providers, model, activity stats, session age - /skills catalog and /skill <name> loader - /agent info card with model capability icons - /steer <instruction> for mid-task redirection - /switch <n> session switching, /delete <n> session cleanup - /start merged with pairing: /start shows prompt, /start <code> pairs - /pair removed (redundant with /start) - Provider-grouped settings model picker with pricing - Thinking level setting (none/low/medium/high) Delivery and reliability: - Desktop-origin terminal delivery with addressability check - Worker-thread drain fix (call_soon_threadsafe) - Long-response paragraph splitting at Telegram 4096-char limit - Action button layout (2-per-row short, full-width long) - Question answer button labels with full-answer preservation Documentation: - Updated remote-access.md with media, steering, settings, commands - Updated Telegram adapter command registration (17 commands) - 439 tests pass, all lint/type checks clean
Desktop-origin turns that complete may never show a terminal card if _finalize_turn throws (e.g. DB connection issue in load_turn_activity). The exception was logged but no message was sent, leaving the typing indicator running forever. Changes: - Add fallback send in drain_pending exception handler: plain '✅ Task complete' or '❌ Task failed' message via adapter.send - Add diagnostic log in _handle_done (session_id, phone_admitted) to trace done-event delivery in production
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
2 participants
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
Commonize the remote-channel contracts and add a provider-neutral iMessage channel alongside Telegram.
What changed
RemoteAdapterRegistryand provider-neutral lifecycle, status, pairing, watermark, streaming, redaction, live-budget, reply-target, and attachment contracts.imsgJSON-RPC over stdio as the primary iMessage provider.Verification
tychecks for changed remote code: passed.origin/main: passed; three existing lint warnings remain.Real-device follow-up
A real Mac and phone are still required to verify macOS Messages/TCC permissions, Apple ID/iCloud state, pairing, inbound/outbound delivery, attachments, process restart recovery, and provider failure recovery.
This PR deliberately does not include ASDD evidence/proposal/task artifacts, the optional protocol Dockerfile, credentials, generated build output, local runtime state, or unrelated desktop permission changes.