Skip to content

Commit 34f7090

Browse files
committed
Merge branch 'beads/bd-6rczoll3-ts-packages-build-step'
2 parents 93ff165 + 79ec9e2 commit 34f7090

7 files changed

Lines changed: 404 additions & 46 deletions

File tree

CLAUDE.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,13 @@ cargo xtask verify # Full verification (Rust + hub-client builds + tes
460460
This runs:
461461
1. `cargo build --workspace` - Build all Rust crates
462462
2. `cargo nextest run --workspace` - Run all Rust tests
463-
3. `cd hub-client && npm run build:all` - Build hub-client (includes WASM)
464-
4. `cd hub-client && npm run test:ci` - Run hub-client tests
463+
3. `npm run build --if-present -w ts-packages/...` - Build the ts-packages
464+
workspaces, then smoke-check the quarto-hub-mcp server (`node
465+
dist/index.js --help` must survive ESM module resolution). These `dist/`
466+
outputs are consumed at runtime by Node consumers (the MCP server);
467+
hub-client bundles ts-packages from source, so nothing else builds them.
468+
4. `cd hub-client && npm run build:all` - Build hub-client (includes WASM)
469+
5. `cd hub-client && npm run test:ci` - Run hub-client tests
465470

466471
**Skip options** (for faster iteration):
467472
```bash
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
# ts-packages build step for xtask build-all / verify (bd-6rczoll3)
2+
3+
## Overview
4+
5+
`cargo xtask build-all` documents itself as the fresh-clone source of truth,
6+
but no step in it (nor in `cargo xtask verify`) ever builds `ts-packages/*`.
7+
This goes unnoticed because hub-client consumes ts-packages **from source**:
8+
every ts-package's exports map points `types`/`source` at `./src/index.ts`,
9+
hub-client's `tsc -b` has no project references into ts-packages, and Vite
10+
bundles from source. The one runtime consumer of ts-package `dist/` output is
11+
the **quarto-hub-mcp server**, which Node executes directly and which resolves
12+
workspace imports through the `"import": "./dist/index.js"` export condition.
13+
A missing/stale dependency dist (observed: `@quarto/quarto-sync-client` had no
14+
`dist/` at all) kills the MCP at startup with `ERR_MODULE_NOT_FOUND`, which
15+
the MCP harness surfaces as a `-32000` error.
16+
17+
Observed tree state at diagnosis time (2026-06-12): 7 of 9 ts-packages had no
18+
`dist/` (`annotated-qmd`, `pandoc-types`, `preview-renderer`,
19+
`preview-runtime`, `quarto-sync-client`, `sync-test-harness`,
20+
`wasm-js-bridge`); the two that did had been built by hand.
21+
22+
## Design
23+
24+
1. **New shared helper** `crates/xtask/src/ts_packages.rs`:
25+
- `package_dirs(project_root) -> Vec<PathBuf>`: sorted list of
26+
`ts-packages/*` subdirectories containing a `package.json`; empty when
27+
`ts-packages/` is absent (old branches). Unit-tested with tempdir
28+
fixtures.
29+
- The build invocation is one root-level npm run:
30+
`npm run build --if-present -w ts-packages/<a> -w ts-packages/<b> ...`
31+
`--if-present` covers packages without a `build` script
32+
(`sync-test-harness`, `wasm-js-bridge`). Build **order doesn't matter**:
33+
since types resolve via `src/`, each package's `tsc` compiles without its
34+
dependencies' `dist/` present.
35+
2. **build-all**: new step between `npm install` and the hub-client build,
36+
with a `--skip-ts-packages-build` flag.
37+
3. **verify**: new step 6 (before the hub-client build; `TOTAL_STEPS` 12→13,
38+
later steps renumbered), same `--skip-ts-packages-build` flag. The step
39+
builds ts-packages, then **smoke-checks the MCP server**:
40+
`node ts-packages/quarto-hub-mcp/dist/index.js --help` must exit 0. The
41+
`--help` path runs after the whole ESM graph links, prints usage, and
42+
exits 0 — no hang, no network, no auth env needed. A broken module graph
43+
exits 1 with `ERR_MODULE_NOT_FOUND` before any code runs.
44+
4. **Cross-platform fix**: quarto-hub-mcp's build script is
45+
`tsc && chmod +x dist/index.js`, which fails on Windows (no `chmod`).
46+
Replace with a portable node one-liner
47+
(`node -e "fs.chmodSync(...)"`-style, no-op semantics on Windows) so the
48+
new verify/build-all step doesn't break Windows.
49+
5. **Docs**: update step lists in `build_all.rs` / `verify.rs` module docs,
50+
the `Verify` / `BuildAll` doc comments in `main.rs`, and the verify
51+
description in `CLAUDE.md`.
52+
53+
Out of scope (deliberately): TypeScript project references between
54+
ts-packages and hub-client; building hub-client/trace-viewer/q2-preview-spa
55+
from the root `npm run build --workspaces` script (they keep their dedicated
56+
steps, which include WASM and embedding concerns the root script doesn't
57+
know about).
58+
59+
## Work Items
60+
61+
### Phase 1: Red (failing checks first)
62+
63+
- [x] Reproduce the failure mechanically: `node
64+
ts-packages/quarto-hub-mcp/dist/index.js --help` exits 1 with
65+
`ERR_MODULE_NOT_FOUND` for `@quarto/quarto-sync-client/dist/index.js`
66+
(recorded below)
67+
- [x] Write unit tests for `ts_packages::workspace_paths` (tempdir fixtures:
68+
sorted result, skips dirs without package.json, missing ts-packages/
69+
→ empty, ignores plain files)
70+
71+
### Phase 2: Implementation
72+
73+
- [x] Add `crates/xtask/src/ts_packages.rs` helper + tests
74+
- [x] Make quarto-hub-mcp's `build` script cross-platform (drop bare `chmod`;
75+
now `node -e "import('node:fs').then(fs => fs.chmodSync(...))"`)
76+
- [x] build-all: add ts-packages step + `--skip-ts-packages-build` flag
77+
- [x] verify: add step 6 (build + MCP smoke check), renumber steps 6–12 →
78+
7–13, add `--skip-ts-packages-build` flag
79+
- [x] Update doc comments in `main.rs`, module docs, `CLAUDE.md`
80+
81+
### Phase 3: Verification
82+
83+
- [x] `cargo nextest run -p xtask` — 4 new unit tests pass
84+
- [x] End-to-end: verify with all other steps skipped → ts-packages step
85+
builds 7 packages (2 have no build script, covered by `--if-present`)
86+
and the MCP smoke check passes; same for the build-all step
87+
- [x] End-to-end: `node ts-packages/quarto-hub-mcp/dist/index.js --help`
88+
exits 0 and prints usage (the original failing command from the bug
89+
report)
90+
- [x] `cargo build --workspace` (clean) + `cargo nextest run --workspace`
91+
(9967 passed, 196 skipped)
92+
- [x] `cargo xtask verify --skip-hub-build` (also exercises the new step
93+
for real) — all steps passed, exit 0
94+
- [ ] Close bd-6rczoll3
95+
96+
## Evidence
97+
98+
### Red state (before fix, 2026-06-12)
99+
100+
```
101+
$ node ts-packages/quarto-hub-mcp/dist/index.js --help
102+
Error [ERR_MODULE_NOT_FOUND]: Cannot find module
103+
'/Users/cscheid/repos/github/quarto-dev/q2/node_modules/@quarto/quarto-sync-client/dist/index.js'
104+
imported from .../ts-packages/quarto-hub-mcp/dist/connection-manager.js
105+
exit code: 1
106+
```
107+
108+
(Tree state: 7 of 9 ts-packages had no `dist/`. The `--help` flag still
109+
fails because ESM module resolution happens before any code runs — which is
110+
exactly why it works as a smoke check.)
111+
112+
### Green state (after fix)
113+
114+
```
115+
$ cargo xtask verify --skip-rust-build --skip-rust-tests --skip-hub-build \
116+
--skip-hub-tests --skip-trace-viewer-build --skip-trace-viewer-tests \
117+
--skip-treesitter-tests --skip-shared-package-tests --skip-q2-preview-spa-build
118+
━━━ Step 6/13: Building ts-packages workspaces ━━━
119+
> @quarto/annotated-qmd@0.1.1 build ... (7 packages built via
120+
npm run build --if-present -w ts-packages/<pkg> ...)
121+
↳ Smoke-checking quarto-hub-mcp module graph (node dist/index.js --help)...
122+
Usage: quarto-hub-mcp --server <url> [--read-only] [--redirect-port <N>]
123+
✓ quarto-hub-mcp module graph loads
124+
✓ ts-packages build complete
125+
...
126+
✓ All verification steps passed!
127+
128+
$ node ts-packages/quarto-hub-mcp/dist/index.js --help
129+
Usage: quarto-hub-mcp --server <url> [--read-only] [--redirect-port <N>]
130+
MCP --help exit code: 0
131+
```
132+
133+
Output inspected: usage text printed, exit 0, and
134+
`ts-packages/quarto-sync-client/dist/index.js` (the originally missing
135+
module) now exists.

crates/xtask/src/build_all.rs

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
//! to produce a working build:
66
//!
77
//! 1. `npm install` at the repo root (npm workspaces)
8-
//! 2. Build hub-client (includes WASM via `npm run build:all`)
9-
//! 3. Build trace-viewer (if present; Phase 4.3+)
10-
//! 4. Build q2-preview-spa (if present; q2-preview Phase A.4 / bd-501n)
11-
//! 5. Build the Rust workspace (`cargo build --workspace`)
8+
//! 2. Build the ts-packages workspaces (`dist/` for Node consumers like
9+
//! the quarto-hub-mcp server; bd-6rczoll3 — see `ts_packages.rs`)
10+
//! 3. Build hub-client (includes WASM via `npm run build:all`)
11+
//! 4. Build trace-viewer (if present; Phase 4.3+)
12+
//! 5. Build q2-preview-spa (if present; q2-preview Phase A.4 / bd-501n)
13+
//! 6. Build the Rust workspace (`cargo build --workspace`)
1214
//!
1315
//! Both SPAs (trace-viewer + q2-preview-spa) must build *before* the
1416
//! Rust workspace because `quarto-trace-server` and `quarto-preview`
@@ -23,6 +25,8 @@ pub struct BuildAllConfig {
2325
/// Skip `npm install`. Useful when running in a loop where dependencies
2426
/// haven't changed.
2527
pub skip_npm_install: bool,
28+
/// Skip the ts-packages build step. No-op when `ts-packages/` is absent.
29+
pub skip_ts_packages_build: bool,
2630
/// Skip the hub-client build step.
2731
pub skip_hub_build: bool,
2832
/// Skip the trace-viewer build step. No-op until Phase 4.3 lands.
@@ -40,6 +44,7 @@ impl Default for BuildAllConfig {
4044
fn default() -> Self {
4145
Self {
4246
skip_npm_install: false,
47+
skip_ts_packages_build: false,
4348
skip_hub_build: false,
4449
skip_trace_viewer_build: false,
4550
skip_q2_preview_spa_build: false,
@@ -52,9 +57,14 @@ impl Default for BuildAllConfig {
5257
/// Run the build-all command.
5358
pub fn run(config: &BuildAllConfig) -> Result<()> {
5459
let project_root = find_project_root()?;
60+
let ts_workspaces = crate::ts_packages::workspace_paths(&project_root);
5561

5662
let steps: Vec<(&str, bool)> = vec![
5763
("npm install (root workspaces)", !config.skip_npm_install),
64+
(
65+
"ts-packages build",
66+
!config.skip_ts_packages_build && !ts_workspaces.is_empty(),
67+
),
5868
("hub-client build (WASM + TS)", !config.skip_hub_build),
5969
(
6070
"trace-viewer build",
@@ -85,6 +95,29 @@ pub fn run(config: &BuildAllConfig) -> Result<()> {
8595
println!("✓ npm install complete");
8696
}
8797

98+
// Step: ts-packages build (bd-6rczoll3)
99+
//
100+
// Emits the `dist/` output that Node consumers (the quarto-hub-mcp
101+
// server) resolve at runtime; nothing else in this sequence builds it
102+
// because hub-client bundles ts-packages from source.
103+
if !config.skip_ts_packages_build && !ts_workspaces.is_empty() {
104+
step_idx += 1;
105+
banner(step_idx, total, "Building ts-packages workspaces");
106+
let mut npm_args: Vec<&str> = vec!["run", "build", "--if-present"];
107+
for workspace in &ts_workspaces {
108+
npm_args.push("-w");
109+
npm_args.push(workspace);
110+
}
111+
run_command(
112+
"npm",
113+
&npm_args,
114+
&project_root,
115+
None,
116+
"ts-packages build failed",
117+
)?;
118+
println!("✓ ts-packages build complete");
119+
}
120+
88121
// Step: hub-client build (WASM + TS)
89122
if !config.skip_hub_build {
90123
step_idx += 1;

crates/xtask/src/main.rs

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ mod stage_doc_examples;
2727
mod switch_task;
2828
mod test;
2929
mod treesitter_crlf;
30+
mod ts_packages;
3031
mod util;
3132
mod verify;
3233

@@ -138,8 +139,9 @@ enum Command {
138139
/// 3. Build all Rust crates (cargo build --workspace, with -D warnings)
139140
/// 4. Test tree-sitter grammars (tree-sitter test)
140141
/// 5. Run all Rust tests (cargo nextest run --workspace, with -D warnings)
141-
/// 6. Build hub-client including WASM (npm run build:all)
142-
/// 7. Run hub-client tests (npm run test:ci)
142+
/// 6. Build ts-packages workspaces + quarto-hub-mcp smoke check
143+
/// 7. Build hub-client including WASM (npm run build:all)
144+
/// 8. Run hub-client tests (npm run test:ci)
143145
///
144146
/// Use this before pushing to ensure nothing will fail in CI.
145147
Verify {
@@ -151,6 +153,10 @@ enum Command {
151153
#[arg(long)]
152154
skip_rust_tests: bool,
153155

156+
/// Skip the ts-packages build + quarto-hub-mcp smoke check.
157+
#[arg(long)]
158+
skip_ts_packages_build: bool,
159+
154160
/// Skip hub-client build.
155161
#[arg(long)]
156162
skip_hub_build: bool,
@@ -221,15 +227,20 @@ enum Command {
221227
/// of truth for what a fresh checkout (or CI) needs to produce a working
222228
/// build:
223229
/// 1. npm install at the repo root (npm workspaces)
224-
/// 2. hub-client build (includes WASM)
225-
/// 3. trace-viewer build (if present; Phase 4.3+)
226-
/// 4. q2-preview-spa build (if present; q2-preview Phase A.4)
227-
/// 5. cargo build --workspace
230+
/// 2. ts-packages build (dist/ for Node consumers like quarto-hub-mcp)
231+
/// 3. hub-client build (includes WASM)
232+
/// 4. trace-viewer build (if present; Phase 4.3+)
233+
/// 5. q2-preview-spa build (if present; q2-preview Phase A.4)
234+
/// 6. cargo build --workspace
228235
BuildAll {
229236
/// Skip `npm install`.
230237
#[arg(long)]
231238
skip_npm_install: bool,
232239

240+
/// Skip the ts-packages build.
241+
#[arg(long)]
242+
skip_ts_packages_build: bool,
243+
233244
/// Skip the hub-client build.
234245
#[arg(long)]
235246
skip_hub_build: bool,
@@ -288,6 +299,7 @@ fn main() -> Result<()> {
288299
Command::Verify {
289300
skip_rust_build,
290301
skip_rust_tests,
302+
skip_ts_packages_build,
291303
skip_hub_build,
292304
skip_hub_tests,
293305
skip_trace_viewer_build,
@@ -302,6 +314,7 @@ fn main() -> Result<()> {
302314
let config = verify::VerifyConfig {
303315
skip_rust_build,
304316
skip_rust_tests,
317+
skip_ts_packages_build,
305318
skip_hub_build,
306319
skip_hub_tests,
307320
skip_trace_viewer_build,
@@ -320,6 +333,7 @@ fn main() -> Result<()> {
320333
Command::BuildQ2PreviewSpa {} => build_q2_preview_spa::run(),
321334
Command::BuildAll {
322335
skip_npm_install,
336+
skip_ts_packages_build,
323337
skip_hub_build,
324338
skip_trace_viewer_build,
325339
skip_q2_preview_spa_build,
@@ -328,6 +342,7 @@ fn main() -> Result<()> {
328342
} => {
329343
let config = build_all::BuildAllConfig {
330344
skip_npm_install,
345+
skip_ts_packages_build,
331346
skip_hub_build,
332347
skip_trace_viewer_build,
333348
skip_q2_preview_spa_build,

0 commit comments

Comments
 (0)