Nimblesite / Nimblesite/SharpLsp

Branch protection on main requires no functional test job — a fully red test run can merge

Open
#192 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug critical
Dominant language
TypeScript
Stars
132
Forks
5
Avg merge
6h 24m
Merged PRs (30d)
27

Description

main's required status checks are:

Detect Changed Paths
Dependency Review
Validate Shipwright Manifest
Analyze (actions)
Analyze (csharp)
Analyze (javascript-typescript)
Analyze (rust)

Every functional test job is absent: Rust / Shard 1..2, Rust / Coverage Gate,
.NET / Sidecars, .NET / Named-Pipe Transport (Windows), VS Code / Full Suite + Coverage, and all seven VS Code (Windows) feature chunks. A pull request whose entire
test matrix is red satisfies branch protection and can be merged to main.

Why this matters concretely

PR #189 restored <PackageReference Update="Microsoft.CodeAnalysis.CSharp" PrivateAssets="none" />,
whose absence shipped the C# sidecar with no Roslyn compiler layer — the sidecar then
resolves Roslyn from the machine SDK and workspace/open crashes. That defect was caught
by VS Code / Full Suite + Coverage and by every Windows chunk. Under current protection,
none of those jobs could have blocked the merge that introduced it.

The same applies to the coverage ratchets: _test-dotnet and _gate-rust-coverage enforce
the thresholds, and neither is required.

Suggested fix

Add to the required set (names as reported by gh pr checks):

  • Rust / Shard 1/2, Rust / Shard 2/2, Rust / Coverage Gate
  • .NET / Sidecars, .NET / Named-Pipe Transport (Windows)
  • VS Code / Full Suite + Coverage
  • the VS Code (Windows) / * chunks

Note that several of these are conditioned on Detect Changed Paths, so they report
skipping rather than pass when their paths are untouched. GitHub treats a skipped
required check as satisfied only when the job is skipped via if: at the job level — worth
confirming the path-filter wiring produces that, otherwise unrelated PRs will block. That
check is the only real work in this change; the rest is repository settings.

Found while shipping #189. Refs [DIST-CI-LAYOUT].

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by inspecting the branch protection settings and the workflow jobs reported by gh pr checks, especially Detect Changed Paths, _test-dotnet, and _gate-rust-coverage. Confirm that the listed Rust, .NET, and VS Code checks are required and that path-filtered jobs skip at the job level for unrelated changes. Done means a fully red functional test matrix cannot merge while unaffected PRs are not blocked.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, github-actions, rust, typescript
Domain
ci-cd, devops, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.