fix(gateway): bound traced channel startup handoff (#82592)

steipete · web-flow · commit 16e5d6692dcc · 2026-05-16T15:15:57.000+01:00
* fix(gateway): bound traced channel startup handoff

* fix(github-copilot): guard device login fetches

* fix(gateway): skip stopped traced channel handoffs

* test(net): keep guarded fetch mocks hermetic
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,8 @@ Docs: https://docs.openclaw.ai
 - Agents/sessions: preserve fresh post-compaction token snapshots across stale usage updates, preventing repeated auto-compaction after every message. Fixes #82576. (#82578) Thanks @njuboy11.
 - Gateway/sessions: discard stale metadata when recreating dead main session rows, so replacement sessions do not inherit old labels or transcript paths.
 - Codex app-server: mark native context compaction completion events as successful, preventing false "Compaction incomplete" notices after successful Codex-managed compaction. Fixes #82470. (#81593) Thanks @Kyzcreig.
+- Gateway/channels: hand off traced channel account startup outside the startup diagnostic phase so long-lived channel tasks do not keep liveness warnings pinned to channel startup. Refs #82398.
+- GitHub Copilot: route device-login requests through the plugin SSRF guard with a GitHub-only policy.
 - Gateway/WebChat: route image attachments through a configured vision-capable `imageModel` plan before inlining images, and carry that image-model fallback chain through runtime retries. (#82524) Thanks @frankekn.
 - WebChat: show progress while manual `/compact` is running by streaming a session operation event to subscribed Control UI clients. Fixes #82407. Thanks @Conan-Scott.
 - Codex app-server: limit canonical OpenAI Codex app-server attribution rewrites to local transcript and trajectory records, leaving runtime/tool routing on the selected OpenAI model metadata so OpenAI API-key backup profiles keep their billing path.
diff --git a/extensions/github-copilot/login.ts b/extensions/github-copilot/login.ts
@@ -7,12 +7,13 @@ import {
   upsertAuthProfileWithLock,
 } from "openclaw/plugin-sdk/provider-auth";
 import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime";
-import { fetchWithSsrFGuard } from "openclaw/plugin-sdk/ssrf-runtime";
+import { fetchWithSsrFGuard, type SsrFPolicy } from "openclaw/plugin-sdk/ssrf-runtime";
 
 const CLIENT_ID = "Iv1.b507a08c87ecfe98";
 const DEVICE_CODE_URL = "https://github.com/login/device/code";
 const ACCESS_TOKEN_URL = "https://github.com/login/oauth/access_token";
 const GITHUB_DEVICE_VERIFICATION_URL = "https://github.com/login/device";
+const GITHUB_AUTH_SSRF_POLICY: SsrFPolicy = { hostnameAllowlist: ["github.com"] };
 
 type DeviceCodeResponse = {
   device_code: string;
@@ -96,6 +97,7 @@ async function postGitHubDeviceFlowForm(params: {
       body: params.body,
     },
     requireHttps: true,
+    policy: GITHUB_AUTH_SSRF_POLICY,
     auditContext: "github-copilot-device-flow",
   });
   try {
diff --git a/src/gateway/server-channels.test.ts b/src/gateway/server-channels.test.ts
@@ -733,12 +733,74 @@ describe("server-channels auto restart", () => {
     const manager = createManager({ startupTrace });
 
     await manager.startChannels();
+    expect(startAccount).not.toHaveBeenCalled();
+
+    await vi.advanceTimersByTimeAsync(0);
+    await flushMicrotasks();
 
     const names = measureMock.mock.calls.map(([name]) => name);
     expect(names).toContain("channels.discord.start");
     expect(names).toContain("channels.discord.list-accounts");
     expect(names).toContain("channels.discord.runtime");
     expect(names).toContain("channels.discord.approval-bootstrap");
+    expect(names).toContain("channels.discord.start-account-handoff");
+    expect(startAccount).toHaveBeenCalledTimes(1);
+  });
+
+  it("ends startup trace spans before long-lived channel account tasks settle", async () => {
+    const activeNames = new Set<string>();
+    const measuredNames: string[] = [];
+    const startupTrace = {
+      measure: async <T>(name: string, run: () => T | Promise<T>) => {
+        activeNames.add(name);
+        measuredNames.push(name);
+        try {
+          return await run();
+        } finally {
+          activeNames.delete(name);
+        }
+      },
+    };
+    const channelTask = createDeferred();
+    const startAccount = vi.fn(() => channelTask.promise);
+
+    installTestRegistry(createTestPlugin({ startAccount }));
+    const manager = createManager({ startupTrace });
+
+    await manager.startChannels();
+    await vi.advanceTimersByTimeAsync(0);
+    await flushMicrotasks();
+
+    expect(startAccount).toHaveBeenCalledTimes(1);
+    expect(measuredNames).toContain("channels.discord.start-account-handoff");
+    expect(activeNames.has("channels.discord.start-account-handoff")).toBe(false);
+    expect(
+      manager.getRuntimeSnapshot().channelAccounts.discord?.[DEFAULT_ACCOUNT_ID]?.running,
+    ).toBe(true);
+
+    channelTask.resolve();
+    await flushMicrotasks();
+  });
+
+  it("does not start traced channel accounts after stop wins the handoff", async () => {
+    const startupTrace = {
+      measure: async <T>(_name: string, run: () => T | Promise<T>) => await run(),
+    };
+    const startAccount = vi.fn(async () => {});
+
+    installTestRegistry(createTestPlugin({ startAccount }));
+    const manager = createManager({ startupTrace });
+
+    await manager.startChannel("discord", DEFAULT_ACCOUNT_ID);
+    const stopTask = manager.stopChannel("discord", DEFAULT_ACCOUNT_ID);
+    await vi.advanceTimersByTimeAsync(0);
+    await stopTask;
+    await flushMicrotasks();
+
+    expect(startAccount).not.toHaveBeenCalled();
+    expect(
+      manager.getRuntimeSnapshot().channelAccounts.discord?.[DEFAULT_ACCOUNT_ID]?.running,
+    ).toBe(false);
   });
 
   it("limits whole-channel account startup fanout to four", async () => {
diff --git a/src/gateway/server-channels.ts b/src/gateway/server-channels.ts
@@ -39,6 +39,13 @@ const MAX_RESTART_ATTEMPTS = 10;
 const CHANNEL_STOP_ABORT_TIMEOUT_MS = 5_000;
 const CHANNEL_STARTUP_CONCURRENCY = 4;
 
+function waitForChannelStartupHandoff(): Promise<void> {
+  return new Promise((resolve) => {
+    const handle = setImmediate(resolve);
+    handle.unref?.();
+  });
+}
+
 type ChannelRuntimeStore = {
   aborts: Map<string, AbortController>;
   starting: Map<string, Promise<void>>;
@@ -512,9 +519,16 @@ export function createChannelManager(opts: ChannelManagerOptions): ChannelManage
             lastError: null,
             reconnectAttempts: preserveRestartAttempts ? (restartAttempts.get(rKey) ?? 0) : 0,
           });
-          const task = Promise.resolve().then(() =>
-            measureStartup(`channels.${channelId}.start-account`, () =>
-              startAccount({
+          const task = Promise.resolve().then(async () => {
+            if (startupTrace) {
+              await waitForChannelStartupHandoff();
+            }
+            if (abort.signal.aborted || manuallyStopped.has(rKey)) {
+              return;
+            }
+            let startAccountTask: ReturnType<typeof startAccount> | undefined;
+            await measureStartup(`channels.${channelId}.start-account-handoff`, () => {
+              startAccountTask = startAccount({
                 cfg,
                 accountId: id,
                 account,
@@ -524,9 +538,10 @@ export function createChannelManager(opts: ChannelManagerOptions): ChannelManage
                 getStatus: () => getRuntime(channelId, id),
                 setStatus: (next) => setRuntime(channelId, id, next),
                 ...(channelRuntimeForTask ? { channelRuntime: channelRuntimeForTask } : {}),
-              }),
-            ),
-          );
+              });
+            });
+            await startAccountTask;
+          });
           const trackedPromise = task
             .then(() => {
               if (abort.signal.aborted || manuallyStopped.has(rKey)) {
diff --git a/src/infra/net/fetch-guard.ts b/src/infra/net/fetch-guard.ts
@@ -347,6 +347,7 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
   if (!defaultFetch) {
     throw new Error("fetch is not available");
   }
+  const isUsingMockedFetch = isMockedFetch(defaultFetch);
 
   const maxRedirects =
     typeof params.maxRedirects === "number" && Number.isFinite(params.maxRedirects)
@@ -413,6 +414,13 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
         shouldUseEnvHttpProxyForUrl(parsedUrl.toString());
       const canUseManagedProxy =
         mode === GUARDED_FETCH_MODE.STRICT && isManagedProxyActive() && hasProxyEnvConfigured();
+      const canUseMockedFetchWithoutDns =
+        isUsingMockedFetch &&
+        params.lookupFn === undefined &&
+        !canUseTrustedEnvProxy &&
+        !canUseManagedProxy &&
+        !usesTrustedExplicitProxyMode &&
+        params.pinDns !== false;
       const timeoutMs = resolveDispatcherTimeoutMs(params.timeoutMs);
 
       // Trusted env-proxy and pinDns=false can skip local DNS pinning, so keep
@@ -434,6 +442,10 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
         // policy, but the proxy does the DNS resolution for the final target.
         assertHostnameAllowedWithPolicy(parsedUrl.hostname, policyForUrl);
         dispatcher = createPolicyDispatcherWithoutPinnedDns(params.dispatcherPolicy, timeoutMs);
+      } else if (canUseMockedFetchWithoutDns) {
+        // Test-installed fetch mocks should stay hermetic. Host/IP policy still runs;
+        // real fetches continue through pinned DNS below.
+        assertHostnameAllowedWithPolicy(parsedUrl.hostname, policyForUrl);
       } else if (params.pinDns === false) {
         await resolvePinnedHostnameWithPolicy(parsedUrl.hostname, {
           lookupFn: params.lookupFn,
@@ -466,7 +478,7 @@ export async function fetchWithSsrFGuard(params: GuardedFetchOptions): Promise<G
             fetchImpl: params.fetchImpl,
             globalFetch: globalThis.fetch,
           })) ||
-        isMockedFetch(defaultFetch);
+        isUsingMockedFetch;
       // Explicit caller stubs and test-installed fetch mocks should win.
       // Otherwise, fall back to undici's fetch whenever we attach a dispatcher,
       // because the default global fetch path will not honor per-request
