fix(core-app): stabilize Windows search launch diagnostics#258
Merged
TalexDreamSoul merged 23 commits intotalex-touch:masterfrom May 8, 2026
Merged
fix(core-app): stabilize Windows search launch diagnostics#258TalexDreamSoul merged 23 commits intotalex-touch:masterfrom
TalexDreamSoul merged 23 commits intotalex-touch:masterfrom
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 284 files, which is 134 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (284)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughWindows diagnostics, search, and UI improvements for Core App: corrupted app display-name detection with fallback rendering, Windows Store app indexing by absolute path, persistent main-process logging via log4js, enhanced foreground-process detection with structured error logging, Everything backend attempt error tracking, and titlebar/window-controls styling adjustments. ChangesDisplay Name Corruption Detection and Search Integration
Logger Persistence Infrastructure
Active App Detection Improvements
Everything Provider Backend Diagnostics
Windows UI Layout and Title Bar Styling
Documentation Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/plan-prd/TODO.md (1)
327-336:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTask statistics table is stale after adding the new section.
The new "Windows Search/Launch Stabilization" section adds 8
[x]and 2[ ]items, but the counts at lines 329–334 still read 139/21/160 from 2026-04-07. Ifpnpm docs:guardvalidates these counters, it will fail.📝 Suggested update
-| 已完成 (`- [x]`) | 139 | -| 未完成 (`- [ ]`) | 21 | -| 总计 | 160 | -| 完成率 | 87% | +| 已完成 (`- [x]`) | 147 | +| 未完成 (`- [ ]`) | 23 | +| 总计 | 170 | +| 完成率 | 86% |-> 统计时间: 2026-04-07（按本文件实时 checkbox 计数）。 +> 统计时间: 2026-05-05（按本文件实时 checkbox 计数）。🤖 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 `@docs/plan-prd/TODO.md` around lines 327 - 336, Update the task statistics table to reflect the new "Windows Search/Launch Stabilization" section: increment 已完成 (`- [x]`) by 8 (139 -> 147), 未完成 (`- [ ]`) by 2 (21 -> 23), 总计 by 10 (160 -> 170) and set 完成率 to 86% (147/170); also update the “统计时间” date to the current snapshot date and ensure the table row labels remain "已完成 (`- [x]`)", "未完成 (`- [ ]`)", "总计", and "完成率" so pnpm docs:guard validation passes.
🧹 Nitpick comments (3)
apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts (2)
329-352: ⚡ Quick winDual
bundleId/bundle_idaccessor smells like API drift.
AppLaunchMetadataaccepts bothbundle_id(the snake_case key emitted bysearch-processing-service.ts) andbundleId(camelCase).onExecutereadslaunchMeta.bundleId || launchMeta.bundle_id, butbuildProcessedAppItemonly emitsbundle_id. Pick one shape (camelCase to matchappUserModelId/launchKind) and drop the alias to prevent future producers from drifting again.Proposed cleanup
- type AppLaunchMetadata = { - path?: string - bundle_id?: string - bundleId?: string + type AppLaunchMetadata = { + path?: string + bundleId?: string…and update
search-processing-service.tsto emitbundleIdconsistently.🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts` around lines 329 - 352, AppLaunchMetadata currently accepts both bundle_id and bundleId which causes API drift; choose one canonical key (prefer camelCase: bundleId to match appUserModelId/launchKind), remove bundle_id from the AppLaunchMetadata type and any fallback logic (e.g. the launchMeta.bundleId || launchMeta.bundle_id check in onExecute), and update the producer buildProcessedAppItem (and search-processing-service.ts) to emit bundleId consistently so all consumers use AppLaunchMetadata.bundleId only.
1785-1918: 💤 Low value
launchAppflow LGTM, butvoid launchApp(...)swallows unhandled rejections.The shortcut-first → direct-path → spawn fallback chain matches the report's intent and includes the AppsFolder/explorer fallback. One small concern:
void this.launchApp(...)at the call site (Line 1780) detaches the promise; the inner method already wraps everything intry/catch, but adding a.catch(...)on the floating promise (or marking the callawait-able for tests) would prevent any future unhandled-rejection regression if athrowis introduced before the existingtryboundary.🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts` around lines 1785 - 1918, The PR currently calls launchApp with a detached promise (void this.launchApp(...)) which can swallow future unhandled rejections; update the call site to either await this.launchApp(...) or append a safe rejection handler like this.launchApp(...).catch(err => logApp(`launchApp promise rejected: ${err instanceof Error ? err.message : String(err)}`, LogStyle.error, { stack: err instanceof Error ? err.stack : undefined })) so any thrown errors from launchApp (the method defined as private async launchApp(...)) are surfaced/logged; no changes needed inside spawnAppFallback other than ensuring launchApp rejections are handled at the caller.apps/core-app/src/main/modules/box-tool/addon/apps/win.ts (1)
411-439: 💤 Low value
getAppInfoAppsFolder/URL branches omitdisplayName.The Start Menu shortcut and direct-path branches set
displayName: appName, but the AppsFolder and URL branches return objects withoutdisplayName. Consumers inapp-provider.tsderive keywords/title fromdisplayName/name, and downstream_mapDbAppToScannedInfoalready tolerates missing values, but for consistency (and to give the AUMID/URL launcher a non-noisy human-readable label) consider populatingdisplayName: appUserModelId/displayName: filePathhere too — or document why it's intentionally omitted.🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/win.ts` around lines 411 - 439, The AppsFolder and URL branches in getAppInfo omit displayName; add displayName: appUserModelId to the APPS_FOLDER_PREFIX branch and displayName: filePath to the isUrlLaunchId branch so both return objects include a human-readable label (refer to symbols APPS_FOLDER_PREFIX, appUserModelId, isUrlLaunchId, filePath and the getAppInfo return object).
🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts`:
- Around line 1199-1209: The function _generateKeywordsForApp currently uses
appInfo.description and appInfo.appUserModelId as sources and can inherit
AUMID-like strings into the description-based matching path; change the logic so
you skip adding description-derived keywords when the description is equal to or
appears to be an AUMID (e.g., contains characters typical of AUMIDs like '.' '!'
'_' or matches a short-manifest-token pattern), keep using appUserModelId only
for validation (don’t treat it as a description), and ensure
INVALID_KEYWORD_REGEX still filters raw tokens; update _generateKeywordsForApp
to detect and ignore AUMID-like descriptions before splitting/adding keywords
(refer to _generateKeywordsForApp, INVALID_KEYWORD_REGEX, and
appInfo.description / appInfo.appUserModelId).
In `@apps/core-app/src/main/modules/box-tool/addon/apps/win.ts`:
- Around line 366-379: The code currently sets description:
toManifestDisplayName(entry.Description) || appUserModelId which uses the raw
AUMID as a fallback and pollutes keyword/indexing; update the apps.push object
(the block that builds each app entry) to remove the fallback so description is
only set from toManifestDisplayName(entry.Description) (i.e., make description
undefined when toManifestDisplayName returns falsy) — this change affects how
_generateKeywordsForApp and processSearchResults receive description and
prevents AUMID strings from being treated as descriptions/keywords.
In `@apps/core-app/src/main/modules/system/active-app.ts`:
- Around line 392-396: When a Windows probe resolves successfully (the awaited
resolveTask yields a truthy result), also reset the warning cooldown so future
failures aren't muted: in the same branch where you set
this.windowsFailureBackoffUntil = 0 (inside the if (result) block after awaiting
resolveTask), also set this.windowsFailureLogCooldownUntil = 0 so the cooldown
is cleared on success.
In `@apps/core-app/src/main/polyfills.ts`:
- Around line 45-49: The override of consoleRef.info currently routes all
messages to globalThis.logger.info which will be suppressed in packaged builds;
restore a native path by always invoking consoleRef._info(message, ...args) so
the original console output is preserved, and then conditionally call
globalThis.logger.info only when appropriate (e.g., when
globalThis.logger.isLevelEnabled('info') or similar) to avoid double-logging;
update the implementation in the consoleRef.info override and keep the saved
original as consoleRef._info for direct use.
In `@docs/plan-prd/TODO.md`:
- Line 364: Update the checklist item in TODO.md to fix product-name casing:
replace "wechat" with "WeChat" and "apple music" with "Apple Music" in the line
that reads "Capture post-fix measurements for `微信`, `wechat`, `codex`, `apple
music`, Everything status and app DB table sizes." — leave the other tokens
(`微信`, `codex`, Everything status, app DB table sizes) unchanged.
---
Outside diff comments:
In `@docs/plan-prd/TODO.md`:
- Around line 327-336: Update the task statistics table to reflect the new
"Windows Search/Launch Stabilization" section: increment 已完成 (`- [x]`) by 8 (139
-> 147), 未完成 (`- [ ]`) by 2 (21 -> 23), 总计 by 10 (160 -> 170) and set 完成率 to 86%
(147/170); also update the “统计时间” date to the current snapshot date and ensure
the table row labels remain "已完成 (`- [x]`)", "未完成 (`- [ ]`)", "总计", and "完成率" so
pnpm docs:guard validation passes.
---
Nitpick comments:
In `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts`:
- Around line 329-352: AppLaunchMetadata currently accepts both bundle_id and
bundleId which causes API drift; choose one canonical key (prefer camelCase:
bundleId to match appUserModelId/launchKind), remove bundle_id from the
AppLaunchMetadata type and any fallback logic (e.g. the launchMeta.bundleId ||
launchMeta.bundle_id check in onExecute), and update the producer
buildProcessedAppItem (and search-processing-service.ts) to emit bundleId
consistently so all consumers use AppLaunchMetadata.bundleId only.
- Around line 1785-1918: The PR currently calls launchApp with a detached
promise (void this.launchApp(...)) which can swallow future unhandled
rejections; update the call site to either await this.launchApp(...) or append a
safe rejection handler like this.launchApp(...).catch(err => logApp(`launchApp
promise rejected: ${err instanceof Error ? err.message : String(err)}`,
LogStyle.error, { stack: err instanceof Error ? err.stack : undefined })) so any
thrown errors from launchApp (the method defined as private async
launchApp(...)) are surfaced/logged; no changes needed inside spawnAppFallback
other than ensuring launchApp rejections are handled at the caller.
In `@apps/core-app/src/main/modules/box-tool/addon/apps/win.ts`:
- Around line 411-439: The AppsFolder and URL branches in getAppInfo omit
displayName; add displayName: appUserModelId to the APPS_FOLDER_PREFIX branch
and displayName: filePath to the isUrlLaunchId branch so both return objects
include a human-readable label (refer to symbols APPS_FOLDER_PREFIX,
appUserModelId, isUrlLaunchId, filePath and the getAppInfo return object).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75c2ff5f-eaa4-4a4a-a320-129be71e3d04
📒 Files selected for processing (24)
apps/core-app/src/main/config/default.tsapps/core-app/src/main/modules/box-tool/addon/apps/app-provider.test.tsapps/core-app/src/main/modules/box-tool/addon/apps/app-provider.tsapps/core-app/src/main/modules/box-tool/addon/apps/app-types.tsapps/core-app/src/main/modules/box-tool/addon/apps/display-name-sync-utils.test.tsapps/core-app/src/main/modules/box-tool/addon/apps/display-name-sync-utils.tsapps/core-app/src/main/modules/box-tool/addon/apps/search-processing-service.test.tsapps/core-app/src/main/modules/box-tool/addon/apps/search-processing-service.tsapps/core-app/src/main/modules/box-tool/addon/apps/win.tsapps/core-app/src/main/modules/box-tool/addon/files/everything-provider.test.tsapps/core-app/src/main/modules/box-tool/addon/files/everything-provider.tsapps/core-app/src/main/modules/system/active-app.test.tsapps/core-app/src/main/modules/system/active-app.tsapps/core-app/src/main/polyfills.tsapps/core-app/src/main/utils/logger.tsapps/core-app/src/renderer/src/styles/layout/_layout-shell.scssapps/core-app/src/renderer/src/views/layout/flat/FlatLayout.vueapps/core-app/src/renderer/src/views/layout/shared/LayoutShell.vueapps/core-app/src/shared/events/everything.tsdocs/plan-prd/01-project/CHANGES.mddocs/plan-prd/TODO.mddocs/plan-prd/report/windows-search-diagnostics-2026-05-05.mdpackages/utils/common/logger/logger-manager.tspackages/utils/common/logger/module-logger.ts
Comment on lines
+392
to
+396
| try { | ||
| const result = await resolveTask | ||
| if (result) { | ||
| this.windowsFailureBackoffUntil = 0 | ||
| } |
Contributor
There was a problem hiding this comment.
Reset the warning cooldown after a successful Windows probe.
windowsFailureLogCooldownUntil survives successful resolutions, so a fresh failure within the next 10s is still muted even after the resolver has recovered. That makes intermittent Windows issues harder to diagnose. Clear the cooldown alongside windowsFailureBackoffUntil when result is truthy.
Suggested fix
try {
const result = await resolveTask
if (result) {
this.windowsFailureBackoffUntil = 0
+ this.windowsFailureLogCooldownUntil = 0
}
return result
} finally {🤖 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 `@apps/core-app/src/main/modules/system/active-app.ts` around lines 392 - 396,
When a Windows probe resolves successfully (the awaited resolveTask yields a
truthy result), also reset the warning cooldown so future failures aren't muted:
in the same branch where you set this.windowsFailureBackoffUntil = 0 (inside the
if (result) block after awaiting resolveTask), also set
this.windowsFailureLogCooldownUntil = 0 so the cooldown is cleared on success.
Comment on lines
+45
to
+49
| consoleRef._info = consoleRef.info | ||
| consoleRef.info = (message?: unknown, ...args: unknown[]) => { | ||
| if (args?.length) globalThis.logger.info(message, args) | ||
| else globalThis.logger.info(message) | ||
| } |
Contributor
There was a problem hiding this comment.
Don't turn console.info() into a production no-op.
This new override sends every console.info() through globalThis.logger.info(), but Lines 81-83 later raise that logger to "warn" in packaged builds. The result is that info-level startup diagnostics from Electron/dependencies are silently dropped instead of being preserved. Please keep a native consoleRef._info path, or route these messages through a sink whose persistence is not gated by the console verbosity level.
🤖 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 `@apps/core-app/src/main/polyfills.ts` around lines 45 - 49, The override of
consoleRef.info currently routes all messages to globalThis.logger.info which
will be suppressed in packaged builds; restore a native path by always invoking
consoleRef._info(message, ...args) so the original console output is preserved,
and then conditionally call globalThis.logger.info only when appropriate (e.g.,
when globalThis.logger.isLevelEnabled('info') or similar) to avoid
double-logging; update the implementation in the consoleRef.info override and
keep the saved original as consoleRef._info for direct use.
| - [x] Record performance and index analysis report: `docs/plan-prd/report/windows-search-diagnostics-2026-05-05.md`. | ||
| - [x] Repair corrupted Windows app display names from stale index data: bad `display_name` values containing replacement/square glyphs now fall back to a clean `name` and are corrected during backfill/full sync. | ||
| - [ ] Re-run target Vitest, `typecheck:web`, dev launch, and production DB snapshot analysis once approval service / child-process permissions are available. | ||
| - [ ] Capture post-fix measurements for `微信`, `wechat`, `codex`, `apple music`, Everything status and app DB table sizes. |
Contributor
There was a problem hiding this comment.
Fix brand-name casing: wechat → WeChat, apple music → Apple Music.
These are proper product names; incorrect casing is flagged by the project's LanguageTool linter.
📝 Proposed fix
-- [ ] Capture post-fix measurements for `微信`, `wechat`, `codex`, `apple music`, Everything status and app DB table sizes.
+- [ ] Capture post-fix measurements for `微信`, `WeChat`, `codex`, `Apple Music`, Everything status and app DB table sizes.🧰 Tools
🪛 LanguageTool
[uncategorized] ~364-~364: The official name of this popular chat service is spelled with a capital “C”.
Context: ...Capture post-fix measurements for 微信, wechat, codex, apple music, Everything st...
(WECHAT)
[uncategorized] ~364-~364: Did you mean the proper noun “Apple Music”?
Context: ...asurements for 微信, wechat, codex, apple music, Everything status and app DB table si...
(APPLE_PRODUCTS)
🤖 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 `@docs/plan-prd/TODO.md` at line 364, Update the checklist item in TODO.md to
fix product-name casing: replace "wechat" with "WeChat" and "apple music" with
"Apple Music" in the line that reads "Capture post-fix measurements for `微信`,
`wechat`, `codex`, `apple music`, Everything status and app DB table sizes." —
leave the other tokens (`微信`, `codex`, Everything status, app DB table sizes)
unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts (2)
647-676:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCleared managed-entry metadata is never removed here.
This update path only re-adds truthy extension rows. If a user clears
launchArgs,workingDirectory,description,alternateNames,icon, or similar fields, the oldfileExtensionsrows survive and the app can keep launching or searching with stale metadata after the edit.Suggested fix
+ await db + .delete(fileExtensions) + .where( + and( + eq(fileExtensions.fileId, existingFile.id), + inArray(fileExtensions.key, [ + 'bundleId', + 'icon', + APP_IDENTITY_EXTENSION_KEY, + APP_LAUNCH_KIND_EXTENSION_KEY, + APP_LAUNCH_TARGET_EXTENSION_KEY, + APP_LAUNCH_ARGS_EXTENSION_KEY, + APP_WORKING_DIRECTORY_EXTENSION_KEY, + APP_DISPLAY_PATH_EXTENSION_KEY, + APP_DESCRIPTION_EXTENSION_KEY, + APP_ALTERNATE_NAMES_EXTENSION_KEY, + APP_ENTRY_SOURCE_EXTENSION_KEY, + APP_ENTRY_ENABLED_EXTENSION_KEY + ]) + ) + ) + const nextExtensions = this.buildManagedEntryExtensions(existingFile.id, appInfo, enabled) await this.dbUtils.addFileExtensions(nextExtensions)🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts` around lines 647 - 676, The update path re-adds only truthy extension rows so cleared fields leave stale fileExtensions; before calling this.dbUtils.addFileExtensions(nextExtensions) compute which extension keys are now cleared by comparing existingExtensions (from existingFile.extensions) to the new nextExtensions (or by checking falsy values in buildManagedEntryExtensions output) and call a removal operation to delete those keys for the file (e.g., add a call like this.dbUtils.removeFileExtensions(fileId, keysToRemove) or a suitable DB delete method) so that entries for launchArgs, workingDirectory, description, alternateNames, icon, etc. that were cleared are deleted; then proceed to add nextExtensions and update the extensions map used in this.mapManagedEntry.
1345-1363:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThis file still has lint blockers.
These new blocks violate the current ESLint profile (
antfu/consistent-chaining,style/arrow-parens,style/operator-linebreak), so the closest quality gate for this file will still fail until they’re normalized.As per coding guidelines, "Run quality checks (lint/typecheck/test/build) closest to changed code without fixing unrelated failures".
Also applies to: 1609-1616, 1848-1860
🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts` around lines 1345 - 1363, The multi-call query creating insertedFile (this.dbUtils!.getDb().insert(filesSchema).values(...).onConflictDoUpdate(...).returning()) must be reformatted to satisfy the antfu/consistent-chaining, style/arrow-parens and style/operator-linebreak rules: break the chain so each method call starts on its own line (e.g. const [insertedFile] = await this.dbUtils!.getDb()\n .insert(filesSchema)\n .values({ ... })\n .onConflictDoUpdate({ ... })\n .returning(); ensure any arrow functions in nearby changed blocks use consistent parentheses around params, and place binary operators (if split across lines) at the end of the previous line rather than the start; re-run the linter to verify the change.
🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.test.ts`:
- Around line 215-216: The two single-line if statements using normalizedNext
and normalizedCurrent violate the antfu/if-newline rule; change both to
block-style if statements (i.e., use braces and put the return on the next line)
for the checks "if (typeof normalizedNext === 'string' &&
normalizedNext.includes('\uFFFD'))" and "if (typeof normalizedCurrent ===
'string' && normalizedCurrent.includes('\uFFFD'))" so each has its own newline
and braces around the return to satisfy ESLint.
- Around line 201-210: The mock is inconsistent: isProbablyCorruptedDisplayName
flags both '\uFFFD' and '\u25A1' as corrupted, but resolveDisplayName (and other
places around resolveDisplayName/normalizeDisplayName) only checks '\uFFFD';
update resolveDisplayName (and any other mock checks e.g., the later check at
the second occurrence noted) to treat '\u25A1' the same way as '\uFFFD' so the
mock consistently detects and falls back for both corruption characters (refer
to isProbablyCorruptedDisplayName and resolveDisplayName to locate the changes).
- Around line 709-713: The arrow function assigned to
privateProvider.fetchExtensionsForFiles uses parentheses around a single
parameter; update the function signature in the fetchExtensionsForFiles mock so
the single parameter is unparenthesized (i.e., change "(files) =>" to "files
=>") while leaving the body intact; reference the
privateProvider.fetchExtensionsForFiles mock and the dbRow usage to locate the
change.
In `@apps/core-app/src/main/modules/box-tool/addon/apps/win.test.ts`:
- Around line 163-166: The if statement inside statMock.mockImplementation uses
a single-line body and triggers antfu/if-newline; update the implementation in
statMock.mockImplementation to put the body on the next line (or use braces) so
the return is on its own line when target === appPath; locate the mock by the
symbol statMock.mockImplementation and the related identifiers
appPath/createFileStat and modify the block to use a newline (or a braced block)
before calling createFileStat().
In
`@apps/core-app/src/main/modules/box-tool/search-engine/recommendation/recommendation-engine.ts`:
- Around line 1719-1722: The current block in recommendation-engine (variable
appMeta and returned cache key using appMeta.bundleId) must fall back to the
legacy meta.app.bundle_id during the cache migration window; update the logic
that builds the cache key (the code that uses appMeta and sourceId) to check
both appMeta.bundleId and appMeta.bundle_id (e.g., use bundleId || bundle_id) so
cached entries written with the old field still resolve to the same key and
de-dupe/pinned filtering remains correct.
In `@apps/core-app/src/main/modules/system/active-app.ts`:
- Around line 60-77: Split single-line if-statements into multi-line blocks with
braces and a newline and remove unnecessary parens from single-parameter arrow
functions: in getCommandErrorCode change the single-line "if (!error || typeof
error !== 'object') return null" into a block with braces and a newline; in
getCommandErrorMessage change "if (error instanceof Error) return error.message"
into a braced multi-line if; and in extractJsonObjectLine replace "(line) =>"
occurrences with "line =>" for map and find callbacks so they follow the
configured arrow-parens rule. Ensure formatting matches project lint rules.
---
Outside diff comments:
In `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.ts`:
- Around line 647-676: The update path re-adds only truthy extension rows so
cleared fields leave stale fileExtensions; before calling
this.dbUtils.addFileExtensions(nextExtensions) compute which extension keys are
now cleared by comparing existingExtensions (from existingFile.extensions) to
the new nextExtensions (or by checking falsy values in
buildManagedEntryExtensions output) and call a removal operation to delete those
keys for the file (e.g., add a call like
this.dbUtils.removeFileExtensions(fileId, keysToRemove) or a suitable DB delete
method) so that entries for launchArgs, workingDirectory, description,
alternateNames, icon, etc. that were cleared are deleted; then proceed to add
nextExtensions and update the extensions map used in this.mapManagedEntry.
- Around line 1345-1363: The multi-call query creating insertedFile
(this.dbUtils!.getDb().insert(filesSchema).values(...).onConflictDoUpdate(...).returning())
must be reformatted to satisfy the antfu/consistent-chaining, style/arrow-parens
and style/operator-linebreak rules: break the chain so each method call starts
on its own line (e.g. const [insertedFile] = await this.dbUtils!.getDb()\n
.insert(filesSchema)\n .values({ ... })\n .onConflictDoUpdate({ ... })\n
.returning(); ensure any arrow functions in nearby changed blocks use consistent
parentheses around params, and place binary operators (if split across lines) at
the end of the previous line rather than the start; re-run the linter to verify
the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2630d26f-d964-4a75-948a-bbc9e2db4a45
📒 Files selected for processing (12)
apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.test.tsapps/core-app/src/main/modules/box-tool/addon/apps/app-provider.tsapps/core-app/src/main/modules/box-tool/addon/apps/search-processing-service.test.tsapps/core-app/src/main/modules/box-tool/addon/apps/search-processing-service.tsapps/core-app/src/main/modules/box-tool/addon/apps/win.test.tsapps/core-app/src/main/modules/box-tool/addon/apps/win.tsapps/core-app/src/main/modules/box-tool/addon/files/everything-provider.test.tsapps/core-app/src/main/modules/box-tool/addon/files/everything-provider.tsapps/core-app/src/main/modules/box-tool/search-engine/recommendation/recommendation-engine.tsapps/core-app/src/main/modules/box-tool/search-engine/search-core.tsapps/core-app/src/main/modules/system/active-app.test.tsapps/core-app/src/main/modules/system/active-app.ts
✅ Files skipped from review due to trivial changes (1)
- apps/core-app/src/main/modules/box-tool/addon/files/everything-provider.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/core-app/src/main/modules/box-tool/addon/apps/search-processing-service.test.ts
- apps/core-app/src/main/modules/box-tool/addon/apps/win.ts
Comment on lines
+201
to
+210
| isProbablyCorruptedDisplayName: vi.fn((value: string | null | undefined) => { | ||
| return typeof value === 'string' && (value.includes('\uFFFD') || value.includes('\u25A1')) | ||
| }), | ||
| normalizeDisplayName: vi.fn((value: string | null | undefined) => value ?? null), | ||
| resolveDisplayName: vi.fn((displayName: string | null | undefined, fallbackName: string) => { | ||
| if (typeof displayName === 'string' && displayName && !displayName.includes('\uFFFD')) { | ||
| return displayName | ||
| } | ||
| return fallbackName | ||
| }), |
Contributor
There was a problem hiding this comment.
Corruption mock logic is inconsistent for \u25A1 and can hide backfill regressions.
Line [201]-Line [203] treats both \uFFFD and \u25A1 as corrupted, but Line [206] and Line [215]-Line [216] only check \uFFFD. This makes the mock internally inconsistent and may miss repair/update behavior for \u25A1 inputs.
Suggested fix
resolveDisplayName: vi.fn((displayName: string | null | undefined, fallbackName: string) => {
- if (typeof displayName === 'string' && displayName && !displayName.includes('\uFFFD')) {
+ if (
+ typeof displayName === 'string'
+ && displayName
+ && !displayName.includes('\uFFFD')
+ && !displayName.includes('\u25A1')
+ ) {
return displayName
}
return fallbackName
}),
shouldUpdateDisplayName: vi.fn(
(current: string | null | undefined, next: string | null | undefined) => {
const normalizedCurrent = current ?? null
const normalizedNext = next ?? null
- if (typeof normalizedNext === 'string' && normalizedNext.includes('\uFFFD')) return false
- if (typeof normalizedCurrent === 'string' && normalizedCurrent.includes('\uFFFD')) return true
+ if (
+ typeof normalizedNext === 'string'
+ && (normalizedNext.includes('\uFFFD') || normalizedNext.includes('\u25A1'))
+ ) {
+ return false
+ }
+ if (
+ typeof normalizedCurrent === 'string'
+ && (normalizedCurrent.includes('\uFFFD') || normalizedCurrent.includes('\u25A1'))
+ ) {
+ return true
+ }
return normalizedCurrent !== normalizedNext
}
)Also applies to: 215-217
🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.test.ts`
around lines 201 - 210, The mock is inconsistent: isProbablyCorruptedDisplayName
flags both '\uFFFD' and '\u25A1' as corrupted, but resolveDisplayName (and other
places around resolveDisplayName/normalizeDisplayName) only checks '\uFFFD';
update resolveDisplayName (and any other mock checks e.g., the later check at
the second occurrence noted) to treat '\u25A1' the same way as '\uFFFD' so the
mock consistently detects and falls back for both corruption characters (refer
to isProbablyCorruptedDisplayName and resolveDisplayName to locate the changes).
Comment on lines
+215
to
+216
| if (typeof normalizedNext === 'string' && normalizedNext.includes('\uFFFD')) return false | ||
| if (typeof normalizedCurrent === 'string' && normalizedCurrent.includes('\uFFFD')) return true |
Contributor
There was a problem hiding this comment.
ESLint failure: if single-line statements violate antfu/if-newline.
Line [215] and Line [216] are currently written as one-line if statements and match the reported lint error. Please format them with newline/block style to unblock checks.
🧰 Tools
🪛 ESLint
[error] 215-215: Expect newline after if
(antfu/if-newline)
[error] 216-216: Expect newline after if
(antfu/if-newline)
🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.test.ts`
around lines 215 - 216, The two single-line if statements using normalizedNext
and normalizedCurrent violate the antfu/if-newline rule; change both to
block-style if statements (i.e., use braces and put the return on the next line)
for the checks "if (typeof normalizedNext === 'string' &&
normalizedNext.includes('\uFFFD'))" and "if (typeof normalizedCurrent ===
'string' && normalizedCurrent.includes('\uFFFD'))" so each has its own newline
and braces around the return to satisfy ESLint.
Comment on lines
+709
to
+713
| privateProvider.fetchExtensionsForFiles = vi.fn(async (files: unknown[]) => | ||
| files.map((file) => ({ | ||
| ...(file as typeof dbRow), | ||
| extensions: { appIdentity: 'path:d:\\weixin\\weixin.exe' } | ||
| })) |
Contributor
There was a problem hiding this comment.
ESLint failure: arrow function uses disallowed parens for single parameter.
Line [710] wraps a single arrow parameter in parentheses ((files) =>) while style/arrow-parens expects files => for this style rule.
Suggested fix
- privateProvider.fetchExtensionsForFiles = vi.fn(async (files: unknown[]) =>
+ privateProvider.fetchExtensionsForFiles = vi.fn(async files =>
files.map((file) => ({
...(file as typeof dbRow),
extensions: { appIdentity: 'path:d:\\weixin\\weixin.exe' }
}))
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| privateProvider.fetchExtensionsForFiles = vi.fn(async (files: unknown[]) => | |
| files.map((file) => ({ | |
| ...(file as typeof dbRow), | |
| extensions: { appIdentity: 'path:d:\\weixin\\weixin.exe' } | |
| })) | |
| privateProvider.fetchExtensionsForFiles = vi.fn(async files => | |
| files.map((file) => ({ | |
| ...(file as typeof dbRow), | |
| extensions: { appIdentity: 'path:d:\\weixin\\weixin.exe' } | |
| })) | |
| ) |
🧰 Tools
🪛 ESLint
[error] 710-710: Unexpected parentheses around single function argument having a body with no curly braces.
(style/arrow-parens)
🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/app-provider.test.ts`
around lines 709 - 713, The arrow function assigned to
privateProvider.fetchExtensionsForFiles uses parentheses around a single
parameter; update the function signature in the fetchExtensionsForFiles mock so
the single parameter is unparenthesized (i.e., change "(files) =>" to "files
=>") while leaving the body intact; reference the
privateProvider.fetchExtensionsForFiles mock and the dbRow usage to locate the
change.
Comment on lines
+163
to
+166
| statMock.mockImplementation(async (target: string) => { | ||
| if (target === appPath) return createFileStat() | ||
| throw new Error(`Unexpected stat path: ${target}`) | ||
| }) |
Contributor
There was a problem hiding this comment.
Fix missing newline after if (ESLint antfu/if-newline error).
Line 164 triggers a lint error — the rule requires a newline between the if condition and its body.
🔧 Proposed fix
statMock.mockImplementation(async (target: string) => {
- if (target === appPath) return createFileStat()
- throw new Error(`Unexpected stat path: ${target}`)
+ if (target === appPath)
+ return createFileStat()
+ throw new Error(`Unexpected stat path: ${target}`)
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| statMock.mockImplementation(async (target: string) => { | |
| if (target === appPath) return createFileStat() | |
| throw new Error(`Unexpected stat path: ${target}`) | |
| }) | |
| statMock.mockImplementation(async (target: string) => { | |
| if (target === appPath) | |
| return createFileStat() | |
| throw new Error(`Unexpected stat path: ${target}`) | |
| }) |
🧰 Tools
🪛 ESLint
[error] 164-164: Expect newline after if
(antfu/if-newline)
🤖 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 `@apps/core-app/src/main/modules/box-tool/addon/apps/win.test.ts` around lines
163 - 166, The if statement inside statMock.mockImplementation uses a
single-line body and triggers antfu/if-newline; update the implementation in
statMock.mockImplementation to put the body on the next line (or use braces) so
the return is on its own line when target === appPath; locate the mock by the
symbol statMock.mockImplementation and the related identifiers
appPath/createFileStat and modify the block to use a newline (or a braced block)
before calling createFileStat().
Comment on lines
1719
to
1722
| const appMeta = meta?.app | ||
| if (appMeta?.bundle_id) { | ||
| return `${sourceId}:bundle:${appMeta.bundle_id}` | ||
| if (appMeta?.bundleId) { | ||
| return `${sourceId}:bundle:${appMeta.bundleId}` | ||
| } |
Contributor
There was a problem hiding this comment.
Keep a legacy bundle_id fallback during the cache migration window.
getCachedRecommendations() can still hydrate items written before this rename. Those cached entries will carry meta.app.bundle_id, and falling back to path here can break de-dupe or pinned filtering for the same app until the recommendation cache expires—especially now that this PR also changes Windows app path semantics.
🤖 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
`@apps/core-app/src/main/modules/box-tool/search-engine/recommendation/recommendation-engine.ts`
around lines 1719 - 1722, The current block in recommendation-engine (variable
appMeta and returned cache key using appMeta.bundleId) must fall back to the
legacy meta.app.bundle_id during the cache migration window; update the logic
that builds the cache key (the code that uses appMeta and sourceId) to check
both appMeta.bundleId and appMeta.bundle_id (e.g., use bundleId || bundle_id) so
cached entries written with the old field still resolve to the same key and
de-dupe/pinned filtering remains correct.
Comment on lines
+60
to
+77
| function getCommandErrorCode(error: unknown): string | null { | ||
| if (!error || typeof error !== 'object') return null | ||
| const value = (error as { code?: unknown }).code | ||
| return typeof value === 'string' ? value : null | ||
| } | ||
|
|
||
| function getCommandErrorMessage(error: unknown): string { | ||
| if (error instanceof Error) return error.message | ||
| return String(error) | ||
| } | ||
|
|
||
| function extractJsonObjectLine(output: string): string { | ||
| const lines = output | ||
| .split(/\r?\n/) | ||
| .map((line) => line.trim()) | ||
| .filter(Boolean) | ||
| return lines.reverse().find((line) => line.startsWith('{') && line.endsWith('}')) || output.trim() | ||
| } |
Contributor
There was a problem hiding this comment.
Fix ESLint violations in the new helper block before merge.
Line 61, Line 67, Line 74, and Line 76 violate configured lint rules (antfu/if-newline, style/arrow-parens), which will fail quality gates.
Suggested patch
function getCommandErrorCode(error: unknown): string | null {
- if (!error || typeof error !== 'object') return null
+ if (!error || typeof error !== 'object')
+ return null
const value = (error as { code?: unknown }).code
return typeof value === 'string' ? value : null
}
function getCommandErrorMessage(error: unknown): string {
- if (error instanceof Error) return error.message
+ if (error instanceof Error)
+ return error.message
return String(error)
}
function extractJsonObjectLine(output: string): string {
const lines = output
.split(/\r?\n/)
- .map((line) => line.trim())
+ .map(line => line.trim())
.filter(Boolean)
- return lines.reverse().find((line) => line.startsWith('{') && line.endsWith('}')) || output.trim()
+ return lines.reverse().find(line => line.startsWith('{') && line.endsWith('}')) || output.trim()
}As per coding guidelines "Run quality checks (lint/typecheck/test/build) closest to changed code without fixing unrelated failures."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function getCommandErrorCode(error: unknown): string | null { | |
| if (!error || typeof error !== 'object') return null | |
| const value = (error as { code?: unknown }).code | |
| return typeof value === 'string' ? value : null | |
| } | |
| function getCommandErrorMessage(error: unknown): string { | |
| if (error instanceof Error) return error.message | |
| return String(error) | |
| } | |
| function extractJsonObjectLine(output: string): string { | |
| const lines = output | |
| .split(/\r?\n/) | |
| .map((line) => line.trim()) | |
| .filter(Boolean) | |
| return lines.reverse().find((line) => line.startsWith('{') && line.endsWith('}')) || output.trim() | |
| } | |
| function getCommandErrorCode(error: unknown): string | null { | |
| if (!error || typeof error !== 'object') | |
| return null | |
| const value = (error as { code?: unknown }).code | |
| return typeof value === 'string' ? value : null | |
| } | |
| function getCommandErrorMessage(error: unknown): string { | |
| if (error instanceof Error) | |
| return error.message | |
| return String(error) | |
| } | |
| function extractJsonObjectLine(output: string): string { | |
| const lines = output | |
| .split(/\r?\n/) | |
| .map(line => line.trim()) | |
| .filter(Boolean) | |
| return lines.reverse().find(line => line.startsWith('{') && line.endsWith('}')) || output.trim() | |
| } |
🧰 Tools
🪛 ESLint
[error] 61-61: Expect newline after if
(antfu/if-newline)
[error] 67-67: Expect newline after if
(antfu/if-newline)
[error] 74-74: Unexpected parentheses around single function argument having a body with no curly braces.
(style/arrow-parens)
[error] 76-76: Unexpected parentheses around single function argument having a body with no curly braces.
(style/arrow-parens)
🤖 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 `@apps/core-app/src/main/modules/system/active-app.ts` around lines 60 - 77,
Split single-line if-statements into multi-line blocks with braces and a newline
and remove unnecessary parens from single-parameter arrow functions: in
getCommandErrorCode change the single-line "if (!error || typeof error !==
'object') return null" into a block with braces and a newline; in
getCommandErrorMessage change "if (error instanceof Error) return error.message"
into a braced multi-line if; and in extractJsonObjectLine replace "(line) =>"
occurrences with "line =>" for map and find callbacks so they follow the
configured arrow-parens rule. Ensure formatting matches project lint rules.
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
Details
.lnk, args, cwd, and launcher uses shortcut first before target fallback.Get-StartAppsand launched via AppsFolder AUMID.display_namevalues containing replacement/square glyphs now fall back to the cleannamefield and are repaired during backfill/full sync.Get-StartAppswith absolute-path AppIDs are treated as path launches instead of fake AppsFolder entries.Verification
pnpm -C apps/core-app run typecheck:nodepnpm -C apps/core-app exec vitest run src/main/modules/box-tool/addon/apps/display-name-sync-utils.test.ts src/main/modules/box-tool/addon/apps/search-processing-service.test.ts src/main/modules/box-tool/addon/apps/app-provider.test.tsgit diff --checkNotes
typecheck:weband live dev launch were not rerun in the final encoding-display commit.C:/Program Files/GitHub CLI/gh.exebecause the current shell PATH had not refreshed.Summary by CodeRabbit
Release Notes
Bug Fixes
Chores