WIP: Cli exec tui prototype#6490
Draft
6543 wants to merge 12 commits into
Draft
Conversation
Contributor
|
Surge PR preview deployment succeeded. View it at https://woodpecker-ci-woodpecker-pr-6490.surge.sh |
Member
Author
|
… flag No behavior change. Pure refactor that prepares the exec command for the upcoming split between TUI and line-mode output paths. - shared/logger: rename isInteractiveTerminal -> IsInteractiveTerminal so it is usable from cli/exec. Two in-package callers updated. - shared/logger: add SetOutput for programmatic redirection of the zerolog global logger after SetupGlobalLogger has run. Returns a restore func. Intended for the TUI to route diagnostic log output into a ring buffer instead of stderr. - cli/exec/flags: add --no-tui (WOODPECKER_EXEC_NO_TUI). Flag is wired up but not yet consumed; currently a no-op.
Self-contained DAG scheduler for the cli exec command. Consumes the []*builder.Item produced by pipeline/frontend/builder and runs ready workflows in parallel respecting depends_on, up to a worker cap. The scheduler is not yet wired into runExec; that is chunk 3. This chunk only adds the package and its tests. Design notes: - Workflow-level events only (Pending/Ready/Running/Success/Failure/ Blocked/Canceled). Step-level tracing and log lines remain the responsibility of the pipeline runtime tracer/logger the caller configures inside RunFunc. This keeps the scheduler agnostic of rendering concerns and lets both the (upcoming) TUI and the line writer share the same scheduler. - No dependency on server/ code. A package-level comment flags the eventual refactor to unify with server/queue/fifo.go, but that is out of scope for this branch. - Worker cap: Options.Parallel defaults to runtime.NumCPU(); negative means unbounded. No cli flag yet. - Fail-fast is OFF: a failed workflow blocks its dependents but independent siblings keep running. This preserves the multierr behavior of the current sequential loop in runExec. - Context cancellation marks pending workflows Canceled, lets running ones drain through their own ctx, then returns the aggregate error. - Events channel is closed on return so consumers can use range. Tests cover linear ordering, parallel-cap saturation under race detection, transitive blocked propagation, fail-fast-off semantics, context cancel mid-run draining, multi-failure aggregation, empty input, and the defensive duplicate/unknown-dep validation (the builder normally strips these before the scheduler sees them). Coverage: 94.7%.
Replace the sequential for-loop in runExec with a scheduler.New() call. Workflows now run concurrently where depends_on permits, bounded by runtime.NumCPU(). This is the observable behavior change of this chunk: independent workflows finish in parallel on line-mode output, matching what the scheduler already made possible. Line output contract (the grep-friendly stream on stderr): - Prefix is now '[step] line' for single-workflow runs, '[wf/step] line' for multi-workflow runs. The wf/ qualifier is necessary because interleaved parallel output is otherwise unattributable. - Dropped the 'Ln' line counter — redundant with the consumer's own line counting — and the elapsed-seconds suffix, which collides with zerolog's own timestamps when users pipe both streams together. - Prefix body is capped at 24 chars with an ellipsis, to stop long step names from pushing the log body off an 80-column terminal. - '# <workflow>' banner still emitted when a workflow starts running. The trigger is now a scheduler event (StateRunning) rather than the loop iterator, so banners appear in actual-start order and not in declaration order — matches what parallel execution produces. - Blocked and canceled workflows emit a short diagnostic line so the user understands why a workflow produced no step output. exec.go: - runExec now builds one pipelineCtx (timeout + SIGTERM) for the whole DAG instead of one per workflow. The scheduler hands each RunFunc invocation a child of this ctx so cancellation fans out. - Logger factory is per-workflow so the multi-workflow prefix is injected at LineWriter construction, not at each Write call. - Dropped the //nolint:contextcheck tag that covered the old sequential loop's ctx mismatch; no longer needed. - Removed the stale TODOs about parallelism and depends_on. Tests: - New cli/exec/line_test.go covers prefix format (single + multi workflow), truncation, missing-newline fix-up, Close no-op, and the '# name' banner contract. make test-cli, golangci-lint, and gofumpt are all clean. The TUI path is still not wired — --no-tui is inert; that comes in chunk 6.
Package skeleton for the interactive split-pane display. This chunk
compiles and has unit-tested pure-logic pieces, but the package is
not yet imported from runExec — the TUI mode stays inert until
chunk 6 wires it in.
Files:
- ringbuf.go Ring is a FIFO line buffer with a byte cap and a
truncation counter. Safe for one writer + one
reader. Used per step for the log pane and once
for the zerolog debug tab.
- budget.go Budget tracks multiple rings against a shared cap
and evicts oldest-from-largest-ring on Enforce.
Constants GlobalLogCapBytes = 200 MiB for the
shared step budget and DebugLogCapBytes = 5 MiB
for the separate zerolog ring.
- ringwriter.go io.Writer adapter that splits on newlines and
appends to a Ring. Installed as the zerolog
destination during TUI mode so stderr writes
don't tear the alt-screen buffer. Buffers
incomplete trailing fragments across Write calls
and flushes on teardown.
- messages.go tea.Msg types: WorkflowStateMsg, StepStateMsg,
LogLineMsg, DebugTickMsg, PipelineDoneMsg,
CancelingMsg. Data-only structs so model Update
is unit-testable without a real pipeline.
- model.go Model is the bubbletea Model. Seeded with
workflow names; exposes DebugRing() and
StepRing(workflow, uuid, name) so cli/exec can
wire ring buffers without reaching into model
internals. Update handles each message type;
View returns a placeholderView for now so the
program is runnable end-to-end.
- styles.go Unicode status glyphs and placeholderView. The
placeholder is deliberately prose so end-to-end
wiring can be verified before committing to a
lipgloss-based visual design in chunk 5.
Tests:
- ringbuf_test.go 14 cases: append within/over cap, eviction,
oversized-line policy, unbounded cap, snapshot
independence, concurrent write+read under -race;
budget oldest-from-largest policy, zero cap inert,
no-ring enforce; RingWriter newline splitting,
cross-Write buffering, Flush with/without data.
- model_test.go Lifecycle (pending → running → success → done),
failure rendering, canceling state, step state +
ring routing, unknown-workflow no-op, quit key.
Deferred to chunk 5 and re-added there: Focus type, width/height/
focus fields, cursor, WindowSizeMsg handler — all dead weight in
the skeleton, cleaner to introduce alongside the layout they
actually serve.
No new direct dependencies: bubbletea/v2, bubbles/v2, lipgloss/v2
were already indirect via huh/v2. Lint, vet, tests, and the full
build-cli are all green.
Full split-pane layout. The tui package now renders a real interactive UI instead of the chunk-4 prose placeholder. It is still not wired into runExec — chunk 6 does that — so running the cli continues to produce line-mode output for now. Layout: - Left: workflow tree with per-step rows, status glyphs, expand/ collapse markers, and a cursor highlight when the tree has focus. - Right: tabbed pane (logs | debug). The log tab is backed by a bubbles/v2 viewport populated from the selected step's Ring; auto- scrolls to bottom only when already at bottom so scrolling back through history isn't snatched away by new lines. The debug tab is backed by a separate viewport sourced from the zerolog Ring. - Footer: focus indicator, N/M step counter, keybind hint. Switches to 'canceling…' / 'done' / 'failed' as the pipeline progresses. State / model changes: - Re-introduced the Focus enum (FocusTree / FocusLog / FocusDebug) and the UI state fields (width, height, cursor, focus, viewReady, logView, debugView) that chunk 4 deliberately omitted. The placeholder view in styles.go stays as the fallback before the first WindowSizeMsg arrives. - Update now handles WindowSizeMsg (propagates to both viewports + flags viewReady), refreshes the active pane on LogLineMsg / DebugTickMsg when the change is visible, and routes KeyPressMsg through a new focus-aware dispatcher. - Renamed CancellingMsg -> CancelingMsg to fix the misspell lint hit that survived chunk 4. New file view.go: - flatten() returns the navigable tree rows in render order — single source of truth used by cursor movement AND the renderer, so 'j' always moves the cursor to something the user sees. - layout() computes the pane widths (3/8 tree, 5/8 log, minimum 22 cols each) and reserves one row for the footer. - resizeViewports() recomputes bubbles viewport sizes on resize. - refreshLogView() / refreshDebugView() rebuild viewport content from the ring snapshots, prepending '[… N lines truncated]' when Ring has evicted anything. - Per plan §7b: lipgloss JoinHorizontal of tree+right, JoinVertical for body+footer. Unicode status glyphs from styles.go. Tree rows get a '›' selection prefix on top of the reverse-video style so focus survives themes without reverse support. New file view_test.go (12 cases) covers pane structure, cursor movement with bounds saturation, tab cycling through all three focus states, L jump to debug, selected-step log refresh vs unselected-step storage-only, progress counter transition, the canceling / failed footer states, and g/G top/bottom navigation. A plainView helper uses charmbracelet/x/ansi.Strip so assertions survive lipgloss's per-rune ANSI wrapping in styled output. Styling (styles.go): - Two colors from the 16-color ANSI palette: accent (cyan) for focused borders and active tabs; muted (bright black) for unfocused borders and inactive tabs. State-specific colors (success/failure/warning) will land with glyph coloring in the polish chunk. - paneStyle(focused bool) returns a rounded-border style with the border color swapped on focus. selectedRowStyle is reverse video so it works on any terminal. Named constants to satisfy mnd: paneBorderWidth, rightPaneTabsHeight, minTotalWidthMultiple, rowInnerPadding. shared/logger/logger.go: reworded two lines in the SetOutput docstring so godot is happy when the wider scope gets linted. No behavior change. Verification: gofumpt clean, vet clean, golangci-lint 0 issues on cli/exec/tui/... and shared/logger/..., 31/31 tests pass under -race, full build-cli green.
runExec now dispatches between two paths based on '--no-tui' and
whether stdout is a terminal. The existing line-mode path is
unchanged for non-interactive invocations and for users who pass
'--no-tui'; on an interactive terminal, the TUI takes over.
exec.go changes:
- Extract the scheduler-driving logic that existed inline in runExec
into runLineMode, unchanged in behavior. The mode decision at the
top of runExec is a single conditional:
useTUI := !c.Bool("no-tui") && logger.IsInteractiveTerminal()
- schedulerEventBuffer = 64 extracted as a named const. The buffer
size mattered to lint (mnd) once it showed up in two call sites,
and naming it lets the rationale live next to the value.
New file exec_tui.go:
- runTUIMode seeds a tui.Model with the workflow names from
builder.Items, installs a RingWriter as the zerolog destination
via logger.SetOutput so stderr writes do not tear the alt-screen,
builds a tea.Program, and starts four cooperating goroutines:
1. signal handler — two-stage sigint. First signal cancels the
run context and Sends CancelingMsg so the model flips its
footer. Second signal os.Exit(130), abandoning the alt-screen
(the terminal restores on process exit).
2. scheduler events drainer — forwards each scheduler.Event to
p.Send(WorkflowStateMsg{Event: ev}).
3. scheduler.Run — executes the DAG; Sends PipelineDoneMsg when
it returns so the model can transition to its final state.
4. the tea event loop (main goroutine via p.Run).
- tuiRunFunc builds a per-workflow tracer that chains
tracing.DefaultTracer (so env vars still populate) with a
prog.Send of StepStateMsg, and a per-workflow logger that hands
CopyLineByLine a tuiStepWriter which forwards each line as
LogLineMsg. No stderr writes on this path; the tui owns the
display.
- tuiStepWriter is a trivial io.Writer that packages bytes into a
LogLineMsg. Write returns len(p) per the io.Writer contract so
upstream accounting stays correct. Close is a no-op.
- flushDebugRingToStderr runs after the alt-screen is gone and
dumps any accumulated zerolog output back to stderr, prefixed
with a truncation marker if the ring evicted anything. This
preserves diagnostics that would otherwise vanish with the
buffer.
tui/model.go:
- View now sets v.AltScreen = true, which is how bubbletea v2
enables the alternate screen buffer (the v1 tea.WithAltScreen
option no longer exists). This is the only semantic change to
the tui package in this chunk; it doesn't affect tests because
they access .Content.
Constants added: sigintExitCode = 130, sigChanBuffer = 2.
context.WithCancel is used inside runTUIMode to layer an explicit
cancel on top of pipelineCtx; it gets a //nolint:forbidigo with a
rationale because the two-stage sigint handler needs to cancel
independently from the outer timeout.
Verification: gofumpt clean, golangci-lint 0 issues on cli/exec/...,
all cli/exec, scheduler, and tui tests pass under -race,
build-cli green.
Two small but load-bearing fixes that close out the plan. tui/model.go: - Init now returns a tickDebug command. Previously Init returned nil, which meant DebugTickMsg was defined and handled but nothing ever produced one — the budget Enforce and the debug pane refresh simply never fired at runtime. - The DebugTickMsg handler now re-arms itself with tickDebug(), so once the loop starts it runs until tea.Quit. Interval is 250ms, chosen as a named const (debugTickInterval) — fast enough that fresh zerolog lines appear interactively, slow enough that the budget scan is cheap. - tickDebug() is a tea.Tick helper shared by Init and the handler. Two new tests pin the contract: TestModelInitSchedulesDebugTick verifies Init's returned cmd produces a DebugTickMsg; TestModelDebugTickReschedules verifies the handler's cmd also produces one. Without these, a misrefactor that breaks the loop could land without visible failure. exec_tui.go: - The deferred cleanup block now flushes the debug ring to stderr in addition to restoring the logger. Previously flushDebugRing ToStderr ran only on the happy path, at the bottom of the function; if prog.Run panicked — or if any of the surrounding setup did — the zerolog output accumulated during the session was silently lost along with the alt-screen buffer. The defer body documents the ordering: restore the logger first so any log calls that happen during the flush itself go to real stderr rather than back into the ring we're draining; then flush the RingWriter's carried-over fragment; then dump the ring to stderr. This runs on success, error, AND panic paths — which is the whole point. - The duplicate flush call near the bottom of runTUIMode is removed now that the defer covers it. Verification: gofumpt clean, vet clean, golangci-lint 0 issues on cli/exec/..., 34/34 tests pass under -race (tui package: 17 model + ring tests + 12 view tests + 5 from chunks 4-5 that the runner counts separately; scheduler package: 14), build-cli green.
Before this fix, running the CLI with multiple workflows under the
docker backend produced errors like:
Error response from daemon: network with name
wp_01KPYB49XKRGNKWNASBTE298FS_default already exists
Error response from daemon: failed to set up container
networking: network wp_..._default not found
Because two or more parallel workflows were all trying to create,
use, and remove the same docker network and volume. The same
workflows running sequentially (before chunk 3) never hit this —
each finished its teardown before the next started.
Root cause: cli/exec/exec.go built a single prefix with
prefix := "wp_" + ulid.Make().String()
and passed it into compiler.WithPrefix(prefix), making it the prefix
for every workflow the builder compiled in that run. The compiler
derives each workflow's network and volume name from the prefix as
"%s_default"
so all workflows ended up with identical docker resource names.
The pipeline builder (pipeline/frontend/builder/builder.go:220-226)
ALREADY generates a unique per-workflow prefix of the form
wp_<ULID>_<workflowID>
but the CLI's "caller options win" post-append in the builder
(line 231-233) meant the CLI's shared prefix clobbered the
per-workflow one.
Fix:
1. Remove compiler.WithPrefix from CompilerOptions. The builder's
per-workflow prefix now wins, and each workflow's Config.Volume
and Config.Network become genuinely unique.
2. The CLI's local-mode workspace volume mount
"<prefix>_default:<workspace-base>" used to be added via
compiler.WithVolumes(...), which applied globally. With
per-workflow prefixes, the global injection point no longer
exists, so we inject the mount after the build, walking each
item's compiled Config and appending to every step's Volumes.
3. The non-local workspace mount is already handled per-workflow
inside the compiler (convert.go:82), so that path needs no
changes.
New file cli/exec/exec_workspace_test.go pins the behavior:
per-workflow mount isolation, every step across every stage gets
the mount, appending to existing Volumes not replacing them, safe
handling of items with nil Config or empty Volume, and multi-stage
workflows.
This is the bug the smoke test caught after shipping chunk 7 —
sequential execution hid it for years; the scheduler exposed it on
its first real parallel run.
… pane
Previously, anything printed before the TUI took over stdout — lint
warnings, validator output, the '# workflow' banner scheme — was
written directly to the terminal. On an interactive tty, that text
sat in the scrollback for a few milliseconds before the alt-screen
buffer wiped it on TUI startup. Users who needed to see the warnings
had to either quit the TUI to check the scrollback, or re-run with
--no-tui.
The TUI's earlier split-pane layout also had a 'debug' tab in the
right pane that carried the same mental-model: zerolog diagnostics
that were neither step output nor tree state. In practice the two
categories of content (pre-run warnings vs runtime diagnostics) are
the same thing: everything-else. This change merges them.
Layout change:
before after
┌──────┬───────────────┐ ┌──────┬────────────────┐
│ tree │ log │ debug │ │ tree │ log │
│ │ (tabbed) │ │ │ │
└──────┴───────────────┘ ├──────┴────────────────┤
│ messages │
└───────────────────────┘
The right pane is now dedicated to step log output only. A new
full-width 'messages' strip sits below the top row and above the
keybind footer, with defaultMessagesHeight = 8 rows (clamped against
minTopRowHeight = 6 and minMessagesHeight = 3 on tight terminals).
cli/exec/exec.go:
- The useTUI decision moves up ahead of the lint-warning print site.
In TUI mode the warning text is captured into a strings.Builder
and passed to runTUIMode as a new preRunMessages parameter
instead of hitting stdout. Line mode keeps printing to stdout
exactly as before — the output contract for CI logs is unchanged.
cli/exec/exec_tui.go:
- runTUIMode takes the new preRunMessages string. Each line is
appended to the model's messages ring via strings.SplitAfter so
trailing newlines are preserved. Zerolog's RingWriter is still
installed into the same ring, so the pane seamlessly transitions
from pre-run seed lines to runtime diagnostics with no visual
split.
cli/exec/tui/:
- view.go: new layout() returns four values (tree width, log width,
top-row height, messages height). resizeViewports sizes both
bubbles viewports accordingly. renderView composes
JoinHorizontal(tree, logPane) over messagesPane over footer.
renderRightPane + renderTabs replaced by renderLogPane and
renderMessagesPane. The log pane title annotates with the
selected 'logs: <workflow>/<step>' so the user knows what
they're reading without cross-referencing the tree cursor.
- styles.go: tabInactiveStyle dropped (no more tabs). Renamed
tabActiveStyle -> paneTitleStyle to match its new role.
- model.go: Focus enum: FocusDebug -> FocusMessages. Field renames
m.debug -> m.messages, m.debugView -> m.messagesView,
refreshDebugView -> refreshMessagesView. DebugRing() ->
MessagesRing() on the public surface. DebugLogCapBytes ->
MessagesLogCapBytes. DebugTickMsg kept (still drives refreshes,
name describes the role not the pane).
- Layout constants added: defaultMessagesHeight = 8,
minTopRowHeight = 6, minMessagesHeight = 3, paneBorderHeight = 2.
Obsolete rightPaneTabsHeight removed.
- L keybind still jumps to the messages pane (previously 'debug').
Footer shows [tree] / [log] / [messages].
Tests:
- view_test.go: existing 'right-pane tabs must render' assertion
becomes 'messages pane must render' on the word 'messages'.
TestDebugKeyJumpsToDebugPane renamed to TestLKeyJumpsToMessages
Pane with the footer assertion updated to '[messages]'. New
TestPreRunMessagesAppearInMessagesPane seeds the ring with two
lint-warning lines, drives the WindowSizeMsg + DebugTickMsg
sequence, and asserts both lines appear in the rendered frame.
Verification: gofumpt clean, vet clean, golangci-lint 0 issues on
cli/exec/..., 32/32 tui tests + all cli/exec tests pass under -race,
build-cli succeeds, line-mode smoke still shows warnings on stdout
as before.
Previously, step rows in the tree materialized lazily: a step only appeared once its first tracer event or first log line arrived, which is also the moment it started running. Pending steps were invisible until they began executing, so the tree effectively showed 'running' and 'finished' steps but never 'what's coming next'. This change pre-seeds the model with the compiled step list for each workflow so every step is visible from the moment the TUI starts, with a pending glyph (·). Each step then visibly transitions through pending → running (●) → success (✓) / failure (✗) / skipped (⊘) as the pipeline executes — giving the user a real-time picture of the plan and its progress instead of pop-ins at each step start. cli/exec/tui/model.go: - New NewFromSeeds constructor takes []WorkflowSeed, each with a Name and a []StepSeed describing the steps the workflow will run. Seeds a stepNode with a log ring per step, registered with the global budget, in StatePending visually. - The bare New(workflowNames) constructor is preserved as a thin wrapper over NewFromSeeds for tests that don't care about steps. - WorkflowSeed and StepSeed are tui-local types, not pass-through of builder.Item — keeps the tui package from depending on the builder and lets tests build fixtures without constructing a full backend_types.Config. - stepNode grows a 'started' bool. It flips true the first time a tracer event reports Started != 0, distinguishing pending from running (both have Exited=false). - handleStepState and StepRing now go through a shared findOrCreateStep helper that matches by UUID first, then by name, then (defensively) creates a new node. The seeded-UUID happy path means production runs never create duplicates; the name fallback keeps tests that use the bare New() constructor working; the create fallback covers unknown steps that might sneak in from future runtime sources. cli/exec/tui/view.go: - stepGlyph now returns glyphPending (·) when neither terminal states nor started are set. Ordering in the switch: terminal first (skipped/success/failure/oomKill), then started (running), then fallback (pending) — so a brief terminal-state-without- started-flip doesn't flicker back through pending. cli/exec/tui/styles.go: - The placeholder view's ad-hoc step glyph logic replaced with a call to stepGlyph so pre-WindowSizeMsg renders match the full layout's treatment. cli/exec/exec_tui.go: - runTUIMode now builds []tui.WorkflowSeed from each item's Config.Stages[].Steps[] and calls tui.NewFromSeeds instead of passing just workflow names. Empty or nil Config is handled gracefully (the workflow still seeds with no steps, matching the pre-change behavior for that edge case). Tests (cli/exec/tui/view_test.go): - TestSeededStepsAppearAsPendingBeforeRunning: three seeded steps all render in the tree with no running/success/failure glyphs present — they're all pending. - TestStepTransitionsPendingToRunningToSuccess: walks a single step through pending → running (Started=<now>, Exited=false) → success (Exited, ExitCode=0), asserting the correct glyph at each stage. - TestStepSeededByUUIDDoesNotDuplicate: seed a step, then send a tracer event with the same UUID; verify the step name appears exactly once in the tree region (no duplicate row from the lookup-by-uuid path missing the seed). Verification: gofumpt clean, vet clean, golangci-lint 0 issues, all tui tests (34/34) pass under -race, full cli/exec tests pass, build-cli green, line-mode smoke still prints warnings to stdout unchanged.
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
!! DO NOT CHERRY PICK !!
disclaymer: claude was used to just see how it could look like and get some impressions... we can take some idears but should not copy the code!!!!
this just drafts what is rougth planed in #6488
it needs to be refined a lot -> #6490 (comment)
depends on:
server/.../step_builderintopipeline/.../builder#3967current state...
example pipeline in webui:
Issues