# Architecture Review — Theseus

> Attack path graph for penetration testing. Rust CLI + axum REST API + embedded Cytoscape.js frontend.
> Reviewed: 2026-06-06

## Summary

Theseus is a well-structured single-binary Rust application with a clear client-server split. The architecture is pragmatic for a pentest tool — not over-engineered, but has some structural debt accumulating in `lib.rs` and `db.rs`. The frontend is cleanly decoupled. Overall solid for a domain tool, with specific areas needing attention before the codebase grows further.

---

## Evaluation

### 1. Separation of Concerns

| | |
|---|---|
| **Score** | **B** |
| **Evidence** | Clear 3-layer split: CLI (`main.rs:1-2657`) → HTTP → Server (`lib.rs` + `routes/*.rs`) → Database (`db.rs`). CLI has zero direct DB access — all subcommands except `serve` are pure HTTP clients (`main.rs:9`, `const DEFAULT_SERVER`). Frontend communicates exclusively via REST + WebSocket (`static/js/api.js:authFetch`, `static/js/ws.js:connectWs`). Route handlers follow a uniform pattern: extract `State<AppState>`, resolve IDs, call `db.*`, broadcast via `tx.send()`. 15 route modules in `routes/mod.rs:1-15`. The `agent/` subsystem is cleanly isolated with its own LLM client, memory store, tool registry, and runtime loop. |
| **Recommendation** | `lib.rs` (901 lines) is doing too many things: `AppError`, `AppState`, `Config`, middleware, `deepseek_chat()`, and three `resolve_*_id()` helpers all live in one file. Extract `AppError` into `src/error.rs`, config loading into `src/config.rs`, and ID resolution into `src/resolve.rs`. The `deepseek_chat()` helper at `lib.rs:539-599` duplicates what `agent/llm.rs:LlmClient` already does — consolidate into a single LLM client. |

### 2. Dependency Direction

| | |
|---|---|
| **Score** | **A** |
| **Evidence** | Dependencies flow cleanly downward with no cycles. `types.rs` is a pure leaf module (no internal imports). `db.rs` imports only `crate::types::*` (`db.rs:8`). Routes import `crate::AppState` and `crate::types::*` — they access the database through `state.db` (e.g., `nodes.rs:52`). `agent/` imports `crate::db::Database` and `crate::types::*` directly (`agent/loop.rs:5-6`, `agent/tools.rs:1-2`). `main.rs` imports only `theseus::types::*` (`main.rs:7`). The dependency graph is strictly acyclic: `main` → `lib` → `routes` → `db` → `types`. |
| **Recommendation** | The one concern: `agent/tools.rs` imports `crate::db::Database` directly (`tools.rs:1`) rather than going through an abstraction. This is fine for now but means the agent tools can never be tested without the real `Database`. Consider a trait if tool testing becomes important. |

### 3. Coupling

| | |
|---|---|
| **Score** | **B-** |
| **Evidence** | Route handlers are tightly coupled to `AppState` struct internals — they access `state.db`, `state.tx`, and `state.config` directly (e.g., `nodes.rs:48`, `analysis.rs:41`, `chat.rs:91`). The `Database` struct itself uses `pub struct Database { conn: Mutex<Connection> }` (`db.rs:12-14`) — every method acquires the lock via `fn c()` (`db.rs:17-19`). Row deserializers use hardcoded column indices: `row.get(0)`, `row.get(1)`, etc. (`db.rs:90-98`, `db.rs:100-122`, `db.rs:124-138`, `db.rs:140-163`). Any SELECT column reordering breaks deserialization silently. The `ToolContext` struct (`tools.rs:8-12`) duplicates `AppState` shape (`db`, `project_id`, `tx`) instead of extracting from it. |
| **Recommendation** | (1) Use named column access (`row.get::<_, String>("id")`) instead of positional indices in all `row_*` functions — this is the single highest-risk coupling point. (2) Make `ToolContext` derive from `AppState` or accept `&AppState` rather than duplicating the shape. (3) Consider a `Database` trait for testability if the codebase grows beyond its current scope. |

### 4. Cohesion

| | |
|---|---|
| **Score** | **B+** |
| **Evidence** | Each route module has a clear single responsibility: `nodes.rs` handles node CRUD + filtering, `analysis.rs` handles BFS attack paths, `chat.rs` handles multi-turn conversation, etc. The `agent/` subsystem has clean internal module boundaries: `llm.rs` (HTTP client), `loop.rs` (turn orchestration), `memory.rs` (persistence), `prompts.rs` (prompt engineering), `tools.rs` (tool definitions + dispatch). Frontend JS modules are also well-scoped: `graph.js` (Cytoscape rendering), `projects.js` (project CRUD), `credentials.js` (credential panel), etc. |
| **Recommendation** | Analysis-related types (`AttackPath`, `PathNode`, `TaggedNode`, `NextStep`, `CrossConnection`, `HealthIssue`, `CheckIssue`, `CheckResult`, `AttackAnalysis`) are defined in `db.rs:724-799` instead of `types.rs`. This breaks the "single source of truth" pattern documented in `modules.md`. Move these to `types.rs` or create `src/types/analysis.rs`. Also, `lib.rs` contains ~200 lines of `resolve_*_id()` logic (`lib.rs:601-781`) that is effectively "domain resolution" — it belongs in its own module. |

### 5. Extensibility

| | |
|---|---|
| **Score** | **A-** |
| **Evidence** | Adding a new route module is straightforward: create `routes/foo.rs`, export `pub(crate) fn router() -> Router<AppState>`, merge in `lib.rs:serve()`. The pattern is repeated 15 times with zero boilerplate beyond the function signature. Adding new agent tools follows the same pattern: define a `fn() -> Tool` in `tools.rs:33-48`, register in `registry()`. The `NodeType` enum uses `#[serde(untagged)] Custom(String)` (`types.rs:54-55`) — any new node type works automatically. WebSocket broadcast is decoupled: any handler can `state.tx.send("entity_action:{pid}")` and the frontend reacts. The `Config` struct supports multiple sources (`lib.rs:353-410`): CLI args → env vars → config file. |
| **Recommendation** | The merge-based router registration (`lib.rs:814-829`) is manual — adding a route module requires editing `lib.rs`. Consider a macro or dynamic registration pattern if the route count grows past ~20. The hardcoded model name `"deepseek-v4-flash"` in `deepseek_chat()` (`lib.rs:548`) should be configurable via `Config`. |

### 6. Design Patterns

| | |
|---|---|
| **Score** | **B+** |
| **Evidence** | Appropriate patterns used without over-engineering: `Box::leak` for `&'static Database` (`lib.rs:791-793`) — pragmatic choice to avoid lifetime complexity in axum's `State` extractor. `Mutex<Connection>` with poison recovery (`db.rs:17-19`) — correct for SQLite's single-writer model. `broadcast::channel` for WebSocket fan-out (`lib.rs:795`) — idiomatic tokio pattern. Middleware stack is clean: CORS → tracing → JSON error wrapper → API key auth (`lib.rs:868-883`). Structured error handling via `AppError` with bilingual messages and builder pattern (`lib.rs:27-37`) is well-designed. Integration tests spawn real axum servers with in-memory SQLite (`modules.md` notes). |
| **Recommendation** | (1) The `deepseek_chat()` function at `lib.rs:539` and `agent/llm.rs:LlmClient` are two separate implementations of the same DeepSeek API client. This violates DRY — the agent's `LlmClient` should be the single implementation, with `deepseek_chat()` as a convenience wrapper or removed entirely. (2) `Box::leak` means the `Database` is never dropped — acceptable for a server process, but prevents graceful shutdown. Consider an `Arc<Database>` if clean shutdown becomes a requirement. (3) The 14 frontend JS modules use globals (`window._currentProjectId`) for state (`state.js:1-13`). This works but limits composability if the frontend grows. |

---

## Architecture Debt Summary

| Priority | Item | Location | Risk |
|----------|------|----------|------|
| **High** | Hardcoded column indices in row deserializers | `db.rs:90-179` | Silent data corruption on SELECT reordering |
| **High** | Dual DeepSeek client implementations | `lib.rs:539` vs `agent/llm.rs` | Bug divergence, duplicated error handling |
| **Medium** | Analysis types misplaced in `db.rs` | `db.rs:724-799` | Violates single-source-of-truth for types |
| **Medium** | `lib.rs` god-file (901 lines, 6 responsibilities) | `lib.rs` | Hard to navigate, merge conflicts |
| **Medium** | `ToolContext` duplicates `AppState` shape | `agent/tools.rs:8-12` | Coupling drift |
| **Low** | `Box::leak` prevents graceful shutdown | `lib.rs:791` | Acceptable for current use case |
| **Low** | Frontend global state via `window.*` | `static/js/state.js` | Limits frontend composability |

---

## Verdict

**B+ overall.** Clean layered architecture with good dependency hygiene. The main risks are the hardcoded column indices (high, silent-breakage risk) and the lib.rs sprawl (medium, developer-experience risk). Both are fixable incrementally. The agent subsystem is the best-structured part of the codebase — use it as the template for future modules.
