`visual-factory/rereview.md`

20 lines
## Verdict
FIX BEFORE SHIP

## Patches reviewed
src/llm.js:10 — hardcoded user-specific Claude fallback — now defaults to `claude` and spawn call uses it at src/llm.js:96 — STATUS good  
server.js:221 — deep health needed same fallback — now matches adapter default — STATUS good  
server.js:223 — executable lookup helper added — covers absolute/relative/PATH, but only checks existence — STATUS concern  
server.js:318 — health probe switched to helper — validates `claude` path lookup, but inherits false-positive executable check — STATUS concern  
.env.example:14 — example copied local path — now documents portable `claude` default — STATUS good  

## New issues introduced
server.js:225 — `fs.access(command)` defaults to existence only, so a non-executable absolute/relative `CLAUDE_CLI` passes `/health/deep` while `spawn()` later fails with EACCES — severity P2  
server.js:231 — PATH lookup also uses existence only, so a non-executable file named `claude` earlier in PATH can make health green while critic execution fails — severity P2  

## Missed call-sites (same problem elsewhere)
none observed

## Recommendations
- Use `fs.access(candidate, fs.constants.X_OK)` in both helper branches, and reject directories if needed before reporting `claude_cli: ok`.
- Keep server and adapter defaults as-is; the portability change itself is consistent.