0 Fork 0

mirror of https://github.com/QwenLM/qwen-code.git synced 2026-11-07 06:19:43 -10:00

Files

T

Shaojin Wen bf2f003294 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.

2026-11-04 01:46:24 +08:00

32 KiB

Raw Permalink Blame History

GitHub Channel Repository Settings in the Web Shell Editor

English | 简体中文

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

Edit the speaker policy and allowlist of an individual allowlisted repository from the channel editor.
Edit requireMention for an individual allowlisted repository from the same place.
Edit the channel's poll interval from the same form.
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:

{
  "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:

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

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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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

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.
Each allowlisted repository shows a require-mention control and a speaker control whose default is "inherit".
Saving an untouched channel writes exactly the same groups object it read.
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.
Keys the editor does not model survive a save unchanged.
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.
Both locales render the new copy, and the build-artifact guard passes.
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.
Repositories whose settings outlive a switch away from the allowlist policy are named in the editor rather than becoming invisible while still enforced.
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.

32 KiB Raw Permalink Blame History