Fix connection check prompting/disabling on a merely-stale session - #1864
Open
isc-klu wants to merge 24 commits into
Open
Fix connection check prompting/disabling on a merely-stale session#1864isc-klu wants to merge 24 commits into
isc-klu wants to merge 24 commits into
Conversation
…drop global checkingConnection - checkConnection -> ensureConnection: the function verifies AND interactively repairs a connection (may prompt for credentials, mutates persisted state), which "check" undersells. Added a doc comment. Renamed the misleading inActivate param to withTimeout (same boolean/behavior, clearer name). - AtelierAPI.request(): method is now a literal union instead of string, options is a documented inline type instead of any. Added a per-request checkingConnection option so request()'s 401/network-error self-heal guard is decided by the specific call, not ambient global state. - Removed the module-level checkingConnection boolean entirely. It served two unrelated purposes: a reentrancy guard for ensureConnection (now a connectionsBeingChecked Set<string> keyed by configName, which also fixes a bug where checking one workspace folder's connection silently no-opped checks for other folders) and the signal read by request() (now purely the per-request option, no ambient fallback).
AtelierAPI's retryAfter401 constructor flag (used only by ensureConnection, which always passed false) left wsOrFile unset, which made request()'s 401/network-error retry-and-self-heal guard always false for ensureConnection's own serverInfo() call - regardless of any other condition. So a merely-stale session (credentials fine, cookie expired) skipped straight to the manual credential prompt, and dismissing it disabled a working connection. Removed retryAfter401 entirely; wsOrFile is now always recorded. The scheduled follow-up ensureConnection calls this unlocks are safe: a redundant one just no-ops via the connectionsBeingChecked reentrancy guard if the original call is still in progress. This also let a per-request checkingConnection option (added earlier this session to work around the symptom) be removed again - the actual fix makes it unnecessary.
isc-klu
marked this pull request as ready for review
September 9, 2026 15:38
Contributor
|
Nice, looks good. Will wait to approve until issue reporter responds. |
isc-bsaviano
approved these changes
Sep 10, 2026
isc-bsaviano
left a comment
isc-bsaviano
left a comment
Contributor
There was a problem hiding this comment.
Upvoting so this can be merged after final confirmation.
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.
Fixes #1861
Root cause:
checkConnection's proberequests always passedretryAfter401=falsetoAtelierAPI. This is likely intended to prevent a recursive loop back intocheckConnection. But it also disablesrequest's unrelated single-retry self-heal, which is what should fix a merely-expired session without user involvement.Fix: Dropped the flag and moved all recursion-prevention logic to
checkConnection(nowensureConnection) itself with aconfigName-scopedSet(ensuringConnection) instead of the unscoped boolean.Cleanup that fell out of this:
checkConnectiontoensureConnection— it's not just a check, it has side effects (prompts, workspace state, etc.).inActivateparam towithTimeout, matching what the boolean actually controls.request()'s signature:method: string-> literal union;options: any-> a concrete type.checkingConnectionresets already covered by the trailing.finally(() => { ... }).