feat: add positional path arguments for single-file/directory targeting#15
feat: add positional path arguments for single-file/directory targeting#15CodeDeficient wants to merge 35 commits into
Conversation
…d less than 7 days ago - Add minPackageAgeDays config option (default: 7 days) - Add analyzePackageVersions method to parse package.json/package-lock.json - Add getNpmPackageAge helper to fetch version publish date from npm registry - Add npmPackageCache to cache npm lookups and avoid repeated API calls - Update detect and analyzeFile to be async for npm lookups
KarpeSlop had no way to target specific files or directories -- it always globs the entire project. Add [path...] positional arguments that accept files or directories to scan, skipping full-project discovery when provided. - Add targetPaths field and constructor parameter to AISlopDetector - Add resolveTargetPaths() to validate, filter, and expand CLI paths - Update detect() to use targetPaths when provided instead of findAllFiles() - Parse positional args (non-flags) as target paths in runIfMain() - Update --strict to report which file(s) had critical issues when targeting - Update help text with path arguments and examples Closes #14
…hs, add -- separator The resolveTargetPaths() method had an incomplete ignore list missing '**/.*', '**/types/**', self-exclusion patterns, and the post-glob filter. This meant targeting a directory would include .git/, coverage/, and the detector itself — files excluded in full-project scan mode. Extract getGlobIgnorePatterns() and isExcludedPath() as shared methods used by both findAllFiles() and resolveTargetPaths(). Also add -- separator support so file paths starting with - aren't misclassified as flags.
…patterns Convert double-negation !(a && b) to positive form (a || b) for readability. Add checks for dotfiles, /types/, .d.ts, and self- exclusion patterns that were in getGlobIgnorePatterns but missing from isExcludedPath.
…for explicit file args
- Replace startsWith('.') with segment-based check so nested dotfiles
like src/.env are properly excluded
- Add startsWith('types/') to match root-level types/ directory
- Run isExcludedPath on explicitly passed file paths in
resolveTargetPaths so excluded files are skipped with a warning
rather than silently scanned
Mirror the three-check pattern used for generated/ — trailing slash variant catches types/ as a directory anywhere in the path, while the prefix-only and contains variants cover segment-edge cases.
…false positives
includes('types/') is a substring match — it would match subtypes/,
mytypes/, and any path where 'types/' appears as a substring of a
segment. Use split('/').includes('types') so only actual segments
named 'types' are excluded.
After switching to split('/').includes('types') in 7db3f3f, the older
includes('/types/') and startsWith('types/') lines were subsumed by
the segment-based check. Remove them to keep the exclusion list clean.
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
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:
📝 WalkthroughWalkthroughAdds configurable npm-publish-time “fresh package version” checks, makes per-file analysis async with bounded registry lookups and caching, centralizes discovery/ignore/exclude logic and target-path expansion (including ChangesFresh Package Version Detection and CLI Targeting
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant AISlopDetector
participant resolveTargetPaths
participant analyzeFile
participant analyzePackageVersions
participant getNpmPackageAge
participant npmRegistry
User->>CLI: run with optional paths / flags
CLI->>AISlopDetector: construct(with parsed targetPaths)
AISlopDetector->>resolveTargetPaths: expand/validate targets
AISlopDetector->>analyzeFile: await per-file analysis
analyzeFile->>analyzePackageVersions: manifest content
analyzePackageVersions->>getNpmPackageAge: request package@version (pLimit)
getNpmPackageAge->>npmRegistry: fetch metadata (abort if slow)
npmRegistry-->>getNpmPackageAge: return publish times
getNpmPackageAge-->>analyzePackageVersions: publish time / age
analyzePackageVersions-->>analyzeFile: emit fresh_package_version (deduped)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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 unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a390199733
ℹ️ 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".
| if (filePath.endsWith('package.json') || filePath.endsWith('package-lock.json')) { | ||
| this.analyzePackageVersions(filePath, content); |
There was a problem hiding this comment.
Make package files reachable before analyzing them
When users rely on the new fresh_package_version rule advertised in the changelog, this branch is never reached from the CLI: findAllFiles() only globs .ts/.tsx/.js/.jsx, and resolveTargetPaths() rejects explicit package.json or package-lock.json targets as unsupported because targetExtensions does not include .json. As a result, both full scans and karpeslop package.json silently skip the only files this rule can analyze.
Useful? React with 👍 / 👎.
| } else if (this.targetExtensions.includes(ext)) { | ||
| resolved.push(targetPath); | ||
| } else { | ||
| console.warn(`⚠️ Target file has unsupported extension (${ext}), skipping: ${targetPath}`); |
There was a problem hiding this comment.
Suggestion: Explicit file targeting rejects .json inputs as unsupported, so users cannot target package.json/package-lock.json directly even though package-age logic was added. This leaves the new package analysis unreachable for targeted scans. Allow JSON files for this specific analysis path. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Users cannot target `package.json`/`package-lock.json` via CLI.
- ⚠️ Manifest-only scans misleadingly report "no issues detected".
- ⚠️ Package freshness rule remains unreachable in typical targeted use.Steps of Reproduction ✅
1. Run the CLI with a manifest path, e.g. `karpeslop package.json`, which is routed
through `runIfMain()` in `ai-slop-detector.ts:27-44`. Since the argument does not start
with `-`, it lands in `targetPaths` (line 43), and `AISlopDetector` is constructed with
those `targetPaths` at line 46.
2. `runIfMain()` computes `quiet`/`strict` (lines 99-100) and calls `await
detector.detect(quiet)` at line 103. In `detect()` at `ai-slop-detector.ts:152-171`,
`this.targetPaths.length > 0` is true, so it calls `this.resolveTargetPaths()` (line 158)
to turn CLI paths into `filesToAnalyze`.
3. Inside `resolveTargetPaths()` at `ai-slop-detector.ts:249-268`, for `targetPath`
pointing to `package.json`, `fs.statSync(targetPath)` reports a file, `ext =
path.extname(targetPath)` becomes `.json`, and `this.isExcludedPath(targetPath)` is false
(check at `ai-slop-detector.ts:208-225` does not match `package.json`).
4. The extension check block at `ai-slop-detector.ts:559-566` then executes: since `.json`
is not in `this.targetExtensions` (defined as `['.ts', '.tsx', '.js', '.jsx']` at class
top), control goes to the `else` branch at line 565, logging `⚠️ Target file has
unsupported extension (.json), skipping: package.json` and never pushing the path into
`resolved`. `detect()` then iterates over an empty `filesToAnalyze` array and
`generateReport()` at `ai-slop-detector.ts:113-120` prints "✅ No AI Slop issues
detected!", meaning `package.json`/`package-lock.json` are never analyzed in targeted
scans despite the new package-age logic in `analyzePackageVersions()`.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ai-slop-detector.ts
**Line:** 563:566
**Comment:**
*Incomplete Implementation: Explicit file targeting rejects `.json` inputs as unsupported, so users cannot target `package.json`/`package-lock.json` directly even though package-age logic was added. This leaves the new package analysis unreachable for targeted scans. Allow JSON files for this specific analysis path.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if (filePath.endsWith('package.json') || filePath.endsWith('package-lock.json')) { | ||
| this.analyzePackageVersions(filePath, content); |
There was a problem hiding this comment.
Suggestion: analyzePackageVersions is async but it's called without await, so detect() can finish and generate/exit before package-age checks complete. This causes nondeterministic missing issues (especially in --strict mode). Await this call so package findings are included before reporting and exit-code decisions. [possible bug]
Severity Level: Major ⚠️
- ⚠️ Package freshness issues may be missing from CLI output.
- ⚠️ `ai-slop-report.json` may omit `fresh_package_version` entries.
- ⚠️ Future strict-mode checks on package age could misfire.Steps of Reproduction ✅
1. The CLI entrypoint `runIfMain()` in `ai-slop-detector.ts:27-47` constructs `const
detector = new AISlopDetector(rootDir, targetPaths.length > 0 ? targetPaths : undefined)`
and then calls `await detector.detect(quiet)` at `ai-slop-detector.ts:102-104`.
2. `detect()` at `ai-slop-detector.ts:152-176` builds `filesToAnalyze` and, for each file,
awaits the async method `this.analyzeFile(file, quiet)` (line 175), expecting all per-file
analysis to complete before returning issues.
3. Inside `analyzeFile()` at `ai-slop-detector.ts:9-237`, after pattern scanning, there is
a special-case block at lines 234-237: `if (filePath.endsWith('package.json') ||
filePath.endsWith('package-lock.json')) { this.analyzePackageVersions(filePath, content);
}` which calls the async `analyzePackageVersions()` (declared at
`ai-slop-detector.ts:25-71`) without `await`.
4. When `analyzeFile()` is eventually invoked with a `package.json` or `package-lock.json`
path (once JSON files are included in `filesToAnalyze` via `findAllFiles()` at
`ai-slop-detector.ts:231-243` or `resolveTargetPaths()` at `ai-slop-detector.ts:249-268`),
it will return to `detect()` before `analyzePackageVersions()` finishes, allowing
`detect()` to call `generateReport()` at `ai-slop-detector.ts:113-120` and `runIfMain()`
to compute exit codes at `ai-slop-detector.ts:108-121` without all `fresh_package_version`
issues having been added to `this.issues`. Currently, JSON files are not yet wired into
scanning (see `targetExtensions` and `findAllFiles()`), so this bug is latent but real in
the new async design.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ai-slop-detector.ts
**Line:** 914:915
**Comment:**
*Possible Bug: `analyzePackageVersions` is async but it's called without `await`, so `detect()` can finish and generate/exit before package-age checks complete. This causes nondeterministic missing issues (especially in `--strict` mode). Await this call so package findings are included before reporting and exit-code decisions.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| const minAgeDays = this.config.minPackageAgeDays ?? 7; | ||
| const minAgeMs = minAgeDays * 24 * 60 * 60 * 1000; |
There was a problem hiding this comment.
Suggestion: minPackageAgeDays is used in arithmetic without validation, so an invalid config value (e.g. non-numeric string) produces NaN and disables the age comparison silently. Validate this config field as a finite positive number before using it. [type error]
Severity Level: Major ⚠️
- ⚠️ Misconfigured `minPackageAgeDays` silently disables package-age checks.
- ⚠️ Users may trust manifest freshness despite broken comparison logic.
- ⚠️ CI policies based on this rule become unreliable under bad config.Steps of Reproduction ✅
1. Create a configuration file such as `.karpesloprc.json` in the project root with a
non-numeric `minPackageAgeDays` (e.g. `{ \"minPackageAgeDays\": \"seven\" }`).
`loadConfig()` at `ai-slop-detector.ts:93-107` reads and parses this config, then calls
`validateConfig()` at `ai-slop-detector.ts:27-87`.
2. `validateConfig()` rigorously validates `customPatterns`, `severityOverrides`, and
`ignorePaths` (lines 15-65) but never checks that `cfg.minPackageAgeDays` exists or is a
finite number; it returns `cfg as KarpeSlopConfig` at line 67, so the string `"seven"` is
stored as `this.config.minPackageAgeDays`.
3. When `analyzePackageVersions()` is invoked for a manifest file at
`ai-slop-detector.ts:25-71`, it computes `const minAgeDays = this.config.minPackageAgeDays
?? 7;` (line 26), which results in `minAgeDays === "seven"`, and then `const minAgeMs =
minAgeDays * 24 * 60 * 60 * 1000;` (line 27), which yields `NaN` because string `"seven"`
cannot be multiplied.
4. In the loop at `ai-slop-detector.ts:53-69`, each `ageInfo.ageMs < minAgeMs` comparison
becomes `ageInfo.ageMs < NaN`, which is always false in JavaScript. This silently prevents
any `fresh_package_version` issues from being added to `this.issues`, effectively
disabling the package-age rule whenever `minPackageAgeDays` is misconfigured, without any
warning to the user.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ai-slop-detector.ts
**Line:** 1065:1066
**Comment:**
*Type Error: `minPackageAgeDays` is used in arithmetic without validation, so an invalid config value (e.g. non-numeric string) produces `NaN` and disables the age comparison silently. Validate this config field as a finite positive number before using it.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| for (const [pkgName, version] of Object.entries(packageData)) { | ||
| if (version.startsWith('^') || version.startsWith('~')) { | ||
| const actualVersion = version.slice(1); | ||
| const ageInfo = await this.getNpmPackageAge(pkgName, actualVersion); |
There was a problem hiding this comment.
Suggestion: The package-age rule only runs for versions starting with ^ or ~, but package-lock.json stores concrete versions (for example 1.2.3) so those entries are never checked. This makes the new lockfile analysis effectively nonfunctional. Handle exact versions too (strip range prefix only when present). [logic error]
Severity Level: Major ⚠️
- ⚠️ `package-lock.json` freshness checks never execute for pinned versions.
- ⚠️ Dependency age assessment relies solely on `package.json` ranges.
- ⚠️ CI consumers may falsely assume lockfile versions are vetted.Steps of Reproduction ✅
1. When `analyzeFile()` eventually processes a `package-lock.json` (special-case call at
`ai-slop-detector.ts:234-237` for paths ending with `package-lock.json`), it passes the
file content into `analyzePackageVersions(filePath, content)` at
`ai-slop-detector.ts:25-71`.
2. In `analyzePackageVersions()`, the `package-lock.json` branch at
`ai-slop-detector.ts:35-45` builds `packageData[name] = version` using the `version` field
from each `pkg.packages` entry, which in a real npm lockfile is a concrete version string
such as `"1.2.3"` without any `^` or `~` prefix.
3. The loop at `ai-slop-detector.ts:53-55` then iterates over
`Object.entries(packageData)` and only proceeds with the age check when
`version.startsWith('^') || version.startsWith('~')`. For lockfile entries (e.g.
`"1.2.3"`), this condition is always false, so the body that computes `const actualVersion
= version.slice(1); const ageInfo = await this.getNpmPackageAge(pkgName, actualVersion);`
never executes.
4. As a result, even if `getNpmPackageAge()` at `ai-slop-detector.ts:76-105` would
indicate that a dependency was published just hours ago, no `fresh_package_version` issues
are ever pushed to `this.issues` from the lockfile path; only ranged versions in
`package.json` (handled at `ai-slop-detector.ts:46-48`) can currently be flagged. This
makes the intended lockfile analysis effectively a no-op.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ai-slop-detector.ts
**Line:** 1092:1095
**Comment:**
*Logic Error: The package-age rule only runs for versions starting with `^` or `~`, but `package-lock.json` stores concrete versions (for example `1.2.3`) so those entries are never checked. This makes the new lockfile analysis effectively nonfunctional. Handle exact versions too (strip range prefix only when present).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| this.analyzePackageVersions(filePath, content); | ||
| } |
There was a problem hiding this comment.
Suggestion: The async package-version scan is invoked without await, so the CLI can print the report and exit before those checks finish. This can silently omit findings and produce incorrect strict-mode behavior. Await the async call inside file analysis. [possible bug]
Severity Level: Major ⚠️
- ⚠️ Runtime CLI may omit package freshness issues in output.
- ⚠️ Generated `ai-slop-report.json` can be incomplete for manifests.
- ⚠️ Future strict-mode gating on package age could be inconsistent.Steps of Reproduction ✅
1. The published CLI entrypoint in `karpeslop-bin.js` constructs `new
AISlopDetector(rootDir)` (see constructor at `karpeslop-bin.js:228-231`) and calls `await
detector.detect(quiet)` in its main run function (not shown in the diff but mirrored from
`ai-slop-detector.ts:102-104`), relying on `detect()` to finish all per-file analysis
before exit.
2. `detect()` in `karpeslop-bin.js:25-45` finds TypeScript/JavaScript files via
`findAllFiles()` (`karpeslop-bin.js:51-92`), filters them based on `quiet`, and for each
one does `await this.analyzeFile(file, quiet)` at line 40.
3. In `analyzeFile()` at `karpeslop-bin.js:21-223`, after running all text-pattern checks,
there is a special-case block at lines 219-222: `if (filePath.endsWith('package.json') ||
filePath.endsWith('package-lock.json')) { this.analyzePackageVersions(filePath, content);
}`. `analyzePackageVersions()` is defined as an async function at
`karpeslop-bin.js:860-904`, but this call is made without `await`.
4. Once the codebase is updated to actually include `package.json`/`package-lock.json`
paths in `filesToAnalyze` (mirroring the intended design in `ai-slop-detector.ts`),
`analyzeFile()` will resolve its promise while `analyzePackageVersions()` is still
running. This allows `detect()` to finish, `generateReport()` at
`karpeslop-bin.js:949-955` to log results, and the CLI to compute exit codes based on an
incomplete `this.issues` array, omitting `fresh_package_version` findings that may be
added slightly later.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** karpeslop-bin.js
**Line:** 720:721
**Comment:**
*Possible Bug: The async package-version scan is invoked without `await`, so the CLI can print the report and exit before those checks finish. This can silently omit findings and produce incorrect strict-mode behavior. Await the async call inside file analysis.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix- Add missing await on analyzePackageVersions call so fresh_package_version issues aren't dropped (fire-and-forget promise) - Lockfile branch now correctly checks all package versions (not just ^/~ ranges) since package-lock stores exact semver - Union dependencies/devDependencies/peerDependencies/optionalDependencies in package.json check instead of dependencies only - Skip nested node_modules entries when scanning package-lock to avoid transitive double-counting - package.json/package-lock.json now included in --quiet mode so fresh version warnings still fire - Add parseVersionRange helper to centralize version-range handling - Fix package-lock.json bin entry to point at karpeslop-cli.js (matching package.json) and bump version to 1.0.25
| const ext = path.extname(targetPath); | ||
| if (this.isExcludedPath(targetPath)) { | ||
| console.warn(`⚠️ Target file is in an excluded path, skipping: ${targetPath}`); | ||
| } else if (this.targetExtensions.includes(ext)) { | ||
| resolved.push(targetPath); | ||
| } else { | ||
| console.warn(`⚠️ Target file has unsupported extension (${ext}), skipping: ${targetPath}`); | ||
| } |
There was a problem hiding this comment.
🟠 Architect Review — HIGH
Explicit file targeting and global discovery only accept .ts/.tsx/.js/.jsx, so package.json/package-lock.json are never analyzed; this makes the new fresh_package_version rule unreachable even when users target manifest files.
Suggestion: Extend file discovery to include package.json/package-lock.json (both in full-project and targeted modes), e.g., by special-casing these manifest paths alongside the existing code extensions so package-age analysis actually runs.
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** ai-slop-detector.ts
**Line:** 560:567
**Comment:**
*HIGH: Explicit file targeting and global discovery only accept .ts/.tsx/.js/.jsx, so package.json/package-lock.json are never analyzed; this makes the new fresh_package_version rule unreachable even when users target manifest files.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
package.json (1)
7-10: Validate CLI entrypoint uses the targeting + package freshness implementation.
package.jsonpublishesbin.karpeslopaskarpeslop-cli.js, and that wrapper runsai-slop-detector.tsvia the localtsxbinary.ai-slop-detector.tscontains the targeting and package freshness logic (targetPaths/resolveTargetPathsandanalyzePackageVersions), so the executable path already references the intended implementation.
npm run buildstill outputskarpeslop-bin.js, but it isn’t thebinentrypoint; alignbinwith the built artifact only if that’s intended.🤖 Prompt for 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. In `@package.json` around lines 7 - 10, package.json's "bin.karpeslop" points to karpeslop-cli.js while the build script emits karpeslop-bin.js, causing a mismatch between the published CLI entrypoint and the built artifact; decide which artifact should be the true CLI and make one of two fixes: either change "bin.karpeslop" to "karpeslop-bin.js" so the published bin matches the build, or alter the "build" script to output karpeslop-cli.js (or update the wrapper) so karpeslop-cli.js runs the compiled code; ensure whichever file is used still invokes the implementation in ai-slop-detector.ts (which contains targetPaths/resolveTargetPaths and analyzePackageVersions) so the targeting and package freshness logic is executed.
🤖 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 `@ai-slop-detector.ts`:
- Around line 533-538: findAllFiles() and resolveTargetPaths() currently filter
to only code extensions so package.json and package-lock.json never reach
analyzeFile(), making fresh_package_version unreachable; update the
file-selection logic to include "package.json" and "package-lock.json" (or a
".json" pattern) in this.targetExtensions or otherwise add explicit patterns for
those filenames so they are collected by the glob and pass through
isExcludedPath to analyzeFile(); ensure resolveTargetPaths() similarly allows
these filenames when expanding explicit targets so analyzeFile() can detect and
handle fresh_package_version.
- Around line 1128-1130: The npm registry fetch using const url =
`https://registry.npmjs.org/${encodeURIComponent(pkgName)}` and await fetch(url)
can stall indefinitely; wrap the fetch in an AbortController with a short
configurable timeout (e.g., 3–10s), start a timer that calls controller.abort(),
pass signal to fetch, clear the timer after fetch completes, and catch the
abort/timeout error to return null (or handle as existing non-ok path) so
detection flow cannot block indefinitely.
- Around line 914-916: The call to analyzePackageVersions(filePath, content) is
currently fire-and-forget and can allow reporting/strict-mode decisions to run
before npm-age issues are added; change the caller (the function containing the
snippet that checks filePath.endsWith('package.json') ||
filePath.endsWith('package-lock.json')) to await analyzePackageVersions(...) (or
return its Promise) so the analysis completes before continuing, and if
necessary mark that enclosing function as async and propagate the Promise to
callers so reporting/strict-mode logic waits for analyzePackageVersions to
finish.
- Line 71: The new config field minPackageAgeDays is typed but not validated in
validateConfig(), which allows non-finite/negative values to break freshness
logic; update validateConfig() to check that config.minPackageAgeDays (when
provided) is a finite integer >= 0 (or enforce a minimum like 0), reject or
coerce invalid values, and surface a clear error message referencing
minPackageAgeDays so callers know the config is invalid; ensure any code that
reads minPackageAgeDays (the freshness rule) relies on the validated value.
- Around line 1076-1081: In analyzePackageVersions() update the lockfile branch
so package entries are recorded even when pkgInfo.name is missing: derive the
package name from pkgPath (e.g., parse node_modules/... to produce '`@scope/pkg`'
or 'pkg') when pkgInfo.name is falsy, continue to use pkgInfo.version when
present, and retain the existing skip for empty pkgPath or 'node_modules/';
ensure you still populate packageData[name] = version (or undefined/empty if
version missing) so lockfile entries aren’t gated by pkgInfo.name, and leave the
later isRange/filePath handling unchanged.
In `@CHANGELOG.md`:
- Line 5: Update the CHANGELOG.md to include a new release header for 1.0.25 to
match package.json: add a top-level section "## [1.0.25] - <release-date>" above
the existing "## [1.0.24] - 2026-04-04" entry and populate it with the
appropriate release notes (or a brief summary/placeholder if the full notes are
elsewhere) so the file versioning aligns with package.json.
---
Nitpick comments:
In `@package.json`:
- Around line 7-10: package.json's "bin.karpeslop" points to karpeslop-cli.js
while the build script emits karpeslop-bin.js, causing a mismatch between the
published CLI entrypoint and the built artifact; decide which artifact should be
the true CLI and make one of two fixes: either change "bin.karpeslop" to
"karpeslop-bin.js" so the published bin matches the build, or alter the "build"
script to output karpeslop-cli.js (or update the wrapper) so karpeslop-cli.js
runs the compiled code; ensure whichever file is used still invokes the
implementation in ai-slop-detector.ts (which contains
targetPaths/resolveTargetPaths and analyzePackageVersions) so the targeting and
package freshness logic is executed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac69d49d-f229-4b78-a664-8e472b35d66e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
CHANGELOG.mdai-slop-detector.tskarpeslop-bin.jspackage.json
Merge main (picomatch + brace-expansion bumps, prior PR #9) and address inline PR review comments from chatgpt-codex-connector and codeant-ai: - Make package.json/package-lock.json reachable: extend findAllFiles to discover manifest files at root, and allow .json extension in resolveTargetPaths for the two manifest filenames. Previously the fresh_package_version rule was unreachable from both full scans and explicit CLI targeting. - Add minPackageAgeDays validation in validateConfig — rejects non-numeric or negative values which previously produced NaN and silently disabled the age comparison. - Rebuild karpeslop-bin.js from updated source so the compiled artifact also awaits analyzePackageVersions.
|
Resolved merge conflict with main and addressed the review comments in 8ad6176:
PR is now MERGEABLE. |
- getNpmPackageAge: wrap fetch in AbortController with 5s timeout so registry requests can't stall the scan indefinitely - analyzePackageVersions: derive package name from pkgPath when pkgInfo.name is missing in package-lock entries - CHANGELOG: add 1.0.25 release header to match package.json
|
Addressed the new CodeRabbit review comments in 7a3924c:
The other inline review items (manifest reachability, |
- Parallelize getNpmPackageAge calls in analyzePackageVersions via Promise.all so registry requests run concurrently instead of sequentially (was N x 5s in worst case) - Cache the full time map from the registry response instead of just the looked-up version, so different versions of the same package hit cache rather than re-fetching megabytes of data - Log a one-time warning when the npm registry can't be reached instead of silently swallowing network errors - Defer detector construction past --help/--version so a bad .karpesloprc.json doesn't break help output - Drop dead info.name branch in lockfile parsing: name is derived from pkgPath since the only entry that has info.name is the root which is already skipped
|
Pushed 4937743. The scan now includes nested workspace manifests in both full scans and directory-targeted scans, and fresh-package findings are deduped by package/version so package.json + package-lock.json do not double-report the same dependency. |
|
Pushed a86c719.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@karpeslop-bin.js`:
- Around line 437-446: isExcludedPath currently checks hardcoded segment names
and file suffixes but ignores this.customIgnorePaths, causing explicitly
configured ignores to be bypassed for positional file targets; update
isExcludedPath to also iterate this.customIgnorePaths and return true if any
custom pattern matches the computed relativePath (or matches a segment as done
by excludedSegment) so that custom ignores are honored for both full-project
discovery and direct targets (apply the same change to the other similar block
referenced around 490-497); use the existing symbols relativePath, segments,
excludedSegment and this.customIgnorePaths to locate where to add the
pattern-match check.
- Around line 413-415: detect() currently continues when explicit target paths
resolve to zero files; after calling this.resolveTargetPaths() and assigning
filesToAnalyze, check if filesToAnalyze.length === 0 and fail fast: log a clear
error mentioning the supplied target paths (this.targetPaths) and exit non‑zero
(e.g., process.exit(1) or throw an Error) instead of printing "Targeting 0
file(s)" and continuing. Update the block that contains the console.log call so
the zero-case triggers the failure path and returns a non‑success status for CI.
- Around line 1021-1025: The dedupe currently builds issueKey as
`${pkgName}|${version}` so reportedFreshPackageKeys suppresses findings across
different manifests; change the key generation in the block that computes
issueKey (where pkgName and version are used) to include a manifest-specific
identifier (e.g., manifest path or manifest id) — e.g.
`${manifestId}|${pkgName}|${version}` — and use that new key when checking and
adding to reportedFreshPackageKeys so deduping is limited to a single
manifest/workspace rather than the entire run.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25567e7a-6817-4b91-a7bb-8d11dcd9f421
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
CHANGELOG.mdai-slop-detector.tskarpeslop-bin.js
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- ai-slop-detector.ts
|
Pushed 04f3ea1. The fresh-package scan now preserves lockfile entry identity instead of keying package-lock entries only by package name. Each lockfile entry keeps its own source path plus package name/version, so workspace manifests and duplicate package names at different versions are reported separately. |
|
Pushed 74a5e14.
Smoke-tested with:
|
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 `@karpeslop-cli.js`:
- Line 64: The primary child exit handler (handleChildExit) must be guarded so
it doesn't terminate the wrapper when a fallback is active: either remove
nodeChild.on('exit', handleChildExit) before starting the fallback or add a
guard inside handleChildExit to ignore exits from the original nodeChild when a
fallbackActive flag is true; update the code paths that start the fallback to
set fallbackActive (or otherwise mark which child is authoritative) and ensure
handleChildExit only acts on the currently authoritative child (reference
nodeChild, handleChildExit, and the new/updated
fallbackActive/authoritative-child marker).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d75a3c1d-dc9a-441c-83ce-14de4c487896
📒 Files selected for processing (3)
ai-slop-detector.tskarpeslop-bin.jskarpeslop-cli.js
🚧 Files skipped from review as they are similar to previous changes (2)
- karpeslop-bin.js
- ai-slop-detector.ts
| process.exit(1); | ||
| }); | ||
|
|
||
| nodeChild.on('exit', handleChildExit); |
There was a problem hiding this comment.
Guard primary child exit when fallback is active.
When ENOENT triggers fallback, the original child can still emit exit; handleChildExit at Line 70 may terminate the wrapper before nodeChild finishes, causing flaky fallback behavior and wrong final exit status.
Suggested fix
+let fallbackActive = false;
+
function handleChildExit(code, signal) {
if (signal) {
process.exit(exitCodeForSignal(signal));
}
process.exit(code ?? 0);
}
@@
child.on('error', (err) => {
@@
if (err.code === 'ENOENT') {
+ fallbackActive = true;
const nodeChild = spawn(
'node',
['--import', 'tsx', detectorPath, ...args],
{ stdio: 'inherit', cwd: process.cwd() }
);
@@
nodeChild.on('exit', handleChildExit);
} else {
process.exit(1);
}
});
-child.on('exit', handleChildExit);
+child.on('exit', (code, signal) => {
+ if (fallbackActive) return;
+ handleChildExit(code, signal);
+});Also applies to: 70-70
🤖 Prompt for 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.
In `@karpeslop-cli.js` at line 64, The primary child exit handler
(handleChildExit) must be guarded so it doesn't terminate the wrapper when a
fallback is active: either remove nodeChild.on('exit', handleChildExit) before
starting the fallback or add a guard inside handleChildExit to ignore exits from
the original nodeChild when a fallbackActive flag is true; update the code paths
that start the fallback to set fallbackActive (or otherwise mark which child is
authoritative) and ensure handleChildExit only acts on the currently
authoritative child (reference nodeChild, handleChildExit, and the new/updated
fallbackActive/authoritative-child marker).
|
Pushed 8f6af27.
Smoke-tested:
|
|
Pushed b1199cd.
Smoke-tested:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
ai-slop-detector.ts (3)
1204-1206:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftModern lockfile scans still miss non-hoisted transitive packages.
Lines 1204-1206 drop every
pkg.packagesentry below the firstnode_modules/level. In npm v2/v3 lockfiles, that's exactly where version-conflicted transitive deps live, so a fresh package that only resolves atnode_modules/a/node_modules/bis never checked. Legacy v1 parsing walks nested dependencies, so the modern branch is currently less complete than the fallback.Prefer collecting all
pkg.packagesentries and deduping bypkgName|versioninstead of excluding nested paths up front.Also applies to: 1251-1257
🤖 Prompt for 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. In `@ai-slop-detector.ts` around lines 1204 - 1206, The code currently skips any pkg.packages entries with nested "node_modules/" by checking pkgPath and splitting on 'node_modules/', which omits non-hoisted transitive packages; remove those early-continue checks and instead collect every pkg.packages entry, compute a dedupe key like `${pkgName}|${version}` (using the package name and resolved version from each pkg entry), track seen keys in a Set, and only process a package if its key hasn't been seen; apply this change at both places where pkgPath-based skipping occurs (the blocks referencing pkgPath and pkg.packages) so nested node_modules entries are considered but final processing is deduplicated by pkgName|version.
1296-1315:⚠️ Potential issue | 🟠 Major | ⚡ Quick winShort-circuit remaining npm lookups after the first registry outage.
Once the fetch path starts failing, this method only suppresses duplicate warnings. Every later uncached package still pays its own 5s timeout, so an offline run against a large lockfile can stall for minutes even though the warning says freshness checks "will be skipped."
Suggested fix
private npmPackageCache: Map<string, Record<string, string>> = new Map(); private registryWarningLogged = false; + private registryUnavailable = false; private async getNpmPackageAge(pkgName: string, version: string): Promise<{ version: string; time: string; ageMs: number } | null> { + if (this.registryUnavailable) { + return null; + } + const now = Date.now(); @@ } catch { + this.registryUnavailable = true; if (!this.registryWarningLogged) { console.warn('⚠️ Could not reach npm registry to check package ages. fresh_package_version checks will be skipped.'); this.registryWarningLogged = true; } return null;🤖 Prompt for 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. In `@ai-slop-detector.ts` around lines 1296 - 1315, The package-age fetch path doesn't short-circuit after the first npm registry outage, causing repeated 5s timeouts; add a persistent flag (e.g., this.registryUnavailable) and check it at the start of the method that performs the fetch so you immediately return null when set, and set this.registryUnavailable = true inside the catch block (in addition to the existing this.registryWarningLogged update) so subsequent calls skip the network lookup entirely; reference the existing this.registryWarningLogged usage and the fetch block in the method to locate where to add the early-return check and where to set the new flag.
573-577:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't bypass built-in exclusions for outside-root targets.
Lines 573-577 short-circuit all of the segment-based checks below, so explicit targets like
../other/dist/index.js,../other/.git/hooks/*, or../other/types/foo.tsare no longer filtered.resolveTargetPaths()uses this as its last gate, which means outside-repo scans can pull in generated/build artifacts and hidden files that the normal scan would skip.Suggested fix
- if (allowOutsideRoot && isOutsideRoot) { - return relativePath.endsWith('.d.ts') || - relativePath.endsWith('ai-slop-detector.ts') || - relativePath.endsWith('improved-ai-slop-detector.ts'); - } + const candidatePath = (allowOutsideRoot && isOutsideRoot ? filePath : relativePath) + .replace(/\\/g, '/'); - const segments = relativePath.split('/'); + const segments = candidatePath.split('/'); const excludedSegment = (name: string) => segments.includes(name); return excludedSegment('generated') || excludedSegment('coverage') || excludedSegment('.next') || excludedSegment('node_modules') || excludedSegment('dist') || excludedSegment('build') || excludedSegment('.git') || excludedSegment('out') || excludedSegment('temp') || excludedSegment('types') || segments.some(segment => segment.startsWith('.')) || - relativePath.endsWith('.d.ts') || - relativePath.endsWith('ai-slop-detector.ts') || - relativePath.endsWith('improved-ai-slop-detector.ts'); + candidatePath.endsWith('.d.ts') || + candidatePath.endsWith('ai-slop-detector.ts') || + candidatePath.endsWith('improved-ai-slop-detector.ts');🤖 Prompt for 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. In `@ai-slop-detector.ts` around lines 573 - 577, The current early return in the allowOutsideRoot && isOutsideRoot branch (using relativePath.endsWith('.d.ts') || relativePath.endsWith('ai-slop-detector.ts') || relativePath.endsWith('improved-ai-slop-detector.ts')) bypasses the segment-based exclusions used elsewhere and lets outside-root paths skip normal filters; change this so that when allowOutsideRoot && isOutsideRoot you still run the same segment-based checks that resolveTargetPaths() applies and only then permit .d.ts or the detector files — i.e., remove the unconditional return and instead combine the existing segment-filter logic with a final allowance of those specific filenames so generated/build/hidden files are still excluded; refer to allowOutsideRoot, isOutsideRoot, relativePath, and resolveTargetPaths() to locate and integrate the fix.
🤖 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.
Outside diff comments:
In `@ai-slop-detector.ts`:
- Around line 1204-1206: The code currently skips any pkg.packages entries with
nested "node_modules/" by checking pkgPath and splitting on 'node_modules/',
which omits non-hoisted transitive packages; remove those early-continue checks
and instead collect every pkg.packages entry, compute a dedupe key like
`${pkgName}|${version}` (using the package name and resolved version from each
pkg entry), track seen keys in a Set, and only process a package if its key
hasn't been seen; apply this change at both places where pkgPath-based skipping
occurs (the blocks referencing pkgPath and pkg.packages) so nested node_modules
entries are considered but final processing is deduplicated by pkgName|version.
- Around line 1296-1315: The package-age fetch path doesn't short-circuit after
the first npm registry outage, causing repeated 5s timeouts; add a persistent
flag (e.g., this.registryUnavailable) and check it at the start of the method
that performs the fetch so you immediately return null when set, and set
this.registryUnavailable = true inside the catch block (in addition to the
existing this.registryWarningLogged update) so subsequent calls skip the network
lookup entirely; reference the existing this.registryWarningLogged usage and the
fetch block in the method to locate where to add the early-return check and
where to set the new flag.
- Around line 573-577: The current early return in the allowOutsideRoot &&
isOutsideRoot branch (using relativePath.endsWith('.d.ts') ||
relativePath.endsWith('ai-slop-detector.ts') ||
relativePath.endsWith('improved-ai-slop-detector.ts')) bypasses the
segment-based exclusions used elsewhere and lets outside-root paths skip normal
filters; change this so that when allowOutsideRoot && isOutsideRoot you still
run the same segment-based checks that resolveTargetPaths() applies and only
then permit .d.ts or the detector files — i.e., remove the unconditional return
and instead combine the existing segment-filter logic with a final allowance of
those specific filenames so generated/build/hidden files are still excluded;
refer to allowOutsideRoot, isOutsideRoot, relativePath, and resolveTargetPaths()
to locate and integrate the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea5218df-2e8f-4454-b354-59e4492cf617
📒 Files selected for processing (2)
ai-slop-detector.tskarpeslop-bin.js
🚧 Files skipped from review as they are similar to previous changes (1)
- karpeslop-bin.js
|
Pushed 22c1a35.
Smoke-tested:
|
|
Pushed 89b0de9.
Smoke-tested:
|
|
Pushed d0314a5.
Smoke-tested:
|
|
Added mock-free behavior coverage in Covered:
Also added a main-module guard so the source can be imported by tests without auto-running the CLI. |
…ment-based exclusions, and parseVersionRange scope
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI Incremental review completed. |
User description
Adds
[path...]positional arguments to KarpeSlop so it can target specific files or directories instead of always globbing the entire project.Closes #14 — the original issue framed the problem as '--strict mode should focus on one file', but the real gap was that single-file targeting didn't exist at all. Once it exists, --strict correctly reports which targeted file(s) had critical issues.
Changes
targetPathsfield and constructor parameter toAISlopDetectorresolveTargetPaths()to validate, filter, and expand CLI pathsdetect()to usetargetPathswhen provided instead offindAllFiles()runIfMain()--separator so file paths starting with-aren't misclassified as flags--strictto report which file(s) had critical issues when targeting[path...]usage and examplesRefactors
getGlobIgnorePatterns()andisExcludedPath()sofindAllFiles()andresolveTargetPaths()share the same exclusion logicisExcludedPathwith De Morgan's law; add missing exclusion patterns (dotfiles,/types/,.d.ts)types/to avoid substring false positives (e.g.,subtypes/,mytypes/)Usage
Summary by CodeRabbit
New Features
--separatorBug Fixes
Refactor
Other
CodeAnt-AI Description
Check package freshness and target specific files or folders from the CLI
What Changed
package.jsonandpackage-lock.jsonfor recently published npm versions and warns when a dependency is too new to trust--is supported so paths that start with-are treated as targets, not flags--strictnow names the targeted file(s) that contain critical issuesImpact
✅ Faster single-file scans✅ Fewer false flag parsing failures✅ Clearer strict-mode failures💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.