Fix ServerSpec cache returning the wrong namespace - #420
Merged
Merged
Conversation
isc-klu
changed the title
isc-klu
changed the title
isc-klu
marked this pull request as ready for review
September 8, 2026 15:43
isc-bsaviano
reviewed
Sep 8, 2026
Auth/cookies are namespace-independent, so fragmenting the cache by namespace (as the previous fix did) loses intersystems#416's session-dedup benefit for the common single-server multi-namespace case. Instead, cache one entry per (username, host, port, pathPrefix) as before intersystems#420, and override the returned namespace to match the current call's rather than whatever was cached first.
isc-klu
marked this pull request as draft
September 9, 2026 18:18
isc-klu
marked this pull request as ready for review
September 9, 2026 18:20
isc-klu
changed the title
isc-bsaviano
reviewed
Sep 9, 2026
isc-bsaviano
reviewed
Sep 9, 2026
Two named servers can point at the same physical server; only the password-change handler should filter by serverName.
… own special case list() ignores serverName entirely (two named servers can point at the same physical server), so callers can pass a whole ServerSpec without over-matching. Deleting by serverName -- only needed on password change -- gets a dedicated method instead.
An unresolved BasicAuthorization reports username "", which used to fall through to "match any known connection to this address" (intersystems#416) but was being filtered as a literal "" and always missing. Also drop values() in favor of list({}).
isc-bsaviano
approved these changes
Sep 10, 2026
ricxJr
added a commit
to consistem/language-server
that referenced
this pull request
Sep 15, 2026
…ção no Marketplace (#16) * Fixes intersystems#395 * Fixes intersystems#396, Prepare 2.8.3 release * auto bump version with release [skip ci] * Update Actions * Create CONTRIBUTING.md * Add ESLint (intersystems#399) * Improve build configuration and add Prettier formatting (intersystems#401) * Extend "Go to Definition" to cover class member definitions. (intersystems#405) * Add issue templates and update CI * Fix intersystems#406: Prevent multiple server sessions at startup (intersystems#408) * Fixes intersystems#391 * OAuth2 Support (intersystems#407) * Fix incompatibilities with pre-OAuth2 version of Server Manager (intersystems#409) * Prepare 2.8.4 release * auto bump version with release [skip ci] * Fix hover intellisense for macros (intersystems#410) * Fix AxiosError on activation (intersystems#412) * Fix `ServerSpec | undefined` type safety and wire up TS project references (intersystems#414) * Fix folding range failure when there's blank lines in between the open curly brace for a method or class and the definition line (intersystems#417) * Activate faster when there are many workspace servers (intersystems#416) * Prepare 2.8.5 release (intersystems#418) * Bump version [skip ci] * Fix ServerSpec cache returning the wrong namespace (intersystems#420) * save * Keep one ServerSpec cache entry per connection, not per namespace Auth/cookies are namespace-independent, so fragmenting the cache by namespace (as the previous fix did) loses intersystems#416's session-dedup benefit for the common single-server multi-namespace case. Instead, cache one entry per (username, host, port, pathPrefix) as before intersystems#420, and override the returned namespace to match the current call's rather than whatever was cached first. * draft * Don't match on serverName when looking up a cached connection Two named servers can point at the same physical server; only the password-change handler should filter by serverName. * Match cached connections on connection fields only; serverName is its own special case list() ignores serverName entirely (two named servers can point at the same physical server), so callers can pass a whole ServerSpec without over-matching. Deleting by serverName -- only needed on password change -- gets a dedicated method instead. * Treat an empty username as unresolved in the connection cache lookup An unresolved BasicAuthorization reports username "", which used to fall through to "match any known connection to this address" (intersystems#416) but was being filtered as a literal "" and always missing. Also drop values() in favor of list({}). * Guard definition REST results against error responses without a result (intersystems#422) Fixes intersystems#421. Three sites in definition.ts dereferenced respdata.data.result.content after only checking respdata !== undefined; 4xx bodies passed through by makeRESTRequest have no result, so this threw. Guard them the same way the rest of the file already does. * chore(ci): remover publicação no Marketplace e Open VSX O fork não tem mais acesso ao VS Code Marketplace nem ao Open VSX, então os passos que publicavam o VSIX para esses registries só falhariam (ou seriam pulados silenciosamente por falta de token). - Removidos os passos `Publish to VS Code Marketplace` e `Publish to Open VSX Registry` do job `publish`, junto com o setup do Node e a instalação global do vsce, que só existiam para eles. - Removida a devDependency `ovsx`, sem uso após a remoção. O build dos 8 targets, o release beta a cada push no master e o anexo dos .vsix nas releases continuam funcionando — é por aí que o VSIX segue disponível para instalação interna. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Brett Saviano <bsaviano@intersystems.com> Co-authored-by: ProjectBot <bot@users.noreply.github.com> Co-authored-by: Kuang-Chen (KC) Lu <klu@intersystems.com> Co-authored-by: Claude Opus 5 (1M context) <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.
Fixes #419 and DP-455229
Root cause:
resolvedServerSpecs(#416) maps${username}@${host}:${port}${pathPrefix}(nonamespace) to a completeServerSpec(with namespace) — error-prone whenever multipleServerSpecs share a key but differ only by namespace. #419 is one instance: two workspace folders share a key but resolve to different namespaces, so the second folder's lookup silently returns the first folder's cached (wrong-namespace)ServerSpec.Solution: Add a wrapper class around the cache so callers can't read a
ServerSpecoff it directly. It exposeslist/add/delete/clear/values; cached values omitnamespaceentirely, so callers must supply the namespace they need — a stale one can no longer leak back out.Test plan: Two folders pointing at different namespaces on the same server; in the second, open a class that references another class in the same namespace — no spurious "class not found".