From e0be67f7047d6e2d477814d0119fcc3845e57b21 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 10:29:48 -0400 Subject: [PATCH 01/21] feat(ui): header and row rendering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add internal/ui with Lip Gloss rendering for the list header and rows: status markers (red ●, yellow ◐), stage column, first-flag reason, key hints from a pure RowAction function, dimmed stale rows with a "~ " stage prefix, per-source header segments, and graceful narrow widths. Styles bind to an injectable renderer so NO_COLOR is tested against a real color terminal profile. Assisted-by: Claude Code:claude-opus-5-5 --- go.mod | 16 +++ go.sum | 35 ++++++ internal/ui/header.go | 65 ++++++++++ internal/ui/header_test.go | 86 +++++++++++++ internal/ui/hint.go | 87 +++++++++++++ internal/ui/hint_test.go | 74 +++++++++++ internal/ui/row.go | 142 +++++++++++++++++++++ internal/ui/row_test.go | 249 +++++++++++++++++++++++++++++++++++++ internal/ui/styles.go | 51 ++++++++ internal/ui/styles_test.go | 51 ++++++++ 10 files changed, 856 insertions(+) create mode 100644 internal/ui/header.go create mode 100644 internal/ui/header_test.go create mode 100644 internal/ui/hint.go create mode 100644 internal/ui/hint_test.go create mode 100644 internal/ui/row.go create mode 100644 internal/ui/row_test.go create mode 100644 internal/ui/styles.go create mode 100644 internal/ui/styles_test.go diff --git a/go.mod b/go.mod index 8ea27d5..cb61bdf 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,9 @@ require ( github.com/aws/aws-sdk-go-v2/service/codepipeline v1.40.1 github.com/aws/aws-sdk-go-v2/service/ecs v1.79.1 github.com/aws/smithy-go v1.25.1 + github.com/charmbracelet/lipgloss v1.1.0 + github.com/charmbracelet/x/ansi v0.11.5 + github.com/muesli/termenv v0.16.0 ) require ( @@ -24,4 +27,17 @@ require ( github.com/aws/aws-sdk-go-v2/service/sso v1.25.3 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.30.1 // indirect github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 // indirect + github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect + github.com/charmbracelet/colorprofile v0.4.1 // indirect + github.com/charmbracelet/x/cellbuf v0.0.15 // indirect + github.com/charmbracelet/x/term v0.2.2 // indirect + github.com/clipperhouse/displaywidth v0.9.0 // indirect + github.com/clipperhouse/stringish v0.1.1 // indirect + github.com/clipperhouse/uax29/v2 v2.5.0 // indirect + github.com/lucasb-eyer/go-colorful v1.3.0 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect + github.com/mattn/go-runewidth v0.0.19 // indirect + github.com/rivo/uniseg v0.4.7 // indirect + github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect + golang.org/x/sys v0.38.0 // indirect ) diff --git a/go.sum b/go.sum index 5beb622..318412f 100644 --- a/go.sum +++ b/go.sum @@ -30,3 +30,38 @@ github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 h1:1XuUZ8mYJw9B6lzAkXhqHlJd/Xv github.com/aws/aws-sdk-go-v2/service/sts v1.33.19/go.mod h1:cQnB8CUnxbMU82JvlqjKR2HBOm3fe9pWorWBza6MBJ4= github.com/aws/smithy-go v1.25.1 h1:J8ERsGSU7d+aCmdQur5Txg6bVoYelvQJgtZehD12GkI= github.com/aws/smithy-go v1.25.1/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= +github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= +github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= +github.com/charmbracelet/colorprofile v0.4.1 h1:a1lO03qTrSIRaK8c3JRxJDZOvhvIeSco3ej+ngLk1kk= +github.com/charmbracelet/colorprofile v0.4.1/go.mod h1:U1d9Dljmdf9DLegaJ0nGZNJvoXAhayhmidOdcBwAvKk= +github.com/charmbracelet/lipgloss v1.1.0 h1:vYXsiLHVkK7fp74RkV7b2kq9+zDLoEU4MZoFqR/noCY= +github.com/charmbracelet/lipgloss v1.1.0/go.mod h1:/6Q8FR2o+kj8rz4Dq0zQc3vYf7X+B0binUUBwA0aL30= +github.com/charmbracelet/x/ansi v0.11.5 h1:NBWeBpj/lJPE3Q5l+Lusa4+mH6v7487OP8K0r1IhRg4= +github.com/charmbracelet/x/ansi v0.11.5/go.mod h1:2JNYLgQUsyqaiLovhU2Rv/pb8r6ydXKS3NIttu3VGZQ= +github.com/charmbracelet/x/cellbuf v0.0.15 h1:ur3pZy0o6z/R7EylET877CBxaiE1Sp1GMxoFPAIztPI= +github.com/charmbracelet/x/cellbuf v0.0.15/go.mod h1:J1YVbR7MUuEGIFPCaaZ96KDl5NoS0DAWkskup+mOY+Q= +github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSgfgZRk= +github.com/charmbracelet/x/term v0.2.2/go.mod h1:kF8CY5RddLWrsgVwpw4kAa6TESp6EB5y3uxGLeCqzAI= +github.com/clipperhouse/displaywidth v0.9.0 h1:Qb4KOhYwRiN3viMv1v/3cTBlz3AcAZX3+y9OLhMtAtA= +github.com/clipperhouse/displaywidth v0.9.0/go.mod h1:aCAAqTlh4GIVkhQnJpbL0T/WfcrJXHcj8C0yjYcjOZA= +github.com/clipperhouse/stringish v0.1.1 h1:+NSqMOr3GR6k1FdRhhnXrLfztGzuG+VuFDfatpWHKCs= +github.com/clipperhouse/stringish v0.1.1/go.mod h1:v/WhFtE1q0ovMta2+m+UbpZ+2/HEXNWYXQgCt4hdOzA= +github.com/clipperhouse/uax29/v2 v2.5.0 h1:x7T0T4eTHDONxFJsL94uKNKPHrclyFI0lm7+w94cO8U= +github.com/clipperhouse/uax29/v2 v2.5.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsVRgg6W7ihQeh4g= +github.com/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag= +github.com/lucasb-eyer/go-colorful v1.3.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-runewidth v0.0.19 h1:v++JhqYnZuu5jSKrk9RbgF5v4CGUjqRfBm05byFGLdw= +github.com/mattn/go-runewidth v0.0.19/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= +github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= +github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= +github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= +github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88= +github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= +github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= +golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= +golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= +golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= diff --git a/internal/ui/header.go b/internal/ui/header.go new file mode 100644 index 0000000..92dc9f5 --- /dev/null +++ b/internal/ui/header.go @@ -0,0 +1,65 @@ +package ui + +import ( + "strings" + "time" + + "github.com/charmbracelet/x/ansi" + "github.com/ericdahl-dev/app-green/internal/resolver" +) + +// RenderHeader renders the header line on the default styles. +func RenderHeader(statuses []resolver.AdapterStatus, now time.Time, width int) string { + return DefaultStyles().Header(statuses, now, width) +} + +// Header renders "app-green" and one short segment per source, in the order +// given (Snapshot.Status order). +func (s Styles) Header(statuses []resolver.AdapterStatus, now time.Time, width int) string { + line := s.header(statuses, now, true) + if ansi.StringWidth(line) <= width { + return line + } + // Too wide: OK sources drop their ages first, then the line is cut. + line = s.header(statuses, now, false) + if ansi.StringWidth(line) <= width { + return line + } + return ansi.Truncate(line, max(width, 0), "…") +} + +func (s Styles) header(statuses []resolver.AdapterStatus, now time.Time, ages bool) string { + parts := []string{s.bold.Render("app-green")} + for _, st := range statuses { + parts = append(parts, s.segment(st, now, ages)) + } + return strings.Join(parts, " ") +} + +// errWidth caps a generic error in the header, in cells. +const errWidth = 30 + +// segment is one source's status: "jira ✓ 12s", "github ✗ token rejected", +// "aws prod-acct ✗ sso expired", "aws stage-acct ⏸ throttled 2m", or +// "jira ✗ ". A source that has not answered yet shows "…". With ages +// false an OK source shows no age. +func (s Styles) segment(st resolver.AdapterStatus, now time.Time, ages bool) string { + switch { + case st.OK && (st.At.IsZero() || !ages): + return st.Name + " " + s.green.Render("✓") + case st.OK: + return st.Name + " " + s.green.Render("✓") + " " + age(now.Sub(st.At)) + case st.Auth: + return st.Name + " " + s.red.Render("✗ token rejected") + case st.SSO: + return st.Name + " " + s.red.Render("✗ sso expired") + case st.Throttled && now.Before(st.RetryAt): + return st.Name + " " + s.yellow.Render("⏸ throttled "+age(st.RetryAt.Sub(now))) + case st.Throttled: + return st.Name + " " + s.yellow.Render("⏸ throttled") + case st.Err != "": + msg, _, _ := strings.Cut(st.Err, "\n") + return st.Name + " " + s.red.Render("✗ "+ansi.Truncate(msg, errWidth, "…")) + } + return st.Name + " " + s.dim.Render("…") +} diff --git a/internal/ui/header_test.go b/internal/ui/header_test.go new file mode 100644 index 0000000..8fee7ae --- /dev/null +++ b/internal/ui/header_test.go @@ -0,0 +1,86 @@ +package ui_test + +import ( + "testing" + "time" + + "github.com/charmbracelet/x/ansi" + "github.com/ericdahl-dev/app-green/internal/resolver" + "github.com/ericdahl-dev/app-green/internal/ui" +) + +func ok(name string, ago time.Duration) resolver.AdapterStatus { + return resolver.AdapterStatus{Name: name, OK: true, At: now.Add(-ago)} +} + +func TestHeaderAllOK(t *testing.T) { + st := []resolver.AdapterStatus{ok("jira", 12*time.Second), ok("github", 8*time.Second), ok("aws stage-acct", 3*time.Minute)} + want := "app-green jira ✓ 12s github ✓ 8s aws stage-acct ✓ 3m" + if got := ui.RenderHeader(st, now, 100); got != want { + t.Errorf("header\n got %q\nwant %q", got, want) + } +} + +func TestHeaderSegments(t *testing.T) { + tests := []struct { + name string + st resolver.AdapterStatus + want string + }{ + {"ok with no time", resolver.AdapterStatus{Name: "jira", OK: true}, "jira ✓"}, + {"token rejected", resolver.AdapterStatus{Name: "github", Err: "token rejected", Auth: true, At: now.Add(-time.Hour)}, "github ✗ token rejected"}, + {"sso expired", resolver.AdapterStatus{Name: "aws prod-acct", Err: "operation error STS: GetCallerIdentity, token has expired", SSO: true}, "aws prod-acct ✗ sso expired"}, + {"throttled", resolver.AdapterStatus{Name: "aws stage-acct", Err: "ThrottlingException: Rate exceeded", Throttled: true, RetryAt: now.Add(2*time.Minute + 5*time.Second)}, "aws stage-acct ⏸ throttled 2m"}, + {"throttled, retry due", resolver.AdapterStatus{Name: "jira", Err: "HTTP 429", Throttled: true, RetryAt: now.Add(-time.Second)}, "jira ⏸ throttled"}, + {"auth beats throttled", resolver.AdapterStatus{Name: "jira", Err: "token rejected", Auth: true, Throttled: true, RetryAt: now.Add(time.Minute)}, "jira ✗ token rejected"}, + {"sso beats throttled", resolver.AdapterStatus{Name: "aws prod-acct", Err: "expired", SSO: true, Throttled: true, RetryAt: now.Add(time.Minute)}, "aws prod-acct ✗ sso expired"}, + {"generic error", resolver.AdapterStatus{Name: "jira", Err: "HTTP 502 Bad Gateway"}, "jira ✗ HTTP 502 Bad Gateway"}, + {"long error", resolver.AdapterStatus{Name: "jira", Err: "Get \"https://example.atlassian.net/rest/api/3/search\": dial tcp: lookup example.atlassian.net: no such host"}, "jira ✗ Get \"https://example.atlassia…"}, + {"multi-line error", resolver.AdapterStatus{Name: "github", Err: "HTTP 500\nupstream body"}, "github ✗ HTTP 500"}, + {"not loaded yet", resolver.AdapterStatus{Name: "aws stage-acct"}, "aws stage-acct …"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + want := "app-green " + tt.want + if got := ui.RenderHeader([]resolver.AdapterStatus{tt.st}, now, 100); got != want { + t.Errorf("header\n got %q\nwant %q", got, want) + } + }) + } +} + +func TestHeaderMixed(t *testing.T) { + st := []resolver.AdapterStatus{ + ok("jira", 12*time.Second), + {Name: "github", Err: "token rejected", Auth: true}, + {Name: "aws stage-acct", Throttled: true, RetryAt: now.Add(2 * time.Minute)}, + {Name: "aws prod-acct", SSO: true, Err: "expired"}, + } + want := "app-green jira ✓ 12s github ✗ token rejected aws stage-acct ⏸ throttled 2m aws prod-acct ✗ sso expired" + if got := ui.RenderHeader(st, now, 120); got != want { + t.Errorf("header\n got %q\nwant %q", got, want) + } +} + +func TestHeaderNarrow(t *testing.T) { + st := []resolver.AdapterStatus{ok("jira", 12*time.Second), ok("github", 8*time.Second), {Name: "aws prod-acct", SSO: true}} + full := "app-green jira ✓ 12s github ✓ 8s aws prod-acct ✗ sso expired" + if got := ui.RenderHeader(st, now, 63); got != full { + t.Errorf("fits exactly:\n got %q\nwant %q", got, full) + } + // One cell short: OK sources drop their ages before anything is cut. + want := "app-green jira ✓ github ✓ aws prod-acct ✗ sso expired" + if got := ui.RenderHeader(st, now, 62); got != want { + t.Errorf("compact:\n got %q\nwant %q", got, want) + } + // Still too wide: cut at the right edge. + want = "app-green jira ✓ github ✓ aws prod-a…" + if got := ui.RenderHeader(st, now, 40); got != want { + t.Errorf("cut:\n got %q\nwant %q", got, want) + } + for w := 0; w <= 120; w++ { + if got := ui.RenderHeader(st, now, w); ansi.StringWidth(got) > w { + t.Fatalf("width %d: header is %d cells: %q", w, ansi.StringWidth(got), got) + } + } +} diff --git a/internal/ui/hint.go b/internal/ui/hint.go new file mode 100644 index 0000000..0ad6d26 --- /dev/null +++ b/internal/ui/hint.go @@ -0,0 +1,87 @@ +package ui + +import ( + "fmt" + + "github.com/ericdahl-dev/app-green/internal/model" +) + +// Action is what a row's key does for its first flag. +type Action int + +const ( + ActionNone Action = iota + ActionRerun // f: re-run the PR's failed Actions runs + ActionOpen // o: open the target in the browser + ActionApprove // a/x: approve or reject the waiting approval + ActionStatus // t: change the Jira status +) + +var actionKeys = [...]string{"", "f", "o", "a/x", "t"} + +// Key is the hint shown at the end of the row, "" for ActionNone. +func (a Action) Key() string { + if a < 0 || int(a) >= len(actionKeys) { + return "" + } + return actionKeys[a] +} + +func (a Action) String() string { + if a == ActionNone { + return "none" + } + if a < 0 || int(a) >= len(actionKeys) { + return fmt.Sprintf("Action(%d)", int(a)) + } + return actionKeys[a] +} + +// RowAction is the action a row offers for c.Flags[0], or ActionNone when the +// flag has no single target the action can work on. Other flags never count. +func RowAction(c model.Chain) Action { + if len(c.Flags) == 0 { + return ActionNone + } + f := c.Flags[0] + switch f.Kind { + case model.FlagCheckFailed: + if f.PR != nil { + return failedCheckAction(f.PR.Failing) + } + case model.FlagPipelineFailed, model.FlagRolledBack, model.FlagDeployUnknown: + if f.Slot != nil { + return ActionOpen + } + case model.FlagStranded, model.FlagCheckExpected: + if f.PR != nil { + return ActionOpen + } + case model.FlagAwaitingApproval: + if s := f.Slot; s != nil && !s.Env.ReadOnly && s.Deploy != nil && s.Deploy.ApprovalToken != "" { + return ActionApprove + } + case model.FlagStatusMismatch: + return ActionStatus + } + return ActionNone +} + +// failedCheckAction is f when every failing check is a re-runnable Actions +// run, o when any is code scanning (a re-run cannot fix it), else none. +func failedCheckAction(cs []model.Check) Action { + if len(cs) == 0 { + return ActionNone + } + rerun := true + for _, c := range cs { + if c.CodeScanning { + return ActionOpen + } + rerun = rerun && c.RunID > 0 + } + if rerun { + return ActionRerun + } + return ActionNone +} diff --git a/internal/ui/hint_test.go b/internal/ui/hint_test.go new file mode 100644 index 0000000..197592d --- /dev/null +++ b/internal/ui/hint_test.go @@ -0,0 +1,74 @@ +package ui_test + +import ( + "testing" + + "github.com/ericdahl-dev/app-green/internal/model" + "github.com/ericdahl-dev/app-green/internal/ui" +) + +func TestRowAction(t *testing.T) { + rspec := model.Check{Name: "rspec", RunID: 42} + codeql := model.Check{Name: "CodeQL", RunID: 43, CodeScanning: true} + external := model.Check{Name: "ci/external"} + failingPR := func(cs ...model.Check) *model.PR { + return &model.PR{Number: 3, State: model.PROpen, Checks: model.ChecksFailing, Failing: cs} + } + waiting := func(readOnly bool, token string) *model.EnvSlot { + return &model.EnvSlot{ + Env: model.Env{Account: "prod-acct", Stage: "Production", ReadOnly: readOnly}, + State: model.SlotAwaitingApproval, + Deploy: &model.Deploy{Status: model.DeployAwaitingApproval, ApprovalToken: token}, + } + } + slot := &model.EnvSlot{Env: model.Env{Account: "stage-acct", Stage: "Test"}} + pr := &model.PR{Number: 3} + tests := []struct { + name string + flag *model.Flag + want ui.Action + key string + }{ + {"no flags", nil, ui.ActionNone, ""}, + {"re-runnable failed check", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(rspec)}, ui.ActionRerun, "f"}, + {"two re-runnable failed checks", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(rspec, model.Check{Name: "lint", RunID: 44})}, ui.ActionRerun, "f"}, + {"code scanning failed", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(codeql)}, ui.ActionOpen, "o"}, + {"code scanning beside a re-runnable check", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(rspec, codeql)}, ui.ActionOpen, "o"}, + {"failed check outside Actions", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(external)}, ui.ActionNone, ""}, + {"failed check with no details", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR()}, ui.ActionNone, ""}, + {"failed check without a PR", &model.Flag{Kind: model.FlagCheckFailed}, ui.ActionNone, ""}, + {"pipeline failed", &model.Flag{Kind: model.FlagPipelineFailed, Slot: slot}, ui.ActionOpen, "o"}, + {"rolled back", &model.Flag{Kind: model.FlagRolledBack, Slot: slot}, ui.ActionOpen, "o"}, + {"stranded", &model.Flag{Kind: model.FlagStranded, PR: pr}, ui.ActionOpen, "o"}, + {"check expected", &model.Flag{Kind: model.FlagCheckExpected, PR: pr}, ui.ActionOpen, "o"}, + {"deploy unknown in an env", &model.Flag{Kind: model.FlagDeployUnknown, Slot: slot}, ui.ActionOpen, "o"}, + {"repo no env deploys", &model.Flag{Kind: model.FlagDeployUnknown}, ui.ActionNone, ""}, + {"pipeline failed without a slot", &model.Flag{Kind: model.FlagPipelineFailed}, ui.ActionNone, ""}, + {"awaiting approval", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: waiting(false, "tok")}, ui.ActionApprove, "a/x"}, + {"awaiting approval, read-only profile", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: waiting(true, "tok")}, ui.ActionNone, ""}, + {"awaiting approval, no token", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: waiting(false, "")}, ui.ActionNone, ""}, + {"awaiting approval, no deploy", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: &model.EnvSlot{State: model.SlotAwaitingApproval}}, ui.ActionNone, ""}, + {"status mismatch", &model.Flag{Kind: model.FlagStatusMismatch}, ui.ActionStatus, "t"}, + {"unhealthy", &model.Flag{Kind: model.FlagUnhealthy, Slot: slot}, ui.ActionNone, ""}, + {"changes requested", &model.Flag{Kind: model.FlagChangesRequested, PR: pr}, ui.ActionNone, ""}, + {"partial prod", &model.Flag{Kind: model.FlagPartialProd, Slot: slot}, ui.ActionNone, ""}, + {"ready to merge", &model.Flag{Kind: model.FlagReadyToMerge, PR: pr}, ui.ActionNone, ""}, + {"stale review", &model.Flag{Kind: model.FlagStaleReview, PR: pr}, ui.ActionNone, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var c model.Chain + if tt.flag != nil { + // A second flag must not change the answer: only Flags[0] counts. + c.Flags = []model.Flag{*tt.flag, {Kind: model.FlagStatusMismatch}} + } + got := ui.RowAction(c) + if got != tt.want { + t.Errorf("RowAction = %v, want %v", got, tt.want) + } + if k := got.Key(); k != tt.key { + t.Errorf("Key = %q, want %q", k, tt.key) + } + }) + } +} diff --git a/internal/ui/row.go b/internal/ui/row.go new file mode 100644 index 0000000..a4d1e17 --- /dev/null +++ b/internal/ui/row.go @@ -0,0 +1,142 @@ +package ui + +import ( + "fmt" + "time" + + "github.com/charmbracelet/x/ansi" + "github.com/ericdahl-dev/app-green/internal/model" +) + +// Layout is the column widths shared by every row of the list. +type Layout struct { + Width int // the whole row, in cells + KeyWidth int // the longest ticket key + TitleWidth int + StageWidth int +} + +// hintWidth fits the widest key hint, "a/x". +const hintWidth = 3 + +// gaps is the cells a row spends outside its key, title, stage and reason +// columns: the marker, five gaps and the hint. +const gaps = 1 + 1 + 2 + 2 + 2 + 2 + hintWidth + +// NewLayout sizes the columns for chains in a list width cells wide. The key +// and stage columns fit their longest entry; the title gets half of what is +// left, at most its longest entry, and the reason the rest. +func NewLayout(chains []model.Chain, now time.Time, width int) Layout { + l := Layout{Width: width} + var title int + for _, c := range chains { + l.KeyWidth = max(l.KeyWidth, ansi.StringWidth(c.Ticket.Key)) + l.StageWidth = max(l.StageWidth, ansi.StringWidth(stageColumn(c, now))) + title = max(title, ansi.StringWidth(c.Ticket.Title)) + } + l.TitleWidth = max(0, min(title, (width-l.KeyWidth-l.StageWidth-gaps)/2)) + return l +} + +// RenderRow renders c as one list row on the default styles. +func RenderRow(c model.Chain, l Layout, now time.Time) string { + return DefaultStyles().Row(c, l, now) +} + +// Row renders c as one row exactly l.Width cells wide: marker, key, title, +// stage, reason and key hint. +func (s Styles) Row(c model.Chain, l Layout, now time.Time) string { + if l.Width <= 0 { + return "" + } + reasonW := l.Width - (l.KeyWidth + l.StageWidth + gaps) - l.TitleWidth + reason := "" + if len(c.Flags) > 0 { + reason = c.Flags[0].Reason + } + title := "" + if l.TitleWidth > 0 { + title = fit(c.Ticket.Title, l.TitleWidth) + " " + } else { + reasonW += 2 // no title column, so no gap after it + } + rest := " " + fit(c.Ticket.Key, l.KeyWidth) + " " + title + + fit(stageColumn(c, now), l.StageWidth) + " " + fit(reason, reasonW) + " " + fit(RowAction(c).Key(), hintWidth) + // Too narrow for every column: cut at the right edge, before styling. + rest = fit(rest, l.Width-1) + if c.Stale { + // Dim the marker and the rest separately: wrapping the colored marker + // in a faint style would let its reset end the dimming. + return s.withDim().marker(c.Level()) + s.dim.Render(rest) + } + return s.marker(c.Level()) + rest +} + +// stageColumn is StageText, after "~ " when the row may be behind. +func stageColumn(c model.Chain, now time.Time) string { + if c.Stale { + return "~ " + StageText(c, now) + } + return StageText(c, now) +} + +// marker is the row's one-cell status glyph: red ●, yellow ◐, or a space. +func (s Styles) marker(l model.Level) string { + switch l { + case model.Red: + return s.red.Render("●") + case model.Yellow: + return s.yellow.Render("◐") + } + return " " +} + +// StageText is the row's stage column: how far c's work has got. +func StageText(c model.Chain, now time.Time) string { + switch c.Stage { + case model.StagePROpen: + for _, p := range c.PRs { + if p.State == model.PROpen { + return fmt.Sprintf("PR #%d", p.Number) + } + } + case model.StageInTest: + return "test ✓" + case model.StageAwaitingProd: + return "prod ⏸" + case model.StageInProd: + if at := lastProd(c); !at.IsZero() { + return "in prod ✓ " + age(now.Sub(at)) + " ago" + } + return "in prod ✓" + } + return c.Stage.String() +} + +// lastProd is when the latest applicable prod deploy of c went live; zero if +// none has a time. +func lastProd(c model.Chain) time.Time { + var last time.Time + for _, s := range c.Slots { + if s.Applies && s.Env.Prod && s.State == model.SlotDeployed && s.At.After(last) { + last = s.At + } + } + return last +} + +// age is d in its largest whole unit: 12s, 5m, 3h, 2d. Negative d (clock +// skew) is 0s. +func age(d time.Duration) string { + switch { + case d < 0: + return "0s" + case d < time.Minute: + return fmt.Sprintf("%ds", int(d.Seconds())) + case d < time.Hour: + return fmt.Sprintf("%dm", int(d.Minutes())) + case d < 24*time.Hour: + return fmt.Sprintf("%dh", int(d.Hours())) + } + return fmt.Sprintf("%dd", int(d.Hours()/24)) +} diff --git a/internal/ui/row_test.go b/internal/ui/row_test.go new file mode 100644 index 0000000..48fffbf --- /dev/null +++ b/internal/ui/row_test.go @@ -0,0 +1,249 @@ +package ui_test + +import ( + "os" + "strings" + "testing" + "time" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" + "github.com/ericdahl-dev/app-green/internal/model" + "github.com/ericdahl-dev/app-green/internal/ui" + "github.com/muesli/termenv" +) + +func TestMain(m *testing.M) { + lipgloss.SetColorProfile(termenv.Ascii) + os.Exit(m.Run()) +} + +var now = time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + +// layout is the fixed layout the exact-output row tests use: an 80-cell row +// leaves 80 - (6 + 12 + 13) - 20 = 29 cells for the reason. +var layout = ui.Layout{Width: 80, KeyWidth: 6, TitleWidth: 20, StageWidth: 12} + +// row renders c and checks the row fills the width exactly; it returns the +// row with trailing padding trimmed so expectations stay readable. +func row(t *testing.T, c model.Chain, l ui.Layout) string { + t.Helper() + got := ui.RenderRow(c, l, now) + if w := ansi.StringWidth(got); w != l.Width { + t.Errorf("row is %d cells wide, want %d: %q", w, l.Width, got) + } + return strings.TrimRight(got, " ") +} + +func ticket(key, title string) model.Ticket { + return model.Ticket{Key: key, Title: title, Status: "In Progress", StatusCategory: model.StatusInProgress} +} + +func TestRowStartedHasNoMarker(t *testing.T) { + c := model.Chain{Ticket: ticket("ABC-1", "Short title"), Stage: model.StageStarted} + want := " ABC-1 Short title started" + if got := row(t, c, layout); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } +} + +func TestRowRedFailingCheck(t *testing.T) { + c := model.Chain{ + Ticket: ticket("ABC-1", "Short title"), + PRs: []model.PR{{Repo: "acme/app", Number: 3, State: model.PROpen, Checks: model.ChecksFailing, + Failing: []model.Check{{Name: "rspec", RunID: 42}}}}, + Stage: model.StagePROpen, + } + c.Flags = []model.Flag{{Level: model.Red, Kind: model.FlagCheckFailed, Reason: "PR #3 rspec failing", PR: &c.PRs[0]}} + want := "● ABC-1 Short title PR #3 PR #3 rspec failing f" + if got := row(t, c, layout); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } +} + +func TestStageText(t *testing.T) { + prodSlot := func(at time.Time, state model.SlotState, applies bool) model.EnvSlot { + return model.EnvSlot{Env: model.Env{Account: "prod-acct", Stage: "Production", Prod: true}, State: state, At: at, Applies: applies} + } + testSlot := model.EnvSlot{Env: model.Env{Account: "stage-acct", Stage: "Test"}, State: model.SlotDeployed, At: now.Add(-time.Minute), Applies: true} + tests := []struct { + name string + c model.Chain + want string + }{ + {"started", model.Chain{Stage: model.StageStarted}, "started"}, + {"first open PR", model.Chain{Stage: model.StagePROpen, PRs: []model.PR{{Number: 2, State: model.PRClosed}, {Number: 7, State: model.PROpen}, {Number: 9, State: model.PROpen}}}, "PR #7"}, + {"PR open with no open PR", model.Chain{Stage: model.StagePROpen}, "PR open"}, + {"merged", model.Chain{Stage: model.StageMerged}, "merged"}, + {"in test", model.Chain{Stage: model.StageInTest}, "test ✓"}, + {"awaiting prod", model.Chain{Stage: model.StageAwaitingProd}, "prod ⏸"}, + {"in prod, latest prod deploy", model.Chain{Stage: model.StageInProd, Slots: []model.EnvSlot{ + testSlot, + prodSlot(now.Add(-5*time.Hour), model.SlotDeployed, true), + prodSlot(now.Add(-3*time.Hour), model.SlotDeployed, true), + prodSlot(now.Add(-time.Hour), model.SlotDeployed, false), // deploys none of the chain's repos + }}, "in prod ✓ 3h ago"}, + {"in prod minutes ago", model.Chain{Stage: model.StageInProd, Slots: []model.EnvSlot{prodSlot(now.Add(-90*time.Second), model.SlotDeployed, true)}}, "in prod ✓ 1m ago"}, + {"in prod seconds ago", model.Chain{Stage: model.StageInProd, Slots: []model.EnvSlot{prodSlot(now.Add(-12*time.Second), model.SlotDeployed, true)}}, "in prod ✓ 12s ago"}, + {"in prod days ago", model.Chain{Stage: model.StageInProd, Slots: []model.EnvSlot{prodSlot(now.Add(-50*time.Hour), model.SlotDeployed, true)}}, "in prod ✓ 2d ago"}, + {"in prod with no time", model.Chain{Stage: model.StageInProd, Slots: []model.EnvSlot{prodSlot(time.Time{}, model.SlotDeployed, true)}}, "in prod ✓"}, + {"in prod clock skew", model.Chain{Stage: model.StageInProd, Slots: []model.EnvSlot{prodSlot(now.Add(time.Minute), model.SlotDeployed, true)}}, "in prod ✓ 0s ago"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ui.StageText(tt.c, now); got != tt.want { + t.Errorf("StageText = %q, want %q", got, tt.want) + } + }) + } +} + +func TestRowYellowAwaitingApproval(t *testing.T) { + c := model.Chain{ + Ticket: ticket("ABC-22", "Form opt-outs"), + Slots: []model.EnvSlot{{ + Env: model.Env{Account: "prod-acct", Stage: "Production", Prod: true}, + State: model.SlotAwaitingApproval, + Applies: true, + Deploy: &model.Deploy{Status: model.DeployAwaitingApproval, ApprovalToken: "tok"}, + }}, + Stage: model.StageAwaitingProd, + } + c.Flags = []model.Flag{{Level: model.Yellow, Kind: model.FlagAwaitingApproval, Reason: "prod-acct Production awaiting approval", Slot: &c.Slots[0]}} + want := "◐ ABC-22 Form opt-outs prod ⏸ prod-acct Production awaitin… a/x" + if got := row(t, c, layout); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } +} + +func TestRowInProdWithAge(t *testing.T) { + c := model.Chain{ + Ticket: ticket("ABC-3", "Item count"), + Slots: []model.EnvSlot{{Env: model.Env{Account: "prod-acct", Stage: "Production", Prod: true}, + State: model.SlotDeployed, At: now.Add(-3 * time.Hour), Applies: true}}, + Stage: model.StageInProd, + } + want := " ABC-3 Item count in prod ✓ 3…" + if got := row(t, c, layout); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } + wide := layout + wide.StageWidth = 16 + want = " ABC-3 Item count in prod ✓ 3h ago" + if got := row(t, c, wide); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } +} + +func TestRowTruncatesLongTitle(t *testing.T) { + c := model.Chain{Ticket: ticket("ABC-4", "Remove the legacy task path from the worker"), Stage: model.StageMerged} + want := " ABC-4 Remove the legacy t… merged" + if got := row(t, c, layout); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } +} + +func TestRowStaleHasTildePrefix(t *testing.T) { + c := model.Chain{Ticket: ticket("ABC-5", "Health checks"), Stage: model.StageInTest, Stale: true, StaleReason: "acme/app failed to load"} + want := " ABC-5 Health checks ~ test ✓" + if got := row(t, c, layout); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } +} + +func TestRowStaleIsDimmed(t *testing.T) { + lipgloss.SetColorProfile(termenv.ANSI) + defer lipgloss.SetColorProfile(termenv.Ascii) + c := model.Chain{Ticket: ticket("ABC-6", "Dim me"), Stage: model.StageMerged, Stale: true} + c.Flags = []model.Flag{{Level: model.Red, Kind: model.FlagStatusMismatch, Reason: "Jira still In Progress, PR merged"}} + got := ui.RenderRow(c, layout, now) + // The marker is red and faint; everything after it is one faint run, so a + // reset inside the row cannot end the dimming early. + marker, rest, ok := strings.Cut(got, "●\x1b[0m") + if !ok || (marker != "\x1b[2;31m" && marker != "\x1b[31;2m") { + t.Fatalf("marker is not red and faint: %q", got) + } + want := "\x1b[2m" + strings.TrimPrefix(ansi.Strip(got), "●") + "\x1b[0m" + if rest != want { + t.Errorf("rest of the row\n got %q\nwant %q", rest, want) + } + + c.Stale = false + if got := ui.RenderRow(c, layout, now); strings.Contains(got, "\x1b[2m") || strings.Contains(got, ";2m") { + t.Errorf("a fresh row is dimmed: %q", got) + } +} + +// board is a list with one row of each kind the tests cover. +func board() []model.Chain { + inProd := model.Chain{ + Ticket: ticket("ABC-1003", "Item count on the add page"), + Slots: []model.EnvSlot{{Env: model.Env{Account: "prod-acct", Stage: "Production", Prod: true}, + State: model.SlotDeployed, At: now.Add(-3 * time.Hour), Applies: true}}, + Stage: model.StageInProd, + Stale: true, + } + failing := model.Chain{ + Ticket: ticket("ABC-1", "Short title"), + PRs: []model.PR{{Repo: "acme/app", Number: 3, State: model.PROpen, Checks: model.ChecksFailing, + Failing: []model.Check{{Name: "rspec", RunID: 42}}}}, + Stage: model.StagePROpen, + } + failing.Flags = []model.Flag{{Level: model.Red, Kind: model.FlagCheckFailed, Reason: "PR #3 rspec failing", PR: &failing.PRs[0]}} + started := model.Chain{Ticket: ticket("ABC-22", "S3 access logging"), Stage: model.StageStarted} + return []model.Chain{failing, started, inProd} +} + +func TestNewLayout(t *testing.T) { + got := ui.NewLayout(board(), now, 100) + // fixed = 8 (key) + 18 ("~ in prod ✓ 3h ago") + 13 = 39, leaving 61; the + // title takes half, capped at the longest title (26). + want := ui.Layout{Width: 100, KeyWidth: 8, TitleWidth: 26, StageWidth: 18} + if got != want { + t.Errorf("NewLayout = %+v, want %+v", got, want) + } + got = ui.NewLayout(board(), now, 80) + want = ui.Layout{Width: 80, KeyWidth: 8, TitleWidth: 20, StageWidth: 18} + if got != want { + t.Errorf("NewLayout = %+v, want %+v", got, want) + } + if got := ui.NewLayout(nil, now, 80); got != (ui.Layout{Width: 80}) { + t.Errorf("NewLayout(nil) = %+v", got) + } +} + +func TestRowsAtEveryWidth(t *testing.T) { + chains := board() + for w := 0; w <= 120; w++ { + l := ui.NewLayout(chains, now, w) + for _, c := range chains { + if got := ui.RenderRow(c, l, now); ansi.StringWidth(got) != w { + t.Fatalf("width %d: row is %d cells: %q", w, ansi.StringWidth(got), got) + } + } + } +} + +func TestRowsAtBoardWidth(t *testing.T) { + chains := board() + l := ui.NewLayout(chains, now, 100) + want := []string{ + "● ABC-1 Short title PR #3 PR #3 rspec failing" + strings.Repeat(" ", 16+2) + "f", + " ABC-22 S3 access logging started", + " ABC-1003 Item count on the add page ~ in prod ✓ 3h ago", + } + for i, c := range chains { + if got := row(t, c, l); got != want[i] { + t.Errorf("row %d\n got %q\nwant %q", i, got, want[i]) + } + } +} + +func TestRowNarrowCutsAtTheRightEdge(t *testing.T) { + chains := board() + l := ui.NewLayout(chains, now, 30) + want := "● ABC-1 PR #3 …" + if got := ui.RenderRow(chains[0], l, now); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } +} diff --git a/internal/ui/styles.go b/internal/ui/styles.go new file mode 100644 index 0000000..7d4785f --- /dev/null +++ b/internal/ui/styles.go @@ -0,0 +1,51 @@ +// Package ui renders app-green's screens with Bubble Tea and Lip Gloss. +package ui + +import ( + "strings" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" +) + +// Styles holds every style the screens use, bound to one renderer, so a test +// can render through a renderer with a known terminal and environment. +type Styles struct { + red lipgloss.Style + yellow lipgloss.Style + green lipgloss.Style + dim lipgloss.Style + bold lipgloss.Style +} + +// NewStyles builds the styles on r. The renderer decides the color profile; +// it honors NO_COLOR. +func NewStyles(r *lipgloss.Renderer) Styles { + return Styles{ + red: r.NewStyle().Foreground(lipgloss.Color("1")), + yellow: r.NewStyle().Foreground(lipgloss.Color("3")), + green: r.NewStyle().Foreground(lipgloss.Color("2")), + dim: r.NewStyle().Faint(true), + bold: r.NewStyle().Bold(true), + } +} + +// withDim is s with the marker colors also faint, for a stale row. +func (s Styles) withDim() Styles { + s.red = s.red.Faint(true) + s.yellow = s.yellow.Faint(true) + return s +} + +// DefaultStyles builds the styles on Lip Gloss's default renderer (stdout). +func DefaultStyles() Styles { return NewStyles(lipgloss.DefaultRenderer()) } + +// fit cuts s to w cells, ending in "…" when cut, and pads it with spaces to +// exactly w cells. w at or below zero gives "". +func fit(s string, w int) string { + if w <= 0 { + return "" + } + s = ansi.Truncate(s, w, "…") + return s + strings.Repeat(" ", w-ansi.StringWidth(s)) +} diff --git a/internal/ui/styles_test.go b/internal/ui/styles_test.go new file mode 100644 index 0000000..320a87d --- /dev/null +++ b/internal/ui/styles_test.go @@ -0,0 +1,51 @@ +package ui_test + +import ( + "io" + "strings" + "testing" + "time" + + "github.com/charmbracelet/lipgloss" + "github.com/ericdahl-dev/app-green/internal/model" + "github.com/ericdahl-dev/app-green/internal/resolver" + "github.com/ericdahl-dev/app-green/internal/ui" + "github.com/muesli/termenv" +) + +// env is a fake process environment for a renderer. +type env map[string]string + +func (e env) Getenv(k string) string { return e[k] } +func (e env) Environ() []string { + var out []string + for k, v := range e { + out = append(out, k+"="+v) + } + return out +} + +// terminal renders through a color terminal with the given environment, the +// way the app renders to a real one: the renderer detects the profile. +func terminal(e env) ui.Styles { + return ui.NewStyles(lipgloss.NewRenderer(io.Discard, termenv.WithTTY(true), termenv.WithEnvironment(e))) +} + +func TestNoColor(t *testing.T) { + c := model.Chain{Ticket: ticket("ABC-1", "Short title"), Stage: model.StagePROpen, Stale: true} + c.Flags = []model.Flag{{Level: model.Red, Kind: model.FlagStatusMismatch, Reason: "Jira still In Progress, PR merged"}} + st := []resolver.AdapterStatus{{Name: "jira", OK: true, At: now.Add(-time.Second)}, {Name: "aws prod-acct", SSO: true}} + render := func(s ui.Styles) string { return s.Header(st, now, 100) + "\n" + s.Row(c, layout, now) } + + // Without NO_COLOR the same terminal gets color, so the test below is + // not passing only because nothing is ever colored. + got := render(terminal(env{"TERM": "xterm-256color"})) + for _, want := range []string{"\x1b[32m✓\x1b[0m", "\x1b[31m✗ sso expired\x1b[0m"} { + if !strings.Contains(got, want) { + t.Fatalf("a color terminal is missing %q: %q", want, got) + } + } + if got := render(terminal(env{"TERM": "xterm-256color", "NO_COLOR": "1"})); strings.Contains(got, "\x1b[") { + t.Errorf("NO_COLOR output has escape codes: %q", got) + } +} From 1f726142a4b3485b8a192c1908000d4be92edb51 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 10:35:31 -0400 Subject: [PATCH 02/21] fix(ui): hint and header rules - f when any failing check is a re-runnable Actions run; a non-Actions check alone opens its URL with o. - A key whose action has two targets across the row's flags (two approvals, re-runnable checks in two PRs) is not offered. - An approval the profile cannot make (read-only env or no token) offers o to open the pipeline console. - A narrow header drops OK sources, last first, before cutting, so a problem source is never hidden while an OK one shows. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/header.go | 25 ++++++++++--- internal/ui/header_test.go | 25 ++++++++++--- internal/ui/hint.go | 59 ++++++++++++++++++++++-------- internal/ui/hint_test.go | 73 +++++++++++++++++++++++++++++++++++--- 4 files changed, 154 insertions(+), 28 deletions(-) diff --git a/internal/ui/header.go b/internal/ui/header.go index 92dc9f5..b7d18f1 100644 --- a/internal/ui/header.go +++ b/internal/ui/header.go @@ -1,6 +1,7 @@ package ui import ( + "slices" "strings" "time" @@ -20,12 +21,26 @@ func (s Styles) Header(statuses []resolver.AdapterStatus, now time.Time, width i if ansi.StringWidth(line) <= width { return line } - // Too wide: OK sources drop their ages first, then the line is cut. - line = s.header(statuses, now, false) - if ansi.StringWidth(line) <= width { - return line + // Too wide: OK sources drop their ages, then drop out one at a time from + // the end, so a problem is never hidden while an OK source shows. Only + // then is the line cut. + shown := slices.Clone(statuses) + for { + line = s.header(shown, now, false) + if ansi.StringWidth(line) <= width { + return line + } + last := -1 + for i, st := range shown { + if st.OK { + last = i + } + } + if last < 0 { + return ansi.Truncate(line, max(width, 0), "…") + } + shown = slices.Delete(shown, last, last+1) } - return ansi.Truncate(line, max(width, 0), "…") } func (s Styles) header(statuses []resolver.AdapterStatus, now time.Time, ages bool) string { diff --git a/internal/ui/header_test.go b/internal/ui/header_test.go index 8fee7ae..7239876 100644 --- a/internal/ui/header_test.go +++ b/internal/ui/header_test.go @@ -73,10 +73,27 @@ func TestHeaderNarrow(t *testing.T) { if got := ui.RenderHeader(st, now, 62); got != want { t.Errorf("compact:\n got %q\nwant %q", got, want) } - // Still too wide: cut at the right edge. - want = "app-green jira ✓ github ✓ aws prod-a…" - if got := ui.RenderHeader(st, now, 40); got != want { - t.Errorf("cut:\n got %q\nwant %q", got, want) + // Still too wide: OK sources drop out, last first, so a problem is never + // hidden while an OK source shows. + for _, tt := range []struct { + width int + want string + }{ + {46, "app-green jira ✓ aws prod-acct ✗ sso expired"}, + {45, "app-green aws prod-acct ✗ sso expired"}, + {38, "app-green aws prod-acct ✗ sso expired"}, + // Only problems left and still too wide: cut at the right edge. + {30, "app-green aws prod-acct ✗ ss…"}, + } { + if got := ui.RenderHeader(st, now, tt.width); got != tt.want { + t.Errorf("width %d:\n got %q\nwant %q", tt.width, got, tt.want) + } + } + // Problems keep their order around a dropped OK source. + mixed := []resolver.AdapterStatus{{Name: "github", Auth: true}, ok("jira", time.Second), {Name: "aws prod-acct", SSO: true}} + want = "app-green github ✗ token rejected aws prod-acct ✗ sso expired" + if got := ui.RenderHeader(mixed, now, 64); got != want { + t.Errorf("mixed:\n got %q\nwant %q", got, want) } for w := 0; w <= 120; w++ { if got := ui.RenderHeader(st, now, w); ansi.StringWidth(got) > w { diff --git a/internal/ui/hint.go b/internal/ui/hint.go index 0ad6d26..96c6b3b 100644 --- a/internal/ui/hint.go +++ b/internal/ui/hint.go @@ -2,6 +2,7 @@ package ui import ( "fmt" + "strings" "github.com/ericdahl-dev/app-green/internal/model" ) @@ -38,12 +39,28 @@ func (a Action) String() string { } // RowAction is the action a row offers for c.Flags[0], or ActionNone when the -// flag has no single target the action can work on. Other flags never count. +// flag has no target the action can work on, or when the row's flags give +// that action more than one target (two approvals waiting, failing re-runnable +// checks in two PRs): then the detail screen chooses. func RowAction(c model.Chain) Action { if len(c.Flags) == 0 { return ActionNone } - f := c.Flags[0] + a := flagAction(c.Flags[0]) + if a == ActionNone { + return ActionNone + } + first := target(c.Flags[0]) + for _, f := range c.Flags[1:] { + if flagAction(f) == a && target(f) != first { + return ActionNone + } + } + return a +} + +// flagAction is the action f alone offers, ActionNone without a target. +func flagAction(f model.Flag) Action { switch f.Kind { case model.FlagCheckFailed: if f.PR != nil { @@ -58,8 +75,11 @@ func RowAction(c model.Chain) Action { return ActionOpen } case model.FlagAwaitingApproval: - if s := f.Slot; s != nil && !s.Env.ReadOnly && s.Deploy != nil && s.Deploy.ApprovalToken != "" { - return ActionApprove + if s := f.Slot; s != nil { + if !s.Env.ReadOnly && s.Deploy != nil && s.Deploy.ApprovalToken != "" { + return ActionApprove + } + return ActionOpen // cannot approve here: open the pipeline console } case model.FlagStatusMismatch: return ActionStatus @@ -67,21 +87,30 @@ func RowAction(c model.Chain) Action { return ActionNone } -// failedCheckAction is f when every failing check is a re-runnable Actions -// run, o when any is code scanning (a re-run cannot fix it), else none. -func failedCheckAction(cs []model.Check) Action { - if len(cs) == 0 { - return ActionNone +// target names what f's action works on: its Env, its PR, or else the ticket. +func target(f model.Flag) string { + switch { + case f.Slot != nil: + return "env " + f.Slot.Env.ID() + case f.PR != nil: + return fmt.Sprintf("pr %s#%d", strings.ToLower(f.PR.Repo), f.PR.Number) } - rerun := true + return "ticket" +} + +// failedCheckAction is f when any failing check is a re-runnable Actions run +// (the re-run acts on those; code scanning stays for the detail screen), o +// when any is code scanning or has a URL to open, else none. +func failedCheckAction(cs []model.Check) Action { + open := false for _, c := range cs { - if c.CodeScanning { - return ActionOpen + if c.RunID > 0 && !c.CodeScanning { + return ActionRerun } - rerun = rerun && c.RunID > 0 + open = open || c.CodeScanning || c.URL != "" } - if rerun { - return ActionRerun + if open { + return ActionOpen } return ActionNone } diff --git a/internal/ui/hint_test.go b/internal/ui/hint_test.go index 197592d..6d5fd0f 100644 --- a/internal/ui/hint_test.go +++ b/internal/ui/hint_test.go @@ -11,6 +11,7 @@ func TestRowAction(t *testing.T) { rspec := model.Check{Name: "rspec", RunID: 42} codeql := model.Check{Name: "CodeQL", RunID: 43, CodeScanning: true} external := model.Check{Name: "ci/external"} + externalURL := model.Check{Name: "ci/external", URL: "https://ci.example.com/acme/app/1"} failingPR := func(cs ...model.Check) *model.PR { return &model.PR{Number: 3, State: model.PROpen, Checks: model.ChecksFailing, Failing: cs} } @@ -33,7 +34,9 @@ func TestRowAction(t *testing.T) { {"re-runnable failed check", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(rspec)}, ui.ActionRerun, "f"}, {"two re-runnable failed checks", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(rspec, model.Check{Name: "lint", RunID: 44})}, ui.ActionRerun, "f"}, {"code scanning failed", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(codeql)}, ui.ActionOpen, "o"}, - {"code scanning beside a re-runnable check", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(rspec, codeql)}, ui.ActionOpen, "o"}, + {"code scanning beside a re-runnable check", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(rspec, codeql)}, ui.ActionRerun, "f"}, + {"outside Actions beside a re-runnable check", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(external, rspec)}, ui.ActionRerun, "f"}, + {"failed check outside Actions with a URL", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(externalURL)}, ui.ActionOpen, "o"}, {"failed check outside Actions", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(external)}, ui.ActionNone, ""}, {"failed check with no details", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR()}, ui.ActionNone, ""}, {"failed check without a PR", &model.Flag{Kind: model.FlagCheckFailed}, ui.ActionNone, ""}, @@ -45,9 +48,10 @@ func TestRowAction(t *testing.T) { {"repo no env deploys", &model.Flag{Kind: model.FlagDeployUnknown}, ui.ActionNone, ""}, {"pipeline failed without a slot", &model.Flag{Kind: model.FlagPipelineFailed}, ui.ActionNone, ""}, {"awaiting approval", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: waiting(false, "tok")}, ui.ActionApprove, "a/x"}, - {"awaiting approval, read-only profile", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: waiting(true, "tok")}, ui.ActionNone, ""}, - {"awaiting approval, no token", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: waiting(false, "")}, ui.ActionNone, ""}, - {"awaiting approval, no deploy", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: &model.EnvSlot{State: model.SlotAwaitingApproval}}, ui.ActionNone, ""}, + {"awaiting approval, read-only profile", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: waiting(true, "tok")}, ui.ActionOpen, "o"}, + {"awaiting approval, no token", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: waiting(false, "")}, ui.ActionOpen, "o"}, + {"awaiting approval, no deploy", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: &model.EnvSlot{State: model.SlotAwaitingApproval}}, ui.ActionOpen, "o"}, + {"awaiting approval without a slot", &model.Flag{Kind: model.FlagAwaitingApproval}, ui.ActionNone, ""}, {"status mismatch", &model.Flag{Kind: model.FlagStatusMismatch}, ui.ActionStatus, "t"}, {"unhealthy", &model.Flag{Kind: model.FlagUnhealthy, Slot: slot}, ui.ActionNone, ""}, {"changes requested", &model.Flag{Kind: model.FlagChangesRequested, PR: pr}, ui.ActionNone, ""}, @@ -72,3 +76,64 @@ func TestRowAction(t *testing.T) { }) } } + +// TestRowActionNeedsOneTarget: when the row's flags give the first flag's key +// two targets, the key is not offered; the detail screen chooses. +func TestRowActionNeedsOneTarget(t *testing.T) { + failing := func(n int, cs ...model.Check) *model.PR { + return &model.PR{Repo: "acme/app", Number: n, State: model.PROpen, Checks: model.ChecksFailing, Failing: cs} + } + approval := func(account string, readOnly bool) *model.EnvSlot { + return &model.EnvSlot{ + Env: model.Env{Account: account, Pipeline: "app", Stage: "Production", ReadOnly: readOnly}, + State: model.SlotAwaitingApproval, + Deploy: &model.Deploy{Status: model.DeployAwaitingApproval, ApprovalToken: "tok"}, + } + } + rspec := model.Check{Name: "rspec", RunID: 42} + codeql := model.Check{Name: "CodeQL", RunID: 43, CodeScanning: true} + failedSlot := &model.EnvSlot{Env: model.Env{Account: "stage-acct", Pipeline: "app", Stage: "Test"}, State: model.SlotFailed} + pr3 := &model.PR{Repo: "acme/app", Number: 3} + tests := []struct { + name string + flags []model.Flag + want ui.Action + }{ + {"two approvals waiting", []model.Flag{ + {Kind: model.FlagAwaitingApproval, Slot: approval("stage-acct", false)}, + {Kind: model.FlagAwaitingApproval, Slot: approval("prod-acct", false)}, + }, ui.ActionNone}, + {"approval beside one the profile cannot approve", []model.Flag{ + {Kind: model.FlagAwaitingApproval, Slot: approval("stage-acct", false)}, + {Kind: model.FlagAwaitingApproval, Slot: approval("prod-acct", true)}, + }, ui.ActionApprove}, + {"re-runnable checks in two PRs", []model.Flag{ + {Kind: model.FlagCheckFailed, PR: failing(3, rspec)}, + {Kind: model.FlagCheckFailed, PR: failing(4, rspec)}, + }, ui.ActionNone}, + {"re-runnable check beside a PR with only code scanning", []model.Flag{ + {Kind: model.FlagCheckFailed, PR: failing(3, rspec)}, + {Kind: model.FlagCheckFailed, PR: failing(4, codeql)}, + }, ui.ActionRerun}, + {"two things to open", []model.Flag{ + {Kind: model.FlagPipelineFailed, Slot: failedSlot}, + {Kind: model.FlagCheckExpected, PR: pr3}, + }, ui.ActionNone}, + {"the same target twice", []model.Flag{ + {Kind: model.FlagCheckExpected, PR: pr3}, + {Kind: model.FlagStranded, PR: &model.PR{Repo: "acme/app", Number: 3}}, + }, ui.ActionOpen}, + {"a second key elsewhere does not count", []model.Flag{ + {Kind: model.FlagPipelineFailed, Slot: failedSlot}, + {Kind: model.FlagAwaitingApproval, Slot: approval("prod-acct", false)}, + {Kind: model.FlagStatusMismatch}, + }, ui.ActionOpen}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ui.RowAction(model.Chain{Flags: tt.flags}); got != tt.want { + t.Errorf("RowAction = %v, want %v", got, tt.want) + } + }) + } +} From 36b45fd327990cfbf2ceb501d7cd5eb1577d6fea Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 10:41:32 -0400 Subject: [PATCH 03/21] fix(ui): clean untrusted text Ticket keys and titles, flag reasons and source errors come from Jira, GitHub and AWS. clean turns tabs, newlines and carriage returns into spaces, strips escape sequences (an OSC 52 sequence could write the clipboard) and drops other control characters and invalid bytes before any text is measured, cut or printed. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/header.go | 17 +++++++++-------- internal/ui/header_test.go | 8 ++++++++ internal/ui/row.go | 10 +++++----- internal/ui/row_test.go | 26 ++++++++++++++++++++++++++ internal/ui/styles.go | 24 ++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 13 deletions(-) diff --git a/internal/ui/header.go b/internal/ui/header.go index b7d18f1..4fbee5d 100644 --- a/internal/ui/header.go +++ b/internal/ui/header.go @@ -59,22 +59,23 @@ const errWidth = 30 // "jira ✗ ". A source that has not answered yet shows "…". With ages // false an OK source shows no age. func (s Styles) segment(st resolver.AdapterStatus, now time.Time, ages bool) string { + name := clean(st.Name) switch { case st.OK && (st.At.IsZero() || !ages): - return st.Name + " " + s.green.Render("✓") + return name + " " + s.green.Render("✓") case st.OK: - return st.Name + " " + s.green.Render("✓") + " " + age(now.Sub(st.At)) + return name + " " + s.green.Render("✓") + " " + age(now.Sub(st.At)) case st.Auth: - return st.Name + " " + s.red.Render("✗ token rejected") + return name + " " + s.red.Render("✗ token rejected") case st.SSO: - return st.Name + " " + s.red.Render("✗ sso expired") + return name + " " + s.red.Render("✗ sso expired") case st.Throttled && now.Before(st.RetryAt): - return st.Name + " " + s.yellow.Render("⏸ throttled "+age(st.RetryAt.Sub(now))) + return name + " " + s.yellow.Render("⏸ throttled "+age(st.RetryAt.Sub(now))) case st.Throttled: - return st.Name + " " + s.yellow.Render("⏸ throttled") + return name + " " + s.yellow.Render("⏸ throttled") case st.Err != "": msg, _, _ := strings.Cut(st.Err, "\n") - return st.Name + " " + s.red.Render("✗ "+ansi.Truncate(msg, errWidth, "…")) + return name + " " + s.red.Render("✗ "+ansi.Truncate(clean(msg), errWidth, "…")) } - return st.Name + " " + s.dim.Render("…") + return name + " " + s.dim.Render("…") } diff --git a/internal/ui/header_test.go b/internal/ui/header_test.go index 7239876..ea94750 100644 --- a/internal/ui/header_test.go +++ b/internal/ui/header_test.go @@ -101,3 +101,11 @@ func TestHeaderNarrow(t *testing.T) { } } } + +func TestHeaderCleansUntrustedText(t *testing.T) { + st := []resolver.AdapterStatus{{Name: "jira", Err: "HTTP 502 \x1b[31mBad\x1b[0m\tGateway\x1b]52;c;ZXZpbA==\x07"}} + want := "app-green jira ✗ HTTP 502 Bad Gateway" + if got := ui.RenderHeader(st, now, 100); got != want { + t.Errorf("header\n got %q\nwant %q", got, want) + } +} diff --git a/internal/ui/row.go b/internal/ui/row.go index a4d1e17..a07a218 100644 --- a/internal/ui/row.go +++ b/internal/ui/row.go @@ -30,9 +30,9 @@ func NewLayout(chains []model.Chain, now time.Time, width int) Layout { l := Layout{Width: width} var title int for _, c := range chains { - l.KeyWidth = max(l.KeyWidth, ansi.StringWidth(c.Ticket.Key)) + l.KeyWidth = max(l.KeyWidth, ansi.StringWidth(clean(c.Ticket.Key))) l.StageWidth = max(l.StageWidth, ansi.StringWidth(stageColumn(c, now))) - title = max(title, ansi.StringWidth(c.Ticket.Title)) + title = max(title, ansi.StringWidth(clean(c.Ticket.Title))) } l.TitleWidth = max(0, min(title, (width-l.KeyWidth-l.StageWidth-gaps)/2)) return l @@ -52,15 +52,15 @@ func (s Styles) Row(c model.Chain, l Layout, now time.Time) string { reasonW := l.Width - (l.KeyWidth + l.StageWidth + gaps) - l.TitleWidth reason := "" if len(c.Flags) > 0 { - reason = c.Flags[0].Reason + reason = clean(c.Flags[0].Reason) } title := "" if l.TitleWidth > 0 { - title = fit(c.Ticket.Title, l.TitleWidth) + " " + title = fit(clean(c.Ticket.Title), l.TitleWidth) + " " } else { reasonW += 2 // no title column, so no gap after it } - rest := " " + fit(c.Ticket.Key, l.KeyWidth) + " " + title + + rest := " " + fit(clean(c.Ticket.Key), l.KeyWidth) + " " + title + fit(stageColumn(c, now), l.StageWidth) + " " + fit(reason, reasonW) + " " + fit(RowAction(c).Key(), hintWidth) // Too narrow for every column: cut at the right edge, before styling. rest = fit(rest, l.Width-1) diff --git a/internal/ui/row_test.go b/internal/ui/row_test.go index 48fffbf..7b37e01 100644 --- a/internal/ui/row_test.go +++ b/internal/ui/row_test.go @@ -247,3 +247,29 @@ func TestRowNarrowCutsAtTheRightEdge(t *testing.T) { t.Errorf("row\n got %q\nwant %q", got, want) } } + +// TestRowCleansUntrustedText: titles, keys and reasons come from Jira, GitHub +// and AWS, so tabs, newlines and escape sequences (an OSC 52 clipboard write +// here) must not reach the terminal or break the layout. +func TestRowCleansUntrustedText(t *testing.T) { + osc52 := "\x1b]52;c;ZXZpbA==\x07" + c := model.Chain{ + Ticket: ticket("ABC-7\r", "Tab\there\nnew"+osc52+"line\x1b[31mred\x9b"), + Stage: model.StageStarted, + } + c.Flags = []model.Flag{{Level: model.Red, Kind: model.FlagCheckFailed, Reason: "PR #3 \x1b[2Jlint\tfailing"}} + chains := []model.Chain{c} + for _, w := range []int{20, 50, 80} { + got := ui.RenderRow(c, ui.NewLayout(chains, now, w), now) + if strings.ContainsAny(got, "\x1b\t\n\r\x07") || strings.Contains(got, "\x9b") { + t.Errorf("width %d: control characters reach the terminal: %q", w, got) + } + if ansi.StringWidth(got) != w { + t.Errorf("width %d: row is %d cells: %q", w, ansi.StringWidth(got), got) + } + } + want := "● ABC-7 Tab here newlinered started PR #3 lint failing" + if got := row(t, c, layout); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } +} diff --git a/internal/ui/styles.go b/internal/ui/styles.go index 7d4785f..a21b8a4 100644 --- a/internal/ui/styles.go +++ b/internal/ui/styles.go @@ -3,6 +3,8 @@ package ui import ( "strings" + "unicode" + "unicode/utf8" "github.com/charmbracelet/lipgloss" "github.com/charmbracelet/x/ansi" @@ -40,6 +42,28 @@ func (s Styles) withDim() Styles { // DefaultStyles builds the styles on Lip Gloss's default renderer (stdout). func DefaultStyles() Styles { return NewStyles(lipgloss.DefaultRenderer()) } +// clean makes untrusted text (anything from Jira, GitHub or AWS) safe to +// print on one line: tabs, newlines and carriage returns become spaces, +// escape sequences are stripped (an OSC 52 sequence could write the +// clipboard), and any other C0 or C1 control or invalid byte is dropped. +func clean(s string) string { + s = strings.NewReplacer("\t", " ", "\n", " ", "\r", " ").Replace(s) + s = ansi.Strip(s) + var b strings.Builder + for i, r := range s { + if r == utf8.RuneError { + if _, size := utf8.DecodeRuneInString(s[i:]); size == 1 { + continue // an invalid byte, such as a raw C1 CSI (0x9b) + } + } + if unicode.IsControl(r) { + continue + } + b.WriteRune(r) + } + return b.String() +} + // fit cuts s to w cells, ending in "…" when cut, and pads it with spaces to // exactly w cells. w at or below zero gives "". func fit(s string, w int) string { From 1a5f1f4f8165bcfae1521b25116c3443815976cf Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 10:44:42 -0400 Subject: [PATCH 04/21] feat(ui): review decisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Flags with an obvious target offer o: unhealthy and partial prod open the pipeline, changes requested, ready to merge and stale review open the PR. flagAction has a case for every FlagKind, and a test fails on a kind added without one. - A failing check with no URL of its own, in Actions or code scanning, offers o for the PR's checks page (ChecksPage). - Narrow rows keep the marker and hint: the reason and title give way first, then the stage and key are cut. - The pause glyph is now ‖ (stage "prod ‖", header "‖ throttled"). - The throttle countdown rounds up, so it never reads shorter than the wait. - Document that a Layout is rebuilt every render; Action.Key is a switch; the dim test renders through a fake color terminal instead of changing the global profile. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/header.go | 22 +++++++- internal/ui/header_test.go | 11 +++- internal/ui/hint.go | 92 +++++++++++++++++++++---------- internal/ui/hint_internal_test.go | 27 +++++++++ internal/ui/hint_test.go | 43 ++++++++++++--- internal/ui/row.go | 25 ++++++--- internal/ui/row_test.go | 28 +++++++--- 7 files changed, 191 insertions(+), 57 deletions(-) create mode 100644 internal/ui/hint_internal_test.go diff --git a/internal/ui/header.go b/internal/ui/header.go index 4fbee5d..1d1a6e7 100644 --- a/internal/ui/header.go +++ b/internal/ui/header.go @@ -1,6 +1,7 @@ package ui import ( + "fmt" "slices" "strings" "time" @@ -51,11 +52,26 @@ func (s Styles) header(statuses []resolver.AdapterStatus, now time.Time, ages bo return strings.Join(parts, " ") } +// countdown is d in its largest unit, rounded up so it never reads shorter +// than the wait: 500ms is 1s, 2m59s is 3m. d must be positive. +func countdown(d time.Duration) string { + up := func(d, unit time.Duration) int64 { return int64((d + unit - 1) / unit) } + switch { + case d <= 59*time.Second: + return fmt.Sprintf("%ds", up(d, time.Second)) + case d <= 59*time.Minute: + return fmt.Sprintf("%dm", up(d, time.Minute)) + case d <= 23*time.Hour: + return fmt.Sprintf("%dh", up(d, time.Hour)) + } + return fmt.Sprintf("%dd", up(d, 24*time.Hour)) +} + // errWidth caps a generic error in the header, in cells. const errWidth = 30 // segment is one source's status: "jira ✓ 12s", "github ✗ token rejected", -// "aws prod-acct ✗ sso expired", "aws stage-acct ⏸ throttled 2m", or +// "aws prod-acct ✗ sso expired", "aws stage-acct ‖ throttled 2m", or // "jira ✗ ". A source that has not answered yet shows "…". With ages // false an OK source shows no age. func (s Styles) segment(st resolver.AdapterStatus, now time.Time, ages bool) string { @@ -70,9 +86,9 @@ func (s Styles) segment(st resolver.AdapterStatus, now time.Time, ages bool) str case st.SSO: return name + " " + s.red.Render("✗ sso expired") case st.Throttled && now.Before(st.RetryAt): - return name + " " + s.yellow.Render("⏸ throttled "+age(st.RetryAt.Sub(now))) + return name + " " + s.yellow.Render("‖ throttled "+countdown(st.RetryAt.Sub(now))) case st.Throttled: - return name + " " + s.yellow.Render("⏸ throttled") + return name + " " + s.yellow.Render("‖ throttled") case st.Err != "": msg, _, _ := strings.Cut(st.Err, "\n") return name + " " + s.red.Render("✗ "+ansi.Truncate(clean(msg), errWidth, "…")) diff --git a/internal/ui/header_test.go b/internal/ui/header_test.go index ea94750..e5f9029 100644 --- a/internal/ui/header_test.go +++ b/internal/ui/header_test.go @@ -30,8 +30,13 @@ func TestHeaderSegments(t *testing.T) { {"ok with no time", resolver.AdapterStatus{Name: "jira", OK: true}, "jira ✓"}, {"token rejected", resolver.AdapterStatus{Name: "github", Err: "token rejected", Auth: true, At: now.Add(-time.Hour)}, "github ✗ token rejected"}, {"sso expired", resolver.AdapterStatus{Name: "aws prod-acct", Err: "operation error STS: GetCallerIdentity, token has expired", SSO: true}, "aws prod-acct ✗ sso expired"}, - {"throttled", resolver.AdapterStatus{Name: "aws stage-acct", Err: "ThrottlingException: Rate exceeded", Throttled: true, RetryAt: now.Add(2*time.Minute + 5*time.Second)}, "aws stage-acct ⏸ throttled 2m"}, - {"throttled, retry due", resolver.AdapterStatus{Name: "jira", Err: "HTTP 429", Throttled: true, RetryAt: now.Add(-time.Second)}, "jira ⏸ throttled"}, + {"throttled", resolver.AdapterStatus{Name: "aws stage-acct", Err: "ThrottlingException: Rate exceeded", Throttled: true, RetryAt: now.Add(2*time.Minute + 5*time.Second)}, "aws stage-acct ‖ throttled 3m"}, + {"throttled, countdown rounds up", resolver.AdapterStatus{Name: "jira", Throttled: true, RetryAt: now.Add(500 * time.Millisecond)}, "jira ‖ throttled 1s"}, + {"throttled 2m59s", resolver.AdapterStatus{Name: "jira", Throttled: true, RetryAt: now.Add(2*time.Minute + 59*time.Second)}, "jira ‖ throttled 3m"}, + {"throttled 59.5s", resolver.AdapterStatus{Name: "jira", Throttled: true, RetryAt: now.Add(59*time.Second + 500*time.Millisecond)}, "jira ‖ throttled 1m"}, + {"throttled exactly 1h", resolver.AdapterStatus{Name: "jira", Throttled: true, RetryAt: now.Add(time.Hour)}, "jira ‖ throttled 1h"}, + {"throttled 1h1s", resolver.AdapterStatus{Name: "jira", Throttled: true, RetryAt: now.Add(time.Hour + time.Second)}, "jira ‖ throttled 2h"}, + {"throttled, retry due", resolver.AdapterStatus{Name: "jira", Err: "HTTP 429", Throttled: true, RetryAt: now.Add(-time.Second)}, "jira ‖ throttled"}, {"auth beats throttled", resolver.AdapterStatus{Name: "jira", Err: "token rejected", Auth: true, Throttled: true, RetryAt: now.Add(time.Minute)}, "jira ✗ token rejected"}, {"sso beats throttled", resolver.AdapterStatus{Name: "aws prod-acct", Err: "expired", SSO: true, Throttled: true, RetryAt: now.Add(time.Minute)}, "aws prod-acct ✗ sso expired"}, {"generic error", resolver.AdapterStatus{Name: "jira", Err: "HTTP 502 Bad Gateway"}, "jira ✗ HTTP 502 Bad Gateway"}, @@ -56,7 +61,7 @@ func TestHeaderMixed(t *testing.T) { {Name: "aws stage-acct", Throttled: true, RetryAt: now.Add(2 * time.Minute)}, {Name: "aws prod-acct", SSO: true, Err: "expired"}, } - want := "app-green jira ✓ 12s github ✗ token rejected aws stage-acct ⏸ throttled 2m aws prod-acct ✗ sso expired" + want := "app-green jira ✓ 12s github ✗ token rejected aws stage-acct ‖ throttled 2m aws prod-acct ✗ sso expired" if got := ui.RenderHeader(st, now, 120); got != want { t.Errorf("header\n got %q\nwant %q", got, want) } diff --git a/internal/ui/hint.go b/internal/ui/hint.go index 96c6b3b..c81037d 100644 --- a/internal/ui/hint.go +++ b/internal/ui/hint.go @@ -18,24 +18,33 @@ const ( ActionStatus // t: change the Jira status ) -var actionKeys = [...]string{"", "f", "o", "a/x", "t"} +// actionUndecided is flagAction's answer for a FlagKind it has no case for; +// RowAction shows it as ActionNone, and a test fails on it. +const actionUndecided Action = -1 // Key is the hint shown at the end of the row, "" for ActionNone. func (a Action) Key() string { - if a < 0 || int(a) >= len(actionKeys) { - return "" + switch a { + case ActionRerun: + return "f" + case ActionOpen: + return "o" + case ActionApprove: + return "a/x" + case ActionStatus: + return "t" } - return actionKeys[a] + return "" } func (a Action) String() string { if a == ActionNone { return "none" } - if a < 0 || int(a) >= len(actionKeys) { - return fmt.Sprintf("Action(%d)", int(a)) + if k := a.Key(); k != "" { + return k } - return actionKeys[a] + return fmt.Sprintf("Action(%d)", int(a)) } // RowAction is the action a row offers for c.Flags[0], or ActionNone when the @@ -47,7 +56,7 @@ func RowAction(c model.Chain) Action { return ActionNone } a := flagAction(c.Flags[0]) - if a == ActionNone { + if a == ActionNone || a == actionUndecided { return ActionNone } first := target(c.Flags[0]) @@ -60,31 +69,47 @@ func RowAction(c model.Chain) Action { } // flagAction is the action f alone offers, ActionNone without a target. +// Every FlagKind has its own case; a kind added later without one gives +// actionUndecided. func flagAction(f model.Flag) Action { - switch f.Kind { - case model.FlagCheckFailed: - if f.PR != nil { - return failedCheckAction(f.PR.Failing) - } - case model.FlagPipelineFailed, model.FlagRolledBack, model.FlagDeployUnknown: + slot := func() Action { if f.Slot != nil { return ActionOpen } - case model.FlagStranded, model.FlagCheckExpected: + return ActionNone + } + pr := func() Action { if f.PR != nil { return ActionOpen } + return ActionNone + } + switch f.Kind { + case model.FlagPipelineFailed, model.FlagRolledBack, model.FlagUnhealthy, model.FlagPartialProd: + return slot() // open the pipeline + case model.FlagDeployUnknown: + // A repo no configured Env deploys has no Slot, so no key. + return slot() + case model.FlagStranded, model.FlagCheckExpected, model.FlagChangesRequested, model.FlagReadyToMerge, model.FlagStaleReview: + return pr() + case model.FlagCheckFailed: + if f.PR == nil { + return ActionNone + } + return failedCheckAction(f.PR) case model.FlagAwaitingApproval: - if s := f.Slot; s != nil { - if !s.Env.ReadOnly && s.Deploy != nil && s.Deploy.ApprovalToken != "" { - return ActionApprove - } - return ActionOpen // cannot approve here: open the pipeline console + s := f.Slot + if s == nil { + return ActionNone + } + if !s.Env.ReadOnly && s.Deploy != nil && s.Deploy.ApprovalToken != "" { + return ActionApprove } + return ActionOpen // cannot approve here: open the pipeline console case model.FlagStatusMismatch: return ActionStatus } - return ActionNone + return actionUndecided } // target names what f's action works on: its Env, its PR, or else the ticket. @@ -98,19 +123,30 @@ func target(f model.Flag) string { return "ticket" } -// failedCheckAction is f when any failing check is a re-runnable Actions run -// (the re-run acts on those; code scanning stays for the detail screen), o -// when any is code scanning or has a URL to open, else none. -func failedCheckAction(cs []model.Check) Action { - open := false - for _, c := range cs { +// failedCheckAction is f when any failing check of pr is a re-runnable +// Actions run (the re-run acts on those; code scanning stays for the detail +// screen). Otherwise it is o when there is a page to open: a failing check's +// own URL, else ChecksPage(pr). Code scanning and checks outside Actions +// follow the same rule. With nothing to open it is none. +func failedCheckAction(pr *model.PR) Action { + open := ChecksPage(*pr) != "" + for _, c := range pr.Failing { if c.RunID > 0 && !c.CodeScanning { return ActionRerun } - open = open || c.CodeScanning || c.URL != "" + open = open || c.URL != "" } if open { return ActionOpen } return ActionNone } + +// ChecksPage is the PR's checks tab, "" when the PR's URL is unknown. o opens +// it for a failing check that has no URL of its own. +func ChecksPage(pr model.PR) string { + if pr.URL == "" { + return "" + } + return strings.TrimSuffix(pr.URL, "/") + "/checks" +} diff --git a/internal/ui/hint_internal_test.go b/internal/ui/hint_internal_test.go new file mode 100644 index 0000000..98e0d90 --- /dev/null +++ b/internal/ui/hint_internal_test.go @@ -0,0 +1,27 @@ +package ui + +import ( + "fmt" + "testing" + + "github.com/ericdahl-dev/app-green/internal/model" +) + +// TestEveryFlagKindHasADecision fails when a FlagKind is added to model +// without an explicit case in flagAction. +func TestEveryFlagKindHasADecision(t *testing.T) { + n := 0 + for k := model.FlagKind(0); k.String() != fmt.Sprintf("FlagKind(%d)", int(k)); k++ { + n++ + f := model.Flag{Kind: k, PR: &model.PR{Number: 3}, Slot: &model.EnvSlot{}} + if flagAction(f) == actionUndecided { + t.Errorf("flagAction has no case for %v", k) + } + } + if n == 0 { + t.Fatal("found no flag kinds") + } + if flagAction(model.Flag{Kind: model.FlagKind(n)}) != actionUndecided { + t.Errorf("an unknown kind is not reported as undecided") + } +} diff --git a/internal/ui/hint_test.go b/internal/ui/hint_test.go index 6d5fd0f..0310386 100644 --- a/internal/ui/hint_test.go +++ b/internal/ui/hint_test.go @@ -13,7 +13,14 @@ func TestRowAction(t *testing.T) { external := model.Check{Name: "ci/external"} externalURL := model.Check{Name: "ci/external", URL: "https://ci.example.com/acme/app/1"} failingPR := func(cs ...model.Check) *model.PR { - return &model.PR{Number: 3, State: model.PROpen, Checks: model.ChecksFailing, Failing: cs} + return &model.PR{Number: 3, URL: "https://github.com/acme/app/pull/3", State: model.PROpen, Checks: model.ChecksFailing, Failing: cs} + } + // noURL is a failing PR whose URL is unknown, so only a check's own URL + // can be opened. + noURL := func(cs ...model.Check) *model.PR { + p := failingPR(cs...) + p.URL = "" + return p } waiting := func(readOnly bool, token string) *model.EnvSlot { return &model.EnvSlot{ @@ -37,8 +44,12 @@ func TestRowAction(t *testing.T) { {"code scanning beside a re-runnable check", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(rspec, codeql)}, ui.ActionRerun, "f"}, {"outside Actions beside a re-runnable check", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(external, rspec)}, ui.ActionRerun, "f"}, {"failed check outside Actions with a URL", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(externalURL)}, ui.ActionOpen, "o"}, - {"failed check outside Actions", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(external)}, ui.ActionNone, ""}, - {"failed check with no details", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR()}, ui.ActionNone, ""}, + {"failed check outside Actions, no URL: the PR's checks page", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR(external)}, ui.ActionOpen, "o"}, + {"failed check with no details: the PR's checks page", &model.Flag{Kind: model.FlagCheckFailed, PR: failingPR()}, ui.ActionOpen, "o"}, + {"failed check outside Actions, nothing to open", &model.Flag{Kind: model.FlagCheckFailed, PR: noURL(external)}, ui.ActionNone, ""}, + {"code scanning, nothing to open", &model.Flag{Kind: model.FlagCheckFailed, PR: noURL(codeql)}, ui.ActionNone, ""}, + {"code scanning with its own URL", &model.Flag{Kind: model.FlagCheckFailed, PR: noURL(model.Check{Name: "CodeQL", CodeScanning: true, URL: "https://github.com/acme/app/security/code-scanning/1"})}, ui.ActionOpen, "o"}, + {"outside Actions with its own URL, PR URL unknown", &model.Flag{Kind: model.FlagCheckFailed, PR: noURL(externalURL)}, ui.ActionOpen, "o"}, {"failed check without a PR", &model.Flag{Kind: model.FlagCheckFailed}, ui.ActionNone, ""}, {"pipeline failed", &model.Flag{Kind: model.FlagPipelineFailed, Slot: slot}, ui.ActionOpen, "o"}, {"rolled back", &model.Flag{Kind: model.FlagRolledBack, Slot: slot}, ui.ActionOpen, "o"}, @@ -53,11 +64,15 @@ func TestRowAction(t *testing.T) { {"awaiting approval, no deploy", &model.Flag{Kind: model.FlagAwaitingApproval, Slot: &model.EnvSlot{State: model.SlotAwaitingApproval}}, ui.ActionOpen, "o"}, {"awaiting approval without a slot", &model.Flag{Kind: model.FlagAwaitingApproval}, ui.ActionNone, ""}, {"status mismatch", &model.Flag{Kind: model.FlagStatusMismatch}, ui.ActionStatus, "t"}, - {"unhealthy", &model.Flag{Kind: model.FlagUnhealthy, Slot: slot}, ui.ActionNone, ""}, - {"changes requested", &model.Flag{Kind: model.FlagChangesRequested, PR: pr}, ui.ActionNone, ""}, - {"partial prod", &model.Flag{Kind: model.FlagPartialProd, Slot: slot}, ui.ActionNone, ""}, - {"ready to merge", &model.Flag{Kind: model.FlagReadyToMerge, PR: pr}, ui.ActionNone, ""}, - {"stale review", &model.Flag{Kind: model.FlagStaleReview, PR: pr}, ui.ActionNone, ""}, + {"unhealthy", &model.Flag{Kind: model.FlagUnhealthy, Slot: slot}, ui.ActionOpen, "o"}, + {"unhealthy without a slot", &model.Flag{Kind: model.FlagUnhealthy}, ui.ActionNone, ""}, + {"changes requested", &model.Flag{Kind: model.FlagChangesRequested, PR: pr}, ui.ActionOpen, "o"}, + {"changes requested without a PR", &model.Flag{Kind: model.FlagChangesRequested}, ui.ActionNone, ""}, + {"partial prod", &model.Flag{Kind: model.FlagPartialProd, Slot: slot}, ui.ActionOpen, "o"}, + {"partial prod without a slot", &model.Flag{Kind: model.FlagPartialProd}, ui.ActionNone, ""}, + {"ready to merge", &model.Flag{Kind: model.FlagReadyToMerge, PR: pr}, ui.ActionOpen, "o"}, + {"stale review", &model.Flag{Kind: model.FlagStaleReview, PR: pr}, ui.ActionOpen, "o"}, + {"stale review without a PR", &model.Flag{Kind: model.FlagStaleReview}, ui.ActionNone, ""}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -137,3 +152,15 @@ func TestRowActionNeedsOneTarget(t *testing.T) { }) } } + +func TestChecksPage(t *testing.T) { + for in, want := range map[string]string{ + "https://github.com/acme/app/pull/3": "https://github.com/acme/app/pull/3/checks", + "https://github.com/acme/app/pull/3/": "https://github.com/acme/app/pull/3/checks", + "": "", + } { + if got := ui.ChecksPage(model.PR{URL: in}); got != want { + t.Errorf("ChecksPage(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/internal/ui/row.go b/internal/ui/row.go index a07a218..bfb978f 100644 --- a/internal/ui/row.go +++ b/internal/ui/row.go @@ -2,13 +2,16 @@ package ui import ( "fmt" + "strings" "time" "github.com/charmbracelet/x/ansi" "github.com/ericdahl-dev/app-green/internal/model" ) -// Layout is the column widths shared by every row of the list. +// Layout is the column widths shared by every row of the list. Build it +// again with NewLayout for every render: the stage column holds an age +// ("in prod ✓ 3h ago") whose width changes as time passes. type Layout struct { Width int // the whole row, in cells KeyWidth int // the longest ticket key @@ -44,7 +47,9 @@ func RenderRow(c model.Chain, l Layout, now time.Time) string { } // Row renders c as one row exactly l.Width cells wide: marker, key, title, -// stage, reason and key hint. +// stage, reason and key hint. When the row is too narrow, the reason and +// title give way first, then the stage and key are cut; the marker and the +// hint keep their places down to 7 cells. func (s Styles) Row(c model.Chain, l Layout, now time.Time) string { if l.Width <= 0 { return "" @@ -60,10 +65,16 @@ func (s Styles) Row(c model.Chain, l Layout, now time.Time) string { } else { reasonW += 2 // no title column, so no gap after it } - rest := " " + fit(clean(c.Ticket.Key), l.KeyWidth) + " " + title + - fit(stageColumn(c, now), l.StageWidth) + " " + fit(reason, reasonW) + " " + fit(RowAction(c).Key(), hintWidth) - // Too narrow for every column: cut at the right edge, before styling. - rest = fit(rest, l.Width-1) + mid := " " + fit(clean(c.Ticket.Key), l.KeyWidth) + " " + title + + fit(stageColumn(c, now), l.StageWidth) + " " + fit(reason, reasonW) + tail := " " + fit(RowAction(c).Key(), hintWidth) + var rest string + if avail := l.Width - 1 - ansi.StringWidth(tail); avail >= 1 { + // Trim the padding first so a cut ends in text, not " …". + rest = fit(strings.TrimRight(mid, " "), avail) + tail + } else { + rest = fit(mid+tail, l.Width-1) // too narrow for the hint + } if c.Stale { // Dim the marker and the rest separately: wrapping the colored marker // in a faint style would let its reset end the dimming. @@ -103,7 +114,7 @@ func StageText(c model.Chain, now time.Time) string { case model.StageInTest: return "test ✓" case model.StageAwaitingProd: - return "prod ⏸" + return "prod ‖" case model.StageInProd: if at := lastProd(c); !at.IsZero() { return "in prod ✓ " + age(now.Sub(at)) + " ago" diff --git a/internal/ui/row_test.go b/internal/ui/row_test.go index 7b37e01..1bde804 100644 --- a/internal/ui/row_test.go +++ b/internal/ui/row_test.go @@ -76,7 +76,7 @@ func TestStageText(t *testing.T) { {"PR open with no open PR", model.Chain{Stage: model.StagePROpen}, "PR open"}, {"merged", model.Chain{Stage: model.StageMerged}, "merged"}, {"in test", model.Chain{Stage: model.StageInTest}, "test ✓"}, - {"awaiting prod", model.Chain{Stage: model.StageAwaitingProd}, "prod ⏸"}, + {"awaiting prod", model.Chain{Stage: model.StageAwaitingProd}, "prod ‖"}, {"in prod, latest prod deploy", model.Chain{Stage: model.StageInProd, Slots: []model.EnvSlot{ testSlot, prodSlot(now.Add(-5*time.Hour), model.SlotDeployed, true), @@ -110,7 +110,7 @@ func TestRowYellowAwaitingApproval(t *testing.T) { Stage: model.StageAwaitingProd, } c.Flags = []model.Flag{{Level: model.Yellow, Kind: model.FlagAwaitingApproval, Reason: "prod-acct Production awaiting approval", Slot: &c.Slots[0]}} - want := "◐ ABC-22 Form opt-outs prod ⏸ prod-acct Production awaitin… a/x" + want := "◐ ABC-22 Form opt-outs prod ‖ prod-acct Production awaitin… a/x" if got := row(t, c, layout); got != want { t.Errorf("row\n got %q\nwant %q", got, want) } @@ -152,11 +152,10 @@ func TestRowStaleHasTildePrefix(t *testing.T) { } func TestRowStaleIsDimmed(t *testing.T) { - lipgloss.SetColorProfile(termenv.ANSI) - defer lipgloss.SetColorProfile(termenv.Ascii) + color := terminal(env{"TERM": "xterm-256color"}) c := model.Chain{Ticket: ticket("ABC-6", "Dim me"), Stage: model.StageMerged, Stale: true} c.Flags = []model.Flag{{Level: model.Red, Kind: model.FlagStatusMismatch, Reason: "Jira still In Progress, PR merged"}} - got := ui.RenderRow(c, layout, now) + got := color.Row(c, layout, now) // The marker is red and faint; everything after it is one faint run, so a // reset inside the row cannot end the dimming early. marker, rest, ok := strings.Cut(got, "●\x1b[0m") @@ -169,7 +168,7 @@ func TestRowStaleIsDimmed(t *testing.T) { } c.Stale = false - if got := ui.RenderRow(c, layout, now); strings.Contains(got, "\x1b[2m") || strings.Contains(got, ";2m") { + if got := color.Row(c, layout, now); strings.Contains(got, "\x1b[2m") || strings.Contains(got, ";2m") { t.Errorf("a fresh row is dimmed: %q", got) } } @@ -217,9 +216,15 @@ func TestRowsAtEveryWidth(t *testing.T) { for w := 0; w <= 120; w++ { l := ui.NewLayout(chains, now, w) for _, c := range chains { - if got := ui.RenderRow(c, l, now); ansi.StringWidth(got) != w { + got := ui.RenderRow(c, l, now) + if ansi.StringWidth(got) != w { t.Fatalf("width %d: row is %d cells: %q", w, ansi.StringWidth(got), got) } + // From the marker, a cell of text and the hint column up, the + // hint always shows. + if k := ui.RowAction(c).Key(); k != "" && w >= 1+1+2+3 && !strings.HasSuffix(strings.TrimRight(got, " "), k) { + t.Errorf("width %d: hint %q is cut: %q", w, k, got) + } } } } @@ -242,7 +247,14 @@ func TestRowsAtBoardWidth(t *testing.T) { func TestRowNarrowCutsAtTheRightEdge(t *testing.T) { chains := board() l := ui.NewLayout(chains, now, 30) - want := "● ABC-1 PR #3 …" + // The hint stays; the stage keeps its place and the reason is dropped. + want := "● ABC-1 PR #3" + strings.Repeat(" ", 8+2) + "f " + if got := ui.RenderRow(chains[0], l, now); got != want { + t.Errorf("row\n got %q\nwant %q", got, want) + } + // Narrower still: the key and stage are cut, never the hint. + l = ui.NewLayout(chains, now, 12) + want = "● ABC-… f " if got := ui.RenderRow(chains[0], l, now); got != want { t.Errorf("row\n got %q\nwant %q", got, want) } From 0fa682a72f50fe4c61c722dd3583ea284ffa966e Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 10:54:25 -0400 Subject: [PATCH 05/21] feat(ui): ticket list screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit App is the top Bubble Tea model. It waits on the resolver's snapshot channel (waitForSnapshot, adapted from jira-green), shows the header, one row per chain, then an Unlinked group, and a footer key line with a dimmed warnings count. - loading… before the first snapshot, nothing in flight when empty. - up/k and down/j move over chains and unlinked PRs, never onto the Unlinked header. The cursor follows the same ticket (or repo#number) when a new snapshot reorders the rows, else keeps its index. - The cursor row is reverse video, turned back on after each reset in the row. Without escape codes (NO_COLOR, no color terminal) its marker cell shows ">" instead. - r sends on the refresh channel without blocking; q and ctrl+c quit. - o opens OpenTarget for a chain (the pipeline, the failing check or checks tab, or the PR when the row offers o, else the ticket) and the PR for an unlinked line. A failed open shows in the footer until the next key. - The body scrolls to keep the cursor in view; header and footer stay. - Adds bubbletea v1.3.10 and bubbles v0.21.1 (jira-green's versions). Assisted-by: Claude Code:claude-opus-5-5 --- go.mod | 7 + go.sum | 15 ++ internal/ui/app.go | 161 ++++++++++++ internal/ui/list.go | 155 ++++++++++++ internal/ui/list_test.go | 531 +++++++++++++++++++++++++++++++++++++++ internal/ui/styles.go | 21 ++ 6 files changed, 890 insertions(+) create mode 100644 internal/ui/app.go create mode 100644 internal/ui/list.go create mode 100644 internal/ui/list_test.go diff --git a/go.mod b/go.mod index cb61bdf..2f12ac1 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,8 @@ require ( github.com/aws/aws-sdk-go-v2/service/codepipeline v1.40.1 github.com/aws/aws-sdk-go-v2/service/ecs v1.79.1 github.com/aws/smithy-go v1.25.1 + github.com/charmbracelet/bubbles v0.21.1 + github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 github.com/charmbracelet/x/ansi v0.11.5 github.com/muesli/termenv v0.16.0 @@ -34,10 +36,15 @@ require ( github.com/clipperhouse/displaywidth v0.9.0 // indirect github.com/clipperhouse/stringish v0.1.1 // indirect github.com/clipperhouse/uax29/v2 v2.5.0 // indirect + github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/lucasb-eyer/go-colorful v1.3.0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect + github.com/mattn/go-localereader v0.0.1 // indirect github.com/mattn/go-runewidth v0.0.19 // indirect + github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect + github.com/muesli/cancelreader v0.2.2 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect golang.org/x/sys v0.38.0 // indirect + golang.org/x/text v0.3.8 // indirect ) diff --git a/go.sum b/go.sum index 318412f..4f9919d 100644 --- a/go.sum +++ b/go.sum @@ -32,6 +32,10 @@ github.com/aws/smithy-go v1.25.1 h1:J8ERsGSU7d+aCmdQur5Txg6bVoYelvQJgtZehD12GkI= github.com/aws/smithy-go v1.25.1/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= +github.com/charmbracelet/bubbles v0.21.1 h1:nj0decPiixaZeL9diI4uzzQTkkz1kYY8+jgzCZXSmW0= +github.com/charmbracelet/bubbles v0.21.1/go.mod h1:HHvIYRCpbkCJw2yo0vNX1O5loCwSr9/mWS8GYSg50Sk= +github.com/charmbracelet/bubbletea v1.3.10 h1:otUDHWMMzQSB0Pkc87rm691KZ3SWa4KUlvF9nRvCICw= +github.com/charmbracelet/bubbletea v1.3.10/go.mod h1:ORQfo0fk8U+po9VaNvnV95UPWA1BitP1E0N6xJPlHr4= github.com/charmbracelet/colorprofile v0.4.1 h1:a1lO03qTrSIRaK8c3JRxJDZOvhvIeSco3ej+ngLk1kk= github.com/charmbracelet/colorprofile v0.4.1/go.mod h1:U1d9Dljmdf9DLegaJ0nGZNJvoXAhayhmidOdcBwAvKk= github.com/charmbracelet/lipgloss v1.1.0 h1:vYXsiLHVkK7fp74RkV7b2kq9+zDLoEU4MZoFqR/noCY= @@ -48,12 +52,20 @@ github.com/clipperhouse/stringish v0.1.1 h1:+NSqMOr3GR6k1FdRhhnXrLfztGzuG+VuFDfa github.com/clipperhouse/stringish v0.1.1/go.mod h1:v/WhFtE1q0ovMta2+m+UbpZ+2/HEXNWYXQgCt4hdOzA= github.com/clipperhouse/uax29/v2 v2.5.0 h1:x7T0T4eTHDONxFJsL94uKNKPHrclyFI0lm7+w94cO8U= github.com/clipperhouse/uax29/v2 v2.5.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsVRgg6W7ihQeh4g= +github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= +github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/lucasb-eyer/go-colorful v1.3.0 h1:2/yBRLdWBZKrf7gB40FoiKfAWYQ0lqNcbuQwVHXptag= github.com/lucasb-eyer/go-colorful v1.3.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4= +github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88= github.com/mattn/go-runewidth v0.0.19 h1:v++JhqYnZuu5jSKrk9RbgF5v4CGUjqRfBm05byFGLdw= github.com/mattn/go-runewidth v0.0.19/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs= +github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 h1:ZK8zHtRHOkbHy6Mmr5D264iyp3TiX5OmNcI5cIARiQI= +github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6/go.mod h1:CJlz5H+gyd6CUWT45Oy4q24RdLyn7Md9Vj2/ldJBSIo= +github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= +github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ= @@ -62,6 +74,9 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavM github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI= golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo= +golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/text v0.3.8 h1:nAL+RVCQ9uMn3vJZbV+MRnydTJFPf8qqY42YiA6MrqY= +golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= diff --git a/internal/ui/app.go b/internal/ui/app.go new file mode 100644 index 0000000..403d9c1 --- /dev/null +++ b/internal/ui/app.go @@ -0,0 +1,161 @@ +package ui + +import ( + "fmt" + "strings" + "time" + + "github.com/charmbracelet/bubbles/key" + tea "github.com/charmbracelet/bubbletea" + "github.com/ericdahl-dev/app-green/internal/resolver" +) + +// App is the top Bubble Tea model: the ticket list, fed by the resolver's +// snapshots. +type App struct { + styles Styles + now func() time.Time + snaps <-chan resolver.Snapshot + refresh chan<- struct{} + openURL func(string) error + + snap *resolver.Snapshot // nil until the first one arrives + cursor int // index into items() + err string // the last open's error, until the next key + offset int // the first body line shown + width, height int +} + +// NewApp builds the App. snaps is the resolver's out channel, refresh its +// refresh channel (sent without blocking), and openURL opens a page in the +// browser. +func NewApp(snaps <-chan resolver.Snapshot, refresh chan<- struct{}, openURL func(string) error) App { + return App{styles: DefaultStyles(), now: time.Now, snaps: snaps, refresh: refresh, openURL: openURL} +} + +// WithStyles is m rendering with s. +func (m App) WithStyles(s Styles) App { m.styles = s; return m } + +// WithClock is m reading the time from now, for header and stage ages. +func (m App) WithClock(now func() time.Time) App { m.now = now; return m } + +// waitForSnapshot delivers the resolver's next snapshot. When the resolver +// has stopped the channel is closed and it delivers nothing, so the list +// keeps its last state. +// Adapted from jira-green main.go waitForSnapshot. +func waitForSnapshot(ch <-chan resolver.Snapshot) tea.Cmd { + return func() tea.Msg { + snap, ok := <-ch + if !ok { + return nil + } + return snap + } +} + +func (m App) Init() tea.Cmd { return waitForSnapshot(m.snaps) } + +func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + m, cmd := m.update(msg) + return m.scroll(), cmd +} + +func (m App) update(msg tea.Msg) (App, tea.Cmd) { + switch msg := msg.(type) { + case tea.WindowSizeMsg: + m.width, m.height = msg.Width, msg.Height + case resolver.Snapshot: + id := m.itemID(m.cursor) + m.snap = &msg + m.cursor = m.find(id) + return m, waitForSnapshot(m.snaps) + case tea.KeyMsg: + return m.key(msg) + } + return m, nil +} + +// keys are the list's key bindings. +var keys = struct { + up, down, refresh, quit, open key.Binding +}{ + up: key.NewBinding(key.WithKeys("up", "k")), + down: key.NewBinding(key.WithKeys("down", "j")), + refresh: key.NewBinding(key.WithKeys("r")), + open: key.NewBinding(key.WithKeys("o")), + quit: key.NewBinding(key.WithKeys("q", "ctrl+c")), +} + +func (m App) key(msg tea.KeyMsg) (App, tea.Cmd) { + m.err = "" + switch { + case key.Matches(msg, keys.quit): + return m, tea.Quit + case key.Matches(msg, keys.up): + m.cursor = max(m.cursor-1, 0) + case key.Matches(msg, keys.down): + m.cursor = min(m.cursor+1, max(m.items()-1, 0)) + case key.Matches(msg, keys.refresh): + select { + case m.refresh <- struct{}{}: + default: // a refresh is already pending + } + case key.Matches(msg, keys.open): + if u := m.target(); u != "" { + if err := m.openURL(u); err != nil { + m.err = "open failed: " + err.Error() + } + } + } + return m, nil +} + +// items is how many lines the cursor can select: every chain, then every +// unlinked PR. +func (m App) items() int { + if m.snap == nil { + return 0 + } + return len(m.snap.Chains) + len(m.snap.Unlinked) +} + +// itemID names item i so the cursor can find it in the next snapshot: the +// ticket key for a chain, "repo#number" (lower case) for an unlinked PR, "" +// for no item. +func (m App) itemID(i int) string { + switch { + case m.snap == nil || i < 0: + return "" + case i < len(m.snap.Chains): + return "ticket " + m.snap.Chains[i].Ticket.Key + case i < m.items(): + p := m.snap.Unlinked[i-len(m.snap.Chains)] + return fmt.Sprintf("pr %s#%d", strings.ToLower(p.Repo), p.Number) + } + return "" +} + +// find is the index of the item named id, or else the cursor clamped to the +// items there are. +func (m App) find(id string) int { + for i := range m.items() { + if id != "" && m.itemID(i) == id { + return i + } + } + return max(min(m.cursor, m.items()-1), 0) +} + +// target is the page o opens for the item under the cursor: OpenTarget for +// a chain, the PR for an unlinked PR. +func (m App) target() string { + switch i := m.cursor; { + case m.snap == nil: + return "" + case i < len(m.snap.Chains): + return OpenTarget(m.snap.Chains[i], resolver.PipelineURL) + case i < m.items(): + return m.snap.Unlinked[i-len(m.snap.Chains)].URL + } + return "" +} diff --git a/internal/ui/list.go b/internal/ui/list.go new file mode 100644 index 0000000..c2abcd7 --- /dev/null +++ b/internal/ui/list.go @@ -0,0 +1,155 @@ +package ui + +import ( + "fmt" + "strings" + + "github.com/charmbracelet/x/ansi" + "github.com/ericdahl-dev/app-green/internal/model" + "github.com/ericdahl-dev/app-green/internal/resolver" +) + +// defaultWidth is the list width before the first WindowSizeMsg. +const defaultWidth = 80 + +// footerKeys is the key line at the bottom of the list. +const footerKeys = "↑/↓ move enter details f re-run a/x approve/reject t status o open r refresh q quit" + +// View is the header, the rows (chains, then the Unlinked group), and the +// footer. When the body is taller than the screen it scrolls to keep the +// cursor in view; the header and footer stay put. +func (m App) View() string { + width := m.width + if width <= 0 { + width = defaultWidth + } + now := m.now() + var status []resolver.AdapterStatus + var body []string + switch { + case m.snap == nil: + body = append(body, "loading…") + case len(m.snap.Chains) == 0 && len(m.snap.Unlinked) == 0: + status = m.snap.Status + body = append(body, "nothing in flight") + default: + status = m.snap.Status + l := NewLayout(m.snap.Chains, now, width) + for i, c := range m.snap.Chains { + body = append(body, m.mark(i, m.styles.Row(c, l, now))) + } + if n := len(m.snap.Unlinked); n > 0 { + body = append(body, ansi.Truncate(fmt.Sprintf("── Unlinked (%d) ──", n), width, "…")) + for i, p := range m.snap.Unlinked { + // Exactly width cells, so the cursor's reverse video spans it. + line := " " + fit(fmt.Sprintf(" %s#%d %s", clean(p.Repo), p.Number, clean(p.Title)), width-1) + body = append(body, m.mark(len(m.snap.Chains)+i, line)) + } + } + } + if budget := m.bodyHeight(); budget >= 0 && len(body) > budget { + body = body[m.offset:min(m.offset+budget, len(body))] + } + lines := append([]string{m.styles.Header(status, now, width)}, body...) + return strings.Join(append(lines, m.footer(width)), "\n") +} + +// mark is line, selected when item i is under the cursor. +func (m App) mark(i int, line string) string { + if i == m.cursor { + return m.styles.selected(line) + } + return line +} + +// OpenTarget is the page o opens for c's row. When the row offers o, it is +// what Flags[0] points at: the pipeline console (pipelineURL) for an Env, a +// failing check's own page (else the PR's checks tab) for a failed check, or +// the PR. Otherwise, or when that page is unknown, it is the ticket. It is +// "" only when the ticket's URL is unknown too. +func OpenTarget(c model.Chain, pipelineURL func(model.Env) string) string { + if RowAction(c) != ActionOpen { + return c.Ticket.URL + } + f := c.Flags[0] + var u string + switch { + case f.Slot != nil: + u = pipelineURL(f.Slot.Env) + case f.PR != nil && f.Kind == model.FlagCheckFailed: + u = ChecksPage(*f.PR) + for _, ch := range f.PR.Failing { + if ch.URL != "" { + u = ch.URL + break + } + } + case f.PR != nil: + u = f.PR.URL + } + if u == "" { + return c.Ticket.URL + } + return u +} + +// footer is the key line, then the open error and the warnings count, if +// any, cut to width. The +// key line gives way first. +func (m App) footer(width int) string { + var extra []string + if m.err != "" { + extra = append(extra, m.styles.red.Render(clean(m.err))) + } + if m.snap != nil && len(m.snap.Warnings) > 0 { + n := len(m.snap.Warnings) + extra = append(extra, m.styles.dim.Render(fmt.Sprintf("%d %s", n, plural(n, "warning")))) + } + if len(extra) == 0 { + return ansi.Truncate(footerKeys, width, "…") + } + tail := " " + strings.Join(extra, " ") + keys := ansi.Truncate(footerKeys, max(width-ansi.StringWidth(tail), 0), "…") + return ansi.Truncate(keys+tail, width, "…") +} + +// plural is word, with an s unless n is 1. +func plural(n int, word string) string { + if n == 1 { + return word + } + return word + "s" +} + +// bodyHeight is how many body lines fit between the header and the footer, +// -1 when the height is not known yet (everything is shown). +func (m App) bodyHeight() int { + if m.height <= 0 { + return -1 + } + return max(m.height-2, 1) +} + +// scroll moves the window of body lines just enough to show the cursor. +func (m App) scroll() App { + budget := m.bodyHeight() + if budget < 0 || m.snap == nil { + m.offset = 0 + return m + } + lines, line := len(m.snap.Chains), m.cursor + if n := len(m.snap.Unlinked); n > 0 { + lines += 1 + n + if m.cursor >= len(m.snap.Chains) { + line++ // past the Unlinked header + } + } + if line < m.offset { + m.offset = line + } + if line >= m.offset+budget { + m.offset = line - budget + 1 + } + m.offset = min(max(m.offset, 0), max(lines-budget, 0)) + return m +} diff --git a/internal/ui/list_test.go b/internal/ui/list_test.go new file mode 100644 index 0000000..a51929c --- /dev/null +++ b/internal/ui/list_test.go @@ -0,0 +1,531 @@ +package ui_test + +import ( + "errors" + "fmt" + "strings" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/x/ansi" + "github.com/ericdahl-dev/app-green/internal/model" + "github.com/ericdahl-dev/app-green/internal/resolver" + "github.com/ericdahl-dev/app-green/internal/ui" +) + +// newApp is an App on the default (ASCII) styles and the fixed clock, sized +// 100x30, with no snapshot yet. +func newApp(t *testing.T) ui.App { + t.Helper() + m := ui.NewApp(make(chan resolver.Snapshot), make(chan struct{}, 1), noOpen(t)). + WithClock(func() time.Time { return now }) + return update(t, m, tea.WindowSizeMsg{Width: 100, Height: 30}) +} + +// noOpen fails the test if the app tries to open a browser. +func noOpen(t *testing.T) func(string) error { + return func(u string) error { t.Errorf("unexpected open %q", u); return nil } +} + +// update sends msg and returns the App, dropping the command. +func update(t *testing.T, m ui.App, msg tea.Msg) ui.App { + t.Helper() + got, _ := m.Update(msg) + app, ok := got.(ui.App) + if !ok { + t.Fatalf("Update returned %T, want ui.App", got) + } + return app +} + +func viewLines(m ui.App) []string { return strings.Split(m.View(), "\n") } + +func TestAppLoading(t *testing.T) { + m := newApp(t) + if v := m.View(); !strings.Contains(v, "loading…") { + t.Errorf("before the first snapshot the view says loading…:\n%s", v) + } +} + +func TestAppNothingInFlight(t *testing.T) { + m := update(t, newApp(t), resolver.Snapshot{At: now}) + v := m.View() + if !strings.Contains(v, "nothing in flight") || strings.Contains(v, "loading…") { + t.Errorf("an empty snapshot says nothing in flight:\n%s", v) + } +} + +// footerKeys is the footer's key line. +const footerKeys = "↑/↓ move enter details f re-run a/x approve/reject t status o open r refresh q quit" + +// fxSnapshot has three chains (a failing check, an approval to open, a plain +// ticket) and one unlinked PR. +func fxSnapshot() resolver.Snapshot { + failing := model.Chain{ + Ticket: model.Ticket{Key: "ABC-1", Title: "Fix login", URL: "https://jira.example.com/browse/ABC-1"}, + PRs: []model.PR{{Repo: "acme/app", Number: 3, State: model.PROpen, URL: "https://github.com/acme/app/pull/3", + Checks: model.ChecksFailing, Failing: []model.Check{{Name: "lint", URL: "https://ci.example.com/lint/1"}}}}, + Stage: model.StagePROpen, + } + failing.Flags = []model.Flag{{Level: model.Red, Kind: model.FlagCheckFailed, Reason: "PR #3 lint failing", PR: &failing.PRs[0]}} + env := model.Env{Account: "prod-acct", Region: "us-east-1", Pipeline: "app-pipeline", Stage: "Production", Prod: true, ReadOnly: true} + waiting := model.Chain{ + Ticket: model.Ticket{Key: "ABC-2", Title: "Add search", URL: "https://jira.example.com/browse/ABC-2"}, + Slots: []model.EnvSlot{{Env: env, State: model.SlotAwaitingApproval, Applies: true}}, + Stage: model.StageAwaitingProd, + } + waiting.Flags = []model.Flag{{Level: model.Yellow, Kind: model.FlagAwaitingApproval, Reason: "prod-acct waiting for approval", Slot: &waiting.Slots[0]}} + plain := model.Chain{ + Ticket: model.Ticket{Key: "ABC-3", Title: "Tidy docs", URL: "https://jira.example.com/browse/ABC-3"}, + Stage: model.StageStarted, + } + return resolver.Snapshot{ + Chains: []model.Chain{failing, waiting, plain}, + Unlinked: []model.PR{{Repo: "acme/app", Number: 9, Title: "Bump deps", URL: "https://github.com/acme/app/pull/9", State: model.PROpen}}, + Status: []resolver.AdapterStatus{{Name: "jira", OK: true, At: now.Add(-12 * time.Second)}, {Name: "github", OK: true}}, + At: now, + } +} + +func TestAppRows(t *testing.T) { + snap := fxSnapshot() + m := update(t, newApp(t), snap) + ls := viewLines(m) + l := ui.NewLayout(snap.Chains, now, 100) + want := []string{ + ui.RenderHeader(snap.Status, now, 100), + ">" + ansi.TruncateLeft(ui.RenderRow(snap.Chains[0], l, now), 1, ""), // the cursor row + ui.RenderRow(snap.Chains[1], l, now), + ui.RenderRow(snap.Chains[2], l, now), + "── Unlinked (1) ──", + " acme/app#9 Bump deps", + } + if len(ls) < len(want)+1 { + t.Fatalf("view has %d lines, want at least %d:\n%s", len(ls), len(want)+1, m.View()) + } + for i, w := range want { + if got := strings.TrimRight(ls[i], " "); got != strings.TrimRight(w, " ") { + t.Errorf("line %d\n got %q\nwant %q", i, got, w) + } + } + if got := ls[len(ls)-1]; got != footerKeys { + t.Errorf("footer\n got %q\nwant %q", got, footerKeys) + } +} + +func key(s string) tea.KeyMsg { + switch s { + case "down": + return tea.KeyMsg{Type: tea.KeyDown} + case "up": + return tea.KeyMsg{Type: tea.KeyUp} + case "enter": + return tea.KeyMsg{Type: tea.KeyEnter} + case "ctrl+c": + return tea.KeyMsg{Type: tea.KeyCtrlC} + } + return tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(s)} +} + +func press(t *testing.T, m ui.App, keys ...string) ui.App { + t.Helper() + for _, k := range keys { + m = update(t, m, key(k)) + } + return m +} + +// cursorLine is the line the ASCII cursor (">" in the first cell) is on, +// failing unless exactly one line has it. +func cursorLine(t *testing.T, m ui.App) string { + t.Helper() + var found []string + for _, l := range viewLines(m) { + if strings.HasPrefix(l, ">") { + found = append(found, l) + } + } + if len(found) != 1 { + t.Fatalf("want one cursor line, got %d:\n%s", len(found), m.View()) + } + return found[0] +} + +func TestAppCursorMoves(t *testing.T) { + m := update(t, newApp(t), fxSnapshot()) + steps := []struct { + key string + want string // text on the cursor line after the key + }{ + {"", "ABC-1"}, + {"up", "ABC-1"}, // stays at the top + {"j", "ABC-2"}, + {"down", "ABC-3"}, + {"j", "acme/app#9"}, // skips the Unlinked header + {"down", "acme/app#9"}, // stays at the bottom + {"k", "ABC-3"}, // skips the header going up too + {"up", "ABC-2"}, + } + for _, s := range steps { + if s.key != "" { + m = press(t, m, s.key) + } + if got := cursorLine(t, m); !strings.Contains(got, s.want) { + t.Errorf("after %q the cursor is on %q, want %s", s.key, got, s.want) + } + } +} + +func TestAppCursorReverseVideo(t *testing.T) { + snap := fxSnapshot() + snap.Chains[0].Stale = true // a red marker and dimmed text: two resets mid-row + styles := terminal(env{"TERM": "xterm-256color"}) + m := update(t, newApp(t).WithStyles(styles), snap) + ls := viewLines(m) + got, plain := ls[1], styles.Row(snap.Chains[0], ui.NewLayout(snap.Chains, now, 100), now) + + const rev, reset = "\x1b[7m", "\x1b[0m" + if !strings.HasPrefix(got, rev) || !strings.HasSuffix(got, reset) { + t.Fatalf("the cursor row is wrapped in reverse video: %q", got) + } + inner := strings.TrimSuffix(got, reset) + if strings.Count(inner, reset) == 0 || strings.Count(inner, reset) != strings.Count(inner, reset+rev) { + t.Errorf("every reset inside the cursor row turns reverse back on: %q", got) + } + if ansi.Strip(got) != ansi.Strip(plain) { + t.Errorf("the cursor row keeps its text\n got %q\nwant %q", ansi.Strip(got), ansi.Strip(plain)) + } + if strings.Contains(ls[2], rev) { + t.Errorf("only the cursor row is reversed: %q", ls[2]) + } +} + +func TestAppCursorFollowsTicket(t *testing.T) { + snap := fxSnapshot() + m := press(t, update(t, newApp(t), snap), "j") // on ABC-2 + + reordered := fxSnapshot() + c := reordered.Chains + reordered.Chains = []model.Chain{c[2], c[0], c[1]} + m = update(t, m, reordered) + if got := cursorLine(t, m); !strings.Contains(got, "ABC-2") { + t.Errorf("a reordered snapshot keeps the cursor on ABC-2: %q", got) + } + + // The selected ticket leaves: the cursor stays at its index, clamped. + gone := fxSnapshot() + gone.Chains = []model.Chain{c[2], c[0]} + gone.Unlinked = nil + m = update(t, m, gone) + if got := cursorLine(t, m); !strings.Contains(got, "ABC-1") { + t.Errorf("with ABC-2 gone the cursor clamps to the last row: %q", got) + } + + // An unlinked PR is followed by repo and number, ignoring case. + m = press(t, update(t, m, fxSnapshot()), "j", "j", "j") + moved := fxSnapshot() + moved.Unlinked = append([]model.PR{{Repo: "acme/web", Number: 1, Title: "New"}}, moved.Unlinked...) + moved.Unlinked[1].Repo = "Acme/App" + m = update(t, m, moved) + if got := cursorLine(t, m); !strings.Contains(got, "Acme/App#9") { + t.Errorf("the cursor stays on the unlinked PR: %q", got) + } +} + +func TestAppRefresh(t *testing.T) { + refresh := make(chan struct{}, 1) + m := ui.NewApp(make(chan resolver.Snapshot), refresh, noOpen(t)).WithClock(func() time.Time { return now }) + m = press(t, m, "r") + if len(refresh) != 1 { + t.Fatalf("r sends on the refresh channel") + } + done := make(chan struct{}) + go func() { + defer close(done) + press(t, m, "r") // the buffer is full: this must not block + }() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("r blocked on a full refresh channel") + } + if len(refresh) != 1 { + t.Errorf("a press while one is pending is coalesced") + } +} + +func TestAppQuit(t *testing.T) { + for _, k := range []string{"q", "ctrl+c"} { + _, cmd := newApp(t).Update(key(k)) + if cmd == nil { + t.Fatalf("%s returns no command", k) + } + if _, ok := cmd().(tea.QuitMsg); !ok { + t.Errorf("%s does not quit", k) + } + } +} + +func fakePipelineURL(e model.Env) string { return "pipeline:" + e.ID() } + +func TestOpenTarget(t *testing.T) { + const ticketURL = "https://jira.example.com/browse/ABC-1" + pr := func(url string, failing ...model.Check) *model.PR { + return &model.PR{Repo: "acme/app", Number: 3, URL: url, State: model.PROpen, Failing: failing} + } + env := model.Env{Account: "prod-acct", Pipeline: "app-pipeline", Stage: "Production", Prod: true} + slot := func(readOnly bool, token string) *model.EnvSlot { + e := env + e.ReadOnly = readOnly + return &model.EnvSlot{Env: e, State: model.SlotAwaitingApproval, Deploy: &model.Deploy{ApprovalToken: token}} + } + other := model.Env{Account: "stage-acct", Pipeline: "app-pipeline", Stage: "Test"} + tests := []struct { + name string + flags []model.Flag + want string + }{ + {"no flags opens the ticket", nil, ticketURL}, + {"failing check opens its own page", + []model.Flag{{Kind: model.FlagCheckFailed, PR: pr("https://github.com/acme/app/pull/3", + model.Check{Name: "scan", CodeScanning: true}, model.Check{Name: "lint", URL: "https://ci.example.com/lint"})}}, + "https://ci.example.com/lint"}, + {"failing check without a URL opens the checks tab", + []model.Flag{{Kind: model.FlagCheckFailed, PR: pr("https://github.com/acme/app/pull/3", model.Check{Name: "lint"})}}, + "https://github.com/acme/app/pull/3/checks"}, + {"re-runnable check offers f, so o opens the ticket", + []model.Flag{{Kind: model.FlagCheckFailed, PR: pr("https://github.com/acme/app/pull/3", model.Check{Name: "rspec", RunID: 7})}}, + ticketURL}, + {"PR flag opens the PR", + []model.Flag{{Kind: model.FlagReadyToMerge, PR: pr("https://github.com/acme/app/pull/3")}}, + "https://github.com/acme/app/pull/3"}, + {"PR flag with no URL opens the ticket", + []model.Flag{{Kind: model.FlagReadyToMerge, PR: pr("")}}, + ticketURL}, + {"pipeline failure opens the pipeline", + []model.Flag{{Kind: model.FlagPipelineFailed, Slot: &model.EnvSlot{Env: env, State: model.SlotFailed}}}, + "pipeline:prod-acct/app-pipeline/Production"}, + {"approval the profile cannot make opens the pipeline", + []model.Flag{{Kind: model.FlagAwaitingApproval, Slot: slot(true, "tok")}}, + "pipeline:prod-acct/app-pipeline/Production"}, + {"approval offers a/x, so o opens the ticket", + []model.Flag{{Kind: model.FlagAwaitingApproval, Slot: slot(false, "tok")}}, + ticketURL}, + {"two pipelines to open: the ticket", + []model.Flag{ + {Kind: model.FlagPipelineFailed, Slot: &model.EnvSlot{Env: env}}, + {Kind: model.FlagUnhealthy, Slot: &model.EnvSlot{Env: other}}, + }, + ticketURL}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := model.Chain{Ticket: model.Ticket{Key: "ABC-1", URL: ticketURL}, Flags: tt.flags} + if got := ui.OpenTarget(c, fakePipelineURL); got != tt.want { + t.Errorf("OpenTarget = %q, want %q", got, tt.want) + } + }) + } +} + +// opened records the URLs the app opens, failing each with err. +type opened struct { + urls []string + err error +} + +func (o *opened) open(u string) error { o.urls = append(o.urls, u); return o.err } + +func TestAppOpen(t *testing.T) { + snap := fxSnapshot() + snap.Chains[2].Ticket.URL = "" // ABC-3 has nowhere to go + o := &opened{} + m := ui.NewApp(make(chan resolver.Snapshot), make(chan struct{}, 1), o.open).WithClock(func() time.Time { return now }) + m = update(t, update(t, m, tea.WindowSizeMsg{Width: 100, Height: 30}), snap) + + m = press(t, m, "o", "j", "o", "j", "o", "j", "o") + want := []string{ + "https://ci.example.com/lint/1", // ABC-1: the failing check's page + resolver.PipelineURL(snap.Chains[1].Slots[0].Env), // ABC-2: approval the profile cannot make + // ABC-3: no URL, nothing opened + "https://github.com/acme/app/pull/9", // the unlinked PR + } + if strings.Join(o.urls, " ") != strings.Join(want, " ") { + t.Errorf("opened\n got %q\nwant %q", o.urls, want) + } + + o.err = errors.New("no browser") + m = press(t, m, "o") + if foot := viewLines(m)[len(viewLines(m))-1]; !strings.Contains(foot, "open failed: no browser") { + t.Errorf("a failed open shows in the footer: %q", foot) + } + m = press(t, m, "k") + if foot := viewLines(m)[len(viewLines(m))-1]; strings.Contains(foot, "open failed") { + t.Errorf("the next key clears the error: %q", foot) + } +} + +func TestAppWarningsCount(t *testing.T) { + foot := func(warnings ...string) string { + snap := fxSnapshot() + snap.Warnings = warnings + ls := viewLines(update(t, newApp(t), snap)) + return ls[len(ls)-1] + } + if got := foot(); strings.Contains(got, "warning") { + t.Errorf("no warnings, no count: %q", got) + } + if got := foot("a"); !strings.HasSuffix(got, " 1 warning") { + t.Errorf("one warning: %q", got) + } + if got := foot("a", "b"); !strings.HasSuffix(got, " 2 warnings") || ansi.StringWidth(got) > 100 { + t.Errorf("two warnings, within the width: %q", got) + } +} + +func TestAppWarningsDimmed(t *testing.T) { + snap := fxSnapshot() + snap.Warnings = []string{"a", "b"} + m := update(t, newApp(t).WithStyles(terminal(env{"TERM": "xterm-256color"})), snap) + ls := viewLines(m) + if foot := ls[len(ls)-1]; !strings.Contains(foot, "\x1b[2m2 warnings\x1b[0m") { + t.Errorf("the count is dimmed: %q", foot) + } +} + +// manySnapshot has n plain chains ABC-1..ABC-n and two unlinked PRs. +func manySnapshot(n int) resolver.Snapshot { + var snap resolver.Snapshot + for i := 1; i <= n; i++ { + snap.Chains = append(snap.Chains, model.Chain{Ticket: model.Ticket{Key: fmt.Sprintf("ABC-%d", i), Title: "Work"}}) + } + snap.Unlinked = []model.PR{{Repo: "acme/app", Number: 1, Title: "One"}, {Repo: "acme/app", Number: 2, Title: "Two"}} + return snap +} + +func TestAppScrollKeepsCursorVisible(t *testing.T) { + const height = 6 // header, 4 body lines, footer + m := update(t, newApp(t), tea.WindowSizeMsg{Width: 100, Height: height}) + m = update(t, m, manySnapshot(10)) + check := func(want string) { + t.Helper() + ls := viewLines(m) + if len(ls) != height { + t.Fatalf("view is %d lines, want %d:\n%s", len(ls), height, m.View()) + } + if !strings.HasPrefix(ls[0], "app-green") || ls[height-1] != footerKeys { + t.Errorf("header and footer stay put:\n%s", m.View()) + } + if got := cursorLine(t, m); !strings.Contains(got, want+" ") { + t.Errorf("cursor on %q, want %s", got, want) + } + } + check("ABC-1") + for i := 2; i <= 10; i++ { + m = press(t, m, "j") + check(fmt.Sprintf("ABC-%d", i)) + } + m = press(t, m, "j", "j") + check("acme/app#2") // the last body line, past the Unlinked header + if ls := viewLines(m); ls[height-2] != cursorLine(t, m) { + t.Errorf("at the end the cursor is on the last body line:\n%s", m.View()) + } + for i := 0; i < 12; i++ { + m = press(t, m, "k") + } + check("ABC-1") + if ls := viewLines(m); ls[1] != cursorLine(t, m) { + t.Errorf("back at the top the first row shows:\n%s", m.View()) + } +} + +func TestAppResize(t *testing.T) { + snap := fxSnapshot() + widths := func(m ui.App) []int { + var ws []int + for _, l := range viewLines(m) { + ws = append(ws, ansi.StringWidth(l)) + } + return ws + } + m := update(t, newApp(t), snap) + m = update(t, m, tea.WindowSizeMsg{Width: 60, Height: 30}) + ws := widths(m) + for i := 1; i <= 3; i++ { // the chain rows + if ws[i] != 60 { + t.Errorf("row %d is %d cells after resizing to 60", i, ws[i]) + } + } + for i, w := range ws { + if w > 60 { + t.Errorf("line %d is %d cells, wider than 60:\n%s", i, w, m.View()) + } + } + if got, want := strings.TrimRight(viewLines(m)[2], " "), strings.TrimRight(ui.RenderRow(snap.Chains[1], ui.NewLayout(snap.Chains, now, 60), now), " "); got != want { + t.Errorf("rows use a layout for the new width\n got %q\nwant %q", got, want) + } + + // Before any size arrives the list is 80 cells wide. + fresh := update(t, ui.NewApp(nil, nil, noOpen(t)).WithClock(func() time.Time { return now }), snap) + if w := widths(fresh)[1]; w != 80 { + t.Errorf("before the first size, rows are %d cells, want 80", w) + } +} + +func TestAppWaitsForSnapshots(t *testing.T) { + snaps := make(chan resolver.Snapshot, 2) + m := ui.NewApp(snaps, make(chan struct{}, 1), noOpen(t)).WithClock(func() time.Time { return now }) + first, second := fxSnapshot(), manySnapshot(2) + snaps <- first + snaps <- second + + cmd := m.Init() + if cmd == nil { + t.Fatal("Init waits for the first snapshot") + } + msg := cmd() + if s, ok := msg.(resolver.Snapshot); !ok || len(s.Chains) != 3 { + t.Fatalf("Init delivers the first snapshot, got %T", msg) + } + next, cmd := m.Update(msg) + if cmd == nil { + t.Fatal("a snapshot waits for the next one") + } + if s, ok := cmd().(resolver.Snapshot); !ok || len(s.Chains) != 2 { + t.Fatalf("the next wait delivers the second snapshot") + } + if !strings.Contains(next.View(), "ABC-1") { + t.Errorf("the first snapshot is shown:\n%s", next.View()) + } + + close(snaps) + if msg := m.Init()(); msg != nil { + t.Errorf("a closed channel (the resolver stopped) delivers nothing, got %T", msg) + } +} + +func TestAppInertKeys(t *testing.T) { + // Keys for later tasks do nothing yet, loaded or not, and o, j and k on + // an empty list are safe. + for _, m := range []ui.App{newApp(t), update(t, newApp(t), resolver.Snapshot{}), update(t, newApp(t), fxSnapshot())} { + before := m.View() + if m = press(t, m, "enter", "f", "a", "x", "t"); m.View() != before { + t.Errorf("enter/f/a/x/t changed the view:\n%s", m.View()) + } + } + for _, m := range []ui.App{newApp(t), update(t, newApp(t), resolver.Snapshot{})} { + press(t, m, "j", "k", "o") // noOpen fails on any open + } +} + +func TestAppNoColor(t *testing.T) { + m := update(t, newApp(t).WithStyles(terminal(env{"TERM": "xterm-256color", "NO_COLOR": "1"})), fxSnapshot()) + m = press(t, m, "j") + if v := m.View(); strings.Contains(v, "\x1b[") { + t.Errorf("NO_COLOR output has escape codes: %q", v) + } + if got := cursorLine(t, m); !strings.Contains(got, "ABC-2") { + t.Errorf("under NO_COLOR the cursor shows as >: %q", got) + } +} diff --git a/internal/ui/styles.go b/internal/ui/styles.go index a21b8a4..f4f02f6 100644 --- a/internal/ui/styles.go +++ b/internal/ui/styles.go @@ -8,6 +8,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/charmbracelet/x/ansi" + "github.com/muesli/termenv" ) // Styles holds every style the screens use, bound to one renderer, so a test @@ -18,6 +19,9 @@ type Styles struct { green lipgloss.Style dim lipgloss.Style bold lipgloss.Style + // plain: the renderer prints no escape codes (no color terminal, or + // NO_COLOR), so the cursor cannot be reverse video. + plain bool } // NewStyles builds the styles on r. The renderer decides the color profile; @@ -29,6 +33,7 @@ func NewStyles(r *lipgloss.Renderer) Styles { green: r.NewStyle().Foreground(lipgloss.Color("2")), dim: r.NewStyle().Faint(true), bold: r.NewStyle().Bold(true), + plain: r.ColorProfile() == termenv.Ascii, } } @@ -73,3 +78,19 @@ func fit(s string, w int) string { s = ansi.Truncate(s, w, "…") return s + strings.Repeat(" ", w-ansi.StringWidth(s)) } + +// SGR codes for the cursor. Every profile but ASCII supports reverse video. +const ( + sgrReverse = "\x1b[7m" + sgrReset = "\x1b[0m" +) + +// selected marks line as the cursor's: reverse video, turned back on after +// every reset the line's own styles end with. Without escape codes the +// line's first cell (its marker) becomes ">". +func (s Styles) selected(line string) string { + if s.plain { + return ">" + ansi.TruncateLeft(line, 1, "") + } + return sgrReverse + strings.ReplaceAll(line, sgrReset, sgrReset+sgrReverse) + sgrReset +} From dc02605c3752ebae946105e5426ece17260b9712 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 10:56:33 -0400 Subject: [PATCH 06/21] chore(deps): match jira-green versions golang.org/x/text goes to v0.23.0, the version jira-green uses, so every module the two repos share is at the same version. Assisted-by: Claude Code:claude-opus-5-5 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 2f12ac1..ce05d74 100644 --- a/go.mod +++ b/go.mod @@ -46,5 +46,5 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect golang.org/x/sys v0.38.0 // indirect - golang.org/x/text v0.3.8 // indirect + golang.org/x/text v0.23.0 // indirect ) diff --git a/go.sum b/go.sum index 4f9919d..e9913f6 100644 --- a/go.sum +++ b/go.sum @@ -78,5 +78,5 @@ golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/text v0.3.8 h1:nAL+RVCQ9uMn3vJZbV+MRnydTJFPf8qqY42YiA6MrqY= -golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= +golang.org/x/text v0.23.0 h1:D71I7dUrlY+VX0gQShAThNGHFxZ13dGLBHQLVl1mJlY= +golang.org/x/text v0.23.0/go.mod h1:/BLNzu4aZCJ1+kcD0DNRotWKage4q2rGVAg4o22unh4= From 563989c5ce30f7c04c10ac2128f0e748aa4c1a73 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 10:59:04 -0400 Subject: [PATCH 07/21] fix(ui): o opens the row's flag target o now opens what the row's first flag is about, whatever its main key: a failing check's page (else the PR's checks tab) on an f row, the pipeline on an a/x row, the PR for a PR flag. A t row, a flag with no target, or an unknown page opens the ticket. With two targets for the same key, o opens the first flag's. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/list.go | 13 +++++++------ internal/ui/list_test.go | 28 ++++++++++++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/internal/ui/list.go b/internal/ui/list.go index c2abcd7..b1370f0 100644 --- a/internal/ui/list.go +++ b/internal/ui/list.go @@ -62,13 +62,14 @@ func (m App) mark(i int, line string) string { return line } -// OpenTarget is the page o opens for c's row. When the row offers o, it is -// what Flags[0] points at: the pipeline console (pipelineURL) for an Env, a -// failing check's own page (else the PR's checks tab) for a failed check, or -// the PR. Otherwise, or when that page is unknown, it is the ticket. It is -// "" only when the ticket's URL is unknown too. +// OpenTarget is the page o opens for c's row: what Flags[0] is about, +// whatever the row's main key. That is the pipeline console (pipelineURL) +// for an Env, a failing check's own page (else the PR's checks tab) for a +// failed check, or the PR. With no flag, a flag with no target, or an +// unknown page, it is the ticket. It is "" only when the ticket's URL is +// unknown too. func OpenTarget(c model.Chain, pipelineURL func(model.Env) string) string { - if RowAction(c) != ActionOpen { + if len(c.Flags) == 0 { return c.Ticket.URL } f := c.Flags[0] diff --git a/internal/ui/list_test.go b/internal/ui/list_test.go index a51929c..7bff297 100644 --- a/internal/ui/list_test.go +++ b/internal/ui/list_test.go @@ -294,9 +294,12 @@ func TestOpenTarget(t *testing.T) { {"failing check without a URL opens the checks tab", []model.Flag{{Kind: model.FlagCheckFailed, PR: pr("https://github.com/acme/app/pull/3", model.Check{Name: "lint"})}}, "https://github.com/acme/app/pull/3/checks"}, - {"re-runnable check offers f, so o opens the ticket", + {"re-runnable check (f) opens the checks tab", []model.Flag{{Kind: model.FlagCheckFailed, PR: pr("https://github.com/acme/app/pull/3", model.Check{Name: "rspec", RunID: 7})}}, - ticketURL}, + "https://github.com/acme/app/pull/3/checks"}, + {"re-runnable check (f) with a URL opens it", + []model.Flag{{Kind: model.FlagCheckFailed, PR: pr("https://github.com/acme/app/pull/3", model.Check{Name: "rspec", RunID: 7, URL: "https://ci.example.com/rspec"})}}, + "https://ci.example.com/rspec"}, {"PR flag opens the PR", []model.Flag{{Kind: model.FlagReadyToMerge, PR: pr("https://github.com/acme/app/pull/3")}}, "https://github.com/acme/app/pull/3"}, @@ -309,15 +312,21 @@ func TestOpenTarget(t *testing.T) { {"approval the profile cannot make opens the pipeline", []model.Flag{{Kind: model.FlagAwaitingApproval, Slot: slot(true, "tok")}}, "pipeline:prod-acct/app-pipeline/Production"}, - {"approval offers a/x, so o opens the ticket", + {"approval (a/x) opens the pipeline", []model.Flag{{Kind: model.FlagAwaitingApproval, Slot: slot(false, "tok")}}, + "pipeline:prod-acct/app-pipeline/Production"}, + {"status mismatch (t) opens the ticket", + []model.Flag{{Kind: model.FlagStatusMismatch, Reason: "Jira still In Progress"}}, + ticketURL}, + {"repo no env deploys has no target: the ticket", + []model.Flag{{Kind: model.FlagDeployUnknown}}, ticketURL}, - {"two pipelines to open: the ticket", + {"two pipelines: the first flag's", []model.Flag{ {Kind: model.FlagPipelineFailed, Slot: &model.EnvSlot{Env: env}}, {Kind: model.FlagUnhealthy, Slot: &model.EnvSlot{Env: other}}, }, - ticketURL}, + "pipeline:prod-acct/app-pipeline/Production"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -340,6 +349,13 @@ func (o *opened) open(u string) error { o.urls = append(o.urls, u); return o.err func TestAppOpen(t *testing.T) { snap := fxSnapshot() snap.Chains[2].Ticket.URL = "" // ABC-3 has nowhere to go + // ABC-2's approval can be made here, so its key is a/x; o still opens + // the pipeline. + snap.Chains[1].Slots[0].Env.ReadOnly = false + snap.Chains[1].Slots[0].Deploy = &model.Deploy{Status: model.DeployAwaitingApproval, ApprovalToken: "tok"} + if a := ui.RowAction(snap.Chains[1]); a != ui.ActionApprove { + t.Fatalf("ABC-2 offers %v, want a/x", a) + } o := &opened{} m := ui.NewApp(make(chan resolver.Snapshot), make(chan struct{}, 1), o.open).WithClock(func() time.Time { return now }) m = update(t, update(t, m, tea.WindowSizeMsg{Width: 100, Height: 30}), snap) @@ -347,7 +363,7 @@ func TestAppOpen(t *testing.T) { m = press(t, m, "o", "j", "o", "j", "o", "j", "o") want := []string{ "https://ci.example.com/lint/1", // ABC-1: the failing check's page - resolver.PipelineURL(snap.Chains[1].Slots[0].Env), // ABC-2: approval the profile cannot make + resolver.PipelineURL(snap.Chains[1].Slots[0].Env), // ABC-2: the approval's pipeline // ABC-3: no URL, nothing opened "https://github.com/acme/app/pull/9", // the unlinked PR } From 42fd14eb3c656a7c765eaa64aa0148f3df2a1574 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:08:23 -0400 Subject: [PATCH 08/21] feat(ui): detail screen enter on a ticket row opens its detail: the ticket line with its Jira status, a stale line when the chain may be behind, one line per PR and per applying env, then every flag reason. Up/down select a PR or env and o opens it (failing checks, the PR, or the pipeline console; the ticket when nothing is selectable). esc and backspace go back with the list cursor unchanged. A new snapshot refreshes the detail, keeping the selection on its PR or env, and returns to the list with a note when the ticket is gone. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/app.go | 21 ++- internal/ui/detail.go | 285 ++++++++++++++++++++++++++++++++++ internal/ui/detail_test.go | 306 +++++++++++++++++++++++++++++++++++++ internal/ui/list.go | 34 +++-- internal/ui/list_test.go | 17 ++- 5 files changed, 644 insertions(+), 19 deletions(-) create mode 100644 internal/ui/detail.go create mode 100644 internal/ui/detail_test.go diff --git a/internal/ui/app.go b/internal/ui/app.go index 403d9c1..d04527a 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -24,6 +24,10 @@ type App struct { err string // the last open's error, until the next key offset int // the first body line shown width, height int + + detail string // the ticket key whose detail screen is open, "" for the list + dsel int // the selected item of the detail screen + note string // why the detail screen closed, until the next key } // NewApp builds the App. snaps is the resolver's out channel, refresh its @@ -66,8 +70,12 @@ func (m App) update(msg tea.Msg) (App, tea.Cmd) { m.width, m.height = msg.Width, msg.Height case resolver.Snapshot: id := m.itemID(m.cursor) + before, _ := m.chain(m.detail) m.snap = &msg m.cursor = m.find(id) + if m.detail != "" { + m = m.refreshDetail(before) + } return m, waitForSnapshot(m.snaps) case tea.KeyMsg: return m.key(msg) @@ -77,17 +85,22 @@ func (m App) update(msg tea.Msg) (App, tea.Cmd) { // keys are the list's key bindings. var keys = struct { - up, down, refresh, quit, open key.Binding + up, down, refresh, quit, open, enter, back key.Binding }{ up: key.NewBinding(key.WithKeys("up", "k")), down: key.NewBinding(key.WithKeys("down", "j")), refresh: key.NewBinding(key.WithKeys("r")), open: key.NewBinding(key.WithKeys("o")), + enter: key.NewBinding(key.WithKeys("enter")), + back: key.NewBinding(key.WithKeys("esc", "backspace")), quit: key.NewBinding(key.WithKeys("q", "ctrl+c")), } func (m App) key(msg tea.KeyMsg) (App, tea.Cmd) { - m.err = "" + m.err, m.note = "", "" + if m.detail != "" { + return m.detailKey(msg) + } switch { case key.Matches(msg, keys.quit): return m, tea.Quit @@ -100,6 +113,10 @@ func (m App) key(msg tea.KeyMsg) (App, tea.Cmd) { case m.refresh <- struct{}{}: default: // a refresh is already pending } + case key.Matches(msg, keys.enter): + if m.snap != nil && m.cursor < len(m.snap.Chains) { + m.detail, m.dsel = m.snap.Chains[m.cursor].Ticket.Key, 0 + } case key.Matches(msg, keys.open): if u := m.target(); u != "" { if err := m.openURL(u); err != nil { diff --git a/internal/ui/detail.go b/internal/ui/detail.go new file mode 100644 index 0000000..bb62044 --- /dev/null +++ b/internal/ui/detail.go @@ -0,0 +1,285 @@ +package ui + +import ( + "fmt" + "strings" + "time" + + "github.com/charmbracelet/bubbles/key" + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/x/ansi" + "github.com/ericdahl-dev/app-green/internal/model" + "github.com/ericdahl-dev/app-green/internal/resolver" +) + +// detailKeys is the key line at the bottom of the detail screen. +const detailKeys = "↑/↓ move o open esc back q quit" + +// chain is the chain whose ticket key is key, and whether there is one. +func (m App) chain(key string) (model.Chain, bool) { + if m.snap != nil { + for _, c := range m.snap.Chains { + if c.Ticket.Key == key { + return c, true + } + } + } + return model.Chain{}, false +} + +// detailKey handles a key on the detail screen. Keys it does not know do +// nothing. +func (m App) detailKey(msg tea.KeyMsg) (App, tea.Cmd) { + switch { + case key.Matches(msg, keys.quit): + return m, tea.Quit + case key.Matches(msg, keys.back): + m.detail = "" + case key.Matches(msg, keys.up): + m.dsel = max(m.dsel-1, 0) + case key.Matches(msg, keys.down): + c, _ := m.chain(m.detail) + m.dsel = min(m.dsel+1, max(len(detailItems(c))-1, 0)) + case key.Matches(msg, keys.open): + c, _ := m.chain(m.detail) + if u := detailTarget(c, m.dsel); u != "" { + if err := m.openURL(u); err != nil { + m.err = "open failed: " + err.Error() + } + } + } + return m, nil +} + +// refreshDetail keeps the detail screen on its ticket after a new snapshot, +// with the selection on the same PR or env as in before, the ticket's chain +// in the last snapshot; when that item is gone the selection stays at its +// index, clamped. When the ticket is gone it returns to the list with a +// note saying so. +func (m App) refreshDetail(before model.Chain) App { + c, ok := m.chain(m.detail) + if !ok { + m.note = m.detail + " is no longer in the list" + m.detail = "" + return m + } + items := detailItems(c) + if old := detailItems(before); m.dsel < len(old) { + id := old[m.dsel].id() + for i, it := range items { + if it.id() == id { + m.dsel = i + return m + } + } + } + m.dsel = max(min(m.dsel, len(items)-1), 0) + return m +} + +// id names the item so the selection can find it in the next snapshot: +// "pr repo#number" (repo in lower case) or "env ". +func (it detailItem) id() string { + if it.slot != nil { + return "env " + it.slot.Env.ID() + } + return fmt.Sprintf("pr %s#%d", strings.ToLower(it.pr.Repo), it.pr.Number) +} + +// detailItem is one selectable line of the detail screen: a PR or an env +// slot. +type detailItem struct { + pr *model.PR + slot *model.EnvSlot +} + +// detailItems is c's selectable lines: its PRs, then its applying slots. +func detailItems(c model.Chain) []detailItem { + var out []detailItem + for i := range c.PRs { + out = append(out, detailItem{pr: &c.PRs[i]}) + } + for i := range c.Slots { + if c.Slots[i].Applies { + out = append(out, detailItem{slot: &c.Slots[i]}) + } + } + return out +} + +// detailTarget is the page o opens on item i of c's detail: a PR's failing +// checks (checksTarget) or else the PR, an env's pipeline console. With no +// item, or an item whose page is unknown, it is the ticket. +func detailTarget(c model.Chain, i int) string { + items := detailItems(c) + var u string + if i >= 0 && i < len(items) { + switch it := items[i]; { + case it.slot != nil: + u = resolver.PipelineURL(it.slot.Env) + case it.pr.Checks == model.ChecksFailing || len(it.pr.Failing) > 0: + u = checksTarget(*it.pr) + default: + u = it.pr.URL + } + } + if u == "" { + return c.Ticket.URL + } + return u +} + +// detailView is the header, the detail of the open ticket, and the footer. +func (m App) detailView(width int) string { + c, _ := m.chain(m.detail) + now := m.now() + body := []string{ticketLine(c, width)} + if c.Stale { + body = append(body, m.styles.dim.Render(fit("~ stale: "+clean(c.StaleReason), width))) + } + items := detailItems(c) + acctW, stageW := 0, 0 + for _, it := range items { + if it.slot != nil { + acctW = max(acctW, ansi.StringWidth(clean(it.slot.Env.Account))) + stageW = max(stageW, ansi.StringWidth(clean(it.slot.Env.Stage))) + } + } + for i, it := range items { + var line string + if it.slot != nil { + line = " " + fit(clean(it.slot.Env.Account), acctW) + " " + fit(clean(it.slot.Env.Stage), stageW) + " " + slotText(*it.slot, now) + } else { + line = " " + prLine(*it.pr, now) + } + body = append(body, m.detailMark(i, fit(line, width))) + } + if len(c.Flags) > 0 { + body = append(body, "Flags") + for _, f := range c.Flags { + body = append(body, fit(" "+m.styles.marker(f.Level)+" "+clean(f.Reason), width)) + } + } + lines := append([]string{m.styles.Header(m.snap.Status, m.now(), width)}, body...) + return strings.Join(append(lines, m.footer(detailKeys, width)), "\n") +} + +// ticketLine is the key and title, with "Jira: " at the right edge. +// The title gives way first. +func ticketLine(c model.Chain, width int) string { + left := clean(c.Ticket.Key) + " " + clean(c.Ticket.Title) + if c.Ticket.Status == "" { + return fit(left, width) + } + right := "Jira: " + clean(c.Ticket.Status) + avail := width - ansi.StringWidth(right) - 2 + if avail < 1 { + return fit(right, width) + } + return fit(fit(left, avail)+" "+right, width) +} + +// detailMark is line, selected when it is the detail screen's item i. +func (m App) detailMark(i int, line string) string { + if i == m.dsel { + return m.styles.selected(line) + } + return line +} + +// prLine is "PR #330 acme/app merged 2d ago ✓ checks ✓ approved": the +// PR's state, checks and review, and "draft" for a draft. +func prLine(p model.PR, now time.Time) string { + parts := []string{fmt.Sprintf("PR #%d", p.Number), clean(p.Repo)} + switch p.State { + case model.PRMerged: + parts = append(parts, "merged"+ago(p.MergedAt, now)) + case model.PROpen: + parts = append(parts, "open") + case model.PRClosed: + parts = append(parts, "closed") + } + switch p.Checks { + case model.ChecksPassing: + parts = append(parts, "✓ checks") + case model.ChecksFailing: + var names []string + for _, ch := range p.Failing { + names = append(names, clean(ch.Name)) + } + if len(names) == 0 { + names = []string{"checks"} + } + parts = append(parts, "✗ "+strings.Join(names, ", ")) + case model.ChecksPending: + parts = append(parts, "checks running") + case model.ChecksExpected: + parts = append(parts, "checks expected") + } + switch p.Review { + case model.ReviewApproved: + parts = append(parts, "✓ approved") + case model.ReviewChangesRequested: + parts = append(parts, "changes requested") + case model.ReviewRequired: + parts = append(parts, "review required") + } + if p.IsDraft { + parts = append(parts, "draft") + } + return strings.Join(parts, " ") +} + +// ago is " 2d ago" for at, "" when at is unknown. +func ago(at, now time.Time) string { + if at.IsZero() { + return "" + } + return " " + age(now.Sub(at)) + " ago" +} + +// slotText is where the work stands in s's env: "✓ deployed 1d ago +// (aaaa111) ECS 2/2", "‖ awaiting approval since 1d", "in progress since +// 2m (cccc333)", "✗ failed 3h ago (bbbb222)", "✗ rejected or expired …", +// "✗ rolled back …", "deploy unknown" or "not yet". Unknown times and SHAs +// are left out. +func slotText(s model.EnvSlot, now time.Time) string { + switch s.State { + case model.SlotDeployed: + t := "✓ deployed" + ago(s.At, now) + sha(s.SHA) + if s.Health.Known { + t += fmt.Sprintf(" ECS %d/%d", s.Health.Healthy, s.Health.Desired) + } + return t + case model.SlotAwaitingApproval: + if s.At.IsZero() { + return "‖ awaiting approval" + } + return "‖ awaiting approval since " + age(now.Sub(s.At)) + case model.SlotInProgress: + t := "in progress" + if !s.At.IsZero() { + t += " since " + age(now.Sub(s.At)) + } + return t + sha(s.SHA) + case model.SlotFailed: + if s.Deploy != nil && s.Deploy.Status == model.DeployRejected { + return "✗ rejected or expired" + ago(s.At, now) + sha(s.SHA) + } + return "✗ failed" + ago(s.At, now) + sha(s.SHA) + case model.SlotRolledBack: + return "✗ rolled back" + ago(s.At, now) + sha(s.SHA) + case model.SlotUnknown: + return "deploy unknown" + } + return "not yet" +} + +// sha is " (aaaa111)", the commit cut to 7 cells, or "" when unknown. +func sha(s string) string { + s = clean(s) + if s == "" { + return "" + } + return " (" + ansi.Truncate(s, 7, "") + ")" +} diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go new file mode 100644 index 0000000..ed25390 --- /dev/null +++ b/internal/ui/detail_test.go @@ -0,0 +1,306 @@ +package ui_test + +import ( + "errors" + "strings" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" + "github.com/ericdahl-dev/app-green/internal/model" + "github.com/ericdahl-dev/app-green/internal/resolver" + "github.com/ericdahl-dev/app-green/internal/ui" +) + +// detailKeys is the detail screen's key line. +const detailKeys = "↑/↓ move o open esc back q quit" + +var ( + stageTest = model.Env{Account: "stage-acct", Region: "us-east-1", Pipeline: "app-pipeline", Stage: "Test", Order: 0} + stageProd = model.Env{Account: "stage-acct", Region: "us-east-1", Pipeline: "app-pipeline", Stage: "Production", Order: 1, Prod: true} + prodTest = model.Env{Account: "prod-acct", Region: "us-west-2", Pipeline: "app-pipeline", Stage: "Test", Order: 2} + prodProd = model.Env{Account: "prod-acct", Region: "us-west-2", Pipeline: "app-pipeline", Stage: "Production", Order: 3, Prod: true} +) + +// fxDetail is ABC-19, the design's detail example: two PRs, four envs that +// apply and one that does not, and two flags. +func fxDetail() model.Chain { + day := 24 * time.Hour + c := model.Chain{ + Ticket: model.Ticket{Key: "ABC-19", Title: "Drop the form-level opt-outs", Status: "Code Review", + URL: "https://jira.example.com/browse/ABC-19"}, + PRs: []model.PR{ + {Repo: "acme/app", Number: 330, State: model.PRMerged, MergedAt: now.Add(-2 * day), + URL: "https://github.com/acme/app/pull/330", Checks: model.ChecksPassing, Review: model.ReviewApproved}, + {Repo: "acme/app", Number: 331, State: model.PROpen, IsDraft: true, + URL: "https://github.com/acme/app/pull/331", Checks: model.ChecksFailing, + Failing: []model.Check{{Name: "rspec", RunID: 7}, {Name: "CodeQL", CodeScanning: true, URL: "https://github.com/acme/app/security/1"}}, + Review: model.ReviewChangesRequested}, + }, + Slots: []model.EnvSlot{ + {Env: stageTest, State: model.SlotDeployed, SHA: "aaaa1111deadbeef", At: now.Add(-day), Applies: true, + Health: model.Health{Known: true, Desired: 2, Healthy: 2}}, + {Env: stageProd, State: model.SlotAwaitingApproval, At: now.Add(-day), Applies: true}, + {Env: model.Env{Account: "other-acct", Pipeline: "web-pipeline", Stage: "Test"}, State: model.SlotNotYet}, + {Env: prodTest, State: model.SlotFailed, SHA: "bbbb2222", At: now.Add(-3 * time.Hour), Applies: true}, + {Env: prodProd, State: model.SlotNotYet, Applies: true}, + }, + Stage: model.StageInTest, + } + c.Flags = []model.Flag{ + {Level: model.Red, Kind: model.FlagPipelineFailed, Reason: "prod-acct Test failed", Slot: &c.Slots[3]}, + {Level: model.Yellow, Kind: model.FlagAwaitingApproval, Reason: "stage-acct Production awaiting approval", Slot: &c.Slots[1]}, + } + return c +} + +// fxDetailSnapshot is fxSnapshot with ABC-19 second. +func fxDetailSnapshot() resolver.Snapshot { + snap := fxSnapshot() + snap.Chains = append([]model.Chain{snap.Chains[0], fxDetail()}, snap.Chains[1:]...) + return snap +} + +// inDetail is the App on fxDetailSnapshot with ABC-19's detail open. +func inDetail(t *testing.T) ui.App { + t.Helper() + return press(t, update(t, newApp(t), fxDetailSnapshot()), "j", "enter") +} + +// trimmed is the view's lines with trailing spaces cut. +func trimmed(m ui.App) []string { + ls := viewLines(m) + for i, l := range ls { + ls[i] = strings.TrimRight(l, " ") + } + return ls +} + +func TestDetailEnvStates(t *testing.T) { + ago := func(d time.Duration) time.Time { return now.Add(-d) } + tests := []struct { + name string + slot model.EnvSlot + want string + }{ + {"deployed", model.EnvSlot{State: model.SlotDeployed, SHA: "aaaa1111", At: ago(time.Hour), + Health: model.Health{Known: true, Desired: 3, Healthy: 1}}, "✓ deployed 1h ago (aaaa111) ECS 1/3"}, + {"deployed, health unknown", model.EnvSlot{State: model.SlotDeployed, SHA: "aaaa1111", At: ago(time.Hour)}, + "✓ deployed 1h ago (aaaa111)"}, + {"deployed, no time or SHA", model.EnvSlot{State: model.SlotDeployed}, "✓ deployed"}, + {"awaiting approval", model.EnvSlot{State: model.SlotAwaitingApproval, At: ago(5 * time.Minute)}, + "‖ awaiting approval since 5m"}, + {"awaiting approval, no time", model.EnvSlot{State: model.SlotAwaitingApproval}, "‖ awaiting approval"}, + {"in progress", model.EnvSlot{State: model.SlotInProgress, SHA: "cccc3333", At: ago(2 * time.Minute)}, + "in progress since 2m (cccc333)"}, + {"failed", model.EnvSlot{State: model.SlotFailed, SHA: "bbbb2222", At: ago(3 * time.Hour)}, "✗ failed 3h ago (bbbb222)"}, + {"rejected", model.EnvSlot{State: model.SlotFailed, SHA: "bbbb2222", At: ago(3 * time.Hour), + Deploy: &model.Deploy{Status: model.DeployRejected}}, "✗ rejected or expired 3h ago (bbbb222)"}, + {"rolled back", model.EnvSlot{State: model.SlotRolledBack, SHA: "dddd4444", At: ago(48 * time.Hour)}, + "✗ rolled back 2d ago (dddd444)"}, + {"unknown", model.EnvSlot{State: model.SlotUnknown}, "deploy unknown"}, + {"not yet", model.EnvSlot{State: model.SlotNotYet}, "not yet"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := model.Chain{Ticket: model.Ticket{Key: "ABC-5", Title: "One env"}} + tt.slot.Env, tt.slot.Applies = stageTest, true + c.Slots = []model.EnvSlot{tt.slot} + snap := resolver.Snapshot{Chains: []model.Chain{c}, At: now} + ls := trimmed(press(t, update(t, newApp(t), snap), "enter")) + if want := "> stage-acct Test " + tt.want; ls[2] != want { + t.Errorf("\n got %q\nwant %q", ls[2], want) + } + }) + } +} + +func TestDetailView(t *testing.T) { + snap := fxDetailSnapshot() + want := []string{ + ui.RenderHeader(snap.Status, now, 100), + "ABC-19 Drop the form-level opt-outs" + strings.Repeat(" ", 100-36-17) + "Jira: Code Review", + "> PR #330 acme/app merged 2d ago ✓ checks ✓ approved", + " PR #331 acme/app open ✗ rspec, CodeQL changes requested draft", + " stage-acct Test ✓ deployed 1d ago (aaaa111) ECS 2/2", + " stage-acct Production ‖ awaiting approval since 1d", + " prod-acct Test ✗ failed 3h ago (bbbb222)", + " prod-acct Production not yet", + "Flags", + " ● prod-acct Test failed", + " ◐ stage-acct Production awaiting approval", + detailKeys, + } + // other-acct's env does not apply, so it is left out. + got := trimmed(inDetail(t)) + if strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Errorf("view\n got:\n%s\nwant:\n%s", strings.Join(got, "\n"), strings.Join(want, "\n")) + } +} + +func TestDetailStale(t *testing.T) { + snap := fxDetailSnapshot() + snap.Chains[1].Stale, snap.Chains[1].StaleReason = true, "acme/app not refreshed since 11:58:00" + ls := trimmed(press(t, update(t, newApp(t), snap), "j", "enter")) + if want := "~ stale: acme/app not refreshed since 11:58:00"; ls[2] != want { + t.Errorf("stale line\n got %q\nwant %q", ls[2], want) + } + if !strings.HasPrefix(ls[3], "> PR #330") { + t.Errorf("the PRs follow the stale line: %q", ls[3]) + } + + styles := terminal(env{"TERM": "xterm-256color"}) + colored := viewLines(press(t, update(t, newApp(t).WithStyles(styles), snap), "j", "enter")) + if !strings.HasPrefix(colored[2], "\x1b[2m") { + t.Errorf("the stale line is dimmed: %q", colored[2]) + } +} + +func TestDetailBack(t *testing.T) { + for _, k := range []string{"esc", "backspace"} { + m := press(t, inDetail(t), "j", "j", k) // moving in detail leaves the list cursor alone + if got := cursorLine(t, m); !strings.Contains(got, "ABC-19") { + t.Errorf("%s returns to the list on ABC-19: %q", k, got) + } + if ls := viewLines(m); ls[len(ls)-1] != footerKeys { + t.Errorf("%s shows the list footer: %q", k, ls[len(ls)-1]) + } + } + for _, k := range []string{"q", "ctrl+c"} { + _, cmd := inDetail(t).Update(key(k)) + if cmd == nil { + t.Fatalf("%s in detail returns no command", k) + } + if _, ok := cmd().(tea.QuitMsg); !ok { + t.Errorf("%s in detail does not quit", k) + } + } +} + +func TestDetailNotForUnlinked(t *testing.T) { + m := press(t, update(t, newApp(t), fxDetailSnapshot()), "j", "j", "j", "j") // the unlinked PR + before := m.View() + if m = press(t, m, "enter"); m.View() != before { + t.Errorf("enter on an unlinked PR changes nothing:\n%s", m.View()) + } +} + +// detailWith is the App, opening URLs with o, on fxDetailSnapshot with +// ABC-19's detail open. +func detailWith(t *testing.T, o *opened, snap resolver.Snapshot) ui.App { + t.Helper() + m := ui.NewApp(make(chan resolver.Snapshot), make(chan struct{}, 1), o.open).WithClock(func() time.Time { return now }) + m = update(t, update(t, m, tea.WindowSizeMsg{Width: 100, Height: 30}), snap) + return press(t, m, "j", "enter") +} + +func TestDetailSelect(t *testing.T) { + m := inDetail(t) + steps := []struct { + key string + want string + }{ + {"", "PR #330"}, + {"up", "PR #330"}, // stays at the top + {"j", "PR #331"}, + {"down", "stage-acct Test"}, + {"j", "stage-acct Production"}, + {"j", "prod-acct Test"}, + {"j", "prod-acct Production"}, + {"j", "prod-acct Production"}, // stays at the bottom: flags are not selectable + {"k", "prod-acct Test"}, + } + for _, s := range steps { + if s.key != "" { + m = press(t, m, s.key) + } + if got := cursorLine(t, m); !strings.Contains(got, s.want) { + t.Errorf("after %q the selection is %q, want %s", s.key, got, s.want) + } + } +} + +func TestDetailOpen(t *testing.T) { + o := &opened{} + snap := fxDetailSnapshot() + m := detailWith(t, o, snap) + press(t, m, "o", "j", "o", "j", "o", "j", "j", "o") + want := []string{ + "https://github.com/acme/app/pull/330", // a PR: its page + "https://github.com/acme/app/security/1", // failing checks: the first with a page + resolver.PipelineURL(stageTest), // an env: its pipeline + resolver.PipelineURL(prodTest), + } + if strings.Join(o.urls, " ") != strings.Join(want, " ") { + t.Errorf("opened\n got %q\nwant %q", o.urls, want) + } + + // Failing checks with no page of their own: the checks tab. + o = &opened{} + snap.Chains[1].PRs[1].Failing[1].URL = "" + press(t, detailWith(t, o, snap), "j", "o") + if want := "https://github.com/acme/app/pull/331/checks"; len(o.urls) != 1 || o.urls[0] != want { + t.Errorf("opened %q, want %q", o.urls, want) + } + + // Nothing selectable: the ticket. + o = &opened{} + snap.Chains[1].PRs, snap.Chains[1].Slots = nil, nil + press(t, detailWith(t, o, snap), "o") + if want := "https://jira.example.com/browse/ABC-19"; len(o.urls) != 1 || o.urls[0] != want { + t.Errorf("opened %q, want %q", o.urls, want) + } + + // A failed open shows in the footer. + o = &opened{err: errors.New("no browser")} + m = press(t, detailWith(t, o, fxDetailSnapshot()), "o") + if ls := viewLines(m); !strings.Contains(ls[len(ls)-1], "open failed: no browser") { + t.Errorf("a failed open shows in the footer: %q", ls[len(ls)-1]) + } +} + +func TestDetailRefresh(t *testing.T) { + m := press(t, inDetail(t), "j", "j", "j") // on stage-acct Production + + // ABC-19 moves to the top, its approval goes through, and a new PR + // appears above the selection. + next := fxDetailSnapshot() + c := next.Chains[1] + c.Slots[1].State, c.Slots[1].SHA, c.Slots[1].At = model.SlotDeployed, "aaaa1111", now + c.PRs = append([]model.PR{{Repo: "acme/app", Number: 340, State: model.PROpen}}, c.PRs...) + next.Chains = append([]model.Chain{c, next.Chains[0]}, next.Chains[2:]...) + m = update(t, m, next) + + ls := trimmed(m) + if !strings.HasPrefix(ls[1], "ABC-19") || ls[len(ls)-1] != detailKeys { + t.Fatalf("still on ABC-19's detail:\n%s", m.View()) + } + if got := cursorLine(t, m); !strings.Contains(got, "stage-acct Production ✓ deployed 0s ago (aaaa111)") { + t.Errorf("the selection follows its env and shows the new state: %q", got) + } + if !strings.Contains(m.View(), "PR #340") { + t.Errorf("the new PR shows:\n%s", m.View()) + } + m = press(t, m, "esc") + if got := cursorLine(t, m); !strings.Contains(got, "ABC-19") { + t.Errorf("back on the list, the cursor followed ABC-19: %q", got) + } +} + +func TestDetailTicketGone(t *testing.T) { + m := update(t, inDetail(t), fxSnapshot()) // without ABC-19 + ls := viewLines(m) + if !strings.Contains(m.View(), "ABC-1 ") || strings.Contains(m.View(), "Jira: Code Review") { + t.Fatalf("back on the list:\n%s", m.View()) + } + if want := " ABC-19 is no longer in the list"; !strings.HasSuffix(ls[len(ls)-1], want) { + t.Errorf("the footer says why\n got %q\nwant suffix %q", ls[len(ls)-1], want) + } + if got := cursorLine(t, m); !strings.Contains(got, "ABC-2") { + t.Errorf("the cursor stays at its index: %q", got) + } + m = press(t, m, "j") + if ls := viewLines(m); strings.Contains(ls[len(ls)-1], "no longer") { + t.Errorf("the next key clears the note: %q", ls[len(ls)-1]) + } +} diff --git a/internal/ui/list.go b/internal/ui/list.go index b1370f0..282fcb2 100644 --- a/internal/ui/list.go +++ b/internal/ui/list.go @@ -23,6 +23,9 @@ func (m App) View() string { if width <= 0 { width = defaultWidth } + if m.detail != "" { + return m.detailView(width) + } now := m.now() var status []resolver.AdapterStatus var body []string @@ -51,7 +54,7 @@ func (m App) View() string { body = body[m.offset:min(m.offset+budget, len(body))] } lines := append([]string{m.styles.Header(status, now, width)}, body...) - return strings.Join(append(lines, m.footer(width)), "\n") + return strings.Join(append(lines, m.footer(footerKeys, width)), "\n") } // mark is line, selected when item i is under the cursor. @@ -78,13 +81,7 @@ func OpenTarget(c model.Chain, pipelineURL func(model.Env) string) string { case f.Slot != nil: u = pipelineURL(f.Slot.Env) case f.PR != nil && f.Kind == model.FlagCheckFailed: - u = ChecksPage(*f.PR) - for _, ch := range f.PR.Failing { - if ch.URL != "" { - u = ch.URL - break - } - } + u = checksTarget(*f.PR) case f.PR != nil: u = f.PR.URL } @@ -94,14 +91,27 @@ func OpenTarget(c model.Chain, pipelineURL func(model.Env) string) string { return u } -// footer is the key line, then the open error and the warnings count, if -// any, cut to width. The -// key line gives way first. -func (m App) footer(width int) string { +// checksTarget is the page for pr's failing checks: the first failing check's +// own page, else ChecksPage(pr). +func checksTarget(pr model.PR) string { + for _, ch := range pr.Failing { + if ch.URL != "" { + return ch.URL + } + } + return ChecksPage(pr) +} + +// footer is the key line keys, then the open error, the note and the +// warnings count, if any, cut to width. The key line gives way first. +func (m App) footer(footerKeys string, width int) string { var extra []string if m.err != "" { extra = append(extra, m.styles.red.Render(clean(m.err))) } + if m.note != "" { + extra = append(extra, m.styles.dim.Render(clean(m.note))) + } if m.snap != nil && len(m.snap.Warnings) > 0 { n := len(m.snap.Warnings) extra = append(extra, m.styles.dim.Render(fmt.Sprintf("%d %s", n, plural(n, "warning")))) diff --git a/internal/ui/list_test.go b/internal/ui/list_test.go index 7bff297..299bbfc 100644 --- a/internal/ui/list_test.go +++ b/internal/ui/list_test.go @@ -124,6 +124,10 @@ func key(s string) tea.KeyMsg { return tea.KeyMsg{Type: tea.KeyEnter} case "ctrl+c": return tea.KeyMsg{Type: tea.KeyCtrlC} + case "esc": + return tea.KeyMsg{Type: tea.KeyEsc} + case "backspace": + return tea.KeyMsg{Type: tea.KeyBackspace} } return tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(s)} } @@ -522,16 +526,19 @@ func TestAppWaitsForSnapshots(t *testing.T) { } func TestAppInertKeys(t *testing.T) { - // Keys for later tasks do nothing yet, loaded or not, and o, j and k on - // an empty list are safe. + // Keys for later tasks do nothing yet, loaded or not, and enter, o, j + // and k on an empty list are safe. for _, m := range []ui.App{newApp(t), update(t, newApp(t), resolver.Snapshot{}), update(t, newApp(t), fxSnapshot())} { before := m.View() - if m = press(t, m, "enter", "f", "a", "x", "t"); m.View() != before { - t.Errorf("enter/f/a/x/t changed the view:\n%s", m.View()) + if m = press(t, m, "f", "a", "x", "t"); m.View() != before { + t.Errorf("f/a/x/t changed the view:\n%s", m.View()) } } for _, m := range []ui.App{newApp(t), update(t, newApp(t), resolver.Snapshot{})} { - press(t, m, "j", "k", "o") // noOpen fails on any open + before := m.View() + if m = press(t, m, "enter", "j", "k", "o"); m.View() != before { // noOpen fails on any open + t.Errorf("enter/j/k/o changed an empty list:\n%s", m.View()) + } } } From fd8ae3f62dd900219f4747e356db05eb43088558 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:10:33 -0400 Subject: [PATCH 09/21] feat(ui): scroll the detail screen A detail taller than the screen scrolls between the fixed header and footer: the selection stays in view, the first item shows the ticket line, and down past the last item (or with nothing selectable) scrolls on so the flags can be read on a short screen. Untrusted text on the detail screen is covered by a test. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/app.go | 9 ++-- internal/ui/detail.go | 84 ++++++++++++++++++++++++++++++-------- internal/ui/detail_test.go | 62 ++++++++++++++++++++++++++++ internal/ui/list.go | 6 ++- 4 files changed, 140 insertions(+), 21 deletions(-) diff --git a/internal/ui/app.go b/internal/ui/app.go index d04527a..a185b1c 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -25,9 +25,10 @@ type App struct { offset int // the first body line shown width, height int - detail string // the ticket key whose detail screen is open, "" for the list - dsel int // the selected item of the detail screen - note string // why the detail screen closed, until the next key + detail string // the ticket key whose detail screen is open, "" for the list + dsel int // the selected item of the detail screen + doffset int // the first detail line shown + note string // why the detail screen closed, until the next key } // NewApp builds the App. snaps is the resolver's out channel, refresh its @@ -115,7 +116,7 @@ func (m App) key(msg tea.KeyMsg) (App, tea.Cmd) { } case key.Matches(msg, keys.enter): if m.snap != nil && m.cursor < len(m.snap.Chains) { - m.detail, m.dsel = m.snap.Chains[m.cursor].Ticket.Key, 0 + m.detail, m.dsel, m.doffset = m.snap.Chains[m.cursor].Ticket.Key, 0, 0 } case key.Matches(msg, keys.open): if u := m.target(); u != "" { diff --git a/internal/ui/detail.go b/internal/ui/detail.go index bb62044..28ea4ac 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -36,10 +36,23 @@ func (m App) detailKey(msg tea.KeyMsg) (App, tea.Cmd) { case key.Matches(msg, keys.back): m.detail = "" case key.Matches(msg, keys.up): - m.dsel = max(m.dsel-1, 0) + // Above the first item (or with none) the window scrolls instead. + if m.dsel == 0 { + m.doffset-- + } else { + m.dsel-- + m = m.showSelection() + } case key.Matches(msg, keys.down): + // Past the last item (or with none) the window scrolls instead, so + // the flags below can be read on a short screen. c, _ := m.chain(m.detail) - m.dsel = min(m.dsel+1, max(len(detailItems(c))-1, 0)) + if m.dsel >= len(detailItems(c))-1 { + m.doffset++ + } else { + m.dsel++ + m = m.showSelection() + } case key.Matches(msg, keys.open): c, _ := m.chain(m.detail) if u := detailTarget(c, m.dsel); u != "" { @@ -69,12 +82,12 @@ func (m App) refreshDetail(before model.Chain) App { for i, it := range items { if it.id() == id { m.dsel = i - return m + return m.showSelection() } } } m.dsel = max(min(m.dsel, len(items)-1), 0) - return m + return m.showSelection() } // id names the item so the selection can find it in the next snapshot: @@ -130,7 +143,21 @@ func detailTarget(c model.Chain, i int) string { } // detailView is the header, the detail of the open ticket, and the footer. +// When the detail is taller than the screen it scrolls; the header and +// footer stay put. func (m App) detailView(width int) string { + body, _ := m.detailBody(width) + if budget := m.bodyHeight(); budget >= 0 && len(body) > budget { + body = body[m.doffset:min(m.doffset+budget, len(body))] + } + lines := append([]string{m.styles.Header(m.snap.Status, m.now(), width)}, body...) + return strings.Join(append(lines, m.footer(detailKeys, width)), "\n") +} + +// detailBody is the detail screen's lines, each width cells, and the index +// of the selected item's line (-1 with nothing to select): the ticket line, +// the stale line, one line per PR and per applying env, then the flags. +func (m App) detailBody(width int) ([]string, int) { c, _ := m.chain(m.detail) now := m.now() body := []string{ticketLine(c, width)} @@ -145,6 +172,7 @@ func (m App) detailView(width int) string { stageW = max(stageW, ansi.StringWidth(clean(it.slot.Env.Stage))) } } + sel := -1 for i, it := range items { var line string if it.slot != nil { @@ -152,16 +180,48 @@ func (m App) detailView(width int) string { } else { line = " " + prLine(*it.pr, now) } - body = append(body, m.detailMark(i, fit(line, width))) + line = fit(line, width) + if i == m.dsel { + sel, line = len(body), m.styles.selected(line) + } + body = append(body, line) } if len(c.Flags) > 0 { - body = append(body, "Flags") + body = append(body, fit("Flags", width)) for _, f := range c.Flags { body = append(body, fit(" "+m.styles.marker(f.Level)+" "+clean(f.Reason), width)) } } - lines := append([]string{m.styles.Header(m.snap.Status, m.now(), width)}, body...) - return strings.Join(append(lines, m.footer(detailKeys, width)), "\n") + return body, sel +} + +// showSelection scrolls the detail just enough to show the selected line, +// and to the top on the first item so the ticket line shows. +func (m App) showSelection() App { + budget := m.bodyHeight() + _, sel := m.detailBody(defaultWidth) + switch { + case budget < 0 || sel < 0: + case m.dsel == 0: + m.doffset = max(sel-budget+1, 0) + case sel < m.doffset: + m.doffset = sel + case sel >= m.doffset+budget: + m.doffset = sel - budget + 1 + } + return m +} + +// detailScroll keeps the detail's window within its lines. +func (m App) detailScroll() App { + budget := m.bodyHeight() + if budget < 0 { + m.doffset = 0 + return m + } + body, _ := m.detailBody(defaultWidth) + m.doffset = min(max(m.doffset, 0), max(len(body)-budget, 0)) + return m } // ticketLine is the key and title, with "Jira: " at the right edge. @@ -179,14 +239,6 @@ func ticketLine(c model.Chain, width int) string { return fit(fit(left, avail)+" "+right, width) } -// detailMark is line, selected when it is the detail screen's item i. -func (m App) detailMark(i int, line string) string { - if i == m.dsel { - return m.styles.selected(line) - } - return line -} - // prLine is "PR #330 acme/app merged 2d ago ✓ checks ✓ approved": the // PR's state, checks and review, and "draft" for a draft. func prLine(p model.PR, now time.Time) string { diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index ed25390..59cef3e 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -304,3 +304,65 @@ func TestDetailTicketGone(t *testing.T) { t.Errorf("the next key clears the note: %q", ls[len(ls)-1]) } } + +func TestDetailScroll(t *testing.T) { + // 8 rows: the header, 6 body lines and the footer, for 10 body lines. + m := update(t, inDetail(t), tea.WindowSizeMsg{Width: 100, Height: 8}) + body := func(m ui.App) string { + ls := trimmed(m) + if len(ls) != 8 { + t.Fatalf("the view is %d lines, want 8:\n%s", len(ls), m.View()) + } + if ls[0] != ui.RenderHeader(fxDetailSnapshot().Status, now, 100) || ls[7] != detailKeys { + t.Errorf("the header and footer stay put:\n%s", m.View()) + } + return strings.Join(ls[1:7], "\n") + } + first := func(m ui.App) string { return trimmed(m)[1] } + + if !strings.HasPrefix(first(m), "ABC-19") { + t.Errorf("opens at the top:\n%s", m.View()) + } + m = press(t, m, "j", "j", "j", "j", "j") // to prod-acct Production, the last item + if got := cursorLine(t, m); !strings.Contains(got, "prod-acct Production") { + t.Fatalf("selection: %q", got) + } + if !strings.HasPrefix(first(m), "> PR #330") && !strings.HasPrefix(first(m), " PR #330") { + t.Errorf("scrolled just enough to show the selection:\n%s", body(m)) + } + m = press(t, m, "j", "j", "j", "j", "j") // past the last item: the flags scroll in + if b := body(m); !strings.HasSuffix(b, "◐ stage-acct Production awaiting approval") { + t.Errorf("the last flag can be reached:\n%s", b) + } + cursorLine(t, m) // the selection is still on screen + m = press(t, m, "k", "k", "k", "k", "k") + if got := cursorLine(t, m); !strings.Contains(got, "PR #330") || !strings.HasPrefix(first(m), "ABC-19") { + t.Errorf("back at the first item the ticket line shows:\n%s", body(m)) + } +} + +func TestDetailCleansText(t *testing.T) { + const evil = "\x1b]52;c;aGk=\x07x\ny\x9b" + snap := fxDetailSnapshot() + c := &snap.Chains[1] + c.Ticket.Title, c.Ticket.Status = "Title"+evil, "Review"+evil + c.Stale, c.StaleReason = true, "acme/app"+evil + c.PRs[0].Repo = "acme/app" + evil + c.PRs[1].Failing[0].Name = "rspec" + evil + c.Slots[0].Env.Account, c.Slots[0].Env.Stage, c.Slots[0].SHA = "stage-acct"+evil, "Test"+evil, "\x1b[31maaaa1111" + c.Flags[0].Reason = "failed" + evil + m := press(t, update(t, newApp(t), snap), "j", "enter") + v := m.View() + if strings.ContainsAny(v, "\x1b\x07") || strings.Contains(v, "\x9b") { + t.Errorf("escape sequences and controls are stripped: %q", v) + } + if n := len(viewLines(m)); n != 13 { + t.Errorf("newlines in the data do not add lines: %d lines\n%s", n, v) + } + for _, want := range []string{"Titlex y", "Jira: Reviewx y", "~ stale: acme/appx y", "acme/appx y", "rspecx y", + "stage-acctx y Testx y", "✓ deployed 1d ago (aaaa111)", "● failedx y"} { + if !strings.Contains(v, want) { + t.Errorf("missing %q in\n%s", want, v) + } + } +} diff --git a/internal/ui/list.go b/internal/ui/list.go index 282fcb2..4fc0815 100644 --- a/internal/ui/list.go +++ b/internal/ui/list.go @@ -141,8 +141,12 @@ func (m App) bodyHeight() int { return max(m.height-2, 1) } -// scroll moves the window of body lines just enough to show the cursor. +// scroll moves the window of body lines just enough to show the cursor. On +// the detail screen it keeps the detail's window within its lines. func (m App) scroll() App { + if m.detail != "" { + return m.detailScroll() + } budget := m.bodyHeight() if budget < 0 || m.snap == nil { m.offset = 0 From 8b611e4d814c2bf8f02f417310dbb4a931105599 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:13:33 -0400 Subject: [PATCH 10/21] fix(ui): mark a rolled-back env's commit as the live one On a rolled-back slot, link sets SHA from the deploy that replaced the chain's commit, so the detail line reads "(now dddd444)" instead of looking like the chain's own commit. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/detail.go | 20 ++++++++++++++------ internal/ui/detail_test.go | 2 +- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/internal/ui/detail.go b/internal/ui/detail.go index 28ea4ac..90523e9 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -293,7 +293,7 @@ func ago(at, now time.Time) string { // slotText is where the work stands in s's env: "✓ deployed 1d ago // (aaaa111) ECS 2/2", "‖ awaiting approval since 1d", "in progress since // 2m (cccc333)", "✗ failed 3h ago (bbbb222)", "✗ rejected or expired …", -// "✗ rolled back …", "deploy unknown" or "not yet". Unknown times and SHAs +// "✗ rolled back 2d ago (now dddd444)", "deploy unknown" or "not yet". Unknown times and SHAs // are left out. func slotText(s model.EnvSlot, now time.Time) string { switch s.State { @@ -320,7 +320,13 @@ func slotText(s model.EnvSlot, now time.Time) string { } return "✗ failed" + ago(s.At, now) + sha(s.SHA) case model.SlotRolledBack: - return "✗ rolled back" + ago(s.At, now) + sha(s.SHA) + // Here SHA is the commit that replaced the chain's (link sets it from + // the newer deploy), so it is marked "now". + t := "✗ rolled back" + ago(s.At, now) + if h := shortSHA(s.SHA); h != "" { + t += " (now " + h + ")" + } + return t case model.SlotUnknown: return "deploy unknown" } @@ -329,9 +335,11 @@ func slotText(s model.EnvSlot, now time.Time) string { // sha is " (aaaa111)", the commit cut to 7 cells, or "" when unknown. func sha(s string) string { - s = clean(s) - if s == "" { - return "" + if h := shortSHA(s); h != "" { + return " (" + h + ")" } - return " (" + ansi.Truncate(s, 7, "") + ")" + return "" } + +// shortSHA is the commit, cleaned and cut to 7 cells. +func shortSHA(s string) string { return ansi.Truncate(clean(s), 7, "") } diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index 59cef3e..3ddf6a4 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -97,7 +97,7 @@ func TestDetailEnvStates(t *testing.T) { {"rejected", model.EnvSlot{State: model.SlotFailed, SHA: "bbbb2222", At: ago(3 * time.Hour), Deploy: &model.Deploy{Status: model.DeployRejected}}, "✗ rejected or expired 3h ago (bbbb222)"}, {"rolled back", model.EnvSlot{State: model.SlotRolledBack, SHA: "dddd4444", At: ago(48 * time.Hour)}, - "✗ rolled back 2d ago (dddd444)"}, + "✗ rolled back 2d ago (now dddd444)"}, // SHA is the commit that replaced it {"unknown", model.EnvSlot{State: model.SlotUnknown}, "deploy unknown"}, {"not yet", model.EnvSlot{State: model.SlotNotYet}, "not yet"}, } From 2f710ebeae4f2c127775b066df067a22a848a2ce Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:17:56 -0400 Subject: [PATCH 11/21] feat(ui): refresh on detail r asks for a refresh on the detail screen too, and its footer says so. o acts on the selected item only while it is on screen; when scrolling has hidden it, o opens the ticket. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/app.go | 13 +++++++++---- internal/ui/detail.go | 17 +++++++++++++++-- internal/ui/detail_test.go | 38 +++++++++++++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/internal/ui/app.go b/internal/ui/app.go index a185b1c..4f9ca17 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -110,10 +110,7 @@ func (m App) key(msg tea.KeyMsg) (App, tea.Cmd) { case key.Matches(msg, keys.down): m.cursor = min(m.cursor+1, max(m.items()-1, 0)) case key.Matches(msg, keys.refresh): - select { - case m.refresh <- struct{}{}: - default: // a refresh is already pending - } + m.requestRefresh() case key.Matches(msg, keys.enter): if m.snap != nil && m.cursor < len(m.snap.Chains) { m.detail, m.dsel, m.doffset = m.snap.Chains[m.cursor].Ticket.Key, 0, 0 @@ -177,3 +174,11 @@ func (m App) target() string { } return "" } + +// requestRefresh asks the resolver to poll now, without blocking. +func (m App) requestRefresh() { + select { + case m.refresh <- struct{}{}: + default: // a refresh is already pending + } +} diff --git a/internal/ui/detail.go b/internal/ui/detail.go index 90523e9..5e17150 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -13,7 +13,7 @@ import ( ) // detailKeys is the key line at the bottom of the detail screen. -const detailKeys = "↑/↓ move o open esc back q quit" +const detailKeys = "↑/↓ move o open r refresh esc back q quit" // chain is the chain whose ticket key is key, and whether there is one. func (m App) chain(key string) (model.Chain, bool) { @@ -35,6 +35,8 @@ func (m App) detailKey(msg tea.KeyMsg) (App, tea.Cmd) { return m, tea.Quit case key.Matches(msg, keys.back): m.detail = "" + case key.Matches(msg, keys.refresh): + m.requestRefresh() case key.Matches(msg, keys.up): // Above the first item (or with none) the window scrolls instead. if m.dsel == 0 { @@ -55,7 +57,11 @@ func (m App) detailKey(msg tea.KeyMsg) (App, tea.Cmd) { } case key.Matches(msg, keys.open): c, _ := m.chain(m.detail) - if u := detailTarget(c, m.dsel); u != "" { + sel := m.dsel + if !m.selectionVisible() { + sel = -1 // o acts only on a selection you can see: else the ticket + } + if u := detailTarget(c, sel); u != "" { if err := m.openURL(u); err != nil { m.err = "open failed: " + err.Error() } @@ -212,6 +218,13 @@ func (m App) showSelection() App { return m } +// selectionVisible reports whether the selected line is on screen. +func (m App) selectionVisible() bool { + _, sel := m.detailBody(defaultWidth) + budget := m.bodyHeight() + return sel >= 0 && (budget < 0 || sel >= m.doffset && sel < m.doffset+budget) +} + // detailScroll keeps the detail's window within its lines. func (m App) detailScroll() App { budget := m.bodyHeight() diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index 3ddf6a4..27dfb22 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -13,7 +13,7 @@ import ( ) // detailKeys is the detail screen's key line. -const detailKeys = "↑/↓ move o open esc back q quit" +const detailKeys = "↑/↓ move o open r refresh esc back q quit" var ( stageTest = model.Env{Account: "stage-acct", Region: "us-east-1", Pipeline: "app-pipeline", Stage: "Test", Order: 0} @@ -366,3 +366,39 @@ func TestDetailCleansText(t *testing.T) { } } } + +func TestDetailRefreshKey(t *testing.T) { + refresh := make(chan struct{}, 1) + m := ui.NewApp(make(chan resolver.Snapshot), refresh, noOpen(t)).WithClock(func() time.Time { return now }) + m = press(t, update(t, m, fxDetailSnapshot()), "j", "enter", "r") + if len(refresh) != 1 { + t.Fatalf("r on the detail screen sends on the refresh channel") + } + m = press(t, m, "r") // the buffer is full: this must not block + if ls := trimmed(m); !strings.HasPrefix(ls[1], "ABC-19") { + t.Errorf("r stays on the detail screen:\n%s", m.View()) + } +} + +func TestDetailOpenHiddenSelection(t *testing.T) { + snap := fxDetailSnapshot() + c := &snap.Chains[1] + for range 4 { + c.Flags = append(c.Flags, model.Flag{Level: model.Yellow, Kind: model.FlagStaleReview, Reason: "more"}) + } + o := &opened{} + m := update(t, detailWith(t, o, snap), tea.WindowSizeMsg{Width: 100, Height: 8}) + m = press(t, m, "j", "j", "j", "j", "j") // the last item, still on screen + m = press(t, m, "o") + m = press(t, m, "j", "j", "j", "j", "j", "j") // scroll on until it is hidden + for _, l := range viewLines(m) { + if strings.HasPrefix(l, ">") { + t.Fatalf("the selection should be scrolled out:\n%s", m.View()) + } + } + press(t, m, "o") + want := []string{resolver.PipelineURL(prodProd), "https://jira.example.com/browse/ABC-19"} + if strings.Join(o.urls, " ") != strings.Join(want, " ") { + t.Errorf("opened\n got %q\nwant %q", o.urls, want) + } +} From 25b0cbee348512770a294b30670f11637541e562 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:20:41 -0400 Subject: [PATCH 12/21] fix(ui): list review fixes - o opens the browser from a command, not from Update; a failure comes back as openFailedMsg and shows in the footer (list and detail). - Only absolute http and https URLs are opened; anything else falls back to the ticket, or opens nothing. - Without color the cursor's ">" sits next to the marker instead of replacing it. - Tiny heights no longer push out the header (bodyHeight floors at 0). - A nil opener opens nothing instead of panicking. - The Unlinked label is dimmed. - Tests for a snapshot shorter than the scroll offset and for an escape-laden unlinked title. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/app.go | 58 +++++++++++++++-- internal/ui/detail.go | 6 +- internal/ui/detail_test.go | 10 +-- internal/ui/list.go | 12 ++-- internal/ui/list_test.go | 128 +++++++++++++++++++++++++++++++++++-- internal/ui/styles.go | 4 +- 6 files changed, 190 insertions(+), 28 deletions(-) diff --git a/internal/ui/app.go b/internal/ui/app.go index 4f9ca17..1a134c1 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -2,6 +2,7 @@ package ui import ( "fmt" + "net/url" "strings" "time" @@ -33,8 +34,11 @@ type App struct { // NewApp builds the App. snaps is the resolver's out channel, refresh its // refresh channel (sent without blocking), and openURL opens a page in the -// browser. +// browser (nil opens nothing). func NewApp(snaps <-chan resolver.Snapshot, refresh chan<- struct{}, openURL func(string) error) App { + if openURL == nil { + openURL = func(string) error { return nil } + } return App{styles: DefaultStyles(), now: time.Now, snaps: snaps, refresh: refresh, openURL: openURL} } @@ -78,6 +82,8 @@ func (m App) update(msg tea.Msg) (App, tea.Cmd) { m = m.refreshDetail(before) } return m, waitForSnapshot(m.snaps) + case openFailedMsg: + m.err = "open failed: " + msg.err.Error() case tea.KeyMsg: return m.key(msg) } @@ -116,11 +122,7 @@ func (m App) key(msg tea.KeyMsg) (App, tea.Cmd) { m.detail, m.dsel, m.doffset = m.snap.Chains[m.cursor].Ticket.Key, 0, 0 } case key.Matches(msg, keys.open): - if u := m.target(); u != "" { - if err := m.openURL(u); err != nil { - m.err = "open failed: " + err.Error() - } - } + return m, m.open(m.target(), m.cursorTicketURL()) } return m, nil } @@ -175,6 +177,23 @@ func (m App) target() string { return "" } +// webPage reports whether u is an absolute http or https URL with a host. +// Anything else (a file: URL, or "-x" that a browser launcher could take for +// a flag) is never opened: the URLs come from Jira, GitHub and AWS. +func webPage(u string) bool { + p, err := url.Parse(u) + return err == nil && (p.Scheme == "http" || p.Scheme == "https") && p.Host != "" +} + +// cursorTicketURL is the ticket URL of the chain under the cursor, "" on an +// unlinked PR or with no snapshot. +func (m App) cursorTicketURL() string { + if m.snap == nil || m.cursor >= len(m.snap.Chains) { + return "" + } + return m.snap.Chains[m.cursor].Ticket.URL +} + // requestRefresh asks the resolver to poll now, without blocking. func (m App) requestRefresh() { select { @@ -182,3 +201,30 @@ func (m App) requestRefresh() { default: // a refresh is already pending } } + +// openFailedMsg reports that the browser could not open a page. +// Adapted from jira-green internal/ui/dashboard.go. +type openFailedMsg struct{ err error } + +// open is a command that opens the first of urls that is a web page (see +// webPage) in the browser, reporting a failure as openFailedMsg; nil when +// none is. Update never opens a page itself. +func (m App) open(urls ...string) tea.Cmd { + var u string + for _, c := range urls { + if webPage(c) { + u = c + break + } + } + if u == "" { + return nil + } + openURL := m.openURL + return func() tea.Msg { + if err := openURL(u); err != nil { + return openFailedMsg{err} + } + return nil + } +} diff --git a/internal/ui/detail.go b/internal/ui/detail.go index 5e17150..44bfc73 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -61,11 +61,7 @@ func (m App) detailKey(msg tea.KeyMsg) (App, tea.Cmd) { if !m.selectionVisible() { sel = -1 // o acts only on a selection you can see: else the ticket } - if u := detailTarget(c, sel); u != "" { - if err := m.openURL(u); err != nil { - m.err = "open failed: " + err.Error() - } - } + return m, m.open(detailTarget(c, sel), c.Ticket.URL) } return m, nil } diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index 27dfb22..5d167dd 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -108,7 +108,7 @@ func TestDetailEnvStates(t *testing.T) { c.Slots = []model.EnvSlot{tt.slot} snap := resolver.Snapshot{Chains: []model.Chain{c}, At: now} ls := trimmed(press(t, update(t, newApp(t), snap), "enter")) - if want := "> stage-acct Test " + tt.want; ls[2] != want { + if want := " >stage-acct Test " + tt.want; ls[2] != want { t.Errorf("\n got %q\nwant %q", ls[2], want) } }) @@ -120,7 +120,7 @@ func TestDetailView(t *testing.T) { want := []string{ ui.RenderHeader(snap.Status, now, 100), "ABC-19 Drop the form-level opt-outs" + strings.Repeat(" ", 100-36-17) + "Jira: Code Review", - "> PR #330 acme/app merged 2d ago ✓ checks ✓ approved", + " >PR #330 acme/app merged 2d ago ✓ checks ✓ approved", " PR #331 acme/app open ✗ rspec, CodeQL changes requested draft", " stage-acct Test ✓ deployed 1d ago (aaaa111) ECS 2/2", " stage-acct Production ‖ awaiting approval since 1d", @@ -145,7 +145,7 @@ func TestDetailStale(t *testing.T) { if want := "~ stale: acme/app not refreshed since 11:58:00"; ls[2] != want { t.Errorf("stale line\n got %q\nwant %q", ls[2], want) } - if !strings.HasPrefix(ls[3], "> PR #330") { + if !strings.HasPrefix(ls[3], " >PR #330") { t.Errorf("the PRs follow the stale line: %q", ls[3]) } @@ -327,7 +327,7 @@ func TestDetailScroll(t *testing.T) { if got := cursorLine(t, m); !strings.Contains(got, "prod-acct Production") { t.Fatalf("selection: %q", got) } - if !strings.HasPrefix(first(m), "> PR #330") && !strings.HasPrefix(first(m), " PR #330") { + if !strings.HasPrefix(first(m), " >PR #330") && !strings.HasPrefix(first(m), " PR #330") { t.Errorf("scrolled just enough to show the selection:\n%s", body(m)) } m = press(t, m, "j", "j", "j", "j", "j") // past the last item: the flags scroll in @@ -392,7 +392,7 @@ func TestDetailOpenHiddenSelection(t *testing.T) { m = press(t, m, "o") m = press(t, m, "j", "j", "j", "j", "j", "j") // scroll on until it is hidden for _, l := range viewLines(m) { - if strings.HasPrefix(l, ">") { + if isCursor(l) { t.Fatalf("the selection should be scrolled out:\n%s", m.View()) } } diff --git a/internal/ui/list.go b/internal/ui/list.go index 4fc0815..2491253 100644 --- a/internal/ui/list.go +++ b/internal/ui/list.go @@ -42,7 +42,7 @@ func (m App) View() string { body = append(body, m.mark(i, m.styles.Row(c, l, now))) } if n := len(m.snap.Unlinked); n > 0 { - body = append(body, ansi.Truncate(fmt.Sprintf("── Unlinked (%d) ──", n), width, "…")) + body = append(body, m.styles.dim.Render(ansi.Truncate(fmt.Sprintf("── Unlinked (%d) ──", n), width, "…"))) for i, p := range m.snap.Unlinked { // Exactly width cells, so the cursor's reverse video spans it. line := " " + fit(fmt.Sprintf(" %s#%d %s", clean(p.Repo), p.Number, clean(p.Title)), width-1) @@ -102,9 +102,9 @@ func checksTarget(pr model.PR) string { return ChecksPage(pr) } -// footer is the key line keys, then the open error, the note and the +// footer is keyLine, then the open error, the note and the // warnings count, if any, cut to width. The key line gives way first. -func (m App) footer(footerKeys string, width int) string { +func (m App) footer(keyLine string, width int) string { var extra []string if m.err != "" { extra = append(extra, m.styles.red.Render(clean(m.err))) @@ -117,10 +117,10 @@ func (m App) footer(footerKeys string, width int) string { extra = append(extra, m.styles.dim.Render(fmt.Sprintf("%d %s", n, plural(n, "warning")))) } if len(extra) == 0 { - return ansi.Truncate(footerKeys, width, "…") + return ansi.Truncate(keyLine, width, "…") } tail := " " + strings.Join(extra, " ") - keys := ansi.Truncate(footerKeys, max(width-ansi.StringWidth(tail), 0), "…") + keys := ansi.Truncate(keyLine, max(width-ansi.StringWidth(tail), 0), "…") return ansi.Truncate(keys+tail, width, "…") } @@ -138,7 +138,7 @@ func (m App) bodyHeight() int { if m.height <= 0 { return -1 } - return max(m.height-2, 1) + return max(m.height-2, 0) } // scroll moves the window of body lines just enough to show the cursor. On diff --git a/internal/ui/list_test.go b/internal/ui/list_test.go index 299bbfc..3afa70d 100644 --- a/internal/ui/list_test.go +++ b/internal/ui/list_test.go @@ -95,7 +95,7 @@ func TestAppRows(t *testing.T) { l := ui.NewLayout(snap.Chains, now, 100) want := []string{ ui.RenderHeader(snap.Status, now, 100), - ">" + ansi.TruncateLeft(ui.RenderRow(snap.Chains[0], l, now), 1, ""), // the cursor row + "●>" + ansi.TruncateLeft(ui.RenderRow(snap.Chains[0], l, now), 2, ""), // the cursor row, next to its marker ui.RenderRow(snap.Chains[1], l, now), ui.RenderRow(snap.Chains[2], l, now), "── Unlinked (1) ──", @@ -132,21 +132,33 @@ func key(s string) tea.KeyMsg { return tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune(s)} } +// press sends each key, runs the command it returns (an open, say) and +// feeds that command's message back, as Bubble Tea would. func press(t *testing.T, m ui.App, keys ...string) ui.App { t.Helper() for _, k := range keys { - m = update(t, m, key(k)) + got, cmd := m.Update(key(k)) + m = got.(ui.App) + if cmd != nil { + if msg := cmd(); msg != nil { + m = update(t, m, msg) + } + } } return m } -// cursorLine is the line the ASCII cursor (">" in the first cell) is on, +// isCursor reports whether l carries the ASCII cursor: ">" in the second +// cell, next to the marker. +func isCursor(l string) bool { return ansi.Cut(l, 1, 2) == ">" } + +// cursorLine is the line the ASCII cursor (">" in the second cell) is on, // failing unless exactly one line has it. func cursorLine(t *testing.T, m ui.App) string { t.Helper() var found []string for _, l := range viewLines(m) { - if strings.HasPrefix(l, ">") { + if isCursor(l) { found = append(found, l) } } @@ -386,6 +398,114 @@ func TestAppOpen(t *testing.T) { } } +func TestAppOpenIsACommand(t *testing.T) { + o := &opened{err: errors.New("no browser")} + m := ui.NewApp(make(chan resolver.Snapshot), make(chan struct{}, 1), o.open).WithClock(func() time.Time { return now }) + m = update(t, update(t, m, tea.WindowSizeMsg{Width: 100, Height: 30}), fxSnapshot()) + for _, mm := range []ui.App{m, press(t, m, "enter")} { // the list, then the detail + o.urls = nil + got, cmd := mm.Update(key("o")) + if len(o.urls) != 0 { + t.Fatalf("Update opened %q itself; the open belongs in a command", o.urls) + } + if cmd == nil { + t.Fatal("o returns no command") + } + msg := cmd() + if len(o.urls) != 1 { + t.Fatalf("the command opens one page, opened %q", o.urls) + } + after := update(t, got.(ui.App), msg) + if ls := viewLines(after); !strings.Contains(ls[len(ls)-1], "open failed: no browser") { + t.Errorf("the command's failure shows in the footer: %q", ls[len(ls)-1]) + } + } +} + +func TestAppOpensOnlyWebPages(t *testing.T) { + snap := fxSnapshot() + snap.Chains[0].PRs[0].Failing[0].URL = "file:///etc/passwd" // ABC-1's flag target + snap.Unlinked[0].URL = "-x" // no ticket to fall back to + snap.Chains[2].Ticket.URL = "file:///tmp/x" // ABC-3: the ticket is no good either + o := &opened{} + m := ui.NewApp(make(chan resolver.Snapshot), make(chan struct{}, 1), o.open).WithClock(func() time.Time { return now }) + m = update(t, update(t, m, tea.WindowSizeMsg{Width: 100, Height: 30}), snap) + m = press(t, m, "o", "j", "j", "o", "j", "o") + // Detail: a PR whose URL is not a web page falls back to the ticket. + snap.Chains[0].PRs[0].URL = "-x" + snap.Chains[0].PRs[0].Checks, snap.Chains[0].PRs[0].Failing = model.ChecksPassing, nil + m = press(t, update(t, press(t, m, "k", "k", "k"), snap), "enter", "o") + want := []string{"https://jira.example.com/browse/ABC-1", "https://jira.example.com/browse/ABC-1"} + if strings.Join(o.urls, " ") != strings.Join(want, " ") { + t.Errorf("opened\n got %q\nwant %q", o.urls, want) + } + if foot := viewLines(m)[len(viewLines(m))-1]; foot != "↑/↓ move o open r refresh esc back q quit" { + t.Errorf("no error for a page that was not opened: %q", foot) + } +} + +func TestAppTinyHeight(t *testing.T) { + snap := fxSnapshot() + for _, m := range []ui.App{update(t, newApp(t), snap), press(t, update(t, newApp(t), snap), "enter")} { + m = update(t, m, tea.WindowSizeMsg{Width: 100, Height: 2}) + ls := viewLines(m) + if len(ls) != 2 || ls[0] != ui.RenderHeader(snap.Status, now, 100) { + t.Errorf("2 rows hold the header and the footer only:\n%s", m.View()) + } + } +} + +func TestAppNilOpener(t *testing.T) { + m := ui.NewApp(make(chan resolver.Snapshot), make(chan struct{}, 1), nil).WithClock(func() time.Time { return now }) + m = press(t, update(t, m, fxSnapshot()), "o", "enter", "o") // must not panic + if ls := viewLines(m); strings.Contains(ls[len(ls)-1], "failed") { + t.Errorf("a nil opener opens nothing, quietly: %q", ls[len(ls)-1]) + } +} + +func TestAppUnlinkedLabelDim(t *testing.T) { + m := update(t, newApp(t).WithStyles(terminal(env{"TERM": "xterm-256color"})), fxSnapshot()) + label := viewLines(m)[4] + if !strings.HasPrefix(label, "\x1b[2m") || ansi.Strip(label) != "── Unlinked (1) ──" { + t.Errorf("the Unlinked label is dimmed: %q", label) + } +} + +func TestAppSnapshotShorterThanOffset(t *testing.T) { + long := fxSnapshot() + long.Unlinked = nil + for i := range 10 { + long.Chains = append(long.Chains, model.Chain{Ticket: model.Ticket{Key: fmt.Sprintf("ABC-%d", 10+i), Title: "More"}}) + } + m := update(t, newApp(t), tea.WindowSizeMsg{Width: 100, Height: 6}) // 4 body lines + m = update(t, m, long) + m = press(t, m, "j", "j", "j", "j", "j", "j", "j", "j", "j", "j", "j", "j") // scrolled to the bottom + short := fxSnapshot() + short.Chains, short.Unlinked = short.Chains[:2], nil + m = update(t, m, short) + ls := viewLines(m) + if len(ls) != 4 || !strings.Contains(ls[1], "ABC-1") || !strings.Contains(ls[2], "ABC-2") { + t.Errorf("a shorter snapshot scrolls back to show its rows:\n%s", m.View()) + } + if got := cursorLine(t, m); !strings.Contains(got, "ABC-2") { + t.Errorf("the cursor clamps to the last row: %q", got) + } +} + +func TestAppCleansUnlinkedTitle(t *testing.T) { + snap := fxSnapshot() + snap.Unlinked[0].Title = "Bump\x1b]52;c;aGk=\x07 deps\nnow" + snap.Unlinked[0].Repo = "acme/\x1b[31mapp" + m := update(t, newApp(t), snap) + v := m.View() + if strings.ContainsAny(v, "\x1b\x07") || len(viewLines(m)) != 7 { + t.Errorf("the unlinked line is cleaned and stays one line: %q", v) + } + if !strings.Contains(v, " acme/app#9 Bump deps now") { + t.Errorf("cleaned text shows:\n%s", v) + } +} + func TestAppWarningsCount(t *testing.T) { foot := func(warnings ...string) string { snap := fxSnapshot() diff --git a/internal/ui/styles.go b/internal/ui/styles.go index f4f02f6..d129f83 100644 --- a/internal/ui/styles.go +++ b/internal/ui/styles.go @@ -87,10 +87,10 @@ const ( // selected marks line as the cursor's: reverse video, turned back on after // every reset the line's own styles end with. Without escape codes the -// line's first cell (its marker) becomes ">". +// line's second cell, next to its marker, becomes ">", so the marker stays. func (s Styles) selected(line string) string { if s.plain { - return ">" + ansi.TruncateLeft(line, 1, "") + return ansi.Truncate(line, 1, "") + ">" + ansi.TruncateLeft(line, 2, "") } return sgrReverse + strings.ReplaceAll(line, sgrReset, sgrReset+sgrReverse) + sgrReset } From 3e8892cd7ed4b48f4bede095eb493e8982a4ed39 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:28:52 -0400 Subject: [PATCH 13/21] feat: wire config, adapters, resolver and UI main.go parses --config, --version and --help, loads the config (a missing file prints "no config at ; see README"), logs config warnings to $XDG_STATE_HOME/app-green/app-green.log (0700 dir, 0600 file, append), builds the resolver with FromConfig, and runs it in the background behind the Bubble Tea UI in the alt screen. Quitting cancels the resolver and waits up to 2s for it. o opens pages with macOS `open -u` or xdg-open, only for http(s) URLs, detached from the screen. Assisted-by: Claude Code:claude-opus-5-5 --- main.go | 241 +++++++++++++++++++++++++++++++++- main_test.go | 357 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 596 insertions(+), 2 deletions(-) create mode 100644 main_test.go diff --git a/main.go b/main.go index f96239d..f579d5b 100644 --- a/main.go +++ b/main.go @@ -1,5 +1,242 @@ +// Command app-green is a terminal app that traces Jira tickets through +// GitHub PRs to the AWS pipeline stages running them. package main -import "fmt" +import ( + "context" + "errors" + "flag" + "fmt" + "io" + "io/fs" + "log/slog" + "net/url" + "os" + "os/exec" + "path/filepath" + "runtime" + "time" -func main() { fmt.Println("app-green") } + tea "github.com/charmbracelet/bubbletea" + "github.com/ericdahl-dev/app-green/internal/config" + "github.com/ericdahl-dev/app-green/internal/resolver" + "github.com/ericdahl-dev/app-green/internal/ui" +) + +// version is set at build time with -ldflags "-X main.version=...". +var version = "dev" + +const usage = `app-green - Jira tickets traced through GitHub PRs to AWS pipeline stages + +Usage: + app-green launch the app + app-green --config use this config instead of the default + app-green --version print the version + app-green --help show this help +` + +func main() { os.Exit(run(os.Args[1:], os.Stdout, os.Stderr)) } + +func run(args []string, stdout, stderr io.Writer) int { + flags := flag.NewFlagSet("app-green", flag.ContinueOnError) + flags.SetOutput(stderr) + flags.Usage = func() {} + configPath := flags.String("config", "", "config file (default $XDG_CONFIG_HOME/app-green/config.toml)") + showVersion := flags.Bool("version", false, "print the version") + if err := flags.Parse(args); err != nil { + if errors.Is(err, flag.ErrHelp) { + _, _ = fmt.Fprint(stdout, usage) + return 0 + } + _, _ = fmt.Fprint(stderr, usage) + return 2 + } + if flags.NArg() > 0 { + _, _ = fmt.Fprintf(stderr, "app-green: unexpected argument %q\n%s", flags.Arg(0), usage) + return 2 + } + if *showVersion { + _, _ = fmt.Fprintf(stdout, "app-green %s\n", version) + return 0 + } + log, closeLog, err := startLog() + if err != nil { + _, _ = fmt.Fprintf(stderr, "app-green: %v\n", err) + return 1 + } + defer closeLog() + path := *configPath + if path == "" { + p, err := config.DefaultPath() + if err != nil { + _, _ = fmt.Fprintf(stderr, "app-green: %v\n", err) + return 1 + } + path = p + } + cfg, err := config.Load(path) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + _, _ = fmt.Fprintf(stderr, "no config at %s; see README\n", path) + return 1 + } + _, _ = fmt.Fprintf(stderr, "app-green: %v\n", err) + return 1 + } + log.Info("start", "version", version, "config", path) + for _, w := range cfg.Warnings() { + log.Warn("config", "warning", w) + } + r, err := resolver.FromConfig(context.Background(), cfg, nil, log) + if err != nil { + log.Error("resolve tokens", "err", err) + _, _ = fmt.Fprintf(stderr, "app-green: %v\n", err) + return 1 + } + if err := runApp(r, log); err != nil { + log.Error("ui", "err", err) + _, _ = fmt.Fprintf(stderr, "app-green: %v\n", err) + return 1 + } + return 0 +} + +// runner is the part of the resolver runApp drives. +type runner interface { + Run(ctx context.Context, out chan<- resolver.Snapshot, refresh <-chan struct{}) +} + +// runProgram runs m full screen until the user quits; tests replace it. +var runProgram = func(m tea.Model) error { + _, err := tea.NewProgram(m, tea.WithAltScreen()).Run() + return err +} + +// stopTimeout is how long runApp waits for the resolver after the UI quits. +var stopTimeout = 2 * time.Second + +// runApp runs the resolver in the background and the UI in front of it. +// When the UI quits (q, ctrl+c, or SIGINT/SIGTERM, which Bubble Tea +// catches), it cancels the resolver and waits up to stopTimeout for it to +// stop. +func runApp(r runner, log *slog.Logger) error { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + snaps := make(chan resolver.Snapshot) + refresh := make(chan struct{}, 1) // one pending press coalesces the rest + done := make(chan struct{}) + go func() { + defer close(done) + defer close(snaps) + r.Run(ctx, snaps, refresh) + }() + err := runProgram(ui.NewApp(snaps, refresh, openBrowser)) + cancel() + select { + case <-done: + case <-time.After(stopTimeout): + log.Warn("resolver did not stop in time", "timeout", stopTimeout) + } + if errors.Is(err, tea.ErrInterrupted) { + return nil // SIGINT from outside; SIGTERM already quits cleanly + } + return err +} + +// startLog opens the log file and makes it the default slog (and log) +// output too, so nothing a dependency logs can draw over the screen. Its +// close func restores the previous default. Tokens are never logged. +func startLog() (*slog.Logger, func(), error) { + path, err := logPath() + if err != nil { + return nil, nil, err + } + f, err := openLog(path) + if err != nil { + return nil, nil, err + } + log := slog.New(slog.NewTextHandler(f, nil)) + prev := slog.Default() + slog.SetDefault(log) + return log, func() { + slog.SetDefault(prev) + _ = f.Close() + }, nil +} + +// logPath is $XDG_STATE_HOME/app-green/app-green.log, or +// ~/.local/state/app-green/app-green.log when XDG_STATE_HOME is unset or +// relative (the XDG spec says to ignore a relative value). +func logPath() (string, error) { + base := os.Getenv("XDG_STATE_HOME") + if !filepath.IsAbs(base) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("find log path: %w", err) + } + base = filepath.Join(home, ".local", "state") + } + return filepath.Join(base, "app-green", "app-green.log"), nil +} + +// openLog opens path for appending, creating it and its directory. The +// directory is app-green's own, so both are made private (0700 and 0600) +// even when they already exist with wider modes. +func openLog(path string) (*os.File, error) { + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, fmt.Errorf("open log: %w", err) + } + if err := os.Chmod(dir, 0o700); err != nil { + return nil, fmt.Errorf("open log: %w", err) + } + f, err := os.OpenFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o600) + if err != nil { + return nil, fmt.Errorf("open log: %w", err) + } + if err := f.Chmod(0o600); err != nil { + _ = f.Close() + return nil, fmt.Errorf("open log: %w", err) + } + return f, nil +} + +// openerCommand is the command that opens u in the default browser on goos. +// u must be an absolute http or https URL with a host, so it can never be +// read as an option: neither opener accepts "--", and macOS open gets the +// URL as the value of -u. +func openerCommand(goos, u string) (string, []string, error) { + p, err := url.Parse(u) + if err != nil || (p.Scheme != "http" && p.Scheme != "https") || p.Host == "" { + return "", nil, fmt.Errorf("not a web page: %q", u) + } + switch goos { + case "darwin": + return "open", []string{"-u", u}, nil + case "windows", "plan9", "js", "wasip1": + return "", nil, fmt.Errorf("opening a browser is not supported on %s", goos) + default: + return "xdg-open", []string{u}, nil + } +} + +// openBrowser opens u in the default browser without waiting for it. +func openBrowser(u string) error { + name, args, err := openerCommand(runtime.GOOS, u) + if err != nil { + return err + } + return startCommand(name, args...) +} + +// startCommand starts name without waiting for it. Its stdin, stdout and +// stderr are the null device, so it cannot draw over the screen, and a +// goroutine reaps it so it does not linger as a zombie. Tests replace it. +var startCommand = func(name string, args ...string) error { + cmd := exec.Command(name, args...) + if err := cmd.Start(); err != nil { + return err + } + go func() { _ = cmd.Wait() }() + return nil +} diff --git a/main_test.go b/main_test.go new file mode 100644 index 0000000..168db47 --- /dev/null +++ b/main_test.go @@ -0,0 +1,357 @@ +package main + +import ( + "bytes" + "context" + "errors" + "log/slog" + "os" + "path/filepath" + "runtime" + "slices" + "strings" + "testing" + "time" + + tea "github.com/charmbracelet/bubbletea" + "github.com/ericdahl-dev/app-green/internal/resolver" + "github.com/ericdahl-dev/app-green/internal/ui" +) + +// isolate points every XDG directory at a fresh temp dir, so a test never +// reads a real config or writes a real log. +func isolate(t *testing.T) { + t.Helper() + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + t.Setenv("XDG_STATE_HOME", t.TempDir()) +} + +func TestRunVersion(t *testing.T) { + isolate(t) + var out, errOut bytes.Buffer + if code := run([]string{"--version"}, &out, &errOut); code != 0 { + t.Fatalf("exit %d, stderr %q", code, errOut.String()) + } + if got := out.String(); got != "app-green dev\n" { + t.Fatalf("got %q", got) + } +} + +func TestRunHelp(t *testing.T) { + isolate(t) + var out, errOut bytes.Buffer + if code := run([]string{"--help"}, &out, &errOut); code != 0 { + t.Fatalf("exit %d", code) + } + for _, want := range []string{"--config", "--version", "--help"} { + if !strings.Contains(out.String(), want) { + t.Errorf("help missing %q: %q", want, out.String()) + } + } + if errOut.Len() != 0 { + t.Errorf("stderr %q", errOut.String()) + } +} + +func TestRunBadArgs(t *testing.T) { + for _, args := range [][]string{{"--nope"}, {"extra"}} { + t.Run(strings.Join(args, " "), func(t *testing.T) { + isolate(t) + var out, errOut bytes.Buffer + if code := run(args, &out, &errOut); code != 2 { + t.Fatalf("exit %d", code) + } + if !strings.Contains(errOut.String(), "Usage:") { + t.Errorf("stderr %q", errOut.String()) + } + }) + } +} + +func TestRunMissingConfigFlag(t *testing.T) { + isolate(t) + path := filepath.Join(t.TempDir(), "nope.toml") + var out, errOut bytes.Buffer + if code := run([]string{"--config", path}, &out, &errOut); code != 1 { + t.Fatalf("exit %d", code) + } + if want := "no config at " + path + "; see README\n"; errOut.String() != want { + t.Fatalf("stderr %q, want %q", errOut.String(), want) + } +} + +func TestRunMissingConfigDefaultPath(t *testing.T) { + isolate(t) + want := filepath.Join(os.Getenv("XDG_CONFIG_HOME"), "app-green", "config.toml") + var out, errOut bytes.Buffer + if code := run(nil, &out, &errOut); code != 1 { + t.Fatalf("exit %d", code) + } + if got := errOut.String(); got != "no config at "+want+"; see README\n" { + t.Fatalf("stderr %q", got) + } +} + +func TestLogPath(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + fallback := filepath.Join(home, ".local", "state", "app-green", "app-green.log") + for _, tc := range []struct{ name, xdg, want string }{ + {"xdg set", "/xdg/state", "/xdg/state/app-green/app-green.log"}, + {"unset", "", fallback}, + {"relative ignored", "rel/state", fallback}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("XDG_STATE_HOME", tc.xdg) + got, err := logPath() + if err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Fatalf("got %q, want %q", got, tc.want) + } + }) + } +} + +func TestOpenLogCreatesPrivate(t *testing.T) { + path := filepath.Join(t.TempDir(), "state", "app-green", "app-green.log") + f, err := openLog(path) + if err != nil { + t.Fatal(err) + } + _ = f.Close() + assertMode(t, filepath.Dir(path), 0o700) + assertMode(t, path, 0o600) +} + +func TestOpenLogAppendsAndTightens(t *testing.T) { + dir := filepath.Join(t.TempDir(), "app-green") + path := filepath.Join(dir, "app-green.log") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("old\n"), 0o644); err != nil { + t.Fatal(err) + } + f, err := openLog(path) + if err != nil { + t.Fatal(err) + } + _, _ = f.WriteString("new\n") + _ = f.Close() + got, _ := os.ReadFile(path) + if string(got) != "old\nnew\n" { + t.Fatalf("content %q", got) + } + assertMode(t, dir, 0o700) + assertMode(t, path, 0o600) +} + +func assertMode(t *testing.T, path string, want os.FileMode) { + t.Helper() + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := fi.Mode().Perm(); got != want { + t.Errorf("%s mode %o, want %o", path, got, want) + } +} + +func TestOpenerCommand(t *testing.T) { + const u = "https://example.com/a?b=c" + for _, tc := range []struct { + goos, name string + args []string + }{ + {"darwin", "open", []string{"-u", u}}, + {"linux", "xdg-open", []string{u}}, + {"freebsd", "xdg-open", []string{u}}, + } { + name, args, err := openerCommand(tc.goos, u) + if err != nil { + t.Fatalf("%s: %v", tc.goos, err) + } + if name != tc.name || !slices.Equal(args, tc.args) { + t.Errorf("%s: got %s %q", tc.goos, name, args) + } + } +} + +func TestOpenerCommandRejects(t *testing.T) { + for _, u := range []string{"-a Calculator", "--help", "file:///etc/passwd", "javascript:alert(1)", "https://", "example.com"} { + if _, _, err := openerCommand("darwin", u); err == nil { + t.Errorf("%q accepted", u) + } + } + if _, _, err := openerCommand("windows", "https://example.com"); err == nil { + t.Error("windows accepted") + } +} + +// tokenFailConfig is a valid config whose env lists no repos (a warning) +// and whose GitHub token variable is unset. +const tokenFailConfig = ` +[jira] + site = "https://example.atlassian.net" + email = "me@example.com" + token_env = "APP_GREEN_TEST_JIRA_TOKEN" + projects = ["ABC"] + +[github] + author = "me" + token_env = "APP_GREEN_TEST_GITHUB_TOKEN" + repos = ["acme/app"] + +[[aws.accounts]] + name = "stage-acct" + profile = "stage" + region = "us-east-1" + +[[envs]] + account = "stage-acct" + pipeline = "app-pipeline" + stage = "Test" + deploy_action = "Deploy" +` + +func TestRunTokenError(t *testing.T) { + isolate(t) + const secret = "fake-SECRET-123" + t.Setenv("APP_GREEN_TEST_JIRA_TOKEN", secret) + t.Setenv("APP_GREEN_TEST_GITHUB_TOKEN", "") + path := filepath.Join(t.TempDir(), "config.toml") + if err := os.WriteFile(path, []byte(tokenFailConfig), 0o600); err != nil { + t.Fatal(err) + } + var out, errOut bytes.Buffer + if code := run([]string{"--config", path}, &out, &errOut); code != 1 { + t.Fatalf("exit %d, stderr %q", code, errOut.String()) + } + if !strings.HasPrefix(errOut.String(), "app-green: ") || !strings.Contains(errOut.String(), "APP_GREEN_TEST_GITHUB_TOKEN") { + t.Errorf("stderr %q", errOut.String()) + } + if strings.Contains(errOut.String(), "lists no repos") { + t.Errorf("warning on stderr: %q", errOut.String()) + } + logBytes, err := os.ReadFile(filepath.Join(os.Getenv("XDG_STATE_HOME"), "app-green", "app-green.log")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(logBytes), "lists no repos") { + t.Errorf("log missing warning: %q", logBytes) + } + for name, s := range map[string]string{"stdout": out.String(), "stderr": errOut.String(), "log": string(logBytes)} { + if strings.Contains(s, secret) { + t.Errorf("%s leaks the token: %q", name, s) + } + } +} + +// fakeRunner stands in for the resolver: it runs until its ctx is canceled. +type fakeRunner struct { + stopped chan struct{} + refresh <-chan struct{} +} + +func (f *fakeRunner) Run(ctx context.Context, _ chan<- resolver.Snapshot, refresh <-chan struct{}) { + f.refresh = refresh + <-ctx.Done() + close(f.stopped) +} + +// fakeProgram makes runProgram return err at once, recording the model. +func fakeProgram(t *testing.T, err error) *tea.Model { + t.Helper() + var got tea.Model + old := runProgram + runProgram = func(m tea.Model) error { got = m; return err } + t.Cleanup(func() { runProgram = old }) + return &got +} + +func TestRunAppStopsResolverOnQuit(t *testing.T) { + model := fakeProgram(t, nil) + r := &fakeRunner{stopped: make(chan struct{})} + if err := runApp(r, slog.New(slog.DiscardHandler)); err != nil { + t.Fatal(err) + } + select { + case <-r.stopped: + default: + t.Fatal("resolver still running after runApp returned") + } + if _, ok := (*model).(ui.App); !ok { + t.Fatalf("program model %T", *model) + } + if cap(r.refresh) != 1 { + t.Errorf("refresh cap %d, want 1", cap(r.refresh)) + } +} + +func TestRunAppProgramErrors(t *testing.T) { + boom := errors.New("boom") + for _, tc := range []struct { + name string + err, out error + }{ + {"interrupt is a clean exit", tea.ErrInterrupted, nil}, + {"other error returned", boom, boom}, + } { + t.Run(tc.name, func(t *testing.T) { + fakeProgram(t, tc.err) + r := &fakeRunner{stopped: make(chan struct{})} + if err := runApp(r, slog.New(slog.DiscardHandler)); !errors.Is(err, tc.out) || (tc.out == nil && err != nil) { + t.Fatalf("got %v, want %v", err, tc.out) + } + <-r.stopped + }) + } +} + +// stuckRunner ignores cancellation until release is closed. +type stuckRunner struct{ release chan struct{} } + +func (s stuckRunner) Run(context.Context, chan<- resolver.Snapshot, <-chan struct{}) { <-s.release } + +func TestRunAppDoesNotHangOnStuckResolver(t *testing.T) { + fakeProgram(t, nil) + old := stopTimeout + stopTimeout = 10 * time.Millisecond + t.Cleanup(func() { stopTimeout = old }) + r := stuckRunner{release: make(chan struct{})} + defer close(r.release) + finished := make(chan error, 1) + go func() { finished <- runApp(r, slog.New(slog.DiscardHandler)) }() + select { + case err := <-finished: + if err != nil { + t.Fatal(err) + } + case <-time.After(5 * time.Second): + t.Fatal("runApp hung waiting for the resolver") + } +} + +func TestOpenBrowser(t *testing.T) { + var got []string + old := startCommand + startCommand = func(name string, args ...string) error { got = append([]string{name}, args...); return nil } + t.Cleanup(func() { startCommand = old }) + + if err := openBrowser("-a Calculator"); err == nil || got != nil { + t.Fatalf("err %v, started %q", err, got) + } + const u = "https://example.com/x" + if err := openBrowser(u); err != nil { + if runtime.GOOS == "windows" { + t.Skip("no opener on windows") + } + t.Fatal(err) + } + name, args, _ := openerCommand(runtime.GOOS, u) + if want := append([]string{name}, args...); !slices.Equal(got, want) { + t.Fatalf("started %q, want %q", got, want) + } +} From 1825012152b11f2d5dc6c3081cdfa7746bf48fb5 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:32:52 -0400 Subject: [PATCH 14/21] fix(ui): detail scroll follows the selection only while it was visible Polls no longer snap the window back to a selection the user scrolled away from, and a resize no longer hides the selection. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/app.go | 7 ++++++- internal/ui/detail.go | 19 +++++++++++++++---- internal/ui/detail_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/internal/ui/app.go b/internal/ui/app.go index 1a134c1..e0b5105 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -72,14 +72,19 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { func (m App) update(msg tea.Msg) (App, tea.Cmd) { switch msg := msg.(type) { case tea.WindowSizeMsg: + visible := m.detail != "" && m.selectionVisible() m.width, m.height = msg.Width, msg.Height + if m.detail != "" { + m = m.followSelection(visible) + } case resolver.Snapshot: id := m.itemID(m.cursor) before, _ := m.chain(m.detail) + visible := m.detail != "" && m.selectionVisible() m.snap = &msg m.cursor = m.find(id) if m.detail != "" { - m = m.refreshDetail(before) + m = m.refreshDetail(before, visible) } return m, waitForSnapshot(m.snaps) case openFailedMsg: diff --git a/internal/ui/detail.go b/internal/ui/detail.go index 44bfc73..a5c3474 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -70,8 +70,9 @@ func (m App) detailKey(msg tea.KeyMsg) (App, tea.Cmd) { // with the selection on the same PR or env as in before, the ticket's chain // in the last snapshot; when that item is gone the selection stays at its // index, clamped. When the ticket is gone it returns to the list with a -// note saying so. -func (m App) refreshDetail(before model.Chain) App { +// note saying so. The window follows the selection only when it was on screen +// (visible) and no longer is; otherwise a poll leaves the scroll alone. +func (m App) refreshDetail(before model.Chain, visible bool) App { c, ok := m.chain(m.detail) if !ok { m.note = m.detail + " is no longer in the list" @@ -84,12 +85,22 @@ func (m App) refreshDetail(before model.Chain) App { for i, it := range items { if it.id() == id { m.dsel = i - return m.showSelection() + return m.followSelection(visible) } } } m.dsel = max(min(m.dsel, len(items)-1), 0) - return m.showSelection() + return m.followSelection(visible) +} + +// followSelection scrolls to the selection only if it was on screen +// (visible) before the change and no longer is. A selection the user +// scrolled away from stays where it is. +func (m App) followSelection(visible bool) App { + if visible && !m.selectionVisible() { + return m.showSelection() + } + return m } // id names the item so the selection can find it in the next snapshot: diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index 5d167dd..8c08583 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -402,3 +402,32 @@ func TestDetailOpenHiddenSelection(t *testing.T) { t.Errorf("opened\n got %q\nwant %q", o.urls, want) } } + +func TestDetailResizeKeepsSelectionOnScreen(t *testing.T) { + m := update(t, inDetail(t), tea.WindowSizeMsg{Width: 100, Height: 30}) + m = press(t, m, "j", "j", "j", "j", "j") // the last item + m = update(t, m, tea.WindowSizeMsg{Width: 100, Height: 8}) + if got := cursorLine(t, m); !strings.Contains(got, "prod-acct Production") { + t.Errorf("after the window shrinks the selection is still on screen: %q", got) + } +} + +func TestDetailPollKeepsScrollPosition(t *testing.T) { + snap := fxDetailSnapshot() + c := &snap.Chains[1] + for range 4 { + c.Flags = append(c.Flags, model.Flag{Level: model.Yellow, Kind: model.FlagStaleReview, Reason: "more"}) + } + m := update(t, press(t, update(t, newApp(t), snap), "j", "enter"), tea.WindowSizeMsg{Width: 100, Height: 8}) + m = press(t, m, "j", "j", "j", "j", "j", "j", "j", "j", "j", "j", "j", "j", "j") // last item, then scroll past it + for _, l := range viewLines(m) { + if isCursor(l) { + t.Fatalf("the selection should be scrolled out:\n%s", m.View()) + } + } + before := trimmed(m)[1] + m = update(t, m, snap) // a poll with nothing new + if got := trimmed(m)[1]; got != before { + t.Errorf("a poll moves the scrolled window\n got %q\nwant %q", got, before) + } +} From 5c9df094ab43e188f15f0a662cfdcdca1a2a86e2 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:33:42 -0400 Subject: [PATCH 15/21] fix(ui): open the parsed URL, and skip non-web check URLs webPage returns the parsed URL and open uses its String(), so what was checked is what is opened. checksTarget skips a failing check whose URL is not a web page and falls to the PR's checks tab instead of the ticket. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/app.go | 18 +++++++++++------- internal/ui/detail_test.go | 21 +++++++++++++++++++++ internal/ui/list.go | 2 +- internal/ui/list_test.go | 3 ++- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/internal/ui/app.go b/internal/ui/app.go index e0b5105..a569c56 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -182,12 +182,16 @@ func (m App) target() string { return "" } -// webPage reports whether u is an absolute http or https URL with a host. -// Anything else (a file: URL, or "-x" that a browser launcher could take for -// a flag) is never opened: the URLs come from Jira, GitHub and AWS. -func webPage(u string) bool { +// webPage parses u and reports whether it is an absolute http or https URL +// with a host. Anything else (a file: URL, or "-x" that a browser launcher +// could take for a flag) is never opened: the URLs come from Jira, GitHub and +// AWS. +func webPage(u string) (*url.URL, bool) { p, err := url.Parse(u) - return err == nil && (p.Scheme == "http" || p.Scheme == "https") && p.Host != "" + if err != nil || (p.Scheme != "http" && p.Scheme != "https") || p.Host == "" { + return nil, false + } + return p, true } // cursorTicketURL is the ticket URL of the chain under the cursor, "" on an @@ -217,8 +221,8 @@ type openFailedMsg struct{ err error } func (m App) open(urls ...string) tea.Cmd { var u string for _, c := range urls { - if webPage(c) { - u = c + if p, ok := webPage(c); ok { + u = p.String() // what was checked is what is opened break } } diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index 8c08583..2d13478 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -431,3 +431,24 @@ func TestDetailPollKeepsScrollPosition(t *testing.T) { t.Errorf("a poll moves the scrolled window\n got %q\nwant %q", got, before) } } + +func TestDetailOpenFailingCheckWithoutWebPage(t *testing.T) { + // A check whose URL is not a web page falls to the checks tab, not the ticket. + o := &opened{} + snap := fxDetailSnapshot() + snap.Chains[1].PRs[1].Failing[1].URL = "file:///etc/passwd" + press(t, detailWith(t, o, snap), "j", "o") + if want := "https://github.com/acme/app/pull/331/checks"; len(o.urls) != 1 || o.urls[0] != want { + t.Errorf("opened %q, want %q", o.urls, want) + } +} + +func TestOpenPassesTheParsedURL(t *testing.T) { + o := &opened{} + snap := fxDetailSnapshot() + snap.Chains[1].PRs[0].URL = "https://github.com/acme/app/pull/330 x" + press(t, detailWith(t, o, snap), "o") + if want := "https://github.com/acme/app/pull/330%20x"; len(o.urls) != 1 || o.urls[0] != want { + t.Errorf("opened %q, want the URL as parsed and re-encoded %q", o.urls, want) + } +} diff --git a/internal/ui/list.go b/internal/ui/list.go index 2491253..80be3da 100644 --- a/internal/ui/list.go +++ b/internal/ui/list.go @@ -95,7 +95,7 @@ func OpenTarget(c model.Chain, pipelineURL func(model.Env) string) string { // own page, else ChecksPage(pr). func checksTarget(pr model.PR) string { for _, ch := range pr.Failing { - if ch.URL != "" { + if _, ok := webPage(ch.URL); ok { return ch.URL } } diff --git a/internal/ui/list_test.go b/internal/ui/list_test.go index 3afa70d..204974f 100644 --- a/internal/ui/list_test.go +++ b/internal/ui/list_test.go @@ -435,7 +435,8 @@ func TestAppOpensOnlyWebPages(t *testing.T) { snap.Chains[0].PRs[0].URL = "-x" snap.Chains[0].PRs[0].Checks, snap.Chains[0].PRs[0].Failing = model.ChecksPassing, nil m = press(t, update(t, press(t, m, "k", "k", "k"), snap), "enter", "o") - want := []string{"https://jira.example.com/browse/ABC-1", "https://jira.example.com/browse/ABC-1"} + // A failing check whose URL is not a web page falls to the PR's checks tab. + want := []string{"https://github.com/acme/app/pull/3/checks", "https://jira.example.com/browse/ABC-1"} if strings.Join(o.urls, " ") != strings.Join(want, " ") { t.Errorf("opened\n got %q\nwant %q", o.urls, want) } From 9ac484ef1325b449ef197e676caffd52a57aa616 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:34:25 -0400 Subject: [PATCH 16/21] refactor(ui): compute the detail's selected line directly Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/detail.go | 34 +++++++++++++++++++++++----------- internal/ui/list.go | 2 +- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/internal/ui/detail.go b/internal/ui/detail.go index a5c3474..4ec3842 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -159,7 +159,7 @@ func detailTarget(c model.Chain, i int) string { // When the detail is taller than the screen it scrolls; the header and // footer stay put. func (m App) detailView(width int) string { - body, _ := m.detailBody(width) + body := m.detailBody(width) if budget := m.bodyHeight(); budget >= 0 && len(body) > budget { body = body[m.doffset:min(m.doffset+budget, len(body))] } @@ -167,10 +167,9 @@ func (m App) detailView(width int) string { return strings.Join(append(lines, m.footer(detailKeys, width)), "\n") } -// detailBody is the detail screen's lines, each width cells, and the index -// of the selected item's line (-1 with nothing to select): the ticket line, -// the stale line, one line per PR and per applying env, then the flags. -func (m App) detailBody(width int) ([]string, int) { +// detailBody is the detail screen's lines, each width cells: the ticket +// line, the stale line, one line per PR and per applying env, then the flags. +func (m App) detailBody(width int) []string { c, _ := m.chain(m.detail) now := m.now() body := []string{ticketLine(c, width)} @@ -185,7 +184,6 @@ func (m App) detailBody(width int) ([]string, int) { stageW = max(stageW, ansi.StringWidth(clean(it.slot.Env.Stage))) } } - sel := -1 for i, it := range items { var line string if it.slot != nil { @@ -195,7 +193,7 @@ func (m App) detailBody(width int) ([]string, int) { } line = fit(line, width) if i == m.dsel { - sel, line = len(body), m.styles.selected(line) + line = m.styles.selected(line) } body = append(body, line) } @@ -205,14 +203,14 @@ func (m App) detailBody(width int) ([]string, int) { body = append(body, fit(" "+m.styles.marker(f.Level)+" "+clean(f.Reason), width)) } } - return body, sel + return body } // showSelection scrolls the detail just enough to show the selected line, // and to the top on the first item so the ticket line shows. func (m App) showSelection() App { budget := m.bodyHeight() - _, sel := m.detailBody(defaultWidth) + sel := m.selectedLine() switch { case budget < 0 || sel < 0: case m.dsel == 0: @@ -225,9 +223,23 @@ func (m App) showSelection() App { return m } +// selectedLine is the index of the selected item's line in the detail body +// (see detailBody), -1 with nothing to select. +func (m App) selectedLine() int { + c, _ := m.chain(m.detail) + if m.dsel >= len(detailItems(c)) { + return -1 + } + line := 1 + m.dsel // the ticket line, then the items + if c.Stale { + line++ + } + return line +} + // selectionVisible reports whether the selected line is on screen. func (m App) selectionVisible() bool { - _, sel := m.detailBody(defaultWidth) + sel := m.selectedLine() budget := m.bodyHeight() return sel >= 0 && (budget < 0 || sel >= m.doffset && sel < m.doffset+budget) } @@ -239,7 +251,7 @@ func (m App) detailScroll() App { m.doffset = 0 return m } - body, _ := m.detailBody(defaultWidth) + body := m.detailBody(defaultWidth) m.doffset = min(max(m.doffset, 0), max(len(body)-budget, 0)) return m } diff --git a/internal/ui/list.go b/internal/ui/list.go index 80be3da..063ab91 100644 --- a/internal/ui/list.go +++ b/internal/ui/list.go @@ -23,7 +23,7 @@ func (m App) View() string { if width <= 0 { width = defaultWidth } - if m.detail != "" { + if m.detail != "" && m.snap != nil { // the detail only opens from a snapshot return m.detailView(width) } now := m.now() From 132de7a6d42884aea9f8d2755da3419f70e6db76 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:39:42 -0400 Subject: [PATCH 17/21] feat(ui): show the pipeline on detail env lines when account and stage collide Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/detail.go | 19 +++++++++++++++---- internal/ui/detail_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/internal/ui/detail.go b/internal/ui/detail.go index 4ec3842..b58fcdc 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -177,17 +177,28 @@ func (m App) detailBody(width int) []string { body = append(body, m.styles.dim.Render(fit("~ stale: "+clean(c.StaleReason), width))) } items := detailItems(c) - acctW, stageW := 0, 0 + acctW, stageW, pipeW := 0, 0, 0 + seen := map[[2]string]bool{} + ambiguous := false // two envs share an account and stage: the pipeline tells them apart for _, it := range items { if it.slot != nil { - acctW = max(acctW, ansi.StringWidth(clean(it.slot.Env.Account))) - stageW = max(stageW, ansi.StringWidth(clean(it.slot.Env.Stage))) + e := it.slot.Env + acctW = max(acctW, ansi.StringWidth(clean(e.Account))) + stageW = max(stageW, ansi.StringWidth(clean(e.Stage))) + pipeW = max(pipeW, ansi.StringWidth(clean(e.Pipeline))) + k := [2]string{e.Account, e.Stage} + ambiguous = ambiguous || seen[k] + seen[k] = true } } for i, it := range items { var line string if it.slot != nil { - line = " " + fit(clean(it.slot.Env.Account), acctW) + " " + fit(clean(it.slot.Env.Stage), stageW) + " " + slotText(*it.slot, now) + line = " " + fit(clean(it.slot.Env.Account), acctW) + " " + fit(clean(it.slot.Env.Stage), stageW) + " " + if ambiguous { + line += fit(clean(it.slot.Env.Pipeline), pipeW) + " " + } + line += slotText(*it.slot, now) } else { line = " " + prLine(*it.pr, now) } diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index 2d13478..5f65138 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -452,3 +452,27 @@ func TestOpenPassesTheParsedURL(t *testing.T) { t.Errorf("opened %q, want the URL as parsed and re-encoded %q", o.urls, want) } } + +func TestDetailPipelineOnlyWhenAmbiguous(t *testing.T) { + env := func(pipeline string) model.Env { + return model.Env{Account: "stage-acct", Pipeline: pipeline, Stage: "Test"} + } + view := func(slots ...model.EnvSlot) []string { + c := model.Chain{Ticket: model.Ticket{Key: "ABC-5", Title: "Two pipelines"}, Slots: slots} + snap := resolver.Snapshot{Chains: []model.Chain{c}, At: now} + return trimmed(press(t, update(t, newApp(t), snap), "enter")) + } + a := model.EnvSlot{Env: env("app-pipeline"), State: model.SlotNotYet, Applies: true} + b := model.EnvSlot{Env: env("web-pipeline"), State: model.SlotNotYet, Applies: true} + + ls := view(a, b) // the same account and stage in two pipelines + if want := " >stage-acct Test app-pipeline not yet"; ls[2] != want { + t.Errorf("\n got %q\nwant %q", ls[2], want) + } + if want := " stage-acct Test web-pipeline not yet"; ls[3] != want { + t.Errorf("\n got %q\nwant %q", ls[3], want) + } + if ls := view(a); strings.Contains(ls[2], "pipeline") { + t.Errorf("an unambiguous env line has no pipeline: %q", ls[2]) + } +} From 72ab13b1f18d294ead0ee4fe81e523168bad51b1 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:44:41 -0400 Subject: [PATCH 18/21] fix(ui): refuse URLs with bytes a browser launcher could mishandle ui.WebPage rejects spaces and other controls, DEL, quotes, backticks, angle brackets and backslashes, on top of the http(s)+host check. The opener in main.go uses the same function so the two cannot drift. Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/app.go | 19 ++++++++++++------- internal/ui/detail_test.go | 16 ++++++++++++++-- internal/ui/list.go | 2 +- main.go | 4 +--- main_test.go | 3 ++- 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/internal/ui/app.go b/internal/ui/app.go index a569c56..548f4e6 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -182,11 +182,16 @@ func (m App) target() string { return "" } -// webPage parses u and reports whether it is an absolute http or https URL -// with a host. Anything else (a file: URL, or "-x" that a browser launcher -// could take for a flag) is never opened: the URLs come from Jira, GitHub and -// AWS. -func webPage(u string) (*url.URL, bool) { +// WebPage parses u and reports whether it is safe to hand to a browser +// launcher: an absolute http or https URL with a host and none of the bytes +// a sloppy launcher could mishandle (space and other controls, DEL, quote, +// backtick, <, > and backslash). Anything else (a file: URL, or "-x" that a +// launcher could take for a flag) is never opened: the URLs come from Jira, +// GitHub, AWS and third-party CI. +func WebPage(u string) (*url.URL, bool) { + if strings.ContainsFunc(u, func(r rune) bool { return r <= ' ' || r == 0x7f || strings.ContainsRune("\"`<>\\", r) }) { + return nil, false + } p, err := url.Parse(u) if err != nil || (p.Scheme != "http" && p.Scheme != "https") || p.Host == "" { return nil, false @@ -216,12 +221,12 @@ func (m App) requestRefresh() { type openFailedMsg struct{ err error } // open is a command that opens the first of urls that is a web page (see -// webPage) in the browser, reporting a failure as openFailedMsg; nil when +// WebPage) in the browser, reporting a failure as openFailedMsg; nil when // none is. Update never opens a page itself. func (m App) open(urls ...string) tea.Cmd { var u string for _, c := range urls { - if p, ok := webPage(c); ok { + if p, ok := WebPage(c); ok { u = p.String() // what was checked is what is opened break } diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index 5f65138..4d5c6e0 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -446,9 +446,9 @@ func TestDetailOpenFailingCheckWithoutWebPage(t *testing.T) { func TestOpenPassesTheParsedURL(t *testing.T) { o := &opened{} snap := fxDetailSnapshot() - snap.Chains[1].PRs[0].URL = "https://github.com/acme/app/pull/330 x" + snap.Chains[1].PRs[0].URL = "https://github.com/acme/app/pull/330é" press(t, detailWith(t, o, snap), "o") - if want := "https://github.com/acme/app/pull/330%20x"; len(o.urls) != 1 || o.urls[0] != want { + if want := "https://github.com/acme/app/pull/330%C3%A9"; len(o.urls) != 1 || o.urls[0] != want { t.Errorf("opened %q, want the URL as parsed and re-encoded %q", o.urls, want) } } @@ -476,3 +476,15 @@ func TestDetailPipelineOnlyWhenAmbiguous(t *testing.T) { t.Errorf("an unambiguous env line has no pipeline: %q", ls[2]) } } + +func TestDetailOpenSkipsURLsWithUnsafeBytes(t *testing.T) { + for _, u := range []string{"https://ci.example.com/x?q=$(id) `id`", "https://ci.example.com/a b", "https://ci.example.com/\"x"} { + o := &opened{} + snap := fxDetailSnapshot() + snap.Chains[1].PRs[1].Failing[1].URL = u + press(t, detailWith(t, o, snap), "j", "o") + if want := "https://github.com/acme/app/pull/331/checks"; len(o.urls) != 1 || o.urls[0] != want { + t.Errorf("%q: opened %q, want %q", u, o.urls, want) + } + } +} diff --git a/internal/ui/list.go b/internal/ui/list.go index 063ab91..41ca108 100644 --- a/internal/ui/list.go +++ b/internal/ui/list.go @@ -95,7 +95,7 @@ func OpenTarget(c model.Chain, pipelineURL func(model.Env) string) string { // own page, else ChecksPage(pr). func checksTarget(pr model.PR) string { for _, ch := range pr.Failing { - if _, ok := webPage(ch.URL); ok { + if _, ok := WebPage(ch.URL); ok { return ch.URL } } diff --git a/main.go b/main.go index f579d5b..3ec3741 100644 --- a/main.go +++ b/main.go @@ -10,7 +10,6 @@ import ( "io" "io/fs" "log/slog" - "net/url" "os" "os/exec" "path/filepath" @@ -206,8 +205,7 @@ func openLog(path string) (*os.File, error) { // read as an option: neither opener accepts "--", and macOS open gets the // URL as the value of -u. func openerCommand(goos, u string) (string, []string, error) { - p, err := url.Parse(u) - if err != nil || (p.Scheme != "http" && p.Scheme != "https") || p.Host == "" { + if _, ok := ui.WebPage(u); !ok { return "", nil, fmt.Errorf("not a web page: %q", u) } switch goos { diff --git a/main_test.go b/main_test.go index 168db47..16ce74d 100644 --- a/main_test.go +++ b/main_test.go @@ -180,7 +180,8 @@ func TestOpenerCommand(t *testing.T) { } func TestOpenerCommandRejects(t *testing.T) { - for _, u := range []string{"-a Calculator", "--help", "file:///etc/passwd", "javascript:alert(1)", "https://", "example.com"} { + for _, u := range []string{"-a Calculator", "--help", "file:///etc/passwd", "javascript:alert(1)", "https://", "example.com", + "https://example.com/x?q=$(id) `id`", "https://example.com/a b", "https://example.com/\"x", "https://example.com/", "https://example.com/a\\b", "https://example.com/a\x7fb"} { if _, _, err := openerCommand("darwin", u); err == nil { t.Errorf("%q accepted", u) } From 59b9428f634fb6685de96b2d4774f5fd06e665f1 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:45:06 -0400 Subject: [PATCH 19/21] feat(ui): say "nothing to open" when o finds no page Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/app.go | 16 ++++++++++------ internal/ui/detail.go | 2 +- internal/ui/detail_test.go | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/internal/ui/app.go b/internal/ui/app.go index 548f4e6..3585be0 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -127,7 +127,9 @@ func (m App) key(msg tea.KeyMsg) (App, tea.Cmd) { m.detail, m.dsel, m.doffset = m.snap.Chains[m.cursor].Ticket.Key, 0, 0 } case key.Matches(msg, keys.open): - return m, m.open(m.target(), m.cursorTicketURL()) + if m.items() > 0 { // with nothing selected there is nothing to say + return m.open(m.target(), m.cursorTicketURL()) + } } return m, nil } @@ -221,9 +223,10 @@ func (m App) requestRefresh() { type openFailedMsg struct{ err error } // open is a command that opens the first of urls that is a web page (see -// WebPage) in the browser, reporting a failure as openFailedMsg; nil when -// none is. Update never opens a page itself. -func (m App) open(urls ...string) tea.Cmd { +// WebPage) in the browser, reporting a failure as openFailedMsg. When none +// is, there is no command and the footer says "nothing to open". Update never +// opens a page itself. +func (m App) open(urls ...string) (App, tea.Cmd) { var u string for _, c := range urls { if p, ok := WebPage(c); ok { @@ -232,10 +235,11 @@ func (m App) open(urls ...string) tea.Cmd { } } if u == "" { - return nil + m.err = "nothing to open" + return m, nil } openURL := m.openURL - return func() tea.Msg { + return m, func() tea.Msg { if err := openURL(u); err != nil { return openFailedMsg{err} } diff --git a/internal/ui/detail.go b/internal/ui/detail.go index b58fcdc..af240d0 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -61,7 +61,7 @@ func (m App) detailKey(msg tea.KeyMsg) (App, tea.Cmd) { if !m.selectionVisible() { sel = -1 // o acts only on a selection you can see: else the ticket } - return m, m.open(detailTarget(c, sel), c.Ticket.URL) + return m.open(detailTarget(c, sel), c.Ticket.URL) } return m, nil } diff --git a/internal/ui/detail_test.go b/internal/ui/detail_test.go index 4d5c6e0..02984a4 100644 --- a/internal/ui/detail_test.go +++ b/internal/ui/detail_test.go @@ -488,3 +488,22 @@ func TestDetailOpenSkipsURLsWithUnsafeBytes(t *testing.T) { } } } + +func TestOpenWithNothingOpenableSaysSo(t *testing.T) { + snap := fxDetailSnapshot() + snap.Unlinked[0].URL = "" // no page, and an unlinked PR has no ticket to fall back to + o := &opened{} + m := ui.NewApp(make(chan resolver.Snapshot), make(chan struct{}, 1), o.open).WithClock(func() time.Time { return now }) + m = update(t, update(t, m, tea.WindowSizeMsg{Width: 100, Height: 30}), snap) + m = press(t, m, "j", "j", "j", "j", "o") // the unlinked PR + if ls := viewLines(m); !strings.Contains(ls[len(ls)-1], "nothing to open") { + t.Errorf("the footer says nothing opened: %q", ls[len(ls)-1]) + } + if len(o.urls) != 0 { + t.Errorf("opened %q", o.urls) + } + // The next key clears it. + if ls := viewLines(press(t, m, "k")); strings.Contains(ls[len(ls)-1], "nothing to open") { + t.Errorf("the next key clears the message: %q", ls[len(ls)-1]) + } +} From 787928af6366f9ea9554800f1318ff635ec236b6 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:46:29 -0400 Subject: [PATCH 20/21] fix(resolver): log a poll warning when it appears, not on every poll Assisted-by: Claude Code:claude-opus-5-5 --- internal/resolver/resolver.go | 8 +++++- internal/resolver/resolver_test.go | 39 +++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 1520b60..7abb7e1 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -151,6 +151,7 @@ type Resolver struct { recheck map[config.PipelineKey]bool // cached sources a refresh wants asked again pipelines map[config.PipelineKey]pipelineData health map[string]healthData // by Env.ID() + warned map[string]bool // the last poll's warnings, so only new ones are logged } // New returns a Resolver over ad. now is the clock (nil is time.Now); log @@ -330,9 +331,14 @@ func (r *Resolver) Poll(ctx context.Context) Snapshot { r.record("aws "+a, ar.err, now) } snap := r.build(ctx, now) + current := make(map[string]bool, len(snap.Warnings)) for _, w := range snap.Warnings { - r.log.Warn("poll warning", "msg", w) + current[w] = true + if !r.warned[w] { // a warning that persists is logged once, not every poll + r.log.Warn("poll warning", "msg", w) + } } + r.warned = current return snap } diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index a668d7c..6786d4b 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -518,7 +518,7 @@ func TestEnvReposFromSourcesOnlyWhenConfigHasNone(t *testing.T) { } } -func TestWarningsDedupedAndLoggedOncePerPoll(t *testing.T) { +func TestWarningsAreDedupedWithinAPoll(t *testing.T) { var buf safeBuffer log := slog.New(slog.NewTextHandler(&buf, nil)) h := newHarness(t, log, testEnv, apiEnv) @@ -539,9 +539,10 @@ func TestWarningsDedupedAndLoggedOncePerPoll(t *testing.T) { t.Errorf("%q logged %d times in one poll, want 1\n%s", w, n, buf.String()) } } + // Still there next poll: not logged again (TestPollWarningIsLoggedOnceWhileItPersists). h.r.Poll(context.Background()) - if n := strings.Count(buf.String(), "github: w2"); n != 2 { - t.Errorf("w2 logged %d times over two polls, want 2", n) + if n := strings.Count(buf.String(), "github: w2"); n != 1 { + t.Errorf("w2 logged %d times over two polls, want 1", n) } } @@ -634,3 +635,35 @@ func TestSSOFlagOnlyForAWS(t *testing.T) { t.Errorf("jira status = %+v, want SSO false (SSO is an AWS state)", j) } } + +func TestPollWarningIsLoggedOnceWhileItPersists(t *testing.T) { + var buf safeBuffer + h := newHarness(t, slog.New(slog.NewTextHandler(&buf, nil))) + h.withShippedChain() + h.tracker.set(func(f *fakeTracker) { f.byKey = map[string]model.Ticket{"ABC-9": ticket("ABC-9")} }) + h.host.set(func(f *fakeHost) { f.prs[repo] = append(f.prs[repo], openPR(9, "ABC-9: x")) }) + fail := func(err error) { h.tracker.set(func(f *fakeTracker) { f.byKeyErr = err }) } + count := func() int { return strings.Count(buf.String(), "poll warning") } + h.r.Poll(context.Background()) + + fail(errors.New("jira: HTTP 500")) + h.r.Poll(context.Background()) + first := count() + if first == 0 { + t.Fatalf("the warning is logged when it first appears: %q", buf.String()) + } + h.r.Poll(context.Background()) + h.r.Poll(context.Background()) + if got := count(); got != first { + t.Errorf("a persistent warning is logged once: %d lines, want %d", got, first) + } + + // Once it clears and returns it is news again. + fail(nil) + h.r.Poll(context.Background()) + fail(errors.New("jira: HTTP 500")) + h.r.Poll(context.Background()) + if got := count(); got != 2*first { + t.Errorf("a warning that returns is logged again: %d lines, want %d", got, 2*first) + } +} From 8f08ac3985db706ec8efddcfb00190d7c0ea8325 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 7 Oct 2026 11:46:39 -0400 Subject: [PATCH 21/21] docs(ui): showSelection comment matches short screens Assisted-by: Claude Code:claude-opus-5-5 --- internal/ui/detail.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/ui/detail.go b/internal/ui/detail.go index af240d0..84c8d8d 100644 --- a/internal/ui/detail.go +++ b/internal/ui/detail.go @@ -218,7 +218,8 @@ func (m App) detailBody(width int) []string { } // showSelection scrolls the detail just enough to show the selected line, -// and to the top on the first item so the ticket line shows. +// and to the top on the first item so the ticket line shows (when the screen +// has room for both; on a one-line body the selection wins). func (m App) showSelection() App { budget := m.bodyHeight() sel := m.selectedLine()