Skip to content

Conversation

@data-douser
Copy link
Collaborator

What This PR Contributes

This pull request introduces significant improvements to the CDS extractor diagnostics system, with a focus on ensuring that all diagnostic file paths are reported as relative to the source root. This change enhances compatibility with CodeQL requirements and improves the clarity of diagnostic messages. The update affects both the implementation in source files and the corresponding test cases, and adds new utility logic for path conversion.

Diagnostic Path Handling Improvements

  • All diagnostic functions in src/diagnostics.ts now accept an optional sourceRoot parameter and use the new convertToRelativePath utility to ensure that diagnostic file paths are relative to the source root, defaulting to . if the file is outside the source root. [1] [2] [3] [4]
  • All calls to diagnostic functions in cds-extractor.ts, src/codeql.ts, and src/cds/compiler/retry.ts have been updated to pass the appropriate sourceRoot argument, ensuring consistent diagnostic reporting. [1] [2] [3] [4] [5] [6] [7] [8] [9]

Diagnostic Utility Enhancements

  • Added convertToRelativePath in src/diagnostics.ts to robustly resolve file paths relative to the source root, with fallback logic for files outside the root.
  • All diagnostic helpers (addCompilationDiagnostic, addDependencyGraphDiagnostic, addDependencyInstallationDiagnostic, addEnvironmentSetupDiagnostic, addJavaScriptExtractorDiagnostic, addNoCdsProjectsDiagnostic) now support relative path conversion via the new utility. [1] [2] [3] [4] [5] [6] [7] [8]

Test Suite Updates

  • All affected tests in test/src/cds/compiler/retry.test.ts, test/src/codeql.test.ts, and test/src/diagnostics.test.ts have been updated to include the new sourceRoot parameter in diagnostic function calls, ensuring correct test coverage of the new logic. [1] [2] [3] [4]

Documentation and Instructions

  • Added .github/instructions/extractors_cds_tools_ts.instructions.md to clearly document requirements, preferences, and constraints for CDS extractor TypeScript source and test files, including the new mandate for relative diagnostic paths.

References:
[1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17] [18] [19] [20] [21] [22] [23] [24] [25] [26]

Future Works

None

Creates the `.github/instructions/` directory and adds the first
`*.instructions.md` file intended for use by Copilot (or other LLM)
in reviewing and maintaining code for this repository.
Updates the CDS extractor source code and unit tests in order
to ensure that tool-level diagnostics only ever use relative
paths.
@data-douser data-douser marked this pull request as ready for review September 26, 2025 23:10
@data-douser data-douser changed the title Data douser/cds extractor maintenance 1 Fix CDS extractor database diagnostics to point to source-relative file paths Sep 26, 2025
@data-douser data-douser requested a review from knewbury01 October 6, 2025 17:36
data-douser and others added 3 commits October 15, 2025 14:02
Improves the diagnostic messages created by the CDS extractor
reports for the edge case where some file path is associated
with a diagnostic warning or error but is not a path within the
source root directory that the CDS extractor was configured to
scan. This change attempts to continue to prevent path injection
and path traversal attacks for any diagnostics generated by the
CDS extractor while ensuring the unlinkability of this edge case
is explained to any user viewing such diagnostics. We don't expect
to encounter situations where a diagnostic error or warning is
reported for any file outside of the scanned source root directory,
but we want to handle such situations well and we do so here by
improving the text of our diagnostic message to the user without
giving the user a link to a non-repo file.
Copy link
Contributor

@jeongsoolee09 jeongsoolee09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks. @data-douser Can you add some details in the PR body on how this PR makes the extractor chain to keep going even when the dependencies fail to be installed?

@data-douser data-douser merged commit ad612bb into main Oct 20, 2025
6 checks passed
@data-douser data-douser deleted the data-douser/cds-extractor-maintenance-1 branch October 20, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants