diff --git a/backend/__tests__/service/taskEventService.offeredAt.test.js b/backend/__tests__/service/taskEventService.offeredAt.test.js index d476c40e8b..f4f56a582c 100644 --- a/backend/__tests__/service/taskEventService.offeredAt.test.js +++ b/backend/__tests__/service/taskEventService.offeredAt.test.js @@ -48,7 +48,7 @@ const install = (agentName = 'scout') => ({ instanceId: 'default', podId: POD_ID.toString(), status: 'active', - config: { boardWake: { enabled: true } }, + config: { runtime: { runtimeType: 'wrapper' }, boardWake: { enabled: true } }, }); const setInstalls = (installs) => { @@ -268,7 +268,7 @@ describe('board wake assigned work', () => { expect(scout.payload.content).toContain('[Your assigned rows (pending/claimed; newest first):]'); expect(scout.payload.content).toContain('TASK-ASSIGN — TASK-ASSIGN [pending] — Lily updated: reassigned to scout'); expect(scout.payload.content).not.toContain('TASK-FOREIGN'); - expect(peer.payload.content).not.toContain('[Your assigned rows'); + expect(peer).toBeUndefined(); }); it('keeps the assigned list to five rows, newest first, after a folded change', async () => { @@ -319,8 +319,135 @@ describe('board wake assigned work', () => { expect(anvil.payload.content).toContain('TASK-INSTANCE — Instance Do it now [pending]'); expect(anvil.payload.content).not.toContain('Instance\nDo it now'); expect(anvil.payload.content).not.toContain('TASK-SCOUT'); - expect(scout.payload.content).toContain('TASK-SCOUT — TASK-SCOUT [pending]'); - expect(scout.payload.content).not.toContain('TASK-INSTANCE —'); + expect(scout).toBeUndefined(); + }); +}); + +describe('own-row board wake routing', () => { + const wakeNames = () => AgentEventService.enqueue.mock.calls.map(([event]) => event.agentName); + + beforeEach(() => setInstalls([install('scout'), install('sprint-impl')])); + + it('does not wake another seat for an unrelated row change', async () => { + const changed = await createTask({ taskId: 'TASK-OTHER', updatedAt: NOW }); + + await notifyPodAgents(POD_ID, changed, 'updated', { isAgent: false }); + + expect(wakeNames()).toEqual([]); + }); + + it('wakes the new assignee only', async () => { + const changed = await createTask({ taskId: 'TASK-ASSIGN-OWNER', updatedAt: NOW, assignee: 'scout' }); + + await notifyPodAgents(POD_ID, changed, 'updated', { isAgent: false }, { assignee: null }); + + expect(wakeNames()).toEqual(['scout']); + }); + + it('wakes the previous assignee when a row is unassigned', async () => { + const changed = await createTask({ taskId: 'TASK-UNASSIGN-OWNER', updatedAt: NOW }); + + await notifyPodAgents(POD_ID, changed, 'updated', { isAgent: false }, { assignee: 'scout' }); + + expect(wakeNames()).toEqual(['scout']); + }); + + it('wakes the row owner for a comment', async () => { + const changed = await createTask({ taskId: 'TASK-COMMENT-OWNER', updatedAt: NOW, assignee: 'scout' }); + await Task.updateOne({ _id: changed._id }, { + $push: { updates: { text: 'Peer commented', author: 'Peer', createdAt: NOW } }, + }); + const commented = await Task.findById(changed._id).lean(); + + await notifyPodAgents(POD_ID, commented, 'updated', { isAgent: false }); + + expect(wakeNames()).toEqual(['scout']); + expect(AgentEventService.enqueue.mock.calls[0][0].payload.content).toContain('Peer commented'); + }); + + it.each(['native', 'durable-object'])('sends a hosted %s owner a type its engine runs', async (engine) => { + setInstalls([ + { ...install('maya-agent'), config: { runtime: { runtimeType: 'hosted' }, hosted: { engine } } }, + install('scout'), + ]); + const changed = await createTask({ taskId: 'TASK-HOSTED', updatedAt: NOW, assignee: 'maya-agent' }); + + await notifyPodAgents(POD_ID, changed, 'updated', { isAgent: false }); + + expect(wakeNames()).toEqual(['maya-agent']); + expect(AgentEventService.enqueue.mock.calls[0][0].type).toBe('chat.mention'); + }); + + it('inherits hosted runtime for a mention-created pod projection', async () => { + setInstalls([{ ...install('maya-agent'), config: { heartbeat: { enabled: true }, autoJoinSource: 'mention-resolution' } }]); + AgentInstallation.findOne.mockReturnValueOnce({ sort: () => ({ lean: async () => ({ + ...install('maya-agent'), config: { runtime: { runtimeType: 'hosted' }, hosted: { engine: 'native' } }, + }) }) }); + const changed = await createTask({ taskId: 'TASK-HOSTED-PROJECTION', updatedAt: NOW, assignee: 'maya-agent' }); + + await notifyPodAgents(POD_ID, changed, 'updated', { isAgent: false }); + + expect(AgentInstallation.findOne.mock.calls[0][0]).toMatchObject({ + agentName: 'maya-agent', instanceId: 'default', status: 'active', + 'config.runtime.runtimeType': { $exists: true }, + }); + expect(AgentEventService.enqueue.mock.calls[0][0].type).toBe('chat.mention'); + }); + + it('keeps an owned wake runnable if a projection runtime lookup fails', async () => { + setInstalls([{ ...install('maya-agent'), config: { autoJoinSource: 'mention-resolution' } }]); + AgentInstallation.findOne.mockImplementationOnce(() => { throw new Error('runtime lookup unavailable'); }); + const changed = await createTask({ taskId: 'TASK-LOOKUP-FAIL', updatedAt: NOW, assignee: 'maya-agent' }); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + await notifyPodAgents(POD_ID, changed, 'updated', { isAgent: false }); + warn.mockRestore(); + + expect(AgentEventService.enqueue.mock.calls[0][0].type).toBe('chat.mention'); + }); + + it('promotes a pending hosted sweep wake when an own-row change folds into it', async () => { + setInstalls([{ ...install('maya-agent'), config: { runtime: { runtimeType: 'hosted' } } }]); + const changed = await createTask({ taskId: 'TASK-HOSTED-FOLD', updatedAt: NOW, assignee: 'maya-agent' }); + AgentEvent.findOneAndUpdate.mockResolvedValueOnce({ _id: 'pending-sweep', payload: { boardChanges: 1 } }); + + await notifyPodAgents(POD_ID, changed, 'updated', { isAgent: false }); + + expect(AgentEvent.findOneAndUpdate.mock.calls[0][1].$set.type).toBe('chat.mention'); + expect(AgentEvent.updateOne).toHaveBeenCalledTimes(1); + }); + + it('wakes the waiting row owner only when its exact dependency clears', async () => { + const prerequisite = await createTask({ taskId: 'TASK-PREREQUISITE', updatedAt: NOW, status: 'done' }); + const waiting = await createTask({ taskId: 'TASK-WAITING', updatedAt: NOW, assignee: 'scout' }); + await Task.updateOne({ _id: waiting._id }, { $set: { dep: 'TASK-PREREQUISITE' } }); + const unrelated = await createTask({ taskId: 'TASK-UNRELATED', updatedAt: NOW, assignee: 'sprint-impl' }); + await Task.updateOne({ _id: unrelated._id }, { $set: { dep: 'TASK-OTHER' } }); + + await notifyPodAgents(POD_ID, prerequisite, 'updated', { isAgent: false }, { status: 'claimed' }); + + expect(wakeNames()).toEqual(['scout']); + }); + + it('does not wake an agent for its own write', async () => { + const changed = await createTask({ taskId: 'TASK-SELF', updatedAt: NOW, assignee: 'scout' }); + + await notifyPodAgents(POD_ID, changed, 'updated', { + isAgent: true, agentName: 'scout', instanceId: 'default', + }); + + expect(wakeNames()).toEqual([]); + }); + + it('still folds a second own-row change into the pending board wake', async () => { + const changed = await createTask({ taskId: 'TASK-FOLD-OWNER', updatedAt: NOW, assignee: 'scout' }); + AgentEvent.findOneAndUpdate.mockResolvedValueOnce({ _id: 'pending-wake', payload: { boardChanges: 1 } }); + + await notifyPodAgents(POD_ID, changed, 'updated', { isAgent: false }); + + expect(wakeNames()).toEqual([]); + expect(AgentEvent.updateOne).toHaveBeenCalledTimes(1); + expect(AgentEvent.updateOne.mock.calls[0][1].$set['payload.content']).toContain('2 changes'); }); }); @@ -328,14 +455,14 @@ describe('offeredAt race probe', () => { it('keeps write two due with a CAS stamp of seen, while a now stamp hides it', async () => { const seen = new Date(NOW.getTime() - 5000); const writeTwo = new Date(NOW.getTime() - 4000); - const casTask = await createTask({ taskId: 'TASK-CAS', updatedAt: seen }); + const casTask = await createTask({ taskId: 'TASK-CAS', updatedAt: seen, assignee: 'scout' }); const nowTask = await createTask({ taskId: 'TASK-NOW', updatedAt: seen }); setInstalls([install()]); AgentEventService.enqueue.mockImplementationOnce(async () => { await Task.updateOne( { _id: casTask._id }, - { $set: { updatedAt: writeTwo } }, + { $set: { updatedAt: writeTwo, assignee: null } }, { timestamps: false }, ); }); diff --git a/backend/__tests__/service/tasks.claim-lease.test.js b/backend/__tests__/service/tasks.claim-lease.test.js index b24471d79b..66c82362f6 100644 --- a/backend/__tests__/service/tasks.claim-lease.test.js +++ b/backend/__tests__/service/tasks.claim-lease.test.js @@ -30,6 +30,7 @@ const Pod = require('../../models/Pod'); const Task = require('../../models/Task'); const User = require('../../models/User'); const { AgentInstallation } = require('../../models/AgentRegistry'); +const KernelWorkSweepService = require('../../services/kernelWorkSweepService').default; const tasksApiRoutes = require('../../routes/tasksApi'); const { hash } = require('../../utils/secret'); const { @@ -338,4 +339,201 @@ describe('POST /api/v1/tasks/:podId/:taskId/claim — lease semantics (ADR-018 D expect(notifyPodAgents).not.toHaveBeenCalled(); }); }); + + test('PATCH wakes the owner immediately before its atomic write under interleaving', async () => { + await seedTask({ assignee: 'alpha' }); + notifyPodAgents.mockClear(); + const originalUpdate = Task.findOneAndUpdate.bind(Task); + let releaseGamma; + let gammaEntered; + const gammaPaused = new Promise((resolve) => { releaseGamma = resolve; }); + const entered = new Promise((resolve) => { gammaEntered = resolve; }); + let pausedGamma = false; + const updateSpy = jest.spyOn(Task, 'findOneAndUpdate').mockImplementation((filter, update, options) => { + if (update?.$set?.assignee === 'gamma' && !pausedGamma) { + pausedGamma = true; + gammaEntered(); + return gammaPaused.then(() => originalUpdate(filter, update, options)); + } + return originalUpdate(filter, update, options); + }); + const patch = (assignee) => request(app) + .patch(`/api/v1/tasks/${pod._id}/T-1`) + .set('Authorization', `Bearer ${ownerToken}`) + .send({ assignee }); + + try { + const gammaRequest = patch('gamma').then((response) => response); + await entered; + await new Promise((resolve) => setTimeout(resolve, 5)); + const betaResponse = await patch('beta'); + expect(betaResponse.status).toBe(200); + releaseGamma(); + const gammaResponse = await gammaRequest; + expect(gammaResponse.status).toBe(200); + + const gammaWake = notifyPodAgents.mock.calls.find((call) => call[1].assignee === 'gamma'); + expect(gammaWake?.[4]).toEqual(expect.objectContaining({ assignee: 'beta' })); + expect(gammaWake?.[1]).toEqual(expect.objectContaining({ assignee: 'gamma' })); + const betaWake = notifyPodAgents.mock.calls.find((call) => call[1].assignee === 'beta'); + expect(gammaWake[1].updatedAt.getTime()).toBeGreaterThan(betaWake[1].updatedAt.getTime()); + expect((await Task.findOne({ podId: pod._id, taskId: 'T-1' })).assignee).toBe('gamma'); + } finally { + releaseGamma(); + updateSpy.mockRestore(); + } + }); + + test('claim wakes the assignee at its successful write after a racing PATCH', async () => { + await seedTask({ assignee: 'alpha' }); + notifyPodAgents.mockClear(); + const originalUpdate = Task.findOneAndUpdate.bind(Task); + let releaseClaim; + let claimEntered; + const claimPaused = new Promise((resolve) => { releaseClaim = resolve; }); + const entered = new Promise((resolve) => { claimEntered = resolve; }); + let pausedClaim = false; + const updateSpy = jest.spyOn(Task, 'findOneAndUpdate').mockImplementation((filter, update, options) => { + if (update?.$set?.claimedBy === owner._id.toString() && !pausedClaim) { + pausedClaim = true; + claimEntered(); + return claimPaused.then(() => originalUpdate(filter, update, options)); + } + return originalUpdate(filter, update, options); + }); + + try { + const claimRequest = claim(ownerToken).then((response) => response); + await entered; + const patchResponse = await request(app) + .patch(`/api/v1/tasks/${pod._id}/T-1`) + .set('Authorization', `Bearer ${ownerToken}`) + .send({ assignee: 'beta' }); + expect(patchResponse.status).toBe(200); + notifyPodAgents.mockClear(); + releaseClaim(); + const claimResponse = await claimRequest; + expect(claimResponse.status).toBe(200); + + const claimWake = notifyPodAgents.mock.calls.find((call) => call[1].claimedBy === owner._id.toString()); + expect(claimWake?.[4]).toEqual(expect.objectContaining({ assignee: 'beta' })); + expect(claimWake?.[1]).toEqual(expect.objectContaining({ assignee: 'beta' })); + } finally { + releaseClaim(); + updateSpy.mockRestore(); + } + }); + + test('claim uses the rescued owner when a sweep writes the same timestamp', async () => { + const task = await seedTask({ + status: 'claimed', + assignee: 'alpha', + claimedBy: 'alpha', + claimedAt: new Date(Date.now() - LEASE_MS - 5000), + claimExpiresAt: new Date(Date.now() - 5000), + }); + notifyPodAgents.mockClear(); + const originalUpdate = Task.findOneAndUpdate.bind(Task); + let releaseClaim; + let claimEntered; + const claimPaused = new Promise((resolve) => { releaseClaim = resolve; }); + const entered = new Promise((resolve) => { claimEntered = resolve; }); + let pausedClaim = false; + const updateSpy = jest.spyOn(Task, 'findOneAndUpdate').mockImplementation((filter, update, options) => { + if (update?.$set?.claimedBy === owner._id.toString() && !pausedClaim) { + pausedClaim = true; + claimEntered(); + return claimPaused.then(() => originalUpdate(filter, update, options)); + } + return originalUpdate(filter, update, options); + }); + + try { + const claimRequest = claim(ownerToken).then((response) => response); + await entered; + // Mongoose normally stamps the sweep with wall-clock time. Hold its + // clock at the existing revision to reproduce a timestamp collision. + const mongoose = require('mongoose'); + const clockSpy = jest.spyOn(mongoose, 'now').mockReturnValue(task.updatedAt); + try { + const { rescued } = await KernelWorkSweepService.rescueLapsed(new Date()); + expect(rescued).toHaveLength(1); + } finally { + clockSpy.mockRestore(); + } + const rescuedTask = await Task.findById(task._id); + expect(rescuedTask.updatedAt.getTime()).toBe(task.updatedAt.getTime()); + expect(rescuedTask.assignee).toBeNull(); + notifyPodAgents.mockClear(); + releaseClaim(); + const claimResponse = await claimRequest; + expect(claimResponse.status).toBe(200); + + const claimWake = notifyPodAgents.mock.calls.find((call) => call[1].claimedBy === owner._id.toString()); + expect(claimWake?.[4]).toEqual(expect.objectContaining({ assignee: null, claimedBy: null })); + expect(claimWake?.[4].assignee).not.toBe('alpha'); + } finally { + releaseClaim(); + updateSpy.mockRestore(); + } + }); + + test('PATCH unassignment offers the exact stored revision before the work sweep', async () => { + await seedTask({ assignee: 'claim-audit-agent' }); + // Force the same monotonic retry the PATCH takes when a competing writer + // has committed after this request chose its timestamp. + await Task.updateOne( + { podId: pod._id, taskId: 'T-1' }, + { $set: { updatedAt: new Date(Date.now() + 1000) } }, + { timestamps: false }, + ); + notifyPodAgents.mockClear(); + const res = await request(app) + .patch(`/api/v1/tasks/${pod._id}/T-1`) + .set('Authorization', `Bearer ${ownerToken}`) + .send({ assignee: null }); + expect(res.status).toBe(200); + const wake = notifyPodAgents.mock.calls.find((call) => call[1].taskId === 'T-1'); + expect(wake?.[4]).toEqual(expect.objectContaining({ assignee: 'claim-audit-agent' })); + const stored = await Task.findOne({ podId: pod._id, taskId: 'T-1' }); + expect(wake[1].updatedAt.getTime()).toBe(stored.updatedAt.getTime()); + + const realNotifyPodAgents = jest.requireActual('../../services/taskEventService').notifyPodAgents; + await realNotifyPodAgents(pod._id, wake[1], wake[2], wake[3], wake[4]); + const offered = await Task.findById(stored._id); + expect(offered.offeredAt?.getTime()).toBe(offered.updatedAt.getTime()); + + const sweep = await require('../../services/kernelWorkSweepService').wakeForFoundWork(new Date()); + expect(sweep.scannedPods).toBe(0); + + const note = await request(app) + .post(`/api/v1/tasks/${pod._id}/T-1/updates`) + .set('Authorization', `Bearer ${ownerToken}`) + .send({ text: 'Still available for pickup' }); + expect(note.status).toBe(200); + const afterNote = await Task.findById(stored._id); + expect(afterNote.updatedAt.getTime()).toBeGreaterThan(offered.offeredAt.getTime()); + const dueSweep = await require('../../services/kernelWorkSweepService').wakeForFoundWork(new Date()); + expect(dueSweep.scannedPods).toBe(1); + + const title = await request(app) + .patch(`/api/v1/tasks/${pod._id}/T-1`) + .set('Authorization', `Bearer ${ownerToken}`) + .send({ title: 'Still open' }); + expect(title.status).toBe(200); + const afterTitle = await Task.findById(stored._id); + expect(afterTitle.updatedAt.getTime()).toBeGreaterThan(afterNote.updatedAt.getTime()); + + const claimed = await claim(ownerToken); + expect(claimed.status).toBe(200); + expect(new Date(claimed.body.task.updatedAt).getTime()).toBeGreaterThan(afterTitle.updatedAt.getTime()); + + const completed = await request(app) + .post(`/api/v1/tasks/${pod._id}/T-1/complete`) + .set('Authorization', `Bearer ${ownerToken}`) + .send({}); + expect(completed.status).toBe(200); + expect(new Date(completed.body.task.updatedAt).getTime()) + .toBeGreaterThan(new Date(claimed.body.task.updatedAt).getTime()); + }); }); diff --git a/backend/__tests__/service/tasks.source-ref-idempotency.test.js b/backend/__tests__/service/tasks.source-ref-idempotency.test.js index ceaa723814..ae6c2f9042 100644 --- a/backend/__tests__/service/tasks.source-ref-idempotency.test.js +++ b/backend/__tests__/service/tasks.source-ref-idempotency.test.js @@ -115,6 +115,12 @@ describe('POST /api/v1/tasks/:podId sourceRef idempotency', () => { it('keeps the pre-check reopen behavior for a completed task', async () => { const existing = await seedTask({ status: 'done', completedAt: new Date() }); + const priorRevision = new Date(Date.now() + 1000); + await Task.updateOne( + { _id: existing._id }, + { $set: { updatedAt: priorRevision } }, + { timestamps: false }, + ); const response = await postTask({ title: 'Existing task', @@ -137,6 +143,7 @@ describe('POST /api/v1/tasks/:podId sourceRef idempotency', () => { }, }); expect(await Task.countDocuments({ podId: pod._id, sourceRef: 'external:ticket:697' })).toBe(1); + expect(new Date(response.body.task.updatedAt).getTime()).toBeGreaterThan(priorRevision.getTime()); }); it('clears the assignee when a same-pair reopen is submitted without one', async () => { diff --git a/backend/__tests__/unit/routes/tasksApi.status-vocabulary.test.js b/backend/__tests__/unit/routes/tasksApi.status-vocabulary.test.js index b4f26365e2..0019c404d9 100644 --- a/backend/__tests__/unit/routes/tasksApi.status-vocabulary.test.js +++ b/backend/__tests__/unit/routes/tasksApi.status-vocabulary.test.js @@ -30,7 +30,7 @@ jest.mock('../../../models/Pod', () => ({ const mockFindOneAndUpdate = jest.fn(); jest.mock('../../../models/Task', () => ({ findOneAndUpdate: (...args) => mockFindOneAndUpdate(...args), - findOne: jest.fn(), + findOne: jest.fn(() => ({ select: () => ({ lean: async () => null }) })), find: jest.fn(), })); @@ -45,8 +45,10 @@ jest.mock('../../../services/githubAppService', () => ({ })); const mockEmitTaskUpdated = jest.fn(); +const mockNotifyPodAgents = jest.fn(); jest.mock('../../../services/taskEventService', () => ({ emitTaskUpdated: (...args) => mockEmitTaskUpdated(...args), + notifyPodAgents: (...args) => mockNotifyPodAgents(...args), })); const mockRecordTaskAttention = jest.fn(); @@ -57,6 +59,7 @@ jest.mock('../../../services/attentionItemService', () => ({ })); const tasksApi = require('../../../routes/tasksApi'); +const Task = require('../../../models/Task'); const app = express(); app.use(express.json()); @@ -68,6 +71,8 @@ beforeEach(() => { mockFindOneAndUpdate.mockReset(); mockFindOneAndUpdate.mockResolvedValue({ taskId: 'TASK-001', status: 'claimed' }); mockEmitTaskUpdated.mockReset(); + mockNotifyPodAgents.mockReset(); + Task.findOne.mockReturnValue({ select: () => ({ lean: async () => null }) }); mockRecordTaskAttention.mockReset(); mockRecordTaskAttention.mockResolvedValue(undefined); mockResolveTaskAttention.mockReset(); @@ -131,12 +136,16 @@ describe('PATCH /:podId/:taskId status vocabulary', () => { // `done` — every one completed through this route, which never told the // attention store. Only the PATCH gate did. const task = { _id: 'task-1', taskId: 'TASK-001', status: 'done', podId: POD_ID, updates: [] }; + Task.findOne.mockReturnValue({ select: () => ({ lean: async () => ({ updatedAt: new Date(Date.now() - 1) }) }) }); mockFindOneAndUpdate.mockResolvedValueOnce(task); const res = await request(app) .post(`/api/v1/tasks/${POD_ID}/TASK-001/complete`) .send({ prUrl: 'https://github.com/Team-Commonly/commonly/pull/1' }); expect(res.status).toBe(200); expect(mockResolveTaskAttention).toHaveBeenCalledWith(expect.objectContaining({ _id: 'task-1' })); + expect(mockNotifyPodAgents).toHaveBeenCalledWith( + POD_ID, task, 'updated', expect.any(Object), { status: 'claimed' }, + ); }); test('rejects unknown statuses with the vocabulary in the error', async () => { @@ -151,11 +160,42 @@ describe('PATCH /:podId/:taskId status vocabulary', () => { }); test('non-status patches are unaffected by the gate', async () => { + Task.findOne.mockReturnValue({ select: () => ({ lean: async () => ({ updatedAt: new Date(Date.now() - 1) }) }) }); const res = await request(app) .patch(`/api/v1/tasks/${POD_ID}/TASK-001`) .send({ title: 'renamed' }); expect(res.status).toBe(200); expect(mockFindOneAndUpdate.mock.calls[0][1].$set.title).toBe('renamed'); + expect(mockFindOneAndUpdate.mock.calls[0][2]).toEqual({ new: true, timestamps: false }); + }); + + test('passes the prior assignee to board wake routing when unassigning', async () => { + mockFindOneAndUpdate.mockResolvedValueOnce({ taskId: 'TASK-001', assignee: 'scout', status: 'claimed' }); + + const res = await request(app) + .patch(`/api/v1/tasks/${POD_ID}/TASK-001`) + .send({ assignee: '' }); + + expect(res.status).toBe(200); + expect(mockFindOneAndUpdate.mock.calls[0][2]).toEqual({ new: false, timestamps: false }); + expect(mockNotifyPodAgents).toHaveBeenCalledWith( + POD_ID, expect.objectContaining({ assignee: null }), 'updated', + expect.any(Object), { assignee: 'scout', claimedBy: undefined, status: 'claimed' }, + ); + }); + + test('gets prior ownership from the successful atomic write', async () => { + mockFindOneAndUpdate.mockResolvedValueOnce({ taskId: 'TASK-001', assignee: 'alpha', status: 'pending' }); + const res = await request(app) + .patch(`/api/v1/tasks/${POD_ID}/TASK-001`) + .send({ assignee: 'scout' }); + + expect(res.status).toBe(200); + expect(mockFindOneAndUpdate).toHaveBeenCalledTimes(1); + expect(mockNotifyPodAgents).toHaveBeenCalledWith( + POD_ID, expect.objectContaining({ assignee: 'scout' }), 'updated', + expect.any(Object), { assignee: 'alpha', claimedBy: undefined, status: 'pending' }, + ); }); }); diff --git a/backend/__tests__/unit/services/nativeRuntimeService.hostedEngine.test.js b/backend/__tests__/unit/services/nativeRuntimeService.hostedEngine.test.js index 5427564d08..74356df161 100644 --- a/backend/__tests__/unit/services/nativeRuntimeService.hostedEngine.test.js +++ b/backend/__tests__/unit/services/nativeRuntimeService.hostedEngine.test.js @@ -197,6 +197,17 @@ describe('nativeRuntimeService — hosted installs on the native engine', () => expect(body.messages[0].content).toMatch(/Keeps the research log\./); }); + test('an own-row board wake reaches the hosted model through the mention route', async () => { + const result = await runAgent(hostedInstall(), { + type: 'chat.mention', eventId: '507f1f77bcf86cd799439014', + payload: { boardWake: true, taskId: 'TASK-414', content: '[The board moved: TASK-414 was assigned to you.]' }, + }); + + expect(result.status).toBe('succeeded'); + expect(AgentRun.create).toHaveBeenCalledTimes(1); + expect(axios.post).toHaveBeenCalledTimes(1); + }); + // Search and code runs are on every plan (Cofounder, 2026-10-16), offered // only where this instance holds the key, and bounded per turn. describe('web search and code runs', () => { diff --git a/backend/__tests__/unit/services/taskEventService.notifyPodAgents.test.js b/backend/__tests__/unit/services/taskEventService.notifyPodAgents.test.js index 17e52600ae..48e7c104eb 100644 --- a/backend/__tests__/unit/services/taskEventService.notifyPodAgents.test.js +++ b/backend/__tests__/unit/services/taskEventService.notifyPodAgents.test.js @@ -11,7 +11,7 @@ const mongoose = require('mongoose'); jest.mock('../../../models/AgentRegistry', () => ({ - AgentInstallation: { find: jest.fn() }, + AgentInstallation: { find: jest.fn(), findOne: jest.fn() }, })); jest.mock('../../../services/agentEventService', () => ({ @@ -23,6 +23,10 @@ jest.mock('../../../models/AgentEvent', () => ({ updateOne: jest.fn(), })); +jest.mock('../../../models/User', () => ({ + findById: jest.fn(), +})); + const mockTaskUpdateOne = jest.fn(); jest.mock('../../../models/Task', () => ({ updateOne: (...args) => mockTaskUpdateOne(...args), @@ -33,6 +37,7 @@ const { AgentInstallation } = require('../../../models/AgentRegistry'); const AgentEventService = require('../../../services/agentEventService'); const AgentEvent = require('../../../models/AgentEvent'); const Task = require('../../../models/Task'); +const User = require('../../../models/User'); const { notifyPodAgents, notifyFoundWork } = require('../../../services/taskEventService'); const POD_ID = new mongoose.Types.ObjectId().toString(); @@ -49,13 +54,14 @@ const install = (agentName, installedBy) => ({ podId: POD_ID, status: 'active', installedBy, - config: { wakeOnMessage: { enabled: true } }, + config: { runtime: { runtimeType: 'wrapper' }, wakeOnMessage: { enabled: true } }, }); const task = (overrides = {}) => ({ taskId: 'TASK-042', title: 'Wire the board to the fleet', status: 'todo', + assignee: 'scout', updatedAt: '2026-08-19T10:00:00.000Z', ...overrides, }); @@ -70,6 +76,8 @@ beforeEach(() => { // Default: nothing undrained, so each test exercises the fresh-enqueue path // unless it explicitly seeds a pending wake. AgentEvent.findOneAndUpdate.mockResolvedValue(null); + AgentInstallation.findOne.mockReturnValue({ sort: () => ({ lean: async () => null }) }); + User.findById.mockReturnValue({ select: () => ({ lean: async () => null }) }); AgentEvent.updateOne.mockResolvedValue({}); mockTaskUpdateOne.mockResolvedValue({ matchedCount: 1 }); Task.find.mockReturnValue({ @@ -81,9 +89,9 @@ describe('notifyPodAgents', () => { it('stamps the offered task revision only after every eligible seat succeeds', async () => { mockInstalls([install('scout', HUMAN_ID), install('sprint-impl', HUMAN_ID)]); const offeredAt = new Date('2026-08-20T20:00:00.000Z'); - const offeredTask = task({ _id: 'task-1', updatedAt: offeredAt }); + const offeredTask = task({ _id: 'task-1', updatedAt: offeredAt, assignee: 'sprint-impl' }); - await notifyPodAgents(POD_ID, offeredTask, 'created', { userId: HUMAN_ID, isAgent: false }); + await notifyPodAgents(POD_ID, offeredTask, 'created', { userId: HUMAN_ID, isAgent: false }, { assignee: 'scout' }); expect(mockTaskUpdateOne).toHaveBeenCalledTimes(1); expect(mockTaskUpdateOne).toHaveBeenCalledWith( @@ -101,9 +109,10 @@ describe('notifyPodAgents', () => { await notifyPodAgents( POD_ID, - task({ _id: 'task-1', updatedAt: new Date('2026-08-20T20:00:00.000Z') }), + task({ _id: 'task-1', updatedAt: new Date('2026-08-20T20:00:00.000Z'), assignee: 'sprint-impl' }), 'created', { userId: HUMAN_ID, isAgent: false }, + { assignee: 'scout' }, ); expect(AgentEventService.enqueue).toHaveBeenCalledTimes(2); @@ -194,10 +203,11 @@ describe('notifyPodAgents', () => { expect(filter.type).toBeUndefined(); }); - it('carries no messageId, so every seat looks and the task CAS arbitrates', async () => { + it('carries no messageId for either current or prior owner', async () => { mockInstalls([install('scout', HUMAN_ID), install('sprint-impl', HUMAN_ID)]); - await notifyPodAgents(POD_ID, task(), 'created', { userId: HUMAN_ID, isAgent: false }); + await notifyPodAgents(POD_ID, task({ assignee: 'sprint-impl' }), 'updated', + { userId: HUMAN_ID, isAgent: false }, { assignee: 'scout' }); // A messageId would re-engage the wrapper's claim gate and put us back to // one-looker-per-change — dedupe at the wrong layer. Contention belongs on @@ -233,9 +243,9 @@ describe('notifyPodAgents', () => { it('never wakes the actor about their own edit', async () => { mockInstalls([install('scout', AGENT_ID), install('sprint-impl', HUMAN_ID)]); - await notifyPodAgents(POD_ID, task(), 'updated', { + await notifyPodAgents(POD_ID, task({ assignee: 'sprint-impl' }), 'updated', { userId: AGENT_ID, isAgent: true, agentName: 'scout', instanceId: 'default', - }); + }, { assignee: 'scout' }); const woken = AgentEventService.enqueue.mock.calls.map((c) => c[0].agentName); expect(woken).toEqual(['sprint-impl']); @@ -249,9 +259,9 @@ describe('notifyPodAgents', () => { // bug from both directions, which is why the key is identity instead. mockInstalls([install('scout', HUMAN_ID), install('sprint-impl', HUMAN_ID)]); - await notifyPodAgents(POD_ID, task(), 'updated', { + await notifyPodAgents(POD_ID, task({ assignee: 'sprint-impl' }), 'updated', { userId: AGENT_ID, isAgent: true, agentName: 'scout', instanceId: 'default', - }); + }, { assignee: 'scout' }); const woken = AgentEventService.enqueue.mock.calls.map((c) => c[0].agentName); expect(woken).toEqual(['sprint-impl']); @@ -260,9 +270,9 @@ describe('notifyPodAgents', () => { it('matches the self-skip case-insensitively, so a capitalised name still skips', async () => { mockInstalls([install('Scout', HUMAN_ID), install('sprint-impl', HUMAN_ID)]); - await notifyPodAgents(POD_ID, task(), 'updated', { + await notifyPodAgents(POD_ID, task({ assignee: 'sprint-impl' }), 'updated', { userId: AGENT_ID, isAgent: true, agentName: 'SCOUT', instanceId: 'default', - }); + }, { assignee: 'scout' }); const woken = AgentEventService.enqueue.mock.calls.map((c) => c[0].agentName); expect(woken).toEqual(['sprint-impl']); @@ -276,7 +286,7 @@ describe('notifyPodAgents', () => { { ...install('scout', HUMAN_ID), instanceId: 'ucc4035c51b' }, ]); - await notifyPodAgents(POD_ID, task(), 'updated', { + await notifyPodAgents(POD_ID, task({ assignee: 'ucc4035c51b' }), 'updated', { userId: AGENT_ID, isAgent: true, agentName: 'scout', instanceId: 'u0da521ab41', }); @@ -284,6 +294,18 @@ describe('notifyPodAgents', () => { expect(woken).toEqual(['ucc4035c51b']); }); + it('does not broadcast a shared agent-name assignment to every named instance', async () => { + mockInstalls([ + { ...install('openclaw', HUMAN_ID), instanceId: 'alpha' }, + { ...install('openclaw', HUMAN_ID), instanceId: 'beta' }, + ]); + + await notifyPodAgents(POD_ID, task({ assignee: 'openclaw' }), 'updated', + { userId: HUMAN_ID, isAgent: false }); + + expect(AgentEventService.enqueue).not.toHaveBeenCalled(); + }); + it('still wakes an agent when the HUMAN who installed it edits the board', async () => { // The self-skip nearly shipped keyed on `installedBy` alone. That field is // the agent's own user id when an agent self-installs, but the HUMAN's id @@ -325,21 +347,58 @@ describe('notifyPodAgents', () => { expect(AgentEventService.enqueue.mock.calls[0][0].payload.dmKind).toBe('agent-agent'); }); - it('respects an opt-OUT, so ambient wakes cannot arrive through a second door', async () => { - // 169 of 367 production installs have wake-on-message off — `commonly-bot` - // and `openclaw`, user-facing seats where an unrequested wake is a noise - // incident. A board change is ambient activity nobody addressed to them, so - // it is governed by the same switch as ambient chat. + it('wakes the owner regardless of ambient opt-in and skips unrelated seats', async () => { mockInstalls([ { ...install('commonly-bot', HUMAN_ID), config: { wakeOnMessage: { enabled: false } } }, { ...install('openclaw', HUMAN_ID), config: {} }, install('sprint-impl', HUMAN_ID), ]); - await notifyPodAgents(POD_ID, task(), 'created', { userId: HUMAN_ID, isAgent: false }); + await notifyPodAgents(POD_ID, task({ assignee: 'commonly-bot' }), 'created', { userId: HUMAN_ID, isAgent: false }); const woken = AgentEventService.enqueue.mock.calls.map((c) => c[0].agentName); - expect(woken).toEqual(['sprint-impl']); + expect(woken).toEqual(['commonly-bot']); + }); + + it('resolves a claimed row through the bot principal, not its installer', async () => { + mockInstalls([install('scout', HUMAN_ID), install('sprint-impl', HUMAN_ID)]); + User.findById.mockReturnValue({ select: () => ({ lean: async () => ({ + isBot: true, botMetadata: { agentName: 'sprint-impl', instanceId: 'default' }, + }) }) }); + + await notifyPodAgents(POD_ID, task({ assignee: null, claimedBy: AGENT_ID }), 'updated', + { userId: HUMAN_ID, isAgent: false }); + + expect(AgentEventService.enqueue.mock.calls.map(([event]) => event.agentName)).toEqual(['sprint-impl']); + }); + + it('does not treat an ambiguous default claim key as every default seat', async () => { + mockInstalls([install('scout', HUMAN_ID), install('sprint-impl', HUMAN_ID)]); + + await notifyPodAgents(POD_ID, task({ assignee: null, claimedBy: 'default' }), 'updated', + { userId: HUMAN_ID, isAgent: false }); + + expect(AgentEventService.enqueue).not.toHaveBeenCalled(); + }); + + it('does not treat an ambiguous default assignee as every default seat', async () => { + mockInstalls([install('scout', HUMAN_ID), install('sprint-impl', HUMAN_ID)]); + + await notifyPodAgents(POD_ID, task({ assignee: 'default' }), 'updated', + { userId: HUMAN_ID, isAgent: false }); + + expect(AgentEventService.enqueue).not.toHaveBeenCalled(); + }); + + it('wakes a claimed dependent row when its exact dependency clears', async () => { + mockInstalls([install('scout', HUMAN_ID), install('sprint-impl', HUMAN_ID)]); + Task.find.mockReturnValueOnce({ lean: async () => [{ assignee: 'sprint-impl' }] }); + + await notifyPodAgents(POD_ID, task({ assignee: null, status: 'done' }), 'updated', + { userId: HUMAN_ID, isAgent: false }, { status: 'claimed' }); + + expect(Task.find.mock.calls[0][0]).toMatchObject({ podId: POD_ID, dep: 'TASK-042' }); + expect(AgentEventService.enqueue.mock.calls.map(([event]) => event.agentName)).toEqual(['sprint-impl']); }); it('stays quiet on delete, where the wake would spend a turn finding nothing', async () => { @@ -360,7 +419,8 @@ describe('notifyPodAgents', () => { .mockResolvedValueOnce({}); await expect( - notifyPodAgents(POD_ID, task(), 'created', { userId: HUMAN_ID, isAgent: false }), + notifyPodAgents(POD_ID, task({ assignee: 'sprint-impl' }), 'updated', + { userId: HUMAN_ID, isAgent: false }, { assignee: 'scout' }), ).resolves.toBeUndefined(); expect(AgentEventService.enqueue).toHaveBeenCalledTimes(2); diff --git a/backend/routes/tasksApi.ts b/backend/routes/tasksApi.ts index f41db6748c..c5ff245ba3 100644 --- a/backend/routes/tasksApi.ts +++ b/backend/routes/tasksApi.ts @@ -57,9 +57,9 @@ const actorOf = (req: any) => { * into the route and turned every board write into a 500: strictly worse than * the silent fan-out this exists to fix. */ -const notifyAgents = (req: any, podId: unknown, task: unknown, kind: string) => { +const notifyAgents = (req: any, podId: unknown, task: unknown, kind: string, previousTask?: unknown) => { try { - const pending = notifyPodAgents(podId, task, kind, actorOf(req)); + const pending = notifyPodAgents(podId, task, kind, actorOf(req), previousTask); if (pending?.catch) { pending.catch((e: Error) => console.warn('[tasks] agent notify failed:', e.message)); } @@ -96,6 +96,52 @@ function auth(req: AuthReq, res: Res, next: () => void) { const router: ReturnType = express.Router(); +// A pending unassigned row is offered again only when updatedAt advances past +// offeredAt. Mongoose's wall-clock timestamp can run behind a revision bumped +// by a concurrent PATCH, so task writes in this route reserve the next value +// against the stored revision instead of letting a later write move it back. +const updateTaskWithRevisionSnapshot = async ( + filter: Record, + update: Record, +) => { + const key = { podId: filter.podId, taskId: filter.taskId }; + for (let attempt = 0; attempt < 4; attempt += 1) { + // eslint-disable-next-line no-await-in-loop + const current = await Task.findOne(key).select('updatedAt claimedBy lapsedFrom assignee status').lean(); + if (!current) return null; + const writeTime = new Date(Math.max(Date.now(), current.updatedAt.getTime() + 1)); + // eslint-disable-next-line no-await-in-loop + const task = await Task.findOneAndUpdate( + { + ...filter, + updatedAt: current.updatedAt, + assignee: current.assignee ?? null, + // A caller may already constrain claimedBy (note renewal). Preserve + // that predicate; otherwise bind the ownership snapshot to this CAS. + ...('claimedBy' in filter ? {} : { claimedBy: current.claimedBy ?? null }), + }, + { ...update, $set: { ...(update.$set as Record || {}), updatedAt: writeTime } }, + { new: true, timestamps: false }, + ); + // The revision and ownership predicates bind this snapshot to the write. + if (task) return { task, previous: current }; + // Retry a changed revision, owner, or status; otherwise a caller predicate + // (such as a live lease) refused the write. + // eslint-disable-next-line no-await-in-loop + const latest = await Task.findOne(key).select('updatedAt assignee claimedBy status').lean(); + if (!latest || (latest.updatedAt.getTime() === current.updatedAt.getTime() + && (latest.assignee ?? null) === (current.assignee ?? null) + && (latest.claimedBy ?? null) === (current.claimedBy ?? null) + && latest.status === current.status)) return null; + } + return null; +}; + +const updateTaskWithRevision = async ( + filter: Record, + update: Record, +) => (await updateTaskWithRevisionSnapshot(filter, update))?.task || null; + const taskWriteRateLimitKey = (req: Request): string => { const authHeader = req.get('Authorization') || req.get('x-auth-token'); if (authHeader) { @@ -268,7 +314,7 @@ router.post('/:podId', rateLimit({ // the ask came from, the title says which ask it is. Keying on the ref // alone made a second ask from one source adopt the first ask's row and // discard the caller's title (TASK-063). Same pair = idempotent retry. - const existing = await Task.findOne({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), sourceRef, title }) as { title?: string; status?: string; assignee?: string; claimedAt?: Date | null; claimExpiresAt?: Date | null; notes?: string; updates: Array<{ text: string; author: string; authorId: string | null; createdAt: Date }>; save: () => Promise; toObject: () => unknown } | null; + const existing = await Task.findOne({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), sourceRef, title }) as { _id: unknown; title?: string; status?: string; assignee?: string; claimedAt?: Date | null; claimExpiresAt?: Date | null; notes?: string; updatedAt: Date; toObject: () => unknown } | null; if (existing) { if (existing.status === 'done') { // `sourceRef` + `title` are an idempotency key (a unique partial @@ -286,27 +332,29 @@ router.post('/:podId', rateLimit({ // and report the mismatch, which put a completed row back to pending // under a title its caller never chose (TASK-063). const previousNotes = (existing.notes || '').trim(); - if (previousNotes) { - existing.updates.push({ - text: `Previous notes preserved from the completed run:\n${previousNotes}`, - author: 'system', - authorId: null, - createdAt: new Date(), - }); - } - existing.status = 'pending'; - existing.assignee = assignee || undefined; - existing.claimedAt = null; - existing.claimExpiresAt = null; - existing.notes = 'Reopened — the same source is active again.'; - existing.updates.push({ + const history = previousNotes ? [{ + text: `Previous notes preserved from the completed run:\n${previousNotes}`, + author: 'system', authorId: null, createdAt: new Date(), + }] : []; + history.push({ text: 'Reopened: task was done but its source is active again — picking up again.', - author: 'system', - authorId: null, - createdAt: new Date(), + author: 'system', authorId: null, createdAt: new Date(), }); - await existing.save(); - const reopenedObj = existing.toObject(); + const writeTime = new Date(Math.max(Date.now(), existing.updatedAt.getTime() + 1)); + const reopened = await Task.findOneAndUpdate( + { _id: existing._id, status: 'done', updatedAt: existing.updatedAt }, + { + $set: { + status: 'pending', assignee: assignee || null, claimedAt: null, + claimExpiresAt: null, notes: 'Reopened — the same source is active again.', + updatedAt: writeTime, + }, + $push: { updates: { $each: history } }, + }, + { new: true, timestamps: false }, + ); + if (!reopened) return res.status(409).json({ error: 'Task changed concurrently; retry' }); + const reopenedObj = reopened.toObject(); emitTaskUpdated(podId, reopenedObj, 'updated'); notifyAgents(req, podId, reopenedObj, 'updated'); // `alreadyExists: true`, because it does: this request did not create @@ -567,17 +615,13 @@ router.post('/:podId/:taskId/claim', rateLimit({ const access = await requirePodMember(podId || '', userId, { write: true }); if (access.error) return res.status(access.status || 500).json({ error: access.error }); const now = new Date(); - // Pre-read the current holder to classify this claim AFTER the CAS wins: + // Classify the current holder only after this claim's CAS wins: // a same-holder continuation (renewal, or re-take after the holder's own // lapse) is no news to the room and must not fan out (the redundant wakes // burned a model turn per seat per renewal and filled Sharpen with seats // narrating why they won't act — measured 2026-08-25, 4 wakes in one - // session for one task). The read races the CAS by design: worst case is - // one wake mis-classified once, which beats a transaction on the hot path. - const preClaim = await Task.findOne({ - podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), - taskId, - }).select('claimedBy lapsedFrom').lean() as { claimedBy?: string | null; lapsedFrom?: string | null } | null; + // session for one task). The revision CAS binds the ownership snapshot to + // this successful write, even if an assignee PATCH raced the claim. // `rescueDeferrals` and `lapsedFrom` reset on every claim (#1080 part 2/3). // The deferral budget is per-lease, not per-task: a seat that renews // normally must never inherit a predecessor's spent budget, or the third @@ -589,7 +633,7 @@ router.post('/:podId/:taskId/claim', rateLimit({ // this named task is blocked. Legacy claims get their effective expiry // derived from claimedAt, so no migration and no instant steal of work // someone claimed minutes ago. - const task = await Task.findOneAndUpdate({ + const result = await updateTaskWithRevisionSnapshot({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), taskId, // A blocked row is deliberately resumable only when a seat names its @@ -597,8 +641,8 @@ router.post('/:podId/:taskId/claim', rateLimit({ // drives the discovery query, where a parked dependency is not work a // peer may pick up speculatively. $or: [...claimableConditions(now, claimedBy), { status: 'blocked' }], - }, update, { new: true }); - if (!task) { + }, update); + if (!result) { const existing = await Task.findOne({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), taskId }).lean() as { claimedBy?: string; status?: string; claimExpiresAt?: Date | null } | null; if (!existing) return res.status(404).json({ error: 'Task not found' }); // Name the live holder AND when the lease frees — a loser standing @@ -611,6 +655,7 @@ router.post('/:podId/:taskId/claim', rateLimit({ claimExpiresAt: existing.claimExpiresAt || null, }); } + const { task, previous: preClaim } = result; emitTaskUpdated(podId, task, 'updated'); // Same-holder continuation: the seat that already held (or just lapsed // from) this task re-claimed it. The board UI still updates (socket emit @@ -619,7 +664,7 @@ router.post('/:podId/:taskId/claim', rateLimit({ const isSameHolderContinuation = !!preClaim && (preClaim.claimedBy === claimedBy || preClaim.lapsedFrom === claimedBy); if (!isSameHolderContinuation) { - notifyAgents(req, podId, task, 'updated'); + notifyAgents(req, podId, task, 'updated', preClaim); } return res.json({ task }); } catch (err) { @@ -644,12 +689,12 @@ router.post('/:podId/:taskId/complete', taskWriteRateLimit(30), auth, async (req const updateText = prUrl ? `Completed by ${author} · PR: ${prUrl}` : `Completed by ${author}`; const now = new Date(); const update = { $set: { status: 'done', completedAt: now, ...(prUrl && { prUrl }), ...(notes && { notes }) }, $push: { updates: { text: updateText, author, authorId: userId?.toString() || null, createdAt: now } } }; - const task = await Task.findOneAndUpdate({ + const task = await updateTaskWithRevision({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), taskId, status: { $in: ['claimed', 'pending'] }, ...writableLeaseCondition(req, userId, now), - }, update, { new: true }) as { taskId?: string; updates?: unknown[] } | null; + }, update) as { taskId?: string; updates?: unknown[] } | null; if (!task) { const racedRefusal = await leaseRefusalForWrite(podId || '', taskId || '', req, userId); if (racedRefusal) return res.status(409).json(racedRefusal); @@ -690,7 +735,7 @@ router.post('/:podId/:taskId/complete', taskWriteRateLimit(30), auth, async (req // tasks completed here. await resolveTaskAttention(task); emitTaskUpdated(podId, task, 'updated'); - notifyAgents(req, podId, task, 'updated'); + notifyAgents(req, podId, task, 'updated', { status: 'claimed' }); return res.json({ task }); } catch (err) { console.error('POST /tasks/complete error:', err); @@ -724,7 +769,7 @@ router.post('/:podId/:taskId/updates', taskWriteRateLimit(60), auth, async (req: // fallback. The note lands either way; only the lease extension is gated. const claimKey = resolveAgentInstanceId(req) || userId?.toString() || ''; let task = claimKey - ? await Task.findOneAndUpdate( + ? await updateTaskWithRevision( { podId: podFilter, taskId, status: 'claimed', claimedBy: claimKey }, // `rescueDeferrals: 0` alongside the lease. Found by @sprint-review // (pod 56544) after #1082 shipped: the note-renewal wrote only @@ -742,7 +787,6 @@ router.post('/:podId/:taskId/updates', taskWriteRateLimit(60), auth, async (req: // 04:35:54 moved claimExpiresAt to 05:05:54 with rescueDeferrals stuck // at 1. { $set: { claimExpiresAt: new Date(now.getTime() + TASK_CLAIM_LEASE_MS), rescueDeferrals: 0 }, $push: note }, - { new: true }, ) : null; let leaseRenewed = Boolean(task); @@ -828,7 +872,7 @@ router.post('/:podId/:taskId/updates', taskWriteRateLimit(60), auth, async (req: ].filter((v): v is string => typeof v === 'string' && v.length > 0); if (!task && identities.length) { - task = await Task.findOneAndUpdate( + task = await updateTaskWithRevision( { podId: podFilter, taskId, status: 'pending', lapsedFrom: { $in: identities } }, { $set: { @@ -841,15 +885,19 @@ router.post('/:podId/:taskId/updates', taskWriteRateLimit(60), auth, async (req: }, $push: note, }, - { new: true }, ); leaseRenewed = Boolean(task); } if (!task) { - task = await Task.findOneAndUpdate({ podId: podFilter, taskId }, { $push: note }, { new: true }); + task = await updateTaskWithRevision({ podId: podFilter, taskId }, { $push: note }); + } + if (!task) { + const existing = await Task.findOne({ podId: podFilter, taskId }).select('_id').lean(); + return existing + ? res.status(409).json({ error: 'Task changed concurrently; retry' }) + : res.status(404).json({ error: 'Task not found' }); } - if (!task) return res.status(404).json({ error: 'Task not found' }); await recordTaskAttention(task); emitTaskUpdated(podId, task, 'updated'); notifyAgents(req, podId, task, 'updated'); @@ -923,6 +971,7 @@ router.patch('/:podId/:taskId', taskWriteRateLimit(60), auth, async (req: AuthRe const refusal = await leaseRefusalForWrite(podId || '', taskId || '', req, userId); if (refusal) return res.status(409).json(refusal); const author = await resolveAuthor(req); + const needsPriorOwner = fieldUpdates.assignee !== undefined || fieldUpdates.status !== undefined; const changeParts: string[] = []; if (fieldUpdates.assignee !== undefined) changeParts.push(`reassigned to ${fieldUpdates.assignee || 'unassigned'}`); if (fieldUpdates.status !== undefined) changeParts.push(`status → ${fieldUpdates.status}`); @@ -931,22 +980,68 @@ router.patch('/:podId/:taskId', taskWriteRateLimit(60), auth, async (req: AuthRe if (fieldUpdates.prUrl !== undefined) changeParts.push(`PR: ${fieldUpdates.prUrl}`); if (fieldUpdates.notes !== undefined) changeParts.push('notes updated'); if (fieldUpdates.title !== undefined) changeParts.push('title updated'); - const update: Record = { $set: fieldUpdates }; + let writeTime = needsPriorOwner ? new Date() : null; + const update: Record = { + $set: writeTime ? { ...fieldUpdates, updatedAt: writeTime } : fieldUpdates, + }; if (changeParts.length > 0) update.$push = { updates: { text: `${author} updated: ${changeParts.join(', ')}`, author, authorId: userId?.toString() || null, createdAt: new Date() } }; - const task = await Task.findOneAndUpdate({ + const taskFilter = { podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), taskId, ...writableLeaseCondition(req, userId, new Date()), - }, update, { new: true }); + }; + let task = null; + let racedRevision = false; + // The pre-image names the exact prior owner. Also keep updatedAt strictly + // increasing if a second writer commits while this request waits for Mongo. + for (let attempt = 0; attempt < 4; attempt += 1) { + // eslint-disable-next-line no-await-in-loop + task = needsPriorOwner + // eslint-disable-next-line no-await-in-loop + ? await Task.findOneAndUpdate( + { ...taskFilter, updatedAt: { $lt: writeTime } }, + update, + { new: false, timestamps: false }, + ) + // eslint-disable-next-line no-await-in-loop + : await updateTaskWithRevision(taskFilter, update); + if (task || !needsPriorOwner) break; + // eslint-disable-next-line no-await-in-loop + const current = await Task.findOne({ podId: taskFilter.podId, taskId }).select('updatedAt').lean(); + if (!current) break; + racedRevision = true; + writeTime = new Date(Math.max(Date.now(), current.updatedAt.getTime() + 1)); + (update.$set as Record).updatedAt = writeTime; + } if (!task) { const racedRefusal = await leaseRefusalForWrite(podId || '', taskId || '', req, userId); if (racedRefusal) return res.status(409).json(racedRefusal); + if (racedRevision) return res.status(409).json({ error: 'Task changed concurrently; retry' }); + if (!needsPriorOwner) { + const existing = await Task.findOne({ podId: taskFilter.podId, taskId }).select('_id').lean(); + if (existing) return res.status(409).json({ error: 'Task changed concurrently; retry' }); + } return res.status(404).json({ error: 'Task not found' }); } + // For ownership/status writes, the atomic update returns its pre-image. + // A separate read can race a second writer and wake a stale owner. Capture + // the pre-image before projecting this successful write onto the response. + const previousTask = needsPriorOwner + ? { assignee: task.assignee, claimedBy: task.claimedBy, status: task.status } + : null; + if (needsPriorOwner) { + if (typeof task.set === 'function') task.set(fieldUpdates); + else Object.assign(task, fieldUpdates); + if (update.$push) { + if (!task.updates) task.updates = []; + task.updates.push((update.$push as { updates: unknown }).updates); + } + task.updatedAt = writeTime; + } if (fieldUpdates.status === 'blocked') await recordTaskAttention(task, { includeBlocked: true }); if (fieldUpdates.status !== undefined && fieldUpdates.status !== 'blocked') await resolveTaskAttention(task); emitTaskUpdated(podId, task, 'updated'); - notifyAgents(req, podId, task, 'updated'); + notifyAgents(req, podId, task, 'updated', previousTask); return res.json({ task }); } catch (err) { console.error('PATCH /tasks error:', err); diff --git a/backend/services/taskEventService.ts b/backend/services/taskEventService.ts index 4b1c4b6ffd..6bfb5479a8 100644 --- a/backend/services/taskEventService.ts +++ b/backend/services/taskEventService.ts @@ -19,6 +19,62 @@ let ioRef: SocketIOLike | null = null; export type TaskEventKind = 'created' | 'updated' | 'deleted'; const MAX_NAMED_ITEMS = 5; const MAX_ASSIGNED_ROWS = 5; +type BoardTask = Record; + +function seatMatchesAssignee(install: Record, assignee: unknown): boolean { + if (!assignee) return false; + const name = String(install.agentName || '').toLowerCase(); + const instance = String(install.instanceId || 'default').toLowerCase(); + const assigned = String(assignee).toLowerCase(); + return instance === 'default' ? assigned === name : assigned === instance; +} + +async function claimHolderKey(claimedBy: unknown): Promise { + if (!claimedBy) return null; + const claim = String(claimedBy); + if (!/^[0-9a-f]{24}$/i.test(claim)) return `::${claim.toLowerCase()}`; + // Claims store the bot User id; installedBy can instead name the human. + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const User = require('../models/User'); + const holder = await User.findById(claim).select('isBot botMetadata').lean(); + if (!holder?.isBot || !holder.botMetadata?.agentName) return null; + return `${String(holder.botMetadata.agentName).toLowerCase()}::${String(holder.botMetadata.instanceId || 'default').toLowerCase()}`; +} + +function seatHoldsClaim(install: Record, holderKey: string | null): boolean { + if (!holderKey) return false; + const name = String(install.agentName || '').toLowerCase(); + const instance = String(install.instanceId || 'default').toLowerCase(); + return holderKey === `${name}::${instance}` + || (instance !== 'default' && holderKey === `::${instance}`) + || (instance === 'default' && holderKey === `::${name}`); +} + +async function usesHostedBoardWake( + install: Record, + AgentInstallation: any, +): Promise { + // Match agentEventService.resolveInProcessInstallation: a mention-created + // pod projection can omit runtime and inherit it from the identity's latest + // configured installation. Both hosted engines answer chat.mention. + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { isHostedInstallation, readRuntimeConfig } = require('./hostedRuntimeService'); + if (readRuntimeConfig(install).runtimeType) return isHostedInstallation(install); + try { + const sibling = await AgentInstallation.findOne({ + agentName: String(install.agentName || '').toLowerCase(), + instanceId: String(install.instanceId || 'default'), + status: 'active', + 'config.runtime.runtimeType': { $exists: true }, + }).sort({ createdAt: -1 }).lean(); + return isHostedInstallation(sibling); + } catch (err) { + // chat.mention is answered by hosted and external drivers. Failing toward + // that route keeps an addressed owner awake when classification is down. + console.warn('[task-event] runtime lookup failed:', (err as Error).message); + return true; + } +} export interface TaskEventPayload { podId: string; @@ -97,14 +153,12 @@ async function stampOfferedRevisions(revisions: OfferRevision[]): Promise } /** - * Wake the pod's agents on a board change (ADR-024 D1, "producer parity"). + * Wake a task's owning seats on a board change (ADR-024 D1). * * `emitTaskUpdated` above reaches Socket.io and stops, so a human watching the - * Kanban sees the board move and an agent installed in the same pod learns - * nothing. That asymmetry is why the fleet is 100% reactive: an agent can only - * respond to being named, because being named is the only thing that reaches - * it. Seven socket-emitting producers in this backend have zero enqueues - * between them; this makes one of them do both. + * Kanban sees the board move while the agent owning that row learns nothing. + * Seven socket-emitting producers in this backend have zero enqueues between + * them; this one also delivers an addressed wake to the row's owner. * * Three deliberate choices, each with a cheaper wrong version: * @@ -128,10 +182,9 @@ async function stampOfferedRevisions(revisions: OfferRevision[]): Promise * * So the bound has to be on DELIVERY. At most one *pending* board wake per * (agent, instance, pod): a second change folds into the undrained first - * instead of appending. 39 writes become 1 wake, and each seat looks at the - * board once rather than once per row. There is no messageId, deliberately - * — every seat SHOULD look; the per-task claim CAS already arbitrates who - * actually takes what, one layer down, where the contention really is. + * instead of appending. Only the row's current or prior owner receives that + * wake; unrelated movement remains on the durable board. There is no + * messageId because an assignment is addressed to its owner. * * This is a step toward ADR-024 D3's inbox, not a workaround around it: * "collapse what is still undelivered" is what an inbox does. @@ -168,6 +221,7 @@ export async function notifyPodAgents( agentName?: string; instanceId?: string; }, + previousTask?: BoardTask | null, ): Promise { // A deletion leaves nothing to pick up, and a tombstone wake spends a model // turn to discover there is no work. @@ -179,8 +233,6 @@ export async function notifyPodAgents( const { AgentInstallation } = require('../models/AgentRegistry'); const AgentEventService = require('./agentEventService'); const Task = require('../models/Task'); - // Lazy so this never forms a load-time cycle with the mention service. - const { boardWakeEnabled } = require('./agentMentionService'); /* eslint-enable global-require, @typescript-eslint/no-require-imports */ const normalizedPodId = String(podId); @@ -189,15 +241,8 @@ export async function notifyPodAgents( status: 'active', }).lean(); - // Gate on the SAME per-install opt-in as ADR-018 D8 wake-on-message. A - // board change is ambient pod activity that nobody addressed to you, which - // is precisely what that switch governs — so an agent whose owner turned - // ambient wakes OFF must not start receiving them through a second door. - // Measured before choosing this: of 367 active installs, 128 have it on - // (including every sprint seat), and the 169 that do not are `commonly-bot` - // and `openclaw` — user-facing and community seats where an unrequested - // wake is a noise incident, not a feature. - const installs = (active || []).filter(boardWakeEnabled); + // Ownership addresses a seat directly, independent of ambient wake opt-in. + const installs = active || []; if (!installs.length) return; const taskId = String(task.taskId || task.id || task._id || 'unknown'); @@ -253,12 +298,42 @@ export async function notifyPodAgents( const actorKey = actor?.isAgent && actor?.agentName ? `${String(actor.agentName).toLowerCase()}::${String(actor.instanceId || 'default')}` : null; + let currentHolder: string | null = null; + let previousHolder: string | null = null; + let dependentRows: BoardTask[] = []; + let dependentHolders: Array = []; + try { + [currentHolder, previousHolder] = await Promise.all([ + claimHolderKey(task.claimedBy), claimHolderKey(previousTask?.claimedBy), + ]); + if (String(task.status) === 'done' && previousTask && String(previousTask.status) !== 'done') { + dependentRows = await Task.find( + { podId: normalizedPodId, dep: taskId, status: { $in: ['pending', 'claimed', 'blocked'] } }, + { assignee: 1, claimedBy: 1 }, + ).lean(); + dependentHolders = await Promise.all(dependentRows.map((row) => claimHolderKey(row.claimedBy))); + } + } catch (err) { + // Preserve known assignees even if a claim or dependency lookup fails. + console.warn('[task-event] own-row lookup failed:', (err as Error).message); + } const outcomes = await Promise.all(installs.map(async (install: Record) => { const installKey = `${String(install.agentName || '').toLowerCase()}::${String(install.instanceId || 'default')}`; if (actorKey && installKey === actorKey) return null; const agentName = install.agentName; const instanceId = install.instanceId || 'default'; + const ownRow = seatMatchesAssignee(install, task.assignee) + || seatMatchesAssignee(install, previousTask?.assignee) + || seatHoldsClaim(install, currentHolder) + || seatHoldsClaim(install, previousHolder) + || dependentRows.some((row) => seatMatchesAssignee(install, row.assignee)) + || dependentHolders.some((holder) => seatHoldsClaim(install, holder)); + if (!ownRow) return null; try { + // Both hosted engines answer chat.mention; they acknowledge a generic + // message.posted without a turn. Other drivers use message.posted. + const eventType = await usesHostedBoardWake(install, AgentInstallation) + ? 'chat.mention' : 'message.posted'; // Assignee is the board's agent-name string. Query the current board // for this seat rather than replaying only the latest pod-wide edit: // an assignment must be visible even when another edit folds into it. @@ -266,7 +341,7 @@ export async function notifyPodAgents( let assignedRows: Array> = []; try { const assignees = instanceId === 'default' || instanceId === agentName - ? [agentName] : [agentName, instanceId]; + ? [agentName] : [instanceId]; assignedRows = await Task.find( { podId: normalizedPodId, assignee: { $in: assignees }, status: { $in: ['pending', 'claimed'] } }, { taskId: 1, title: 1, status: 1, updatedAt: 1, updates: { $slice: -1 } }, @@ -276,9 +351,9 @@ export async function notifyPodAgents( console.warn(`[task-event] assigned rows lookup failed for ${agentName}:`, (err as Error).message); } // Fold into this seat's undrained board wake if it has one. Matches on - // `payload.boardWake` rather than the event type, because the type is - // deliberately `message.posted` (shared with ordinary chat) and folding - // a board change into someone's unread MESSAGE would destroy it. + // Match `payload.boardWake`, never the event type: non-hosted wakes + // share message.posted with chat, and hosted wakes use chat.mention. + // Folding into an unread chat message would destroy it. const folded = await AgentEvent.findOneAndUpdate( { agentName, @@ -292,7 +367,11 @@ export async function notifyPodAgents( // A human edit arriving on top of agent churn must re-price the // whole batch as human — otherwise one agent write early in a sweep // permanently marks a wake the human later contributed to. - ...(dmKind === 'user-agent' ? { $set: { 'payload.dmKind': 'user-agent' } } : {}), + ...(eventType === 'chat.mention' || dmKind === 'user-agent' + ? { $set: { + ...(eventType === 'chat.mention' ? { type: 'chat.mention' } : {}), + ...(dmKind === 'user-agent' ? { 'payload.dmKind': 'user-agent' } : {}), + } } : {}), }, { new: true }, ); @@ -313,11 +392,10 @@ export async function notifyPodAgents( agentName, instanceId, podId: normalizedPodId, - type: 'message.posted', + type: eventType, payload: { content: buildContent(0, assignedRows), - // No messageId, deliberately — see choice 2. Every seat should - // look; the per-task claim CAS arbitrates who takes what. + // No messageId; ownership itself is the address. podId: normalizedPodId, boardWake: true, boardChanges: 0, diff --git a/docs/adr/ADR-018-agent-attention-claims.md b/docs/adr/ADR-018-agent-attention-claims.md index e2d88ad490..d7f4e19fa0 100644 --- a/docs/adr/ADR-018-agent-attention-claims.md +++ b/docs/adr/ADR-018-agent-attention-claims.md @@ -412,7 +412,7 @@ D8's "default **off**" becomes **on for every agent a person brings into a pod** - **Who gets the default.** A new install of an agent the person brought from the Bring your own page: the own-agent seat (`runtimeType: 'webhook'`, polling, `host: 'byo'`) and the computer placement (`'wrapper'`). - **Who doesn't.** Scout and first-party apps keep their own settings. So does `agent attach`, which keeps its explicit `--wake-on-message` flag. So do hosted agents: a hosted agent can't take an ambient turn yet, because `HOSTED_NATIVE_EVENT_TYPES` answers only `chat.mention` and `first_contact`. Hosted ambient and an in-app switch are the follow-up. - **Human-only.** A defaulted install carries `wakeOnMessage.humanOnly: true`. An agent-authored message never wakes it ambiently, nor does a sender not known to be human, so the default can't start agents answering each other. The agents still see each other's messages in the context they read when a person's message wakes them. Explicit opt-ins carry no such flag, and their delivery is unchanged. -- **Board wakes stay off.** The default also writes `boardWake.enabled: false`, because an absent `boardWake` inherits `wakeOnMessage` and would otherwise subscribe the agent to task-board wakes as a side effect. +- **Ambient board wakes stay off.** The default also writes `boardWake.enabled: false`, because an absent `boardWake` inherits `wakeOnMessage` and would otherwise subscribe the agent to unclaimed-work nudges as a side effect. A change to the agent's own row is addressed work under D8.2. - **Still a setting, not a ratchet.** An explicit `wakeOnMessage` in the install request always wins, and the owner can turn it off at any time, as D8 says. - **New installs only.** No existing installation is migrated. @@ -420,6 +420,12 @@ Implementation: `backend/services/wakeDefaultService.ts` (the rule), the registr > **Scope boundary.** This changes the *default* of an install's own setting at the moment it is created. It does **not** gate any wake on pod size, member count or workspace shape. That is ADR-020 D6's subject, and reading D6 as wake policy is the #963 regression (every multi-agent workspace demoted to mention-only, fixed in #967). Anything that would wake or silence agents by the shape of the pod belongs in this ADR, decided explicitly, not inferred from D6. +### D8.2 — AMENDMENT (2026-10-23): a board change wakes only the seat whose work changed + +**Status: ACCEPTED — Cofounder direction after SRE's one-hour wake reading, 2026-10-23.** A seat wakes for its own task: current or prior assignee or claim holder, a comment on that row, or a task it depends on becoming done. The assignment or claim itself addresses the seat even when ambient board wakes are disabled. The actor does not wake itself. Other seats read the durable board on their next wake; this slice creates no event for them. The kernel's unclaimed-work sweep keeps its existing wake policy. + +Prior ownership must be the pre-image of the successful board write. A separate read before a concurrent write can name a stale assignee and miss the seat just unassigned. The wake's `updatedAt` must equal the stored revision and advance under concurrent writes, so `offeredAt` can mark that exact revision once. Every later task write must advance the same revision, including claims, completions, comments, non-ownership edits, and source-ref reopens; a wall-clock timestamp can otherwise move it behind `offeredAt` and hide an unassigned pending row from the sweep. + ### D9 — Heartbeat shrinks; it does not die Wake-on-message replaces heartbeat's *"did anything happen?"* role in pods