* Add API changelog tooling: semantic OpenAPI diff with severity ranking
Answering "has the API changelog been updated?" currently means reading a
10k-line JSON diff by hand, so it falls to whoever is asked in Slack. This adds
the mechanical part of that job as two mage tasks.
`./bin/mage openapi-diff OLD.json NEW.json [--severity breaking]`
resources/openapi/openapi.json is generated from the Malli endpoint schemas, so
the spec at any git ref is what the API was at that ref. The task parses two
specs into {METHOD /path -> shape} maps, inlines $refs (component schemas change
too), and compares params, request body, and response schemas.
Findings are classified BREAKING / ADDITIVE / DOC_ONLY and sorted breaking-first.
A change is breaking when it requires MORE from the caller or provides LESS to
the caller; anything else is just a change. That rule is directional and it
inverts between request and response, so severity is decided from both schemas
rather than from how a change happens to render:
request breaking: newly required, type/enum narrowed, schema closed, field removed
additive: new OPTIONAL field or param, type/enum widened, made nullable
response breaking: field removed, field may now be null
additive: new field returned
Adding an optional parameter is not breaking; returning extra data is not
breaking. A public diff of two Stripe specs found 663 of 679 changes additive,
which is why filtering matters more than summarizing.
`./bin/mage openapi-staleness`
The self-healing CI job runs only on PRs labelled `openapi-self-healing`, so the
committed spec drifts; it is currently ~38 API commits behind. A stale spec makes
the diff under-report, so this reports drift and names the commits rather than
letting a clean diff read as "no changes". The label gate is deliberate (#67854
introduced it to stage the rollout after #67294 removed the spec entirely over
merge conflicts), so it is left alone here.
Response coverage is partial: ~199 response entries declare a schema and can be
compared, ~1,700 are description-only 2XX/4XX/5XX stubs and cannot be.
Tests cover both directions of every request and response rule, $ref resolution
including the SCIM schemas whose property is literally named "$ref", a guard
against the failure mode in mozilla-ai/otari#1315 (breaking markers dropped at
render time), and a signal-to-noise case asserting additive bulk cannot drown
the breaking few.
* Generate specs from git refs, so the diff is not bound to the committed blob
`./bin/mage openapi-diff --refs <old-ref> <new-ref>` checks each ref out into a
throwaway worktree, runs that ref's own `generate-openapi-spec`, and diffs the
two results. Nothing is committed and no checkout is touched.
This removes the staleness problem rather than reporting it. The committed
`openapi.json` only updates on PRs labelled `openapi-self-healing`, so it lags
source badly: `--committed origin/release-x.63.x origin/master` reports 0
findings, because both refs carry the same stale blob. Generating from source
reports 670 -> 714 operations and 10 removed endpoints for the same pair.
It needs no support from the ref being diffed beyond `generate-openapi-spec`
already existing, so it works on historical refs. `--committed` keeps the fast
path and now prints a warning that its result under-reports, since an empty diff
from a stale spec is indistinguishable from an API that did not change.
Validated against the real v63 -> master delta. The top removals are the three
`GET .../execute` endpoints that the hand-written 0.64 changelog documents as
replaced by `POST .../execute/values`, and it independently surfaces a query
param rename (`pinned_state` -> `pinned-state`) that has no entry yet.
That run also shows a precision limit worth stating: it reports 239 breaking
findings, but 217 are the identical `additionalProperties: false` line from one
PR (#82447, closing mu/defn argument schemas). The classification is right --
closing a schema does require more of the caller -- but as changelog input that
is one entry, not 217. The skill now tells the drafter to collapse findings by
text and treat a repeated finding as a single systematic change.
* Group identical findings so a systematic change reads as one entry
`--grouped` collapses findings with identical text across endpoints and sorts by
blast radius, because the per-endpoint view is the wrong shape for drafting a
changelog.
On the real v63 -> master delta it turns 239 breaking findings into 113 distinct
changes. The largest spans 217 endpoints and is a single upstream PR (#82447,
closing mu/defn argument schemas): one changelog entry, reported once with its
endpoints, instead of 217 identical lines to scroll past.
Grouping is a separate render mode rather than a transformation of the default.
The per-endpoint view answers "what changed on this route?", the grouped view
answers "what distinct changes happened?", and collapsing the former into the
latter would lose the ability to check a specific endpoint.
The long tail is where the individually-interesting entries live: the same run
surfaces a `pinned_state` -> `pinned-state` query param rename that no changelog
entry covers yet.
* Fix review findings: two defects let breaking changes pass silently
Eric's review found 13 issues. The two that mattered both caused false
negatives - the tool reporting nothing where a caller would break.
The depth guard counted nesting levels, not `$ref` hops. `seen` already breaks
cycles, so the counter was doing a different job than its name claimed: past 12
levels both specs collapsed to the literal "<deep>" and compared equal, hiding
every change in 52 of 637 operations. It now advances only on a `$ref` hop, and
52 falls to 17.
`response-lines` compared only top-level properties and delegated anything deeper
to `(widening? new old)`. Swapping those arguments inverts value-set semantics -
which is why the nullable case read correctly - but not object-property
semantics, so a removed nested response field read as additive. It now recurses
the way `body-lines` does.
Chasing the second one surfaced a third of the same shape: `widening?` never
compared array `items`, so a narrowing inside an array was invisible. Found by a
test written for the SCIM `$ref`-named-property case.
Also fixed:
- `committed-spec!` built a shell command by interpolation, so a ref containing
`$`, a quote, or a space produced a wrong file. Values now ride as positional
arguments and the shell never parses them.
- The `--grouped` header counted severity-filtered-out changes, inflating the
count above the body printed beneath it.
- The removed and added endpoint lists were severity-gated, so `--severity
breaking` hid the added half of an endpoint move - at exactly the severity the
skill tells a drafter to start with. Both lists now print at every severity.
- The staleness check watched 66 of the 158 files that contain `defendpoint`,
missing `src/metabase/*/api/*.clj` and all of `enterprise/backend/src`, so it
could report `OK: spec is current` with dozens of newer endpoint files on disk.
It also compared `%cI` strings across a history carrying both `+08:00` and `Z`,
and listed commits from a narrower pathspec than the date came from.
- `DOC_ONLY` was unreachable: a description-only change produced no findings and
was dropped. Descriptions are what the skill reads to spot changes it cannot
explain, so a reworded docstring now surfaces as doc-only.
- A misspelled `--severity` fell through to nil and silently printed everything.
It now exits. `--committed` without `--refs` was accepted and ignored; it now
exits too.
- Each `--refs` run left two multi-megabyte specs in /tmp.
- `brief` truncated its non-map branch without the "..." its map branch adds.
- The first commit message claimed a test covered the SCIM `$ref`-named property.
No such test existed; one exists now, along with tests for the depth guard, the
nested response removal, and the doc-only path.
One correction to that commit message stands: `babashka.json` would have avoided
the `:discouraged-namespace` ratchet entirely, since `mage/doctor.clj` already
uses it unsuppressed. Keeping cheshire for consistency with the three other mage
namespaces that use it; the budget moves 68 -> 69 against current master.
The `de-slop` skill the review could not find does exist, at
~/.claude/skills/de-slop - it is a personal skill, not in the repo.
* Fix second review: four more directions where a finding was inverted or absent
All nine findings addressed. The reviewer supplied a failing test for each of the
first five; all five reproduced on the previous head, and all now pass.
Three of the four major findings were the same mistake in different places: a
schema shape that the output path handed to `widening?` with swapped arguments,
which inverts value-set semantics but not object-property semantics.
- A response array of objects went to the leaf branch, so a removed element field
read as additive and an added one read as breaking. 24 of the 199 comparable
response schemas in the committed spec are top-level arrays. Arrays now recurse
through `items` in the output direction.
- `schema-props` unwraps a nullable `oneOf`, so a nullable OBJECT reached the
object branch, where only keys and requiredness were compared and the type
change was invisible. The committed spec has 726 nullable object properties.
Both `body-lines` and `response-lines` now compare type sets there, with the
severity test pointing the correct way for each direction.
- `response-lines` discarded the required sets, so a response field the API may
now omit produced nothing - the same loss of guarantee as one that may now be
null, which was already breaking. 174 of 637 operations declare `required` in a
response schema.
The fourth major finding was a shortcut in `widening?`: any empty old schema was
treated as "no constraint", so a field going from Malli `:any` to a concrete type
read as additive. `operations` stored an absent body as `{}`, which made "no body"
and "any value" indistinguishable; an absent body is now nil and `{}` falls
through to the type and enum checks. After ref resolution the committed spec has
1,182 property schemas that are `{}`.
The depth limit is gone rather than re-tuned. `seen` already terminates every
path against a finite component set, so the limit only ever replaced deep nodes
with the same literal on both sides - which compares equal and hides the change.
The MBQL clause schemas form chains long enough to hit it: 17 operations carried
`<deep>`, and now none do while 18 terminate on genuine cycles via `<recursive>`.
A self-diff of the committed spec costs 2.9s instead of 1.4s and still reports 0
findings.
Also fixed:
- `format-epoch` shelled out to `date -r`, which only BSD and macOS read as epoch
seconds; on GNU coreutils it means "the mtime of this file" and printed nothing,
silently, because the call was `:quiet?`. Now `java.time`.
- `openapi-staleness` claimed the spec was "current" from endpoint files alone.
661 of 763 component schemas live outside `.api` namespaces, so a shared-schema
change moves the generated spec invisibly. The OK message now says what it
actually checked and points at `--refs`.
- A `--refs` run that could neither generate nor read the second ref leaked the
first spec: materialization happened in the `let` bindings, outside the `try`
whose `finally` removes the directory.
- The skill told an agent that two identical committed specs prove the API did not
change, and that the commit touching `openapi.json` is the PR that changed the
API. Both are wrong for the same reason - the committed spec only moves on
`openapi-self-healing` PRs. It now says to re-run without `--committed` before
reporting no changes, and to search endpoint source for attribution.
* Drop the ratchet bump: babashka.json instead of cheshire, and fix stale docs
The `:discouraged-namespace` increase was avoidable and the rationale I gave for
it was backwards. `babashka.json` is not in the discouraged list, and 6 mage
namespaces already use it unsuppressed against 5 on cheshire - so the
"consistency" argument pointed the other way. Swapping to it removes both the
68 -> 69 bump and the `:clj-kondo/ignore`, and this PR no longer touches
.clj-kondo/ratchets.edn at all. `read-str` keywordizes by default, so the two
spec reads pass `{:key-fn identity}`; this namespace is string-key throughout.
Verified unchanged behavior: self-diff still 0 findings, the real 0.64 change
still 1 breaking.
Three docstring counts were wrong. The `api-source-pathspecs` docstring claimed
238 `/api/ee/` paths (actual 106 - it looks like 106 paths plus 132 operations,
double-counted), 66 files matching `src/metabase/**/api.clj` (actual 65), and 158
files containing `defendpoint` (actual 160). The argument the docstring makes is
unaffected - the narrow pathspec still misses most endpoint files - but the
numbers are now the measured ones.
Also removed documentation that described code this branch already deleted:
- A comment in `resolve-refs` said "Only a $ref hop advances depth" while the
docstring 17 lines above correctly says there is no depth limit. There is no
depth counter left; the comment described the previous commit's version.
- Two test comments and a test name referenced a `<deep>` placeholder guard that
no longer exists, and cited a "52 of 637" measurement of the mechanism that was
removed. The tests themselves are valid - they assert deep and long-chain
changes are detected - so only the prose changed.
- SKILL.md quoted "`--committed` reports 0 findings" for v63 vs master as a fixed
figure. That was true when written and is not now: the blobs have since
diverged and it reports 1. The underlying point stands, so it is stated
qualitatively rather than as a number that drifts.
- SKILL.md said `--refs` "works on any ref". The code only attempts that and has
a documented fallback for refs that will not build with the current toolchain;
the text now says so.
* Fix third review: object unions and nullable arrays hid breaking changes
- A union of two or more object variants was compared by its first
variant only, so removing or editing any other variant produced no
finding. Such unions now compare whole: widening only when every old
variant survives unchanged.
- Nullable arrays (`oneOf [array, null]`) skipped the array branch, so a
change to their items was ranked additive. Both array branches now
unwrap the null first.
- The render test never rendered. It now asserts breaking operations
print under the breaking headings of both renderers, and a new test
covers the --severity filter.
* Rank changes to unmodeled schema keywords breaking, and compare bounds, allOf, and tuples
The old widening check returned "safe" for any keyword it did not
recognize, so each review round found another way for a breaking change
to read as additive: allOf, minimum/minLength, map-of value schemas,
prefixItems, nullable enums, closed nullable objects.
`compatible?` replaces it. It takes a direction (:request or :response),
compares unions variant by variant, compares bounds, pattern, and format,
and compares allOf and prefixItems by position. A change to any keyword
it does not model ranks breaking. A false breaking finding costs one
look; a missed one ships undocumented.
Documentation keys (description, title, examples, default) are stripped
before comparison, so a reworded field description is doc-only instead
of an additive schema change.
Also:
- When two summaries are identical, the finding names where the schemas
differ (e.g. `items.enum: +"measure"`), so distinct changes no longer
merge in the grouped view.
- Generation deletes the checked-out spec before running, so a generator
that logs and swallows an error no longer passes the committed spec off
as fresh. A timeout falls back to the committed spec instead of
throwing.
- An empty or non-spec input file exits 1 instead of reporting 0 BREAKING.
- openapi-staleness lists commits after the spec commit, not including it.
* Close the gaps an independent check found in the compatible? rewrite
- Only a nullable union unwraps to its object. `string | object` and
similar unions are compared whole, so a change to the non-object
variant is no longer hidden.
- Keywords beside an object's properties (allOf, a map-of
additionalProperties, minProperties) are compared.
- If the field walk finds nothing but the stripped schemas differ, the
whole schema is ranked instead of falling through to doc-only.
- Inclusive and exclusive numeric bounds compare as one bound.
- A response allOf may gain members (narrowing output); a request allOf
may lose them.
- An enum or const without a type widening to its type is additive.
- requestBody.required and request content types are compared, and a
response that declares a schema for the first time is reported as
additive.
- A missing input file says so.
* Match operations across path-variable renames; count only visible severities in the grouped header
- Operations and path params are keyed by position, not variable name.
Renaming `{id}` to `{card-id}` leaves the URL a caller sends unchanged,
so it is now a doc-only "path variable renamed" finding rather than a
removed endpoint plus an added one. Routes that would merge under
position-keying (`/api/database/{id}/schemas` and
`/api/database/{virtual-db}/schemas`) keep their names. Reports still
print the real spec paths.
- At --severity breaking, the grouped header no longer counts the
new-endpoint list, which is additive. The list still prints.
- The skill's attribution section no longer opens with the
openapi.json git log that it then says names the wrong author.
* Rank a changed or dropped request default as breaking
`default` was stripped with the documentation keys, so a default that
flipped (say a query param from false to true) ranked doc-only and
--severity breaking hid it. The schema still validates the same
requests, but a caller who omits the field now gets different
behavior.
Request defaults are now compared: changed or dropped is breaking,
added where there was none is additive. Response defaults are ignored.
* State one definition of breaking that covers every rule
A change is breaking when an existing caller, sending exactly what it
sent before, can now fail or get a different result. The old wording
(requires more / provides less) did not cover changed defaults, and it
presented two conservative rules - removed request fields and changes
to unmodeled keywords - as if they followed from it. Those are now
stated as what the tool does when it cannot tell.
* Skill: point to docs-write, and attribute with all six API source pathspecs
de-slop is a personal skill the repo does not ship; docs-write carries
the Metabase style guide. The attribution git log used three of the six
pathspecs in api-source-pathspecs and missed enterprise api/ dirs and
routes.clj files (35 tracked files, e.g. sso/api/saml.clj).
* Remove stale :discouraged-namespace ignores
These six ignores suppressed a namespace-level policy that no longer
exists in config.edn -- clojure.pprint, clojure.java.io, clj-yaml.core,
metabase.util.jvm, and metabase.config.core aren't configured as
discouraged namespaces anymore (clojure.pprint is only discouraged at
the var level now). Verified clean with a scoped kondo run requesting
:discouraged-namespace at :warning over just these files: zero findings.
* Give :discouraged-var and :discouraged-namespace their own per-symbol ratchet budgets
One shared budget per linter let a new ignore on a previously-clean var
land silently as long as the linter's aggregate count had room, and made
"introduce a policy for just this one var" impossible. Each configured
symbol now gets its own field: :discouraged-var-counts and
:discouraged-namespace-counts, siblings of :ignore-counts, keyed to
mirror the symbol's own namespace and name rather than the linter
that discourages it (metabase.entity-retrieval.core/search-unfiltered
-> :entity-retrieval.core/search-unfiltered; an enterprise namespace of
the same shape gets an `ee.` prefix so it can't collide with its OSS
counterpart).
Attribution resolves which symbol an inline ignore covers by running a
scoped `clojure -M:kondo` analysis pass over the files that carry one of
these two ignores (JSON output, since a generated var can have a name
that isn't valid EDN) and matching each ignore to the nearest covered
usage of a known symbol -- this is the one thing in the ratchet tooling
that needs a live kondo run, so `./bin/mage kondo-ratchets` is no longer
pure Babashka. `:comment-exempt` is untouched: occurrences still carry
the bare linter name in :linters, only the two new count fields are
keyed per symbol.
`--seed :discouraged-var`/`:discouraged-namespace` (no specific symbol)
seeds every symbol currently configured under it in one shot, unused
ones at zero, the same ergonomics as landing a new linter. A stale flat
:ignore-counts entry for either linter is self-healing: `fix!` drops it
every run and `check` fails if one reappears, since it would otherwise
double-count every occurrence the new field already covers.
Ported onto the prod/test ratchet split: both ratchet files carry the
per-symbol fields, and each file's occurrences are attributed separately.
* Attribute discouraged-var ignores from kondo findings, not nearest usage
Matching each ignore to the nearest later usage of a configured symbol
counted usages kondo never flagged (scoped :config-in-ns entries, :off
overrides) and let a stale ignore claim an unrelated usage. Attribution
now lints copies of the files with the discouraged ignores disabled,
joins each finding to its var or namespace usage at the same row and
column, and charges it to the last ignore before it naming the linter.
An ignore counts once per file and distinct symbol it covers. The copies
rename the ignore key to one of the same length rather than blanking the
ignore, so every form reads as before and every row and column stays put.
A failed kondo run now throws instead of zeroing every budget. Findings
that resolve to no configured symbol fail the check and stop the shrink
from writing; ignores that cover no finding only warn. Without an
:attribute, a discouraged ignore throws rather than reading as zero.
Each ratchet file's occurrences are attributed separately.
`--seed :discouraged-var/clojure.core/println` seeds one symbol, a bare
`--seed :discouraged-var` seeds every symbol at its actual count, and an
empty suffix is rejected. Bulk seeding at zero is gone: the next shrink
dropped those zeros anyway. The shrink workflow runs prepare-backend,
since the shrink now needs the Clojure CLI.
Budgets are re-seeded from the new attribution. Several were undercounted
or charged to the wrong symbol before.
This squashes the unpushed review-fix commits on top of the per-symbol
commit, since #83383 rewrote the same functions each of them changed.
* Remove a stale :discouraged-namespace ignore on clj-yaml.core
clj-yaml.core is no longer a discouraged namespace, so the ignore on its
require in lib-be.metadata.jvm suppresses nothing.
* Don't charge findings kondo reports with the ignores in place
Attribution charged every discouraged finding on the disabled-ignore
copies to the last ignore before it, including findings no ignore
suppresses. Unsuppressed usages in dev/ inflated the eval, println and prn
budgets, and a new usage of a symbol an earlier ignore already covers
went uncounted.
Lint the original files in the same kondo run and drop any finding they
also report, so only findings an ignore suppresses are attributed. An
ignore nested inside another for the same linter still takes the
findings under it, leaving the outer one reported as covering nothing.
* Parse kondo's JSON output with babashka.json
mage already depends on babashka.json, so attribution doesn't need
cheshire or the :discouraged-namespace ignore that came with it. That
returns the cheshire.core budget to 4.
* Attribute prod and test ignores in one kondo run
check and fix! attributed the prod and test occurrences in separate
calls, and each call started its own JVM and lint pass. :attribute now
takes a seq of occurrence groups and returns one attribution per group,
so attribute-occurrences! lints every file once and splits the result.
kondo-ratchets drops from about 39s to 33s.
* Drop budgets for symbols no longer configured instead of refusing
Removing a symbol from config.edn left its per-symbol budget naming an
unknown key, and both check and the shrink threw on it. The shrink, the
tool meant to repair that drift, couldn't. Two PRs could each pass alone
and break the shrink on master: one seeding a symbol, the other removing
it from the config. check now warns about such a budget, and the shrink
drops it at zero like any unused budget, since attribution never charges
an unconfigured symbol. --seed still rejects an unconfigured symbol.
A bare --seed :discouraged-var no longer warns once per configured symbol
with no ignores, and check's ok line counts the per-symbol budgets
alongside :ignore-counts.
* Share the ratchet setup and config walks between their callers
check and fix! repeated the same setup: validate the linters, read the
module and test ratchet files, scan, split prod from test, and attribute.
They now share read-policies and scan-and-attribute. fix! still resolves
and validates --seed before the scan, so a bad seed fails without waiting
on kondo.
The repository-linter and discouraged-symbol lookups each walked
config.edn with an atom to collect nested :linters maps. Both now read
them from one linters-maps. The budget-line formatter is shared, and the
empty and mergeable field sets derive from ratchet-field-order. Dropped
the unused one-argument discouraged-symbols.
* Correct the ratchet task's runtime claim and plain up its wording
The kondo-ratchets :doc said the check takes seconds; with the kondo
attribution pass it takes about half a minute. The CLAUDE.md paragraph
and the ratchet-file headers no longer open sentences with code.
The unattributed-ignore warning now names a nested ignore for the same
linter as a cause: the inner ignore takes the finding, so the outer one
reads as covering nothing even though removing it would surface a
warning.
* Test that a failed kondo run throws rather than reading as no findings
If a failed or garbled kondo run read as an empty result, attribution
would count zero ignores and the shrink would drop every per-symbol
budget. The exit and parse checks move into parse-kondo-output, which
tests now cover for a nonzero exit and for output that isn't a map.
The attribution tests build matching findings and var usages with one
var-hits helper, and the check test's attribution stub reads as what it
returns.
* Refuse colliding discouraged symbols before an unseeded shrink
Dropping the unknown-key validation also dropped the shrink's only call
to discouraged-count-keys, which throws when two configured symbols share
a ratchet key. An unseeded shrink could then merge both symbols' ignores
into one budget. read-policies now reads the configured keys up front, so
check and fix! both refuse a collision before scanning.
* Reject kondo output without a findings list
A run that exits cleanly but prints a map with no :findings, such as {},
parsed as a result with no findings. Attribution would then count zero
ignores and the shrink would drop every per-symbol budget.
parse-kondo-output now requires :findings to be a list. A missing
analysis bucket already fails loudly, since every finding comes back
unresolved.
* Note where nested ignores leave a budget short
Attribution charges a finding to the last ignore before it, so an outer
ignore around a nested one for the same linter counts toward no budget.
State that next to the rule, where a reader of the code will look for it.
* Flag the nested-ignore undercount as a known gap
Lead the comment with the gotcha and its consequence, a budget one short
that an unattributed warning hides behind, so a reader doesn't mistake
the last-ignore rule for a complete one.
* Key aliased discouraged vars by the namespace the file requires
Kondo resolves a re-exported var to the namespace it learned the var came
from, and what it has learned depends on its cache and on which files
share the run. lib.core re-exports ->legacy-MBQL from lib.convert, so
after an ordinary lint of lib/core.cljc the same ten ignores moved from
:lib.core/->legacy-MBQL to :lib.convert/->legacy-MBQL and the check
failed.
A var used through an alias now takes its namespace from the file's own
require for that alias, per language so a .cljc file's clj and cljs
requires stay apart, and the attribution lint runs with --cache false.
* Split the attribution test by behaviour
One deftest each for counting, which findings are charged, and how a
finding resolves to a symbol, so a failure names the behaviour that broke.
* Say a stale flat discouraged budget is never read, not double-counted
The check removes these linters from the occurrences before comparing
:ignore-counts, so a leftover flat entry is ignored rather than counted a
second time.
* Mark the nested-ignore gap as a way past the ratchet, with a TODO
A new ignore wrapped around an existing one for the same linter needs no
budget, so say that plainly and record the planned fix: lint copies with
one ignore disabled each and charge a finding to the copy that reveals it.
* Split kondo ratchet budgets into separate prod and test files
Prod and test kondo suppressions shared one .clj-kondo/ratchets.edn budget per
linter, so a pile of test-code ignores could mask a prod-code regression (or
vice versa) inside the same number. Adds .clj-kondo/ratchets-test.edn, tracking
:ignore-counts for anything under a `test` directory, independent of the prod
file's budget. :config-counts (.clj-kondo/config.edn suppressions) stays
combined in the prod file -- config.edn's own scoping mixes test and prod
namespaces in several groups, so it can't be reliably split per suppression.
Updates every consumer: dev.kondo-ratchet's fix!/check, bin/merge-kondo-ratchets,
the backport and release-cut disable scripts, the shrink workflow, and
file-paths.yaml/.gitignore.
* Fix test-ratchets validation and exemption-suggestion bugs from roborev
fix!/check read the test ratchets file with *ratchets-file* rebound to
*test-ratchets-file*, but validated it after that binding ended, so an
unknown linter there was reported against the prod path instead. Validation
now runs inside the binding, and both now reject a nonempty :config-counts
on the test file instead of letting fix! silently drop it.
kondo-insert-ignores also only ever suggested adding a new linter's
:comment-exempt to the prod file, even when every unjustified occurrence
was test-only. It now checks prod and test occurrences against each file's
own exemptions independently and names whichever file(s) actually need it.
* Dedupe test-ratchets read+validate, simplify test-occurrence?
fix! and check each rebuilt the same read-validate-test-file sequence
inline; extracted into read-validated-test-ratchets. test-occurrence?
no longer builds a throwaway set just to check one membership.
* Note what splitting :config-counts later would actually cost
* Fix Eastwood reflection warning in new test helper
empty-test-ratchets-file! returned an untyped value, so .getPath on it
needed reflection at each call site. Hinting the return type as
java.io.File fixes it -- CI's changed-tests Eastwood run caught this.
* Move return type hint onto the arg vector
Clj-Kondo prefers the hint on the arg vector to the one on the fn name.
* Move shrink PR body file reading into dev.kondo-ratchet
The bb task only parses its arguments now; reading and combining the
ratchet files lives next to shrink-pr-body, with a test.
* Give the test ratchets file its own header and drop its :config-counts
The test file was rendered with the prod header, whose last lines point
readers at ratchets-test.edn and say :config-counts "stays here" -- both
wrong inside the test file. It also carried an empty :config-counts that
the check rejects whenever it's nonempty.
render-test writes the test file with its own header and no
:config-counts, and throws on a nonempty one rather than dropping it.
fix!, the normalization check, and the three-way merge (a new "test"
kind) all use it.
* Consolidate the module viewers into one explorer
`./bin/mage modules-tree --html` writes a single page with three views
of the module config: the tree, a coupling ranking, and a dependency
graph around focused modules. `dev.module-explorer/open!` builds the
same page in a dev REPL with namespace-level edges, so the graph can
show which API namespaces each consumer uses.
The graph replaces dev.module-viz and its Jetty server. Source paths,
stats and teams go through the shared resolver, so nested and renamed
modules report correctly. module-team moves into hooks.common.modules.
* Label module counts as used by and uses
* Refine module explorer semantics and state
* Make ratchet shrinking explicit, and check it in one place
The ratchet test fixture ran `fix!` on every non-CI run, so `project-tests
ratchets` rewrote .clj-kondo/ratchets.edn in your working tree before
asserting against it. A check that edits what it checks leaves a diff nobody
asked for, in the file every PR conflicts over, and its only reachable
failure was a bug in the fixer that had just run.
Removing the auto-fix forces the dev/CI split to go too: a strict local test
that no longer tightens would fail on every branch that removes an ignore.
So over-budget is now the rule everywhere, and shrinking is only
`./bin/mage kondo-ratchets-shrink`, which the shrink workflow runs on master.
That left two implementations of nearly the same check. The babashka
`check-report` gains the justification finding — the one assertion the JVM
suite had that it lacked — so `./bin/mage kondo-ratchets` covers over-budget,
config over-budget, unjustified ignores, normalization and unknown linters on
its own. CI runs it instead of booting a JVM, and the five now-duplicated
in-repo assertions leave the test namespace, which drops from 6.2s to 0.8s
because it no longer walks the source tree.
Task names follow: check-kondo-ratchets (which nothing called) becomes
kondo-ratchets, fix-kondo-ratchets becomes kondo-ratchets-shrink, and the
merge task becomes private -kondo-ratchets-merge, since only the git merge
driver invokes it. bin/merge-kondo-ratchets keeps its name; installed merge
drivers point at it.
* Polish kondo ratchet guidance
* Prime the ratchet shrink workflow
Intentionally raise the :case-symbol-test budget from 1 to 2 without adding a suppression. The read-only check allows the unused budget; after this PR merges, the shrink workflow should restore it to 1 through a Tighten ratchets automation PR.
* Make the ratchet suite check the project
Keep the ratchet tooling unit tests in the backend suite, but make project-tests ratchets run the read-only repository check. Include it in the bare project-tests command so local project-level checks still validate the checked-in ratchet policy.
* List project-test suites once
* Address review findings on the ratchet check
check-report threw NPE on a ratchets map without :comment-exempt, while
tolerating a missing :ignore-counts or :config-counts. Default it, so the
three policy keys behave alike.
The run-project-ratchet-checks description and the step comment both still
described the old wiring. The check now runs alongside the backend checks,
which no longer include it, and "no JVM" is not quite true: resolving
clj-kondo on a cold cache shells out to the JDK prepare-backend installs.
* Warn about stale comment exemptions
The only check that a :comment-exempt entry is still needed was a local-only
test, which went vacuous when the dev and CI semantics were unified, so
nothing named a stale exemption any more. The check now reports one the way
it reports an :unlimited policy nobody uses: informational, never a failure,
since only the shrink workflow removes them.
* Say why the merge driver finds mage through its own path
The script runs against whatever repository holds the conflict, which is not
always the one it lives in -- the tests merge stages in a throwaway repo. A
relative bin/mage looks plausible and fails there, so record the reason.
* Stop wrapping two comments that did not need it
One fits on a single line once it says less. The other breaks at its sentence
boundary rather than mid-clause.
* Break the ratchets header on its sentences
Four sentences started mid-line and wrapped mid-clause, one of them trailing
"in the PR." onto a line of its own. Same words, one sentence per line.
* Keep comment exemptions until someone removes them
fix! dropped a :comment-exempt entry as soon as every ignore of its linter
carried a comment, or the last uncommented one was deleted -- so an exemption
could disappear as a side effect of unrelated work, and getting it back is a
change to defend in review.
An exemption records a decision about a linter, not a count, which is the
reason :unlimited policies are already kept at zero and only warned about.
Treat the two alike: the check and the shrinker name a stale exemption, and a
person deletes it.
* Add a manual kondo ratchet conflict merger
* Split kondo ratchet merge tests
* Document the ratchet conflict merger
* Simplify unlimited ratchet merge policy
* Simplify ratchet merger for final policy schema
* Resolve ratchet conflicts to the stricter policy
Concurrent policy changes now always resolve: the smaller budget wins, a
finite budget beats :unlimited, and a removed entry beats any budget.
Comment exemptions merge on their own. Every stage is read through
read-ratchets and shape-checked before a disabled stage decides the result.
bin/merge-kondo-ratchets reads the index stages by number, so a file
deleted on one side and changed on the other exits nonzero with nothing
staged, and both-sides-add merges against an empty base. It runs from any
directory in the repository and stages only .clj-kondo/ratchets.edn.
mage.merge-kondo-ratchets-test drives the script against real conflicts
in temporary repositories.
* Point the clojure-write skill at the ratchet merge resolver
* Reject malformed stages before merging ratchets
The shape checks move out of read-ratchets into validate-policies, and merge-ratchets runs them on every stage, so a stage with :config-counts [] or :comment-exempt [] is an error rather than an empty set of policies that the merge would read as removals.
* Cover unreadable stages in the merger tests
An empty file, two forms, or a non-map stage now fails the reader instead of merging as no policies; the script leaves such a conflict for a human and stages nothing.
* Reject a non-map ratchet stage before merging
A nil stage passed every field check because dissoc, keys, and the defaults all accept nil, so a malformed stage could merge as an empty set of policies.
* Reject non-keyword linter names
The merge keys on (str linter) and renders the same way, so a string, nil or
symbol name was rewritten as a keyword, collided with one, or produced a file
that no longer read back -- and the script still exited 0 saying it had staged
the merge. Also describe the two sides by git stage: "current" and "incoming"
are inverted during a rebase.
* Run each refused merge once in the tests
leaves-unresolved? and output each ran the script, so every refusal ran it twice
and the "changed nothing" assertion covered a different run from the one whose
message was checked. Read the merged policies with the EDN reader while here.
* Compare the conflicted file, not just its status
A conflicted path reports UU, DU or UD however its contents are rewritten, so
comparing git status alone did not show that a refused run left the file alone.
* Tighten merge-tool docstrings and the ratchet conflict docs
* Compare the index stages of a refused merge too
Porcelain status reports a conflicted path as DU/UU whatever its stages hold, so it alone does not show
that a refused run left the conflict alone. The stages carry each side's blob.
* Spell out the unmerged status codes
UU/DU/UD as bare shorthand assumes the reader knows git's porcelain codes for a conflicted path.
* Clarify the unmerged-state snapshot comment
* Add trusted master ratchet shrinker
* Retry ratchet shrink after concurrent master pushes
* Test the check-kondo-ratchets report
* Apply ratchet policy to the check report
* Add severity-based inline ratchet policies
* Update ratchet policy documentation
* Finalize ratchet policy enforcement
* Move seeded linters to bounded ratchets
* Simplify unlimited ratchet policies
* Finish ratchet CI integration
* Document unlimited ratchet policy values
* Keep ignore counts as the policy map
* Validate ratchet policy keys against known linters
Policy keys in .clj-kondo/ratchets.edn must name a clj-kondo built-in from the
pinned version, a linter configured under .clj-kondo/, or an allowlisted
external diagnostic. The check and the fixer fail with one error listing every
unknown name instead of counting it. Config budgets must be non-negative
integers.
Drop :unresolved-require, a linter that never existed, from the semantic-search
ignore and the ratchets file.
* Keep unlimited ratchet policies with no ignores left
An :unlimited entry in :ignore-counts records a decision about the linter, so
the fixer no longer drops it when its count reaches zero. The check and the
fixer print one sorted warning naming such entries; it never fails CI. Bounded
zero budgets are still dropped, and :comment-exempt is unchanged.
* Deliver ratchet shrinking through an automation PR
* Validate seeds and read only tracked kondo configs
The known-linter set now comes from git-tracked `.edn` files under
`.clj-kondo`, so the dependency configs `mage kondo` copies in locally
(gitignored, absent from a clean checkout) no longer make validation
depend on the machine. `--seed` is checked against the same set before
anything is written, and a hand-written bounded `0` with no ignores
left is reported when the fixer drops it.
* Reopen the ratchet automation PR after a close
The "already carries this result" skip now requires an open PR, so a
branch left behind by a PR closed without merging is overwritten and
gets a fresh PR instead of silencing every later run. The compare step
reads PR state with the default token under `pull-requests: read`.
* Let the ratchet automation PR approve and merge itself
Copies the backport recipe: github-actions[bot] approves the PR with the default token (a user cannot approve their own PR, and the automation user owns it), then the automation user enables squash auto-merge. The commit identity is the docs generator's, which merges with that one approval. A ratchets-only change no longer flips backend_all, so the PR waits on the Project tests job alone.
* Describe the self-merging ratchet automation PR
* Expect the ratchet check namespace in the project-test suites
* Read the ratchets file as exactly one map
An empty file, a non-map, or a trailing second form used to read as an empty policy set, which the merger would have staged as the removal of every budget. Tracked kondo configs missing from the worktree (unstaged deletions, sparse checkouts) are skipped instead of failing the scan.
* Skip the uberjar for a ratchets-only change
* Keep fork PRs away from the ratchet automation steps
The head-branch lookup matched any repository's branch of that name, so a fork PR could have reached the approval and auto-merge steps; only an open PR from this repository counts now. Approval and auto-merge key on the open PR rather than on whether the branch was pushed, so a run that failed after opening the PR is repaired by the next one.
* Look up the ratchet automation PR by owner-qualified head
gh pr list caps at 30 results and filtered forks client-side, so enough fork PRs on the same branch name could hide the real automation PR and make the create step fail. The REST head=owner:branch filter is applied server-side and matches only this repository's branch.
* Stop checkout credentials from overriding the automation push token
actions/checkout persists GITHUB_TOKEN as an Authorization header, which takes precedence over the token embedded in the push URL, so the force-push to the automation branch would be rejected.
* Tolerate config budget and exemption reductions in CI
CI already let inline counts fall below their budgets so the shrinker could record them after merge, but a lowered config budget or a stale comment exemption still needed an exact match, so two PRs that each removed a suppression could fail the second merge. Config counts now fail CI only when they grow, stale exemptions never fail CI, and check-kondo-ratchets reports config growth alongside inline growth. Local runs stay exact.
* Say what CI rejects without listing the cases
* Keep actionable ratchets bounded
* Exclude the ratchets file with one pattern
Path filters are OR-ed, so a separate "!.clj-kondo/ratchets.edn" entry matched
every path that was not that file: backend_all became true for every PR, and
no_source_code_changes could never be true. The exclusion has to live inside the
pattern, as modules/drivers does above.
* Arm auto-merge against a known commit
The approve and merge steps keyed on the PR number alone, so nothing had checked
what the fixed automation branch name currently holds. Both paths now record the
sha they pushed or compared, and auto-merge matches it. Also guard the job to
this repository, as the workflow it replaces did.
* Report ratchet validation failures without a stack dump
check read and validated the file, then read it again. Now it parses once and
catches inside, so read-ratchets, validate-linters! and scan all report the same
one-line message. Also: CI=false was truthy in the tests, seeding a recorded
linter with no ignores said "nothing to seed" while dropping its policy, and
lowered-counts rebuilt the seeded set per entry.
* Keep the drift message off stdout
The drifted path prints its findings and then exits; folding it into fail! added
a summary line that check-reports-empty-unlimited-test pins. Split the throw from
the print so only messages the run has not already shown are printed.
* Tighten ratchet docstrings and the generated header
* Guard against standalone negations in the path filters
The patterns in a filter are OR-ed, so a "!..." entry matches every path the other entries don't
instead of subtracting from them, and the filter is then always on. The ratchets exclusion is written
inside one pattern for that reason; this keeps the next one from being written the other way.
* Reuse the automation PR on its ratchet file alone
The extra check that master had not touched ratchets.edn since the branch was cut only bites when master
moved to a different value that shrinks back to the same result, since any change to the result is already
caught by comparing the file. Dropping it means only a shallow fetch of the branch tip is needed.
* Force-push the automation PR when it conflicts
A PR whose ratchet file already matches is otherwise left alone, and a conflict is the one thing that
state cannot clear on its own. Mergeability is computed lazily, so UNKNOWN is polled past and, if it
never resolves, left for the next run.
* Ask once whether the automation PR conflicts
Polling for a definitive answer bought nothing: the next push to master runs this again within the hour,
so a mergeability check still in flight can read as not conflicted and be settled then.
* Say what the reuse comparison is against
git diff FETCH_HEAD -- <path> reads as a commit-to-commit diff. It is against the working tree, which is
the only reason comparing the branch tip answers whether the PR still delivers the right file.
* Fix the circular Filter type alias
Record<string, Filter> is expanded eagerly, so naming it inside Filter's own definition trips TS2456. An
inline index signature is deferred and holds the same shape.
* Prove a ratchets-only change skips the backend filters
* Run the path-filter specs when the filters change
* Use gh CLI to approve ratchet PRs
* Internal load-namespaces tool to check for compilation errors
* Ignore load-namespaces in module checks
* lint load-namespaces
* Reject positional args
* Test the new utility
* Guard against nonexistent root.
Make the code more testable.
Drop the quarantine-conflict flag: a driver marked `skip` in ci-test-config
whose own `modules/drivers/*` files changed now simply runs, instead of
spinning up a runner that fails immediately and demands a `ci:run-<driver>`
label. Changes to another driver's files, or to shared code, leave the skip
in place.
Rename the decision logic's "quarantine" vocabulary to "skip", matching the
config's own `status: skip`. "Quarantine" now refers only to ci-conductor's
suite-level gating rules.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* consolidate driver CI labels under ci:run-<driver>
break-quarantine-<driver> and ci:run-<driver> were two spellings of the
same override: ci:run-* is checked at Priority 3, ahead of the quarantine
check, so it already forces a quarantined driver to run.
Drop the break-quarantine-* family and point the quarantine-conflict hint,
the mage help, and the workflow error messages at ci:run-<driver>.
* rename ci:all-cloud-drivers to ci:run-all-cloud-drivers
Every other driver force-run label is spelled ci:run-*.
Co-authored-by: Chris Vandoodewaard <chris.vandoodewaard@metabase.com>
* Honor suite-level quarantine rules in the gate
/api/quarantine serves a top-level `suites` array of glob rules alongside
`tests`. A rule matching the job's reporting suite quarantines the whole
suite, so the gate passes without comparing failures test by test.
Globs match as an unanchored, case-insensitive substring and support `*`,
`?` and `{a,b}` — a subset of the minimatch semantics ci-conductor
validates against, compiled inline because this package carries no
runtime dependencies and CI runs it straight from the checkout.
Both response keys are read as optional, so the script and the server
need no lockstep deploy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Drop the driver "info" status
Suite-level quarantine rules say the same thing from CI Conductor, for
every suite rather than drivers alone: run it, but don't let it gate.
mage now reads only "skip" from ci-test-config.json, and no longer emits
a per-driver status; the workflows drop the plumbing that carried it,
along with the job summary and PR comment that reported an info driver's
real outcome. The quarantine gate's own verdict and CI Conductor's UI
cover that visibility.
"skip" stays: not running a cloud driver at all saves real spend and has
no equivalent in the quarantine endpoint.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Remove dead checks from drivers
* Tweak comment
* Drop the "info" status from notification
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bump babashka postgresql pod to 0.1.6 so mage runs on Apple Silicon
The 0.1.0 pod manifest has no macos-aarch64 entry -- only macos-amd64.
Babashka downloads that x86_64 artifact and caches it under the aarch64 path
anyway, so on an Apple Silicon machine without Rosetta every mage command dies
before doing any work:
Cannot run program ".../mac_os_x/aarch64/pod-babashka-postgresql":
Exec failed, error: 86 (Bad CPU type in executable)
Clearing the pod cache does not help -- the re-download is the same x86_64
binary. 0.1.5 is the first release with a real macos-aarch64 artifact; this
bumps to the current 0.1.6, which resolves to a native arm64 executable.
0.1.6 keeps every platform 0.1.0 shipped (macos-amd64, linux-amd64,
windows-amd64) and adds macos-aarch64 and linux-aarch64, so no one loses
coverage. `pg/execute!` is the only pod fn used anywhere in the repo, and its
signature is unchanged.
Verified on arm64: the cached binary is now `Mach-O 64-bit executable arm64`,
`mage.bot.setup-worktree` loads, and `./bin/mage fix-kondo-ratchets` runs to
completion.
Claude-Session: https://claude.ai/code/session_01X5dVcVX59aLonqqsYsqU65
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* mage: read driver skip/info status from ci-test-config
Replace the legacy `ignored.drivers` quarantine list with the new top-level
`drivers` array in ci-test-config.json, where each entry carries a status:
skip -- do not run the driver (quarantined; break-quarantine-<driver> overrides)
info -- run the driver, but its result must not gate (data collection only)
required -- not listed: runs and gates as usual (the default)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Add ci:run-<driver> and ci:run-all-drivers labels to mage driver decisions
Allow force-running specific drivers or all drivers via PR labels,
overriding quarantine. Inserted at priority 3 (after H2/Postgres
always-run, before quarantine check). Added Labels of Interest and
All Drivers sections to help output.
* Add hint about ci:run labels in CI driver decisions output
Shows a helpful message in CI logs pointing users to the new labels
and the local help command for more info.
---------
Co-authored-by: Ben Grabow <ben.grabow@gmail.com>
* ci-test-repl mage task
* Use ci port map to set port. Include junit name in job string
* Move to bgrabow
* Add dry-run mode
* task! wrapper to support helptext
Babashka task that starts up an isolated dev environment per worktree. Expands on @edpaget's excellent work, with interactive fzf prompts for edition, token, appdb, and warehouse services. Optional `--run` flag to launch backend + frontend in the background with PID tracking. Persistent config in `.dev-env-state.edn` for reuse across invocations.
* Add worktree dev environment tooling
Add `mage dev-env` task for isolated worktree dev environments with
Docker containers and deterministic per-worktree ports. Includes workmux
layout with nREPL dev server, frontend build, and nvim panes. Adds a
custom Claude Code Docker image with Clojure tooling via mise, and
a bin/claude-dangerous helper script.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Move dev-env mage task to personal -edpaget-dev-env task
Move the worktree dev environment tooling from the shared mage task
namespace (mage.dev-env) to a personal task namespace (mage.edpaget.dev-env),
remove the sample .workmux.example.yaml, and update .workmux.yaml hooks
to use the new command name.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* Make ci-report work without GNU timeout
* Destroy process on timeout
* Cherry-pick b9ff360230
---------
Co-authored-by: Bryan Maass <bryan.maass@gmail.com>
This introduces a pre-commit hook that automatically removes unused require
statements detected by clj-kondo, preventing CI failures.
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
* Skips cloud driver tests on PRs unless files changed
Cloud drivers (Athena, BigQuery, Databricks, Redshift, Snowflake) now
only run on PRs when their files changed or 'ci:all-cloud-drivers' label
is present. Full coverage maintained on master/release branches.
* [do not merge] add failing databricks test
* Revert "[do not merge] add failing databricks test"
This reverts commit de5b6fa602.
* empty commit
* Fix cloud driver skip logic to respect ci:all-cloud-drivers label
The previous condition had three checks that could block cloud drivers:
1. !inputs.skip - false when no backend files changed
2. skip != 'true' - from mage can-skip-driver-tests
3. X-should-run == 'true' - the per-driver decision
The problem: checks 1 and 2 would override check 3, so the label had no
effect. Even with ci:all-cloud-drivers, cloud drivers were skipped.
We don't need inputs.skip or can-skip-driver-tests for cloud drivers
because the X-should-run output already encapsulates the full decision:
- master/release branch → run
- ci:all-cloud-drivers label → run
- driver files changed → run
- otherwise → skip
The label is an explicit override, so it should work even on PRs with no
backend changes. Removing the redundant checks lets the label work.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* mage -driver-decisions + roll in module analysis
* Remove quarantine check from test-driver action
Quarantine logic is now consolidated in mage -driver-decisions.
The check happens earlier in determine-driver-skips, so quarantined
drivers get should-run=false and their jobs never start at all.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Fail CI if quarantined driver has file changes without break-quarantine label
If you change files in a quarantined driver's module (e.g., modules/drivers/snowflake/)
but don't add the break-quarantine-<driver> label, a new check-quarantine-conflicts
job will fail with a clear error message.
This prevents accidentally merging changes to quarantined drivers without running
their tests.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Add --github-output-only flag to mage driver-decisions
Instead of using grep to filter mage output, the task now supports two modes:
- Default: prints human-readable analysis + GITHUB_OUTPUT preview
- --github-output-only: prints only clean key=value lines for CI
The workflow now calls mage twice: once for debugging visibility,
once with --github-output-only redirected to $GITHUB_OUTPUT.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Add quarantine conflict checks to all driver jobs
Extends the quarantine conflict detection to ALL drivers, not just cloud
drivers. When a driver is quarantined but has file changes in
modules/drivers/<driver>/, the job will run but immediately fail with
a clear error message explaining how to add the break-quarantine label.
Changes:
- Add quarantine conflict check steps to presto-jdbc, sparksql, sqlite,
sqlserver, and vertica jobs
- Update login-to-ecr job condition for vertica quarantine conflicts
- Fix verbose output suppression in --github-output-only mode by adding
verbose? parameter to changes-important-file-for-drivers?
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Add driver decision priority documentation and tests
Adds comprehensive documentation to driver-decision function showing the
priority order as a table, plus explanations of what triggers "driver
module affected" and the difference between cloud vs self-hosted drivers.
Adds mage.modules-test with 14 tests (78 assertions) covering:
- Quarantine logic and break-quarantine labels
- Global skip behavior
- H2 always-runs behavior
- Master/release branch behavior
- Driver module affected behavior
- Cloud driver special rules (labels, file changes)
- Self-hosted driver skip logic
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Comment out starburst from driver-directory mapping
Starburst tests are currently disabled in drivers.yml, so the mapping
entry was inconsistent with all-drivers list. Commented out with a note
explaining why, so it can be easily re-enabled when starburst tests
come back.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* silence messages when in github-output-only mode
* remove redundant cloud-driver change detection
- this happens inside the script instead
* get rid of redundant rules definitions
- now only print how we make decisions in help for the task
* make a temporary redshift change
* Revert "make a temporary redshift change"
This reverts commit 3d93c60368.
* format usage fn code
* light refactor
* Rename driver decision variables for clarity and flatten decision tree
- driver-module-affected? → driver-deps-affected? (module dependency graph)
- drivers-changed → particular-driver-changed? (specific driver's files)
- Reorder priority: skip/H2/master now before quarantine
- Flatten nested cond for cloud driver rules
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* Quarantine now respected on master/release branches
- Move quarantine check before master/release in driver-decision priority
- Quarantined drivers are now skipped even on master/release (unless break-quarantine label)
- Update tests and priority table documentation to reflect new order
- Cloud drivers don't benefit from driver-deps-affected (only label or file changes)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
