Sort toolsnaps JSON keys recursively to eliminate noop churn

internal/toolsnaps/toolsnaps.go

@@ -67,15 +67,35 @@ func Test(toolName string, tool any) error {
 }
 
 func writeSnap(snapPath string, contents []byte) error {
+	// Sort the JSON keys recursively to ensure consistent output.
+	// We do this by unmarshaling and remarshaling, which ensures Go's JSON encoder
+	// sorts all map keys alphabetically at every level.
+	sortedJSON, err := sortJSONKeys(contents)
+	if err != nil {
+		return fmt.Errorf("failed to sort JSON keys: %w", err)
+	}
+
 	// Ensure the directory exists
 	if err := os.MkdirAll(filepath.Dir(snapPath), 0700); err != nil {
 		return fmt.Errorf("failed to create snapshot directory: %w", err)
 	}
 
 	// Write the snapshot file
-	if err := os.WriteFile(snapPath, contents, 0600); err != nil {
+	if err := os.WriteFile(snapPath, sortedJSON, 0600); err != nil {
 		return fmt.Errorf("failed to write snapshot file: %w", err)
 	}
 
 	return nil
 }
+
+// sortJSONKeys recursively sorts all object keys in a JSON byte array by
+// unmarshaling to map[string]any and remarshaling. Go's JSON encoder
+// automatically sorts map keys alphabetically.
+func sortJSONKeys(jsonData []byte) ([]byte, error) {
+	var data any
+	if err := json.Unmarshal(jsonData, &data); err != nil {
+		return nil, err
+	}
+
+	return json.MarshalIndent(data, "", "  ")
+}

internal/toolsnaps/toolsnaps_test.go

@@ -131,3 +131,124 @@ func TestMalformedSnapshotJSON(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "failed to parse snapshot JSON for dummy", "expected error about malformed snapshot JSON")
 }
+
+func TestSortJSONKeys(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "simple object",
+			input:    `{"z": 1, "a": 2, "m": 3}`,
+			expected: "{\n  \"a\": 2,\n  \"m\": 3,\n  \"z\": 1\n}",
+		},
+		{
+			name:     "nested object",
+			input:    `{"z": {"y": 1, "x": 2}, "a": 3}`,
+			expected: "{\n  \"a\": 3,\n  \"z\": {\n    \"x\": 2,\n    \"y\": 1\n  }\n}",
+		},
+		{
+			name:     "array with objects",
+			input:    `{"items": [{"z": 1, "a": 2}, {"y": 3, "b": 4}]}`,
+			expected: "{\n  \"items\": [\n    {\n      \"a\": 2,\n      \"z\": 1\n    },\n    {\n      \"b\": 4,\n      \"y\": 3\n    }\n  ]\n}",
+		},
+		{
+			name:     "deeply nested",
+			input:    `{"z": {"y": {"x": 1, "a": 2}, "b": 3}, "m": 4}`,
+			expected: "{\n  \"m\": 4,\n  \"z\": {\n    \"b\": 3,\n    \"y\": {\n      \"a\": 2,\n      \"x\": 1\n    }\n  }\n}",
+		},
+		{
+			name:     "properties field like in toolsnaps",
+			input:    `{"name": "test", "properties": {"repo": {"type": "string"}, "owner": {"type": "string"}, "page": {"type": "number"}}}`,
+			expected: "{\n  \"name\": \"test\",\n  \"properties\": {\n    \"owner\": {\n      \"type\": \"string\"\n    },\n    \"page\": {\n      \"type\": \"number\"\n    },\n    \"repo\": {\n      \"type\": \"string\"\n    }\n  }\n}",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := sortJSONKeys([]byte(tt.input))
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, string(result))
+		})
+	}
+}
+
+func TestSortJSONKeysIdempotent(t *testing.T) {
+	// Given a JSON string that's already sorted
+	input := `{"a": 1, "b": {"x": 2, "y": 3}, "c": [{"m": 4, "n": 5}]}`
+
+	// When we sort it once
+	sorted1, err := sortJSONKeys([]byte(input))
+	require.NoError(t, err)
+
+	// And sort it again
+	sorted2, err := sortJSONKeys(sorted1)
+	require.NoError(t, err)
+
+	// Then the results should be identical
+	assert.Equal(t, string(sorted1), string(sorted2))
+}
+
+func TestToolSnapKeysSorted(t *testing.T) {
+	withIsolatedWorkingDir(t)
+
+	// Given a tool with fields that could be in any order
+	type complexTool struct {
+		Name        string                 `json:"name"`
+		Description string                 `json:"description"`
+		Properties  map[string]interface{} `json:"properties"`
+		Annotations map[string]interface{} `json:"annotations"`
+	}
+
+	tool := complexTool{
+		Name:        "test_tool",
+		Description: "A test tool",
+		Properties: map[string]interface{}{
+			"zzz":   "last",
+			"aaa":   "first",
+			"mmm":   "middle",
+			"owner": map[string]interface{}{"type": "string", "description": "Owner"},
+			"repo":  map[string]interface{}{"type": "string", "description": "Repo"},
+		},
+		Annotations: map[string]interface{}{
+			"readOnly": true,
+			"title":    "Test",
+		},
+	}
+
+	// When we write the snapshot
+	t.Setenv("UPDATE_TOOLSNAPS", "true")
+	err := Test("complex", tool)
+	require.NoError(t, err)
+
+	// Then the snapshot file should have sorted keys
+	snapJSON, err := os.ReadFile("__toolsnaps__/complex.snap")
+	require.NoError(t, err)
+
+	// Verify that the JSON is properly sorted by checking key order
+	var parsed map[string]interface{}
+	err = json.Unmarshal(snapJSON, &parsed)
+	require.NoError(t, err)
+
+	// Check that properties are sorted
+	propsJSON, _ := json.MarshalIndent(parsed["properties"], "", "  ")
+	propsStr := string(propsJSON)
+	// The properties should have "aaa" before "mmm" before "zzz"
+	aaaIndex := -1
+	mmmIndex := -1
+	zzzIndex := -1
+	for i, line := range propsStr {
+		if line == 'a' && i+2 < len(propsStr) && propsStr[i:i+3] == "aaa" {
+			aaaIndex = i
+		}
+		if line == 'm' && i+2 < len(propsStr) && propsStr[i:i+3] == "mmm" {
+			mmmIndex = i
+		}
+		if line == 'z' && i+2 < len(propsStr) && propsStr[i:i+3] == "zzz" {
+			zzzIndex = i
+		}
+	}
+	assert.Greater(t, mmmIndex, aaaIndex, "mmm should come after aaa")
+	assert.Greater(t, zzzIndex, mmmIndex, "zzz should come after mmm")
+}

