Skip to content

[codex] Fix Codex workspace skill autocomplete#2954

Closed
PabloSzx wants to merge 1 commit into
pingdotgg:mainfrom
PabloSzx:codex/workspace-skill-autocomplete-only
Closed

[codex] Fix Codex workspace skill autocomplete#2954
PabloSzx wants to merge 1 commit into
pingdotgg:mainfrom
PabloSzx:codex/workspace-skill-autocomplete-only

Conversation

@PabloSzx

@PabloSzx PabloSzx commented Jun 4, 2026

Copy link
Copy Markdown

Summary

Fix Codex $skill autocomplete for project/worktree-local skills.

Root cause

Codex provider status was probed from the server process cwd, so codex app-server skills/list did not receive the active project or worktree cwd. Project-local .agents/skills entries could be parsed by Codex, but T3 Code's provider snapshot did not include them for the active workspace, leaving $create-st and similar skill queries with no matches.

Changes

  • Thread optional cwd context through server.refreshProviders, the WebSocket contract, provider registry refreshes, and managed provider snapshots.
  • Make managed providers remember an explicit refresh context so later background refreshes keep using the active workspace cwd.
  • Probe Codex provider status with the active project/worktree cwd, falling back to the configured server cwd.
  • Refresh the active Codex provider when the active workspace root changes so the existing $ skill autocomplete has the right skill list.
  • Add regression coverage for Codex cwd forwarding, registry refresh context, managed-provider context reuse, and local API forwarding.

Related

Related to #2637, but this PR intentionally does not change / command suggestions or close that issue.
Related to #2048, but this PR only wires the cwd consumer for Codex; Claude project skill discovery remains separate.
Builds on the closed stale approach from #2330, with the cwd coming from the active workspace rather than only the server config cwd.
Supersedes draft #2953 with a narrower $ autocomplete-only scope.

Validation

  • mise exec -- pnpm exec vp check
  • mise exec -- pnpm exec vp test run apps/server/src/provider/makeManagedServerProvider.test.ts apps/server/src/provider/Layers/ProviderRegistry.test.ts apps/web/src/localApi.test.ts
  • mise exec -- pnpm exec vp run --filter t3 --filter @t3tools/web --filter @t3tools/contracts typecheck

Note: full mise exec -- pnpm exec vp run typecheck currently fails on fresh origin/main in unrelated apps/desktop Effect diagnostics (effect/Scope resolution and any-in-requirements errors), before any changed package fails.

Note

Fix Codex workspace skill autocomplete by passing cwd through provider refresh

  • Adds an optional cwd field to the ProviderSnapshotRefreshInput type and threads it through the refresh call chain: RPC schema → ProviderRegistrymakeManagedServerProvidercheckCodexProviderStatus.
  • makeManagedServerProvider now accepts checkProvider as a function (instead of a plain Effect) and persists the last explicit refresh input so background rechecks reuse the same cwd.
  • ChatView gains a useEffect that triggers a one-time Codex provider refresh with the active workspace cwd whenever the active provider is Codex and a workspace root is set, ensuring project-local skills are resolved correctly.
  • Behavioral Change: ServerProviderShape.refresh changes from a plain Effect to a function accepting optional input; all driver and test call sites are updated accordingly.
📊 Macroscope summarized 957ecf6. 14 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

@coderabbitai

coderabbitai Bot commented Jun 4, 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

Run ID: aa59eb4e-4dcc-4615-a92b-0ef0a3bc7670

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 and usage tips.

@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 4, 2026
@PabloSzx

PabloSzx commented Jun 4, 2026

Copy link
Copy Markdown
Author

Superseded by #2955, which keeps the same scoped Codex skill autocomplete fix on the requested pabloszx/ branch prefix.

@PabloSzx PabloSzx closed this Jun 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low

const checkProvider = checkOpenCodeProviderStatus(

The checkProvider function at line 152 ignores the input parameter, so any cwd override passed during refresh is discarded. The interface expects checkProvider: (input?: ProviderSnapshotRefreshInput) => Effect.Effect<...> where input?.cwd can override the working directory, but the current implementation captures serverConfig.cwd at instance creation time and discards runtime cwd values. This breaks the refresh contract when a different working directory is requested.

🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Drivers/OpenCodeDriver.ts around line 139:

The `checkProvider` function at line 152 ignores the `input` parameter, so any `cwd` override passed during refresh is discarded. The interface expects `checkProvider: (input?: ProviderSnapshotRefreshInput) => Effect.Effect<...>` where `input?.cwd` can override the working directory, but the current implementation captures `serverConfig.cwd` at instance creation time and discards runtime `cwd` values. This breaks the refresh contract when a different working directory is requested.

Evidence trail:
- apps/server/src/provider/Drivers/OpenCodeDriver.ts lines 139-152 (REVIEWED_COMMIT): `checkProvider` pre-built with `serverConfig.cwd`, then passed as `() => checkProvider` ignoring input
- apps/server/src/provider/makeManagedServerProvider.ts lines 31-33 (REVIEWED_COMMIT): interface expects `checkProvider: (input?: ProviderSnapshotRefreshInput) => Effect.Effect<...>`
- apps/server/src/provider/makeManagedServerProvider.ts lines 130-131 (REVIEWED_COMMIT): framework passes resolved `refreshInput` to `input.checkProvider(refreshInput)`
- apps/server/src/provider/Services/ServerProvider.ts lines 6-8 (REVIEWED_COMMIT): `ProviderSnapshotRefreshInput` defines `readonly cwd?: string | undefined`
- apps/server/src/provider/Drivers/CodexDriver.ts lines 163-168 (REVIEWED_COMMIT): sibling driver correctly accepts `refreshInput?.cwd ?? serverConfig.cwd`
- apps/server/src/provider/Layers/OpenCodeProvider.ts lines 301-304 (REVIEWED_COMMIT): `checkOpenCodeProviderStatus` takes `cwd` as a parameter, so dynamic values are supported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 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.

1 participant