From b953a024037feded1204b5ffde820049ee48b5ca Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 25 Oct 2026 02:34:19 -0700 Subject: [PATCH 1/5] feat(grants): draft per-task enforcement and close revocation Co-authored-by: vera Agent-Model: codex/gpt-6.1-sol/high Agent-Model: claude/claude-opus-5-5/default --- .../__tests__/unit/routes/grants.mint.test.js | 30 ++++ .../routes/tasksApi.taskGrantClose.test.js | 86 ++++++++++ .../services/kernelWorkSweepService.test.js | 1 + .../services/roomGrantTaskBinding.test.js | 153 ++++++++++++++++++ .../unit/services/taskBoundBroker.test.js | 71 ++++++++ backend/models/RoomGrant.ts | 12 ++ backend/models/Task.ts | 7 + backend/routes/grants.ts | 15 +- backend/routes/tasksApi.ts | 40 ++++- backend/services/githubGigService.ts | 5 +- .../services/grantBrokerProjectionService.ts | 22 ++- backend/services/kernelWorkSweepService.ts | 1 + backend/services/roomGrantService.ts | 126 ++++++++++++++- backend/services/starterTaskService.ts | 4 +- docs/plans/task-bound-grants.md | 68 ++++++++ 15 files changed, 627 insertions(+), 14 deletions(-) create mode 100644 backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js create mode 100644 backend/__tests__/unit/services/roomGrantTaskBinding.test.js create mode 100644 backend/__tests__/unit/services/taskBoundBroker.test.js create mode 100644 docs/plans/task-bound-grants.md diff --git a/backend/__tests__/unit/routes/grants.mint.test.js b/backend/__tests__/unit/routes/grants.mint.test.js index a50b49e022..cc6d8cf7ce 100644 --- a/backend/__tests__/unit/routes/grants.mint.test.js +++ b/backend/__tests__/unit/routes/grants.mint.test.js @@ -409,3 +409,33 @@ describe('POST /api/grants on a hosted-mcp connection', () => { expect(await RoomGrant.countDocuments({})).toBe(0); }); }); + +describe('task-bound mint', () => { + const Task = require('../../../models/Task'); + beforeEach(async () => { + await Task.deleteMany({}); + await Task.create({ podId: POD, taskNum: 1, taskId: 'TASK-1', title: 'Task-bound grant', status: 'pending' }); + }); + test('stores the authorized task tuple and ignores caller epoch values', async () => { + const res = await mint({ binding: { taskId: 'TASK-1', podId: POD }, taskEpoch: 999 }); + expect(res.status).toBe(201); + expect(res.body.binding).toEqual({ taskId: 'TASK-1', podId: POD }); + expect(res.body.taskEpoch).toBe(0); + }); + test.each([ + { taskId: 'TASK-1', podId: 'aaaaaaaaaaaaaaaaaaaaaa02' }, + { taskId: 'TASK-404', podId: POD }, + { taskId: 'TASK-1' }, + ])('rejects foreign or missing task context without saving: %p', async (binding) => { + const res = await mint({ binding }); + expect([400, 404]).toContain(res.status); + expect(await RoomGrant.countDocuments({})).toBe(0); + }); + test('rejects a closed task', async () => { + await Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $set: { status: 'done' } }); + const res = await mint({ binding: { taskId: 'TASK-1', podId: POD } }); + expect(res.status).toBe(403); + expect(res.body.error).toBe('task_closed'); + expect(await RoomGrant.countDocuments({})).toBe(0); + }); +}); diff --git a/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js b/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js new file mode 100644 index 0000000000..05119adaa4 --- /dev/null +++ b/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js @@ -0,0 +1,86 @@ +const express = require('express'); +const request = require('supertest'); +const mongoose = require('mongoose'); +const { setupMongoDb, clearMongoDb, closeMongoDb } = require('../../utils/testUtils'); +const mockActor = { id: new mongoose.Types.ObjectId().toString(), bot: false }; +jest.mock('../../../middleware/auth', () => (req, _res, next) => { + req.userId = mockActor.id; + req.user = { _id: mockActor.id, isBot: mockActor.bot, botMetadata: { instanceId: 'default' } }; + next(); +}); +jest.mock('../../../middleware/agentRuntimeAuth', () => (req, _res, next) => next()); +jest.mock('../../../services/taskEventService', () => ({ emitTaskUpdated: jest.fn(), notifyPodAgents: jest.fn(async () => {}) })); +jest.mock('../../../services/attentionItemService', () => ({ resolveTaskAttention: jest.fn(async () => {}), recordTaskAttention: jest.fn(async () => {}) })); +jest.mock('../../../services/systemExchangeTriggers', () => ({ recordTaskCompleted: jest.fn(async () => {}) })); +jest.mock('../../../services/agentSendFreshness', () => ({ unseenMessages: jest.fn(async () => null) })); +const Task = require('../../../models/Task'); +const Pod = require('../../../models/Pod'); +const RoomGrant = require('../../../models/RoomGrant'); +const { mintGrant, assertGrantUsable } = require('../../../services/roomGrantService'); +const { completeTaskForMergedPull } = require('../../../services/githubGigService'); +const { completeConnectAgentStarterTask, CONNECT_AGENT_SOURCE_REF } = require('../../../services/starterTaskService'); +const router = require('../../../routes/tasksApi'); +const POD = new mongoose.Types.ObjectId().toString(); +let app; +let grant; +beforeAll(async () => { await setupMongoDb(); app = express(); app.use(express.json()); app.use('/tasks', router); }); +afterEach(clearMongoDb); +afterAll(closeMongoDb); +beforeEach(async () => { + mockActor.bot = false; + await Pod.create({ _id: POD, name: 'Bound task', createdBy: mockActor.id, members: [mockActor.id] }); + await Task.create({ podId: POD, taskId: 'TASK-1', taskNum: 1, title: 'Bound task', status: 'claimed', + claimedBy: mockActor.id, claimExpiresAt: new Date(Date.now() + 60000), + prUrl: 'https://github.com/Team-Commonly/commonly/pull/123', sourceRef: CONNECT_AGENT_SOURCE_REF }); + grant = await mintGrant({ connectionId: 'c', installationId: 'i', target: { kind: 'pod', id: POD }, + tools: ['issues.read'], audience: [mockActor.id], writeMode: 'read', brokerId: 'b', + expiresAt: new Date(Date.now() + 3600000), binding: { podId: POD, taskId: 'TASK-1' } }); +}); + +test.each(['complete', 'patch', 'github', 'onboarding'])('%s close revokes the bound grant before returning', async (writer) => { + if (writer === 'complete') expect((await request(app).post(`/tasks/${POD}/TASK-1/complete`).send({})).status).toBe(200); + if (writer === 'patch') expect((await request(app).patch(`/tasks/${POD}/TASK-1`).send({ status: 'done' })).status).toBe(200); + if (writer === 'github') await completeTaskForMergedPull(POD, 'https://github.com/Team-Commonly/commonly/pull/123'); + if (writer === 'onboarding') await completeConnectAgentStarterTask({ podIds: [POD], agentLabel: 'Scout' }); + const task = await Task.findOne({ podId: POD, taskId: 'TASK-1' }); + expect(task.status).toBe('done'); + expect(task.grantEpoch).toBe(1); + const revoked = await RoomGrant.findById(grant._id); + expect(revoked.revocationReason).toBe('task.closed'); + expect(revoked.revokedAt).toBeInstanceOf(Date); + await expect(assertGrantUsable({ grantId: grant.grantId, agentUserId: mockActor.id, currentMemberIds: [mockActor.id] })) + .rejects.toMatchObject({ code: 'grant_revoked' }); +}); + +test('claim stamps authenticated identity and refuses renewal by a different User sharing its display key', async () => { + await Task.updateOne({ taskId: 'TASK-1' }, { $set: { status: 'pending' } }); + mockActor.bot = true; + const first = await request(app).post(`/tasks/${POD}/TASK-1/claim`).send({ continueAnyway: true }); + expect(first.status).toBe(200); + expect(first.body.task.claimedBy).toBe('default'); + expect(first.body.task.claimedByUserId).toBe(mockActor.id); + await Task.updateOne({ taskId: 'TASK-1' }, { $set: { claimedByUserId: new mongoose.Types.ObjectId().toString() } }); + const peer = await request(app).post(`/tasks/${POD}/TASK-1/claim`).send({ continueAnyway: true }); + expect(peer.status).toBe(409); +}); + +test('status-only release and reclaim cannot reuse the previous authenticated principal', async () => { + await Task.updateOne({ taskId: 'TASK-1' }, { $set: { claimedByUserId: mockActor.id } }); + expect((await request(app).patch(`/tasks/${POD}/TASK-1`).send({ status: 'pending' })).status).toBe(200); + expect((await Task.findOne({ taskId: 'TASK-1' })).claimedByUserId).toBeNull(); + expect((await request(app).patch(`/tasks/${POD}/TASK-1`).send({ status: 'claimed', claimedByUserId: mockActor.id })).status).toBe(200); + expect((await Task.findOne({ taskId: 'TASK-1' })).claimedByUserId).toBeNull(); + await expect(assertGrantUsable({ grantId: grant.grantId, agentUserId: mockActor.id, currentMemberIds: [mockActor.id] })) + .rejects.toMatchObject({ code: 'task_claim_required' }); +}); + + +test('legacy display-key renewal stays compatible without granting authenticated task authority', async () => { + mockActor.bot = true; + await Task.updateOne({ taskId: 'TASK-1' }, { $set: { claimedBy: 'default', claimedByUserId: null } }); + const renewed = await request(app).post(`/tasks/${POD}/TASK-1/claim`).send({ continueAnyway: true }); + expect(renewed.status).toBe(200); + expect(renewed.body.task.claimedByUserId).toBeNull(); + await expect(assertGrantUsable({ grantId: grant.grantId, agentUserId: mockActor.id, currentMemberIds: [mockActor.id] })) + .rejects.toMatchObject({ code: 'task_claim_required' }); +}); diff --git a/backend/__tests__/unit/services/kernelWorkSweepService.test.js b/backend/__tests__/unit/services/kernelWorkSweepService.test.js index 5c1a9d371c..3b6516442e 100644 --- a/backend/__tests__/unit/services/kernelWorkSweepService.test.js +++ b/backend/__tests__/unit/services/kernelWorkSweepService.test.js @@ -278,6 +278,7 @@ describe('#1080 part 3 — provenance survives the clearing', () => { // as unclaimed work. lapsedFrom is the record that outlives it. expect(update.$set.assignee).toBeNull(); expect(update.$set.claimedBy).toBeNull(); + expect(update.$set.claimedByUserId).toBeNull(); expect(update.$set.lapsedFrom).toBe('sprint-impl'); expect(update.$push.updates.text).toContain('was: sprint-impl'); }); diff --git a/backend/__tests__/unit/services/roomGrantTaskBinding.test.js b/backend/__tests__/unit/services/roomGrantTaskBinding.test.js new file mode 100644 index 0000000000..85b3d316a1 --- /dev/null +++ b/backend/__tests__/unit/services/roomGrantTaskBinding.test.js @@ -0,0 +1,153 @@ +const { setupMongoDb, clearMongoDb, closeMongoDb } = require('../../utils/testUtils'); +const mongoose = require('mongoose'); +const RoomGrant = require('../../../models/RoomGrant'); +const Task = require('../../../models/Task'); +const Pod = require('../../../models/Pod'); +const service = require('../../../services/roomGrantService'); + +const POD = new mongoose.Types.ObjectId().toString(); +const OTHER_POD = new mongoose.Types.ObjectId().toString(); +const AGENT = new mongoose.Types.ObjectId().toString(); +const binding = { taskId: 'TASK-1', podId: POD }; +const input = (overrides = {}) => ({ + connectionId: 'connection', installationId: 'installation', target: { kind: 'pod', id: POD }, + tools: ['issues.read'], writeMode: 'read', audience: [AGENT], + expiresAt: new Date(Date.now() + 3600000), brokerId: 'broker', binding, ...overrides, +}); +const use = (grant, overrides = {}) => service.assertGrantUsable({ + grantId: grant.grantId, agentUserId: AGENT, currentMemberIds: [AGENT], + ...overrides, +}); + +beforeAll(setupMongoDb); +afterEach(clearMongoDb); +afterAll(closeMongoDb); + +beforeEach(async () => { + await Pod.create({ _id: POD, name: 'Task pod', createdBy: AGENT, members: [AGENT] }); + await Task.create({ podId: POD, taskNum: 1, taskId: 'TASK-1', title: 'Investigate issue', status: 'claimed', + claimedBy: AGENT, claimExpiresAt: new Date(Date.now() + 1800000) }); +}); + +test('unbound grants retain their audience and expiry behavior without task context', async () => { + const grant = await service.mintGrant(input({ binding: undefined })); + await expect(use(grant)).resolves.toBeTruthy(); +}); + +test.each([ + { taskId: 'TASK-404', podId: POD }, + { taskId: 'TASK-1', podId: OTHER_POD }, + { taskId: 'TASK-1' }, + { taskId: '', podId: POD }, +])('invalid or foreign task binding cannot mint a grant: %p', async (invalid) => { + await expect(service.mintGrant(input({ binding: invalid }))).rejects.toMatchObject({ code: expect.any(String) }); + expect(await RoomGrant.countDocuments()).toBe(0); +}); + +test('closed tasks cannot mint and a stale read cannot make a grant usable after close/reopen', async () => { + const grant = await service.mintGrant(input()); + await Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $set: { status: 'done' }, $inc: { grantEpoch: 1 } }); + await expect(use(grant)).rejects.toMatchObject({ code: 'task_closed' }); + await expect(service.mintGrant(input())).rejects.toMatchObject({ code: 'task_closed' }); + await Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $set: { status: 'claimed' } }); + await expect(use(grant)).rejects.toMatchObject({ code: 'task_closed' }); +}); + +test('a task binding is inherited by attenuation and expiry remains a ceiling', async () => { + const parent = await service.mintGrant(input()); + const child = await service.attenuateGrant({ parentGrantId: parent.grantId, expiresAt: new Date(Date.now() + 7200000) }); + expect(child.binding.toObject()).toEqual(binding); + expect(child.taskEpoch).toBe(parent.taskEpoch); + expect(child.expiresAt.getTime()).toBe(parent.expiresAt.getTime()); + await expect(service.attenuateGrant({ parentGrantId: parent.grantId, binding: null })) + .rejects.toMatchObject({ code: 'grant_not_attenuated' }); + await expect(service.attenuateGrant({ parentGrantId: parent.grantId, binding: { taskId: 'TASK-2', podId: POD } })) + .rejects.toMatchObject({ code: 'grant_not_attenuated' }); +}); + +test('the close hook revokes bound descendants, once, without touching another pod or unbound grants', async () => { + const parent = await service.mintGrant(input()); + const child = await service.attenuateGrant({ parentGrantId: parent.grantId }); + const unbound = await service.mintGrant(input({ binding: undefined })); + await service.revokeTaskGrants({ ...binding, closedEpoch: 1 }); + expect((await RoomGrant.findById(parent._id)).revokedAt).toBeInstanceOf(Date); + expect((await RoomGrant.findById(child._id)).revocationReason).toBe('task.closed'); + expect((await RoomGrant.findById(unbound._id)).revokedAt).toBeNull(); + await expect(service.revokeTaskGrants({ ...binding, closedEpoch: 1 })).resolves.toBe(0); +}); + + +test('only the authenticated current claimant may use a task-bound grant', async () => { + const grant = await service.mintGrant(input()); + await expect(use(grant)).resolves.toBeTruthy(); + await Task.updateOne({ taskId: binding.taskId }, { $set: { claimedBy: 'another-agent' } }); + await expect(use(grant)).rejects.toMatchObject({ code: 'task_claim_required' }); +}); + +test.each(['pending', 'blocked', 'lapsed', 'missing'])('refuses a %s claim before tool use', async (state) => { + const grant = await service.mintGrant(input()); + if (state === 'missing') await Task.deleteOne({ taskId: binding.taskId }); + else await Task.updateOne({ taskId: binding.taskId }, state === 'lapsed' + ? { $set: { claimExpiresAt: new Date(Date.now() - 1) } } + : { $set: { status: state } }); + await expect(use(grant)).rejects.toMatchObject({ code: state === 'missing' ? 'task_not_found' : 'task_claim_required' }); +}); + +test('closing one pod never revokes a same-number task in another pod or a post-reopen grant', async () => { + const oldGrant = await service.mintGrant(input()); + await Task.updateOne({ taskId: binding.taskId, podId: POD }, { $inc: { grantEpoch: 1 } }); + const newGrant = await service.mintGrant(input()); + await Task.create({ podId: OTHER_POD, taskNum: 1, taskId: 'TASK-1', title: 'Other pod task', status: 'claimed', + claimedBy: AGENT, claimExpiresAt: new Date(Date.now() + 60000) }); + const otherGrant = await service.mintGrant(input({ target: { kind: 'pod', id: OTHER_POD }, binding: { taskId: 'TASK-1', podId: OTHER_POD } })); + await service.revokeTaskGrants({ ...binding, closedEpoch: 1 }); + expect((await RoomGrant.findById(oldGrant._id)).revokedAt).toBeInstanceOf(Date); + expect((await RoomGrant.findById(newGrant._id)).revokedAt).toBeNull(); + expect((await RoomGrant.findById(otherGrant._id)).revokedAt).toBeNull(); +}); + + +test('a display-name claim alone is never treated as an authenticated identity', async () => { + const grant = await service.mintGrant(input()); + await Task.updateOne({ taskId: binding.taskId }, { $set: { claimedBy: 'default' } }); + await expect(use(grant)).rejects.toMatchObject({ code: 'task_claim_required' }); + await Task.updateOne({ taskId: binding.taskId }, { $set: { claimedByUserId: AGENT } }); + await expect(use(grant)).resolves.toBeTruthy(); +}); + +test('expiry still refuses a bound grant while its task and claim remain live', async () => { + const grant = await service.mintGrant(input()); + await RoomGrant.updateOne({ _id: grant._id }, { $set: { expiresAt: new Date(Date.now() - 1) } }); + await expect(use(grant)).rejects.toMatchObject({ code: 'grant_expired' }); +}); + +test('a close and reopen racing grant insertion cannot hand out the old capability', async () => { + const create = RoomGrant.create.bind(RoomGrant); + const spy = jest.spyOn(RoomGrant, 'create').mockImplementationOnce(async (...args) => { + const row = await create(...args); + // The close hook cannot have seen this insertion; the epoch still fences it. + await Task.updateOne({ podId: POD, taskId: binding.taskId }, { $set: { status: 'done' }, $inc: { grantEpoch: 1 } }); + await Task.updateOne({ podId: POD, taskId: binding.taskId }, { $set: { status: 'claimed' } }); + return row; + }); + await expect(service.mintGrant(input())).rejects.toMatchObject({ code: 'task_closed' }); + spy.mockRestore(); + const row = await RoomGrant.findOne({ 'binding.taskId': binding.taskId }); + expect(row.revokedAt).toBeInstanceOf(Date); +}); + +test('an unbound parent can be narrowed to a task without revoking the parent at close', async () => { + const parent = await service.mintGrant(input({ binding: undefined })); + const child = await service.attenuateGrant({ parentGrantId: parent.grantId, binding }); + await service.revokeTaskGrants({ ...binding, closedEpoch: 1 }); + await expect(use(child)).rejects.toMatchObject({ code: 'grant_revoked' }); + await expect(use(parent)).resolves.toBeTruthy(); +}); + + +test('seat bindings canonicalize pod ObjectIds so close revocation cannot miss an uppercase tuple', async () => { + const grant = await service.mintGrant(input({ target: { kind: 'seat', id: AGENT }, + binding: { taskId: binding.taskId, podId: POD.toUpperCase() } })); + expect(grant.binding.podId).toBe(POD); + await expect(service.revokeTaskGrants({ ...binding, closedEpoch: 1 })).resolves.toBe(1); +}); diff --git a/backend/__tests__/unit/services/taskBoundBroker.test.js b/backend/__tests__/unit/services/taskBoundBroker.test.js new file mode 100644 index 0000000000..decba6b50b --- /dev/null +++ b/backend/__tests__/unit/services/taskBoundBroker.test.js @@ -0,0 +1,71 @@ +/** Real grant/task authority across catalogue, call and approved-call paths. + * Only provider I/O, the PG trail and unrelated confinement are mocked. */ +const { setupMongoDb, clearMongoDb, closeMongoDb } = require('../../utils/testUtils'); +const mongoose = require('mongoose'); +const mockProvider = { listOpenIssues: jest.fn(), createIssue: jest.fn() }; +const mockTrail = { create: jest.fn(async () => {}) }; +jest.mock('../../../services/githubAppService', () => mockProvider); +jest.mock('../../../models/ToolCall', () => ({ + ...jest.requireActual('../../../models/ToolCall'), + create: (...args) => mockTrail.create(...args), +})); +jest.mock('../../../models/Integration', () => ({ findOne: jest.fn(async () => ({ + type: 'github-app', status: 'connected', createdBy: 'owner', createdAt: new Date('2020-01-01'), + config: { installationId: 'fixture', owner: 'fixture', repo: 'fixture' }, +})) })); +jest.mock('../../../services/seatGrantConfinement', () => ({ + judgeSeatConfinement: jest.fn(async () => ({ refusal: null, scope: 'unbound' })), +})); +jest.mock('../../../services/outsiderGrantPodEligibility', () => ({ + outsiderGrantPodRefusal: jest.fn(async () => null), outsiderGrantRefusalMessage: jest.fn(), +})); +const Task = require('../../../models/Task'); +const Pod = require('../../../models/Pod'); +const { mintGrant } = require('../../../services/roomGrantService'); +const { callTool, listToolsForGrant, executeApprovedToolCall } = require('../../../services/toolBrokerService'); +const { selectLiveGrantsForIdentities } = require('../../../services/grantBrokerProjectionService'); +const { GRANT_BROKER_ID } = require('../../../services/installable/toolInstallables'); +const { digestArgs } = require('../../../models/ToolCall'); +const POD = new mongoose.Types.ObjectId().toString(); +const AGENT = new mongoose.Types.ObjectId().toString(); +let grant; +beforeAll(setupMongoDb); +afterEach(clearMongoDb); +afterAll(closeMongoDb); +beforeEach(async () => { + jest.clearAllMocks(); + mockProvider.listOpenIssues.mockResolvedValue([]); + await Pod.create({ _id: POD, name: 'Broker fixture', createdBy: AGENT, members: [AGENT] }); + await Task.create({ podId: POD, taskNum: 1, taskId: 'TASK-1', title: 'Bound work', status: 'claimed', + claimedBy: AGENT, claimedByUserId: AGENT, claimExpiresAt: new Date(Date.now() + 60000) }); + grant = await mintGrant({ connectionId: 'fixture', installationId: 'fixture', target: { kind: 'pod', id: POD }, + tools: ['github.list_issues', 'github.create_issue'], writeMode: 'write-with-confirm', audience: [AGENT], + expiresAt: new Date(Date.now() + 3600000), brokerId: GRANT_BROKER_ID, binding: { podId: POD, taskId: 'TASK-1' } }); +}); +const caller = () => ({ grantId: grant.grantId, agentUserId: AGENT }); + +test('a live authenticated claimant receives tools and can execute an allowed read', async () => { + expect((await selectLiveGrantsForIdentities({ identityIds: [AGENT], podIds: [POD] })).get(AGENT)).toHaveLength(1); + const result = await callTool({ ...caller(), tool: 'github.list_issues', args: {} }); + expect(result.outcome).toBe('ok'); + expect(mockProvider.listOpenIssues).toHaveBeenCalledTimes(1); +}); + +test('a task claimed by somebody else disappears from projection and list, and cannot reach the provider', async () => { + await Task.updateOne({ taskId: 'TASK-1' }, { $set: { claimedByUserId: new mongoose.Types.ObjectId().toString() } }); + expect((await selectLiveGrantsForIdentities({ identityIds: [AGENT], podIds: [POD] })).size).toBe(0); + await expect(listToolsForGrant(caller())).rejects.toMatchObject({ code: 'task_claim_required' }); + await expect(callTool({ ...caller(), tool: 'github.list_issues', args: {} })) + .rejects.toMatchObject({ code: 'task_claim_required' }); + expect(mockProvider.listOpenIssues).not.toHaveBeenCalled(); + expect(mockTrail.create).toHaveBeenCalledWith(expect.objectContaining({ outcome: 'refused', reason: 'task_claim_required' })); +}); + +test('an approval captured before close cannot execute after close, even if revocation cleanup missed the grant', async () => { + const args = { title: 'Approved fixture' }; + const approved = { ...caller(), tool: 'github.create_issue', args, expectedArgsDigest: digestArgs(args), approvalId: 'fixture' }; + await Task.updateOne({ taskId: 'TASK-1' }, { $set: { status: 'done' }, $inc: { grantEpoch: 1 } }); + await expect(executeApprovedToolCall(approved)).rejects.toMatchObject({ code: 'task_closed' }); + expect(mockProvider.createIssue).not.toHaveBeenCalled(); + expect(mockTrail.create).toHaveBeenCalledWith(expect.objectContaining({ outcome: 'refused', reason: 'task_closed', approvalId: 'fixture' })); +}); diff --git a/backend/models/RoomGrant.ts b/backend/models/RoomGrant.ts index f4176525df..d08527b687 100644 --- a/backend/models/RoomGrant.ts +++ b/backend/models/RoomGrant.ts @@ -21,7 +21,12 @@ export interface IRoomGrantTarget { id: string; } +export interface RoomGrantTaskBinding { taskId: string; podId: string } + export interface IRoomGrant extends Document { + binding?: RoomGrantTaskBinding; + taskEpoch?: number; + revocationReason?: string; grantId: string; connectionId: string; installationId: string; @@ -82,6 +87,12 @@ const RoomGrantSchema = new Schema( // `grantId` is the public, opaque identifier used by the MCP endpoint; // Mongo's _id remains an implementation detail. grantId: { type: String, required: true, unique: true, trim: true }, + binding: { + type: new Schema({ taskId: { type: String, required: true }, podId: { type: String, required: true } }, { _id: false }), + default: undefined, + }, + taskEpoch: { type: Number, min: 0 }, + revocationReason: { type: String }, connectionId: { type: String, required: true, trim: true }, installationId: { type: String, required: true, trim: true }, target: { type: GrantTargetSchema, required: true }, @@ -108,6 +119,7 @@ const RoomGrantSchema = new Schema( ); RoomGrantSchema.index({ target: 1, revokedAt: 1, expiresAt: 1 }); +RoomGrantSchema.index({ 'binding.podId': 1, 'binding.taskId': 1, taskEpoch: 1, revokedAt: 1 }); RoomGrantSchema.index({ parentGrantId: 1 }); RoomGrantSchema.index({ rootGrantId: 1 }); RoomGrantSchema.index({ connectionId: 1, installationId: 1 }); diff --git a/backend/models/Task.ts b/backend/models/Task.ts index 90eaeb85f2..9d9d15423b 100644 --- a/backend/models/Task.ts +++ b/backend/models/Task.ts @@ -14,6 +14,8 @@ export interface ITaskUpdate { export interface ITask extends Document { podId: Types.ObjectId; taskNum: number; + /** Monotonic close revision; reopening never restores prior grants. */ + grantEpoch?: number; taskId: string; title: string; priority: TaskPriority; @@ -25,6 +27,8 @@ export interface ITask extends Document { parentTask?: string | null; status: TaskStatus; claimedBy?: string | null; + /** Authenticated principal, separate from the display/lease-holder key. */ + claimedByUserId?: string | null; claimedAt?: Date | null; // ADR-018 D4: a claim is a lease, never permanent. Null on legacy claims — // readers derive their effective expiry from claimedAt + the route's lease. @@ -55,6 +59,7 @@ const TaskSchema = new Schema( { podId: { type: Schema.Types.ObjectId, required: true, ref: 'Pod' }, taskNum: { type: Number, required: true }, + grantEpoch: { type: Number, default: 0, min: 0 }, taskId: { type: String, required: true }, title: { type: String, required: true }, priority: { type: String, enum: ['P0', 'P1', 'P2', 'P3'], default: 'P2' }, @@ -71,6 +76,8 @@ const TaskSchema = new Schema( default: 'pending', }, claimedBy: { type: String, default: null }, + // Missing identifies pre-authenticated-principal claims; null explicitly clears authority. + claimedByUserId: { type: String }, claimedAt: { type: Date, default: null }, claimExpiresAt: { type: Date, default: null }, // No default: missing means this revision has not been offered. diff --git a/backend/routes/grants.ts b/backend/routes/grants.ts index 414cc7128d..99b8b3af36 100644 --- a/backend/routes/grants.ts +++ b/backend/routes/grants.ts @@ -16,6 +16,7 @@ import { attenuateGrant, createGrant, effectiveAudience, + normalizeTaskBinding, revokeGrant, RoomGrantError, } from '../services/roomGrantService'; @@ -82,7 +83,7 @@ interface AuthenticatedRequest extends express.Request { * broker's business; the raw `audience` snapshot still names agents who have * left, so only the effective audience goes out. */ -type GrantRow = Pick & { audience?: string[]; connectionId?: string }; const projectGrant = ( @@ -99,6 +100,8 @@ const projectGrant = ( budget: grant.budget ? { calls: grant.budget.calls, windowMs: grant.budget.windowMs } : null, effectiveAudience: effectiveAudience(grant, currentMembers), expiresAt: grant.expiresAt, + ...(grant.binding ? { binding: grant.binding } : {}), + ...(grant.revocationReason ? { revocationReason: grant.revocationReason } : {}), revokedAt: grant.revokedAt ?? null, revokedBy: grant.revokedBy ?? null, parentGrantId: grant.parentGrantId ?? null, @@ -405,7 +408,12 @@ router.post('/', grantRateLimit, auth, async (req: AuthenticatedRequest, res: ex }); } + const binding = normalizeTaskBinding(body.binding); + if (binding && target.kind === 'seat') { + await ensureTargetAccess({ kind: 'pod', id: binding.podId }, userId); + } const grantInput: RoomGrantCreateInput = { + binding, connectionId, installationId, target, @@ -454,8 +462,13 @@ router.post( }); } const body = (req.body || {}) as Record; + const binding = body.binding === null ? null : normalizeTaskBinding(body.binding); + if (binding && parent.target.kind === 'seat') { + await ensureTargetAccess({ kind: 'pod', id: binding.podId }, agentId); + } const child = await attenuateGrant({ parentGrantId: parent.grantId, + binding, tools: body.tools as string[] | undefined, writeMode: body.writeMode as any, budget: body.budget as any, diff --git a/backend/routes/tasksApi.ts b/backend/routes/tasksApi.ts index 7556046c17..67c7ffc688 100644 --- a/backend/routes/tasksApi.ts +++ b/backend/routes/tasksApi.ts @@ -1,4 +1,5 @@ import { requirePodMember } from '../services/podMembershipService'; +import { onTaskClosed } from '../services/roomGrantService'; import { compareTaskPriority, isTaskPriority } from '../utils/taskPriority'; import { createHash } from 'crypto'; import type { Request, Response } from 'express'; @@ -352,7 +353,7 @@ router.post('/:podId', rateLimit({ { _id: existing._id, status: 'done', updatedAt: existing.updatedAt }, { $set: { - status: 'pending', assignee: assignee || null, claimedAt: null, + status: 'pending', assignee: assignee || null, claimedByUserId: null, claimedAt: null, claimExpiresAt: null, notes: 'Reopened — the same source is active again.', updatedAt: writeTime, }, @@ -637,22 +638,38 @@ router.post('/:podId/:taskId/claim', rateLimit({ // 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 // holder of a hot task gets rescued on its first lapse. - const update = { $set: { status: 'claimed', claimedBy, claimedAt: now, claimExpiresAt: new Date(now.getTime() + TASK_CLAIM_LEASE_MS), rescueDeferrals: 0, lapsedFrom: null }, $push: { updates: { text: `Claimed by ${author}`, author, authorId: userId?.toString() || null, createdAt: now } } }; - // One CAS, five ways to win: the task is unclaimed; the caller already + const update = { $set: { status: 'claimed', claimedBy, claimedByUserId: userId?.toString() || null, claimedAt: now, claimExpiresAt: new Date(now.getTime() + TASK_CLAIM_LEASE_MS), rescueDeferrals: 0, lapsedFrom: null }, $push: { updates: { text: `Claimed by ${author}`, author, authorId: userId?.toString() || null, createdAt: now } } }; + // The primary CAS stamps authenticated ownership. Five ways to win: the task is unclaimed; the caller already // holds it (renewal); the holder's lease lapsed; the claim predates // leases entirely (claimExpiresAt null) and is older than one lease; or // 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 result = await updateTaskWithRevisionSnapshot({ + let result = await updateTaskWithRevisionSnapshot({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), taskId, // A blocked row is deliberately resumable only when a seat names its // task id. Do not add it to claimableConditions: that predicate also // drives the discovery query, where a parked dependency is not work a // peer may pick up speculatively. - $or: [...claimableConditions(now, claimedBy), { status: 'blocked' }], + $or: [ + ...claimableConditions(now), + { status: 'claimed', claimedBy, $or: [ + { claimedByUserId: userId?.toString() }, + // Legacy runtime-bearer claims already used this immutable ID. + { claimedByUserId: null, claimedBy: userId?.toString() }, + ] }, + { status: 'blocked' }, + ], }, update); + if (!result && claimedBy !== userId?.toString()) { + // Preserve name-only legacy renewal, but never promote a display key to + // authenticated grant authority. A fresh claim after release/expiry does. + result = await updateTaskWithRevisionSnapshot({ + podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), taskId, + status: 'claimed', claimedBy, claimedByUserId: null, + }, { ...update, $set: { ...update.$set, claimedByUserId: null } }); + } 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' }); @@ -725,13 +742,13 @@ router.post('/:podId/:taskId/complete', taskWriteRateLimit(30), auth, async (req if (refusal) return res.status(409).json(refusal); 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 update = { $inc: { grantEpoch: 1 }, $set: { status: 'done', claimedByUserId: null, completedAt: now, ...(prUrl && { prUrl }), ...(notes && { notes }) }, $push: { updates: { text: updateText, author, authorId: userId?.toString() || null, createdAt: now } } }; const task = await updateTaskWithRevision({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), taskId, status: { $in: ['claimed', 'pending'] }, ...writableLeaseCondition(req, userId, now), - }, update) as { taskId?: string; updates?: unknown[] } | null; + }, update) as { podId?: unknown; taskId?: string; status?: string; grantEpoch?: number; updates?: unknown[] } | null; if (!task) { const racedRefusal = await leaseRefusalForWrite(podId || '', taskId || '', req, userId); if (racedRefusal) return res.status(409).json(racedRefusal); @@ -770,6 +787,7 @@ router.post('/:podId/:taskId/complete', taskWriteRateLimit(30), auth, async (req // PATCH gate already does this on a status change; this route wrote // `done` without it, and 11 of 12 open handoffs on 2026-09-19 pointed at // tasks completed here. + await onTaskClosed(task); await resolveTaskAttention(task); emitTaskUpdated(podId, task, 'updated'); recordTaskTransition({ podId, task, from: null, to: 'done', at: now, doneBy: userId }); @@ -954,6 +972,8 @@ router.post('/:podId/:taskId/updates', taskWriteRateLimit(60), auth, async (req: ...priorityUpdate, status: 'claimed', claimedBy: claimKey, + // Legacy restoration matches display provenance, not an authenticated holder. + claimedByUserId: null, claimedAt: now, claimExpiresAt: new Date(now.getTime() + TASK_CLAIM_LEASE_MS), rescueDeferrals: 0, @@ -1062,6 +1082,7 @@ router.patch('/:podId/:taskId', taskWriteRateLimit(60), auth, async (req: AuthRe }); } fieldUpdates.status = normalized; + if (normalized !== 'claimed') fieldUpdates.claimedByUserId = null; } const access = await requirePodMember(podId || '', userId, { write: true }); if (access.error) return res.status(access.status || 500).json({ error: access.error }); @@ -1087,6 +1108,7 @@ router.patch('/:podId/:taskId', taskWriteRateLimit(60), auth, async (req: AuthRe const update: Record = { $set: writeTime ? { ...fieldUpdates, updatedAt: writeTime } : fieldUpdates, }; + if (fieldUpdates.status === 'done') update.$inc = { grantEpoch: 1 }; if (changeParts.length > 0) update.$push = { updates: { text: `${author} updated: ${changeParts.join(', ')}`, author, authorId: userId?.toString() || null, createdAt: new Date() } }; const taskFilter = { podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), @@ -1142,6 +1164,10 @@ router.patch('/:podId/:taskId', taskWriteRateLimit(60), auth, async (req: AuthRe task.updatedAt = writeTime; } if (fieldUpdates.status === 'blocked') await recordTaskAttention(task, { includeBlocked: true }); + if (fieldUpdates.status === 'done') { + task.grantEpoch = (task.grantEpoch || 0) + 1; + await onTaskClosed(task); + } if (fieldUpdates.status !== undefined && fieldUpdates.status !== 'blocked') await resolveTaskAttention(task); emitTaskUpdated(podId, task, 'updated'); if (fieldUpdates.status !== undefined && previousTask && writeTime) { diff --git a/backend/services/githubGigService.ts b/backend/services/githubGigService.ts index b90f5d5f05..b6371f125b 100644 --- a/backend/services/githubGigService.ts +++ b/backend/services/githubGigService.ts @@ -1,6 +1,7 @@ /** GitHub App activity projected into an explicitly bound pod. */ import { Types } from 'mongoose'; import Task from '../models/Task'; +import { onTaskClosed } from './roomGrantService'; import GithubGigBinding from '../models/GithubGigBinding'; import { MERGE_EVIDENCE_TEXT } from './initiativeStatus'; @@ -89,12 +90,14 @@ export const completeTaskForMergedPull = async (podId: string, href: string): Pr // eslint-disable-next-line no-await-in-loop const task = await Task.findOneAndUpdate( { podId: new Types.ObjectId(podId), prUrl: href, status: { $in: ['pending', 'claimed'] } }, - { $set: { status: 'done', completedAt: at }, $push: { updates: { + { $inc: { grantEpoch: 1 }, $set: { status: 'done', claimedByUserId: null, completedAt: at }, $push: { updates: { text: MERGE_EVIDENCE_TEXT, author: 'system', authorId: null, createdAt: at, } } }, { new: true }, ); if (!task) return; + // eslint-disable-next-line no-await-in-loop + await onTaskClosed(task); emitTaskUpdated(podId, task, 'updated'); recordTaskTransition({ podId, task, from: null, to: 'done', at }); } diff --git a/backend/services/grantBrokerProjectionService.ts b/backend/services/grantBrokerProjectionService.ts index c01c3cc7ca..77113a9c16 100644 --- a/backend/services/grantBrokerProjectionService.ts +++ b/backend/services/grantBrokerProjectionService.ts @@ -29,11 +29,15 @@ import { GRANT_BROKER_AUTHORIZATION } from './seatEnvironmentProjection'; import { allToolDefinitions, executionErrorOutcome } from './toolBrokerService'; import Pod from '../models/Pod'; import RoomGrant from '../models/RoomGrant'; +import { assertGrantUsable, RoomGrantError } from './roomGrantService'; +import type { RoomGrantTaskBinding } from '../models/RoomGrant'; export type GrantTarget = { kind?: unknown; id?: unknown }; export interface LiveGrant { grantId: string; + binding?: RoomGrantTaskBinding; + taskEpoch?: number; target: GrantTarget; audience?: unknown[]; tools?: unknown[]; @@ -100,7 +104,7 @@ export const selectLiveGrantsForIdentities = async ( revokedAt: null, expiresAt: { $gt: new Date() }, audience: { $in: ids }, - }).select('grantId target audience tools writeMode').lean() as LiveGrant[]; + }).select('grantId target audience tools writeMode binding taskEpoch expiresAt revokedAt parentGrantId').lean() as LiveGrant[]; if (!grants.length) return output; const pods = scopedPodIds.length @@ -114,7 +118,7 @@ export const selectLiveGrantsForIdentities = async ( ); for (const id of ids) { - const selected = grants.filter((grant) => { + const candidates = grants.filter((grant) => { const grantId = typeof grant.grantId === 'string' ? grant.grantId : ''; if (!grantId) return false; const inAudience = (grant.audience || []).map(String).includes(id); @@ -127,6 +131,20 @@ export const selectLiveGrantsForIdentities = async ( } return false; }); + const selected: LiveGrant[] = []; + for (const grant of candidates) { + if (grant.binding) { + try { + // Same authority as list/call; no task context supplied by the model. + await assertGrantUsable({ grant: grant as unknown as Record, agentUserId: id, + currentMemberIds: grant.target.kind === 'pod' ? [...(podMembers.get(String(grant.target.id)) || [])] : [id] }); + } catch (error) { + if (error instanceof RoomGrantError) continue; + throw error; + } + } + selected.push(grant); + } if (selected.length) output.set(id, selected); } return output; diff --git a/backend/services/kernelWorkSweepService.ts b/backend/services/kernelWorkSweepService.ts index 23323124a3..296987551a 100644 --- a/backend/services/kernelWorkSweepService.ts +++ b/backend/services/kernelWorkSweepService.ts @@ -263,6 +263,7 @@ class KernelWorkSweepService { status: 'pending', assignee: null, claimedBy: null, + claimedByUserId: null, claimedAt: null, claimExpiresAt: null, rescueDeferrals: 0, diff --git a/backend/services/roomGrantService.ts b/backend/services/roomGrantService.ts index 8604ba425c..72a8566d5f 100644 --- a/backend/services/roomGrantService.ts +++ b/backend/services/roomGrantService.ts @@ -1,13 +1,18 @@ import { randomUUID } from 'crypto'; +import { Types } from 'mongoose'; +import Task from '../models/Task'; +import Pod from '../models/Pod'; import RoomGrant, { IRoomGrant, IRoomGrantBudget, RoomGrantTargetKind, + RoomGrantTaskBinding, RoomGrantWriteMode, } from '../models/RoomGrant'; export interface RoomGrantCreateInput { grantId?: string; + binding?: RoomGrantTaskBinding; connectionId: string; installationId: string; target: { kind: RoomGrantTargetKind; id: string }; @@ -25,6 +30,7 @@ export interface RoomGrantCreateInput { } export interface RoomGrantAttenuationInput { + binding?: RoomGrantTaskBinding | null; parentGrantId: string; tools?: string[]; writeMode?: RoomGrantWriteMode; @@ -230,10 +236,19 @@ const assertGrantLineageActive = async ( }; const markMintedGrantRejected = async (grantId: string, error: RoomGrantError): Promise => { - await RoomGrant.updateOne({ grantId }, { $set: { revokedAt: new Date() } }); + await RoomGrant.updateOne({ grantId }, { $set: { revokedAt: new Date(), + ...(error.code === 'task_closed' ? { revocationReason: 'task.closed' } : {}), + } }); throw error; }; +export const onTaskClosed = async (task: { + podId?: unknown; taskId?: string; status?: string; grantEpoch?: number; +}): Promise => { + if (task.status !== 'done' || !task.podId || !task.taskId) return; + await revokeTaskGrants({ podId: String(task.podId), taskId: task.taskId, closedEpoch: task.grantEpoch || 1 }); +}; + export const effectiveAudience = ( grant: Pick | { audience?: string[] }, currentMemberIds: string[], @@ -242,6 +257,81 @@ export const effectiveAudience = ( return Array.from(new Set((grant.audience || []).map(String))).filter((id) => current.has(id)); }; +/** Task IDs are pod-local: always resolve the complete board tuple. */ +export const normalizeTaskBinding = (value: unknown): RoomGrantTaskBinding | undefined => { + if (value === undefined) return undefined; + const binding = value as Partial | null; + if (!binding || typeof binding.taskId !== 'string' || !binding.taskId.trim() + || typeof binding.podId !== 'string' || !Types.ObjectId.isValid(binding.podId)) { + throw new RoomGrantError('invalid_task_binding', 'binding requires taskId and podId'); + } + return { taskId: binding.taskId.trim(), podId: new Types.ObjectId(binding.podId).toHexString() }; +}; + +const loadOpenBoundTask = async (binding: RoomGrantTaskBinding) => { + const task = await Task.findOne({ taskId: binding.taskId, podId: binding.podId }) + .select('status grantEpoch claimedBy claimedByUserId claimedAt claimExpiresAt').lean(); + if (!task) throw new RoomGrantError('task_not_found', 'bound task not found', 404); + if (task.status === 'done') throw new RoomGrantError('task_closed', 'bound task is closed', 403); + return task; +}; + +const assertTaskBindingTarget = async (binding: RoomGrantTaskBinding, target: RoomGrantCreateInput['target']) => { + if (target.kind === 'pod') { + if (String(target.id) !== binding.podId) { + throw new RoomGrantError('invalid_task_binding', 'bound task must be in the target pod'); + } + } else { + const pod = await Pod.findById(binding.podId).select('members').lean(); + if (!pod || !(pod.members || []).some((id) => String(id) === String(target.id))) { + throw new RoomGrantError('invalid_task_binding', 'target seat must belong to the bound task pod'); + } + } +}; + +const assertBoundTaskOpen = async (grant: IRoomGrant | Record) => { + const binding = normalizeTaskBinding(readGrantValue(grant, 'binding')); + if (!binding) return; + const task = await loadOpenBoundTask(binding); + if ((task.grantEpoch || 0) !== readGrantValue(grant, 'taskEpoch')) { + throw new RoomGrantError('task_closed', 'bound task has closed since this grant was issued', 403); + } +}; + +/** A selector is not authority: the authenticated caller must hold the board's live lease. */ +const assertTaskContext = async (grant: IRoomGrant | Record, options: GrantUsabilityOptions) => { + const binding = normalizeTaskBinding(readGrantValue(grant, 'binding')); + if (!binding) return; + const task = await loadOpenBoundTask(binding); + const expiresAt = task.claimExpiresAt; + if (task.status !== 'claimed' || !expiresAt || new Date(expiresAt).getTime() <= Date.now()) { + throw new RoomGrantError('task_claim_required', 'bound task requires a live claim', 403); + } + // Older runtime-token claims already stored the User ID. Name-only legacy + // claims need a fresh claim: instance names are not globally unique identities. + const claimant = task.claimedByUserId === undefined + ? (Types.ObjectId.isValid(String(task.claimedBy || '')) ? task.claimedBy : null) + : task.claimedByUserId; + if (!claimant || claimant !== options.agentUserId) { + throw new RoomGrantError('task_claim_required', 'claim the bound task as this agent before using its grant', 403); + } + const pod = await Pod.findById(binding.podId).select('members').lean(); + if (!pod || !(pod.members || []).some((id) => String(id) === options.agentUserId)) { + throw new RoomGrantError('not_in_audience', 'agent is no longer in the bound task pod', 403); + } +}; + +/** Hook successful close transitions. An old close never revokes new grants issued after reopen. */ +export const revokeTaskGrants = async ({ taskId, podId, closedEpoch }: RoomGrantTaskBinding & { + closedEpoch: number; +}): Promise => { + const result = await RoomGrant.updateMany({ + 'binding.taskId': taskId, 'binding.podId': podId, + taskEpoch: { $lt: closedEpoch }, revokedAt: null, + }, { $set: { revokedAt: new Date(), revocationReason: 'task.closed' } }); + return result.modifiedCount; +}; + export const mintGrant = async (input: RoomGrantCreateInput): Promise => { if (input.expiresAt === undefined || input.expiresAt === null || input.expiresAt === '') { throw new RoomGrantError('missing_expiry', 'expiresAt is required'); @@ -249,10 +339,13 @@ export const mintGrant = async (input: RoomGrantCreateInput): Promise(root, 'rootGrantId') || readGrantValue(root, 'grantId')); } @@ -261,8 +354,17 @@ export const mintGrant = async (input: RoomGrantCreateInput): Promise Date: Sun, 25 Oct 2026 21:15:43 -0700 Subject: [PATCH 2/5] =?UTF-8?q?fix(grants):=20task=20binding=20carries=20e?= =?UTF-8?q?ndsOn=20task.done=20and=20close=20revokes=20with=20reason=20tas?= =?UTF-8?q?k.done=20(record-v1,=20ADR-033=20=C2=A75.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto main after #2739 put record-v1 on main. The spec's revoke reasons are task.done|expired|cascade|manual, so the draft's task.closed would be rejected by the verifier. The binding now stores endsOn; v1 accepts only task.done and refuses expiresAt, because this task requires a bound grant to end when its task closes. Co-Authored-By: Claude Opus 5.5 Co-authored-by: vera Agent-Model: claude/claude-opus-5-5/default --- .../__tests__/unit/routes/grants.mint.test.js | 8 ++++- .../routes/tasksApi.taskGrantClose.test.js | 2 +- .../services/roomGrantTaskBinding.test.js | 16 ++++++++-- backend/models/RoomGrant.ts | 15 ++++++--- backend/services/roomGrantService.ts | 19 +++++++----- docs/plans/task-bound-grants.md | 31 +++++++++++-------- 6 files changed, 63 insertions(+), 28 deletions(-) diff --git a/backend/__tests__/unit/routes/grants.mint.test.js b/backend/__tests__/unit/routes/grants.mint.test.js index cc6d8cf7ce..34c891be07 100644 --- a/backend/__tests__/unit/routes/grants.mint.test.js +++ b/backend/__tests__/unit/routes/grants.mint.test.js @@ -419,7 +419,7 @@ describe('task-bound mint', () => { test('stores the authorized task tuple and ignores caller epoch values', async () => { const res = await mint({ binding: { taskId: 'TASK-1', podId: POD }, taskEpoch: 999 }); expect(res.status).toBe(201); - expect(res.body.binding).toEqual({ taskId: 'TASK-1', podId: POD }); + expect(res.body.binding).toEqual({ taskId: 'TASK-1', podId: POD, endsOn: 'task.done' }); expect(res.body.taskEpoch).toBe(0); }); test.each([ @@ -431,6 +431,12 @@ describe('task-bound mint', () => { expect([400, 404]).toContain(res.status); expect(await RoomGrant.countDocuments({})).toBe(0); }); + test('refuses a binding that ends on anything but task.done, without saving', async () => { + const res = await mint({ binding: { taskId: 'TASK-1', podId: POD, endsOn: 'expiresAt' } }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('invalid_task_binding'); + expect(await RoomGrant.countDocuments({})).toBe(0); + }); test('rejects a closed task', async () => { await Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $set: { status: 'done' } }); const res = await mint({ binding: { taskId: 'TASK-1', podId: POD } }); diff --git a/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js b/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js index 05119adaa4..fff24472ec 100644 --- a/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js +++ b/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js @@ -46,7 +46,7 @@ test.each(['complete', 'patch', 'github', 'onboarding'])('%s close revokes the b expect(task.status).toBe('done'); expect(task.grantEpoch).toBe(1); const revoked = await RoomGrant.findById(grant._id); - expect(revoked.revocationReason).toBe('task.closed'); + expect(revoked.revocationReason).toBe('task.done'); expect(revoked.revokedAt).toBeInstanceOf(Date); await expect(assertGrantUsable({ grantId: grant.grantId, agentUserId: mockActor.id, currentMemberIds: [mockActor.id] })) .rejects.toMatchObject({ code: 'grant_revoked' }); diff --git a/backend/__tests__/unit/services/roomGrantTaskBinding.test.js b/backend/__tests__/unit/services/roomGrantTaskBinding.test.js index 85b3d316a1..9011936669 100644 --- a/backend/__tests__/unit/services/roomGrantTaskBinding.test.js +++ b/backend/__tests__/unit/services/roomGrantTaskBinding.test.js @@ -44,6 +44,18 @@ test.each([ expect(await RoomGrant.countDocuments()).toBe(0); }); +test('a binding records endsOn task.done, the only v1 value, and refuses any other', async () => { + const grant = await service.mintGrant(input()); + expect(grant.binding.toObject()).toEqual({ ...binding, endsOn: 'task.done' }); + await expect(service.mintGrant(input({ binding: { ...binding, endsOn: 'task.done' } }))).resolves.toBeTruthy(); + for (const endsOn of ['expiresAt', 'task.closed', null]) { + // eslint-disable-next-line no-await-in-loop + await expect(service.mintGrant(input({ binding: { ...binding, endsOn } }))) + .rejects.toMatchObject({ code: 'invalid_task_binding' }); + } + expect(await RoomGrant.countDocuments()).toBe(2); +}); + test('closed tasks cannot mint and a stale read cannot make a grant usable after close/reopen', async () => { const grant = await service.mintGrant(input()); await Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $set: { status: 'done' }, $inc: { grantEpoch: 1 } }); @@ -56,7 +68,7 @@ test('closed tasks cannot mint and a stale read cannot make a grant usable after test('a task binding is inherited by attenuation and expiry remains a ceiling', async () => { const parent = await service.mintGrant(input()); const child = await service.attenuateGrant({ parentGrantId: parent.grantId, expiresAt: new Date(Date.now() + 7200000) }); - expect(child.binding.toObject()).toEqual(binding); + expect(child.binding.toObject()).toEqual({ ...binding, endsOn: 'task.done' }); expect(child.taskEpoch).toBe(parent.taskEpoch); expect(child.expiresAt.getTime()).toBe(parent.expiresAt.getTime()); await expect(service.attenuateGrant({ parentGrantId: parent.grantId, binding: null })) @@ -71,7 +83,7 @@ test('the close hook revokes bound descendants, once, without touching another p const unbound = await service.mintGrant(input({ binding: undefined })); await service.revokeTaskGrants({ ...binding, closedEpoch: 1 }); expect((await RoomGrant.findById(parent._id)).revokedAt).toBeInstanceOf(Date); - expect((await RoomGrant.findById(child._id)).revocationReason).toBe('task.closed'); + expect((await RoomGrant.findById(child._id)).revocationReason).toBe('task.done'); expect((await RoomGrant.findById(unbound._id)).revokedAt).toBeNull(); await expect(service.revokeTaskGrants({ ...binding, closedEpoch: 1 })).resolves.toBe(0); }); diff --git a/backend/models/RoomGrant.ts b/backend/models/RoomGrant.ts index d08527b687..5a2a0b70cd 100644 --- a/backend/models/RoomGrant.ts +++ b/backend/models/RoomGrant.ts @@ -21,12 +21,15 @@ export interface IRoomGrantTarget { id: string; } -export interface RoomGrantTaskBinding { taskId: string; podId: string } +// record-v1 §2 / ADR-033 §5.1. Only `task.done` is accepted in v1: a bound +// grant always ends when its task closes, with `expiresAt` as a ceiling. +export type RoomGrantBindingEndsOn = 'task.done'; +export interface RoomGrantTaskBinding { taskId: string; podId: string; endsOn: RoomGrantBindingEndsOn } export interface IRoomGrant extends Document { binding?: RoomGrantTaskBinding; taskEpoch?: number; - revocationReason?: string; + revocationReason?: 'task.done'; grantId: string; connectionId: string; installationId: string; @@ -88,11 +91,15 @@ const RoomGrantSchema = new Schema( // Mongo's _id remains an implementation detail. grantId: { type: String, required: true, unique: true, trim: true }, binding: { - type: new Schema({ taskId: { type: String, required: true }, podId: { type: String, required: true } }, { _id: false }), + type: new Schema({ + taskId: { type: String, required: true }, + podId: { type: String, required: true }, + endsOn: { type: String, enum: ['task.done'], required: true, default: 'task.done' }, + }, { _id: false }), default: undefined, }, taskEpoch: { type: Number, min: 0 }, - revocationReason: { type: String }, + revocationReason: { type: String, enum: ['task.done'] }, connectionId: { type: String, required: true, trim: true }, installationId: { type: String, required: true, trim: true }, target: { type: GrantTargetSchema, required: true }, diff --git a/backend/services/roomGrantService.ts b/backend/services/roomGrantService.ts index 72a8566d5f..2a4c4f4167 100644 --- a/backend/services/roomGrantService.ts +++ b/backend/services/roomGrantService.ts @@ -237,7 +237,7 @@ const assertGrantLineageActive = async ( const markMintedGrantRejected = async (grantId: string, error: RoomGrantError): Promise => { await RoomGrant.updateOne({ grantId }, { $set: { revokedAt: new Date(), - ...(error.code === 'task_closed' ? { revocationReason: 'task.closed' } : {}), + ...(error.code === 'task_closed' ? { revocationReason: 'task.done' } : {}), } }); throw error; }; @@ -260,12 +260,18 @@ export const effectiveAudience = ( /** Task IDs are pod-local: always resolve the complete board tuple. */ export const normalizeTaskBinding = (value: unknown): RoomGrantTaskBinding | undefined => { if (value === undefined) return undefined; - const binding = value as Partial | null; + const binding = value as Partial> | null; if (!binding || typeof binding.taskId !== 'string' || !binding.taskId.trim() || typeof binding.podId !== 'string' || !Types.ObjectId.isValid(binding.podId)) { throw new RoomGrantError('invalid_task_binding', 'binding requires taskId and podId'); } - return { taskId: binding.taskId.trim(), podId: new Types.ObjectId(binding.podId).toHexString() }; + if (binding.endsOn !== undefined && binding.endsOn !== 'task.done') { + throw new RoomGrantError( + 'invalid_task_binding', + "binding.endsOn must be 'task.done': a task-bound grant ends when its task closes", + ); + } + return { taskId: binding.taskId.trim(), podId: new Types.ObjectId(binding.podId).toHexString(), endsOn: 'task.done' }; }; const loadOpenBoundTask = async (binding: RoomGrantTaskBinding) => { @@ -322,13 +328,12 @@ const assertTaskContext = async (grant: IRoomGrant | Record, op }; /** Hook successful close transitions. An old close never revokes new grants issued after reopen. */ -export const revokeTaskGrants = async ({ taskId, podId, closedEpoch }: RoomGrantTaskBinding & { - closedEpoch: number; -}): Promise => { +export const revokeTaskGrants = async ({ taskId, podId, closedEpoch }: Pick + & { closedEpoch: number }): Promise => { const result = await RoomGrant.updateMany({ 'binding.taskId': taskId, 'binding.podId': podId, taskEpoch: { $lt: closedEpoch }, revokedAt: null, - }, { $set: { revokedAt: new Date(), revocationReason: 'task.closed' } }); + }, { $set: { revokedAt: new Date(), revocationReason: 'task.done' } }); return result.modifiedCount; }; diff --git a/docs/plans/task-bound-grants.md b/docs/plans/task-bound-grants.md index af742db83e..1035182978 100644 --- a/docs/plans/task-bound-grants.md +++ b/docs/plans/task-bound-grants.md @@ -2,17 +2,21 @@ Status: implementation draft. Not cleared for merge or release. Lifecycle records still depend on the shared `seats.record` writer owned by TASK-513. This document -is not a second record specification. +is not a second record specification: names follow +`packages/seats/docs/record-v1.md` (on main via #2739) and ADR-033 §5.1. ## Authority -A grant may carry `binding: { taskId, podId }`. Task numbers are local to a pod; -resolve both fields. Mint requires an existing, unfinished task in the target +A grant may carry `binding: { taskId, podId, endsOn }`. Task numbers are local to +a pod; resolve both fields. `endsOn` is stored as `task.done`, the only value v1 +accepts: record-v1 also defines `expiresAt`, but a grant that outlives its task's +close contradicts this task's requirement, so mint refuses it with +`invalid_task_binding` until someone needs that mode. Mint requires an existing, unfinished task in the target pod. For seat targets, both the issuer and seat must have access to the task pod. An unbound parent can be narrowed to a task. A bound child cannot remove or change the binding. The parent's expiry remains a ceiling. -Use requires the authenticated caller to be in the grant audience and the task +Use requires (ruled by the CTO for v1, 2026-10-25) the authenticated caller to be in the grant audience and the task pod, and to hold the bound task's unexpired claim. A task selector supplied by the model conveys no authority. The board claim route stores `claimedByUserId` from authentication separately from its historical display key, `claimedBy`. @@ -36,7 +40,8 @@ revive an old grant. Mint checks the epoch again after inserting the grant to cover an insertion racing close. The close hook marks bound grants and their bound descendants revoked with -`revocationReason: task.closed`. It is awaited by the complete route, status +`revocationReason: task.done`, the record-v1 reason (the verifier rejects +`task.closed`). It is awaited by the complete route, status PATCH, GitHub merge completion and onboarding completion. There is no poller. The hook only revokes grants from epochs before that close, preserving a new, explicitly issued grant after a subsequent reopen. Unbound grants are unchanged. @@ -48,15 +53,15 @@ provider dispatch. ## Record dependency and acceptance -The single record contract is `packages/seats/docs/record-v1.md` and the -corresponding grant-record ADR. At the implementation base `136709aa0` that -specification and its backend writer are absent. The private ADR draft leaves -server run IDs, sequencing, storage and actor normalization open. This draft -stores the revocation reason but does not yet emit `grant.issued` or -`grant.revoked`; that is an incomplete requirement and blocks clearance. +The single record contract is `packages/seats/docs/record-v1.md` and ADR-033 +(#2691). The spec is on main; the server writer (TASK-513) is not. This change +stores what that writer needs (the binding with `endsOn`, `taskEpoch`, +`revokedAt` and `revocationReason`) but does not emit `grant.issued` or +`grant.revoked` itself; that is an incomplete requirement and blocks clearance. -Once TASK-513 supplies the writer, issuance must include the binding and close -must emit `grant.revoked` with reason `task.closed`, using the canonical envelope. +Once TASK-513 supplies the writer, issuance must carry the binding and close +must emit `task.done` then `grant.revoked {grantIds, reason: task.done}` (ADR-033 +§4), using the canonical envelope. No credential values or raw tool arguments enter those records. Failed record emission must not undo enforcement. A non-author must walk issuance, allowed use, closure, refused replay and the exported records before public activation. From 7e7ea27a0bdf28aa446b0e45c9a1c55519355d89 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:23:12 -0700 Subject: [PATCH 3/5] fix(grants): a failed close-revocation write no longer turns a committed close into a 500 The close write moves grantEpoch, so the epoch fence refuses every bound grant from that instant; the revocation write only records it. Throwing after the commit skipped attention/notify and could not be retried, because the retry finds the task done. Log instead. The status-vocabulary suite mocks Task, so it now mocks the hook and asserts the done transitions call it. Co-Authored-By: Claude Opus 5.5 Co-authored-by: vera Agent-Model: claude/claude-opus-5-5/default --- .../unit/routes/tasksApi.status-vocabulary.test.js | 10 ++++++++++ .../unit/routes/tasksApi.taskGrantClose.test.js | 13 +++++++++++++ backend/services/roomGrantService.ts | 14 +++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/backend/__tests__/unit/routes/tasksApi.status-vocabulary.test.js b/backend/__tests__/unit/routes/tasksApi.status-vocabulary.test.js index 0019c404d9..e1a171b3ee 100644 --- a/backend/__tests__/unit/routes/tasksApi.status-vocabulary.test.js +++ b/backend/__tests__/unit/routes/tasksApi.status-vocabulary.test.js @@ -58,6 +58,13 @@ jest.mock('../../../services/attentionItemService', () => ({ resolveTaskAttention: (...args) => mockResolveTaskAttention(...args), })); +// The close hook writes RoomGrant, which this file's mocked Task cannot back. +// Its real-Mongo behaviour is covered in tasksApi.taskGrantClose.test.js. +const mockOnTaskClosed = jest.fn(async () => {}); +jest.mock('../../../services/roomGrantService', () => ({ + onTaskClosed: (...args) => mockOnTaskClosed(...args), +})); + const tasksApi = require('../../../routes/tasksApi'); const Task = require('../../../models/Task'); @@ -77,6 +84,7 @@ beforeEach(() => { mockRecordTaskAttention.mockResolvedValue(undefined); mockResolveTaskAttention.mockReset(); mockResolveTaskAttention.mockResolvedValue(undefined); + mockOnTaskClosed.mockClear(); }); describe('PATCH /:podId/:taskId status vocabulary', () => { @@ -129,6 +137,7 @@ describe('PATCH /:podId/:taskId status vocabulary', () => { expect(res.status).toBe(200); expect(mockResolveTaskAttention).toHaveBeenCalledWith(task); + expect(mockOnTaskClosed).toHaveBeenCalledWith(expect.objectContaining({ taskId: 'TASK-001', status: 'done' })); }); test('POST /complete resolves the task\'s outstanding handoffs, like a PATCH out of blocked does', async () => { @@ -143,6 +152,7 @@ describe('PATCH /:podId/:taskId status vocabulary', () => { .send({ prUrl: 'https://github.com/Team-Commonly/commonly/pull/1' }); expect(res.status).toBe(200); expect(mockResolveTaskAttention).toHaveBeenCalledWith(expect.objectContaining({ _id: 'task-1' })); + expect(mockOnTaskClosed).toHaveBeenCalledWith(expect.objectContaining({ _id: 'task-1', status: 'done' })); expect(mockNotifyPodAgents).toHaveBeenCalledWith( POD_ID, task, 'updated', expect.any(Object), { status: 'claimed' }, ); diff --git a/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js b/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js index fff24472ec..48e647559b 100644 --- a/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js +++ b/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js @@ -52,6 +52,19 @@ test.each(['complete', 'patch', 'github', 'onboarding'])('%s close revokes the b .rejects.toMatchObject({ code: 'grant_revoked' }); }); +test('a failed revocation write leaves the close committed and the grant refused by its epoch', async () => { + const spy = jest.spyOn(RoomGrant, 'updateMany').mockRejectedValueOnce(new Error('revocation write failed')); + expect((await request(app).post(`/tasks/${POD}/TASK-1/complete`).send({})).status).toBe(200); + expect(spy).toHaveBeenCalledTimes(1); + spy.mockRestore(); + expect((await Task.findOne({ podId: POD, taskId: 'TASK-1' })).status).toBe('done'); + expect((await RoomGrant.findById(grant._id)).revokedAt).toBeNull(); + await Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $set: { status: 'claimed', claimedByUserId: mockActor.id, + claimExpiresAt: new Date(Date.now() + 60000) } }); + await expect(assertGrantUsable({ grantId: grant.grantId, agentUserId: mockActor.id, currentMemberIds: [mockActor.id] })) + .rejects.toMatchObject({ code: 'task_closed' }); +}); + test('claim stamps authenticated identity and refuses renewal by a different User sharing its display key', async () => { await Task.updateOne({ taskId: 'TASK-1' }, { $set: { status: 'pending' } }); mockActor.bot = true; diff --git a/backend/services/roomGrantService.ts b/backend/services/roomGrantService.ts index 2a4c4f4167..78a6057914 100644 --- a/backend/services/roomGrantService.ts +++ b/backend/services/roomGrantService.ts @@ -242,11 +242,23 @@ const markMintedGrantRejected = async (grantId: string, error: RoomGrantError): throw error; }; +/** + * Runs after the close write commits. That write already moved the task's + * grantEpoch, so every bound grant is refused at use from that instant; this + * write records the revocation. A failure is logged, not thrown: a 500 after a + * committed close would skip the caller's remaining close work, and a retry + * finds the task done and never reaches this hook again. + */ export const onTaskClosed = async (task: { podId?: unknown; taskId?: string; status?: string; grantEpoch?: number; }): Promise => { if (task.status !== 'done' || !task.podId || !task.taskId) return; - await revokeTaskGrants({ podId: String(task.podId), taskId: task.taskId, closedEpoch: task.grantEpoch || 1 }); + try { + await revokeTaskGrants({ podId: String(task.podId), taskId: task.taskId, closedEpoch: task.grantEpoch || 1 }); + } catch (error) { + console.error(`[grants] task.done revocation write failed for ${String(task.podId)}/${task.taskId};` + + ' the epoch fence still refuses its grants', error); + } }; export const effectiveAudience = ( From 1a91cd08c0a9139c3c5ae06962344b9b62bbfd0b Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:56:04 -0700 Subject: [PATCH 4/5] fix(grants): log the failed revocation write with a constant format string CodeQL js/tainted-format-string: taskId is caller-supplied and sat in console.error's format argument. Co-Authored-By: Claude Opus 5.5 Co-authored-by: vera Agent-Model: claude/claude-opus-5-5/default --- backend/services/roomGrantService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/services/roomGrantService.ts b/backend/services/roomGrantService.ts index 78a6057914..6a1e1cf25b 100644 --- a/backend/services/roomGrantService.ts +++ b/backend/services/roomGrantService.ts @@ -256,8 +256,8 @@ export const onTaskClosed = async (task: { try { await revokeTaskGrants({ podId: String(task.podId), taskId: task.taskId, closedEpoch: task.grantEpoch || 1 }); } catch (error) { - console.error(`[grants] task.done revocation write failed for ${String(task.podId)}/${task.taskId};` - + ' the epoch fence still refuses its grants', error); + console.error('[grants] task.done revocation write failed for %s/%s; the epoch fence still refuses its grants', + String(task.podId), task.taskId, error); } }; From b875857123a39e48fa9f64de8de5f769040efdf0 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 00:10:21 -0700 Subject: [PATCH 5/5] =?UTF-8?q?feat(grants):=20record=20grant.issued=20and?= =?UTF-8?q?=20grant.revoked=20for=20task-bound=20grants=20(ADR-033=20?= =?UTF-8?q?=C2=A74)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TASK-513's writer is on main (#2772), so the two lines the plan deferred are written now. Both land on the bound task's own chain. - mintGrant writes grant.issued once every post-insert check has passed, with scope, target, expiry, binding and issuedBy from authentication: the connection owner on mint, the delegating agent (as an agent actor) on attenuation. - The close hook writes grant.revoked {grantIds, reason: task.done} as system:board, naming only the grants its own write revoked (read back by that write's revokedAt). - All four close writers now queue task.done before the hook runs, so the chain reads task.done, then grant.revoked. Unbound issuance and the cascade/manual/expired revocations stay with the TASK-513 grant writers. Nothing is written outside RECORD_V1_PODS. Co-Authored-By: Claude Opus 5.5 Co-authored-by: vera Agent-Model: claude/claude-opus-5-5/default --- .../routes/tasksApi.taskGrantClose.test.js | 42 ++++- .../unit/services/roomGrantRecord.test.js | 170 ++++++++++++++++++ backend/routes/grants.ts | 3 + backend/routes/tasksApi.ts | 18 +- backend/services/githubGigService.ts | 2 +- backend/services/roomGrantService.ts | 69 ++++++- backend/services/starterTaskService.ts | 2 +- docs/plans/task-bound-grants.md | 38 ++-- 8 files changed, 313 insertions(+), 31 deletions(-) create mode 100644 backend/__tests__/unit/services/roomGrantRecord.test.js diff --git a/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js b/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js index 48e647559b..aeaa5f44ed 100644 --- a/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js +++ b/backend/__tests__/unit/routes/tasksApi.taskGrantClose.test.js @@ -13,6 +13,26 @@ jest.mock('../../../services/taskEventService', () => ({ emitTaskUpdated: jest.f jest.mock('../../../services/attentionItemService', () => ({ resolveTaskAttention: jest.fn(async () => {}), recordTaskAttention: jest.fn(async () => {}) })); jest.mock('../../../services/systemExchangeTriggers', () => ({ recordTaskCompleted: jest.fn(async () => {}) })); jest.mock('../../../services/agentSendFreshness', () => ({ unseenMessages: jest.fn(async () => null) })); +// An in-memory record chain store with the repository's contract. +const mockChains = new Map(); +jest.mock('../../../repositories/recordChainRepository', () => { + const actual = jest.requireActual('../../../repositories/recordChainRepository'); + return { + ...actual, + appendToChain: jest.fn(async (ref, runId, eventKey, build) => { + const key = `${ref.podId}|${ref.taskId || ''}`; + if (!mockChains.has(key)) mockChains.set(key, { runId, lines: [], keys: new Set() }); + const chain = mockChains.get(key); + if (chain.keys.has(eventKey)) return { appended: 0, duplicate: true }; + const { lines } = build(chain.lines[chain.lines.length - 1] || null, chain.runId); + chain.lines.push(...lines); + chain.keys.add(eventKey); + return { appended: lines.length, duplicate: false }; + }), + }; +}); +const { flushRecordWrites } = require('../../../services/recordService'); +const { verifyRecord } = require('../../../vendor/seats/src/record/format'); const Task = require('../../../models/Task'); const Pod = require('../../../models/Pod'); const RoomGrant = require('../../../models/RoomGrant'); @@ -23,10 +43,19 @@ const router = require('../../../routes/tasksApi'); const POD = new mongoose.Types.ObjectId().toString(); let app; let grant; -beforeAll(async () => { await setupMongoDb(); app = express(); app.use(express.json()); app.use('/tasks', router); }); -afterEach(clearMongoDb); -afterAll(closeMongoDb); +const recordPods = process.env.RECORD_V1_PODS; +beforeAll(async () => { + process.env.RECORD_V1_PODS = POD; + await setupMongoDb(); app = express(); app.use(express.json()); app.use('/tasks', router); +}); +afterEach(async () => { await flushRecordWrites(); await clearMongoDb(); }); +afterAll(async () => { + if (recordPods === undefined) delete process.env.RECORD_V1_PODS; + else process.env.RECORD_V1_PODS = recordPods; + await closeMongoDb(); +}); beforeEach(async () => { + mockChains.clear(); mockActor.bot = false; await Pod.create({ _id: POD, name: 'Bound task', createdBy: mockActor.id, members: [mockActor.id] }); await Task.create({ podId: POD, taskId: 'TASK-1', taskNum: 1, title: 'Bound task', status: 'claimed', @@ -50,6 +79,13 @@ test.each(['complete', 'patch', 'github', 'onboarding'])('%s close revokes the b expect(revoked.revokedAt).toBeInstanceOf(Date); await expect(assertGrantUsable({ grantId: grant.grantId, agentUserId: mockActor.id, currentMemberIds: [mockActor.id] })) .rejects.toMatchObject({ code: 'grant_revoked' }); + // ADR-033 §4: the close's task.done, then grant.revoked for its bound grants. + await flushRecordWrites(); + const { lines } = mockChains.get(`${POD}|TASK-1`); + expect(verifyRecord(lines.map((line) => `${line}\n`).join(''))).toMatchObject({ ok: true }); + expect(lines.map((line) => JSON.parse(line)).map((l) => [l.kind, l.data.grantIds || null])).toEqual([ + ['grant.issued', null], ['task.done', null], ['grant.revoked', [grant.grantId]], + ]); }); test('a failed revocation write leaves the close committed and the grant refused by its epoch', async () => { diff --git a/backend/__tests__/unit/services/roomGrantRecord.test.js b/backend/__tests__/unit/services/roomGrantRecord.test.js new file mode 100644 index 0000000000..12e134b174 --- /dev/null +++ b/backend/__tests__/unit/services/roomGrantRecord.test.js @@ -0,0 +1,170 @@ +// Task-bound grants write record-v1 lines (ADR-033 §4): grant.issued on mint +// and attenuation, and grant.revoked {reason: task.done} when the task closes. +// The chain store is in memory with the repository's contract, as in +// recordService.test.js; every line must pass the shared reference verifier. +const mockChains = new Map(); +jest.mock('../../../repositories/recordChainRepository', () => { + const actual = jest.requireActual('../../../repositories/recordChainRepository'); + const key = (ref) => `${ref.podId}|${ref.taskId || ''}`; + return { + ...actual, + appendToChain: jest.fn(async (ref, runId, eventKey, build) => { + if (!mockChains.has(key(ref))) mockChains.set(key(ref), { runId, lines: [], keys: new Set() }); + const chain = mockChains.get(key(ref)); + if (chain.keys.has(eventKey)) return { appended: 0, duplicate: true }; + const { lines } = build(chain.lines[chain.lines.length - 1] || null, chain.runId); + chain.lines.push(...lines); + chain.keys.add(eventKey); + return { appended: lines.length, duplicate: false }; + }), + }; +}); + +const mongoose = require('mongoose'); +const { setupMongoDb, clearMongoDb, closeMongoDb } = require('../../utils/testUtils'); +const Task = require('../../../models/Task'); +const Pod = require('../../../models/Pod'); +const service = require('../../../services/roomGrantService'); +const { flushRecordWrites } = require('../../../services/recordService'); +const { verifyRecord } = require('../../../vendor/seats/src/record/format'); + +const POD = new mongoose.Types.ObjectId().toString(); +const OWNER = new mongoose.Types.ObjectId().toString(); +const AGENT = new mongoose.Types.ObjectId().toString(); +const binding = { taskId: 'TASK-1', podId: POD }; +const input = (overrides = {}) => ({ + connectionId: 'connection', + installationId: 'installation', + target: { kind: 'pod', id: POD }, + tools: ['issues.read'], + writeMode: 'read', + audience: [AGENT], + expiresAt: new Date(Date.now() + 3600000), + brokerId: 'broker', + binding, + issuer: { type: 'human', id: OWNER }, + ...overrides, +}); +const chainLines = async (podId = POD, taskId = 'TASK-1') => { + await flushRecordWrites(); + const chain = mockChains.get(`${podId}|${taskId}`); + if (!chain) return []; + expect(verifyRecord(chain.lines.map((line) => `${line}\n`).join(''))).toMatchObject({ ok: true }); + return chain.lines.map((line) => JSON.parse(line)); +}; +const close = async () => { + const task = await Task.findOneAndUpdate( + { podId: POD, taskId: 'TASK-1' }, + { $inc: { grantEpoch: 1 }, $set: { status: 'done' } }, + { new: true }, + ); + await service.onTaskClosed(task); +}; + +const env = process.env.RECORD_V1_PODS; +beforeAll(setupMongoDb); +afterEach(async () => { await flushRecordWrites(); await clearMongoDb(); }); +afterAll(async () => { + if (env === undefined) delete process.env.RECORD_V1_PODS; + else process.env.RECORD_V1_PODS = env; + await closeMongoDb(); +}); +beforeEach(async () => { + mockChains.clear(); + process.env.RECORD_V1_PODS = POD; + await Pod.create({ + _id: POD, name: 'Task pod', createdBy: OWNER, members: [OWNER, AGENT], + }); + await Task.create({ + podId: POD, + taskNum: 1, + taskId: 'TASK-1', + title: 'Investigate issue', + status: 'claimed', + claimedBy: AGENT, + claimExpiresAt: new Date(Date.now() + 1800000), + }); +}); + +test('a bound mint writes grant.issued on its task chain, with the binding and the issuer', async () => { + const grant = await service.mintGrant(input({ budget: { calls: 5 } })); + const lines = await chainLines(); + expect(lines.map((l) => l.kind)).toEqual(['grant.issued']); + expect(lines[0].actor).toEqual({ type: 'human', id: OWNER }); + expect(lines[0].data).toEqual({ + grantId: grant.grantId, + scope: { + connectionId: 'connection', tools: ['issues.read'], writeMode: 'read', budget: { calls: 5 }, audience: [AGENT], + }, + target: { kind: 'pod', id: POD }, + issuedBy: OWNER, + expiresAt: grant.expiresAt.toISOString(), + binding: { ...binding, endsOn: 'task.done' }, + }); +}); + +test('an agent attenuating a bound grant is recorded as the agent, with the parent', async () => { + const parent = await service.mintGrant(input()); + const child = await service.attenuateGrant({ parentGrantId: parent.grantId, issuer: { type: 'agent', id: AGENT } }); + const lines = await chainLines(); + expect(lines.map((l) => [l.kind, l.data.grantId])).toEqual([ + ['grant.issued', parent.grantId], ['grant.issued', child.grantId], + ]); + expect(lines[1].actor).toEqual({ type: 'agent', id: AGENT }); + expect(lines[1].data).toMatchObject({ parentGrantId: parent.grantId, issuedBy: AGENT }); +}); + +test('an unbound mint has no task chain and writes nothing', async () => { + await service.mintGrant(input({ binding: undefined })); + await flushRecordWrites(); + expect(mockChains.size).toBe(0); +}); + +test('a mint refused after insertion writes no grant.issued', async () => { + const realCreate = mongoose.model('RoomGrant').create.bind(mongoose.model('RoomGrant')); + const spy = jest.spyOn(mongoose.model('RoomGrant'), 'create').mockImplementationOnce(async (doc) => { + const created = await realCreate(doc); + await Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $inc: { grantEpoch: 1 } }); + return created; + }); + await expect(service.mintGrant(input())).rejects.toMatchObject({ code: 'task_closed' }); + spy.mockRestore(); + expect(await chainLines()).toEqual([]); +}); + +test('close writes one grant.revoked naming the bound grant and its descendant, as the board', async () => { + const parent = await service.mintGrant(input()); + const child = await service.attenuateGrant({ parentGrantId: parent.grantId, issuer: { type: 'agent', id: AGENT } }); + await close(); + await service.onTaskClosed(await Task.findOne({ podId: POD, taskId: 'TASK-1' })); + const revoked = (await chainLines()).filter((l) => l.kind === 'grant.revoked'); + expect(revoked).toHaveLength(1); + expect(revoked[0].actor).toEqual({ type: 'system', id: 'board' }); + expect(revoked[0].data.reason).toBe('task.done'); + expect([...revoked[0].data.grantIds].sort()).toEqual([parent.grantId, child.grantId].sort()); +}); + +test('a second close after reopen names only the grant issued since, not the one already revoked', async () => { + const first = await service.mintGrant(input()); + await close(); + await Task.updateOne({ podId: POD, taskId: 'TASK-1' }, { $set: { status: 'claimed' } }); + const second = await service.mintGrant(input()); + await close(); + const revoked = (await chainLines()).filter((l) => l.kind === 'grant.revoked'); + expect(revoked.map((l) => l.data.grantIds)).toEqual([[first.grantId], [second.grantId]]); +}); + +test('a close with no live bound grant writes no grant.revoked', async () => { + await service.mintGrant(input({ binding: undefined })); + await close(); + expect((await chainLines()).filter((l) => l.kind === 'grant.revoked')).toEqual([]); +}); + +test('a pod outside RECORD_V1_PODS gets no lines, and its grants still revoke', async () => { + process.env.RECORD_V1_PODS = new mongoose.Types.ObjectId().toString(); + const grant = await service.mintGrant(input()); + await close(); + await flushRecordWrites(); + expect(mockChains.size).toBe(0); + expect((await mongoose.model('RoomGrant').findById(grant._id)).revocationReason).toBe('task.done'); +}); diff --git a/backend/routes/grants.ts b/backend/routes/grants.ts index 99b8b3af36..f2df7a3e30 100644 --- a/backend/routes/grants.ts +++ b/backend/routes/grants.ts @@ -26,6 +26,7 @@ import { grantBrokerRefusal } from '../services/grantBrokerConfinement'; import type { GrantBrokerRefusal } from '../services/grantBrokerConfinement'; import { projectSeatEnvironments, seatEnvironmentKey } from '../services/seatEnvironmentProjection'; import { outsiderGrantPodRefusal, outsiderGrantRefusalMessage } from '../services/outsiderGrantPodEligibility'; +import { agentActor, requestActor } from '../services/recordService'; // eslint-disable-next-line @typescript-eslint/no-require-imports const auth = require('../middleware/auth'); @@ -426,6 +427,7 @@ router.post('/', grantRateLimit, auth, async (req: AuthenticatedRequest, res: ex // Bound here, at mint, to the row the caller just proved they own. connectionRowId, ownerId: connectionOwnerId(connection), + issuer: requestActor(req as Parameters[0]), }; const grant = await createGrant(grantInput); return res.status(201).json(grant); @@ -474,6 +476,7 @@ router.post( budget: body.budget as any, audience: body.audience as string[] | undefined, expiresAt: body.expiresAt as string | undefined, + issuer: agentActor(req.agentUser as Parameters[0]), }); return res.status(201).json(child); } catch (error) { diff --git a/backend/routes/tasksApi.ts b/backend/routes/tasksApi.ts index 67c7ffc688..671200aae2 100644 --- a/backend/routes/tasksApi.ts +++ b/backend/routes/tasksApi.ts @@ -787,10 +787,11 @@ router.post('/:podId/:taskId/complete', taskWriteRateLimit(30), auth, async (req // PATCH gate already does this on a status change; this route wrote // `done` without it, and 11 of 12 open handoffs on 2026-09-19 pointed at // tasks completed here. + // task.done is queued before the close hook's grant.revoked (ADR-033 §4). + recordTaskTransition({ podId, task, from: null, to: 'done', at: now, doneBy: userId }); await onTaskClosed(task); await resolveTaskAttention(task); emitTaskUpdated(podId, task, 'updated'); - recordTaskTransition({ podId, task, from: null, to: 'done', at: now, doneBy: userId }); notifyAgents(req, podId, task, 'updated', { status: 'claimed' }); return res.json({ task }); } catch (err) { @@ -1163,13 +1164,7 @@ router.patch('/:podId/:taskId', taskWriteRateLimit(60), auth, async (req: AuthRe } task.updatedAt = writeTime; } - if (fieldUpdates.status === 'blocked') await recordTaskAttention(task, { includeBlocked: true }); - if (fieldUpdates.status === 'done') { - task.grantEpoch = (task.grantEpoch || 0) + 1; - await onTaskClosed(task); - } - if (fieldUpdates.status !== undefined && fieldUpdates.status !== 'blocked') await resolveTaskAttention(task); - emitTaskUpdated(podId, task, 'updated'); + // Before the close hook, so task.done precedes its grant.revoked (ADR-033 §4). if (fieldUpdates.status !== undefined && previousTask && writeTime) { recordTaskTransition({ podId, @@ -1181,6 +1176,13 @@ router.patch('/:podId/:taskId', taskWriteRateLimit(60), auth, async (req: AuthRe doneBy: userId, }); } + if (fieldUpdates.status === 'blocked') await recordTaskAttention(task, { includeBlocked: true }); + if (fieldUpdates.status === 'done') { + task.grantEpoch = (task.grantEpoch || 0) + 1; + await onTaskClosed(task); + } + if (fieldUpdates.status !== undefined && fieldUpdates.status !== 'blocked') await resolveTaskAttention(task); + emitTaskUpdated(podId, task, 'updated'); notifyAgents(req, podId, task, 'updated', previousTask); return res.json({ task }); } catch (err) { diff --git a/backend/services/githubGigService.ts b/backend/services/githubGigService.ts index b6371f125b..537d5f92ba 100644 --- a/backend/services/githubGigService.ts +++ b/backend/services/githubGigService.ts @@ -96,10 +96,10 @@ export const completeTaskForMergedPull = async (podId: string, href: string): Pr { new: true }, ); if (!task) return; + recordTaskTransition({ podId, task, from: null, to: 'done', at }); // eslint-disable-next-line no-await-in-loop await onTaskClosed(task); emitTaskUpdated(podId, task, 'updated'); - recordTaskTransition({ podId, task, from: null, to: 'done', at }); } }; diff --git a/backend/services/roomGrantService.ts b/backend/services/roomGrantService.ts index 6a1e1cf25b..7de0c3fce1 100644 --- a/backend/services/roomGrantService.ts +++ b/backend/services/roomGrantService.ts @@ -9,6 +9,7 @@ import RoomGrant, { RoomGrantTaskBinding, RoomGrantWriteMode, } from '../models/RoomGrant'; +import { BOARD_ACTOR, emitRecord, type RecordActor } from './recordService'; export interface RoomGrantCreateInput { grantId?: string; @@ -27,6 +28,8 @@ export interface RoomGrantCreateInput { connectionRowId?: string; /** The row's owner at mint; set by the mint, never the body. */ ownerId?: string; + /** Who issued it, from authentication; set by the route, never the body. */ + issuer?: RecordActor | null; } export interface RoomGrantAttenuationInput { @@ -39,6 +42,8 @@ export interface RoomGrantAttenuationInput { expiresAt?: Date | string | number; /** Accepted for API compatibility but never copied into the row. */ grantId?: string; + /** The delegating agent, from authentication; set by the route, never the body. */ + issuer?: RecordActor | null; } export interface GrantUsabilityOptions { @@ -339,16 +344,72 @@ const assertTaskContext = async (grant: IRoomGrant | Record, op } }; -/** Hook successful close transitions. An old close never revokes new grants issued after reopen. */ +/** + * Hook successful close transitions. An old close never revokes new grants issued after reopen. + * updateMany reports a count, not ids, so the record line reads back the grants + * carrying this write's own `revokedAt`. A grant an earlier close or another + * writer revoked keeps its own timestamp and is not named again. + */ export const revokeTaskGrants = async ({ taskId, podId, closedEpoch }: Pick & { closedEpoch: number }): Promise => { - const result = await RoomGrant.updateMany({ + const filter = { 'binding.taskId': taskId, 'binding.podId': podId, taskEpoch: { $lt: closedEpoch }, revokedAt: null, - }, { $set: { revokedAt: new Date(), revocationReason: 'task.done' } }); + }; + const revokedAt = new Date(); + const result = await RoomGrant.updateMany(filter, { $set: { revokedAt, revocationReason: 'task.done' } }); + if (result.modifiedCount > 0) { + const revoked = await RoomGrant.find({ + 'binding.taskId': taskId, 'binding.podId': podId, revokedAt, revocationReason: 'task.done', + }).select('grantId').lean(); + const grantIds = Array.from(new Set(revoked.map((grant) => String(grant.grantId)))); + // ADR-033 §4: the close's own task.done line is queued first, by the caller. + if (grantIds.length) { + emitRecord({ podId, taskId }, `grant.revoked/task.done/${closedEpoch}`, [ + { kind: 'grant.revoked', actor: BOARD_ACTOR, data: { grantIds, reason: 'task.done' } }, + ]); + } + } return result.modifiedCount; }; +/** + * The record's grant.issued line, for a task-bound grant only: a record chain + * is one task's, and the line's binding must name that chain's task. Unbound + * issuance has no task chain to land on and belongs to the grant writers + * (TASK-513). Never throws, and runs only after every post-insert check passed. + */ +const recordGrantIssued = (grant: IRoomGrant, issuer: RecordActor | null | undefined): void => { + const binding = normalizeTaskBinding(grant.binding); + if (!binding) return; + const calls = grant.budget?.calls; + const windowMs = grant.budget?.windowMs; + const budget = typeof calls === 'number' || typeof windowMs === 'number' + ? { ...(typeof calls === 'number' ? { calls } : {}), ...(typeof windowMs === 'number' ? { windowMs } : {}) } + : undefined; + emitRecord({ podId: binding.podId, taskId: binding.taskId }, `grant.issued/${grant.grantId}`, [{ + kind: 'grant.issued', + // Explicit, because the format's default reads issuedBy as a human: an + // agent delegating a child is named as the agent it is. + actor: issuer ?? null, + data: { + grantId: grant.grantId, + ...(grant.parentGrantId ? { parentGrantId: grant.parentGrantId } : {}), + scope: { + connectionId: grant.connectionId, + tools: [...grant.tools], + writeMode: grant.writeMode, + ...(budget ? { budget } : {}), + audience: (grant.audience || []).map(String), + }, + target: { kind: grant.target.kind, id: String(grant.target.id) }, + ...(issuer?.id ? { issuedBy: issuer.id } : {}), + expiresAt: new Date(grant.expiresAt).toISOString(), + binding, + }, + }]); +}; + export const mintGrant = async (input: RoomGrantCreateInput): Promise => { if (input.expiresAt === undefined || input.expiresAt === null || input.expiresAt === '') { throw new RoomGrantError('missing_expiry', 'expiresAt is required'); @@ -448,6 +509,7 @@ export const mintGrant = async (input: RoomGrantCreateInput): Promise