Add telemetry for durable session restore and session management - #757
Open
Yuandi (DDKinger) wants to merge 1 commit into
Open
Add telemetry for durable session restore and session management#757Yuandi (DDKinger) wants to merge 1 commit into
Yuandi (DDKinger) wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A couple of telemetry schema/details are inconsistent with runtime behavior (and one Terminal restore outcome can be misclassified as policy-blocked), which should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds end-to-end telemetry coverage for the durable-session “save → restart → restore → session/load” chain and improves session-management instrumentation so drop-offs and non-resumable branches are measurable across both Terminal (C++) and WTA (Rust), with the catalog updated accordingly.
Changes:
- Introduces new WTA events for
session/loadcompletion (AcpLoadSessionComplete), restore startup (AgentPaneResumeStarted), and abandoned resume flows (SessionResumeAbandoned), plus richerSessionResumeInvokedcoverage. - Adds new Terminal-side events to measure persisted agent state (
AgentLayoutSaved), restore outcomes (AgentPaneRestoreCompleted), deferred prewarm draining (AgentPanePrewarmAfterStartup), session binding changes, and shell-pane resume behavior. - Extends settings telemetry (
IntelligentFeatureConfigured) to includeFirstWindowPreferenceas a durable-session denominator and updatesTelemetryEvents.md.
File summaries
| File | Description |
|---|---|
| tools/wta/src/telemetry.rs | Adds new WTA telemetry events and expands session-resume instrumentation fields. |
| tools/wta/src/protocol/acp/mock_agent_tests.rs | Updates load-session test helpers for the new route field. |
| tools/wta/src/protocol/acp/client.rs | Introduces LoadSessionRoute, refines session/load failure bucketing, and emits AcpLoadSessionComplete. |
| tools/wta/src/helper/runtime.rs | Stamps boot-time restore params and emits AgentPaneResumeStarted during helper startup. |
| tools/wta/src/helper/config.rs | Adds stable string labels for InitialView to support telemetry/logging. |
| tools/wta/src/app.rs | Adds abandoned-resume reporting and ensures SessionResumeInvoked covers all decision branches. |
| tools/wta/src/app_events.rs | Wires route through WT→WTA events into LoadSessionForTab. |
| TelemetryEvents.md | Documents new/updated events and their controlled field values and privacy boundaries. |
| src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp | Reports FirstWindowPreference via existing intelligent-feature telemetry helper. |
| src/cascadia/TerminalApp/TerminalPage.h | Adds _AgentLayoutCounts and returns counts from _StampAgentResumeCommandlines. |
| src/cascadia/TerminalApp/TerminalPage.cpp | Implements multiple new Terminal-side telemetry events for restore/prewarm/layout/bind/resume. |
| src/cascadia/TerminalApp/TabManagement.cpp | Returns agent-layout counts while stamping resume command lines for persisted layouts. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1560
to
+1570
| const auto agentId = tab->HasAgentOverride() ? tab->AgentIdOverride() : winrt::hstring{}; | ||
| const auto isCustomAgent = _IsCustomAgentId(agentId); | ||
| auto blockedByPolicy = false; | ||
| if (!agentId.empty() && !isCustomAgent) | ||
| { | ||
| namespace Registry = ::Microsoft::Terminal::Settings::Model::AgentRegistry; | ||
| const auto allowed = Registry::FilteredAcpAgents(); | ||
| blockedByPolicy = std::none_of(allowed.begin(), allowed.end(), [&](const auto& agent) { | ||
| return agent.id == std::wstring_view{ agentId }; | ||
| }); | ||
| } |
Comment on lines
+150
to
+154
| `SessionResumeAbandoned.Reason` names the lifecycle event that ended the | ||
| wait — `AgentRebind`, `NewSession`, `ResetAgent`, `MasterRestart`, or | ||
| `PaneClosed` — rather than collapsing them into one generic failure, since | ||
| they are indistinguishable to the user: the pane says "Resuming session …" | ||
| and then comes back as a cold start. |
Comment on lines
+1125
to
+1132
| crate::telemetry::log_agent_pane_resume_started( | ||
| &config.agent_id.clone().unwrap_or_else(|| { | ||
| crate::agent_registry::resolve_agent_id_from_cmd(&config.agent) | ||
| .to_string() | ||
| }), | ||
| config.initial_view.label(), | ||
| config.start_stashed, | ||
| ); |
Comment on lines
+356
to
+360
| /// Every one of these leaves the user looking at a pane that said | ||
| /// "Resuming session …" and then silently became a cold start, so the | ||
| /// `reason` set is deliberately one value per distinct lifecycle event | ||
| /// rather than a single generic failure: `AgentRebind`, `NewSession`, | ||
| /// `ResetAgent`, `MasterRestart`, or `PaneClosed`. |
The intelligent-features telemetry catalog covers prompts, turns, cold starts, slash commands, MCP calls and hooks, but the durable-session chain is uninstrumented end to end, and the session view reports only two of its four outcomes. This fills both gaps in the same style. The resume funnel is now measurable at every hop: AgentLayoutSaved counts what a save is about to write, AgentPaneRestoreCompleted counts what the next launch rebuilds, AgentPaneResumeStarted counts the helpers that actually received the request, and AcpLoadSessionComplete is the outcome. AcpLoadSessionComplete splits ResourceNotFound out of AcpError. That is the error behind the user-visible "Failed to resume session ... Resource not found", and folded into a generic bucket it would be invisible. Classification goes through AgentFailure::from_acp_error so it tracks ACP error codes rather than message text. SessionResumeInvoked now covers Focus and NotResumable as well. The four NotResumableReason values are the dead ends a user actually hits, so reporting only the two routes that resume measured the happy path alone. Also added: - SessionResumeAbandoned, one reason per lifecycle event, for the panes that show "Resuming session ..." and quietly become a cold start. - AgentSessionBindingChanged: a shell pane that never binds can never persist a resume command line, and that failure was silent. - AgentShellPaneResumed and AgentPanePrewarmAfterStartup. - FirstWindowPreference on IntelligentFeatureConfigured. The whole feature is inert unless it is set to a restore value, so without it there is no denominator to read restore rates against. No session id, command line, working directory, or session title is reported. Agent names go through the same controlled set used elsewhere, including the hook-reported ones, which arrive from outside Terminal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2b25dec1-4de7-48fc-9854-9eb050a2428b
Yuandi (DDKinger)
force-pushed
the
dev/yuazha/telemetry-durable-session
branch
from
September 2, 2026 03:21
3961540 to
d7fd758
Compare
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
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
The intelligent-features telemetry catalog covers prompts, turns, cold starts, slash commands, MCP calls and hooks — but the durable-session chain (save → restart → restore → ACP
session/load) has no instrumentation on any hop, and the session view reports only two of its four outcomes. This fills both gaps, following the conventions already established inTelemetryEvents.md: same providers, sameMICROSOFT_KEYWORD_MEASURES, same privacy tags, same controlled agent set, catalog updated alongside.The resume funnel
Four events, one per hop, so a drop-off can be attributed rather than guessed at:
AgentLayoutSavedAgentPaneRestoreCompletedAgentPaneResumeStartedAcpLoadSessionCompleteThe gap between
AgentPaneRestoreCompletedandAgentPaneResumeStartedis the C++ → WTA handoff loss, which neither side can see alone.AcpLoadSessionCompletesplitsResourceNotFoundout ofAcpError. That is the error behind the user-visible "Failed to resume session … Resource not found" — the failure a stale persisted session id produces. Folded into a generic bucket it is invisible. Classification goes through the existingAgentFailure::from_acp_error, so it tracks ACP error codes rather than message text, andRoute(Restore/SessionView) separates a layout restore from a session-view resume — the two have very different latency profiles.Session management
SessionResumeInvokedcoveredResumeInAgentPaneandResumeCliFlagand explicitly skipped the other two branches, which measured the happy path alone. It now covers all four, and carriesNotResumableReason—LiveWithoutPane,LoadSessionNotSupported,CliHasNoResumeFlag,UnknownCli. Those four are exactly the dead ends a user runs into, anddecide_enter_actionis a closed pure function, so the set cannot drift.Reporting stays on the dispatch side; the
session_mgmtdecision boundary is untouched.Also added
SessionResumeAbandoned— oneReasonper lifecycle event (AgentRebind,NewSession,ResetAgent,MasterRestart,PaneClosed) rather than a generic failure, since they are indistinguishable to the user: the pane says "Resuming session …" and then quietly comes back as a cold start. Gated onloading_session, so the resets that run with no resume pending report nothing.AgentSessionBindingChanged— a shell pane that never binds an agent session can never persist a resume command line, so this is the upstream gate on every shell-pane restore, and it was failing silently.AgentShellPaneResumed— carriesBufferRestoreSuppressed, which must always be true; a resumed pane''s scrollback is deliberately not seeded because the CLI replays its own transcript.AgentPanePrewarmAfterStartup—Deferredmust equalPrewarmed + Skipped + failures; a mismatch is the "tab ends up with two agent panes, or none" bug class.FirstWindowPreferenceonIntelligentFeatureConfigured— the whole feature rides the saved window layout and is inert unless this ispersistedLayoutorpersistedLayoutAndContent. Without it there is no denominator to read restore rates against. Uses the existingemitIntelligentFeatureConfiguredhelper.AgentLayoutSavedis suppressed when a window has no agent state. Every save path funnels throughGetWindowLayout, including the five-minute crash-protection timer, so an unconditional event would mostly be windows that have never opened an agent pane.Privacy
No session id, command line, working directory, session title, or prompt text is reported; all new C++ fields are counts and closed enums. Agent names go through the same controlled set as the rest — including the hook-reported ones at
AgentSessionBindingChanged, which arrive from outside Terminal and are bucketed tocustom(orunknownwhen absent) rather than reported verbatim.Non-goals
LoadSessionRouteis telemetry-only and never used for routing; the restore and session-view paths stay deliberately identical past the point it is recorded. The GPO check behindAgentPaneRestoreCompleted.Result = BlockedByPolicyonly observes the decision — the gate that enforces it remains in_AutoCreateHiddenAgentPaneShared.Validation
cargo test --target x86_64-pc-windows-msvc --manifest-path tools/wta/Cargo.toml→ 1872 passed, 0 failed, 1 ignored, including the newload_session_failure_fields_separates_resource_not_found, which pins each ACP error code to its bucket.cargo fmtclean;cargo clippy --all-targetssurfaces nothing new (the oneunused import: CliSourcewarning is pre-existing onmainin an untouched file).cmd.exe /c "tools\razzle.cmd && bcz no_clean"→ 0 errors, re-verified after clang-format.