MAJOR:
- LoadRepoPersonas: add MaxPersonaFileSize check before parsing to
prevent resource exhaustion from oversized YAML files committed
to target repositories
MINOR:
- isNotFoundError: tighten substring match to 'HTTP 404' only to
avoid masking auth/transport errors containing generic 'not found'
- main.go: remove duplicate flag.Parse() call
- main.go: add comment explaining nil map indexing is safe in Go
when LoadRepoPersonas returns an error
Tests updated to reflect the intentional behavior change in
isNotFoundError and added test case for oversized file rejection.
Implements #60.
- Add ParsePersonaBytes() for parsing personas from byte data
- Add LoadRepoPersonas() to fetch personas from repo via Gitea API
- Add MergePersonas() to combine built-in and repo personas
- Add GetBuiltinPersonasMap() helper
- Update main.go to load repo personas first, fall back to built-in
- Add giteaClientAdapter to bridge gitea.Client to review.GiteaClient
When --persona is specified, the bot now:
1. Attempts to fetch personas from .review-bot/personas/*.yaml
2. If the named persona exists in the repo, uses it
3. Otherwise falls back to built-in personas
This allows repos to define domain-specific personas (e.g., trading
experts for gargoyle, crypto experts for kms-lite) without modifying
the review-bot codebase.
Addresses PR #58 MINOR finding: YAML decoder now rejects unknown fields.
- Enable KnownFields(true) on YAML decoder to catch typos like
'focuss' or 'identiy' in persona files
- Since yaml.Node.Decode() doesn't support KnownFields, we now
do a two-pass decode: first pass checks depth limits, second
pass decodes with strict field checking
- Add tests for unknown field rejection at top-level and nested levels
Addresses GPT review feedback:
1. MAJOR - Test deps now validated: All direct module deps (from go.mod)
are checked against the allowlist, whether used in prod or tests.
2. MINOR - Prefix match: Uses grep -E with word boundary (^pkg(/|$|$))
to avoid false positives on similarly-prefixed modules.
3. MINOR - Bash version check: Script now fails early with helpful
message if Bash < 4 (macOS default). Added shebang: #!/usr/bin/env bash
4. NIT - Removed redundant grep -v '_test' (go list -deps already
excludes test-only deps without -test flag).
Addresses review feedback:
1. MAJOR - Scope enforcement: Script now parses the Scope column and
ensures 'test only' packages don't appear in non-test code. Uses
'go list -deps' to check production imports.
2. MINOR - Portability: Replaced 'grep -P' (GNU-only) with awk-based
parsing that works on macOS/BSD.
3. MINOR - Robustness: Table parsing uses awk to split on '|' and
extract columns properly, handling whitespace variations.
4. MINOR - Glob safety: Prefix matching now uses parameter expansion
instead of glob patterns to prevent metacharacter issues.
Fixes:
- Single source of truth: script now parses allowlist from CONVENTIONS.md
- Fail closed: script exits non-zero if 'go list' fails
- Direct deps only: uses '-f' flag to exclude transitive deps
- Added 'precommit' to .PHONY in Makefile
- Removed unused ALLOWED_PATTERN variable
- Added Scope column to distinguish test-only vs production deps
- Clarified that transitive deps of approved packages are allowed
- Added note that enforcement script parses the table
STRICT ALLOWLIST policy: Only packages explicitly listed in CONVENTIONS.md
may be imported. No exceptions.
## Changes
- Updates CONVENTIONS.md with strict allowlist language
- Adds scripts/check-deps.sh to enforce the allowlist
- Adds 'make check-deps' and 'make precommit' targets
- CI will fail if any unapproved dependency is detected
## Approved packages
- gopkg.in/yaml.v3 — YAML parsing
- github.com/google/go-cmp — test comparisons
## Process for new dependencies
1. Open a PR that ONLY updates CONVENTIONS.md
2. Requires explicit approval from Aaron
3. After merge, a separate PR may use the package
When a new push arrives while review-bot is processing, the review
would be posted against a stale commit. This causes noise in the
PR timeline with findings that reference code that no longer exists.
Before posting, re-fetch PR metadata and compare HEAD SHA with the
commit we evaluated against. If they differ, log a warning and exit
successfully — a new workflow run should already be processing the
new HEAD.
Fixes#52
Rodin
