From 4a1804cda34a57147d3f43ed30486a70bbd3bf95 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 21:10:18 +0000 Subject: [PATCH] Review round 1 for the Node build module: build depth (1490), bin scripts for the test script (1491), toolchain.reported and its stream (1483, 1488); delta part 3 of 4 --- modules/mesh-build-node/README.md | 18 +++--- .../cmd/mesh-build-node/build.go | 21 ++++++- .../cmd/mesh-build-node/build_test.go | 45 +++++++++++++-- .../cmd/mesh-build-node/check.go | 2 +- .../cmd/mesh-build-node/check_test.go | 38 +++++++++++++ .../cmd/mesh-build-node/emit_test.go | 55 +++++++++++++++++++ 6 files changed, 164 insertions(+), 15 deletions(-) create mode 100644 modules/mesh-build-node/cmd/mesh-build-node/emit_test.go diff --git a/modules/mesh-build-node/README.md b/modules/mesh-build-node/README.md index 732b224c..a2ec6a1f 100644 --- a/modules/mesh-build-node/README.md +++ b/modules/mesh-build-node/README.md @@ -45,21 +45,23 @@ toolchain install: a long step answers at once with its id and ends on `.< - `build_node_fetch` {repository, commit, ref} (issue 1489): the commit over ssh as the module's account (`buildstep.FetchCommit`, remote `ssh://mesh-forge//.git`) into `//tree`, then for each TypeScript module (a module.json with a typescript artifact) the packages its package.json names beyond the SDK, - `typescript` and `@types/*` (the toolchain carries those): none installs nothing; some need a package-lock.json, else - the step fails naming the directory, and are installed as the build agent does (scratch copy without the SDK, + `typescript` and `@types/*` (the toolchain carries those): none installs nothing; some need a package-lock.json, and + are installed as the build agent does (scratch copy without the SDK, devDependencies and scripts; `npm ci --omit=dev --omit=peer --ignore-scripts --no-bin-links`; copies of the SDK - removed). A dependency on another `@novox/` package fails the step: a fetch holds no registry credential - (ADR 0466 ยง4). Every command runs through `forgekey.ExecRunnerClean(0)` with PATH, HOME, the locale and its own + removed). The mesh's registry reaches `npm ci` through a private config file (`--userconfig`), so + other `@novox/` packages install. A module with packages and no lock does not fail the fetch: its result lists it + under `refused` with the reason, and a build of that bundle fails naming it (issue 1459). Every command runs through `forgekey.ExecRunnerClean(0)` with PATH, HOME, the locale and its own words, nothing of this process's environment. Workspaces older than a day are removed at each fetch. - `build_node_test` {workspace, module, level} (issue 1491): the unit level is the type-check (the toolchain's tsc, the package's `tsconfig.json` or the build agent's flags over its .ts files) and the `test` script; the integration level is the `test:integration` script, the type-check being its setup. Bounds (the Go module's; ADR 0445 names none for - TypeScript): unit 5 s, integration 30 s setup and 60 s running. A level over its bound says `over_bound` and passes; + TypeScript): unit 5 s, integration 30 s setup and 60 s running. The test script finds `tsc` and `esbuild` on its PATH through `//bin` (shell scripts calling the toolchain's). A level over its bound says `over_bound` and passes; a failed one finishes the step with `passed` false and the output (the step fails only when the tests could not be run). Neither script nor .ts files: no tests. The toolchain makes no bin links, so a script runs `node` and files, not `tsc` by name. - `build_node_build` {workspace, module, artifact} (issue 1490): the build agent's tsc and esbuild lines (mesh-controller internal/builder, cited in `build.go`), the launchers, `package.json`, `buildstep.Pack`, and the - digest. No network. The toolchain's packages (not typescript or esbuild) are copied to `//node_modules`, - above the tree, which is where tsc, esbuild and node find `@novox/mesh-sdk` and the types by walking up; a copy and - not a link, because the mesh makes no symlink. A bundle that keeps an external gets them in its own `node_modules`. + digest. No network. The build runs in a staging copy of the module at `//stage-/modules/module`, with the + toolchain's packages (not typescript or esbuild) copied to `stage-/node_modules`: the build agent's depth + (`/app/modules/module`, `/app/node_modules`), so esbuild's `../../node_modules/...` path comments, and the bundle's + digest, equal its. Copies, not links, because the mesh makes no symlink. A bundle that keeps an external gets them in its own `node_modules`. The packed archive is written beside the tree as `.tar.gz`. diff --git a/modules/mesh-build-node/cmd/mesh-build-node/build.go b/modules/mesh-build-node/cmd/mesh-build-node/build.go index 1d00bffb..a5f9519a 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/build.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/build.go @@ -83,8 +83,25 @@ func (m *Module) build(ctx context.Context, args map[string]any) (any, error) { if err != nil { return nil, err } - if err := m.provide(root); err != nil { - return nil, fmt.Errorf("providing the toolchain's packages to the workspace: %w", err) + if why, err := lockProblem(dir, module); err != nil || why != "" { + if err == nil { + err = fmt.Errorf("%s was refused by the fetch: %s", name, why) + } + return nil, err + } + // Built at the build agent's depth (builder.go: the module at /app/modules/module, the packages at /app/node_modules), + // so the path comments esbuild writes, `../../node_modules/...`, and with them the bundle's bytes equal its. + stage := filepath.Join(root, "stage-"+a.Name) + if err := os.RemoveAll(stage); err != nil { + return nil, err + } + src := dir + dir = filepath.Join(stage, "modules", "module") + if err := copyTree(src, dir); err != nil { + return nil, fmt.Errorf("staging the module: %w", err) + } + if err := copyPackages(filepath.Join(m.toolchainDir(), "node_modules"), filepath.Join(stage, "node_modules")); err != nil { + return nil, fmt.Errorf("providing the toolchain's packages to the build: %w", err) } out, final := Out(a.Name), Out(a.Name)+".bundled" if err := os.MkdirAll(filepath.Join(dir, out), 0o755); err != nil { diff --git a/modules/mesh-build-node/cmd/mesh-build-node/build_test.go b/modules/mesh-build-node/cmd/mesh-build-node/build_test.go index edba7ff2..e59aefb2 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/build_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/build_test.go @@ -76,7 +76,7 @@ func TestTscAndEsbuildRunWithTheBuildAgentsFlags(t *testing.T) { if _, err := m.build(context.Background(), buildArgs()); err != nil { t.Fatal(err) } - dir := filepath.Join(work, "w1", "tree", "modules", "m") + dir := filepath.Join(work, "w1", "stage-tools", "modules", "module") nm := filepath.Join(m.toolchainDir(), "node_modules") banner := "--banner:js=import { createRequire as __meshRequire } from 'node:module'; const require = __meshRequire(import.meta.url);" common := []string{"--bundle", "--platform=node", "--format=esm", "--target=node22", @@ -143,7 +143,7 @@ func TestTheBundleIsPackedWithItsLauncherAndPackageJson(t *testing.T) { if err != nil || again.(BuildResult).Digest != r.Digest { t.Errorf("a second build: %v, digest %v, want %s", err, again, r.Digest) } - launcher, _ := os.ReadFile(filepath.Join(filepath.Dir(r.Archive), "tree", "modules", "m", ".mesh-build", "tools", "tools", "index.serve.mjs")) + launcher, _ := os.ReadFile(filepath.Join(filepath.Dir(r.Archive), "stage-tools", "modules", "module", ".mesh-build", "tools", "tools", "index.serve.mjs")) if !strings.Contains(string(launcher), `import { serveRegisteredOverStdio } from "@novox/mesh-sdk/stdio";`) || !strings.Contains(string(launcher), `await import("./index.js");`) { t.Errorf("launcher = %q", launcher) } @@ -162,7 +162,7 @@ func TestNoSymlinkIsMadeInTheWorkspace(t *testing.T) { } } -// 1490: the bundle's SDK comes from the workspace's node_modules, copied beside the tree, and the packed bundle holds +// 1490: the bundle's SDK comes from the staging directory's node_modules, two levels above the module, and the packed bundle holds // the toolchain's packages only when the artifact keeps one external, never typescript or esbuild. func TestThePackagesAreCopiedBesideTheTreeAndIntoTheBundleOnlyForExternals(t *testing.T) { m, _, work := builtMachine(t, ``) @@ -170,7 +170,7 @@ func TestThePackagesAreCopiedBesideTheTreeAndIntoTheBundleOnlyForExternals(t *te if err != nil { t.Fatal(err) } - nm := filepath.Join(work, "w1", "node_modules") + nm := filepath.Join(work, "w1", "stage-tools", "node_modules") for _, p := range []string{"@novox/mesh-sdk/dist/stdio.js", "@types/node/index.d.ts", "nats/lib/nats.js"} { if _, err := os.Stat(filepath.Join(nm, p)); err != nil { t.Errorf("the workspace's node_modules lacks %s", p) @@ -217,3 +217,40 @@ func TestOnlyATypeScriptBundleOfTheModuleIsBuilt(t *testing.T) { } } } + +// 1490 (review 7): the build agent runs a module mounted at /app/modules/module with the toolchain's packages at +// /app/node_modules (mesh-controller builder.go `within`), so esbuild writes the path comments of what it inlines +// relative to the module's directory as `../../node_modules/...`. The step builds at that depth: in a staging +// directory whose module directory has the packages two levels above it, whatever the module's place in the +// repository, so the bundle's bytes (and its digest) equal the build agent's. +func TestTheBuildRunsWithThePackagesTwoLevelsAboveTheModule(t *testing.T) { + for _, module := range []string{"modules/m", "."} { + m, f, work := newMachine(t) + files := map[string]string{"module.json": strings.Replace(bundleManifest, "%s", "", 1), "tools/index.ts": "export {}"} + if module != "." { + files = map[string]string{"modules/m/module.json": strings.Replace(bundleManifest, "%s", "", 1), "modules/m/tools/index.ts": "export {}"} + } + workspaceWith(t, work, files) + f.effect = esbuildEffect(t) + args := buildArgs() + args["module"] = module + if _, err := m.build(context.Background(), args); err != nil { + t.Fatalf("%s: %v", module, err) + } + for _, c := range f.calls { + rel, err := filepath.Rel(c.dir, filepath.Join(c.dir, "..", "..", "node_modules")) + if err != nil || rel != "../../node_modules" { + t.Fatalf("%s: relative path %q (%v)", module, rel, err) + } + if _, err := os.Stat(filepath.Join(c.dir, "..", "..", "node_modules", "@novox", "mesh-sdk", "dist", "stdio.js")); err != nil { + t.Errorf("%s: %s ran in %s, which has no packages at ../../node_modules: %v", module, filepath.Base(c.name), c.dir, err) + } + if filepath.Base(c.dir) != "module" || filepath.Base(filepath.Dir(c.dir)) != "modules" { + t.Errorf("%s: ran in %s, want .../modules/module", module, c.dir) + } + } + if !strings.HasPrefix(f.calls[0].dir, filepath.Join(work, "w1")) { + t.Errorf("%s: staged outside the workspace: %s", module, f.calls[0].dir) + } + } +} diff --git a/modules/mesh-build-node/cmd/mesh-build-node/check.go b/modules/mesh-build-node/cmd/mesh-build-node/check.go index c56327f9..672a3539 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/check.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/check.go @@ -148,7 +148,7 @@ func (m *Module) test(ctx context.Context, args map[string]any) (any, error) { } if hasScript && len(failed) == 0 { start := m.now() - env := []string{"npm_config_cache=" + filepath.Join(m.Toolchain.Paths.Cache, "npm")} + env := []string{"PATH=" + binDir(root) + ":" + pathOrDefault(), "npm_config_cache=" + filepath.Join(m.Toolchain.Paths.Cache, "npm")} r := m.runIn(ctx, dir, env, "npm", scriptArgs(script)...) run = m.now().Sub(start) if err := ran(script, r); err != nil { diff --git a/modules/mesh-build-node/cmd/mesh-build-node/check_test.go b/modules/mesh-build-node/cmd/mesh-build-node/check_test.go index f7402838..dc3e79fa 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/check_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/check_test.go @@ -2,6 +2,8 @@ package main import ( "context" + "os" + "path/filepath" "strings" "testing" "time" @@ -159,3 +161,39 @@ func TestAWorkspaceOutsideTheWorkDirectoryIsRefused(t *testing.T) { } } } + +// 1491 (review 6): --no-bin-links leaves the toolchain no bin directory, so `npm run build` found no tsc or esbuild +// and exited 127, which read as a failed test. The workspace now has a bin directory of small shell scripts (files, +// not links) that call the toolchain's tsc and esbuild, first on the PATH of the test script. +func TestTheTestScriptFindsTscAndEsbuildOnItsPath(t *testing.T) { + m, f, work := newMachine(t) + workspaceWith(t, work, map[string]string{"package.json": `{"scripts":{"test":"tsc && esbuild"}}`}) + if _, err := m.test(context.Background(), testArgs("unit")); err != nil { + t.Fatal(err) + } + bin := filepath.Join(work, "w1", "bin") + nm := filepath.Join(m.toolchainDir(), "node_modules") + for name, want := range map[string]string{ + "tsc": "exec node '" + nm + "/typescript/bin/tsc' \"$@\"", + "esbuild": "exec '" + nm + "/esbuild/bin/esbuild' \"$@\"", + } { + info, err := os.Lstat(filepath.Join(bin, name)) + if err != nil || !info.Mode().IsRegular() || info.Mode().Perm()&0o111 == 0 { + t.Fatalf("bin/%s: %v %v, want an executable file", name, info, err) + } + raw, _ := os.ReadFile(filepath.Join(bin, name)) + if !strings.HasPrefix(string(raw), "#!/bin/sh\n") || !strings.Contains(string(raw), want) { + t.Errorf("bin/%s = %q, want a script calling the toolchain's program", name, raw) + } + } + npm := f.calls[len(f.calls)-1] + path := "" + for _, e := range npm.env { + if strings.HasPrefix(e, "PATH=") { + path = strings.TrimPrefix(e, "PATH=") // the last PATH wins + } + } + if npm.name != "npm" || !strings.HasPrefix(path, bin+":") { + t.Errorf("npm ran with PATH %q, want %s first", path, bin) + } +} diff --git a/modules/mesh-build-node/cmd/mesh-build-node/emit_test.go b/modules/mesh-build-node/cmd/mesh-build-node/emit_test.go new file mode 100644 index 00000000..5d98c239 --- /dev/null +++ b/modules/mesh-build-node/cmd/mesh-build-node/emit_test.go @@ -0,0 +1,55 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "slices" + "testing" +) + +// Issues 1483 and 1488 (for the indexer, issue 1444): the guard emits toolchain.reported after each write of the +// toolchains key, with the node and the language. +func TestTheGuardEmitsToolchainReportedAfterEachWrite(t *testing.T) { + m := moduleWith(newStates(), "v26.10.0") + type event struct { + typ string + body map[string]string + } + var got []event + m.Emit = func(typ string, body any) error { + raw, _ := json.Marshal(body) + var b map[string]string + json.Unmarshal(raw, &b) + got = append(got, event{typ, b}) + return nil + } + if err := m.Check(context.Background()); err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].typ != "toolchain.reported" || got[0].body["node"] != "ace" || got[0].body["language"] != "typescript" || len(got[0].body) != 2 { + t.Errorf("events = %+v, want one toolchain.reported for ace, typescript", got) + } + m.Check(context.Background()) + if len(got) != 2 { + t.Errorf("a second guard pass emitted %d events in all, want 2", len(got)) + } +} + +// The event is carried by the module's stream, not named in emits (ADR 0363: one place); the validator checks that. +func TestTheManifestDeclaresTheToolchainsStream(t *testing.T) { + raw, err := os.ReadFile(filepath.Join("..", "..", "module.json")) + if err != nil { + t.Fatal(err) + } + var m struct { + Streams []struct { + Name string `json:"name"` + Events []string `json:"events"` + } `json:"streams"` + } + if err := json.Unmarshal(raw, &m); err != nil || len(m.Streams) != 1 || m.Streams[0].Name != "toolchains-node" || !slices.Equal(m.Streams[0].Events, []string{"toolchain.**"}) { + t.Errorf("streams = %+v (%v)", m.Streams, err) + } +}