diff --git a/backend/__tests__/unit/routes/tasksApi.titleAssigneeNormalization.test.js b/backend/__tests__/unit/routes/tasksApi.titleAssigneeNormalization.test.js new file mode 100644 index 000000000..bc85b2fa8 --- /dev/null +++ b/backend/__tests__/unit/routes/tasksApi.titleAssigneeNormalization.test.js @@ -0,0 +1,176 @@ +/** + * Title and assignee are single-line on every write path (TASK-354). + * + * A newline in a title breaks each single-line surface that renders it, and an + * assignee with an inner space or control character names a different actor to + * claims and the wake, which match it verbatim. Case is left alone on purpose: + * the name is the identity, and lowering it would merge two real names. + */ +const request = require('supertest'); +const express = require('express'); + +jest.mock('../../../middleware/auth', () => (req, res, next) => { + req.userId = 'u1'; + req.user = { id: 'u1', _id: 'u1' }; + next(); +}); +jest.mock('../../../middleware/agentRuntimeAuth', () => (req, res, next) => next()); + +jest.mock('../../../models/Pod', () => ({ + findById: jest.fn(() => ({ + lean: jest.fn().mockResolvedValue({ type: 'chat', members: [{ toString: () => 'u1' }] }), + })), +})); + +const mockCreate = jest.fn(); +const mockFindOneAndUpdate = jest.fn(); +jest.mock('../../../models/Task', () => ({ + find: jest.fn(() => ({ sort: () => ({ lean: async () => [] }) })), + findOne: jest.fn(() => ({ + sort: () => ({ lean: async () => null, select: () => ({ lean: async () => null }) }), + select: () => ({ lean: async () => ({ updatedAt: new Date(0), assignee: null, claimedBy: null }) }), + lean: async () => null, + })), + findOneAndUpdate: (...args) => mockFindOneAndUpdate(...args), + countDocuments: jest.fn().mockResolvedValue(0), + create: (...args) => mockCreate(...args), +})); + +jest.mock('../../../models/User', () => ({ + findById: jest.fn(() => ({ + select: jest.fn(() => ({ lean: jest.fn().mockResolvedValue({ username: 'alice' }) })), + })), +})); + +const tasksApiRoutes = require('../../../routes/tasksApi'); + +const POD = '69b7ddff0ce64c9648365fc4'; +const TASK = '69b7ddff0ce64c9648365fc5'; + +describe('task title and assignee normalization on write (TASK-354)', () => { + let app; + + beforeEach(() => { + jest.clearAllMocks(); + mockCreate.mockImplementation(async (doc) => ({ + ...doc, + toObject() { return doc; }, + })); + mockFindOneAndUpdate.mockImplementation(async (_filter, update) => ({ + ...update.$set, + toObject() { return update.$set; }, + })); + app = express(); + app.use(express.json()); + app.use('/api/v1/tasks', tasksApiRoutes); + }); + + describe('POST create', () => { + test('a newline in the title collapses to one line before it is stored', async () => { + const res = await request(app) + .post(`/api/v1/tasks/${POD}`) + .send({ title: ' Fix the\r\n board lease\nrow ' }); + expect(res.status).toBe(201); + expect(mockCreate.mock.calls[0][0].title).toBe('Fix the board lease row'); + }); + + test('a title that is only a newline is refused as empty', async () => { + const res = await request(app).post(`/api/v1/tasks/${POD}`).send({ title: '\n' }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('title is required'); + expect(mockCreate).not.toHaveBeenCalled(); + }); + + test('the assignee is trimmed and its case is kept', async () => { + const res = await request(app) + .post(`/api/v1/tasks/${POD}`) + .send({ title: 'Ship it', assignee: ' Nova ' }); + expect(res.status).toBe(201); + expect(mockCreate.mock.calls[0][0].assignee).toBe('Nova'); + }); + + test('an assignee with an inner space is refused before anything is stored', async () => { + const res = await request(app) + .post(`/api/v1/tasks/${POD}`) + .send({ title: 'Ship it', assignee: 'nova ops' }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/assign by handle/); + expect(mockCreate).not.toHaveBeenCalled(); + }); + + test('an assignee with a control character is refused', async () => { + const res = await request(app) + .post(`/api/v1/tasks/${POD}`) + .send({ title: 'Ship it', assignee: 'nova\u0007' }); + expect(res.status).toBe(400); + expect(mockCreate).not.toHaveBeenCalled(); + }); + + test('a control character from the C1 range is refused too', async () => { + const res = await request(app) + .post(`/api/v1/tasks/${POD}`) + .send({ title: 'Ship it', assignee: 'nova\u0085' }); + expect(res.status).toBe(400); + expect(mockCreate).not.toHaveBeenCalled(); + }); + + test('a replay looks up the exact raw multiline title before the normalized one', async () => { + const Task = require('../../../models/Task'); + await request(app) + .post(`/api/v1/tasks/${POD}`) + .send({ title: 'Fix\nboard', sourceRef: 'msg:1' }); + expect(Task.findOne.mock.calls[0][0].title).toBe('Fix\nboard'); + }); + + test('a blank assignee stores as unassigned', async () => { + const res = await request(app) + .post(`/api/v1/tasks/${POD}`) + .send({ title: 'Ship it', assignee: ' ' }); + expect(res.status).toBe(201); + expect(mockCreate.mock.calls[0][0].assignee).toBeNull(); + }); + }); + + describe('PATCH update', () => { + test('a newline in the title collapses to one line before the write', async () => { + const res = await request(app) + .patch(`/api/v1/tasks/${POD}/${TASK}`) + .send({ title: 'Renamed\nin two lines' }); + expect(res.status).toBe(200); + expect(mockFindOneAndUpdate.mock.calls[0][1].$set.title).toBe('Renamed in two lines'); + }); + + test('a non-string title is refused before the write, so null never reaches Mongo', async () => { + const res = await request(app) + .patch(`/api/v1/tasks/${POD}/${TASK}`) + .send({ title: null }); + expect(res.status).toBe(400); + expect(mockFindOneAndUpdate).not.toHaveBeenCalled(); + }); + + test('an assignee with an inner space is refused before the write', async () => { + const res = await request(app) + .patch(`/api/v1/tasks/${POD}/${TASK}`) + .send({ assignee: 'sprint impl' }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/assign by handle/); + expect(mockFindOneAndUpdate).not.toHaveBeenCalled(); + }); + + test('a padded assignee is trimmed on the write', async () => { + const res = await request(app) + .patch(`/api/v1/tasks/${POD}/${TASK}`) + .send({ assignee: ' Lily ' }); + expect(res.status).toBe(200); + expect(mockFindOneAndUpdate.mock.calls[0][1].$set.assignee).toBe('Lily'); + }); + + test('a title that is only whitespace is refused rather than stored blank', async () => { + const res = await request(app) + .patch(`/api/v1/tasks/${POD}/${TASK}`) + .send({ title: ' \n ' }); + expect(res.status).toBe(400); + expect(mockFindOneAndUpdate).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/backend/__tests__/unit/services/nativeRuntimeService.webFetch.test.js b/backend/__tests__/unit/services/nativeRuntimeService.webFetch.test.js index 84230d7e1..f36c8d4bb 100644 --- a/backend/__tests__/unit/services/nativeRuntimeService.webFetch.test.js +++ b/backend/__tests__/unit/services/nativeRuntimeService.webFetch.test.js @@ -24,6 +24,11 @@ jest.mock('../../../models/Pod', () => ({ }), })); jest.mock('../../../models/AgentRun', () => ({ create: jest.fn(), countDocuments: jest.fn() })); +const mockTaskCreate = jest.fn(); +jest.mock('../../../models/Task', () => ({ + countDocuments: jest.fn().mockResolvedValue(0), + create: (...args) => mockTaskCreate(...args), +})); // The agent's own bot User (its seat). Null in the arm where it cannot be found. const SEAT_ID = '507f1f77bcf86cd799439077'; const SAM_ID = '507f1f77bcf86cd7994390aa'; @@ -526,6 +531,31 @@ describe('native turn with commonly_fetch_url', () => { expect(taskMessage.content).toMatch(/unavailable for the rest of this turn because web results are in it/); }); + test('commonly_create_task accepts an assignee handle and stores it (TASK-354)', async () => { + mockTaskCreate.mockClear(); + mockTaskCreate.mockResolvedValue({ taskId: 'N-1', toObject: () => ({}) }); + axios.post + .mockResolvedValueOnce(toolCalls(call('c1', 'commonly_create_task', { title: 'Ship it', assignee: 'nova' }))) + .mockResolvedValueOnce(finalReply); + + await runAgent(install(), mention()); + + expect(mockTaskCreate).toHaveBeenCalledWith(expect.objectContaining({ title: 'Ship it', assignee: 'nova' })); + expect(toolMessages(1)[0].content).not.toMatch(/assign by handle/); + }); + + test('commonly_create_task refuses a display-name assignee before it writes a row (TASK-354)', async () => { + mockTaskCreate.mockClear(); + axios.post + .mockResolvedValueOnce(toolCalls(call('c1', 'commonly_create_task', { title: 'Ship it', assignee: 'Sprint Impl' }))) + .mockResolvedValueOnce(finalReply); + + await runAgent(install(), mention()); + + expect(toolMessages(1)[0].content).toMatch(/assign by handle/); + expect(mockTaskCreate).not.toHaveBeenCalled(); + }); + test('a guard refusal reaches the model and does not narrow the turn', async () => { mockGetPage.mockRejectedValue(refusal('address 10.0.0.5 is not public')); axios.post diff --git a/backend/routes/tasksApi.ts b/backend/routes/tasksApi.ts index af5e98250..abeaafd93 100644 --- a/backend/routes/tasksApi.ts +++ b/backend/routes/tasksApi.ts @@ -22,6 +22,8 @@ const AgentSendFreshness = require('../services/agentSendFreshness'); const { emitTaskUpdated, notifyPodAgents } = require('../services/taskEventService'); // eslint-disable-next-line global-require const { recordTaskAttention, resolveTaskAttention } = require('../services/attentionItemService'); +// eslint-disable-next-line global-require +const { parseAssignee, toSingleLineTitle } = require('../utils/taskFieldNormalization'); /** * Who made this board change, for the agent fan-out. @@ -301,8 +303,10 @@ router.post('/:podId', rateLimit({ } // Materialize primitives after runtime validation so Mongo never receives // user-supplied query objects (for example, operator-shaped values). - const title = titleInput === undefined ? undefined : String(titleInput); - const assignee = assigneeInput === undefined ? undefined : String(assigneeInput); + const title = titleInput === undefined ? undefined : toSingleLineTitle(String(titleInput)); + const assigneeParsed = assigneeInput === undefined ? undefined : parseAssignee(String(assigneeInput)); + if (assigneeParsed?.error) return res.status(400).json({ error: assigneeParsed.error }); + const assignee = assigneeParsed?.value ?? undefined; const dep = depInput === undefined ? undefined : String(depInput); const parentTask = parentTaskInput === undefined ? undefined : String(parentTaskInput); const source = sourceInput === undefined ? undefined : String(sourceInput); @@ -316,7 +320,16 @@ router.post('/:podId', rateLimit({ // the ask came from, the title says which ask it is. Keying on the ref // alone made a second ask from one source adopt the first ask's row and // discard the caller's title (TASK-063). Same pair = idempotent retry. - const existing = await Task.findOne({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), sourceRef, title }) as { _id: unknown; title?: string; status?: string; assignee?: string; claimedAt?: Date | null; claimExpiresAt?: Date | null; notes?: string; updatedAt: Date; toObject: () => unknown } | null; + // Rows written before titles were collapsed hold the raw text (`Fix\nboard`); + // a replay of that input must still find them, or the pair index admits a twin. + // The exact raw title is tried first so a `Fix board` row cannot shadow it. + const titleCandidates = [titleInput === undefined ? undefined : String(titleInput), title].filter((t, i, all) => t !== undefined && all.indexOf(t) === i); + let existing = null as { _id: unknown; title?: string; status?: string; assignee?: string; claimedAt?: Date | null; claimExpiresAt?: Date | null; notes?: string; updatedAt: Date; toObject: () => unknown } | null; + for (const candidate of titleCandidates) { + // eslint-disable-next-line no-await-in-loop + existing = await Task.findOne({ podId: mongoose.Types.ObjectId.createFromHexString(podId || ''), sourceRef, title: candidate }) as typeof existing; + if (existing) break; + } if (existing) { if (existing.status === 'done') { // `sourceRef` + `title` are an idempotency key (a unique partial @@ -959,6 +972,16 @@ router.patch('/:podId/:taskId', taskWriteRateLimit(60), auth, async (req: AuthRe if (typeof fieldUpdates.assignee === 'string' && fieldUpdates.assignee.trim() === '') { fieldUpdates.assignee = null; } + if (fieldUpdates.title !== undefined) { + if (typeof fieldUpdates.title !== 'string') return res.status(400).json({ error: 'title must be a string' }); + fieldUpdates.title = toSingleLineTitle(fieldUpdates.title); + if (!fieldUpdates.title) return res.status(400).json({ error: 'title is required' }); + } + if (typeof fieldUpdates.assignee === 'string') { + const parsed = parseAssignee(fieldUpdates.assignee); + if (parsed.error) return res.status(400).json({ error: parsed.error }); + fieldUpdates.assignee = parsed.value; + } if (Object.keys(fieldUpdates).length === 0) return res.status(400).json({ error: 'No updatable fields provided' }); if (fieldUpdates.status !== undefined) { const raw = String(fieldUpdates.status).trim().toLowerCase(); diff --git a/backend/services/nativeRuntimeService.ts b/backend/services/nativeRuntimeService.ts index 099c043c0..5fdd6f94e 100644 --- a/backend/services/nativeRuntimeService.ts +++ b/backend/services/nativeRuntimeService.ts @@ -39,6 +39,7 @@ import { getFromPublicAddress, isOutboundUrlRefused } from './outboundUrlGuard'; import { GivenAddresses } from './webAddressProvenance'; import { nativeCueFromKernel } from './nativeKernelCue'; import { agentComputerBroker, agentComputerEnabled } from './agentComputerBroker'; +import { parseAssignee, toSingleLineTitle } from '../utils/taskFieldNormalization'; import { untrustedConnectorMarker } from './brokerResultMarker'; import { engineForNewHostedInstall, HOSTED_ENGINE_NATIVE, hostedNativeDecline, hostedNativeRunConfig, hostedRunsNative, @@ -289,7 +290,7 @@ export const TOOLS = [ type: 'object', properties: { title: { type: 'string' }, - assignee: { type: 'string', description: 'Optional assignee display name' }, + assignee: { type: 'string', description: "Optional assignee: the agent's agentName, one token with no spaces. Completion memory matches on agentName, so an instanceId is not accepted here." }, notes: { type: 'string' }, }, required: ['title'], @@ -886,10 +887,14 @@ export async function dispatchTool( } case 'commonly_create_task': { - const title = String(args.title || '').trim(); + const title = toSingleLineTitle(String(args.title || '')); if (!title) { return { content: { ok: false, error: 'title is required' }, error: 'missing_title' }; } + const assigneeParsed = parseAssignee(args.assignee ? String(args.assignee) : ''); + if (assigneeParsed.error) { + return { content: { ok: false, error: assigneeParsed.error }, error: 'invalid_assignee' }; + } const Task = require('../models/Task'); // Count existing tasks in the pod to derive a taskNum — the Task // schema requires a unique (podId, taskId) tuple, so we mint a @@ -902,7 +907,7 @@ export async function dispatchTool( taskNum, taskId, title, - assignee: args.assignee ? String(args.assignee) : null, + assignee: assigneeParsed.value, notes: args.notes ? String(args.notes) : null, status: 'pending', source: 'agent', diff --git a/backend/utils/taskFieldNormalization.ts b/backend/utils/taskFieldNormalization.ts new file mode 100644 index 000000000..5f54ce015 --- /dev/null +++ b/backend/utils/taskFieldNormalization.ts @@ -0,0 +1,32 @@ +/** + * Task title and assignee rules, shared by every path that writes a task (the + * tasks API for POST and PATCH, and the native runtime's create tool). + * + * A title is one line: any run of whitespace, newlines included, collapses to a + * single space, so every single-line surface that shows it stays one line. + */ +export const toSingleLineTitle = (raw: string): string => raw.replace(/\s+/g, ' ').trim(); + +/** C0, DEL and C1 control characters. */ +const isControlCharacter = (ch: string): boolean => { + const code = ch.charCodeAt(0); + return code < 0x20 || (code >= 0x7f && code <= 0x9f); +}; + +/** + * An assignee is one handle (an agentName or instanceId). A board wake matches + * that handle verbatim, so an inner space or control character names someone + * else, and a display name stores a row no seat is woken for. Trimmed; case is + * kept. Blank after trimming means unassigned. + */ +export const parseAssignee = (raw: string): { value: string | null; error?: string } => { + const value = raw.trim(); + if (value === '') return { value: null }; + if (/\s/.test(value) || [...value].some(isControlCharacter)) { + return { + value: null, + error: 'assign by handle: one word with no spaces or control characters, e.g. nova', + }; + } + return { value }; +}; diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 552be1f34..bdf4deb8a 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -1335,7 +1335,7 @@ "title": "Title", "titlePlaceholder": "What needs to happen?", "assignee": "Assignee (optional)", - "assigneePlaceholder": "agent or teammate name", + "assigneePlaceholder": "agent handle, e.g. nova", "create": "Create task", "creating": "Creating…" } diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index a390d6891..dd53a2303 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -1327,7 +1327,7 @@ "title": "标题", "titlePlaceholder": "需要做什么?", "assignee": "负责人(可选)", - "assigneePlaceholder": "智能体或队友名称", + "assigneePlaceholder": "智能体代号", "create": "创建任务", "creating": "创建中…" }