feat: add paths output and conf-backed config #24

Jercik · 2026-01-15T12:32:23Z

Summary

replace subcommands with flag-based CLI surface
add --paths output and show resolved defaults in help
store config via conf and keep rules source defaults consistent
update docs and tests for new CLI behavior

Testing

pnpm test

github-actions

PR Review Details

Scope: This PR replaces subcommands (init, config-path, sync) with flags (--init, --paths) while adding conf library integration for config storage and improving path handling.

What looks good:

Clean CLI simplification from subcommands to flags
Good flag validation with clear error messages
Improved resolveInside path traversal checks following cross-platform best practices
Updated normalizePath regex properly handles ~user vs ~/path distinction
Tests cover flag validation edge cases well
Documentation is consistent with implementation

Summary: Found 1 medium issue with the --paths flag showing default rulesSource even when config fails to parse for non-ENOENT reasons. The overall refactoring is solid and improves the CLI UX.

Review by Claude (opus)

src/cli/main.ts

path.relative is case-insensitive on Windows, ensuring the default config detection works correctly regardless of path casing.

Copilot

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/config/loader.ts

src/cli/main.test.ts

src/cli/main.ts

Separates access errors (EACCES, not a file) from parse errors (invalid JSON, schema validation) so users get appropriate hints.

github-actions

PR Review Details

What looks good:

Clean transition from subcommands to flags simplifies the CLI surface
Good flag validation with clear error messages
The resolvePaths function nicely handles error cases while still showing useful path info
Path validation improvements using path.relative are correct for Windows case-insensitivity
The ~user path handling fix is a good improvement for safety

Architectural notes:

The conf library integration is straightforward; the store is created per-load which is fine for a CLI tool
The removal of the three separate command registration files reduces complexity

Summary: No critical or high issues found. One medium observation about error handling behavior. The implementation is solid and the documentation is consistent with the code.

Review by Claude (opus)

src/config/loader.ts

src/cli/main.ts

src/utils/paths.ts

github-actions

PR Review Details

Reviewed: README.md, src/cli/main.ts, src/config/{constants.ts,loader.ts}, src/utils/{paths.ts,errors.ts}.
Ran: corepack enable && pnpm install --frozen-lockfile && pnpm test (all passing).

Key notes:

CLI now uses flags ("--init", "--paths") instead of subcommands; docs appear updated accordingly.
Path handling improvements reduce risk of accidental traversal/tilde mis-expansion.

Summary: Found 2 🟡 medium issues (both are small UX/correctness around "--paths"/default config detection) and 2 ℹ️ info notes.

Review by Copilot (gpt-5.2)

src/cli/main.ts

src/config/loader.ts

src/utils/paths.ts

github-actions

PR Review Details

Internal docs: main() JSDoc still mentions "subcommands" even though the CLI is now flag-based.
Error UX: ConfigParseError suggests sync-rules --help for "schema", but the CLI help text appears to be flags + examples only; consider pointing to README or the generated sample config instead.

Summary: Found 1 medium issue. Overall the config loading error taxonomy and Windows-safe default config detection look solid.

Review by Copilot (gpt-5.2)

src/cli/main.ts

src/config/loader.ts

github-actions

PR Review Details

Observed the CLI refactor to flags-only flow; help text and README updates align with the new options.
Config loading now routes through Conf; the overall flow stays thin and readable.

Summary: Found 1 medium issue. See inline comment about --paths masking missing custom configs.

Review by Codex (gpt-5.2-codex)

src/cli/main.ts

Avoid writing system-specific paths to the sample config file. Use "/path/to/rules" as a clear placeholder.

github-actions

PR Review Details

This PR refactors the CLI to use flags instead of subcommands, which simplifies the interface but is a breaking change (correctly documented). It also switches to conf for config loading and improves path security.

Architectural Observations:

