fix(agentctl): log canceled git status refreshes at debug (#2398)

This commit is contained in:
Carlos Florêncio
2026-08-07 21:01:47 +01:00
committed by GitHub
parent 9ed977a3fe
commit 33ecd0d945
2 changed files with 88 additions and 2 deletions
@@ -2,6 +2,7 @@ package process
import (
"context"
"errors"
"fmt"
"path/filepath"
"regexp"
@@ -36,10 +37,19 @@ func (wt *WorkspaceTracker) updateGitStatus(ctx context.Context) bool {
func (wt *WorkspaceTracker) updateGitStatusClass(ctx context.Context, class subproc.GitWorkClass) bool {
status, err := wt.getGitStatusClass(ctx, class)
if err != nil {
wt.logger.Warn("updateGitStatus: getGitStatus failed", zap.Error(err))
// A cancellation is the expected outcome when the tracker is being
// torn down (Stop cancels cancelCtx, which kills the in-flight git
// command) or when the poll's admission slot is revoked. That is not a
// failure worth a warning — it mirrors handleGitPollFailure, which also
// downgrades cancellation-class errors to Debug to keep shutdown quiet.
if wt.isGitStatusCancellation(err) {
wt.logger.Debug("updateGitStatus: getGitStatus canceled", zap.Error(err))
} else {
wt.logger.Warn("updateGitStatus: getGitStatus failed", zap.Error(err))
}
return false
}
if ctx.Err() != nil || wt.cancelCtx.Err() != nil {
if ctx.Err() != nil || (wt.cancelCtx != nil && wt.cancelCtx.Err() != nil) {
return false
}
@@ -52,6 +62,17 @@ func (wt *WorkspaceTracker) updateGitStatusClass(ctx context.Context, class subp
return true
}
// isGitStatusCancellation reports whether a getGitStatus error is a benign
// cancellation rather than a real git failure: the passed context or the
// tracker's own shutdown context was canceled, or the background admission slot
// was revoked. These are all expected during teardown and don't warrant a warn.
func (wt *WorkspaceTracker) isGitStatusCancellation(err error) bool {
if errors.Is(err, context.Canceled) || errors.Is(err, subproc.ErrAdmissionCanceled) {
return true
}
return wt.cancelCtx != nil && wt.cancelCtx.Err() != nil
}
// tryUpdateGitStatus attempts a non-blocking git status update. If another
// update is already in progress (from the other polling loop or an explicit
// refresh), the call is skipped — the running update will produce the same
@@ -2,14 +2,79 @@ package process
import (
"context"
"errors"
"os"
"path/filepath"
"testing"
"github.com/kandev/kandev/internal/agentctl/types"
"github.com/kandev/kandev/internal/common/subproc"
"go.uber.org/zap/zapcore"
)
// TestUpdateGitStatus_CancellationLogsDebugNotWarn verifies that a canceled
// observation (the expected outcome during tracker teardown) is logged at
// Debug, while a genuine failure still warns. This keeps shutdown from
// emitting a burst of "getGitStatus failed" warnings for context.Canceled.
func TestUpdateGitStatus_CancellationLogsDebugNotWarn(t *testing.T) {
tests := []struct {
name string
observerErr error
cancelWT bool
wantWarn bool
}{
{"context canceled", context.Canceled, false, false},
{"admission canceled", subproc.ErrAdmissionCanceled, false, false},
{"tracker context canceled with opaque error", errors.New("exit status 1"), true, false},
{"real failure", errors.New("git exploded"), false, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
log, observed := newObservedTestLogger(t)
wt := NewWorkspaceTracker(t.TempDir(), log)
t.Cleanup(wt.Stop)
wt.gitStatusObserver = func(context.Context) (types.GitStatusUpdate, error) {
return types.GitStatusUpdate{}, tt.observerErr
}
if tt.cancelWT {
wt.cancelFunc()
}
if wt.updateGitStatus(context.Background()) {
t.Fatal("updateGitStatus returned true on error")
}
warns := observed.FilterMessage("updateGitStatus: getGitStatus failed").
FilterLevelExact(zapcore.WarnLevel).Len()
debugs := observed.FilterMessage("updateGitStatus: getGitStatus canceled").
FilterLevelExact(zapcore.DebugLevel).Len()
if tt.wantWarn {
if warns != 1 {
t.Fatalf("warn count = %d, want 1", warns)
}
if debugs != 0 {
t.Fatalf("debug count = %d, want 0", debugs)
}
return
}
if warns != 0 {
t.Fatalf("warn count = %d, want 0 for cancellation", warns)
}
if debugs != 1 {
t.Fatalf("debug count = %d, want 1 for cancellation", debugs)
}
})
}
}
func TestWorkspaceTrackerIsGitStatusCancellation_NilCancelCtx(t *testing.T) {
wt := &WorkspaceTracker{}
if wt.isGitStatusCancellation(errors.New("git exploded")) {
t.Fatal("isGitStatusCancellation returned true with nil cancelCtx")
}
}
func TestUnquoteGitPath(t *testing.T) {
tests := []struct {
name string