Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ linters:
- gochecknoglobals # checks that no global variables exist
- cyclop # replaced by revive
- gocyclo # replaced by revive
- forbidigo # needs configuration to be useful
- funlen # replaced by revive
- godox # TODO's are OK
- ireturn # It's OK
Expand Down Expand Up @@ -151,10 +152,10 @@ linters:

nakedret:
# Default: 30
max-func-lines: 4
max-func-lines: 7

nestif:
min-complexity: 12
min-complexity: 15

nolintlint:
# Exclude following linters from requiring an explanation.
Expand All @@ -170,17 +171,11 @@ linters:
rules:
- name: add-constant
severity: warning
disabled: false
exclude: [""]
arguments:
- max-lit-count: "5"
allow-strs: '"","\n"'
allow-ints: "0,1,2,3,24,30,365,1024,0o600,0o700,0o750,0o755"
allow-floats: "0.0,0.,1.0,1.,2.0,2."
disabled: true
- name: cognitive-complexity
arguments: [55]
disabled: true # prefer maintidx
- name: cyclomatic
arguments: [60]
disabled: true # prefer maintidx
- name: function-length
arguments: [150, 225]
- name: line-length-limit
Expand Down Expand Up @@ -212,8 +207,14 @@ linters:
os-temp-dir: true

varnamelen:
max-distance: 40
max-distance: 75
min-name-length: 2
check-receivers: false
ignore-names:
- r
- w
- f
- err

exclusions:
# Default: []
Expand Down
16 changes: 13 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ LINTERS :=
FIXERS :=

GOLANGCI_LINT_CONFIG := $(LINT_ROOT)/.golangci.yml
GOLANGCI_LINT_VERSION ?= v2.3.1
GOLANGCI_LINT_VERSION ?= v2.5.0
GOLANGCI_LINT_BIN := $(LINT_ROOT)/out/linters/golangci-lint-$(GOLANGCI_LINT_VERSION)-$(LINT_ARCH)
$(GOLANGCI_LINT_BIN):
mkdir -p $(LINT_ROOT)/out/linters
Expand Down Expand Up @@ -234,9 +234,19 @@ yamllint-lint: $(YAMLLINT_BIN)
PYTHONPATH=$(YAMLLINT_ROOT)/dist $(YAMLLINT_ROOT)/dist/bin/yamllint .

.PHONY: _lint $(LINTERS)
_lint: $(LINTERS)
_lint:
@exit_code=0; \
for target in $(LINTERS); do \
$(MAKE) $$target || exit_code=1; \
done; \
exit $$exit_code

.PHONY: fix $(FIXERS)
fix: $(FIXERS)
fix:
@exit_code=0; \
for target in $(FIXERS); do \
$(MAKE) $$target || exit_code=1; \
done; \
exit $$exit_code

# END: lint-install .
1 change: 0 additions & 1 deletion cmd/goose/browser_rate_limiter.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Package main implements browser rate limiting for PR auto-open feature.
package main

