Update @fedify/cli for Optique 1.0.0#677
Merged
dahlia merged 6 commits intofedify-dev:mainfrom Apr 15, 2026
Merged
Conversation
Upgrade the shared Optique packages to 1.0.0 so fedify can use the stable runner and config-context APIs. This switches the CLI entrypoint from runWithConfig() to run() with contextOptions, updates parser tests for the new config annotation flow, and keeps help entrypoints runnable by removing an outdated @fxts/core helper from @fedify/init. Assisted-by: OpenCode:gpt-5.4
|
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:
📝 WalkthroughWalkthroughReplaces in-module config wiring with a new CLI runner exporting Changes
Sequence Diagram(s)sequenceDiagram
participant User as User CLI
participant Runner as CLI Runner
participant Config as Config Loader
participant Parser as Option Parser
participant CmdExec as Command Executor
rect rgba(200, 150, 255, 0.5)
Note over User,CmdExec: Old Flow (pre-runner)
User->>Runner: start (old entry)
Runner->>Config: setActiveConfig()/global config
Runner->>Parser: parse(args) [reads global active config]
Parser->>CmdExec: return parsed command
Runner->>CmdExec: runWithConfig(command, configContext, ...)
Runner->>Config: clearActiveConfig()
end
rect rgba(150, 200, 255, 0.5)
Note over User,CmdExec: New Flow (runner-based)
User->>Runner: runCli(args)
Runner->>Parser: parse(parser, args, { annotations })
Runner->>Config: loadConfig(parsed) (system/user/project/custom)
Config-->>Runner: { config, meta } | undefined
Runner->>CmdExec: dispatch and execute selected command
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
issues-auto-labeler
Bot
added
breaking change
dahlia
added
dependencies
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the @optique suite of dependencies to version 1.0.0 and refactors the CLI and initialization logic to align with the new APIs. Key changes include replacing runWithConfig with the new run function in the main entry point, updating test helpers to use a custom parseWithConfig utility instead of global configuration state, and refactoring functional piping in the project initializer for better clarity. Feedback was provided regarding the parseWithConfig test helper, which relies on internal execution phases and manual type casting that may be brittle.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/lookup.test.ts (1)
2-11:⚠️ Potential issue | 🟡 MinorSwitch this test file to
@fedify/fixture.It still uses
node:test, which skips the repository’s runtime-agnostic wrapper.As per coding guidelines,
**/*.test.{ts,tsx}: Importtestfrom@fedify/fixturefor runtime-agnostic tests instead of using environment-specific test APIs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/lookup.test.ts` around lines 2 - 11, The test file imports the test runner from node:test which bypasses the repo’s runtime-agnostic wrapper; replace the import "test from 'node:test'" with importing test from "@fedify/fixture" (i.e., update the import statement that provides the test symbol) so all tests use the repository’s fixture wrapper and remain runtime-agnostic; ensure any usages of the test symbol and related test utilities remain unchanged after switching the import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/lookup.test.ts`:
- Around line 33-46: The test helper parseWithConfig bypasses the real context
loading, so add at least one runner-level regression test that calls the CLI
entrypoint run(...) (the runner) and exercises --config and --ignore-config
paths using real contextOptions.load behavior; specifically add a test that
invokes run with args including --config and another with --ignore-config,
stubbing or creating a temporary config file and asserting behavior differs as
expected, ensuring you reference and cover the run function in
packages/cli/src/mod.ts and avoid using parseWithConfig for these cases so the
contextOptions.load path is exercised.
In `@packages/cli/src/tunnel.test.ts`:
- Around line 2-4: Replace the environment-specific test import with the repo's
runtime-agnostic fixture: remove "import test from 'node:test'" and import the
test entrypoint from "@fedify/fixture" instead (so existing uses of the test
function in this file—e.g., tests that call test(...)—continue to work); keep
the other imports (runSync, deepEqual, rejects) unchanged and verify any test
options/signatures align with the `@fedify/fixture` test API.
In `@packages/init/src/action/configs.ts`:
- Around line 74-83: The needsUnstableTemporal() logic is inverted: it currently
returns true for Deno >= 2.7.0 but should return true only for Deno < 2.7.0;
update the function so when parseVersion yields an array it uses an "earlier
than" check (or negates isLaterOrEqualThan(TEMPORAL_STABLE_FROM)) to return true
for versions before TEMPORAL_STABLE_FROM, and when version is not an array
return false (omit unstable.temporal). Keep references to
getDenoVersionFromRuntime, getDenoVersionFromCommand, parseVersion and
TEMPORAL_STABLE_FROM when making the change.
---
Outside diff comments:
In `@packages/cli/src/lookup.test.ts`:
- Around line 2-11: The test file imports the test runner from node:test which
bypasses the repo’s runtime-agnostic wrapper; replace the import "test from
'node:test'" with importing test from "@fedify/fixture" (i.e., update the import
statement that provides the test symbol) so all tests use the repository’s
fixture wrapper and remain runtime-agnostic; ensure any usages of the test
symbol and related test utilities remain unchanged after switching the import.
🪄 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: 9e13929a-2ffc-4598-a682-241b3e2efaa5
⛔ Files ignored due to path filters (2)
deno.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
deno.jsonpackages/cli/src/lookup.test.tspackages/cli/src/lookup.tspackages/cli/src/mod.tspackages/cli/src/options.tspackages/cli/src/tunnel.test.tspackages/cli/src/webfinger/mod.test.tspackages/init/src/action/configs.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- packages/cli/src/webfinger/mod.test.ts
Codecov Report❌ Patch coverage is
... and 26 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Add runner-level CLI tests for --config and --ignore-config so the Optique 1.0 load path is exercised instead of only parser annotations. This also fixes the Deno Temporal version check in @fedify/init and adds regression tests for versions before and after Deno 2.7.0. Assisted-by: Codex:gpt-5.4
Member
Author
|
For the outside-diff suggestion in CodeRabbit's review summary about switching packages/cli/src/lookup.test.ts to @fedify/fixture, I kept node:test there as well. The CLI package tests are intentionally Node-specific, the same as in the tunnel test thread. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/mod.ts (1)
21-24:⚠️ Potential issue | 🟠 MajorMissing
awaiton async command handlers.Both
runInboxandrunNodeInfoare async functions (perpackages/cli/src/nodeinfo.ts:109-111), but they're called withoutawait. This means:
main()completes before these commands finish executing- Any errors thrown will result in unhandled promise rejections
- The process exit code won't reflect command failures
🔧 Proposed fix
} else if (result.command === "inbox") { - runInbox(result); + await runInbox(result); } else if (result.command === "nodeinfo") { - runNodeInfo(result); + await runNodeInfo(result); } else if (result.command === "tunnel") {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/mod.ts` around lines 21 - 24, The branches calling the async command handlers call runInbox(result) and runNodeInfo(result) without awaiting them, causing main to exit early and unhandled promise rejections; change those calls in the command dispatch to await runInbox(result) and await runNodeInfo(result) so main waits for completion and errors propagate correctly (identify the calls to runInbox and runNodeInfo in the dispatch block and prepend await).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/runner.ts`:
- Around line 122-149: Add a JSDoc comment for the exported function runCli
describing its purpose as the main CLI entry point, document the args parameter
(string[]), and note the return value (the result of run(...), e.g., a Command
runner or Promise if applicable). Place the JSDoc immediately above the runCli
declaration and include short descriptions for the function, the args parameter,
and the return type so callers and generated docs can understand its usage.
In `@packages/init/src/action/configs.test.ts`:
- Around line 1-2: Replace the runtime-specific import of the test runner:
instead of importing test from "node:test" at the top of the file, import test
from "@fedify/fixture" so the test uses the repository's runtime-agnostic
harness; update the import statement that currently references "node:test" to
reference "@fedify/fixture" (leave the assert import and existing test usages
like test(...) unchanged).
---
Outside diff comments:
In `@packages/cli/src/mod.ts`:
- Around line 21-24: The branches calling the async command handlers call
runInbox(result) and runNodeInfo(result) without awaiting them, causing main to
exit early and unhandled promise rejections; change those calls in the command
dispatch to await runInbox(result) and await runNodeInfo(result) so main waits
for completion and errors propagate correctly (identify the calls to runInbox
and runNodeInfo in the dispatch block and prepend await).
🪄 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: 985484aa-ca08-4ea8-93d2-9a5de5871a3c
📒 Files selected for processing (5)
packages/cli/src/lookup.test.tspackages/cli/src/mod.tspackages/cli/src/runner.tspackages/init/src/action/configs.test.tspackages/init/src/action/configs.ts
Await the inbox and nodeinfo handlers so the CLI does not exit before those commands finish and still reports failures through the process exit status. This also documents the shared runCli() helper now that it acts as the entrypoint for the Optique 1.0 runner setup. Assisted-by: Codex:gpt-5.4
Member
Author
|
For CodeRabbit's outside-diff note about missing await in packages/cli/src/mod.ts, I fixed that in 4b28b9a by awaiting both runInbox() and runNodeInfo(). |
Split the inbox and relay command parsers out of the runtime-heavy modules so source-level CLI tests do not import .tsx or node:sqlite under Node and Bun. This also rewrites the Deno version regression test to replace the global safely and raises the Bun timeout for vocab-tools so mise test stays green again. Assisted-by: OpenCode:gpt-5.4
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/inbox/command.ts`:
- Around line 89-91: Update the CLI command help description string (the
description property in the inbox command definition) to correct the grammar:
replace "an one-time actor" with "a one-time actor" so the user-facing message
reads "Spins up an ephemeral server that serves the ActivityPub inbox with a
one-time actor, through a short-lived public DNS with HTTPS. You can monitor the
incoming activities in real-time."
In `@packages/init/src/action/configs.test.ts`:
- Around line 6-43: The test helper createInitData currently uses a double type
cast to InitCommandData which bypasses type safety; replace it by building a
Partial<InitCommandData> (or implement a small builder function like
buildInitCommandData that accepts overrides) and populate all required fields
with sensible defaults (including initializer, kv, mq, env, projectDir, dir,
etc.), then return the constructed object as InitCommandData or merge overrides
into the defaults—this preserves compile-time checks and prevents future silent
breaks in createInitData/InitCommandData usage.
🪄 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: c9f392d9-cf1a-4f48-bede-facd2d4e5c77
📒 Files selected for processing (7)
packages/cli/src/inbox.tsxpackages/cli/src/inbox/command.tspackages/cli/src/relay.tspackages/cli/src/relay/command.tspackages/cli/src/runner.tspackages/init/src/action/configs.test.tspackages/vocab-tools/package.json
Fix the inbox help text typo so the extracted command definition keeps its user-facing wording clean. This also replaces the configs test's double-cast fixture with a real InitCommandData object, so changes to that type keep surfacing as compile-time failures. Assisted-by: Codex:gpt-5.4
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/inbox/command.ts`:
- Around line 53-67: The duplicated default actor strings used in the bindConfig
blocks for actorName and actorSummary should be extracted into constants to
avoid drift; create descriptive constants (e.g., DEFAULT_EPHEMERAL_INBOX_NAME
and DEFAULT_EPHEMERAL_INBOX_SUMMARY) near the top of the module and replace the
repeated literals in both the key fallback expressions and default properties
used by bindConfig/actorName and bindConfig/actorSummary so both the
option("--actor-summary", ...) and the actorName option reference the same
constant values.
In `@packages/init/src/action/configs.test.ts`:
- Around line 74-96: The test title and assertions are mismatched: the first
test's title claims "2.7.0 and newer" but only checks the boundary 2.7.0; either
rename the title to indicate it's a boundary-only check or add an explicit newer
case; to fix, update the test suite either by changing the test title for the
case that calls setDenoVersion({ deno: "2.7.0", ... }) to something like
"loadDenoConfig omits unstable.temporal on Deno 2.7.0 (boundary)" or add a
second test that calls setDenoVersion({ deno: "2.8.0", ... }) and asserts
loadDenoConfig(createInitData()).data.unstable === undefined, using the same
helpers setDenoVersion, restoreDeno, createInitData and the tested function
loadDenoConfig.
🪄 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: b7cb88fe-dc58-404b-9a31-c972b22b8fa0
📒 Files selected for processing (2)
packages/cli/src/inbox/command.tspackages/init/src/action/configs.test.ts
Extract the inbox actor fallback strings into shared constants so the config defaults stay aligned in one place. This also narrows the Deno Temporal regression test title to the exact 2.7.0 boundary case that it asserts, which keeps the test intent precise. Assisted-by: Codex:gpt-5.4
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
runWithConfig()torun(), passcontextsandcontextOptions, and update the help/completion wiring for Optique 1.0.fedifystartup smoke tests.Why
Optique 1.0.0 removes
runWithConfig()and replaces it with the finalized runner and context API. This keeps the Fedify CLI compatible with the 1.0 release without changing config loading or shell completion behavior.Release notes: dahlia/optique#796
Testing
deno task -f @fedify/cli testnode --test --experimental-transform-types 'src/**/*.test.ts' '!src/init/test/**'in packages/clipnpm testin packages/initnode --disable-warning=ExperimentalWarning dist/mod.js --helpin packages/clinode --disable-warning=ExperimentalWarning dist/mod.js completions bashin packages/clinode dist/test/mod.js --helpin packages/initnode dist/mod.js --helpin packages/create