From 1686d9e2520725604d8bdc23b6c229c251b47b17 Mon Sep 17 00:00:00 2001 From: aa-blinov Date: Thu, 3 Sep 2026 12:46:41 +0000 Subject: [PATCH] =?UTF-8?q?fix(tools):=20honour=20select=20fields=20in=20b?= =?UTF-8?q?24=5Ftask=5Flist=20=E2=80=94=20task=20body=20was=20unreachable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #203. `select: ['id', 'title', 'description']` reached Bitrix24 intact and Bitrix24 returned the body — then `toTaskShort` rebuilt each row from a hard-coded field list and dropped it. The select looked honoured while the task body was unreachable through the MCP entirely, since there is no single-task get tool either: an agent asked "what is this task about?" had nothing but the title to go on. Two changes: - `description` is projected when the caller puts it in the `select`, in full and never truncated, alongside Bitrix24's `descriptionInBbcode` flag ("Y"/"N" coerced to a boolean — true means the body is BBCode, false HTML) so the agent knows how to read the markup. - The select-only scalars `groupId`, `createdBy`, `parentId`, `changedDate` and `closedDate` are projected whenever Bitrix24 ships them. Bitrix24 omits these unless they are selected, so "present on the wire" already means "the operator asked for it" — no flag needed. `parentId` also unblocks walking subtasks (`MANUAL-TEST-PHRASES.md` §10.2). Default listings are byte-identical to before — the default select asks for none of these fields. The body stays behind an explicit opt-in (`ToTaskShortOptions. withDescription`) rather than riding along with the scalars, because `toTaskShort` is shared with create / update / rate and the seven lifecycle verbs, and Bitrix24 echoes the full task on those endpoints. An always-on `description` would bill the agent for a body it had just written, on every mutation. The nested `group` / `creator` objects Bitrix24 adds unbidden next to `groupId` / `createdBy` stay dropped — they carry member counts, avatar URLs and work positions, which is a lot of tokens nobody asked for. Verified against a live portal: with `description` in the select the body comes back (764 chars, `descriptionInBbcode: true`); without it the response is unchanged. +9 unit tests. --- CHANGELOG.md | 4 + README.md | 2 +- server/mcp/tools/tasks/list-tasks.ts | 17 +++- server/utils/tasks.ts | 80 +++++++++++++++-- tests/unit/tasks-utils.test.ts | 103 ++++++++++++++++++++++ tests/unit/tools/tasks/list-tasks.test.ts | 44 +++++++++ 6 files changed, 239 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e994a9e..0df80f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project are documented here. Format follows [Keep a ## [Unreleased] +### Fixed + +- **`b24_task_list` silently dropped every field it was asked for beyond the default seven (#203).** `select: ['id', 'title', 'description']` reached Bitrix24 correctly and Bitrix24 returned the body — then `toTaskShort` rebuilt the row from a hard-coded field list and threw it away, so the `select` looked honoured while the task body was unreachable through the MCP entirely (there is no single-task `get` tool either). Two changes: (1) `description` is now projected when the caller puts it in the `select`, in full and never truncated, alongside Bitrix24's `descriptionInBbcode` markup flag (`'Y'`/`'N'` coerced to a boolean — `true` means the body is BBCode, `false` HTML); (2) the select-only scalars `groupId`, `createdBy`, `parentId`, `changedDate` and `closedDate` are projected whenever Bitrix24 ships them, which — since Bitrix24 omits them unless they are selected — is the same thing as honouring the select. Default listings are byte-identical to before: the default select asks for none of these. The body stays behind an explicit opt-in flag (`ToTaskShortOptions.withDescription`) rather than riding along everywhere, because `toTaskShort` is shared with create / update / rate / the seven lifecycle verbs, and Bitrix24 echoes the full task on those — an always-on `description` would bill the agent for a body it had just written on every mutation. The nested `group` / `creator` objects Bitrix24 adds unbidden next to `groupId` / `createdBy` (member counts, avatar URLs, work positions) stay dropped. +9 unit tests. + ### Changed - **Dependency modernization — production native bump `better-sqlite3` 11 → 12.** `better-sqlite3` moves from the exact pin `11.10.0` to `12.11.1` (still exact-pinned — native module). v12's only breaking change is dropping EOL Node.js 18 / Electron 26–28; this project already requires Node ≥22 (`engines`), so there is **no consumer-facing API change** — `Database` / `prepare` / `statement.get|run|all` behave identically, verified with a native round-trip plus the full token-store suite. Operators on the pinned Docker image get a rebuilt native addon on the next image pull; no data migration (the `oauth.sqlite` on-disk format is unchanged). `@types/archiver` also moves 7 → 8 to align with the already-8.x `archiver` runtime used by the DXT zip build. Ships alongside the in-range sweep (#254): `nuxt` 4.4.8, `eslint` 10.6 (enables the new core rule `preserve-caught-error`), `@bitrix24/b24jssdk(-nuxt)` 1.3.0, `@bitrix24/b24ui-nuxt` 2.9.0, and `packageManager` `pnpm` 11.9.0 (clears three pnpm advisories). The credential-surface audit mandated by `docs/SECURITY-AUDIT.md` was run for the SDK 1.3.0 and b24ui-nuxt 2.9.0 bumps and recorded there. diff --git a/README.md b/README.md index 8e16e44..de7ca2b 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ Open Nuxt DevTools in the browser to reach the MCP Inspector for interactive too | `b24_user_me` | Returns the Bitrix24 user that owns the configured webhook. Useful as a connectivity check. | | `b24_user_find` | Find users by name / surname / position / department, or free-text. **Call this before any tool that takes a userId** — operators speak in names, not numeric ids. | | `b24_task_create` | Create a task — title, responsibleId required; description / deadline / groupId / priority / accomplices / auditors optional. | -| `b24_task_list` | List tasks with filter (`{ RESPONSIBLE_ID, STATUS, "!STATUS", ">=DEADLINE", … }`), order, select, and pagination (page size fixed at 50). | +| `b24_task_list` | List tasks with filter (`{ RESPONSIBLE_ID, STATUS, "!STATUS", ">=DEADLINE", … }`), order, select, and pagination (page size fixed at 50). Add `description` to `select` to read task bodies (with the `descriptionInBbcode` markup flag); `groupId` / `createdBy` / `parentId` / `changedDate` / `closedDate` likewise come back when selected. | | `b24_task_update` | Update an existing task by id with a partial UPPERCASE-keyed `fields` object. | | `b24_task_comment_add` | Append a comment to a task (BBCode-friendly). | | `b24_task_start` | Move a task to In progress (3). | diff --git a/server/mcp/tools/tasks/list-tasks.ts b/server/mcp/tools/tasks/list-tasks.ts index 9bbbe9b..1d1a576 100644 --- a/server/mcp/tools/tasks/list-tasks.ts +++ b/server/mcp/tools/tasks/list-tasks.ts @@ -35,7 +35,7 @@ const DEFAULT_SELECT_WIRE = normalizeBitrix24Select(DEFAULT_SELECT_CAMEL) export default defineMcpTool({ name: 'b24_task_list', description: - 'List Bitrix24 tasks. Filter / order / select keys are camelCase task fields (`responsibleId`, `status`, `deadline`, `groupId`, …) — same convention as every other task tool. Legacy UPPERCASE keys (`RESPONSIBLE_ID`, `STATUS`, …) are also accepted. Page size is fixed at 50 by Bitrix24; use `start` for pagination ((page-1)*50). Returns a trimmed list — id/title/status/deadline/responsibleId/createdDate.', + 'List Bitrix24 tasks. Filter / order / select keys are camelCase task fields (`responsibleId`, `status`, `deadline`, `groupId`, …) — same convention as every other task tool. Legacy UPPERCASE keys (`RESPONSIBLE_ID`, `STATUS`, …) are also accepted. Page size is fixed at 50 by Bitrix24; use `start` for pagination ((page-1)*50). Returns a trimmed list by default — id/title/status/deadline/responsibleId/createdDate/priority. To read a task BODY, add `description` to `select`: it then comes back in full (never truncated) together with `descriptionInBbcode` — true means the body is BBCode, false means HTML. Bodies run to thousands of characters, so ask for `description` when you actually need to read the task, not for a routine listing; narrow the `filter` first. `groupId`, `createdBy`, `parentId`, `changedDate` and `closedDate` also come back when you put them in `select` (use `parentId` to walk subtasks).', inputSchema: { filter: z .record(z.string(), z.unknown()) @@ -53,7 +53,7 @@ export default defineMcpTool({ .array(z.string()) .optional() .describe( - `Fields to return as camelCase names. Defaults to ${DEFAULT_SELECT_CAMEL.join(', ')}. UPPERCASE forms accepted. Always set this explicitly when you need a predictable shape.`, + `Fields to return as camelCase names. Defaults to ${DEFAULT_SELECT_CAMEL.join(', ')} — note the default does NOT include the task body. Add \`description\` to get it (plus the \`descriptionInBbcode\` markup flag Bitrix24 ships with it). UPPERCASE forms accepted. Always set this explicitly when you need a predictable shape.`, ), start: z .number() @@ -75,8 +75,19 @@ export default defineMcpTool({ }, 'Failed to list Bitrix24 tasks', ) + // The projection drops the task body unless it was asked for — see + // `ToTaskShortOptions.withDescription`. `select` is the operator's + // request, so match on it rather than on what Bitrix24 happened to + // return: DESCRIPTION_IN_BBCODE rides along automatically and must not + // be enough on its own to switch the body on. Accept either casing, + // matching `normalizeBitrix24Select`'s tolerance. + const wantsDescription = (select ?? []).some((field) => { + const normalized = field.trim().toLowerCase() + return normalized === 'description' || normalized === 'descriptioninbbcode' + }) + const tasks: TaskShort[] = (data?.tasks ?? []) - .map(toTaskShort) + .map((task) => toTaskShort(task, { withDescription: wantsDescription })) .filter((t): t is TaskShort => t !== null) return { diff --git a/server/utils/tasks.ts b/server/utils/tasks.ts index 728a280..0a26278 100644 --- a/server/utils/tasks.ts +++ b/server/utils/tasks.ts @@ -8,12 +8,15 @@ * map them to the UPPERCASE keys that the REST methods actually require. */ -import { pick } from '~/server/utils/wire-coerce' +import { pick, toBool } from '~/server/utils/wire-coerce' /** Subset of task fields we surface back to the agent. The full Bitrix24 * response carries 50+ fields; trimming to the agent-useful ones keeps the * context window cheap. Agents that need more should use list-tasks with an - * explicit `select`. */ + * explicit `select`. + * + * `description` is the one field that is **opt-in per call** rather than + * always-on — see {@link ToTaskShortOptions}. */ export interface TaskShort { id: number | string title: string @@ -22,15 +25,50 @@ export interface TaskShort { responsibleId?: string createdDate?: string priority?: string + /** Scalar fields Bitrix24 ships **only when they are in the `select`**, so + * projecting them when present is the same thing as honouring the select. + * They stay absent from a default listing. Issue #203 reported them + * dropped alongside `description`. */ + groupId?: string + createdBy?: string + parentId?: string + changedDate?: string | null + closedDate?: string | null + /** The task body. Only present when the caller opted in AND Bitrix24 + * actually shipped it (i.e. `description` was in the `select`). */ + description?: string + /** Bitrix24's `DESCRIPTION_IN_BBCODE` flag ("Y"/"N" on the wire), coerced + * to a boolean: `true` → `description` holds BBCode, `false` → HTML. Ships + * alongside `description` and tells the agent how to read the markup. */ + descriptionInBbcode?: boolean +} + +export interface ToTaskShortOptions { + /** + * Project `description` / `descriptionInBbcode` when the wire carries them. + * + * Off by default, and deliberately so: a task body runs to hundreds or + * thousands of characters, and `toTaskShort` is shared by every task tool + * — create / update / rate / the seven lifecycle verbs all project their + * response through it. Bitrix24 echoes the full task on those endpoints, so + * an always-on `description` would bill the agent for a body it just wrote + * on every single mutation. Only the read path (`b24_task_list`, when the + * operator puts `description` in the `select`) asks for it. + * + * Issue #203: before this option existed the field was dropped + * unconditionally, so `select: ['id', 'title', 'description']` silently + * returned no body — the `select` looked honoured but the projection ate it. + */ + withDescription?: boolean } -export function toTaskShort(raw: unknown): TaskShort | null { +export function toTaskShort(raw: unknown, options: ToTaskShortOptions = {}): TaskShort | null { if (!raw || typeof raw !== 'object') return null const r = raw as Record const id = pick(r, 'id', 'ID') const title = pick(r, 'title', 'TITLE') if (id === null || title === null) return null - return { + const short: TaskShort = { id, title, status: pick(r, 'status', 'STATUS') ?? undefined, @@ -39,6 +77,34 @@ export function toTaskShort(raw: unknown): TaskShort | null { createdDate: pick(r, 'createdDate', 'CREATED_DATE') ?? undefined, priority: pick(r, 'priority', 'PRIORITY') ?? undefined, } + // Bitrix24 omits these unless selected, so "present on the wire" already + // means "the operator asked for it" — no flag needed. Deliberately NOT + // projected: the nested `group` / `creator` objects Bitrix24 adds unbidden + // next to `groupId` / `createdBy` (they carry member counts, avatar URLs + // and work positions — a lot of tokens nobody asked for). + const scalars: [keyof TaskShort, string, string][] = [ + ['groupId', 'groupId', 'GROUP_ID'], + ['createdBy', 'createdBy', 'CREATED_BY'], + ['parentId', 'parentId', 'PARENT_ID'], + ['changedDate', 'changedDate', 'CHANGED_DATE'], + ['closedDate', 'closedDate', 'CLOSED_DATE'], + ] + for (const [key, lower, upper] of scalars) { + const value = pick(r, lower, upper) + if (value !== null) Object.assign(short, { [key]: value }) + } + + if (options.withDescription) { + // An empty description is a real state (a task with no body), so `''` + // is surfaced as-is; only an absent field stays absent. + const description = pick(r, 'description', 'DESCRIPTION') + if (description !== null) short.description = description + // The flag only means something next to a description, and Bitrix24 + // ships it automatically whenever DESCRIPTION is selected. + const inBbcode = pick(r, 'descriptionInBbcode', 'DESCRIPTION_IN_BBCODE') + if (inBbcode !== null) short.descriptionInBbcode = typeof inBbcode === 'boolean' ? inBbcode : toBool(inBbcode) + } + return short } /** @@ -46,15 +112,15 @@ export function toTaskShort(raw: unknown): TaskShort | null { * Some other endpoints (e.g. `tasks.task.add`) wrap in `{result: {task: {...}}}`. * This function tolerates both shapes and a few null variants. */ -export function extractTasks(rawResult: unknown): TaskShort[] { +export function extractTasks(rawResult: unknown, options: ToTaskShortOptions = {}): TaskShort[] { if (!rawResult || typeof rawResult !== 'object') return [] const r = rawResult as Record const tasks = r.tasks ?? r.task if (Array.isArray(tasks)) { - return tasks.map(toTaskShort).filter((t): t is TaskShort => t !== null) + return tasks.map((t) => toTaskShort(t, options)).filter((t): t is TaskShort => t !== null) } if (tasks && typeof tasks === 'object') { - const single = toTaskShort(tasks) + const single = toTaskShort(tasks, options) return single ? [single] : [] } return [] diff --git a/tests/unit/tasks-utils.test.ts b/tests/unit/tasks-utils.test.ts index 49a6a01..2de14d0 100644 --- a/tests/unit/tasks-utils.test.ts +++ b/tests/unit/tasks-utils.test.ts @@ -43,6 +43,109 @@ describe('toTaskShort', () => { ).toMatchObject({ id: '7', title: 'demo', status: '2', responsibleId: '5' }) }) + it('projects the select-only scalars when Bitrix24 ships them (issue #203)', () => { + // Bitrix24 omits these unless they are in the select, so presence on the + // wire is the opt-in. A default listing carries none of them. + expect(toTaskShort({ ID: '1', TITLE: 'demo' })).toEqual({ + id: '1', + title: 'demo', + status: undefined, + deadline: undefined, + responsibleId: undefined, + createdDate: undefined, + priority: undefined, + }) + + expect( + toTaskShort({ + id: '4153', + title: 'demo', + groupId: '111', + createdBy: '35', + parentId: '0', + changedDate: '2026-09-03T17:32:26+05:00', + closedDate: null, + }), + ).toMatchObject({ + groupId: '111', + createdBy: '35', + parentId: '0', + changedDate: '2026-09-03T17:32:26+05:00', + }) + }) + + it('drops the nested group / creator objects Bitrix24 adds unbidden', () => { + const short = toTaskShort({ + ID: '1', + TITLE: 'demo', + GROUP_ID: '111', + group: { id: '111', name: 'Проект «Баги»', membersCount: 1 }, + creator: { id: '35', name: 'Иван Петров', icon: 'https://portal/avatar.png' }, + }) + expect(short).toMatchObject({ groupId: '111' }) + expect(short).not.toHaveProperty('group') + expect(short).not.toHaveProperty('creator') + }) + + it('drops the task body unless the caller opts in (issue #203)', () => { + const raw = { + ID: '4153', + TITLE: 'demo', + DESCRIPTION: '[b]Причина[/b] длинное тело задачи', + DESCRIPTION_IN_BBCODE: 'Y', + } + // Default: body absent, so a mutation response never pays for it. + const withoutBody = toTaskShort(raw) + expect(withoutBody).not.toHaveProperty('description') + expect(withoutBody).not.toHaveProperty('descriptionInBbcode') + + // Opted in: body verbatim + the markup flag coerced from "Y"/"N". + expect(toTaskShort(raw, { withDescription: true })).toMatchObject({ + description: '[b]Причина[/b] длинное тело задачи', + descriptionInBbcode: true, + }) + }) + + it('reads the body from camelCase too, and treats "N" as HTML', () => { + expect( + toTaskShort( + { id: 1, title: 'demo', description: '

