fix(tasks): make duplicate sourceRef creates idempotent

backend/__tests__/service/tasks.source-ref-idempotency.test.js

@@ -0,0 +1,162 @@
+process.env.PG_HOST = '';
+process.env.JWT_SECRET = 'tasks-source-ref-idempotency-test-secret';
+
+const express = require('express');
+const request = require('supertest');
+const Pod = require('../../models/Pod');
+const Task = require('../../models/Task');
+const User = require('../../models/User');
+const tasksApiRoutes = require('../../routes/tasksApi');
+const {
+  setupMongoDb,
+  closeMongoDb,
+  clearMongoDb,
+  generateTestToken,
+} = require('../utils/testUtils');
+
+describe('POST /api/v1/tasks/:podId sourceRef idempotency', () => {
+  let app;
+  let owner;
+  let pod;
+  let token;
+
+  beforeAll(async () => {
+    await setupMongoDb();
+    await Task.init();
+    app = express();
+    app.use(express.json());
+    app.use('/api/v1/tasks', tasksApiRoutes);
+  });
+
+  beforeEach(async () => {
+    await clearMongoDb();
+    owner = await User.create({
+      username: `task-owner-${Date.now()}`,
+      email: `task-owner-${Date.now()}@test.com`,
+      password: 'Password123!',
+      verified: true,
+    });
+    pod = await Pod.create({
+      name: 'Task idempotency pod',
+      type: 'team',
+      createdBy: owner._id,
+      members: [owner._id],
+    });
+    token = generateTestToken(owner._id);
+  });
+
+  afterAll(async () => {
+    await clearMongoDb();
+    await closeMongoDb();
+  });
+
+  const postTask = (body) => request(app)
+    .post(`/api/v1/tasks/${pod._id}`)
+    .set('Authorization', `Bearer ${token}`)
+    .send(body);
+
+  const seedTask = (overrides = {}) => Task.create({
+    podId: pod._id,
+    taskNum: 1,
+    taskId: 'TASK-001',
+    title: 'Existing task',
+    source: 'github',
+    sourceRef: 'GH#697',
+    updates: [{
+      text: 'Seeded for test',
+      author: 'system',
+      authorId: null,
+      createdAt: new Date(),
+    }],
+    ...overrides,
+  });
+
+  it('returns an existing pending task through the pre-check', async () => {
+    const existing = await seedTask();
+
+    const response = await postTask({
+      title: 'Retry of existing task',
+      sourceRef: 'GH#697',
+    }).expect(200);
+
+    expect(response.body.alreadyExists).toBe(true);
+    expect(response.body.task._id).toBe(String(existing._id));
+    expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'GH#697' })).toBe(1);
+  });
+
+  it('keeps the pre-check reopen behavior for a completed task', async () => {
+    const existing = await seedTask({ status: 'done', completedAt: new Date() });
+
+    const response = await postTask({
+      title: 'Still-open source issue',
+      sourceRef: 'GH#697',
+      assignee: 'codex',
+    }).expect(200);
+
+    expect(response.body).toMatchObject({
+      alreadyExists: false,
+      reopened: true,
+      task: {
+        _id: String(existing._id),
+        status: 'pending',
+        assignee: 'codex',
+      },
+    });
+    expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'GH#697' })).toBe(1);
+  });
+
+  it('reconciles a sourceRef E11000 race as an idempotent 200', async () => {
+    const existing = await seedTask();
+    const findOneSpy = jest.spyOn(Task, 'findOne');
+    findOneSpy.mockImplementationOnce(() => Promise.resolve(null));
+
+    const response = await postTask({
+      title: 'Concurrent retry',
+      sourceRef: 'GH#697',
+    }).expect(200);
+
+    expect(response.body.alreadyExists).toBe(true);
+    expect(response.body.task._id).toBe(String(existing._id));
+    expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'GH#697' })).toBe(1);
+  });
+
+  it('does not misclassify a taskId E11000 as sourceRef idempotency', async () => {
+    await seedTask({ sourceRef: 'GH#different-source' });
+    const findOneSpy = jest.spyOn(Task, 'findOne');
+    // The sourceRef pre-check legitimately finds no match. Then force the
+    // nextTaskId read to miss the existing TASK-001 so Mongo raises E11000
+    // from the *taskId* index while the request still carries a sourceRef.
+    findOneSpy.mockImplementationOnce(() => Promise.resolve(null));
+    findOneSpy.mockImplementationOnce(() => ({
+      sort: () => ({
+        select: () => ({
+          lean: () => Promise.resolve(null),
+        }),
+      }),
+    }));
+    jest.spyOn(console, 'error').mockImplementation(() => {});
+
+    const response = await postTask({
+      title: 'Colliding task number',
+      sourceRef: 'GH#697',
+    }).expect(500);
+
+    expect(response.body.alreadyExists).toBeUndefined();
+    expect(response.body.error).toBe('Failed to create task');
+    expect(await Task.countDocuments({ podId: pod._id, taskId: 'TASK-001' })).toBe(1);
+    expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'GH#697' })).toBe(0);
+  });
+
+  it('rejects an operator-shaped sourceRef before it reaches Mongo', async () => {
+    const findOneSpy = jest.spyOn(Task, 'findOne');
+
+    const response = await postTask({
+      title: 'Attempted query injection',
+      sourceRef: { $ne: null },
+    }).expect(400);
+
+    expect(response.body.error).toBe('sourceRef must be a string');
+    expect(findOneSpy).not.toHaveBeenCalled();
+    expect(await Task.countDocuments({ podId: pod._id })).toBe(0);
+  });
+});

backend/routes/tasksApi.ts

@@ -1,3 +1,6 @@
+import { createHash } from 'crypto';
+import type { Request, Response } from 'express';
+import rateLimit, { ipKeyGenerator } from 'express-rate-limit';
 // eslint-disable-next-line global-require
 const express = require('express');
 // eslint-disable-next-line global-require
@@ -39,6 +42,14 @@ function auth(req: AuthReq, res: Res, next: () => void) {
 
 const router: ReturnType<typeof express.Router> = express.Router();
 
+const taskWriteRateLimitKey = (req: Request): string => {
+  const authHeader = req.get('Authorization') || req.get('x-auth-token');
+  if (authHeader) {
+    return `tok:${createHash('sha256').update(authHeader).digest('hex').slice(0, 16)}`;
+  }
+  return req.ip ? ipKeyGenerator(req.ip) : 'anon';
+};
+
 async function resolveAuthor(req: AuthReq): Promise<string> {
   const agentInstance = req.user?.isBot ? (req.user.botMetadata?.instanceId || req.user.botMetadata?.agentName) : null;
   if (agentInstance) return agentInstance;
@@ -104,11 +115,67 @@ router.get('/:podId', auth, async (req: AuthReq, res: Res) => {
   }
 });
 
-router.post('/:podId', auth, async (req: AuthReq, res: Res) => {
+router.post('/:podId', rateLimit({
+  windowMs: 60_000,
+  max: 20,
+  standardHeaders: true,
+  legacyHeaders: false,
+  keyGenerator: taskWriteRateLimitKey,
+  handler: (_req: Request, res: Response) => res.status(429).json({ error: 'rate limit exceeded: 20 task creates per 60s' }),
+}), auth, async (req: AuthReq, res: Res) => {
   try {
     const { podId } = req.params || {};
     const userId = req.userId || req.user?._id || req.agentUser?._id;
-    const { title, assignee, dep, depMockOk, parentTask, source, sourceRef, githubIssueNumber, githubIssueUrl, createGithubIssue } = (req.body || {}) as { title?: string; assignee?: string; dep?: string; depMockOk?: boolean; parentTask?: string; source?: string; sourceRef?: string; githubIssueNumber?: number; githubIssueUrl?: string; createGithubIssue?: boolean };
+    const {
+      title: titleInput,
+      assignee: assigneeInput,
+      dep: depInput,
+      depMockOk: depMockOkInput,
+      parentTask: parentTaskInput,
+      source: sourceInput,
+      sourceRef: sourceRefInput,
+      githubIssueNumber: githubIssueNumberInput,
+      githubIssueUrl: githubIssueUrlInput,
+      createGithubIssue: createGithubIssueInput,
+    } = req.body || {};
+    const stringInputs: Record<string, unknown> = {
+      title: titleInput,
+      assignee: assigneeInput,
+      dep: depInput,
+      parentTask: parentTaskInput,
+      source: sourceInput,
+      sourceRef: sourceRefInput,
+      githubIssueUrl: githubIssueUrlInput,
+    };
+    const invalidStringField = Object.entries(stringInputs)
+      .find(([, value]) => value !== undefined && typeof value !== 'string');
+    if (invalidStringField) {
+      return res.status(400).json({ error: `${invalidStringField[0]} must be a string` });
+    }
+    if (githubIssueNumberInput !== undefined
+      && (typeof githubIssueNumberInput !== 'number'
+        || !Number.isSafeInteger(githubIssueNumberInput)
+        || githubIssueNumberInput < 1)) {
+      return res.status(400).json({ error: 'githubIssueNumber must be a positive integer' });
+    }
+    if (depMockOkInput !== undefined && typeof depMockOkInput !== 'boolean') {
+      return res.status(400).json({ error: 'depMockOk must be a boolean' });
+    }
+    if (createGithubIssueInput !== undefined && typeof createGithubIssueInput !== 'boolean') {
+      return res.status(400).json({ error: 'createGithubIssue must be a boolean' });
+    }
+    // Materialize primitives after runtime validation so Mongo never receives
+    // user-supplied query objects (for example, operator-shaped values).
+    const title = titleInput === undefined ? undefined : String(titleInput);
+    const assignee = assigneeInput === undefined ? undefined : String(assigneeInput);
+    const dep = depInput === undefined ? undefined : String(depInput);
+    const parentTask = parentTaskInput === undefined ? undefined : String(parentTaskInput);
+    const source = sourceInput === undefined ? undefined : String(sourceInput);
+    const sourceRef = sourceRefInput === undefined ? undefined : String(sourceRefInput);
+    const githubIssueUrl = githubIssueUrlInput === undefined ? undefined : String(githubIssueUrlInput);
+    const githubIssueNumber = githubIssueNumberInput === undefined ? undefined : Number(githubIssueNumberInput);
+    const depMockOk = depMockOkInput === true;
+    const createGithubIssue = createGithubIssueInput === true;
     if (!title) return res.status(400).json({ error: 'title is required' });
     const access = await requirePodMember(podId || '', userId, { write: true });
     if (access.error) return res.status(access.status || 500).json({ error: access.error });
@@ -151,7 +218,38 @@ router.post('/:podId', auth, async (req: AuthReq, res: Res) => {
     if (sourceRef) initUpdate.text = `Created by ${author} from ${sourceRef}${assignee ? ` · assigned to ${assignee}` : ''}`;
     if (ghNumber) initUpdate.text += ` · GH#${ghNumber}`;
     if (parentTask) initUpdate.text += ` · sub-task of ${parentTask}`;
-    const task = await Task.create({ podId, taskNum, taskId, title, assignee: assignee || null, dep: dep || null, depMockOk: !!depMockOk, parentTask: parentTask || null, source: source || (ghNumber ? 'github' : 'human'), sourceRef: sourceRef || (ghNumber ? `GH#${ghNumber}` : undefined), githubIssueNumber: ghNumber, githubIssueUrl: ghUrl, updates: [initUpdate] });
+    let task;
+    try {
+      task = await Task.create({ podId, taskNum, taskId, title, assignee: assignee || null, dep: dep || null, depMockOk: !!depMockOk, parentTask: parentTask || null, source: source || (ghNumber ? 'github' : 'human'), sourceRef: sourceRef || (ghNumber ? `GH#${ghNumber}` : undefined), githubIssueNumber: ghNumber, githubIssueUrl: ghUrl, updates: [initUpdate] });
+    } catch (createErr) {
+      const duplicate = createErr as {
+        code?: number;
+        keyPattern?: Record<string, number>;
+        message?: string;
+      };
+      const sourceRefIndexCollision = duplicate.code === 11000
+        && !!sourceRef
+        && (
+          (
+            duplicate.keyPattern?.podId === 1
+            && duplicate.keyPattern?.sourceRef === 1
+            && duplicate.keyPattern?.taskId === undefined
+          )
+          || duplicate.message?.includes('podId_1_sourceRef_1_partial')
+        );
+      if (!sourceRefIndexCollision) throw createErr;
+
+      // The pre-check can lose a race to another request. Re-read the winner
+      // only after Mongo identifies the sourceRef index as the collision.
+      // Do not reopen here: the concurrent winner has just created a fresh
+      // task, so this request is an idempotent replay of that creation.
+      const existing = await Task.findOne({
+        podId: mongoose.Types.ObjectId.createFromHexString(podId || ''),
+        sourceRef,
+      }) as { toObject: () => unknown } | null;
+      if (!existing) throw createErr;
+      return res.json({ task: existing.toObject(), alreadyExists: true });
+    }
     if (parentTask && GitHubAppService.isPatConfigured()) {
       try {
         const parent = await Task.findOne({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), taskId: parentTask }).lean() as { githubIssueNumber?: number } | null;