`the-ai-factory/review.md`

37 lines

## Snapshot
- Local AI artifact factory: prompt → Claude Code subagent/skill draft → review → manual activation.
- Stack: Node.js, Express, vanilla HTML/JS, PostgreSQL, Ollama, Claude CLI, Playwright.
- How it's run: manual `node server.js`/`node viewer.js`; `package.json:11` has PM2 command for both.
- Status: live-local; recent generated outputs and wired pipeline.

## Top Risks (P0 — fix this week)
- `server.js:67`, `server.js:95`, `server.js:31`: no auth plus `Access-Control-Allow-Origin: *`; any webpage Steve visits can POST to localhost and trigger builds/activation. Fix: require `ADMIN_TOKEN` Bearer auth on all mutating routes and restrict CORS to the viewer origin.
- `server.js:95`, `server.js:104`, `server.js:143`: activation copies LLM-generated files into `~/.claude` with only status gating, and `?force=true` bypasses approval. Fix: remove force from HTTP or require token + explicit local CLI confirmation; show/record a deterministic diff before copy.
- `src/stages/scaffold.js:16`, `src/stages/smoke_test.js:20`: generated subagents may receive `Bash`, `Write`, or `Edit`; smoke tests only check that tools exist, not whether they are safe. Fix: parse frontmatter and deny dangerous tools unless explicitly allowlisted by artifact type/request.
- Secrets leaked in code: none observed; checked-in `.env` is blank (`.env:4`, `.env:10`, `.env:14`).
- SQL injection: none observed; queries use placeholders.
- Obvious dependency CVEs from package files: none observed.

## Notable Issues (P1 — fix this month)
- `server.js:78`: pipeline is fire-and-forget; process crash after insert leaves runs stuck with no recovery. Fix: add a resumable worker loop or mark stale `running` jobs failed on boot.
- `src/stages/memory_write.js:32`: read-modify-write append to `MEMORY.md` races under concurrent activations and can drop entries. Fix: serialize with a lockfile or append atomically.
- `src/pipeline.js:52`, `src/llm.js:51`: no concurrency limit around Ollama/Claude CLI; multiple POSTs can saturate the machine/quota. Fix: single-flight queue or max concurrency of 1-2.
- `README.md:17` vs `package.json:8-9`: docs say `npm run start` boots orchestrator and viewer, but it only starts `server.js`. Fix docs or change `start` to run both.
- `PLAN.md:106-109`: completed items are duplicated as open TODOs, conflicting with lines `101-105`. Fix the plan so operator status is trustworthy.
- `package.json:15`, `.env.example:4`, `src/llm.js:5`: Anthropic SDK/API key remain in config despite “No Anthropic API.” Fix: remove unused dependency/env or document why retained.
- Tests: no project tests observed; risky because activation and path-sandbox checks are critical.

## Polish (P2 — when convenient)
- `public/index.html:69`: hardcoded orchestrator port ignores `ORCHESTRATOR_PORT`. Serve config from backend or derive from env-injected script.
- `output/`: generated run artifacts/screenshots are not ignored by `.gitignore`; add `output/` unless these are intentional fixtures.
- `server.js`: `ADMIN_TOKEN` exists in env templates but is unused; either wire it or remove it.

## Strengths
- SQL access is parameterized throughout the reviewed routes.
- Activation has useful sandbox path validation before copying (`server.js:118-122`).
- The `dw_unified` guard is explicit and prevents accidental use of the wrong production-ish DB (`server.js:10-15`).

## Open questions for the owner
- Should browser access be allowed at all, or should activation be CLI-only?
- Are generated artifacts intended to be committed, or should `output/` stay local?
- Is PM2 already running this, or is it still manual during development?