mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-21 14:46:19 +00:00
fix(github-channel): validate and document reasonFilter (#8035)
* feat(github-channel): add reasonFilter config to skip unwanted notification reasons Adds an optional `reasonFilter` allowlist to the GitHub channel config. When set, notifications whose `reason` is not in the list are skipped before any lane dispatch, reducing unnecessary API calls and agent work for notification types the operator does not care about. - New `reasonFilter?: string[]` field on `GithubConfig` - O(1) Set lookup (`reasonFilterSet`); undefined = no filter (all reasons) - Early-skip in the poll loop, before subject URL extraction and lane dispatch - Two tests: filtered reasons skipped, unset filter processes all Default behavior is unchanged (undefined = process all reasons). * fix(channels): log github reason filter skips * fix(channels): validate github reason filter * fix(channels): address github reason filter comments * fix(github-channel): reject invalid reason filters * fix(github-channel): validate reason filters on connect --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
This commit is contained in:
易良
GitHub
parent
d2ab8a5597
commit
3d5924bd2f
3 changed files with 165 additions and 6 deletions
|
|
@ -71,10 +71,16 @@ For GitHub Enterprise Server, set `baseUrl`:
|
|||
| `token` | (required) | Classic PAT with `notifications` scope |
|
||||
| `pollInterval` | `60000` | Poll interval in ms |
|
||||
| `baseUrl` | `https://api.github.com` | API base URL (for GHE) |
|
||||
| `reasonFilter` | all reasons | Optional list of GitHub notification reasons to process before dispatch |
|
||||
| `groupPolicy` | `"disabled"` | Must be `"open"` for notifications to flow |
|
||||
| `senderPolicy` | `"allowlist"` | Who can trigger the bot |
|
||||
| `groups.*.requireMention` | `true` | Require @mentions for ordinary comments; directed notification reasons still run |
|
||||
| `reasonFilter` | unset | Optional allowlist of GitHub notification reasons to process |
|
||||
|
||||
Use `reasonFilter` to drop noisy notification classes such as `ci_activity` or `state_change`. Do not use `reasonFilter: ["mention"]` as a replacement for `groups.*.requireMention`: GitHub's `mention` reason is sticky at the thread level, so real new @mentions can arrive later under `comment`, `subscribed`, `author`, or other reasons and would be skipped.
|
||||
|
||||
Valid `reasonFilter` values are `mention`, `review_requested`, `assign`, `author`, `comment`, `ci_activity`, `manual`, `state_change`, `subscribed`, `team_mention`, `security_alert`, `approval_requested`, `invitation`, `member_feature_requested`, and `security_advisory_credit`.
|
||||
|
||||
Filtered notifications are still marked read before they are skipped. Removing the filter later will not replay notifications the channel already skipped.
|
||||
|
||||
## ⚠️ Security
|
||||
|
||||
|
|
|
|||
|
|
@ -922,7 +922,7 @@ describe('GithubChannel', () => {
|
|||
|
||||
it('normalizes configured reasonFilter entries before matching', async () => {
|
||||
await initWithoutLoop({
|
||||
reasonFilter: [' COMMENT ', 123, ''],
|
||||
reasonFilter: [' COMMENT ', ''],
|
||||
});
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([
|
||||
|
|
@ -1118,6 +1118,117 @@ describe('GithubChannel', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('reasonFilter', () => {
|
||||
function connectWithReasonFilter(reasonFilter: unknown): Promise<void> {
|
||||
channel = new TestableGithubChannel(
|
||||
'test-github',
|
||||
makeConfig({ reasonFilter }),
|
||||
makeBridge(),
|
||||
);
|
||||
return channel.connect();
|
||||
}
|
||||
|
||||
it('skips notifications whose reason is not in the allowlist', async () => {
|
||||
const stderrWrite = vi
|
||||
.spyOn(process.stderr, 'write')
|
||||
.mockImplementation(() => true);
|
||||
await initWithoutLoop({
|
||||
reasonFilter: ['mention'],
|
||||
});
|
||||
try {
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([
|
||||
makeNotification({
|
||||
reason: 'comment',
|
||||
last_read_at: '2026-07-01T12:00:00.000Z',
|
||||
}),
|
||||
makeNotification({
|
||||
reason: 'mention',
|
||||
last_read_at: '2026-07-01T12:00:00.000Z',
|
||||
}),
|
||||
])
|
||||
.mockResolvedValueOnce([makeComment({ body: 'hello @test-bot' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(1);
|
||||
expect(channel.inboundEnvelopes[0]!.metadata).toContain(
|
||||
'Trigger: mention.',
|
||||
);
|
||||
expect(
|
||||
mockOctokit.rest.activity.markNotificationsAsRead,
|
||||
).toHaveBeenCalledWith({
|
||||
last_read_at: '2026-07-02T10:00:00.000Z',
|
||||
read: true,
|
||||
});
|
||||
expect(stderrWrite).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
'skipping notification (reason=comment not in reasonFilter, subject=https://api.github.com/repos/owner/repo/issues/42)',
|
||||
),
|
||||
);
|
||||
} finally {
|
||||
stderrWrite.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects unrecognized reasonFilter values', async () => {
|
||||
await expect(connectWithReasonFilter(['mentions'])).rejects.toThrow(
|
||||
'Unrecognized reasonFilter values for channel test-github: mentions',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects non-array reasonFilter values', async () => {
|
||||
await expect(connectWithReasonFilter('mention')).rejects.toThrow(
|
||||
'reasonFilter for channel test-github must be an array of GitHub notification reasons.',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects non-string reasonFilter entries', async () => {
|
||||
await expect(connectWithReasonFilter([42])).rejects.toThrow(
|
||||
'reasonFilter entries for channel test-github must be strings.',
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts documented security notification reasons', async () => {
|
||||
await expect(
|
||||
connectWithReasonFilter(['security_alert']),
|
||||
).resolves.toBeUndefined();
|
||||
channel.disconnect();
|
||||
});
|
||||
|
||||
it('processes all reasons when filter is empty or unset', async () => {
|
||||
await initWithoutLoop();
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([
|
||||
makeNotification({
|
||||
reason: 'subscribed',
|
||||
last_read_at: '2026-07-01T12:00:00.000Z',
|
||||
}),
|
||||
])
|
||||
.mockResolvedValueOnce([makeComment({ body: 'plain comment' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('processes all reasons when filter is an empty array', async () => {
|
||||
await initWithoutLoop({ reasonFilter: [] });
|
||||
mockOctokit.paginate
|
||||
.mockResolvedValueOnce([
|
||||
makeNotification({
|
||||
reason: 'subscribed',
|
||||
last_read_at: '2026-07-01T12:00:00.000Z',
|
||||
}),
|
||||
])
|
||||
.mockResolvedValueOnce([makeComment({ body: 'plain comment' })]);
|
||||
|
||||
await pollOnce();
|
||||
|
||||
expect(channel.inboundEnvelopes).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('publication contract', () => {
|
||||
async function connectForPublication() {
|
||||
mockOctokit.paginate.mockResolvedValue([]);
|
||||
|
|
|
|||
|
|
@ -22,6 +22,24 @@ interface GithubConfig extends ChannelConfig {
|
|||
reasonFilter?: unknown;
|
||||
}
|
||||
|
||||
const KNOWN_NOTIFICATION_REASONS = new Set([
|
||||
'mention',
|
||||
'review_requested',
|
||||
'assign',
|
||||
'author',
|
||||
'comment',
|
||||
'ci_activity',
|
||||
'manual',
|
||||
'state_change',
|
||||
'subscribed',
|
||||
'team_mention',
|
||||
'security_alert',
|
||||
'approval_requested',
|
||||
'invitation',
|
||||
'member_feature_requested',
|
||||
'security_advisory_credit',
|
||||
]);
|
||||
|
||||
interface GithubCursor {
|
||||
lastProcessedAt: string;
|
||||
metaFloor?: string;
|
||||
|
|
@ -85,13 +103,34 @@ interface NotificationContext {
|
|||
reason: string;
|
||||
}
|
||||
|
||||
function normalizeReasonFilter(config: GithubConfig): Set<string> | null {
|
||||
if (!Array.isArray(config.reasonFilter)) return null;
|
||||
function normalizeReasonFilter(
|
||||
config: GithubConfig,
|
||||
channelName: string,
|
||||
): Set<string> | null {
|
||||
if (config.reasonFilter === undefined) return null;
|
||||
if (!Array.isArray(config.reasonFilter)) {
|
||||
throw new Error(
|
||||
`reasonFilter for channel ${channelName} must be an array of GitHub notification reasons.`,
|
||||
);
|
||||
}
|
||||
if (config.reasonFilter.some((reason) => typeof reason !== 'string')) {
|
||||
throw new Error(
|
||||
`reasonFilter entries for channel ${channelName} must be strings.`,
|
||||
);
|
||||
}
|
||||
const reasons = config.reasonFilter
|
||||
.filter((reason): reason is string => typeof reason === 'string')
|
||||
.map((reason) => reason.trim().toLowerCase())
|
||||
.filter((reason) => reason.length > 0);
|
||||
return new Set(reasons);
|
||||
const unknownReasons = reasons.filter(
|
||||
(reason) => !KNOWN_NOTIFICATION_REASONS.has(reason),
|
||||
);
|
||||
if (unknownReasons.length > 0) {
|
||||
throw new Error(
|
||||
`Unrecognized reasonFilter values for channel ${channelName}: ${unknownReasons.join(', ')}`,
|
||||
);
|
||||
}
|
||||
return reasons.length > 0 ? new Set(reasons) : null;
|
||||
}
|
||||
|
||||
interface PostedGithubComment {
|
||||
|
|
@ -212,7 +251,7 @@ export class GithubChannel extends PollingChannelBase<GithubCursor> {
|
|||
|
||||
async connect(): Promise<void> {
|
||||
const cfg = this.config as GithubConfig;
|
||||
this.reasonFilter = normalizeReasonFilter(cfg);
|
||||
this.reasonFilter = normalizeReasonFilter(cfg, this.name);
|
||||
const baseUrl = cfg.baseUrl || 'https://api.github.com';
|
||||
this.webOrigin = baseUrl
|
||||
.replace(/\/api\/v3\/?$/, '')
|
||||
|
|
@ -444,6 +483,9 @@ export class GithubChannel extends PollingChannelBase<GithubCursor> {
|
|||
const lastReadAt = notification.last_read_at;
|
||||
const reason = String(notification.reason ?? '').toLowerCase();
|
||||
if (this.reasonFilter && !this.reasonFilter.has(reason)) {
|
||||
process.stderr.write(
|
||||
`[Channel:${this.name}] skipping notification (reason=${reason} not in reasonFilter, subject=${notification.subject.url})\n`,
|
||||
);
|
||||
this.logDebugPayload('Github', {
|
||||
event: 'reasonFilter.skip',
|
||||
chatId,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue