Refactor and clean up code across multiple modules
- Simplified token type assignment in OAuth service. - Removed unused session_lock module and re-exported Session from zesdex_entities. - Cleaned up session entity by removing unnecessary comments and code. - Consolidated session handling in HTTP handlers for better readability. - Improved formatting and readability in OAuth repository tests. - Enhanced session lock repository with clearer match statements. - Streamlined session repository error handling. - Refined RNG tests for better clarity. - Adjusted module visibility and organization in lib.rs. - Updated IPC client and connection code for better error handling and clarity. - Improved frame handling in IPC for better readability. - Organized module imports and added test utilities for IPC. - Enhanced database connection error handling. - Simplified JWT token creation error handling. - Improved password verification error handling. - Cleaned up state management code for better readability. - Refactored middleware for session authentication and rate limiting. - Simplified clipboard utility for better error handling. - Enhanced logging initialization for better error reporting. - Improved pagination utility with clearer method annotations. - Cleaned up sanitization functions for filenames and paths. - Enhanced slug generation functions for better clarity and usability.
This commit is contained in:
@@ -1,7 +1,11 @@
|
||||
#![allow(clippy::cast_possible_truncation, clippy::cast_sign_loss, clippy::cast_precision_loss, clippy::cast_possible_wrap)]
|
||||
#![allow(
|
||||
clippy::cast_possible_truncation,
|
||||
clippy::cast_sign_loss,
|
||||
clippy::cast_precision_loss,
|
||||
clippy::cast_possible_wrap
|
||||
)]
|
||||
//! Adaptive quality-review triggering, build/test probing, staleness
|
||||
//! sweeps for stored lessons, and the pending-lesson approval workflow.
|
||||
use std::process::Command;
|
||||
use crate::app::state::rest::AppStateRest;
|
||||
use crate::app::state::runtime::TurnEvent;
|
||||
use crate::app::state::types::{Origin, Toast, ToastKind};
|
||||
@@ -9,6 +13,7 @@ use crate::app::subagent::context::build_subagent_context;
|
||||
use crate::app::subagent::engine::run_subagent;
|
||||
use crate::app::subagent::spawn::AgentDefinition;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::process::Command;
|
||||
use zesdex_cms::domain::memory::Memory;
|
||||
use zesdex_cms::domain::repository::MemoryRepository;
|
||||
use zesdex_cms::infrastructure::persistence::memory_repo::MarkdownMemoryRepository;
|
||||
@@ -74,11 +79,12 @@ pub struct Lesson {
|
||||
///
|
||||
/// Return: `true` if a review should be triggered this turn.
|
||||
pub fn should_trigger_review(state: &AppStateRest, origin: Origin) -> bool {
|
||||
|
||||
if origin != Origin::Main {
|
||||
return false;
|
||||
}
|
||||
let Some(runtime) = &state.session_runtime else { return false };
|
||||
let Some(runtime) = &state.session_runtime else {
|
||||
return false;
|
||||
};
|
||||
if !state.settings.flags.review_enabled {
|
||||
return false;
|
||||
}
|
||||
@@ -119,18 +125,28 @@ pub struct ProbeResult {
|
||||
/// Return: `None` if no workspace exists, no command could be resolved,
|
||||
/// or the process failed to spawn/poll; otherwise `Some(ProbeResult)`
|
||||
/// describing pass/fail/timeout and truncated output.
|
||||
pub fn probe_build_test(workspaces: &[std::path::PathBuf], verify_command: Option<&str>, timeout_ms: u64) -> Option<ProbeResult> {
|
||||
pub fn probe_build_test(
|
||||
workspaces: &[std::path::PathBuf],
|
||||
verify_command: Option<&str>,
|
||||
timeout_ms: u64,
|
||||
) -> Option<ProbeResult> {
|
||||
let probe_dir = workspaces.first()?;
|
||||
let cmd = resolve_verify_command(probe_dir, verify_command)?;
|
||||
|
||||
let (cmd_prog, cmd_args) = cmd.split_once(' ').map_or_else(|| (cmd.clone(), String::new()), |(p, a)| (p.to_string(), a.to_string()));
|
||||
let (cmd_prog, cmd_args) = cmd.split_once(' ').map_or_else(
|
||||
|| (cmd.clone(), String::new()),
|
||||
|(p, a)| (p.to_string(), a.to_string()),
|
||||
);
|
||||
|
||||
let Ok(mut child) = Command::new(&cmd_prog)
|
||||
.args(cmd_args.split_whitespace())
|
||||
.current_dir(probe_dir)
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stderr(std::process::Stdio::piped())
|
||||
.spawn() else { return None };
|
||||
.spawn()
|
||||
else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let start = std::time::Instant::now();
|
||||
let timed_out = loop {
|
||||
@@ -141,9 +157,19 @@ pub fn probe_build_test(workspaces: &[std::path::PathBuf], verify_command: Optio
|
||||
match child.try_wait() {
|
||||
Ok(Some(status)) => {
|
||||
let output = child.wait_with_output().ok();
|
||||
let stdout = output.as_ref().map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string()).unwrap_or_default();
|
||||
let stderr = output.as_ref().map(|o| String::from_utf8_lossy(&o.stderr).trim().to_string()).unwrap_or_default();
|
||||
let combined = if stderr.is_empty() { stdout } else { format!("{stdout}\n{stderr}") };
|
||||
let stdout = output
|
||||
.as_ref()
|
||||
.map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string())
|
||||
.unwrap_or_default();
|
||||
let stderr = output
|
||||
.as_ref()
|
||||
.map(|o| String::from_utf8_lossy(&o.stderr).trim().to_string())
|
||||
.unwrap_or_default();
|
||||
let combined = if stderr.is_empty() {
|
||||
stdout
|
||||
} else {
|
||||
format!("{stdout}\n{stderr}")
|
||||
};
|
||||
return Some(ProbeResult {
|
||||
command: cmd.clone(),
|
||||
passed: status.success(),
|
||||
@@ -151,7 +177,9 @@ pub fn probe_build_test(workspaces: &[std::path::PathBuf], verify_command: Optio
|
||||
timed_out: false,
|
||||
});
|
||||
}
|
||||
Ok(None) => { std::thread::sleep(std::time::Duration::from_millis(50)); }
|
||||
Ok(None) => {
|
||||
std::thread::sleep(std::time::Duration::from_millis(50));
|
||||
}
|
||||
Err(_) => return None,
|
||||
}
|
||||
};
|
||||
@@ -180,7 +208,10 @@ pub fn probe_build_test(workspaces: &[std::path::PathBuf], verify_command: Optio
|
||||
///
|
||||
/// Return: `Some(command)` if a command could be determined, `None` if
|
||||
/// no marker files matched (e.g. plain Python project with no test dir).
|
||||
fn resolve_verify_command(probe_dir: &std::path::Path, override_cmd: Option<&str>) -> Option<String> {
|
||||
fn resolve_verify_command(
|
||||
probe_dir: &std::path::Path,
|
||||
override_cmd: Option<&str>,
|
||||
) -> Option<String> {
|
||||
if let Some(cmd) = override_cmd {
|
||||
if !cmd.trim().is_empty() {
|
||||
return Some(cmd.trim().to_string());
|
||||
@@ -201,18 +232,35 @@ fn resolve_verify_command(probe_dir: &std::path::Path, override_cmd: Option<&str
|
||||
let pkg = std::fs::read_to_string(probe_dir.join("package.json")).ok()?;
|
||||
if let Ok(v) = serde_json::from_str::<serde_json::Value>(&pkg) {
|
||||
let scripts = v.get("scripts")?;
|
||||
if scripts.get("test").and_then(|s| s.as_str()).as_ref().is_some_and(|s| !s.is_empty()) {
|
||||
if scripts
|
||||
.get("test")
|
||||
.and_then(|s| s.as_str())
|
||||
.as_ref()
|
||||
.is_some_and(|s| !s.is_empty())
|
||||
{
|
||||
return Some("npm test 2>&1".to_string());
|
||||
}
|
||||
if scripts.get("build").and_then(|s| s.as_str()).as_ref().is_some_and(|s| !s.is_empty()) {
|
||||
if scripts
|
||||
.get("build")
|
||||
.and_then(|s| s.as_str())
|
||||
.as_ref()
|
||||
.is_some_and(|s| !s.is_empty())
|
||||
{
|
||||
return Some("npm run build 2>&1".to_string());
|
||||
}
|
||||
}
|
||||
return Some("npm test 2>&1".to_string());
|
||||
}
|
||||
if has_file("pyproject.toml") || has_file("requirements.txt") || has_file("setup.py") || has_file("setup.cfg") || has_file("Pipfile") || has_file("poetry.lock") {
|
||||
if has_file("pyproject.toml")
|
||||
|| has_file("requirements.txt")
|
||||
|| has_file("setup.py")
|
||||
|| has_file("setup.cfg")
|
||||
|| has_file("Pipfile")
|
||||
|| has_file("poetry.lock")
|
||||
{
|
||||
if has_file("pyproject.toml") {
|
||||
let content = std::fs::read_to_string(probe_dir.join("pyproject.toml")).unwrap_or_default();
|
||||
let content =
|
||||
std::fs::read_to_string(probe_dir.join("pyproject.toml")).unwrap_or_default();
|
||||
if content.contains("[tool.pytest") {
|
||||
return Some("python -m pytest --tb=short -q 2>&1".to_string());
|
||||
}
|
||||
@@ -307,10 +355,7 @@ fn truncate_output(s: &str, max: usize) -> String {
|
||||
/// propagate from constructing the subagent context, not from the review
|
||||
/// itself (that failure is reported via a `SystemNote` instead).
|
||||
/// Compose the system prompt for the quality-review subagent.
|
||||
fn compose_review_prompt(
|
||||
state: &AppStateRest,
|
||||
probe_note: &str,
|
||||
) -> String {
|
||||
fn compose_review_prompt(state: &AppStateRest, probe_note: &str) -> String {
|
||||
let diff_output = if let Some(workspace) = state.workspace_roots.first() {
|
||||
std::process::Command::new("git")
|
||||
.arg("diff")
|
||||
@@ -323,10 +368,15 @@ fn compose_review_prompt(
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
|
||||
|
||||
let history_output = if let Some(rt) = &state.session_runtime {
|
||||
let msgs: Vec<String> = rt.messages.iter()
|
||||
.filter(|m| m.role == crate::dto::chat::message::Role::Assistant || m.role == crate::dto::chat::message::Role::User)
|
||||
let msgs: Vec<String> = rt
|
||||
.messages
|
||||
.iter()
|
||||
.filter(|m| {
|
||||
m.role == crate::dto::chat::message::Role::Assistant
|
||||
|| m.role == crate::dto::chat::message::Role::User
|
||||
})
|
||||
.rev()
|
||||
.take(10)
|
||||
.map(|m| format!("{:?}: {}", m.role, m.content.as_deref().unwrap_or("")))
|
||||
@@ -371,7 +421,6 @@ fn compose_review_prompt(
|
||||
/// Return: `Ok(())` once the review has been kicked off; errors only
|
||||
/// propagate from constructing the subagent context, not from the review
|
||||
/// itself (that failure is reported via a `SystemNote` instead).
|
||||
#[allow(clippy::unnecessary_debug_formatting)]
|
||||
pub fn trigger_review(state: &mut AppStateRest) {
|
||||
state.misc.lesson_running = true;
|
||||
|
||||
@@ -380,17 +429,22 @@ pub fn trigger_review(state: &mut AppStateRest) {
|
||||
let content = std::fs::read_to_string(&gitignore_path).unwrap_or_default();
|
||||
if !content.contains("docs/lesson") {
|
||||
use std::io::Write;
|
||||
if let Ok(mut file) = std::fs::OpenOptions::new().create(true).append(true).open(&gitignore_path) {
|
||||
let prefix = if content.is_empty() || content.ends_with('\n') { "" } else { "\n" };
|
||||
if let Ok(mut file) = std::fs::OpenOptions::new()
|
||||
.create(true)
|
||||
.append(true)
|
||||
.open(&gitignore_path)
|
||||
{
|
||||
let prefix = if content.is_empty() || content.ends_with('\n') {
|
||||
""
|
||||
} else {
|
||||
"\n"
|
||||
};
|
||||
let _ = writeln!(file, "{prefix}docs/lesson/");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let mut def = AgentDefinition::new(
|
||||
"lesson-generator".to_string(),
|
||||
"reviewer".to_string(),
|
||||
);
|
||||
let mut def = AgentDefinition::new("lesson-generator".to_string(), "reviewer".to_string());
|
||||
// Explicitly allow write_file for docs/lesson
|
||||
def.allowed_tools = Some(vec![
|
||||
"read".to_string(),
|
||||
@@ -402,7 +456,7 @@ pub fn trigger_review(state: &mut AppStateRest) {
|
||||
let mut ctx = build_subagent_context(&def);
|
||||
ctx.session_dir.clone_from(&state.session_dir);
|
||||
ctx.workspaces.clone_from(&state.workspace_roots);
|
||||
|
||||
|
||||
let probe_result = probe_build_test(
|
||||
&state.workspace_roots,
|
||||
state.settings.verify_command.as_deref(),
|
||||
@@ -416,7 +470,10 @@ pub fn trigger_review(state: &mut AppStateRest) {
|
||||
} else if r.timed_out {
|
||||
format!("Build/test verification timed out ({}).", r.command)
|
||||
} else {
|
||||
format!("Build/test verification failed ({}). Output: {}", r.command, r.output)
|
||||
format!(
|
||||
"Build/test verification failed ({}). Output: {}",
|
||||
r.command, r.output
|
||||
)
|
||||
}
|
||||
}
|
||||
None => "No build/test probe matched.".to_string(),
|
||||
@@ -432,13 +489,22 @@ pub fn trigger_review(state: &mut AppStateRest) {
|
||||
let mut rx = rx;
|
||||
while let Some(event) = rx.blocking_recv() {
|
||||
match &event {
|
||||
SubagentEvent::ToolCall { tool, .. } => tracing::debug!("[review] tool call: {}", tool),
|
||||
SubagentEvent::ToolResult { tool, .. } => tracing::debug!("[review] tool result: {}", tool),
|
||||
SubagentEvent::ToolCall { tool, .. } => {
|
||||
tracing::debug!("[review] tool call: {}", tool)
|
||||
}
|
||||
SubagentEvent::ToolResult { tool, .. } => {
|
||||
tracing::debug!("[review] tool result: {}", tool)
|
||||
}
|
||||
SubagentEvent::StepCompleted { .. } => tracing::trace!("[review] step completed"),
|
||||
SubagentEvent::StepFailed { step, error } => tracing::warn!("[review] step {} failed: {}", step, error),
|
||||
SubagentEvent::StepFailed { step, error } => {
|
||||
tracing::warn!("[review] step {} failed: {}", step, error)
|
||||
}
|
||||
SubagentEvent::Progress(_) => {}
|
||||
SubagentEvent::Completed { .. } => tracing::debug!("[review] completed"),
|
||||
SubagentEvent::Usage { tokens_in, tokens_out } => {
|
||||
SubagentEvent::Completed => tracing::debug!("[review] completed"),
|
||||
SubagentEvent::Usage {
|
||||
tokens_in,
|
||||
tokens_out,
|
||||
} => {
|
||||
if let Ok(mut q) = turn_events_for_drain.lock() {
|
||||
q.push_back(TurnEvent::ReviewUsage {
|
||||
tokens_in: *tokens_in,
|
||||
@@ -449,7 +515,7 @@ pub fn trigger_review(state: &mut AppStateRest) {
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
let turn_events = state.turn_events.clone();
|
||||
|
||||
std::thread::spawn(move || {
|
||||
@@ -487,15 +553,18 @@ const STALE_AFTER_DAYS: i64 = 60;
|
||||
/// `mem.write`.
|
||||
pub fn run_staleness_sweep(memory_dir: &std::path::Path) -> std::io::Result<Vec<String>> {
|
||||
let mut flagged = Vec::new();
|
||||
let names = MarkdownMemoryRepository::new().list(memory_dir).unwrap_or_default();
|
||||
let names = MarkdownMemoryRepository::new()
|
||||
.list(memory_dir)
|
||||
.unwrap_or_default();
|
||||
let now = chrono::Utc::now().timestamp_millis();
|
||||
let cutoff = now - STALE_AFTER_DAYS * 24 * 3600 * 1000;
|
||||
for name in names {
|
||||
if let Ok(mut mem) = MarkdownMemoryRepository::new().load(memory_dir, &name) {
|
||||
if mem.updated_at < cutoff && mem.lifecycle != "stale" {
|
||||
mem.lifecycle = "stale".to_string();
|
||||
MarkdownMemoryRepository::new().save(memory_dir, &mem)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?;
|
||||
MarkdownMemoryRepository::new()
|
||||
.save(memory_dir, &mem)
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
flagged.push(name);
|
||||
}
|
||||
}
|
||||
@@ -521,7 +590,11 @@ pub fn maybe_run_staleness_sweep(state: &mut AppStateRest) {
|
||||
if !flagged.is_empty() {
|
||||
state.push_toast(Toast::new(
|
||||
ToastKind::Info,
|
||||
format!("Staleness sweep: {} lesson(s) flagged as stale: {}", flagged.len(), flagged.join(", ")),
|
||||
format!(
|
||||
"Staleness sweep: {} lesson(s) flagged as stale: {}",
|
||||
flagged.len(),
|
||||
flagged.join(", ")
|
||||
),
|
||||
));
|
||||
}
|
||||
}
|
||||
@@ -551,7 +624,10 @@ pub fn load_pending_lessons(session_dir: &std::path::Path) -> Vec<PendingLesson>
|
||||
/// Write the session's pending-lessons queue to disk as pretty JSON.
|
||||
///
|
||||
/// Return: `Ok(())`, or an I/O error from writing the file.
|
||||
pub fn save_pending_lessons(session_dir: &std::path::Path, pending: &[PendingLesson]) -> std::io::Result<()> {
|
||||
pub fn save_pending_lessons(
|
||||
session_dir: &std::path::Path,
|
||||
pending: &[PendingLesson],
|
||||
) -> std::io::Result<()> {
|
||||
let path = session_dir.join("pending_lessons.json");
|
||||
let data = serde_json::to_string_pretty(pending)?;
|
||||
std::fs::write(&path, data)
|
||||
@@ -570,7 +646,10 @@ pub fn save_pending_lessons(session_dir: &std::path::Path, pending: &[PendingLes
|
||||
///
|
||||
/// Return: the still-pending lessons (post-commit), or an I/O error from
|
||||
/// writing memory files or the queue.
|
||||
pub fn process_pending_lessons(session_dir: &std::path::Path, memory_dir: &std::path::Path) -> std::io::Result<Vec<PendingLesson>> {
|
||||
pub fn process_pending_lessons(
|
||||
session_dir: &std::path::Path,
|
||||
memory_dir: &std::path::Path,
|
||||
) -> std::io::Result<Vec<PendingLesson>> {
|
||||
let pending = load_pending_lessons(session_dir);
|
||||
let now = chrono::Utc::now().timestamp_millis();
|
||||
let grace_window = 5_000;
|
||||
@@ -599,8 +678,9 @@ pub fn process_pending_lessons(session_dir: &std::path::Path, memory_dir: &std::
|
||||
after_snippet: None,
|
||||
provenances: vec![],
|
||||
};
|
||||
MarkdownMemoryRepository::new().save(memory_dir, &mem)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?;
|
||||
MarkdownMemoryRepository::new()
|
||||
.save(memory_dir, &mem)
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
}
|
||||
|
||||
save_pending_lessons(session_dir, &remaining)?;
|
||||
@@ -644,8 +724,9 @@ pub fn resolve_pending_lesson(
|
||||
after_snippet: None,
|
||||
provenances: vec![],
|
||||
};
|
||||
MarkdownMemoryRepository::new().save(memory_dir, &mem)
|
||||
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e.to_string()))?;
|
||||
MarkdownMemoryRepository::new()
|
||||
.save(memory_dir, &mem)
|
||||
.map_err(|e| std::io::Error::other(e.to_string()))?;
|
||||
}
|
||||
} else {
|
||||
remaining.push(p);
|
||||
|
||||
Reference in New Issue
Block a user