Skip to content

AII-836 #1360

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iceljc merged 1 commit into from
Jun 1, 2026
Merged

AII-836 #1360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,8 @@ public interface IMembaseApi

[Post("/graph/{graphId}/pgt-definitions/{definitionId}/validate")]
Task<PgtValidationResponse> ValidatePgtDefinitionAsync(string graphId, string definitionId, [Body] PgtValidationRequest request);

[Post("/graph/{graphId}/pgt-external/{correlationId}/complete")]
Task<PgtExternalCompleteResponse> CompletePgtExternalAsync(string graphId, string correlationId);
#endregion
}
42 changes: 42 additions & 0 deletions src/Plugins/BotSharp.Plugin.Membase/Models/Responses/PgtExternalCompleteResponse.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System.Text.Json.Serialization;

namespace BotSharp.Plugin.Membase.Models;

public class PgtExternalCompleteResponse
{
public PgtExternalTask Task { get; set; } = new();

[JsonPropertyName("already_completed")]
public bool AlreadyCompleted { get; set; }
}

public class PgtExternalTask
{
public string TaskId { get; set; } = string.Empty;
public string RunId { get; set; } = string.Empty;
public string GraphId { get; set; } = string.Empty;
public string NodeId { get; set; } = string.Empty;
public string TaskType { get; set; } = string.Empty;
public string CorrelationId { get; set; } = string.Empty;
public string Status { get; set; } = string.Empty;

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public Dictionary<string, object>? Request { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public Dictionary<string, object>? Response { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public object? Error { get; set; }

public DateTime CreatedAt { get; set; }
public DateTime UpdatedAt { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public DateTime? CompletedAt { get; set; }

public long NotBefore { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public DateTime? ClaimedAt { get; set; }
Comment on lines +15 to +41
Copy link
Copy Markdown
Contributor

@qodo-code-review qodo-code-review Bot Jun 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Missing jsonpropertyname for task fields 📘 Rule violation ≡ Correctness

PgtExternalCompleteResponse explicitly maps already_completed as snake_case, but the nested
PgtExternalTask properties (e.g., TaskId, RunId, GraphId, CreatedAt) lack
JsonPropertyName attributes and will be bound using the repo’s default System.Text.Json behavior
(camelCase), which can fail if the Membase API returns snake_case keys like task_id/created_at.
This mismatch can leave fields at defaults (e.g., string.Empty, 0), causing contract drift,
silent data loss, and downstream logic failures.
Agent Prompt 
## Issue description
`PgtExternalTask` likely does not correctly map to Membase’s JSON contract because its properties are missing `[JsonPropertyName]` attributes for fields that are commonly returned as snake_case (e.g., `task_id`, `run_id`, `created_at`). Since the Refit client is not configured with a snake_case naming policy and the repo defaults to System.Text.Json’s usual behavior, these fields can silently deserialize to default values (empty/0), causing data loss and breaking downstream logic.

## Issue Context
- `AlreadyCompleted` is explicitly mapped from `already_completed`, indicating the upstream contract uses snake_case at least for some fields.
- Other Membase response models in this plugin handle snake_case via explicit `[JsonPropertyName]` mappings.
- Refit is configured without a custom JSON serializer/naming policy, so snake_case keys will not automatically bind to PascalCase properties.
- Fields at risk include (as applicable in the DTO): `task_id`, `run_id`, `graph_id`, `node_id`, `task_type`, `correlation_id`, `created_at`, `updated_at`, `completed_at`, `not_before`, `claimed_at`.

## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.Membase/Models/Responses/PgtExternalCompleteResponse.cs[13-42]
- src/Plugins/BotSharp.Plugin.Membase/MembasePlugin.cs[19-36]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
Loading