feat(cli): allow long /goal conditions - #6665
Merged
Merged
Conversation
DragonnZhang
marked this pull request as ready for review
July 10, 2026 09:51
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: This is a real, documented product limitation — the fixed 4,000-character cap on Direction: Aligned. Removing an arbitrary fixed limit and letting model context windows be the natural bound is a sensible product decision. The issue is P2-prioritized and on the roadmap (#4228). No concerns about scope — this doesn't touch auth, sandbox, or public contract areas. Size: 13 production lines (one file), 50 test lines (two files). Core path production changes: 0 (only a test file in Approach: Clean and minimal. The diff does exactly one thing: removes 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个真实存在且有据可查的产品限制—— 方向：对齐。移除任意固定限制、让模型上下文窗口作为自然边界是合理的产品决策。Issue 已列为 P2 优先级并与路线图 #4228 关联。不涉及 auth、sandbox 或公开接口等敏感区域，无顾虑。 规模：13 行生产代码（1 个文件），50 行测试代码（2 个文件）。核心路径生产变更：0（仅 方案：简洁且聚焦。diff 只做一件事：移除 — Qwen Code · qwen3.7-max |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
DragonnZhang
enabled auto-merge
July 10, 2026 10:02
Collaborator
2a. Code ReviewThe diff matches my independent proposal exactly: remove No correctness bugs, security issues, or AGENTS.md violations. The change is 13 production lines in a single file — about as minimal as it gets. Unit tests pass: 2b. Real-Scenario TestingBefore (installed build — qwen 0.19.8)CLI exits immediately with the 4,000-character rejection. After (this PR — npm run dev)The 4,010-character condition is accepted — no length error. The CLI proceeds past the (now-removed) length check into the normal hooks/tool execution flow. The tool-approval warnings are unrelated to the goal condition; they're about non-interactive mode not being able to auto-execute model tool calls. Verdict: Before/after confirms the fix works as described. The condition that was previously rejected at the command boundary is now accepted and flows through to the agent execution pipeline. 中文说明2a. 代码审查diff 与我的独立方案完全一致：移除 无正确性 bug、安全问题或 AGENTS.md 违规。生产代码变更仅 13 行，分布在单个文件中——极其精简。 单元测试通过： 2b. 真实场景测试修改前（已安装的 qwen 0.19.8）CLI 立即退出并显示 4,000 字符拒绝信息。 修改后（本 PR — npm run dev）4,010 字符 condition 被接受——无长度错误。CLI 通过已移除的长度检查，进入正常的 hooks/tool 执行流程。工具审批警告与 goal condition 无关，是非交互模式下的正常行为。 结论： Before/after 确认修复有效。之前被命令边界拒绝的 condition 现在被接受并流入 agent 执行管道。 — Qwen Code · qwen3.7-max |
Collaborator
|
This PR is the cleanest kind of change: removing a limit that shouldn't exist, with zero new logic introduced. The code is simpler after than before — fewer lines, fewer branches, one less constant to maintain. The problem is real and well-evidenced. Issue #6663 makes a compelling case: complex tasks need detailed requirements, checklists, and verification criteria that exceed 4,000 characters, and a file reference isn't equivalent because the one-shot judge has no tools to read files. The fixed cap was a product limit, not a model limit. The implementation matches my independent proposal exactly — remove the constant, delete the check, renumber the branches. The regression coverage is thorough: parameterized tests across all three execution modes verify the full condition reaches active goal state, the UI card, and the judge prompt. The existing transcript-truncation test continues to pass, confirming the evidence bounds are untouched. Before/after tmux testing confirms the behavior: installed No reservations. Ships the feature cleanly. 中文说明这个 PR 是最干净的那种变更：移除一个不应存在的限制，且未引入任何新逻辑。修改后的代码比修改前更简洁——更少的行数、更少的分支、更少的常量。 问题是真实的且有充分证据。Issue #6663 有力地论证了：复杂任务需要超过 4,000 字符的详细需求、检查清单和验证标准，而文件引用不等价，因为一次性 judge 没有工具读取文件。固定上限是产品限制，不是模型限制。 实现与我的独立方案完全一致——移除常量、删除检查、重编号分支。回归覆盖充分：三种执行模式的参数化测试验证了完整 condition 到达 active goal state、UI 卡片和 judge prompt。现有的 transcript 截断测试继续通过，确认证据边界未变。 Before/after tmux 测试确认了行为：已安装的 无顾虑。干净利落地交付了功能。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jul 10, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/cli/src/ui/commands/goalCommand.ts:163 |
No replacement guardrail after removing MAX_GOAL_LENGTH. The 4000-char cap was the only input-size guard. After removal, conditions are unbounded and flow into the initial model prompt, continuation prompts (up to 50 iterations), and judge prompts (up to 50 iterations). A 100K-char condition could consume ~2.5M tokens across the loop lifetime. |
Add a higher but still meaningful cap (e.g., 16K or 32K chars) as a safety net, or add a soft warning above a generous threshold while keeping a hard ceiling. |
packages/cli/src/serve/routes/scheduled-tasks.ts:55 |
Inconsistency with MAX_CONDITION_LENGTH precedent. The scheduled-tasks route enforces MAX_CONDITION_LENGTH = 10_000 with a comment explaining the condition is re-sent on every fire. The /goal system has the identical re-send pattern but now has no cap at all. |
Align the two systems — either introduce a comparable cap in the goal path or add a cross-reference comment explaining why they differ. |
— qwen3.7-max via Qwen Code /review
wenshao
added a commit
to wenshao/qwen-code
that referenced
this pull request
Jul 10, 2026
…hell QwenLM#6665 removed the 4,000-character cap `/goal` applied when setting a goal, but the restore path and the Web Shell form still enforced it. After merging main that split the surfaces: a long condition `/goal` now accepts was persisted as a `set` card, then refused by `restoreGoalFromHistory` on the next resume and dropped from the replay entirely — the goal died on reload and the user never saw a card explaining why. Remove the cap everywhere rather than reinstate it at set time. A corrupted or hand-edited transcript can now restore an arbitrarily long condition, but that is exactly what `/goal` itself permits, so it is no longer a distinct risk. The empty-condition gate stays: it is the one case that is meaningless rather than merely large. - `goalConditionBlockedBy` rejects only an empty condition. - `HistoryReplayer` no longer skips long goal cards. - `GoalsDialog` drops the form check and the `maxLength` attribute, which had been silently truncating a long condition before the user could submit it. - `MAX_GOAL_LENGTH` and the now-orphaned `goals.error.tooLong` i18n strings are deleted, along with the drift test's length half; the clear-keyword half of that test still guards the constant that is genuinely duplicated. Also drops the `MAX_GOAL_LENGTH` import QwenLM#6665 left unused in `goalCommand.ts`, which failed `eslint --max-warnings 0`.
pull Bot
pushed a commit
to AmirulAndalib/qwen-code
that referenced
this pull request
Jul 18, 2026
… daemon resume (QwenLM#6561) * fix(goals): persist goal cards and restore the hook on daemon resume In daemon mode a `/goal` was silently lost whenever its session was reloaded or `qwen serve` restarted: the goal card vanished from the transcript and the Stop hook was never re-registered, so the loop simply stopped advancing. The TUI does neither of these things wrong; the ACP path was missing both halves. Goal cards were only ever emitted as live SSE `_meta` (MessageEmitter's emitGoalStatus / emitGoalTerminal) and never written to the transcript, so the one durable store — the ChatRecord JSONL — had nothing to restore from. Record them from Session.emitGoalStatus, the single choke point for `set` and `cleared` (the sessionGoalClear ext method routes through it too), and from the goal terminal observer for `achieved` / `failed` / `aborted`. Persisting `cleared` matters on its own: without it the last stored card stays `set`, and a later resume would revive a goal the user explicitly dropped. HistoryReplayer dropped those records on the way back out — it reads only `item['text']`, and a goal card has no `text` field — so re-emit them as `_meta.goalStatus`. Per-iteration `checking` cards are skipped: a TUI transcript stores one per stop-hook turn and clients suppress them as noise. That costs no fidelity, because restore reads the records directly rather than the replay output. With the transcript carrying the goal again, add #restoreGoalOnResume to loadSession and unstable_resumeSession, alongside #restoreWorktreeOnResume. It rebuilds the goal cards from the resumed ChatRecords (they live inside system/slash_command records' outputHistoryItems) and reuses the existing findGoalToRestore / findLastTerminalGoal / registerGoalHook logic, trust and hook-policy gates included. * feat(web-shell): add a workspace Goals page `/goal` had no visual surface in the web shell. You could set and clear one from the composer, but the only feedback was a status-bar pill and a transcript card, and there was no way to see every goal running in the workspace at once. Add a full-pane Goals page alongside Scheduled Tasks. Each row shows the condition, the session driving it, whether the loop is mid-turn, the judge's turn count and last verdict, and how long the goal has been running. A row opens its session — the transcript IS the goal's history — or clears the goal. A form starts a new goal in a fresh session, so the loop doesn't take over a conversation already in progress. Reading the goals needs a round trip. They live in the owning `qwen --acp` child's in-memory store, and serve runs in a separate process holding only a bridge, so there is nothing local to read. Add a `sessionGoalGet` ext method that reports one session's goal state, wrap it in bridge.getSessionGoal (mirroring clearSessionGoal), and have `GET /goals` fan out over the workspace's live sessions concurrently — one timeout for a wedged child rather than one per session. A session whose probe rejects is dropped rather than failing the whole list. Clearing reuses `POST /session/:id/goal/clear`, so the page and a `/goal clear` typed in chat take the same path through the daemon. Only loaded sessions appear, which is the honest answer rather than a limitation: a goal advances only while its session is resident. Three entry points: a sidebar button, the status-bar goal pill (now a button), and a bare `/goal`, which opens the page instead of asking the daemon to print its status as text — matching how `/schedule` behaves. It sends no prompt and touches no session, so it works mid-turn too. `/goal <condition>` and `/goal clear` are unchanged. The integration test exercises the whole chain against a real daemon: `GET /goals` -> bridge -> ext method in a spawned `qwen --acp` child. * fix(web-shell): stop the Goals poll from overlapping itself `GET /goals` fans out one ext-method probe per live session, and a wedged child holds it for the bridge's 10s `initTimeoutMs` — the same order as the 10s poll interval. `withActionTimeout` rejects the wait at 30s but never aborts the underlying fetch, so a fixed `setInterval` could stack several fan-outs against an already-struggling daemon. `reloadSeqRef` only keeps a stale response from overwriting state; it does nothing about the pile-up. Replace the interval with a single self-chaining loop that owns both the initial load and the polling, scheduling each fetch only once the previous one has settled. Folding the mount load into the chain matters: left in its own effect, the first timer would still fire while it was in flight. Reported by Copilot on QwenLM#6561. * fix(goals): address review — clear-keyword condition, silent failures, theme vars From the /review suggestions on QwenLM#6561. Applied the ones that held up under verification; the rest are answered in the PR thread with evidence. - The New goal form accepted a clear keyword as a condition. It travels as `/goal <condition>`, so "clear" (or stop/off/reset/none/cancel) reached the daemon as a clear command: the fresh session dropped its own goal the instant it was set, with nothing to show for it. Reject it in the form. The keyword list and `/goal` arg parsing move to `utils/goalCondition.ts` so the page and App share one definition instead of the page reaching into App. - Starting a goal failed silently. `onCreateGoal` switches to the chat view first, which unmounts the Goals page, so the inline form error that `sendPrompt` rejection produced was dropped by the page's own unmount guard. Surface it as a toast instead. - `GoalsDialog.module.css` used `var(--destructive, #dc2626)`, but nothing defines `--destructive`; the hardcoded fallback stayed the same red in both themes. Use `--error-color` and match ScheduledTasksDialog's focus outline. - `recordGoalStatusItem` swallowed recording failures with a bare `catch {}`. Silently losing that write is precisely the failure this recording exists to prevent, so log it. - `GET /goals` dropped failed probes silently — an empty page and a page whose probes all failed look identical to the client. Log the dropped sessions and their reasons. Tests: clear-keyword and MAX_GOAL_LENGTH form validation, goalCondition unit tests, `sessionGoalGet` argument validation, session load surviving a throwing goal restore, `/goals` drop logging, and a regression test showing `/goal clear` sent as a prompt does persist its cleared card (a reviewer flagged this as missing; it is not). * fix(goals): cap restored conditions, keep goal-creation errors on screen Second round of review on QwenLM#6561. - `restoreGoalFromHistory` re-registered whatever condition the transcript held, skipping the 4000-char cap `/goal` enforces at set time. A transcript is a file: a corrupted or hand-edited `condition` would ride along in every judge call and continuation prompt for the rest of the session. Gate it alongside the existing trust and hook-policy gates. `MAX_GOAL_LENGTH` moves to `restoreGoal.ts` and `goalCommand.ts` imports it — the reverse direction would be a cycle, since goalCommand already depends on this module. - Starting a goal switched to the chat view before awaiting `sendPrompt`, which unmounted the Goals page. The previous commit routed the rejection to a toast, but the better fix is not to leave: switch views only once the prompt is admitted, so the error lands in the form the user is looking at. `GoalsDialog` keeps a toast fallback for the case where the page is closed while the prompt is still in flight. - Move the `debugLogger` declaration below the imports in `restoreGoal.ts`. Imports are hoisted so this compiled, but a statement wedged between two import blocks is not something to leave behind. * fix(goals): surface restore/record failures, report unprobed sessions Third round of review on QwenLM#6561. - `debugLogger.warn` no-ops unless a debug session is active (`debugLogger.ts:216`), so a failed goal restore and a failed goal-card write were both invisible in production — the two failure modes this PR exists to fix. Promote them to `writeStderrLine`, which both `ui/App.tsx` and `session/Session.ts` already use. - `GET /goals` now returns `droppedCount`. A brownout in which every probe fails returned `{ goals: [] }`, indistinguishable from a workspace with no goals — so the user re-creates goals that are already running. The Goals page shows a notice when the list is incomplete. - `running` on the wire is really "the owning session is mid-turn", which a manual prompt in that session also sets. Renamed to `hasActivePrompt` so the field reports what the daemon actually knows. The UI still maps it to Working/Waiting. - Fix the stale "keep in sync" pointer in `goalCommand.ts`: the clear keywords moved from `App.tsx` to `utils/goalCondition.ts` in the previous commit. Tests for the four coverage gaps the review named: the `systemMessage` fallback in `goalTerminalEventToHistoryItem` (including the known lossy collapse when both fields are set), `#restoreGoalOnResume` on an empty transcript, `listGoals`/`clearGoal` in `actions.ts`, and the `sendPrompt`-after- `createNewSession` failure path (added last commit). Plus `droppedCount` projection and the degradation notice. * test(goals): update the /goals integration test for droppedCount Adding `droppedCount` to the `GET /goals` payload broke the end-to-end assertions, which still expected `{ v: 1, goals: [] }`. Caught in review, not by CI: the Integration Tests job is gated off for this PR, so nothing ran these against a real daemon after the shape changed. `droppedCount: 0` is the load-bearing half of the live-session assertion. A dropped probe also yields an empty `goals`, so the old assertion could not tell a successful ext-method round trip from a silently failed one. Re-ran against a spawned `qwen serve` + `qwen --acp` child: green with the fix, red without it. * fix(goals): refuse to replay an oversized goal card `restoreGoalFromHistory` gates the condition at MAX_GOAL_LENGTH, but `HistoryReplayer` did not: a corrupted or hand-edited transcript could still ship an unbounded `condition` to every client inside `_meta.goalStatus`. Apply the same gate at the replay emit site, so neither the card nor the hook survives an oversized condition. The gate deliberately does NOT move into `parseGoalStatusItem`, which would be the tidier-looking place. `findGoalToRestore` and `findLastTerminalGoal` scan backwards and stop at the FIRST goal card they meet, so dropping a card at parse time silently promotes the card before it. A transcript ending in an oversized `cleared` would then restore the `set` that preceded it — resurrecting a goal the user explicitly cleared, the exact failure persisting `cleared` was added to prevent. Parsing therefore stays lossless and the length check lives at each consumer. Tests pin both halves: replay refuses at 4001 and emits at exactly 4000, and three scanner tests show an oversized card still wins the scan so restore can fail closed on it. * fix(goals): keep the terminal observer alive across ACP resume Addresses the latest review round on QwenLM#6561. `registerGoalHook` calls `unregisterGoalHook`, which clears the session's goal-terminal observer. The ACP restore path passes no `addItem`, so nothing reinstalled it: a restored goal reached achieved/failed/aborted with no wire update and no persisted terminal card, and the next reload revived a goal that had already finished. The no-goal branch unregisters too, so every ACP resume lost the observer, not just ones with a goal. `#restoreGoalOnResume` now reinstalls it unconditionally. A restore blocked by trust or hook policy left the client showing an active goal that nothing drives. Restore now reports `blockedBy`, and history replay emits a trailing `cleared` card naming the reason. The card is emitted, not recorded, so a later resume in a trusted folder still restores the goal. It is emitted from inside replay because `loadSession` batches replay updates into its response, and a notification sent afterwards would reach the client first. Gated behind a `HistoryReplayer` option: export and `restoreSessionHistory` render a transcript rather than resume it, and the export config is a stub that throws on any method it does not implement. Transcript payloads are now treated as untrusted. `outputHistoryItems` is checked with `Array.isArray` before iteration and each entry for being a plain object before any field is read; a hand-edited record could otherwise throw and take the whole restore down, skipping the hook while replay still showed the goal as active. Also: - Carry `setAt` across resume instead of restarting the clock, scanning back to the run's `set` card when the newest card is a `checking` card (which had no `setAt`; they now persist one). - Refuse to restore an empty condition, as `/goal` does. - Warn instead of silently no-opping when no chat recording service is present. - Cap `GET /goals` session probes at 10 in flight. - Drop `lastTerminal` from the `sessionGoalGet` response and `BridgeSessionGoal` — no consumer reads it, and it was returned unprojected. - `GoalsDialog` keeps the form and the typed condition when creation fails, and clears a stale dropped-session count when a reload fails outright. - Cross-package test pinning `GOAL_CLEAR_KEYWORDS` and `MAX_GOAL_LENGTH` against the CLI sources they mirror. * fix(goals): drop the condition length cap on restore and in the web shell QwenLM#6665 removed the 4,000-character cap `/goal` applied when setting a goal, but the restore path and the Web Shell form still enforced it. After merging main that split the surfaces: a long condition `/goal` now accepts was persisted as a `set` card, then refused by `restoreGoalFromHistory` on the next resume and dropped from the replay entirely — the goal died on reload and the user never saw a card explaining why. Remove the cap everywhere rather than reinstate it at set time. A corrupted or hand-edited transcript can now restore an arbitrarily long condition, but that is exactly what `/goal` itself permits, so it is no longer a distinct risk. The empty-condition gate stays: it is the one case that is meaningless rather than merely large. - `goalConditionBlockedBy` rejects only an empty condition. - `HistoryReplayer` no longer skips long goal cards. - `GoalsDialog` drops the form check and the `maxLength` attribute, which had been silently truncating a long condition before the user could submit it. - `MAX_GOAL_LENGTH` and the now-orphaned `goals.error.tooLong` i18n strings are deleted, along with the drift test's length half; the clear-keyword half of that test still guards the constant that is genuinely duplicated. Also drops the `MAX_GOAL_LENGTH` import QwenLM#6665 left unused in `goalCommand.ts`, which failed `eslint --max-warnings 0`. * fix(web-shell): reuse the empty session a failed goal attempt leaves behind Setting a goal starts a fresh session and then sends `/goal <condition>` into it. The daemon session is not created by the "new session" step, though — `clearSession` only detaches and clears local state. `ensureSessionForPrompt` creates the session lazily inside `sendPrompt`, so a prompt that fails after the session exists leaves a created-but-empty one behind. The Goals form keeps the condition and invites a retry, and the retry called `createNewSession()` again: the empty session from the previous attempt was abandoned and another created in its place. A user retrying a few times against a busy daemon ended up with a column of blank chats in the sidebar. Remember the stranded session and reuse it when it is still the current one, rather than creating another. Nothing is deleted — a session is only reused when the failed attempt left it empty and it has not been switched away from. Once a goal actually lands, the session belongs to it, so the next goal starts a fresh one as before. * fix(goals): forget the stranded goal session on leaving the Goals page Addresses the latest review round on QwenLM#6561. The stranded-session reuse added in bee3295 was only safe while the Goals page stayed up. Leaving it (Back button) and then talking to that session from the composer turned it into a real conversation, but the ref still pointed at it: returning to Goals and setting a goal would reuse it and drop the goal loop on top of the user's conversation — the exact thing starting a fresh session exists to prevent. The ref is now cleared whenever the view leaves 'goals', so reuse can only ever hit a session the failed attempt itself created. Also: - `registerGoalHook` rejects a `setAt` in the future, not just a non-finite or non-positive one. Every duration downstream is `Date.now() - setAt`, so a transcript claiming the goal starts tomorrow rendered negative elapsed times. - `makeRestoreInnerConfig` gains `isTrustedFolder`. Without it, `goalRestoreBlockedBy` threw `config.isTrustedFolder is not a function` on every resume in these tests, and `#restoreGoalOnResume` swallowed it — so the goal-gate assertions passed through the catch rather than the branch each one names. The hooks-disabled test now pins the branch it took, and fails if the config regresses. - The status-bar goal pill names the goal in its accessible label. The visible pill is only "◎ /goal active (2m)" and the condition lived solely in `title`, a hover tooltip screen readers do not reliably announce. - `.iconAction` gains a `:focus-visible` rule, matching `.iconButton` in DialogShell.module.css; keyboard users had no focus indicator on the clear-goal button. - `GoalsDialog.test.tsx` restores real timers in `afterEach` rather than inline per test, so a failing assertion can no longer leak fake timers into the rest of the file. - Tests for the Goals form's Cancel button and for the status-bar pill, neither of which had any coverage. * fix(goals): identify a goal run by its condition, not just its card kinds Addresses the latest review round on QwenLM#6561. `findSetAtOfRun` walked back from the active card for the `setAt` on the `set` card that opened the run, stopping at any card that was not `set`/`checking`. That assumed a terminal card always separates two goals, and a transcript is a file: hand-edited, truncated, or written by a version that did not persist terminal cards, it can hold two goals back to back. The scan then walked past the second goal's cards into the first and returned ITS start time, so the active goal's elapsed time was measured from a goal that had already ended. The condition is what identifies a run, so the scan now stops when it changes. Also: - A malformed condition is reported once on resume, not twice. `restoreGoalFromHistory` is the only caller that knows the condition is bad, and three of its four callers (the TUI ones) discard the result entirely, so it stays the reporter; `#restoreGoalOnResume` no longer adds a second line for `condition-invalid`. The env gates were already reporting exactly once. - Goal-restore stderr can no longer take down a session load. `writeStderrLine` reaches `process.stderr.write`, which throws on EPIPE or a closed fd; a throw from the catch block would have escaped into `loadSession`, so a best-effort restore would fail the very load it promises not to block. - `isGoalClearCommand` checks the `/goal` prefix instead of assuming it. `goalArgOf` returns unrecognised text unchanged, so a bare `"clear"` — an ordinary thing to type into a chat box — answered true. Latent today because every caller pre-validates the prefix, but the contract was a trap. - Tests for the throw path reinstalling the terminal observer, and for the Goals page opening a goal's session (success and failure), neither of which had any coverage. * fix(web-shell): announce Goals dialog errors and give its buttons a focus ring Addresses the latest review round on QwenLM#6561. The form-validation error and the goal-list load error were painted but never announced: `role="alert"` puts them in a live region, so a screen-reader user learns the submit was rejected instead of believing the goal was created, and learns the list went stale on a poll that failed after the page was already up. Matches the existing pattern in RewindDialog. `.primaryButton` / `.secondaryButton` had no `:focus-visible` rule, so keyboard users tabbing to Set goal / Cancel saw no focus indicator — an inconsistency with `.iconAction` and `.sessionLink` in the same file. They now take the ring the form controls already use (`outline: 2px solid var(--primary)`), offset outwards rather than inset: `.primaryButton` is filled with `--primary`, so an inset ring in that colour would be invisible on it. * fix(cli): stop a broken stderr from abandoning a transcript replay Addresses the latest review round on QwenLM#6561. `process.stderr.write` throws on EPIPE or a closed fd — reachable whenever the reader goes away (`qwen … | head`) or a daemon redirects its stderr. The goal path writes diagnostics from inside work that must not be destroyed by a failed diagnostic, and `bee3295aa` only guarded one of the five sites. The worst of the rest was in `HistoryReplayer`: the "skipping a goal card whose condition is empty" line sits inside the loop over a record's cards. A throw there abandoned that record's remaining cards, propagated to the record loop, and aborted the whole replay — the user lost their transcript because we failed to complain about one bad card. Add `writeStderrLineSafe` to stdioHelpers and route the goal path's five sites through it, replacing the one-off `#warnGoalRestore` wrapper in acpAgent so there is a single implementation. It is deliberately not the default: `writeStderrLine` still throws, because most of the CLI wants a broken stderr to be loud. This variant is for writes that are incidental to real work. Also adds the first tests for `stdioHelpers`, and covers two untested Goals dialog behaviours: the Refresh button, and the clear button disabling itself while its clear is in flight (a double-click otherwise fired two concurrent clears at the same session). * fix(web-shell): keep the Goals page mounted across createNewSession main's `createNewSession` gained a `setMainView('chat')` of its own, fired synchronously before any await. That silently defeated the Goals handler's deferred switch: by the time `sendPrompt` rejected, the page — and the form that renders the error — was already gone, dropping the user into an empty chat with no explanation. This is the exact failure the deferred switch was written to prevent; the two changes only had to meet for it to come back. `createNewSession` takes a `keepView` opt-out, and the Goals handler uses it, so the page survives until the prompt is admitted. Saving and restoring `mainView` around the call would also work but flips the view to chat and back, which the user would see. A test pins the page staying mounted across a failed submit; it fails if `keepView` stops being honoured. Also from the same round: - `registerGoalHook`'s `initialSetAt` guards are now tested — a future timestamp, NaN, Infinity, 0 and a negative all fall back to now, and a usable value survives. The future case is the one with teeth: `Date.now() - setAt` renders a negative elapsed time rather than failing loudly, and nothing covered it. - The goals list carries `role="list"` / `role="listitem"`. They are divs, and even a real `<ul>` loses its implicit role under `display: flex` in Safari. - The open-session button names the action *and* the session. Its visible text is only the session name, which says nothing about what activating it does; the name stays in the accessible name so it still contains the visible label. - `.fieldLabel` matches ScheduledTasksDialog's `--muted-foreground`. The two dialogs sit side by side and had drifted. Not taken: deferring `setMainView` in `onOpenSession` until the load resolves. The sibling `handleOpenSessionFromOverview` switches first by the same pattern, and `loadSidebarSession` clears the transcript and shows a loading skeleton — which is the feedback for the common success path. Deferring would leave a click looking dead until the load lands, and would make Goals diverge from the Session Overview panel. If we want that behaviour it should change both. * fix(web-shell): stop the visuals spec asserting a badge QwenLM#7035 removed The "Capture web-shell visuals" job fails on this PR at `screenshots.spec.ts:395`, asserting the sidebar's "Primary" badge is visible: Error: expect(locator).toBeVisible() failed Error: element(s) not found Not from this branch. The chain is on main: - 2026-07-15 QwenLM#6880 adds the visuals spec, asserting the "Primary" badge — correct at the time. - 2026-07-17 QwenLM#7035 drops that badge as redundant (the workspace selector's checkmark already conveys the default target), removing the `primaryLabel` prop and its `<span className={styles.badge}>` render, and updates the *unit* test to assert its absence — but leaves this spec asserting it is visible. The capture job only runs on pull requests (it needs a PR head and a merge-base), so main never went red for it and the breakage surfaces on the next PR to merge main — this one. Assert the badge's absence instead of deleting the check, mirroring the unit test QwenLM#7035 added, so a regression re-adding it still fails here. --------- Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Removes the fixed 4,000-character command-level limit from
/goalconditions so detailed, self-contained task specifications can be used in interactive, non-interactive, and ACP sessions.The existing judge evidence bounds remain unchanged. Recent transcript parts are still bounded, while the authoritative goal condition continues to be appended separately and in full. Regression coverage verifies both behaviors.
Why it's needed
Complex coding tasks can require more than 4,000 characters of requirements, constraints, checklists, and verification criteria. Moving those requirements into a file is not equivalent because the one-shot goal judge has no tools and cannot independently read the file. The fixed command-level rejection therefore prevents valid self-contained stopping conditions even when the configured models can accept them.
Reviewer Test Plan
How to verify
Set a
/goalcondition longer than 4,000 characters in interactive, non-interactive, and ACP execution modes. Confirm that the command is accepted, the unique suffix at the end of the condition is preserved in active goal state and the initial model instruction, and the judge receives the complete condition. Separately confirm that an oversized ordinary transcript part remains truncated before judge evaluation.Evidence (Before & After)
Before: the released
qwen 0.19.8CLI exits with status 1 and reportsGoal condition is limited to 4000 characters (got 4001).After: the locally built bundle accepts the same 4,001-character condition and advances to the expected safe-mode hooks policy gate. Targeted regressions confirm that the complete condition reaches the initial instruction, active goal state, status history, and judge request while ordinary transcript evidence remains bounded.
Tested on
Environment (optional)
Node.js v24.18.0, local source build and bundled CLI, no sandbox for unit/static checks; safe mode for the deterministic bundled CLI smoke test.
Risk & Scope
Linked Issues
Fixes #6663
Related to #4228
中文说明
本 PR 做了什么
移除
/goalcondition 固定的 4,000 字符命令层限制，使交互模式、非交互模式和 ACP 会话都可以使用详细且自包含的任务规格。现有 judge 证据边界保持不变。最近的 transcript part 仍然受限，而权威 goal condition 继续作为独立内容完整追加。回归测试分别验证了这两种行为。
为什么需要
复杂编码任务的需求、约束、检查清单和验证标准可能超过 4,000 字符。把这些需求移到文件中并不等价，因为一次性 goal judge 没有工具，无法独立读取该文件。因此，即使已配置的模型能够接受更长输入，固定的命令层拒绝仍会阻止有效且自包含的停止条件。
Reviewer Test Plan
如何验证
分别在交互、非交互和 ACP 执行模式中设置超过 4,000 字符的
/goalcondition。确认命令被接受，condition 末尾的唯一标记在 active goal state 和初始模型指令中保持完整，并且 judge 收到完整 condition。另行确认超长的普通 transcript part 在 judge 评估前仍会被截断。证据（修改前与修改后）
修改前：已发布的
qwen 0.19.8CLI 以状态码 1 退出，并报告Goal condition is limited to 4000 characters (got 4001).修改后：本地构建的 bundle 接受相同的 4,001 字符 condition，并继续进入预期的 safe mode hooks policy gate。定向回归测试确认完整 condition 会到达初始指令、active goal state、状态历史和 judge 请求，同时普通 transcript 证据仍然有界。
测试平台
环境（可选）
Node.js v24.18.0，本地源码构建和 bundle CLI；单元/静态检查未使用 sandbox，确定性 bundle CLI smoke test 使用 safe mode。
风险与范围
关联 Issue
Fixes #6663
Related to #4228