fix(cli): gate cron scheduler startup on config initialization (#5022) (#5230)
Some checks are pending
Qwen Code CI / Classify PR (push) Waiting to run
Qwen Code CI / Lint (push) Blocked by required conditions
Qwen Code CI / Test (macos-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (ubuntu-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Test (windows-latest, Node 22.x) (push) Blocked by required conditions
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Blocked by required conditions
Qwen Code CI / Integration Tests (CLI, No Sandbox) (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run

The TUI startup path called enableDurable() fire-and-forget and then
start() synchronously. Because start() installs onFire before
enableDurable() finishes loading tasks from disk, overdue durable fires
were delivered instantly into the notification queue and reached a chat
client whose startChat() had not completed — producing "Chat not
initialized" on fresh launches with pending durable work.

Gate the cron effect on isConfigInitialized and await enableDurable()
before start(), matching the ordering the ACP (Session.ts) and headless
(nonInteractiveCli.ts) paths already use. A `stopped` flag prevents a
stale start() if the component unmounts during the async gap.

On enableDurable() failure the catch falls through (does not return) so
start() still runs: a failed durable init must not silently disable
session-only cron tasks (created via cron_create during the session) —
only durable/persistent tasks are lost.

Tests: gate ordering (with a real async gap so the assertion has teeth),
gate-stays-closed, unmount-during-gap, and enableDurable-rejection-
still-starts. The last two lock in the unmount guard and the fall-through
fix (both verified by mutation).

Co-authored-by: Shaojin Wen <szujobs@gmail.com>
This commit is contained in:
qwen-code-ci-bot 2026-06-20 01:00:40 +08:00 committed by GitHub
parent 715ef938f5
commit ad56a84eff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 241 additions and 26 deletions

View file

@ -943,11 +943,11 @@ describe('AppContainer State Management', () => {
describe('Cancel Handler (issue #3204)', () => {
// The cancel handler is wired through useGeminiStream's onCancelSubmit
// arg (positional index 14 — see the useGeminiStream call site in
// arg (positional index 15 — see the useGeminiStream call site in
// AppContainer.tsx). We capture it via mockImplementation so a future
// signature change surfaces as a clear test failure rather than silently
// grabbing the wrong callback.
const ON_CANCEL_SUBMIT_ARG_INDEX = 14;
const ON_CANCEL_SUBMIT_ARG_INDEX = 15;
type CapturedCancelSubmit = (info?: {
pendingItem: HistoryItemWithoutId | null;
lastTurnUserItem: { id: number; text: string } | null;

View file

@ -1483,6 +1483,7 @@ export const AppContainer = (props: AppContainerProps) => {
historyManager.history,
historyManager.addItem,
config,
isConfigInitialized,
settings,
onDebugMessage,
handleSlashCommand,

View file

@ -332,6 +332,7 @@ describe('useGeminiStream', () => {
props.history,
props.addItem,
props.config,
true,
props.loadedSettings,
props.onDebugMessage,
props.handleSlashCommand,
@ -620,6 +621,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -719,6 +721,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -854,6 +857,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -996,6 +1000,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -1115,6 +1120,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -1239,6 +1245,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -1352,6 +1359,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -1458,6 +1466,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -1557,6 +1566,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -1645,6 +1655,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -1757,6 +1768,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -1861,6 +1873,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -2140,6 +2153,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -2278,6 +2292,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -2425,6 +2440,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -2540,6 +2556,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -2662,6 +2679,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -2863,6 +2881,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -2988,6 +3007,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -3126,6 +3146,7 @@ describe('useGeminiStream', () => {
historyWithToolGroup,
mockAddItem,
config,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -3307,6 +3328,7 @@ describe('useGeminiStream', () => {
history,
mockAddItem,
config,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -3804,6 +3826,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -3853,6 +3876,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -3909,6 +3933,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -3968,6 +3993,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -4049,6 +4075,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -4104,6 +4131,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -4161,6 +4189,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -4234,6 +4263,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -4582,6 +4612,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -4639,6 +4670,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
testConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -5132,6 +5164,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -5187,6 +5220,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -5243,6 +5277,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -5355,6 +5390,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -5409,6 +5445,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -5483,6 +5520,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -5580,6 +5618,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -5986,6 +6025,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -6102,6 +6142,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -6175,6 +6216,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -6245,6 +6287,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -6374,6 +6417,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -6429,6 +6473,7 @@ describe('useGeminiStream', () => {
[],
mockAddItem,
mockConfig,
true,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand,
@ -7498,6 +7543,150 @@ describe('useGeminiStream', () => {
});
});
});
describe('cron scheduler initialization', () => {
// Renders useGeminiStream wired to a provided cron scheduler mock, with a
// controllable isConfigInitialized gate. `config` identity is stable across
// rerenders so the cron effect only re-runs when `initialized` flips.
const renderCronHook = (scheduler: unknown, initialized: boolean) => {
const cronConfig = {
...mockConfig,
isCronEnabled: vi.fn(() => true),
getCronScheduler: vi.fn(() => scheduler),
} as unknown as Config;
return renderHook(
(props: { initialized: boolean }) =>
useGeminiStream(
new MockedGeminiClientClass(cronConfig),
[],
mockAddItem,
cronConfig,
props.initialized,
mockLoadedSettings,
mockOnDebugMessage,
mockHandleSlashCommand as unknown as (
cmd: PartListUnion,
) => Promise<SlashCommandProcessorResult | false>,
false,
() => 'vscode' as EditorType,
() => {},
() => Promise.resolve(),
false,
() => {},
() => {},
() => {},
() => {},
80,
24,
),
{ initialProps: { initialized } },
);
};
it('defers enableDurable and start until isConfigInitialized is true', async () => {
const callOrder: string[] = [];
const scheduler = {
// A real async gap before recording: a synchronous push would make the
// order assertion pass even if production dropped the `await`.
enableDurable: vi.fn().mockImplementation(async () => {
await new Promise((r) => setTimeout(r, 10));
callOrder.push('enableDurable');
}),
start: vi.fn().mockImplementation(() => {
callOrder.push('start');
}),
stop: vi.fn(),
getExitSummary: vi.fn(() => null),
hasPendingWork: false,
};
const { rerender } = renderCronHook(scheduler, false);
// Before initialization: the scheduler must not be touched.
expect(scheduler.enableDurable).not.toHaveBeenCalled();
expect(scheduler.start).not.toHaveBeenCalled();
rerender({ initialized: true });
await waitFor(() => {
expect(scheduler.start).toHaveBeenCalled();
});
// enableDurable is awaited before start despite the 10ms gap.
expect(callOrder).toEqual(['enableDurable', 'start']);
});
it('does not start scheduler when isConfigInitialized remains false', async () => {
const scheduler = {
enableDurable: vi.fn().mockResolvedValue(undefined),
start: vi.fn(),
stop: vi.fn(),
getExitSummary: vi.fn(() => null),
hasPendingWork: false,
};
renderCronHook(scheduler, false);
// Give effects time to run.
await new Promise((resolve) => setTimeout(resolve, 50));
expect(scheduler.enableDurable).not.toHaveBeenCalled();
expect(scheduler.start).not.toHaveBeenCalled();
});
it('does not start scheduler if unmounted during the enableDurable gap', async () => {
// enableDurable stays pending until we resolve it by hand, so we can
// unmount inside the async gap — the exact race the `stopped` flag guards.
let resolveEnable: () => void = () => {};
const scheduler = {
enableDurable: vi.fn().mockImplementation(
() =>
new Promise<void>((resolve) => {
resolveEnable = resolve;
}),
),
start: vi.fn(),
stop: vi.fn(),
getExitSummary: vi.fn(() => null),
hasPendingWork: false,
};
const { unmount } = renderCronHook(scheduler, true);
await waitFor(() => {
expect(scheduler.enableDurable).toHaveBeenCalled();
});
expect(scheduler.start).not.toHaveBeenCalled();
// Unmount while enableDurable is still in flight, then let it resolve.
unmount();
resolveEnable();
await new Promise((resolve) => setTimeout(resolve, 10));
// The stopped guard must suppress the late start(); cleanup ran stop().
expect(scheduler.start).not.toHaveBeenCalled();
expect(scheduler.stop).toHaveBeenCalled();
});
it('still starts the scheduler when enableDurable rejects', async () => {
const scheduler = {
enableDurable: vi.fn().mockRejectedValue(new Error('lock contention')),
start: vi.fn(),
stop: vi.fn(),
getExitSummary: vi.fn(() => null),
hasPendingWork: false,
};
renderCronHook(scheduler, true);
// A failed enableDurable must NOT skip start(): session-only cron tasks
// (created via cron_create during this session) still need the scheduler
// running — only durable/persistent tasks are lost. Regression guard for
// the catch falling through instead of returning (#5022 review).
await waitFor(() => {
expect(scheduler.start).toHaveBeenCalled();
});
});
});
});
describe('classifyApiError', () => {

View file

@ -349,6 +349,7 @@ export const useGeminiStream = (
history: HistoryItem[],
addItem: UseHistoryManagerReturn['addItem'],
config: Config,
isConfigInitialized: boolean,
settings: LoadedSettings,
onDebugMessage: (message: string) => void,
handleSlashCommand: (
@ -2924,44 +2925,68 @@ export const useGeminiStream = (
const cronSessionIdRef = useRef(sessionStates.sessionId);
cronSessionIdRef.current = sessionStates.sessionId;
// Start the cron scheduler on mount, stop on unmount.
// Start the cron scheduler once config is initialized, stop on unmount.
// Cron fires enqueue onto the shared notification queue.
// Gated on isConfigInitialized: without this gate, enableDurable() runs
// before config.initialize() completes, and overdue-task fires delivered
// through the notification drain reach a chat client whose startChat() has
// not yet run — producing "Chat not initialized" on every fresh launch
// that has pending durable work (#5022). This matches the ordering the
// ACP (Session.ts) and headless (nonInteractiveCli.ts) paths already use.
useEffect(() => {
if (!isConfigInitialized) return;
if (!config.isCronEnabled()) return;
const scheduler = config.getCronScheduler();
// Enable durable (file-backed) cron support (loads tasks from the
// user's per-project runtime dir, acquires the lock). The tasks file
// lives under ~/.qwen, not the working tree, so it's user-owned rather
// than project-controlled — no folder-trust gate needed; the user's
// own loops run regardless of how the folder is trusted.
// Missed one-shots arrive as late fires through the start() callback.
void scheduler.enableDurable(cronSessionIdRef.current).catch((err) => {
debugLogger.warn(
`Durable cron init failed — persistent tasks will not fire in this session: ${err}`,
let stopped = false;
// Await enableDurable before start so overdue fires buffer into
// pendingFires (onFire is still null) and flush through start()'s
// buffer-drain — matching the ACP and headless startup order.
void (async () => {
try {
// Enable durable (file-backed) cron support (loads tasks from the
// user's per-project runtime dir, acquires the lock). The tasks file
// lives under ~/.qwen, not the working tree, so it's user-owned
// rather than project-controlled — no folder-trust gate needed; the
// user's own loops run regardless of how the folder is trusted.
// Missed one-shots arrive as late fires through the start() callback.
await scheduler.enableDurable(cronSessionIdRef.current);
} catch (err) {
// Fall through (no `return`): a failed enableDurable must NOT skip
// start(), or session-only cron tasks (created via cron_create during
// this session) would silently never fire. Only durable/persistent
// tasks are lost when enableDurable fails. Pre-#5022 the unconditional
// start() preserved this; keep that behavior.
debugLogger.warn(
`Durable cron init failed — persistent tasks will not fire in this session: ${err}`,
);
}
// Unmount may have happened during the await above; the cleanup below
// already ran scheduler.stop(), so do not (re)install onFire.
if (stopped) return;
scheduler.start(
(job: { prompt: string; cronExpr?: string; missed?: boolean }) => {
const label = job.prompt.slice(0, 40);
const source = job.cronExpr === '@wakeup' ? 'Loop' : 'Cron';
notificationQueueRef.current.push({
displayText: `${job.missed ? 'Missed' : source}: ${label}`,
modelText: job.prompt,
sendMessageType: SendMessageType.Cron,
});
setNotificationTrigger((n) => n + 1);
},
);
});
})();
scheduler.start(
(job: { prompt: string; cronExpr?: string; missed?: boolean }) => {
const label = job.prompt.slice(0, 40);
const source = job.cronExpr === '@wakeup' ? 'Loop' : 'Cron';
notificationQueueRef.current.push({
displayText: `${job.missed ? 'Missed' : source}: ${label}`,
modelText: job.prompt,
sendMessageType: SendMessageType.Cron,
});
setNotificationTrigger((n) => n + 1);
},
);
return () => {
stopped = true;
const summary = scheduler.getExitSummary();
scheduler.stop();
if (summary) {
process.stderr.write(summary + '\n');
}
};
}, [config]);
}, [config, isConfigInitialized]);
// Register background agent notification callback onto the shared queue.
useEffect(() => {