fix(bin): guard n-rules.js CLI dispatch behind isRunAsCli for testability - #202
Merged
Conversation
…типологія 1/2a/2b/3, задачі T1-T10 Фіксує рішення brainstorm-сесії: три копії тір/ACP-логіки (llm-lib JS, llm-cascade Rust, tauri-components) зводяться до одного Rust-крейта llm-lib (нині llm-cascade) з napi-проєкцією для Node і tauri-командами для webview. Типологія викликів: 1 ACP (cursor/codex/pi), 2a OpenAI- сумісний sync, 2b batch (v1 — емуляція, перший споживач doc-files), 3 pi-npm (Node-only). MIN/AVG/MAX — спільний Tier-інтерфейс усіх типів. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lity The CLI dispatch block ran at module top level with no guard, so importing n-rules.js for testing immediately executed the real CLI against the test process's argv and mutated the filesystem. That's why the file sat at 0% test coverage and failed the repo's 80% line-coverage gate. Wraps the dispatch block in `export async function runCli(argv)`, guarded at the bottom via this repo's existing isRunAsCli helper (cli-entry.mjs), and exports the previously-private helper functions so they're directly testable. Pure refactor — no CLI behavior change, verified via lint --help and unknown-command smoke runs. Adds unit tests bringing n-rules.js to 88.98% line coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Auto-generated by the npm-tsc-types pre-commit hook after exporting the CLI dispatch function and helpers in the previous commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-612660 # Conflicts: # npm/bin/n-rules.js
…no-exports-in-scripts Exporting functions/runCli directly from npm/bin/n-rules.js (a shebang'd script) tripped eslint-plugin-unicorn's no-exports-in-scripts rule in CI. Move all the dispatch logic and helper functions into a new non-shebang sibling module, npm/bin/n-rules-cli.mjs (mirrors the existing bin/rename-yaml-extensions.mjs pattern), leaving n-rules.js as a thin shebang + guard that imports and calls runCli. Update tests to import from n-rules-cli.mjs, add a dedicated guard test (bin/tests/n-rules-entry.test.mjs) to restore n-rules.js's own line coverage, and fix the various eslint/oxlint/knip violations the new test files tripped (console-store-restore, direct process.env access, require-await, static regex hoisting, array mutation helpers, a knip false-positive on a dynamically-imported test fixture, and the runCli dispatch's now-measurable cognitive complexity). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
n-rules.js no longer has exports after the n-rules-cli.mjs split, so its
.d.ts collapses to just `export {}`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…kages The changelog/consistency autofix in the previous commit misattributed this branch's n-rules.js refactor to demo/plugins-ci-github/plugins-lang-js because this dev environment's local `main` ref has diverged from origin/main (resolveNewestMergeBase's ancestor-recency heuristic can't order two branches that never merge, so it fell back to the stale local ref as comparison base — a pre-existing tooling edge case, not something to fix here). None of those packages are touched by this branch's actual diff; a fresh CI checkout won't hit this since it has no such local/origin main divergence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous eslint-disable-line trailing comment on the function declaration got reformatted onto its own line by oxfmt, which broke the line-based suppression (oxlint then reported both the original complexity violation AND an "unused eslint-disable directive"). Switch to a block `eslint-disable`/implicit-EOF scope comment placed before the JSDoc (kept directly adjacent to the export, satisfying doc-files' header-JSDoc adjacency check) instead of a directive attached to the declaration line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…kages (again) Same pre-existing local-environment issue as the earlier revert commit: this dev environment's local main ref has diverged from origin/main in a way resolveNewestMergeBase's ancestor-recency heuristic can't resolve, so the changelog/consistency autofix keeps misattributing this branch's n-rules.js work to demo/plugins-ci-azure/plugins-lang-js. Reverting the same way — none of those packages are touched by this branch's actual diff, and a fresh CI checkout doesn't have this local/origin divergence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
npm/bin/n-rules.js's CLI dispatch (switch(command)+ top-level try/catch + exit handling) ran at module top level with no guard, so simplyimport-ing the file for tests immediately executed the real CLI against the test process'sargv, mutated the filesystem, and could spawn subprocesses / call process-exit machinery. That's why the file sat at 0% test coverage and failed the repo's 80% line-coverage gate.export async function runCli(argv), guarded at the bottom via this repo's existingisRunAsClihelper (npm/scripts/cli-entry.mjs, already used/tested elsewhere) — no new guard pattern invented.normalizeRuleName,readConfig,runSync,syncSkills, etc.) so they're directly unit-testable — mechanicalexportadditions only, no signature/logic changes.npm/bin/tests/) — mostly real functional tests against tmp dirs, plus targetedvi.mocks for the heavier dispatch branches — bringingn-rules.jsline coverage to 88.98% (v8 provider), clearing the 80% gate.node bin/n-rules.js lint --help(prints help, exit 0) and an unknown-command run (prints error, exit 1), matching prior behavior.Test plan
bunx vitest run npm/bin/— 114 new tests passbunx vitest run(full workspace) — no regressions; the only 2 failing files (mirror-parity.test.mjs,docgen-scan.test.mjs) are pre-existing, confirmed identical viagit stashon a clean treen-rules.jsline coverage 88.98% (751/844), above the 80% gatelint --helpand an unknown command behave identically to before the refactornpm/.changes/260724-1333.md)npm/bin/docs/n-rules.mdCRC frontmatter is now stale after this edit — needs regeneration via/n-doc-filesin a follow-up (left out of this PR's scope, per repo convention of keeping doc-files as its own step)🤖 Generated with Claude Code