feat: Audit remediation — Alpha → Beta (P0 + P1)

[calebrosario]

Summary

Comprehensive audit remediation based on a full 82-file, ~11,800 LOC codebase review. This PR moves agentic-armor from Alpha to Beta by fixing critical blockers, wiring stubs, eliminating type safety issues, and establishing consistent naming.

74 files changed · +5,853 / -1,937 lines · 183 new tests

---

P0 — Fix Immediately (Blocks Everything)

Task	What Changed
P0-1	Removed stray } in src/docker/manager.ts:55 that caused TS1128
P0-2	Fixed 7 failing test suites: converted bun:test → @jest/globals, fixed hook type signature mismatch (Error → string), added typed Jest mocks, added describe.skip for empty suite, fixed Docker mock interop (__esModule: true)
P0-3	Resolved MCP identity crisis — server implements raw HTTP POST, not the Model Context Protocol. Renamed src/mcp/ → src/api/, updated all imports, config vars (MCP_* → API_*), type names (MCPRequest → ApiRequest), and documentation

P1 — Before Beta

Task	What Changed
P1-1	Deleted stale .bak / .backup2 files via git rm
P1-2	Replaced 4 hardcoded stub tools in server.ts with 8 real implementations from tools.ts (Zod validation, TaskRegistry, TaskLifecycle, DockerManager)
P1-3	Replaced Math.random() < 0.1 fake conflict with real version-based optimistic locking. Added expectedVersion? parameter to acquireLock → acquireLockWithRetry chain
P1-4	Created src/util/retry.ts with isTransientError() classifier and withRetry() exponential backoff (1s→2s→4s, 3 retries). Wrapped 4 critical Docker lifecycle ops (create, start, stop, remove). Added RetryableDockerError class
P1-5	Created 3 type guards in src/util/errors.ts: isNodeError, isErrorWithCode, isErrorWithStatusCode. Eliminated all 9 (error as any).code / .statusCode casts across multi-layer.ts, docker-helper.ts, network-manager.ts
P1-6	Added typed resetInstance(): void to 30 singleton classes (20 new + 10 existing fixed from undefined as any → undefined!). Replaced 5 as any casts in test files
P1-7	Unified naming to "agentic-armor" / "Agentic Armor" across package.json, README, AGENTS.md, docs, monitoring headers. Was 4 conflicting names
P1-8	Added 5 new test files with 79 new tests: task/lifecycle.test.ts (14), persistence/multi-layer.test.ts (12), util/retry.test.ts (13), util/optimistic-lock.test.ts (18), util/errors-custom.test.ts (22)
P1-9	Deleted duplicate migration 0000_high_mastermind.sql (identical CREATE TABLE to 0000_initial_schema.sql)
P1-10	Removed 2 unnecessary as any casts in config/index.ts (LOG_LEVEL and NODE_ENV already validated by Zod enums)
P1-GATE	✅ type-check clean · ✅ build clean · ✅ 405 tests pass · ⚠️ 13 pre-existing lint errors (untouched)

---

Before → After Metrics

Metric	Before	After	Delta
TypeScript errors	2	0	-2
Test failures	7	1 (pre-existing)	-6
Test suites passing	28	39	+11
Total tests passing	222	405	+183
API tools (stubbed)	4	0	-4
API tools (wired)	0	8	+8
Singleton classes with resetInstance()	10	30	+20
(error as any) casts	9	0	-9
Project names	4	1	-3

New Files

• src/util/retry.ts — Retry utility with exponential backoff
• .eslintrc.json — ESLint config (was missing)
• tests/task/lifecycle.test.ts — Task lifecycle state machine tests
• tests/persistence/multi-layer.test.ts — Multi-layer persistence tests
• tests/util/retry.test.ts — Retry utility tests
• tests/util/optimistic-lock.test.ts — Optimistic locking tests
• tests/util/errors-custom.test.ts — Custom error class tests

Key Decisions

1. P0-3 MCP → HTTP API: Chose rename over implementing real MCP protocol (30min vs 4h). Can add MCP transport as a future P2 item.
2. P1-6 undefined! pattern: Used TypeScript non-null assertion on undefined for singleton reset instead of as any — undefined! evaluates to never which is assignable to all types.
3. P1-7 Naming: Unified on "agentic-armor" (matches CLI binary and package intent).

What's Next (P2 — not in this PR)

• P2-1: HTTP auth (API key / bearer token)
• P2-2: Persist metrics to PostgreSQL
• P2-3: Task retry/resume
• P2-4: Break up docker/manager.ts (1402 lines)
• P2-5: Upgrade ESLint 8→10
• P2-6: Wire webhook alerting
• P2-7: Session module decision (delete or document)
• P2-8: Custom error classes consistently

[calebrosario]

🔍 Comprehensive PR Review Summary

Review Agents: Code Quality, Test Coverage, Error Handling, Type Design

---

