mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
* fix(ci): secrets can't be used in step if conditions [skip-regression-check] (#787) GitHub Actions step-level `if:` doesn't have access to `secrets` context. Replace `if: secrets.X != ''` with `continue-on-error: true` and let the Set token step handle the fallback. Co-authored-by: Claude Sonnet 4.6 <[email protected]> * fix(ci): clean up staging pipeline — remove hacks, skip redundant checks [skip-regression-check] (#794) - Remove continue-on-error from staging-ci.yml app token steps (secrets are configured) - Skip test.yml and code_style.yml on PRs targeting staging (staging-ci.yml already runs tests before promoting, promotion PR gets full CI on main) - Allow ironclaw-ci[bot] in Claude Code review for bot-created promotion PRs Co-authored-by: Claude Opus 4.6 <[email protected]> * fix(ci): run fmt + clippy on staging PRs, skip Windows clippy [skip-regression-check] (#802) - Remove branches:[main] filter from code_style.yml so it runs on all PRs - Gate clippy-windows with `if: github.base_ref == 'main'` (skip on staging PRs) - Update rollup job to allow skipped clippy-windows - Simplify claude-review.yml to only trigger on labeled event (avoids duplicate runs) Co-authored-by: Claude Opus 4.6 <[email protected]> * 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]> * refactor: unify three agentic loops into single AgenticLoop engine (#654) Replace three independent copy-pasted agentic loops (dispatcher, worker, container runtime) with a single shared engine in `agentic_loop.rs` that all consumers customize via the `LoopDelegate` trait. Phase 1 — Shared engine (`src/agent/agentic_loop.rs`, 205 lines): - `run_agentic_loop()` owns the core LLM → tool exec → repeat cycle - `LoopDelegate` trait (Send + Sync, &dyn dispatch) with 6 hook points - Tool intent nudge logic consolidated (was duplicated in 3 files) - Iteration limit + force-text behavior preserved Phase 2 — Three delegate implementations: - `ChatDelegate` (dispatcher.rs): 3-phase approval flow, hooks, cost guard, context compaction, skill attenuation, interruption - `JobDelegate` (worker/job.rs): planning pre-loop phase, parallel JoinSet exec, mark_completed/stuck/failed, SSE streaming, self-repair - `ContainerDelegate` (worker/container.rs): sequential tool exec, HTTP-proxied LLM, container-safe tools, credential injection Phase 3 — File moves and cleanup: - Delete `src/agent/worker.rs` — job logic moved to `src/worker/job.rs` - Rename `src/worker/runtime.rs` → `src/worker/container.rs` - Re-export `Worker`/`WorkerDeps` from `crate::worker` in `agent/mod.rs` - Update `scheduler.rs` imports to new worker location Shared helpers (`src/tools/execute.rs`): - `execute_tool_with_safety()` replaces 4 copies of validate → timeout → execute → serialize - `process_tool_result()` replaces 3 copies of sanitize → wrap → ChatMessage (also used by thread_ops.rs approval resume paths) Net result: -2,408 lines, zero duplicated loop logic, single code path for tool intent nudge and completion detection. Closes #654 Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review feedback from Copilot 1. scheduler.rs: Replace `unwrap_or` fallback with proper error propagation when parsing tool output JSON — surfaces bugs instead of silently changing the output type. 2. worker/job.rs: Drop MutexGuard before the cancellation `.await` in `check_signals()` to avoid holding a lock across an async I/O call (prevents `await_holding_lock` lint). 3. worker/job.rs: Restore consecutive rate-limit counter (MAX_CONSECUTIVE_RATE_LIMITS = 10) so sustained rate limiting marks the job stuck with "Persistent rate limiting" instead of silently burning through max_iterations. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: incorporate staging changes — token budget tracking + mark_failed Merge staging's changes into the refactored JobDelegate: - Add token budget tracking in call_llm (update_context/add_tokens) - mark_stuck → mark_failed for iteration cap and rate-limit exhaustion (aligns with staging's #788 fix) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address zmanian's PR review — eliminate type erasure, clean up Address all 6 review points from zmanian on PR #800: 1. Replace LoopOutcome::Custom(Box<dyn Any>) with typed LoopOutcome::NeedApproval(Box<PendingApproval>) — eliminates type erasure and downcast, resolves clippy large_enum_variant. 2. Remove dead max_tool_iterations field from ChatDelegate struct. 3. Add on_tool_intent_nudge() hook to LoopDelegate trait with implementations in Job and Container delegates for observability. 4. Fix SSE events in job worker to emit raw sanitized content instead of XML-wrapped <tool_output> tags. 5. Remove 4 duplicate completion tests from job.rs that were already covered by the shared util module. 6. Avoid logging full tool results — use result_size_bytes in debug logs (execute.rs, job.rs). Also updates path references in CLAUDE.md, COVERAGE_PLAN.md, and add-sse-event.md command. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat(doctor): expand diagnostics from 7 to 16 health checks * test: add unit tests for agentic_loop and execute shared modules Add 16 tests covering the two new critical shared modules: agentic_loop.rs (10 tests): - Text response exits loop immediately - Tool call → text response continuation - LoopSignal::Stop exits before LLM call - LoopSignal::InjectMessage adds user message to context - Max iterations terminates with LoopOutcome::MaxIterations - Tool intent nudge fires twice then caps - before_llm_call early exit bypasses LLM - truncate_for_preview: short string, long string, multibyte safety execute.rs (6 tests): - execute_tool_with_safety success path - Missing tool returns ToolError::NotFound - Tool execution failure propagates - Per-tool timeout enforcement (50ms) - process_tool_result XML wrapping on success - process_tool_result error formatting All 2,777 unit tests pass, 0 clippy warnings. Co-Authored-By: Claude Opus 4.6 <[email protected]> * style: cargo fmt Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address code review — 9 issues across agentic loop, job worker, container CRITICAL fixes: - Rate-limit exhaustion now returns Err(LlmError::RateLimited) instead of Ok(Text("")), stopping the loop immediately with no ghost iteration. Below-threshold retries still use Text("") with an explicit empty-string guard in handle_text_response to skip injection. - check_signals drains the entire message channel before returning, prioritizing Stop over UserMessage. Previously returned early on first UserMessage, silently dropping any queued Stop or additional messages. - check_signals now detects all non-progressing job states (Cancelled, Failed, Stuck, Completed, Submitted, Accepted) instead of only Cancelled and Failed. HIGH fixes: - Error path in process_tool_result_job applies truncate_for_preview to bound error strings in SSE/DB events (was unbounded). - Document Send+Sync lifetime constraint on LoopDelegate trait. - Test mock before_llm_call refactored from double-lock to single lock acquisition, eliminating deadlock risk on refactor. MEDIUM fixes: - CompletionReport includes actual iteration count via shared Arc<Mutex<u32>> tracker (was hardcoded 0). - process_tool_result_job return type changed from Result<bool> to Result<()> — the bool was always false (dead API). - Deduplicate truncate in container.rs; now uses truncate_for_preview from agentic_loop. Verified: 0 clippy warnings, 2781 tests pass, cargo fmt clean. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Henry Park <[email protected]> Co-authored-by: Claude Sonnet 4.6 <[email protected]> Co-authored-by: Illia Polosukhin <[email protected]> Co-authored-by: Umesh Kumar Singh <[email protected]> Co-authored-by: reidliu41 <[email protected]>
179 lines
5.6 KiB
Rust
179 lines
5.6 KiB
Rust
//! Shared utility functions used across the codebase.
|
|
|
|
/// Find the largest valid UTF-8 char boundary at or before `pos`.
|
|
///
|
|
/// Polyfill for `str::floor_char_boundary` (nightly-only). Use when
|
|
/// truncating strings by byte position to avoid panicking on multi-byte
|
|
/// characters.
|
|
pub fn floor_char_boundary(s: &str, pos: usize) -> usize {
|
|
if pos >= s.len() {
|
|
return s.len();
|
|
}
|
|
let mut i = pos;
|
|
while i > 0 && !s.is_char_boundary(i) {
|
|
i -= 1;
|
|
}
|
|
i
|
|
}
|
|
|
|
/// Check if an LLM response explicitly signals that a job/task is complete.
|
|
///
|
|
/// Uses phrase-level matching to avoid false positives from bare words like
|
|
/// "done" or "complete" appearing in non-completion contexts (e.g. "not done yet",
|
|
/// "the download is incomplete").
|
|
pub fn llm_signals_completion(response: &str) -> bool {
|
|
let lower = response.to_lowercase();
|
|
|
|
// Superset of phrases from worker/job.rs and worker/container.rs.
|
|
let positive_phrases = [
|
|
"job is complete",
|
|
"job is done",
|
|
"job is finished",
|
|
"task is complete",
|
|
"task is done",
|
|
"task is finished",
|
|
"work is complete",
|
|
"work is done",
|
|
"work is finished",
|
|
"successfully completed",
|
|
"have completed the job",
|
|
"have completed the task",
|
|
"have finished the job",
|
|
"have finished the task",
|
|
"all steps are complete",
|
|
"all steps are done",
|
|
"i have completed",
|
|
"i've completed",
|
|
"all done",
|
|
"all tasks complete",
|
|
];
|
|
|
|
let negative_phrases = [
|
|
"not complete",
|
|
"not done",
|
|
"not finished",
|
|
"incomplete",
|
|
"unfinished",
|
|
"isn't done",
|
|
"isn't complete",
|
|
"isn't finished",
|
|
"not yet done",
|
|
"not yet complete",
|
|
"not yet finished",
|
|
];
|
|
|
|
let has_negative = negative_phrases.iter().any(|p| lower.contains(p));
|
|
if has_negative {
|
|
return false;
|
|
}
|
|
|
|
positive_phrases.iter().any(|p| lower.contains(p))
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use crate::util::{floor_char_boundary, llm_signals_completion};
|
|
|
|
// ── floor_char_boundary ──
|
|
|
|
#[test]
|
|
fn floor_char_boundary_at_valid_boundary() {
|
|
assert_eq!(floor_char_boundary("hello", 3), 3);
|
|
}
|
|
|
|
#[test]
|
|
fn floor_char_boundary_mid_multibyte_char() {
|
|
// h = 1 byte, é = 2 bytes, total 3 bytes
|
|
let s = "hé";
|
|
assert_eq!(floor_char_boundary(s, 2), 1); // byte 2 is mid-é, back up to 1
|
|
}
|
|
|
|
#[test]
|
|
fn floor_char_boundary_past_end() {
|
|
assert_eq!(floor_char_boundary("hi", 100), 2);
|
|
}
|
|
|
|
#[test]
|
|
fn floor_char_boundary_at_zero() {
|
|
assert_eq!(floor_char_boundary("hello", 0), 0);
|
|
}
|
|
|
|
#[test]
|
|
fn floor_char_boundary_empty_string() {
|
|
assert_eq!(floor_char_boundary("", 5), 0);
|
|
}
|
|
|
|
// ── llm_signals_completion ──
|
|
|
|
#[test]
|
|
fn signals_completion_positive() {
|
|
assert!(llm_signals_completion("The job is complete."));
|
|
assert!(llm_signals_completion("I have completed the task."));
|
|
assert!(llm_signals_completion("All done, here are the results."));
|
|
assert!(llm_signals_completion("Task is finished successfully."));
|
|
assert!(llm_signals_completion(
|
|
"I have completed the task successfully."
|
|
));
|
|
assert!(llm_signals_completion(
|
|
"All steps are complete and verified."
|
|
));
|
|
assert!(llm_signals_completion(
|
|
"I've done all the work. The work is done."
|
|
));
|
|
assert!(llm_signals_completion(
|
|
"Successfully completed the migration."
|
|
));
|
|
assert!(llm_signals_completion(
|
|
"I have completed the job ahead of schedule."
|
|
));
|
|
assert!(llm_signals_completion("I have finished the task."));
|
|
assert!(llm_signals_completion("All steps are done now."));
|
|
assert!(llm_signals_completion("I've completed everything."));
|
|
assert!(llm_signals_completion("All tasks complete."));
|
|
}
|
|
|
|
#[test]
|
|
fn signals_completion_negative() {
|
|
assert!(!llm_signals_completion("The task is not complete yet."));
|
|
assert!(!llm_signals_completion("This is not done."));
|
|
assert!(!llm_signals_completion("The work is incomplete."));
|
|
assert!(!llm_signals_completion("Build is unfinished."));
|
|
assert!(!llm_signals_completion(
|
|
"The migration is not yet finished."
|
|
));
|
|
assert!(!llm_signals_completion("The job isn't done yet."));
|
|
assert!(!llm_signals_completion("This remains unfinished."));
|
|
}
|
|
|
|
#[test]
|
|
fn signals_completion_no_bare_substrings() {
|
|
assert!(!llm_signals_completion("The download completed."));
|
|
assert!(!llm_signals_completion(
|
|
"Function done_callback was called."
|
|
));
|
|
assert!(!llm_signals_completion("Set is_complete = true"));
|
|
assert!(!llm_signals_completion("Running step 3 of 5"));
|
|
assert!(!llm_signals_completion(
|
|
"I need to complete more work first."
|
|
));
|
|
assert!(!llm_signals_completion(
|
|
"Let me finish the remaining steps."
|
|
));
|
|
assert!(!llm_signals_completion(
|
|
"I'm done analyzing, now let me fix it."
|
|
));
|
|
assert!(!llm_signals_completion(
|
|
"I completed step 1 but step 2 remains."
|
|
));
|
|
}
|
|
|
|
#[test]
|
|
fn signals_completion_tool_output_injection() {
|
|
assert!(!llm_signals_completion("TASK_COMPLETE"));
|
|
assert!(!llm_signals_completion("JOB_DONE"));
|
|
assert!(!llm_signals_completion(
|
|
"The tool returned: TASK_COMPLETE signal"
|
|
));
|
|
}
|
|
}
|