diff --git a/packages/mcp/src/tools/contracts.test.ts b/packages/mcp/src/tools/contracts.test.ts index 3000338d..98d37e6f 100644 --- a/packages/mcp/src/tools/contracts.test.ts +++ b/packages/mcp/src/tools/contracts.test.ts @@ -1,4 +1,4 @@ -import { FeatureNotEnabledError, type Terminal49Client } from '@terminal49/sdk'; +import { FeatureNotEnabledError, NotFoundError, type Terminal49Client } from '@terminal49/sdk'; import { describe, expect, it, vi } from 'vitest'; import { executeGetContainer } from './get-container.js'; import { executeGetContainerRoute } from './get-container-route.js'; @@ -669,23 +669,122 @@ describe('MCP tool contracts', () => { ); expect(events).toHaveBeenCalledWith('container-1', { format: 'raw' }); - expect(result.summary.total_events).toBe(2); - expect(result.summary.timeline[0]).toMatchObject({ + expect(result.total_events).toBe(2); + expect(result.timeline[0]).toMatchObject({ event: 'container.transport.vessel_loaded', }); - expect(result.summary.milestones).toMatchObject({ + expect(result.milestones).toMatchObject({ vessel_loaded_at: '2025-01-01T00:00:00.000Z', vessel_departed_at: '2025-01-02T00:00:00.000Z', }); + // A 200 from the sub-resource means the container exists; surface + // container_found consistently with the fallback path. + expect(result._metadata.source).toBe('transport_events_subresource'); + expect(result._metadata.container_found).toBe(true); + // No duplicate mapped payload should be surfaced. + expect(result.summary).toBeUndefined(); + expect(result.mapped).toBeUndefined(); }); - it('get_container_transport_events returns empty summary when events fetch fails', async () => { - const client = asClient({ - containers: { - events: vi.fn().mockRejectedValue(new Error('Not Found')), + it('get_container_transport_events falls back to the container include path when the dedicated sub-resource 404s', async () => { + const events = vi.fn().mockRejectedValue(new NotFoundError('Not Found')); + const get = vi.fn().mockResolvedValue({ + data: { + id: 'container-1', + type: 'container', + attributes: { number: 'CAIU1234567' }, + relationships: { + transport_events: { + data: [ + { id: 'evt-1', type: 'transport_event' }, + { id: 'evt-2', type: 'transport_event' }, + ], + }, + }, + }, + included: [ + { + id: 'evt-1', + type: 'transport_event', + attributes: { + event: 'container.transport.vessel_loaded', + timestamp: '2025-01-01T00:00:00Z', + // Canonical transport_event payloads carry location_locode (and a + // location relationship that is NOT side-loaded on the include + // path), not a location_name attribute. + location_locode: 'CNSHA', + }, + }, + { + id: 'evt-2', + type: 'transport_event', + attributes: { + event: 'container.transport.vessel_departed', + timestamp: '2025-01-02T00:00:00Z', + location_locode: 'CNSHA', + }, + }, + ], + }); + + const client = asClient({ containers: { events, get } }); + + const result = await executeGetContainerTransportEvents( + { id: 'container-1' }, + client, + ); + + expect(events).toHaveBeenCalledWith('container-1', { format: 'raw' }); + expect(get).toHaveBeenCalledWith('container-1', ['transport_events'], { + format: 'raw', + }); + // Fallback returns the real events, never a false-empty timeline. + expect(result.total_events).toBe(2); + expect(result.timeline[0]).toMatchObject({ + event: 'container.transport.vessel_loaded', + }); + // The movement location must survive the fallback even though the related + // port resource is not side-loaded — resolve it from location_locode. + expect(result.timeline[0].location).toMatchObject({ code: 'CNSHA' }); + expect(result.milestones).toMatchObject({ + vessel_loaded_at: '2025-01-01T00:00:00.000Z', + vessel_departed_at: '2025-01-02T00:00:00.000Z', + }); + expect(result._metadata.source).toBe('container_include_fallback'); + // It must NOT masquerade a success-shaped empty timeline carrying an error. + expect(result._metadata.error).toBeUndefined(); + }); + + it('get_container_transport_events surfaces a real error for a genuinely missing container', async () => { + const events = vi.fn().mockRejectedValue(new NotFoundError('Not Found')); + const get = vi.fn().mockRejectedValue(new NotFoundError('Not Found')); + + const client = asClient({ containers: { events, get } }); + + await expect( + executeGetContainerTransportEvents({ id: 'missing-container' }, client), + ).rejects.toBeInstanceOf(NotFoundError); + + expect(events).toHaveBeenCalledWith('missing-container', { format: 'raw' }); + expect(get).toHaveBeenCalledWith('missing-container', ['transport_events'], { + format: 'raw', + }); + }); + + it('get_container_transport_events returns an empty-but-valid timeline for a container with no events', async () => { + const events = vi.fn().mockRejectedValue(new NotFoundError('Not Found')); + const get = vi.fn().mockResolvedValue({ + data: { + id: 'container-1', + type: 'container', + attributes: { number: 'CAIU1234567' }, + relationships: { transport_events: { data: [] } }, }, + included: [], }); + const client = asClient({ containers: { events, get } }); + const result = await executeGetContainerTransportEvents( { id: 'container-1' }, client, @@ -693,9 +792,21 @@ describe('MCP tool contracts', () => { expect(result.total_events).toBe(0); expect(result.timeline).toEqual([]); - expect(result._metadata).toMatchObject({ - error: 'Not Found', - }); + // Empty-but-valid is distinct from a bad container id: no error metadata. + expect(result._metadata.error).toBeUndefined(); + expect(result._metadata.container_found).toBe(true); + }); + + it('get_container_transport_events does not fall back for non-NotFound failures', async () => { + const events = vi.fn().mockRejectedValue(new Error('boom')); + const get = vi.fn(); + + const client = asClient({ containers: { events, get } }); + + await expect( + executeGetContainerTransportEvents({ id: 'container-1' }, client), + ).rejects.toThrow('boom'); + expect(get).not.toHaveBeenCalled(); }); it('search_container handles partially missing fields safely', async () => { diff --git a/packages/mcp/src/tools/get-container-transport-events.ts b/packages/mcp/src/tools/get-container-transport-events.ts index bd94e75c..5bdae326 100644 --- a/packages/mcp/src/tools/get-container-transport-events.ts +++ b/packages/mcp/src/tools/get-container-transport-events.ts @@ -3,7 +3,7 @@ * Retrieves transport event timeline for a container */ -import { Terminal49Client } from '@terminal49/sdk'; +import { NotFoundError, Terminal49Client } from '@terminal49/sdk'; export interface GetContainerTransportEventsArgs { id: string; @@ -49,61 +49,131 @@ export async function executeGetContainerTransportEvents( try { const result = await client.containers.events(args.id, { format: 'raw' }); const raw = (result as any)?.raw ?? result; - const mapped = (result as any)?.mapped; - const duration = Date.now() - startTime; - - console.error( - JSON.stringify({ - event: 'tool.execute.complete', - tool: 'get_container_transport_events', - container_id: args.id, - event_count: raw?.data?.length || (Array.isArray(mapped) ? mapped.length : 0) || 0, - duration_ms: duration, - timestamp: new Date().toISOString(), - }) - ); - - const summary = formatTransportEventsResponse(raw); - return mapped ? { mapped, summary } : summary; + + logComplete(args.id, eventCount(raw), startTime, 'transport_events_subresource'); + + // A 200 from the dedicated sub-resource means the container exists, even + // when it carries zero events. Surface `container_found` consistently with + // the fallback path so an empty-but-valid primary timeline is never + // mistaken for a missing container. + return formatTransportEventsResponse(raw, { + source: 'transport_events_subresource', + containerFound: true, + }); } catch (error) { - const duration = Date.now() - startTime; - const message = (error as Error).message; - - console.error( - JSON.stringify({ - event: 'tool.execute.error', - tool: 'get_container_transport_events', - container_id: args.id, - error: (error as Error).name, - message, - duration_ms: duration, - timestamp: new Date().toISOString(), - }) - ); + if (!isNotFound(error)) { + logError(args.id, error, startTime); + throw error; + } - return { - total_events: 0, - event_categories: { - vessel_events: 0, - rail_events: 0, - truck_events: 0, - terminal_events: 0, - other_events: 0, - }, - timeline: [], - milestones: {}, - _metadata: { - presentation_guidance: - 'No transport events were returned for this container. Use get_container for current status and retry later.', - error: message, - remediation: - 'Confirm the container ID exists and has event data, then retry get_container_transport_events.', - }, - }; + // The dedicated /containers/{id}/transport_events sub-resource can 404 even + // when the container exists and has events (it is not enabled/populated for + // every container). Fall back to the container's include path before + // concluding there is nothing to show — never present a success-shaped empty + // timeline that secretly carries a "Not Found" error. + logFallback(args.id, error); + return fallbackToContainerInclude(args.id, client, startTime); } } -function formatTransportEventsResponse(apiResponse: any): any { +async function fallbackToContainerInclude( + id: string, + client: Terminal49Client, + startTime: number +): Promise { + let raw: any; + try { + const fallbackResult = await client.containers.get(id, ['transport_events'], { + format: 'raw', + }); + raw = (fallbackResult as any)?.raw ?? fallbackResult; + } catch (fallbackError) { + // A genuinely-missing container surfaces as a real tool error, distinct + // from an empty-but-valid timeline. + logError(id, fallbackError, startTime); + throw fallbackError; + } + + const events = extractIncludedTransportEvents(raw); + + logComplete(id, events.length, startTime, 'container_include_fallback'); + + return formatTransportEventsResponse( + { data: events, included: raw?.included || [] }, + { source: 'container_include_fallback', containerFound: true } + ); +} + +function eventCount(raw: any): number { + if (Array.isArray(raw)) return raw.length; + if (Array.isArray(raw?.data)) return raw.data.length; + return 0; +} + +function isNotFound(error: unknown): boolean { + return ( + error instanceof NotFoundError || + (error as any)?.status === 404 || + (error as any)?.name === 'NotFoundError' + ); +} + +function logComplete(id: string, count: number, startTime: number, source: string): void { + console.error( + JSON.stringify({ + event: 'tool.execute.complete', + tool: 'get_container_transport_events', + container_id: id, + event_count: count, + source, + duration_ms: Date.now() - startTime, + timestamp: new Date().toISOString(), + }) + ); +} + +function logFallback(id: string, error: unknown): void { + // The primary 404 is expected (the sub-resource is not enabled for every + // container), so it is not surfaced as an error — but operators + // investigating fallback traffic need a signal to correlate against. + console.error( + JSON.stringify({ + event: 'tool.execute.fallback', + tool: 'get_container_transport_events', + container_id: id, + reason: 'transport_events_subresource_not_found', + error: (error as Error).name, + message: (error as Error).message, + timestamp: new Date().toISOString(), + }) + ); +} + +function logError(id: string, error: unknown, startTime: number): void { + console.error( + JSON.stringify({ + event: 'tool.execute.error', + tool: 'get_container_transport_events', + container_id: id, + error: (error as Error).name, + message: (error as Error).message, + duration_ms: Date.now() - startTime, + timestamp: new Date().toISOString(), + }) + ); +} + +function extractIncludedTransportEvents(raw: any): any[] { + const included = Array.isArray(raw?.included) ? raw.included : []; + return included.filter((item: any) => item?.type === 'transport_event'); +} + +interface FormatOptions { + source: 'transport_events_subresource' | 'container_include_fallback'; + containerFound?: boolean; +} + +function formatTransportEventsResponse(apiResponse: any, options: FormatOptions): any { const events = Array.isArray(apiResponse) ? apiResponse : Array.isArray(apiResponse?.data) @@ -128,40 +198,73 @@ function formatTransportEventsResponse(apiResponse: any): any { const eventType = normalizeText(attrs.event); const timestamp = normalizeTimestamp(attrs.timestamp); - // Find location info from included data - const locationId = relationships.location?.data?.id; - const location = included.find((item: any) => item.id === locationId); - return { event: eventType, timestamp, timezone: normalizeText(attrs.timezone), voyage_number: normalizeText(attrs.voyage_number), - location: location - ? { - name: normalizeText(location.attributes?.name), - code: - normalizeText(location.attributes?.code) || normalizeText(location.attributes?.locode), - type: normalizeText(location.type), - } - : null, + location: resolveLocation(attrs, relationships, included), }; }); + const metadata: Record = { + source: options.source, + presentation_guidance: + events.length > 0 + ? 'Present events chronologically as a journey timeline. ' + + 'Highlight key milestones: vessel loaded, departed, arrived, discharged, delivery. ' + + 'For rail containers, emphasize rail movements.' + : 'This container exists but has no transport events yet. ' + + 'Report an empty timeline (not an error) and use get_container for current status.', + }; + + if (options.containerFound !== undefined) { + metadata.container_found = options.containerFound; + } + return { total_events: events.length, event_categories: categorized, timeline: formattedEvents, milestones: extractKeyMilestones(sortedEvents), - _metadata: { - presentation_guidance: - 'Present events chronologically as a journey timeline. ' + - 'Highlight key milestones: vessel loaded, departed, arrived, discharged, delivery. ' + - 'For rail containers, emphasize rail movements.', - }, + _metadata: metadata, }; } +function resolveLocation(attrs: any, relationships: any, included: any[]): any { + // Dedicated sub-resource: location is a related resource in `included`. + const locationId = relationships.location?.data?.id; + const includedLocation = locationId + ? included.find((item: any) => item.id === locationId) + : undefined; + if (includedLocation) { + return { + name: normalizeText(includedLocation.attributes?.name), + code: + normalizeText(includedLocation.attributes?.code) || + normalizeText(includedLocation.attributes?.locode), + type: normalizeText(includedLocation.type), + }; + } + + // Container-include fallback: the related port/metro_area resource is not + // side-loaded, so resolve from the event's own attributes. Canonical + // transport_event payloads carry `location_locode` (and usually no + // `location_name`), so surface a location whenever either is present rather + // than dropping the movement location entirely. + const embeddedName = normalizeText(attrs.location_name); + const embeddedCode = normalizeText(attrs.location_locode) || normalizeText(attrs.port_locode); + if (embeddedName || embeddedCode) { + return { + name: embeddedName, + code: embeddedCode, + type: undefined, + }; + } + + return null; +} + function categorizeEvents(events: any[]): any { const categories = { vessel: [] as any[],