instruct log optimize

[yileicn]

User description

1. /instruct/agentId interface add logId response
2. /logger/instruction/log/{logId}/states update log states

---

PR Type

Enhancement

---

Description

• Add LogId field to instruction response models

• Implement API endpoint to update instruction log states

• Integrate log ID generation and tracking in instruction execution

• Add MongoDB repository support for updating instruction log states

---

Diagram Walkthrough

flowchart LR
  A["InstructService"] -- "generates LogId" --> B["InstructResult"]
  B -- "returns LogId" --> C["InstructResponseModel"]
  C -- "saves with LogId" --> D["InstructionLogDocument"]
  E["LoggerController"] -- "updates states" --> F["UpdateInstructionLogStates"]
  F -- "persists changes" --> D

Loading

File Walkthrough

Relevant files

Enhancement

11 files InstructResponseModel.cs Add LogId property to response model                                          +1/-0      InstructResult.cs Add LogId property with JSON serialization                              +3/-0      ILoggerService.cs Add UpdateInstructionLogStates method to interface              +2/-0      IBotSharpRepository.cs Add UpdateInstructionLogStates repository method                  +3/-0      UpdateInstructionLogStatesModel.cs Create new model for updating log states                                  +15/-0    InstructService.Execute.cs Generate and assign LogId during instruction execution      +2/-0      LoggerService.Instruction.cs Implement UpdateInstructionLogStates service method with validation +10/-0    InstructionLogHook.cs Include LogId when saving instruction logs                              +1/-0      LoggerController.cs Add POST endpoint for updating log states                                +9/-0      InstructionLogDocument.cs Map LogId field in MongoDB document model                                +1/-0      MongoRepository.Log.cs Implement MongoDB update logic for instruction log states +28/-0

---

[qodo-merge-pro]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Unsafe JSON handling Description: Possible JSON injection/parse issues when treating user-supplied state values as JSON without validation in UpdateInstructionLogStates, which could cause unexpected data shape or denial via large payloads; add size limits and strict validation per key. MongoRepository.Log.cs [181-197] Referred Code { var key = updateInstructionStates.StateKeyPrefix + pair.Key; try { var jsonStr = JsonSerializer.Serialize(new { Data = JsonDocument.Parse(pair.Value) }, _botSharpOptions.JsonSerializerOptions); var json = BsonDocument.Parse(jsonStr); logDoc.States[key] = json; } catch { var jsonStr = JsonSerializer.Serialize(new { Data = pair.Value }, _botSharpOptions.JsonSerializerOptions); var json = BsonDocument.Parse(jsonStr); logDoc.States[key] = json; } } await _dc.InstructionLogs.ReplaceOneAsync(p => p.Id == id, logDoc); return true;
	Missing authorization Description: Missing authorization/ownership checks on the public endpoint to update instruction log states allows any authenticated caller (or potentially anonymous if not globally restricted) to modify log states by guessing logId. LoggerController.cs [81-87] Referred Code [HttpPost("/logger/instruction/log/{logId}/states")] public async Task<bool> UpdateInstructionLogStates([FromRoute] string logId, UpdateInstructionLogStatesModel request) { request.LogId = logId; var logging = _services.GetRequiredService<ILoggerService>(); return await logging.UpdateInstructionLogStates(request); }
Ticket Compliance
⚪	🎫 No ticket provided - [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr>
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use atomic updates to prevent race conditions Refactor the UpdateInstructionLogStates method to use an atomic $set update with UpdateOneAsync instead of the current "read-modify-write" pattern to prevent race conditions and improve efficiency. src/Plugins/BotSharp.Plugin.MongoStorage/Repository/MongoRepository.Log.cs [173-198] public async Task<bool> UpdateInstructionLogStates(UpdateInstructionLogStatesModel updateInstructionStates) { var id = updateInstructionStates.LogId; - - var logDoc = await _dc.InstructionLogs.Find(p => p.Id == id).FirstOrDefaultAsync(); - if (logDoc == null) return false; + var filter = Builders<InstructionLogDocument>.Filter.Eq(p => p.Id, id); + var updateBuilder = Builders<InstructionLogDocument>.Update; + var updates = new List<UpdateDefinition<InstructionLogDocument>>(); foreach (var pair in updateInstructionStates.States) { - var key = updateInstructionStates.StateKeyPrefix + pair.Key; + var key = $"States.{updateInstructionStates.StateKeyPrefix}{pair.Key}"; + BsonDocument bsonValue; try { var jsonStr = JsonSerializer.Serialize(new { Data = JsonDocument.Parse(pair.Value) }, _botSharpOptions.JsonSerializerOptions); - var json = BsonDocument.Parse(jsonStr); - logDoc.States[key] = json; + bsonValue = BsonDocument.Parse(jsonStr); } - catch + catch (JsonException) { var jsonStr = JsonSerializer.Serialize(new { Data = pair.Value }, _botSharpOptions.JsonSerializerOptions); - var json = BsonDocument.Parse(jsonStr); - logDoc.States[key] = json; + bsonValue = BsonDocument.Parse(jsonStr); } + updates.Add(updateBuilder.Set(key, bsonValue)); } - await _dc.InstructionLogs.ReplaceOneAsync(p => p.Id == id, logDoc); - return true; + + if (!updates.Any()) + { + return true; + } + + var result = await _dc.InstructionLogs.UpdateOneAsync(filter, updateBuilder.Combine(updates)); + return result.IsAcknowledged && result.ModifiedCount > 0; } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a potential race condition in the "read-modify-write" pattern and proposes using an atomic $set operation, which is the standard, safer, and more performant solution in MongoDB.	High
General	Validate consistency between route and body parameters In UpdateInstructionLogStates, validate that if request.LogId is provided in the request body, it must match the logId from the URL route to ensure data consistency. src/Infrastructure/BotSharp.OpenAPI/Controllers/LoggerController.cs [81-87] [HttpPost("/logger/instruction/log/{logId}/states")] public async Task<bool> UpdateInstructionLogStates([FromRoute] string logId, UpdateInstructionLogStatesModel request) { + if (!string.IsNullOrEmpty(request.LogId) && request.LogId != logId) + { + throw new BadHttpRequestException("Log ID in the request body does not match the Log ID in the URL."); + } request.LogId = logId; var logging = _services.GetRequiredService<ILoggerService>(); return await logging.UpdateInstructionLogStates(request); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion improves API robustness by adding validation to ensure consistency between the logId in the URL route and the request body, preventing potential client-side confusion.	Low
Learned best practice	Validate inputs and JSON safely Guard against null/empty States, keys, and values before parsing; validate JSON with TryParse to avoid exceptions and unnecessary try/catch. src/Plugins/BotSharp.Plugin.MongoStorage/Repository/MongoRepository.Log.cs [180-195] -foreach (var pair in updateInstructionStates.States) +if (updateInstructionStates.States != null) { - var key = updateInstructionStates.StateKeyPrefix + pair.Key; - try + foreach (var pair in updateInstructionStates.States) { - var jsonStr = JsonSerializer.Serialize(new { Data = JsonDocument.Parse(pair.Value) }, _botSharpOptions.JsonSerializerOptions); - var json = BsonDocument.Parse(jsonStr); - logDoc.States[key] = json; - } - catch - { - var jsonStr = JsonSerializer.Serialize(new { Data = pair.Value }, _botSharpOptions.JsonSerializerOptions); - var json = BsonDocument.Parse(jsonStr); + if (string.IsNullOrWhiteSpace(pair.Key)) continue; + var key = (updateInstructionStates.StateKeyPrefix ?? string.Empty) + pair.Key; + var value = pair.Value; + BsonDocument json; + if (!string.IsNullOrWhiteSpace(value) && JsonDocument.TryParseValue(ref new Utf8JsonReader(Encoding.UTF8.GetBytes(value)), out _)) + { + var jsonStr = JsonSerializer.Serialize(new { Data = JsonDocument.Parse(value) }, _botSharpOptions.JsonSerializerOptions); + json = BsonDocument.Parse(jsonStr); + } + else + { + var jsonStr = JsonSerializer.Serialize(new { Data = value }, _botSharpOptions.JsonSerializerOptions); + json = BsonDocument.Parse(jsonStr); + } logDoc.States[key] = json; } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Validate API/async usage semantics to avoid misuse; guard inputs before use and avoid unhandled exceptions during JSON parsing.	Low
More

[adenchen123]

Reviewed