Skip to content

Commit df4b5ef

Browse files
atilafassinaatilafassina
committed
Fix workspace and relative PATH handling in apps validation
1 parent 8b895f5 commit df4b5ef

10 files changed

Lines changed: 168 additions & 99 deletions

File tree

‎acceptance/apps/validate/output.txt‎

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
>>> withdir app [CLI] apps validate --path ..
44
Validating project using pnpm...
55
✅ Installing dependencies ([DURATION])
6-
⏭️ Skipped Generating types (script "typegen" not found)
7-
⏭️ Skipped Running AST-grep lint (script "lint:ast-grep" not found)
8-
⏭️ Skipped Type checking (script "typecheck" not found)
6+
✅ Generating types ([DURATION])
7+
✅ Running AST-grep lint ([DURATION])
8+
✅ Type checking ([DURATION])
99
✅ Building ([DURATION])
1010
✅ Running tests ([DURATION])
1111
✅ All validation checks passed ([DURATION])
@@ -14,9 +14,9 @@ Validating project using pnpm...
1414
>>> [CLI] apps validate --skip-tests
1515
Validating project using pnpm...
1616
✅ Installing dependencies ([DURATION])
17-
⏭️ Skipped Generating types (script "typegen" not found)
18-
⏭️ Skipped Running AST-grep lint (script "lint:ast-grep" not found)
19-
⏭️ Skipped Type checking (script "typecheck" not found)
17+
✅ Generating types ([DURATION])
18+
✅ Running AST-grep lint ([DURATION])
19+
✅ Type checking ([DURATION])
2020
✅ Building ([DURATION])
2121
⏭️ Skipped Running tests
2222
✅ All validation checks passed ([DURATION])
@@ -25,9 +25,9 @@ Validating project using pnpm...
2525
>>> VALIDATION_TEST_FAIL=run --if-present test musterr [CLI] apps validate
2626
Validating project using pnpm...
2727
✅ Installing dependencies ([DURATION])
28-
⏭️ Skipped Generating types (script "typegen" not found)
29-
⏭️ Skipped Running AST-grep lint (script "lint:ast-grep" not found)
30-
⏭️ Skipped Type checking (script "typecheck" not found)
28+
✅ Generating types ([DURATION])
29+
✅ Running AST-grep lint ([DURATION])
30+
✅ Type checking ([DURATION])
3131
✅ Building ([DURATION])
3232
Error: tests failed (duration: [DURATION])
3333
❌ Running tests failed ([DURATION])
@@ -44,9 +44,9 @@ Error: validation failed
4444
>>> VALIDATION_TEST_FAIL=run --if-present build musterr [CLI] apps deploy
4545
Validating project using pnpm...
4646
✅ Installing dependencies ([DURATION])
47-
⏭️ Skipped Generating types (script "typegen" not found)
48-
⏭️ Skipped Running AST-grep lint (script "lint:ast-grep" not found)
49-
⏭️ Skipped Type checking (script "typecheck" not found)
47+
✅ Generating types ([DURATION])
48+
✅ Running AST-grep lint ([DURATION])
49+
✅ Type checking ([DURATION])
5050
Error: build failed (duration: [DURATION])
5151
❌ Building failed ([DURATION])
5252
validation failed (exit code 17)
@@ -61,40 +61,66 @@ Error: validation failed - fix errors before deploying
6161
=== Commands executed in the project directory
6262
>>> cat commands.log
6363
pnpm install
64+
pnpm run --if-present typegen
65+
pnpm run --if-present lint:ast-grep
66+
pnpm run --if-present typecheck
6467
pnpm run --if-present build
6568
pnpm run --if-present test
6669
pnpm install
70+
pnpm run --if-present typegen
71+
pnpm run --if-present lint:ast-grep
72+
pnpm run --if-present typecheck
6773
pnpm run --if-present build
6874
pnpm install
75+
pnpm run --if-present typegen
76+
pnpm run --if-present lint:ast-grep
77+
pnpm run --if-present typecheck
6978
pnpm run --if-present build
7079
pnpm run --if-present test
7180
pnpm install
81+
pnpm run --if-present typegen
82+
pnpm run --if-present lint:ast-grep
83+
pnpm run --if-present typecheck
7284
pnpm run --if-present build
7385

