fix: flush telemetry events before process exit#46
Merged
anandgupta42 merged 2 commits intomainfrom Mar 5, 2026
Merged
Conversation
Telemetry events were silently lost because: 1. Telemetry.shutdown() was only called from the session prompt flow (prompt.ts). Non-session commands (auth, upgrade, mcp, etc.) tracked events but never flushed them before process.exit(). 2. shutdown() didn't await the in-flight init() promise. Since init() is called fire-and-forget in CLI middleware, shutdown() would see enabled=false and skip the flush entirely. Fix: add Telemetry.shutdown() in the index.ts finally block (before process.exit) and make shutdown() await initPromise so buffered events are flushed even when init hasn't completed yet. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jontsai
reviewed
Mar 5, 2026
jontsai
left a comment
jontsai
left a comment
There was a problem hiding this comment.
PR #46 Review Tour Guide
fix: flush telemetry events before process exit
| # | File | +/- | Purpose |
|---|---|---|---|
| 1 | packages/altimate-code/src/index.ts |
+9 | Add Telemetry.shutdown() in the CLI finally block |
| 2 | packages/altimate-code/src/telemetry/index.ts |
+10 | Make shutdown() await the in-flight init() promise |
Summary
Solid fix for a real bug — telemetry events from non-session CLI commands (auth, upgrade, mcp, etc.) were silently lost because shutdown() was only called from the session prompt path. Additionally, the fire-and-forget init() could still be in-flight when shutdown() ran, causing it to see enabled=false and skip the flush.
Both changes are purely additive (+19/-0), defensive, and idempotent. Error handling is correct — telemetry failures are swallowed so they never block process exit. The initPromise await in shutdown() neatly closes the race condition.
No issues found. Clean PR, well-documented commit message and test plan. LGTM.
| // Wait for init to complete so we know whether telemetry is enabled | ||
| // and have a valid endpoint to flush to. init() is fire-and-forget | ||
| // in CLI middleware, so it may still be in-flight when shutdown runs. | ||
| if (initPromise) { |
There was a problem hiding this comment.
praise: Nice defensive pattern — awaiting the in-flight initPromise before checking enabled state closes the race condition cleanly. The catch swallow is correct here since a failed init means there's nothing to flush.
The TUI worker subprocess calls Telemetry.init() but its shutdown() handler never called Telemetry.shutdown(), causing events tracked in the worker (MCP server status, engine events) to be lost when the worker exits. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
anandgupta42
added a commit
that referenced
this pull request
Mar 5, 2026
- Security FAQ: document well-known URL auth confirmation prompt and command validation (from #45) - Troubleshooting: add MCP server initialization failure section — init errors are now logged instead of silently swallowed (from #45) - Telemetry: document flush retry and flush-before-exit behavior (from #45, #46) - Getting started: mention postinstall welcome banner and upgrade changelog display (from #48) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5 tasks
anandgupta42
added a commit
that referenced
this pull request
Mar 5, 2026
- Security FAQ: document well-known URL auth confirmation prompt and command validation (from #45) - Troubleshooting: add MCP server initialization failure section — init errors are now logged instead of silently swallowed (from #45) - Telemetry: document flush retry and flush-before-exit behavior (from #45, #46) - Getting started: mention postinstall welcome banner and upgrade changelog display (from #48) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
anandgupta42
added a commit
that referenced
this pull request
Mar 17, 2026
* fix: flush telemetry events before process exit Telemetry events were silently lost because: 1. Telemetry.shutdown() was only called from the session prompt flow (prompt.ts). Non-session commands (auth, upgrade, mcp, etc.) tracked events but never flushed them before process.exit(). 2. shutdown() didn't await the in-flight init() promise. Since init() is called fire-and-forget in CLI middleware, shutdown() would see enabled=false and skip the flush entirely. Fix: add Telemetry.shutdown() in the index.ts finally block (before process.exit) and make shutdown() await initPromise so buffered events are flushed even when init hasn't completed yet. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: flush telemetry in TUI worker shutdown The TUI worker subprocess calls Telemetry.init() but its shutdown() handler never called Telemetry.shutdown(), causing events tracked in the worker (MCP server status, engine events) to be lost when the worker exits. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
anandgupta42
added a commit
that referenced
this pull request
Mar 17, 2026
- Security FAQ: document well-known URL auth confirmation prompt and command validation (from #45) - Troubleshooting: add MCP server initialization failure section — init errors are now logged instead of silently swallowed (from #45) - Telemetry: document flush retry and flush-before-exit behavior (from #45, #46) - Getting started: mention postinstall welcome banner and upgrade changelog display (from #48) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Telemetry.shutdown()in the CLI entry point'sfinallyblock (index.ts) so buffered events are flushed beforeprocess.exit()— previously only the TUI session path called shutdown, causing all non-session commands (auth, upgrade, mcp, etc.) to silently lose their telemetry eventsshutdown()await the in-flightinit()promise (telemetry/index.ts) — sinceinit()is called fire-and-forget in CLI middleware,shutdown()could run before init completed, seeingenabled=falseand skipping the flush entirelyTest plan
test-telemetry-debug.tsthat events are accepted by App Insights (HTTP 200,itemsAccepted: 1)init()+ immediateshutdown()correctly flushes all buffered eventsshutdown()twice is safe (first flushes, second is no-op)🤖 Generated with Claude Code