fix(sarif): drop schema-invalid fixes — GitHub rejected every upload - #45
Conversation
SARIF 2.1.0 requires every `fix` object to carry `artifactChanges` — a fix IS a concrete edit (artifactLocation + replacements). We emitted `fixes[].description` alone, so GitHub's code-scanning upload rejected the whole run: instance.runs[0].results[1].fixes[0] requires property "artifactChanges" Every result carrying a suggestion tripped it, so no OikosBot SARIF has ever been ingestible. Caught by the enaction-engine pilot (metadatastician/enaction-engine#21) — the analysis step succeeded and the upload step failed. OikosBot's suggestions are advisory prose ("use a hash map for O(1) lookup"), not text replacements, so `fix` was the wrong construct entirely. They now ride in the result message and in `properties.suggestion`. The `Fix` type is removed with a note so it is not reintroduced. Added a regression test asserting no emitted fix lacks artifactChanges. Note the trap it guards: serde_json parsing successfully proves only that the output is valid JSON, NOT that it satisfies the SARIF schema — that distinction is what let this ship. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime. Code Review ✅ ApprovedRemoves schema-invalid SARIF
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
1 participant
No OikosBot SARIF has ever been ingestible by GitHub code scanning.
SARIF 2.1.0 requires every
fixobject to carryartifactChanges. We emittedfixes[].descriptionalone, so the upload API rejects the entire run:Every result with a suggestion trips it. Found by the enaction-engine pilot (metadatastician/enaction-engine#21): the analysis step succeeded, the container worked, and the upload step failed — which is precisely why we piloted one repo before sweeping seventeen.
Fix: OikosBot's suggestions are advisory prose, not text replacements, so
fixwas the wrong SARIF construct. They now appear in the result message andproperties.suggestion. TheFixtype is deleted with an explanatory note.Regression guard: a test asserts no emitted fix lacks
artifactChanges. Worth noting the trap it closes —serde_json::from_strsucceeding proves only parseable JSON, not schema-valid SARIF. I relied on that weaker check earlier and it is what let this ship.Verified: 51 workspace tests pass. True verification comes after merge, when
:latestrepublishes and the pilot re-runs against GitHub's own validator.🤖 Generated with Claude Code