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.
Full Public Reader
Code Review: Agent Swarm
Summary
- 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 |
---
Critical Findings
| 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`. |
---
High Findings
| 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 providers. | `github.ts`, `linear.ts` |
| A8 | API | Supabase response `as StoredWorkItem[]` without runtime validation. | `supabase-store.ts:119` |
| P3 | Perf | `fullTextAccumulator` in Claude adapter grows unbounded (no cap). | `claude.ts:373` |
| P4 | Perf | Codex event buffer drops oldest events including critical `turn_complete`/`session_end`. | `codex.ts:555` |
| P5 | Perf | Linear `mapIssues()` fires 3 parallel sub-fetches per issue. 50 issues = 150 concurrent API calls. | `linear.ts:124` |
| P7 | Perf | `execSync` in Discord `sendToChannel()` blocks the entire event loop. | `discord.ts:43` |
| D1 | Deps | All 10 workspace packages declare zero dependencies despite importing externals. | All `package.json` |
| D7 | Deps | `bun test` script exists but zero test files in the project. | `package.json:10` |
---
Medium Findings
| ID | Pass | Description | File |
|---|---|---|---|
| S6 | Security | Unvalidated status in Supabase query filter via `as any` cast. | `supabase-store.ts:156` |
| S7 | Security | POST `/api/work-items` accepts untrusted JSON with no schema validation. | `api-server.ts:80` |
| S8 | Security | No rate limiting on webhook endpoints. | `webhook-server.ts:15` |
| S9 | Security | Error responses leak internal implementation details. | `webhook-server.ts:62` |
| C6 | Concurrency | Webhook handler reads `req.text()` without payload size limit. | `webhook-server.ts:43` |
| C7 | Concurrency | Audit log write in `transition()` never checks response status. | `supabase-store.ts:279` |
| C8 | Concurrency | File watcher AbortController may never be released on error. | `prompt-renderer.ts:561` |
| C9 | Concurrency | All Supabase `fetch()` calls lack AbortSignal timeouts. | `supabase-store.ts` (multiple) |
| C10 | Concurrency | Event listeners on child process streams not explicitly removed. | `workspace-manager.ts:161` |
| A2 | API | `WorkflowConfig.agent` is plain string, should be `AgentKind` enum. | `prompt-renderer.ts:20` |
| A9 | API | Approval mode switch cases lack exhaustive `never` check. | `claude.ts:96` |
| A10 | API | No schema validation on manual work item creation. | `api-server.ts:82` |
| A11 | API | `startSession({resumeId})` accepts resume on adapters that don't support it. | `interface.ts:172` |
| A12 | API | `resolveNextStatus()` lacks exhaustive check, can return undefined. | `transitions.ts:66` |
| P6 | Perf | `new Set(issue.labels)` recreated inside workflow match loop for every workflow. | `prompt-renderer.ts:472` |
| P9 | Perf | `/api/work-items` has no hard max limit. Fetches all then slices in memory. | `api-server.ts:59` |
| P10 | Perf | Sequential `getByIdentifier()` lookups per source event. Should batch with `WHERE IN`. | `orchestrator.ts:103` |
| P11 | Perf | `SENSITIVE_PATTERNS` array tested per key per field. Should be single combined regex. | `logger.ts:61` |
| P12 | Perf | Conversation history `.slice()` copies entire array on every cap hit. Use circular buffer. | `gemini.ts:150` |
| D2 | Deps | `@linear/sdk: ^77.0.0` caret range allows untested patch versions. | `package.json:17` |
| D3 | Deps | `@types/bun: ^1.3.10` caret may introduce type mismatches with locked bun-types. | `package.json:14` |
| D6 | Deps | Transitive peer deps (graphql, undici-types) not explicitly declared. | `bun.lock` |
| D10 | Deps | TypeScript not in devDependencies. `bunx tsc` downloads unpinned version. | `package.json:11` |
---
Cross-Cutting Patterns
| Pattern | Affected Passes | Finding IDs | Root Cause |
|---|---|---|---|
| No input validation at trust boundaries | Security, API, Perf | S6, S7, A1, A10, C6, P9 | API endpoints accept raw JSON without schema validation. Status fields use `as any` casts. No payload size limits. |
| Missing timeouts on external I/O | Concurrency, Perf | C9, P5, P7, C7 | All Supabase fetch calls, Linear SDK calls, and Discord `execSync` lack timeouts or AbortSignals. Network hangs block indefinitely. |
| Stringly-typed where enums should be | API, Security | A2, A4, A9, A12, S6 | Status fields, agent kinds, approval modes, and error codes are plain strings despite having well-defined enum sets. No exhaustive switch checks. |
| No test infrastructure | DX, Deps | X3, D7 | Zero test files. `bun test` passes vacuously. No test framework deps. State machine transitions, webhook parsing, and dispatch logic are untested. |
| Unsafe concurrent state access | Concurrency | C1, C3, C4, C12 | `activeSessions`, `agentLoad` Maps and interval handles accessed from multiple async contexts without synchronization or lifecycle tracking. |
| Committed secrets / credential hygiene | Security, DX | S1, X1 | `.env` with real credentials committed to git. `.env.example` doesn't clearly mark required vs optional vars. |
---
Blind Spots
- `packages/metrics/src/index.ts`: Counter/Gauge/Histogram implementations not stress-tested for label cardinality explosion. No pass examined what happens with 10K+ unique label combinations.
- `packages/prompt-renderer/src/index.ts`: Liquid template engine not fuzz-tested for deeply nested conditionals or self-referential templates (could infinite loop in `processConditionals` safety limit of 50 iterations).
---
Priority Matrix
| Rank | ID | Severity | Blast Radius | Fix Effort | Score |
|---|---|---|---|---|---|
| 1 | S1 | Critical | Org-wide (all Supabase + GitHub) | 10min | 100 |
| 2 | S3 | High | Full system (arbitrary code exec) | 30min | 90 |
| 3 | A1 | Critical | Data integrity (invalid state queries) | 10min | 85 |
| 4 | S2 | High | Webhook auth bypass | 15min | 80 |
| 5 | S4 | High | GitHub API abuse | 15min | 80 |
| 6 | C1 | Critical | Memory leak, unclean shutdown | 10min | 75 |
| 7 | C4 | High | Session count corruption | 30min | 75 |
| 8 | P7 | High | Event loop blocked per Discord msg | 15min | 70 |
| 9 | S7 | Medium | DoS via malformed payloads | 30min | 65 |
| 10 | C9 | Medium | Hung connections, resource exhaust | 30min | 65 |
---
Remediation Roadmap
Wave 1: Credential Rotation + Security Hardening (30min)
Files: `.env`, `github.ts`, `workspace-manager.ts`, `api-server.ts`
1. S1: Rotate Supabase service key, GitHub PAT, webhook secret. Remove `.env` from git history.
2. S2: Fix HMAC timing side-channel. Always compute expected signature before comparing.
3. S3: Restrict `runHook()` to a whitelist of known hook commands. No arbitrary `sh -c`.
4. S4: Validate `owner`/`repo` against `[a-zA-Z0-9._-]+` regex in `parseExternalId()`.
5. A1: Add status validation in API server before passing to `listByStatus()`.
Wave 2: Concurrency + Resource Safety (45min)
Files: `orchestrator.ts`, `supabase-store.ts`, `webhook-server.ts`
1. C1: Store Linear polling interval handle, clear on shutdown.
2. C4: Add simple async mutex around `activeSessions`/`agentLoad` access.
3. C3: Store session promises, await them with timeout on shutdown.
4. C6: Add `Content-Length` check (10MB max) on webhook payloads.
5. C9: Wrap all Supabase `fetch()` in `fetchWithTimeout()` helper (10s default).
Wave 3: Type Safety + API Contracts (45min)
Files: `interface.ts`, `transitions.ts`, `prompt-renderer.ts`, `supabase-store.ts`
1. A4: Use `AgentErrorCode` enum in `AgentEvent.error.code` instead of `string?`.
2. A9/A12: Add exhaustive `never` checks to all switch statements.
3. A2: Change `WorkflowConfig.agent` from `string` to `AgentKind`.
4. A8: Add runtime type guard for Supabase response validation.
5. S5: Replace custom YAML parser's blocklist with a property key allowlist.
Wave 4: Performance Quick Wins (30min)
Files: `discord.ts`, `prompt-renderer.ts`, `orchestrator.ts`, `claude.ts`
1. P7: Replace `execSync` with `Bun.spawn` + async in Discord sendToChannel.
2. P6: Create `issueLabels` Set once outside the workflow match loop.
3. P3: Cap `fullTextAccumulator` at 1MB.
4. P10: Batch `getByIdentifier` checks with single `WHERE identifier IN (...)` query.
Wave 5: DX + Test Infrastructure (1hr)
Files: `.github/workflows/ci.yml`, test files, `package.json`
1. X2: Create GitHub Actions CI workflow.
2. X3/D7: Add test files for state machine transitions, dispatch logic, webhook parsing.
3. D1: Declare dependencies in workspace package.json files.
4. D10: Pin TypeScript in devDependencies.
5. X8: Restructure `.env.example` with required/optional sections.
Promotion Decision
Keep as idea/proposal unless evidence and implementation anchors exist.
Source Anchor
projects/agent-swarm/CODE_REVIEW.md
Detected Structure
Method · Code Anchors · Architecture