Skip to content

Commit 56b6023

Browse files
Simon Rosenbergclaude
andcommitted
Address review 5: mint a fresh id on create (global id uniqueness)
The active pointer is keyed on AgentProfile.id, so ids must be globally unique, not just stable per file. save now mints a server id on create (ignoring any client-supplied id) and preserves it only on overwrite. Prevents a client from creating two names with the same id, which made the pointer ambiguous (deleting one could clear the active selection while a namesake id lives on). Test: creating 'b' with 'a's id yields a distinct id; activating b then deleting a leaves the pointer intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 94c6c5c commit 56b6023

2 files changed

Lines changed: 28 additions & 4 deletions

File tree

openhands-agent-server/openhands/agent_server/agent_profiles_router.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from collections.abc import Iterator
1616
from contextlib import contextmanager
1717
from typing import Annotated, Any
18-
from uuid import UUID
18+
from uuid import UUID, uuid4
1919

2020
from fastapi import APIRouter, HTTPException, Path, Request, status
2121
from pydantic import BaseModel, Field, ValidationError
@@ -408,14 +408,18 @@ async def save_agent_profile(
408408
profile = _decrypt_profile_mcp_tools(profile, cipher)
409409

410410
store = AgentProfileStore()
411-
# The id is stable state, not a defaultable request field: overwriting a
412-
# namesake keeps its id (so an active pointer never dangles) and bumps the
413-
# revision, even when a create-style body omits both.
411+
# The id is server-managed state, never a client-settable field — the active
412+
# pointer is keyed on it. Overwriting a namesake keeps its id (so the pointer
413+
# never dangles) and bumps the revision; creating a new name always mints a
414+
# fresh id (ignoring any client-supplied one) so ids stay globally unique and
415+
# the pointer is never ambiguous.
414416
existing_id, existing_rev = _existing_identity(store, name)
415417
if existing_id is not None:
416418
profile = profile.model_copy(
417419
update={"id": existing_id, "revision": (existing_rev or 0) + 1}
418420
)
421+
else:
422+
profile = profile.model_copy(update={"id": uuid4()})
419423
try:
420424
with _store_errors():
421425
store.save(profile, cipher=cipher, max_profiles=MAX_AGENT_PROFILES)

tests/agent_server/test_agent_profiles_router.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,26 @@ def test_overwrite_preserves_id_and_pointer(client, store):
205205
assert client.get("/api/settings").json()["active_agent_profile_id"] == pid
206206

207207

208+
def test_create_mints_fresh_id_ignoring_client_id(client):
209+
"""Creating a new name never reuses a client-supplied id (ids stay unique).
210+
211+
Duplicate ids would make the id-keyed active pointer ambiguous — deleting
212+
one profile could clear the active selection while a namesake id lives on.
213+
"""
214+
client.post("/api/agent-profiles/a", json={"llm_profile_ref": "x"})
215+
a_id = client.get("/api/agent-profiles/a").json()["profile"]["id"]
216+
217+
# Try to create 'b' reusing a's id; the server must mint a fresh one.
218+
client.post("/api/agent-profiles/b", json={"llm_profile_ref": "y", "id": a_id})
219+
b_id = client.get("/api/agent-profiles/b").json()["profile"]["id"]
220+
assert b_id != a_id
221+
222+
# Activate b, delete a: the pointer must survive (ids are distinct).
223+
client.post(f"/api/agent-profiles/{b_id}/activate")
224+
client.delete("/api/agent-profiles/a")
225+
assert client.get("/api/settings").json()["active_agent_profile_id"] == b_id
226+
227+
208228
def test_save_path_name_is_authoritative(client, store):
209229
"""The path name overrides any ``name`` in the body."""
210230
response = client.post(

0 commit comments

Comments
 (0)