Skip to content

Await which() in dependency configure guards - #14731

Open
cderv wants to merge 2 commits into
quarto-dev:mainfrom
cderv:claude/typst-gather-which-await-t8dae2
Open

Await which() in dependency configure guards#14731
cderv wants to merge 2 commits into
quarto-dev:mainfrom
cderv:claude/typst-gather-which-await-t8dae2

Conversation

@cderv

@cderv cderv commented Jul 26, 2026

Copy link
Copy Markdown
Member

Description

The non-vendored branch of each dependency's configure callback in
package/src/common/dependencies/ verifies the binary is on PATH with:

} else {
  // verify that the binary is on PATH, but otherwise don't do anything
  if (which(file) === undefined) {
    throw new Error(
      `${file} is not on PATH. Please install it and add it to PATH.`,
    );
  }
}

which is declared export async function which(cmd: string) in
src/core/path.ts, so this compares a Promise to undefined. A Promise is
never === undefined, the condition is always false, and the guard never fires.
The enclosing configure is already async, so the fix is one await per site.

Why it matters

When building with QUARTO_VENDOR_BINARIES=false — which is what conda-forge
does, so binary deps come from conda packages rather than downloaded release
assets — configureDependency skips the download but still calls configure().
This guard is the only thing meant to catch "the binary is nowhere to be found".

Because it never fires, the build reports Configuring typst-gather /
typst-gather complete. and succeeds, silently producing a Quarto tree with no
typst-gather binary at all. Confirmed against a real green CI build of
quarto 1.10.18 on conda-forge. The failure surfaces only at runtime: renders
print typst-gather analyze failed; staging all packages as fallback: ... on
every Typst render, and quarto call typst-gather dies with a misleading error.

The same bug is present verbatim in four dependencies, all fixed here:

File Binary
package/src/common/dependencies/typst-gather.ts typst-gather
package/src/common/dependencies/typst.ts typst
package/src/common/dependencies/pandoc.ts pandoc
package/src/common/dependencies/esbuild.ts esbuild

(deno_dom.ts uses a synchronous Deno.env.get check and is unaffected;
dartsass.ts and deno.ts have no else branch at all, which is arguably a
separate gap and not touched here.)

Stale error message

Also refreshes the "binary not found" message in typstGatherBinaryPath()
(src/core/typst-gather.ts) and in the dev-call typst-gather command. Both
said:

typst-gather binary not found.
Run ./configure.sh to build and install it.

That is stale as of 1.10 — configure.sh no longer builds typst-gather (the
cargo step and package/typst-gather/ were both removed; it is now a downloaded
release binary from quarto-dev/typst-gather). Neither message mentioned
QUARTO_TYPST_GATHER, which both code paths already honour. The new messages
name the path actually checked, and distinguish "you pointed the env var
somewhere wrong" from "your install is missing the bundled binary".

Notes for reviewers

  • Nothing here is compile-checked or test-run. No CI workflow runs
    deno check over package/src, and package/src has no test coverage.
    Promise<string | undefined> === undefined is TS error 2367 — a type-check
    pass would have caught all four of these at build time. Adding a
    deno check package/src/** step is probably the durable fix for this class of
    bug, but felt out of scope here; happy to open a follow-up.

  • Backport. Per dev-docs/checklist-backport-a-pr.md this targets main
    (1.11), with the changelog entry under ## Regression fixes to signal the
    backport. The v1.10 branch is where this actually bites for packaging, so a
    cherry-pick to v1.10 would be welcome.

  • Two more instances of the same class turned up in a repo-wide scan and are
    deliberately not in this PR, since each is a real behavior change worth
    reviewing on its own:

    • src/command/render/latexmk/pdf.ts:394
      if (bibCommand !== "biber" && !hasTexLive()). hasTexLive is async, so
      !Promise is always false: the Windows \bibdata{...}.bib aux-file fixup is
      dead code on every Windows PDF render using bibtex.
    • src/tools/impl/tinytex.ts:446if (!isTinyTex()) in uninstall() never
      fires, so quarto uninstall tinytex skips its "this is not a TinyTeX
      install" guard.

    Say the word and I'll open a separate PR for those.

Checklist

I have (if applicable):

  • referenced the GitHub issue this PR closes
  • updated the appropriate changelog in the PR
  • ensured the present test suite passes
  • added new tests
  • created a separate documentation PR in Quarto's website repo and linked it to this PR
AI-assisted PR
  • AI tool used: Claude Code
  • Codebase grounding: local clone — src/core/path.ts (confirmed which is
    async), the four configure callbacks in
    package/src/common/dependencies/, a repo-wide scan for unawaited async
    predicates used in conditions, dev-docs/checklist-backport-a-pr.md and
    .claude/rules/changelog.md for branch/changelog conventions.
  • Human review: I have reviewed, tested, and verified the AI-generated
    content before submitting.

The non-vendored branch of each dependency's configure callback verifies
the binary is on PATH with `if (which(file) === undefined)`. `which` is
async, so this compares a Promise to undefined: always false, guard never
fires.

Effect: with QUARTO_VENDOR_BINARIES=false, configureDependency skips the
download but still calls configure(). The guard is the only thing meant to
catch a missing binary, so the build reports success and produces a Quarto
tree without it. For typst-gather this surfaces only at render time, as
"typst-gather analyze failed; staging all packages as fallback".

Fixes the same bug in typst-gather, typst, pandoc and esbuild.

Also refresh the "binary not found" message in typstGatherBinaryPath() and
the dev-call command: configure.sh no longer builds typst-gather (the cargo
step and package/typst-gather/ are gone; it is a downloaded release binary),
and neither message mentioned QUARTO_TYPST_GATHER, which both already honour.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@posit-snyk-bot

posit-snyk-bot commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

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

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

3 participants