perf(hooks): lint only changed Go packages (#3860)

Co-authored-by: Kandev Agent <agent@kandev.dev>
This commit is contained in:
João Salavisa
2026-09-22 09:50:15 +01:00
committed by GitHub
parent 82d52844b7
commit 6e3fc6cc2a
8 changed files with 522 additions and 3 deletions
+4 -3
View File
@@ -49,11 +49,12 @@ repos:
pass_filenames: true pass_filenames: true
- id: go-lint - id: go-lint
name: Go lint (PR changed code) name: Go lint (changed packages)
entry: bash -c 'unset GIT_DIR GIT_WORK_TREE; base=$(scripts/resolve-go-lint-base --comparison-base) || exit $?; cd apps/backend && golangci-lint run ./... --new-from-rev="$base" --timeout=5m' entry: bash scripts/lint-go-changed
language: system language: system
files: ^apps/backend/.*\.go$ files: ^apps/backend/.*\.go$
pass_filenames: false pass_filenames: true
require_serial: true
# Web app (TypeScript/React) hooks # Web app (TypeScript/React) hooks
- repo: local - repo: local
+74
View File
@@ -0,0 +1,74 @@
---
created: 2026-09-22
status: done
requirements:
- REQ-PLATFORM-GO-PRE-COMMIT-LINT-001
system_design:
- ../../specs/platform/system-design/go-pre-commit-lint.md
legacy_specs: []
---
# Implementation Plan: Go pre-commit lint
## Overview
Limit commit-time Go lint to the packages represented by pre-commit's selected
files. One work order owns hook wiring, the wrapper, regression coverage, and
the linked documentation. This record includes the implementation and PR
remediation; external CI and merge evidence remain in the PR task record.
## Scope
- Select and deduplicate complete containing packages, with serial execution.
- Preserve comparison-base policy, error propagation, and pre-commit staging.
- Isolate Git repository context from inherited location and storage overrides.
- Exclude changes to application behavior, other hooks, and full-backend CI.
## Technical approach
`.pre-commit-config.yaml` forwards filenames to `scripts/lint-go-changed`.
The wrapper selects exact package paths and reuses `scripts/resolve-go-lint-base`.
The regression suite drives real Git/pre-commit and substitutes only the linter.
## Tests
All named tests are in `scripts/lint-go-changed.test.py`.
| Acceptance criteria | Evidence |
| --- | --- |
| `AC-PLATFORM-GO-PRE-COMMIT-LINT-001.1`, `.2` | `test_lints_each_staged_package_once`, `test_lints_multiple_packages_in_one_process_without_recursing` |
| `AC-PLATFORM-GO-PRE-COMMIT-LINT-001.3` | `test_non_go_commit_skips_lint`, `test_empty_package_selection_never_falls_back_to_full_lint` |
| `AC-PLATFORM-GO-PRE-COMMIT-LINT-001.4` | `test_lint_failure_blocks_commit`, `test_invalid_comparison_base_fails_before_lint`, and the 15 existing resolver checks |
| `AC-PLATFORM-GO-PRE-COMMIT-LINT-001.5` | `test_ignores_inherited_repository_environment` |
| `AC-PLATFORM-GO-PRE-COMMIT-LINT-001.6` | `test_lints_each_staged_package_once` preserves unstaged edits |
| `AC-PLATFORM-GO-PRE-COMMIT-LINT-001.7` | Inspection confirms the CI and Makefile lint commands retain `./...`; neither file is modified |
## Work orders
- [x] [Task 01: Scope Go commit lint to selected packages](task-01-scope-go-lint.md)
## Verification results
The original package-selection regression failed on `./...` before the wrapper
was introduced. The inherited-environment regression failed during base resolution
before the additional Git variables were cleared.
- All seven hook tests and all 15 resolver checks passed.
- The real `go-lint` hook passed for `internal/common/gitref/gitref.go`.
- Pre-commit configuration and Bash syntax validation passed.
- The documentation catalog validated 294 decisions and 1077 specifications.
- All 36 specification-linter tests and the complete specification lint passed.
- The PR coverage evaluator returned `covered` for the changed files and this
linked delivery package.
- `git diff --check` passed. Public docs need no change because product behavior
and public APIs are unaffected.
Remote CI, automated review disposition, and merge status are tracked externally
on PR #3860; these local results do not claim those gates are complete.
## Risks
- Go still loads dependencies for type information; package selection does not
guarantee a fixed latency improvement.
- Local lint does not analyze unchanged reverse dependents. Full-backend CI
retains that responsibility.
@@ -0,0 +1,108 @@
---
id: "01-scope-go-lint"
title: "Scope Go commit lint to selected packages"
status: done
wave: 1
depends_on: []
plan: "plan.md"
requirements:
- REQ-PLATFORM-GO-PRE-COMMIT-LINT-001
acceptance_criteria:
- AC-PLATFORM-GO-PRE-COMMIT-LINT-001.1
- AC-PLATFORM-GO-PRE-COMMIT-LINT-001.2
- AC-PLATFORM-GO-PRE-COMMIT-LINT-001.3
- AC-PLATFORM-GO-PRE-COMMIT-LINT-001.4
- AC-PLATFORM-GO-PRE-COMMIT-LINT-001.5
- AC-PLATFORM-GO-PRE-COMMIT-LINT-001.6
- AC-PLATFORM-GO-PRE-COMMIT-LINT-001.7
system_design:
- ../../specs/platform/system-design/go-pre-commit-lint.md
---
# Task 01: Scope Go commit lint to selected packages
## Summary
Pass selected filenames to a package-scoping wrapper and preserve lint failures,
comparison-base policy, and staged-file handling. Cover inherited Git repository
overrides and record the completed implementation's delivery contracts.
## In scope
- Hook filename forwarding and serial execution.
- Package selection, deduplication, empty selection, and Git environment isolation.
- Focused process-boundary tests and linked requirement/design documentation.
## Out of scope
- Other hooks, lint rules, backend CI selection, and application changes.
- Deleted-only changes and changes to module/configuration triggers.
## Acceptance
- Selected source/test files produce exact containing-package targets once per batch.
- Empty selections succeed without lint; invalid bases and lint failures block commits.
- Inherited Git overrides do not redirect the wrapper, and unstaged edits survive.
## Verification
Run from the repository root:
```bash
python3 scripts/lint-go-changed.test.py
bash scripts/resolve-go-lint-base.test.sh
pre-commit run go-lint --files apps/backend/internal/common/gitref/gitref.go
pre-commit validate-config
bash -n scripts/lint-go-changed
python3 scripts/list-docs.py validate
python3 scripts/lint-spec-files.test.py
python3 scripts/lint-spec-files.py --all
git diff --check
```
## Files likely touched
- `.pre-commit-config.yaml`
- `scripts/lint-go-changed`
- `scripts/lint-go-changed.test.py`
- `docs/specs/platform/requirements/go-pre-commit-lint.md`
- `docs/specs/platform/system-design/go-pre-commit-lint.md`
- `docs/specs/platform/README.md`
- `docs/plans/go-pre-commit-lint/plan.md`
- This work order.
## Dependencies
None.
## Risks
Dependencies still load for type checking; full-backend CI covers cross-package
effects outside the selected lint targets.
## Parallelism
`sequential`
## Inputs
- [Requirements](../../specs/platform/requirements/go-pre-commit-lint.md)
- [System design](../../specs/platform/system-design/go-pre-commit-lint.md)
- Existing hook configuration, resolver, and resolver tests.
## Results
Package-selection and inherited-environment regressions were observed before
their fixes. Final local results:
- `python3 scripts/lint-go-changed.test.py`: 7 passed.
- `bash scripts/resolve-go-lint-base.test.sh`: 15 checks passed.
- The explicit `pre-commit run go-lint` command above passed with the real linter.
- `pre-commit validate-config` and `bash -n scripts/lint-go-changed`: passed.
- `python3 scripts/list-docs.py validate`: 294 decisions and 1077 specifications.
- `python3 scripts/lint-spec-files.test.py`: 36 passed.
- `python3 scripts/lint-spec-files.py --all` and `git diff --check`: passed.
- The repository PR documentation evaluator accepted this work order and its
linked plan, requirement, and design with status `covered`.
PR #3860 owns subsequent remote CI, review, and merge evidence.
+3
View File
@@ -15,6 +15,9 @@ The platform system owns cross-cutting runtime services, configuration,
observability, notifications, localization, lifecycle safety, and shared observability, notifications, localization, lifecycle safety, and shared
operational guarantees. operational guarantees.
Repository validation performance, including local commit checks and shared CI
execution, belongs here. The CI system retains hosted workflow trust policy.
Shared read-capacity protection and aggregate reporting availability belong here. Shared read-capacity protection and aggregate reporting availability belong here.
Domain systems retain ownership of source records and workspace context identity. Domain systems retain ownership of source records and workspace context identity.
@@ -0,0 +1,60 @@
---
status: active
system: platform
created: 2026-09-22
owners:
- kandev
---
# Go pre-commit lint requirements
## Overview
Contributors need Go commit checks whose analysis targets follow the staged
change. Platform owns development validation performance and shared operational
guarantees; the CI system retains ownership of hosted workflow trust policy.
## Terminology
- **Selected files:** Existing Go source or test files supplied to the commit
check. Ordinary commits select staged files; explicit manual runs can select
other files.
- **Selected package:** The complete Go package containing a selected file.
Loading dependencies for type information does not select them as lint targets.
## Requirements
### REQ-PLATFORM-GO-PRE-COMMIT-LINT-001: Bound local Go lint analysis
**Intent:** Avoid analyzing unrelated backend packages on every commit while
retaining package context and changed-code enforcement.
#### Acceptance criteria
- **AC-PLATFORM-GO-PRE-COMMIT-LINT-001.1:** Selected source and test files shall
select their complete containing packages. Unrelated packages and unchanged
descendants shall not become lint targets solely because another package changed.
- **AC-PLATFORM-GO-PRE-COMMIT-LINT-001.2:** Each package shall appear once in each
supplied batch. Batches shall run serially to avoid competing linter processes.
- **AC-PLATFORM-GO-PRE-COMMIT-LINT-001.3:** A selection without eligible Go files
shall finish without invoking the linter or falling back to a full scan.
- **AC-PLATFORM-GO-PRE-COMMIT-LINT-001.4:** The check shall preserve the existing
pull-request comparison-base and changed-code reporting policy. Base-resolution
errors and linter failures shall prevent the commit.
- **AC-PLATFORM-GO-PRE-COMMIT-LINT-001.5:** Inherited repository-location, index,
and object-storage overrides shall not redirect the check to another repository.
- **AC-PLATFORM-GO-PRE-COMMIT-LINT-001.6:** Ordinary commits shall retain
pre-commit's staged-file selection and restoration of unstaged edits.
- **AC-PLATFORM-GO-PRE-COMMIT-LINT-001.7:** CI and the explicit full-backend lint
command shall retain backend-wide analysis, including impacts on other packages.
## Out of scope
- Changed-line-only compilation, reverse-dependency selection, or lint caching.
- Changing other commit hooks, lint rules, comparison-base precedence, or CI scope.
- Adding checks for deleted-only files or module/configuration-only changes.
- Application behavior and user interfaces.
## Implementation plans
- [Go pre-commit lint](../../../plans/go-pre-commit-lint/plan.md)
@@ -0,0 +1,65 @@
---
status: current
system: platform
requirements:
- REQ-PLATFORM-GO-PRE-COMMIT-LINT-001
---
# Go pre-commit lint system design
## Purpose and boundaries
The local Go lint hook selects package targets from pre-commit's filenames.
Platform owns this contributor feedback contract alongside
[CI performance](ci-performance.md). Hosted workflow authorization remains
with the CI system.
## Requirement mapping
| Requirement | Design sections |
| --- | --- |
| `REQ-PLATFORM-GO-PRE-COMMIT-LINT-001` | [Selection and execution](#selection-and-execution), [Repository context and failures](#repository-context-and-failures), [Verification boundaries](#verification-boundaries) |
## Selection and execution
`.pre-commit-config.yaml` passes repository-relative filenames to
`bash scripts/lint-go-changed` and sets `require_serial: true`. The existing
`^apps/backend/.*\.go$` filter controls eligibility. Pre-commit continues to
own staging, exclusion of removed files, and temporary handling of unstaged edits.
The wrapper accepts backend Go paths, converts each to its containing directory
relative to `apps/backend`, and deduplicates the package argument array. Root
files select `.`; nested files select their exact `./internal/example` package,
without a recursive `/...` suffix. Quoted arrays preserve argument boundaries.
An empty package array exits successfully before resolving the comparison base.
Each non-empty batch invokes one `golangci-lint run` with those packages,
`--new-from-rev` set to the existing resolver result, and `--timeout=5m`.
Pre-commit may split exceptionally large filename lists into serial batches.
Dependencies can still be loaded for type information.
## Repository context and failures
Before Git discovery, the wrapper clears `GIT_DIR`, `GIT_WORK_TREE`,
`GIT_COMMON_DIR`, `GIT_INDEX_FILE`, `GIT_OBJECT_DIRECTORY`, and
`GIT_ALTERNATE_OBJECT_DIRECTORIES`. It locates the checkout from the wrapper's
own path, then runs `scripts/resolve-go-lint-base --comparison-base` there.
Authentication and transport environment settings remain available.
The resolver retains configured-base, canonical-upstream, merge-base, and
incoming-base-merge behavior. A resolution failure exits before lint. The
wrapper uses `exec` for the linter so its diagnostics and failure status reach
pre-commit directly. There is no fallback to a different comparison base or to
backend-wide analysis.
## Verification boundaries
`scripts/lint-go-changed.test.py` exercises real Git and pre-commit with a
recording linter at the external process boundary. It covers staged selection,
package deduplication, root and nested packages, empty selections, unstaged-edit
restoration, linter failures, invalid bases, and inherited Git overrides.
`scripts/resolve-go-lint-base.test.sh` retains the existing resolver coverage.
`.github/workflows/backend-tests.yml` and `apps/backend/Makefile` continue to
select `./...`. Cross-package effects remain subject to that broader validation;
the local optimization is not a replacement for CI.
+30
View File
@@ -0,0 +1,30 @@
#!/usr/bin/env bash
# Pre-commit supplies repository-relative Go filenames. Lint their complete
# packages for type information; CI owns the full-backend scan.
set -euo pipefail
unset GIT_DIR GIT_WORK_TREE GIT_COMMON_DIR GIT_INDEX_FILE \
GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES
cd "$(dirname "${BASH_SOURCE[0]}")/.."
packages=()
for file in "$@"; do
case "$file" in
apps/backend/*.go) ;;
*) continue ;;
esac
package="./${file#apps/backend/}"
package=${package%/*}
for existing in ${packages[@]+"${packages[@]}"}; do
if [ "$existing" = "$package" ]; then
continue 2
fi
done
packages+=("$package")
done
[ "${#packages[@]}" -gt 0 ] || exit 0
base=$(scripts/resolve-go-lint-base --comparison-base)
cd apps/backend
exec golangci-lint run "${packages[@]}" --new-from-rev="$base" --timeout=5m
+178
View File
@@ -0,0 +1,178 @@
#!/usr/bin/env python3
"""Exercise the Go lint hook with real Git/pre-commit and a recording linter."""
import json
import os
import shutil
import subprocess
import tempfile
import textwrap
import unittest
from pathlib import Path
ROOT = Path(__file__).resolve().parents[1]
class ChangedGoLintTest(unittest.TestCase):
def setUp(self):
temporary = tempfile.TemporaryDirectory(prefix="go lint hook ")
self.addCleanup(temporary.cleanup)
self.repo = Path(temporary.name)
self.calls = self.repo / "lint-calls.jsonl"
self.env = {key: value for key, value in os.environ.items() if not key.startswith("GIT_")}
self.env.pop("SKIP", None)
self.env["GO_LINT_TEST_CALLS"] = str(self.calls)
self.env["PATH"] = f"{self.repo / 'bin'}{os.pathsep}{self.env['PATH']}"
self.write(
"bin/golangci-lint",
textwrap.dedent("""\
#!/usr/bin/env python3
import json
import os
import sys
with open(os.environ["GO_LINT_TEST_CALLS"], "a") as calls:
calls.write(json.dumps({"cwd": os.getcwd(), "args": sys.argv[1:]}) + "\\n")
sys.exit(int(os.environ.get("GO_LINT_TEST_EXIT", "0")))
"""),
).chmod(0o755)
shutil.copy2(ROOT / ".pre-commit-config.yaml", self.repo)
(self.repo / "scripts").mkdir()
for name in ("resolve-go-lint-base", "lint-go-changed"):
source = ROOT / "scripts" / name
if source.exists():
shutil.copy2(source, self.repo / "scripts" / name)
self.write("apps/backend/internal/first/one.go", "package first\n")
self.write("apps/backend/internal/first/two_test.go", "package first\n")
self.write("apps/backend/internal/second/other.go", "package second\n")
self.git("init", "-q", "-b", "main")
self.git("config", "core.hooksPath", str(self.repo / "no-hooks"))
self.git("config", "user.name", "Hook test")
self.git("config", "user.email", "hook@example.test")
self.git("add", ".")
self.git("-c", "commit.gpgsign=false", "commit", "-qm", "initial")
self.git("update-ref", "refs/remotes/origin/main", "HEAD")
self.git("symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/main")
self.base = self.git("rev-parse", "HEAD").stdout.strip()
def write(self, relative, content):
target = self.repo / relative
target.parent.mkdir(parents=True, exist_ok=True)
target.write_text(content, encoding="utf-8")
return target
def git(self, *args):
return subprocess.run(
["git", *args], cwd=self.repo, env=self.env, text=True, capture_output=True, check=True
)
def run_hook(self, *args):
return subprocess.run(
["pre-commit", "run", "go-lint", *args],
cwd=self.repo, env=self.env, text=True, capture_output=True,
)
def lint_calls(self):
if not self.calls.exists():
return []
return [json.loads(line) for line in self.calls.read_text().splitlines()]
def test_lints_each_staged_package_once(self):
for filename in ("one.go", "two_test.go"):
self.write(f"apps/backend/internal/first/{filename}", "package first\n// staged\n")
self.git("add", "apps/backend/internal/first")
unstaged = "package second\n// unstaged\n"
other = self.write("apps/backend/internal/second/other.go", unstaged)
result = self.run_hook()
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.lint_calls(), [{
"cwd": str(self.repo / "apps/backend"),
"args": ["run", "./internal/first", f"--new-from-rev={self.base}", "--timeout=5m"],
}])
self.assertEqual(other.read_text(), unstaged)
def test_lints_multiple_packages_in_one_process_without_recursing(self):
files = [f"apps/backend/internal/first/file{index}.go" for index in range(12)]
files.extend([
"apps/backend/internal/first/child/child.go",
"apps/backend/internal/second/other.go",
"apps/backend/main.go",
])
for filename in files:
self.write(filename, "package fixture\n")
result = self.run_hook("--files", *files)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
calls = self.lint_calls()
self.assertEqual(len(calls), 1, calls)
self.assertCountEqual(calls[0]["args"], [
"run", ".", "./internal/first", "./internal/first/child", "./internal/second",
f"--new-from-rev={self.base}", "--timeout=5m",
])
def test_non_go_commit_skips_lint(self):
self.write("README.md", "Fixture\n")
self.git("add", "README.md")
result = self.run_hook()
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.lint_calls(), [])
def test_empty_package_selection_never_falls_back_to_full_lint(self):
for args in ([], ["README.md", "apps/web/example.ts", "other/example.go"]):
with self.subTest(args=args):
result = subprocess.run(
["bash", "scripts/lint-go-changed", *args],
cwd=self.repo, env=self.env, text=True, capture_output=True,
)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.lint_calls(), [])
def test_lint_failure_blocks_commit(self):
self.env["GO_LINT_TEST_EXIT"] = "7"
result = self.run_hook("--files", "apps/backend/internal/first/one.go")
self.assertNotEqual(result.returncode, 0)
self.assertIn("exit code: 7", result.stdout)
self.assertEqual(len(self.lint_calls()), 1)
def test_invalid_comparison_base_fails_before_lint(self):
self.git("config", "kandev.lintBaseRef", "origin/missing")
result = self.run_hook("--files", "apps/backend/internal/first/one.go")
self.assertNotEqual(result.returncode, 0)
self.assertIn("base ref does not resolve locally", result.stdout)
self.assertEqual(self.lint_calls(), [])
def test_ignores_inherited_repository_environment(self):
env = {
**self.env,
"GIT_DIR": str(self.repo / "foreign.git"),
"GIT_WORK_TREE": str(self.repo / "foreign-worktree"),
"GIT_COMMON_DIR": str(self.repo / "foreign-common"),
"GIT_INDEX_FILE": str(self.repo / "foreign-index"),
"GIT_OBJECT_DIRECTORY": str(self.repo / "foreign-objects"),
"GIT_ALTERNATE_OBJECT_DIRECTORIES": str(self.repo / "foreign-alternates"),
}
result = subprocess.run(
["bash", "scripts/lint-go-changed", "apps/backend/internal/first/one.go"],
cwd=self.repo, env=env, text=True, capture_output=True,
)
self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
self.assertEqual(self.lint_calls(), [{
"cwd": str(self.repo / "apps/backend"),
"args": ["run", "./internal/first", f"--new-from-rev={self.base}", "--timeout=5m"],
}])
if __name__ == "__main__":
unittest.main()