- Goal: Tighten contracts, typing, and observability around the shared node execution pipeline without changing current behavior.
- What:
- Introduce a narrower
TemplateContext(or similar) forevalTemplateinworkflow/WorkflowExecution/Core/execution/inputs.tsso only the maps actually read byreplaceIndexedInputsare required, avoiding broadIndexedExecutionContext | Partial<IndexedExecutionContext>casts. - Document mode-specific guarantees for workflow vs single-node execution (e.g., presence of
cliMetadata, scheduler usage, workspace context) nearNodeDispatchand the LLM/CLI handlers to make responsibilities and differences between modes explicit. - Consider routing all workspace-root concerns through explicit execution-context data rather than ad-hoc resolution in handlers if the architecture moves further toward data-first execution contexts.
- Clean up unused timeout constants and other dead symbols around LLM execution now that timeout policy lives solely inside
runLLMCore, keeping the source of truth clear. - Decide whether all user-facing templated strings and conditions should go through
evalTemplateor whether some call sites intentionally usereplaceIndexedInputsdirectly, and document that decision so future changes to templating semantics remain predictable. - Optionally unify logging prefixes in LLM runners and attachment resolution so logs clearly identify node id, label/title, and mode across both workflow and single-node paths.
- Add focused unit tests for
runLLMCore,runCLICore, andevalTemplateto lock in behavior for model resolution, approval flows, timeouts, exit-code handling, and template substitution.
- Introduce a narrower
- Why: Keeps the core execution model explicit and type-safe, improves diagnosability in complex graphs, and makes future refactors to templating or execution contexts safer without increasing complexity.
- Goal: Improve robustness, documentation, and consistency for OpenRouter model display formatting.
- What:
- Add input validation for empty model IDs in
getOpenRouterDisplayTitle()to prevent producingOR:with no model name. - Update README.md to document the new display format for OpenRouter models.
- Ensure consistency with other provider prefixes (e.g.,
W:) by checking if the transformation logic should be generalized or kept isolated. - Consider adding unit tests for
getOpenRouterDisplayTitle()when test infrastructure is introduced.
- Add input validation for empty model IDs in
- Why: Prevents silent failures, improves user-facing documentation, and maintains consistent prefix handling across providers, making the feature more maintainable.
-
Goal: Tighten SDK contract assumptions, typing, and reuse around the new per-node
systemPromptTemplatefeature. -
What:
- Adjust the
createAmp(...)call sites in ExecuteSingleNode.ts and run-llm.ts sosystemPromptTemplateis only included in the options object when defined, avoiding reliance on the SDK treating a present-but-undefinedproperty as "no override". - Narrow LLM execution branches to the typed
LLMNodeshape (or itsdatasubset) instead of using(node as any).data.systemPromptTemplate, so accesses tosystemPromptTemplateand other LLM-specific fields are type-safe and resilient to future refactors; update both single-node and workflow runners. - Extract a small shared helper on the extension side (e.g.,
normalizeSystemPrompt(template: unknown): string | undefined) to centralize trimming/"empty means no override" behavior used when wiringsystemPromptTemplateintocreateAmp(...), and mirror or intentionally duplicate minimal logic on the webview side as needed. - Remove the unnecessary
as anycast on theonUpdatecall forsystemPromptTemplatein LLMProperties.tsx so the properties panel relies on the existingPartial<LLMNode['data']>typing. - Optionally enrich the "System prompt override" UX by showing a short first-line summary of the override in the sidebar and/or a subtle indicator on the LLM node card when a custom system prompt is active, keeping the behavior simple but more discoverable.
- Adjust the
-
Why: Ensures the per-node system prompt override remains robust against SDK contract changes, improves type safety and maintainability in core execution paths, and provides a clear foundation for future UX polish without introducing complexity.
-
Goal: Refine alias behavior and webview tool listing now that tool-name metadata is sourced from the Amp SDK.
-
What:
-
Clarify or adjust alias precedence between SDK and NebulaFlow-local aliases in toolNames.ts, documenting that SDK resolution wins when both define the same alias (e.g.,
GitDiff). -
Decide whether to keep or remove redundant local aliases that duplicate SDK entries (such as
GitDiff), or clearly mark them as defensive back-compat shims. -
If future UX requires hiding or reordering tools in the LLM node "Tools" selector, introduce a thin filtering/ordering layer on top of the SDK-provided
getAllToolNames()output so the UI retains control without forking canonical metadata.- Consider normalizing any legacy
disabledToolsentries that still contain raw aliases (for example,bashinstead ofBash) to canonical tool names when workflows are loaded in the webview, and document that behavior so UI state and backend normalization stay aligned for older flows.
- Consider normalizing any legacy
-
Why: Keeps alias semantics explicit and maintainable as the SDK evolves, avoids silent override assumptions, and preserves flexibility to shape the UI tool list while still using the SDK as the single source of truth.
Recommended improvements and optimizations for future implementation.
- Goal: Capture UX, accessibility, and state-management follow-ups for the new builtin tools accordion and master checkbox in the LLM Property Editor.
- What:
- Refine accordion header interaction so toggling the master checkbox or its label does not unintentionally expand/collapse the section (for example, by stopping propagation on the label or separating the expand/collapse trigger from the checkbox area) to keep bulk-enable/disable actions predictable.
- Consider using the shadcn
CheckedStateindeterminate mode for the master checkbox when only some tools are disabled, so partial-selection state is visually communicated instead of appearing simply unchecked. - Revisit keyboard and focus behavior inside the accordion header to ensure the nested checkbox and the trigger work intuitively for keyboard users, potentially separating the trigger region (chevron/summary) from the checkbox region for clearer tab stops.
- Why: Improves the ergonomics and accessibility of the builtin tools accordion while keeping the current
disabledToolsdata model and safety semantics unchanged.
- Goal: Preserve full node state semantics and improve robustness for assistant timelines, thread IDs, and clipboard payloads without changing current UX.
- What:
- Preserve the richer
NodeSavedStateshape when building clipboardnodeResults(status, error, andtokenCount) or explicitly document that clipboard state intentionally only carries outputs. - Tighten
nodeAssistantContentvalidation inisWorkflowStateDTOso each item is structurally compatible withAssistantContentItem(at minimum: an object with a stringtypefield, or ideally via a dedicatedisAssistantContentItemguard). - Decide whether assistant timelines are treated as immutable; if mutation is or may become necessary, use a deep copy (for example,
structuredCloneor a JSON round-trip) instead ofitems.slice()when pasting to avoid shared nested references between original and copied nodes. - Monitor and, if needed, bound clipboard payload size for long LLM histories (for example, truncating to the most recent N messages or logging when payloads exceed a threshold), and document that full assistant history is currently copied.
- Clarify whether copied nodes should share the original LLM
threadIDor fork into a new backend thread; if independent forks are desired, design a thread-cloning flow that still rehydrates the visible timeline. - Replace
any[]uses aroundnodeAssistantContentand clipboardassistantEntrieswithAssistantContentItem[]-based types to surface mismatches at compile time. - Route new copy/paste logging through a dev-guarded logger or flag so production consoles are not flooded with payload diagnostics by default.
- Consider extracting a small helper for hydrating
WorkflowStateDTOinto the webviewMapstructures and reuse it for bothworkflow_loadedand clipboard paste paths to keep future state fields in sync.
- Preserve the richer
- Why: Keeps cross-workflow copy/paste behavior semantically accurate as the protocol evolves, hardens the clipboard path against malformed or huge payloads, and improves type safety and maintainability around assistant content and thread tracking.
- Goal: Harden workspace-level Amp/OpenRouter configuration semantics and reduce noise while keeping current behavior.
- What:
- Add runtime type guards around
internal.primaryModelwhen reading from.nebulaflow/settings.json(both in the LLMIntegration model loader and the LLM runner) so only non-empty strings are accepted and malformed values are ignored instead of throwing. - Add runtime type guards for
openrouter.modelsconfiguration: Introduce validation to ensure each model entry contains the requiredmodelfield (string) and properly handle optional fields likeprovider,maxOutputTokens,contextWindow,isReasoning, andreasoning_effortwithout breaking if malformed. - Decide whether workspace-level
amp.dangerouslyAllowAllshould be able to override node-level Bash gating; if not, clear or ignore this flag in merged settings when Bash tools are disabled so the safety posture remains consistent. - Treat missing
.nebulaflow/settings.json(ENOENT) as a non-warning condition inreadAmpSettingsFromRoot, reserving warnings for malformed JSON or structurally invalid configs to keep dev logs focused on real misconfigurations. - Optionally cache parsed workspace settings per process (or per settings path) to avoid repeated disk reads on every
get_modelsand LLM node run, keeping the implementation simple but reducing I/O churn in large workflows. - Consider a more compact or highlighted label for the synthetic "Workspace default" model entry (and optional group-top ordering) so it remains visible and readable in narrow model selectors.
- Tighten the helper’s return type by introducing a lightweight
AmpSettingstype (e.g., aPartialwith known keys like'internal.primaryModel'and'openrouter.key') while still allowing forward-compatible keys via an index signature.
- Tighten the helper’s return type by introducing a lightweight
- Why: Makes workspace-scoped LLM configuration more robust to user error, clarifies how global allow-all interacts with per-node safety toggles, keeps logs and I/O efficient, and improves type safety and UI clarity around workspace defaults.
- Goal: Tighten robustness and semantics around the Electron workspace folder feature and storage-scope signaling without changing current behavior.
- What:
- Wire a consumer for the
refresh_custom_nodesmessage (or remove the unused event) so Electron’s Open Folder… action either triggers a real custom-node refresh or avoids sending no-op signals. - Add lightweight error logging around
ElectronWorkspaceconfig load/save (loadConfig/saveConfig) while still falling back to safe defaults, making corrupted or unwritable config states diagnosable. - Centralize storage-scope + base-path computation in a shared helper so
storage_scopeevents always expose the resolved filesystem root instead of a raw, potentially invalidnebulaFlow.globalStoragePathstring. - Optionally validate and await
setWorkspaceFolderwrites (existence, directory check, and serializedsaveConfigcalls) to avoid persisting bad paths or interleaving config writes. - Consider clarifying Electron’s default workspace semantics (home directory vs. “no workspace”) and deduplicating window-title update logic and
os.homedir()calls behind a tiny helper to keep title behavior consistent.
- Wire a consumer for the
- Why: Ensures the new Electron workspace behavior is transparent and diagnosable, avoids drift between backend storage roots and UI
storage_scopestate, and keeps the host abstraction clean as the Electron flavor evolves.
-
Goal: Restore conservative safety posture and reduce duplication around link handling, custom node operations, IPC lifetime, and resume subgraph computation.
-
What:
- Tighten
open_external_linkbehavior so VS Code returns early with a clear warning when links resolve outside the active workspace, and optionally align Electron with a workspace-only policy (or document a deliberate exception) to avoid silently opening arbitrary local files. - Reintroduce confirmations for custom-node overwrite, delete, and rename operations by extending the host window abstraction with a warning/confirmation API, keeping accidental destructive actions harder in both VS Code and Electron.
- Ensure Electron IPC listeners created by
IMessagePortare disposed alongside session cleanup so repeated window open/close cycles do not accumulateipcMainhandlers; track and call the returned disposable duringcleanupSession. - Refine
ElectronWindow.showOpenDialogto build itspropertiesarray from the supplied options (canSelectFiles,canSelectFolders,canSelectMany) so future callers can request folder-only or multi-select dialogs without unexpected file-picker behavior. - Decide and document whether Electron workspace configuration remains flat string-keyed or should normalize into a nested object tree to better mirror VS Code’s
getConfigurationsemantics, avoiding surprises for future settings callers. - Gate or remove debug logging around workspace roots in LLM runners so production logs stay quiet unless explicitly running in a development context, while still allowing targeted diagnostics when needed.
- Extract the resume pruning and seed-filtering logic into a shared pure helper (e.g.,
computeResumeSubgraph) reused byworkflow-sessionand any other runners that manipulate seeds to keep resume behavior consistent and easier to evolve.
- Tighten
-
Why: Brings link and file operations back in line with previous safety expectations, prevents subtle resource leaks in Electron, and centralizes shared algorithms so future behavior changes remain consistent across hosts and execution paths.
-
Goal: Clarify and harden behavior around the default GPT-5.1 model for legacy workflows and provider-specific behavior.
-
What:
-
- Define and document explicit upgrade semantics for workflows that lack an LLM
model(e.g., always migrate to the current default GPT-5.1 while preserving explicit legacy IDs) nearnormalizeModelsInWorkflowin fs.ts and/or slice-level docs.
- Define and document explicit upgrade semantics for workflows that lack an LLM
-
- Consider a small compatibility map (e.g., mapping
anthropic/claude-sonnet-4-5-20250929toopenai/gpt-5.1where appropriate) if product direction calls for automatic migration from legacy Anthropic defaults to GPT-5.1.
- Consider a small compatibility map (e.g., mapping
-
- Add lightweight tests or assertions to ensure all execution paths use the shared default model constants from default-model.ts rather than hard-coded literals, preventing drift when the default changes.
-
- Validate GPT-5.1 behavior under the existing
computeLLMAmpSettingsconfiguration (token limits, reasoning effort mapping, etc.) and adjust settings only if provider-specific differences require it.
- Validate GPT-5.1 behavior under the existing
-
- Confirm that telemetry, cost estimation, and any model-related configuration or documentation reflect the switch from Anthropic Sonnet to OpenAI GPT-5.1.
-
Why: Makes the GPT-5.1 default behavior explicit for both new and legacy workflows, keeps all call sites aligned on a single source of truth, and reduces risk from provider-specific differences or future default changes.
- Goal: Tighten typing, safety posture, and add tests after the large-file refactor.
- What:
- Typed CustomEvents: Replace string-typed CustomEvents in effect hooks with typed constants and payloads; extend
WindowEventMapand define payload interfaces fornebula-edit-node,nebula-run-from-here,nebula-run-only-this, subflow events. Affects useEditNode.ts, useRunFromHere.ts, useRunOnlyThis.ts, and subflow hooks. - CLI safety posture: Denylist-based guard remains in place; consider moving to an allowlist or capability-scoped execution model for higher assurance. Track in Core execution sanitize.ts.
- Unit tests for Core helpers: Add unit coverage for pure functions in Core/execution, e.g., inputs.ts, combine.ts, sanitize.ts, and graph.ts.
- Minor polish: Deduplicate
HANDLE_THICKNESSacross LeftSidebarContainer.tsx and RightSidebarContainer.tsx; forward and/or remove unused props (onRunFromHere) in RightSidebarContainer.tsx; consideruseCallbackon small inline handlers where beneficial (e.g., FlowCanvas.tsx).
- Typed CustomEvents: Replace string-typed CustomEvents in effect hooks with typed constants and payloads; extend
- Why: Improves type safety and reliability, strengthens security posture over time, and adds confidence via tests without increasing complexity.
- Priority: P2 (typing/robustness)
- Goal: Polish typing, reuse, and small edge cases for the new modal switching UX.
- What:
- Add typed custom events to the global
WindowEventMapfornebula-open-result-editorand reuse the existing typed shape fornebula-edit-nodeto removeanycasts in RightSidebar.tsx. - Disable or guard the "Save" action when input is empty or unchanged in TextEditorModal.tsx to reduce accidental saves; keep semantics simple.
- Extract a small helper (e.g.,
openResultEditorForNode(id: string)) to dispatchnebula-open-result-editorand reuse it across nodes: CLI_Node.tsx, LLM_Node.tsx, Text_Node.tsx, Variable_Node.tsx. - Optionally handle re-opening the same preview id by toggling
previewNodeIdthroughnullbefore setting it again in RightSidebar.tsx to force a fresh open when needed.
- Add typed custom events to the global
- Why: Improves type safety, removes duplication, and polishes interaction edge cases without increasing complexity.
- Priority: P2 (polish/typing)
- Goal: Tighten typing, accessibility, and edge-case behavior for the new Accumulator text editing flow while keeping current UX and contracts.
- What:
- Introduce a shared
EditEventDetailtype (andWindowEventMapentry) for thenebula-edit-nodepayload and reuse it acrossuseEditNodeand all node components (Accumulator, Text, LLM, CLI, Variable) to eliminateanyindispatchEditEventand strengthen compile-time guarantees. - Align Accumulator canvas accessibility with the Text node by adding
role,tabIndex, and a keyboard-accessible trigger for editing (e.g., Enter/Space on a focused content area), so double‑click editing is discoverable without a mouse. - Consider initializing the Accumulator sidebar modal draft from the textarea’s current value (or ensuring
onUpdateis always synchronous locally) to avoid the rare stale-value case when users double‑click immediately after typing. - Clean up the "Input Text" label/id pairing in AccumulatorProperties.tsx so
htmlForand the textareaidmatch, improving a11y and aligning with the rest of the sidebar.
- Introduce a shared
- Why: Brings the new Accumulator editing path up to the same type-safety and a11y bar as existing nodes, and addresses minor edge cases called out in review without altering the current behavior.
- Goal: Small polish items to keep imports and aliases consistent and ergonomic.
- What:
- Normalize import style in the one remaining file noted in the review to match the alias-first pattern (prefer alias imports over deep relative paths consistently).
- Optional alias polish: consider adding an alias for
servicesif future refactors increase cross-feature usage; keep unused aliases out of config to avoid clutter.
- Why: Maintains consistency after the slice/alias migration and keeps config lean.
- Priority: P3 (polish)
- Goal: Finish the transition to slice-first routing and reduce duplication/maintenance risk.
- What:
- Guard duplicate router registrations and remove dead legacy switch cases that are now handled by slices; the
load_workflowstub in register.ts can be removed once consumers are fully on the slice path. - Centralize common slice wiring types (
SliceEnv,Router) and small helpers (e.g.,readStorageScope) under Shared to avoid copy/paste across slice registers. A good home would beworkflow/Shared/Infrastructurewith named exports; update imports in Library register, Subflows register, and WorkflowPersistence register. - Add consistent
try/catcherror guards to Library CRUD handlers to match the defensive pattern used in Subflows and WorkflowPersistence; see Library register. - Normalize
safePostusage with minimal error handling for symmetry across slices (log or user warning on failure where appropriate). - Consider dropping explicit ".js" extensions in TS import specifiers in slice files to reduce bundling brittleness across tooling; verify current bundler settings resolve extensionless paths reliably.
- Guard duplicate router registrations and remove dead legacy switch cases that are now handled by slices; the
- Why: Prevents drift between legacy and slice paths, reduces boilerplate across slices, and keeps error handling and module resolution consistent.
- Goal: Keep the Shell (CLI) advanced sidebar section maintainable and improve small UX/a11y details without changing behavior.
- What:
-
- Optionally factor the advanced area in CLIProperties.tsx into small presentational subcomponents (Shell flags, Stdin advanced, Env advanced, Safety advanced) to reduce the size and complexity of the main CLI properties component.
-
- Add a small "Advanced options" heading above the grouped advanced controls and/or wrap them in a labeled group (fieldset/legend or ARIA region) so screen readers and sighted users see a clear section boundary.
-
- Surface lightweight validation feedback for the "Static env (JSON object)" editor instead of silently ignoring invalid JSON (for example, an inline error state while the last parse failed), keeping behavior simple.
- Why: Improves readability and long-term maintainability of the Shell node properties panel while making the advanced section clearer and more accessible, without altering the underlying
CLINodeConfigcontract.
- Goal: Refine modal editing ergonomics and typing for the Shell (CLI) node without changing behavior.
- What:
- Ensure a safe string fallback for the Command input value in PropertyEditor.tsx to avoid uncontrolled→controlled warnings when
contentis undefined. - Clear the local
draftstate after commit/cancel in CLI_Node.tsx to prevent stale values on subsequent edits. - Type the
nebula-edit-nodepayload explicitly in CLI_Node.tsx for stronger compile-time safety.
- Ensure a safe string fallback for the Command input value in PropertyEditor.tsx to avoid uncontrolled→controlled warnings when
- Why: Improves robustness (no React console warnings), prevents subtle UX nits with stale drafts, and strengthens type safety with minimal code.
- Priority: P3 (polish)
- Goal: Provide true live streaming of process output to the UI and improve input validation feedback.
- What:
- Implement buffer-by-buffer streaming from spawn to the webview so users see output as it arrives (maintain truncation guard).
- Surface JSON parse errors for Static Env editor inline (with a small error state) instead of silently ignoring invalid JSON.
- Clarify parent-index stdin semantics: validate out-of-range indices and allow choosing 0- or 1-based indexing explicitly; show a brief helper hint.
- Unify shell imports (named vs dynamic) across workflow/single-node handlers to keep style consistent.
- Why: Aligns UX with terminal expectations, prevents silent misconfiguration, and keeps code style consistent.
- Priority: P2 (UX/robustness)
- Goal: Consolidate keyboard handling and confirm focus reachability after handler relocation.
- What:
- Move Enter-key deselection logic exclusively to
handleBackgroundKeyDownand remove duplication fromhandleBackgroundClickin selectionHandling.ts. - Verify keyboard focus is intuitive when the inner canvas holds
role/tabIndexand the outer wrapper no longer does, especially on narrow layouts in Flow.tsx.
- Move Enter-key deselection logic exclusively to
- Why: Reduces duplication and ensures accessible, predictable keyboard navigation after scoping handlers to the canvas.
- Priority: P2 (robustness/a11y)
- Goal: Make the Playbox assistant/result deduplication more robust and less coupled to backend string formatting while keeping current behavior.
- What:
- Reconcile multi-block assistant messages by reconstructing the final assistant text from the timeline using the same rules as
run-llm.ts(or, preferably, by using a structural identifier from the backend) so dedup works even when the last assistant turn is emitted as multiple text blocks. - Relax equality checks beyond
.trim()(for example, collapse internal whitespace and/or lowercase both sides) to tolerate harmless formatting differences between the timeline and Result string while still avoiding obvious false positives. - Ensure the hidden assistant text is truly the last user-visible assistant element (not just the last
type === 'text'item) so potential future non-text assistant blocks do not cause the wrong item to disappear. - Consider passing a small piece of structural metadata (e.g., the assistant message id used for
finalText) from the backend to the webview and using it to select which assistant item(s) should be hidden, reducing coupling to string assembly details. - Add a tiny early-return guard for empty assistant timelines and keep
pairedMapconstruction aligned with whatever filteringgetAssistantDisplayItemsapplies if future versions also hide tool-related items.
- Reconcile multi-block assistant messages by reconstructing the final assistant text from the timeline using the same rules as
- Why: Improves UX predictability for multi-block or slightly reformatted assistant answers and keeps the dedup logic resilient to backend formatting changes without increasing complexity.
- Goal: Tighten abort/draft semantics, history sizing, contracts, and accessibility for the new per-node LLM chat continuation feature without changing current behavior.
- What:
-
- Decide and document whether aborting a chat turn or full execution should preserve or clear
nodeThreadIDsand PlayboxchatDrafts; if a clean-slate UX is preferred for aborts, clear these maps onexecution_completedwhen the final status isinterrupted/error.
- Decide and document whether aborting a chat turn or full execution should preserve or clear
-
- Make the assistant-vs-Result duplication filter conditional so non-chat LLM runs can still hide a duplicate last assistant text in the Playbox when there is only a single assistant message and no
threadID, while chat-enabled nodes (with athreadID) always show the full assistant timeline.
- Make the assistant-vs-Result duplication filter conditional so non-chat LLM runs can still hide a duplicate last assistant text in the Playbox when there is only a single assistant message and no
-
- Consider adding a soft cap (by message count or character length) or a manual "Clear chat history for this node" affordance to keep
nodeAssistantContentandnodeThreadIDsfrom growing unbounded over very long chats.
- Consider adding a soft cap (by message count or character length) or a manual "Clear chat history for this node" affordance to keep
-
- Either remove the unused
modefield fromLLMNodeChatCommandand its guards, or start honoringmode: 'workflow'explicitly in the extension handler with a small inline comment explaining current support; keep the protocol and implementation aligned.
- Either remove the unused
-
- Add an explicit accessible label for the Playbox chat textarea (e.g.,
aria-labeloraria-labelledbytied to the "Chat" heading) and consider allowing limited vertical growth or resize behavior so longer follow-ups remain readable while preserving the Ctrl/Cmd-Enter shortcut hint in the UI.
- Add an explicit accessible label for the Playbox chat textarea (e.g.,
-
- Optionally harden the
llm_node_chathandler by re-checkingpauseRequestedimmediately before invokingexecuteLLMChatTurn, treating a newly-set pause as an immediate interrupt for chat-only executions.
- Optionally harden the
-
- Add a focused integration or slice-level test that sends
llm_node_chat, verifies the existingthreadIDis reused by the runner, and asserts that assistant content events arrive with the samethreadIDin the webview handler.
- Add a focused integration or slice-level test that sends
-
- Keep external protocol docs and any client libraries in sync with the
llm_node_chatshape, including guidance onthreadIDusage and (if retained) themodefield.
- Keep external protocol docs and any client libraries in sync with the
- Why: Clarifies abort semantics for chat, prevents subtle state drift or unbounded growth for long-running conversations, aligns protocol contracts with the actual implementation, and raises the a11y/UX bar for the new Playbox chat experience without adding complexity.
- Goal: Standardize containment and add an extra safety stop.
- What:
- Add
onMouseDown={e => e.stopPropagation()}on the right sidebar panel to prevent mouse-down from bubbling, matching click/key containment in Flow.tsx. - Choose a single strategy (stopPropagation vs target guards) and apply it consistently for background deselection and sidebar containment.
- Add a narrow type guard in selection handlers to reduce casts when refining
MouseEvent/KeyboardEventbranches in selectionHandling.ts. - Confirm/document Shift+click semantics on empty canvas (currently a no-op that keeps selection) in selectionHandling.ts.
- If/when tests exist, consider coverage for sidebar/canvas Shift/Enter containment behavior.
- Add
- Why: Further reduces accidental deselection and keeps containment behavior uniform across inputs.
- Priority: P3 (polish/consistency)
- Goal: Harden and polish the open/provide subflow pipeline without changing core behavior.
- What:
- Add a typed
CustomEvent<{ subflowId: string }>alias for the open event and a sibling alias for provide to removeanyand improve maintainability. - Add minimal dev-only logging or a user notification when
postMessagecalls throw, to aid diagnosis without spamming users. - Consider centralizing the deep snapshot logic into a tiny
cloneGraph(nodes, edges)helper to guarantee deep copies and avoid drift across call sites. - Optional micro-optimization: coalesce ref-sync effects (nodes/edges/activeSubflowId) if profiling shows reflow pressure.
- Optional: subtle visual hint (e.g., a brief highlight) when opening a subflow during execution to acknowledge the action.
- Add a typed
- Why: Improves type safety and debuggability, keeps snapshots consistent, and adds small UX clarity without increasing complexity.
- Priority: P3 (robustness/UX polish)
- Goal: Tighten typing for subflow-scoped webview events and centralize deep cloning used on subflow open.
- What:
-
- Replace
anypayloads in subflow message handlers with exact Protocol types by importing the discriminated message contracts in messageHandling.ts.
- Replace
-
- Introduce a small
cloneGraph(nodes, edges)that prefersstructuredClonewith a JSON fallback to replace ad-hoc deepClone usage in Flow.tsx.
- Introduce a small
-
- Optional: route dev logs through a minimal logger with levels behind a dev flag to keep console noise controlled as the surface grows.
- Why: Improves type safety and maintainability for subflow live updates, and ensures consistent, safe snapshotting when opening subflows mid-run.
- Priority: P2 (robustness/maintainability)
- Goal: Prevent future drift in Bash detection and reduce runtime overhead in tool name normalization.
- What:
- Extract a shared helper
isBash(name: string): booleanused by both settings and runtime guards to centralize case/trim normalization. - Remove unnecessary
as stringcast in normalization path in llm-settings.ts to tighten types. - Cache SDK resolver availability from
safeRequireSDK()in llm-settings.ts to avoid repeated try/catch on frequent normalization calls. - Why: A single source of truth for Bash checks prevents subtle mismatches; minor type cleanup improves safety; caching avoids unnecessary overhead during frequent operations.
- Priority: P3 (code quality/robustness)
- Goal: Align LoopStart while-mode UX, typing, and accessibility with the underlying execution semantics without changing behavior.
- What:
-
- Clamp
maxSafeIterationsvalues to a minimum of 1 in the Loop Start sidebar (or otherwise reflect the effective default) so entering0or a negative number cannot silently resolve to the runtime fallback of 100 iterations.
- Clamp
-
- Replace structural event typings like
{ target: { value: any } }inLoopStartProperties.tsxwithReact.ChangeEvent<HTMLInputElement>(or a shared alias) to restore type safety and editor autocomplete for sidebar inputs.
- Replace structural event typings like
-
- Add
aria-pressed(and, optionally, a small ARIA group) to theLoop Modesegmented buttons so screen readers recognize the active mode, and consider centralizing the'while-variable-not-empty'literal behind a shared constant/union reused by model, runner, and UI.
- Add
- Why: Prevents surprising max-iteration behavior for users, strengthens type safety around sidebar inputs, and improves accessibility and maintainability of the LoopStart loop-mode UI while keeping runtime semantics unchanged.
- Goal: Document and harden hybrid segment dependency semantics and pause behavior without changing current runtime semantics.
- What:
-
- Add brief inline documentation in
runParallelSegmentdescribing how parents outside the segment but insidenodeIndexare treated as upstream dependencies, while parents outsidenodeIndexare assumed satisfied via seeds, so future maintainers understand the intended behavior.
- Add brief inline documentation in
-
- Introduce targeted tests for resume-from-node and resume-from-pause scenarios over graphs that combine loops and IF/ELSE nodes, ensuring children of out-of-subgraph parents still execute correctly under the hybrid scheduler.
-
- Consider a small comment or docs note near the seeds/resume wiring in
workflow-session.tsto keep the subgraph/seed assumptions discoverable.
- Consider a small comment or docs note near the seeds/resume wiring in
-
- Why: Makes the hybrid scheduler’s dependency model explicit, reduces the risk of accidental regressions in complex resume flows, and keeps behavior around pruned subgraphs and seeds easier to reason about.
+### Hybrid Loop-Aware Parallel Execution – Follow-ups + +- Goal: Document and harden hybrid segment dependency semantics and pause behavior without changing current runtime semantics. +- What:
-
- Add brief inline documentation in
runParallelSegmentdescribing how parents outside the segment but insidenodeIndexare treated as upstream dependencies, while parents outsidenodeIndexare assumed satisfied via seeds, so future maintainers understand the intended behavior.
- Add brief inline documentation in
-
- Introduce targeted tests for resume-from-node and resume-from-pause scenarios over graphs that combine loops and IF/ELSE nodes, ensuring children of out-of-subgraph parents still execute correctly under the hybrid scheduler.
-
- Consider a small comment or docs note near the seeds/resume wiring in
workflow-session.tsto keep the subgraph/seed assumptions discoverable. +- Why: Makes the hybrid scheduler’s dependency model explicit, reduces the risk of accidental regressions in complex resume flows, and keeps behavior around pruned subgraphs and seeds easier to reason about.
- Consider a small comment or docs note near the seeds/resume wiring in
+- Goal: Improve pause granularity and observability for loop bodies over time while preserving current behavior. +- What:
-
- Explore adding optional
pausechecks insiderunLoopBlockso long-running loop bodies can honor pause requests between iterations, mirroring thePausedErrorpattern used by the parallel scheduler.
- Explore adding optional
-
- Consider a low-noise debug or telemetry hook emitted when loop bodies are running under the sequential block runner to make pause limitations and performance characteristics more diagnosable. +- Why: Keeps loop execution semantics explicit and makes it easier to evolve toward finer-grained pause points without surprising existing workflows.
+- Goal: Reduce small redundancies and duplication between runParallelWholeGraph and runParallelSegment while keeping the scheduler simple.
+- What:
-
- Optionally extract shared helpers for concurrency bookkeeping and inflight management so the global and per-segment schedulers share a single implementation of the core scheduling mechanics.
-
- Revisit how
loopInternalIdsare populated (single pass vs. per-loop extension) and clarify the intended strategy in code comments to avoid confusion for future maintainers.
- Revisit how
-
- Add a brief note around
UNSUPPORTED_TYPES_V1describing how and when to extend it if new node types require an explicit opt-out from parallel execution. +- Why: Improves maintainability and readability of the parallel scheduler without altering current semantics or public APIs.
- Add a brief note around
- Goal: Harden loop-aware graph composition and sidebar behavior with tests and small UX improvements without changing the current runtime semantics.
- What:
-
- Add focused unit tests around
processGraphCompositioninworkflow/WorkflowExecution/Core/engine/node-sorting.tsfor loop-only graphs, mixed loop + independent components, and no-loop graphs to lock in the new display-mode semantics and guard against ordering regressions.
- Add focused unit tests around
-
- Clarify (in code comments or slice docs) that non-loop nodes appended after
processLoopWithCyclesare intended to represent disconnected components, and document the ordering trade-offs when edges exist between loop and non-loop segments.
- Clarify (in code comments or slice docs) that non-loop nodes appended after
-
- Confirm with product/UX whether suppressing the Playbox parallel view whenever loop nodes are active is the desired long-term behavior or an interim safeguard, and, if kept, surface a small inline note or tooltip explaining why the parallel analysis is disabled for loop-containing workflows.
-
- Optionally emit a low-noise debug or telemetry signal in the webview when
hasParallelAnalysis && hasLoopNodesbut the parallel view is suppressed, to make the gating behavior diagnosable in complex graphs.
- Optionally emit a low-noise debug or telemetry signal in the webview when
-
- Extract a small shared
isLoopNodehelper (e.g., in a shared models/util module) and replace repeatednode.type === NodeType.LOOP_START || node.type === NodeType.LOOP_ENDpredicates innode-sorting.ts,parallel-analysis.ts, andRightSidebar.tsxto keep loop-node detection centralized.
- Extract a small shared
- Why: Improves correctness confidence for loop-aware composition, makes the sidebar’s parallel/sequence trade-offs explicit to users, and reduces drift by centralizing loop-type checks, all without altering the existing execution behavior.
- Goal: Tighten typing and diagnostics around the new
clear_workflowprotocol message without changing behavior. - What:
-
- Replace
as anyusage when postingclear_workflowfrom the webview with the shared, typed protocol contract (for example, usingWorkflowToExtensionor a dedicatedClearWorkflowCommandtype) so message payloads stay aligned with the discriminated union.
- Replace
-
- Consider adding minimal dev-only logging or
console.errorwhenvscodeAPI.postMessagethrows for Clear-related messages, keeping production behavior silent while making message pipeline failures diagnosable during development.
- Consider adding minimal dev-only logging or
-
- Optionally tidy the
isWorkflowToExtensionguard by grouping trivialtruecases for commands likereset_resultsandclear_workflowfor readability, while preserving existing semantics.
- Optionally tidy the
- Why: Preserves the simple, additive
clear_workflowbehavior while improving type safety and debuggability of the clear path over time.
-
Goal: Tighten shared contracts, path semantics, and UX around LLM image attachments without changing current behavior.
-
What:
- DRY up
AttachmentRefand related attachment types by importing them fromworkflow/Core/models.tswherever feasible in the webview (e.g.,workflow/Web/components/nodes/Nodes.tsx,LLM_Node.tsx) to avoid duplicate type definitions drifting over time. - Clarify and, if needed, harden
resolveAttachmentFilePathsemantics by usingnode:path.resolveplus a workspace-root prefix check so relative paths cannot escape the active workspace roots unintentionally; document how multi-root workspaces are handled. - Consider expanding path resolution to probe all workspace roots when resolving relative attachment paths (or explicitly document that only the first root is used) so expectations are clear in multi-root setups.
- Add light validation and UX hints in the LLM sidebar properties panel for attachment inputs: basic URL validation (e.g., via
new URL()), optional guidance on how relative file paths are resolved, and an accessible remove button with anaria-label. - Decide whether
AttachmentRef.altTextshould be wired through end-to-end (UI + runner + Amp integration if supported) or removed from the contract until a concrete consumer exists, to keep the model surface focused. - Optionally consolidate repeated
@prinova/amp-sdkrequirecalls and gate new debug logging around attachment resolution andrunJSONLimage counts behind an existing debug/logging flag to keep production logs quiet.
- DRY up
-
Why: Keeps LLM image attachment behavior type-safe and maintainable, makes file/URL semantics and workspace scoping explicit, and polishes the sidebar UX and logging without altering the current feature.
-
Goal: Consolidate sidebar accordion trigger styling and lock in the left-aligned chevron layout over time.
-
What:
-
- Extract a shared
SIDEBAR_ACCORDION_TRIGGER_CLASSES(or similar) helper used by bothWorkflowSidebarandRightSidebarso the Tailwind +.sidebar-accordion-triggercombination lives in one place, reducing duplication if more sidebars adopt the pattern.
- Extract a shared
-
- Add or update visual/screenshot regression coverage for left and right sidebar accordions (library sections, assistant items, per-node results) to assert that chevrons remain visually on the left and headers preserve current spacing.
-
- Document the UX decision that sidebars use the left-chevron pattern via
.sidebar-accordion-trigger, while non-sidebar accordions keep the default layout, so future contributors don’t mix patterns inadvertently.
- Document the UX decision that sidebars use the left-chevron pattern via
-
Why: Keeps the new sidebar chevron layout maintainable and explicit, reduces styling drift across panels, and makes the UX convention clear without changing current behavior.
- Goal: Clarify and harden behavior for the new "while variable is not empty" loop mode without changing current semantics.
- What:
-
- Decide whether
shouldContinueLoopinworkflow/WorkflowExecution/Core/execution/task-list.tsshould remain string-only or be broadened to treat other value shapes (arrays/objects) as non-empty collections, and document the chosen behavior clearly in code and/or UI.
- Decide whether
-
- Refine
maxSafeIterationshandling by validating or clamping non-positive values instead of silently treating0/negative numbers as the default, and surface a brief explanation in the Loop Start properties UI so users understand the safety cap.
- Refine
-
- Document in the Loop Start properties panel and/or inline help that
iterations-overrideinputs only apply whenloopMode === 'fixed'and are ignored forwhile-variable-not-empty, avoiding confusion when users wire overrides into while-mode loops.
- Document in the Loop Start properties panel and/or inline help that
-
- Extend the Loop Start property UI to expose
loopModeselection,collectionVariableinput, and a numericmaxSafeIterationsfield with a short description of its safety role, so the new behavior is discoverable without external tooling.
- Extend the Loop Start property UI to expose
-
- Optionally adjust naming and docs (e.g.,
collectionVariablevs a more precise name likeconditionVariable, file name fortask-list.ts, safety-cap comments nearrunLoopBlock) and consider a low-noise telemetry/log hook when loops terminate due tomaxSafeIterationsto aid future debugging.
- Optionally adjust naming and docs (e.g.,
- Why: Keeps the new while-style loop mode explicit, predictable, and diagnosable as the engine evolves, while avoiding surprises around type assumptions, safety caps, and when overrides do or do not apply.
- Goal: Clarify loop iteration semantics and make the sequential fallback path observable without changing the current behavior.
- What:
- Document that any active Loop Start/End node currently causes the engine to fall back to a sequential executor for the entire graph, and that parallel-specific options (like
concurrencyandperType) are effectively ignored in this mode. - Emit a low-noise debug or telemetry signal when the scheduler detects loop nodes and chooses sequential fallback so performance characteristics are diagnosable in complex workflows.
- Align or explicitly document the relationship between static iteration overrides used at graph-composition time (via
getLoopIterations) and runtime overrides consumed byexecuteLoopStartNode, especially when the override source is dynamic rather than a simple Text node.
- Goal: Make LLM assistant timeline normalization explicit, mode-aware, and strongly typed while preserving the current UX fix.
- What:
-
- Document the invariant that the earliest
user_messagein a thread represents the workflow prompt in current flows, both nearfilterInitialUserMessageinworkflow/Web/components/hooks/messageHandling.tsand (optionally) alongside assistant-content event docs inworkflow/Core/Contracts/Protocol.ts, so future features understand why that item is filtered.
- Document the invariant that the earliest
-
- Consider gating
filterInitialUserMessagebehind the newmodehint (for example, only filtering whenmode !== 'single-node'or when a thread started from a workflow run) if future pure-chat entrypoints are added that start threads without a workflow prompt.
- Consider gating
-
- Introduce a shared
ExecutionMode = 'workflow' | 'single-node'type (and smallisExecutionModehelper) inworkflow/Core/Contracts/Protocol.tsand reuse it across protocol, guards, andLLMRunArgsinstead of repeating the literal union and manual checks.
- Introduce a shared
-
- Tighten the
nodeAssistantContentmap typing in the webview (e.g.,Map<string, AssistantContentItem[]>inmessageHandling.tsand related hooks) to align with the protocol and thefilterInitialUserMessagehelper, reducingany[]usage.
- Tighten the
-
- Clarify in
LLMRunArgsand/or the LLM runner docs thatmodeis a diagnostic/UI hint carried through logs and assistant-content events and does not currently alter Amp SDK behavior, to prevent accidental behavioral divergence between modes.
- Clarify in
- Why: Keeps the sidebar timeline fix robust as new entrypoints and content types are added, reduces duplication and drift around execution modes, and strengthens type safety around assistant content without changing current behavior.
- Goal: Make user messages first-class in the assistant timeline for all content types and keep spacing and rendering logic maintainable.
- What:
-
- Extend
extractAssistantTimelineto log or otherwise surface unhandled usercontentblock types (e.g., images, files) rather than silently dropping them, and decide whether to represent them via newAssistantContentItemvariants once multimodal user input is supported.
- Extend
-
- Optionally filter or avoid emitting empty
user_messageitems whenblock.textis missing to reduce noise in the timeline.
- Optionally filter or avoid emitting empty
-
- Consolidate the duplicated rendering for
textanduser_messageinRightSidebar.tsx(both in the accordion detail view and in the inside-timeline segments) into small shared helpers so typography and spacing stay in sync over time.
- Consolidate the duplicated rendering for
-
- Align vertical spacing rules between assistant and user message blocks (e.g., reuse the
assistantMarginToplogic or a shared spacing helper) so sequences ofthinking,text, anduser_messageitems have consistent rhythm.
- Align vertical spacing rules between assistant and user message blocks (e.g., reuse the
- Why: Prepares the LLM node timeline for future multimodal user content, avoids silently losing information, and keeps the user/assistant block rendering simple and consistent to maintain.
- Goal: Tighten accessibility, small UX semantics, and minor code duplication around the new LLM Prompt card without changing current behavior.
- What:
-
- Make the Prompt card keyboard-activatable by adding
role="button",tabIndex={0}, and anonKeyDownhandler for Enter/Space that triggers the existingnebula-edit-nodestart event, so keyboard and assistive-technology users can edit the prompt from the RightSidebar.
- Make the Prompt card keyboard-activatable by adding
-
- Consolidate prompt detection by computing the trimmed
promptstring once inrenderNodeItemand derivinghasPromptfromprompt.length > 0, avoiding duplicatednode.data.content.trim()logic in the timeline renderer.
- Consolidate prompt detection by computing the trimmed
-
- Optionally gate the RightSidebar Result section on
nodeResults.has(node.id)again so LLM nodes with a prompt but no results don’t show an empty "Result" header, while still mounting the Prompt/timeline container when a prompt exists.
- Optionally gate the RightSidebar Result section on
-
- Consider adding
tw-break-wordsto the Prompt body<p>inRightSidebar.tsxto handle extremely long unbroken tokens (URLs, base64 strings) without stretching the sidebar layout.
- Consider adding
-
- If/when the rest of the UI is localized, route the hard-coded "Prompt" label through the existing i18n mechanism so the card title participates in localization.
- Why: Brings the Prompt card up to the same a11y and polish bar as the rest of the RightSidebar assistant UI, keeps prompt detection logic DRY and predictable, and leaves room for future localization and layout hardening.
- Goal: Prevent future drift in Bash detection and reduce runtime overhead in tool name normalization.
- What:
- Extract a shared helper
isBash(name: string): booleanused by both settings and runtime guards to centralize case/trim normalization. - Remove unnecessary
as stringcast in normalization path in llm-settings.ts to tighten types. - Cache SDK resolver availability from
safeRequireSDK()in llm-settings.ts to avoid repeated try/catch on frequent normalization calls. - Why: A single source of truth for Bash checks prevents subtle mismatches; minor type cleanup improves safety; caching avoids unnecessary overhead during frequent operations.
- Priority: P3 (code quality/robustness)
-
Decide and document whether iteration overrides are intended to be static (node content) or dynamic (runtime output), and adjust validation or configuration constraints accordingly.
-
Consider extracting the embedded
executeWorkflowSequentialimplementation inparallel-scheduler.tsinto a dedicated module for reuse and to avoid future duplication if additional entrypoints require sequential execution.- Clarify and, if needed, adjust semantics for nested loops so inner
LOOP_STARTnodes either drive their ownmaxIterationsvia a nestedrunLoopBlockor are explicitly unsupported and rejected at validation time; add targeted nested-loop tests to lock in the chosen policy. - Decide whether IF/ELSE decisions inside loops should be re-evaluated on every iteration or remain "sticky" via the global
disabledNodesset, then document and test that behavior with a loop containing an IF/ELSE branch. - Confirm the intended behavior for
onError: 'continue-subgraph'when errors occur inside loop bodies or at loop ends (stop further iterations vs. continue remaining iterations), and add a small test matrix to keep future changes aligned with that policy. - Why: Makes loop behavior easier to reason about for users wiring more dynamic overrides, improves diagnosability when sequential fallback affects performance, and reduces long-term maintenance risk around the sequential executor and complex loop shapes (nested loops, branching, and error handling).
- Clarify and, if needed, adjust semantics for nested loops so inner
-
Goal: Keep loop iteration override behavior maintainable and explicit across runtime, scheduler, and UI.
-
What:
- Extract a small shared helper (or tightly documented algorithm) for parsing and clamping iteration override values so the loop runner, scheduler, and webview all rely on the same rules instead of reimplementing them in three places.
- Decide and document clear semantics for multiple
iterations-overrideedges (for example, enforcing a single incoming override edge vs. a "first valid override wins" rule) and surface a gentle UI/validation hint if overrides are miswired. - Clean up helper signatures and typing around iteration overrides (e.g., remove unused parameters like
baseIterationswhen unused, alignstring[] | undefinedtyping, and dropanywhereoverrideIterationsis already in the Loop Start data type) to reduce future drift and improve readability. - Optionally add lightweight logging or a UI hint when an invalid override value is ignored (non-numeric, < 1, or out of range) so users can understand why their override is not taking effect.
- Centralize iteration min/max constants (e.g., 1 and 100) in a shared location so Web, runtime, and static analysis all respect the same bounds without risk of drift.
-
Why: Reduces duplication and ambiguity around iteration overrides, keeps loop behavior predictable as the feature evolves, and makes misconfigurations easier to diagnose without changing current semantics.
-
Goal: Keep loop node UI categorization and handle semantics maintainable as new loop-related handles or node variants are introduced.
-
What:
- Centralize node-type-to-category label mapping for the library (including Loop Start/End) in a small shared helper or config object, re-used by
WorkflowSidebarand any future UIs that group nodes by category. - Document that all non-
iterations-overridehandles on Loop Start are currently treated as main loop body inputs, and revisit the filtering strategy if additional specialized handles (e.g., accumulator feeds) are added in the future. - Optionally add lightweight validation to guard against malformed or unexpected handles on Loop Start nodes, so miswired graphs fail early with clear messages instead of silently treating unknown handles as main inputs.
- Centralize node-type-to-category label mapping for the library (including Loop Start/End) in a small shared helper or config object, re-used by
-
Why: Keeps the loop UI and runner behavior explicit and resilient to future node/handle additions without introducing extra complexity today.
- Goal: Refine the new condition editor UX and typing without changing behavior
- What:
- Reset
conditionDraftwhen the selected node changes to avoid stale draft values. - Avoid a static
idon the condition textarea (e.g., derive fromnode.id) to prevent potential collisions in multi-panel contexts. - Add a small "Edit" button next to the textarea for keyboard users to open the modal without requiring double‑click.
- Tighten
onChangetyping toReact.ChangeEvent<HTMLTextAreaElement>(removeany).
- Reset
- Why: Improves accessibility, prevents minor edge‑case collisions, and strengthens type safety with minimal complexity.
- Priority: P3 (polish)
- Goal: Improve tab accessibility and small interaction details in the combined modal
- What:
- Add
aria-pressedandaria-controlsto the Preview/Edit toggle buttons; userole="tablist"/role="tab"semantics for better screen reader support. - Restore focus to the toggle that opened the current view when switching tabs for keyboard users.
- Optional: remember the last-used tab per session and open the modal with that tab selected (still default to Preview on first open).
- Optional: show a subtle "Edited" indicator when the draft differs from the initial value before saving.
- Add
- Why: Enhances accessibility and clarity without changing core behavior or adding dependencies.
- Priority: P3 (polish)
- Goal: Harden status parsing and polish visual consistency for tool run indicators
- What:
- Fix null-check edge case in
hasToolRunErrorsonullis not treated as an object when checkingerrorfields. - Support string-form error payloads (e.g.,
error: "message") in addition to object-shaped errors when deriving failure state. - Memoize or cache JSON parsing of paired
tool_resultonce per item to avoid repeatedJSON.parsework across renders. - Replace hard-coded spinner color with theme tokens (VS Code variables) for consistent theming; verify success/error icon colors also use tokens.
- Fix null-check edge case in
- Why: Improves correctness for mixed error shapes, reduces unnecessary parsing overhead, and aligns UI with the current theme for clarity and consistency.
- Priority: P2 (robustness/UX polish)
- Goal: Tighten subflow editor UX and execution semantics
- What:
- Add autosave and Ctrl/Cmd+S shortcut in subflow editor; show unsaved indicator next to title.
- Propagate abort/fail-fast semantics from inner nodes to parent subflow node with clear status mapping (success/error/interrupted) and error surface in RightSidebar.
- Preserve and edit subflow output mapping UI (rename, reorder, delete) with validation against duplicate names and stable indices.
- Carry ordered inner edges out as deterministic order metadata when mapping outputs to parent connections.
- Improve keyboard accessibility: visible focus ring, Enter/Space to open editor, ARIA label with subflow title.
- Optional: preview unsaved changes diff before leaving subflow (nodes/edges/outputs) to help users decide to discard or save.
- Why: Rounds out the subflow user journey (edit, save, navigate, execute) with predictable persistence, clear failure behavior, and accessible controls.
- Goal: Clarify bypass-node interactivity and widen style typing for future-proofing
- What:
- Consider making bypassed nodes non-interactive by applying
pointerEvents: 'none'ingetNodeStylewhenbypass === true(opt-in; verify UX trade-offs) - Broaden
defaultBorderStyleparameter typing in Nodes.tsx toReact.CSSProperties['borderStyle']to support all valid CSS border styles - Fix minor nit: remove leading space before color value in any
borderstring interpolations to ensure consistent CSS parsing
- Consider making bypassed nodes non-interactive by applying
- Why: Provides a clear UX option for bypass behavior, strengthens typing for style extensibility, and tidies up minor styling nits
- Goal: Prevent state updates after unmount and avoid timer leaks in the clipboard button
- What: Store the
setTimeouthandle in CopyToClipboardButton.tsx and clear it in auseEffectcleanup to preventsetCopied(false)from firing after unmount - Why: React may warn about state updates on unmounted components; cleaning up the timer removes a subtle footgun and improves component hygiene
- Goal: Refine the workflow clipboard protocol, selection DTOs, and context menu UX beyond the initial implementation.
- What:
-
- Preserve and round-trip any additional edge metadata in the
WorkflowPayloadDTO(such as labels or ordering hints) when copying and pasting selections so future edge features are not lost during clipboard operations.
- Preserve and round-trip any additional edge metadata in the
-
- Consolidate the clipboard graph cloning logic into a single helper that remaps node/edge IDs and applies offsets, and reuse it across webview clipboard handlers to avoid drift between different copy/paste paths.
-
- Clarify and, if needed, extend clipboard precedence semantics so users have predictable behavior when the system clipboard contains non-NebulaFlow content (e.g., explicit rules for when in-memory vs system clipboard wins).
-
- Add small dev-only logging and optional visual hints around context-menu copy/paste actions to aid debugging without increasing noise in normal usage.
-
- Extend paste-time
nodeResultscloning to include derived keys such as${nodeId}_tokens(or other per-node result metadata) so PREVIEW and LLM nodes preserve token-count and related metadata alongside their main output when duplicated.
- Extend paste-time
-
- Optionally collapse the three
idMapiteration loops inapplyClipboardPayloadinto a single pass that updatesnodeResults,nodeAssistantContent, andnodeThreadIDstogether, and simplify redundant runtime guards/type checks once the involved maps remain type-safe.
- Optionally collapse the three
-
- Document or, if needed, strengthen the immutability assumption for
nodeAssistantContentitems (or replaceitems.slice()with a slightly deeper clone) so future in-place mutations cannot leak between original and pasted nodes.
- Document or, if needed, strengthen the immutability assumption for
-
- Keep the
applyClipboardPayloaduseCallbackcomment and dependency array in sync by either relying on setter stability (omitting them from deps) or updating the comment to describe the current exhaustive-deps strategy.
- Keep the
- Why: Keeps the clipboard flow data-oriented and robust as the protocol evolves, reduces duplication in cloning helpers, makes behavior predictable across panels, and leaves room for UX and code polish around copy/paste state cloning without complicating the core implementation.
- Goal: Make the RightSidebar minimum width user-friendly and adaptable across displays
- What: Revisit the
useRightSidebarResizeconfiguration in Flow.tsx to avoid locking the minimum at 380px; consider a smaller min, responsive breakpoint logic, and/or persisting user-chosen width - Why: A fixed larger minimum can constrain canvas space on smaller screens; flexibility improves ergonomics without sacrificing readability
-
Goal: Make LLM assistant auto-scroll behavior fully robust and predictable across programmatic and user-driven scrolls.
-
What:
- Mark all code-initiated scrolls on the assistant container (including the ref callback that runs on mount/resizes) in
programmaticScrollNodesbefore settingscrollTop, sohandleAssistantScrollconsistently ignores those events and only treats genuine user scrolls as intent to pause or resume auto-follow. - Add a small cleanup strategy for
programmaticScrollNodesto avoid rare cases where a programmaticscrollTopassignment does not emit ascrollevent (for example, by only adding the id when the offset changes, or by clearing stale ids on a short timeout or per-tick basis). - Re-verify the fullscreen layout choice (
tw-max-h-[60vh]on the inner assistant container) on small-height viewports and refine the max-height if needed to minimize double-scroll scenarios while keeping the scrollbar visually inside the LLM node accordion item. - Optionally tighten inline comments around the fullscreen assistant container to call out that auto-scroll logic assumes this inner element is the scroll owner in both normal and fullscreen modes.
- Mark all code-initiated scrolls on the assistant container (including the ref callback that runs on mount/resizes) in
-
Why: Preserves the simple "auto-follow until I scroll" mental model, avoids subtle ignored-first-scroll edge cases, and keeps fullscreen layout/scrollbar ownership clear and maintainable without changing current behavior.
-
Goal: Capture medium/low-priority polish items for the new LLM assistant UX and related SDK/Electron changes.
-
What:
-
- Treat future
vendor/amp-sdk/amp-sdk.tgzupdates as explicit SDK upgrades by tying them to a known upstream version/CHANGELOG entry and running the usual checks (e.g.,npm audit, smoke tests) so binary bumps remain transparent and verifiable.
- Treat future
-
- Guard
fullscreenNodeIdwith a small effect that clears the fullscreen state when the referenced node is removed fromsortedNodes, preventing a blank fullscreen panel when graphs are mutated while fullscreen is active.
- Guard
-
- Extract the LLM assistant
displayItems→segmentsconstruction (includingpairedMap,pendingNonText, andflushNonTextAccordion) into a pure helper or dedicated test target so temporal ordering, de-duplication, and grouping behavior can be unit tested and evolved safely.
- Extract the LLM assistant
-
- Simplify
renderAssistantAccordionItemby either removing or repurposing the now-unusedtextbranch, and align any future text rendering there with the Markdown-based styling used for streamed assistant messages.
- Simplify
-
- Add an explicit
aria-labelto the LLM fullscreen toggle button in the RightSidebar so the icon-only control is fully accessible and consistent with other Playbox toggle buttons.
- Add an explicit
-
- Optionally add a brief comment or small tests documenting the
filterLastTextMatchingResultbehavior that de-duplicates the final assistanttextitem againstnodeResults, including whitespace-only edge cases.
- Optionally add a brief comment or small tests documenting the
-
Why: Keeps the new LLM assistant UX maintainable and accessible, makes SDK upgrades auditable, and reduces regression risk around the more complex assistant segmentation logic without changing current behavior.
-
Goal: Make the auto-follow active-node behavior explicit, test-backed, and aligned with the broader tool/SDK surface without changing current semantics.
-
What:
- Document the immutability expectation for
executingNodeIds(always replace with a newSetwhen membership changes) wherever that state is produced, so thesingleActiveNodeIdmemo in RightSidebar.tsx remains reliable. - Add focused tests around the auto-follow lifecycle to lock in behavior: manual accordion interaction disables auto-follow for the rest of the run, new
executionRunIdre-enables it, andpendingApprovalNodeIdalways wins over auto-follow when present. - Clarify or, if needed, refine how the "primary" executing node is chosen when there are multiple executing nodes (e.g., engine-level tracking vs. relying on
Setinsertion order) and document that contract whereexecutingNodeIdsis derived. - When ready to surface the SDK
GrepcontextLinesenhancement in the NebulaFlow UI, extendTOOL_KEYS.grepin RightSidebar.tsx to includecontextLinesso traces show when ripgrep is run with match context. - Consider adding a small, explicit "Auto-follow" toggle in the RightSidebar header that reflects
autoFollowActiveNodestate and lets users re-enable it mid-run, keeping the current implicit behavior as the default. - Treat vendored
amp-sdk.tgzbumps as explicit SDK upgrades: ensure versioning/changelog notes remain in sync and run the usual dependency/security checks (e.g.,npm audit) after updates.
- Document the immutability expectation for
-
Why: Encodes the new auto-follow behavior in tests and documentation, keeps the Grep tool display aligned with the upstream SDK contract, and makes auto-follow and SDK dependency behavior more discoverable and robust over time.
-
Goal: Tighten the auto-expand effect and document the
pending_approvalcontract for Shell (CLI) nodes. -
What:
- Narrow the
useEffectdependency surface in Flow.tsx that auto-expands the Right Sidebar on CLIpending_approvalso it reacts only to the minimal state required (e.g.,pendingApprovalNodeId,rightCollapsed, and the specific node type), avoiding unnecessary renders. - Capture the
pending_approvalstatus contract and expectations (who emits it, when it clears, and what UI it drives) in a small vertical-slice note or inline docs so future changes to execution events keep Shell approval behavior aligned.
- Narrow the
-
Why: Keeps the auto-expand behavior explicit and efficient while making the status contract discoverable for future engine or UI changes.
-
Goal: Improve accessibility and consistency for the Playbox toggle while approvals are pending.
-
What:
- Add a short
aria-disabled-friendly message or tooltip to the disabled Playbox burger in RightSidebar.tsx that explains it cannot be collapsed during Shell approval (e.g., "Sidebar locked while a Shell approval is pending"). - Centralize the rules that determine when the Right Sidebar may collapse (e.g., no pending approvals, no blocking dialogs) into a small helper so toggle behavior stays consistent as new approval surfaces or safety flows are added.
- Add a short
-
Why: Makes the disabled state understandable for keyboard and screen reader users and reduces the risk of drift in collapse behavior as the Playbox evolves.
-
Goal: Refine sidebar toggle behavior and discoverability beyond the minimal collapse/expand implementation
-
What:
- Disable or ignore drag resizing while a sidebar is collapsed, or store/restore the last expanded width when re-expanding to avoid surprising size jumps
- Make the sidebar headers sticky (
tw-sticky tw-top-0) so the burger remains pinned even when content scrolls, in both collapsed and expanded states - Return keyboard focus to the toggle button after collapsing/expanding to improve keyboard navigation
- Normalize burger button dimensions between collapsed and expanded headers (consistent
tw-h-8 tw-w-8 tw-p-0) for visual consistency - Review the Help (
?) control for labeling consistency; ensure cleartitle/aria-labeland consider icon consistency with shadcn button sizing - Optional: persist
leftCollapsed/rightCollapsedin workspace settings or webview state so layout preference survives reloads - Optional: add a keyboard shortcut to toggle each sidebar for power users (e.g.,
[and]with a modifier), ensuring shortcuts are discoverable and conflict-free
-
Why: Reduces accidental layout changes during collapse, improves accessibility and keyboard UX, and makes the UI more predictable and consistent across states without increasing complexity
- Goal: Improve accessibility and affordances of the inline Markdown Result viewer
- What:
- Add
tabIndex,role="region", and anaria-label(e.g., "Node Result (Markdown)") to the container for keyboard and screen reader support - Optional: add a small "Copy" button to copy rendered Markdown source or plain text for quick reuse
- Add
- Why: Ensures the rendered results remain discoverable and navigable via keyboard/screen readers and improves usability for quick copy operations
- Goal: Keep the UI responsive for very large Markdown outputs without losing detail
- What:
- Add a simple truncation with "Show more" expand control after a configurable threshold (e.g., first 2,000–5,000 characters)
- Optionally support a collapse/expand control for long code blocks to reduce scroll fatigue
- Why: Extremely large outputs can impact render performance and overwhelm the sidebar. A progressive disclosure pattern keeps the UI fast and readable while still allowing full inspection when needed
- Goal: Align Markdown link behavior and sanitizer with extension handler and improve editor navigation UX
- What:
- Make
#Lxx/#Lxx-Lyyfragment parsing case-insensitive when extracting line anchors in register.ts - When a line range is present, select the full range instead of a single cursor point after revealing in register.ts
- Expand DOMPurify allowed attributes for Markdown to include
id,aria-*, anddata-*to preserve anchors and accessibility/data hooks in Markdown.tsx
- Make
- Why: Ensures robust navigation for line-anchored links, preserves useful attributes for accessibility and in-page anchors, and keeps sanitizer aligned with supported link schemes without over-restricting content.
- Goal: Streamline updates and add resilience for the new vendored SDK approach
- What:
- Add integrity/hash check and size sanity check for
vendor/amp-sdk/amp-sdk.tgzduringnpm run updateAmpSDKto catch corrupt artifacts early - Optional fallback: when
@prinova/amp-sdkfails to resolve at runtime, log a clear message suggestingnpm run updateAmpSDK(keep dynamic require as-is to avoid hard failures) - Consider a small script to bump the vendored SDK from a known upstream path or registry tag with a single command (kept manual to avoid CI side effects)
- Document expected require path for contributors to avoid reintroducing
@sourcegraph/amp-sdk
- Add integrity/hash check and size sanity check for
- Why: Ensures the vendored SDK remains easy to refresh and reduces friction when resolving local environment mismatches without re-coupling the build to external paths
- Goal: Improve robustness and UX around storage scope switching and configuration
- What:
- Deduplicate storage scope initialization messages by removing any redundant
get_storage_scope/eagerstorage_scopesends so the webview receives a single authoritative update per load - Debounce or temporarily disable rapid scope toggles in the sidebar badge to avoid overlapping config updates; re-enable on
storage_scopeacknowledgment - Pre-create default directories for the current scope before opening Save dialogs to prevent surprising empty paths or permission errors
- Tighten guards around user base path: validate
nebulaFlow.globalStoragePathexists, is absolute, and is writable; surface actionable error when invalid - Minor cleanup: remove unused
dirUrihelper and any unused locals (e.g., transientscopevariables) if no longer required - Centralize config reads (
storageScope/globalStoragePath) into a single shared helper to avoid drift across modules
- Deduplicate storage scope initialization messages by removing any redundant
- Why: Reduces race conditions and edge-case errors, improves perceived responsiveness, and consolidates configuration logic for maintainability
- Goal: Define and enforce explicit policy for bypass on conditional nodes
- What: IF/ELSE nodes marked as
bypasscurrently require an explicit seeded decision to complete; if no seed exists, pre-completion is deferred. Define whether to:- (A) Disallow bypass on IF/ELSE nodes entirely (simplest, prevents accidental misuse)
- (B) Require explicit decision seed and fail/warn if seed is missing (requires user discipline)
- (C) Allow auto-pause when bypass IF/ELSE lacks a seed, prompting user for decision (safest but more complex)
- Why: Bypass IF/ELSE nodes have no "default" sensible value; choosing a branch without user intent risks silent data loss and unexpected control flow. Current implementation defers completion (option B), but the policy should be explicit and documented.
- Priority: P1 (robustness; defines semantics for unsafe edge case)
- Status: Deferred pending policy decision; implementation ready for any choice
- Goal: Surface visibility when bypass nodes fall back to empty-string seeds
- What: When a bypass node lacks cached output and must use empty string as placeholder, emit a debug log or UI warning (once per run) to clarify the fallback behavior
- Affects parallel-scheduler.ts bypass pre-completion and workflowExecution.ts seed computation
- Why: Empty-string propagation through the workflow can cause silent data loss or unexpected behavior for downstream nodes. Users should understand when bypass is using a placeholder vs. actual cached output.
- Priority: P2 (debuggability; improves observability)
- Status: Not included in pause feature release; deferred for future visibility enhancement
- Goal: Extend pause semantics to single-node execution
- What: Make
RunOnlyThisbutton honor the pause gate; when pause is requested during single-node execution, wait for that node to complete before pausing- Affects ExecuteSingleNode.ts single-node handler
- Currently pause is only enforced by the parallel scheduler path; single-node execution skips pause logic
- Why: Pause should provide consistent semantics across all execution modes. Currently,
RunOnlyThisignores pause requests, potentially confusing users who expect uniform behavior. - Priority: P2 (consistency; improves semantic uniformity of pause feature)
- Status: Not included in pause feature release; deferred for future enhancement
+### RunOnlyThis – Type Safety, Coverage, and Guard Consolidation
- Goal: Tighten CLI node typing across core and webview and remove minor DTO nits without changing behaviour.
- What:
- Extend the core
WorkflowNodesunion to includeCLINodeso helpers that switch onnode.typeget the richer CLI-specific data shape by default. - Remove the redundant
?? undefinedonsourceHandle/targetHandlewhen building edge DTOs in workflowActions.ts since JSON serialisation already dropsundefined. - Audit and, if needed, clean up any remaining direct
shouldAbortassumptions on CLI nodedatato rely on the sharedBaseNodeData.shouldAbort?contract instead.
- Extend the core
- Why: Completes the type-level integration of CLI nodes in the core model, keeps DTO shapes minimal and explicit, and avoids future drift around CLI abort semantics while preserving the current runtime behaviour.
+- Goal: Tighten typing and guard behavior for the RunOnlyThis affordance in the RightSidebar while keeping execution semantics simple.
+- What:
-
- Extend the
WorkflowNodes[NodeType.SUBFLOW]data type sosubflowIdis modeled explicitly and remove the(node as any)access used in the RightSidebar disabled condition for SUBFLOW nodes in RightSidebar.tsx.
- Extend the
-
- Define a single
RUN_ONLY_THIS_ENABLED_TYPES(or similar) list alongside node-type definitions in Nodes.tsx or models.ts and reuse it in RightSidebar.tsx to decide when the button is shown, avoiding drift as new executable node types are added.
- Define a single
-
- Extract a small helper (e.g.,
canRunOnlyThis(node, state)) local to the webview slice to centralizeisPaused,executingNodeIds.size > 0, and SUBFLOW-without-subflowIdchecks so both the button’s disabled state and theuseRunOnlyThisguard follow the same rules.
- Extract a small helper (e.g.,
-
- Add focused tests (when/where test harness exists) that cover visibility per node type, disabled behavior for paused/executing/SUBFLOW-without-id scenarios, and that clicking the enabled button dispatches the
nebula-run-only-thisevent with the correctnodeId. +- Why: Makes the RunOnlyThis affordance safer and more maintainable as the node catalog evolves, removesanycasts around SUBFLOW metadata, and keeps UI/handler guards aligned through a single decision point while preserving the existing single-node execution flow.
- Add focused tests (when/where test harness exists) that cover visibility per node type, disabled behavior for paused/executing/SUBFLOW-without-id scenarios, and that clicking the enabled button dispatches the
-
Goal: Polish the last-workflow auto-load behavior and supporting utilities without changing core UX.
-
What:
- Decide whether multiple open NebulaFlow panels should each auto-load the last workflow or if only the first panel should do so; if the latter, promote the per-hook
hasRequestedLastWorkflowRefguard in messageHandling.ts to a shared/module-level flag or store. - In dev builds, add lightweight diagnostics when
.nebulaflow/last-workflow.jsonis malformed or unreadable in fs.ts so silent best-effort behavior remains in production but issues are easier to debug during development. - Consider small naming/structure cleanups in the last-workflow helpers (e.g., clarifying directory vs file naming and simplifying
readWorkflowFromUristate spreading) to keep the data-access slice easy to read. - When/if tests are added for this slice, cover
setLastWorkflowUri/getLastWorkflowUriround-trips andloadLastWorkflowbehavior for missing/invalid/unsupported files.
- Decide whether multiple open NebulaFlow panels should each auto-load the last workflow or if only the first panel should do so; if the latter, promote the per-hook
-
Why: Keeps the auto-restore feature predictable, debuggable, and maintainable while staying aligned with the existing data-access and messaging patterns.
-
Goal: Prevent race condition when user rapidly alternates pause/resume during active execution
-
What: Debounce or disable the Resume button briefly after pause is requested, preventing immediate resume while extension still has active task inflight
- Affects SidebarActionsBar.tsx pause button state logic
- After pause request posted, button should remain disabled until
execution_pausedevent arrives
-
Why: If user clicks Pause then Resume while tasks are still draining, the resume message arrives before the extension has fully paused, causing it to be rejected by the concurrency guard
-
Priority: P1 (UX; prevents confusing user-facing error when rapid pause/resume occurs)
- Goal: Prevent unsafe clearing of workflow structure while paused
- What: Disable the "Clear" (Delete workflow) button while execution is paused or active
- Affects SidebarActionsBar.tsx
- Currently only the Execute button changes state during execution; Clear remains enabled
- Why: Clearing while paused could create ambiguous state or lose workflow structure needed for resume. Disabling prevents accidental destruction during execution/pause states.
- Priority: P2 (UX safety; prevents mixed trigger semantics)
- Goal: Improve type safety of bypass node field access
- What: Remove unnecessary
anycasts arounddata.bypassfield accesses in parallel-scheduler.ts at lines 123, 162, and 523- Current pattern:
(parent as any)?.data?.bypass - Recommended:
parent.data?.bypass(bypass is typed in BaseNodeData)
- Current pattern:
- Why: The
bypass?: booleanfield is already defined in the node data type, so casting toanyis unnecessary and reduces type safety. Direct property access provides compile-time type checking. - Priority: P2 (code quality; improves type safety)
- Goal: Clarify bypass semantics in bypass node in-degree calculation
- What: Update comment at parallel-scheduler.ts L119 to reflect bypass exception
- Current: "Treat seeded parents as satisfied"
- Recommended: "Treat seeded parents as satisfied unless the parent is included and bypassed"
- Why: The code now includes a special case for bypass nodes marked with
bypass === true, but the comment still reflects the older logic. Updated comment prevents future maintainers from misinterpreting the conditional logic. - Priority: P2 (code quality; improves code clarity and maintainability)
- Goal: Clarify and document behavior when bypassing nodes without cached results
- What: Add UI hint or tooltip explaining that empty strings are used as fallback when a bypassed node has no prior result cached
- Consider visual indicator (icon or small label) in PropertyEditor checkbox row
- Or add help text: "Node will use previous result if available; empty string if none"
- Why: Silent fallback to empty string when no cached result exists could confuse users. Explicit documentation prevents expectation mismatches.
- Priority: P2 (UX/documentation; nice-to-have for clarity)
- Goal: Improve semantics and observability of bypass fallback behavior
- What: Add optional warning or status indicator when bypass is enabled on a node that currently has no cached result, or when the cached result is an empty string
- Could display: "No cached result available" badge during property editor display
- Or log/notify when bypass executes without a prior result
- Why: Users should understand when bypass will execute with an empty value. A visual cue distinguishes "never executed" from "intentionally empty result" scenarios.
- Priority: P2 (UX; nice-to-have improvement for workflow clarity)
- Goal: Clean up dead code in resume filter pruning logic
- What: Remove unused
seedIdslocal variable in register.ts where bypass seeds are preserved during resume - Why: Variable is computed but never referenced. Removing it reduces cognitive load and clarifies the actual pruning semantics.
- Priority: P2 (code quality; minor cleanup)
- Goal: Reduce duplication of bypass seed computation logic
- What: Extract the bypass seed computation logic from workflowExecution.ts and workflowExecution.ts#L122-L131 into a shared helper function
- Helper signature:
computeBypassSeeds(nodes: BaseNode[], nodeResults: Map<string, string>, bypassedNodeIds: Set<string>): Record<string, string> - Used in both execute and resume branches
- Helper signature:
- Why: Same logic appears in two places, risking drift if bypass behavior changes. A shared helper improves maintainability and ensures consistent semantics.
- Priority: P2 (code quality; improves maintainability)
- Goal: Provide user confirmation when results are cleared
- What: Add a brief toast/snackbar notification when the Reset button is clicked to acknowledge the action
- Could display message like "Results cleared" for 2–3 seconds
- Would appear in the RightSidebar or as a dismissible notification
- Why: Silent reset may go unnoticed if the user clicks quickly and navigates away. Visual feedback confirms the action completed successfully.
- Priority: P2 (UX; nice-to-have improvement based on user feedback)
- Goal: Align prop naming and UI labels with the broadened Clear/Delete behavior and keep reset helpers easy to maintain.
- What:
- Rename the
resetExecutionStateprop at the container boundary to something likeclearWorkflow/onClearWorkflowand thread that name through LeftSidebarContainer.tsx, LeftSidebar.tsx, and SidebarActionsBar.tsx so it clearly represents "Clear graph + results". - Optionally make the Clear button text more explicit (e.g., "Clear Workflow" or "Delete Workflow & Results") if user feedback suggests the distinction from "Reset Results" is still unclear.
- Consider either including
setNodeResults/setNodesin theresetResultsStatedependency array in Flow.tsx for symmetry, or briefly documenting that setters are intentionally omitted because they are stable.
- Rename the
- Why: Reduces conceptual mismatch between prop names and behavior, keeps the Reset vs Clear distinction obvious in the UI, and makes the reset helper’s dependency choices clearer for future maintainers.
- Goal: Improve robustness when custom nodes extend BaseNodeData
- What: Add defensive checks in the reset handler to validate optional fields before clearing:
- Check if
contentandtokenCountfields exist before setting them to empty/zero - Consider iterating over node data schema instead of assuming field presence
- Check if
- Why: If custom nodes add required fields or diverge from BaseNodeData shape, the reset logic could inadvertently break node state. Defensive checks prevent failures when custom nodes extend the base contract.
- Priority: P2 (robustness; prevents edge cases with extended node data)
- Goal: Provide alternative keyboard navigation for modal dismissal in edge cases where Escape propagation is blocked
- What: While Escape key propagation has been enabled for standard dismissal, consider adding additional keyboard affordances (e.g., Ctrl+Enter to confirm, Tab+Enter navigation) for users who may experience keyboard event handling issues in rare environments
- Related: TextEditorModal.tsx
- Related: dialog.tsx
- Why: While Escape works in standard environments, providing fallback keyboard shortcuts improves accessibility for users with non-standard input setups or browser configurations
- Priority: P2 (accessibility; nice-to-have improvement)
- Goal: Enhance accessibility of the Text Editor modal dialog with ARIA patterns for focus trapping and return focus
- What: Add ARIA dialog patterns to the portal-rendered modal, including:
role="dialog"on the content wrapper (already present at dialog.tsx L67)aria-modal="true"attribute (already present at dialog.tsx L68)- Focus trapping within modal to prevent keyboard navigation outside dialog bounds
- Return focus to triggering element when modal closes
aria-labelledbylinking dialog title to heading for screen readers
- Why: Standard ARIA dialog patterns improve screen reader support and keyboard navigation accessibility, particularly for users relying on assistive technology
- Priority: P2 (accessibility; improves A11y compliance)
- Goal: Evaluate and document SSR hydration considerations for portal-rendered dialogs
- What: While the portal includes an SSR guard (
typeof document !== 'undefined'), document the implications:- If SSR is introduced in the future, portal markup rendered on server will differ from client (SSR renders null, client renders portal)
- Consider mounting portals only after a client-only flag is set via useEffect to prevent hydration mismatches
- Review hydration behavior with any future static generation or server-side rendering features
- Related: dialog.tsx L73
- Why: Current guard is appropriate for client-side-only webview context (VS Code). Document as a known limitation for future SSR contexts to prevent subtle hydration bugs.
- Priority: P1 (documentation; prevents future SSR-related issues)
- Goal: Re-evaluate whether textarea resizing should be allowed based on user feedback
- What: Currently textarea uses
tw-resize-noneto disable manual resizing. If user feedback indicates desire for resize control, consider enabling vertical-only resize (tw-resize-y) to allow users to adjust modal height to their preference- Affects: TextEditorModal.tsx L54
- Why: The 90vh container is large, but some users may prefer the ability to make the textarea smaller or larger based on their content. Vertical-only resize allows fine-tuning without breaking horizontal layout.
- Priority: P2 (UX; nice-to-have based on user feedback)
- Goal: Clarify and validate protocol expectations for single-node error/abort handling
- What: Single-node execution (ExecuteSingleNode.ts) currently emits both
node_execution_statusandexecution_completedevents when a node errors or is aborted. Confirm whether the webview expects both events (for consistency with full-workflow semantics) or if a dedicated single-event protocol would simplify state management- Evidence: ExecuteSingleNode.ts emits events sequentially on error/abort
- Related: messageHandling.ts handles both event types
- Why: Dual events add protocol complexity. Validating whether both are necessary prevents future confusion when debugging multi-event scenarios and clarifies the minimal event set for single-node execution semantics.
- Priority: P2 (protocol clarity; improves documentation)
- Goal: Define clear precedence when both
interruptedNodeIdandstoppedAtNodeIdare set - What: If upstream execution handlers guarantee only one of
interruptedNodeIdorstoppedAtNodeIdis ever set in a single execution completion, this enhancement is not needed. If both can be set simultaneously, document or enforce precedence in RightSidebar.tsx to prevent sidebar highlighting two nodes for the same execution context- Current: Sidebar renders both nodes if both are set; visual confusion if both borders appear
- Why: Clarifies UI semantics when both indicators are present, preventing ambiguous visual feedback to users
- Priority: P2 (UX clarity; depends on upstream guarantees)
- Goal: Improve naming clarity to distinguish between "last executed" and "last completed" node states
- What: Rename
lastExecutedNodeIdtracking (and related variables) tolastCompletedNodeIdacross ExecuteWorkflow.ts and status handler at ExecuteWorkflow.ts, and the webview ref in messageHandling.ts - Why: Naming clarity prevents future developers from misinterpreting the tracking semantics. "Completed" more accurately reflects that the node finished execution (including errors/interrupts), not merely started execution.
- Priority: P1 (code quality; improves naming consistency)
- Goal: Improve visibility and handling of loop nodes in parallel execution analysis
- What: Loop nodes are currently excluded from parallel step analysis and assigned step -1 ("unsupported"). Consider adding a dedicated UI grouping/label in RightSidebar or canvas to indicate "unsupported parallel execution" status, making it visually explicit which nodes cannot participate in parallel execution paths
- Affects parallel-analysis.ts and RightSidebar.tsx grouping logic
- Why: Users may not understand why loop nodes are missing from parallel step groupings. A dedicated "Unsupported Loop Nodes" section or visual indicator on the canvas would clarify their status and limitations.
- Priority: P2 (UX clarity; improves workflow comprehension for complex graphs with loops)
- Goal: Improve accuracy of branch hint labels in RightSidebar parallel step groups
- What: Current implementation marks a step "True" only if every IF node's true-set contains all step nodes. For multiple IF nodes this rarely passes. Consider computing per-step/per-IF hints using exclusivity to specific IF nodes rather than requiring global exclusivity across all IFs
- Affects RightSidebar.tsx branch suffix logic
- Why: More nuanced branch hints (e.g., "Step 3 (2 nodes) – true if IF#1 is true") would accurately describe conditional execution without overconstraining to global exclusivity. Current all-or-nothing approach means most steps show no branch hint despite being conditional.
- Priority: P2 (UX refinement; improves accuracy of parallel path hints)
- Goal: Clarify cycle-breaking fallback behavior in topological sort
- What: In parallel-analysis.ts, when the Kahn queue empties during cycle detection, the fallback picks the min in-degree node. Add a code comment explaining the intent and clarifying whether this marks a separate "recovery step" or continues the current step assignment
- Affects logic at L80-L90
- Why: The fallback is a rare case (graphs should be acyclic), but future maintainers need clarity on step boundary semantics when cycles force recovery. A brief comment prevents confusion about whether recovered nodes are grouped with the current step or assigned separately.
- Priority: P2 (code clarity; improves maintainability for edge cases)
- Goal: Document behavior of nodes without outgoing edges
- What: In parallel-analysis.ts, nodes without outgoing edges return priority
+∞. Document the expected ordering and grouping behavior for sink nodes and explain why infinite priority is appropriate for this context- Affects
getNodePriorityForParallelAnalysis()function
- Affects
- Why: Without documentation, maintainers may misinterpret infinity as a bug or unintended behavior. A clear comment explains the design choice and prevents accidental refactoring.
- Priority: P2 (code clarity; improves documentation completeness)
- Goal: Improve screen reader support for parallel step grouping headers
- What: Add
aria-levelor semantic heading level attribute to "Parallel Step N" headers in RightSidebar.tsx to provide correct navigation context for assistive technology - Why: Screen reader users benefit from proper heading hierarchy. Currently the step headers may be treated as plain text, reducing navigability.
- Priority: P2 (accessibility; improves screen reader experience for complex workflows)
- Goal: Clarify how loop nodes are represented in parallel step labels in the RightSidebar Playbox.
- What:
getStepLabelin RightSidebar.tsx still contains agroupIndex === -1branch that returns "Unsupported (Loop)", butparallelGroupIndexfor items inallItemsInOrderis always non-negative. Decide whether to remove this dead branch or explicitly propagate a-1sentinel from the parallel analysis when loop groups are present, keeping behavior consistent with parallel-analysis.ts. - Why: Avoids misleading dead code and ensures any future loop-aware UI in the Playbox has an explicit, documented contract.
- Priority: P2 (code clarity; current loop behavior remains unchanged).
- Goal: Harden the Playbox parallel grouping UI and guard the new numbering behavior.
- What: Update the parallel group container key in RightSidebar.tsx to include
parallelGroupIndex(for example,key={`step-${item.stepIndex}-${item.parallelGroupIndex}`}) and add focused tests that verify: (1) contiguous parallel numbering when sequential-only steps are interleaved, (2) correct label strings such as "Parallel Step 1 (3)", and (3) fallback behavior whenparallelStepsanalysis is unavailable. - Why: Ensures React list keys remain unique if grouping semantics evolve and prevents regressions in the clarified parallel numbering behavior.
- Priority: P2 (robustness; nice-to-have hardening).
- Goal: Make refreshes of the vendored Amp SDK bundle low-risk and repeatable.
- What: When amp-sdk.tgz is updated, document and automate a minimal validation checklist (for example,
npm run check, a smoke test of representative workflows, and verification of tool catalog wiring) so SDK bumps are consistently exercised before release. - Why: The vendored SDK is opaque in this repo; a small, explicit validation step reduces the chance of runtime or type-level regressions entering NebulaFlow via SDK upgrades.
- Priority: P1 (release robustness; medium risk if skipped).
- Goal: Improve type safety and document multi-select behavior for node selection
- What: Narrow
selectedNodeTypetoNodeType | nullinstead of current implicit type; document and decide semantics for multi-select "primary" node when multiple nodes are selected in sidebar- Affects RightSidebar.tsx and Nodes.tsx
- Why: Current implementation extracts a single
selectedNodeIdfrom theselectedNodesarray but doesn't define which node is "primary" when multiple are selected. Explicit type narrowing and documented semantics prevent ambiguity and reduce risk of unexpected behavior when multi-select is extended - Priority: P1 (clarity; improves type safety and behavioral documentation)
- Goal: Improve naming clarity and future extensibility of selection summary for multi-node workflows
- What: Consider renaming
selectionSummarytoselectionContextor similar, and document future extensibility for multi-select aggregates (e.g., "3 nodes selected" badges, bulk operations)- Current: Helper extracts single node ID; future: could expand to aggregate metadata for multi-selection UI
- Why: Current naming assumes single-node context; explicit naming and extensibility planning reduce refactoring burden when multi-select features are added
- Priority: P2 (code quality; improves maintainability for future features)
- Goal: Improve accessibility and semantics of RightSidebar title header
- What: Add
aria-labelattribute to the "Playbox" heading in RightSidebar.tsx to provide descriptive label for screen readers - Why: Screen reader users benefit from explicit aria labels that describe the purpose of the sidebar section. The label should clarify what content is displayed in this panel (e.g., "Playbox: Assistant and workflow execution results").
- Priority: P2 (accessibility; improves experience for screen reader users)
- Goal: Maintain consistent heading styling across sidebar components
- What: Review and align the "Playbox" header styling with other sidebar section headers (e.g., left sidebar headers) to ensure uniform visual treatment and semantic HTML usage
- Why: Consistent header styling improves visual cohesion and reduces maintenance burden when updating shared header patterns across the UI
- Priority: P2 (UX consistency; improves visual polish)
- Goal: Prevent accidental mixed trigger of "Run from here" and "Run only this" while execution is active
- What: Disable the "Run From Here" button alongside "Run Only This" in all node types when
data.executingis true- Affects CLI_Node.tsx, IfElse_Node.tsx, LLM_Node.tsx, Text_Node.tsx, Variable_Node.tsx
- Why: Currently only "Run Only This" disables during execution. "Run From Here" remains enabled, which could allow users to trigger a workflow-level execution while a single-node execution is in-flight, leading to potentially confusing concurrent operations
- Priority: P2 (UX; prevents mixed trigger semantics during execution)
- Goal: Provide clear visual indication of disabled execution state
- What: Add optional spinner/dim effect to disabled RunOnlyThis buttons for better affordance of the disabled state
- Context: Node toolbars in LLM_Node.tsx contain button groups with possible visual treatments
- Why: Currently the button disables but may lack obvious visual affordance (appearance change beyond opacity). Spinner or dimming effect makes the disabled state more discoverable to users
- Priority: P2 (UX polish; improves visual clarity)
- Goal: Eliminate copy-paste drift in node action buttons across node types
- What: Extract recurring button groups ("Run Only This", "Run From Here") from individual node components into a shared toolbar component
- Affects CLI_Node.tsx, IfElse_Node.tsx, LLM_Node.tsx, Text_Node.tsx, Variable_Node.tsx
- Why: Maintains toolbar consistency and reduces maintenance burden. Changes to action buttons only need to be made in one place, reducing risk of drift when adding new actions or modifying button behavior
- Priority: P2 (code quality; improves maintainability and prevents drift)
- Goal: Eliminate global state race condition for concurrent multi-panel workflows
- What: The
activeWorkflowUriin workspace.ts is a single module-level variable. With two workflow panels open executing concurrently, the last panel to set the value overwrites the previous one, causing the wrong workspace root to be used during execution initiated from the first panel. - Why: Global state doesn't support true concurrent multi-panel execution. When panel A starts execution but panel B becomes active in the meantime, panel A's execution will use panel B's workspace roots instead of its own.
- Recommendation: Associate workspace roots with the requesting panel via a
Map<Webview, Uri>keyed by webview instance, or pass explicit roots alongside execution messages in the request contract so each panel carries its own context. - Priority: P1 (concurrency; blocks reliable multi-panel execution)
- Goal: Prevent duplicate roots and mis-ordering on case-insensitive file systems
- What: Path comparison in workspace.ts uses string inequality (
r !== preferred) to filter and reorder workspace roots. On case-insensitive file systems (macOS default, Windows), paths like/Users/Dev/projectand/users/dev/projectwould be treated as different roots even though they refer to the same folder. - Why: Case-insensitive file systems can silently create duplicate root entries with different casing, breaking root ordering assumptions and potentially confusing SDK context resolution.
- Recommendation: Normalize paths before comparison using
vscode.Uri.toString()lowercased or a path canonicalization resolver to ensure consistent matching across file system types. - Priority: P1 (robustness; prevents cross-platform root ordering bugs)
- Goal: Improve robustness of condition string parsing in single-node If/Else execution
- What: In ExecuteSingleNode.ts, add trimming of individual tokens after splitting condition string on operator. Currently inputs like
a === b(extra spaces) or leading/trailing spaces can cause unexpected inequality. - Why: Whitespace sensitivity can produce silent comparison failures. Trimming tokens ensures robust parsing for user inputs with variable formatting
- Priority: P1 (robustness; prevents silent condition mismatches)
- Goal: Reduce overhead and clarify async semantics in single-node If/Else execution
- What: Remove
asynckeyword fromexecuteSingleIfElseNodein ExecuteSingleNode.ts since the function performs no awaits and executes synchronously - Why: Unnecessary
asyncadds promise wrapper overhead. Removing it reflects actual sync behavior and improves performance - Priority: P2 (optimization; minor performance improvement)
- Goal: Prevent accidental double-trigger of single-node execution and improve UX clarity
- What: Update RunOnlyThisButton.tsx to disable the button while an execution is active (track execution state from parent or global hook) and add
aria-disabledattribute for accessibility - Why: Allows users to understand execution state visually and prevents multiple rapid clicks that could queue executions unexpectedly
- Priority: P2 (UX; improves interaction feedback and prevents user errors)
- Goal: Clarify limitations of condition evaluation in single-node mode
- What: Add UI helptext or documentation in IfElse_Node.tsx explaining that quoted values and operators beyond
===/!==are not supported and resolve tofalse - Why: Current implementation mirrors full-workflow behavior but may surprise users. Explicit documentation prevents confusion about which expressions are supported
- Priority: P2 (UX/documentation; improves user understanding)
- Goal: Provide clear feedback when unsupported node types are executed in single-node mode
- What: Ensure ExecuteSingleNode.ts explicitly rejects IF_ELSE, LOOP_START/END, and ACCUMULATOR node types with descriptive error messages
- Why: Some node types (control flow, aggregation) cannot execute in isolation. Clear errors guide users to understand which nodes support RunOnlyThis feature
- Priority: P2 (UX; improves error messaging for unsupported operations)
- Goal: Prevent invalid timeout configurations in single-node execution
- What: In ExecuteSingleNode.ts, clamp or disable negative
timeoutSecvalues to prevent unintended timeout behavior - Why: Negative timeouts can produce unexpected results in timing logic. Explicit validation (clamp to zero or disable override) prevents silent misconfigurations
- Priority: P1 (robustness; prevents configuration errors)
- Goal: Improve predictability of edge ordering when edge order metadata is missing
- What: In Flow.tsx, replace the
?? 0fallback for missingorderNumberwith a stable fallback (e.g.,Number.MAX_SAFE_INTEGER) to prevent unintended reordering of edges during single-node input collection - Why: Using 0 as fallback can reorder edges unexpectedly if the first edge naturally has
orderNumber: 0. A large number ensures unordered edges sort to the end, preserving insertion order for intentionally-ordered edges and preventing silent reordering bugs - Priority: P2 (robustness; prevents subtle ordering bugs)
- Goal: Eliminate hard-coded default model reference in single-node execution
- What: Extract the hard-coded default model key from ExecuteSingleNode.ts into a shared constant alongside
DEFAULT_LLM_MODEL_IDandDEFAULT_LLM_MODEL_TITLE - Why: Hard-coded strings reduce maintainability and increase risk of inconsistency across execution paths. A centralized constant ensures model selection remains synchronized
- Priority: P2 (code quality; improves consistency and maintainability)
- Goal: Clean up dead code in single-node handler
- What: Remove unused
dangerouslyAllowAlllocal variable in single-node LLM path and consider removing...argsfromrouteNodeExecutionin NodeDispatch.ts - Why: Unused locals reduce code clarity and may indicate incomplete refactoring. Removing them improves code maintainability and reduces cognitive load
- Priority: P2 (code quality; minor cleanup for clarity)
- Goal: Prevent accidental double-trigger of single-node execution
- What: In RunOnlyThisButton.tsx, disable the button while an execution is active and add
aria-disabledattribute for accessibility - Why: Allows users to understand execution state visually and prevents multiple rapid clicks that could queue executions unexpectedly
- Priority: P2 (UX; improves interaction feedback and prevents user errors)
- Goal: Clarify behavior when parent nodes produce non-string outputs
- What: In Flow.tsx, document or explicitly convert non-string parent outputs when building inputs for single-node execution
- Why: Currently non-string outputs are silently ignored. Making this behavior explicit prevents confusion about which inputs are used and clarifies the data type contract
- Priority: P2 (code quality; improves clarity of data handling)
- Goal: Extend sanitizer to handle runtime-only types beyond functions and symbols
- What: Consider extending nodeDto.ts
sanitizeValuefunction to handleDate,Map,Set, and other non-primitive types that may appear in node data - Why: Current sanitizer only removes functions and symbols. If node data contains Date objects, Map/Set collections, or other non-cloneable types, they would still trigger DataCloneError on postMessage
- Priority: P2 (robustness; prevents edge cases with complex data structures)
- Goal: Improve purity and maintainability of the node-to-DTO conversion
- What: Refactor nodeDto.ts
toWorkflowNodeDTOto be non-mutating; currently setsonUpdate = undefineddirectly on the input node object instead of only on the returned DTO copy - Why: Mutation of input parameters reduces predictability and can cause unexpected side effects for callers. A purely functional approach (shallow copy node, then sanitize) improves testability and safety.
- Priority: P1 (code quality; improves functional purity)
- Goal: Evaluate completeness of persisted node layout metadata
- What: Confirm whether additional layout fields (e.g.,
width,height,style) should be included in the WorkflowNodeDTO contract to ensure custom node load behavior is visually consistent - Why: Currently
positionand basic node properties are preserved, but custom node styling or sizing might be lost. Verify workflow expectations for custom node layout restoration after load. - Priority: P2 (nice-to-have; improves visual consistency for custom nodes)
- Goal: Improve type safety of error handling in message dispatch
- What: Update catch clause in vscode.ts to use
unknowninstead of implicitanyfor the caught error - Why: TypeScript best practice for error handling;
unknownrequires explicit type narrowing, preventing accidental misuse of error object without checking its type first - Priority: P2 (code quality; improves error handling type safety)
- Goal: Improve type safety of edit event communication between nodes and Flow component
- What: Create a typed
NebulaEditNodeEventinterface and apply it to thenebula-edit-nodecustom event dispatch in Text_Node.tsx and listener in Flow.tsx- Define event shape:
{ id: string; action: 'start' | 'commit' | 'cancel'; content?: string; title?: string } - Apply generic type parameter to
CustomEvent<T>whereTis the typed payload - Update event listener to cast
e.detailto the typed interface
- Define event shape:
- Why: Currently using
anytype for custom event payload reduces IDE support and makes refactoring riskier. Typed events provide compile-time safety and clearer contract documentation - Priority: P1 (code quality; improves type safety)
- Goal: Clean up unused code and clarify edit state management
- What: Remove or justify the unused
shouldRefocusRefin Text_Node.tsx - Why: The ref was intended for focus management but is never updated or read in the current implementation. Removing it reduces confusion about edit lifecycle.
- Priority: P2 (code quality; minor cleanup)
- Goal: Eliminate duplication of the default title fallback string across modules
- What: The magic 'Text' fallback appears in both titleValidation.ts and Text_Node.tsx. Consider exporting
DEFAULT_NODE_TITLEconstant from titleValidation.ts and importing it in Text_Node.tsx - Why: Centralized constant prevents accidental divergence and makes future default changes easier to manage
- Priority: P2 (code quality; improves maintainability)
- Goal: Improve robustness of drop-zone coordinate conversion
- What: Add a null/undefined check for
reactFlowInstancein the drop handler Flow.tsx before callingscreenToFlowPosition() - Why: If ReactFlow provider context is unavailable or hook fails silently, calling methods on undefined instance could cause runtime errors. Explicit guard prevents potential crashes.
- Priority: P1 (defensive programming; prevents potential null-ref errors)
- Goal: Improve type safety and reduce conditional logic in path strategy selection
- What:
- Consider using an enum for
EdgeStyleKeyto reduce risk of typos at call sites (currently using string union in edgePaths.ts) - Simplify
selectEdgePathStrategyselector logic with union exhaustiveness checking to eliminate the need for a separate default case:export function selectEdgePathStrategy(style: EdgeStyleKey = 'bezier'): EdgePathStrategy { return STRATEGY_MAP[style] }
- Consider using an enum for
- Why: Enum prevents typos at call sites; exhaustive union checking makes the selector more maintainable as new styles are added and reduces verbose fallback logic
- Priority: P2 (code quality; improves maintainability and type safety)
- Goal: Validate correctness and parity of custom edge path strategies
- What: Add lightweight unit tests comparing both
bezierPathStrategyandsmoothStepPathStrategyfunctions against the original @xyflow/react helpers (getBezierPathandgetSmoothStepPath) for a fixed coordinate set - Why: Ensures custom strategy implementations produce identical output to upstream helpers, preventing visual regression and providing confidence for future style additions
- Priority: P2 (quality assurance; improves test coverage for edge path rendering)
- Goal: Clarify edge style selection semantics for workflow authors
- What: Add JSDoc documentation to
selectEdgePathStrategyand strategy functions explaining when to prefer bezier vs smoothstep styles - Why: Helps future developers and workflow authors understand the visual and performance tradeoffs between edge styles
- Priority: P2 (documentation; improves developer experience)
- Goal: Improve code clarity and maintainability of approval state management
- What: Extract
PendingApprovaltype definition in register.ts to a dedicated type or interface, and remove the unused_nodeIdfield - Why: Explicit typing improves readability and makes the pending approval contract clearer for future developers. The unused
_nodeIdfield adds cognitive load and should be removed as part of type cleanup. - Priority: P1 (nice-to-have; improves code clarity)
- Goal: Prevent approval overwrites during rapid sequential approval requests within a single panel
- What: Implement an approval queue instead of single
pendingApprovalper panel to handle cases where a second approval request arrives before the first is resolved - Why: Current implementation stores a single pending approval promise per panel. While rare given sequential node execution, concurrent approval requests could overwrite earlier pending approvals, leading to one request never resolving. A queue ensures all approvals are processed in order.
- Priority: P2 (optional enhancement; rare edge case but improves robustness)
- Goal: Improve extensibility and maintainability of the shell executor interface
- What: Type the executor options parameter as
Pick<ExecOptions, 'cwd'>instead of a loose object type - Why: Provides explicit, typed contract for what executor options are supported, making future extensions (e.g., timeout, env vars) safer and clearer for contributors
- Priority: P1 (nice-to-have; improves type safety and future extensibility)
- Goal: Make CLI node behavior configurable for workspaces with multiple root folders
- What: Extend cwd selection beyond hardcoded index 0; consider UI picker or active resource detection
- Why: Currently always uses the first workspace folder. In multi-root setups, users may want to target a specific folder or the folder containing their active file
- Priority: P2 (optional enhancement; improves UX for multi-root workspace users)
- Goal: Clarify user intent when no workspace is open
- What: Consider prompting users to open a folder when CLI node is executed without an active workspace, rather than silently falling back to extension directory
- Why: Running commands in the extension process directory can be confusing and lead to failed commands. Explicit prompt surfaces the issue and guides correct setup
- Priority: P2 (UX improvement; helps users avoid setup confusion)
- Goal: Reduce redundant data transmission when workflow filename is already known to the webview
- What: Only include workflow URI in the
workflow_loadedprotocol message when the file path information is necessary for the webview consumer - Why: Currently, the URI is always sent even though the webview primarily uses it to trigger title updates via the existing panel title mechanism. Conditional passing reduces message payload and clarifies data flow semantics.
- Priority: P1 (nice-to-have; improves protocol clarity without affecting current functionality)
- Goal: Improve maintainability and consistency of the NebulaFlow title prefix
- What: Extract the "NebulaFlow" prefix string into a shared constant (e.g.,
NEBULA_FLOW_TITLE_PREFIX) in the register.ts or a dedicated constants module - Why: Centralizes the title prefix definition, reducing the risk of inconsistent text across save/load/create operations and making future branding changes easier.
- Priority: P2 (code quality; improves maintainability for future changes)
- Goal: Maintain accurate window titles when workflows are renamed or moved outside the editor
- What: Detect when a loaded workflow file has been renamed or moved externally, and either update the title accordingly or prompt the user to re-save
- Why: If a user renames a workflow file outside the editor, the title would become stale, potentially causing confusion when managing multiple files.
- Priority: P2 (optional enhancement; edge case handling for better user experience)
- Goal: Improve ergonomics for
FlowandRightSidebarcomponents when used outside theuseWorkflowExecutionhook (e.g., tests, stories) - What: Make
executionRunIdprop optional inFlowandRightSidebarcomponents with sensible defaults - Why: External consumers (test suites, Storybook stories) may not have access to
useWorkflowExecutionhook state, causing prop drilling complexity. Optional prop with default value simplifies integration. - Priority: P2 (nice-to-have, improves developer experience for edge cases)
- Goal: Clarify the initialization and batching behavior of the
executionRunIdreset mechanism - What: Add code comment or inline documentation explaining:
- Why
executionRunIdis initialized to 0 on first mount - How React batching affects the order of reset effects vs. message posting
- Why the reset effect checks
executionRunId > 0before triggering - Why: Future maintainers and contributors need clear understanding of the state management pattern to safely modify or extend the reset logic.
- Priority: P1 (improves maintainability; consider implementing alongside any follow-up fixes)
- Goal: Remove type assertions and improve type safety in reasoning effort implementation
- What:
- Extract
ReasoningEfforttype to a shared constant module (e.g.,Core/ConstantsorShared/Types) - Remove
as anycasts in PropertyEditor.tsx and ExecuteWorkflow.ts - Update PropertyEditor and ExecuteWorkflow to import and use the shared type
- Extract
- Why: Eliminates unnecessary type assertions, reduces literal duplication across files, and improves maintainability by having a single source of truth for valid reasoning effort values.
- Priority: P1 (nice-to-have; improves code clarity and type safety)
- Goal: Validate server-side behavior for reasoning effort defaults
- What: Confirm with Amp SDK that it correctly handles the
reasoning.effort= "medium" default now being sent unconditionally from ExecuteWorkflow.ts on every LLM node execution - Why: Backend now always sets
reasoning.effortto "medium" as fallback when value is missing or invalid. Server behavior should be validated to ensure no regressions or unexpected interaction with SDK's internal defaults - Priority: P1 (verification step; ensures backend contract alignment)
- Goal: Avoid unnecessary object allocations during workflow execution
- What: Hoist
validReasoningEffortsvalidation set to module scope in ExecuteWorkflow.ts to prevent re-allocation on each LLM node execution - Why: Currently creates a new Set on every LLM node execution, allocating memory repeatedly for an immutable set of valid values
- Priority: P3 (optimization; good-to-have for performance polish)
- Goal: Enhance accessibility and performance of reasoning effort button group
- What:
- Add
aria-pressedattribute to reasoning effort buttons in PropertyEditor.tsx for screen reader users - Replace
console.logwithconsole.debugin ExecuteWorkflow.ts to reduce log noise - Why: Improves semantic HTML and screen reader support; reduces log verbosity during workflow execution.
- Priority: P2 (optimization and polish; good-to-have for production quality)
- Goal: Ensure preview merge order respects ordered edges throughout the message pipeline
- What: Verify that
orderedEdgesparameter is properly threaded through all message handler calls in messageHandling.ts that invokecomputePreviewContent(), and confirm edge order maps consistently use ordered edge indices - Why: After fixing the preview merge order to use
orderedEdges, ensure the pattern is applied consistently across all code paths where preview content is computed to prevent future regressions - Priority: P2 (code consistency; validates fix robustness)
- Goal: Clean up dead code in preview content computation
- What: Remove unused
previewNodeparameter fromcomputePreviewContent()function in messageHandling.ts - Why: Parameter was intended for potential future use but is not accessed in the current implementation; removing it reduces cognitive load and clarifies intent
- Priority: P2 (code quality; trivial cleanup)
- Goal: Extend preview updates to transitive chains of preview nodes
- What: Enhance
getDownstreamPreviewNodes()and thenode_execution_statushandler to recursively update preview nodes connected to other preview nodes (A → Preview1 → Preview2 chain) - Why: Currently only direct previews connected to completed nodes are updated. Multi-hop chains would be blank until the intervening preview executes, limiting preview utility in complex workflows
- Priority: P2 (optional enhancement; improves preview coverage for advanced workflows)
- Goal: Improve maintainability and reliability of the dangerously allow all commands feature
- What:
- Update documentation/labels explaining that "dangerously allow all" auto-approves all blocked commands regardless of SDK
toAllowenumeration - Add unit and integration tests covering auto-approval scenarios with and without
toAllowarrays - Document failure modes and observability hooks (audit logging) in feature documentation
- Update documentation/labels explaining that "dangerously allow all" auto-approves all blocked commands regardless of SDK
- Why: The feature behavior now unconditionally auto-approves when enabled; documenting this intent and testing both paths ensures future maintainers understand the design and can safely refactor without regressions
- Priority: P2 (quality assurance; improves test coverage and clarity for future changes)
- Goal: Add test coverage for default model assignment during node creation and workflow loading
- What: Create unit tests asserting that new LLM nodes receive Sonnet 4.5 as default model and that legacy workflows without models are normalized correctly on load
- Why: Ensures default model behavior is validated and protected against future regressions; complements the existing migration logic
- Priority: P2 (quality assurance; improves confidence in default model behavior)
- Goal: Provide workflow-level control over line ending normalization on live INPUT output
- What: Add an optional configuration flag to control CRLF normalization behavior when rendering active INPUT nodes in live-preview mode
- Why: CRLF normalization can alter workflow semantics for users working with files that require specific line endings (e.g., Windows batch files, shell scripts with CRLF requirements). Making this optional prevents silent data mutation for workflows with strict line-ending requirements.
- Priority: P2 (optional enhancement; improves workflow portability across platforms)
- Goal: Reduce semantic confusion in ExecuteWorkflow helper functions
- What: Rename
getInactiveNodestogetReachableNodesor document its true purpose (returns nodes reachable from a given node), and update all call sites accordingly - Why: The current name is misleading; the function actually computes forward-reachable nodes, not inactive ones. Renaming prevents future maintainers from making incorrect assumptions about its behavior.
- Priority: P2 (code quality; improves maintainability and reduces misuse risk)
- Goal: Reduce computational overhead during workflow execution resumption
- What: Replace edge array iteration with direct
edgeIndex.bySourcelookup when constructingallowedNodesfor resume filtering - Why: Current approach scans the entire edge array; using a pre-indexed map structure (if available) would reduce time complexity from O(E) to O(1) per lookup, improving performance on large workflows.
- Priority: P3 (optimization; good-to-have for scalability)
- Goal: Eliminate
anytype assertions in INPUT node output rendering - What: Add typed
content?: stringfield to INPUT node data contract instead of casting to(parentNode as any).data?.content - Why: Explicit typing provides compile-time safety and clarifies the data shape for future developers working with INPUT nodes.
- Priority: P2 (code quality; improves type safety)
- Goal: Simplify guard logic in workflow resumption filtering
- What: Remove redundant
resume?.fromNodeIdcheck in the resume filter, keeping only theallowedNodesvalidation - Why: The
allowedNodesset already encodes which nodes are executable; the additionalfromNodeIdcheck is redundant and adds unnecessary cognitive load. - Priority: P3 (code quality; minor simplification)
- Goal: Improve observability during workflow resumption
- What: Add debug logging or UI status indicator when nodes are skipped during resume due to resume filters
- Why: Silent skipping makes it difficult for users to understand why certain nodes didn't execute during a resume operation. Debug logs or UI feedback surfaces the reason.
- Priority: P2 (UX/observability; improves debugging experience)
- Goal: Improve robustness of parallel execution when unsupported control-flow nodes are present
- What: Instead of aborting the entire workflow when LOOP nodes are encountered, signal unsupported node types and allow the caller to fall back to sequential execution for that workflow
- Why: Currently throws an error if LOOP nodes are present, even if those nodes are unreachable. Graceful fallback to sequential mode would allow mixed workflows to execute instead of failing completely.
- Priority: P2 (optional enhancement; improves UX by avoiding hard failures for edge cases)
- Goal: Optimize performance of If/Else branch detection in large workflows
- What: Replace the O(N×E) nested loop in
buildConditionalEdges()with a single-pass edge iteration that identifies If/Else nodes and their branches - Why: Currently loops over nodes and filters edges per node, leading to quadratic complexity. A single pass over edges would reduce to O(E), improving performance on workflows with many nodes and edges.
- Priority: P2 (performance optimization; beneficial for large graphs)
- Goal: Clarify the purpose of conditional edge skipping during child in-degree decrement
- What: Add a code comment explaining why conditional edges are explicitly skipped when decrementing child in-degrees in
decrementChildInDegrees() - Why: With placeholder in-degrees now properly applied, the explicit skip is technically redundant but kept for clarity. A brief comment would help future maintainers understand the design decision.
- Priority: P2 (code clarity; improves maintainability for future changes)
- Goal: Eliminate duplication and improve maintainability of state hydration logic
- What: Consolidate state hydration into a single, reusable helper function instead of duplicating logic across
messageHandling.tsandworkflowExecution.ts - Why: Hydration logic currently exists in two places, risking drift and maintenance burden. A shared helper ensures consistent behavior and reduces cognitive load for future developers.
- Priority: P1 (code quality; improves maintainability)
- Goal: Decouple version bumping from specific feature presence
- What: Evaluate whether version management should be tied to the presence of optional state fields or managed independently as part of broader versioning strategy
- Why: Binding version selection to a single feature makes versioning logic fragile and harder to reason about during future feature additions. Independent versioning provides clearer semantic meaning.
- Priority: P1 (architectural; improves versioning robustness)
- Goal: Improve fidelity of persisted node state
- What: Capture and persist actual node execution status during save instead of always setting status to 'completed'
- Why: Currently all nodes are saved with status 'completed' regardless of their actual runtime state. Storing real status enables more accurate resume behavior and better observability when loading workflows.
- Priority: P2 (optional enhancement; improves state fidelity for debugging)
- Goal: Reduce protocol payload size and improve clarity of data contracts
- What: Omit undefined
statefield from protocol payload construction inconverters.tsinstead of passing it through unconditionally - Why: Undefined fields add payload bulk and reduce clarity about optional vs. required contract fields. Omitting them produces cleaner protocol messages and improves maintainability.
- Priority: P3 (optimization; improves protocol clarity without affecting functionality)
- Goal: Optimize fit-to-view logic for workflow loading by removing duplicate operations
- What: Consolidate fit requests between initial
fitViewprop in Flow.tsx and the fit orchestrator effect triggered by state hydration in messageHandling.ts - Why: Both mechanisms currently attempt to fit the view when a workflow loads. Evaluating whether the initial
fitViewprop is necessary (or if it ever fires) would allow removal of duplicate fit operations, simplifying the flow and reducing unnecessary DOM queries/animations. - Priority: P2 (optional optimization; reduces complexity without affecting user experience)
- Goal: Simplify graph composition mode condition logic and improve performance
- What: Replace
edges.some(...)presence checks withedges.length > 0in node-sorting.ts during edge filtering initialization - Why: When checking whether a filtered edge set exists for node composition,
length > 0is more direct and avoids the overhead of thesome()method which stops iteration early but still adds function call overhead. This is a minor optimization but improves code clarity by using the most straightforward boolean test. - Priority: P2 (code quality; minor performance improvement and readability)
- Goal: Reduce code duplication and maintenance burden for assistant timeline helpers
- What: Extract shared timeline-building and stringify helpers used in both ExecuteSingleNode.ts and ExecuteWorkflow.ts into a centralized module (e.g.,
timeline.tsorassistantHelpers.ts) - Why: Timeline and stringify logic is duplicated across execution paths, increasing maintenance burden when timeline format changes and risking divergence between single-node and workflow execution. A shared helper ensures consistent timeline semantics across both paths.
- Priority: P2 (code quality; improves maintainability and reduces drift risk)
- Goal: Improve type safety and reduce runtime uncertainty in variable handling
- What: Replace multiple
as anycasts around variable map construction and interpolation in ExecuteSingleNode.ts and ExecuteWorkflow.ts with a lightweight helper function that properly types variable objects - Why: Numerous
as anyassertions reduce type safety and make refactoring risky. A small helper that constructs and validates the variable map would eliminate assertions and clarify the expected data shape for interpolation operations. - Priority: P2 (code quality; improves type safety and maintainability)
- Goal: Prevent potential null-ref errors in single-node LLM approval flow
- What: Add a defensive null/undefined check for
thread.idbefore passing toamp.sendToolInputin ExecuteSingleNode.ts - Why: If thread object is missing or malformed, calling
amp.sendToolInputwith undefined thread ID could cause silent failures or runtime errors. An explicit guard provides clearer error messaging and prevents downstream issues. - Priority: P2 (defensive programming; prevents edge case errors)
- Goal: Reduce console noise while maintaining debuggability for development
- What: Gate new
console.debugtraces in ExecuteSingleNode.ts and ExecuteWorkflow.ts behind an environment flag (e.g.,DEBUG_WORKFLOW_EXECUTION=trueor check againstprocess.env.NODE_ENV) - Why: Debug logging during normal operation adds noise to the extension console. Gating behind an environment flag allows developers to opt in to detailed tracing without affecting end-user experience or test output.
- Priority: P2 (code quality; improves observability without affecting default behavior)
- Goal: Prevent state desynchronization between modal component and underlying dialog primitive
- What: Forward the
openboolean parameter in theonOpenChangecallback to the Dialog component in ConfirmDeleteWorkflowModal.tsx, matching the dialog API contract- Current:
onOpenChange={() => setOpen(!open)}(toggle semantics) - Recommended:
onOpenChange={(isOpen) => setOpen(isOpen)}(forward the boolean state)
- Current:
- Why: The dialog component provides the boolean state parameter for a reason; ignoring it means the modal state could drift from the dialog's internal state if future behavior depends on programmatic state updates or external control
- Priority: P1 (robustness; prevents future state desync issues)
- Goal: Prevent unsafe clearing during active workflow execution
- What: Disable the "Clear" (Trash) button when a workflow is executing (check
executingprop from parent)- Evidence: SidebarActionsBar.tsx renders Clear button without execution state guard
- Related: Flow.tsx tracks execution state and passes
executingto SidebarActionsBar
- Why: If clearing while nodes are executing is unsafe (partial state cleanup, race conditions), the button should be visually disabled to prevent accidental invocation during active runs
- Priority: P2 (UX; prevents mixed trigger semantics if clearing mid-execution has unintended side effects)
- Goal: Prevent modal close before async operations complete
- What: If
onClear()can be async or throw errors, guard the modal close and await completion before callingsetOpen(false)- Current code: SidebarActionsBar.tsx calls
onClear()but doesn't await or guard for errors
- Current code: SidebarActionsBar.tsx calls
- Why: If
onClear()initiates async work (file operations, state cleanup) and the modal closes immediately, the user might think the action failed. Guarding ensures the modal stays open until the callback completes. - Priority: P2 (UX/robustness; improves feedback for async operations)
- Goal: Improve screen reader support for confirmation dialog
- What: Add
aria-describedbyattribute linking the dialog description text to the heading in ConfirmDeleteWorkflowModal.tsx - Why: Screen reader users benefit from explicit semantic links that clarify the purpose and content of the dialog. The current structure has description text but lacks the accessibility contract.
- Priority: P2 (accessibility; improves screen reader experience)
- Goal: Improve UX feedback when connecting to fan-in node bodies during drag operations
- What: Relax drag-time validation for body hover on fan-in nodes; if
targetHandleis undefined during hover (mid-drag), return true so the UI can snap to the fan-in body; strict enforcement still happens in onConnect with the patched handle assigned- Affects edgeValidation.ts
isValidEdgeConnectioncall in Flow.tsx
- Affects edgeValidation.ts
- Why: Currently connections are invalid during drag if
targetHandleis absent, blocking snap/highlight when aiming at the fan-in body. Tolerating undefined handles during drag provides visual feedback; strict validation defers to onConnect where the handle is assigned. This improves UX without compromising correctness. - Priority: P2 (UX improvement; enhances drag feedback for fan-in connections)
- Goal: Eliminate duplication of fan-in handle parsing logic across modules
- What: Unify fan-in index extraction from handle names into a single
parseFanInIndex(handleId: string): numberhelper; use it consistently in edgeOperations.ts, edgeValidation.ts, and nodeStateTransforming.ts - Why: The same parsing pattern (extract numeric index from
in-Nhandle names) appears in three places. A shared helper reduces duplication, improves maintainability, and prevents inconsistencies if parsing logic evolves. - Priority: P1 (code quality; eliminates duplication and improves maintainability)
- Goal: Prevent edge count mismatches when multiple edges are added to the same fan-in node in one tick
- What: Re-check the "used" set inside the functional state update in edgeOperations.ts to detect concurrent edge additions and assign the next truly-free slot instead of relying on a stale snapshot
- Why: Rare race condition: if two edges are added to the same fan-in node in one React tick, both might compute the same "next free" index based on the same snapshot, resulting in duplicate handle assignments. Re-checking inside the functional update prevents this.
- Priority: P1 (robustness; prevents edge case with concurrent connections)
- Goal: Improve hit-testing and click responsiveness on dynamically generated fan-in handles
- What: Consider setting
pointerEvents: 'all'on generated fan-in handles in FanInTargetHandles.tsx if hit-testing edge cases are observed during drag operations - Why: In some React Flow configurations, dynamically generated handles may not receive pointer events correctly if parent container has
pointerEvents: noneor other CSS constraints. An explicitpointerEvents: 'all'ensures reliable hit detection. - Priority: P2 (optional enhancement; investigate if drag snapping issues persist)
- Goal: Clarify UX expectations for fan-in adoption on Text nodes
- What: Confirm user intent for enabling fan-in by default on Text (INPUT) nodes at Nodes.tsx
- Why: Text nodes now support fan-in connections, but the feature is opt-in per node. Confirm whether fan-in should be enabled by default on all new Text nodes or only when explicitly configured. This decision affects default workflow composition.
- Priority: P3 (UX clarity; low impact, informational)
- Goal: Improve type safety of edit event communication for Preview nodes
- What: Create a typed
NebulaEditNodeEventinterface and apply it to thenebula-edit-nodecustom event dispatch in Preview_Node.tsx and listener in Flow.tsx- Define event shape:
{ id: string; action: 'commit'; content: string } - Apply generic type parameter to
CustomEvent<T>whereTis the typed payload - Update event dispatch to cast payload to the typed interface
- Define event shape:
- Why: Currently using
anytype for custom event payload reduces IDE support and makes refactoring riskier. Typed events provide compile-time safety and clearer contract documentation. This follows the same pattern as the Text node edit events. - Priority: P1 (code quality; improves type safety, mirrors Text node enhancement)
- Goal: Prevent bogus percentage displays from malformed tool output
- What: Add clamping or non-negative guard on percent value after parsing in RightSidebar.tsx
- Current:
const percent = parseFloat(item.content?.tokens?.percent) - Recommended:
const percent = Math.max(0, parseFloat(item.content?.tokens?.percent))
- Current:
- Why: Defensive parsing prevents negative or invalid percentages from appearing in the UI when tool output is truncated or malformed
- Priority: P2 (code quality; improves robustness)
- Goal: Eliminate per-render function allocation in Agent Node header
- What: Replace IIFE pattern in header section rendering RightSidebar.tsx with a plain block or extract to a sub-component
- Current:
{(() => { ... })()}inside JSX - Recommended: Move rendering logic to a separate helper function or component defined outside render
- Current:
- Why: Per-render IIFEs create new function objects on each render, adding allocation overhead. Plain blocks or extracted components provide cleaner semantics and better performance
- Priority: P2 (optimization; minor performance improvement)
- Goal: Improve screen reader support for token percentage indicator
- What: Add
aria-labeland optionaltitleattribute to the percentage display span in RightSidebar.tsx- Suggested label: "Token budget used: x %"
- Why: Screen reader users benefit from explicit semantic labels that describe the purpose of the indicator. The label clarifies what the percentage represents without reading raw numbers.
- Priority: P2 (accessibility; improves experience for screen reader users)