🐛 Bugfix: Fixed an issue where the created_by field was not written when publishing an agent version. (#3287)

Author: YehongPan

backend/services/agent_version_service.py

@@ -44,8 +44,6 @@ def _remove_audit_fields_for_insert(data: dict) -> None:
     """
     data.pop('create_time', None)
     data.pop('update_time', None)
-    data.pop('created_by', None)
-    data.pop('updated_by', None)
     data.pop('delete_flag', None)
 
 
@@ -90,6 +88,7 @@ def publish_version_impl(
     agent_snapshot.pop('version_no', None)
     agent_snapshot.pop('current_version_no', None)
     agent_snapshot['version_no'] = new_version_no
+    agent_snapshot['updated_by'] = user_id
     _remove_audit_fields_for_insert(agent_snapshot)
 
     # Insert agent snapshot
@@ -100,6 +99,7 @@ def publish_version_impl(
         tool_snapshot = tool.copy()
         tool_snapshot.pop('version_no', None)
         tool_snapshot['version_no'] = new_version_no
+        tool_snapshot['updated_by'] = user_id
         _remove_audit_fields_for_insert(tool_snapshot)
         insert_tool_snapshot(tool_snapshot)
 
@@ -115,6 +115,7 @@ def publish_version_impl(
         rel_snapshot.pop('version_no', None)
         rel_snapshot['version_no'] = new_version_no
         rel_snapshot['selected_agent_version_no'] = child_version
+        rel_snapshot['updated_by'] = user_id
         _remove_audit_fields_for_insert(rel_snapshot)
         insert_relation_snapshot(rel_snapshot)
 
@@ -131,6 +132,7 @@ def publish_version_impl(
         skill_snapshot = skill.copy()
         skill_snapshot.pop('version_no', None)
         skill_snapshot['version_no'] = new_version_no
+        skill_snapshot['updated_by'] = user_id
         _remove_audit_fields_for_insert(skill_snapshot)
         insert_skill_snapshot(skill_snapshot)
 

test/backend/services/test_agent_version_service.py

@@ -305,6 +305,20 @@ def test_publish_version_impl_success(monkeypatch, mock_agent_draft, mock_tools_
     assert relation_snapshot["selected_agent_version_no"] == 1
     assert mock_insert_skill.call_count == 1
 
+    # Verify updated_by is set to user_id on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+
+    tool_snapshot_0 = mock_insert_tool.call_args_list[0][0][0]
+    tool_snapshot_1 = mock_insert_tool.call_args_list[1][0][0]
+    assert tool_snapshot_0["updated_by"] == "user1"
+    assert tool_snapshot_1["updated_by"] == "user1"
+
+    assert relation_snapshot["updated_by"] == "user1"
+
+    skill_snapshot = mock_insert_skill.call_args[0][0]
+    assert skill_snapshot["updated_by"] == "user1"
+
 
 def test_publish_version_impl_unpublished_sub_agent(
     monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft
@@ -376,6 +390,14 @@ def test_publish_version_impl_with_rollback_source(monkeypatch, mock_agent_draft
     assert call_args["source_type"] == "ROLLBACK"
     assert call_args["source_version_no"] == 1
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_skills(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test publishing version with skill instances"""
@@ -412,6 +434,11 @@ def test_publish_version_impl_with_skills(monkeypatch, mock_agent_draft, mock_to
     assert result["version_no"] == 3
     assert mock_insert_skill.call_count == 3
 
+    # Verify updated_by is set on all skill snapshots
+    for i, call in enumerate(mock_insert_skill.call_args_list):
+        skill_snapshot = call[0][0]
+        assert skill_snapshot["updated_by"] == "user1"
+
 
 def test_publish_version_impl_empty_tools_relations(monkeypatch, mock_agent_draft, mock_skills_draft):
     """Test publishing version with no tools or relations"""
@@ -1473,8 +1500,10 @@ def test_remove_audit_fields_for_insert():
     assert "other_field" in data
     assert "create_time" not in data
     assert "update_time" not in data
-    assert "created_by" not in data
-    assert "updated_by" not in data
+    assert "created_by" in data
+    assert data["created_by"] == "user1"
+    assert "updated_by" in data
+    assert data["updated_by"] == "user2"
     assert "delete_flag" not in data
 
 
@@ -1908,6 +1937,14 @@ def test_publish_version_impl_with_a2a_new_agent(monkeypatch, mock_agent_draft,
         version="1",
     )
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_existing_agent(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test publishing version with publish_as_a2a=True for an existing A2A agent"""
@@ -2068,6 +2105,14 @@ def test_publish_version_impl_without_a2a(monkeypatch, mock_agent_draft, mock_to
     a2a_agent_db_mock.get_server_agent_by_agent_id.assert_not_called()
     a2a_agent_db_mock.create_server_agent.assert_not_called()
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_streaming_agent(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test publishing A2A agent that supports streaming"""
@@ -2109,6 +2154,14 @@ def test_publish_version_impl_with_a2a_streaming_agent(monkeypatch, mock_agent_d
     assert result["a2a_agent"]["streaming"] is True
     assert result["a2a_agent_card"]["streaming"] is True
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_existing_agent_no_name(monkeypatch, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test publishing version with publish_as_a2a=True for an existing A2A agent that has no name - uses default name"""
@@ -2187,6 +2240,14 @@ def test_publish_version_impl_with_a2a_existing_agent_no_name(monkeypatch, mock_
         version="1",
     )
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_empty_string_name(monkeypatch, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test publishing with A2A when agent name is empty string - uses default name"""
@@ -2242,6 +2303,14 @@ def test_publish_version_impl_with_a2a_empty_string_name(monkeypatch, mock_tools
     call_kwargs = a2a_agent_db_mock.create_server_agent.call_args[1]
     assert call_kwargs["name"] == "Agent-55"
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_missing_endpoint_id_in_response(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test A2A agent creation response without endpoint_id - card is still created with None endpoint_id"""
@@ -2343,6 +2412,14 @@ def test_publish_version_impl_with_a2a_existing_agent_keeps_endpoint_id(monkeypa
     assert result["a2a_agent_card"]["agent_card_url"] == "/nb/a2a/a2a_1_persistent/.well-known/agent-card.json"
     assert result["a2a_agent_card"]["rest_endpoints"]["message_send"] == "/nb/a2a/a2a_1_persistent/message:send"
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_result_contains_both_keys(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test that publish_version_impl returns both a2a_agent and a2a_agent_card keys when publish_as_a2a=True"""
@@ -2392,6 +2469,14 @@ def test_publish_version_impl_with_a2a_result_contains_both_keys(monkeypatch, mo
     assert isinstance(result["a2a_agent"], dict)
     assert isinstance(result["a2a_agent_card"], dict)
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_description_none(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test A2A agent creation when agent draft has no description"""
@@ -2439,6 +2524,14 @@ def test_publish_version_impl_with_a2a_description_none(monkeypatch, mock_agent_
     # Agent card should reflect None description
     assert result["a2a_agent_card"]["description"] is None
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_existing_agent_description_update(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test that existing A2A agent updates its description from agent_draft"""
@@ -2493,6 +2586,14 @@ def test_publish_version_impl_with_a2a_existing_agent_description_update(monkeyp
     assert result["a2a_agent_card"]["name"] == "Test Agent"
     assert result["a2a_agent_card"]["description"] == "Test Description"
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_with_a2a_agent_card_all_fields(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft):
     """Test A2A agent card contains all expected fields"""
@@ -2554,6 +2655,14 @@ def test_publish_version_impl_with_a2a_agent_card_all_fields(monkeypatch, mock_a
     assert card["jsonrpc_url"] == f"{expected_base_path}/v1"
     assert card["jsonrpc_methods"] == ["SendMessage", "SendStreamingMessage", "GetTask"]
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_a2a_logging_on_create(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft, caplog):
     """Test that appropriate log messages are emitted for A2A agent creation"""
@@ -2599,6 +2708,14 @@ def test_publish_version_impl_a2a_logging_on_create(monkeypatch, mock_agent_draf
     assert any("Creating/updating A2A Server agent" in msg for msg in log_messages)
     assert any("A2A Server agent created/updated with endpoint_id=a2a_1_log" in msg for msg in log_messages)
 
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"
+
 
 def test_publish_version_impl_a2a_logging_on_update(monkeypatch, mock_agent_draft, mock_tools_draft, mock_relations_draft, mock_skills_draft, caplog):
     """Test that appropriate log messages are emitted for A2A agent update"""
@@ -2651,3 +2768,11 @@ def test_publish_version_impl_a2a_logging_on_update(monkeypatch, mock_agent_draf
     assert any("A2A Server agent already exists" in msg for msg in log_messages)
     assert any("Creating/updating A2A Server agent" in msg for msg in log_messages)
     assert any("A2A Server agent created/updated with endpoint_id=a2a_1_existing" in msg for msg in log_messages)
+
+    # Verify updated_by is set on all snapshot types
+    agent_snapshot = mock_insert_agent.call_args[0][0]
+    assert agent_snapshot["updated_by"] == "user1"
+    for call in mock_insert_tool.call_args_list:
+        assert call[0][0]["updated_by"] == "user1"
+    assert mock_insert_relation.call_args[0][0]["updated_by"] == "user1"
+    assert mock_insert_skill.call_args[0][0]["updated_by"] == "user1"