diff --git a/backend/__tests__/service/agentSendFreshnessCommitFence.test.js b/backend/__tests__/service/agentSendFreshnessCommitFence.test.js new file mode 100644 index 0000000000..a6777fd930 --- /dev/null +++ b/backend/__tests__/service/agentSendFreshnessCommitFence.test.js @@ -0,0 +1,60 @@ +/** A timestamp cannot fence an INSERT whose transaction commits after a read. */ +/* eslint-disable import/extensions, import/no-unresolved -- backend TypeScript modules expose CJS exports. */ +const fs = require('fs'); +const path = require('path'); +const mongoose = require('mongoose'); +const { pool } = require('../../config/db-pg'); +const Cursor = require('../../models/AgentPodReadCursor'); +const Freshness = require('../../services/agentSendFreshness'); + +const d = process.env.INTEGRATION_TEST === 'true' ? describe : describe.skip; +const podId = new mongoose.Types.ObjectId(); +const agentId = new mongoose.Types.ObjectId(); +const peerId = new mongoose.Types.ObjectId(); + +d('agent freshness: in-flight PostgreSQL insert', () => { + beforeAll(async () => { + await mongoose.connect(process.env.MONGO_URI); + await pool.query(fs.readFileSync(path.join(__dirname, '../../config/schema.sql'), 'utf8')); + await pool.query( + "INSERT INTO pods (id, name, type, created_by) VALUES ($1, 'fence', 'chat', $2)", + [String(podId), String(peerId)], + ); + await pool.query( + "INSERT INTO users (_id, username) VALUES ($1, 'fence-peer')", + [String(peerId)], + ); + }); + + afterAll(async () => { + await Cursor.deleteOne({ _id: `${agentId}:${podId}` }); + await pool.query('DELETE FROM pods WHERE id = $1', [String(podId)]); + await pool.query('DELETE FROM users WHERE _id = $1', [String(peerId)]); + await mongoose.disconnect(); + await pool.end(); + }); + + it('holds a peer message that began before the read but committed afterward', async () => { + const writer = await pool.connect(); + try { + await writer.query('BEGIN'); + const inserted = await writer.query( + 'INSERT INTO messages (pod_id, user_id, content) VALUES ($1, $2, $3) RETURNING created_at', + [String(podId), String(peerId), 'committed after the read'], + ); + await new Promise((resolve) => { setTimeout(resolve, 20); }); + const start = await Freshness.readStart(podId); + expect(inserted.rows[0].created_at.getTime()).toBeLessThanOrEqual(start.at.getTime()); + expect(await pool.query('SELECT id FROM messages WHERE pod_id = $1', [String(podId)])) + .toMatchObject({ rows: [] }); + await Freshness.recordRead(agentId, podId, start, []); + await writer.query('COMMIT'); + const hold = await Freshness.unseenMessages(agentId, podId); + expect(hold).toMatchObject({ code: 'unseen_messages', unseenCount: 1 }); + expect(hold.messages[0].content).toBe('committed after the read'); + } finally { + await writer.query('ROLLBACK').catch(() => undefined); + writer.release(); + } + }); +}); diff --git a/backend/__tests__/service/agentSendFreshnessMongoCommitFence.test.js b/backend/__tests__/service/agentSendFreshnessMongoCommitFence.test.js new file mode 100644 index 0000000000..c1c5653f24 --- /dev/null +++ b/backend/__tests__/service/agentSendFreshnessMongoCommitFence.test.js @@ -0,0 +1,122 @@ +/* eslint-disable import/extensions, import/no-unresolved -- backend TypeScript modules expose CJS exports. */ +const mongoose = require('mongoose'); +const Message = require('../../models/Message'); +const Cursor = require('../../models/AgentPodReadCursor'); +const Freshness = require('../../services/agentSendFreshness'); +const { setupMongoDb, closeMongoDb } = require('../utils/testUtils'); + +describe('agent freshness: delayed Mongo-only write', () => { + const originalPgHost = process.env.PG_HOST; + const podId = new mongoose.Types.ObjectId(); + const agentId = new mongoose.Types.ObjectId(); + const peerId = new mongoose.Types.ObjectId(); + + beforeAll(async () => { + delete process.env.PG_HOST; + await setupMongoDb(); + }); + + afterAll(async () => { + await closeMongoDb(); + if (originalPgHost === undefined) delete process.env.PG_HOST; + else process.env.PG_HOST = originalPgHost; + }); + + beforeEach(async () => { + await Message.deleteMany({}); + await Cursor.deleteMany({}); + }); + + it('holds an insert whose createdAt predates a successful read but whose save finishes later', async () => { + const peerMessage = new Message({ + podId, + userId: peerId, + content: 'late commit', + createdAt: new Date(Date.now() - 1000), + }); + const insert = Message.collection.insertOne.bind(Message.collection); + let resumeInsert; + let notifyInsert; + const paused = new Promise((resolve) => { notifyInsert = resolve; }); + const gate = new Promise((resolve) => { resumeInsert = resolve; }); + const spy = jest.spyOn(Message.collection, 'insertOne').mockImplementation(async (...args) => { + notifyInsert(); + await gate; + return insert(...args); + }); + + let start; + try { + const saving = peerMessage.save(); + await paused; + start = await Freshness.readStart(podId); + expect(await Message.find({ podId }).lean()).toHaveLength(0); + await Freshness.recordRead(agentId, podId, start, []); + resumeInsert(); + await saving; + } finally { + resumeInsert(); + spy.mockRestore(); + } + + const stored = await Message.findById(peerMessage._id).lean(); + expect(stored.freshnessPending).toBe(false); + expect(stored.freshnessCommittedAt.getTime()).toBeGreaterThanOrEqual(start.at.getTime()); + const hold = await Freshness.unseenMessages(agentId, podId); + expect(hold).toMatchObject({ code: 'unseen_messages', unseenCount: 1 }); + expect(hold.messages[0].content).toBe('late commit'); + }); + + it('holds a persisted write left pending if its commit stamp fails', async () => { + const start = await Freshness.readStart(podId); + await Freshness.recordRead(agentId, podId, start, []); + await Message.collection.insertOne({ + podId, + userId: peerId, + content: 'pending commit', + messageType: 'text', + createdAt: new Date(Date.now() - 1000), + freshnessPending: true, + }); + expect(await Freshness.unseenMessages(agentId, podId)) + .toMatchObject({ code: 'unseen_messages', unseenCount: 1 }); + }); + + it('does not clear a hold when a delayed commit falls outside the returned tail', async () => { + const now = Date.now(); + const previousAt = new Date(now - 60000); + await Cursor.create({ + _id: `${agentId}:${podId}`, agentUserId: agentId, podId, readAt: previousAt, + }); + const newer = Array.from({ length: 21 }, (_, i) => ({ + podId, + userId: peerId, + content: `newer ${i}`, + messageType: 'text', + createdAt: new Date(now - 20000 + i), + freshnessPending: false, + freshnessCommittedAt: new Date(now - 10000 + i), + })); + await Message.collection.insertMany([ + ...newer, + { + podId, + userId: peerId, + content: 'delayed older timestamp', + messageType: 'text', + createdAt: new Date(now - 30000), + freshnessPending: false, + freshnessCommittedAt: new Date(now - 1000), + }, + ]); + + const start = await Freshness.readStart(podId); + const returnedTail = await Message.find({ podId }).sort({ createdAt: -1 }).limit(20).lean(); + expect(returnedTail.map((message) => message.content)).not.toContain('delayed older timestamp'); + await Freshness.recordRead(agentId, podId, start, returnedTail.map((message) => String(message._id))); + expect((await Cursor.findById(`${agentId}:${podId}`)).readAt).toEqual(previousAt); + const hold = await Freshness.unseenMessages(agentId, podId); + expect(hold.unseenCount).toBe(22); + expect(hold.messages[0].content).toBe('delayed older timestamp'); + }); +}); diff --git a/backend/__tests__/service/clawdbot-e2e.test.js b/backend/__tests__/service/clawdbot-e2e.test.js index 52aa40415c..d6cd410d5a 100644 --- a/backend/__tests__/service/clawdbot-e2e.test.js +++ b/backend/__tests__/service/clawdbot-e2e.test.js @@ -17,6 +17,15 @@ const mongoose = require('mongoose'); const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../utils/testUtils'); +// This suite exercises the legacy Mongo-backed Clawdbot flow. Freshness has +// its own real-database service coverage; Tier 1's PostgreSQL host is present +// here without a message schema for this fixture. +jest.mock('../../services/agentSendFreshness', () => ({ + readStart: jest.fn().mockResolvedValue({ at: new Date(0) }), + recordRead: jest.fn().mockResolvedValue(undefined), + unseenMessages: jest.fn().mockResolvedValue(null), +})); + // Models const User = require('../../models/User'); const Pod = require('../../models/Pod'); @@ -652,6 +661,12 @@ describe('Clawdbot E2E Integration Tests', () => { .send({ label: 'Message Test Token' }); agentToken = tokenRes.body.token; + // Read the current pod tail before writing; send-time freshness holds + // a blind first post when other participants already spoke here. + const contextRead = await request(app) + .get(`/api/agents/runtime/pods/${testPod._id}/messages`) + .set('Authorization', `Bearer ${agentToken}`); + expect(contextRead.status).toBe(200); }); test('should post a message to pod via runtime API', async () => { @@ -659,6 +674,7 @@ describe('Clawdbot E2E Integration Tests', () => { .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${agentToken}`) .send({ + continueAnyway: true, content: 'Hello from Clawdbot! Based on the recent discussions, here are my insights...', messageType: 'text', metadata: { @@ -678,7 +694,7 @@ describe('Clawdbot E2E Integration Tests', () => { const bareRes = await request(app) .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${agentToken}`) - .send({ content: bareContent, messageType: 'text' }); + .send({ content: bareContent, messageType: 'text', continueAnyway: true }); expect(bareRes.status).toBe(200); expect(bareRes.body.success).toBe(true); @@ -689,7 +705,7 @@ describe('Clawdbot E2E Integration Tests', () => { const escapedRes = await request(app) .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${agentToken}`) - .send({ content: escapedContent, messageType: 'text' }); + .send({ content: escapedContent, messageType: 'text', continueAnyway: true }); expect(escapedRes.status).toBe(200); expect(escapedRes.body.success).toBe(true); @@ -723,6 +739,7 @@ describe('Clawdbot E2E Integration Tests', () => { .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${agentToken}`) .send({ + continueAnyway: true, messageType: 'text', }); @@ -738,6 +755,7 @@ describe('Clawdbot E2E Integration Tests', () => { .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${agentToken}`) .send({ + continueAnyway: true, content: 'RGCTX', messageType: 'text', }); @@ -820,10 +838,14 @@ describe('Clawdbot E2E Integration Tests', () => { 3. Set up daily standups to track progress`; // 4. Post response to pod + await request(app) + .get(`/api/agents/runtime/pods/${testPod._id}/messages`) + .set('Authorization', `Bearer ${agentToken}`); const postRes = await request(app) .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${agentToken}`) .send({ + continueAnyway: true, content: clawdbotResponse, messageType: 'text', metadata: { @@ -891,6 +913,7 @@ describe('Clawdbot E2E Integration Tests', () => { .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${agentToken}`) .send({ + continueAnyway: true, content: `Response to batch ${i + 1}`, messageType: 'text', }); @@ -982,22 +1005,30 @@ describe('Clawdbot E2E Integration Tests', () => { }); test('should allow both agents to post messages to the same pod', async () => { + await request(app) + .get(`/api/agents/runtime/pods/${testPod._id}/messages`) + .set('Authorization', `Bearer ${clawdbotToken}`); // Clawdbot posts const clawdbotPost = await request(app) .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${clawdbotToken}`) .send({ + continueAnyway: true, content: 'Message from Clawdbot Bridge', messageType: 'text', }); expect(clawdbotPost.status).toBe(200); + await request(app) + .get(`/api/agents/runtime/pods/${testPod._id}/messages`) + .set('Authorization', `Bearer ${commonlyBotToken}`); // Commonly Bot posts const commonlyBotPost = await request(app) .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${commonlyBotToken}`) .send({ + continueAnyway: true, content: 'Message from Commonly Bot', messageType: 'text', }); diff --git a/backend/__tests__/service/two-way-integration-e2e.test.js b/backend/__tests__/service/two-way-integration-e2e.test.js index d7f10c1b46..475f50d186 100644 --- a/backend/__tests__/service/two-way-integration-e2e.test.js +++ b/backend/__tests__/service/two-way-integration-e2e.test.js @@ -588,6 +588,7 @@ describe('Two-Way Integration E2E Tests', () => { .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${runtimeToken}`) .send({ + continueAnyway: true, content: summaryContent, messageType: 'text', // Valid enum values are: text, image metadata: { @@ -784,6 +785,7 @@ describe('Two-Way Integration E2E Tests', () => { .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${runtimeToken}`) .send({ + continueAnyway: true, content: summaryContent, messageType: 'text', }); @@ -898,6 +900,7 @@ describe('Two-Way Integration E2E Tests', () => { .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${runtimeToken}`) .send({ + continueAnyway: true, content: `πŸ“Š **Discord Update**\n\nPR review discussion between Dev1 and Dev2.`, messageType: 'text', }); @@ -1181,6 +1184,7 @@ Based on the GroupMe activity, the team is discussing a PR review for the auth f .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${clawdbotToken}`) .send({ + continueAnyway: true, content: aiResponse, messageType: 'text', metadata: { @@ -1329,6 +1333,7 @@ Based on the GroupMe activity, the team is discussing a PR review for the auth f .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${commonlyToken}`) .send({ + continueAnyway: true, content: 'πŸ“Š **Discord Summary**: Deployment pipeline issue reported.', messageType: 'text', }); @@ -1338,6 +1343,7 @@ Based on the GroupMe activity, the team is discussing a PR review for the auth f .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${clawdbotToken}`) .send({ + continueAnyway: true, content: `πŸ€– **Clawdbot Analysis** I noticed the deployment pipeline is failing. Based on similar issues I've seen: @@ -1427,6 +1433,7 @@ Would you like me to help debug this further?`, .post(`/api/agents/runtime/pods/${testPod._id}/messages`) .set('Authorization', `Bearer ${tokenRes.body.token}`) .send({ + continueAnyway: true, content: `πŸ” **Code Review** \`\`\`javascript diff --git a/backend/__tests__/unit/routes/agentsRuntime.contextBudget.test.js b/backend/__tests__/unit/routes/agentsRuntime.contextBudget.test.js index 2f639fb753..a64160b00a 100644 --- a/backend/__tests__/unit/routes/agentsRuntime.contextBudget.test.js +++ b/backend/__tests__/unit/routes/agentsRuntime.contextBudget.test.js @@ -30,6 +30,10 @@ jest.mock('../../../services/agentIdentityService', () => ({ ensureAgentInPod: jest.fn().mockResolvedValue(undefined), })); jest.mock('../../../services/agentMessageService', () => ({ getRecentMessages: jest.fn() })); +jest.mock('../../../services/agentSendFreshness', () => ({ + readStart: jest.fn().mockResolvedValue({ at: new Date(0) }), + recordRead: jest.fn().mockResolvedValue(undefined), +})); jest.mock('../../../services/agentThreadService', () => ({})); jest.mock('../../../services/podContextService', () => ({ getPodContext: jest.fn().mockResolvedValue({ _status: 'success', stats: {} }), diff --git a/backend/__tests__/unit/routes/agentsRuntime.messagesPagination.test.js b/backend/__tests__/unit/routes/agentsRuntime.messagesPagination.test.js index 43193f9c52..1df1e5bef6 100644 --- a/backend/__tests__/unit/routes/agentsRuntime.messagesPagination.test.js +++ b/backend/__tests__/unit/routes/agentsRuntime.messagesPagination.test.js @@ -27,6 +27,11 @@ jest.mock('../../../services/agentIdentityService', () => ({ jest.mock('../../../services/agentMessageService', () => ({ getRecentMessages: jest.fn(), })); +jest.mock('../../../services/agentSendFreshness', () => ({ + readStart: jest.fn(async () => ({ at: new Date() })), + recordRead: jest.fn().mockResolvedValue(undefined), + unseenMessages: jest.fn().mockResolvedValue(null), +})); jest.mock('../../../services/agentThreadService', () => ({})); jest.mock('../../../services/podContextService', () => ({})); jest.mock('../../../services/globalModelConfigService', () => ({})); @@ -45,6 +50,7 @@ jest.mock('../../../models/AgentRegistry', () => ({ const express = require('express'); const request = require('supertest'); const AgentMessageService = require('../../../services/agentMessageService'); +const AgentSendFreshness = require('../../../services/agentSendFreshness'); const router = require('../../../routes/agentsRuntime'); const app = express(); @@ -77,6 +83,7 @@ describe('GET /pods/:podId/messages pagination contract (#795)', () => { ], hasMore: true, }); + expect(AgentSendFreshness.recordRead).not.toHaveBeenCalled(); }); test('reports end-of-history when the extra row is absent', async () => { @@ -91,6 +98,23 @@ describe('GET /pods/:podId/messages pagination contract (#795)', () => { expect(res.status).toBe(200); expect(res.body.hasMore).toBe(false); expect(res.body.messages.map((message) => message.id)).toEqual(['oldest', 'newest']); + expect(AgentSendFreshness.recordRead).toHaveBeenCalledWith( + 'bot-1', 'pod-1', expect.objectContaining({ at: expect.any(Date) }), ['oldest', 'newest'], + ); + }); + + test('does not acknowledge a Mongo fallback while PostgreSQL is configured', async () => { + const originalHost = process.env.PG_HOST; + process.env.PG_HOST = 'configured'; + AgentMessageService.getRecentMessages.mockResolvedValue([]); // no postgres provenance + try { + const res = await request(app).get('/api/agents/runtime/pods/pod-1/messages'); + expect(res.status).toBe(200); + expect(AgentSendFreshness.recordRead).not.toHaveBeenCalled(); + } finally { + if (originalHost === undefined) delete process.env.PG_HOST; + else process.env.PG_HOST = originalHost; + } }); test('keeps the documented 50-message clamp while fetching a proof row', async () => { diff --git a/backend/__tests__/unit/routes/agentsRuntime.noInlineAvatars.test.js b/backend/__tests__/unit/routes/agentsRuntime.noInlineAvatars.test.js index f32df668c2..33032c81f5 100644 --- a/backend/__tests__/unit/routes/agentsRuntime.noInlineAvatars.test.js +++ b/backend/__tests__/unit/routes/agentsRuntime.noInlineAvatars.test.js @@ -32,6 +32,11 @@ jest.mock('../../../services/agentIdentityService', () => ({ jest.mock('../../../services/agentMessageService', () => ({ getRecentMessages: jest.fn(), })); +jest.mock('../../../services/agentSendFreshness', () => ({ + readStart: jest.fn(async () => ({ at: new Date() })), + recordRead: jest.fn().mockResolvedValue(undefined), + unseenMessages: jest.fn().mockResolvedValue(null), +})); jest.mock('../../../services/agentThreadService', () => ({})); jest.mock('../../../services/podContextService', () => ({})); jest.mock('../../../services/globalModelConfigService', () => ({})); diff --git a/backend/__tests__/unit/routes/agentsRuntime.post-message.config.test.js b/backend/__tests__/unit/routes/agentsRuntime.post-message.config.test.js index ff1fa36e55..281d9b9a8c 100644 --- a/backend/__tests__/unit/routes/agentsRuntime.post-message.config.test.js +++ b/backend/__tests__/unit/routes/agentsRuntime.post-message.config.test.js @@ -9,15 +9,24 @@ jest.mock('../../../services/agentIdentityService', () => ({ buildAgentUsername: jest.fn((agentName, instanceId = 'default') => ( instanceId === 'default' ? agentName : `${agentName}-${instanceId}` )), + getOrCreateAgentUser: jest.fn().mockResolvedValue({ _id: 'bot-1' }), + ensureAgentInPod: jest.fn(), })); jest.mock('../../../services/agentMessageService', () => ({ postMessage: jest.fn(), + getRecentMessages: jest.fn(), + sanitizeAgentContent: jest.fn((content) => content), +})); +jest.mock('../../../services/agentSendFreshness', () => ({ + readStart: jest.fn(async () => ({ at: new Date('2026-10-24T00:00:00Z') })), + recordRead: jest.fn(), + unseenMessages: jest.fn().mockResolvedValue(null), })); jest.mock('../../../services/mentionNonMemberNoticeService', () => ({ findNonMemberMentionNotices: jest.fn(), })); jest.mock('../../../services/agentThreadService', () => ({})); -jest.mock('../../../services/podContextService', () => ({})); +jest.mock('../../../services/podContextService', () => ({ getPodContext: jest.fn() })); jest.mock('../../../services/globalModelConfigService', () => ({})); jest.mock('../../../services/socialPolicyService', () => ({})); // agentsRuntime mounts its upload route at module load. This test exercises @@ -31,6 +40,8 @@ jest.mock('../../../integrations', () => ({ get: jest.fn() })); jest.mock('../../../models/Activity', () => ({})); jest.mock('../../../models/User', () => ({ findById: jest.fn() })); jest.mock('../../../models/Post', () => ({ findById: jest.fn() })); +jest.mock('../../../models/File', () => ({ findOne: jest.fn(), deleteOne: jest.fn() })); +jest.mock('../../../services/objectStore', () => ({ getObjectStore: jest.fn() })); jest.mock('../../../models/Pod', () => ({ find: jest.fn(), findById: jest.fn(() => ({ select: () => ({ lean: async () => ({ type: 'chat' }) }) })), @@ -42,6 +53,10 @@ jest.mock('../../../models/AgentRegistry', () => ({ })); const AgentMessageService = require('../../../services/agentMessageService'); +const AgentSendFreshness = require('../../../services/agentSendFreshness'); +const PodContextService = require('../../../services/podContextService'); +const File = require('../../../models/File'); +const { getObjectStore } = require('../../../services/objectStore'); const { findNonMemberMentionNotices } = require('../../../services/mentionNonMemberNoticeService'); const router = require('../../../routes/agentsRuntime'); @@ -174,4 +189,119 @@ describe('agentsRuntime post message installation config forwarding', () => { expect(AgentMessageService.postMessage).toHaveBeenCalledTimes(1); expect(AgentMessageService.postMessage.mock.calls[0][0]).not.toHaveProperty('answeringMessageId'); }); + + it('holds an unseen conversation and permits an explicit override', async () => { + const handler = getRouteHandler('/pods/:podId/messages', 'post'); + const installation = { podId: 'pod-1', agentName: 'openclaw', instanceId: 'tarik', config: {} }; + const req = { + params: { podId: 'pod-1' }, body: { content: 'hello' }, agentUser: { _id: 'bot-1' }, + agentInstallation: installation, agentInstallations: [installation], + }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + const hold = { code: 'unseen_messages', unseenCount: 4, messages: [{ id: '7' }] }; + AgentSendFreshness.unseenMessages.mockResolvedValueOnce(hold); + + await handler(req, res); + expect(res.status).toHaveBeenCalledWith(409); + expect(res.json).toHaveBeenCalledWith(hold); + expect(AgentMessageService.postMessage).not.toHaveBeenCalled(); + + req.body.continueAnyway = true; + AgentMessageService.postMessage.mockResolvedValue({ success: true }); + await handler(req, res); + expect(AgentSendFreshness.unseenMessages).toHaveBeenCalledTimes(1); + expect(AgentMessageService.postMessage).toHaveBeenCalledTimes(1); + }); +}); + +describe('bot-token send-time freshness', () => { + const user = { + _id: 'bot-1', username: 'openclaw', isBot: true, + botMetadata: { agentName: 'openclaw', instanceId: 'default' }, + }; + const req = { params: { podId: 'pod-1' }, userId: 'bot-1', query: {}, body: { content: 'hello' } }; + const response = () => ({ status: jest.fn().mockReturnThis(), json: jest.fn() }); + + beforeEach(() => { + jest.clearAllMocks(); + require('../../../models/User').findById.mockReturnValue({ lean: async () => user }); + require('../../../models/AgentRegistry').AgentInstallation.findOne.mockReturnValue({ + lean: async () => ({ agentName: 'openclaw', instanceId: 'default', displayName: 'OpenClaw' }), + }); + }); + + it('stamps only a successful bot-token message read and holds a stale post', async () => { + const messages = [{ id: 'm1', content: 'new message' }]; + AgentMessageService.getRecentMessages.mockResolvedValue(messages); + const get = getRouteHandler('/bot/pods/:podId/messages', 'get'); + const getRes = response(); + await get(req, getRes); + expect(getRes.json).toHaveBeenCalledWith({ messages }); + expect(AgentSendFreshness.recordRead).toHaveBeenCalledWith( + 'bot-1', 'pod-1', expect.objectContaining({ at: expect.any(Date) }), ['m1'], + ); + + AgentSendFreshness.unseenMessages.mockResolvedValueOnce({ code: 'unseen_messages', unseenCount: 1 }); + const post = getRouteHandler('/bot/pods/:podId/messages', 'post'); + const heldRes = response(); + await post(req, heldRes); + expect(heldRes.status).toHaveBeenCalledWith(409); + expect(AgentMessageService.postMessage).not.toHaveBeenCalled(); + + AgentMessageService.postMessage.mockResolvedValueOnce({ success: true }); + const allowedRes = response(); + await post({ ...req, body: { content: 'hello', continueAnyway: true } }, allowedRes); + expect(allowedRes.json).toHaveBeenCalledWith({ success: true }); + expect(AgentMessageService.postMessage).toHaveBeenCalledTimes(1); + }); + + it('stamps bot-token context only when its recent messages were read', async () => { + const get = getRouteHandler('/bot/pods/:podId/context', 'get'); + PodContextService.getPodContext.mockResolvedValueOnce({ _recentMessagesRead: false, summary: 'cached' }); + await get(req, response()); + expect(AgentSendFreshness.recordRead).not.toHaveBeenCalled(); + + PodContextService.getPodContext.mockResolvedValueOnce({ _recentMessagesRead: true, summary: 'fresh' }); + const res = response(); + await get(req, res); + expect(AgentSendFreshness.recordRead).toHaveBeenCalledTimes(1); + expect(res.json).toHaveBeenCalledWith({ summary: 'fresh' }); + }); +}); + +describe('attachment freshness', () => { + const installation = { podId: 'pod-1', agentName: 'openclaw', instanceId: 'default' }; + const req = { + params: { podId: 'pod-1', fileName: 'file-1.txt' }, + agentUser: { _id: 'bot-1' }, + agentInstallation: installation, + agentInstallations: [installation], + }; + const response = () => ({ status: jest.fn().mockReturnThis(), json: jest.fn() }); + + beforeEach(() => jest.clearAllMocks()); + + it('holds before upload when the pod has unseen messages', async () => { + const handler = getRouteHandler('/pods/:podId/send-freshness', 'get'); + const hold = { code: 'unseen_messages', unseenCount: 1 }; + AgentSendFreshness.unseenMessages.mockResolvedValueOnce(hold); + const res = response(); + await handler(req, res); + expect(res.status).toHaveBeenCalledWith(409); + expect(res.json).toHaveBeenCalledWith(hold); + }); + + it('keeps a published file card outside any cleanup route', async () => { + const post = getRouteHandler('/pods/:podId/messages', 'post'); + AgentMessageService.postMessage.mockResolvedValue({ success: true, message: { id: 'm1' } }); + const res = response(); + await post({ + ...req, + body: { content: '[[upload:file-1.txt|report.txt|42|document]]', continueAnyway: true }, + }, res); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ success: true })); + expect(() => getRouteHandler('/pods/:podId/uploads/:fileName', 'delete')).toThrow('Route not found'); + expect(File.deleteOne).not.toHaveBeenCalled(); + expect(getObjectStore).not.toHaveBeenCalled(); + }); }); diff --git a/backend/__tests__/unit/routes/tasksApi.leaseGuard.test.js b/backend/__tests__/unit/routes/tasksApi.leaseGuard.test.js index cf04f697de..adcb7f21d1 100644 --- a/backend/__tests__/unit/routes/tasksApi.leaseGuard.test.js +++ b/backend/__tests__/unit/routes/tasksApi.leaseGuard.test.js @@ -83,6 +83,10 @@ jest.mock('../../../models/User', () => ({ jest.mock('../../../services/githubAppService', () => ({ isPatConfigured: jest.fn(() => false) })); jest.mock('../../../services/taskEventService', () => ({ emitTaskUpdated: jest.fn() })); +// This suite isolates lease ownership. Freshness is covered by its own route tests. +jest.mock('../../../services/agentSendFreshness', () => ({ + unseenMessages: jest.fn().mockResolvedValue(null), +})); const mockRecordTaskAttention = jest.fn(); const mockResolveTaskAttention = jest.fn(); jest.mock('../../../services/attentionItemService', () => ({ diff --git a/backend/__tests__/unit/routes/tasksApi.updateRenewsLease.test.js b/backend/__tests__/unit/routes/tasksApi.updateRenewsLease.test.js index af9ca4777f..1582758093 100644 --- a/backend/__tests__/unit/routes/tasksApi.updateRenewsLease.test.js +++ b/backend/__tests__/unit/routes/tasksApi.updateRenewsLease.test.js @@ -55,6 +55,9 @@ jest.mock('../../../middleware/agentRuntimeAuth', () => (req, res, next) => { } next(); }); +jest.mock('../../../services/agentSendFreshness', () => ({ + unseenMessages: jest.fn().mockResolvedValue(null), +})); const POD_ID = 'aaaaaaaaaaaaaaaaaaaaaaaa'; @@ -90,6 +93,7 @@ jest.mock('../../../services/attentionItemService', () => ({ })); const Task = require('../../../models/Task'); +const AgentSendFreshness = require('../../../services/agentSendFreshness'); const tasksApi = require('../../../routes/tasksApi'); const MIN = 60 * 1000; @@ -131,10 +135,45 @@ afterAll(async () => { beforeEach(async () => { await Task.deleteMany({}); + AgentSendFreshness.unseenMessages.mockReset(); + AgentSendFreshness.unseenMessages.mockResolvedValue(null); mockRecordTaskAttention.mockReset(); mockRecordTaskAttention.mockResolvedValue(undefined); }); +describe('send-time freshness on board writes', () => { + const agentId = '6a693bfbe833c668acdce53b'; + const agentPost = (suffix, body) => request(app) + .post(`/api/v1/tasks/${POD_ID}/TASK-001/${suffix}`) + .set('x-test-user', agentId) + .set('x-test-username', 'tern') + .send(body); + + it('holds a claim without changing the row, then accepts continueAnyway', async () => { + await seed({ status: 'pending' }); + AgentSendFreshness.unseenMessages.mockResolvedValueOnce({ code: 'unseen_messages', unseenCount: 2 }); + const held = await agentPost('claim', {}); + expect(held.status).toBe(409); + expect((await Task.findOne({ taskId: 'TASK-001' }).lean()).status).toBe('pending'); + + const continued = await agentPost('claim', { continueAnyway: true }); + expect(continued.status).toBe(200); + expect((await Task.findOne({ taskId: 'TASK-001' }).lean()).status).toBe('claimed'); + }); + + it('holds an update without appending a note, then accepts continueAnyway', async () => { + await seed({ status: 'pending' }); + AgentSendFreshness.unseenMessages.mockResolvedValueOnce({ code: 'unseen_messages', unseenCount: 1 }); + const held = await agentPost('updates', { text: 'progress' }); + expect(held.status).toBe(409); + expect((await Task.findOne({ taskId: 'TASK-001' }).lean()).updates).toHaveLength(0); + + const continued = await agentPost('updates', { text: 'progress', continueAnyway: true }); + expect(continued.status).toBe(200); + expect((await Task.findOne({ taskId: 'TASK-001' }).lean()).updates).toHaveLength(1); + }); +}); + describe('POST /updates renews a holder-authored lease', () => { it('the holder\'s note pushes the lease out by a full period', async () => { const claimedAt = new Date(Date.now() - 25 * MIN); diff --git a/backend/__tests__/unit/services/agentMessageService.selfIdentity.test.js b/backend/__tests__/unit/services/agentMessageService.selfIdentity.test.js index c21dd60865..6e340ed204 100644 --- a/backend/__tests__/unit/services/agentMessageService.selfIdentity.test.js +++ b/backend/__tests__/unit/services/agentMessageService.selfIdentity.test.js @@ -68,6 +68,7 @@ describe('getRecentMessages β€” PG path', () => { const out = await AgentMessageService.getRecentMessages(POD, 10, SELF); expect(out.map((m) => m.self)).toEqual([true, false]); + expect(out.readSource).toBe('postgres'); }); test('passes the exclusive history cursor to PostgreSQL', async () => { @@ -159,6 +160,7 @@ describe('getRecentMessages β€” Mongo fallback path', () => { const out = await AgentMessageService.getRecentMessages(POD, 10); expect(out[0].isBot).toBe(true); + expect(out.readSource).toBe('mongo'); }); test('stamps `self` on the Mongo path too', async () => { diff --git a/backend/__tests__/unit/services/agentSendFreshness.test.js b/backend/__tests__/unit/services/agentSendFreshness.test.js new file mode 100644 index 0000000000..e5f2179e9b --- /dev/null +++ b/backend/__tests__/unit/services/agentSendFreshness.test.js @@ -0,0 +1,123 @@ +jest.mock('../../../models/AgentPodReadCursor', () => ({ + findOne: jest.fn(), + updateOne: jest.fn(), +})); +jest.mock('../../../models/Message', () => ({ + countDocuments: jest.fn(), + find: jest.fn(), + exists: jest.fn(), +})); +jest.mock('../../../config/db-pg', () => ({ pool: { query: jest.fn() } })); + +const Cursor = require('../../../models/AgentPodReadCursor'); +const Message = require('../../../models/Message'); +const { pool } = require('../../../config/db-pg'); +const Freshness = require('../../../services/agentSendFreshness'); +const originalPgHost = process.env.PG_HOST; + +describe('agent send-time freshness', () => { + beforeEach(() => { + jest.clearAllMocks(); + delete process.env.PG_HOST; + Message.exists.mockResolvedValue(null); + Cursor.findOne.mockReturnValue({ select: () => ({ lean: async () => ({ readAt: new Date('2026-10-24T00:00:00Z') }) }) }); + }); + afterAll(() => { + if (originalPgHost === undefined) delete process.env.PG_HOST; + else process.env.PG_HOST = originalPgHost; + }); + + it('counts other authors since the read and previews no more than three', async () => { + Message.countDocuments.mockResolvedValue(7); + const rows = [1, 2, 3].map((id) => ({ _id: id, content: `message ${id}`, userId: { username: 'peer' }, createdAt: new Date() })); + Message.find.mockReturnValue({ sort: () => ({ limit: () => ({ populate: () => ({ lean: async () => rows }) }) }) }); + + const hold = await Freshness.unseenMessages('agent-id', 'pod-id'); + expect(hold.unseenCount).toBe(7); + expect(hold.messages).toHaveLength(3); + expect(hold.message).toContain('continueAnyway:true'); + expect(Message.countDocuments).toHaveBeenCalledWith({ + podId: 'pod-id', + $and: [ + { $or: [{ userId: { $ne: 'agent-id' } }, { authorKind: 'kernel' }] }, + { $or: [ + { freshnessPending: true }, + { freshnessCommittedAt: { $gte: new Date('2026-10-24T00:00:00Z') } }, + { + freshnessPending: { $exists: false }, + freshnessCommittedAt: { $exists: false }, + createdAt: { $gt: new Date('2026-10-24T00:00:00Z') }, + }, + ] }, + ], + }); + }); + + it('lets a quiet pod through and fails closed when the message store fails', async () => { + Message.countDocuments.mockResolvedValueOnce(0).mockRejectedValueOnce(new Error('db down')); + Message.find.mockReturnValue({ sort: () => ({ limit: () => ({ populate: () => ({ lean: async () => [] }) }) }) }); + expect(await Freshness.unseenMessages('agent-id', 'pod-id')).toBeNull(); + await expect(Freshness.unseenMessages('agent-id', 'pod-id')).rejects.toThrow('db down'); + }); + + it('uses a monotonic watermark for a successful read', async () => { + const at = new Date('2026-10-24T01:00:00Z'); + Cursor.findOne.mockReturnValueOnce({ select: () => ({ lean: async () => null }) }); + await Freshness.recordRead('agent-id', 'pod-id', { at }, []); + expect(Cursor.updateOne).toHaveBeenCalledWith( + { _id: 'agent-id:pod-id' }, + { $max: { readAt: at }, $setOnInsert: { agentUserId: 'agent-id', podId: 'pod-id' } }, + { upsert: true }, + ); + }); + + it('does not mistake an unavailable PostgreSQL store for a quiet pod', async () => { + process.env.PG_HOST = 'configured'; + pool.query.mockRejectedValue(new Error('pg down')); + await expect(Freshness.unseenMessages('agent-id', 'pod-id')).rejects.toThrow('pg down'); + expect(Message.countDocuments).not.toHaveBeenCalled(); + }); + + it('uses the PostgreSQL count and a bounded preview when PostgreSQL is configured', async () => { + process.env.PG_HOST = 'configured'; + pool.query.mockResolvedValue({ rows: [{ + id: 23, content: 'new plan', author: 'peer', created_at: new Date(), unseen_count: 8, + }] }); + const hold = await Freshness.unseenMessages('agent-id', 'pod-id'); + expect(hold.unseenCount).toBe(8); + expect(hold.messages).toEqual([expect.objectContaining({ id: '23', content: 'new plan' })]); + expect(pool.query.mock.calls[0][0]).toContain('LIMIT 3'); + expect(pool.query.mock.calls[0][0]).toContain('freshness_revision >'); + expect(Message.countDocuments).not.toHaveBeenCalled(); + }); + + it('captures the PostgreSQL revision before the read and advances it with the cursor', async () => { + process.env.PG_HOST = 'configured'; + pool.query.mockResolvedValueOnce({ rows: [{ revision: '7' }] }).mockResolvedValueOnce({ rows: [] }); + const start = await Freshness.readStart('pod-id'); + expect(start).toEqual({ at: expect.any(Date), revision: 7 }); + expect(pool.query.mock.calls[0][0]).toContain('pod_message_revisions'); + Cursor.findOne.mockReturnValueOnce({ select: () => ({ lean: async () => null }) }); + await Freshness.recordRead('agent-id', 'pod-id', start, []); + expect(Cursor.updateOne.mock.calls[0][1].$max).toEqual({ readAt: start.at, readRevision: 7 }); + }); + + it('does not stamp a PostgreSQL read that omitted a newly committed message', async () => { + process.env.PG_HOST = 'configured'; + pool.query.mockResolvedValue({ rows: [{ '?column?': 1 }] }); + await Freshness.recordRead('agent-id', 'pod-id', { at: new Date(), revision: 12 }, ['19']); + expect(pool.query.mock.calls[0][1][4]).toEqual([19]); + expect(Cursor.updateOne).not.toHaveBeenCalled(); + }); + + it('stamps a PostgreSQL read when every new peer message was returned', async () => { + process.env.PG_HOST = 'configured'; + Cursor.findOne.mockReturnValueOnce({ + select: () => ({ lean: async () => ({ readAt: new Date('2026-10-24T00:00:00Z'), readRevision: 4 }) }), + }); + pool.query.mockResolvedValue({ rows: [] }); + await Freshness.recordRead('agent-id', 'pod-id', { at: new Date(), revision: 6 }, ['5', '6']); + expect(pool.query.mock.calls[0][1]).toEqual(['pod-id', 4, 6, 'agent-id', [5, 6]]); + expect(Cursor.updateOne).toHaveBeenCalledTimes(1); + }); +}); diff --git a/backend/config/schema.sql b/backend/config/schema.sql index a76fc0fc4a..93d142b6c5 100644 --- a/backend/config/schema.sql +++ b/backend/config/schema.sql @@ -60,6 +60,8 @@ CREATE TABLE IF NOT EXISTS messages ( -- addressing. NULL means "not in a thread"; a root is an ordinary message -- other messages point at, and a root's own thread_root_id stays NULL. thread_root_id INTEGER REFERENCES messages(id) ON DELETE SET NULL, + -- Per-pod commit-order fence for agent send freshness. Existing rows stay 0. + freshness_revision BIGINT NOT NULL DEFAULT 0, created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP ); @@ -105,6 +107,36 @@ ALTER TABLE messages ADD COLUMN IF NOT EXISTS author_kind VARCHAR(20); ALTER TABLE messages ADD COLUMN IF NOT EXISTS thread_root_id INTEGER REFERENCES messages(id) ON DELETE SET NULL; +ALTER TABLE messages ADD COLUMN IF NOT EXISTS freshness_revision BIGINT NOT NULL DEFAULT 0; + +CREATE TABLE IF NOT EXISTS pod_message_revisions ( + pod_id VARCHAR(24) PRIMARY KEY REFERENCES pods(id) ON DELETE CASCADE, + revision BIGINT NOT NULL DEFAULT 0 +); + +-- The counter update and message insert commit together. Concurrent inserts +-- into one pod serialize on this row, so revision order is commit order even +-- when an INSERT began before a reader and committed after it. +CREATE OR REPLACE FUNCTION assign_message_freshness_revision() +RETURNS TRIGGER AS $$ +BEGIN + INSERT INTO pod_message_revisions (pod_id, revision) + VALUES (NEW.pod_id, 1) + ON CONFLICT (pod_id) DO UPDATE + SET revision = pod_message_revisions.revision + 1 + RETURNING revision INTO NEW.freshness_revision; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS messages_freshness_revision ON messages; +CREATE TRIGGER messages_freshness_revision + BEFORE INSERT ON messages + FOR EACH ROW EXECUTE FUNCTION assign_message_freshness_revision(); + +CREATE INDEX IF NOT EXISTS idx_messages_pod_freshness_revision + ON messages(pod_id, freshness_revision); + -- Migration records. There was no such table anywhere under backend/ before -- this (@sprint-review, 56859, checked with a positive control): schema.sql is -- idempotent boot DDL, which says what the shape IS and never when it changed. diff --git a/backend/models/AgentPodReadCursor.ts b/backend/models/AgentPodReadCursor.ts new file mode 100644 index 0000000000..7f67513060 --- /dev/null +++ b/backend/models/AgentPodReadCursor.ts @@ -0,0 +1,14 @@ +import mongoose, { Schema } from 'mongoose'; + +const AgentPodReadCursorSchema = new Schema({ + _id: { type: String, required: true }, + agentUserId: { type: Schema.Types.ObjectId, ref: 'User', required: true }, + podId: { type: Schema.Types.ObjectId, ref: 'Pod', required: true }, + readAt: { type: Date, required: true }, + readRevision: { type: Number }, +}); + +const AgentPodReadCursor = mongoose.model('AgentPodReadCursor', AgentPodReadCursorSchema); +export default AgentPodReadCursor; +// eslint-disable-next-line @typescript-eslint/no-require-imports +module.exports = exports.default; diff --git a/backend/models/Message.ts b/backend/models/Message.ts index 9021499718..2f2a38b33c 100644 --- a/backend/models/Message.ts +++ b/backend/models/Message.ts @@ -23,6 +23,8 @@ export interface IMessage extends Document { metadata?: Record; createdAt: Date; updatedAt: Date; + freshnessPending?: boolean; + freshnessCommittedAt?: Date; } const MessageSchema = new Schema({ @@ -41,14 +43,32 @@ const MessageSchema = new Schema({ metadata: { type: Schema.Types.Mixed }, createdAt: { type: Date, default: Date.now }, updatedAt: { type: Date, default: Date.now }, + // New Mongo-only writes start pending. The post-save hook stamps a server + // commit marker after the insert is acknowledged; a crash before that stamp + // leaves pending=true, which makes the send-time hold fail closed. + freshnessPending: { type: Boolean }, + freshnessCommittedAt: { type: Date }, }); MessageSchema.pre('save', function rememberWhetherThisIsANewPost(next) { (this as any).$locals.wasNewPost = this.isNew; + if (this.isNew) this.freshnessPending = true; next(); }); MessageSchema.post('save', async (doc) => { + if ((doc as any).$locals?.wasNewPost) { + try { + await (doc.constructor as Model).updateOne( + { _id: doc._id }, + { $set: { freshnessPending: false }, $currentDate: { freshnessCommittedAt: true } }, + ); + } catch (error) { + // The insert already committed. Keep its pending marker (a safe hold) + // rather than report failure and invite a duplicate Mongo fallback. + console.warn('[Message] freshness commit stamp failed:', (error as Error).message); + } + } // Mongo is the availability fallback for chat writes, so it must materialize // the same recipient-owned fact as the normal PostgreSQL writer. // eslint-disable-next-line global-require diff --git a/backend/routes/agentsRuntime.ts b/backend/routes/agentsRuntime.ts index d2147f9f7d..830b29fbc1 100644 --- a/backend/routes/agentsRuntime.ts +++ b/backend/routes/agentsRuntime.ts @@ -25,6 +25,7 @@ const AgentEventService = require('../services/agentEventService'); const HostedRuntimeMeter = require('../services/hostedRuntimeService'); const AgentIdentityService = require('../services/agentIdentityService'); const AgentMessageService = require('../services/agentMessageService'); +const AgentSendFreshness = require('../services/agentSendFreshness'); const { findNonMemberMentionNotices } = require('../services/mentionNonMemberNoticeService'); const AgentThreadService = require('../services/agentThreadService'); const PodContextService = require('../services/podContextService'); @@ -1407,6 +1408,7 @@ router.get( return clamp(parsed, 1, max); }; + const readStartedAt = await AgentSendFreshness.readStart(podId); const context = await PodContextService.getPodContext({ podId, userId: user._id, @@ -1420,7 +1422,14 @@ router.get( skillRefreshHours: parseLimit(req.query.skillRefreshHours, 6, 72), }); - return res.json(context); + const { _recentMessagesRead, ...publicContext } = context as Record; + if (_recentMessagesRead) { + await AgentSendFreshness.recordRead( + user._id, podId, readStartedAt, + ((publicContext.recentMessages || []) as Array<{ id?: unknown }>).map((message) => String(message.id || '')), + ); + } + return res.json(publicContext); } catch (error: any) { console.error('Error fetching bot pod context:', error); return res.status(500).json({ message: 'Failed to fetch pod context' }); @@ -1462,7 +1471,14 @@ router.get( const limit = Math.min(Math.max(parseInt(req.query.limit, 10) || 20, 1), 50); // Same server-computed `self` flag as the runtime-token route (#757). + const readStartedAt = await AgentSendFreshness.readStart(podId); const messages = await AgentMessageService.getRecentMessages(podId, limit, user._id); + if (!process.env.PG_HOST || (messages as typeof messages & { readSource?: string }).readSource === 'postgres') { + await AgentSendFreshness.recordRead( + user._id, podId, readStartedAt, + messages.map((message: Record) => String(message.id || message._id || '')), + ); + } return res.json({ messages }); } catch (error: any) { console.error('Error fetching bot messages:', error); @@ -1504,6 +1520,11 @@ router.post( } const { content, metadata, messageType } = req.body || {}; + const wouldPost = messageType === 'card' || Boolean(AgentMessageService.sanitizeAgentContent(content)); + if (wouldPost && req.body?.continueAnyway !== true) { + const hold = await AgentSendFreshness.unseenMessages(user._id, podId); + if (hold) return res.status(409).json(hold); + } const result = await AgentMessageService.postMessage({ agentName: resolvedAgentName, instanceId, @@ -1716,6 +1737,7 @@ router.get('/pods/:podId/context', phase4RateLimit, agentRuntimeAuth, async (req } } + const readStartedAt = await AgentSendFreshness.readStart(podId); const context = await PodContextService.getPodContext({ podId, userId: agentUser._id, @@ -1730,11 +1752,19 @@ router.get('/pods/:podId/context', phase4RateLimit, agentRuntimeAuth, async (req maxContextTokens, }); + const { _recentMessagesRead, ...publicContext } = context as Record; + if (_recentMessagesRead) { + await AgentSendFreshness.recordRead( + agentUser._id, podId, readStartedAt, + ((publicContext.recentMessages || []) as Array<{ id?: unknown }>).map((message) => String(message.id || '')), + ); + } + // PodContextService attaches tokenEstimate/tokenBudget to `stats` only when // a budget applied, so an absent budget was invisible to the caller AND // unattributable. Say which of the four states produced it. return res.json({ - ...(context as Record), + ...publicContext, contextBudget: { applied: maxContextTokens > 0, source: contextBudgetSource }, }); } catch (error: any) { @@ -1817,6 +1847,7 @@ router.get('/pods/:podId/messages', phase4RateLimit, agentRuntimeAuth, async (re // `self` flag. The wrapper uses it to tell "I posted via my tool" from // "some OTHER agent posted while I was thinking" β€” the latter used to // silently swallow this agent's reply in any multi-agent pod (#757). + const readStartedAt = await AgentSendFreshness.readStart(podId); const messages = await AgentMessageService.getRecentMessages( podId, limit + 1, req.agentUser?._id, beforeMs, rawThreadRootId, ); @@ -1825,6 +1856,18 @@ router.get('/pods/:podId/messages', phase4RateLimit, agentRuntimeAuth, async (re // the newest `limit + 1` rows, so the extra proof row is the oldest one. const page = hasMore ? messages.slice(-limit) : messages; + if (beforeMs === undefined && rawThreadRootId === undefined + && (!process.env.PG_HOST || (messages as typeof messages & { readSource?: string }).readSource === 'postgres')) { + const agentUser = req.agentUser || await AgentIdentityService.getOrCreateAgentUser(installation.agentName, { + instanceId: installation.instanceId || 'default', + displayName: installation.displayName, + }); + await AgentSendFreshness.recordRead( + agentUser._id, podId, readStartedAt, + page.map((message: Record) => String(message.id || message._id || '')), + ); + } + return res.json({ messages: page, hasMore }); } catch (error: any) { console.error('Error fetching pod messages:', error); @@ -2132,6 +2175,19 @@ router.post('/pods/:podId/messages', phase4RateLimit, agentRuntimeAuth, async (r return res.status(403).json({ message: 'Agent token not authorized for this pod' }); } + // A total-match silence or runtime artifact is not a post. Let the + // service suppress it without making the agent read a conversation first. + const wouldPost = req.body?.messageType === 'card' + || Boolean(AgentMessageService.sanitizeAgentContent(req.body?.content)); + if (wouldPost && req.body?.continueAnyway !== true) { + const agentUser = req.agentUser || await AgentIdentityService.getOrCreateAgentUser(installation.agentName, { + instanceId: installation.instanceId || 'default', + displayName: installation.displayName, + }); + const hold = await AgentSendFreshness.unseenMessages(agentUser._id, podId); + if (hold) return res.status(409).json(hold); + } + const { content, metadata, messageType, replyToMessageId, threadRootId, } = req.body || {}; @@ -4060,6 +4116,21 @@ router.post('/asks/:requestId/respond', phase4RateLimit, agentRuntimeAuth, async const { handleUpload, uploadSingle } = require('./uploads'); // eslint-disable-next-line @typescript-eslint/no-require-imports const { validateOfficeContent } = require('../services/officeContentValidator'); +router.get('/pods/:podId/send-freshness', phase4RateLimit, agentRuntimeAuth, async (req: any, res: any) => { + try { + const { podId } = req.params; + if (!ensurePodMatch(req.agentInstallations || req.agentInstallation, podId, req.agentAuthorizedPodIds)) { + return res.status(403).json({ message: 'Agent token not authorized for this pod' }); + } + if (!req.agentUser?._id) return res.status(401).json({ message: 'Agent identity required' }); + const hold = await AgentSendFreshness.unseenMessages(req.agentUser._id, podId); + return hold ? res.status(409).json(hold) : res.json({ ok: true }); + } catch (error: any) { + console.error('Agent freshness check error:', error); + return res.status(500).json({ message: 'Failed to check send freshness' }); + } +}); + router.post('/pods/:podId/uploads', phase4RateLimit, agentRuntimeAuth, uploadSingle('file'), async (req: any, res: any) => { try { const { podId } = req.params; diff --git a/backend/routes/tasksApi.ts b/backend/routes/tasksApi.ts index f41db6748c..bbf0224142 100644 --- a/backend/routes/tasksApi.ts +++ b/backend/routes/tasksApi.ts @@ -17,6 +17,8 @@ const Task = require('../models/Task'); // eslint-disable-next-line global-require const User = require('../models/User'); // eslint-disable-next-line global-require +const AgentSendFreshness = require('../services/agentSendFreshness'); +// eslint-disable-next-line global-require const { emitTaskUpdated, notifyPodAgents } = require('../services/taskEventService'); // eslint-disable-next-line global-require const { recordTaskAttention, resolveTaskAttention } = require('../services/attentionItemService'); @@ -566,6 +568,10 @@ router.post('/:podId/:taskId/claim', rateLimit({ const author = await resolveAuthor(req); const access = await requirePodMember(podId || '', userId, { write: true }); if (access.error) return res.status(access.status || 500).json({ error: access.error }); + if (actorOf(req).isAgent && req.body?.continueAnyway !== true) { + const hold = await AgentSendFreshness.unseenMessages(userId, podId); + if (hold) return res.status(409).json(hold); + } const now = new Date(); // Pre-read the current holder to classify this claim AFTER the CAS wins: // a same-holder continuation (renewal, or re-take after the holder's own @@ -706,6 +712,10 @@ router.post('/:podId/:taskId/updates', taskWriteRateLimit(60), auth, async (req: if (!text?.trim()) return res.status(400).json({ error: 'text is required' }); const access = await requirePodMember(podId || '', userId, { write: true }); if (access.error) return res.status(access.status || 500).json({ error: access.error }); + if (actorOf(req).isAgent && req.body?.continueAnyway !== true) { + const hold = await AgentSendFreshness.unseenMessages(userId, podId); + if (hold) return res.status(409).json(hold); + } const author = await resolveAuthor(req); const now = new Date(); const note = { updates: { text: text.trim(), author, authorId: userId?.toString() || null, createdAt: now } }; diff --git a/backend/services/agentMessageService.ts b/backend/services/agentMessageService.ts index 3dc7f836cf..59eb14d63b 100644 --- a/backend/services/agentMessageService.ts +++ b/backend/services/agentMessageService.ts @@ -2229,7 +2229,7 @@ class AgentMessageService { threadRootId ?? null, ); - return messages.map((msg) => { + const normalized = messages.map((msg) => { const username = (msg.username as string) || 'Unknown'; // Trust the mirrored `is_bot` column only. This used to fall back to // a username substring test ('-bot', endsWith 'bot', 'openclaw-'), @@ -2264,6 +2264,10 @@ class AgentMessageService { replyTo: (msg.replyTo as MessageNormalized['replyTo']) ?? null, }; }); + // Internal provenance for send-time read acknowledgement. A Mongo + // fallback may omit PG-only messages and must not clear a PG cursor. + Object.defineProperty(normalized, 'readSource', { value: 'postgres' }); + return normalized; } catch (pgError) { console.error('PostgreSQL message fetch failed, falling back to MongoDB:', pgError); } @@ -2289,7 +2293,7 @@ class AgentMessageService { .populate('userId', 'username profilePicture isBot') .lean(); - return messages.reverse().map((msg) => { + const normalized = messages.reverse().map((msg) => { const userId = msg.userId as { username?: string; profilePicture?: string; isBot?: boolean; _id?: unknown } | null; const username = userId?.username || 'Unknown'; return { @@ -2305,6 +2309,8 @@ class AgentMessageService { createdAt: msg.createdAt as Date, }; }) as MessageNormalized[]; + Object.defineProperty(normalized, 'readSource', { value: 'mongo' }); + return normalized; } } diff --git a/backend/services/agentSendFreshness.ts b/backend/services/agentSendFreshness.ts new file mode 100644 index 0000000000..7ab88fa632 --- /dev/null +++ b/backend/services/agentSendFreshness.ts @@ -0,0 +1,168 @@ +// A pod-wide read watermark, recorded only after a successful current-tail read. +// eslint-disable-next-line @typescript-eslint/no-require-imports +const Cursor = require('../models/AgentPodReadCursor'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const MongoMessage = require('../models/Message'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +const { pool } = require('../config/db-pg'); + +type Preview = { id: string; author: string; content: string; createdAt: unknown }; +type Hold = { code: 'unseen_messages'; unseenCount: number; messages: Preview[]; message: string } | null; + +type ReadStart = { at: Date; revision?: number }; + +async function readStart(podId: unknown): Promise { + const at = new Date(Date.now() - 1); + if (pool && process.env.PG_HOST) { + // Read the committed revision BEFORE fetching the tail. An insert still + // in flight cannot advance this value until its message is visible too. + const result = await pool.query( + 'SELECT revision FROM pod_message_revisions WHERE pod_id = $1', + [String(podId)], + ); + return { at, revision: Number(result.rows[0]?.revision || 0) }; + } + const mongo = MongoMessage.db?.db; + if (!mongo) throw new Error('MongoDB unavailable for freshness read'); + const hello = await mongo.admin().command({ hello: 1 }); + if (!(hello.localTime instanceof Date)) throw new Error('MongoDB did not return a freshness clock'); + return { at: hello.localTime }; +} +const cursorId = (agentUserId: unknown, podId: unknown): string => `${String(agentUserId)}:${String(podId)}`; + +async function recordRead( + agentUserId: unknown, podId: unknown, start: ReadStart, observedIds: string[], +): Promise { + if (!Array.isArray(observedIds)) throw new Error('Read acknowledgement needs returned message IDs'); + const previous = await Cursor.findOne({ _id: cursorId(agentUserId, podId) }) + .select('readAt readRevision').lean(); + // The first read also needs evidence: a truncated tail cannot acknowledge + // older history just because no cursor existed yet. + if (await hasUnshownMessages( + agentUserId, podId, previous || { readAt: new Date(0), readRevision: -1 }, start, observedIds, + )) return; + // A slow earlier read must not roll a newer read's watermark backwards. + const filter = { _id: cursorId(agentUserId, podId) }; + const maximum = { + readAt: start.at, + ...(start.revision === undefined ? {} : { readRevision: start.revision }), + }; + try { + await Cursor.updateOne(filter, { $max: maximum, $setOnInsert: { agentUserId, podId } }, { upsert: true }); + } catch (error) { + if ((error as { code?: number }).code !== 11000) throw error; + // Two first reads can race on the deterministic _id; the loser still + // advances the winner's cursor without ever replacing a later timestamp. + await Cursor.updateOne(filter, { $max: maximum }); + } +} + +async function hasUnshownMessages( + agentUserId: unknown, + podId: unknown, + previous: { readAt?: Date; readRevision?: number }, + start: ReadStart, + observedIds: string[], +): Promise { + const ownId = String(agentUserId); + if (pool && process.env.PG_HOST) { + if (start.revision === undefined) throw new Error('PostgreSQL read revision missing'); + const ids = observedIds.filter((id) => /^\d+$/.test(id)).map(Number); + const result = await pool.query(` + SELECT 1 FROM messages m + WHERE m.pod_id = $1 AND m.freshness_revision > $2 AND m.freshness_revision <= $3 + AND (m.user_id IS DISTINCT FROM $4 OR m.author_kind = 'kernel') + AND NOT (m.id = ANY($5::int[])) + LIMIT 1 + `, [String(podId), previous.readRevision ?? -1, start.revision, ownId, ids]); + return result.rows.length > 0; + } + const observed = observedIds.filter((id) => /^[a-f\d]{24}$/i.test(id)); + const query = { + podId, + _id: { $nin: observed }, + $and: [ + { $or: [{ userId: { $ne: ownId } }, { authorKind: 'kernel' }] }, + { $or: [ + { freshnessCommittedAt: { $gte: previous.readAt || new Date(0), $lte: start.at } }, + { + freshnessPending: { $exists: false }, + freshnessCommittedAt: { $exists: false }, + createdAt: { $gt: previous.readAt || new Date(0), $lte: start.at }, + }, + ] }, + ], + }; + return Boolean(await MongoMessage.exists(query)); +} + +async function unseenMessages(agentUserId: unknown, podId: unknown): Promise { + const cursor = await Cursor.findOne({ _id: cursorId(agentUserId, podId) }).select('readAt readRevision').lean(); + const after = cursor?.readAt || new Date(0); + const afterRevision = cursor?.readRevision ?? -1; + const ownId = String(agentUserId); + let unseenCount: number; + let messages: Preview[]; + + if (pool && process.env.PG_HOST) { + const result = await pool.query(` + SELECT m.id, m.content, m.created_at, COALESCE(u.username, 'Unknown') AS author, + COUNT(*) OVER ()::integer AS unseen_count + FROM messages m LEFT JOIN users u ON u._id = m.user_id + WHERE m.pod_id = $1 AND m.freshness_revision > $2 + AND (m.user_id IS DISTINCT FROM $3 OR m.author_kind = 'kernel') + ORDER BY m.freshness_revision DESC, m.id DESC LIMIT 3 + `, [String(podId), afterRevision, ownId]); + unseenCount = Number(result.rows[0]?.unseen_count || 0); + messages = result.rows.map((row: Record) => ({ + id: String(row.id), + author: String(row.author), + content: String(row.content || '').slice(0, 500), + createdAt: row.created_at, + })); + } else { + ({ unseenCount, messages } = await unseenFromMongo(podId, ownId, after)); + } + + if (!unseenCount) return null; + return { + code: 'unseen_messages', + unseenCount, + messages, + message: `There are ${unseenCount} unseen messages. Read the conversation and retry, or call again with continueAnyway:true to continue.`, + }; +} + +async function unseenFromMongo(podId: unknown, ownId: string, after: Date): Promise<{ unseenCount: number; messages: Preview[] }> { + const query = { + podId, + $and: [ + { $or: [{ userId: { $ne: ownId } }, { authorKind: 'kernel' }] }, + { $or: [ + { freshnessPending: true }, + // Server clocks are millisecond-granular; equality is a hold, not an + // acknowledgement of a message that may have committed later. + { freshnessCommittedAt: { $gte: after } }, + // Rows written before this marker existed retain the old date rule. + { freshnessPending: { $exists: false }, freshnessCommittedAt: { $exists: false }, createdAt: { $gt: after } }, + ] }, + ], + }; + const [unseenCount, rows] = await Promise.all([ + MongoMessage.countDocuments(query), + MongoMessage.find(query).sort({ freshnessPending: -1, freshnessCommittedAt: -1, createdAt: -1, _id: -1 }).limit(3) + .populate('userId', 'username').lean(), + ]); + return { + unseenCount, + messages: rows.map((row: Record) => ({ + id: String(row._id), + author: row.userId?.username || 'Unknown', + content: String(row.content || '').slice(0, 500), + createdAt: row.createdAt, + })), + }; +} + +module.exports = { readStart, recordRead, unseenMessages }; +export {}; diff --git a/backend/services/podContextService.ts b/backend/services/podContextService.ts index 00b4c67e6d..83e669d6e8 100644 --- a/backend/services/podContextService.ts +++ b/backend/services/podContextService.ts @@ -438,8 +438,11 @@ class PodContextService { // not just read a rolled-up summary. Honors the tool's "recent messages" // contract. Compact + capped; full history is commonly_get_messages. let recentMessages: Array<{ id: string; author: string; content: string; createdAt: unknown }> = []; + let recentMessagesRead = false; try { const recent = await AgentMessageService.getRecentMessages(podId, 12); + recentMessagesRead = !process.env.PG_HOST + || (recent as typeof recent & { readSource?: string }).readSource === 'postgres'; recentMessages = (Array.isArray(recent) ? recent : []).map((m: Record) => ({ id: String(m.id || m._id || ''), author: (m.username as string) @@ -629,6 +632,9 @@ class PodContextService { members, files, recentMessages, + // Route-only acknowledgement guard: a degraded context read must not + // clear the hold for messages the caller did not receive. + _recentMessagesRead: recentMessagesRead, task: task || null, stats: { summaries: finalSummaries.length, diff --git a/commonly-mcp/__tests__/tools.test.mjs b/commonly-mcp/__tests__/tools.test.mjs index e749d6ed7d..416bff530d 100644 --- a/commonly-mcp/__tests__/tools.test.mjs +++ b/commonly-mcp/__tests__/tools.test.mjs @@ -88,6 +88,13 @@ describe('commonly_post_message', () => { }); }); + it('passes an explicit freshness override to the runtime route', async () => { + const fetchSpy = installFetch(async () => okResponse({ id: 'm2' })); + await byName.commonly_post_message.call({ podId: 'POD123', content: 'hi', continueAnyway: true }); + expect(JSON.parse(fetchSpy.mock.calls[0][1].body).continueAnyway).toBe(true); + expect(byName.commonly_post_message.inputSchema.properties.continueAnyway.type).toBe('boolean'); + }); + it('forwards threadRootId β€” the ping-free thread continuation (#1176 backend, added here in 0.3.4)', async () => { const fetchSpy = installFetch(async () => okResponse({ id: 'm2' })); const result = await byName.commonly_post_message.call({ @@ -139,6 +146,48 @@ describe('commonly_get_messages', () => { }); }); +describe('commonly_attach_file freshness', () => { + const filePath = new URL('../package.json', import.meta.url).pathname; + const held = { ok: false, status: 409, text: async () => JSON.stringify({ code: 'unseen_messages', unseenCount: 1 }) }; + + it('checks freshness before reading or uploading a file', async () => { + const fetchSpy = installFetch(async () => held); + const result = await byName.commonly_attach_file.call({ podId: 'POD', filePath }); + expect(result.isError).toBe(true); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(fetchSpy.mock.calls[0][0]).toContain('/pods/POD/send-freshness'); + }); + + it('keeps an uploaded file if a new message holds its card post', async () => { + const fetchSpy = installFetch(async (_url, _init) => { + const n = fetchSpy.mock.calls.length; + if (n === 1) return okResponse({ ok: true }); + if (n === 2) return okResponse({ fileName: 'file-1.json', originalName: 'package.json', size: 42 }); + if (n === 3) return held; + throw new Error('A held card must not trigger a cleanup request'); + }); + const result = await byName.commonly_attach_file.call({ podId: 'POD', filePath }); + expect(result.isError).toBe(true); + expect(JSON.parse(result.content[0].text).status).toBe(409); + expect(fetchSpy.mock.calls.map(([, init]) => init.method)).toEqual(['GET', 'POST', 'POST']); + expect(JSON.parse(result.content[0].text).body.uploadedFile).toEqual({ + fileName: 'file-1.json', originalName: 'package.json', + }); + }); + + it('lets an explicit override attach without a freshness preflight', async () => { + const fetchSpy = installFetch(async (_url, _init) => ( + fetchSpy.mock.calls.length === 1 + ? okResponse({ fileName: 'file-2.json', originalName: 'package.json', size: 42 }) + : okResponse({ id: 'm1' }) + )); + const result = await byName.commonly_attach_file.call({ podId: 'POD', filePath, continueAnyway: true }); + expect(result.isError).toBeUndefined(); + expect(fetchSpy.mock.calls.map(([, init]) => init.method)).toEqual(['POST', 'POST']); + expect(JSON.parse(fetchSpy.mock.calls[1][1].body).continueAnyway).toBe(true); + }); +}); + describe('commonly_get_context', () => { it('GETs the pod context endpoint', async () => { const fetchSpy = installFetch(async () => okResponse({ pod: {}, recentMessages: [] })); @@ -263,6 +312,14 @@ describe('commonly_get_tasks / create / claim / complete / update', () => { expect(url).toBe('https://x.example/api/v1/tasks/POD/TASK-001/claim'); }); + it('passes an explicit freshness override to task claim and update', async () => { + const fetchSpy = installFetch(async () => okResponse({ ok: true })); + await byName.commonly_claim_task.call({ podId: 'POD', taskId: 'TASK-001', continueAnyway: true }); + await byName.commonly_update_task.call({ podId: 'POD', taskId: 'TASK-001', text: 'work', continueAnyway: true }); + expect(JSON.parse(fetchSpy.mock.calls[0][1].body).continueAnyway).toBe(true); + expect(JSON.parse(fetchSpy.mock.calls[1][1].body).continueAnyway).toBe(true); + }); + it('teaches that blocked tasks resume through claim', () => { expect(byName.commonly_claim_task.description).toContain('pending or blocked'); expect(byName.commonly_claim_task.description).toContain('resumes it into active work'); diff --git a/commonly-mcp/package-lock.json b/commonly-mcp/package-lock.json index fcaf605055..a400b1892a 100644 --- a/commonly-mcp/package-lock.json +++ b/commonly-mcp/package-lock.json @@ -1,12 +1,12 @@ { "name": "@commonlyai/mcp", - "version": "0.3.14", + "version": "0.3.16", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@commonlyai/mcp", - "version": "0.3.14", + "version": "0.3.16", "license": "Apache-2.0", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.4" diff --git a/commonly-mcp/package.json b/commonly-mcp/package.json index 51da041108..88c7f6d879 100644 --- a/commonly-mcp/package.json +++ b/commonly-mcp/package.json @@ -1,7 +1,7 @@ { "name": "@commonlyai/mcp", "mcpName": "io.github.Team-Commonly/commonly", - "version": "0.3.14", + "version": "0.3.16", "license": "Apache-2.0", "repository": { "type": "git", diff --git a/commonly-mcp/server.json b/commonly-mcp/server.json index bd0ab14c9c..5265ee2c09 100644 --- a/commonly-mcp/server.json +++ b/commonly-mcp/server.json @@ -9,12 +9,12 @@ "source": "github", "subfolder": "commonly-mcp" }, - "version": "0.3.14", + "version": "0.3.16", "packages": [ { "registryType": "npm", "identifier": "@commonlyai/mcp", - "version": "0.3.14", + "version": "0.3.16", "transport": { "type": "stdio" }, diff --git a/commonly-mcp/src/tools.js b/commonly-mcp/src/tools.js index a364432008..369d07395d 100644 --- a/commonly-mcp/src/tools.js +++ b/commonly-mcp/src/tools.js @@ -168,18 +168,19 @@ export const buildTools = (config) => { }, { name: 'commonly_post_message', - description: 'Post a chat message into a pod as this agent.\n\nA pod is a CHAT ROOM a human may scroll, not a report surface. These are hard constraints, not preferences β€” an earlier version of this text said "keep it concise" and produced a 2,698-character median, because "concise" is unfalsifiable and a model can believe it complied at any length:\n- Aim under 400 characters per message. NEVER hit that by cutting content: if you have more to say, send another message. Two short messages beat one wall, and both beat saying less than you meant.\n- Over ~800 characters of ONE indivisible thing (a diff, a table, a doc) it is not a message β€” attach it with commonly_attach_file and post one line saying what it is.\n- Post the RESULT, not your reasoning. The thinking earned the answer; it is not the answer. Reasoning belongs in a PR body or a doc.\n- Never open with a bold sentence. No section headers, no βœ…/❌ lists, no pasted tables β€” those are report furniture and they are what make agent rooms unreadable.\n- Never narrate your own diligence ("noting this for the record", "stated precisely so it is not misread"). Delete those sentences entirely.\n- One idea per message. A second header means it should be two messages or a linked document.\n- Cap 3 messages per minute AND 3 in a row. The rate cap alone produced 24-message monologues: at 3/min sustained for seven minutes, one agent owned the entire room. A rate bounds how FAST you talk, never how LONG you hold the floor.\n- Before a 4th consecutive message with nobody else having spoken, STOP. Not "wrap up" β€” stop. A run of your own messages is a monologue whatever its rate, and the reader experiences it as one wall you pressed enter inside of. If the remaining material genuinely needs saying, it is a document: attach it and post one line.\n- Splitting is for one answer that does not fit, not for thinking out loud in public. Three messages answering one question is fine. Three messages arriving at an answer is reasoning, and the rule above already says reasoning does not go in the room.\n\nReply to what was actually said. If you would add nothing, do not post β€” in a 1:1 DM you may return the literal string NO_REPLY (and ONLY that string) to stay silent.\n\nThree addressing modes: a plain post broadcasts to the room; `replyToMessageId` addresses a specific message AND pings its author (matches the backend field name in ADR-004 Β§Message shape); `threadRootId` continues an existing thread WITHOUT pinging anyone β€” use it for follow-ups, elaboration, and anything past your first message on a topic. Prose overflow goes in a thread, not as consecutive top-level messages: post one top-level message, then put the detail in its thread by passing the first message\'s id as threadRootId.' + '\n\nIf a genuine fork needs a human to choose, call commonly_request_decision; include @human in the question when a specific person should rule, and let the tool post its own interactive card. Do not duplicate the ask in chat; status updates and ordinary questions remain messages.', + description: 'Post a chat message into a pod as this agent. If a 409 reports unseen messages, read them and retry, or pass continueAnyway:true to send despite them.\n\nA pod is a CHAT ROOM a human may scroll, not a report surface. These are hard constraints, not preferences β€” an earlier version of this text said "keep it concise" and produced a 2,698-character median, because "concise" is unfalsifiable and a model can believe it complied at any length:\n- Aim under 400 characters per message. NEVER hit that by cutting content: if you have more to say, send another message. Two short messages beat one wall, and both beat saying less than you meant.\n- Over ~800 characters of ONE indivisible thing (a diff, a table, a doc) it is not a message β€” attach it with commonly_attach_file and post one line saying what it is.\n- Post the RESULT, not your reasoning. The thinking earned the answer; it is not the answer. Reasoning belongs in a PR body or a doc.\n- Never open with a bold sentence. No section headers, no βœ…/❌ lists, no pasted tables β€” those are report furniture and they are what make agent rooms unreadable.\n- Never narrate your own diligence ("noting this for the record", "stated precisely so it is not misread"). Delete those sentences entirely.\n- One idea per message. A second header means it should be two messages or a linked document.\n- Cap 3 messages per minute AND 3 in a row. The rate cap alone produced 24-message monologues: at 3/min sustained for seven minutes, one agent owned the entire room. A rate bounds how FAST you talk, never how LONG you hold the floor.\n- Before a 4th consecutive message with nobody else having spoken, STOP. Not "wrap up" β€” stop. A run of your own messages is a monologue whatever its rate, and the reader experiences it as one wall you pressed enter inside of. If the remaining material genuinely needs saying, it is a document: attach it and post one line.\n- Splitting is for one answer that does not fit, not for thinking out loud in public. Three messages answering one question is fine. Three messages arriving at an answer is reasoning, and the rule above already says reasoning does not go in the room.\n\nReply to what was actually said. If you would add nothing, do not post β€” in a 1:1 DM you may return the literal string NO_REPLY (and ONLY that string) to stay silent.\n\nThree addressing modes: a plain post broadcasts to the room; `replyToMessageId` addresses a specific message AND pings its author (matches the backend field name in ADR-004 Β§Message shape); `threadRootId` continues an existing thread WITHOUT pinging anyone β€” use it for follow-ups, elaboration, and anything past your first message on a topic. Prose overflow goes in a thread, not as consecutive top-level messages: post one top-level message, then put the detail in its thread by passing the first message\'s id as threadRootId.' + '\n\nIf a genuine fork needs a human to choose, call commonly_request_decision; include @human in the question when a specific person should rule, and let the tool post its own interactive card. Do not duplicate the ask in chat; status updates and ordinary questions remain messages.', inputSchema: reqWith({ podId: STRING, content: STRING, replyToMessageId: STRING, threadRootId: STRING, + continueAnyway: { type: 'boolean' }, metadata: { type: 'object', additionalProperties: true }, }, ['podId', 'content']), - call: wrap(async ({ podId, content, replyToMessageId, threadRootId, metadata }) => request(config, { + call: wrap(async ({ podId, content, replyToMessageId, threadRootId, metadata, continueAnyway }) => request(config, { method: 'POST', path: `/api/agents/runtime/pods/${encodeURIComponent(podId)}/messages`, - body: { content, replyToMessageId, threadRootId, metadata }, + body: { content, replyToMessageId, threadRootId, metadata, continueAnyway }, })), }, { @@ -271,14 +272,18 @@ export const buildTools = (config) => { }, { name: 'commonly_attach_file', - description: 'Attach a file from THIS machine into a pod so humans and other agents can see and read it β€” a report you wrote, a diff, a generated CSV/deck, etc. Pass `filePath` (a path on the local filesystem). It uploads the file and posts it into the pod as a file card; add `message` for a line of context alongside it. Returns the uploaded file metadata (its `fileName` is what commonly_read_file takes).', - inputSchema: reqWith({ podId: STRING, filePath: STRING, message: STRING }, ['podId', 'filePath']), - call: wrap(async ({ podId, filePath, message }) => { + description: 'Attach a file from THIS machine into a pod so humans and other agents can see and read it β€” a report you wrote, a diff, a generated CSV/deck, etc. Pass `filePath` (a path on the local filesystem). It uploads the file and posts it into the pod as a file card; add `message` for a line of context alongside it. If a 409 reports unseen messages, read them and retry, or pass continueAnyway:true. If the card is held after upload, the error returns the existing fileName so you can post its directive after reading without reuploading. Returns the uploaded file metadata (its `fileName` is what commonly_read_file takes).', + inputSchema: reqWith({ podId: STRING, filePath: STRING, message: STRING, continueAnyway: { type: 'boolean' } }, ['podId', 'filePath']), + call: wrap(async ({ podId, filePath, message, continueAnyway }) => { + const podPath = `/api/agents/runtime/pods/${encodeURIComponent(podId)}`; + if (continueAnyway !== true) { + await request(config, { method: 'GET', path: `${podPath}/send-freshness` }); + } const buffer = readFileSync(filePath); const name = basename(filePath); const contentType = CONTENT_TYPES[extname(name).toLowerCase()] || 'application/octet-stream'; const uploaded = await requestUpload(config, { - path: `/api/agents/runtime/pods/${encodeURIComponent(podId)}/uploads`, + path: `${podPath}/uploads`, fileBuffer: buffer, fileName: name, contentType, @@ -290,11 +295,24 @@ export const buildTools = (config) => { const u = uploaded || {}; const directive = `[[upload:${u.fileName || name}|${u.originalName || name}|${u.size || buffer.length}|${u.kind || 'document'}]]`; const content = message ? `${message} ${directive}` : directive; - await request(config, { - method: 'POST', - path: `/api/agents/runtime/pods/${encodeURIComponent(podId)}/messages`, - body: { content }, - }); + try { + await request(config, { + method: 'POST', + path: `${podPath}/messages`, + body: { content, continueAnyway }, + }); + } catch (postError) { + // The upload is a separate pod artifact. A 409 means this card was + // held, but another post may already reference the same file. Keep + // the bytes and give the caller its name to reuse after reading. + if (postError instanceof HttpError && postError.status === 409 && u.fileName) { + throw new HttpError(409, { + ...postError.body, + uploadedFile: { fileName: u.fileName, originalName: u.originalName }, + }, `File uploaded as ${u.fileName}, but its card was held. Read the conversation, then post the existing file directive or continue anyway.`); + } + throw postError; + } return uploaded; }), }, @@ -355,12 +373,12 @@ export const buildTools = (config) => { }, { name: 'commonly_claim_task', - description: 'Claim a pending or blocked task β€” atomic: exactly one claimant wins. Claiming a blocked task resumes it into active work. The claim is a ~30-minute renewable LEASE, not a tenure (ADR-018 D4): call again to renew while genuinely working; a lapsed lease makes the task claimable by peers, so a dead claimant never holds work forever. A 409 names the live holder and when their lease frees.', - inputSchema: reqWith({ podId: STRING, taskId: STRING }, ['podId', 'taskId']), - call: wrap(async ({ podId, taskId }) => request(config, { + description: 'Claim a pending or blocked task β€” atomic: exactly one claimant wins. Claiming a blocked task resumes it into active work. The claim is a ~30-minute renewable LEASE, not a tenure (ADR-018 D4): call again to renew while genuinely working; a lapsed lease makes the task claimable by peers, so a dead claimant never holds work forever. A 409 names the live holder and when their lease frees. If a 409 reports unseen messages, read them and retry, or pass continueAnyway:true to claim despite them.', + inputSchema: reqWith({ podId: STRING, taskId: STRING, continueAnyway: { type: 'boolean' } }, ['podId', 'taskId']), + call: wrap(async ({ podId, taskId, continueAnyway }) => request(config, { method: 'POST', path: `/api/v1/tasks/${encodeURIComponent(podId)}/${encodeURIComponent(taskId)}/claim`, - body: {}, + body: { continueAnyway }, })), }, { @@ -380,16 +398,17 @@ export const buildTools = (config) => { }, { name: 'commonly_update_task', - description: 'Append an update note to a task without changing status β€” visible in the task drawer history.', + description: 'Append an update note to a task without changing status β€” visible in the task drawer history. If a 409 reports unseen messages, read them and retry, or pass continueAnyway:true to update despite them.', inputSchema: reqWith({ podId: STRING, taskId: STRING, text: STRING, + continueAnyway: { type: 'boolean' }, }, ['podId', 'taskId', 'text']), - call: wrap(async ({ podId, taskId, text }) => request(config, { + call: wrap(async ({ podId, taskId, text, continueAnyway }) => request(config, { method: 'POST', path: `/api/v1/tasks/${encodeURIComponent(podId)}/${encodeURIComponent(taskId)}/updates`, - body: { text }, + body: { text, continueAnyway }, })), }, { diff --git a/docs/plans/send-time-freshness-hold-2026-10-23.md b/docs/plans/send-time-freshness-hold-2026-10-23.md new file mode 100644 index 0000000000..1aeb595838 --- /dev/null +++ b/docs/plans/send-time-freshness-hold-2026-10-23.md @@ -0,0 +1,19 @@ +# Send-time freshness hold (TASK-441) + +## Boundary + +The server checks agent writes at the three existing entry points: a pod message, a task claim, and a task update. Human writes bypass it. A write with the literal boolean `continueAnyway: true` proceeds, including when the agent intentionally decides that the unseen conversation is irrelevant. Without that override, a 409 returns the unseen count and at most three messages; it performs no write. + +## Read cursor + +There is no durable agent conversation read cursor today. The context and message routes read messages without recording what the caller saw. Add one cursor for each agent user and pod. Capture the PostgreSQL pod revision **before** an unpaged pod read, then stamp that revision only after the read succeeds. Every message insert increments its pod revision in the same transaction. An insert that began before the read but commits afterward gets a later revision and triggers a hold even if its `created_at` predates the read. In Mongo-only mode, capture the Mongo server's clock before the read. New messages start with `freshnessPending: true`; after the insert is acknowledged, a server-side `$currentDate` write stamps `freshnessCommittedAt` and clears pending. A write left pending after a crash still holds. Historical unmarked Mongo rows use `createdAt`. Older paginated reads and thread-only reads do not advance the pod cursor: neither proves the caller saw the current pod tail. A later cursor update must never move it backwards. + +The comparison is against other authors' messages and kernel-authored messages, so an agent's own posts do not stop its next write while server prose under its identity can still be seen. A missing cursor means the agent has not read that pod; if there are messages from others, the first write holds. A successful tail read advances the cursor only when every eligible message through its start fence appears in the returned IDs. A truncated tail cannot silently clear an older delayed commit that was omitted by `createdAt` sorting. Hold previews use commit order so that row remains visible in the first three examples. For a pod with more unread history than the maximum page, the caller can explicitly use `continueAnyway`. The examples are a preview, not an automatic read acknowledgement. + +## Availability and scope + +The hold runs after authentication and pod membership checks, before any claim or message side effect. A failed freshness lookup cannot be treated as an empty pod; return an unavailable error and leave the write untouched. Query PostgreSQL when configured, or MongoDB in a Mongo-only deployment. A PostgreSQL outage must fail closed because the Mongo fallback may lack messages committed only to PostgreSQL. A degraded read served from Mongo while PostgreSQL is configured also cannot advance the cursor. Thread reads remain scoped and cannot silently clear a pod-wide hold. This initial boundary intentionally applies to the pod conversation even when a post continues a thread; thread-specific cursor semantics can be added with an explicit product rule. + +## Proof + +Focused service and route tests cover an unseen message holding each agent write, a successful read clearing the hold, the explicit override writing, a human bypass, and a failed lookup leaving the write untouched. CI runs the full suite; local runs stay focused while fleet temp storage is constrained.