feat: add --debug flag for verbose network and runtime logging#471
Conversation
|
@sonukapoor please review the PR when you have free time |
There was a problem hiding this comment.
Pull request overview
Adds a maintainer-focused --debug flag to emit verbose runtime/network diagnostics to stderr while keeping normal CLI output (including JSON/SARIF/CDX) clean on stdout.
Changes:
- Introduces a reusable debug logger (
createDebugLogger) and wires--debugthrough CLI args/options and the main runtime. - Adds debug tracing for cache hit/miss decisions and OSV request/response/error lifecycle.
- Adds/updates Jest coverage to verify debug logging behavior and arg parsing.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/output.test.ts | Adds a test ensuring debug logs write to stderr only when enabled. |
| tests/helpers.test.ts | Extends parseArgs tests to assert --debug parsing. |
| src/types.ts | Adds debug?: boolean to parsed CLI options typing. |
| src/scanner.ts | Creates debug logger per scan and logs cache hit/miss; passes logger into advisory source. |
| src/output/debug.ts | New debug logger utility that writes formatted debug lines to console.error. |
| src/index.ts | Wires --debug into runtime logging (package parsing summary + stack trace on fatal errors). |
| src/cli/help.ts | Documents --debug (and additionally updates help text for other commands/options). |
| src/cli/args.ts | Adds --debug flag parsing. |
| src/advisory/osv-advisory-source.ts | Adds debug logging around OSV requests/responses/errors (including timing). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5807734 to
ef2b1e4
Compare
1e609d6 to
ec46ef1
Compare
|
@sonukapoor please review the PR when you have free time |
sonukapoor
left a comment
There was a problem hiding this comment.
Both issues from the last round are fixed — duplicate --ca-cert is gone and pluralize is used correctly. The implementation is solid: typed DebugLogger, no-op for disabled mode, comprehensive OSV request/response/error coverage, and good test coverage. Two small things before this merges.
|
Great start — the Output should go to a file, not stderr Right now When Everything else goes into the file only. This keeps the terminal output clean and gives you a persistent artefact you can share when reporting an issue. What's currently missing The debug output has two other problems today:
Proposed log format Every entry in the file should be a single JSON line with a timestamp prefix: Why CVE Lite sends up to 5 OSV requests concurrently via Without a correlation ID, a failed or hanging batch is impossible to diagnose — you can see a request was sent but you can't tell which packages were in it. With The same pattern applies to registry calls in fix command generation — each A simple counter is enough — no need for UUIDs. Just increment an integer per session and format it as Performance — debug mode must be a true no-op when disabled This is important. Every new log call added to hot paths like // Wrong — JSON.stringify runs on every scan regardless of debug mode
debugLog('OSV request', JSON.stringify(payload));
// Right — the object literal is only evaluated if debugLog is not a no-op
debugLog('OSV request', { batchId, queryCount, sample });Similarly, opening the log file, incrementing the Files that need changes
Happy to discuss any of this — the goal is a debug log that fits on one screen for a typical scan and gives you a clear timeline of exactly where time was spent and where something went wrong. |
|
@sonukapoor please review the PR when you have free time |
I will review this later today. |
sonukapoor
left a comment
There was a problem hiding this comment.
Good progress — the file-only output is exactly right, the batchId correlation on OSV requests/responses is clean, and the cache check summary (instead of per-package lines) is the correct call. The foundation is solid.
Before we merge, the log format is still missing a significant portion of what I outlined in my earlier comment. Comparing the proposed format against what the file produces today:
src/index.ts
The [debug] Writing debug log to ... line currently appears before the banner because createDebugLogger is called outside main(). Move it inside main(), after printBanner(), so it appears after "Fast. Local. Developer-first." in the output.
Also missing: the CLI started, Config loaded, and Advisory source entries from the proposed format. And when NODE_EXTRA_CA_CERTS is set (line 123), that should be logged — it's the first thing to check when diagnosing SSL proxy failures and right now it's completely invisible in the debug output.
The stack trace on error (line 341) goes to console.error which prints to the terminal. That should go to the log file instead — same as everything else.
src/scanner.ts
Missing: npm transitive graph, Workspace map, Findings classified, and Scan finished entries. These are all in the proposed format and give the timing and classification picture that makes the log useful for performance diagnosis.
src/remediation/npm-registry.ts
This file has no debug logging at all. fetchPackument has a catch {} that silently returns null — if the npm registry is unreachable or rate-limits, fix commands fail to generate with no trace in the log. Thread debugLog through fetchPackument, resolvePublishedFixVersion, and resolveLowestKnownNonVulnerableVersion and log each registry call, its outcome, and any errors.
src/utils/fix-runner.ts
runInstallCommand discards all stdout/stderr from the install process and debugLog isn't threaded through from index.ts at all. In debug mode, log the exact command being run and its exit code.
src/osv/cache.ts
Missing the Cache loaded entry with path and entry counts. Also the catch { return createEmptyCache() } fallback is completely silent — a corrupted or unreadable cache file produces no trace.
The goal is a log that covers the full lifecycle from startup to scan finished, with enough signal to diagnose network failures, cache issues, and fix command generation problems without having to add temporary console.log calls. The proposed format in the original comment covers exactly that — worth getting all of it in before merge.
16a7649 to
defb907
Compare
|
@sonukapoor please review the PR When you have free time |
sonukapoor
left a comment
There was a problem hiding this comment.
This is coming together really well. The lifecycle coverage is solid — Cache loaded, OSV request/response with batch IDs, Registry fetch, Fix command with exit codes, Findings classified, Scan finished — that is exactly the kind of structured trace that makes network and performance issues diagnosable without adding temporary console.log calls. The file-only output design is the right call too. Good work getting this far.
Two things to fix before this merges, then a few gaps in diagnostic coverage worth filling.
Commit message on defb907 — the message starts with [200~feat(debug)..., which is a bracketed paste escape code that leaked into git. Please squash or amend this before merge so the history is clean.
Redundant configureNpmRegistryDebug call in index.ts — lines 233-235:
if (options.debug) {
configureNpmRegistryDebug(debugLog);
}This is redundant. scanPackages already calls configureNpmRegistryDebug(log) unconditionally when !offline (scanner.ts line 103), and log is debugLog ?? (() => {}), so the same logger lands either way. Remove the index.ts call — every line needs to earn its place.
saveCache is silent — loadCache logs path and entry counts on every branch, but saveCache writes nothing. A user trying to diagnose "why isn't my cache persisting" would see Cache loaded on the way in but no confirmation on the way out. Add a Cache saved entry to saveCache with the path and entry counts.
--usage and --only-used have no debug coverage — in scanProject, when options.usage is set there's a filesystem scan with no timing logged. When options.onlyUsed is set, findings are silently filtered with no trace. Log a Usage scan entry with timing and matched package count, and a Findings filtered entry showing before/after counts when onlyUsed drops findings.
Scan finished is missing packageCount — currently just totalDurationMs and findings. Adding packages: params.scanInput.packages.length lets you correlate scan time with problem size, which is the first thing to check when someone reports a slow scan.
Zero-packages early exit leaves no trace — the packages.length === 0 path in index.ts calls process.exit(0) with no debug entry. A user running --debug on a folder with no recognizable lockfile would get a log that just stops. Add a Scan skipped entry before the early exit so the log tells a complete story.
e51dea9 to
4828655
Compare
|
@sonukapoor please review the PR when you have free time |
sonukapoor
left a comment
There was a problem hiding this comment.
Both previous issues are fixed: configureNpmRegistryDebug is no longer called from index.ts (scanner handles it), and saveCache now logs. The logging coverage across loadCache, OsvAdvisorySource, and fix-runner is solid.
Two things before this is merge-ready:
1. The test writes real files to disk. In output.test.ts, createDebugLogger(true) followed by debug.log(...) actually calls fs.appendFileSync and creates a cve-lite-debug-*.log file in the project root during every test run. captureErrors only mocks console.error — the file write is not mocked. After each npm test, untracked log files will appear in the working tree.
Fix by unlinking the file after the test:
it("announces debug log file path to stderr when enabled", () => {
let logFile: string | undefined;
const enabledLines = captureErrors(() => {
const debug = createDebugLogger(true);
debug.log("cache hit");
debug.announcePath();
// capture filename from the announce line so we can clean up
logFile = enabledLines?.[0]?.match(/cve-lite-debug-[^\s]+/)?.[0];
});
// ... assertions ...
if (logFile && fs.existsSync(logFile)) fs.unlinkSync(logFile);
});Or mock fs.appendFileSync for the block so no file is created at all.
2. Commit history needs squashing. The branch has 8 commits, and commit 2df7ffc still has the ^[[200~feat(debug): bracketed-paste artifact in its message. Please squash all 8 commits into one clean commit and push with --force-with-lease.
|
@sonukapoor Please review the PR When you have free time |
sonukapoor
left a comment
There was a problem hiding this comment.
Test fix is exactly right — fs.appendFileSync is mocked before the enabled logger runs, the spy assertion proves the write path executes, and the finally cleanup is a good safety net. Everything looks good. Going ahead and squash-merging on our end to clean up the commit history.
Closes #393
Summary
Adds a
--debugflag tocve-litefor maintainer-focused diagnosticswithout affecting normal CLI output behavior.
When
--debugis enabled, the CLI emits verbose runtime/network logsto
stderr, including:Debug output is routed to
stderrso JSON/SARIF/CDX output streamsremain clean and machine-readable on
stdout.Motivation
Intermittent or machine-specific OSV/network failures are currently
invisible from CLI output alone. This flag gives maintainers a
structured trace to identify exactly where in the request lifecycle
a failure occurred, without impacting normal users.
Changes
src/output/debug.ts— reusable debug logger (writes tostderr)src/cli/args.ts—--debugflag parsingsrc/cli/help.ts— help text for--debugsrc/types.ts— debug option type definitionsrc/scanner.ts— debug logging for lockfile parsing and cache decisionssrc/advisory/osv-advisory-source.ts— debug logging for OSV requests/responsessrc/index.ts— wire debug flag through runtimeTest Plan
npm test -- tests/helpers.test.ts tests/output.test.ts✅npm test -- tests/cli-integration.test.ts✅No behavior changes for users who do not pass
--debug.