feat: relax tool matching on resume

[ixchio]

Summary

Only fail if a tool that was used in history is missing. Allow adding new tools freely when resuming conversations.

Changes

• Add optional used_tools: set[str] | None param to resolve_diff_from_deserialized()
• Extract used tool names from event history in state.py before reconciliation
• When used_tools is provided, only validate that those tools exist in runtime
• New tools can be added to resumed conversations without breaking anything

Tests

• test_conversation_allows_removing_unused_tools - removing tools that were never used is allowed
• test_conversation_allows_adding_new_tools - adding new tools when resuming is allowed

All existing reconciliation tests continue to pass.

Fixes #1533

[neubig]

@OpenHands /codereview-roasted including checking if you can (1) reproduce the issue with the previous code, and (2) run the new code and demonstrate that the problem that existed with the previous code is now fixed.

[openhands-ai]

I'm on it! neubig can track my progress at all-hands.dev

[ixchio]

Closing - recreating with clean single commit

[openhands-ai]

Code Review for PR #1537: "feat: relax tool matching on resume"

Verification Results

✅ Issue Reproduction (main branch)

Successfully reproduced the issue on the main branch:

• Test 1 (Adding new tool): ❌ FAILED with "Tools don't match between runtime and persisted agents. Missing in persisted: {'FileEditorTool'}."
• Test 2 (Removing unused tool): ❌ FAILED with "Tools don't match between runtime and persisted agents. Missing in runtime: {'FileEditorTool'}."

✅ Fix Verification (PR branch)

Successfully verified the fix on the relax-tool-matching-1533 branch:

• Test 1 (Adding new tool): ✅ PASS - Adding new tools when resuming is allowed
• Test 2 (Removing unused tool): ✅ PASS - Removing unused tools when resuming is allowed
• All existing tests: ✅ 8 passed in test_agent_reconciliation.py
• New workspace tests: ✅ 13 passed in test_workspace_pause_resume.py

---

Taste Rating: 🟡 Acceptable - Works but could be cleaner

---

Linus-Style Analysis

[IMPROVEMENT OPPORTUNITIES] (Should fix - violates good taste)

1. [base.py, Lines 386-394] Skipping Strict Comparison
   When used_tools is provided, the entire strict model comparison is skipped. This means other agent configuration changes (beyond tools) could slip through without validation. Consider only skipping the tools comparison, not the entire model comparison.

2. [tests/cross/test_agent_reconciliation.py] Missing Critical Test Case
   There's no test verifying that removing a tool that WAS actually used in history still fails. This is the critical safety check that ensures the fix doesn't break consistency. Add a test like:

   def test_conversation_fails_when_removing_used_tool():
       """Test that removing a tool that was used in history fails."""
       # Create conversation, use a tool (create ActionEvent), then try to resume without it

3. [PR Scope] Unrelated Changes
   The workspace pause() and resume() methods are unrelated to issue Allow adding new tools when resuming conversations without failing #1533 about tool matching. These should be in a separate PR to maintain clean commit history and easier review.

[STYLE NOTES] (Minor)

1. [base.py, Line 371] Comment Clarity
   The comment "Update tools to match runtime (allows new tools to be added)" could be clearer about what's happening - the persisted agent's tools are being replaced with runtime tools.

---

VERDICT:

✅ Worth merging with minor improvements suggested

The core logic is sound and solves the real problem described in issue #1533. The implementation is pragmatic - only fail if tools that were actually used are missing, which provides a better user experience while maintaining consistency.

---

KEY INSIGHT:

The fix correctly identifies that the strict tool matching was overly conservative. By tracking which tools were actually used in history, the code can now allow flexible tool additions while still preventing the removal of tools that would break conversation continuity.

_{View full conversation}

[neubig]

BTW @ixchio it's not necessary to create a single commit -- we squash merge all commits, so they get merged into a single commit anyway.

[ixchio]

BTW @ixchio it's not necessary to create a single commit -- we squash merge all commits, so they get merged into a single commit anyway.

Ah good to know! Thanks for the tip - will skip the cleanup next time since you squash merge anyway. Recreated as #1538 with clean branch, but same code.