chore: code-review cleanup on today's shipped PRs
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) <noreply@anthropic.com>
This commit is contained in:
parent
bd630a83b7
commit
fc3ae5a63a
@ -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);
|
||||
|
||||
@ -40,7 +40,6 @@ const mockStore = {
|
||||
nodeData: Record<string, unknown>;
|
||||
} | null,
|
||||
closeContextMenu,
|
||||
removeNode: vi.fn(),
|
||||
updateNodeData: vi.fn(),
|
||||
selectNode: vi.fn(),
|
||||
setPanelTab: vi.fn(),
|
||||
|
||||
@ -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, ", "),
|
||||
)
|
||||
}
|
||||
|
||||
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user