Skip to content
Merged
16 changes: 8 additions & 8 deletions chatgpt-app-submission.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,23 +135,23 @@
"user_prompt": "Find the synthetic review fixture container CAIU1234567 in my Terminal49 account and summarize its current status.",
"file_attachment_urls": null,
"tools_triggered": "search_container",
"expected_output": "Returns matching container records with identifiers, status, shipping line, and available terminal information.",
"expected_output": "If the fixture exists in the connected account, returns matching container identifiers, status, shipping line, and available terminal information. Otherwise, clearly reports zero matches without inventing shipment data.",
"expected_output_url": null
},
{
"description": "List recently updated containers with a bounded result set.",
"user_prompt": "Show the first 10 containers updated recently in my Terminal49 account.",
"description": "List a bounded page of containers.",
"user_prompt": "Show the first 10 containers in my Terminal49 account.",
"file_attachment_urls": null,
"tools_triggered": "list_containers",
"expected_output": "Returns up to 10 recent container records and clearly indicates any pagination or result limits.",
"expected_output": "Returns up to 10 container records, clearly labels the page as unfiltered, and indicates pagination or result limits.",
"expected_output_url": null
},
{
"description": "List shipments filtered by carrier.",
"user_prompt": "Show my recent Maersk shipments and include their containers.",
"description": "List actively tracked shipments without nested containers.",
"user_prompt": "Show the first 10 shipments where shipping-line tracking has not stopped. Do not include nested containers.",
"file_attachment_urls": null,
"tools_triggered": "list_shipments",
"expected_output": "Returns shipments matching the carrier filter with available container relationships and pagination details.",
"expected_output": "Returns up to 10 shipments matching tracking_stopped=false without nested container relationships, with pagination details.",
"expected_output_url": null
},
{
Expand All @@ -167,7 +167,7 @@
"user_prompt": "Track the synthetic review fixture container CAIU1234567 with carrier SCAC MAEU in my Terminal49 account.",
"file_attachment_urls": null,
"tools_triggered": "track_container",
"expected_output": "Returns the existing matching container or creates a tracking request and clearly reports whether a new request was created.",
"expected_output": "Returns the existing matching container, creates a tracking request, or clearly reports that no request was created because the number/carrier could not be resolved. It must not claim the fixture exists or is pending unless the account response confirms that state.",
"expected_output_url": null
}
],
Expand Down
160 changes: 158 additions & 2 deletions packages/mcp/src/annotations.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { readFileSync } from 'node:fs';
import { describe, expect, it, vi } from 'vite-plus/test';
import { createTerminal49McpServer } from './server.js';

Expand All @@ -15,14 +16,72 @@ type ToolAnnotations = {
openWorldHint?: boolean;
};

type ChatGptSubmission = {
$schema: string;
schema_version: number;
app_info: {
display_name: string;
subtitle: string;
description: string;
category: string;
};
tools: Record<
string,
{
annotations: ToolAnnotations;
justifications: Record<string, string>;
}
>;
test_cases: Array<{
description: string;
user_prompt: string;
file_attachment_urls: string[] | null;
tools_triggered: string;
expected_output: string;
expected_output_url: string | null;
}>;
negative_test_cases: Array<{
description: string;
user_prompt: string;
file_attachment_urls: string[] | null;
tools_triggered: null;
expected_output: string;
expected_output_url: string | null;
}>;
};

type ClaudeSubmission = {
server: {
url: string;
transport: string;
url_type: string;
authentication: string;
};
listing: {
name: string;
tagline: string;
documentation_url: string;
privacy_policy_url: string;
terms_of_service_url: string;
support_email: string;
icon: string;
icon_dark: string;
};
capabilities: {
reads_data: boolean;
writes_data: boolean;
primary_use_cases: string[];
};
};

function getRegisteredTools(): Record<
string,
{ annotations?: ToolAnnotations }
{ title?: string; annotations?: ToolAnnotations }
> {
const server = createTerminal49McpServer('token');
return (server as any)._registeredTools as Record<
string,
{ annotations?: ToolAnnotations }
{ title?: string; annotations?: ToolAnnotations }
>;
}

Expand Down Expand Up @@ -88,7 +147,104 @@ describe('MCP tool annotations', () => {
).toBeGreaterThanOrEqual(allTools.length);

for (const [name, tool] of Object.entries(tools)) {
expect(tool.title, `${name}.title`).toEqual(expect.any(String));
expect(tool.title?.trim().length, `${name}.title`).toBeGreaterThan(0);
expect(tool.annotations, name).toBeDefined();
}
});

