Skip to content

[concurrency] Concurrency Safety Issue in create_agent_session - Module-Level Shared State Causes Temp File Collision #28686

@github-actions

Description

@github-actions

Summary

The create_agent_session tool maintains module-level mutable state (_allResults) that is shared across all invocations within the same Node.js process. When the MCP server processes concurrent tool calls, taskIndex is computed from _allResults.length before results are appended, meaning two interleaved async calls can derive the same index and write to the same temporary file — clobbering each other's task description.

Issue Details

Type: Shared Mutable State / Temporary File Naming Race
Location: actions/setup/js/create_agent_session.cjs:19-98

Code Pattern:

// Line 19 — module-level mutable state, reset once in main() and shared by all handleMessage calls
let _allResults = [];

// Lines 96-98 — taskIndex derived from shared array length BEFORE the push
const taskIndex = _allResults.length + 1;
const taskFile = path.join(tmpDir, `agent-task-description-\$\{taskIndex}.md`);
fs.writeFileSync(taskFile, taskDescription, "utf8");

// Line 117 — async yield — another handleMessage call can run here
taskOutput = await exec.getExecOutput("gh", ghArgs, { ... });

// Lines 152/156 — push happens AFTER the await, so _allResults.length is stale
_allResults.push({ number: taskNumber, url: output, success: true });

Race Condition Scenario:

  1. Call A enters handleMessage, reads _allResults.length = 0taskIndex = 1, writes agent-task-description-1.md
  2. Call A hits await exec.getExecOutput(...) and yields control
  3. Call B enters handleMessage, reads _allResults.length = 0 (still, because Call A has not pushed yet) → taskIndex = 1, overwrites agent-task-description-1.md with Call B's description
  4. Call A resumes and calls gh agent-task create --from-file agent-task-description-1.md — but the file now contains Call B's task description
  5. Result: Call A creates an agent session with the wrong task description
Detailed Analysis

Root Cause

Node.js is single-threaded but uses cooperative multitasking via the event loop. Synchronous code runs atomically, but await yields execution to other pending callbacks. The sequence between reading _allResults.length and pushing to _allResults spans an await exec.getExecOutput(...) call — creating a window for interleaving.

Concurrent Execution Timeline

T=0ms: Call A reads _allResults.length=0, computes taskIndex=1
T=1ms: Call A writes agent-task-description-1.md  (Call A's description)
T=2ms: Call A awaits exec.getExecOutput [YIELDS]
T=3ms: Call B reads _allResults.length=0 (no push yet!), computes taskIndex=1
T=4ms: Call B writes agent-task-description-1.md  ← OVERWRITES Call A's file
T=5ms: Call B awaits exec.getExecOutput [YIELDS]
T=6ms: Call A resumes, executes gh with agent-task-description-1.md ← WRONG CONTENT

Impact Assessment

  • Data Integrity: Agent sessions created with wrong task descriptions silently
  • Reliability: Non-deterministic — only triggered if the MCP server handles concurrent create_agent_session calls
  • Security: Low — task descriptions remain within the same workflow run's inputs

Recommended Fix

Approach: Use a counter that is incremented atomically at the point of use, or use a UUID / timestamp for temp file naming to avoid collisions entirely.

// ✅ SAFE Option 1: Use timestamp + random suffix for unique temp filenames
const taskFile = path.join(tmpDir, `agent-task-description-\$\{Date.now()}-\$\{Math.random().toString(36).slice(2)}.md`);

// ✅ SAFE Option 2: Capture length synchronously as part of the push
const taskIndex = _allResults.length + 1;
_allResults.push(null); // Reserve the slot immediately (synchronous)
const taskFile = path.join(tmpDir, `agent-task-description-\$\{taskIndex}.md`);
fs.writeFileSync(taskFile, taskDescription, "utf8");
// ... then replace the reserved slot after exec completes
_allResults[taskIndex - 1] = { number: taskNumber, url: output, success: true };

Implementation Steps:

  1. Change temp file naming in create_agent_session.cjs to use a unique identifier (timestamp + random or UUID)
  2. Apply the same fix to assign_to_agent.cjs which has an identical _allResults pattern (actions/setup/js/assign_to_agent.cjs:20)
  3. Run make fmt-cjs && make lint-cjs to validate
Additional Related Module-Level State

During analysis, two other files with similar module-level mutable state were noted for awareness (not necessarily bugs, but worth reviewing):

  • actions/setup/js/missing_messages_helper.cjs:14let collectedMissings = null (global singleton set via setCollectedMissings). If two concurrent workflow runs share a process, the last setCollectedMissings call wins.
  • actions/setup/js/effective_tokens.cjs:27let _parsedMultipliers = undefined (lazy cache). This is a read-through cache and is safe as long as the parsed value is stable (idempotent initialization).

Testing Strategy

// Test concurrent execution
describe('create_agent_session concurrency safety', () => {
  test('uses unique temp files for concurrent calls', async () => {
    // Mock exec.getExecOutput to capture the file path used
    const filesUsed = [];
    jest.spyOn(exec, 'getExecOutput').mockImplementation((cmd, args) => {
      const fileArg = args[args.indexOf('--from-file') + 1];
      filesUsed.push(fileArg);
      return Promise.resolve({ stdout: 'https://github.com/org/repo/issues/1', exitCode: 0 });
    });
    
    // Launch 3 concurrent calls
    const handleMessage = await main(config);
    await Promise.all([
      handleMessage({ body: 'Task A description' }),
      handleMessage({ body: 'Task B description' }),
      handleMessage({ body: 'Task C description' }),
    ]);
    
    // All files should be unique
    expect(new Set(filesUsed).size).toBe(3);
  });
});

Priority: P2-Medium
Effort: Small
Expected Impact: Prevents silent wrong-task-description bugs in concurrent MCP server usage

Generated by Daily MCP Tool Concurrency Analysis · ● 1.4M ·

  • expires on May 4, 2026, 10:02 AM UTC

Metadata

Metadata

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions