Files
zesdex/docs/superpowers/plans/2026-07-18-dry-refactor-high-priority.md
T
asepharyanaandClaude Opus 4.8 e32501ee59 refactor: extract session_id extraction helper, DRY auth.rs
Extract duplicated session ID extraction + validation logic from
SessionAuthMiddleware::call() and require_session() into two shared
helper functions: extract_session_id and validate_and_build_identity.

Removes ~60 lines of duplicated code while preserving behavior:
- Both call sites now rely on the same extraction/validation path
- User-Agent default remains empty string (existing behavior unchanged)
- Error response format (401 with header/validation messages) unchanged

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-18 03:18:27 +07:00

32 KiB
Raw Blame History

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<T: Serialize>(path: &Path, data: &T, mode: Option<u32>) -> Result<()>

Step 1: Create atomic_write.rs module

//! 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<T: Serialize>(path: &Path, data: &T, mode: Option<u32>) -> 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 310: Replace 10 call sites

Each caller follows the same pattern — replace 1020 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):

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):

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 awaywrite_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):

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.

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:

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:

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

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;):

#![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 265: 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):

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

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

/// Generate the boilerplate Tower `Service` impl for a middleware struct.
///
/// Usage:
/// ```ignore
/// impl_tower_middleware!(MyMiddleware<S> [ inner: S, extra_field: Type ]);
/// ```
///
/// Expands to:
/// - `type Response = S::Response`
/// - `type Error = S::Error`
/// - `type Future = Pin<Box<dyn Future<Output = Result<...>> + Send + 'static>>`
/// - `fn poll_ready(&mut self, cx) { self.inner.poll_ready(cx) }`
#[macro_export]
macro_rules! impl_tower_middleware {
    ($name:ident<S $(, $extra:ident: $ty:ty)*>) => {
        impl<S, ReqBody> tower::Service<axum::http::Request<ReqBody>> for $name<S>
        where
            S: tower::Service<axum::http::Request<ReqBody>, 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<dyn std::future::Future<Output = Result<Self::Response, Self::Error>> + Send + 'static>,
            >;

            fn poll_ready(
                &mut self,
                cx: &mut std::task::Context<'_>,
            ) -> std::task::Poll<Result<(), Self::Error>> {
                self.inner.poll_ready(cx)
            }
        }
    };
}

Step 2: Apply to auth.rs

Before (lines 124-137):

impl<S, ReqBody> Service<Request<ReqBody>> for SessionAuthMiddleware<S>
where
    S: Service<Request<ReqBody>, Response = Response> + Send + 'static,
    S::Future: Send + 'static,
    ReqBody: Send + 'static,
{
    type Response = S::Response;
    type Error = S::Error;
    type Future =
        Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static>>;

    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
        self.inner.poll_ready(cx)
    }

    fn call(&mut self, mut req: Request<ReqBody>) -> Self::Future {
        // ... 40 lines of actual logic
    }
}

After:

impl_tower_middleware!(SessionAuthMiddleware<S>);

impl<S, ReqBody> SessionAuthMiddleware<S>
where
    S: Service<Request<ReqBody>, Response = Response> + Send + 'static,
    S::Future: Send + 'static,
    ReqBody: Send + 'static,
{
    fn call(&mut self, mut req: Request<ReqBody>) -> Self::Future {
        // ... same 40 lines
    }
}

Wait — the macro generates the impl<S, ReqBody> Service<Request<ReqBody>> 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:

// Generated by macro:
impl<S, ReqBody> Service<Request<ReqBody>> for SessionAuthMiddleware<S>
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

/// 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<ReqBody>) -> Result<String, Response> {
    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<ReqBody>,
) -> Result<SessionIdentity, Response> {
    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:

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:

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:

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

use axum::http::Request;
// ... existing imports

Step 5: Build & test

Run: cargo build -p zesdex-middleware && cargo test -p zesdex-middleware

Step 6: Commit

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

//! 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):

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:

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:

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):

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:

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):

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:

let retry_backoff = |attempt: u32| crate::app::util::backoff::backoff_seconds(attempt, 8);

Step 5: Create mod.rs if needed

// crates/zesdex-backend/src/app/util/mod.rs
pub mod backoff;

If the directory doesn't exist:

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

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

/// 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):

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:

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):

let is_auth = err_str.contains("API error 401")
    || err_str.contains("API error 402")
    || err_str.contains("API error 403");

Replace with:

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

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<Arc<AtomicBool>> 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

//! Shared abort-flag checks.
//!
//! The two variants (Option<Arc<AtomicBool>> 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<Arc<AtomicBool>>) -> 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

pub mod abort;

Step 3: Replace 16 call sites

In engine.rs (4 sites):

// 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):

// 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):

crate::app::util::abort::is_aborted(&sp.abort_flag)

In provider.rs (2 sites):

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

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

/// Mark state dirty and return an empty action list.
fn mark(state: &mut AppStateRest) -> Vec<Action> {
    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

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: ~450600 lines of duplication removed across ~85 file changes.