ToolCallingLoop: avoid duplicate start hooks

[bharatvansh]

What

Fixes a foot gun where runStartHooks() can be invoked by callers (e.g. to run SessionStart/SubagentStart before other hooks) and then run() would effectively run the same “start” work again.

Why

This can lead to duplicate SessionStart / SubagentStart executions (and transcript initialization/logging) for a single ToolCallingLoop instance.

Changes

• Make ToolCallingLoop.run() always call runStartHooks() up-front.
• Make runStartHooks() idempotent per loop instance via a promise latch, so multiple calls safely share the same work.
• Add regression tests ensuring SessionStart/SubagentStart aren’t executed twice when runStartHooks() is called before run().

Also (separate commit)

• Stabilize chat replay notebook integer formatting by using a deterministic formatter (avoids locale-dependent output like 100000 → 1,00,000 on en-IN, which makes tests/output flaky).

Verification

• npm run test:unit
• npm run test:extension
• npm run lint

[Copilot]

Pull request overview

This PR makes ToolCallingLoop start-hook execution (SessionStart / SubagentStart) safe to call multiple times by making it idempotent per loop instance, preventing duplicated hook execution and transcript side effects. It also stabilizes chat replay notebook output by using locale-independent integer formatting.

Changes:

• Make ToolCallingLoop.run() always invoke runStartHooks() and remove duplicate “start” work paths.
• Make runStartHooks() idempotent via a per-instance promise latch; add regression tests preventing double SessionStart/SubagentStart.
• Replace locale-dependent toLocaleString() formatting in replay notebook output with deterministic Intl.NumberFormat('en-US').

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/extension/intents/node/toolCallingLoop.ts	Adds a promise latch to make start hooks idempotent; ensures run() triggers start hooks exactly once.
src/extension/intents/test/node/toolCallingLoopHooks.spec.ts	Adds regression tests for “runStartHooks called before run” to prevent duplicate start-hook execution.
src/extension/replay/vscode-node/chatReplayNotebookSerializer.ts	Uses deterministic integer formatting to avoid locale-dependent notebook output.

[Copilot]

These new tests build a ChatRequest via createMockChatRequest() that omits required fields like hasHooksEnabled (it’s required by the VS Code type). Because the helper casts the object to ChatRequest, this can mask behavior differences between hasHooksEnabled: true/false (e.g. transcript initialization) and makes the mock less representative. Consider setting a real boolean default in createMockChatRequest (or using the existing TestChatRequest helper) and overriding it explicitly per test.

[Copilot]

runStartHooks() is now latched on the first call’s outputStream/token. Any subsequent call with a different stream/token will silently reuse the original promise, which can lead to missing hook progress output or cancellation behavior that doesn’t match the caller’s token. Consider either (1) asserting/logging when subsequent calls pass different arguments, or (2) capturing only the hook execution promise and keeping progress reporting/cancellation independent of the first caller.

Suggested change

	public async runStartHooks(outputStream: ChatResponseStream | undefined, token: CancellationToken): Promise<void> {
	this._startHooksPromise ??= this.doRunStartHooks(outputStream, token);
	private _startHooksFirstOutputStream: ChatResponseStream | undefined;
	private _startHooksFirstToken: CancellationToken | undefined;
	public async runStartHooks(outputStream: ChatResponseStream | undefined, token: CancellationToken): Promise<void> {
	if (!this._startHooksPromise) {
	this._startHooksFirstOutputStream = outputStream;
	this._startHooksFirstToken = token;
	this._startHooksPromise = this.doRunStartHooks(outputStream, token);
	} else if (outputStream !== this._startHooksFirstOutputStream || token !== this._startHooksFirstToken) {
	this._logService.warn('[ToolCallingLoop] runStartHooks called with different outputStream or token after hooks were already started; reusing the first call\'s arguments.');
	}