fix(workspace-server): strip JSON5 // comments from manifest.json before parsing #1496

Closed
infra-runtime-be wants to merge 1 commits from fix/runtime-registry-manifest-v2 into main
2 changed files with 122 additions and 1 deletions
@@ -96,13 +96,70 @@ var fallbackRuntimes = map[string]struct{}{
// Caller logs + falls back to fallbackRuntimes on any error. Not
// returning the fallback here ourselves so the caller can decide
// how loud to be about the miss (prod = WARN, tests = silent).
// stripJSON5Comments removes a JSON5-style // trailing comment from manifest.json.
// The Integration Tester appends "// Triggered by ..." at the very end of the file.
// This comment is always after the final closing brace, so we scan only that
// suffix rather than trying to track string-context across the whole file.
// This avoids false-positives on legitimate // in URL values (e.g. http://foo.com/bar).
func stripJSON5Comments(data []byte) []byte {
// Find the last '}' — everything before it is guaranteed standard JSON.
lastBrace := -1
for i := len(data) - 1; i >= 0; i-- {
if data[i] == '}' {
lastBrace = i
break
}
}
if lastBrace == -1 {
return data // no JSON structure found — return as-is, json.Unmarshal will error
}
// Everything after lastBrace is the trailing suffix to clean.
suffixStart := lastBrace + 1
if suffixStart >= len(data) {
return data // no suffix
}
suffix := data[suffixStart:]
// Strip leading whitespace at the start of the suffix.
cleanSuffix := trimLeadingWhitespace(suffix)
if len(cleanSuffix) == 0 || cleanSuffix[0] != '/' {
return data // suffix is empty or starts with non-comment — nothing to strip
}
// Remove the trailing comment (everything from the first // to end of file).
// Rebuild: prefix + suffix with comment stripped.
before := data[:suffixStart]
// Trim trailing whitespace from before so we don't leave a dangling newline.
trimmedBefore := trimTrailingWhitespace(before)
// Append a single newline so the JSON file ends cleanly.
result := append(trimmedBefore, '\n')
return result
}
func trimLeadingWhitespace(b []byte) []byte {
i := 0
for i < len(b) && (b[i] == ' ' || b[i] == '\t' || b[i] == '\n' || b[i] == '\r') {
i++
}
return b[i:]
}
func trimTrailingWhitespace(b []byte) []byte {
i := len(b)
for i > 0 && (b[i-1] == ' ' || b[i-1] == '\t' || b[i-1] == '\n' || b[i-1] == '\r') {
i--
}
return b[:i]
}
func loadRuntimesFromManifest(path string) (map[string]struct{}, error) {
data, err := os.ReadFile(path)
if err != nil {
return nil, err
}
// The Integration Tester appends "// Triggered by ..." to manifest.json.
// json.Unmarshal rejects it; strip // comments first (same as clone-manifest.sh).
clean := stripJSON5Comments(data)
var m manifestFile
if err := json.Unmarshal(data, &m); err != nil {
if err := json.Unmarshal(clean, &m); err != nil {
return nil, err
}
out := map[string]struct{}{
@@ -83,6 +83,70 @@ func TestLoadRuntimesFromManifest_MalformedJSON(t *testing.T) {
}
}
func TestLoadRuntimesFromManifest_TrailingJSON5Comment(t *testing.T) {
// The Integration Tester appends "// Triggered by Integration Tester at ..."
// to manifest.json after cloning. json.Unmarshal rejects it; stripJSON5Comments
// must remove the trailing comment so load succeeds.
dir := t.TempDir()
path := filepath.Join(dir, "manifest.json")
_ = os.WriteFile(path, []byte(`{
"workspace_templates": [
{"name": "langgraph", "repo": "org/t"}
]
}
// Triggered by Integration Tester at 2026-05-10T08:52Z`), 0600)
got, err := loadRuntimesFromManifest(path)
if err != nil {
t.Fatalf("load failed despite trailing comment: %v", err)
}
if _, ok := got["langgraph"]; !ok {
t.Errorf("langgraph missing from result: %v", keys(got))
}
}
func TestStripJSON5Comments(t *testing.T) {
cases := []struct {
name string
in string
want string
}{
{
name: "trailing comment after closing brace removed",
in: "{}\n// Triggered by Integration Tester\n",
want: "{}\n",
},
{
name: "embedded_in_url_preserved",
in: `{"url":"http://foo.com/bar"}`,
want: `{"url":"http://foo.com/bar"}`,
},
{
name: "no_closing_brace_returns_input_unchanged",
in: "no json here // comment",
want: "no json here // comment",
},
{
name: "comment_only_after_closing_brace_stripped",
in: `{"a":1}` + "\n// Triggered by Integration Tester at 2026-05-10T08:52Z",
want: `{"a":1}` + "\n",
},
{
name: "clean_json_unchanged",
in: `{"workspace_templates":[]}` + "\n",
want: `{"workspace_templates":[]}` + "\n",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := string(stripJSON5Comments([]byte(tc.in)))
if got != tc.want {
t.Errorf("stripJSON5Comments(%q): got %q, want %q", tc.in, got, tc.want)
}
})
}
}
// TestRealManifestParses — sanity check against the actual
// monorepo manifest.json so a future schema change to that file
// (e.g. workspace_templates → workspace_runtime_templates) surfaces