From 0775515fd5d304b178aaa222122dffd4642c9617 Mon Sep 17 00:00:00 2001 From: "d3mlabs-ai-flow[bot]" <305891656+d3mlabs-ai-flow[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:36:29 -0400 Subject: [PATCH 1/3] =?UTF-8?q?ai-flow=20/build:=20PR=20A:=20Close=20the?= =?UTF-8?q?=20sealed=20Command=20hierarchy=20honestly=20=E2=80=94=20Builti?= =?UTF-8?q?nBody=20interface,=20final=20BuiltinCommand=20holding=20a=20bod?= =?UTF-8?q?y,=20delete=20the=20sorbet-runtime=20ivar=20pokes,=20un-private?= =?UTF-8?q?=20CommandRepository?= 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/builtin_body.rb | 43 +++++++++++++ src/dev/builtins.rb | 7 ++- src/dev/builtins/cache_command.rb | 5 +- src/dev/builtins/cd_command.rb | 5 +- src/dev/builtins/check_command.rb | 5 +- src/dev/builtins/clone_command.rb | 5 +- src/dev/builtins/cred_command.rb | 5 +- src/dev/builtins/deps_command.rb | 5 +- src/dev/builtins/install_deps_command.rb | 5 +- src/dev/builtins/learnings_command.rb | 5 +- src/dev/builtins/plan_command.rb | 5 +- src/dev/builtins/provide_image_command.rb | 5 +- src/dev/builtins/reset_container_command.rb | 5 +- src/dev/builtins/runner_setup_command.rb | 5 +- src/dev/builtins/up_command.rb | 5 +- src/dev/builtins/update_deps_command.rb | 5 +- src/dev/command.rb | 51 ++++++++++------ src/dev/command_repository.rb | 6 +- src/dev/runner.rb | 23 ++++--- test/dev/command_executor_test.rb | 31 +++++----- test/dev/command_repository_test.rb | 46 +++++++------- test/dev/command_service_test.rb | 44 +++++++------ test/dev/command_test.rb | 68 ++++++++++++++++++--- 23 files changed, 258 insertions(+), 131 deletions(-) create mode 100644 src/dev/builtin_body.rb diff --git a/src/dev/builtin_body.rb b/src/dev/builtin_body.rb new file mode 100644 index 0000000..9108f81 --- /dev/null +++ b/src/dev/builtin_body.rb @@ -0,0 +1,43 @@ +# typed: strict +# frozen_string_literal: true + +require_relative "execution_context" + +module Dev + # The behavior contract of a builtin dev command: a Ruby body plus the + # trait readings BuiltinCommand delegates to. Implementations live under + # src/dev/builtins/ (one class per builtin, collaborators + # constructor-injected); the composition root wraps each body in + # BuiltinCommand, the sealed hierarchy's final leaf. + # + # This would be an `interface!`, but Sorbet interfaces forbid default + # implementations and the traits deliberately default (most builtins are + # visible, guarded, and non-stamping) — hence `abstract!` with + # overridable trait defaults mirroring Command's. + module BuiltinBody + extend T::Sig + extend T::Helpers + abstract! + + sig { abstract.returns(String) } + def desc; end + + # Whether this builtin is callable but omitted from `dev`/`dev --help` + # usage (see Command#hidden?). Visible by default. + sig { overridable.returns(T::Boolean) } + def hidden? = false + + # Whether the staleness guard skips this builtin (see + # Command#staleness_exempt?). Guarded by default. + sig { overridable.returns(T::Boolean) } + def staleness_exempt? = false + + # Whether a fully-successful run records the installed stamp (see + # Command#stamps?). Non-stamping by default. + sig { overridable.returns(T::Boolean) } + def stamps? = false + + sig { abstract.params(args: T::Array[String], context: ExecutionContext).void } + def call(args:, context:); end + end +end diff --git a/src/dev/builtins.rb b/src/dev/builtins.rb index e3c3c8c..af727b0 100644 --- a/src/dev/builtins.rb +++ b/src/dev/builtins.rb @@ -2,11 +2,12 @@ # frozen_string_literal: true module Dev - # One class per builtin dev command. Each exposes call(args:, context:) - # with its collaborators constructor-injected; per-call values stay + # One class per builtin dev command, each implementing BuiltinBody with + # its collaborators constructor-injected; per-call values stay # method-side. The composition root (Runner) decides which builtins exist # for a given project (config-gated: runner-setup only with a `runner:` - # block, provide-image/reset-container only with a build container). + # block, provide-image/reset-container only with a build container) and + # wraps each body in the sealed hierarchy's BuiltinCommand leaf. module Builtins; end end diff --git a/src/dev/builtins/cache_command.rb b/src/dev/builtins/cache_command.rb index f130fe7..fbaad73 100644 --- a/src/dev/builtins/cache_command.rb +++ b/src/dev/builtins/cache_command.rb @@ -1,8 +1,8 @@ # typed: strict # frozen_string_literal: true +require "dev/builtin_body" require "dev/cli/flag_parser" -require "dev/command" require "dev/deps/cache_gc" require "dev/deps/lockfile" require "build_container" @@ -11,8 +11,9 @@ module Dev module Builtins # `dev cache gc [--keep N]`: reclaim stale install-dir versions and, when # a build container is configured, its stale content-tagged images. - class CacheCommand < BuiltinCommand + class CacheCommand extend T::Sig + include BuiltinBody # Builds the GC over a project's lockfile (per-call project root). CacheGcFactory = T.type_alias do diff --git a/src/dev/builtins/cd_command.rb b/src/dev/builtins/cd_command.rb index 65f2b2f..8eb535b 100644 --- a/src/dev/builtins/cd_command.rb +++ b/src/dev/builtins/cd_command.rb @@ -1,16 +1,17 @@ # typed: strict # frozen_string_literal: true +require "dev/builtin_body" require "dev/cd" -require "dev/command" module Dev module Builtins # `dev cd` is dispatched globally (before dev.yml lookup) in bin/dev; # this builtin only surfaces it in `dev --help` and keeps it callable # inside a project. - class CdCommand < BuiltinCommand + class CdCommand extend T::Sig + include BuiltinBody sig { params(accessor: Dev::Cd::Accessor).void } def initialize(accessor: Dev::Cd::Accessor.new) diff --git a/src/dev/builtins/check_command.rb b/src/dev/builtins/check_command.rb index 010e2e6..9b0c453 100644 --- a/src/dev/builtins/check_command.rb +++ b/src/dev/builtins/check_command.rb @@ -1,15 +1,16 @@ # typed: strict # frozen_string_literal: true -require "dev/command" +require "dev/builtin_body" require "dev/dependency_service" module Dev module Builtins # `dev check`: report the dependency-state freshness the staleness guard # would act on, and exit non-zero when anything drifted. - class CheckCommand < BuiltinCommand + class CheckCommand extend T::Sig + include BuiltinBody sig { params(dependency_service: DependencyService).void } def initialize(dependency_service:) diff --git a/src/dev/builtins/clone_command.rb b/src/dev/builtins/clone_command.rb index 368ad38..829797b 100644 --- a/src/dev/builtins/clone_command.rb +++ b/src/dev/builtins/clone_command.rb @@ -1,16 +1,17 @@ # typed: strict # frozen_string_literal: true +require "dev/builtin_body" require "dev/clone" -require "dev/command" module Dev module Builtins # `dev clone` is dispatched globally (before dev.yml lookup) in bin/dev; # this builtin only surfaces it in `dev --help` and keeps it callable # inside a project. - class CloneCommand < BuiltinCommand + class CloneCommand extend T::Sig + include BuiltinBody sig { params(accessor: Dev::Clone::Accessor).void } def initialize(accessor: Dev::Clone::Accessor.new) diff --git a/src/dev/builtins/cred_command.rb b/src/dev/builtins/cred_command.rb index 3b33161..af9140f 100644 --- a/src/dev/builtins/cred_command.rb +++ b/src/dev/builtins/cred_command.rb @@ -1,7 +1,7 @@ # typed: strict # frozen_string_literal: true -require "dev/command" +require "dev/builtin_body" require "dev/credential_accessor" module Dev @@ -9,8 +9,9 @@ module Builtins # `dev cred` is dispatched globally (before dev.yml lookup) in bin/dev; # this builtin only surfaces it in `dev --help` and keeps it callable # inside a project. - class CredCommand < BuiltinCommand + class CredCommand extend T::Sig + include BuiltinBody sig { params(accessor: Dev::CredentialAccessor).void } def initialize(accessor: Dev::CredentialAccessor.new) diff --git a/src/dev/builtins/deps_command.rb b/src/dev/builtins/deps_command.rb index ee4ecfd..00ae072 100644 --- a/src/dev/builtins/deps_command.rb +++ b/src/dev/builtins/deps_command.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require "pathname" -require "dev/command" +require "dev/builtin_body" require "dev/deps/accessor" require "dev/deps/cache" require "dev/deps/lockfile" @@ -11,8 +11,9 @@ module Dev module Builtins # `dev deps`: read-only lookups over the lockfile + content cache (e.g. # `dev deps path ficsit `). - class DepsCommand < BuiltinCommand + class DepsCommand extend T::Sig + include BuiltinBody # Builds the accessor over a project's lockfile (the project root is a # per-call value, so the collaborator arrives as a factory). diff --git a/src/dev/builtins/install_deps_command.rb b/src/dev/builtins/install_deps_command.rb index 7aa6fe0..46916ad 100644 --- a/src/dev/builtins/install_deps_command.rb +++ b/src/dev/builtins/install_deps_command.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require "pathname" -require "dev/command" +require "dev/builtin_body" require "dev/deps" require "dev/deps/cache" require "dev/deps/gem_skill_linker" @@ -18,8 +18,9 @@ module Builtins # machine — shared with the `up` builtin, which composes this command. # Host integrations install on the host (not the build container) so # their artifacts can be volume-mounted in. - class InstallDepsCommand < BuiltinCommand + class InstallDepsCommand extend T::Sig + include BuiltinBody # Builds the DependencyInstaller for a lockfile + integrations pair; # injected so tests can substitute a fake without touching the host. diff --git a/src/dev/builtins/learnings_command.rb b/src/dev/builtins/learnings_command.rb index 44c08b4..b10b090 100644 --- a/src/dev/builtins/learnings_command.rb +++ b/src/dev/builtins/learnings_command.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require "pathname" -require "dev/command" +require "dev/builtin_body" require "dev/learnings" module Dev @@ -10,8 +10,9 @@ module Builtins # `dev learnings` is dispatched globally (before dev.yml lookup) in # bin/dev; this builtin only surfaces it in `dev --help` and keeps it # callable inside a project. - class LearningsCommand < BuiltinCommand + class LearningsCommand extend T::Sig + include BuiltinBody # Builds the accessor for the enclosing project (per-call root). AccessorFactory = T.type_alias do diff --git a/src/dev/builtins/plan_command.rb b/src/dev/builtins/plan_command.rb index 4109640..2eec311 100644 --- a/src/dev/builtins/plan_command.rb +++ b/src/dev/builtins/plan_command.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require "pathname" -require "dev/command" +require "dev/builtin_body" require "dev/plan" module Dev @@ -10,8 +10,9 @@ module Builtins # `dev plan` is dispatched globally (before dev.yml lookup) in bin/dev; # this builtin only surfaces it in `dev --help` and keeps it callable # inside a project. - class PlanCommand < BuiltinCommand + class PlanCommand extend T::Sig + include BuiltinBody # Builds the accessor for the enclosing project (per-call root). AccessorFactory = T.type_alias do diff --git a/src/dev/builtins/provide_image_command.rb b/src/dev/builtins/provide_image_command.rb index 5d1e09a..0f56dd2 100644 --- a/src/dev/builtins/provide_image_command.rb +++ b/src/dev/builtins/provide_image_command.rb @@ -1,7 +1,7 @@ # typed: strict # frozen_string_literal: true -require "dev/command" +require "dev/builtin_body" require "dev/credentials" require "build_container" @@ -13,8 +13,9 @@ module Builtins # tag is capturable). Publishing to the shared registry stays gated on # DEV_PUBLISH_IMAGE, same as containerized commands. Exists only when a # build container is configured (the composition root gates it). - class ProvideImageCommand < BuiltinCommand + class ProvideImageCommand extend T::Sig + include BuiltinBody sig { override.returns(String) } def desc = "Resolve the build container image (local/pull/build) and print its tag" diff --git a/src/dev/builtins/reset_container_command.rb b/src/dev/builtins/reset_container_command.rb index f36842f..b94fd85 100644 --- a/src/dev/builtins/reset_container_command.rb +++ b/src/dev/builtins/reset_container_command.rb @@ -1,15 +1,16 @@ # typed: strict # frozen_string_literal: true -require "dev/command" +require "dev/builtin_body" require "build_container" module Dev module Builtins # Teardown for the persistent build container, only where a project opts # in (build.container.persist — the composition root gates it). - class ResetContainerCommand < BuiltinCommand + class ResetContainerCommand extend T::Sig + include BuiltinBody sig { override.returns(String) } def desc = "Remove the persistent build container (clears its incremental cache)" diff --git a/src/dev/builtins/runner_setup_command.rb b/src/dev/builtins/runner_setup_command.rb index 925be32..8ed0c55 100644 --- a/src/dev/builtins/runner_setup_command.rb +++ b/src/dev/builtins/runner_setup_command.rb @@ -1,8 +1,8 @@ # typed: strict # frozen_string_literal: true +require "dev/builtin_body" require "dev/cli/flag_parser" -require "dev/command" require "dev/runner_setup" require "dev/runner_setup_config" @@ -18,8 +18,9 @@ module Builtins # bespoke setup script. `--labels`/`--dir`/`--name` override the block # for hosts that differ from the repo default (e.g. registering the Mac # org-wide from a repo whose block describes the CI box). - class RunnerSetupCommand < BuiltinCommand + class RunnerSetupCommand extend T::Sig + include BuiltinBody # Builds the RunnerSetup for the resolved config and flags; injected # so tests can observe the wiring without touching gh or the host. diff --git a/src/dev/builtins/up_command.rb b/src/dev/builtins/up_command.rb index 0271382..07cbbca 100644 --- a/src/dev/builtins/up_command.rb +++ b/src/dev/builtins/up_command.rb @@ -1,8 +1,8 @@ # typed: strict # frozen_string_literal: true +require "dev/builtin_body" require "dev/cd" -require "dev/command" require "dev/credentials" module Dev @@ -13,8 +13,9 @@ module Builtins # provisioning. Projects with only a dependencies.rb get `dev up` for # free. `up` also ensures the `dev cd` shell hook (idempotent) — # provisioning is where dev's RC hooks land, next to the shadowenv one. - class UpCommand < BuiltinCommand + class UpCommand extend T::Sig + include BuiltinBody sig do params( diff --git a/src/dev/builtins/update_deps_command.rb b/src/dev/builtins/update_deps_command.rb index 0127ae8..d947901 100644 --- a/src/dev/builtins/update_deps_command.rb +++ b/src/dev/builtins/update_deps_command.rb @@ -3,7 +3,7 @@ require "digest" require "pathname" -require "dev/command" +require "dev/builtin_body" require "dev/deps" require "dev/deps/lockfile" require "dev/deps/registry" @@ -14,8 +14,9 @@ module Builtins # `dev update-deps`: resolve the dependencies.rb declarations and write # the lockfiles. Everything here is derived from the per-call project # root, so no collaborators need injecting. - class UpdateDepsCommand < BuiltinCommand + class UpdateDepsCommand extend T::Sig + include BuiltinBody sig { override.returns(String) } def desc = "Resolve dependency constraints and write lockfiles" diff --git a/src/dev/command.rb b/src/dev/command.rb index 79178fc..b8fb977 100644 --- a/src/dev/command.rb +++ b/src/dev/command.rb @@ -1,13 +1,14 @@ # typed: strict # frozen_string_literal: true +require_relative "builtin_body" require_relative "execution_context" module Dev # Sealed command data hierarchy. A command is one of exactly three shapes: # - # - BuiltinCommand: a Ruby body dev ships (abstract here; concretes live - # under src/dev/builtins/) + # - BuiltinCommand: a final wrapper delegating to the BuiltinBody it + # holds (bodies live under src/dev/builtins/) # - ProjectCommand: pure data parsed from a dev.yml `commands:` entry # - OverriddenCommand: a project command occupying a builtin's slot (the # builtin runs first, like a hardcoded super()) @@ -15,8 +16,8 @@ module Dev # Sealing makes any other nesting unrepresentable: CommandExecutor # dispatches exhaustively over these three variants (case + T.absurd). # Sorbet requires the direct subclasses of a sealed class beside it, which - # is why the whole hierarchy shares this file; BuiltinCommand is abstract - # but deliberately NOT sealed, so its concretes get their own files. + # is why the whole hierarchy shares this file; every leaf is closed + # (BuiltinCommand is final!), so the seal holds with no open edge. class Command extend T::Sig extend T::Helpers @@ -46,27 +47,37 @@ def staleness_exempt? = false def stamps? = false end - # Built-in command that executes Ruby code. Concretes live under - # src/dev/builtins/, one class per builtin, with collaborators injected - # through their constructors; per-call values arrive through #call. + # Built-in command that executes Ruby code: a concrete, final wrapper + # holding the BuiltinBody it delegates everything to. The Ruby bodies + # live under src/dev/builtins/, one class per builtin; the composition + # root wraps each in this leaf. final! means no subclasses can exist, so + # Command's sealed runtime hook has nothing to reject — the hierarchy is + # closed without reaching into sorbet-runtime internals. class BuiltinCommand < Command extend T::Sig extend T::Helpers - abstract! + final! - sig { abstract.params(args: T::Array[String], context: ExecutionContext).void } - def call(args:, context:); end - end + sig(:final) { params(body: BuiltinBody).void } + def initialize(body:) + @body = T.let(body, BuiltinBody) + end + + sig(:final) { override.returns(String) } + def desc = @body.desc - # Statically, `sealed!` binds only Command's direct subclasses, so - # BuiltinCommand concretes may live in their own files — but sorbet-runtime's - # inherited hook also rides down to BuiltinCommand's subclasses and would - # reject them for lacking a sealed declaration. Registering an empty - # decl-file prefix marks BuiltinCommand as the hierarchy's deliberately open - # edge: the hook accepts subclasses from any file (src/dev/builtins/, test - # fakes), matching the static rule. - BuiltinCommand.instance_variable_set(:@sorbet_sealed_module_decl_file, "") - BuiltinCommand.instance_variable_set(:@sorbet_sealed_module_all_subclasses, []) + sig(:final) { override.returns(T::Boolean) } + def hidden? = @body.hidden? + + sig(:final) { override.returns(T::Boolean) } + def staleness_exempt? = @body.staleness_exempt? + + sig(:final) { override.returns(T::Boolean) } + def stamps? = @body.stamps? + + sig(:final) { params(args: T::Array[String], context: ExecutionContext).void } + def call(args:, context:) = @body.call(args:, context:) + end # Project command from a dev.yml `commands:` entry. Pure data: the run # string, optional description, repl flag, and container opt-out. When diff --git a/src/dev/command_repository.rb b/src/dev/command_repository.rb index c35fad0..eea700c 100644 --- a/src/dev/command_repository.rb +++ b/src/dev/command_repository.rb @@ -10,8 +10,8 @@ module Dev # builtin's slot — their OverriddenCommand composition. Data in, never a # path, never a parse. # - # Onion rule: CommandService is the only production consumer; the constant - # is private and construction is confined to the composition root. + # Onion rule: CommandService is the only production consumer, and + # construction is confined to the composition root. class CommandRepository extend T::Sig @@ -77,6 +77,4 @@ def assemble(builtins, project_commands) commands end end - - private_constant :CommandRepository end diff --git a/src/dev/runner.rb b/src/dev/runner.rb index d3fe3c2..7a4bd5e 100644 --- a/src/dev/runner.rb +++ b/src/dev/runner.rb @@ -3,6 +3,7 @@ require "pathname" require "stringio" +require "dev/builtin_body" require "dev/builtins" require "dev/cli" require "dev/command" @@ -105,10 +106,11 @@ def exit_for(error) end end - # The composition root: the one place the repository (private to this - # onion) and the builtin set are constructed. Which builtins exist is - # config-gated here — runner-setup only with a `runner:` block, - # provide-image/reset-container only with a build container. + # The composition root: the one place the repository (consumed only by + # CommandService, the onion rule) and the builtin set are constructed. + # Which builtins exist is config-gated here — runner-setup only with a + # `runner:` block, provide-image/reset-container only with a build + # container. # # @param manifest [ProjectManifest] # @return [CommandService] @@ -136,7 +138,7 @@ def build_command_service(manifest) end def build_builtins(manifest, dependency_service) install_deps = Builtins::InstallDepsCommand.new - builtins = T.let({ + bodies = T.let({ "update-deps" => Builtins::UpdateDepsCommand.new, "install-deps" => install_deps, # `up` composes the same install the install-deps builtin runs. @@ -149,11 +151,12 @@ def build_builtins(manifest, dependency_service) "cache" => Builtins::CacheCommand.new, "cred" => Builtins::CredCommand.new, "plan" => Builtins::PlanCommand.new, - }, T::Hash[String, BuiltinCommand]) - builtins["provide-image"] = Builtins::ProvideImageCommand.new if manifest.build_container - builtins["reset-container"] = Builtins::ResetContainerCommand.new if manifest.build_container&.persist - builtins["runner-setup"] = Builtins::RunnerSetupCommand.new if manifest.runner - builtins + }, T::Hash[String, BuiltinBody]) + bodies["provide-image"] = Builtins::ProvideImageCommand.new if manifest.build_container + bodies["reset-container"] = Builtins::ResetContainerCommand.new if manifest.build_container&.persist + bodies["runner-setup"] = Builtins::RunnerSetupCommand.new if manifest.runner + # Each body enters the sealed hierarchy through its final leaf here. + bodies.transform_values { |body| BuiltinCommand.new(body:) } end end end diff --git a/test/dev/command_executor_test.rb b/test/dev/command_executor_test.rb index cc01e1e..bdece13 100644 --- a/test/dev/command_executor_test.rb +++ b/test/dev/command_executor_test.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "test_helper" +require "dev/builtin_body" require "dev/command_executor" require "dev/command" require "shadowenv_ruby" @@ -9,16 +10,18 @@ require "pathname" require "tmpdir" -# Builtin fake for dispatch-order assertions; traits configurable so the -# override path can exercise both wait shapes. -class ExecutorFakeBuiltin < Dev::BuiltinCommand +# Body fake for dispatch-order assertions; traits configurable so the +# override path can exercise both wait shapes. Tests wrap it in +# BuiltinCommand the same way the composition root does. +class ExecutorFakeBody + include Dev::BuiltinBody + attr_reader :calls def initialize(stamps: false, &body) @stamps = stamps @calls = [] @body = body - super() end def desc = "a builtin" @@ -29,7 +32,7 @@ def call(args:, context:) @calls << [args, context] @body&.call end -end unless defined?(ExecutorFakeBuiltin) +end unless defined?(ExecutorFakeBody) transform!(RSpock::AST::Transformation) class Dev::CommandExecutorTest < Minitest::Test @@ -43,16 +46,16 @@ def build_context(project_root) test "a builtin command runs its Ruby body in-process" do Given "a builtin and an executor" - builtin = ExecutorFakeBuiltin.new + body = ExecutorFakeBody.new executor = Dev::CommandExecutor.new root = Pathname.new(Dir.mktmpdir("executor-builtin-")) context = build_context(root) When "executing" - executor.execute(builtin, args: ["--verbose"], context: context) + executor.execute(Dev::BuiltinCommand.new(body: body), args: ["--verbose"], context: context) Then "the body received args and context; no child process was involved" - builtin.calls == [[["--verbose"], context]] + body.calls == [[["--verbose"], context]] Cleanup FileUtils.rm_rf(root) @@ -88,9 +91,9 @@ def build_context(project_root) original_cwd = Dir.pwd root = Pathname.new(Dir.mktmpdir("executor-up-wait-")) execution_order = [] - builtin = ExecutorFakeBuiltin.new(stamps: true) { execution_order << :builtin_install } + body = ExecutorFakeBody.new(stamps: true) { execution_order << :builtin_install } command = Dev::OverriddenCommand.new( - builtin: builtin, + builtin: Dev::BuiltinCommand.new(body: body), project: Dev::ProjectCommand.new(run: "./bin/up.rb", desc: "Setup", container: false), ) executor = Dev::CommandExecutor.new @@ -117,7 +120,7 @@ def build_context(project_root) original_cwd = Dir.pwd root = Pathname.new(Dir.mktmpdir("executor-up-fail-")) command = Dev::OverriddenCommand.new( - builtin: ExecutorFakeBuiltin.new(stamps: true), + builtin: Dev::BuiltinCommand.new(body: ExecutorFakeBody.new(stamps: true)), project: Dev::ProjectCommand.new(run: "./bin/up.rb", desc: "Setup", container: false), ) executor = Dev::CommandExecutor.new @@ -150,9 +153,9 @@ def build_context(project_root) Given "a non-stamping builtin slot overridden by a project command" original_cwd = Dir.pwd root = Pathname.new(Dir.mktmpdir("executor-override-exec-")) - builtin = ExecutorFakeBuiltin.new(stamps: false) + body = ExecutorFakeBody.new(stamps: false) command = Dev::OverriddenCommand.new( - builtin: builtin, + builtin: Dev::BuiltinCommand.new(body: body), project: Dev::ProjectCommand.new(run: "./bin/lint.sh", desc: "Lint", container: false), ) executor = Dev::CommandExecutor.new @@ -162,7 +165,7 @@ def build_context(project_root) executor.execute(command, args: [], context: build_context(root)) Then "nothing sequences after execute, so the exec tail-call is safe and kept" - builtin.calls.size == 1 + body.calls.size == 1 1 * Kernel.exec(anything, "shadowenv", "exec", "--", "sh", "-c", includes("./bin/lint.sh")) 0 * Kernel.system(any_parameters) diff --git a/test/dev/command_repository_test.rb b/test/dev/command_repository_test.rb index 8382464..d9fa24c 100644 --- a/test/dev/command_repository_test.rb +++ b/test/dev/command_repository_test.rb @@ -2,22 +2,18 @@ # frozen_string_literal: true require "test_helper" +require "dev/builtin_body" require "dev/command_repository" require "dev/command" -# The repository is service-private in production (private_constant, onion -# rule); tests alias it through const_get rather than weakening the privacy. -CommandRepositoryUnderTest = Dev.const_get(:CommandRepository) unless defined?(CommandRepositoryUnderTest) -unless defined?(CommandNotFoundErrorUnderTest) - CommandNotFoundErrorUnderTest = CommandRepositoryUnderTest.const_get(:CommandNotFoundError) -end +# A named no-op body for assembly assertions; the tests wrap it in +# BuiltinCommand the same way the composition root does. +class RepositoryFakeBody + include Dev::BuiltinBody -# A named no-op builtin for assembly assertions. -class RepositoryFakeBuiltin < Dev::BuiltinCommand def initialize(desc: "a builtin", hidden: false) @desc = desc @hidden = hidden - super() end attr_reader :desc @@ -25,14 +21,18 @@ def initialize(desc: "a builtin", hidden: false) def hidden? = @hidden def call(args:, context:); end -end unless defined?(RepositoryFakeBuiltin) +end unless defined?(RepositoryFakeBody) transform!(RSpock::AST::Transformation) class Dev::CommandRepositoryTest < Minitest::Test + def build_builtin(desc: "a builtin", hidden: false) + Dev::BuiltinCommand.new(body: RepositoryFakeBody.new(desc: desc, hidden: hidden)) + end + test "fetch returns a builtin-only command as the builtin" do Given "a repository with one builtin and no project commands" - builtin = RepositoryFakeBuiltin.new(desc: "resolve deps") - repository = CommandRepositoryUnderTest.new(builtins: { "update-deps" => builtin }, project_commands: {}) + builtin = build_builtin(desc: "resolve deps") + repository = Dev::CommandRepository.new(builtins: { "update-deps" => builtin }, project_commands: {}) Expect "the builtin occupies its slot" repository.fetch("update-deps") == builtin @@ -41,7 +41,7 @@ class Dev::CommandRepositoryTest < Minitest::Test test "fetch returns a project-only command as the ProjectCommand" do Given "a repository with one project command and no builtins" project = Dev::ProjectCommand.new(run: "./bin/test.sh", desc: "Run tests") - repository = CommandRepositoryUnderTest.new(builtins: {}, project_commands: { "test" => project }) + repository = Dev::CommandRepository.new(builtins: {}, project_commands: { "test" => project }) Expect "the project command occupies its slot" repository.fetch("test") == project @@ -49,9 +49,9 @@ class Dev::CommandRepositoryTest < Minitest::Test test "a project command on a builtin's name composes into an OverriddenCommand" do Given "a repository where a project up: collides with the up builtin" - builtin = RepositoryFakeBuiltin.new(desc: "built-in up") + builtin = build_builtin(desc: "built-in up") project = Dev::ProjectCommand.new(run: "./bin/up.sh", desc: "project up") - repository = CommandRepositoryUnderTest.new( + repository = Dev::CommandRepository.new( builtins: { "up" => builtin }, project_commands: { "up" => project }, ) @@ -68,20 +68,20 @@ class Dev::CommandRepositoryTest < Minitest::Test test "fetch raises CommandNotFoundError for an unknown name" do Given "an empty repository" - repository = CommandRepositoryUnderTest.new(builtins: {}, project_commands: {}) + repository = Dev::CommandRepository.new(builtins: {}, project_commands: {}) When "fetching a nonexistent command" repository.fetch("nope") Then - raises CommandNotFoundErrorUnderTest + raises Dev::CommandRepository::CommandNotFoundError end test "visible_commands lists builtins then project commands, overrides in the builtin's position" do Given "a repository with a builtin, a project command, and an override" - builtin = RepositoryFakeBuiltin.new(desc: "built-in up") - repository = CommandRepositoryUnderTest.new( - builtins: { "update-deps" => RepositoryFakeBuiltin.new(desc: "resolve"), "up" => builtin }, + builtin = build_builtin(desc: "built-in up") + repository = Dev::CommandRepository.new( + builtins: { "update-deps" => build_builtin(desc: "resolve"), "up" => builtin }, project_commands: { "up" => Dev::ProjectCommand.new(run: "./bin/up.sh", desc: "project up"), "test" => Dev::ProjectCommand.new(run: "rspec", desc: "Run tests"), @@ -98,9 +98,9 @@ class Dev::CommandRepositoryTest < Minitest::Test test "visible_commands omits hidden commands but fetch still resolves them" do Given "a repository with a hidden builtin" - hidden = RepositoryFakeBuiltin.new(desc: "plumbing", hidden: true) - repository = CommandRepositoryUnderTest.new( - builtins: { "provide-image" => hidden, "up" => RepositoryFakeBuiltin.new }, + hidden = build_builtin(desc: "plumbing", hidden: true) + repository = Dev::CommandRepository.new( + builtins: { "provide-image" => hidden, "up" => build_builtin }, project_commands: {}, ) diff --git a/test/dev/command_service_test.rb b/test/dev/command_service_test.rb index 1df5aa9..a341627 100644 --- a/test/dev/command_service_test.rb +++ b/test/dev/command_service_test.rb @@ -2,20 +2,23 @@ # frozen_string_literal: true require "test_helper" +require "dev/builtin_body" require "dev/command_service" require "dev/command" require "pathname" -# Builtin fake whose traits drive the service's guard/stamp decisions and -# whose call records the dispatch. -class ServiceFakeBuiltin < Dev::BuiltinCommand +# Body fake whose traits drive the service's guard/stamp decisions and +# whose call records the dispatch; tests wrap it in BuiltinCommand the +# same way the composition root does. +class ServiceFakeBody + include Dev::BuiltinBody + attr_reader :calls def initialize(staleness_exempt: false, stamps: false) @staleness_exempt = staleness_exempt @stamps = stamps @calls = [] - super() end def desc = "a builtin" @@ -27,23 +30,24 @@ def stamps? = @stamps def call(args:, context:) @calls << [args, context] end -end unless defined?(ServiceFakeBuiltin) +end unless defined?(ServiceFakeBody) transform!(RSpock::AST::Transformation) class Dev::CommandServiceTest < Minitest::Test include SorbetHelper - CommandRepositoryClass = Dev.const_get(:CommandRepository) - CommandNotFoundErrorClass = CommandRepositoryClass.const_get(:CommandNotFoundError) - def build_service(builtins:, dependency_service:, executor: Dev::CommandExecutor.new) Dev::CommandService.new( - repository: CommandRepositoryClass.new(builtins: builtins, project_commands: {}), + repository: Dev::CommandRepository.new(builtins: builtins, project_commands: {}), executor: executor, dependency_service: dependency_service, ) end + def wrap(body) + Dev::BuiltinCommand.new(body: body) + end + def fake_context Dev::ExecutionContext.new( ui: typed_mock(Dev::Cli::Ui), @@ -61,15 +65,15 @@ def fake_dependency_service test "execute fetches the command, dispatches it, and passes args and context through" do Given "a service over one builtin" - builtin = ServiceFakeBuiltin.new(staleness_exempt: true) - service = build_service(builtins: { "deps" => builtin }, dependency_service: fake_dependency_service) + body = ServiceFakeBody.new(staleness_exempt: true) + service = build_service(builtins: { "deps" => wrap(body) }, dependency_service: fake_dependency_service) context = fake_context When "executing the command" service.execute("deps", args: ["path", "xcode"], context: context) Then "the builtin body ran once with the args and context" - builtin.calls == [[["path", "xcode"], context]] + body.calls == [[["path", "xcode"], context]] end test "execute raises CommandNotFoundError (the repository's own) for an unknown name" do @@ -80,7 +84,7 @@ def fake_dependency_service service.execute("nonexistent", args: [], context: fake_context) Then "the error bubbles under its native namespace" - raises CommandNotFoundErrorClass + raises Dev::CommandRepository::CommandNotFoundError end test "execute guards staleness before a non-exempt command" do @@ -88,7 +92,7 @@ def fake_dependency_service dependency_service = typed_mock(Dev::DependencyService) dependency_service.expects(:guard!).once service = build_service( - builtins: { "build" => ServiceFakeBuiltin.new(staleness_exempt: false) }, + builtins: { "build" => wrap(ServiceFakeBody.new(staleness_exempt: false)) }, dependency_service: dependency_service, ) @@ -105,7 +109,7 @@ def fake_dependency_service dependency_service.expects(:guard!).never dependency_service.stubs(:lock!) service = build_service( - builtins: { "update-deps" => ServiceFakeBuiltin.new(staleness_exempt: true) }, + builtins: { "update-deps" => wrap(ServiceFakeBody.new(staleness_exempt: true)) }, dependency_service: dependency_service, ) @@ -122,7 +126,7 @@ def fake_dependency_service dependency_service.stubs(:guard!) dependency_service.expects(:lock!).once service = build_service( - builtins: { "install-deps" => ServiceFakeBuiltin.new(staleness_exempt: true, stamps: true) }, + builtins: { "install-deps" => wrap(ServiceFakeBody.new(staleness_exempt: true, stamps: true)) }, dependency_service: dependency_service, ) @@ -139,7 +143,7 @@ def fake_dependency_service dependency_service.stubs(:guard!) dependency_service.expects(:lock!).never service = build_service( - builtins: { "deps" => ServiceFakeBuiltin.new }, + builtins: { "deps" => wrap(ServiceFakeBody.new) }, dependency_service: dependency_service, ) @@ -158,7 +162,7 @@ def fake_dependency_service executor = typed_mock(Dev::CommandExecutor) executor.stubs(:execute).raises(Dev::CommandRunner::CommandFailedError.new(exit_status: 7)) service = build_service( - builtins: { "up" => ServiceFakeBuiltin.new(staleness_exempt: true, stamps: true) }, + builtins: { "up" => wrap(ServiceFakeBody.new(staleness_exempt: true, stamps: true)) }, dependency_service: dependency_service, executor: executor, ) @@ -172,10 +176,10 @@ def fake_dependency_service test "visible_commands serves the repository's usage view" do Given "a service over one visible builtin" - builtin = ServiceFakeBuiltin.new + builtin = wrap(ServiceFakeBody.new) service = build_service(builtins: { "deps" => builtin }, dependency_service: fake_dependency_service) - Expect "the usage view flows through the service (the repository stays private)" + Expect "the usage view flows through the service (the onion rule)" service.visible_commands == { "deps" => builtin } end end diff --git a/test/dev/command_test.rb b/test/dev/command_test.rb index d4abf4e..857d684 100644 --- a/test/dev/command_test.rb +++ b/test/dev/command_test.rb @@ -2,21 +2,26 @@ # frozen_string_literal: true require "test_helper" +require "dev/builtin_body" require "dev/command" -# A minimal concrete builtin for exercising the abstract base's defaults -# and the OverriddenCommand composition. -class FakeBuiltinCommand < Dev::BuiltinCommand - def initialize(desc: "fake builtin", staleness_exempt: false, stamps: false, &body) +# A minimal BuiltinBody for exercising the trait defaults, the +# BuiltinCommand delegation, and the OverriddenCommand composition. +class FakeBuiltinBody + include Dev::BuiltinBody + + def initialize(desc: "fake builtin", hidden: false, staleness_exempt: false, stamps: false, &body) @desc = desc + @hidden = hidden @staleness_exempt = staleness_exempt @stamps = stamps @body = body - super() end attr_reader :desc + def hidden? = @hidden + def staleness_exempt? = @staleness_exempt def stamps? = @stamps @@ -24,11 +29,12 @@ def stamps? = @stamps def call(args:, context:) @body&.call(args, context) end -end unless defined?(FakeBuiltinCommand) +end unless defined?(FakeBuiltinBody) transform!(RSpock::AST::Transformation) class CommandTest < Minitest::Test extend T::Sig + include SorbetHelper test "initialize with only run uses default desc and repl" do Given "we build a ProjectCommand with only run" @@ -96,9 +102,53 @@ class CommandTest < Minitest::Test cmd.hidden? end + test "a builtin body without trait overrides gets the defaults" do + Given "a body defining only desc and call" + body = Class.new do + include Dev::BuiltinBody + + def desc = "minimal" + + def call(args:, context:); end + end.new + + Expect "the BuiltinBody trait defaults hold" + !body.hidden? + !body.staleness_exempt? + !body.stamps? + end + + test "a BuiltinCommand delegates call, desc, and every trait to its body" do + Given "a distinctive body wrapped in the sealed hierarchy's final leaf" + calls = [] + body = FakeBuiltinBody.new(desc: "wrapped", hidden: true, staleness_exempt: true, stamps: true) do |args, context| + calls << [args, context] + end + cmd = Dev::BuiltinCommand.new(body: body) + context = typed_mock(Dev::ExecutionContext) + + When "calling through the wrapper" + cmd.call(args: ["--verbose"], context: context) + + Then "every reading came from the body" + cmd.desc == "wrapped" + cmd.hidden? + cmd.staleness_exempt? + cmd.stamps? + calls == [[["--verbose"], context]] + end + + test "BuiltinCommand is final: subclassing raises, keeping the sealed hierarchy closed" do + When "declaring a subclass of the final leaf" + Class.new(Dev::BuiltinCommand) + + Then "sorbet-runtime rejects the open edge" + raises RuntimeError + end + test "an OverriddenCommand takes desc and hidden from the project override" do Given "a builtin slot overridden by a hidden project command" - builtin = FakeBuiltinCommand.new(desc: "builtin up") + builtin = Dev::BuiltinCommand.new(body: FakeBuiltinBody.new(desc: "builtin up")) project = Dev::ProjectCommand.new(run: "./bin/up.sh", desc: "project up", hidden: true) cmd = Dev::OverriddenCommand.new(builtin: builtin, project: project) @@ -109,7 +159,7 @@ class CommandTest < Minitest::Test test "an OverriddenCommand takes guard and stamp traits from the builtin slot" do Given "a stamping, staleness-exempt builtin slot overridden by a project command" - builtin = FakeBuiltinCommand.new(staleness_exempt: true, stamps: true) + builtin = Dev::BuiltinCommand.new(body: FakeBuiltinBody.new(staleness_exempt: true, stamps: true)) project = Dev::ProjectCommand.new(run: "./bin/up.sh", desc: "project up") cmd = Dev::OverriddenCommand.new(builtin: builtin, project: project) @@ -120,7 +170,7 @@ class CommandTest < Minitest::Test test "an OverriddenCommand exposes its typed halves" do Given "an overridden command" - builtin = FakeBuiltinCommand.new + builtin = Dev::BuiltinCommand.new(body: FakeBuiltinBody.new) project = Dev::ProjectCommand.new(run: "./bin/up.sh") cmd = Dev::OverriddenCommand.new(builtin: builtin, project: project) From 5bcc76f9f006c139af7d8dd51ad7ac861a42c059 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Date: Tue, 18 Aug 2026 12:25:47 -0400 Subject: [PATCH 2/3] Rework the seal: Command becomes a sealed module, BuiltinCommand the abstract open-edge class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A sealed module's included hook fires only for direct includers (include never transfers singleton methods), so the hierarchy closes without the BuiltinBody indirection or the final wrapper leaf: builtins subclass BuiltinCommand directly, the data leaves are final!, and the composition root wires builtins straight in. BuiltinCommand stays a class because Sorbet flattens module mixins — a module open edge would re-include the sealed Command in every builtin and fail the same-file check statically. Co-authored-by: Cursor --- src/dev/builtin_body.rb | 43 -------- src/dev/builtins.rb | 12 +- src/dev/builtins/cache_command.rb | 5 +- src/dev/builtins/cd_command.rb | 5 +- src/dev/builtins/check_command.rb | 5 +- src/dev/builtins/clone_command.rb | 5 +- src/dev/builtins/cred_command.rb | 5 +- src/dev/builtins/deps_command.rb | 5 +- src/dev/builtins/install_deps_command.rb | 5 +- src/dev/builtins/learnings_command.rb | 5 +- src/dev/builtins/plan_command.rb | 5 +- src/dev/builtins/provide_image_command.rb | 5 +- src/dev/builtins/reset_container_command.rb | 5 +- src/dev/builtins/runner_setup_command.rb | 5 +- src/dev/builtins/up_command.rb | 5 +- src/dev/builtins/update_deps_command.rb | 5 +- src/dev/command.rb | 115 ++++++++++---------- src/dev/runner.rb | 14 +-- test/dev/command_executor_test.rb | 32 +++--- test/dev/command_repository_test.rb | 12 +- test/dev/command_service_test.rb | 36 +++--- test/dev/command_test.rb | 77 ++++++------- 22 files changed, 171 insertions(+), 240 deletions(-) delete mode 100644 src/dev/builtin_body.rb diff --git a/src/dev/builtin_body.rb b/src/dev/builtin_body.rb deleted file mode 100644 index 9108f81..0000000 --- a/src/dev/builtin_body.rb +++ /dev/null @@ -1,43 +0,0 @@ -# typed: strict -# frozen_string_literal: true - -require_relative "execution_context" - -module Dev - # The behavior contract of a builtin dev command: a Ruby body plus the - # trait readings BuiltinCommand delegates to. Implementations live under - # src/dev/builtins/ (one class per builtin, collaborators - # constructor-injected); the composition root wraps each body in - # BuiltinCommand, the sealed hierarchy's final leaf. - # - # This would be an `interface!`, but Sorbet interfaces forbid default - # implementations and the traits deliberately default (most builtins are - # visible, guarded, and non-stamping) — hence `abstract!` with - # overridable trait defaults mirroring Command's. - module BuiltinBody - extend T::Sig - extend T::Helpers - abstract! - - sig { abstract.returns(String) } - def desc; end - - # Whether this builtin is callable but omitted from `dev`/`dev --help` - # usage (see Command#hidden?). Visible by default. - sig { overridable.returns(T::Boolean) } - def hidden? = false - - # Whether the staleness guard skips this builtin (see - # Command#staleness_exempt?). Guarded by default. - sig { overridable.returns(T::Boolean) } - def staleness_exempt? = false - - # Whether a fully-successful run records the installed stamp (see - # Command#stamps?). Non-stamping by default. - sig { overridable.returns(T::Boolean) } - def stamps? = false - - sig { abstract.params(args: T::Array[String], context: ExecutionContext).void } - def call(args:, context:); end - end -end diff --git a/src/dev/builtins.rb b/src/dev/builtins.rb index af727b0..0eb0ede 100644 --- a/src/dev/builtins.rb +++ b/src/dev/builtins.rb @@ -2,12 +2,12 @@ # frozen_string_literal: true module Dev - # One class per builtin dev command, each implementing BuiltinBody with - # its collaborators constructor-injected; per-call values stay - # method-side. The composition root (Runner) decides which builtins exist - # for a given project (config-gated: runner-setup only with a `runner:` - # block, provide-image/reset-container only with a build container) and - # wraps each body in the sealed hierarchy's BuiltinCommand leaf. + # One class per builtin dev command, each subclassing BuiltinCommand — + # the sealed Command hierarchy's declared open edge — with its + # collaborators constructor-injected; per-call values stay method-side. + # The composition root (Runner) decides which builtins exist for a given + # project (config-gated: runner-setup only with a `runner:` block, + # provide-image/reset-container only with a build container). module Builtins; end end diff --git a/src/dev/builtins/cache_command.rb b/src/dev/builtins/cache_command.rb index fbaad73..f130fe7 100644 --- a/src/dev/builtins/cache_command.rb +++ b/src/dev/builtins/cache_command.rb @@ -1,8 +1,8 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" require "dev/cli/flag_parser" +require "dev/command" require "dev/deps/cache_gc" require "dev/deps/lockfile" require "build_container" @@ -11,9 +11,8 @@ module Dev module Builtins # `dev cache gc [--keep N]`: reclaim stale install-dir versions and, when # a build container is configured, its stale content-tagged images. - class CacheCommand + class CacheCommand < BuiltinCommand extend T::Sig - include BuiltinBody # Builds the GC over a project's lockfile (per-call project root). CacheGcFactory = T.type_alias do diff --git a/src/dev/builtins/cd_command.rb b/src/dev/builtins/cd_command.rb index 8eb535b..65f2b2f 100644 --- a/src/dev/builtins/cd_command.rb +++ b/src/dev/builtins/cd_command.rb @@ -1,17 +1,16 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" require "dev/cd" +require "dev/command" module Dev module Builtins # `dev cd` is dispatched globally (before dev.yml lookup) in bin/dev; # this builtin only surfaces it in `dev --help` and keeps it callable # inside a project. - class CdCommand + class CdCommand < BuiltinCommand extend T::Sig - include BuiltinBody sig { params(accessor: Dev::Cd::Accessor).void } def initialize(accessor: Dev::Cd::Accessor.new) diff --git a/src/dev/builtins/check_command.rb b/src/dev/builtins/check_command.rb index 9b0c453..010e2e6 100644 --- a/src/dev/builtins/check_command.rb +++ b/src/dev/builtins/check_command.rb @@ -1,16 +1,15 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" +require "dev/command" require "dev/dependency_service" module Dev module Builtins # `dev check`: report the dependency-state freshness the staleness guard # would act on, and exit non-zero when anything drifted. - class CheckCommand + class CheckCommand < BuiltinCommand extend T::Sig - include BuiltinBody sig { params(dependency_service: DependencyService).void } def initialize(dependency_service:) diff --git a/src/dev/builtins/clone_command.rb b/src/dev/builtins/clone_command.rb index 829797b..368ad38 100644 --- a/src/dev/builtins/clone_command.rb +++ b/src/dev/builtins/clone_command.rb @@ -1,17 +1,16 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" require "dev/clone" +require "dev/command" module Dev module Builtins # `dev clone` is dispatched globally (before dev.yml lookup) in bin/dev; # this builtin only surfaces it in `dev --help` and keeps it callable # inside a project. - class CloneCommand + class CloneCommand < BuiltinCommand extend T::Sig - include BuiltinBody sig { params(accessor: Dev::Clone::Accessor).void } def initialize(accessor: Dev::Clone::Accessor.new) diff --git a/src/dev/builtins/cred_command.rb b/src/dev/builtins/cred_command.rb index af9140f..3b33161 100644 --- a/src/dev/builtins/cred_command.rb +++ b/src/dev/builtins/cred_command.rb @@ -1,7 +1,7 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" +require "dev/command" require "dev/credential_accessor" module Dev @@ -9,9 +9,8 @@ module Builtins # `dev cred` is dispatched globally (before dev.yml lookup) in bin/dev; # this builtin only surfaces it in `dev --help` and keeps it callable # inside a project. - class CredCommand + class CredCommand < BuiltinCommand extend T::Sig - include BuiltinBody sig { params(accessor: Dev::CredentialAccessor).void } def initialize(accessor: Dev::CredentialAccessor.new) diff --git a/src/dev/builtins/deps_command.rb b/src/dev/builtins/deps_command.rb index 00ae072..ee4ecfd 100644 --- a/src/dev/builtins/deps_command.rb +++ b/src/dev/builtins/deps_command.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require "pathname" -require "dev/builtin_body" +require "dev/command" require "dev/deps/accessor" require "dev/deps/cache" require "dev/deps/lockfile" @@ -11,9 +11,8 @@ module Dev module Builtins # `dev deps`: read-only lookups over the lockfile + content cache (e.g. # `dev deps path ficsit `). - class DepsCommand + class DepsCommand < BuiltinCommand extend T::Sig - include BuiltinBody # Builds the accessor over a project's lockfile (the project root is a # per-call value, so the collaborator arrives as a factory). diff --git a/src/dev/builtins/install_deps_command.rb b/src/dev/builtins/install_deps_command.rb index 46916ad..7aa6fe0 100644 --- a/src/dev/builtins/install_deps_command.rb +++ b/src/dev/builtins/install_deps_command.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require "pathname" -require "dev/builtin_body" +require "dev/command" require "dev/deps" require "dev/deps/cache" require "dev/deps/gem_skill_linker" @@ -18,9 +18,8 @@ module Builtins # machine — shared with the `up` builtin, which composes this command. # Host integrations install on the host (not the build container) so # their artifacts can be volume-mounted in. - class InstallDepsCommand + class InstallDepsCommand < BuiltinCommand extend T::Sig - include BuiltinBody # Builds the DependencyInstaller for a lockfile + integrations pair; # injected so tests can substitute a fake without touching the host. diff --git a/src/dev/builtins/learnings_command.rb b/src/dev/builtins/learnings_command.rb index b10b090..44c08b4 100644 --- a/src/dev/builtins/learnings_command.rb +++ b/src/dev/builtins/learnings_command.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require "pathname" -require "dev/builtin_body" +require "dev/command" require "dev/learnings" module Dev @@ -10,9 +10,8 @@ module Builtins # `dev learnings` is dispatched globally (before dev.yml lookup) in # bin/dev; this builtin only surfaces it in `dev --help` and keeps it # callable inside a project. - class LearningsCommand + class LearningsCommand < BuiltinCommand extend T::Sig - include BuiltinBody # Builds the accessor for the enclosing project (per-call root). AccessorFactory = T.type_alias do diff --git a/src/dev/builtins/plan_command.rb b/src/dev/builtins/plan_command.rb index 2eec311..4109640 100644 --- a/src/dev/builtins/plan_command.rb +++ b/src/dev/builtins/plan_command.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require "pathname" -require "dev/builtin_body" +require "dev/command" require "dev/plan" module Dev @@ -10,9 +10,8 @@ module Builtins # `dev plan` is dispatched globally (before dev.yml lookup) in bin/dev; # this builtin only surfaces it in `dev --help` and keeps it callable # inside a project. - class PlanCommand + class PlanCommand < BuiltinCommand extend T::Sig - include BuiltinBody # Builds the accessor for the enclosing project (per-call root). AccessorFactory = T.type_alias do diff --git a/src/dev/builtins/provide_image_command.rb b/src/dev/builtins/provide_image_command.rb index 0f56dd2..5d1e09a 100644 --- a/src/dev/builtins/provide_image_command.rb +++ b/src/dev/builtins/provide_image_command.rb @@ -1,7 +1,7 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" +require "dev/command" require "dev/credentials" require "build_container" @@ -13,9 +13,8 @@ module Builtins # tag is capturable). Publishing to the shared registry stays gated on # DEV_PUBLISH_IMAGE, same as containerized commands. Exists only when a # build container is configured (the composition root gates it). - class ProvideImageCommand + class ProvideImageCommand < BuiltinCommand extend T::Sig - include BuiltinBody sig { override.returns(String) } def desc = "Resolve the build container image (local/pull/build) and print its tag" diff --git a/src/dev/builtins/reset_container_command.rb b/src/dev/builtins/reset_container_command.rb index b94fd85..f36842f 100644 --- a/src/dev/builtins/reset_container_command.rb +++ b/src/dev/builtins/reset_container_command.rb @@ -1,16 +1,15 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" +require "dev/command" require "build_container" module Dev module Builtins # Teardown for the persistent build container, only where a project opts # in (build.container.persist — the composition root gates it). - class ResetContainerCommand + class ResetContainerCommand < BuiltinCommand extend T::Sig - include BuiltinBody sig { override.returns(String) } def desc = "Remove the persistent build container (clears its incremental cache)" diff --git a/src/dev/builtins/runner_setup_command.rb b/src/dev/builtins/runner_setup_command.rb index 8ed0c55..925be32 100644 --- a/src/dev/builtins/runner_setup_command.rb +++ b/src/dev/builtins/runner_setup_command.rb @@ -1,8 +1,8 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" require "dev/cli/flag_parser" +require "dev/command" require "dev/runner_setup" require "dev/runner_setup_config" @@ -18,9 +18,8 @@ module Builtins # bespoke setup script. `--labels`/`--dir`/`--name` override the block # for hosts that differ from the repo default (e.g. registering the Mac # org-wide from a repo whose block describes the CI box). - class RunnerSetupCommand + class RunnerSetupCommand < BuiltinCommand extend T::Sig - include BuiltinBody # Builds the RunnerSetup for the resolved config and flags; injected # so tests can observe the wiring without touching gh or the host. diff --git a/src/dev/builtins/up_command.rb b/src/dev/builtins/up_command.rb index 07cbbca..0271382 100644 --- a/src/dev/builtins/up_command.rb +++ b/src/dev/builtins/up_command.rb @@ -1,8 +1,8 @@ # typed: strict # frozen_string_literal: true -require "dev/builtin_body" require "dev/cd" +require "dev/command" require "dev/credentials" module Dev @@ -13,9 +13,8 @@ module Builtins # provisioning. Projects with only a dependencies.rb get `dev up` for # free. `up` also ensures the `dev cd` shell hook (idempotent) — # provisioning is where dev's RC hooks land, next to the shadowenv one. - class UpCommand + class UpCommand < BuiltinCommand extend T::Sig - include BuiltinBody sig do params( diff --git a/src/dev/builtins/update_deps_command.rb b/src/dev/builtins/update_deps_command.rb index d947901..0127ae8 100644 --- a/src/dev/builtins/update_deps_command.rb +++ b/src/dev/builtins/update_deps_command.rb @@ -3,7 +3,7 @@ require "digest" require "pathname" -require "dev/builtin_body" +require "dev/command" require "dev/deps" require "dev/deps/lockfile" require "dev/deps/registry" @@ -14,9 +14,8 @@ module Builtins # `dev update-deps`: resolve the dependencies.rb declarations and write # the lockfiles. Everything here is derived from the per-call project # root, so no collaborators need injecting. - class UpdateDepsCommand + class UpdateDepsCommand < BuiltinCommand extend T::Sig - include BuiltinBody sig { override.returns(String) } def desc = "Resolve dependency constraints and write lockfiles" diff --git a/src/dev/command.rb b/src/dev/command.rb index b8fb977..6d9a2b7 100644 --- a/src/dev/command.rb +++ b/src/dev/command.rb @@ -1,24 +1,34 @@ # typed: strict # frozen_string_literal: true -require_relative "builtin_body" require_relative "execution_context" module Dev - # Sealed command data hierarchy. A command is one of exactly three shapes: + # Sealed command hierarchy. A command is one of exactly three shapes: # - # - BuiltinCommand: a final wrapper delegating to the BuiltinBody it - # holds (bodies live under src/dev/builtins/) + # - BuiltinCommand: a Ruby body dev ships (an abstract class, the + # hierarchy's one declared open edge; subclasses live under + # src/dev/builtins/) # - ProjectCommand: pure data parsed from a dev.yml `commands:` entry # - OverriddenCommand: a project command occupying a builtin's slot (the # builtin runs first, like a hardcoded super()) # - # Sealing makes any other nesting unrepresentable: CommandExecutor - # dispatches exhaustively over these three variants (case + T.absurd). - # Sorbet requires the direct subclasses of a sealed class beside it, which - # is why the whole hierarchy shares this file; every leaf is closed - # (BuiltinCommand is final!), so the seal holds with no open edge. - class Command + # Sealing makes a fourth variant unrepresentable: CommandExecutor + # dispatches exhaustively over these three (case + T.absurd), and Sorbet + # requires a sealed module's direct heirs beside it, which is why the + # hierarchy shares this file. + # + # Command is a module rather than a class deliberately. A sealed class's + # runtime `inherited` hook rides down the singleton chain to every + # descendant, so builtins subclassing an abstract BuiltinCommand class + # raise at definition time unless sorbet-runtime internals are faked open + # (the ivar pokes this file used to carry). A sealed module's `included` + # hook fires only for its direct includers — the three heirs below — + # because `include` never transfers singleton methods, so subclassing + # BuiltinCommand is an honest open edge with nothing to suppress. Descent + # is closed everywhere it is not explicitly declared: the two data leaves + # are final!. + module Command extend T::Sig extend T::Helpers abstract! @@ -47,60 +57,52 @@ def staleness_exempt? = false def stamps? = false end - # Built-in command that executes Ruby code: a concrete, final wrapper - # holding the BuiltinBody it delegates everything to. The Ruby bodies - # live under src/dev/builtins/, one class per builtin; the composition - # root wraps each in this leaf. final! means no subclasses can exist, so - # Command's sealed runtime hook has nothing to reject — the hierarchy is - # closed without reaching into sorbet-runtime internals. - class BuiltinCommand < Command + # Built-in command that executes Ruby code: the hierarchy's declared open + # edge. Subclasses live under src/dev/builtins/, one class per builtin, + # with collaborators injected through their constructors; per-call values + # arrive through #call. Test fakes subclass it the same way. + # + # A class rather than a module because Sorbet flattens module mixins: + # were this a module, every includer would gain sealed Command as a + # direct mixin in the symbol table and fail the same-file check + # statically. A superclass edge is not flattened, so subclasses inherit + # Command's membership without re-including it — legal statically, and + # invisible to the seal's runtime hooks. + class BuiltinCommand extend T::Sig extend T::Helpers - final! - - sig(:final) { params(body: BuiltinBody).void } - def initialize(body:) - @body = T.let(body, BuiltinBody) - end - - sig(:final) { override.returns(String) } - def desc = @body.desc - - sig(:final) { override.returns(T::Boolean) } - def hidden? = @body.hidden? - - sig(:final) { override.returns(T::Boolean) } - def staleness_exempt? = @body.staleness_exempt? - - sig(:final) { override.returns(T::Boolean) } - def stamps? = @body.stamps? + include Command + abstract! - sig(:final) { params(args: T::Array[String], context: ExecutionContext).void } - def call(args:, context:) = @body.call(args:, context:) + sig { abstract.params(args: T::Array[String], context: ExecutionContext).void } + def call(args:, context:); end end # Project command from a dev.yml `commands:` entry. Pure data: the run # string, optional description, repl flag, and container opt-out. When # build.container is declared, commands run inside the container by # default unless container: false. - class ProjectCommand < Command + class ProjectCommand extend T::Sig + extend T::Helpers + include Command + final! - sig { returns(String) } + sig(:final) { returns(String) } attr_reader :run - sig { override.returns(String) } + sig(:final) { override.returns(String) } attr_reader :desc - sig { returns(T::Boolean) } + sig(:final) { returns(T::Boolean) } attr_reader :repl # Whether this command should run inside the build container (when one is # configured). Defaults to true; set to false via `container: false` in dev.yml. - sig { returns(T::Boolean) } + sig(:final) { returns(T::Boolean) } attr_reader :container - sig do + sig(:final) do params(run: String, desc: String, repl: T::Boolean, container: T::Boolean, hidden: T::Boolean).void end def initialize(run:, desc: "(no description)", repl: false, container: true, hidden: false) @@ -111,10 +113,10 @@ def initialize(run:, desc: "(no description)", repl: false, container: true, hid @hidden = T.let(hidden, T::Boolean) end - sig { override.returns(T::Boolean) } + sig(:final) { override.returns(T::Boolean) } def hidden? = @hidden - sig { params(other: Object).returns(T::Boolean) } + sig(:final) { params(other: Object).returns(T::Boolean) } def ==(other) return false unless other.is_a?(ProjectCommand) @@ -122,12 +124,12 @@ def ==(other) @container == other.container && @hidden == other.hidden? end - sig { params(other: Object).returns(T::Boolean) } + sig(:final) { params(other: Object).returns(T::Boolean) } def eql?(other) self == other end - sig { returns(Integer) } + sig(:final) { returns(Integer) } def hash [@run, @desc, @repl, @container, @hidden].hash end @@ -137,16 +139,19 @@ def hash # dispatch: the override owns the slot, and its implementation calls # super() at the top — CommandExecutor runs the builtin body first, then # the project command. - class OverriddenCommand < Command + class OverriddenCommand extend T::Sig + extend T::Helpers + include Command + final! - sig { returns(BuiltinCommand) } + sig(:final) { returns(BuiltinCommand) } attr_reader :builtin - sig { returns(ProjectCommand) } + sig(:final) { returns(ProjectCommand) } attr_reader :project - sig { params(builtin: BuiltinCommand, project: ProjectCommand).void } + sig(:final) { params(builtin: BuiltinCommand, project: ProjectCommand).void } def initialize(builtin:, project:) @builtin = T.let(builtin, BuiltinCommand) @project = T.let(project, ProjectCommand) @@ -154,19 +159,19 @@ def initialize(builtin:, project:) # The override owns the slot, so its description wins — a project `up:` # shows its own desc in usage, not the generic builtin one. - sig { override.returns(String) } + sig(:final) { override.returns(String) } def desc = @project.desc - sig { override.returns(T::Boolean) } + sig(:final) { override.returns(T::Boolean) } def hidden? = @project.hidden? # Guard and stamp traits belong to the slot, not the override: a project # `up:` still is the provisioning command, so it inherits the builtin's # exemption and stamping behavior. - sig { override.returns(T::Boolean) } + sig(:final) { override.returns(T::Boolean) } def staleness_exempt? = @builtin.staleness_exempt? - sig { override.returns(T::Boolean) } + sig(:final) { override.returns(T::Boolean) } def stamps? = @builtin.stamps? end end diff --git a/src/dev/runner.rb b/src/dev/runner.rb index 7a4bd5e..3dc9aed 100644 --- a/src/dev/runner.rb +++ b/src/dev/runner.rb @@ -3,7 +3,6 @@ require "pathname" require "stringio" -require "dev/builtin_body" require "dev/builtins" require "dev/cli" require "dev/command" @@ -138,7 +137,7 @@ def build_command_service(manifest) end def build_builtins(manifest, dependency_service) install_deps = Builtins::InstallDepsCommand.new - bodies = T.let({ + builtins = T.let({ "update-deps" => Builtins::UpdateDepsCommand.new, "install-deps" => install_deps, # `up` composes the same install the install-deps builtin runs. @@ -151,12 +150,11 @@ def build_builtins(manifest, dependency_service) "cache" => Builtins::CacheCommand.new, "cred" => Builtins::CredCommand.new, "plan" => Builtins::PlanCommand.new, - }, T::Hash[String, BuiltinBody]) - bodies["provide-image"] = Builtins::ProvideImageCommand.new if manifest.build_container - bodies["reset-container"] = Builtins::ResetContainerCommand.new if manifest.build_container&.persist - bodies["runner-setup"] = Builtins::RunnerSetupCommand.new if manifest.runner - # Each body enters the sealed hierarchy through its final leaf here. - bodies.transform_values { |body| BuiltinCommand.new(body:) } + }, T::Hash[String, BuiltinCommand]) + builtins["provide-image"] = Builtins::ProvideImageCommand.new if manifest.build_container + builtins["reset-container"] = Builtins::ResetContainerCommand.new if manifest.build_container&.persist + builtins["runner-setup"] = Builtins::RunnerSetupCommand.new if manifest.runner + builtins end end end diff --git a/test/dev/command_executor_test.rb b/test/dev/command_executor_test.rb index bdece13..63fe26d 100644 --- a/test/dev/command_executor_test.rb +++ b/test/dev/command_executor_test.rb @@ -2,7 +2,6 @@ # frozen_string_literal: true require "test_helper" -require "dev/builtin_body" require "dev/command_executor" require "dev/command" require "shadowenv_ruby" @@ -10,12 +9,9 @@ require "pathname" require "tmpdir" -# Body fake for dispatch-order assertions; traits configurable so the -# override path can exercise both wait shapes. Tests wrap it in -# BuiltinCommand the same way the composition root does. -class ExecutorFakeBody - include Dev::BuiltinBody - +# Builtin fake for dispatch-order assertions; traits configurable so the +# override path can exercise both wait shapes. +class ExecutorFakeBuiltin < Dev::BuiltinCommand attr_reader :calls def initialize(stamps: false, &body) @@ -32,7 +28,7 @@ def call(args:, context:) @calls << [args, context] @body&.call end -end unless defined?(ExecutorFakeBody) +end unless defined?(ExecutorFakeBuiltin) transform!(RSpock::AST::Transformation) class Dev::CommandExecutorTest < Minitest::Test @@ -46,16 +42,16 @@ def build_context(project_root) test "a builtin command runs its Ruby body in-process" do Given "a builtin and an executor" - body = ExecutorFakeBody.new + builtin = ExecutorFakeBuiltin.new executor = Dev::CommandExecutor.new root = Pathname.new(Dir.mktmpdir("executor-builtin-")) context = build_context(root) When "executing" - executor.execute(Dev::BuiltinCommand.new(body: body), args: ["--verbose"], context: context) + executor.execute(builtin, args: ["--verbose"], context: context) - Then "the body received args and context; no child process was involved" - body.calls == [[["--verbose"], context]] + Then "the builtin received args and context; no child process was involved" + builtin.calls == [[["--verbose"], context]] Cleanup FileUtils.rm_rf(root) @@ -91,9 +87,9 @@ def build_context(project_root) original_cwd = Dir.pwd root = Pathname.new(Dir.mktmpdir("executor-up-wait-")) execution_order = [] - body = ExecutorFakeBody.new(stamps: true) { execution_order << :builtin_install } + builtin = ExecutorFakeBuiltin.new(stamps: true) { execution_order << :builtin_install } command = Dev::OverriddenCommand.new( - builtin: Dev::BuiltinCommand.new(body: body), + builtin: builtin, project: Dev::ProjectCommand.new(run: "./bin/up.rb", desc: "Setup", container: false), ) executor = Dev::CommandExecutor.new @@ -120,7 +116,7 @@ def build_context(project_root) original_cwd = Dir.pwd root = Pathname.new(Dir.mktmpdir("executor-up-fail-")) command = Dev::OverriddenCommand.new( - builtin: Dev::BuiltinCommand.new(body: ExecutorFakeBody.new(stamps: true)), + builtin: ExecutorFakeBuiltin.new(stamps: true), project: Dev::ProjectCommand.new(run: "./bin/up.rb", desc: "Setup", container: false), ) executor = Dev::CommandExecutor.new @@ -153,9 +149,9 @@ def build_context(project_root) Given "a non-stamping builtin slot overridden by a project command" original_cwd = Dir.pwd root = Pathname.new(Dir.mktmpdir("executor-override-exec-")) - body = ExecutorFakeBody.new(stamps: false) + builtin = ExecutorFakeBuiltin.new(stamps: false) command = Dev::OverriddenCommand.new( - builtin: Dev::BuiltinCommand.new(body: body), + builtin: builtin, project: Dev::ProjectCommand.new(run: "./bin/lint.sh", desc: "Lint", container: false), ) executor = Dev::CommandExecutor.new @@ -165,7 +161,7 @@ def build_context(project_root) executor.execute(command, args: [], context: build_context(root)) Then "nothing sequences after execute, so the exec tail-call is safe and kept" - body.calls.size == 1 + builtin.calls.size == 1 1 * Kernel.exec(anything, "shadowenv", "exec", "--", "sh", "-c", includes("./bin/lint.sh")) 0 * Kernel.system(any_parameters) diff --git a/test/dev/command_repository_test.rb b/test/dev/command_repository_test.rb index d9fa24c..70bc69e 100644 --- a/test/dev/command_repository_test.rb +++ b/test/dev/command_repository_test.rb @@ -2,15 +2,11 @@ # frozen_string_literal: true require "test_helper" -require "dev/builtin_body" require "dev/command_repository" require "dev/command" -# A named no-op body for assembly assertions; the tests wrap it in -# BuiltinCommand the same way the composition root does. -class RepositoryFakeBody - include Dev::BuiltinBody - +# A named no-op builtin for assembly assertions. +class RepositoryFakeBuiltin < Dev::BuiltinCommand def initialize(desc: "a builtin", hidden: false) @desc = desc @hidden = hidden @@ -21,12 +17,12 @@ def initialize(desc: "a builtin", hidden: false) def hidden? = @hidden def call(args:, context:); end -end unless defined?(RepositoryFakeBody) +end unless defined?(RepositoryFakeBuiltin) transform!(RSpock::AST::Transformation) class Dev::CommandRepositoryTest < Minitest::Test def build_builtin(desc: "a builtin", hidden: false) - Dev::BuiltinCommand.new(body: RepositoryFakeBody.new(desc: desc, hidden: hidden)) + RepositoryFakeBuiltin.new(desc: desc, hidden: hidden) end test "fetch returns a builtin-only command as the builtin" do diff --git a/test/dev/command_service_test.rb b/test/dev/command_service_test.rb index a341627..73ae121 100644 --- a/test/dev/command_service_test.rb +++ b/test/dev/command_service_test.rb @@ -2,17 +2,13 @@ # frozen_string_literal: true require "test_helper" -require "dev/builtin_body" require "dev/command_service" require "dev/command" require "pathname" -# Body fake whose traits drive the service's guard/stamp decisions and -# whose call records the dispatch; tests wrap it in BuiltinCommand the -# same way the composition root does. -class ServiceFakeBody - include Dev::BuiltinBody - +# Builtin fake whose traits drive the service's guard/stamp decisions and +# whose call records the dispatch. +class ServiceFakeBuiltin < Dev::BuiltinCommand attr_reader :calls def initialize(staleness_exempt: false, stamps: false) @@ -30,7 +26,7 @@ def stamps? = @stamps def call(args:, context:) @calls << [args, context] end -end unless defined?(ServiceFakeBody) +end unless defined?(ServiceFakeBuiltin) transform!(RSpock::AST::Transformation) class Dev::CommandServiceTest < Minitest::Test @@ -44,10 +40,6 @@ def build_service(builtins:, dependency_service:, executor: Dev::CommandExecutor ) end - def wrap(body) - Dev::BuiltinCommand.new(body: body) - end - def fake_context Dev::ExecutionContext.new( ui: typed_mock(Dev::Cli::Ui), @@ -65,15 +57,15 @@ def fake_dependency_service test "execute fetches the command, dispatches it, and passes args and context through" do Given "a service over one builtin" - body = ServiceFakeBody.new(staleness_exempt: true) - service = build_service(builtins: { "deps" => wrap(body) }, dependency_service: fake_dependency_service) + builtin = ServiceFakeBuiltin.new(staleness_exempt: true) + service = build_service(builtins: { "deps" => builtin }, dependency_service: fake_dependency_service) context = fake_context When "executing the command" service.execute("deps", args: ["path", "xcode"], context: context) - Then "the builtin body ran once with the args and context" - body.calls == [[["path", "xcode"], context]] + Then "the builtin ran once with the args and context" + builtin.calls == [[["path", "xcode"], context]] end test "execute raises CommandNotFoundError (the repository's own) for an unknown name" do @@ -92,7 +84,7 @@ def fake_dependency_service dependency_service = typed_mock(Dev::DependencyService) dependency_service.expects(:guard!).once service = build_service( - builtins: { "build" => wrap(ServiceFakeBody.new(staleness_exempt: false)) }, + builtins: { "build" => ServiceFakeBuiltin.new(staleness_exempt: false) }, dependency_service: dependency_service, ) @@ -109,7 +101,7 @@ def fake_dependency_service dependency_service.expects(:guard!).never dependency_service.stubs(:lock!) service = build_service( - builtins: { "update-deps" => wrap(ServiceFakeBody.new(staleness_exempt: true)) }, + builtins: { "update-deps" => ServiceFakeBuiltin.new(staleness_exempt: true) }, dependency_service: dependency_service, ) @@ -126,7 +118,7 @@ def fake_dependency_service dependency_service.stubs(:guard!) dependency_service.expects(:lock!).once service = build_service( - builtins: { "install-deps" => wrap(ServiceFakeBody.new(staleness_exempt: true, stamps: true)) }, + builtins: { "install-deps" => ServiceFakeBuiltin.new(staleness_exempt: true, stamps: true) }, dependency_service: dependency_service, ) @@ -143,7 +135,7 @@ def fake_dependency_service dependency_service.stubs(:guard!) dependency_service.expects(:lock!).never service = build_service( - builtins: { "deps" => wrap(ServiceFakeBody.new) }, + builtins: { "deps" => ServiceFakeBuiltin.new }, dependency_service: dependency_service, ) @@ -162,7 +154,7 @@ def fake_dependency_service executor = typed_mock(Dev::CommandExecutor) executor.stubs(:execute).raises(Dev::CommandRunner::CommandFailedError.new(exit_status: 7)) service = build_service( - builtins: { "up" => wrap(ServiceFakeBody.new(staleness_exempt: true, stamps: true)) }, + builtins: { "up" => ServiceFakeBuiltin.new(staleness_exempt: true, stamps: true) }, dependency_service: dependency_service, executor: executor, ) @@ -176,7 +168,7 @@ def fake_dependency_service test "visible_commands serves the repository's usage view" do Given "a service over one visible builtin" - builtin = wrap(ServiceFakeBody.new) + builtin = ServiceFakeBuiltin.new service = build_service(builtins: { "deps" => builtin }, dependency_service: fake_dependency_service) Expect "the usage view flows through the service (the onion rule)" diff --git a/test/dev/command_test.rb b/test/dev/command_test.rb index 857d684..077abd9 100644 --- a/test/dev/command_test.rb +++ b/test/dev/command_test.rb @@ -2,14 +2,11 @@ # frozen_string_literal: true require "test_helper" -require "dev/builtin_body" require "dev/command" -# A minimal BuiltinBody for exercising the trait defaults, the -# BuiltinCommand delegation, and the OverriddenCommand composition. -class FakeBuiltinBody - include Dev::BuiltinBody - +# A minimal builtin for exercising the trait defaults, the hierarchy's +# open edge, and the OverriddenCommand composition. +class FakeBuiltin < Dev::BuiltinCommand def initialize(desc: "fake builtin", hidden: false, staleness_exempt: false, stamps: false, &body) @desc = desc @hidden = hidden @@ -29,7 +26,7 @@ def stamps? = @stamps def call(args:, context:) @body&.call(args, context) end -end unless defined?(FakeBuiltinBody) +end unless defined?(FakeBuiltin) transform!(RSpock::AST::Transformation) class CommandTest < Minitest::Test @@ -102,45 +99,49 @@ class CommandTest < Minitest::Test cmd.hidden? end - test "a builtin body without trait overrides gets the defaults" do - Given "a body defining only desc and call" - body = Class.new do - include Dev::BuiltinBody - + test "a builtin without trait overrides gets the Command defaults" do + Given "a builtin defining only desc and call" + builtin = Class.new(Dev::BuiltinCommand) do def desc = "minimal" def call(args:, context:); end end.new - Expect "the BuiltinBody trait defaults hold" - !body.hidden? - !body.staleness_exempt? - !body.stamps? + Expect "the Command trait defaults hold" + !builtin.hidden? + !builtin.staleness_exempt? + !builtin.stamps? end - test "a BuiltinCommand delegates call, desc, and every trait to its body" do - Given "a distinctive body wrapped in the sealed hierarchy's final leaf" - calls = [] - body = FakeBuiltinBody.new(desc: "wrapped", hidden: true, staleness_exempt: true, stamps: true) do |args, context| - calls << [args, context] - end - cmd = Dev::BuiltinCommand.new(body: body) - context = typed_mock(Dev::ExecutionContext) + test "subclassing BuiltinCommand is the hierarchy's declared open edge" do + Given "a builtin subclass" + builtin = FakeBuiltin.new(desc: "open edge") - When "calling through the wrapper" - cmd.call(args: ["--verbose"], context: context) + Expect "it enters the sealed hierarchy through the BuiltinCommand variant" + builtin.is_a?(Dev::BuiltinCommand) + builtin.is_a?(Dev::Command) + builtin.desc == "open edge" + end - Then "every reading came from the body" - cmd.desc == "wrapped" - cmd.hidden? - cmd.staleness_exempt? - cmd.stamps? - calls == [[["--verbose"], context]] + test "including Command directly raises: the seal admits only its three declared variants" do + When "including the sealed module outside its declaring file" + Class.new { include Dev::Command } + + Then "sorbet-runtime rejects the include" + raises RuntimeError + end + + test "ProjectCommand is final: subclassing raises, keeping descent closed" do + When "declaring a subclass of the data leaf" + Class.new(Dev::ProjectCommand) + + Then "sorbet-runtime rejects the open edge" + raises RuntimeError end - test "BuiltinCommand is final: subclassing raises, keeping the sealed hierarchy closed" do - When "declaring a subclass of the final leaf" - Class.new(Dev::BuiltinCommand) + test "OverriddenCommand is final: subclassing raises, keeping descent closed" do + When "declaring a subclass of the data leaf" + Class.new(Dev::OverriddenCommand) Then "sorbet-runtime rejects the open edge" raises RuntimeError @@ -148,7 +149,7 @@ def call(args:, context:); end test "an OverriddenCommand takes desc and hidden from the project override" do Given "a builtin slot overridden by a hidden project command" - builtin = Dev::BuiltinCommand.new(body: FakeBuiltinBody.new(desc: "builtin up")) + builtin = FakeBuiltin.new(desc: "builtin up") project = Dev::ProjectCommand.new(run: "./bin/up.sh", desc: "project up", hidden: true) cmd = Dev::OverriddenCommand.new(builtin: builtin, project: project) @@ -159,7 +160,7 @@ def call(args:, context:); end test "an OverriddenCommand takes guard and stamp traits from the builtin slot" do Given "a stamping, staleness-exempt builtin slot overridden by a project command" - builtin = Dev::BuiltinCommand.new(body: FakeBuiltinBody.new(staleness_exempt: true, stamps: true)) + builtin = FakeBuiltin.new(staleness_exempt: true, stamps: true) project = Dev::ProjectCommand.new(run: "./bin/up.sh", desc: "project up") cmd = Dev::OverriddenCommand.new(builtin: builtin, project: project) @@ -170,7 +171,7 @@ def call(args:, context:); end test "an OverriddenCommand exposes its typed halves" do Given "an overridden command" - builtin = Dev::BuiltinCommand.new(body: FakeBuiltinBody.new) + builtin = FakeBuiltin.new project = Dev::ProjectCommand.new(run: "./bin/up.sh") cmd = Dev::OverriddenCommand.new(builtin: builtin, project: project) From b8ed631d4d2445c3233d9290df4bbdf4064ecb67 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Date: Tue, 18 Aug 2026 13:39:04 -0400 Subject: [PATCH 3/3] Call super() in every initializer that derives from the Command hierarchy Ruby silently skips base initialization when a subclass overrides initialize without super. The calls are inert today (no ancestor defines initialize), but they make future base-class constructor state either run automatically (defaulted params) or fail loudly at first instantiation (required params) instead of constructing half-initialized objects. Applied uniformly: the two data leaves, all builtins, and the test fakes. Co-authored-by: Cursor --- src/dev/builtins/cache_command.rb | 1 + src/dev/builtins/cd_command.rb | 1 + src/dev/builtins/check_command.rb | 1 + src/dev/builtins/clone_command.rb | 1 + src/dev/builtins/cred_command.rb | 1 + src/dev/builtins/deps_command.rb | 1 + src/dev/builtins/install_deps_command.rb | 1 + src/dev/builtins/learnings_command.rb | 1 + src/dev/builtins/plan_command.rb | 1 + src/dev/builtins/runner_setup_command.rb | 1 + src/dev/builtins/up_command.rb | 1 + src/dev/command.rb | 2 ++ test/dev/command_executor_test.rb | 1 + test/dev/command_repository_test.rb | 1 + test/dev/command_service_test.rb | 1 + test/dev/command_test.rb | 1 + 16 files changed, 17 insertions(+) diff --git a/src/dev/builtins/cache_command.rb b/src/dev/builtins/cache_command.rb index f130fe7..8925dca 100644 --- a/src/dev/builtins/cache_command.rb +++ b/src/dev/builtins/cache_command.rb @@ -24,6 +24,7 @@ def initialize( cache_gc_factory: ->(lockfile) { Dev::Deps::CacheGc.new(lockfile:) }, flag_parser: Cli::FlagParser.new ) + super() @cache_gc_factory = T.let(cache_gc_factory, CacheGcFactory) @flag_parser = T.let(flag_parser, Cli::FlagParser) end diff --git a/src/dev/builtins/cd_command.rb b/src/dev/builtins/cd_command.rb index 65f2b2f..3325d9d 100644 --- a/src/dev/builtins/cd_command.rb +++ b/src/dev/builtins/cd_command.rb @@ -14,6 +14,7 @@ class CdCommand < BuiltinCommand sig { params(accessor: Dev::Cd::Accessor).void } def initialize(accessor: Dev::Cd::Accessor.new) + super() @accessor = T.let(accessor, Dev::Cd::Accessor) end diff --git a/src/dev/builtins/check_command.rb b/src/dev/builtins/check_command.rb index 010e2e6..7209424 100644 --- a/src/dev/builtins/check_command.rb +++ b/src/dev/builtins/check_command.rb @@ -13,6 +13,7 @@ class CheckCommand < BuiltinCommand sig { params(dependency_service: DependencyService).void } def initialize(dependency_service:) + super() @dependency_service = T.let(dependency_service, DependencyService) end diff --git a/src/dev/builtins/clone_command.rb b/src/dev/builtins/clone_command.rb index 368ad38..5e41dcf 100644 --- a/src/dev/builtins/clone_command.rb +++ b/src/dev/builtins/clone_command.rb @@ -14,6 +14,7 @@ class CloneCommand < BuiltinCommand sig { params(accessor: Dev::Clone::Accessor).void } def initialize(accessor: Dev::Clone::Accessor.new) + super() @accessor = T.let(accessor, Dev::Clone::Accessor) end diff --git a/src/dev/builtins/cred_command.rb b/src/dev/builtins/cred_command.rb index 3b33161..9449112 100644 --- a/src/dev/builtins/cred_command.rb +++ b/src/dev/builtins/cred_command.rb @@ -14,6 +14,7 @@ class CredCommand < BuiltinCommand sig { params(accessor: Dev::CredentialAccessor).void } def initialize(accessor: Dev::CredentialAccessor.new) + super() @accessor = T.let(accessor, Dev::CredentialAccessor) end diff --git a/src/dev/builtins/deps_command.rb b/src/dev/builtins/deps_command.rb index ee4ecfd..766af33 100644 --- a/src/dev/builtins/deps_command.rb +++ b/src/dev/builtins/deps_command.rb @@ -29,6 +29,7 @@ def initialize( ) } ) + super() @accessor_factory = T.let(accessor_factory, AccessorFactory) end diff --git a/src/dev/builtins/install_deps_command.rb b/src/dev/builtins/install_deps_command.rb index 7aa6fe0..f101969 100644 --- a/src/dev/builtins/install_deps_command.rb +++ b/src/dev/builtins/install_deps_command.rb @@ -50,6 +50,7 @@ def initialize( gem_skill_linker_factory: ->(project_root) { Dev::Deps::GemSkillLinker.new(project_root:) }, synchronizer: Dev::Learnings::Synchronizer.for ) + super() @installer_factory = T.let(installer_factory, InstallerFactory) @gem_skill_linker_factory = T.let(gem_skill_linker_factory, GemSkillLinkerFactory) @synchronizer = T.let(synchronizer, T.untyped) diff --git a/src/dev/builtins/learnings_command.rb b/src/dev/builtins/learnings_command.rb index 44c08b4..ce8456a 100644 --- a/src/dev/builtins/learnings_command.rb +++ b/src/dev/builtins/learnings_command.rb @@ -20,6 +20,7 @@ class LearningsCommand < BuiltinCommand sig { params(accessor_factory: AccessorFactory).void } def initialize(accessor_factory: ->(project_root) { Dev::Learnings::Accessor.new(project_root:) }) + super() @accessor_factory = T.let(accessor_factory, AccessorFactory) end diff --git a/src/dev/builtins/plan_command.rb b/src/dev/builtins/plan_command.rb index 4109640..1327f73 100644 --- a/src/dev/builtins/plan_command.rb +++ b/src/dev/builtins/plan_command.rb @@ -20,6 +20,7 @@ class PlanCommand < BuiltinCommand sig { params(accessor_factory: AccessorFactory).void } def initialize(accessor_factory: ->(project_root) { Dev::Plan::Accessor.new(project_root:) }) + super() @accessor_factory = T.let(accessor_factory, AccessorFactory) end diff --git a/src/dev/builtins/runner_setup_command.rb b/src/dev/builtins/runner_setup_command.rb index 925be32..2d4df5d 100644 --- a/src/dev/builtins/runner_setup_command.rb +++ b/src/dev/builtins/runner_setup_command.rb @@ -36,6 +36,7 @@ def initialize( runner_setup_factory: ->(config, repo, org) { Dev::RunnerSetup.new(config:, repo:, org:) }, flag_parser: Cli::FlagParser.new ) + super() @runner_setup_factory = T.let(runner_setup_factory, RunnerSetupFactory) @flag_parser = T.let(flag_parser, Cli::FlagParser) end diff --git a/src/dev/builtins/up_command.rb b/src/dev/builtins/up_command.rb index 0271382..71b244b 100644 --- a/src/dev/builtins/up_command.rb +++ b/src/dev/builtins/up_command.rb @@ -23,6 +23,7 @@ class UpCommand < BuiltinCommand ).void end def initialize(install_deps_command:, hook_installer: Dev::Cd::HookInstaller.new) + super() @install_deps_command = T.let(install_deps_command, InstallDepsCommand) @hook_installer = T.let(hook_installer, Dev::Cd::HookInstaller) end diff --git a/src/dev/command.rb b/src/dev/command.rb index 6d9a2b7..9e5ae59 100644 --- a/src/dev/command.rb +++ b/src/dev/command.rb @@ -106,6 +106,7 @@ class ProjectCommand params(run: String, desc: String, repl: T::Boolean, container: T::Boolean, hidden: T::Boolean).void end def initialize(run:, desc: "(no description)", repl: false, container: true, hidden: false) + super() @run = T.let(run, String) @desc = T.let(desc, String) @repl = T.let(repl, T::Boolean) @@ -153,6 +154,7 @@ class OverriddenCommand sig(:final) { params(builtin: BuiltinCommand, project: ProjectCommand).void } def initialize(builtin:, project:) + super() @builtin = T.let(builtin, BuiltinCommand) @project = T.let(project, ProjectCommand) end diff --git a/test/dev/command_executor_test.rb b/test/dev/command_executor_test.rb index 63fe26d..b3b0848 100644 --- a/test/dev/command_executor_test.rb +++ b/test/dev/command_executor_test.rb @@ -15,6 +15,7 @@ class ExecutorFakeBuiltin < Dev::BuiltinCommand attr_reader :calls def initialize(stamps: false, &body) + super() @stamps = stamps @calls = [] @body = body diff --git a/test/dev/command_repository_test.rb b/test/dev/command_repository_test.rb index 70bc69e..084e9ac 100644 --- a/test/dev/command_repository_test.rb +++ b/test/dev/command_repository_test.rb @@ -8,6 +8,7 @@ # A named no-op builtin for assembly assertions. class RepositoryFakeBuiltin < Dev::BuiltinCommand def initialize(desc: "a builtin", hidden: false) + super() @desc = desc @hidden = hidden end diff --git a/test/dev/command_service_test.rb b/test/dev/command_service_test.rb index 73ae121..8e178bb 100644 --- a/test/dev/command_service_test.rb +++ b/test/dev/command_service_test.rb @@ -12,6 +12,7 @@ class ServiceFakeBuiltin < Dev::BuiltinCommand attr_reader :calls def initialize(staleness_exempt: false, stamps: false) + super() @staleness_exempt = staleness_exempt @stamps = stamps @calls = [] diff --git a/test/dev/command_test.rb b/test/dev/command_test.rb index 077abd9..2ad5ef7 100644 --- a/test/dev/command_test.rb +++ b/test/dev/command_test.rb @@ -8,6 +8,7 @@ # open edge, and the OverriddenCommand composition. class FakeBuiltin < Dev::BuiltinCommand def initialize(desc: "fake builtin", hidden: false, staleness_exempt: false, stamps: false, &body) + super() @desc = desc @hidden = hidden @staleness_exempt = staleness_exempt