feat(safegres): drive source, outputs and baselines from the config file - #1615
Merged
Conversation
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
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
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
Everything a CI job needs already exists as a flag, which is why every repo that uses safegres ends up with a wrapper script holding the flag string (
constructive-db/bin/safegres-check.cjsis 181 lines of exactly that). The four things a job repeats every run had no config-file equivalent: where the database comes from, where artifacts go, which baseline to diff, and whether to gate on new debt. They do now, so the job is one word:$ safegres audit # or `safegres lint`, the new alias, in a package.json scriptoutputs.dir(and--out <dir>) writessafegres.{json,md,sarif}, so nobody has to remember which extension goes with which renderer; a namedoutputs.jsonstill overrides its own file. Directories are created — CI should not have tomkdir -p.Path semantics, the one decision here. A path from the config file resolves against that file; a path from a flag stays relative to cwd. Otherwise a job that
cds into a package reads a different baseline than the one committed next to the rules, and--write-json out.jsonstops meaning what every other CLI means by it. Flags win when both are given.Merging is one pure function,
resolveRunPaths(argv, config, configDir)incli/shared.ts, so the precedence is unit-tested rather than asserted eight times inline in the command.Two escapes, now that a committed file can gate. The same file is read by more than one job, so both directions need a way out:
source.pgpm—safegres audit --database stagingaudits staging instead of deploying a throwaway copy of the same schema.PGDATABASEdoes not count: it is set on every runner.--report-onlyevaluates and reports every gate, then exits 0 — how to run a gated config as an advisory job against a second database, where the file's baseline doesn't apply.Nothing about analysis or scoring moves: the new keys aren't in the fingerprint's scoring surface (it's a whitelist), so an identical database still fingerprints identically. Sealed runs are unaffected — they don't read a config file, so they can't pick up a
baselineor asourceeither.Also a
## Featureslist near the top of the README, placed like pgsql-parser's.Next: this unblocks cutting a release and deleting
bin/safegres-check.cjsin constructive-db (planning #1370).Link to Devin session: https://app.devin.ai/sessions/b7874ecee0c7471ea271e6e7193869dc
Requested by: @pyramation