# 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.