Add OpenAI host support for skill discovery - #6
Conversation
- Add openai to HostId union - Add .openai/skills to project and user skill roots - Add openaiHome path helper - Test project openai skill discovery
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8435b2d3ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | "grok" | ||
| | "codex" | ||
| | "cursor" | ||
| | "openai" |
There was a problem hiding this comment.
Keep the host APIs in sync with their specs
This changes HostId and skillRoots, and src/paths.ts adds openaiHome, but neither specs/catalog/catalog.spec.md nor specs/paths/paths.spec.md is updated; both still enumerate only the old host and helper sets. The public contracts are therefore stale under the repository's strict spec-sync gate, so update both module specs in the same change.
AGENTS.md reference: AGENTS.md:L55-L58
Useful? React with 👍 / 👎.
| ); | ||
| const ctx = buildScanContext({ cwd: root, scope: "project" }); | ||
| const r = await findAssets("skills", ctx, { host: "openai" }); | ||
| expect(r.items).toHaveLength(1); |
There was a problem hiding this comment.
Isolate the OpenAI skill fixture from user catalogs
On any developer or CI account that already has entries under ~/.openai/skills, this project-scoped query includes those entries because the default configuration has include_user_skills=true; the host filter retains every OpenAI card, so this count exceeds one and the following index-based assertions may inspect a user skill instead of the fixture. Explicitly disable user-skill inclusion in this test context or select the fixture card by path or name.
Useful? React with 👍 / 👎.
| if (root.includes("/.openai/")) { | ||
| return "openai"; |
There was a problem hiding this comment.
Limit OpenAI attribution to the actual skill root
When the repository itself is anywhere beneath a .openai directory, such as ~/.openai/worktrees/repo, this ancestor-wide substring matches every project skill root, including <repo>/skills and <repo>/.agents/skills. Those generic project skills are consequently labeled openai, receive OpenAI IDs, and disappear from --host project; compare against the specific project or user OpenAI skill root rather than any .openai ancestor.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| /** OpenAI agent/CLI home. */ | ||
| export function openaiHome(): string { |
There was a problem hiding this comment.
Re-export the OpenAI home helper from the package root
The package exposes only its . entry through src/index.ts, and every existing host-home helper is re-exported there, but this new helper is not. Consumers of @corvidlabs/let therefore cannot import openaiHome even though the source declares it as exported; add it to the path export list in src/index.ts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d2b517d7b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| root === join(openaiHome(), "skills") || | ||
| (ctx.repoRoot !== null && root === join(ctx.repoRoot, ".openai", "skills")) |
There was a problem hiding this comment.
Attribute exact OpenAI roots before ancestor hosts
When a repository lives beneath a host-owned directory such as ~/.claude/worktrees/repo (and similarly beneath .grok or .cursor), its <repo>/.openai/skills root satisfies an earlier broad ancestor check and returns that ancestor host before reaching this exact OpenAI comparison. As a result, let find skills --host openai omits valid project OpenAI skills and exposes them under the wrong host; perform the exact OpenAI-root match before the broad host predicates.
Useful? React with 👍 / 👎.
| | grok | worktrees.db | yes | bundled | — | path-only | memtrace | — | — | — | | ||
| | codex | yes | optional | yes | — | path-only | sqlite/dir | yes | config path | — | | ||
| | cursor | weak | yes | yes | yes | chats path | — | — | mcp.json | plans | | ||
| | openai | — | yes | — | — | — | — | — | — | — | |
There was a problem hiding this comment.
Replace the new matrix em dashes
This newly added OpenAI row introduces em-dash characters in authored content, directly violating the repository rule requiring hyphens or colons instead; replace these placeholders with permitted characters.
AGENTS.md reference: AGENTS.md:L61-L61
Useful? React with 👍 / 👎.
Adds
.openai/skillsto the federated skill scan so that skills installed for the OpenAI provider are discoverable viafledge let find skills --host openai.\n\nChanges:\n- Addopenaito theHostIdunion.\n- Add.openai/skillsto project and user skill roots.\n- AddopenaiHome()path helper.\n- Recognize.openai/paths as hostopenaiwhen deriving host from a skill root.\n- Add a test covering project-scope OpenAI skill discovery.\n\nAll existing tests continue to pass (84 pass, 0 fail).