Local Skills and slash commands#3787
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
Bot
added
vouch:unvouched
lmtr0
changed the title
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| const snapshotCapabilities = useMemo(() => { | ||
| const provider = | ||
| target.providers?.find((candidate) => candidate.instanceId === target.providerInstanceId) ?? | ||
| null; | ||
| return queryTarget === null && | ||
| provider !== null && | ||
| isProviderProjectCapabilitiesProviderQueryable(provider) | ||
| ? provider | ||
| : null; | ||
| }, [queryTarget, target.providerInstanceId, target.providers]); |
There was a problem hiding this comment.
🟡 Medium state/queries.ts:152
snapshotCapabilities is returned whenever queryTarget === null, including when cwd is null/blank. In that case the hook returns the provider's global skills and slashCommands instead of an empty project-scoped set, so the UI shows commands/skills for the wrong repository context — reintroducing non-project-scoped commands exactly when no project/worktree path is available. The snapshot fallback was meant to cover the case where the project lacks a provider that supports capabilities queries, not the case where the caller has no cwd. Consider gating the fallback on queryTarget === null && provider !== null && cwd is present, or otherwise excluding the no-cwd case so callers receive empty arrays when there is no project path.
| const snapshotCapabilities = useMemo(() => { | |
| const provider = | |
| target.providers?.find((candidate) => candidate.instanceId === target.providerInstanceId) ?? | |
| null; | |
| return queryTarget === null && | |
| provider !== null && | |
| isProviderProjectCapabilitiesProviderQueryable(provider) | |
| ? provider | |
| : null; | |
| }, [queryTarget, target.providerInstanceId, target.providers]); | |
| const snapshotCapabilities = useMemo(() => { | |
| const provider = | |
| target.providers?.find((candidate) => candidate.instanceId === target.providerInstanceId) ?? | |
| null; | |
| return queryTarget === null && | |
| provider !== null && | |
| target.cwd != null && | |
| isProviderProjectCapabilitiesProviderQueryable(provider) | |
| ? provider | |
| : null; | |
| }, [queryTarget, target.providerInstanceId, target.providers, target.cwd]); |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/state/queries.ts around lines 152-161:
`snapshotCapabilities` is returned whenever `queryTarget === null`, including when `cwd` is `null`/blank. In that case the hook returns the provider's global `skills` and `slashCommands` instead of an empty project-scoped set, so the UI shows commands/skills for the wrong repository context — reintroducing non-project-scoped commands exactly when no project/worktree path is available. The snapshot fallback was meant to cover the case where the project lacks a provider that supports capabilities queries, not the case where the caller has no `cwd`. Consider gating the fallback on `queryTarget === null && provider !== null && cwd is present`, or otherwise excluding the no-`cwd` case so callers receive empty arrays when there is no project path.
| useEffect(() => { | ||
| if ( | ||
| forceReloadTargetKey !== null && | ||
| (forceReloadTargetKey !== targetKey || queryTarget === null) | ||
| ) { | ||
| setForceReloadTargetKey(null); | ||
| } | ||
| }, [forceReloadTargetKey, queryTarget, targetKey]); |
There was a problem hiding this comment.
🟡 Medium state/queries.ts:261
After the first manual refresh(), forceReload stays true forever for the current targetKey. The cleanup useEffect only resets forceReloadTargetKey when targetKey changes or queryTarget becomes null, so on a stable target it never clears. This keeps the hook bound to the { forceReload: true } query atom, and because projectCapabilities atoms revalidate on mount, every subsequent remount or revalidation re-sends forceReload: true, repeatedly bypassing the cache and forcing unnecessary rescans of project skills and slash commands. Consider clearing forceReloadTargetKey after the forced query completes — e.g. when result is no longer pending — so the flag is only set transiently.
useEffect(() => {
- if (
- forceReloadTargetKey !== null &&
- (forceReloadTargetKey !== targetKey || queryTarget === null)
- ) {
+ if (
+ forceReloadTargetKey !== null &&
+ (forceReloadTargetKey !== targetKey || queryTarget === null || !result.isPending)
+ ) {
setForceReloadTargetKey(null);
}
- }, [forceReloadTargetKey, queryTarget, targetKey]);
+ }, [forceReloadTargetKey, queryTarget, targetKey, result.isPending]);🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/state/queries.ts around lines 261-268:
After the first manual `refresh()`, `forceReload` stays `true` forever for the current `targetKey`. The cleanup `useEffect` only resets `forceReloadTargetKey` when `targetKey` changes or `queryTarget` becomes `null`, so on a stable target it never clears. This keeps the hook bound to the `{ forceReload: true }` query atom, and because `projectCapabilities` atoms revalidate on mount, every subsequent remount or revalidation re-sends `forceReload: true`, repeatedly bypassing the cache and forcing unnecessary rescans of project skills and slash commands. Consider clearing `forceReloadTargetKey` after the forced query completes — e.g. when `result` is no longer pending — so the flag is only set transiently.
| : composerTrigger?.kind === "skill" || composerTrigger?.kind === "slash-command" | ||
| ? providerProjectCapabilities.error | ||
| : null; | ||
| const showComposerCommandPopover = |
There was a problem hiding this comment.
🟠 High threads/ThreadComposer.tsx:503
Typing /model in the composer no longer shows any autocomplete suggestions or selection UI, breaking the existing slash-command workflow for switching models. detectComposerTrigger classifies /model as kind: "slash-model", but showComposerCommandPopover returns false for that trigger kind and the built-in /model menu item was removed from composerMenuItems, so the popover never appears. Consider re-enabling the popover for slash-model triggers or providing an alternative model-selection UI within the composer for that trigger.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadComposer.tsx around line 503:
Typing `/model` in the composer no longer shows any autocomplete suggestions or selection UI, breaking the existing slash-command workflow for switching models. `detectComposerTrigger` classifies `/model` as `kind: "slash-model"`, but `showComposerCommandPopover` returns `false` for that trigger kind and the built-in `/model` menu item was removed from `composerMenuItems`, so the popover never appears. Consider re-enabling the popover for `slash-model` triggers or providing an alternative model-selection UI within the composer for that trigger.
| )?.skills ?? [], | ||
| [selectedEnvironmentServerConfig, selectedModel?.instanceId], | ||
| ); | ||
| const selectedProviderCapabilities = useProviderProjectCapabilities({ |
There was a problem hiding this comment.
🟡 Medium threads/new-task-flow-provider.tsx:384
useProviderProjectCapabilities receives cwd: selectedWorktreePath || selectedProject?.workspaceRoot, but selectedWorktreePath is not cleared when the user switches workspaceMode back to "local" — setWorkspaceMode preserves the existing selectedWorktreePath in the updated draft. After a user selects a worktree branch and then returns to local mode, the hook still queries capabilities for the stale worktree path, so the composer shows skills and slash commands scoped to the abandoned worktree instead of the local workspace. Consider passing workspaceMode === "worktree" ? (selectedWorktreePath || selectedProject?.workspaceRoot || null) : (selectedProject?.workspaceRoot || null) as the cwd argument.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/new-task-flow-provider.tsx around line 384:
`useProviderProjectCapabilities` receives `cwd: selectedWorktreePath || selectedProject?.workspaceRoot`, but `selectedWorktreePath` is not cleared when the user switches `workspaceMode` back to `"local"` — `setWorkspaceMode` preserves the existing `selectedWorktreePath` in the updated draft. After a user selects a worktree branch and then returns to local mode, the hook still queries capabilities for the stale worktree path, so the composer shows skills and slash commands scoped to the abandoned worktree instead of the local workspace. Consider passing `workspaceMode === "worktree" ? (selectedWorktreePath || selectedProject?.workspaceRoot || null) : (selectedProject?.workspaceRoot || null)` as the `cwd` argument.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 5 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6f046fc. Configure here.
| slashCommands: | ||
| result.data?.slashCommands ?? | ||
| snapshotCapabilities?.slashCommands ?? | ||
| EMPTY_PROVIDER_SLASH_COMMANDS, |
There was a problem hiding this comment.
Web hides capability errors
Medium Severity
The command menu doesn't correctly surface errors when project capabilities (skills/slash commands) fail to load or when project context is unavailable. Instead of showing the error, it falls back to displaying built-in commands or global/cached capabilities, which hides the underlying issue and presents irrelevant options.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6f046fc. Configure here.
| return; | ||
| } | ||
| setForceReloadTargetKey(targetKey); | ||
| }, [forceReload, result.refresh, targetKey]); |
There was a problem hiding this comment.
Refresh leaves forceReload enabled
Medium Severity
Calling refresh on useProviderProjectCapabilities sets forceReloadTargetKey and never clears it while the same environment, provider instance, and cwd stay selected. Every later fetch keeps sending forceReload: true to providers.projectCapabilities, not only the intended retry.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6f046fc. Configure here.
| result.data?.slashCommands ?? | ||
| snapshotCapabilities?.slashCommands ?? | ||
| EMPTY_PROVIDER_SLASH_COMMANDS, | ||
| error: result.error, |
There was a problem hiding this comment.
Global skills without project cwd
Medium Severity
useProviderProjectCapabilities falls back to the global ServerProvider snapshot for skills and slashCommands whenever the project RPC target is skipped because cwd (or other key fields) is missing, even though the provider is otherwise queryable. The UI can show non–project-scoped capabilities until a valid cwd arrives.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6f046fc. Configure here.
| const builtIn = allBuiltIn.filter((item) => item.command.includes(q)); | ||
|
|
||
| const providerCommands: ComposerCommandItem[] = []; | ||
| for (const cmd of selectedProviderStatus?.slashCommands ?? []) { |
There was a problem hiding this comment.
Mobile dropped slash model command
Medium Severity
The mobile composer built-in slash list no longer includes /model, which was removed while refactoring to providerProjectCapabilities.slashCommands. Typing /mo no longer surfaces the model switch entry that web still offers via built-ins.
Reviewed by Cursor Bugbot for commit 6f046fc. Configure here.
| result.data?.slashCommands ?? | ||
| snapshotCapabilities?.slashCommands ?? | ||
| EMPTY_PROVIDER_SLASH_COMMANDS, | ||
| error: result.error, |
There was a problem hiding this comment.
Failed fetch keeps stale skills
Medium Severity
After a successful project capabilities load, a later failed refetch still exposes result.data skills and slash commands while error is set, because the hook always prefers cached query data and never clears it on failure.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6f046fc. Configure here.
There was a problem hiding this comment.
The new ProviderProjectCapabilitiesError violates the error-modeling conventions: it adds a detail field that is populated by copying the underlying failure's message/detail and then builds the wrapper message from it. Two related spots flagged inline.
Posted via Macroscope — Effect Service Conventions
| provider: Schema.String, | ||
| instanceId: Schema.String, | ||
| cwd: Schema.String, | ||
| detail: Schema.String, |
There was a problem hiding this comment.
detail: Schema.String here merely copies the underlying failure's text (via makeProviderProjectCapabilitiesError → describeCapabilityProbeFailure, which returns cause.detail/cause.message), and the message getter then interpolates it (: ${this.detail}).
Per the conventions, don't add a detail field that copies cause.message/raw defect text and use it to construct the wrapper message; derive message exclusively from the stable structural attributes (provider, instanceId, cwd) and keep the underlying failure only in cause. Suggested fix: drop the detail field and change the getter to e.g. Provider '${this.provider}' failed to list project capabilities for instance '${this.instanceId}' in '${this.cwd}'., letting cause carry the real reason.
Posted via Macroscope — Effect Service Conventions
| function describeCapabilityProbeFailure(cause: unknown): string { | ||
| if ( | ||
| cause && | ||
| typeof cause === "object" && | ||
| "detail" in cause && | ||
| typeof cause.detail === "string" && | ||
| cause.detail.trim().length > 0 | ||
| ) { | ||
| return cause.detail; | ||
| } | ||
| if ( | ||
| cause && | ||
| typeof cause === "object" && | ||
| "message" in cause && | ||
| typeof cause.message === "string" && | ||
| cause.message.trim().length > 0 | ||
| ) { | ||
| return cause.message; | ||
| } | ||
| return "Provider project capability probe failed."; | ||
| } | ||
|
|
||
| export function withProviderProjectCapabilitiesTimeout<A, E, R>( | ||
| effect: Effect.Effect<A, E, R>, | ||
| ): Effect.Effect<A, E | ProviderProjectCapabilitiesProbeTimeoutError, R> { | ||
| return effect.pipe( | ||
| Effect.timeoutOption(Duration.millis(PROJECT_CAPABILITIES_PROBE_TIMEOUT_MS)), | ||
| Effect.flatMap( | ||
| Option.match({ | ||
| onNone: () => | ||
| Effect.fail( | ||
| new ProviderProjectCapabilitiesProbeTimeoutError({ | ||
| timeoutMs: PROJECT_CAPABILITIES_PROBE_TIMEOUT_MS, | ||
| }), | ||
| ), | ||
| onSome: Effect.succeed, | ||
| }), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| export function makeProviderProjectCapabilitiesError(input: { | ||
| readonly provider: ProviderDriverKind; | ||
| readonly instanceId: ProviderInstanceId; | ||
| readonly cwd: string; | ||
| readonly cause: unknown; | ||
| }): ProviderProjectCapabilitiesError { | ||
| return new ProviderProjectCapabilitiesError({ | ||
| provider: input.provider, | ||
| instanceId: input.instanceId, | ||
| cwd: input.cwd, | ||
| detail: describeCapabilityProbeFailure(input.cause), |
There was a problem hiding this comment.
makeProviderProjectCapabilitiesError (with describeCapabilityProbeFailure) exists only to extract cause.detail/cause.message into the wrapper's detail field — the exact pattern the conventions forbid (copying arbitrary defect text into detail). Once detail is removed from ProviderProjectCapabilitiesError, construct the error at the failure boundary with its structural fields plus the real cause and drop this factory and describeCapabilityProbeFailure, rather than normalizing raw failure text into a caller-visible string.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Needs human review 4 blocking correctness issues found. This PR introduces a new project-scoped capabilities feature with changes across mobile, web, and server. Multiple unresolved review comments identify substantive issues including a broken /model command on mobile and fallback logic bugs that show incorrect capabilities in certain contexts. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Important
This is the shared implementation for all the other adapters. To make the PR small and easy to review I have created different branches for the other adapters. This branch contains all the shared code and #3788 contains the codex implementation. If they both get merged, I'll create PRs for claude code, opencode, and cursor which I already have implemented, but I would rather not spam the repository with tons of PRs.
THIS PR needs to be merged before any adapter is merged. It contains the base for all of them.
Why
I use skills to organize my projects; having the ability to access them easier inside t3 would be a game changer.
Why am I doing it? I saw the issue, thought it would be a quick fix; it actually is, and started using my fork; it improved my experience, so I thought it would be a good PR.
Closes #3576
What Changed
Adds project-scoped provider skills and slash-commands discovery.
Code Changes
providers.projectCapabilitiesRPC with shared contract schemas.Testing
vp run typecheckvp checkUI Changes
For context, here is a test skill:
Current nightly app on a repo with multiple skills and commands, in opencode:
Same repo, nightly, in codex:
My pr, same repo, in opencode:
same repo, in codex:
Checklist
Note
Medium Risk
Touches provider registry, a new orchestration-read RPC, and composer UX across web and mobile; probe failures are surfaced instead of cached as empty, but unavailable instances still resolve to empty capabilities without error.
Overview
Adds project/worktree-scoped discovery for provider skills and slash commands instead of relying only on global provider snapshots.
Server & contracts: New
providers.projectCapabilitiesRPC with input/result/error schemas.ProviderRegistry.listProjectCapabilitiesdelegates to optional per-instancecomposerCapabilities.listprobes (with timeout/error mapping); disabled, unavailable, or unsupported instances return empty lists without masking live probe failures.Clients: Shared
useProviderProjectCapabilities(web + mobile) queries by environment, provider instance, andcwd, with snapshot fallback andforceReloadrefresh. Composers and thread feeds use thread/worktreecwdfor menus, editor skill highlighting, and loading/error UI in the command popover.Note: Driver-specific probes are wired via
composerCapabilitiesonProviderInstance; this PR lands the shared plumbing adapters build on in follow-up branches.Reviewed by Cursor Bugbot for commit 6f046fc. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add provider project capabilities RPC with client query hooks and server-side plumbing
providers.projectCapabilitiesWebSocket RPC in ws.ts that fetches provider-scoped skills and slash commands for a givencwd, requiringAuthOrchestrationReadScope.listProjectCapabilitiesin ProviderRegistry.ts; unsupported or unavailable providers return empty arrays, while live providers delegate to the driver'scomposerCapabilities.list.useProviderProjectCapabilitieshooks for web and mobile that query capabilities via the new RPC, with snapshot fallback and aforceReloadmechanism.ChatComposer,ChatView,ThreadComposer, andThreadDetailScreento source skills and slash commands from the new hook instead of provider status snapshots.ServerProviderProjectCapabilitiesInput/Result/Error) with anExactNonBlankStringcwdfield and default-emptyskills/slashCommandsarrays.📊 Macroscope summarized 6f046fc. 19 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.