Skip to content

Commit ce3b35e

Browse files
pieternclaude
pietern
and
claude
authored
Fix Node.js debug env propagation and remove stored context from app structs (#4696)
## Summary - Fix `NodeApp.enableDebugging` which called `os.Setenv` to set `NODE_OPTIONS`, but `startAppProcess` constructs `appCmd.Env` explicitly without inheriting the parent process environment — so the `--inspect` flag never reached the child process - Have `GetCommand` return extra env vars alongside the command, which the caller appends to `appCmdEnv` - Remove stored `context.Context` from `NodeApp` and `PythonApp` structs, passing it through method arguments instead (follow-up to #4690) ## Test plan - [x] New `node_test.go` tests cover debug env, appending to existing `NODE_OPTIONS`, and custom port - [x] Existing `python_test.go` tests updated and passing - [x] `go test ./libs/apps/runlocal/... ./cmd/apps/...` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 4d74e8b commit ce3b35e

File tree

6 files changed

+114
-39
lines changed

6 files changed

+114
-39
lines changed

‎cmd/apps/run_local.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func setupWorkspaceAndConfig(cmd *cobra.Command, entryPoint string, appPort int)
5353
func setupApp(cmd *cobra.Command, config *runlocal.Config, spec *runlocal.AppSpec, customEnv []string, prepareEnvironment bool) (runlocal.App, []string, error) {
5454
ctx := cmd.Context()
5555
cfg := cmdctx.ConfigUsed(ctx)
56-
app, err := runlocal.NewApp(ctx, config, spec)
56+
app, err := runlocal.NewApp(config, spec)
5757
if err != nil {
5858
return nil, nil, err
5959
}
@@ -70,7 +70,7 @@ func setupApp(cmd *cobra.Command, config *runlocal.Config, spec *runlocal.AppSpe
7070
env = append(env, appEnv...)
7171

7272
if prepareEnvironment {
73-
err := app.PrepareEnvironment()
73+
err := app.PrepareEnvironment(ctx)
7474
if err != nil {
7575
return app, nil, err
7676
}
@@ -81,7 +81,7 @@ func setupApp(cmd *cobra.Command, config *runlocal.Config, spec *runlocal.AppSpe
8181

8282
func startAppProcess(cmd *cobra.Command, config *runlocal.Config, app runlocal.App, env []string, debug bool) (*exec.Cmd, error) {
8383
ctx := cmd.Context()
84-
specCommand, err := app.GetCommand(debug)
84+
specCommand, cmdEnv, err := app.GetCommand(ctx, debug)
8585
if err != nil {
8686
return nil, err
8787
}
@@ -98,6 +98,7 @@ func startAppProcess(cmd *cobra.Command, config *runlocal.Config, app runlocal.A
9898
appCmdEnv = append(appCmdEnv, envVar.String())
9999
}
100100
appCmdEnv = append(appCmdEnv, env...)
101+
appCmdEnv = append(appCmdEnv, cmdEnv...)
101102
appCmd.Env = appCmdEnv
102103
appCmd.Dir = config.AppPath
103104

‎libs/apps/runlocal/apps.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ import (
77
)
88

99
type App interface {
10-
PrepareEnvironment() error
11-
GetCommand(bool) ([]string, error)
10+
PrepareEnvironment(ctx context.Context) error
11+
GetCommand(ctx context.Context, debug bool) (cmd, env []string, err error)
1212
}
1313

14-
func NewApp(ctx context.Context, config *Config, spec *AppSpec) (App, error) {
14+
func NewApp(config *Config, spec *AppSpec) (App, error) {
1515
// Check if the app is a Node.js app by checking if there is a package.json file in the root of the app
1616
packageJsonPath := filepath.Join(config.AppPath, "package.json")
1717
_, err := os.Stat(packageJsonPath)
@@ -21,8 +21,8 @@ func NewApp(ctx context.Context, config *Config, spec *AppSpec) (App, error) {
2121
if err != nil {
2222
return nil, err
2323
}
24-
return NewNodeApp(ctx, config, spec, packageJson), nil
24+
return NewNodeApp(config, spec, packageJson), nil
2525
}
2626

27-
return NewPythonApp(ctx, config, spec), nil
27+
return NewPythonApp(config, spec), nil
2828
}

‎libs/apps/runlocal/node.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"encoding/json"
66
"os"
7+
8+
"github.com/databricks/cli/libs/env"
79
)
810

911
const NODE_DEBUG_PORT = "9229"
@@ -14,64 +16,62 @@ type PackageJson struct {
1416
}
1517

1618
type NodeApp struct {
17-
ctx context.Context
1819
config *Config
1920
spec *AppSpec
2021
packageJson *PackageJson
2122
}
2223

23-
func NewNodeApp(ctx context.Context, config *Config, spec *AppSpec, packageJson *PackageJson) *NodeApp {
24+
func NewNodeApp(config *Config, spec *AppSpec, packageJson *PackageJson) *NodeApp {
2425
if config.DebugPort == "" {
2526
config.DebugPort = NODE_DEBUG_PORT
2627
}
2728

2829
return &NodeApp{
29-
ctx: ctx,
3030
config: config,
3131
spec: spec,
3232
packageJson: packageJson,
3333
}
3434
}
3535

36-
func (n *NodeApp) PrepareEnvironment() error {
36+
func (n *NodeApp) PrepareEnvironment(ctx context.Context) error {
3737
// Install dependencies
3838
installArgs := []string{"npm", "install"}
39-
if err := runCommand(n.ctx, n.config.AppPath, installArgs); err != nil {
39+
if err := runCommand(ctx, n.config.AppPath, installArgs); err != nil {
4040
return err
4141
}
4242

4343
// Run build script if it exists
4444
if _, ok := n.packageJson.Scripts["build"]; ok {
4545
buildArgs := []string{"npm", "run", "build"}
46-
if err := runCommand(n.ctx, n.config.AppPath, buildArgs); err != nil {
46+
if err := runCommand(ctx, n.config.AppPath, buildArgs); err != nil {
4747
return err
4848
}
4949
}
5050

5151
return nil
5252
}
5353

54-
func (n *NodeApp) GetCommand(debug bool) ([]string, error) {
54+
func (n *NodeApp) GetCommand(ctx context.Context, debug bool) ([]string, []string, error) {
55+
var cmdEnv []string
5556
if debug {
56-
n.enableDebugging()
57+
cmdEnv = n.enableDebugging(ctx)
5758
}
5859

5960
if n.spec.Command == nil {
60-
return []string{"npm", "run", "start"}, nil
61+
return []string{"npm", "run", "start"}, cmdEnv, nil
6162
}
6263

63-
return n.spec.Command, nil
64+
return n.spec.Command, cmdEnv, nil
6465
}
6566

66-
func (n *NodeApp) enableDebugging() {
67-
// Set NODE_OPTIONS environment variable to enable debugging
68-
// This will make Node.js listen for debugger connections on the debug port
69-
if os.Getenv("NODE_OPTIONS") == "" {
70-
os.Setenv("NODE_OPTIONS", "--inspect="+n.config.DebugPort)
71-
} else {
72-
// If NODE_OPTIONS already exists, append the inspect flag
73-
os.Setenv("NODE_OPTIONS", os.Getenv("NODE_OPTIONS")+" --inspect="+n.config.DebugPort)
67+
// enableDebugging returns environment variables that enable Node.js debugging.
68+
func (n *NodeApp) enableDebugging(ctx context.Context) []string {
69+
nodeOpts := env.Get(ctx, "NODE_OPTIONS")
70+
if nodeOpts != "" {
71+
nodeOpts += " "
7472
}
73+
nodeOpts += "--inspect=" + n.config.DebugPort
74+
return []string{"NODE_OPTIONS=" + nodeOpts}
7575
}
7676

7777
func readPackageJson(packageJsonPath string) (*PackageJson, error) {

‎libs/apps/runlocal/node_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package runlocal
2+
3+
import (
4+
"testing"
5+
6+
"github.com/databricks/cli/libs/env"
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestNodeAppGetCommand(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
debug bool
15+
command []string
16+
wantCmd []string
17+
wantEnv []string
18+
}{
19+
{
20+
name: "default command without debug",
21+
wantCmd: []string{"npm", "run", "start"},
22+
},
23+
{
24+
name: "custom command without debug",
25+
command: []string{"node", "server.js"},
26+
wantCmd: []string{"node", "server.js"},
27+
},
28+
{
29+
name: "default command with debug",
30+
debug: true,
31+
wantCmd: []string{"npm", "run", "start"},
32+
wantEnv: []string{"NODE_OPTIONS=--inspect=9229"},
33+
},
34+
{
35+
name: "custom command with debug",
36+
debug: true,
37+
command: []string{"node", "server.js"},
38+
wantCmd: []string{"node", "server.js"},
39+
wantEnv: []string{"NODE_OPTIONS=--inspect=9229"},
40+
},
41+
}
42+
43+
for _, tt := range tests {
44+
t.Run(tt.name, func(t *testing.T) {
45+
config := &Config{}
46+
spec := &AppSpec{config: config, Command: tt.command}
47+
app := NewNodeApp(config, spec, &PackageJson{})
48+
cmd, cmdEnv, err := app.GetCommand(t.Context(), tt.debug)
49+
require.NoError(t, err)
50+
assert.Equal(t, tt.wantCmd, cmd)
51+
assert.Equal(t, tt.wantEnv, cmdEnv)
52+
})
53+
}
54+
}
55+
56+
func TestNodeAppGetCommandDebugAppendsToExistingNodeOptions(t *testing.T) {
57+
ctx := t.Context()
58+
ctx = env.Set(ctx, "NODE_OPTIONS", "--max-old-space-size=4096")
59+
60+
config := &Config{}
61+
spec := &AppSpec{config: config}
62+
app := NewNodeApp(config, spec, &PackageJson{})
63+
_, cmdEnv, err := app.GetCommand(ctx, true)
64+
require.NoError(t, err)
65+
assert.Equal(t, []string{"NODE_OPTIONS=--max-old-space-size=4096 --inspect=9229"}, cmdEnv)
66+
}
67+
68+
func TestNodeAppGetCommandDebugCustomPort(t *testing.T) {
69+
config := &Config{DebugPort: "5555"}
70+
spec := &AppSpec{config: config}
71+
app := NewNodeApp(config, spec, &PackageJson{})
72+
_, cmdEnv, err := app.GetCommand(t.Context(), true)
73+
require.NoError(t, err)
74+
assert.Equal(t, []string{"NODE_OPTIONS=--inspect=5555"}, cmdEnv)
75+
}

‎libs/apps/runlocal/python.go‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,33 +37,32 @@ var defaultLibraries = []string{
3737
}
3838

3939
type PythonApp struct {
40-
ctx context.Context
4140
config *Config
4241
spec *AppSpec
4342
uvArgs []string
4443
}
4544

46-
func NewPythonApp(ctx context.Context, config *Config, spec *AppSpec) *PythonApp {
45+
func NewPythonApp(config *Config, spec *AppSpec) *PythonApp {
4746
if config.DebugPort == "" {
4847
config.DebugPort = DEBUG_PORT
4948
}
50-
return &PythonApp{ctx: ctx, config: config, spec: spec}
49+
return &PythonApp{config: config, spec: spec}
5150
}
5251

5352
// PrepareEnvironment creates a Python virtual environment using uv and installs required dependencies.
5453
// It first creates a virtual environment, then installs default libraries specified in defaultLibraries,
5554
// and finally installs any additional requirements from requirements.txt if it exists.
5655
// Returns an error if any step fails.
57-
func (p *PythonApp) PrepareEnvironment() error {
56+
func (p *PythonApp) PrepareEnvironment(ctx context.Context) error {
5857
// Create venv first
5958
venvArgs := []string{"uv", "venv"}
60-
if err := runCommand(p.ctx, p.config.AppPath, venvArgs); err != nil {
59+
if err := runCommand(ctx, p.config.AppPath, venvArgs); err != nil {
6160
return err
6261
}
6362

6463
// Install default libraries
6564
installArgs := append([]string{"uv", "pip", "install"}, defaultLibraries...)
66-
if err := runCommand(p.ctx, p.config.AppPath, installArgs); err != nil {
65+
if err := runCommand(ctx, p.config.AppPath, installArgs); err != nil {
6766
return err
6867
}
6968

@@ -72,7 +71,7 @@ func (p *PythonApp) PrepareEnvironment() error {
7271
// We also execute command with CWD set at p.config.AppPath
7372
// so we can just path local path to requirements.txt here
7473
reqArgs := []string{"uv", "pip", "install", "-r", "requirements.txt"}
75-
if err := runCommand(p.ctx, p.config.AppPath, reqArgs); err != nil {
74+
if err := runCommand(ctx, p.config.AppPath, reqArgs); err != nil {
7675
return err
7776
}
7877
}
@@ -86,21 +85,21 @@ func (p *PythonApp) PrepareEnvironment() error {
8685
// If not, the function looks for a python file in the app directory and returns a command
8786
// to run that file. If the app is in a virtual environment, the command is modified to point
8887
// to the python binary in the virtual environment.
89-
func (p *PythonApp) GetCommand(debug bool) ([]string, error) {
88+
func (p *PythonApp) GetCommand(_ context.Context, debug bool) ([]string, []string, error) {
9089
spec := p.spec
9190
// if no spec, find python file and use it to run app
9291
if len(spec.Command) == 0 {
9392
files, err := filepath.Glob(filepath.Join(spec.config.AppPath, "*.py"))
9493
if err != nil {
95-
return nil, fmt.Errorf("error reading source code directory: %w", err)
94+
return nil, nil, fmt.Errorf("error reading source code directory: %w", err)
9695
}
9796

9897
if len(files) > 0 {
9998
spec.Command = []string{"python", files[0]}
10099
}
101100

102101
if len(spec.Command) == 0 {
103-
return nil, errors.New("no python file found")
102+
return nil, nil, errors.New("no python file found")
104103
}
105104

106105
} else {
@@ -121,7 +120,7 @@ func (p *PythonApp) GetCommand(debug bool) ([]string, error) {
121120
spec.Command = append(p.uvArgs, spec.Command...)
122121
}
123122

124-
return spec.Command, nil
123+
return spec.Command, nil, nil
125124
}
126125

127126
// enableDebugging enables debugging for the app by starting the app with debugpy

‎libs/apps/runlocal/python_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ func TestPythonAppGetCommand(t *testing.T) {
8888
for _, tt := range tests {
8989
t.Run(tt.name, func(t *testing.T) {
9090
config, spec := tt.setup()
91-
app := NewPythonApp(t.Context(), config, spec)
92-
cmd, err := app.GetCommand(tt.debug)
91+
app := NewPythonApp(config, spec)
92+
cmd, _, err := app.GetCommand(t.Context(), tt.debug)
9393

9494
if !tt.wantErr {
9595
tt.checkResult(t, cmd)

0 commit comments

Comments
 (0)