Skip to content

fix: sanitize component key input in pipeline editor#8

Merged
TerrifiedBug merged 3 commits intomainfrom
fix/component-key-ux
Mar 5, 2026
Merged

fix: sanitize component key input in pipeline editor#8
TerrifiedBug merged 3 commits intomainfrom
fix/component-key-ux

Conversation

@TerrifiedBug
Copy link
Owner

Summary

  • Auto-sanitize component key input: spaces → underscores, strip invalid chars, prefix leading digits
  • Add helper text below the input showing allowed format
  • Prevents cryptic Zod validation errors when users type names like "Traefik Logs"

Test plan

  • Type a name with spaces (e.g. "Traefik Logs") — should become "Traefik_Logs"
  • Type special characters (hyphens, dots) — should be stripped
  • Type a leading number — should get prefixed with underscore
  • Save pipeline — no validation error

Automatically convert spaces to underscores and strip invalid
characters as users type component keys. Adds helper text showing
the allowed format. Prevents cryptic server-side validation errors
from Vector's identifier requirements.
@github-actions github-actions bot added the fix label Mar 5, 2026
@greptile-apps
Copy link

greptile-apps bot commented Mar 5, 2026

Greptile Summary

This PR adds input sanitization to the component key field in the pipeline detail panel, converting spaces to underscores, stripping invalid characters, and prefixing leading digits — preventing cryptic Zod validation errors at save time. It introduces a local displayKey state variable and a useEffect to keep it in sync with the Zustand store.

Key issues found:

  • setDisplayKey(raw) instead of setDisplayKey(sanitized) (line 157): Because updateNodeKey immediately changes storeKey, the useEffect fires after each keystroke and snaps displayKey back to the sanitized value. Setting displayKey to raw first causes a visible flicker/jump on every sanitizable character — it should be set to sanitized directly.
  • Unused componentKey variable (line 223): componentKey is still destructured from selectedNode.data but is no longer consumed anywhere in the template; removing it avoids a lint warning.

The sanitization logic (handleKeyChange) and the guard against empty keys are correct and address the root cause well.

Confidence Score: 3/5

  • Safe to merge after fixing the input flicker bug and removing unused variable — both are one-line changes with no data-loss or pipeline-correctness risks.
  • The sanitization logic is correct and the empty-string guard works properly. However, there are two concrete issues: (1) setDisplayKey(raw) causes visible input flicker on every sanitizable keystroke due to the immediate useEffect override, and (2) unused componentKey destructuring will produce lint warnings. Both are easy one-line fixes and don't represent functional bugs.
  • src/components/flow/detail-panel.tsx — lines 157 and 223

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["User types in Component Key input"] --> B["handleKeyChange(raw)"]
    B --> C["sanitized = raw\n.replace spaces→_\n.replace invalid chars\n.prefix leading digit"]
    C --> D{sanitized\nnon-empty?}
    D -- "No (e.g. '!!!')" --> E["setDisplayKey(storeKey)\n(revert to last saved key)"]
    D -- "Yes" --> F["setDisplayKey(raw) ⚠️\n(should be sanitized)"]
    F --> G["updateNodeKey(id, sanitized)\n→ Zustand store"]
    G --> H["storeKey changes\n→ useEffect fires"]
    H --> I["setDisplayKey(storeKey)\n(snaps to sanitized — causes flicker)"]
    I --> J["Input shows sanitized value"]
    E --> J
Loading

Comments Outside Diff (2)

  1. src/components/flow/detail-panel.tsx, line 157 (link)

    displayKey set to raw instead of sanitized

    setDisplayKey(raw) stores the unsanitized string (e.g. "Traefik Logs") into React state, while updateNodeKey writes the sanitized version ("Traefik_Logs") to the Zustand store. Because storeKey then changes, the useEffect on line 128 fires after the browser paints and calls setDisplayKey(storeKey), snapping the input back to the sanitized value. This produces a visible flicker on every keystroke that involves a sanitizable character (spaces, hyphens, etc.) — the input briefly shows the raw text, then jumps to the sanitized form.

    Since the intended UX (per the PR description) is for the input to show the sanitized value immediately, displayKey should be set to sanitized:

  2. src/components/flow/detail-panel.tsx, line 223 (link)

    Unused componentKey destructuring

    componentKey is destructured from selectedNode.data here but is no longer referenced anywhere in the component — the Input's value now uses displayKey instead. With strict TypeScript/ESLint this will likely produce a no-unused-vars warning. It can simply be removed from the destructuring:

Last reviewed commit: 79a4e71

Comment on lines 144 to 150
if (selectedNodeId) {
updateNodeKey(selectedNodeId, key);
const sanitized = raw
.replace(/\s+/g, "_")
.replace(/[^a-zA-Z0-9_]/g, "")
.replace(/^(\d+)/, "_$1");
updateNodeKey(selectedNodeId, sanitized);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty string written to store when all characters are stripped

If the user types a value composed entirely of characters that are removed by the second replace step (e.g. "!!!"), sanitized becomes "" and updateNodeKey(selectedNodeId, "") is called. This stores an empty component key, which will silently fail Zod validation only at save time, potentially leaving the user with a broken node state.

Adding a guard prevents this silent bad state:

Suggested change
if (selectedNodeId) {
updateNodeKey(selectedNodeId, key);
const sanitized = raw
.replace(/\s+/g, "_")
.replace(/[^a-zA-Z0-9_]/g, "")
.replace(/^(\d+)/, "_$1");
updateNodeKey(selectedNodeId, sanitized);
}
const sanitized = raw
.replace(/\s+/g, "_")
.replace(/[^a-zA-Z0-9_]/g, "")
.replace(/^(\d+)/, "_$1");
if (sanitized) updateNodeKey(selectedNodeId, sanitized);

@TerrifiedBug TerrifiedBug merged commit 7955a1b into main Mar 5, 2026
9 checks passed
@TerrifiedBug TerrifiedBug deleted the fix/component-key-ux branch March 5, 2026 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant