Fix workspace-scoped Codex skill discovery#3059
Conversation
- Propagate cwd through provider status probes - Add server RPC for workspace skill discovery - Load workspace skills in the composer UI
- Clarify bogus skill as a durable discovery test fixture - Stabilize composer fallback skill array identity
- Skip Codex skill spawning for disabled instances - Move bogus skill fixture out of workspace discovery
- Refresh workspace skill cache on provider and connection changes - Validate Codex skill cwd before spawning the app server - Cover server.listProviderSkills RPC branches
- Track the active workspace key in provider skill state - Reset pending skills when switching workspace targets
|
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 Plus 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
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review This PR introduces significant new functionality including a new RPC endpoint, server-side request caching infrastructure, and cross-platform state management for workspace-scoped skill discovery. The scope (3163 additions across server, web, mobile, and shared packages) and the introduction of new capabilities require human review. You can customize Macroscope's approvability policy. Learn more. |
- Keep pending skill lookups scoped to the active workspace key - Surface Codex skill-list timeout errors in the composer - Add regression coverage for stale pending skills
github-actions
Bot
added
size:XL
# Conflicts: # apps/server/src/provider/Layers/CursorProvider.ts # apps/web/src/components/chat/ChatComposer.tsx
6acc264 to
9f66820
Compare
github-actions
Bot
added
size:L
# Conflicts: # apps/web/src/environments/runtime/connection.test.ts # apps/web/src/environments/runtime/service.savedEnvironments.test.ts # apps/web/src/environments/runtime/service.threadSubscriptions.test.ts # apps/web/src/localApi.ts # packages/client-runtime/src/wsRpcClient.ts
Co-authored-by: codex <codex@users.noreply.github.com>
9f66820 to
00cebad
Compare
- Keep prior workspace skills visible while refreshes are pending - Add tests for loaded, pending, workspace-switch, and empty states
- Preserve and clear cached workspace skills by query state - Cover pending data, empty data, and workspace switches
github-actions
Bot
added
size:XL
Co-authored-by: codex <codex@users.noreply.github.com>
- Forward configured launch arguments to workspace skill probes - Clear retained workspace skills after failed refreshes - Add focused probe and snapshot regressions
|
@jakeleventhal Thanks for the pointer. I confirmed #3982 carries the provider-neutral Codex/Grok approach and explicitly supersedes #3901. This branch still includes additional mobile composer/feed coverage, timeline skill decoration, shared web/mobile refresh/error policy, and bounded structured failure handling, so I’ll leave the overlap visible for maintainers to decide what to merge or port. |
- Explain why provider snapshots stay hidden during pending lookups
- Pass a null skill cwd until a draft worktree materializes - Keep local checkout skill queries and path searches unchanged - Cover local, materialized, and future worktree resolution
# Conflicts: # apps/server/src/provider/Layers/CodexProvider.test.ts
- Keep matching workspace snapshots even when they contain no skills - Cover empty disconnected snapshots with a regression test
- Keep snapshots keyed while lazy skill menus close - Clear retained data when the workspace target changes
# Conflicts: # apps/web/src/components/ChatView.logic.test.ts # apps/web/src/components/ChatView.tsx # apps/web/src/components/chat/MessagesTimeline.test.tsx
# Conflicts: # apps/web/src/components/chat/ComposerCommandMenu.tsx
- Follow mobile environment connectivity and lazy lookup state - Clear retained skill snapshots across workspace switches
- Cache successful workspace skill probes for the configured TTL - Expire failed probes immediately and cover retry behavior - Document the success-only cache policy in branch details
# Conflicts: # apps/web/src/components/ChatView.tsx
- Activate lookups from the skill menu, draft, or user feed - Cover empty threads and non-user skill references
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 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 9388e7f. Configure here.
|
|
||
| function requestKey(input: ProviderSkillsListInput): string { | ||
| return JSON.stringify([input.instanceId, input.cwd]); | ||
| } |
There was a problem hiding this comment.
Skills cache ignores normalized cwd
Low Severity
requestKey caches by the raw request cwd, but the Codex probe runs against normalizeWorkspaceRoot output. Equivalent paths that resolve to the same directory miss coalescing and the success TTL, so identical workspace lookups can spawn extra app-server processes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9388e7f. Configure here.
| ? PROVIDER_WORKSPACE_SKILLS_UNAVAILABLE_MESSAGE | ||
| : formatProviderWorkspaceSkillsError({ error: query.error, cause: query.errorCause }), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Duplicated workspace skills hooks
Medium Severity
Web and mobile each ship a full copy of useProviderWorkspaceSkills with the same target-key, unavailable, inactive-snapshot, and fallback policy. Only the useEnvironmentQuery / serverEnvironment imports differ, so policy fixes must be landed twice.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9388e7f. Configure here.
| addTextField(output, "name", cause.name); | ||
| addTextField(output, "message", cause.message); | ||
| return output; | ||
| } |
There was a problem hiding this comment.
Dead Error sanitization branch
Low Severity
sanitizeErrorCause handles typeof cause === "object" before the instanceof Error check. Real Error values are objects, so the later branch is unreachable and any Error-only handling there never runs.
Reviewed by Cursor Bugbot for commit 9388e7f. Configure here.
3 participants
PR ID: 3059
PR URL: #3059
PR TITLE: Fix workspace-scoped Codex skill discovery
Summary
Fix Codex repo-local skill discovery by resolving skills for the active project or worktree cwd instead of relying on the provider-wide status snapshot.
The implementation adds a workspace-scoped provider-skills RPC and a shared web/mobile resolution policy so composer suggestions and rendered skill chips refresh for the correct workspace without leaking stale skills across workspace, environment, or provider changes.
What Changed
server.listProviderSkillscontracts, WebSocket and local API handlers, and environment-query wiring keyed by environment, provider instance, and cwd.ProviderSkillsListerto return snapshot skills for non-Codex or disabled Codex providers and run Codexskills/listin the validated workspace cwd for enabled Codex providers.$menu, complete draft skill tokens, and sent user skill references activate the shared lookup for composer and feed metadata.$menu or existing complete$skillreferences, and load timeline skills only when a sent user prompt contains a complete skill reference, including one at the end of the message; web and mobile new-worktree drafts deliberately use snapshot fallback until their cwd exists and never probe the base checkout for repo-local skills.Why
Fixes #3040.
Codex discovers repo-local skills relative to the cwd supplied to
skills/list, but a provider status snapshot is global to the provider instance. A workspace-scoped query is therefore required to show the correct skills for each project and worktree, especially while switching between workspaces or refreshing under failure.Validation
pnpm exec vp test --maxWorkers=1 apps/server/src/provider/ProviderSkillsLister.test.ts apps/server/src/provider/Layers/CodexProvider.test.ts packages/client-runtime/src/state/providerWorkspaceSkills.test.ts packages/client-runtime/src/state/runtime.test.ts apps/web/src/lib/providerWorkspaceSkillsState.test.ts apps/web/src/composer-editor-mentions.test.ts apps/web/src/components/chat/ComposerCommandMenu.test.tsx apps/web/src/components/chat/MessagesTimeline.test.tsx apps/mobile/src/features/threads/new-task-provider-skills.test.ts apps/mobile/src/features/threads/thread-composer-skill-items.test.ts(10 files, 92 tests passed)pnpm exec vp test --maxWorkers=1 apps/server/src/server.test.ts -t "server.listProviderSkills"(5 tests passed)pnpm exec vp test run --passWithNoTests apps/mobile/src/lib/nativeMarkdownText.test.ts packages/shared/src/skillInlineTokens.test.ts(native and shared inline-token coverage passed as part of the focused 4-file run)pnpm exec vp check apps/server/src/provider/Layers/CodexProvider.ts apps/server/src/provider/Layers/CodexProvider.test.ts apps/server/src/provider/ProviderSkillsLister.ts apps/server/scripts/codex-skills-mock-app-server.ts packages/client-runtime/src/state/providerWorkspaceSkills.ts packages/client-runtime/src/state/providerWorkspaceSkills.test.ts apps/web/src/lib/providerWorkspaceSkillsState.ts apps/web/src/lib/providerWorkspaceSkillsState.test.ts apps/mobile/src/state/providerWorkspaceSkillsState.ts(9 files passed formatting and lint/type-aware checks)pnpm exec vp check(0 errors; 9 existing React warnings)pnpm exec vp run typecheck(15 tasks passed)pnpm exec vp run lint:mobile(passed; SwiftLint, ktlint, and detekt were unavailable)5735and server13775: paired the isolated app, added thecodex-skillsworktree, and verified that typing$opened the skill picker with repo-scoped entries such asiOS Debugger Agentalongside system skills.pnpm exec vp test run --passWithNoTests --project unit src/components/ChatView.logic.test.ts src/components/chat/ComposerCommandMenu.test.tsx src/lib/providerWorkspaceSkillsState.test.tsfromapps/web(3 files, 47 tests passed after the future-worktree review fix)pnpm exec vp check src/components/ChatView.logic.ts src/components/ChatView.logic.test.ts src/components/ChatView.tsx src/components/chat/ChatComposer.tsxfromapps/web(4 files passed formatting and lint/type-aware checks)5735and server13775: with$open, a future-worktree draft sent 0server.listProviderSkillsframes; switching the same draft to the current checkout sent 1 request with the assigned worktree cwd.pnpm exec vp test run --passWithNoTests src/state/providerWorkspaceSkills.test.tsfrompackages/client-runtime, the two focused mobile provider-skill tests, and the focused web workspace-skill state test (4 files, 42 tests passed after the mobile connection/state review fixes).pnpm exec vp check apps/mobile/src/state/providerWorkspaceSkillsState.ts apps/mobile/src/features/threads/new-task-flow-provider.tsx apps/mobile/src/features/threads/ThreadDetailScreen.tsx packages/client-runtime/src/state/providerWorkspaceSkills.ts packages/client-runtime/src/state/providerWorkspaceSkills.test.ts(5 files passed formatting and lint/type-aware checks).5735and server13775; both browser automation profiles were locked by other sessions, and the available simulator lacked the requiredcom.t3tools.t3code.devclient, so no new runtime assertion was recorded. All owned processes were stopped and isolated state was cleaned up.pnpm exec vp test run --passWithNoTests src/provider/ProviderSkillsLister.test.tsfromapps/server(1 file, 5 tests passed after making failed cache entries immediately retryable).pnpm exec vp check src/provider/ProviderSkillsLister.ts src/provider/ProviderSkillsLister.test.tsfromapps/server(2 files passed formatting and lint/type-aware checks).pnpm exec vp test run --passWithNoTests src/features/threads/thread-provider-skills.test.ts src/features/threads/new-task-provider-skills.test.ts src/features/threads/thread-composer-skill-items.test.tsfromapps/mobile(3 files, 11 tests passed after making mobile thread-detail workspace-skill lookup lazy).pnpm exec vp check apps/mobile/src/features/threads/ThreadDetailScreen.tsx apps/mobile/src/features/threads/ThreadComposer.tsx apps/mobile/src/features/threads/thread-provider-skills.ts apps/mobile/src/features/threads/thread-provider-skills.test.ts BRANCH_DETAILS.md(5 files passed formatting; all 4 TypeScript files passed lint/type-aware checks).com.t3tools.t3code.devclient and isolated backend state on assigned server port13775, but Metro failed before application code loaded because@expo/cli@56.1.16injectedexpo/virtual/env, which is absent from the installedexpo@56.0.12package. Both standard and client-env-disabled launches reproduced the same dependency mismatch; all owned processes and the simulator were stopped, and the disposable state was moved to Trash.Proof
Note
Medium Risk
Touches WebSocket server paths, spawns short-lived Codex app-server probes per workspace, and changes composer/timeline skill resolution across web and mobile; mis-cwd or stale-cache bugs could show wrong repo skills but failures fall back to snapshots with visible errors.
Overview
Fixes repo-local Codex skills showing up incorrectly by resolving skills for the active project/worktree cwd instead of the global provider status snapshot.
Server: New
server.listProviderSkillsRPC andProviderSkillsListerrun Codexskills/listin the requested cwd (with validation, bounded coalescing/TTL on successes only, 15s timeouts, and structured errors). Non-Codex or disabled Codex providers still return snapshot skills. Codex status probes now use the configured server cwd, notprocess.cwd().Clients: Shared
providerWorkspaceSkillspolicy in client-runtime plususeProviderWorkspaceSkillson web and mobile key queries by environment, provider, and cwd. Workspace lookups run lazily (open$menu, complete$skillin draft, or sent user messages on the timeline). Future worktree drafts skip cwd probes until a directory exists. Composers show loading/errors with snapshot fallback; timeline and markdown chips use the same skill list.Shared:
@t3tools/shared/skillInlineTokenscentralizes$skillboundary rules (e.g.$HOME/ PHP$valueno longer match) across web, mobile, and native markdown.Reviewed by Cursor Bugbot for commit 9388e7f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add workspace-scoped Codex skill discovery via a new
serverListProviderSkillsRPCserver.listProviderSkillsWebSocket RPC endpoint that spawns a scoped Codex app-server probe for a specificcwd, performs an initialize/account handshake, and returns workspace-specific skills; returns an empty list when OpenAI auth is required but no account is present.parseInlineSkillTokens/hasInlineSkillTokenutilities inpackages/sharedto consistently detect inline$skillreferences across web, mobile, and server, replacing per-platform regex implementations.useProviderWorkspaceSkillshook that queries workspace skills when the prompt or timeline contains a skill reference and a connection is available, falling back to provider-snapshot skills on error or when offline.ComposerCommandMenuon web,ComposerCommandPopoveron mobile), including structuredServerProviderSkillsListErrordetail messages.resolveNextProviderWorkspaceSkillsSnapshot/resolveProviderWorkspaceSkillsutilities inpackages/client-runtimegovern snapshot retention, pending-state preservation, and fallback selection.checkCodexProviderStatusnow uses a caller-suppliedcwdinstead ofprocess.cwd(), changing behavior for any caller that previously relied on the server process working directory.Macroscope summarized 9388e7f.