Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
1a2c0f5
Add shared package release dispatch workflow and stop Dependabot dupl…
edvilme Jun 30, 2026
e488b40
Use sed + uv pip compile for the pip dependency update
edvilme Jun 30, 2026
cc3220e
Harden shared package release workflow from review feedback
edvilme Jun 30, 2026
d1f2a44
Experiment: integrate shared package as a git submodule (#696)
edvilme Jul 1, 2026
2292581
Simplify and harden shared package submodule sync workflow
edvilme Jul 1, 2026
35003ad
Address review feedback on submodule sync and install
edvilme Jul 1, 2026
393f13d
Harden submodule sync workflow and clean dependency metadata
edvilme Jul 2, 2026
def29cc
Verify extension/shared-package Python floors in submodule sync
edvilme Jul 2, 2026
4f5a1d6
Split submodule sync into discrete, readable steps
edvilme Jul 2, 2026
7e44824
Document submodule initialization in the README
edvilme Jul 2, 2026
dae53d4
Verify extension/shared-package Node versions in submodule sync
edvilme Jul 2, 2026
6022f97
Harden shared-package submodule sync workflow
edvilme Jul 2, 2026
c7e5006
Update shared package submodule to v0.8.1
edvilme Jul 3, 2026
ca39d20
Probe bare release tags symmetrically in submodule sync
edvilme Jul 21, 2026
bcd212a
Parse only a >=/~= lower bound from requires-python
edvilme Jul 21, 2026
4de51fc
Validate the computed branch ref before emitting it
edvilme Jul 21, 2026
1ba3691
Harden tracking-issue lookup in submodule sync
edvilme Jul 21, 2026
40bd139
Guard shared-package postinstall build
edvilme Jul 21, 2026
68b2c64
Document dead shared-package Dependabot ignores as no-ops
edvilme Jul 21, 2026
9ef3e7c
Fix 7: re-notify maintainers on re-dispatch when the sync branch alre…
edvilme Jul 21, 2026
9483820
Fix 8: resolve symbolic minimumPythonVersion constants in the Python …
edvilme Jul 21, 2026
b26364c
Fix 9: fail fast with guidance when the shared-package submodule is m…
edvilme Jul 21, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ updates:
npm-minor-patch:
patterns:
- '*'
# Exclude @vscode/common-python-lsp so it gets its own standalone PR (weekly).
exclude-patterns:
- '@vscode/common-python-lsp'
update-types:
- 'minor'
- 'patch'
open-pull-requests-limit: 10
ignore:
# @vscode/common-python-lsp is handled by the shared-package-submodule-sync
# dispatch workflow, so ignore it here to avoid duplicate PRs.
- dependency-name: '@vscode/common-python-lsp'
- dependency-name: '@types/vscode'
- dependency-name: '@types/node'
- dependency-name: 'vscode-languageclient'
Expand All @@ -39,12 +39,13 @@ updates:
pip-minor-patch:
patterns:
- '*'
# Exclude vscode-common-python-lsp so it gets its own standalone PR (weekly).
exclude-patterns:
- 'vscode-common-python-lsp'
update-types:
- 'minor'
- 'patch'
ignore:
# vscode-common-python-lsp is handled by the shared-package-submodule-sync
# dispatch workflow, so ignore it here to avoid duplicate PRs.
- dependency-name: 'vscode-common-python-lsp'

# Python test dependencies are updated weekly, minor updates are grouped (1 PR "pip-test-minor-patch").
- package-ecosystem: 'pip'
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/pr-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v6.0.2
with:
submodules: recursive

- name: Build VSIX
uses: ./.github/actions/build-vsix
Expand All @@ -31,6 +33,8 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v6.0.2
with:
submodules: recursive

- name: Lint
uses: ./.github/actions/lint
Expand All @@ -56,6 +60,10 @@ jobs:
with:
path: ${{ env.special-working-directory-relative }}

- name: Checkout submodules

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 .github/workflows/pr-check.yml:60
The Python-floor and Node-floor compatibility gates live only in the dispatch workflow (shared-package-submodule-sync.yml). A maintainer who bumps external/vscode-common-python-lsp by hand in a PR bypasses both floors, and CI stays green even if the extension now claims support for an older Python than the shared lib requires. Mirror the floor checks as a shared step in pr-check.yml/push-check.yml.

[verified]

run: git submodule update --init --recursive
shell: bash

# Install bundled libs using env.PYTHON_VERSION even though you test it on other versions.
- name: Use Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@v6
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/push-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v6.0.2
with:
submodules: recursive

- name: Build VSIX
uses: ./.github/actions/build-vsix
Expand All @@ -38,6 +40,8 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v6.0.2
with:
submodules: recursive

- name: Lint
uses: ./.github/actions/lint
Expand All @@ -64,6 +68,10 @@ jobs:
with:
path: ${{ env.special-working-directory-relative }}

- name: Checkout submodules
run: git submodule update --init --recursive
shell: bash

# Install bundled libs using env.PYTHON_VERSION even though you test it on other versions.
- name: Use Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@v6
Expand Down
157 changes: 157 additions & 0 deletions .github/workflows/shared-package-submodule-sync.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
name: Shared Package Submodule Sync
Comment thread
edvilme marked this conversation as resolved.

on:
repository_dispatch:
types: [shared-package-release]

permissions:
contents: write
issues: write

env:
SUBMODULE_PATH: external/vscode-common-python-lsp

jobs:
prepare:
name: Validate and normalize the release
runs-on: ubuntu-latest
outputs:
version: ${{ steps.normalize.outputs.version }}
branch: ${{ steps.normalize.outputs.branch }}
steps:
- name: Normalize release tag
id: normalize
env:
RELEASE_TAG: ${{ github.event.client_payload.release_tag }}
run: |
set -euo pipefail
# Require a real dotted release so a branch/ref name can never be malformed.
if ! printf '%s' "${RELEASE_TAG}" | grep -Eq '^v?[0-9]+\.[0-9]+\.[0-9]+([-+][0-9A-Za-z.]+)*$'; then
echo "::error::release_tag '${RELEASE_TAG:-<empty>}' is missing or not a valid semver release."
exit 1
fi
VERSION="${RELEASE_TAG#v}"
{
echo "version=${VERSION}"
echo "branch=shared-package-v${VERSION}"
} >>"$GITHUB_OUTPUT"

sync:
Comment thread
edvilme marked this conversation as resolved.
name: Update shared package submodule
needs: prepare
runs-on: ubuntu-latest
# Key on the normalized branch so `v1.2.3` and `1.2.3` share one group.
concurrency:
group: shared-package-submodule-${{ needs.prepare.outputs.branch }}
cancel-in-progress: false
env:
VERSION: ${{ needs.prepare.outputs.version }}
BRANCH: ${{ needs.prepare.outputs.branch }}
RELEASE_TAG: ${{ github.event.client_payload.release_tag }}
RELEASE_URL: ${{ github.event.client_payload.release_url }}
REPO: ${{ github.repository }}
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
steps:
- name: Checkout
uses: actions/checkout@v4
with:
submodules: recursive

- name: Set up Node
uses: actions/setup-node@v4
with:
node-version-file: .nvmrc

- name: Update submodule and open tracking issue
run: |
set -euo pipefail

# Retry wrapper for flaky network operations (fetch/push).
retry() {
local attempt=1
until "$@"; do
if [ "${attempt}" -ge 3 ]; then
echo "::error::Command failed after ${attempt} attempts: $*"
return 1
fi
echo "Attempt ${attempt} failed: $* — retrying..." >&2
attempt=$((attempt + 1))
sleep $((attempt * 5))
done
}

# 1. Never clobber an existing release branch: once it exists a
# maintainer may be editing it, so this workflow leaves it alone.
if git ls-remote --exit-code --heads origin "refs/heads/${BRANCH}" >/dev/null 2>&1; then
echo "Branch ${BRANCH} already exists; leaving it untouched."
exit 0
fi

# 2. Build the release branch from the latest main.
retry git fetch origin main
git checkout -B "${BRANCH}" FETCH_HEAD
git submodule update --init --recursive "${SUBMODULE_PATH}"

# 3. Move the submodule to the released commit (fail if the tag is unknown).
(
cd "${SUBMODULE_PATH}"
retry git fetch --tags --force origin
TARGET=''
for CANDIDATE in "refs/tags/${RELEASE_TAG}" "refs/tags/v${VERSION}"; do
if git rev-parse -q --verify "${CANDIDATE}^{commit}" >/dev/null; then
TARGET="${CANDIDATE}"
break
fi
done
if [ -z "${TARGET}" ]; then
echo "::error::Release tag '${RELEASE_TAG}' was not found in the shared package repository."
exit 1
fi
echo "Checking out submodule at ${TARGET}."
git checkout --detach "${TARGET}"
)

# 4. Stop early if the submodule pointer did not move.
if git diff --quiet -- "${SUBMODULE_PATH}"; then
echo "Submodule already at ${RELEASE_TAG}; nothing to do."
exit 0
fi

# 5. Refresh the lockfile so the branch stays `npm ci`-mergeable, then
# commit both the submodule pointer and the regenerated lockfile.
npm install --package-lock-only --ignore-scripts
git config user.name 'github-actions[bot]'
git config user.email 'github-actions[bot]@users.noreply.github.com'
git add "${SUBMODULE_PATH}" package-lock.json
git commit -m "Update shared package submodule to ${RELEASE_TAG}"
# New branch only (guarded above), so a plain push cannot overwrite work.
retry git push --set-upstream origin "${BRANCH}"

# 6. Point a maintainer at the compare page via the job summary and a tracking issue.
COMPARE_URL="https://github.com/${REPO}/compare/main...${BRANCH}?expand=1"
TITLE="[Shared Package] Open PR to update submodule to ${RELEASE_TAG}"
BODY_FILE="$(mktemp)"
{
echo "### Shared package submodule update ready"
echo ''
echo "Branch \`${BRANCH}\` has been pushed, moving \`${SUBMODULE_PATH}\` to ${RELEASE_TAG}."
echo ''

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 .github/workflows/shared-package-submodule-sync.yml:150
The Skeptic notes the branch is pushed and a [Shared Package] Open PR… issue is filed claiming the update is "ready," but the lockfile refresh uses --ignore-scripts, so the shared TS package is never compiled and bundled/libs is never assembled in this job. A non-building submodule commit (or one adding an undeclared Python runtime dep) is reported as ready and only surfaces as broken when a human opens the PR. Consider running npm ci + build (and nox -s install_bundled_libs) on the branch before declaring it ready, or word the issue as "needs validation."

[verified]

echo 'Organization settings prevent this workflow from opening pull requests automatically. Please open it manually:'
echo ''
echo "[Open the pull request](${COMPARE_URL})"
echo ''
echo "Source release: ${RELEASE_URL:-n/a}"
} | tee -a "$GITHUB_STEP_SUMMARY" >"$BODY_FILE"

# Reuse an existing open tracking issue for this release if present.
EXISTING="$(gh issue list --repo "$REPO" --state open \
--search "in:title ${TITLE}" --json number,title \
--jq "map(select(.title == \"${TITLE}\")) | .[0].number // empty" 2>/dev/null || true)"
if [ -n "${EXISTING}" ]; then
echo "Reusing existing tracking issue #${EXISTING}."
gh issue comment "${EXISTING}" --repo "$REPO" --body-file "$BODY_FILE" \
|| echo 'Could not comment on the tracking issue; the compare URL is in the job summary.'
else
gh issue create --repo "$REPO" --title "${TITLE}" --body-file "$BODY_FILE" \
|| echo 'Could not create the tracking issue; the compare URL is in the job summary.'
fi
Comment thread
edvilme marked this conversation as resolved.
Comment thread
edvilme marked this conversation as resolved.
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[submodule "external/vscode-common-python-lsp"]
path = external/vscode-common-python-lsp
url = https://github.com/microsoft/vscode-common-python-lsp.git
1 change: 1 addition & 0 deletions .vscodeignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
.vscode-test/**
out/**
node_modules/**
external/**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 .vscodeignore:5
The Skeptic notes .vscodeignore external/** + the file: link is correct only because webpack bundles the built shared JS into dist/extension.js before packaging. This silently depends on postinstall having produced that build first; if postinstall is skipped (--ignore-scripts) before npm run package, webpack resolves an unbuilt main and the VSIX can ship broken with no error. Consider a guard/verification that the shared package built before packaging.

[verified]

src/**
.gitignore
.yarnrc
Expand Down
2 changes: 2 additions & 0 deletions build/azure-devdiv-pipeline.pre-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ parameters:
- name: buildSteps
type: stepList
default:
- script: git submodule update --init --recursive
displayName: Checkout submodules
- script: npm ci
displayName: Install NPM dependencies

Expand Down
2 changes: 2 additions & 0 deletions build/azure-devdiv-pipeline.stable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ parameters:
- name: buildSteps
type: stepList
default:
- script: git submodule update --init --recursive
displayName: Checkout submodules
- script: npm ci
displayName: Install NPM dependencies

Expand Down
2 changes: 2 additions & 0 deletions build/azure-pipeline.pre-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ extends:
architecture: 'x64'
displayName: Select Python version

- script: git submodule update --init --recursive
displayName: Checkout submodules
- script: npm ci
displayName: Install NPM dependencies

Expand Down
2 changes: 2 additions & 0 deletions build/azure-pipeline.stable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ extends:
architecture: 'x64'
displayName: Select Python version

- script: git submodule update --init --recursive
displayName: Checkout submodules
- script: npm ci
displayName: Install NPM dependencies

Expand Down
2 changes: 2 additions & 0 deletions build/azure-pipeline.validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ parameters:
- name: buildSteps
type: stepList
default:
- script: git submodule update --init --recursive
displayName: Checkout submodules
- script: npm ci
displayName: Install NPM dependencies

Expand Down
19 changes: 19 additions & 0 deletions build/postinstall.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Builds the shared package that lives in the git submodule after install.
//
// The build is skipped (with guidance) when the submodule has not been
// checked out, so a clone made without `--recurse-submodules` does not fail
// `npm install`/`npm ci` at the postinstall step with a confusing error.
const { existsSync } = require("fs");
const { execSync } = require("child_process");

const pkgDir = "external/vscode-common-python-lsp/typescript";

if (!existsSync(`${pkgDir}/package.json`)) {
console.warn(
`[postinstall] Shared package submodule not found at "${pkgDir}". ` +
"Run `git submodule update --init --recursive` and reinstall to build it.",
);
process.exit(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 build/postinstall.js:11
A clean-clone probe verified that exiting successfully without the submodule lets npm ci pass while the subsequent compile fails to resolve @vscode/common-python-lsp. Fail immediately with the initialization command or initialize the submodule automatically instead of leaving a success-shaped unusable install.

[verified]

}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 build/postinstall.js:11
The missing-submodule guard is largely dead code: npm resolves the file:external/vscode-common-python-lsp/typescript dependency (package.json:212) during install/link, before the root postinstall lifecycle runs. On a clone without --recurse-submodules npm aborts earlier with a cryptic ENOENT/link:true failure, so this friendly warn-and-exit(0) branch is rarely reached. Either drop/repurpose it, or bootstrap the submodule before the file: link resolves; at minimum don't advertise a graceful path npm preempts.

[verified]


Comment thread
edvilme marked this conversation as resolved.
execSync(`npm --prefix ${pkgDir} run build`, { stdio: "inherit" });
Comment thread
edvilme marked this conversation as resolved.
1 change: 1 addition & 0 deletions external/vscode-common-python-lsp
10 changes: 10 additions & 0 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,16 @@ def install_bundled_libs(session):
"-r",
"./requirements.txt",
)
# Source the shared Python library from the git submodule instead of the
# published package so the bundled copy matches the pinned submodule commit.
session.install(
"-t",
"./bundled/libs",
"--no-cache-dir",
"--no-deps",
"--upgrade",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 noxfile.py:99
Partially-addressed prior High. session.install(..., '--no-deps', ...) makes requirements.txt responsible for the submodule's Python runtime deps, and while pygls/packaging/lsprotocol are now documented and pinned in requirements.in, nothing enforces the subset invariant. A future submodule bump that adds a Python runtime dep would ship a bundled/libs missing it and fail at import (fails closed, but latent). Add a sync-time assertion comparing the submodule's pyproject runtime deps against requirements.txt, or drop --no-deps.

[verified]

"./external/vscode-common-python-lsp/python",
)
Comment thread
edvilme marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 noxfile.py:99
The Advocate flagged that _update_npm_packages rewrites every non-pinned dependency to the latest ^-version from the npm registry, and @vscode/common-python-lsp is not in the pinned set — so running nox -s update_packages would overwrite the file:external/... link back to a published ^0.x version, silently undoing this PR's core mechanism. Add @vscode/common-python-lsp to the pinned set so the submodule wiring is protected.

[verified]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this installs the shared Python project with --no-deps, every runtime dependency must remain manually mirrored in requirements.txt. A later shared-package release that adds a dependency can produce a bundled library that fails at import time. Add a dependency-contract check during sync, or install the local project with its dependencies.



@nox.session()
Expand Down
Loading
Loading