pkg/github/__toolsnaps__/actions_get.snap

@@ -5,16 +5,8 @@
   },
   "description": "Get details about specific GitHub Actions resources.\nUse this tool to get details about individual workflows, workflow runs, jobs, and artifacts by their unique IDs.\n",
   "inputSchema": {
-    "type": "object",
-    "required": [
-      "method",
-      "owner",
-      "repo",
-      "resource_id"
-    ],
     "properties": {
       "method": {
-        "type": "string",
         "description": "The method to execute",
         "enum": [
           "get_workflow",
@@ -23,21 +15,29 @@
           "download_workflow_run_artifact",
           "get_workflow_run_usage",
           "get_workflow_run_logs_url"
-        ]
+        ],
+        "type": "string"
       },
       "owner": {
-        "type": "string",
-        "description": "Repository owner"
+        "description": "Repository owner",
+        "type": "string"
       },
       "repo": {
-        "type": "string",
-        "description": "Repository name"
+        "description": "Repository name",
+        "type": "string"
       },
       "resource_id": {
-        "type": "string",
-        "description": "The unique identifier of the resource. This will vary based on the \"method\" provided, so ensure you provide the correct ID:\n- Provide a workflow ID or workflow file name (e.g. ci.yaml) for 'get_workflow' method.\n- Provide a workflow run ID for 'get_workflow_run', 'get_workflow_run_usage', and 'get_workflow_run_logs_url' methods.\n- Provide an artifact ID for 'download_workflow_run_artifact' method.\n- Provide a job ID for 'get_workflow_job' method.\n"
+        "description": "The unique identifier of the resource. This will vary based on the \"method\" provided, so ensure you provide the correct ID:\n- Provide a workflow ID or workflow file name (e.g. ci.yaml) for 'get_workflow' method.\n- Provide a workflow run ID for 'get_workflow_run', 'get_workflow_run_usage', and 'get_workflow_run_logs_url' methods.\n- Provide an artifact ID for 'download_workflow_run_artifact' method.\n- Provide a job ID for 'get_workflow_job' method.\n",
+        "type": "string"
       }
-    }
+    },
+    "required": [
+      "method",
+      "owner",
+      "repo",
+      "resource_id"
+    ],
+    "type": "object"
   },
   "name": "actions_get"
 }

pkg/github/__toolsnaps__/actions_list.snap

@@ -5,68 +5,66 @@
   },
   "description": "Tools for listing GitHub Actions resources.\nUse this tool to list workflows in a repository, or list workflow runs, jobs, and artifacts for a specific workflow or workflow run.\n",
   "inputSchema": {
-    "type": "object",
     "properties": {
       "method": {
-        "type": "string",
         "description": "The action to perform",
         "enum": [
           "list_workflows",
           "list_workflow_runs",
           "list_workflow_jobs",
           "list_workflow_run_artifacts"
-        ]
+        ],
+        "type": "string"
       },
       "owner": {
-        "type": "string",
-        "description": "Repository owner"
+        "description": "Repository owner",
+        "type": "string"
       },
       "page": {
-        "type": "number",
         "description": "Page number for pagination (default: 1)",
-        "minimum": 1
+        "minimum": 1,
+        "type": "number"
       },
       "per_page": {
-        "type": "number",
         "description": "Results per page for pagination (default: 30, max: 100)",
+        "maximum": 100,
         "minimum": 1,
-        "maximum": 100
+        "type": "number"
       },
       "repo": {
-        "type": "string",
-        "description": "Repository name"
+        "description": "Repository name",
+        "type": "string"
       },
       "resource_id": {
-        "type": "string",
-        "description": "The unique identifier of the resource. This will vary based on the \"method\" provided, so ensure you provide the correct ID:\n- Do not provide any resource ID for 'list_workflows' method.\n- Provide a workflow ID or workflow file name (e.g. ci.yaml) for 'list_workflow_runs' method, or omit to list all workflow runs in the repository.\n- Provide a workflow run ID for 'list_workflow_jobs' and 'list_workflow_run_artifacts' methods.\n"
+        "description": "The unique identifier of the resource. This will vary based on the \"method\" provided, so ensure you provide the correct ID:\n- Do not provide any resource ID for 'list_workflows' method.\n- Provide a workflow ID or workflow file name (e.g. ci.yaml) for 'list_workflow_runs' method, or omit to list all workflow runs in the repository.\n- Provide a workflow run ID for 'list_workflow_jobs' and 'list_workflow_run_artifacts' methods.\n",
+        "type": "string"
       },
       "workflow_jobs_filter": {
-        "type": "object",
+        "description": "Filters for workflow jobs. **ONLY** used when method is 'list_workflow_jobs'",
         "properties": {
           "filter": {
-            "type": "string",
             "description": "Filters jobs by their completed_at timestamp",
             "enum": [
               "latest",
               "all"
-            ]
+            ],
+            "type": "string"
           }
         },
-        "description": "Filters for workflow jobs. **ONLY** used when method is 'list_workflow_jobs'"
+        "type": "object"
       },
       "workflow_runs_filter": {
-        "type": "object",
+        "description": "Filters for workflow runs. **ONLY** used when method is 'list_workflow_runs'",
         "properties": {
           "actor": {
-            "type": "string",
-            "description": "Filter to a specific GitHub user's workflow runs."
+            "description": "Filter to a specific GitHub user's workflow runs.",
+            "type": "string"
           },
           "branch": {
-            "type": "string",
-            "description": "Filter workflow runs to a specific Git branch. Use the name of the branch."
+            "description": "Filter workflow runs to a specific Git branch. Use the name of the branch.",
+            "type": "string"
           },
           "event": {
-            "type": "string",
             "description": "Filter workflow runs to a specific event type",
             "enum": [
               "branch_protection_rule",
@@ -101,28 +99,30 @@
               "workflow_call",
               "workflow_dispatch",
               "workflow_run"
-            ]
+            ],
+            "type": "string"
           },
           "status": {
-            "type": "string",
             "description": "Filter workflow runs to only runs with a specific status",
             "enum": [
               "queued",
               "in_progress",
               "completed",
               "requested",
               "waiting"
-            ]
+            ],
+            "type": "string"
           }
         },
-        "description": "Filters for workflow runs. **ONLY** used when method is 'list_workflow_runs'"
+        "type": "object"
       }
     },
     "required": [
       "method",
       "owner",
       "repo"
-    ]
+    ],
+    "type": "object"
   },
   "name": "actions_list"
 }