mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 19:34:45 +00:00
fix(gateway): bound traced channel startup handoff (#82592)
* 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
This commit is contained in:
committed by
GitHub
parent
eebdbabae9
commit
16e5d6692d
@@ -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.
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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)) {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user