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
