fix: replace chalk.yellow with theme-aware hex in session-directory warning#229
fix: replace chalk.yellow with theme-aware hex in session-directory warning#229LifeJiggy wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: a3db91c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4120b6f0e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -0,0 +1,39 @@ | |||
| import { describe, expect, it } from 'vitest'; | |||
There was a problem hiding this comment.
The local apps/kimi-code/AGENTS.md test placement rule says not to create a generic feature test just to land a small change; this new top-level theme-colors.test.ts only checks existing theme exports/helpers and duplicates coverage that can live with the existing TUI/theme-related tests. Keeping it as a separate generic file makes the test tree noisier and violates that directory's documented maintenance rule, so please fold these assertions into the corresponding existing theme/TUI state suite instead.
Useful? React with 👍 / 👎.
… guard test bugs
- Move theme-colors.test.ts assertions into existing terminal-theme.test.ts to
avoid a generic feature test file (Codex P2)
- Fix chalk-named-color-guard.test.ts:
- Replace require('fs') with ESM import (readdirSync) (P1)
- Fix over-broad comment-skip logic — use proper inBlockComment state
tracker instead of line.includes('*') which silently skipped valid
code lines containing asterisks (P1)
- Remove dead code (const entries = [['dir', '']]) (P2)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3db91c748
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (target.workDir !== workDir) { | ||
| stderr.write( | ||
| `${chalk.yellow( | ||
| `${chalk.hex('#E8A838')( |
There was a problem hiding this comment.
When kimi -p -r <session> hits the wrong-directory path on a light/white terminal, this hard-coded dark-theme warning color has only about 2.1:1 contrast against white; the palette already uses lightColors.warning (#92660A, about 5.1:1) for this case. Because print mode has no theme detection here, replacing ANSI yellow with the dark palette hex makes this warning difficult to read for light-terminal users instead of fixing the contrast problem.
Useful? React with 👍 / 👎.
1 participant
Related Issue
Resolve #177
Problem
kimi-tui.tsusedchalk.yellow(a named chalk color) to render a session-directory mismatch warning. Named chalk colors bypass the theme system, so the warning does not respond to theme switches and may have poor contrast in light mode.The same pattern existed in
run-prompt.ts.What changed
chalk.yellow(...)withchalk.hex(this.state.theme.colors.warning)(...), making the warning respect the active theme's color palette.chalk.yellow(...)withchalk.hex('#E8A838')(...)for the same warning message (CLI code has no theme access).apps/kimi-code/src/forchalk.<named_color>(patterns and rejects them, preventing regressions.ColorPalette.warningexists in both themes, andThemeStyles.warning()correctly wraps text.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.