refactor(scan): require a manifests tar hash for reachability analysis - #1532
Merged
Oskar Haarklou Veileborg (BarrensZeppelin) merged 2 commits intoSep 4, 2026
Merged
Conversation
performReachabilityAnalysis guarded its manifest upload on `uploadManifests && orgSlug && packagePaths`, all three of which were optional. Falling through that guard left tarHash undefined, which dropped both --manifests-tar-hash and --run-without-docker from the Coana invocation — a silent switch to Docker mode with locally resolved manifests rather than an error. No caller actually did this, but nothing in the type stopped a new one from doing it. Make orgSlug and packagePaths required, drop the uploadManifests option, and flatten the upload into straight-line code so tarHash is a definite string by the time the args are built. A hash-less upload response still fails hard at the parse boundary.
Oskar Haarklou Veileborg (BarrensZeppelin)
requested review from
Benjamin Barslev Nielsen (barslev),
Jeppe Fredsgaard Blaabjerg (jfblaa) and
Martin Torp (mtorp)
and removed request for
Jeppe Fredsgaard Blaabjerg (jfblaa)
September 4, 2026 07:02
Oskar Haarklou Veileborg (BarrensZeppelin)
marked this pull request as ready for review
September 4, 2026 07:37
Coana no longer supports --lazy-mode, so the CLI must not pass it. The hidden --reach-lazy-mode flag stays accepted and is swallowed, matching how --reach-disable-analysis-splitting was retired, so existing invocations keep working. The hand-maintained exclusion in the "any boolean reach flag implies --reach" check becomes a set of deprecated no-op flag names, so passing --reach-lazy-mode on its own no longer trips the requires---reach error.
Martin Torp (mtorp)
approved these changes
Sep 4, 2026
Martin Torp (mtorp)
left a comment
Contributor
There was a problem hiding this comment.
Awesome thanks!
Oskar Haarklou Veileborg (BarrensZeppelin)
merged commit Sep 4, 2026
8817945
into
v1.x
8 checks passed
Oskar Haarklou Veileborg (BarrensZeppelin)
deleted the
oskar/reach-tar-hash
branch
September 4, 2026 08:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
performReachabilityAnalysisguarded its manifest upload onuploadManifests && orgSlug && packagePaths, all three of which were optional inReachabilityAnalysisOptions. Falling through that guard lefttarHashundefined, which dropped both--manifests-tar-hashand--run-without-dockerfrom the Coana invocation.That fallback is a different execution mode, not a missing flag: Coana would attempt Docker and resolve manifests from the local tree, which is deprecated.
Why now
No live caller can currently hit it. Of the callers that reach reachability analysis,
scan reachandscan createboth gate on!!orgSlugin theircmd-*files and onpackagePaths.length > 0before calling;scan githubandcidon't gateorgSlugbut hard-coderunReachabilityAnalysis: false. So theelsebranch was dead — but nothing in the type stopped a new caller from reviving it.Changes
orgSlugandpackagePathsare now required (string/string[]); theuploadManifestsoption is gone.optionsparameter is required rather thanoptions?— it had required fields already.tarHashis a definitestringby the timecoanaArgsis built and the two flags are unconditional. A hash-less upload response still fails hard at the parse boundary.Also:
--lazy-modeis no longer forwardedCoana dropped support for
--lazy-mode, so the CLI must stop passing it.reachLazyModeis gone fromReachabilityOptionsand from every call site that was threading it through.The hidden
--reach-lazy-modeflag itself stays accepted and is now swallowed, so existing invocations don't start erroring — the same retirement--reach-disable-analysis-splittinggot. Its description says it's a deprecated no-op.One knock-on:
scan createdecides "you passed a reach flag, so you need--reach" by comparing every boolean reach flag against its declared default, with a hand-maintained exclusion forreachDisableAnalysisSplitting. That exclusion becomes aDEPRECATED_NO_OP_REACH_FLAGSset holding both names, so--reach-lazy-modeon its own no longer trips the requires---reacherror.Tests
The suite previously leaned on the no-upload branch to keep the SDK/API import chains out, so
setupSdkandhandleApiCallwere barevi.fn()s returningundefined. They now return an SDK stub and a tar hash, and all call sites pass the required options. Two new tests: the hash and--run-without-dockeralways reach Coana, and a hash-less upload fails without spawning Coana at all.pnpm test:uniton the three affected scan test files: 39 passed.pnpm run lint: passed.No changelog entry — internal, no user-visible behavior change.