Code Review: Agent Swarm

- **Findings**: 66 total (4 critical, 18 high, 23 medium, 21 low) - **Cross-Cutting Patterns**: 6 identified - **Overall Health**: Functional but needs hardening before production use. Core architecture is sound. Security and input validation are the most urgent gaps. | Pass | Critical | High | Medium | Low | Total | |------|----------|------|--------|-----|-------| | Security & Input Validation | 1 | 4 | 4 | 2 | 11 | | Concurrency & Resource Safety | 1 | 4 | 5 | 2 | 12 | | API Design & Type Safety | 1 | 6 | 4 | 2 | 13 | | Dependency Health | 0 | 2 | 5 | 8 | 15 | | Performance & Scalability | 0 | 5 | 6 | 4 | 15 | | Developer Experience | 3 | 6 | 5 | 4 | 18 | | ID | Pass | Description | File | Fix | |----|------|-------------|------|-----| | S1 | Security | `.env` committed with real Supabase service key, GitHub token, webhook secret | `.env` | Rotate all credentials immediately. Remove from git history with `git filter-repo`. | | C1 | Concurrency | Linear polling `setInterval` never stored or cleared on shutdown. Leaks timer. | `orchestrator.ts:344` | Store handle in `linearPollingHandle`, clear in `shutdown()`. | | A1 | API | `as any` cast on user-supplied status string bypasses type safety entirely. | `api-server.ts:64` | Validate against `VALID_STATUSES` set before casting. | | X2 | DX | No CI/CD workflow. No `.github/workflows/` directory. | project root | Create `ci.yml` with `bun install && bun lint && bun test`. | | ID | Pass | Description | File | |----|------|-------------|------| | S2 | Security | HMAC verification has timing side-channel via early return on missing signature. | `github.ts:74` | | S3 | Security | `runHook()` passes untrusted string to `sh -c`, enabling command injection. | `workspace-manager.ts:150` | | S4 | Security | `parseExternalId()` does not validate owner/repo, allowing path traversal in GitHub API URLs. | `github.ts:394` | | S5 | Security | YAML parser prototype pollution guards use blocklist only. | `prompt-renderer.ts:149` | | C3 | Concurrency | Background session runner has no cancellation tracking or promise handle storage. | `orchestrator.ts:266` | | C4 | Concurrency | `activeSessions` Map accessed concurrently from dispatch loop and session runner without mutex. | `orchestrator.ts:254,454` | | C5 | Concurrency | Timer in `runHook()` not cleared on all abort code paths. | `workspace-manager.ts:146` | | A3 | API | Silent `JSON.parse` failure on Claude stream output hides malformed events. | `claude.ts:403` | | A4 | API | `AgentErrorCode` enum exists but `AgentEvent.error.code` is `string?`. Enum never used. | `interface.ts:102` | | A6 | API | `Record<string, unknown>` metadata bags everywhere. No discriminated types. | Multiple files | | A7 | API | Inconsistent null-vs-throw error contracts across source prov