cleanup: remove StorageManager entirely

[shouples]

Summary of Changes

ℹ️ Click for background info

Back in the early days of this extension's development, I was annoyed that most extension examples showed the ExtensionContext being passed around to various functions in order to call things like globalState/workspaceState/secrets, so I put together the StorageManager singleton class as an easy way of working with extension state.

Fast forward a bit and we added some basic get/set logic for the ExtensionContext, which was then used in various places when we had to access the package.json info, static root, etc.

Eventually, we started noticing that StorageManager was missing some methods and event listeners, and it was lacking compared to interacting with the Memento (globalState/workspaceState) and SecretStorage (secrets) interfaces directly, so some parts of the codebase just started using getExtensionContext().___.

On top of that, we started adding the whole ResourceLoader layer to incorporate some caching. At this point we ended up with the following stack for managing storage for our primary resources:

- ResourceLoader (or a connection-specific subclass of it)
  - ResourceManager
    - StorageManager
      - Memento/SecretStorage calls

StorageManager was arguably the weakest layer, and only added to maintenance headaches and complicating the mental model.

This PR gets rid of the entire StorageManager layer we had between the extension and the VS Code Memento/SecretStorage calls.

If I had it the way I originally intended with this branch, it would've meant callers would use getExtensionContext() and work with the ExtensionContext's globalState, workspaceState, and secrets directly, but unfortunately they don't play nicely with sinon stubbing and throw TypeError: Descriptor for property [globalState|workspaceState|secrets] is non-configurable and non-writable errors.

So for a happy middle-ground, we have a few little convenience functions that lend themselves nicely to testing while also pointing directly at Memento/SecretStorage, with an arguably smaller mental model required for use:

vscode/src/storage/utils.ts

Lines 6 to 31 in d7ed387

	/**
	* Minimal wrapper around the {@link ExtensionContext}'s
	* {@linkcode ExtensionContext.globalState globalState} to allow stubbing {@link Memento} for tests.
	*/
	export function getGlobalState(): GlobalState {
	const context: ExtensionContext = getExtensionContext();
	return context.globalState;
	}
	/**
	* Minimal wrapper around the {@link ExtensionContext}'s
	* {@linkcode ExtensionContext.workspaceState workspaceState} to allow stubbing {@link Memento} for tests.
	*/
	export function getWorkspaceState(): WorkspaceState {
	const context: ExtensionContext = getExtensionContext();
	return context.workspaceState;
	}
	/**
	* Minimal wrapper around the {@link ExtensionContext}'s
	* {@linkcode ExtensionContext.secrets secrets} to allow stubbing {@link SecretStorage} for tests.
	*/
	export function getSecretStorage(): SecretStorage {
	const context: ExtensionContext = getExtensionContext();
	return context.secrets;
	}

However, there were a lot of places dealing with the StorageManager, so many of the other files edited here are updating imports and caller syntax.

Closes #1669.

Any additional details or context that should be provided?

• We had many places in the tests that were all handling their own StorageManager method stubbing, and I wanted to start pushing for some test-wide convenience functions to help those patterns and get us away from copying and pasting the same kind of stub setup code (as part of Create stubs.ts with helper functions to attach sinon stubs/stubbedInstances/spies/etc to sandboxes #508). As a result, tests/stubs/ was created with extensionStorage.ts to include a few helper functions to get stubbed "instances" of Memento/SecretStorage: getStubbedGlobalState, getStubbedWorkspaceState, and getStubbedSecretStorage. Each of these return fully-stubbed objects that can easily be adjusted per-test/-suite and should help reduce boilerplate. Example of working with it in a test (from storage/migrationManager.test.ts) where we previously had to set up two stubs while only altering the return value of one:

• Since we're working more directly with the Memento/SecretStorage interfaces, some parts of ResourceManager that were previously async calls no longer need to be async (due to StorageManager not quite being a 1:1 replacement, see background section above). This is mainly for the globalState.get/workspaceState.get calls, but the ResourceManager methods using them are left as async because changing them would've meant blowing this branch up even more.
  
  • To help find those, I added the require-await eslint rule, but then had to comment it out due to it flagging 150+ warnings and failing in CI:
    

Pull request checklist

Please check if your PR fulfills the following (if applicable):

Tests

• Added new
• Updated existing
• Deleted existing

Other

• All new disposables (event listeners, views, channels, etc.) collected as for eventual cleanup?

• Does anything in this PR need to be mentioned in the user-facing CHANGELOG or README?
• Have you validated this change locally by packaging and installing the extension .vsix file?

  gulp clicktest

[shouples]

This was the only method from StorageManager worth saving

[shouples]

Updated to stub the call made more closely to this portion of createSession():

vscode/src/authn/ccloudProvider.ts

Lines 188 to 192 in 602c956

	// update the auth status in the secret store so other workspaces can be notified of the change
	// and the middleware doesn't get an outdated status before the poller can update it
	await getResourceManager().setCCloudAuthStatus(
	authenticatedConnection.status.authentication.status,
	);

[shouples]

Memento doesn't have a dedicated delete method: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/227fe5bd71ef4cc4fdaea251ccbc8d9056c3c24e/types/vscode/index.d.ts#L8559-L8568

[shouples]

Starting to explicitly add void where we don't want to await/handle the promise here (as part of #1498)

[shouples]

Unrelated to this PR, but made it easier to update test assertions

[shouples]

This hasn't been used in months

[shouples]

This setup was getting a little too complicated; I figured it was best to split up into explicit tests for easier maintenance

[sonarqube-confluent]

Analysis Details

0 Issues

• 0 Bugs
• 0 Vulnerabilities
• 0 Code Smells

Coverage and Duplications

• 94.00% Coverage (61.60% Estimated after merge)
• No duplication information (1.10% Estimated after merge)

Project ID: vscode

View in SonarQube

[Copilot]

Pull Request Overview

This PR removes the StorageManager abstraction and shifts all storage interactions to lightweight helpers (getWorkspaceState, getGlobalState, getSecretStorage, etc.), simplifying and streamlining how state and secrets are accessed and tested.

• Replaced all StorageManager calls with direct state/secret helpers in code and tests
• Updated migrations and migrationManager to use typed enums and storage utils
• Refactored tests to stub the new helpers and removed singleton-based stubbing boilerplate

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/storage/resourceManager.test.ts	Switched from StorageManager to getWorkspaceState/clearWorkspaceState
src/storage/migrations/v2.ts	Replaced ExtensionContext.secrets with getSecretStorage()
src/storage/migrations/base.ts	Switched store-type strings to MigrationStorageType enum values
src/storage/migrationManager.ts	Removed StorageManager param; uses helpers for all storage ops
src/storage/migrationManager.test.ts	Refactored tests to stub new helpers; removed singleton manager stubs
src/storage/index.ts	Deleted StorageManager class entirely
src/storage/constants.ts	Introduced MigrationStorageType enum; cleaned up comments
src/sidecar/sidecarManager.ts	Replaced secret storage calls with getSecretStorage()
src/sidecar/middlewares.ts	Swapped getExtensionContext().secrets for getSecretStorage()
src/loaders/resourceLoader.test.ts	Replaced test activation stubs to use getTestExtensionContext()
src/flinkSql/languageClient.ts	Swapped getStorageManager().getSecret for getSecretStorage().get
src/extension.ts	Updated extension activation to use setupStorage() helpers
src/extension.test.ts	Removed StorageManager from activation tests
src/docker/configs.ts	Replaced StorageManager in Docker creds logic with getSecretStorage
src/directConnectManager.ts	Swapped secret-change listener to getSecretStorage().onDidChange
src/commands/diffs.ts	Replaced workspace-state calls with getWorkspaceState()
src/commands/diffs.test.ts	Updated tests to read/write from context.workspaceState
src/authn/ccloudProvider.ts	Replaced all secret calls with getSecretStorage()
src/authn/ccloudProvider.test.ts	Refactored tests to stub getSecretStorage() and clear state via helper
eslint.config.mjs	Commented out require-await rule

Comments suppressed due to low confidence (4)

src/storage/migrationManager.test.ts:11

• The tests reference CODEBASE_STORAGE_VERSION but it’s not imported, which causes runtime ReferenceErrors. Please add an import for CODEBASE_STORAGE_VERSION (e.g. import { CODEBASE_STORAGE_VERSION } from './migrationManager';).

import { DURABLE_STORAGE_VERSION_KEY, MigrationStorageType } from "./constants";

src/sidecar/sidecarManager.ts:548

• [nitpick] The variable existing_secret uses snake_case and should be renamed to existingSecret (or token, cachedToken) to follow the project's camelCase naming conventions.

const existing_secret: string | undefined = await getSecretStorage().get(

src/storage/utils.ts:1

• The new clearWorkspaceState helper isn't covered by unit tests. Adding dedicated tests (e.g. setting and then clearing keys) will ensure it behaves as expected.

export function clearWorkspaceState() {

eslint.config.mjs:36

• [nitpick] This commented-out ESLint rule adds noise. Consider either removing it or re-enabling require-await by addressing the flagged functions incrementally to enforce consistency.

// "require-await": "warn",

[jlrobins]

Thanks!