diff --git a/packages/web-shell/client/components/channels/ChannelEditorDialog.module.css b/packages/web-shell/client/components/channels/ChannelEditorDialog.module.css index 882128f7ea5..e9804616362 100644 --- a/packages/web-shell/client/components/channels/ChannelEditorDialog.module.css +++ b/packages/web-shell/client/components/channels/ChannelEditorDialog.module.css @@ -192,6 +192,21 @@ background: color-mix(in srgb, var(--muted) 62%, transparent); } +.optionRow[data-disabled='true'] { + cursor: not-allowed; + opacity: 0.55; +} + +/* A checked-but-disabled row keeps the selected border; only unselected + disabled rows lose the hover emphasis. */ +.optionRow[data-disabled='true']:not([data-selected='true']):hover { + border-color: var(--border); +} + +.optionCheckbox:disabled { + cursor: not-allowed; +} + .optionCheckbox { width: 14px; height: 14px; diff --git a/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx b/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx index 02c34e3356f..7dc536a9bac 100644 --- a/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx +++ b/packages/web-shell/client/components/channels/ChannelEditorDialog.test.tsx @@ -866,6 +866,540 @@ describe('ChannelEditorDialog', () => { ); }); + it('frames both bound-session fields in one panel', async () => { + await renderDialog({ descriptor: GITHUB_BOUND }); + + const delivery = fieldByLabel('Deliver To PR-Bound Sessions'); + const watch = fieldByLabel('PR Watch Events'); + expect(delivery).not.toBeNull(); + expect(watch).not.toBeNull(); + expect(delivery!.closest('section')).toBe(watch!.closest('section')); + expect(sectionHeadingOf(delivery)).toBe('PR-bound sessions'); + }); + + it('disables the watch boxes with an inline hint until delivery is on', async () => { + await renderDialog({ descriptor: GITHUB_BOUND }); + + const boxes = () => + Array.from( + document.querySelectorAll( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ), + ); + expect(boxes().length).toBeGreaterThan(0); + expect(boxes().every((box) => box.disabled)).toBe(true); + // The dimmed-row styling hook must track the gate, not just the input. + expect( + boxes().every( + (box) => box.closest('label')?.getAttribute('data-disabled') === 'true', + ), + ).toBe(true); + expect(document.body.textContent).toContain( + 'Enable "Deliver To PR-Bound Sessions" above to select watch events.', + ); + + const toggle = document.querySelector( + 'button[role="switch"]', + ); + expect(toggle).not.toBeNull(); + await act(async () => { + toggle!.click(); + }); + + expect(boxes().every((box) => box.disabled)).toBe(false); + expect(document.body.textContent).not.toContain( + 'above to select watch events.', + ); + }); + + it('shows the disabled hint in zh-CN', async () => { + await renderDialog({ descriptor: GITHUB_BOUND, language: 'zh-CN' }); + + expect(document.body.textContent).toContain( + '开启上方「投递到 PR 绑定会话」后即可选择监视事件。', + ); + }); + + it('saves the selected watch events once delivery is enabled', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + await renderDialog({ descriptor: GITHUB_BOUND, onSave }); + + await act(async () => { + setInputValue(inputByLabel('Instance name')!, 'github-bot'); + setInputValue(inputByLabel('Personal Access Token')!, 'ghp_test'); + document + .querySelector('button[role="switch"]')! + .click(); + }); + const box = Array.from( + document.querySelectorAll( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ), + ).find((b) => b.closest('label')?.textContent?.includes('ci_failure')); + expect(box).toBeDefined(); + await act(async () => { + box!.click(); + }); + + const save = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ); + await act(async () => { + save?.click(); + }); + + expect(onSave).toHaveBeenCalledWith( + 'github-bot', + expect.objectContaining({ + config: expect.objectContaining({ + deliverToBoundSessions: true, + prWatchEvents: ['ci_failure'], + }), + }), + ); + }); + + it('names the delivery dependency when a stored selection lacks it', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + prWatchEvents: ['ci_failure'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance, onSave }); + + const box = document.querySelector( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ); + expect(box!.checked).toBe(true); + // The stored selection stays clearable even while the gate is closed. + expect(box!.disabled).toBe(false); + + const save = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ); + await act(async () => { + save?.click(); + }); + + expect(onSave).not.toHaveBeenCalled(); + expect(document.body.textContent).toContain( + 'Enable "Deliver To PR-Bound Sessions" to save the selected watch events.', + ); + + // Following the instruction must retire the instruction: turning the + // switch on clears the stale dependency error without another Save. + await act(async () => { + document + .querySelector('button[role="switch"]')! + .click(); + }); + expect(document.body.textContent).not.toContain( + 'to save the selected watch events.', + ); + }); + + it('lets a stored selection be cleared while delivery is off', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + prWatchEvents: ['ci_failure'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance, onSave }); + + const boxes = Array.from( + document.querySelectorAll( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ), + ); + const selected = boxes.find((b) => + b.closest('label')?.textContent?.includes('ci_failure'), + )!; + expect(selected.checked).toBe(true); + expect(selected.disabled).toBe(false); + expect(boxes.filter((b) => b !== selected).every((b) => b.disabled)).toBe( + true, + ); + // A clearable stored row keeps the selected styling, not the dimmed one. + expect(selected.closest('label')?.getAttribute('data-disabled')).toBe( + 'false', + ); + expect( + boxes + .filter((b) => b !== selected) + .every( + (b) => b.closest('label')?.getAttribute('data-disabled') === 'true', + ), + ).toBe(true); + + await act(async () => { + selected.click(); + }); + const save = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ); + await act(async () => { + save?.click(); + }); + + expect(onSave).toHaveBeenCalledTimes(1); + const request = onSave.mock.calls[0][1] as { + config: Record; + }; + expect(request.config['prWatchEvents']).toBeUndefined(); + expect(request.config['deliverToBoundSessions']).toBe(false); + }); + + it('keeps the invalid-option message when delivery is switched on', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + prWatchEvents: ['bogus_event'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance }); + + const save = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ); + await act(async () => { + save?.click(); + }); + expect(document.body.textContent).toContain( + "Remove values that aren't in the allowed list.", + ); + + // Switching delivery on remedies the dependency, not the unknown value: + // the higher-precedence invalidOption message must survive. + await act(async () => { + document + .querySelector('button[role="switch"]')! + .click(); + }); + expect(document.body.textContent).toContain( + "Remove values that aren't in the allowed list.", + ); + expect(document.body.textContent).not.toContain( + 'to save the selected watch events.', + ); + }); + + it('clears the dependency message when the last event is unticked', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + prWatchEvents: ['ci_failure'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance }); + + const save = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ); + await act(async () => { + save?.click(); + }); + expect(document.body.textContent).toContain( + 'to save the selected watch events.', + ); + + // Unticking the last event is the other remedy; the alert must retire + // without another Save. + const box = Array.from( + document.querySelectorAll( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ), + ).find((b) => b.closest('label')?.textContent?.includes('ci_failure'))!; + await act(async () => { + box.click(); + }); + expect(document.body.textContent).not.toContain( + 'to save the selected watch events.', + ); + }); + + it('retires the invalid-option message when the offending value is unticked', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + prWatchEvents: ['bogus_event'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance }); + + await act(async () => { + ( + Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ) as HTMLButtonElement + ).click(); + }); + expect(document.body.textContent).toContain( + "Remove values that aren't in the allowed list.", + ); + + // Unticking the offending value is the remedy the message asks for; it + // must retire the message without a second Save. + const bogus = Array.from( + document.querySelectorAll( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ), + ).find((b) => b.closest('label')?.textContent?.includes('bogus_event'))!; + await act(async () => { + bogus.click(); + }); + expect(document.body.textContent).not.toContain( + "Remove values that aren't in the allowed list.", + ); + }); + + it('replaces the invalid-option message with the dependency one when a valid event remains', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + prWatchEvents: ['ci_failure', 'bogus_event'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance }); + + await act(async () => { + ( + Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ) as HTMLButtonElement + ).click(); + }); + expect(document.body.textContent).toContain( + "Remove values that aren't in the allowed list.", + ); + + const bogus = Array.from( + document.querySelectorAll( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ), + ).find((b) => b.closest('label')?.textContent?.includes('bogus_event'))!; + await act(async () => { + bogus.click(); + }); + // The remaining value is allowed, so the invalid-option instruction is + // gone; the still-unsatisfied dependency takes over the slot. + expect(document.body.textContent).not.toContain( + "Remove values that aren't in the allowed list.", + ); + expect(document.body.textContent).toContain( + 'to save the selected watch events.', + ); + }); + + it('does not surface validation before the first submit', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + prWatchEvents: ['ci_failure', 'merge_conflict'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance }); + expect(document.querySelectorAll('[role="alert"]').length).toBe(0); + + // A legitimate gated edit must not reveal validation state the operator + // never submitted; submit stays the only population site. + const box = Array.from( + document.querySelectorAll( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ), + ).find((b) => b.closest('label')?.textContent?.includes('merge_conflict'))!; + await act(async () => { + box.click(); + }); + expect(document.querySelectorAll('[role="alert"]').length).toBe(0); + }); + + it('keeps a shown invalid-option message when delivery is switched off', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + deliverToBoundSessions: true, + prWatchEvents: ['bogus_event'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance }); + const clickSave = async () => { + await act(async () => { + ( + Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ) as HTMLButtonElement + ).click(); + }); + }; + const clickSwitch = async () => { + await act(async () => { + document + .querySelector('button[role="switch"]')! + .click(); + }); + }; + + await clickSave(); + expect(document.body.textContent).toContain( + "Remove values that aren't in the allowed list.", + ); + // The mirror case: a shown instruction survives the switch moving off + // (re-derived, not silently dropped), in both directions. + await clickSwitch(); + expect(document.body.textContent).toContain( + "Remove values that aren't in the allowed list.", + ); + await clickSwitch(); + expect(document.body.textContent).toContain( + "Remove values that aren't in the allowed list.", + ); + }); + + it('renders no bound-session panel for channels without the fields', async () => { + await renderDialog({ descriptor: DINGTALK }); + expect(document.body.textContent).not.toContain('PR-bound sessions'); + }); + + it('renders no bound-session panel for channels without the fields (zh-CN)', async () => { + await renderDialog({ descriptor: DINGTALK, language: 'zh-CN' }); + expect(document.body.textContent).not.toContain('PR 绑定会话'); + }); + + it('scrolls a non-advanced watch-events error into view on save', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + prWatchEvents: ['ci_failure'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + // jsdom has no scrollIntoView; stub it so the scroll is observable. + const scrollIntoView = vi.fn(); + const originalScrollIntoView = Element.prototype.scrollIntoView; + Element.prototype.scrollIntoView = scrollIntoView; + try { + await renderDialog({ descriptor: GITHUB_BOUND, instance }); + await act(async () => { + ( + Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Save', + ) as HTMLButtonElement + ).click(); + }); + expect(scrollIntoView).toHaveBeenCalledWith({ block: 'center' }); + const target = scrollIntoView.mock.instances[0] as HTMLElement; + expect(target.id.endsWith('-prWatchEvents')).toBe(true); + } finally { + Element.prototype.scrollIntoView = originalScrollIntoView; + } + }); + + it('associates the disabled hint with the checkbox group', async () => { + await renderDialog({ descriptor: GITHUB_BOUND }); + + const group = document.querySelector( + '[role="group"][aria-label="PR Watch Events"]', + )!; + const describedBy = group.getAttribute('aria-describedby'); + expect(describedBy).not.toBeNull(); + const hint = document.getElementById(describedBy!); + expect(hint?.textContent).toContain('above to select watch events'); + expect(hint?.getAttribute('aria-live')).toBe('polite'); + + await act(async () => { + document + .querySelector('button[role="switch"]')! + .click(); + }); + expect(group.getAttribute('aria-describedby')).toBeNull(); + }); + + it('mounts a stored delivery-on instance with its events enabled', async () => { + const instance: DaemonChannelInstanceSnapshot = { + name: 'github-bot', + config: { + type: 'github', + deliverToBoundSessions: true, + prWatchEvents: ['ci_failure'], + }, + secrets: { token: { present: true, source: 'literal' } }, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + await renderDialog({ descriptor: GITHUB_BOUND, instance }); + + const boxes = Array.from( + document.querySelectorAll( + '[role="group"][aria-label="PR Watch Events"] input[type="checkbox"]', + ), + ); + expect(boxes.length).toBeGreaterThan(0); + expect(boxes.every((box) => !box.disabled)).toBe(true); + expect( + boxes.find((b) => + b.closest('label')?.textContent?.includes('ci_failure'), + )!.checked, + ).toBe(true); + expect(document.body.textContent).not.toContain( + 'above to select watch events.', + ); + + // The live region must exist (empty) before the hint appears, and the + // same node must survive the transition — a region mounted together + // with its text is exactly what aria-live cannot announce. + const region = Array.from( + document.querySelectorAll('span[aria-live="polite"]'), + ).find((node) => node.id.endsWith('-prWatchEvents-disabled-hint')); + expect(region).toBeDefined(); + expect(region!.textContent).toBe(''); + await act(async () => { + document + .querySelector('button[role="switch"]')! + .click(); + }); + expect(region!.isConnected).toBe(true); + expect(region!.textContent).toContain('above to select watch events'); + }); + it('localizes the instructions copy for zh-CN operators', async () => { await renderDialog({ descriptor: MULTILINE_INSTRUCTIONS, @@ -1275,6 +1809,11 @@ describe('ChannelEditorDialog', () => { toggle!.click(); }); + // The pre-existing hint call sites pass no hintId: the local-gh hint + // must keep rendering without an id or a live region. + expect(document.querySelectorAll('span[aria-live]').length).toBe(0); + expect(document.querySelectorAll('span[id]').length).toBe(0); + const save = Array.from(document.querySelectorAll('button')).find( (button) => button.textContent?.trim() === 'Save', ); diff --git a/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx b/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx index 6535043593a..3546da9403e 100644 --- a/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx +++ b/packages/web-shell/client/components/channels/ChannelEditorDialog.tsx @@ -134,6 +134,11 @@ const SHARED_SESSION_FIELD_KEYS = new Set([ 'multiSession', 'instructions', ]); +// One panel: watch events only mean something with bound delivery enabled. +const BOUND_SESSION_FIELD_KEYS = new Set([ + 'deliverToBoundSessions', + 'prWatchEvents', +]); const SHARED_FIELD_LABEL_KEYS: Record = { outputMode: 'channels.editor.field.shared.outputMode', @@ -247,6 +252,7 @@ function FieldShell({ label, required, hint, + hintId, description, error, children, @@ -255,6 +261,7 @@ function FieldShell({ label: string; required?: boolean; hint?: string; + hintId?: string; description?: string; error?: string; children: ReactNode; @@ -270,7 +277,17 @@ function FieldShell({ ) : null} - {hint ? {hint} : null} + {hint || hintId ? ( + // A hintId keeps the live region mounted while the hint is empty, + // so screen readers announce the hint when it appears. + + {hint} + + ) : null} {children} {description ? ( @@ -348,7 +365,11 @@ export function ChannelEditorDialog({ (field) => !SHARED_ACCESS_FIELD_KEYS.has(field.key) && !SHARED_SESSION_FIELD_KEYS.has(field.key) && - !ADVANCED_FIELD_KEYS.has(field.key), + !ADVANCED_FIELD_KEYS.has(field.key) && + !BOUND_SESSION_FIELD_KEYS.has(field.key), + ); + const boundSessionFields = descriptor.fields.filter((field) => + BOUND_SESSION_FIELD_KEYS.has(field.key), ); useEffect(() => { @@ -375,10 +396,11 @@ export function ChannelEditorDialog({ }, [workspaceCwd]); useEffect(() => { - if (!advancedOpen || !scrollToErrorKey) return; - // Runs after the forced-open panel content mounts; the id is the same one - // FieldShell renders, so the scroll lands on the erroring control. The - // optional call keeps jsdom (no scrollIntoView) from throwing. + if (!scrollToErrorKey) return; + // Runs after the forced-open panel content mounts (submit sets both in + // one batch); the id is the same one FieldShell renders, so the scroll + // lands on the erroring control. The optional call keeps jsdom (no + // scrollIntoView) from throwing. document .getElementById(`${formId}-${scrollToErrorKey}`) ?.scrollIntoView?.({ block: 'center' }); @@ -451,6 +473,8 @@ export function ChannelEditorDialog({ return t('channels.editor.validation.conflictingRepoAliases'); if (code === 'invalidOption') return t('channels.editor.validation.invalidOption'); + if (code === 'requiresBoundDelivery') + return t('channels.editor.validation.requiresBoundDelivery'); if (code === 'number') return t('channels.editor.validation.number'); if (code === 'outOfRange') { const numberField = field && field.kind === 'number' ? field : undefined; @@ -479,6 +503,37 @@ export function ChannelEditorDialog({ }); }; + // Re-derive only the errors already on screen, so each remedy (switch in + // either direction, untick) retires exactly the instruction it satisfies — + // including invalidOption, whose remedy is unticking the offending value — + // without surfacing an error the operator never submitted. Submit stays the + // only population site; deriving through validateChannelEditorDraft keeps + // the daemon precedence that the parity table pins. + const refreshErrors = (values: ChannelEditorDraft['values']) => + setErrors((current) => { + const shown = Object.keys(current); + if (shown.length === 0) return current; + const validation = validateChannelEditorDraft( + descriptor, + { ...draft, values }, + existingNames, + instance, + ); + const next = { ...current }; + for (const key of shown) { + const code = validation[key]; + if (code) { + next[key] = validationMessage( + descriptor.fields.find((field) => field.key === key), + code, + ); + } else { + delete next[key]; + } + } + return next; + }); + const submit = async (event: FormEvent) => { event.preventDefault(); const validation = validateChannelEditorDraft( @@ -488,15 +543,14 @@ export function ChannelEditorDialog({ instance, ); if (Object.keys(validation).length > 0) { - // An error on a collapsed field would otherwise be invisible: open the - // panel and bring the first erroring advanced field into view. + // An error below the fold or on a collapsed field would otherwise be + // invisible: open the advanced panel when needed and bring the first + // erroring field — advanced or not — into view. const advancedErrorKey = Object.keys(validation).find((key) => ADVANCED_FIELD_KEYS.has(key), ); - if (advancedErrorKey) { - setAdvancedOpen(true); - setScrollToErrorKey(advancedErrorKey); - } + if (advancedErrorKey) setAdvancedOpen(true); + setScrollToErrorKey(advancedErrorKey ?? Object.keys(validation)[0]); setErrors( Object.fromEntries( Object.entries(validation).map(([key, code]) => [ @@ -668,6 +722,14 @@ export function ChannelEditorDialog({ field.key === 'useLocalGh' && isRepositoryOnlyChannelType(descriptor.type); const updateSwitch = (checked: boolean) => { + if (field.key === 'deliverToBoundSessions') { + // Both directions re-derive shown errors: on retires the dependency + // instruction, off keeps a shown one instead of dropping it silently. + refreshErrors({ + ...draft.values, + deliverToBoundSessions: checked, + }); + } if (!isLocalGhSwitch) { update(checked); return; @@ -753,6 +815,11 @@ export function ChannelEditorDialog({ } if (field.kind === 'string-list' && field.options?.length) { const options = field.options; + // Same scope as the validation mirror in channel-editor-state. + const watchField = + field.key === 'prWatchEvents' && descriptor.type === 'github'; + const gated = + watchField && draft.values['deliverToBoundSessions'] !== true; const selected = toList(String(value ?? '')); // The adapter matches these values case-insensitively, so a stored // `COMMENT` has to tick the canonical `comment` box instead of rendering @@ -772,14 +839,18 @@ export function ChannelEditorDialog({ const next = new Set(selectedKeys); if (checked) next.add(target.toLowerCase()); else next.delete(target.toLowerCase()); - update( - [ - ...options - .map((option) => option.value) - .filter((entry) => next.has(entry.toLowerCase())), - ...stored.filter((entry) => next.has(entry.toLowerCase())), - ].join(', '), - ); + const joined = [ + ...options + .map((option) => option.value) + .filter((entry) => next.has(entry.toLowerCase())), + ...stored.filter((entry) => next.has(entry.toLowerCase())), + ].join(', '); + // Unticking is a remedy too: re-derive so a satisfied instruction + // retires without waiting for a second Save. + if (watchField) { + refreshErrors({ ...draft.values, [field.key]: joined }); + } + update(joined); }; return (
{[ ...options, ...stored.map((entry) => ({ value: entry, label: entry })), ].map((option) => { const isSelected = selectedKeys.has(option.value.toLowerCase()); + // A stored selection stays clearable while gated, so turning + // delivery off never strands the operator with events they + // cannot remove. + const optionDisabled = gated && !isSelected; const optionLabel = fieldOptionLabel( field, option.value, @@ -811,11 +893,13 @@ export function ChannelEditorDialog({ key={option.value} className={styles.optionRow} data-selected={isSelected} + data-disabled={optionDisabled} > toggleValue(option.value, event.target.checked) } @@ -1286,6 +1370,20 @@ export function ChannelEditorDialog({ ); })} + {boundSessionFields.length > 0 ? ( +
+
+

+ {t('channels.editor.section.boundSessions')} +

+

+ {t('channels.editor.section.boundSessions.description')} +

+
+ {boundSessionFields.map(renderField)} +
+ ) : null} + {sessionFields.length > 0 ? (

diff --git a/packages/web-shell/client/components/channels/channel-editor-parity.test.ts b/packages/web-shell/client/components/channels/channel-editor-parity.test.ts new file mode 100644 index 00000000000..6eda135f642 --- /dev/null +++ b/packages/web-shell/client/components/channels/channel-editor-parity.test.ts @@ -0,0 +1,103 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { plugin } from '@qwen-code/channel-github'; +import type { + DaemonChannelInstanceSnapshot, + DaemonChannelTypeDescriptor, +} from '@qwen-code/sdk/daemon'; +import { + createChannelEditorDraft, + validateChannelEditorDraft, +} from './channel-editor-state'; + +// Parity between the editor's client mirror and the daemon authority. The +// descriptor is the plugin's own field list, so a lone change on either side +// (dependency dropped, second precondition added, vocabulary moved) fails +// this table instead of leaving both suites green. Test-only import: the +// plugin is a devDependency and never reaches the web-shell bundle. +// management is optional on ChannelPlugin by contract; narrow once so a +// hypothetical missing descriptor fails with a clear message instead of a +// module-load TypeError (and stays TS18048-clean if test files ever join the +// package typecheck). +const management = plugin.management; +if (!management) { + throw new Error('@qwen-code/channel-github declares no management metadata.'); +} + +const descriptor: DaemonChannelTypeDescriptor = { + type: 'github', + displayName: 'GitHub', + manageable: true, + fields: management.fields, +}; + +const CASES = [ + { + name: 'delivery off with a selection', + events: ['ci_failure'], + delivery: false, + code: 'requiresBoundDelivery', + daemon: /requires deliverToBoundSessions/, + }, + { + name: 'delivery on with a selection', + events: ['ci_failure'], + delivery: true, + code: undefined, + daemon: undefined, + }, + { + name: 'delivery off with an empty selection', + events: [], + delivery: false, + code: undefined, + daemon: undefined, + }, + { + name: 'delivery off with an unknown value', + events: ['typo_event'], + delivery: false, + code: 'invalidOption', + daemon: /Unrecognized prWatchEvents/, + }, +] as const; + +describe('bound-delivery rule parity (editor mirror vs daemon authority)', () => { + it.each(CASES)('$name', ({ events, delivery, code, daemon }) => { + const config = { + type: 'github', + useLocalGh: true, + deliverToBoundSessions: delivery, + prWatchEvents: [...events], + }; + const instance: DaemonChannelInstanceSnapshot = { + name: 'parity', + config, + secrets: {}, + startsWithServe: false, + runtime: { state: 'stopped' }, + }; + const draft = createChannelEditorDraft(descriptor, instance); + draft.name = 'parity'; + + const clientCode = validateChannelEditorDraft( + descriptor, + draft, + [], + instance, + )['prWatchEvents']; + expect(clientCode).toBe(code); + + const daemonMessage = management.validateConfig?.(config); + if (daemon) { + expect(daemonMessage).toMatch(daemon); + } else { + expect(daemonMessage).toBeUndefined(); + } + }); +}); diff --git a/packages/web-shell/client/components/channels/channel-editor-state.test.ts b/packages/web-shell/client/components/channels/channel-editor-state.test.ts index 53a6b112704..5a68089fc8e 100644 --- a/packages/web-shell/client/components/channels/channel-editor-state.test.ts +++ b/packages/web-shell/client/components/channels/channel-editor-state.test.ts @@ -17,7 +17,10 @@ import { retainedRepositorySettings, validateChannelEditorDraft, } from './channel-editor-state'; -import type { ChannelGroupOverrideDraft } from './channel-editor-state'; +import type { + ChannelEditorDraft, + ChannelGroupOverrideDraft, +} from './channel-editor-state'; const DINGTALK: DaemonChannelTypeDescriptor = { type: 'dingtalk', @@ -972,6 +975,61 @@ const GITHUB: DaemonChannelTypeDescriptor = { ], }; +describe('GitHub watch events require bound delivery', () => { + const BOUND: DaemonChannelTypeDescriptor = { + ...GITHUB, + fields: [ + ...GITHUB.fields, + { + key: 'deliverToBoundSessions', + label: 'Deliver To PR-Bound Sessions', + kind: 'boolean', + }, + { + key: 'prWatchEvents', + label: 'PR Watch Events', + kind: 'string-list', + options: [ + { value: 'ci_failure', label: 'ci_failure' }, + { value: 'merge_conflict', label: 'merge_conflict' }, + ], + }, + ], + }; + + function draftWithEvents(events: string): ChannelEditorDraft { + const draft = createChannelEditorDraft(BOUND); + draft.name = 'my-bot'; + draft.values.useLocalGh = true; + draft.values.prWatchEvents = events; + return draft; + } + + it('refuses a watch selection while bound delivery is off', () => { + expect( + validateChannelEditorDraft(BOUND, draftWithEvents('ci_failure'), []), + ).toEqual({ prWatchEvents: 'requiresBoundDelivery' }); + }); + + it('allows the selection once bound delivery is on', () => { + const draft = draftWithEvents('ci_failure'); + draft.values.deliverToBoundSessions = true; + expect(validateChannelEditorDraft(BOUND, draft, [])).toEqual({}); + }); + + it('leaves an empty selection alone with delivery off', () => { + expect(validateChannelEditorDraft(BOUND, draftWithEvents(''), [])).toEqual( + {}, + ); + }); + + it('reports invalidOption before the delivery dependency', () => { + expect( + validateChannelEditorDraft(BOUND, draftWithEvents('typo_event'), []), + ).toEqual({ prWatchEvents: 'invalidOption' }); + }); +}); + describe('Descriptor-driven privatePolicy', () => { it('defaults enum fields to the first option for new channels', () => { const draft = createChannelEditorDraft(GITHUB); diff --git a/packages/web-shell/client/components/channels/channel-editor-state.ts b/packages/web-shell/client/components/channels/channel-editor-state.ts index 5bb68b2f8c2..405be598f81 100644 --- a/packages/web-shell/client/components/channels/channel-editor-state.ts +++ b/packages/web-shell/client/components/channels/channel-editor-state.ts @@ -60,7 +60,8 @@ export type ChannelEditorValidationCode = | 'invalidGroupEntry' | 'number' | 'outOfRange' - | 'policy'; + | 'policy' + | 'requiresBoundDelivery'; export type ChannelEditorValidationErrors = Record< string, @@ -423,6 +424,16 @@ export function validateChannelEditorDraft( if (!hasToken && draft.values['useLocalGh'] !== true) { errors['token'] = 'credential'; } + // Mirror validateBoundSessionConfig so the dependency is named at the + // field instead of surfacing as a raw daemon rejection after submit. + // invalidOption wins, the same order the runtime validates in. + if ( + !errors['prWatchEvents'] && + toList(String(draft.values['prWatchEvents'] ?? '')).length > 0 && + draft.values['deliverToBoundSessions'] !== true + ) { + errors['prWatchEvents'] = 'requiresBoundDelivery'; + } } if ( !isRepositoryOnlyChannelType(descriptor.type) && diff --git a/packages/web-shell/client/e2e/visuals/screenshots.spec.ts b/packages/web-shell/client/e2e/visuals/screenshots.spec.ts index 67ec42ff7f1..2cac96a4223 100644 --- a/packages/web-shell/client/e2e/visuals/screenshots.spec.ts +++ b/packages/web-shell/client/e2e/visuals/screenshots.spec.ts @@ -1423,6 +1423,22 @@ for (const theme of THEMES) { { value: 'ci_activity', label: 'ci_activity' }, ], }, + { + key: 'deliverToBoundSessions', + label: 'Deliver To PR-Bound Sessions', + kind: 'boolean', + }, + { + key: 'prWatchEvents', + label: 'PR Watch Events', + kind: 'string-list', + options: [ + { value: 'ci_failure', label: 'ci_failure' }, + { value: 'merge_conflict', label: 'merge_conflict' }, + { value: 'head_change', label: 'head_change' }, + { value: 'state_change', label: 'state_change' }, + ], + }, { key: 'privatePolicy', label: 'Private Policy', @@ -1502,6 +1518,29 @@ for (const theme of THEMES) { .scrollIntoViewIfNeeded(); await expect(reasonFilter).toBeInViewport({ ratio: 1 }); await captureScreenshot(page, `github-channel-editor-reasons-${theme}`); + // The bound-session panel gates watch events on delivery. Capture the + // gated state (disabled boxes + hint), then enabled with a selection, + // so the published pictures document the dependency itself. + const watchGroup = page.getByRole('group', { name: 'PR Watch Events' }); + await watchGroup.scrollIntoViewIfNeeded(); + await expect(watchGroup).toBeInViewport({ ratio: 1 }); + await expect(watchGroup.getByRole('checkbox')).toHaveCount(4); + const ciFailure = watchGroup.getByRole('checkbox', { + name: /ci_failure/, + }); + await expect(ciFailure).toBeDisabled(); + await captureScreenshot( + page, + `github-channel-editor-watch-gated-${theme}`, + ); + await page + .getByRole('switch', { name: 'Deliver To PR-Bound Sessions' }) + .click(); + await expect(ciFailure).toBeEnabled(); + await ciFailure.check(); + await watchGroup.scrollIntoViewIfNeeded(); + await expect(watchGroup).toBeInViewport({ ratio: 1 }); + await captureScreenshot(page, `github-channel-editor-watch-on-${theme}`); await page.getByLabel('Instance name').fill('github-bot'); await page.getByRole('button', { name: 'Save' }).click(); await expect( diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 6d3fcdf63f8..e6f8c62b609 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -4458,6 +4458,9 @@ const EN: Messages = { 'channels.editor.section.credentials': 'Credentials', 'channels.editor.section.connection': 'Connection', 'channels.editor.section.routing': 'Notifications and routing', + 'channels.editor.section.boundSessions': 'PR-bound sessions', + 'channels.editor.section.boundSessions.description': + 'Watch events apply to PRs bound to a live session. Enable delivery first, then choose the transitions to watch.', 'channels.editor.section.session': 'Conversation management', 'channels.editor.section.access': 'Access control', 'channels.editor.section.access.description': @@ -4585,6 +4588,8 @@ const EN: Messages = { 'channels.editor.field.github.prWatchEvents': 'PR Watch Events', 'channels.editor.field.github.prWatchEvents.description': 'Optional. State transitions to watch for bound PRs (requires Deliver To PR-Bound Sessions). Leave empty to disable.', + 'channels.editor.field.github.prWatchEvents.disabledHint': + 'Enable "Deliver To PR-Bound Sessions" above to select watch events.', 'channels.editor.field.github.prWatchEvents.option.ci_failure': 'ci_failure — CI checks turn red on a head commit', 'channels.editor.field.github.prWatchEvents.option.merge_conflict': @@ -4776,6 +4781,8 @@ const EN: Messages = { `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.requiresBoundDelivery': + 'Enable "Deliver To PR-Bound Sessions" to save the selected watch events.', 'channels.editor.validation.number': 'Enter a valid number.', 'channels.editor.validation.outOfRange': (v) => v?.min !== undefined && v?.max !== undefined @@ -8947,6 +8954,9 @@ const ZH: Messages = { 'channels.editor.section.credentials': '应用凭据', 'channels.editor.section.connection': '连接', 'channels.editor.section.routing': '通知与路由', + 'channels.editor.section.boundSessions': 'PR 绑定会话', + 'channels.editor.section.boundSessions.description': + '监视事件作用于已绑定到活跃会话的 PR。先开启投递，再选择要监视的状态转换。', 'channels.editor.section.session': '会话管理', 'channels.editor.section.access': '访问控制', 'channels.editor.section.access.description': @@ -9071,6 +9081,8 @@ const ZH: Messages = { 'channels.editor.field.github.prWatchEvents': 'PR 状态监视事件', 'channels.editor.field.github.prWatchEvents.description': '可选。为绑定 PR 监视的状态转换（需开启"投递到 PR 绑定会话"）。留空则禁用。', + 'channels.editor.field.github.prWatchEvents.disabledHint': + '开启上方「投递到 PR 绑定会话」后即可选择监视事件。', 'channels.editor.field.github.prWatchEvents.option.ci_failure': 'ci_failure — CI 检查在 head 提交上变红', 'channels.editor.field.github.prWatchEvents.option.merge_conflict': @@ -9252,6 +9264,8 @@ const ZH: Messages = { 'channels.editor.validation.invalidGroupEntry': (v) => `该渠道把以下群组条目存成了非对象：${v?.repositories ?? ''}。只要它还在，daemon 就会拒绝每次保存，而表单里没有对应的行。请在 settings.json 里修正或删除该条目，然后重新加载渠道。`, 'channels.editor.validation.invalidOption': '请移除不在允许列表中的值。', + 'channels.editor.validation.requiresBoundDelivery': + '需开启「投递到 PR 绑定会话」才能保存所选的监视事件。', 'channels.editor.validation.number': '请输入有效数字。', 'channels.editor.validation.outOfRange': (v) => v?.min !== undefined && v?.max !== undefined diff --git a/packages/web-shell/package.json b/packages/web-shell/package.json index 35b590e81f9..9f519435ed2 100644 --- a/packages/web-shell/package.json +++ b/packages/web-shell/package.json @@ -110,6 +110,7 @@ "@pierre/trees": "1.0.0-beta.6", "@playwright/test": "1.61.1", "@qwen-code/acp-bridge": "file:../acp-bridge", + "@qwen-code/channel-github": "file:../channels/github", "@qwen-code/sdk": "file:../sdk-typescript", "@tailwindcss/vite": "^4.3.2", "@types/node": "^22.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3a8ad214980..32cabfc6baf 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1715,6 +1715,9 @@ importers: '@qwen-code/acp-bridge': specifier: workspace:* version: link:../acp-bridge + '@qwen-code/channel-github': + specifier: workspace:* + version: link:../channels/github '@qwen-code/sdk': specifier: workspace:* version: link:../sdk-typescript