Fix three pre-existing breakages in the example apps and task graph - #166
Fix three pre-existing breakages in the example apps and task graph#166KayleeWilliams wants to merge 4 commits into
Conversation
Two pre-existing breakages, both surfaced by running the monorepo build.
`apps/astro-example` could not build at all. `satteri` — leadtype's MDX parser —
is a napi-rs module whose loader picks a platform binding with a bare
`require("@bruits/satteri-darwin-arm64")`. Bundled into Astro's prerender
chunk that require resolves relative to `dist/.prerender/chunks/`, where the
binding isn't reachable, so static generation died with "Cannot find native
binding".
Declaring it in `resolve.external` does not work: `leadtype` is a workspace
package, so Astro's dependency crawl marks it `noExternal`, and noExternal
wins. A `resolveId` hook is unambiguous. Externalizing it also means the app
has to depend on it directly — under bun's isolated layout a transitive dep is
not resolvable from `apps/astro-example/dist/` — so `satteri` is now an
explicit dependency at the version leadtype pins.
The SvelteKit example generates into `static/` rather than `public/`, and the
two ignore lists had drifted: `feeds/`, `mcp.json`, `robots.txt`,
`schema-map.xml`, and both sitemaps were ignored under `public/` but not under
`static/`, so `bun run build` left six untracked build artifacts behind. The
lists now mirror each other and both use `apps/*/`, so a future example with
either output root is covered.
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Astro example adds ChangesAstro native parser and validation updates
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ Both fixes are sound — a few rough edges inline, none blocking.
Reviewed changes — full review of 888dab2: the Astro native-binding workaround and the .gitignore generalization.
- Astro build fix — a local Vite plugin (
astro.config.mjs6-33, registered at 36-38) markssatteriand@bruits/satteri-*external from aresolveIdhook, keeping leadtype's napi-rs MDX parser out of the prerender chunk. satterideclared on the app — added toapps/astro-example/package.jsondependenciesat^0.9.3, so the externalized import resolves fromdist/under bun's isolated layout..gitignoregeneralization — the fiveapps/sveltekit-example/static/*entries become elevenapps/*/static/*entries mirroring theapps/*/public/*list, plus a comment on why two lists exist.bun.lock— records the new dependency and incidentally correctspackages/leadtype0.3.1 → 0.4.2.
I verified the .gitignore half end-to-end and it is correct and complete. Out roots are public for astro/fumadocs/next/nuxt and static for sveltekit only (c15t and tanstack ignore public/ wholesale locally), so apps/*/public/ + apps/*/static/ is the right pair. Every out-root artifact is now covered under both roots — feeds/schema.jsonl and schema-map.xml (src/nlweb/artifacts.ts:8-9), root mcp.json (src/mcp/card.ts:228), .well-known/, both llms*.txt, robots, both sitemaps — with search-index.json / agent-readability.json covered by docs/ and the configured /changelog/*.xml feeds covered by changelog/. Nothing tracked sits under the broadened glob (apps/astro-example/public/styles.css is the only tracked file there, matched by no pattern).
I also confirmed the plugin's blast radius is safe even though it applies to every Vite environment: the client graph cannot reach satteri, since the app's only client-side leadtype imports are leadtype/webmcp and leadtype/search/client, whose Rollup entries transitively reach none of the four satteri importers.
ℹ️ Nothing builds the example apps in CI, so both of these will regress silently
ci.yml builds, lints, typechecks and tests packages/leadtype only — no job runs astro build or any other example build. That is precisely why both of these breakages sat in main unnoticed until someone ran bun run build by hand, and why nothing stops either from coming back. The .gitignore half is self-healing once merged, but the Astro plugin is a load-bearing workaround with no test pinning it.
Technical details
# Example app builds are not covered by CI
## Affected sites
- `.github/workflows/ci.yml:32-53` — the `validate` job runs `bun run --filter leadtype build`, `lint`, `check-types`, `test`. Every step is scoped to `packages/leadtype`; no example app is ever built.
- `apps/astro-example/astro.config.mjs:24-38` (new) — the workaround this PR adds has no regression guard. If a future refactor drops the plugin, or `leadtype`'s import graph changes so `satteri` arrives by another path, CI stays green.
## Required outcome
- A merge-blocking signal that fails when an example app can no longer build, covering at minimum the Astro example that this PR just repaired.
## Suggested approach (optional)
- Add a job (or matrix leg) that runs `bun run --filter astro-example build` after the `leadtype` build + `Link CLI` steps. Note the example's `build` script already chains `bun run --filter leadtype build && bun run docs:generate`, so it is close to self-contained.
- A matrix over the example apps is the fuller version, but it is meaningfully slower — one Astro leg buys most of the protection for this specific PR.
## Open questions for the human
- Is leaving the examples out of CI a deliberate cost/runtime tradeoff? If so, this is fine to close — but then the Astro workaround is worth a comment noting it is unguarded.
- Would you rather this land as a follow-up issue than expand the scope of this PR?ℹ️ Nitpicks
bun.lock:228also correctspackages/leadtype0.3.1 → 0.4.2, drift left behind by release commitb5857b4rather than anything in this PR. Harmless to carry here, but worth knowing it rode along givenci.yml:30andrelease.yml:55both runbun install --frozen-lockfile.apps/{next,nuxt,sveltekit}-example/test-results/.last-run.jsonare tracked in git — Playwright output committed in exactly the way this PR's.gitignorecomment warns about.test-results/is ignored only inapps/tanstack/.gitignore:6, and those three apps have no local.gitignore. Pre-existing and out of scope, but adjacent enough to mention; note it needsgit rm --cachedas well as an ignore entry, so it is not a one-line fix.
Claude Opus ｜ 𝕏
`bun run check-types` failed on main, and failed a *different* app each run — tanstack, then nuxt-example, then sveltekit-example. Not a type error: every example app's `check-types` script began with `bun run --filter leadtype build`, which rm -rf's and rebuilds `packages/leadtype/dist`. Turbo runs the tasks in parallel, so eight of those raced on one directory and whichever app was reading dist when another deleted it lost. Turbo already knows the dependency; the scripts were working around it. Adding `^build` to the `check-types` task builds leadtype once, ahead of every dependent, and the per-app self-build comes out. Verified with three consecutive full runs: 9 successful, 9 total, each time.
KayleeWilliams
changed the title
There was a problem hiding this comment.
Important
The check-types half of the race is fixed correctly, but the identical rm -rf dist race is still live in turbo run build.
Reviewed changes — delta since the prior review of 888dab2; the new commit 81adabb is the task-graph fix.
- Moved the leadtype build into turbo's task graph —
check-typesbecomesdependsOn: ["^build", "^check-types"](turbo.json:16-18), sopackages/leadtype/distis produced once ahead of every dependent. - Dropped the per-app self-build — the
bun run --filter leadtype build &&prefix comes out of all seven apps'check-typesscripts. - Refreshed a tracked generated artifact —
apps/next-example/app/generated/docs-mdx-map.tsis re-sorted and gains nine doc routes (mdxSlugs41 → 50), which also widensgenerateStaticParams.
I confirmed the diagnosis rather than taking it from the description: packages/leadtype/rollup.config.ts:66-71 has a cleanDist plugin doing rm("dist", { recursive: true, force: true }) in buildStart, and apps read dist by literal path — apps/next-example's docs:generate runs bun ../../packages/leadtype/dist/cli.js — so concurrent self-builds really do delete dist out from under a sibling mid-read.
The turbo semantics are right. ^build and ^check-types reference only upstream workspace dependencies, so packages/leadtype's own check-types node gets an empty prerequisite set — no self-build, no cycle. Nothing outside turbo invokes an app's check-types either: ci.yml:50 runs bun run --filter leadtype check-types only, .husky/pre-commit runs ultracite fix plus bun test, and root package.json:21 (turbo run check-types) is the sole in-repo consumer. Direct bun run --filter <app> check-types on a clean checkout does now fail, but that is the intended consequence of moving ordering into turbo, not a regression to undo — re-adding the prefix would reintroduce the race.
The docs-mdx-map.ts refresh is genuine generator output, not a hand edit: apps/next-example/scripts/build-mdx-map.mjs sorts with .sort(), the new file is sorted and the old one was not.
⚠️ turbo run build still lets seven parallel self-builds rm -rf the same dist
check-types is fixed; build is untouched. turbo.json:5-9 already had dependsOn: ["^build"], and all seven apps/*/package.json build scripts still begin with bun run --filter leadtype build. Turbo dedupes its own packages/leadtype#build node, not a raw shell command sitting inside a sibling task's script body — so after leadtype builds once, seven concurrent apps/*#build tasks each re-invoke rollup, each hitting cleanDist while a sibling is reading dist/cli.js. That is the same failure this PR exists to fix, in the task where a wiped dist is most likely to be observed.
Technical details
# The `dist` race is fixed for `check-types` but left live for `build`, `dev`, `mcp` and `test:e2e`
## Affected sites
- `turbo.json:5-9` — the `build` task (unmodified) is `dependsOn: ["^build"]`, exactly the shape that made the `check-types` prefix redundant. Nothing was removed from the app scripts to match.
- `apps/astro-example/package.json:9`, `apps/c15t-example/package.json:9`, `apps/fumadocs-example/package.json:9`, `apps/next-example/package.json:10`, `apps/nuxt-example/package.json:9`, `apps/sveltekit-example/package.json:9`, `apps/tanstack/package.json:8` — every `build` script still opens with `bun run --filter leadtype build &&`. None of these lines are in the diff.
- `apps/*/package.json` `dev` (all seven) plus `apps/tanstack/package.json:11` (`mcp`) and `:13` (`test:e2e`) carry the same prefix. `dev` is lower risk in practice because root `package.json:18` filters it to `tanstack` alone.
- `packages/leadtype/rollup.config.ts:66-71` — `cleanDist` runs `rm("dist", { recursive: true, force: true })` in `buildStart`, so each redundant invocation opens a window where `dist` does not exist.
- `packages/leadtype/package.json` `build` is `rollup -c ... && bun run --silent docs:generate` — a plain bun subprocess when reached via `--filter`, invisible to turbo's graph and cache, so there is no dedup or locking.
## Required outcome
- `turbo run build` must not be able to delete `packages/leadtype/dist` while a sibling app task is reading it — the same guarantee `81adabb` establishes for `check-types`.
- Whatever is decided, the `turbo.json` comment should describe the state the repo is actually in.
## Suggested approach (optional)
- Drop the `bun run --filter leadtype build &&` prefix from the seven `build` scripts and rely on the existing `^build`, mirroring what this commit did for `check-types`. `dev`, `mcp` and `test:e2e` are the same one-line change if you want them consistent.
## Open questions for the human
- Is the prefix kept in `build` deliberately, so `bun run --filter <app> build` stays self-sufficient for someone who bypasses turbo? That goal and race-freedom are in direct conflict, and `check-types` has now picked the other side — worth making the split explicit rather than incidental.
- No CI job runs `turbo run build` or `turbo run check-types`, so neither the fix nor the remaining hazard is exercised anywhere. Does that change how much you want to invest here versus in the CI gap?ℹ️ Nitpicks
apps/c15t-example/README.md:25-26still says "Thedev,build, andcheck-typesscripts all rebuild the localleadtypepackage", which is no longer true forcheck-types. It is the only README in the repo making that claim.apps/next-example/app/generated/docs-mdx-map.tsis stampedGenerated by scripts/build-mdx-map.mjs — do not editand is rewritten bydocs:preparebefore everybuildandcheck-types, yet it is tracked — so it re-dirties the working tree on every run, which is how it came to ride along in a commit about the task graph. Same hygiene family as this PR's.gitignorechange, and worth deciding whether it should be ignored instead. The refresh itself is a real improvement: nine doc routes were missing fromgenerateStaticParamsonmain.
Claude Opus ｜ 𝕏
| // `^build` so turbo builds leadtype's dist once, ahead of every dependent. | ||
| // Each app used to run `--filter leadtype build` itself, and eight of those | ||
| // in parallel rm -rf the same dist — failing a different app every run. |
There was a problem hiding this comment.
Two details in this comment do not hold. The count is seven, not eight — at 888dab2 exactly seven app check-types scripts carried the prefix (turbo's 9 successful is 8 check-types tasks plus leadtype's build, which is probably where the eight came from).
And "used to run --filter leadtype build itself" is still present tense for build, dev, mcp and test:e2e, which all keep the prefix — see the body section. A future reader will trust this comment to mean the race is gone repo-wide.
The Astro attribution was wrong, and wrong in the way that matters — it told the next reader the cause was Astro's dependency crawl marking `leadtype` noExternal, which cannot happen: that crawl matches only packages with an astro dep, an astro keyword, or an `astro-` name prefix, and leadtype has none. The comment now says what is actually true and checkable — this is monorepo-specific because `leadtype` is a linked workspace package — and records that the per-environment Vite keys were tried too, so nobody repeats that. The `@bruits/satteri-` arm of the resolver was unreachable: the loader reaches its platform binding through `createRequire`, so those ids are runtime CJS calls Vite never resolves, and once `satteri` is external its loader is not parsed at all. Verified by deleting it — 51 pages, exit 0 — so it is deleted. The satteri dependency now carries a note that externalizing the import makes it resolve to the app's copy rather than leadtype's, so the two ranges have to move together. The turbo comment said eight scripts; it was seven — the ninth task in turbo's output is leadtype's own `build`. It also read as though the prefix were gone repo-wide, when `build`, `dev`, `mcp`, and `test:e2e` still carry it. The race is fixed for `check-types`, which is the one turbo fans out.
There was a problem hiding this comment.
Important
Three of the four comment corrections are right. The fourth replaces an imprecise claim with a false one — and the false claim is the reason the remaining dist race won't get fixed.
Reviewed changes — delta since the prior review of 81adabb; the new commit 39223d4 rewrites four comments and deletes one dead code path. Only astro.config.mjs and turbo.json changed.
- Corrected the Astro plugin's causation comment — the inaccurate
noExternalattribution is gone, replaced by "monorepo-specific:leadtypeis a linked workspace package, so it is bundled rather than externalized", and the record thatresolve.externalplus Astro 6'senvironments.ssr/environments.prerenderequivalents were both tried is retained. - Deleted the unreachable
@bruits/satteri-resolver arm — the hook is nowif (id === NATIVE_PARSER)andNATIVE_PARSER_BINDINGis removed (astro.config.mjs:38). - Documented the
satteriversion coupling — a comment atastro.config.mjs:6-9explains that externalizing makes the import resolve to this app's copy, so leadtype's range must be kept in step. - Rewrote the
turbo.jsoncheck-typescomment — the count is corrected to seven and the scripts still carrying the self-build prefix are now named.
I verified the resolver-arm deletion is behavior-preserving rather than taking it on the comment's word. satteri/dist/index.js:13-14 statically imports only the #binding subpath, which maps to dist/binding.js; that file reaches its platform binding through createRequire(import.meta.url) followed by a runtime require('@bruits/satteri-*') — a call expression Rollup's static specifier discovery cannot see. Independently, a resolveId returning { external: true } bypasses the load/transform pipeline entirely, so index.js is never parsed once satteri is external. Nothing in the repo, in leadtype's built dist, or in packages/leadtype/src references a binding id statically. The arm was dead before this PR, and removing it is safe.
The count in the turbo.json comment is also now correct: exactly seven app check-types scripts carried the prefix, and none carry it after 81adabb.
ℹ️ Nitpicks
apps/astro-example/astro.config.mjs:6-9— the new coupling comment sits aboveconst NATIVE_PARSER = "satteri", a module specifier rather than a version range, so "Kept in step withpackages/leadtype/package.json's range" reads as though this line held the range. Someone bumpingsatteriatapps/astro-example/package.json:23never passes it.package.jsonis strict JSON and cannot carry the note itself, so there may be nowhere strictly better — but naming the dependency line explicitly in the wording would close the gap.
Claude Opus ｜ 𝕏
| // different app every run. Other scripts (`build`, `dev`, `mcp`, | ||
| // `test:e2e`) still carry the prefix; they are not run concurrently by | ||
| // turbo, so the race is fixed for `check-types`, not repo-wide. |
There was a problem hiding this comment.
they are not run concurrently by turbo is false for build, which is the one case that matters. build (turbo.json:5-9) is dependsOn: ["^build"] — upstream-only — so once packages/leadtype#build finishes, the seven apps/*#build tasks are siblings with no edge between them and run in parallel under turbo's default --concurrency of 10. All seven still open with bun run --filter leadtype build &&, a raw shell command turbo cannot dedupe, so seven rollup runs rm -rf the shared dist while siblings read dist/cli.js.
Root package.json:17 is turbo run build, so plain bun run build reaches this. That makes the new wording worse than what it replaced: the previous comment was merely vague, whereas this one tells a future maintainer the race cannot happen here.
Technical details
# `turbo.json`'s new comment asserts a scheduling guarantee turbo does not provide
## Affected sites
- `turbo.json:16-18` — claims `build`, `dev`, `mcp` and `test:e2e` "are not run concurrently by turbo". Not accurate for any of them for the stated reason.
- `turbo.json:5-9` — `build` is `dependsOn: ["^build"]`, upstream-only; nothing orders the seven `apps/*#build` tasks against each other.
- `package.json:17` — root `build` is `turbo run build`, unfiltered across all workspaces.
- `apps/astro-example/package.json:9`, `apps/c15t-example/package.json:9`, `apps/fumadocs-example/package.json:9`, `apps/next-example/package.json:10`, `apps/nuxt-example/package.json:9`, `apps/sveltekit-example/package.json:9`, `apps/tanstack/package.json:8` — all seven `build` scripts still open with `bun run --filter leadtype build &&`. None of these lines are in the diff.
- `packages/leadtype/rollup.config.ts:66-71` — `cleanDist` runs `rm("dist", { recursive: true, force: true })` in `buildStart`, opening the window in which `dist` does not exist.
## What is actually true, per script name
- `build` — **false.** Seven sibling tasks run concurrently, each re-invoking leadtype's build as a shell command invisible to turbo's graph and cache.
- `dev` — true today, but only because `package.json:18` filters to `tanstack`. `turbo.json:22-25` has no `dependsOn`, so a bare `turbo run dev` fans out to all seven, each carrying the prefix. The safety is a convention in the root script, not turbo scheduling.
- `mcp`, `test:e2e` — vacuously true: neither is a task in `turbo.json` and both exist only in `apps/tanstack`, so turbo never schedules them.
## Required outcome
- The comment must not assert a concurrency guarantee turbo does not make. Either drop the seven `build` prefixes so the claim becomes true, or state plainly that `turbo run build` still races and why that was left alone.
## Suggested approach (optional)
- Dropping `bun run --filter leadtype build &&` from the seven `build` scripts mirrors exactly what `81adabb` did for `check-types`, relying on the `^build` edge `turbo.json:5-9` already has.
## Open questions for the human
- Is the prefix kept in `build` deliberately, so `bun run --filter <app> build` stays self-sufficient for someone bypassing turbo? `check-types` has now picked the other side. If that split is intentional it is worth stating as a tradeoff rather than describing as an absent race.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@turbo.json`:
- Around line 13-20: Remove the nested `bun run --filter leadtype build`
invocation from each app’s `build` script so app builds no longer independently
rebuild `leadtype`. Rely on the root Turbo `build` dependency configuration,
including the existing `^build` behavior, to build `leadtype` once before
dependents.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6ab5f081-dee6-4b56-b2aa-e67ea1d3a17c
📒 Files selected for processing (2)
apps/astro-example/astro.config.mjsturbo.json
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: pullfrog
🔇 Additional comments (1)
apps/astro-example/astro.config.mjs (1)
6-9: LGTM!Also applies to: 21-43, 45-48
| // `^build` so turbo builds leadtype's dist once, ahead of every dependent. | ||
| // Seven app `check-types` scripts used to run `--filter leadtype build` | ||
| // themselves, and those in parallel rm -rf the same dist — failing a | ||
| // different app every run. Other scripts (`build`, `dev`, `mcp`, | ||
| // `test:e2e`) still carry the prefix; they are not run concurrently by | ||
| // turbo, so the race is fixed for `check-types`, not repo-wide. | ||
| "check-types": { | ||
| "dependsOn": ["^check-types"] | ||
| "dependsOn": ["^build", "^check-types"] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 3 \
'bun run --filter leadtype build|turbo( run)? build|--concurrency' \
--glob 'package.json' \
--glob 'turbo.json' \
--glob '*.yml' \
--glob '*.yaml' \
. || trueRepository: inthhq/leadtype
Length of output: 9595
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- turbo.json ---'
cat -n turbo.json
printf '%s\n' '--- root package.json scripts ---'
python3 - <<'PY'
import json
from pathlib import Path
p = json.loads(Path("package.json").read_text())
print(json.dumps({"scripts": p.get("scripts"), "workspaces": p.get("workspaces")}, indent=2))
PY
printf '%s\n' '--- workspace build scripts and leadtype references ---'
python3 - <<'PY'
import json
from pathlib import Path
for path in sorted(Path(".").glob("**/package.json")):
try:
data = json.loads(path.read_text())
except Exception:
continue
scripts = data.get("scripts", {})
if "build" in scripts or "leadtype" in json.dumps(data):
print(path)
for name, value in scripts.items():
if name in {"build", "dev", "mcp", "test:e2e", "check-types"} or "leadtype" in value:
print(f" {name}: {value}")
PY
printf '%s\n' '--- leadtype build implementation and CI invocations ---'
rg -n -C 4 \
'rm -rf[^\\n]*(dist|packages/leadtype)|["'\'']build["'\'']|turbo( run)? build|--concurrency|bun run --filter leadtype build' \
--glob 'package.json' \
--glob 'turbo.json' \
--glob '*.yml' \
--glob '*.yaml' \
--glob '*.sh' \
--glob '!node_modules' \
.Repository: inthhq/leadtype
Length of output: 17771
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- turbo.json ---'
cat -n turbo.json
printf '%s\n' '--- root package.json scripts ---'
python3 - <<'PY'
import json
from pathlib import Path
data = json.loads(Path("package.json").read_text())
print(json.dumps({
"scripts": data.get("scripts"),
"workspaces": data.get("workspaces"),
}, indent=2))
PY
printf '%s\n' '--- workspace scripts ---'
python3 - <<'PY'
import json
from pathlib import Path
for path in sorted(Path(".").glob("**/package.json")):
try:
data = json.loads(path.read_text())
except Exception:
continue
scripts = data.get("scripts", {})
if any(
key in scripts
for key in ("build", "dev", "mcp", "test:e2e", "check-types")
):
print(path)
for key in ("build", "dev", "mcp", "test:e2e", "check-types"):
if key in scripts:
print(f" {key}: {scripts[key]}")
PY
printf '%s\n' '--- build cleanup and CI invocations ---'
rg -n -C 4 \
'rm -rf[^[:cntrl:]]*(dist|packages/leadtype)|"build"|turbo( run)? build|--concurrency|bun run --filter leadtype build' \
--glob 'package.json' \
--glob 'turbo.json' \
--glob '*.yml' \
--glob '*.yaml' \
--glob '*.sh' \
--glob '!node_modules' \
.Repository: inthhq/leadtype
Length of output: 16741
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- leadtype build configuration ---'
rg -n -C 5 \
'output|dir|dist|clean|emptyOutDir|rm -rf|generate-docs' \
packages/leadtype \
--glob 'rollup.config.*' \
--glob 'package.json' \
--glob '*.ts' \
--glob '*.js' \
--glob '*.mjs' \
--glob '*.cjs' || true
printf '%s\n' '--- tracked CI and task-runner configuration ---'
git ls-files | rg '(^|/)(\.github/workflows/|.*(ci|workflow|turbo|package)\.(yml|yaml|json)$)' || true
printf '%s\n' '--- read-only task-graph verifier ---'
python3 - <<'PY'
import json
from pathlib import Path
root = json.loads(Path("package.json").read_text())
turbo = json.loads(
Path("turbo.json").read_text().replace(
"// `^build` so turbo builds leadtype's dist once, ahead of every dependent.\n"
"// Seven app `check-types` scripts used to run `--filter leadtype build`\n"
"// themselves, and those in parallel rm -rf the same dist — failing a\n"
"// different app every run. Other scripts (`build`, `dev`, `mcp`,\n"
"// `test:e2e`) still carry the prefix; they are not run concurrently by\n"
"// turbo, so the race is fixed for `check-types`, not repo-wide.\n",
"",
)
)
root_build = root.get("scripts", {}).get("build")
build_depends = turbo["tasks"]["build"].get("dependsOn")
nested = []
for path in sorted(Path("apps").glob("*/package.json")):
data = json.loads(path.read_text())
script = data.get("scripts", {}).get("build", "")
if "bun run --filter leadtype build" in script:
nested.append(str(path))
print(f"root_build={root_build!r}")
print(f"build_dependsOn={build_depends!r}")
print(f"nested_leadtype_builds={len(nested)}")
for path in nested:
print(f" {path}")
print(f"turbo_concurrency_configured={'concurrency' in turbo}")
PYRepository: inthhq/leadtype
Length of output: 50372
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- leadtype build configuration ---'
rg -n -C 5 \
'output|dir|dist|clean|emptyOutDir|rm -rf|generate-docs' \
packages/leadtype \
--glob 'rollup.config.*' \
--glob 'package.json' \
--glob '*.ts' \
--glob '*.js' \
--glob '*.mjs' \
--glob '*.cjs' || true
printf '%s\n' '--- tracked CI and task-runner configuration ---'
git ls-files | rg '(^|/)(\.github/workflows/|.*(ci|workflow|turbo|package)\.(yml|yaml|json)$)' || true
printf '%s\n' '--- read-only task-graph verifier ---'
python3 - <<'PY'
import json
from pathlib import Path
root = json.loads(Path("package.json").read_text())
turbo_text = Path("turbo.json").read_text()
turbo_text = "\n".join(
line for line in turbo_text.splitlines()
if not line.lstrip().startswith("//")
)
turbo = json.loads(turbo_text)
nested = []
for path in sorted(Path("apps").glob("*/package.json")):
data = json.loads(path.read_text())
script = data.get("scripts", {}).get("build", "")
if "bun run --filter leadtype build" in script:
nested.append(str(path))
print(f"root_build={root.get('scripts', {}).get('build')!r}")
print(f"build_dependsOn={turbo['tasks']['build'].get('dependsOn')!r}")
print(f"nested_leadtype_builds={len(nested)}")
for path in nested:
print(f" {path}")
print(f"turbo_concurrency_configured={'concurrency' in turbo}")
PYRepository: inthhq/leadtype
Length of output: 50374
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- verifier result ---'
for log in /tmp/coderabbit-shell-logs/shell-output-Pv20ta /tmp/coderabbit-shell-logs/shell-output-YzBcA7; do
if [ -f "$log" ]; then
rg -n -A 20 -- '--- read-only task-graph verifier ---|root_build=|nested_leadtype_builds=|turbo_concurrency_configured=' "$log" || true
fi
done
printf '%s\n' '--- leadtype build-related tracked files ---'
git ls-files packages/leadtype | rg '(^|/)(rollup|vite|tsup|webpack|package)\.[^/]+$|generate-docs|docs.*\.(ts|js|mjs|cjs)$' || true
printf '%s\n' '--- Rollup/build config ---'
for path in $(git ls-files packages/leadtype | rg '(^|/)(rollup|vite|tsup|webpack)\.[^/]+$'); do
printf '%s\n' "--- $path ---"
cat -n "$path"
done
printf '%s\n' '--- package scripts and generated output paths ---'
cat -n packages/leadtype/package.json | sed -n '195,207p'
rg -n -C 3 \
'dist/|dist\b|rm\(|rmSync|outputFile|writeFile|mkdir\(' \
packages/leadtype/scripts \
packages/leadtype/src/cli \
--glob '*.ts' \
--glob '*.js' \
--glob '*.mjs' \
--glob '*.cjs' \
| head -n 300 || trueRepository: inthhq/leadtype
Length of output: 27075
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- verifier result ---'
for log in /tmp/coderabbit-shell-logs/shell-output-Pv20ta /tmp/coderabbit-shell-logs/shell-output-YzBcA7; do
if [ -f "$log" ]; then
rg -n -A 20 \
-- '--- read-only task-graph verifier ---|root_build=|nested_leadtype_builds=|turbo_concurrency_configured=' \
"$log" || true
fi
done
printf '%s\n' '--- leadtype build-related tracked files ---'
git ls-files packages/leadtype |
rg '(^|/)(rollup|vite|tsup|webpack|package)\.[^/]+$|generate-docs|docs.*\.(ts|js|mjs|cjs)$' || true
printf '%s\n' '--- Rollup/build config ---'
while IFS= read -r path; do
printf '%s\n' "--- $path ---"
cat -n "$path"
done < <(
git ls-files packages/leadtype |
rg '(^|/)(rollup|vite|tsup|webpack)\.[^/]+$'
)
printf '%s\n' '--- package scripts and generated output paths ---'
cat -n packages/leadtype/package.json | sed -n '195,207p'
rg -n -C 3 \
'dist/|dist\b|rm\(|rmSync|outputFile|writeFile|mkdir\(' \
packages/leadtype/scripts \
packages/leadtype/src/cli \
--glob '*.ts' \
--glob '*.js' \
--glob '*.mjs' \
--glob '*.cjs' |
head -n 300 || trueRepository: inthhq/leadtype
Length of output: 1856
Remove nested leadtype builds from app build scripts. The root turbo run build has no concurrency limit, and seven app builds still invoke bun run --filter leadtype build. Each invocation removes and rewrites packages/leadtype/dist, so concurrent tasks can corrupt build outputs. Rely on ^build instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@turbo.json` around lines 13 - 20, Remove the nested `bun run --filter
leadtype build` invocation from each app’s `build` script so app builds no
longer independently rebuild `leadtype`. Rely on the root Turbo `build`
dependency configuration, including the existing `^build` behavior, to build
`leadtype` once before dependents.
The leak detector globbed `leadtype-generate-*` across the shared os.tmpdir(), snapshotting before and after the run. Both the source mirror and the generate lock live there, and vitest workers run test files in parallel, so a concurrent run's lock directory was attributed to this test and failed it. The test's own lock cannot be what it caught: generate acquires and releases it inside the awaited runCli, so it is always gone before the after-snapshot. Point the run at a private TMPDIR instead. os.tmpdir() reads TMPDIR per call, so both the lock path and the mkdtemp mirror land there, and every `leadtype-generate-*` remaining is provably this run's.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/leadtype/src/cli.test.ts`:
- Around line 2203-2234: Update the temporary-directory setup around runCli to
set TMPDIR, TMP, and TEMP to privateTmpDir, then restore or delete each variable
according to its original value in the finally block. Keep the existing leak
scan unchanged so it checks the directory actually used by the CLI across
platforms.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 283c4945-9be8-47b7-ac7f-7ce6207e70bb
📒 Files selected for processing (1)
packages/leadtype/src/cli.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: pullfrog
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/src/cli.test.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/src/cli.test.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
packages/leadtype/src/cli.test.ts
🪛 ast-grep (0.45.0)
packages/leadtype/src/cli.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
| const previousTmpDir = process.env.TMPDIR; | ||
| process.env.TMPDIR = privateTmpDir; | ||
|
|
||
| let code: number; | ||
| try { | ||
| code = await runCli( | ||
| [ | ||
| "generate", | ||
| "--src", | ||
| srcDir, | ||
| "--out", | ||
| outDir, | ||
| "--include", | ||
| "guides/**", | ||
| "--format", | ||
| "json", | ||
| ], | ||
| capture.io | ||
| ); | ||
| } finally { | ||
| if (previousTmpDir === undefined) { | ||
| delete process.env.TMPDIR; | ||
| } else { | ||
| process.env.TMPDIR = previousTmpDir; | ||
| } | ||
| } | ||
|
|
||
| const afterTempDirs = new Set( | ||
| await fg("leadtype-generate-*", { | ||
| absolute: true, | ||
| cwd: tmpdir(), | ||
| onlyDirectories: true, | ||
| }) | ||
| ); | ||
| const leakedTempDirs = [...afterTempDirs].filter( | ||
| (dir) => !beforeTempDirs.has(dir) | ||
| ); | ||
| const leakedTempDirs = await fg("leadtype-generate-*", { | ||
| absolute: true, | ||
| cwd: privateTmpDir, | ||
| onlyDirectories: true, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Check whether this repository supports or tests Windows.
rg -n -C 3 --hidden \
--glob '!node_modules/**' \
--glob '!**/dist/**' \
'(windows-latest|windows-202|process\.platform|TMPDIR|TEMP|TMP)' \
.github packages package.json 2>/dev/null || true
# Inspect the full test scope for platform guards or concurrent execution.
sed -n '2160,2260p' packages/leadtype/src/cli.test.tsRepository: inthhq/leadtype
Length of output: 8288
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- temporary-directory call sites ---'
rg -n -C 4 --hidden \
--glob '!node_modules/**' \
--glob '!**/dist/**' \
'os\.tmpdir|tmpdir\(' packages/leadtype/src
printf '%s\n' '--- relevant package and CI configuration ---'
rg -n -C 3 --hidden \
--glob '!node_modules/**' \
--glob '!**/dist/**' \
'vitest|pool|threads|forks|windows-latest|windows-202|matrix|TMPDIR|TEMP|TMP' \
.github package.json packages/leadtype/package.json 2>/dev/null || true
printf '%s\n' '--- Node runtime behavior for environment-variable precedence ---'
node - <<'JS'
const os = require("node:os");
const original = {
TMPDIR: process.env.TMPDIR,
TMP: process.env.TMP,
TEMP: process.env.TEMP,
};
const keys = ["TMPDIR", "TMP", "TEMP"];
for (const key of keys) delete process.env[key];
console.log("baseline", os.tmpdir());
for (const key of keys) {
for (const other of keys) delete process.env[other];
process.env[key] = `/probe/${key}`;
console.log(key, os.tmpdir());
}
for (const key of keys) {
if (original[key] === undefined) delete process.env[key];
else process.env[key] = original[key];
}
JSRepository: inthhq/leadtype
Length of output: 42502
Set all temporary-directory environment variables.
On Windows, os.tmpdir() ignores TMPDIR. Set and restore TMPDIR, TMP, and TEMP so the test scans the directory used by the CLI.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/leadtype/src/cli.test.ts` around lines 2203 - 2234, Update the
temporary-directory setup around runCli to set TMPDIR, TMP, and TEMP to
privateTmpDir, then restore or delete each variable according to its original
value in the finally block. Keep the existing leak scan unchanged so it checks
the directory actually used by the CLI across platforms.
There was a problem hiding this comment.
ℹ️ The test change is sound and genuinely falsifiable — two nitpicks, neither blocking.
Reviewed changes — delta since the prior review of 39223d4; the new commit add9551 touches packages/leadtype/src/cli.test.ts and nothing else.
- Scoped the temp-dir leak assertion to a private
TMPDIR— the test "cleans up mirrored sources when the generate pipeline fails" no longer snapshotsleadtype-generate-*in the sharedos.tmpdir()before and afterrunCliand diffs the two sets. It creates a private dir, pointsprocess.env.TMPDIRat it for the duration of the CLI call, and globs only that dir. - Restored the env in a
finally— including the originally-unset case, so the mutation cannot outlive therunClicall.
I mutation-tested the assertion rather than trusting that the override lands: patching createSourceMirror's cleanup (packages/leadtype/src/cli/generate.ts:2553) to a no-op makes the test fail with + ["/tmp/leadtype-cli-XXXX/leadtype-generate-XXXX/"]. So the override is effective and the assertion can still catch the leak it exists to catch — the usual failure mode for this refactor, where the glob silently starts pointing at an always-empty directory, is not present here.
No coverage is lost relative to the old glob either. Every tmpdir() call site under src/ sits inside a function rather than a module-level constant (cli/generate.ts:2244,2531, internal/generate-lock.ts:73, source/index.ts:439, openapi/index.ts:2114), so all of them re-read process.env.TMPDIR per call, and this test's code path spawns no subprocess that could inherit a stale env. The comment's stated flake mechanism also checks out: generateLockPath returns leadtype-generate-<key>.lock and tryAcquire creates it with mkdir (internal/generate-lock.ts:167), so despite the suffix it is a directory and did match the old glob's onlyDirectories: true — a concurrent worker's lock landing inside the before/after window really was misattributable. And the env mutation cannot leak: tests in this file are sequential (no .concurrent), and vitest workers hold independent process.env copies. privateTmpDir itself comes from createTempDir() (cli.test.ts:97), so afterEach (cli.test.ts:156) already removes it.
ℹ️ Nitpicks
packages/leadtype/src/cli.test.ts:2204—os.tmpdir()readsTEMP/TMPon Windows and ignoresTMPDIRentirely, so thereprivateTmpDirstays empty andexpect(leakedTempDirs).toEqual([])passes vacuously rather than failing. CI is ubuntu-only (ci.yml:19) so there is no merge-gate impact, but a Windows contributor loses the assertion silently. SettingTMP/TEMPalongsideTMPDIRwould close it if the repo cares about that platform.- The PR title and body still describe "three pre-existing breakages in the example apps and task graph". This commit is a fourth change, and a test-flake fix in
packages/leadtypeis neither an example app nor the task graph — worth a line in the body so the merge commit records why it rode along.
Note on scope: add9551 does not touch turbo.json, so the open thread on turbo.json:16-18 from the previous review is unaffected and stays open.
Claude Opus ｜ 𝕏
1 participant
Three pre-existing breakages, all surfaced by running
bun run buildandbun run check-typesacross the monorepo while working on #157. Independent of that stack, so this is a standalone PR offmain.1.
bun run check-typesfailed onmain, differently each runtanstack, then nuxt-example, then sveltekit-example. Not a type error — every example app's
check-typesscript began withbun run --filter leadtype build, whichrm -rfs and rebuildspackages/leadtype/dist. Turbo runs the tasks in parallel, so eight of those raced on one directory and whichever app was readingdistwhen another deleted it lost.Turbo already knows the dependency; the scripts were working around it. Adding
^buildto thecheck-typestask builds leadtype once, ahead of every dependent, and the per-app self-build comes out.Verified with three consecutive full runs: 9 successful, 9 total, each time.
2.
apps/astro-examplecould not build at allsatteri— leadtype's MDX parser — is a napi-rs module whose loader picks a platform binding with a barerequire("@bruits/satteri-darwin-arm64"). Bundled into Astro's prerender chunk, that require resolves relative todist/.prerender/chunks/, where the binding isn't reachable:Two things were needed, and the first alone isn't enough:
resolveIdhook rather thanresolve.external.leadtypeis a workspace package, so Astro's dependency crawl marks itnoExternal— and noExternal wins. Astro 6's Vite 7 environment keys (environments.ssr/environments.prerender) don't get around that either; I tried both.satterias an explicit dependency of the app. Under bun's isolated node_modules layout a transitive dep isn't resolvable fromapps/astro-example/dist/, so externalizing without declaring just moves the failure toCannot find package 'satteri'. Pinned at the version leadtype uses.Verified: 51 pages built, exit 0.
3. SvelteKit ignore drift
The SvelteKit example generates into
static/rather thanpublic/. The two ignore lists had drifted —feeds/,mcp.json,robots.txt,schema-map.xml, and both sitemaps were ignored underpublic/but not understatic/— sobun run buildleft six untracked build artifacts in the tree, onegit add -Aaway from being committed.The lists now mirror each other and both use
apps/*/, so a future example with either output root is covered without a third list to keep in sync.