fix: directions_tool turn-by-turn instructions never populated

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Unreleased
 
+### Fixed
+
+- **`directions_tool`: turn-by-turn `instructions` now actually populate.** The tool has always requested `steps=true` and documented its response as including "turn-by-turn instructions," but the extraction code read announcements from `step.voiceInstructions`, which the Directions API only returns when the separate `voice_instructions=true` parameter is set — never sent by this tool. So `instructions` was silently empty for every call, regardless of route length. Switched to `step.maneuver.instruction` (e.g. `"Bear right onto Great Portland Street/A4201. Continue on A4201."`), a human-readable instruction already present on every step whenever `steps=true` is set, no extra parameter required. Also raised the output cap from 10 to 200: the old cap was sized for the old (broken) source, which could emit several announcements per turn; `maneuver.instruction` is one entry per real turn, so a normal route — even a long one with dozens of turns — stays well under the new cap. Confirmed live against the real Directions API.
+
 ### New Features
 
 - **The server now identifies which MCP client connected to it.** `server.server.getClientVersion()` (populated from the `clientInfo` sent in the client's `initialize` request) is now logged on connect, e.g. `Client identified as: claude-ai v1.0.0` — useful for support/debugging when behavior differs across Claude Desktop, Cursor, VS Code, etc. Reading it required moving the read to a `server.server.oninitialized` callback rather than right after `server.connect()`, since `getClientVersion()`/`getClientCapabilities()` are only populated once the client's `initialize` request has actually been processed, which is not guaranteed by the time `connect()`'s promise resolves (it only waits for the transport to start). Confirmed live that reading capabilities immediately after `connect()` reliably returned `undefined` even for a client that declared them; moving both reads into `oninitialized` fixed the same-shaped bug in the existing capability-gated tool registration (currently dormant, since no tool is registered through that path yet, but was silently broken for whenever one is added). The client name/version is also recorded as `mcp.client.name`/`mcp.client.version` on every subsequent tool-execution trace span, so OTel-backed traces can be filtered or grouped by client.

src/tools/directions-tool/cleanResponseData.ts

@@ -42,13 +42,13 @@ interface RawIncident {
   [key: string]: unknown;
 }
 
-interface RawVoiceInstruction {
-  announcement?: string;
+interface RawManeuver {
+  instruction?: string;
   [key: string]: unknown;
 }
 
 interface RawStep {
-  voiceInstructions?: RawVoiceInstruction[];
+  maneuver?: RawManeuver;
   [key: string]: unknown;
 }
 
@@ -200,8 +200,8 @@ export function cleanResponseData(
       length?: number;
     }> = [];
 
-    // Collect voice instruction announcements from all steps
-    const routeAnnouncements: string[] = [];
+    // Collect turn-by-turn instructions from all steps
+    const routeInstructions: string[] = [];
 
     let totalDistanceWeightedSpeed = 0; // Sum of (speed × distance) for each segment
     let sumDistanceMeters = 0;
@@ -284,15 +284,17 @@ export function cleanResponseData(
           });
         }
 
