fix(skill): surface per-platform install failures + handle UNKNOWN symlink errors (#93)#124
Merged
Merged
Conversation
…mlink errors (legeling#93) Users trying to install skills via Symlink reported that the skill was 'not installed' without any error. Investigation showed two problems layered on top of each other: 1. In `batchInstall` (use-skill-platform.ts), individual per-platform install failures were caught, console.error'd, and then silently dropped. The caller only got `{ successCount, totalCount }`, so a partial failure still produced a green 'installation successful' toast that did not mention which platforms failed or why. 2. On Windows without Developer Mode / admin rights, `fs.symlink` permission failures can surface as code 'UNKNOWN' rather than the expected 'EPERM'. The existing fallback-to-copy path only handled EPERM / EACCES / ENOTSUP, so 'UNKNOWN' escaped the fallback and propagated up as an unexplained throw — the user saw no install. ### Changes - apps/desktop/src/renderer/components/skill/use-skill-platform.ts: `BatchInstallResult` now carries a `failures: BatchInstallFailure[]` list. Each caught error becomes a structured entry with the platformId and a localizable reason. - apps/desktop/src/renderer/components/skill/SkillFullDetailPage.tsx: The `batchInstall` handler now renders any failures as an explicit error toast, one platform per line, so the user can tell immediately which platforms failed and what the underlying error was. - apps/desktop/src/main/services/skill-installer-platform.ts: `installSkillMdSymlink` also falls back to copy on code 'UNKNOWN', and when no fallback applies it now throws an error message that includes the skill name, platform, error code, and original reason. The root cause is attached as `.cause` for diagnostics. - apps/desktop/tests/unit/main/skill-installer-platform.test.ts: new regression tests cover both the UNKNOWN fallback path and the richer error thrown for non-fallback failures. - i18n: new `skill.installPartialFailure` key across all 7 locales. ### Verification - `pnpm lint` → clean - `pnpm test -- --run tests/unit/main/skill-installer-platform.test.ts tests/unit/services/skill-platform-sync.test.ts` → 7/7 passing Fixes legeling#93
|
@TuTouPower is attempting to deploy a commit to the legeling's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 Walkthrough通览该 PR 增强了技能安装失败的错误处理和报告。后端现在支持 UNKNOWN 符号链接错误作为回退情况，并为所有非回退错误生成可操作的错误消息。前端批量安装功能现在跟踪每个平台的失败原因，在 UI 中显示失败摘要，并为 7 种语言添加了国际化支持。 变更技能安装失败处理与报告
🎯 3 (中等) | ⏱️ ~25 分钟 Sequence DiagramsequenceDiagram
participant User
participant UI
participant batchInstall
participant installer
participant fs
User->>UI: 发起批量安装
UI->>batchInstall: 调用 batchInstall(platforms)
loop 每个平台
batchInstall->>installer: installSkill(platformId)
installer->>fs: fs.symlink(...)
alt symlink success
fs-->>installer: success
else symlink error (EPERM/EACCES/ENOTSUP/UNKNOWN)
fs-->>installer: error
installer->>installer: fallbackInstall -> writeFile SKILL.md
else other symlink error
fs-->>installer: error
installer-->>batchInstall: throw formatted Error (.cause = original)
end
installer-->>batchInstall: result or error
end
batchInstall-->>UI: {successCount,totalCount,failures}
alt failures.length > 0
UI->>User: 显示 partial failure toast (skill.installPartialFailure)
else
UI->>User: 显示 success toast
end
诗
🚥 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)
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 |
Review Summary by QodoSurface per-platform install failures and handle UNKNOWN symlink errors
WalkthroughsDescription• Surface per-platform install failures in batch installer UI • Handle Windows UNKNOWN symlink errors with copy fallback • Add structured failure tracking to BatchInstallResult • Display platform-specific error messages in error toast • Add i18n key for partial installation failures Diagramflowchart LR
A["Symlink install attempt"] -->|UNKNOWN error| B["Fallback to copy"]
A -->|Other errors| C["Throw formatted error"]
C --> D["BatchInstallFailure captured"]
D --> E["Error toast with platform details"]
B --> F["Silent success"]
File Changes1. apps/desktop/src/main/services/skill-installer-platform.ts
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
- Drop Chinese comment lines introduced by this PR (AGENTS.md rule)
- Replace hand-assembled 'Label: reason' + newline string in the
partial-failure toast with i18n interpolation:
* new key `skill.installFailureRow` ({{platform}}: {{reason}})
* `skill.installPartialFailure` now takes {{details}}
added to all 7 locales so translators can change the separator
- Fix silent partial failures in SkillQuickInstall:
* handleInstall now reads result.failures and shows the same
error toast as the main detail page
* auto-close only when every selected platform succeeded; keep
the modal open on partial failure so the user can retry
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/skill/SkillQuickInstall.tsx (1)
26-26: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win导出的组件函数缺少显式返回类型注解
该组件函数已导出但未声明返回类型。根据编码规范，所有导出的函数必须具有显式的返回类型注解。
🔧 建议的修复
-export function SkillQuickInstall({ skill, onClose }: SkillQuickInstallProps) { +export function SkillQuickInstall({ skill, onClose }: SkillQuickInstallProps): JSX.Element {根据编码规范："All exported functions must have explicit return type annotations"
🤖 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/desktop/src/renderer/components/skill/SkillQuickInstall.tsx` at line 26, The exported component function SkillQuickInstall lacks an explicit return type; update its signature to include a TypeScript return annotation (e.g., : JSX.Element or : React.ReactElement) on the SkillQuickInstall function declaration so it conforms to the rule that all exported functions must have explicit return types; adjust any imports if you choose React.ReactElement (ensure React is imported) and keep the component implementation unchanged.
🤖 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/desktop/src/renderer/components/skill/SkillQuickInstall.tsx`:
- Around line 60-82: The exported function SkillQuickInstall is missing an
explicit return type annotation; update its function signature (the exported
SkillQuickInstall declaration) to include a React return type such as
React.ReactElement or JSX.Element (import React if needed) so the export has a
clear type; you don't need to change the implementation that handles
result.failures or the platform?.name ?? failure.platformId expression—only add
the explicit return type to the SkillQuickInstall function signature.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/skill/SkillQuickInstall.tsx`:
- Line 26: The exported component function SkillQuickInstall lacks an explicit
return type; update its signature to include a TypeScript return annotation
(e.g., : JSX.Element or : React.ReactElement) on the SkillQuickInstall function
declaration so it conforms to the rule that all exported functions must have
explicit return types; adjust any imports if you choose React.ReactElement
(ensure React is imported) and keep the component implementation unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 016495aa-f9b6-4855-82c5-362e0b0329e0
📒 Files selected for processing (12)
apps/desktop/src/main/services/skill-installer-platform.tsapps/desktop/src/renderer/components/skill/SkillFullDetailPage.tsxapps/desktop/src/renderer/components/skill/SkillQuickInstall.tsxapps/desktop/src/renderer/components/skill/use-skill-platform.tsapps/desktop/src/renderer/i18n/locales/de.jsonapps/desktop/src/renderer/i18n/locales/en.jsonapps/desktop/src/renderer/i18n/locales/es.jsonapps/desktop/src/renderer/i18n/locales/fr.jsonapps/desktop/src/renderer/i18n/locales/ja.jsonapps/desktop/src/renderer/i18n/locales/zh-TW.jsonapps/desktop/src/renderer/i18n/locales/zh.jsonapps/desktop/tests/unit/main/skill-installer-platform.test.ts
✅ Files skipped from review due to trivial changes (6)
- apps/desktop/src/renderer/i18n/locales/en.json
- apps/desktop/src/renderer/i18n/locales/es.json
- apps/desktop/src/renderer/i18n/locales/zh.json
- apps/desktop/src/renderer/i18n/locales/zh-TW.json
- apps/desktop/src/renderer/i18n/locales/ja.json
- apps/desktop/tests/unit/main/skill-installer-platform.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/desktop/src/renderer/i18n/locales/fr.json
- apps/desktop/src/renderer/i18n/locales/de.json
- apps/desktop/src/renderer/components/skill/SkillFullDetailPage.tsx
- apps/desktop/src/main/services/skill-installer-platform.ts
- apps/desktop/src/renderer/components/skill/use-skill-platform.ts
Comment on lines
+60
to
+82
| if (result.failures.length > 0) { | ||
| const details = result.failures | ||
| .map((failure) => { | ||
| const platform = availablePlatforms.find( | ||
| (entry) => entry.id === failure.platformId, | ||
| ); | ||
| const label = platform?.name ?? failure.platformId; | ||
| return t("skill.installFailureRow", { | ||
| platform: label, | ||
| reason: failure.reason, | ||
| defaultValue: "{{platform}}: {{reason}}", | ||
| }); | ||
| }) | ||
| .join("\n"); | ||
| showToast( | ||
| t("skill.installPartialFailure", { | ||
| details, | ||
| defaultValue: | ||
| "Some platforms could not be installed\n{{details}}", | ||
| }), | ||
| "error", | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 描述: 验证 batchInstall 的返回类型定义，确认 failures 字段的契约
# 查找 batchInstall 函数的类型定义或实现
rg -nP --type=ts -A 10 'function\s+batchInstall|const\s+batchInstall.*=' apps/desktop/src/renderer/components/skill/use-skill-platform.ts
# 查找相关的类型定义（BatchInstallResult 或类似）
rg -nP --type=ts -A 5 'interface.*BatchInstall.*Result|type.*BatchInstall.*Result' apps/desktop/src/renderer/components/skill/Repository: legeling/PromptHub
Length of output: 1112
🏁 Script executed:
# Get the complete BatchInstallResult interface definition
rg -nP --type=ts -A 15 'export interface BatchInstallResult' apps/desktop/src/renderer/components/skill/use-skill-platform.tsRepository: legeling/PromptHub
Length of output: 661
🏁 Script executed:
# Check all exported functions in SkillQuickInstall.tsx for return type annotations
rg -nP --type=tsx 'export\s+(async\s+)?function|export\s+const.*=.*\(' apps/desktop/src/renderer/components/skill/SkillQuickInstall.tsx | head -20Repository: legeling/PromptHub
Length of output: 90
🏁 Script executed:
# Get the full context of lines 60-82 in SkillQuickInstall.tsx to see function signature
sed -n '1,90p' apps/desktop/src/renderer/components/skill/SkillQuickInstall.tsx | head -90Repository: legeling/PromptHub
Length of output: 2973
🏁 Script executed:
# Get the complete SkillQuickInstall function signature with return type if present
sed -n '23,40p' apps/desktop/src/renderer/components/skill/SkillQuickInstall.tsxRepository: legeling/PromptHub
Length of output: 578
🏁 Script executed:
# Also check for any other exported functions in this file
rg -nP --type=ts 'export\s+(async\s+)?function' apps/desktop/src/renderer/components/skill/SkillQuickInstall.tsxRepository: legeling/PromptHub
Length of output: 145
为导出函数添加显式返回类型注解
第 26 行的导出函数 SkillQuickInstall 缺少显式返回类型注解。根据编码规范，所有导出函数必须有明确的返回类型声明。应为该函数添加返回类型注解（如 React.ReactNode 或 JSX.Element）。
关于 result.failures 的处理：契约已保证 BatchInstallResult.failures 始终为数组类型，代码安全地假设其存在。第 73 行使用的可选链操作符与空值合并操作符 (platform?.name ?? failure.platformId) 正确地显式处理了 null/undefined 情况。
🤖 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/desktop/src/renderer/components/skill/SkillQuickInstall.tsx` around
lines 60 - 82, The exported function SkillQuickInstall is missing an explicit
return type annotation; update its function signature (the exported
SkillQuickInstall declaration) to include a React return type such as
React.ReactElement or JSX.Element (import React if needed) so the export has a
clear type; you don't need to change the implementation that handles
result.failures or the platform?.name ?? failure.platformId expression—only add
the explicit return type to the SkillQuickInstall function signature.
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
Fixes #93. A user reported that installing a skill via Symlink mode silently produced no install. Investigation found two layered problems:
Silent failures in the batch installer. `batchInstall` in `use-skill-platform.ts` caught individual per-platform install errors, logged them to the console, and then discarded them. The caller only ever saw `{ successCount, totalCount }`, so the green "Installation successful" toast still showed even when a platform had failed. The user had no way to know what went wrong.
Windows symlink errors can be `UNKNOWN`, not `EPERM`. On Windows without Developer Mode / admin, `fs.symlink` can fail with code `UNKNOWN`. The existing fallback-to-copy path only handled `EPERM` / `EACCES` / `ENOTSUP`, so `UNKNOWN` escaped the fallback, propagated as an unexplained throw, and was then silently discarded by problem (1).
Changes
Verification
Notes
Summary by CodeRabbit
新功能
错误修复
国际化