feat: add grok, omp, and reasonix built-in providers - #557
Open
AgileInnov8tor wants to merge 1 commit into
Open
Conversation
Register Oh My Pi, Grok CLI, and Reasonix in DEFAULT_PROVIDERS after pi so asm list/install work against those skill roots without customPaths. Upgrade merge inserts the three names between pi and hermes.
Owner
|
Reviewed this one — the change itself looks great. Full unit suite is green at One thing blocking merge: the branch is 2 commits behind git fetch origin && git rebase origin/main
# in README.md keep both: main's refreshed catalog stats + this PR's 22-provider counts
git push --force-with-lease origin feat/providers-grok-omp-reasonixOnce that's in, this is good to merge from my side. Two small notes, neither blocking:
|
5 tasks
luongnv89
added a commit
that referenced
this pull request
Sep 5, 2026
…d Grok CLI (#617) (#618) ## Description Reorders the interactive `asm install` provider picker and extends the built-in provider list. New canonical order: **Agents, Claude Code, Pi, OpenCode, Codex, Oh My Pi, Grok CLI**, then the remaining providers in their existing relative order. The Agents row carries a note that most harnesses support it, except Claude Code; the note leads the hint so it survives truncation on a narrow terminal. Three things the issue asked for that needed a decision: - **`amp code` already existed** as the built-in `amp` (`~/.amp/skills`). No second entry was added; a duplicate name would have been dropped silently by `mergeWithDefaults`. - **`omp` and `grok build`** are new. They reuse the definitions from open PR #557 so the two stay compatible: `omp` = Oh My Pi at `~/.omp/agent/skills`, `grok` = Grok CLI at `~/.grok/skills`. #557's third provider, `reasonix`, was deliberately not carried in — #617 does not ask for it. - **Reordering the defaults alone would not have fixed anything for existing users.** `mergeWithDefaults` only splices in missing defaults and never reorders what is already saved, so the reporter's own picker would have looked exactly as in their screenshot. It now canonicalizes the provider order on load. User-added providers keep their relative anchor and nothing is dropped. Mid-flight addition from the reporter, recorded on the issue: first-time setup pre-checks **Agents and Claude Code**, not Agents alone. It never pre-checks a provider the user has disabled. ## Related Issue Closes #617 Overlaps open PR #557 (`omp`, `grok`, `reasonix`). This PR carries the first two with the same names and paths. #557 can be closed or rebased down to `reasonix` alone. ## Type of Change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Documentation update - [ ] Refactoring (no functional changes) ## Decision Record | Decision | Why | | --- | --- | | Canonicalize saved provider order in `mergeWithDefaults` | Reordering `DEFAULT_PROVIDERS` alone reaches only fresh installs; the reporter has a saved config, so acceptance criterion 1 would have failed on their own machine | | Anchor user-added providers rather than appending them | Keeps `preserves user-added custom providers in place` passing; a custom provider stays adjacent to the built-in it followed | | Note leads the Agents hint, path trails | `checkbox-picker` truncates the hint at narrow widths and drops it entirely below 10 columns of room; putting the note first means the requested text is the part that survives | | No new `ProviderConfig` field for the note | The note is picker presentation, and `ProviderConfig` is persisted to `config.json` — a new field would mean migration churn for one string | | Skipped `reasonix` | Not requested by #617 | ## Test Results `CI=true npm test` — **2664 passed, 83 files, 0 failed**. `npm run typecheck`, `npm run lint`, `npm run lint:site` — all clean. New and updated coverage: - `src/config.test.ts` — pins the 21-provider canonical order; new test canonicalizes a pre-#617 saved config and asserts nothing the user had is dropped. - `src/installer.test.ts` — first-run pre-check is Agents + Claude Code; a disabled provider is never pre-checked; the Agents hint text is pinned. ## Acceptance Criteria Verification | Criterion | Status | Evidence | | --- | --- | --- | | Picker lists Agents, Claude Code, Pi, OpenCode, Codex as its first five, in order | ✓ | `src/config.test.ts` `all 21 default providers in priority order` | | Every provider still appears after those five, none dropped | ✓ | `canonicalizes a saved config that predates the #617 reorder` asserts every legacy name survives | | Agents entry shows the harness-support note | ✓ | `src/installer.test.ts` pins the hint string | | Grok Build, Amp Code, and omp are selectable | ✓ | `omp` and `grok` added; `amp` was already present | | Select All / Deselect All and selection behave as before | ✓ | `src/utils/checkbox-picker.test.ts` and the `resolveProvider` picker suite unchanged and passing | | First-time setup pre-checks Claude Code + Agents (added mid-flight) | ✓ | `no saved tools defaults agents + claude to checked` | ## Review One blocking finding, fixed in 40ed06a: the first-run pre-check ignored `enabled`, so a user who had disabled Claude Code would have had it pre-checked and installed on Enter. Gated on `p.enabled`. ## Additional Notes Existing users see their `~/.config/agent-skill-manager/config.json` provider array rewritten into the new order on the next load. `preferences.selectedTools` is stored by name, so saved selections are unaffected. https://claude.ai/code/session_01L7wEboaD5rvJst7KeKiEzq --------- Co-authored-by: Test <test@test.com>
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.
Description
asm list/asm installcan now target Oh My Pi, Grok CLI, and Reasonix without adding custom provider entries. Existing configs pick the three names up on upgrade, inserted afterpiand beforehermes.Oh My Pi uses
~/.omp/agent/skills(not~/.omp/skillsand not Pi's~/.pi/skills).Related Issue
None.
Type of Change
Checklist
Additional Notes
Focused
npx vitest run src/config.test.ts: 30/30. The new upgrade case asserts a saved 19-provider config gainsomp/grok/reasonixbetweenpiandhermes.src/views/config.test.tsxstill passes (8/8).npm run typecheckandnpm run lintfail on this checkout independently of this change (src/commands/stats.test.tsIndexedSkill/no-colorerrors; typescript-eslint vs TS 7). Not addressed here.Post-Deploy Monitoring & Validation
No additional operational monitoring required until the next
agent-skill-managerrelease. After publish, confirmasm config showincludes the three names on a fresh or upgraded config, and thatasm list -p grok|omp|reasonixscans the documented global paths. Rollback: disable the providers in config, or revert this commit.