fix(server): discover project skills for Codex and OpenCode - #8778

💭 I'm reviewing the changed files—ProviderRegistry, CodexProvider, opencodeRuntime, and ProviderDriver—for adherence to Effect service conventions. I notice OpenCodeRuntimeShape is a pre-existing standalone Shape interface that technically violates the "define interface inline" convention, but since it predates this PR and the changes just add methods to it, I'm treating it as legacy rather than a new violation. Similarly, I'm checking ProviderRegistryShape and the new error constructions—one embeds a cwd path in the detail string, another embeds an exit code rather than using a structured field, and neither includes a cause where relevant. I want to compare these against existing patterns in the actual files before flagging them as issues.

1. git_tools — runOpenCodeCommand|OPENCODE_SKILL_DISCOVERY_MAX_OUTPUT_BYTES|parseSkillsCliOutput|loadSkillsFromCli|OpenCodeRuntimeError

2. browse_code — apps/server/src/provider/Layers/CodexProvider.ts

💭 I'm noticing that probeCodexSkillsForCwd duplicates the setup from probeCodexAppServerProvider, which is worth flagging even though it's not strictly an Effect service convention violation. I want to check whether buildCodexInitializeParams actually exists in the file, since the new function uses it while the older code just inlines the params directly. Going through the remaining rules — imports, service definitions, dependency boundaries, errors, file layout — I'm scanning the diff-added code against each. ProviderRegistryShape is a pre-existing standalone shape interface with just one new method, so per the "no unrelated cleanup" rule I'll leave that alone. Imports across the modified test and driver files look correct, and ChatComposer.tsx is just client React code unrelated to Effect services. In CodexDriver.ts, I'm checking whether probeCodexSkillsForCwd properly provides the ChildProcessSpawner service from a captured spawner instance, confirming that's a valid dependency-provision pattern rather than reaching into environment state improperly.

3. browse_code — apps/server/src/provider/Drivers/CodexDriver.ts

4. git_tools — buildCodexInitializeParams

💭 This matches an existing pattern where the spawner comes from the environment via provideService. Checking the error details in snapshotForCwd, the path context in the message is acceptable since it's not derived from the cause. For OpenCode, the error correctly omits a cause field since a nonzero exit code isn't an underlying error itself. The exit code is embedded in the message string rather than a structured field — I want to compare this against similar spots in the file to see if that's the established convention.

5. browse_code — apps/server/src/provider/opencodeRuntime.ts

6. browse_code — apps/server/src/provider/opencodeRuntime.ts

💭 I'm noting the new code follows existing patterns, using the legacy Data.TaggedError style rather than Schema.TaggedErrorClass, which is pre-existing in this file. Checking ProviderRegistry.ts's service definition, I see the standalone OpenCodeRuntimeShape interface is also a repo-wide legacy pattern that this PR just extends rather than introduces, so the "no standalone Shape interface" rule doesn't really apply here. The same logic applies to ProviderRegistryShape—the PR just adds a method to an existing shape, and the pre-existing split between Services/ProviderRegistry.ts and Layers/ProviderRegistry.ts shouldn't be flagged since demanding a merge would be unrelated repository-wide cleanup rather than something tied to this change. I'm also checking ChatComposer.tsx's hook usage, which looks fine within React boundaries, and confirming that forkDetach usage in ProviderCommandReactor isn't a convention violation. One thing worth looking closer at is how refreshWorkspaceSnapshot is constructed as an Effect value once and reused—I want to check whether that reuse pattern is safe. I'm digging into whether both usage paths actually re-run the effect concurrently, since that's more of a runtime logic risk than a convention violation, and checking that ProviderRegistry.ts declares an error-free effect type while its implementation handles failures through a catchCause recovery step. I'm also reviewing how the workspace refreshes are tracked with the Ref map and confirming the ws.ts changes look clean, with no unexpected Layer.succeed or ManagedRuntime additions in the diff. Now I'm scanning the changed files for direct imports from "effect" to make sure ChatComposer and providerSkills.ts only pull type-only imports where appropriate.

7. git_tools — from "effect";

8. browse_code — apps/server/src/provider/Services/ProviderRegistry.ts

💭 I notice the new method is missing a doc comment, which is just a style nitpick given this is a pre-existing pattern. Now I want to check the ProviderRegistry.ts implementation around the refreshWorkspaceSnapshot region to see how it handles conventions like Effect.fn and catchTag versus catchTags, plus review the new test additions since this is a behavior change that genuinely warrants test coverage.

9. browse_code — apps/server/src/provider/Layers/ProviderRegistry.ts

10. browse_code — apps/server/src/provider/Layers/ProviderRegistry.ts

💭 This all looks consistent with existing conventions—the deduplication using the ProviderInstance object as a map key is fine, and no new error classes were introduced, just a static detail message with cwd that doesn't violate the "no copying cause.message" rule. Now I'm checking the ProviderCommandReactor change. I want to verify Effect.forkDetach actually exists as a real API in this codebase's Effect version rather than assuming it. The ChatComposer's second useEffect has a dependency array issue with selectedProviderStatus, but that's a React hooks lint concern rather than an Effect service convention violation, so it's out of scope for this particular check.

11. git_tools — forkDetach

12. browse_code — apps/server/src/orchestration/Layers/ProviderCommandReactor.ts

💭 The client-runtime provider skills helpers and the contracts RPC schema addition both look fine. I'm now checking whether the OpenCodeDriver's loadSkillsForCwd has any convention issues, looking at how it uses the openCodeRuntime service.

13. browse_code — apps/server/src/provider/Drivers/OpenCodeDriver.ts

💭 I'm checking that openCodeRuntime is captured properly and reused consistently across service calls, similar to the existing checkProvider pattern. Everything looks consistent so far—no clear convention violations, just one borderline case in the provider directory worth double-checking. Since I don't have any findings, I'm reconciling the schema requirements with the "All clear" instruction—setting the summary field to exactly "All clear" while keeping the response minimal and valid JSON.