🚨 Critical Issues (9 found) - Must Fix Before Merge

Code Quality

1. src/mcp/tools.ts:108,200,245 - Task-to-container mapping missing

   • const containerId = params.taskId; assumes taskId === containerId
   • Fix: Query TaskRegistry for actual containerId

2. src/hooks/safety-hooks/resource-monitor.ts:26,81-87 - Global Map race condition

   • activeMonitors Map has no locking; intervals leak on task crash
   • Fix: Per-instance Map; register stop hook on AfterTaskComplete

3. src/util/network-isolator.ts:387-402 - Host iptables exec security hole

   • child_process.exec('iptables ...') requires root; fails non-root
   • Fix: Use Docker-only isolation (internal: true bridge)

Error Handling

4. src/util/network-isolator.ts:388-402 - execIptables silently swallows errors

   • Promise always resolve() despite exec errors
   • Fix: if (error) reject(error); before resolve()

5. src/util/network-isolator.ts:364-366 - removeNetworkIsolation swallows errors

   • .catch(() => {}) silently ignores chain cleanup failures
   • Fix: .catch((err) => logger.warn("Chain cleanup failed", { err }))

Test Coverage

6. tests/docker/manager.test.ts - No Dockerode exec mocks

   • Tests only check method existence, no behavior validation
   • Fix: Mock docker.getContainer().exec() with exitCode/output assertions

7. tests/docker/manager.test.ts - No timeout error tests

   • Missing mock for exec timeout rejection
   • Fix: Mock exec.start() with Promise.reject(new Error('timeout'))

Type Design

8. src/docker/manager.ts - Loose Dockerode types with as any

   • Heavy use of as any breaks type safety
   • Fix: Define strict interfaces; use type guards

9. src/mcp/tools.ts - Untyped Record<string, any> params

   • No per-tool schema validation
   • Fix: Define typed interfaces per tool with Zod validation

---

⚠️ Important Issues (10 found) - Should Fix

File	Issue	Fix
src/docker/manager.ts:943-969	demuxStream uses as any for writers	Define WritableChunk interface
src/docker/manager.ts:861-902	Fragile digest regex on stream chunks	Use modern pull API; inspect post-pull
src/mcp/tools.ts:153-167	list_tasks returns empty stub	Implement taskRegistry.listTasks()
src/hooks/safety-hooks/resource-monitor.ts:147-151	Hardcoded /10000 pids limit	Fetch from container.HostConfig.PidsLimit
src/util/network-isolator.ts:30,404	Global Maps race on multi-task	Use WeakMap or per-instance
src/util/network-isolator.ts:435-444	tcpdump/pkill fire-and-forget	Promisify exec; handle errors
tests/hooks/git-hooks.test.ts	No error propagation tests	Mock createTaskBranch failure
tests/hooks/plan-hooks.test.ts	Only tests task not found	Add task exists path with fs.writeFile
src/docker/container-config.ts	ImagePullResult lacks discriminated union	Use {status: 'failed', error: string}
src/mcp/tools.ts	Inconsistent return types	Define ToolResult<T> union

---

💡 Suggestions (8 found) - Nice to Have

• src/docker/manager.ts:1001-end - Split 200+ line buildCreateOptions into sub-builders
• src/hooks/safety-hooks/isolation-checker.ts:58-91 - Remove dead/unused verify functions
• src/hooks/safety-hooks/isolation-checker.ts:23-35 - Throw on !isIsolated instead of warn
• src/hooks/safety-hooks/resource-monitor.ts:53-59 - Add failure backoff for repeated errors
• src/docker/manager.ts:1378-1383 - Global init catch swallows startup failure
• tests/persistence/checkpoint-optimizer.test.ts - No rotation execution tests
• src/config/index.ts - Add grouped WebhookConfig with dependent validation
• src/docker/container-config.ts - Add invariants comment for ExecResult

---

✅ Strengths

• Test stabilization excellent: Fixed global mock pollution; singleton resets work
• Error wrapping good: OpenCodeError with context throughout Docker manager
• Logging comprehensive: All major operations logged with structured data
• Timeout handling: execInContainer/ensureImage properly implement timeouts
• MCP server robust: All paths log/respond with errors; no swallows detected
• Persistence tests strong: Real fs/risk logic, pollution fixed via resets

---

📊 Statistics

Metric	Value
Files Changed	35
Lines Added	+1,920
Lines Deleted	-671
Tests	234 pass, 3 skip, 0 fail

---

🎯 Recommended Action

1. Block merge until CRITICAL issues #1-9 are fixed
2. Address IMPORTANT issues in follow-up PR
3. Consider SUGGESTIONS as tech debt backlog

Top Priority Fixes:

1. Add task-to-container mapping in MCP tools
2. Add Dockerode mocks to Docker manager tests
3. Fix execIptables error handling
4. Remove host iptables exec (use Docker-only isolation)