Repository navigation
Fix PNPM Workspace Dependencies Scan and Add New License Discovery System - #7
Conversation
📝 WalkthroughWalkthroughIntroduces comprehensive SPDX license handling: new SPDX generator and license module, type/schema bump to 1.2, build integration, version-aware package resolution and license-text extraction, aggregator license composition (declared/inferred/status/risk), and UI updates to render and filter structured license data. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (runs build)
participant Build as build-spdx
participant FS as src/generated/spdx.ts (generated)
participant Aggregator as src/aggregator.ts
participant Utils as src/utils.ts
participant License as src/license.ts
participant UI as report-ui/main.ts
Dev->>Build: run npm run build:spdx
Build->>FS: fetch SPDX JSON, generate Sets
FS-->>Aggregator: provide SPDX lookup data (import)
Aggregator->>Utils: resolvePackageJsonPath(name, paths, version)
Utils-->>Aggregator: package.json path, licenseFile, licenseText
Aggregator->>License: validateSpdxExpression(declared), inferLicenseFromText(licenseText)
License-->>Aggregator: SpdxValidationResult, inference match, normalized IDs
Aggregator->>Aggregator: buildLicenseInfo (status, licenseIds), pickLicenseRisk
Aggregator-->>UI: emit aggregated data with structured license object
UI->>License: format/resolvePrimaryLicense for display
UI-->>Dev: render report with declared/inferred/status/risk
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.dependency-radar/dependency-radar/import-graph.json (1)
1-81:⚠️ Potential issue | 🟠 MajorRemove temporary
.dependency-radar/artifacts from the PR.This looks like generated dependency metadata; please drop it from version control (or keep only when using
--keep-temp) and add/update ignore rules if needed.Based on learnings: Temporary files in
.dependency-radar/may contain dependency metadata; avoid committing them and remove unless debugging with--keep-temp.
🤖 Fix all issues with AI agents
In `@scripts/build-spdx.ts`:
- Around line 29-49: fetchJson currently can hang without a timeout and treats
any status <400 as OK; modify fetchJson to enforce a strict 2xx check and a
request timeout: when calling https.get capture the returned ClientRequest
(e.g., const req = https.get(...)), start a timer (e.g., 10s) that will
destroy/abort the request and reject the Promise with a timeout Error if fired,
and ensure the timer is cleared on successful 'end' and on 'error'; inside the
response handler validate res.statusCode is between 200 and 299 and reject (and
res.resume()) for any other status, and keep the existing 'error' handler but
make sure it also clears the timeout before rejecting.
In `@src/license.ts`:
- Around line 48-117: The guard in validateSpdxExpression currently calls
input.trim() before checking typeof string and also rejects any parentheses;
change the initial invalid.normalized assignment to use the raw trimmed value
only after confirming typeof input === 'string' (i.e., perform the type check
before trimming) and remove the early return for parentheses. Update the
tokenizer/loop that builds tokens/normalizedTokens to treat '(' and ')' as valid
tokens (allow '(' when expectTerm is true and ')' when not expecting a term),
track parentheses nesting to ensure they are balanced, and integrate parentheses
into the normalized output while keeping existing logic for normalizeSpdxId,
normalizeExceptionId, handling 'WITH', and operator validation (AND/OR) so
expressions like "MIT AND (LGPL-2.1-or-later OR BSD-3-Clause)" parse and
validate correctly.
🧹 Nitpick comments (2)
scripts/build-spdx.ts (1)
24-27: Pin SPDX source data for reproducible builds.
Pulling from a moving upstream branch can make builds non-deterministic; consider pinning to a tagged SPDX release/commit or a vetted snapshot with an explicit refresh step.dependency-radar.json (1)
1-4612: Consider normalizing environment-specific fields if this is a committed fixture.Fields like
generatedAt,branch, andprojectDirare machine-specific and will churn; if this file is meant as a stable sample, scrub or relocate it under test fixtures.
| async function fetchJson<T>(url: string): Promise<T> { | ||
| return new Promise((resolve, reject) => { | ||
| https | ||
| .get(url, (res) => { | ||
| if (res.statusCode && res.statusCode >= 400) { | ||
| reject(new Error(`Failed to fetch ${url} (${res.statusCode})`)); | ||
| res.resume(); | ||
| return; | ||
| } | ||
| const chunks: Buffer[] = []; | ||
| res.on('data', (d) => chunks.push(Buffer.from(d))); | ||
| res.on('end', () => { | ||
| try { | ||
| const raw = Buffer.concat(chunks).toString('utf8'); | ||
| resolve(JSON.parse(raw) as T); | ||
| } catch (err) { | ||
| reject(err); | ||
| } | ||
| }); | ||
| }) | ||
| .on('error', (err) => reject(err)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and read the relevant section
fd -e ts "build-spdx" | head -20Repository: JosephMaynard/dependency-radar
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Find and read the file
find . -name "build-spdx.ts" -type f 2>/dev/null | head -5Repository: JosephMaynard/dependency-radar
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Search for the fetchJson function
rg "fetchJson" -t ts -A 25 | head -100Repository: JosephMaynard/dependency-radar
Length of output: 3115
Add timeout + stricter status code validation in fetchJson.
The current implementation lacks a timeout mechanism, allowing the Promise to hang indefinitely on network stalls. Additionally, treating only status ≥ 400 as failures is too lenient—3xx redirects won't auto-follow and may produce unparseable responses; strict 2xx validation (200-299) is more robust.
🛠️ Suggested hardening
return new Promise((resolve, reject) => {
- https
- .get(url, (res) => {
- if (res.statusCode && res.statusCode >= 400) {
+ const req = https.get(url, (res) => {
+ if (!res.statusCode || res.statusCode < 200 || res.statusCode >= 300) {
reject(new Error(`Failed to fetch ${url} (${res.statusCode})`));
res.resume();
return;
}
const chunks: Buffer[] = [];
res.on('data', (d) => chunks.push(Buffer.from(d)));
res.on('end', () => {
try {
const raw = Buffer.concat(chunks).toString('utf8');
resolve(JSON.parse(raw) as T);
} catch (err) {
reject(err);
}
});
- })
- .on('error', (err) => reject(err));
+ });
+ req.on('error', (err) => reject(err));
+ req.setTimeout(15_000, () => {
+ req.destroy(new Error(`Timeout fetching ${url}`));
+ });
});
}🤖 Prompt for AI Agents
In `@scripts/build-spdx.ts` around lines 29 - 49, fetchJson currently can hang
without a timeout and treats any status <400 as OK; modify fetchJson to enforce
a strict 2xx check and a request timeout: when calling https.get capture the
returned ClientRequest (e.g., const req = https.get(...)), start a timer (e.g.,
10s) that will destroy/abort the request and reject the Promise with a timeout
Error if fired, and ensure the timer is cleared on successful 'end' and on
'error'; inside the response handler validate res.statusCode is between 200 and
299 and reject (and res.resume()) for any other status, and keep the existing
'error' handler but make sure it also clears the timeout before rejecting.
1 participant
Summary by CodeRabbit
New Features
Chores