Skip to content

Conversation

@xingyaoww
Copy link
Collaborator

@xingyaoww xingyaoww commented Oct 7, 2025

Description

This PR fixes issue #668 by allowing the security_analyzer field to be different between runtime and persisted agents during conversation restoration.

Problem

Previously, when trying to restart a conversation with a different security analyzer configuration, the reconciliation logic in AgentBase.resolve_diff_from_deserialized() would fail with an error:

ValueError: The Agent provided is different from the one in persisted state.
Diff: security_analyzer: '<missing>' -> {'kind': 'LLMSecurityAnalyzer'}

This prevented users from:

  • Removing security analyzers when switching to weaker LLMs that cannot handle the security_risk field
  • Adding security analyzers mid-conversation
  • Changing security analyzer configurations without complex workarounds

Solution

Modified AgentBase.resolve_diff_from_deserialized() to explicitly allow the security_analyzer field to differ, with the runtime agent's value taking precedence. This is appropriate because:

  1. The security analyzer doesn't affect conversation history or state
  2. It's a runtime concern about how to analyze actions
  3. Users should be able to switch it on/off based on which LLM they're using

Changes

  • openhands/sdk/agent/base.py: Updated resolve_diff_from_deserialized() to include security_analyzer in the updates dict, ensuring the runtime agent's security analyzer is used regardless of what's persisted
  • tests/cross/test_agent_reconciliation.py: Added comprehensive tests covering:
    • Unit tests for removing and adding security analyzers during reconciliation
    • Integration tests for full conversation restart scenarios with different security analyzers

Testing

All tests pass, including:

  • ✅ New tests for security analyzer reconciliation scenarios
  • ✅ All existing reconciliation tests continue to pass
  • ✅ Pre-commit hooks (type checking, linting, formatting) pass

Related Issues

Fixes #668

Breaking Changes

None. This is a backward-compatible change that relaxes validation to support a previously unsupported use case.

@xingyaoww can click here to continue refining the PR


Agent Server images for this PR

GHCR package: https://github.com/All-Hands-AI/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant Base Image Docs / Tags
golang golang:1.21-bookworm Link
java eclipse-temurin:17-jdk Link
python nikolaik/python-nodejs:python3.12-nodejs22 Link

Pull (multi-arch manifest)

docker pull ghcr.io/all-hands-ai/agent-server:233775b-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-233775b-python \
  ghcr.io/all-hands-ai/agent-server:233775b-python

All tags pushed for this build

ghcr.io/all-hands-ai/agent-server:233775b-golang
ghcr.io/all-hands-ai/agent-server:v1.0.0_golang_tag_1.21-bookworm_golang
ghcr.io/all-hands-ai/agent-server:233775b-java
ghcr.io/all-hands-ai/agent-server:v1.0.0_eclipse-temurin_tag_17-jdk_java
ghcr.io/all-hands-ai/agent-server:233775b-python
ghcr.io/all-hands-ai/agent-server:v1.0.0_nikolaik_s_python-nodejs_tag_python3.12-nodejs22_python

The 233775b tag is a multi-arch manifest (amd64/arm64); your client pulls the right arch automatically.

This change allows the security_analyzer field to be different between
runtime and persisted agents during conversation restoration. The runtime
agent's security_analyzer now takes precedence, enabling users to:

- Remove security analyzers when switching to weaker LLMs that cannot
  handle the security_risk field
- Add security analyzers mid-conversation
- Change security analyzer configurations without manual workarounds

This fixes issue #668 by modifying AgentBase.resolve_diff_from_deserialized()
to explicitly update the security_analyzer field with the runtime value,
preventing reconciliation errors.

Added comprehensive tests covering:
- Removing security analyzer from persisted agent
- Adding security analyzer to persisted agent
- Full conversation restart scenarios with different security analyzers

Co-authored-by: openhands <[email protected]>
@xingyaoww xingyaoww marked this pull request as ready for review October 8, 2025 16:46
@xingyaoww xingyaoww merged commit ae493e0 into main Oct 8, 2025
14 checks passed
@xingyaoww xingyaoww deleted the openhands/allow-security-analyzer-diff branch October 8, 2025 16:46
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.

Cannot easily remove security analyzer from agent mid conversation

4 participants