AII-836#1360
Merged
Merged
Conversation
Contributor
Review Summary by QodoAdd PGT external task completion API endpoint
WalkthroughsDescription• Add new API endpoint for completing external PGT tasks • Introduce PgtExternalCompleteResponse model with task details • Support correlation ID-based task completion workflow Diagramflowchart LR
API["IMembaseApi Interface"]
ENDPOINT["POST /graph/{graphId}/pgt-external/{correlationId}/complete"]
RESPONSE["PgtExternalCompleteResponse"]
TASK["PgtExternalTask"]
API -- "adds endpoint" --> ENDPOINT
ENDPOINT -- "returns" --> RESPONSE
RESPONSE -- "contains" --> TASK
File Changes1. src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs
|
Contributor
Code Review by Qodo
1. Missing JsonPropertyName for task fields
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+15
to
+41
| 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; } |
Contributor
There was a problem hiding this comment.
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.