Use json module for JSON validation in output-cache - #4096
Conversation
1a0c458 to
93d793c
Compare
There was a problem hiding this comment.
Pull request overview
Refactors output-cache validation to use shared JSON schema utilities.
Changes:
- Validates version information using JSON schemas.
- Validates cache entries before accessing the cached version.
Show a summary per file
| File | Description |
|---|---|
src/cli/output-cache.ts |
Replaces manual checks with shared JSON validators. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 1/2 changed files
- Comments generated: 1
- Review effort level: Balanced
mbg
left a comment
There was a problem hiding this comment.
Looks OK, but a good catch from Copilot there that we missed on the previous PR. I also added a comment about avoiding the disconnect between the schema and VersionInfo type. When using the json module for schemas, it is desirable to derive the corresponding type with FromSchema to avoid them going out of sync.
|
@mario-campos Also looks like the linter check is failing again. If you haven't already enabled |
This makes it easier to test. Credit to @mbg.
That was a downstream (in That being said, I wonder if it would be better to define a JSON validator for |
jsonmodule to improve (make more readable) the validation of the output ofcodeql version.getCachedCodeQlVersionandcacheCodeQlVersionto accept the cache-file-path as an argument, rather than construct it; this makes it easier to test.VersionInfotype to be derived from its JSON schema.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, Code Quality, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist