sessions: make navigation history size observable#315391
Merged
Merged
Conversation
- Add _historySize observable so _canGoForward derived recomputes when history is mutated without changing _currentIndex (e.g. session removal) - Update _historySize in all mutation paths: _pushEntry, _navigateTo, _removeEntriesMatching - Fix misleading 'remove and try again' comment in _navigateTo — the code only removes the missing entry without retrying Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR follows up on the Agents window session navigation history implementation by making the history size observable (so canGoForward updates correctly when the backing array changes) and by correcting an inaccurate comment in the missing-session navigation path.
Changes:
- Introduces
_historySizeas an observable and uses it in the_canGoForwardderived. - Updates all history mutation paths to keep
_historySizein sync. - Adjusts a comment in
_navigateToto match the actual behavior when a session no longer exists.
Show a summary per file
| File | Description |
|---|---|
src/vs/sessions/services/sessions/browser/sessionNavigation.ts |
Adds an observable for history length to keep forward-navigation context state correct and updates comments around missing-session handling. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/vs/sessions/services/sessions/browser/sessionNavigation.ts:220
_removeEntriesMatchingcan set_currentIndexto0even when all history entries are removed (becausenewCurrentIdxcan become-1but is clamped viaMath.max(0, ...)). This makes the index point to a non-existent entry and can incorrectly keep Back enabled if_beyondHistoryis true. Handle the empty-history case explicitly (e.g. set_currentIndexto-1and clear_beyondHistorywhenthis._history.length === 0).
if (newCurrentIdx !== currentIdx) {
this._currentIndex.set(Math.max(0, newCurrentIdx), undefined);
}
this._historySize.set(this._history.length, undefined);
}
- Files reviewed: 1/1 changed files
- Comments generated: 1
Contributor
blocks-ci screenshots changedReplace the contents of Updated blocks-ci-screenshots.md<!-- auto-generated by CI — do not edit manually -->
#### editor/codeEditor/CodeEditor/Dark

#### editor/codeEditor/CodeEditor/Light

#### editor/inlineChatZoneWidget/InlineChatZoneWidget/Dark

#### editor/inlineChatZoneWidget/InlineChatZoneWidget/Light

#### editor/inlineChatZoneWidget/InlineChatZoneWidgetTerminated/Dark

#### editor/inlineChatZoneWidget/InlineChatZoneWidgetTerminated/Light
 |
dmitrivMS
approved these changes
May 8, 2026
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.
Follow-up to #315382.
Addresses two review comments from Copilot:
_history.lengthis not observable —_canGoForwardderived readthis._history.length(a plain array), so mutations that don't change_currentIndex(e.g. session removal at a position after the current index) could leave the derived stale. Fix: add a_historySizeobservable, update it in all mutation paths (_pushEntry,_navigateTo,_removeEntriesMatching), and read it in the_canGoForwardderived.Misleading comment —
_navigateTosaid "remove it and try again" but the code only removes the missing entry without retrying. Updated the comment to match the actual behavior.