Harden cycle recovery in Oak analysis
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 337
- Forks
- 32
- Avg merge
- 8d 17h
- Merged PRs (30d)
- 11
Description
https://github.com/posit-dev/positron/issues/15631 reported workspaces where the language server persistently stopped working. This was due to a Salsa cycle hit in scenarios where the implicit R/ folder collation was conflicting with an explicit source() call from the user. That cycle was fixed in https://github.com/posit-dev/ark/pull/1393, but the fix was targeted. This issue is about more generally controlling the risk of LSP crashes due to cyclic Salsa queries.
In Oak, even ordinary function calls can produce effects like Attach, Source, NSE, etc. Because of this, figuring out how a given R file works (what symbols it imports from other files, what's attached to the search path, how its scopes are structured) requires resolving symbols that can themselves live in other files, which in turn requires understanding how these files work, and so on. With this recursive logic it is easy to introduce cycles in the analysis, sometimes in unexpected paths. All these cycles need defined behavior, even when the R program has no actual recursion at runtime.
Unfortunately it's hard to guarantee that Oak's Salsa queries correctly handle cycles:
-
Recovery depends on the repeated query key. Different entry points and edit histories can expose different keys. A handler elsewhere in the cycle does not protect an unhandled repeated key. Furthermore, a cached query never causes a cycle panic. Entry order and edit history therefore influence whether a cycle panics.
-
The dependency graph is not visible in query declarations alone. Edges also pass through regular Rust functions, methods, etc. An unchanged query signature can acquire a new recursive dependency due to internal changes in the functions it calls.
-
Adding handlers can change results and make analysis less accurate. Salsa substitutes each cycle participant with its own fallback, if it has one, and adding handlers too coarsely can make the analysis less useful. For example, if a diagnostics query participates in a cycle and falls back to an empty list, that would conceal the analysis failure. Recovery should generally happen lower within the analysis layer, and we can't just give blanket cycle handlers to all queries to prevent panics.
Proposed measures to manage this:
-
Snapshots of query structure: Snapshot Salsa declarations and recovery policies in
oak_dbandoak_scan. Snapshot updates revealing query signatures, keys, options, or handlers would prompt a review for potential cycles. As a side benefit the snapshot will document the query surface, both for us and for agents. -
Snapshots of recovery activity: Record which handlers recovery fire and for which query keys. This makes changes in exercised recovery behavior visible. The fuzzing PR will rely on this mechanism too.
-
Hardening with invariants: Introduce restricted database interfaces so the resolver can call only selected recoverable semantic queries, while recovery helpers have only foundation-level access that are guaranteed to never cycle. This makes allowed dependencies explicit and prevents accidental direct calls outside those boundaries.
-
Fuzz testing: Fuzz workspaces, effectful source code, package relationships, query entries, and edit histories to produce randomised test scenarios to feed through the Oak analyzer. Fuzz-testing will have several benefits: extend our coverage beyond fixed tests and scenarios, ensure the analysis engine is panic-free, ensure some properties hold (such as purity of query implementations, i.e. a fresh analysis should match one reached via edits).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the Salsa declarations and recovery policies in oak_db and oak_scan, then map the proposed snapshots, recovery tracking, restricted interfaces, and fuzz testing measures. Done means Oak has defined cycle behavior without blanket handlers, visible recovery changes, enforced dependency boundaries, and fuzz coverage for panic-free analysis and edit consistency.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- r, rust
- Domain
- compilers, testing-qa
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100