html body

', descriptionInBbcode: 'N' }, + { withDescription: true }, + ), + ).toMatchObject({ description: '

html body

', descriptionInBbcode: false }) + }) + + it('keeps an empty body as an empty string and never truncates a long one', () => { + expect( + toTaskShort({ id: 1, title: 'demo', DESCRIPTION: '' }, { withDescription: true }), + ).toMatchObject({ description: '' }) + + const long = 'а'.repeat(5000) + expect( + toTaskShort({ id: 1, title: 'demo', DESCRIPTION: long }, { withDescription: true })?.description, + ).toBe(long) + }) + + it('omits the body when opted in but Bitrix24 did not ship one', () => { + expect(toTaskShort({ id: 1, title: 'demo' }, { withDescription: true })).not.toHaveProperty( + 'description', + ) + }) + + it('extractTasks threads the option through to every row', () => { + const envelope = { + tasks: [ + { ID: '1', TITLE: 'a', DESCRIPTION: 'body a' }, + { ID: '2', TITLE: 'b', DESCRIPTION: 'body b' }, + ], + } + expect(extractTasks(envelope).map((t) => t.description)).toEqual([undefined, undefined]) + expect(extractTasks(envelope, { withDescription: true }).map((t) => t.description)).toEqual([ + 'body a', + 'body b', + ]) + }) + it('returns null when id or title is missing', () => { expect(toTaskShort({ TITLE: 'no id' })).toBeNull() expect(toTaskShort({ ID: 1 })).toBeNull() diff --git a/tests/unit/tools/tasks/list-tasks.test.ts b/tests/unit/tools/tasks/list-tasks.test.ts index 3235729..55610d0 100644 --- a/tests/unit/tools/tasks/list-tasks.test.ts +++ b/tests/unit/tools/tasks/list-tasks.test.ts @@ -31,6 +31,50 @@ describe('b24_task_list', () => { fake.v2Call.mockReset() }) + it('returns the task body only when `description` is in the select (issue #203)', async () => { + const wire = { + tasks: [ + { + id: '4153', + title: 'demo', + description: '[b]Причина[/b] тело задачи', + descriptionInBbcode: 'Y', + }, + ], + total: 1, + } + + // Default select: Bitrix24 wouldn't ship a body, and even if it does the + // projection must not smuggle it in. + fake.v2Call.mockResolvedValue(fakeOk(wire)) + const withoutBody = JSON.parse((await tool.handler({})).content[0]!.text) + expect(withoutBody.tasks[0]).not.toHaveProperty('description') + + // Asked for explicitly: body verbatim + the markup flag. + fake.v2Call.mockResolvedValue(fakeOk(wire)) + const withBody = JSON.parse( + (await tool.handler({ select: ['id', 'title', 'description'] })).content[0]!.text, + ) + expect(withBody.tasks[0]).toMatchObject({ + description: '[b]Причина[/b] тело задачи', + descriptionInBbcode: true, + }) + + // The select still reaches Bitrix24 in its UPPER_SNAKE form. + const args = fake.v2Call.mock.calls[1]![0] as unknown as { params: { select: string[] } } + expect(args.params.select).toEqual(['ID', 'TITLE', 'DESCRIPTION']) + }) + + it('accepts UPPERCASE DESCRIPTION in the select as the same opt-in', async () => { + fake.v2Call.mockResolvedValue( + fakeOk({ tasks: [{ ID: '1', TITLE: 'demo', DESCRIPTION: 'тело' }], total: 1 }), + ) + const payload = JSON.parse( + (await tool.handler({ select: ['ID', 'TITLE', 'DESCRIPTION'] })).content[0]!.text, + ) + expect(payload.tasks[0].description).toBe('тело') + }) + it('passes UPPERCASE filter/order/select/start through unchanged (back-compat) and shapes the response', async () => { fake.v2Call.mockResolvedValue( fakeOk({