Compare commits

...

2 Commits

Author SHA1 Message Date
6f963c7660 fix: resolve pending and jumping 2026-08-03 14:46:25 +02:00
4fcc479779 chore: remove old and unused code 2026-08-03 13:26:57 +02:00
21 changed files with 654 additions and 201 deletions

View File

@@ -239,10 +239,8 @@ Configuration is optional TOML. diple checks:
1. `--config FILE`; 1. `--config FILE`;
2. `DIPLE_CONFIG`; 2. `DIPLE_CONFIG`;
3. `GH_THREADS_CONFIG` as a migration fallback; 3. `$XDG_CONFIG_HOME/diple/config.toml`; and
4. `$XDG_CONFIG_HOME/diple/config.toml`; 4. the operating-system configuration directory.
5. the operating-system configuration directory; and
6. legacy `gh-threads` paths when no diple configuration exists.
Common default paths: Common default paths:
@@ -525,16 +523,19 @@ Cached data is labelled when first shown. A normal refresh does not repeatedly
reintroduce the cached header. reintroduce the cached header.
Read state and recoverable drafts live beside the configuration file as Read state and recoverable drafts live beside the configuration file as
`state.json` and `drafts.json`. Confirmed reversible GitHub writes are kept in `state.json` and `drafts.json`. Reversible GitHub writes are kept in
`mutation-queue.json` until GitHub confirms them. Experimental AI data defaults `mutation-queue.json` until a live refresh verifies them. Experimental AI data defaults
to the `ai` directory beside the configuration. These files are versioned and to the `ai` directory beside the configuration. These files are versioned and
written atomically; sensitive user-authored state uses restrictive permissions. written atomically; sensitive user-authored state uses restrictive permissions.
Cached permission gates are treated as the last known truth while offline: Cached permission gates are treated as the last known truth while offline:
actions granted by the snapshot can be queued, while actions denied by it stay actions granted by the snapshot can be queued, while actions denied by it stay
disabled. Pending replies and projected PR or thread changes are labelled in disabled. Queued replies and projected PR or thread changes are displayed
the UI without being written into the read cache. Replay preserves global optimistically as successful without being written into the read cache. A
enqueue order, including across repositories. pending marker appears only after a live refresh cannot verify the change or a
definite rejection needs attention. Retryable transport failures remain
optimistic and are recorded in Health. Replay preserves global enqueue order,
including across repositories.
If GitHub permissions or the target changed, replay pauses before the first If GitHub permissions or the target changed, replay pauses before the first
unsafe operation and presents choices to keep it queued, discard that item and unsafe operation and presents choices to keep it queued, discard that item and

View File

@@ -26,14 +26,16 @@ const (
) )
type aiPreparedMsg struct { type aiPreparedMsg struct {
preview AIPreview generation uint64
threadID string preview AIPreview
err error threadID string
err error
} }
type aiCompletedMsg struct { type aiCompletedMsg struct {
result AIResult generation uint64
err error result AIResult
err error
} }
type aiStatusMsg struct { type aiStatusMsg struct {
@@ -41,15 +43,24 @@ type aiStatusMsg struct {
} }
type aiProgressMsg struct { type aiProgressMsg struct {
progress AIRunProgress generation uint64
progress AIRunProgress
} }
type aiProviderTestCompletedMsg struct { type aiProviderTestCompletedMsg struct {
model string generation uint64
err error model string
err error
} }
type aiAnimationTickMsg time.Time type aiAnimationTickMsg struct {
generation uint64
}
func (m *App) nextAIGeneration() uint64 {
m.aiGeneration++
return m.aiGeneration
}
func (m *App) openAIMenu() { func (m *App) openAIMenu() {
if m.screen == prScreen { if m.screen == prScreen {
@@ -97,6 +108,7 @@ func (m *App) beginAIPrepare(threadID, message string) tea.Cmd {
} }
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
m.aiCancel = cancel m.aiCancel = cancel
generation := m.nextAIGeneration()
controller, details := m.ai, m.details controller, details := m.ai, m.details
m.aiMode = aiPreparing m.aiMode = aiPreparing
m.err = nil m.err = nil
@@ -112,9 +124,11 @@ func (m *App) beginAIPrepare(threadID, message string) tea.Cmd {
} }
prepare := func() tea.Msg { prepare := func() tea.Msg {
preview, err := controller.Prepare(ctx, details, threadID, message) preview, err := controller.Prepare(ctx, details, threadID, message)
return aiPreparedMsg{preview: preview, threadID: threadID, err: err} return aiPreparedMsg{
generation: generation, preview: preview, threadID: threadID, err: err,
}
} }
return tea.Batch(prepare, nextAIAnimationTick()) return tea.Batch(prepare, nextAIAnimationTick(generation))
} }
func (m *App) returnFromAIPrepareFailure(threadID string) { func (m *App) returnFromAIPrepareFailure(threadID string) {
@@ -133,6 +147,7 @@ func (m *App) beginAIRun() tea.Cmd {
} }
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
m.aiCancel = cancel m.aiCancel = cancel
generation := m.nextAIGeneration()
controller, preview := m.ai, m.aiPreview controller, preview := m.ai, m.aiPreview
m.aiMode = aiBusy m.aiMode = aiBusy
m.aiSpinner = 0 m.aiSpinner = 0
@@ -146,18 +161,21 @@ func (m *App) beginAIRun() tea.Cmd {
m.aiEvents = events m.aiEvents = events
work := func() tea.Msg { work := func() tea.Msg {
go func() { go func() {
defer close(events)
result, err := controller.RunWithProgress(ctx, preview, func(progress AIRunProgress) { result, err := controller.RunWithProgress(ctx, preview, func(progress AIRunProgress) {
select { select {
case events <- aiProgressMsg{progress: progress}: case events <- aiProgressMsg{generation: generation, progress: progress}:
default: default:
} }
}) })
events <- aiCompletedMsg{result: result, err: err} select {
close(events) case events <- aiCompletedMsg{generation: generation, result: result, err: err}:
case <-ctx.Done():
}
}() }()
return <-events return <-events
} }
return tea.Batch(work, nextAIAnimationTick()) return tea.Batch(work, nextAIAnimationTick(generation))
} }
func (m *App) beginAIProviderTest() tea.Cmd { func (m *App) beginAIProviderTest() tea.Cmd {
@@ -168,6 +186,7 @@ func (m *App) beginAIProviderTest() tea.Cmd {
} }
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
m.aiCancel = cancel m.aiCancel = cancel
generation := m.nextAIGeneration()
controller := m.ai controller := m.ai
m.aiMode = aiProviderTestBusy m.aiMode = aiProviderTestBusy
m.aiSpinner = 0 m.aiSpinner = 0
@@ -181,18 +200,23 @@ func (m *App) beginAIProviderTest() tea.Cmd {
m.aiEvents = events m.aiEvents = events
work := func() tea.Msg { work := func() tea.Msg {
go func() { go func() {
defer close(events)
model, err := controller.TestProvider(ctx, func(progress AIRunProgress) { model, err := controller.TestProvider(ctx, func(progress AIRunProgress) {
select { select {
case events <- aiProgressMsg{progress: progress}: case events <- aiProgressMsg{generation: generation, progress: progress}:
default: default:
} }
}) })
events <- aiProviderTestCompletedMsg{model: model, err: err} select {
close(events) case events <- aiProviderTestCompletedMsg{
generation: generation, model: model, err: err,
}:
case <-ctx.Done():
}
}() }()
return <-events return <-events
} }
return tea.Batch(work, nextAIAnimationTick()) return tea.Batch(work, nextAIAnimationTick(generation))
} }
func waitAIEvent(events <-chan tea.Msg) tea.Cmd { func waitAIEvent(events <-chan tea.Msg) tea.Cmd {
@@ -204,9 +228,9 @@ func waitAIEvent(events <-chan tea.Msg) tea.Cmd {
} }
} }
func nextAIAnimationTick() tea.Cmd { func nextAIAnimationTick(generation uint64) tea.Cmd {
return tea.Tick(100*time.Millisecond, func(at time.Time) tea.Msg { return tea.Tick(100*time.Millisecond, func(time.Time) tea.Msg {
return aiAnimationTickMsg(at) return aiAnimationTickMsg{generation: generation}
}) })
} }
@@ -215,7 +239,7 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) {
case tea.WindowSizeMsg: case tea.WindowSizeMsg:
return m, nil, false return m, nil, false
case aiPreparedMsg: case aiPreparedMsg:
if m.aiMode != aiPreparing { if msg.generation != m.aiGeneration || m.aiMode != aiPreparing {
return m, nil, true return m, nil, true
} }
m.aiCancel = nil m.aiCancel = nil
@@ -230,15 +254,19 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) {
} }
return m, nil, true return m, nil, true
case aiAnimationTickMsg: case aiAnimationTickMsg:
if msg.generation != m.aiGeneration {
return m, nil, true
}
switch m.aiMode { switch m.aiMode {
case aiPreparing, aiBusy, aiProviderTestBusy: case aiPreparing, aiBusy, aiProviderTestBusy:
m.aiSpinner++ m.aiSpinner++
return m, nextAIAnimationTick(), true return m, nextAIAnimationTick(msg.generation), true
default: default:
return m, nil, true return m, nil, true
} }
case aiProgressMsg: case aiProgressMsg:
if m.aiMode != aiBusy && m.aiMode != aiProviderTestBusy { if msg.generation != m.aiGeneration ||
(m.aiMode != aiBusy && m.aiMode != aiProviderTestBusy) {
return m, nil, true return m, nil, true
} }
m.aiProgress = msg.progress m.aiProgress = msg.progress
@@ -247,7 +275,7 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) {
m.aiStatus, m.aiStatusBusy = msg.status, false m.aiStatus, m.aiStatusBusy = msg.status, false
return m, nil, true return m, nil, true
case aiCompletedMsg: case aiCompletedMsg:
if m.aiMode != aiBusy { if msg.generation != m.aiGeneration || m.aiMode != aiBusy {
return m, nil, true return m, nil, true
} }
m.aiCancel, m.aiEvents = nil, nil m.aiCancel, m.aiEvents = nil, nil
@@ -281,7 +309,7 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) {
m.recordHealth("AI provider", healthOK, healthMessage) m.recordHealth("AI provider", healthOK, healthMessage)
return m, nil, true return m, nil, true
case aiProviderTestCompletedMsg: case aiProviderTestCompletedMsg:
if m.aiMode != aiProviderTestBusy { if msg.generation != m.aiGeneration || m.aiMode != aiProviderTestBusy {
return m, nil, true return m, nil, true
} }
m.aiCancel, m.aiEvents = nil, nil m.aiCancel, m.aiEvents = nil, nil
@@ -322,6 +350,7 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) {
m.aiCancel() m.aiCancel()
m.aiCancel = nil m.aiCancel = nil
} }
m.nextAIGeneration()
m.aiMode, m.aiInput, m.writeThreadID, m.aiEvents = aiNone, "", "", nil m.aiMode, m.aiInput, m.writeThreadID, m.aiEvents = aiNone, "", "", nil
m.aiInputEditor = textEditor{} m.aiInputEditor = textEditor{}
return m, nil, true return m, nil, true

