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

976 lines
32 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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
```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<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):
```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 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):
```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<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):
```rust
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:
```rust
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:
```rust
// 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
```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<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:
```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<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`
```rust
//! 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`
```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<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
```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: **~450600 lines of duplication removed** across ~85 file changes.