feat(cli): customize banner area (logo, title, hide)#3710
Merged
Conversation
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. |
Document the design for issue #3005 (customize CLI banner area). Covers the banner region taxonomy and what is replaceable vs. locked, the three proposed settings (`ui.hideBanner`, `ui.customBannerTitle`, `ui.customAsciiArt`) and their resolution pipeline, the schema additions and wiring touch points, five alternative shapes considered, and the security / failure-handling guards. Mirrored EN + zh-CN under `docs/design/customize-banner-area/`. No code changes in this commit; implementation lands in a follow-up PR. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Adds three opt-in `ui.*` settings that let users replace brand chrome on
startup while keeping the operational lines (version, auth, model, path)
locked: `hideBanner`, `customBannerTitle`, `customAsciiArt` (string,
{path}, or {small,large}).
A new resolver in `packages/cli/src/ui/utils/customBanner.ts` walks the
loaded settings, normalizes each tier per scope (so {path} resolves
against the file that declared it), reads the file with O_NOFOLLOW and a
64 KB cap on POSIX, sanitizes via a banner-specific stripper that drops
OSC/CSI/SS2/SS3 sequences while preserving newlines, and caps art at 200
lines × 200 cols and titles at 80 chars. Every soft failure logs a
`[BANNER]` warn and falls through to the bundled QWEN logo or default
brand title — banner config can never crash the CLI.
`<Header />` now picks the widest custom tier that fits via a shared
`pickAsciiArtTier` helper and falls back to `shortAsciiLogo` otherwise;
`<AppHeader />` extends the existing `showBanner` gate to honor
`hideBanner` alongside the screen-reader fallback.
Tracks #3005 and the design merged in #3671.
d2d751e to
38441be
Compare
Reformats the EN and zh-CN design docs in `docs/design/customize-banner-area/` to satisfy `npx prettier --check`: table column alignment and trailing commas in `jsonc` examples. No content changes — the words, tables, and code blocks all say the same thing as before. Carries forward the only actionable feedback from the now-closed docs-only PR #3671, where the prettier check was the sole change requested.
4 tasks
Three audit-driven fixes for the banner customization feature:
1. **VSCode JSON schema accepts every documented shape.** The
`ui.customAsciiArt` entry in
`packages/vscode-ide-companion/schemas/settings.schema.json` was
declared as `type: object`, which made VSCode flag the inline-string
form (`"customAsciiArt": " ___"`) — a shape the runtime accepts and
the design doc recommends — as a schema violation. Replaced with a
`oneOf` covering string, `{path}`, and `{small,large}` (with each
tier itself string-or-`{path}`).
2. **Narrow terminals no longer leak the QWEN logo over a white-label
deployment.** When a user supplied custom ASCII art but neither tier
fit the terminal, `Header.tsx` previously fell back to the bundled
`shortAsciiLogo` — silently undoing the white-label intent on small
windows. The fallback now distinguishes "user supplied custom art"
from "no custom art at all": in the first case the logo column is
hidden entirely (info panel still renders); in the second case the
default logo shows as before. Soft-failure paths (missing file,
sanitization rejection) still fall through to `shortAsciiLogo`.
3. **Sanitizer strips C1 control bytes (0x80-0x9F).** The art and title
strippers previously stopped at 0x7F, leaving single-byte CSI
(`0x9B`), DCS (`0x90`), ST (`0x9C`) and other C1 controls intact —
which legacy 8-bit terminals would still interpret. Aligned the
ranges with the repo's existing `stripUnsafeCharacters` (in
`textUtils.ts`) so banner content can't carry interpreted control
bytes through.
New tests cover: C1 strip in art and title, absolute path reads,
symlink rejection on POSIX, narrow-terminal hide-on-custom-art, and
end-to-end `<AppHeader />` rendering through `resolveCustomBanner`.
The full banner suite is 48 tests (was 42).
Two clarifications surfaced by the audit on the implementation PR:
1. The design said `customAsciiArt` follows standard merge precedence,
but the resolver actually walks scopes per-tier so workspace can
override only `large` while user keeps `small`. Document that this
per-tier walk is intentional — both because each `{path}` has to
resolve against the file that declared it (the merged view loses
that information) and because it lets users keep a personal default
tier and override the other one per-workspace.
2. The render-time tier-selection step now distinguishes "user
supplied custom art but neither tier fits" (hide the logo column
entirely; falling back to `shortAsciiLogo` would silently undo a
white-label deployment on narrow terminals) from "user supplied no
custom art at all" (fall through to `shortAsciiLogo` and let the
default-logo width gate decide). Step 5's pure soft-failure
fallback (missing file, sanitization rejection) is unchanged —
still `shortAsciiLogo`.
Mirrored both edits in the zh-CN translation.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Question raised on the implementation PR: "why is the test logo `CCA`
instead of the full `Custom Code Agent` — is there a character limit?"
There is no character-count limit on titles or art. There is a
**width budget** driven by terminal columns, plus an absolute
hard cap (200×200 art, 80-char title) to keep malformed input from
freezing layout. The existing user-facing guide didn't quantify the
budget anywhere, so users were guessing why long inline names didn't
render.
Add a "How wide can the logo be? — the size budget" subsection that
spells out the formula
(`availableLogoWidth = terminalCols − 4 − 2 − 44`), tabulates it at
80 / 100 / 120 / 200 cols, calls out that a 17-char brand like
"Custom Code Agent" can't render as a single ANSI Shadow line on most
terminals (~120 cols of art), and shows the stacked-words
`{ small, large }` recipe — including the `figlet` one-liner that
generates the corresponding `banner-large.txt`.
Mirrored in the zh-CN translation.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The banner-customization design now has the size budget written down, but the per-cap limits (80-char title, 200×200 art, 64 KB file) were buried inside the size-budget formula table. Surface them as their own "Limits at a glance" subsection at the top of the user-configuration guide so users see the hard caps before they start hand-crafting art. Also switch the running example from "Custom Code Agent" (17 chars, ~120 cols of ANSI Shadow art on one line — too wide for any common terminal) to "Custom Agent" (12 chars, two-word stack at ~54 cols × 12 lines, fits any terminal ≥ 104 cols). The figlet recipe is now a two-word pipeline so a copy-paste run produces art the size the doc claims. Mirrored both changes in the zh-CN translation. The implementation itself is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
requested changes
May 5, 2026
| large?: ScopedSource; | ||
| } { | ||
| const order: SettingsFile[] = [ | ||
| settings.system, |
Collaborator
There was a problem hiding this comment.
[Critical] collectScopedTiers() reads the raw workspace scope even when the loaded settings mark the workspace as untrusted. The normal merge path explicitly drops workspace settings when isTrusted is false, but this resolver bypasses settings.merged and can still honor an untrusted repo's ui.customAsciiArt, including {path} entries. That lets an untrusted checkout influence startup rendering and trigger local file reads before the folder is trusted. Please skip settings.workspace here unless settings.isTrusted is true, and add a regression test for isTrusted: false.
Suggested change
| settings.system, | |
| const order: SettingsFile[] = [ | |
| settings.system, | |
| ...(settings.isTrusted ? [settings.workspace] : []), | |
| settings.user, | |
| settings.systemDefaults, | |
| ]; |
— gpt-5.5 via Qwen Code /review
Collaborator
Author
There was a problem hiding this comment.
Fixed in 417e428. collectScopedTiers() now skips settings.workspace entirely when settings.isTrusted is false (using your suggested order array literal). Added two regression tests in customBanner.test.ts:
ignores untrusted workspace settings — does not honor an untrusted checkout— workspace tries both an inline string and a{path}tier; both must be silenced and the user-scope value renders instead.honors workspace settings when isTrusted is true (sanity check on the trust gate)— paired with the above to make the gate diff visible.
| "description": "Replace the default \">_ Qwen Code\" title shown in the banner info panel. The version suffix is always appended.", | ||
| "type": "string", | ||
| "default": "" | ||
| }, |
Collaborator
There was a problem hiding this comment.
[Suggestion] The committed schema is not reproducible from the source schema. Running npm run generate:settings-schema rewrites this hand-authored oneOf into a generic object schema because packages/cli/src/config/settingsSchema.ts declares ui.customAsciiArt as type: 'object' and the generator has no metadata for the intended string / {path} / tiered union. The next schema regeneration will dirty the worktree and remove VS Code validation/completion for the documented bare-string form. Please teach the schema source/generator to emit this union, then regenerate this file instead of editing only the generated artifact.
— gpt-5.5 via Qwen Code /review
Collaborator
Author
There was a problem hiding this comment.
Fixed in 417e428. The schema is now reproducible from source.
- Added
jsonSchemaOverride?: Record<string, unknown>onSettingDefinitionas an escape hatch for shapes the type field can't express (description still carried forward from the SettingDefinition). scripts/generate-settings-schema.tsshort-circuits when an override is set and emits it verbatim.ui.customAsciiArtcarries the union override (string |{path}|{small,large}where each tier is string-or-{path}).npm run generate:settings-schemanow produces the same file that's committed; two back-to-back regens are byte-identical, so CI's regenerate-and-diff check is satisfied.
Two reviewer findings on PR #3710 (and the Lint job that fails for the same root cause): 1. **Schema regen now reproduces the committed JSON Schema.** The CI Lint step runs `npm run generate:settings-schema` and fails when the worktree dirties — my earlier hand-authored `oneOf` got blown away because `customAsciiArt` is `type: 'object'` in the source schema and the generator had no way to emit a union. Add a `jsonSchemaOverride` escape-hatch field on `SettingDefinition`: when set, the generator emits the override verbatim (description carried forward) instead of the type-driven shape. Set it on `customAsciiArt` to express the runtime union (string | {path} | {small,large} where each tier is itself string-or-{path}). The committed schema is now regenerated from source and CI's regenerate-and-diff check passes; two back-to-back regens produce identical output. 2. **Untrusted workspace settings no longer influence the banner.** `collectScopedTiers()` walked `settings.workspace` directly because per-scope file paths are needed to resolve relative `{path}` entries — but that bypassed the trust gate that `settings.merged` enforces. An untrusted checkout could therefore render its own ASCII art and trigger local file reads through a `{path}` entry before the user trusts the folder. Skip `settings.workspace` entirely when `settings.isTrusted` is false. Two regression tests cover the gate (untrusted = workspace silenced, falls through to user; trusted = workspace honored). Test suite for the banner is now 30 resolver tests + the existing Header / AppHeader / settingsSchema tests = 66 total, all green.
wenshao
requested changes
May 5, 2026
|
|
||
| function readArtFile(absolutePath: string): string | undefined { | ||
| let fd: number | undefined; | ||
| try { |
Collaborator
There was a problem hiding this comment.
[Critical] openSync() happens before the resolver verifies that the configured path is a regular file. On POSIX, opening a FIFO/named pipe read-only can block until a writer connects, so ui.customAsciiArt.path can hang CLI startup before the later fstatSync(...).isFile() guard gets a chance to soft-fail. Please reject non-regular files before opening (for example with lstatSync/statSync), or use nonblocking open where available while keeping the post-open fstatSync check for TOCTOU safety.
— gpt-5.5 via Qwen Code /review
|
|
||
| const rawLines = s.split('\n'); | ||
| const truncatedRows = rawLines.length > MAX_ART_LINES; | ||
| const limitedLines = truncatedRows |
Collaborator
There was a problem hiding this comment.
[Suggestion] This cap is based on UTF-16 string length, not terminal display width. The docs and constant describe a 200-column limit, but 200 CJK full-width characters pass this check and render at roughly 400 terminal cells; the same mismatch also affects the banner fit check because getAsciiArtWidth() uses line.length. Please apply the existing display-width helper (getCachedStringWidth / string-width semantics) when enforcing the column cap and when selecting a tier, truncating by code point/grapheme until the visual width fits.
— gpt-5.5 via Qwen Code /review
| { type: 'string' }, | ||
| { | ||
| type: 'object', | ||
| properties: { |
Collaborator
There was a problem hiding this comment.
[Suggestion] This object schema allows path, small, and large to appear together because it restricts property names but does not make the {path} and tiered shapes mutually exclusive. Runtime behavior is different: normalizeTiers() checks path first and treats { path, small } as a bare {path} source, silently ignoring small. Please split the schema into exclusive object branches and mirror that at runtime by rejecting objects that combine path with tier keys.
— gpt-5.5 via Qwen Code /review
wenshao
reviewed
May 5, 2026
| * single space — a hostile or accidental escape can't paint, redirect, or | ||
| * hyperlink in the user's terminal. | ||
| */ | ||
| function sanitizeArt(input: string): string { |
Collaborator
There was a problem hiding this comment.
[Suggestion] sanitizeArt（line 261）和 sanitizeTitle（line 317）各自独立复制了 stripTerminalControlSequences（packages/core/src/utils/terminalSafe.ts）中的三条 escape sequence 正则（OSC/CSI/SS2）。三个独立维护的副本已经产生分歧：新代码添加了 C1 控制字符（0x80-0x9f）过滤，但原始 stripTerminalControlSequences 缺失这一层。
Suggested change
| function sanitizeArt(input: string): string { | |
| // 建议: 将 C1 过滤回移植到 stripTerminalControlSequences，sanitizeArt/sanitizeTitle 调用它后再做 banner 特有的行/列裁剪。 | |
| // 或将 shared escape regex 提取为 core 包常量，消除三份副本。 |
— deepseek-v4-pro via Qwen Code /review
Collaborator
Author
There was a problem hiding this comment.
Fixed in 7ccbfae — went with both halves of your suggestion.
- Backported the C1 strip (0x80-0x9F) into
stripTerminalControlSequencesin@qwen-code/qwen-code-coreso session-title and any other caller picks up the same single-byte CSI / DCS / ST coverage that the banner sanitizer was the only one catching. - Extracted the OSC / CSI / SS2-3 regexes as exported constants (
TERMINAL_OSC_REGEX,TERMINAL_CSI_REGEX,TERMINAL_SHIFT_DCS_REGEX) from the same module.customBanner.tsreuses the constants insanitizeArt(which still has to preserve\n/\tso it can't just call the core helper) and delegates the title/subtitle path entirely tostripTerminalControlSequences.
No more divergent copies of the regexes; C1 coverage is universal.
| if (small && large) break; | ||
| const raw = file.settings.ui?.customAsciiArt; | ||
| if (raw === undefined || raw === null) continue; | ||
| if (!file.path) continue; |
Collaborator
There was a problem hiding this comment.
[Suggestion] collectScopedTiers 在 if (!file.path) continue（line 105）处跳过所有没有 file.path 的 scope，但 file.path 仅在 tier 使用 {path} 引用时才需要（用于相对路径解析）。内联字符串 art 不需要 ownerDir，却被此守卫一起跳过。当前 systemDefaults 不含 banner 设置故无实际影响，但这是不必要的耦合——未来若某个 scope 无路径但设置了内联 art，会被静默跳过。
Suggested change
| if (!file.path) continue; | |
| // 将路径守卫移到 {path} 解析分支内，或仅在 tiers 实际包含 {path} 源时才检查 file.path。 | |
| // 例如：在 normalizeTiers 返回后，仅对 AsciiArtSource 为 {path} 的 tier 检查 file.path。 |
— deepseek-v4-pro via Qwen Code /review
Collaborator
Author
There was a problem hiding this comment.
Fixed in 7ccbfae.
collectScopedTiers no longer drops a whole scope on !file.path. Inline-string tiers don't need an owning settings directory, so the path-presence check moved inside the {path} branch via a tier-by-tier considerTier helper; only {path} entries from a path-less scope soft-fail (with a tier-specific [BANNER] warn). Two regression tests cover both sides — inline string from a path-less scope still resolves, but a {path} entry from the same scope is dropped with a warn.
wenshao
requested changes
May 5, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Re-review on commit 417e428.
Fixed:
- Trust gate:
collectScopedTiers()skipssettings.workspacewhenisTrustedis false, with two regression tests. - Schema reproducibility: new
jsonSchemaOverrideescape hatch; the generator emits the union verbatim and regenerate-and-diff is clean.
Still open from the previous round:
- [Critical] FIFO/pipe at
ui.customAsciiArt.pathblocks CLI startup —customBanner.ts:221still opens before theisFile()check, andO_NOFOLLOWdoesn't help here. - [Suggestion] Column cap and tier-fit measure with UTF-16 length, not display width — CJK / emoji art is undercounted by ~2x.
- [Suggestion] Schema and runtime allow
{ path, small, large }together;pathsilently wins and tier keys are dropped without a warning.
Soft-fail design, sanitizer (OSC-8 / single-byte CSI / SS2-3 / DCS / C0+C1), 200x200 caps, scope-aware path resolution, and the white-label-respecting fallback are all solid. Just need (1) before merge, and (2)+(3) ideally same PR so the same file isn't churned again later.
| typeof fs.constants.O_NOFOLLOW === 'number' | ||
| ? fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW | ||
| : fs.constants.O_RDONLY; | ||
| fd = fs.openSync(absolutePath, flags); |
Collaborator
There was a problem hiding this comment.
[Critical, still open from previous review] openSync() is still happening before the regular-file check (stat.isFile() is on line 223, after the open). On POSIX, opening a FIFO read-only blocks until a writer connects, and O_NOFOLLOW does not help — it only refuses symlinks at the final path component, not FIFOs / sockets / devices. A user can hang CLI startup just by setting customAsciiArt: { "path": "/tmp/some-fifo" }, and there is no regression test for this path.
Two fixes either of which works:
lstatSync(absolutePath).isFile()beforeopenSync(still keep the post-openfstatSynccheck for TOCTOU safety against symlink swap).flags |= O_NONBLOCKwhere available (Linux/BSD); FIFO open returns immediately and the subsequentreadSyncerrors out cleanly.
Please add a POSIX-only vitest case (mirror the existing symlink test's if (process.platform === 'win32') return; guard) that mkfifo's a named pipe and asserts the resolver returns undefined without hanging.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 7ccbfae.
readArtFile now does lstatSync() and refuses non-regular files (FIFO / socket / device / symlink) BEFORE openSync(). The post-open fstatSync check is kept for TOCTOU safety against a swap between the two calls. POSIX-only regression test mkfifos a named pipe, points customAsciiArt: { path: 'pipe.fifo' } at it, and asserts the resolver returns within 1 s — if openSync ever regresses to blocking on the pipe, the test will hang well past that and the assertion catches it.
| // doesn't expand differently per terminal. | ||
| const detabbed = line.replace(/\t/g, ' '); | ||
| const trimmed = detabbed.replace(/\s+$/u, ''); | ||
| if (trimmed.length > MAX_ART_COLS) { |
Collaborator
There was a problem hiding this comment.
[Suggestion, still open from previous review] Two places still measure with UTF-16 .length instead of terminal display width: this MAX_ART_COLS cap, and getAsciiArtWidth() in textUtils.ts (called by the new pickAsciiArtTier()). 200 CJK fullwidth characters or 100 emoji pass the 200-col cap and render at ~400 cells, so the column budget and tier-fit selection are visually wrong on any non-ASCII art.
getCachedStringWidth (already imported in Header.tsx, based on string-width) is the right helper. Switch both the column cap here and the width measurement passed to pickAsciiArtTier to use it; truncate at the grapheme / code-point boundary that fits within MAX_ART_COLS rather than slicing on UTF-16 units (which can split a fullwidth character).
Collaborator
Author
There was a problem hiding this comment.
Fixed in 7ccbfae.
getAsciiArtWidth(intextUtils.ts) now usesgetCachedStringWidthper line so the tier-fit check passed topickAsciiArtTiermeasures in terminal cells. The QWEN logo is pure ASCII so its width didn't change.MAX_ART_COLStruncation insanitizeArtwalks code points (toCodePoints) and adds them to the kept prefix only whilegetCachedStringWidth(kept + cp) ≤ MAX_ART_COLS, so we never split a fullwidth code point or surrogate pair mid-character.- New regression test feeds 150 fullwidth
一characters (300 cells) and asserts the truncated output is ≤ 100 chars (≤ 200 cells) and is still a valid run of一(no mid-codepoint slice).
| { type: 'string' }, | ||
| { | ||
| type: 'object', | ||
| properties: { |
Collaborator
There was a problem hiding this comment.
[Suggestion, still open from previous review] The schema (and the regenerated settings.schema.json) still allows { "path": "p", "small": "s", "large": "l" } to validate as a single object — additionalProperties: false only constrains which keys may appear, not which combinations. normalizeTiers() matches path first and silently drops small / large, so a misconfigured user gets no error and a result they didn't ask for.
Recommended split: make the second oneOf branch two mutually-exclusive object branches — one with required: ['path'] and additionalProperties: false (so small / large cannot appear), and one with properties: { small, large } and additionalProperties: false (so path cannot appear). Then mirror the runtime: in normalizeTiers, if path and (small | large) co-exist, log the [BANNER] warn and return undefined instead of letting path silently win.
Collaborator
Author
There was a problem hiding this comment.
Fixed in 7ccbfae.
The jsonSchemaOverride is now three mutually-exclusive oneOf branches: string, {path} (additionalProperties: false, no tier keys allowed), and {small?, large?} (no path allowed at this level; each tier itself string-or-{path}). normalizeTiers() mirrors that — an object combining path with small / large is now soft-rejected with a [BANNER] warn rather than letting path win and silently drop the tier values. Regression test pins the runtime side.
wenshao
previously approved these changes
May 5, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No new issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review
Adds a fourth opt-in setting to the banner customization surface. The info panel renders four rows (title, subtitle/spacer, status, path); the second row was a hard-coded single-space spacer up to now. With this change a fork or white-label deployment can set `ui.customBannerSubtitle` to a one-line subtitle (e.g. "Built-in DataWorks Official Skills") and have it render in the secondary text color in place of the spacer. Empty/unset preserves the previous blank-spacer layout, so the change is back-compat. The subtitle is sanitized through the same `sanitizeSingleLine` helper as the title (now factored out): OSC / CSI / SS2 / SS3 leaders dropped, every other C0/C1 control byte replaced with a space, internal whitespace collapsed, ends trimmed. Capped at 160 characters — looser than the title's 80 because tagline / "powered by" copy commonly runs longer — with the same `[BANNER]` warn on truncation. Wiring: - `settingsSchema.ts` — new `customBannerSubtitle` entry next to `customBannerTitle`, `showInDialog: false` (free-form text in the TUI dialog isn't worth its own picker). - `customBanner.ts` — `ResolvedBanner.subtitle` field; `resolveCustomBanner` populates it; `sanitizeTitle` and the new `sanitizeSubtitle` share the same helper. - `Header.tsx` — when `customBannerSubtitle` is truthy the spacer row renders the string (secondary color, single line) instead of `<Text> </Text>`. Auth/model and path still sit at their usual positions. - `AppHeader.tsx` — pipes `resolvedBanner.subtitle` through. - VSCode JSON schema regenerated from source (idempotent). Tests: 5 new resolver tests (default, sanitize, length cap, empty, newline + C1 strip), 2 new Header tests (renders subtitle between title and auth; spacer preserved when unset), 1 new AppHeader integration test (end-to-end through resolver). Banner suite is now 35 + 17 + 6 + 16 = 74 tests, all green. Design docs (EN + zh-CN) updated: region taxonomy now lists four B-rows; "Limits at a glance" table grows a subtitle row; "Customization rules" matrix and "How to modify" section gain a "Add a brand subtitle" example with a rendered four-row preview.
Self-review found several sections of the banner customization design doc still framed for the original three settings; bring them in line with the four-setting reality landed in c7aa4a4: - Region taxonomy ASCII diagram now shows four B-rows (① title, ② subtitle, ③ status, ④ path). - Resolution-pipeline ASCII diagram and step list pick up customBannerSubtitle on the input side and the title/subtitle sanitize step on the resolver side. - "Settings schema additions" section lists the fourth entry, customBannerSubtitle, and notes the customAsciiArt jsonSchemaOverride that landed for VS Code schema reproducibility. - "Wiring changes" section updates the Header prop list and the HeaderProps interface, replaces the brittle line-number anchors with file-level anchors, drops the obsolete `paths` second arg from resolveCustomBanner, and adds the trust-gate sentence. - "Security & failure handling" table replaces the stripTerminalControlSequences shorthand with the actual banner-specific stripper, splits the title/subtitle row to cover both, and adds the untrusted-workspace gate as its own row. - "Verification plan" gains two scenarios: the subtitle row, and the untrusted-workspace check that the Critical reviewer comment on the impl PR explicitly asked us to lock down. Mirrored every edit in the zh-CN translation. The implementation itself is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
OverviewAdds four opt-in Code qualityStrengths
Issues / suggestionsMinor:
|
…, regex dedupe) Addresses the five findings on PR #3710 from the latest re-review: 1. **[Critical] FIFO/pipe at `customAsciiArt.path` no longer hangs startup.** The resolver was calling `openSync(path, O_NOFOLLOW)` *before* the `fstatSync(...).isFile()` check; on POSIX, opening a FIFO read-only blocks until a writer connects, and `O_NOFOLLOW` doesn't help — it only refuses symlinks at the final path component. `readArtFile` now `lstatSync()`s first and refuses non-regular files (FIFO / socket / device / symlink) before the open, while keeping the post-open `fstatSync` check for TOCTOU safety against a swap between the lstat and the open. New POSIX-only regression test `mkfifo`s a named pipe and asserts the resolver soft-fails inside 1 s; if the open ever regresses to blocking, the test will hang past the timeout and the assertion will catch it. 2. **[Suggestion] `{path}` and `{small,large}` are now mutually exclusive in both schema and runtime.** The `jsonSchemaOverride` on `ui.customAsciiArt` is split into three branches (string, `{path}`, `{small?, large?}`); none of them allow `path` and tier keys to co-exist. `normalizeTiers()` mirrors that — an object carrying both kinds of keys is now soft-rejected with a `[BANNER]` warn rather than letting `path` silently win and dropping the tier values. New regression test pins the runtime side. 3. **[Suggestion] Column cap and tier-fit selection now measure in terminal cells.** `getAsciiArtWidth` (in `textUtils.ts`) and the `MAX_ART_COLS` cap in `customBanner.ts` were both using UTF-16 `.length`, so 200 CJK fullwidth characters would slip the cap and render at ~400 cells, and `pickAsciiArtTier`'s width-fit check was wrong for any non-ASCII art. Switched both to `getCachedStringWidth` (string-width semantics, already in the repo); art truncation walks code points until adding another would push the cell width past the cap, so we never split a fullwidth code point or surrogate pair down the middle. New regression test exercises the CJK fullwidth case. 4. **[Suggestion] `collectScopedTiers()` no longer drops a whole scope just because it has no `file.path`.** Inline-string tiers don't need an owning settings directory; only `{path}` tiers do. The path-presence check was moved into the `{path}` branch, so a path-less scope (e.g. `systemDefaults`, future SDK-injected scopes) can still contribute inline art. `{path}` entries in such a scope soft-fail with a tier-specific `[BANNER]` warn rather than killing the whole scope. Two regression tests cover both sides. 5. **[Suggestion] OSC / CSI / SS2-3 regex are now authored once.** Extracted `TERMINAL_OSC_REGEX`, `TERMINAL_CSI_REGEX`, `TERMINAL_SHIFT_DCS_REGEX` from `stripTerminalControlSequences` in `@qwen-code/qwen-code-core` and re-export them from the package index. `customBanner.ts` reuses the constants for `sanitizeArt` (which still has to preserve `\n` / `\t`) and delegates the title/subtitle pipeline directly to `stripTerminalControlSequences`. Also backported the C1 control strip (0x80-0x9F) into the core helper so all callers (session-title, etc.) benefit from the same coverage; banner sanitizer was the only place catching single-byte CSI / DCS / ST. Banner suite is now 40 + 17 + 6 + 16 = 79 tests, all green. Schema regen is still byte-for-byte idempotent. `npm run typecheck` and prettier clean on touched files.
The FIFO regression test in 7ccbfae used a synchronous `require()` to pull in `node:child_process` so the test could lazy-load `execFileSync` only when needed. CI Lint flagged it under `no-restricted-syntax` — the repo enforces ES6 imports throughout, including in tests, with no exception for `require()`. Move the import to the top of the file alongside the other `node:` / vitest imports. The `try/catch` around `execFileSync('mkfifo', ...)` still gates the test on `mkfifo` being available (rare on a fresh container, so we skip rather than fail). 40 / 40 tests still pass and ESLint is clean on the touched file.
Collaborator
Code ReviewOverviewAdds four opt-in
Strengths
Issues / SuggestionsMinor — empty-file vs. empty-inline asymmetry
Minor —
|
wenshao
approved these changes
May 7, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. 在本地按设计文档逐项核对了实现,并通过 tmux 真实 TTY 跑了多种场景。
自动化验证(本地)
vitest(packages/cli全套):312 文件 / 4907 通过 / 7 跳过tsc --noEmit -p packages/cli/tsconfig.json:cleaneslint(全部改动文件):cleannpm run bundle:成功npm run generate:settings-schema重新生成的 VS Code schema 与提交字节级一致
tmux 端到端 smoke
| 场景 | 结果 |
|---|---|
| 自定义 title + subtitle + inline ASCII art(180 cols) | logo / 标题 / subtitle / 锁定行(version、auth/model、path)全部对位 |
hideBanner: true |
banner 整体隐藏,Tips 与输入框照常(由 hideTips 独立控制) |
title / subtitle / art 三处都塞 \x1b[..m 与 OSC-8 |
全部以字面文本渲染,输出里无 [31m / [41m 参数残留 |
customAsciiArt: { path: "./does-not-exist.txt" } |
CLI 启动正常,art 软回退到默认 QWEN logo,debug 日志写入 [BANNER] ... ENOENT |
{ small, large } 在 180 / 80 cols |
大屏选 large,中屏降级 small |
{ small, large } 两层都装不下(70 cols) |
隐藏 logo 列且不回退到 QWEN logo,info 面板照常,守住白标语义 |
实现质量亮点
- 路径攻击面:预先
lstatSync拒绝 FIFO/socket 避免启动挂死 →O_NOFOLLOW打开 →fstatSync二次确认,64 KB 截断,全程软失败带[BANNER]warn,有 timing 断言的回归测试。 - 信任闸门重申:resolver 必须绕过
settings.merged才能拿到每个 scope 的path,因此在collectScopedTiers显式跳过!isTrusted时的 workspace,有专测覆盖 inline +{path}两种 untrusted 输入都被丢弃。 - 视觉宽度截断:
MAX_ART_COLS=200用getCachedStringWidth(string-width)而非.length,CJK 全角与代理对都安全;getAsciiArtWidth同步从.length升级到视觉宽度,自定义 art 的宽度预算计算正确。 - Schema-runtime 互斥一致:JSON Schema 的
oneOf三分支与normalizeTiers的运行时path + small/large互斥校验对齐,VS Code 与 CLI 看法一致。 - C1 控制字符 (0x80–0x9F) 扩展剥离:
stripTerminalControlSequences收紧到[\x00-\x1f\x7f-\x9f],挡住 8-bit 终端可能解释的单字节 CSI/DCS/ST,既有调用点(sessionTitle)的测试已间接回归通过。
小提示(非阻塞,squash 时可顺手处理)
PR 标题 "logo, title, hide" 写于 customBannerSubtitle 加入之前,设计文档与 PR body 表均已含 subtitle,建议合并时把 subtitle 也补进标题。
This was referenced May 7, 2026
12 tasks
TaimoorSiddiquiOfficial
pushed a commit
to TaimoorSiddiquiOfficial/HopCode
that referenced
this pull request
May 7, 2026
* docs(design): add banner customization design (QwenLM#3005) Document the design for issue QwenLM#3005 (customize CLI banner area). Covers the banner region taxonomy and what is replaceable vs. locked, the three proposed settings (`ui.hideBanner`, `ui.customBannerTitle`, `ui.customAsciiArt`) and their resolution pipeline, the schema additions and wiring touch points, five alternative shapes considered, and the security / failure-handling guards. Mirrored EN + zh-CN under `docs/design/customize-banner-area/`. No code changes in this commit; implementation lands in a follow-up PR. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(cli): customize banner area (logo, title, hide) Adds three opt-in `ui.*` settings that let users replace brand chrome on startup while keeping the operational lines (version, auth, model, path) locked: `hideBanner`, `customBannerTitle`, `customAsciiArt` (string, {path}, or {small,large}). A new resolver in `packages/cli/src/ui/utils/customBanner.ts` walks the loaded settings, normalizes each tier per scope (so {path} resolves against the file that declared it), reads the file with O_NOFOLLOW and a 64 KB cap on POSIX, sanitizes via a banner-specific stripper that drops OSC/CSI/SS2/SS3 sequences while preserving newlines, and caps art at 200 lines × 200 cols and titles at 80 chars. Every soft failure logs a `[BANNER]` warn and falls through to the bundled QWEN logo or default brand title — banner config can never crash the CLI. `<Header />` now picks the widest custom tier that fits via a shared `pickAsciiArtTier` helper and falls back to `shortAsciiLogo` otherwise; `<AppHeader />` extends the existing `showBanner` gate to honor `hideBanner` alongside the screen-reader fallback. Tracks QwenLM#3005 and the design merged in QwenLM#3671. * docs(design): apply prettier to banner customization design Reformats the EN and zh-CN design docs in `docs/design/customize-banner-area/` to satisfy `npx prettier --check`: table column alignment and trailing commas in `jsonc` examples. No content changes — the words, tables, and code blocks all say the same thing as before. Carries forward the only actionable feedback from the now-closed docs-only PR QwenLM#3671, where the prettier check was the sole change requested. * fix(cli): address banner audit findings Three audit-driven fixes for the banner customization feature: 1. **VSCode JSON schema accepts every documented shape.** The `ui.customAsciiArt` entry in `packages/vscode-ide-companion/schemas/settings.schema.json` was declared as `type: object`, which made VSCode flag the inline-string form (`"customAsciiArt": " ___"`) — a shape the runtime accepts and the design doc recommends — as a schema violation. Replaced with a `oneOf` covering string, `{path}`, and `{small,large}` (with each tier itself string-or-`{path}`). 2. **Narrow terminals no longer leak the QWEN logo over a white-label deployment.** When a user supplied custom ASCII art but neither tier fit the terminal, `Header.tsx` previously fell back to the bundled `shortAsciiLogo` — silently undoing the white-label intent on small windows. The fallback now distinguishes "user supplied custom art" from "no custom art at all": in the first case the logo column is hidden entirely (info panel still renders); in the second case the default logo shows as before. Soft-failure paths (missing file, sanitization rejection) still fall through to `shortAsciiLogo`. 3. **Sanitizer strips C1 control bytes (0x80-0x9F).** The art and title strippers previously stopped at 0x7F, leaving single-byte CSI (`0x9B`), DCS (`0x90`), ST (`0x9C`) and other C1 controls intact — which legacy 8-bit terminals would still interpret. Aligned the ranges with the repo's existing `stripUnsafeCharacters` (in `textUtils.ts`) so banner content can't carry interpreted control bytes through. New tests cover: C1 strip in art and title, absolute path reads, symlink rejection on POSIX, narrow-terminal hide-on-custom-art, and end-to-end `<AppHeader />` rendering through `resolveCustomBanner`. The full banner suite is 48 tests (was 42). * docs(design): clarify cross-scope tier merge and white-label fallback Two clarifications surfaced by the audit on the implementation PR: 1. The design said `customAsciiArt` follows standard merge precedence, but the resolver actually walks scopes per-tier so workspace can override only `large` while user keeps `small`. Document that this per-tier walk is intentional — both because each `{path}` has to resolve against the file that declared it (the merged view loses that information) and because it lets users keep a personal default tier and override the other one per-workspace. 2. The render-time tier-selection step now distinguishes "user supplied custom art but neither tier fits" (hide the logo column entirely; falling back to `shortAsciiLogo` would silently undo a white-label deployment on narrow terminals) from "user supplied no custom art at all" (fall through to `shortAsciiLogo` and let the default-logo width gate decide). Step 5's pure soft-failure fallback (missing file, sanitization rejection) is unchanged — still `shortAsciiLogo`. Mirrored both edits in the zh-CN translation. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(design): add size budget section to banner customization Question raised on the implementation PR: "why is the test logo `CCA` instead of the full `Custom Code Agent` — is there a character limit?" There is no character-count limit on titles or art. There is a **width budget** driven by terminal columns, plus an absolute hard cap (200×200 art, 80-char title) to keep malformed input from freezing layout. The existing user-facing guide didn't quantify the budget anywhere, so users were guessing why long inline names didn't render. Add a "How wide can the logo be? — the size budget" subsection that spells out the formula (`availableLogoWidth = terminalCols − 4 − 2 − 44`), tabulates it at 80 / 100 / 120 / 200 cols, calls out that a 17-char brand like "Custom Code Agent" can't render as a single ANSI Shadow line on most terminals (~120 cols of art), and shows the stacked-words `{ small, large }` recipe — including the `figlet` one-liner that generates the corresponding `banner-large.txt`. Mirrored in the zh-CN translation. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(design): add limits-at-a-glance table; switch demo to Custom Agent The banner-customization design now has the size budget written down, but the per-cap limits (80-char title, 200×200 art, 64 KB file) were buried inside the size-budget formula table. Surface them as their own "Limits at a glance" subsection at the top of the user-configuration guide so users see the hard caps before they start hand-crafting art. Also switch the running example from "Custom Code Agent" (17 chars, ~120 cols of ANSI Shadow art on one line — too wide for any common terminal) to "Custom Agent" (12 chars, two-word stack at ~54 cols × 12 lines, fits any terminal ≥ 104 cols). The figlet recipe is now a two-word pipeline so a copy-paste run produces art the size the doc claims. Mirrored both changes in the zh-CN translation. The implementation itself is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address PR review + CI Lint failure Two reviewer findings on PR QwenLM#3710 (and the Lint job that fails for the same root cause): 1. **Schema regen now reproduces the committed JSON Schema.** The CI Lint step runs `npm run generate:settings-schema` and fails when the worktree dirties — my earlier hand-authored `oneOf` got blown away because `customAsciiArt` is `type: 'object'` in the source schema and the generator had no way to emit a union. Add a `jsonSchemaOverride` escape-hatch field on `SettingDefinition`: when set, the generator emits the override verbatim (description carried forward) instead of the type-driven shape. Set it on `customAsciiArt` to express the runtime union (string | {path} | {small,large} where each tier is itself string-or-{path}). The committed schema is now regenerated from source and CI's regenerate-and-diff check passes; two back-to-back regens produce identical output. 2. **Untrusted workspace settings no longer influence the banner.** `collectScopedTiers()` walked `settings.workspace` directly because per-scope file paths are needed to resolve relative `{path}` entries — but that bypassed the trust gate that `settings.merged` enforces. An untrusted checkout could therefore render its own ASCII art and trigger local file reads through a `{path}` entry before the user trusts the folder. Skip `settings.workspace` entirely when `settings.isTrusted` is false. Two regression tests cover the gate (untrusted = workspace silenced, falls through to user; trusted = workspace honored). Test suite for the banner is now 30 resolver tests + the existing Header / AppHeader / settingsSchema tests = 66 total, all green. * feat(cli): add ui.customBannerSubtitle for the spacer row Adds a fourth opt-in setting to the banner customization surface. The info panel renders four rows (title, subtitle/spacer, status, path); the second row was a hard-coded single-space spacer up to now. With this change a fork or white-label deployment can set `ui.customBannerSubtitle` to a one-line subtitle (e.g. "Built-in DataWorks Official Skills") and have it render in the secondary text color in place of the spacer. Empty/unset preserves the previous blank-spacer layout, so the change is back-compat. The subtitle is sanitized through the same `sanitizeSingleLine` helper as the title (now factored out): OSC / CSI / SS2 / SS3 leaders dropped, every other C0/C1 control byte replaced with a space, internal whitespace collapsed, ends trimmed. Capped at 160 characters — looser than the title's 80 because tagline / "powered by" copy commonly runs longer — with the same `[BANNER]` warn on truncation. Wiring: - `settingsSchema.ts` — new `customBannerSubtitle` entry next to `customBannerTitle`, `showInDialog: false` (free-form text in the TUI dialog isn't worth its own picker). - `customBanner.ts` — `ResolvedBanner.subtitle` field; `resolveCustomBanner` populates it; `sanitizeTitle` and the new `sanitizeSubtitle` share the same helper. - `Header.tsx` — when `customBannerSubtitle` is truthy the spacer row renders the string (secondary color, single line) instead of `<Text> </Text>`. Auth/model and path still sit at their usual positions. - `AppHeader.tsx` — pipes `resolvedBanner.subtitle` through. - VSCode JSON schema regenerated from source (idempotent). Tests: 5 new resolver tests (default, sanitize, length cap, empty, newline + C1 strip), 2 new Header tests (renders subtitle between title and auth; spacer preserved when unset), 1 new AppHeader integration test (end-to-end through resolver). Banner suite is now 35 + 17 + 6 + 16 = 74 tests, all green. Design docs (EN + zh-CN) updated: region taxonomy now lists four B-rows; "Limits at a glance" table grows a subtitle row; "Customization rules" matrix and "How to modify" section gain a "Add a brand subtitle" example with a rendered four-row preview. * docs(design): sweep stale 3-setting references after subtitle add Self-review found several sections of the banner customization design doc still framed for the original three settings; bring them in line with the four-setting reality landed in c7aa4a4: - Region taxonomy ASCII diagram now shows four B-rows (① title, ② subtitle, ③ status, ④ path). - Resolution-pipeline ASCII diagram and step list pick up customBannerSubtitle on the input side and the title/subtitle sanitize step on the resolver side. - "Settings schema additions" section lists the fourth entry, customBannerSubtitle, and notes the customAsciiArt jsonSchemaOverride that landed for VS Code schema reproducibility. - "Wiring changes" section updates the Header prop list and the HeaderProps interface, replaces the brittle line-number anchors with file-level anchors, drops the obsolete `paths` second arg from resolveCustomBanner, and adds the trust-gate sentence. - "Security & failure handling" table replaces the stripTerminalControlSequences shorthand with the actual banner-specific stripper, splits the title/subtitle row to cover both, and adds the untrusted-workspace gate as its own row. - "Verification plan" gains two scenarios: the subtitle row, and the untrusted-workspace check that the Critical reviewer comment on the impl PR explicitly asked us to lock down. Mirrored every edit in the zh-CN translation. The implementation itself is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address banner re-review (FIFO, mutex schema, display width, regex dedupe) Addresses the five findings on PR QwenLM#3710 from the latest re-review: 1. **[Critical] FIFO/pipe at `customAsciiArt.path` no longer hangs startup.** The resolver was calling `openSync(path, O_NOFOLLOW)` *before* the `fstatSync(...).isFile()` check; on POSIX, opening a FIFO read-only blocks until a writer connects, and `O_NOFOLLOW` doesn't help — it only refuses symlinks at the final path component. `readArtFile` now `lstatSync()`s first and refuses non-regular files (FIFO / socket / device / symlink) before the open, while keeping the post-open `fstatSync` check for TOCTOU safety against a swap between the lstat and the open. New POSIX-only regression test `mkfifo`s a named pipe and asserts the resolver soft-fails inside 1 s; if the open ever regresses to blocking, the test will hang past the timeout and the assertion will catch it. 2. **[Suggestion] `{path}` and `{small,large}` are now mutually exclusive in both schema and runtime.** The `jsonSchemaOverride` on `ui.customAsciiArt` is split into three branches (string, `{path}`, `{small?, large?}`); none of them allow `path` and tier keys to co-exist. `normalizeTiers()` mirrors that — an object carrying both kinds of keys is now soft-rejected with a `[BANNER]` warn rather than letting `path` silently win and dropping the tier values. New regression test pins the runtime side. 3. **[Suggestion] Column cap and tier-fit selection now measure in terminal cells.** `getAsciiArtWidth` (in `textUtils.ts`) and the `MAX_ART_COLS` cap in `customBanner.ts` were both using UTF-16 `.length`, so 200 CJK fullwidth characters would slip the cap and render at ~400 cells, and `pickAsciiArtTier`'s width-fit check was wrong for any non-ASCII art. Switched both to `getCachedStringWidth` (string-width semantics, already in the repo); art truncation walks code points until adding another would push the cell width past the cap, so we never split a fullwidth code point or surrogate pair down the middle. New regression test exercises the CJK fullwidth case. 4. **[Suggestion] `collectScopedTiers()` no longer drops a whole scope just because it has no `file.path`.** Inline-string tiers don't need an owning settings directory; only `{path}` tiers do. The path-presence check was moved into the `{path}` branch, so a path-less scope (e.g. `systemDefaults`, future SDK-injected scopes) can still contribute inline art. `{path}` entries in such a scope soft-fail with a tier-specific `[BANNER]` warn rather than killing the whole scope. Two regression tests cover both sides. 5. **[Suggestion] OSC / CSI / SS2-3 regex are now authored once.** Extracted `TERMINAL_OSC_REGEX`, `TERMINAL_CSI_REGEX`, `TERMINAL_SHIFT_DCS_REGEX` from `stripTerminalControlSequences` in `@qwen-code/qwen-code-core` and re-export them from the package index. `customBanner.ts` reuses the constants for `sanitizeArt` (which still has to preserve `\n` / `\t`) and delegates the title/subtitle pipeline directly to `stripTerminalControlSequences`. Also backported the C1 control strip (0x80-0x9F) into the core helper so all callers (session-title, etc.) benefit from the same coverage; banner sanitizer was the only place catching single-byte CSI / DCS / ST. Banner suite is now 40 + 17 + 6 + 16 = 79 tests, all green. Schema regen is still byte-for-byte idempotent. `npm run typecheck` and prettier clean on touched files. * fix(cli): replace require() with ES6 import in FIFO test (lint) The FIFO regression test in 7ccbfae used a synchronous `require()` to pull in `node:child_process` so the test could lazy-load `execFileSync` only when needed. CI Lint flagged it under `no-restricted-syntax` — the repo enforces ES6 imports throughout, including in tests, with no exception for `require()`. Move the import to the top of the file alongside the other `node:` / vitest imports. The `try/catch` around `execFileSync('mkfifo', ...)` still gates the test on `mkfifo` being available (rare on a fresh container, so we skip rather than fail). 40 / 40 tests still pass and ESLint is clean on the touched file. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
tanzhenxin
pushed a commit
that referenced
this pull request
May 7, 2026
Add ui.customBannerTitle, ui.customBannerSubtitle, and ui.customAsciiArt to the user-facing settings table. Also reword ui.hideBanner to note that it covers both the logo column and the info panel and that Tips render independently. These settings landed in #3710 but only ui.hideBanner was listed in the table, so users had no way to discover the other three short of reading the schema or the design doc.
DragonnZhang
pushed a commit
that referenced
this pull request
May 8, 2026
* docs(design): add banner customization design (#3005) Document the design for issue #3005 (customize CLI banner area). Covers the banner region taxonomy and what is replaceable vs. locked, the three proposed settings (`ui.hideBanner`, `ui.customBannerTitle`, `ui.customAsciiArt`) and their resolution pipeline, the schema additions and wiring touch points, five alternative shapes considered, and the security / failure-handling guards. Mirrored EN + zh-CN under `docs/design/customize-banner-area/`. No code changes in this commit; implementation lands in a follow-up PR. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(cli): customize banner area (logo, title, hide) Adds three opt-in `ui.*` settings that let users replace brand chrome on startup while keeping the operational lines (version, auth, model, path) locked: `hideBanner`, `customBannerTitle`, `customAsciiArt` (string, {path}, or {small,large}). A new resolver in `packages/cli/src/ui/utils/customBanner.ts` walks the loaded settings, normalizes each tier per scope (so {path} resolves against the file that declared it), reads the file with O_NOFOLLOW and a 64 KB cap on POSIX, sanitizes via a banner-specific stripper that drops OSC/CSI/SS2/SS3 sequences while preserving newlines, and caps art at 200 lines × 200 cols and titles at 80 chars. Every soft failure logs a `[BANNER]` warn and falls through to the bundled QWEN logo or default brand title — banner config can never crash the CLI. `<Header />` now picks the widest custom tier that fits via a shared `pickAsciiArtTier` helper and falls back to `shortAsciiLogo` otherwise; `<AppHeader />` extends the existing `showBanner` gate to honor `hideBanner` alongside the screen-reader fallback. Tracks #3005 and the design merged in #3671. * docs(design): apply prettier to banner customization design Reformats the EN and zh-CN design docs in `docs/design/customize-banner-area/` to satisfy `npx prettier --check`: table column alignment and trailing commas in `jsonc` examples. No content changes — the words, tables, and code blocks all say the same thing as before. Carries forward the only actionable feedback from the now-closed docs-only PR #3671, where the prettier check was the sole change requested. * fix(cli): address banner audit findings Three audit-driven fixes for the banner customization feature: 1. **VSCode JSON schema accepts every documented shape.** The `ui.customAsciiArt` entry in `packages/vscode-ide-companion/schemas/settings.schema.json` was declared as `type: object`, which made VSCode flag the inline-string form (`"customAsciiArt": " ___"`) — a shape the runtime accepts and the design doc recommends — as a schema violation. Replaced with a `oneOf` covering string, `{path}`, and `{small,large}` (with each tier itself string-or-`{path}`). 2. **Narrow terminals no longer leak the QWEN logo over a white-label deployment.** When a user supplied custom ASCII art but neither tier fit the terminal, `Header.tsx` previously fell back to the bundled `shortAsciiLogo` — silently undoing the white-label intent on small windows. The fallback now distinguishes "user supplied custom art" from "no custom art at all": in the first case the logo column is hidden entirely (info panel still renders); in the second case the default logo shows as before. Soft-failure paths (missing file, sanitization rejection) still fall through to `shortAsciiLogo`. 3. **Sanitizer strips C1 control bytes (0x80-0x9F).** The art and title strippers previously stopped at 0x7F, leaving single-byte CSI (`0x9B`), DCS (`0x90`), ST (`0x9C`) and other C1 controls intact — which legacy 8-bit terminals would still interpret. Aligned the ranges with the repo's existing `stripUnsafeCharacters` (in `textUtils.ts`) so banner content can't carry interpreted control bytes through. New tests cover: C1 strip in art and title, absolute path reads, symlink rejection on POSIX, narrow-terminal hide-on-custom-art, and end-to-end `<AppHeader />` rendering through `resolveCustomBanner`. The full banner suite is 48 tests (was 42). * docs(design): clarify cross-scope tier merge and white-label fallback Two clarifications surfaced by the audit on the implementation PR: 1. The design said `customAsciiArt` follows standard merge precedence, but the resolver actually walks scopes per-tier so workspace can override only `large` while user keeps `small`. Document that this per-tier walk is intentional — both because each `{path}` has to resolve against the file that declared it (the merged view loses that information) and because it lets users keep a personal default tier and override the other one per-workspace. 2. The render-time tier-selection step now distinguishes "user supplied custom art but neither tier fits" (hide the logo column entirely; falling back to `shortAsciiLogo` would silently undo a white-label deployment on narrow terminals) from "user supplied no custom art at all" (fall through to `shortAsciiLogo` and let the default-logo width gate decide). Step 5's pure soft-failure fallback (missing file, sanitization rejection) is unchanged — still `shortAsciiLogo`. Mirrored both edits in the zh-CN translation. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(design): add size budget section to banner customization Question raised on the implementation PR: "why is the test logo `CCA` instead of the full `Custom Code Agent` — is there a character limit?" There is no character-count limit on titles or art. There is a **width budget** driven by terminal columns, plus an absolute hard cap (200×200 art, 80-char title) to keep malformed input from freezing layout. The existing user-facing guide didn't quantify the budget anywhere, so users were guessing why long inline names didn't render. Add a "How wide can the logo be? — the size budget" subsection that spells out the formula (`availableLogoWidth = terminalCols − 4 − 2 − 44`), tabulates it at 80 / 100 / 120 / 200 cols, calls out that a 17-char brand like "Custom Code Agent" can't render as a single ANSI Shadow line on most terminals (~120 cols of art), and shows the stacked-words `{ small, large }` recipe — including the `figlet` one-liner that generates the corresponding `banner-large.txt`. Mirrored in the zh-CN translation. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(design): add limits-at-a-glance table; switch demo to Custom Agent The banner-customization design now has the size budget written down, but the per-cap limits (80-char title, 200×200 art, 64 KB file) were buried inside the size-budget formula table. Surface them as their own "Limits at a glance" subsection at the top of the user-configuration guide so users see the hard caps before they start hand-crafting art. Also switch the running example from "Custom Code Agent" (17 chars, ~120 cols of ANSI Shadow art on one line — too wide for any common terminal) to "Custom Agent" (12 chars, two-word stack at ~54 cols × 12 lines, fits any terminal ≥ 104 cols). The figlet recipe is now a two-word pipeline so a copy-paste run produces art the size the doc claims. Mirrored both changes in the zh-CN translation. The implementation itself is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address PR review + CI Lint failure Two reviewer findings on PR #3710 (and the Lint job that fails for the same root cause): 1. **Schema regen now reproduces the committed JSON Schema.** The CI Lint step runs `npm run generate:settings-schema` and fails when the worktree dirties — my earlier hand-authored `oneOf` got blown away because `customAsciiArt` is `type: 'object'` in the source schema and the generator had no way to emit a union. Add a `jsonSchemaOverride` escape-hatch field on `SettingDefinition`: when set, the generator emits the override verbatim (description carried forward) instead of the type-driven shape. Set it on `customAsciiArt` to express the runtime union (string | {path} | {small,large} where each tier is itself string-or-{path}). The committed schema is now regenerated from source and CI's regenerate-and-diff check passes; two back-to-back regens produce identical output. 2. **Untrusted workspace settings no longer influence the banner.** `collectScopedTiers()` walked `settings.workspace` directly because per-scope file paths are needed to resolve relative `{path}` entries — but that bypassed the trust gate that `settings.merged` enforces. An untrusted checkout could therefore render its own ASCII art and trigger local file reads through a `{path}` entry before the user trusts the folder. Skip `settings.workspace` entirely when `settings.isTrusted` is false. Two regression tests cover the gate (untrusted = workspace silenced, falls through to user; trusted = workspace honored). Test suite for the banner is now 30 resolver tests + the existing Header / AppHeader / settingsSchema tests = 66 total, all green. * feat(cli): add ui.customBannerSubtitle for the spacer row Adds a fourth opt-in setting to the banner customization surface. The info panel renders four rows (title, subtitle/spacer, status, path); the second row was a hard-coded single-space spacer up to now. With this change a fork or white-label deployment can set `ui.customBannerSubtitle` to a one-line subtitle (e.g. "Built-in DataWorks Official Skills") and have it render in the secondary text color in place of the spacer. Empty/unset preserves the previous blank-spacer layout, so the change is back-compat. The subtitle is sanitized through the same `sanitizeSingleLine` helper as the title (now factored out): OSC / CSI / SS2 / SS3 leaders dropped, every other C0/C1 control byte replaced with a space, internal whitespace collapsed, ends trimmed. Capped at 160 characters — looser than the title's 80 because tagline / "powered by" copy commonly runs longer — with the same `[BANNER]` warn on truncation. Wiring: - `settingsSchema.ts` — new `customBannerSubtitle` entry next to `customBannerTitle`, `showInDialog: false` (free-form text in the TUI dialog isn't worth its own picker). - `customBanner.ts` — `ResolvedBanner.subtitle` field; `resolveCustomBanner` populates it; `sanitizeTitle` and the new `sanitizeSubtitle` share the same helper. - `Header.tsx` — when `customBannerSubtitle` is truthy the spacer row renders the string (secondary color, single line) instead of `<Text> </Text>`. Auth/model and path still sit at their usual positions. - `AppHeader.tsx` — pipes `resolvedBanner.subtitle` through. - VSCode JSON schema regenerated from source (idempotent). Tests: 5 new resolver tests (default, sanitize, length cap, empty, newline + C1 strip), 2 new Header tests (renders subtitle between title and auth; spacer preserved when unset), 1 new AppHeader integration test (end-to-end through resolver). Banner suite is now 35 + 17 + 6 + 16 = 74 tests, all green. Design docs (EN + zh-CN) updated: region taxonomy now lists four B-rows; "Limits at a glance" table grows a subtitle row; "Customization rules" matrix and "How to modify" section gain a "Add a brand subtitle" example with a rendered four-row preview. * docs(design): sweep stale 3-setting references after subtitle add Self-review found several sections of the banner customization design doc still framed for the original three settings; bring them in line with the four-setting reality landed in c7aa4a4: - Region taxonomy ASCII diagram now shows four B-rows (① title, ② subtitle, ③ status, ④ path). - Resolution-pipeline ASCII diagram and step list pick up customBannerSubtitle on the input side and the title/subtitle sanitize step on the resolver side. - "Settings schema additions" section lists the fourth entry, customBannerSubtitle, and notes the customAsciiArt jsonSchemaOverride that landed for VS Code schema reproducibility. - "Wiring changes" section updates the Header prop list and the HeaderProps interface, replaces the brittle line-number anchors with file-level anchors, drops the obsolete `paths` second arg from resolveCustomBanner, and adds the trust-gate sentence. - "Security & failure handling" table replaces the stripTerminalControlSequences shorthand with the actual banner-specific stripper, splits the title/subtitle row to cover both, and adds the untrusted-workspace gate as its own row. - "Verification plan" gains two scenarios: the subtitle row, and the untrusted-workspace check that the Critical reviewer comment on the impl PR explicitly asked us to lock down. Mirrored every edit in the zh-CN translation. The implementation itself is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address banner re-review (FIFO, mutex schema, display width, regex dedupe) Addresses the five findings on PR #3710 from the latest re-review: 1. **[Critical] FIFO/pipe at `customAsciiArt.path` no longer hangs startup.** The resolver was calling `openSync(path, O_NOFOLLOW)` *before* the `fstatSync(...).isFile()` check; on POSIX, opening a FIFO read-only blocks until a writer connects, and `O_NOFOLLOW` doesn't help — it only refuses symlinks at the final path component. `readArtFile` now `lstatSync()`s first and refuses non-regular files (FIFO / socket / device / symlink) before the open, while keeping the post-open `fstatSync` check for TOCTOU safety against a swap between the lstat and the open. New POSIX-only regression test `mkfifo`s a named pipe and asserts the resolver soft-fails inside 1 s; if the open ever regresses to blocking, the test will hang past the timeout and the assertion will catch it. 2. **[Suggestion] `{path}` and `{small,large}` are now mutually exclusive in both schema and runtime.** The `jsonSchemaOverride` on `ui.customAsciiArt` is split into three branches (string, `{path}`, `{small?, large?}`); none of them allow `path` and tier keys to co-exist. `normalizeTiers()` mirrors that — an object carrying both kinds of keys is now soft-rejected with a `[BANNER]` warn rather than letting `path` silently win and dropping the tier values. New regression test pins the runtime side. 3. **[Suggestion] Column cap and tier-fit selection now measure in terminal cells.** `getAsciiArtWidth` (in `textUtils.ts`) and the `MAX_ART_COLS` cap in `customBanner.ts` were both using UTF-16 `.length`, so 200 CJK fullwidth characters would slip the cap and render at ~400 cells, and `pickAsciiArtTier`'s width-fit check was wrong for any non-ASCII art. Switched both to `getCachedStringWidth` (string-width semantics, already in the repo); art truncation walks code points until adding another would push the cell width past the cap, so we never split a fullwidth code point or surrogate pair down the middle. New regression test exercises the CJK fullwidth case. 4. **[Suggestion] `collectScopedTiers()` no longer drops a whole scope just because it has no `file.path`.** Inline-string tiers don't need an owning settings directory; only `{path}` tiers do. The path-presence check was moved into the `{path}` branch, so a path-less scope (e.g. `systemDefaults`, future SDK-injected scopes) can still contribute inline art. `{path}` entries in such a scope soft-fail with a tier-specific `[BANNER]` warn rather than killing the whole scope. Two regression tests cover both sides. 5. **[Suggestion] OSC / CSI / SS2-3 regex are now authored once.** Extracted `TERMINAL_OSC_REGEX`, `TERMINAL_CSI_REGEX`, `TERMINAL_SHIFT_DCS_REGEX` from `stripTerminalControlSequences` in `@qwen-code/qwen-code-core` and re-export them from the package index. `customBanner.ts` reuses the constants for `sanitizeArt` (which still has to preserve `\n` / `\t`) and delegates the title/subtitle pipeline directly to `stripTerminalControlSequences`. Also backported the C1 control strip (0x80-0x9F) into the core helper so all callers (session-title, etc.) benefit from the same coverage; banner sanitizer was the only place catching single-byte CSI / DCS / ST. Banner suite is now 40 + 17 + 6 + 16 = 79 tests, all green. Schema regen is still byte-for-byte idempotent. `npm run typecheck` and prettier clean on touched files. * fix(cli): replace require() with ES6 import in FIFO test (lint) The FIFO regression test in 7ccbfae used a synchronous `require()` to pull in `node:child_process` so the test could lazy-load `execFileSync` only when needed. CI Lint flagged it under `no-restricted-syntax` — the repo enforces ES6 imports throughout, including in tests, with no exception for `require()`. Move the import to the top of the file alongside the other `node:` / vitest imports. The `try/catch` around `execFileSync('mkfifo', ...)` still gates the test on `mkfifo` being available (rare on a fresh container, so we skip rather than fail). 40 / 40 tests still pass and ESLint is clean on the touched file. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
DragonnZhang
pushed a commit
that referenced
this pull request
May 8, 2026
Add ui.customBannerTitle, ui.customBannerSubtitle, and ui.customAsciiArt to the user-facing settings table. Also reword ui.hideBanner to note that it covers both the logo column and the info panel and that Tips render independently. These settings landed in #3710 but only ui.hideBanner was listed in the table, so users had no way to discover the other three short of reading the schema or the design doc.
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
* docs(design): add banner customization design (QwenLM#3005) Document the design for issue QwenLM#3005 (customize CLI banner area). Covers the banner region taxonomy and what is replaceable vs. locked, the three proposed settings (`ui.hideBanner`, `ui.customBannerTitle`, `ui.customAsciiArt`) and their resolution pipeline, the schema additions and wiring touch points, five alternative shapes considered, and the security / failure-handling guards. Mirrored EN + zh-CN under `docs/design/customize-banner-area/`. No code changes in this commit; implementation lands in a follow-up PR. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * feat(cli): customize banner area (logo, title, hide) Adds three opt-in `ui.*` settings that let users replace brand chrome on startup while keeping the operational lines (version, auth, model, path) locked: `hideBanner`, `customBannerTitle`, `customAsciiArt` (string, {path}, or {small,large}). A new resolver in `packages/cli/src/ui/utils/customBanner.ts` walks the loaded settings, normalizes each tier per scope (so {path} resolves against the file that declared it), reads the file with O_NOFOLLOW and a 64 KB cap on POSIX, sanitizes via a banner-specific stripper that drops OSC/CSI/SS2/SS3 sequences while preserving newlines, and caps art at 200 lines × 200 cols and titles at 80 chars. Every soft failure logs a `[BANNER]` warn and falls through to the bundled QWEN logo or default brand title — banner config can never crash the CLI. `<Header />` now picks the widest custom tier that fits via a shared `pickAsciiArtTier` helper and falls back to `shortAsciiLogo` otherwise; `<AppHeader />` extends the existing `showBanner` gate to honor `hideBanner` alongside the screen-reader fallback. Tracks QwenLM#3005 and the design merged in QwenLM#3671. * docs(design): apply prettier to banner customization design Reformats the EN and zh-CN design docs in `docs/design/customize-banner-area/` to satisfy `npx prettier --check`: table column alignment and trailing commas in `jsonc` examples. No content changes — the words, tables, and code blocks all say the same thing as before. Carries forward the only actionable feedback from the now-closed docs-only PR QwenLM#3671, where the prettier check was the sole change requested. * fix(cli): address banner audit findings Three audit-driven fixes for the banner customization feature: 1. **VSCode JSON schema accepts every documented shape.** The `ui.customAsciiArt` entry in `packages/vscode-ide-companion/schemas/settings.schema.json` was declared as `type: object`, which made VSCode flag the inline-string form (`"customAsciiArt": " ___"`) — a shape the runtime accepts and the design doc recommends — as a schema violation. Replaced with a `oneOf` covering string, `{path}`, and `{small,large}` (with each tier itself string-or-`{path}`). 2. **Narrow terminals no longer leak the QWEN logo over a white-label deployment.** When a user supplied custom ASCII art but neither tier fit the terminal, `Header.tsx` previously fell back to the bundled `shortAsciiLogo` — silently undoing the white-label intent on small windows. The fallback now distinguishes "user supplied custom art" from "no custom art at all": in the first case the logo column is hidden entirely (info panel still renders); in the second case the default logo shows as before. Soft-failure paths (missing file, sanitization rejection) still fall through to `shortAsciiLogo`. 3. **Sanitizer strips C1 control bytes (0x80-0x9F).** The art and title strippers previously stopped at 0x7F, leaving single-byte CSI (`0x9B`), DCS (`0x90`), ST (`0x9C`) and other C1 controls intact — which legacy 8-bit terminals would still interpret. Aligned the ranges with the repo's existing `stripUnsafeCharacters` (in `textUtils.ts`) so banner content can't carry interpreted control bytes through. New tests cover: C1 strip in art and title, absolute path reads, symlink rejection on POSIX, narrow-terminal hide-on-custom-art, and end-to-end `<AppHeader />` rendering through `resolveCustomBanner`. The full banner suite is 48 tests (was 42). * docs(design): clarify cross-scope tier merge and white-label fallback Two clarifications surfaced by the audit on the implementation PR: 1. The design said `customAsciiArt` follows standard merge precedence, but the resolver actually walks scopes per-tier so workspace can override only `large` while user keeps `small`. Document that this per-tier walk is intentional — both because each `{path}` has to resolve against the file that declared it (the merged view loses that information) and because it lets users keep a personal default tier and override the other one per-workspace. 2. The render-time tier-selection step now distinguishes "user supplied custom art but neither tier fits" (hide the logo column entirely; falling back to `shortAsciiLogo` would silently undo a white-label deployment on narrow terminals) from "user supplied no custom art at all" (fall through to `shortAsciiLogo` and let the default-logo width gate decide). Step 5's pure soft-failure fallback (missing file, sanitization rejection) is unchanged — still `shortAsciiLogo`. Mirrored both edits in the zh-CN translation. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(design): add size budget section to banner customization Question raised on the implementation PR: "why is the test logo `CCA` instead of the full `Custom Code Agent` — is there a character limit?" There is no character-count limit on titles or art. There is a **width budget** driven by terminal columns, plus an absolute hard cap (200×200 art, 80-char title) to keep malformed input from freezing layout. The existing user-facing guide didn't quantify the budget anywhere, so users were guessing why long inline names didn't render. Add a "How wide can the logo be? — the size budget" subsection that spells out the formula (`availableLogoWidth = terminalCols − 4 − 2 − 44`), tabulates it at 80 / 100 / 120 / 200 cols, calls out that a 17-char brand like "Custom Code Agent" can't render as a single ANSI Shadow line on most terminals (~120 cols of art), and shows the stacked-words `{ small, large }` recipe — including the `figlet` one-liner that generates the corresponding `banner-large.txt`. Mirrored in the zh-CN translation. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * docs(design): add limits-at-a-glance table; switch demo to Custom Agent The banner-customization design now has the size budget written down, but the per-cap limits (80-char title, 200×200 art, 64 KB file) were buried inside the size-budget formula table. Surface them as their own "Limits at a glance" subsection at the top of the user-configuration guide so users see the hard caps before they start hand-crafting art. Also switch the running example from "Custom Code Agent" (17 chars, ~120 cols of ANSI Shadow art on one line — too wide for any common terminal) to "Custom Agent" (12 chars, two-word stack at ~54 cols × 12 lines, fits any terminal ≥ 104 cols). The figlet recipe is now a two-word pipeline so a copy-paste run produces art the size the doc claims. Mirrored both changes in the zh-CN translation. The implementation itself is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address PR review + CI Lint failure Two reviewer findings on PR QwenLM#3710 (and the Lint job that fails for the same root cause): 1. **Schema regen now reproduces the committed JSON Schema.** The CI Lint step runs `npm run generate:settings-schema` and fails when the worktree dirties — my earlier hand-authored `oneOf` got blown away because `customAsciiArt` is `type: 'object'` in the source schema and the generator had no way to emit a union. Add a `jsonSchemaOverride` escape-hatch field on `SettingDefinition`: when set, the generator emits the override verbatim (description carried forward) instead of the type-driven shape. Set it on `customAsciiArt` to express the runtime union (string | {path} | {small,large} where each tier is itself string-or-{path}). The committed schema is now regenerated from source and CI's regenerate-and-diff check passes; two back-to-back regens produce identical output. 2. **Untrusted workspace settings no longer influence the banner.** `collectScopedTiers()` walked `settings.workspace` directly because per-scope file paths are needed to resolve relative `{path}` entries — but that bypassed the trust gate that `settings.merged` enforces. An untrusted checkout could therefore render its own ASCII art and trigger local file reads through a `{path}` entry before the user trusts the folder. Skip `settings.workspace` entirely when `settings.isTrusted` is false. Two regression tests cover the gate (untrusted = workspace silenced, falls through to user; trusted = workspace honored). Test suite for the banner is now 30 resolver tests + the existing Header / AppHeader / settingsSchema tests = 66 total, all green. * feat(cli): add ui.customBannerSubtitle for the spacer row Adds a fourth opt-in setting to the banner customization surface. The info panel renders four rows (title, subtitle/spacer, status, path); the second row was a hard-coded single-space spacer up to now. With this change a fork or white-label deployment can set `ui.customBannerSubtitle` to a one-line subtitle (e.g. "Built-in DataWorks Official Skills") and have it render in the secondary text color in place of the spacer. Empty/unset preserves the previous blank-spacer layout, so the change is back-compat. The subtitle is sanitized through the same `sanitizeSingleLine` helper as the title (now factored out): OSC / CSI / SS2 / SS3 leaders dropped, every other C0/C1 control byte replaced with a space, internal whitespace collapsed, ends trimmed. Capped at 160 characters — looser than the title's 80 because tagline / "powered by" copy commonly runs longer — with the same `[BANNER]` warn on truncation. Wiring: - `settingsSchema.ts` — new `customBannerSubtitle` entry next to `customBannerTitle`, `showInDialog: false` (free-form text in the TUI dialog isn't worth its own picker). - `customBanner.ts` — `ResolvedBanner.subtitle` field; `resolveCustomBanner` populates it; `sanitizeTitle` and the new `sanitizeSubtitle` share the same helper. - `Header.tsx` — when `customBannerSubtitle` is truthy the spacer row renders the string (secondary color, single line) instead of `<Text> </Text>`. Auth/model and path still sit at their usual positions. - `AppHeader.tsx` — pipes `resolvedBanner.subtitle` through. - VSCode JSON schema regenerated from source (idempotent). Tests: 5 new resolver tests (default, sanitize, length cap, empty, newline + C1 strip), 2 new Header tests (renders subtitle between title and auth; spacer preserved when unset), 1 new AppHeader integration test (end-to-end through resolver). Banner suite is now 35 + 17 + 6 + 16 = 74 tests, all green. Design docs (EN + zh-CN) updated: region taxonomy now lists four B-rows; "Limits at a glance" table grows a subtitle row; "Customization rules" matrix and "How to modify" section gain a "Add a brand subtitle" example with a rendered four-row preview. * docs(design): sweep stale 3-setting references after subtitle add Self-review found several sections of the banner customization design doc still framed for the original three settings; bring them in line with the four-setting reality landed in c7aa4a4: - Region taxonomy ASCII diagram now shows four B-rows (① title, ② subtitle, ③ status, ④ path). - Resolution-pipeline ASCII diagram and step list pick up customBannerSubtitle on the input side and the title/subtitle sanitize step on the resolver side. - "Settings schema additions" section lists the fourth entry, customBannerSubtitle, and notes the customAsciiArt jsonSchemaOverride that landed for VS Code schema reproducibility. - "Wiring changes" section updates the Header prop list and the HeaderProps interface, replaces the brittle line-number anchors with file-level anchors, drops the obsolete `paths` second arg from resolveCustomBanner, and adds the trust-gate sentence. - "Security & failure handling" table replaces the stripTerminalControlSequences shorthand with the actual banner-specific stripper, splits the title/subtitle row to cover both, and adds the untrusted-workspace gate as its own row. - "Verification plan" gains two scenarios: the subtitle row, and the untrusted-workspace check that the Critical reviewer comment on the impl PR explicitly asked us to lock down. Mirrored every edit in the zh-CN translation. The implementation itself is unchanged. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): address banner re-review (FIFO, mutex schema, display width, regex dedupe) Addresses the five findings on PR QwenLM#3710 from the latest re-review: 1. **[Critical] FIFO/pipe at `customAsciiArt.path` no longer hangs startup.** The resolver was calling `openSync(path, O_NOFOLLOW)` *before* the `fstatSync(...).isFile()` check; on POSIX, opening a FIFO read-only blocks until a writer connects, and `O_NOFOLLOW` doesn't help — it only refuses symlinks at the final path component. `readArtFile` now `lstatSync()`s first and refuses non-regular files (FIFO / socket / device / symlink) before the open, while keeping the post-open `fstatSync` check for TOCTOU safety against a swap between the lstat and the open. New POSIX-only regression test `mkfifo`s a named pipe and asserts the resolver soft-fails inside 1 s; if the open ever regresses to blocking, the test will hang past the timeout and the assertion will catch it. 2. **[Suggestion] `{path}` and `{small,large}` are now mutually exclusive in both schema and runtime.** The `jsonSchemaOverride` on `ui.customAsciiArt` is split into three branches (string, `{path}`, `{small?, large?}`); none of them allow `path` and tier keys to co-exist. `normalizeTiers()` mirrors that — an object carrying both kinds of keys is now soft-rejected with a `[BANNER]` warn rather than letting `path` silently win and dropping the tier values. New regression test pins the runtime side. 3. **[Suggestion] Column cap and tier-fit selection now measure in terminal cells.** `getAsciiArtWidth` (in `textUtils.ts`) and the `MAX_ART_COLS` cap in `customBanner.ts` were both using UTF-16 `.length`, so 200 CJK fullwidth characters would slip the cap and render at ~400 cells, and `pickAsciiArtTier`'s width-fit check was wrong for any non-ASCII art. Switched both to `getCachedStringWidth` (string-width semantics, already in the repo); art truncation walks code points until adding another would push the cell width past the cap, so we never split a fullwidth code point or surrogate pair down the middle. New regression test exercises the CJK fullwidth case. 4. **[Suggestion] `collectScopedTiers()` no longer drops a whole scope just because it has no `file.path`.** Inline-string tiers don't need an owning settings directory; only `{path}` tiers do. The path-presence check was moved into the `{path}` branch, so a path-less scope (e.g. `systemDefaults`, future SDK-injected scopes) can still contribute inline art. `{path}` entries in such a scope soft-fail with a tier-specific `[BANNER]` warn rather than killing the whole scope. Two regression tests cover both sides. 5. **[Suggestion] OSC / CSI / SS2-3 regex are now authored once.** Extracted `TERMINAL_OSC_REGEX`, `TERMINAL_CSI_REGEX`, `TERMINAL_SHIFT_DCS_REGEX` from `stripTerminalControlSequences` in `@qwen-code/qwen-code-core` and re-export them from the package index. `customBanner.ts` reuses the constants for `sanitizeArt` (which still has to preserve `\n` / `\t`) and delegates the title/subtitle pipeline directly to `stripTerminalControlSequences`. Also backported the C1 control strip (0x80-0x9F) into the core helper so all callers (session-title, etc.) benefit from the same coverage; banner sanitizer was the only place catching single-byte CSI / DCS / ST. Banner suite is now 40 + 17 + 6 + 16 = 79 tests, all green. Schema regen is still byte-for-byte idempotent. `npm run typecheck` and prettier clean on touched files. * fix(cli): replace require() with ES6 import in FIFO test (lint) The FIFO regression test in 7ccbfae used a synchronous `require()` to pull in `node:child_process` so the test could lazy-load `execFileSync` only when needed. CI Lint flagged it under `no-restricted-syntax` — the repo enforces ES6 imports throughout, including in tests, with no exception for `require()`. Move the import to the top of the file alongside the other `node:` / vitest imports. The `try/catch` around `execFileSync('mkfifo', ...)` still gates the test on `mkfifo` being available (rare on a fresh container, so we skip rather than fail). 40 / 40 tests still pass and ESLint is clean on the touched file. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
Add ui.customBannerTitle, ui.customBannerSubtitle, and ui.customAsciiArt to the user-facing settings table. Also reword ui.hideBanner to note that it covers both the logo column and the info panel and that Tips render independently. These settings landed in QwenLM#3710 but only ui.hideBanner was listed in the table, so users had no way to discover the other three short of reading the schema or the design doc.
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
2 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.
Summary
Implements the banner customization design merged in #3671 (tracks #3005). Adds three opt-in
ui.*settings that let users replace brand chrome on startup while keeping the operational lines (version, auth, model, working directory) locked.ui.hideBannerui.customBannerTitle>_ Qwen Codetitle. Version suffix is always appended.ui.customAsciiArt{path}, or{small, large}for width-aware tiers.{ "ui": { "hideBanner": false, "customBannerTitle": "Acme CLI", "customAsciiArt": { "small": " ACME\n ----", "large": { "path": "./brand-wide.txt" } } } }What stays locked (operational signals): version suffix, auth + model line, working directory. Per the design rationale these are critical for support and security posture.
Implementation notes
packages/cli/src/ui/utils/customBanner.tsnormalizes per-scope so each{path}resolves against the file that declared it (workspace.qwen/vs. user~/.qwen/), then reads withO_NOFOLLOWand a 64 KB cap on POSIX (Windows: plain read-only).\n, then caps art at 200 lines × 200 cols and titles at 80 characters.[BANNER]warn and falls back to the bundled QWEN logo or default brand title. The CLI never crashes on a banner config error.<Header />picks the widest custom tier that fits via a sharedpickAsciiArtTierhelper and falls back toshortAsciiLogootherwise.<AppHeader />extends the existingshowBannergate to honorhideBanneralongside the screen-reader fallback.customAsciiArtandcustomBannerTitleareshowInDialog: falsebecause a multi-line ASCII editor in the TUI is its own project; power users editsettings.jsondirectly.hideBannermirrorshideTips(showInDialog: true).Test plan
npx vitest run— all 4648 tests inpackages/clipass (incl. 24 new resolver tests + 5 new Header/AppHeader tests).npx tsc --noEmit -p packages/cli/tsconfig.json— clean.npx eslinton changed files — clean.~/.qwen/settings.jsonwithcustomBannerTitle: \"Acme CLI\"+ inlinecustomAsciiArtshows the new title and art; version + auth + model + path still visible.hideBanner: truestarts with no banner; tips and chat render normally.customAsciiArt: { \"small\": ..., \"large\": ... }swaps tiers correctly when resizing; logo column hidden at very narrow widths.\\x1b[31mhostileintocustomBannerTitle— renders as literal text, not painted red.pathat a missing file — CLI starts;[BANNER]warn appears in~/.qwen/debug/<sessionId>.txt; default art renders.Now bundles both the design docs (originally in #3671, now closed) and the implementation. Single PR for review.
example