fix(observability): stop using Grafana's $__ macro prefix as the SQL "All" sentinel - #5716
Merged
Merged
Conversation
…"All" sentinel
The ai-usage/maintainer-reviews/miner-usage dashboards used allValue: "$__all"
and compared it against a literal '$__all' to detect an unfiltered "All"
selection. Confirmed live against a real Grafana + frser-sqlite-datasource
instance: ${var:sqlstring} does not sql-quote a value that itself starts with
"$__" (Grafana treats it as a macro reference, not literal data), so the
substituted query carried the raw unquoted token $__all on both sides of the
comparison. SQLite then parsed that token as its own $__all named bind
parameter, which was never supplied -- every "All"-filtered panel query
either errored ("missing named argument \"__all\"") or silently returned zero
rows, even though the underlying reporting data was present and fresh.
Switches the sentinel to a plain string (__ALL__) that sqlstring quotes
normally, and updates both dashboards' own regression test simulations (which
had baked in the same wrong assumption about Grafana's substitution behavior,
which is why this shipped undetected).
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
❌ 7 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
This was referenced Jul 14, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 14, 2026
…aintainer-reviews dashboard test (#5719) #5716 moved every panel in grafana/dashboards/maintainer-reviews.json from the "\$__all" Grafana-macro-prefixed sentinel to a plain "__ALL__" literal, but missed updating this test file's assertions and its expandGrafanaRange() helper default. The stale "\$__all" default meant simulating the "All repos" selection built a WHERE clause that could never match the dashboard's real "__ALL__" sentinel, so every issue-activity/PR-panel row-count assertion silently fell through to the single-repo branch and returned 0 rows instead of the true total.
11 tasks
This was referenced Jul 14, 2026
JSONbored
added a commit
that referenced
this pull request
Jul 14, 2026
Three independent, pre-existing gate breaks, none introduced by this branch -- found while chasing why release-please's PRs (#5710-5713) looked flaky under local parallel test runs, and confirmed to reproduce identically on a clean origin/main checkout: - test/unit/agent-sdk-driver.test.ts, ai-summaries.test.ts, upstream-ruleset.test.ts: each does real, non-trivial work (git subprocess round-trips, a 42-entry forbidden-word sweep, 29 sequential D1-backed scenarios) that reliably exceeds vitest's 15s default under load. Widened each to an explicit, realistic timeout instead of leaving them to flake. - test/unit/selfhost-grafana-dashboard.test.ts: 5f187c2 (#5716) fixed a real production bug -- Grafana's ${var:sqlstring} doesn't sql-quote a value that starts with $__, so the old $__all "All repos" sentinel leaked through unescaped and SQLite misparsed it as its own bind parameter, silently zeroing every "All"-filtered panel. That commit updated the dashboard JSON and two sibling test files but missed this one. - apps/gittensory-ui/public/openapi.json + 3 route/component .tsx files: 40e6cdf (#5715, the @gittensory -> @Loopover bot-mention rename) changed copy text but never regenerated the OpenAPI spec or ran prettier on the reflowed JSX text nodes, so ui:openapi:check and ui:lint both fail on main. Regenerated the spec (npm run ui:openapi) and ran eslint --fix on the affected files; both are mechanical, zero-semantic-diff outputs.
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
ai-usage.json/maintainer-reviews.json/miner-usage.jsonusedallValue: "$__all"and compared it against a literal'$__all'to detect an unfiltered "All" selection:(${var:sqlstring} = '$__all' OR col = ${var:sqlstring}).frser-sqlite-datasourceinstance:${var:sqlstring}does not SQL-quote a value that itself starts with$__(Grafana treats it as a macro reference, not literal data), so the substituted query carried the raw unquoted token$__allon both sides. SQLite then parsed that token as its own$__allnamed bind parameter, which was never supplied — every "All"-filtered panel either errored (missing named argument "__all") or silently returned zero rows, even with fresh underlying data (verified: the redacted reporting export had 61,741 rows with a last-event timestamp minutes old — this ruled out a data/pipeline problem entirely).__ALL__) thatsqlstringquotes normally — verified with a directsqlite3execution of both the old and new pattern against the real reporting snapshot on the dedicated server (old pattern: 0 rows; new pattern: full expected row count).Scope
site/,CNAME,**/lovable/**CHANGELOG.mdeditValidation
npx vitest run test/unit/selfhost-grafana-ai-usage-dashboard.test.ts test/unit/selfhost-grafana-miner-usage-dashboard.test.ts test/unit/ai-usage-index.test.ts— all pass, including realsqlite3-execution assertions against the fixed query textsqlite3execution against the live reporting snapshotgit diff --check— cleanSafety