Skip to content

fix(agent): 🐛 Emit requested events for task tools - #153

Merged
jorben merged 1 commit into
masterfrom
fix/tasktool-requested-event
Apr 29, 2026
Merged

jorben merged 1 commit into
masterfrom
fix/tasktool-requested-event

Conversation

@jorben

@jorben jorben commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Emit tool_requested events for task tools before they enter the running state.
  • Align task tool persistence with the requested → running lifecycle.
  • Add a ThreadStream regression test to preserve task tool names and inputs in live routing.

Test Plan

  • cargo fmt --manifest-path src-tauri/Cargo.toml -- --check
  • npm run test:unit -- src/services/thread-stream/thread-stream.test.ts
  • npm run typecheck
  • cargo test --manifest-path src-tauri/Cargo.toml --test task_tracking

🤖 Generated with TiyCode

@github-actions

Copy link
Copy Markdown

AI Code Review Summary

PR: #153 (fix(agent): 🐛 Emit requested events for task tools)
Preferred language: English

Overall Assessment

Detected 1 actionable findings, prioritize CRITICAL/HIGH before merge.

Major Findings by Severity

  • MEDIUM (1)
    • src-tauri/src/core/agent_session_execution.rs:546 - Silently ignored send error on ToolRequested event

Actionable Suggestions

  • Log the ToolRequested send error with eprintln! or tracing::warn rather than silently dropping it.
  • Re-evaluate whether a failed status update to running should abort tool execution entirely or proceed with a fallback warning.

Potential Risks

  • If the event channel is closed, ToolRequested becomes invisible while execution proceeds.
  • A transient status-update failure may terminate the entire agent run unnecessarily.

Test Suggestions

  • Add a test that simulates event sender closure before ToolRequested and asserts that the method handles the channel gracefully (e.g., logs warning).
  • Add a test that simulates a failing tool_call status update and verifies that the agent run error message distinguishes between DB error and tool logic failure.

File-Level Coverage Notes

  • src-tauri/src/core/agent_session_execution.rs: The change introduces a new ToolRequested lifecycle event and a distinct DB status transition, which improves observability. However, the send error for the new event is silently swallowed while DB update failures treat transient issues as hard tool failures, creating subtle correctness and observability gaps. (The rest of the file was not modified in this batch; no further issues detected.)
  • src/services/thread-stream/thread-stream.test.ts: ok (Test correctly asserts that tool_requested, tool_running, and tool_completed events are routed with preserved toolName and toolInput, including the task-specific structured input and result. No regressions or edge cases missed.)

Inline Downgraded Items (processed but not inline)

  • None

Coverage Status

  • Target files: 2
  • Covered files: 2
  • Uncovered files: 0
  • No-patch/binary covered as file-level: 0
  • Findings with unknown confidence (N/A): 0

Uncovered list:

  • None

No-patch covered list:

  • None

Runtime/Budget

  • Rounds used: 1/4
  • Planned batches: 2
  • Executed batches: 2
  • Sub-agent runs: 2
  • Planner calls: 1
  • Reviewer calls: 2
  • Model calls: 3/64
  • Structured-output summary-only degradation: NO

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR review completed.

  • Findings kept: 1
  • Findings with unknown confidence: 0
  • Inline comments attempted: 1
  • Target files: 2
  • Covered files: 2
  • Uncovered files: 0
    See the summary comment for detailed analysis and coverage details.

return agent_error_result(format!("failed to persist tool call: {error}"));
}

let _ = self.event_tx.send(ThreadStreamEvent::ToolRequested {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Silently ignored send error on ToolRequested event

The ToolRequested event send error is silently dropped with let _ = ..., making it impossible to know if frontend subscribers actually received the new state.

Suggestion: Either log a warning when the send fails, or propagate the error as a non-fatal early-warning event so that the caller can detect the broken channel and clean up gracefully.

Risk: If the event channel is closed, the new ToolRequested state becomes invisible to consumers while the method continues execution, leading to subtle UI inconsistencies that are hard to reproduce.

Confidence: 0.85

[From SubAgent: general]

@jorben
jorben merged commit 5e10f91 into master Apr 29, 2026
4 checks passed
@jorben
jorben deleted the fix/tasktool-requested-event branch April 29, 2026 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant