From fe6ada46c2bae7f8bc5cc22f9bf98de070ed0e23 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Tue, 12 May 2026 17:47:12 +0000 Subject: [PATCH 01/12] fix(handlers/discovery): nil-guard role in filterPeersByQuery (mc#731) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit queryPeerMaps sets peer["role"] = nil when the DB role column is empty (discovery.go lines 337-341). filterPeersByQuery did a bare type assertion p["role"].(string) which panics on nil. Fix: use the comma-ok form so nil → "" (empty string) — both name and role fields now use x, _ := p["key"].(string) rather than x := p["key"].(string). Add TestFilterPeersByQuery_NilRoleRegression with three cases: - nil role matches on name substring - nil name/role with empty q (no-op, returns all) - all nil — no panic, returns empty Regression gate for mc#730/#731. Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/discovery.go | 4 +- .../internal/handlers/discovery_test.go | 74 +++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/workspace-server/internal/handlers/discovery.go b/workspace-server/internal/handlers/discovery.go index 79315016..5c798c81 100644 --- a/workspace-server/internal/handlers/discovery.go +++ b/workspace-server/internal/handlers/discovery.go @@ -292,8 +292,8 @@ func filterPeersByQuery(peers []map[string]interface{}, q string) []map[string]i needle := strings.ToLower(q) out := make([]map[string]interface{}, 0, len(peers)) for _, p := range peers { - name := p["name"].(string) - role := p["role"].(string) + name, _ := p["name"].(string) // nil → "" — safe on empty-role rows + role, _ := p["role"].(string) // nil → "" — queryPeerMaps sets nil when DB role is empty if strings.Contains(strings.ToLower(name), needle) || strings.Contains(strings.ToLower(role), needle) { out = append(out, p) diff --git a/workspace-server/internal/handlers/discovery_test.go b/workspace-server/internal/handlers/discovery_test.go index 892a1f0a..3070fcf4 100644 --- a/workspace-server/internal/handlers/discovery_test.go +++ b/workspace-server/internal/handlers/discovery_test.go @@ -394,6 +394,80 @@ func TestPeers_Q_NoMatches_RawBodyIsArrayNotNull(t *testing.T) { } } +// TestFilterPeersByQuery_NilRoleRegression is the regression gate for +// mc#730/#731: queryPeerMaps sets peer["role"] = nil when the DB role column +// is empty (discovery.go lines 337-341). filterPeersByQuery did a bare +// type assertion p["role"].(string) which panics on nil. The fix uses the +// comma-ok form so nil → "". The test passes a map with nil name and nil +// role and asserts no panic + correct filter behaviour. +func TestFilterPeersByQuery_NilRoleRegression(t *testing.T) { + cases := []struct { + name string + peers []map[string]interface{} + q string + wantLen int + wantIDs []string + }{ + { + name: "nil role matches on name", + peers: []map[string]interface{}{ + {"id": "ws-a", "name": nil, "role": nil}, + {"id": "ws-b", "name": "Alpha Builder", "role": nil}, + {"id": "ws-c", "name": "Beta Builder", "role": nil}, + }, + q: "alpha", + wantLen: 1, + wantIDs: []string{"ws-b"}, + }, + { + name: "nil name matches on nil role (empty string)", + peers: []map[string]interface{}{ + {"id": "ws-x", "name": nil, "role": nil}, + {"id": "ws-y", "name": "Dev Workspace", "role": nil}, + }, + q: "", + wantLen: 2, // empty q is a no-op + wantIDs: []string{"ws-x", "ws-y"}, + }, + { + name: "all nil — no panic, returns input", + peers: []map[string]interface{}{ + {"id": "ws-z", "name": nil, "role": nil}, + }, + q: "anything", + wantLen: 0, + wantIDs: nil, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := filterPeersByQuery(tc.peers, tc.q) + if len(got) != tc.wantLen { + t.Fatalf("len: got %d, want %d", len(got), tc.wantLen) + } + gotIDs := make([]string, len(got)) + for i, p := range got { + gotIDs[i] = p["id"].(string) + } + if tc.wantIDs != nil { + for _, id := range tc.wantIDs { + found := false + for _, g := range gotIDs { + if g == id { + found = true + break + } + } + if !found { + t.Errorf("missing id %q; got IDs: %v", id, gotIDs) + } + } + } + }) + } +} + func keysOf(m map[string]struct{}) []string { out := make([]string, 0, len(m)) for k := range m { From ea320ff7a9ef0b14671f1dd918e1f857880deeb3 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Tue, 12 May 2026 17:57:42 +0000 Subject: [PATCH 02/12] fix(handlers/terminal): surface AWS subprocess stderr in send-ssh-public-key Detail (mc#687) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mc#687 root-cause from mc#424: when the diagnose probe's send-ssh-public-key step fails (IAM permission gap), the Go error string says only "exec: exit status 1" — the actionable AWS permission error is in the subprocess stderr captured by CombinedOutput() but was not being surfaced as `detail`. Fix: add unwrapGoError() helper that extracts subprocess stderr from the Go-wrapped error string (the fmt.Errorf wraps CombinedOutput in parens). The send-ssh-public-key step now populates both Error (Go error string) and Detail (subprocess stderr), so the E2E smoke (which now reads detail) sees e.g. "AccessDeniedException: ... is not authorized to perform: ec2-instance-connect:OpenTunnel" verbatim. Complements PR #748 which fixes the E2E test to read detail field. Regression gate for mc#687. Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/terminal_diagnose.go | 40 +++++++++++++++- .../handlers/terminal_diagnose_test.go | 47 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/workspace-server/internal/handlers/terminal_diagnose.go b/workspace-server/internal/handlers/terminal_diagnose.go index b78c8955..c6cfe7b8 100644 --- a/workspace-server/internal/handlers/terminal_diagnose.go +++ b/workspace-server/internal/handlers/terminal_diagnose.go @@ -43,6 +43,38 @@ func (s *syncBuf) String() string { return s.b.String() } +// unwrapGoError extracts subprocess stderr from a Go-wrapped error that +// includes combined output. e.g. from sendSSHPublicKey's +// fmt.Errorf("send-ssh-public-key: %w (%s)", err, combinedOut), this +// returns the " (%s)" portion — the actionable subprocess signal like +// "AccessDeniedException: ... is not authorized to perform: +// ec2-instance-connect:OpenTunnel". Returns "" when the output is +// identical to the error string (no stderr captured). +func unwrapGoError(errMsg string) string { + // Find the last ") " at the end — the fmt.Errorf wrapper puts + // subprocess output in parentheses at the end of the string. + // e.g. "send-ssh-public-key: exec: exit status 1 (AccessDenied...)" + if len(errMsg) < 4 { + return "" + } + // Find last ")(" pattern: Go's %w wraps before the output paren + // e.g. "send-ssh-public-key: exit status 1 (AccessDeniedException: ...)" + // We want everything after the last ") " — the subprocess stderr. + // Safe heuristic: strip up to and including the last ") " prefix. + sep := ") " + idx := strings.LastIndex(errMsg, sep) + if idx == -1 { + return "" + } + candidate := errMsg[idx+len(sep):] + // If stripping the last ") ..." leaves the string unchanged, there + // was no real subprocess output. + if candidate == errMsg { + return "" + } + return candidate +} + // HandleDiagnose handles GET /workspaces/:id/terminal/diagnose. It runs the // same per-step pipeline as HandleConnect (ssh-keygen → EIC send-key → tunnel // → ssh) but non-interactively, captures the first failing step and its @@ -214,12 +246,18 @@ func (h *TerminalHandler) diagnoseRemote(ctx context.Context, workspaceID, insta } // Step 2: send-ssh-public-key (AWS Instance Connect) + // mc#687: populate Detail so the E2E smoke sees the AWS permission error + // verbatim. The subprocess stderr (e.g. "AccessDeniedException: ... is not + // authorized to perform: ec2-instance-connect:OpenTunnel") is captured by + // sendSSHPublicKey's CombinedOutput() and embedded in the Go error string. t0 = time.Now() if err := sendSSHPublicKey(ctx, region, instanceID, osUser, strings.TrimSpace(string(pubKey))); err != nil { + errMsg := err.Error() return stop("send-ssh-public-key", diagnoseStep{ Name: "send-ssh-public-key", DurationMs: time.Since(t0).Milliseconds(), - Error: err.Error(), + Error: errMsg, + Detail: unwrapGoError(errMsg), }) } res.Steps = append(res.Steps, diagnoseStep{Name: "send-ssh-public-key", OK: true, DurationMs: time.Since(t0).Milliseconds()}) diff --git a/workspace-server/internal/handlers/terminal_diagnose_test.go b/workspace-server/internal/handlers/terminal_diagnose_test.go index 15b94945..1364c2c2 100644 --- a/workspace-server/internal/handlers/terminal_diagnose_test.go +++ b/workspace-server/internal/handlers/terminal_diagnose_test.go @@ -245,3 +245,50 @@ func TestDiagnoseRemote_StopsAtSSHProbe(t *testing.T) { } } +// TestUnwrapGoError pins the unwrapGoError helper that extracts subprocess +// stderr from the Go-wrapped error string produced by sendSSHPublicKey. +// Regression gate for mc#687: the E2E smoke now reads detail (not error), +// so detail MUST contain the actionable AWS permission signal. +func TestUnwrapGoError(t *testing.T) { + cases := []struct { + name string + input string + want string + }{ + { + name: "AWS permission denied", + input: "send-ssh-public-key: exec: exit status 1 (AccessDeniedException: User: arn:aws:iam::123456789012:role/TestRole is not authorized to perform: ec2-instance-connect:OpenTunnel)", + want: "AccessDeniedException: User: arn:aws:iam::123456789012:role/TestRole is not authorized to perform: ec2-instance-connect:OpenTunnel", + }, + { + name: "generic exec error no output", + input: "send-ssh-public-key: exec: exit status 1", + want: "", + }, + { + name: "empty string", + input: "", + want: "", + }, + { + name: "short string below threshold", + input: "err", + want: "", + }, + { + name: "no parentheses", + input: "send-ssh-public-key: something went wrong", + want: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := unwrapGoError(tc.input) + if got != tc.want { + t.Errorf("unwrapGoError(%q): got %q, want %q", tc.input, got, tc.want) + } + }) + } +} + From 27ddbdad5bb25c6baca0bd82a24528039045d7a9 Mon Sep 17 00:00:00 2001 From: core-be Date: Tue, 12 May 2026 19:13:20 +0000 Subject: [PATCH 03/12] ci: trigger CI rerun [empty commit] From 724723ab2388700d1174550ce0b414c838b803dc Mon Sep 17 00:00:00 2001 From: core-be Date: Tue, 12 May 2026 19:27:32 +0000 Subject: [PATCH 04/12] =?UTF-8?q?fix(handlers/terminal):=20fix=20unwrapGoE?= =?UTF-8?q?rror=20separator=20=E2=80=94=20use=20LastIndex("(")=20not=20")?= =?UTF-8?q?=20"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/handlers/terminal_diagnose.go | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/workspace-server/internal/handlers/terminal_diagnose.go b/workspace-server/internal/handlers/terminal_diagnose.go index c6cfe7b8..cf1422e2 100644 --- a/workspace-server/internal/handlers/terminal_diagnose.go +++ b/workspace-server/internal/handlers/terminal_diagnose.go @@ -51,28 +51,17 @@ func (s *syncBuf) String() string { // ec2-instance-connect:OpenTunnel". Returns "" when the output is // identical to the error string (no stderr captured). func unwrapGoError(errMsg string) string { - // Find the last ") " at the end — the fmt.Errorf wrapper puts - // subprocess output in parentheses at the end of the string. - // e.g. "send-ssh-public-key: exec: exit status 1 (AccessDenied...)" - if len(errMsg) < 4 { - return "" - } - // Find last ")(" pattern: Go's %w wraps before the output paren + // Extract content between the last '(' and trailing ')'. The + // sendSSHPublicKey wrapper uses fmt.Errorf("...: %w (%s)", err, combinedOut) + // so the subprocess stderr is always the last parenthesised segment, // e.g. "send-ssh-public-key: exit status 1 (AccessDeniedException: ...)" - // We want everything after the last ") " — the subprocess stderr. - // Safe heuristic: strip up to and including the last ") " prefix. - sep := ") " - idx := strings.LastIndex(errMsg, sep) - if idx == -1 { + // — note the closing ')' is at the very end with no trailing space. + open := strings.LastIndex(errMsg, "(") + if open < 0 { return "" } - candidate := errMsg[idx+len(sep):] - // If stripping the last ") ..." leaves the string unchanged, there - // was no real subprocess output. - if candidate == errMsg { - return "" - } - return candidate + inner := errMsg[open+1:] + return strings.TrimSuffix(inner, ")") } // HandleDiagnose handles GET /workspaces/:id/terminal/diagnose. It runs the From 8a0d12ee6b32f7390f4533cb733dfbba26b4e5b1 Mon Sep 17 00:00:00 2001 From: core-devops Date: Tue, 12 May 2026 20:50:56 +0000 Subject: [PATCH 05/12] ci: rerun after mc#724 all-required fix lands From f1ad640197ae23aac4e7829dc10b8a624120f94f Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Tue, 12 May 2026 21:17:46 +0000 Subject: [PATCH 06/12] ci: rerun after concurrency-block clear From 7d66f6199cfbb3d9ad9505c256b9e43038bf4ae5 Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Tue, 12 May 2026 21:26:12 +0000 Subject: [PATCH 07/12] ci: clean-slate rerun From 0e97788bf86b411a145e3480d8b39909e95b4ac5 Mon Sep 17 00:00:00 2001 From: claude-ceo-assistant Date: Tue, 12 May 2026 21:30:44 +0000 Subject: [PATCH 08/12] ci: post-restart rerun From 29c5f0a77dcbe8b5800558ddf28898ebca921112 Mon Sep 17 00:00:00 2001 From: core-lead Date: Tue, 12 May 2026 21:35:28 +0000 Subject: [PATCH 09/12] ci: clean-slate rerun v2 From 1301d09ec6b9132a593c506d34d2c14077301a0d Mon Sep 17 00:00:00 2001 From: core-lead Date: Tue, 12 May 2026 21:44:51 +0000 Subject: [PATCH 10/12] ci: global-zombie-purge rerun From f624d1adad0bb8622a341164286652b176ef682d Mon Sep 17 00:00:00 2001 From: core-lead Date: Tue, 12 May 2026 21:48:50 +0000 Subject: [PATCH 11/12] ci: post-full-purge rerun From 31b3ae9b64b8b856bd5a27927c0df80c7406e2f3 Mon Sep 17 00:00:00 2001 From: core-devops Date: Tue, 12 May 2026 22:07:39 +0000 Subject: [PATCH 12/12] ci: post-purge rerun