Strategy/Scala: route sbt 1.4+ with explicit DependencyTreePlugin to built-in command (TKT-15490)

[zlav]

Route sbt 1.4+ DependencyTreePlugin to the uppercase dependencyBrowseTreeHTML task, and detect plugins from the real sbt plugins output layout — restoring complete deep-dependency graphs on sbt 1.4+.

What changed

• On sbt 1.4+ with addDependencyTreePlugin, fossa ran the lowercase dependencyBrowseTreeHtml task (added for sbt <1.4 compat); modern sbt only exposes the uppercase task, so it failed and analysis silently fell back to a poms-only graph. Detection now classifies legacy (net.virtualvoid…DependencyGraphPlugin, lowercase) vs modern (sbt.plugins.DependencyTreePlugin, uppercase) and runs the matching task.
• Plugin detection now parses sbt's actual plugins layout — bare FQCNs under Enabled plugins in <project>:, with disablePlugins(...) entries excluded via the trailing not-enabled section. An interim : enabled in suffix anchor matched nothing in real output and dropped the built-in MiniDependencyTreePlugin on plain sbt 1.4+ projects.
• hasDependencyPlugins returns a DependencyPluginsDetected record instead of (Bool, Bool); the one caller (Strategy.Scala.findProjects) is updated.

Validation

• cabal build; fourmolu --mode check.
• unit-tests --match "detectDependencyPlugins" → 8/8 pass, including a verbatim sbt -batch -no-colors plugins capture as a regression guard.
• Full Scala integration test needs nix+sbt+network (~15 min) — relying on CI.

References

• Support ticket TKT-15490; prior routing work ANE-2718. Casing regression originated in Use -batch and -no-colors for sbt for compatibility with older SBT. #1356 (215a74f8).

Why not to merge this

• String-parse coupling — keys off sbt's human-readable plugins section headers; a future sbt reformat silently breaks detection again (no machine-readable contract).
• Legacy branches unverified live — only the sbt 1.9.8 fixtures are real captures; the net.virtualvoid and modern-without-mini fixtures are hand-built to the same layout, not captured from old sbt.
• Substring, not exact-line match — isInfixOf on the enabled section tolerates suffix variants but would miscount a plugin whose FQCN is a substring of another (none exist today).
• Symptom vs source — the detection regression was self-inflicted by earlier commits on this branch; squashing them would avoid shipping the bug+fix pair, though keeping them documents the trap.
• Worth merging anyway: restores complete deep graphs for the common sbt 1.4+ case, proven against the exact fixture the integration test exercises.

[zlav]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

This PR refactors Scala sbt plugin detection from boolean tuple results to structured types. New types DependencyTreePluginKind and DependencyPluginsDetected replace tuple returns in hasDependencyPlugins, detectDependencyPlugins, and genTreeJson. The detection logic now classifies plugins as modern or legacy with modern preference when both exist. A new mkDependencyBrowseTreeCmd helper maps plugin kinds to correct sbt task names (dependencyBrowseTreeHTML for modern, dependencyBrowseTreeHtml for legacy). The strategy layer imports and destructures the new record type, extracting the optional plugin kind and passing it to tree generation. Tests and fixtures are updated to validate the new behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description includes overview, validation, references, and acknowledgment of risks, but lacks explicit sections for acceptance criteria, testing plan, metrics, and checklist items as specified in the template.	Add the missing template sections: expand acceptance criteria, provide a numbered testing plan with concrete reproduction steps, document metrics if applicable, and include the full checklist with checkmarks or explanations for each item.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: routing sbt 1.4+ projects with explicit DependencyTreePlugin to the correct built-in command, with a reference to the issue tracker.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Strategy/Scala.hs (1)

206-212: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Consider making context and error messages task-agnostic.

The context message on line 206 and error message on line 212 both hardcode dependencyBrowseTreeHTML, but with the new routing logic the actual task executed could be either dependencyBrowseTreeHTML (modern) or dependencyBrowseTreeHtml (legacy) depending on the detected plugin kind. While these messages appear in a code path that runs after tree JSON generation (and thus don't directly cause incorrect behavior), using a generic message like "Analyzing sbt dependencies using dependency tree JSON" would be more accurate and less confusing for users who might see the legacy task invoked.

📝 Suggested message updates

-analyzeWithDepTreeJson (ScalaProject _ treeJson closure) = context "Analyzing sbt dependencies using dependencyBrowseTreeHTML" $ do
+analyzeWithDepTreeJson (ScalaProject _ treeJson closure) = context "Analyzing sbt dependencies using dependency tree JSON" $ do
   treeJson' <-
     errCtx MissingFullDependencyPluginCtx $
       errHelp MissingFullDependencyPluginHelp $
         errDoc sbtDepsGraphPluginUrl $
           errDoc scalaFossaDocUrl $
-            fromMaybeText "Could not retrieve output from sbt dependencyBrowseTreeHTML" treeJson
+            fromMaybeText "Could not retrieve dependency tree JSON output from sbt" treeJson

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Strategy/Scala.hs` around lines 206 - 212, The context and error messages
in analyzeWithDepTreeJson (pattern ScalaProject _ treeJson closure) hardcode
dependencyBrowseTreeHTML which can be either dependencyBrowseTreeHTML or
dependencyBrowseTreeHtml; update the context string and the fromMaybeText error
message to be task-agnostic (e.g., "Analyzing sbt dependencies using dependency
tree JSON" and "Could not retrieve output from sbt dependency tree JSON") so
they accurately reflect either modern or legacy task execution while still
referencing treeJson/ScalaProject/analyzeWithDepTreeJson to locate the change.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/Strategy/Scala.hs`:
- Around line 206-212: The context and error messages in analyzeWithDepTreeJson
(pattern ScalaProject _ treeJson closure) hardcode dependencyBrowseTreeHTML
which can be either dependencyBrowseTreeHTML or dependencyBrowseTreeHtml; update
the context string and the fromMaybeText error message to be task-agnostic
(e.g., "Analyzing sbt dependencies using dependency tree JSON" and "Could not
retrieve output from sbt dependency tree JSON") so they accurately reflect
either modern or legacy task execution while still referencing
treeJson/ScalaProject/analyzeWithDepTreeJson to locate the change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: f374c435-f539-45fd-b422-6fda86503657

📥 Commits

Reviewing files that changed from the base of the PR and between ae690f8 and fe7b09d.

📒 Files selected for processing (4)

• Changelog.md
• src/Strategy/Scala.hs
• src/Strategy/Scala/Plugin.hs
• test/Scala/PluginSpec.hs

[spatten]

Looks good to me!

Claude is calling out that there might be a separate mechanism that would cause missing dependencies. If hasMini = True and they have a multi-module project, then they get to line 184 of src/Strategy/Scala.hs, and fallback to a pom-only partial graph.

I think that's probably not the case here. I'm guessing we have a debug log to show us that the problem was the casing of the task. But I thought I'd mention it just in case it was useful.

Either way, I think that's a separate PR and this one is good to go.

[spatten]

I'm going to fix the vulns while I'm in here. This is the same problem we saw on Sparkle - they're all Rust vulns