feat: complete WorkOS skill install + refresh loop with doctor --fix#130
Merged
Conversation
autoInstallSkills now returns a summary describing which skills were installed to which agents. handleInstall prints a clack info line in TTY mode so users know their coding agent has up-to-date WorkOS guidance. Suppressed in JSON mode. Also writes a per-agent .workos-skill-version marker alongside installed skills so downstream checks can detect staleness.
Compare each detected coding agent's .workos-skill-version marker against the bundled @workos/skills version. Emit a SKILLS_OUTDATED warning with a remediation pointing users at `workos skills install`. Returns null when no agent has WorkOS skills installed, so doctor stays quiet for users who never installed through the CLI.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughInstaller and doctor flows now detect, atomically install, and manage bundled WorkOS skills. Installs use temp→backup→rename with rollback and orphan cleanup; auto-install returns structured results or null. Doctor can inspect marker staleness and optionally refresh allowlisted skills, reporting before/after marker states. Changes
🚥 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)
Comment |
Greptile SummaryThis PR completes the WorkOS skill install/refresh loop: recursive prune-replace install (
Confidence Score: 4/5Safe to merge after fixing the output.ts agent-filtering bug; all other logic is well-tested and architecturally sound. One confirmed P1 in output.ts (success checkmark printed for failed-install agents) and one P2 in install-skill.ts (orphaned backup dir not cleaned in the outer catch on double-rename failure). No security concerns. Test coverage is thorough across all new code paths. src/doctor/output.ts (P1 agent filtering), src/commands/install-skill.ts (P2 backupDir cleanup in catch) Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI
participant installSkill
participant refreshWorkOSSkills
participant checkSkills
participant maybeRefreshSkills
Note over CLI,maybeRefreshSkills: workos auth login / workos install
CLI->>refreshWorkOSSkills: autoInstallSkills()
refreshWorkOSSkills->>refreshWorkOSSkills: detectAgents() + discoverSkills()
loop each agent x skill
refreshWorkOSSkills->>installSkill: installSkill(src, skill, agent)
installSkill->>installSkill: mkdtemp → cp → rename(target→bak) → rename(tmp→target)
installSkill-->>refreshWorkOSSkills: {success, error}
end
refreshWorkOSSkills->>refreshWorkOSSkills: writeAgentSkillMarker (per succeeded agent)
refreshWorkOSSkills-->>CLI: RefreshResult (perAgentBefore/After)
Note over CLI,maybeRefreshSkills: workos doctor --fix
CLI->>checkSkills: checkSkills()
checkSkills-->>CLI: SkillsInfo (stale agents)
CLI->>maybeRefreshSkills: maybeRefreshSkills({fix:true}, skills)
maybeRefreshSkills->>refreshWorkOSSkills: refreshWorkOSSkills({skills: FIXABLE_SKILLS})
refreshWorkOSSkills-->>maybeRefreshSkills: RefreshResult
maybeRefreshSkills->>checkSkills: checkSkills() re-read post-refresh
checkSkills-->>maybeRefreshSkills: updated SkillsInfo
maybeRefreshSkills-->>CLI: {skillsRefresh, skills}
CLI->>CLI: formatReport() → render before→after per agent
Reviews (7): Last reviewed commit: "docs(readme): mention auth-login skill h..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/install-skill.ts`:
- Around line 206-230: The code returns the full discovered skills list and
writes the SKILL_VERSION_MARKER_FILENAME for any agent with at least one
successful install, which can over-report installs; change the install loop in
the block using targetAgents/installSkill so that for each agent you collect the
subset of successfully installed skill names (e.g., per-agent installedSkills
array), only add the agent to succeededAgents when installedSkills.length > 0,
include those installed skill names in the final AutoInstallResult.skills (or
build a map/unique list of actually installed skills instead of using the
original skills array), and only write the .workos-skill-version marker for an
agent when installedSkills.length === skills.length (i.e., all requested skills
succeeded) to avoid marking partially-updated agents as up-to-date.
- Around line 17-23: The getBundledSkillsVersion function currently uses
synchronous file I/O (readFileSync); change it to an async function (export
async function getBundledSkillsVersion(...): Promise<string | null>) and replace
readFileSync/JSON.parse with await fs.promises.readFile (or import { promises as
fs } and use fs.readFile) and JSON.parse inside the try/catch, returning the
version or null on error; also update the caller autoInstallSkills to await the
new async getBundledSkillsVersion (e.g., const version = await
getBundledSkillsVersion(skillsDir)) and adjust any surrounding code to handle
the returned Promise.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 6060ef64-104b-41b1-8bb8-b9aee2235055
📒 Files selected for processing (9)
src/commands/install-skill.spec.tssrc/commands/install-skill.tssrc/commands/install.spec.tssrc/commands/install.tssrc/doctor/checks/skills.spec.tssrc/doctor/checks/skills.tssrc/doctor/index.tssrc/doctor/issues.tssrc/doctor/types.ts
Phase 2 of cus-feedback-fixes. Closes the gap where `workos install`
shipped only `SKILL.md` and not the `references/` tree the WorkOS
skill router instructs the agent to Read. Without those reference
files, the auto-installed skill was functionally empty.
* Rewrite `installSkill` from a single `copyFile(SKILL.md)` to a
recursive prune-replace of the whole `<skillsDir>/<skillName>/` tree
using `mkdtemp` + `cp { recursive: true }` + sibling `.workos.bak-`
backup-rename. Rolls back to the original target if the temp→target
rename fails mid-flight; tempDir is cleaned up on copy failure.
Backup cleanup is best-effort post-success — the install does not
fail if the backup remains, and `cleanupStaleOrphans` will reap it
on the next run after 1h.
* Add `cleanupStaleOrphans` with a 1-hour mtime cutoff (constant
`ORPHAN_STALE_MS`). Concurrent runs that leave fresh `.tmp-*` /
`.bak-*` siblings are never touched — the cutoff guarantees we
only nuke orphans from runs that crashed long ago.
* Extract `refreshWorkOSSkills(opts)` as a reusable primitive returning
`RefreshResult` with `perAgentBefore` / `perAgentAfter` keyed by
`agent.name`. Both `autoInstallSkills` (best-effort hook) and
doctor `--fix` (Phase 3) will call this — no duplicate copy logic.
`autoInstallSkills` becomes a thin back-compat wrapper that returns
the existing `AutoInstallResult` shape (`install.ts` consumers stay
unchanged).
* Add `installSkillsAfterLogin` helper in `login.ts`, wired in after
`provisionStagingEnvironment`. Wraps `autoInstallSkills` in its own
try/catch so a skill install failure NEVER fails login itself.
Skips logging in JSON mode, mirrors install.ts's success copy.
Extracted as a separate exported function so it's unit-testable
without standing up the device-auth polling loop.
* Tests:
- `installSkill` copies `references/` subdirectory
- planted stale `references/workos-stale.md` is gone after re-install
- simulated rename failure restores original target from backup
- copy failure cleans up tempDir (no leftover `.workos.tmp-*`)
- `cleanupStaleOrphans` removes >1h orphans of both prefixes,
preserves <1h orphans of both prefixes
- peer skill directories (different prefix) are never touched
- `refreshWorkOSSkills` reports per-agent before/after marker state,
respects `writeMarker: false`, filters by `skills` and `agents`
- `installSkillsAfterLogin` calls `autoInstallSkills`, returns
successfully when installer throws, JSON-mode-aware, singular
vs plural copy
NOT INCLUDED in this commit (intentional — see open items):
- `package.json` bump from `@workos/skills@0.2.4` to `0.4.0`. Phase 1
(the skills-repo release providing the new content) is committed
but not yet published to npm. The bump becomes a one-line follow-up
once `0.4.0` is on the registry.
Phase 3 of cus-feedback-fixes. `workos doctor` (no flag) keeps its
warn-only behavior — `--fix` is the explicit opt-in for mutating
state from a diagnostic command. When the user runs `workos doctor
--fix` and at least one agent reports a stale or missing version
marker, refresh is scoped to a hardcoded allowlist of bundled WorkOS
skills (`workos`, `workos-widgets`) and a per-agent before/after
summary is rendered.
* `FIXABLE_SKILLS` is hardcoded `['workos', 'workos-widgets']` —
NOT derived from `discoverSkills()`. A future bundled skill needs
an explicit opt-in here before doctor `--fix` will write to its
target directory. This is the contract's promise that `--fix`
only ever touches `workos/` and `workos-widgets/`.
* `maybeRefreshSkills(options, skills)` extracted as a testable
helper. Calls Phase 2's `refreshWorkOSSkills` with the allowlist
via the `skills` filter, then re-runs `checkSkills()` so issue
detection sees the post-refresh marker state. Without the re-run,
`detectIssues` would still report `SKILLS_OUTDATED` immediately
after fixing it.
* Renderer in `output.ts` adds a "Skills" section that prints one
line per agent: `✓ Updated WorkOS skills for {DisplayName}: {before}
→ {after}` (treating `null` as `(none)`). JSON mode picks up
`skillsRefresh` automatically via `JSON.stringify(report)`.
* Wired through `src/bin.ts` (yargs `--fix` boolean), `src/commands/
doctor.ts` (DoctorArgs pass-through), and `src/doctor/types.ts`
(`fix?: boolean` on DoctorOptions; `SkillsRefreshResult` interface
with before/after keyed by agent.name; `skillsRefresh?` on
DoctorReport).
* Tests in `src/doctor/checks/skills-fix.spec.ts`:
- `--fix=false` does NOT call refresh even with stale skills
- `--fix=true` is no-op when no skills info / when nothing is stale
- allowlist passed to refreshWorkOSSkills exactly matches FIXABLE_SKILLS
- success path returns skillsRefresh + post-refresh re-read of skills
- refresh-returns-null preserves original skills (no skillsRefresh)
- integration test against real refreshWorkOSSkills: planted
third-party-skill and hypothetical workos-future-skill files
at the agent target are byte-identical after `--fix` runs
Open Item from spec deferred: whether `--fix` should also fix
non-skill issues in the future. Out of scope here.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/doctor/index.ts (1)
81-125:⚠️ Potential issue | 🟠 Major
--fixstill feeds stale skills into AI analysis.
earlyIssuesandcheckAiAnalysis()run beforemaybeRefreshSkills(), so a successful refresh can clearreport.issueswhilereport.aiAnalysisstill contains pre-fix stale-skill findings or remediation. Move the refresh ahead ofearlyIssues, or recompute the AI input after the refresh.🔁 Reorder the refresh so AI sees post-fix state
let skills = checkSkills() ?? undefined; + const refreshOutcome = await maybeRefreshSkills(options, skills); + const skillsRefresh = refreshOutcome.skillsRefresh; + skills = refreshOutcome.skills; // Dashboard settings + auth patterns + AI analysis (parallel, all need sdk/framework results) // AI analysis also receives early issues as context to avoid duplication const earlyIssues = detectIssues({ @@ - // `--fix`: refresh stale WorkOS skills BEFORE issue detection so the report - // reflects the post-refresh state (no lingering SKILLS_OUTDATED warning - // after the refresh has already fixed it). - const refreshOutcome = await maybeRefreshSkills(options, skills); - const skillsRefresh = refreshOutcome.skillsRefresh; - skills = refreshOutcome.skills; - // Build partial report🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/doctor/index.ts` around lines 81 - 125, Move the skills refresh so AI receives post-fix state: call maybeRefreshSkills(...) before computing earlyIssues and invoking checkAiAnalysis(...), or after refreshing update the skills variable and recompute earlyIssues and the payload passed to checkAiAnalysis; specifically adjust the sequence around skills, maybeRefreshSkills, detectIssues (earlyIssues) and checkAiAnalysis so checkAiAnalysis receives the refreshed skills (references: maybeRefreshSkills, detectIssues, checkAiAnalysis, skills, skillsRefresh).
♻️ Duplicate comments (1)
src/commands/install-skill.ts (1)
305-336:⚠️ Potential issue | 🟠 MajorDon't mark partially refreshed agents as current.
agentSucceededflips totrueafter the first successful copy, so a single failed skill still writes the version marker and returns the full requestedskillsset. That makes doctor treat partially updated agents as fresh and overstates what was installed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/install-skill.ts` around lines 305 - 336, The loop currently sets agentSucceeded true after the first successful install so partially-updated agents get marked current and the full requested skills list is returned; change the logic in the block that iterates detected agents to track per-agent per-skill success (call installSkill for each skill, record each result), set agentSucceeded to true only if every installSkill returned result.success (or equivalently if there are no failures), write the SKILL_VERSION_MARKER_FILENAME via writeFile only when agentSucceeded is true, and adjust the returned payload so it does not advertise the full requested skills set for an agent that had partial failures (use the actual successful installs rather than the original skills array); reference functions/variables installSkill, agentSucceeded, succeededAgents, perAgentBefore, perAgentAfter, writeFile, and SKILL_VERSION_MARKER_FILENAME when updating the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/install-skill.spec.ts`:
- Around line 389-413: The test never forces a resolvable bundled version, so it
can pass without exercising the marker-write success path; modify the spec to
make the bundled version deterministic (e.g., create a package.json with a
"version" in the temp skillsDir/artifact or stub getBundledSkillsVersion) before
calling autoInstallSkills(), then assert that SKILL_VERSION_MARKER_FILENAME in
join(homeDir, '.claude/skills') exists and its contents equal result.version;
target symbols: SKILL_VERSION_MARKER_FILENAME, autoInstallSkills, skillsDir,
homeDir, and the test block that builds skill-a so the marker-write branch is
provably executed.
In `@src/commands/install-skill.ts`:
- Around line 290-337: The explicit install flow should reuse
refreshWorkOSSkills so markers get written; change runInstallSkill (and the
other direct-install path that currently loops 339-349) to call
refreshWorkOSSkills with the requested skills and detected agents (e.g.
refreshWorkOSSkills({ skills: requestedSkills, agents: detectedAgents,
writeMarker: true })) instead of performing the old per-skill install loop using
installSkill directly, ensuring the returned RefreshResult is propagated so
.workos-skill-version markers are created consistently.
- Line 127: Several synchronous fs calls violate the async-only guideline:
replace existsSync(targetDir) in installSkill with await
stat(targetDir).catch(() => null) and use that truthiness for targetExisted;
similarly change existsSync(parent) in cleanupStaleOrphans to await
stat(parent).catch(() => null); convert readSkillVersionMarker into an async
function that uses readFile from fs/promises (instead of readFileSync) and
update its callers in refreshWorkOSSkills to await the new async function; and
in discoverSkills replace existsSync(join(skillsDir, e.name, 'SKILL.md')) with
an await stat(...).catch(() => null) check. Ensure to import stat/readFile from
'fs/promises' and preserve existing error handling/returns when stat/readFile
return null.
In `@src/commands/login.ts`:
- Around line 86-95: The post-login hook installSkillsAfterLogin currently
awaits autoInstallSkills(), which can hang and block runLogin(); change this so
skill installation cannot stall login by either (a) running autoInstallSkills()
without awaiting its Promise (fire-and-forget) or (b) wrapping the await in a
bounded timeout (e.g., Promise.race with a timeout) so installSkillsAfterLogin
returns promptly; update references in installSkillsAfterLogin and any callers
so credentials/success UI are not dependent on autoInstallSkills() completing.
---
Outside diff comments:
In `@src/doctor/index.ts`:
- Around line 81-125: Move the skills refresh so AI receives post-fix state:
call maybeRefreshSkills(...) before computing earlyIssues and invoking
checkAiAnalysis(...), or after refreshing update the skills variable and
recompute earlyIssues and the payload passed to checkAiAnalysis; specifically
adjust the sequence around skills, maybeRefreshSkills, detectIssues
(earlyIssues) and checkAiAnalysis so checkAiAnalysis receives the refreshed
skills (references: maybeRefreshSkills, detectIssues, checkAiAnalysis, skills,
skillsRefresh).
---
Duplicate comments:
In `@src/commands/install-skill.ts`:
- Around line 305-336: The loop currently sets agentSucceeded true after the
first successful install so partially-updated agents get marked current and the
full requested skills list is returned; change the logic in the block that
iterates detected agents to track per-agent per-skill success (call installSkill
for each skill, record each result), set agentSucceeded to true only if every
installSkill returned result.success (or equivalently if there are no failures),
write the SKILL_VERSION_MARKER_FILENAME via writeFile only when agentSucceeded
is true, and adjust the returned payload so it does not advertise the full
requested skills set for an agent that had partial failures (use the actual
successful installs rather than the original skills array); reference
functions/variables installSkill, agentSucceeded, succeededAgents,
perAgentBefore, perAgentAfter, writeFile, and SKILL_VERSION_MARKER_FILENAME when
updating the code.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 5e013ffb-f61a-499f-9896-2aa129249ab7
📒 Files selected for processing (10)
src/bin.tssrc/commands/doctor.tssrc/commands/install-skill.spec.tssrc/commands/install-skill.tssrc/commands/login.spec.tssrc/commands/login.tssrc/doctor/checks/skills-fix.spec.tssrc/doctor/index.tssrc/doctor/output.tssrc/doctor/types.ts
Comment on lines
+290
to
+337
| export async function refreshWorkOSSkills(opts: RefreshOptions = {}): Promise<RefreshResult | null> { | ||
| const home = homedir(); | ||
| const skillsDir = getSkillsDir(); | ||
| const detected = opts.agents ?? detectAgents(createAgents(home)); | ||
| const allSkills = await discoverSkills(skillsDir).catch(() => []); | ||
| const skills = opts.skills ? allSkills.filter((s) => opts.skills!.includes(s)) : allSkills; | ||
| const writeMarker = opts.writeMarker ?? true; | ||
|
|
||
| if (skills.length === 0 || detected.length === 0) return null; | ||
|
|
||
| if (skills.length === 0 || targetAgents.length === 0) return; | ||
| const version = getBundledSkillsVersion(skillsDir); | ||
| const perAgentBefore: Record<string, string | null> = {}; | ||
| const perAgentAfter: Record<string, string | null> = {}; | ||
| const succeededAgents: AgentConfig[] = []; | ||
|
|
||
| for (const agent of detected) { | ||
| perAgentBefore[agent.name] = readSkillVersionMarker(agent); | ||
|
|
||
| let agentSucceeded = false; | ||
| for (const skill of skills) { | ||
| for (const agent of targetAgents) { | ||
| await installSkill(skillsDir, skill, agent); | ||
| const result = await installSkill(skillsDir, skill, agent); | ||
| if (result.success) agentSucceeded = true; | ||
| } | ||
|
|
||
| if (agentSucceeded) { | ||
| succeededAgents.push(agent); | ||
| if (writeMarker && version) { | ||
| try { | ||
| await writeFile(join(agent.globalSkillsDir, SKILL_VERSION_MARKER_FILENAME), version, 'utf8'); | ||
| } catch { | ||
| // Marker is best-effort; doctor treats missing marker as "unknown". | ||
| } | ||
| } | ||
| } | ||
|
|
||
| perAgentAfter[agent.name] = readSkillVersionMarker(agent); | ||
| } | ||
|
|
||
| if (succeededAgents.length === 0) return null; | ||
|
|
||
| return { | ||
| agents: succeededAgents, | ||
| skills, | ||
| version, | ||
| perAgentBefore, | ||
| perAgentAfter, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Route the explicit install command through the new refresh path.
refreshWorkOSSkills() is now the only place that writes .workos-skill-version, but runInstallSkill() still installs skills via its old loop. That means a user can follow the SKILLS_OUTDATED remediation with workos skills install and still keep an old or missing marker on the next doctor run.
Also applies to: 339-349
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/install-skill.ts` around lines 290 - 337, The explicit install
flow should reuse refreshWorkOSSkills so markers get written; change
runInstallSkill (and the other direct-install path that currently loops 339-349)
to call refreshWorkOSSkills with the requested skills and detected agents (e.g.
refreshWorkOSSkills({ skills: requestedSkills, agents: detectedAgents,
writeMarker: true })) instead of performing the old per-skill install loop using
installSkill directly, ensuring the returned RefreshResult is propagated so
.workos-skill-version markers are created consistently.
Phase 1 of cus-feedback-fixes is now live on npm. Pulling the new content in so the recursive prune-replace install machinery added in Phase 2 actually delivers the new guardrails: * references/workos-cli-upgrade.md (CLI upgrade-path topic) * SKILL.md routing rule for outdated-CLI symptoms * references/workos-management.md "Detecting CLI upgrades" subsection Exact pin matching the prior 0.2.4 convention.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 54: The package.json dependency pin for "@workos/skills": "0.4.0" is
invalid and will fail install; update that dependency entry to a released
version (for example "0.2.4") by replacing the current version string for the
"@workos/skills" key with the actual released version (e.g., change
"@workos/skills": "0.4.0" to "@workos/skills": "0.2.4") and run install to
verify.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 65e7409a-38f3-4391-a6df-1da880c09e79
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
package.json
| "@napi-rs/keyring": "^1.2.0", | ||
| "@workos-inc/node": "^8.7.0", | ||
| "@workos/skills": "0.2.4", | ||
| "@workos/skills": "0.4.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For npm package @workos/skills` version 0.4.0, confirm:
- it still exports getSkillsDir,
- its package layout still supports resolving package.json by walking up three directories from getSkillsDir() result,
- there were no breaking changes from 0.2.4 that affect CLI embedding/install workflows.
Please include links to changelog/release notes or package source.`
💡 Result:
No evidence found for npm package @workos/skills version 0.4.0 existing as of 2026-04-26. Latest documented version is 0.2.4 (https://github.com/workos/skills/releases/tag/v0.2.4), with later releases up to 0.3.0 mentioned in PRs but no 0.4.0. Repo README and docs explicitly list getSkillsDir as exported in current versions (https://github.com/workos/skills), described as returning absolute path to the skills/ directory containing skill folders like workos/. 1) Cannot confirm for non-existent v0.4.0; exported in latest available (0.2.4+). 2) Cannot confirm layout for v0.4.0. Repo structure shows skills/ under repo root (https://github.com/workos/skills/blob/main/CLAUDE.md); when installed via npm, node_modules/@workos/skills/package.json is at module root. Typical path resolution from skills/ dir (getSkillsDir) would be .. (to @workos/skills) -> ../../ (to project root) -> ../../.. (outside), but "three directories up" likely reaches project root package.json if skills/ is nested as plugins/workos/skills/ per CLAUDE.md. No breaking layout changes evident. 3) From 0.2.4 changelog: only bug fix for Next.js AuthKit skill (no CLI/embedding impact). Earlier releases (0.2.1-0.2.3) are minor bug fixes/package tweaks (https://github.com/workos/skills/releases). No breaking changes affecting CLI embedding/install workflows identified. Sources: GitHub repo (1,4,6), releases (2,47-49), libraries.io (3), PRs (50,51). Cannot verify v0.4.0 specifics due to lack of evidence.
Citations:
- 1: https://github.com/workos/skills
- 2: https://github.com/workos/skills/releases/tag/v0.2.4
- 3: https://libraries.io/npm/@workos%2Fskills
- 4: workos/skills@v0.2.3...v0.2.4
- 5: https://github.com/workos/skills/blob/main/CLAUDE.md
- 6: https://github.com/workos/skills/releases/tag/v0.2.3
- 7: https://github.com/workos/skills/releases/tag/v0.2.2
- 8: https://github.com/workos/skills/releases/tag/v0.2.1
- 9: chore(main): release 0.2.5 skills#18
- 10: chore(main): release 0.3.0 skills#22
@workos/skills@0.4.0 does not exist; update to latest available version (0.2.4).
The web search confirmed that @workos/skills version 0.4.0 has not been released. The latest documented version is 0.2.4 (https://github.com/workos/skills/releases/tag/v0.2.4). Versions 0.3.0 have been mentioned in PRs but lack official releases. This dependency will fail at install time. Update the pin to an actual released version such as 0.2.4 or 0.3.0 (if officially available in your npm registry).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 54, The package.json dependency pin for
"@workos/skills": "0.4.0" is invalid and will fail install; update that
dependency entry to a released version (for example "0.2.4") by replacing the
current version string for the "@workos/skills" key with the actual released
version (e.g., change "@workos/skills": "0.4.0" to "@workos/skills": "0.2.4")
and run install to verify.
nicknisi
changed the title
Review surface findings on PR #130. Each fix below addresses a specific reviewer comment. * `refreshWorkOSSkills` no longer over-reports installed skill count. Tracks the union of skills that succeeded for at least one agent and returns that as `RefreshResult.skills` instead of the full filtered attempt list. Fixes the "Installed N skills" line inflating when some skills failed to copy. (Greptile P1, CodeRabbit Major) * `runInstallSkill` (the explicit `workos skills install` command) now writes per-agent `.workos-skill-version` markers after success, so `workos doctor` doesn't immediately re-flag the freshly-installed skills as stale. Closes the loop where the SKILLS_OUTDATED remediation pointed at an install path that didn't update markers. (Codex P2, CodeRabbit Major) * `installSkill` setup operations (mkdir, mkdtemp, cleanupStaleOrphans) now run inside the try block. Filesystem errors before the copy (EACCES, ENOTDIR, etc.) surface as `{ success: false }` instead of rejecting — both runInstallSkill and refreshWorkOSSkills accumulate per-(skill, agent) failures, and a single bad agent dir would otherwise abort the whole batch (and now: abort the entire `doctor --fix` run). (Codex P2) * `checkSkills` only reports agents that actually have a WorkOS skill installed (marker file OR `workos/` subdir present), instead of any agent with a `skills/` directory. Without this, a Claude Code user who had unrelated skills but never installed WorkOS would have `installedVersion === null` reported, which `--fix` would then interpret as "missing marker, refresh" and write `workos/` + `workos-widgets/` to that agent unprompted. (Codex P2) * Staleness check uses semver ordering instead of string inequality. String comparison would flag `installed > bundled` (downgrade scenario: user installed via newer CLI then downgraded) as stale and the SKILLS_OUTDATED remediation would silently downgrade their agent's skills. Falls back to string inequality when either version is non-semver. (Greptile P2) * `installSkillsAfterLogin` wraps `autoInstallSkills` in a 30s Promise.race timeout. The hook can no longer block login completion on a hung filesystem call. (CodeRabbit Major; was an open item in the spec) * `maybeRefreshSkills` runs BEFORE `earlyIssues` and AI analysis in runDoctor, so AI prompt context (and downstream issue detection) see the post-refresh state. Without this move, a successful `--fix` could clear `report.issues` while `report.aiAnalysis` still references the just-fixed SKILLS_OUTDATED warning. (CodeRabbit Major, outside-diff finding) * `--fix` flag now appears in `workos doctor --help --json` via `src/utils/help-json.ts`. The CLI intercepts `--help --json` from this static registry; without the entry, automation/agent consumers (the same audience this whole feature targets) wouldn't discover `--fix` from machine-readable help. (Codex P3) * `install.spec.ts` mocks for `autoInstallSkills` now include the required `version` field, matching the AutoInstallResult type. (Greptile P2) * `install-skill.spec.ts` "writes a version marker per agent when the bundled version is resolvable" now plants a deterministic `<packageRoot>/package.json` + `<packageRoot>/plugins/workos/skills/` layout so `getBundledSkillsVersion` returns a known value (`9.9.9`) and the marker-write success path is genuinely exercised. (CodeRabbit Minor) Pushed back / dismissed: * CodeRabbit Critical "0.4.0 doesn't exist": false positive. The registry data the bot's web search saw was stale; `npm view @workos/skills@0.4.0 version` returns `0.4.0` and `pnpm install` resolves cleanly. * CodeRabbit Major "use async fs in getBundledSkillsVersion": the file's pre-existing convention is sync APIs for one-shot path probes (`existsSync`, `readFileSync` in checkSkills, install-skill). Converting one function would cascade through `checkSkills` → `runDoctor` for marginal benefit. Keeping consistent.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/install-skill.ts (1)
181-258:⚠️ Potential issue | 🟠 MajorAdd JSON-mode handling to
skills install.This handler still emits only human text and exits directly, so automation cannot get structured output from
workos skills install --json. Please branch on the global output mode here and add matching JSON-mode coverage insrc/commands/install-skill.spec.ts.As per coding guidelines, "Implement both human and JSON output modes in commands; check
OutputModeusage insrc/bin.ts" and "Write.spec.tstest files alongside every command file and include JSON mode tests".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/install-skill.ts` around lines 181 - 258, runInstallSkill currently only emits human-readable logs and calls process.exit, so automation can't consume structured output; update runInstallSkill to branch on the global OutputMode (see OutputMode usage in src/bin.ts) and emit a JSON object (including arrays of successes and failures with skill, agent.displayName, and error fields, plus overall status and bundled skills version) when mode === 'json' instead of human console output, ensuring exit code reflects failure when any installs fail; retain existing human output branch. Also add JSON-mode unit tests in src/commands/install-skill.spec.ts that invoke runInstallSkill (or the CLI entry) with OutputMode set to json and assert the emitted JSON structure for both all-success and partial-failure cases, and include a test that verifies version marker writing behavior tied to getBundledSkillsVersion, writeFile, and SKILL_VERSION_MARKER_FILENAME.
♻️ Duplicate comments (1)
src/commands/install-skill.ts (1)
233-246:⚠️ Potential issue | 🟠 MajorOnly write the version marker after a full per-agent success.
Both branches persist
.workos-skill-versionas soon as any requested skill lands for an agent. If one skill fails,checkSkills()will still treat that agent as current anddoctor --fixstops retrying the missing install.Possible fix
- const version = getBundledSkillsVersion(skillsDir); - if (version) { - const succeededAgents = new Set<AgentConfig>(); - for (const r of successful) succeededAgents.add(r.agent); - for (const agent of succeededAgents) { + const version = getBundledSkillsVersion(skillsDir); + if (version) { + const agentResults = new Map<AgentConfig, { successes: number; failures: number }>(); + for (const r of results) { + const current = agentResults.get(r.agent) ?? { successes: 0, failures: 0 }; + if (r.success) current.successes += 1; + else current.failures += 1; + agentResults.set(r.agent, current); + } + for (const [agent, summary] of agentResults) { + if (summary.failures > 0 || summary.successes === 0) continue; try { await writeFile(join(agent.globalSkillsDir, SKILL_VERSION_MARKER_FILENAME), version, 'utf8'); } catch { // Marker is best-effort; doctor treats missing marker as "unknown". } } }Also applies to: 339-347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/install-skill.ts` around lines 233 - 246, Only write the .workos-skill-version marker when every requested skill for that specific agent succeeded: build a per-agent set (or count) of requested installs and compare it to the per-agent successes in successful (instead of marking an agent when any single install succeeded via succeededAgents); only call writeFile(join(agent.globalSkillsDir, SKILL_VERSION_MARKER_FILENAME), version, 'utf8') for agents where successful entries fully cover that agent's requested skills so checkSkills() won't treat partially-installed agents as current.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/login.ts`:
- Around line 90-91: The timeout Promise created with setTimeout (using
SKILL_INSTALL_TIMEOUT_MS) leaves an active timer after
Promise.race([autoInstallSkills(), timeout]) resolves, preventing Node from
exiting; fix by storing the timer handle returned by setTimeout and either call
clearTimeout(timer) in a finally block after awaiting the race or call
timer.unref() immediately so the pending timer does not keep the event loop
alive; update the code around the timeout/Promise.race invocation (the timeout
variable, SKILL_INSTALL_TIMEOUT_MS, and autoInstallSkills()) accordingly.
In `@src/doctor/checks/skills.ts`:
- Around line 47-48: The check currently only treats the presence of workos/ or
the markerPath as a signal to skip processing, which misses agents that have the
older explicit install directory 'workos-widgets'; update the condition in the
loop that computes workosSkillDir (variable workosSkillDir, markerPath and
agent.globalSkillsDir) to also check for the existence of a workos-widgets
directory (e.g., join(agent.globalSkillsDir, 'workos-widgets')) and treat it the
same as workos when deciding to continue; in short, include
existsSync(join(agent.globalSkillsDir, 'workos-widgets')) as an alternative in
the OR check so agents with workos-widgets are recognized as pre-marker
installs.
---
Outside diff comments:
In `@src/commands/install-skill.ts`:
- Around line 181-258: runInstallSkill currently only emits human-readable logs
and calls process.exit, so automation can't consume structured output; update
runInstallSkill to branch on the global OutputMode (see OutputMode usage in
src/bin.ts) and emit a JSON object (including arrays of successes and failures
with skill, agent.displayName, and error fields, plus overall status and bundled
skills version) when mode === 'json' instead of human console output, ensuring
exit code reflects failure when any installs fail; retain existing human output
branch. Also add JSON-mode unit tests in src/commands/install-skill.spec.ts that
invoke runInstallSkill (or the CLI entry) with OutputMode set to json and assert
the emitted JSON structure for both all-success and partial-failure cases, and
include a test that verifies version marker writing behavior tied to
getBundledSkillsVersion, writeFile, and SKILL_VERSION_MARKER_FILENAME.
---
Duplicate comments:
In `@src/commands/install-skill.ts`:
- Around line 233-246: Only write the .workos-skill-version marker when every
requested skill for that specific agent succeeded: build a per-agent set (or
count) of requested installs and compare it to the per-agent successes in
successful (instead of marking an agent when any single install succeeded via
succeededAgents); only call writeFile(join(agent.globalSkillsDir,
SKILL_VERSION_MARKER_FILENAME), version, 'utf8') for agents where successful
entries fully cover that agent's requested skills so checkSkills() won't treat
partially-installed agents as current.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f2a42966-9ecf-44fa-902d-7d97fcc8c11b
📒 Files selected for processing (8)
src/commands/install-skill.spec.tssrc/commands/install-skill.tssrc/commands/install.spec.tssrc/commands/login.tssrc/doctor/checks/skills.spec.tssrc/doctor/checks/skills.tssrc/doctor/index.tssrc/utils/help-json.ts
Three findings I missed in the previous review-fix pass plus the
broader sync-fs cascade I had pushed back on. Reconsidered after
re-reading CLAUDE.md's 'Avoid Node-specific sync APIs' guideline as
a project rule rather than a suggestion.
* `installSkillsAfterLogin`'s setTimeout handle is now stored, both
`unref()`'d on creation and `clearTimeout()`'d in a finally block.
Without this, when `autoInstallSkills` won the race the pending
timer kept the Node event loop alive for up to 30s and the CLI
appeared to hang. (CodeRabbit, login.ts:91)
* `checkSkills` now treats `<agent>/skills/workos-widgets/` as a
pre-marker install signal in addition to `workos/`. An agent that
only had `workos-widgets/` from an older explicit install was
invisible to doctor under the previous narrowing. (CodeRabbit,
skills.ts:48)
* Sync fs APIs replaced with async equivalents in install-skill.ts
and doctor/checks/skills.ts:
- `getBundledSkillsVersion` → async (await readFile)
- `readSkillVersionMarker` → async (await readFile + access-based pathExists)
- `checkSkills` → async (cascades to runDoctor, maybeRefreshSkills)
- `installSkill`'s `existsSync(targetDir)` → `pathExists`
- `cleanupStaleOrphans`'s `existsSync(parent)` → `pathExists`
- `discoverSkills` filter uses `Promise.all(pathExists(...))`
- `existsSync` import retained only for `agent.detect()` callbacks
(sync-by-design boolean predicates inside detection table)
(CodeRabbit, install-skill.ts:28+132 and skills.ts:5)
Plus an opportunistic cleanup that came out of the cascade:
* Extracted `writeAgentSkillMarker` helper so `runInstallSkill` and
`refreshWorkOSSkills` share the same best-effort marker semantics
(single source of truth, no behavioral drift). Partially addresses
CodeRabbit's "route runInstallSkill through refreshWorkOSSkills" —
the marker-write logic is the actual functional duplication and is
now shared; the install loop itself stays separate because
runInstallSkill needs granular per-(skill, agent) failure rendering
that the primitive doesn't expose.
Tests:
- `skills.spec.ts` updated for async checkSkills + new test for
workos-widgets-only install + new test for downgrade scenario
(installed > bundled must NOT be flagged stale, semver fix)
- `skills-fix.spec.ts` switched to mockResolvedValueOnce for the now
async checkSkills mock
- All 1614 tests pass
Comment on lines
+163
to
+168
| const slugs = Object.keys(report.skillsRefresh.before); | ||
| for (const slug of slugs) { | ||
| const before = formatVersion(report.skillsRefresh.before[slug] ?? null); | ||
| const after = formatVersion(report.skillsRefresh.after[slug] ?? null); | ||
| console.log(` ${Chalk.green('✓')} Updated WorkOS skills for ${displayName(slug)}: ${before} → ${after}`); | ||
| } |
There was a problem hiding this comment.
✓ Updated rendered for agents where install failed
report.skillsRefresh.before (and .after) are keyed by agent.name for every detected agent, including those where all skill installs failed. refreshWorkOSSkills populates perAgentBefore/perAgentAfter inside the full detected loop, but only adds agents to succeededAgents when at least one install succeeds.
For a partially-failed run (e.g., Claude Code succeeds, Codex fails), the current code prints:
✓ Updated WorkOS skills for Codex: (none) → (none)
— a green checkmark with no version change, which users will reasonably read as a success.
Scope the rendered slugs to agents where before !== after:
const slugs = Object.keys(report.skillsRefresh.before).filter(
(slug) => report.skillsRefresh!.before[slug] !== report.skillsRefresh!.after[slug],
);
This was referenced May 12, 2026
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
1 participant
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
Closes Batch A of the cus-feedback-fixes initiative — the WorkOS skill content the CLI auto-installs is now correctly delivered end-to-end (
SKILL.mdplus the entirereferences/tree), self-healing on stale state viaworkos doctor --fix, and refreshed on everyworkos auth login. This is the surface that addresses the customer friction case where claude diagnosed a CLI gap, sent users to fabricated commands or non-existent dashboard click-paths, and had no way to recover.Builds on top of the existing branch (which surfaced auto-install + added a doctor staleness warning) by replacing the install machinery, extracting a refresh primitive, wiring the post-login hook, and adding an explicit
--fixflag on doctor.What's in this PR
Existing (already in the branch)
autoInstallSkillsruns after everyworkos install, copies bundled@workos/skillscontent to every detected coding agent, prints a one-line summary in TTY mode, suppresses in JSON mode, writes a.workos-skill-versionmarker per agent.workos doctorcheckSkillscompares each agent's marker against the bundled version and emitsSKILLS_OUTDATEDwhen they drift.New: recursive prune-replace install (Phase 2)
installSkillwas a singlecopyFile(SKILL.md)— it shipped the router but not the topic files the router instructs the agent to Read. Rewritten to:mkdtemp+ recursivecp+ sibling.workos.bak-<skill>-<rand>backup-rename + rollback if the temp→target rename fails. Operation is effectively atomic per skill.cleanupStaleOrphansreaps it after 1h.cleanupStaleOrphanswith a 1-hour mtime cutoff: orphans from crashed prior runs are removed, but fresh siblings from a concurrent run are preserved (no race-deletion).New: refresh primitive (Phase 2)
refreshWorkOSSkills(opts)is a reusable primitive returningRefreshResultwithperAgentBefore/perAgentAfterkeyed byagent.name. BothautoInstallSkills(best-effort hook) and doctor--fixcall this — no duplicate copy logic.autoInstallSkillsbecomes a thin back-compat wrapper.New: auth-login skill install hook (Phase 2)
installSkillsAfterLoginhelper wired intorunLoginafterprovisionStagingEnvironment. WrapsautoInstallSkillsin its own try/catch so a skill-install failure NEVER fails login itself. JSON-mode-aware. Extracted as a separate exported function so it's unit-testable without standing up the device-auth polling loop.New:
workos doctor --fix(Phase 3)workos doctor(no flag) keeps its warn-only behavior.--fixis the explicit opt-in for mutating state from a diagnostic command. When set and at least one agent has a stale or missing marker:FIXABLE_SKILLS = ['workos', 'workos-widgets']allowlist — NOT derived fromdiscoverSkills(). A future bundled skill needs explicit opt-in here before doctor will write to its target directory.checkSkills()is re-read anddetectIssuesruns against the post-refresh state — no lingeringSKILLS_OUTDATEDwarning after the refresh has already fixed it.✓ Updated WorkOS skills for {Agent}: {before} → {after}(null →(none)).skillsRefreshviaJSON.stringify(report).Bump:
@workos/skills0.2.4 → 0.4.0@workos/skills@0.4.0published today (workos/skills#26 + the release-please release PR). Pulling the new content in so the recursive install machinery actually delivers the new guardrails:references/workos-cli-upgrade.md— router-style content for "user is on outdatedworkos" with explicit "do not fabricate the latest version" guardrails (instructs the agent to runnpm view workos version).SKILL.mdRule 6 sub-case routing outdated-CLI symptoms to the new reference.workos-management.md"Detecting and recommending CLI upgrades" subsection.Test plan
pnpm typecheck— cleanpnpm build— cleanpnpm test src/commands/install-skill.spec.ts src/commands/login.spec.ts src/doctor— 183 tests passpnpm test— 1611 tests across 124 filespnpm installresolves@workos/skills@0.4.0;node_modules/@workos/skills/plugins/workos/skills/workos/references/workos-cli-upgrade.mdis presentreferences/, prune-replace (planted stale file gone after re-install), rollback on simulated rename failure, cp-failure tempDir cleanup, mtime cutoff for orphan cleanup (both prefixes, both directions), peer-skill safetyautoInstallSkills, returns successfully when installer throws, JSON-mode skip, singular/plural copyskills-fix.spec.tscovers:--fix=falseno-op,--fix=trueno-op when no skills info / when nothing stale, allowlist passed exactly torefreshWorkOSSkills, success path returnsskillsRefresh+ post-refresh re-read, refresh-null preserves original skills, integration: planted third-party-skill and hypotheticalworkos-future-skillfiles at the agent target are byte-identical after--fixSequencing notes
@workos/skills@0.4.0.getUnsupportedOperations()named export +workos helpcommand) are Batch B per the contract — can defer to a follow-up sprint.Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests