Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 113 additions & 0 deletions backend/__tests__/service/migrate-task-source-ref-identity.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
process.env.PG_HOST = '';

const Task = require('../../models/Task');
const {
migrateTaskSourceRefIdentity,
} = require('../../scripts/migrate-task-source-ref-identity');
const {
setupMongoDb,
closeMongoDb,
clearMongoDb,
} = require('../utils/testUtils');

const LEGACY_INDEX = 'podId_1_sourceRef_1_partial';
const PAIR_INDEX = 'podId_1_sourceRef_1_title_1_partial';

// TASK-063's migration is load-bearing, not housekeeping: autoIndex CREATES the
// declared pair index but never DROPS the legacy ref-only index, and the legacy
// index is STRICTER — while it exists, a second ask under one sourceRef cannot
// be inserted at all (the route answers that with a named 503). So this
// exercises the drop/create against a real mongod rather than trusting the
// script's shape.
describe('migrate-task-source-ref-identity', () => {
beforeAll(async () => {
await setupMongoDb();
});

afterAll(async () => {
await clearMongoDb();
await closeMongoDb();
});

const indexNames = async () => {
try {
return (await Task.collection.indexes()).map((i) => String(i.name));
} catch (error) {
// A database that has never held a task has no `tasks` collection to list
// indexes from, and mongod answers NamespaceNotFound (code 26).
if (error.code === 26) return [];
throw error;
}
};

const seedLegacyIndex = async () => {
// Reproduce a pre-TASK-063 database: the declared pair index absent, the
// ref-only index present.
const names = await indexNames();
if (names.includes(PAIR_INDEX)) await Task.collection.dropIndex(PAIR_INDEX);
if (!names.includes(LEGACY_INDEX)) {
await Task.collection.createIndex(
{ podId: 1, sourceRef: 1 },
{ unique: true, name: LEGACY_INDEX, partialFilterExpression: { sourceRef: { $type: 'string' } } },
);
}
};

beforeEach(async () => {
await clearMongoDb();
await seedLegacyIndex();
});

it('reports the legacy index and changes nothing on a dry run', async () => {
const result = await migrateTaskSourceRefIdentity({ dryRun: true });

expect(result).toMatchObject({
dryRun: true,
legacyIndexPresent: true,
pairIndexPresentBefore: false,
pairIndexPresentAfter: false,
droppedLegacy: false,
});
const names = await indexNames();
expect(names).toContain(LEGACY_INDEX);
expect(names).not.toContain(PAIR_INDEX);
});

it('drops the legacy index and creates the pair index', async () => {
const result = await migrateTaskSourceRefIdentity();

expect(result).toMatchObject({
dryRun: false,
legacyIndexPresent: true,
pairIndexPresentBefore: false,
pairIndexPresentAfter: true,
droppedLegacy: true,
});
const names = await indexNames();
expect(names).not.toContain(LEGACY_INDEX);
expect(names).toContain(PAIR_INDEX);
});

it('is idempotent, and the pair index is what a second ask needs', async () => {
await migrateTaskSourceRefIdentity();
const second = await migrateTaskSourceRefIdentity();
expect(second).toMatchObject({
legacyIndexPresent: false,
pairIndexPresentBefore: true,
pairIndexPresentAfter: true,
droppedLegacy: false,
});

const podId = new (require('mongoose').Types.ObjectId)();
const base = {
podId,
source: 'import',
sourceRef: 'external:ticket:697',
updates: [],
};
await Task.create({ ...base, taskNum: 1, taskId: 'TASK-001', title: 'First ask' });
await Task.create({ ...base, taskNum: 2, taskId: 'TASK-002', title: 'Second ask' });

expect(await Task.countDocuments({ podId, sourceRef: 'external:ticket:697' })).toBe(2);
});
});
135 changes: 126 additions & 9 deletions backend/__tests__/service/tasks.source-ref-idempotency.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ const {
generateTestToken,
} = require('../utils/testUtils');

// TASK-063. A create's identity is the (sourceRef, title) PAIR. Everything in
// this file follows from that one sentence: the same pair is an idempotent
// retry (200, nothing mutated), a different title under the same ref is a
// different ask (201, its own row), and a completed row can only be reopened by
// the ask that completed it.
describe('POST /api/v1/tasks/:podId sourceRef idempotency', () => {
let app;
let owner;
Expand Down Expand Up @@ -75,59 +80,146 @@ describe('POST /api/v1/tasks/:podId sourceRef idempotency', () => {
const existing = await seedTask();

const response = await postTask({
title: 'Retry of existing task',
title: 'Existing task',
sourceRef: 'external:ticket:697',
}).expect(200);

expect(response.body.alreadyExists).toBe(true);
expect(response.body.reopened).toBeUndefined();
expect(response.body.task._id).toBe(String(existing._id));
expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'external:ticket:697' })).toBe(1);
});

it('creates a second row when one sourceRef carries a different ask', async () => {
// The 58290 incident: one message raised two asks with two owners, the
// filer used the message id as the ref for both, and the second ask came
// back holding the first ask's row with its own title discarded.
const first = await seedTask();

const response = await postTask({
title: 'Second ask from the same source',
sourceRef: 'external:ticket:697',
assignee: 'pod-architect',
}).expect(201);

expect(response.body.task._id).not.toBe(String(first._id));
expect(response.body.task.title).toBe('Second ask from the same source');
expect(response.body.task.assignee).toBe('pod-architect');
expect(response.body.alreadyExists).toBeUndefined();
expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'external:ticket:697' })).toBe(2);

const reloadedFirst = await Task.findById(first._id);
expect(reloadedFirst.title).toBe('Existing task');
expect(reloadedFirst.status).toBe('pending');
});

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-active source record',
title: 'Existing task',
sourceRef: 'external:ticket:697',
assignee: 'codex',
}).expect(200);

// `alreadyExists: true` because it does: this request did not create a row,
// it reopened one. It used to answer `false`, and a caller branches on that
// field to decide whether its create landed, so the caller concluded it had
// created a task while holding a different task's row.
expect(response.body).toMatchObject({
alreadyExists: false,
alreadyExists: true,
reopened: true,
task: {
_id: String(existing._id),
status: 'pending',
assignee: 'codex',
title: 'Existing task',
},
});
expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'external:ticket:697' })).toBe(1);
});

it('preserves the completed run\'s notes in history and reports a differing submitted title', async () => {
it('clears the assignee when a same-pair reopen is submitted without one', async () => {
// The gap sprint-review measured (73815, 2026-09-25): every other reopen
// case passes an assignee, so the half the tool description warns about was
// documented in three places and pinned nowhere. `existing.assignee =
// assignee || undefined` (tasksApi.ts:290) means an OMITTED assignee clears
// the row's — a later change that made reopen preserve it would leave the
// tool description, task-board.mdx and guides.json all wrong with the rest
// of this file green.
const existing = await seedTask({
status: 'done',
completedAt: new Date(),
notes: 'Shipped as PR #1, squash-merged. Patch-id verified against the gated head.',
assignee: 'ux-lead',
});
// Positive control: the field we assert cleared is genuinely set before the
// request, so a pass cannot come from the seed never carrying it.
expect((await Task.findById(existing._id)).assignee).toBe('ux-lead');

const response = await postTask({
title: 'Existing task',
sourceRef: 'external:ticket:697',
}).expect(200);

expect(response.body).toMatchObject({ alreadyExists: true, reopened: true });
// The route assigns `undefined`, Mongoose `$unset`s the path, and hydration
// hands the String path back as its null default — so the two readers
// disagree on the sentinel (undefined in the response's in-memory doc, null
// on reload) while agreeing on the fact. Assert the fact, both ways round:
// the old assignee is gone, on the response AND in the store.
const reloaded = await Task.findById(existing._id);
expect([null, undefined]).toContain(response.body.task.assignee);
expect([null, undefined]).toContain(reloaded.assignee);
});

it('does not reopen a completed row when the submitted title differs', async () => {
// TASK-163, reproduced live 2026-09-25T08:09Z: a create with a done row's
// ref put that row back to pending under a title its caller never chose,
// and the caller's own ask was never filed.
const completed = await seedTask({
status: 'done',
completedAt: new Date(),
notes: 'Shipped as PR #1169, squash-merged. Patch-id verified against the gated head.',
});
const updatesBefore = (await Task.findById(completed._id)).updates.length;

const response = await postTask({
title: 'A follow-up filed against the same source',
sourceRef: 'external:ticket:697',
}).expect(201);

expect(response.body.task.title).toBe('A follow-up filed against the same source');
expect(response.body.task._id).not.toBe(String(completed._id));

const reloaded = await Task.findById(completed._id);
expect(reloaded.status).toBe('done');
expect(reloaded.notes).toBe('Shipped as PR #1169, squash-merged. Patch-id verified against the gated head.');
expect(reloaded.updates.length).toBe(updatesBefore);
expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'external:ticket:697' })).toBe(2);
});

it('preserves the completed run\'s notes in history when a reopen does fire', async () => {
const existing = await seedTask({
status: 'done',
completedAt: new Date(),
notes: 'Shipped as PR #1, squash-merged. Patch-id verified against the gated head.',
});

const response = await postTask({
title: 'Existing task',
sourceRef: 'external:ticket:697',
}).expect(200);

expect(response.body.reopened).toBe(true);
expect(response.body.task.title).toBe('Existing task');
expect(response.body.task.notes).toContain('Submitted title ("A follow-up filed against the same source") was NOT applied');
expect(response.body.task.notes).toBe('Reopened — the same source is active again.');

const reloaded = await Task.findById(existing._id);
const history = reloaded.updates.map((u) => u.text).join('\n---\n');
// The previous notes are the durable record of the completed run; a reopen
// used to destroy them by overwriting `notes` with one sentence.
expect(history).toContain('Previous notes preserved from the completed run');
expect(history).toContain('Patch-id verified against the gated head.');
expect(history).toContain('Submitted title differed and was not applied');
});

it('adds no conservation noise when the reopen changes nothing but the status', async () => {
Expand All @@ -143,16 +235,18 @@ describe('POST /api/v1/tasks/:podId sourceRef idempotency', () => {
const reloaded = await Task.findById(existing._id);
const history = reloaded.updates.map((u) => u.text).join('\n---\n');
expect(history).not.toContain('Previous notes preserved');
expect(history).not.toContain('Submitted title differed');
});

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));

// Same ref AND same title: the insert really does collide on the pair
// index, so this exercises the E11000 classification against a live index
// rather than a mock of one.
const response = await postTask({
title: 'Concurrent retry',
title: 'Existing task',
sourceRef: 'external:ticket:697',
}).expect(200);

Expand All @@ -161,6 +255,29 @@ describe('POST /api/v1/tasks/:podId sourceRef idempotency', () => {
expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'external:ticket:697' })).toBe(1);
});

it('answers a named 503 when the database still carries the ref-only index', async () => {
// Deploy-order guard: with the pre-TASK-063 index still in place, a second
// title under one ref collides there, so the create the caller asked for is
// not one this database can perform. Fail with the migration's name rather
// than a generic 500.
await seedTask({ status: 'done', completedAt: new Date() });
jest.spyOn(Task, 'findOne').mockImplementationOnce(() => Promise.resolve(null));
jest.spyOn(Task, 'create').mockRejectedValueOnce(Object.assign(
new Error('E11000 duplicate key error collection: commonly.tasks index: podId_1_sourceRef_1_partial dup key'),
{ code: 11000, keyPattern: { podId: 1, sourceRef: 1 } },
));
jest.spyOn(console, 'error').mockImplementation(() => {});

const response = await postTask({
title: 'A second ask, on a legacy index',
sourceRef: 'external:ticket:697',
}).expect(503);

expect(response.body.code).toBe('task_source_ref_index_migration_pending');
expect(response.body.error).toContain('migrate:task-source-ref-identity');
expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'external:ticket:697' })).toBe(1);
});

it('does not misclassify a taskId E11000 as sourceRef idempotency', async () => {
await seedTask({ sourceRef: 'external:ticket:different-source' });
const findOneSpy = jest.spyOn(Task, 'findOne');
Expand Down
40 changes: 40 additions & 0 deletions backend/__tests__/unit/models/Task.sourceRefIndex.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
const Task = require('../../../models/Task');

// TASK-063. `sourceRef` is PROVENANCE, not identity: one source — a message, a
// PR — can raise several asks with several owners. The unique key is therefore
// the (sourceRef, title) PAIR. The ref-only index that preceded it made the
// second ask adopt the first ask's row: it discarded the caller's title and
// reopened whatever row that ref already belonged to, twice in production
// (TASK-052, TASK-163), the second time on a row a merged PR had completed.
//
// This asserts the DECLARATION, because the declaration is the whole guarantee.
// No runtime branch can notice if someone relaxes the key back to sourceRef
// alone — the route's pre-check and the index would silently agree with each
// other again, which is exactly the state that produced the incident. The
// partial filter is pinned because `sparse` on a compound index indexes every
// doc that has podId (i.e. all of them) and would E11000 the second
// sourceRef-less task in a pod; the name is pinned because
// scripts/migrate-task-source-ref-identity.ts drops the legacy index by name.
describe('Task model — sourceRef identity index', () => {
test('uniquely keys (podId, sourceRef, title) in that order', () => {
const pairIndex = Task.schema.indexes().find(([fields]) => (
fields.podId === 1 && fields.sourceRef === 1 && fields.title === 1
));

expect(pairIndex).toBeDefined();
expect(Object.keys(pairIndex[0])).toEqual(['podId', 'sourceRef', 'title']);
expect(pairIndex[1]).toMatchObject({
unique: true,
name: 'podId_1_sourceRef_1_title_1_partial',
partialFilterExpression: { sourceRef: { $type: 'string' } },
});
});

test('declares no ref-only unique index', () => {
const refOnly = Task.schema.indexes().find(([fields]) => (
fields.sourceRef === 1 && fields.title === undefined
));

expect(refOnly).toBeUndefined();
});
});
7 changes: 4 additions & 3 deletions backend/controllers/authController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,10 @@ const finishWorkspaceOnboarding = async (pod: any, userId: any) => {
notes: 'Workspaces are better shared — humans and agents in the same room, one project memory. Use the pod invite link from the inspector panel.',
},
];
// Distinct sourceRefs give each seed a stable identity under the
// unique (podId, sourceRef) partial index — re-running the seeding
// for a pod can never silently duplicate the checklist.
// Distinct sourceRefs — each with a fixed title — give every seed a
// stable identity under the unique (podId, sourceRef, title) partial
// index, so re-running the seeding for a pod cannot silently duplicate
// the checklist.
await Task.create(starter.map((t, i) => ({
podId: pod._id,
taskNum: i + 1,
Expand Down
Loading
Loading