import (
Expand Down
120 changes: 68 additions & 52 deletions cmd/goose/cache.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Package main - cache.go provides caching functionality for Turn API responses.
package main

import (
Expand All @@ -23,6 +22,70 @@ type cacheEntry struct {
UpdatedAt time.Time `json:"updated_at"`
}

// checkCache checks the cache for a PR and returns the cached data if valid.
// Returns (cachedData, cacheHit, hasRunningTests).
func (app *App) checkCache(cacheFile, url string, updatedAt time.Time) (cachedData *turn.CheckResponse, cacheHit bool, hasRunningTests bool) {
fileData, readErr := os.ReadFile(cacheFile)
if readErr != nil {
if !os.IsNotExist(readErr) {
slog.Debug("[CACHE] Cache file read error", "url", url, "error", readErr)
}
return nil, false, false
}

var entry cacheEntry
if unmarshalErr := json.Unmarshal(fileData, &entry); unmarshalErr != nil {
slog.Warn("Failed to unmarshal cache data", "url", url, "error", unmarshalErr)
// Remove corrupted cache file
if removeErr := os.Remove(cacheFile); removeErr != nil {
slog.Error("Failed to remove corrupted cache file", "error", removeErr)
}
return nil, false, false
}

// Check if cache is expired or PR updated
if time.Since(entry.CachedAt) >= cacheTTL || !entry.UpdatedAt.Equal(updatedAt) {
// Log why cache was invalid
if !entry.UpdatedAt.Equal(updatedAt) {
slog.Debug("[CACHE] Cache miss - PR updated",
"url", url,
"cached_pr_time", entry.UpdatedAt.Format(time.RFC3339),
"current_pr_time", updatedAt.Format(time.RFC3339))
} else {
slog.Debug("[CACHE] Cache miss - TTL expired",
"url", url,
"cached_at", entry.CachedAt.Format(time.RFC3339),
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
"ttl", cacheTTL)
}
return nil, false, false
}

// Check for incomplete tests that should invalidate cache
cacheAge := time.Since(entry.CachedAt)
testState := entry.Data.PullRequest.TestState
isTestIncomplete := testState == "running" || testState == "queued" || testState == "pending"
if entry.Data != nil && isTestIncomplete && cacheAge < runningTestsCacheBypass {
slog.Debug("[CACHE] Cache invalidated - tests incomplete and cache entry is fresh",
"url", url,
"test_state", testState,
"cache_age", cacheAge.Round(time.Minute),
"cached_at", entry.CachedAt.Format(time.RFC3339))
return nil, false, true
}

// Cache hit
slog.Debug("[CACHE] Cache hit",
"url", url,
"cached_at", entry.CachedAt.Format(time.RFC3339),
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339))
if app.healthMonitor != nil {
app.healthMonitor.recordCacheAccess(true)
}
return entry.Data, true, false
}

// turnData fetches Turn API data with caching.
func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (*turn.CheckResponse, bool, error) {
hasRunningTests := false
Expand All @@ -45,57 +108,10 @@ func (app *App) turnData(ctx context.Context, url string, updatedAt time.Time) (

// Skip cache if --no-cache flag is set
if !app.noCache {
// Try to read from cache (gracefully handle all cache errors)
if data, readErr := os.ReadFile(cacheFile); readErr == nil {
var entry cacheEntry
if unmarshalErr := json.Unmarshal(data, &entry); unmarshalErr != nil {
slog.Warn("Failed to unmarshal cache data", "url", url, "error", unmarshalErr)
// Remove corrupted cache file
if removeErr := os.Remove(cacheFile); removeErr != nil {
slog.Error("Failed to remove corrupted cache file", "error", removeErr)
}
} else if time.Since(entry.CachedAt) < cacheTTL && entry.UpdatedAt.Equal(updatedAt) {
// Check if cache is still valid (10 day TTL, but PR UpdatedAt is primary check)
// But invalidate cache for PRs with incomplete tests if cache entry is fresh (< 90 minutes old)
cacheAge := time.Since(entry.CachedAt)
testState := entry.Data.PullRequest.TestState
isTestIncomplete := testState == "running" || testState == "queued" || testState == "pending"
if entry.Data != nil && isTestIncomplete && cacheAge < runningTestsCacheBypass {
hasRunningTests = true
slog.Debug("[CACHE] Cache invalidated - tests incomplete and cache entry is fresh",
"url", url,
"test_state", testState,
"cache_age", cacheAge.Round(time.Minute),
"cached_at", entry.CachedAt.Format(time.RFC3339))
// Don't return cached data - fall through to fetch fresh data with current time
} else {
slog.Debug("[CACHE] Cache hit",
"url", url,
"cached_at", entry.CachedAt.Format(time.RFC3339),
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
"pr_updated_at", entry.UpdatedAt.Format(time.RFC3339))
if app.healthMonitor != nil {
app.healthMonitor.recordCacheAccess(true)
}
return entry.Data, true, nil
}
} else {
// Log why cache was invalid
if !entry.UpdatedAt.Equal(updatedAt) {
slog.Debug("[CACHE] Cache miss - PR updated",
"url", url,
"cached_pr_time", entry.UpdatedAt.Format(time.RFC3339),
"current_pr_time", updatedAt.Format(time.RFC3339))
} else if time.Since(entry.CachedAt) >= cacheTTL {
slog.Debug("[CACHE] Cache miss - TTL expired",
"url", url,
"cached_at", entry.CachedAt.Format(time.RFC3339),
"cache_age", time.Since(entry.CachedAt).Round(time.Second),
"ttl", cacheTTL)
}
}
} else if !os.IsNotExist(readErr) {
slog.Debug("[CACHE] Cache file read error", "url", url, "error", readErr)
if cachedData, cacheHit, runningTests := app.checkCache(cacheFile, url, updatedAt); cacheHit {
return cachedData, true, nil
} else if runningTests {
hasRunningTests = true
}
}

Expand Down
15 changes: 10 additions & 5 deletions cmd/goose/github.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Package main - github.go contains GitHub API integration functions.
package main

import (
Expand Down Expand Up @@ -62,7 +61,7 @@ func (app *App) initClients(ctx context.Context) error {
// initSprinklerOrgs fetches the user's organizations and starts sprinkler monitoring.
func (app *App) initSprinklerOrgs(ctx context.Context) error {
if app.client == nil || app.sprinklerMonitor == nil {
return fmt.Errorf("client or sprinkler not initialized")
return errors.New("client or sprinkler not initialized")
}

// Get current user
Expand All @@ -74,7 +73,7 @@ func (app *App) initSprinklerOrgs(ctx context.Context) error {
user = app.targetUser
}
if user == "" {
return fmt.Errorf("no user configured")
return errors.New("no user configured")
}

slog.Info("[SPRINKLER] Fetching user's organizations", "user", user)
Expand Down Expand Up @@ -136,7 +135,7 @@ func (app *App) initSprinklerOrgs(ctx context.Context) error {
// Update sprinkler with all orgs at once
if len(allOrgs) > 0 {
app.sprinklerMonitor.updateOrgs(allOrgs)
if err := app.sprinklerMonitor.start(); err != nil {
if err := app.sprinklerMonitor.start(ctx); err != nil {
return fmt.Errorf("start sprinkler: %w", err)
}
}
Expand Down Expand Up @@ -287,7 +286,13 @@ func (app *App) executeGitHubQuery(ctx context.Context, query string, opts *gith
return result, err
}

func (app *App) executeGitHubQueryInternal(ctx context.Context, query string, opts *github.SearchOptions, result **github.IssuesSearchResult, resp **github.Response) error {
func (app *App) executeGitHubQueryInternal(
ctx context.Context,
query string,
opts *github.SearchOptions,
result **github.IssuesSearchResult,
resp **github.Response,
) error {
return retry.Do(func() error {
// Create timeout context for GitHub API call
githubCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
Expand Down
10 changes: 3 additions & 7 deletions cmd/goose/icons.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,17 @@ const (
// getIcon returns the icon bytes for the given type.
func getIcon(iconType IconType) []byte {
switch iconType {
case IconGoose:
case IconGoose, IconBoth:
// For both, we'll use the goose icon as primary
return iconGoose
case IconPopper:
return iconPopper
case IconCockroach:
return iconCockroach
case IconSmiling:
return iconSmiling
case IconWarning:
return iconWarning
case IconLock:
return iconLock
case IconBoth:
// For both, we'll use the goose icon as primary
return iconGoose
default:
return iconSmiling
}
Expand All @@ -47,7 +43,7 @@ func getIcon(iconType IconType) []byte {
// setTrayIcon updates the system tray icon based on PR counts.
func (app *App) setTrayIcon(iconType IconType) {
iconBytes := getIcon(iconType)
if iconBytes == nil || len(iconBytes) == 0 {
if len(iconBytes) == 0 {
slog.Warn("Icon bytes are empty, skipping icon update", "type", iconType)
return
}
Expand Down
1 change: 0 additions & 1 deletion cmd/goose/loginitem_darwin.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//go:build darwin

// Package main - loginitem_darwin.go provides macOS-specific login item management.
package main

import (
Expand Down
Loading
Loading