From 0dccb0588118109107fe88ba3f84069e73924086 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:31:21 -0700 Subject: [PATCH 1/5] feat(record): grant and connector-call lines in the pod's record (TASK-560) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TASK-513 slice 2, on top of TASK-559. Room grants and brokered connector calls write record-v1 lines to their pod's taskless chain, using ADR-033's B1–B5 table field for field. No line carries argsDigest (CTO launch rule, ADR-033 §8 q9). - B1 refused, B2 parked (asked of the credential owner, on its card) and B5 executed come from the broker's trail write. A trail row that carries an approval id and is not the park is an approved call's run; the resolver records that, so each call gets one terminal decision. - B3 is written by resolveApproval once the ending is saved: the owner's decision, then tool.update from the ending. B4 is written on expiry at decide time and in the sweep. - grant.issued comes from the two routes that mint: the owner as a person, an attenuating agent as an agent. revokeGrant now returns what it moved (RoomGrant.revokeCascadeMoved), giving manual or cascade per pod. A grant found expired at use is revoked once, by the server. - Pod-target grants only. A seat grant names no pod and writes nothing. - GET /api/pods/:podId/record serves the pod chain under the task route's gate. Co-Authored-By: Claude Opus 5.5 Co-authored-by: otto Agent-Model: claude/claude-opus-5-5/default --- .../service/grantRecord.realPg.test.js | 226 ++++++++++++++++++ .../__tests__/unit/routes/grants.mint.test.js | 41 +++- .../services/approvalToolCallService.test.js | 54 +++++ .../unit/services/brokerRecordLines.test.js | 127 ++++++++++ .../unit/services/brokerRecordService.test.js | 152 ++++++++++++ .../unit/services/toolBrokerService.test.js | 38 +++ backend/models/RoomGrant.ts | 20 +- backend/routes/grants.ts | 11 + backend/routes/podRecords.ts | 25 +- backend/services/approvalActionService.ts | 9 + backend/services/brokerRecordLines.ts | 194 +++++++++++++++ backend/services/brokerRecordService.ts | 153 ++++++++++++ backend/services/recordService.ts | 23 +- backend/services/roomGrantService.ts | 15 +- backend/services/toolBrokerService.ts | 12 + docs/plans/task-513-server-record.md | 27 +++ 16 files changed, 1109 insertions(+), 18 deletions(-) create mode 100644 backend/__tests__/service/grantRecord.realPg.test.js create mode 100644 backend/__tests__/unit/services/brokerRecordLines.test.js create mode 100644 backend/__tests__/unit/services/brokerRecordService.test.js create mode 100644 backend/services/brokerRecordLines.ts create mode 100644 backend/services/brokerRecordService.ts diff --git a/backend/__tests__/service/grantRecord.realPg.test.js b/backend/__tests__/service/grantRecord.realPg.test.js new file mode 100644 index 0000000000..bd3351cde9 --- /dev/null +++ b/backend/__tests__/service/grantRecord.realPg.test.js @@ -0,0 +1,226 @@ +/* eslint-disable import/no-unresolved, import/extensions -- TypeScript modules expose CJS exports. */ +/** + * TASK-560: grants and brokered calls reach the pod's record through the + * real broker, the real approval resolver and the real grant service, on real + * MongoDB (grants, approvals, the board) and real PostgreSQL (tool_calls, the + * chain). Only the vendor and the delivery side effects are stubbed: GitHub, + * the seat-confinement and outsider-pod guards, card posting, sockets and + * event delivery. None of them decides what the record says. + */ +process.env.JWT_SECRET = process.env.JWT_SECRET || 'grant-record-test-secret'; + +const mockGithub = { + createIssue: jest.fn(), + closeIssue: jest.fn(), + listOpenIssues: jest.fn(), + getIssue: jest.fn(), +}; +jest.mock('../../services/githubAppService', () => mockGithub); +jest.mock('../../services/seatGrantConfinement', () => ({ + judgeSeatConfinement: jest.fn(async () => ({ refusal: null, scope: 'unbound' })), +})); +jest.mock('../../services/outsiderGrantPodEligibility', () => ({ + outsiderGrantPodRefusal: jest.fn(async () => null), + outsiderGrantRefusalMessage: jest.fn(() => 'refused'), +})); +jest.mock('../../services/agentMessageService', () => ({ + postMessage: jest.fn(async () => ({ success: true, message: { _id: 'card-message' } })), +})); +jest.mock('../../config/socket', () => ({ getIO: jest.fn(() => null) })); +jest.mock('../../services/agentEventService', () => ({ enqueue: jest.fn(async () => ({})) })); + +const express = require('express'); +const request = require('supertest'); +const Pod = require('../../models/Pod'); +const User = require('../../models/User'); +const Integration = require('../../models/Integration').default || require('../../models/Integration'); +const RoomGrant = require('../../models/RoomGrant').default; +const ApprovalAction = require('../../models/ApprovalAction').default || require('../../models/ApprovalAction'); +const { pool } = require('../../config/db-pg'); +const roomGrants = require('../../services/roomGrantService'); +const { callTool } = require('../../services/toolBrokerService'); +const approvals = require('../../services/approvalActionService'); +const { recordGrantIssued } = require('../../services/brokerRecordService'); +const { flushRecordWrites, humanActor, agentActor } = require('../../services/recordService'); +const { verifyRecord } = require('../../vendor/seats/src/record/format'); +const { toOtlpJson } = require('../../vendor/seats/src/record/otel'); +const { + setupMongoDb, closeMongoDb, clearMongoDb, setupPgDb, closePgDb, generateTestToken, +} = require('../utils/testUtils'); + +const real = process.env.INTEGRATION_TEST === 'true' ? describe : describe.skip; +const flushAll = async () => { + // Approval outcomes read the grant before they queue; let that settle first. + await new Promise((resolve) => { setTimeout(resolve, 50); }); + await flushRecordWrites(); +}; + +real('grant and broker lines on real PostgreSQL and MongoDB', () => { + let app; + let owner; + let agent; + let pod; + + beforeAll(async () => { + await setupMongoDb(); + await setupPgDb(); + app = express(); + app.use(express.json()); + app.use('/api/pods', require('../../routes/podRecords')); + }); + + beforeEach(async () => { + await clearMongoDb(); + const stamp = Date.now(); + owner = await User.create({ username: `grant-owner-${stamp}`, email: `grant-owner-${stamp}@test.com`, password: 'Password123!', verified: true }); + agent = await User.create({ + username: `grant-agent-${stamp}`, email: `grant-agent-${stamp}@agents.commonly.local`, password: 'Password123!', + verified: true, isBot: true, botType: 'agent', botMetadata: { agentName: 'grant-agent', instanceId: 'default', ownerUserId: String(owner._id) }, + }); + pod = await Pod.create({ name: 'Grant pod', type: 'team', createdBy: owner._id, members: [owner._id, agent._id] }); + await Integration.create({ + podId: pod._id, type: 'github-app', status: 'connected', createdBy: owner._id, installationId: 'install-1', + config: { installationId: 'install-1', owner: 'octo', repo: 'demo' }, + createdAt: new Date(Date.now() - 60_000), + }); + mockGithub.createIssue.mockResolvedValue({ number: 1, title: 'x', html_url: 'https://github.com/octo/demo/issues/1' }); + mockGithub.closeIssue.mockResolvedValue({ number: 7, title: 'closed', html_url: 'https://github.com/octo/demo/issues/7', state: 'closed' }); + process.env.RECORD_V1_PODS = String(pod._id); + }); + + afterEach(async () => { + await flushAll(); + await pool.query('DELETE FROM record_lines WHERE chain_id IN (SELECT id FROM record_chains WHERE pod_id = $1)', [String(pod._id)]); + await pool.query('DELETE FROM record_chains WHERE pod_id = $1', [String(pod._id)]); + delete process.env.RECORD_V1_PODS; + }); + + afterAll(async () => { + await clearMongoDb(); + await closeMongoDb(); + await closePgDb(); + }); + + const mint = async (over = {}) => { + const grant = await roomGrants.createGrant({ + connectionId: 'install-1', installationId: 'install-1', brokerId: 'commonly-grant-broker', + target: { kind: 'pod', id: String(pod._id) }, tools: ['github.create_issue', 'github.close_issue'], + writeMode: 'write-with-confirm', audience: [String(agent._id)], + expiresAt: new Date(Date.now() + 86_400_000), ...over, + }); + // What POST /api/grants does after the mint (its wiring is in grants.mint.test.js). + recordGrantIssued(grant.toObject(), humanActor(owner._id)); + return grant; + }; + const readPod = async () => { + await flushAll(); + const res = await request(app).get(`/api/pods/${pod._id}/record`).set('Authorization', `Bearer ${generateTestToken(owner._id)}`); + expect(res.status).toBe(200); + expect(verifyRecord(res.text)).toMatchObject({ ok: true }); + return { text: res.text, lines: res.text ? res.text.trimEnd().split('\n').map((line) => JSON.parse(line)) : [] }; + }; + const call = (grant, tool, args) => callTool({ grantId: grant.grantId, agentUserId: String(agent._id), tool, args }); + const kinds = (lines) => lines.map((l) => (l.kind === 'permission.decided' ? `decided:${l.data.approval || l.data.rule.list}` : l.kind)); + + it('issue, attenuate, revoke cascade and expiry', async () => { + const root = await mint(); + const child = await roomGrants.attenuateGrant({ + parentGrantId: root.grantId, tools: ['github.close_issue'], expiresAt: new Date(Date.now() + 3_600_000), + }); + recordGrantIssued(child.toObject(), agentActor(agent)); + expect(await roomGrants.revokeGrant(root.grantId, String(owner._id))).toBe(2); + const late = await mint(); + await RoomGrant.updateOne({ grantId: late.grantId }, { $set: { expiresAt: new Date(Date.now() - 1000) } }); + for (let i = 0; i < 2; i += 1) { + // eslint-disable-next-line no-await-in-loop + await expect(roomGrants.assertGrantUsable({ grantId: late.grantId, agentUserId: String(agent._id), currentMemberIds: [String(agent._id)] })) + .rejects.toMatchObject({ code: 'grant_expired' }); + } + + const { lines } = await readPod(); + expect(lines.map((l) => [l.kind, l.data.grantId || l.data.grantIds])).toEqual([ + ['grant.issued', root.grantId], + ['grant.issued', child.grantId], + ['grant.revoked', [root.grantId, child.grantId]], + ['grant.issued', late.grantId], + ['grant.revoked', [late.grantId]], + ]); + expect(lines[1]).toMatchObject({ actor: { type: 'agent', id: String(agent._id), owner: String(owner._id) }, data: { parentGrantId: root.grantId } }); + expect(lines[2]).toMatchObject({ actor: { type: 'human', id: String(owner._id) }, data: { reason: 'cascade' } }); + expect(lines[4]).toMatchObject({ actor: { type: 'system', id: 'commonly' }, data: { reason: 'expired' } }); + }); + + it('B1 refused, B5 executed, B2 parked then B3 declined: one terminal decision per call', async () => { + const grant = await mint({ writeMode: 'write', tools: ['github.close_issue', 'github.create_issue'] }); + await expect(call(grant, 'github.list_issues', {})).rejects.toMatchObject({ code: 'tool_not_allowed' }); + await call(grant, 'github.close_issue', { issueNumber: 7 }); + const parked = await call(grant, 'github.create_issue', { title: 'needs approval' }).catch((error) => error); + expect(parked).toMatchObject({ code: 'approval_required' }); + const declined = await approvals.resolveApproval({ approvalId: parked.details.approvalId, callerUserId: String(owner._id), decision: 'declined' }); + expect(declined.status).toBe(200); + + const { lines, text } = await readPod(); + const calls = lines.filter((l) => l.kind !== 'grant.issued'); + expect(kinds(calls)).toEqual([ + 'tool.call', 'permission.requested', 'decided:deny', 'tool.update', + 'tool.call', 'permission.requested', 'decided:allow', 'tool.update', + 'tool.call', 'permission.requested', 'decided:rejected', 'tool.update', + ]); + const parkedId = parked.details.callId; + const forParked = calls.filter((l) => l.data.toolCallId === parkedId); + expect(forParked.map((l) => l.kind)).toEqual(['tool.call', 'permission.requested', 'permission.decided', 'tool.update']); + expect(forParked[1].data).toMatchObject({ askedOf: { id: String(owner._id), role: 'credential-owner' }, decisionCardId: parked.details.approvalId }); + expect(forParked[2]).toMatchObject({ actor: { type: 'human', id: String(owner._id) }, data: { decidedBy: String(owner._id), decidedByRole: 'credential-owner' } }); + // The declined call's own PG trail row (refused/approval_declined) did not add a second decision. + expect(calls.filter((l) => l.kind === 'permission.decided')).toHaveLength(3); + expect(text).not.toMatch(/argsDigest|sha256:/); + expect(JSON.stringify(toOtlpJson(lines, { serviceName: 'commonly' }))).not.toMatch(/args_digest|argsDigest/); + }); + + it('B2 parked then B3 approved and run, and B4 timed out in the sweep', async () => { + const grant = await mint(); + const first = await call(grant, 'github.create_issue', { title: 'approve me' }).catch((error) => error); + const resolved = await approvals.resolveApproval({ approvalId: first.details.approvalId, callerUserId: String(owner._id), decision: 'approved' }); + expect(resolved.status).toBe(200); + expect(mockGithub.createIssue).toHaveBeenCalledTimes(1); + const second = await call(grant, 'github.create_issue', { title: 'let it lapse' }).catch((error) => error); + await ApprovalAction.updateOne({ _id: second.details.approvalId }, { $set: { expiresAt: new Date(Date.now() - 1000) } }); + expect(await approvals.sweepExpiredToolCallApprovals()).toBe(1); + + const { lines } = await readPod(); + const of = (id) => kinds(lines.filter((l) => l.data.toolCallId === id)); + expect(of(first.details.callId)).toEqual(['tool.call', 'permission.requested', 'decided:approved', 'tool.update']); + expect(lines.filter((l) => l.data.toolCallId === first.details.callId).pop().data.status).toBe('completed'); + expect(of(second.details.callId)).toEqual(['tool.call', 'permission.requested', 'decided:timed_out', 'tool.update']); + // The approved run wrote its own ok trail row in PG; it adds no line of its own. + expect(lines.filter((l) => l.kind === 'tool.call')).toHaveLength(2); + }); + + it('the pod chain keeps the task route\'s gate: outsiders 404, admin non-members 403', async () => { + await mint(); + await flushAll(); + const stamp = Date.now(); + const outsider = await User.create({ username: `out-${stamp}`, email: `out-${stamp}@test.com`, password: 'Password123!', verified: true }); + const admin = await User.create({ username: `adm-${stamp}`, email: `adm-${stamp}@test.com`, password: 'Password123!', verified: true, role: 'admin' }); + const get = (user) => request(app).get(`/api/pods/${pod._id}/record`).set('Authorization', `Bearer ${generateTestToken(user._id)}`); + expect(await get(outsider)).toMatchObject({ status: 404, body: { error: 'record_not_found' } }); + expect(await get(admin)).toMatchObject({ status: 403, body: { error: 'record_requires_membership' } }); + expect((await get(owner)).headers['x-record-entries']).toBe('1'); + }); + + it('a seat grant writes nothing to any chain, even with the record on everywhere', async () => { + // With the pilot gate open, only the pod-target rule stands between a seat + // grant and a chain keyed by the seat's id. + process.env.RECORD_V1_PODS = '*'; + await roomGrants.createGrant({ + connectionId: 'install-1', installationId: 'install-1', brokerId: 'commonly-grant-broker', + target: { kind: 'seat', id: String(agent._id) }, tools: ['github.close_issue'], writeMode: 'write', + audience: [String(agent._id)], expiresAt: new Date(Date.now() + 86_400_000), + }); + const seat = await RoomGrant.findOne({ 'target.kind': 'seat' }); + await call(seat, 'github.close_issue', { issueNumber: 7 }).catch(() => {}); + await flushAll(); + const { rows } = await pool.query('SELECT count(*)::int AS n FROM record_chains WHERE pod_id = ANY($1)', [[String(pod._id), String(agent._id)]]); + expect(rows[0].n).toBe(0); + }); +}); diff --git a/backend/__tests__/unit/routes/grants.mint.test.js b/backend/__tests__/unit/routes/grants.mint.test.js index 15478037b6..03c9b65039 100644 --- a/backend/__tests__/unit/routes/grants.mint.test.js +++ b/backend/__tests__/unit/routes/grants.mint.test.js @@ -18,7 +18,16 @@ jest.mock('../../../middleware/auth', () => (req, res, next) => { req.userId = id; return next(); }); -jest.mock('../../../middleware/agentRuntimeAuth', () => (req, res, next) => next()); +jest.mock('../../../middleware/agentRuntimeAuth', () => (req, res, next) => { + const agent = req.get('x-test-agent'); + if (agent) req.agentUser = { _id: agent, isBot: true, botMetadata: { ownerUserId: req.get('x-test-agent-owner') } }; + return next(); +}); +const mockRecordGrantIssued = jest.fn(); +jest.mock('../../../services/brokerRecordService', () => ({ + ...jest.requireActual('../../../services/brokerRecordService'), + recordGrantIssued: (...args) => mockRecordGrantIssued(...args), +})); jest.mock('../../../models/Pod', () => ({ findById: jest.fn() })); jest.mock('../../../models/Integration', () => ({ findById: jest.fn(), findOne: jest.fn() })); jest.mock('../../../models/Installable', () => ({ findOne: jest.fn() })); @@ -409,3 +418,33 @@ describe('POST /api/grants on a hosted-mcp connection', () => { expect(await RoomGrant.countDocuments({})).toBe(0); }); }); + +// TASK-560: a minted grant reaches the record once, issued by whoever minted it. +describe('record wiring', () => { + test('the owner mints: grant.issued names the owner as a person', async () => { + const res = await mint({}); + expect(res.status).toBe(201); + expect(mockRecordGrantIssued).toHaveBeenCalledTimes(1); + const [grant, issuer] = mockRecordGrantIssued.mock.calls[0]; + expect(grant).toMatchObject({ grantId: res.body.grantId, target: { kind: 'pod', id: POD } }); + expect(typeof grant.toObject).toBe('undefined'); + expect(issuer).toEqual({ type: 'human', id: OWNER }); + }); + + test('a refused mint writes nothing', async () => { + expect((await mint({ audience: [] })).status).toBe(400); + expect(mockRecordGrantIssued).not.toHaveBeenCalled(); + }); + + test('an agent attenuates: the child is issued by that agent, as an agent', async () => { + const parent = await mint({}); + mockRecordGrantIssued.mockClear(); + const res = await request(app).post(`/api/grants/${parent.body.grantId}/attenuate`) + .set('x-test-agent', SEAT).set('x-test-agent-owner', OWNER) + .send({ tools: ['github.list_issues'], expiresAt: new Date(Date.now() + 86400000).toISOString() }); + expect(res.status).toBe(201); + const [child, issuer] = mockRecordGrantIssued.mock.calls[0]; + expect(child).toMatchObject({ grantId: res.body.grantId, parentGrantId: parent.body.grantId }); + expect(issuer).toEqual({ type: 'agent', id: SEAT, owner: OWNER }); + }); +}); diff --git a/backend/__tests__/unit/services/approvalToolCallService.test.js b/backend/__tests__/unit/services/approvalToolCallService.test.js index b8a870e5a3..fb34dbca29 100644 --- a/backend/__tests__/unit/services/approvalToolCallService.test.js +++ b/backend/__tests__/unit/services/approvalToolCallService.test.js @@ -30,6 +30,10 @@ jest.mock('../../../config/socket', () => ({ getIO: jest.fn(() => null) })); const mockEnqueue = jest.fn().mockResolvedValue({}); jest.mock('../../../services/agentEventService', () => ({ enqueue: (...args) => mockEnqueue(...args) })); jest.mock('../../../models/Message', () => ({ updateOne: jest.fn().mockResolvedValue({}) })); +const mockRecordApprovalOutcome = jest.fn(); +jest.mock('../../../services/brokerRecordService', () => ({ + recordApprovalOutcome: (...args) => mockRecordApprovalOutcome(...args), +})); const service = require('../../../services/approvalActionService'); const broker = require('../../../services/toolBrokerService'); @@ -380,3 +384,53 @@ test('every terminal state, including moot, scrubs canonicalArgs and keeps argsD expect(pending.toolCall.argsDigest).toBe(toolCall.argsDigest); expect(pending.toolCall.canonicalArgs).toBeUndefined(); }); + +// TASK-560: one terminal record per parked call, after its ending is saved. +describe('record wiring', () => { + const outcomeCall = () => { + expect(mockRecordApprovalOutcome).toHaveBeenCalledTimes(1); + return mockRecordApprovalOutcome.mock.calls[0]; + }; + + test('approved: after the run, with the approver, and the saved ending', async () => { + const pending = row(); + const resolved = row({ status: 'resolved', decision: 'approved' }); + mockApproval.findById.mockResolvedValue(pending); + mockApproval.findOneAndUpdate.mockResolvedValue(resolved); + broker.executeApprovedToolCall.mockResolvedValue({ callId: 'tool_call_second', outcome: 'ok', result: {} }); + await service.resolveApproval({ approvalId: pending._id, callerUserId: OWNER, decision: 'approved' }); + const [approval, parked, outcome] = outcomeCall(); + expect(approval).toBe(resolved); + expect(approval.toolCallEnding).toBe('ran'); + expect(parked).toMatchObject({ grantId: 'grant-1', callId: 'tool_call_original', tool: toolCall.tool }); + expect(outcome).toEqual({ kind: 'approved', resolvedBy: OWNER }); + }); + + test('declined', async () => { + const pending = row(); + mockApproval.findById.mockResolvedValue(pending); + mockApproval.findOneAndUpdate.mockResolvedValue(row({ status: 'resolved', decision: 'declined' })); + await service.resolveApproval({ approvalId: pending._id, callerUserId: OWNER, decision: 'declined' }); + expect(outcomeCall()[2]).toEqual({ kind: 'declined', resolvedBy: OWNER }); + }); + + test('expired at decide time, and in the sweep', async () => { + mockApproval.findById.mockResolvedValue(row({ expiresAt: new Date(Date.now() - 1) })); + mockApproval.findOneAndUpdate.mockResolvedValue(row({ status: 'expired' })); + await service.resolveApproval({ approvalId: 'approval-1', callerUserId: OWNER, decision: 'approved' }); + expect(outcomeCall()[2]).toEqual({ kind: 'expired' }); + mockRecordApprovalOutcome.mockClear(); + mockApproval.find.mockResolvedValue([row({ expiresAt: new Date(Date.now() - 1) })]); + await service.sweepExpiredToolCallApprovals(); + expect(outcomeCall()[2]).toEqual({ kind: 'expired' }); + }); + + test('a lost race and a refused decider write nothing', async () => { + mockApproval.find.mockResolvedValue([row({ expiresAt: new Date(Date.now() - 1) })]); + mockApproval.findOneAndUpdate.mockResolvedValue(null); + await service.sweepExpiredToolCallApprovals(); + mockApproval.findById.mockResolvedValue(row()); + await service.resolveApproval({ approvalId: 'approval-1', callerUserId: OTHER, decision: 'approved' }); + expect(mockRecordApprovalOutcome).not.toHaveBeenCalled(); + }); +}); diff --git a/backend/__tests__/unit/services/brokerRecordLines.test.js b/backend/__tests__/unit/services/brokerRecordLines.test.js new file mode 100644 index 0000000000..6408e64f88 --- /dev/null +++ b/backend/__tests__/unit/services/brokerRecordLines.test.js @@ -0,0 +1,127 @@ +/* eslint-disable import/no-unresolved, import/extensions -- TypeScript modules expose CJS exports. */ +const { appendEntry, verifyRecord } = require('../../../vendor/seats/src/record/format'); +const { toOtlpJson } = require('../../../vendor/seats/src/record/otel'); +const lines = require('../../../services/brokerRecordLines'); + +const POD = '6a692a1be833c668acdb84cf'; +const AGENT = { type: 'agent', id: '6a920d43b08dee6c42a81c9e', owner: '6a47891ee025ce2e0ca1d30b' }; +const OWNER = '6a47891ee025ce2e0ca1d30b'; +const CALL = { toolCallId: 'tool_call_1', tool: 'sentry.resolve_issue', grantId: 'grant_abc', agent: AGENT }; + +// Every line goes through the shared writer, so a field the format refuses fails here. +const write = (inputs) => { + let prev = null; + const out = inputs.map((input, index) => { + prev = appendEntry(prev, { + runId: 'srv-pod-0123456789abcdef', podId: POD, ts: `2026-10-26T05:00:${String(index).padStart(2, '0')}.000Z`, + kind: input.kind, ...(input.actor === undefined ? {} : { actor: input.actor }), data: input.data, + }); + return prev; + }); + return out.map((line) => `${line}\n`).join(''); +}; +const parse = (jsonl) => jsonl.trimEnd().split('\n').map((line) => JSON.parse(line)); + +describe('ADR-033 B1–B5 broker lines', () => { + it('B1 refused: asked of the grant policy, denied, failed', () => { + const entries = parse(write(lines.refusedCallLines(CALL))); + expect(entries.map((e) => e.kind)).toEqual(['tool.call', 'permission.requested', 'permission.decided', 'tool.update']); + expect(entries[0]).toMatchObject({ actor: AGENT, data: { toolCallId: 'tool_call_1', title: CALL.tool, name: CALL.tool, surface: 'connector' } }); + expect(entries[1].data).toMatchObject({ askedOf: { id: 'grant:grant_abc', role: 'policy' }, question: CALL.tool, grantId: 'grant_abc' }); + expect(entries[1].data.options).toEqual([{ optionId: 'approve', kind: 'allow_once' }, { optionId: 'decline', kind: 'reject_once' }]); + expect(entries[2]).toMatchObject({ + actor: { type: 'policy', id: 'grant:grant_abc' }, + data: { optionId: 'decline', kind: 'reject_once', rule: { list: 'deny', pattern: null }, decidedBy: 'policy', decidedByRole: 'policy' }, + }); + expect(entries[2].data.approval).toBeUndefined(); + expect(entries[3].data).toEqual({ toolCallId: 'tool_call_1', status: 'failed' }); + }); + + it('B2 parked then B3 approved: asked of the credential owner, decided by them on the card', () => { + const jsonl = write([ + ...lines.parkedCallLines(CALL, OWNER, 'approval_1'), + ...lines.resolvedCallLines(CALL, true, OWNER, 'approval_1'), + lines.toolUpdate(CALL, 'completed'), + ]); + const entries = parse(jsonl); + expect(entries.map((e) => e.kind)).toEqual(['tool.call', 'permission.requested', 'permission.decided', 'tool.update']); + expect(entries[1].data).toMatchObject({ askedOf: { id: OWNER, role: 'credential-owner' }, decisionCardId: 'approval_1' }); + expect(entries[2]).toMatchObject({ + actor: { type: 'human', id: OWNER }, + data: { optionId: 'approve', approval: 'approved', rule: { list: 'ask', pattern: null }, decidedBy: OWNER, decidedByRole: 'credential-owner', decisionCardId: 'approval_1' }, + }); + expect(entries[3].data.status).toBe('completed'); + }); + + it('B3 declined writes the decision and a failed update in one event', () => { + const entries = parse(write(lines.resolvedCallLines(CALL, false, OWNER))); + expect(entries.map((e) => [e.kind, e.data.approval || e.data.status])).toEqual([ + ['permission.decided', 'rejected'], ['tool.update', 'failed'], + ]); + }); + + it('B4 expired: the policy declines as timed out', () => { + const entries = parse(write(lines.expiredCallLines(CALL, 'approval_1'))); + expect(entries[0]).toMatchObject({ + actor: { type: 'policy', id: 'grant:grant_abc' }, + data: { approval: 'timed_out', decidedBy: 'policy', decidedByRole: 'policy', decisionCardId: 'approval_1' }, + }); + expect(entries[1].data.status).toBe('failed'); + }); + + it('B5 executed: allowed by the grant policy, completed or failed', () => { + expect(parse(write(lines.executedCallLines(CALL, true))).map((e) => e.data.status || e.data.rule?.list)) + .toEqual([undefined, undefined, 'allow', 'completed']); + expect(parse(write(lines.executedCallLines(CALL, false)))[3].data.status).toBe('failed'); + }); + + it('carries no argsDigest on any line, in JSONL or OTLP (CTO launch rule)', () => { + const jsonl = write([ + ...lines.refusedCallLines(CALL), + ...lines.parkedCallLines({ ...CALL, toolCallId: 'tool_call_2' }, OWNER, 'approval_1'), + ...lines.resolvedCallLines({ ...CALL, toolCallId: 'tool_call_2' }, true, OWNER, 'approval_1'), + ...lines.expiredCallLines({ ...CALL, toolCallId: 'tool_call_3' }), + ...lines.executedCallLines({ ...CALL, toolCallId: 'tool_call_4' }, true), + ]); + expect(verifyRecord(jsonl).ok).toBe(true); + expect(jsonl).not.toMatch(/argsDigest|args_digest|sha256:/); + expect(JSON.stringify(toOtlpJson(parse(jsonl), { serviceName: 'commonly' }))).not.toMatch(/args_digest|argsDigest|sha256:/); + }); + + it('refuses a credential-owner role from a pod member: the format allows pod-member only on decision.ruled', () => { + const bad = lines.resolvedCallLines(CALL, true, OWNER)[0]; + bad.data.decidedByRole = 'pod-member'; + expect(() => write([bad])).toThrow(/record:/); + }); +}); + +describe('grant lines', () => { + const grant = { + grantId: 'grant_child', parentGrantId: 'grant_root', connectionId: 'conn_1', tools: ['sentry.resolve_issue'], + writeMode: 'write-with-confirm', budget: { calls: 5 }, audience: [OWNER], target: { kind: 'pod', id: POD }, + expiresAt: new Date('2026-10-27T00:00:00.000Z'), + }; + + it('grant.issued names connection, exact tools, mode, budget, audience, target, issuer and expiry', () => { + const [entry] = parse(write([lines.grantIssuedLine(grant, { type: 'human', id: OWNER })])); + expect(entry).toMatchObject({ + actor: { type: 'human', id: OWNER }, + data: { + grantId: 'grant_child', parentGrantId: 'grant_root', + scope: { connectionId: 'conn_1', tools: ['sentry.resolve_issue'], writeMode: 'write-with-confirm', budget: { calls: 5 }, audience: [OWNER] }, + target: { kind: 'pod', id: POD }, issuedBy: OWNER, expiresAt: '2026-10-27T00:00:00.000Z', + }, + }); + }); + + it('an attenuating agent is the issuer, as an agent', () => { + const [entry] = parse(write([lines.grantIssuedLine(grant, AGENT)])); + expect(entry.actor).toEqual(AGENT); + expect(entry.data.issuedBy).toBe(AGENT.id); + }); + + it('grant.revoked: cascade lists the ancestor first', () => { + const [entry] = parse(write([lines.grantRevokedLine(['grant_root', 'grant_child'], 'cascade', { type: 'human', id: OWNER })])); + expect(entry.data).toEqual({ grantIds: ['grant_root', 'grant_child'], reason: 'cascade' }); + }); +}); diff --git a/backend/__tests__/unit/services/brokerRecordService.test.js b/backend/__tests__/unit/services/brokerRecordService.test.js new file mode 100644 index 0000000000..c44e157edb --- /dev/null +++ b/backend/__tests__/unit/services/brokerRecordService.test.js @@ -0,0 +1,152 @@ +/* eslint-disable import/no-unresolved, import/extensions -- TypeScript modules expose CJS exports. */ +const mockEmits = []; +jest.mock('../../../services/recordService', () => { + const actual = jest.requireActual('../../../services/recordService'); + return { + ...actual, + emitRecord: jest.fn((ref, key, inputs) => { mockEmits.push({ ref, key, inputs }); }), + }; +}); +const mockUser = { findById: jest.fn() }; +jest.mock('../../../models/User', () => mockUser); +const mockRoomGrant = { findOne: jest.fn() }; +jest.mock('../../../models/RoomGrant', () => ({ __esModule: true, default: mockRoomGrant })); + +const service = require('../../../services/brokerRecordService'); + +const POD = '6a692a1be833c668acdb84cf'; +const AGENT = '6a920d43b08dee6c42a81c9e'; +const OWNER = '6a47891ee025ce2e0ca1d30b'; +const podGrant = { grantId: 'grant_1', target: { kind: 'pod', id: POD } }; +const seatGrant = { grantId: 'grant_2', target: { kind: 'seat', id: AGENT } }; +const lean = (value) => ({ select: () => ({ lean: async () => value }) }); + +// Resolve one emit's lines, whether given directly or built in the queue. +const linesOf = async (emit) => (typeof emit.inputs === 'function' ? emit.inputs() : emit.inputs); +const flush = () => new Promise((resolve) => { setImmediate(resolve); }); + +beforeEach(() => { + mockEmits.length = 0; + process.env.RECORD_V1_PODS = POD; + mockUser.findById.mockReturnValue(lean({ _id: AGENT, isBot: true, botMetadata: { ownerUserId: OWNER } })); + mockRoomGrant.findOne.mockReturnValue(lean(podGrant)); +}); +afterAll(() => { delete process.env.RECORD_V1_PODS; }); + +const row = (over) => ({ grant: podGrant, callId: 'tool_call_1', grantId: 'grant_1', agentUserId: AGENT, tool: 'sentry.resolve_issue', ...over }); + +describe('recordBrokerCall', () => { + it.each([ + ['refused', {}, ['tool.call', 'permission.requested', 'permission.decided', 'tool.update']], + ['ok', {}, ['tool.call', 'permission.requested', 'permission.decided', 'tool.update']], + ['failed', {}, ['tool.call', 'permission.requested', 'permission.decided', 'tool.update']], + ['pending_approval', { approvalId: 'approval_1', credentialOwnerId: OWNER }, ['tool.call', 'permission.requested']], + ])('%s writes its ADR-033 lines to the grant pod chain', async (outcome, over, kinds) => { + service.recordBrokerCall(row({ outcome, ...over })); + expect(mockEmits).toHaveLength(1); + expect(mockEmits[0].ref).toEqual({ podId: POD }); + expect(mockEmits[0].key).toBe('tool_call/tool_call_1'); + const lines = await linesOf(mockEmits[0]); + expect(lines.map((l) => l.kind)).toEqual(kinds); + expect(lines[0].actor).toEqual({ type: 'agent', id: AGENT, owner: OWNER }); + }); + + it('asks a parked call of the credential owner, on its card', async () => { + service.recordBrokerCall(row({ outcome: 'pending_approval', approvalId: 'approval_1', credentialOwnerId: OWNER })); + const [, requested] = await linesOf(mockEmits[0]); + expect(requested.data).toMatchObject({ askedOf: { id: OWNER, role: 'credential-owner' }, decisionCardId: 'approval_1' }); + }); + + it('leaves an approved call\'s own run row to the approval resolver, so nothing doubles', () => { + service.recordBrokerCall(row({ outcome: 'ok', approvalId: 'approval_1' })); + service.recordBrokerCall(row({ outcome: 'failed', approvalId: 'approval_1' })); + service.recordBrokerCall(row({ outcome: 'refused', approvalId: 'approval_1' })); + expect(mockEmits).toHaveLength(0); + }); + + it('records nothing for a seat grant, a missing grant, or a pod the record is off for', () => { + // The gate open everywhere: only the pod-target rule can refuse the seat grant. + process.env.RECORD_V1_PODS = '*'; + service.recordBrokerCall(row({ outcome: 'ok', grant: seatGrant })); + service.recordBrokerCall(row({ outcome: 'refused', grant: undefined })); + expect(mockEmits).toHaveLength(0); + process.env.RECORD_V1_PODS = 'another'; + service.recordBrokerCall(row({ outcome: 'ok' })); + expect(mockEmits).toHaveLength(0); + }); +}); + +describe('recordApprovalOutcome', () => { + const approval = (over) => ({ _id: 'approval_1', podId: POD, agentUserId: AGENT, toolCallEnding: 'ran', ...over }); + const parked = { grantId: 'grant_1', callId: 'tool_call_1', tool: 'sentry.resolve_issue' }; + + it('approved and run: decided by the owner, then completed', async () => { + service.recordApprovalOutcome(approval(), parked, { kind: 'approved', resolvedBy: OWNER }); + await flush(); + const lines = await linesOf(mockEmits[0]); + expect(mockEmits[0].key).toBe('approval/approval_1/approved'); + expect(lines.map((l) => [l.kind, l.data.approval || l.data.status])).toEqual([ + ['permission.decided', 'approved'], ['tool.update', 'completed'], + ]); + expect(lines[0].data).toMatchObject({ decidedBy: OWNER, decidedByRole: 'credential-owner', toolCallId: 'tool_call_1' }); + }); + + it('approved but the run failed: the update says failed', async () => { + service.recordApprovalOutcome(approval({ toolCallEnding: 'failed' }), parked, { kind: 'approved', resolvedBy: OWNER }); + await flush(); + expect((await linesOf(mockEmits[0]))[1].data.status).toBe('failed'); + }); + + it('declined, and expired, each end the call once', async () => { + service.recordApprovalOutcome(approval(), parked, { kind: 'declined', resolvedBy: OWNER }); + service.recordApprovalOutcome(approval(), parked, { kind: 'expired' }); + await flush(); + expect((await linesOf(mockEmits[0])).map((l) => l.data.approval || l.data.status)).toEqual(['rejected', 'failed']); + expect((await linesOf(mockEmits[1])).map((l) => l.data.approval || l.data.status)).toEqual(['timed_out', 'failed']); + }); + + it('records nothing when the card sits in a DM for a seat grant', async () => { + mockRoomGrant.findOne.mockReturnValue(lean(seatGrant)); + service.recordApprovalOutcome(approval(), { ...parked, grantId: 'grant_2' }, { kind: 'declined', resolvedBy: OWNER }); + // A pod grant whose card is somewhere else is not this pod's call either. + mockRoomGrant.findOne.mockReturnValue(lean({ grantId: 'grant_1', target: { kind: 'pod', id: '6a692a1be833c668acdb84ce' } })); + service.recordApprovalOutcome(approval(), parked, { kind: 'declined', resolvedBy: OWNER }); + await flush(); + expect(mockEmits).toHaveLength(0); + }); +}); + +describe('grant lines', () => { + it('a revoke with no descendants is manual; with them, cascade, ancestor first, per pod', async () => { + const OTHER = '6a692a1be833c668acdb84ce'; + service.recordGrantsRevoked('grant_root', [{ grantId: 'grant_root', target: { kind: 'pod', id: POD } }], OWNER); + service.recordGrantsRevoked('grant_root', [ + { grantId: 'grant_root', target: { kind: 'pod', id: POD } }, + { grantId: 'grant_child', target: { kind: 'pod', id: POD } }, + { grantId: 'grant_far', target: { kind: 'pod', id: OTHER } }, + { grantId: 'grant_seat', target: { kind: 'seat', id: AGENT } }, + ], OWNER); + const data = await Promise.all(mockEmits.map(async (e) => [e.ref.podId, (await linesOf(e))[0].data])); + expect(data).toEqual([ + [POD, { grantIds: ['grant_root'], reason: 'manual' }], + [POD, { grantIds: ['grant_root', 'grant_child'], reason: 'cascade' }], + [OTHER, { grantIds: ['grant_root', 'grant_far'], reason: 'cascade' }], + ]); + expect((await linesOf(mockEmits[0]))[0].actor).toEqual({ type: 'human', id: OWNER }); + }); + + it('an expired grant is revoked by the server, keyed so it is written once', async () => { + service.recordGrantExpired(podGrant); + expect(mockEmits[0].key).toBe('grant.expired/grant_1'); + expect((await linesOf(mockEmits[0]))[0]).toMatchObject({ + actor: { type: 'system', id: 'commonly' }, data: { grantIds: ['grant_1'], reason: 'expired' }, + }); + }); + + it('grant.issued only for a pod grant', () => { + process.env.RECORD_V1_PODS = '*'; + service.recordGrantIssued({ ...seatGrant, tools: [] }, { type: 'human', id: OWNER }); + service.recordGrantIssued({ ...podGrant, tools: ['sentry.resolve_issue'] }, { type: 'human', id: OWNER }); + expect(mockEmits.map((e) => e.key)).toEqual(['grant.issued/grant_1']); + }); +}); diff --git a/backend/__tests__/unit/services/toolBrokerService.test.js b/backend/__tests__/unit/services/toolBrokerService.test.js index c2b0bf9e43..b255de63f3 100644 --- a/backend/__tests__/unit/services/toolBrokerService.test.js +++ b/backend/__tests__/unit/services/toolBrokerService.test.js @@ -27,6 +27,8 @@ jest.mock('../../../services/approvalActionService', () => ({ proposeAction: jest.fn().mockResolvedValue({ ok: true, approvalId: 'approval-test' }), })); jest.mock('../../../services/dmService', () => mockDmService); +const mockRecordBrokerCall = jest.fn(); +jest.mock('../../../services/brokerRecordService', () => ({ recordBrokerCall: (...args) => mockRecordBrokerCall(...args) })); // The seat's own declaration is judged inside `callTool`/`listToolsForGrant` // (TASK-175). This suite has no Mongo, so the RESOLUTION is mocked here and the // resolution itself is witnessed on memory Mongo in @@ -509,3 +511,39 @@ describe('a suspended github-app row owner (§7)', () => { expect(mockUser.findById).not.toHaveBeenCalled(); }); }); + +// TASK-560: every committed trail row reaches the record once, as the same call. +describe('record wiring', () => { + const recorded = () => { + expect(mockRecordBrokerCall).toHaveBeenCalledTimes(mockToolCall.create.mock.calls.length); + const [row] = mockRecordBrokerCall.mock.calls[0]; + const [trail] = mockToolCall.create.mock.calls[0]; + expect(row).toMatchObject({ callId: trail.callId, outcome: trail.outcome, grantId: trail.grantId, tool: trail.tool, agentUserId: 'agent-a' }); + return row; + }; + + it('a refusal', async () => { + mockRoomGrant.findOne.mockResolvedValue(seatGrant({ tools: ['github.get_issue'] })); + await expect(callTool({ grantId: 'grant-1', agentUserId: 'agent-a', tool: 'github.list_issues', args: {} })) + .rejects.toMatchObject({ code: 'tool_not_allowed' }); + expect(recorded().outcome).toBe('refused'); + }); + + it('a park, with its card and the credential owner', async () => { + mockRoomGrant.findOne.mockResolvedValue(seatGrant({ tools: ['github.create_issue'], writeMode: 'write-with-confirm' })); + await expect(callTool({ grantId: 'grant-1', agentUserId: 'agent-a', tool: 'github.create_issue', args: { title: 'x' } })) + .rejects.toMatchObject({ code: 'approval_required' }); + expect(recorded()).toMatchObject({ outcome: 'pending_approval', approvalId: 'approval-test', credentialOwnerId: 'owner-1' }); + }); + + it('an unattended run, with the grant it ran under', async () => { + const grant = seatGrant({ tools: ['github.close_issue'], writeMode: 'write' }); + mockRoomGrant.findOne.mockResolvedValue(grant); + mockGithub.closeIssue.mockResolvedValue({ number: 7, title: 'closed', html_url: 'https://github.com/x/y/issues/7', state: 'closed' }); + await callTool({ grantId: 'grant-1', agentUserId: 'agent-a', tool: 'github.close_issue', args: { issueNumber: 7 } }); + const row = recorded(); + expect(row.outcome).toBe('ok'); + expect(row.approvalId).toBeUndefined(); + expect(row.grant).toBe(grant); + }); +}); diff --git a/backend/models/RoomGrant.ts b/backend/models/RoomGrant.ts index a27fe3caaa..f018694c48 100644 --- a/backend/models/RoomGrant.ts +++ b/backend/models/RoomGrant.ts @@ -47,6 +47,10 @@ export interface IRoomGrant extends Document { export interface RoomGrantModel extends Model { /** Revoke a grant and all descendants with one updateMany write. */ revokeCascade(grantId: string, revokedBy: string): Promise; + revokeCascadeMoved( + grantId: string, + revokedBy: string, + ): Promise>; } const GrantBudgetSchema = new Schema( @@ -107,6 +111,18 @@ RoomGrantSchema.statics.revokeCascade = async function revokeCascade( grantId: string, revokedBy: string, ): Promise { + return (await (this as RoomGrantModel).revokeCascadeMoved(grantId, revokedBy)).length; +}; + +/** + * revokeCascade, returning the grants this write moved (grantId and target), + * for the record's grant.revoked lines (TASK-560). A grant already revoked is + * not moved again and is not returned. + */ +RoomGrantSchema.statics.revokeCascadeMoved = async function revokeCascadeMoved( + grantId: string, + revokedBy: string, +): Promise> { const ids: string[] = [grantId]; const seen = new Set(ids); let frontier: string[] = [grantId]; @@ -132,7 +148,9 @@ RoomGrantSchema.statics.revokeCascade = async function revokeCascade( }, { $set: { revokedAt, revokedBy } }, ); - return result.modifiedCount || 0; + if (!result.modifiedCount) return []; + // The stamp this write set identifies the rows it moved. + return this.find({ grantId: { $in: ids }, revokedAt, revokedBy }).select('grantId target').lean(); }; const RoomGrant: RoomGrantModel = diff --git a/backend/routes/grants.ts b/backend/routes/grants.ts index 361ade62e2..cfbc3fd4dc 100644 --- a/backend/routes/grants.ts +++ b/backend/routes/grants.ts @@ -25,6 +25,13 @@ import { grantBrokerRefusal } from '../services/grantBrokerConfinement'; import type { GrantBrokerRefusal } from '../services/grantBrokerConfinement'; import { projectSeatEnvironments, seatEnvironmentKey } from '../services/seatEnvironmentProjection'; import { outsiderGrantPodRefusal, outsiderGrantRefusalMessage } from '../services/outsiderGrantPodEligibility'; +import { recordGrantIssued } from '../services/brokerRecordService'; +import { agentActor, humanActor } from '../services/recordService'; + +const plainGrant = (grant: unknown) => { + const doc = grant as { toObject?: () => Record }; + return (typeof doc?.toObject === 'function' ? doc.toObject() : grant) as Parameters[0]; +}; // eslint-disable-next-line @typescript-eslint/no-require-imports const auth = require('../middleware/auth'); @@ -428,6 +435,8 @@ router.post('/', grantRateLimit, auth, async (req: AuthenticatedRequest, res: ex brokerId: broker.brokerId, }; const grant = await createGrant(grantInput); + // The connection owner minted it (checked above); the record names them. + recordGrantIssued(plainGrant(grant), humanActor(userId)); return res.status(201).json(grant); } catch (error) { return handleError(res, error); @@ -470,6 +479,8 @@ router.post( audience: body.audience as string[] | undefined, expiresAt: body.expiresAt as string | undefined, }); + // An agent attenuated its own grant: the agent is the issuer. + recordGrantIssued(plainGrant(child), agentActor(req.agentUser as Parameters[0])); return res.status(201).json(child); } catch (error) { return handleError(res, error); diff --git a/backend/routes/podRecords.ts b/backend/routes/podRecords.ts index 579363e743..f7002e89e1 100644 --- a/backend/routes/podRecords.ts +++ b/backend/routes/podRecords.ts @@ -31,8 +31,9 @@ type RecordReq = express.Request & { const notFound = (res: express.Response) => res.status(404).json({ error: 'record_not_found' }); /** - * One task's record as record-v1 JSONL, or `?format=otlp` for the spec's - * OTLP/JSON export. + * A chain as record-v1 JSONL, or `?format=otlp` for the spec's OTLP/JSON + * export. `taskId` null is the pod's taskless chain (grants and the calls + * made under them). * * Access follows today's pod access (canViewPod), checked before any record * byte is loaded, and fails as not-found so the route never confirms a pod or @@ -41,10 +42,10 @@ const notFound = (res: express.Response) => res.status(404).json({ error: 'recor * without breaking verification. That is the same set runTraces shows those * ids to, so admins and agent-DM observers who pass canViewPod get a 403. */ -router.get('/:podId/record/tasks/:taskId', recordReadLimit, auth, async (req: RecordReq, res) => { +const serveRecord = async (req: RecordReq, res: express.Response, taskId: string | null) => { try { - const { podId, taskId } = req.params; - if (!mongoose.isObjectIdOrHexString(podId) || !TASK_ID.test(String(taskId))) return notFound(res); + const { podId } = req.params; + if (!mongoose.isObjectIdOrHexString(podId) || (taskId !== null && !TASK_ID.test(taskId))) return notFound(res); if (!isRecordEnabled(podId)) return notFound(res); const userId = req.userId || req.user?._id || req.agentUser?._id; const pod = await Pod.findById(podId).select('members type').lean(); @@ -52,12 +53,11 @@ router.get('/:podId/record/tasks/:taskId', recordReadLimit, auth, async (req: Re // Lean and unpopulated, so members are ObjectIds. const isMember = (pod.members || []).some((member: unknown) => String(member) === String(userId)); if (!isMember) return res.status(403).json({ error: 'record_requires_membership' }); - const task = await Task.findOne({ podId: pod._id, taskId }).select('_id').lean(); - if (!task) return notFound(res); + if (taskId !== null && !await Task.findOne({ podId: pod._id, taskId }).select('_id').lean()) return notFound(res); let record; try { - record = await readRecordExport({ podId: String(pod._id), taskId: String(taskId) }); + record = await readRecordExport({ podId: String(pod._id), ...(taskId !== null ? { taskId } : {}) }); } catch (error) { // A store outage is an error, never an apparently empty record. if (!(error instanceof RecordStoreUnavailableError)) { @@ -78,6 +78,13 @@ router.get('/:podId/record/tasks/:taskId', recordReadLimit, auth, async (req: Re console.error('GET /record error:', (error as Error).name, String((error as Error).message || '').slice(0, 160)); return res.status(500).json({ error: 'Failed to read record' }); } -}); +}; + +router.get('/:podId/record/tasks/:taskId', recordReadLimit, auth, (req: RecordReq, res: express.Response) => ( + serveRecord(req, res, String(req.params.taskId)) +)); +router.get('/:podId/record', recordReadLimit, auth, (req: RecordReq, res: express.Response) => ( + serveRecord(req, res, null) +)); module.exports = router; diff --git a/backend/services/approvalActionService.ts b/backend/services/approvalActionService.ts index 354132ee4e..de38dc52a0 100644 --- a/backend/services/approvalActionService.ts +++ b/backend/services/approvalActionService.ts @@ -18,6 +18,7 @@ import ApprovalAction, { import Pod from '../models/Pod'; import User from '../models/User'; import { assertAgentRuntimeMayJoinPod } from './outsiderGrantAgentJoinPolicy'; +import { recordApprovalOutcome } from './brokerRecordService'; // eslint-disable-next-line @typescript-eslint/no-require-imports const { AgentInstallation, AgentRegistry } = require('../models/AgentRegistry'); @@ -871,6 +872,7 @@ export const resolveApproval = async (options: ResolveOptions): Promise ({ id: `grant:${grantId}`, role: 'policy' }); + +const toolCall = (call: BrokerCall): RecordLineInput => ({ + kind: 'tool.call', + actor: call.agent, + data: { toolCallId: call.toolCallId, title: call.tool, name: call.tool, surface: 'connector' }, +}); + +const requested = (call: BrokerCall, askedOf: { id: string; role: string }, decisionCardId?: string): RecordLineInput => ({ + kind: 'permission.requested', + actor: call.agent, + data: { + toolCallId: call.toolCallId, + options: OPTS.map((option) => ({ ...option })), + question: call.tool, + askedOf, + grantId: call.grantId, + ...(decisionCardId ? { decisionCardId } : {}), + }, +}); + +type Decision = { + approve: boolean; + list: 'allow' | 'deny' | 'ask'; + approval?: 'approved' | 'rejected' | 'timed_out'; + decidedBy: string; + decidedByRole: 'policy' | 'credential-owner'; + decisionCardId?: string; +}; + +// permission.decided takes the spec's default actor: the policy +// (grant:) or the human decider, derived from decidedBy. +const decided = (call: BrokerCall, decision: Decision): RecordLineInput => ({ + kind: 'permission.decided', + data: { + outcome: 'selected', + optionId: decision.approve ? 'approve' : 'decline', + kind: decision.approve ? 'allow_once' : 'reject_once', + toolCallId: call.toolCallId, + rule: { list: decision.list, pattern: null }, + ...(decision.approval ? { approval: decision.approval } : {}), + decidedBy: decision.decidedBy, + decidedByRole: decision.decidedByRole, + grantId: call.grantId, + ...(decision.decisionCardId ? { decisionCardId: decision.decisionCardId } : {}), + }, +}); + +export const toolUpdate = (call: Pick, status: 'completed' | 'failed'): RecordLineInput => ({ + kind: 'tool.update', + actor: call.agent, + data: { toolCallId: call.toolCallId, status }, +}); + +/** B1: the broker refused before reaching the vendor. */ +export const refusedCallLines = (call: BrokerCall): RecordLineInput[] => [ + toolCall(call), + requested(call, policyAsk(call.grantId)), + decided(call, { approve: false, list: 'deny', decidedBy: 'policy', decidedByRole: 'policy' }), + toolUpdate(call, 'failed'), +]; + +/** B2: a write parked for the credential owner's approval. No decision yet. */ +export const parkedCallLines = (call: BrokerCall, ownerUserId: string, decisionCardId?: string): RecordLineInput[] => [ + toolCall(call), + requested(call, { id: ownerUserId, role: 'credential-owner' }, decisionCardId), +]; + +/** B3: the credential owner approved or declined the parked call. */ +export const resolvedCallLines = ( + call: BrokerCall, + approved: boolean, + resolvedBy: string, + decisionCardId?: string, +): RecordLineInput[] => [ + decided(call, { + approve: approved, + list: 'ask', + approval: approved ? 'approved' : 'rejected', + decidedBy: resolvedBy, + decidedByRole: 'credential-owner', + decisionCardId, + }), + // An approved call's outcome arrives with its execution (toolUpdate). + ...(approved ? [] : [toolUpdate(call, 'failed')]), +]; + +/** B4: nobody answered in time; the policy declines. */ +export const expiredCallLines = (call: BrokerCall, decisionCardId?: string): RecordLineInput[] => [ + decided(call, { + approve: false, + list: 'ask', + approval: 'timed_out', + decidedBy: 'policy', + decidedByRole: 'policy', + decisionCardId, + }), + toolUpdate(call, 'failed'), +]; + +/** B5: executed under the grant with no ask. */ +export const executedCallLines = (call: BrokerCall, ok: boolean): RecordLineInput[] => [ + toolCall(call), + requested(call, policyAsk(call.grantId)), + decided(call, { approve: true, list: 'allow', decidedBy: 'policy', decidedByRole: 'policy' }), + toolUpdate(call, ok ? 'completed' : 'failed'), +]; + +export type GrantShape = { + grantId: string; + parentGrantId?: string | null; + connectionId?: string | null; + tools?: string[]; + writeMode?: string; + budget?: { calls?: number; windowMs?: number } | null; + audience?: unknown[]; + target?: { kind?: string; id?: unknown } | null; + expiresAt?: Date | null; +}; + +/** + * grant.issued. `issuedBy` is the user who minted it: the connection owner + * for a grant, the attenuating agent for a child. An agent issuer is named as + * an agent actor, since the format's default would call it a human. + */ +export const grantIssuedLine = (grant: GrantShape, issuer: RecordActor | null): RecordLineInput => { + const budget = grant.budget && (grant.budget.calls !== undefined || grant.budget.windowMs !== undefined) + ? { + ...(grant.budget.calls !== undefined ? { calls: grant.budget.calls } : {}), + ...(grant.budget.windowMs !== undefined ? { windowMs: grant.budget.windowMs } : {}), + } + : undefined; + return { + kind: 'grant.issued', + actor: issuer, + data: { + grantId: grant.grantId, + ...(grant.parentGrantId ? { parentGrantId: grant.parentGrantId } : {}), + scope: { + ...(grant.connectionId ? { connectionId: String(grant.connectionId) } : {}), + tools: [...(grant.tools || [])], + ...(grant.writeMode ? { writeMode: grant.writeMode } : {}), + ...(budget ? { budget } : {}), + audience: (grant.audience || []).map(String), + }, + ...(grant.target?.kind && grant.target.id ? { target: { kind: grant.target.kind, id: String(grant.target.id) } } : {}), + ...(issuer ? { issuedBy: issuer.id } : {}), + ...(grant.expiresAt instanceof Date ? { expiresAt: grant.expiresAt.toISOString() } : {}), + }, + }; +}; + +/** + * grant.revoked. `cascade` puts the revoked ancestor first and its + * descendants after it; `manual` names exactly the grants a person or policy + * ended; `expired` names one grant whose expiresAt passed; `task.done` names + * the grants bound to a task the board just closed (written after task.done). + */ +export const grantRevokedLine = ( + grantIds: string[], + reason: 'cascade' | 'manual' | 'expired' | 'task.done', + actor: RecordActor | null, +): RecordLineInput => ({ + kind: 'grant.revoked', + actor, + data: { grantIds: [...grantIds], reason }, +}); diff --git a/backend/services/brokerRecordService.ts b/backend/services/brokerRecordService.ts new file mode 100644 index 0000000000..cfd6ca9eeb --- /dev/null +++ b/backend/services/brokerRecordService.ts @@ -0,0 +1,153 @@ +/** + * Where grant and broker writes meet the record (TASK-560, ADR-033 §4). + * Each function runs after its source write commits, picks the lines from + * brokerRecordLines, and emits them to the grant's pod chain. Never throws. + * + * Pod-target grants only. A seat grant's target is a seat and its approval + * card lives in a DM, so neither names a pod the call belongs to; those calls + * stay unrecorded until a pod is ruled for them, rather than being given one. + * No grant carries a task binding yet (TASK-288), so every line here is + * taskless and lands on the pod chain. + */ +import { + agentActor, emitRecord, humanActor, isRecordEnabled, SERVER_ACTOR, type RecordActor, +} from './recordService'; +import { + executedCallLines, expiredCallLines, grantIssuedLine, grantRevokedLine, parkedCallLines, + refusedCallLines, resolvedCallLines, toolUpdate, type GrantShape, +} from './brokerRecordLines'; + +/* eslint-disable @typescript-eslint/no-require-imports, global-require */ +const lazyUser = () => require('../models/User'); +const lazyRoomGrant = () => require('../models/RoomGrant').default; +/* eslint-enable @typescript-eslint/no-require-imports, global-require */ + +type GrantLike = { grantId?: unknown; target?: { kind?: unknown; id?: unknown } | null } | Record | null | undefined; + +/** The pod a grant belongs to, or null for a seat grant (or no grant). */ +export const podOfGrant = (grant: GrantLike): string | null => { + const target = (grant as { target?: { kind?: unknown; id?: unknown } } | null)?.target; + return target?.kind === 'pod' && target.id ? String(target.id) : null; +}; + +/** The calling agent, with its owner, from its user id. Unknown stays null. */ +const agentById = async (agentUserId: unknown): Promise => { + if (!agentUserId) return null; + const user = await lazyUser().findById(String(agentUserId)).select('_id isBot botMetadata.ownerUserId').lean(); + return agentActor(user || { _id: agentUserId }); +}; + +export type BrokerTrailRow = { + grant: GrantLike; + callId: string; + grantId: string; + agentUserId: string; + tool: string; + outcome: 'ok' | 'refused' | 'failed' | 'pending_approval'; + approvalId?: string; + credentialOwnerId?: string; +}; + +/** + * B1, B2 and B5, from the broker's own trail row. A row that carries an + * approval id and is not the park itself is the outcome of an approved call; + * the approval resolver records that, so it is skipped here and never doubles. + */ +export const recordBrokerCall = (row: BrokerTrailRow): void => { + const podId = podOfGrant(row.grant); + if (!podId || !isRecordEnabled(podId)) return; + if (row.approvalId && row.outcome !== 'pending_approval') return; + emitRecord({ podId }, `tool_call/${row.callId}`, async () => { + const call = { toolCallId: row.callId, tool: row.tool, grantId: row.grantId, agent: await agentById(row.agentUserId) }; + if (row.outcome === 'refused') return refusedCallLines(call); + if (row.outcome === 'pending_approval') { + // Asked of the credential owner, who is the only one who may decide (D5). + return row.credentialOwnerId ? parkedCallLines(call, String(row.credentialOwnerId), row.approvalId) : []; + } + return executedCallLines(call, row.outcome === 'ok'); + }); +}; + +type ParkedCall = { grantId?: unknown; callId?: unknown; tool?: unknown } | null | undefined; +type ApprovalRow = { + _id?: unknown; + podId?: unknown; + agentUserId?: unknown; + toolCallEnding?: string | null; +}; + +/** + * B3 and B4, after the approval row's terminal write. The card's pod is the + * grant's own pod only for a pod grant; a seat grant's card sits in a DM, so + * the grant is read to tell them apart. + */ +export const recordApprovalOutcome = ( + row: ApprovalRow, + parkedCall: ParkedCall, + outcome: { kind: 'approved' | 'declined'; resolvedBy: string } | { kind: 'expired' }, +): void => { + const cardPodId = row?.podId ? String(row.podId) : ''; + if (!parkedCall?.callId || !parkedCall.grantId || !cardPodId || !isRecordEnabled(cardPodId)) return; + const approvalId = row._id ? String(row._id) : undefined; + void (async () => { + const grant = await lazyRoomGrant().findOne({ grantId: String(parkedCall.grantId) }).select('target').lean(); + if (podOfGrant(grant) !== cardPodId) return; + emitRecord({ podId: cardPodId }, `approval/${approvalId || parkedCall.callId}/${outcome.kind}`, async () => { + const call = { + toolCallId: String(parkedCall.callId), + tool: String(parkedCall.tool || ''), + grantId: String(parkedCall.grantId), + agent: await agentById(row.agentUserId), + }; + if (outcome.kind === 'expired') return expiredCallLines(call, approvalId); + const lines = resolvedCallLines(call, outcome.kind === 'approved', outcome.resolvedBy, approvalId); + if (outcome.kind !== 'approved') return lines; + return [...lines, toolUpdate(call, row.toolCallEnding === 'ran' ? 'completed' : 'failed')]; + }); + })().catch((error: Error) => console.warn('[record] approval outcome skipped:', error.name, String(error.message || '').slice(0, 160))); +}; + +/** grant.issued, once the grant row exists and survived the mint race. */ +export const recordGrantIssued = (grant: GrantShape & GrantLike, issuer: RecordActor | null): void => { + const podId = podOfGrant(grant); + if (!podId || !grant?.grantId) return; + emitRecord({ podId }, `grant.issued/${grant.grantId}`, [grantIssuedLine(grant, issuer)]); +}; + +/** + * grant.revoked for the grants one revoke moved, per pod. The revoked grant + * comes first; `manual` when it was the only one, `cascade` when descendants + * went with it (record-v1 §2). + */ +export const recordGrantsRevoked = ( + rootGrantId: string, + moved: Array, + revokedBy: unknown, +): void => { + const byPod = new Map(); + for (const grant of moved) { + const podId = podOfGrant(grant); + const grantId = (grant as { grantId?: unknown })?.grantId; + if (!podId || !grantId) continue; + byPod.set(podId, [...(byPod.get(podId) || []), String(grantId)]); + } + const actor = humanActor(revokedBy); + for (const [podId, ids] of byPod) { + const grantIds = [rootGrantId, ...ids.filter((id) => id !== rootGrantId)]; + const reason = grantIds.length === 1 ? 'manual' : 'cascade'; + emitRecord({ podId }, `grant.revoked/${rootGrantId}/${[...ids].sort().join(',')}`, [grantRevokedLine(grantIds, reason, actor)]); + } +}; + +/** + * grant.revoked {reason: expired}, written the first time the server sees an + * expired grant: the broker checks expiry at use, and nothing sweeps it. A + * grant nobody tries to use again gets no line; R2 says an absent line is an + * audit gap, never proof the grant is usable. + */ +export const recordGrantExpired = (grant: GrantLike): void => { + const podId = podOfGrant(grant); + const grantId = (grant as { grantId?: unknown })?.grantId; + if (!podId || !grantId) return; + emitRecord({ podId }, `grant.expired/${grantId}`, [grantRevokedLine([String(grantId)], 'expired', SERVER_ACTOR)]); +}; diff --git a/backend/services/recordService.ts b/backend/services/recordService.ts index 999f067136..fedfad8a82 100644 --- a/backend/services/recordService.ts +++ b/backend/services/recordService.ts @@ -45,6 +45,12 @@ export type RecordLineInput = { export { RecordStoreUnavailableError, type ChainRef }; export const BOARD_ACTOR: RecordActor = format.BOARD_ACTOR; +/** + * The server itself, for an entry no person or board write caused (a grant + * found expired at use). The format's own default system actor is `seats`, + * the local run writer, which would misname the server. + */ +export const SERVER_ACTOR: RecordActor = Object.freeze({ type: 'system', id: 'commonly' }) as RecordActor; /** * Pods the record is written and served for. Nothing is public until a @@ -140,19 +146,28 @@ const chainKey = (ref: ChainRef): string => `${ref.podId}\u0000${ref.taskId || ' /** * Record a committed source event, best-effort. Returns at once; never throws. + * `inputs` may be a function, called in the chain's queue, when building the + * lines needs a lookup: the event keeps the queue position of the call. * A failure logs one bounded line naming the chain and kinds, never data. */ -export const emitRecord = (ref: ChainRef, eventKey: string, inputs: RecordLineInput[]): void => { - if (!ref?.podId || inputs.length === 0 || !isRecordEnabled(ref.podId)) return; +export const emitRecord = ( + ref: ChainRef, + eventKey: string, + inputs: RecordLineInput[] | (() => Promise), +): void => { + if (!ref?.podId || !isRecordEnabled(ref.podId)) return; + if (Array.isArray(inputs) && inputs.length === 0) return; const key = chainKey(ref); const run = async (): Promise => { + let lines: RecordLineInput[] = []; try { - await appendRecord(ref, eventKey, inputs); + lines = Array.isArray(inputs) ? inputs : await inputs(); + if (lines.length > 0) await appendRecord(ref, eventKey, lines); } catch (error) { const err = error as { name?: string; code?: string; message?: string }; console.warn( `[record] append failed pod=${ref.podId} task=${ref.taskId || '-'} ` - + `kinds=${inputs.map((input) => input.kind).join(',')}: ` + + `kinds=${lines.map((input) => input.kind).join(',') || '?'}: ` + `${err.code || err.name || 'Error'} ${String(err.message || '').slice(0, 160)}`, ); } diff --git a/backend/services/roomGrantService.ts b/backend/services/roomGrantService.ts index 662b611ef7..a84438e2e5 100644 --- a/backend/services/roomGrantService.ts +++ b/backend/services/roomGrantService.ts @@ -1,4 +1,5 @@ import { randomUUID } from 'crypto'; +import { recordGrantExpired, recordGrantsRevoked } from './brokerRecordService'; import RoomGrant, { IRoomGrant, IRoomGrantBudget, @@ -220,7 +221,10 @@ const assertGrantLineageActive = async ( const lineage = await loadGrantLineage(grant); for (const item of lineage) { if (grantIsRevoked(item)) throw new RoomGrantError('grant_revoked', 'grant lineage is revoked', 403); - if (grantIsExpired(item, now)) throw new RoomGrantError('grant_expired', 'grant lineage is expired', 403); + if (grantIsExpired(item, now)) { + recordGrantExpired(item); + throw new RoomGrantError('grant_expired', 'grant lineage is expired', 403); + } } return lineage; }; @@ -392,7 +396,9 @@ export const revokeGrant = async (grantId: string, revokedBy: string): Promise | undefined, + callId, + grantId: input.grantId, + agentUserId: input.agentUserId, + tool: input.tool, + outcome, + approvalId: overrides?.approvalId, + credentialOwnerId: overrides?.credentialOwnerId, + }); return callId; }; diff --git a/docs/plans/task-513-server-record.md b/docs/plans/task-513-server-record.md index b3f1fdb249..fdbe33f7d3 100644 --- a/docs/plans/task-513-server-record.md +++ b/docs/plans/task-513-server-record.md @@ -51,6 +51,33 @@ endpoint. Every writer and the endpoint are off unless the pod is listed in process so one process appends in commit order, and never changes the source write's result. +### Decisions taken in slice 2 (TASK-560) + +- **Where each line comes from.** The broker's trail write (`recordCall`) + emits B1 (refused), B2 (parked, asked of the credential owner, on its card) + and B5 (run under the grant with no ask). A trail row that carries an + approval id and is not the park is the run of an approved call. The approval + resolver records that, so it is skipped here, and no call gets two terminal + decisions. The resolver writes B3 once the ending is saved: the decision by + the credential owner, then `tool.update` from the saved ending. Expiry at + decide time and in the sweep writes B4. +- **Pod-target grants only.** A seat grant's target is a seat and its card + sits in the granter–seat DM, so neither names the pod a call belongs to. + Those calls write nothing until a pod is ruled for them. The seat tests run + with the pilot gate open everywhere, so only this rule can refuse them. +- **Grants.** `grant.issued` comes from the two routes that mint, because the + mint itself has no issuer: the connection owner as a person, the attenuating + agent as an agent. `revokeGrant` returns what it moved, through + `RoomGrant.revokeCascadeMoved`. One line per pod gives `manual` when the + revoked grant went alone and `cascade` (ancestor first) when descendants went + with it. Expiry is written the first time the server sees an expired grant + (at use, or as a lineage parent), keyed so it is written once, with the + server as actor (`system`/`commonly`). Nothing sweeps expiry, so a grant + nobody uses again gets no line; R2 makes that an audit gap, not access. +- **No `argsDigest`** on any line, JSONL or OTLP (CTO, ADR-033 §8 q9). +- **Reads.** `GET /api/pods/:podId/record` serves the pod's taskless chain + under the same gate as the task route. + ## Measured inputs (2026-10-25) - TASK-504 landed in #2684, merge `ab5132313ad399d8fb990d69bc39db0ccf99e055`. From 93a17affde9dd4b3300896cc7794b6a9dbe8c168 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:18:27 -0700 Subject: [PATCH 2/5] fix(record): B3 and B4 read the decider the approval row stored (TASK-560) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The approval row now stores decidedBy and decidedByRole with its terminal write (#2785, TASK-512), so the record reads the decider from the row instead of taking the caller from the resolver. A row whose stored decider is missing, or does not fit its outcome (B3 needs credential-owner, B4 needs policy, and "policy" pairs only with the policy role), writes no lines and logs a warning: an audit gap, not a guessed decider. Also adds the read-route case from Lumen's #2772 security gate: an agent-DM observer, who may view the DM under the §3.7 fan-out but is not a member, gets 403 on both the task and pod record reads, and the chain is never read. Written by Otto, a Commonly agent. Co-Authored-By: Claude Opus 5.5 Co-authored-by: otto Agent-Model: claude/claude-opus-5-5/default --- .../service/podRecords.realPg.test.js | 23 ++++++++++ .../services/approvalToolCallService.test.js | 4 +- .../unit/services/brokerRecordService.test.js | 45 ++++++++++++++++--- backend/services/approvalActionService.ts | 2 +- backend/services/brokerRecordService.ts | 25 ++++++++++- docs/plans/task-513-server-record.md | 6 ++- 6 files changed, 92 insertions(+), 13 deletions(-) diff --git a/backend/__tests__/service/podRecords.realPg.test.js b/backend/__tests__/service/podRecords.realPg.test.js index d1fdc5d3df..10520c8f24 100644 --- a/backend/__tests__/service/podRecords.realPg.test.js +++ b/backend/__tests__/service/podRecords.realPg.test.js @@ -204,6 +204,29 @@ real('task record route on real PostgreSQL and MongoDB', () => { expect(readChain).not.toHaveBeenCalled(); }); + it('an agent-DM observer can view the DM but reads neither of its records', async () => { + // §3.7: a person who shares a pod with either DM member may view the DM. + const peer = await User.create({ + username: `record-peer-${Date.now()}`, + email: `record-peer-${Date.now()}@agents.commonly.local`, + password: 'Password123!', + isBot: true, + botType: 'agent', + botMetadata: { agentName: 'record-peer', instanceId: 'default', ownerUserId: String(owner._id) }, + }); + const dm = await Pod.create({ name: 'agent DM', type: 'agent-dm', createdBy: agent._id, members: [agent._id, peer._id] }); + await Pod.create({ name: 'Shared pod', type: 'team', createdBy: owner._id, members: [outsider._id, agent._id] }); + await Task.create({ podId: dm._id, taskNum: 1, taskId: 'TASK-1', title: 'dm task', status: 'pending' }); + process.env.RECORD_V1_PODS = [...podIds(), String(dm._id)].join(','); + const readChain = jest.spyOn(repository, 'readChain'); + const observer = generateTestToken(outsider._id); + + expect(await read(observer, dm._id)).toMatchObject({ status: 403, body: { error: 'record_requires_membership' } }); + expect(await request(app).get(`/api/pods/${dm._id}/record`).set(as(observer))) + .toMatchObject({ status: 403, body: { error: 'record_requires_membership' } }); + expect(readChain).not.toHaveBeenCalled(); + }); + it('answers a store outage with 503, never an empty record', async () => { jest.spyOn(repository, 'readChain').mockRejectedValueOnce(new repository.RecordStoreUnavailableError()); expect(await read(generateTestToken(owner._id))).toMatchObject({ status: 503, body: { error: 'record_store_unavailable' } }); diff --git a/backend/__tests__/unit/services/approvalToolCallService.test.js b/backend/__tests__/unit/services/approvalToolCallService.test.js index fb34dbca29..751eff35f4 100644 --- a/backend/__tests__/unit/services/approvalToolCallService.test.js +++ b/backend/__tests__/unit/services/approvalToolCallService.test.js @@ -403,7 +403,7 @@ describe('record wiring', () => { expect(approval).toBe(resolved); expect(approval.toolCallEnding).toBe('ran'); expect(parked).toMatchObject({ grantId: 'grant-1', callId: 'tool_call_original', tool: toolCall.tool }); - expect(outcome).toEqual({ kind: 'approved', resolvedBy: OWNER }); + expect(outcome).toEqual({ kind: 'approved' }); }); test('declined', async () => { @@ -411,7 +411,7 @@ describe('record wiring', () => { mockApproval.findById.mockResolvedValue(pending); mockApproval.findOneAndUpdate.mockResolvedValue(row({ status: 'resolved', decision: 'declined' })); await service.resolveApproval({ approvalId: pending._id, callerUserId: OWNER, decision: 'declined' }); - expect(outcomeCall()[2]).toEqual({ kind: 'declined', resolvedBy: OWNER }); + expect(outcomeCall()[2]).toEqual({ kind: 'declined' }); }); test('expired at decide time, and in the sweep', async () => { diff --git a/backend/__tests__/unit/services/brokerRecordService.test.js b/backend/__tests__/unit/services/brokerRecordService.test.js index c44e157edb..4f869db761 100644 --- a/backend/__tests__/unit/services/brokerRecordService.test.js +++ b/backend/__tests__/unit/services/brokerRecordService.test.js @@ -77,11 +77,16 @@ describe('recordBrokerCall', () => { }); describe('recordApprovalOutcome', () => { - const approval = (over) => ({ _id: 'approval_1', podId: POD, agentUserId: AGENT, toolCallEnding: 'ran', ...over }); + // The decider is the one the row stored with its terminal write (TASK-512). + const approval = (over) => ({ + _id: 'approval_1', podId: POD, agentUserId: AGENT, toolCallEnding: 'ran', + decidedBy: OWNER, decidedByRole: 'credential-owner', ...over, + }); + const expiredApproval = (over) => approval({ decidedBy: 'policy', decidedByRole: 'policy', toolCallEnding: 'expired', ...over }); const parked = { grantId: 'grant_1', callId: 'tool_call_1', tool: 'sentry.resolve_issue' }; it('approved and run: decided by the owner, then completed', async () => { - service.recordApprovalOutcome(approval(), parked, { kind: 'approved', resolvedBy: OWNER }); + service.recordApprovalOutcome(approval(), parked, { kind: 'approved' }); await flush(); const lines = await linesOf(mockEmits[0]); expect(mockEmits[0].key).toBe('approval/approval_1/approved'); @@ -92,25 +97,51 @@ describe('recordApprovalOutcome', () => { }); it('approved but the run failed: the update says failed', async () => { - service.recordApprovalOutcome(approval({ toolCallEnding: 'failed' }), parked, { kind: 'approved', resolvedBy: OWNER }); + service.recordApprovalOutcome(approval({ toolCallEnding: 'failed' }), parked, { kind: 'approved' }); await flush(); expect((await linesOf(mockEmits[0]))[1].data.status).toBe('failed'); }); it('declined, and expired, each end the call once', async () => { - service.recordApprovalOutcome(approval(), parked, { kind: 'declined', resolvedBy: OWNER }); - service.recordApprovalOutcome(approval(), parked, { kind: 'expired' }); + service.recordApprovalOutcome(approval(), parked, { kind: 'declined' }); + service.recordApprovalOutcome(expiredApproval(), parked, { kind: 'expired' }); await flush(); expect((await linesOf(mockEmits[0])).map((l) => l.data.approval || l.data.status)).toEqual(['rejected', 'failed']); expect((await linesOf(mockEmits[1])).map((l) => l.data.approval || l.data.status)).toEqual(['timed_out', 'failed']); }); + it('writes the decider the row stored, not the caller', async () => { + const other = '6a693bfee833c668acdce5a5'; + service.recordApprovalOutcome(approval({ decidedBy: other }), parked, { kind: 'declined' }); + await flush(); + expect((await linesOf(mockEmits[0]))[0].data).toMatchObject({ decidedBy: other, decidedByRole: 'credential-owner' }); + }); + + it.each([ + ['approved', { decidedBy: undefined, decidedByRole: undefined }], + ['approved', { decidedBy: null }], + ['declined', { decidedByRole: undefined }], + ['approved', { decidedBy: 'policy', decidedByRole: 'policy' }], + ['declined', { decidedBy: 'policy' }], + ['expired', { decidedBy: OWNER, decidedByRole: 'credential-owner' }], + ['expired', { decidedBy: OWNER, decidedByRole: 'policy' }], + ['expired', { decidedBy: undefined, decidedByRole: undefined }], + ])('writes no %s lines when the stored decider is %o', async (kind, over) => { + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + const base = kind === 'expired' ? expiredApproval : approval; + service.recordApprovalOutcome(base(over), parked, { kind }); + await flush(); + expect(mockEmits).toHaveLength(0); + expect(warn).toHaveBeenCalledWith('[record] approval outcome skipped: the row stores no', kind, 'decider'); + warn.mockRestore(); + }); + it('records nothing when the card sits in a DM for a seat grant', async () => { mockRoomGrant.findOne.mockReturnValue(lean(seatGrant)); - service.recordApprovalOutcome(approval(), { ...parked, grantId: 'grant_2' }, { kind: 'declined', resolvedBy: OWNER }); + service.recordApprovalOutcome(approval(), { ...parked, grantId: 'grant_2' }, { kind: 'declined' }); // A pod grant whose card is somewhere else is not this pod's call either. mockRoomGrant.findOne.mockReturnValue(lean({ grantId: 'grant_1', target: { kind: 'pod', id: '6a692a1be833c668acdb84ce' } })); - service.recordApprovalOutcome(approval(), parked, { kind: 'declined', resolvedBy: OWNER }); + service.recordApprovalOutcome(approval(), parked, { kind: 'declined' }); await flush(); expect(mockEmits).toHaveLength(0); }); diff --git a/backend/services/approvalActionService.ts b/backend/services/approvalActionService.ts index de38dc52a0..dc45d3cef0 100644 --- a/backend/services/approvalActionService.ts +++ b/backend/services/approvalActionService.ts @@ -971,7 +971,7 @@ export const resolveApproval = async (options: ResolveOptions): Promise { + const role = kind === 'expired' ? 'policy' : 'credential-owner'; + const decidedBy = row?.decidedBy ? String(row.decidedBy) : ''; + if (row?.decidedByRole !== role || !decidedBy) return null; + return (decidedBy === 'policy') === (role === 'policy') ? decidedBy : null; }; /** @@ -84,10 +100,15 @@ type ApprovalRow = { export const recordApprovalOutcome = ( row: ApprovalRow, parkedCall: ParkedCall, - outcome: { kind: 'approved' | 'declined'; resolvedBy: string } | { kind: 'expired' }, + outcome: { kind: OutcomeKind }, ): void => { const cardPodId = row?.podId ? String(row.podId) : ''; if (!parkedCall?.callId || !parkedCall.grantId || !cardPodId || !isRecordEnabled(cardPodId)) return; + const decidedBy = storedDecider(row, outcome.kind); + if (!decidedBy) { + console.warn('[record] approval outcome skipped: the row stores no', outcome.kind, 'decider'); + return; + } const approvalId = row._id ? String(row._id) : undefined; void (async () => { const grant = await lazyRoomGrant().findOne({ grantId: String(parkedCall.grantId) }).select('target').lean(); @@ -100,7 +121,7 @@ export const recordApprovalOutcome = ( agent: await agentById(row.agentUserId), }; if (outcome.kind === 'expired') return expiredCallLines(call, approvalId); - const lines = resolvedCallLines(call, outcome.kind === 'approved', outcome.resolvedBy, approvalId); + const lines = resolvedCallLines(call, outcome.kind === 'approved', decidedBy, approvalId); if (outcome.kind !== 'approved') return lines; return [...lines, toolUpdate(call, row.toolCallEnding === 'ran' ? 'completed' : 'failed')]; }); diff --git a/docs/plans/task-513-server-record.md b/docs/plans/task-513-server-record.md index fdbe33f7d3..f467479f5e 100644 --- a/docs/plans/task-513-server-record.md +++ b/docs/plans/task-513-server-record.md @@ -60,7 +60,11 @@ endpoint. Every writer and the endpoint are off unless the pod is listed in resolver records that, so it is skipped here, and no call gets two terminal decisions. The resolver writes B3 once the ending is saved: the decision by the credential owner, then `tool.update` from the saved ending. Expiry at - decide time and in the sweep writes B4. + decide time and in the sweep writes B4. Both read the decider the approval + row stored with its terminal write (`decidedBy`, `decidedByRole`, TASK-512), + never the caller. A row whose stored decider is missing, or does not fit its + outcome (B3 needs `credential-owner`, B4 needs `policy`), writes no lines and + logs a warning: an audit gap, not a guessed decider. - **Pod-target grants only.** A seat grant's target is a seat and its card sits in the granter–seat DM, so neither names the pod a call belongs to. Those calls write nothing until a pod is ruled for them. The seat tests run From 56eb2691ab5986f5f83714723ff223331fda53d8 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:29:05 -0700 Subject: [PATCH 3/5] fix(record): a failed agent lookup keeps the call's lines (TASK-560) agentById awaited the owner lookup unguarded inside the emitRecord factory, so a rejected Mongo read dropped the whole B1-B5 line set even though the trail row already names the agent. The lookup now only adds the owner: on failure the lines go out with the agent's id and a warning. The plan doc also said revokeGrant returns what it moved; it returns the count. RoomGrant.revokeCascadeMoved returns the moved grants, and revokeGrant passes them to the recorder. Both from CodeRabbit's review of 93a17aff. Written by Otto, a Commonly agent. Co-Authored-By: Claude Opus 5.5 Co-authored-by: otto Agent-Model: claude/claude-opus-5-5/default --- .../unit/services/brokerRecordService.test.js | 11 +++++++++++ backend/services/brokerRecordService.ts | 14 +++++++++++--- docs/plans/task-513-server-record.md | 5 +++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/backend/__tests__/unit/services/brokerRecordService.test.js b/backend/__tests__/unit/services/brokerRecordService.test.js index 4f869db761..15228c4995 100644 --- a/backend/__tests__/unit/services/brokerRecordService.test.js +++ b/backend/__tests__/unit/services/brokerRecordService.test.js @@ -51,6 +51,17 @@ describe('recordBrokerCall', () => { expect(lines[0].actor).toEqual({ type: 'agent', id: AGENT, owner: OWNER }); }); + it('a failed agent lookup loses only the owner, never the lines', async () => { + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); + mockUser.findById.mockReturnValue({ select: () => ({ lean: async () => { throw new Error('mongo down'); } }) }); + service.recordBrokerCall(row({ outcome: 'ok' })); + const lines = await linesOf(mockEmits[0]); + expect(lines.map((l) => l.kind)).toEqual(['tool.call', 'permission.requested', 'permission.decided', 'tool.update']); + expect(lines[0].actor).toEqual({ type: 'agent', id: AGENT }); + expect(warn).toHaveBeenCalledWith('[record] agent owner lookup failed:', 'Error'); + warn.mockRestore(); + }); + it('asks a parked call of the credential owner, on its card', async () => { service.recordBrokerCall(row({ outcome: 'pending_approval', approvalId: 'approval_1', credentialOwnerId: OWNER })); const [, requested] = await linesOf(mockEmits[0]); diff --git a/backend/services/brokerRecordService.ts b/backend/services/brokerRecordService.ts index 705aed0ce2..fa7f8baa67 100644 --- a/backend/services/brokerRecordService.ts +++ b/backend/services/brokerRecordService.ts @@ -30,11 +30,19 @@ export const podOfGrant = (grant: GrantLike): string | null => { return target?.kind === 'pod' && target.id ? String(target.id) : null; }; -/** The calling agent, with its owner, from its user id. Unknown stays null. */ +/** + * The calling agent, with its owner, from its user id. Unknown stays null. A + * failed lookup only loses the owner: the lines still go out with the id. + */ const agentById = async (agentUserId: unknown): Promise => { if (!agentUserId) return null; - const user = await lazyUser().findById(String(agentUserId)).select('_id isBot botMetadata.ownerUserId').lean(); - return agentActor(user || { _id: agentUserId }); + let user: unknown = null; + try { + user = await lazyUser().findById(String(agentUserId)).select('_id isBot botMetadata.ownerUserId').lean(); + } catch (error) { + console.warn('[record] agent owner lookup failed:', (error as Error).name); + } + return agentActor((user as Parameters[0]) || { _id: agentUserId }); }; export type BrokerTrailRow = { diff --git a/docs/plans/task-513-server-record.md b/docs/plans/task-513-server-record.md index f467479f5e..73e1c69a60 100644 --- a/docs/plans/task-513-server-record.md +++ b/docs/plans/task-513-server-record.md @@ -71,8 +71,9 @@ endpoint. Every writer and the endpoint are off unless the pod is listed in with the pilot gate open everywhere, so only this rule can refuse them. - **Grants.** `grant.issued` comes from the two routes that mint, because the mint itself has no issuer: the connection owner as a person, the attenuating - agent as an agent. `revokeGrant` returns what it moved, through - `RoomGrant.revokeCascadeMoved`. One line per pod gives `manual` when the + agent as an agent. `RoomGrant.revokeCascadeMoved` returns the grants one + revoke moved, and `revokeGrant` passes them to the recorder (it returns + only their count). One line per pod gives `manual` when the revoked grant went alone and `cascade` (ancestor first) when descendants went with it. Expiry is written the first time the server sees an expired grant (at use, or as a lineage parent), keyed so it is written once, with the From d0b40143a95bba503d7d056bf4562b40b6af4a40 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:37:10 -0700 Subject: [PATCH 4/5] test(record): an owner lookup that finds no user keeps the lines too (TASK-560) The fallback test now covers both paths to the id-only agent actor, a rejected lookup (which warns) and a lookup that finds no user (which does not), and runs the emitted lines through the shared record writer, so an agent actor with no owner is shown to persist. From CodeRabbit's review of 56eb2691. Written by Otto, a Commonly agent. Co-Authored-By: Claude Opus 5.5 Co-authored-by: otto Agent-Model: claude/claude-opus-5-5/default --- .../unit/services/brokerRecordService.test.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/backend/__tests__/unit/services/brokerRecordService.test.js b/backend/__tests__/unit/services/brokerRecordService.test.js index 15228c4995..4f70df85f0 100644 --- a/backend/__tests__/unit/services/brokerRecordService.test.js +++ b/backend/__tests__/unit/services/brokerRecordService.test.js @@ -13,6 +13,7 @@ const mockRoomGrant = { findOne: jest.fn() }; jest.mock('../../../models/RoomGrant', () => ({ __esModule: true, default: mockRoomGrant })); const service = require('../../../services/brokerRecordService'); +const { appendEntry } = require('../../../vendor/seats/src/record/format'); const POD = '6a692a1be833c668acdb84cf'; const AGENT = '6a920d43b08dee6c42a81c9e'; @@ -51,14 +52,26 @@ describe('recordBrokerCall', () => { expect(lines[0].actor).toEqual({ type: 'agent', id: AGENT, owner: OWNER }); }); - it('a failed agent lookup loses only the owner, never the lines', async () => { + it.each([ + ['rejects', () => { throw new Error('mongo down'); }, true], + ['finds no user', () => null, false], + ])('an owner lookup that %s loses only the owner, never the lines', async (_, result, warns) => { const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); - mockUser.findById.mockReturnValue({ select: () => ({ lean: async () => { throw new Error('mongo down'); } }) }); + mockUser.findById.mockReturnValue({ select: () => ({ lean: async () => result() }) }); service.recordBrokerCall(row({ outcome: 'ok' })); const lines = await linesOf(mockEmits[0]); expect(lines.map((l) => l.kind)).toEqual(['tool.call', 'permission.requested', 'permission.decided', 'tool.update']); expect(lines[0].actor).toEqual({ type: 'agent', id: AGENT }); - expect(warn).toHaveBeenCalledWith('[record] agent owner lookup failed:', 'Error'); + // The shared writer takes an agent actor with no owner, so the lines persist. + let prev = null; + lines.forEach((line, i) => { + prev = appendEntry(prev, { + runId: 'srv-pod-0123456789abcdef', podId: POD, ts: `2026-10-28T17:00:0${i}.000Z`, + kind: line.kind, ...(line.actor === undefined ? {} : { actor: line.actor }), data: line.data, + }); + }); + if (warns) expect(warn).toHaveBeenCalledWith('[record] agent owner lookup failed:', 'Error'); + else expect(warn).not.toHaveBeenCalled(); warn.mockRestore(); }); From 99c022c5bc6d24307e64deb7e0de7438a628eb40 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 31 Oct 2026 19:18:31 -0700 Subject: [PATCH 5/5] fix(record): a task-bound grant gets one grant.issued line (TASK-693) The merge brought mintGrant's task-chain writer in beside the route's pod-chain call, so a bound mint or attenuation wrote the line twice. The route now writes only unbound grants; mintGrant owns bound ones. A test pins the route's side. Co-Authored-By: Claude Haiku 5.5 Co-authored-by: sprint-impl Agent-Model: claude/claude-haiku-5-5/default --- backend/__tests__/unit/routes/grants.mint.test.js | 6 ++++++ backend/routes/grants.ts | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/backend/__tests__/unit/routes/grants.mint.test.js b/backend/__tests__/unit/routes/grants.mint.test.js index c628ea7328..c02ab3f2d9 100644 --- a/backend/__tests__/unit/routes/grants.mint.test.js +++ b/backend/__tests__/unit/routes/grants.mint.test.js @@ -476,6 +476,12 @@ describe('task-bound mint', () => { expect(res.body.error).toBe('invalid_task_binding'); expect(await RoomGrant.countDocuments({})).toBe(0); }); + test('the route leaves a bound grant to mintGrant, so the record gets one line', async () => { + mockRecordGrantIssued.mockClear(); + const res = await mint({ binding: { taskId: 'TASK-1', podId: POD } }); + expect(res.status).toBe(201); + expect(mockRecordGrantIssued).not.toHaveBeenCalled(); + }); 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/routes/grants.ts b/backend/routes/grants.ts index 76c0b58e13..8612296759 100644 --- a/backend/routes/grants.ts +++ b/backend/routes/grants.ts @@ -34,6 +34,14 @@ const plainGrant = (grant: unknown) => { return (typeof doc?.toObject === 'function' ? doc.toObject() : grant) as Parameters[0]; }; +// Unbound issuance only. A task-bound grant's line is written by mintGrant on +// its task chain (roomGrantService), so writing it here too would duplicate it. +const recordUnboundIssuance = (grant: unknown, issuer: Parameters[1]) => { + const plain = plainGrant(grant); + if ((plain as { binding?: unknown }).binding) return; + recordGrantIssued(plain, issuer); +}; + // eslint-disable-next-line @typescript-eslint/no-require-imports const auth = require('../middleware/auth'); // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -437,7 +445,7 @@ router.post('/', grantRateLimit, auth, async (req: AuthenticatedRequest, res: ex }; const grant = await createGrant(grantInput); // The connection owner minted it (checked above); the record names them. - recordGrantIssued(plainGrant(grant), humanActor(userId)); + recordUnboundIssuance(grant, humanActor(userId)); return res.status(201).json(grant); } catch (error) { return handleError(res, error); @@ -487,7 +495,7 @@ router.post( issuer: agentActor(req.agentUser as Parameters[0]), }); // An agent attenuated its own grant: the agent is the issuer. - recordGrantIssued(plainGrant(child), agentActor(req.agentUser as Parameters[0])); + recordUnboundIssuance(child, agentActor(req.agentUser as Parameters[0])); return res.status(201).json(child); } catch (error) { return handleError(res, error);