From 216b460c564c34956d81501650f73bcd85a61b3c Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Thu, 24 Sep 2026 09:27:22 +0200 Subject: [PATCH] fix(meetings): page-number offset, author match type, and auth propagation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes to the meetings service: - `offset` was documented as a 1-based page number but implemented as an item index, so with `pageSize: 20` a request for page 2 returned items 2-21 — nineteen of the twenty the caller already had. This changes the observable behaviour of `openproject_search_meetings`: paging now returns disjoint pages. Shipping two identical pagination bugs with only the work package one fixed would have been worse. - `author` was added as a shallow field but never to the `matchType` union, so `resolveMatchType` fell through and reported a meeting found by a person's name as having matched on project name. - `getMeeting` retained `.catch(() => undefined)` on its agenda-item fetch, swallowing 401/403/429. This is the tool a model calls immediately after a search hit, so an expired token would have been reported as "this meeting has no agenda items". Also drops the duplicate `items` array from `MeetingSearchPage`; the `items` field on `PaginatedResult` and on `listMeetings` is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- src/services/meetings.ts | 33 +++++++-- tests/meetings.test.ts | 146 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 6 deletions(-) diff --git a/src/services/meetings.ts b/src/services/meetings.ts index d92e81d..945d28f 100644 --- a/src/services/meetings.ts +++ b/src/services/meetings.ts @@ -7,7 +7,7 @@ import type { OpenProjectClient } from "../client/api-client.ts"; import { extractIdFromHref, extractRawText } from "../client/hal-parser.ts"; import type { HalCollection, HalResource } from "../client/types.ts"; import { resolveClient, resolveProjectId } from "./helper.ts"; -import { searchPipeline } from "../search/pipeline.ts"; +import { isFatalSearchError, searchPipeline } from "../search/pipeline.ts"; import { rankRecords } from "../search/rank.ts"; import type { FieldSpec, MatchMode } from "../search/rank.ts"; @@ -51,7 +51,7 @@ export interface MeetingDetail extends MeetingSummary { export interface MeetingSearchResult { meeting: MeetingSummary; - matchType: "title" | "location" | "agenda_item" | "participant" | "project"; + matchType: "title" | "location" | "agenda_item" | "participant" | "author" | "project"; matchedAgendaItems?: Array<{ id: number; title: string; @@ -72,7 +72,14 @@ export interface PaginatedResult { items?: T[]; } -export interface MeetingSearchPage extends PaginatedResult { +/** + * Search results are exposed under `elements` only. `items` is deliberately + * omitted: the tool layer JSON-stringifies this whole object, so carrying the + * same array twice doubles the token cost of every search response. + */ +export interface MeetingSearchPage + extends Omit, "items"> { + elements: MeetingSearchResult[]; /** True when some deep content could not be read. */ degraded: boolean; enrichmentFailures: number; @@ -348,7 +355,15 @@ export async function getMeeting( meetingPromise, opClient .get>>(`/api/v3/meetings/${id}/agenda_items`) - .catch(() => undefined), + .catch((error: unknown) => { + // An expired token or a rate limit must not be reported to the + // model as "this meeting has no agenda items". Degrade only on + // errors that genuinely mean "agenda unavailable" (404, 500, ...). + if (isFatalSearchError(error)) { + throw error; + } + return undefined; + }), ]); let agendaItems: AgendaItem[] | undefined; @@ -410,6 +425,11 @@ function resolveMatchType(matchedFields: string[]): MeetingSearchResult["matchTy return "agenda_item"; } if (matchedFields.includes("participant")) return "participant"; + // `author` sits below `participant` and above the `project` fallback: both + // are person matches, and author carries the lower field weight. Without + // this branch an author-only hit falls through and is reported to the model + // as a project-name match. + if (matchedFields.includes("author")) return "author"; return "project"; } @@ -533,9 +553,11 @@ export async function searchMeetings( }; }); + // `offset` is a 1-based PAGE number, as the tool schema documents and as the + // rest of this codebase and the OpenProject API treat it — not an item index. const offset = params.offset ?? 1; const pageSize = params.pageSize ?? 20; - const startIndex = Math.max(0, offset - 1); + const startIndex = Math.max(0, offset - 1) * pageSize; const paged = all.slice(startIndex, startIndex + pageSize); return { @@ -544,7 +566,6 @@ export async function searchMeetings( pageSize, offset, elements: paged, - items: paged, degraded: result.degraded, enrichmentFailures: result.enrichmentFailures, }; diff --git a/tests/meetings.test.ts b/tests/meetings.test.ts index 62782a7..6b29017 100644 --- a/tests/meetings.test.ts +++ b/tests/meetings.test.ts @@ -577,3 +577,149 @@ describe("Meetings Fuzzy Search", () => { ); }); }); + +describe("Meetings Search Regressions", () => { + function meetingResource( + id: number, + title: string, + extra: Record = {}, + links: Record = {} + ) { + return { + _type: "Meeting", + id, + title, + state: "open", + startTime: `2026-09-${String(id).padStart(2, "0")}T10:00:00Z`, + endTime: `2026-09-${String(id).padStart(2, "0")}T11:00:00Z`, + _links: { project: { href: "/api/v3/projects/1", title: "Demo" }, ...links }, + ...extra, + }; + } + + function collectionClient(elements: unknown[]) { + return { + get: async (path: string) => { + if (path.startsWith("/api/v3/meetings?") || path === "/api/v3/meetings") { + return { + _type: "Collection", + total: elements.length, + count: elements.length, + pageSize: 100, + offset: 1, + _embedded: { elements }, + }; + } + if (path.includes("/agenda_items")) { + return { _type: "Collection", total: 0, _embedded: { elements: [] } }; + } + const id = Number(path.match(/meetings\/(\d+)/)?.[1] ?? NaN); + return elements.find((e) => (e as { id: number }).id === id); + }, + } as unknown as OpenProjectClient; + } + + test("an author-only match is reported as an author match, not a project match", async () => { + const client = collectionClient([ + meetingResource( + 1, + "Weekly sync", + {}, + { author: { href: "/api/v3/users/7", title: "Ingrid Kowalczyk" } } + ), + meetingResource( + 2, + "Retrospective", + {}, + { author: { href: "/api/v3/users/8", title: "Sam Okafor" } } + ), + ]); + + const res = await searchMeetings({ query: "Kowalczyk" }, client); + expect(res.elements).toHaveLength(1); + expect(res.elements[0]!.meeting.id).toBe(1); + expect(res.elements[0]!.matchedFields).toContain("author"); + // Before the fix this fell through resolveMatchType and reported + // "project", telling the model the meeting matched a project name. + expect(res.elements[0]!.matchType).toBe("author"); + }); + + test("offset is a page number: page 2 does not overlap page 1", async () => { + const elements = Array.from({ length: 12 }, (_, i) => + meetingResource(i + 1, `Arch Review ${i + 1}`) + ); + const client = collectionClient(elements); + + const whole = await searchMeetings( + { query: "Arch Review", offset: 1, pageSize: 100 }, + client + ); + const page1 = await searchMeetings({ query: "Arch Review", offset: 1, pageSize: 5 }, client); + const page2 = await searchMeetings({ query: "Arch Review", offset: 2, pageSize: 5 }, client); + + const allIds = whole.elements.map((e) => e.meeting.id); + const ids1 = page1.elements.map((e) => e.meeting.id); + const ids2 = page2.elements.map((e) => e.meeting.id); + + expect(allIds).toHaveLength(12); + expect(ids1).toEqual(allIds.slice(0, 5)); + // The bug returned allIds.slice(1, 6) here — four of five items repeated. + expect(ids2).toEqual(allIds.slice(5, 10)); + expect(ids1.filter((id) => ids2.includes(id))).toEqual([]); + }); + + test("does not ship the result array twice", async () => { + const client = collectionClient([meetingResource(1, "Arch Review")]); + const res = await searchMeetings({ query: "Arch Review" }, client); + expect(res.elements).toHaveLength(1); + expect(Object.keys(res)).not.toContain("items"); + expect(JSON.parse(JSON.stringify(res))).not.toHaveProperty("items"); + }); +}); + +describe("getMeeting error propagation", () => { + function clientFailingAgendaWith(error: unknown) { + return { + get: async (path: string) => { + if (path.includes("/agenda_items")) { + throw error; + } + return { + _type: "Meeting", + id: 5, + title: "Weekly sync", + state: "open", + startTime: "2026-09-05T10:00:00Z", + endTime: "2026-09-05T11:00:00Z", + _links: { project: { href: "/api/v3/projects/1", title: "Demo" } }, + }; + }, + } as unknown as OpenProjectClient; + } + + test("propagates an authentication failure from the agenda fetch", async () => { + await expect( + getMeeting(5, { includeAgendaItems: true }, clientFailingAgendaWith( + new OpenProjectAuthenticationError() + )) + ).rejects.toThrow(OpenProjectAuthenticationError); + }); + + test("propagates a rate limit from the agenda fetch", async () => { + await expect( + getMeeting(5, { includeAgendaItems: true }, clientFailingAgendaWith( + new OpenProjectError("rate limited", { statusCode: 429 }) + )) + ).rejects.toThrow("rate limited"); + }); + + test("still degrades to an empty agenda on a non-fatal failure", async () => { + const detail = await getMeeting( + 5, + { includeAgendaItems: true }, + clientFailingAgendaWith(new OpenProjectError("boom", { statusCode: 500 })) + ); + expect(detail.id).toBe(5); + expect(detail.agendaItems).toEqual([]); + }); +});