huskies/docs/architecture/service-modules.md

# Service Module Conventions

This document defines the layout, layering rules, and patterns for all service
modules under `server/src/service/`. Every extraction from the HTTP handlers to
a service module **must** follow these conventions.

---

## 1. Directory Layout

```
server/src/service/<domain>/
    mod.rs      — public API, typed Error, orchestration, integration tests
    io.rs       — every side-effectful call; the ONLY file that may touch the
                  filesystem, spawn processes, or call external crates that do
    <topic>.rs  — pure logic for a named concern within the domain; no I/O
```

### Rules

- `<domain>` matches the HTTP handler filename (e.g. `agents`, `settings`,
  `oauth`).
- **No file named `logic.rs`** — use a descriptive domain name instead
  (e.g. `selection.rs`, `token.rs`, `validation.rs`).
- New topic files are added when a pure concern grows beyond ~50 lines or when
  it has independent test coverage needs.

---

## 2. The Functional-Core / Imperative-Shell Rule

```
io.rs (imperative shell)  ←→  mod.rs (orchestrator)  ←→  <topic>.rs (functional core)
```

| Layer | Allowed | Forbidden |
|-------|---------|-----------|
| `<topic>.rs` | Pure Rust, data-transformation, branching logic, pattern matching | Any I/O |
| `io.rs` | `std::fs`, `std::process`, `tokio::fs`, network calls, `SystemTime::now` | Business logic beyond a thin wrapper |
| `mod.rs` | Calls into `io.rs` and `<topic>.rs`; owns the `Error` type | Direct I/O without going through `io.rs` |

**Grep-enforceable check:** The following must NOT appear in any `service/<domain>/` file other than `io.rs`:

- `std::fs`
- `std::process`
- `std::thread::sleep`
- `tokio::fs`
- `reqwest`
- `SystemTime::now`

---

## 3. Error Type Pattern

Each service domain declares its own typed error enum in `mod.rs`:

```rust
/// Errors returned by `service::agents` operations.
#[derive(Debug)]
pub enum Error {
    ProjectRootNotConfigured,
    AgentNotFound(String),
    WorkItemNotFound(String),
    WorktreeError(String),
    ConfigError(String),
    IoError(String),
}

impl std::fmt::Display for Error { ... }
```

HTTP handlers map service errors to **specific** HTTP status codes:

| Error variant | HTTP status |
|--------------|-------------|
| `ProjectRootNotConfigured` | 400 Bad Request |
| `AgentNotFound` | 404 Not Found |
| `WorkItemNotFound` | 404 Not Found |
| `WorktreeError` | 400 Bad Request |
| `ConfigError` | 400 Bad Request |
| `IoError` | 500 Internal Server Error |

**No generic `bad_request` for everything** — distinguish 400 vs 404 vs 500.

---

## 4. Test Pattern

### Pure topic files (`<topic>.rs`)

```rust
#[cfg(test)]
mod tests {
    use super::*;

    // Unit tests MUST:
    //   - Use no tempdir, tokio runtime, or filesystem
    //   - Cover every branch of every public function
    #[test]
    fn filter_removes_archived_agents() { ... }
}
```

### `io.rs`

```rust
#[cfg(test)]
mod tests {
    use super::*;
    use tempfile::TempDir;

    // IO tests MAY use tempdirs and real filesystem.
    // Keep them few and focused on the thin I/O wrapper contract.
    #[test]
    fn is_archived_returns_true_when_in_done() { ... }
}
```

### `mod.rs`

```rust
#[cfg(test)]
mod tests {
    use super::*;

    // Integration tests compose io + pure layers end-to-end.
    // May use tempdirs. Keep the count small — they are integration-level.
    #[tokio::test]
    async fn list_agents_excludes_archived() { ... }
}
```

---

## 5. Dependency Injection Pattern

Service functions take **only the dependencies they actually use**:

```rust
// Good — takes only what it needs
pub async fn start_agent(
    pool: &AgentPool,
    project_root: &Path,
    story_id: &str,
    agent_name: Option<&str>,
) -> Result<AgentInfo, Error> { ... }

// Bad — takes the whole AppContext
pub async fn start_agent(ctx: &AppContext, ...) -> Result<AgentInfo, Error> { ... }
```

Standard injected dependencies for `service::agents`:

| Type | Purpose |
|------|---------|
| `&AgentPool` | Agent lifecycle operations |
| `&Path` (`project_root`) | Filesystem operations scoped to the project |
| `&WorkflowState` | In-memory test result cache |

**The dependency set chosen for `agents` is the reference pattern for all future
service module extractions.**

---

## 6. HTTP Handler Contract

After extraction, HTTP handlers are thin adapters:

```rust
async fn start_agent(&self, payload: Json<StartAgentPayload>) -> OpenApiResult<...> {
    let project_root = self.ctx.agents.get_project_root(&self.ctx.state)
        .map_err(|e| bad_request(e))?;                  // extract from AppContext
    let info = service::agents::start_agent(             // call service
        &self.ctx.agents, &project_root, &payload.story_id, payload.agent_name.as_deref(),
    ).await.map_err(map_service_error)?;                 // map typed error → HTTP
    Ok(Json(AgentInfoResponse { ... }))                  // shape DTO
}
```

Handlers must contain **no**:
- `std::fs` / file reads
- `std::process` invocations
- Inline load-mutate-save sequences
- Inline validation that belongs in the service layer

---

## 7. Follow-up Extractions

See [future-extractions.md](future-extractions.md) for the recommended order
and rationale for remaining extraction targets.