Chronicle: Circuit breaker improvement and empty result hint (#317670)

* Chronicle: Circuit breaker improvement and empty result hint

* feedback changes

extensions/copilot/assets/prompts/skills/chronicle/SKILL.md

@@ -29,8 +29,9 @@ When the user asks for a standup, daily summary, or "what did I do":
 
 1. Call `copilot_sessionStoreSql` with `action: "standup"` and `description: "Generate standup"`.
 2. The tool returns pre-fetched session data (sessions, turns, files, refs from the last 24 hours).
-3. For any PR references in the data, check their current status (open, merged, draft) if possible.
-4. Format the returned data as a standup report grouped by work stream (branch/feature):
+3. If the result is empty, tell the user no sessions were found in the last 24h, suggest `/chronicle:reindex`, and stop — do not fabricate a standup.
+4. For any PR references in the data, check their current status (open, merged, draft) if possible.
+5. Format the returned data as a standup report grouped by work stream (branch/feature):
 
 ```
 Standup for <date>:

extensions/copilot/src/extension/chronicle/common/circuitBreaker.ts

@@ -26,6 +26,8 @@ export interface CircuitBreakerOptions {
 	probeTimeoutMs: number;
 	/** Maximum reset timeout after exponential backoff on failed probes. Defaults to resetTimeoutMs (no backoff). */
 	maxResetTimeoutMs?: number;
+	/** Optional callback fired whenever the circuit state changes. */
+	onStateChange?: (from: CircuitState, to: CircuitState) => void;
 }
 
 const DEFAULT_OPTIONS: CircuitBreakerOptions = {
@@ -59,6 +61,19 @@ export class CircuitBreaker {
 		this.currentResetTimeoutMs = this.options.resetTimeoutMs;
 	}
 
+	private _setState(next: CircuitState): void {
+		if (this.state === next) {
+			return;
+		}
+		const prev = this.state;
+		this.state = next;
+		try {
+			this.options.onStateChange?.(prev, next);
+		} catch {
+			// Best-effort: a misbehaving listener must not break the breaker.
+		}
+	}
+
 	/**
 	 * Get the current state of the circuit breaker.
 	 */
@@ -103,7 +118,16 @@ export class CircuitBreaker {
 		this.failureCount = 0;
 		this.probeInFlight = false;
 		this.currentResetTimeoutMs = this.options.resetTimeoutMs;
-		this.state = CircuitState.CLOSED;
+		this._setState(CircuitState.CLOSED);
+	}
+
+	/**
+	 * Release a probe slot that was consumed by `canRequest()` but for which
+	 * no real result is available (e.g. the caller decided not to make a
+	 * request after all). Does not change failure count or open/closed state.
+	 */
+	cancelProbe(): void {
+		this.probeInFlight = false;
 	}
 
 	/**
@@ -111,12 +135,18 @@ export class CircuitBreaker {
 	 */
 	recordFailure(): void {
 		const wasHalfOpen = this.state === CircuitState.HALF_OPEN;
-		this.failureCount++;
+		// Clamp the failure count at the threshold once the circuit is open so
+		// it does not grow unboundedly across many failed probes (otherwise
+		// telemetry reports an ever-increasing number that suggests the breaker
+		// is permanently dead even though it is still probing).
+		if (this.failureCount < this.options.failureThreshold) {
+			this.failureCount++;
+		}
 		this.lastFailureTime = Date.now();
 		this.probeInFlight = false;
 
 		if (this.failureCount >= this.options.failureThreshold) {
-			this.state = CircuitState.OPEN;
+			this._setState(CircuitState.OPEN);
 		}
 
 		// Exponential backoff: double the probe interval after each failed probe
@@ -137,12 +167,12 @@ export class CircuitBreaker {
 	 * Force reset the circuit breaker to closed state.
 	 */
 	reset(): void {
-		this.state = CircuitState.CLOSED;
 		this.failureCount = 0;
 		this.lastFailureTime = 0;
 		this.probeInFlight = false;
 		this.probeStartTime = 0;
 		this.currentResetTimeoutMs = this.options.resetTimeoutMs;
+		this._setState(CircuitState.CLOSED);
 	}
 
 	/**
@@ -153,7 +183,7 @@ export class CircuitBreaker {
 		if (this.state === CircuitState.OPEN) {
 			const elapsed = Date.now() - this.lastFailureTime;
 			if (elapsed >= this.currentResetTimeoutMs) {
-				this.state = CircuitState.HALF_OPEN;
+				this._setState(CircuitState.HALF_OPEN);
 			}
 		}
 	}

extensions/copilot/src/extension/chronicle/common/test/circuitBreaker.spec.ts

@@ -149,4 +149,59 @@ describe('CircuitBreaker', () => {
 		// Probe timed out, should allow another
 		expect(cb.canRequest()).toBe(true);
 	});
+
+	it('cancelProbe releases an unused probe slot without changing state', () => {
+		const cb = new CircuitBreaker({ failureThreshold: 1, resetTimeoutMs: 0 });
+		cb.recordFailure();
+		expect(cb.getState()).toBe(CircuitState.HALF_OPEN);
+
+		expect(cb.canRequest()).toBe(true); // probe consumed
+		expect(cb.canRequest()).toBe(false); // second probe blocked
+
+		cb.cancelProbe();
+		expect(cb.getState()).toBe(CircuitState.HALF_OPEN);
+		expect(cb.getFailureCount()).toBe(1);
+		expect(cb.canRequest()).toBe(true); // probe available again immediately
+	});
+
+	it('clamps failure count at the threshold across repeated probe failures', () => {
+		const cb = new CircuitBreaker({
+			failureThreshold: 3,
+			resetTimeoutMs: 0,
+			maxResetTimeoutMs: 100,
+			probeTimeoutMs: 5,
+		});
+		for (let i = 0; i < 3; i++) {
+			cb.recordFailure();
+		}
+		expect(cb.getFailureCount()).toBe(3);
+
+		for (let i = 0; i < 10; i++) {
+			cb.canRequest();
+			cb.recordFailure();
+		}
+		expect(cb.getFailureCount()).toBe(3);
+	});
+
+	it('fires onStateChange for every transition', () => {
+		const transitions: Array<[CircuitState, CircuitState]> = [];
+		const cb = new CircuitBreaker({
+			failureThreshold: 1,
+			resetTimeoutMs: 10,
+			probeTimeoutMs: 5,
+			onStateChange: (from, to) => transitions.push([from, to]),
+		});
+
+		cb.recordFailure(); // CLOSED → OPEN
+		vi.advanceTimersByTime(10);
+		expect(cb.getState()).toBe(CircuitState.HALF_OPEN); // OPEN → HALF_OPEN
+		cb.canRequest();
+		cb.recordSuccess(); // HALF_OPEN → CLOSED
+
+		expect(transitions).toEqual([
+			[CircuitState.CLOSED, CircuitState.OPEN],
+			[CircuitState.OPEN, CircuitState.HALF_OPEN],
+			[CircuitState.HALF_OPEN, CircuitState.CLOSED],
+		]);
+	});
 });

extensions/copilot/src/extension/chronicle/vscode-node/remoteSessionExporter.ts

@@ -237,6 +237,14 @@ export class RemoteSessionExporter extends Disposable implements IExtensionContr
 			failureThreshold: 5,
 			resetTimeoutMs: 1_000,
 			maxResetTimeoutMs: 30_000,
+			onStateChange: (_from, to) => {
+				this._telemetryService.sendMSFTTelemetryEvent('chronicle.cloudSync', {
+					operation: 'circuitBreaker',
+					transition: to.toLowerCase(),
+				}, {
+					failureCount: this._circuitBreaker.getFailureCount(),
+				});
+			},
 		});
 
 		// Register delete cloud sessions command
@@ -818,7 +826,7 @@ export class RemoteSessionExporter extends Disposable implements IExtensionContr
 						"indexingLevel": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "The indexing level for the session." },
 						"droppedEvents": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "comment": "Number of events in a failed batch." },
 						"reason": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Reason session was disabled (no_consent, no_repo, init_error, create_error, policy_blocked_cached)." },
-						"transition": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Circuit breaker state transition (open, closed)." },
+						"transition": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "comment": "Circuit breaker state transition (open, half_open, closed)." },
 						"eventsCount": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "comment": "Number of actually submitted events (sum of eventsBySession sizes)." },
 						"orphanedCount": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "comment": "Number of orphaned events not submitted (re-queued or dropped)." },
 						"batchDurationMs": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "comment": "Time to submit batch in ms." },
@@ -1144,6 +1152,9 @@ export class RemoteSessionExporter extends Disposable implements IExtensionContr
 		// splice/unshift churn on each timer tick. _bufferEvents still enforces
 		// MAX_BUFFER_SIZE so memory stays bounded.
 		if (this._cloudClient.isRateLimited()) {
+			// Release the probe slot consumed by canRequest() above so we don't
+			// burn it on a flush we never actually attempted.
+			this._circuitBreaker.cancelProbe();
 			return;
 		}
 
@@ -1255,9 +1266,7 @@ export class RemoteSessionExporter extends Disposable implements IExtensionContr
 					orphanedCount: orphanedEntries.length,
 					batchDurationMs: Date.now() - batchStart,
 					bufferSize: this._eventBuffer.length,
-				});
-
-				if (!this._firstCloudWriteLogged) {
+				}); if (!this._firstCloudWriteLogged) {
 					this._firstCloudWriteLogged = true;
 
 					this._telemetryService.sendMSFTTelemetryEvent('chronicle.cloudSync', {
@@ -1270,14 +1279,20 @@ export class RemoteSessionExporter extends Disposable implements IExtensionContr
 				this._setSyncState({ kind: 'error' });
 
 				this._telemetryService.sendMSFTTelemetryEvent('chronicle.cloudSync', {
-					operation: 'circuitBreaker',
-					transition: 'open',
+					operation: 'flushFailure',
 				}, {
 					failureCount: this._circuitBreaker.getFailureCount(),
 					eventsCount: submittedCount,
 					orphanedCount: orphanedEntries.length,
 					bufferSize: this._eventBuffer.length,
 				});
+			} else {
+				// Nothing failed but there was also nothing to submit (eg. all
+				// entries were orphans, or only policy/rate-limited sessions).
+				// Release any probe slot consumed by canRequest() so HALF_OPEN
+				// can probe again on the next tick instead of waiting for the
+				// probe timeout.
+				this._circuitBreaker.cancelProbe();
 			}
 
 			if (policyBlockedSessions > 0) {