From fc3ae5a63ab9ff7c23a1ca7ec086359b76e0a015 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Mon, 20 Apr 2026 16:04:57 -0700 Subject: [PATCH] chore: code-review cleanup on today's shipped PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three nits identified during post-merge review of #1119, #1133: 1. ContextMenu.tsx imported `removeNode` from the canvas store but stopped using it when the delete-confirm flow moved to Canvas in #1133. Also removed the now-unused mock entry in the keyboard test so the test inventory matches the real call list. 2. Preflight's YAML parse failure was a silent pass — defensible since the in-container preflight owns the schema, but invisible to ops if a template ships malformed YAML. Log at WARN so the signal surfaces without blocking the provision. 3. formatMissingEnvError rendered its slice via %q, producing `["A" "B"]` which is Go-literal-looking and ugly in a user-facing error. Join with ", " instead. Test updated to assert the new format. No behavioural changes beyond the log line; fixes are review nits, not bug fixes. Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/ContextMenu.tsx | 1 - .../__tests__/ContextMenu.keyboard.test.tsx | 1 - .../internal/handlers/workspace_preflight.go | 11 +++++++++-- .../internal/handlers/workspace_preflight_test.go | 6 +++--- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/canvas/src/components/ContextMenu.tsx b/canvas/src/components/ContextMenu.tsx index 4211b7b4..34c17b01 100644 --- a/canvas/src/components/ContextMenu.tsx +++ b/canvas/src/components/ContextMenu.tsx @@ -18,7 +18,6 @@ interface MenuItem { export function ContextMenu() { const contextMenu = useCanvasStore((s) => s.contextMenu); const closeContextMenu = useCanvasStore((s) => s.closeContextMenu); - const removeNode = useCanvasStore((s) => s.removeNode); const updateNodeData = useCanvasStore((s) => s.updateNodeData); const selectNode = useCanvasStore((s) => s.selectNode); const setPanelTab = useCanvasStore((s) => s.setPanelTab); diff --git a/canvas/src/components/__tests__/ContextMenu.keyboard.test.tsx b/canvas/src/components/__tests__/ContextMenu.keyboard.test.tsx index b37a0e4f..5381ed81 100644 --- a/canvas/src/components/__tests__/ContextMenu.keyboard.test.tsx +++ b/canvas/src/components/__tests__/ContextMenu.keyboard.test.tsx @@ -40,7 +40,6 @@ const mockStore = { nodeData: Record; } | null, closeContextMenu, - removeNode: vi.fn(), updateNodeData: vi.fn(), selectNode: vi.fn(), setPanelTab: vi.fn(), diff --git a/workspace-server/internal/handlers/workspace_preflight.go b/workspace-server/internal/handlers/workspace_preflight.go index 837786c7..647f4f23 100644 --- a/workspace-server/internal/handlers/workspace_preflight.go +++ b/workspace-server/internal/handlers/workspace_preflight.go @@ -2,6 +2,8 @@ package handlers import ( "fmt" + "log" + "strings" "gopkg.in/yaml.v3" ) @@ -39,6 +41,11 @@ func missingRequiredEnv(configFiles map[string][]byte, envVars map[string]string } var schema requiredEnvSchema if err := yaml.Unmarshal(raw, &schema); err != nil { + // Safe default: the in-container preflight is the source of truth + // for config.yaml shape, so we don't block the provision here. But + // log at WARN so operators can notice a template with malformed + // YAML — otherwise a silently-skipped preflight is invisible. + log.Printf("Preflight: WARN — config.yaml unparseable, skipping required_env check: %v", err) return nil } if len(schema.RuntimeConfig.RequiredEnv) == 0 { @@ -64,7 +71,7 @@ func formatMissingEnvError(missing []string) string { ) } return fmt.Sprintf( - "missing required env vars %q — add them under Config → Env Vars (or as Global secrets) and retry", - missing, + "missing required env vars %s — add them under Config → Env Vars (or as Global secrets) and retry", + strings.Join(missing, ", "), ) } diff --git a/workspace-server/internal/handlers/workspace_preflight_test.go b/workspace-server/internal/handlers/workspace_preflight_test.go index fcb865af..1a53f6f9 100644 --- a/workspace-server/internal/handlers/workspace_preflight_test.go +++ b/workspace-server/internal/handlers/workspace_preflight_test.go @@ -117,8 +117,8 @@ func TestFormatMissingEnvError_Single(t *testing.T) { } func TestFormatMissingEnvError_Multiple(t *testing.T) { - msg := formatMissingEnvError([]string{"A", "B"}) - if !strings.Contains(msg, "A") || !strings.Contains(msg, "B") { - t.Errorf("message should name both vars: %q", msg) + msg := formatMissingEnvError([]string{"FOO", "BAR"}) + if !strings.Contains(msg, "FOO, BAR") { + t.Errorf("multi-var message should join with ', ' — got %q", msg) } }