fix: resolve pending and jumping
This commit is contained in:
13
README.md
13
README.md
@@ -523,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
|
||||||
|
|||||||
11
health.go
11
health.go
@@ -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 (
|
||||||
|
|||||||
@@ -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) {
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -102,7 +102,7 @@ func (m *App) loadPREditUsers() tea.Cmd {
|
|||||||
}
|
}
|
||||||
|
|
||||||
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 {
|
||||||
|
|||||||
82
tui.go
82
tui.go
@@ -577,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 {
|
||||||
@@ -999,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:
|
||||||
@@ -1017,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
|
||||||
@@ -1046,8 +1079,18 @@ 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),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -1091,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
|
||||||
@@ -2460,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
|
||||||
|
|||||||
54
tui_test.go
54
tui_test.go
@@ -1629,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
|
||||||
|
|||||||
@@ -1,3 +1,3 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
const dipleVersion = "0.5.0"
|
const dipleVersion = "0.5.1"
|
||||||
|
|||||||
Reference in New Issue
Block a user