From 34652dc5846e66c55647e13b213bcbad09bdc851 Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Fri, 24 Jul 2026 17:39:36 -0700 Subject: [PATCH 1/3] fix(gitinfo): pick workflow-dispatch inputs and stop command-goroutine crashes Prompt for each workflow_dispatch input in turn: choice and boolean inputs are picked from their valid values (preselected on the workflow default), and free-form inputs are pre-filled with the default and accept a custom value, instead of a single free-form key=value blob. A workflow with no inputs dispatches directly; unreadable inputs fall back to the free-form composer. Fix the residual dispatch crash: panics inside Bubble Tea command goroutines are uncaught by crashlog.GuardTUI (which only guards Init/Update/View) and become a fatal ErrProgramPanic. Add crashlog.CaptureCmdPanic (a quiet crash-report writer) and gitinfo.guardedGitHubCmd, wrapping every GitHub data loader and the dispatch closures so a panic becomes a crash report plus an error toast instead of killing the TUI; a recovered panic also clears stranded Loading state. Also guard seven GitHub command functions (cancel run, merge PR, comment, request reviewers, close/reopen issue, create PR, assign self) against a nil client, and serialise crashlog.Write with collision-free filenames so concurrent command panics no longer overwrite reports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b9a3d924-7b48-4000-9d4d-fb592f3485b6 --- CHANGELOG.md | 7 + .../workflow-dispatch-param-picker/spec.md | 100 ++++++ .../test-plan.md | 65 ++++ internal/crashlog/crashlog_test.go | 42 +++ internal/crashlog/recovery.go | 33 ++ internal/crashlog/writer.go | 24 +- internal/notify/messages.go | 1 + internal/notify/modal.go | 29 ++ internal/notify/notify.go | 2 +- internal/notify/notify_test.go | 37 +++ internal/panels/gitinfo/gitinfo.go | 47 ++- .../gitinfo/gitinfo_dispatch_panic_test.go | 303 ++++++++++++++++-- internal/panels/gitinfo/gitinfo_github.go | 246 ++++++++++++-- .../panels/gitinfo/gitinfo_helpers_test.go | 26 +- .../panels/gitinfo/gitinfo_modal_handlers.go | 88 ++--- .../panels/gitinfo/gitinfo_notifications.go | 4 +- web/src/pages/docs/github-workflows.astro | 6 +- 17 files changed, 938 insertions(+), 122 deletions(-) create mode 100644 docs/specs/workflow-dispatch-param-picker/spec.md create mode 100644 docs/specs/workflow-dispatch-param-picker/test-plan.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 04e8fd40..8c2f4138 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- Dispatching a GitHub Actions workflow now prompts for each `workflow_dispatch` input in turn: `choice` and `boolean` inputs are picked from their valid values (preselected on the workflow's default) and free-form inputs are pre-filled with the default and accept a custom value, instead of a single free-form `key=value` text blob. + +### Fixed +- grut no longer crashes when dispatching a GitHub Actions workflow. Panics inside Bubble Tea command goroutines (which the model-level panic guard cannot catch) are now captured to a crash report and surfaced as an error toast instead of killing the TUI, across the GitHub data and dispatch paths. +- Several GitHub actions (cancel run, merge PR, comment, request reviewers, close/reopen issue, create PR, assign self) now show a clear error instead of risking a crash when no GitHub client is available. + ## [0.3.0] - 2026-05-18 ### Contributors diff --git a/docs/specs/workflow-dispatch-param-picker/spec.md b/docs/specs/workflow-dispatch-param-picker/spec.md new file mode 100644 index 00000000..11f1b744 --- /dev/null +++ b/docs/specs/workflow-dispatch-param-picker/spec.md @@ -0,0 +1,100 @@ +--- +title: Workflow-dispatch parameter picker and command-goroutine crash safety net +status: draft +issue: pending +scope: P1 +--- + +## Problem + +Dispatching a GitHub Actions workflow from the GitHub panel had two shortcomings: + +1. **Parameter entry was a raw text blob.** Every `workflow_dispatch` input — + including `choice` and `boolean` inputs that have a fixed set of valid + values — was entered as free-form `key=value` lines in a single multi-line + composer. Users had to know the valid options and type them exactly, with no + guidance and no protection against invalid values. + +2. **grut still crashed after a dispatch.** Even after PR #362 added + `crashlog.GuardTUI`, dispatching could still kill the whole TUI with + `program was killed: program experienced a panic`, leaving an empty crashes + directory. `GuardTUI` only guards the model's `Init`/`Update`/`View` on the + main goroutine; a panic inside a `tea.Cmd` goroutine (run by Bubble Tea's + `execBatchMsg`) is converted by `recoverFromGoPanic` into a fatal + `ErrProgramPanic` that no crash report captures. Historical + `bubbletea-panic-*.log` stacks pinpointed nil-client dereferences in the + GitHub data loaders (`loadGitHubMeta`, `loadIssuesPage`, `loadReleasesPage`) + running in those goroutines. + +## Approach + +- **Per-input prompting.** After the ref is chosen, prompt once per declared + input. `choice` and `boolean` inputs are shown as an action picker of their + valid values, preselected on the workflow's declared default. Free-form + inputs (`string`, `environment`, unspecified) use a text field pre-filled + with the default, so the user can accept it or enter a custom value. A + workflow with no inputs dispatches immediately. If the workflow YAML cannot + be read, fall back to the free-form `key=value` composer. + +- **Command-goroutine crash safety net.** Add `crashlog.CaptureCmdPanic`, a + quiet crash-report writer (no stderr output, since the TUI still owns the + terminal), and `gitinfo.guardedGitHubCmd`, which wraps a GitHub command + closure so a panic becomes a crash report plus an error toast instead of + killing the TUI. Wrap every GitHub data loader and the dispatch closures. + +- **Close the remaining nil-client gap.** Seven command functions + (`cancelWorkflowRunCmd`, `mergePRCmd`, `commentCmd`, `requestReviewersCmd`, + `closeReopenIssueCmd`, `createPRCmd`, `assignSelfCmd`) still dereferenced a + nil client in their goroutine. Guard each with an explicit failure toast, + matching the loaders fixed in 659cc28. + +## Trade-offs / decisions + +- **Picker vs. custom value for `choice`.** GitHub rejects a `choice` value that + is not one of the declared options, so `choice`/`boolean` inputs are + picker-only (no custom entry); only free-form inputs allow a custom value. + This matches GitHub's own dispatch semantics. +- **Recover-and-toast vs. re-panic in command goroutines.** `GuardTUI` + re-panics so Bubble Tea can restore the terminal; a command goroutine must + instead recover *without* re-panicking, because Bubble Tea's + `recoverFromGoPanic` turns any re-panic into a fatal program kill. So + `guardedGitHubCmd` recovers, writes a report, and returns an error toast. +- **Empty input value keeps the default.** A blank value is omitted from the + dispatch payload so GitHub applies the workflow's declared default rather + than an empty override. + +## Out of scope + +- `environment`-type inputs are not populated from the repository's configured + environments (they use a free-form text field); fetching environments is a + separate enhancement. + +## Acceptance criteria + +- AC1: A `choice` input is presented as a picker of its options, preselected on + the declared default. +- AC2: A `boolean` input is presented as a true/false picker, preselected on + the declared default. +- AC3: A free-form input is a text field pre-filled with the declared default + and accepts a custom value. +- AC4: A workflow with no declared inputs dispatches without prompting. +- AC5: When the workflow inputs cannot be read, the free-form `key=value` + composer is offered as a fallback. +- AC6: A blank input value is omitted so the workflow default is applied. +- AC7: A panic inside a GitHub command goroutine is captured to a crash report + and surfaced as an error toast instead of killing the TUI. +- AC8: The seven previously-unguarded GitHub command functions surface a nil + client as a failure toast rather than crashing. + + +## Pipeline Status +Phase: SHIPPING (late entry via `go verify`) + +Certification (Phase 4): `go build`, `go vet ./...`, `golangci-lint ./...` (0 issues), +`gofumpt -l .` (clean), `deadcode ./...` (0 new; all 210 findings pre-existing/allowlisted), +`govulncheck ./...` (no vulnerabilities), full `go test ./...` (exit 0). Race/WSL/benchmark +steps of `mage preflight` are CI-covered (need cgo/WSL unavailable locally). doc-check: astro +GitHub-workflows page + CHANGELOG updated; keybindings unchanged (up-to-date test passes). +Review: code-review (1 MEDIUM, fixed) + rubber-duck (crash-collision + stranded-loading fixed; +async-identity/inputsKnown/blank-trim assessed as pre-existing or intentional — see test-plan +triage). Test plan COVERED, 0 gaps. diff --git a/docs/specs/workflow-dispatch-param-picker/test-plan.md b/docs/specs/workflow-dispatch-param-picker/test-plan.md new file mode 100644 index 00000000..f47b80bc --- /dev/null +++ b/docs/specs/workflow-dispatch-param-picker/test-plan.md @@ -0,0 +1,65 @@ +# Test Plan — Workflow-dispatch parameter picker and crash safety net + +## Status: COVERED + +## Planned Tests + +| ID | AC | Description | Test | State | +|----|----|-------------|------|-------| +| T1 | AC1 | `choice` input → action picker of options, preselected on default | `TestHandleWorkflowInputsFetched_KnownInputsUsePicker` | automated | +| T2 | AC2 | `boolean` input → true/false picker, preselected on default | `TestHandleWorkflowInputsFetched_BooleanInputUsesPicker` | automated | +| T3 | AC3 | free-form input → text field pre-filled with default | `TestHandleWorkflowInputsFetched_FreeFormInputUsesTextField` | automated | +| T4 | AC1,AC3,AC6 | multi-input flow collects a picked choice + a custom text value and dispatches both | `TestWorkflowDispatch_MultiInput_CollectsChoiceAndText` | automated | +| T5 | AC4 | no declared inputs → dispatches directly, no prompt | `TestHandleWorkflowInputsFetched_NoInputsDispatchesDirectly` | automated | +| T6 | AC5 | unreadable inputs → free-form composer fallback (`opWorkflowDispatchRaw`) | `TestHandleWorkflowInputsFetched_UnknownInputsUseRawComposer`, `TestHandleWorkflowInputsFetched_NoInputs` | automated | +| T7 | AC6 | blank value omitted so workflow default applies | `TestWorkflowDispatch_EmptyValueKeepsDefault` | automated | +| T8 | AC7 | panic in a GitHub command goroutine → `ghCmdPanicMsg` (no crash) | `TestGuardedGitHubCmd_RecoversPanicToPanicMsg` | automated | +| T9 | AC7 | guarded command is transparent on the normal path | `TestGuardedGitHubCmd_PassesThroughNormalMsg` | automated | +| T10 | AC7 | `crashlog.CaptureCmdPanic` writes a report and records the last-crash path, quietly | `TestCaptureCmdPanic_WritesReportAndRecordsPath` | automated | +| T11 | AC8 | per-input dispatch path guards a nil client (no panic, error result) | `TestFireWorkflowDispatch_NilClientNoPanic` | automated | +| T12 | AC8 | free-form fallback path guards a nil client (no panic, error toast) | `TestHandleWorkflowDispatchRaw_NilClientNoPanic` | automated | +| T13 | AC1 | picker preselects the selected action's cursor, not always index 0 | `notify.TestShowActionPickerWithSelection_PreselectsCursor`, `notify.TestActionIndexByID` | automated | +| T14 | AC1..AC6 | full dispatch flow (D → ref → per-input → submit → result → toast) does not panic and dispatches once | `TestWorkflowDispatch_FullFlow_NoPanicSuccessToast` | automated | +| T15 | AC7 | post-dispatch render across sizes does not panic | `TestWorkflowDispatch_RenderAfterReload_NoPanic` | automated | +| T16 | AC7 | panic message clears stranded loading flags + shows error toast | `TestHandleGHCmdPanic_ClearsLoadingAndToasts` | automated | +| T17 | AC7 | concurrent command-goroutine panics each get a distinct crash file (no overwrite) | `TestCaptureCmdPanic_ConcurrentReportsAreDistinct` | automated | +| T18 | AC8 | representative guarded action (`cancelWorkflowRunCmd`) yields an error toast on nil client | `TestGuardedCommand_NilClientYieldsToast` | automated | +| T19 | AC1..AC3 | input prompt label building (description/name/required) | `TestWorkflowInputPrompt` | automated | + +## Functionality Inventory (Phase 3 reconciliation) + +Built from `git diff` of the changed source files. Every unit maps to a covering test. + +| Unit | Covering test | +|------|---------------| +| `notify.ShowActionPickerWithSelection` (new constructor) | T13, and used by T1/T2/T4 | +| `notify.actionIndexByID` (helper) | T13 | +| `ShowModalMsg.SelectedID` + manager preselect (`actionCursor`) | T13 | +| `crashlog.CaptureCmdPanic` (quiet writer, records path) | T10 | +| `crashlog.Write` mutex + atomic-seq unique filename | T17 (distinct files, `-race` in CI) | +| `gitinfo.guardedGitHubCmd` (recover → `ghCmdPanicMsg`) | T8, T9 | +| `gitinfo.ghCmdPanicMsg` + `handleGHCmdPanic` (clear loading + toast) | T16 | +| `gitinfo.ghClientUnavailableCmd` (nil-client toast) | T11 (per-input), T12 (raw), T18 | +| `gitinfo.dispatchWorkflowCmd` (shared dispatch cmd + guards) | T4, T7 (fire path), T12 (raw path), T11 (nil client) | +| `handleWorkflowInputsFetched` (known/unknown/no-input branches) | T1, T3, T5, T6 | +| `promptNextWorkflowInput` (choice/boolean/text switch + fire) | T1, T2, T3, T4 | +| `fireWorkflowDispatch` (collect values, reset draft, delegate) | T4, T7, T11 | +| `handleWorkflowDispatchInputs` (record value, advance idx) | T4, T7, T14 | +| `handleWorkflowDispatchRaw` + `parseKeyValueInputs` | T6, T12 | +| `handleWorkflowDispatch` sets `inputsKnown` | T14 (`fetched.inputsKnown` asserted) | +| `workflowInputPrompt` / `optionsToActions` | T19; picker actions exercised by T1/T2/T4 | +| nil-client guards on 7 command funcs (cancel/merge/comment/reviewers/close/createPR/assignSelf) | T18 (representative) + shared `ghClientUnavailableCmd` (T11/T12); pre-existing per-command tests still pass | +| loader closures wrapped with `guardedGitHubCmd` (loadGitHubData/Meta/Issues/PRs/Actions/Workflows/Releases/Notifications) | T8/T9/T16 cover the wrapper + stranded-loading recovery; existing loader tests assert unchanged normal-path behavior | + +**Gaps: 0.** + +## Review findings triage (Phase 3) + +- **code-review MEDIUM — crash-report filename collision + unsynchronized `Write`** (introduced by making `Write` concurrent/repeatable via `CaptureCmdPanic`): FIXED — `sync.Mutex` around `Write`/`pruneOld` + atomic-sequence unique filenames (T17). +- **rubber-duck #3 — panic recovery could strand a "Loading…" flag**: FIXED — `guardedGitHubCmd` now emits `ghCmdPanicMsg`, handled by `handleGHCmdPanic` which clears loading flags (T16). +- **rubber-duck #5** duplicates the crash-collision finding: FIXED (T17). +- **rubber-duck #1 (stale async / no request identity)**: NOT CHANGED — the dispatch flow is a strictly sequential, modal-gated flow; the async-fetch window and use of current-repo identity are pre-existing characteristics (the prior `pendingName`-based flow behaved identically), not a regression. A generation-ID system is over-engineering for a single-modal flow. Documented as out of scope. +- **rubber-duck #2 (`inputsKnown` cannot distinguish parse-failure from no-inputs)**: NOT CHANGED — `GetWorkflowInputs` returns `(nil,nil)` only for valid no-input cases and for YAML that fails to parse; a workflow listed by the API is GitHub-validated YAML, so the parse-failure branch is practically unreachable, and dispatching a genuinely-broken/no-trigger workflow fails fast at GitHub (422) rather than after a pointless composer. Acceptable. +- **rubber-duck #4 (blank/trim)**: NOT CHANGED — blank = "use the workflow default" is the intentional, documented design (AC6); choice/boolean pickers return non-empty IDs so omission never triggers for them; whitespace-padded options are pathological. +- **Self-finding (refactoring)**: duplicated dispatch-command construction extracted into `dispatchWorkflowCmd`. + diff --git a/internal/crashlog/crashlog_test.go b/internal/crashlog/crashlog_test.go index 0d47bfb8..c6046a6c 100644 --- a/internal/crashlog/crashlog_test.go +++ b/internal/crashlog/crashlog_test.go @@ -496,6 +496,48 @@ func TestWriteRecovery_WithLogTail(t *testing.T) { assert.Contains(t, reports[0].LogTail[0], "pre-crash-log") } +// --------------------------------------------------------------------------- +// CaptureCmdPanic +// --------------------------------------------------------------------------- + +func TestCaptureCmdPanic_WritesReportAndRecordsPath(t *testing.T) { + setTestDataHome(t) + + path := CaptureCmdPanic("cmd boom", "gitinfo.loadGitHubData") + require.NotEmpty(t, path, "should return the crash report path") + assert.Equal(t, path, LastCrashPath(), "should record the last crash path") + + reports, err := List() + require.NoError(t, err) + require.Len(t, reports, 1) + assert.Equal(t, "cmd boom", reports[0].PanicValue) + assert.Equal(t, "gitinfo.loadGitHubData", reports[0].Context) +} + +// TestCaptureCmdPanic_ConcurrentReportsAreDistinct verifies that concurrent +// command-goroutine panics each get a distinct crash file (no same-second +// filename collision) and that the write path is race-free (run with -race). +// CaptureCmdPanic keeps the process alive, so — unlike the pre-existing +// write-then-exit callers — Write can be invoked concurrently and repeatedly. +func TestCaptureCmdPanic_ConcurrentReportsAreDistinct(t *testing.T) { + setTestDataHome(t) + + const n = 15 // below maxCrashFiles (20) so none are pruned + var wg sync.WaitGroup + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + CaptureCmdPanic(fmt.Sprintf("panic-%d", i), "concurrent-ctx") + }(i) + } + wg.Wait() + + reports, err := List() + require.NoError(t, err) + assert.Len(t, reports, n, "each concurrent report must get its own file (no overwrite)") +} + // --------------------------------------------------------------------------- // RecoverAndReport // --------------------------------------------------------------------------- diff --git a/internal/crashlog/recovery.go b/internal/crashlog/recovery.go index e7a99204..ab831c62 100644 --- a/internal/crashlog/recovery.go +++ b/internal/crashlog/recovery.go @@ -59,6 +59,39 @@ func WriteRecovery(panicVal any, ctx string, tail *LogTailHandler) { fmt.Fprintf(os.Stderr, "Run 'grut report' to file a GitHub issue.\n") } +// CaptureCmdPanic writes a crash report for a panic recovered inside a Bubble +// Tea command goroutine and records it as the last crash path. Unlike +// WriteRecovery it prints nothing to stderr: the TUI still owns the terminal +// (alternate screen, raw mode), so stray output would corrupt the display. +// It returns the crash report path, or "" if the report could not be written. +// +// Bubble Tea runs each command in its own goroutine and turns any panic there +// into a fatal ErrProgramPanic that tears the whole program down. GuardTUI only +// covers the model's Init/Update/View on the main goroutine, so a command that +// panics — e.g. on a nil field in live API data — would otherwise kill the TUI. +// Recovering in the command closure and reporting via this function keeps the +// TUI alive while preserving the stack for diagnosis. Use it from a deferred +// recover in the command: +// +// return func() (msg tea.Msg) { +// defer func() { +// if r := recover(); r != nil { +// crashlog.CaptureCmdPanic(r, "gitinfo.loadGitHubData") +// msg = errorToast(r) +// } +// }() +// ... +// } +func CaptureCmdPanic(panicVal any, ctx string) string { + report := NewReport(panicVal, debug.Stack(), ctx) + path, err := Write(report) + if err != nil { + return "" + } + lastCrashPath.Store(&path) + return path +} + // lastCrashPath records the path of the most recent crash report written by // GuardTUI. It lets the program surface the report location after Bubble Tea // has restored the terminal, since anything GuardTUI itself prints would land diff --git a/internal/crashlog/writer.go b/internal/crashlog/writer.go index d88e7a92..1dbf7fa0 100644 --- a/internal/crashlog/writer.go +++ b/internal/crashlog/writer.go @@ -8,6 +8,8 @@ import ( "path/filepath" "slices" "strings" + "sync" + "sync/atomic" "github.com/jongio/grut/internal/config" ) @@ -15,6 +17,19 @@ import ( // maxCrashFiles is the default retention limit for crash report files. const maxCrashFiles = 20 +// writeMu serialises Write (and the pruneOld it calls) so concurrent crash +// reports cannot interleave their os.WriteFile / os.Remove calls. This matters +// because CaptureCmdPanic keeps the process alive and is invoked from Bubble +// Tea command goroutines that run concurrently (tea.Batch), unlike the +// pre-existing callers that wrote a single report immediately before exit. +var writeMu sync.Mutex + +// writeSeq is a process-local monotonic counter that guarantees crash report +// filenames are unique even when several reports are written within the same +// clock tick (the timestamp and the short ID alone are not fine-grained enough +// on coarse-resolution clocks, e.g. Windows). +var writeSeq atomic.Uint64 + // crashDir returns the directory where crash reports are stored. func crashDir() string { return filepath.Join(config.DataDir(), "crashes") @@ -24,6 +39,9 @@ func crashDir() string { // directory. It returns the full path of the created file. Old reports // beyond the retention limit are pruned automatically. func Write(report *CrashReport) (string, error) { + writeMu.Lock() + defer writeMu.Unlock() + dir := crashDir() if err := os.MkdirAll(dir, 0o700); err != nil { return "", fmt.Errorf("creating crash directory: %w", err) @@ -34,7 +52,11 @@ func Write(report *CrashReport) (string, error) { if len(shortID) > 8 { shortID = shortID[:8] } - filename := fmt.Sprintf("crash-%s-%s.json", ts, shortID) + // The atomic sequence disambiguates reports written within the same second + // (the timestamp is second-resolution and shortID is stable for ~100s), so + // concurrent command-goroutine panics do not overwrite each other's report. + seq := writeSeq.Add(1) + filename := fmt.Sprintf("crash-%s-%s-%d.json", ts, shortID, seq) path := filepath.Join(dir, filename) data, err := json.MarshalIndent(report, "", " ") diff --git a/internal/notify/messages.go b/internal/notify/messages.go index 83c4142b..4caf8d18 100644 --- a/internal/notify/messages.go +++ b/internal/notify/messages.go @@ -53,6 +53,7 @@ type ShowModalMsg struct { Placeholder string // used only for input modals Value string // initial value for input modals (pre-fill) CheckboxLabel string // label for the checkbox (ConfirmWithCheckbox only) + SelectedID string // action ID to preselect (ModalActionPicker only; empty = first) Actions []ActionOption // list of actions (ModalActionPicker only) Kind ModalKind // confirm, input, confirmWithCheckbox, or actionPicker } diff --git a/internal/notify/modal.go b/internal/notify/modal.go index 3dc57edd..64727f57 100644 --- a/internal/notify/modal.go +++ b/internal/notify/modal.go @@ -803,6 +803,35 @@ func ShowActionPickerWithMessage(title, message string, actions []ActionOption) } } +// ShowActionPickerWithSelection returns a tea.Cmd that produces a +// ShowModalMsg for an action picker whose cursor starts on the action with +// the given ID. An empty or unknown selectedID starts on the first action. +func ShowActionPickerWithSelection(title, message string, actions []ActionOption, selectedID string) tea.Cmd { + return func() tea.Msg { + return ShowModalMsg{ + Kind: ModalActionPicker, + Title: title, + Message: message, + Actions: actions, + SelectedID: selectedID, + } + } +} + +// actionIndexByID returns the index of the action with the given ID, or 0 +// when the ID is empty or absent. +func actionIndexByID(actions []ActionOption, id string) int { + if id == "" { + return 0 + } + for i, a := range actions { + if a.ID == id { + return i + } + } + return 0 +} + // renderCheckbox renders the checkbox toggle for a ConfirmWithCheckbox modal. func (ms *modalState) renderCheckbox(width int) string { icon := "○" // unchecked diff --git a/internal/notify/notify.go b/internal/notify/notify.go index c82a4b2b..bb257b9d 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -179,7 +179,7 @@ func (m *Manager) Update(msg tea.Msg) tea.Cmd { selected: msg.Kind == ModalConfirm || msg.Kind == ModalConfirmWithCheckbox, checkboxLabel: msg.CheckboxLabel, actions: msg.Actions, - actionCursor: 0, + actionCursor: actionIndexByID(msg.Actions, msg.SelectedID), } m.mu.Unlock() return nil diff --git a/internal/notify/notify_test.go b/internal/notify/notify_test.go index d4e54f35..6a9e36c7 100644 --- a/internal/notify/notify_test.go +++ b/internal/notify/notify_test.go @@ -1238,6 +1238,43 @@ func TestShowActionPickerConstructor(t *testing.T) { assert.Equal(t, "checkout", smm.Actions[0].ID) } +func TestShowActionPickerWithSelection(t *testing.T) { + actions := testActions() + cmd := ShowActionPickerWithSelection("Pick", "sub", actions, actions[2].ID) + require.NotNil(t, cmd) + + msg := cmd() + smm, ok := msg.(ShowModalMsg) + require.True(t, ok) + assert.Equal(t, ModalActionPicker, smm.Kind) + assert.Equal(t, "Pick", smm.Title) + assert.Equal(t, "sub", smm.Message) + assert.Equal(t, actions[2].ID, smm.SelectedID) +} + +// TestShowActionPickerWithSelection_PreselectsCursor verifies the manager +// starts the picker cursor on the selected action, not always the first. +func TestShowActionPickerWithSelection_PreselectsCursor(t *testing.T) { + actions := testActions() + m := NewManager() + m.Update(ShowModalMsg{ + Kind: ModalActionPicker, + Title: "Pick", + Actions: actions, + SelectedID: actions[2].ID, + }) + require.NotNil(t, m.modal) + assert.Equal(t, 2, m.modal.actionCursor, "cursor should start on the selected action") +} + +func TestActionIndexByID(t *testing.T) { + actions := testActions() + assert.Equal(t, 0, actionIndexByID(actions, ""), "empty id → first action") + assert.Equal(t, 0, actionIndexByID(actions, "does-not-exist"), "unknown id → first action") + assert.Equal(t, 2, actionIndexByID(actions, actions[2].ID)) + assert.Equal(t, 0, actionIndexByID(nil, "anything")) +} + // --- Tab key tests --- func TestModalConfirmTab(t *testing.T) { diff --git a/internal/panels/gitinfo/gitinfo.go b/internal/panels/gitinfo/gitinfo.go index 6303b26b..6ac2e86c 100644 --- a/internal/panels/gitinfo/gitinfo.go +++ b/internal/panels/gitinfo/gitinfo.go @@ -156,7 +156,8 @@ const ( opTagPush // awaiting tag push confirmation opTagCheckout // awaiting tag checkout confirmation (detached HEAD) opWorkflowDispatch // awaiting workflow dispatch ref input - opWorkflowDispatchInputs // awaiting workflow dispatch inputs (key=value) + opWorkflowDispatchInputs // awaiting the value for one declared workflow_dispatch input + opWorkflowDispatchRaw // awaiting free-form key=value inputs (workflow YAML unreadable) opPRMergeStrategy // awaiting merge strategy selection opPRMergeConfirm // awaiting merge confirmation opPRDeleteBranchAfterMerge // awaiting post-merge branch deletion confirmation @@ -363,6 +364,18 @@ type prCreateDraft struct { title string } +// workflowDispatchDraft holds the state of the multi-step workflow dispatch +// flow. After the ref is chosen, grut prompts once per declared +// workflow_dispatch input, then fires the dispatch with the collected values. +type workflowDispatchDraft struct { + values map[string]any // collected input values, keyed by input name + workflowName string + ref string + inputs []ghclient.WorkflowInput + workflowID int64 + idx int // index of the input currently being prompted +} + // --------------------------------------------------------------------------- // Panel // --------------------------------------------------------------------------- @@ -382,19 +395,20 @@ type Panel struct { cfg config.GitConfig colors panelColors theme *theme.Theme - tabCursor [tabCount]int // cursor per tab - tabOffset [tabCount]int // viewport offset per tab - mode PanelMode // which tab subset to display - activeTab tabID // currently active tab - filterQuery string // incremental filter query for the active git list tab - filteredIdx []int // indices into tabItems[activeTab]; nil = no filter - compareBase string // currently pinned diff compare base - remoteCount int // actual number of remotes (distinct from tabItems len which includes sub-rows) - pending pendingOp // operation awaiting modal result - prDraft prCreateDraft // in-progress fields for the multi-step create-PR flow - pendingPRCheckout ghPRItem // PR captured for the checkout action picker - actionsWatchFrame int // current animation frame index into watchFrames - lastWidth int // last rendered width, used for click zone calculation + tabCursor [tabCount]int // cursor per tab + tabOffset [tabCount]int // viewport offset per tab + mode PanelMode // which tab subset to display + activeTab tabID // currently active tab + filterQuery string // incremental filter query for the active git list tab + filteredIdx []int // indices into tabItems[activeTab]; nil = no filter + compareBase string // currently pinned diff compare base + remoteCount int // actual number of remotes (distinct from tabItems len which includes sub-rows) + pending pendingOp // operation awaiting modal result + prDraft prCreateDraft // in-progress fields for the multi-step create-PR flow + wfDispatch workflowDispatchDraft // in-progress fields for the multi-step workflow dispatch flow + pendingPRCheckout ghPRItem // PR captured for the checkout action picker + actionsWatchFrame int // current animation frame index into watchFrames + lastWidth int // last rendered width, used for click zone calculation // CI watch animation state — animated indicator when in-progress runs exist. actionsWatching bool // true when in-progress/queued runs exist AND polling is active // Live GitHub Actions log follow state. The followed job is the first failed @@ -991,6 +1005,8 @@ func (p *Panel) Update(msg tea.Msg) (panels.Panel, tea.Cmd) { return p.handleWorkflowDispatchResult(msg) case workflowInputsFetchedMsg: return p.handleWorkflowInputsFetched(msg) + case ghCmdPanicMsg: + return p.handleGHCmdPanic(msg) case prMergeResultMsg: return p.handlePRMergeResult(msg) @@ -2663,6 +2679,7 @@ func (p *Panel) handleModalResult(msg notify.ModalResultMsg) (panels.Panel, tea. if !msg.Accept { // Abort any in-progress create-PR flow so a later run starts clean. p.prDraft = prCreateDraft{} + p.wfDispatch = workflowDispatchDraft{} return p, nil } a := modalArgs{ @@ -2722,6 +2739,8 @@ func (p *Panel) handleModalResult(msg notify.ModalResultMsg) (panels.Panel, tea. return p.handleWorkflowDispatch(a) case opWorkflowDispatchInputs: return p.handleWorkflowDispatchInputs(a) + case opWorkflowDispatchRaw: + return p.handleWorkflowDispatchRaw(a) case opPRMergeStrategy: return p.handlePRMergeStrategy(a) case opPRMergeConfirm: diff --git a/internal/panels/gitinfo/gitinfo_dispatch_panic_test.go b/internal/panels/gitinfo/gitinfo_dispatch_panic_test.go index c1375519..84443e88 100644 --- a/internal/panels/gitinfo/gitinfo_dispatch_panic_test.go +++ b/internal/panels/gitinfo/gitinfo_dispatch_panic_test.go @@ -4,6 +4,7 @@ import ( "testing" tea "charm.land/bubbletea/v2" + "github.com/adrg/xdg" ghclient "github.com/jongio/grut/internal/github" "github.com/jongio/grut/internal/notify" "github.com/stretchr/testify/assert" @@ -25,10 +26,10 @@ func findMsg[T tea.Msg](t *testing.T, msgs []tea.Msg) T { return zero } -// TestHandleWorkflowInputsFetched_UsesMultilineModal verifies the inputs step -// uses a multi-line composer (issue #361 AC #4): the content is one key=value -// per line, which a single-line input can neither hold nor let the user extend. -func TestHandleWorkflowInputsFetched_UsesMultilineModal(t *testing.T) { +// TestHandleWorkflowInputsFetched_KnownInputsUsePicker verifies that a declared +// choice input is presented as an action picker preselected on its default, so +// the user can pick a valid value (issue: parameter picker). +func TestHandleWorkflowInputsFetched_KnownInputsUsePicker(t *testing.T) { t.Parallel() p := newTestPanel(t, defaultMock()) @@ -36,28 +37,113 @@ func TestHandleWorkflowInputsFetched_UsesMultilineModal(t *testing.T) { workflowID: 7, workflowName: "Deploy", ref: "main", + inputsKnown: true, inputs: []ghclient.WorkflowInput{ - {Name: "env", Default: "prod"}, - {Name: "version", Default: "1.0"}, + {Name: "env", Type: "choice", Options: []string{"dev", "staging", "prod"}, Default: "staging"}, }, }) require.NotNil(t, cmd) modal, ok := cmd().(notify.ShowModalMsg) require.True(t, ok) - assert.Equal(t, notify.ModalMultilineInput, modal.Kind, - "multi-line key=value content needs a multi-line composer") - assert.Equal(t, "env=prod\nversion=1.0", modal.Value) + assert.Equal(t, notify.ModalActionPicker, modal.Kind, "choice input should use a picker") + assert.Equal(t, "staging", modal.SelectedID, "picker should preselect the declared default") + require.Len(t, modal.Actions, 3) + assert.Equal(t, "dev", modal.Actions[0].ID) + assert.Equal(t, opWorkflowDispatchInputs, p.pending) } -// TestHandleWorkflowDispatchInputs_NilClientNoPanic verifies a nil GitHub -// client surfaces as a failure toast rather than a nil dereference that crashes -// the TUI (issue #361 AC #3). -func TestHandleWorkflowDispatchInputs_NilClientNoPanic(t *testing.T) { +// TestHandleWorkflowInputsFetched_FreeFormInputUsesTextField verifies a string +// input is a text field pre-filled with its default, so the user can accept it +// or type a custom value. +func TestHandleWorkflowInputsFetched_FreeFormInputUsesTextField(t *testing.T) { + t.Parallel() + p := newTestPanel(t, defaultMock()) + + _, cmd := p.handleWorkflowInputsFetched(workflowInputsFetchedMsg{ + workflowID: 7, + workflowName: "Deploy", + ref: "main", + inputsKnown: true, + inputs: []ghclient.WorkflowInput{{Name: "tag", Default: "v1.0"}}, + }) + require.NotNil(t, cmd) + + modal, ok := cmd().(notify.ShowModalMsg) + require.True(t, ok) + assert.Equal(t, notify.ModalInput, modal.Kind, "free-form input should use a text field") + assert.Equal(t, "v1.0", modal.Value, "text field should pre-fill the declared default") +} + +// TestHandleWorkflowInputsFetched_UnknownInputsUseRawComposer verifies the +// free-form key=value composer is offered when the workflow inputs could not be +// read (fetch/parse failed). +func TestHandleWorkflowInputsFetched_UnknownInputsUseRawComposer(t *testing.T) { + t.Parallel() + p := newTestPanel(t, defaultMock()) + + _, cmd := p.handleWorkflowInputsFetched(workflowInputsFetchedMsg{ + workflowID: 7, + workflowName: "Deploy", + ref: "main", + inputsKnown: false, + }) + require.NotNil(t, cmd) + + modal, ok := cmd().(notify.ShowModalMsg) + require.True(t, ok) + assert.Equal(t, notify.ModalMultilineInput, modal.Kind, "unreadable inputs fall back to the composer") + assert.Equal(t, opWorkflowDispatchRaw, p.pending) +} + +// TestHandleWorkflowInputsFetched_NoInputsDispatchesDirectly verifies a +// workflow with no declared inputs dispatches immediately without prompting. +func TestHandleWorkflowInputsFetched_NoInputsDispatchesDirectly(t *testing.T) { + t.Parallel() + ghMock := &mockGHClientFull{} + p := newGHPanelWithClient(t, defaultMock(), ghMock) + + _, cmd := p.handleWorkflowInputsFetched(workflowInputsFetchedMsg{ + workflowID: 42, + workflowName: "Deploy", + ref: "main", + inputsKnown: true, + inputs: nil, + }) + require.NotNil(t, cmd) + result := findMsg[workflowDispatchResultMsg](t, collectCmdMsgs(cmd)) + require.NoError(t, result.err) + assert.Equal(t, 1, ghMock.dispatchCalls, "no-input workflow should dispatch directly") + assert.Nil(t, ghMock.dispatchInputs, "no inputs collected") +} + +// TestFireWorkflowDispatch_NilClientNoPanic verifies the per-input dispatch +// path surfaces a nil client as a failure toast rather than crashing the TUI. +func TestFireWorkflowDispatch_NilClientNoPanic(t *testing.T) { + t.Parallel() + p := newTestPanel(t, defaultMock()) + p.gh.client = nil + p.wfDispatch = workflowDispatchDraft{workflowID: 9, workflowName: "Deploy", ref: "main"} + + var result workflowDispatchResultMsg + assert.NotPanics(t, func() { + _, cmd := p.fireWorkflowDispatch() + require.NotNil(t, cmd) + var ok bool + result, ok = cmd().(workflowDispatchResultMsg) + require.True(t, ok) + }) + require.Error(t, result.err, "nil client must yield an error result") + assert.Empty(t, p.wfDispatch.workflowName, "draft should be reset after firing") +} + +// TestHandleWorkflowDispatchRaw_NilClientNoPanic verifies the free-form +// fallback path guards a nil client instead of dereferencing it (issue #361). +func TestHandleWorkflowDispatchRaw_NilClientNoPanic(t *testing.T) { t.Parallel() p := newTestPanel(t, defaultMock()) p.gh.client = nil - p.pending = opWorkflowDispatchInputs + p.pending = opWorkflowDispatchRaw p.pendingName = "9:Deploy:main" var result workflowDispatchResultMsg @@ -77,9 +163,9 @@ func TestHandleWorkflowDispatchInputs_NilClientNoPanic(t *testing.T) { } // TestWorkflowDispatch_FullFlow_NoPanicSuccessToast drives the entire dispatch -// flow (D → ref → inputs → submit → result) through the panel's Update and -// asserts it dispatches exactly once and ends with a success toast, without -// panicking (issue #361 AC #2, #5). +// flow (D → ref → per-input prompt → submit → result) through the panel's +// Update and asserts it dispatches exactly once with the collected input and +// ends with a success toast, without panicking (issue #361 AC #2, #5). func TestWorkflowDispatch_FullFlow_NoPanicSuccessToast(t *testing.T) { t.Parallel() ghMock := &mockGHClientFull{ @@ -103,15 +189,17 @@ func TestWorkflowDispatch_FullFlow_NoPanicSuccessToast(t *testing.T) { _, cmd = p.Update(notify.ModalResultMsg{Accept: true, Value: "main"}) fetched := findMsg[workflowInputsFetchedMsg](t, collectCmdMsgs(cmd)) require.Equal(t, "main", fetched.ref) + require.True(t, fetched.inputsKnown) - // Step 3: inputs modal appears as a multi-line composer. + // Step 3: the single declared input appears as a text field. _, cmd = p.Update(fetched) modal := findMsg[notify.ShowModalMsg](t, collectCmdMsgs(cmd)) - require.Equal(t, notify.ModalMultilineInput, modal.Kind) + require.Equal(t, notify.ModalInput, modal.Kind) + require.Equal(t, "prod", modal.Value) require.Equal(t, opWorkflowDispatchInputs, p.pending) - // Step 4: submit inputs → dispatch fires. - _, cmd = p.Update(notify.ModalResultMsg{Accept: true, Value: "env=prod"}) + // Step 4: submit the input value → dispatch fires. + _, cmd = p.Update(notify.ModalResultMsg{Accept: true, Value: "prod"}) result := findMsg[workflowDispatchResultMsg](t, collectCmdMsgs(cmd)) require.NoError(t, result.err) @@ -155,3 +243,176 @@ func TestWorkflowDispatch_RenderAfterReload_NoPanic(t *testing.T) { } }) } + +// TestHandleWorkflowInputsFetched_BooleanInputUsesPicker verifies a boolean +// input is offered as a true/false picker preselected on the declared default. +func TestHandleWorkflowInputsFetched_BooleanInputUsesPicker(t *testing.T) { + t.Parallel() + p := newTestPanel(t, defaultMock()) + + _, cmd := p.handleWorkflowInputsFetched(workflowInputsFetchedMsg{ + workflowID: 7, + workflowName: "Deploy", + ref: "main", + inputsKnown: true, + inputs: []ghclient.WorkflowInput{{Name: "dry_run", Type: "boolean", Default: "true"}}, + }) + modal := findMsg[notify.ShowModalMsg](t, collectCmdMsgs(cmd)) + require.Equal(t, notify.ModalActionPicker, modal.Kind) + assert.Equal(t, "true", modal.SelectedID) + require.Len(t, modal.Actions, 2) + assert.Equal(t, "true", modal.Actions[0].ID) + assert.Equal(t, "false", modal.Actions[1].ID) +} + +// TestWorkflowDispatch_MultiInput_CollectsChoiceAndText drives a two-input +// dispatch (a choice then a free-form string), asserting each value is +// collected and passed to the dispatch. +func TestWorkflowDispatch_MultiInput_CollectsChoiceAndText(t *testing.T) { + t.Parallel() + ghMock := &mockGHClientFull{} + p := newGHPanelWithClient(t, defaultMock(), ghMock) + + _, cmd := p.handleWorkflowInputsFetched(workflowInputsFetchedMsg{ + workflowID: 42, + workflowName: "Deploy", + ref: "main", + inputsKnown: true, + inputs: []ghclient.WorkflowInput{ + {Name: "env", Type: "choice", Options: []string{"dev", "prod"}, Default: "prod"}, + {Name: "tag", Default: "v1.0"}, + }, + }) + // First prompt: choice picker preselected on "prod". + modal := findMsg[notify.ShowModalMsg](t, collectCmdMsgs(cmd)) + require.Equal(t, notify.ModalActionPicker, modal.Kind) + require.Equal(t, "prod", modal.SelectedID) + + // User picks "dev" → second prompt is a text field pre-filled with default. + _, cmd = p.Update(notify.ModalResultMsg{Accept: true, Value: "dev"}) + modal = findMsg[notify.ShowModalMsg](t, collectCmdMsgs(cmd)) + require.Equal(t, notify.ModalInput, modal.Kind) + require.Equal(t, "v1.0", modal.Value) + + // User enters a custom tag → dispatch fires with both values. + _, cmd = p.Update(notify.ModalResultMsg{Accept: true, Value: "custom"}) + result := findMsg[workflowDispatchResultMsg](t, collectCmdMsgs(cmd)) + require.NoError(t, result.err) + + require.Equal(t, 1, ghMock.dispatchCalls) + require.NotNil(t, ghMock.dispatchInputs) + assert.Equal(t, "dev", ghMock.dispatchInputs["env"]) + assert.Equal(t, "custom", ghMock.dispatchInputs["tag"]) +} + +// TestWorkflowDispatch_EmptyValueKeepsDefault verifies an input left blank is +// omitted, so GitHub applies the workflow's declared default. +func TestWorkflowDispatch_EmptyValueKeepsDefault(t *testing.T) { + t.Parallel() + ghMock := &mockGHClientFull{} + p := newGHPanelWithClient(t, defaultMock(), ghMock) + + _, cmd := p.handleWorkflowInputsFetched(workflowInputsFetchedMsg{ + workflowID: 42, + workflowName: "Deploy", + ref: "main", + inputsKnown: true, + inputs: []ghclient.WorkflowInput{{Name: "tag", Default: "v1.0"}}, + }) + _ = findMsg[notify.ShowModalMsg](t, collectCmdMsgs(cmd)) + + // Submit an empty value → omitted from inputs. + _, cmd = p.Update(notify.ModalResultMsg{Accept: true, Value: ""}) + result := findMsg[workflowDispatchResultMsg](t, collectCmdMsgs(cmd)) + require.NoError(t, result.err) + assert.Equal(t, 1, ghMock.dispatchCalls) + assert.Nil(t, ghMock.dispatchInputs, "blank input should be omitted, keeping the default") +} + +// TestGuardedGitHubCmd_RecoversPanicToPanicMsg verifies a panic inside a GitHub +// command goroutine is captured to a crash report and converted to a +// ghCmdPanicMsg (handled on the main goroutine) instead of killing the TUI +// (issue #361 durable fix). +func TestGuardedGitHubCmd_RecoversPanicToPanicMsg(t *testing.T) { + // Redirect crash reports to a temp dir so the real data dir stays clean. + orig := xdg.DataHome + xdg.DataHome = t.TempDir() + t.Cleanup(func() { xdg.DataHome = orig }) + + cmd := guardedGitHubCmd("test.panic", func() tea.Msg { + panic("kaboom") + }) + var msg tea.Msg + assert.NotPanics(t, func() { msg = cmd() }) + _, ok := msg.(ghCmdPanicMsg) + require.True(t, ok, "panic should be converted to ghCmdPanicMsg, got %T", msg) +} + +// TestHandleGHCmdPanic_ClearsLoadingAndToasts verifies the panic message clears +// stranded loading flags and surfaces an error toast. +func TestHandleGHCmdPanic_ClearsLoadingAndToasts(t *testing.T) { + t.Parallel() + p := newTestPanel(t, defaultMock()) + for i := range p.tabPaging { + p.tabPaging[i].loading = true + } + p.gh.notifLoading = true + + _, cmd := p.handleGHCmdPanic(ghCmdPanicMsg{label: "gitinfo.loadIssuesPage"}) + for i := range p.tabPaging { + assert.False(t, p.tabPaging[i].loading, "loading flag %d should be cleared", i) + } + assert.False(t, p.gh.notifLoading, "notifLoading should be cleared") + toast := findMsg[notify.ShowToastMsg](t, collectCmdMsgs(cmd)) + assert.Equal(t, notify.Error, toast.Level) +} + +// TestGuardedGitHubCmd_PassesThroughNormalMsg verifies the wrapper is +// transparent when the command does not panic. +func TestGuardedGitHubCmd_PassesThroughNormalMsg(t *testing.T) { + t.Parallel() + cmd := guardedGitHubCmd("test.ok", func() tea.Msg { + return workflowDispatchResultMsg{workflowName: "X"} + }) + result, ok := cmd().(workflowDispatchResultMsg) + require.True(t, ok) + assert.Equal(t, "X", result.workflowName) +} + +// TestGuardedCommand_NilClientYieldsToast verifies a GitHub action command +// (representative: cancelWorkflowRunCmd) surfaces a nil client as an error +// toast via ghClientUnavailableCmd rather than dereferencing it. +func TestGuardedCommand_NilClientYieldsToast(t *testing.T) { + t.Parallel() + p := newTestPanel(t, defaultMock()) + p.gh.client = nil // no GitHub client configured + + cmd := p.cancelWorkflowRunCmd(1) + require.NotNil(t, cmd) + toast, ok := cmd().(notify.ShowToastMsg) + require.True(t, ok, "nil client should yield a toast, got %T", cmd()) + assert.Equal(t, notify.Error, toast.Level) + assert.Equal(t, errGitHubClientUnavailable.Error(), toast.Message) +} + +// TestWorkflowInputPrompt covers the prompt-label construction branches: +// description vs. name fallback, name prefixing, and the required marker. +func TestWorkflowInputPrompt(t *testing.T) { + t.Parallel() + tests := []struct { + name string + in ghclient.WorkflowInput + want string + }{ + {"name only", ghclient.WorkflowInput{Name: "env"}, "env"}, + {"description prefixed with name", ghclient.WorkflowInput{Name: "env", Description: "Target environment"}, "env: Target environment"}, + {"required flag", ghclient.WorkflowInput{Name: "env", Required: true}, "env (required)"}, + {"description + required", ghclient.WorkflowInput{Name: "env", Description: "Target", Required: true}, "env: Target (required)"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, workflowInputPrompt(tc.in)) + }) + } +} diff --git a/internal/panels/gitinfo/gitinfo_github.go b/internal/panels/gitinfo/gitinfo_github.go index 2c4a1d65..1f2a8fe6 100644 --- a/internal/panels/gitinfo/gitinfo_github.go +++ b/internal/panels/gitinfo/gitinfo_github.go @@ -13,6 +13,7 @@ import ( "charm.land/lipgloss/v2" gh "github.com/google/go-github/v89/github" + "github.com/jongio/grut/internal/crashlog" "github.com/jongio/grut/internal/git" ghclient "github.com/jongio/grut/internal/github" "github.com/jongio/grut/internal/notify" @@ -247,6 +248,21 @@ type workflowInputsFetchedMsg struct { ref string inputs []ghclient.WorkflowInput workflowID int64 + inputsKnown bool // true when the workflow inputs were read successfully +} + +// workflow_dispatch input type identifiers (GitHub Actions). +const ( + inputTypeChoice = "choice" + inputTypeBoolean = "boolean" + boolTrue = "true" + boolFalse = "false" +) + +// boolActions is the picker option set for a boolean workflow_dispatch input. +var boolActions = []notify.ActionOption{ + {ID: boolTrue, Label: boolTrue}, + {ID: boolFalse, Label: boolFalse}, } // githubPollTickMsg triggers periodic GitHub data refresh. @@ -634,7 +650,7 @@ func (p *Panel) loadGitHubData() tea.Cmd { } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx - return func() tea.Msg { + return guardedGitHubCmd("gitinfo.loadGitHubData", func() tea.Msg { var result ghDataLoadedMsg // Fetch repo metadata (visibility). repoInfo, err := client.RepoInfo(ctx, owner, repo) @@ -790,7 +806,7 @@ func (p *Panel) loadGitHubData() tea.Cmd { } } return result - } + }) } func (p *Panel) handleGHDataLoaded(msg ghDataLoadedMsg) (panels.Panel, tea.Cmd) { @@ -911,7 +927,7 @@ func (p *Panel) loadGitHubMeta() tea.Cmd { } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx - return func() tea.Msg { + return guardedGitHubCmd("gitinfo.loadGitHubMeta", func() tea.Msg { var result ghMetaLoadedMsg repoInfo, err := client.RepoInfo(ctx, owner, repo) if err != nil { @@ -927,7 +943,7 @@ func (p *Panel) loadGitHubMeta() tea.Cmd { result.user = *user.Login } return result - } + }) } // loadIssuesPage fetches a single page of issues. @@ -940,7 +956,7 @@ func (p *Panel) loadIssuesPage(page int, replace bool) tea.Cmd { ctx := p.ctx pageSize := p.gh.pageSize state := p.gh.issueState.apiValue() - return func() tea.Msg { + return guardedGitHubCmd("gitinfo.loadIssuesPage", func() tea.Msg { issues, pr, err := client.ListIssuesPage(ctx, owner, repo, &gh.IssueListByRepoOptions{ State: state, ListOptions: gh.ListOptions{Page: page, PerPage: pageSize}, @@ -980,7 +996,7 @@ func (p *Panel) loadIssuesPage(page int, replace bool) tea.Cmd { }) } return ghIssuesPageMsg{issues: items, nextPage: pr.NextPage, replace: replace} - } + }) } // loadPRsPage fetches a single page of pull requests. @@ -993,7 +1009,7 @@ func (p *Panel) loadPRsPage(page int, replace bool) tea.Cmd { ctx := p.ctx pageSize := p.gh.pageSize state := p.gh.prState.apiValue() - return func() tea.Msg { + return guardedGitHubCmd("gitinfo.loadPRsPage", func() tea.Msg { prs, pr, err := client.ListPRsPage(ctx, owner, repo, &gh.PullRequestListOptions{ State: state, ListOptions: gh.ListOptions{Page: page, PerPage: pageSize}, @@ -1007,7 +1023,7 @@ func (p *Panel) loadPRsPage(page int, replace bool) tea.Cmd { items = append(items, ghPRItemFromPullRequest(ghPR, owner)) } return ghPRsPageMsg{prs: items, nextPage: pr.NextPage, replace: replace} - } + }) } // loadActionsPage fetches a single page of workflow runs. @@ -1019,7 +1035,7 @@ func (p *Panel) loadActionsPage(page int, replace bool) tea.Cmd { owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx pageSize := p.gh.pageSize - return func() tea.Msg { + return guardedGitHubCmd("gitinfo.loadActionsPage", func() tea.Msg { runs, pr, err := client.ListWorkflowRunsPage(ctx, owner, repo, &gh.ListWorkflowRunsOptions{ ListOptions: gh.ListOptions{Page: page, PerPage: pageSize}, }) @@ -1041,7 +1057,7 @@ func (p *Panel) loadActionsPage(page int, replace bool) tea.Cmd { }) } return ghActionsPageMsg{actions: items, nextPage: pr.NextPage, replace: replace} - } + }) } // loadWorkflowsPage fetches a single page of workflow definitions. @@ -1053,7 +1069,7 @@ func (p *Panel) loadWorkflowsPage(page int, replace bool) tea.Cmd { owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx pageSize := p.gh.pageSize - return func() tea.Msg { + return guardedGitHubCmd("gitinfo.loadWorkflowsPage", func() tea.Msg { workflows, pr, err := client.ListWorkflowsPage(ctx, owner, repo, &gh.ListOptions{Page: page, PerPage: pageSize}) if err != nil { slog.Warn("github: fetch workflows page failed", "owner", owner, "repo", repo, "page", page, "err", err) @@ -1070,7 +1086,7 @@ func (p *Panel) loadWorkflowsPage(page int, replace bool) tea.Cmd { }) } return ghWorkflowsPageMsg{workflows: items, nextPage: pr.NextPage, replace: replace} - } + }) } // loadReleasesPage fetches a single page of releases. @@ -1082,7 +1098,7 @@ func (p *Panel) loadReleasesPage(page int, replace bool) tea.Cmd { owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx pageSize := p.gh.pageSize - return func() tea.Msg { + return guardedGitHubCmd("gitinfo.loadReleasesPage", func() tea.Msg { releases, pr, err := client.ListReleasesPage(ctx, owner, repo, &gh.ListOptions{Page: page, PerPage: pageSize}) if err != nil { slog.Warn("github: fetch releases page failed", "owner", owner, "repo", repo, "page", page, "err", err) @@ -1116,7 +1132,7 @@ func (p *Panel) loadReleasesPage(page int, replace bool) tea.Cmd { }) } return ghReleasesPageMsg{releases: items, nextPage: pr.NextPage, replace: replace} - } + }) } // --------------------------------------------------------------------------- @@ -2291,6 +2307,9 @@ func (p *Panel) handleActionRerunResult(msg actionRerunResultMsg) (panels.Panel, // cancelWorkflowRunCmd returns a Cmd that cancels a workflow run. func (p *Panel) cancelWorkflowRunCmd(runID int64) tea.Cmd { client := p.gh.client + if client == nil { + return ghClientUnavailableCmd() + } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx return func() tea.Msg { @@ -2375,30 +2394,173 @@ func (p *Panel) handleWorkflowDispatchResult(msg workflowDispatchResultMsg) (pan ) } -// handleWorkflowInputsFetched receives the parsed workflow_dispatch inputs -// from the YAML file and shows a pre-populated input dialog. +// handleWorkflowInputsFetched receives the parsed workflow_dispatch inputs and +// begins collecting values. When the inputs are known it prompts once per +// input — a picker for choice/boolean inputs (so the user selects a valid +// value), a text field for free-form inputs (so the user can enter a custom +// value) — then dispatches. A workflow with no inputs dispatches immediately. +// When the inputs could not be read (fetch/parse failed) it falls back to a +// free-form key=value composer so the user can still pass inputs. func (p *Panel) handleWorkflowInputsFetched(msg workflowInputsFetchedMsg) (panels.Panel, tea.Cmd) { - // Wire up pending state so the next modal result hits opWorkflowDispatchInputs. + if !msg.inputsKnown { + // Inputs unreadable — offer the free-form composer as a fallback. + p.pending = opWorkflowDispatchRaw + p.pendingName = fmt.Sprintf("%d:%s:%s", msg.workflowID, msg.workflowName, msg.ref) + title := fmt.Sprintf("Inputs for %s", msg.workflowName) + return p, notify.ShowMultilineInput(title, "key=value per line (empty to skip) • ctrl+d submit") + } + p.wfDispatch = workflowDispatchDraft{ + workflowID: msg.workflowID, + workflowName: msg.workflowName, + ref: msg.ref, + inputs: msg.inputs, + values: make(map[string]any), + } + return p.promptNextWorkflowInput() +} + +// promptNextWorkflowInput advances the workflow-dispatch flow: it prompts for +// the next declared input, or fires the dispatch once every input has been +// collected. Choice and boolean inputs use an action picker preselected on the +// declared default; free-form inputs use a text field pre-filled with the +// default so the user can accept it or type a custom value. +func (p *Panel) promptNextWorkflowInput() (panels.Panel, tea.Cmd) { + d := &p.wfDispatch + if d.idx >= len(d.inputs) { + return p.fireWorkflowDispatch() + } + input := d.inputs[d.idx] p.pending = opWorkflowDispatchInputs - p.pendingName = fmt.Sprintf("%d:%s:%s", msg.workflowID, msg.workflowName, msg.ref) - // Build pre-populated value with actual field names and defaults. - var prePopulated string - if len(msg.inputs) > 0 { - lines := make([]string, 0, len(msg.inputs)) - for _, input := range msg.inputs { - lines = append(lines, input.Name+"="+input.Default) + title := fmt.Sprintf("%s — input %d of %d", d.workflowName, d.idx+1, len(d.inputs)) + prompt := workflowInputPrompt(input) + switch { + case input.Type == inputTypeChoice && len(input.Options) > 0: + return p, notify.ShowActionPickerWithSelection(title, prompt, optionsToActions(input.Options), input.Default) + case input.Type == inputTypeBoolean: + selected := boolFalse + if input.Default == boolTrue { + selected = boolTrue + } + return p, notify.ShowActionPickerWithSelection(title, prompt, boolActions, selected) + default: + return p, notify.ShowInputWithValue(title, prompt, input.Default) + } +} + +// fireWorkflowDispatch dispatches the workflow with the values collected across +// the per-input prompts, then resets the draft. +func (p *Panel) fireWorkflowDispatch() (panels.Panel, tea.Cmd) { + d := p.wfDispatch + p.wfDispatch = workflowDispatchDraft{} + var inputs map[string]any + if len(d.values) > 0 { + inputs = d.values + } + return p, p.dispatchWorkflowCmd(d.workflowID, d.workflowName, d.ref, inputs) +} + +// dispatchWorkflowCmd builds the command that dispatches a workflow run. It +// returns nil when the target is incomplete (missing id/ref), and surfaces a +// nil client as a failure toast rather than a nil dereference that would crash +// the TUI. Shared by the per-input flow and the free-form fallback. +func (p *Panel) dispatchWorkflowCmd(workflowID int64, workflowName, ref string, inputs map[string]any) tea.Cmd { + if workflowID == 0 || ref == "" { + return nil + } + owner, repo := p.gh.owner, p.gh.repo + ghClient := p.gh.client + ctx := p.ctx + if ghClient == nil { + return func() tea.Msg { + return workflowDispatchResultMsg{workflowName: workflowName, err: errGitHubClientUnavailable} + } + } + return guardedGitHubCmd("gitinfo.dispatchWorkflow", func() tea.Msg { + err := ghClient.DispatchWorkflow(ctx, owner, repo, workflowID, ref, inputs) + return workflowDispatchResultMsg{workflowName: workflowName, err: err} + }) +} + +// workflowInputPrompt builds the prompt line shown while collecting a single +// workflow_dispatch input, favouring the description and flagging required +// inputs. +func workflowInputPrompt(in ghclient.WorkflowInput) string { + label := in.Description + if label == "" { + label = in.Name + } + if in.Name != "" && label != in.Name { + label = in.Name + ": " + label + } + if in.Required { + label += " (required)" + } + return label +} + +// optionsToActions converts declared choice options into picker actions. +func optionsToActions(opts []string) []notify.ActionOption { + actions := make([]notify.ActionOption, 0, len(opts)) + for _, o := range opts { + actions = append(actions, notify.ActionOption{ID: o, Label: o}) + } + return actions +} + +// ghCmdPanicMsg is emitted by guardedGitHubCmd when a GitHub command goroutine +// panics (after the panic is captured to a crash report). The panel handles it +// by clearing any in-flight loading state — which the panicking command's +// normal typed result would otherwise have cleared — so the panel is not left +// stuck showing "Loading…", and by surfacing an error toast. +type ghCmdPanicMsg struct{ label string } + +// guardedGitHubCmd wraps a GitHub command closure so a panic in the Bubble Tea +// command goroutine is captured to a crash report and surfaced as an error +// toast, instead of killing the whole TUI. crashlog.GuardTUI only covers the +// model's Init/Update/View on the main goroutine; command goroutines (run by +// Bubble Tea's execBatchMsg) are otherwise unprotected, so an unexpected panic +// on the GitHub data path — e.g. a nil field in live API data — would abort the +// program with a swallowed stack (issue #361). +func guardedGitHubCmd(label string, fn func() tea.Msg) tea.Cmd { + return func() (msg tea.Msg) { + defer func() { + if r := recover(); r != nil { + crashlog.CaptureCmdPanic(r, label) + msg = ghCmdPanicMsg{label: label} + } + }() + return fn() + } +} + +// handleGHCmdPanic clears any in-flight loading state stranded by a panicking +// GitHub command (its normal typed result never arrived to clear it) and shows +// an error toast. The crash report was already written by guardedGitHubCmd. +func (p *Panel) handleGHCmdPanic(_ ghCmdPanicMsg) (panels.Panel, tea.Cmd) { + for i := range p.tabPaging { + p.tabPaging[i].loading = false + } + p.gh.notifLoading = false + return p, func() tea.Msg { + return notify.ShowToastMsg{ + Message: "GitHub request failed unexpectedly — details saved to the crash log", + Level: notify.Error, } - prePopulated = strings.Join(lines, "\n") } - title := fmt.Sprintf("Inputs for %s", msg.workflowName) - placeholder := "key=value per line (empty to skip)" - if len(msg.inputs) > 0 { - placeholder = "edit values below (empty to skip)" +} + +// ghClientUnavailableCmd surfaces a failure toast when a GitHub action is +// attempted with no client configured, instead of dereferencing a nil client +// in a command goroutine and crashing the TUI. Its loader siblings return nil +// (silent no-op); an explicit action instead tells the user why nothing +// happened. +func ghClientUnavailableCmd() tea.Cmd { + return func() tea.Msg { + return notify.ShowToastMsg{ + Message: errGitHubClientUnavailable.Error(), + Level: notify.Error, + } } - // The content is one key=value per line, so use a multi-line composer - // (Enter=newline, Ctrl+D=submit). A single-line input can neither hold - // nor let the user add newlines, so multi-input workflows were unusable. - return p, notify.ShowMultilineInputWithValue(title, placeholder, prePopulated) } // --------------------------------------------------------------------------- @@ -2562,6 +2724,9 @@ func (p *Panel) prCheckoutWorktreeCmd(pr ghPRItem, branch, path string) tea.Cmd // mergePRCmd returns a tea.Cmd that executes the merge asynchronously. func (p *Panel) mergePRCmd(number int, strategy string, headBranch string) tea.Cmd { client := p.gh.client + if client == nil { + return ghClientUnavailableCmd() + } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx return func() tea.Msg { @@ -2684,6 +2849,9 @@ func (p *Panel) doCommentOnItem() (panels.Panel, tea.Cmd) { // commentCmd returns a tea.Cmd that posts the comment asynchronously. func (p *Panel) commentCmd(number int, body, kind string) tea.Cmd { client := p.gh.client + if client == nil { + return ghClientUnavailableCmd() + } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx return func() tea.Msg { @@ -2891,6 +3059,9 @@ func parseReviewerLogins(input string) []string { // requestReviewersCmd returns a tea.Cmd that requests reviewers asynchronously. func (p *Panel) requestReviewersCmd(number int, reviewers []string) tea.Cmd { client := p.gh.client + if client == nil { + return ghClientUnavailableCmd() + } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx return func() tea.Msg { @@ -2989,6 +3160,9 @@ func parseIssueStateName(name string) (number int, state string, ok bool) { // closeReopenIssueCmd returns a tea.Cmd that closes or reopens an issue. func (p *Panel) closeReopenIssueCmd(number int, targetState string) tea.Cmd { client := p.gh.client + if client == nil { + return ghClientUnavailableCmd() + } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx return func() tea.Msg { @@ -3119,6 +3293,9 @@ func (p *Panel) branchIsPushed(name string) bool { // createPRCmd returns a tea.Cmd that opens the pull request asynchronously. func (p *Panel) createPRCmd(head, base, title, body string) tea.Cmd { client := p.gh.client + if client == nil { + return ghClientUnavailableCmd() + } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx return func() tea.Msg { @@ -3621,6 +3798,9 @@ func parseAssignSelfName(name string) (kind string, number int, ok bool) { // current user login is not cached, it is fetched first. func (p *Panel) assignSelfCmd(kind string, number int) tea.Cmd { client := p.gh.client + if client == nil { + return ghClientUnavailableCmd() + } owner, repo := p.gh.owner, p.gh.repo ctx := p.ctx login := p.gh.user diff --git a/internal/panels/gitinfo/gitinfo_helpers_test.go b/internal/panels/gitinfo/gitinfo_helpers_test.go index 6a2c90e3..e9c6f492 100644 --- a/internal/panels/gitinfo/gitinfo_helpers_test.go +++ b/internal/panels/gitinfo/gitinfo_helpers_test.go @@ -938,10 +938,10 @@ func TestHandleModalResult_WorkflowDispatch_MalformedName(t *testing.T) { assert.Nil(t, cmd, "malformed pendingName should return nil cmd") } -func TestHandleModalResult_WorkflowDispatchInputs(t *testing.T) { +func TestHandleModalResult_WorkflowDispatchRaw(t *testing.T) { t.Parallel() p := newTestPanel(t, defaultMock()) - p.pending = opWorkflowDispatchInputs + p.pending = opWorkflowDispatchRaw p.pendingName = "456:Deploy:main" p.gh.client = &mockGHClientFull{} // non-nil to avoid panic @@ -954,10 +954,10 @@ func TestHandleModalResult_WorkflowDispatchInputs(t *testing.T) { assert.NoError(t, result.err) } -func TestHandleModalResult_WorkflowDispatchInputs_EmptyInputs(t *testing.T) { +func TestHandleModalResult_WorkflowDispatchRaw_EmptyInputs(t *testing.T) { t.Parallel() p := newTestPanel(t, defaultMock()) - p.pending = opWorkflowDispatchInputs + p.pending = opWorkflowDispatchRaw p.pendingName = "456:Deploy:main" p.gh.client = &mockGHClientFull{} @@ -969,20 +969,20 @@ func TestHandleModalResult_WorkflowDispatchInputs_EmptyInputs(t *testing.T) { assert.Equal(t, "Deploy", result.workflowName) } -func TestHandleModalResult_WorkflowDispatchInputs_InvalidID(t *testing.T) { +func TestHandleModalResult_WorkflowDispatchRaw_InvalidID(t *testing.T) { t.Parallel() p := newTestPanel(t, defaultMock()) - p.pending = opWorkflowDispatchInputs + p.pending = opWorkflowDispatchRaw p.pendingName = "0:Deploy:main" // workflowID=0 _, cmd := p.handleModalResult(notify.ModalResultMsg{Accept: true, Value: "key=val"}) assert.Nil(t, cmd, "workflowID=0 should return nil cmd") } -func TestHandleModalResult_WorkflowDispatchInputs_MissingRef(t *testing.T) { +func TestHandleModalResult_WorkflowDispatchRaw_MissingRef(t *testing.T) { t.Parallel() p := newTestPanel(t, defaultMock()) - p.pending = opWorkflowDispatchInputs + p.pending = opWorkflowDispatchRaw p.pendingName = "456:Deploy" // only 2 parts, ref missing → ref="" _, cmd := p.handleModalResult(notify.ModalResultMsg{Accept: true, Value: "key=val"}) @@ -1085,6 +1085,7 @@ func TestHandleWorkflowInputsFetched_WithInputs(t *testing.T) { workflowID: 42, workflowName: "Deploy", ref: "main", + inputsKnown: true, inputs: []ghclient.WorkflowInput{ {Name: "environment", Default: "staging"}, {Name: "version", Default: "latest"}, @@ -1092,26 +1093,29 @@ func TestHandleWorkflowInputsFetched_WithInputs(t *testing.T) { }) require.NotNil(t, cmd) assert.Equal(t, opWorkflowDispatchInputs, p.pending) - assert.Equal(t, "42:Deploy:main", p.pendingName) + require.Len(t, p.wfDispatch.inputs, 2) + assert.Equal(t, 0, p.wfDispatch.idx) msg := cmd() modal, ok := msg.(notify.ShowModalMsg) require.True(t, ok) assert.Contains(t, modal.Title, "Deploy") + assert.Equal(t, "staging", modal.Value, "first input's default pre-fills the field") } func TestHandleWorkflowInputsFetched_NoInputs(t *testing.T) { t.Parallel() p := newTestPanel(t, defaultMock()) + // Inputs could not be read → free-form composer fallback. _, cmd := p.handleWorkflowInputsFetched(workflowInputsFetchedMsg{ workflowID: 42, workflowName: "CI", ref: "main", - inputs: nil, + inputsKnown: false, }) require.NotNil(t, cmd) - assert.Equal(t, opWorkflowDispatchInputs, p.pending) + assert.Equal(t, opWorkflowDispatchRaw, p.pending) } // --------------------------------------------------------------------------- diff --git a/internal/panels/gitinfo/gitinfo_modal_handlers.go b/internal/panels/gitinfo/gitinfo_modal_handlers.go index 45ea0ca1..b0939ce1 100644 --- a/internal/panels/gitinfo/gitinfo_modal_handlers.go +++ b/internal/panels/gitinfo/gitinfo_modal_handlers.go @@ -331,15 +331,17 @@ func (p *Panel) handleWorkflowDispatch(a modalArgs) (panels.Panel, tea.Cmd) { owner, repo := p.gh.owner, p.gh.repo ghClient := p.gh.client ctx := a.ctx - return p, func() tea.Msg { + return p, guardedGitHubCmd("gitinfo.getWorkflowInputs", func() tea.Msg { var wfInputs []ghclient.WorkflowInput + inputsKnown := false if ghClient != nil && workflowPath != "" { fetched, err := ghClient.GetWorkflowInputs(ctx, owner, repo, workflowPath, ref) if err != nil { - // Non-fatal: fall back to generic dialog. - _ = err + // Non-fatal: fall back to the free-form key=value composer. + slog.Warn("github: fetch workflow inputs failed", "path", workflowPath, "err", err) } else { wfInputs = fetched + inputsKnown = true } } return workflowInputsFetchedMsg{ @@ -347,13 +349,36 @@ func (p *Panel) handleWorkflowDispatch(a modalArgs) (panels.Panel, tea.Cmd) { workflowName: workflowName, ref: ref, inputs: wfInputs, + inputsKnown: inputsKnown, } - } + }) } +// handleWorkflowDispatchInputs records the value entered (or picked) for the +// current workflow_dispatch input and advances to the next input, firing the +// dispatch once every input has been collected. An empty value keeps the +// workflow's declared default. func (p *Panel) handleWorkflowDispatchInputs(a modalArgs) (panels.Panel, tea.Cmd) { - // Step 2 complete: got the inputs. Parse and dispatch. - // pendingName format: "id:name:ref" + d := &p.wfDispatch + if d.idx >= len(d.inputs) { + // Defensive: no input to record (stale state) — just fire. + return p.fireWorkflowDispatch() + } + input := d.inputs[d.idx] + if value := strings.TrimSpace(a.msg.Value); value != "" { + if d.values == nil { + d.values = make(map[string]any) + } + d.values[input.Name] = value + } + d.idx++ + return p.promptNextWorkflowInput() +} + +// handleWorkflowDispatchRaw dispatches a workflow using free-form key=value +// inputs entered in the fallback composer (used when the workflow's declared +// inputs could not be read). pendingName format: "id:name:ref". +func (p *Panel) handleWorkflowDispatchRaw(a modalArgs) (panels.Panel, tea.Cmd) { var workflowID int64 var workflowName, ref string parts := strings.SplitN(a.name, ":", 3) @@ -362,42 +387,31 @@ func (p *Panel) handleWorkflowDispatchInputs(a modalArgs) (panels.Panel, tea.Cmd workflowName = parts[1] ref = parts[2] } - if workflowID == 0 || ref == "" { - return p, nil + return p, p.dispatchWorkflowCmd(workflowID, workflowName, ref, parseKeyValueInputs(a.msg.Value)) +} + +// parseKeyValueInputs parses free-form "key=value" lines into a dispatch input +// map. Blank lines and lines without an "=" are ignored. Returns nil when no +// valid pairs are present. +func parseKeyValueInputs(s string) map[string]any { + text := strings.TrimSpace(s) + if text == "" { + return nil } - // Parse inputs from "key=value" lines. - var inputs map[string]any - inputText := strings.TrimSpace(a.msg.Value) - if inputText != "" { - inputs = make(map[string]any) - for _, line := range strings.Split(inputText, "\n") { - line = strings.TrimSpace(line) - if line == "" { - continue - } - if kv := strings.SplitN(line, "=", 2); len(kv) == 2 { - inputs[strings.TrimSpace(kv[0])] = strings.TrimSpace(kv[1]) - } + inputs := make(map[string]any) + for _, line := range strings.Split(text, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue } - if len(inputs) == 0 { - inputs = nil + if kv := strings.SplitN(line, "=", 2); len(kv) == 2 { + inputs[strings.TrimSpace(kv[0])] = strings.TrimSpace(kv[1]) } } - owner, repo := p.gh.owner, p.gh.repo - ghClient := p.gh.client - ctx := a.ctx - if ghClient == nil { - // Defensive: the dispatch flow should not start without a client, - // but guard here so a nil client surfaces as a failure toast rather - // than a nil dereference that crashes the TUI. - return p, func() tea.Msg { - return workflowDispatchResultMsg{workflowName: workflowName, err: errGitHubClientUnavailable} - } - } - return p, func() tea.Msg { - err := ghClient.DispatchWorkflow(ctx, owner, repo, workflowID, ref, inputs) - return workflowDispatchResultMsg{workflowName: workflowName, err: err} + if len(inputs) == 0 { + return nil } + return inputs } func (p *Panel) handlePRMergeStrategy(a modalArgs) (panels.Panel, tea.Cmd) { diff --git a/internal/panels/gitinfo/gitinfo_notifications.go b/internal/panels/gitinfo/gitinfo_notifications.go index c49d6186..bb623be4 100644 --- a/internal/panels/gitinfo/gitinfo_notifications.go +++ b/internal/panels/gitinfo/gitinfo_notifications.go @@ -48,7 +48,7 @@ func (p *Panel) loadNotifications() tea.Cmd { return nil } ctx := p.ctx - return func() tea.Msg { + return guardedGitHubCmd("gitinfo.loadNotifications", func() tea.Msg { var result ghNotificationsLoadedMsg notifs, err := client.ListNotifications(ctx, &gh.NotificationListOptions{All: false}) if err != nil { @@ -71,7 +71,7 @@ func (p *Panel) loadNotifications() tea.Cmd { }) } return result - } + }) } // handleNotificationsLoaded stores loaded notifications into the tab. diff --git a/web/src/pages/docs/github-workflows.astro b/web/src/pages/docs/github-workflows.astro index 4cfea365..1d6c40c2 100644 --- a/web/src/pages/docs/github-workflows.astro +++ b/web/src/pages/docs/github-workflows.astro @@ -31,8 +31,10 @@ import KeybindingTable from '../../components/KeybindingTable.astro';

When you select a workflow, the preview panel shows the workflow YAML definition alongside its recent run history. Dispatching a workflow - opens an input form where you can provide parameter values before - triggering the run. + prompts you for the ref, then for each declared input in turn: choice and + boolean inputs are presented as a picker of their valid values (preselected + on the workflow's default), while free-form inputs offer a text field + pre-filled with the default so you can accept it or enter a custom value.

From e9c2d7e8b4c0a834702e582cb69152666193701b Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Fri, 24 Jul 2026 17:45:11 -0700 Subject: [PATCH 2/3] docs(spec): mark workflow-dispatch picker spec shipped (PR #370) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b9a3d924-7b48-4000-9d4d-fb592f3485b6 --- .../workflow-dispatch-param-picker/spec.md | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/specs/workflow-dispatch-param-picker/spec.md b/docs/specs/workflow-dispatch-param-picker/spec.md index 11f1b744..d951c9e0 100644 --- a/docs/specs/workflow-dispatch-param-picker/spec.md +++ b/docs/specs/workflow-dispatch-param-picker/spec.md @@ -1,7 +1,8 @@ --- title: Workflow-dispatch parameter picker and command-goroutine crash safety net -status: draft -issue: pending +status: shipped +issue: n/a (follow-up to #362) +pr: https://github.com/jongio/grut/pull/370 scope: P1 --- @@ -88,13 +89,14 @@ Dispatching a GitHub Actions workflow from the GitHub panel had two shortcomings ## Pipeline Status -Phase: SHIPPING (late entry via `go verify`) +Phase: SHIPPED (PR https://github.com/jongio/grut/pull/370) Certification (Phase 4): `go build`, `go vet ./...`, `golangci-lint ./...` (0 issues), `gofumpt -l .` (clean), `deadcode ./...` (0 new; all 210 findings pre-existing/allowlisted), -`govulncheck ./...` (no vulnerabilities), full `go test ./...` (exit 0). Race/WSL/benchmark -steps of `mage preflight` are CI-covered (need cgo/WSL unavailable locally). doc-check: astro -GitHub-workflows page + CHANGELOG updated; keybindings unchanged (up-to-date test passes). -Review: code-review (1 MEDIUM, fixed) + rubber-duck (crash-collision + stranded-loading fixed; -async-identity/inputsKnown/blank-trim assessed as pre-existing or intentional — see test-plan -triage). Test plan COVERED, 0 gaps. +`govulncheck ./...` (no vulnerabilities), full `go test ./...` (exit 0, pre- and post-rebase). +Race/WSL/benchmark steps of `mage preflight` are CI-covered (need cgo/WSL unavailable locally). +doc-check: astro GitHub-workflows page + CHANGELOG updated; keybindings unchanged (up-to-date +test passes). Review: code-review (1 MEDIUM, fixed) + rubber-duck (crash-collision + +stranded-loading fixed; async-identity/inputsKnown/blank-trim assessed as pre-existing or +intentional — see test-plan triage). Test plan COVERED, 0 gaps. Rebased cleanly onto origin/main +(dep bump #363); full suite green post-rebase. From 12d12c33458dbdd0c1b7f7fed7b0d3b26ee39fd0 Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Fri, 24 Jul 2026 17:57:13 -0700 Subject: [PATCH 3/3] fix(notify): drop now-unused ShowMultilineInputWithValue (deadcode) The per-input dispatch flow replaced the pre-populated multi-line composer with per-input prompts, leaving ShowMultilineInputWithValue with no production caller, which failed CI's `mage deadcode` gate. Remove it and its constructor test; retain the multi-line edit/submit coverage by driving the modal via ShowModalMsg directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b9a3d924-7b48-4000-9d4d-fb592f3485b6 --- internal/notify/modal.go | 14 -------------- internal/notify/notify_test.go | 25 +++++++++---------------- 2 files changed, 9 insertions(+), 30 deletions(-) diff --git a/internal/notify/modal.go b/internal/notify/modal.go index 64727f57..5656d832 100644 --- a/internal/notify/modal.go +++ b/internal/notify/modal.go @@ -750,20 +750,6 @@ func ShowMultilineInput(title, placeholder string) tea.Cmd { } } -// ShowMultilineInputWithValue returns a tea.Cmd that produces a ShowModalMsg -// for a multi-line text composer pre-filled with the given value. Enter -// inserts a newline, Ctrl+D submits, and Esc cancels. -func ShowMultilineInputWithValue(title, placeholder, value string) tea.Cmd { - return func() tea.Msg { - return ShowModalMsg{ - Title: title, - Placeholder: placeholder, - Value: value, - Kind: ModalMultilineInput, - } - } -} - // ShowConfirmWithCheckbox returns a tea.Cmd that produces a ShowModalMsg // for a confirmation dialog with a "remember this choice" checkbox. func ShowConfirmWithCheckbox(title, message, checkboxLabel string) tea.Cmd { diff --git a/internal/notify/notify_test.go b/internal/notify/notify_test.go index 6a9e36c7..df6ed2c5 100644 --- a/internal/notify/notify_test.go +++ b/internal/notify/notify_test.go @@ -534,23 +534,16 @@ func TestShowMultilineInput(t *testing.T) { assert.Equal(t, ModalMultilineInput, smm.Kind) } -func TestShowMultilineInputWithValue(t *testing.T) { - cmd := ShowMultilineInputWithValue("Inputs for Deploy", "edit values", "env=prod\nversion=1.0") - require.NotNil(t, cmd) - - msg := cmd() - smm, ok := msg.(ShowModalMsg) - require.True(t, ok) - assert.Equal(t, "Inputs for Deploy", smm.Title) - assert.Equal(t, "edit values", smm.Placeholder) - assert.Equal(t, "env=prod\nversion=1.0", smm.Value) - assert.Equal(t, ModalMultilineInput, smm.Kind) -} - -func TestShowMultilineInputWithValueSubmitsEditedValue(t *testing.T) { +// TestMultilineInputSubmitsEditedValue verifies a multi-line modal pre-filled +// with a value (via ShowModalMsg) starts the cursor at the end of the value, +// inserts a newline on Enter, and submits the composed value on Ctrl+D. +func TestMultilineInputSubmitsEditedValue(t *testing.T) { m := NewManager() - cmd := ShowMultilineInputWithValue("Inputs for Deploy", "edit values", "env=prod") - m.Update(cmd()) + m.Update(ShowModalMsg{ + Title: "Inputs for Deploy", + Value: "env=prod", + Kind: ModalMultilineInput, + }) require.True(t, m.HasModal()) // Cursor starts at end of the pre-filled value; add a second input line.