mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
feat(channels): add GitHub repository event filter with allowlist and blocklist (#16493)
* feat(channels): add GitHub repository event filter with allowlist and blocklist The GitHub channel processed notifications from every repository the authenticated account can see. The only repository-level control was the group access allowlist, which cannot express "everything except these repositories" and hides a plain event filter behind access-policy machinery. Add repoFilterMode (off / allowlist / blocklist) and repoFilter to the GitHub plugin. The filter runs at notification intake, before the reason filter, journals capped repo_filter skip rows plus one collapsed count row, and never holds the poll cursor. One shared validator rejects an active mode with an empty or malformed list at save time and at connect time; while the mode is off a stored list is ignored, so it can be parked without blocking unrelated saves. Notifications about bound PRs are filtered too; only their PR state watch events bypass the filter. The Web Shell editor gains a Repository Filter select and a Filtered Repositories list shown only while a mode is active, with the visibility and validation gated on the same predicate. Pasted repository or PR URLs are shortened to owner/repo on save; non-repository GitHub URLs are rejected instead of silently shortened. A parity table binds the editor mirror to the plugin's own validator, and the visuals suite captures the new editor in both themes. * fix(channels): close review findings on the repository event filter Address the verify-round findings and the triage review notes on #16493: - F1 (Medium): the reserved-path guard missed sibling GitHub areas, so pasted URLs like github.com/apps/renovate or /features/copilot were still silently shortened to nonexistent repositories that both validators accepted — fatal under an allowlist. Extend the guard with the measured missing roots plus the other known two-segment areas, export the set, and replace the sampled fixture with an enumerated test over every member plus explicit regression pins for the reported shapes. Removing a listed root now fails the pins; the denylist's best-effort nature is documented instead of overclaimed. - F2 (Low): pin the documented ordering — a notification excluded by both gates is journaled repo_filter, not reason_filter. The verify round's M7 mutation (swapping the two blocks) survived the old suite and now fails this test. - Triage boundary note: pin that a bound PR's state watch events bypass the repository filter, so a future refactor of the watch call site cannot silently silence bound sessions. - Docs: soften the URL-rejection claim to "known non-repository areas" in the user guide and both design docs, and state the residual denylist risk. * fix(channels): close the ten inline review findings on the repository filter - R1-1: close the arbitrary-host class structurally. The paste-shortener now only shortens URLs on the channel's own GitHub host (github.com plus the GHES host derived from baseUrl, api. prefix stripped); api.github.com, gist.github.com, gitlab.com and any other host pass through for validation to reject. Add the missing advisories/copilot areas, a snapshot assertion over the whole reserved set (removing any member now fails, which the enumeration alone could not witness), and foreign-host fixtures. - R1-2: seed paginate before connect() in the rejection helper so a successful connect no longer prints an unasserted poll-error backoff line. - R1-3: dialog case opening on a stored active mode — the list shows the stored text and a re-save keeps both keys. - R1-4: pin the notification side of case-insensitive matching (mixed-case subject URL vs lowercase entry) and the normalizer's trim/empty handling. - R1-5: pin the complement of the watch bypass — a notification about a bound PR in a blocklisted repository is filtered, not delivered. - R1-6: capitalized reserved-root variant pins the guard's own lowercasing. - R1-7: vocabulary test binding the repoFilterMode descriptor options and default to the exported REPO_FILTER_MODES, mirroring prWatchEvents. - R1-9: two-poll journal budget reset test (42 rows, two counted rows). - R1-10: extend the Off-retirement case with the discriminating switch back to Allowlist, where a stale error would remount. - R1-11: typing a valid entry (or correcting a malformed one) now retires the shown instruction through refreshErrors, with two no-save dialog cases; Submit remains the only error population site. Mutation witnesses on this head: dropping chatId lowercasing, the host gate, the list-typing retirement branch, and the refreshErrors guard each fail exactly their new test; all restored byte-identical. * fix(channels): exclude the channel's own API host and paths from URL shortening Round-3 review findings on #16493, all closed in this commit: - R1-1 (Critical, fix-induced): the host gate re-admitted the channel's own API host — with baseUrl set, a pasted REST URL shortened to `repos/octo` (api.github.com) or `api/v3` (GHES /api/v3), phantom repositories both validators accepted. Only the WEB host now qualifies: an api.-prefixed baseUrl host contributes only its stripped form, and paths under the baseUrl's own API prefix are excluded. Genuine GHES web URLs still shorten. Pinned with baseUrl actually passed through validation, for both REST shapes; re-admitting the raw API host reddens the test. - R2-1: the baseUrl argument now has a call-site witness on both paths — a GHES draft validates clean and saves ['octo/repo'] through buildChannelUpsertRequest, mirrored in the parity table against the daemon authority. Dropping either argument reddens its half. - Deferred notes (all three closed, none deferred onward): trailing sentence punctuation is stripped before matching (a pasted `…/octo/repo.` no longer becomes the phantom `octo/repo.`); `enterprises` joins the reserved set (snapshot updated); and a dialog case witnesses the parked-list round trip — hidden while Off, restored with its text on re-enable. Mutation witnesses: re-admitting the raw API host, dropping the validator baseUrl argument, and dropping the save-path baseUrl argument each fail exactly their new test; all restored byte-identical.
This commit is contained in:
1 parent
752e399b3b
commit
f2e9a3fc66
14 files changed
+2136
-5
No files matched your search
@@ -0,0 +1,221 @@
|
||||
# GitHub Channel Repository Event Filter (Allowlist / Blocklist)
|
||||
|
||||
[English](github-repo-filter.md) | [简体中文](github-repo-filter.zh-CN.md)
|
||||
|
||||
Status: implemented in this change.
|
||||
|
||||
## Problem
|
||||
|
||||
The GitHub channel processes notifications from every repository the
|
||||
authenticated account can see. The only repository-level control today is
|
||||
`groupPolicy: "allowlist"` plus `groups` keys: an operator who wants "everything
|
||||
except these noisy repositories" has no way to express it, and an operator who
|
||||
wants a simple event filter must understand the access-policy machinery
|
||||
(group gates, sender policies, per-repository overrides) to get one.
|
||||
|
||||
In the Web Shell editor the repository allowlist lives under "Group Policy →
|
||||
Allowlist → Allowed repositories". There is no blocklist, and the access-policy
|
||||
framing makes a plain "filter events by repository" task unintuitive.
|
||||
|
||||
## Current state
|
||||
|
||||
- Inbound events arrive through the notification poll loop in
|
||||
`packages/channels/github/src/GithubAdapter.ts` (`pollOnce`). Each
|
||||
notification's subject URL yields `chatId = owner/repo`; `reasonFilter`
|
||||
already skips notifications there, journals a capped `reason_filter` skip
|
||||
row per poll plus one collapsed count row, and lets the cursor advance.
|
||||
- Repository access is gated later, per envelope, by `GroupGate`
|
||||
(`groupPolicy` + `groups`). The editor renders repository rows only for this
|
||||
access path.
|
||||
- A second event source, PR state watch (`runPrStateWatch`), serves PRs that a
|
||||
user explicitly bound to a live session. It does not flow through the
|
||||
notification loop.
|
||||
- The daemon store (`channel-settings-store.ts`) validates every config key
|
||||
against the plugin descriptor and runs the plugin's synchronous
|
||||
`validateConfig` for cross-field rules. The editor mirrors those rules in
|
||||
`channel-editor-state.ts`; `channel-editor-parity.test.ts` binds the two
|
||||
sides with the plugin's own descriptor.
|
||||
|
||||
## Goals
|
||||
|
||||
- Filter inbound GitHub events by repository with two explicit modes:
|
||||
allowlist (only listed repositories) and blocklist (all except listed).
|
||||
- Make the filter a first-class, self-explanatory pair of fields in the Web
|
||||
Shell editor, with repository-format validation and URL-paste tolerance.
|
||||
- Keep the runtime, the save-time validator, and the editor mirror in sync
|
||||
through one shared validation function and a parity table.
|
||||
|
||||
## Scope boundaries
|
||||
|
||||
- The filter applies to every incoming notification, including notifications
|
||||
about PRs bound to a live session. Only PR state watch events
|
||||
(`runPrStateWatch`) — explicit per-PR subscriptions created by the user in
|
||||
a session — bypass it. The field description states this.
|
||||
- `groupPolicy` / `groups` keep their access-control meaning unchanged. The
|
||||
repository filter is an additional intake filter; when both are active the
|
||||
effective set is their intersection.
|
||||
- No change to other channels, to `GroupGate`, or to the shared `GroupPolicy`
|
||||
type.
|
||||
|
||||
## Proposed solution
|
||||
|
||||
### Configuration (GitHub plugin)
|
||||
|
||||
Two new flat descriptor fields, placed after `reasonFilter`:
|
||||
|
||||
- `repoFilterMode`: enum `off | allowlist | blocklist`, default `off`.
|
||||
- `repoFilter`: string-list of `owner/repo` entries.
|
||||
|
||||
```json
|
||||
{
|
||||
"type": "github",
|
||||
"useLocalGh": true,
|
||||
"groupPolicy": "open",
|
||||
"repoFilterMode": "blocklist",
|
||||
"repoFilter": ["octo/noisy-repo", "octo/archive"]
|
||||
}
|
||||
```
|
||||
|
||||
Flat fields (not one object field) because the Web Shell editor cannot edit
|
||||
`object`-kind fields.
|
||||
|
||||
### Runtime (`GithubAdapter.ts`)
|
||||
|
||||
- `validateRepoFilterConfig(config)` — exported pure function returning a
|
||||
string error or `undefined`. Rules:
|
||||
- `repoFilterMode` absent or `off`: the list is ignored entirely (a stale
|
||||
list never blocks a save or a connect while the filter is off).
|
||||
- `repoFilterMode` set to `allowlist`/`blocklist`: `repoFilter` must be a
|
||||
non-empty array of strings, each matching `^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`
|
||||
after trim + lowercase. An active allowlist with an empty list would
|
||||
silently drop every event, so it is rejected instead.
|
||||
- Any other mode value is rejected.
|
||||
- `normalizeRepoFilter(config, name)` calls the same validator and throws on
|
||||
error; `connect()` stores `{ mode, repos: Set<string> } | null` beside
|
||||
`reasonFilter`.
|
||||
- In the poll loop, immediately after `extractFromSubjectUrl` and before
|
||||
`reasonFilter`: allowlist skips repositories not in the set, blocklist skips
|
||||
repositories in the set (case-insensitive). A skipped notification is
|
||||
logged to stderr, emitted as a `repoFilter.skip` debug payload, and
|
||||
journaled as `inbound / skipped / repo_filter` with the same per-poll cap
|
||||
(20 rows) and collapsed count row as `reason_filter`. The cursor advances
|
||||
past filtered notifications exactly as it does for reason-filtered ones.
|
||||
- `index.ts` `validateConfig` calls `validateRepoFilterConfig`, so the daemon
|
||||
rejects an invalid combination at save time with the same message the
|
||||
runtime would throw.
|
||||
|
||||
### Editor (`web-shell`)
|
||||
|
||||
- `channel-editor-state.ts`:
|
||||
- `normalizeRepositoryFilterInput(value, baseUrl)` — splits on commas,
|
||||
trims, strips trailing sentence punctuation a paste can carry, lowercases,
|
||||
shortens GitHub web URLs (`https://github.com/owner/repo`, with optional
|
||||
`.git`, trailing slash, query, fragment, or `/pull/123`-style suffix) to
|
||||
`owner/repo`, deduplicates, joins with `, `. Two gates decide whether a
|
||||
URL is shortened at all. Structurally, only URLs on the channel's own
|
||||
GitHub WEB host qualify: `github.com` plus the web host derived from
|
||||
`baseUrl` for GitHub Enterprise — an `api.`-prefixed baseUrl host
|
||||
contributes only its stripped form, never the raw API host, and paths
|
||||
under the baseUrl's own API prefix (e.g. `/api/v3/`) are excluded, so a
|
||||
pasted REST URL cannot shorten to `repos/octo` or `api/v3`. Any other
|
||||
host — `gist.github.com`, `gitlab.com`, an arbitrary site — passes
|
||||
through unchanged for validation to reject, which closes the "first two
|
||||
segments of any host" class. On a qualified
|
||||
host, a URL whose first path segment is a reserved GitHub area (`orgs`,
|
||||
`apps`, `settings`, `trending`, ...) still passes through unchanged so
|
||||
validation rejects it instead of silently filtering a nonexistent
|
||||
repository. That area list is best-effort by construction — a denylist
|
||||
of GitHub's top-level paths cannot be complete, and a future area can
|
||||
reintroduce the hazard until it is added. A snapshot assertion pins the
|
||||
whole exported set (removing any member fails it), an enumerated test
|
||||
keeps every listed root rejected, and explicit regression pins cover the
|
||||
reported shapes including a capitalized root. Other non-URL entries
|
||||
pass through unchanged so validation can name them.
|
||||
- Validation (github only): when `repoFilterMode` is active, an empty list
|
||||
is `repoFilterRequired`; entries that still fail the repository regex
|
||||
after normalization are `invalidRepoId`. When the mode is `off`, the list
|
||||
is not validated, matching the runtime's ignore-while-off rule and the
|
||||
conditional-field visibility below.
|
||||
- `buildChannelUpsertRequest` normalizes the github `repoFilter` value with
|
||||
the same function before sending.
|
||||
- `initialFieldValue` seeds `repoFilterMode` with `off` for existing
|
||||
instances whose config lacks the key (the generic existing-instance path
|
||||
would otherwise leave the select empty).
|
||||
- `ChannelEditorDialog.tsx`: the list field is rendered only while the mode
|
||||
is active (progressive disclosure). Visibility and validation are gated on
|
||||
the same predicate, so a hidden field can never strand a save. The draft
|
||||
keeps the list text while the mode is `off`, so re-enabling restores it.
|
||||
- i18n (en + zh): field labels ("Repository Filter" / "Filtered repositories"),
|
||||
mode option labels (Off / Allowlist — only listed repositories / Blocklist —
|
||||
all except listed), descriptions (including the URL-paste note and the
|
||||
bound-PR-watch boundary), placeholder `owner/repo, owner/repo`, and the two
|
||||
validation messages.
|
||||
|
||||
### Parity
|
||||
|
||||
`channel-editor-parity.test.ts` gains a repo-filter table: the same configs
|
||||
feed the editor mirror and `plugin.management.validateConfig`, using the
|
||||
plugin's own descriptor (active mode + empty list, malformed entry, valid
|
||||
allowlist/blocklist, off + stale list).
|
||||
|
||||
## Design decisions and rationale
|
||||
|
||||
- **Separate intake filter, not a new `groupPolicy` value.** `GroupPolicy` is
|
||||
shared by every channel and enforced by `GroupGate`; adding `blocklist`
|
||||
there would change access-control semantics across all platforms. An
|
||||
adapter-level filter is GitHub-only, sits where `reasonFilter` already sits,
|
||||
and composes with any group policy.
|
||||
- **Explicit `off` mode instead of "empty list means off".** An empty
|
||||
allowlist must not silently mean "block everything" or "filter disabled";
|
||||
the mode makes the intent explicit and lets the runtime reject the
|
||||
ambiguous combination.
|
||||
- **Ignore the list while off.** Operators can park a list, switch the filter
|
||||
off, and save unrelated edits without the parked list blocking the save —
|
||||
the same tolerance the store already grants unchanged numeric values.
|
||||
- **Normalize URLs in the editor, keep the runtime strict.** Pasting a
|
||||
repository or PR URL is the natural gesture; the editor shortens it on
|
||||
save. `settings.json` hand-edits keep one canonical spelling and get a
|
||||
clear error otherwise.
|
||||
- **Notifications filtered, watch events not.** Every notification is
|
||||
filtered, including notifications about bound PRs — they arrive through the
|
||||
same intake. Bound-PR watch is an explicit user subscription with its own
|
||||
durable state machine; filtering it is a separate decision and stays out
|
||||
of this change.
|
||||
|
||||
## Files affected
|
||||
|
||||
- `packages/channels/github/src/GithubAdapter.ts`, `index.ts`,
|
||||
`GithubAdapter.test.ts`
|
||||
- `packages/web-shell/client/components/channels/channel-editor-state.ts`,
|
||||
`ChannelEditorDialog.tsx`, their tests, `channel-editor-parity.test.ts`
|
||||
- `packages/web-shell/client/i18n.tsx`
|
||||
- `packages/web-shell/client/e2e/visuals/screenshots.spec.ts` (repository
|
||||
filter editor captures)
|
||||
- `docs/users/features/channels/github.md`
|
||||
- `docs/design/github-repo-filter.md`, `docs/design/github-repo-filter.zh-CN.md`
|
||||
|
||||
## Validation plan
|
||||
|
||||
- Unit: adapter normalize/validate rules; poll-loop skip behavior for both
|
||||
modes (case-insensitive match, journal cap and collapsed row, cursor
|
||||
advances); editor normalization/validation/save payload; parity table.
|
||||
- E2E (`.qwen/e2e-tests/github-repo-filter.md`): save both modes through the
|
||||
daemon API against a local GitHub-compatible stub, observe skipped vs
|
||||
processed notifications and the `repo_filter` journal rows; editor
|
||||
rendering checks.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `repoFilterMode: allowlist` processes only listed repositories; `blocklist`
|
||||
processes all except listed; `off`/absent processes everything.
|
||||
- An active mode with an empty or malformed list is rejected at save time
|
||||
(daemon and editor) and at connect time (runtime), with matching messages.
|
||||
- Filtered notifications are journaled as `repo_filter` skips and do not hold
|
||||
the cursor.
|
||||
- The editor shows the repository list only while a mode is active, accepts
|
||||
pasted GitHub URLs, and round-trips stored values unchanged when untouched.
|
||||
- Existing channels without the new keys behave exactly as before.
|
||||
|
||||
## Open questions
|
||||
|
||||
None.
|
||||
@@ -0,0 +1,190 @@
|
||||
# GitHub 频道按仓库过滤事件(白名单 / 黑名单)
|
||||
|
||||
[English](github-repo-filter.md) | [简体中文](github-repo-filter.zh-CN.md)
|
||||
|
||||
状态:已随本次变更实现。
|
||||
|
||||
## 问题
|
||||
|
||||
GitHub 频道会处理认证账号可见的所有仓库的通知。目前唯一的仓库级控制是
|
||||
`groupPolicy: "allowlist"` 加 `groups` 键:想要「除了这些吵闹的仓库以外都处理」
|
||||
的管理员无法表达这个意图;想要一个简单事件过滤器的管理员必须先理解访问策略
|
||||
机制(群门禁、发言者策略、按仓库覆盖)才能配出来。
|
||||
|
||||
在 Web Shell 编辑器里,仓库白名单藏在「Group Policy → Allowlist →
|
||||
Allowed repositories」下面。没有黑名单,而且访问策略的框架让「按仓库过滤
|
||||
事件」这件简单的事变得不直观。
|
||||
|
||||
## 现状
|
||||
|
||||
- 入站事件经由 `packages/channels/github/src/GithubAdapter.ts` 的通知轮询循环
|
||||
(`pollOnce`)到达。每条通知的 subject URL 解析出 `chatId = owner/repo`;
|
||||
`reasonFilter` 已经在那里跳过通知、按轮记录有上限的 `reason_filter` 跳过
|
||||
行外加一条折叠计数行,并让游标继续前进。
|
||||
- 仓库访问在其后由 `GroupGate`(`groupPolicy` + `groups`)按信封把关。编辑器
|
||||
只为这条访问路径渲染仓库行。
|
||||
- 第二个事件源是 PR 状态观察(`runPrStateWatch`),服务于用户在会话里显式
|
||||
绑定到活跃会话的 PR。它不经过通知循环。
|
||||
- daemon 存储(`channel-settings-store.ts`)按插件描述符校验每个配置键,并
|
||||
调用插件的同步 `validateConfig` 做跨字段规则。编辑器在
|
||||
`channel-editor-state.ts` 里镜像这些规则;`channel-editor-parity.test.ts`
|
||||
用插件自己的描述符把两侧绑在一起。
|
||||
|
||||
## 目标
|
||||
|
||||
- 用两种显式模式按仓库过滤入站 GitHub 事件:白名单(只处理列出的仓库)与
|
||||
黑名单(处理列出仓库以外的全部)。
|
||||
- 在 Web Shell 编辑器里把过滤器做成一对一等公民、自解释的字段,带仓库格式
|
||||
校验,并容忍粘贴 URL。
|
||||
- 通过一个共享校验函数和一张一致性表,让运行时、保存时校验器与编辑器镜像
|
||||
保持同步。
|
||||
|
||||
## 范围边界
|
||||
|
||||
- 过滤器作用于每一条入站通知,包括绑定到活跃会话的 PR 的通知。只有 PR
|
||||
状态观察事件(`runPrStateWatch`)——用户在会话里显式创建的按 PR 订阅——
|
||||
绕过它。字段描述会写明这一点。
|
||||
- `groupPolicy` / `groups` 的访问控制含义不变。仓库过滤器是额外的入站过滤;
|
||||
两者同时生效时,有效集合是它们的交集。
|
||||
- 不改其他频道、不改 `GroupGate`、不改共享的 `GroupPolicy` 类型。
|
||||
|
||||
## 方案
|
||||
|
||||
### 配置(GitHub 插件)
|
||||
|
||||
两个新的扁平描述符字段,放在 `reasonFilter` 之后:
|
||||
|
||||
- `repoFilterMode`:枚举 `off | allowlist | blocklist`,默认 `off`。
|
||||
- `repoFilter`:`owner/repo` 条目的字符串列表。
|
||||
|
||||
```json
|
||||
{
|
||||
"type": "github",
|
||||
"useLocalGh": true,
|
||||
"groupPolicy": "open",
|
||||
"repoFilterMode": "blocklist",
|
||||
"repoFilter": ["octo/noisy-repo", "octo/archive"]
|
||||
}
|
||||
```
|
||||
|
||||
用扁平字段(而不是一个 object 字段),因为 Web Shell 编辑器无法编辑
|
||||
`object` 类字段。
|
||||
|
||||
### 运行时(`GithubAdapter.ts`)
|
||||
|
||||
- `validateRepoFilterConfig(config)` —— 导出的纯函数,返回字符串错误或
|
||||
`undefined`。规则:
|
||||
- `repoFilterMode` 缺省或 `off`:完全忽略列表(过滤器关闭时,陈旧列表
|
||||
永远不会挡住保存或连接)。
|
||||
- `repoFilterMode` 为 `allowlist`/`blocklist`:`repoFilter` 必须是非空
|
||||
字符串数组,每项 trim + 小写后匹配
|
||||
`^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`。生效的白名单若列表为空会静默丢掉
|
||||
所有事件,因此直接拒绝。
|
||||
- 其他 mode 取值一律拒绝。
|
||||
- `normalizeRepoFilter(config, name)` 调用同一个校验器,出错即抛;
|
||||
`connect()` 在 `reasonFilter` 旁边存入 `{ mode, repos: Set<string> } | null`。
|
||||
- 在轮询循环里,紧跟 `extractFromSubjectUrl` 之后、`reasonFilter` 之前:
|
||||
白名单跳过不在集合里的仓库,黑名单跳过在集合里的仓库(大小写不敏感)。
|
||||
被跳过的通知写 stderr、发 `repoFilter.skip` 调试载荷,并按
|
||||
`inbound / skipped / repo_filter` 记入日志,沿用与 `reason_filter` 相同的
|
||||
每轮上限(20 行)与折叠计数行。游标越过被过滤的通知继续前进,与
|
||||
reason 过滤完全一致。
|
||||
- `index.ts` 的 `validateConfig` 调用 `validateRepoFilterConfig`,因此
|
||||
daemon 在保存时就以运行时相同的消息拒绝非法组合。
|
||||
|
||||
### 编辑器(`web-shell`)
|
||||
|
||||
- `channel-editor-state.ts`:
|
||||
- `normalizeRepositoryFilterInput(value, baseUrl)` —— 按逗号拆分、trim、
|
||||
剥掉粘贴可能带上的句尾标点、小写,把 GitHub web URL
|
||||
(`https://github.com/owner/repo`,可带 `.git`、尾斜杠、query、fragment
|
||||
或 `/pull/123` 式后缀)缩短为 `owner/repo`,去重后用 `, ` 连接。是否
|
||||
缩短由两道门决定。结构上,只有频道自己的 GitHub WEB 主机上的 URL 才
|
||||
可缩短:`github.com` 加上由 `baseUrl` 派生的 GitHub Enterprise web
|
||||
主机——带 `api.` 前缀的 baseUrl 主机只贡献去前缀后的形式,绝不加入
|
||||
原始 API 主机;baseUrl 自身 API 前缀(如 `/api/v3/`)下的路径也被
|
||||
排除,因此粘贴 REST URL 不会被缩短成 `repos/octo` 或 `api/v3`。其他
|
||||
任何主机——`gist.github.com`、`gitlab.com`、任意站点——原样透传交给
|
||||
校验拒绝,从结构上关闭「任意主机的头两段路径」这一类隐患。在合格
|
||||
主机上,首段路径是 GitHub 保留区域(`orgs`、`apps`、`settings`、
|
||||
`trending` 等)的 URL 仍原样透传,让校验拒绝它,而不是静默地去过滤
|
||||
一个不存在的仓库。这份区域清单在构造上就是尽力而为——对 GitHub 顶层
|
||||
路径做黑名单不可能穷尽,未来新增的区域在被补进清单之前会重新引入该
|
||||
隐患。快照断言钉住整个导出集合(移除任何成员都会红),枚举测试保证
|
||||
每个已列根路径都被拒绝,显式回归钉覆盖报告所列形态(含大写根路径)。
|
||||
其他非 URL 条目也原样通过,让校验能点名它们。
|
||||
- 校验(仅 github):`repoFilterMode` 生效时,空列表为
|
||||
`repoFilterRequired`;归一化后仍不匹配仓库正则的条目为
|
||||
`invalidRepoId`。mode 为 `off` 时不校验列表,与运行时的「关闭即忽略」
|
||||
规则及下面的条件可见性一致。
|
||||
- `buildChannelUpsertRequest` 在发送前用同一个函数归一化 github 的
|
||||
`repoFilter` 值。
|
||||
- `initialFieldValue` 为配置里缺键的既有实例把 `repoFilterMode` 种子值
|
||||
设为 `off`(通用的既有实例路径否则会让下拉框为空)。
|
||||
- `ChannelEditorDialog.tsx`:列表字段只在 mode 生效时渲染(渐进披露)。
|
||||
可见性与校验挂在同一个谓词上,隐藏字段永远不会卡住保存。mode 为 `off`
|
||||
时草稿保留列表文本,重新启用即恢复。
|
||||
- i18n(en + zh):字段标签(「Repository Filter」/「Filtered
|
||||
repositories」)、mode 选项标签(Off / Allowlist — only listed
|
||||
repositories / Blocklist — all except listed)、描述(含粘贴 URL 说明与
|
||||
PR 绑定观察的边界)、占位符 `owner/repo, owner/repo`,以及两条校验消息。
|
||||
|
||||
### 一致性
|
||||
|
||||
`channel-editor-parity.test.ts` 增加一张 repo-filter 表:同一组配置喂编辑器
|
||||
镜像与 `plugin.management.validateConfig`,使用插件自己的描述符(生效
|
||||
mode + 空列表、畸形条目、合法白名单/黑名单、off + 陈旧列表)。
|
||||
|
||||
## 设计决策与理由
|
||||
|
||||
- **独立的入站过滤器,而不是新的 `groupPolicy` 取值。** `GroupPolicy` 由
|
||||
所有频道共享、由 `GroupGate` 执行;在那里加 `blocklist` 会改变所有平台
|
||||
的访问控制语义。适配器级过滤器只属于 GitHub,放在 `reasonFilter` 已经
|
||||
在的位置,并与任何 group policy 复合。
|
||||
- **显式 `off` mode,而不是「空列表即关闭」。** 空白名单不能静默地表示
|
||||
「屏蔽一切」或「过滤关闭」;mode 让意图显式,并让运行时拒绝这个歧义
|
||||
组合。
|
||||
- **关闭时忽略列表。** 管理员可以停放一份列表、关掉过滤器、保存不相关的
|
||||
编辑,而不被停放的列表挡住保存——与存储对未变更数值已有的容忍一致。
|
||||
- **编辑器归一化 URL,运行时保持严格。** 粘贴仓库或 PR URL 是自然手势;
|
||||
编辑器在保存时缩短它。`settings.json` 手编保持唯一规范拼写,否则得到
|
||||
清晰报错。
|
||||
- **通知被过滤,观察事件不被过滤。** 每条通知都被过滤,包括绑定 PR 的
|
||||
通知——它们走同一个入站口。PR 绑定观察是带自己持久状态机的显式用户
|
||||
订阅;要不要过滤它是另一个决策,不在本次变更内。
|
||||
|
||||
## 涉及文件
|
||||
|
||||
- `packages/channels/github/src/GithubAdapter.ts`、`index.ts`、
|
||||
`GithubAdapter.test.ts`
|
||||
- `packages/web-shell/client/components/channels/channel-editor-state.ts`、
|
||||
`ChannelEditorDialog.tsx`、它们的测试、`channel-editor-parity.test.ts`
|
||||
- `packages/web-shell/client/i18n.tsx`
|
||||
- `packages/web-shell/client/e2e/visuals/screenshots.spec.ts`(仓库过滤器
|
||||
编辑器截图)
|
||||
- `docs/users/features/channels/github.md`
|
||||
- `docs/design/github-repo-filter.md`、`docs/design/github-repo-filter.zh-CN.md`
|
||||
|
||||
## 验证计划
|
||||
|
||||
- 单元:适配器 normalize/validate 规则;轮询循环两种模式的跳过行为(大小写
|
||||
不敏感匹配、日志上限与折叠行、游标前进);编辑器归一化/校验/保存载荷;
|
||||
一致性表。
|
||||
- E2E(`.qwen/e2e-tests/github-repo-filter.md`):对着本地 GitHub 兼容桩,
|
||||
通过 daemon API 保存两种模式,观察跳过与处理的通知以及 `repo_filter`
|
||||
日志行;编辑器渲染检查。
|
||||
|
||||
## 验收标准
|
||||
|
||||
- `repoFilterMode: allowlist` 只处理列出的仓库;`blocklist` 处理列出仓库
|
||||
以外的全部;`off`/缺省处理一切。
|
||||
- 生效 mode 配空列表或畸形列表,在保存时(daemon 与编辑器)与连接时
|
||||
(运行时)都被拒绝,消息一致。
|
||||
- 被过滤的通知记为 `repo_filter` 跳过,且不卡游标。
|
||||
- 编辑器只在 mode 生效时显示仓库列表,接受粘贴的 GitHub URL,未触碰的
|
||||
存储值原样往返。
|
||||
- 没有新键的既有频道行为与之前完全一致。
|
||||
|
||||
## 待定问题
|
||||
|
||||
无。
|
||||
@@ -89,6 +89,8 @@ Local `gh` authentication requires an HTTPS `baseUrl` so the daemon host credent
|
||||
| `groupPolicy` | `"disabled"` | Must be `"open"`, `"allowlist"` with the repo (`owner/repo`) listed in `groups`, or `"pairing"` with the repo approved for notifications to flow |
|
||||
| `groups.*.requireMention` | `true` | Require @mentions for ordinary comments; directed notification reasons still run |
|
||||
| `reasonFilter` | unset | Optional allowlist of GitHub notification reasons to process |
|
||||
| `repoFilterMode` | `"off"` | Repository event filter: `"allowlist"` processes only the repositories listed in `repoFilter`, `"blocklist"` processes all except them, `"off"` processes everything. Applies to notification intake only |
|
||||
| `repoFilter` | unset | `owner/repo` repository names used by `repoFilterMode`; required while the mode is `"allowlist"` or `"blocklist"` |
|
||||
| `deliverToBoundSessions` | `false` | Deliver events for PRs bound to a live session into that session as a prompt instead of a channel-owned session (daemon-managed channels only) |
|
||||
| `prWatchEvents` | unset | Optional state transitions to watch for bound PRs: `ci_failure`, `merge_conflict`, `head_change`, `state_change`. Requires `deliverToBoundSessions` |
|
||||
| `boundSessionActions` | unset | Optional operator instruction templates for bound-session delivery, keyed by event class with optional role prefixes (`review:`, `verify:`, `fix:`). Enables role-aware fan-out. Requires `deliverToBoundSessions` |
|
||||
@@ -99,6 +101,25 @@ Valid `reasonFilter` values are `mention`, `review_requested`, `assign`, `author
|
||||
|
||||
Filtered notifications are marked read only after all accepted work in the poll window completes. Removing the filter later will not replay notifications the channel already skipped.
|
||||
|
||||
### Repository event filter
|
||||
|
||||
Use `repoFilterMode` together with `repoFilter` to filter incoming notifications by repository:
|
||||
|
||||
```json
|
||||
{
|
||||
"repoFilterMode": "blocklist",
|
||||
"repoFilter": ["octo/noisy-repo", "octo/archive"]
|
||||
}
|
||||
```
|
||||
|
||||
- `"allowlist"` processes only the listed repositories; `"blocklist"` processes every repository except the listed ones; `"off"` (the default) processes everything.
|
||||
- Repository names are matched case-insensitively and must be written as `owner/repo`. An active mode with an empty or malformed list is rejected at save time and at channel start.
|
||||
- While the mode is `"off"`, a stored list is ignored, so you can park a list without it affecting the channel.
|
||||
- The filter composes with `groupPolicy`: when both restrict repositories, only the intersection is processed. It applies to every incoming notification, including notifications about PRs bound to a live session (`deliverToBoundSessions`); only the PR state watch events (`prWatchEvents`) bypass it.
|
||||
- Filtered notifications are journaled as `repo_filter` skips in the Channel event timeline and do not hold the poll cursor. Like `reasonFilter` skips, they are marked read with the window and are not replayed when the filter changes.
|
||||
|
||||
In the Web Shell editor, **Repository Filter** offers Off / Allowlist / Blocklist, and the **Filtered Repositories** list appears while a mode is active. You can paste repository or PR URLs there; they are shortened to `owner/repo` on save. Only URLs on the channel's own GitHub host are shortened — `github.com`, or the GitHub Enterprise host derived from the API base URL; URLs on any other host are rejected. On that host, URLs under known non-repository GitHub areas (`orgs/`, `apps/`, `settings/`, `trending/`, and similar) are also rejected instead of shortened. That area list is best-effort: a URL from a GitHub area not on the list can still be shortened, so check the saved `owner/repo` entries after pasting.
|
||||
|
||||
### Editing from the Web Shell
|
||||
|
||||
Settings → Channels → GitHub exposes the poll interval under **Connection**, in milliseconds. It is kept between 1 and 86400000 (one day): a larger value is clamped to the ceiling and a smaller positive one is raised to 1, because Node rounds a sub-millisecond timeout to about 1 ms and clamps one above its 32-bit range to 1 ms as well — either would turn the poll loop into a hot loop. Leave it empty for the 60000 ms default. When `groupPolicy` is `"allowlist"`, the **Access control** section also shows one row per allowlisted repository, each with a require-mention control and a speaker control.
|
||||
|
||||
@@ -4370,6 +4370,347 @@ describe('GithubChannel', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('repoFilter', () => {
|
||||
function connectWithRepoFilter(
|
||||
overrides: Record<string, unknown>,
|
||||
): Promise<void> {
|
||||
channel = new TestableGithubChannel(
|
||||
'test-github',
|
||||
makeConfig(overrides),
|
||||
makeBridge(),
|
||||
);
|
||||
// connect() ends in startPollLoop(); seed the first poll so a successful
|
||||
// connect does not print a backoff line from an unseeded paginate mock.
|
||||
mockOctokit.paginate.mockResolvedValueOnce([]);
|
||||
return channel.connect();
|
||||
}
|
||||
|
||||
function notificationFor(repo: string, issue: number) {
|
||||
return makeNotification({
|
||||
subject: {
|
||||
title: 'Test Issue',
|
||||
url: `https://api.github.com/repos/${repo}/issues/${issue}`,
|
||||
type: 'Issue',
|
||||
},
|
||||
repository: { full_name: repo },
|
||||
});
|
||||
}
|
||||
|
||||
it('allowlist processes only listed repositories', async () => {
|
||||
const stderrWrite = vi
|
||||
.spyOn(process.stderr, 'write')
|
||||
.mockImplementation(() => true);
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'allowlist',
|
||||
repoFilter: ['owner/repo'],
|
||||
});
|
||||
try {
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([
|
||||
notificationFor('noise/archive', 7),
|
||||
notificationFor('owner/repo', 42),
|
||||
])
|
||||
.mockResolvedValueOnce([makeComment({ body: 'hello @test-bot' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(1);
|
||||
expect(channel.inboundEnvelopes[0]!.chatId).toBe('owner/repo');
|
||||
expect(stderrWrite).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
'skipping notification (repository=noise/archive excluded by repoFilter allowlist',
|
||||
),
|
||||
);
|
||||
expect(
|
||||
readChannelEvents().filter(
|
||||
(event) => event['reason'] === 'repo_filter',
|
||||
),
|
||||
).toEqual([
|
||||
expect.objectContaining({
|
||||
type: 'inbound',
|
||||
outcome: 'skipped',
|
||||
reason: 'repo_filter',
|
||||
repository: 'noise/archive',
|
||||
thread: 'issue:7',
|
||||
number: 7,
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
stderrWrite.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('blocklist processes all except listed repositories', async () => {
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['noise/archive'],
|
||||
});
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([
|
||||
notificationFor('noise/archive', 7),
|
||||
notificationFor('owner/repo', 42),
|
||||
])
|
||||
.mockResolvedValueOnce([makeComment({ body: 'hello @test-bot' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(1);
|
||||
expect(channel.inboundEnvelopes[0]!.chatId).toBe('owner/repo');
|
||||
expect(
|
||||
readChannelEvents().filter(
|
||||
(event) => event['reason'] === 'repo_filter',
|
||||
),
|
||||
).toEqual([
|
||||
expect.objectContaining({
|
||||
repository: 'noise/archive',
|
||||
outcome: 'skipped',
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it('matches repository names case-insensitively', async () => {
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'allowlist',
|
||||
repoFilter: ['OWNER/Repo'],
|
||||
});
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([notificationFor('owner/repo', 42)])
|
||||
.mockResolvedValueOnce([makeComment({ body: 'hello @test-bot' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('matches a mixed-case notification repository against a lowercase entry', async () => {
|
||||
// The mirror of the config-side case above: GitHub returns the canonical
|
||||
// casing in subject URLs, so the comparison site must lowercase chatId.
|
||||
// Deleting chatId.toLowerCase() there leaves the config-side case green
|
||||
// and fails this one.
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['owner/repo'],
|
||||
});
|
||||
mockOctokit.paginate.mockResolvedValueOnce([
|
||||
notificationFor('Owner/Repo', 42),
|
||||
]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(0);
|
||||
expect(
|
||||
readChannelEvents().filter(
|
||||
(event) => event['reason'] === 'repo_filter',
|
||||
),
|
||||
).toEqual([expect.objectContaining({ repository: 'Owner/Repo' })]);
|
||||
});
|
||||
|
||||
it('trims entries and drops empty ones when normalizing the list', async () => {
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'allowlist',
|
||||
repoFilter: [' Owner/Repo ', '', 'owner/repo'],
|
||||
});
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([notificationFor('owner/repo', 42)])
|
||||
.mockResolvedValueOnce([makeComment({ body: 'hello @test-bot' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(1);
|
||||
expect(
|
||||
readChannelEvents().some((event) => event['reason'] === 'repo_filter'),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('processes every repository when the mode is off or unset', async () => {
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'off',
|
||||
repoFilter: ['owner/repo'],
|
||||
});
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([notificationFor('noise/archive', 7)])
|
||||
.mockResolvedValueOnce([makeComment({ body: 'hello @test-bot' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(1);
|
||||
expect(
|
||||
readChannelEvents().some((event) => event['reason'] === 'repo_filter'),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('advances the cursor past a poll of only filtered notifications', async () => {
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['owner/repo'],
|
||||
});
|
||||
mockOctokit.paginate.mockResolvedValueOnce([
|
||||
makeNotification({ last_read_at: '2026-07-01T12:00:00.000Z' }),
|
||||
]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(0);
|
||||
expect(
|
||||
mockOctokit.rest.activity.markNotificationsAsRead,
|
||||
).toHaveBeenCalledWith({
|
||||
last_read_at: '2026-07-02T10:00:00.000Z',
|
||||
read: true,
|
||||
});
|
||||
expect(channel.cursor.lastProcessedAt).toBe('2026-07-02T10:00:00.000Z');
|
||||
});
|
||||
|
||||
it('journals at most 20 individual skips plus one counted row', async () => {
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'allowlist',
|
||||
repoFilter: ['owner/repo'],
|
||||
});
|
||||
const flood = Array.from({ length: 25 }, (_, index) =>
|
||||
notificationFor(`noise/repo-${index}`, 100 + index),
|
||||
);
|
||||
mockOctokit.paginate.mockResolvedValueOnce(flood);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
const events = readChannelEvents().filter(
|
||||
(event) => event['reason'] === 'repo_filter',
|
||||
);
|
||||
expect(events).toHaveLength(21);
|
||||
const individual = events.filter(
|
||||
(event) => event['detail'] === undefined,
|
||||
);
|
||||
expect(individual).toHaveLength(20);
|
||||
expect(individual[0]).toMatchObject({
|
||||
repository: 'noise/repo-0',
|
||||
thread: 'issue:100',
|
||||
number: 100,
|
||||
});
|
||||
expect(events.at(-1)).toEqual(
|
||||
expect.objectContaining({
|
||||
type: 'inbound',
|
||||
outcome: 'skipped',
|
||||
reason: 'repo_filter',
|
||||
detail: { filtered: 5 },
|
||||
}),
|
||||
);
|
||||
expect(events.at(-1)).not.toHaveProperty('repository');
|
||||
});
|
||||
|
||||
it('resets the journal budget on the next poll', async () => {
|
||||
// The counters live in the poll body, so each busy poll journals its own
|
||||
// 20 individual rows plus one counted row. Hoisting them to instance
|
||||
// fields would yield 22 rows total and fail here.
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'allowlist',
|
||||
repoFilter: ['owner/repo'],
|
||||
});
|
||||
const flood = Array.from({ length: 25 }, (_, index) =>
|
||||
notificationFor(`noise/repo-${index}`, 100 + index),
|
||||
);
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce(flood)
|
||||
.mockResolvedValueOnce(flood);
|
||||
|
||||
await pollOnce();
|
||||
await pollOnce();
|
||||
|
||||
const events = readChannelEvents().filter(
|
||||
(event) => event['reason'] === 'repo_filter',
|
||||
);
|
||||
expect(events).toHaveLength(42);
|
||||
const counted = events.filter(
|
||||
(event) =>
|
||||
(event['detail'] as { filtered?: number } | undefined)?.filtered !==
|
||||
undefined,
|
||||
);
|
||||
expect(counted).toHaveLength(2);
|
||||
expect(
|
||||
counted.every((event) => event['detail']?.['filtered'] === 5),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('journals repo_filter, not reason_filter, when both gates exclude a notification', async () => {
|
||||
// Pins the documented ordering: the repository filter runs before the
|
||||
// reason filter, so a notification excluded by both is journaled under
|
||||
// repo_filter. Swapping the two blocks flips this row to reason_filter.
|
||||
await initWithoutLoop({
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['owner/repo'],
|
||||
reasonFilter: ['mention'],
|
||||
});
|
||||
mockOctokit.paginate.mockResolvedValueOnce([
|
||||
makeNotification({ reason: 'comment' }),
|
||||
]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(
|
||||
readChannelEvents()
|
||||
.filter((event) => event['type'] === 'inbound')
|
||||
.map((event) => event['reason']),
|
||||
).toEqual(['repo_filter']);
|
||||
});
|
||||
|
||||
it('rejects an active mode with an empty list', async () => {
|
||||
await expect(
|
||||
connectWithRepoFilter({ repoFilterMode: 'allowlist', repoFilter: [] }),
|
||||
).rejects.toThrow(
|
||||
'repoFilter for channel test-github requires at least one owner/repo entry when repoFilterMode is "allowlist".',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects an active mode without a list', async () => {
|
||||
await expect(
|
||||
connectWithRepoFilter({ repoFilterMode: 'blocklist' }),
|
||||
).rejects.toThrow(
|
||||
'repoFilter for channel test-github must be an array of owner/repo repository names when repoFilterMode is "blocklist".',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects malformed repository entries', async () => {
|
||||
await expect(
|
||||
connectWithRepoFilter({
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['https://github.com/owner/repo'],
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
'repoFilter entries for channel test-github must be owner/repo repository names: https://github.com/owner/repo',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects non-string entries', async () => {
|
||||
await expect(
|
||||
connectWithRepoFilter({
|
||||
repoFilterMode: 'allowlist',
|
||||
repoFilter: [42],
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
'repoFilter entries for channel test-github must be strings.',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects an unknown mode', async () => {
|
||||
await expect(
|
||||
connectWithRepoFilter({
|
||||
repoFilterMode: 'denylist',
|
||||
repoFilter: ['owner/repo'],
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
'repoFilterMode for channel test-github must be one of: off, allowlist, blocklist.',
|
||||
);
|
||||
});
|
||||
|
||||
it('ignores a parked list while the mode is off', async () => {
|
||||
await expect(
|
||||
connectWithRepoFilter({
|
||||
repoFilterMode: 'off',
|
||||
repoFilter: ['not a repo'],
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
channel.disconnect();
|
||||
});
|
||||
});
|
||||
|
||||
describe('publication contract', () => {
|
||||
const nameHash = createHash('sha256')
|
||||
.update('test-github')
|
||||
@@ -15139,6 +15480,67 @@ describe('GithubChannel bound-session delivery and PR state watch', () => {
|
||||
expect(boundSessions.deliverPrompt).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('delivers watch events for a repository the notification filter excludes', async () => {
|
||||
// Pins the documented boundary: the repository filter governs
|
||||
// notification intake only. A bound PR's state watch events bypass it,
|
||||
// so blocklisting the repository must not silence its watch delivery.
|
||||
await initChannel(
|
||||
{
|
||||
deliverToBoundSessions: true,
|
||||
prWatchEvents: ['ci_failure'],
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['owner/repo'],
|
||||
},
|
||||
{ boundSessions },
|
||||
);
|
||||
mockPull({});
|
||||
mockNotificationsOnce([]);
|
||||
mockChecks([{ status: 'completed', conclusion: 'success' }]);
|
||||
await pollOnce();
|
||||
|
||||
mockPull({ updated_at: '2026-07-02T10:00:00.000Z' });
|
||||
mockNotificationsOnce([]);
|
||||
mockChecks([
|
||||
{ status: 'completed', conclusion: 'failure', name: 'build' },
|
||||
]);
|
||||
await pollOnce();
|
||||
|
||||
expect(boundSessions.deliverPrompt).toHaveBeenCalledTimes(1);
|
||||
expect(boundSessions.deliverPrompt.mock.calls[0]![1]).toContain(
|
||||
'CI failed on head sha-1',
|
||||
);
|
||||
});
|
||||
|
||||
it('filters notifications about a bound PR instead of delivering them', async () => {
|
||||
// The complement of the watch-bypass case: bound-session routing must
|
||||
// not exempt a notification from the repository filter. Moving bound
|
||||
// delivery above the filter (or early-continuing bound threads into it)
|
||||
// delivers this prompt and fails here.
|
||||
await initChannel(
|
||||
{
|
||||
deliverToBoundSessions: true,
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['owner/repo'],
|
||||
},
|
||||
{ boundSessions },
|
||||
);
|
||||
mockNotificationsOnce([makeNotification({ reason: 'mention' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(boundSessions.deliverPrompt).not.toHaveBeenCalled();
|
||||
expect(
|
||||
readChannelEvents().filter(
|
||||
(event) => event['reason'] === 'repo_filter',
|
||||
),
|
||||
).toEqual([
|
||||
expect.objectContaining({
|
||||
repository: 'owner/repo',
|
||||
outcome: 'skipped',
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it('fires ci_failure once per head and not again for the same head', async () => {
|
||||
await initChannel(watchConfig, { boundSessions });
|
||||
mockPull({});
|
||||
|
||||
@@ -40,7 +40,12 @@ import {
|
||||
import { testBotMention, stripBotMention } from './mention.js';
|
||||
import { resolveGithubWebOrigin } from './web-origin.js';
|
||||
|
||||
interface GithubConfig extends ChannelConfig {
|
||||
interface RepoFilterConfigFields {
|
||||
repoFilterMode?: unknown;
|
||||
repoFilter?: unknown;
|
||||
}
|
||||
|
||||
interface GithubConfig extends ChannelConfig, RepoFilterConfigFields {
|
||||
baseUrl?: string;
|
||||
reasonFilter?: unknown;
|
||||
useLocalGh?: boolean;
|
||||
@@ -310,6 +315,89 @@ function normalizeReasonFilter(
|
||||
return reasons.length > 0 ? new Set(reasons) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The repository event filter modes, shared by the descriptor options and
|
||||
* the validator so the UI and the runtime cannot drift. `off` (or absent)
|
||||
* processes every repository and ignores the list entirely.
|
||||
*/
|
||||
export const REPO_FILTER_MODES = ['off', 'allowlist', 'blocklist'] as const;
|
||||
|
||||
const REPOSITORY_FULL_NAME_RE = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/;
|
||||
|
||||
interface RepoFilter {
|
||||
mode: 'allowlist' | 'blocklist';
|
||||
repos: Set<string>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse and validate the repository event filter. Repository full names are
|
||||
* case-insensitive on GitHub, so entries are matched lowercased. An active
|
||||
* mode with an empty list is rejected: an empty allowlist would silently
|
||||
* drop every event, and an empty blocklist is a no-op nobody asked for.
|
||||
* While the mode is off the list is ignored, so a parked list never blocks
|
||||
* an unrelated save or connect.
|
||||
*/
|
||||
function normalizeRepoFilter(
|
||||
config: RepoFilterConfigFields,
|
||||
channelName: string,
|
||||
): RepoFilter | null {
|
||||
const mode = config.repoFilterMode;
|
||||
if (mode === undefined || mode === 'off') return null;
|
||||
if (mode !== 'allowlist' && mode !== 'blocklist') {
|
||||
throw new Error(
|
||||
`repoFilterMode for channel ${channelName} must be one of: ${REPO_FILTER_MODES.join(', ')}.`,
|
||||
);
|
||||
}
|
||||
if (!Array.isArray(config.repoFilter)) {
|
||||
throw new Error(
|
||||
`repoFilter for channel ${channelName} must be an array of owner/repo repository names when repoFilterMode is "${mode}".`,
|
||||
);
|
||||
}
|
||||
if (config.repoFilter.some((entry) => typeof entry !== 'string')) {
|
||||
throw new Error(
|
||||
`repoFilter entries for channel ${channelName} must be strings.`,
|
||||
);
|
||||
}
|
||||
const repos = new Set(
|
||||
config.repoFilter
|
||||
.filter((entry): entry is string => typeof entry === 'string')
|
||||
.map((entry) => entry.trim().toLowerCase())
|
||||
.filter((entry) => entry.length > 0),
|
||||
);
|
||||
if (repos.size === 0) {
|
||||
throw new Error(
|
||||
`repoFilter for channel ${channelName} requires at least one owner/repo entry when repoFilterMode is "${mode}".`,
|
||||
);
|
||||
}
|
||||
const invalid = [...repos].filter(
|
||||
(entry) => !REPOSITORY_FULL_NAME_RE.test(entry),
|
||||
);
|
||||
if (invalid.length > 0) {
|
||||
throw new Error(
|
||||
`repoFilter entries for channel ${channelName} must be owner/repo repository names: ${invalid.join(', ')}`,
|
||||
);
|
||||
}
|
||||
return { mode, repos };
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared validation for the plugin descriptor (returns the message) and
|
||||
* connect() (throws it): one source of truth for the repository filter
|
||||
* rules, so the management UI and the adapter cannot drift (the
|
||||
* validateBoundSessionConfig pattern).
|
||||
*/
|
||||
export function validateRepoFilterConfig(
|
||||
config: RepoFilterConfigFields,
|
||||
channelName: string,
|
||||
): string | undefined {
|
||||
try {
|
||||
normalizeRepoFilter(config, channelName);
|
||||
} catch (err) {
|
||||
return err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* The prWatchEvents vocabulary, shared by the config validator, the watch
|
||||
* emit path, and the management UI's option list (built from this constant
|
||||
@@ -785,6 +873,8 @@ const MAX_WINDOW_KEYS = 8192;
|
||||
* first few individually and collapse the rest into one counted row.
|
||||
*/
|
||||
const MAX_REASON_FILTER_EVENTS_PER_POLL = 20;
|
||||
/** Same journal bound for repository-filter skips (see the reason above). */
|
||||
const MAX_REPO_FILTER_EVENTS_PER_POLL = 20;
|
||||
|
||||
interface InboundTaskDedupe {
|
||||
dispatchedBodies?: string[];
|
||||
@@ -1214,6 +1304,7 @@ export class GithubChannel extends PollingChannelBase<GithubCursor> {
|
||||
private pendingFinalDeliveryRequestsActive = 0;
|
||||
private pendingFinalDeliveryRetryStopRequested = false;
|
||||
private reasonFilter: Set<string> | null = null;
|
||||
private repoFilter: RepoFilter | null = null;
|
||||
private deliverToBoundSessions = false;
|
||||
private prWatchEvents = new Set<PrWatchEventType>();
|
||||
/**
|
||||
@@ -1449,6 +1540,7 @@ export class GithubChannel extends PollingChannelBase<GithubCursor> {
|
||||
async connect(): Promise<void> {
|
||||
const cfg = this.config as GithubConfig;
|
||||
this.reasonFilter = normalizeReasonFilter(cfg, this.name);
|
||||
this.repoFilter = normalizeRepoFilter(cfg, this.name);
|
||||
const boundConfigError = validateBoundSessionConfig(cfg, this.name);
|
||||
if (boundConfigError) {
|
||||
throw new Error(boundConfigError);
|
||||
@@ -2386,6 +2478,8 @@ export class GithubChannel extends PollingChannelBase<GithubCursor> {
|
||||
}
|
||||
let reasonFilterJournaled = 0;
|
||||
let reasonFilterCollapsed = 0;
|
||||
let repoFilterJournaled = 0;
|
||||
let repoFilterCollapsed = 0;
|
||||
let transientHeld = false;
|
||||
let sharedHeld = false;
|
||||
for (const notification of notifications) {
|
||||
@@ -2398,6 +2492,38 @@ export class GithubChannel extends PollingChannelBase<GithubCursor> {
|
||||
const { chatId, threadId, issueNumber } = extracted;
|
||||
const lastReadAt = notification.last_read_at;
|
||||
const reason = String(notification.reason ?? '').toLowerCase();
|
||||
if (this.repoFilter) {
|
||||
const listed = this.repoFilter.repos.has(chatId.toLowerCase());
|
||||
const filtered =
|
||||
this.repoFilter.mode === 'allowlist' ? !listed : listed;
|
||||
if (filtered) {
|
||||
process.stderr.write(
|
||||
`[Channel:${this.name}] skipping notification (repository=${sanitizeLogText(chatId, 140)} excluded by repoFilter ${this.repoFilter.mode}, subject=${sanitizeLogText(String(notification.subject.url ?? ''), 200)})\n`,
|
||||
);
|
||||
this.logDebugPayload('Github', {
|
||||
event: 'repoFilter.skip',
|
||||
chatId,
|
||||
threadId,
|
||||
mode: this.repoFilter.mode,
|
||||
reason: notification.reason,
|
||||
});
|
||||
if (repoFilterJournaled < MAX_REPO_FILTER_EVENTS_PER_POLL) {
|
||||
repoFilterJournaled += 1;
|
||||
this.recordChannelEvent({
|
||||
type: 'inbound',
|
||||
outcome: 'skipped',
|
||||
reason: 'repo_filter',
|
||||
triggerKind: reason,
|
||||
repository: chatId,
|
||||
thread: threadId,
|
||||
number: issueNumber,
|
||||
});
|
||||
} else {
|
||||
repoFilterCollapsed += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if (this.reasonFilter && !this.reasonFilter.has(reason)) {
|
||||
process.stderr.write(
|
||||
`[Channel:${this.name}] skipping notification (reason=${sanitizeLogText(reason, 64)} not in reasonFilter, subject=${sanitizeLogText(String(notification.subject.url ?? ''), 200)})\n`,
|
||||
@@ -2576,6 +2702,14 @@ export class GithubChannel extends PollingChannelBase<GithubCursor> {
|
||||
detail: { filtered: reasonFilterCollapsed },
|
||||
});
|
||||
}
|
||||
if (repoFilterCollapsed > 0) {
|
||||
this.recordChannelEvent({
|
||||
type: 'inbound',
|
||||
outcome: 'skipped',
|
||||
reason: 'repo_filter',
|
||||
detail: { filtered: repoFilterCollapsed },
|
||||
});
|
||||
}
|
||||
if (this.hasRecoverableInboundTasks()) {
|
||||
this.inboundRecoveryPending = true;
|
||||
}
|
||||
|
||||
@@ -1,7 +1,11 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { PR_BINDING_ROLES } from '@qwen-code/channel-base';
|
||||
import { plugin } from './index.js';
|
||||
import { PR_NOTIFICATION_REASONS, PR_WATCH_EVENTS } from './GithubAdapter.js';
|
||||
import {
|
||||
PR_NOTIFICATION_REASONS,
|
||||
PR_WATCH_EVENTS,
|
||||
REPO_FILTER_MODES,
|
||||
} from './GithubAdapter.js';
|
||||
|
||||
function validate(overrides: Record<string, unknown>): string | undefined {
|
||||
return plugin.management?.validateConfig?.({
|
||||
@@ -78,6 +82,19 @@ describe('github plugin validateConfig', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it('builds the repoFilterMode UI options from the adapter vocabulary', () => {
|
||||
const field = plugin.management?.fields.find(
|
||||
(candidate) => candidate.key === 'repoFilterMode',
|
||||
);
|
||||
// The settings store's enum branch requires membership in this option
|
||||
// list, and the editor always submits the seeded value: dropping options
|
||||
// or a mode would reject every Web Shell save with no other test red.
|
||||
expect(field?.options?.map((option) => option.value)).toEqual([
|
||||
...REPO_FILTER_MODES,
|
||||
]);
|
||||
expect(field?.default).toBe('off');
|
||||
});
|
||||
|
||||
it('describes and groups every boundSessionActions key for management UIs', () => {
|
||||
const field = plugin.management?.fields.find(
|
||||
(candidate) => candidate.key === 'boundSessionActions',
|
||||
|
||||
@@ -6,8 +6,10 @@ import type { ChannelPlugin } from '@qwen-code/channel-base';
|
||||
import {
|
||||
GithubChannel,
|
||||
validateBoundSessionConfig,
|
||||
validateRepoFilterConfig,
|
||||
PR_NOTIFICATION_REASONS,
|
||||
PR_WATCH_EVENTS,
|
||||
REPO_FILTER_MODES,
|
||||
} from './GithubAdapter.js';
|
||||
|
||||
/**
|
||||
@@ -139,6 +141,24 @@ export const plugin: ChannelPlugin = {
|
||||
label: value,
|
||||
})),
|
||||
},
|
||||
{
|
||||
key: 'repoFilterMode',
|
||||
label: 'Repository Filter',
|
||||
kind: 'enum',
|
||||
default: 'off',
|
||||
description:
|
||||
'Filter incoming notifications by repository: allowlist processes only the listed repositories, blocklist processes all except the listed ones. Notifications about PRs bound to a session are filtered too; only their PR state watch events bypass the filter.',
|
||||
// Built from the adapter's exported vocabulary so the UI options
|
||||
// cannot drift from the validator (the PR_WATCH_EVENTS pattern).
|
||||
options: REPO_FILTER_MODES.map((value) => ({ value, label: value })),
|
||||
},
|
||||
{
|
||||
key: 'repoFilter',
|
||||
label: 'Filtered Repositories',
|
||||
kind: 'string-list',
|
||||
description:
|
||||
'Comma-separated owner/repo repository names used by the repository filter. Requires Repository Filter to be set to allowlist or blocklist.',
|
||||
},
|
||||
{
|
||||
key: 'deliverToBoundSessions',
|
||||
label: 'Deliver To PR-Bound Sessions',
|
||||
@@ -172,7 +192,10 @@ export const plugin: ChannelPlugin = {
|
||||
if (!token && config['useLocalGh'] !== true) {
|
||||
return 'Channel requires a token or local GitHub CLI authentication (useLocalGh).';
|
||||
}
|
||||
return validateBoundSessionConfig(config, 'github');
|
||||
return (
|
||||
validateBoundSessionConfig(config, 'github') ??
|
||||
validateRepoFilterConfig(config, 'github')
|
||||
);
|
||||
},
|
||||
},
|
||||
createChannel: (name, config, bridge, options) =>
|
||||
|
||||
@@ -3772,3 +3772,308 @@ describe('Channel editor progressive disclosure', () => {
|
||||
expect(trigger?.textContent).toContain('已配置 2 项');
|
||||
});
|
||||
});
|
||||
|
||||
describe('GitHub repository event filter', () => {
|
||||
// Same field keys as the real descriptor; labels intentionally differ from
|
||||
// the i18n values so a missing FIELD_LABEL_KEYS entry surfaces the
|
||||
// untranslated descriptor fallback instead of passing the assertions.
|
||||
const GITHUB_REPO_FILTER: DaemonChannelTypeDescriptor = {
|
||||
...GITHUB_LOCAL_GH,
|
||||
fields: [
|
||||
...GITHUB_LOCAL_GH.fields,
|
||||
{
|
||||
key: 'repoFilterMode',
|
||||
label: 'Repo filter mode (descriptor)',
|
||||
kind: 'enum',
|
||||
default: 'off',
|
||||
options: [
|
||||
{ value: 'off', label: 'off' },
|
||||
{ value: 'allowlist', label: 'allowlist' },
|
||||
{ value: 'blocklist', label: 'blocklist' },
|
||||
],
|
||||
},
|
||||
{
|
||||
key: 'repoFilter',
|
||||
label: 'Repo filter list (descriptor)',
|
||||
kind: 'string-list',
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const FILTER_INSTANCE: DaemonChannelInstanceSnapshot = {
|
||||
name: 'github-bot',
|
||||
config: { type: 'github', useLocalGh: true },
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
|
||||
async function saveDialog() {
|
||||
const save = Array.from(document.querySelectorAll('button')).find(
|
||||
(button) => button.textContent?.trim() === 'Save',
|
||||
);
|
||||
await act(async () => {
|
||||
save?.click();
|
||||
});
|
||||
}
|
||||
|
||||
it('defaults to Off and hides the repository list', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: FILTER_INSTANCE,
|
||||
});
|
||||
|
||||
expect(fieldByLabel('Repository Filter')).not.toBeNull();
|
||||
expect(inputByLabel('Filtered Repositories')).toBeNull();
|
||||
});
|
||||
|
||||
it('renders the Chinese labels', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: FILTER_INSTANCE,
|
||||
language: 'zh-CN',
|
||||
});
|
||||
|
||||
expect(fieldByLabel('按仓库过滤')).not.toBeNull();
|
||||
await selectOption('按仓库过滤', '黑名单 — 除列出的仓库以外');
|
||||
expect(inputByLabel('过滤的仓库')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('reveals the list for an active mode and saves normalized repositories', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: FILTER_INSTANCE,
|
||||
onSave,
|
||||
});
|
||||
|
||||
await selectOption(
|
||||
'Repository Filter',
|
||||
'Blocklist — all except listed repositories',
|
||||
);
|
||||
const input = inputByLabel('Filtered Repositories');
|
||||
expect(input).not.toBeNull();
|
||||
await act(async () => {
|
||||
setInputValue(input!, 'https://github.com/Octo/Repo/pull/123, octo/repo');
|
||||
});
|
||||
await saveDialog();
|
||||
|
||||
expect(onSave).toHaveBeenCalledTimes(1);
|
||||
const request = onSave.mock.calls[0]![1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['repoFilterMode']).toBe('blocklist');
|
||||
expect(request.config['repoFilter']).toEqual(['octo/repo']);
|
||||
});
|
||||
|
||||
it('blocks an active mode with an empty list and retires the error on Off', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: FILTER_INSTANCE,
|
||||
onSave,
|
||||
});
|
||||
|
||||
await selectOption(
|
||||
'Repository Filter',
|
||||
'Allowlist — only listed repositories',
|
||||
);
|
||||
await saveDialog();
|
||||
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
expect(document.body.textContent).toContain(
|
||||
'List at least one repository, or set Repository Filter to Off.',
|
||||
);
|
||||
|
||||
await selectOption('Repository Filter', 'Off — all repositories');
|
||||
expect(document.body.textContent).not.toContain(
|
||||
'List at least one repository, or set Repository Filter to Off.',
|
||||
);
|
||||
expect(inputByLabel('Filtered Repositories')).toBeNull();
|
||||
|
||||
// The discriminating half: while Off the field is unmounted, so the
|
||||
// assertion above cannot see a stale error. Switching back remounts it —
|
||||
// the message must stay retired, which only happens if the mode switch
|
||||
// re-derived the shown errors instead of leaving them in the map.
|
||||
await selectOption(
|
||||
'Repository Filter',
|
||||
'Allowlist — only listed repositories',
|
||||
);
|
||||
expect(inputByLabel('Filtered Repositories')).not.toBeNull();
|
||||
expect(document.body.textContent).not.toContain(
|
||||
'List at least one repository, or set Repository Filter to Off.',
|
||||
);
|
||||
await selectOption('Repository Filter', 'Off — all repositories');
|
||||
|
||||
await saveDialog();
|
||||
expect(onSave).toHaveBeenCalledTimes(1);
|
||||
const request = onSave.mock.calls[0]![1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['repoFilterMode']).toBe('off');
|
||||
});
|
||||
|
||||
it('retires the shown error when a valid entry is typed, without saving', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: FILTER_INSTANCE,
|
||||
onSave,
|
||||
});
|
||||
|
||||
await selectOption(
|
||||
'Repository Filter',
|
||||
'Allowlist — only listed repositories',
|
||||
);
|
||||
await saveDialog();
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
expect(document.body.textContent).toContain(
|
||||
'List at least one repository, or set Repository Filter to Off.',
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
setInputValue(inputByLabel('Filtered Repositories')!, 'octo/repo');
|
||||
});
|
||||
expect(document.body.textContent).not.toContain(
|
||||
'List at least one repository, or set Repository Filter to Off.',
|
||||
);
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('retires the format error when the entry is corrected, without saving', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: FILTER_INSTANCE,
|
||||
onSave,
|
||||
});
|
||||
|
||||
await selectOption(
|
||||
'Repository Filter',
|
||||
'Blocklist — all except listed repositories',
|
||||
);
|
||||
await act(async () => {
|
||||
setInputValue(inputByLabel('Filtered Repositories')!, 'not-a-repo');
|
||||
});
|
||||
await saveDialog();
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
expect(document.body.textContent).toContain(
|
||||
'Enter each repository as owner/repo.',
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
setInputValue(inputByLabel('Filtered Repositories')!, 'octo/repo');
|
||||
});
|
||||
expect(document.body.textContent).not.toContain(
|
||||
'Enter each repository as owner/repo.',
|
||||
);
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('opens on a stored active mode with its list and re-saves both keys', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: {
|
||||
...FILTER_INSTANCE,
|
||||
config: {
|
||||
...FILTER_INSTANCE.config,
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['octo/repo'],
|
||||
},
|
||||
},
|
||||
onSave,
|
||||
});
|
||||
|
||||
const input = inputByLabel('Filtered Repositories');
|
||||
expect(input).not.toBeNull();
|
||||
expect(input).toHaveProperty('value', 'octo/repo');
|
||||
|
||||
await saveDialog();
|
||||
expect(onSave).toHaveBeenCalledTimes(1);
|
||||
const request = onSave.mock.calls[0]![1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['repoFilterMode']).toBe('blocklist');
|
||||
expect(request.config['repoFilter']).toEqual(['octo/repo']);
|
||||
});
|
||||
|
||||
it('hides the list while Off and restores it on re-enable', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: {
|
||||
...FILTER_INSTANCE,
|
||||
config: {
|
||||
...FILTER_INSTANCE.config,
|
||||
repoFilterMode: 'blocklist',
|
||||
repoFilter: ['octo/repo'],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(inputByLabel('Filtered Repositories')).toHaveProperty(
|
||||
'value',
|
||||
'octo/repo',
|
||||
);
|
||||
|
||||
await selectOption('Repository Filter', 'Off — all repositories');
|
||||
expect(inputByLabel('Filtered Repositories')).toBeNull();
|
||||
|
||||
await selectOption(
|
||||
'Repository Filter',
|
||||
'Blocklist — all except listed repositories',
|
||||
);
|
||||
expect(inputByLabel('Filtered Repositories')).toHaveProperty(
|
||||
'value',
|
||||
'octo/repo',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects entries that are not owner/repo after normalization', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: FILTER_INSTANCE,
|
||||
onSave,
|
||||
});
|
||||
|
||||
await selectOption(
|
||||
'Repository Filter',
|
||||
'Blocklist — all except listed repositories',
|
||||
);
|
||||
await act(async () => {
|
||||
setInputValue(inputByLabel('Filtered Repositories')!, 'not-a-repo');
|
||||
});
|
||||
await saveDialog();
|
||||
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
expect(document.body.textContent).toContain(
|
||||
'Enter each repository as owner/repo.',
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps a parked list exactly as stored while the mode is off', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_REPO_FILTER,
|
||||
instance: {
|
||||
...FILTER_INSTANCE,
|
||||
config: {
|
||||
...FILTER_INSTANCE.config,
|
||||
repoFilterMode: 'off',
|
||||
repoFilter: ['Owner/Parked'],
|
||||
},
|
||||
},
|
||||
onSave,
|
||||
});
|
||||
|
||||
await saveDialog();
|
||||
|
||||
expect(onSave).toHaveBeenCalledTimes(1);
|
||||
const request = onSave.mock.calls[0]![1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['repoFilterMode']).toBe('off');
|
||||
expect(request.config['repoFilter']).toEqual(['Owner/Parked']);
|
||||
});
|
||||
});
|
||||
@@ -76,6 +76,7 @@ import {
|
||||
configuredPrivatePolicy,
|
||||
PRIVATE_ONLY_FIELD_KEYS,
|
||||
invalidRepositoryGroupEntries,
|
||||
isRepoFilterActive,
|
||||
projectRepositoryGroup,
|
||||
recordHasRoute,
|
||||
repositoryAllowlistIds,
|
||||
@@ -117,6 +118,8 @@ const FIELD_LABEL_KEYS: Record<string, Record<string, string>> = {
|
||||
pollInterval: 'channels.editor.field.github.pollInterval',
|
||||
groupPolicy: 'channels.editor.field.github.groupPolicy',
|
||||
reasonFilter: 'channels.editor.field.github.reasonFilter',
|
||||
repoFilterMode: 'channels.editor.field.github.repoFilterMode',
|
||||
repoFilter: 'channels.editor.field.github.repoFilter',
|
||||
deliverToBoundSessions:
|
||||
'channels.editor.field.github.deliverToBoundSessions',
|
||||
prWatchEvents: 'channels.editor.field.github.prWatchEvents',
|
||||
@@ -166,6 +169,7 @@ const SHARED_FIELD_LABEL_KEYS: Record<string, string> = {
|
||||
/** Per-platform placeholders that spell out the expected value shape. */
|
||||
const FIELD_PLACEHOLDER_KEYS: Record<string, string> = {
|
||||
'github.baseUrl': 'https://ghe.example.com/api/v3',
|
||||
'github.repoFilter': 'owner/repo, owner/repo',
|
||||
};
|
||||
|
||||
const CONNECTION_FIELD_KEYS = new Set(['baseUrl', 'wsUrl', 'pollInterval']);
|
||||
@@ -483,6 +487,8 @@ export function ChannelEditorDialog({
|
||||
return t('channels.editor.validation.invalidGroupId');
|
||||
if (code === 'invalidRepoId')
|
||||
return t('channels.editor.validation.invalidRepoId');
|
||||
if (code === 'repoFilterRequired')
|
||||
return t('channels.editor.validation.repoFilterRequired');
|
||||
if (code === 'conflictingRepoAliases')
|
||||
return t('channels.editor.validation.conflictingRepoAliases');
|
||||
if (code === 'invalidOption')
|
||||
@@ -720,7 +726,13 @@ export function ChannelEditorDialog({
|
||||
const id = `${formId}-${field.key}`;
|
||||
const value = draft.values[field.key];
|
||||
const error = errors[field.key];
|
||||
const update = (next: string | boolean) =>
|
||||
const update = (next: string | boolean) => {
|
||||
if (field.key === 'repoFilterMode' || field.key === 'repoFilter') {
|
||||
// Both remedies retire a shown error: switching to off hides the
|
||||
// list, and typing a valid entry satisfies the instruction. Submit
|
||||
// stays the only site that populates errors.
|
||||
refreshErrors({ ...draft.values, [field.key]: next });
|
||||
}
|
||||
setDraft((current) => ({
|
||||
...current,
|
||||
values: {
|
||||
@@ -731,6 +743,7 @@ export function ChannelEditorDialog({
|
||||
: {}),
|
||||
},
|
||||
}));
|
||||
};
|
||||
if (field.kind === 'boolean') {
|
||||
const isLocalGhSwitch =
|
||||
field.key === 'useLocalGh' &&
|
||||
@@ -1545,7 +1558,16 @@ export function ChannelEditorDialog({
|
||||
|
||||
{(['credentials', 'connection'] as const).map((section) => {
|
||||
const sectionFields = credentialFields.filter(
|
||||
(field) => fieldSectionKey(field) === section,
|
||||
(field) =>
|
||||
fieldSectionKey(field) === section &&
|
||||
// The repository list only means something while a filter
|
||||
// mode is active; validation is gated on the same
|
||||
// predicate, so the hidden field can never strand a save.
|
||||
!(
|
||||
field.key === 'repoFilter' &&
|
||||
isRepositoryOnlyChannelType(descriptor.type) &&
|
||||
!isRepoFilterActive(draft.values['repoFilterMode'])
|
||||
),
|
||||
);
|
||||
if (sectionFields.length === 0) return null;
|
||||
return (
|
||||
|
||||
@@ -11,6 +11,7 @@ import type {
|
||||
DaemonChannelTypeDescriptor,
|
||||
} from '@qwen-code/sdk/daemon';
|
||||
import {
|
||||
buildChannelUpsertRequest,
|
||||
createChannelEditorDraft,
|
||||
validateChannelEditorDraft,
|
||||
} from './channel-editor-state';
|
||||
@@ -101,3 +102,139 @@ describe('bound-delivery rule parity (editor mirror vs daemon authority)', () =>
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
const REPO_FILTER_CASES = [
|
||||
{
|
||||
name: 'active allowlist with an empty list',
|
||||
mode: 'allowlist',
|
||||
repos: [] as string[] | undefined,
|
||||
code: 'repoFilterRequired',
|
||||
daemon: /requires at least one owner\/repo entry/,
|
||||
},
|
||||
{
|
||||
name: 'active blocklist without a list',
|
||||
mode: 'blocklist',
|
||||
repos: undefined,
|
||||
code: 'repoFilterRequired',
|
||||
daemon: /must be an array of owner\/repo/,
|
||||
},
|
||||
{
|
||||
name: 'malformed entry',
|
||||
mode: 'blocklist',
|
||||
repos: ['not-a-repo'],
|
||||
code: 'invalidRepoId',
|
||||
daemon: /must be owner\/repo repository names/,
|
||||
},
|
||||
{
|
||||
name: 'valid allowlist',
|
||||
mode: 'allowlist',
|
||||
repos: ['octo/repo'],
|
||||
code: undefined,
|
||||
daemon: undefined,
|
||||
},
|
||||
{
|
||||
name: 'valid blocklist',
|
||||
mode: 'blocklist',
|
||||
repos: ['octo/repo'],
|
||||
code: undefined,
|
||||
daemon: undefined,
|
||||
},
|
||||
{
|
||||
name: 'off with a stale list',
|
||||
mode: 'off',
|
||||
repos: ['not-a-repo'],
|
||||
code: undefined,
|
||||
daemon: undefined,
|
||||
},
|
||||
] as const;
|
||||
|
||||
describe('repository filter rule parity (editor mirror vs daemon authority)', () => {
|
||||
it.each(REPO_FILTER_CASES)('$name', ({ mode, repos, code, daemon }) => {
|
||||
const config: Record<string, unknown> = {
|
||||
type: 'github',
|
||||
useLocalGh: true,
|
||||
repoFilterMode: mode,
|
||||
};
|
||||
if (repos !== undefined) config['repoFilter'] = [...repos];
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'parity',
|
||||
config,
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
const draft = createChannelEditorDraft(descriptor, instance);
|
||||
draft.name = 'parity';
|
||||
|
||||
const clientCode = validateChannelEditorDraft(
|
||||
descriptor,
|
||||
draft,
|
||||
[],
|
||||
instance,
|
||||
)['repoFilter'];
|
||||
expect(clientCode).toBe(code);
|
||||
|
||||
const daemonMessage = management.validateConfig?.(config);
|
||||
if (daemon) {
|
||||
expect(daemonMessage).toMatch(daemon);
|
||||
} else {
|
||||
expect(daemonMessage).toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
it('shortens pasted URLs before the daemon authority sees them', () => {
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'parity',
|
||||
config: { type: 'github', useLocalGh: true },
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
const draft = createChannelEditorDraft(descriptor, instance);
|
||||
draft.name = 'parity';
|
||||
draft.values['repoFilterMode'] = 'blocklist';
|
||||
draft.values['repoFilter'] = 'https://github.com/Octo/Repo/pull/9';
|
||||
|
||||
expect(validateChannelEditorDraft(descriptor, draft, [], instance)).toEqual(
|
||||
{},
|
||||
);
|
||||
const { config } = buildChannelUpsertRequest(
|
||||
descriptor,
|
||||
draft,
|
||||
'rev-1',
|
||||
instance,
|
||||
);
|
||||
expect(config['repoFilter']).toEqual(['octo/repo']);
|
||||
expect(management.validateConfig?.(config)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('binds a GHES paste through the editor and the daemon authority', () => {
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'parity',
|
||||
config: {
|
||||
type: 'github',
|
||||
useLocalGh: true,
|
||||
baseUrl: 'https://ghe.example.com/api/v3',
|
||||
},
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
const draft = createChannelEditorDraft(descriptor, instance);
|
||||
draft.name = 'parity';
|
||||
draft.values['repoFilterMode'] = 'blocklist';
|
||||
draft.values['repoFilter'] = 'https://ghe.example.com/octo/repo';
|
||||
|
||||
expect(validateChannelEditorDraft(descriptor, draft, [], instance)).toEqual(
|
||||
{},
|
||||
);
|
||||
const { config } = buildChannelUpsertRequest(
|
||||
descriptor,
|
||||
draft,
|
||||
'rev-1',
|
||||
instance,
|
||||
);
|
||||
expect(config['repoFilter']).toEqual(['octo/repo']);
|
||||
expect(management.validateConfig?.(config)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -12,7 +12,10 @@ import type {
|
||||
import {
|
||||
buildChannelUpsertRequest,
|
||||
createChannelEditorDraft,
|
||||
GITHUB_RESERVED_URL_PATHS,
|
||||
invalidRepositoryGroupEntries,
|
||||
isRepoFilterActive,
|
||||
normalizeRepositoryFilterInput,
|
||||
repositoryAllowlistIds,
|
||||
retainedRepositorySettings,
|
||||
validateChannelEditorDraft,
|
||||
@@ -2953,3 +2956,386 @@ describe('GitHub poll interval', () => {
|
||||
).toEqual({ pollInterval: 'outOfRange' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('GitHub repository event filter', () => {
|
||||
const REPO_FILTER: DaemonChannelTypeDescriptor = {
|
||||
...GITHUB,
|
||||
fields: [
|
||||
...GITHUB.fields,
|
||||
{
|
||||
key: 'baseUrl',
|
||||
label: 'Base URL',
|
||||
kind: 'string',
|
||||
envResolvable: true,
|
||||
},
|
||||
{
|
||||
key: 'repoFilterMode',
|
||||
label: 'Repository Filter',
|
||||
kind: 'enum',
|
||||
default: 'off',
|
||||
options: [
|
||||
{ value: 'off', label: 'off' },
|
||||
{ value: 'allowlist', label: 'allowlist' },
|
||||
{ value: 'blocklist', label: 'blocklist' },
|
||||
],
|
||||
},
|
||||
{
|
||||
key: 'repoFilter',
|
||||
label: 'Filtered Repositories',
|
||||
kind: 'string-list',
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
function filterDraft(
|
||||
mode: string,
|
||||
list: string,
|
||||
instance?: DaemonChannelInstanceSnapshot,
|
||||
): ChannelEditorDraft {
|
||||
const draft = createChannelEditorDraft(REPO_FILTER, instance);
|
||||
draft.name = 'my-bot';
|
||||
draft.values.useLocalGh = true;
|
||||
draft.values.repoFilterMode = mode;
|
||||
draft.values.repoFilter = list;
|
||||
return draft;
|
||||
}
|
||||
|
||||
describe('normalizeRepositoryFilterInput', () => {
|
||||
it('shortens pasted GitHub web URLs to owner/repo', () => {
|
||||
expect(
|
||||
normalizeRepositoryFilterInput(
|
||||
'https://github.com/Octo/Repo, https://github.com/Octo/Repo/pull/123, octo/repo',
|
||||
),
|
||||
).toBe('octo/repo');
|
||||
});
|
||||
|
||||
it('shortens URLs on the GHES host derived from baseUrl', () => {
|
||||
expect(
|
||||
normalizeRepositoryFilterInput(
|
||||
'https://ghe.example.com/octo/repo.git/',
|
||||
'https://ghe.example.com/api/v3',
|
||||
),
|
||||
).toBe('octo/repo');
|
||||
// Without the channel's baseUrl, the same host is not shortenable.
|
||||
expect(
|
||||
normalizeRepositoryFilterInput(
|
||||
'https://ghe.example.com/octo/repo.git/',
|
||||
),
|
||||
).toBe('https://ghe.example.com/octo/repo.git/');
|
||||
});
|
||||
|
||||
it('does not shorten URLs on hosts other than the channel GitHub host', () => {
|
||||
// The structural half of the guard: the first two path segments of an
|
||||
// arbitrary host are not a repository. Each of these once shortened to
|
||||
// a plausible owner/repo that both validators accepted.
|
||||
const foreign = [
|
||||
'https://api.github.com/repos/octo/repo',
|
||||
'https://api.github.com/repos/QwenLM/qwen-code/issues/42',
|
||||
'https://gist.github.com/octocat/6cad1f0e5d3b2a9c',
|
||||
'https://gitlab.com/group/project',
|
||||
];
|
||||
for (const url of foreign) {
|
||||
expect(normalizeRepositoryFilterInput(url)).toBe(url.toLowerCase());
|
||||
expect(
|
||||
validateChannelEditorDraft(
|
||||
REPO_FILTER,
|
||||
filterDraft('allowlist', url),
|
||||
[],
|
||||
),
|
||||
).toEqual({ repoFilter: 'invalidRepoId' });
|
||||
}
|
||||
});
|
||||
|
||||
it('shortens URLs carrying a query or fragment', () => {
|
||||
expect(
|
||||
normalizeRepositoryFilterInput(
|
||||
'https://github.com/Octo/Repo?tab=readme-ov-file, https://github.com/Octo/Repo#readme',
|
||||
),
|
||||
).toBe('octo/repo');
|
||||
});
|
||||
|
||||
it('rejects REST-shaped URLs on the channel own API host', () => {
|
||||
// The host gate must not re-admit the channel's own API host: with
|
||||
// baseUrl set, a REST URL (the shape `gh api` prints) must pass through
|
||||
// for validation to reject, not shorten to `repos/octo` or `api/v3`.
|
||||
const cases = [
|
||||
['https://api.github.com/repos/octo/repo', 'https://api.github.com'],
|
||||
[
|
||||
'https://ghe.example.com/api/v3/repos/octo/repo',
|
||||
'https://ghe.example.com/api/v3',
|
||||
],
|
||||
];
|
||||
for (const [url, baseUrl] of cases) {
|
||||
expect(normalizeRepositoryFilterInput(url!, baseUrl)).toBe(
|
||||
url!.toLowerCase(),
|
||||
);
|
||||
const draft = filterDraft('allowlist', url!);
|
||||
draft.values.baseUrl = baseUrl!;
|
||||
expect(validateChannelEditorDraft(REPO_FILTER, draft, [])).toEqual({
|
||||
repoFilter: 'invalidRepoId',
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('strips trailing sentence punctuation from a pasted URL', () => {
|
||||
expect(
|
||||
normalizeRepositoryFilterInput(
|
||||
'https://github.com/octo/repo., https://github.com/octo/repo)',
|
||||
),
|
||||
).toBe('octo/repo');
|
||||
expect(
|
||||
validateChannelEditorDraft(
|
||||
REPO_FILTER,
|
||||
filterDraft('blocklist', 'https://github.com/octo/repo.'),
|
||||
[],
|
||||
),
|
||||
).toEqual({});
|
||||
});
|
||||
|
||||
it('binds the GHES host through both the validator and the save path', () => {
|
||||
// Dropping either baseUrl argument must fail here: the validator would
|
||||
// reject the paste while the save path still shortens it (or vice
|
||||
// versa) — the editor/daemon divergence the shared normalizer prevents.
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'my-bot',
|
||||
config: {
|
||||
type: 'github',
|
||||
useLocalGh: true,
|
||||
baseUrl: 'https://ghe.example.com/api/v3',
|
||||
},
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
const draft = filterDraft(
|
||||
'blocklist',
|
||||
'https://ghe.example.com/octo/repo',
|
||||
instance,
|
||||
);
|
||||
expect(
|
||||
validateChannelEditorDraft(REPO_FILTER, draft, [], instance),
|
||||
).toEqual({});
|
||||
const request = buildChannelUpsertRequest(
|
||||
REPO_FILTER,
|
||||
draft,
|
||||
'rev-1',
|
||||
instance,
|
||||
);
|
||||
expect(request.config['repoFilter']).toEqual(['octo/repo']);
|
||||
});
|
||||
|
||||
it('does not shorten non-repository GitHub URLs', () => {
|
||||
// Regression pins for the measured verify-round shapes: each of these
|
||||
// once shortened to a plausible-but-nonexistent owner/repo that both
|
||||
// validators accepted — fatal under an allowlist. The capitalized
|
||||
// variant pins the guard's own lowercasing of the path root.
|
||||
const hostile = [
|
||||
'https://github.com/orgs/community',
|
||||
'https://github.com/Orgs/community',
|
||||
'https://github.com/settings/profile',
|
||||
'https://github.com/apps/renovate',
|
||||
'https://github.com/features/copilot',
|
||||
'https://github.com/trending/typescript',
|
||||
'https://github.com/pricing/enterprise',
|
||||
'https://github.com/advisories/GHSA-xxxx-xxxx-xxxx',
|
||||
'https://github.com/copilot/c/abc123',
|
||||
];
|
||||
for (const url of hostile) {
|
||||
expect(normalizeRepositoryFilterInput(url)).toBe(url.toLowerCase());
|
||||
expect(
|
||||
validateChannelEditorDraft(
|
||||
REPO_FILTER,
|
||||
filterDraft('allowlist', url),
|
||||
[],
|
||||
),
|
||||
).toEqual({ repoFilter: 'invalidRepoId' });
|
||||
}
|
||||
});
|
||||
|
||||
it('pins the whole reserved-area set, not a sample of it', () => {
|
||||
// A snapshot over every member: removing any root from the set fails
|
||||
// here, which the it.each enumeration alone cannot witness (it would
|
||||
// simply run one case fewer).
|
||||
expect([...GITHUB_RESERVED_URL_PATHS].sort()).toEqual(
|
||||
[
|
||||
'about',
|
||||
'account',
|
||||
'accounts',
|
||||
'advisories',
|
||||
'apps',
|
||||
'codespaces',
|
||||
'collections',
|
||||
'contact',
|
||||
'copilot',
|
||||
'customer-stories',
|
||||
'discussions',
|
||||
'enterprise',
|
||||
'enterprises',
|
||||
'events',
|
||||
'explore',
|
||||
'features',
|
||||
'followers',
|
||||
'following',
|
||||
'issues',
|
||||
'join',
|
||||
'login',
|
||||
'logout',
|
||||
'marketplace',
|
||||
'mobile',
|
||||
'new',
|
||||
'notifications',
|
||||
'orgs',
|
||||
'organizations',
|
||||
'password_reset',
|
||||
'premium-support',
|
||||
'pricing',
|
||||
'pulls',
|
||||
'readme',
|
||||
'search',
|
||||
'security',
|
||||
'sessions',
|
||||
'settings',
|
||||
'signup',
|
||||
'site',
|
||||
'sponsors',
|
||||
'stars',
|
||||
'team',
|
||||
'topics',
|
||||
'trending',
|
||||
'users',
|
||||
'watching',
|
||||
].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it.each([...GITHUB_RESERVED_URL_PATHS])(
|
||||
'rejects every reserved GitHub area root: %s',
|
||||
(root) => {
|
||||
// Enumerates the whole guard set so a newly added root is covered the
|
||||
// moment it lands, instead of waiting for a hand-picked fixture.
|
||||
const url = `https://github.com/${root}/some-page`;
|
||||
expect(normalizeRepositoryFilterInput(url)).toBe(url);
|
||||
expect(
|
||||
validateChannelEditorDraft(
|
||||
REPO_FILTER,
|
||||
filterDraft('blocklist', url),
|
||||
[],
|
||||
),
|
||||
).toEqual({ repoFilter: 'invalidRepoId' });
|
||||
},
|
||||
);
|
||||
|
||||
it('lowercases and deduplicates plain entries', () => {
|
||||
expect(
|
||||
normalizeRepositoryFilterInput('Owner/Repo, owner/repo, other/repo'),
|
||||
).toBe('owner/repo, other/repo');
|
||||
});
|
||||
|
||||
it('passes non-URL entries through for validation to name', () => {
|
||||
expect(normalizeRepositoryFilterInput('not-a-repo, a/b')).toBe(
|
||||
'not-a-repo, a/b',
|
||||
);
|
||||
expect(normalizeRepositoryFilterInput(' ')).toBe('');
|
||||
});
|
||||
});
|
||||
|
||||
it('isRepoFilterActive only accepts the two list modes', () => {
|
||||
expect(isRepoFilterActive('allowlist')).toBe(true);
|
||||
expect(isRepoFilterActive('blocklist')).toBe(true);
|
||||
expect(isRepoFilterActive('off')).toBe(false);
|
||||
expect(isRepoFilterActive(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it('seeds Off for a new draft and for an instance without the keys', () => {
|
||||
expect(createChannelEditorDraft(REPO_FILTER).values.repoFilterMode).toBe(
|
||||
'off',
|
||||
);
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'my-bot',
|
||||
config: { type: 'github', useLocalGh: true },
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
expect(
|
||||
createChannelEditorDraft(REPO_FILTER, instance).values.repoFilterMode,
|
||||
).toBe('off');
|
||||
});
|
||||
|
||||
it('requires a non-empty list while a mode is active', () => {
|
||||
expect(
|
||||
validateChannelEditorDraft(REPO_FILTER, filterDraft('allowlist', ''), []),
|
||||
).toEqual({ repoFilter: 'repoFilterRequired' });
|
||||
expect(
|
||||
validateChannelEditorDraft(
|
||||
REPO_FILTER,
|
||||
filterDraft('blocklist', ' '),
|
||||
[],
|
||||
),
|
||||
).toEqual({ repoFilter: 'repoFilterRequired' });
|
||||
});
|
||||
|
||||
it('rejects entries that are not owner/repo after normalization', () => {
|
||||
expect(
|
||||
validateChannelEditorDraft(
|
||||
REPO_FILTER,
|
||||
filterDraft('blocklist', 'not-a-repo'),
|
||||
[],
|
||||
),
|
||||
).toEqual({ repoFilter: 'invalidRepoId' });
|
||||
});
|
||||
|
||||
it('accepts pasted URLs and ignores the list while off', () => {
|
||||
expect(
|
||||
validateChannelEditorDraft(
|
||||
REPO_FILTER,
|
||||
filterDraft('allowlist', 'https://github.com/Octo/Repo/pull/1'),
|
||||
[],
|
||||
),
|
||||
).toEqual({});
|
||||
expect(
|
||||
validateChannelEditorDraft(
|
||||
REPO_FILTER,
|
||||
filterDraft('off', 'not-a-repo'),
|
||||
[],
|
||||
),
|
||||
).toEqual({});
|
||||
});
|
||||
|
||||
it('normalizes the list in the save payload while a mode is active', () => {
|
||||
const request = buildChannelUpsertRequest(
|
||||
REPO_FILTER,
|
||||
filterDraft(
|
||||
'blocklist',
|
||||
'https://github.com/Octo/Repo/pull/9, Other/Repo',
|
||||
),
|
||||
'rev-1',
|
||||
);
|
||||
expect(request.config['repoFilterMode']).toBe('blocklist');
|
||||
expect(request.config['repoFilter']).toEqual(['octo/repo', 'other/repo']);
|
||||
});
|
||||
|
||||
it('keeps a parked list exactly as stored while the mode is off', () => {
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'my-bot',
|
||||
config: {
|
||||
type: 'github',
|
||||
useLocalGh: true,
|
||||
repoFilterMode: 'off',
|
||||
repoFilter: ['Owner/Parked'],
|
||||
},
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
const draft = createChannelEditorDraft(REPO_FILTER, instance);
|
||||
draft.name = 'my-bot';
|
||||
const request = buildChannelUpsertRequest(
|
||||
REPO_FILTER,
|
||||
draft,
|
||||
'rev-1',
|
||||
instance,
|
||||
);
|
||||
expect(request.config['repoFilter']).toEqual(['Owner/Parked']);
|
||||
});
|
||||
});
|
||||
@@ -56,6 +56,7 @@ export type ChannelEditorValidationCode =
|
||||
| 'invalidGroupId'
|
||||
| 'invalidOption'
|
||||
| 'invalidRepoId'
|
||||
| 'repoFilterRequired'
|
||||
| 'conflictingRepoAliases'
|
||||
| 'invalidGroupEntry'
|
||||
| 'number'
|
||||
@@ -89,6 +90,154 @@ export function repositoryAllowlistIds(value: string): string[] {
|
||||
);
|
||||
}
|
||||
|
||||
/** Whether the repository event filter processes a list at all. */
|
||||
export function isRepoFilterActive(mode: unknown): boolean {
|
||||
return mode === 'allowlist' || mode === 'blocklist';
|
||||
}
|
||||
|
||||
const GITHUB_REPOSITORY_URL_RE =
|
||||
/^https?:\/\/[^/]+\/([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+?)(?:\.git)?(?:[/?#].*)?$/;
|
||||
|
||||
/**
|
||||
* GitHub areas whose two-segment URLs look like `owner/repo` but are not
|
||||
* repositories. Shortening one would silently filter a nonexistent
|
||||
* repository — fatal under an allowlist — so these URLs pass through
|
||||
* unchanged and validation rejects them by name instead.
|
||||
*
|
||||
* Best-effort by construction: a denylist of GitHub's top-level paths cannot
|
||||
* be complete, and a future GitHub area can reintroduce the hazard until it
|
||||
* is added here. The enumerated test over this set keeps every listed root
|
||||
* rejected; the residual risk is documented in the design doc.
|
||||
*
|
||||
* Exported so the test can enumerate every member instead of sampling a few.
|
||||
*/
|
||||
export const GITHUB_RESERVED_URL_PATHS = new Set([
|
||||
'about',
|
||||
'account',
|
||||
'accounts',
|
||||
'advisories',
|
||||
'apps',
|
||||
'codespaces',
|
||||
'collections',
|
||||
'contact',
|
||||
'copilot',
|
||||
'customer-stories',
|
||||
'discussions',
|
||||
'enterprise',
|
||||
'enterprises',
|
||||
'events',
|
||||
'explore',
|
||||
'features',
|
||||
'followers',
|
||||
'following',
|
||||
'issues',
|
||||
'join',
|
||||
'login',
|
||||
'logout',
|
||||
'marketplace',
|
||||
'mobile',
|
||||
'new',
|
||||
'notifications',
|
||||
'orgs',
|
||||
'organizations',
|
||||
'password_reset',
|
||||
'premium-support',
|
||||
'pricing',
|
||||
'pulls',
|
||||
'readme',
|
||||
'search',
|
||||
'security',
|
||||
'sessions',
|
||||
'settings',
|
||||
'signup',
|
||||
'site',
|
||||
'sponsors',
|
||||
'stars',
|
||||
'team',
|
||||
'topics',
|
||||
'trending',
|
||||
'users',
|
||||
'watching',
|
||||
]);
|
||||
|
||||
/**
|
||||
* The hosts whose two-segment paths may be repository URLs: github.com, plus
|
||||
* the channel's own GitHub Enterprise WEB host derived from baseUrl. Only the
|
||||
* web host qualifies: an `api.`-prefixed baseUrl host denotes the deployment's
|
||||
* API, so the stripped form is added and the raw API host is NOT (pasting a
|
||||
* REST URL like https://api.github.com/repos/octo/repo must not shorten to
|
||||
* `repos/octo`). The baseUrl's own API path prefix (e.g. /api/v3) is returned
|
||||
* alongside so REST-shaped paths on a shared web+API host are excluded too.
|
||||
* Any other host — gist.github.com, gitlab.com, an arbitrary site — passes
|
||||
* through unchanged for validation to reject.
|
||||
*/
|
||||
function repositoryUrlContext(baseUrl: string): {
|
||||
hosts: Set<string>;
|
||||
apiPath: string;
|
||||
} {
|
||||
const hosts = new Set(['github.com', 'www.github.com']);
|
||||
let apiPath = '';
|
||||
const trimmed = baseUrl.trim();
|
||||
if (trimmed) {
|
||||
try {
|
||||
const url = new URL(trimmed);
|
||||
const host = url.hostname.toLowerCase();
|
||||
hosts.add(host.startsWith('api.') ? host.slice(4) : host);
|
||||
const path = url.pathname.replace(/\/+$/, '').toLowerCase();
|
||||
if (path) apiPath = path;
|
||||
} catch {
|
||||
// A malformed baseUrl is its own field's validation error; only
|
||||
// github.com remains shortenable here.
|
||||
}
|
||||
}
|
||||
return { hosts, apiPath };
|
||||
}
|
||||
|
||||
/**
|
||||
* Canonicalize the repository filter input: lowercase, deduplicate, strip
|
||||
* trailing sentence punctuation a paste can carry (`.`, `)`, …), and shorten
|
||||
* GitHub web URLs (repository, PR, or issue pages, with an optional `.git`,
|
||||
* trailing path, query, or fragment) to `owner/repo` — only when the URL's
|
||||
* host is the channel's own GitHub web host and its path is not the baseUrl's
|
||||
* API prefix. Entries that are not repository URLs on that host pass through
|
||||
* unchanged so validation can name them. Shared by the validator and the save
|
||||
* path so the two cannot disagree.
|
||||
*/
|
||||
export function normalizeRepositoryFilterInput(
|
||||
value: string,
|
||||
baseUrl = '',
|
||||
): string {
|
||||
const { hosts, apiPath } = repositoryUrlContext(baseUrl);
|
||||
const entries = toList(value)
|
||||
.map((raw) => raw.replace(/[.,;:!?)\]]+$/, ''))
|
||||
.filter(Boolean)
|
||||
.map((entry) => {
|
||||
const match = GITHUB_REPOSITORY_URL_RE.exec(entry);
|
||||
let url: URL | undefined;
|
||||
if (match) {
|
||||
try {
|
||||
url = new URL(entry);
|
||||
} catch {
|
||||
url = undefined;
|
||||
}
|
||||
}
|
||||
const onApiPath =
|
||||
url !== undefined &&
|
||||
apiPath !== '' &&
|
||||
url.pathname.toLowerCase().startsWith(`${apiPath}/`);
|
||||
const shortened =
|
||||
match &&
|
||||
url !== undefined &&
|
||||
hosts.has(url.hostname.toLowerCase()) &&
|
||||
!onApiPath &&
|
||||
!GITHUB_RESERVED_URL_PATHS.has(match[1].toLowerCase())
|
||||
? `${match[1]}/${match[2]}`
|
||||
: entry;
|
||||
return shortened.toLowerCase();
|
||||
});
|
||||
return [...new Set(entries)].join(', ');
|
||||
}
|
||||
|
||||
export function toList(value: string): string[] {
|
||||
return value
|
||||
.split(',')
|
||||
@@ -224,6 +373,7 @@ function initialFieldValue(
|
||||
if (field.key === 'privatePolicy')
|
||||
return configuredPrivatePolicy(instance);
|
||||
if (field.key === 'groupPolicy') return 'disabled';
|
||||
if (field.key === 'repoFilterMode') return 'off';
|
||||
if (field.key === 'outputMode') return field.default ?? '';
|
||||
if (field.key === 'dmPolicy') return 'open';
|
||||
if (field.key !== 'sessionScope') return '';
|
||||
@@ -434,6 +584,23 @@ export function validateChannelEditorDraft(
|
||||
) {
|
||||
errors['prWatchEvents'] = 'requiresBoundDelivery';
|
||||
}
|
||||
// Mirror validateRepoFilterConfig: an active mode needs a non-empty list
|
||||
// of owner/repo entries. While the mode is off the list is ignored (and
|
||||
// hidden), matching the runtime's ignore-while-off rule, so a parked list
|
||||
// never strands a save on a field the operator cannot see.
|
||||
if (isRepoFilterActive(draft.values['repoFilterMode'])) {
|
||||
const normalized = normalizeRepositoryFilterInput(
|
||||
String(draft.values['repoFilter'] ?? ''),
|
||||
String(draft.values['baseUrl'] ?? ''),
|
||||
);
|
||||
if (!normalized) {
|
||||
errors['repoFilter'] = 'repoFilterRequired';
|
||||
} else if (
|
||||
toList(normalized).some((id) => !REPOSITORY_FULL_NAME_RE.test(id))
|
||||
) {
|
||||
errors['repoFilter'] = 'invalidRepoId';
|
||||
}
|
||||
}
|
||||
}
|
||||
if (
|
||||
!isRepositoryOnlyChannelType(descriptor.type) &&
|
||||
@@ -843,6 +1010,22 @@ export function buildChannelUpsertRequest(
|
||||
}
|
||||
assignField(config, field, draft.values[field.key]);
|
||||
}
|
||||
if (
|
||||
repositoryOnly &&
|
||||
isRepoFilterActive(config['repoFilterMode']) &&
|
||||
Array.isArray(config['repoFilter'])
|
||||
) {
|
||||
// Same canonicalization the validator applied, so pasted URLs reach the
|
||||
// daemon as owner/repo. Gated on the active mode: a parked list stays
|
||||
// exactly as stored on an unrelated save.
|
||||
const normalized = normalizeRepositoryFilterInput(
|
||||
config['repoFilter']
|
||||
.filter((entry): entry is string => typeof entry === 'string')
|
||||
.join(', '),
|
||||
typeof config['baseUrl'] === 'string' ? config['baseUrl'] : '',
|
||||
);
|
||||
config['repoFilter'] = normalized ? normalized.split(', ') : [];
|
||||
}
|
||||
if (repositoryOnly) {
|
||||
// The editor used to persist the injected private-access defaults here
|
||||
// even though these channels never read them, and the pairing panel would
|
||||
|
||||
@@ -1595,6 +1595,69 @@ for (const theme of THEMES) {
|
||||
);
|
||||
});
|
||||
|
||||
test(`Channel repository filter editor`, async ({ page }, testInfo) => {
|
||||
const scenario = createWebShellDaemonScenario({
|
||||
capabilities: {
|
||||
features: [
|
||||
'session_events',
|
||||
'permission_vote',
|
||||
'session_permission_vote',
|
||||
'session_scope_override',
|
||||
'session_source_metadata',
|
||||
'channel_management',
|
||||
],
|
||||
},
|
||||
channelTypes: [
|
||||
{
|
||||
type: 'github',
|
||||
displayName: 'GitHub',
|
||||
manageable: true,
|
||||
fields: githubManagement.fields,
|
||||
},
|
||||
],
|
||||
});
|
||||
await page.addInitScript(() => {
|
||||
window.sessionStorage.setItem('qwen-daemon-token', 'visual-token');
|
||||
});
|
||||
const daemon = await installScenario(
|
||||
page,
|
||||
scenario,
|
||||
resolveBaseURL(testInfo),
|
||||
);
|
||||
await gotoSession(page, scenario, daemon, theme);
|
||||
|
||||
await page.getByRole('button', { name: 'Channels' }).click();
|
||||
await page.getByRole('button', { name: 'Configure GitHub' }).click();
|
||||
const editor = page.getByRole('dialog');
|
||||
await expect(
|
||||
editor.getByRole('heading', { name: 'Configure GitHub' }),
|
||||
).toBeVisible();
|
||||
await editor
|
||||
.getByRole('switch', { name: 'Use Local GitHub CLI Authentication' })
|
||||
.click();
|
||||
|
||||
// Anchors: the filter defaults to Off and the repository list stays
|
||||
// hidden until a mode is chosen. A renderer that always shows the list
|
||||
// (or lacks the field) fails here instead of shipping a stale capture.
|
||||
const modeSelect = editor.getByLabel('Repository Filter');
|
||||
await modeSelect.scrollIntoViewIfNeeded();
|
||||
await expect(editor.getByLabel('Filtered Repositories')).toHaveCount(0);
|
||||
await captureScreenshot(page, `channel-repo-filter-off-${theme}`);
|
||||
|
||||
await modeSelect.click();
|
||||
await page
|
||||
.getByRole('option', {
|
||||
name: 'Blocklist — all except listed repositories',
|
||||
})
|
||||
.click();
|
||||
const repositories = editor.getByLabel('Filtered Repositories');
|
||||
await expect(repositories).toBeVisible();
|
||||
await repositories.fill(
|
||||
'https://github.com/QwenLM/qwen-code/pull/16117, octo/noisy-repo',
|
||||
);
|
||||
await captureScreenshot(page, `channel-repo-filter-blocklist-${theme}`);
|
||||
});
|
||||
|
||||
test(`Channel events`, async ({ page }, testInfo) => {
|
||||
const scenario = createWebShellDaemonScenario({
|
||||
capabilities: {
|
||||
|
||||
@@ -4770,6 +4770,18 @@ const EN: Messages = {
|
||||
'member_feature_requested — organization members asked to enable a feature such as Copilot',
|
||||
'channels.editor.field.github.reasonFilter.option.security_advisory_credit':
|
||||
'security_advisory_credit — the bot was credited for contributing to a security advisory',
|
||||
'channels.editor.field.github.repoFilterMode': 'Repository Filter',
|
||||
'channels.editor.field.github.repoFilterMode.description':
|
||||
'Filter incoming notifications by repository. Notifications about PRs bound to a session are filtered too; only their PR state watch events bypass the filter.',
|
||||
'channels.editor.field.github.repoFilterMode.option.off':
|
||||
'Off — all repositories',
|
||||
'channels.editor.field.github.repoFilterMode.option.allowlist':
|
||||
'Allowlist — only listed repositories',
|
||||
'channels.editor.field.github.repoFilterMode.option.blocklist':
|
||||
'Blocklist — all except listed repositories',
|
||||
'channels.editor.field.github.repoFilter': 'Filtered Repositories',
|
||||
'channels.editor.field.github.repoFilter.description':
|
||||
'Comma-separated owner/repo names. You can paste repository or PR URLs; they are shortened to owner/repo on save.',
|
||||
'channels.editor.field.github.deliverToBoundSessions':
|
||||
'Deliver To PR-Bound Sessions',
|
||||
'channels.editor.field.github.deliverToBoundSessions.description':
|
||||
@@ -4997,6 +5009,8 @@ const EN: Messages = {
|
||||
"Remove values that aren't in the allowed list.",
|
||||
'channels.editor.validation.requiresBoundDelivery':
|
||||
'Enable "Deliver To PR-Bound Sessions" to save the selected watch events.',
|
||||
'channels.editor.validation.repoFilterRequired':
|
||||
'List at least one repository, or set Repository Filter to Off.',
|
||||
'channels.editor.validation.number': 'Enter a valid number.',
|
||||
'channels.editor.validation.outOfRange': (v) =>
|
||||
v?.min !== undefined && v?.max !== undefined
|
||||
@@ -9472,6 +9486,17 @@ const ZH: Messages = {
|
||||
'member_feature_requested — 组织成员请求启用 Copilot 等功能',
|
||||
'channels.editor.field.github.reasonFilter.option.security_advisory_credit':
|
||||
'security_advisory_credit — 机器人因参与安全公告而被致谢',
|
||||
'channels.editor.field.github.repoFilterMode': '按仓库过滤',
|
||||
'channels.editor.field.github.repoFilterMode.description':
|
||||
'按仓库过滤入站通知。绑定到会话的 PR,其通知同样被过滤;只有它们的状态监视事件绕过此过滤器。',
|
||||
'channels.editor.field.github.repoFilterMode.option.off': '关闭 — 所有仓库',
|
||||
'channels.editor.field.github.repoFilterMode.option.allowlist':
|
||||
'白名单 — 仅列出的仓库',
|
||||
'channels.editor.field.github.repoFilterMode.option.blocklist':
|
||||
'黑名单 — 除列出的仓库以外',
|
||||
'channels.editor.field.github.repoFilter': '过滤的仓库',
|
||||
'channels.editor.field.github.repoFilter.description':
|
||||
'以逗号分隔的 owner/repo 仓库名。可以直接粘贴仓库或 PR 链接,保存时会自动缩短为 owner/repo。',
|
||||
'channels.editor.field.github.deliverToBoundSessions': '投递到 PR 绑定会话',
|
||||
'channels.editor.field.github.deliverToBoundSessions.description':
|
||||
'将已绑定到活跃会话的 PR 事件以 prompt 形式投递进该会话,而不是频道自有会话。需要 daemon 托管的频道。',
|
||||
@@ -9685,6 +9710,8 @@ const ZH: Messages = {
|
||||
'channels.editor.validation.invalidOption': '请移除不在允许列表中的值。',
|
||||
'channels.editor.validation.requiresBoundDelivery':
|
||||
'需开启「投递到 PR 绑定会话」才能保存所选的监视事件。',
|
||||
'channels.editor.validation.repoFilterRequired':
|
||||
'请至少列出一个仓库,或把「按仓库过滤」设为关闭。',
|
||||
'channels.editor.validation.number': '请输入有效数字。',
|
||||
'channels.editor.validation.outOfRange': (v) =>
|
||||
v?.min !== undefined && v?.max !== undefined
|
||||
|
||||
Reference in new issue
Block a user