it('keeps live annotations and locked store listings consistent', () => {
const tools = getRegisteredTools();
const chatGpt = JSON.parse(
readFileSync(
new URL('../../../chatgpt-app-submission.json', import.meta.url),
'utf8',
),
) as ChatGptSubmission;
const claude = JSON.parse(
readFileSync(
new URL('../../../claude-connector-submission.json', import.meta.url),
'utf8',
),
) as ClaudeSubmission;

expect(Object.keys(chatGpt.tools).sort()).toEqual(
Object.keys(tools).sort(),
);
for (const [name, tool] of Object.entries(tools)) {
const liveAnnotations = tool.annotations;
expect(chatGpt.tools[name]?.annotations, name).toMatchObject({
readOnlyHint: liveAnnotations?.readOnlyHint,
destructiveHint: liveAnnotations?.destructiveHint,
openWorldHint: liveAnnotations?.openWorldHint,
});
expect(
Object.values(chatGpt.tools[name]?.justifications ?? {}).every(
(justification) => justification.trim().length > 0,
),
`${name}.justifications`,
).toBe(true);
expect(
Object.keys(chatGpt.tools[name]?.justifications ?? {}),
`${name}.justifications`,
).toHaveLength(3);
}

expect(chatGpt.$schema).toBe(
'https://developers.openai.com/apps-sdk/schemas/chatgpt-app-submission.v1.json',
);
expect(chatGpt.schema_version).toBe(1);
expect(chatGpt.app_info).toMatchObject({
display_name: 'Terminal49',
subtitle: 'Track ocean shipments',
category: 'BUSINESS',
});
expect(chatGpt.app_info.description).toContain('Terminal49 helps users');
expect(chatGpt.test_cases).toHaveLength(5);
expect(chatGpt.negative_test_cases).toHaveLength(3);
expect(chatGpt.test_cases[0]?.expected_output).toContain(
'Otherwise, clearly reports zero matches',
);
expect(chatGpt.test_cases[4]?.expected_output).toContain(
'no request was created',
);
for (const testCase of chatGpt.test_cases) {
expect(testCase.description.trim()).not.toBe('');
expect(testCase.user_prompt.trim()).not.toBe('');
expect(testCase.tools_triggered).toBeTypeOf('string');
expect(testCase.expected_output.trim()).not.toBe('');
expect(testCase.file_attachment_urls).toBeNull();
expect(testCase.expected_output_url).toBeNull();
}
for (const testCase of chatGpt.negative_test_cases) {
expect(testCase.description.trim()).not.toBe('');
expect(testCase.user_prompt.trim()).not.toBe('');
expect(testCase.tools_triggered).toBeNull();
expect(testCase.expected_output.trim()).not.toBe('');
expect(testCase.file_attachment_urls).toBeNull();
expect(testCase.expected_output_url).toBeNull();
}
expect(claude.server).toMatchObject({
url: 'https://mcp.terminal49.com',
transport: 'streamable-http',
url_type: 'universal',
authentication: 'oauth',
});
expect(claude.listing).toMatchObject({
name: 'Terminal49',
tagline: 'Track ocean shipments',
documentation_url: 'https://docs.terminal49.com/mcp/home',
privacy_policy_url: 'https://terminal49.com/privacy',
terms_of_service_url: 'https://terminal49.com/terms',
support_email: 'support@terminal49.com',
});
expect(claude.listing.icon).toMatch(/terminal49-light\.png$/);
expect(claude.listing.icon_dark).toMatch(/terminal49-dark\.png$/);
expect(claude.listing.tagline.length).toBeLessThanOrEqual(55);
expect(claude.capabilities).toMatchObject({
reads_data: true,
writes_data: true,
});
expect(claude.capabilities.primary_use_cases.length).toBeGreaterThan(0);
});
});
31 changes: 23 additions & 8 deletions packages/mcp/src/mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@ vi.mock('@sentry/node', () => ({
// Stubbed Terminal49Client so server tools can be exercised end-to-end without
// hitting the live API. Tests configure these mocks per-case. `vi.hoisted`
// is required because vi.mock factories are hoisted above normal declarations.
const { shippingLinesList, containersList, shipmentsList } = vi.hoisted(() => ({
shippingLinesList: vi.fn(),
containersList: vi.fn(),
shipmentsList: vi.fn(),
}));
const { search, shippingLinesList, containersList, shipmentsList } = vi.hoisted(
() => ({
search: vi.fn(),
shippingLinesList: vi.fn(),
containersList: vi.fn(),
shipmentsList: vi.fn(),
}),
);

vi.mock('@terminal49/sdk', () => ({
Terminal49Client: class Terminal49Client {
search = search;
shippingLines = { list: shippingLinesList };
containers = { list: containersList };
shipments = { list: shipmentsList };
Expand All @@ -37,6 +41,7 @@ vi.mock('@terminal49/sdk', () => ({
}));

beforeEach(() => {
search.mockReset();
shippingLinesList.mockReset();
containersList.mockReset();
shipmentsList.mockReset();
Expand Down Expand Up @@ -476,14 +481,24 @@ describe('MCP server wiring', () => {
it('captures handled tool errors when Sentry is initialized', async () => {
const Sentry = await import('@sentry/node');
vi.mocked(Sentry.isInitialized).mockReturnValue(true);
search.mockRejectedValue(
new Error(
'upstream failed at https://internal.example/v2?token=secret-token',
),
);

const server = createTerminal49McpServer('token');
const searchTool = (server as any)._registeredTools.search_container;

const result = await searchTool.handler({ query: ' ' });
const result = await searchTool.handler({ query: 'CAIU1234567' });

expect(result.isError).toBe(true);
expect(Sentry.captureException).toHaveBeenCalledWith(expect.any(Error));
const captured = vi.mocked(Sentry.captureException).mock.calls[0]?.[0];
expect(captured).toBeInstanceOf(Error);
expect((captured as Error).message).not.toContain('internal.example');
expect((captured as Error).message).not.toContain('secret-token');
expect((captured as Error).message).toContain('could not be completed');
expect(Sentry.flush).toHaveBeenCalledWith(2000);
});

Expand Down Expand Up @@ -622,7 +637,7 @@ describe('MCP server wiring', () => {
{
name: 'list_shipments',
args: {
carrier: 'MAEU',
tracking_stopped: false,
include_containers: true,
page: 1,
page_size: 10,
Expand All @@ -646,7 +661,7 @@ describe('MCP server wiring', () => {
self: 'https://api.test/shipments?page[number]=1&page[size]=10',
},
meta: { total: 1 },
unsupportedFilters: ['carrier'],
unsupportedFilters: [],
},
},
])(
Expand Down
10 changes: 7 additions & 3 deletions packages/mcp/src/resources/query-guidance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ export function readQueryGuidanceResource(): string {
' - "Which containers have been discharged but not picked up?"',
' - "Any holds on [X]?"',
'- Primary tools: list_containers or list_shipments then get_container',
'- Supported list filters: status, port, carrier, updated_after (no has_hold filter exists).',
'- "Discharged but not picked up" is derived client-side: keep rows where podDischargedAt is set and podFullOutAt is empty. Hold state comes from the holdsAtPodTerminal field on each row, not a filter.',
'- list_containers has no server-side operational filters. Paginate and inspect returned rows; do not describe one page as a complete account-wide worklist.',
'- "Discharged but not picked up" is derived client-side from returned rows: keep rows where podDischargedAt is set and podFullOutAt is empty. Hold state comes from holdsAtPodTerminal, not a filter.',
'',
'### 4) Arrival / ETAs / delays',
'- Question examples:',
Expand All @@ -65,7 +65,11 @@ export function readQueryGuidanceResource(): string {
' - "Do I have any containers with demurrage risk?"',
' - "Which containers are at risk of LFD?"',
'- Primary tool: list_containers plus get_container',
'- Supported list filters: status, port, carrier, updated_after. The list endpoint has no server-side sort; order rows client-side by the pickupLfd field returned on each container, and surface holdsAtPodTerminal alongside it.',
'- list_containers has no server-side filter or sort. Order each returned page client-side by pickupLfd and surface holdsAtPodTerminal, but do not claim the page represents every at-risk container in the account.',
'',
'### 7) Shipment list filtering',
'- list_shipments supports shipment number and tracking_stopped filters.',
'- Status, port, carrier, and updated-time filters are not supported by the API and must not be presented as applied.',
'',
'## Output Formatting Guidance',
'',
Expand Down
9 changes: 8 additions & 1 deletion packages/mcp/src/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,14 @@ export function instrumentMcpServer<TServer extends McpServer>(

export function captureMcpException(error: unknown): void {
if (Sentry.isInitialized()) {
Sentry.captureException(error);
const safeError = new Error(
'The Terminal49 upstream request could not be completed.',
);
const name = error instanceof Error ? error.name : 'Error';
safeError.name = /^[A-Za-z][A-Za-z0-9_.:-]{0,63}$/.test(name)
? name
: 'Error';
Sentry.captureException(safeError);
Comment on lines +97 to +104

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Generic errors discard diagnostics

captureMcpException replaces every caught tool exception with a new generic Error, so Sentry loses the original call-site stack and cause chain and groups distinct upstream failures at this sanitizer, making production incidents harder to diagnose.

Knowledge Base Used: MCP Server Core (@terminal49/mcp)

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/mcp/src/sentry.ts
Line: 97-104

Comment:
**Generic errors discard diagnostics**

`captureMcpException` replaces every caught tool exception with a new generic `Error`, so Sentry loses the original call-site stack and cause chain and groups distinct upstream failures at this sanitizer, making production incidents harder to diagnose.

**Knowledge Base Used:** [MCP Server Core (`@terminal49/mcp`)](https://app.greptile.com/terminal49/-/custom-context/knowledge-base/terminal49/api/-/docs/mcp-server-core.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code

Comment on lines +97 to +104

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing every exception with a newly constructed Error discards the original stack, cause, HTTP status, and sanitized SDK details. The new stack always points to captureMcpException(), so failures with the same error name become effectively indistinguishable in Sentry. Since stderr logging also redacts message, operators no longer have enough context to identify the failing request path or cause. Redact sensitive fields in a Sentry beforeSend hook, or construct a sanitized exception that preserves safe stack frames and diagnostic tags such as status/tool/operation.

}
}

Expand Down
Loading
Loading