CLI Refactor: Moving from init command to --init flag makes the tool more single-purpose, which fits its role well.
Config Loading: The switch to conf for reading the configuration file is interesting. While conf is typically used for managing application persistence (acting as a key-value store in a standard location), using it here to read a specific file path works but adds a heavy dependency for what could be fs.readFile + JSON.parse. However, it doesn't cause any issues.
Security: The updates to resolveInside significantly harden directory traversal prevention.

Summary: Found 0 high, 1 low, and 1 info issue. The changes are solid and tests pass. The CLI refactor is clean, and path resolution is more robust.

Review by Gemini (gemini-3-pro-preview)

src/config/constants.ts

src/utils/paths.ts

github-actions

PR Review Details

This PR refactors the CLI from subcommand-based (init, sync, config-path) to flag-based (--init, --paths) architecture. The implementation is clean and the flag validation logic at lines 102-110 of src/cli/main.ts properly prevents conflicting flag combinations.

Notable improvements:

Good use of conf library for configuration storage
Proper error class hierarchy with ConfigAccessError for permission issues
The resolveInside function improvement using path.relative for Windows case-insensitive path comparison
The ~user expansion fix in normalizePath is correctly handled

The test coverage has been updated appropriately for the new CLI surface.

Summary: Found 1 medium issue regarding error message clarity for --verbose flag. The implementation is solid overall.

Review by Claude (opus)

src/cli/main.ts

Copilot

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/utils/errors.ts

src/config/loader.createSampleConfig.test.ts

github-actions

PR Review Details

Architectural Observations:

Good consolidation: The move from subcommands to a flag-based CLI surface (--init, --paths, --force) simplifies the user experience and reduces code by removing three separate command registration files.
Conf library integration: Using conf for config storage provides a well-tested abstraction for configuration persistence. The createConfigStore wrapper in constants.ts maintains a clean interface.
Improved path validation: The updated resolveInside function in paths.ts now correctly handles Windows case-insensitivity by using path.relative() instead of string comparison, and properly checks for path traversal with the separator-aware ..${path.sep} check.
Better tilde handling: The regex update ^~(?=$|[\\/]) correctly distinguishes between home directory expansion (~ or ~/foo) and Unix-style user home references (~user).
Clear error taxonomy: The new ConfigAccessError class provides a distinct error type for permission/access issues, separating them from parse errors.

Summary: Clean refactor from subcommand architecture to flag-based CLI. Found 1 medium issue with potential unintended behavior when config path has no extension. The code follows good patterns for error handling and path normalization.

Review by Claude (opus)

src/config/constants.ts

src/cli/main.ts

src/utils/paths.ts

github-actions

PR Review Details

The refactor to a flag-based CLI structure simplifies the command interface. The path handling improvements in src/utils/paths.ts are particularly noteworthy for fixing edge cases in path resolution and expansion.

Summary: Found 1 low issue regarding dependency usage. The logic changes are sound and well-tested.

Review by Gemini (gemini-3-pro-preview)

src/config/loader.ts

src/utils/paths.ts

github-actions · 2026-01-15T14:52:10Z

src/utils/paths.ts

