From b1fac110f2d7c9d7927989ba76107a112dc50ede Mon Sep 17 00:00:00 2001 From: Molecule AI Fullstack Engineer Date: Mon, 18 May 2026 14:13:56 +0000 Subject: [PATCH] fix(workspace-server): strip JSON5 // comments from manifest.json before parsing Fixes E2E Chat deterministically failing in CI with "invalid character '/' after top-level value". The Integration Tester appends "// Triggered by " to manifest.json after cloning. Go's json.Unmarshal rejects JSON5-style // comments. Added stripJSON5Comments() that strips // comments while preserving URLs with // in them (e.g. https://), then call it before json.Unmarshal in loadRuntimesFromManifest. Added 8 new tests covering: - stripJSON5Comments: standalone comments, trailing comments, URLs preserved, inline comments, Integration Tester append, no-op - loadRuntimesFromManifest: with trailing // comment, with inline // comment Closes #1496 Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/runtime_registry.go | 37 +++++ .../handlers/runtime_registry_test.go | 131 ++++++++++++++++++ 2 files changed, 168 insertions(+) diff --git a/workspace-server/internal/handlers/runtime_registry.go b/workspace-server/internal/handlers/runtime_registry.go index 0efa2ec0c..4ded65abe 100644 --- a/workspace-server/internal/handlers/runtime_registry.go +++ b/workspace-server/internal/handlers/runtime_registry.go @@ -86,6 +86,40 @@ var fallbackRuntimes = map[string]struct{}{ "mock": {}, } +// stripJSON5Comments removes // single-line comments from JSON5-formatted +// data. The Integration Tester appends "// Triggered by " to +// manifest.json after cloning, which causes json.Unmarshal to fail with +// "invalid character '/'". This strips trailing and mid-file comments +// before parsing so Go's strict JSON parser accepts JSON5 files. +// +// Handles: +// - Standalone comment lines: // comment +// - Trailing comments: "key": "value", // comment +// - Comments inside strings are NOT touched ("http://example.com") +func stripJSON5Comments(data []byte) []byte { + var result []byte + inString := false + i := 0 + for i < len(data) { + if data[i] == '"' && (i == 0 || data[i-1] != '\\') { + inString = !inString + result = append(result, data[i]) + i++ + continue + } + if !inString && i+1 < len(data) && data[i] == '/' && data[i+1] == '/' { + // Skip to end of line + for i < len(data) && data[i] != '\n' { + i++ + } + continue + } + result = append(result, data[i]) + i++ + } + return result +} + // loadRuntimesFromManifest builds the runtime allowlist from // manifest.json. Each workspace_templates[].name is normalized to its // base runtime identifier (strips the `-default` suffix templates @@ -101,6 +135,9 @@ func loadRuntimesFromManifest(path string) (map[string]struct{}, error) { if err != nil { return nil, err } + // Strip JSON5 // comments before parsing. The Integration Tester + // appends "// Triggered by " to manifest.json after cloning. + data = stripJSON5Comments(data) var m manifestFile if err := json.Unmarshal(data, &m); err != nil { return nil, err diff --git a/workspace-server/internal/handlers/runtime_registry_test.go b/workspace-server/internal/handlers/runtime_registry_test.go index 78c2c6878..54dedbc6a 100644 --- a/workspace-server/internal/handlers/runtime_registry_test.go +++ b/workspace-server/internal/handlers/runtime_registry_test.go @@ -8,6 +8,7 @@ package handlers // fallback (tested at the initKnownRuntimes level via integration). import ( + "encoding/json" "os" "path/filepath" "testing" @@ -111,3 +112,133 @@ func keys(m map[string]struct{}) []string { } return out } + +// ── stripJSON5Comments tests ───────────────────────────────────────────────── + +func TestStripJSON5Comments_Standalone(t *testing.T) { + // Whitespace before the // is preserved; only the // and its text are removed. + // The result is still valid JSON. + input := "{\n\t// This is a comment\n\t\"workspace_templates\": []\n}" + got := string(stripJSON5Comments([]byte(input))) + // Stripping should produce valid JSON: try parsing it + var m manifestFile + if err := json.Unmarshal([]byte(got), &m); err != nil { + t.Errorf("output is not valid JSON: %v\ngot: %q", err, got) + } + if m.WorkspaceTemplates == nil { + t.Error("WorkspaceTemplates field should be present after parsing") + } +} + +func TestStripJSON5Comments_Trailing(t *testing.T) { + input := `{"key": "value"} // trailing comment` + got := string(stripJSON5Comments([]byte(input))) + want := `{"key": "value"} ` + if got != want { + t.Errorf("trailing comment:\ngot: %q\nwant: %q", got, want) + } +} + +func TestStripJSON5Comments_URLsPreserved(t *testing.T) { + // URLs with // in them must NOT be stripped + input := `{"url": "https://example.com/path"}` + got := string(stripJSON5Comments([]byte(input))) + if got != input { + t.Errorf("URL stripped: got %q, want %q", got, input) + } +} + +func TestStripJSON5Comments_InlineComment(t *testing.T) { + // Whitespace before // is preserved; only the comment text is removed. + // Result must be valid JSON. + input := "{\n\t\"workspace_templates\": [] // inline comment\n}" + got := string(stripJSON5Comments([]byte(input))) + var m manifestFile + if err := json.Unmarshal([]byte(got), &m); err != nil { + t.Errorf("output is not valid JSON: %v\ngot: %q", err, got) + } +} + +func TestStripJSON5Comments_IntegrationTesterAppend(t *testing.T) { + // Simulates what the Integration Tester does: appends // Triggered by ... + // to the end of a valid JSON file. + input := `{ + "workspace_templates": [ + {"name": "hermes", "repo": "org/hermes"} + ] +} +// Triggered by e2e-test job 12345 +` + got := string(stripJSON5Comments([]byte(input))) + if got == input { + t.Error("Integration Tester comment was NOT stripped") + } + // After stripping, it should be valid JSON + var m manifestFile + if err := json.Unmarshal([]byte(got), &m); err != nil { + t.Errorf("stripped content is not valid JSON: %v\ngot: %q", err, got) + } + if len(m.WorkspaceTemplates) != 1 || m.WorkspaceTemplates[0].Name != "hermes" { + t.Errorf("workspace_templates not parsed: %+v", m.WorkspaceTemplates) + } +} + +func TestStripJSON5Comments_NoComments(t *testing.T) { + input := `{"workspace_templates": [{"name": "hermes"}]}` + got := string(stripJSON5Comments([]byte(input))) + if got != input { + t.Errorf("unmodified JSON changed: got %q", got) + } +} + +// ── loadRuntimesFromManifest with JSON5 comments ───────────────────────────── + +func TestLoadRuntimesFromManifest_WithJSON5TrailingComment(t *testing.T) { + // Regression: Integration Tester appends "// Triggered by ..." to manifest.json + // after cloning. Before the fix, this caused json.Unmarshal to fail with + // "invalid character '/'". After the fix, the comment is stripped first. + dir := t.TempDir() + path := filepath.Join(dir, "manifest.json") + manifest := `{ + "workspace_templates": [ + {"name": "hermes", "repo": "org/hermes"}, + {"name": "claude-code-default", "repo": "org/cc"} + ] + } + // Triggered by e2e-test job + ` + if err := os.WriteFile(path, []byte(manifest), 0600); err != nil { + t.Fatalf("write: %v", err) + } + got, err := loadRuntimesFromManifest(path) + if err != nil { + t.Fatalf("loadRuntimesFromManifest with trailing comment: %v", err) + } + for _, must := range []string{"hermes", "claude-code"} { + if _, ok := got[must]; !ok { + t.Errorf("expected runtime %q in result: %v", must, keys(got)) + } + } +} + +func TestLoadRuntimesFromManifest_WithInlineJSON5Comment(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "manifest.json") + // JSON5 with inline comment + manifest := `{ + // runtime templates + "workspace_templates": [ + {"name": "langgraph", "repo": "org/lg"} // the default + ] + }` + if err := os.WriteFile(path, []byte(manifest), 0600); err != nil { + t.Fatalf("write: %v", err) + } + got, err := loadRuntimesFromManifest(path) + if err != nil { + t.Fatalf("loadRuntimesFromManifest with inline comment: %v", err) + } + if _, ok := got["langgraph"]; !ok { + t.Errorf("expected langgraph in result: %v", keys(got)) + } +} -- 2.52.0