linter

[akshaydeo]

Summary

Introduces bifrostlint, a custom static analysis tool for Bifrost-specific code quality rules, along with a GitHub Actions CI workflow and golangci-lint configuration to enforce them automatically on PRs and pushes.

Changes

• Added cmd/bifrostlint as a standalone Go module containing three go/analysis-based analyzers:
  • sqlconcurrent: Flags CREATE INDEX statements missing CONCURRENTLY unless the code path is guarded against Postgres or is inside a migration transaction (where CONCURRENTLY is invalid).
  • sqlmatview: Flags CREATE/REFRESH MATERIALIZED VIEW operations on the boot path unless they run inside a goroutine or a function annotated/named as a background task.
  • sqldialect: Flags Postgres-only SQL tokens (e.g., CONCURRENTLY, pg_advisory_lock) in string literals that are not gated by a dialect check.
• Added exportedtests as a subcommand of bifrostlint that verifies every exported symbol in core/ and framework/ has at least one test reference, with baseline file support for pre-existing untested symbols.
• Added shared internal packages ignore (parses // bifrostlint:ignore <rule> <reason> directives) and sqlscan (regex-based SQL keyword detection and execution-call detection).
• Added .golangci.yml enabling errcheck, forbidigo, govet, ineffassign, staticcheck, and unused. The forbidigo rule bans json.Marshal/json.MarshalIndent in core/ and framework/, directing callers to schemas.MarshalSorted instead.
• Added .github/workflows/lint.yml running both golangci-lint and bifrostlint on PRs and pushes to dev and main.
• Added a bifrostlint-go-filenames pre-commit hook that rejects Go filenames containing underscores (except _test.go).
• Added make lint-bifrost and make lint-bifrost-baseline Makefile targets; make lint now depends on lint-bifrost.
• Annotated existing migration CREATE INDEX calls in framework/configstore/migrations.go and a test fixture in framework/logstore/rdb_postgres_perf_test.go with // bifrostlint:ignore sqlconcurrent directives explaining why CONCURRENTLY is intentionally absent (migration transaction context or test setup).

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (React)
• Docs

How to test

# Build and run bifrostlint analyzers
make lint-bifrost

# Run full lint suite (golangci-lint + bifrostlint)
make lint

# Run analyzer unit tests
cd cmd/bifrostlint && go test ./...

# Regenerate the exportedtests baseline if needed
make lint-bifrost-baseline

The sqlconcurrent, sqlmatview, and sqldialect analyzers each have testdata fixtures under their respective testdata/src/ directories that are exercised by analysistest.Run.

Breaking changes

• Yes
• No

Related issues

Security considerations

None. This change only adds static analysis tooling and CI enforcement; it does not modify runtime behavior, authentication, secrets handling, or data access.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
7 out of 13 committers have signed the CLA.

✅ impoiler
✅ BearTS
✅ Javtor
✅ kevinpdev
✅ Madhuvod
✅ binbandit
✅ d3lm
❌ akshaydeo
❌ Pratham-Mishra04
❌ roroghost17
❌ TejasGhatte
❌ etnperlong
❌ danpiths
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Introduced bifrostlint, a custom static analyzer suite for Go code with rules for SQL safety (index concurrency, materialized view operations, dialect-specific SQL) and test coverage validation for exported symbols.

• Chores

  • Added GitHub Actions linting workflow and configuration files (.golangci.yml, .pre-commit-config.yaml).
  • Updated Makefile with new linting targets.
  • Applied linting suppressions to existing code as needed.

Walkthrough

A new custom linting tool bifrostlint is introduced with four Go static analyzers: sqlconcurrent enforces concurrent index creation in Postgres, sqldialect guards Postgres-only SQL with dialect checks, sqlmatview prevents materialized view operations on blocking paths, and exportedtests requires test coverage for all exported symbols. CI workflows, Makefile targets, and existing migrations are updated accordingly.

Changes

BifrostLint Static Analysis Tool

Layer / File(s)	Summary
CI/CD and Build Configuration .github/workflows/lint.yml, .golangci.yml, .pre-commit-config.yaml, Makefile, cmd/bifrostlint/go.mod	GitHub Actions workflows run golangci-lint-action and make lint-bifrost. New Makefile targets build and run bifrostlint with baseline support. Go module declares golang.org/x/tools dependency for analyzer infrastructure.
Shared Linting Infrastructure cmd/bifrostlint/internal/ignore/ignore.go, cmd/bifrostlint/internal/sqlscan/sqlscan.go, cmd/bifrostlint/internal/sqlscan/exec.go	Directive suppression parsing for // bifrostlint:ignore comments, SQL pattern detection (CREATE INDEX, materialized views, CONCURRENTLY keyword), and execution-call recognition via AST stack inspection.
SQL Concurrent Indexing Analyzer cmd/bifrostlint/analyzers/sqlconcurrent/sqlconcurrent.go, cmd/bifrostlint/analyzers/sqlconcurrent/sqlconcurrent_test.go, cmd/bifrostlint/analyzers/sqlconcurrent/testdata/src/concurrent/concurrent.go	sqlconcurrent analyzer scans for unguarded CREATE INDEX without CONCURRENTLY, detecting Postgres paths via dialect comparisons and early returns. Test fixtures cover guarded branches, non-concurrent variants, and suppression directives.
SQL Dialect Guard Analyzer cmd/bifrostlint/analyzers/sqldialect/sqldialect.go, cmd/bifrostlint/analyzers/sqldialect/sqldialect_test.go, cmd/bifrostlint/analyzers/sqldialect/testdata/src/dialect/dialect.go	sqldialect analyzer detects Postgres-only SQL (keywords, materialized-view ops) lacking dialect guards via if/switch/type switch conditions. Test fixtures demonstrate gating patterns, constant assignments, and suppression.
Materialized View Analyzer cmd/bifrostlint/analyzers/sqlmatview/sqlmatview.go, cmd/bifrostlint/analyzers/sqlmatview/sqlmatview_test.go, cmd/bifrostlint/analyzers/sqlmatview/testdata/src/matview/matview.go	sqlmatview analyzer flags CREATE/REFRESH MATERIALIZED VIEW on blocking paths, recognizing background contexts (goroutines, background function prefixes, bifrostlint:background annotations). Test fixtures cover blocking calls and background execution.
Exported Function Test Coverage Analyzer cmd/bifrostlint/analyzers/exportedtests/exportedtests.go	exportedtests subcommand enforces test references for exported symbols via types.Info lookups in _test.go files, supporting baseline files and -emit-baseline mode for baseline regeneration.
BifrostLint CLI Entry Point cmd/bifrostlint/main.go	Wires three SQL analyzers into a multichecker; routes exportedtests subcommand argument to dedicated Run() function with separate baseline/emit-baseline flag handling.
Apply Linting Rules to Existing Code framework/configstore/migrations.go, framework/logstore/rdb_postgres_perf_test.go	Adds // bifrostlint:ignore sqlconcurrent suppression directives on index creation within migration transactions and performance test setup to comply with concurrent-indexing analyzer.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

• danpiths

Poem

🐰 Hop, skip, and lint we go,
Four new rules to help us grow!
Concurrent indexes, dialects in place,
Matviews guarded with graceful pace,
And tests that vouch for exports true—
Bifrost lints the code we brew! 🌈

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR does not address the linked issue #123 (Files API Support). The PR adds linting tooling and static analysis, while the issue requests implementation of file upload APIs for providers.	Either link the correct issues related to linting/CI improvements, or if this PR was incorrectly linked to #123, remove that association and ensure the PR is linked to the appropriate issues.
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'linter' is vague and does not clearly describe the specific changes; it lacks meaningful detail about what linting tooling was added.	Use a more descriptive title such as 'Add bifrostlint custom analyzer tool with CI integration' to better convey the scope and purpose.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description is comprehensive and includes all required sections from the template with detailed explanations of the changes, rationale, testing instructions, and security considerations.
Out of Scope Changes check	✅ Passed	All changes align with the PR's stated objective of adding bifrostlint custom analyzers and CI enforcement; no unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 05-17-linter

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[akshaydeo]

• linter #3555 👈 (View in Graphite)
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Actionable comments posted: 7

🧹 Nitpick comments (2)

Makefile (1)

1516-1522: 💤 Low value

Target body length exceeds checkmake recommendation.

The lint-bifrost target has 6 lines, exceeding checkmake's suggested limit of 5. This is a minor style concern; the logic is clear and correct: build bifrostlint, run analyzers, run exportedtests with baseline.

Based on checkmake static analysis hint.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 1516 - 1522, The lint-bifrost Makefile target body is
over the 5-line checkmake recommendation; collapse commands into fewer lines by
chaining related commands with && or using semicolons so the target has at most
5 physical lines while preserving behavior: keep the build step (cd
cmd/bifrostlint && GOWORK=off go build -o ../../bin/bifrostlint .), the analyzer
run (./bin/bifrostlint ./...), and the exportedtests run with baseline
(./bin/bifrostlint exportedtests -baseline=cmd/bifrostlint/baseline.txt ./...)
but combine adjacent echo/build/run lines into single lines under the
lint-bifrost target to reduce total lines.

cmd/bifrostlint/internal/sqlscan/sqlscan.go (1)

60-65: ⚡ Quick win

Cache compiled regexes for repeated keyword/phrase checks.

regexp.MustCompile on every invocation adds avoidable overhead in analyzer hot paths; memoizing by pattern keeps behavior the same and improves runtime.

Also applies to: 79-90

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/bifrostlint/internal/sqlscan/sqlscan.go` around lines 60 - 65,
ContainsKeyword currently recompiles the regex on every call; change it to
memoize compiled *regexp.Regexp instances keyed by the pattern (built with
`(?i)\b` + regexp.QuoteMeta(keyword) + `\b`) in a package-level cache (use
sync.Map or a map+sync.RWMutex) and load-or-store the compiled regexp
(regexp.MustCompile or Compile) before calling MatchString; apply the same
memoization approach to the other regex-based keyword/phrase check in this file
so repeated analyzer hot-path calls reuse compiled regexes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/lint.yml:
- Around line 9-33: The workflow grants the default write GITHUB_TOKEN; add
explicit minimal permissions to avoid excess write access by adding a
permissions block (e.g., permissions: contents: read) for the jobs in
.github/workflows/lint.yml—specifically add a permissions: { contents: read }
under the golangci job and the bifrostlint job (or add a top-level permissions
key to apply to all jobs) so the golangci and bifrostlint jobs only have
read-only access needed for checkout and linting.
- Around line 18-20: Update the golangci-lint GitHub Action and its tool
version: replace the action reference golangci/golangci-lint-action@v6 with the
newer action tag and change the tool version field currently set to "v2.9.0" to
"v2.12.2" so the workflow uses the latest golangci-lint release; ensure both the
action ref and the version: value are updated together to "v2.12.2".
- Line 17: Update the Go version in the CI workflow by changing the go-version
value used in the lint job from "1.26.2" to "1.26.3"; locate the go-version key
in the workflow (the go-version: "1.26.2" entry) and replace it with "1.26.3" so
the workflow uses the latest stable Go release.

In @.pre-commit-config.yaml:
- Around line 32-38: The YAML parser error comes from the unquoted bash
one-liner in the entry field for the hook id bifrostlint-go-filenames; update
the entry value to be a proper YAML string or block literal to avoid
mapping/quote conflicts and long-line issues: either wrap the entire command in
double quotes and escape internal double quotes, or replace the scalar with a
block literal (entry: |) and place the bash -c '... --' script on the next line;
ensure the modified entry references the same bash one-liner logic (failed loop,
basename, *_test.go skip, underscore check, exit $failed) but is correctly
quoted/escaped so the YAML parser accepts it and the line-length problem is
resolved.

In `@cmd/bifrostlint/analyzers/exportedtests/exportedtests.go`:
- Around line 240-274: collectReferenced is recording symbols from any package
because pkgPath is ignored; change it to only call recordRef when the referenced
object's package path equals the package-under-test (use pkgPath). In both
places where you get obj (the *types.Obj from info.Uses inside collectReferenced
for *ast.Ident and *ast.SelectorExpr), add a guard that obj.Pkg() != nil &&
obj.Pkg().Path() == pkgPath before calling recordRef(referenced), so only
same-package references are recorded; keep using the existing recordRef and
referenced map.

In `@cmd/bifrostlint/internal/sqlscan/exec.go`:
- Around line 43-45: The current switch treats any bare *ast.Ident as a DB exec
call (using dbExecMethods[fn.Name]), causing false positives; restrict detection
to selector calls only by removing or disabling the *ast.Ident branch and
instead match on *ast.SelectorExpr (use sel.Sel.Name) and, if available, confirm
the qualifier (sel.X) is a DB-like receiver via type info (pkg.TypesInfo) or
known var names before consulting dbExecMethods; update the logic that
references the case *ast.Ident, fn.Name, and dbExecMethods accordingly.

In `@cmd/bifrostlint/main.go`:
- Line 5: The usage comment in cmd/bifrostlint/main.go is out of sync: the
current comment "bifrostlint ./...  # runs sqlmatview + sqldialect" omits the
sqlconcurrent analyzer that is actually included (see the analyzer list around
the analyzer registration in the file). Update that single-line usage comment to
list all default analyzers (sqlmatview, sqldialect, sqlconcurrent) so the usage
text matches the analyzers registered in main (referencing the usage/comment
string near the top of main.go and the analyzer registration block where
sqlconcurrent is added).

---

Nitpick comments:
In `@cmd/bifrostlint/internal/sqlscan/sqlscan.go`:
- Around line 60-65: ContainsKeyword currently recompiles the regex on every
call; change it to memoize compiled *regexp.Regexp instances keyed by the
pattern (built with `(?i)\b` + regexp.QuoteMeta(keyword) + `\b`) in a
package-level cache (use sync.Map or a map+sync.RWMutex) and load-or-store the
compiled regexp (regexp.MustCompile or Compile) before calling MatchString;
apply the same memoization approach to the other regex-based keyword/phrase
check in this file so repeated analyzer hot-path calls reuse compiled regexes.

In `@Makefile`:
- Around line 1516-1522: The lint-bifrost Makefile target body is over the
5-line checkmake recommendation; collapse commands into fewer lines by chaining
related commands with && or using semicolons so the target has at most 5
physical lines while preserving behavior: keep the build step (cd
cmd/bifrostlint && GOWORK=off go build -o ../../bin/bifrostlint .), the analyzer
run (./bin/bifrostlint ./...), and the exportedtests run with baseline
(./bin/bifrostlint exportedtests -baseline=cmd/bifrostlint/baseline.txt ./...)
but combine adjacent echo/build/run lines into single lines under the
lint-bifrost target to reduce total lines.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dac325a4-d0b3-4b32-a658-bb3c37de7ecb

📥 Commits

Reviewing files that changed from the base of the PR and between 89a99f0 and 85643ef.

⛔ Files ignored due to path filters (1)

• cmd/bifrostlint/go.sum is excluded by !**/*.sum

📒 Files selected for processing (21)

• .github/workflows/lint.yml
• .golangci.yml
• .pre-commit-config.yaml
• Makefile
• cmd/bifrostlint/analyzers/exportedtests/exportedtests.go
• cmd/bifrostlint/analyzers/sqlconcurrent/sqlconcurrent.go
• cmd/bifrostlint/analyzers/sqlconcurrent/sqlconcurrent_test.go
• cmd/bifrostlint/analyzers/sqlconcurrent/testdata/src/concurrent/concurrent.go
• cmd/bifrostlint/analyzers/sqldialect/sqldialect.go
• cmd/bifrostlint/analyzers/sqldialect/sqldialect_test.go
• cmd/bifrostlint/analyzers/sqldialect/testdata/src/dialect/dialect.go
• cmd/bifrostlint/analyzers/sqlmatview/sqlmatview.go
• cmd/bifrostlint/analyzers/sqlmatview/sqlmatview_test.go
• cmd/bifrostlint/analyzers/sqlmatview/testdata/src/matview/matview.go
• cmd/bifrostlint/go.mod
• cmd/bifrostlint/internal/ignore/ignore.go
• cmd/bifrostlint/internal/sqlscan/exec.go
• cmd/bifrostlint/internal/sqlscan/sqlscan.go
• cmd/bifrostlint/main.go
• framework/configstore/migrations.go
• framework/logstore/rdb_postgres_perf_test.go

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Set explicit permissions for GITHUB_TOKEN.

Both jobs run without explicit permissions, granting default write access to the workflow token. Restrict permissions to the minimum required scope.

🔒 Proposed fix to add read-only permissions

+permissions:
+  contents: read
+
 jobs:
   golangci:

Based on CodeQL static analysis hint.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	jobs:
	golangci:
	name: golangci-lint
	runs-on: ubuntu-latest
	steps:
	- uses: actions/checkout@v4
	- uses: actions/setup-go@v5
	with:
	go-version: "1.26.2"
	- uses: golangci/golangci-lint-action@v6
	with:
	version: v2.9.0
	args: --timeout=5m
	bifrostlint:
	name: bifrostlint
	runs-on: ubuntu-latest
	steps:
	- uses: actions/checkout@v4
	with:
	fetch-depth: 0
	- uses: actions/setup-go@v5
	with:
	go-version: "1.26.2"
	- name: Run bifrostlint analyzers
	run: make lint-bifrost
	permissions:
	contents: read
	jobs:
	golangci:
	name: golangci-lint
	runs-on: ubuntu-latest
	steps:
	- uses: actions/checkout@v4
	- uses: actions/setup-go@v5
	with:
	go-version: "1.26.2"
	- uses: golangci/golangci-lint-action@v6
	with:
	version: v2.9.0
	args: --timeout=5m
	bifrostlint:
	name: bifrostlint
	runs-on: ubuntu-latest
	steps:
	- uses: actions/checkout@v4
	with:
	fetch-depth: 0
	- uses: actions/setup-go@v5
	with:
	go-version: "1.26.2"
	- name: Run bifrostlint analyzers
	run: make lint-bifrost

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 11-22: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}

---

[warning] 23-33: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/lint.yml around lines 9 - 33, The workflow grants the
default write GITHUB_TOKEN; add explicit minimal permissions to avoid excess
write access by adding a permissions block (e.g., permissions: contents: read)
for the jobs in .github/workflows/lint.yml—specifically add a permissions: {
contents: read } under the golangci job and the bifrostlint job (or add a
top-level permissions key to apply to all jobs) so the golangci and bifrostlint
jobs only have read-only access needed for checkout and linting.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What is the latest stable Go version released as of May 2026?

💡 Result:

As of May 17, 2026, the latest stable version of the Go programming language is Go 1.26.3 [1][2]. This version was released on May 7, 2026 [3].

Citations:

• 1: https://go.dev/VERSION
• 2: https://go.dev/dl/
• 3: https://go.dev/doc/devel/release

---

Update Go version to 1.26.3 (latest stable release).

Go 1.26.2 is released, but Go 1.26.3 is now the latest stable version as of May 2026 (released May 7, 2026). Update the workflow to use 1.26.3 instead.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/lint.yml at line 17, Update the Go version in the CI
workflow by changing the go-version value used in the lint job from "1.26.2" to
"1.26.3"; locate the go-version key in the workflow (the go-version: "1.26.2"
entry) and replace it with "1.26.3" so the workflow uses the latest stable Go
release.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What is the latest version of golangci-lint as of May 2026?

💡 Result:

As of May 17, 2026, the latest version of golangci-lint is v2.12.2, which was released on May 6, 2026 [1][2][3].

Citations:

• 1: https://github.com/golangci/golangci-lint/releases/tag/v2.12.2
• 2: https://golangci-lint.run/docs/product/changelog/
• 3: https://github.com/golangci/golangci-lint

---

Update golangci-lint to the latest version.

The workflow currently uses golangci/golangci-lint-action@v6 with version: v2.9.0. This is outdated; as of May 2026, the latest version is v2.12.2. Update to the current version to benefit from bug fixes and improvements.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/lint.yml around lines 18 - 20, Update the golangci-lint
GitHub Action and its tool version: replace the action reference
golangci/golangci-lint-action@v6 with the newer action tag and change the tool
version field currently set to "v2.9.0" to "v2.12.2" so the workflow uses the
latest golangci-lint release; ensure both the action ref and the version: value
are updated together to "v2.12.2".

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Validate the .pre-commit-config.yaml syntax

yamllint .pre-commit-config.yaml 2>&1 || true

Repository: maximhq/bifrost

Length of output: 904

---

Fix YAML syntax error in the entry field.

Line 37 contains a YAML syntax error at position 146: "mapping values are not allowed here". The bash one-liner in the entry field must be properly quoted to avoid YAML parser conflicts. The line also exceeds the 80-character limit.

The fix requires wrapping the entry value in double quotes and escaping any internal double quotes (or use YAML's block literal syntax with | or |-):

entry: "bash -c 'failed=0; for f in \"$@\"; do base=$(basename \"$f\" .go); case \"$f\" in *_test.go) continue;; esac; case \"$base\" in *_*) echo \"$f: underscores not allowed in Go filenames (see AGENTS.md Code Style)\"; failed=1;; esac; done; exit $failed' --"

Or use YAML block literal (cleaner):

entry: |
  bash -c 'failed=0; for f in "$@"; do base=$(basename "$f" .go); case "$f" in *_test.go) continue;; esac; case "$base" in *_*) echo "$f: underscores not allowed in Go filenames (see AGENTS.md Code Style)"; failed=1;; esac; done; exit $failed' --

🧰 Tools

🪛 YAMLlint (1.38.0)

[error] 37-37: syntax error: mapping values are not allowed here

(syntax)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.pre-commit-config.yaml around lines 32 - 38, The YAML parser error comes
from the unquoted bash one-liner in the entry field for the hook id
bifrostlint-go-filenames; update the entry value to be a proper YAML string or
block literal to avoid mapping/quote conflicts and long-line issues: either wrap
the entire command in double quotes and escape internal double quotes, or
replace the scalar with a block literal (entry: |) and place the bash -c '...
--' script on the next line; ensure the modified entry references the same bash
one-liner logic (failed loop, basename, *_test.go skip, underscore check, exit
$failed) but is correctly quoted/escaped so the YAML parser accepts it and the
line-length problem is resolved.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Constrain test references to the package under test.

collectReferenced currently records every referenced object from any _test.go file, while pkgPath is unused (see Line 273). That allows unrelated package tests to satisfy coverage for a symbol, which weakens the rule’s stated same-package/foo_test contract.

💡 Proposed fix

 func collectReferenced(
 	file *ast.File,
 	info *types.Info,
 	pkgPath string,
 	referenced map[string]bool,
 ) {
 	if file == nil {
 		return
 	}
 	ast.Inspect(file, func(n ast.Node) bool {
 		switch x := n.(type) {
 		case *ast.Ident:
 			if info == nil {
 				return true
 			}
 			obj := info.Uses[x]
 			if obj == nil {
 				return true
 			}
 			if obj.Pkg() == nil {
 				return true
 			}
+			objPkg := strings.TrimSuffix(strings.TrimSuffix(obj.Pkg().Path(), "_test"), ".test")
+			if objPkg != pkgPath {
+				return true
+			}
 			recordRef(obj, referenced)
 		case *ast.SelectorExpr:
 			if info == nil {
 				return true
 			}
-			if obj := info.Uses[x.Sel]; obj != nil && obj.Pkg() != nil {
+			if obj := info.Uses[x.Sel]; obj != nil && obj.Pkg() != nil {
+				objPkg := strings.TrimSuffix(strings.TrimSuffix(obj.Pkg().Path(), "_test"), ".test")
+				if objPkg != pkgPath {
+					return true
+				}
 				recordRef(obj, referenced)
 			}
 		}
 		return true
 	})
-	_ = pkgPath
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/bifrostlint/analyzers/exportedtests/exportedtests.go` around lines 240 -
274, collectReferenced is recording symbols from any package because pkgPath is
ignored; change it to only call recordRef when the referenced object's package
path equals the package-under-test (use pkgPath). In both places where you get
obj (the *types.Obj from info.Uses inside collectReferenced for *ast.Ident and
*ast.SelectorExpr), add a guard that obj.Pkg() != nil && obj.Pkg().Path() ==
pkgPath before calling recordRef(referenced), so only same-package references
are recorded; keep using the existing recordRef and referenced map.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Narrow direct identifier matching to avoid cross-analyzer false positives.

Treating *ast.Ident as DB execution for names like Get/Select can flag unrelated helper calls and cascade noisy findings in both SQL analyzers.

Suggested fix

 func isDBExecCall(call *ast.CallExpr) bool {
 	switch fn := call.Fun.(type) {
 	case *ast.SelectorExpr:
 		return dbExecMethods[fn.Sel.Name]
 	case *ast.Ident:
-		return dbExecMethods[fn.Name]
+		// Keep direct function support only for unambiguous execution names.
+		switch fn.Name {
+		case "Exec", "ExecContext", "MustExec", "Query", "QueryContext", "QueryRow", "QueryRowContext":
+			return true
+		default:
+			return false
+		}
 	}
 	return false
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	case *ast.Ident:
	return dbExecMethods[fn.Name]
	}
	case *ast.Ident:
	// Keep direct function support only for unambiguous execution names.
	switch fn.Name {
	case "Exec", "ExecContext", "MustExec", "Query", "QueryContext", "QueryRow", "QueryRowContext":
	return true
	default:
	return false
	}
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/bifrostlint/internal/sqlscan/exec.go` around lines 43 - 45, The current
switch treats any bare *ast.Ident as a DB exec call (using
dbExecMethods[fn.Name]), causing false positives; restrict detection to selector
calls only by removing or disabling the *ast.Ident branch and instead match on
*ast.SelectorExpr (use sel.Sel.Name) and, if available, confirm the qualifier
(sel.X) is a DB-like receiver via type info (pkg.TypesInfo) or known var names
before consulting dbExecMethods; update the logic that references the case
*ast.Ident, fn.Name, and dbExecMethods accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Usage text is out of sync with the actual default analyzer set.

Line 5 says bifrostlint ./... runs only sqlmatview + sqldialect, but Lines 29-33 also include sqlconcurrent. Update the usage comment to prevent operator confusion.

Suggested patch

-//	bifrostlint ./...                            # runs sqlmatview + sqldialect
+//	bifrostlint ./...                            # runs sqlconcurrent + sqlmatview + sqldialect

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// bifrostlint ./... # runs sqlmatview + sqldialect
	// bifrostlint ./... # runs sqlconcurrent + sqlmatview + sqldialect

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/bifrostlint/main.go` at line 5, The usage comment in
cmd/bifrostlint/main.go is out of sync: the current comment "bifrostlint ./... 
# runs sqlmatview + sqldialect" omits the sqlconcurrent analyzer that is
actually included (see the analyzer list around the analyzer registration in the
file). Update that single-line usage comment to list all default analyzers
(sqlmatview, sqldialect, sqlconcurrent) so the usage text matches the analyzers
registered in main (referencing the usage/comment string near the top of main.go
and the analyzer registration block where sqlconcurrent is added).

[greptile-apps]

Confidence Score: 4/5

Safe to merge; the new linter tool runs entirely at analysis time and has no runtime effect on the application.

All four analyzers and their tests look correct. The only concerns are housekeeping items in exportedtests.go (dead exportedFile map, unused pkgPath parameter, empty if block) and sqlscan.go (uncached regexes, dead ContainsPhrase). None affect the tool's correctness today.

cmd/bifrostlint/analyzers/exportedtests/exportedtests.go (dead code); cmd/bifrostlint/internal/sqlscan/sqlscan.go (uncached regexes, dead ContainsPhrase)

Important Files Changed

Filename	Overview
cmd/bifrostlint/analyzers/exportedtests/exportedtests.go	New cross-package exported-symbols-must-have-tests analyzer; contains dead exportedFile map, unused pkgPath parameter, and an empty no-op if hasLoadErrors block
cmd/bifrostlint/internal/sqlscan/sqlscan.go	SQL-scanning helpers; ContainsKeyword and ContainsPhrase recompile their regex on every call, and ContainsPhrase is never used in the codebase
cmd/bifrostlint/analyzers/sqlconcurrent/sqlconcurrent.go	New analyzer flagging CREATE INDEX without CONCURRENTLY; dialect-guard and early-return detection logic is correct
cmd/bifrostlint/analyzers/sqldialect/sqldialect.go	New analyzer enforcing Postgres-only SQL inside dialect guards; detection logic is sound
cmd/bifrostlint/analyzers/sqlmatview/sqlmatview.go	New analyzer requiring materialized-view DDL to run in a background context; background detection via goroutine, name prefix, and doc-comment annotation is correct
cmd/bifrostlint/internal/ignore/ignore.go	Suppression-directive parser; correctly handles trailing same-line and preceding doc-comment cases with mandatory reason field
cmd/bifrostlint/main.go	Entry point; routes exportedtests subcommand before multichecker.Main, design is intentional and correct
framework/configstore/migrations.go	Added bifrostlint:ignore sqlconcurrent suppressions to migration-transaction CREATE INDEX statements with clear justification comments
.github/workflows/lint.yml	New CI workflow running golangci-lint and bifrostlint on PRs and pushes to dev/main
.golangci.yml	New golangci-lint v2 config enabling errcheck, forbidigo (json.Marshal ban scoped to core/framework), govet, ineffassign, staticcheck, unused

_{Reviews (1): Last reviewed commit: "linter" | Re-trigger Greptile}

[greptile-apps]

exportedFile map is allocated and written but never read

exportedFile is populated in three places inside collectExported (lines 172, 188, 199) but nothing in Run ever reads it. It consumes memory proportional to the number of exported symbols across all packages and has no effect on the tool's output. Either use it for richer diagnostics or drop it (along with the exportedFile parameter from collectExported).

[greptile-apps]

pkgPath is passed into collectReferenced but is immediately discarded with _ = pkgPath. The parameter serves no purpose and should be removed from the signature.

Suggested change

	func collectReferenced(
	file *ast.File,
	info *types.Info,
	pkgPath string,
	referenced map[string]bool,
	) {
	if file == nil {
	return
	}
	ast.Inspect(file, func(n ast.Node) bool {
	switch x := n.(type) {
	case *ast.Ident:
	if info == nil {
	return true
	}
	obj := info.Uses[x]
	if obj == nil {
	return true
	}
	if obj.Pkg() == nil {
	return true
	}
	recordRef(obj, referenced)
	case *ast.SelectorExpr:
	if info == nil {
	return true
	}
	if obj := info.Uses[x.Sel]; obj != nil && obj.Pkg() != nil {
	recordRef(obj, referenced)
	}
	}
	return true
	})
	_ = pkgPath
	}
	func collectReferenced(
	file *ast.File,
	info *types.Info,
	referenced map[string]bool,
	) {
	if file == nil {
	return
	}
	ast.Inspect(file, func(n ast.Node) bool {
	switch x := n.(type) {
	case *ast.Ident:
	if info == nil {
	return true
	}
	obj := info.Uses[x]
	if obj == nil {
	return true
	}
	if obj.Pkg() == nil {
	return true
	}
	recordRef(obj, referenced)
	case *ast.SelectorExpr:
	if info == nil {
	return true
	}
	if obj := info.Uses[x.Sel]; obj != nil && obj.Pkg() != nil {
	recordRef(obj, referenced)
	}
	}
	return true
	})
	}

[greptile-apps]

Regex recompiled on every ContainsKeyword and ContainsPhrase call

Both functions call regexp.MustCompile inline on every invocation. ContainsAnyKeyword calls ContainsKeyword once per needle, and sqldialect passes four fixed needles for every string literal in every file. The code comment acknowledges the missing cache but defers it. A simple sync.Map keyed on the pattern string would eliminate the overhead. ContainsPhrase is also never called anywhere in the current codebase and appears to be dead code.

[greptile-apps]

Empty if hasLoadErrors block is confusing

The block has no executable body; the intent is conveyed only by the comment inside it. An empty if reads as accidental missing code to reviewers and linters alike. The comment can be moved to an unconditional position above the block, or the block dropped entirely.