From 6e3fc6cc2a9e08e17ab9763bd021ed0fff0e26c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Tue, 22 Sep 2026 09:50:15 +0100 Subject: [PATCH] perf(hooks): lint only changed Go packages (#3860) Co-authored-by: Kandev Agent --- .pre-commit-config.yaml | 7 +- docs/plans/go-pre-commit-lint/plan.md | 74 ++++++++ .../task-01-scope-go-lint.md | 108 +++++++++++ docs/specs/platform/README.md | 3 + .../requirements/go-pre-commit-lint.md | 60 ++++++ .../system-design/go-pre-commit-lint.md | 65 +++++++ scripts/lint-go-changed | 30 +++ scripts/lint-go-changed.test.py | 178 ++++++++++++++++++ 8 files changed, 522 insertions(+), 3 deletions(-) create mode 100644 docs/plans/go-pre-commit-lint/plan.md create mode 100644 docs/plans/go-pre-commit-lint/task-01-scope-go-lint.md create mode 100644 docs/specs/platform/requirements/go-pre-commit-lint.md create mode 100644 docs/specs/platform/system-design/go-pre-commit-lint.md create mode 100644 scripts/lint-go-changed create mode 100644 scripts/lint-go-changed.test.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6f33818349..2282481925 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -49,11 +49,12 @@ repos: pass_filenames: true - id: go-lint - name: Go lint (PR changed code) - 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' + name: Go lint (changed packages) + entry: bash scripts/lint-go-changed language: system files: ^apps/backend/.*\.go$ - pass_filenames: false + pass_filenames: true + require_serial: true # Web app (TypeScript/React) hooks - repo: local diff --git a/docs/plans/go-pre-commit-lint/plan.md b/docs/plans/go-pre-commit-lint/plan.md new file mode 100644 index 0000000000..bae0069066 --- /dev/null +++ b/docs/plans/go-pre-commit-lint/plan.md @@ -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. diff --git a/docs/plans/go-pre-commit-lint/task-01-scope-go-lint.md b/docs/plans/go-pre-commit-lint/task-01-scope-go-lint.md new file mode 100644 index 0000000000..373ad87ee2 --- /dev/null +++ b/docs/plans/go-pre-commit-lint/task-01-scope-go-lint.md @@ -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. diff --git a/docs/specs/platform/README.md b/docs/specs/platform/README.md index 5f2710b334..4482f2e9cb 100644 --- a/docs/specs/platform/README.md +++ b/docs/specs/platform/README.md @@ -15,6 +15,9 @@ The platform system owns cross-cutting runtime services, configuration, observability, notifications, localization, lifecycle safety, and shared 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. Domain systems retain ownership of source records and workspace context identity. diff --git a/docs/specs/platform/requirements/go-pre-commit-lint.md b/docs/specs/platform/requirements/go-pre-commit-lint.md new file mode 100644 index 0000000000..d6a2de6bbe --- /dev/null +++ b/docs/specs/platform/requirements/go-pre-commit-lint.md @@ -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) diff --git a/docs/specs/platform/system-design/go-pre-commit-lint.md b/docs/specs/platform/system-design/go-pre-commit-lint.md new file mode 100644 index 0000000000..47ed07ce06 --- /dev/null +++ b/docs/specs/platform/system-design/go-pre-commit-lint.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. diff --git a/scripts/lint-go-changed b/scripts/lint-go-changed new file mode 100644 index 0000000000..85779b07d6 --- /dev/null +++ b/scripts/lint-go-changed @@ -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 diff --git a/scripts/lint-go-changed.test.py b/scripts/lint-go-changed.test.py new file mode 100644 index 0000000000..370319b7e3 --- /dev/null +++ b/scripts/lint-go-changed.test.py @@ -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()