From 0619e2b617f8ea484a530f127acb644ec9c18cca Mon Sep 17 00:00:00 2001
From: Rodin <rodin@forgedthought.ai>
Date: Sun, 10 May 2026 14:09:03 -0700
Subject: [PATCH] fix: address review feedback on check-deps script

- Accept import paths starting with digit (e.g., 9fans.net/go) by
  relaxing filter from ^[a-zA-Z] to ^[[:alnum:]]
- Fix misleading comment: loop uses Bash process substitution, not POSIX
- Remove redundant \$ from grep regex (literal $ unreachable)
- Capture stderr separately in go list to avoid mixing errors with output
- Clarify CONVENTIONS.md wording on transitive vs direct deps

Reviewed-by: sonnet-review-bot
Reviewed-by: gpt-review-bot
---
 CONVENTIONS.md        |  2 +-
 scripts/check-deps.sh | 18 ++++++++++++------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/CONVENTIONS.md b/CONVENTIONS.md
index 5c3839c..2e96112 100644
--- a/CONVENTIONS.md
+++ b/CONVENTIONS.md
@@ -14,7 +14,7 @@
 
 **Any import not in this table or the Go standard library is forbidden.**
 
-Transitive dependencies of approved packages are automatically allowed.
+Only *direct* dependencies (listed in go.mod without `// indirect`) are checked against this allowlist. Transitive dependencies pulled in by approved packages are implicitly allowed.
 
 To request a new dependency:
 1. Open a PR that ONLY updates this table
diff --git a/scripts/check-deps.sh b/scripts/check-deps.sh
index f88efd2..97c0906 100755
--- a/scripts/check-deps.sh
+++ b/scripts/check-deps.sh
@@ -23,7 +23,8 @@ if [ ! -f "$CONVENTIONS_FILE" ]; then
     exit 1
 fi
 
-# Parse approved packages from CONVENTIONS.md table using awk (POSIX-compatible)
+# Parse approved packages from CONVENTIONS.md table
+# Note: uses Bash process substitution (< <(...)) for the loop
 # Format: | `package` | use case | scope |
 declare -A ALLOWED_PROD=()
 declare -A ALLOWED_TEST=()
@@ -33,7 +34,8 @@ while IFS= read -r line; do
     pkg=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]*`|`[[:space:]]*$/, "", $2); print $2}')
     scope=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]+|[[:space:]]+$/, "", $4); print tolower($4)}')
     
-    if [ -n "$pkg" ] && [ "$pkg" != "Package" ] && [[ "$pkg" =~ ^[a-zA-Z] ]]; then
+    # Accept packages starting with letter or digit (e.g., 9fans.net/go)
+    if [ -n "$pkg" ] && [ "$pkg" != "Package" ] && [[ "$pkg" =~ ^[[:alnum:]] ]]; then
         if [[ "$scope" == *"test"* ]]; then
             ALLOWED_TEST["$pkg"]=1
         else
@@ -69,8 +71,12 @@ matches_allowlist() {
 }
 
 # Get direct module dependencies from go.mod
-DIRECT_IMPORTS=$(go list -m -f '{{if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all 2>&1) || {
-    echo "❌ Failed to list dependencies: $DIRECT_IMPORTS"
+# Capture stderr separately to avoid mixing error messages with package list
+GO_LIST_STDERR=$(mktemp)
+trap 'rm -f "$GO_LIST_STDERR"' EXIT
+DIRECT_IMPORTS=$(go list -m -f '{{if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all 2>"$GO_LIST_STDERR") || {
+    echo "❌ Failed to list dependencies:"
+    cat "$GO_LIST_STDERR"
     exit 1
 }
 DIRECT_IMPORTS=$(echo "$DIRECT_IMPORTS" | grep -v '^$' || true)
@@ -106,8 +112,8 @@ PROD_IMPORTS=$(go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}' ./.
 
 TEST_ONLY_IN_PROD=""
 for test_pkg in "${!ALLOWED_TEST[@]}"; do
-    # Use word-boundary matching: exact match or followed by /
-    if echo "$PROD_IMPORTS" | grep -qE "^${test_pkg}(/|\$|$)"; then
+    # Match exact package or subpackages (pkg or pkg/...)
+    if echo "$PROD_IMPORTS" | grep -qE "^${test_pkg}(/|$)"; then
         TEST_ONLY_IN_PROD="${TEST_ONLY_IN_PROD}  - ${test_pkg} (marked 'test only' but used in production code)"$'\n'
     fi
 done