Add runtime-aware command discovery#818
Conversation
Introduce @optique/discover with defineCommand(), recursive command module discovery, command-tree parser composition, and runProgram() for handler dispatch. The discovery defaults are runtime-aware so Deno and Bun can load TypeScript command modules directly while Node.js keeps to JavaScript unless a TypeScript loader is active. The tests cover command definition validation, nested discovery, conflict detection, parser dispatch, root help output, and async handler execution. The temporal timezone parser now accepts curated single-segment identifiers before runtime Temporal validation so the existing cross-runtime Bun test remains stable. References #812 Assisted-by: Codex:gpt-5.5
Add the @optique/discover package README and a VitePress concept page that explains command modules, runProgram(), runtime-aware extension defaults, eager loading, path conflicts, and why the package name is @optique/discover rather than @optique/program. Expose the page in the docs navigation, add the package to the root README and agent package list, and record the release note. References #812 Assisted-by: Codex:gpt-5.5
Ignore TypeScript declaration files during command discovery so .d.ts, .d.mts, and .d.cts files are not imported when TypeScript extensions are configured. This keeps generated type output from becoming accidental command modules. Also narrow createProgramParser() root metadata to the fields it can actually place in parser documentation fragments. References #812 Assisted-by: Codex:gpt-5.5
Mirror the package deno.json permissions in the npm Deno test scripts so pnpm --filter @optique/discover test:deno and test-all can create and read the temporary command modules used by the tests. References #812 Assisted-by: Codex:gpt-5.5
Return discoverCommands() results in the documented command-path order after configured extensions are stripped. This keeps the public API stable when raw filename order differs from the derived command path. References #812 Assisted-by: Codex:gpt-5.5
Return the discovered leaf-command documentation even when root help asks for the parser in an unavailable state through the runner help wrapper. This keeps runProgram() root help from collapsing nested command trees to blank namespace entries. References #812 Assisted-by: Codex:gpt-5.5
Forward phase-two source context requests with the matched command's parsed value instead of the ProgramInvocation dispatch wrapper. This preserves config/env-style context loaders that derive paths from CLI fields while keeping runProgram() handler dispatch unchanged. References #812 Assisted-by: Codex:gpt-5.5
Include TypeScript command module extensions in Node defaults when the
runtime reports built-in TypeScript support. This keeps runProgram({ dir })
from missing .ts command files on modern Node without requiring users to
pass extensions manually.
References #812
Assisted-by: Codex:gpt-5.5
Add a cookbook recipe and runnable examples that show how @optique/discover maps command modules to command paths and dispatches parsed values to handlers. Also refresh the command discovery docs to describe Node runtimes that include TypeScript extensions through native support or a TypeScript loader. Assisted-by: Codex:gpt-5.5
Add a commands-based runProgram() path for CLIs that import their command modules explicitly. Static commands declare their command path through defineCommand(), so command trees can work with tree shaking and single-file executable builds without runtime directory discovery. Keep file discovery compatible by accepting matching declared paths and rejecting mismatches. Document the trade-offs in the tutorial, cookbook, concept guide, and package README. Assisted-by: Codex:gpt-5
|
@codex review |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #818 +/- ##
=========================================
- Coverage 95.24% 0 -95.25%
=========================================
Files 50 0 -50
Lines 30776 0 -30776
Branches 7987 0 -7987
=========================================
- Hits 29312 0 -29312
+ Misses 1401 0 -1401
+ Partials 63 0 -63 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis pull request introduces the Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/discover/src/command.ts`:
- Around line 50-57: Change the handler member from a method-shorthand to a
readonly function-typed field to enforce contravariant parameter checking:
replace the current method declaration "handler(value: T): void |
Promise<void>;" with a readonly arrow-typed field like "readonly handler:
(value: T) => void | Promise<void>;" (preserving the same return type and
generic T) so that the handler is immutable and strictFunctionTypes will
correctly validate against the parser's inferred T.
In `@packages/discover/src/index.test.ts`:
- Line 328: Replace the boolean equality assertion using
assert.equal(result.success, true) with the preferred assert.ok(result.success)
in the test; locate the failing assertion in the test function that uses the
local variable result (and the same pattern at the other occurrence), and update
both places to call assert.ok(result.success) to follow the boolean-assert
guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9452d9b6-142e-4abd-9211-cc460ec8f8df
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
AGENTS.mdCHANGES.mdREADME.mddocs/.vitepress/config.mtsdocs/concepts/discover.mddocs/cookbook.mddocs/package.jsondocs/tutorial.mdexamples/patterns/command-discovery.tsexamples/patterns/command-discovery/build.tsexamples/patterns/command-discovery/deploy.tspackages/discover/README.mdpackages/discover/deno.jsonpackages/discover/package.jsonpackages/discover/src/command.tspackages/discover/src/index.test.tspackages/discover/src/index.tspackages/discover/tsdown.config.tspackages/temporal/src/index.ts
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the @optique/discover package for runtime-aware command module discovery, including full documentation, examples, and core implementation. It also fixes a cross-runtime inconsistency in @optique/temporal regarding single-segment time zone parsing. Review feedback highlighted a potential issue in duplicate path detection where space-joined keys could lead to false collisions; using the existing null-byte-separated helper was suggested for better robustness.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd8090f281
ℹ️ 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".
Use a readonly function-typed handler field so command handlers are checked contravariantly against the parser value type. Erase handler value types only at the collection boundary, where discovered commands with different parsed values need to share one internal representation. Use the existing null-delimited command path key for duplicate detection while keeping space-joined paths in user-facing error messages. This avoids false duplicate matches when path segments themselves contain spaces. #818 (comment) #818 (comment) #818 (comment) #818 (comment) #818 (comment) Assisted-by: Codex:gpt-5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/discover/src/index.test.ts`:
- Around line 55-90: Update the tests in packages/discover/src/index.test.ts
that call defineCommand so they assert both the exact error type and exact
message string instead of using regex; for each assert.throws referencing
defineCommand (the parser, handler, and path cases), pass the Error constructor
(or an object with name/message) as the expected error and use the exact message
strings "Command parser must be an Optique parser.", "Command handler must be a
function.", and "Command path must be a non-empty array of non-empty strings."
so the assertions check precise type+message for the defineCommand validations.
In `@packages/discover/src/index.ts`:
- Around line 555-562: staticCommandsToEntries currently maps AnyStaticCommand
values into DiscoveredCommand entries without checking they satisfy the runtime
guard isCommand, allowing malformed static inputs to surface later; update
staticCommandsToEntries to run each command through the isCommand(command)
runtime check and throw or reject with a clear error (including which path or
command) when validation fails, otherwise return the mapped { path, command }
for valid items—use the existing isCommand function to perform the guard and
reference the staticCommandsToEntries function and
DiscoveredCommand/AnyStaticCommand types when implementing the check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1cf33de4-aa9d-4bbf-855f-a8733448f08f
📒 Files selected for processing (3)
packages/discover/src/command.tspackages/discover/src/index.test.tspackages/discover/src/index.ts
There was a problem hiding this comment.
Code Review
This pull request introduces the @optique/discover package, which provides runtime-aware command module discovery for building file-based CLI applications. It includes the defineCommand and runProgram APIs, supported by extensive documentation, cookbook recipes, and usage examples. Additionally, the timeZone parser in @optique/temporal was refactored to prioritize a curated allowlist for single-segment identifiers, improving cross-runtime consistency. Feedback was provided to enhance the file discovery mechanism by supporting symbolic links.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f64bcc2e7f
ℹ️ 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".
Validate static command entries with the same command guard used for file discovery, so malformed values fail with a direct diagnostic before parser construction. Unwrap CommonJS default-wrapped command exports during discovery and follow symlinked command files and directories while avoiding recursive symlink loops. Tighten defineCommand() validation tests to assert exact TypeError diagnostics. #818 (comment) #818 (comment) #818 (comment) #818 (comment) Assisted-by: Codex:gpt-5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9daaf9135
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/discover/src/index.ts`:
- Around line 276-307: discoverCommands currently eagerly imports every module
in the loop (using import(pathToFileURL(filePath).href)) which runs top-level
side effects; instead, only discover and validate the file path metadata (using
commandPathFromFile and commandPathKey) and defer importing until the chosen
command is dispatched. Concretely: in the loop inside discoverCommands() remove
the dynamic import and unwrapCommandExport usage there; keep storing { path,
filePath } (and any lightweight metadata) into discovered; update the discovered
item type to make command optional/nullable; move the module import, call to
unwrapCommandExport(mod.default), and the path-vs-declared-path consistency
check (the code that currently throws if commandDefinition.path mismatches) into
the dispatch/resolution code that runs only for the selected command (the place
that previously used discovered.command), and throw the same TypeErrors there if
the loaded module is invalid; retain commandPathKey/commandPathFromFile
uniqueness checks (seen map) as-is so duplicates are still detected without
importing modules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a5e9b32a-5174-4e3c-a771-ccd9f7068e51
📒 Files selected for processing (2)
packages/discover/src/index.test.tspackages/discover/src/index.ts
There was a problem hiding this comment.
Code Review
This pull request introduces the @optique/discover package, which enables runtime-aware command module discovery from the file system. It allows developers to structure larger CLI applications by placing each command in its own module with its own parser, metadata, and handler. The PR also includes a fix for single-segment time zone parsing in @optique/temporal to ensure cross-runtime consistency for identifiers like CET. Feedback on the implementation suggests simplifying the internal wrapping of source contexts by using direct property assignment instead of Object.defineProperty for better readability and idiomatic consistency.
Reject static command values that were created by defineCommand() but do
not declare a command path before parser construction. This keeps
runProgram({ commands }) failures in the same TypeError shape as the
other static command validation errors.
Simplify the program context wrapper by assigning optional hooks directly
on the fresh wrapper object instead of defining them through descriptors.
#818 (comment)
#818 (comment)
#818 (comment)
Assisted-by: Codex:gpt-5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the @optique/discover package, which enables runtime-aware command module discovery from the file system. It includes the core implementation for scanning directories, importing command modules, and building nested parser trees, along with comprehensive documentation, examples, and tests. Additionally, a fix was applied to @optique/temporal to ensure single-segment time zone parsing remains consistent across different runtimes by applying the curated allowlist before runtime validation. One review comment was provided regarding the potential for a TypeError when accessing process.features in environments where it might be undefined, suggesting the use of nullish coalescing for safer access.
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Treat a missing process.features object as no native TypeScript support. This keeps runtime extension detection from throwing in environments that provide only a partial node:process compatibility surface. #818 (comment) Assisted-by: Codex:gpt-5
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces the @optique/discover package, which enables runtime-aware command discovery from the file system, allowing CLI applications to be structured with one command per module. The changes include the new package implementation, comprehensive documentation, cookbook recipes, and usage examples. Additionally, the @optique/temporal package was updated to fix an issue where single-segment time zone parsing failed on runtimes that reject certain IANA links. Feedback was provided regarding the isParser utility in the new package, which incorrectly checks for the mode property instead of the $mode property required by the project's internal dispatch patterns.
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Add @optique/discover, a new package for CLIs that keep large command trees in separate command modules. Command modules use
defineCommand()to pair parser metadata with handlers, andrunProgram()builds the nested parser tree, wires help/version/completion through @optique/run, and dispatches to the selected handler.This supports both runtime file discovery and explicitly imported static commands. The static
commandspath is meant for bundlers, tree shaking, and single-file executable builds where dynamic command discovery is not a good fit.The PR also documents the package in README.md, CHANGES.md, docs/concepts/discover.md, docs/cookbook.md, docs/tutorial.md, and packages/discover/README.md, adds a command discovery example under examples/patterns/command-discovery/, and registers @optique/discover in the docs config and package metadata.
Notes
timeZone()in packages/temporal/src/index.ts now applies the curated single-segment time zone allowlist before runtime Temporal validation, so links such asCETstay accepted on runtimes whose Temporal implementation rejects them directly.Fixes #812.
Testing
deno test --allow-read --allow-write --allow-env packages/discover/src/index.test.tsmise checkcd docs && pnpm buildmise run --jobs=1 test