Inline idempotency docs into README #68

KSemenenko · 2025-10-11T11:45:10Z

Summary

inline the command correlation/tracing guidance into the README under the command pattern section
fold the idempotency architecture review into the README and remove the redundant docs pages

Testing

~/.dotnet/dotnet test ManagedCode.Communication.Tests/ManagedCode.Communication.Tests.csproj

https://chatgpt.com/codex/tasks/task_e_68e9fbc7ad5c83268e969223e18fcead

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

@codex fix this CI failure
@codex address that feedback

chatgpt-codex-connector · 2025-10-11T11:48:50Z

ManagedCode.Communication.Orleans/Grains/CommandIdempotencyGrain.cs

    public async Task<bool> TryStartProcessingAsync()
    {
-        // Check if already processing or completed
-        if (state.State.Status != CommandExecutionStatus.NotFound)
+        // Reject concurrent executions
+        if (state.State.Status is CommandExecutionStatus.InProgress or CommandExecutionStatus.Processing)
+        {
+            return false;
+        }
+
+        // Allow retries from failed or completed states by clearing previous outcome
+        if (state.State.Status is CommandExecutionStatus.Completed or CommandExecutionStatus.Failed)
+        {
+            state.State.Result = null;
+            state.State.ErrorMessage = null;
+            state.State.CompletedAt = null;
+            state.State.FailedAt = null;
+        }
+        else if (state.State.Status is not CommandExecutionStatus.NotFound and not CommandExecutionStatus.NotStarted)
        {


Prevent retries when command already succeeded

TryStartProcessingAsync now returns true when the command state is Completed or Failed, clearing the stored result and error metadata before flipping the status back to Processing. This allows the same command id to execute again and overwrites the cached outcome that idempotency is intended to preserve. Callers that rely on TryStartProcessingAsync to guard duplicate execution will re‑run completed commands instead of short‑circuiting, breaking idempotent guarantees and potentially reissuing side effects.

Useful? React with 👍 / 👎.

Copilot

Pull Request Overview

This PR enhances the documentation and implementation of ManagedCode.Communication by consolidating and improving idempotency handling. The changes focus on better documentation organization and fixing null handling in command execution patterns.

Inlines command correlation/tracing guidance and idempotency architecture documentation directly into README
Improves null result handling across idempotency stores and extensions
Adds comprehensive tests for concurrent execution scenarios and null value caching

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 23 comments.

Show a summary per file

File	Description
README.md	Added detailed sections on command correlation/tracing identifiers and idempotency architecture overview
MemoryCacheCommandIdempotencyStore.cs	Replaced global semaphore with per-command locking using reference counting
CommandIdempotencyExtensions.cs	Simplified execution flow and improved null result handling
CommandTests.cs	Added tests for timestamp validation, GUID version checking, and derived type handling
CommandIdempotencyTests.cs	Enhanced test coverage for concurrent execution, null caching, and batch operations
OrleansCommandIdempotencyStore.cs	Fixed result handling in SetCommandStatusAsync and improved type casting
CommandIdempotencyGrain.cs	Enhanced retry logic to allow execution from completed/failed states
CommandIdempotencyExtensions.cs (AspNetCore)	Applied same execution flow improvements as main extensions

README.md

Copilot · 2025-10-11T11:48:53Z

README.md

+| `TraceId` | Connects to distributed tracing spans | OpenTelemetry/`Activity` context | The library stores, but never generate
+s, trace identifiers. |


Line break occurred mid-word in "generates". The word should be complete on line 593.

Suggested change

| `TraceId` | Connects to distributed tracing spans | OpenTelemetry/`Activity` context | The library stores, but never generate

s, trace identifiers. |

| `TraceId` | Connects to distributed tracing spans | OpenTelemetry/`Activity` context | The library stores, but never generates, trace identifiers. |

Copilot · 2025-10-11T11:48:56Z

README.md

+- **Wait semantics rely on polling.** Adaptive polling keeps responsiveness reasonable, but distributed stores can swap in push-
+style notifications if tail latency becomes critical.


Line break occurred mid-word in "push-style". The word should be complete on line 643.

Suggested change

- **Wait semantics rely on polling.** Adaptive polling keeps responsiveness reasonable, but distributed stores can swap in push-

style notifications if tail latency becomes critical.

- **Wait semantics rely on polling.** Adaptive polling keeps responsiveness reasonable, but distributed stores can swap in push-style notifications if tail latency becomes critical.

Copilot · 2025-10-11T11:48:57Z

README.md

+- **Status retention policies.** The memory store’s cleanup removes status and result after a TTL; other implementations must pr
+ovide similar hygiene to avoid unbounded growth while keeping enough history for retries.


Line break occurred mid-word in "provide". The word should be complete on line 645.

Suggested change

- **Status retention policies.** The memory store’s cleanup removes status and result after a TTL; other implementations must pr

ovide similar hygiene to avoid unbounded growth while keeping enough history for retries.

- **Status retention policies.** The memory store’s cleanup removes status and result after a TTL; other implementations must provide similar hygiene to avoid unbounded growth while keeping enough history for retries.

Copilot · 2025-10-11T11:48:57Z

README.md

+2. Consider extending the store contract with a boolean flag (or sentinel wrapper) that differentiates cached `default` values f
+rom missing entries.


Line break occurred mid-word in "from". The word should be complete on line 650.

Suggested change

2. Consider extending the store contract with a boolean flag (or sentinel wrapper) that differentiates cached `default` values f

rom missing entries.

2. Consider extending the store contract with a boolean flag (or sentinel wrapper) that differentiates cached `default` values from missing entries.

Copilot · 2025-10-11T11:48:57Z

README.md

+3. Monitor lock-pool growth in long-lived applications and log keys that never release to diagnose misbehaving callers before me
+mory pressure builds up.
+


Line break occurred mid-word in "memory". The word should be complete on line 652.

Suggested change

3. Monitor lock-pool growth in long-lived applications and log keys that never release to diagnose misbehaving callers before me

mory pressure builds up.

3. Monitor lock-pool growth in long-lived applications and log keys that never release to diagnose misbehaving callers before memory pressure builds up.

Copilot · 2025-10-11T11:48:57Z

ManagedCode.Communication/Commands/Stores/MemoryCacheCommandIdempotencyStore.cs

-            await SetCommandStatusAsync(commandId, newStatus, cancellationToken);
-
-            return (currentStatus, true);
+            _commandLocks.TryRemove(new KeyValuePair<string, CommandLock>(commandId, commandLock));


Using KeyValuePair for TryRemove is less efficient than the overload that takes a key and value separately. Consider using _commandLocks.TryRemove(commandId, commandLock) instead.

Suggested change

_commandLocks.TryRemove(new KeyValuePair<string, CommandLock>(commandId, commandLock));

_commandLocks.TryRemove(commandId, commandLock);

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

)

* Guard completed Orleans commands from retries * Handle disposed service provider when resolving CommunicationLogger (#68) * Add grain-side status compare-and-set helper (#70) * Add grain-side status compare-and-set helper * Update ManagedCode.Communication.Orleans/Grains/CommandIdempotencyGrain.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Inline idempotency docs into README

7359fae

Copilot AI review requested due to automatic review settings October 11, 2025 11:45

KSemenenko added the codex label Oct 11, 2025 — with ChatGPT Codex Connector

chatgpt-codex-connector bot reviewed Oct 11, 2025

View reviewed changes

Copilot AI reviewed Oct 11, 2025

View reviewed changes

KSemenenko and others added 4 commits October 11, 2025 15:28

Update README.md

52c6f9d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

a079405

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

1aa9a51

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update README.md

2a69bea

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

KSemenenko closed this Oct 11, 2025

KSemenenko added a commit that referenced this pull request Oct 11, 2025

Handle disposed service provider when resolving CommunicationLogger (#68

9eaa9c9

)

KSemenenko deleted the codex/analyze-project-for-idempotent-command-support branch November 12, 2025 08:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Inline idempotency docs into README #68

Inline idempotency docs into README #68

Uh oh!

KSemenenko commented Oct 11, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Oct 11, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Copilot AI Oct 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		\| `TraceId` \| Connects to distributed tracing spans \| OpenTelemetry/`Activity` context \| The library stores, but never generate
		s, trace identifiers. \|

		- Wait semantics rely on polling. Adaptive polling keeps responsiveness reasonable, but distributed stores can swap in push-
		style notifications if tail latency becomes critical.

		- Status retention policies. The memory store’s cleanup removes status and result after a TTL; other implementations must pr
		ovide similar hygiene to avoid unbounded growth while keeping enough history for retries.

		2. Consider extending the store contract with a boolean flag (or sentinel wrapper) that differentiates cached `default` values f
		rom missing entries.

		3. Monitor lock-pool growth in long-lived applications and log keys that never release to diagnose misbehaving callers before me
		mory pressure builds up.

	_commandLocks.TryRemove(new KeyValuePair<string, CommandLock>(commandId, commandLock));
	_commandLocks.TryRemove(commandId, commandLock);

Inline idempotency docs into README #68

Inline idempotency docs into README #68

Uh oh!

Conversation

KSemenenko commented Oct 11, 2025

Summary

Testing

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants