Adopt vscode-common-python-lsp as a git submodule, synced via repository_dispatch, and stop Dependabot duplicates

[Eduardo Villalpando Mello (edvilme)]

Summary

Consumes the shared LSP package (vscode-common-python-lsp) as a git submodule pinned to a release tag, instead of the published npm/PyPI packages, and keeps that submodule updated via a repository_dispatch event from vscode-common-python-lsp rather than Dependabot.

1. Git submodule

• Adds external/vscode-common-python-lsp (.gitmodules) pinned to a shared-package release tag.
• TypeScript: package.json sources it via "@vscode/common-python-lsp": "file:external/vscode-common-python-lsp/typescript"; build/postinstall.js builds the submodule's TS output on install (skips when already built or when the submodule toolchain is absent).
• Python: noxfile.py installs ./external/vscode-common-python-lsp/python into ./bundled/libs with --no-deps --upgrade, so the bundled copy matches the pinned submodule commit.

2. New .github/workflows/shared-package-submodule-sync.yml

Handles the shared-package-release repository_dispatch: branches off main, advances the submodule pointer to the released tag, refreshes the npm lockfile, verifies the pinned tag and the Python requires-python floor, then pushes the branch and opens a tracking issue with a manual compare/PR link (org settings prevent the workflow from opening PRs automatically).

3. .github/dependabot.yml

Moves the shared package into the npm and pip ignore lists so Dependabot no longer opens duplicate update PRs for it.

[Rich Chiodo (rchiodo)]

The dispatch workflow is a reasonable approach, but the dependency-pinning step needs to be made precise and verifiable before merge: scope the pip upgrade to the shared package and assert the pin actually changed. A few retry/idempotency and reproducibility hardening notes are also worth addressing.

[Eduardo Villalpando Mello (edvilme)]

Thanks Rich Chiodo (@rchiodo) — addressed the feedback in the latest commit:

• Python version / reproducibility: the workflow now pins PYTHON_VERSION to the version documented in this repo's requirements.in and passes --python-version to uv pip compile, so environment markers (e.g. exceptiongroup) aren't dropped when the lockfile is regenerated.
• Blanket upgrade: replaced --upgrade with --upgrade-package "${PIP_DEP}" so only the shared package (and its forced transitives) move.
• Silent sed no-op: the pin rewrite is now case-insensitive and is followed by a grep assertion that fails the job if requirements.in wasn't actually re-pinned.
• Payload validation: a new step fails fast if release_tag is empty or not version/ref-shaped (^v?[0-9A-Za-z][0-9A-Za-z.+-]*$), preventing degenerate branches and npm install pkg@ resolving to latest.
• Idempotency / re-runs: added a concurrency group keyed on release_tag, switched the push to --force-with-lease (fetching any existing remote branch first), de-duplicated the tracking issue (reuse/comment on an existing open one), and made issue creation tolerant of Issues being disabled by also writing the compare URL to the job summary.

[Rich Chiodo (rchiodo)]

The submodule-based sync approach is reasonable, but the new workflow has two integrity/safety issues worth fixing before merge: (1) the bare --force-with-lease after re-fetching the remote branch tip provides no real protection and can overwrite maintainer commits, and (2) the tag-resolution step silently falls back to origin/main while still committing/labeling as the release tag. Note: the first-pass candidates referencing .github/workflows/shared-package-release.yml were dropped because that file is not part of this PR's diff (the added workflow is shared-package-submodule-sync.yml).

[Rich Chiodo (rchiodo)]

Overall this is a solid, well-guarded change. A couple of edge-case reliability notes on the sync workflow (unreachable warn-branch in the Node compat check, and a permanently-dropped tracking-issue notification on the branch-exists path) are worth addressing, plus two architectural notes about the file: link coupling and the manual --no-deps contract. None are blocking.

[Rich Chiodo (rchiodo)]

Approved via Review Center.

[Rich Chiodo (rchiodo)]

🔒 Automated review in progress — Rich Chiodo (@rchiodo) is auto-reviewing this PR.

[Eduardo Villalpando Mello (edvilme)]

Review fixes applied (6 commits)

Pushed 6 identical commits across all 5 consumer PRs (one per fix). The workflow/postinstall.js/dependabot.yml changes are byte-identical in every repo:

1. sync.yml tag resolution — added refs/tags/${VERSION} (bare) as a third candidate so v-prefixed and bare release tags both resolve symmetrically.
2. sync.yml requires-python floor — rewrote the parse to extract the quoted value and read only the >=/~= lower bound (empty → warn-skip) instead of greedily grabbing the first major.minor.
3. sync.yml ref hardening — added git check-ref-format "refs/heads/${BRANCH}" to reject multiline / .. injection before emitting outputs.
4. sync.yml tracking-issue step — quoted the in:title search and stopped swallowing gh failures (removed 2>/dev/null || true) so errors surface under set -e.
5. postinstall.js — skip the build when dist/index.js already exists, and skip-with-guidance (instead of hard-failing) when the submodule TS toolchain is absent (dev-pruned installs).
6. dependabot.yml — annotated the two now-inert shared-package ignore entries as intentional defensive no-ops.

Deferred (need human/CI judgement — left unresolved intentionally)

Design/verification-level threads are not auto-fixed: lockfile drift from npm install --package-lock-only, the compat-guard "validates the wrong minimum" redesign (runtime.txt/.nvmrc vs constants), the noxfile --no-deps cross-repo coupling guard, project-wide tsconfig skipLibCheck, the file: vendoring/.vscodeignore bundling verification, and the sync-notification/branch-exists robustness gap. These warrant maintainer review rather than a mechanical change.

[Eduardo Villalpando Mello (edvilme)]

Follow-up review fixes (3 more commits)

Three additional identical commits address further review feedback:

7. sync.yml — re-notify on re-dispatch. The "Open or update the tracking issue" step now also runs when the sync branch already exists (exists == 'true'). If a prior run pushed the branch but died before notifying (or the tracking issue was closed), a re-dispatch now re-ensures the tracking issue and re-emits the compare URL to the job summary instead of silently exit 0-ing. The step is idempotent (it reuses any open issue). Combined with the earlier removal of the swallowed gh failure, the notification can no longer be permanently lost.
8. sync.yml — resolve symbolic Python floor. The Python-compat gate now falls back to resolving MINIMUM_PYTHON_MAJOR/MINIMUM_PYTHON_MINOR when minimumPythonVersion is declared via named constants (as in vscode-pylint), so the gate is no longer a silent no-op there.
9. noxfile.py — submodule guard. Added a friendly missing-submodule check (git submodule update --init --recursive) before installing the shared Python lib, symmetric with the build/postinstall.js guard.

Already addressed in the current workflow (earlier-round threads resolved)

Several earlier comments referenced code that in-place hardening already fixed: the ls-remote branch-exists check now retries and treats only exit code 2 as "absent"; the Node floor is read from engines.node (not .nvmrc); the Python floor is compared against src/common/constants.ts (not runtime.txt); and an unparseable floor now warns-and-skips while the compat steps are gated on an actual submodule change.

Still open for maintainer decision (not auto-changed)

Lockfile drift from npm install --package-lock-only, whether the sync should build/validate (npm ci + nox install_bundled_libs) before declaring the branch "ready" vs. wording the issue as "needs validation", the noxfile --no-deps runtime-dep mirroring strategy, preventing nox -s update_packages from rewriting the file: dependency, project-wide tsconfig skipLibCheck, and the .vscodeignore/pr-check verification items. These are design/verification calls I've left for review.

[Eduardo Villalpando Mello (edvilme)]

Review follow-up complete

All review threads on this PR are now resolved. Latest round of identical fixes across all five consumer PRs:

• Floor-parse hardening — Node/Python compatibility gates now warn-and-skip an unparseable floor instead of aborting the step under set -euo pipefail.
• nox -s update_packages safety — @vscode/common-python-lsp is pinned so the updater can't rewrite the file: link back to a registry range.
• Honest postinstall comment — clarified that a non-recursive clone fails at file: resolution during install, not at postinstall.
• Deterministic tracking issue — the tracking-issue title now uses the canonical v${VERSION}, so re-dispatching the same release as 1.2.3 and v1.2.3 reuses one issue instead of duplicating it; the issue body now states the pushed branch is unvalidated and must pass PR checks before merge.

Remaining review notes (lockfile --package-lock-only re-resolution, Python --no-deps dependency mirroring, project-wide skipLibCheck, and the dispatch-only compatibility gate) were acknowledged inline as documented, intentional trade-offs of the submodule consumption model.

[Rich Chiodo (rchiodo)]

GitHub cannot anchor PR review comments to unchanged lines in the diff. Falling back to a general PR comment for noxfile.py:L628.

Installing the shared Python package with --no-deps means its runtime dependencies must stay manually mirrored in requirements.in. A future automatically synced release that adds a dependency can bundle successfully but fail with ImportError at runtime, and the sync workflow does not validate that dependency set. Please add a check that the shared package's runtime dependencies are covered by the pinned requirements, or otherwise ensure they are installed from the submodule metadata.

[Rich Chiodo (rchiodo)]

Approved via Review Center.

[Heejae Chang (heejaechang)]

Approved via Review Center.