3
cli.go
View File

@@ -108,8 +108,7 @@ Other:
Boolean options accept explicit values, for example --cache=false. Boolean options accept explicit values, for example --cache=false.
Command-line options override TOML settings. GH_REPO is used only when Command-line options override TOML settings. GH_REPO is used only when
--repo is absent. DIPLE_CONFIG selects a configuration file; GH_THREADS_CONFIG --repo is absent. DIPLE_CONFIG selects a configuration file.
is retained as a migration fallback.
Authentication uses GH_TOKEN or GITHUB_TOKEN when set, otherwise the active Authentication uses GH_TOKEN or GITHUB_TOKEN when set, otherwise the active
credential from 'gh auth login'. Run 'diple completion --help' for completion credential from 'gh auth login'. Run 'diple completion --help' for completion

View File

@@ -137,16 +137,8 @@ func configPath() (string, error) {
if path := os.Getenv("DIPLE_CONFIG"); path != "" { if path := os.Getenv("DIPLE_CONFIG"); path != "" {
return path, nil return path, nil
} }
// Preserve the old override during the rename so existing scripts do not
// silently start with a fresh configuration.
if path := os.Getenv("GH_THREADS_CONFIG"); path != "" {
return path, nil
}
if base := os.Getenv("XDG_CONFIG_HOME"); base != "" { if base := os.Getenv("XDG_CONFIG_HOME"); base != "" {
return firstExistingOrDefault( return filepath.Join(base, "diple", "config.toml"), nil
filepath.Join(base, "diple", "config.toml"),
filepath.Join(base, "gh-threads", "config.toml"),
), nil
} }
base, err := os.UserConfigDir() base, err := os.UserConfigDir()
if err != nil { if err != nil {
@@ -158,19 +150,11 @@ func configPath() (string, error) {
return "", fmt.Errorf("find home directory: %w", err) return "", fmt.Errorf("find home directory: %w", err)
} }
dotConfig := filepath.Join(home, ".config", "diple", "config.toml") dotConfig := filepath.Join(home, ".config", "diple", "config.toml")
legacyPreferred := filepath.Join(base, "gh-threads", "config.toml") return existingConfigPath(preferred, dotConfig), nil
legacyDotConfig := filepath.Join(home, ".config", "gh-threads", "config.toml")
return firstExistingOrDefault(
preferred, dotConfig, legacyPreferred, legacyDotConfig,
), nil
} }
func existingConfigPath(preferred, fallback string) string { func existingConfigPath(preferred, fallback string) string {
return firstExistingOrDefault(preferred, fallback) for _, candidate := range []string{preferred, fallback} {
}
func firstExistingOrDefault(preferred string, alternatives ...string) string {
for _, candidate := range append([]string{preferred}, alternatives...) {
if _, err := os.Stat(candidate); err == nil || !errors.Is(err, os.ErrNotExist) { if _, err := os.Stat(candidate); err == nil || !errors.Is(err, os.ErrNotExist) {
return candidate return candidate
} }
@@ -259,10 +243,7 @@ func defaultCacheDir() (string, error) {
if err != nil { if err != nil {
return "", fmt.Errorf("find user cache directory: %w", err) return "", fmt.Errorf("find user cache directory: %w", err)
} }
return firstExistingOrDefault( return filepath.Join(base, "diple"), nil
filepath.Join(base, "diple"),
filepath.Join(base, "gh-threads"),
), nil
} }
func validateThreadStatusOrder(order []string) error { func validateThreadStatusOrder(order []string) error {

View File

@@ -233,7 +233,6 @@ func TestLoadConfigRejectsUnknownSettings(t *testing.T) {
func TestConfigPathHonorsEnvironmentOverride(t *testing.T) { func TestConfigPathHonorsEnvironmentOverride(t *testing.T) {
t.Setenv("DIPLE_CONFIG", "/tmp/custom-diple.toml") t.Setenv("DIPLE_CONFIG", "/tmp/custom-diple.toml")
t.Setenv("GH_THREADS_CONFIG", "")
got, err := configPath() got, err := configPath()
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@@ -243,18 +242,6 @@ func TestConfigPathHonorsEnvironmentOverride(t *testing.T) {
} }
} }
func TestConfigPathHonorsLegacyEnvironmentOverride(t *testing.T) {
t.Setenv("DIPLE_CONFIG", "")
t.Setenv("GH_THREADS_CONFIG", "/tmp/legacy-gh-threads.toml")
got, err := configPath()
if err != nil {
t.Fatal(err)
}
if got != "/tmp/legacy-gh-threads.toml" {
t.Fatalf("legacy config path = %q", got)
}
}
func TestExistingConfigPathFallsBackToDotConfig(t *testing.T) { func TestExistingConfigPathFallsBackToDotConfig(t *testing.T) {
root := t.TempDir() root := t.TempDir()
preferred := filepath.Join(root, "Library", "Application Support", "diple", "config.toml") preferred := filepath.Join(root, "Library", "Application Support", "diple", "config.toml")
@@ -280,23 +267,7 @@ func TestExistingConfigPathFallsBackToDotConfig(t *testing.T) {
} }
} }
func TestFirstExistingConfigPathFallsBackToLegacyName(t *testing.T) {
root := t.TempDir()
current := filepath.Join(root, "diple", "config.toml")
legacy := filepath.Join(root, "gh-threads", "config.toml")
if err := os.MkdirAll(filepath.Dir(legacy), 0o700); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(legacy, []byte("theme = \"dark\"\n"), 0o600); err != nil {
t.Fatal(err)
}
if got := firstExistingOrDefault(current, legacy); got != legacy {
t.Fatalf("migration config path = %q, want %q", got, legacy)
}
}
func TestConfigPathHonorsXDGConfigHome(t *testing.T) { func TestConfigPathHonorsXDGConfigHome(t *testing.T) {
t.Setenv("GH_THREADS_CONFIG", "")
t.Setenv("DIPLE_CONFIG", "") t.Setenv("DIPLE_CONFIG", "")
t.Setenv("XDG_CONFIG_HOME", "/tmp/xdg-config") t.Setenv("XDG_CONFIG_HOME", "/tmp/xdg-config")
got, err := configPath() got, err := configPath()

101
github.go
View File

@@ -1093,16 +1093,6 @@ func (c *GitHubClient) allCheckContexts(
return nodes, nil return nodes, nil
} }
func checkMayHaveUsefulAnnotations(check githubCheckContext) bool {
state := strings.ToUpper(firstNonEmpty(check.Conclusion, check.State, check.Status))
switch state {
case "FAILURE", "ERROR", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED", "STALE":
return true
default:
return false
}
}
func (c *GitHubClient) checkAnnotations( func (c *GitHubClient) checkAnnotations(
ctx context.Context, checkID string, ctx context.Context, checkID string,
) ([]githubCheckAnnotation, error) { ) ([]githubCheckAnnotation, error) {
@@ -1427,6 +1417,17 @@ func (c *GitHubClient) EnrichPullRequest(
Owner: details.Owner, Repository: details.Repository, Number: details.Number, Owner: details.Owner, Repository: details.Repository, Number: details.Number,
HeadOID: details.HeadOID, CheckAnnotations: make(map[string][]CheckAnnotation), HeadOID: details.HeadOID, CheckAnnotations: make(map[string][]CheckAnnotation),
} }
type annotationJob struct {
index int
check Check
}
type annotationResult struct {
checkID string
annotations []CheckAnnotation
err error
}
var jobs []annotationJob
var annotations []annotationResult
for _, check := range details.Checks { for _, check := range details.Checks {
if check.ID == "" || !checkStateMayHaveUsefulAnnotations(check) { if check.ID == "" || !checkStateMayHaveUsefulAnnotations(check) {
continue continue
@@ -1435,37 +1436,69 @@ func (c *GitHubClient) EnrichPullRequest(
result.CheckAnnotations[check.ID] = annotations result.CheckAnnotations[check.ID] = annotations
continue continue
} }
nodes, err := c.checkAnnotations(ctx, check.ID) jobs = append(jobs, annotationJob{index: len(annotations), check: check})
if err != nil { annotations = append(annotations, annotationResult{checkID: check.ID})
result.Issues = append(result.Issues, DataIssue{
Component: "check annotations", Message: err.Error(),
})
continue
}
annotations := make([]CheckAnnotation, 0, len(nodes))
for _, annotation := range nodes {
annotations = append(annotations, CheckAnnotation{
Path: annotation.Path, StartLine: annotation.Location.Start.Line,
EndLine: annotation.Location.End.Line, Level: annotation.AnnotationLevel,
Title: annotation.Title, Message: annotation.Message,
})
}
c.storeAnnotations(check.ID, annotations)
result.CheckAnnotations[check.ID] = annotations
} }
var wait sync.WaitGroup
queue := make(chan annotationJob, len(jobs))
for _, job := range jobs {
queue <- job
}
close(queue)
for range min(4, len(jobs)) {
wait.Add(1)
go func() {
defer wait.Done()
for job := range queue {
nodes, err := c.checkAnnotations(ctx, job.check.ID)
if err != nil {
annotations[job.index].err = err
continue
}
converted := make([]CheckAnnotation, 0, len(nodes))
for _, annotation := range nodes {
converted = append(converted, CheckAnnotation{
Path: annotation.Path, StartLine: annotation.Location.Start.Line,
EndLine: annotation.Location.End.Line, Level: annotation.AnnotationLevel,
Title: annotation.Title, Message: annotation.Message,
})
}
c.storeAnnotations(job.check.ID, converted)
annotations[job.index].annotations = converted
}
}()
}
var conflictFiles []string
var conflictErr error
if details.Mergeable == "CONFLICTING" && c.conflicts != nil { if details.Mergeable == "CONFLICTING" && c.conflicts != nil {
files, err := c.loadConflictFiles( wait.Add(1)
ctx, details.RepositoryURL, details.Number, details.BaseRef, go func() {
details.BaseOID, details.HeadOID, defer wait.Done()
) conflictFiles, conflictErr = c.loadConflictFiles(
if err != nil { ctx, details.RepositoryURL, details.Number, details.BaseRef,
details.BaseOID, details.HeadOID,
)
}()
}
wait.Wait()
for _, loaded := range annotations {
if loaded.err != nil {
result.Issues = append(result.Issues, DataIssue{ result.Issues = append(result.Issues, DataIssue{
Component: "conflict file scan", Message: err.Error(), Component: "check annotations", Message: loaded.err.Error(),
}) })
} else { } else {
result.ConflictFiles = files result.CheckAnnotations[loaded.checkID] = loaded.annotations
} }
} }
if conflictErr != nil {
result.Issues = append(result.Issues, DataIssue{
Component: "conflict file scan", Message: conflictErr.Error(),
})
} else if conflictFiles != nil {
result.ConflictFiles = conflictFiles
}
level, summary := healthOK, "secondary PR data loaded" level, summary := healthOK, "secondary PR data loaded"
if len(result.Issues) > 0 { if len(result.Issues) > 0 {
level, summary = healthWarning, fmt.Sprintf( level, summary = healthWarning, fmt.Sprintf(

View File

@@ -3,11 +3,13 @@ package main
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"fmt"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"reflect" "reflect"
"strconv" "strconv"
"strings" "strings"
"sync/atomic"
"testing" "testing"
"time" "time"
) )
@@ -406,6 +408,44 @@ func TestCheckContextsAndAnnotationsArePaginated(t *testing.T) {
} }
} }
func TestPullRequestEnrichmentBoundsConcurrentAnnotationRequests(t *testing.T) {
var active, maximum atomic.Int32
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var request graphQLRequest
if err := json.NewDecoder(r.Body).Decode(&request); err != nil {
t.Error(err)
return
}
current := active.Add(1)
defer active.Add(-1)
for observed := maximum.Load(); current > observed; observed = maximum.Load() {
if maximum.CompareAndSwap(observed, current) {
break
}
}
time.Sleep(20 * time.Millisecond)
_, _ = w.Write([]byte(`{"data":{"node":{"annotations":{
"pageInfo":{"hasNextPage":false},"nodes":[]
}}}}`))
}))
defer server.Close()
client := NewGitHubClient(server.URL, "secret")
details := PRDetails{}
for index := range 6 {
details.Checks = append(details.Checks, Check{
ID: fmt.Sprintf("check-%d", index), Conclusion: "FAILURE",
})
}
result := client.EnrichPullRequest(context.Background(), details)
if len(result.CheckAnnotations) != len(details.Checks) || len(result.Issues) != 0 {
t.Fatalf("enrichment = %#v", result)
}
if got := maximum.Load(); got < 2 || got > 4 {
t.Fatalf("maximum concurrent annotation requests = %d, want 2..4", got)
}
}
func TestCheckQueriesUseCurrentGitHubSchemaShape(t *testing.T) { func TestCheckQueriesUseCurrentGitHubSchemaShape(t *testing.T) {
for name, query := range map[string]string{"annotations": checkAnnotationsPageQuery} { for name, query := range map[string]string{"annotations": checkAnnotationsPageQuery} {
if strings.Contains(query, "output {") || if strings.Contains(query, "output {") ||

4
go.mod
View File

@@ -9,6 +9,8 @@ require (
github.com/charmbracelet/glamour v1.0.0 github.com/charmbracelet/glamour v1.0.0
github.com/charmbracelet/lipgloss v1.1.1-0.20250404203927-76690c660834 github.com/charmbracelet/lipgloss v1.1.1-0.20250404203927-76690c660834
github.com/charmbracelet/x/ansi v0.10.2 github.com/charmbracelet/x/ansi v0.10.2
github.com/muesli/termenv v0.16.0
github.com/rivo/uniseg v0.4.7
) )
require ( require (
@@ -29,8 +31,6 @@ require (
github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect
github.com/muesli/cancelreader v0.2.2 // indirect github.com/muesli/cancelreader v0.2.2 // indirect
github.com/muesli/reflow v0.3.0 // indirect github.com/muesli/reflow v0.3.0 // indirect
github.com/muesli/termenv v0.16.0 // indirect
github.com/rivo/uniseg v0.4.7 // indirect
github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
github.com/yuin/goldmark v1.7.13 // indirect github.com/yuin/goldmark v1.7.13 // indirect
github.com/yuin/goldmark-emoji v1.0.6 // indirect github.com/yuin/goldmark-emoji v1.0.6 // indirect

View File

@@ -36,6 +36,17 @@ func (r *requestCoordinator) current(id uint64) bool {
return r.id == id return r.id == id
} }
func (r *requestCoordinator) supersede() uint64 {
r.mu.Lock()
defer r.mu.Unlock()
if r.cancel != nil {
r.cancel()
r.cancel = nil
}
r.id++
return r.id
}
type HealthLevel string type HealthLevel string
const ( const (

View File

@@ -145,7 +145,7 @@ func TestRequestCoordinatorCancelsSupersededRequest(t *testing.T) {
var coordinator requestCoordinator var coordinator requestCoordinator
first, cancelFirst, firstID := coordinator.start(time.Minute) first, cancelFirst, firstID := coordinator.start(time.Minute)
defer cancelFirst() defer cancelFirst()
_, cancelSecond, secondID := coordinator.start(time.Minute) second, cancelSecond, secondID := coordinator.start(time.Minute)
defer cancelSecond() defer cancelSecond()
select { select {
case <-first.Done(): case <-first.Done():
@@ -159,6 +159,16 @@ func TestRequestCoordinatorCancelsSupersededRequest(t *testing.T) {
if !errors.Is(first.Err(), context.Canceled) { if !errors.Is(first.Err(), context.Canceled) {
t.Fatalf("first context error = %v", first.Err()) t.Fatalf("first context error = %v", first.Err())
} }
claimedID := coordinator.supersede()
select {
case <-second.Done():
default:
t.Fatal("claimed snapshot did not cancel the active request")
}
if coordinator.current(secondID) || !coordinator.current(claimedID) {
t.Fatalf("claimed request ids: second=%t claimed=%t",
coordinator.current(secondID), coordinator.current(claimedID))
}
} }
func TestPartialRefreshPreservesLastCompleteSubsections(t *testing.T) { func TestPartialRefreshPreservesLastCompleteSubsections(t *testing.T) {

View File

@@ -6,18 +6,67 @@ import (
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
"sync"
"github.com/alecthomas/chroma/v2/lexers" "github.com/alecthomas/chroma/v2/lexers"
"github.com/alecthomas/chroma/v2/quick" "github.com/alecthomas/chroma/v2/quick"
) )
const reviewContextLines = 3 const reviewContextLines = 3
const highlightedDiffCacheLimit = 256
var codeHighlightTheme = "github-dark" var codeHighlightTheme = "github-dark"
var colorEnabled = true var colorEnabled = true
var hunkHeaderPattern = regexp.MustCompile(`^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@`) var hunkHeaderPattern = regexp.MustCompile(`^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@`)
type highlightedDiffCacheKey struct {
path, hunk, side, theme string
startLine, endLine int
color bool
}
type highlightedDiffCache struct {
mu sync.Mutex
entries map[highlightedDiffCacheKey][]highlightedDiffLine
order []highlightedDiffCacheKey
}
var highlightedDiffs = highlightedDiffCache{
entries: make(map[highlightedDiffCacheKey][]highlightedDiffLine),
}
func (c *highlightedDiffCache) get(key highlightedDiffCacheKey) ([]highlightedDiffLine, bool) {
c.mu.Lock()
defer c.mu.Unlock()
lines, ok := c.entries[key]
return append([]highlightedDiffLine(nil), lines...), ok
}
func (c *highlightedDiffCache) put(
key highlightedDiffCacheKey, lines []highlightedDiffLine,
) []highlightedDiffLine {
c.mu.Lock()
defer c.mu.Unlock()
if cached, ok := c.entries[key]; ok {
return append([]highlightedDiffLine(nil), cached...)
}
if len(c.entries) >= highlightedDiffCacheLimit {
delete(c.entries, c.order[0])
c.order = c.order[1:]
}
c.entries[key] = append([]highlightedDiffLine(nil), lines...)
c.order = append(c.order, key)
return append([]highlightedDiffLine(nil), lines...)
}
func (c *highlightedDiffCache) clear() {
c.mu.Lock()
defer c.mu.Unlock()
c.entries = make(map[highlightedDiffCacheKey][]highlightedDiffLine)
c.order = nil
}
type highlightedDiffLine struct { type highlightedDiffLine struct {
gutter string gutter string
code string code string
@@ -34,6 +83,17 @@ type parsedDiffLine struct {
} }
func highlightDiff(path, hunk string, startLine, endLine int, side string) []highlightedDiffLine { func highlightDiff(path, hunk string, startLine, endLine int, side string) []highlightedDiffLine {
key := highlightedDiffCacheKey{
path: path, hunk: hunk, side: side, theme: codeHighlightTheme,
startLine: startLine, endLine: endLine, color: colorEnabled,
}
if cached, ok := highlightedDiffs.get(key); ok {
return cached
}
return highlightedDiffs.put(key, highlightDiffUncached(path, hunk, startLine, endLine, side))
}
func highlightDiffUncached(path, hunk string, startLine, endLine int, side string) []highlightedDiffLine {
if hunk == "" { if hunk == "" {
return []highlightedDiffLine{{code: "(GitHub did not return a diff hunk)"}} return []highlightedDiffLine{{code: "(GitHub did not return a diff hunk)"}}
} }

View File

@@ -106,3 +106,17 @@ func TestHighlightDiffRemovesOnlyCommonIndent(t *testing.T) {
t.Fatalf("dedented code:\n%q\nwant:\n%q", got, want) t.Fatalf("dedented code:\n%q\nwant:\n%q", got, want)
} }
} }
func TestHighlightDiffCacheReturnsIndependentSlices(t *testing.T) {
highlightedDiffs.clear()
first := highlightDiff("main.go", "@@ -1 +1 @@\n-old\n+new", 1, 1, "RIGHT")
if len(first) == 0 {
t.Fatal("highlighted diff is empty")
}
first[0].code = "mutated"
second := highlightDiff("main.go", "@@ -1 +1 @@\n-old\n+new", 1, 1, "RIGHT")
if len(second) == 0 || second[0].code == "mutated" {
t.Fatalf("cached highlighted diff shares caller storage: %#v", second)
}
}

View File

@@ -63,8 +63,7 @@ func main() {
flag.Visit(func(item *flag.Flag) { visited[item.Name] = true }) flag.Visit(func(item *flag.Flag) { visited[item.Name] = true })
config, err := loadConfig( config, err := loadConfig(
*configFile, *configFile,
visited["config"] || os.Getenv("DIPLE_CONFIG") != "" || visited["config"] || os.Getenv("DIPLE_CONFIG") != "",
os.Getenv("GH_THREADS_CONFIG") != "",
) )
if err != nil { if err != nil {
exitf("configuration: %v", err) exitf("configuration: %v", err)

View File

@@ -27,28 +27,30 @@ const (
) )
type mutationOperation struct { type mutationOperation struct {
ID string `json:"id"` ID string `json:"id"`
Kind mutationKind `json:"kind"` Kind mutationKind `json:"kind"`
Owner string `json:"owner"` Owner string `json:"owner"`
Repository string `json:"repository"` Repository string `json:"repository"`
Number int `json:"number"` Number int `json:"number"`
PRID string `json:"pull_request_id"` PRID string `json:"pull_request_id"`
ThreadID string `json:"thread_id,omitempty"` ThreadID string `json:"thread_id,omitempty"`
Body string `json:"body,omitempty"` Body string `json:"body,omitempty"`
Resolved bool `json:"resolved,omitempty"` Resolved bool `json:"resolved,omitempty"`
Viewer string `json:"viewer,omitempty"` Viewer string `json:"viewer,omitempty"`
Original PullRequestMetadata `json:"original,omitempty"` Original PullRequestMetadata `json:"original,omitempty"`
Update PullRequestMetadata `json:"update,omitempty"` Update PullRequestMetadata `json:"update,omitempty"`
Permissions ViewerPermissions `json:"permissions"` Permissions ViewerPermissions `json:"permissions"`
ThreadCanReply bool `json:"thread_can_reply,omitempty"` ThreadCanReply bool `json:"thread_can_reply,omitempty"`
ThreadCanResolve bool `json:"thread_can_resolve,omitempty"` ThreadCanResolve bool `json:"thread_can_resolve,omitempty"`
ThreadCanUnresolve bool `json:"thread_can_unresolve,omitempty"` ThreadCanUnresolve bool `json:"thread_can_unresolve,omitempty"`
PeopleDone bool `json:"people_done,omitempty"` PeopleDone bool `json:"people_done,omitempty"`
Attempted bool `json:"attempted,omitempty"` Attempted bool `json:"attempted,omitempty"`
Blocked bool `json:"blocked,omitempty"` AwaitingVerification bool `json:"awaiting_verification,omitempty"`
Ambiguous bool `json:"ambiguous,omitempty"` Unverified bool `json:"unverified,omitempty"`
LastError string `json:"last_error,omitempty"` Blocked bool `json:"blocked,omitempty"`
EnqueuedAt time.Time `json:"enqueued_at"` Ambiguous bool `json:"ambiguous,omitempty"`
LastError string `json:"last_error,omitempty"`
EnqueuedAt time.Time `json:"enqueued_at"`
} }
type mutationQueueEnvelope struct { type mutationQueueEnvelope struct {
@@ -235,6 +237,7 @@ type mutationReplayState int
const ( const (
mutationReplayApplied mutationReplayState = iota mutationReplayApplied mutationReplayState = iota
mutationReplayVerifying
mutationReplayWaiting mutationReplayWaiting
mutationReplayBlocked mutationReplayBlocked
) )
@@ -303,18 +306,51 @@ func executeQueuedMutation(
operation mutationOperation, details PRDetails, operation mutationOperation, details PRDetails,
) mutationReplayMsg { ) mutationReplayMsg {
blocked := func(reason string, ambiguous bool) mutationReplayMsg { blocked := func(reason string, ambiguous bool) mutationReplayMsg {
operation.AwaitingVerification = false
operation.Unverified = true
if operation.Kind == mutationPREdit {
operation.PeopleDone = false
}
operation.Blocked, operation.Ambiguous, operation.LastError = true, ambiguous, reason operation.Blocked, operation.Ambiguous, operation.LastError = true, ambiguous, reason
if err := store.update(operation); err != nil { if err := store.update(operation); err != nil {
return mutationReplayMsg{operation: operation, state: mutationReplayBlocked, details: details, reason: reason, err: err} return mutationReplayMsg{operation: operation, state: mutationReplayBlocked, details: details, reason: reason, err: err}
} }
return mutationReplayMsg{operation: operation, state: mutationReplayBlocked, details: details, reason: reason} return mutationReplayMsg{operation: operation, state: mutationReplayBlocked, details: details, reason: reason}
} }
verifying := func() mutationReplayMsg {
operation.AwaitingVerification = true
operation.Blocked, operation.Ambiguous, operation.LastError = false, false, ""
if err := store.update(operation); err != nil {
return blocked("could not checkpoint successful mutation for verification", true)
}
return mutationReplayMsg{
operation: operation, state: mutationReplayVerifying, details: details,
}
}
markUnverified := func() *mutationReplayMsg {
if !operation.AwaitingVerification {
return nil
}
operation.AwaitingVerification = false
operation.Unverified = true
if operation.Kind == mutationPREdit {
operation.PeopleDone = false
}
if err := store.update(operation); err != nil {
result := blocked("could not record failed mutation verification", true)
return &result
}
return nil
}
thread := findReviewThread(details.Threads, operation.ThreadID) thread := findReviewThread(details.Threads, operation.ThreadID)
switch operation.Kind { switch operation.Kind {
case mutationReply: case mutationReply:
if queuedReplyPresent(details, operation) { if queuedReplyPresent(details, operation) {
return mutationReplayMsg{operation: operation, state: mutationReplayApplied, details: details} return mutationReplayMsg{operation: operation, state: mutationReplayApplied, details: details}
} }
if result := markUnverified(); result != nil {
return *result
}
if thread == nil { if thread == nil {
return blocked("the review thread no longer exists", false) return blocked("the review thread no longer exists", false)
} }
@@ -339,7 +375,7 @@ func executeQueuedMutation(
if _, err := writer.ReplyToThread(ctx, operation.ThreadID, operation.Body); err != nil { if _, err := writer.ReplyToThread(ctx, operation.ThreadID, operation.Body); err != nil {
return mutationReplayMsg{operation: operation, state: mutationReplayWaiting, details: details, err: err} return mutationReplayMsg{operation: operation, state: mutationReplayWaiting, details: details, err: err}
} }
return mutationReplayMsg{operation: operation, state: mutationReplayApplied, details: details} return verifying()
case mutationResolution: case mutationResolution:
if thread == nil { if thread == nil {
return blocked("the review thread no longer exists", false) return blocked("the review thread no longer exists", false)
@@ -347,6 +383,9 @@ func executeQueuedMutation(
if thread.IsResolved == operation.Resolved { if thread.IsResolved == operation.Resolved {
return mutationReplayMsg{operation: operation, state: mutationReplayApplied, details: details} return mutationReplayMsg{operation: operation, state: mutationReplayApplied, details: details}
} }
if result := markUnverified(); result != nil {
return *result
}
if operation.Resolved && !thread.ViewerCanResolve { if operation.Resolved && !thread.ViewerCanResolve {
return blocked("GitHub no longer grants resolve permission for this thread", false) return blocked("GitHub no longer grants resolve permission for this thread", false)
} }
@@ -368,8 +407,14 @@ func executeQueuedMutation(
if _, err := writer.SetThreadResolved(ctx, operation.ThreadID, operation.Resolved); err != nil { if _, err := writer.SetThreadResolved(ctx, operation.ThreadID, operation.Resolved); err != nil {
return mutationReplayMsg{operation: operation, state: mutationReplayWaiting, details: details, err: err} return mutationReplayMsg{operation: operation, state: mutationReplayWaiting, details: details, err: err}
} }
return mutationReplayMsg{operation: operation, state: mutationReplayApplied, details: details} return verifying()
case mutationPREdit: case mutationPREdit:
if queuedPREditPresent(details, operation.Update) {
return mutationReplayMsg{operation: operation, state: mutationReplayApplied, details: details}
}
if result := markUnverified(); result != nil {
return *result
}
if !details.Permissions.CanUpdatePR { if !details.Permissions.CanUpdatePR {
return blocked("GitHub no longer grants permission to update this pull request", false) return blocked("GitHub no longer grants permission to update this pull request", false)
} }
@@ -404,12 +449,18 @@ func executeQueuedMutation(
} }
details.Title, details.Body, details.BaseRef = result.Title, result.Body, result.BaseRef details.Title, details.Body, details.BaseRef = result.Title, result.Body, result.BaseRef
} }
return mutationReplayMsg{operation: operation, state: mutationReplayApplied, details: details} return verifying()
default: default:
return blocked("queued mutation has an unsupported kind", false) return blocked("queued mutation has an unsupported kind", false)
} }
} }
func queuedPREditPresent(details PRDetails, update PullRequestMetadata) bool {
return samePRMetadataCore(update, currentMetadata(details)) &&
slices.Equal(normalizedLogins(update.Reviewers), normalizedLogins(details.RequestedReviewers)) &&
slices.Equal(normalizedLogins(update.Assignees), normalizedLogins(details.Assignees))
}
func findReviewThread(threads []ReviewThread, id string) *ReviewThread { func findReviewThread(threads []ReviewThread, id string) *ReviewThread {
for index := range threads { for index := range threads {
if threads[index].ID == id { if threads[index].ID == id {
@@ -494,30 +545,38 @@ func (m App) projectQueuedMutations(details PRDetails) PRDetails {
} }
pendingID := "pending:" + operation.ID pendingID := "pending:" + operation.ID
found := false found := false
for _, comment := range thread.Comments { for index := range thread.Comments {
found = found || comment.ID == pendingID if thread.Comments[index].ID == pendingID {
thread.Comments[index].Pending = mutationNeedsAttention(operation)
found = true
}
} }
if !found { if !found {
thread.Comments = append(thread.Comments, ReviewComment{ thread.Comments = append(thread.Comments, ReviewComment{
ID: pendingID, Author: operation.Viewer, Body: operation.Body, ID: pendingID, Author: operation.Viewer, Body: operation.Body,
CreatedAt: operation.EnqueuedAt, Pending: true, CreatedAt: operation.EnqueuedAt, Pending: mutationNeedsAttention(operation),
}) })
} }
case mutationResolution: case mutationResolution:
if thread := findReviewThread(details.Threads, operation.ThreadID); thread != nil { if thread := findReviewThread(details.Threads, operation.ThreadID); thread != nil {
thread.IsResolved, thread.Pending = operation.Resolved, true thread.IsResolved = operation.Resolved
thread.Pending = mutationNeedsAttention(operation)
} }
case mutationPREdit: case mutationPREdit:
details.Title, details.Body, details.BaseRef = operation.Update.Title, operation.Update.Body, operation.Update.BaseRef details.Title, details.Body, details.BaseRef = operation.Update.Title, operation.Update.Body, operation.Update.BaseRef
details.RequestedReviewers = slices.Clone(operation.Update.Reviewers) details.RequestedReviewers = slices.Clone(operation.Update.Reviewers)
details.Assignees = slices.Clone(operation.Update.Assignees) details.Assignees = slices.Clone(operation.Update.Assignees)
details.Reviewers = projectRequestedReviewers(details.Reviewers, operation.Update.Reviewers) details.Reviewers = projectRequestedReviewers(details.Reviewers, operation.Update.Reviewers)
details.Pending = true details.Pending = mutationNeedsAttention(operation)
} }
} }
return details return details
} }
func mutationNeedsAttention(operation mutationOperation) bool {
return operation.Blocked || operation.Unverified
}
func (m App) projectQueuedPullRequests(prs []PullRequest) []PullRequest { func (m App) projectQueuedPullRequests(prs []PullRequest) []PullRequest {
result := slices.Clone(prs) result := slices.Clone(prs)
for _, operation := range m.mutations.list() { for _, operation := range m.mutations.list() {
@@ -527,7 +586,8 @@ func (m App) projectQueuedPullRequests(prs []PullRequest) []PullRequest {
for index := range result { for index := range result {
if result[index].Owner == operation.Owner && result[index].Repository == operation.Repository && if result[index].Owner == operation.Owner && result[index].Repository == operation.Repository &&
result[index].Number == operation.Number { result[index].Number == operation.Number {
result[index].Title, result[index].Pending = operation.Update.Title, true result[index].Title = operation.Update.Title
result[index].Pending = mutationNeedsAttention(operation)
} }
} }
} }
@@ -592,7 +652,8 @@ func (m *App) resolveBlockedMutation(choice int) tea.Cmd {
} }
return command return command
case "Retry this mutation now": case "Retry this mutation now":
operation.Attempted, operation.Blocked = false, false operation.Attempted, operation.AwaitingVerification = false, false
operation.Unverified, operation.Blocked = false, false
operation.Ambiguous, operation.LastError = false, "" operation.Ambiguous, operation.LastError = false, ""
if err := m.mutations.update(operation); err != nil { if err := m.mutations.update(operation); err != nil {
m.err = err m.err = err

View File

@@ -150,9 +150,9 @@ func TestQueuedMutationsProjectWithoutChangingSnapshot(t *testing.T) {
if snapshot.Title != "remote" || snapshot.Threads[0].IsResolved || len(snapshot.Threads[0].Comments) != 0 { if snapshot.Title != "remote" || snapshot.Threads[0].IsResolved || len(snapshot.Threads[0].Comments) != 0 {
t.Fatalf("source snapshot was mutated: %#v", snapshot) t.Fatalf("source snapshot was mutated: %#v", snapshot)
} }
if projected.Title != "queued" || !projected.Pending || !projected.Threads[0].IsResolved || if projected.Title != "queued" || projected.Pending || !projected.Threads[0].IsResolved ||
!projected.Threads[0].Pending || len(projected.Threads[0].Comments) != 1 || projected.Threads[0].Pending || len(projected.Threads[0].Comments) != 1 ||
!projected.Threads[0].Comments[0].Pending { projected.Threads[0].Comments[0].Pending {
t.Fatalf("projection = %#v", projected) t.Fatalf("projection = %#v", projected)
} }
} }
@@ -173,7 +173,7 @@ func TestCachedGrantedPermissionQueuesReply(t *testing.T) {
updated, command := m.Update(message) updated, command := m.Update(message)
m = updated.(App) m = updated.(App)
if store.count() != 1 || m.writeMode != writeNone || if store.count() != 1 || m.writeMode != writeNone ||
len(m.details.Threads[0].Comments) != 1 || !m.details.Threads[0].Comments[0].Pending { len(m.details.Threads[0].Comments) != 1 || m.details.Threads[0].Comments[0].Pending {
t.Fatalf("queued cached reply: count=%d mode=%d details=%#v command=%v", t.Fatalf("queued cached reply: count=%d mode=%d details=%#v command=%v",
store.count(), m.writeMode, m.details, command) store.count(), m.writeMode, m.details, command)
} }
@@ -303,6 +303,87 @@ func TestReplayReconcilesReplyBeforeAskingAboutAmbiguousDelivery(t *testing.T) {
} }
} }
func TestSuccessfulResolutionWaitsForLiveVerification(t *testing.T) {
store := loadMutationQueue("")
operation := mutationOperation{
ID: "resolution", Kind: mutationResolution, Owner: "o", Repository: "r", Number: 1,
ThreadID: "thread", Resolved: true, EnqueuedAt: time.Now(),
}
if err := store.add(operation); err != nil {
t.Fatal(err)
}
service := &recordingService{}
details := PRDetails{Threads: []ReviewThread{{ID: "thread", ViewerCanResolve: true}}}
result := executeQueuedMutation(context.Background(), service, store, operation, details)
if result.state != mutationReplayVerifying || !service.writeResolved {
t.Fatalf("successful resolution = %#v service=%#v", result, service)
}
stored, ok := store.front()
if !ok || !stored.AwaitingVerification || stored.Unverified || stored.Blocked {
t.Fatalf("resolution awaiting verification = %#v", stored)
}
details.Threads[0].IsResolved = true
result = executeQueuedMutation(context.Background(), service, store, stored, details)
if result.state != mutationReplayApplied {
t.Fatalf("verified resolution = %#v", result)
}
}
func TestRetryableReplyFailureRemainsOptimisticallyApplied(t *testing.T) {
store := loadMutationQueue("")
operation := mutationOperation{
ID: "reply", Kind: mutationReply, Owner: "o", Repository: "r", Number: 1,
ThreadID: "thread", Body: "body", Viewer: "me", EnqueuedAt: time.Now(),
}
if err := store.add(operation); err != nil {
t.Fatal(err)
}
details := PRDetails{
PullRequest: PullRequest{Owner: "o", Repository: "r", Number: 1},
Threads: []ReviewThread{{ID: "thread", ViewerCanReply: true}},
}
result := executeQueuedMutation(
context.Background(), &failingReplyService{}, store, operation, details,
)
if result.state != mutationReplayWaiting || result.err == nil {
t.Fatalf("retryable reply = %#v", result)
}
m := NewApp(nil, "o", "r", false, 50, time.Minute)
m.mutations = store
projected := m.projectQueuedMutations(details)
if len(projected.Threads[0].Comments) != 1 || projected.Threads[0].Comments[0].Pending {
t.Fatalf("retryable reply was not optimistic: %#v", projected.Threads[0].Comments)
}
}
func TestFailedReplyVerificationMarksOptimisticCommentForAttention(t *testing.T) {
store := loadMutationQueue("")
operation := mutationOperation{
ID: "reply", Kind: mutationReply, Owner: "o", Repository: "r", Number: 1,
ThreadID: "thread", Body: "body", Viewer: "me", Attempted: true,
AwaitingVerification: true, EnqueuedAt: time.Now(),
}
if err := store.add(operation); err != nil {
t.Fatal(err)
}
details := PRDetails{
PullRequest: PullRequest{Owner: "o", Repository: "r", Number: 1},
Threads: []ReviewThread{{ID: "thread", ViewerCanReply: true}},
}
result := executeQueuedMutation(context.Background(), &recordingService{}, store, operation, details)
if result.state != mutationReplayBlocked || !result.operation.Unverified {
t.Fatalf("failed verification = %#v", result)
}
m := NewApp(nil, "o", "r", false, 50, time.Minute)
m.mutations = store
projected := m.projectQueuedMutations(details)
if len(projected.Threads[0].Comments) != 1 || !projected.Threads[0].Comments[0].Pending {
t.Fatalf("unverified reply was not marked for attention: %#v", projected.Threads[0].Comments)
}
}
func TestQueuedPREditThreeWayMergeOnlyBlocksConflictingFields(t *testing.T) { func TestQueuedPREditThreeWayMergeOnlyBlocksConflictingFields(t *testing.T) {
base := PullRequestMetadata{Title: "old", Body: "old body", BaseRef: "main"} base := PullRequestMetadata{Title: "old", Body: "old body", BaseRef: "main"}
desired := base desired := base

View File

@@ -28,6 +28,7 @@ func (m *App) startPREdit() tea.Cmd {
return nil return nil
} }
m.writeMode = writePREdit m.writeMode = writePREdit
m.prEditGeneration++
m.prEditField = prEditBodyField m.prEditField = prEditBodyField
modal := m.editorMode == "vim" modal := m.editorMode == "vim"
m.prEditEditors[prEditTitleField] = newTextEditor(m.details.Title, modal) m.prEditEditors[prEditTitleField] = newTextEditor(m.details.Title, modal)
@@ -71,12 +72,14 @@ func (m *App) loadPREditBranches() tea.Cmd {
return nil return nil
} }
m.prEditBranchesLoading = true m.prEditBranchesLoading = true
owner, repo := m.details.Owner, m.details.Repository owner, repo, generation := m.details.Owner, m.details.Repository, m.prEditGeneration
return func() tea.Msg { return func() tea.Msg {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel() defer cancel()
branches, err := service.ListBranches(ctx, owner, repo) branches, err := service.ListBranches(ctx, owner, repo)
return branchesLoadedMsg{owner: owner, repo: repo, branches: branches, err: err} return branchesLoadedMsg{
generation: generation, owner: owner, repo: repo, branches: branches, err: err,
}
} }
} }
@@ -87,17 +90,19 @@ func (m *App) loadPREditUsers() tea.Cmd {
return nil return nil
} }
m.prEditUsersLoading = true m.prEditUsersLoading = true
owner, repo := m.details.Owner, m.details.Repository owner, repo, generation := m.details.Owner, m.details.Repository, m.prEditGeneration
return func() tea.Msg { return func() tea.Msg {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel() defer cancel()
users, err := service.ListRepositoryUsers(ctx, owner, repo) users, err := service.ListRepositoryUsers(ctx, owner, repo)
return repositoryUsersLoadedMsg{owner: owner, repo: repo, users: users, err: err} return repositoryUsersLoadedMsg{
generation: generation, owner: owner, repo: repo, users: users, err: err,
}
} }
} }
func (m App) pullRequestUpdateUnavailable() string { func (m App) pullRequestUpdateUnavailable() string {
if m.loading { if m.loading && m.mutations == nil {
return "pull request update unavailable while PR data is refreshing" return "pull request update unavailable while PR data is refreshing"
} }
if m.details.FromCache && m.mutations == nil { if m.details.FromCache && m.mutations == nil {

View File

@@ -617,10 +617,6 @@ func normalEditorLineLast(value string, cursor, wrapWidth int) int {
return start return start
} }
func moveNormalCursorLine(value string, cursor, delta int) int {
return moveEditorCursorLine(value, cursor, delta, 0, true)
}
func nextWordStart(value string, cursor int, big bool) int { func nextWordStart(value string, cursor int, big bool) int {
runes := []rune(value) runes := []rune(value)
cursor = clamp(cursor, 0, len(runes)) cursor = clamp(cursor, 0, len(runes))
@@ -666,10 +662,6 @@ func previousWordStart(value string, cursor int, big bool) int {
return cursor return cursor
} }
func wordEnd(value string, cursor int, big bool) int {
return wordEndAtWidth(value, cursor, big, 0)
}
func wordEndAtWidth(value string, cursor int, big bool, wrapWidth int) int { func wordEndAtWidth(value string, cursor int, big bool, wrapWidth int) int {
runes := []rune(value) runes := []rune(value)
cursor = clamp(cursor, 0, len(runes)) cursor = clamp(cursor, 0, len(runes))

View File

@@ -103,6 +103,7 @@ func applyTheme(name string, custom ...CustomThemeConfig) error {
commentMarkdownRenderers.Clear() commentMarkdownRenderers.Clear()
commentMarkdownLines.clear() commentMarkdownLines.clear()
renderedSuggestions.clear() renderedSuggestions.clear()
highlightedDiffs.clear()
return nil return nil
} }

98
tui.go
View File

@@ -102,13 +102,15 @@ type detailsLoadedMsg struct {
} }
type branchesLoadedMsg struct { type branchesLoadedMsg struct {
owner string generation uint64
repo string owner string
branches []RepositoryBranch repo string
err error branches []RepositoryBranch
err error
} }
type repositoryUsersLoadedMsg struct { type repositoryUsersLoadedMsg struct {
generation uint64
owner, repo string owner, repo string
users []RepositoryUser users []RepositoryUser
err error err error
@@ -171,6 +173,7 @@ type App struct {
prEditUsersLoading bool prEditUsersLoading bool
prEditUsersError string prEditUsersError string
prEditUserIndex int prEditUserIndex int
prEditGeneration uint64
cursorOutput *terminalCursorOutput cursorOutput *terminalCursorOutput
foldResolved bool foldResolved bool
@@ -209,6 +212,7 @@ type App struct {
aiProgress AIRunProgress aiProgress AIRunProgress
aiSpinner int aiSpinner int
aiEvents <-chan tea.Msg aiEvents <-chan tea.Msg
aiGeneration uint64
difflet diffletModel difflet diffletModel
mutations *mutationQueueStore mutations *mutationQueueStore
mutationReplayBusy bool mutationReplayBusy bool
@@ -573,7 +577,7 @@ func (m App) writeActionUnavailable(action string, thread *ReviewThread) string
} }
return "" return ""
} }
if m.loading { if m.loading && m.mutations == nil {
return "write action unavailable while PR data is refreshing" return "write action unavailable while PR data is refreshing"
} }
if m.details.FromCache && m.mutations == nil { if m.details.FromCache && m.mutations == nil {
@@ -995,7 +999,23 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
} }
return m, m.difflet.setState(diffletRecoverableError) return m, m.difflet.setState(diffletRecoverableError)
} }
selected := ""
if msg.operation.Kind == mutationResolution {
selected = m.selectionAfterResolution(
msg.operation.ThreadID, msg.operation.Resolved,
)
}
m.details = m.projectQueuedMutations(m.details) m.details = m.projectQueuedMutations(m.details)
if msg.operation.Kind == mutationResolution {
if thread := m.threadByID(msg.operation.ThreadID); thread != nil {
m.folded[thread.ID] = thread.IsResolved && m.foldResolved
}
if msg.operation.Resolved {
m.markThreadRead(msg.operation.ThreadID)
}
sortReviewThreads(m.details.Threads, m.threadStatusOrder, m.threadWithinStatus)
m.threadIndex = indexThread(m.details.Threads, selected)
}
m.err = nil m.err = nil
switch msg.operation.Kind { switch msg.operation.Kind {
case mutationReply: case mutationReply:
@@ -1013,18 +1033,35 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
} }
m.writeMode = writeNone m.writeMode = writeNone
if m.details.FromCache { if m.details.FromCache {
return m, m.difflet.setState(diffletIdle) return m, m.difflet.setState(diffletSuccess)
} }
return m, tea.Batch(m.startMutationReplay(), m.difflet.setState(diffletLoading)) return m, tea.Batch(m.startMutationReplay(), m.difflet.setState(diffletSuccess))
case mutationReplayMsg: case mutationReplayMsg:
m.mutationReplayBusy = false m.mutationReplayBusy = false
if msg.state == mutationReplayWaiting { if msg.state == mutationReplayWaiting {
if msg.err != nil { if msg.err != nil {
m.recordHealth("mutation replay", healthWarning, msg.err.Error()) m.recordHealth("mutation replay", healthWarning, msg.err.Error())
} }
return m, m.difflet.setState(diffletRecoverableError) m.details = m.projectQueuedMutations(m.details)
m.err = nil
return m, m.difflet.setState(diffletSuccess)
}
if msg.state == mutationReplayVerifying {
m.details = m.projectQueuedMutations(m.details)
m.err = nil
if m.details.Owner == msg.operation.Owner &&
m.details.Repository == msg.operation.Repository &&
m.details.Number == msg.operation.Number {
m.loading = true
return m, tea.Batch(
m.loadDetails(m.details.PullRequest, false),
m.difflet.setState(diffletSuccess),
)
}
return m, m.difflet.setState(diffletSuccess)
} }
if msg.state == mutationReplayBlocked { if msg.state == mutationReplayBlocked {
m.details = m.projectQueuedMutations(m.details)
m.blockedMutation = &msg.operation m.blockedMutation = &msg.operation
m.blockedMutationDetails = msg.details m.blockedMutationDetails = msg.details
m.blockedMutationChoice = 0 m.blockedMutationChoice = 0
@@ -1042,14 +1079,24 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
if m.details.Owner == msg.operation.Owner && if m.details.Owner == msg.operation.Owner &&
m.details.Repository == msg.operation.Repository && m.details.Number == msg.operation.Number { m.details.Repository == msg.operation.Repository && m.details.Number == msg.operation.Number {
m.loading = true m.loading = true
details := msg.details
requestID := uint64(0)
if m.requests != nil {
requestID = m.requests.supersede()
}
return m, tea.Batch( return m, tea.Batch(
m.loadDetails(m.details.PullRequest, false), func() tea.Msg {
return detailsLoadedMsg{
owner: details.Owner, repo: details.Repository, number: details.Number,
details: details, requestID: requestID,
}
},
m.difflet.setState(diffletSuccess), m.difflet.setState(diffletSuccess),
) )
} }
return m, tea.Batch(m.startMutationReplay(), m.difflet.setState(diffletSuccess)) return m, tea.Batch(m.startMutationReplay(), m.difflet.setState(diffletSuccess))
case branchesLoadedMsg: case branchesLoadedMsg:
if m.writeMode != writePREdit || if msg.generation != m.prEditGeneration || m.writeMode != writePREdit ||
msg.owner != m.details.Owner || msg.repo != m.details.Repository { msg.owner != m.details.Owner || msg.repo != m.details.Repository {
return m, nil return m, nil
} }
@@ -1065,7 +1112,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.prEditBranchIndex = 0 m.prEditBranchIndex = 0
m.ensurePREditCursorVisible() m.ensurePREditCursorVisible()
case repositoryUsersLoadedMsg: case repositoryUsersLoadedMsg:
if m.writeMode != writePREdit || if msg.generation != m.prEditGeneration || m.writeMode != writePREdit ||
msg.owner != m.details.Owner || msg.repo != m.details.Repository { msg.owner != m.details.Owner || msg.repo != m.details.Repository {
return m, nil return m, nil
} }
@@ -1087,17 +1134,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
m.recordHealth("thread resolution", healthError, msg.err.Error()) m.recordHealth("thread resolution", healthError, msg.err.Error())
return m, m.difflet.setState(diffletRecoverableError) return m, m.difflet.setState(diffletRecoverableError)
} }
selected := "" selected := m.selectionAfterResolution(msg.threadID, msg.thread.IsResolved)
if m.threadIndex >= 0 && m.threadIndex < len(m.details.Threads) {
selected = m.details.Threads[m.threadIndex].ID
}
if msg.thread.IsResolved && selected == msg.threadID {
if m.threadIndex+1 < len(m.details.Threads) {
selected = m.details.Threads[m.threadIndex+1].ID
} else if m.threadIndex > 0 {
selected = m.details.Threads[m.threadIndex-1].ID
}
}
for index := range m.details.Threads { for index := range m.details.Threads {
if m.details.Threads[index].ID != msg.threadID { if m.details.Threads[index].ID != msg.threadID {
continue continue
@@ -2456,6 +2493,23 @@ func (m App) threadByID(id string) *ReviewThread {
return nil return nil
} }
func (m App) selectionAfterResolution(threadID string, resolved bool) string {
if m.threadIndex < 0 || m.threadIndex >= len(m.details.Threads) {
return ""
}
selected := m.details.Threads[m.threadIndex].ID
if !resolved || selected != threadID {
return selected
}
if m.threadIndex+1 < len(m.details.Threads) {
return m.details.Threads[m.threadIndex+1].ID
}
if m.threadIndex > 0 {
return m.details.Threads[m.threadIndex-1].ID
}
return selected
}
type helpBinding struct { type helpBinding struct {
key string key string
action string action string

View File

@@ -919,6 +919,63 @@ func TestPullRequestAIPreparationFailureIgnoresOldDiscussionThread(t *testing.T)
} }
} }
func TestAIStaleOperationMessagesAreIgnored(t *testing.T) {
m := NewApp(nil, "o", "r", false, 50, time.Second)
m.aiGeneration = 2
m.aiMode = aiBusy
m.aiProgress = AIRunProgress{Stage: "current run"}
updated, command, handled := m.updateAI(aiProgressMsg{
generation: 1, progress: AIRunProgress{Stage: "stale run"},
})
m = updated.(App)
if !handled || command != nil || m.aiProgress.Stage != "current run" {
t.Fatalf("stale progress handled=%v command=%v progress=%q",
handled, command, m.aiProgress.Stage)
}
updated, command, handled = m.updateAI(aiCompletedMsg{generation: 1})
m = updated.(App)
if !handled || command != nil || m.aiMode != aiBusy {
t.Fatalf("stale completion handled=%v command=%v mode=%v",
handled, command, m.aiMode)
}
m.aiMode = aiPreparing
updated, command, handled = m.updateAI(aiPreparedMsg{generation: 1})
m = updated.(App)
if !handled || command != nil || m.aiMode != aiPreparing {
t.Fatalf("stale preparation handled=%v command=%v mode=%v",
handled, command, m.aiMode)
}
}
func TestPREditStaleRecommendationMessagesAreIgnored(t *testing.T) {
m := NewApp(nil, "o", "r", false, 50, time.Second)
m.details = PRDetails{PullRequest: PullRequest{Owner: "o", Repository: "r"}}
m.writeMode = writePREdit
m.prEditGeneration = 2
m.prEditBranches = []RepositoryBranch{{Name: "current"}}
updated, command := m.Update(branchesLoadedMsg{
generation: 1, owner: "o", repo: "r",
branches: []RepositoryBranch{{Name: "stale"}},
})
m = updated.(App)
if command != nil || len(m.prEditBranches) != 1 || m.prEditBranches[0].Name != "current" {
t.Fatalf("stale branches command=%v branches=%#v", command, m.prEditBranches)
}
updated, command = m.Update(branchesLoadedMsg{
generation: 2, owner: "o", repo: "r",
branches: []RepositoryBranch{{Name: "accepted"}},
})
m = updated.(App)
if command != nil || len(m.prEditBranches) != 1 || m.prEditBranches[0].Name != "accepted" {
t.Fatalf("current branches command=%v branches=%#v", command, m.prEditBranches)
}
}
func TestVimModeUsesGlobalFooterBar(t *testing.T) { func TestVimModeUsesGlobalFooterBar(t *testing.T) {
m := NewApp(nil, "o", "r", false, 50, time.Second) m := NewApp(nil, "o", "r", false, 50, time.Second)
m.width, m.height = 80, 10 m.width, m.height = 80, 10
@@ -1572,6 +1629,60 @@ func TestResolvingSelectedThreadKeepsSelectionNearItsPreviousPosition(t *testing
} }
} }
func TestQueuedResolutionOptimisticallyMovesToNeighborWithoutPendingLabel(t *testing.T) {
store := loadMutationQueue("")
operation := mutationOperation{
ID: "resolve", Kind: mutationResolution, Owner: "o", Repository: "r", Number: 1,
ThreadID: "b", Resolved: true, EnqueuedAt: time.Now(),
}
if err := store.add(operation); err != nil {
t.Fatal(err)
}
settings := defaultAppSettings()
settings.Mutations = store
m := NewAppWithSettings(nil, "o", "r", false, 50, time.Minute, settings)
m.screen, m.loading, m.focus = threadScreen, false, threadDetailPane
m.details = PRDetails{
PullRequest: PullRequest{Owner: "o", Repository: "r", Number: 1},
Threads: []ReviewThread{
{ID: "a", Path: "a.go"},
{ID: "b", Path: "b.go"},
{ID: "c", Path: "c.go"},
},
}
m.threadIndex = 1
updated, _ := m.Update(mutationQueuedMsg{operation: operation})
m = updated.(App)
if got := m.details.Threads[m.threadIndex].ID; got != "c" {
t.Fatalf("selected thread = %q, want c", got)
}
thread := m.threadByID("b")
if thread == nil || !thread.IsResolved || thread.Pending {
t.Fatalf("optimistic resolved thread = %#v", thread)
}
}
func TestMutationQueueAllowsResolvingNextThreadDuringVerificationRefresh(t *testing.T) {
store := loadMutationQueue("")
settings := defaultAppSettings()
settings.Mutations = store
m := NewAppWithSettings(&recordingService{}, "o", "r", false, 50, time.Minute, settings)
m.screen, m.loading = threadScreen, true
m.details = PRDetails{
PullRequest: PullRequest{Owner: "o", Repository: "r", Number: 1},
Threads: []ReviewThread{{
ID: "next", ViewerCanResolve: true,
}},
}
m.startResolveToggle()
if m.err != nil || m.writeMode != writeResolveConfirm || !m.resolveTarget {
t.Fatalf("resolve during verification mode=%d target=%t err=%v",
m.writeMode, m.resolveTarget, m.err)
}
}
func TestPollingMarksNewThreadCommentsUnread(t *testing.T) { func TestPollingMarksNewThreadCommentsUnread(t *testing.T) {
m := NewApp(nil, "o", "r", false, 50, 10*time.Second) m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
m.screen, m.width, m.height = threadScreen, 80, 8 m.screen, m.width, m.height = threadScreen, 80, 8

View File

@@ -1,3 +1,3 @@
package main package main
const dipleVersion = "0.4.0" const dipleVersion = "0.5.1"