The previous implementation used `flushIfNeeded` to batch most telemetry events, but it was not reliably sending them, leading to data loss. Notably, the `startSession` event, which already used `flushToClearcut`, was working correctly, indicating an issue with the batching logic itself.
This change replaces all calls to `flushIfNeeded` with `flushToClearcut` to align all event logging with the working `startSession` implementation and ensure that events are sent immediately. This prioritizes the reliability of data collection over network efficiency.
This is a temporary solution to prevent further data loss. The underlying issue with the batching mechanism in `flushIfNeeded` should be investigated and fixed in the future, at which point this change can be reverted.
Renames the `disableDataCollection` flag to the more intuitive and positive `dataCollectionEnabled`.
This change improves code clarity by avoiding double negatives and making the purpose of the flag more direct. The logic has been inverted wherever the flag is used to accommodate the new naming convention.
Using a suffix like `"Enabled"` follows a common convention that improves readability.
- A condition like `if (dataCollectionEnabled)` reads like a natural language sentence ("if data collection is enabled"), which reduces cognitive load.
- Distinguishes the boolean flag (representing a state) from potential functions that would perform an action (e.g., `enableDataCollection()` or `disableDataCollection()`), avoiding ambiguity between checking a value and calling a function.
#750
- Introduces a suite of tests for the hook, covering various scenarios including:
- Successful tool execution
- Tool not found errors
- Errors during
- Errors during tool execution
- Tool confirmation (approved and cancelled) - (currently skipped)
- Live output updates - (currently skipped)
- Cancellation of tool calls (before execution and during approval) - (currently skipped)
- Execution of multiple tool calls
- Preventing scheduling while other calls are running - (currently skipped)
- Includes tests for the utility function to ensure correct mapping of tool call states to display objects.
- Mocks dependencies like , , and individual instances.
- Uses fake timers to control asynchronous operations.
Note: Some tests involving complex asynchronous interactions (confirmations, live output, cancellations) are currently skipped due to challenges in reliably testing these scenarios with the current setup. These will be addressed in future work.
Josh Soref