mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
feat: persist user_id in save_job and expose job_id on routine runs (#709)
* feat: persist worker events to DB and fix activity tab rendering In-process Worker (used by Scheduler::dispatch_job) now persists events via save_job_event at key execution points: plan creation, LLM responses, tool_use, tool_result, and job completion/failure/stuck. Event data shapes match the container worker format so the gateway activity tab renders them correctly. Frontend: tool_result errors now show a red X icon with danger styling instead of a silent empty output. The result event falls back to the error field when message is absent. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: wire RoutineEngine into gateway for direct manual trigger firing Replace the message-channel hack in routines_trigger_handler with a direct call to RoutineEngine::fire_manual(), ensuring FullJob routines dispatch correctly when triggered from the web UI. Inject the engine into GatewayState from Agent::run after construction. Also persists user_id in save_job for both PG and libSQL backends, removes the source='sandbox' filter so all jobs are visible, and exposes job_id on RoutineRunInfo for the frontend job link. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: remove stale gateway_state argument from Agent::new test call sites The gateway_state parameter was removed from Agent::new during rebase (replaced by post-construction set_routine_engine_slot), but three test call sites still passed the extra None argument. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address PR review — restore sandbox source filter, remove blank lines - Revert removal of `source = 'sandbox'` filter in all SandboxStore queries (8 sites across PG and libSQL). Sandbox-specific APIs should stay scoped to sandbox jobs; unified job listing for the Jobs tab should use a separate query path. - Remove extra blank lines in agent_loop.rs and worker.rs that caused formatting CI failure. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review — regenerate Cargo.lock, add user_id regression test - Regenerate Cargo.lock from main's lockfile to eliminate dependency version downgrades (anyhow, syn, etc.) that were churn from rebase. - Add regression test verifying user_id round-trips through save_job and get_job in the libSQL backend. Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: remove trailing blank line in libsql jobs.rs [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * test: add Postgres-side regression test for user_id persistence in save_job Mirrors the existing libSQL test (test_save_job_persists_user_id) for the Postgres backend. Gated behind #[cfg(feature = "postgres")] + #[ignore] since it requires a running PostgreSQL instance (integration tier). Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
6e12ce6f2d
commit
bcef04b821
@@ -108,6 +108,7 @@ pub async fn routines_detail_handler(
|
||||
status: format!("{:?}", run.status),
|
||||
result_summary: run.result_summary.clone(),
|
||||
tokens_used: run.tokens_used,
|
||||
job_id: run.job_id,
|
||||
})
|
||||
.collect();
|
||||
|
||||
@@ -252,6 +253,7 @@ pub async fn routines_runs_handler(
|
||||
status: format!("{:?}", run.status),
|
||||
result_summary: run.result_summary.clone(),
|
||||
tokens_used: run.tokens_used,
|
||||
job_id: run.job_id,
|
||||
})
|
||||
.collect();
|
||||
|
||||
|
||||
@@ -2017,6 +2017,7 @@ async fn routines_detail_handler(
|
||||
status: format!("{:?}", run.status),
|
||||
result_summary: run.result_summary.clone(),
|
||||
tokens_used: run.tokens_used,
|
||||
job_id: run.job_id,
|
||||
})
|
||||
.collect();
|
||||
|
||||
@@ -2169,6 +2170,7 @@ async fn routines_runs_handler(
|
||||
status: format!("{:?}", run.status),
|
||||
result_summary: run.result_summary.clone(),
|
||||
tokens_used: run.tokens_used,
|
||||
job_id: run.job_id,
|
||||
})
|
||||
.collect();
|
||||
|
||||
|
||||
@@ -776,6 +776,7 @@ pub struct RoutineRunInfo {
|
||||
pub status: String,
|
||||
pub result_summary: Option<String>,
|
||||
pub tokens_used: Option<i32>,
|
||||
pub job_id: Option<Uuid>,
|
||||
}
|
||||
|
||||
// --- Settings ---
|
||||
|
||||
@@ -28,14 +28,16 @@ impl JobStore for LibSqlBackend {
|
||||
r#"
|
||||
INSERT INTO agent_jobs (
|
||||
id, conversation_id, title, description, category, status, source,
|
||||
user_id,
|
||||
budget_amount, budget_token, bid_amount, estimated_cost, estimated_time_secs,
|
||||
actual_cost, repair_attempts, created_at, started_at, completed_at
|
||||
) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17)
|
||||
) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18)
|
||||
ON CONFLICT (id) DO UPDATE SET
|
||||
title = excluded.title,
|
||||
description = excluded.description,
|
||||
category = excluded.category,
|
||||
status = excluded.status,
|
||||
user_id = excluded.user_id,
|
||||
estimated_cost = excluded.estimated_cost,
|
||||
estimated_time_secs = excluded.estimated_time_secs,
|
||||
actual_cost = excluded.actual_cost,
|
||||
@@ -51,6 +53,7 @@ impl JobStore for LibSqlBackend {
|
||||
opt_text(ctx.category.as_deref()),
|
||||
status,
|
||||
"direct",
|
||||
ctx.user_id.as_str(),
|
||||
opt_text_owned(ctx.budget.map(|d| d.to_string())),
|
||||
opt_text(ctx.budget_token.as_deref()),
|
||||
opt_text_owned(ctx.bid_amount.map(|d| d.to_string())),
|
||||
|
||||
@@ -482,6 +482,24 @@ mod tests {
|
||||
assert_eq!(timeout, 5000);
|
||||
}
|
||||
|
||||
/// Regression test: save_job must persist user_id and get_job must return it.
|
||||
#[tokio::test]
|
||||
async fn test_save_job_persists_user_id() {
|
||||
use crate::context::JobContext;
|
||||
use crate::db::JobStore;
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let db_path = dir.path().join("test_user_id.db");
|
||||
let backend = LibSqlBackend::new_local(&db_path).await.unwrap();
|
||||
backend.run_migrations().await.unwrap();
|
||||
|
||||
let ctx = JobContext::with_user("test-user-42", "Test Job", "A test job");
|
||||
backend.save_job(&ctx).await.unwrap();
|
||||
|
||||
let loaded = backend.get_job(ctx.job_id).await.unwrap().unwrap();
|
||||
assert_eq!(loaded.user_id, "test-user-42");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_concurrent_writes_succeed() {
|
||||
// Use a temp file so connections share state (in-memory DBs are connection-local)
|
||||
|
||||
+36
-1
@@ -149,14 +149,16 @@ impl Store {
|
||||
r#"
|
||||
INSERT INTO agent_jobs (
|
||||
id, conversation_id, title, description, category, status, source,
|
||||
user_id,
|
||||
budget_amount, budget_token, bid_amount, estimated_cost, estimated_time_secs,
|
||||
actual_cost, repair_attempts, created_at, started_at, completed_at
|
||||
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17)
|
||||
) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18)
|
||||
ON CONFLICT (id) DO UPDATE SET
|
||||
title = EXCLUDED.title,
|
||||
description = EXCLUDED.description,
|
||||
category = EXCLUDED.category,
|
||||
status = EXCLUDED.status,
|
||||
user_id = EXCLUDED.user_id,
|
||||
estimated_cost = EXCLUDED.estimated_cost,
|
||||
estimated_time_secs = EXCLUDED.estimated_time_secs,
|
||||
actual_cost = EXCLUDED.actual_cost,
|
||||
@@ -172,6 +174,7 @@ impl Store {
|
||||
&ctx.category,
|
||||
&status,
|
||||
&"direct", // source
|
||||
&ctx.user_id,
|
||||
&ctx.budget,
|
||||
&ctx.budget_token,
|
||||
&ctx.bid_amount,
|
||||
@@ -2133,4 +2136,36 @@ mod tests {
|
||||
assert_eq!(summary.channel, ch);
|
||||
}
|
||||
}
|
||||
|
||||
/// Regression test: save_job must persist user_id and get_job must return it.
|
||||
/// Requires a running PostgreSQL instance (integration tier).
|
||||
#[cfg(feature = "postgres")]
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn test_save_job_persists_user_id() {
|
||||
use crate::config::Config;
|
||||
use crate::context::JobContext;
|
||||
|
||||
let _ = dotenvy::dotenv();
|
||||
let config = Config::from_env().await.expect("Failed to load config");
|
||||
let store = Store::new(&config.database)
|
||||
.await
|
||||
.expect("Failed to connect to database");
|
||||
store
|
||||
.run_migrations()
|
||||
.await
|
||||
.expect("Failed to run migrations");
|
||||
|
||||
let ctx = JobContext::with_user("test-user-42", "PG user_id test", "regression test");
|
||||
store.save_job(&ctx).await.unwrap();
|
||||
|
||||
let loaded = store.get_job(ctx.job_id).await.unwrap().unwrap();
|
||||
assert_eq!(loaded.user_id, "test-user-42");
|
||||
|
||||
// Clean up
|
||||
let conn = store.conn().await.unwrap();
|
||||
conn.execute("DELETE FROM agent_jobs WHERE id = $1", &[&ctx.job_id])
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user