Commit graph

8 commits

Author SHA1 Message Date
易良
9a4e924cf1
fix(github-channel): retry definite no-write deliveries (#8087)
* fix(github-channel): retry definite no-write deliveries

* fix(github-channel): preserve concurrent pending deliveries

* fix(github-channel): harden pending retry updates

* fix(github-channel): avoid duplicate pending retries on reconnect

* fix(github-channel): audit recovered deliveries before cleanup

* fix(github-channel): avoid duplicate recovered comments

* fix(github-channel): skip malformed audit entries

* fix(github-channel): bound pending retry recovery

* fix(serve): restore session service import

* fix(serve): remove unused session service import

* fix(github-channel): harden pending delivery recovery

* docs(github): update channel state paths

* fix(github-channel): audit ambiguous pending retries

* fix(github-channel): guard legacy state migration

* fix(github-channel): avoid pending delivery id collisions
2026-07-31 08:06:28 +00:00
易良
f005f3eee8
feat(github-channel): add transient working reaction (#8061)
* feat(channels): acknowledge GitHub requests

* fix(channels): remove transient GitHub reactions

* test(github-channel): cover reaction failures

* test(github-channel): cover pending reaction dedup

* docs(github-channel): clarify final-only output

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-07-30 03:00:29 +00:00
易良
3d5924bd2f
fix(github-channel): validate and document reasonFilter (#8035)
* feat(github-channel): add reasonFilter config to skip unwanted notification reasons

Adds an optional `reasonFilter` allowlist to the GitHub channel config.
When set, notifications whose `reason` is not in the list are skipped
before any lane dispatch, reducing unnecessary API calls and agent work
for notification types the operator does not care about.

- New `reasonFilter?: string[]` field on `GithubConfig`
- O(1) Set lookup (`reasonFilterSet`); undefined = no filter (all reasons)
- Early-skip in the poll loop, before subject URL extraction and lane dispatch
- Two tests: filtered reasons skipped, unset filter processes all

Default behavior is unchanged (undefined = process all reasons).

* fix(channels): log github reason filter skips

* fix(channels): validate github reason filter

* fix(channels): address github reason filter comments

* fix(github-channel): reject invalid reason filters

* fix(github-channel): validate reason filters on connect

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
2026-07-30 01:17:44 +00:00
Nothing Chan
a771e4449e
fix(channels): reject unusable GitHub self-allowlists (#8055) 2026-07-29 15:48:56 +00:00
Mark Xian
c19d321d1f
feat(github-channel): filter notification reasons (#8031) 2026-07-29 15:34:16 +00:00
易良
dc2f61d910
feat(channels): dispatch GitHub notifications by reason (#7826)
* feat(channels): dispatch GitHub notifications by reason

Route each GitHub notification by notification.reason into one of five
lanes, instead of dispatching every new comment regardless of trigger:

- mention: only dispatch comments that actually @ the bot (noise reduction)
- review_requested (PR): fetch PR meta via pulls.get and dispatch a
  review-specific prompt, even with no new comments
- assign: fetch issue meta and dispatch a triage-specific prompt
- author/comment: aggregate the window's new comments into one check-and-
  respond prompt
- other reasons: generic fallback (current behavior)

Add cursor dedup via dispatchedComments (by comment node_id) and
dispatchedNotifications (by notification id), surviving a
markNotificationsAsRead failure that leaves the cursor un-advanced.

Closes #7807

* fix(channels): mark review_requested/assign envelopes as mentioned

GroupGate defaults to requireMention: true, which silently drops
isMentioned:false envelopes as 'mention_required'. The review_requested
and assign lanes are explicit directed triggers — the bot was asked to
review or assigned — equivalent to a mention, so set isMentioned: true
so they pass the gate instead of being inert on the documented default
config.

Addresses review Critical on #7826.

* fix(channels): resolve github routing review comments

* fix(channels): dedupe github meta lane comments

* fix(channels): conditional assign framing for PR threads

The assign route already detected PR threads to use pulls.get, but the
trigger framing text always read 'assigned to this issue' even for PRs.
Make it conditional so PR assignments read 'assigned to this pull request'.

* fix(channels): dedup meta lane dispatch inputs

* fix(channels): simplify GitHub reason dispatch

* fix(channels): respect mention gate for github aggregate lane

* fix(channels): truncate aggregate comment bodies by code points

Match the code-point-aware truncation already used for meta-lane bodies
so a supplementary-plane emoji at the MAX_COMMENT_CHARS boundary is not
split into a lone surrogate.

* fix(channels): harden GitHub dispatch failures, event window, and framing (#7826)

- Classify deleted/transferred subjects (404/410) as terminal so a single
  dead notification is logged and skipped instead of wedging the batch's
  mark-read and cursor advance every poll.
- Widen the review_requested/assign event search to the newest ~100 events
  by merging the preceding page when the last page is partial, instead of
  inspecting only the last page (which can hold a single event).
- Move the aggregate lane's untrusted-data warning to the head of the prompt
  text so it precedes the comment text it describes (metadata is appended
  after text by ChannelBase).
- Add regression tests: permanent-failure two-poll advance, terminal 404
  no-retry, multi-page event search, prompt caps, and the no-actor guard.

* fix(channels): drop lastReadAt filter in findMetaTrigger, add review coverage (#7826)

* fix(github): keep aggregate and meta windows bounded

* fix(channels): apply windowSince lower bound in findMetaTrigger (#7826)

* fix(channels): bound retry wedge, compute aggregate isMentioned, fix pairing pre-filter (#7826)

* fix(github): record dispatch before handler

* fix(github): persist skipped notifications

* fix(github): close dispatch retry loss cases

* test(github): cover cursor trim and meta floor validation

* fix(github): simplify notification reason dispatch

* fix(github): preserve batched dispatch comments

* fix(github): restore direct event dedup

* fix(github): preserve directed mention context

* fix(github): keep review fixes scoped

* fix(github): preserve delayed direct triggers

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
2026-07-28 14:05:21 +00:00
OrbitZore
4895726600
fix(channels): use username as senderId in GitHub adapter to fix allowlist gate (#7727)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
* fix(channels): use username as senderId in GitHub adapter to fix allowlist gate

isAuthorizedForSharedSessionTarget compares config.allowedUsers (logins)
against envelope.senderId — but senderId was a numeric ID resolved via
getByUsername, so every allowlisted user was rejected from /who, /clear,
/status, /loop, and channel memory commands.

Fix by using user.login as senderId throughout, assuming GitHub users
don't change usernames. This also removes the getByUsername resolution
step that made connect() non-idempotent on daemon reconnect.

- Remove botUserId field; bot self-filter uses botUsername
- Remove allowedUsers login-to-ID resolution in connect()
- Pass config.allowedUsers logins directly to gate
- senderId in envelopes uses user.login

* fix(channels): normalize allowlist/senderId to lowercase for case-insensitive matching

GitHub logins are case-insensitive but Set.has/Array.includes are not.
Without normalization, allowedUsers: ['Alice'] silently rejects a
commenter whose canonical login is 'alice' — a regression from the old
getByUsername round-trip which normalized casing implicitly.

- Normalize config.allowedUsers and gate to lowercase in connect()
- Lowercase senderId at both envelope assignment sites
- Remove dead != null guard in bot self-comment filter
- Add case-insensitive gate test and connect idempotency test
- Add senderId/allowedUsers comparability guard to dispatch test
- Document username-based allowlist rename risk in security section
2026-07-26 00:21:22 +00:00
OrbitZore
62e009a952
feat(channels): GitHub polling adapter with notification-as-wakeup architecture (#7632)
* feat(channels): add GitHub polling adapter with notification-as-wakeup architecture

Introduce a GitHub channel adapter that monitors notifications and
responds to @mentions on issues/PRs by posting comments. Uses
last_read_at as a per-thread watermark for comment enumeration,
replacing the unreliable latest_comment_url approach.

Foundation changes to ChannelBase:
- sendThreadMessage for thread-targeted delivery (IM adapters unchanged)
- Envelope.metadata appended to prompt after command parsing
- chat_thread session scope (channel:chatId:threadId) prevents
  cross-repo session collision
- polling-helpers: testBotMention/stripBotMention (separate detection
  from stripping, no whitespace collapsing), cursor persistence,
  abortableSleep

GitHub adapter design:
- Notifications as wake-up signals only (unread filtering)
- listComments enumeration with last_read_at watermark
- Bot self-comment filtering, case-insensitive mention regex
- In-memory recentlyProcessed set for mark-read failure dedup
- First-contact: new issue body @bot triggers processing
- Error comment + cursor advance on handleInbound failure
- pollInterval minimum 60s, exponential backoff 2s-30s

* refactor(channels): extract PollingChannelBase from polling-helpers

Replace the loose polling-helpers module with a PollingChannelBase<Cursor>
abstract class that encapsulates the poll loop, cursor persistence (JSON,
atomic write), exponential backoff, and start/stop lifecycle. Subclasses
implement only pollOnce() and createInitialCursor().

- Delete polling-helpers.ts (cursor fns + abortableSleep moved into base)
- Move mention utilities (testBotMention/stripBotMention) to github pkg
- GithubAdapter now extends PollingChannelBase<{ lastProcessedAt }>

* fix(channels): remove Gitea/GitLab mention from sendThreadMessage JSDoc

* fix(channels): match /pulls/N in notification subject URL

GitHub PR notifications use /repos/{owner}/{repo}/pulls/{N} in
subject.url, not /issues/{N}. The regex only matched /issues/,
causing PR notifications to be skipped and marked read.

Also sets threadId to 'pr:N' for PRs (was always 'issue:N').

* test(channels): add PR body first-contact unit test

Verify that PR notifications with @mention in the body (not a comment)
correctly trigger the first-contact path: extractFromSubjectUrl matches
/pulls/N, listComments returns empty, tryFirstContactBody fetches the
PR body and dispatches to handleInbound with threadId 'pr:N'.

* feat(channels): read pollInterval from channel config in PollingChannelBase

Move pollInterval config reading from GithubAdapter to the base class.
The user's configured pollInterval in settings.json is now respected
directly without a minimum enforcement. Defaults to 60000ms when not
configured.

* fix(channels): prepend metadata before prompt text

Agent sees issue/PR context (type, title, URL) before the user's
request, improving comprehension. Metadata is still appended after
slash-command parsing so commands are not affected.

* refactor(channels): route all ChannelBase delivery through sendThreadMessage

Replace all internal sendMessage calls with sendThreadMessage, passing
envelope.threadId (or target.threadId / undefined) so polling adapters
can deliver to the correct thread. IM adapters are unaffected — the
default sendThreadMessage falls through to sendMessage.

* docs(channels): document sendThreadMessage delivery architecture

* fix(channels): address review findings

- Cap recentlyProcessed Set at 10k entries to prevent unbounded growth
- Validate cursor JSON shape (non-null object) in loadCursorFromDisk
- sendThreadMessage falls through to sendMessage when threadId is
  undefined instead of silently dropping
- Remove duplicate pollInterval from GithubConfig (now in ChannelConfig)
- Fix chat_thread routing key trailing colon when threadId is undefined

* docs(channels): fix metadata JSDoc — prepended, not appended

* fix(channels): use recentlyProcessed dedup for first-contact body

Replace the fragile createdAt-vs-cursor check in tryFirstContactBody
with the recentlyProcessed set. The cursor advances globally based on
notification updated_at — when a different notification with a later
updated_at is processed first, the cursor can advance past the issue's
created_at, causing the first-contact check to incorrectly skip the
issue body (forget reply bug, found in E2E TC-2b).

* refactor(channels): two-layer dedup for GitHub adapter

Layer 1: global cursor filters notifications by updated_at (sorted
ascending, old first). Layer 2: server-side last_read_at filters
comments by created_at (sorted ascending).

- Delete recentlyProcessed Set (no longer needed)
- Sort notifications by updated_at ascending before processing
- Sort comments by created_at ascending before processing
- Pass latest comment created_at to markThreadAsRead as last_read_at

* fix(channels): address review findings on GitHub adapter

Blockers:
- sessionScope: add defaultSessionScope to ChannelPlugin, apply in
  parseChannelConfig so router and adapter agree on 'chat_thread'
- channel-registry.test.ts: add 'github' to expected type list

Should-fix:
- Replace per-thread markThreadAsRead (PATCH) with bulk
  markNotificationsAsRead (PUT /notifications + last_read_at).
  API errors stop the batch without marking failed notifications
  read; handleInbound errors still advance (error comment posted).
- connect() throws on bot identity failure instead of failing open
- metadata appended after promptText (inside sender attribution)
- isSharedSessionTarget includes 'chat_thread' scope

Nits:
- startPollLoop re-entrancy guard
- clean-package-build-artifacts.js includes github
- index.ts re-exports GithubChannel

* fix(channels): use max updated_at of all fetched notifications as last_read_at

Prevents re-fetching the same notifications in the next poll cycle.
The bulk PUT /notifications marks all fetched notifications as read
up to the max updated_at, regardless of per-notification success.

* fix(channels): address review round 2 findings

- #12: loadCursorFromDisk rejects arrays
- #13: pollInterval validates positive finite number
- #19: first-contact gate uses dispatchedMention flag (not newComments.length)
- #25: stripBotMention no longer trims (preserves indentation)
- #27: remove adapter-level requireMention, unify on GroupGate
- #31: add chat_thread SessionRouter routing key tests
- #33: clear metadata on collect-mode synthetic envelope
- #35: fix PollingChannelBase.test import path
- #36: add @octokit/rest to 15-channel-adapters.md dependencies

* docs(channels): document known limitations for GitHub adapter

- First start skips existing unread notifications (cursor = now)
- Requires classic PAT (fine-grained PATs lack notifications API)
- PR review comments not enumerated (issue comments only)

* fix(channels): address review round 3 findings

- #9: buildMetadata derives web URL from baseUrl (GHE support)
- #12: sendThreadMessage throws on invalid threadId format
- #19: mention lookbehind matches cc:@bot and "@bot" patterns
- #23: cursor file name uses sha256 hash to prevent collision
- #26: test verifies cursor persistence to disk
- #31: postErrorComment double-failure logs to stderr
- #45: tests use mkdtempSync isolation instead of real QWEN_HOME

* fix(channels): pass threadId through pairing flow + sendResponseMessage test

- #13+16: onPairingRequired receives envelope.threadId and passes it
  to sendThreadMessage, so pairing codes are delivered on threaded
  channels (GitHub) instead of throwing
- #6: add test verifying sendResponseMessage resolves threadId from
  router.getTarget and passes it to sendThreadMessage

* fix(channels): pass proxy to Octokit for daemon-worker environments

- #44: read this.proxy from ChannelBaseOptions and pass
  HttpsProxyAgent to Octokit request.agent, matching the
  Telegram adapter pattern

* fix(channels): address review findings — immutable senderId, comment time window, validateCursor, retry wrapper

- senderId uses immutable user.id; allowedUsers resolved to IDs at connect
- Comment filter upper bound: updated_at <= maxUpdatedAt (batch window)
- Per-notification errors use continue (best-effort), not break
- validateCursor() virtual hook for subclass cursor shape validation
- sendThreadMessage/postErrorComment wrapped in githubApi() retry
- webOrigin handles default api.github.com → github.com
- Docs: classic PAT only, markNotificationsAsRead, dedup claims removed
- Tests: threadId priority, metadata consumption, defaultSessionScope,
  QWEN_HOME isolation, persistent mock rejection

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(channels): mark notifications read before processing to prevent duplicate replies

Bot's own replies bump notification updated_at past the pre-captured
maxUpdatedAt, so markNotificationsAsRead(maxUpdatedAt) failed to mark
them read — the next poll re-fetched the same comments and replied
again.

Move markNotificationsAsRead + cursor advance before the processing
loop (best-effort delivery). This is safe because bot's own comments
do not flip notifications back to unread. Update docs to reflect the
new poll cycle order and best-effort semantics.

* fix(channels): update sender gate after allowedUser ID resolution and harden tests

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(channels): cursor-based comment window to prevent duplicate replies

PUT /notifications is async (202) with a last_read_at cutoff — the
bot's reply bumps updated_at past the cutoff before the server
processes the mark, so the notification is never marked read and gets
re-fetched on the next poll, causing duplicate replies.

Use the cursor value before advancement as an exclusive lower bound
for the comment enumeration window: (windowSince, maxUpdatedAt].
Comments already eligible in a previous poll are excluded regardless
of whether the mark succeeded. Zero new persistent state.

* fix(channels): cursor-based comment window to prevent duplicate replies

PUT /notifications is async (202) with a last_read_at cutoff — the
bot's reply bumps updated_at past the cutoff before the server
processes the mark, so the notification is never marked read and gets
re-fetched on the next poll, causing duplicate replies.

Use the cursor value before advancement as an exclusive lower bound
for the comment enumeration window, with per-notification last_read_at
as the preferred lower bound when available (server-side per-thread
watermark). Comments already eligible in a previous poll are excluded
regardless of whether the mark succeeded. Zero new persistent state.

* fix(channels): address review findings — null guard, cursor validation, metadata dedup, abortable sleep, docs

- Guard against null notification.subject.url in pollOnce
- Validate lastProcessedAt is a parseable date in validateCursor
- Add metadata: undefined to second collect-mode drain path
- Refactor abortableSleep as protected method on PollingChannelBase
- Fix docs: requireMention is nested under groups.*
- Add tests: chat_thread shared session, dispatchedBodies eviction,
  cursor enumeration window, last_read_at in mention tests

* docs(channels): sync docs with implementation — cursor shape, error handling, GitHub adapter tables, first-contact

- Design doc: update Cursor to { lastProcessedAt, dispatchedBodies? }, add
  validateCursor date check, abortableSleep protected method, break-on-error
  semantics, subject.url null guard
- Developer docs: add GitHub to adapter table and adapter matrix
- User guide: add first-contact step to How It Works, clarify mark-before-process

* fix(channels): address review round 2 — error dedup, abortable retry, backoff reset, window test

- Record dispatchedBody on first-contact handleInbound failure to prevent
  duplicate error comments when mark-read async hasn't taken effect
- Use abortableSleep instead of raw setTimeout in githubApi retry so
  disconnect() can interrupt rate-limit cooldowns
- Reset consecutiveErrors in startPollLoop so stop/restart cycles don't
  inherit stale elevated backoff
- Add test for cursor window client-side lower-bound exclusion filter

* fix(channels): address review round 3 — cursor validation, error dedup, sender gate, bot-self body

- validateCursor: normalize falsy non-array dispatchedBodies (false/0/""/null)
  to [] instead of passing them through to .includes() which throws TypeError
- Set dispatchedMention after postErrorComment to prevent first-contact from
  posting a duplicate error comment on the same thread
- Only set dispatchedMention when the sender passes the sender gate, so a
  disallowed commenter's mention no longer suppresses a valid first-contact
  body from an allowed issue author
- Skip bot-authored issue bodies in tryFirstContactBody to prevent
  self-response loops under open sender policy

* fix(channels): address review suggestions — test coverage, cursor filename, assertion precision

- Pairing flow: add threadId pass-through regression test
- pollInterval: add table-driven edge cases (0, -1, NaN, Infinity, string)
- Add null-URL notification followed by valid notification batch test
- Fix comment window test to assert paginate call 3 (listComments) not call 2
- Truncate cursor filename encoded prefix to 200 chars (filesystem 255 limit)
- Assert mark-read uses batch maxUpdatedAt, not just { read: true }
- Assert real GitHub plugin declares defaultSessionScope chat_thread
- Add invocationCallOrder assertion for mark-before-process ordering

* fix(channels): address review round 4 — allowedUsers throw on resolve failure, crash table fix, mark-read failure test

* fix(channels): address review round 5 — created_at filter, retry-after NaN guard, retry/sendThreadMessage tests, docs fixes

* fix(channels): address ci-bot review 4778587403 — reconnect idempotency, github type enumerations, retry/webOrigin tests

* chore(channels): align channel-github version to 0.21.0 after upstream merge

* chore(channels): update package-lock.json for channel-github 0.21.0

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: OrbitZore <orbitzore@users.noreply.github.com>
2026-07-25 09:31:50 +00:00