From 2ad6ef2d71b8b60893097ff02cbcda00d69db339 Mon Sep 17 00:00:00 2001 From: "d3mlabs-ai-flow[bot]" <305891656+d3mlabs-ai-flow[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 09:23:55 -0400 Subject: [PATCH] =?UTF-8?q?ai-flow=20/build:=20stamp=5Finstalled=20is=20un?= =?UTF-8?q?reachable=20when=20a=20project=20defines=20its=20own=20up:=20?= =?UTF-8?q?=E2=80=94=20provisioning=20commands=20need=20spawn-and-wait,=20?= =?UTF-8?q?not=20exec?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com> --- src/dev/command.rb | 1 + src/dev/command_runner.rb | 84 +++++++++++++++++++++++++++------ src/dev/execution_context.rb | 5 ++ src/dev/runner.rb | 19 +++++++- test/dev/command_runner_test.rb | 59 +++++++++++++++++++++++ test/dev/runner_test.rb | 79 +++++++++++++++++++++++++++++++ 6 files changed, 230 insertions(+), 17 deletions(-) diff --git a/src/dev/command.rb b/src/dev/command.rb index 3b0e558..78eacb0 100644 --- a/src/dev/command.rb +++ b/src/dev/command.rb @@ -79,6 +79,7 @@ def execute(args:, context:) python_version: context.python_version, build_container: context.build_container, project_root: context.project_root, + wait: context.wait, ) runner.run(self, args:) end diff --git a/src/dev/command_runner.rb b/src/dev/command_runner.rb index 8d9d2fa..ed5d2cc 100644 --- a/src/dev/command_runner.rb +++ b/src/dev/command_runner.rb @@ -12,12 +12,19 @@ require "shadowenv_ruby" module Dev - # Runs dev commands by exec-ing into the child process. Dev prints a colored - # header (command name) and then replaces itself: + # Runs dev commands by handing the process over to the child. Dev prints a + # colored header (command name) and then, by default, exec-replaces itself: # - # - repl commands: exec directly (no footer, for interactive sessions) - # - non-repl commands: exec into a shell wrapper that runs the command and - # prints ✓ Done / ✗ Failed based on exit code + # - repl commands: the bare shell command (no footer, for interactive + # sessions) + # - non-repl commands: a shell wrapper that runs the command and prints + # ✓ Done / ✗ Failed based on exit code + # + # In wait mode (wait: true) the same commands run spawn-and-wait instead of + # exec-replace: the child is waited on, and a failure raises + # CommandFailedError carrying its exit status — so a caller with + # success-contingent post-steps (e.g. Runner's installed stamp) can sequence + # them after execute while preserving the child's exit code (#85). # # The child has full terminal access — CLI::UI features (frames, spinners, # prompts) all work natively without any interception. @@ -31,6 +38,23 @@ module Dev class CommandRunner extend T::Sig + # Raised in wait mode when the child command fails, carrying its exit + # status so the caller can skip success-contingent post-steps and exit + # with the child's code. (Exec-replace mode never raises — the child's + # status becomes the process's own.) + class CommandFailedError < StandardError + extend T::Sig + + sig { returns(Integer) } + attr_reader :exit_status + + sig { params(exit_status: Integer).void } + def initialize(exit_status:) + @exit_status = T.let(exit_status, Integer) + super("command failed with exit status #{exit_status}") + end + end + sig do params( ui: Dev::Cli::Ui, @@ -38,14 +62,17 @@ class CommandRunner python_version: T.nilable(String), build_container: T.nilable(Dev::BuildContainerConfig), project_root: Pathname, + wait: T::Boolean, ).void end - def initialize(ui:, ruby_version:, python_version: nil, build_container: nil, project_root: Dev.target_project_root) + def initialize(ui:, ruby_version:, python_version: nil, build_container: nil, project_root: Dev.target_project_root, + wait: false) @ui = T.let(ui, Dev::Cli::Ui) @ruby_version = T.let(ruby_version, String) @python_version = T.let(python_version, T.nilable(String)) @build_container = T.let(build_container, T.nilable(Dev::BuildContainerConfig)) @project_root = T.let(project_root, Pathname) + @wait = T.let(wait, T::Boolean) end sig { params(cmd: ShellCommand, args: T::Array[String]).void } @@ -58,9 +85,9 @@ def run(cmd, args: []) else ensure_shadowenv_provisioned! if cmd.repl - run_replace_process(shell_command) + run_bare(shell_command) else - run_exec_with_status(shell_command) + run_with_status_footer(shell_command) end end end @@ -98,7 +125,7 @@ def run_in_container(_cmd, shell_command) docker_argv = container_command(config, image_tag, shell_command) Dir.chdir(@project_root) - Kernel.exec(*T.unsafe(docker_argv)) + run_child(docker_argv) end # docker argv for a containerized command: a `docker exec` into the reused @@ -234,18 +261,26 @@ def ensure_llvm_provisioned!(project_root) ShadowenvLlvm.setup!(project_root: project_root, llvm_prefix: prefix) end + # Runs the bare shell command with no footer, so an interactive (repl) + # session owns the terminal end to end. + # + # @param shell_command [String] + # @return [void] sig { params(shell_command: String).void } - def run_replace_process(shell_command) + def run_bare(shell_command) Dir.chdir(@project_root) - Kernel.exec(child_env, "shadowenv", "exec", "--", "sh", "-c", shell_command) + run_child([child_env, "shadowenv", "exec", "--", "sh", "-c", shell_command]) end - # Execs into a shell wrapper that runs the command, then prints a colored - # success/failure footer based on the exit code. + # Runs the command inside a shell wrapper that prints a colored + # success/failure footer based on the exit code (and preserves it). + # + # @param shell_command [String] + # @return [void] sig { params(shell_command: String).void } - def run_exec_with_status(shell_command) + def run_with_status_footer(shell_command) Dir.chdir(@project_root) - Kernel.exec(child_env, "shadowenv", "exec", "--", "sh", "-c", <<~SH) + run_child([child_env, "shadowenv", "exec", "--", "sh", "-c", <<~SH]) #{shell_command} __dev_status=$? if [ $__dev_status -eq 0 ]; then @@ -264,5 +299,24 @@ def run_exec_with_status(shell_command) fi SH end + + # Hands the assembled argv (optional env hash first) to the child process. + # Exec-replace by default — the right shape for a leaf command: TTY and + # signal passthrough, no double process tree. In wait mode, spawn-and-wait + # instead, so control returns to the caller's post-execute steps. + # + # @param argv [Array] Kernel.exec / Kernel.system argv + # @return [void] + # @raise [CommandFailedError] in wait mode, when the child fails + sig { params(argv: T::Array[T.untyped]).void } + def run_child(argv) + return Kernel.exec(*T.unsafe(argv)) unless @wait + + return if Kernel.system(*T.unsafe(argv)) + + # $? is nil when the child could not be spawned at all, and exitstatus + # is nil for a signal-terminated child; report a generic failure then. + raise CommandFailedError.new(exit_status: $?&.exitstatus || 1) + end end end diff --git a/src/dev/execution_context.rb b/src/dev/execution_context.rb index 20ae83e..b561f3d 100644 --- a/src/dev/execution_context.rb +++ b/src/dev/execution_context.rb @@ -16,5 +16,10 @@ class ExecutionContext < T::Struct const :project_root, Pathname const :build_container, T.nilable(Dev::BuildContainerConfig), default: nil const :runner, T.nilable(Dev::RunnerSetupConfig), default: nil + + # When true, exec-style commands run spawn-and-wait (CommandRunner wait + # mode) instead of exec-replacing the dev process, so the caller can + # sequence post-execute work such as the installed stamp (#85). + const :wait, T::Boolean, default: false end end diff --git a/src/dev/runner.rb b/src/dev/runner.rb index ce706c3..febc1ed 100644 --- a/src/dev/runner.rb +++ b/src/dev/runner.rb @@ -5,6 +5,7 @@ require 'pathname' require 'dev/config_parser' require 'dev/command_registry' +require 'dev/command_runner' require 'dev/credential_accessor' require 'dev/credentials' require 'dev/execution_context' @@ -72,11 +73,19 @@ def run(argv, ui:, out: $stdout) project_root: Dev.target_project_root, build_container: @config.build_container, runner: @config.runner, + # Stamping commands sequence the installed stamp after execute, so an + # exec-style project command (e.g. a dev.yml `up:`) must spawn-and-wait + # — exec-replace would make the stamp unreachable (#85). + wait: STAMPING_COMMANDS.include?(cmd_name), ) guard_staleness(cmd_name, context.project_root) provision_build_credentials if cmd_name == "up" cmd.execute(args:, context:) stamp_installed(cmd_name, context.project_root) + rescue CommandRunner::CommandFailedError => e + # The child already reported its failure (the shell wrapper prints its + # ✗ Failed footer); skip the stamp and preserve the child's exit code. + Kernel.exit(e.exit_status) rescue CommandRegistry::CommandNotFoundError => e $stderr.puts "dev: #{e}" $stderr.puts "Run 'dev' or 'dev --help' to see available commands." @@ -97,6 +106,11 @@ def run(argv, ui:, out: $stdout) # no installed stamp exists yet. STALENESS_EXEMPT_COMMANDS = T.let(%w[up install-deps update-deps check plan provide-image].freeze, T::Array[String]) + # Provisioning commands that record the installed stamp after a fully + # successful run (see #stamp_installed). Their post-execute step is why + # #run selects CommandRunner's wait mode for them. + STAMPING_COMMANDS = T.let(%w[up install-deps].freeze, T::Array[String]) + # Two O(1) digest checks at every command start (see Dev::Deps::Staleness): # manifest vs lockfile, lockfile vs installed stamp. Warn on workstations; # error in CI, where a stale state is a pipeline bug, not a reminder. @@ -117,10 +131,11 @@ def guard_staleness(cmd_name, project_root) # Record the installed stamp after a fully-successful provisioning command # (`dev up` treats a stale stamp as its expected precondition and rewrites # it; `install-deps` is the CI-side install). Reached only when execute - # didn't raise. + # didn't raise — exec-style project commands run in CommandRunner wait + # mode (see #run), so a project-defined `up:` gets here too (#85). sig { params(cmd_name: String, project_root: Pathname).void } def stamp_installed(cmd_name, project_root) - return unless ["up", "install-deps"].include?(cmd_name) + return unless STAMPING_COMMANDS.include?(cmd_name) Dev::Deps::Staleness.new(project_root:).stamp_installed! end diff --git a/test/dev/command_runner_test.rb b/test/dev/command_runner_test.rb index 9f00653..9749da7 100644 --- a/test/dev/command_runner_test.rb +++ b/test/dev/command_runner_test.rb @@ -126,6 +126,65 @@ def teardown Dir.chdir(@original_cwd) end + # --- Wait mode (spawn-and-wait for callers with post-execute steps) --- + + test "wait mode spawns and waits instead of exec-replacing the process" do + Given "a wait-mode runner and a non-repl command" + runner = Dev::CommandRunner.new(ui: @ui, ruby_version: "4.0.1", project_root: @project_root, wait: true) + runner.stubs(:ensure_shadowenv_provisioned!) + cmd = Dev::ShellCommand.new(run: "./bin/setup.rb", repl: false) + + When "we run the command" + runner.run(cmd) + + Then "the command runs as a waited child, never via exec" + 1 * Kernel.system(has_entries("GEM_HOME" => nil, "RUBYLIB" => anything), "shadowenv", "exec", "--", "sh", "-c", includes("./bin/setup.rb")) >> true + 0 * Kernel.exec(any_parameters) + + Cleanup + Dir.chdir(@original_cwd) + end + + test "wait mode raises CommandFailedError carrying the child's exit status" do + Given "a wait-mode runner whose child exits 7" + runner = Dev::CommandRunner.new(ui: @ui, ruby_version: "4.0.1", project_root: @project_root, wait: true) + runner.stubs(:ensure_shadowenv_provisioned!) + cmd = Dev::ShellCommand.new(run: "./bin/setup.rb", repl: false) + Kernel.stubs(:system).returns(false) + # Kernel.system is stubbed, so wait on a real child here to leave the + # thread-local $? at exit status 7 — what a real failed child would set. + Process.wait(Process.spawn("sh", "-c", "exit 7")) + + When "we run the command" + error = assert_raises(Dev::CommandRunner::CommandFailedError) { runner.run(cmd) } + + Then "the error carries the child's exit status" + error.exit_status == 7 + + Cleanup + Dir.chdir(@original_cwd) + end + + test "wait mode runs the containerized command spawn-and-wait" do + Given "a wait-mode runner with a build container" + config = Dev::BuildContainerConfig.new(image: "myapp-linux", registry: "myregistry") + runner = Dev::CommandRunner.new(ui: @ui, ruby_version: "4.0.1", build_container: config, project_root: @project_root, wait: true) + cmd = Dev::ShellCommand.new(run: "./bin/up.sh", repl: false) + + When "the image resolves and we run the command" + BuildContainer.stubs(:ensure_image!).returns("myregistry/myapp-linux:content-abc123") + BuildContainer.stubs(:docker_run_command) + .returns(["docker", "run", "--rm", "myregistry/myapp-linux:content-abc123", "sh", "-c", "./bin/up.sh"]) + runner.run(cmd) + + Then "docker runs as a waited child, never via exec" + 1 * Kernel.system("docker", "run", "--rm", "myregistry/myapp-linux:content-abc123", "sh", "-c", "./bin/up.sh") >> true + 0 * Kernel.exec(any_parameters) + + Cleanup + Dir.chdir(@original_cwd) + end + # --- Container execution --- test "run execs docker run when build_container is configured and command opts in" do diff --git a/test/dev/runner_test.rb b/test/dev/runner_test.rb index 89dbcf6..6f544f7 100644 --- a/test/dev/runner_test.rb +++ b/test/dev/runner_test.rb @@ -265,6 +265,85 @@ class RunnerTest < Minitest::Test execution_order == [:builtin_install, :project_script] end + # Regression for dev#85: a project-defined `up:` used to exec-replace the + # dev process, so Runner#run never reached stamp_installed and the + # staleness gate reported "never installed" forever. + test "up with a project up command runs it spawn-and-wait and stamps installed" do + Given "a Runner whose dev.yml defines up, pinned to an empty project root" + original_cwd = Dir.pwd + root = Pathname.new(Dir.mktmpdir("runner-up-stamp-")) + Dev.stubs(:target_project_root).returns(root) + runner = build_runner(commands: { "up" => { "run" => "./bin/up.rb", "desc" => "Setup", "container" => false } }) + runner.stubs(:resolve_ruby_version).returns("4.0.1") + runner.stubs(:install_locked_deps) + Dev::Cd::HookInstaller.any_instance.stubs(:ensure_installed).returns(:already_present) + Dev::CommandRunner.any_instance.stubs(:ensure_shadowenv_provisioned!) + Dev::Deps::Staleness.any_instance.expects(:stamp_installed!).once + + When "we run up" + runner.run(["up"], ui: fake_ui) + + Then "the project script runs as a waited child, never via exec-replace" + 1 * Kernel.system(anything, "shadowenv", "exec", "--", "sh", "-c", includes("./bin/up.rb")) >> true + 0 * Kernel.exec(any_parameters) + + Cleanup + Dir.chdir(original_cwd) + FileUtils.rm_rf(root) + end + + test "a failing project up command skips the stamp and exits with the child's status" do + Given "a Runner whose dev.yml defines up, whose script exits 7" + original_cwd = Dir.pwd + root = Pathname.new(Dir.mktmpdir("runner-up-fail-")) + Dev.stubs(:target_project_root).returns(root) + runner = build_runner(commands: { "up" => { "run" => "./bin/up.rb", "desc" => "Setup", "container" => false } }) + runner.stubs(:resolve_ruby_version).returns("4.0.1") + runner.stubs(:install_locked_deps) + Dev::Cd::HookInstaller.any_instance.stubs(:ensure_installed).returns(:already_present) + Dev::CommandRunner.any_instance.stubs(:ensure_shadowenv_provisioned!) + Kernel.stubs(:system).returns(false) + # Kernel.system is stubbed, so wait on a real child here to leave the + # thread-local $? at exit status 7 — what a real failed child would set. + Process.wait(Process.spawn("sh", "-c", "exit 7")) + Dev::Deps::Staleness.any_instance.expects(:stamp_installed!).never + Kernel.expects(:exit).with(7).once + # Guard: a regression to exec-replace would otherwise replace the test + # process itself (Kernel.system above is stubbed, Kernel.exec is real). + Kernel.expects(:exec).never + + When "we run up" + runner.run(["up"], ui: fake_ui) + + Then "the expectations hold: no stamp, exit with the child's status" + true + + Cleanup + Dir.chdir(original_cwd) + FileUtils.rm_rf(root) + end + + test "generic project commands keep the exec tail-call" do + Given "a Runner with a test command, pinned to an empty project root" + original_cwd = Dir.pwd + root = Pathname.new(Dir.mktmpdir("runner-exec-tail-")) + Dev.stubs(:target_project_root).returns(root) + runner = build_runner(commands: { "test" => { "run" => "./bin/test.sh", "desc" => "Run tests", "container" => false } }) + runner.stubs(:resolve_ruby_version).returns("4.0.1") + Dev::CommandRunner.any_instance.stubs(:ensure_shadowenv_provisioned!) + + When "we run a non-stamping command" + runner.run(["test"], ui: fake_ui) + + Then "the command exec-replaces the process, never spawn-and-wait" + 1 * Kernel.exec(anything, "shadowenv", "exec", "--", "sh", "-c", includes("./bin/test.sh")) + 0 * Kernel.system(any_parameters) + + Cleanup + Dir.chdir(original_cwd) + FileUtils.rm_rf(root) + end + test "up resolves docker build arg credentials before executing" do Given "a Runner with build container build_args and an up command" runner = build_runner(