Fix agent type update

[owaiskazi19]

Description

Fix agent update

{
"error": {
  "root_cause": [
  {
  "type": "illegal_argument_exception",
  "reason": "Agent type cannot be updated"
  }
  ],
  "type": "illegal_argument_exception",
  "reason": "Agent type cannot be updated"
  },
  "status": 400
}

Related Issues

Resolves #4340

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Summary by CodeRabbit

• New Features

  • Version 3.4.0 compatibility support added
  • Agent type field now included in update operations

• Improvements

  • Agent type is now immutable during updates; modification attempts will be rejected with validation error

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[rithin-pullela-aws]

LGTM!

Can you rebase your main with current main? Looks like around 27 commits are pushed

[pyek-bot]

rest LGTM, thanks for this!

[mingshl]

Thanks for fixing this!

[coderabbitai]

Walkthrough

Adds a new VERSION_3_4_0 constant to support version-aware serialization. Introduces a type field to MLAgentUpdateInput with version-conditional serialization and validation to prevent agent type modifications during updates.

Changes

Cohort / File(s)	Summary
Version constant common/src/main/java/org/opensearch/ml/common/CommonValue.java	Added VERSION_3_4_0 constant for version-aware serialization support
Agent update type field and validation common/src/main/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInput.java	Added type field with Lombok getter; version-aware serialization/deserialization (VERSION_3_4_0); XContent integration; validation to prevent type changes via IllegalArgumentException in toMLAgent
Type validation tests common/src/test/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInputTest.java	Added testParseWithAllFields with type field; new testAgentTypeValidation to verify same-type updates allowed and type changes rejected

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Version compatibility: Verify VERSION_3_4_0 checks are applied consistently in StreamInput/StreamOutput paths for forward/backward compatibility
• Validation logic: Confirm the IllegalArgumentException is thrown correctly when type mismatch is detected in toMLAgent
• Serialization: Ensure conditional reads/writes for type field align with version checks across toXContent, writeTo, and parse methods

Poem

🐰 A type field now stands guard so true,
Preventing agent swaps that once slipped through!
With version awareness, serialization's aligned,
No more silent failures—the fix is refined! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix agent type update' directly relates to the main objective of preventing silent failures when attempting to update an agent's type field, as described in the linked issue #4340.
Description check	✅ Passed	The PR description follows the repository template with Description, Related Issues, and Check List sections. It includes a concrete error response example and references the linked issue #4340. However, the description lacks detail about what the fix actually does.
Linked Issues check	✅ Passed	The PR code changes implement validation to prevent agent type updates by throwing an IllegalArgumentException when type mismatch is detected, directly addressing issue #4340's requirement to fix the silent failure when changing agent type.
Out of Scope Changes check	✅ Passed	All code changes are directly within scope: VERSION_3_4_0 constant addition supports version-gated serialization, MLAgentUpdateInput type field and validation enforce the fix, and tests verify the new behavior. No extraneous changes detected.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

common/src/test/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInputTest.java (1)

999-1034: Consider adding type field to stream serialization tests.

The testStreamInputOutputWithVersion test doesn't include the type field in the builder, so the version-conditional serialization of the type field (using VERSION_3_4_0) isn't being explicitly tested. Consider adding .type("flow") to verify round-trip serialization works correctly.

     @Test
     public void testStreamInputOutputWithVersion() throws IOException {
         MLAgentUpdateInput input = MLAgentUpdateInput
             .builder()
             .agentId("test-agent-id")
             .name("test-agent")
+            .type("flow")
             .description("test description")
             .llmModelId("test-model-id")
             .llmParameters(Map.of("temperature", "0.7"))
             .memoryType("conversation_index")
             .memorySessionId("test-session")
             .memoryWindowSize(10)
             .appType("rag")
             .lastUpdateTime(Instant.ofEpochMilli(1234567890))
             .tenantId("test-tenant")
             .build();

         // Test with different versions
         BytesStreamOutput output = new BytesStreamOutput();
         input.writeTo(output);

         StreamInput streamInput = output.bytes().streamInput();
         MLAgentUpdateInput parsedInput = new MLAgentUpdateInput(streamInput);

         assertEquals(input.getAgentId(), parsedInput.getAgentId());
         assertEquals(input.getName(), parsedInput.getName());
+        assertEquals(input.getType(), parsedInput.getType());
         assertEquals(input.getDescription(), parsedInput.getDescription());

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2854ad2 and f6c1bc6.

📒 Files selected for processing (3)

• common/src/main/java/org/opensearch/ml/common/CommonValue.java (1 hunks)
• common/src/main/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInput.java (11 hunks)
• common/src/test/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInputTest.java (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

common/src/main/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInput.java (1)

common/src/main/java/org/opensearch/ml/common/CommonValue.java (1)

• CommonValue (14-150)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: spotless
• GitHub Check: spotless

🔇 Additional comments (10)

common/src/main/java/org/opensearch/ml/common/CommonValue.java (1)

100-100: LGTM!

The new version constant follows the established pattern and is correctly placed in sequence after VERSION_3_3_0.

common/src/test/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInputTest.java (2)

390-440: LGTM!

Good addition of type field testing in the parsing test. The test properly validates that the type is correctly parsed from JSON input.

---

964-997: Good test coverage for type validation.

The test comprehensively covers all three scenarios:

1. Same type update is allowed
2. Different type throws IllegalArgumentException with correct message
3. Omitting type preserves the original

common/src/main/java/org/opensearch/ml/common/transport/agent/MLAgentUpdateInput.java (7)

11-11: LGTM!

Correct import of VERSION_3_4_0 for version-conditional serialization.

---

55-107: LGTM!

The type field is properly declared and integrated into the constructor following the existing patterns in this class.

---

109-135: LGTM!

The version-conditional deserialization is correctly implemented. The type field is only read when the stream version is 3.4.0 or later, maintaining backward compatibility with older nodes.

---

144-146: LGTM!

The type field is correctly serialized to XContent when present.

---

197-199: LGTM!

The version-conditional serialization correctly matches the deserialization logic, ensuring proper backward compatibility during rolling upgrades.

---

233-346: LGTM!

The parsing logic correctly handles the type field following the established pattern for other fields in this method.

---

348-351: Core fix for the bug - looks correct.

This validation properly addresses the issue where agent type updates were silently ignored. The check:

1. Allows updates when no type is provided (type == null)
2. Allows updates when the same type is provided
3. Throws IllegalArgumentException when attempting to change the type

This provides clear feedback to API callers instead of the previous silent failure behavior.