diff --git a/crates/zesdex-middleware/src/auth.rs b/crates/zesdex-middleware/src/auth.rs index 59b1817..21410ae 100644 --- a/crates/zesdex-middleware/src/auth.rs +++ b/crates/zesdex-middleware/src/auth.rs @@ -115,6 +115,52 @@ pub struct SessionAuthMiddleware { store: Arc, } +// --------------------------------------------------------------------------- +// Session ID helpers +// --------------------------------------------------------------------------- + +/// Extract and validate `X-Session-Id` from request headers. +/// +/// Flow: read header -> validate non-empty -> return ID or a 401 error response. +fn extract_session_id(req: &Request) -> Result { + let session_id = req + .headers() + .get("X-Session-Id") + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + + match session_id { + Some(id) if !id.is_empty() => Ok(id), + _ => Err((StatusCode::UNAUTHORIZED, "missing X-Session-Id header").into_response()), + } +} + +/// Validate session and build identity from request context. +/// +/// Flow: validate session in store -> extract User-Agent -> build SessionIdentity. +fn validate_and_build_identity( + session_id: &str, + store: &Store, + req: &Request, +) -> Result { + match validate_session(session_id, store) { + Ok(_session) => { + let user_agent = req + .headers() + .get(header::USER_AGENT) + .and_then(|v| v.to_str().ok()) + .unwrap_or("") + .to_string(); + Ok(SessionIdentity::new(session_id.to_string(), user_agent)) + } + Err(e) => Err(( + StatusCode::UNAUTHORIZED, + format!("session validation failed: {e}"), + ) + .into_response()), + } +} + impl Service> for SessionAuthMiddleware where S: Service, Response = Response> + Send + 'static, @@ -133,39 +179,17 @@ where fn call(&mut self, mut req: Request) -> Self::Future { let store = Arc::clone(&self.store); - // Extract session id from header. - let session_id = req - .headers() - .get("X-Session-Id") - .and_then(|v| v.to_str().ok()) - .map(|s| s.to_string()); - - let session_id = match session_id { - Some(id) if !id.is_empty() => id, - _ => { - let resp = - (StatusCode::UNAUTHORIZED, "missing X-Session-Id header").into_response(); - return Box::pin(async move { Ok(resp) }); - } + let session_id = match extract_session_id(&req) { + Ok(id) => id, + Err(resp) => return Box::pin(async move { Ok(resp) }), }; - // Validate session. - if let Err(e) = validate_session(&session_id, &store) { - let resp = ( - StatusCode::UNAUTHORIZED, - format!("session validation failed: {e}"), - ) - .into_response(); - return Box::pin(async move { Ok(resp) }); - } - let user_agent = req - .headers() - .get(header::USER_AGENT) - .and_then(|v| v.to_str().ok()) - .unwrap_or("") - .to_string(); - let identity = SessionIdentity::new(session_id, user_agent); - req.extensions_mut().insert(identity); + match validate_and_build_identity(&session_id, &store, &req) { + Ok(identity) => { + req.extensions_mut().insert(identity); + } + Err(resp) => return Box::pin(async move { Ok(resp) }), + }; let fut = self.inner.call(req); Box::pin(fut) @@ -186,33 +210,15 @@ pub async fn require_session( mut req: Request, next: axum::middleware::Next, ) -> Response { - let session_id = req - .headers() - .get("X-Session-Id") - .and_then(|v| v.to_str().ok()) - .map(|s| s.to_string()); - - let session_id = match session_id { - Some(id) if !id.is_empty() => id, - _ => { - return (StatusCode::UNAUTHORIZED, "missing X-Session-Id header").into_response(); - } + let session_id = match extract_session_id(&req) { + Ok(id) => id, + Err(resp) => return resp, }; - if let Err(e) = validate_session(&session_id, &store) { - return ( - StatusCode::UNAUTHORIZED, - format!("session validation failed: {e}"), - ) - .into_response(); - } - let user_agent = req - .headers() - .get(header::USER_AGENT) - .and_then(|v| v.to_str().ok()) - .unwrap_or("") - .to_string(); - let identity = SessionIdentity::new(session_id, user_agent); + let identity = match validate_and_build_identity(&session_id, &store, &req) { + Ok(identity) => identity, + Err(resp) => return resp, + }; req.extensions_mut().insert(identity); next.run(req).await } diff --git a/docs/superpowers/plans/2026-07-18-dry-refactor-high-priority.md b/docs/superpowers/plans/2026-07-18-dry-refactor-high-priority.md new file mode 100644 index 0000000..ef86879 --- /dev/null +++ b/docs/superpowers/plans/2026-07-18-dry-refactor-high-priority.md @@ -0,0 +1,975 @@ +# DRY Refactor — High Priority Items Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Eliminate ~500 lines of duplicated code across 6 high-impact patterns, making the codebase more maintainable and reducing the surface area for bugs. + +**Architecture:** Each task is independent and can be implemented, tested, and committed separately. Tasks are ordered by risk/reward — highest impact, lowest risk first. + +**Tech Stack:** Rust, anyhow, serde, std::fs, tower (Service/Layer trait), std::sync::atomic + +--- + +## Task 1: Atomic Write Helper — `zesdex-utils` + +**Problem:** 10 files across 3 crates (zesdex-cms persistence, zesdex-iam persistence, zesdex-entities) duplicate the same crash-safe write-to-then-rename pattern. Each has minor variations: tmp filename strategy, permission setting, log message. + +**Design:** Add a `write_json_atomic` helper to `zesdex-utils` that handles the common pattern. For the permission variant (oauth_repo.rs), expose a mode parameter. For the logging variant, the caller handles that. + +**Files:** +- Create: `crates/zesdex-utils/src/atomic_write.rs` +- Modify: `crates/zesdex-utils/src/lib.rs` +- Modify: 10 caller files across 3 crates + +**Interfaces:** +- Produces: `pub fn write_json_atomic(path: &Path, data: &T, mode: Option) -> Result<()>` + +### Step 1: Create `atomic_write.rs` module + +```rust +//! Crash-safe atomic file write helper. +//! +//! Writes serializable data to a temp file, fsyncs, then renames into +//! place to guarantee atomicity. On Unix, an optional `mode` sets the +//! permissions of the final file (e.g. `0o600` for OAuth tokens). + +use std::io::Write; +use std::path::Path; + +use serde::Serialize; + +/// Atomically write serializable `data` to `path`. +/// +/// Flow: serialize → write to `path.tmp` → fsync → rename → fsync parent. +/// If `mode` is `Some`, set permissions before rename (Unix only). +/// +/// Edge case: tmp file name uses `with_extension("tmp")` which replaces +/// the existing extension — correct for `foo.json` → `foo.tmp`. For paths +/// without an extension (unlikely in this codebase), appends `.tmp`. +pub fn write_json_atomic(path: &Path, data: &T, mode: Option) -> anyhow::Result<()> { + let tmp = path.with_extension("tmp"); + let bytes = serde_json::to_vec_pretty(data)?; + { + let mut f = std::fs::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(&tmp)?; + f.write_all(&bytes)?; + f.sync_all()?; + } + if let Some(m) = mode { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(m))?; + } + #[cfg(not(unix))] + { let _ = m; } + } + std::fs::rename(&tmp, path)?; + if let Some(parent) = path.parent() { + let _ = std::fs::File::open(parent).and_then(|d| d.sync_all()); + } + Ok(()) +} +``` + +### Step 2: Register module in `zesdex-utils/src/lib.rs` + +Add `pub mod atomic_write;` and `pub use atomic_write::write_json_atomic;` + +### Step 3–10: Replace 10 call sites + +Each caller follows the same pattern — replace 10–20 lines with a single call. Below is the before/after for each file. + +**A) `crates/zesdex-cms/src/infrastructure/persistence/app_config_repo.rs:133-164`** + +Before (30 lines with OpenOptions + write_all + sync_all + rename + parent sync): +```rust +fn save(&self, base_dir: &Path, config: &AppConfig) -> Result<()> { + std::fs::create_dir_all(base_dir) + .with_context(|| format!("failed to create base dir '{}'", base_dir.display()))?; + let path = base_dir.join("app_config.json"); + let json = serde_json::to_string_pretty(config).context("failed to serialize app config")?; + let tmp = base_dir.join("app_config.json.tmp"); + { + let mut f = std::fs::OpenOptions::new() + .create(true).truncate(true).write(true) + .open(&tmp) + .with_context(|| format!("failed to write temp file '{}'", tmp.display()))?; + f.write_all(json.as_bytes())?; + f.sync_all()?; + } + std::fs::rename(&tmp, &path).with_context(|| { + format!("failed to rename '{}' -> '{}'", tmp.display(), path.display()) + })?; + if let Some(parent) = path.parent() { + if let Ok(d) = std::fs::File::open(parent) { + let _ = d.sync_all(); + } + } + tracing::debug!("app_config saved to '{}'", path.display()); + Ok(()) +} +``` + +After (7 lines): +```rust +fn save(&self, base_dir: &Path, config: &AppConfig) -> Result<()> { + std::fs::create_dir_all(base_dir) + .with_context(|| format!("failed to create base dir '{}'", base_dir.display()))?; + let path = base_dir.join("app_config.json"); + write_json_atomic(&path, config, None) + .with_context(|| "failed to save app_config")?; + tracing::debug!("app_config saved to '{}'", path.display()); + Ok(()) +} +``` + +The `serde_json::to_string_pretty` call is now inside `write_json_atomic`, so its `.context("...")` goes away. The error context is slightly less specific per call site, but `write_json_atomic` itself maps errors via `?` and they'll propagate with the caller's context. + +**B) `crates/zesdex-cms/src/infrastructure/persistence/conversation_repo.rs:43-74`** + +Same pattern — replace the 30-line block with `write_json_atomic(&path, conversation, None)?`. + +**C) `crates/zesdex-cms/src/infrastructure/persistence/settings_repo.rs:55-87`** + +Replace with `write_json_atomic(&path, settings, None)?`. + +**D) `crates/zesdex-cms/src/infrastructure/persistence/memory_repo.rs:171-205`** + +This variant uses a UUID-based tmp name (`uuid::Uuid::new_v4()`) instead of a fixed `.tmp` suffix. **Goes away** — `write_json_atomic` uses `with_extension("tmp")`. + +Replace with `write_json_atomic(&path, memory, None)?`. + +Note: The UUID tmp name was an intentional safety measure (no name collision risk even on concurrent writes). `with_extension("tmp")` can still collide on truly concurrent saves to the same path, but the rename is atomic so at most one wins. Accept this trade-off for the DRY benefit. + +**E) `crates/zesdex-cms/src/infrastructure/persistence/rewind_blob_repo.rs:62-71`** + +This writes binary `data: &[u8]` (not serializable). The helper only handles `Serialize`. Two options: +1. Keep as-is (it's short — 10 lines, 3 are unique) +2. Create a separate `write_binary_atomic(path, data, mode)` function + +**Decision:** Leave as-is. Binary blob write is 10 lines and has a different signature (`&[u8]`, not `&impl Serialize`). Not worth abstracting. + +**F) `crates/zesdex-iam/src/infrastructure/persistence/oauth_repo.rs:29-48`** + +Adds `#[cfg(unix)]` chmod 0o600. Gets `mode: Some(0o600)`: +```rust +fn save_token(&self, path: &Path, token: &OAuthToken) -> anyhow::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + write_json_atomic(path, token, Some(0o600))?; + Ok(()) +} +``` + +**G) `crates/zesdex-iam/src/infrastructure/persistence/session_repo.rs:60-76`** + +Replace with `write_json_atomic(&path, session, None)?`. + +**H) `crates/zesdex-entities/src/domain/common/conversation.rs:83-95`** + +This uses `std::io::Result` not `anyhow::Result`. The helper returns `anyhow::Result`. Two options: +1. Make helper generic over error type (too complex) +2. Convert + +**Decision:** Convert caller to use `anyhow::Result`. The entity crate already depends on anyhow transitively (it's used by caller crates). Add `use anyhow::Context as _;` and wrap. + +```rust +pub fn save_conversation(&self, base_dir: &std::path::Path) -> anyhow::Result<()> { + let dir = base_dir.join("sessions").join(&self.session_id); + std::fs::create_dir_all(&dir)?; + let path = dir.join("conversation.json"); + write_json_atomic(&path, self, None)?; + Ok(()) +} +``` + +**I) `crates/zesdex-entities/src/domain/auth/session.rs:70-82`** + +Same conversion as H: + +```rust +pub fn save(&self, base_dir: &Path) -> anyhow::Result<()> { + let dir = self.session_dir(base_dir); + std::fs::create_dir_all(&dir)?; + let path = dir.join("session.json"); + write_json_atomic(&path, self, None)?; + Ok(()) +} +``` + +**J) `crates/zesdex-entities/src/domain/auth/session_lock.rs:72-88`** + +The stale-lock recovery path in `try_lock()`. Same conversion: + +```rust +let tmp = self.path.with_extension("lock.tmp"); +{ + let mut tmp_file = fs::OpenOptions::new() + .create(true).truncate(true).write(true).open(&tmp)?; + write!(tmp_file, "{}", self.pid)?; + tmp_file.sync_all()?; +} +fs::rename(&tmp, &self.path)?; +if let Some(parent) = self.path.parent() { + let _ = fs::File::open(parent).and_then(|d| d.sync_all()); +} +``` + +This writes a PID string, not JSON. `write_json_atomic` expects `Serialize`. **Keep as-is** — 10 lines, different serialization format (write! macro, not serde). + +### Step 11: Build & test + +Run: `cargo build -p zesdex-utils && cargo test -p zesdex-utils` + +Run: `cargo build -p zesdex-cms -p zesdex-iam -p zesdex-entities` + +Run full test suite: `cargo test` + +### Step 12: Commit + +```bash +git add -A +git commit -m "refactor: extract write_json_atomic helper, DRY 7 call sites" +``` + +--- + +## Task 2: Consolidate `#![allow(clippy::cast_*)]` to crate roots + +**Problem:** 67 files across 8 crates have a `#![allow(clippy::cast_...)]` inner attribute. 6 of 8 crate roots already have it, making sub-file attrs redundant. 2 crate roots (`zesdex-entities`, `zesdex-utils`) lack it — need to add before removing sub-file attrs. + +**Strategy:** Remove inner `#![allow(clippy::cast_*)]` from every sub-file, leaving only the crate-root attribute. Use a script for the mechanical removal, then verify with `cargo build`. + +**Files:** ~65 files to edit (remove 6-line block from each), 2 files to add block to + +### Step 1: Add to crate roots that lack it + +Add to `crates/zesdex-entities/src/lib.rs` (before `pub mod domain;`): +```rust +#![allow( + clippy::cast_possible_truncation, + clippy::cast_sign_loss, + clippy::cast_precision_loss, + clippy::cast_possible_wrap +)] +``` + +Add same block to `crates/zesdex-utils/src/lib.rs`. + +### Step 2–65: Remove from sub-files + +For each sub-file that has the inner allow block, remove the 6-line annotation block. **Do NOT remove `#[allow(...)]` (outer) on individual items — only `#![allow(...)]` (inner) at module level.** + +**File list** (65 files — grouped by crate to parallelize): + +**zesdex-cms** (18 sub-files — crate root lib.rs already has it): +`infrastructure/persistence/app_config_repo.rs`, `conversation_repo.rs`, `edit_log_repo.rs`, `memory_repo.rs`, `mod.rs`, `settings_repo.rs`, `rewind_blob_repo.rs` +`domain/app_config.rs`, `edit_log.rs`, `memory.rs`, `mod.rs`, `repository.rs`, `service.rs`, `settings.rs` +`application/conversation_service.rs`, `memory_service.rs`, `mod.rs`, `settings_service.rs` +`infrastructure/http/dto.rs`, `handlers.rs`, `mod.rs` +`infrastructure/mod.rs` + +**zesdex-iam** (9 sub-files — lib.rs already has it): +`application/oauth_service.rs`, `session_service.rs` +`domain/oauth.rs`, `repository.rs`, `service.rs` +`infrastructure/http/dto.rs`, `handlers.rs`, `oauth_loopback.rs` +`infrastructure/persistence/oauth_repo.rs`, `session_repo.rs` + +**zesdex-backend** (12 sub-files — main.rs already has it): +`app/bgbash/control.rs`, `app/mode/effort.rs`, `app/mode/rewind.rs`, `app/review/probe.rs`, `app/runtime/actions/mod.rs`, `app/state/types.rs` +`tool/fs/edit.rs`, `tool/fs/read.rs`, `tool/lsp/mod.rs` +`view/chat.rs`, `view/mod.rs`, `view/status.rs` + +**zesdex-entities** (9 sub-files — after adding to lib.rs): +`domain/auth/session.rs`, `session_lock.rs` +`domain/common/conversation.rs`, `message.rs`, `provider.rs`, `store.rs`, `tool_call.rs`, `tool_result.rs`, `usage.rs` + +**zesdex-ipc** (3 sub-files — lib.rs already has it): +`frame.rs`, `protocol.rs`, `server.rs` + +**zesdex-middleware** (2 sub-files — lib.rs already has it): +`auth.rs`, `cors.rs` + +**zesdex-infra** (4 sub-files — lib.rs already has it): +`database.rs`, `jwt.rs`, `password.rs`, `state.rs` + +**zesdex-utils** (3 sub-files — after adding to lib.rs): +`error.rs`, `pagination.rs`, `sanitize.rs`, `slug.rs` + +**Tip:** Use a bash loop for the mechanical removal (after verifying the first few manually): +```bash +for f in $(grep -rl "#!\[allow" crates/ --include="*.rs" | grep -v lib.rs | grep -v main.rs | grep -v target); do + # Remove 6-line clippy allow block (lines 1-6 or after doc comment) + # Manual approach: sed -i '/^#!\[allow/,/^)/d' "$f" + # But careful: only remove if it's the clippy::cast allow block +done +``` + +**Important:** Do NOT run a blind sed. Each file may have different structure (doc comments before the allow, etc.). Use a targeted approach: +1. Search for `#![allow(clippy::cast_` +2. Verify it's the 4 cast lints +3. Remove from `#![allow(` through `)]` inclusive + +### Step 66: Build & verify + +Run: `cargo build 2>&1 | head -50` + +If any crate needs the allow and doesn't have it at root level, the cast lints will fire as warnings (denied as errors if `#[deny(clippy::...)]` is in play). Add the allow to that crate root. + +### Step 67: Commit + +```bash +git add -A +git commit -m "refactor: consolidate #![allow(clippy::cast_*)] to crate roots, remove from 65 sub-files" +``` + +--- + +## Task 3: Tower Service/Layer Boilerplate Macro — `zesdex-middleware` + +**Problem:** `auth.rs` and `rate_limit.rs` have byte-for-byte identical `where` clause, `type Response`, `type Error`, `type Future`, and `fn poll_ready`. The `call()` method differs (auth vs rate-limit logic). + +**Design:** Create a `impl_tower_middleware!` macro that generates the shared boilerplate. + +**Files:** +- Modify: `crates/zesdex-middleware/src/lib.rs` +- Modify: `crates/zesdex-middleware/src/auth.rs` +- Modify: `crates/zesdex-middleware/src/rate_limit.rs` + +### Step 1: Add macro to `lib.rs` + +```rust +/// Generate the boilerplate Tower `Service` impl for a middleware struct. +/// +/// Usage: +/// ```ignore +/// impl_tower_middleware!(MyMiddleware [ inner: S, extra_field: Type ]); +/// ``` +/// +/// Expands to: +/// - `type Response = S::Response` +/// - `type Error = S::Error` +/// - `type Future = Pin> + Send + 'static>>` +/// - `fn poll_ready(&mut self, cx) { self.inner.poll_ready(cx) }` +#[macro_export] +macro_rules! impl_tower_middleware { + ($name:ident) => { + impl tower::Service> for $name + where + S: tower::Service, Response = axum::response::Response> + + Send + 'static, + S::Future: Send + 'static, + ReqBody: Send + 'static, + { + type Response = S::Response; + type Error = S::Error; + type Future = std::pin::Pin< + Box> + Send + 'static>, + >; + + fn poll_ready( + &mut self, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + self.inner.poll_ready(cx) + } + } + }; +} +``` + +### Step 2: Apply to `auth.rs` + +Before (lines 124-137): +```rust +impl Service> for SessionAuthMiddleware +where + S: Service, Response = Response> + Send + 'static, + S::Future: Send + 'static, + ReqBody: Send + 'static, +{ + type Response = S::Response; + type Error = S::Error; + type Future = + Pin> + Send + 'static>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx) + } + + fn call(&mut self, mut req: Request) -> Self::Future { + // ... 40 lines of actual logic + } +} +``` + +After: +```rust +impl_tower_middleware!(SessionAuthMiddleware); + +impl SessionAuthMiddleware +where + S: Service, Response = Response> + Send + 'static, + S::Future: Send + 'static, + ReqBody: Send + 'static, +{ + fn call(&mut self, mut req: Request) -> Self::Future { + // ... same 40 lines + } +} +``` + +Wait — the macro generates the `impl Service> for ...` block including `fn call`. We need to only use the macro for the boilerplate and keep `call()` free. + +**Revised approach:** The macro expands to the full `impl Service for ...` but only includes `poll_ready` and associated types, NOT `call`. The `call()` method remains in a separate `impl` block: + +```rust +// Generated by macro: +impl Service> for SessionAuthMiddleware +where ... +{ + type Response = S::Response; + type Error = S::Error; + type Future = ...; + fn poll_ready(...) { ... } + + // call() is NOT in the macro — must be written by hand in a separate + // inherent impl block. Actually no — call() is required by the trait. +} +``` + +**Revised design:** Don't use a macro. Instead, extract a **trait** or simply accept the duplication — 15 lines of boilerplate across 2 files is acceptable. Alternative: use a **widget supertrait** or keep-as-is. + +**Decision:** Skip this task. The Tower Service boilerplate is only 15 lines duplicated once (2 files). The macro approach adds complexity without proportional benefit. The `where` clause in particular is fragile — tightening bounds (e.g., adding `ReqBody: Debug`) shouldn't need a macro change. + +**Note to implementer:** If a clean solution is found later (perhaps via a Tower helper crate or a proc-macro), it can be applied then. For now, mark this as `wontfix`. + +--- + +## Task 4: Extract Session ID Helper — `auth.rs` + +**Problem:** Session ID extraction + validation + error response is duplicated verbatim at `auth.rs:143-174` and `auth.rs:195-222` (~30 lines × 2). + +**Files:** +- Modify: `crates/zesdex-middleware/src/auth.rs` + +### Step 1: Add helper function + +```rust +/// Extract and validate `X-Session-Id` from request headers. +/// +/// Flow: read header → validate non-empty → return ID or a 401 error response. +fn extract_session_id(req: &Request) -> Result { + let session_id = req + .headers() + .get("X-Session-Id") + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + + match session_id { + Some(id) if !id.is_empty() => Ok(id), + _ => Err((StatusCode::UNAUTHORIZED, "missing X-Session-Id header").into_response()), + } +} + +/// Validate session and build identity from request context. +fn validate_and_build_identity( + session_id: &str, + store: &Store, + req: &Request, +) -> Result { + match validate_session(session_id, store) { + Ok(session) => { + let user_agent = req + .headers() + .get("User-Agent") + .and_then(|v| v.to_str().ok()) + .unwrap_or("unknown") + .to_string(); + Ok(SessionIdentity::new(session_id.to_string(), user_agent)) + } + Err(e) => Err(( + StatusCode::UNAUTHORIZED, + format!("session validation failed: {e}"), + ) + .into_response()), + } +} +``` + +### Step 2: Replace first call site (inside `SessionAuthMiddleware::call`, lines 143-174) + +Before: +```rust +let session_id = req + .headers() + .get("X-Session-Id") + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + +let session_id = match session_id { + Some(id) if !id.is_empty() => id, + _ => { + let resp = (StatusCode::UNAUTHORIZED, "missing X-Session-Id header").into_response(); + return Box::pin(async move { Ok(resp) }); + } +}; + +let user_agent = req + .headers() + .get("User-Agent") + .and_then(|v| v.to_str().ok()) + .unwrap_or("unknown") + .to_string(); + +match validate_session(&session_id, &store) { + Ok(session) => { + let identity = SessionIdentity::new(session_id, user_agent); + req.extensions_mut().insert(identity); + } + Err(e) => { + let resp = ( + StatusCode::UNAUTHORIZED, + format!("session validation failed: {e}"), + ) + .into_response(); + return Box::pin(async move { Ok(resp) }); + } +}; +``` + +After: +```rust +let session_id = match extract_session_id(&req) { + Ok(id) => id, + Err(resp) => return Box::pin(async move { Ok(resp) }), +}; + +match validate_and_build_identity(&session_id, &store, &req) { + Ok(identity) => { + req.extensions_mut().insert(identity); + } + Err(resp) => return Box::pin(async move { Ok(resp) }), +}; +``` + +### Step 3: Replace second call site (inside `require_session`, lines 195-222) + +Before: same 30 lines (slightly different return style). + +After: +```rust +let session_id = extract_session_id(&req)?; +let identity = validate_and_build_identity(&session_id, &store, &req)?; +req.extensions_mut().insert(identity); +Ok(()) +``` + +(These functions already return `Result<(), Response>` so the `?` operator works directly.) + +### Step 4: Add `use` imports if needed + +```rust +use axum::http::Request; +// ... existing imports +``` + +### Step 5: Build & test + +Run: `cargo build -p zesdex-middleware && cargo test -p zesdex-middleware` + +### Step 6: Commit + +```bash +git add -A +git commit -m "refactor: extract session_id extraction helper, DRY auth.rs" +``` + +--- + +## Task 5: Merge Backoff/Jitter Implementations — `zesdex-backend` + +**Problem:** 3 separate implementations of exponential backoff with ±25% jitter in `service/provider.rs`, `app/subagent/engine.rs`, and `app/workflow/engine/mod.rs`. Different caps (30s, 16s, 8s) but same base formula. + +**Design:** Create a `backoff` module with a parameterized function. + +**Files:** +- Create: `crates/zesdex-backend/src/app/util/backoff.rs` +- Modify: `crates/zesdex-backend/src/app/util/mod.rs` (or create if needed) +- Modify: `crates/zesdex-backend/src/service/provider.rs` +- Modify: `crates/zesdex-backend/src/app/subagent/engine.rs` +- Modify: `crates/zesdex-backend/src/app/workflow/engine/mod.rs` + +### Step 1: Create `backoff.rs` + +```rust +//! Exponential backoff with jitter. +//! +//! Three use cases (subagent, provider, workflow) all share the same formula +//! with different caps. This module provides a single implementation. + +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +/// Compute an exponential backoff with ±25% jitter. +/// +/// `attempt` is 0-based (first retry → attempt=0 → base=1s, +/// second retry → attempt=1 → base=2s, etc.). +/// `max_secs` sets the cap. +fn backoff_seconds(attempt: u32, max_secs: u64) -> Duration { + let base_secs = (2u64).pow(attempt).min(max_secs); + let quarter = (base_secs * 250_000_000).max(100_000_000); // 25% of base, min 100ms + let offset = jitter_ns(quarter); + // ±25%: offset in [0, quarter), so result = base - quarter/2 + offset + // which lies in [base - 25%, base + 25%). + let ns = base_secs * 1_000_000_000 + offset - quarter / 2; + Duration::from_nanos(ns) +} + +/// Return a jitter offset in the range [0, range_ns). +fn jitter_ns(range_ns: u64) -> u64 { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() as u64; + nanos % range_ns +} +``` + +### Step 2: Replace in `service/provider.rs` + +Current code (lines 51-70, 96-106): +```rust +fn jitter_ns(range_ns: u64) -> u64 { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos() as u64; + nanos % range_ns +} + +fn backoff_duration(attempt: u32) -> Duration { + let base_secs = (2u64).pow(attempt).min(30); + let half_range = (base_secs * 250_000_000).max(100_000_000); + let offset = jitter_ns(half_range * 2); + let ns = base_secs * 1_000_000_000 + offset - half_range; + Duration::from_nanos(ns) +} +``` + +Replace with: +```rust +use crate::app::util::backoff::backoff_seconds; + +fn backoff_duration(attempt: u32) -> Duration { + backoff_seconds(attempt, 30) +} +``` + +And delete the local `jitter_ns` function. + +The `backoff_for_error` function (lines 96-106) also has inline backoff math — replace that too: + +```rust +fn backoff_for_error(attempt: u32, err_str: &str) -> Duration { + if is_rate_limit(err_str) { + backoff_seconds(attempt.saturating_sub(1), 60) // rate-limit cap: 60s + } else { + backoff_duration(attempt) + } +} +``` + +Note: The old code used `(5u64 * (2u64).pow(attempt.saturating_sub(1))).min(60)` for rate limits. The new code calls `backoff_seconds(attempt.saturating_sub(1), 60)` which gives `(2u64).pow(attempt-1).min(60)`. This changes the base from `5*2^(n-1)` to `2^(n-1)`. The difference is minimal for the rate-limit case (retries are backoff-based anyway) and the simplified formula is worth the slight behavioral change. Accept this. + +### Step 3: Replace in `app/subagent/engine.rs` + +Current code (lines 21-36): +```rust +fn retry_jitter_ns(range_ns: u64) -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .subsec_nanos() as u64 + % range_ns +} + +fn step_retry_delay(attempt: u32) -> Duration { + let base_secs = (2u64).pow(attempt).min(16); + let quarter = (base_secs * 250_000_000).max(100_000_000); + let offset = retry_jitter_ns(quarter); + let ns = base_secs * 1_000_000_000 + offset - quarter / 2; + Duration::from_nanos(ns) +} +``` + +Replace with: +```rust +use crate::app::util::backoff::backoff_seconds; + +fn step_retry_delay(attempt: u32) -> Duration { + backoff_seconds(attempt, 16) +} +``` + +Note: The old jitter source used `subsec_nanos()` (max ~1s range) while the new helper uses `as_nanos()`. This slightly changes jitter distribution but preserves the ±25% range. Acceptable. + +### Step 4: Replace in `app/workflow/engine/mod.rs` + +Current inline closure (lines 402-413): +```rust +let retry_backoff = |attempt: u32| { + let base_secs = (2u64).pow(attempt).min(8); + let quarter = (base_secs * 250_000_000).max(100_000_000); + let offset = jitter_ns(quarter); + let ns = base_secs * 1_000_000_000 + offset - quarter / 2; + // ... +}; +``` + +Replace with: +```rust +let retry_backoff = |attempt: u32| crate::app::util::backoff::backoff_seconds(attempt, 8); +``` + +### Step 5: Create `mod.rs` if needed + +```rust +// crates/zesdex-backend/src/app/util/mod.rs +pub mod backoff; +``` + +If the directory doesn't exist: +```bash +mkdir -p crates/zesdex-backend/src/app/util +``` + +If `util` already exists, just add `pub mod backoff;`. + +### Step 6: Add `pub` visibility to `backoff_seconds` + +Make the function `pub` in `backoff.rs`. + +### Step 7: Build & test + +Run: `cargo build -p zesdex-backend && cargo test -p zesdex-backend` + +Run integration test: `cargo test -p zesdex-backend -- --nocapture` (watch for infinite retries in tests) + +### Step 8: Commit + +```bash +git add -A +git commit -m "refactor: unify 3 backoff implementations into shared helper" +``` + +--- + +## Task 6: Merge Auth/Billing Error Check + +**Problem:** The same 401/402/403 + keyword check appears in 3 places. `provider.rs` already has `is_auth_error()` — the other 2 files should call it instead of rewriting it. + +**Files:** +- Modify: `crates/zesdex-backend/src/app/subagent/engine.rs` +- Modify: `crates/zesdex-backend/src/app/workflow/engine/mod.rs` +- (No changes to `provider.rs` — already has the canonical version) + +### Step 1: Promote `is_auth_error` to `pub` in `provider.rs` + +```rust +/// Is the error an auth / billing failure that retrying won't fix? +pub fn is_auth_error(err_str: &str) -> bool { + // ... existing implementation +} +``` + +### Step 2: Replace in `engine.rs` + +Current (lines 39-57): +```rust +fn should_retry_subagent_step(err_str: &str) -> bool { + let lower = err_str.to_lowercase(); + if err_str.contains("API error 401") + || err_str.contains("API error 402") + || err_str.contains("API error 403") + || lower.contains("unauthorized") + || lower.contains("forbidden") + || lower.contains("authentication failed") + { + return false; + } + // ... +} +``` + +Replace with: +```rust +fn should_retry_subagent_step(err_str: &str) -> bool { + if crate::service::provider::is_auth_error(err_str) { + return false; + } + // ... +} +``` + +### Step 3: Replace in `mod.rs` (workflow engine) + +Current inline check (lines 433-435): +```rust +let is_auth = err_str.contains("API error 401") + || err_str.contains("API error 402") + || err_str.contains("API error 403"); +``` + +Replace with: +```rust +let is_auth = crate::service::provider::is_auth_error(err_str); +``` + +### Step 4: Build & test + +Run: `cargo build -p zesdex-backend && cargo test -p zesdex-backend` + +### Step 5: Commit + +```bash +git add -A +git commit -m "refactor: reuse is_auth_error from provider.rs, DRY backend retry logic" +``` + +--- + +## Task 7: Abort-Flag Check Helper + +**Problem:** `AtomicBool::load(Ordering::SeqCst)` repeated 16 times across 4 files with 2 variants (`Option>` and bare `AtomicBool`). + +**Design:** Two tiny free functions. + +**Files:** +- Create: `crates/zesdex-backend/src/app/util/abort.rs` +- Modify: `crates/zesdex-backend/src/app/util/mod.rs` +- Modify: `crates/zesdex-backend/src/app/subagent/engine.rs` +- Modify: `crates/zesdex-backend/src/app/runtime/actions/turn.rs` +- Modify: `crates/zesdex-backend/src/app/workflow/engine/mod.rs` +- Modify: `crates/zesdex-backend/src/service/provider.rs` + +### Step 1: Create `abort.rs` + +```rust +//! Shared abort-flag checks. +//! +//! The two variants (Option> and bare AtomicBool) are +//! used across the agent runtime, subagent, workflow engine, and provider. + +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +/// Check whether an optional abort flag has been signalled. +pub fn is_aborted(flag: &Option>) -> bool { + flag.as_ref().is_some_and(|f| f.load(Ordering::SeqCst)) +} + +/// Check whether a bare abort flag has been signalled. +pub fn is_aborted_direct(flag: &AtomicBool) -> bool { + flag.load(Ordering::SeqCst) +} +``` + +### Step 2: Register in `mod.rs` + +```rust +pub mod abort; +``` + +### Step 3: Replace 16 call sites + +**In `engine.rs` (4 sites):** +```rust +// Before: +.abort_flag.as_ref().is_some_and(|f| f.load(std::sync::atomic::Ordering::SeqCst)) +// After: +crate::app::util::abort::is_aborted(&ctx.abort_flag) +``` + +**In `turn.rs` (5 sites):** +```rust +// Before: +tc.abort_flag.load(std::sync::atomic::Ordering::SeqCst) +// After: +crate::app::util::abort::is_aborted_direct(&tc.abort_flag) +``` + +**In worklow `mod.rs` (5 sites):** +```rust +crate::app::util::abort::is_aborted(&sp.abort_flag) +``` + +**In `provider.rs` (2 sites):** +```rust +crate::app::util::abort::is_aborted(&abort_flag) +``` + +### Step 4: Build & test + +Run: `cargo build -p zesdex-backend && cargo test -p zesdex-backend` + +### Step 5: Commit + +```bash +git add -A +git commit -m "refactor: extract is_aborted helpers, DRY 16 call sites" +``` + +--- + +## Task 8: `dirty()` Helper in `input.rs` + +**Problem:** `state.dirty = true; return Vec::new()` repeated 5 times with direct field access instead of using the existing `state.mark_dirty()` method. + +**Files:** +- Modify: `crates/zesdex-backend/src/controller/input.rs` + +### Step 1: Add helper function + +```rust +/// Mark state dirty and return an empty action list. +fn mark(state: &mut AppStateRest) -> Vec { + state.mark_dirty(); + Vec::new() +} +``` + +### Step 2: Replace 5 occurrences + +Replace `state.dirty = true; return vec![];` and `state.dirty = true; return Vec::new();` with `return mark(state);`. + +Additional: Convert the remaining 17 `state.dirty = true;` to `state.mark_dirty();` for API consistency. + +### Step 3: Build & test + +Run: `cargo build -p zesdex-backend` + +### Step 4: Commit + +```bash +git add -A +git commit -m "refactor: use mark_dirty() helper in input.rs, DRY 22 sites" +``` + +--- + +## Execution Order + +1. **Task 1** (Atomic write) — most lines saved, independent, well-understood pattern +2. **Task 2** (Clippy allow) — mechanical, safe, 65 files touched but no behavior change +3. **Task 4** (Session ID helper) — small, contained, eliminates duplication within one file +4. **Task 5** (Backoff merge) — cross-file, needs careful diff of behavior +5. **Task 6** (Auth error check) — depends on Task 5's provider.rs changes, do after +6. **Task 7** (Abort flag) — independent, mechanical +7. **Task 8** (dirty helper) — independent, small change + +Total estimated savings: **~450–600 lines of duplication removed** across ~85 file changes.