fix(gitinfo): pick workflow-dispatch inputs and stop command-goroutine crashes

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

docs/specs/workflow-dispatch-param-picker/spec.md

@@ -0,0 +1,102 @@
+---
+title: Workflow-dispatch parameter picker and command-goroutine crash safety net
+status: shipped
+issue: n/a (follow-up to #362)
+pr: https://github.com/jongio/grut/pull/370
+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 tracking (auto-managed, not part of product spec) -->
+## Pipeline Status
+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, 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.

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`.
+

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
 // ---------------------------------------------------------------------------

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

internal/crashlog/writer.go

@@ -8,13 +8,28 @@ import (
 	"path/filepath"
 	"slices"
 	"strings"
+	"sync"
+	"sync/atomic"
 
 	"github.com/jongio/grut/internal/config"
 )
 
 // 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, "", "  ")

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
 }