Code Review: PR #46

Commit: 3586278e302d351a26f34ab275237f960cbff0f0

Summary

This PR makes all 14 ApexStack hooks work when cwd is inside a workspace project directory (e.g., workspace/curios-dog/backend/) instead of requiring the ops repo root. It does this via (a) an inline bash -c resolver in settings.json that walks up from $PWD to find onboarding.yaml, and (b) --repo flag parsing in 4 cross-repo hooks so gh API calls target the correct repository.

Checklist Results

• Architecture & Design: Pass -- the resolver pattern is simple, consistent across all 14 hooks, and the --repo parsing in shell hooks is a reasonable approach for cross-repo support.
• Code Quality: Pass with one issue (see below) -- the hook changes are clean and follow existing patterns.
• Testing: N/A -- shell hook infrastructure; the PR description documents manual test coverage.
• Security: Pass -- no secrets, no injection vectors. The $REPO_FLAG is derived from command parsing and passed to gh which validates it.
• Performance: Pass -- the resolver is a simple directory walk, not on a hot path.
• PR Description & Glossary: Pass -- clear summary, change table, test plan, and glossary present.
• Technical Decisions (AgDR): N/A -- this is a bug fix / infrastructure improvement, not a new technology choice or architecture decision. The resolver pattern is a minimal shell idiom, not an architectural decision requiring an AgDR.

Issues Found

1. $schema URL changed to a broken URL (non-blocking but should fix)

In .claude/settings.json line 2, the $schema was changed from:

"$schema": "https://json.schemastore.org/claude-code-settings.json"

to:

"$schema": "https://json-schema.org/claude-code-settings.json"

The old URL (json.schemastore.org) redirects to a valid schema (HTTP 200). The new URL (json-schema.org) returns HTTP 404. This appears to be an accidental edit -- it is unrelated to the hook resolver feature. Should be reverted to the original URL.

Suggestions (non-blocking)

1. --repo=value syntax not parsed

The sed pattern 's/.*--repo[[:space:]]+([^[:space:]]+).*/\1/p' only handles --repo value (space-separated). The gh CLI also accepts --repo=value (equals-sign). If someone uses the equals form, CMD_REPO will be empty and the hook falls back to local repo context -- safe degradation, not a crash, but worth noting. A future enhancement could handle both:

sed -nE 's/.*--repo[=[:space:]]+([^[:space:]]+).*/\1/p'

2. Resolver failure mode when onboarding.yaml is missing entirely

If the walker reaches / without finding onboarding.yaml, r becomes empty string, and exec "/.claude/hooks/..." will fail with "No such file or directory". This is acceptable behavior (the hook effectively becomes a no-op with a non-zero exit), but a future improvement could add a guard: [ -z "$r" ] || [ "$r" = "/" ] && exit 0.

Verdict

COMMENT -- The code logic is correct and well-structured. The only concrete issue is the broken $schema URL which should be reverted before merge. The --repo= and resolver-failure-mode points are non-blocking suggestions for future hardening.

Reviewed by Rex (Code Reviewer Agent)
Reviewed commit: 3586278e302d351a26f34ab275237f960cbff0f0