fix: add timeouts to agent discovery, health checks, and GitHub CLI (#440)

This commit is contained in:
Carlos Florêncio
2026-03-14 23:09:30 +00:00
committed by GitHub
parent 74bba464b3
commit 6d71e27bee
14 changed files with 257 additions and 55 deletions
@@ -65,12 +65,15 @@ func (a *ClaudeACP) Logo(v LogoVariant) []byte {
}
func (a *ClaudeACP) IsInstalled(ctx context.Context) (*DiscoveryResult, error) {
// ACP agents use npx which auto-installs the package, so we just need npx available
result, err := Detect(ctx, WithCommand("npx"))
// Use the same detection as claude-code: check for ~/.claude.json which is
// created after onboarding. Just having npx is not enough — the user needs
// Claude Code actually set up.
result, err := Detect(ctx, WithFileExists("~/.claude.json"))
if err != nil {
return result, err
}
result.SupportsMCP = true
result.InstallationPaths = []string{expandHomePath("~/.claude.json")}
result.Capabilities = DiscoveryCapabilities{
SupportsSessionResume: true,
}
@@ -177,8 +177,14 @@ func (r *Registry) getCached() []Availability {
return copied
}
const detectAllTimeout = 15 * time.Second
// detectAll runs IsInstalled for all agents concurrently.
// A timeout bounds the overall detection to prevent hanging when agent
// binaries are missing or unresponsive (e.g. fresh K8s deploy).
func (r *Registry) detectAll(ctx context.Context) []Availability {
ctx, cancel := context.WithTimeout(ctx, detectAllTimeout)
defer cancel()
type indexedResult struct {
index int
avail Availability
@@ -219,20 +225,27 @@ func (r *Registry) detectAll(ctx context.Context) []Availability {
}
// Collect results preserving original order.
// If the context expires before all agents respond, return partial results.
slots := make([]Availability, len(r.agents))
valid := make([]bool, len(r.agents))
for range r.agents {
res := <-ch
if res.err != nil {
r.logger.Warn("discovery: detect failed for agent",
zap.String("agent", r.agents[res.index].ID()),
zap.Error(res.err),
)
continue
select {
case res := <-ch:
if res.err != nil {
r.logger.Warn("discovery: detect failed for agent",
zap.String("agent", r.agents[res.index].ID()),
zap.Error(res.err),
)
continue
}
slots[res.index] = res.avail
valid[res.index] = true
case <-ctx.Done():
r.logger.Warn("discovery: detectAll timed out, returning partial results")
goto collect
}
slots[res.index] = res.avail
valid[res.index] = true
}
collect:
results := make([]Availability, 0, len(r.agents))
for i, v := range valid {
@@ -297,6 +297,13 @@ func (c *Controller) httpListRepoBranches(ctx *gin.Context) {
repo := ctx.Param("repo")
branches, err := c.service.ListRepoBranches(ctx.Request.Context(), owner, repo)
if err != nil {
if errors.Is(err, ErrNoClient) {
ctx.JSON(http.StatusServiceUnavailable, gin.H{
"error": "GitHub is not configured. Install the gh CLI and run 'gh auth login', or add a GITHUB_TOKEN secret.",
"code": "github_not_configured",
})
return
}
status := http.StatusInternalServerError
var apiErr *GitHubAPIError
if errors.As(err, &apiErr) {
+12 -2
View File
@@ -43,19 +43,29 @@ func NewClient(ctx context.Context, secrets SecretProvider, log *logger.Logger)
log.Debug("gh CLI available but not authenticated", zap.Error(err))
}
// Fall back to PAT from environment variable
if token := os.Getenv("GITHUB_TOKEN"); token != "" {
log.Info("using GITHUB_TOKEN from environment for GitHub integration")
return NewPATClient(token), AuthMethodPAT, nil
}
if token := os.Getenv("GH_TOKEN"); token != "" {
log.Info("using GH_TOKEN from environment for GitHub integration")
return NewPATClient(token), AuthMethodPAT, nil
}
// Fall back to PAT from secrets store
if secrets != nil {
token, err := findGitHubPAT(ctx, secrets)
if err == nil && token != "" {
log.Info("using PAT from secrets store for GitHub integration")
return NewPATClient(token), "pat", nil
return NewPATClient(token), AuthMethodPAT, nil
}
if err != nil {
log.Debug("failed to find GitHub PAT in secrets", zap.Error(err))
}
}
return &NoopClient{}, "none", nil
return &NoopClient{}, AuthMethodNone, nil
}
// findGitHubPAT looks for a secret named "GITHUB_TOKEN" or "github_token".
+21 -4
View File
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"net/url"
"os/exec"
@@ -34,16 +35,19 @@ func (c *GHClient) IsAuthenticated(ctx context.Context) (bool, error) {
if err == nil {
return true, nil
}
// Propagate context errors so callers can distinguish cancellation/timeout
// from a genuine "not authenticated" result.
if ctx.Err() != nil {
return false, ctx.Err()
// Propagate timeout/cancellation errors so callers can distinguish them
// from a genuine "not authenticated" result. Check the returned error
// (not ctx.Err()) because run() may apply its own child deadline.
if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
return false, err
}
return false, nil
}
// RunAuthDiagnostics executes gh auth status and captures the raw output for troubleshooting.
func (c *GHClient) RunAuthDiagnostics(ctx context.Context) *AuthDiagnostics {
ctx, cancel := withDefaultGHTimeout(ctx)
defer cancel()
cmd := exec.CommandContext(ctx, "gh", "auth", "status", "--hostname", "github.com")
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
@@ -411,9 +415,22 @@ func (c *GHClient) ListRepoBranches(ctx context.Context, owner, repo string) ([]
return branches, nil
}
const ghCLITimeout = 30 * time.Second
// withDefaultGHTimeout applies a 30s timeout if the context has no deadline.
func withDefaultGHTimeout(ctx context.Context) (context.Context, context.CancelFunc) {
if _, ok := ctx.Deadline(); ok {
return ctx, func() {}
}
return context.WithTimeout(ctx, ghCLITimeout)
}
// run executes a gh CLI command and returns its stdout output.
// Stderr is captured separately to avoid contaminating JSON output.
// A default 30s timeout is applied if the context has no deadline.
func (c *GHClient) run(ctx context.Context, args ...string) (string, error) {
ctx, cancel := withDefaultGHTimeout(ctx)
defer cancel()
cmd := exec.CommandContext(ctx, "gh", args...)
var stdout, stderr bytes.Buffer
cmd.Stdout = &stdout
+17 -6
View File
@@ -15,6 +15,12 @@ import (
"github.com/kandev/kandev/internal/events/bus"
)
// Auth method constants.
const (
AuthMethodNone = "none"
AuthMethodPAT = "pat"
)
// Service coordinates GitHub integration operations.
type Service struct {
mu sync.Mutex
@@ -49,8 +55,11 @@ func (s *Service) TestStore() *Store {
}
// IsAuthenticated returns whether the service has a working GitHub client.
// Returns false when using the NoopClient fallback (authMethod == "none").
func (s *Service) IsAuthenticated() bool {
return s.client != nil
s.mu.Lock()
defer s.mu.Unlock()
return s.client != nil && s.authMethod != AuthMethodNone
}
// AuthMethod returns the authentication method ("gh_cli", "pat", or "none").
@@ -59,9 +68,10 @@ func (s *Service) AuthMethod() string {
}
// GetStatus returns the current GitHub connection status.
// If no client exists, it retries client creation to pick up auth changes made after startup.
// If not authenticated, it retries client creation to pick up auth changes
// (e.g. GITHUB_TOKEN secret added after startup).
func (s *Service) GetStatus(ctx context.Context) (*GitHubStatus, error) {
if s.client == nil {
if !s.IsAuthenticated() {
s.retryClientCreation(ctx)
}
@@ -93,12 +103,13 @@ func (s *Service) GetStatus(ctx context.Context) (*GitHubStatus, error) {
return status, nil
}
// retryClientCreation attempts to create a GitHub client when none exists.
// retryClientCreation attempts to create a GitHub client when not authenticated.
// This picks up auth changes made after startup (secrets added, env vars set).
func (s *Service) retryClientCreation(ctx context.Context) {
s.mu.Lock()
defer s.mu.Unlock()
if s.client != nil {
return // another goroutine already recovered
if s.authMethod != AuthMethodNone {
return // already authenticated
}
client, authMethod, err := NewClient(ctx, s.secrets, s.logger)
if err != nil {
+23 -3
View File
@@ -1,6 +1,10 @@
package health
import "context"
import (
"context"
"errors"
"time"
)
// GitHubStatusProvider abstracts the GitHub service status check.
type GitHubStatusProvider interface {
@@ -60,13 +64,29 @@ func NewAgentChecker(provider AgentDiscoveryProvider) *AgentChecker {
return &AgentChecker{provider: provider}
}
const agentCheckTimeout = 10 * time.Second
func (c *AgentChecker) Check(ctx context.Context) []Issue {
if c.provider == nil {
return nil
}
available, err := c.provider.HasAvailableAgents(ctx)
checkCtx, cancel := context.WithTimeout(ctx, agentCheckTimeout)
defer cancel()
available, err := c.provider.HasAvailableAgents(checkCtx)
if err != nil {
return nil
title := "Agent detection failed"
if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
title = "Agent detection timed out"
}
return []Issue{{
ID: "agent_detection_failed",
Category: "agents",
Title: title,
Message: "Could not verify agent installations. Check Settings > Agents for details.",
Severity: SeverityWarning,
FixURL: "/settings/agents",
FixLabel: "Check Agents",
}}
}
if !available {
return []Issue{{
+8 -2
View File
@@ -94,8 +94,14 @@ func TestAgentChecker_NotAvailable(t *testing.T) {
func TestAgentChecker_Error(t *testing.T) {
checker := NewAgentChecker(&mockAgentProvider{err: fmt.Errorf("discovery failed")})
issues := checker.Check(context.Background())
if len(issues) != 0 {
t.Errorf("expected 0 issues on error, got %d", len(issues))
if len(issues) != 1 {
t.Fatalf("expected 1 issue on error, got %d", len(issues))
}
if issues[0].ID != "agent_detection_failed" {
t.Errorf("issue ID = %q, want %q", issues[0].ID, "agent_detection_failed")
}
if issues[0].Severity != SeverityWarning {
t.Errorf("severity = %q, want %q", issues[0].Severity, SeverityWarning)
}
}
+26 -7
View File
@@ -45,19 +45,38 @@ export function GitHubStatusCard() {
if (!status || !status.authenticated) {
return (
<div>
<div className="space-y-3">
<div className="flex items-center gap-2">
<IconX className="h-4 w-4 text-red-500" />
<span className="text-sm">Not connected to GitHub</span>
<span className="text-xs text-muted-foreground">
Run <code className="bg-muted px-1 rounded">gh auth login</code> or add a GITHUB_TOKEN
secret
</span>
<span className="text-sm font-medium">Not connected to GitHub</span>
<Button variant="ghost" size="sm" onClick={refresh} className="cursor-pointer h-6 px-2">
<IconRefresh className="h-3.5 w-3.5" />
<span className="sr-only">Refresh GitHub status</span>
Refresh
</Button>
</div>
<div className="text-xs text-muted-foreground space-y-1.5">
<p>Connect using one of these methods:</p>
<ol className="list-decimal list-inside space-y-1 pl-1">
<li>
<strong>Environment variable</strong> — set{" "}
<code className="bg-muted px-1 rounded">GITHUB_TOKEN</code> or{" "}
<code className="bg-muted px-1 rounded">GH_TOKEN</code> when starting Kandev
</li>
<li>
<strong>Kandev secret</strong> — add a secret named{" "}
<code className="bg-muted px-1 rounded">GITHUB_TOKEN</code> in{" "}
<a href="/settings/general/secrets" className="underline cursor-pointer">
Settings &gt; Secrets
</a>
, then click Refresh above
</li>
<li>
<strong>GitHub CLI</strong> — install{" "}
<code className="bg-muted px-1 rounded">gh</code> and run{" "}
<code className="bg-muted px-1 rounded">gh auth login</code>
</li>
</ol>
</div>
{status?.diagnostics && <DiagnosticsOutput diagnostics={status.diagnostics} />}
</div>
);
@@ -296,9 +296,13 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) {
setGitHubBranchesLoading(true);
setGitHubPrHeadBranch(null);
const branchesPromise = fetchRepoBranches(parsed.owner, parsed.repo);
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 15_000);
const fetchOpts = { init: { signal: controller.signal } };
const branchesPromise = fetchRepoBranches(parsed.owner, parsed.repo, fetchOpts);
const prPromise = parsed.prNumber
? fetchPRInfo(parsed.owner, parsed.repo, parsed.prNumber).catch(() => null)
? fetchPRInfo(parsed.owner, parsed.repo, parsed.prNumber, fetchOpts).catch(() => null)
: Promise.resolve(null);
Promise.all([branchesPromise, prPromise])
@@ -312,16 +316,27 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) {
setGitHubPrHeadBranch(prInfo.head_branch);
}
})
.catch(() => {
.catch((err) => {
if (cancelled) return;
setGitHubUrlError("Repository not found or not accessible");
const isAbort = err instanceof DOMException && err.name === "AbortError";
const isNotConfigured = err instanceof Error && err.message.includes("not configured");
let errorMessage = "Repository not found or not accessible";
if (isAbort) {
errorMessage = "Request timed out. Check your GitHub configuration in Settings.";
} else if (isNotConfigured) {
errorMessage = "GitHub is not configured. Set up a token in Settings > GitHub.";
}
setGitHubUrlError(errorMessage);
setGitHubBranches([]);
})
.finally(() => {
clearTimeout(timeoutId);
if (!cancelled) setGitHubBranchesLoading(false);
});
return () => {
cancelled = true;
clearTimeout(timeoutId);
controller.abort();
};
}, [
open,
@@ -15,9 +15,9 @@ export function QuickChatButton({ workspaceId }: { workspaceId?: string | null }
return (
<KeyboardShortcutTooltip shortcut={SHORTCUTS.QUICK_CHAT} description="Quick Chat">
<Button
size="sm"
size="icon"
variant="outline"
className="cursor-pointer px-2"
className="cursor-pointer"
onClick={handleOpenQuickChat}
>
<IconMessageCircle className="h-4 w-4" />
+1 -1
View File
@@ -339,12 +339,12 @@ function TopBarRight({
isAgentctlReady={isAgentctlReady}
/>
<PRTopbarButton />
<QuickChatButton workspaceId={workspaceId} />
<VcsSplitButton sessionId={activeSessionId ?? null} baseBranch={baseBranch} />
<LayoutPresetSelector />
<EditorsMenu activeSessionId={activeSessionId ?? null} />
</>
)}
<QuickChatButton workspaceId={workspaceId} />
<Tooltip>
<TooltipTrigger asChild>
<Button size="sm" variant="outline" className="cursor-pointer px-2" asChild>
+66
View File
@@ -0,0 +1,66 @@
import { test, expect } from "../fixtures/test-base";
import { KanbanPage } from "../pages/kanban-page";
test.describe("First-time setup: timeouts and error handling", () => {
// Allow one retry for transient cold-start timing issues on first test.
test.describe.configure({ retries: 1 });
test("GitHub branch fetch network failure shows error", async ({ testPage, backend }) => {
// Intercept branch fetch endpoint and abort (simulates network failure / timeout)
await testPage.route(
`${backend.baseUrl}/api/v1/github/repos/slow-owner/slow-repo/branches`,
(route) => route.abort("failed"),
);
const kanban = new KanbanPage(testPage);
await kanban.goto();
await kanban.createTaskButton.first().click();
await expect(testPage.getByTestId("create-task-dialog")).toBeVisible();
// Toggle to GitHub URL mode
await testPage.getByTestId("toggle-github-url").click();
await testPage.getByTestId("github-url-input").fill("https://github.com/slow-owner/slow-repo");
// The error should appear after the fetch fails
const errorEl = testPage.getByTestId("github-url-error");
await expect(errorEl).toBeVisible({ timeout: 10_000 });
await expect(errorEl).toContainText("not found or not accessible");
});
test("health indicator shows issues and opens dialog", async ({ testPage, backend }) => {
// Intercept the health endpoint and return a detection timeout issue
await testPage.route(`${backend.baseUrl}/api/v1/system/health`, (route) =>
route.fulfill({
status: 200,
contentType: "application/json",
body: JSON.stringify({
healthy: false,
issues: [
{
id: "agent_detection_failed",
category: "agents",
title: "Agent detection timed out",
message: "Could not verify agent installations. Check Settings > Agents for details.",
severity: "warning",
fix_url: "/settings/agents",
fix_label: "Check Agents",
},
],
}),
}),
);
await testPage.goto("/");
// Health indicator should appear with the warning
const healthBtn = testPage.getByRole("button", { name: "Setup Issues" });
await expect(healthBtn).toBeVisible({ timeout: 15_000 });
// Click to open the dialog and verify the issue content
await healthBtn.click();
const dialog = testPage.getByRole("dialog");
await expect(dialog).toBeVisible({ timeout: 5_000 });
await expect(dialog.getByText("Agent detection timed out")).toBeVisible();
await expect(dialog.getByText("Check Agents")).toBeVisible();
});
});
@@ -1,28 +1,43 @@
import { useEffect } from "react";
import { useEffect, useRef } from "react";
import { useAppStore } from "@/components/state-provider";
import { listAgentDiscovery } from "@/lib/api";
const DISCOVERY_TIMEOUT_MS = 20_000;
export function useAgentDiscovery(enabled = true) {
const agentDiscovery = useAppStore((state) => state.agentDiscovery);
const setAgentDiscovery = useAppStore((state) => state.setAgentDiscovery);
const setAgentDiscoveryLoading = useAppStore((state) => state.setAgentDiscoveryLoading);
const fetchingRef = useRef(false);
useEffect(() => {
if (!enabled) return;
if (agentDiscovery.loaded || agentDiscovery.loading) return;
if (!enabled || agentDiscovery.loaded || fetchingRef.current) return;
fetchingRef.current = true;
setAgentDiscoveryLoading(true);
listAgentDiscovery({ cache: "no-store" })
let active = true;
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), DISCOVERY_TIMEOUT_MS);
listAgentDiscovery({ cache: "no-store", init: { signal: controller.signal } })
.then((response) => {
setAgentDiscovery(response.agents);
if (active) setAgentDiscovery(response.agents);
})
.catch(() => setAgentDiscovery([]));
}, [
agentDiscovery.loaded,
agentDiscovery.loading,
enabled,
setAgentDiscovery,
setAgentDiscoveryLoading,
]);
.catch(() => {
if (active) setAgentDiscovery([]);
})
.finally(() => {
fetchingRef.current = false;
clearTimeout(timeoutId);
});
return () => {
active = false;
fetchingRef.current = false;
clearTimeout(timeoutId);
controller.abort();
};
}, [enabled, agentDiscovery.loaded, setAgentDiscovery, setAgentDiscoveryLoading]);
return agentDiscovery;
}