mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
feat(web-shell): edit GitHub repository settings from the channel editor (#16149)
* feat(web-shell): edit GitHub repository settings from the channel editor Three settings decide how a GitHub Channel behaves and none of them could be changed from the Web Shell: which accounts may speak in a given repository, whether that repository answers without a mention, and how often the notifications API is polled. Narrowing one repository meant hand-editing settings.json on the daemon host, so the permissive channel-wide default stayed in place by inertia. The poll interval becomes a declared descriptor field, which is also what makes it persistable: the store rejects an undeclared key outright. Per-repository settings stay in the editor, because the store already validates their shapes and a new descriptor field kind would touch the SDK and every client for a shape only one platform family uses. Each repository row is tri-state and defaults to inherit. The save compares every field against the same reading of the stored object it seeded the draft from, so an untouched repository keeps its keys verbatim -- including an empty allowlist, which means nobody, and a list with no policy beside it, which inherits the policy but overrides the members. Materializing resolved defaults instead would freeze today's inheritance and stop a later change to the channel-wide default from reaching those repositories. The section map is keyed by field name, so Email's existing poll interval moves to Connection alongside GitHub's; a test pins that rather than leaving it to prose. * fix(web-shell): let a chosen repository allowlist own its empty speaker list Picking "Allowlist only" on a repository row and leaving the speaker box empty persisted the policy with no list at all. The runtime resolves a missing per-repository list through `??` to the channel-wide one, so the operator's most restrictive available action kept the wider audience while the field beside it said "Empty means nobody." The merge only wrote the list when the speaker text changed. On a freshly added row the text starts empty and stays empty, so changing only the policy skipped the write entirely -- the default state of the feature, not an edge case. Choosing the policy now owns the list too. The rule is deliberately scoped to a policy the operator actually changed: a stored allowlist with no list is a legitimate hand-written shape meaning "members inherited", and writing an empty list there would narrow a channel nobody edited. Both directions have their own case and their own mutation. * fix(web-shell): make the repository speaker list symmetric and its hint truthful Two defects in the per-repository speaker list, both in the path where the operator moves the policy without touching the text. Leaving an allowlist behind kept its empty list. An empty array is not nullish, so the runtime kept denying everyone while the row read "Inherit" and the hint promised the channel-wide list was back. No gesture the form offers could repair it, because every one of them reads back as an empty box and wrote nothing. The policy-changed branch now handles both directions: entering an allowlist owns an empty list, leaving one releases an empty list, and a list with names in it stays because the box still shows it. The hint was derived from the draft policy alone, which tells two deliberately preserved stored shapes the opposite of the truth: a stored allowlist with no list inherits the channel-wide members but was told "empty means nobody", and a stored empty list with no policy admits nobody but was told "leave empty to keep the channel-wide list". The row now asks the save path what it will own, so the copy and the persisted object cannot diverge again. Also pins the paths the previous suite left unexecuted: returning a stored require-mention and policy to inheritance, writing a list while the policy stays inherited, keeping a stored list when the policy moves to an allowlist, each row showing and editing only its own speakers, an explicitly open row, and the localized interval label. * fix(channels): bound the poll interval and stop a new bound stranding old channels Declaring pollInterval exposed two problems the field did not have while it was unknown to the descriptor. Nothing bounded it from above. Node clamps a timeout past its 32-bit range to 1ms, so "poll once a month" persisted a hot loop that burns the notifications quota and settles into error backoff. Number descriptors now carry a maximum beside exclusiveMinimum, mirrored through the SDK, the registry's metadata validation, the store and the editor, and GitHub declares the same one-day ceiling Email already enforces at runtime. PollingChannelBase also falls back past it, so a hand-written configuration cannot hot-loop either. And a stored value the runtime tolerates became unsavable. A channel holding pollInterval 0 works today because the poll loop falls back to 60s, but the editor re-sends that 0 verbatim and both the editor and the store now rejected it -- blocking an operator who only wanted to add a repository. Both skip the range check for a finite number deep-equal to what is stored. The tolerance is deliberately not generalized: an existing contract pins that an unchanged stored scalar of the wrong type is still rejected, and a type error should be fixed rather than forgiven. Also names the repositories whose settings outlive a switch away from the allowlist policy. The runtime resolves per-repository senders and requireMention without consulting groupPolicy, so those entries stayed enforced while the rows disappeared; deleting them would destroy hand-written configuration, so the editor lists them and says how to edit them instead. Closes the test gaps the review found: the delete-to-inherit paths, writing a list under an inherited policy, the array guard, per-row speaker values and two-row editing, an explicitly open row, the localized label, a mixed-case chat group id, unmodelled keys on a case-retyped repository, the descriptor field itself, and row-scoped visual assertions with the last row in frame. * fix(web-shell): make the repository hints describe what saving actually does Three places where the form promised something the save would not do, plus a ceiling that repaired too much. The speaker hint asked the projection what the current draft owns. A repository storing a speaker list with no policy beside it therefore read "empty means nobody", while clearing the box deletes the list and hands the repository back the channel-wide members -- widening it while reporting success. The hint now asks the projection about an empty box, which is the hypothetical it describes. Row edits were projected only under the allowlist policy. Editing a row and then switching the group policy rebuilt groups from the stored config, so the save closed the dialog and restored the value the operator had just changed. The non-allowlist branch projects the same overrides now, and still drops entries that carry nothing. The retained-settings alert claimed the settings still apply under every policy that is not Allowlist. Under Disabled the group gate drops every envelope before either lookup, so the alert now says they are kept but not applied, and the Chinese copy names the option the select actually renders. The interval ceiling fell back to 60s for anything above one day. Values between one day and Node's 32-bit timer limit were honoured before and are not hot loops, so a two-day GitLab interval became 1440 polls a day. Clamp instead of replacing, and state the ceiling in the GitLab and hosting-contract docs. The range message also invented a lower bound of 0 for a field declaring only a maximum, and now names only the bounds the descriptor declares. Tests: the three registry maximum-validation branches, the maximum half of the unchanged-value tolerance on both the editor and the store, the inherit sentinel mapping through both row selects, the disabled alert, the empty-box hint, and both shapes of the range message. Docs: the design no longer claims no SDK or schema change, credits the right class for speaker resolution, and the serve protocol reference documents maximum. * fix(web-shell): say what an empty speaker box saves, and sweep the invariant The inherited-speaker hint promised to "keep" the channel-wide list, but a repository storing its own list is not keeping it: clearing the box deletes that list and hands the repository the channel-wide members, which can be many more people. "Use" holds for every shape that reaches the hint. The case pinning that shape now also clears the box and asserts the request, so the copy and the save are pinned together rather than separately. Pass the group policy into the retained-settings alert instead of deriving it a second time. The two derivations were equivalent, but nothing kept them that way, and a drift between them is what made the alert claim settings still apply under a disabled policy. Add a three-part invariant sweep over twelve stored shapes crossed with every single-field edit, driven through the real save path: an untouched row reproduces its stored object byte-for-byte, the empty-box hint agrees with what that save admits, and no edit widens the admitted set unless the row's own controls asked for it. Admission resolves per-repository then wildcard, so an absent list means the channel-wide members rather than nobody. Correct the docs where they described the superseded implementation: the hint decision still named the current-draft projection, the user guide contradicted itself on clearing an explicit allowlist, the clamp's blast radius omitted Email not extending the base at all, and the files table listed fifteen of the twenty-four files this change touches. * fix(web-shell): keep repository edits on every non-Allowlist save path The branch that retains per-repository settings when the group policy leaves Allowlist was gated on the STORED policy, so a channel already open never reached it: an operator who switched to Allowlist, narrowed a repository and switched back saved successfully while the request restored the old open entry. It also iterated stored groups only, so a repository typed during that visit was never written and fell back to the wildcard. Both admitted a sender the operator had just excluded, and the alert promises those edits are kept. Projecting the draft onto conflicting aliases was the third path. The draft holds one entry per lower-cased id, so two stored spellings that disagree both received the last one's settings, turning an alice-only repository open while the conflict check stayed silent — it only runs when the final policy is Allowlist. Those entries are now left as stored, which is what the runtime resolved before and cannot widen anything. Refusing the save here instead would strand the operator: the conflict error is reported on a field that is not rendered once the policy leaves Allowlist. Both consumers now share one alias set rather than two judgements that could drift. The invariant sweep missed all three because its fixture hardcodes Allowlist, so it never varied the policy dimension. A second sweep crosses the same shapes with the stored and final policies, for a stored repository and one that exists only in the draft, asserting the runtime's own rule: speakers are resolved without consulting groupPolicy, so a row must write the same entry whichever policy the save lands on. * fix(web-shell): keep an ambiguous repository alias exactly as stored Preserving conflicting aliases from the projection was not enough: the empty-entry filter still ran, so a no-op save on a channel stored Open deleted an empty alias. The runtime matches that exact key and inherits the wildcard allowlist, while deleting it lets the case-insensitive fallback reach the wider spelling — a repository nobody edited went from one allowed sender to everyone. The same gate one type over deleted a hand-written non-record value with the same result; the store preserves an unchanged non-record value, so passing it through keeps the save a no-op instead. An ambiguous entry is now exempt from the filter whatever it holds, and a non-ambiguous empty entry still drops so a row returned to inherit leaves no stub. Skipping the draft for ambiguous ids also discarded edits the operator had actually made, and the dialog closed on success while the request restored the stored policy. An edited ambiguous row is now refused, with the error on groupPolicy because the allowlist field it used to be reported on is not rendered once the policy leaves Allowlist. The refusal is scoped to a real change against the reading that seeded the draft, so an untouched switch still saves; refusing everything would strand an operator who only changed the policy. The alias dimension is now crossed into a sweep rather than left at two examples: every stored shape against a disagreeing lower spelling, over both stored policies and all three non-Allowlist final policies, asserting that an untouched save is a byte-for-byte no-op admitting the same people through the runtime's exact-then-fallback lookup, that a real edit is refused, and that a no-op edit still saves. Two dialog cases click the same flows and assert whether onSave is reached, which the state layer cannot see. * fix(web-shell): enforce the interval floor and stop rewriting entries the form cannot show The poll interval had a ceiling but no floor. The descriptor's lower bound is exclusive, so 0.5 passed the editor and the store, and setTimeout rounds it to about 1ms — the hot loop the ceiling exists to prevent, reached from below and persistable only because this change made the field managed. The getter now clamps into both ends from one pair of exported constants, and the GitHub descriptor declares its maximum from the same export so the two cannot drift. An exclusive minimum of 1 is not available, since it would reject the documented 1, so the field description and the user doc state that a smaller positive value is raised to 1. The claim that the store preserves an unchanged non-record group entry was wrong, and it was mine: the tolerance covers the whole groups value, while a single non-record entry is refused on every save. A case against the real store now pins that. Rather than weaken per-entry validation to make the comment true, the editor refuses such a save with a message that names the entry and points at settings.json, because no row can represent it and a generic daemon error leaves nothing to act on. The entry is still not deleted, whether or not it has an alias twin: deleting the exact key the runtime matches first is a rewrite nobody asked for. The retained-settings alert enumerated storage instead of the save, so it never named a repository typed during the visit even though the save writes it, and it announced an allowedUsers list beside an inherited open policy as still applying when senderGateFor builds no allowlist for it. The list now comes from the request the save builds and is classified by resolved effect. An unrelated edit also rewrote the file: a channel that never had an allowlist lost an already-empty entry, because the filter treated every empty entry as a stub. It now distinguishes leaving Allowlist from never having had one, while a repository typed during the visit stays a placeholder that is dropped when it carries nothing.
This commit is contained in:
1 parent
d6b3e1fca7
commit
bf2f003294
25 files changed
+4253
-30
No files matched your search
@@ -68,7 +68,7 @@ Because the cursor is global (not per-thread), a notification that arrives in a
|
||||
`PollingChannelBase<Cursor>` (in `packages/channels/base/`) extends `ChannelBase` and provides the poll loop infrastructure:
|
||||
|
||||
- **Poll loop**: start/stop via `startPollLoop()`/`stopPollLoop()`, called from `connect()`/`disconnect()`
|
||||
- **Poll interval**: read from channel config `pollInterval` (ms), validated as positive finite number, defaults to 60000
|
||||
- **Poll interval**: read from channel config `pollInterval` (ms), validated as a positive finite number, clamped to a maximum of 86400000 (one day), defaults to 60000. The clamp exists because Node clamps a timeout above its 32-bit range to 1ms, which would turn the loop into a hot loop; values between one day and that limit are clamped rather than replaced, so a previously honoured interval is not silently multiplied
|
||||
- **Cursor persistence**: JSON cursor saved atomically after each successful `pollOnce()`; loaded on construction (corrupt or unparseable date → fallback to `createInitialCursor()`)
|
||||
- **Cursor validation**: `validateCursor()` virtual hook — base rejects non-objects and arrays; subclasses add shape checks (e.g. GitHub rejects missing/invalid `lastProcessedAt` date)
|
||||
- **Backoff**: exponential 2s → 30s on poll errors, reset on success
|
||||
|
||||
@@ -0,0 +1,501 @@
|
||||
# GitHub Channel Repository Settings in the Web Shell Editor
|
||||
|
||||
[English](github-channel-repository-settings.md) | [简体中文](github-channel-repository-settings.zh-CN.md)
|
||||
|
||||
Status: implemented in this change. Follows the merged editor usability work (#16096) and closes the
|
||||
"per-repository settings remain editable only through `settings.json`" gap that
|
||||
PR listed under Risk & Scope.
|
||||
|
||||
## Problem statement
|
||||
|
||||
Three settings decide how a GitHub Channel behaves, and none of them can be
|
||||
changed from the Web Shell:
|
||||
|
||||
| Setting | Scope | What it controls |
|
||||
| :------------------------------------------------ | :------------- | :--------------------------------------------------------- |
|
||||
| `groups["<owner/repo>"].senders` + `allowedUsers` | per repository | which GitHub accounts may start tasks in that repository |
|
||||
| `groups["<owner/repo>"].requireMention` | per repository | whether an unmentioned comment is processed |
|
||||
| `pollInterval` | channel | how often the notifications API is polled, in milliseconds |
|
||||
|
||||
The editor today exposes only the channel-wide speaker default
|
||||
(`groups["*"].senders` / `allowedUsers`) and the repository allowlist itself.
|
||||
An operator who wants one repository restricted to two maintainers, or one
|
||||
repository to answer without a mention, must hand-edit `settings.json` on the
|
||||
daemon host and restart nothing — the value is simply invisible in the form.
|
||||
|
||||
The practical consequence is that the permissive default stays in place. The
|
||||
channel-wide speaker default is `open`, so narrowing it means narrowing every
|
||||
repository at once; the only way to narrow one repository is the file the form
|
||||
does not show.
|
||||
|
||||
## Current state
|
||||
|
||||
- `packages/channels/github/src/index.ts` declares `token`, `useLocalGh`,
|
||||
`baseUrl`, `groupPolicy`, `reasonFilter`. It does not declare `pollInterval`,
|
||||
so the descriptor-driven form never renders it. The Email plugin already
|
||||
declares the same key as a `number` field with `exclusiveMinimum: 0` and
|
||||
`default: '60000'`, which is the precedent to follow.
|
||||
- `GithubChannel extends PollingChannelBase`, whose `pollInterval` getter reads
|
||||
`config.pollInterval` and falls back to `60_000` when it is absent, not a
|
||||
finite number, or not positive.
|
||||
- `GroupGate.check()` resolves `requireMention` as
|
||||
`groupConfigFor(chatId)?.requireMention ?? groups["*"]?.requireMention ?? true`,
|
||||
and returns `{allowed: false, reason: 'disabled'}` for a disabled policy before
|
||||
it reaches the mention gate. It contains no speaker logic at all.
|
||||
- The speaker policy and members are resolved by `ChannelBase.groupSendersFor`
|
||||
and `groupAllowedUsersFor`: the per-group `senders` falling back to `"*"` and
|
||||
then to `open`, and the per-group `allowedUsers` falling back to `"*"` and then
|
||||
to `[]`. `senderGateFor` builds the gate from those two. Both classes' lookups
|
||||
go through the case-insensitive group matcher added by #16096.
|
||||
- `channel-editor-state.ts` builds `groups` from the allowlist string, preserving
|
||||
each listed repository's stored settings verbatim, and writes the channel-wide
|
||||
speaker default into `groups["*"]`. Per-repository entries are carried through
|
||||
untouched; there is no draft state for them.
|
||||
- The daemon already accepts exactly the edits this design needs.
|
||||
`channel-settings-store.ts` validates `groups.<id>.requireMention` as a
|
||||
boolean, `senders` as `open` or `allowlist`, and `allowedUsers` as a string
|
||||
array, and it validates descriptor-declared number fields against
|
||||
`exclusiveMinimum`. **The per-repository `groups` edits need no daemon, SDK or
|
||||
descriptor-schema change.** The poll-interval ceiling does extend the
|
||||
number-field descriptor with `maximum`, mirrored at five sites — see decision
|
||||
9 and the files table.
|
||||
|
||||
## Goals
|
||||
|
||||
1. Edit the speaker policy and allowlist of an individual allowlisted repository
|
||||
from the channel editor.
|
||||
2. Edit `requireMention` for an individual allowlisted repository from the same
|
||||
place.
|
||||
3. Edit the channel's poll interval from the same form.
|
||||
4. Keep inheritance intact: a repository the operator has not customized must
|
||||
stay an empty override that inherits from `"*"` and then from the built-in
|
||||
default.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- No change to any default value. The group-policy default divergence is tracked
|
||||
separately in #16144 and is deliberately out of scope here.
|
||||
- No editing of `groups["*"].requireMention`, `dispatchMode`, or
|
||||
`groupHistoryLimit`. Those keys keep round-tripping untouched.
|
||||
- No new descriptor field _kind_, and no per-repository UI for non-repository
|
||||
platforms. Chat platforms keep the current form. The number descriptor does
|
||||
gain a `maximum` bound (decision 9), which is a vocabulary extension mirrored
|
||||
into the SDK types, not a new kind.
|
||||
- No repository picker backed by the GitHub API. Repositories are still typed as
|
||||
`owner/repo` in the allowlist field.
|
||||
|
||||
## Proposed solution
|
||||
|
||||
### Channel layer: declare the poll interval
|
||||
|
||||
Add one field to the GitHub plugin's management descriptor, mirroring Email:
|
||||
|
||||
```json
|
||||
{
|
||||
"key": "pollInterval",
|
||||
"label": "Poll Interval (ms)",
|
||||
"kind": "number",
|
||||
"exclusiveMinimum": 0,
|
||||
"maximum": 86400000,
|
||||
"default": "60000"
|
||||
}
|
||||
```
|
||||
|
||||
The existing descriptor-driven path then renders, validates, and persists it
|
||||
with no editor code. The editor's section map places it in **Connection**
|
||||
alongside `baseUrl`, because it describes how the channel reaches GitHub rather
|
||||
than who may use it.
|
||||
|
||||
The section map is keyed by field name, not by platform, so **Email's existing
|
||||
`pollInterval` field moves from Credentials to Connection as well**. That is the
|
||||
same setting meaning the same thing on a second polling platform, and it is
|
||||
where the field belongs; scoping the map to GitHub would add a conditional just
|
||||
to keep a misfiled field misfiled. No other platform declares the key.
|
||||
|
||||
A new channel's interval stays **empty**: the editor does not prefill number
|
||||
fields from the descriptor default, so nothing is persisted until the operator
|
||||
types a value and the runtime keeps owning its 60-second default. The declared
|
||||
`default` documents the effective value on the wire, matching Email.
|
||||
|
||||
### Editor state: a tri-state override per repository
|
||||
|
||||
Add one draft field:
|
||||
|
||||
```ts
|
||||
export interface ChannelGroupOverrideDraft {
|
||||
/** '' means inherit; otherwise the explicit boolean as a string. */
|
||||
requireMention: '' | 'true' | 'false';
|
||||
/** '' means inherit; otherwise the explicit policy. */
|
||||
senders: '' | 'open' | 'allowlist';
|
||||
allowedUsers: string;
|
||||
}
|
||||
|
||||
// on ChannelEditorDraft
|
||||
groupOverrides: Record<string, ChannelGroupOverrideDraft>;
|
||||
```
|
||||
|
||||
Keys are canonical lower-case repository IDs, matching the keys `assignGroups`
|
||||
already writes for repository-only channels.
|
||||
|
||||
- **Reading.** `createChannelEditorDraft` seeds one entry per stored repository
|
||||
key (excluding `"*"`), translating a stored `requireMention` boolean into
|
||||
`'true'` / `'false'` and an absent key into `''`. A stored `senders` becomes
|
||||
itself; absent becomes `''`. `allowedUsers` is joined with `, ` exactly like
|
||||
the channel-wide field. The reader is module-private and shared with the save
|
||||
path.
|
||||
- **Writing.** `assignGroups` merges the draft override over the stored group
|
||||
object **field by field, comparing against that same reading of the stored
|
||||
object**. A field whose draft value still equals the stored reading keeps its
|
||||
stored key verbatim; only a changed field is written, and a changed field set
|
||||
back to `''` has its key deleted. Keys the editor does not model
|
||||
(`dispatchMode`, `groupHistoryLimit`, and anything unknown) are copied through
|
||||
untouched. A repository removed from the allowlist loses its entry, as today.
|
||||
- **Why compare instead of overwrite.** Two stored shapes cannot be expressed by
|
||||
a tri-state control plus a text box: an empty allowlist (`allowedUsers: []`,
|
||||
meaning _nobody_) and a list with no policy beside it (`allowedUsers` without
|
||||
`senders`, meaning _inherit the policy, override the members_). A plain
|
||||
"write the parsed list, delete when empty" rule would turn both into something
|
||||
else on a save the operator meant as a no-op. Comparing against the initial
|
||||
reading makes an untouched field untouchable by construction, so the
|
||||
round-trip is exact without tracking dirty flags.
|
||||
- **An explicit allowlist owns its list.** When the row's own policy is
|
||||
`allowlist`, an empty text box writes `[]` — nobody — rather than deleting the
|
||||
key and silently inheriting the channel-wide list. When the row inherits,
|
||||
clearing the text deletes the per-repository list so the channel-wide one
|
||||
applies again. The two cases are told apart by the merged group's `senders`.
|
||||
- **Choosing the policy owns the list too, even if the text never moved.** This
|
||||
is the default state of a freshly added row, so it is the most likely path
|
||||
rather than an edge case: the operator picks "Allowlist only", leaves the box
|
||||
empty, and both readings of `allowedUsers` are `''`. A guard that only fires
|
||||
when the _text_ changed would skip the write, persist `{senders: 'allowlist'}`
|
||||
with no list, and the runtime's `??` would hand the channel-wide list straight
|
||||
back — the most restrictive action available would be a no-op that the UI
|
||||
reports as success. So the empty list is also written when the operator
|
||||
**changed the policy** to `allowlist` and the stored group has no list.
|
||||
- **That rule is scoped to a changed policy, deliberately.** A stored
|
||||
`{senders: 'allowlist'}` with no `allowedUsers` is a legitimate hand-written
|
||||
shape meaning "allowlist policy, members inherited from `'*'`". Writing `[]`
|
||||
there would narrow a channel nobody edited — the same class of silent
|
||||
migration the compare-with-initial rule exists to prevent. Both directions
|
||||
are pinned by their own case and their own mutation.
|
||||
- **Inheritance is the default.** A repository with no customization writes `{}`
|
||||
and keeps inheriting. The editor must never materialize `requireMention: true`
|
||||
or `senders: 'open'` into an untouched repository: doing so would freeze
|
||||
today's default into the configuration and silently stop a later change to
|
||||
`"*"` from reaching that repository.
|
||||
|
||||
### Editor UI: one row per allowlisted repository
|
||||
|
||||
Inside the existing Access control section, directly under the repository
|
||||
allowlist field and only when the group policy is `allowlist`, render one block
|
||||
per repository parsed from the allowlist string (same `toList` + lower-casing +
|
||||
de-duplication the save path uses). Each block shows the repository ID as its
|
||||
heading and three controls:
|
||||
|
||||
| Control | Widget | Values |
|
||||
| :--------------- | :--------- | :---------------------------------------------------------------------------------------------------------------------- |
|
||||
| Require mention | select | Inherit / Require a mention / Answer without a mention |
|
||||
| Who may speak | select | Inherit / Anyone / Allowlist only |
|
||||
| Allowed speakers | text input | comma-separated GitHub logins, shown when the row's _effective_ policy is `allowlist` — its own, or the one it inherits |
|
||||
|
||||
The inherited value is named in the option label so the operator can see what
|
||||
they are inheriting — for example `Inherit (require a mention)` when `"*"` or the
|
||||
built-in default resolves to `true`. The resolution shown is the same
|
||||
per-repository → `"*"` → built-in order the runtime uses. The speaker input is
|
||||
gated on the effective policy rather than the row's own, because a repository
|
||||
that inherits an allowlist still has a member list worth editing; its helper
|
||||
text says that leaving it empty uses the channel-wide list.
|
||||
|
||||
The row's option labels and placeholder are repository-specific (`Anyone`,
|
||||
`Allowlist only`, `alice, bob`). Reusing the shared group-speaker strings would
|
||||
put `Any group member`, `Listed members only`, and `user-a, user-b` back into a
|
||||
GitHub form — the exact chat/repository mismatch #16096 removed.
|
||||
|
||||
Each row is a `role="group"` labelled by its own repository heading. Without
|
||||
that, every row's controls would share the accessible names "Require a mention"
|
||||
and "Who may speak", and a screen-reader user could not tell which repository
|
||||
they were editing once more than one is listed.
|
||||
|
||||
Rows are derived from the allowlist on every render, so typing a repository adds
|
||||
its row immediately and deleting it removes the row. Override state for a
|
||||
removed repository is dropped on save by `assignGroups`; no separate cleanup
|
||||
step is needed.
|
||||
|
||||
Use a native `<input type="checkbox">` if a boolean toggle is preferred over a
|
||||
select; #16096 established that a Radix `Checkbox` inside this `<form>` and
|
||||
`Dialog` triggers an infinite update loop. This design uses selects for both
|
||||
tri-state controls, which avoids the question entirely and expresses "inherit"
|
||||
as a first-class value.
|
||||
|
||||
### Copy
|
||||
|
||||
New i18n keys in both locales for the section heading, the three controls, their
|
||||
options, and the inherited-value hints. No string may begin with `settings.` —
|
||||
the transcript build-artifact guard rejects that prefix.
|
||||
|
||||
## Files affected
|
||||
|
||||
| File | Change |
|
||||
| :----------------------------------------------------------------------------- | :----------------------------------------------------- |
|
||||
| `packages/channels/github/src/index.ts` | declare `pollInterval` with both bounds |
|
||||
| `packages/channels/base/src/types.ts` | `maximum` on the number descriptor |
|
||||
| `packages/channels/base/src/PollingChannelBase.ts` | clamp the interval to the ceiling |
|
||||
| `packages/sdk-typescript/src/daemon/types.ts` | mirror `maximum` on the wire descriptor |
|
||||
| `packages/cli/src/commands/channel/channel-registry.ts` | validate `maximum` metadata |
|
||||
| `packages/cli/src/serve/channel-settings-store.ts` | enforce `maximum`; tolerate an unchanged stored number |
|
||||
| `docs/developers/qwen-serve-protocol.md` | document `maximum` in the descriptor vocabulary |
|
||||
| `packages/web-shell/client/components/channels/channel-editor-state.ts` | override draft type, read, merge-on-write |
|
||||
| `packages/web-shell/client/components/channels/ChannelEditorDialog.tsx` | per-repository rows, `pollInterval` section mapping |
|
||||
| `packages/web-shell/client/components/channels/ChannelEditorDialog.module.css` | row layout |
|
||||
| `packages/web-shell/client/i18n.tsx` | EN + ZH copy |
|
||||
| `packages/web-shell/client/components/channels/channel-editor-state.test.ts` | state round-trip, inheritance, preservation, sweep |
|
||||
| `packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx` | rendering and interaction |
|
||||
| `packages/web-shell/client/e2e/visuals/screenshots.spec.ts` | capture the new rows |
|
||||
| `docs/users/features/channels/github.md` | document that the editor now exposes these settings |
|
||||
| `docs/users/features/channels/gitlab.md` | document the ceiling for the other polling channel |
|
||||
| `docs/users/features/channels/overview.md` | point the group-settings section at the new rows |
|
||||
| `docs/design/channels/channel-code-hosting.md` | state the clamp in the shared polling contract |
|
||||
| `packages/channels/github/src/GithubAdapter.test.ts` | ceiling, clamp and declared bounds |
|
||||
| `packages/cli/src/commands/channel/channel-registry.test.ts` | `maximum` metadata validation |
|
||||
| `packages/cli/src/commands/channel/channel-descriptor-sdk-mirror.test.ts` | `maximum` in the mirrored key set |
|
||||
| `packages/cli/src/serve/channel-settings-store.test.ts` | ceiling enforcement and the unchanged-value tolerance |
|
||||
|
||||
## Design decisions and rationale
|
||||
|
||||
1. **Editor-only for per-repository settings, descriptor field for the poll
|
||||
interval.** A new descriptor field kind would touch the SDK, the daemon
|
||||
validation mirror, and every client that renders descriptors, for a shape
|
||||
only one platform family uses. The editor already special-cases
|
||||
repository-only channels and already writes `groups`, so the per-repository
|
||||
rows belong there. The poll interval is an ordinary channel-level scalar, so
|
||||
it takes the ordinary descriptor route.
|
||||
2. **Tri-state, not boolean.** `GroupConfig` distinguishes absent from `false`.
|
||||
A two-state control could not express "inherit", and defaulting it to the
|
||||
resolved value would write that value into every repository on the first
|
||||
save — a silent, widening-or-narrowing migration depending on the channel.
|
||||
3. **Rows derived from the allowlist, not stored separately.** One source of
|
||||
truth for membership means a repository cannot have settings without being
|
||||
admitted, and the alias-conflict and `owner/repo` validation from #16096 keep
|
||||
applying to the same string.
|
||||
4. **Lower-case keys.** GitHub repository paths are case-insensitive and #16096
|
||||
made the runtime match them that way. Override keys use the same canonical
|
||||
form so a retyped case does not orphan a repository's settings.
|
||||
5. **No new validation for an empty speaker allowlist.** Under an explicit
|
||||
`allowlist` an empty list means "nobody", which is a legitimate configuration
|
||||
and is already how the channel-wide field behaves. Under an inherited policy
|
||||
an empty text box means "no per-repository list", so the channel-wide one
|
||||
applies. Adding a rejection here would make the two controls disagree.
|
||||
6. **`inherit` is a sentinel, not the empty string.** Radix `Select.Item` rejects
|
||||
an empty `value`, so the controls map `''` to `'inherit'` at the widget
|
||||
boundary and back on change. The draft keeps `''`, which is what makes the
|
||||
comparison against the stored reading straightforward.
|
||||
7. **Switching a row to "Anyone" keeps a speaker list that has names in it.**
|
||||
The list is inert under an open policy, and deleting it would destroy work the
|
||||
operator may want back when they return to an allowlist. An **empty** list is
|
||||
the exception: it is released, because leaving it behind would keep denying
|
||||
everyone while the row reads as inheriting. Only the field the operator
|
||||
actually changed is written, which is the same rule that makes an untouched
|
||||
save a no-op.
|
||||
8. **The hint is derived from the save projection of an empty box, not the
|
||||
draft.** The sentence under the box describes what _leaving it empty_ does,
|
||||
so the row calls the same `projectRepositoryGroup` the save path calls with
|
||||
`{ ...override, allowedUsers: '' }` and asks whether the result owns an
|
||||
`allowedUsers` array. Projecting the current draft instead told a repository
|
||||
storing a list with no policy beside it "nobody", while clearing that box
|
||||
deletes the list and widens the repository back to the channel-wide members.
|
||||
Reading the hint off `override.senders` alone is wrong for the same reason,
|
||||
and no amount of care keeps two independent derivations in sync across later
|
||||
edits.
|
||||
9. **The poll interval gets a real upper bound in the descriptor vocabulary.**
|
||||
`maximum` joins `exclusiveMinimum` on number fields, mirrored in the SDK, the
|
||||
registry's metadata validation, the store, and the editor. Node clamps a
|
||||
timeout above its 32-bit range to 1 ms, so an unbounded interval field lets
|
||||
the form persist a hot loop that burns the API quota. The ceiling is
|
||||
86 400 000 ms, the same one Email already enforces at runtime, and
|
||||
`PollingChannelBase` clamps to it — rather than falling back to 60 s — so a
|
||||
hand-written configuration cannot hot-loop either, and an interval between
|
||||
one day and Node's limit keeps most of its intent instead of being replaced.
|
||||
DWS overrides the getter with a constant and Email does not extend the base,
|
||||
so GitHub and GitLab are the only channels the clamp reaches.
|
||||
The same getter enforces the low end. The vocabulary has only an
|
||||
_exclusive_ minimum, so `exclusiveMinimum: 0` admits `0.5`, which
|
||||
`setTimeout` rounds to ~1 ms — the hot loop again, reached from below, and
|
||||
persistable only because this change made the field managed. Raising
|
||||
`exclusiveMinimum` to 1 is not the fix: it would reject the documented
|
||||
value 1 itself. `MIN_POLL_INTERVAL_MS` and `MAX_POLL_INTERVAL_MS` are
|
||||
exported from the base, the getter clamps into both, and the GitHub
|
||||
descriptor declares its `maximum` from the same constant so the two cannot
|
||||
drift; the field description states that a smaller positive value is
|
||||
raised to 1.
|
||||
10. **A newly declared bound forgives an unchanged stored number, and only its
|
||||
range.** The editor re-sends stored values verbatim, so declaring
|
||||
`pollInterval` would otherwise block every unrelated edit to a channel
|
||||
storing `0` — a value `PollingChannelBase` deliberately tolerates by falling
|
||||
back to 60 s. Both the editor and the store skip the range check when the
|
||||
value is a finite number deep-equal to what is stored. The tolerance is
|
||||
deliberately **not** generalized to all kinds: an existing contract test
|
||||
pins that an unchanged stored scalar of the wrong type is still re-validated
|
||||
and rejected, and that is the right rule — a type error should be fixed, a
|
||||
tolerated range should not strand the operator.
|
||||
11. **Settings that outlive the policy are named, not deleted.** Per-repository
|
||||
senders and `requireMention` are resolved without consulting `groupPolicy`,
|
||||
so under `open` and `pairing` they stay enforced after the rows disappear.
|
||||
Under `disabled` the group gate drops every envelope before either lookup,
|
||||
so the entries are retained but inert — the alert says which of the two
|
||||
cases applies rather than claiming they all still apply. Deleting them would
|
||||
destroy hand-written configuration; instead the editor lists the
|
||||
repositories that still carry settings and says how to edit them. Row edits
|
||||
made before the switch are projected in that branch too, so the save cannot
|
||||
report success while restoring the stored value.
|
||||
That branch runs for every repository-only channel, not only one whose
|
||||
stored policy was `allowlist`: the rows are reachable from any policy, so
|
||||
gating on storage dropped the edits of an operator who switched to
|
||||
Allowlist, narrowed a repository and switched back before saving. It also
|
||||
unions the typed allowlist with the stored keys, because a repository
|
||||
added during that visit exists only in the draft. Conflicting aliases are
|
||||
the exception — one lower-cased draft cannot speak for two spellings that
|
||||
disagree, so both are left exactly as stored and the Allowlist path's
|
||||
conflict error stays the way to resolve them. Two refinements keep that
|
||||
exception from becoming a widening of its own: an ambiguous entry is
|
||||
preserved whatever it holds, because the runtime matches the exact key
|
||||
first — an empty entry inherits `"*"`, and deleting either it or a
|
||||
hand-written non-record value lets the case-insensitive fallback reach
|
||||
the wider alias. The store does **not** tolerate a non-record entry: its
|
||||
unchanged-value tolerance covers the whole `groups` value, while a single
|
||||
non-record entry is refused on every save with
|
||||
`Channel field "groups.<id>" is invalid.`. Passing one through therefore
|
||||
cannot be promised as a no-op, so validation refuses earlier with a
|
||||
message that names the entry and points at `settings.json` — the form has
|
||||
no row for it, and a generic daemon error would leave nothing to act on.
|
||||
The entry is still not deleted, because deleting it is the widening.
|
||||
A row the operator actually edited is refused rather than skipped, with
|
||||
the error on `groupPolicy` because the allowlist field is no longer on
|
||||
screen. An untouched switch still saves.
|
||||
Running the branch for a channel that never had an allowlist must not
|
||||
clean up its stored entries either: an entry that was already empty is
|
||||
kept unless the channel is actually leaving Allowlist or the projection
|
||||
emptied it, so an unrelated edit cannot rewrite the file. Admission is
|
||||
the same either way — an absent key and an empty entry both fall through
|
||||
to `"*"` — which is why that one is about the file, not about access.
|
||||
The alert is derived from the request the save builds, not from storage,
|
||||
and names an entry only when it can still take effect: a resolved
|
||||
`senders` of `allowlist`, or a `requireMention` differing from the
|
||||
inherited one. An `allowedUsers` array beside an inherited `open` policy
|
||||
restricts nobody, and a repository typed during the visit is written by
|
||||
the save but absent from storage; enumerating stored groups mis-announced
|
||||
both.
|
||||
|
||||
## Constraints
|
||||
|
||||
- The daemon's stored-value guard rejects an invalid nested type, so the editor
|
||||
must write a real boolean for `requireMention` and a real string array for
|
||||
`allowedUsers` — not strings.
|
||||
- Existing tests pin that untouched per-repository keys survive a save
|
||||
(`dispatchMode`, `groupHistoryLimit`, `requireMention`). The merge must be
|
||||
additive over the stored object, never a replacement.
|
||||
- The repository allowlist may be empty or contain an invalid entry while the
|
||||
operator is typing. Rows render only for entries that parse as `owner/repo`;
|
||||
the existing validation error still blocks the save.
|
||||
- `pollInterval` must stay optional. An empty input deletes the key so the
|
||||
runtime keeps its own 60-second default rather than persisting a number the
|
||||
operator never chose.
|
||||
|
||||
## Risks
|
||||
|
||||
- **Widening access by accident.** Choosing "Anyone" for a repository that
|
||||
inherited an allowlist widens it. The option label names the inherited value
|
||||
so the change is visible before saving, and the row shows the effective
|
||||
policy, not just the draft.
|
||||
- **Freezing defaults.** Materializing inherited values would be a silent
|
||||
migration. The tri-state model and a test that asserts an untouched repository
|
||||
still saves as `{}` guard against it.
|
||||
- **Losing hand-written settings.** The merge preserves unknown keys; the
|
||||
existing preservation tests are the guard, and a new case covers a repository
|
||||
that has both an unknown key and an edited known key.
|
||||
- **Scope creep into other platforms.** The rows are gated on the same
|
||||
`isRepositoryOnlyChannelType` helper #16096 introduced, so chat platforms are
|
||||
untouched by construction.
|
||||
|
||||
## Validation plan
|
||||
|
||||
- Unit: state round-trip (read → save is a no-op for an untouched channel),
|
||||
explicit override writes, inherit deletes the key, unknown keys preserved,
|
||||
removed repository drops its entry, case-retyped repository keeps its
|
||||
settings, empty allowlist, invalid repository entry.
|
||||
- Invariant sweep: twelve stored shapes crossed with every single-field edit,
|
||||
driven through the real save path rather than the projection alone — an
|
||||
untouched row reproduces its stored object byte-for-byte, the empty-box hint
|
||||
agrees with what that save admits, and no edit widens the admitted set unless
|
||||
the row's own controls asked for it. A second sweep crosses the same shapes
|
||||
with the stored and final group policies, because the runtime never consults
|
||||
`groupPolicy` when resolving speakers: a row must write the same entry, and
|
||||
admit the same people, whichever policy the save lands on. It covers a
|
||||
repository that exists only in the draft as well as a stored one.
|
||||
- Non-Allowlist saves: a row narrowed during a temporary switch survives on a
|
||||
channel stored Open, a repository added during that visit is written rather
|
||||
than dropped, conflicting aliases are left as stored while the Allowlist
|
||||
path still refuses them, an untouched channel round-trips byte-for-byte,
|
||||
and a chat platform gains no entries from the typed allowlist. Each asserts
|
||||
the admitted set, not only the saved shape.
|
||||
- Conflicting aliases: the same shapes as the upper spelling against a fixed
|
||||
open lower spelling, crossed with the stored and final policies. Untouched
|
||||
must be a byte-for-byte no-op that admits the same people through the
|
||||
runtime's exact-then-case-insensitive lookup; an edit that really changes
|
||||
the collapsed draft must be refused, and one that does not must still save.
|
||||
Two dialog cases click through the same flows and assert whether `onSave`
|
||||
is reached.
|
||||
- Component: rows appear per allowlisted repository, typing a repository adds a
|
||||
row, the speaker input appears when the row's **effective** policy is an
|
||||
allowlist — its own or the inherited one — and is hidden when it is `open`,
|
||||
the inherited hint reflects `"*"`, each row shows and edits only its own
|
||||
speakers, the hint's wording follows what an empty box will save rather than
|
||||
the draft policy, repositories whose settings outlive the policy are named,
|
||||
and the poll interval renders in Connection in both locales.
|
||||
- Mutation: each behavioural claim above is pinned by introducing the
|
||||
corresponding mutation and confirming exactly the describing cases fail.
|
||||
- Build guards: the three vite builds plus `client/build-artifact.test.ts`,
|
||||
because new copy enters the transcript bundle.
|
||||
- Visual: the repository's own visuals harness, both themes, with an assertion
|
||||
that the per-repository rows exist before capture.
|
||||
- Manual: against a real daemon, save a repository override and confirm the
|
||||
stored `settings.json` contains the boolean and array shapes the store
|
||||
validates.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
1. A GitHub Channel's poll interval can be set, cleared, and validated from the
|
||||
editor between 1 and 86400000 ms; an empty input leaves the key absent, and a
|
||||
stored value outside the bounds does not block an unrelated edit.
|
||||
2. Each allowlisted repository shows a require-mention control and a speaker
|
||||
control whose default is "inherit".
|
||||
3. Saving an untouched channel writes exactly the same `groups` object it read.
|
||||
4. An explicit override writes a boolean `requireMention` and, for an allowlist,
|
||||
a string array `allowedUsers`. Returning the **policy** to "inherit" removes
|
||||
the policy key; a speaker list the operator typed survives until the box is
|
||||
cleared too, and an empty list left behind by a policy the operator abandoned
|
||||
is released so inheritance really returns the channel-wide members.
|
||||
5. Keys the editor does not model survive a save unchanged.
|
||||
6. No chat platform's form changes. Email's poll interval moves to Connection
|
||||
alongside GitHub's, because the section map is keyed by field name.
|
||||
7. Both locales render the new copy, and the build-artifact guard passes.
|
||||
8. The speaker hint describes what an empty box will save, so a stored allowlist
|
||||
with no list reads as inheriting and a stored empty list reads as nobody.
|
||||
9. Repositories whose settings outlive a switch away from the allowlist policy
|
||||
are named in the editor rather than becoming invisible while still enforced.
|
||||
10. A repository narrowed during a temporary switch to Allowlist keeps that
|
||||
narrowing when the save lands on another policy, including on a channel
|
||||
that was never Allowlist and a repository added during the visit;
|
||||
conflicting aliases are left as stored instead of being rewritten from
|
||||
one draft. An empty ambiguous entry survives an untouched save, and an
|
||||
edited ambiguous row is refused on a control that is still visible
|
||||
rather than discarded.
|
||||
|
||||
## Open questions
|
||||
|
||||
- Should the channel-wide `"*"` require-mention default also become editable?
|
||||
It is the value every repository inherits, and today it is only reachable
|
||||
through `settings.json`. Left out of this change to keep the diff about
|
||||
per-repository control; worth a follow-up decision.
|
||||
- Should the poll interval be expressed in seconds in the UI? Milliseconds
|
||||
match the stored key and the Email plugin's existing label, so this design
|
||||
keeps milliseconds for consistency.
|
||||
@@ -0,0 +1,343 @@
|
||||
# 在 Web Shell 编辑器里配置 GitHub 渠道的仓库级设置
|
||||
|
||||
[English](github-channel-repository-settings.md) | [简体中文](github-channel-repository-settings.zh-CN.md)
|
||||
|
||||
状态:已在本次变更中实现。承接已合入的编辑器可用性工作(#16096),补上该 PR 在 Risk & Scope 里列出的
|
||||
「按仓库设置仍只能改 `settings.json`」这一缺口。
|
||||
|
||||
## 问题陈述
|
||||
|
||||
有三项设置决定 GitHub 渠道的行为,而它们都无法从 Web Shell 修改:
|
||||
|
||||
| 设置 | 作用域 | 控制什么 |
|
||||
| :------------------------------------------------ | :----- | :------------------------------------- |
|
||||
| `groups["<owner/repo>"].senders` + `allowedUsers` | 按仓库 | 哪些 GitHub 账号可以在该仓库里发起任务 |
|
||||
| `groups["<owner/repo>"].requireMention` | 按仓库 | 未被 @ 的评论是否会被处理 |
|
||||
| `pollInterval` | 渠道级 | 轮询通知 API 的间隔,单位毫秒 |
|
||||
|
||||
今天的编辑器只暴露渠道级发言人默认值(`groups["*"].senders` / `allowedUsers`)与仓库允许列表本身。
|
||||
操作者若想把某个仓库限制给两位维护者,或让某个仓库无需 @ 就响应,就必须到 daemon 宿主机上手改
|
||||
`settings.json`——表单里根本看不到这个值。
|
||||
|
||||
实际后果是放开的默认值一直留在原地。渠道级发言人默认是 `open`,所以收窄它等于一次性收窄所有仓库;
|
||||
只收窄一个仓库的唯一途径,就是表单不展示的那个文件。
|
||||
|
||||
## 现状
|
||||
|
||||
- `packages/channels/github/src/index.ts` 声明了 `token`、`useLocalGh`、`baseUrl`、`groupPolicy`、
|
||||
`reasonFilter`,没有声明 `pollInterval`,所以描述符驱动的表单永远不会渲染它。Email 插件已经把同一个键
|
||||
声明为 `number` 字段,带 `exclusiveMinimum: 0` 与 `default: '60000'`,这就是要照搬的先例。
|
||||
- `GithubChannel extends PollingChannelBase`,其 `pollInterval` getter 读取 `config.pollInterval`,
|
||||
在缺失、非有限数值或非正数时回落到 `60_000`。
|
||||
- `GroupGate.check()` 解析 `requireMention` 的顺序是
|
||||
`groupConfigFor(chatId)?.requireMention ?? groups["*"]?.requireMention ?? true`,
|
||||
并且在到达 mention 判断之前就对 `disabled` 策略返回 `{allowed: false, reason: 'disabled'}`。
|
||||
它**完全不含发言人逻辑**。
|
||||
- 发言人策略与成员由 `ChannelBase.groupSendersFor` 与 `groupAllowedUsersFor` 解析:
|
||||
按仓库的 `senders` 回落到 `"*"` 再回落到 `open`;按仓库的 `allowedUsers` 回落到 `"*"` 再回落到 `[]`。
|
||||
`senderGateFor` 用这两者构造闸门。两个类的查表都走 #16096 加入的大小写不敏感群组匹配。
|
||||
- `channel-editor-state.ts` 由允许列表字符串构建 `groups`,逐字保留每个已列仓库的既存设置,
|
||||
并把渠道级发言人默认值写进 `groups["*"]`。按仓库的条目原样带过,没有对应的草稿状态。
|
||||
- daemon 已经接受本设计需要的全部编辑。`channel-settings-store.ts` 把
|
||||
`groups.<id>.requireMention` 校验为布尔、`senders` 校验为 `open` 或 `allowlist`、
|
||||
`allowedUsers` 校验为字符串数组,并按 `exclusiveMinimum` 校验描述符声明的 number 字段。
|
||||
**按仓库的 `groups` 编辑不需要改 daemon、SDK 或描述符 schema。** 但轮询间隔的上界确实要给
|
||||
number 描述符扩展 `maximum`,并在五处镜像——见决策 9 与受影响文件表。
|
||||
|
||||
## 目标
|
||||
|
||||
1. 在渠道编辑器里修改单个已列仓库的发言人策略与允许列表。
|
||||
2. 在同一处修改单个已列仓库的 `requireMention`。
|
||||
3. 在同一张表单里修改渠道的轮询间隔。
|
||||
4. 保持继承语义:操作者没有自定义过的仓库必须仍是一个空覆盖,从 `"*"` 再回落到内置默认。
|
||||
|
||||
## 非目标
|
||||
|
||||
- 不改任何默认值。群组策略默认值的分歧单独记录在 #16144,有意不在本次范围内。
|
||||
- 不编辑 `groups["*"].requireMention`、`dispatchMode`、`groupHistoryLimit`。这些键继续原样往返。
|
||||
- 不新增描述符字段*类型*,也不为非仓库型平台提供按仓库 UI。聊天平台保持现有表单。
|
||||
number 描述符确实新增 `maximum` 界限(决策 9),那是镜像进 SDK 类型的词汇扩展,不是新类型。
|
||||
- 不做基于 GitHub API 的仓库选择器。仓库仍在允许列表字段里按 `owner/repo` 手填。
|
||||
|
||||
## 方案
|
||||
|
||||
### 渠道层:声明轮询间隔
|
||||
|
||||
在 GitHub 插件的管理描述符里加一个字段,照搬 Email:
|
||||
|
||||
```json
|
||||
{
|
||||
"key": "pollInterval",
|
||||
"label": "Poll Interval (ms)",
|
||||
"kind": "number",
|
||||
"exclusiveMinimum": 0,
|
||||
"maximum": 86400000,
|
||||
"default": "60000"
|
||||
}
|
||||
```
|
||||
|
||||
随后既有的描述符驱动路径就会渲染、校验并持久化它,无需编辑器代码。编辑器的分区映射把它放进
|
||||
**连接**(与 `baseUrl` 同区),因为它描述的是渠道如何连到 GitHub,而不是谁可以使用它。
|
||||
|
||||
分区映射按字段名而不是按平台生效,所以 **Email 既有的 `pollInterval` 字段也会从「应用凭据」移到「连接」**。
|
||||
那是第二个轮询平台上含义相同的同一个设置,而它本来就该在那里;把映射限定到 GitHub 只会为了
|
||||
让一个放错位置的字段继续放错而多加一个条件判断。没有其它平台声明这个键。
|
||||
|
||||
新渠道的间隔保持**为空**:编辑器不会用描述符默认值预填 number 字段,所以在操作者输入之前不会持久化任何值,
|
||||
60 秒默认仍归运行时所有。声明的 `default` 用于在线上契约里记录生效值,与 Email 一致。
|
||||
|
||||
### 编辑器状态:每个仓库一份三态覆盖
|
||||
|
||||
新增一个草稿字段:
|
||||
|
||||
```ts
|
||||
export interface ChannelGroupOverrideDraft {
|
||||
/** '' 表示继承;否则是显式布尔的字符串形式。 */
|
||||
requireMention: '' | 'true' | 'false';
|
||||
/** '' 表示继承;否则是显式策略。 */
|
||||
senders: '' | 'open' | 'allowlist';
|
||||
allowedUsers: string;
|
||||
}
|
||||
|
||||
// 挂在 ChannelEditorDraft 上
|
||||
groupOverrides: Record<string, ChannelGroupOverrideDraft>;
|
||||
```
|
||||
|
||||
键是规范小写的仓库 ID,与 `assignGroups` 已经为仓库型渠道写入的键一致。
|
||||
|
||||
- **读取。** `createChannelEditorDraft` 为每个已存仓库键(排除 `"*"`)生成一条,把已存的
|
||||
`requireMention` 布尔翻译成 `'true'` / `'false'`,键缺失翻译成 `''`。已存的 `senders` 原样取用,
|
||||
缺失取 `''`。`allowedUsers` 用 `, ` 连接,与渠道级字段完全一致。该读取函数是模块私有的,并与保存路径共用。
|
||||
- **写入。** `assignGroups` 把草稿覆盖合并到已存群组对象之上,**逐字段与「对已存对象的同一份读取」比对**。
|
||||
草稿值仍等于已存读取的字段,原样保留它的已存键;只有发生变化的字段才写入,而变回 `''` 的字段删除其键。
|
||||
编辑器不建模的键(`dispatchMode`、`groupHistoryLimit` 以及任何未知键)原样复制。
|
||||
从允许列表里删掉的仓库会失去条目,与今天一致。
|
||||
- **为什么比对而不是覆盖。** 有两种已存形态无法用「三态控件 + 文本框」表达:空允许列表
|
||||
(`allowedUsers: []`,意为*没有人*),以及只有列表没有策略(有 `allowedUsers` 而无 `senders`,
|
||||
意为*继承策略、覆盖成员*)。简单的「写入解析出的列表,空则删除」规则会在操作者本想无操作的保存里
|
||||
把这两种形态都改成别的东西。与初始读取比对,让未触碰的字段在构造上就不可能被改动,
|
||||
于是无需脏标记也能保证往返精确。
|
||||
- **显式允许列表拥有自己的列表。** 当该行自己的策略是 `allowlist` 时,空文本框写入 `[]`——没有人——
|
||||
而不是删除该键、悄悄继承渠道级列表。当该行继承时,清空文本删除按仓库的列表,让渠道级列表重新生效。
|
||||
两种情况由合并后群组的 `senders` 区分。
|
||||
- **选择策略本身也拥有列表,即使文本从未改动。** 这是新加一行的默认状态,所以它是最可能走到的路径而不是边角:
|
||||
操作者选「仅允许列表」、输入框留空,此时 `allowedUsers` 的两份读取都是 `''`。
|
||||
只在*文本*变化时才触发的守卫会跳过写入,落盘成没有列表的 `{senders: 'allowlist'}`,
|
||||
运行时的 `??` 会把渠道级列表原样交回来——操作者手上最严格的那个操作变成一次空操作,而界面报告成功。
|
||||
所以当操作者**把策略改成** `allowlist` 且已存群组没有列表时,也要写入空列表。
|
||||
- **这条规则有意只作用于「策略被改过」的情形。** 已存的 `{senders: 'allowlist'}` 而没有 `allowedUsers`
|
||||
是一种合法的手写形态,意为「允许列表策略、成员从 `'*'` 继承」。在那里写 `[]` 会收窄一个没人编辑过的渠道——
|
||||
正是「与初始读取比对」这条规则要防的那类静默迁移。两个方向各有自己的用例与自己的变异钉住。
|
||||
- **继承是默认。** 没有自定义的仓库写出 `{}` 并继续继承。编辑器绝不能把 `requireMention: true`
|
||||
或 `senders: 'open'` 物化进未触碰的仓库:那会把今天的默认冻进配置,并让之后对 `"*"` 的修改
|
||||
悄悄无法到达该仓库。
|
||||
|
||||
### 编辑器 UI:每个已列仓库一行
|
||||
|
||||
在既有的「访问控制」分区内、仓库允许列表字段正下方,且仅当群组策略为 `allowlist` 时,
|
||||
按允许列表字符串解析出的每个仓库渲染一个块(用与保存路径相同的 `toList` + 小写化 + 去重)。
|
||||
每块以仓库 ID 为标题,含三个控件:
|
||||
|
||||
| 控件 | 组件 | 取值 |
|
||||
| :----------- | :------- | :------------------------------------------------------------------------------------ |
|
||||
| 需要 @ | 下拉 | 继承 / 需要 @ / 无需 @ 也响应 |
|
||||
| 谁可以发言 | 下拉 | 继承 / 任何人 / 仅允许列表 |
|
||||
| 允许的发言人 | 文本输入 | 逗号分隔的 GitHub 登录名,当该行的*生效*策略是 `allowlist` 时显示——自己的,或继承来的 |
|
||||
|
||||
继承值写在选项标签里,让操作者看见自己继承的是什么——例如当 `"*"` 或内置默认解析为 `true` 时显示
|
||||
`继承(需要 @)`。展示的解析顺序与运行时一致:按仓库 → `"*"` → 内置。发言人输入按生效策略而不是
|
||||
该行自己的策略把门,因为继承允许列表的仓库仍然有一份值得编辑的成员列表;它的辅助文案说明留空即使用渠道级列表。
|
||||
|
||||
行内的选项标签与占位符是仓库专属的(`任何人`、`仅允许列表`、`alice, bob`)。复用共享的群组成员字符串
|
||||
会把 `Any group member`、`Listed members only`、`user-a, user-b` 带回 GitHub 表单——
|
||||
正是 #16096 消除的那种群聊/仓库错配。
|
||||
|
||||
每一行都是一个 `role="group"`,由它自己的仓库标题标注。否则所有行的控件都会共用
|
||||
「需要 @」「谁可以发言」这两个可访问名称,一旦列出多个仓库,读屏用户就分不清自己在编辑哪个仓库。
|
||||
|
||||
行在每次渲染时由允许列表推导,所以输入一个仓库会立刻出现它的行,删掉则行消失。被删仓库的覆盖状态
|
||||
在保存时由 `assignGroups` 丢弃,不需要单独的清理步骤。
|
||||
|
||||
如果更想用布尔开关而不是下拉,必须用原生 `<input type="checkbox">`;#16096 已经确认 Radix 的
|
||||
`Checkbox` 放进这个 `<form>` 与 `Dialog` 会触发无限更新循环。本设计两个三态控件都用下拉,
|
||||
既绕开这个问题,也把「继承」表达成一等取值。
|
||||
|
||||
### 文案
|
||||
|
||||
两种语言都新增 i18n 键:分区标题、三个控件、它们的选项、以及继承值提示。任何字符串都不得以
|
||||
`settings.` 开头——transcript 构建产物守卫会拒绝该前缀。
|
||||
|
||||
## 受影响文件
|
||||
|
||||
| 文件 | 改动 |
|
||||
| :----------------------------------------------------------------------------- | :----------------------------------- |
|
||||
| `packages/channels/github/src/index.ts` | 声明带上下界的 `pollInterval` |
|
||||
| `packages/channels/base/src/types.ts` | number 描述符上的 `maximum` |
|
||||
| `packages/channels/base/src/PollingChannelBase.ts` | 把间隔钳制到上界 |
|
||||
| `packages/sdk-typescript/src/daemon/types.ts` | 在 wire 描述符上镜像 `maximum` |
|
||||
| `packages/cli/src/commands/channel/channel-registry.ts` | 校验 `maximum` 元数据 |
|
||||
| `packages/cli/src/serve/channel-settings-store.ts` | 强制 `maximum`;容忍未变更的已存数字 |
|
||||
| `docs/developers/qwen-serve-protocol.md` | 在描述符词汇表里记录 `maximum` |
|
||||
| `packages/web-shell/client/components/channels/channel-editor-state.ts` | 覆盖草稿类型、读取、写入时合并 |
|
||||
| `packages/web-shell/client/components/channels/ChannelEditorDialog.tsx` | 按仓库行、`pollInterval` 分区映射 |
|
||||
| `packages/web-shell/client/components/channels/ChannelEditorDialog.module.css` | 行布局 |
|
||||
| `packages/web-shell/client/i18n.tsx` | 中英文案 |
|
||||
| `packages/web-shell/client/components/channels/channel-editor-state.test.ts` | 状态往返、继承、保留性、不变式扫描 |
|
||||
| `packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx` | 渲染与交互 |
|
||||
| `packages/web-shell/client/e2e/visuals/screenshots.spec.ts` | 截取新增的行 |
|
||||
| `docs/users/features/channels/github.md` | 说明编辑器现已暴露这些设置 |
|
||||
| `docs/users/features/channels/gitlab.md` | 为另一个轮询渠道写明上限 |
|
||||
| `docs/users/features/channels/overview.md` | 让群组设置一节指向新增的行 |
|
||||
| `docs/design/channels/channel-code-hosting.md` | 在共享轮询契约里写明钳制 |
|
||||
| `packages/channels/github/src/GithubAdapter.test.ts` | 上限、钳制与声明的边界 |
|
||||
| `packages/cli/src/commands/channel/channel-registry.test.ts` | `maximum` 元数据校验 |
|
||||
| `packages/cli/src/commands/channel/channel-descriptor-sdk-mirror.test.ts` | 镜像键集合里的 `maximum` |
|
||||
| `packages/cli/src/serve/channel-settings-store.test.ts` | 上限强制与未变更值容忍 |
|
||||
|
||||
## 设计决策与理由
|
||||
|
||||
1. **按仓库设置只改编辑器,轮询间隔走描述符字段。** 新增描述符字段类型会波及 SDK、daemon 的校验镜像,
|
||||
以及所有渲染描述符的客户端,而它服务的形态只有一个平台家族在用。编辑器本来就对仓库型渠道做特判、
|
||||
本来就写 `groups`,所以按仓库的行属于这里。轮询间隔是普通的渠道级标量,因此走普通的描述符路径。
|
||||
2. **三态,而不是布尔。** `GroupConfig` 区分「缺失」与 `false`。两态控件无法表达「继承」,
|
||||
而把它默认成解析后的值,会在第一次保存时把该值写进每个仓库——这是一次静默迁移,
|
||||
方向取决于渠道,可能放开也可能收窄。
|
||||
3. **行由允许列表推导,不单独存储。** 成员关系只有一个真相来源,意味着一个仓库不可能「有设置却未被放行」,
|
||||
而 #16096 的别名冲突与 `owner/repo` 校验继续作用于同一个字符串。
|
||||
4. **小写键。** GitHub 仓库路径不区分大小写,#16096 已让运行时按此匹配。覆盖键使用同一规范形式,
|
||||
所以重打大小写不会让某个仓库的设置变成孤儿。
|
||||
5. **不为空的发言人允许列表新增校验。** 在显式 `allowlist` 下,空列表意味着「没有人」,
|
||||
这是合法配置,也已经是渠道级字段的行为。在继承策略下,空文本框意味着「没有按仓库的列表」,
|
||||
于是渠道级列表生效。在这里加拒绝会让两个控件互相矛盾。
|
||||
6. **`inherit` 是哨兵值,不是空字符串。** Radix 的 `Select.Item` 拒绝空 `value`,
|
||||
所以控件在组件边界把 `''` 映射成 `'inherit'`,变更时再映射回来。草稿保留 `''`,
|
||||
这正是与已存读取做比对得以直截了当的原因。
|
||||
7. **把一行切到「任何人」会保留其中有名字的发言人列表。** 该列表在开放策略下不起作用,
|
||||
而删掉它会毁掉操作者改回允许列表时可能还想用的内容。**空**列表是例外:它会被释放,
|
||||
因为留着它会在该行读作「继承」的同时继续拒绝所有人。只写入操作者真正改动的那个字段,
|
||||
这与「未触碰的保存是 no-op」是同一条规则。
|
||||
8. **提示文案由「空输入框」的保存投影推导,而不是由草稿推导。** 输入框下面那句话说的是
|
||||
_留空_ 会怎样,所以该行用 `{ ...override, allowedUsers: '' }` 调用与保存路径同一个
|
||||
`projectRepositoryGroup`,并询问结果是否拥有 `allowedUsers` 数组。改成投影当前草稿,
|
||||
会对「存了列表但旁边没有策略」的仓库说「没有人」,而清空那个输入框其实会删除列表、
|
||||
把仓库放宽回渠道级成员。只读 `override.senders` 出于同样的原因也是错的,
|
||||
而再小心也无法让两条独立推导在后续改动中保持同步。
|
||||
9. **轮询间隔在描述符词汇表里获得真正的上界。** `maximum` 与 `exclusiveMinimum` 并列加入 number 字段,
|
||||
并在 SDK、注册表的元数据校验、store 与编辑器里镜像。Node 会把超过其 32 位范围的 timeout 钳制为 1 毫秒,
|
||||
所以一个无上界的间隔字段能让表单落盘一个烧尽 API 配额的热循环。上界取 86 400 000 毫秒,
|
||||
与 Email 已在运行时强制的上限相同;`PollingChannelBase` 会**钳制**到该上限(而不是回落 60 秒),
|
||||
这样手写配置同样无法热循环,且介于一天与 Node 上限之间的间隔保留了大部分原意而不是被替换。
|
||||
DWS 用常量覆盖了该 getter,Email 不继承该基类,所以钳制实际只作用于 GitHub 与 GitLab。
|
||||
同一个 getter 也强制低端。词汇表里只有**排他**下界,所以 `exclusiveMinimum: 0` 会接受 `0.5`,
|
||||
而 `setTimeout` 把它舍入到约 1 毫秒——同样是从下方到达的热循环,且正因为本次改动把该字段
|
||||
变成受管字段才可持久化。把 `exclusiveMinimum` 提到 1 不是修法:那会连文档承诺的 1 本身一起拒绝。
|
||||
`MIN_POLL_INTERVAL_MS` 与 `MAX_POLL_INTERVAL_MS` 由 base 导出,getter 向两端钳制,
|
||||
GitHub 描述符用同一个常量声明 `maximum`,两者因此不会漂移;字段描述写明更小的正值会被抬到 1。
|
||||
10. **新声明的界限宽恕未变更的已存数字,且只宽恕它的区间。** 编辑器会原样重发已存值,
|
||||
所以声明 `pollInterval` 否则会挡住一个存着 `0` 的渠道的所有无关编辑——而 `PollingChannelBase`
|
||||
正是有意容忍这个值并回落到 60 秒。编辑器与 store 都在「值是与已存深相等的有限数字」时跳过区间检查。
|
||||
这条容忍**有意不**推广到所有类型:既有契约测试钉住「未变更但类型错误的已存标量仍要重新校验并拒绝」,
|
||||
而那是对的规则——类型错误应当被修正,被容忍的区间不该把操作者困住。
|
||||
11. **比策略活得更久的设置被点名,而不是被删除。** 按仓库的 senders 与 `requireMention`
|
||||
在解析时不读取 `groupPolicy`,所以在 `open` 与 `pairing` 下,行消失后它们仍被执行。
|
||||
在 `disabled` 下,群组闸门在两个查表之前就丢弃每个信封,所以这些条目被保留但不生效——
|
||||
告警会说明当前属于哪一种,而不是一律声称仍然生效。删掉它们会毁掉手写配置;
|
||||
改为由编辑器列出仍带设置的仓库,并说明如何编辑它们。切换之前做的行内修改在该分支同样会被投影,
|
||||
所以保存不可能一边报告成功一边把已存值还原回去。
|
||||
该分支对每个仓库型渠道都运行,而不只是存量为 `allowlist` 的那种:行从任何策略都可达,
|
||||
所以按存量把门会丢掉「切到允许列表、收窄某仓库、再切回来才保存」这类编辑。它还会把
|
||||
输入的允许列表与已存键取并集,因为那次访问中新增的仓库只存在于草稿里。冲突别名是例外——
|
||||
一份小写草稿无法代表两个互相矛盾的拼写,所以两者都原样保留,
|
||||
解决它们仍然走允许列表路径的冲突报错。两处细化让这条例外不至于自己变成放宽:
|
||||
冲突条目无论存的是什么都保留,因为运行时先精确匹配该键——空条目继承 `"*"`,
|
||||
而删掉它或删掉一个手写的非对象值,都会让大小写不敏感回退命中更宽的那个别名。
|
||||
store **并不**容忍非对象条目:它的未变更值容忍覆盖的是整个 `groups` 值,
|
||||
而单个非对象条目在每次保存时都会被拒,报 `Channel field "groups.<id>" is invalid.`。
|
||||
因此原样传过去不能被承诺为 no-op,校验改为更早拒绝,并在消息里点名该条目、
|
||||
指向 `settings.json`——表单里没有它的行,而 daemon 的通用错误会让操作者无从下手。
|
||||
该条目仍然不被删除,因为删除它正是那次放宽。
|
||||
而操作者确实编辑过的行会被拒绝而不是被跳过,错误挂在
|
||||
`groupPolicy` 上,因为此时允许列表字段已不在屏幕上。未触碰的切换仍然可以保存。
|
||||
对从未启用允许列表的渠道运行该分支,也不等于有权清理它的已存条目:
|
||||
本来就空的条目会被保留,除非渠道确实在离开 Allowlist、或投影把它清空了,
|
||||
这样一次无关编辑就无法改写文件。两种情况下准入都相同——缺失键与空条目都回落到 `"*"`——
|
||||
所以那一条关乎文件,而不关乎访问。
|
||||
告警从保存实际构建的请求派生,而不是从存量派生,且只点名仍能生效的条目:
|
||||
解析后的 `senders` 为 `allowlist`,或 `requireMention` 与继承值不同。
|
||||
继承到 `open` 策略旁边的 `allowedUsers` 数组限制不了任何人,
|
||||
而访问期间输入的仓库会被保存写出、却不在存量里;枚举已存 groups 把这两者都说错了。
|
||||
|
||||
## 约束
|
||||
|
||||
- daemon 的既存值守卫会拒绝非法的嵌套类型,所以编辑器必须为 `requireMention` 写真布尔、
|
||||
为 `allowedUsers` 写真字符串数组,而不是字符串。
|
||||
- 既有测试钉住「未触碰的按仓库键在一次保存后存活」(`dispatchMode`、`groupHistoryLimit`、
|
||||
`requireMention`)。合并必须是在已存对象之上做加法,绝不能整体替换。
|
||||
- 操作者输入过程中,仓库允许列表可能为空或含非法条目。行只为能解析成 `owner/repo` 的条目渲染;
|
||||
既有的校验错误仍然阻止保存。
|
||||
- `pollInterval` 必须保持可选。空输入删除该键,让运行时保留它自己的 60 秒默认,
|
||||
而不是持久化一个操作者从未选过的数字。
|
||||
|
||||
## 风险
|
||||
|
||||
- **意外放开访问。** 给一个原本继承允许列表的仓库选「任何人」会放开它。选项标签写明继承值,
|
||||
让改动在保存前可见;行里展示的是生效策略,而不只是草稿。
|
||||
- **冻结默认值。** 物化继承值会是一次静默迁移。三态模型与一条「未触碰的仓库仍保存为 `{}`」的测试
|
||||
共同防住它。
|
||||
- **丢掉手写设置。** 合并保留未知键;既有保留性测试就是守卫,并新增一条用例覆盖
|
||||
「同一仓库既有未知键又有被编辑的已知键」。
|
||||
- **范围蔓延到其它平台。** 这些行由 #16096 引入的同一个 `isRepositoryOnlyChannelType` 助手把门,
|
||||
所以聊天平台在构造上就不受影响。
|
||||
|
||||
## 验证计划
|
||||
|
||||
- 单测:状态往返(未触碰的渠道读取→保存是 no-op)、显式覆盖写入、继承删除键、未知键保留、
|
||||
被删仓库丢弃条目、重打大小写的仓库保留设置、空允许列表、非法仓库条目。
|
||||
- 不变式扫描:十二种存量形态叉乘每一种单字段编辑,走真实保存路径而不是只走投影——
|
||||
未触碰的行逐字节还原它存的对象、空框提示与该次保存实际准入的人一致、
|
||||
且除非行内控件自己要求,任何编辑都不放宽准入集。第二组扫描把同样的形态与
|
||||
存量策略、最终策略叉乘,因为运行时解析发言人时从不读取 `groupPolicy`:
|
||||
无论保存落到哪个策略,同一行必须写出同样的条目、准入同样的人。
|
||||
它同时覆盖只存在于草稿里的仓库与已存仓库。
|
||||
- 非允许列表保存:临时切换期间收窄的行在存量为 Open 的渠道上存活、那次访问中新增的仓库被写出
|
||||
而不是被丢弃、冲突别名原样保留而允许列表路径仍然拒绝它们、未触碰的渠道逐字节往返、
|
||||
以及聊天平台不会从输入的允许列表凭空得到条目。每条都断言准入集,而不只是落盘形状。
|
||||
- 冲突别名:同样的形态作为大写拼写、对一个固定的开放小写拼写,再叉乘存量与最终策略。
|
||||
未触碰必须是逐字节的 no-op,且按运行时「先精确、再大小写不敏感」的查表准入同样的人;
|
||||
确实改变了折叠后草稿的编辑必须被拒绝,而没有改变它的编辑必须仍能保存。
|
||||
另有两条对话框用例真实点击同样的流程,断言是否走到 `onSave`。
|
||||
- 组件:每个已列仓库出现行、输入仓库即新增行、发言人输入在该行**生效**策略为允许列表时出现
|
||||
(自身的或继承来的)、策略为 `open` 时隐藏、继承提示反映 `"*"`、每行只显示并编辑自己的发言人、
|
||||
提示文案跟随「空输入框保存后将拥有什么」而不是草稿策略、设置比策略活得更久的仓库被点名、
|
||||
轮询间隔在两种语言下都渲染在「连接」区。
|
||||
- 变异:上面每条行为主张都用「引入对应变异并确认恰好描述它的用例变红」钉住。
|
||||
- 构建守卫:三个 vite 构建加 `client/build-artifact.test.ts`,因为新文案会进入 transcript 产物。
|
||||
- 视觉:仓库自带的 visuals 装置,两套主题,截图前断言按仓库的行存在。
|
||||
- 手工:对真实 daemon 保存一次仓库覆盖,确认落盘的 `settings.json` 里是 store 会校验的布尔与数组形态。
|
||||
|
||||
## 验收标准
|
||||
|
||||
1. GitHub 渠道的轮询间隔可以在编辑器里于 1 到 86400000 毫秒之间设置、清空与校验;
|
||||
空输入让该键保持缺失,且超出区间的已存值不会挡住一次无关编辑。
|
||||
2. 每个已列仓库显示「需要 @」控件与发言人控件,默认都是「继承」。
|
||||
3. 保存一个未触碰的渠道,写出的 `groups` 对象与读到的完全一致。
|
||||
4. 显式覆盖写出布尔 `requireMention`,允许列表写出字符串数组 `allowedUsers`。
|
||||
把**策略**改回「继承」会删除策略键;操作者填过的发言人列表会保留,直到输入框也被清空;
|
||||
而被操作者放弃的策略所留下的空列表会被释放,让继承真正交回渠道级成员。
|
||||
5. 编辑器不建模的键在一次保存后原样存活。
|
||||
6. 任何聊天平台的表单都不变。Email 的轮询间隔与 GitHub 的一起移到「连接」,因为分区映射按字段名生效。
|
||||
7. 两种语言都渲染新文案,且构建产物守卫通过。
|
||||
8. 发言人提示描述的是「空输入框保存后将拥有什么」,所以已存的「有策略无列表」读作继承,
|
||||
已存的空列表读作没有人。
|
||||
9. 策略切离允许列表后仍然生效的按仓库设置会在编辑器里被点名,
|
||||
而不是一边不可见一边继续被执行。
|
||||
10. 临时切到允许列表期间被收窄的仓库,在保存落到其它策略时保留该收窄——包括从未是允许列表的
|
||||
渠道,以及那次访问中新增的仓库;冲突别名原样保留,而不是被一份草稿改写。
|
||||
空的冲突条目在未触碰保存后存活;被编辑过的冲突行会在仍然可见的控件上被拒绝,而不是被丢弃。
|
||||
|
||||
## 待决问题
|
||||
|
||||
- 渠道级 `"*"` 的「需要 @」默认值是否也应该可编辑?它是每个仓库继承的值,而今天只能改
|
||||
`settings.json`。本次不做,以保持 diff 聚焦在按仓库控制;值得后续单独决定。
|
||||
- 轮询间隔在 UI 里是否应该用秒表示?毫秒与存储键以及 Email 插件既有标签一致,
|
||||
所以本设计为保持一致仍用毫秒。
|
||||
@@ -1268,7 +1268,11 @@ presence metadata, startup state, and runtime state; literal secrets are never
|
||||
returned. Channel snapshots use `Cache-Control: no-store`.
|
||||
|
||||
Field descriptors can expose nested object metadata through `properties`.
|
||||
Numeric descriptors can use `exclusiveMinimum` for open lower bounds. String
|
||||
Numeric descriptors can use `exclusiveMinimum` for an open lower bound and
|
||||
`maximum` for an inclusive upper bound. A stored number outside a newly declared
|
||||
bound is still accepted when the client re-sends it unchanged, so declaring a
|
||||
bound does not block unrelated edits to an existing channel; a value the client
|
||||
actually changed must satisfy both bounds. String
|
||||
and secret descriptors can use `multiline` to ask clients for a multi-line text
|
||||
area; the descriptor types allow it only on top-level fields. Clients that do
|
||||
not render an advertised field kind must preserve its existing config value
|
||||
|
||||
@@ -98,6 +98,29 @@ 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.
|
||||
|
||||
### 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.
|
||||
|
||||
A repository with no stored setting starts at **Inherit**, and the option label names the value it inherits — for example `Inherit (require a mention)`. A repository that already stores a value opens on that value instead, so a stored `requireMention: false` shows _Answer without a mention_. A row left at Inherit writes no key, so the repository keeps following `groups["*"]` and any later change to `"*"` still reaches it.
|
||||
|
||||
Choosing an explicit value writes that repository's `groups` entry. Choosing Inherit again removes the **policy** key, and what happens to a speaker list you typed depends on the policy the row then inherits:
|
||||
|
||||
- It inherits an allowlist from `groups["*"]`: the names stay in force, and the speaker box stays visible so you can keep editing them.
|
||||
- It inherits **Anyone**: the names stay in the file but are not applied, because an open policy admits every group member. They apply again if the channel-wide policy later becomes an allowlist. The speaker box is not shown in this case, so there is nothing to clear.
|
||||
|
||||
The speaker box appears whenever the row's effective policy is an allowlist, whether the row set it or inherited it. What an **empty** box will do depends on the row's own policy, and the hint under the box says which case you are in:
|
||||
|
||||
- The row is set to **Allowlist only**: saving an empty box writes `allowedUsers: []`, which means _nobody_. The repository admits no one and does not fall back to the channel-wide list.
|
||||
- The row inherits its policy: saving an empty box removes the per-repository list, so the channel-wide `groups["*"].allowedUsers` applies. This is also what happens when you clear a box that was showing stored names — the repository widens back to the channel-wide list rather than locking down.
|
||||
- A stored `"senders": "allowlist"` with no `allowedUsers` beside it inherits its members, so its empty box reads as _inherit_; a stored `allowedUsers: []` with no policy admits _nobody_. The hint distinguishes the two.
|
||||
|
||||
Switching the group policy away from `"allowlist"` hides the rows but does not delete these entries. Under `"open"` and `"pairing"` the runtime keeps enforcing them, because per-repository senders and `requireMention` are resolved without consulting `groupPolicy`; under `"disabled"` the group gate drops every notification first, so the entries are kept but nothing is processed. The editor names the repositories whose settings this save keeps **and that can still take effect**, says which of those two cases applies, and tells you to switch back to Allowlist to edit them. A repository you typed during this visit is named too, even though it is not in the stored file yet. An entry is only announced when it can still change something: its own `senders` is `"allowlist"`, or its `requireMention` differs from the value it inherits. An `allowedUsers` list beside an inherited `"open"` policy restricts nobody — every group member is admitted regardless — so it is kept in the file but not announced as a setting that still applies. Row edits you made before switching are still written — including on a channel that was already `"open"`, and including a repository you added while the policy was `"allowlist"` — so the save cannot report success while restoring the old value. The one exception is a repository stored under two spellings that disagree, such as `Acme/Widgets` and `acme/widgets`: the editor cannot tell which spelling an edit was meant for, so an untouched save leaves both entries exactly as stored — including an empty one, which is what makes the exact spelling inherit `groups["*"]` rather than fall through to the other. If you edit that row and then switch the policy away from `"allowlist"`, the save is refused with a message under **Group policy** asking you to remove one of the two entries from `settings.json`, so the narrowing you just typed is never quietly dropped. Switching back to `"allowlist"` reports the same conflict.
|
||||
|
||||
Per-repository settings the editor does not model, such as `dispatchMode` and `groupHistoryLimit`, are preserved when you save. An entry that is already empty is preserved as well when the channel never used an allowlist, so an unrelated edit does not rewrite the file.
|
||||
|
||||
If `settings.json` holds a group entry whose value is not an object, the editor refuses the save and names the entry: the daemon rejects such an entry on every save, and no row in the form can represent it. Fix or remove it in `settings.json` and reload the channel. The editor does not delete it for you — removing the exact key would hand that repository to a differently spelled entry through the runtime's case-insensitive fallback.
|
||||
|
||||
## ⚠️ Security
|
||||
|
||||
On a **public repository**, setting `groups: { "*": { "senders": "open" } }` allows **any GitHub user** who triggers a supported notification reason to submit prompts that drive the agent in your `cwd`. This includes reading code, spending tokens, posting comments, and (subject to permission policy) running tools.
|
||||
|
||||
@@ -65,7 +65,7 @@ For self-hosted instances, set `baseUrl`:
|
||||
| Option | Default | Description |
|
||||
| ------------------------ | ------------------------- | ------------------------------------------------------------------------------------------------- |
|
||||
| `token` | (required) | PAT with `read_api` + `api` scopes |
|
||||
| `pollInterval` | `60000` | Poll interval in ms |
|
||||
| `pollInterval` | `60000` | Poll interval in ms, clamped to a maximum of `86400000` (one day) |
|
||||
| `baseUrl` | `https://gitlab.com` | GitLab instance URL |
|
||||
| `action_prompt_template` | (required for processing) | Maps GitLab action names to metadata templates |
|
||||
| `groupPolicy` | `"disabled"` | Must be `"open"`, `"allowlist"` with the project listed, or `"pairing"` with the project approved |
|
||||
|
||||
@@ -371,7 +371,7 @@ All group settings use field-by-field inheritance: the specific group, then `"*"
|
||||
|
||||
On the `github` and `gitlab` channels all inbound traffic is group traffic, so only `groupPolicy` and group member settings govern admission — see those channels' Security notes before using it on a public repository or project. Under `sessionScope: "single"` direct messages and groups share one conversation, so opening a group also exposes direct-message history to its members.
|
||||
|
||||
In the Web Shell channel editor, **Who can talk in groups** and **Allowed group member IDs** edit `groups["*"]`, and **Session operators** edits `operators`. When `senders` is unset, the editor shows its effective default and writes nothing until you pick a value. Per-group overrides and an explicitly empty `"operators": []` are set in `settings.json`; saving from the editor keeps them.
|
||||
In the Web Shell channel editor, **Who can talk in groups** and **Allowed group member IDs** edit `groups["*"]`, and **Session operators** edits `operators`. When `senders` is unset, the editor shows its effective default and writes nothing until you pick a value. On `github`, the editor also shows one row per allowlisted repository for that repository's `senders`, `allowedUsers`, and `requireMention` — see [GitHub](./github.md#editing-from-the-web-shell). Other per-group overrides (`dispatchMode`, `groupHistoryLimit`, and per-group `requireMention` on chat platforms) and an explicitly empty `"operators": []` are set in `settings.json`; saving from the editor keeps them.
|
||||
|
||||
### Group History Backfill
|
||||
|
||||
|
||||
@@ -13,6 +13,17 @@ import { sanitizeLogText } from './sanitize.js';
|
||||
const INITIAL_BACKOFF = 2_000;
|
||||
const MAX_BACKOFF = 30_000;
|
||||
|
||||
/**
|
||||
* Inclusive bounds the poll loop enforces at runtime. Node clamps a timeout
|
||||
* above its 32-bit range to 1ms and rounds a sub-millisecond one to ~1ms, so
|
||||
* either end left open turns the loop into a hot loop that burns the platform's
|
||||
* rate quota. The descriptor vocabulary has only an exclusive lower bound, so
|
||||
* the inclusive floor a field documents is enforced here; a plugin declares
|
||||
* `maximum: MAX_POLL_INTERVAL_MS` so the two cannot drift apart.
|
||||
*/
|
||||
export const MIN_POLL_INTERVAL_MS = 1;
|
||||
export const MAX_POLL_INTERVAL_MS = 86_400_000;
|
||||
|
||||
export abstract class PollingChannelBase<Cursor> extends ChannelBase {
|
||||
protected cursor: Cursor;
|
||||
private abortController = new AbortController();
|
||||
@@ -74,7 +85,21 @@ export abstract class PollingChannelBase<Cursor> extends ChannelBase {
|
||||
Number.isFinite(configured) &&
|
||||
configured > 0
|
||||
) {
|
||||
return configured;
|
||||
// Node clamps a timeout above its 32-bit range to 1ms, which would turn
|
||||
// the poll loop into a hot loop that burns the platform's rate quota.
|
||||
// Clamp rather than fall back: an interval between one day and that limit
|
||||
// was honoured before this ceiling existed and is not a hot loop, so
|
||||
// replacing it with 60s would multiply the poll rate by orders of
|
||||
// magnitude. One day matches the ceiling Email enforces and GitHub
|
||||
// declares.
|
||||
//
|
||||
// The low end needs the same treatment from the other direction: a
|
||||
// fractional interval such as 0.5 passes the descriptor's exclusive
|
||||
// lower bound and the store, and setTimeout rounds it to ~1ms.
|
||||
return Math.min(
|
||||
Math.max(configured, MIN_POLL_INTERVAL_MS),
|
||||
MAX_POLL_INTERVAL_MS,
|
||||
);
|
||||
}
|
||||
return 60_000;
|
||||
}
|
||||
|
||||
@@ -12,7 +12,11 @@ export {
|
||||
readChannelEvents,
|
||||
} from './channel-events.js';
|
||||
export type { ChannelEventQuery, ChannelEventType } from './channel-events.js';
|
||||
export { PollingChannelBase } from './PollingChannelBase.js';
|
||||
export {
|
||||
PollingChannelBase,
|
||||
MIN_POLL_INTERVAL_MS,
|
||||
MAX_POLL_INTERVAL_MS,
|
||||
} from './PollingChannelBase.js';
|
||||
export { ACP_EVENT_LOOP_STALL_RESTART_MS, AcpBridge } from './AcpBridge.js';
|
||||
export {
|
||||
ACP_PRIVATE_PARENT_CAPABILITY_ENV,
|
||||
|
||||
@@ -563,6 +563,9 @@ export interface ChannelConfigNumberFieldDescriptor
|
||||
envResolvable?: never;
|
||||
multiline?: never;
|
||||
exclusiveMinimum?: number;
|
||||
/** Inclusive upper bound. A poll interval above Node's 32-bit timer range is
|
||||
* clamped to 1ms, so an unbounded interval field can persist a hot loop. */
|
||||
maximum?: number;
|
||||
properties?: never;
|
||||
}
|
||||
|
||||
|
||||
@@ -5514,6 +5514,65 @@ describe('GithubChannel', () => {
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('respects the one-day ceiling', () => {
|
||||
const ch = new TestableGithubChannel(
|
||||
'test',
|
||||
makeConfig({ pollInterval: 86_400_000 }),
|
||||
makeBridge(),
|
||||
);
|
||||
expect((ch as unknown as { pollInterval: number }).pollInterval).toBe(
|
||||
86_400_000,
|
||||
);
|
||||
});
|
||||
|
||||
it.each([86_400_001, 172_800_000, 2_592_000_000])(
|
||||
'clamps pollInterval %s to the ceiling instead of falling back',
|
||||
(value) => {
|
||||
// These were honoured before the ceiling existed and are nowhere near
|
||||
// Node's 32-bit timer limit, so replacing them with 60s would multiply
|
||||
// the poll rate by orders of magnitude. Clamp, do not fall back.
|
||||
const ch = new TestableGithubChannel(
|
||||
'test',
|
||||
makeConfig({ pollInterval: value }),
|
||||
makeBridge(),
|
||||
);
|
||||
expect((ch as unknown as { pollInterval: number }).pollInterval).toBe(
|
||||
86_400_000,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([0.5, 0.001, 0.999])(
|
||||
'raises a sub-millisecond pollInterval %s to the documented floor',
|
||||
(value) => {
|
||||
// The descriptor's lower bound is exclusive, so a fraction passes both
|
||||
// the editor and the store; setTimeout then rounds it to ~1ms, which is
|
||||
// the hot loop the ceiling exists to prevent, reached from below. The
|
||||
// field documents 1 as the minimum, so the runtime has to enforce it.
|
||||
const ch = new TestableGithubChannel(
|
||||
'test',
|
||||
makeConfig({ pollInterval: value }),
|
||||
makeBridge(),
|
||||
);
|
||||
expect((ch as unknown as { pollInterval: number }).pollInterval).toBe(
|
||||
1,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it('keeps a whole millisecond at the floor and just above it', () => {
|
||||
for (const value of [1, 2, 60_000]) {
|
||||
const ch = new TestableGithubChannel(
|
||||
'test',
|
||||
makeConfig({ pollInterval: value }),
|
||||
makeBridge(),
|
||||
);
|
||||
expect((ch as unknown as { pollInterval: number }).pollInterval).toBe(
|
||||
value,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('plugin', () => {
|
||||
@@ -5522,6 +5581,20 @@ describe('GithubChannel', () => {
|
||||
expect(plugin.defaultSessionScope).toBe('chat_thread');
|
||||
});
|
||||
|
||||
it('declares a bounded poll interval so the form cannot persist a hot loop', async () => {
|
||||
const { plugin } = await import('./index.js');
|
||||
const field = plugin.management?.fields.find(
|
||||
(item) => item.key === 'pollInterval',
|
||||
);
|
||||
// The ceiling matches the one the Email channel enforces at runtime.
|
||||
expect(field).toMatchObject({
|
||||
kind: 'number',
|
||||
exclusiveMinimum: 0,
|
||||
maximum: 86_400_000,
|
||||
default: '60000',
|
||||
});
|
||||
});
|
||||
|
||||
it('allows explicit local gh authentication without a configured token', async () => {
|
||||
const { plugin } = await import('./index.js');
|
||||
const tokenField = plugin.management?.fields.find(
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { MAX_POLL_INTERVAL_MS } from '@qwen-code/channel-base';
|
||||
import type { ChannelPlugin } from '@qwen-code/channel-base';
|
||||
import {
|
||||
GithubChannel,
|
||||
@@ -37,6 +38,16 @@ export const plugin: ChannelPlugin = {
|
||||
description:
|
||||
'GitHub Enterprise API root (e.g. https://ghe.example.com/api/v3). Leave empty for github.com',
|
||||
},
|
||||
{
|
||||
key: 'pollInterval',
|
||||
label: 'Poll Interval (ms)',
|
||||
kind: 'number',
|
||||
exclusiveMinimum: 0,
|
||||
maximum: MAX_POLL_INTERVAL_MS,
|
||||
default: '60000',
|
||||
description:
|
||||
'How often to poll the notifications API, in ms. Kept between 1 and 86400000 (one day): a smaller positive value is raised to 1. Leave empty for 60000',
|
||||
},
|
||||
{
|
||||
key: 'groupPolicy',
|
||||
label: 'Group Policy',
|
||||
|
||||
@@ -87,12 +87,14 @@ function assertDescriptorWireShape(
|
||||
}
|
||||
if (descriptor.kind === 'number') {
|
||||
allowedKeys.add('exclusiveMinimum');
|
||||
allowedKeys.add('maximum');
|
||||
}
|
||||
expect((descriptor as { properties?: unknown }).properties).toBeUndefined();
|
||||
if (descriptor.kind !== 'number') {
|
||||
expect(
|
||||
(descriptor as { exclusiveMinimum?: unknown }).exclusiveMinimum,
|
||||
).toBeUndefined();
|
||||
expect((descriptor as { maximum?: unknown }).maximum).toBeUndefined();
|
||||
}
|
||||
}
|
||||
for (const key of Object.keys(descriptor)) {
|
||||
|
||||
@@ -374,6 +374,45 @@ describe('channel registry', () => {
|
||||
message:
|
||||
'Channel field "retentionDays" can only declare exclusiveMinimum on number fields.',
|
||||
},
|
||||
{
|
||||
type: 'invalid-maximum-on-string',
|
||||
fields: [
|
||||
{
|
||||
key: 'retentionDays',
|
||||
label: 'Retention days',
|
||||
kind: 'string',
|
||||
maximum: 0,
|
||||
},
|
||||
],
|
||||
message:
|
||||
'Channel field "retentionDays" can only declare maximum on number fields.',
|
||||
},
|
||||
{
|
||||
type: 'invalid-non-finite-maximum',
|
||||
fields: [
|
||||
{
|
||||
key: 'retries',
|
||||
label: 'Retries',
|
||||
kind: 'number',
|
||||
maximum: Number.NaN,
|
||||
},
|
||||
],
|
||||
message: 'Channel field "retries" must declare a finite maximum.',
|
||||
},
|
||||
{
|
||||
type: 'invalid-maximum-leaves-no-valid-value',
|
||||
fields: [
|
||||
{
|
||||
key: 'retries',
|
||||
label: 'Retries',
|
||||
kind: 'number',
|
||||
exclusiveMinimum: 5,
|
||||
maximum: 5,
|
||||
},
|
||||
],
|
||||
message:
|
||||
'Channel field "retries" declares a maximum that leaves no valid value.',
|
||||
},
|
||||
{
|
||||
type: 'invalid-env-resolvable-number',
|
||||
fields: [
|
||||
|
||||
@@ -243,6 +243,25 @@ function assertManagementField(
|
||||
);
|
||||
}
|
||||
}
|
||||
const maximum = (field as { maximum?: unknown }).maximum;
|
||||
if (maximum !== undefined) {
|
||||
if (field.kind !== 'number') {
|
||||
throw new Error(
|
||||
`Channel field "${path}" can only declare maximum on number fields.`,
|
||||
);
|
||||
}
|
||||
if (typeof maximum !== 'number' || !Number.isFinite(maximum)) {
|
||||
throw new Error(`Channel field "${path}" must declare a finite maximum.`);
|
||||
}
|
||||
if (
|
||||
field.exclusiveMinimum !== undefined &&
|
||||
maximum <= field.exclusiveMinimum
|
||||
) {
|
||||
throw new Error(
|
||||
`Channel field "${path}" declares a maximum that leaves no valid value.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
if (field.kind === 'enum') {
|
||||
if (!Array.isArray(field.options) || field.options.length === 0) {
|
||||
throw new Error(
|
||||
|
||||
@@ -124,6 +124,13 @@ describe('WorkspaceChannelSettingsStore', () => {
|
||||
kind: 'number',
|
||||
exclusiveMinimum: 0,
|
||||
},
|
||||
{
|
||||
key: 'boundedSeconds',
|
||||
label: 'Bounded seconds',
|
||||
kind: 'number',
|
||||
exclusiveMinimum: 0,
|
||||
maximum: 10,
|
||||
},
|
||||
{
|
||||
key: 'mode',
|
||||
label: 'Mode',
|
||||
@@ -1498,6 +1505,49 @@ describe('WorkspaceChannelSettingsStore', () => {
|
||||
},
|
||||
);
|
||||
|
||||
it('rejects a non-record per-entry group value even when it is unchanged', async () => {
|
||||
// The tolerance above covers the whole `groups` value, not an entry inside
|
||||
// it. A hand-written non-record entry is therefore refused on every save,
|
||||
// so an editor that passes one through cannot promise a no-op — it has to
|
||||
// refuse earlier with a message that names the entry.
|
||||
const groups = {
|
||||
'Acme/Widgets': 'hand-written-garbage',
|
||||
'acme/widgets': { senders: 'open' },
|
||||
};
|
||||
writeWorkspaceSettings(
|
||||
JSON.stringify({
|
||||
$version: 4,
|
||||
channels: {
|
||||
bot: {
|
||||
type: 'management-validation-test',
|
||||
clientId: 'client-id',
|
||||
clientSecret: 'existing-secret',
|
||||
groups,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
const store = new WorkspaceChannelSettingsStore(workspace);
|
||||
const before = fs.readFileSync(settingsPath, 'utf8');
|
||||
|
||||
await expect(
|
||||
store.upsert('bot', {
|
||||
expectedRevision: store.snapshot().revision,
|
||||
config: {
|
||||
type: 'management-validation-test',
|
||||
clientId: 'client-id',
|
||||
groups,
|
||||
},
|
||||
secrets: { clientSecret: { operation: 'preserve' } },
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
code: 'channel_settings_invalid_config',
|
||||
message: 'Channel field "groups.Acme/Widgets" is invalid.',
|
||||
});
|
||||
|
||||
expect(fs.readFileSync(settingsPath, 'utf8')).toBe(before);
|
||||
});
|
||||
|
||||
it('rejects unchanged non-record groups containing an unsafe key', async () => {
|
||||
const groups = [JSON.parse('{"__proto__":{"polluted":true}}') as unknown];
|
||||
writeWorkspaceSettings(
|
||||
@@ -3270,4 +3320,102 @@ describe('WorkspaceChannelSettingsStore', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('numeric bounds and unchanged stored values', () => {
|
||||
const baseConfig = {
|
||||
type: 'management-validation-test',
|
||||
clientId: 'client-id',
|
||||
};
|
||||
|
||||
it('tolerates re-sending a stored number a newer bound would reject', async () => {
|
||||
// The editor re-sends stored values verbatim on every save. Declaring a
|
||||
// field must not block an unrelated edit to a channel whose stored value
|
||||
// the runtime already tolerates.
|
||||
writeWorkspaceSettings(
|
||||
JSON.stringify({
|
||||
channels: {
|
||||
bounded: {
|
||||
...baseConfig,
|
||||
clientSecret: '$BOT_TOKEN',
|
||||
backoffSeconds: 0,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
const store = new WorkspaceChannelSettingsStore(workspace);
|
||||
|
||||
const next = await store.upsert('bounded', {
|
||||
expectedRevision: store.snapshot().revision,
|
||||
config: { ...baseConfig, backoffSeconds: 0, retries: 4 } as never,
|
||||
secrets: {},
|
||||
});
|
||||
|
||||
expect(next.channels['bounded']['backoffSeconds']).toBe(0);
|
||||
expect(next.channels['bounded']['retries']).toBe(4);
|
||||
});
|
||||
|
||||
it('still rejects a number the operator changed to an invalid value', async () => {
|
||||
const store = new WorkspaceChannelSettingsStore(workspace);
|
||||
await store.upsert('bounded', {
|
||||
expectedRevision: store.snapshot().revision,
|
||||
config: { ...baseConfig, backoffSeconds: 5 } as never,
|
||||
secrets: { clientSecret: { operation: 'replace', value: 'secret' } },
|
||||
});
|
||||
|
||||
await expect(
|
||||
store.upsert('bounded', {
|
||||
expectedRevision: store.snapshot().revision,
|
||||
config: { ...baseConfig, backoffSeconds: 0 } as never,
|
||||
secrets: {},
|
||||
}),
|
||||
).rejects.toMatchObject({ code: 'channel_settings_invalid_config' });
|
||||
});
|
||||
|
||||
it('enforces a declared maximum', async () => {
|
||||
const store = new WorkspaceChannelSettingsStore(workspace);
|
||||
const revision = store.snapshot().revision;
|
||||
|
||||
await expect(
|
||||
store.upsert('bounded', {
|
||||
expectedRevision: revision,
|
||||
config: { ...baseConfig, boundedSeconds: 11 } as never,
|
||||
secrets: { clientSecret: { operation: 'replace', value: 'secret' } },
|
||||
}),
|
||||
).rejects.toMatchObject({ code: 'channel_settings_invalid_config' });
|
||||
|
||||
const next = await store.upsert('bounded', {
|
||||
expectedRevision: store.snapshot().revision,
|
||||
config: { ...baseConfig, boundedSeconds: 10 } as never,
|
||||
secrets: { clientSecret: { operation: 'replace', value: 'secret' } },
|
||||
});
|
||||
expect(next.channels['bounded']['boundedSeconds']).toBe(10);
|
||||
});
|
||||
|
||||
it('tolerates re-sending a stored number above a newly declared maximum', async () => {
|
||||
// The minimum half alone would leave an operator storing 300000000
|
||||
// stranded on every unrelated edit once a ceiling is declared, which is
|
||||
// the regression the tolerance exists to prevent.
|
||||
writeWorkspaceSettings(
|
||||
JSON.stringify({
|
||||
channels: {
|
||||
bounded: {
|
||||
...baseConfig,
|
||||
clientSecret: '$BOT_TOKEN',
|
||||
boundedSeconds: 300,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
const store = new WorkspaceChannelSettingsStore(workspace);
|
||||
|
||||
const next = await store.upsert('bounded', {
|
||||
expectedRevision: store.snapshot().revision,
|
||||
config: { ...baseConfig, boundedSeconds: 300, retries: 4 } as never,
|
||||
secrets: {},
|
||||
});
|
||||
|
||||
expect(next.channels['bounded']['boundedSeconds']).toBe(300);
|
||||
expect(next.channels['bounded']['retries']).toBe(4);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -354,8 +354,14 @@ function assertDescriptorValue(
|
||||
(field.kind === 'number' &&
|
||||
typeof value === 'number' &&
|
||||
Number.isFinite(value) &&
|
||||
(field.exclusiveMinimum === undefined ||
|
||||
value > field.exclusiveMinimum)) ||
|
||||
(((field.exclusiveMinimum === undefined ||
|
||||
value > field.exclusiveMinimum) &&
|
||||
(field.maximum === undefined || value <= field.maximum)) ||
|
||||
// A newly declared bound must not strand an unrelated edit: the editor
|
||||
// re-sends the stored number verbatim, and the runtime already tolerates
|
||||
// it (a non-positive poll interval falls back to the default). Only the
|
||||
// range is forgiven — a wrong type is still rejected, unchanged or not.
|
||||
isDeepStrictEqual(previous, value))) ||
|
||||
(field.kind === 'enum' &&
|
||||
typeof value === 'string' &&
|
||||
field.options?.some((option) => option.value === value) === true) ||
|
||||
|
||||
@@ -4942,6 +4942,9 @@ export interface DaemonChannelConfigNumberFieldDescriptor
|
||||
envResolvable?: never;
|
||||
multiline?: never;
|
||||
exclusiveMinimum?: number;
|
||||
/** Inclusive upper bound. A poll interval above Node's 32-bit timer range is
|
||||
* clamped to 1ms, so an unbounded interval field can persist a hot loop. */
|
||||
maximum?: number;
|
||||
properties?: never;
|
||||
}
|
||||
|
||||
|
||||
@@ -336,6 +336,36 @@
|
||||
line-height: 1.5;
|
||||
}
|
||||
|
||||
.repositoryList {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 8px;
|
||||
}
|
||||
|
||||
.repositoryListTitle {
|
||||
margin: 0;
|
||||
font-size: 12px;
|
||||
font-weight: 500;
|
||||
color: var(--muted-foreground);
|
||||
}
|
||||
|
||||
.repositoryRow {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 8px;
|
||||
padding: 10px 11px;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: 8px;
|
||||
background: var(--card);
|
||||
}
|
||||
|
||||
.repositoryName {
|
||||
margin: 0;
|
||||
font-size: 12px;
|
||||
font-weight: 600;
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
@media (max-width: 520px) {
|
||||
.policyGrid {
|
||||
grid-template-columns: 1fr;
|
||||
|
||||
@@ -1842,3 +1842,681 @@ describe('ChannelEditorDialog', () => {
|
||||
expect(listPairingRequests).toHaveBeenCalledWith('release-bot');
|
||||
});
|
||||
});
|
||||
|
||||
describe('GitHub per-repository settings', () => {
|
||||
const GITHUB_POLLING: DaemonChannelTypeDescriptor = {
|
||||
...GITHUB_CONFIGURED,
|
||||
fields: [
|
||||
...GITHUB_CONFIGURED.fields,
|
||||
{
|
||||
key: 'pollInterval',
|
||||
label: 'Poll Interval (ms)',
|
||||
kind: 'number',
|
||||
exclusiveMinimum: 0,
|
||||
maximum: 86_400_000,
|
||||
default: '60000',
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
function githubInstance(
|
||||
groups: Record<string, unknown>,
|
||||
): DaemonChannelInstanceSnapshot {
|
||||
return {
|
||||
name: 'github-bot',
|
||||
config: { type: 'github', groupPolicy: 'allowlist', groups },
|
||||
secrets: { token: { present: true, source: 'literal' } },
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
}
|
||||
|
||||
function rowLabels(text: string): HTMLElement[] {
|
||||
return Array.from(document.querySelectorAll('label')).filter((label) =>
|
||||
label.textContent?.includes(text),
|
||||
);
|
||||
}
|
||||
|
||||
function rowControl(
|
||||
repositoryId: string,
|
||||
kind: 'mention' | 'senders' | 'speakers',
|
||||
): HTMLElement | null {
|
||||
return document.querySelector(`[id$="-${kind}-${repositoryId}"]`);
|
||||
}
|
||||
|
||||
async function chooseRowOption(
|
||||
repositoryId: string,
|
||||
kind: 'mention' | 'senders',
|
||||
optionLabel: string,
|
||||
) {
|
||||
const trigger = rowControl(repositoryId, kind);
|
||||
expect(trigger).not.toBeNull();
|
||||
await act(async () => {
|
||||
trigger!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
|
||||
});
|
||||
const option = Array.from(
|
||||
document.querySelectorAll<HTMLElement>('[role="option"]'),
|
||||
).find((item) => item.textContent?.trim() === optionLabel);
|
||||
expect(option).toBeDefined();
|
||||
await act(async () => {
|
||||
option!.dispatchEvent(new MouseEvent('click', { bubbles: true }));
|
||||
});
|
||||
}
|
||||
|
||||
async function save() {
|
||||
const button = Array.from(document.querySelectorAll('button')).find(
|
||||
(item) => item.textContent?.trim() === 'Save',
|
||||
);
|
||||
await act(async () => {
|
||||
button?.click();
|
||||
});
|
||||
}
|
||||
|
||||
it('renders one row per allowlisted repository', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': {},
|
||||
'qwenlm/qwen-code': {},
|
||||
'acme/widgets': {},
|
||||
}),
|
||||
});
|
||||
|
||||
expect(rowLabels('Require a mention')).toHaveLength(2);
|
||||
expect(rowLabels('Who may speak')).toHaveLength(2);
|
||||
expect(document.body.textContent).toContain('qwenlm/qwen-code');
|
||||
expect(document.body.textContent).toContain('acme/widgets');
|
||||
});
|
||||
|
||||
it('adds a row as soon as a repository is typed into the allowlist', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({ 'qwenlm/qwen-code': {} }),
|
||||
});
|
||||
expect(rowLabels('Require a mention')).toHaveLength(1);
|
||||
|
||||
const allowlist = inputByLabel('Allowed repositories');
|
||||
expect(allowlist).not.toBeNull();
|
||||
await act(async () => {
|
||||
setInputValue(allowlist!, 'qwenlm/qwen-code, acme/widgets');
|
||||
});
|
||||
|
||||
expect(rowLabels('Require a mention')).toHaveLength(2);
|
||||
expect(rowControl('acme/widgets', 'mention')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('names the inherited value in the inherit option', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { requireMention: false, senders: 'allowlist' },
|
||||
'qwenlm/qwen-code': {},
|
||||
}),
|
||||
});
|
||||
|
||||
expect(rowControl('qwenlm/qwen-code', 'mention')?.textContent).toContain(
|
||||
'Inherit (answer without a mention)',
|
||||
);
|
||||
expect(rowControl('qwenlm/qwen-code', 'senders')?.textContent).toContain(
|
||||
'Inherit (allowlist only)',
|
||||
);
|
||||
});
|
||||
|
||||
it('shows the speaker input only once the row allows a list', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'open' },
|
||||
'qwenlm/qwen-code': {},
|
||||
}),
|
||||
});
|
||||
expect(rowControl('qwenlm/qwen-code', 'speakers')).toBeNull();
|
||||
|
||||
await chooseRowOption('qwenlm/qwen-code', 'senders', 'Allowlist only');
|
||||
|
||||
expect(rowControl('qwenlm/qwen-code', 'speakers')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('shows the speaker input for a row inheriting an allowlist', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'qwenlm/qwen-code': {},
|
||||
}),
|
||||
});
|
||||
|
||||
expect(rowControl('qwenlm/qwen-code', 'speakers')).not.toBeNull();
|
||||
expect(rowControl('qwenlm/qwen-code', 'senders')?.textContent).toContain(
|
||||
'Inherit (allowlist only)',
|
||||
);
|
||||
});
|
||||
|
||||
it('saves an explicit per-repository override', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({ 'qwenlm/qwen-code': {} }),
|
||||
onSave,
|
||||
});
|
||||
|
||||
await chooseRowOption(
|
||||
'qwenlm/qwen-code',
|
||||
'mention',
|
||||
'Answer without a mention',
|
||||
);
|
||||
await chooseRowOption('qwenlm/qwen-code', 'senders', 'Allowlist only');
|
||||
await act(async () => {
|
||||
setInputValue(
|
||||
rowControl('qwenlm/qwen-code', 'speakers') as HTMLInputElement,
|
||||
'alice, bob',
|
||||
);
|
||||
});
|
||||
await save();
|
||||
|
||||
expect(onSave).toHaveBeenCalledTimes(1);
|
||||
const request = onSave.mock.calls[0][1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['groups']).toEqual({
|
||||
'qwenlm/qwen-code': {
|
||||
requireMention: false,
|
||||
senders: 'allowlist',
|
||||
allowedUsers: ['alice', 'bob'],
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('labels each row so repeated control names stay distinguishable', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'qwenlm/qwen-code': {},
|
||||
'acme/widgets': {},
|
||||
}),
|
||||
});
|
||||
|
||||
const rows = Array.from(
|
||||
document.querySelectorAll('[role="group"][aria-labelledby]'),
|
||||
).filter((row) => row.querySelector('h4'));
|
||||
expect(rows).toHaveLength(2);
|
||||
expect(rows.map((row) => row.querySelector('h4')?.textContent)).toEqual([
|
||||
'qwenlm/qwen-code',
|
||||
'acme/widgets',
|
||||
]);
|
||||
for (const row of rows) {
|
||||
expect(row.getAttribute('aria-labelledby')).toBe(
|
||||
row.querySelector('h4')?.id,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('renders the poll interval under Connection', async () => {
|
||||
await renderDialog({ descriptor: GITHUB_POLLING });
|
||||
|
||||
expect(sectionHeadingOfLabel('Poll Interval (ms)')).toBe('Connection');
|
||||
expect(inputByLabel('Poll Interval (ms)')?.type).toBe('number');
|
||||
});
|
||||
|
||||
it('files the poll interval under Connection for any platform declaring it', async () => {
|
||||
// The section map is keyed by field name, so Email's existing interval
|
||||
// moves too. Pin the documented side effect instead of leaving it to prose.
|
||||
const email: DaemonChannelTypeDescriptor = {
|
||||
type: 'email',
|
||||
displayName: 'Email',
|
||||
manageable: true,
|
||||
fields: [
|
||||
{
|
||||
key: 'pollInterval',
|
||||
label: 'Poll Interval (ms)',
|
||||
kind: 'number',
|
||||
exclusiveMinimum: 0,
|
||||
default: '60000',
|
||||
},
|
||||
],
|
||||
};
|
||||
await renderDialog({ descriptor: email });
|
||||
|
||||
expect(sectionHeadingOfLabel('Poll Interval (ms)')).toBe('Connection');
|
||||
expect(document.body.textContent).not.toContain('Per-repository settings');
|
||||
});
|
||||
|
||||
it('keeps repository rows away from chat platforms', async () => {
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'chat-bot',
|
||||
config: {
|
||||
type: 'dingtalk',
|
||||
clientId: 'id',
|
||||
clientSecret: 'secret',
|
||||
groupPolicy: 'allowlist',
|
||||
// Shaped like a repository on purpose: a `group-a` fixture would pass
|
||||
// even if the platform gate were removed, because the owner/repo
|
||||
// filter would hide the rows anyway.
|
||||
groups: { 'acme/widgets': {} },
|
||||
},
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
await renderDialog({ descriptor: DINGTALK_WITH_ACCESS, instance });
|
||||
|
||||
expect(rowLabels('Require a mention')).toHaveLength(0);
|
||||
expect(document.body.textContent).not.toContain('Per-repository settings');
|
||||
});
|
||||
|
||||
it('derives the speaker hint from what the save will own, not the draft policy', async () => {
|
||||
// Two stored shapes this PR deliberately preserves. A policy with no list
|
||||
// inherits the channel-wide members; a list with no policy admits nobody.
|
||||
// Reading the hint off the draft policy tells both the opposite.
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op', 'release-bot'] },
|
||||
'qwenlm/qwen-code': { senders: 'allowlist' },
|
||||
'acme/widgets': { allowedUsers: [] },
|
||||
}),
|
||||
});
|
||||
|
||||
expect(descriptionOf(rowControl('qwenlm/qwen-code', 'speakers'))).toContain(
|
||||
'Leave empty to use the channel-wide list',
|
||||
);
|
||||
expect(descriptionOf(rowControl('acme/widgets', 'speakers'))).toContain(
|
||||
'Empty means nobody',
|
||||
);
|
||||
});
|
||||
|
||||
it('shows each row its own stored speakers and edits only that row', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'qwenlm/qwen-code': {
|
||||
senders: 'allowlist',
|
||||
allowedUsers: ['alice', 'bob'],
|
||||
},
|
||||
'acme/widgets': {},
|
||||
}),
|
||||
onSave,
|
||||
});
|
||||
|
||||
expect(
|
||||
(rowControl('qwenlm/qwen-code', 'speakers') as HTMLInputElement).value,
|
||||
).toBe('alice, bob');
|
||||
expect(
|
||||
(rowControl('acme/widgets', 'speakers') as HTMLInputElement).value,
|
||||
).toBe('');
|
||||
|
||||
await act(async () => {
|
||||
setInputValue(
|
||||
rowControl('acme/widgets', 'speakers') as HTMLInputElement,
|
||||
'carol',
|
||||
);
|
||||
});
|
||||
await save();
|
||||
|
||||
const request = onSave.mock.calls[0][1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['groups']).toEqual({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'qwenlm/qwen-code': {
|
||||
senders: 'allowlist',
|
||||
allowedUsers: ['alice', 'bob'],
|
||||
},
|
||||
'acme/widgets': { allowedUsers: ['carol'] },
|
||||
});
|
||||
});
|
||||
|
||||
it('shows an explicitly open row as Anyone rather than as inheriting', async () => {
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'qwenlm/qwen-code': { senders: 'open' },
|
||||
}),
|
||||
});
|
||||
|
||||
expect(rowControl('qwenlm/qwen-code', 'senders')?.textContent).toContain(
|
||||
'Anyone',
|
||||
);
|
||||
expect(rowControl('qwenlm/qwen-code', 'speakers')).toBeNull();
|
||||
});
|
||||
|
||||
it('renders the localized interval label, not the descriptor fallback', async () => {
|
||||
// The fixture's descriptor label is byte-identical to the English string,
|
||||
// so only a zh-CN render proves the label mapping is load-bearing.
|
||||
await renderDialog({ descriptor: GITHUB_POLLING, language: 'zh-CN' });
|
||||
|
||||
expect(inputByLabel('轮询间隔(毫秒)')).not.toBeNull();
|
||||
expect(sectionHeadingOfLabel('轮询间隔(毫秒)')).toBe('连接');
|
||||
});
|
||||
|
||||
it('names the repositories whose settings outlive the allowlist policy', async () => {
|
||||
// The runtime resolves per-repository senders without consulting
|
||||
// groupPolicy, so switching the policy away must not make those entries
|
||||
// invisible while they still decide who may drive the agent.
|
||||
const instance = githubInstance({
|
||||
'*': {},
|
||||
'acme/widgets': { senders: 'allowlist', allowedUsers: ['alice'] },
|
||||
});
|
||||
instance.config['groupPolicy'] = 'open';
|
||||
await renderDialog({ descriptor: GITHUB_CONFIGURED, instance });
|
||||
|
||||
expect(rowLabels('Require a mention')).toHaveLength(0);
|
||||
expect(document.body.textContent).toContain(
|
||||
'Repository settings still apply',
|
||||
);
|
||||
expect(document.body.textContent).toContain('acme/widgets');
|
||||
});
|
||||
|
||||
it('says nothing about retained settings when there are none', async () => {
|
||||
const instance = githubInstance({ '*': { senders: 'open' } });
|
||||
instance.config['groupPolicy'] = 'open';
|
||||
await renderDialog({ descriptor: GITHUB_CONFIGURED, instance });
|
||||
|
||||
expect(document.body.textContent).not.toContain(
|
||||
'Repository settings still apply',
|
||||
);
|
||||
});
|
||||
|
||||
it('saves an unrelated edit on a channel whose stored interval the runtime tolerates', async () => {
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'github-bot',
|
||||
config: { type: 'github', groupPolicy: 'open', pollInterval: 0 },
|
||||
secrets: { token: { present: true, source: 'literal' } },
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
await renderDialog({ descriptor: GITHUB_POLLING, instance, onSave });
|
||||
|
||||
expect(inputByLabel('Poll Interval (ms)')?.value).toBe('0');
|
||||
await save();
|
||||
|
||||
expect(onSave).toHaveBeenCalledTimes(1);
|
||||
const request = onSave.mock.calls[0][1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['pollInterval']).toBe(0);
|
||||
});
|
||||
|
||||
it('tells a stored list under an inherited policy that clearing it returns the channel-wide list', async () => {
|
||||
// The hint promises what an EMPTY box will do. Asking the projection about
|
||||
// the untouched draft would say "nobody" here, while clearing the box
|
||||
// actually deletes the list and widens the repository back to the wildcard.
|
||||
// The repository's own list is the admitted set until then, so the hint has
|
||||
// to say the channel-wide list will be *used*, not that it is already kept.
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op', 'release-bot'] },
|
||||
'qwenlm/qwen-code': { allowedUsers: ['alice'] },
|
||||
}),
|
||||
onSave,
|
||||
});
|
||||
|
||||
const speakers = rowControl(
|
||||
'qwenlm/qwen-code',
|
||||
'speakers',
|
||||
) as HTMLInputElement;
|
||||
expect(speakers.value).toBe('alice');
|
||||
expect(descriptionOf(speakers)).toContain(
|
||||
'Leave empty to use the channel-wide list',
|
||||
);
|
||||
|
||||
await act(async () => setInputValue(speakers, ''));
|
||||
await save();
|
||||
|
||||
// What the hint promised: the repository no longer owns a list, so the
|
||||
// runtime's `??` hands it the two channel-wide members instead of alice.
|
||||
const request = onSave.mock.calls[0][1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['groups']).toEqual({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op', 'release-bot'] },
|
||||
'qwenlm/qwen-code': {},
|
||||
});
|
||||
});
|
||||
|
||||
it('says the retained settings are kept but inert while the policy is Disabled', async () => {
|
||||
// The group gate drops every envelope before the mention or sender lookup,
|
||||
// so "still apply" would be false under Disabled.
|
||||
const instance = githubInstance({
|
||||
'*': {},
|
||||
'acme/widgets': { senders: 'allowlist', allowedUsers: ['alice'] },
|
||||
});
|
||||
instance.config['groupPolicy'] = 'disabled';
|
||||
await renderDialog({ descriptor: GITHUB_CONFIGURED, instance });
|
||||
|
||||
expect(document.body.textContent).toContain(
|
||||
'Repository settings are kept but not applied',
|
||||
);
|
||||
expect(document.body.textContent).not.toContain(
|
||||
'Repository settings still apply',
|
||||
);
|
||||
expect(document.body.textContent).toContain('acme/widgets');
|
||||
});
|
||||
|
||||
it('refuses an edited row when the policy leaves a conflicting allowlist', async () => {
|
||||
// The rows disappear once the policy leaves Allowlist and the save path
|
||||
// keeps both spellings as stored, so an edit made before switching would be
|
||||
// discarded while the dialog closes on success. The error has to land on a
|
||||
// control still on screen — the policy select, not the hidden allowlist.
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'github-bot',
|
||||
config: {
|
||||
type: 'github',
|
||||
groupPolicy: 'allowlist',
|
||||
groups: {
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'Acme/Widgets': { senders: 'allowlist', allowedUsers: ['alice'] },
|
||||
'acme/widgets': { senders: 'open' },
|
||||
},
|
||||
},
|
||||
secrets: { token: { present: true, source: 'literal' } },
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
await renderDialog({ descriptor: GITHUB_CONFIGURED, instance, onSave });
|
||||
|
||||
await chooseRowOption('acme/widgets', 'senders', 'Allowlist only');
|
||||
await selectOption('Group Policy', 'Open');
|
||||
await save();
|
||||
|
||||
expect(document.body.textContent).toContain(
|
||||
'This channel lists one repository twice with different settings',
|
||||
);
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still saves an untouched switch away from a conflicting allowlist', async () => {
|
||||
// Refusing every save would strand an operator who only changed the policy:
|
||||
// keeping both spellings verbatim is what the runtime already resolved.
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
const instance: DaemonChannelInstanceSnapshot = {
|
||||
name: 'github-bot',
|
||||
config: {
|
||||
type: 'github',
|
||||
groupPolicy: 'allowlist',
|
||||
groups: {
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'Acme/Widgets': { senders: 'allowlist', allowedUsers: ['alice'] },
|
||||
'acme/widgets': { senders: 'open' },
|
||||
},
|
||||
},
|
||||
secrets: { token: { present: true, source: 'literal' } },
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
};
|
||||
await renderDialog({ descriptor: GITHUB_CONFIGURED, instance, onSave });
|
||||
|
||||
await selectOption('Group Policy', 'Open');
|
||||
await save();
|
||||
|
||||
expect(onSave).toHaveBeenCalledTimes(1);
|
||||
const request = onSave.mock.calls[0][1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
expect(request.config['groups']).toEqual({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'Acme/Widgets': { senders: 'allowlist', allowedUsers: ['alice'] },
|
||||
'acme/widgets': { senders: 'open' },
|
||||
});
|
||||
});
|
||||
|
||||
it('names a repository the save writes but storage has never seen', async () => {
|
||||
// Typing a repository, narrowing its row and switching the policy away
|
||||
// writes an entry that keeps deciding who may drive that repository. The
|
||||
// alert used to enumerate stored groups, so it named nothing and the
|
||||
// dialog reported success in silence.
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
}),
|
||||
onSave,
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
setInputValue(inputByLabel('Allowed repositories')!, 'acme/widgets');
|
||||
});
|
||||
await chooseRowOption('acme/widgets', 'senders', 'Allowlist only');
|
||||
await act(async () => {
|
||||
setInputValue(
|
||||
rowControl('acme/widgets', 'speakers') as HTMLInputElement,
|
||||
'alice',
|
||||
);
|
||||
});
|
||||
await selectOption('Group Policy', 'Open');
|
||||
|
||||
expect(document.body.textContent).toContain(
|
||||
'Repository settings still apply',
|
||||
);
|
||||
expect(document.body.textContent).toContain('acme/widgets');
|
||||
});
|
||||
|
||||
it('does not announce a speaker list the resolved open policy makes inert', async () => {
|
||||
// senderGateFor builds an allowlist only when the resolved policy is
|
||||
// allowlist, so this array restricts nobody and must not be announced as a
|
||||
// setting that still applies.
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'open' },
|
||||
'acme/widgets': { allowedUsers: ['alice'] },
|
||||
}),
|
||||
});
|
||||
|
||||
await selectOption('Group Policy', 'Open');
|
||||
|
||||
expect(document.body.textContent).not.toContain(
|
||||
'Repository settings still apply',
|
||||
);
|
||||
});
|
||||
|
||||
it('refuses a save while a stored group entry is not an object, and names it', async () => {
|
||||
// The store rejects such an entry on every save, unchanged or not, and no
|
||||
// row can represent it. A generic daemon error would leave the operator
|
||||
// with nothing to act on, so the refusal names the entry and where to fix
|
||||
// it — and the entry is still not deleted, which would widen the alias.
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'Acme/Widgets': 'hand-written-garbage',
|
||||
}),
|
||||
onSave,
|
||||
});
|
||||
|
||||
await selectOption('Group Policy', 'Open');
|
||||
await save();
|
||||
|
||||
expect(document.body.textContent).toContain(
|
||||
'stores a group entry that is not an object',
|
||||
);
|
||||
expect(document.body.textContent).toContain('Acme/Widgets');
|
||||
expect(document.body.textContent).toContain('settings.json');
|
||||
expect(onSave).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('maps the Inherit option back to an absent key in both row selects', async () => {
|
||||
// The widget's `inherit` sentinel only becomes the draft's '' here, and a
|
||||
// cast hides a regression from the type checker: passing the raw value
|
||||
// through would write requireMention = ('inherit' === 'true') = false.
|
||||
const onSave = vi.fn().mockResolvedValue(undefined);
|
||||
await renderDialog({
|
||||
descriptor: GITHUB_CONFIGURED,
|
||||
instance: githubInstance({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'qwenlm/qwen-code': {
|
||||
requireMention: false,
|
||||
senders: 'allowlist',
|
||||
allowedUsers: ['alice'],
|
||||
},
|
||||
}),
|
||||
onSave,
|
||||
});
|
||||
|
||||
await chooseRowOption(
|
||||
'qwenlm/qwen-code',
|
||||
'mention',
|
||||
'Inherit (require a mention)',
|
||||
);
|
||||
await chooseRowOption(
|
||||
'qwenlm/qwen-code',
|
||||
'senders',
|
||||
'Inherit (allowlist only)',
|
||||
);
|
||||
await save();
|
||||
|
||||
const request = onSave.mock.calls[0][1] as {
|
||||
config: Record<string, unknown>;
|
||||
};
|
||||
// A typed list survives a return to Inherit; only the two policy keys go.
|
||||
expect(request.config['groups']).toEqual({
|
||||
'*': { senders: 'allowlist', allowedUsers: ['op'] },
|
||||
'qwenlm/qwen-code': { allowedUsers: ['alice'] },
|
||||
});
|
||||
});
|
||||
|
||||
it('names only the bounds the descriptor declares in the range message', async () => {
|
||||
const maxOnly: DaemonChannelTypeDescriptor = {
|
||||
type: 'example',
|
||||
displayName: 'Example',
|
||||
manageable: true,
|
||||
fields: [
|
||||
{
|
||||
key: 'concurrency',
|
||||
label: 'Concurrency',
|
||||
kind: 'number',
|
||||
maximum: 4,
|
||||
},
|
||||
],
|
||||
};
|
||||
await renderDialog({ descriptor: maxOnly });
|
||||
await act(async () => {
|
||||
setInputValue(inputByLabel('Concurrency')!, '9');
|
||||
});
|
||||
await save();
|
||||
|
||||
expect(document.body.textContent).toContain('Enter a number at most 4.');
|
||||
expect(document.body.textContent).not.toContain('greater than');
|
||||
});
|
||||
|
||||
it('names both bounds when the descriptor declares both', async () => {
|
||||
await renderDialog({ descriptor: GITHUB_POLLING });
|
||||
await act(async () => {
|
||||
setInputValue(inputByLabel('Poll Interval (ms)')!, '2592000000');
|
||||
});
|
||||
await save();
|
||||
|
||||
expect(document.body.textContent).toContain(
|
||||
'Enter a number greater than 0 and at most 86400000.',
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -61,11 +61,16 @@ import {
|
||||
hasDescriptorPrivatePolicy,
|
||||
configuredPrivatePolicy,
|
||||
PRIVATE_ONLY_FIELD_KEYS,
|
||||
invalidRepositoryGroupEntries,
|
||||
projectRepositoryGroup,
|
||||
repositoryAllowlistIds,
|
||||
retainedRepositorySettings,
|
||||
toList,
|
||||
validateChannelEditorDraft,
|
||||
type ChannelEditorDraft,
|
||||
type ChannelSecretDraft,
|
||||
type ChannelEditorValidationCode,
|
||||
type ChannelGroupOverrideDraft,
|
||||
type ChannelGroupSenders,
|
||||
} from './channel-editor-state';
|
||||
import {
|
||||
@@ -94,6 +99,7 @@ const FIELD_LABEL_KEYS: Record<string, Record<string, string>> = {
|
||||
token: 'channels.editor.field.github.token',
|
||||
useLocalGh: 'channels.editor.field.github.useLocalGh',
|
||||
baseUrl: 'channels.editor.field.github.baseUrl',
|
||||
pollInterval: 'channels.editor.field.github.pollInterval',
|
||||
groupPolicy: 'channels.editor.field.github.groupPolicy',
|
||||
reasonFilter: 'channels.editor.field.github.reasonFilter',
|
||||
deliverToBoundSessions:
|
||||
@@ -139,13 +145,20 @@ const FIELD_PLACEHOLDER_KEYS: Record<string, string> = {
|
||||
'github.baseUrl': 'https://ghe.example.com/api/v3',
|
||||
};
|
||||
|
||||
const CONNECTION_FIELD_KEYS = new Set(['baseUrl', 'wsUrl']);
|
||||
const CONNECTION_FIELD_KEYS = new Set(['baseUrl', 'wsUrl', 'pollInterval']);
|
||||
const ROUTING_FIELD_KEYS = new Set([
|
||||
'reasonFilter',
|
||||
'messageRoutes',
|
||||
'defaultMessageRoute',
|
||||
]);
|
||||
|
||||
/** Every repository starts fully inheriting; nothing is materialized. */
|
||||
const EMPTY_GROUP_OVERRIDE: ChannelGroupOverrideDraft = {
|
||||
requireMention: '',
|
||||
senders: '',
|
||||
allowedUsers: '',
|
||||
};
|
||||
|
||||
function fieldSectionKey(
|
||||
field: DaemonChannelConfigFieldDescriptor,
|
||||
): 'credentials' | 'connection' | 'routing' {
|
||||
@@ -364,9 +377,24 @@ export function ChannelEditorDialog({
|
||||
return t('channels.editor.validation.invalidOption');
|
||||
if (code === 'number') return t('channels.editor.validation.number');
|
||||
if (code === 'outOfRange') {
|
||||
const numberField = field && field.kind === 'number' ? field : undefined;
|
||||
// Only name the bounds the descriptor actually declares: a field with a
|
||||
// maximum and no exclusiveMinimum has no floor, and inventing "greater
|
||||
// than 0" would state a constraint the validator does not enforce.
|
||||
return t('channels.editor.validation.outOfRange', {
|
||||
min:
|
||||
field && field.kind === 'number' ? (field.exclusiveMinimum ?? 0) : 0,
|
||||
...(numberField?.exclusiveMinimum !== undefined
|
||||
? { min: numberField.exclusiveMinimum }
|
||||
: {}),
|
||||
...(numberField?.maximum !== undefined
|
||||
? { max: numberField.maximum }
|
||||
: {}),
|
||||
});
|
||||
}
|
||||
if (code === 'invalidGroupEntry') {
|
||||
// Name the entries: the form has no row for them, so a generic refusal
|
||||
// would leave the operator with nothing to act on.
|
||||
return t('channels.editor.validation.invalidGroupEntry', {
|
||||
repositories: invalidRepositoryGroupEntries(instance).join(', '),
|
||||
});
|
||||
}
|
||||
if (code === 'policy') return t('channels.editor.validation.policy');
|
||||
@@ -817,6 +845,226 @@ export function ChannelEditorDialog({
|
||||
);
|
||||
};
|
||||
|
||||
const updateGroupOverride = (
|
||||
repositoryId: string,
|
||||
patch: Partial<ChannelGroupOverrideDraft>,
|
||||
) => {
|
||||
setDraft((current) => {
|
||||
const existing =
|
||||
current.groupOverrides[repositoryId] ?? EMPTY_GROUP_OVERRIDE;
|
||||
return {
|
||||
...current,
|
||||
groupOverrides: {
|
||||
...current.groupOverrides,
|
||||
[repositoryId]: { ...existing, ...patch },
|
||||
},
|
||||
};
|
||||
});
|
||||
};
|
||||
|
||||
const renderRepositoryOverrides = () => {
|
||||
const repositories = repositoryAllowlistIds(draft.allowedGroupIds);
|
||||
if (repositories.length === 0) return null;
|
||||
const groups = instance?.config['groups'];
|
||||
// Fold the stored keys the same way the save path does, so a mixed-case
|
||||
// stored repository still finds its own row.
|
||||
const storedGroups = isRecord(groups)
|
||||
? Object.fromEntries(
|
||||
Object.entries(groups).map(([key, value]) => [
|
||||
key.toLowerCase(),
|
||||
value,
|
||||
]),
|
||||
)
|
||||
: {};
|
||||
const wildcard = isRecord(storedGroups['*']) ? storedGroups['*'] : {};
|
||||
const inheritedRequireMention =
|
||||
typeof wildcard['requireMention'] === 'boolean'
|
||||
? wildcard['requireMention']
|
||||
: true;
|
||||
const inheritedSenders = draft.groupSenders || defaultGroupSenders();
|
||||
return (
|
||||
<div className={styles.repositoryList}>
|
||||
<p className={styles.repositoryListTitle}>
|
||||
{t('channels.editor.field.github.repositorySettings')}
|
||||
</p>
|
||||
<p className={styles.fieldDescription}>
|
||||
{t('channels.editor.field.github.repositorySettings.description')}
|
||||
</p>
|
||||
{repositories.map((repositoryId) => {
|
||||
const override =
|
||||
draft.groupOverrides[repositoryId] ?? EMPTY_GROUP_OVERRIDE;
|
||||
const effectiveSenders = override.senders || inheritedSenders;
|
||||
const storedGroup = isRecord(storedGroups[repositoryId])
|
||||
? storedGroups[repositoryId]
|
||||
: {};
|
||||
// The hint promises what an EMPTY box will do, so ask the projection
|
||||
// about that hypothetical rather than about the current draft. A
|
||||
// stored list under an inherited policy is deleted when the box is
|
||||
// cleared, handing the repository back the channel-wide members —
|
||||
// asking about the untouched draft would promise "nobody" instead.
|
||||
const ownsSpeakerList = Array.isArray(
|
||||
projectRepositoryGroup(storedGroup, {
|
||||
...override,
|
||||
allowedUsers: '',
|
||||
})['allowedUsers'],
|
||||
);
|
||||
const headingId = `${formId}-repo-${repositoryId}`;
|
||||
const mentionId = `${formId}-mention-${repositoryId}`;
|
||||
const sendersId = `${formId}-senders-${repositoryId}`;
|
||||
const speakersId = `${formId}-speakers-${repositoryId}`;
|
||||
return (
|
||||
// Labelled by its own heading so the repeated control names are
|
||||
// distinguishable once more than one repository is listed.
|
||||
<div
|
||||
key={repositoryId}
|
||||
className={styles.repositoryRow}
|
||||
role="group"
|
||||
aria-labelledby={headingId}
|
||||
>
|
||||
<h4 id={headingId} className={styles.repositoryName}>
|
||||
{repositoryId}
|
||||
</h4>
|
||||
<FieldShell
|
||||
id={mentionId}
|
||||
label={t('channels.editor.field.github.requireMention')}
|
||||
>
|
||||
<Select
|
||||
value={override.requireMention || 'inherit'}
|
||||
onValueChange={(value) =>
|
||||
updateGroupOverride(repositoryId, {
|
||||
requireMention:
|
||||
value === 'inherit'
|
||||
? ''
|
||||
: (value as ChannelGroupOverrideDraft['requireMention']),
|
||||
})
|
||||
}
|
||||
>
|
||||
<SelectTrigger id={mentionId} className="w-full">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
<SelectItem value="inherit">
|
||||
{t(
|
||||
inheritedRequireMention
|
||||
? 'channels.editor.field.github.requireMention.inheritOn'
|
||||
: 'channels.editor.field.github.requireMention.inheritOff',
|
||||
)}
|
||||
</SelectItem>
|
||||
<SelectItem value="true">
|
||||
{t('channels.editor.field.github.requireMention.on')}
|
||||
</SelectItem>
|
||||
<SelectItem value="false">
|
||||
{t('channels.editor.field.github.requireMention.off')}
|
||||
</SelectItem>
|
||||
</SelectContent>
|
||||
</Select>
|
||||
</FieldShell>
|
||||
<FieldShell
|
||||
id={sendersId}
|
||||
label={t('channels.editor.field.github.repositorySenders')}
|
||||
>
|
||||
<Select
|
||||
value={override.senders || 'inherit'}
|
||||
onValueChange={(value) =>
|
||||
updateGroupOverride(repositoryId, {
|
||||
senders:
|
||||
value === 'inherit'
|
||||
? ''
|
||||
: (value as ChannelGroupOverrideDraft['senders']),
|
||||
})
|
||||
}
|
||||
>
|
||||
<SelectTrigger id={sendersId} className="w-full">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
<SelectItem value="inherit">
|
||||
{t(
|
||||
inheritedSenders === 'allowlist'
|
||||
? 'channels.editor.field.github.repositorySenders.inheritAllowlist'
|
||||
: 'channels.editor.field.github.repositorySenders.inheritOpen',
|
||||
)}
|
||||
</SelectItem>
|
||||
<SelectItem value="open">
|
||||
{t('channels.editor.field.github.repositorySenders.open')}
|
||||
</SelectItem>
|
||||
<SelectItem value="allowlist">
|
||||
{t(
|
||||
'channels.editor.field.github.repositorySenders.allowlist',
|
||||
)}
|
||||
</SelectItem>
|
||||
</SelectContent>
|
||||
</Select>
|
||||
</FieldShell>
|
||||
{effectiveSenders === 'allowlist' ? (
|
||||
<FieldShell
|
||||
id={speakersId}
|
||||
label={t('channels.editor.field.github.repositorySpeakers')}
|
||||
description={t(
|
||||
ownsSpeakerList
|
||||
? 'channels.editor.field.github.repositorySpeakers.description'
|
||||
: 'channels.editor.field.github.repositorySpeakers.inherited',
|
||||
)}
|
||||
>
|
||||
<Input
|
||||
id={speakersId}
|
||||
value={override.allowedUsers}
|
||||
placeholder={t(
|
||||
'channels.editor.field.github.repositorySpeakers.placeholder',
|
||||
)}
|
||||
onChange={(event) =>
|
||||
updateGroupOverride(repositoryId, {
|
||||
allowedUsers: event.target.value,
|
||||
})
|
||||
}
|
||||
/>
|
||||
</FieldShell>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
const renderRetainedRepositoryOverrides = (policy: string) => {
|
||||
// Read the list back from the request this save builds, not from storage:
|
||||
// a repository typed during this visit is written by the save but absent
|
||||
// from the stored groups, and announcing only stored entries would let the
|
||||
// dialog report success without naming it.
|
||||
const retained = retainedRepositorySettings(descriptor, draft, instance);
|
||||
// Under open/pairing the runtime resolves per-repository senders and
|
||||
// requireMention without consulting groupPolicy, so an entry that can still
|
||||
// restrict someone keeps deciding who may drive the agent — but an
|
||||
// `allowedUsers` array beside an inherited `open` policy restricts nobody,
|
||||
// and naming it as "still applying" would be false. Under disabled the group
|
||||
// gate drops every envelope before either lookup, so every retained entry is
|
||||
// kept but inert. The policy is the caller's, so the copy cannot disagree
|
||||
// with the gate that decided to show this alert.
|
||||
const disabled = policy === 'disabled';
|
||||
const named = disabled
|
||||
? retained.map((entry) => entry.repositoryId)
|
||||
: retained.filter((entry) => entry.applies).map((e) => e.repositoryId);
|
||||
if (named.length === 0) return null;
|
||||
const variant = disabled ? '.disabled' : '';
|
||||
return (
|
||||
<Alert>
|
||||
<KeyRoundIcon />
|
||||
<AlertTitle>
|
||||
{t(
|
||||
`channels.editor.field.github.retainedRepositorySettings${variant}.title`,
|
||||
)}
|
||||
</AlertTitle>
|
||||
<AlertDescription>
|
||||
{t(
|
||||
`channels.editor.field.github.retainedRepositorySettings${variant}.description`,
|
||||
{ repositories: named.join(', ') },
|
||||
)}
|
||||
</AlertDescription>
|
||||
</Alert>
|
||||
);
|
||||
};
|
||||
|
||||
return (
|
||||
<Dialog
|
||||
open={open}
|
||||
@@ -1163,6 +1411,12 @@ export function ChannelEditorDialog({
|
||||
/>
|
||||
</FieldShell>
|
||||
) : null}
|
||||
{repositoryOnly && effectiveGroupPolicy === 'allowlist'
|
||||
? renderRepositoryOverrides()
|
||||
: null}
|
||||
{repositoryOnly && effectiveGroupPolicy !== 'allowlist'
|
||||
? renderRetainedRepositoryOverrides(effectiveGroupPolicy)
|
||||
: null}
|
||||
{showGroupSenders ? (
|
||||
<FieldShell
|
||||
id={`${formId}-groupSenders`}
|
||||
|
||||
File diff suppressed because it is too large.
Load diff
@@ -35,6 +35,17 @@ export interface ChannelEditorDraft {
|
||||
allowedGroupIds: string;
|
||||
groupSenders: ChannelGroupSenders;
|
||||
groupAllowedUsers: string;
|
||||
groupOverrides: Record<string, ChannelGroupOverrideDraft>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-repository settings. `''` means the key is absent, so the repository
|
||||
* inherits from `groups["*"]` and then from the built-in default.
|
||||
*/
|
||||
export interface ChannelGroupOverrideDraft {
|
||||
requireMention: '' | 'true' | 'false';
|
||||
senders: '' | 'open' | 'allowlist';
|
||||
allowedUsers: string;
|
||||
}
|
||||
|
||||
export type ChannelEditorValidationCode =
|
||||
@@ -46,6 +57,7 @@ export type ChannelEditorValidationCode =
|
||||
| 'invalidOption'
|
||||
| 'invalidRepoId'
|
||||
| 'conflictingRepoAliases'
|
||||
| 'invalidGroupEntry'
|
||||
| 'number'
|
||||
| 'outOfRange'
|
||||
| 'policy';
|
||||
@@ -65,6 +77,17 @@ export const PRIVATE_ONLY_FIELD_KEYS = new Set([
|
||||
|
||||
const REPOSITORY_FULL_NAME_RE = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/;
|
||||
|
||||
/**
|
||||
* Canonical repository IDs the allowlist currently names, in typing order.
|
||||
* Entries that are not `owner/repo` are left out: the save path rejects them,
|
||||
* and a row for a half-typed repository would only flicker.
|
||||
*/
|
||||
export function repositoryAllowlistIds(value: string): string[] {
|
||||
return [...new Set(toList(value.toLowerCase()))].filter((id) =>
|
||||
REPOSITORY_FULL_NAME_RE.test(id),
|
||||
);
|
||||
}
|
||||
|
||||
export function toList(value: string): string[] {
|
||||
return value
|
||||
.split(',')
|
||||
@@ -188,6 +211,55 @@ function initialFieldValue(
|
||||
return typeof value === 'string' ? value : '';
|
||||
}
|
||||
|
||||
/**
|
||||
* Read one repository's overrides. Shared by the draft seed and the save path:
|
||||
* the save compares against this same reading, so a field the operator never
|
||||
* touched keeps its stored key verbatim — including shapes the form cannot
|
||||
* express, such as an empty allowlist or a list with no policy beside it.
|
||||
*/
|
||||
function readGroupOverride(
|
||||
group: Record<string, unknown>,
|
||||
): ChannelGroupOverrideDraft {
|
||||
const requireMention = group['requireMention'];
|
||||
const senders = group['senders'];
|
||||
const users = group['allowedUsers'];
|
||||
return {
|
||||
requireMention:
|
||||
typeof requireMention === 'boolean'
|
||||
? requireMention
|
||||
? 'true'
|
||||
: 'false'
|
||||
: '',
|
||||
senders: senders === 'open' || senders === 'allowlist' ? senders : '',
|
||||
allowedUsers: Array.isArray(users)
|
||||
? users
|
||||
.filter((user): user is string => typeof user === 'string')
|
||||
.join(', ')
|
||||
: '',
|
||||
};
|
||||
}
|
||||
|
||||
function configuredGroupOverrides(
|
||||
instance?: DaemonChannelInstanceSnapshot,
|
||||
): Record<string, ChannelGroupOverrideDraft> {
|
||||
const groups = instance?.config['groups'];
|
||||
if (!isRecord(groups)) return {};
|
||||
// fromEntries defines own properties, so a hand-written `__proto__` group key
|
||||
// in settings.json becomes an ordinary entry instead of reassigning the
|
||||
// prototype of the map every row is looked up in.
|
||||
return Object.fromEntries(
|
||||
Object.entries(groups)
|
||||
.filter(
|
||||
(entry): entry is [string, Record<string, unknown>] =>
|
||||
entry[0] !== '*' && isRecord(entry[1]),
|
||||
)
|
||||
.map(([groupId, group]) => [
|
||||
groupId.toLowerCase(),
|
||||
readGroupOverride(group),
|
||||
]),
|
||||
);
|
||||
}
|
||||
|
||||
export function createChannelEditorDraft(
|
||||
descriptor: DaemonChannelTypeDescriptor,
|
||||
instance?: DaemonChannelInstanceSnapshot,
|
||||
@@ -236,6 +308,7 @@ export function createChannelEditorDraft(
|
||||
: configuredGroups,
|
||||
groupSenders: configuredGroupSenders(instance),
|
||||
groupAllowedUsers: configuredGroupAllowedUsers(instance),
|
||||
groupOverrides: repositoryOnly ? configuredGroupOverrides(instance) : {},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -291,11 +364,21 @@ export function validateChannelEditorDraft(
|
||||
draftValue.trim() !== ''
|
||||
) {
|
||||
const parsed = Number(draftValue);
|
||||
// A stored value the runtime already tolerates must not block an
|
||||
// unrelated edit. The editor re-sends it verbatim and the store applies
|
||||
// the same unchanged-value tolerance, so rejecting it here would strand
|
||||
// the operator on a field they never touched.
|
||||
const unchanged =
|
||||
instance !== undefined &&
|
||||
typeof instance.config[field.key] === 'number' &&
|
||||
instance.config[field.key] === parsed;
|
||||
if (!Number.isFinite(parsed)) {
|
||||
errors[field.key] = 'number';
|
||||
} else if (
|
||||
field.exclusiveMinimum !== undefined &&
|
||||
parsed <= field.exclusiveMinimum
|
||||
!unchanged &&
|
||||
((field.exclusiveMinimum !== undefined &&
|
||||
parsed <= field.exclusiveMinimum) ||
|
||||
(field.maximum !== undefined && parsed > field.maximum))
|
||||
) {
|
||||
errors[field.key] = 'outOfRange';
|
||||
}
|
||||
@@ -353,9 +436,61 @@ export function validateChannelEditorDraft(
|
||||
errors['allowedGroupIds'] = 'conflictingRepoAliases';
|
||||
}
|
||||
}
|
||||
if (
|
||||
isRepositoryOnlyChannelType(descriptor.type) &&
|
||||
String(draft.values['groupPolicy'] ?? '') !== 'allowlist'
|
||||
) {
|
||||
// Away from Allowlist the rows are hidden and the save path leaves an
|
||||
// ambiguous alias exactly as stored, so an edit made before switching would
|
||||
// be discarded while the dialog reports success and closes. Refuse it on the
|
||||
// policy field, which stays visible under every policy. An untouched switch
|
||||
// still saves: preserving both spellings is what the runtime resolved before.
|
||||
const groups = instance?.config['groups'];
|
||||
if (isRecord(groups)) {
|
||||
const initial = configuredGroupOverrides(instance);
|
||||
for (const id of conflictingRepositoryAliases(groups)) {
|
||||
const before = initial[id];
|
||||
const after = draft.groupOverrides[id];
|
||||
if (!before || !after) continue;
|
||||
if (
|
||||
before.requireMention !== after.requireMention ||
|
||||
before.senders !== after.senders ||
|
||||
before.allowedUsers !== after.allowedUsers
|
||||
) {
|
||||
errors['groupPolicy'] = 'conflictingRepoAliases';
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (isRepositoryOnlyChannelType(descriptor.type)) {
|
||||
if (invalidRepositoryGroupEntries(instance).length > 0) {
|
||||
// The store refuses a non-record group entry on every save, unchanged or
|
||||
// not, and no row in the form can represent it. Deleting it on the way
|
||||
// past would remove the exact key the runtime matches first and hand the
|
||||
// repository to a case-insensitive fallback, so refuse here with the
|
||||
// entry named instead of letting the daemon answer with a generic error.
|
||||
errors['groupPolicy'] = 'invalidGroupEntry';
|
||||
}
|
||||
}
|
||||
return errors;
|
||||
}
|
||||
|
||||
/**
|
||||
* Stored repository entries whose value is not an object. The form has no row
|
||||
* for them and the store rejects them on every save, so they have to be named
|
||||
* rather than silently dropped or passed through to a generic refusal.
|
||||
*/
|
||||
export function invalidRepositoryGroupEntries(
|
||||
instance?: DaemonChannelInstanceSnapshot,
|
||||
): string[] {
|
||||
const groups = instance?.config['groups'];
|
||||
if (!isRecord(groups)) return [];
|
||||
return Object.entries(groups)
|
||||
.filter(([groupId, group]) => groupId !== '*' && !isRecord(group))
|
||||
.map(([groupId]) => groupId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the stored allowlist holds two spellings of one repository that
|
||||
* carry different settings. GitHub treats them as the same repository, but
|
||||
@@ -366,6 +501,18 @@ function hasConflictingRepositoryAliases(
|
||||
): boolean {
|
||||
const groups = instance?.config['groups'];
|
||||
if (!isRecord(groups)) return false;
|
||||
return conflictingRepositoryAliases(groups).size > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* The lower-cased ids whose stored spellings disagree about settings. The draft
|
||||
* holds one entry per id, so it cannot say which spelling an edit was meant
|
||||
* for. Validation and the non-Allowlist save path consult this same set: the
|
||||
* first refuses the save, the second leaves both spellings alone.
|
||||
*/
|
||||
function conflictingRepositoryAliases(
|
||||
groups: Record<string, unknown>,
|
||||
): Set<string> {
|
||||
const shapes = new Map<string, Set<string>>();
|
||||
for (const [groupId, group] of Object.entries(groups)) {
|
||||
if (groupId === '*') continue;
|
||||
@@ -373,10 +520,9 @@ function hasConflictingRepositoryAliases(
|
||||
const shape = canonicalGroupShape(group);
|
||||
shapes.set(key, (shapes.get(key) ?? new Set<string>()).add(shape));
|
||||
}
|
||||
for (const variants of shapes.values()) {
|
||||
if (variants.size > 1) return true;
|
||||
}
|
||||
return false;
|
||||
return new Set(
|
||||
[...shapes].filter(([, variants]) => variants.size > 1).map(([key]) => key),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -460,11 +606,64 @@ function splitList(value: string): string[] {
|
||||
return [...new Set(toList(value))];
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply one repository's draft over its stored settings. Only a field that
|
||||
* differs from the stored reading is written, so an untouched repository keeps
|
||||
* every key it had — including ones this form does not model.
|
||||
*
|
||||
* Exported because the row's helper text must describe what this projection
|
||||
* writes, not what the draft happens to hold: deriving the copy from the draft
|
||||
* policy alone tells two deliberately-preserved stored shapes the opposite of
|
||||
* what the runtime does.
|
||||
*/
|
||||
export function projectRepositoryGroup(
|
||||
stored: Record<string, unknown>,
|
||||
override: ChannelGroupOverrideDraft,
|
||||
): Record<string, unknown> {
|
||||
const initial = readGroupOverride(stored);
|
||||
const group = { ...stored };
|
||||
if (override.requireMention !== initial.requireMention) {
|
||||
if (override.requireMention === '') delete group['requireMention'];
|
||||
else group['requireMention'] = override.requireMention === 'true';
|
||||
}
|
||||
const policyChanged = override.senders !== initial.senders;
|
||||
if (policyChanged) {
|
||||
if (override.senders === '') delete group['senders'];
|
||||
else group['senders'] = override.senders;
|
||||
}
|
||||
const users = splitList(override.allowedUsers);
|
||||
if (override.allowedUsers !== initial.allowedUsers) {
|
||||
// An explicit allowlist owns its list: empty means nobody, not "inherit".
|
||||
if (users.length > 0 || group['senders'] === 'allowlist') {
|
||||
group['allowedUsers'] = users;
|
||||
} else {
|
||||
delete group['allowedUsers'];
|
||||
}
|
||||
} else if (policyChanged) {
|
||||
// The policy moved while the text did not — the default state both of a
|
||||
// freshly added row and of a row being returned to inheritance. Both
|
||||
// directions have to write, or the row's copy describes something the save
|
||||
// did not do. Scoped to a changed policy: a row that already stored the
|
||||
// policy keeps its members, so an untouched save stays a no-op.
|
||||
if (group['senders'] === 'allowlist') {
|
||||
// Without the key the runtime's `??` hands the channel-wide list back,
|
||||
// so "empty means nobody" would silently keep the wider audience.
|
||||
if (!Array.isArray(group['allowedUsers'])) group['allowedUsers'] = [];
|
||||
} else if (users.length === 0) {
|
||||
// `[]` is not nullish, so a list left behind keeps denying everyone
|
||||
// while the row reads "Inherit" and promises the channel-wide list.
|
||||
delete group['allowedUsers'];
|
||||
}
|
||||
}
|
||||
return group;
|
||||
}
|
||||
|
||||
function assignGroups(
|
||||
config: Record<string, unknown>,
|
||||
allowedGroupIds: string,
|
||||
instance?: DaemonChannelInstanceSnapshot,
|
||||
normalizeKeys = false,
|
||||
overrides?: Record<string, ChannelGroupOverrideDraft>,
|
||||
): void {
|
||||
const previous = instance?.config['groups'];
|
||||
const storedGroups = isRecord(previous) ? previous : {};
|
||||
@@ -486,7 +685,9 @@ function assignGroups(
|
||||
for (const groupId of splitList(allowedGroupIds)) {
|
||||
const key = normalizeKeys ? groupId.toLowerCase() : groupId;
|
||||
if (key === '*') continue;
|
||||
groups[key] = isRecord(previousGroups[key]) ? previousGroups[key] : {};
|
||||
const stored = isRecord(previousGroups[key]) ? previousGroups[key] : {};
|
||||
const override = overrides?.[key];
|
||||
groups[key] = override ? projectRepositoryGroup(stored, override) : stored;
|
||||
}
|
||||
if (Object.keys(groups).length > 0) {
|
||||
config['groups'] = groups;
|
||||
@@ -513,16 +714,81 @@ function assignGroupSenders(
|
||||
|
||||
function removeGroupAllowlistMembership(
|
||||
config: Record<string, unknown>,
|
||||
instance: DaemonChannelInstanceSnapshot,
|
||||
instance: DaemonChannelInstanceSnapshot | undefined,
|
||||
overrides?: Record<string, ChannelGroupOverrideDraft>,
|
||||
allowedGroupIds = '',
|
||||
): void {
|
||||
const previous = instance.config['groups'];
|
||||
const previous = instance?.config['groups'];
|
||||
const previousGroups = isRecord(previous) ? previous : {};
|
||||
// The rows are hidden once the policy leaves Allowlist, but the draft still
|
||||
// holds whatever the operator edited before switching, and the runtime keeps
|
||||
// resolving these entries under open/pairing. Project the edits here too, or
|
||||
// the save reports success while silently restoring the stored value.
|
||||
const entries = new Map<string, unknown>(Object.entries(previousGroups));
|
||||
if (overrides) {
|
||||
// A repository typed while the policy was Allowlist has a draft but no
|
||||
// stored entry. Iterating storage alone drops it, and the repository falls
|
||||
// back to the channel-wide policy it was just narrowed against. Match on
|
||||
// the canonical id so a mixed-case stored spelling is not duplicated.
|
||||
const storedIds = new Set(
|
||||
[...entries.keys()].map((id) => id.toLowerCase()),
|
||||
);
|
||||
for (const groupId of repositoryAllowlistIds(allowedGroupIds)) {
|
||||
if (!storedIds.has(groupId)) entries.set(groupId, {});
|
||||
}
|
||||
}
|
||||
// One lower-cased draft cannot speak for two spellings that disagree:
|
||||
// projecting it onto both hands the last alias's settings to the first and
|
||||
// widens a repository nobody edited. Leave those entries as stored — the
|
||||
// Allowlist path already refuses the save until the operator resolves them.
|
||||
const ambiguous = conflictingRepositoryAliases(previousGroups);
|
||||
// A channel that never had an allowlist is not leaving one, so its stored
|
||||
// entries are not membership markers this save is entitled to clean up.
|
||||
const leavingAllowlist = instance?.config['groupPolicy'] === 'allowlist';
|
||||
const groups = Object.fromEntries(
|
||||
Object.entries(previousGroups).filter(
|
||||
([groupId, groupConfig]) =>
|
||||
isRecord(groupConfig) &&
|
||||
(groupId === '*' || Object.keys(groupConfig).length > 0),
|
||||
),
|
||||
[...entries]
|
||||
.map(([groupId, stored]) => {
|
||||
const ambiguousAlias =
|
||||
groupId !== '*' && ambiguous.has(groupId.toLowerCase());
|
||||
const override =
|
||||
groupId === '*' || ambiguousAlias
|
||||
? undefined
|
||||
: overrides?.[groupId.toLowerCase()];
|
||||
const projected =
|
||||
override && isRecord(stored)
|
||||
? projectRepositoryGroup(stored, override)
|
||||
: stored;
|
||||
return { groupId, projected, ambiguousAlias };
|
||||
})
|
||||
.filter(({ groupId, projected, ambiguousAlias }) => {
|
||||
// An ambiguous alias is kept verbatim: one lower-cased draft cannot say
|
||||
// which spelling an edit was meant for, and an empty entry is what makes
|
||||
// the runtime's exact match inherit "*" instead of falling through to a
|
||||
// wider alias.
|
||||
if (ambiguousAlias) return true;
|
||||
// "Stored" has to mean present in the file, not present in the merged
|
||||
// map: a repository typed during this visit is seeded with a `{}`
|
||||
// placeholder that must not be mistaken for a stored empty entry.
|
||||
const wasStored = Object.hasOwn(previousGroups, groupId);
|
||||
const stored = wasStored ? previousGroups[groupId] : undefined;
|
||||
// An entry the form cannot represent is never deleted here. A non-record
|
||||
// value is refused by validation with a message that names it, because
|
||||
// the store rejects it on every save; deleting it would remove the exact
|
||||
// key the runtime matches first, a rewrite nobody asked for whether or
|
||||
// not another spelling exists to fall through to.
|
||||
if (groupId !== '*' && wasStored && !isRecord(stored)) return true;
|
||||
if (!isRecord(projected)) return false;
|
||||
if (groupId === '*' || Object.keys(projected).length > 0) return true;
|
||||
// Drop an empty entry when it is the stub a row leaves behind on being
|
||||
// returned to inherit, or when the channel is leaving the allowlist
|
||||
// whose membership the entry expressed. Keep one that was already
|
||||
// empty in a channel that never had an allowlist: an unrelated edit
|
||||
// must not silently rewrite the stored file.
|
||||
const wasStoredEmpty =
|
||||
isRecord(stored) && Object.keys(stored).length === 0;
|
||||
return wasStoredEmpty && !leavingAllowlist;
|
||||
})
|
||||
.map(({ groupId, projected }) => [groupId, projected]),
|
||||
);
|
||||
if (Object.keys(groups).length > 0) {
|
||||
config['groups'] = groups;
|
||||
@@ -569,11 +835,78 @@ export function buildChannelUpsertRequest(
|
||||
}
|
||||
if (hasDescriptorGroupPolicy(descriptor)) {
|
||||
if (config['groupPolicy'] === 'allowlist') {
|
||||
assignGroups(config, draft.allowedGroupIds, instance, repositoryOnly);
|
||||
} else if (instance?.config['groupPolicy'] === 'allowlist') {
|
||||
removeGroupAllowlistMembership(config, instance);
|
||||
assignGroups(
|
||||
config,
|
||||
draft.allowedGroupIds,
|
||||
instance,
|
||||
repositoryOnly,
|
||||
repositoryOnly ? draft.groupOverrides : undefined,
|
||||
);
|
||||
} else if (
|
||||
repositoryOnly ||
|
||||
instance?.config['groupPolicy'] === 'allowlist'
|
||||
) {
|
||||
// Repository rows are reachable from any policy: the operator can switch
|
||||
// to Allowlist, narrow a repository, and switch back before saving. Gating
|
||||
// this on the STORED policy would drop those edits on a channel that was
|
||||
// already Open, while the alert promises they are kept.
|
||||
removeGroupAllowlistMembership(
|
||||
config,
|
||||
instance,
|
||||
repositoryOnly ? draft.groupOverrides : undefined,
|
||||
draft.allowedGroupIds,
|
||||
);
|
||||
}
|
||||
assignGroupSenders(config, draft);
|
||||
}
|
||||
return { expectedRevision, config, secrets };
|
||||
}
|
||||
|
||||
export interface RetainedRepositorySetting {
|
||||
repositoryId: string;
|
||||
/** Whether the entry can still change who is admitted or what is processed. */
|
||||
applies: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* The repositories whose settings this save keeps, read back from the request
|
||||
* the save actually builds rather than from storage: a repository the operator
|
||||
* typed during this visit exists only in the draft, and enumerating stored
|
||||
* groups would let the dialog report success without naming it.
|
||||
*
|
||||
* `applies` follows the runtime, not the shape of the entry. `senderGateFor`
|
||||
* builds an allowlist only when the resolved senders policy is `allowlist`, so
|
||||
* an `allowedUsers` array beside an inherited `open` policy restricts nobody;
|
||||
* and a `requireMention` equal to the inherited value changes nothing either.
|
||||
*/
|
||||
export function retainedRepositorySettings(
|
||||
descriptor: DaemonChannelTypeDescriptor,
|
||||
draft: ChannelEditorDraft,
|
||||
instance?: DaemonChannelInstanceSnapshot,
|
||||
): RetainedRepositorySetting[] {
|
||||
const { config } = buildChannelUpsertRequest(descriptor, draft, '', instance);
|
||||
const groups = isRecord(config['groups']) ? config['groups'] : {};
|
||||
const wildcard = isRecord(groups['*']) ? groups['*'] : {};
|
||||
const inheritedSenders =
|
||||
wildcard['senders'] === 'allowlist' ? 'allowlist' : 'open';
|
||||
const inheritedMention =
|
||||
typeof wildcard['requireMention'] === 'boolean'
|
||||
? wildcard['requireMention']
|
||||
: true;
|
||||
return Object.entries(groups)
|
||||
.filter(
|
||||
([groupId, group]) =>
|
||||
groupId !== '*' && isRecord(group) && Object.keys(group).length > 0,
|
||||
)
|
||||
.map(([groupId, group]) => {
|
||||
const entry = group as Record<string, unknown>;
|
||||
const senders = entry['senders'] ?? inheritedSenders;
|
||||
const mention = entry['requireMention'];
|
||||
return {
|
||||
repositoryId: groupId,
|
||||
applies:
|
||||
senders === 'allowlist' ||
|
||||
(typeof mention === 'boolean' && mention !== inheritedMention),
|
||||
};
|
||||
});
|
||||
}
|
||||
@@ -1532,6 +1532,164 @@ for (const theme of THEMES) {
|
||||
]);
|
||||
});
|
||||
|
||||
test(`GitHub channel repository settings`, async ({ page }, testInfo) => {
|
||||
const scenario = createWebShellDaemonScenario({
|
||||
capabilities: {
|
||||
features: [
|
||||
'session_events',
|
||||
'permission_vote',
|
||||
'session_permission_vote',
|
||||
'session_scope_override',
|
||||
'session_source_metadata',
|
||||
'workspace_settings',
|
||||
'workspace_voice',
|
||||
'channel_management',
|
||||
],
|
||||
},
|
||||
channelTypes: [
|
||||
{
|
||||
type: 'github',
|
||||
displayName: 'GitHub',
|
||||
manageable: true,
|
||||
fields: [
|
||||
{
|
||||
key: 'token',
|
||||
label: 'Personal Access Token',
|
||||
kind: 'secret',
|
||||
envResolvable: true,
|
||||
},
|
||||
{
|
||||
key: 'useLocalGh',
|
||||
label: 'Use Local GitHub CLI Authentication',
|
||||
kind: 'boolean',
|
||||
},
|
||||
{
|
||||
key: 'baseUrl',
|
||||
label: 'Base URL',
|
||||
kind: 'string',
|
||||
envResolvable: true,
|
||||
},
|
||||
{
|
||||
key: 'pollInterval',
|
||||
label: 'Poll Interval (ms)',
|
||||
kind: 'number',
|
||||
exclusiveMinimum: 0,
|
||||
default: '60000',
|
||||
},
|
||||
{
|
||||
key: 'groupPolicy',
|
||||
label: 'Group Policy',
|
||||
kind: 'enum',
|
||||
required: true,
|
||||
default: 'open',
|
||||
options: [
|
||||
{ value: 'open', label: 'Open' },
|
||||
{ value: 'allowlist', label: 'Allowlist' },
|
||||
{ value: 'disabled', label: 'Disabled' },
|
||||
],
|
||||
},
|
||||
{
|
||||
key: 'privatePolicy',
|
||||
label: 'Private Policy',
|
||||
kind: 'enum',
|
||||
required: true,
|
||||
options: [
|
||||
{ value: 'disabled', label: 'Disabled' },
|
||||
{ value: 'allowlist', label: 'Allowlist' },
|
||||
{ value: 'pairing', label: 'Pairing' },
|
||||
{ value: 'open', label: 'Open' },
|
||||
],
|
||||
},
|
||||
{
|
||||
key: 'allowedUsers',
|
||||
label: 'Allowed Users',
|
||||
kind: 'string-list',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
channels: {
|
||||
revision: '1',
|
||||
instances: {
|
||||
'github-bot': {
|
||||
name: 'github-bot',
|
||||
config: {
|
||||
type: 'github',
|
||||
useLocalGh: true,
|
||||
pollInterval: 30000,
|
||||
groupPolicy: 'allowlist',
|
||||
groups: {
|
||||
'*': { senders: 'allowlist', allowedUsers: ['operator'] },
|
||||
'qwenlm/qwen-code': {
|
||||
requireMention: false,
|
||||
senders: 'allowlist',
|
||||
allowedUsers: ['alice', 'bob'],
|
||||
},
|
||||
'acme/widgets': {},
|
||||
},
|
||||
},
|
||||
secrets: {},
|
||||
startsWithServe: false,
|
||||
runtime: { state: 'stopped' },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
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: 'Edit github-bot' }).click();
|
||||
await expect(
|
||||
page.getByRole('heading', { name: 'Edit GitHub' }),
|
||||
).toBeVisible();
|
||||
|
||||
// Anchor the capture to the rows themselves: a regression that drops them
|
||||
// must fail here rather than republish a picture of the old form.
|
||||
await expect(page.getByText('Per-repository settings')).toBeVisible();
|
||||
await expect(page.getByLabel('Require a mention')).toHaveCount(2);
|
||||
await expect(page.getByLabel('Who may speak')).toHaveCount(2);
|
||||
await expect(page.getByLabel('Allowed speakers')).toHaveCount(2);
|
||||
// Scope every assertion to its own row: counting controls alone would
|
||||
// still pass if the rows were wired to the wrong repository, or if the
|
||||
// speaker boxes all showed the channel-wide list.
|
||||
const customized = page.getByRole('group', { name: 'qwenlm/qwen-code' });
|
||||
const inheriting = page.getByRole('group', { name: 'acme/widgets' });
|
||||
await expect(customized.getByLabel('Require a mention')).toContainText(
|
||||
'Answer without a mention',
|
||||
);
|
||||
await expect(customized.getByLabel('Who may speak')).toContainText(
|
||||
'Allowlist only',
|
||||
);
|
||||
await expect(customized.getByLabel('Allowed speakers')).toHaveValue(
|
||||
'alice, bob',
|
||||
);
|
||||
await expect(inheriting.getByLabel('Require a mention')).toContainText(
|
||||
'Inherit (require a mention)',
|
||||
);
|
||||
await expect(inheriting.getByLabel('Who may speak')).toContainText(
|
||||
'Inherit (allowlist only)',
|
||||
);
|
||||
await expect(inheriting.getByLabel('Allowed speakers')).toHaveValue('');
|
||||
await expect(page.getByLabel('Poll Interval (ms)')).toHaveValue('30000');
|
||||
|
||||
// Put the last row in frame, not just the section title: a capture can
|
||||
// otherwise show the heading with every row below the fold. Only the last
|
||||
// row is required to fit — at this viewport the two rows together are
|
||||
// taller than the dialog body, so demanding both would fail on geometry
|
||||
// rather than on a regression.
|
||||
await inheriting.scrollIntoViewIfNeeded();
|
||||
await expect(inheriting).toBeInViewport({ ratio: 1 });
|
||||
await captureScreenshot(page, `github-channel-repositories-${theme}`);
|
||||
});
|
||||
|
||||
test(`mermaid diagram`, async ({ page }, testInfo) => {
|
||||
const scenario = createWebShellDaemonScenario({
|
||||
events: [
|
||||
|
||||
@@ -4502,6 +4502,41 @@ const EN: Messages = {
|
||||
'channels.editor.field.github.baseUrl': 'API Base URL',
|
||||
'channels.editor.field.github.baseUrl.description':
|
||||
'GitHub Enterprise API root (e.g. https://ghe.example.com/api/v3). Leave empty for github.com',
|
||||
'channels.editor.field.github.pollInterval': 'Poll Interval (ms)',
|
||||
'channels.editor.field.github.pollInterval.description':
|
||||
'How often to check GitHub for new notifications. Leave empty for 60000 (one minute).',
|
||||
'channels.editor.field.github.repositorySettings': 'Per-repository settings',
|
||||
'channels.editor.field.github.repositorySettings.description':
|
||||
'Each allowlisted repository inherits the channel-wide settings below until you override it here.',
|
||||
'channels.editor.field.github.requireMention': 'Require a mention',
|
||||
'channels.editor.field.github.requireMention.inheritOn':
|
||||
'Inherit (require a mention)',
|
||||
'channels.editor.field.github.requireMention.inheritOff':
|
||||
'Inherit (answer without a mention)',
|
||||
'channels.editor.field.github.requireMention.on': 'Require a mention',
|
||||
'channels.editor.field.github.requireMention.off': 'Answer without a mention',
|
||||
'channels.editor.field.github.repositorySenders': 'Who may speak',
|
||||
'channels.editor.field.github.repositorySenders.inheritOpen':
|
||||
'Inherit (anyone)',
|
||||
'channels.editor.field.github.repositorySenders.inheritAllowlist':
|
||||
'Inherit (allowlist only)',
|
||||
'channels.editor.field.github.repositorySenders.open': 'Anyone',
|
||||
'channels.editor.field.github.repositorySenders.allowlist': 'Allowlist only',
|
||||
'channels.editor.field.github.repositorySpeakers': 'Allowed speakers',
|
||||
'channels.editor.field.github.repositorySpeakers.placeholder': 'alice, bob',
|
||||
'channels.editor.field.github.repositorySpeakers.description':
|
||||
'GitHub logins allowed to start tasks in this repository. Empty means nobody.',
|
||||
'channels.editor.field.github.repositorySpeakers.inherited':
|
||||
'GitHub logins allowed in this repository. Leave empty to use the channel-wide list.',
|
||||
'channels.editor.field.github.retainedRepositorySettings.title':
|
||||
'Repository settings still apply',
|
||||
'channels.editor.field.github.retainedRepositorySettings.description': (v) =>
|
||||
`These repositories keep their own speaker and mention settings even though the group policy no longer uses an allowlist: ${v?.repositories ?? ''}. Switch the group policy back to Allowlist to edit them.`,
|
||||
'channels.editor.field.github.retainedRepositorySettings.disabled.title':
|
||||
'Repository settings are kept but not applied',
|
||||
'channels.editor.field.github.retainedRepositorySettings.disabled.description':
|
||||
(v) =>
|
||||
`The group policy is Disabled, so no repository traffic is processed. These repositories keep their own speaker and mention settings, and they apply again once the policy admits group traffic: ${v?.repositories ?? ''}. Switch the group policy back to Allowlist to edit them.`,
|
||||
'channels.editor.field.github.groupPolicy': 'Group Policy',
|
||||
'channels.editor.field.github.groupPolicy.description':
|
||||
'Must be "Open", "Allowlist", or "Pairing" for notifications to flow',
|
||||
@@ -4732,11 +4767,17 @@ const EN: Messages = {
|
||||
'Enter each repository as owner/repo.',
|
||||
'channels.editor.validation.conflictingRepoAliases':
|
||||
'This channel lists one repository twice with different settings, spelled differently (for example Owner/repo and owner/repo). Remove one of the entries from settings.json before saving.',
|
||||
'channels.editor.validation.invalidGroupEntry': (v) =>
|
||||
`This channel stores a group entry that is not an object: ${v?.repositories ?? ''}. The daemon refuses every save while it is there, and the form has no row for it. Fix or remove that entry in settings.json, then reload the channel.`,
|
||||
'channels.editor.validation.invalidOption':
|
||||
"Remove values that aren't in the allowed list.",
|
||||
'channels.editor.validation.number': 'Enter a valid number.',
|
||||
'channels.editor.validation.outOfRange': (v) =>
|
||||
`Enter a number greater than ${v?.min ?? 0}.`,
|
||||
v?.min !== undefined && v?.max !== undefined
|
||||
? `Enter a number greater than ${v.min} and at most ${v.max}.`
|
||||
: v?.max !== undefined
|
||||
? `Enter a number at most ${v.max}.`
|
||||
: `Enter a number greater than ${v?.min ?? 0}.`,
|
||||
'channels.editor.validation.policy': 'Choose an access policy.',
|
||||
'channels.editor.saveError': 'Changes were not saved',
|
||||
'channels.editor.reloadLatest': 'Reload latest',
|
||||
@@ -8944,6 +8985,40 @@ const ZH: Messages = {
|
||||
'channels.editor.field.github.baseUrl': 'API 基础 URL',
|
||||
'channels.editor.field.github.baseUrl.description':
|
||||
'GitHub Enterprise API 根地址(如 https://ghe.example.com/api/v3),github.com 留空',
|
||||
'channels.editor.field.github.pollInterval': '轮询间隔(毫秒)',
|
||||
'channels.editor.field.github.pollInterval.description':
|
||||
'多久向 GitHub 查一次新通知。留空为 60000(一分钟)。',
|
||||
'channels.editor.field.github.repositorySettings': '按仓库设置',
|
||||
'channels.editor.field.github.repositorySettings.description':
|
||||
'每个已允许的仓库都继承下面的渠道级设置,直到你在这里覆盖它。',
|
||||
'channels.editor.field.github.requireMention': '需要 @',
|
||||
'channels.editor.field.github.requireMention.inheritOn': '继承(需要 @)',
|
||||
'channels.editor.field.github.requireMention.inheritOff':
|
||||
'继承(无需 @ 也响应)',
|
||||
'channels.editor.field.github.requireMention.on': '需要 @',
|
||||
'channels.editor.field.github.requireMention.off': '无需 @ 也响应',
|
||||
'channels.editor.field.github.repositorySenders': '谁可以发言',
|
||||
'channels.editor.field.github.repositorySenders.inheritOpen':
|
||||
'继承(任何人)',
|
||||
'channels.editor.field.github.repositorySenders.inheritAllowlist':
|
||||
'继承(仅允许列表)',
|
||||
'channels.editor.field.github.repositorySenders.open': '任何人',
|
||||
'channels.editor.field.github.repositorySenders.allowlist': '仅允许列表',
|
||||
'channels.editor.field.github.repositorySpeakers': '允许的发言人',
|
||||
'channels.editor.field.github.repositorySpeakers.placeholder': 'alice, bob',
|
||||
'channels.editor.field.github.repositorySpeakers.description':
|
||||
'可以在该仓库发起任务的 GitHub 登录名。留空表示没有人。',
|
||||
'channels.editor.field.github.repositorySpeakers.inherited':
|
||||
'可以在该仓库发言的 GitHub 登录名。留空则使用渠道级列表。',
|
||||
'channels.editor.field.github.retainedRepositorySettings.title':
|
||||
'按仓库设置仍然生效',
|
||||
'channels.editor.field.github.retainedRepositorySettings.description': (v) =>
|
||||
`虽然群组策略已不再使用白名单,这些仓库仍保留自己的发言人与 @ 设置:${v?.repositories ?? ''}。把群组策略切回「白名单」即可编辑它们。`,
|
||||
'channels.editor.field.github.retainedRepositorySettings.disabled.title':
|
||||
'按仓库设置已保留,但当前不生效',
|
||||
'channels.editor.field.github.retainedRepositorySettings.disabled.description':
|
||||
(v) =>
|
||||
`群组策略为「禁用」,因此不处理任何仓库流量。这些仓库保留自己的发言人与 @ 设置,待策略重新放行群组流量后再次生效:${v?.repositories ?? ''}。把群组策略切回「白名单」即可编辑它们。`,
|
||||
'channels.editor.field.github.groupPolicy': '群组策略',
|
||||
'channels.editor.field.github.groupPolicy.description':
|
||||
'必须设为 "Open"、"Allowlist" 或 "Pairing" 才能接收通知',
|
||||
@@ -9164,10 +9239,16 @@ const ZH: Messages = {
|
||||
'channels.editor.validation.invalidRepoId': '仓库需写成 owner/repo 形式。',
|
||||
'channels.editor.validation.conflictingRepoAliases':
|
||||
'该渠道把同一个仓库以不同大小写写了两次且设置不同(例如 Owner/repo 与 owner/repo)。请先在 settings.json 里删掉其中一条再保存。',
|
||||
'channels.editor.validation.invalidGroupEntry': (v) =>
|
||||
`该渠道把以下群组条目存成了非对象:${v?.repositories ?? ''}。只要它还在,daemon 就会拒绝每次保存,而表单里没有对应的行。请在 settings.json 里修正或删除该条目,然后重新加载渠道。`,
|
||||
'channels.editor.validation.invalidOption': '请移除不在允许列表中的值。',
|
||||
'channels.editor.validation.number': '请输入有效数字。',
|
||||
'channels.editor.validation.outOfRange': (v) =>
|
||||
`请输入大于 ${v?.min ?? 0} 的数字。`,
|
||||
v?.min !== undefined && v?.max !== undefined
|
||||
? `请输入大于 ${v.min} 且不超过 ${v.max} 的数字。`
|
||||
: v?.max !== undefined
|
||||
? `请输入不超过 ${v.max} 的数字。`
|
||||
: `请输入大于 ${v?.min ?? 0} 的数字。`,
|
||||
'channels.editor.validation.policy': '请选择准入策略。',
|
||||
'channels.editor.saveError': '未能保存更改',
|
||||
'channels.editor.reloadLatest': '加载最新配置',
|
||||
|
||||
Reference in new issue
Block a user