[concurrency] Concurrency Safety Issue in `create_issue`: Shared Mutable Closure State Enables Max-Count Bypass and Duplicate Parent Issues

[github-actions]

Analysis Date

2026-04-17

Tool

create_issue

File

actions/setup/js/create_issue.cjs

---

Summary

The create_issue handler factory (main()) creates a closure capturing several mutable variables — processedCount, temporaryIdMap, parentIssueCache, and createdIssues — that are shared across all concurrent invocations of the returned handleCreateIssue function. In the MCP server context, multiple agent tool calls can arrive and execute concurrently; their async execution interleaves at every await point. This creates two exploitable race conditions: (1) the max-count enforcement can be bypassed, and (2) duplicate parent group issues can be created.

Additionally, issuesToAssignCopilotGlobal is a module-level mutable array (never written in production but mutated synchronously in tests), which carries a low-severity risk if test infrastructure is reused across concurrent test runs.

---

Issue 1 — HIGH: processedCount Read-Modify-Write Race Bypasses Max Count

Location: actions/setup/js/create_issue.cjs:326 (read) and :573 (write)

Code Pattern:

// Line 326 — check
if (processedCount >= maxCount) {
  core.warning(`Skipping create_issue: max count of \$\{maxCount} reached`);
  return { success: false, error: `Max count of \$\{maxCount} reached` };
}

// ... many await operations in between (API calls, etc.) ...

// Line 573 — increment
processedCount++;
```

**Race Condition Scenario**:
1. Call A and Call B both arrive when `processedCount = 9`, `maxCount = 10`
2. Call A reads `processedCount = 9` → passes the guard
3. Before Call A reaches line 573, the event loop yields at an `await` (e.g., GitHub API call)
4. Call B reads `processedCount = 9` (still unchanged) → also passes the guard
5. Call A eventually increments: `processedCount = 10`
6. Call B eventually increments: `processedCount = 11`
7. **Result**: Two issues created when only one was permitted

<details>
<summary>Detailed Concurrent Execution Timeline</summary>

```
T=0ms    Call A: processedCount check → reads 9 (< 10, passes guard)
T=1ms    Call A: await githubClient.rest.issues.create(...)  ← yields to event loop
T=2ms    Call B: processedCount check → reads 9 (still 9!)  ← passes guard
T=3ms    Call B: await githubClient.rest.issues.create(...)  ← yields
T=100ms  Call A: resumes, processedCount++ → now 10
T=150ms  Call B: resumes, processedCount++ → now 11
         ❌ Two issues created, max of 10 enforced incorrectly

This is a classic TOCTOU (time-of-check/time-of-use) race in async JavaScript.

Recommended Fix:

Move the guard check and increment to be adjacent with no await in between, or use an atomic check-and-increment pattern:

// ✅ SAFE: Atomic check-and-increment (no await between check and write)
if (processedCount >= maxCount) {
  core.warning(`Skipping create_issue: max count of \$\{maxCount} reached`);
  return { success: false, error: `Max count of \$\{maxCount} reached` };
}
processedCount++;  // ← immediately increment, before any await

// ... rest of processing with no further processedCount reads ...

By incrementing processedCount immediately after the guard (before any await), subsequent synchronous code in the same tick sees the updated value. Since JavaScript is single-threaded and the increment is synchronous, no interleaving is possible between the check and the increment.

---

Issue 2 — MEDIUM: parentIssueCache Race Causes Duplicate Parent Group Issues

Location: actions/setup/js/create_issue.cjs:679–699

Code Pattern:

// Line 679 — check cache
let groupParentNumber = parentIssueCache.get(groupId);

if (!groupParentNumber) {
  // Not in cache — multiple concurrent calls all find cache empty!
  groupParentNumber = await findOrCreateParentIssue({ ... });  // ← async

  if (groupParentNumber) {
    parentIssueCache.set(groupId, groupParentNumber);  // ← written too late
  }
}

Race Condition Scenario:

