-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Expose project provider skills in chat #3982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1918ac0
ea1eacd
0e500da
16a31f1
ecda648
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -36,11 +36,15 @@ import { ServerConfig } from "../../config.ts"; | |||||||||||||||||||||||||||||||||||
| import { ServerSettingsService } from "../../serverSettings.ts"; | ||||||||||||||||||||||||||||||||||||
| import { ProviderDriverError } from "../Errors.ts"; | ||||||||||||||||||||||||||||||||||||
| import { makeCodexAdapter } from "../Layers/CodexAdapter.ts"; | ||||||||||||||||||||||||||||||||||||
| import { checkCodexProviderStatus, makePendingCodexProvider } from "../Layers/CodexProvider.ts"; | ||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||
| checkCodexProviderStatus, | ||||||||||||||||||||||||||||||||||||
| makePendingCodexProvider, | ||||||||||||||||||||||||||||||||||||
| probeCodexAppServerProvider, | ||||||||||||||||||||||||||||||||||||
| } from "../Layers/CodexProvider.ts"; | ||||||||||||||||||||||||||||||||||||
| import { ProviderEventLoggers } from "../Layers/ProviderEventLoggers.ts"; | ||||||||||||||||||||||||||||||||||||
| import { makeManagedServerProvider } from "../makeManagedServerProvider.ts"; | ||||||||||||||||||||||||||||||||||||
| import type { ProviderDriver, ProviderInstance } from "../ProviderDriver.ts"; | ||||||||||||||||||||||||||||||||||||
| import type { ServerProviderDraft } from "../providerSnapshot.ts"; | ||||||||||||||||||||||||||||||||||||
| import { AUTH_PROBE_TIMEOUT_MS, type ServerProviderDraft } from "../providerSnapshot.ts"; | ||||||||||||||||||||||||||||||||||||
| import { mergeProviderInstanceEnvironment } from "../ProviderInstanceEnvironment.ts"; | ||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||
| enrichProviderSnapshotWithVersionAdvisory, | ||||||||||||||||||||||||||||||||||||
|
|
@@ -161,6 +165,42 @@ export const CodexDriver: ProviderDriver<CodexSettings, CodexDriverEnv> = { | |||||||||||||||||||||||||||||||||||
| ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| const textGeneration = yield* makeCodexTextGeneration(effectiveConfig, processEnv); | ||||||||||||||||||||||||||||||||||||
| const listSkills = Effect.fn("CodexDriver.listSkills")(function* (cwd: string) { | ||||||||||||||||||||||||||||||||||||
| if (!effectiveConfig.enabled) { | ||||||||||||||||||||||||||||||||||||
| return []; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return yield* probeCodexAppServerProvider({ | ||||||||||||||||||||||||||||||||||||
| binaryPath: effectiveConfig.binaryPath, | ||||||||||||||||||||||||||||||||||||
| homePath: effectiveConfig.homePath, | ||||||||||||||||||||||||||||||||||||
| cwd, | ||||||||||||||||||||||||||||||||||||
| customModels: [], | ||||||||||||||||||||||||||||||||||||
| environment: processEnv, | ||||||||||||||||||||||||||||||||||||
| includeModels: false, | ||||||||||||||||||||||||||||||||||||
| }).pipe( | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+173
to
+180
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium
Suggested change
🤖 Copy this AI Prompt to have your agent fix this:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Codex skills omit launch argsMedium Severity The Reviewed by Cursor Bugbot for commit ecda648. Configure here. |
||||||||||||||||||||||||||||||||||||
| Effect.scoped, | ||||||||||||||||||||||||||||||||||||
| Effect.map((result) => result.skills), | ||||||||||||||||||||||||||||||||||||
| Effect.mapError( | ||||||||||||||||||||||||||||||||||||
| (cause) => | ||||||||||||||||||||||||||||||||||||
| new ProviderDriverError({ | ||||||||||||||||||||||||||||||||||||
| driver: DRIVER_KIND, | ||||||||||||||||||||||||||||||||||||
| instanceId, | ||||||||||||||||||||||||||||||||||||
| detail: `Failed to list Codex skills: ${cause.message}`, | ||||||||||||||||||||||||||||||||||||
| cause, | ||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||
| Effect.timeoutOrElse({ | ||||||||||||||||||||||||||||||||||||
| duration: Duration.millis(AUTH_PROBE_TIMEOUT_MS), | ||||||||||||||||||||||||||||||||||||
| orElse: () => | ||||||||||||||||||||||||||||||||||||
| new ProviderDriverError({ | ||||||||||||||||||||||||||||||||||||
| driver: DRIVER_KIND, | ||||||||||||||||||||||||||||||||||||
| instanceId, | ||||||||||||||||||||||||||||||||||||
| detail: `Timed out listing Codex skills after ${AUTH_PROBE_TIMEOUT_MS}ms`, | ||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||
| Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Build a managed snapshot whose settings never change — mutations come | ||||||||||||||||||||||||||||||||||||
| // in as instance rebuilds from the registry rather than in-place | ||||||||||||||||||||||||||||||||||||
|
|
@@ -209,6 +249,7 @@ export const CodexDriver: ProviderDriver<CodexSettings, CodexDriverEnv> = { | |||||||||||||||||||||||||||||||||||
| snapshot, | ||||||||||||||||||||||||||||||||||||
| adapter, | ||||||||||||||||||||||||||||||||||||
| textGeneration, | ||||||||||||||||||||||||||||||||||||
| listSkills, | ||||||||||||||||||||||||||||||||||||
| } satisfies ProviderInstance; | ||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a Codex instance is configured with
launchArgsorT3CODE_CODEX_LAUNCH_ARGS(for example--strict-config,--config, or feature flags), status checks and real sessions resolve and pass those args, but this newserver.listProviderSkillspath starts a plain app-server without them. In that configuration the$menu can discover a different skill set or fail even though the provider itself works with the configured app-server; pass the resolved launch args intoprobeCodexAppServerProviderhere as the other Codex app-server callers do.Useful? React with 👍 / 👎.