fix: maintain working directory across script steps

[jessicatarra]

This PR addresses the issue raised in #470, where the working directory is not maintained across multiple steps in a script.

For example, consider the following script:

fail:
  steps:
    - cd packages/
    - pwd
    

When running this script using melos run, the output will show the workspace root directory, not the packages/ directory as expected.

This PR introduces a change to ensure that the working directory is properly maintained across multiple steps in a script.

The key changes are:

• Modify the script execution logic to preserve the working directory from one step to the next.
• Add a safeguard to prevent users from mixing && operators with multi-step scripts, as this can lead to unexpected behavior.

By running the script defined in the issue with the fix implemented in this PR, the output is now:

➜  melos git:(bugfix/melos_run_script_steps_working_directory) ✗ melos success
Building package executable...
Built melos:melos.
melos run success
  └> cd packages/
     └> RUNNING


melos run success
  └> cd packages/
     └> SUCCESS

melos run success
  └> pwd
     └> RUNNING

/Users/jessicatarra/Development/melos/packages

melos run success
  └> pwd
     └> SUCCESS

Type of Change

• ✨ feat -- New feature (non-breaking change which adds functionality)
• 🛠️ fix -- Bug fix (non-breaking change which fixes an issue)
• ❌ ! -- Breaking change (fix or feature that would cause existing functionality to change)
• 🧹 refactor -- Code refactor
• ✅ ci -- Build configuration change
• 📝 docs -- Documentation
• 🗑️ chore -- Chore

[spydon]

I think there must be a better way to keep the environment between steps than to look for cd commands 🤔
cd is also not the only way to change directory in Linux, for example pushd and popd are quite commonly used in scripts.

And I guess other things in the environment should be taken into consideration too, like exporting variables for example.

[jessicatarra]

It makes sense, I'll think of another approach then. Thanks for the feedback!

[Salakar]

what about if a step and/or script could define it's working directory rather than us trying to detect it and magically preserve it perhaps, e.g.:

fail:
  # optional, for all steps:
  workingDirectory: packages/
  steps:
    # Step as a YamlMap definition:
    - run: pwd
      # optional, for an individual step + overrides top level if set
      workingDirectory: packages/
    # Step as a string in the YamlArray:
    - ls -la

Would also make it more cross-platform friendly and not worry about Windows weirdness too!

[jessicatarra]

Nice! I'm all in, it makes a tons of sense. I will start working on it

[spydon]

@Salakar I think that many of the people that have asked for this feature don't just need to be in one directory, but jump around during the multi-step script. Isn't it possible to keep the shell session alive and just keep on executing all steps in there, instead of creating a new shell for each step? 🤔

[jessicatarra]

I did a proof of concept of the approach mentioned by @spydon, and it's doable. It actually follows the same logic of GitHub workflows, also mentioned in the comment replied by @timcreatedit:

I would understand if you keep it like this (also because it's more consistent with GitHub workflows)

The code that I used for the PoC is as follows:

class PersistentShell {
  PersistentShell({required bool isWindows}) {
    _isWindows = isWindows;
  }
  late Process _process;
  late bool _isWindows;

  Future<void> startShell() async {
    _process = await Process.start(
      _isWindows ? 'cmd.exe' : '/bin/sh',
      _isWindows ? ['/Q'] : [],
    );

    _process.stdout.transform(utf8.decoder).listen(print);
    _process.stderr.transform(utf8.decoder).listen((data) {
      print('Error: $data');
    });
  }

  void sendCommand(String command) {
    _process.stdin.writeln(_isWindows ? command : 'eval "$command"');
  }

  Future<void> stopShell() async {
    await _process.stdin.close();
    await _process.exitCode;
  }
}

void main() async {
  var shell = PersistentShell(isWindows: Platform.isWindows);
  await shell.startShell();
  shell.sendCommand('echo Hello World');
  await shell.stopShell();
}

I implemented the code and it works really well, as the hardcoded condition to check if 'cd ' is part of the step command but way better, because it's dynamic. Let me know if this is what you meant @spydon

[spydon]

Sorry for the late reply, this is what I meant indeed! :)

[spydon]

Any progress on this @jessicatarra? 😊

[jessicatarra]

Hey everyone,
@spydon Sorry for the late reply. Work's been pretty busy lately, but I finally got some time this weekend to dig back into this fix. I managed to make things work, but now I'm stuck on figuring out how to keep the logs consistent with our current style.
The issue is, this persistent shell (as it is implemented) makes it tricky to track the results of individual commands. To help us brainstorm a solution, I'm sharing my implementation so far. Let's see what we can come up with together.

Example script

melos:test:
  steps:
    - cd packages/melos
    - pwd
    - dart test .





Output

melos run melos:test
➡️ step: cd packages/melos
➡️ step: pwd
/Users/jessicatarra/Development/melos/packages/melos
➡️ step: dart test .

… (long logs from dart test step)


ERROR: Shell process completed with errors.
Shell process completed some steps successfully.



[spydon]

@spydon Sorry for the late reply. Work's been pretty busy lately, but I finally got some time this weekend to dig back into this fix.

No worries! Thanks for looking into it again. :)

I managed to make things work, but now I'm stuck on figuring out how to keep the logs consistent with our current style.

I left a few suggestions that I think might help, let me know what you think.

[spydon]

Starts to look very good!
No idea about the test failing on windows though...

[spydon]

I see that there are some tests failing for windows on main too, is it maybe the same ones @jessicatarra, and we just have bad tests that aren't affected by this PR?

[jessicatarra]

That's a good point @spydon . I'll check that out. Maybe it would be a good idea to separate the work and open a new PR to fix the failing Windows tests.

[spydon]

That's a good point @spydon . I'll check that out. Maybe it would be a good idea to separate the work and open a new PR to fix the failing Windows tests.

Yeah, if there aren't tests failing due to this PR that sounds like a good idea

[spydon]

Lgtm! @jessicatarra this is ready for merge now right?