Found this with 🤖 and saving for later so i investigate this more closely
Two guards in src/ have been permanently disabled since February 2021 because
an async predicate is called without await. !somePromise is always false,
so in both cases the guarded branch is unreachable.
Found while fixing the same class of bug in package/src/common/dependencies/
(#14731). These two are in src/ and, unlike the ones in that PR, each has real
behavioral consequences — so filing rather than folding them into that PR.
1. src/command/render/latexmk/pdf.ts:394 — Windows bibtex .aux fixup
if (isWindows) {
if (bibCommand !== "biber" && !hasTexLive()) {
// See https://github.com/rstudio/tinytex/blob/b2d1bae772f3f979e77fca9fb5efda05855b39d2/R/latex.R#L284
// Strips the '.bib' from any match ...
hasTexLive is async (src/command/render/latexmk/texlive.ts:52), so
!hasTexLive() is always false. The \bibdata{...}.bib stripping has never
run. Affects Windows + natbib (i.e. bibCommand === "bibtex") renders where
tlmgr is not on PATH. Introduced 2021-02-16 (24e6757).
The enclosing makeBibliographyIntermediates is async, so await compiles —
but I don't think a bare await is the right fix here. The comment says
"If we're on windows and auto-install isn't enabled", while the code tests
hasTexLive(), not pkgMgr.autoInstall. Comment and condition have disagreed
for five years, and it isn't clear which reflects the intent. Awaiting as-is
would switch on .aux rewriting for every affected render after five years of
those users rendering without it — worth deciding deliberately, and worth
checking whether the upstream MiKTeX/bibtex behavior the tinytex workaround
targets is still current.
2. src/tools/impl/tinytex.ts:446 — uninstall() TinyTeX check
async function uninstall(context: InstallContext) {
if (!isTinyTex()) {
context.error("Current LateX installation does not appear to be TinyTex");
return Promise.reject();
}
isTinyTex is async (same file, line 636), so the guard never fires.
Introduced 2021-02-05 (fc1e03a). Note line 91 in the same file calls it
correctly (return await isTinyTex();), so this is an inconsistency within one
file.
Scope of the impact is narrower than it first looks: the removal step targets
tinyTexInstallDir(), a fixed path (~/.TinyTeX, ~/Library/TinyTeX,
%APPDATA%\TinyTeX), so a system TeX Live is never deleted. The real exposure is
the --update-path branch, which calls removePath(await texLiveContext(true))
→ tlmgr path remove against the detected installation. With a system TeX Live
present and no Quarto TinyTeX, that can strip the system install's symlinks
before failing with "Couldn't find install directory".
Again, a bare await may not be safe. It would activate a predicate that has
never executed, and isTinyTex() looks fragile for the default layout:
- The directory-name check is
root.match(/[/\\][Tt]iny[Tt]e[Xx][/\\]?/), which requires / or \
immediately before TinyTeX. The Linux install dir is ~/.TinyTeX — a .
intervenes, so this fails and the function depends entirely on the
texmf-dist/web2c/fmtutil.cnf fallback.
texLiveRoot() derives the root with three .. on Windows but four
elsewhere. That only reconciles if tlmgr resolves through a symlink to
tlmgr.pl on non-Windows; I could not verify that without a real install.
If either misfires, awaiting turns a currently-working quarto uninstall tinytex
into one that refuses to run. The guard and its predicate probably want fixing
together, with testing on real installs on all three platforms.
Prevention
The four instances in #14731 compared the promise to undefined, which is
TS error 2367 — deno check over package/src would have caught them, and
nothing in CI runs it today.
These two are different: !promise is perfectly valid TypeScript, so no
type-check pass catches them. They need a lint rule — typescript-eslint's
no-misused-promises is the canonical one; I'm not sure deno lint has an
equivalent, which is worth checking before assuming CI can prevent recurrence.
Completeness
I scanned src/ and package/src/ for async functions called without await
in a boolean position, in two passes: direct use in a condition, and assignment
to a variable that is then tested. Six instances total — the four in #14731 plus
these two. Everything else flagged was a false positive (a sync function sharing
a name with an async one elsewhere).
Not built or test-run; the analysis above is from reading the code.
Found this with 🤖 and saving for later so i investigate this more closely
Two guards in
src/have been permanently disabled since February 2021 becausean
asyncpredicate is called withoutawait.!somePromiseis alwaysfalse,so in both cases the guarded branch is unreachable.
Found while fixing the same class of bug in
package/src/common/dependencies/(#14731). These two are in
src/and, unlike the ones in that PR, each has realbehavioral consequences — so filing rather than folding them into that PR.
1.
src/command/render/latexmk/pdf.ts:394— Windows bibtex.auxfixuphasTexLiveisasync(src/command/render/latexmk/texlive.ts:52), so!hasTexLive()is alwaysfalse. The\bibdata{...}.bibstripping has neverrun. Affects Windows +
natbib(i.e.bibCommand === "bibtex") renders wheretlmgris not on PATH. Introduced 2021-02-16 (24e6757).The enclosing
makeBibliographyIntermediatesisasync, soawaitcompiles —but I don't think a bare
awaitis the right fix here. The comment says"If we're on windows and auto-install isn't enabled", while the code tests
hasTexLive(), notpkgMgr.autoInstall. Comment and condition have disagreedfor five years, and it isn't clear which reflects the intent. Awaiting as-is
would switch on
.auxrewriting for every affected render after five years ofthose users rendering without it — worth deciding deliberately, and worth
checking whether the upstream MiKTeX/bibtex behavior the tinytex workaround
targets is still current.
2.
src/tools/impl/tinytex.ts:446—uninstall()TinyTeX checkisTinyTexisasync(same file, line 636), so the guard never fires.Introduced 2021-02-05 (fc1e03a). Note line 91 in the same file calls it
correctly (
return await isTinyTex();), so this is an inconsistency within onefile.
Scope of the impact is narrower than it first looks: the removal step targets
tinyTexInstallDir(), a fixed path (~/.TinyTeX,~/Library/TinyTeX,%APPDATA%\TinyTeX), so a system TeX Live is never deleted. The real exposure isthe
--update-pathbranch, which callsremovePath(await texLiveContext(true))→
tlmgr path removeagainst the detected installation. With a system TeX Livepresent and no Quarto TinyTeX, that can strip the system install's symlinks
before failing with "Couldn't find install directory".
Again, a bare
awaitmay not be safe. It would activate a predicate that hasnever executed, and
isTinyTex()looks fragile for the default layout:root.match(/[/\\][Tt]iny[Tt]e[Xx][/\\]?/), which requires/or\immediately before
TinyTeX. The Linux install dir is~/.TinyTeX— a.intervenes, so this fails and the function depends entirely on the
texmf-dist/web2c/fmtutil.cnffallback.texLiveRoot()derives the root with three..on Windows but fourelsewhere. That only reconciles if
tlmgrresolves through a symlink totlmgr.plon non-Windows; I could not verify that without a real install.If either misfires, awaiting turns a currently-working
quarto uninstall tinytexinto one that refuses to run. The guard and its predicate probably want fixing
together, with testing on real installs on all three platforms.
Prevention
The four instances in #14731 compared the promise to
undefined, which isTS error 2367 —
deno checkoverpackage/srcwould have caught them, andnothing in CI runs it today.
These two are different:
!promiseis perfectly valid TypeScript, so notype-check pass catches them. They need a lint rule — typescript-eslint's
no-misused-promisesis the canonical one; I'm not suredeno linthas anequivalent, which is worth checking before assuming CI can prevent recurrence.
Completeness
I scanned
src/andpackage/src/for async functions called withoutawaitin a boolean position, in two passes: direct use in a condition, and assignment
to a variable that is then tested. Six instances total — the four in #14731 plus
these two. Everything else flagged was a false positive (a sync function sharing
a name with an async one elsewhere).
Not built or test-run; the analysis above is from reading the code.