Skip to content

Fix workspace-scoped Codex skill discovery#3059

Open
Quicksaver wants to merge 98 commits into
pingdotgg:mainfrom
Quicksaver:fix/codex-skills
Open

Fix workspace-scoped Codex skill discovery#3059
Quicksaver wants to merge 98 commits into
pingdotgg:mainfrom
Quicksaver:fix/codex-skills

Conversation

@Quicksaver

@Quicksaver Quicksaver commented Jun 12, 2026

Copy link
Copy Markdown

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

  • Add server.listProviderSkills contracts, WebSocket and local API handlers, and environment-query wiring keyed by environment, provider instance, and cwd.
  • Add ProviderSkillsLister to return snapshot skills for non-Codex or disabled Codex providers and run Codex skills/list in the validated workspace cwd for enabled Codex providers.
  • Prepare the configured Codex home and forward configured launch arguments before each probe, and return bounded structured errors for provider lookup, settings, cwd, home-preparation, timeout, and app-server failures while projecting both Error instances and plain objects through one bounded diagnostic path.
  • Coalesce requests by provider and normalized workspace root with a short TTL cache for successful lookups only, keep failed probes immediately retryable, bound cache capacity and concurrent probes, and terminate short-lived Codex app-server processes when requests finish or time out.
  • Run Codex provider status probes with the configured server cwd instead of the backend process cwd.
  • Share workspace-skill target preparation, query-state resolution, snapshot retention, fallback selection, and error formatting across web and mobile adapters: preserve settled skills during a same-workspace refresh, clear them while switching targets or after a failed refresh, and fall back to provider snapshot skills after empty or failed lookups while keeping errors visible, without refreshing workspace probes when only client-side fallback skills change.
  • Share settings-adjusted provider-instance selection between the web composer and timeline, including enabled state, availability, provider locks, continuation groups, and deterministic fallback, so both surfaces query and render skills for the same provider when a persisted selection is no longer usable.
  • Align mobile workspace-skill lookup with environment connectivity and lazy activation: disconnected clients retain only verified same-workspace skills or provider fallback with a reconnect error, inactive same-workspace lookups preserve their verified snapshot, and pending workspace switches clear snapshots from other targets.
  • Keep mobile thread-detail lookup lazy: empty thread opens do not start workspace RPCs, while the composer $ menu, complete draft skill tokens, and sent user skill references activate the shared lookup for composer and feed metadata.
  • Load composer workspace skills only when needed for the $ menu or existing complete $skill references, 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.
  • Reuse workspace-scoped skills for web and mobile composer suggestions, inline editor chips, pending answers, and submitted user-message skill chips, with shared token-boundary handling across web and mobile rendering.
  • Add focused coverage for RPC handling, workspace validation, Codex app-server cwd and lifecycle behavior, timeout and request coalescing, query-key isolation, stale-state prevention, lazy skill references, fallback/error rendering, search ranking, and timeline chips.

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)
  • Playwright smoke against web 5735 and server 13775: paired the isolated app, added the codex-skills worktree, and verified that typing $ opened the skill picker with repo-scoped entries such as iOS Debugger Agent alongside 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.ts from apps/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.tsx from apps/web (4 files passed formatting and lint/type-aware checks)
  • Playwright follow-up against web 5735 and server 13775: with $ open, a future-worktree draft sent 0 server.listProviderSkills frames; 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.ts from packages/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).
  • Integrated follow-up used isolated state on assigned web 5735 and server 13775; both browser automation profiles were locked by other sessions, and the available simulator lacked the required com.t3tools.t3code.dev client, 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.ts from apps/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.ts from apps/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.ts from apps/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).
  • Integrated iOS verification used an installed com.t3tools.t3code.dev client and isolated backend state on assigned server port 13775, but Metro failed before application code loaded because @expo/cli@56.1.16 injected expo/virtual/env, which is absent from the installed expo@56.0.12 package. 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.
  • pnpm exec vp test run --passWithNoTests src/provider/ProviderSkillsLister.test.ts src/diagnostics/ErrorCause.test.ts src/workspace/WorkspacePaths.test.ts from apps/server (3 files, 16 tests passed after normalizing provider-skill cache keys and removing the dead sanitizer branch).
  • Focused client-runtime, web, and mobile provider-skill suites passed 48 tests across 5 files after centralizing the shared query policy.
  • pnpm exec vp check BRANCH_DETAILS.md apps/mobile/src/state/providerWorkspaceSkillsState.ts apps/server/src/diagnostics/ErrorCause.ts apps/server/src/diagnostics/ErrorCause.test.ts apps/server/src/provider/ProviderSkillsLister.ts apps/server/src/provider/ProviderSkillsLister.test.ts apps/server/src/workspace/WorkspacePaths.ts apps/web/src/lib/providerWorkspaceSkillsState.ts packages/client-runtime/src/state/providerWorkspaceSkills.ts packages/client-runtime/src/state/providerWorkspaceSkills.test.ts (all 10 files passed formatting; all 9 TypeScript files passed lint/type-aware checks).
  • Integrated browser follow-up used isolated state on assigned web 5735 and server 13775. Playwright's shared profile was locked before navigation, so the controlled browser fallback paired the app, added the codex-skills checkout, refreshed an initially timed-out Codex provider, selected Codex, and verified that $ displayed all 4 repo-local skills alongside personal and system skills. Both development processes were stopped and the disposable state was removed.
  • pnpm exec vp test run --passWithNoTests --project unit src/providerInstances.test.ts src/components/ChatView.logic.test.ts from apps/web (2 files, 66 tests passed after sharing provider-instance fallback selection).
  • pnpm exec vp check BRANCH_DETAILS.md apps/web/src/providerInstances.ts apps/web/src/providerInstances.test.ts apps/web/src/components/ChatView.tsx apps/web/src/components/chat/ChatComposer.tsx (all 5 files passed formatting; all 4 TypeScript files passed lint/type-aware checks).
  • Integrated browser verification used isolated state on assigned web 5735 and server 13775; after selecting Codex in the codex-skills checkout, the $ menu displayed all 4 repo-local skills alongside personal and system skills. The owned development processes were stopped after verification.

Proof

  • No external proof artifacts; validation is covered by the automated checks above.

Note

Medium Risk
Touches WebSocket RPC, Codex child-process probes, and cross-platform skill state; incorrect cwd or cache behavior could show wrong repo skills or stale chips, but fallbacks and bounded errors limit blast radius.

Overview
Fixes repo-local Codex skill discovery by resolving skills for the active project/worktree cwd instead of the global provider status snapshot.

Server: New server.listProviderSkills RPC routes through ProviderSkillsLister, which returns snapshot skills for non-Codex or disabled Codex providers and runs a bounded, coalesced Codex app-server skills/list probe for enabled Codex (validated cwd, 15s timeout, short TTL on successes only). Failures return structured ServerProviderSkillsListError with sanitized causes. Codex status checks now take an explicit server cwd rather than process.cwd().

Clients: Shared packages/client-runtime policy (prepareProviderWorkspaceSkillsTarget, resolveProviderWorkspaceSkillsQuery) drives web and mobile hooks: query keyed by environment + instance + cwd, preserve skills on same-workspace refresh, clear on switch/failed refresh, fall back to provider snapshot when empty/offline/error, and show reconnect/errors in the skill menu. Lazy activation — workspace RPC runs when the $ menu is open or the draft/timeline has a complete $skill token (not on every mount). Future worktree drafts skip cwd probes until a directory exists.

UI: Composer and timeline use workspace skills for picker, inline chips, and sent-message decoration; web/mobile share parseInlineSkillTokens and resolveProviderInstanceSelection so composer and timeline target the same provider instance. Skill load errors render as alerts alongside fallback skills.

Reviewed by Cursor Bugbot for commit 7e6406a. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix workspace-scoped Codex skill discovery in composer and message timeline

  • Adds a server.listProviderSkills WebSocket RPC backed by a new makeProviderSkillsLister that spawns a Codex app-server process scoped to the active workspace cwd, with a bounded coalescing cache and structured error types.
  • Introduces useProviderWorkspaceSkills hooks (web and mobile) that gate queries on connection availability and fall back to snapshot skills; the web ChatComposer and mobile ThreadComposer now load workspace skills when the user types a skill trigger or the prompt/timeline contains a skill reference.
  • Extracts shared skill token parsing into parseInlineSkillTokens / hasInlineSkillToken in @t3tools/shared and updates all consumers (SkillInlineText, native markdown renderer, composer mention detection) to use the canonical parser, changing boundary detection semantics for skill decorations.
  • ComposerCommandMenu (web) and ComposerCommandPopover (mobile) now show a typed error banner and suppress empty-state copy when a workspace skill lookup fails.
  • checkCodexProviderStatus now accepts an explicit cwd argument instead of using process.cwd().
  • Risk: skill token boundary rules changed across web and mobile renderers — tokens adjacent to code-variable-like characters are no longer decorated.

Macroscope summarized 7e6406a.

- 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
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c9747c38-bbdd-4f9a-8652-1064c9852c31

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jun 12, 2026
Comment thread apps/web/src/lib/providerWorkspaceSkillsState.ts Outdated
Comment thread apps/web/src/components/chat/ChatComposer.tsx
Comment thread apps/server/src/ws.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR introduces a substantial new feature for workspace-scoped skill discovery, including a new RPC endpoint, server-side bounded request caching, and client state management across web and mobile platforms. The scope and cross-cutting nature of these changes warrant 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 github-actions Bot added size:XL 500-999 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Jun 12, 2026
# Conflicts:
#	apps/server/src/provider/Layers/CursorProvider.ts
#	apps/web/src/components/chat/ChatComposer.tsx
@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:XL 500-999 changed lines (additions + deletions). labels Jun 19, 2026
Quicksaver and others added 2 commits June 19, 2026 14:07
# 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>
Comment thread apps/web/src/lib/providerWorkspaceSkillsState.ts Outdated
- 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 github-actions Bot added size:XL 500-999 changed lines (additions + deletions). and removed size:L 100-499 changed lines (additions + deletions). labels Jun 19, 2026
- 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
Comment thread apps/mobile/src/state/providerWorkspaceSkillsState.ts Outdated
Comment thread apps/mobile/src/state/providerWorkspaceSkillsState.ts Outdated
Comment thread packages/client-runtime/src/state/providerWorkspaceSkills.ts Outdated
- Follow mobile environment connectivity and lazy lookup state
- Clear retained skill snapshots across workspace switches
Comment thread BRANCH_DETAILS.md
Comment thread apps/server/src/provider/ProviderSkillsLister.ts
- 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
Comment thread apps/mobile/src/features/threads/ThreadDetailScreen.tsx
# 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
Comment thread apps/server/src/provider/ProviderSkillsLister.ts
Comment thread apps/web/src/lib/providerWorkspaceSkillsState.ts
Comment thread apps/server/src/diagnostics/ErrorCause.ts Outdated
- Normalize cache keys before bounded provider-skill lookups
- Share workspace query policy across web and mobile adapters
- Remove dead sanitizer logic and test bounded error projection
Comment thread apps/web/src/components/ChatView.tsx
- Share enabled and available provider-instance resolution
- Keep timeline workspace skills on the composer's fallback instance
- Cover disabled and unavailable persisted selections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Repo-local Codex skills do not appear in T3 Code $ skill picker

3 participants