Compare commits

...
Author SHA1 Message Date
Henry ParkandClaude Opus 4.6 f714b0a3dd fix(ci): run all test jobs when invoked via workflow_call
When staging-ci.yml calls test.yml as a reusable workflow, the event
name is the caller's event (workflow_dispatch/schedule), not push or
pull_request. This caused telegram-tests, windows-build, wasm-wit-compat,
and docker-build to be skipped during staging promotion.

Add github.event_name == 'workflow_call' to each gated job's condition.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>
2026-03-09 19:38:17 -07:00
83950d11a4 fix: job token budget, iteration cap → Failed, web cancel stops worker (#788)
* 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]>

* fix: add job token budget, change iteration cap to Failed, fix web cancel (#698)

Jobs could enter infinite retry loops because: (1) no token budget was
enforced, (2) iteration cap marked jobs as Stuck (allowing self-repair to
restart them), and (3) the web UI cancel button only updated the DB without
stopping the running worker.

- Add `max_tokens_per_job` config (settings.json + AGENT_MAX_TOKENS_PER_JOB
  env var, default 0 = unlimited) with per-job metadata override
- Track token usage after respond_with_tools() and fail the job on budget
  exceeded
- Change iteration cap and persistent rate limiting from mark_stuck to
  mark_failed, preventing self-repair restart loops
- Fix web cancel handler to call scheduler.stop() which updates in-memory
  state AND aborts the worker task, falling back to DB-only update

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review — always persist cancel to DB, simplify token check

- Cancel handler now always persists Cancelled to DB regardless of whether
  scheduler.stop() ran, fixing the edge case where stop() returns Ok(())
  for jobs not in the scheduler map
- Collapse nested ifs per clippy (let-chains)
- Add NOTE comment about select_tools() not exposing TokenUsage

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: rustfmt formatting in wizard.rs (pre-existing)

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-10 02:19:56 +00:00
Nick PismenkovandGitHub 764be8547f fix: fmt (#805) 2026-03-09 19:14:36 -07:00
7de639e782 fix(ci): cherry-pick fmt + clippy for staging PRs [skip-regression-check] (#803)
Cherry-pick of #802: run fmt + clippy on staging PRs, skip Windows clippy,
simplify claude-review trigger to labeled-only.

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-09 18:44:57 -07:00
a5f88b32fd fix(setup): pass NEARAI_API_KEY to provider in model selection step (#799)
When users authenticate via NEAR AI Cloud API key (option 4) during
onboarding, the key is stored as an env var but fetch_nearai_models()
was hardcoding api_key: None. This caused resolve_bearer_token() to
re-trigger the interactive auth prompt at step 4 (model selection).

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-10 01:26:12 +00:00
Nick PismenkovandGitHub 7d8576a464 fix: destructive actions from ambiguous user prompts (#782)
* fix: destructive actions from ambiguous user prompts

* review fixes

* review fixes
2026-03-09 18:03:39 -07:00
f4b7309523 fix(ci): cherry-pick CI cleanup onto staging [skip-regression-check] (#798)
Cherry-pick of #794: remove continue-on-error hack, skip redundant checks
on staging PRs, allow ironclaw-ci[bot] in Claude Code review.

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-09 17:59:59 -07:00
Henry ParkandClaude Sonnet 4.6 577e26eff4 fix(ci): secrets can't be used in step if conditions [skip-regression-check]
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]>
2026-03-09 16:41:43 -07:00
20 changed files with 484 additions and 49 deletions
+2
View File
@@ -115,6 +115,8 @@ AGENT_NAME=ironclaw
AGENT_MAX_PARALLEL_JOBS=5 AGENT_MAX_PARALLEL_JOBS=5
AGENT_JOB_TIMEOUT_SECS=3600 AGENT_JOB_TIMEOUT_SECS=3600
AGENT_STUCK_THRESHOLD_SECS=300 AGENT_STUCK_THRESHOLD_SECS=300
# Maximum tokens per job (0 = unlimited, also settable via settings.json agent.max_tokens_per_job)
# AGENT_MAX_TOKENS_PER_JOB=0
# Enable planning phase before tool execution (default: true) # Enable planning phase before tool execution (default: true)
AGENT_USE_PLANNING=true AGENT_USE_PLANNING=true
+2 -1
View File
@@ -2,7 +2,7 @@ name: Claude Code Review
on: on:
pull_request: pull_request:
types: [opened, labeled] types: [labeled]
permissions: permissions:
contents: read contents: read
@@ -28,6 +28,7 @@ jobs:
uses: anthropics/claude-code-action@v1 uses: anthropics/claude-code-action@v1
with: with:
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
allowed_bots: "ironclaw-ci[bot]"
claude_args: "--max-turns 50 --model claude-haiku-4-5-20251001 --allowedTools 'Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*),Bash(git blame:*),Bash(git log:*),Bash(git diff:*)'" claude_args: "--max-turns 50 --model claude-haiku-4-5-20251001 --allowedTools 'Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*),Bash(git blame:*),Bash(git log:*),Bash(git diff:*)'"
prompt: | prompt: |
Code review this pull request. Follow these steps precisely: Code review this pull request. Follow these steps precisely:
+7 -1
View File
@@ -44,6 +44,7 @@ jobs:
clippy-windows: clippy-windows:
name: Clippy Windows (${{ matrix.name }}) name: Clippy Windows (${{ matrix.name }})
if: github.base_ref == 'main'
runs-on: windows-latest runs-on: windows-latest
strategy: strategy:
fail-fast: false fail-fast: false
@@ -76,7 +77,12 @@ jobs:
needs: [format, clippy, clippy-windows] needs: [format, clippy, clippy-windows]
steps: steps:
- run: | - run: |
if [[ "${{ needs.format.result }}" != "success" || "${{ needs.clippy.result }}" != "success" || "${{ needs.clippy-windows.result }}" != "success" ]]; then if [[ "${{ needs.format.result }}" != "success" || "${{ needs.clippy.result }}" != "success" ]]; then
echo "One or more jobs failed" echo "One or more jobs failed"
exit 1 exit 1
fi fi
# clippy-windows only runs on main PRs, so skip/success are both acceptable
if [[ "${{ needs.clippy-windows.result }}" == "failure" ]]; then
echo "Windows clippy failed"
exit 1
fi
-2
View File
@@ -115,7 +115,6 @@ jobs:
- name: Generate GitHub App token - name: Generate GitHub App token
id: app-token id: app-token
if: ${{ secrets.GH_RELEASES_MANAGER_APP_ID != '' }}
uses: actions/create-github-app-token@v2 uses: actions/create-github-app-token@v2
with: with:
app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }} app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }}
@@ -230,7 +229,6 @@ jobs:
- name: Generate GitHub App token - name: Generate GitHub App token
id: app-token id: app-token
if: ${{ secrets.GH_RELEASES_MANAGER_APP_ID != '' }}
uses: actions/create-github-app-token@v2 uses: actions/create-github-app-token@v2
with: with:
app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }} app-id: ${{ secrets.GH_RELEASES_MANAGER_APP_ID }}
+6
View File
@@ -2,6 +2,8 @@ name: Run Tests
on: on:
workflow_call: workflow_call:
pull_request: pull_request:
branches:
- main
push: push:
branches: branches:
- main - main
@@ -41,6 +43,7 @@ jobs:
name: Telegram Channel Tests name: Telegram Channel Tests
if: > if: >
github.event_name == 'push' || github.event_name == 'push' ||
github.event_name == 'workflow_call' ||
(github.event_name == 'pull_request' && github.base_ref != 'staging') (github.event_name == 'pull_request' && github.base_ref != 'staging')
runs-on: ubuntu-latest runs-on: ubuntu-latest
steps: steps:
@@ -56,6 +59,7 @@ jobs:
name: Windows Build (${{ matrix.name }}) name: Windows Build (${{ matrix.name }})
if: > if: >
github.event_name == 'push' || github.event_name == 'push' ||
github.event_name == 'workflow_call' ||
(github.event_name == 'pull_request' && github.base_ref != 'staging') (github.event_name == 'pull_request' && github.base_ref != 'staging')
runs-on: windows-latest runs-on: windows-latest
strategy: strategy:
@@ -83,6 +87,7 @@ jobs:
name: WASM WIT Compatibility name: WASM WIT Compatibility
if: > if: >
github.event_name == 'push' || github.event_name == 'push' ||
github.event_name == 'workflow_call' ||
(github.event_name == 'pull_request' && github.base_ref != 'staging') (github.event_name == 'pull_request' && github.base_ref != 'staging')
runs-on: ubuntu-latest runs-on: ubuntu-latest
steps: steps:
@@ -106,6 +111,7 @@ jobs:
name: Docker Build name: Docker Build
if: > if: >
github.event_name == 'push' || github.event_name == 'push' ||
github.event_name == 'workflow_call' ||
(github.event_name == 'pull_request' && github.base_ref != 'staging') (github.event_name == 'pull_request' && github.base_ref != 'staging')
runs-on: ubuntu-latest runs-on: ubuntu-latest
steps: steps:
+93
View File
@@ -1205,6 +1205,7 @@ mod tests {
max_tool_iterations: 50, max_tool_iterations: 50,
auto_approve_tools: false, auto_approve_tools: false,
default_timezone: "UTC".to_string(), default_timezone: "UTC".to_string(),
max_tokens_per_job: 0,
}, },
deps, deps,
Arc::new(ChannelManager::new()), Arc::new(ChannelManager::new()),
@@ -1263,6 +1264,96 @@ mod tests {
} }
} }
#[test]
fn test_always_approval_requirement_bypasses_session_auto_approve() {
// Regression test: even if tool is auto-approved in session,
// ApprovalRequirement::Always must still trigger approval.
use crate::tools::ApprovalRequirement;
let mut session = Session::new("user-1");
let tool_name = "tool_remove";
// Manually auto-approve tool_remove in this session
session.auto_approve_tool(tool_name);
assert!(
session.is_tool_auto_approved(tool_name),
"tool should be auto-approved"
);
// However, ApprovalRequirement::Always should always require approval
// This is verified by the dispatcher logic: Always => true (ignores session state)
let always_req = ApprovalRequirement::Always;
let requires_approval = match always_req {
ApprovalRequirement::Never => false,
ApprovalRequirement::UnlessAutoApproved => !session.is_tool_auto_approved(tool_name),
ApprovalRequirement::Always => true,
};
assert!(
requires_approval,
"ApprovalRequirement::Always must require approval even when tool is auto-approved"
);
}
#[test]
fn test_always_approval_requirement_vs_unless_auto_approved() {
// Verify the two requirements behave differently
use crate::tools::ApprovalRequirement;
let mut session = Session::new("user-2");
let tool_name = "http";
// Scenario 1: Tool is auto-approved
session.auto_approve_tool(tool_name);
// UnlessAutoApproved → doesn't require approval if auto-approved
let unless_req = ApprovalRequirement::UnlessAutoApproved;
let unless_needs = match unless_req {
ApprovalRequirement::Never => false,
ApprovalRequirement::UnlessAutoApproved => !session.is_tool_auto_approved(tool_name),
ApprovalRequirement::Always => true,
};
assert!(
!unless_needs,
"UnlessAutoApproved should not need approval when auto-approved"
);
// Always → always requires approval
let always_req = ApprovalRequirement::Always;
let always_needs = match always_req {
ApprovalRequirement::Never => false,
ApprovalRequirement::UnlessAutoApproved => !session.is_tool_auto_approved(tool_name),
ApprovalRequirement::Always => true,
};
assert!(
always_needs,
"Always must always require approval, even when auto-approved"
);
// Scenario 2: Tool is NOT auto-approved
let new_tool = "new_tool";
assert!(!session.is_tool_auto_approved(new_tool));
// UnlessAutoApproved → requires approval
let unless_needs = match unless_req {
ApprovalRequirement::Never => false,
ApprovalRequirement::UnlessAutoApproved => !session.is_tool_auto_approved(new_tool),
ApprovalRequirement::Always => true,
};
assert!(
unless_needs,
"UnlessAutoApproved should need approval when not auto-approved"
);
// Always → always requires approval
let always_needs = match always_req {
ApprovalRequirement::Never => false,
ApprovalRequirement::UnlessAutoApproved => !session.is_tool_auto_approved(new_tool),
ApprovalRequirement::Always => true,
};
assert!(always_needs, "Always must always require approval");
}
#[test] #[test]
fn test_pending_approval_serialization_backcompat_without_deferred_calls() { fn test_pending_approval_serialization_backcompat_without_deferred_calls() {
// PendingApproval from before the deferred_tool_calls field was added // PendingApproval from before the deferred_tool_calls field was added
@@ -1953,6 +2044,7 @@ mod tests {
max_tool_iterations, max_tool_iterations,
auto_approve_tools: true, auto_approve_tools: true,
default_timezone: "UTC".to_string(), default_timezone: "UTC".to_string(),
max_tokens_per_job: 0,
}, },
deps, deps,
Arc::new(ChannelManager::new()), Arc::new(ChannelManager::new()),
@@ -2069,6 +2161,7 @@ mod tests {
max_tool_iterations: max_iter, max_tool_iterations: max_iter,
auto_approve_tools: true, auto_approve_tools: true,
default_timezone: "UTC".to_string(), default_timezone: "UTC".to_string(),
max_tokens_per_job: 0,
}, },
deps, deps,
Arc::new(ChannelManager::new()), Arc::new(ChannelManager::new()),
+16
View File
@@ -160,6 +160,13 @@ impl Scheduler {
.create_job_for_user(user_id, title, description) .create_job_for_user(user_id, title, description)
.await?; .await?;
// Apply token budget from config, allowing per-job metadata override.
let max_tokens = metadata
.as_ref()
.and_then(|m| m.get("max_tokens"))
.and_then(|v| v.as_u64())
.unwrap_or(self.config.max_tokens_per_job);
// Apply metadata if provided // Apply metadata if provided
if let Some(meta) = metadata { if let Some(meta) = metadata {
self.context_manager self.context_manager
@@ -169,6 +176,15 @@ impl Scheduler {
.await?; .await?;
} }
// Set token budget (separate update to avoid overwriting metadata)
if max_tokens > 0 {
self.context_manager
.update_context(job_id, |ctx| {
ctx.max_tokens = max_tokens;
})
.await?;
}
// Persist to DB before scheduling so the worker's FK references are valid // Persist to DB before scheduling so the worker's FK references are valid
if let Some(ref store) = self.store { if let Some(ref store) = self.store {
let ctx = self.context_manager.get_context(job_id).await?; let ctx = self.context_manager.get_context(job_id).await?;
+100 -3
View File
@@ -417,7 +417,8 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
iteration += 1; iteration += 1;
if iteration > max_iterations { if iteration > max_iterations {
self.mark_stuck("Maximum iterations exceeded").await?; self.mark_failed("Maximum iterations exceeded: job hit the iteration cap")
.await?;
return Ok(()); return Ok(());
} }
@@ -437,7 +438,8 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
"LLM rate limited during tool selection, backing off" "LLM rate limited during tool selection, backing off"
); );
if consecutive_rate_limits >= MAX_CONSECUTIVE_RATE_LIMITS { if consecutive_rate_limits >= MAX_CONSECUTIVE_RATE_LIMITS {
self.mark_stuck("Persistent rate limiting").await?; self.mark_failed("Persistent rate limiting: exceeded retry limit")
.await?;
return Ok(()); return Ok(());
} }
self.log_event( self.log_event(
@@ -467,7 +469,8 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
"LLM rate limited during respond_with_tools, backing off" "LLM rate limited during respond_with_tools, backing off"
); );
if consecutive_rate_limits >= MAX_CONSECUTIVE_RATE_LIMITS { if consecutive_rate_limits >= MAX_CONSECUTIVE_RATE_LIMITS {
self.mark_stuck("Persistent rate limiting").await?; self.mark_failed("Persistent rate limiting: exceeded retry limit")
.await?;
return Ok(()); return Ok(());
} }
self.log_event( self.log_event(
@@ -483,6 +486,20 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
Err(e) => return Err(e.into()), Err(e) => return Err(e.into()),
}; };
// Track token usage from LLM call against the job budget.
// NOTE: select_tools() also makes LLM calls but doesn't expose
// TokenUsage; only respond_with_tools() usage is tracked here.
let total_tokens = respond_output.usage.total() as u64;
if total_tokens > 0
&& let Err(msg) = self
.context_manager()
.update_context(self.job_id, |ctx| ctx.add_tokens(total_tokens))
.await?
{
self.mark_failed(&msg).await?;
return Ok(());
}
match respond_output.result { match respond_output.result {
RespondResult::Text(response) => { RespondResult::Text(response) => {
// Check for explicit completion phrases. Use word-boundary // Check for explicit completion phrases. Use word-boundary
@@ -1762,4 +1779,84 @@ mod tests {
"Always tool should be allowed with permission" "Always tool should be allowed with permission"
); );
} }
#[tokio::test]
async fn test_token_budget_exceeded_fails_job() {
let worker = make_worker(vec![]).await;
// Transition to InProgress (required for mark_failed)
worker
.context_manager()
.update_context(worker.job_id, |ctx| {
ctx.transition_to(JobState::InProgress, None)
})
.await
.unwrap()
.unwrap();
// Set a token budget
worker
.context_manager()
.update_context(worker.job_id, |ctx| {
ctx.max_tokens = 100;
})
.await
.unwrap();
// Simulate adding tokens that exceed the budget
let budget_result = worker
.context_manager()
.update_context(worker.job_id, |ctx| ctx.add_tokens(200))
.await
.unwrap();
assert!(
budget_result.is_err(),
"Should return error when token budget exceeded"
);
// Verify that mark_failed transitions job to Failed
worker
.mark_failed(&budget_result.unwrap_err())
.await
.unwrap();
let ctx = worker
.context_manager()
.get_context(worker.job_id)
.await
.unwrap();
assert_eq!(ctx.state, JobState::Failed);
}
#[tokio::test]
async fn test_iteration_cap_marks_failed_not_stuck() {
let worker = make_worker(vec![]).await;
// Transition to InProgress (required for mark_failed)
worker
.context_manager()
.update_context(worker.job_id, |ctx| {
ctx.transition_to(JobState::InProgress, None)
})
.await
.unwrap()
.unwrap();
// Simulate what the execution loop does when max_iterations is exceeded
worker
.mark_failed("Maximum iterations exceeded: job hit the iteration cap")
.await
.unwrap();
let ctx = worker
.context_manager()
.get_context(worker.job_id)
.await
.unwrap();
assert_eq!(
ctx.state,
JobState::Failed,
"Iteration cap should transition to Failed, not Stuck"
);
}
} }
+15 -1
View File
@@ -276,11 +276,25 @@ pub async fn jobs_cancel_handler(
}))); })));
} }
// Fall back to agent job cancellation via DB status update. // Fall back to agent job cancellation: stop the worker via the scheduler
// (which updates the in-memory ContextManager AND aborts the task handle),
// then persist the status to the DB as a fallback.
if let Some(ref store) = state.store if let Some(ref store) = state.store
&& let Ok(Some(job)) = store.get_job(job_id).await && let Ok(Some(job)) = store.get_job(job_id).await
{ {
if job.state.is_active() { if job.state.is_active() {
// Try to stop via scheduler (aborts the worker task + updates
// in-memory ContextManager). This is best-effort — the job may
// not be in the scheduler map if it already finished.
if let Some(ref slot) = state.scheduler
&& let Some(ref scheduler) = *slot.read().await
{
let _ = scheduler.stop(job_id).await;
}
// Always persist cancellation to the DB so the state is
// consistent even if the scheduler wasn't available or the
// job wasn't in its in-memory map.
store store
.update_job_status( .update_job_status(
job_id, job_id,
+2
View File
@@ -108,6 +108,7 @@ pub async fn routines_detail_handler(
status: format!("{:?}", run.status), status: format!("{:?}", run.status),
result_summary: run.result_summary.clone(), result_summary: run.result_summary.clone(),
tokens_used: run.tokens_used, tokens_used: run.tokens_used,
job_id: run.job_id,
}) })
.collect(); .collect();
@@ -252,6 +253,7 @@ pub async fn routines_runs_handler(
status: format!("{:?}", run.status), status: format!("{:?}", run.status),
result_summary: run.result_summary.clone(), result_summary: run.result_summary.clone(),
tokens_used: run.tokens_used, tokens_used: run.tokens_used,
job_id: run.job_id,
}) })
.collect(); .collect();
+2
View File
@@ -2017,6 +2017,7 @@ async fn routines_detail_handler(
status: format!("{:?}", run.status), status: format!("{:?}", run.status),
result_summary: run.result_summary.clone(), result_summary: run.result_summary.clone(),
tokens_used: run.tokens_used, tokens_used: run.tokens_used,
job_id: run.job_id,
}) })
.collect(); .collect();
@@ -2169,6 +2170,7 @@ async fn routines_runs_handler(
status: format!("{:?}", run.status), status: format!("{:?}", run.status),
result_summary: run.result_summary.clone(), result_summary: run.result_summary.clone(),
tokens_used: run.tokens_used, tokens_used: run.tokens_used,
job_id: run.job_id,
}) })
.collect(); .collect();
+1
View File
@@ -776,6 +776,7 @@ pub struct RoutineRunInfo {
pub status: String, pub status: String,
pub result_summary: Option<String>, pub result_summary: Option<String>,
pub tokens_used: Option<i32>, pub tokens_used: Option<i32>,
pub job_id: Option<Uuid>,
} }
// --- Settings --- // --- Settings ---
+7
View File
@@ -29,6 +29,8 @@ pub struct AgentConfig {
pub auto_approve_tools: bool, pub auto_approve_tools: bool,
/// Default timezone for new sessions (IANA name, e.g. "America/New_York"). /// Default timezone for new sessions (IANA name, e.g. "America/New_York").
pub default_timezone: String, pub default_timezone: String,
/// Maximum tokens per job (0 = unlimited).
pub max_tokens_per_job: u64,
} }
impl AgentConfig { impl AgentConfig {
@@ -50,6 +52,7 @@ impl AgentConfig {
max_tool_iterations: 10, max_tool_iterations: 10,
auto_approve_tools: true, auto_approve_tools: true,
default_timezone: "UTC".to_string(), default_timezone: "UTC".to_string(),
max_tokens_per_job: 0,
} }
} }
@@ -105,6 +108,10 @@ impl AgentConfig {
} }
tz tz
}, },
max_tokens_per_job: parse_optional_env(
"AGENT_MAX_TOKENS_PER_JOB",
settings.agent.max_tokens_per_job,
)?,
}) })
} }
} }
+4 -1
View File
@@ -28,14 +28,16 @@ impl JobStore for LibSqlBackend {
r#" r#"
INSERT INTO agent_jobs ( INSERT INTO agent_jobs (
id, conversation_id, title, description, category, status, source, id, conversation_id, title, description, category, status, source,
user_id,
budget_amount, budget_token, bid_amount, estimated_cost, estimated_time_secs, budget_amount, budget_token, bid_amount, estimated_cost, estimated_time_secs,
actual_cost, repair_attempts, created_at, started_at, completed_at 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 ON CONFLICT (id) DO UPDATE SET
title = excluded.title, title = excluded.title,
description = excluded.description, description = excluded.description,
category = excluded.category, category = excluded.category,
status = excluded.status, status = excluded.status,
user_id = excluded.user_id,
estimated_cost = excluded.estimated_cost, estimated_cost = excluded.estimated_cost,
estimated_time_secs = excluded.estimated_time_secs, estimated_time_secs = excluded.estimated_time_secs,
actual_cost = excluded.actual_cost, actual_cost = excluded.actual_cost,
@@ -51,6 +53,7 @@ impl JobStore for LibSqlBackend {
opt_text(ctx.category.as_deref()), opt_text(ctx.category.as_deref()),
status, status,
"direct", "direct",
ctx.user_id.as_str(),
opt_text_owned(ctx.budget.map(|d| d.to_string())), opt_text_owned(ctx.budget.map(|d| d.to_string())),
opt_text(ctx.budget_token.as_deref()), opt_text(ctx.budget_token.as_deref()),
opt_text_owned(ctx.bid_amount.map(|d| d.to_string())), opt_text_owned(ctx.bid_amount.map(|d| d.to_string())),
+18
View File
@@ -482,6 +482,24 @@ mod tests {
assert_eq!(timeout, 5000); 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] #[tokio::test]
async fn test_concurrent_writes_succeed() { async fn test_concurrent_writes_succeed() {
// Use a temp file so connections share state (in-memory DBs are connection-local) // Use a temp file so connections share state (in-memory DBs are connection-local)
+36 -1
View File
@@ -149,14 +149,16 @@ impl Store {
r#" r#"
INSERT INTO agent_jobs ( INSERT INTO agent_jobs (
id, conversation_id, title, description, category, status, source, id, conversation_id, title, description, category, status, source,
user_id,
budget_amount, budget_token, bid_amount, estimated_cost, estimated_time_secs, budget_amount, budget_token, bid_amount, estimated_cost, estimated_time_secs,
actual_cost, repair_attempts, created_at, started_at, completed_at 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 ON CONFLICT (id) DO UPDATE SET
title = EXCLUDED.title, title = EXCLUDED.title,
description = EXCLUDED.description, description = EXCLUDED.description,
category = EXCLUDED.category, category = EXCLUDED.category,
status = EXCLUDED.status, status = EXCLUDED.status,
user_id = EXCLUDED.user_id,
estimated_cost = EXCLUDED.estimated_cost, estimated_cost = EXCLUDED.estimated_cost,
estimated_time_secs = EXCLUDED.estimated_time_secs, estimated_time_secs = EXCLUDED.estimated_time_secs,
actual_cost = EXCLUDED.actual_cost, actual_cost = EXCLUDED.actual_cost,
@@ -172,6 +174,7 @@ impl Store {
&ctx.category, &ctx.category,
&status, &status,
&"direct", // source &"direct", // source
&ctx.user_id,
&ctx.budget, &ctx.budget,
&ctx.budget_token, &ctx.budget_token,
&ctx.bid_amount, &ctx.bid_amount,
@@ -2133,4 +2136,36 @@ mod tests {
assert_eq!(summary.channel, ch); 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();
}
} }
+5
View File
@@ -386,6 +386,10 @@ pub struct AgentSettings {
/// Default timezone for new sessions (IANA name, e.g. "America/New_York"). /// Default timezone for new sessions (IANA name, e.g. "America/New_York").
#[serde(default = "default_timezone")] #[serde(default = "default_timezone")]
pub default_timezone: String, pub default_timezone: String,
/// Maximum tokens per job (0 = unlimited).
#[serde(default)]
pub max_tokens_per_job: u64,
} }
fn default_agent_name() -> String { fn default_agent_name() -> String {
@@ -442,6 +446,7 @@ impl Default for AgentSettings {
max_tool_iterations: default_max_tool_iterations(), max_tool_iterations: default_max_tool_iterations(),
auto_approve_tools: false, auto_approve_tools: false,
default_timezone: default_timezone(), default_timezone: default_timezone(),
max_tokens_per_job: 0,
} }
} }
} }
+103 -32
View File
@@ -1573,46 +1573,18 @@ impl SetupWizard {
} }
/// Fetch available models from the NEAR AI API. /// Fetch available models from the NEAR AI API.
///
/// Uses [`build_nearai_model_fetch_config`] to construct the provider config,
/// which reads `NEARAI_API_KEY` from the environment when present.
async fn fetch_nearai_models(&self) -> Vec<String> { async fn fetch_nearai_models(&self) -> Vec<String> {
let session = match self.session_manager { let session = match self.session_manager {
Some(ref s) => Arc::clone(s), Some(ref s) => Arc::clone(s),
None => return vec![], None => return vec![],
}; };
use crate::config::LlmConfig;
use crate::llm::create_llm_provider; use crate::llm::create_llm_provider;
let base_url = std::env::var("NEARAI_BASE_URL") let config = build_nearai_model_fetch_config();
.unwrap_or_else(|_| "https://private.near.ai".to_string());
let auth_base_url = std::env::var("NEARAI_AUTH_URL")
.unwrap_or_else(|_| "https://private.near.ai".to_string());
let config = LlmConfig {
backend: "nearai".to_string(),
session: crate::llm::session::SessionConfig {
auth_base_url,
session_path: crate::config::llm::default_session_path(),
},
nearai: crate::config::NearAiConfig {
model: "dummy".to_string(),
cheap_model: None,
base_url,
api_key: None,
fallback_model: None,
max_retries: 3,
circuit_breaker_threshold: None,
circuit_breaker_recovery_secs: 30,
response_cache_enabled: false,
response_cache_ttl_secs: 3600,
response_cache_max_entries: 1000,
failover_cooldown_secs: 300,
failover_cooldown_threshold: 3,
smart_routing_cascade: true,
},
provider: None,
bedrock: None,
request_timeout_secs: 120,
};
match create_llm_provider(&config, session).await { match create_llm_provider(&config, session).await {
Ok(provider) => match provider.list_models().await { Ok(provider) => match provider.list_models().await {
@@ -3240,6 +3212,52 @@ async fn discover_wasm_channels(dir: &std::path::Path) -> Vec<(String, ChannelCa
/// Mask an API key for display: show first 6 + last 4 chars. /// Mask an API key for display: show first 6 + last 4 chars.
/// ///
/// Uses char-based indexing to avoid panicking on multi-byte UTF-8. /// Uses char-based indexing to avoid panicking on multi-byte UTF-8.
/// Build the `LlmConfig` used by `fetch_nearai_models` to list available models.
///
/// Reads `NEARAI_API_KEY` from the environment so that users who authenticated
/// via Cloud API key (option 4) don't get re-prompted during model selection.
fn build_nearai_model_fetch_config() -> crate::config::LlmConfig {
let base_url =
std::env::var("NEARAI_BASE_URL").unwrap_or_else(|_| "https://private.near.ai".to_string());
let auth_base_url =
std::env::var("NEARAI_AUTH_URL").unwrap_or_else(|_| "https://private.near.ai".to_string());
// If the user authenticated via API key (option 4), the key is stored
// as an env var. Pass it through so `resolve_bearer_token()` doesn't
// re-trigger the interactive auth prompt.
let api_key = std::env::var("NEARAI_API_KEY")
.ok()
.filter(|k| !k.is_empty())
.map(secrecy::SecretString::from);
crate::config::LlmConfig {
backend: "nearai".to_string(),
session: crate::llm::session::SessionConfig {
auth_base_url,
session_path: crate::config::llm::default_session_path(),
},
nearai: crate::config::NearAiConfig {
model: "dummy".to_string(),
cheap_model: None,
base_url,
api_key,
fallback_model: None,
max_retries: 3,
circuit_breaker_threshold: None,
circuit_breaker_recovery_secs: 30,
response_cache_enabled: false,
response_cache_ttl_secs: 3600,
response_cache_max_entries: 1000,
failover_cooldown_secs: 300,
failover_cooldown_threshold: 3,
smart_routing_cascade: true,
},
provider: None,
bedrock: None,
request_timeout_secs: 120,
}
}
fn mask_api_key(key: &str) -> String { fn mask_api_key(key: &str) -> String {
let chars: Vec<char> = key.chars().collect(); let chars: Vec<char> = key.chars().collect();
if chars.len() < 12 { if chars.len() < 12 {
@@ -3640,6 +3658,14 @@ mod tests {
} }
impl EnvGuard { impl EnvGuard {
fn set(key: &'static str, value: &str) -> Self {
let original = std::env::var(key).ok();
unsafe {
std::env::set_var(key, value);
}
Self { key, original }
}
fn clear(key: &'static str) -> Self { fn clear(key: &'static str) -> Self {
let original = std::env::var(key).ok(); let original = std::env::var(key).ok();
unsafe { unsafe {
@@ -3826,4 +3852,49 @@ mod tests {
}; };
assert!(settings.secrets_master_key_hex.is_some()); assert!(settings.secrets_master_key_hex.is_some());
} }
/// Regression test for #799: `fetch_nearai_models` hardcoded `api_key: None`,
/// causing the auth prompt to re-appear during model selection when the user
/// had authenticated via NEAR AI Cloud API key (option 4).
#[test]
fn test_build_nearai_model_fetch_config_picks_up_api_key_env() {
use secrecy::ExposeSecret;
let _guard = EnvGuard::set("NEARAI_API_KEY", "test-cloud-api-key-12345");
let config = build_nearai_model_fetch_config();
assert!(
config.nearai.api_key.is_some(),
"config should include NEARAI_API_KEY from env"
);
assert_eq!(
config.nearai.api_key.as_ref().unwrap().expose_secret(),
"test-cloud-api-key-12345"
);
}
/// Regression test for #799: when NEARAI_API_KEY is absent or empty,
/// the config should have `api_key: None` (session token path).
#[test]
fn test_build_nearai_model_fetch_config_none_when_no_api_key() {
let _guard = EnvGuard::clear("NEARAI_API_KEY");
let config = build_nearai_model_fetch_config();
assert!(
config.nearai.api_key.is_none(),
"config should have no api_key when env var is absent"
);
}
/// Regression test for #799: empty NEARAI_API_KEY should be treated as absent.
#[test]
fn test_build_nearai_model_fetch_config_none_when_empty_api_key() {
let _guard = EnvGuard::set("NEARAI_API_KEY", "");
let config = build_nearai_model_fetch_config();
assert!(
config.nearai.api_key.is_none(),
"config should have no api_key when env var is empty"
);
}
} }
+32 -4
View File
@@ -451,8 +451,8 @@ impl Tool for ToolRemoveTool {
} }
fn description(&self) -> &str { fn description(&self) -> &str {
"Remove an installed extension (channel, tool, or MCP server). \ "Permanently remove an installed extension (channel, tool, or MCP server) from disk. \
Unregisters tools and deletes configuration." This action cannot be undone the WASM binary and configuration files will be deleted."
} }
fn parameters_schema(&self) -> serde_json::Value { fn parameters_schema(&self) -> serde_json::Value {
@@ -492,7 +492,7 @@ impl Tool for ToolRemoveTool {
} }
fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement { fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement {
ApprovalRequirement::UnlessAutoApproved ApprovalRequirement::Always
} }
} }
@@ -701,10 +701,38 @@ mod tests {
assert_eq!(tool.name(), "tool_remove"); assert_eq!(tool.name(), "tool_remove");
assert_eq!( assert_eq!(
tool.requires_approval(&serde_json::json!({})), tool.requires_approval(&serde_json::json!({})),
ApprovalRequirement::UnlessAutoApproved ApprovalRequirement::Always
); );
} }
#[test]
fn tool_remove_always_requires_approval_regardless_of_params() {
use crate::tools::tool::ApprovalRequirement;
let tool = ToolRemoveTool {
manager: test_manager_stub(),
};
let test_cases = vec![
("no params", serde_json::json!({})),
("empty name", serde_json::json!({"name": ""})),
("slack", serde_json::json!({"name": "slack"})),
("github-cli", serde_json::json!({"name": "github-cli"})),
(
"with extra fields",
serde_json::json!({"name": "tool", "extra": "field"}),
),
];
for (case_name, params) in test_cases {
assert_eq!(
tool.requires_approval(&params),
ApprovalRequirement::Always,
"tool_remove must always require approval for case: {}",
case_name
);
}
}
#[test] #[test]
fn test_tool_upgrade_schema() { fn test_tool_upgrade_schema() {
use crate::tools::tool::ApprovalRequirement; use crate::tools::tool::ApprovalRequirement;
+33 -3
View File
@@ -709,7 +709,8 @@ impl Tool for SkillRemoveTool {
} }
fn description(&self) -> &str { fn description(&self) -> &str {
"Remove an installed skill by name. Only user-installed skills can be removed." "Permanently remove an installed skill from disk. This action cannot be undone — \
the skill files will be deleted."
} }
fn parameters_schema(&self) -> serde_json::Value { fn parameters_schema(&self) -> serde_json::Value {
@@ -770,7 +771,7 @@ impl Tool for SkillRemoveTool {
} }
fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement { fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement {
ApprovalRequirement::UnlessAutoApproved ApprovalRequirement::Always
} }
} }
@@ -837,12 +838,41 @@ mod tests {
assert_eq!(tool.name(), "skill_remove"); assert_eq!(tool.name(), "skill_remove");
assert_eq!( assert_eq!(
tool.requires_approval(&serde_json::json!({})), tool.requires_approval(&serde_json::json!({})),
ApprovalRequirement::UnlessAutoApproved ApprovalRequirement::Always
); );
let schema = tool.parameters_schema(); let schema = tool.parameters_schema();
assert!(schema["properties"].get("name").is_some()); assert!(schema["properties"].get("name").is_some());
} }
#[test]
fn skill_remove_always_requires_approval_regardless_of_params() {
use crate::tools::tool::ApprovalRequirement;
let tool = SkillRemoveTool::new(test_registry());
let test_cases = vec![
("no params", serde_json::json!({})),
("empty name", serde_json::json!({"name": ""})),
(
"deployment skill",
serde_json::json!({"name": "deployment"}),
),
("custom skill", serde_json::json!({"name": "custom-skill"})),
(
"with extra fields",
serde_json::json!({"name": "skill", "extra": "field"}),
),
];
for (case_name, params) in test_cases {
assert_eq!(
tool.requires_approval(&params),
ApprovalRequirement::Always,
"skill_remove must always require approval for case: {}",
case_name
);
}
}
#[test] #[test]
fn test_validate_fetch_url_allows_https() { fn test_validate_fetch_url_allows_https() {
assert!(super::validate_fetch_url("https://clawhub.ai/api/v1/download?slug=foo").is_ok()); assert!(super::validate_fetch_url("https://clawhub.ai/api/v1/download?slug=foo").is_ok());