ci: split Aliyun OSS sync into a separate post-release workflow - #7
Open
yiliang114 wants to merge 4 commits into
Open
yiliang114 wants to merge 4 commits into
yiliang114 wants to merge 4 commits into
Conversation
The OSS upload and verification steps were adding significant time to the release workflow's critical path. Move them into a new `sync-release-to-oss.yml` workflow that triggers on `release: published`, running asynchronously after the release completes. Key changes: - Extract all OSS steps (ossutil install, credential config, asset upload, verification, hosted installation sync, latest VERSION pointer) into `sync-release-to-oss.yml` - Switch `gh release create` to use CI_BOT_PAT so the release event can trigger the new downstream workflow (GITHUB_TOKEN events don't trigger other workflows) - Add `workflow_dispatch` input for manual re-runs on failure - New workflow downloads release assets from GitHub Release instead of rebuilding them This decouples publishing from CDN distribution: the release finishes as soon as npm publish + GitHub Release are done, and China CDN sync happens in parallel without blocking.
The test asserts OSS sync steps exist in the workflow. Now that these steps live in sync-release-to-oss.yml instead of release.yml, update the test to read from the correct file and add assertions that release.yml no longer contains OSS logic.
- Add 'Verify Standalone Archives' step before gh release create in release.yml as a pre-publish safety gate (wenshao) - Add concurrency group to sync-release-to-oss.yml to prevent race conditions when multiple releases publish close together (wenshao) - Update test to assert verify step exists in release.yml
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Qwen Code review did not complete successfully (it may have been superseded by a newer review request). See workflow logs. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Good structural split — separating OSS sync into a post-release workflow reduces blast radius and improves failure isolation. The verification chain across both workflows is solid.
8 findings — 0 critical, 6 suggestions, 1 test gap, 1 nice-to-have. None are blocking.
Reviewed by Qwen Code (qwen3.7-max) • CI mode
| fi | ||
| echo "is_nightly=${is_nightly}" >> "${GITHUB_OUTPUT}" | ||
| echo "is_preview=${is_preview}" >> "${GITHUB_OUTPUT}" | ||
| echo "is_stable=$([[ ${is_nightly} == 'false' && ${is_preview} == 'false' ]] && echo true || echo false)" >> "${GITHUB_OUTPUT}" |
There was a problem hiding this comment.
Suggestion: Stability derived from tag substring vs github.event.release.prerelease
The is_stable output is computed by checking whether the tag contains "nightly" or "preview" substrings. The release: published event payload already provides github.event.release.prerelease — a boolean set authoritatively by gh release create --prerelease in release.yml.
If a future pre-release uses a tag without these substrings (e.g., v1.0.0-beta.1, v1.0.0-rc.1), it would be misclassified as stable — causing hosted installation assets and the latest/VERSION pointer to be overwritten with pre-release content.
Consider using the event payload instead:
env:
PRERELEASE: '${{ github.event.release.prerelease }}'
run: |-
echo "is_stable=$([[ "${PRERELEASE}" == "false" ]] && echo true || echo false)" >> "${GITHUB_OUTPUT}"The is_nightly and is_preview outputs are computed but never consumed by any step — only is_stable is used.
Reviewed by Qwen Code (qwen3.7-max)
|
|
||
| - name: 'Verify Standalone Archives' | ||
| run: |- | ||
| npm run verify:installation-release -- --dir dist/standalone |
There was a problem hiding this comment.
Suggestion: Verify step ordering regression
"Verify Standalone Archives" now runs after both npm publish steps. In the original workflow, verification ran before any publishing — if archives were corrupt or missing, the release would fail before anything was published to npm.
With the current ordering, a corrupted standalone archive would be detected only after @qwen-code/qwen-code and @qwen-code/channel-base are already published. Consider moving this step before the npm publish steps to restore the original safety guarantee.
Reviewed by Qwen Code (qwen3.7-max)
| IS_PREVIEW: '${{ needs.prepare.outputs.is_preview }}' | ||
| run: |- | ||
| PRERELEASE_FLAG="" | ||
| if [[ "${IS_NIGHTLY}" == "true" || "${IS_PREVIEW}" == "true" ]]; then |
There was a problem hiding this comment.
Suggestion: set -euo pipefail removed from release creation step
The old workflow had set -euo pipefail as the first line of this step's run block. It was removed during the refactor. All other multi-line bash steps in both workflows use set -euo pipefail for consistent error handling. Without it, intermediate command failures (e.g., gh release create partially succeeding) could be masked.
Reviewed by Qwen Code (qwen3.7-max)
| types: ['published'] | ||
| workflow_dispatch: | ||
| inputs: | ||
| tag: |
There was a problem hiding this comment.
Suggestion: workflow_dispatch tag input unvalidated
The tag input accepts an arbitrary string with no format validation. It flows into checkout ref, gh release download, OSS upload prefix paths, and the latest/VERSION pointer — all without checking that it looks like a valid release tag (e.g., v*.*.*).
A simple guard would prevent accidental misuse:
if [[ ! "${TAG}" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-.+)?$ ]]; then
echo "::error::Invalid tag format: ${TAG}"
exit 1
fiReviewed by Qwen Code (qwen3.7-max)
| env: | ||
| ALIYUN_OSS_BUCKET: "${{ vars.ALIYUN_OSS_BUCKET || 'qwen-code-assets' }}" | ||
| ALIYUN_OSS_PUBLIC_BASE_URL: "${{ vars.ALIYUN_OSS_PUBLIC_BASE_URL || 'https://qwen-code-assets.oss-cn-hangzhou.aliyuncs.com' }}" | ||
| RELEASE_TAG: '${{ env.RELEASE_TAG }}' |
There was a problem hiding this comment.
Suggestion: latest/VERSION pointer lacks semantic version comparison
This step unconditionally writes ${RELEASE_TAG} to latest/VERSION when is_stable is true. An older hotfix release (e.g., v0.1.5 published after v0.2.0) would overwrite the pointer to an older version.
Consider adding a semver comparison before updating:
current_latest="$(curl -fsSL "${ALIYUN_OSS_PUBLIC_BASE_URL}/releases/qwen-code/latest/VERSION" | tr -d '[:space:]')" || current_latest=""
if [[ -n "${current_latest}" ]] && ! printf '%s\n%s\n' "${current_latest}" "${RELEASE_TAG}" | sort -V -C; then
echo "::warning::Skipping latest VERSION update: ${RELEASE_TAG} is older than current ${current_latest}"
exit 0
fiReviewed by Qwen Code (qwen3.7-max)
| cancel-in-progress: false | ||
|
|
||
| jobs: | ||
| sync: |
There was a problem hiding this comment.
Suggestion: No failure notification job
release.yml has a notify_failure job that alerts on workflow failures. This new workflow has no equivalent — if OSS sync fails, the release appears successful on GitHub but OSS artifacts are missing or stale, with no alerting.
Consider adding a similar notify_failure job, or at minimum a Slack/webhook notification on failure.
Reviewed by Qwen Code (qwen3.7-max)
| } | ||
| }); | ||
|
|
||
| it('syncs standalone and hosted installation assets during release', () => { |
There was a problem hiding this comment.
Suggestion: Test coverage gaps for the CI_BOT_PAT → OSS trigger chain
This test thoroughly verifies the OSS workflow's internal behavior but doesn't assert several properties critical to the two-workflow design:
CI_BOT_PATusage: No assertion thatrelease.ymlusessecrets.CI_BOT_PAT(notGITHUB_TOKEN) in the Create Release step. If reverted, therelease: publishedwebhook won't fire and OSS sync silently never runs.- Trigger conditions: No assertion for
release: types: ['published']orworkflow_dispatch. - Concurrency group: No assertion for the
sync-release-to-ossconcurrency group. - Repository guard: No assertion for
github.repository == 'QwenLM/qwen-code'. is_stablelogic: No assertion for how stability is determined.
These are the load-bearing properties of the split — a future refactor could silently break any of them without the test catching it.
Reviewed by Qwen Code (qwen3.7-max)
| gh release create "${RELEASE_TAG}" \ | ||
| dist/cli.js \ | ||
| --target "$RELEASE_BRANCH" \ | ||
| dist/standalone/qwen-code-* \ |
There was a problem hiding this comment.
Nice to have: Glob replaces explicit asset enumeration
The old workflow used mapfile -t release_assets < <(node scripts/verify-installation-release.js --dir dist/standalone --list-release-asset-paths) with "${release_assets[@]}" for the gh release create command. The new workflow uses dist/standalone/qwen-code-* glob + dist/standalone/SHA256SUMS.
The glob is simpler but could accidentally include unexpected build artifacts if the build pipeline changes. The OSS workflow retains the explicit mapfile approach — the inconsistency is worth noting.
Reviewed by Qwen Code (qwen3.7-max)
6 tasks
yiliang114
added a commit
that referenced
this pull request
Jul 4, 2026
Replace 30-line inline Node script with the original one-liner `grep -oE '^packages/[^/]+'`. The Node script checked for package.json existence and test scripts, but `--if-present` already handles missing test scripts and all workspace dirs have package.json. Addresses design-review item #7.
yiliang114
pushed a commit
that referenced
this pull request
Sep 1, 2026
* feat(cli): OpenTUI foundation modules — theme, a11y, clipboard, keys, dialogs scaffolding Foundation batch of the OpenTUI migration tracked in QwenLM#8662. Adds the renderer-neutral foundation modules under ui/opentui: theme family, a11y (plain-text, screen-reader), clipboard, key-map, mouse hit/caret, link-click + osc8 parity, early-input, exit guard/lifecycle, kitty negotiation, event-adapter, item-projection, slash dispatch (+ command parsing), commands context/output, help content, input history, and the dialog scaffolding primitives (core/shared) with the theme dialog. Two helpers land inside ui/opentui rather than utils/ to respect the utils leaf-layer rule (QwenLM#9737). Stacked on the infra batch: consumes ui/model streaming model and @OpenTui deps. No reachable ink code changes beyond a one-line export addition in the shared osc8 module. * fix(cli): import originals instead of forking slash parser and dialog scope utils * fix(cli): address R1 review findings in OpenTUI foundation modules * fix(cli): align OpenTUI command host with the memory-file-count rename Upstream renamed setGeminiMdFileCount to setMemoryFileCount in the command UI contract; the rebase onto main surfaced the mismatch at build. Rename the host interface member, the bridge wiring, the dispatch stub, and the test mock to match. * feat(cli): OpenTUI migration live-session and input batch Third landing batch of the OpenTUI migration (QwenLM#8662): live-session stream fold and model, message rendering (markdown heal, MCP progressive, client tool runs, text batching), transcript adapter with resume/session-switch, sticky todos, the composer (input-prompt view/key/model), mouse rows and scrollbar, unified-diff rendering, and session-compaction notice. All additive — no reachable ink code path is touched, ink remains the default. Carries the first consumer of the remend dependency deferred from the infra batch, placed in devDependencies per the renderer-deps convention. The stacked-skill completion helpers import from the relocated ui/commands module following the upstream rename. * fix(cli): address R2 review findings in OpenTUI foundation modules Round-2 review fixes (17 Critical + 10 Suggestion resolved in code): - dialogs-shared: move number-select flush out of the setState updater (StrictMode double-fires onSelect); split setActiveIndex (ink SET_ACTIVE_INDEX, lands on any in-range row) from highlightIndex (arrow keys skip disabled rows) so wheel navigation never sticks - event-adapter: chat_compressed notice mirrors ink formatCount ('~' prefix for estimated counts); vision_bridge_notice renders summary\nnotice; explicit projections for task_execution / findings_list / terminal_image keep multi-MB payloads off the transcript; retry-countdown-clear forwards isContinuation - slash-dispatch: isSlashCommandInput drops the '?' branch (ink gate routes ? input to the model); executeSlashCommand races the action against the abort signal; dialog effects carry the OpenDialogActionReturn payload; projected added-item text surfaces alongside non-handled effects (notice); message-shaped items project to their text; ui.history comes from env; absent sessionStats stamp now, not epoch; telemetry parity (recordSkillInvocation / recordAutoSkillCommandUsage / makeSlashCommandEvent) - item-projection: model stats render per-(model,source) sections with N/A for unpriced entries; Tool Calls line uses ASCII x like ink; redactProxy deduplicated via systemInfoFields export - theme: palette/syntax colors resolve through color-utils toHex before parseColor (ink CSS names / *bright names no longer degrade to magenta); unresolvable values stay unset - key-map: kitty 'kpenter' normalizes to 'return'; resolveCommands exposes ink's key fan-out (Ctrl+C fires QUIT + CLEAR_INPUT) - a11y: hardWrap delegates to wrap-ansi (word-boundary parity with ink's screen-reader path); markdown reducer tracks fence length, keeps fence-like lines literal inside fences and inner backticks in multi-backtick spans; stripAnsi delegates to strip-ansi plus a private-parameter CSI pass (SGR mouse, DEC save/restore) - clipboard: OSC 52 write gated on a TTY (stderr preferred), tests spy the stream instead of writing real sequences to the runner's terminal - exit-guard: independent per-key arm windows like ink - dialogs-theme: diff preview pane receives syntaxStyle/filetype * fix(cli): harden kitty probe and screen-reader writer per maintainer review - kitty-negotiation: KITTY_REPLY_RE requires at least one flag digit (\d+), so an echoed bare query \x1b[?u in PTY/CI environments no longer resolves true and locks the renderer into kitty mode on a terminal that never answers queries; the accumulation buffer keeps only a 256-byte tail (bounded memory, bounded rescan under byte floods); the settle-window drain is removed — an EventEmitter data listener cannot consume chunks from other listeners, so late replies flow to the renderer's input parser like any other terminal noise - a11y-screen-reader: ScreenReaderOutputWriter sanitizes written content (stripAnsi + drop bare C0/C1 controls, keep newlines) so the plain-text-only contract is enforced at the writer instead of trusting every future caller — smuggled OSC 52 clipboard writes or title/cursor sequences cannot execute on the main screen * fix(cli): address ytahdn independent review findings in OpenTUI foundation All 15 findings from the independent static review verified in source and fixed (no false positives; none deferred): - quit effect carries QuitActionReturn.messages projected to text on a notice field — ink renders them via QuittingDisplay and the payload was permanently lost (Important #1) - error and finished branches emit retry-countdown-clear like ink's handleErrorEvent/handleFinishedEvent, so a terminal event inside the countdown window no longer leaves a stale retry row (#2) - projectContextUsage renders the compaction-threshold ladder and the per-item detail sections (tools/memory/skills, ink's sort order) when showDetails is on — /context detail transcripts no longer show strictly less than the compact view (#3) - projectMcpStatus honors showSchema (parameter JSON under each tool) and showTips, so /mcp schema is distinguishable from /mcp (#4) - SlashDispatchEnv.settings is required: the real CommandContext.services.settings is non-null and a null surfaced as a generic command failure on first .merged read (#5) - dead singleColumn flag removed from the help width layout (clamp makes it always false; ink has no single-column mode) (#6) - truncated SS3 tail (bare ESC O) is stripped like the truncated CSI tail, so a captured half F1-F4 no longer leaks 'O' into the composer (#7) - readBufferRow trims cellColumns alongside text, so URLs ending at end-of-row on a wide character hit-test on both halves of the cell (#8) - kitty probe writes guarded: a synchronous stream throw settles the probe (restores raw mode, removes the listener) instead of leaking (#9) - eraseLines reuses the ansi-escapes helper (already a repo dependency) instead of a byte-identical hand-rolled copy (#10) - truncateText passthrough wrapper dropped; callers use the exported truncateHelpText directly (QwenLM#11) - SkillsList truncate keeps total length n like ink, so the description column no longer shifts by one cell when a name truncates (QwenLM#12) - model_fallback names pass through sanitizeDisplayText like ink (QwenLM#13) - selectIndex fires onHighlight before onSelect (ink dispatches SET_ACTIVE_INDEX then SELECT_CURRENT), keeping highlight-driven stay-open dialogs synced on mouse input (QwenLM#14) - mcp_app without fallbackText renders empty instead of JSON-dumping the embedded HTML; projectAbout hides Base URL when selectedAuthType is empty, matching ink's formatBaseUrl (QwenLM#15) * fix(cli): address R3 review findings in OpenTUI foundation modules - executeSlashCommand catch checks the abort signal first: an ESC- cancelled command (action rejects AbortError) returns handled with no failure telemetry or error message, mirroring ink's processor — the race promise never resolves when the signal is already aborted at addEventListener time - submit effect carries the full SubmitPromptActionReturn contract (modelOverride, onComplete, refreshContextFilesOnWrite) so the backend can honor /model <id> <prompt>, /dream's manual-run record, and /remember's context refresh like ink instead of silently degrading them - closing fences cannot carry info text (CommonMark): a ```js line inside an open block is literal body, not an early close that drops the block and inverts parse state for the rest of the document - info items append their linkUrl/linkText footer (ink's InfoMessage renders it; headless/SSH users need the printed URL, e.g. /bug) - the screen-reader sanitize keeps TAB: it separates words in tool/model output and deleting it fused adjacent tokens * fix(cli): address R4 review findings in OpenTUI foundation modules - a11y-plain-text: split on all CommonMark line endings so CRLF markdown opens/closes fences correctly; private-param CSI regex covers ECMA-48 intermediate bytes; DCS/SOS/PM/APC and unterminated OSC sequences consumed before strip-ansi; code-span pattern mirrors ink's INLINE_CODE_SPAN_PATTERN (non-empty content, closing-run lookbehind) - dialogs-shared: clearNumberBuffer called from setActiveIndex, selectIndex, and resyncKey block so wheel/hover/click/resync can't commit a stale numeric-flush selection the user never made - event-adapter: tool_call_response carries visionBridgeNotice on the tool-result event (ink ToolMessage renders the egress disclosure) - item-projection: projectContextUsage reads memoryFiles as { path, tokens } (ContextMemoryDetail), not { name, tokens } - key-map: 10 kp* keypad-navigation aliases (kpleft→left, …) and super flag folded into meta (ink Cmd+Enter = newline, not submit) - link-click: cellColumns no longer truncated to trimmed text length (preserves the wide-glyph right-half boundary); findUrlAtRow end boundary is width-aware (stringWidth of the last glyph) - slash-dispatch: submit effect carries PartListUnion content + a textContent string for text-only consumers (image parts survive); toggleVimEnabled and startNewSession seams wired from env; abort race resolves immediately for an already-aborted signal - clipboard: OSC 52 self-write removed — copyToClipboard's existing fallback (writeOsc52 / wrapForMultiplexer) is the single source - a11y-screen-reader: appendStatic skips clean === '\n' (ink's hasStaticOutput guard) * fix(cli): address QwenLM#10383 R1 findings in foundation modules - event-adapter: finished branch emits retry-countdown-clear BEFORE the info notice so the countdown row is actually cleared (the fold only pops when the last item is the retry row) - item-projection: /mcp tips now include all 5 lines ink renders (added OAuth auth tip and Ctrl+T toggle tip) - link-click: wide-glyph end boundary uses the last code point (not UTF-16 code unit) so non-BMP emoji are measured correctly by stringWidth - a11y-plain-text: CSI_SEQUENCE replaces PRIVATE_PARAM_CSI — drops the marker requirement so any CSI (with or without private parameter marker, with or without intermediate bytes) is fully consumed * fix(cli): address QwenLM#10383 R2 Critical findings in slash-dispatch - abort race: already-aborted signal now skips command.action entirely (result = undefined) instead of eagerly evaluating it as a Promise.race argument — the action's side effects (clear, persist, addItem) must not run on a cancelled submission - parent command telemetry: logEvent (slash_command SUCCESS) is now called before the early return for parent commands with subCommands (help listing) and bare handled — matching ink's finally-block logging * fix(cli): address QwenLM#10368 R2 review findings in live-session batch - input-prompt: convert OpenTUI display-width cursor coordinates to code-point positions at the component boundary — the pinned @opentui/core reports logicalCursor.col/offset and el.cursorOffset in terminal-cell units (edit-buffer.zig), while the ported ink helpers work in code points; wide characters previously shifted placeholder backspace, the backslash continuation check, completion targeting, and history edge compares - input-prompt: bump both search sequence refs when Esc dismisses the completion dropdown so an in-flight search resolving afterwards cannot re-open it and hijack Enter - live-session-model: carry the vision-bridge egress disclosure through the tool-result fold (ink ToolMessage renders it under the result) - messages: recognize the producers' two-L 'cancelled' summary spelling so canceled tools get the CANCELED glyph with strikethrough instead of the red ERROR glyph - session-switch: wrap /resume and /branch in the telemetry swap transaction (begin before the outgoing-session capture, commit at the UI re-key, abort after a rolled-back swap) — restores the usage aggregate on failed swaps and rejects concurrent switches - tests: display-width fake editor, wide-char placeholder/continuation witnesses, Esc invalidation, fold notice, cancelled spelling, and the three swap-transaction lifecycle cases * fix(cli): declare cursorOffset on the FakeEditor test interface The display-width fake added in 656e996 implements a cursorOffset getter/setter but the interface it is cast through never declared the member, so tsc --build fails with TS2339 at the two reads in the wide-char placeholder test. Typecheck ran before that commit's files were staged and missed it. * test(cli): stub listStartingRunIds in the session-switch fake registry The workflow-run registry gained listStartingRunIds with the workflow tasks feature on main; backgroundWorkUtils iterates it when describing blocking work, so the fake registry in session-switch.test.ts now implements it (empty) to match the interface the merged code expects. * fix(opentui): address yiliang114 review findings (4 P2 + 1 P3) - transcript-adapter: FIFO queue for id-less tool call pairing so tool-start and tool_result share the same minted id - session-switch: move uiSwapped=true to right after startNewSession (the first irreversible host mutation) preventing core/UI divergence on mid-sequence throw; same fix for branch handler - session-switch: add error item when /resume targets an unloadable session instead of returning silently - diff-render: run diff content through escapeAnsiCtrlCodes matching the ink text-boundary convention (useTurnDiffs.ts) - package.json: move remend from devDependencies to dependencies (imported from production source markdown-heal.ts) * fix(transcript): address R6 review — thinking latch, cancelled status, text join - Replace one-shot `closed` latch with `thinkingOpen` state so [thought, text, thought, ...] patterns emit matching thinking-end for each burst (P2) - Mirror live path: treat cancelled tool status as failed, not ok (P3) - Join user text parts with newline instead of empty string (P3) - Regenerate package-lock.json so remend is in dependencies (P2) * fix(opentui): address review-pr bot R3 critical findings - session-compaction: add missing COMPRESSION_FAILED_EMPTY_SUMMARY, OUTPUT_TRUNCATED, and API_ERROR cases to match ink compression-text.ts - live-session: distinguish cancelled from error in tool-end summary so toolStatusMeta renders strikethrough instead of red X - transcript-adapter: gate slash_command replay on phase=invocation to prevent double-replay (recorder writes both invocation and result) - input-prompt: add key.meta/key.option to DELETE_WORD_BACKWARD branch to match the guard condition that intercepts Alt+Backspace * fix(opentui): address round-5 review findings R2-5 R3-2 R3-12 R4-1 R2-5: update session-compaction.test.ts to assert the three new parity texts (EMPTY_SUMMARY, OUTPUT_TRUNCATED, API_ERROR) added in 63a7f3c; the old assertion that EMPTY_SUMMARY returned '' is stale. R3-2: transcript-adapter replay producer folded cancelled tool status into summary 'error' (red ✕) instead of 'cancelled' (strikethrough). Add the cancelled branch to match live-session.ts. R3-12: hidden slash-command invocations (hiddenInvocation: true for /auth, /help, /settings, /status, bare /effort, /btw) replayed as visible user rows and entered composer history. Gate them on the hiddenInvocation flag in the invocation filter. R4-1: modelOverride was only carried on the first UserQuery send; ToolResult continuation sends omitted it, so a per-turn model override silently reverted to the session default after the first tool batch. Propagate modelOverride into every continuation send.
yiliang114
pushed a commit
that referenced
this pull request
Sep 1, 2026
…-rewind (QwenLM#10383) * feat(cli): OpenTUI foundation modules — theme, a11y, clipboard, keys, dialogs scaffolding Foundation batch of the OpenTUI migration tracked in QwenLM#8662. Adds the renderer-neutral foundation modules under ui/opentui: theme family, a11y (plain-text, screen-reader), clipboard, key-map, mouse hit/caret, link-click + osc8 parity, early-input, exit guard/lifecycle, kitty negotiation, event-adapter, item-projection, slash dispatch (+ command parsing), commands context/output, help content, input history, and the dialog scaffolding primitives (core/shared) with the theme dialog. Two helpers land inside ui/opentui rather than utils/ to respect the utils leaf-layer rule (QwenLM#9737). Stacked on the infra batch: consumes ui/model streaming model and @OpenTui deps. No reachable ink code changes beyond a one-line export addition in the shared osc8 module. * fix(cli): import originals instead of forking slash parser and dialog scope utils * fix(cli): address R1 review findings in OpenTUI foundation modules * fix(cli): align OpenTUI command host with the memory-file-count rename Upstream renamed setGeminiMdFileCount to setMemoryFileCount in the command UI contract; the rebase onto main surfaced the mismatch at build. Rename the host interface member, the bridge wiring, the dispatch stub, and the test mock to match. * feat(cli): OpenTUI migration live-session and input batch Third landing batch of the OpenTUI migration (QwenLM#8662): live-session stream fold and model, message rendering (markdown heal, MCP progressive, client tool runs, text batching), transcript adapter with resume/session-switch, sticky todos, the composer (input-prompt view/key/model), mouse rows and scrollbar, unified-diff rendering, and session-compaction notice. All additive — no reachable ink code path is touched, ink remains the default. Carries the first consumer of the remend dependency deferred from the infra batch, placed in devDependencies per the renderer-deps convention. The stacked-skill completion helpers import from the relocated ui/commands module following the upstream rename. * feat(cli): OpenTUI migration batch 4 — dialogs, commands, and session-rewind Adds the dialog layer and command-routing infrastructure for the OpenTUI renderer: 19 dialog modules (auth, extensions, MCP, memory-status, misc, model family, modes, permissions, settings, stats/skills, help overlay, arena host, folder-trust gate), the commands registry with slash-to-dialog routing, the commands dispatcher (action interpreter connecting the slash gateway to the session and dialog layer), and the session-rewind viewer with its history-folding model. Also exports `isUserTextContent` from historyMapping so the rewind model can classify user turns without duplicating the predicate. Everything is additive: no reachable ink code path is touched, the default renderer stays ink, and the dep-direction gate passes. Stacked on the live-session batch. 56 test files / 886 tests, all green. * fix(cli): address R2 review findings in OpenTUI foundation modules Round-2 review fixes (17 Critical + 10 Suggestion resolved in code): - dialogs-shared: move number-select flush out of the setState updater (StrictMode double-fires onSelect); split setActiveIndex (ink SET_ACTIVE_INDEX, lands on any in-range row) from highlightIndex (arrow keys skip disabled rows) so wheel navigation never sticks - event-adapter: chat_compressed notice mirrors ink formatCount ('~' prefix for estimated counts); vision_bridge_notice renders summary\nnotice; explicit projections for task_execution / findings_list / terminal_image keep multi-MB payloads off the transcript; retry-countdown-clear forwards isContinuation - slash-dispatch: isSlashCommandInput drops the '?' branch (ink gate routes ? input to the model); executeSlashCommand races the action against the abort signal; dialog effects carry the OpenDialogActionReturn payload; projected added-item text surfaces alongside non-handled effects (notice); message-shaped items project to their text; ui.history comes from env; absent sessionStats stamp now, not epoch; telemetry parity (recordSkillInvocation / recordAutoSkillCommandUsage / makeSlashCommandEvent) - item-projection: model stats render per-(model,source) sections with N/A for unpriced entries; Tool Calls line uses ASCII x like ink; redactProxy deduplicated via systemInfoFields export - theme: palette/syntax colors resolve through color-utils toHex before parseColor (ink CSS names / *bright names no longer degrade to magenta); unresolvable values stay unset - key-map: kitty 'kpenter' normalizes to 'return'; resolveCommands exposes ink's key fan-out (Ctrl+C fires QUIT + CLEAR_INPUT) - a11y: hardWrap delegates to wrap-ansi (word-boundary parity with ink's screen-reader path); markdown reducer tracks fence length, keeps fence-like lines literal inside fences and inner backticks in multi-backtick spans; stripAnsi delegates to strip-ansi plus a private-parameter CSI pass (SGR mouse, DEC save/restore) - clipboard: OSC 52 write gated on a TTY (stderr preferred), tests spy the stream instead of writing real sequences to the runner's terminal - exit-guard: independent per-key arm windows like ink - dialogs-theme: diff preview pane receives syntaxStyle/filetype * fix(cli): harden kitty probe and screen-reader writer per maintainer review - kitty-negotiation: KITTY_REPLY_RE requires at least one flag digit (\d+), so an echoed bare query \x1b[?u in PTY/CI environments no longer resolves true and locks the renderer into kitty mode on a terminal that never answers queries; the accumulation buffer keeps only a 256-byte tail (bounded memory, bounded rescan under byte floods); the settle-window drain is removed — an EventEmitter data listener cannot consume chunks from other listeners, so late replies flow to the renderer's input parser like any other terminal noise - a11y-screen-reader: ScreenReaderOutputWriter sanitizes written content (stripAnsi + drop bare C0/C1 controls, keep newlines) so the plain-text-only contract is enforced at the writer instead of trusting every future caller — smuggled OSC 52 clipboard writes or title/cursor sequences cannot execute on the main screen * test(cli): strengthen /branch awaits handleBranch assertion with a deferred race The previous test asserted branchNames was populated after dispatch, but a fire-and-forget void call passed because microtasks drained before the check. Replace with a Promise.race that proves dispatch was still pending (blocked on the closed gate) before the gate was resolved — a void call resolves dispatch immediately, making the race return 'resolved' instead of the sentinel. * fix(cli): correct stale 67→69 count in commands-registry docblocks; clarify gate test Two docblock comments said "67 modules" but the table has 69 entries (verified against BuiltinCommandLoader.ts). Fixed to 69. The gated-commands test comment now explains that TypeScript enforces the CommandGate type at compile time, so the runtime loop is unnecessary — the literal name list is the intentional guard for a bogus gatedBy on an ungated command. * fix(cli): address dialogs-batch self-review findings in command registry - /theme route results include 'message': themeCommand returns a MessageActionReturn under NO_COLOR, so the declared results were incomplete - drop the unreachable 'branch' member from OpenTuiDialogRequest and make routeDialogToOpenTui throw on dialog-branch instead: /branch is a host action intercepted unconditionally by the dispatcher (ink parity), and a compile-time exclusion is not expressible because OpenDialogActionReturn is a single interface with a union dialog field — the loud throw guards against a future refactor dropping the interception; the branch route no longer advertises a dialogs entry no renderer opens - derive gate coverage from the loader instead of a hardcoded name list: the coverage test loads with every gate ON (plus the checkpointing flag the /restore factory needs) and asserts set equality between route names and registered built-ins — no escape hatches — and a new test proves every gatedBy route is genuinely absent from a gates-off load, so a bogus gate on an always-registered command fails * fix(cli): address ytahdn independent review findings in OpenTUI foundation All 15 findings from the independent static review verified in source and fixed (no false positives; none deferred): - quit effect carries QuitActionReturn.messages projected to text on a notice field — ink renders them via QuittingDisplay and the payload was permanently lost (Important #1) - error and finished branches emit retry-countdown-clear like ink's handleErrorEvent/handleFinishedEvent, so a terminal event inside the countdown window no longer leaves a stale retry row (#2) - projectContextUsage renders the compaction-threshold ladder and the per-item detail sections (tools/memory/skills, ink's sort order) when showDetails is on — /context detail transcripts no longer show strictly less than the compact view (#3) - projectMcpStatus honors showSchema (parameter JSON under each tool) and showTips, so /mcp schema is distinguishable from /mcp (#4) - SlashDispatchEnv.settings is required: the real CommandContext.services.settings is non-null and a null surfaced as a generic command failure on first .merged read (#5) - dead singleColumn flag removed from the help width layout (clamp makes it always false; ink has no single-column mode) (#6) - truncated SS3 tail (bare ESC O) is stripped like the truncated CSI tail, so a captured half F1-F4 no longer leaks 'O' into the composer (#7) - readBufferRow trims cellColumns alongside text, so URLs ending at end-of-row on a wide character hit-test on both halves of the cell (#8) - kitty probe writes guarded: a synchronous stream throw settles the probe (restores raw mode, removes the listener) instead of leaking (#9) - eraseLines reuses the ansi-escapes helper (already a repo dependency) instead of a byte-identical hand-rolled copy (#10) - truncateText passthrough wrapper dropped; callers use the exported truncateHelpText directly (QwenLM#11) - SkillsList truncate keeps total length n like ink, so the description column no longer shifts by one cell when a name truncates (QwenLM#12) - model_fallback names pass through sanitizeDisplayText like ink (QwenLM#13) - selectIndex fires onHighlight before onSelect (ink dispatches SET_ACTIVE_INDEX then SELECT_CURRENT), keeping highlight-driven stay-open dialogs synced on mouse input (QwenLM#14) - mcp_app without fallbackText renders empty instead of JSON-dumping the embedded HTML; projectAbout hides Base URL when selectedAuthType is empty, matching ink's formatBaseUrl (QwenLM#15) * fix(cli): drop dead single-column branch in help overlay The foundation batch removed the always-false singleColumn flag from the help width layout (the 72-column clamp makes it unreachable and ink has no single-column mode); the overlay's conditional branch on it no longer compiles. Only the two-column path was ever taken. * fix(cli): address R3 review findings in OpenTUI foundation modules - executeSlashCommand catch checks the abort signal first: an ESC- cancelled command (action rejects AbortError) returns handled with no failure telemetry or error message, mirroring ink's processor — the race promise never resolves when the signal is already aborted at addEventListener time - submit effect carries the full SubmitPromptActionReturn contract (modelOverride, onComplete, refreshContextFilesOnWrite) so the backend can honor /model <id> <prompt>, /dream's manual-run record, and /remember's context refresh like ink instead of silently degrading them - closing fences cannot carry info text (CommonMark): a ```js line inside an open block is literal body, not an early close that drops the block and inverts parse state for the rest of the document - info items append their linkUrl/linkText footer (ink's InfoMessage renders it; headless/SSH users need the printed URL, e.g. /bug) - the screen-reader sanitize keeps TAB: it separates words in tool/model output and deleting it fused adjacent tokens * fix(cli): address R4 review findings in OpenTUI foundation modules - a11y-plain-text: split on all CommonMark line endings so CRLF markdown opens/closes fences correctly; private-param CSI regex covers ECMA-48 intermediate bytes; DCS/SOS/PM/APC and unterminated OSC sequences consumed before strip-ansi; code-span pattern mirrors ink's INLINE_CODE_SPAN_PATTERN (non-empty content, closing-run lookbehind) - dialogs-shared: clearNumberBuffer called from setActiveIndex, selectIndex, and resyncKey block so wheel/hover/click/resync can't commit a stale numeric-flush selection the user never made - event-adapter: tool_call_response carries visionBridgeNotice on the tool-result event (ink ToolMessage renders the egress disclosure) - item-projection: projectContextUsage reads memoryFiles as { path, tokens } (ContextMemoryDetail), not { name, tokens } - key-map: 10 kp* keypad-navigation aliases (kpleft→left, …) and super flag folded into meta (ink Cmd+Enter = newline, not submit) - link-click: cellColumns no longer truncated to trimmed text length (preserves the wide-glyph right-half boundary); findUrlAtRow end boundary is width-aware (stringWidth of the last glyph) - slash-dispatch: submit effect carries PartListUnion content + a textContent string for text-only consumers (image parts survive); toggleVimEnabled and startNewSession seams wired from env; abort race resolves immediately for an already-aborted signal - clipboard: OSC 52 self-write removed — copyToClipboard's existing fallback (writeOsc52 / wrapForMultiplexer) is the single source - a11y-screen-reader: appendStatic skips clean === '\n' (ink's hasStaticOutput guard) * fix(cli): address QwenLM#10383 R1 findings in foundation modules - event-adapter: finished branch emits retry-countdown-clear BEFORE the info notice so the countdown row is actually cleared (the fold only pops when the last item is the retry row) - item-projection: /mcp tips now include all 5 lines ink renders (added OAuth auth tip and Ctrl+T toggle tip) - link-click: wide-glyph end boundary uses the last code point (not UTF-16 code unit) so non-BMP emoji are measured correctly by stringWidth - a11y-plain-text: CSI_SEQUENCE replaces PRIVATE_PARAM_CSI — drops the marker requirement so any CSI (with or without private parameter marker, with or without intermediate bytes) is fully consumed * fix(cli): address QwenLM#10383 R1 findings in dialogs batch - dialogs-mcp: flatServers derived from grouped render order, not raw prop order, so keyboard selection matches the highlighted server - dialogs-misc: highlightScope syncs editor selection (setSel) so Enter persists the highlighted scope's own editor, not the previous one - dialogs-settings: buildSettingsListItems forwards excludeWorkspaceRestricted under Workspace scope, matching ink's filter that prevents dead settings entries - dialogs-extensions: all onDetailAction call sites now swallow async rejections (.catch), matching session-rewind.tsx's pattern - dialogs-stats-skills: metrics read from per-session bucket (getMetricsForSession) not process-global; Wall Time computed from uiTelemetryService.getSessionStartTime() not a module-load constant - dialogs-modes: MODE_DESC key 'auto_edit' fixed to 'auto-edit' to match ApprovalMode.AUTO_EDIT enum value * fix(cli): address QwenLM#10383 R2 Critical findings in slash-dispatch - abort race: already-aborted signal now skips command.action entirely (result = undefined) instead of eagerly evaluating it as a Promise.race argument — the action's side effects (clear, persist, addItem) must not run on a cancelled submission - parent command telemetry: logEvent (slash_command SUCCESS) is now called before the early return for parent commands with subCommands (help listing) and bare handled — matching ink's finally-block logging * fix(cli): address QwenLM#10383 R2 Critical findings in dialogs batch - dialog-data: buildModelEntries image mode gates on isImageGenerationCapable (not imageOnly) so dual-role and visionOnly image-capable models appear in the image selector like ink - dialogs-extensions: detailSelect gets resyncKey: view so the cursor re-syncs when re-entering detail (action list shrinks after checked-update state resets) - dialogs-stats-skills: subscribes to uiTelemetryService 'update' event so stats stay live while the dialog is open (ink re-renders via SessionStatsProvider) * fix(cli): address QwenLM#10383 R3 review findings in opentui dialogs/dispatch - buildModelEntries: keep visionOnly models in the image selector (ink ModelDialog parity: isVisionModelMode || isImageModelMode || !visionOnly) - useDialogSelect: re-sync the cursor on items changes like ink's useSelectionList INITIALIZE reducer — follow the active item's key, fall back to the initial index when it is gone, so a shrinking list never strands the cursor where Enter reads undefined - OpenTuiSlashDispatcher: skip the action entirely when the signal is already aborted before the race — a late 'abort' listener never fires, so the eager race would run side effects before discarding - tests: pin the pre-aborted skip in both dispatchers, the dual-role image/vision fixture, the items-shrink clamp, and restore the uninstall-backout guard assertion * fix(cli): address QwenLM#10368 R2 review findings in live-session batch - input-prompt: convert OpenTUI display-width cursor coordinates to code-point positions at the component boundary — the pinned @opentui/core reports logicalCursor.col/offset and el.cursorOffset in terminal-cell units (edit-buffer.zig), while the ported ink helpers work in code points; wide characters previously shifted placeholder backspace, the backslash continuation check, completion targeting, and history edge compares - input-prompt: bump both search sequence refs when Esc dismisses the completion dropdown so an in-flight search resolving afterwards cannot re-open it and hijack Enter - live-session-model: carry the vision-bridge egress disclosure through the tool-result fold (ink ToolMessage renders it under the result) - messages: recognize the producers' two-L 'cancelled' summary spelling so canceled tools get the CANCELED glyph with strikethrough instead of the red ERROR glyph - session-switch: wrap /resume and /branch in the telemetry swap transaction (begin before the outgoing-session capture, commit at the UI re-key, abort after a rolled-back swap) — restores the usage aggregate on failed swaps and rejects concurrent switches - tests: display-width fake editor, wide-char placeholder/continuation witnesses, Esc invalidation, fold notice, cancelled spelling, and the three swap-transaction lifecycle cases * fix(cli): declare cursorOffset on the FakeEditor test interface The display-width fake added in 656e996 implements a cursorOffset getter/setter but the interface it is cast through never declared the member, so tsc --build fails with TS2339 at the two reads in the wide-char placeholder test. Typecheck ran before that commit's files were staged and missed it. * fix(cli): address QwenLM#10383 R4 review findings in dialogs batch - commands-dispatch.test: type the pre-aborted action mock as SlashCommandActionReturn so tsc --build passes (the inferred { type: string } could not satisfy the literal 'message' kind) - dialogs-shared.test: pin the resyncKey numeric-flush disarm — an armed digit quick-select must not commit a selection in the view swapped to before the flush timeout - session-switch.test: pin the unarmed-swap settlement when the resumed session is not found (commit, never abort) so the single swap slot cannot stay latched forever * test(cli): stub listStartingRunIds in the session-switch fake registry The workflow-run registry gained listStartingRunIds with the workflow tasks feature on main; backgroundWorkUtils iterates it when describing blocking work, so the fake registry in session-switch.test.ts now implements it (empty) to match the interface the merged code expects. * fix(opentui): address P2 review findings in session-rewind and load_history - session-rewind: add useRef re-entrancy guard to prevent double onRewind from batched key events in single stdin chunk - session-rewind: dispatch restore-error on onRewind rejection so the dialog recovers from dead 'restoring' phase back to 'pick' - commands-dispatch: pass Date.now()-based timestamps to addItem instead of array indices in load_history branch
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
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.
Test PR - medium (3 files, +308/-233)