-        // Process steps if they exist to collect voice instructions
+        // Process steps if they exist to collect turn-by-turn instructions.
+        // `maneuver.instruction` (e.g. "Bear right onto Great Portland
+        // Street/A4201") is present on every step whenever steps=true is
+        // requested, unlike `step.voiceInstructions`, which requires the
+        // separate voice_instructions=true parameter -- never sent by this
+        // tool, so that field was always empty and `instructions` never
+        // actually populated.
         if (leg.steps) {
           leg.steps.forEach((step) => {
-            if (step.voiceInstructions) {
-              step.voiceInstructions.forEach((instruction) => {
-                if (instruction.announcement) {
-                  routeAnnouncements.push(instruction.announcement);
-                }
-              });
+            if (step.maneuver?.instruction) {
+              routeInstructions.push(step.maneuver.instruction);
             }
           });
         }
@@ -312,10 +314,18 @@ export function cleanResponseData(
     // Add all incidents with the specified fields as a new property on the route
     cleanedRoute.incidents_summary = routeIncidents;
 
-    // Add voice instruction announcements only if there are 1 to 10 of them
-    // If there are more than 10, it's just too many, and if there is 0 then we don't have them.
-    if (routeAnnouncements.length >= 1 && routeAnnouncements.length <= 10) {
-      cleanedRoute.instructions = routeAnnouncements;
+    // One instruction per real maneuver (unlike the old voice-announcement
+    // source, which could repeat several announcements per turn), so a
+    // normal route -- even a long one with dozens of turns -- stays well
+    // within a generous cap. The cap here is just a backstop against a
+    // pathological case (e.g. many waypoints each with a complex urban leg),
+    // not a limit expected to bite in practice.
+    const MAX_INSTRUCTIONS = 200;
+    if (
+      routeInstructions.length >= 1 &&
+      routeInstructions.length <= MAX_INSTRUCTIONS
+    ) {
+      cleanedRoute.instructions = routeInstructions;
     }
 
     cleanedRoute.num_legs = route.legs?.length || 0;

test/tools/directions-tool/cleanResponseData.test.ts

@@ -213,26 +213,22 @@ describe('cleanResponseData', () => {
     expect(result.routes[0].incidents_summary[0].extra_field).toBeUndefined();
   });
 
-  it('should collect voice instructions when within limits', () => {
+  it('should collect turn-by-turn instructions from step.maneuver.instruction', () => {
     const mockData = {
       routes: [
         {
           legs: [
             {
               steps: [
+                { maneuver: { instruction: 'Drive northeast on Main St.' } },
                 {
-                  voiceInstructions: [
-                    { announcement: 'Turn right in 100 meters' },
-                    { announcement: 'Turn right now' }
-                  ]
+                  maneuver: {
+                    instruction: 'Bear right onto Elm St. Continue on Elm St.'
+                  }
                 },
-                {
-                  voiceInstructions: [
-                    { announcement: 'Continue straight for 500 meters' }
-                  ]
-                }
+                { maneuver: { instruction: 'Arrive at your destination.' } }
               ],
-              summary: 'Leg with voice instructions'
+              summary: 'Leg with turn-by-turn instructions'
             }
           ]
         }
@@ -242,27 +238,72 @@ describe('cleanResponseData', () => {
     const result = cleanResponseData(mockInput, mockData);
 
     expect(result.routes[0].instructions).toEqual([
-      'Turn right in 100 meters',
-      'Turn right now',
-      'Continue straight for 500 meters'
+      'Drive northeast on Main St.',
+      'Bear right onto Elm St. Continue on Elm St.',
+      'Arrive at your destination.'
     ]);
   });
 
-  it('should not include instructions when there are too many', () => {
+  it('ignores steps with no maneuver.instruction', () => {
+    const mockData = {
+      routes: [
+        {
+          legs: [
+            {
+              steps: [
+                { maneuver: { instruction: 'Drive north on Main St.' } },
+                { maneuver: {} },
+                {}
+              ],
+              summary: 'Leg with a step missing an instruction'
+            }
+          ]
+        }
+      ]
+    };
+
+    const result = cleanResponseData(mockInput, mockData);
+
+    expect(result.routes[0].instructions).toEqual(['Drive north on Main St.']);
+  });
+
+  it('still includes turn-by-turn instructions for a long route with dozens of real turns', () => {
+    // Mirrors a real long driving route (confirmed live: London->Edinburgh
+    // has ~40 steps in a single leg) -- well within the new cap, unlike the
+    // old 10-instruction cap (calibrated for a different, always-empty
+    // voice-announcement source) which would have excluded this outright.
+    const mockData = {
+      routes: [
+        {
+          legs: [
+            {
+              steps: Array.from({ length: 40 }, (_, i) => ({
+                maneuver: { instruction: `Turn ${i + 1}` }
+              })),
+              summary: 'Leg with many real turns'
+            }
+          ]
+        }
+      ]
+    };
+
+    const result = cleanResponseData(mockInput, mockData);
+
+    expect(result.routes[0].instructions).toHaveLength(40);
+    expect(result.routes[0].instructions?.[0]).toBe('Turn 1');
+    expect(result.routes[0].instructions?.[39]).toBe('Turn 40');
+  });
+
+  it('excludes instructions entirely past the pathological-case cap', () => {
     const mockData = {
       routes: [
         {
           legs: [
             {
-              steps: Array(6)
-                .fill(0)
-                .map(() => ({
-                  voiceInstructions: [
-                    { announcement: 'Instruction 1' },
-                    { announcement: 'Instruction 2' }
-                  ]
-                })),
-              summary: 'Leg with many instructions'
+              steps: Array.from({ length: 201 }, (_, i) => ({
+                maneuver: { instruction: `Turn ${i + 1}` }
+              })),
+              summary: 'Leg with an unreasonable number of turns'
             }
           ]
         }
@@ -271,8 +312,6 @@ describe('cleanResponseData', () => {
 
     const result = cleanResponseData(mockInput, mockData);
 
-    // With 6 steps and 2 instructions each, we'd have 12 instructions total
-    // The function should exclude them since it's > 10
     expect(result.routes[0].instructions).toBeUndefined();
   });