diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 1a98a2e222..cc65d6dbb2 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -113,6 +113,12 @@ env: # exists to stop an unproductive LOOP, not to ration ordinary iteration — # a genuinely stuck PR still stops, just later. MAX_ROUNDS: '10' + # Suggestions may improve a young PR, but continuing to implement them after + # five change-producing rounds expands the diff and creates fresh review + # churn. From round 6 onward, only Critical findings, formally requested + # changes, failed checks, and base conflicts may drive code changes; + # lower-severity feedback is recorded and left open. + CRITICAL_ONLY_AFTER_ROUND: '5' # An auth/access model error (401/402/403, "no access"/"does not exist") # never self-heals - only a maintainer can fix the key - and every retry # costs an agent run AND a PR comment. Cap those attempts far below @@ -2259,15 +2265,11 @@ jobs: | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) | select((.state // "") | IN("CHANGES_REQUESTED", "COMMENTED")) ] | length' \ "${WORKDIR}/rv.json")" - # Per AGENTS.md's review policy, Suggestion-level findings ARE - # addressed during a PR's first ~5 review rounds; only past that are - # they deferred (with a recorded reason). The loop's MAX_ROUNDS cap is - # that same boundary — every round the loop actually runs is within - # the address-Suggestions window — so /review `**[Suggestion]**` - # inline comments count as actionable feedback here. The agent's - # triage still decides implement-vs-defer per finding and records the - # decision in its summary comment, and the MAX_ROUNDS handoff is the - # defer-to-a-human boundary. + # Per AGENTS.md's review policy, Suggestion-level findings are + # actionable during the first five change-producing rounds. The + # scan still selects later feedback so prepare can record and + # watermark its deferral; only the address job filters what may + # drive code changes. N_COMMENTS="$(jq --arg wm "${EFF_WM}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ --argjson trust "${TRUSTED_ASSOC}" ' [ .[] @@ -2817,9 +2819,76 @@ jobs: STALE='true' echo "⛔ live round ${ROUND} already at MAX_ROUNDS (${MAX_ROUNDS}) — discarding without action or marker" fi + CRITICAL_ONLY='false' + if [[ "${ROUND}" -ge "${CRITICAL_ONLY_AFTER_ROUND}" ]]; then + CRITICAL_ONLY='true' + fi echo "stale=${STALE}" >> "${GITHUB_OUTPUT}" echo "effective_round=${ROUND}" >> "${GITHUB_OUTPUT}" + rm -f "${WORKDIR}/deferred-feedback.md" + if [[ "${CRITICAL_ONLY}" == "true" ]]; then + PR_URL="https://github.com/${REPO}/pull/${PR}" + { + echo '## Deferred non-Critical feedback' + echo + echo "Critical-only mode is active after ${CRITICAL_ONLY_AFTER_ROUND} change-producing rounds. Any items listed below stay open for human follow-up; do not modify code, resolve threads, or reply on their behalf." + echo + jq -r --arg wm "${WATERMARK}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ + --argjson trust "${TRUSTED_ASSOC}" --arg pr_url "${PR_URL}" ' + .[] + | select((.submitted_at // "") > $wm) + | select((.user.login // "") != $ab) + | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) + | select((.state // "") == "COMMENTED") + | select(((.body // "") | contains("**[Critical]**")) | not) + | "- Review by @\(.user.login): \(.html_url // $pr_url)"' \ + "${WORKDIR}/rv.json" + jq -rs --arg wm "${WATERMARK}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ + --argjson trust "${TRUSTED_ASSOC}" --arg pr_url "${PR_URL}" \ + --slurpfile reviews "${WORKDIR}/rv.json" ' + add as $comments + | ($reviews | add) as $reviews + | $comments[] + | select((.created_at // "") > $wm) + | select((.user.login // "") != $ab) + | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) + | select(( + ((.body // "") | contains("**[Critical]**")) + or ((.in_reply_to_id // null) as $root + | $root != null + and any($comments[]; + .id == $root + and ((.body // "") | contains("**[Critical]**")))) + or ((.pull_request_review_id // null) as $review + | $review != null + and any($reviews[]; + .id == $review + and ((.state // "") == "CHANGES_REQUESTED"))) + ) | not) + | "- Inline rc:\(.id) \(.path // "?"):\(.line // "?"): \(.html_url // $pr_url)"' \ + "${WORKDIR}/rc.json" + jq -r --arg wm "${WATERMARK}" --arg rb "${REVIEW_BOT}" --arg ab "${AUTOFIX_BOT}" \ + --argjson trust "${TRUSTED_ASSOC}" --arg pr_url "${PR_URL}" ' + .[] + | select((.created_at // "") > $wm) + | select((.user.login // "") != $ab) + | select(((.author_association // "") | IN($trust[])) or (.user.login // "") == $rb) + | select((.body // "") | test("' fi printf '\n' - printf '**Sandboxed verification: %s** - [workflow run](%s)\n\n' "$HEADLINE" "$RUN_URL" + printf '**Sandboxed verification: %s** - [workflow run](%s)\n' "$HEADLINE" "$RUN_URL" + printf '**沙箱验证：%s**\n\n' "${HEADLINE_ZH:-$HEADLINE}" if [ "${VERDICT:-}" = 'pass' ]; then printf '%s\n\n' "$SCOPE_EN" printf '%s\n\n' "$SCOPE_ZH" diff --git a/.qwen/skills/autofix/SKILL.md b/.qwen/skills/autofix/SKILL.md index 56e6888ebb..a3efc13739 100644 --- a/.qwen/skills/autofix/SKILL.md +++ b/.qwen/skills/autofix/SKILL.md @@ -202,6 +202,13 @@ implement — satisfying a nit is never a reason to bloat the code. reason per finding (out of scope, conflicts with the PR's direction, or not worth the diff growth) so the deferral is visible in the PR thread — never drop one silently. +- Critical-only mode: when `feedback.md` contains a + `Deferred non-Critical feedback` section, the PR has already completed five + suggestion-capable, change-producing rounds. That section is an audit record, + not work: do not modify code, resolve threads, or write comment replies for + those items. Act only on Critical feedback and formally requested changes + rendered in the actionable sections, failed checks, and the requested + base-conflict resolution. - Needs a maintainer's decision: a finding that turns on a judgment that is NOT yours to make — a product or scope tradeoff (is this acceptable for v1? should the PR be split?), two reviewers asking for opposite things, or whether diff --git a/.qwen/skills/verify-pr/SKILL.md b/.qwen/skills/verify-pr/SKILL.md index 93503cc1d5..e5c4ac42ee 100644 --- a/.qwen/skills/verify-pr/SKILL.md +++ b/.qwen/skills/verify-pr/SKILL.md @@ -347,25 +347,31 @@ central claim from being tested — say why. 1. **Verdict line first**, with assertion totals and the verified head OID (`git rev-parse HEAD^2` — not the snapshot's, which may have drifted). -2. **Central claim + A/B table** (cells, oracles, head vs control counts). -3. **Corrections**, when an earlier review round or bot comment described +2. **中文摘要** in a collapsed `
` block, **immediately after the + verdict**: verdict, A/B 结论, findings, 未覆盖范围. Collapsed, so it costs a + reader who does not want it exactly one line; placed here rather than at + the end, because the whole report is already inside a `
` on the + PR — burying the Chinese summary under it made a Chinese reader expand a + fold and scroll the entire report to reach the one section written for + them. Cite the tables below by name instead of restating their numbers in + prose: a number written twice is a number that can disagree with itself. +3. **Central claim + A/B table** (cells, oracles, head vs control counts). +4. **Corrections**, when an earlier review round or bot comment described the code inaccurately (a wrong ARIA role, a wrong mechanism, a misattributed cause). State the correct fact with its evidence and label it explicitly as a correction to the description — not as a request to change the code. Leaving a wrong description standing costs the next reader more than the original finding did. -4. **Findings**, ordered by severity, each with the exact reproducing +5. **Findings**, ordered by severity, each with the exact reproducing command; for a blocker, enumerate the blast radius (the affected call sites, not just the one you hit), demonstrate the sharpest consequence end-to-end when budget allows, and where the cause is clear add a collapsed minimal suggested fix that preserves the original commit's intent. -5. **Not covered** — every claim, surface, or gate you skipped. A silent cap +6. **Not covered** — every claim, surface, or gate you skipped. A silent cap reads as "covered everything"; never allow that. -6. **Methodology** — one paragraph: environment, how each harness drove the +7. **Methodology** — one paragraph: environment, how each harness drove the code, where the raw logs live. -7. **中文摘要** in a collapsed `
` block: verdict, A/B 结论, findings, - 未覆盖范围. ## Hard rules diff --git a/docs/design/2026-07-28-web-shell-composer-intent-suggestions.md b/docs/design/2026-07-28-web-shell-composer-intent-suggestions.md new file mode 100644 index 0000000000..06b052d14e --- /dev/null +++ b/docs/design/2026-07-28-web-shell-composer-intent-suggestions.md @@ -0,0 +1,71 @@ +# Web Shell Composer Intent Suggestions + +## Summary + +Extend Web Shell's existing new-topic suggestion so one conservative +classification can recommend either asking a side question with `/btw` or +sending a substantial new topic in a fresh session. + +The composer continues to show at most one non-blocking action. A valid +`none` decision renders nothing. Invalid, failed, or cancelled classifications +also render nothing. + +## Decision contract + +```ts +type SuggestionKind = 'btw' | 'new_session' | 'none'; + +interface SuggestionDecision { + suggestion: SuggestionKind; + confidence: number; +} +``` + +Only `btw` and `new_session` decisions at or above the existing confidence +threshold become actionable. The actionable state records the exact classified +draft and source session so both can be checked again when the user clicks. + +## Behavior + +- `btw` is for a quick, self-contained side question that should not disturb + the main task. +- `new_session` is for a clearly different, substantial task or topic. +- `none` covers continuations, uncertainty, and drafts that fit neither action. +- BTW classification starts after one prior user/assistant exchange. New-session + suggestions keep their stricter existing context thresholds. +- Follow-up-like wording may be classified for BTW, but can never use the + relaxed BTW threshold to surface a new-session action. +- Clicking a `btw` suggestion submits `/btw ` through the existing + editor path, which preserves the command's current history and composer-clear + semantics. +- A draft with an image or composer tag is never eligible for `btw`. +- `new_session` retains the existing clear, detach, create, and auto-submit + sequence, including image preservation and session-race cancellation. + +## Safety + +The classifier remains conservative and fail-closed: + +- malformed output, unknown actions, invalid confidence, errors, and + cancellation produce no action; +- a session change aborts pending classification and invalidates a visible + suggestion; +- a draft or attachment change invalidates a visible suggestion; +- click handling checks the current draft, source session, and attachment state + immediately before executing; +- attachments are treated as present until ChatEditor reports otherwise, so a + transient unknown state cannot expose a `/btw` action. + +## Scope + +The change stays inside Web Shell. It reuses existing daemon session generation, +editor submission, and `/btw` behavior. It does not add daemon or SDK routes, +change styling, or introduce a general-purpose suggestion framework. + +## Test strategy + +- Hook tests cover the three decision values, strict parsing, confidence, + attachment gating, and stale-session results. +- App tests cover `/btw` execution and composer clearing, stale draft/session + rejection, attachment rejection, and the existing new-session races. +- ChatEditor tests cover attachment-presence reporting. diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 24c8a1b42a..59a43f342f 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -51,6 +51,7 @@ type ChatEditorTestProps = { ) => boolean | void; onCancel?: () => void; onInputTextChange?: (text: string) => void; + onAttachmentsChange?: (hasAttachments: boolean) => void; onStartNewSessionSuggestion?: () => void; newSessionSuggestion?: { isVisible: boolean; classifiedInput: string } | null; skills?: Array<{ name: string; description: string }>; @@ -170,6 +171,7 @@ const { mockConnection: connection, mockSessionActions: { sendPrompt: vi.fn().mockResolvedValue(undefined), + btwSession: vi.fn().mockResolvedValue({ answer: 'side answer' }), generateSessionContent: vi.fn(async function* () {}), createSession: vi.fn().mockResolvedValue({ sessionId: 'session-1' }), attachSession: vi.fn().mockResolvedValue(undefined), @@ -400,12 +402,23 @@ vi.mock('./components/ChatEditor', async () => { props: ChatEditorTestProps, ref: React.ForwardedRef<{ clear: () => void; + hasAttachments: () => boolean; hasInput: () => boolean; insertText: (text: string) => void; + submit: (input?: { text?: string }) => void; focus: () => void; }>, ) { testState.latestChatEditorProps = props; + const { onAttachmentsChange } = props; + React.useEffect(() => { + onAttachmentsChange?.( + Boolean( + testState.promptImages?.length || + testState.inputAnnotations?.length, + ), + ); + }, [onAttachmentsChange]); React.useImperativeHandle(ref, () => ({ clear: () => { testState.prompt = ''; @@ -413,17 +426,23 @@ vi.mock('./components/ChatEditor', async () => { props.onInputTextChange?.(''); editorClear(); }, + hasAttachments: () => + Boolean( + testState.promptImages?.length || + testState.inputAnnotations?.length, + ), hasInput: () => testState.prompt.trim().length > 0, insertText: editorInsertText, - submit: () => { - props.onSubmit( - testState.prompt, + submit: (input) => { + const accepted = props.onSubmit( + input?.text ?? testState.prompt, testState.promptImages, editorCommit, testState.inputAnnotations ? { inputAnnotations: testState.inputAnnotations } : undefined, ); + if (accepted) editorCommit(); }, // The panel focus effect calls editorRef.current?.focus() when a panel // closes with no pending approval (e.g. resuming a session). @@ -1587,6 +1606,7 @@ beforeEach(() => { if (typeof value === 'function' && 'mockClear' in value) value.mockClear(); } mockSessionActions.sendPrompt.mockResolvedValue(undefined); + mockSessionActions.btwSession.mockResolvedValue({ answer: 'side answer' }); mockSessionActions.createSession.mockResolvedValue({ sessionId: 'session-1', }); @@ -5477,7 +5497,7 @@ describe('App session callbacks', () => { requestId: 'req-1', seq: 0, text: JSON.stringify({ - shouldSuggestNewSession: true, + suggestion: 'new_session', confidence: 0.91, }), }; @@ -5534,6 +5554,207 @@ describe('App session callbacks', () => { expect(editorInsertText).not.toHaveBeenCalled(); }); + it('suggests sending a side question with BTW and clears the accepted draft', async () => { + vi.useFakeTimers(); + mockConnection.capabilities.features = ['session_generation']; + ( + mockConnection as typeof mockConnection & { + tokenCount?: number; + contextWindow?: number; + } + ).tokenCount = 600; + ( + mockConnection as typeof mockConnection & { + tokenCount?: number; + contextWindow?: number; + } + ).contextWindow = 1000; + testState.messages = Array.from({ length: 8 }, (_, index) => ({ + id: `m-btw-${index}`, + role: index % 2 === 0 ? 'user' : 'assistant', + content: `existing session topic ${index} about daemon generation review work`, + timestamp: index, + })); + const sideQuestion = '这里的 confidence 阈值为什么是 0.75？'; + testState.prompt = sideQuestion; + mockSessionActions.generateSessionContent.mockImplementation( + async function* () { + yield { + type: 'delta', + requestId: 'req-btw', + seq: 0, + text: JSON.stringify({ + suggestion: 'btw', + confidence: 0.92, + }), + }; + yield { + type: 'done', + requestId: 'req-btw', + model: 'fast-model', + modelSource: 'fast', + }; + }, + ); + + const { container } = renderApp(); + await flush(); + + act(() => { + testState.latestChatEditorProps?.onInputTextChange?.(testState.prompt); + }); + await flush(); + act(() => { + vi.advanceTimersByTime(121); + }); + await flush(); + act(() => { + vi.advanceTimersByTime(701); + }); + await flush(); + + expect( + container.querySelector('[data-testid="btw-suggestion"]')?.textContent, + ).toContain('side question'); + + await act(async () => { + container + .querySelector('[data-testid="btw-suggestion-send"]') + ?.click(); + await Promise.resolve(); + }); + + expect(mockSessionActions.btwSession).toHaveBeenCalledWith( + sideQuestion, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + expect(editorCommit).toHaveBeenCalledTimes(1); + + const newTask = '帮我写一篇新的设计文档，主题是 Web Shell 新功能方案'; + mockSessionActions.generateSessionContent.mockImplementation( + async function* () { + yield { + type: 'delta', + requestId: 'req-new-session-after-btw', + seq: 0, + text: JSON.stringify({ + suggestion: 'new_session', + confidence: 0.94, + }), + }; + yield { + type: 'done', + requestId: 'req-new-session-after-btw', + model: 'fast-model', + modelSource: 'fast', + }; + }, + ); + testState.prompt = newTask; + act(() => { + testState.latestChatEditorProps?.onInputTextChange?.(newTask); + }); + await flush(); + act(() => { + vi.advanceTimersByTime(121); + }); + await flush(); + act(() => { + vi.advanceTimersByTime(701); + }); + await flush(); + + expect( + container.querySelector('[data-testid="new-session-suggestion"]'), + ).not.toBeNull(); + }); + + it('refuses a visible BTW suggestion when an inline tag is added before acceptance', async () => { + vi.useFakeTimers(); + mockConnection.capabilities.features = ['session_generation']; + ( + mockConnection as typeof mockConnection & { + tokenCount?: number; + contextWindow?: number; + } + ).tokenCount = 600; + ( + mockConnection as typeof mockConnection & { + tokenCount?: number; + contextWindow?: number; + } + ).contextWindow = 1000; + testState.messages = Array.from({ length: 8 }, (_, index) => ({ + id: `m-btw-attachment-${index}`, + role: index % 2 === 0 ? 'user' : 'assistant', + content: `existing session topic ${index} about daemon generation review work`, + timestamp: index, + })); + testState.prompt = '顺便看看这里为什么会报错？'; + mockSessionActions.generateSessionContent.mockImplementation( + async function* () { + yield { + type: 'delta', + requestId: 'req-btw-attachment', + seq: 0, + text: JSON.stringify({ + suggestion: 'btw', + confidence: 0.95, + }), + }; + yield { + type: 'done', + requestId: 'req-btw-attachment', + model: 'fast-model', + modelSource: 'fast', + }; + }, + ); + + const { container } = renderApp(); + await flush(); + + act(() => { + testState.latestChatEditorProps?.onInputTextChange?.(testState.prompt); + }); + await flush(); + act(() => { + vi.advanceTimersByTime(121); + }); + await flush(); + act(() => { + vi.advanceTimersByTime(701); + }); + await flush(); + + expect( + container.querySelector('[data-testid="btw-suggestion"]'), + ).not.toBeNull(); + + testState.inputAnnotations = [ + { + type: 'reference', + text: '@src/App.tsx', + start: 0, + end: 12, + reference: { + id: 'src/App.tsx', + value: 'src/App.tsx', + serialized: '@src/App.tsx', + }, + }, + ]; + await act(async () => { + container + .querySelector('[data-testid="btw-suggestion-send"]') + ?.click(); + await Promise.resolve(); + }); + + expect(mockSessionActions.btwSession).not.toHaveBeenCalled(); + expect(editorCommit).not.toHaveBeenCalled(); + }); + it('waits for the current session to detach before auto-submitting the suggested new-session draft', async () => { vi.useFakeTimers(); const clear = deferred(); @@ -5570,7 +5791,7 @@ describe('App session callbacks', () => { requestId: 'req-2', seq: 0, text: JSON.stringify({ - shouldSuggestNewSession: true, + suggestion: 'new_session', confidence: 0.91, }), }; @@ -5659,7 +5880,7 @@ describe('App session callbacks', () => { requestId: 'req-stale', seq: 0, text: JSON.stringify({ - shouldSuggestNewSession: true, + suggestion: 'new_session', confidence: 0.91, }), }; @@ -5747,7 +5968,7 @@ describe('App session callbacks', () => { requestId: 'req-switch', seq: 0, text: JSON.stringify({ - shouldSuggestNewSession: true, + suggestion: 'new_session', confidence: 0.91, }), }; diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index 66c0c7b1be..8b8d00d8b8 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -2618,6 +2618,9 @@ export function App({ null, ); const [composerText, setComposerText] = useState(''); + const [hasComposerAttachments, setHasComposerAttachments] = useState< + boolean | null + >(null); const [isStartingNewSessionSuggestion, setIsStartingNewSessionSuggestion] = useState(false); const streamingState = useStreamingState(); @@ -5184,6 +5187,13 @@ export function App({ }, 120); }, []); + const handleComposerAttachmentsChange = useCallback( + (hasAttachments: boolean) => { + setHasComposerAttachments(hasAttachments); + }, + [], + ); + const { suggestion: newSessionSuggestion, dismiss: dismissNewSessionSuggestion, @@ -5200,6 +5210,7 @@ export function App({ : 0, isRunning: streamingState !== 'idle', dialogOpen: interactionBlocked || approvalOverlayActive, + hasAttachments: hasComposerAttachments, generateContent: sessionActions.generateSessionContent, }); @@ -5263,7 +5274,11 @@ export function App({ const handleAcceptNewSessionSuggestion = useCallback(() => { const draft = composerTextRef.current.trim(); if (!draft || isStartingNewSessionSuggestion) return; - if (newSessionSuggestion?.classifiedInput !== draft) { + if ( + newSessionSuggestion?.suggestion !== 'new_session' || + newSessionSuggestion.classifiedInput !== draft || + newSessionSuggestion.sourceSessionId !== connectionRef.current.sessionId + ) { dismissNewSessionSuggestion(); return; } @@ -5303,6 +5318,24 @@ export function App({ suppressNewSessionSuggestion, ]); + const handleAcceptBtwSuggestion = useCallback(() => { + const draft = composerTextRef.current.trim(); + if ( + !draft || + newSessionSuggestion?.suggestion !== 'btw' || + newSessionSuggestion.classifiedInput !== draft || + newSessionSuggestion.sourceSessionId !== + connectionRef.current.sessionId || + editorRef.current?.hasAttachments() !== false + ) { + dismissNewSessionSuggestion(); + return; + } + dismissNewSessionSuggestion(); + editorRef.current?.submit({ text: `/btw ${draft}` }); + editorRef.current?.focus(); + }, [dismissNewSessionSuggestion, newSessionSuggestion]); + const shellApi = useMemo( () => ({ openSplitView: () => { @@ -8653,7 +8686,11 @@ export function App({
- {t('editor.newSessionSuggestionTitle')} + {newSessionSuggestion.suggestion === 'btw' + ? t('editor.btwSuggestionTitle') + : t('editor.newSessionSuggestionTitle')}
@@ -8703,6 +8752,9 @@ export function App({ ref={setEditorHandle} onSubmit={handleEditorSubmit} onInputTextChange={handleComposerTextChange} + onAttachmentsChange={ + handleComposerAttachmentsChange + } onCycleMode={handleCycleMode} onToggleShortcuts={handleToggleShortcuts} onCancel={handleCancel} diff --git a/packages/web-shell/client/components/ChatEditor.test.tsx b/packages/web-shell/client/components/ChatEditor.test.tsx index d54ad3a5c0..efc68ac079 100644 --- a/packages/web-shell/client/components/ChatEditor.test.tsx +++ b/packages/web-shell/client/components/ChatEditor.test.tsx @@ -24,6 +24,7 @@ Element.prototype.scrollIntoView = vi.fn(); const mockComposerCoreState = vi.hoisted(() => ({ composerTags: [] as WebShellComposerTag[], + pastedImages: [] as Array<{ data: string; media_type: string }>, removeTopTag: vi.fn(), })); @@ -129,6 +130,9 @@ vi.mock('../hooks/useComposerCore', async (importOriginal) => { clearText: vi.fn(), getText: vi.fn(() => ''), hasInput: vi.fn(() => false), + hasAttachments: + mockComposerCoreState.pastedImages.length > 0 || + mockComposerCoreState.composerTags.length > 0, hasContent: false, handle: { focus: vi.fn(), @@ -139,8 +143,11 @@ vi.mock('../hooks/useComposerCore', async (importOriginal) => { addTags: vi.fn(), removeInlineTags: vi.fn(), submit: vi.fn(), + hasAttachments: () => + mockComposerCoreState.pastedImages.length > 0 || + mockComposerCoreState.composerTags.length > 0, }, - pastedImages: [], + pastedImages: mockComposerCoreState.pastedImages, removeImage: vi.fn(), composerTags: mockComposerCoreState.composerTags, removeTopTag: mockComposerCoreState.removeTopTag, @@ -224,11 +231,13 @@ afterEach(() => { portalRoot.remove(); } mockComposerCoreState.composerTags = []; + mockComposerCoreState.pastedImages = []; mockComposerCoreState.removeTopTag.mockReset(); }); function renderChatEditor(props: { composerTags?: WebShellComposerTag[]; + pastedImages?: Array<{ data: string; media_type: string }>; gitBranch?: string; workspaceName?: string; workspaceTitle?: string; @@ -240,10 +249,12 @@ function renderChatEditor(props: { availableModels?: Array<{ id: string; label?: string }>; onSelectMode?: (mode: string) => void; onSelectModel?: (model: string) => void; + onAttachmentsChange?: (hasAttachments: boolean) => void; customization?: WebShellCustomization; }) { const { composerTags, + pastedImages, customization, renderComposerTagTooltip, onComposerTagClick, @@ -252,6 +263,9 @@ function renderChatEditor(props: { if (composerTags) { mockComposerCoreState.composerTags = composerTags; } + if (pastedImages) { + mockComposerCoreState.pastedImages = pastedImages; + } const container = document.createElement('div'); container.dataset.webShellRoot = ''; const portalRoot = document.createElement('div'); @@ -307,6 +321,36 @@ describe('ChatEditor voice toolbar integration', () => { }); }); +describe('ChatEditor attachment reporting', () => { + it('reports whether the composer has tags or pasted images', () => { + const onEmptyAttachmentsChange = vi.fn(); + renderChatEditor({ + onAttachmentsChange: onEmptyAttachmentsChange, + }); + expect(onEmptyAttachmentsChange).toHaveBeenLastCalledWith(false); + + const onTaggedAttachmentsChange = vi.fn(); + renderChatEditor({ + composerTags: [ + { + id: 'file:reference', + kind: 'file', + value: 'reference', + }, + ], + onAttachmentsChange: onTaggedAttachmentsChange, + }); + expect(onTaggedAttachmentsChange).toHaveBeenLastCalledWith(true); + + const onImageAttachmentsChange = vi.fn(); + renderChatEditor({ + pastedImages: [{ data: 'abc', media_type: 'image/png' }], + onAttachmentsChange: onImageAttachmentsChange, + }); + expect(onImageAttachmentsChange).toHaveBeenLastCalledWith(true); + }); +}); + describe('ChatEditor composer tag icons', () => { it('renders built-in icons for top composer tags', () => { const kinds = ['extension', 'file', 'mcp', 'skill'] as const; diff --git a/packages/web-shell/client/components/ChatEditor.tsx b/packages/web-shell/client/components/ChatEditor.tsx index b8f5c1cf25..c335a28c87 100644 --- a/packages/web-shell/client/components/ChatEditor.tsx +++ b/packages/web-shell/client/components/ChatEditor.tsx @@ -116,6 +116,7 @@ interface ChatEditorProps { metadata?: ComposerSubmitMetadata, ) => boolean | void; onInputTextChange?: (text: string) => void; + onAttachmentsChange?: (hasAttachments: boolean) => void; onCycleMode?: () => void; onToggleShortcuts?: () => void; onCancel?: () => void; @@ -1161,6 +1162,7 @@ export const ChatEditor = memo( const { onSubmit, onInputTextChange, + onAttachmentsChange, onCycleMode, onToggleShortcuts, onCancel, @@ -1268,6 +1270,10 @@ export const ChatEditor = memo( useImperativeHandle(ref, () => core.handle, [core.handle]); + useEffect(() => { + onAttachmentsChange?.(core.hasAttachments); + }, [core.hasAttachments, onAttachmentsChange]); + const [modeDropdownOpen, setModeDropdownOpen] = useState(false); const [modelDropdownOpen, setModelDropdownOpen] = useState(false); const [quickActionsOpen, setQuickActionsOpen] = useState(false); diff --git a/packages/web-shell/client/hooks/useComposerCore.dom.test.tsx b/packages/web-shell/client/hooks/useComposerCore.dom.test.tsx index 38fd24b157..4f451c2128 100644 --- a/packages/web-shell/client/hooks/useComposerCore.dom.test.tsx +++ b/packages/web-shell/client/hooks/useComposerCore.dom.test.tsx @@ -880,6 +880,28 @@ describe('useComposerCore tags', () => { ).toHaveLength(kinds.length); }); + it('reports inline composer tags as attachments', async () => { + await mount(); + + expect(latest!.handle.hasAttachments()).toBe(false); + expect(latest!.hasAttachments).toBe(false); + + act(() => { + latest!.addTags( + [{ id: 'orders', value: 'orders', serialized: '@orders' }], + { placement: 'inline' }, + ); + }); + expect(latest!.handle.hasAttachments()).toBe(true); + expect(latest!.hasAttachments).toBe(true); + + act(() => { + latest!.removeInlineTags(); + }); + expect(latest!.handle.hasAttachments()).toBe(false); + expect(latest!.hasAttachments).toBe(false); + }); + it('keeps inline tags after trimming leading whitespace on submit', async () => { const { onSubmit } = await mount(); diff --git a/packages/web-shell/client/hooks/useComposerCore.ts b/packages/web-shell/client/hooks/useComposerCore.ts index 7ab0668eff..19fdde3802 100644 --- a/packages/web-shell/client/hooks/useComposerCore.ts +++ b/packages/web-shell/client/hooks/useComposerCore.ts @@ -961,6 +961,7 @@ export interface EditorHandle extends WebShellComposerApi { clearText(): void; focus(): void; getText(): string; + hasAttachments(): boolean; hasInput(): boolean; retryLast(): void; restoreImages(images: readonly PromptImage[]): void; @@ -1268,6 +1269,7 @@ export interface UseComposerCoreReturn { clearText: () => void; getText: () => string; hasInput: () => boolean; + hasAttachments: boolean; hasContent: boolean; handle: EditorHandle; pastedImages: PromptImage[]; @@ -1607,6 +1609,7 @@ export function useComposerCore( const [composerTags, setComposerTags] = useState([]); const composerTagsRef = useRef([]); composerTagsRef.current = composerTags; + const [hasInlineTags, setHasInlineTags] = useState(false); const historyDraftComposerTagsRef = useRef( null, ); @@ -2825,6 +2828,7 @@ export function useComposerCore( triggerCleanupListener, // Update hasContent state when document changes EditorView.updateListener.of((update) => { + setHasInlineTags(getInlineComposerTags(update.view).length > 0); if (update.docChanged) { const text = getDocText(update.state); if (draftIdentityRef.current.storageKey === undefined) { @@ -3553,6 +3557,17 @@ export function useComposerCore( ); }, [isTouchComposer]); + const hasAttachments = useCallback(() => { + const inlineTags = viewRef.current + ? getInlineComposerTags(viewRef.current) + : []; + return ( + inlineTags.length > 0 || + composerTagsRef.current.length > 0 || + pastedImagesRef.current.length > 0 + ); + }, []); + const submit = useCallback( (input?: WebShellComposerInput) => { const view = viewRef.current; @@ -3856,6 +3871,7 @@ export function useComposerCore( clear, focus, getText, + hasAttachments, hasInput, setText, addTags, @@ -3871,6 +3887,7 @@ export function useComposerCore( clearText, focus, getText, + hasAttachments, hasInput, insertText, removeTopTag, @@ -3902,6 +3919,8 @@ export function useComposerCore( clearText, getText, hasInput, + hasAttachments: + hasInlineTags || composerTags.length > 0 || pastedImages.length > 0, hasContent, handle, pastedImages, diff --git a/packages/web-shell/client/hooks/useNewSessionSuggestion.test.tsx b/packages/web-shell/client/hooks/useNewSessionSuggestion.test.tsx index 44bcb4b5df..7e5f192786 100644 --- a/packages/web-shell/client/hooks/useNewSessionSuggestion.test.tsx +++ b/packages/web-shell/client/hooks/useNewSessionSuggestion.test.tsx @@ -29,6 +29,7 @@ const testState = { contextUsageRatio: 0, isRunning: false, dialogOpen: false, + hasAttachments: false as boolean | null, generateContent: vi.fn(async function* () {}), }; @@ -78,6 +79,7 @@ afterEach(async () => { testState.contextUsageRatio = 0; testState.isRunning = false; testState.dialogOpen = false; + testState.hasAttachments = false; testState.generateContent.mockReset(); vi.useRealTimers(); }); @@ -129,7 +131,7 @@ describe('useNewSessionSuggestion', () => { requestId: 'req-1', seq: 0, text: JSON.stringify({ - shouldSuggestNewSession: true, + suggestion: 'new_session', confidence: 0.9, }), }; @@ -149,8 +151,9 @@ describe('useNewSessionSuggestion', () => { expect(testState.generateContent).toHaveBeenCalledOnce(); expect(latestSuggestion).toEqual({ - isVisible: true, + suggestion: 'new_session', classifiedInput: '帮我写一篇新的设计文档，主题是 Web Shell 新功能方案', + sourceSessionId: 'session-1', }); testState.inputText = '顺手补个测试'; @@ -182,10 +185,14 @@ describe('useNewSessionSuggestion', () => { }, ] as Message[]; - async function classify(decisionText: string) { + async function classify( + decisionText: string, + inputText = NEW_TASK_DRAFT, + messages = CONTEXT_MESSAGES, + ) { vi.useFakeTimers(); - testState.inputText = NEW_TASK_DRAFT; - testState.messages = CONTEXT_MESSAGES; + testState.inputText = inputText; + testState.messages = messages; testState.generateContent.mockImplementation(async function* () { yield { type: 'delta', @@ -208,44 +215,137 @@ describe('useNewSessionSuggestion', () => { await flush(3); } + it('classifies a side question after only one prior exchange', async () => { + const sideQuestion = '这里的 confidence 阈值为什么是 0.75？'; + await classify( + JSON.stringify({ suggestion: 'btw', confidence: 0.92 }), + sideQuestion, + ); + + expect(testState.generateContent).toHaveBeenCalledOnce(); + expect(latestSuggestion).toEqual({ + suggestion: 'btw', + classifiedInput: sideQuestion, + sourceSessionId: 'session-1', + }); + }); + + it('lets common side-question wording reach the classifier', async () => { + const sideQuestion = '顺手问下，这里的 confidence 阈值为什么是 0.75？'; + await classify( + JSON.stringify({ suggestion: 'btw', confidence: 0.9 }), + sideQuestion, + ); + + expect(testState.generateContent).toHaveBeenCalledOnce(); + expect(latestSuggestion?.suggestion).toBe('btw'); + }); + + it('does not surface new_session from the relaxed BTW context floor', async () => { + await classify( + JSON.stringify({ suggestion: 'new_session', confidence: 0.96 }), + '这里的 confidence 阈值为什么是 0.75？', + ); + + expect(testState.generateContent).toHaveBeenCalledOnce(); + expect(latestSuggestion).toBeNull(); + }); + + it('does not classify BTW with less than one prior exchange', async () => { + await classify( + JSON.stringify({ suggestion: 'btw', confidence: 0.96 }), + '这里的 confidence 阈值为什么是 0.75？', + CONTEXT_MESSAGES.slice(0, 1), + ); + + expect(testState.generateContent).not.toHaveBeenCalled(); + expect(latestSuggestion).toBeNull(); + }); + + it.each([true, null])( + 'does not classify a low-context side question when attachment presence is %s', + async (hasAttachments) => { + testState.hasAttachments = hasAttachments; + await classify( + JSON.stringify({ suggestion: 'btw', confidence: 0.96 }), + '这里的 confidence 阈值为什么是 0.75？', + ); + + expect(testState.generateContent).not.toHaveBeenCalled(); + expect(latestSuggestion).toBeNull(); + }, + ); + it('recovers a positive decision wrapped in prose (observed live)', async () => { // Verbatim shape from a live run: prose preamble + bare JSON. await classify( 'The user is explicitly switching to a completely new task, which is ' + 'unrelated to the previous discussion. This is a clear topic change.\n\n' + - JSON.stringify({ shouldSuggestNewSession: true, confidence: 0.98 }), + JSON.stringify({ suggestion: 'new_session', confidence: 0.98 }), ); expect(testState.generateContent).toHaveBeenCalledOnce(); expect(latestSuggestion).toEqual({ - isVisible: true, + suggestion: 'new_session', classifiedInput: NEW_TASK_DRAFT, + sourceSessionId: 'session-1', }); }); it('recovers a positive decision inside a code fence', async () => { await classify( '```json\n' + - JSON.stringify({ shouldSuggestNewSession: true, confidence: 0.95 }) + + JSON.stringify({ suggestion: 'new_session', confidence: 0.95 }) + '\n```', ); expect(latestSuggestion).toEqual({ - isVisible: true, + suggestion: 'new_session', classifiedInput: NEW_TASK_DRAFT, + sourceSessionId: 'session-1', }); }); - it('keeps the banner hidden for a prose-wrapped negative decision', async () => { + it('keeps the banner hidden for a valid none decision', async () => { await classify( 'This is a follow-up on the same topic.\n\n' + - JSON.stringify({ shouldSuggestNewSession: false, confidence: 0.97 }), + JSON.stringify({ suggestion: 'none', confidence: 0.97 }), ); expect(testState.generateContent).toHaveBeenCalledOnce(); expect(latestSuggestion).toBeNull(); }); + it('suggests BTW for a side question without attachments', async () => { + await classify(JSON.stringify({ suggestion: 'btw', confidence: 0.92 })); + + expect(latestSuggestion).toEqual({ + suggestion: 'btw', + classifiedInput: NEW_TASK_DRAFT, + sourceSessionId: 'session-1', + }); + }); + + it.each([true, null])( + 'does not suggest BTW when attachment presence is %s', + async (hasAttachments) => { + testState.hasAttachments = hasAttachments; + await classify(JSON.stringify({ suggestion: 'btw', confidence: 0.92 })); + + expect(latestSuggestion).toBeNull(); + }, + ); + + it.each([ + JSON.stringify({ shouldSuggestNewSession: true, confidence: 0.98 }), + JSON.stringify({ suggestion: 'later', confidence: 0.98 }), + JSON.stringify({ suggestion: 'btw', confidence: 1.1 }), + ])('stays fail-closed for an invalid decision: %s', async (decision) => { + await classify(decision); + + expect(latestSuggestion).toBeNull(); + }); + it('stays fail-closed on prose with no recoverable JSON object', async () => { await classify( 'I think {this draft} switches topics, but here is no JSON to parse.', diff --git a/packages/web-shell/client/hooks/useNewSessionSuggestion.ts b/packages/web-shell/client/hooks/useNewSessionSuggestion.ts index 5e538b8cff..f7344b48e4 100644 --- a/packages/web-shell/client/hooks/useNewSessionSuggestion.ts +++ b/packages/web-shell/client/hooks/useNewSessionSuggestion.ts @@ -3,6 +3,7 @@ import type { Message } from '../adapters/types'; import type { DaemonSessionActions } from '@qwen-code/webui/daemon-react-sdk'; const MIN_PROMPT_LENGTH = 12; +const MIN_BTW_MESSAGE_COUNT = 2; const MIN_MESSAGE_COUNT = 8; const MIN_CONTEXT_USAGE_RATIO = 0.35; const MIN_EXPLICIT_CUE_MESSAGE_COUNT = 2; @@ -50,14 +51,17 @@ const EXPLICIT_NEW_TASK_PATTERNS = [ /brainstorm/i, ]; -interface TopicShiftDecision { - shouldSuggestNewSession: boolean; +type SuggestionKind = 'btw' | 'new_session' | 'none'; + +interface ComposerSuggestionDecision { + suggestion: SuggestionKind; confidence: number; } export interface NewSessionSuggestionState { - isVisible: boolean; + suggestion: Exclude; classifiedInput: string; + sourceSessionId: string; } export interface UseNewSessionSuggestionOptions { @@ -68,6 +72,7 @@ export interface UseNewSessionSuggestionOptions { contextUsageRatio: number; isRunning: boolean; dialogOpen: boolean; + hasAttachments: boolean | null; generateContent?: DaemonSessionActions['generateSessionContent']; } @@ -114,16 +119,20 @@ function buildPrompt(params: { currentInput: string; contextUsageRatio: number; messageCount: number; + allowBtw: boolean; + allowNewSession: boolean; }): string { const recent = params.recentMessages .map((message, index) => `${index + 1}. ${message.role}: ${message.text}`) .join('\n'); return [ - "You are deciding whether a user's new message still belongs in the current coding session.", - 'Suggest starting a new session only when the new message is clearly a different task or topic, and continuing in the current session would likely add context noise or wasted token usage.', - 'Be conservative. When in doubt, keep the current session.', - 'Do NOT suggest a new session for follow-up questions, implementation continuations, debugging iterations, review follow-ups, or adjacent design discussion about the same repo, PR, bug, or feature.', - 'Return JSON only with keys: shouldSuggestNewSession (boolean) and confidence (0-1 number).', + "You are deciding how a user's new message should be handled in the current coding session.", + 'Choose "new_session" only when the message is clearly a different task or topic and continuing here would add context noise.', + 'Choose "btw" only for a brief side question that can be answered without changing the main task or adding its answer to the main conversation context.', + 'Choose "none" for follow-ups, implementation continuations, debugging iterations, review follow-ups, and adjacent discussion about the same repo, PR, bug, or feature.', + 'Be conservative. When in doubt, choose "none".', + `Allowed actions: btw=${params.allowBtw ? 'yes' : 'no'}, new_session=${params.allowNewSession ? 'yes' : 'no'}. Never choose an action marked no.`, + 'Return JSON only with keys: suggestion ("btw", "new_session", or "none") and confidence (0-1 number).', '', `Context usage ratio: ${params.contextUsageRatio.toFixed(2)}`, `Visible message count: ${params.messageCount}`, @@ -136,13 +145,26 @@ function buildPrompt(params: { ].join('\n'); } -function tryParseDecision(text: string): TopicShiftDecision | null { +function tryParseDecision(text: string): ComposerSuggestionDecision | null { try { - const parsed = JSON.parse(text) as Partial; - if (typeof parsed.shouldSuggestNewSession !== 'boolean') return null; - if (typeof parsed.confidence !== 'number') return null; + const parsed = JSON.parse(text) as Partial; + if ( + parsed.suggestion !== 'btw' && + parsed.suggestion !== 'new_session' && + parsed.suggestion !== 'none' + ) { + return null; + } + if ( + typeof parsed.confidence !== 'number' || + !Number.isFinite(parsed.confidence) || + parsed.confidence < 0 || + parsed.confidence > 1 + ) { + return null; + } return { - shouldSuggestNewSession: parsed.shouldSuggestNewSession, + suggestion: parsed.suggestion, confidence: parsed.confidence, }; } catch { @@ -150,7 +172,7 @@ function tryParseDecision(text: string): TopicShiftDecision | null { } } -function parseDecision(text: string): TopicShiftDecision | null { +function parseDecision(text: string): ComposerSuggestionDecision | null { const direct = tryParseDecision(text); if (direct) return direct; // Despite the JSON-only instruction, the model sometimes wraps a perfectly @@ -176,6 +198,7 @@ export function useNewSessionSuggestion({ contextUsageRatio, isRunning, dialogOpen, + hasAttachments, generateContent, }: UseNewSessionSuggestionOptions): UseNewSessionSuggestionReturn { const [suggestion, setSuggestion] = @@ -226,22 +249,22 @@ export function useNewSessionSuggestion({ setSuggestion(null); return; } - if (isFollowupLike(trimmed)) { - setSuggestion(null); - return; - } const explicitNewTaskCue = hasExplicitNewTaskCue(trimmed); if (isRunning || dialogOpen) { setSuggestion(null); return; } - if ( - explicitNewTaskCue - ? recentMessages.length < MIN_EXPLICIT_CUE_MESSAGE_COUNT && - contextUsageRatio < MIN_EXPLICIT_CUE_CONTEXT_USAGE_RATIO - : recentMessages.length < MIN_MESSAGE_COUNT && - contextUsageRatio < MIN_CONTEXT_USAGE_RATIO - ) { + const allowBtw = + hasAttachments === false && + recentMessages.length >= MIN_BTW_MESSAGE_COUNT; + const allowNewSession = + !isFollowupLike(trimmed) && + (explicitNewTaskCue + ? recentMessages.length >= MIN_EXPLICIT_CUE_MESSAGE_COUNT || + contextUsageRatio >= MIN_EXPLICIT_CUE_CONTEXT_USAGE_RATIO + : recentMessages.length >= MIN_MESSAGE_COUNT || + contextUsageRatio >= MIN_CONTEXT_USAGE_RATIO); + if (!allowBtw && !allowNewSession) { setSuggestion(null); return; } @@ -259,6 +282,8 @@ export function useNewSessionSuggestion({ currentInput: trimmed, contextUsageRatio, messageCount: recentMessages.length, + allowBtw, + allowNewSession, }); void (async () => { let text = ''; @@ -280,10 +305,16 @@ export function useNewSessionSuggestion({ const decision = parseDecision(text.trim()); if ( decision && - decision.shouldSuggestNewSession && + decision.suggestion !== 'none' && + ((decision.suggestion === 'btw' && allowBtw) || + (decision.suggestion === 'new_session' && allowNewSession)) && decision.confidence >= MIN_CONFIDENCE ) { - setSuggestion({ isVisible: true, classifiedInput: trimmed }); + setSuggestion({ + suggestion: decision.suggestion, + classifiedInput: trimmed, + sourceSessionId: sessionId, + }); return; } setSuggestion(null); @@ -308,6 +339,7 @@ export function useNewSessionSuggestion({ dialogOpen, enabled, generateContent, + hasAttachments, inputText, isRunning, recentMessages, diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 2294fac0c3..17542b25e8 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -881,6 +881,8 @@ const EN: Messages = { 'editor.searchPlaceholder': 'type to search...', 'editor.newSessionSuggestionTitle': 'This looks like a new topic', 'editor.newSessionSuggestionStart': 'Send in new session', + 'editor.btwSuggestionTitle': 'This looks like a side question', + 'editor.btwSuggestionSend': 'Ask with BTW', 'quickActions.open': 'more actions', 'quickActions.title': 'more actions', 'quickActions.mcp': 'MCP', @@ -3353,6 +3355,8 @@ const ZH: Messages = { 'editor.searchPlaceholder': '输入以搜索…', 'editor.newSessionSuggestionTitle': '这条消息看起来像新话题', 'editor.newSessionSuggestionStart': '在新会话发送', + 'editor.btwSuggestionTitle': '这条消息看起来像顺带一问', + 'editor.btwSuggestionSend': '用 BTW 提问', 'quickActions.open': '更多操作', 'quickActions.title': '更多操作', 'quickActions.mcp': 'MCP', diff --git a/scripts/generate-release-notes.js b/scripts/generate-release-notes.js index 9efb09d443..0d6227a624 100644 --- a/scripts/generate-release-notes.js +++ b/scripts/generate-release-notes.js @@ -34,11 +34,7 @@ export function buildPullRequestQuery(numbers) { pr${index}: pullRequest(number: ${number}) { number body - additions - deletions - changedFiles labels(first: 20) { nodes { name } } - files(first: 40) { nodes { path } } }`, ) .join('\n'); @@ -262,14 +258,7 @@ function compactEntry(entry) { return { number: entry.number, title: entry.title, - body: (entry.body || '').slice(0, 3000), - labels: (entry.labels || []).map((label) => - typeof label === 'string' ? label : label.name, - ), - files: (entry.files || []).slice(0, 40), - additions: entry.additions, - deletions: entry.deletions, - changedFiles: entry.changedFiles, + body: (entry.body || '').slice(0, 700), category: classifyChange(entry), }; } @@ -375,15 +364,10 @@ export function enrichEntries(entries, metadata) { const byNumber = new Map(metadata.map((item) => [item.number, item])); return entries.map((entry) => { const details = byNumber.get(entry.number) || {}; - const files = details.files?.nodes || details.files || []; return { ...entry, body: details.body || '', labels: details.labels?.nodes || details.labels || [], - files: files.map((file) => (typeof file === 'string' ? file : file.path)), - additions: details.additions || 0, - deletions: details.deletions || 0, - changedFiles: details.changedFiles || files.length, }; }); } @@ -468,6 +452,7 @@ export function createOpenAiCompleter({ if (remainingMs <= 0) { throw deadlineError(); } + const attemptStartedAt = Date.now(); try { const response = await fetchImpl(endpoint, { method: 'POST', @@ -495,10 +480,16 @@ export function createOpenAiCompleter({ if (typeof content !== 'string' || !content.trim()) { throw new Error(CONTENT_VALIDATION_ERROR_MESSAGE); } + console.error( + `Model ${request.kind} request succeeded in ${Date.now() - attemptStartedAt}ms (prompt ${prompt.user.length} chars).`, + ); return content; } catch (error) { lastError = error; attempt += 1; + console.error( + `Model ${request.kind} request failed after ${Date.now() - attemptStartedAt}ms (prompt ${prompt.user.length} chars): ${escapeWorkflowCommand(error.message)}`, + ); if (Date.now() >= deadline) { throw deadlineError(); } diff --git a/scripts/tests/generate-release-notes.test.js b/scripts/tests/generate-release-notes.test.js index 9d3edb0973..eb19fb88d0 100644 --- a/scripts/tests/generate-release-notes.test.js +++ b/scripts/tests/generate-release-notes.test.js @@ -39,10 +39,6 @@ const entry = (number, title, labels = []) => ({ author: 'alice', labels, body: '', - files: [], - additions: 1, - deletions: 0, - changedFiles: 1, }); describe('parseGeneratedEntries', () => { @@ -240,6 +236,34 @@ describe('generateAiContent', () => { ]); }); + it('sends only title, a bounded body excerpt, and category to the model', async () => { + const long = { ...entry(1, 'feat: long body'), body: 'x'.repeat(5000) }; + const calls = []; + const complete = async (request) => { + calls.push(request); + if (request.kind === 'summaries') { + return JSON.stringify({ + summaries: request.entries.map((item) => ({ + pr: item.number, + summary: 'Summary.', + })), + }); + } + return JSON.stringify({ highlights: [] }); + }; + + await generateAiContent([long], complete); + + const [payload] = calls[0].entries; + expect(Object.keys(payload).sort()).toEqual([ + 'body', + 'category', + 'number', + 'title', + ]); + expect(payload.body).toHaveLength(700); + }); + it('falls back to original titles for an invalid summary batch', async () => { const entries = [entry(1, 'feat: original'), entry(2, 'fix: original')]; const complete = async (request) => { @@ -359,17 +383,13 @@ describe('enrichEntries', () => { number: 1, body: 'Why it matters.', labels: [{ name: 'type/bug' }], - files: [{ path: 'packages/core/a.ts' }], - additions: 3, - deletions: 2, - changedFiles: 1, }, ]); expect(enriched.map((item) => item.number)).toEqual([2, 1]); expect(enriched[0].body).toBe(''); expect(enriched[1].body).toBe('Why it matters.'); - expect(enriched[1].files).toEqual(['packages/core/a.ts']); + expect(enriched[1].labels).toEqual([{ name: 'type/bug' }]); }); }); @@ -379,7 +399,8 @@ describe('buildPullRequestQuery', () => { expect(query).toContain('pr0: pullRequest(number: 12)'); expect(query).toContain('pr1: pullRequest(number: 8)'); - expect(query).toContain('files(first: 40)'); + expect(query).toContain('labels(first: 20)'); + expect(query).not.toContain('files(first: 40)'); expect(query).not.toContain('pullRequest(number: undefined)'); }); }); @@ -480,7 +501,7 @@ describe('generateReleaseNotes', () => { ' process.exit(0);', '}', "if (args[0] === 'api' && args[1] === 'graphql') {", - " process.stdout.write(JSON.stringify({ data: { repository: { pr0: { number: 1, body: 'Body.', additions: 1, deletions: 0, changedFiles: 1, labels: { nodes: [] }, files: { nodes: [] } } } } }));", + " process.stdout.write(JSON.stringify({ data: { repository: { pr0: { number: 1, body: 'Body.', labels: { nodes: [] } } } } }));", ' process.exit(0);', '}', 'process.exit(1);', diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index e34b22b96e..094e4ebabd 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -2289,13 +2289,14 @@ describe('qwen-autofix workflow', () => { '--json headRefName,headRefOid,statusCheckRollup,createdAt,labels', ); // Command-style comments are instructions, not feedback — excluded at - // ALL FOUR feedback sites (scan count via $cf; NEWEST, LIVE_NEW, and - // the renderer inline) so /triage-, /review-, and /takeover-style + // ALL FIVE feedback sites (scan count via $cf; NEWEST, LIVE_NEW, + // Critical-only deferral rendering, and the renderer inline) so /triage-, + // /review-, and /takeover-style // invocations never burn an agent cycle on a no-action report. expect(reviewScanJob).toContain("COMMAND_FILTER='^\\s*@qwen-code /'"); expect(reviewScanJob).toContain('test($cf) | not'); expect(workflow.split('test("^\\\\s*@qwen-code /") | not').length - 1).toBe( - 3, + 4, ); }); @@ -3204,25 +3205,420 @@ describe('qwen-autofix workflow', () => { expect(broken.body).toBe(''); }); - it('treats Suggestion-level review findings as actionable feedback', () => { - // AGENTS.md: Suggestions ARE addressed during a PR's first ~5 review - // rounds; only past that are they deferred with a recorded reason. The - // loop's MAX_ROUNDS cap is that same boundary, so every round the loop - // runs is within the address-Suggestions window — the scan and the - // feedback rendering must NOT filter `**[Suggestion]**` /review comments. - expect(workflow).not.toContain('QWEN_SUGGESTION_FILTER'); - // The filter REGEX (escaped form only ever appears in filter code, not in - // prose comments) must be gone from both the scan and the feedback render. - expect(workflow).not.toContain('\\*\\*\\[Suggestion\\]\\*\\*'); - // The agent-facing policy lives in the SKILL: implement valuable - // suggestions, decline only with a recorded per-finding reason. + it('switches to Critical-only feedback after five change rounds', () => { + // ROUND counts change-producing rounds, so 4 still starts the fifth + // suggestion-capable change while 5 starts the first Critical-only round. + expect(workflow).toContain("CRITICAL_ONLY_AFTER_ROUND: '5'"); + expect(prepareBranchAndFeedbackStep).toContain( + '[[ "${ROUND}" -ge "${CRITICAL_ONLY_AFTER_ROUND}" ]]', + ); + const modeBlock = prepareBranchAndFeedbackStep.match( + /(CRITICAL_ONLY='false'\n\s+if \[\[ "\$\{ROUND\}" -ge "\$\{CRITICAL_ONLY_AFTER_ROUND\}" \]\]; then\n\s+CRITICAL_ONLY='true'\n\s+fi)/, + )?.[1]; + expect(modeBlock).toBeTruthy(); + const modeAt = (round) => + execFileSync( + 'bash', + [ + '-c', + `ROUND=${round}\nCRITICAL_ONLY_AFTER_ROUND=5\n${modeBlock}\nprintf '%s' "$CRITICAL_ONLY"`, + ], + { encoding: 'utf8' }, + ); + expect(modeAt(4)).toBe('false'); + expect(modeAt(5)).toBe('true'); + + // Once the boundary is crossed, only an explicit Critical inline finding + // or a formal changes-requested review is actionable. Suggestion and + // unclassified comments stay open instead of driving more code changes. + expect(prepareBranchAndFeedbackStep).toContain('CRITICAL_ONLY'); + expect(prepareBranchAndFeedbackStep).toContain('**[Critical]**'); + const inlineFilter = prepareBranchAndFeedbackStep.match( + /echo "## Inline comments"[\s\S]*?jq -rs --arg wm "\$\{WATERMARK\}"[\s\S]*?--slurpfile reviews "\$\{WORKDIR\}\/rv\.json" '([\s\S]*?)' \\\n\s+"\$\{WORKDIR\}\/rc\.json"/, + )?.[1]; + expect(inlineFilter).toBeTruthy(); + const inlineFeedback = [ + { + id: 10, + created_at: '2025-12-31T00:00:00Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: '**[Critical]** stale owner routes writes to the wrong runtime', + }, + { + id: 11, + created_at: '2026-01-02T00:00:00Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: '**[Critical]** wrong workspace is mutated', + }, + { + id: 12, + created_at: '2026-01-02T00:00:01Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: '**[Suggestion]** add an aria-label', + }, + { + id: 13, + created_at: '2026-01-02T00:00:02Z', + user: { login: 'maintainer' }, + author_association: 'MEMBER', + body: 'Could this helper be renamed?', + }, + { + id: 14, + in_reply_to_id: 10, + created_at: '2026-01-02T00:00:03Z', + user: { login: 'maintainer' }, + author_association: 'MEMBER', + body: 'This still routes through the legacy primary.', + }, + { + id: 15, + pull_request_review_id: 20, + created_at: '2026-01-02T00:00:04Z', + user: { login: 'maintainer' }, + author_association: 'MEMBER', + body: 'The null branch still crashes.', + }, + ]; + const reviews = [ + { + id: 20, + state: 'CHANGES_REQUESTED', + }, + ]; + const countInline = (criticalOnly) => + Number( + execFileSync( + 'jq', + [ + '-s', + '--arg', + 'wm', + '2026-01-01T00:00:00Z', + '--arg', + 'rb', + 'qwen-code-ci-bot', + '--arg', + 'ab', + 'qwen-code-dev-bot', + '--argjson', + 'critical_only', + String(criticalOnly), + '--argjson', + 'trust', + '["OWNER","MEMBER","COLLABORATOR"]', + '--argjson', + 'reviews', + JSON.stringify([reviews]), + `[\n${inlineFilter}\n] | length`, + ], + { + encoding: 'utf8', + input: JSON.stringify(inlineFeedback), + }, + ), + ); + expect(countInline(false)).toBe(5); + expect(countInline(true)).toBe(3); + + // Actionable reviews and issue-level comments filters: extract and + // execute against fixture data with critical_only both ways, mirroring + // the inline filter test above. + const actionableReviewsFilter = prepareBranchAndFeedbackStep.match( + /echo "## Reviews"[\s\S]*?jq -r --arg wm "\$\{WATERMARK\}" --arg rb "\$\{REVIEW_BOT\}" --arg ab "\$\{AUTOFIX_BOT\}" \\\n\s+--argjson critical_only "\$\{CRITICAL_ONLY\}" --argjson trust "\$\{TRUSTED_ASSOC\}" '([\s\S]*?)' \\\n\s+"\$\{WORKDIR\}\/rv\.json"/, + )?.[1]; + expect(actionableReviewsFilter).toBeTruthy(); + const actionableReviews = [ + { + id: 20, + state: 'CHANGES_REQUESTED', + submitted_at: '2026-01-02T00:00:00Z', + user: { login: 'maintainer' }, + author_association: 'MEMBER', + body: 'The null branch still crashes.', + }, + { + id: 21, + state: 'COMMENTED', + submitted_at: '2026-01-02T00:00:01Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: 'Looks good overall', + }, + { + id: 22, + state: 'COMMENTED', + submitted_at: '2026-01-02T00:00:02Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: '**[Critical]** memory leak in the owner route', + }, + ]; + const countActionableReviews = (criticalOnly) => + Number( + execFileSync( + 'jq', + [ + '--arg', + 'wm', + '2026-01-01T00:00:00Z', + '--arg', + 'rb', + 'qwen-code-ci-bot', + '--arg', + 'ab', + 'qwen-code-dev-bot', + '--argjson', + 'critical_only', + String(criticalOnly), + '--argjson', + 'trust', + '["OWNER","MEMBER","COLLABORATOR"]', + `[${actionableReviewsFilter}] | length`, + ], + { encoding: 'utf8', input: JSON.stringify(actionableReviews) }, + ), + ); + // All three are actionable while suggestions are in scope; in + // Critical-only mode the non-Critical COMMENTED review is excluded. + expect(countActionableReviews(false)).toBe(3); + expect(countActionableReviews(true)).toBe(2); + + const actionableIssueFilter = prepareBranchAndFeedbackStep.match( + /echo "## Issue-level comments"[\s\S]*?jq -r --arg wm "\$\{WATERMARK\}" --arg rb "\$\{REVIEW_BOT\}" --arg ab "\$\{AUTOFIX_BOT\}" \\\n\s+--argjson critical_only "\$\{CRITICAL_ONLY\}" --argjson trust "\$\{TRUSTED_ASSOC\}" '([\s\S]*?)' \\\n\s+"\$\{WORKDIR\}\/ic\.json"/, + )?.[1]; + expect(actionableIssueFilter).toBeTruthy(); + const actionableIssueComments = [ + { + id: 30, + created_at: '2026-01-02T00:00:00Z', + user: { login: 'maintainer' }, + author_association: 'MEMBER', + body: 'Please also update the docs.', + }, + { + id: 31, + created_at: '2026-01-02T00:00:01Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: '**[Critical]** data loss on concurrent writes', + }, + { + id: 32, + created_at: '2026-01-02T00:00:02Z', + user: { login: 'maintainer' }, + author_association: 'MEMBER', + body: '@qwen-code /review', + }, + ]; + const countActionableIssue = (criticalOnly) => + Number( + execFileSync( + 'jq', + [ + '--arg', + 'wm', + '2026-01-01T00:00:00Z', + '--arg', + 'rb', + 'qwen-code-ci-bot', + '--arg', + 'ab', + 'qwen-code-dev-bot', + '--argjson', + 'critical_only', + String(criticalOnly), + '--argjson', + 'trust', + '["OWNER","MEMBER","COLLABORATOR"]', + `[${actionableIssueFilter}] | length`, + ], + { encoding: 'utf8', input: JSON.stringify(actionableIssueComments) }, + ), + ); + // Normal and Critical comments are actionable while suggestions are in + // scope; the command-style comment is always excluded. In Critical-only + // mode, only the Critical comment remains. + expect(countActionableIssue(false)).toBe(2); + expect(countActionableIssue(true)).toBe(1); + + // Deferred queries: extract and execute against fixture data, + // mirroring the actionable inline filter test above. + const deferredReviewsFilter = prepareBranchAndFeedbackStep.match( + /## Deferred non-Critical feedback[\s\S]*?jq -r --arg wm "\$\{WATERMARK\}" --arg rb "\$\{REVIEW_BOT\}" --arg ab "\$\{AUTOFIX_BOT\}" \\\n\s+--argjson trust "\$\{TRUSTED_ASSOC\}" --arg pr_url "\$\{PR_URL\}" '([\s\S]*?)' \\\n\s+"\$\{WORKDIR\}\/rv\.json"/, + )?.[1]; + expect(deferredReviewsFilter).toBeTruthy(); + const deferredReviews = [ + ...reviews, + { + id: 21, + state: 'COMMENTED', + submitted_at: '2026-01-02T00:00:00Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: 'Looks good overall', + html_url: 'https://github.com/test/pull/1#review-21', + }, + { + id: 22, + state: 'COMMENTED', + submitted_at: '2026-01-02T00:00:01Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: '**[Critical]** memory leak in the owner route', + html_url: 'https://github.com/test/pull/1#review-22', + }, + ]; + const countDeferredReviews = Number( + execFileSync( + 'jq', + [ + '--arg', + 'wm', + '2026-01-01T00:00:00Z', + '--arg', + 'rb', + 'qwen-code-ci-bot', + '--arg', + 'ab', + 'qwen-code-dev-bot', + '--argjson', + 'trust', + '["OWNER","MEMBER","COLLABORATOR"]', + '--arg', + 'pr_url', + 'https://github.com/test/pull/1', + `[${deferredReviewsFilter}] | length`, + ], + { encoding: 'utf8', input: JSON.stringify(deferredReviews) }, + ), + ); + // COMMENTED non-Critical review is deferred; CHANGES_REQUESTED and + // COMMENTED Critical reviews are not. + expect(countDeferredReviews).toBe(1); + + const deferredInlineFilter = prepareBranchAndFeedbackStep.match( + /jq -rs --arg wm "\$\{WATERMARK\}" --arg rb "\$\{REVIEW_BOT\}" --arg ab "\$\{AUTOFIX_BOT\}" \\\n\s+--argjson trust "\$\{TRUSTED_ASSOC\}" --arg pr_url "\$\{PR_URL\}" \\\n\s+--slurpfile reviews "\$\{WORKDIR\}\/rv\.json" '([\s\S]*?)' \\\n\s+"\$\{WORKDIR\}\/rc\.json"/, + )?.[1]; + expect(deferredInlineFilter).toBeTruthy(); + const countDeferredInline = Number( + execFileSync( + 'jq', + [ + '-s', + '--arg', + 'wm', + '2026-01-01T00:00:00Z', + '--arg', + 'rb', + 'qwen-code-ci-bot', + '--arg', + 'ab', + 'qwen-code-dev-bot', + '--argjson', + 'trust', + '["OWNER","MEMBER","COLLABORATOR"]', + '--arg', + 'pr_url', + 'https://github.com/test/pull/1', + '--argjson', + 'reviews', + JSON.stringify([reviews]), + `[\n${deferredInlineFilter}\n] | length`, + ], + { encoding: 'utf8', input: JSON.stringify(inlineFeedback) }, + ), + ); + // Suggestion (id 12) and unclassified (id 13) are deferred; Critical + // (11), reply-to-Critical (14), and CHANGES_REQUESTED-associated (15) + // are not. + expect(countDeferredInline).toBe(2); + + const deferredIssueFilter = prepareBranchAndFeedbackStep.match( + /"\$\{WORKDIR\}\/rc\.json"\n\s+jq -r --arg wm "\$\{WATERMARK\}" --arg rb "\$\{REVIEW_BOT\}" --arg ab "\$\{AUTOFIX_BOT\}" \\\n\s+--argjson trust "\$\{TRUSTED_ASSOC\}" --arg pr_url "\$\{PR_URL\}" '([\s\S]*?)' \\\n\s+"\$\{WORKDIR\}\/ic\.json"/, + )?.[1]; + expect(deferredIssueFilter).toBeTruthy(); + const issueComments = [ + { + id: 30, + created_at: '2026-01-02T00:00:00Z', + user: { login: 'maintainer' }, + author_association: 'MEMBER', + body: 'Please also update the docs.', + html_url: 'https://github.com/test/pull/1#issuecomment-30', + }, + { + id: 31, + created_at: '2026-01-02T00:00:01Z', + user: { login: 'qwen-code-ci-bot' }, + author_association: 'NONE', + body: '**[Critical]** data loss on concurrent writes', + html_url: 'https://github.com/test/pull/1#issuecomment-31', + }, + { + id: 32, + created_at: '2026-01-02T00:00:02Z', + user: { login: 'maintainer' }, + author_association: 'MEMBER', + body: '@qwen-code /review', + html_url: 'https://github.com/test/pull/1#issuecomment-32', + }, + ]; + const countDeferredIssue = Number( + execFileSync( + 'jq', + [ + '--arg', + 'wm', + '2026-01-01T00:00:00Z', + '--arg', + 'rb', + 'qwen-code-ci-bot', + '--arg', + 'ab', + 'qwen-code-dev-bot', + '--argjson', + 'trust', + '["OWNER","MEMBER","COLLABORATOR"]', + '--arg', + 'pr_url', + 'https://github.com/test/pull/1', + `[${deferredIssueFilter}] | length`, + ], + { encoding: 'utf8', input: JSON.stringify(issueComments) }, + ), + ); + // Normal comment is deferred; Critical and command-style comments are + // not. + expect(countDeferredIssue).toBe(1); + + // CHANGES_REQUESTED is a formal merge blocker, so its review summary and + // associated inline details remain actionable even without the marker. + expect(prepareBranchAndFeedbackStep).toContain( + 'or (.state // "") == "CHANGES_REQUESTED"', + ); + expect(inlineFilter).toContain('pull_request_review_id'); + + // Scan still selects fresh suggestions so a no-op report can advance the + // watermark; prepare hides their bodies from the agent and the + // deterministic report records links to the items left open. + expect(reviewScanJob).not.toContain('CRITICAL_ONLY'); + expect(prepareBranchAndFeedbackStep).toContain( + '## Deferred non-Critical feedback', + ); + expect(prepareBranchAndFeedbackStep).toContain('deferred-feedback.md'); + expect(pushAndReportStep).toContain('deferred-feedback.md'); + + // The agent-facing policy is an independent second guard: even if someone + // later changes the rendering, a declared Critical-only round must never + // modify code for the deferred section. const skill = readAutofixSkill(); - expect(skill).toContain('never'); - expect(skill).toContain('drop one silently'); - // A third disposition beyond fix/decline: escalate a judgment that is the - // maintainer's to make, instead of silently deciding it. - expect(skill).toContain("Needs a maintainer's decision"); - expect(skill).toContain('escalate when the'); + expect(skill).toContain('Critical-only mode'); + expect(skill).toContain('do not modify code'); + expect(skill).toContain('Deferred non-Critical feedback'); }); it('requires the address path to run verification and record it as evidence', () => { @@ -4764,10 +5160,12 @@ describe('qwen-autofix workflow', () => { '::warning::Failed to post handoff comment on PR #${PR}', ); expect(reviewAddressReportStep).toContain('human should take over'); - // Token-breaking neutralization at ALL SIX agent-derived publish sites + // Token-breaking neutralization at ALL EIGHT agent-derived publish sites // (address-summary, no-action, DETAIL_FILE, API_ERROR_DETAIL, the - // gate-rejection body, and the comment-reply body, whose content is - // agent stdout that can echo external comment text), and it + // gate-rejection body, the comment-reply body whose content is agent + // stdout that can echo external comment text, and the two + // deferred-feedback report sections, which render untrusted + // review-comment paths into a bot-authored comment), and it // must be LINE-INDEPENDENT: a whole-comment strip misses a marker whose // --> sits on another line, while jq scan() matches across newlines. // Proven end-to-end on a split forged marker. @@ -4776,7 +5174,7 @@ describe('qwen-autofix workflow', () => { // backslashes — a NO-OP on both GNU and BSD sed, verified) left the count // at four and this test green, shipping an unescaped publish site. const escapeSites = workflow.match(/sed 's\/