Add configuration file support for CLI#562
Conversation
Summary of ChangesHello @sij411, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Fedify CLI by introducing robust configuration file support. Users can now define default options for various commands using TOML files, which are loaded hierarchically. This change streamlines the CLI experience by reducing the need for repetitive command-line arguments and improves reproducibility. The integration leverages new configuration management libraries and includes comprehensive documentation and testing. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces configuration file support for the CLI, which is a great enhancement for user experience. The implementation uses @optique/config and valibot for schema validation, which is a solid approach. I've found a few areas for improvement, including a logic issue in how a configuration option is handled, some inconsistencies in default values and documentation, and a recurring pattern in the usage of bindConfig that could be simplified for better maintainability. All original comments have been retained as they do not contradict the provided rules.
2e3b62a to
40e1964
Compare
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
4633fb2 to
667d0b3
Compare
The globals.ts file mixed unrelated concerns: CLI option definitions and
logging configuration. This refactoring improves code organization by moving
each piece to its natural home:
- debugOption moved to options.ts, where all other CLI options are defined
- configureLogging() moved to log.ts, where recordingSink and other logging
utilities already reside
This eliminates globals.ts entirely and makes the codebase easier to navigate
by grouping related functionality together.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Introduce config.ts that consolidates configuration handling:
- Valibot schemas for validating config structure (global debug option,
command-specific sections for webfinger, lookup, inbox, relay, nodeinfo)
- TOML parsing via smol-toml library
- Hierarchical config loading from system (/etc/fedify/config.toml),
user (~/.config/fedify/config.toml), and project (.fedify.toml) levels
- Recursive merging where later configs override earlier ones
This lays the groundwork for issue fedify-dev#265.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Introduce configOption with two mutually exclusive flags:
- -c, --config PATH: loads an additional configuration file on top of the
standard hierarchy (system, user, project)
- -I, --ignore-config: skips all configuration files entirely, useful for
CI environments requiring reproducible behavior
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The config module now delegates file loading and merging to @optique/config instead of implementing custom logic. This removes: - Manual config file path resolution - Multi-file loading and merging logic - Custom TOML parsing with error handling The configOption now uses withDefault() to properly handle the case when neither --ignore-config nor --config is specified. Also removed short flags (-I, -c) to keep only long flags (--ignore-config, --config) for clarity. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implement config file loading that merges multiple sources in order: 1. /etc/fedify/config.toml (system-wide, silently skipped if missing) 2. ~/.config/fedify/config.toml (user config) 3. ./.fedify.toml (project config) 4. --config PATH (custom, throws on error if specified but missing) Use Optique's runWithConfig with load callback for two-pass parsing, and es-toolkit's merge for deep merging config objects. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
[ci skip]
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Integrate Optique's bindConfig() to allow CLI options to fall back to config file values. Uses type assertion on the key function return type to properly narrow types when defaults are provided. Commands updated: - relay: protocol, port, name - inbox: actorName, actorSummary, authorizedFetch - webfinger: userAgent, allowPrivateAddress, maxRedirection - lookup: userAgent, timeout - nodeinfo: userAgent Also fixes version mismatches across workspace packages (1.10.3). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add tunnel section to config schema and wrap the service option with bindConfig() to allow tunnel service to be configured via config file. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add missing fields to config schema: - lookup: firstKnock, defaultFormat, separator - relay: persistent, acceptFollow, rejectFollow - nodeinfo: raw, bestEffort, noFavicon, metadata Add bindConfig() for: - lookup: separator - relay: persistent Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extend config file support for more CLI options: - lookup: authorizedFetch, traverse, suppressErrors, format (defaultFormat) - nodeinfo: raw, bestEffort, noFavicon, metadata The nodeinfo options were restructured from an or() discriminated union to a flat object structure to enable bindConfig integration. This simplifies the runtime checks from `"raw" in command && command.raw` to `command.raw`. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This version includes a fix for type inference with bindConfig and multiple() for array options. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Refine config schema to only support fields listed in the issue spec: - Global settings: debug, userAgent, logFile, tunnelService - webfinger: allowPrivateAddress, maxRedirection - lookup: authorizedFetch, firstKnock, traverse, suppressErrors, defaultFormat, separator, timeout - inbox: actorName, actorSummary, authorizedFetch, noTunnel, follow, acceptFollow - relay: protocol, port, name, persistent, noTunnel, acceptFollow, rejectFollow - nodeinfo: raw, bestEffort, showFavicon, showMetadata Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Bind follow and acceptFollow arrays with config defaults
- Fix actorName/actorSummary to use ?? fallback instead of type cast
- Fix authorizedFetch to have proper boolean default
- Use createTunnelOption("inbox") for tunnel options
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Bind userAgent to global config.userAgent (optional) - Bind allowPrivateAddresses with default false - Bind maxRedirection without default (optional) - Remove type casts in favor of proper undefined handling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Bind authorizedFetch, firstKnock, traverse, suppressErrors with defaults - Bind format (defaultFormat) and separator with defaults - Bind timeout without default (optional) - Add shared userAgentOption from options.ts - Replace type casts with ?? fallbacks for type safety Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Upgrade @optique/config, @optique/core, @optique/run to 0.10.0-dev.348 which fixes a bug where bindConfig.complete crashed with undefined state when no CLI args were consumed. Update test expectations to reflect bindConfig behavior: - userAgent now returns default value instead of undefined - Invalid option values fall back to default instead of failing See: dahlia/optique#94 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Documents the new TOML configuration file feature for @fedify/cli in the Version 2.0.0 section. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The noFavicon option was incorrectly reading showFavicon from config. When showFavicon was true, noFavicon would also be true, which is inverted. Changed to use strict equality check (=== false) so noFavicon is only true when showFavicon is explicitly set to false. Also updated CHANGES.md to include inbox and nodeinfo in the list of commands with configuration support. Additionally simplified some bindConfig key functions: - Removed redundant type assertion in lookup.ts firstKnock - Removed redundant nullish coalescing in determineSpec - Changed `as number` to `?? default` for port and maxRedirection Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
667d0b3 to
735f481
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces TOML configuration file support for the CLI, which is a great enhancement for usability. The implementation is solid, using @optique/config to bind options and a hierarchical loading system. I've identified a couple of areas for improvement in error handling to make the new feature more robust and user-friendly. I also found a small typo in package.json that could affect Deno users. Overall, great work on this feature.
| "engines": { | ||
| "node": ">=20.0.0", | ||
| "bun": ">=1.2.0", | ||
| "denp": ">=2.0.0" |
| const custom = parsed.configPath | ||
| ? parseToml(readFileSync(parsed.configPath, "utf-8")) | ||
| : {}; |
There was a problem hiding this comment.
When a user provides a custom configuration file via --config that is invalid (e.g., doesn't exist, is unreadable, or contains invalid TOML), the application currently crashes with an unhandled exception. It would be better to catch this error and provide a more user-friendly message before exiting. Re-throwing a more descriptive error will allow runWithConfig to handle it gracefully.
const custom = (() => {
if (!parsed.configPath) return {};
try {
return parseToml(readFileSync(parsed.configPath, "utf-8"));
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
throw new Error(
`Failed to load or parse config file "${parsed.configPath}": ${message}`,
);
}
})();| } catch { | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
The current catch block swallows all errors silently, including syntax errors in the TOML file or file permission issues. This can make it difficult for users to debug their configuration. It would be more user-friendly to log a warning for errors other than "file not found" (ENOENT), which is an expected case for optional config files.
} catch (error) {
if (error instanceof Error && "code" in error && error.code === "ENOENT") {
// File not found is not an error in this context.
return {};
}
const message = error instanceof Error ? error.message : String(error);
console.error(`Warning: Could not load or parse config file at "${path}": ${message}`);
return {};
}
Summary
--config(-C) option to specify a custom configuration file path--ignore-configoption to skip loading configuration filesbindConfigfrom@optique/configfor commands:lookup,webfinger,tunnel,relay,inbox, andnodeinfoCloses #265
File changes
New files
packages/cli/src/config.ts- Configuration schema and loading logic using ValibotModified files
packages/cli/src/mod.ts- IntegratedrunWithConfigfor hierarchical config loadingpackages/cli/src/options.ts- Added--configand--ignore-configoptions, wrapped options withbindConfigpackages/cli/src/lookup.ts- Bound lookup options with configpackages/cli/src/lookup.test.ts- Added tests for authorizedFetchOption with configpackages/cli/src/nodeinfo.ts- Bound nodeinfo options with configpackages/cli/src/inbox.tsx- Bound inbox options with configpackages/cli/src/relay.ts- Bound relay options with configpackages/cli/src/tunnel.ts- Updated tunnel commandpackages/cli/src/webfinger/command.ts- Bound webfinger options with configpackages/cli/src/webfinger/mod.test.ts- Updated tests for bindConfig compatibilitypackages/cli/deno.json- Added@optique/configdependenciespackages/cli/package.json- Added@optique/configdependenciesdocs/cli.md- Added configuration file documentationCHANGES.md- Added changelog entry for Version 2.0.0Removed files
packages/cli/src/globals.ts- Relocated contents to appropriate modulesTest plan
--configoption with custom config file path--ignore-configoption skips all config filesdeno task testin packages/cli to verify all tests pass🤖 Generated with Claude Code