Fix/import identifications - #928
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a feature to import dependency identifications from another project alongside inventories: new API types, database queries, extraction/adapter logic, a new reuse task, IPC handler/service wiring, and a renderer UI checkbox with a dependencies preview table. Includes bug fixes, i18n strings, changelog, and version bump. ChangesDependency Identification Import Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectSelectorDialog
participant ProjectService
participant ProjectKnowledgeExtractor
participant ReuseDependencyIdentificationTask
participant DependencyService
ProjectSelectorDialog->>ProjectService: extractInventoryKnowledge(includeDependencies=true)
ProjectService->>ProjectKnowledgeExtractor: extractInventoryData()
ProjectKnowledgeExtractor-->>ProjectService: inventories + dependencies
ProjectService-->>ProjectSelectorDialog: ProjectKnowledgeExtractionResult
ProjectSelectorDialog->>ProjectService: acceptInventoryKnowledge(dependencyKnowledgeExtraction)
ProjectService->>ReuseDependencyIdentificationTask: run()
ReuseDependencyIdentificationTask->>DependencyService: acceptAllByIds(accepted dependencies)
ReuseDependencyIdentificationTask-->>ProjectService: done
ProjectService-->>ProjectSelectorDialog: accepted inventories
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/task/reuseIdentification/ReuseIdentificationTask.ts (1)
72-81: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a surviving file id for
source.input.fileMapper.get(Object.keys(this.reuseIdentification.inventoryKnowledgeExtraction)[0])can still beundefinedwhen the first extraction key has no local match. In overwrite mode that value reachesRestore, whereQueryBuilderFileIdIncalls.toString()on it and throws.🤖 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 `@src/main/task/reuseIdentification/ReuseIdentificationTask.ts` around lines 72 - 81, Update ReuseIdentificationTask so the Accept source.input never relies on a possibly missing fileMapper lookup when overwrite is enabled; the current fileMapper.get(Object.keys(this.reuseIdentification.inventoryKnowledgeExtraction)[0]) path can yield undefined and later break Restore via QueryBuilderFileIdIn. Use a surviving file id or a safe fallback derived from the available inventories/extraction keys inside ReuseIdentificationTask before constructing the Accept input, and keep the PATH branch unchanged.src/main/services/DependencyService.ts (1)
207-214: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAttach the dependency’s licenses to matched components
src/main/services/DependencyService.ts:207-214
comp.licensesis the component’s already-attached set, so this branch filters those licenses out and leavesattachempty for normal existing-component matches. Use the incoming dependency’sd.licenseshere, then diff against the component’s current attachments before callingattachLicensesToComponentBulk.🤖 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 `@src/main/services/DependencyService.ts` around lines 207 - 214, The attachment logic in DependencyService uses the component’s existing licenses instead of the dependency’s incoming licenses, so matched components never get new licenses attached. Update the forEach branch in DependencyService to build the attachment list from d.licenses, compare against the component’s current license attachments via licenseMapper, and then pass the resulting ids into attachLicensesToComponentBulk using the existing compid/attach flow.
🧹 Nitpick comments (1)
src/main/modules/projectKnowledge/ProjectKnowledgeExtractor.ts (1)
19-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale JSDoc return type.
The doc comment still says
@return InventoryKnowledgeExtraction, but the method now returnsProjectKnowledgeExtractionResult./** * `@brief` extracts the inventory data from external projects - * `@return` InventoryKnowledgeExtraction + * `@return` ProjectKnowledgeExtractionResult * */ public async extractInventoryData():Promise<ProjectKnowledgeExtractionResult> {🤖 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 `@src/main/modules/projectKnowledge/ProjectKnowledgeExtractor.ts` around lines 19 - 41, Update the JSDoc for extractInventoryData so the documented return type matches the actual ProjectKnowledgeExtractionResult returned by the method. Keep the comment aligned with the function signature and its returned inventories/dependencies object, and remove the stale InventoryKnowledgeExtraction type reference.
🤖 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 `@src/api/handlers/project.handler.ts`:
- Around line 223-225: The PROJECT handler is triggering async dependency status
updates without waiting for completion, so the response can be sent before tree
status changes are persisted. Update the logic around
treeService.updateDependencyStatusOnTree() in project.handler.ts to await the
call (and any returned promise chain) before continuing, so updateDone()
completes before the handler finishes and PROJECT_READ_TREE sees fresh status.
In `@src/main/model/project/models/ProjectKnowledgeModel.ts`:
- Around line 41-49: In extractProjectDependencyData, the LIKE filter is
building SQL by interpolating folder directly, which is vulnerable to malformed
paths and wildcard over-matching. Update the query construction to bind folder
as a parameter instead of string concatenation, and escape any % or _ characters
in the folder value before adding the trailing % pattern so only the intended
subtree matches.
In `@src/main/task/reuseIdentification/ReuseDependencyIdentificationTask.ts`:
- Around line 36-48: In ReuseDependencyIdentificationTask’s localDependencies
processing, the current guard only skips FileStatusType.IDENTIFIED entries when
overwrite is false, so ORIGINAL/rejected dependencies can still be re-imported
and unintentionally un-rejected. Update the filtering logic in the forEach block
to treat ORIGINAL the same as IDENTIFIED unless this.params.overwrite is true,
keeping rejected entries untouched unless an overwrite is explicitly requested.
---
Outside diff comments:
In `@src/main/services/DependencyService.ts`:
- Around line 207-214: The attachment logic in DependencyService uses the
component’s existing licenses instead of the dependency’s incoming licenses, so
matched components never get new licenses attached. Update the forEach branch in
DependencyService to build the attachment list from d.licenses, compare against
the component’s current license attachments via licenseMapper, and then pass the
resulting ids into attachLicensesToComponentBulk using the existing
compid/attach flow.
In `@src/main/task/reuseIdentification/ReuseIdentificationTask.ts`:
- Around line 72-81: Update ReuseIdentificationTask so the Accept source.input
never relies on a possibly missing fileMapper lookup when overwrite is enabled;
the current
fileMapper.get(Object.keys(this.reuseIdentification.inventoryKnowledgeExtraction)[0])
path can yield undefined and later break Restore via QueryBuilderFileIdIn. Use a
surviving file id or a safe fallback derived from the available
inventories/extraction keys inside ReuseIdentificationTask before constructing
the Accept input, and keep the PATH branch unchanged.
---
Nitpick comments:
In `@src/main/modules/projectKnowledge/ProjectKnowledgeExtractor.ts`:
- Around line 19-41: Update the JSDoc for extractInventoryData so the documented
return type matches the actual ProjectKnowledgeExtractionResult returned by the
method. Keep the comment aligned with the function signature and its returned
inventories/dependencies object, and remove the stale
InventoryKnowledgeExtraction type reference.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: aadc7595-d717-4cde-90b9-ee0320781f8d
📒 Files selected for processing (30)
CHANGELOG.mdassets/i18n/en/Common.jsonassets/i18n/en/Table.jsonassets/i18n/en/Title.jsonassets/i18n/es/Common.jsonassets/i18n/es/Table.jsonassets/i18n/es/Title.jsonassets/i18n/fr/Common.jsonassets/i18n/fr/Table.jsonassets/i18n/fr/Title.jsonassets/i18n/jp/Common.jsonassets/i18n/jp/Table.jsonassets/i18n/jp/Title.jsonassets/i18n/zh/Common.jsonassets/i18n/zh/Table.jsonassets/i18n/zh/Title.jsonrelease/app/package.jsonsrc/api/handlers/project.handler.tssrc/api/services/project.service.tssrc/api/types.tssrc/main/model/project/models/ProjectKnowledgeModel.tssrc/main/model/queryBuilder/QueryBuilderMD5FileIn.tssrc/main/model/querys_db.tssrc/main/modules/projectKnowledge/ProjectKnowledgeExtractor.tssrc/main/modules/projectKnowledge/projectKnowledgeAdapters/projectKnowledgeAdapter.tssrc/main/services/DependencyService.tssrc/main/services/ProjectService.tssrc/main/task/reuseIdentification/ReuseDependencyIdentificationTask.tssrc/main/task/reuseIdentification/ReuseIdentificationTask.tssrc/renderer/ui/dialog/ProjectSelectorDialog.tsx
Comment on lines
+223
to
+225
| if (param.dependencyKnowledgeExtraction && Object.keys(param.dependencyKnowledgeExtraction).length > 0) { | ||
| treeService.updateDependencyStatusOnTree(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check TreeService.updateDependencyStatusOnTree signature/implementation
rg -n -A15 'updateDependencyStatusOnTree' src/main/servicesRepository: scanoss/sbom-workbench
Length of output: 1308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## TreeService.ts\n'
ast-grep outline src/main/services/TreeService.ts --view expanded
printf '\n## project.handler.ts\n'
ast-grep outline src/api/handlers/project.handler.ts --view expanded
printf '\n## PROJECT_READ_TREE references\n'
rg -n -A20 -B10 'PROJECT_READ_TREE|updateDependencyStatusOnTree|updateDependencyStatus\(' srcRepository: scanoss/sbom-workbench
Length of output: 22040
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## TreeService helpers\n'
sed -n '1,140p' src/main/services/TreeService.ts
printf '\n## file/dependency model write signatures\n'
rg -n -A2 -B2 'restore\(|identified\(|ignored\(|getStatus\(' src/main -g '!**/dist/**' -g '!**/build/**'Repository: scanoss/sbom-workbench
Length of output: 19224
src/api/handlers/project.handler.ts:223-225 — Await treeService.updateDependencyStatusOnTree()
updateDependencyStatusOnTree() kicks off async work and returns immediately, so this handler can reply before the dependency status update and updateDone() finish. A subsequent PROJECT_READ_TREE can then see stale statuses.
🤖 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 `@src/api/handlers/project.handler.ts` around lines 223 - 225, The PROJECT
handler is triggering async dependency status updates without waiting for
completion, so the response can be sent before tree status changes are
persisted. Update the logic around treeService.updateDependencyStatusOnTree() in
project.handler.ts to await the call (and any returned promise chain) before
continuing, so updateDone() completes before the handler finishes and
PROJECT_READ_TREE sees fresh status.
Comment on lines
+41
to
+49
| public async extractProjectDependencyData(projectPath: string, folder: string){ | ||
| const db = await this.openDb(); | ||
| const call = await this.attach(db,projectPath); | ||
| const query = `${queries.SQL_GET_KNOWLEDGE_DEPENDENCIES} AND target.path LIKE '${folder}%';`; | ||
| const dependencies = await this.getInventories(db,query); | ||
| await call(`DETACH DATABASE aux`); | ||
| db.close(); | ||
| return dependencies; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
git ls-files src/main/model/project/models/ProjectKnowledgeModel.ts src/main/model | sed -n '1,120p'Repository: scanoss/sbom-workbench
Length of output: 2828
🏁 Script executed:
sed -n '1,220p' src/main/model/project/models/ProjectKnowledgeModel.tsRepository: scanoss/sbom-workbench
Length of output: 2379
🏁 Script executed:
rg -n "getInventories\\(" src/main -g '*.ts'Repository: scanoss/sbom-workbench
Length of output: 524
🏁 Script executed:
rg -n "folder" src/main -g '*.ts'Repository: scanoss/sbom-workbench
Length of output: 14807
Bind folder instead of interpolating it into the LIKE clause, and escape %/_ before appending %. A quoted path can alter the SQL, and wildcard characters in real folder names will over-match.
🤖 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 `@src/main/model/project/models/ProjectKnowledgeModel.ts` around lines 41 - 49,
In extractProjectDependencyData, the LIKE filter is building SQL by
interpolating folder directly, which is vulnerable to malformed paths and
wildcard over-matching. Update the query construction to bind folder as a
parameter instead of string concatenation, and escape any % or _ characters in
the folder value before adding the trailing % pattern so only the intended
subtree matches.
Comment on lines
+36
to
+48
| localDependencies.forEach((dep: any) => { | ||
| // Skip already identified dependencies unless overwriting | ||
| if (dep.status === FileStatusType.IDENTIFIED && !this.params.overwrite) return; | ||
| const extracted = extractedByKey.get(`${dep.path}-${dep.purl}`); | ||
| if (!extracted) return; | ||
| const licenses = extracted.licenses ? extracted.licenses.split(',').filter((l) => l) : []; | ||
| // A declared dependency identification requires a license; skip incomplete ones | ||
| if (licenses.length === 0) return; | ||
| dep.version = extracted.version; | ||
| dep.licenses = licenses; | ||
| dep.scope = extracted.scope; | ||
| dependenciesToAccept.push(dep); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## FileStatusType references\n'
rg -n "FileStatusType\.(ORIGINAL|IDENTIFIED|PENDING)|rejectedAt|status\s*===\s*FileStatusType\.ORIGINAL" src -g'*.ts'
printf '\n## Candidate files\n'
git ls-files 'src/main/task/**/Reuse*Task.ts' 'src/**/FileStatusType*.ts' 'src/**/status*.ts' | sed -n '1,120p'
printf '\n## Outline of target files\n'
for f in \
src/main/task/reuseIdentification/ReuseDependencyIdentificationTask.ts \
src/main/task/reuseIdentification/ReuseIdentificationTask.ts
do
if [ -f "$f" ]; then
echo "FILE: $f"
ast-grep outline "$f" --view expanded
fi
doneRepository: scanoss/sbom-workbench
Length of output: 8338
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## FileStatusType references\n'
rg -n "FileStatusType\.(ORIGINAL|IDENTIFIED|PENDING)|rejectedAt|status\s*===\s*FileStatusType\.ORIGINAL" src -g'*.ts'
printf '\n## Candidate files\n'
git ls-files 'src/main/task/**/Reuse*Task.ts' 'src/**/FileStatusType*.ts' 'src/**/status*.ts' | sed -n '1,120p'
printf '\n## Target file slices\n'
for f in \
src/main/task/reuseIdentification/ReuseDependencyIdentificationTask.ts \
src/main/task/reuseIdentification/ReuseIdentificationTask.ts
do
if [ -f "$f" ]; then
echo "FILE: $f"
sed -n '1,220p' "$f" | nl -ba | sed -n '1,220p'
fi
doneRepository: scanoss/sbom-workbench
Length of output: 6621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
set -x
rg -n "enum FileStatusType|FileStatusType" src -g'*.ts' || true
rg -n "rejectedAt" src -g'*.ts' || true
rg -n "status: FileStatusType.ORIGINAL|status === FileStatusType.ORIGINAL|FileStatusType.ORIGINAL" src -g'*.ts' || trueRepository: scanoss/sbom-workbench
Length of output: 11180
Skip ORIGINAL dependencies when overwrite is false
ORIGINAL maps to rejected entries (rejectedAt != null), so this import can silently un-reject previously rejected dependencies. Treat it like IDENTIFIED unless overwrite is enabled.
🤖 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 `@src/main/task/reuseIdentification/ReuseDependencyIdentificationTask.ts`
around lines 36 - 48, In ReuseDependencyIdentificationTask’s localDependencies
processing, the current guard only skips FileStatusType.IDENTIFIED entries when
overwrite is false, so ORIGINAL/rejected dependencies can still be re-imported
and unintentionally un-rejected. Update the filtering logic in the forEach block
to treat ORIGINAL the same as IDENTIFIED unless this.params.overwrite is true,
keeping rejected entries untouched unless an overwrite is explicitly requested.
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 by CodeRabbit
New Features
Bug Fixes
Documentation