1. Calls A, B, and C all arrive for the same groupId before any parent exists
2. All three read parentIssueCache.get(groupId) → undefined
3. All three call findOrCreateParentIssue() (async, yields to event loop)
4. Each creates a new parent issue independently
5. Result: Up to 3 duplicate parent group issues created

Recommended Fix

Use an in-flight promise cache to prevent concurrent calls from duplicating work:

// ✅ SAFE: In-flight promise deduplication
// Add to closure: const parentIssuePending = new Map();

let groupParentNumber = parentIssueCache.get(groupId);
if (!groupParentNumber) {
  // Check if there's already an in-flight request for this group
  if (!parentIssuePending.has(groupId)) {
    // No in-flight request — start one and cache the promise
    const promise = findOrCreateParentIssue({ ... })
      .then(num => {
        if (num) parentIssueCache.set(groupId, num);
        parentIssuePending.delete(groupId);
        return num;
      });
    parentIssuePending.set(groupId, promise);
  }
  // Await the shared in-flight promise (all concurrent callers share it)
  groupParentNumber = await parentIssuePending.get(groupId);
}

This ensures only one findOrCreateParentIssue call is in flight per groupId at any time.

---

Issue 3 — LOW: Module-Level Mutable Array issuesToAssignCopilotGlobal

Location: actions/setup/js/create_issue.cjs:13

Code Pattern:

// Line 13
let issuesToAssignCopilotGlobal = [];

The module comment acknowledges this is dead code in production (never populated) but it is a module-level mutable let. The in-place mutation issuesToAssignCopilotGlobal.length = 0 in resetIssuesToAssignCopilot() is safe in Node.js single-threaded context. Severity is low because it's never written during production operation.

Recommended Fix (optional cleanup):

If truly dead code, remove it. If needed for tests, document clearly and consider using const with array replacement via a wrapper:

// ✅ Option A: Remove entirely (preferred if truly unused)
// (delete lines 13–32 and the exports)

// ✅ Option B: Keep for tests but use const + module ref
const state = { issuesToAssignCopilot: [] };
function getIssuesToAssignCopilot() { return [...state.issuesToAssignCopilot]; }
function resetIssuesToAssignCopilot() { state.issuesToAssignCopilot = []; }

---

Testing Strategy

To verify the fix for Issue 1:

describe('create_issue max-count concurrency safety', () => {
  test('concurrent calls respect maxCount=1 limit', async () => {
    // Arrange: handler with maxCount=1, processedCount starts at 0
    const handler = await main({ max: 1, /* ...other config */ });
    
    // Act: launch 5 concurrent calls simultaneously
    const results = await Promise.all(
      Array(5).fill(0).map(() => handler({ title: 'Test', body: 'body' }, {}))
    );
    
    // Assert: exactly 1 success, 4 failures
    const successes = results.filter(r => r.success);
    const failures = results.filter(r => !r.success);
    expect(successes).toHaveLength(1);
    expect(failures).toHaveLength(4);
  });
});

---

Implementation Steps

1. In create_issue.cjs, move processedCount++ to immediately follow the guard check (before any await) to make the check-and-increment atomic within the JS event loop
2. Add an parentIssuePending in-flight promise map to the main() closure alongside parentIssueCache to deduplicate concurrent findOrCreateParentIssue calls
3. Consider removing issuesToAssignCopilotGlobal if it is confirmed dead code
4. Add concurrency regression tests for processedCount enforcement

---

Priority: P1-High
Effort: Small
Expected Impact: Prevents max-count bypass and duplicate parent group issues under concurrent MCP tool call load

References:

• §24558590338

Generated by Daily MCP Tool Concurrency Analysis · ● 1.7M · ◷

• expires on Apr 24, 2026, 10:01 AM UTC

[github-actions]

🍪 Issue Monster has assigned this to Copilot!

I've identified this issue as a good candidate for automated resolution and assigned it to the Copilot coding agent.

The Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster · ● 281.9K · ◷