7486
=== Warn but continue on conflicting lockfiles
7587
>>> [CLI] apps validate --path conflicting
7688
Warn: found conflicting package manager lockfiles; using package-lock.json and ignoring pnpm-lock.yaml
7789
Validating project using npm...
7890
✅ Installing dependencies ([DURATION])
79-
⏭️ Skipped Generating types (script "typegen" not found)
80-
⏭️ Skipped Running AST-grep lint (script "lint:ast-grep" not found)
81-
⏭️ Skipped Type checking (script "typecheck" not found)
82-
⏭️ Skipped Building (script "build" not found)
83-
⏭️ Skipped Running tests (script "test" not found)
91+
✅ Generating types ([DURATION])
92+
✅ Running AST-grep lint ([DURATION])
93+
✅ Type checking ([DURATION])
94+
✅ Building ([DURATION])
95+
✅ Running tests ([DURATION])
8496
✅ All validation checks passed ([DURATION])
8597

8698
=== Warn but continue on unsupported lockfiles
8799
>>> [CLI] apps validate --path unsupported
88100
Warn: ignoring unsupported lockfile yarn.lock; apps validation uses npm or pnpm
89101
Validating project using npm...
90102
✅ Installing dependencies ([DURATION])
91-
⏭️ Skipped Generating types (script "typegen" not found)
92-
⏭️ Skipped Running AST-grep lint (script "lint:ast-grep" not found)
93-
⏭️ Skipped Type checking (script "typecheck" not found)
94-
⏭️ Skipped Building (script "build" not found)
95-
⏭️ Skipped Running tests (script "test" not found)
103+
✅ Generating types ([DURATION])
104+
✅ Running AST-grep lint ([DURATION])
105+
✅ Type checking ([DURATION])
106+
✅ Building ([DURATION])
107+
✅ Running tests ([DURATION])
96108
✅ All validation checks passed ([DURATION])
97109

98110
=== Reject malformed package.json
99111
>>> musterr [CLI] apps validate --path invalid
100112
Error: validation error: failed to parse package.json: unexpected end of JSON input
113+
114+
=== Validate npm workspace scripts without root scripts
115+
>>> grep -x workspace build ran LOG.workspaces
116+
workspace build ran
117+
118+
>>> grep -F validation failed (exit code 17) LOG.workspaces
119+
validation failed (exit code 17)
120+
121+
=== Deploy rejects a failing npm workspace build
122+
>>> grep -x workspace build ran LOG.workspaces-deploy
123+
workspace build ran
124+
125+
>>> grep -F validation failed - fix errors before deploying LOG.workspaces-deploy
126+
Error: validation failed - fix errors before deploying

‎acceptance/apps/validate/script‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
validation_path="$PATH"
12
export PATH="$PWD/bin:$PATH"
23

34
title "Validate the directory selected by --path"
@@ -23,3 +24,15 @@ trace $CLI apps validate --path unsupported
2324

2425
title "Reject malformed package.json"
2526
trace musterr $CLI apps validate --path invalid
27+
28+
title "Validate npm workspace scripts without root scripts"
29+
export PATH="$validation_path"
30+
mkdir -p workspaces/node_modules
31+
trace musterr $CLI apps validate --path workspaces &> LOG.workspaces
32+
trace grep -x "workspace build ran" LOG.workspaces
33+
trace grep -F "validation failed (exit code 17)" LOG.workspaces
34+
35+
title "Deploy rejects a failing npm workspace build"
36+
trace withdir workspaces musterr $CLI apps deploy &> LOG.workspaces-deploy
37+
trace grep -x "workspace build ran" LOG.workspaces-deploy
38+
trace grep -F "validation failed - fix errors before deploying" LOG.workspaces-deploy

‎acceptance/apps/validate/test.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]
22
Env.VALIDATION_TEST_FAIL = ""
3-
Ignore = ["commands.log"]
3+
Ignore = ["commands.log", "node_modules"]
44

55
[[Repls]]
66
Old = '\d+\.\d+s'

‎acceptance/apps/validate/workspaces/.npmrc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
workspaces=true
2+
update-notifier=false

‎acceptance/apps/validate/workspaces/databricks.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
bundle:
2+
name: validation-workspaces

‎acceptance/apps/validate/workspaces/package.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "validation-workspaces",
3+
"private": true,
4+
"workspaces": ["packages/*"]
5+
}

‎acceptance/apps/validate/workspaces/packages/app/package.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "validation-workspace-app",
3+
"version": "1.0.0",
4+
"scripts": {
5+
"build": "node -e \"console.log('workspace build ran'); process.exit(17)\""
6+
}
7+
}

‎cmd/apps/validate.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ The package manager is detected from lockfiles in the project directory:
2424
- package-lock.json or npm-shrinkwrap.json: npm
2525
- pnpm-lock.yaml: pnpm
2626
27-
Only npm and pnpm are supported. Yarn and Bun lockfiles are rejected.
28-
Projects without a lockfile use npm. Lockfiles for both npm and pnpm
29-
must be resolved before validation. The selected package manager must be on PATH.
27+
Only npm and pnpm are supported. Yarn and Bun lockfiles produce a warning
28+
and are ignored. Projects without a supported lockfile use npm. If both npm
29+
and pnpm lockfiles exist, validation warns and uses npm.
30+
The selected package manager must be on PATH in the project directory.
3031
Dependencies are installed when node_modules is absent.
3132
3233
Examples:

‎libs/apps/validation/nodejs.go‎

Lines changed: 10 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@ import (
44
"bytes"
55
"context"
66
"encoding/json"
7-
"errors"
87
"fmt"
98
"os"
10-
osexec "os/exec"
119
"path/filepath"
1210
"time"
1311

@@ -32,19 +30,14 @@ func (v *ValidationNodeJs) Validate(ctx context.Context, workDir string, opts Va
3230
startTime := time.Now()
3331

3432
// Reject an invalid manifest before running commands that may modify it.
35-
if _, err := readPackageScripts(workDir); err != nil {
33+
if err := validatePackageJSON(workDir); err != nil {
3634
return nil, err
3735
}
3836

3937
manager, err := DetectPackageManager(ctx, workDir)
4038
if err != nil {
4139
return nil, err
4240
}
43-
// ErrDot means the manager was found via a relative PATH entry (e.g. node_modules/.bin);
44-
// the shell that runs the command resolves it fine, so only a genuine miss is fatal.
45-
if _, err := osexec.LookPath(manager); err != nil && !errors.Is(err, osexec.ErrDot) {
46-
return nil, fmt.Errorf("cannot run %s; install it and ensure it is on PATH: %w", manager, err)
47-
}
4841

4942
cmdio.LogString(ctx, "Validating project using "+manager+"...")
5043

@@ -97,20 +90,15 @@ func (v *ValidationNodeJs) Validate(ctx context.Context, workDir string, opts Va
9790
continue
9891
}
9992
if step.script != "" {
100-
scripts, err := readPackageScripts(workDir)
101-
if err != nil {
93+
if err := validatePackageJSON(workDir); err != nil {
10294
return nil, err
10395
}
104-
if _, ok := scripts[step.script]; !ok {
105-
cmdio.LogString(ctx, fmt.Sprintf("⏭️ Skipped %s (script %q not found)", step.displayName, step.script))
106-
continue
107-
}
10896
}
10997

11098
command := manager + " install"
11199
if step.script != "" {
112-
// Pass --if-present as a safety net: if our presence check and the manager
113-
// disagree about a script, this skips it instead of failing the run.
100+
// Let the manager resolve scripts, including configured npm workspaces:
101+
// https://docs.npmjs.com/cli/using-npm/workspaces#running-commands-in-the-context-of-workspaces
114102
command = manager + " run --if-present " + step.script
115103
}
116104

@@ -151,30 +139,19 @@ func (v *ValidationNodeJs) Validate(ctx context.Context, workDir string, opts Va
151139
}, nil
152140
}
153141

154-
// readPackageScripts returns the runnable scripts declared in package.json. It reloads
155-
// the manifest because earlier validation steps can change it, and keeps only non-empty
156-
// string values so a script set to null, "", or a non-string (like a "//": [...] comment)
157-
// is treated as absent, matching how npm and pnpm decide a script is runnable.
158-
func readPackageScripts(workDir string) (map[string]string, error) {
142+
// validatePackageJSON reloads the manifest because earlier validation steps can change it.
143+
func validatePackageJSON(workDir string) error {
159144
packageJSON, err := os.ReadFile(filepath.Join(workDir, "package.json"))
160145
if err != nil {
161-
return nil, fmt.Errorf("failed to read package.json: %w", err)
146+
return fmt.Errorf("failed to read package.json: %w", err)
162147
}
163148
// npm strips a UTF-8 BOM before decoding: https://github.com/npm/json-parse-even-better-errors.
164149
packageJSON = bytes.TrimPrefix(packageJSON, []byte("\xef\xbb\xbf"))
165-
var project struct {
166-
Scripts map[string]any `json:"scripts"`
167-
}
150+
var project struct{}
168151
if err := json.Unmarshal(packageJSON, &project); err != nil {
169-
return nil, fmt.Errorf("failed to parse package.json: %w", err)
152+
return fmt.Errorf("failed to parse package.json: %w", err)
170153
}
171-
scripts := make(map[string]string, len(project.Scripts))
172-
for name, value := range project.Scripts {
173-
if s, ok := value.(string); ok && s != "" {
174-
scripts[name] = s
175-
}
176-
}
177-
return scripts, nil
154+
return nil
178155
}
179156

180157
// hasNodeModules returns true if node_modules directory exists in the workDir.

0 commit comments

Comments
 (0)