Commit c61f5a4
authored
feat: complete WorkOS skill install + refresh loop with doctor --fix (#130)
* feat: surface WorkOS skill auto-install and track version
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.
* feat(doctor): warn when WorkOS skills are outdated
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.
* feat(install): recursive prune-replace skill copy with rollback
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.
* feat(doctor): add --fix to refresh stale WorkOS skills
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.
* chore(deps): bump @workos/skills from 0.2.4 to 0.4.0
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.
* chore: formatting
* fix: address PR review feedback (CodeRabbit / Greptile / Codex)
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.
* fix: address remaining CodeRabbit findings on PR #130
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
* chore: oxfmt format
* docs(readme): mention auth-login skill hook and doctor --fix1 parent 9728bd7 commit c61f5a4
19 files changed
Lines changed: 1283 additions & 57 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
62 | | - | |
| 62 | + | |
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| |||
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
343 | 343 | | |
344 | 344 | | |
345 | 345 | | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
346 | 351 | | |
347 | 352 | | |
348 | 353 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
| 23 | + | |
22 | 24 | | |
23 | 25 | | |
24 | 26 | | |
| |||
0 commit comments