Skip to content

feat: consolidate print-graph flags [CSENG-190]#6822

Open
neema-beglou-snyk wants to merge 4 commits into
mainfrom
feat/CSENG-190/print-graph-consolidation
Open

feat: consolidate print-graph flags [CSENG-190]#6822
neema-beglou-snyk wants to merge 4 commits into
mainfrom
feat/CSENG-190/print-graph-consolidation

Conversation

@neema-beglou-snyk

@neema-beglou-snyk neema-beglou-snyk commented May 19, 2026

Copy link
Copy Markdown

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages
    are release-note ready, emphasizing
    what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

Replaces four overlapping --print-graph family flags with a composable two-flag model:

New flags Behavior
--print-graph --prune Pruned/deduplicated dep graphs, JSONL output
--print-graph --jsonl Complete (unpruned) dep graphs, JSONL output
--print-graph (bare) Plaintext output (unchanged, Phase 2: becomes JSONL)

The old flags (--print-effective-graph, --print-effective-graph-with-errors, --print-output-jsonl-with-errors) are accepted as deprecated aliases. When used, they emit a deprecation warning to stderr and are tracked via analytics.add('deprecatedLegacyDepGraphFlag', flag) for Phase 2 removal.

Internally, flag interpretation is separated into three distinct concerns:

  • shouldPrintGraph(opts) — whether to print
  • isJsonl(opts) — output format (JSONL vs plaintext)
  • shouldEmbedErrors(opts) — whether to embed scan errors inline or throw

Additional fixes included:

  • --prune plumbed into pruneIsRequired() for monitor and test paths
  • DeriveExitCode now handles snyk_errors.Error (e.g. SNYK-CLI-0008 "no supported files found" maps to exit code 3 instead of generic 2), using the existing mapping from behavior/maperrortoexitcode.go
  • Relative file path bug fixed in printDepGraphError()targetFile is now resolved before calling path.relative()

Where should the reviewer start?

src/lib/snyk-test/common.ts — the mapLegacyGraphFlags() function and the three concern helpers (shouldPrintGraph, isJsonl, shouldEmbedErrors). This is the core of the change.

Then src/lib/snyk-test/run-test.ts for how the new flags are consumed in the test path.

How should this be manually tested?

# New flags
snyk test --print-graph --prune <target>       # pruned JSONL
snyk test --print-graph --jsonl <target>       # complete JSONL
snyk test --print-graph <target>               # plaintext (unchanged)

# Legacy flags should still work with deprecation warning on stderr
snyk test --print-effective-graph <target>
snyk test --print-effective-graph-with-errors <target>
snyk test --print-output-jsonl-with-errors <target>

# Verify --prune triggers pruning in monitor path
snyk monitor --print-graph --prune <target>

# Verify exit code 3 for unsupported projects
snyk test --print-graph --prune <dir-with-no-supported-files>
echo $?  # should be 3

What's the product update that needs to be communicated to CLI users?

CLI users should not be aware of the changes, this layer of interfacing with the CLI is for extensions/plugins. Ideally we move all consumers to interface with extension-dep-graph instead of directly with the legacyCLI.

Risk assessment (Low | Medium | High)?

Medium

  • Legacy flags are preserved as aliases, so no existing integrations break.
  • The flag resolution logic is covered by 9 new unit tests in print-graph-flag-resolution.spec.ts.
  • Main risk is downstream consumers that parse CLI stdout — the output format itself is unchanged for each mode, only the flags to request it changed.

Any background context you want to provide?

This is the CLI (producer) side of a cross-repo initiative (CSENG-190). Companion PRs in the dep-graph router (CSENG-191), SBOM extension (CSENG-192), snyk-delta (CSENG-193), and container-cli (CSENG-198) update consumers to use the new flags. The CLI ships first; consumers can migrate independently because legacy flags remain functional.
The migration for consumers is 2 fold, always parse JSONL and if we don't want partial results then parse the embedded error and throw.

Phase 2 (pending analytics confirming zero legacy flag usage): remove legacy flag mappings, remove --jsonl (bare --print-graph becomes JSONL), delete plaintext output path.

What are the relevant tickets?

  • CSENG-190 (this PR)
  • CSENG-191 (dep-graph router)
  • CSENG-192 (SBOM extension)
  • CSENG-193 (snyk-delta)
  • CSENG-198 (container-cli)

@neema-beglou-snyk neema-beglou-snyk requested review from a team as code owners May 19, 2026 13:35
@snyk-io

snyk-io Bot commented May 19, 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.

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
Warnings
⚠️ There are multiple commits on your branch, please squash them locally before merging!
⚠️

"[fix: map legacy flags in allow-incomplete-sbom tests and apply prettier CSENG-190](https://api.github.com/repos/snyk/cli/git/commits/f59b51378250d8820fc90a54cd34b1a7278199e5)" is too long. Keep the first line of your commit message under 72 characters.

Generated by 🚫 dangerJS against 6bd1db6

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot

This comment has been minimized.

@neema-beglou-snyk neema-beglou-snyk force-pushed the feat/CSENG-190/print-graph-consolidation branch from 4d694ea to 6bd1db6 Compare June 17, 2026 17:19
@snyk-pr-review-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

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

Logic Error in Pruning 🟠 [major]

In assembleLocalPayloads, the condition !isJsonl(options) || options['prune'] at line 902 decides whether to prune the graph. However, the legacy mappings in common.ts (e.g., --print-effective-graph) set both jsonl = true and prune = true. For non-deprecated --print-graph --jsonl (no prune), this correctly skips pruning. But for the legacy --print-output-jsonl-with-errors, which mapped to prune: false (line 209 of common.ts), this logic remains correct. The risk is that if a user uses --print-graph --jsonl (new bare JSONL), they get unpruned graphs, which is intended, but the check !isJsonl(options) means the legacy 'plaintext' --print-graph (which is Phase 1) will ALWAYS prune, which might be a change in behavior if bare --print-graph previously didn't prune by default.

if (packageManager && (!isJsonl(options) || options['prune'])) {
Reference Error 🟠 [major]

In monitorDepGraph, the variable pruneIsRequired is initialized at line 320, but the previous line 322 in the old hunk shows it was already being used by pruneGraph. The new hunk defines pruneIsRequired locally at line 320, but if monitorDepGraph is called and scannedProject.depGraph is null (triggering the throw at 295), this is fine. However, in the provided diff for monitorDepGraph, there is no evidence that pruneIsRequired was declared before its use in the call to pruneGraph at line 322, potentially leading to a ReferenceError if the JS engine reaches that line and the const declaration at 320 was intended to be the only definition.

const pruneIsRequired =
  options.pruneRepeatedSubdependencies || !!options['prune'];
depGraph = await pruneGraph(depGraph, packageManager, pruneIsRequired);
📚 Repository Context Analyzed

This review considered 33 relevant code sections from 10 files (average relevance: 0.98)

🤖 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