Skip to content

feat(webui): add model, from-step, steps, exclude, dry-run to start pipeline dialogs#698

Merged
nextlevelshit merged 2 commits intomainfrom
690-webui-start-flags
Apr 3, 2026
Merged

feat(webui): add model, from-step, steps, exclude, dry-run to start pipeline dialogs#698
nextlevelshit merged 2 commits intomainfrom
690-webui-start-flags

Conversation

@nextlevelshit
Copy link
Copy Markdown
Collaborator

Summary

  • Adds model override dropdown (haiku/sonnet/opus), from-step dropdown, steps multi-select, exclude multi-select, and dry-run toggle to all "Start Pipeline" dialogs across pipelines list, pipeline detail, and runs pages
  • Expands StartPipelineRequest with new fields (Model, FromStep, Steps, Exclude, DryRun) and wires them into ExecutorOptions in the control handler
  • Step lists are dynamically populated from the pipeline definition via existing manifest loader
  • JavaScript startPipeline() collects all form fields and sends them in the API request
  • Comprehensive test coverage for the new handler logic including validation and field wiring

Related to #690

Changes

  • internal/webui/types.go — expanded StartPipelineRequest with Model, FromStep, Steps, Exclude, DryRun fields
  • internal/webui/handlers_control.go — wires new request fields into ExecutorOptions for pipeline execution
  • internal/webui/handlers_control_test.go — new test file with table-driven tests for start pipeline handler
  • internal/webui/static/app.js — updated startPipeline() to collect and send new form fields
  • internal/webui/templates/pipelines.html — added form controls to start dialog
  • internal/webui/templates/pipeline_detail.html — added form controls to start dialog
  • internal/webui/templates/runs.html — added form controls to re-run dialog
  • specs/690-webui-start-flags/ — spec, plan, and tasks documents

Test Plan

  • Unit tests in handlers_control_test.go cover: default request (input only), all flags populated, model validation, empty steps/exclude filtering, and dry-run toggle
  • Manual validation: templates render correct form elements with proper data attributes for step population

@nextlevelshit nextlevelshit force-pushed the 690-webui-start-flags branch from cca56f5 to ae92595 Compare March 30, 2026 21:24
@nextlevelshit nextlevelshit force-pushed the 690-webui-start-flags branch from 154b3f2 to d08f799 Compare April 3, 2026 18:15
…ipeline dialogs #690

Expose 5 high-priority CLI flags in the web UI's Start Pipeline dialogs:

- Model override dropdown (auto/haiku/sonnet/opus)
- From-step dropdown for resume from specific step
- Steps checkboxes for selective step execution
- Exclude checkboxes for skipping steps
- Dry-run toggle for validation without execution

Backend: expand StartPipelineRequest and SubmitRunRequest types, wire
fields into executor options via launchParams, add handleDryRun for
validation-only mode. Validate mutual exclusivity of from-step vs steps.

Frontend: collapsible Advanced Options section on all three start dialogs
(runs page, pipelines list, pipeline detail). Step lists populated
dynamically from GET /api/pipelines/{name} on pipeline selection.
@nextlevelshit nextlevelshit force-pushed the 690-webui-start-flags branch from d08f799 to ef988a6 Compare April 3, 2026 18:21
@nextlevelshit
Copy link
Copy Markdown
Collaborator Author

Code Review: Request Changes

The PR adds Advanced Options UI (model override, step filtering, dry-run) to the WebUI pipeline start dialogs, but has critical implementation gaps that make the feature non-functional. Two JavaScript functions called from both templates (collectAdvancedOptions and showDryRunReport) are never defined anywhere in the codebase, causing uncaught ReferenceErrors when users interact with the Advanced Options section. The from-step dropdown collects user input that the backend silently discards since StartPipelineRequest lacks a FromStep field. Additionally, the security review identified missing request body size limits on two POST handlers, and the quality review found significant code duplication, incomplete mutual exclusivity logic, superficial test assertions, and dead dry-run code paths. The feature cannot ship in its current state.


Critical

  • internal/webui/templates/pipeline_detail.html:187collectAdvancedOptions('qs') is called in submitQuickStart() but is never defined anywhere in the codebase. Clicking 'Start' with advanced options will throw an uncaught ReferenceError, silently failing to start the pipeline.

    Suggestion: Define collectAdvancedOptions(prefix) in app.js to read model dropdown, from-step dropdown, steps/exclude checkboxes, and dry-run checkbox values using the prefix parameter for element ID resolution.

  • internal/webui/templates/pipelines.html:142 — Same undefined collectAdvancedOptions('qs') call duplicated in the pipelines list template. Both templates share this identical bug.

    Suggestion: Define the shared function in app.js rather than inline in each template to avoid further duplication.

  • internal/webui/templates/pipeline_detail.html:193showDryRunReport(data) is called when the dry-run response contains a findings field, but this function is never defined. Currently masked by the backend never returning findings, but remains a latent crash.

    Suggestion: Implement showDryRunReport() in app.js, or remove the dead dry-run branch from both templates.

Major

  • internal/webui/types.go:200 — The from-step dropdown UI collects a value that the backend silently discards. StartPipelineRequest has no FromStep field, and handleStartPipeline never passes a fromStep argument to launchPipelineExecution.

    Suggestion: Either add FromStep string \json:"from_step,omitempty"`toStartPipelineRequest` and wire it through, or remove the from-step dropdown from quickstart dialogs.

  • internal/webui/templates/pipeline_detail.html:208 — Three JS functions (loadQsStepList, onQsFromStepChange, submitQuickStart) and 36 lines of HTML are copy-pasted verbatim between pipeline_detail.html and pipelines.html. Any bug fix must be applied in two places.

    Suggestion: Extract shared JS into app.js and shared HTML into a Go template partial (templates/partials/advanced_options.html), following existing codebase patterns like dag_svg.html and run_row.html.

  • internal/webui/handlers_control.go:204handleStartPipeline and handleSubmitRun decode JSON request bodies without applying http.MaxBytesReader, unlike other POST handlers (handleGateApprove, handleForkRun, handleRewindRun) which set a 1MB limit. An attacker could send arbitrarily large payloads causing memory exhaustion.

    Suggestion: Add r.Body = http.MaxBytesReader(w, r.Body, 1<<20) before the json.NewDecoder call in both handlers.

Minor

  • internal/webui/templates/pipeline_detail.html:237onQsFromStepChange() disables steps checkboxes when from-step is selected, but does not disable exclude checkboxes. The reverse direction is also missing — selecting step checkboxes does not disable from-step. No server-side validation of mutual exclusivity exists.

    Suggestion: Extend onQsFromStepChange to disable exclude checkboxes. Add onStepCheckboxChange to disable from-step when steps are checked. Add server-side 400 validation in the handler.

  • internal/webui/handlers_control.go:142 — Dry-run response handling in templates checks for data.findings but the backend never returns findings — it simply marks the run as completed. The dry-run feature is indistinguishable from a normal start from the user's perspective.

    Suggestion: Either implement DryRunValidator integration in the handler to return findings, or simplify the UI to show a clear "Dry run completed - validated" message.

  • internal/webui/handlers_control_test.go:691 — New tests only assert HTTP 201 status codes without verifying that model/steps/exclude values are passed through to executor options. Missing test cases for mutual exclusivity validation and handleSubmitRun with advanced options.

    Suggestion: Use a mock or spy executor to verify correct ExecutorOption values are passed. Add mutual exclusivity validation test and handleSubmitRun tests.

Suggestions

  • internal/webui/handlers_control.go:223 — Model and Adapter string fields are passed to downstream functions without allowlist validation at the handler level. While downstream code likely handles unknown values gracefully, defense-in-depth suggests validating at the API boundary.

    Suggestion: Validate Model against known values (haiku, sonnet, opus) and Adapter against available adapters before passing to execution options.

  • internal/webui/templates/pipeline_detail.html:136 — Both "Run Only These Steps" and "Exclude Steps" are shown simultaneously as separate checkbox groups, allowing conflicting selections. The runs.html page handles this better with a unified list and auto-detection.

    Suggestion: Adopt the runs.html pattern: single checkbox list with auto-detect steps/exclude mode based on selection count, or add a toggle to switch between include/exclude modes.


Automated review by Wave CI — reviewed at 2026-04-03T21:01:20Z

@nextlevelshit nextlevelshit merged commit 2252156 into main Apr 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant