Compare commits

...

1 Commits

Author SHA1 Message Date
fullstack-engineer 07e619403c fix(handlers): truncate manifest.json at last '}' before JSON parsing
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 3s
CI / Detect changes (pull_request) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 12s
E2E API Smoke Test / detect-changes (pull_request) Successful in 10s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 4s
Harness Replays / detect-changes (pull_request) Successful in 9s
E2E Chat / detect-changes (pull_request) Successful in 17s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 17s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 7s
qa-review / approved (pull_request) Successful in 4s
security-review / approved (pull_request) Successful in 4s
gate-check-v3 / gate-check (pull_request) Successful in 17s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 3s
sop-tier-check / tier-check (pull_request) Successful in 19s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m25s
Harness Replays / Harness Replays (pull_request) Successful in 4s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 9s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Failing after 1m29s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m56s
CI / Platform (Go) (pull_request) Successful in 4m2s
CI / Canvas (Next.js) (pull_request) Failing after 15m53s
E2E Chat / E2E Chat (pull_request) Failing after 13m59s
CI / all-required (pull_request) Failing after 16m8s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / Python Lint & Test (pull_request) Failing after 20m53s
audit-force-merge / audit (pull_request) Has been skipped
The repo-root manifest.json has a trailing JSON5-style // comment
appended by the Integration Tester CI trigger (// Triggered by
Integration Tester at 2026-05-10T08:52Z).  Go's encoding/json throws
"invalid character '/' after top-level value" on the '/', causing
loadRuntimesFromManifest to fall back to the fallback allowlist at
runtime registry init.  In E2E Chat runs (MOLECULE_ENV=development),
the platform server starts from workspace-server/ and resolves
../manifest.json — the file with the trailing comment — so the echo
runtime is never registered and all round-trip Chat tests fail.

Fix: bytes.LastIndex(data, []byte("}")) + slice to that position
before json.Unmarshal, so any trailing comment or garbage after the
closing brace is discarded silently.

Two regression tests added:
- TestLoadRuntimesFromManifest_StripsJSON5TrailingComment:
  real manifest content + "// Triggered by ..." after }
- TestLoadRuntimesFromManifest_ExtraDataAfterClosingBrace:
  valid JSON + arbitrary trailing text

Closes #1480

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-18 06:23:21 +00:00
2 changed files with 55 additions and 0 deletions
@@ -25,6 +25,7 @@ package handlers
// to the pre-refactor hardcoded list so nothing regresses.
import (
"bytes"
"encoding/json"
"log"
"os"
@@ -101,6 +102,13 @@ func loadRuntimesFromManifest(path string) (map[string]struct{}, error) {
if err != nil {
return nil, err
}
// Truncate at the last '}' to drop trailing JSON5-style // comments
// (e.g. "// Triggered by Integration Tester at 2026-05-10T08:52Z")
// that would otherwise cause Go's json.Unmarshal to fail with
// "invalid character '/' after top-level value".
if last := bytes.LastIndex(data, []byte("}")); last != -1 {
data = data[:last+1]
}
var m manifestFile
if err := json.Unmarshal(data, &m); err != nil {
return nil, err
@@ -83,6 +83,53 @@ func TestLoadRuntimesFromManifest_MalformedJSON(t *testing.T) {
}
}
func TestLoadRuntimesFromManifest_StripsJSON5TrailingComment(t *testing.T) {
// Regression: manifest.json has a trailing JSON5-style // comment appended
// by the Integration Tester (// Triggered by ...). Go's encoding/json
// throws "invalid character '/' after top-level value" on the '/'.
// loadRuntimesFromManifest must truncate at the last '}' before parsing
// so the comment is ignored and the valid JSON body is processed.
dir := t.TempDir()
path := filepath.Join(dir, "manifest.json")
content := `{
"workspace_templates": [
{"name": "claude-code-default", "repo": "org/t-cc"},
{"name": "langgraph", "repo": "org/t-lg"}
]
}
// Triggered by Integration Tester at 2026-05-10T08:52Z`
_ = os.WriteFile(path, []byte(content), 0600)
got, err := loadRuntimesFromManifest(path)
if err != nil {
t.Fatalf("expected successful parse despite trailing comment, got: %v", err)
}
for _, must := range []string{"claude-code", "langgraph", "external", "kimi", "kimi-cli"} {
if _, ok := got[must]; !ok {
t.Errorf("expected runtime %q in set after JSON5 comment strip: %v", must, keys(got))
}
}
}
func TestLoadRuntimesFromManifest_ExtraDataAfterClosingBrace(t *testing.T) {
// A file with trailing garbage after the closing '}' (not a // comment)
// should also be tolerated — the parser truncates at last '}';
// anything after is discarded.
dir := t.TempDir()
path := filepath.Join(dir, "manifest.json")
content := `{"workspace_templates":[{"name":"hermes","repo":"org/t"}]}
extra data that should be ignored`
_ = os.WriteFile(path, []byte(content), 0600)
got, err := loadRuntimesFromManifest(path)
if err != nil {
t.Fatalf("expected parse with trailing data: %v", err)
}
if _, ok := got["hermes"]; !ok {
t.Errorf("expected hermes runtime after truncation: %v", keys(got))
}
}
// 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