Sync shared package via repository_dispatch and stop Dependabot duplicates

[edvilme]

Summary

Two related changes so the shared package (@vscode/common-python-lsp npm / vscode-common-python-lsp pip) is updated via a repository_dispatch event from vscode-common-python-lsp instead of by Dependabot.

1. New .github/workflows/shared-package-release.yml

Handles repository_dispatch (shared-package-release): branches off main, bumps the npm dependency, rewrites the pip pin in requirements.in and recompiles requirements.txt with uv, then pushes the branch and opens a tracking issue with a manual compare/PR link (org settings prevent the workflow from opening PRs automatically).

2. .github/dependabot.yml

Moves the shared package from the group exclude-patterns (which still produced a standalone Dependabot PR) into the npm and pip ignore lists, so Dependabot no longer opens duplicate PRs for it.

[rchiodo]

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR via the Review Center.

Other automated reviewers defer to this claim to avoid duplicate, conflicting feedback. The claim is refreshed while the review is active; if it is not refreshed within the ownership timeout it is treated as vacant and another reviewer may take over. This is an automated marker.

[rchiodo]

Solid approach overall. Main themes to address: make the workflow idempotent/re-runnable (branch push + issue creation), validate the incoming payload, and align the Python version with the documented lockfile-compile process.

[rchiodo]

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR via the Review Center.

Other automated reviewers defer to this claim to avoid duplicate, conflicting feedback. The claim is refreshed while the review is active; if it is not refreshed within the ownership timeout it is treated as vacant and another reviewer may take over. This is an automated marker.

[edvilme]

Thanks @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.

[rchiodo]

Most of the first-pass concerns are already handled in this revision (concurrency group, payload validation, PYTHON_VERSION: '3.10', issue-dedup with fallback, and the post-sed pin assertion all exist). The one substantive remaining issue is that --force-with-lease no longer actually protects the branch because of the preceding fetch into its tracking ref.

[rchiodo]

GitHub cannot anchor PR review comments to unchanged lines in the diff. Falling back to a general PR comment for requirements.txt:L188.

📍 requirements.txt:185
The Architect observes this file was hand-edited: the pin was removed but lsprotocol/pygls/packaging still carry # via … vscode-common-python-lsp annotations, and the header still claims uv pip compile … ./requirements.in. Runtime is fine (those deps are pulled transitively via pygls), but the next genuine uv pip compile will produce an unexpected diff. Regenerate with uv pip compile so the header command and via annotations are truthful.

[verified]

[rchiodo]

Main themes: (1) the submodule-sync workflow's --force-with-lease safety net is effectively a plain --force because the remote branch ref is fetched right before the push; (2) build/clone robustness — the file: link + postinstall build break plain (non---recurse-submodules) clones and the root lockfile goes stale when the gitlink advances, so the produced branch may fail npm ci. Worth tightening the push lease and guarding the postinstall.

[rchiodo]

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

[rchiodo]

The Python-compat guard validates the wrong "minimum": EXT_MIN is parsed from runtime.txt (the bundling Python), not the extension's declared minimum supported interpreter in src/common/constants.ts/noxfile.py. If runtime.txt is 3.13 but the extension still supports 3.9, the guard computes 3.13 >= PKG_FLOOR and passes — yet a user on 3.9 hits an ImportError from the bundled lib, the exact failure this guard claims to prevent. The error message names three files but only inspects one. Compare PKG_FLOOR against the declared minimum, not runtime.txt.

[rchiodo]

Two structural issues in the compat guard: (1) PKG_FLOOR is parsed from ${SUBMODULE_PATH}/python/pyproject.toml's requires-python, so any upstream repackaging (moved file, setup.cfg, or a dropped requires-python) yields an empty value and hard-fails every future sync until this workflow is edited. (2) It runs before the no-op short-circuit (git diff --quiet), so a re-dispatch that genuinely has nothing to do can still fail loudly. Consider treating an unparseable floor as warn-and-skip, and reordering the no-op check ahead of the drift guard.

[rchiodo]

Narrow notification gap: if git push succeeds but the runner dies before the tracking-issue/summary step, a re-dispatch hits this guard-and-skip and exit 0s before ever emitting a tracking issue or compare URL — the branch is left orphaned with no maintainer signal. On the skip path, consider checking for an existing tracking issue for ${BRANCH} and re-emitting the compare URL to $GITHUB_STEP_SUMMARY before exiting. Low priority; the guard-and-skip itself is correct.

[rchiodo]

npm install --package-lock-only --ignore-scripts is not wrapped in the retry helper used for fetch/push, so a flaky registry fails the job late. Because the root manifest carries ^/range deps, install (not ci) can also resolve newer transitives and commit lockfile churn beyond the gitlink bump. Consider wrapping it in retry and/or asserting the lockfile delta is limited to the linked package. Low priority.

[rchiodo]

Overall the submodule-sync approach looks solid. A few robustness notes on the sync workflow's Python-floor guard and dependency-constraint coupling are worth a look, but nothing blocking.

[rchiodo]

Approved via Review Center.

[rchiodo]

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

[rchiodo]

This Node guard validates against the wrong "minimum." It parses PKG_NODE from the submodule's .nvmrc (an exact dev pin) and requires EXT_NODE >= PKG_NODE, but the submodule's actual buildability floor is engines.node (>=18.0.0). If the submodule bumps its .nvmrc above the extension's Node version while still declaring engines.node >=18, the extension can build it fine yet this step hard-fails the whole sync. The strict full-X.Y.Z regex also hard-fails every future sync if the submodule .nvmrc is absent or partial (22, lts/*). Prefer comparing against engines.node (or drop the guard and let npm ci/postinstall fail loudly), and treat an unparseable floor as warn-and-skip.

[rchiodo]

The PR description is stale and now misleading: it narrates a shared-package-release.yml doing uv pip compile/sed/requirements.in rewriting, but the shipped file is shared-package-submodule-sync.yml doing git-submodule vendoring. Only the Dependabot section is accurate. Please rewrite the description to match the submodule-sync mechanism before merge.

[rchiodo]

Two structural points on the compat guards. (1) They reach across the repo boundary into the submodule's internal layout (${SUBMODULE_PATH}/python/pyproject.toml, ${SUBMODULE_PATH}/.nvmrc), so any upstream repackaging (moved file, setup.cfg, dropped requires-python) hard-fails every future sync until this workflow is edited. (2) Both guards run before the step-5 git diff --quiet no-op check, so a genuinely no-op re-dispatch does the extra work and can fail loudly with nothing to sync. Consider reordering the no-op detection ahead of the drift guards, and treating an unparseable floor as warn-and-skip.

[rchiodo]

The branch-exists guard treats any git ls-remote failure as "branch absent" — a transient network error falls into the else → exists=false → the workflow proceeds to create and push. Unlike the later network ops, this ls-remote is not retried. Consider wrapping it in retry, or distinguishing exit code 2 (no ref) from a genuine failure, so a network blip can't bypass the guard.

[rchiodo]

Overall this is sound. Two themes worth addressing before merge: (1) the Node/Python compatibility guards in the sync workflow validate against the submodule's exact dev pins (.nvmrc, runtime.txt) rather than its declared build/support floors (engines.node, declared minimum Python), which can hard-fail future syncs on benign upstream changes; and (2) the PR description still describes a different mechanism (shared-package-release.yml doing uv pip compile) than what shipped (shared-package-submodule-sync.yml doing git-submodule vendoring) — please update it.

[rchiodo]

Approved via Review Center.

[rchiodo]

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

[rchiodo]

📍 .github/workflows/shared-package-submodule-sync.yml:88
Notification gap on the guard-skip path: if git push succeeds but the runner dies before the "Open or update the tracking issue" step, a re-dispatch hits the branch-exists guard (exists=true) and skips all gated steps — so the pushed branch is left with no tracking issue or compare URL. The guard conflates "maintainer is editing" with "prior run crashed mid-flight." On the exists=true path, consider re-emitting the compare URL to $GITHUB_STEP_SUMMARY (and/or checking for an existing tracking issue) before finishing. Low priority; the guard-and-skip itself is correct.

[rchiodo]

Overall this is sound. One real structural gap worth tracking (submodule Python runtime deps are satisfied only by hand-pins in requirements.in with no drift enforcement) plus a stale PR description. Nothing blocking.

[rchiodo]

Approved via Review Center.

[rchiodo]

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

[rchiodo]

📍 .github/workflows/shared-package-submodule-sync.yml:145
The Node guard parses PKG_NODE as the first integer sequence in engines.node, which has two latent edge cases: (1) sort -V orders a bare-major .nvmrc (e.g. 18) before a full 18.0.0 floor, spuriously hard-failing a compatible sync; and (2) a compound range like >=20 || ^18.18.0 or >=18 <=20 is mis-classified (first-integer-wins, no upper-bound check). Neither trips today (.nvmrc is full 24.17.0, engines.node is open-ended >=18.0.0). Normalize both operands to a common component count (or use semver) if you want to harden the floor comparison.

[rchiodo]

Overall this looks solid and is safe to merge. A few optional hardening notes are left inline (Python dep drift, workflow version-parsing edge cases, and a crash-window notification gap); none are blocking.

[rchiodo]

Approved via Review Center.