fix: remove debug_assert guards that panic on valid error paths

[henrypark133]

Summary

• Removes two debug_assert! calls from fix: add debug_assert invariant guards to critical code paths #1312 that fire on expected runtime error paths, turning graceful error returns into panics in debug/test builds
• state.rs: Completed→Cancelled is a user-facing error (cancel a completed job) — handled by transition_to() returning Err, not a bug
• execute.rs: empty tool_name from malformed LLM output — handled by ToolError::NotFound, not a bug
• Keeps the circuit-breaker debug_assert (genuinely guards a caller invariant)

Fixes the 2 test failures on staging CI:

• tools::builtin::job::tests::test_cancel_job_completed
• tools::execute::tests::test_execute_empty_tool_name_returns_not_found

Test plan

• Both previously failing tests now pass
• Full lib test suite passes (3223 passed, 0 failed)
• cargo clippy --all --benches --tests --examples --all-features — zero warnings

🤖 Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors error handling by removing debug_assert! statements that were causing debug and test builds to panic on conditions that are, in fact, expected error paths. By allowing these conditions to be handled gracefully as errors rather than assertions, the system's robustness is improved, and two previously failing tests are now passing.

Highlights

• Removed debug_assert! calls: Two debug_assert! calls were removed that previously caused panics on expected runtime error paths in debug/test builds.
• Job State Transition Error: The debug_assert! for Completed -> Cancelled job state transitions in state.rs was removed, allowing graceful error handling.
• Empty Tool Name Error: The debug_assert! for empty tool_name from malformed LLM output in execute.rs was removed, enabling ToolError::NotFound to be returned gracefully.
• Circuit-breaker debug_assert retained: A critical circuit-breaker debug_assert that genuinely guards a caller invariant was intentionally kept.
• Test Fixes: Two specific test failures on staging CI, tools::builtin::job::tests::test_cancel_job_completed and tools::execute::tests::test_execute_empty_tool_name_returns_not_found, were resolved.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly removes two debug_assert! calls that were causing panics in debug builds for what should be handled as recoverable runtime errors. In src/context/state.rs, an invalid job state transition is now consistently handled by returning a Result::Err, which is appropriate for user-facing error paths. In src/tools/execute.rs, an empty tool name, which can result from malformed LLM output, is now gracefully handled by the existing logic that returns a ToolError::NotFound. These changes improve the robustness of the error handling by correctly distinguishing between programmer errors (invariants) and expected runtime failures.

[Copilot]

Pull request overview

This PR removes debug_assert! invariant guards that were firing on expected runtime error paths, causing debug/test builds to panic instead of returning graceful errors. It restores the intended behavior for job state transitions and tool execution when inputs are malformed.

Changes:

• Removed the debug_assert! guarding empty tool_name in execute_tool_with_safety, allowing the existing ToolError::NotFound path to handle malformed calls.
• Removed the debug_assert! guarding invalid job state transitions in JobContext::transition_to, allowing the existing Err(...) return to handle user-facing invalid transitions.
• Updated the empty-tool-name regression test comment to reflect “graceful error, not panic” behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/tools/execute.rs	Removes the empty-tool-name debug_assert! and adjusts the related regression test comment.
src/context/state.rs	Removes the state-transition debug_assert! so invalid transitions return Err instead of panicking in debug/test.

Comments suppressed due to low confidence (1)

src/tools/execute.rs:30

• With the debug_assert removed, an empty tool_name now returns ToolError::NotFound { name: "" }, which formats as Tool not found (blank tool name) and is hard to diagnose. Consider special-casing tool_name.is_empty() to return a clearer error (e.g., use a placeholder like "" in the NotFound name or return InvalidParameters with an explicit message).

    let tool = tools
        .get(tool_name)
        .await
        .ok_or_else(|| crate::error::ToolError::NotFound {
            name: tool_name.to_string(),
        })?;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Removes debug_assert! invariant guards that were triggering panics on expected runtime error paths in debug/test builds, restoring graceful error handling for invalid job state transitions and malformed tool calls.

Changes:

• Removed a debug_assert! in execute_tool_with_safety so empty/malformed tool_name returns ToolError::NotFound instead of panicking in debug/test.
• Removed a debug_assert! in JobContext::transition_to so invalid (but user-triggerable) state transitions return Err without panicking.
• Strengthened the regression test to assert the exact error variant returned for an empty tool name.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/tools/execute.rs	Removes debug panic path for empty tool names and updates regression test expectations.
src/context/state.rs	Removes debug panic path for invalid job state transitions while keeping graceful error return.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.