* feat(channels): role-aware action templates for GitHub bound-session delivery Bound-session delivery injected a hardcoded generic prompt and routed each event to a single winner, so a PR followed by review, verify, and fix sessions in different workspaces could only ever reach one of them, with no declarative way to say what each should do. Bindings gain an optional closed-vocabulary role (review/verify/fix): the command-position backfill mints review/verify from /review and /verify-pr, fix binds explicitly through the metadata route (null clears), and a re-offer never overwrites an existing role. A new channel-level boundSessionActions record maps event classes (watch:<type> or notification reason, optionally role-qualified) to operator instruction templates rendered with GitLab-style %var% substitution into a labeled trusted block; substituted GitHub values stay sanitized, capped data. With the map configured, routing generalizes the base design's single-winner decision to one winner per role with fan-out across roles: per-(event, target) durable tasks, independent backpressure, role-scoped gone fallback, and public fallback only at zero live followers. With the map absent, every behavior and the generic prompt stay byte-identical (pinned by tests). Design: docs/design/2026-11-04-github-channel-bound-session-action-templates.md (EN/ZH); the base design's Key decision 4 carries a forward pointer. Co-authored-by: Qwen-Coder <[email protected]> * test(cli): expect the command-stamped role in backfill binding assertions The /review command now stamps role: 'review' on every binding form it mints (role-aware bound delivery). Four strict-equality assertions in the backfill suite predated the field: the Aone URL-form and unplanned-repair cases, the fork-url state-pairing case, and the capped-plan live-bridge sync projection. CI's sharded Test lane surfaced only the first before fail-fast skipped the remaining shards; the full local suites (backfill 177, server 1442, bridge 1043, transport 431, daemon-worker 182, session-list/pr-refresh 87) now pass. Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): delete a record key when its editor row is cleared Addresses the triage review's record-renderer observation. The save serializer already filtered empty values (so no unsavable state existed), but the form state kept a touched-then-cleared key as "" until save; the editor now deletes the key on clear, matching what the upsert payload will contain. A new dialog-level test pins the end-to-end outcome for the 80-key boundSessionActions surface (and the GitLab record fields sharing the renderer). Also documents at roleByNumber that per-number uniqueness is guaranteed upstream by noteMention, so the Map constructor's last-wins semantics cannot override the first-wins command rule. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close the role-aware delivery review findings (C1-C5, D1-D3) C1: the first task of a multi-target comment dispatch now carries the full fan-out intent; recovery reconstructs missing siblings BEFORE applying the survivor's event-wide dedupe, so a crash between claims no longer loses the unclaimed roles permanently. C2: a durable bounded delivery ring (github-bound-delivered.json) records every private acceptance; no gone-fallback or untargeted recovery may route an already-delivered event publicly, even after every follower disappears and the daemon restarts. An unreadable ring fails closed for the public fallback. C3: delivery-time re-validation retires a role-aware comment whose role lost its match on a complete snapshot instead of injecting the obsolete role's instruction into the frozen session. C4: split-sidecar merge lets the newer half win on role, absence included, so an explicit role:null clear survives an archive/restore merge. C5: FIELD_LABEL_KEYS.github registers boundSessionActions, making the zh-CN label and description reachable. D1: the map-skip retire now requires a complete fresh snapshot; under a partial or failing scan the event defers as backpressure instead of being consumed on incomplete evidence. D2: comment-lane events resolve a lane class (comment) after the thread reason, so fix:comment catches comments delivered under mention/subscribed/author while reason-specific keys still win. D3: the worker accessor validates roles against the shared PR_BINDING_ROLES constant instead of hardcoded literals. Each correctness fix has a dedicated regression; C1/C2/C3 regressions were mutation-witnessed (removing the fix turns them red). Design docs (EN/ZH) and the user guide updated to the new semantics. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close the role-aware delivery re-review findings (C1 residual, R2-1..R2-4) Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): complete fanout slots durably and keep private retirement terminal (R2-2/R2-4 residuals) Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): make the fanout completion mark fail-closed with durable acceptance evidence (R2-2 write-failure gap) Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]>
68 KiB
GitHub Channel Events to PR-Bound Sessions
Status: proposed (decisions confirmed by the maintainer on 2026-11-02: prompt-injection delivery, in-channel state watch, event scope = CI failure + merge conflict + new comments + head change / merged / closed).
Revision 2026-11-04: Layer 1 reads the PR sidecars directly (one directory listing per workspace) instead of paginating the activity-ordered session list, so partial now means that part of the catalog could not be read (a registry entry without a runtime, an unlistable chats directory, an unreadable sidecar or transcript, or an unavailable live session list); the session count never makes a snapshot partial.
Problem
A user who follows a PR in a Qwen Code session (a Web Shell or CLI session that created, reviewed, or verified the PR) gets no help from the GitHub channel when that PR changes:
- Two disconnected session systems. The GitHub channel routes every event to a channel-owned session keyed by
github:<owner>/<repo>:pr:<N>(SessionRouter,chat_threadscope). The session that actually follows the PR — the one with the worktree, the context, and the PR binding in itsprssidecar — never sees the event. - Event coverage gaps. The adapter polls the Notifications API and enumerates comments. A pure CI failure (reason
ci_activity, no new comment) dispatches nothing. A merge conflict produces no GitHub notification at all. New pushes and merge/close transitions are likewise invisible unless someone comments.
The building blocks for closing both gaps already exist:
- PR bindings:
packages/core/src/services/session-pr-service.tspersists<chatsDir>/<sessionId>.pr.json; the daemon refresh timer (packages/cli/src/serve/server/session-pr-refresh.ts) already scans every trusted workspace's sidecars on a cycle. There is no reverse lookup (PR → sessions) yet. - Delivery into an existing session:
POST /session/:id/promptaccepts prompts for any live session, resolves the owner runtime by session id (cross-workspace capable), and FIFO-queues while the session is busy (DEFAULT_MAX_PENDING_PROMPTS_PER_SESSION = 5inpackages/acp-bridge/src/session-control-plane.ts; queue full → HTTP 503prompt_queue_full+Retry-After). - Session creation: the channel's existing
SessionRouterpath already creates a channel session when no route exists.
Goals
- When a GitHub event concerns a PR that is bound to a live session, deliver the event into that session as a prompt turn, so the session can handle it with its existing context.
- Sense the events the Notifications API cannot deliver: CI failure on the PR head, merge-conflict appearance, head advancement, and PR merged/closed transitions — for the set of PRs bound to sessions.
- When no bound session exists, keep today's behavior: the event flows through the normal channel path and creates (or reuses) a channel session.
Non-goals
- No webhook push ingestion. The daemon's
POST /channels/:channelName/webhooks/:sourceroute stays as it is (static targets); true push delivery is future work. - No channel takeover of bound sessions (
SessionRouter.activateManagedSessionis not used): a bound session's replies must NOT be auto-published as GitHub comments under the channel publication contract. - No cross-session peer messaging for this path (review-class gating and external-peer registration add complexity without benefit here).
- No automatic PR binding of channel-created sessions. Channel sessions keep their existing routing and public-reply behavior.
- No support for standalone
qwen channel start(non-daemon ACP mode): the reverse lookup is a daemon route. The feature degrades to today's behavior there. - Inline PR review comments remain out of scope (existing adapter limitation).
Design
Layer 1 — daemon reverse-lookup route
New process-global, read-only route:
GET /sessions/pr-bindings
→ { "bindings": [ { sessionId, workspaceCwd, sourceType, updatedAt, prs: [{number, url, state?}] } ], "partial": false }
- Ownership class: process-global (same class as
POST /sessions/backfill-prs). It iterates the registry's workspace entries (listAllEntries()) and, for every entry that has a runtime and can serve sessions (trusted, or the untrusted primary, which is exempt from the trust gate), lists the workspace's active PR sidecars directly —listSessionIdsWithPrSidecar('active'), the samereaddir-based enumeration the PR refresh timer uses — and reads each sidecar withreadSessionPrsDetailed. Attribution comes from the session's persisted record (getSessionListItem:sourceType, transcript mtime asupdatedAt), refined by the bridge's live session list when the session is live (a binding written before the first transcript flush has no persisted record yet, and a live session's activity can be newer than its last flush — the later timestamp wins). A sidecar whose transcript belongs to another project sharing the chats dir is skipped; a transcript that exists but cannot be read makes the session unknown (partial), and an active sidecar whose transcript lives in the archive is a leftover of an archived session and is skipped. Bindings the bridge holds in memory but that have not reached a sidecar yet (the metadata route updates the bridge before persisting the sidecar, and the write can fail) are merged from the live session list: a session with no sidecar row gets one from its live bindings, and a listed non-empty row is topped up with the live PR numbers its sidecar lacks while it is belowSESSION_PR_LIST_LIMIT(then trimmed to the newest) — keyed by number alone, as the session list's merge is, because the sidecar writers replace an entry by number when it is re-bound to another repository and the bridge keeps the replaced entry; an empty listed row is an observed unbind and is never filled, and a session whose sidecar or transcript could not be read this scan is never re-listed from its live subset. A listed row is therefore the sidecar projection plus any bindings the bridge holds that have not been persisted yet. The entry state does not gate the scan: a draining or transitioning workspace still serves its sessions and its files stay readable. Untrusted non-primary workspaces are skipped (they cannot create or restore sessions, so their absence is legitimate). - Archived sessions are excluded (their sidecars live under
chats/archive/, which the scan does not list).sourceTypeis included so callers can exclude channel-owned sessions.prsentries are projected to{number, url, state?}. Every readable sidecar gets a row, including an empty one: aprs: []row is a tombstone meaning "scanned, no bindings" (an observed unbind), which callers must be able to distinguish from "this answer did not enumerate the session" — otherwise any partial answer would revive a stale binding after an explicit unbind. A tombstone names no delivery target, so it carries no attribution; it still pays the ownership check, because an empty sidecar of a hidden-catalog or archived session is a leftover, not an observed unbind. A sidecar that EXISTS but could not be read (I/O error, invalid JSON, failed shape check — core'sreadSessionPrsDetailedabsent/invalid distinction) is omitted withpartialset: its bindings are unknown, not empty. Hidden-catalog sessions (agent-host/agent, the machine-owned task sessions no session listing or count shows) are excluded explicitly, without settingpartial: ownership is decided from whichever record states it (the persisted session record or the live session list) before any read that can fail, so an unreadable sidecar or transcript of a hidden-catalog session is a skip, not a scan failure; only a session whose ownership cannot be read at all (its transcript is unreadable and it is not live) is unknown and setspartial. Those sessions are not interactive delivery targets, so a PR bound only to such a session — the prompt-route bind (/review <N>//verify-pr) does not gate by source type, so an agent task session can acquire a binding — is treated as unbound and its events take the public channel path. The public reply is safe (no loss or duplication); the private-delivery contract deliberately does not extend to machine-owned agent task sessions. partialis true only when part of the catalog could not be read: a registry entry has no runtime (its session storage cannot be located), a workspace's chats directory could not be listed, a sidecar or transcript could not be read, or the workspace's live session list could not be fetched (it is the only source for a binding that has not reached a sidecar). A directory listing is a snapshot, so the number of sessions never makes the answer partial — a workspace with hundreds of active sessions still produces a complete snapshot. Consumers must not treat a missing entry in a partial answer as an authoritative absence (the adapter merges partial answers into its known bindings and skips watch-state pruning). A workspace whose chats directory cannot be listed is logged to stderr by workspace; per-session failures are counted and logged as one line per workspace scan (count, workspace, first failing session and cause), not one line per session per poll, so a persistent partial has a diagnostic surface without flooding the log.- Authenticated like the other session routes (bearer when
--require-auth). No new write surface. - The route is a scan, not an index: one directory listing per workspace plus, per readable sidecar, the project-membership head read, the sidecar read and the list-item projection (a bounded record scan plus the title scan) — about four opens and two stats; tombstones pay the same, because ownership (hidden-catalog or archived) must be decided for them too, and only their attribution is omitted from the row. The filesystem leg touches every session with a sidecar, tombstones included — an unbind keeps the sidecar (the clear writes
{prs: []}) — so it scales with sessions ever bound rather than with currently bound ones, and is still cheaper than the full-catalog listing because sessions that never bound a PR are skipped; the in-memory leg (listWorkspaceSessions, fetched once per workspace per poll for attribution and live-only bindings) materializes one summary per live session, so it scales with the workspace's live sessions. The adapter caches the result for one poll cycle. There is deliberately no page cap on the filesystem leg: it is bounded by the sidecar count (sessions ever bound), each sidecar costs a few small reads, and it is strictly cheaper than the full-catalog live-merged listing it replaced, so a cap would only reintroduce a partial state.
Layer 2 — worker-provided delivery accessor
ChannelBaseOptions (in packages/channels/base/src/ChannelBase.ts) gains one optional field; the ChannelAgentBridge contract is unchanged:
boundSessions?: {
listPrBindings(): Promise<{ bindings: PrBindingSummary[]; partial: boolean }>;
deliverPrompt(
sessionId: string,
prompt: string,
idempotencyKey?: string,
): Promise<void>;
};
- The daemon channel worker (
packages/cli/src/commands/channel/daemon-worker.ts) implements it with the SDK client it already holds:listPrBindingscalls the new route and rebuilds its session→cwd cache from each complete answer (sessions that left the bindings drop out); a malformed HTTP-200 body (non-arraybindings) throws instead of returning an authoritative empty snapshot, routing the failure into the adapter's fail-closed refresh handling. Empty-prs tombstone rows pass through to the adapter (they are the observed-unbind signal); ANY malformed PR entry — or a dropped session row — flags the answer partial, and a listed session whose PR entries ALL fail shape validation is additionally skipped instead of projecting an authoritative empty set.deliverPromptposts the prompt as a text content block and treats acceptance into the FIFO queue as delivery — the worker does not wait for the target session's turn to finish. Every POST carries the caller'sidempotencyKey(a stable event identity derived from the source chat and message id): the prompt route remembers admitted keys per session and returns the original admission on a repeat, so a retry after a lost acceptance mark or a lost 202 re-injects nothing within the daemon's lifetime — dedupe is best-effort under the two FIFO caps (128 keys per session, 1024 sessions), and the ring dies with the in-memory prompt queue, so after a restart re-injection is the correct recovery (Key decision 10). Every reachable pre-enqueue admission check throws SYNCHRONOUSLY — including the reserved standalone working-directory guard, which answers a retryable409 working_directory_missingwhile the session's managed conversation binding has not released — so the route never returns a phantom 202 that the relay would consume as durable acceptance; the route's key-forget on a deferred non-admission remains defense-in-depth. The POST rides the same channel-turn classification as the bridge — three_metakeys that admission verifies together and strips before forwarding: theqwen.channel.promptmarker, theqwen.daemon.channelBoundRelayintent marker, and theqwen.daemon.channelPromptAuthorizationworker credential (the worker sends it only when it holds one) — so the relay is not treated as an interactive user prompt (loop-detected rejection, live PR-binding backfill on relayed text); a POST carrying the two markers without the credential is admitted as an ordinary interactive prompt. The worker authorization is a process-level relay credential scoped to its reason: home-workspace turns ride the per-workspace registration; CROSS-workspace classification additionally requires the explicitqwen.daemon.channelBoundRelaymarker and a primary/trusted target, because bound delivery is cross-workspace (Key decision 3) but a plain prompt carrying a foreign worker token must stay an ordinary interactive prompt. A forged or unregistered token is still dropped at the route and again at the bridge admission strip. Every response body is consumed or cancelled and every body read is bounded by an abort signal: an unread undici body holds its connection until GC, and a stalled mid-body read would otherwise hang the poll loop. - Dormant wake: the bindings route lists persisted active sessions, but the prompt route only serves live ones; after a daemon restart a bound session is dormant and answers 404. On 404 the accessor restores the session through the same resume path the channel bridge uses for its own dormant sessions (workspace cwd taken from the cached bindings, refreshed once if unknown), then retries the prompt once. The restore mints a client registration; the accessor releases it (detach) once the retried prompt settles — a non-empty registration blocks the daemon's prompt-settled idle close, so discarding it without detaching would pin the session live until the TTL reaper. When the cwd is unknown and the refresh returns a partial snapshot that omits the session, the accessor defers with a backpressure error (
BoundSessionUnavailableError) instead of classifying the 404 as gone or consuming the failure budget: absence from an incomplete enumeration is not evidence of deletion. - Wake error classification: a 404 restore (session record gone, or the workspace itself unregistered — the restore route answers a distinguishable
workspace_not_found404 for permanent absence, while reversible draining/transitioning states keepworkspace_mismatch/workspace_runtime_unavailable), an archived session (409session_archived, excluded from the candidate set by contract), a superseded session (409worktree_session_superseded— a worktree reset severs it permanently, never restored again), and the interrupted-worktree shapes (409worktree_marker_missing/worktree_reset_interrupted, whose repair is a worktree-reset retry rather than a resume retry, so deferring them would dead-letter without ever falling back) becomeBoundSessionGoneError. Queue-full, temporarily-unavailable workspaces/sessions (503, the daemon's own prompt-tier rate limit 429 and other 5xx, restore conflicts, session limits, an armed worktree-reset prompt barrier 409worktree_reset_active— the reset flow tells channel-worker callers to retry), bindings-refresh failures, and a 404 on the retry AFTER a successful wake (the owner may have begun draining between the resume and the retry, and the prompt route answers the same reversible 404 for a non-active owner) are backpressure or transient errors so the adapter defers or retries instead of degrading into public channel delivery — the next wake's resume reports a permanent 404/409 if the session is truly gone. The direct prompt path classifies these 409 codes the same way as wake (archived/superseded → gone; reset barrier/restore-in-progress/busy → backpressure), plus the unreleased standalone working-directory binding (working_directory_missing, which the daemon marks retryable withRetry-After) as backpressure: the bind/release window around a conversation restore is transient, so the event defers instead of burning the failure budget. An authoritative Gone also evicts the session's cached wake cwd, so a permanent partial snapshot cannot make every later delivery pay a doomed resume round-trip. deliverPromptsurfaces gone, queue-full (BoundSessionQueueFullError) and temporarily-unavailable (BoundSessionUnavailableError) as distinct errors so the adapter can fall back, defer, or retry.- When the option is absent (standalone mode, tests), adapters behave exactly as today.
Layer 3 — GitHub adapter changes
All in packages/channels/github/src/GithubAdapter.ts plus the plugin descriptor in index.ts.
Configuration
Two new management fields:
| Field | Kind | Default | Meaning |
|---|---|---|---|
deliverToBoundSessions |
boolean | false |
Route events for PRs bound to a live session into that session instead of the channel session. |
prWatchEvents |
string-list | [] (off) |
State transitions to watch for bound PRs: ci_failure, merge_conflict, head_change, state_change. |
validateConfig rejects a non-empty prWatchEvents when deliverToBoundSessions is off (the watch set is defined by bindings; watching without delivery has no MVP meaning). The descriptor delegates to the adapter's shared validator (validateBoundSessionConfig), so the management UI and connect() enforce one vocabulary and cannot drift. Both fields are opt-in; existing deployments are unaffected.
Bound-session selection
At the start of each poll, when deliverToBoundSessions is on, the adapter fetches GET /sessions/pr-bindings and builds a map owner/repo#number → session. The key lowercases the repo path on both sides: GitHub URLs are case-insensitive (core's canonicalSessionPrUrl lowercases them), binding URLs may carry any case while notification subject URLs carry the canonical case. A leading www. is stripped on both sides of the host comparison (users paste www.github.com links; core's repoKeyFromWebUrl normalizes the same alias). Binding URL parsing tolerates trailing tab segments (/files, /commits, /checks) because the binding routes accept any http(s) URL naming the PR.
- Keep only bindings whose PR URL host matches the channel's GitHub host (Aone and other-provider bindings are ignored).
- Keep only entries with recorded
statenotmerged/closedfor the active watch set, unless the terminal transition has not been observed yet (see PR state watch); delivery candidates for comment events accept any recorded state (a comment on a just-merged PR still deserves delivery while the binding refresh catches up). - Exclude sessions with
sourceType === 'channel'— channel sessions are reached through the normal channel path, which preserves their public-reply contract. - Multiple candidates: pick the most recently updated session (
updatedAtdesc; for a live session with no persisted record yet the route falls back tocreatedAt, so a fresh live session is never outranked by a stale persisted one; a persisted session's livecreatedAtis not activity — the bridge stamps a fresh one on restore, so merely restoring an idle follower never hands it the PR's events). Deterministic, no broadcast. - A failed refresh keeps the previous map (stale candidates fail at delivery time and fall back); the watch pass is skipped for that poll so a transient failure cannot prune the persisted baseline. A
partialsnapshot is merged into the previous map and binding list — known bindings the incomplete answer failed to list are retained, so private delivery never degrades to the public path on a scan failure — and watch-state pruning is disabled. Retention is per session, not per PR key: a LISTED session's PR set is authoritative (the route returns each listed session's sidecar projection topped up with the bindings the bridge still holds for it, trimmed toSESSION_PR_LIST_LIMIT), so retention never revives a PR that session just unbound; and a retained candidate competes by recency, so a partial answer listing only an older session cannot displace the newer known one. The accessor likewise keeps its session→cwd wake cache on partial answers. - Cold start: until the first snapshot succeeds (complete OR partial — the map is populated from partial answers too), the comment lane is deferred (the cursor does not advance). Claiming delivery against a cold empty map would permanently freeze events onto the public channel path even after the scan recovers; persisted tasks still recover while the lane is deferred. The gate opens on any successful snapshot (
boundSnapshotSeen), not complete-only: a permanent partial (a persistently unreadable sidecar, or a workspace whose runtime is missing) would otherwise stall the lane forever. An EMPTY partial answer is additionally bounded: while no complete snapshot has ever succeeded and the map is empty, consecutive empty-partial answers keep the lane closed for up toMAX_COLD_EMPTY_PARTIAL_POLLS = 30polls (throttled stderr) — an unreadable workspace answering{bindings: [], partial: true}on the first poll after a restart must not route every event publicly and commit the cursor before the real snapshot arrives. At the cap the lane opens anyway (a permanently empty partial must not freeze the channel), and unmatched PR events then take the per-event bounded deferral below instead of the public path. Re-validation and watch pruning still require a complete snapshot (boundSnapshotTrusted). Because a binding refresh that has NEVER succeeded (an old daemon without the route, rotated credentials) would otherwise freeze every notification behind a misleading "keeping previous map" line, the deferral logs a distinct, throttled, actionable message naming the stalled lane and the route to check — the gate itself stays fail-closed. - Absence is never established by an incomplete scan: while
boundSnapshotTrustedis false, an event whose PR has no candidate in the map is NOT routed publicly. It is claimed and deferred as backpressure (no delivery yet, same durable retry andMAX_BOUND_DEFERRALShorizon as every other deferral, dead-letter at the horizon); recovery re-resolves such deferred events against the current map, so a complete snapshot that lands later routes them privately. Known targets are unaffected and proceed immediately. Public routing of an unmatched PR event requires a complete snapshot to have established the absence at least once; issue events are exempt from the whole per-event gate (Key decision 11).
Comment delivery intercept
dispatchEnvelope is the single funnel for accepted comment/body/review/assign events. Before claiming the inbound task it consults the bound-session map:
- Bound session found → the persisted
InboundTaskRecordgainsdelivery: { kind: 'bound-session', sessionId }.runInboundTaskdelivers the event prompt throughboundSessions.deliverPromptinstead ofhandleInbound. Because this bypasseshandleInbound, the bound path runs the samepreflightInbound(group policy, sender gate, pairing — side effects included) before delivering; watch events are exempt (no human sender, operator opt-in). The frozen target is re-validated against the latest complete snapshot at delivery time, so a live rebind retargets the event instead of misdelivering to the old session; on a partial/failed scan the persisted target stands (fail-closed). Dedupe keys and the recoverable-task machinery are unchanged — only the terminal delivery differs. - No bound session → today's path for comment events (
handleInbound→ channel session). Watch events are private-only: they are operator-facing machine state with no pre-existing public path, and a channel session's reply would publish the internal prompt text on the PR (and re-gate it through the sender allowlist). A watch event with no bound target — at delivery, after a gone re-resolution, or at drain time — is logged and retired, never fallen back publicly. - Delivery fails: 404 → the accessor first tries the dormant wake (Layer 2); only a session that cannot be restored surfaces as gone, and the adapter then evicts the dead session from its binding maps (so a partial snapshot cannot retain it by recency and shadow a live follower) and iterates through the PR's remaining live followers from a fresh binding fetch (most recent first, excluding every session already Gone in this attempt); each live candidate is re-delivered in turn (a Gone from a re-delivery hop evicts that candidate and tries the next instead of consuming the failure budget or falling back early — the Gone set bounds the loop by the finite follower count), no candidate left on a COMPLETE snapshot → comment events fall back to
handleInbound, watch events retire with a log; under a partial/failed refresh the comment fallback defers as backpressure until a complete snapshot establishes absence (Key decision 11). Queue-full and temporarily-unavailable sessions/workspaces are backpressure, not failure: they do not consume the retry budget, so a busy or draining session defers the event instead of losing it afterMAX_INBOUND_TASK_ATTEMPTS = 3. Backpressure is bounded (MAX_BOUND_DEFERRALS = 240, ~4 hours at the default poll): beyond the horizon the event is retired to a private dead-letter log (github-bound-deadletter.jsonl) with a stderr alert — surfaced, never silently consumed. If the dead-letter append itself fails, the task is KEPT (the payload is dumped to stderr and the durable record survives for the next poll's retry or operator recovery) — a removal there would leave one log line as the event's only trace. Pure bound backpressure — including the targetless deferrals an incomplete snapshot produces — does not hold the shared notification cursor (one stuck session must not freeze mark-as-read for every unrelated PR); other recoverable tasks still hold it, except watch tasks while reconciliation is blocked (they cannot make progress and do not ride the notification cursor). Non-backpressure bound failures exhaust the 3-attempt cap and likewise retire to the dead-letter log (the private path's exhaustion surface — it posts no public error comment). Acceptance is persisted (deliveredAt) before the task is removed; the mark is an optimization, not the replay guard: every injection carries a stable event identity (source chat + message id) as an admission-sideidempotencyKey, so a crash or write failure in the acceptance window (or a lost 202) replays the POST with the SAME key — even when recovery re-creates the task with a new id — and the daemon deduplicates it into the original admission (Key decision 10). A crash-loop charges one attempt per recovery pickup so it terminates. Recovery after restart honors the persisteddeliveryfield and re-resolves targetless backpressure deferrals against the current map.
The injected prompt reuses the webhook-task prompt shape (buildChannelWebhookPrompt style): event header, explicit "untrusted event data" warning, PR identity (repo, number, title, URL), the lane's Trigger:/Event: lines plus the direct lane's factual Author: details, the comment author/body or event details, and guidance that the reply stays in this session (the session is not under the channel publication contract; it acts on GitHub through its own normal tools if needed). The mention and first-contact lanes supply no Author: metadata line, so the prompt falls back to the envelope's sender name — the session must be able to tell a maintainer's instruction from a drive-by commenter's. Channel-facing publication wording never reaches the bound session: the metadata's publication instructions are dropped, and the aggregate lane's "output exactly <no-reply/>" prefix is replaced with a plain "New comments on the PR" header. No tail is silently consumed: comment content is capped at 6,000 code points with an explicit [prompt truncated: N more characters omitted] notice appended outside the slice; an aggregate batch that drops older comments at the 20-comment cap prepends an omission notice (at the FRONT, so the code-point cap cannot truncate the notice itself); and each aggregated comment's 400-character display cap carries its own [comment truncated: N more characters] marker — every such comment is already recorded in dispatchedComments, so a silent cut at any level would lose it from every surface while the prompt reads complete.
PR state watch
When prWatchEvents is non-empty, pollOnce reconciles durable pending events, then runs a watch detection pass after the binding refresh and before generic inbound recovery and notification processing:
- Watch set: bound PRs from the same binding fetch, filtered to the channel's GitHub host, capped at 30 (most recently updated bindings first) for active polling. When more than 30 PRs are bound, the overflow is logged to stderr (count + excluded keys) so the oldest bindings being deterministically unwatched is surfaced, never silent. A binding recorded
merged/closedby the independent refresh stays polled until the poll itself observes the terminal transition (prState: 'closed'), sostate_changeis never skipped by the refresh winning the race. Once observed, the terminal MARKER is retained without polling for as long as the binding lists the PR — pruning it would re-add the closed PR as a first observation on the next poll (query oscillation). Binding retention and active polling are separate concerns. No bound PRs → zero API calls. Persisted entries are pruned against the full candidate set (not the capped watch set, so PRs rotating out of the cap keep their baseline), only when the snapshot is complete — a partial snapshot never prunes — and never while the entry still holds undelivered pending events. Because a permanent partial never prunes, retention is bounded separately: at most 1024 unlisted sessions and 1024 idle watch entries are kept (oldest-first eviction, surfaced on stderr for both caps), so a permanently unscannable workspace cannot grow adapter state without bound. Watch-entry eviction additionally skips entries a retained binding still claims or an in-flight watch task still owns; an evicted entry's PR has no delivery target, and a later rebind re-baselines silently like any first observation, so eviction bounds memory without losing a deliverable event. A dropped session's unmatched events defer (an evicted binder is unlisted, so absence is not authoritative) and surface in the dead-letter at the horizon. - Per PR:
pulls.getevery poll (head SHA,mergeable,mergeable_state,state,updated_at). Check runs (checks.listForRef) and combined commit status (repos.getCombinedStatusForRef) are fetched only whenci_failureis subscribed AND: the head SHA changed since the last poll, no CI observation exists yet, the last observed CI aggregate was pending AND the 5-minute pending re-check interval (CI_PENDING_RECHECK_INTERVAL_MS) has elapsed, the PR'supdated_atadvanced, the aggregate isnonewithin a 10-minute grace window after the head was first observed (checks register asynchronously), or the last CI fetch is older than the 10-minute settled-head re-check interval. The pending interval is load-bearing in the other direction: a workflow queued for environment approval or an offline runner can stay pending for hours, and an unbounded per-poll re-fetch would turn the settled-head cost model into ~3,600 CI calls/hour at the watch cap — over budget oncepulls.getis counted. The periodic re-check is load-bearing too: GitHub does NOT bump a PR'supdated_aton check activity (verified against the live API), so a settled head whose check re-run turns red — or CI that appears late on a manualworkflow_dispatch— would otherwise be missed forever. Steady-state cost is one call per PR per poll plus the bounded periodic re-checks. A failed CI fetch is isolated in its own try/catch: CI is a conditional refinement of thepulls.getbaseline, so a persistent Checks-endpoint failure (a classic PAT without Checks permission, an enterprise policy) logs and continues WITHOUT writing a CI state —merge_conflict/state_changestill fire from the data that already arrived, and the last known aggregate stands (writingnonewould re-fireci_failurewhen the endpoint heals). Watch API calls fail fast on conditions a retry cannot fix — credential failure (401), any rate limit (403/429; GitHub does not always send aretry-afterheader on a secondary/abuse limit) and permanent per-PR statuses (404/410/451 — the PR was deleted, transferred, or the repo became unavailable) — with no blocking cooldown sleep. The shared conditions (401/403/429) abort the WHOLE pass at the failing PR: walking the rest of the set would issue one doomed request per watched PR per poll for the whole reset window; per-PR permanent statuses stay isolated so one deleted PR does not stop the others. The fail-fast predicate is watch-local so the publication lane keeps its own retry semantics, and the notification lane still runs in the same poll. - Transition detection against persisted watch state (
channelFilePath('github-pr-watch.json'), pruned against the full candidate set on complete snapshots — see item 1):ci_failure: aggregate turns to failure (any completed check run withfailure/timed_out/action_required/startup_failureconclusion, or a nonempty combined status reportingfailure/error). An empty combined status (total_count: 0, which GitHub returns asstate: "pending"for SHAs without legacy statuses) never counts as active CI. Fires once per head SHA via the durablefiredring, even if the aggregate flaps back to success and fails again; a new head that fails fires again. A per-entryciFiredShapin backs the ring: the capped ring can evict the per-headci_failurekey after 64 non-ci episodes on a long-lived head, and the pin preserves the once-per-head guarantee even after that eviction (it resets when the head changes). The event summary names the failed checks (up to 5) so the session can act without re-querying.merge_conflict:mergeable_stateentersdirty. Edge-triggered;dirty → clean → dirtyfires again.mergeable === null(GitHub still computing) never overwrites the last known state.head_change: head SHA differs from the last observed SHA (first observation initializes silently). A force-push revisit of a recent SHA is a new transition and fires again.state_change: PRstateleavesopen(merged or closed). First observation initializes silently; close → reopen → close fires again.- Dedupe key identity: every watch key bakes in the PR number and the detection timestamp (
watch:<type>:<pr>:<seq>:<detectedAt>;watch:ci_failure:<pr>:<sha>:<detectedAt>). The PR number scopes the key because the delivery idempotency key carries only chat + message id, and two PRs of one repo bound to the same session issue parallel episode counters. The timestamp keeps instances unique across episode-counter resets (prune, corruption recovery): the daemon's idempotency ring outlives the worker, so a re-issued bare counter key would dedupe into the old admission and silently swallow the new transition.ci_failurestill fires once per head SHA — the fired ring and pending queue match its keys by PR+SHA prefix — while the other three key on the episode counter, a transition INSTANCE rather than a state value, so the legitimate recurrences above are never suppressed by the fired ring. The counter is persisted with the baseline. An event key that would exceed the prompt route's 200-character cap is replaced by a deterministic hash of the same identity (github:h:<sha256-40>), so retries still dedupe instead of burning the failure budget on 400s. Watch identities are instance-local by design: two channel configurations watching the same repository each deliver their own watch notifications (at-least-once per subscription) — the daemon ring dedupes retries of ONE delivery, not independent subscriptions; comment events still dedupe across instances via their stable GitHub ids.
- Durable event queue: detected events are appended to a
pendingqueue persisted in the same write as the new baseline, so a crash between detection and claim loses nothing. A reconciliation pass drains ALL pending entries (no GitHub calls) BEFORE generic recovery and independently of the live watch pass: a failing or rate-limited API pass cannot starve already-detected events, and recovery cannot deliver-and-remove a task that a later drain would re-claim (a crash between the claim and the pending→fired save leaves both records on disk). When reconciliation ITSELF fails (e.g. the watch file is unreadable), generic recovery skips watch tasks entirely until a reconciliation succeeds — delivering and removing an accepted watch task with its pending record still unreconciled on disk would let a later drain claim and deliver the same transition a second time; unrelated comment recovery keeps running. Blocked watch tasks also stop holding the shared notification cursor: they cannot make progress while reconciliation fails, and watch events are detected from PR state rather than notifications, so freezing mark-as-read for them would stall every unrelated PR until the sidecar recovers; once reconciliation succeeds they hold the cursor again until their bounded retries settle them. Draining then, per event: re-validate the recipient against the latest complete snapshot, claim the inbound task (durable — a terminal failed/cancelled record for a recurring WATCH key is replaced, never silently reused; comment/event/issue-body keys are stable identifiers that never re-fire, so their terminal records stay a permanent dedupe barrier — replacing them would revive a cancelled comment after a crash leaves the cursor behind the cancel), move the key frompendingto a cappedfiredring (durable), then run the task. A crash at any point leaves either a pending event or a claimed task; thefiredring is the durable once-per-transition record (task-level dedupe alone would let a flapping aggregate re-fire the same head after the completed task is removed). A failed claim leaves the event queued for the next poll. The drain stops at the first failed delivery so a later event cannot jump ahead of an earlier retrying one (per-PR ordering); pending and fired are already persisted, so stopping loses nothing. The ordering holds across passes too: while an earlier watch transition of the same PR still has an unsettled task, the whole entry's drain is skipped (the watch pass re-drains after reconcile, and the next poll's reconcile runs before recovery) — a task whose key equals the pending head is that event's own crash-window claim and drains normally. A claim persistence failure propagates out of the drain so reconcile fails and the recovery guard holds. The per-entry pending queue is capped at 64: a persistently failing claim cannot grow it without bound; overflow retires the OLDEST queued event to the dead-letter, and a failed dead-letter write keeps it queued for the next detection. A failed pending→fired save AFTER a successful claim rolls the claim back (removes the task, restores the pending event) and retries next poll, so a watch-file-unwritable / task-file-writable divergence cannot deliver the same transition twice across a restart. Event tasks usesource.messageId = watch:<eventType>:<pr>:<headSha|seq>:<detectedAt>and are delivered privately like comments — but with no public fallback (see Comment delivery intercept). - Binding state upkeep stays with the existing 5-minute refresh timer; the adapter does not PATCH metadata. Once the timer records
merged/closedAND the watch has observed the terminal transition, the PR leaves active polling; its terminal marker is retained until the binding itself disappears. - Persistence failure semantics: a transient read error (EACCES/EMFILE/EIO) fails CLOSED — the watch pass is skipped and nothing is saved until the state loads again, so a degraded in-memory state can never overwrite the intact file. Only ENOENT (first start) and persistent JSON corruption start fresh; a syntactically valid file whose individual entries are malformed is repaired in place — each bad entry is dropped (with a stderr line) and the healthy entries survive, so one corrupt value cannot starve the API-independent drain of the entries after it. The per-entry shape check validates pending events with the SAME envelope and delivery validators the task store applies at claim time: a shape-valid envelope missing
chatIdwould otherwise throw in the binding-key lookup and abort the whole reconcile loop, and a partial envelope would be claimed verbatim, rejected on the next task-file read, and wedge the store until an operator deletes the file. Failed atomic writes clean up their UUID-named tmp file so sustained disk pressure cannot accumulate orphans. The state file and directory are mode 0600/0700 (they persist private comment envelopes).
API budget
Default 60 s poll, cap 30 watched PRs: steady state ≤ 1 800 calls/hour, plus the periodic settled-head re-check (≤ 2 calls per PR per 10-minute interval → ≤ 360/hour) and the bounded pending re-check (≤ 2 calls per PR per 5-minute interval → ≤ 720/hour worst case with every watched PR pending). CI-active bursts add 1 combined-status call plus ceil(checkRuns/100) check-list pages per affected PR per re-check — a 300-check monorepo head costs 4 calls, so large-CI bursts can still exceed the 5 000 calls/hour token budget; the watch then aborts the pass on the shared rate-limit/credential failure (one doomed call, not one per PR) and retries next poll rather than blocking. Notification polling overhead is unchanged. The adapter logs and skips the watch pass when the binding fetch fails (notifications still flow).
Key decisions
- Prompt injection, not channel attach.
activateManagedSessionwould bind the followed session's outbound target to the GitHub thread, so every final reply of an interactive session would be published as a public PR comment (publication contract). Injection keeps replies in the session and leaves GitHub actions to the session's own judgment and tools. - In-channel state watch, not webhooks. The watch reuses the channel's existing poll loop and credentials; detection happens at the API layer and costs zero model tokens until a real transition. Webhook push would need a public tunnel plus repository webhook creation (operator consent) and a rework of the static webhook targets. The watch is polling, but it is the gate-probe shape the maintainer accepts: cheap detection outside the model, model only on change.
- Reverse lookup lives in the daemon. Bindings span workspaces; the worker is workspace-scoped and already talks to the daemon over authenticated HTTP. A read-only process-global route reuses the refresh timer's scan pattern instead of duplicating sidecar access in the worker.
- One candidate, most recently updated. Broadcasting to every bound session multiplies model cost and produces duplicate GitHub actions; the freshest non-archived, non-channel session is the one most likely to hold the working context. The proposed Bound-Session Action Templates design generalizes this to one winner per role when
boundSessionActionsis configured; with the field absent, this decision stands verbatim. - Channel sessions excluded from candidates. A channel-created session bound via
/reviewbackfill must keep its public-reply contract; the normal channel path already serves it. - Opt-in defaults. Public-repository events driving an interactive session is a new trust surface; operators enable it explicitly, and comment sender gates still apply before delivery.
- Wake dormant sessions instead of falling back. E2E showed that after a daemon restart the bound session is dormant and the prompt route 404s; treating that as "gone" silently diverted every post-restart event into public channel delivery — breaking the feature's core private-delivery promise. The accessor therefore restores the session (the same resume path the channel bridge already uses) and retries once; only an unrestorable session falls back. The restore mints a client registration that the accessor explicitly detaches once the retried prompt settles: a non-empty registration blocks the daemon's prompt-settled idle close, so without the detach the woken session would stay pinned live until the TTL reaper (or forever with the reaper disabled).
- Watch events are private-only. A comment has a pre-existing public path to fall back to; a watch event does not. Falling a watch event back to a channel session would publish operator-facing machine state ("CI failed on head X. Inspect the failed checks…") as a public PR comment and re-gate it through the sender allowlist. Maintainer decision (2026-11-02, resolving the triage/verification escalation): a watch event with no bound target is logged and retired, never published. The retirement is surfaced (stderr + task removal), not a silent drop.
- Bounded backpressure with a dead-letter surface. Queue-full and unavailable conditions defer without consuming the failure budget, but the deferral horizon is bounded (~4 hours) and pure backpressure — bound deliveries AND the targetless deferrals an incomplete snapshot produces — does not hold the shared notification cursor: one stuck session must not freeze mark-as-read for every unrelated PR, and a permanently stuck session must not wedge a task forever. Events that exhaust the horizon or the failure budget retire to a private dead-letter log: the private path posts no public error comment, so the dead-letter is its exhaustion surface. A failed dead-letter write keeps the task instead of removing it.
- Admission-side idempotency, not a bigger acceptance window. The durable
deliveredAtmark can only protect crashes AFTER it lands; the acceptance write itself can fail (EIO) or the 202 can be lost, and the daemon mints a fresh prompt id per request, so a retry used to re-inject the same event. Moving the mark before the POST would trade a duplicate for a silent loss. Instead the POST carries a stable event identity (source chat + message id) as anidempotencyKey— stable across task re-creation, which a task-scoped key would not be — and the prompt route deduplicates admitted keys per session (bounded in-memory ring, 128 keys per session, 1024 sessions, FIFO): a retry returns the original admission without enqueueing. The ring's lifetime equals the in-memory prompt queue's — a daemon restart loses both together, making re-injection the correct recovery there. Either cap can also evict a key while its queue is still live — the per-session cap in the ordinary case of one long-lived session admitting more than 128 distinct events, the session cap when more than 1024 sessions hold keys — and a retry after such an eviction re-injects. The mark remains as a fast path that retires the task without a round-trip. For aggregate comment batches the message id is a content hash of the sorted member ids (agg:<sha256-40>), not the first comment: a crash before the cursor save can re-list a SUPERSET batch, and a first-comment key would let the daemon ring dedupe the grown batch into the original admission, silently dropping the new members; identical batches still dedupe across the acceptance crash window. The cursor'sdispatchedCommentsring backs this with window-aware eviction: each key carries its comment'screated_at(dispatchedCommentTimes, captured at fetch), and once past the soft cap (500) eviction drops only keys whose comment can no longer be re-listed (created_at <=the committed cursor) — a still-re-listable key (or a legacy key with no recorded time) is protected, so a frozen cursor cannot slide an already-replied comment out of the ring and let a re-listed superset re-publish it under a fresh hash. There is deliberately no hard-cap refusal: refusing dispatches without freezing the cursor would mark-read the refused events (silent loss), and freezing the cursor would deadlock (the ring can only shrink once the cursor advances). The trade-off is that a sustained mark-read outage grows the protected ring with the re-list window (bounded by traffic × outage duration); it shrinks again once mark-read recovers. Recovery re-records a still-pending task's keys (restoring dedupe); a re-record without a captured time stays protected, the conservative direction. The auxiliary time map is itself bounded (MAX_COMMENT_TIMES = 8192, oldest non-ring entries dropped first, ring keys' times always retained) and pruned every poll in afinallyaround the mark-read commit — after a successful commit against the ADVANCED cursor, and after a failed commit against the FROZEN cursor — so a frozen cursor with zero dispatch (a sender-rejected stream, or mark-read itself failing) cannot grow it without bound. The publication audit's consumption verdict is the LAST outcome per source key: a terminalfailed(a provable no-write) afterpostingleaves the event retryable, while an ambiguous failure (failed_ambiguous— the write may have landed) and apostingwith no terminal record (crash mid-post) consume at-most-once, since a second public reply would be irreversible; an unreadable audit freezes the poll's cursor rather than skipping events past a mark-read, and every dispatch lane honors the refusal before recording its dedupe key. The audit WRITE is best-effort by contrast: an unwritable audit store does not block the public reply (a full or corrupted audit file must not break the reply contract — pinned by the "keeps successful publication when its audit write fails" test), so a storage failure spanning thepostingrecord, the POST, and the task removal can leave no durable consumption intent and let a later recovery re-publish. Under total storage failure no durable intent can be recorded at all, so the write path trades strict at-most-once for reply availability while the read path stays fail-closed; making thepostingwrite fail-closed (write-ahead) would restore at-most-once at the cost of blocking replies whenever the audit store is unwritable. The freeze is enforced at DISPATCH time: a recovery pass holding only watch tasks reads neither public store (watch events are never published, so they can match neither a pending delivery nor an audit key), and a private watch retire completes even while the audit is unreadable — the next public dispatch still refuses and freezes. - An incomplete scan never establishes absence. A partial bindings answer — especially an EMPTY one after a restart — is not evidence that a PR is unbound, and public routing is irreversible (the cursor commits in the same poll). So the cold gate defers empty-partial answers for a bounded number of polls, and unmatched PR events defer per-event (recoverable, dead-letter at the horizon) instead of taking the public path whenever absence is not authoritative — before the first complete snapshot, and during ANY partial answer or failing refresh. A partial answer cannot prove absence even when every OBSERVED binder is listed and excludes the PR: an unscannable workspace may hold a never-observed follower, and a listed tombstone proves only that the listed session unbound (it still stops private delivery and watching immediately). Known targets proceed immediately. Issue events are exempt from the entire PER-EVENT absence gate, including its
!boundSnapshotTrustedcold-start clause: bindings are PR-only and GitHub's issue/PR number space is exclusive, so an issue event can never have a bound target — its absence is provable without any snapshot, and once the lane is open it must not stall (or dead-letter) behind a degraded binding scan, including a permanent partial where a complete snapshot never lands. The lane-level cold-start gate is separate and still holds issue events with every other notification until the first successful binding lookup (see the Cold start bullet above). The bounds keep a permanently degraded deployment from freezing: the lane opens at the cap and PR events surface in the dead-letter rather than vanishing.
Files affected
| Layer | File | Change |
|---|---|---|
| core types | packages/core/src/services/session-pr-service.ts |
additive readSessionPrsDetailed (absent vs invalid sidecar distinction); readSessionPrs delegates, unchanged contract |
| daemon route | packages/cli/src/serve/routes/session-pr-bindings.ts (new) + registration |
GET /sessions/pr-bindings sidecar scan route (readdir + readSessionPrsDetailed, persisted/live attribution) with empty-prs tombstones; an unreadable sidecar or transcript → partial |
| daemon projection | packages/cli/src/serve/server/session-list.ts, packages/acp-bridge/src/bridgeTypes.ts |
sidecar enrichment leaves prs unset for an unreadable sidecar instead of projecting an empty list (the earlier prsReadFailed flag had no reader once the snapshot read sidecars itself and was removed) |
| daemon route | packages/cli/src/serve/routes/session.ts |
restore route: distinguishable workspace_not_found 404 for permanent workspace absence; prompt route: relay-marker-scoped cross-workspace worker authorization + admission-side idempotencyKey dedupe |
| daemon admission | packages/cli/src/serve/channel-worker-prompt-authorization.ts |
process-level relay predicate isChannelWorkerPromptToken + explicit CHANNEL_BOUND_RELAY_META_KEY intent marker for cross-workspace classification |
| bridge admission | packages/acp-bridge/src/session-control-plane.ts |
three prompt-admission failures (missing session, closing session, reserved standalone working-directory guard) converted from return Promise.reject(...) to synchronous throw, so the route can never answer a phantom 202 (Key decision 10) |
| channel base | packages/channels/base/src/ChannelBase.ts, types.ts, bound-session-delivery.ts (new), index.ts |
optional boundSessions option + PrBindingSummary type + shared delivery error classes (gone / queue-full / unavailable) |
| channel base | packages/channels/base/src/PollingChannelBase.ts |
poll-error path sanitizes the error text (sanitizeLogText, 200-code-point cap) in both the stderr line and the recorded channel event — shared base of every polling channel |
| worker | packages/cli/src/commands/channel/daemon-worker.ts |
implement the accessor with the SDK client |
| adapter | packages/channels/github/src/GithubAdapter.ts, index.ts |
config fields, selection map, delivery intercept, state watch, watch-state persistence |
| docs | docs/users/features/channels/github.md, docs/developers/qwen-serve-protocol.md |
document the two fields, event semantics, limits, the two stall states, the restore 404/400 split, and idempotencyKey |
| web shell | packages/web-shell/client/components/channels/ChannelEditorDialog.tsx, client/i18n.tsx |
label/description keys (EN + ZH) for the two new descriptor fields |
| tests | collocated .test.ts for each changed file, except session-control-plane.ts (covered by packages/acp-bridge/src/bridge.test.ts) |
see validation |
Constraints and risks
- Untrusted content into an interactive session. Event payloads are marked untrusted in the injected prompt (same contract as webhook tasks). Comment sender gates apply unchanged. State events have no human sender; they exist only because the operator opted in, and they are private-only (no public fallback). The target session's own approval mode governs its actions.
- Process-level relay credential (widened worker authorization). Bound delivery is cross-workspace, so the prompt route cannot keep the old home===target equality for the worker token. The widening is scoped to its reason: cross-workspace channel classification requires the explicit
qwen.daemon.channelBoundRelaymarker IN ADDITION to a registered worker token and a primary/trusted target, and a plain prompt carrying a foreign worker token gets no channel classification and no caller-supplied display projection. Residual risk, stated plainly: the marker is intent, not authentication — a holder of workspace B's worker token can set it and relay into a trusted workspace A, opting that turn out of loop-detected rejection and supplying its display text. The security boundary remains token possession plus the target's trust (the same trust model the bindings route enumerates); a daemon serving an internal interactive workspace alongside a workspace whose channel ingests attacker-controlled text extends that channel's reach into the trusted workspace's sessions. Operators who cannot accept that should not register channel workers on the same daemon, or should keep the ingestion workspace untrusted (untrusted non-primary workspaces are already excluded from both the bindings scan and the relay gate). - Queue pressure. A burst of events can fill the target's 5-slot prompt queue; queue-full and temporarily-unavailable conditions are backpressure (they do not consume the retry budget) and delivery retries on later polls. The deferral horizon is bounded (
MAX_BOUND_DEFERRALS, ~4 hours); beyond it — or when non-backpressure failures exhaust the 3-attempt cap — the event retires to the private dead-letter log with a stderr alert. Pure bound backpressure does not hold the shared notification cursor; other recoverable tasks still hold it, except watch tasks while reconciliation is blocked (they cannot make progress and do not ride the notification cursor). No bound event is silently consumed: it is delivered, deferred, or dead-lettered. - Rate limits. The watch caps, conditional check fetches and the bounded periodic/pending re-checks keep the steady state under the 5 000 calls/hour token budget (large-CI bursts can exceed it; see API budget). A shared credential/rate-limit failure (401/403/429) aborts the whole watch pass with a log — one doomed call, not one per watched PR — and notifications are unaffected, rather than sleeping out the reset window ahead of the notification lane. The daemon's own prompt-tier 429 is backpressure for delivery (defer, never burn the failure budget).
- Stale bindings. A dormant bound session is woken by the accessor (Layer 2), so daemon restarts do not divert events into public channel delivery; the cold-start gate defers the comment lane until the first successful snapshot populates the binding map, and empty-partial answers keep it closed for a bounded number of polls (Key decision 11). Unmatched PR events defer recoverably instead of routing publicly whenever absence is not authoritative (no complete snapshot yet, or any partial answer or failing refresh); issue events are exempt from this per-event gate — bindings are PR-only, so their absence needs no snapshot (the lane-level cold-start gate still holds them with every other notification until the first successful lookup). A binding whose session was truly deleted fails the wake → comment events fall back to the channel path, watch events retire with a log; the next poll's binding fetch heals the map. Under a partial/failed scan the comment fallback defers as backpressure until a complete snapshot establishes absence — an unscannable workspace may still hold a never-observed follower, and the public reply would be irreversible. An observed unbind (a scanned session listed with an empty PR set) stops private delivery and watching even on a partial answer — the tombstone row makes "scanned, no bindings" distinguishable from "not enumerated" — while public routing for the PR still waits for a complete snapshot.
- Closed sessions.
DELETE /session/:idcloses a session without deleting its record; the record stays resumable and bound, so such sessions are still woken for delivery (verified in E2E). Only record deletion (POST /sessions/delete) or archiving removes a session from the candidate set (an archived session's restore answers 409session_archived, classified as gone for delivery). Closing is a resource action, not an unsubscribe; the binding expresses the follow intent. - Scan cost. The route reads the PR sidecars per call: one directory listing per workspace plus, per readable sidecar, the membership head read, the sidecar read and the list-item projection (about four opens and two stats); at current scale this is milliseconds, and the cost scales with the sessions that have ever bound a PR (tombstones included) rather than with the whole session catalog. The listing is a snapshot, so the answer is complete regardless of session count (an earlier revision paginated the activity-ordered session list and had to flag every multi-page scan partial, which made any workspace with more than one page of active sessions permanently partial). An in-memory index invalidated on sidecar writes remains a possible follow-up if the per-poll reads ever matter.
Validation plan
- Unit: route scan (trusted/untrusted, archived exclusion, sidecar corruption tolerance); adapter selection (host filter, state filter, channel-source exclusion, recency tie-break); delivery intercept (bound → inject, unbound → channel path, 404 fallback, queue-full retry, recovery honors persisted delivery); watch transitions (each event type, dedupe per head, null-mergeable guard, first-observation silence, state-before-dispatch ordering); config validation.
- E2E (daemon + local build): bind a PR to a session via PATCH metadata; simulate events with a stubbed GitHub API layer; assert the bound session receives exactly one injected prompt per transition, replies stay in-session, and an unbound PR still creates a channel session. Plan details in
.qwen/e2e-tests/github-channel-bound-session-events.md.
Acceptance criteria
- With both fields enabled, a new comment on a bound PR arrives as a prompt in the bound session (not the channel session), and the session's reply is not auto-posted to GitHub.
- A CI failure on the bound PR's head, a transition to
mergeable_state: dirty, a head advance, and a merge/close each produce exactly one injected prompt per transition instance, surviving daemon restarts without re-firing; legitimate recurrences (dirty → clean → dirty, close → reopen → close, ahead_changeSHA revisit, a new head failing) fire again, and a settled head whose check re-run turns red is caught by the periodic re-check.ci_failureis once per head SHA within the fired-ring window — a revisited SHA's CI failure is suppressed while that head's key remains in the ring, andhead_changecarries the revisit itself. - With no bound session on a complete snapshot, comment events behave exactly as today (channel session created/reused, public reply contract intact); under a partial snapshot or a failing refresh the fallback defers until a complete snapshot establishes absence (Key decision 11); watch events are logged and retired, never published.
- With the fields disabled (default), behavior and API call volume are unchanged.
- Queue-full, rate-limited (429), and unavailable paths defer without losing events and without freezing the shared cursor; session-gone comment paths fall back without loss; watch paths retire privately; exhausted events are dead-lettered (and a failed dead-letter write keeps the task); within a daemon's lifetime, no crash window replays an accepted injection — a retry after a lost acceptance mark or a lost 202 carries the same idempotency key and deduplicates into the original admission; dedupe is best-effort under the two FIFO caps (128 keys per session, 1024 sessions), and the ring dies with the in-memory prompt queue, so after a restart re-injection is the correct recovery (Key decision 10) — and none silently consumes a transition; an empty partial cold snapshot never routes events publicly, and an observed unbind stops private delivery even on a partial answer.
Open questions
None. Webhook push delivery and an in-memory binding index are recorded as future work, not blockers.