fix: gateway auth injection and cumulative diff error handling (#682)

This commit is contained in:
Carlos Florêncio
2026-04-23 18:31:00 +01:00
committed by GitHub
parent 7076a40805
commit de040a8987
13 changed files with 315 additions and 54 deletions
+98
View File
@@ -0,0 +1,98 @@
---
name: debug-logs
description: Add temporary debug logs to investigate issues. Use when the user needs to trace runtime behavior in the frontend or backend. Debug logs must be stripped before creating a PR.
---
# Debug Logs
Add temporary debug logs to investigate runtime issues. These logs are **never merged** — they must be removed before creating a PR.
## Rules
1. **All debug logs are temporary.** Strip them before running `/commit` or `/pr`.
2. Use a consistent, searchable prefix so logs can be found and removed easily.
3. Log object fields **inline** — do not log raw objects (they render as `Object` in the browser console and require expanding).
## Frontend (TypeScript)
- **Level:** `console.log` — not `console.debug` (hidden by default) or `console.warn` (noisy).
- **Prefix:** `[WS-DEBUG]` (or another `[AREA-DEBUG]` prefix agreed with the user).
- **Format:** Log fields inline as key-value pairs so they are visible without expanding.
### ✅ Correct
```typescript
console.log("[WS-DEBUG] subscribeSession", {
sessionId,
refCount: current + 1,
sentMessage: shouldSend,
});
```
Console output:
```
[WS-DEBUG] subscribeSession {sessionId: 'abc', refCount: 2, sentMessage: true}
```
### ❌ Wrong — raw object renders as `Object`
```typescript
console.log("[WS-DEBUG] subscribeSession", session);
// Output: [WS-DEBUG] subscribeSession Object ← useless without expanding
```
### ❌ Wrong — wrong log level
```typescript
console.warn("[WS-DEBUG] subscribeSession", { sessionId }); // ← use console.log
console.debug("[WS-DEBUG] subscribeSession", { sessionId }); // ← hidden by default
```
## Backend (Go)
- **Level:** `WARN` — stands out from normal `DEBUG`/`INFO` output without being an error.
- **Prefix:** `[DEBUG]` (or another `[AREA-DEBUG]` prefix agreed with the user).
- **Method:** Use the structured logger: `s.logger.Warn("[DEBUG] description", "key", value, ...)`.
### ✅ Correct
```go
s.logger.Warn("[DEBUG] handleTaskMoved entering",
"task_id", taskID,
"session_id", sessionID,
"from_step", fromStepID,
"to_step", toStepID,
)
```
### ❌ Wrong — wrong level
```go
s.logger.Debug("[DEBUG] handleTaskMoved", "task_id", taskID) // ← lost in noise
s.logger.Error("[DEBUG] handleTaskMoved", "task_id", taskID) // ← triggers alerts
```
## Workflow
1. **Add debug logs** to the relevant code paths. Do not commit them — keep them as unstaged changes.
2. **Let the user test** the app and report back with console/log output.
3. **Iterate** — add, move, or refine logs as needed based on findings. Still no commits.
4. **Fix the issue** once the root cause is identified.
5. **Strip all debug logs** before committing the fix. Only commit the actual fix.
## Stripping Debug Logs
When the issue is fixed and the user asks to commit, remove all debug logs first:
```bash
# Find frontend debug logs
grep -rn 'console.log("\[WS-DEBUG\]' apps/web/
# Find backend debug logs
grep -rn '\[DEBUG\]' apps/backend/
# Or use the prefix agreed with the user
grep -rn '\[AREA-DEBUG\]' apps/
```
Verify no debug logs remain in staged files before proceeding with `/commit` or `/pr`.
@@ -727,15 +727,25 @@ func (h *GitHandlers) wsCumulativeDiff(ctx context.Context, msg *ws.Message) (*w
return nil, fmt.Errorf("session_id is required")
}
agentClient, err := h.getAgentCtlClient(req.SessionID)
// Use GetOrEnsureExecution to recover workspace after backend restarts.
// This is a workspace-oriented operation that doesn't require a running agent process.
execution, err := h.lifecycleMgr.GetOrEnsureExecution(ctx, req.SessionID)
if err != nil {
if isSessionNotReadyError(err) {
if errors.Is(err, lifecycle.ErrSessionWorkspaceNotReady) || isSessionNotReadyError(err) {
return ws.NewResponse(msg.ID, msg.Action, map[string]any{
"cumulative_diff": nil,
"ready": false,
})
}
return nil, err
return nil, fmt.Errorf("failed to get execution for session %s: %w", req.SessionID, err)
}
agentClient := execution.GetAgentCtlClient()
if agentClient == nil {
return ws.NewResponse(msg.ID, msg.Action, map[string]any{
"cumulative_diff": nil,
"ready": false,
})
}
// Look up base commit SHA from the session metadata
@@ -395,7 +395,10 @@ func TestWsGitCommits_NotReady(t *testing.T) {
func TestWsCumulativeDiff_NotReady(t *testing.T) {
log := newTestLogger()
lookup := &mockExecutionLookup{} // no executions → "no agent running" error
// Use "no agent running for session" error which matches isSessionNotReadyError
lookup := &mockExecutionLookup{
ensureErr: fmt.Errorf("no agent running for session session-1"),
}
h := NewGitHandlers(lookup, nil, log)
msg, _ := ws.NewRequest("test-1", ws.ActionSessionCumulativeDiff, CumulativeDiffRequest{SessionID: "session-1"})
@@ -420,6 +423,82 @@ func TestWsCumulativeDiff_NotReady(t *testing.T) {
}
}
func TestWsCumulativeDiff_WorkspaceNotReady(t *testing.T) {
log := newTestLogger()
lookup := &mockExecutionLookup{
ensureErr: lifecycle.ErrSessionWorkspaceNotReady,
}
h := NewGitHandlers(lookup, nil, log)
msg, _ := ws.NewRequest("test-1", ws.ActionSessionCumulativeDiff, CumulativeDiffRequest{SessionID: "session-1"})
resp, err := h.wsCumulativeDiff(context.Background(), msg)
if err != nil {
t.Fatalf("expected graceful response for workspace not ready, got error: %v", err)
}
if resp == nil {
t.Fatal("expected non-nil response")
}
var payload map[string]any
if err := json.Unmarshal(resp.Payload, &payload); err != nil {
t.Fatalf("failed to parse payload: %v", err)
}
if ready, ok := payload["ready"].(bool); !ok || ready {
t.Errorf("expected ready=false, got %v", payload["ready"])
}
if payload["cumulative_diff"] != nil {
t.Errorf("expected cumulative_diff=nil, got %v", payload["cumulative_diff"])
}
}
func TestWsCumulativeDiff_NilAgentClient(t *testing.T) {
log := newTestLogger()
// Execution exists but has no agentctl client yet
lookup := &mockExecutionLookup{
executions: map[string]*lifecycle.AgentExecution{
"session-1": {ID: "exec-1", SessionID: "session-1"},
},
}
h := NewGitHandlers(lookup, nil, log)
msg, _ := ws.NewRequest("test-1", ws.ActionSessionCumulativeDiff, CumulativeDiffRequest{SessionID: "session-1"})
resp, err := h.wsCumulativeDiff(context.Background(), msg)
if err != nil {
t.Fatalf("expected graceful response for nil agent client, got error: %v", err)
}
if resp == nil {
t.Fatal("expected non-nil response")
}
var payload map[string]any
if err := json.Unmarshal(resp.Payload, &payload); err != nil {
t.Fatalf("failed to parse payload: %v", err)
}
if ready, ok := payload["ready"].(bool); !ok || ready {
t.Errorf("expected ready=false, got %v", payload["ready"])
}
if payload["cumulative_diff"] != nil {
t.Errorf("expected cumulative_diff=nil, got %v", payload["cumulative_diff"])
}
}
func TestWsCumulativeDiff_UnexpectedError(t *testing.T) {
log := newTestLogger()
lookup := &mockExecutionLookup{
ensureErr: errors.New("database connection failed"),
}
h := NewGitHandlers(lookup, nil, log)
msg, _ := ws.NewRequest("test-1", ws.ActionSessionCumulativeDiff, CumulativeDiffRequest{SessionID: "session-1"})
_, err := h.wsCumulativeDiff(context.Background(), msg)
if err == nil {
t.Fatal("expected error for unexpected failure")
}
}
func TestIsSessionNotReadyError(t *testing.T) {
tests := []struct {
name string
@@ -586,6 +586,12 @@ func (c *Client) BaseURL() string {
return c.baseURL
}
// AuthToken returns the Bearer token used for authenticating requests to agentctl.
// Returns empty string if no token is configured.
func (c *Client) AuthToken() string {
return c.authToken
}
// Host returns the host portion (without port) of the agentctl client URL.
func (c *Client) Host() string {
parsed, err := url.Parse(c.baseURL)
@@ -122,7 +122,8 @@ func (h *PortProxyHandler) resolveProxy(c *gin.Context, sessionID string, port i
return nil, err
}
proxy := h.createProxy(cacheKey, target)
authToken := agentctlClient.AuthToken()
proxy := h.createProxy(cacheKey, target, authToken)
h.proxies[cacheKey] = &portProxyEntry{proxy: proxy, target: baseURL}
h.logger.Info("created port proxy",
@@ -133,12 +134,16 @@ func (h *PortProxyHandler) resolveProxy(c *gin.Context, sessionID string, port i
return proxy, nil
}
func (h *PortProxyHandler) createProxy(cacheKey string, target *url.URL) *httputil.ReverseProxy {
func (h *PortProxyHandler) createProxy(cacheKey string, target *url.URL, authToken string) *httputil.ReverseProxy {
proxy := &httputil.ReverseProxy{}
proxy.Rewrite = func(r *httputil.ProxyRequest) {
r.SetURL(target)
r.Out.URL.Path = r.In.URL.Path
r.Out.URL.RawPath = ""
// Inject agentctl auth token
if authToken != "" {
r.Out.Header.Set("Authorization", "Bearer "+authToken)
}
if r.Out.Header.Get("Upgrade") != "" {
r.Out.Header.Set("Connection", "Upgrade")
}
@@ -73,13 +73,13 @@ func (m *TunnelManager) StartTunnel(sessionID string, port int, tunnelPort int)
m.tunnels[cacheKey] = nil
m.mu.Unlock()
target, ln, err := m.resolveAndBind(cacheKey, sessionID, tunnelPort)
target, authToken, ln, err := m.resolveAndBind(cacheKey, sessionID, tunnelPort)
if err != nil {
return 0, err
}
actualPort := ln.Addr().(*net.TCPAddr).Port
proxy := m.createTunnelProxy(cacheKey, target, port)
proxy := m.createTunnelProxy(cacheKey, target, port, authToken)
ctx, cancel := context.WithCancel(context.Background())
srv := &http.Server{Handler: proxy}
@@ -108,7 +108,7 @@ func (m *TunnelManager) StartTunnel(sessionID string, port int, tunnelPort int)
// resolveAndBind resolves the agentctl target URL for the session and binds a
// local TCP listener for the tunnel. On failure it cleans up the placeholder
// entry reserved by StartTunnel.
func (m *TunnelManager) resolveAndBind(cacheKey, sessionID string, tunnelPort int) (*url.URL, net.Listener, error) {
func (m *TunnelManager) resolveAndBind(cacheKey, sessionID string, tunnelPort int) (*url.URL, string, net.Listener, error) {
cleanup := func() {
m.mu.Lock()
if m.tunnels[cacheKey] == nil {
@@ -120,31 +120,32 @@ func (m *TunnelManager) resolveAndBind(cacheKey, sessionID string, tunnelPort in
execution, ok := m.lifecycleMgr.GetExecutionBySessionID(sessionID)
if !ok {
cleanup()
return nil, nil, fmt.Errorf("session not found or no active execution")
return nil, "", nil, fmt.Errorf("session not found or no active execution")
}
agentctlClient := execution.GetAgentCtlClient()
if agentctlClient == nil {
cleanup()
return nil, nil, fmt.Errorf("agentctl client not available")
return nil, "", nil, fmt.Errorf("agentctl client not available")
}
target, err := url.Parse(agentctlClient.BaseURL())
if err != nil {
cleanup()
return nil, nil, fmt.Errorf("failed to parse agentctl URL: %w", err)
return nil, "", nil, fmt.Errorf("failed to parse agentctl URL: %w", err)
}
authToken := agentctlClient.AuthToken()
ln, err := net.Listen("tcp", fmt.Sprintf(":%d", tunnelPort))
if err != nil {
cleanup()
if isAddrInUse(err) {
return nil, nil, fmt.Errorf("port %d is already in use, choose a different port", tunnelPort)
return nil, "", nil, fmt.Errorf("port %d is already in use, choose a different port", tunnelPort)
}
return nil, nil, fmt.Errorf("failed to bind tunnel port %d: %w", tunnelPort, err)
return nil, "", nil, fmt.Errorf("failed to bind tunnel port %d: %w", tunnelPort, err)
}
return target, ln, nil
return target, authToken, ln, nil
}
// serveTunnel starts the tunnel HTTP server and its shutdown goroutine.
@@ -271,7 +272,7 @@ func (m *TunnelManager) removeTunnel(cacheKey string) {
delete(m.tunnels, cacheKey)
}
func (m *TunnelManager) createTunnelProxy(cacheKey string, target *url.URL, port int) *httputil.ReverseProxy {
func (m *TunnelManager) createTunnelProxy(cacheKey string, target *url.URL, port int, authToken string) *httputil.ReverseProxy {
portStr := strconv.Itoa(port)
proxy := &httputil.ReverseProxy{}
@@ -284,6 +285,10 @@ func (m *TunnelManager) createTunnelProxy(cacheKey string, target *url.URL, port
}
r.Out.URL.Path = "/api/v1/port-proxy/" + portStr + incoming
r.Out.URL.RawPath = ""
// Inject agentctl auth token
if authToken != "" {
r.Out.Header.Set("Authorization", "Bearer "+authToken)
}
// Preserve original Host header for CORS/Origin validation.
r.Out.Host = r.In.Host
if r.Out.Header.Get("Upgrade") != "" {
@@ -149,7 +149,8 @@ func (h *VscodeProxyHandler) resolveProxy(c *gin.Context, sessionID string) (*ht
return nil, err
}
proxy := h.createProxy(sessionID, target)
authToken := agentctlClient.AuthToken()
proxy := h.createProxy(sessionID, target, authToken)
h.proxies[sessionID] = &proxyEntry{proxy: proxy, target: baseURL}
h.logger.Info("created vscode proxy for session",
@@ -161,13 +162,17 @@ func (h *VscodeProxyHandler) resolveProxy(c *gin.Context, sessionID string) (*ht
}
// createProxy builds a reverse proxy for the given target URL.
func (h *VscodeProxyHandler) createProxy(sessionID string, target *url.URL) *httputil.ReverseProxy {
func (h *VscodeProxyHandler) createProxy(sessionID string, target *url.URL, authToken string) *httputil.ReverseProxy {
proxy := httputil.NewSingleHostReverseProxy(target)
// Allow WebSocket upgrades by preserving hop-by-hop headers
originalDirector := proxy.Director
proxy.Director = func(req *http.Request) {
originalDirector(req)
// Inject agentctl auth token
if authToken != "" {
req.Header.Set("Authorization", "Bearer "+authToken)
}
// Preserve WebSocket headers that SingleHostReverseProxy strips
if req.Header.Get("Upgrade") != "" {
req.Header.Set("Connection", "Upgrade")
@@ -113,7 +113,8 @@ export function resolvePRPanelTargetGroup(
centerGroupId: string,
): string {
const sessionPanel = api.getPanel(`session:${sessionId}`);
return sessionPanel?.group?.id ?? centerGroupId;
const resolved = sessionPanel?.group?.id ?? centerGroupId;
return resolved;
}
/**
@@ -141,14 +142,14 @@ export function useAutoPRPanel() {
const api = useDockviewStore.getState().api;
if (!api) return;
const decision = shouldAutoAddPRPanel({
const decisionParams = {
hasPR,
panelExists: !!api.getPanel("pr-detail"),
isRestoringLayout: useDockviewStore.getState().isRestoringLayout,
isMaximized: useDockviewStore.getState().preMaximizeLayout !== null,
wasOffered: wasPRPanelOffered(sessionId),
});
};
const decision = shouldAutoAddPRPanel(decisionParams);
if (decision === "remove") {
api.getPanel("pr-detail")?.api.close();
return;
@@ -27,6 +27,26 @@ import { useArchivedTaskState } from "./task-archived-context";
import { useRepositories } from "@/hooks/domains/workspace/use-repositories";
import { useWorkspacePRs } from "@/hooks/domains/github/use-task-pr";
/**
* Stabilize a derived array of primary session IDs so the reference only
* changes when the actual contents change. This prevents the bulk-subscribe
* effect from tearing down and recreating all subscriptions on every kanban
* snapshot update.
*/
function useStablePrimarySessionIds(
allTasks: Array<{ primarySessionId?: string | null }>,
): string[] {
const key = useMemo(
() =>
allTasks
.map((t) => t.primarySessionId)
.filter((id): id is string => id != null)
.join("\0"),
[allTasks],
);
return useMemo(() => (key ? key.split("\0") : []), [key]);
}
/** Find a task across all workflow snapshots */
function findTaskInSnapshots(
snapshots: Record<string, { tasks: KanbanState["tasks"] }>,
@@ -264,10 +284,7 @@ function useSidebarData(workspaceId: string | null) {
// Stable list of primary session IDs for the bulk-subscribe effect.
// Derived from kanban tasks (always available) rather than sessionsByTaskId (loaded on-demand).
const primarySessionIds = useMemo(
() => allTasks.map((t) => t.primarySessionId).filter((id): id is string => id != null),
[allTasks],
);
const primarySessionIds = useStablePrimarySessionIds(allTasks);
return {
activeTaskId,
@@ -512,13 +529,18 @@ function useSidebarActions(store: StoreApi) {
function useBulkGitStatusSubscription(primarySessionIds: string[]) {
const connectionStatus = useAppStore((state) => state.connection.status);
const activeSessionId = useAppStore((state) => state.tasks.activeSessionId);
useEffect(() => {
if (connectionStatus !== "connected" || primarySessionIds.length === 0) return;
const client = getWebSocketClient();
if (!client) return;
const unsubscribes = primarySessionIds.map((id) => client.subscribeSession(id));
// Skip active session — it's already subscribed + focused by the task page hooks
const backgroundIds = activeSessionId
? primarySessionIds.filter((id) => id !== activeSessionId)
: primarySessionIds;
const unsubscribes = backgroundIds.map((id) => client.subscribeSession(id));
return () => unsubscribes.forEach((u) => u());
}, [primarySessionIds, connectionStatus]);
}, [primarySessionIds, connectionStatus, activeSessionId]);
}
function useEffectiveSidebarView() {
@@ -478,5 +478,16 @@ test.describe("PR detail panel", () => {
await session.idleInput().waitFor({ state: "visible", timeout: 30_000 });
await expect(session.prDetailTab()).toBeVisible({ timeout: 15_000 });
await session.expectPrPanelAndSessionShareGroup();
// Switch BACK to Task A — regression: the fast-path layout switch used to
// remove the old session tab then look for a chat/session panel to anchor
// the new tab. With the old tab gone and the PR panel being non-chat, the
// lookup failed and fell through to a sidebar split, placing the session
// tab in a new group while the PR panel stayed in the old center group.
await session.clickTaskInSidebar("First PR Task");
await session.waitForLoad();
await session.idleInput().waitFor({ state: "visible", timeout: 30_000 });
await expect(session.prDetailTab()).toBeVisible({ timeout: 15_000 });
await session.expectPrPanelAndSessionShareGroup();
});
});
@@ -21,7 +21,9 @@ async function fetchAndStoreMessages(
store: ReturnType<typeof useAppStoreApi>,
): Promise<Message[]> {
const client = getWebSocketClient();
if (!client) return [];
if (!client) {
return [];
}
const response = await client.request<MessageListResponse>(
"message.list",
@@ -29,7 +31,6 @@ async function fetchAndStoreMessages(
10000,
);
const fetched = [...(response.messages ?? [])].reverse();
// Merge: keep WS-delivered messages that aren't in the fetch response.
// This prevents a slow fetch (sent before messages existed) from wiping
// messages that arrived via real-time notifications while the fetch was
@@ -145,6 +146,32 @@ export function useVisibilityBackfill(
}, [taskSessionId, store]);
}
function useSessionSubscription(
taskSessionId: string | null,
connectionStatus: string,
isSessionStartingOrUnknown: boolean,
store: ReturnType<typeof useAppStoreApi>,
) {
useEffect(() => {
if (!taskSessionId || connectionStatus !== "connected") {
return;
}
const client = getWebSocketClient();
if (!client) {
return;
}
const unsubscribe = client.subscribeSession(taskSessionId);
// Re-fetch messages after subscribing to close the gap between SSR
// (which may have run before the agent responded) and this subscription.
fetchAndStoreMessages(taskSessionId, store).catch(() => {});
return () => {
unsubscribe();
};
}, [taskSessionId, connectionStatus, store, isSessionStartingOrUnknown]);
}
export function useSessionMessages(taskSessionId: string | null): UseSessionMessagesReturn {
const store = useAppStoreApi();
const messages = useAppStore((state) =>
@@ -232,23 +259,7 @@ export function useSessionMessages(taskSessionId: string | null): UseSessionMess
// churning on every subsequent RUNNING ↔ WAITING_FOR_INPUT transition.
const isSessionStartingOrUnknown = taskSessionState === null || taskSessionState === "STARTING";
useEffect(() => {
if (!taskSessionId || connectionStatus !== "connected") return;
const client = getWebSocketClient();
if (!client) return;
const unsubscribe = client.subscribeSession(taskSessionId);
// Re-fetch messages after subscribing to close the gap between SSR
// (which may have run before the agent responded) and this subscription.
// Without this, fast-responding agents can complete a turn before the
// subscription is active, causing the response to never appear.
fetchAndStoreMessages(taskSessionId, store).catch(() => {});
return () => {
unsubscribe();
};
}, [taskSessionId, connectionStatus, store, isSessionStartingOrUnknown]);
useSessionSubscription(taskSessionId, connectionStatus, isSessionStartingOrUnknown, store);
useVisibilityBackfill(taskSessionId, store);
const terminalFetchRefs = useMemo(
+12 -5
View File
@@ -96,6 +96,16 @@ function tryFastSessionSwitch(params: SessionSwitchParams): LayoutGroupIds | nul
// would discard the current ones without restoring the target session's.
if (saved && savedLayoutHasEphemeralPanels(saved as SerializedDockview)) return null;
// Capture the outgoing session tab's group BEFORE removing it so we can
// place the new session tab in the same group. Without this, the lookup
// after removeEphemeralPanels finds no chat/session panels and falls
// through to a sidebar split — putting the new tab in a new group while
// the PR panel stays in the old center group.
const outgoingSessionPanel = api.panels.find(
(p) => p.id.startsWith("session:") || p.api.component === "chat",
);
const outgoingGroupId = outgoingSessionPanel?.group?.id;
// Fast path: keep the grid structure, clean up ephemeral panels and any
// session chat tabs that belong to a different session (cross-task switch).
// Session-scoped portals (browser, vscode, etc.) will be re-acquired
@@ -106,13 +116,10 @@ function tryFastSessionSwitch(params: SessionSwitchParams): LayoutGroupIds | nul
// old tab and creating the new one. useAutoSessionTab will detect the panel
// already exists and skip creation.
if (!api.getPanel(`session:${newSessionId}`)) {
const chatPanel = api.panels.find(
(p) => p.id.startsWith("session:") || p.api.component === "chat",
);
const sidebarPanel = api.getPanel("sidebar");
let position: import("dockview-react").AddPanelOptions["position"];
if (chatPanel) {
position = { referenceGroup: chatPanel.group.id };
if (outgoingGroupId && api.groups.some((g) => g.id === outgoingGroupId)) {
position = { referenceGroup: outgoingGroupId };
} else if (sidebarPanel) {
position = { direction: "right" as const, referencePanel: "sidebar" };
}
+3 -2
View File
@@ -152,7 +152,6 @@ export class WebSocketClient {
const currentCount = this.sessionSubscriptions.get(sessionId) ?? 0;
const nextCount = currentCount + 1;
this.sessionSubscriptions.set(sessionId, nextCount);
if (this.status === "open" && nextCount === 1) {
this.send({
id: generateUUID(),
@@ -231,7 +230,9 @@ export class WebSocketClient {
unsubscribeSession(sessionId: string) {
const currentCount = this.sessionSubscriptions.get(sessionId);
if (!currentCount) return;
if (!currentCount) {
return;
}
const nextCount = currentCount - 1;
if (nextCount <= 0) {