+    throw new Error(`Refusing to write outside ${base}: ${relativePath}`);
+  }
+  if (relative_ === ".." || relative_.startsWith(`..${path.sep}`)) {
+    throw new Error(`Refusing to write outside ${base}: ${relativePath}`);


ℹ️ Info: This is a great improvement. The previous check startsWith("..") incorrectly blocked valid sibling filenames starting with .. (e.g. ..foo). The new logic strictly targets directory traversal.

github-actions

PR Review Details

I reviewed the changes to the CLI flags, config loading logic, and path security.

Architectural Observation: `conf` for Config Loading

The introduction of the conf library to load a specific user-provided configuration file (--config) is risky. conf is primarily designed for managing application-owned settings (persisted in system directories), not for loading arbitrary static config files. It brings opinionated behaviors—most critically, the default handling of invalid JSON—that are dangerous in this context.

While the CLI refactoring and path security improvements are solid, the potential for data loss with the new config loader needs immediate attention.

Summary: Found 1 critical issue regarding potential user data loss and 1 info note on security improvements.

Review by Gemini (gemini-3-pro-preview)

src/config/constants.ts

github-actions · 2026-01-15T14:54:54Z

src/utils/paths.ts

+  if (path.isAbsolute(relative_)) {
+    throw new Error(`Refusing to write outside ${base}: ${relativePath}`);
+  }
+  if (relative_ === ".." || relative_.startsWith(`..${path.sep}`)) {


ℹ️ Info: This updated check for parent directory escape (..) is much more robust than the previous implementation. Good fix.

github-actions

\n

PR Review Details

\n\n- Consolidated CLI flags and config loading looks coherent with README updates.\n- CLI help output not verified here due to sandbox permission errors when running `node src/cli/main.ts --help`.\n\n

\n\n**Summary:** Found 0 issues. Changes appear consistent with the updated CLI behavior and config handling.\n\n---\n\n_Review by Codex (gpt-5.2-codex)_

github-actions

\n

PR Review Details

\n\nNo architecture concerns found. CLI flag-based interface is consistent with docs; --paths and --init validation is clear. Error types and path normalization look solid.\n\n

\n\n**Summary:** Found 0 issues. Changes look consistent and well-scoped.\n\n---\n\n_Review by Codex (gpt-5.2-codex)_

github-actions

PR Review Details

CLI UX: moved from subcommands (init/config-path/sync) to flags (--init/--paths), with explicit flag validation and consistent errors.
Config loading now uses the "conf" store: the constructor reads the file during initialization (via #initializeStore calling .store), and .store reads from disk and will create the config directory on ENOENT; the pre-stat guard in loadConfig prevents that side-effect for missing configs.
Docs: README usage examples and option list appear consistent with the new CLI surface.

Summary: Found 0 critical/high/medium issues (3 info notes). The changes look correct and tests pass locally.

Review by Copilot (gpt-5.2)

github-actions · 2026-01-15T14:59:56Z

src/utils/paths.ts

 export function normalizePath(input: string): string {
  const expanded = input.startsWith("~")
-    ? input.replace(/^~/u, homedir())
+    ? input.replace(/^~(?=$|[\\/])/u, homedir())


ℹ️ Info: Good hardening of normalizePath() — restricting ~ expansion to only bare ~/~/... avoids accidentally rewriting ~someone/... into the current user’s home. Also nice tightening in resolveInside() with the .. + path.sep check to avoid false positives like ..foo.

github-actions · 2026-01-15T14:59:56Z

src/config/loader.ts

  try {
-    const configContent = await readFile(normalizedPath, "utf8");
-    return parseConfig(configContent);
+    const configStat = await stat(normalizedPath);


ℹ️ Info: The upfront stat() check is a good fit with conf: its constructor reads the file on init (#initializeStore calling .store), and the .store getter creates the directory on ENOENT (see get store() in conf source: https://raw.githubusercontent.com/sindresorhus/conf/main/source/index.ts). This keeps loadConfig() from creating config dirs when the file is missing.

github-actions · 2026-01-15T14:59:56Z

src/cli/main.ts

+    const wantsSyncFlags =
+      (options.dryRun ?? false) || (options.porcelain ?? false);
+
+    if (options.force && !wantsInit) {


ℹ️ Info: The explicit flag guardrails here (e.g., --force only with --init, and mutually exclusive --init/--paths) should prevent ambiguous CLI states and makes error handling predictable for callers/scripts.

github-actions · 2026-01-15T15:44:43Z

🎉 This PR is included in version 5.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

feat: add paths output and conf-backed config

512c40a

Copilot AI review requested due to automatic review settings January 15, 2026 12:32

Copilot started reviewing on behalf of Jercik January 15, 2026 12:32 View session

This comment was marked as resolved.

Sign in to view

fix: harden config init and tests

00f5965

This comment was marked as resolved.

Sign in to view

fix: stat config before conf load

721d1ac

Copilot AI review requested due to automatic review settings January 15, 2026 13:15

Copilot started reviewing on behalf of Jercik January 15, 2026 13:16 View session

This comment was marked as resolved.

Sign in to view

fix: normalize config path before conf

6b6c160

This comment was marked as resolved.

Sign in to view

test: assert init error cause

f3e810e

This comment was marked as resolved.

Sign in to view

Copilot AI review requested due to automatic review settings January 15, 2026 13:19

Copilot started reviewing on behalf of Jercik January 15, 2026 13:20 View session

This comment was marked as resolved.

Sign in to view

Copilot started reviewing on behalf of Jercik January 15, 2026 14:34 View session

github-actions bot reviewed Jan 15, 2026

View reviewed changes

src/cli/main.ts Show resolved Hide resolved

fix: use path.relative for Windows case-insensitive path comparison

7a24dcb

path.relative is case-insensitive on Windows, ensuring the default config detection works correctly regardless of path casing.

Copilot AI reviewed Jan 15, 2026

View reviewed changes

src/config/loader.ts Show resolved Hide resolved

src/config/loader.ts Show resolved Hide resolved

src/cli/main.test.ts Show resolved Hide resolved

src/cli/main.test.ts Show resolved Hide resolved

src/cli/main.ts Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

feat: add ConfigAccessError for file access issues

f3de9ff

Separates access errors (EACCES, not a file) from parse errors (invalid JSON, schema validation) so users get appropriate hints.

This comment was marked as resolved.

Sign in to view

github-actions bot reviewed Jan 15, 2026

View reviewed changes

src/config/loader.ts Outdated Show resolved Hide resolved

src/cli/main.ts Show resolved Hide resolved

src/utils/paths.ts Show resolved Hide resolved

src/utils/paths.ts Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

github-actions bot reviewed Jan 15, 2026

View reviewed changes

src/cli/main.ts Show resolved Hide resolved

src/config/loader.ts Show resolved Hide resolved

src/utils/paths.ts Show resolved Hide resolved

src/utils/paths.ts Show resolved Hide resolved

github-actions bot reviewed Jan 15, 2026

View reviewed changes

src/cli/main.ts Show resolved Hide resolved

src/config/loader.ts Show resolved Hide resolved

src/config/loader.ts Show resolved Hide resolved

github-actions bot reviewed Jan 15, 2026

View reviewed changes

src/cli/main.ts Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

fix: use placeholder path in sample config

09a6549

Avoid writing system-specific paths to the sample config file. Use "/path/to/rules" as a clear placeholder.

Copilot AI review requested due to automatic review settings January 15, 2026 14:47

Copilot started reviewing on behalf of Jercik January 15, 2026 14:47 View session

github-actions bot reviewed Jan 15, 2026

View reviewed changes

src/config/constants.ts Show resolved Hide resolved

src/utils/paths.ts Show resolved Hide resolved

github-actions bot reviewed Jan 15, 2026

View reviewed changes

src/cli/main.ts Show resolved Hide resolved

src/cli/main.ts Show resolved Hide resolved

Copilot AI reviewed Jan 15, 2026

View reviewed changes

src/utils/errors.ts Show resolved Hide resolved

src/config/loader.createSampleConfig.test.ts Show resolved Hide resolved

refactor: rename store to configStore for clarity

a14165c

github-actions bot reviewed Jan 15, 2026

View reviewed changes

src/config/constants.ts Show resolved Hide resolved

src/cli/main.ts Show resolved Hide resolved

src/utils/paths.ts Show resolved Hide resolved

src/utils/paths.ts Show resolved Hide resolved

github-actions bot reviewed Jan 15, 2026

View reviewed changes

Jercik merged commit ed4f488 into main Jan 15, 2026
10 checks passed

Jercik deleted the feat/paths-config branch January 15, 2026 15:43

github-actions bot added the released label Jan 15, 2026

feat: add paths output and conf-backed config #24

feat: add paths output and conf-backed config #24

Uh oh!

Conversation

Jercik commented Jan 15, 2026

Summary

Testing

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Architectural Observation: `conf` for Config Loading