Skip to content

fix(cloudformation): Make CloudFormation LSP download strategy consistent#8750

Open
satyakigh wants to merge 1 commit into
aws:masterfrom
satyakigh:cfn-lsp-download
Open

fix(cloudformation): Make CloudFormation LSP download strategy consistent#8750
satyakigh wants to merge 1 commit into
aws:masterfrom
satyakigh:cfn-lsp-download

Conversation

@satyakigh

@satyakigh satyakigh commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

Summary

These changes make the CloudFormation LSP server download, installation, caching, and cleanup strategy consistent across all client plugins and safe for concurrent multi-IDE usage.

Previously each plugin had its own ad-hoc approach to downloading and managing the LSP binary. Now all clients share the same design:

  • Atomic installs — download to a PID-stamped temp directory, then rename into place
  • PID-based in-use tracking — marker files prevent one IDE from deleting a version another is actively using
  • File-based manifest caching — a plain manifest.json on disk replaces IDE-specific storage
  • Unified cache directory — all plugins write to the same OS-standard cache path
  • Stale temp cleanup — sweeps orphaned .tmp.<pid> directories from crashed processes
  • Local fallback resolution — when both network and cached manifest fail, scans installed versions for a usable binary

Motivation

Users commonly have both the standalone CloudFormation extension AND the AWS Toolkit installed in VS Code, or use both VS Code and JetBrains. Without a shared cache directory and in-use tracking, these plugins fight over the same LSP binary — downloading redundant copies, deleting each other's active versions, and corrupting installations mid-session.

The LSP download happens over the network from GitHub. In corporate environments with proxies, air-gapped networks, or intermittent connectivity, the download can fail at any point. The previous code had minimal resilience: no atomic writes (corruption on crash), no offline fallback (complete failure), and IDE-specific caching (no cross-plugin benefit).

extension.ts — Extension name hardcoded to "aws.toolkit.vscode". Removes VSCODE_EXTENSION_ID_CONSTANTS import.

githubManifestAdapter.ts — Massive simplification. Removes the entire GitHub Releases API fallback (fetchGitHubReleases, convertRelease, extractTargets, filterByEnvironment) and all associated interfaces (GitHubRelease, GitHubAsset). Now only fetches from the manifest JSON URL. Constructor simplified — no more repoOwner/repoName params. Stores lastRawManifest text so the installer can cache it to disk.

lspInstaller.ts — Major rewrite. File-based manifest cache: writes raw manifest to manifest.json via atomic tmp+rename, reads from cache on fetch failure. resolve() override adds findLocalFallback() which scans downloaded versions for a compatible server binary. postInstall() writes the in-use marker before cleanup runs. rootDir override via getCfnLspRootDir() points to the unified cross-plugin cache path.

lspServerProvider.tsdispose() now iterates matchedProviders and calls dispose() on each. Previously was a no-op, which meant in-use markers were never cleaned on extension deactivation.

remoteLspServerProvider.ts — Tracks versionDir from resolution result. dispose() removes the in-use marker.

baseLspInstaller.ts — Adds rootDir?: string to LspConfig interface and passes it through to LanguageServerResolver. This lets the CloudFormation installer override the default cache directory (shared with other LSPs like Amazon Q).

lspResolver.ts — New rootDir constructor parameter (defaults to LanguageServerResolver.defaultDir()). downloadRemoteTargetContent() uses atomic tmp+rename. getCachedVersions() filters .tmp. entries. getLatestCompatibleVersion() prefers the version with latest: true flag. defaultDownloadFolder() uses this.rootDir instead of the static default.

types.ts — Adds latest?: boolean field to LspVersion interface. This lets the manifest explicitly mark a preferred version rather than relying solely on semver sorting.

cleanup.ts — Complete rewrite. Removes isDelisted() helper and partition-based logic. New sweepStaleTmpDirs() removes dead-PID tmp dirs. Cleanup uses a keep-set {current, highestFallback} plus InUseTracker checks. The delisted-version concept is removed entirely.

inUseTracker.ts (new) — Node.js implementation of PID-based marker tracking. Same API as the other two codebases.

@satyakigh satyakigh requested a review from a team as a code owner April 20, 2026 16:20
@amazon-inspector-ohio

Copy link
Copy Markdown

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

@github-actions

Copy link
Copy Markdown
  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

@amazon-inspector-ohio

Copy link
Copy Markdown

✅ I finished the code review, and didn't find any security or code quality issues.

@satyakigh satyakigh force-pushed the cfn-lsp-download branch 4 times, most recently from a1555ab to 012554c Compare April 20, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants