Skip to content

fix: preserve monitor identity for poetry.lock#188

Open
thomasschafer wants to merge 1 commit into
mainfrom
chore/preserve-monitor-identity
Open

fix: preserve monitor identity for poetry.lock#188
thomasschafer wants to merge 1 commit into
mainfrom
chore/preserve-monitor-identity

Conversation

@thomasschafer
Copy link
Copy Markdown
Contributor

@thomasschafer thomasschafer commented May 22, 2026

This PR adds a small fix to Identity.TargetFile to ensure it matches the logic in Registry. Currently, depGraphOutputToSCAResult forwards the legacy CLI's targetFileFromPlugin field into Identity.TargetFile, and that value flows downstream into the monitor project identity. If it ever diverges from what the legacy CLI itself sends, existing monitor projects are not found and duplicates get created.

Registry applies one compatibility transform on input: exact-string "poetry.lock""pyproject.toml". snyk-python-plugin already produces "pyproject.toml", so the transform is unused today, but if that invariant ever breaks (e.g. changes to the Poetry resolver) we would no longer be matching the behaviour in Registry.

@github-actions github-actions Bot added the chore label May 22, 2026
@snyk-io
Copy link
Copy Markdown

snyk-io Bot commented May 22, 2026

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.

Mirror registry's poetry.lock → pyproject.toml rewrite client-side and pin
the identity contract for depGraphOutputToSCAResult.

Identity.TargetFile is derived from the legacy CLI's targetFileFromPlugin
and flows downstream into the monitor project identity. If it ever diverges
from what the legacy CLI itself sends, existing monitor projects can become
unfindable and duplicates get created.

Registry applies one compatibility transform on input: exact-string
"poetry.lock" → "pyproject.toml". The bundled python plugin already
produces "pyproject.toml" form, so this is dead code today, but if any
upstream invariant ever breaks we would silently start creating duplicate
projects. mapToTargetFileIdentity mirrors the rewrite and logs if it fires,
so an upstream regression would be visible rather than silent.

Also adds a contract comment warning against substituting NormalisedTargetFile
when targetFileFromPlugin is nil — doing so would change identity for
workspaces, maven, sbt, rubygems and break existing customers.
@thomasschafer thomasschafer force-pushed the chore/preserve-monitor-identity branch from be35bf7 to 5ef7e07 Compare May 22, 2026 13:25
@thomasschafer thomasschafer changed the title chore: preserve monitor identity for legacy CLI dep graphs fix: preserve monitor identity for poetry.lock May 22, 2026
@github-actions github-actions Bot added fix and removed chore labels May 22, 2026
@thomasschafer thomasschafer marked this pull request as ready for review May 22, 2026 13:26
@thomasschafer thomasschafer requested a review from a team as a code owner May 22, 2026 13:26
@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Compilation Error (2 occurrences) 🔴 [critical]
  1. pkg/ecosystems/legacy/plugin.go
    The function mapToTargetFileIdentity uses new("pyproject.toml"). In Go, the built-in new function takes a Type as its argument (e.g., new(string)), not a value. This will result in a compilation error: 'pyproject.toml' is not a type. To return a pointer to a string literal, you must either use a helper function (like the stringPtr function that was removed in this PR) or assign the value to a variable and take its address.

  2. pkg/ecosystems/legacy/plugin_test.go
    The test cases in TestPlugin_TargetFileForwarding have been updated to use new("project.assets.json") and similar patterns. This will cause a compilation error as new expects a type, not a constant value. The previous implementation used a local helper stringPtr, which is the standard way to handle this in Go tests.

📚 Repository Context Analyzed

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant