Greptile Summary

This PR refactors the onboard-skills credential setup flow: skills without a primaryEnv are now filtered up front into missingCredentialSkills, a summary note ("Skill credentials") is shown before any prompts, and a helper formatCredentialSetupMessage is extracted. A new test covers the keyless-vs-keyed separation.

• The PR description and root-cause analysis describe a completely different change — a Windows ACL principal-classification fix involving src/security/windows-acl.test.ts — but the actual diff only touches src/commands/onboard-skills.ts and its test. The described security fix does not appear in this PR; please verify whether the description was copy-pasted from another PR and whether the Windows ACL bug is still unaddressed.

Confidence Score: 3/5

The onboarding code change itself is correct, but the PR description claims a Windows ACL security fix that is not present in the diff — needs verification before merging.

A P1 concern exists: the PR description describes a security fix (Windows ACL world-equivalent SID classification) that is absent from the code. If the described fix was intentional and security-relevant, merging this PR as-is would leave that bug unaddressed while falsely marking it resolved. The onboarding code change itself looks correct and well-tested.

No file-level code issues; the concern is the gap between the stated Windows ACL fix and the actual diff.

This is a comment left during a code review.
Path: src/commands/onboard-skills.test.ts
Line: 190-291

Comment:
**PR description does not match the implemented change**

The PR description and root-cause analysis describe a Windows ACL principal-classification fix (expanding world-equivalent SIDs, files `src/security/windows-acl.test.ts` / `src/security/audit-filesystem-windows.test.ts`), but the actual diff only touches `src/commands/onboard-skills.ts` and its test. The described security fix — misclassifying `Anonymous Logon`, `S-1-5-7`, etc. — does not appear to be implemented in this PR. Please verify whether the description was copy-pasted from a different PR and whether the intended Windows ACL fix was accidentally omitted.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/commands/onboard-skills.ts
Line: 50-52

Comment:
**Trivial wrapper used exactly once**

`formatCredentialSetupMessage` is a single-use, one-line string template that adds indirection without encapsulating any meaningful logic. The same template was already inline before this PR, and the helper is only called in one place. Consider inlining it to keep the code simpler.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(onboard): clarify keyed vs keyless s..." | Re-trigger Greptile}

Codex review: needs changes before merge.

Summary
The PR adds a Skill credentials note in setupSkills, filters credential prompts to missing skills with primaryEnv, and adds a mixed keyless/keyed onboarding regression test.

Reproducibility: yes. Current main c136bb0e still moves from dependency installation directly into per-skill env prompts, and the skill metadata confirms the reported local Whisper versus API-backed Whisper/ElevenLabs split.

Next step before merge
A narrow automated repair can add the required changelog bullet to this PR branch; no product or security decision is blocking that fix.

Security
Cleared: The net diff only changes onboarding prompt framing and a unit test, with no workflow, dependency, install script, permission, or secret-storage semantic changes.

Review findings

• [P3] Add the onboarding fix to the changelog — src/commands/onboard-skills.ts:205

Codex review: needs maintainer review before merge.

What this changes:

This PR adds a Skill credentials note, filters onboarding credential prompts to missing skills with primaryEnv and missing env requirements, and adds a regression test for mixed local Whisper plus API-backed Whisper/ElevenLabs skills.

Maintainer follow-up before merge:

This is an open implementation PR with a plausible narrow fix, but the remaining decision is maintainer review and reconciliation with overlapping PR #74417, not an autonomous repair lane.

Best possible solution:

Pick one narrow implementation for #74382, keep the credential explanation in setupSkills, retain regression coverage for mixed keyless and API-backed skills, add any required changelog entry, and merge only after the targeted onboarding tests and changed checks are green.

Acceptance criteria:

• pnpm test src/commands/onboard-skills.test.ts
• pnpm test src/agents/skills.buildworkspaceskillstatus.test.ts
• pnpm exec oxfmt --check --threads=1 src/commands/onboard-skills.ts src/commands/onboard-skills.test.ts CHANGELOG.md
• pnpm check:changed

What I checked:

• Current main onboarding flow: setupSkills still prompts for credentials by looping over missing and checking primaryEnv inline; there is no credential-summary note or precomputed credential-only list on current main. (src/commands/onboard-skills.ts:201, 2d53b49b20e1)
• Current main test gap: The current setupSkills tests end with Homebrew selection coverage and do not include a mixed keyless local skill plus credential-gated skill scenario. (src/commands/onboard-skills.test.ts:136, 2d53b49b20e1)
• Skill metadata confirms the UX distinction: openai-whisper requires only the whisper binary, while openai-whisper-api declares OPENAI_API_KEY and sag declares ELEVENLABS_API_KEY as primary env requirements. (skills/openai-whisper/SKILL.md:2, 2d53b49b20e1)
• PR diff surface: The current PR head net diff modifies only src/commands/onboard-skills.ts and src/commands/onboard-skills.test.ts, adding the note/filter and the regression test described in the PR body. (src/commands/onboard-skills.ts:198, f61564e8df53)
• Related overlapping PR: Open PR fix(onboard): clarify skill credential prompts #74417 targets the same [Bug]: Asks for OpenAI Whisper/ElevenLabs API key even though initial list says KEY not needed #74382 onboarding UX bug and also adds a changelog entry, so maintainers should choose one implementation path instead of landing both independently. (CHANGELOG.md:21, dd9e67584ad0)
• Security review pass: The current net diff does not touch workflows, dependencies, lockfiles, publish/install scripts, permissions, or secret handling. Two intermediate Discord API export commits were added and then reverted, leaving no net Discord diff at the PR head. (f61564e8df53)

Likely related people:

• Vincent Koc: Local blame on the credential prompt loop and the relevant bundled skill descriptors points to ad2516b1, which added the current setupSkills flow, tests, and Whisper/SAG skill metadata in this checkout. (role: recent maintainer / current-main importer; confidence: medium; commits: ad2516b1c876; files: src/commands/onboard-skills.ts, src/commands/onboard-skills.test.ts, skills/openai-whisper/SKILL.md)
• steipete: The v2026.4.26 release-tag commit in local history also carries the same onboarding and skill descriptor additions, making this a relevant maintainer routing signal for shipped behavior. (role: release/current-main maintainer; confidence: medium; commits: be8c24633aaa; files: src/commands/onboard-skills.ts, src/commands/onboard-skills.test.ts, skills/openai-whisper/SKILL.md)
• Ayaan Zaidi: The existing ClawSweeper context for the linked onboarding issue attributes earlier history for the setupSkills flow and bundled skill descriptors to 49f94db5; the local checkout history is compacted, so this is useful secondhand routing evidence. (role: introduced behavior in prior issue history; confidence: low; commits: 49f94db5d46b; files: src/commands/onboard-skills.ts, skills/openai-whisper/SKILL.md, skills/openai-whisper-api/SKILL.md)

Remaining risk / open question:

• This PR and open PR fix(onboard): clarify skill credential prompts #74417 solve the same [Bug]: Asks for OpenAI Whisper/ElevenLabs API key even though initial list says KEY not needed #74382 onboarding bug, so landing both without reconciliation would duplicate user-facing copy and tests.
• This PR does not add the changelog entry that fix(onboard): clarify skill credential prompts #74417 includes; if this branch is chosen, maintainers should decide whether the user-facing onboarding fix needs a changelog bullet.
• The current head still had at least one queued CI check at inspection time, and no local tests were run because this was a read-only review.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2d53b49b20e1.

Codex审核：合并前需要维护者审核。

这将带来哪些变化：

此 PR 添加了一条Skill credentials注释，将入职凭证提示筛选为缺少技能primaryEnv和缺少环境要求，并为混合本地 Whisper 和 API 支持的 Whisper/ElevenLabs 技能添加了回归测试。

合并前维护者跟进：

这是一个开放实现 PR，其中包含一个合理的小范围修复方案，但剩余的决定是维护者审查并与重叠的 PR #74417进行协调，而不是一个独立的修复通道。

最佳解决方案：

为#74382选择一个狭窄的实现，保留凭证说明setupSkills，保留混合无密钥和 API 支持技能的回归覆盖率，添加任何所需的变更日志条目，并且只有在目标入职测试和变更检查都通过后才合并。

验收标准：

• pnpm 测试 src/commands/onboard-skills.test.ts
• pnpm 测试 src/agents/skills.buildworkspaceskillstatus.test.ts
• pnpm exec oxfmt --check --threads=1 src/commands/onboard-skills.ts src/commands/onboard-skills.test.ts CHANGELOG.md
• pnpm 检查：已更改

我检查了以下内容：

• 当前主要注册流程： 仍然通过循环检查并内联的setupSkills方式提示输入凭据；当前主要流程中没有凭据摘要说明或预先计算的仅供凭据使用的列表。（，2d53b49b20e1）missing``primaryEnvsrc/commands/onboard-skills.ts:201
• **当前主要测试缺口：**目前的setupSkills测试仅涵盖自制软件的选择，并未包含混合的无密钥本地技能和凭证控制技能场景。（src/commands/onboard-skills.test.ts:136，2d53b49b20e1）
• 技能元数据证实了用户体验的区别： openai-whisper仅需要whisper二进制文件，而openai-whisper-api声明OPENAI_API_KEY并sag声明ELEVENLABS_API_KEY为主要环境要求。（skills/openai-whisper/SKILL.md:2，2d53b49b20e1）
• **PR 差异表面：**当前 PR 头部网络差异仅修改src/commands/onboard-skills.ts和src/commands/onboard-skills.test.ts，并添加了 PR 主体中描述的注释/过滤器和回归测试。（src/commands/onboard-skills.ts:198，f61564e8df53）
• 相关重叠的 PR： Open PR fix(onboard): 澄清技能凭证提示 #74417针对的是同一个[Bug]：即使初始列表显示不需要密钥，仍会要求输入 OpenAI Whisper/ElevenLabs API 密钥 #74382，这是一个引导用户体验错误，并且还添加了一个变更日志条目，因此维护者应该选择一种实现方式，而不是分别提交两种方式。( , dd9e67584ad0 )CHANGELOG.md:21
• **安全审查通过：**当前网络差异不涉及工作流、依赖项、锁定文件、发布/安装脚本、权限或密钥处理。添加了两个中间 Discord API 导出提交，然后又撤销了这些提交，因此 PR 头部没有留下任何 Discord 网络差异。( f61564e8df53 )

可能有关联的人：

• **Vincent Koc：**本地问题归咎于凭证提示循环和相关的捆绑技能描述符ad2516b1，该问题指向[此处应填写提交记录]，该提交记录在此检出版本中添加了当前setupSkills流程、测试和Whisper/SAG技能元数据。（角色：近期维护者/当前主要导入者；置信度：中等；提交记录：ad2516b1c876；文件：src/commands/onboard-skills.ts[此处应填写文件列表src/commands/onboard-skills.test.ts] skills/openai-whisper/SKILL.md）
• **steipete：**本地历史记录中的 v2026.4.26 发布标签提交也包含了相同的入门和技能描述符添加，这使其成为已发布行为的相关维护者路由信号。（角色：发布/当前主要维护者；置信度：中等；提交：be8c24633aaa ；文件：src/commands/onboard-skills.ts，，src/commands/onboard-skills.test.ts）skills/openai-whisper/SKILL.md
• **Ayaan Zaidi：**现有 ClawSweeper 上下文中关联的入职问题属性包含setupSkills流程的早期历史记录和捆绑的技能描述符；本地检出历史记录已压缩，因此这是有用的二手路由证据。（角色：在先前问题历史记录中引入的行为49f94db5；置信度：低；提交：49f94db5d46b；文件：src/commands/onboard-skills.ts，，skills/openai-whisper/SKILL.md）skills/openai-whisper-api/SKILL.md

剩余风险/未决问题：

• 此 PR 和未解决的 PR修复（入职）：澄清技能凭证提示 #74417解决了相同的[Bug]：即使初始列表显示不需要密钥，仍会要求提供 OpenAI Whisper/ElevenLabs API 密钥 #74382入职错误，因此在没有协调的情况下同时提交这两个 PR 会导致面向用户的文本和测试重复。
• 此 PR 不包含fix(onboard): 澄清技能凭证提示 #74417所包含的变更日志条目；如果选择此分支，维护者应决定面向用户的入职修复是否需要变更日志项目。
• 当前负责人检查时至少还有一个待处理的 CI 检查，并且由于这是一次只读审查，因此没有运行本地测试。

Codex 审查说明：模型 gpt-5.5，推理能力高；与2d53b49b20e1进行审查。

Thanks for the review and for calling out the overlap with #74417.

I’ve already addressed the actionable feedback on this PR:

• Updated the PR description/body to match the actual onboarding scope (issue [Bug]: Asks for OpenAI Whisper/ElevenLabs API key even though initial list says KEY not needed #74382), not the Windows ACL fix.
• Inlined the single-use formatCredentialSetupMessage helper.
• Fixed follow-up CI issues on this branch and rebased to current openclaw/main.

On overlap with #74417:

• Both PRs target the same UX bug in [Bug]: Asks for OpenAI Whisper/ElevenLabs API key even though initial list says KEY not needed #74382 (keyless install hints vs API-key prompts).
• This PR keeps the fix narrow to setupSkills prompt framing + regression coverage for the mixed openai-whisper / openai-whisper-api / sag path.
• If maintainers prefer fix(onboard): clarify skill credential prompts #74417 as the canonical PR, I’m happy to close this one.
  If maintainers prefer this branch, fix(onboard): clarify skill credential prompts #74417 can be closed as duplicate.

I’m good with either direction—please pick whichever is easier to land.