Skip to content

chore(language-server): integrate LS - #7170

Open
team-ide-user wants to merge 1 commit into
mainfrom
chore/automatic-upgrade-of-ls
Open

chore(language-server): integrate LS#7170
team-ide-user wants to merge 1 commit into
mainfrom
chore/automatic-upgrade-of-ls

Conversation

@team-ide-user

@team-ide-user team-ide-user commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Changes since last integration of Language Server

commit 83d0e592d905fb1468cae76b74af050f3b5894d8
Author: Ben Durrans <Benjamin.Durrans@snyk.io>
Date:   Fri Aug 28 20:03:57 2026 +0100

    fix: mention configuration in untrusted-folder trust banner [IDE-2137] (#1418)
    
    Update the LS tree-view trust banner to warn that scanning may execute
    code and configuration, not just code, so users understand malicious
    project configuration risks before trusting a folder.
    
    Co-authored-by: Cursor Agent <cursoragent@cursor.com>

M	domain/ide/treeview/tree_builder.go

commit 8fcb26e70c2bd55588237ad48aeb166ac4aa6abc
Author: cursor[bot] <206951365+cursor[bot]@users.noreply.github.com>
Date:   Fri Aug 28 16:29:36 2026 +0200

    fix: stabilize TestInitializer_whenNoCli_Installs flaky on macOS [IDE-2443] (#1420)
    
    ### Description
    
    **Root cause:** `TestInitializer_whenNoCli_Installs` (and sibling `TestInitializer_whenBinaryUpdatesNotAllowed_DoesNotInstall`) in `infrastructure/cli/initializer_test.go` started `initializer.Init(t.Context())` in a detached goroutine and returned as soon as `assert.Eventually`/`require.Never` observed the expected installer state, without waiting for that goroutine to actually finish. `Initializer.Init`'s retry loop uses a plain `time.Sleep` (not context-aware), so the goroutine could keep running past the end of the test. That leftover goroutine raced `t.TempDir()`'s `t.Cleanup`-driven directory removal, re-entering `installCli` against a path whose backing temp directory had already started being torn down.
    
    On the macOS CI runner this produced:
    ```
    --- FAIL: TestInitializer_whenNoCli_Installs (0.02s)
        testing.go:1464: TempDir RemoveAll cleanup: unlinkat /var/folders/.../TestInitializer_whenNoCli_Installs.../002: directory not empty
    ```
    
    **Fix:** both tests now capture goroutine completion on a channel and block in `t.Cleanup` (registered after `t.TempDir()`, so it runs first due to LIFO cleanup order) until the goroutine finishes, before the temp dir is removed. No sleeps, timeout inflation, retries around the assertion, or skips were added.
    
    **Test evidence:**
    - Baseline (before fix), `go test ./infrastructure/cli/... -race -count=50`: 18/50 explicit `FAIL`, leaked goroutine hitting `no such file or directory` against the already-removed temp dir.
    - After fix, `-race -count=50`: 100/100 PASS, 0 FAIL.
    - After fix, `-race -count=200`: 400/400 PASS, 0 FAIL, 0 DATA RACE.
    - Full package regression, `go test ./infrastructure/cli/... -race`: all green.
    - `make test` (full unit suite): green, at the pushed commit.
    - `INTEG_TESTS=1 make test`: `infrastructure/cli` and `infrastructure/cli/install` pass; the one unrelated failure in `infrastructure/snyk_api` (a Pact contract test needing a live Snyk API token) reproduces identically on unmodified `main`, confirming it's pre-existing and unrelated to this change.
    
    This fix was produced by an automated flake-fix loop.
    
    ### Checklist
    
    - [x] Tests added and all succeed
    - [x] Regenerated mocks, etc. (`make generate`)
    - [x] Linted (`make lint-fix`)
    - [ ] README.md updated, if user-facing
    - [ ] License file updated, if new 3rd-party dependency is introduced
    
    
    <div><a href="https://cursor.com/agents/bc-3ec2f8d2-1fc4-4a98-8a52-9877e392ced6?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/automations/235317e1-a2b6-11f1-b532-320a589b8025"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/view-automation-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/view-automation-light.png"><img alt="View Automation" width="141" height="28" src="https://cursor.com/assets/images/view-automation-dark.png"></picture></a>&nbsp;</div>

M	infrastructure/cli/initializer_test.go

commit f1a58b559851d5718c07b007792f09e31ccdd908
Author: Andrew Robinson Hodges <andrew.robinsonhodges@snyk.io>
Date:   Thu Aug 27 15:46:08 2026 +0100

    chore: add agents files (#1417)

D	.cursor/rules/general.mdc
M	.gitignore
D	.windsurf/rules/general.mdc
D	.windsurfrules
D	AGENT.md
A	AGENTS.md
A	CLAUDE.md

commit 8a887d12281eac2ae58a067eb5eaff417e41f30c
Author: Ben Durrans <Benjamin.Durrans@snyk.io>
Date:   Tue Aug 25 17:37:58 2026 +0100

    chore: migrate off legacy AI Explain service [IDE-2487] (#1414)
    
    Autofix responses now carry the AI explanation inline via `code-client-go`, so the separate legacy AI Explain service call path in snyk-ls is redundant.
    
    - Removes the legacy `EnrichWithExplain` flow from `AiFixHandler` (explain-endpoint lookup, timeout, and cancellation logic) and its call site in `CodeFixDiffsCommand` — explanations now arrive as a passthrough field from `code-client-go`
    - Bumps `code-client-go` from v1.31.3 to v1.31.7 to bring in that passthrough explanation field
    - Removes pre–IDE-727 autofix leftovers with no production callers since autofix moved to `code-client-go` (#864): the local `AutofixUnifiedDiffSuggestion`/`AutofixResponse` types, the `gotextdiff`-based diff conversion in `convert.go`, and their tests. **Unified diffs are still computed** — in `code-client-go/llm/convert.go`, which `GetAutofixDiffs` already delegates to
    - `GetDetailsHtml` now warns when an autofix suggestion has no explanation
    - Adds a unit test asserting the explanation renders in the issue details HTML panel
    - Adds an end-to-end smoke-test check that polls the rendered issue description HTML until autofix reaches a terminal state, then asserts a real, non-empty explanation reaches both `AiFixHandler`'s internal state and the client-visible HTML
    
    ---------
    
    Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

M	application/server/server_smoke_test.go
M	domain/ide/command/code_fix_diffs.go
M	go.mod
M	go.sum
M	infrastructure/code/ai_fix_handler.go
D	infrastructure/code/ai_fix_handler_test.go
M	infrastructure/code/autofix.go
M	infrastructure/code/code_html.go
M	infrastructure/code/code_html_test.go
M	infrastructure/code/convert.go
M	infrastructure/code/convert_test.go
M	infrastructure/code/types.go

commit 15df8ee9885be8cbe08ff2d14e90b4eafbc51630
Author: Bastian Doetsch <bastian.doetsch@snyk.io>
Date:   Tue Aug 25 15:17:33 2026 +0200

    feat(server): expose structured Code AI Fix result via $/snyk.aiFix [IDE-2451] (#1405)
    
    ## Summary
    - Adds `types.AiFixNotification` / `types.AiFixResult` DTOs and a new `$/snyk.aiFix` case in `registerNotifier` (`application/server/notification.go`), following the exact existing pattern used for `$/snyk.scan`, `$/snyk.scanSummary`, and `$/snyk.treeView`.
    - Emits the notification from the existing `setStateCallback` seam in `domain/ide/command/code_fix_diffs.go`, alongside (not instead of) the existing `SendShowDocumentRequest` call, whenever the real `AiFixHandler` state machine transitions (`AiFixInProgress` / `AiFixSuccess` / `AiFixError`).
    - Payload is `{issueId, status, fixes: [{fixId, filePath}]}`, built from `AiFixHandler`'s real `AiStatus` and `[]llm.AutofixUnifiedDiffSuggestion` results via a new pure `aiFixResultsFrom` helper (one `AiFixResult` per file per suggestion, sorted for determinism).
    - Purely additive: `fixDiffs`'s fire-and-forget contract, the `showDocument` request/webview behavior, and all other `$/snyk.*` wire formats are unchanged.
    
    ## Test plan
    - [x] Integration test on the real notifier plane (`application/server/notification_test.go`, `Test_AiFixNotification`) — real `di.Notifier()`, real `registerNotifier`, real JSON-RPC harness; confirmed RED before the `registerNotifier` case existed.
    - [x] Integration test through the real production entrypoint (`domain/ide/command/code_fix_diffs_test.go`, `Test_codeFixDiffs_Execute_SendsAiFixNotification`) — real `Execute()` → `handleResponse()` → real `AiFixHandler` + real `Notifier`, driven to the deterministic network-free ERROR path (no bundle hash registered); confirmed RED before the wiring existed.
    - [x] Unit tests for the pure `aiFixResultsFrom` payload helper (no suggestions, single file, multiple suggestions spanning multiple files) — `Test_aiFixResultsFrom`.
    - [x] `go build ./...` and `go vet ./...` clean (whole tree)
    - [x] `GOOS=windows go build ./...` clean
    - [x] `go test -race ./...` clean (full repo, no regressions)
    - [x] `make lint` (golangci-lint) — 0 issues
    - [x] Snyk Code and Snyk SCA scans — clean (no new issues, no new vulnerable dependencies)

M	application/server/bdd_lint_test.go
M	application/server/bdd_steps_test.go
M	application/server/notification.go
M	application/server/notification_test.go
M	domain/ide/command/code_fix_diffs.go
M	domain/ide/command/code_fix_diffs_test.go
A	features/ai-fix-notification.feature
M	infrastructure/code/issue_enhancer.go
M	infrastructure/code/issue_enhancer_test.go
M	internal/types/lsp.go

commit 125215cde9c5e34bd73dee78962a6efb3b944501
Author: Bastian Doetsch <bastian.doetsch@snyk.io>
Date:   Tue Aug 25 10:41:58 2026 +0200

    fix: delta findings fail open when no baseline exists [IDE-2418] (#1397)
    
    * test: scaffold godog BDD harness and requirement-mapping lint [IDE-2418]
    
    Adds the CP0 BDD scaffold from the IDE-2418 plan:
    
    - godog test runner (application/server/bdd_test.go) and shared step
      registry (bdd_steps_test.go), reusing the existing unexported test
      harness (setupServer, testsupport.JsonRPCRecorder) instead of a
      second, divergent one.
    - One green feature file, features/harness.feature, driving the real
      initialize LSP handshake end-to-end.
    - Requirement-mapping lint (bdd_lint.go / bdd_lint_test.go) that fails
      the build if a scenario is missing its `# maps:` anchor or a tracked
      requirement has no covering scenario. ide2418Requirements is a
      growable list so each future checkpoint extends it alongside its own
      feature file, in the same commit.
    - make test-bdd target, a bdd CI job in build.yaml, and
      .github/scripts/bdd_coverage_gate.sh, which hard-fails feat: commits
      with no features/*.feature change and warns on fix: commits.
    - CONTRIBUTING.md section documenting the convention.
    
    No IDE-2418 fix logic yet — this PR is scaffold only.
    
    * refactor(server): use strings.SplitSeq in splitRequirementIDs [IDE-2418]
    
    Lint suggestion (stringsseq) on code introduced in the prior commit:
    range over strings.SplitSeq instead of strings.Split to avoid
    allocating the intermediate slice when it is only ranged over once.
    No behavior change.
    
    * fix(bdd): scope BDD server teardown to each scenario [IDE-2418]
    
    Two review-bot findings (Cursor Bugbot, snyk-pr-review-bot) confirmed
    against the godog BDD harness added for IDE-2418 CP0:
    
    - bddSteps reused the suite-level *testing.T across every scenario, so
      setupServer's t.Cleanup (which stops the jrpc2 server/listener) only
      ran once at the end of TestBDD instead of after each scenario,
      leaking a server/listener per scenario. Fixed by giving each
      scenario its own *testing.T via a t.Run bridge in new
      beforeScenario/afterScenario hooks wired through godog's
      sc.Before/sc.After. Options.TestingT is removed from TestBDD's godog
      Options because godog's own TestingT integration would otherwise
      wrap the same scenario in a second, colliding t.Run subtest (godog
      already runs Before/After inside its own per-scenario subtest when
      TestingT is set, but never exposes that concrete *testing.T to step
      definitions, so we still need our own bridge to get one).
    
    - parseFeatureScenarios' backward scan for a `# maps:` anchor stopped
      at the first non-blank line above a Scenario:, so a Gherkin tag
      (e.g. `@smoke`) placed between the anchor comment and the Scenario:
      line hid the anchor and made checkFeatureMappings falsely report the
      scenario as unmapped. Fixed by also skipping `@`-prefixed tag lines
      during the backward scan.
    
    * fix(ci): match feat!:/fix!: breaking-change commits in BDD coverage gate [IDE-2418]
    
    bdd_coverage_gate.sh's feat:/fix: regexes didn't allow the
    conventional-commit breaking-change marker (an optional `!` right
    before the colon), so a commit subject like `feat!: x` or
    `feat(scope)!: x` silently bypassed the hard-fail check that requires
    a features/*.feature change. Confirmed valid by two independent CI
    review bots (Cursor Bugbot, snyk-pr-review-bot).
    
    No existing test harness covers this script; verified manually with
    grep -E against the example subjects feat:, feat(scope):, feat!:,
    feat(scope)!:, fix!:, fix:, fix(scope):, and chore: (non-matching),
    confirming feat!:/feat(scope)!: now hard-fail as feat: does, fix!:
    now warns as fix: does, and chore: still matches neither regex.
    
    * fix(ci): quote BDD-gate step name to fix YAML parse failure [IDE-2418]
    
    The "Enforce BDD coverage for feat:/fix: commits" step name contained an
    unquoted colon-space sequence ("fix: commits"), which YAML parses as a
    nested mapping key inside a scalar context - breaking the whole workflow
    file (confirmed: run 30988182952 scheduled 0 jobs). Quote the name so it
    parses as a plain string, matching the pattern already used elsewhere in
    this file for names with special characters.
    
    * fix(server): skip plain doc comments when scanning for a maps anchor [IDE-2418]
    
    parseFeatureScenarios's backward scan for the `# maps:` anchor above a
    Scenario line already skipped blank lines and `@`-tag lines. It did not
    skip a plain `#`-prefixed documentation comment placed between the anchor
    and the scenario, so any such comment shadowed a real anchor and the
    scenario was incorrectly reported as unmapped. Keep scanning upward past
    plain comment lines, stopping only at a real anchor match or actual
    non-comment content.
    
    Adds a test fixture with a plain comment between the anchor and the
    scenario, proving the anchor is still found.
    
    * fix(server): run BDD step bodies on the goroutine that owns scenarioT [IDE-2418]
    
    godog invokes registered Given/When/Then functions on its own goroutine,
    which is not the goroutine beforeScenario's t.Run bridge spawns to own
    scenarioT. Step bodies call helpers (testutil.UnitTestWithEngine,
    setupServer) that take scenarioT and can call t.Fatal/t.FailNow on it.
    FailNow's runtime.Goexit only unwinds the goroutine that called it, so
    running a step directly on godog's goroutine would Goexit the wrong
    goroutine and hang the scenario's own goroutine forever instead of
    failing the subtest.
    
    Bridge step execution onto the scenario's own goroutine via a
    stepFunc/stepResult channel pair and a runOnScenarioGoroutine helper:
    the per-scenario subtest loops reading step closures off stepFunc and
    running them locally, so any t.Fatal/FailNow unwinds the correct
    goroutine and reports an error over stepResult instead of hanging.
    
    Adds Test_BDDSteps_RunOnScenarioGoroutine_SurvivesGoexit (proves a step
    that hits runtime.Goexit reports an error instead of hanging) and
    Test_BDDSteps_RunOnScenarioGoroutine_ReturnsStepResult (proves the
    normal non-failing path is unaffected).
    
    * refactor(server): trim repeated comment explanations in bdd_steps_test.go [IDE-2418]
    
    The goroutine-handoff rationale (why step bodies must run on scenarioT's
    goroutine instead of godog's) was spelled out in full four times: the
    bddSteps doc comment, runStep, runOnScenarioGoroutine, and a test doc
    comment. State it once, on runOnScenarioGoroutine, and replace the other
    occurrences with short pointers or trims of WHAT-level detail that was
    already visible in the surrounding code. No logic changes.
    
    * refactor(server): trim remaining restate-what comments in bdd_lint_test.go [IDE-2418]
    
    Cut three comments down to their genuine WHY, removing text that
    just restated the adjacent code/function name:
    - ide2418Requirements: kept only the ratchet-list rationale.
    - Test_FeatureFiles_MapEveryRequirement: comment fully restated the
      function name and its 3-line body; deleted.
    - Test_FeatureFiles_ScenarioWithoutMapsAnchorFails: kept only the
      reason the test exists (guards against the lint becoming a ghost
      check), dropped the restated sub-test structure.
    
    No logic changes.
    
    * refactor(server): drop ticket ID from symbol name per CLAUDE.md Rule 8 [IDE-2418]
    
    Rename ide2418Requirements to trackedFeatureRequirements in
    bdd_lint_test.go. Pure rename, no logic change: the identifier embedded
    a ticket ID in violation of CLAUDE.md Rule 8. Comment updated to
    reference the new name; wording otherwise unchanged as it was already
    ticket-agnostic.
    
    * refactor(server): further trim bdd_steps_test.go comments per CLAUDE.md Rule 8 [IDE-2418]
    
    Comment-only change, no logic touched. Tightened the bddSteps type doc
    to 4 lines, trimmed beforeScenario/runStep to state only the
    non-obvious WHY, and dropped afterScenario's comment entirely since
    its two-line body plus the function name already say everything it
    restated. runOnScenarioGoroutine is left untouched as the single place
    carrying the full goroutine/Goexit explanation.
    
    * fix: recurse feature-file scan, recover step panics [IDE-2418]
    
    parseFeatureDir used os.ReadDir, which only lists entries directly
    under features/, while godog's own loader walks the tree recursively.
    A .feature file placed in a subdirectory would run under TestBDD but
    silently skip the requirement-mapping lint. Switch to
    filepath.WalkDir so both walk the same tree.
    
    Also fix a related crash: runStep's deferred cleanup only handled a
    step body reaching t.Fatal/FailNow via runtime.Goexit. A genuine
    panic() from a step body (or something it calls) was not recovered
    and crashed the whole TestBDD process instead of failing just that
    scenario. Add recover() to the same deferred func so the send on
    stepResult is guaranteed on every path - return, Goexit, or panic.
    
    Update parseFeatureScenarios' doc comment, which claimed the upward
    anchor scan skips "only blank lines" - it also skips @tags and plain
    # comments. Mark the loop's remaining stop condition (any other line,
    including Background:/Rule:/Examples:) as a known, deliberate
    limitation rather than generalizing to every Gherkin keyword.
    
    * test: assert exact server name in BDD harness scenario [IDE-2418]
    
    theServerRespondsWithItsCapabilities only checked ServerInfo.Name for
    non-emptiness. Assert it equals the literal "snyk-ls" set in the
    initialize handler (application/server/server.go), so a regression
    that changes or drops the name is actually caught.
    
    ServerInfo.Version is left as a comparison against
    config.LsProtocolVersion: it's a ldflags-injected build var with no
    independent source of truth in a test binary, so the only way to
    assert something "more real" would be to mutate the same global the
    handler reads - which doesn't add coverage of the handler itself and
    couples the test to build-time injection mechanics instead.
    
    * fix(server): include stack trace in BDD step panic recovery [IDE-2418]
    
    runStep's panic recovery discarded the stack trace Go would otherwise
    print for an unrecovered panic, making real panics in BDD step bodies
    hard to diagnose. Capture runtime/debug.Stack() at recovery time and
    include it in the reported error.
    
    Adds Test_BDDSteps_RunStep_PanicIncludesStackTrace, which triggers a
    panic via runOnScenarioGoroutine and asserts the resulting error
    contains stack-trace content.
    
    * fix(server): fail the per-scenario BDD subtest on a step error [IDE-2418]
    
    Only the outer TestBDD learned of a step failure (via godog's own
    bookkeeping surfaced through t.Fatal), so the per-scenario subtest
    spawned by beforeScenario reported PASS even when one of its steps
    returned an error. Running that one subtest in isolation
    (go test -run 'TestBDD/scenario_name') or viewing it in an IDE test
    explorer would show a false PASS.
    
    runStep now calls s.scenarioT.Fail() itself on a step error or panic.
    Fail (not FailNow/Fatal) is safe here since it doesn't call
    runtime.Goexit, so it can't disrupt the channel-based step loop or the
    existing panic-recovery/Goexit-survival logic.
    
    Test_BDDSteps_ScenarioSubtest_FailsOnStepError proves this via a
    subprocess re-exec of a helper test: a real scenario-subtest failure
    propagates to the Go test that spawned it, so it has to happen in an
    isolated process rather than failing this package's own test run.
    
    * refactor(server): introduce config.LsServerName constant [IDE-2418]
    
    The server name "snyk-ls" was duplicated as a string literal in the
    initialize handler and in the BDD harness's capability assertion,
    mirroring the existing LsProtocolVersion precedent for the protocol
    version string. Introduce config.LsServerName and use it in both
    places so the two can't silently drift apart.
    
    Scope is deliberately limited to these two call sites; other unrelated
    "snyk-ls" occurrences elsewhere in the codebase are out of scope for
    this change.
    
    * docs(server): clarify recursion doc comment wording [IDE-2418]
    
    The doc comment on Test_ParseFeatureDir_RecursesIntoSubdirectories read
    as if godog's own loader does NOT recurse into subdirectories, which is
    the opposite of what the test guards against (parseFeatureDir must
    match godog's correct recursive behavior). Reworded for clarity.
    
    * refactor(server): trim comment growth from the scenarioT.Fail() fix [IDE-2418]
    
    * feat(config): per-folder Ambient Canary autonomy setting [IDE-2279] (#1399)
    
    * feat(config): per-folder Ambient Canary autonomy setting [IDE-2279]
    
    Adds a per-folder ambient_canary_autonomy setting so a developer can
    choose "Notify only" or "Allow autonomous fixes" per watched folder in
    the Snyk config dialog, or leave it unset to follow ambient-canary's
    own --autonomy-level default (OD-1/OD-2).
    
    - Register ambient_canary_autonomy as a folder-scoped setting with an
      empty-string default (not notify_only), so an untouched folder omits
      the key entirely from $/snyk.configuration and ambient-canary's
      default governs it.
    - Include the setting in computeEffectiveConfig so the dialog can
      render its current value and source (explicit vs inherited).
    - Add the folder-pane control to config.html, mirroring the
      scan_automatic block: two options only (no third "unset" state per
      OD-5), defaulting the selection to "Allow autonomous fixes" when
      unset (OD-5a), with the existing source-indicator/lock template
      helpers reused as-is.
    - Document the new folder-scope setting in docs/configuration.md.
    
    snyk-ls only stores, persists and transmits this value — it never
    acts on it. This lands ahead of the ambient-canary consumer change
    (IDE-2427) per ADR-39 cross-repo sequencing.
    
    * fix(config): stop unset ambient_canary_autonomy from rendering as autonomous_fixes [IDE-2426]
    
    Two confirmed snyk-pr-review-bot findings on the per-folder Ambient
    Canary autonomy setting:
    
    1. The folder-scope ambient_canary_autonomy <select> marked
       "autonomous_fixes" as selected whenever the setting was unset, even
       though the registry default is an empty string. This made an
       untouched folder read back as an explicit autonomous_fixes choice.
       Fixed by only selecting an option when its value explicitly matches
       the effective value, mirroring the existing notify_only condition.
       The control still renders exactly two options; an unset folder now
       selects neither.
    
    2. FOLDER_RESET_FIELDS in form-handler.js was missing
       ambient_canary_autonomy, so "Reset overrides" did not clear it.
       Added it to the array.
    
    * test(bdd): add acceptance coverage for per-folder ambient-canary autonomy [IDE-2426]
    
    Adds a godog scenario (# maps: M2) exercising the real $/snyk.configuration
    LSP notification: a folder's ambient_canary_autonomy has no override until
    the editor sets one via workspace/didChangeConfiguration, after which the
    effective per-folder value is reflected on the wire. Satisfies the BDD
    coverage gate for the per-folder Ambient Canary autonomy feat commit.
    
    ---------
    
    Co-authored-by: Bastian Doetsch <>
    
    * feat: add LLM provider and endpoint settings to the config dialog [IDE-2274] (#1400)
    
    * feat(config): add LLM provider and endpoint settings to the config dialog [IDE-2274]
    
    Adds an "LLM provider" and "custom API endpoint" machine-scope setting to
    the existing Snyk configuration dialog, so a developer can choose which
    LLM backend the Snyk Remediation Agent (Remy) uses, and optionally point
    it at a self-hosted endpoint. This is CP-1 of IDE-2274: settings-only and
    behavior-neutral for remediation - domain/snyk/remediation/remy.go is not
    touched (that is CP-2, a separate ticket).
    
    - Register llm_provider / llm_base_url as machine-scope settings following
      the existing cli_release_channel pattern exactly (internal/types).
    - Render the new fields in the config dialog template and reset-handler
      defaults; regenerate the config-dialog script fixtures via `make generate`.
    - Persist and echo back the chosen provider/endpoint in
      ConstructSettingsFromConfig and applyLlmProviderConfig
      (application/server/configuration.go), mapping provider -> base-URL env
      var (anthropic/vertex/litellm/ollama); openai has no such env var in the
      CLI extension, so a custom endpoint chosen with openai is persisted and
      shown but never exported to the process environment (documented in the
      field's tooltip). Never unsets an env var it did not itself set.
    - The LLM API key is never added as a field, stored, or logged - it
      continues to come only from the developer's own process environment.
    - New godog feature (features/llm-provider-setting.feature, 3 scenarios)
      written first and confirmed RED before any production code; the BDD
      harness needed WithRealDI() (application/server/bdd_steps_test.go) to
      exercise the real command service instead of TestInit's mock, matching
      existing precedent in configuration_smoke_test.go,
      background_init_lifecycle_test.go, and dispatch_starvation_test.go.
    
    * docs(server): restore trimmed doc comment on Test_BDDSteps_PerScenarioCleanup [IDE-2274]
    
    A stack-sync rebase onto the rewritten parent tip (feat/IDE-2418-delta-fail-open)
    reintroduced the verbose form of this doc comment, which had already been
    trimmed on the parent branch across multiple rounds this session for
    CLAUDE.md Rule 8 (WHY only, no big blocks) compliance. Restore the trimmed
    wording; no logic change.
    
    * fix(config): resolve leftover git conflict markers in config_html_test.go [IDE-2274]
    
    The previous stack-sync rebase (onto PR #1399's squash merge) resolved this
    file's conflict in the working tree but the raw markers ended up committed
    and pushed instead of the resolution, breaking compilation of the
    infrastructure/configuration package. Commits the already-verified merge
    that keeps both PR #1399's folder-autonomy tests and PR #1400's LLM-provider
    tests.
    
    * fix(server): make BDD ambient-canary-autonomy assertions wait for a matching notification [IDE-2274]
    
    The Ambient Canary autonomy-override BDD scenario failed only on Windows CI:
    "expected ambient_canary_autonomy override ..., got no override". The
    $/snyk.configuration notification is delivered to the jrpc2 test client on a
    goroutine spawned per received message (see jrpc2 Client.accept), so two
    notifications sent moments apart are not guaranteed to be recorded by the
    test's JsonRPCRecorder in send order. latestFolderConfigNotification assumed
    recorder order matched send order and returned whichever notification for
    the folder was recorded last, which can be a stale one under that race.
    
    Replace it with waitForFolderConfigNotification, which polls until it finds
    a notification whose settings actually match the expected state, rather
    than trusting recording order. This is a test-harness fix only; the
    production notify path (registerNotifier's single sequential listener
    goroutine) already delivers notifications in enqueue order.
    
    ---------
    
    Co-authored-by: Bastian Doetsch <>
    
    * fix(server): serialize LLM provider env var updates against concurrent requests
    
    applyLlmProviderConfig read the persisted LLM provider, then decided which
    process env vars to Unsetenv/Setenv based on that read, all without any
    synchronization. The language server's jrpc2 handlers run with unbounded
    concurrency (Concurrency: 0), so two overlapping workspace/didChangeConfiguration
    requests (e.g. two folders saving settings close together) could both read the
    same stale provider before either wrote, then race their env var mutations -
    leaving the process environment out of sync with whichever provider/base-URL
    was actually persisted, or briefly exposing both providers' base-URL env vars
    at once to a concurrently spawned CLI subprocess.
    
    Adds a dedicated mutex around the whole read-old/decide/write-new sequence so
    concurrent calls are fully serialized. No existing lock in this file already
    covered this path, so a new one scoped to this setting was added rather than
    reusing an unrelated lock.
    
    Addresses a snyk-pr-review-bot finding on PR #1397.
    
    * fix(server): prevent runOnScenarioGoroutine from hanging on a dead scenario goroutine
    
    If the scenario goroutine spawned in beforeScenario dies (e.g. panics
    during t.Run's own setup) before it reaches the select loop that reads
    s.stepFunc, runOnScenarioGoroutine's unbuffered channel send would
    block forever with no reader; likewise the following receive from
    s.stepResult would hang if the goroutine died mid-step. Use the
    existing s.scenarioDied channel as a select escape hatch on both the
    send and the receive so the call returns an error instead of hanging.
    
    Addresses a minor snyk-pr-review-bot finding on PR #1397.
    
    * feat: add LLM model setting for providers that require one [IDE-2274] (#1401)
    
    * feat(config): add LLM model setting for providers that require one [IDE-2274]
    
    Add llm_model as a third config-dialog field alongside the existing LLM
    provider and custom endpoint settings. Ollama and LiteLLM ship with no
    default model in remy-cli-extension, so those two of the five providers
    committed to by M1 are unusable without an explicit model. This persists
    and round-trips the model choice through the same settings pipeline as
    provider/endpoint (register_configurations.go, ldx_sync_config.go,
    configuration_command.go, config.html), with no environment-variable
    side effect (unlike base URL) since the model is consumed later via
    CP-2's GAF config key at fix time.
    
    Regenerated JS snapshot and config-dialog HTML fixtures to include the
    new field.
    
    * test(server): make LLM provider concurrency test race-free
    
    TestApplyLlmProviderConfig_ConcurrentUpdatesAreSerialized had failed
    twice on CI with a false "both env vars set at once" failure, never
    locally (30+ runs, incl. -race -count=20). applyLlmProviderConfig's
    lock correctly serializes the whole read-old/write-new sequence, so
    the production code was not the issue.
    
    The monitor goroutine read ANTHROPIC_BASE_URL and VERTEX_BASE_URL with
    two independent, unlocked os.Getenv calls. A writer's entire
    unset-then-set transition can complete between those two reads, so the
    monitor can see a stale "true" from before the transition and a fresh
    "true" from after it - a false positive with no real torn state ever
    existing.
    
    Fix: the monitor now takes llmProviderConfigMu around its own paired
    read, making the check atomic with respect to the writers it observes.
    Verified the test still fails reliably (10/10) with the production
    lock removed, and passes cleanly with it restored (50/50, -race).
    
    ---------
    
    Co-authored-by: Bastian Doetsch <>
    
    * fix: serve all findings when delta is enabled and no baseline exists [IDE-2418] (#1404)
    
    * test: add delta fail-open BDD harness and scenarios [IDE-2418]
    
    Adds features/delta-fail-open.feature (scenarios M2-M6) plus the godog
    step definitions and DI wiring needed to drive them through a real LSP
    server: ScanPersister override support in test_init.go, and step defs
    covering first scan, push/pull diagnostics, tree view, the existing
    regression case, and mixed products.
    
    These scenarios are RED against the current delta filtering logic
    (findings are hidden when no baseline exists); the fix lands in a
    follow-up commit stacked on this branch.
    
    * fix: serve all findings when delta is enabled and no baseline exists [IDE-2418]
    
    enrichCachedIssuesWithDelta returned early on ErrBaselineDoesntExist
    before DiffAndEnrich ever stamped IsNew, and filterIssuesWithConfig
    unconditionally kept only IsNew issues. With delta findings enabled but
    no baseline yet (first scan, corrupted snapshot, or a baseline that is
    lost after being available), every finding was silently hidden instead
    of shown.
    
    Folder now tracks per-product baseline availability
    (IsBaselineAvailable/IsDeltaAppliedForProduct/DeltaStatusForProduct) and
    filterByIsNew keeps an issue whenever its product has no available
    baseline, falling open instead of hiding everything. GetPersistedIssueList
    is hoisted above the early return so baseline availability is recorded
    even when there are no current issues to enrich.
    
    Turns the delta-fail-open BDD scenarios (feat/IDE-2420-delta-fail-open-guard)
    GREEN, and adds integration/unit coverage for the new Folder methods.
    
    * test: add delta fail-open BDD harness and scenarios [IDE-2418]
    
    * test(server): restore BDD steps dropped by a botched rebase [IDE-2420]
    
    A prior rebase onto feat/IDE-2418-delta-fail-open silently dropped 12
    step implementations (Ambient Canary per-folder autonomy, LLM
    provider/endpoint config dialog) plus a runOnScenarioGoroutine
    deadlock guard and its regression test, keeping only this branch's
    own delta-fail-open steps. Restore the missing methods, struct
    fields, and step registrations verbatim from the parent branch, and
    union bdd_lint_test.go's trackedFeatureRequirements so both this
    branch's and the parent's requirement IDs stay covered.
    
    * test(server): make LLM provider concurrency test race-free
    
    TestApplyLlmProviderConfig_ConcurrentUpdatesAreSerialized had failed
    twice on CI with a false "both env vars set at once" failure, never
    locally (30+ runs, incl. -race -count=20). applyLlmProviderConfig's
    lock correctly serializes the whole read-old/write-new sequence, so
    the production code was not the issue.
    
    The monitor goroutine read ANTHROPIC_BASE_URL and VERTEX_BASE_URL with
    two independent, unlocked os.Getenv calls. A writer's entire
    unset-then-set transition can complete between those two reads, so the
    monitor can see a stale "true" from before the transition and a fresh
    "true" from after it - a false positive with no real torn state ever
    existing.
    
    Fix: the monitor now takes llmProviderConfigMu around its own paired
    read, making the check atomic with respect to the writers it observes.
    Verified the test still fails reliably (10/10) with the production
    lock removed, and passes cleanly with it restored (50/50, -race).
    
    * fix(server): latest-per-URI publishDiagnostics semantics in smoke helper [IDE-2420]
    
    getIssueListFromPublishDiagnosticsNotification summed diagnostics across
    every recorded textDocument/publishDiagnostics notification for a URI,
    but per LSP semantics each notification for a URI replaces the previous
    one rather than adding to it. When a URI is published more than once in
    a scan cycle (e.g. an initial publish followed by a later re-publish),
    the helper double-counted stale issues from the earlier publish.
    
    Track only the most recent PublishDiagnosticsParams per URI before
    building the filtered issue list.
    
    ---------
    
    Co-authored-by: Bastian Doetsch <>
    
    * feat(remediation): pass configured LLM provider/model to remy fix workflow [IDE-2274][IDE-2430] (#1402)
    
    * feat(remediation): pass configured LLM provider/model to remy fix workflow [IDE-2430]
    
    buildRemyFixConfig now reads the developer's persisted LLM provider and
    model (types.SettingLlmProvider/SettingLlmModel via types.GetGlobalString)
    and forwards them to the fix workflow under the "provider"/"model" keys
    remy-cli-extension's FlagProvider/FlagModel read. Keys are only set when
    non-empty, matching the existing sast/agentic empty-key discipline, so a
    developer who never chose a provider forces no default (IDE-2274-M5).
    
    Extends the llm-provider-setting.feature CP-2 scenario end-to-end through
    the real workspace/executeCommand -> remyProvider.FixFolder -> gafRunner ->
    buildRemyFixConfig -> workflow.Engine.Invoke path, substituting only the
    external "fix" workflow itself (IDE-2448).
    
    * fix(server): wire a real FolderRemediator into the BDD harness [IDE-2430]
    
    The rebase onto feat/IDE-2418-delta-fail-open replaced aRunningLanguageServer's
    WithRealDI() setup (which wired the real remediation provider via di.Init)
    with delta-fail-open's WithDeps()-based setup, which builds command.Service
    manually and passed nil for remediationProvider. That made
    snyk.remediationAgent.fixFolder scenarios fail with "remediation agent is not
    enabled" even though production wiring in application/di/init.go was
    untouched and correct.
    
    Build a real FolderRemediator via remediation.NewRemyProvider(engine, nil),
    mirroring init.go's construction, and pass it into command.NewService instead
    of nil.
    
    Also removes a leftover "<<<<<<< HEAD" conflict marker left in
    bdd_steps_test.go by the same rebase.
    
    ---------
    
    Co-authored-by: Bastian Doetsch <>
    
    * chore: trim non-WHY comments to CLAUDE.md Rule 8
    
    Removed ticket IDs (IDE-2274/2426/2448) and internal plan-jargon
    shorthand (M1-M6, D1-D4, OD-1/OD-2, CP-1/CP-2, ACC-level, UNIT-9)
    from doc comments and inline comments across the LLM provider
    settings, ambient-canary autonomy, and BDD harness code. Kept only
    comments that explain a non-obvious WHY (concurrency ordering,
    external contract keys, goroutine ownership). Tightened the
    bdd_coverage_gate.sh header to concise usage instead of a
    restate-everything block.
    
    No production logic, test assertions, or wire formats changed.
    
    * chore: bump Go toolchain to 1.26.5 to fix SNYK-GOLANG-STDOS-17905377 [IDE-2418]
    
    Go 1.26.4's std/os package has a high-severity symlink attack
    vulnerability (SNYK-GOLANG-STDOS-17905377), fixed in 1.26.5. Update
    the go.mod toolchain directive and the CircleCI security-scans job
    image so the Open Source gate stops failing on the pinned stdlib
    version.
    
    * fix(remediation): synchronize LLM endpoint env vars with in-flight Remy fix invocation [IDE-2418]
    
    applyLlmProviderConfig's mutex only serialized concurrent settings saves
    against each other. It never synchronized with gafRunner's Remy fix
    invocation, which reads the same ANTHROPIC_BASE_URL/VERTEX_BASE_URL/
    LITELLM_BASE_URL/OLLAMA_HOST env vars via the external remy-cli-extension's
    os.Getenv calls at an unbounded point during the workflow - so a concurrent
    provider/base-URL save could be observed as a torn (briefly unset) endpoint
    mid-fix.
    
    remy-cli-extension has no config-based override for the base URL, so the
    env var mutation cannot be eliminated from this side. Instead, replace the
    private mutex with an exported sync.RWMutex (remediation.LLMProviderEnvMu)
    shared between the writer (Lock/Unlock around the full read-old/write-new
    sequence in applyLlmProviderConfig) and the reader (RLock/RUnlock around
    gafRunner's entire eng.Invoke call, since the read has no bounded snapshot
    point to lock only through).
    
    * fix(server): stop LLM provider settings save from starving on Remy's lock [IDE-2418]
    
    gafRunner holds remediation.LLMProviderEnvMu.RLock() for the whole
    duration of a Remy invocation, which could previously block a settings
    save waiting on the write lock to sync env vars. applyLlmProviderConfig
    now persists provider/baseUrl/model under a short, independent mutex,
    then reconciles the environment via TryLock: synchronously when free,
    otherwise via a single coalescing background worker that re-reads the
    latest persisted values when it finally acquires the lock, so queued
    updates always converge on the newest setting rather than replaying a
    stale one.
    
    Process-global state tracks which env var this process itself applied,
    preserving the invariant that a developer-supplied endpoint variable is
    never unset unless this process previously set it.
    
    * fix(server): correct LLM env error handling, lock layering, and delta baseline restart gap [IDE-2418]
    
    Three defects found during a verification pass on this branch:
    
    - applyLlmProviderEnvLocked marked appliedLlmEnvVar as changed even when
      the underlying os.Setenv/os.Unsetenv call failed, so a failed env write
      was silently treated as applied and a later reconcile would skip it.
      It now only updates appliedLlmEnvVar on success.
    - application/server locked/unlocked remediation.LLMProviderEnvMu
      directly, reaching into a domain-layer mutex from the application
      layer. The mutex is now private to the remediation package and
      reachable only via TryWithLLMProviderEnvLock/WithLLMProviderEnvLock.
    - Folder.IsBaselineAvailable returned the empty-cache zero value (false)
      until a scan ran in the current session, so a language server restart
      made an existing on-disk delta baseline look unavailable and briefly
      showed a fail-open (all findings) result instead of the delta view.
      It now lazily consults the persister when there's no cached entry yet.
    
    * test(bdd): make delta scenarios reusable, cover folder-level scope [IDE-2418]
    
    Parameterize the mixed-product delta scenario by product name instead of
    hardcoding Code/Open Source in bespoke step functions, add a scenario
    covering the folder-level ("workspace folder") variant of delta enablement
    alongside the existing global one, reword "enabled ... for the workspace" to
    "switched to globally" to avoid folder-scope ambiguity, reword the LLM
    endpoint step text to avoid confusion with the Snyk endpoint, and align the
    requirement-mapping comments to the IDE-2418-M# format already used by the
    LLM provider feature file (with the tracked-requirements ratchet in
    bdd_lint_test.go updated to match).
    
    * chore(bdd): fix copyright year on newly added harness files [IDE-2418]
    
    These files were created in this PR but carried a stale "2022-2026" header
    instead of "2026".
    
    * test(remediation): use exported const instead of hardcoded string in config test [IDE-2418]
    
    assert.False(t, conf.IsSet("provider")) checked a hardcoded string, which
    would silently pass if the production key ever changed; use
    remyProviderConfigKey instead. Also reworded a comment that undersold how
    likely the model-without-provider case is.
    
    * test(server): replace blocking receive and sleep-based assertion with timeout-guarded helpers [IDE-2418]
    
    Swap a second bare <-fixDone receive for testsupport.RequireEventuallyClosed
    (mirroring the existing fix earlier in the same file) so a wedged fix
    workflow fails the test instead of hanging, and replace a fixed
    time.Sleep(50ms) + assert with require.Never so a flap that happens right at
    the sleep boundary can't silently pass.
    
    * fix(config): lay out LLM provider and model fields side by side [IDE-2418]
    
    Use the existing Bootstrap row/col-md-6 grid pattern already used elsewhere
    in this file (rather than a one-off inline style) so the two fields sit side
    by side instead of stacked, and drop the outdated "(Remy)" name from the LLM
    provider tooltip. Includes the regenerated scripts/config-dialog/*.html
    outputs derived from this template.
    
    * refactor(server): extract LLM provider env-var management into its own type [IDE-2418]
    
    Move the LLM provider env-var state (which env var is currently applied,
    the reconcile-pending flag, the settings mutex) and its functions out of
    package-level globals in configuration.go into an llmProviderEnvManager
    type in a new file, with injectable setenv/unsetenv fields instead of
    swapping package-level function vars for tests. configuration.go now
    holds one long-lived singleton instance.
    
    Also reorders the LLM provider tests so the unit-style
    TestApplyLlmProviderConfig_*/TestApplyLlmProviderEnvLocked_* tests come
    before the integration-style TestUpdateSettings_Llm* round-trip tests,
    and adds a pointer comment on the "M5 guard" test to where that
    requirement is mapped (features/llm-provider-setting.feature).
    
    * feat(config): add Ambient Canary autonomy to Project Defaults [IDE-2418]
    
    Mirrors the existing scan_automatic dual-scope pattern so the setting
    can be set globally, not just per-folder, matching how other
    folder-scoped settings already work at Project Defaults level. Adds a
    "Not set" empty-value option to the new global select (matching the
    llm_provider convention for a genuine tri-state setting) - the
    per-folder select intentionally keeps its existing two-option-only
    design, since that was itself a deliberate fix (IDE-2426) for a prior
    bug where an unset folder rendered as an explicit autonomous_fixes
    choice. Updates the JS form-payload snapshot fixture and the generated
    scripts/config-dialog/*.html outputs derived from this template.
    
    ---------
    
    Co-authored-by: Cursor Agent <cursoragent@cursor.com>

A	.github/scripts/bdd_coverage_gate.sh
M	.github/workflows/build.yaml
M	CONTRIBUTING.md
M	Makefile
M	application/config/config.go
M	application/di/test_init.go
A	application/server/bdd_lint.go
A	application/server/bdd_lint_test.go
A	application/server/bdd_steps_test.go
A	application/server/bdd_test.go
M	application/server/configuration.go
M	application/server/configuration_test.go
A	application/server/llm_provider_env.go
M	application/server/server.go
M	application/server/server_smoke_test.go
M	docs/configuration.md
M	domain/ide/command/configuration_command.go
M	domain/ide/command/configuration_command_test.go
M	domain/ide/command/folder_handler_test.go
M	domain/ide/workspace/folder.go
M	domain/ide/workspace/folder_test.go
M	domain/ide/workspace/workspace_test.go
M	domain/snyk/persistence/nop_scan_persister.go
M	domain/snyk/remediation/remy.go
M	domain/snyk/remediation/remy_config_test.go
A	features/delta-fail-open.feature
A	features/harness.feature
A	features/llm-provider-setting.feature
M	go.mod
M	go.sum
M	infrastructure/configuration/config_html_test.go
M	infrastructure/configuration/template/config.html
M	infrastructure/configuration/template/js/ui/form-handler.js
M	infrastructure/configuration/template/js/ui/reset-handler.js
M	internal/types/config_resolver_test.go
M	internal/types/ldx_sync_config.go
M	internal/types/ldx_sync_config_test.go
M	internal/types/lsp.go
M	internal/types/mock_types/workspace_mock.go
M	internal/types/register_configurations.go
M	internal/types/register_configurations_test.go
M	internal/types/workspace.go
M	js-tests/folder-reset.test.mjs
M	js-tests/snapshots/form-payload.json
M	scripts/config-dialog/config_output_multi_project.html
M	scripts/config-dialog/config_output_no_projects.html
M	scripts/config-dialog/config_output_single_solution.html

@team-ide-user
team-ide-user requested a review from a team as a code owner August 25, 2026 08:49
@snyk-io

snyk-io Bot commented Aug 25, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️

"chore: automatic integration of language server 83d0e592d905fb1468cae76b74af050f3b5894d8" is too long. Keep the first line of your commit message under 72 characters.

Generated by 🚫 dangerJS against 8a4fa9b

@snyk-pr-review-bot

This comment has been minimized.

@team-ide-user
team-ide-user force-pushed the chore/automatic-upgrade-of-ls branch from e901a73 to 83f0551 Compare August 25, 2026 13:26
@snyk-pr-review-bot

This comment has been minimized.

@team-ide-user
team-ide-user force-pushed the chore/automatic-upgrade-of-ls branch from 83f0551 to 7c4e968 Compare August 25, 2026 16:45
@snyk-pr-review-bot

This comment has been minimized.

@team-ide-user
team-ide-user force-pushed the chore/automatic-upgrade-of-ls branch from 7c4e968 to 72f7ca2 Compare August 27, 2026 14:54
@snyk-pr-review-bot

This comment has been minimized.

@team-ide-user
team-ide-user force-pushed the chore/automatic-upgrade-of-ls branch from 72f7ca2 to 7f7d554 Compare August 28, 2026 14:38
@snyk-pr-review-bot

This comment has been minimized.

@team-ide-user
team-ide-user force-pushed the chore/automatic-upgrade-of-ls branch from 7f7d554 to 8a4fa9b Compare August 28, 2026 19:11
@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected
📚 Repository Context Analyzed

This review considered 7 relevant code sections from 6 files (average relevance: 0.90)

🤖 Repository instructions applied (from AGENTS.md)

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