PR D: Split CommandExecutor into a dispatching composite with injectable BuiltinExecutor / ProjectExecutor / OverriddenExecutor strategies (exec_into vs run_waiting) - #122
Merged
JPDuchesne merged 8 commits intoAug 19, 2026
Conversation
…ite with injectable BuiltinExecutor / ProjectExecutor / OverriddenExecutor strategies (exec_into vs run_waiting) Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
JPDuchesne
reviewed
Aug 18, 2026
JPDuchesne
left a comment
JPDuchesne
left a comment
Contributor
There was a problem hiding this comment.
/build let's fix the fake classes, put them within the test class
Comment on lines
+9
to
+24
| # Builtin fake whose body records its invocations, so the test can assert | ||
| # the in-process delegation passed args and context through untouched. | ||
| class BuiltinExecutorFakeBuiltin < Dev::BuiltinCommand | ||
| attr_reader :calls | ||
|
|
||
| def initialize | ||
| @calls = [] | ||
| super() | ||
| end | ||
|
|
||
| def desc = "a builtin" | ||
|
|
||
| def call(args:, context:) | ||
| @calls << [args, context] | ||
| end | ||
| end unless defined?(BuiltinExecutorFakeBuiltin) |
Contributor
There was a problem hiding this comment.
Let's move the fake to be inside Dev::BuiltinExecutorTest
Contributor
Author
There was a problem hiding this comment.
Moved the fake into Dev::BuiltinExecutorTest as a nested FakeBuiltin class, dropping the unless defined? guard and the BuiltinExecutor name prefix — both were only needed while the constant lived at file top level.
Addressed in 2e05625.
Comment on lines
+11
to
+22
| class OverriddenExecutorFakeBuiltin < Dev::BuiltinCommand | ||
| def initialize(stamps:) | ||
| @stamps = stamps | ||
| super() | ||
| end | ||
|
|
||
| def desc = "a builtin" | ||
|
|
||
| def stamps? = @stamps | ||
|
|
||
| def call(args:, context:); end | ||
| end unless defined?(OverriddenExecutorFakeBuiltin) |
Contributor
Author
There was a problem hiding this comment.
Same fix in Dev::OverriddenExecutorTest: the stamping-trait fake is now a nested FakeBuiltin, guard and prefix removed.
Addressed in 2e05625.
…bin/test.rb runner Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
/build let's fix the fake classes, put them within the test class (Re-firing the review-submitted command from #122 (review) — GitHub creates no workflow run for review-surface events while a PR is merge-conflicted, and this PR was conflicted with main between 17:47 and 18:33 UTC. The conflict is now resolved.)
|
Merged
1 task
…class Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com>
Every command declares which usage section it lists under (Lifecycle, Development flow, or project commands). Abstract rather than defaulted so no builtin lands in a section silently; OverriddenCommand takes the slot's category, consistent with its guard/stamp trait delegation. Co-authored-by: Cursor <cursoragent@cursor.com>
dev help renders the usage listing as a first-class command. It lists the catalog that contains it, so the listing arrives as a commands provider resolved at call time; the composition root closes that self-reference. Staleness-exempt: help is how the remediation commands get discovered. Co-authored-by: Cursor <cursoragent@cursor.com>
Bare dev, --help, and -h now route to the help builtin like any command (dev help works too, and a project help: override composes via the usual slot mechanics). UsagePrinter renders the listing as Category sections — project commands, Lifecycle, Development flow — alphabetized so the output is deterministic regardless of registration order. With help inside the command path, the --help lazy-toolchain deferral is dropped: the toolchain pass over dependencies.rb runs once per invocation, unconditionally, and the composition root collapses to a single pass in Runner#run (kept there, not the constructor, so dependencies.rb errors still flow through the exit_for mapping). ui/out are per-process collaborators and move to Runner's constructor; the help builtin's self-referential listing is closed at the root with a call-time provider. Co-authored-by: Cursor <cursoragent@cursor.com>
CommandRunner's wait: constructor flag was a per-call mode hiding in construction; it becomes two public messages, exec_into and run_waiting, mirroring ProjectExecutor's seam (the flag survives only on the private run_child boundary). With the toolchain pass eager, the runner's collaborators are all known at the composition root, so it is built once from the run's ExecutionContext and injected into ProjectExecutor — the project-side executor messages stop carrying a context they never used. Co-authored-by: Cursor <cursoragent@cursor.com>
with_toolchain now runs once per invocation (help included); the lazy --help carve-out is gone, so the comment states the real property: dependencies.rb is a declaration file and must stay cheap. Co-authored-by: Cursor <cursoragent@cursor.com>
JPDuchesne
approved these changes
Aug 19, 2026
JPDuchesne
added a commit
that referenced
this pull request
Aug 19, 2026
916fe89 Merge pull request #128 from d3mlabs/jpd/skill-installer-ephemeral-guard b9ea143 Move the ephemeral-source guard to SkillInstaller, the seam all links share 36803f7 Merge pull request #127 from d3mlabs/jpd/capture-learning-root-cause-gate d6e081e Name the wide-angle goal, not one command: an exact git-log depth invites checkbox compliance f876dbe capture-learning: gate workaround learnings on root cause, add wide angle 7249198 Merge pull request #123 from d3mlabs/ai/119-pr-b-typed-child-process-failure-taxonom e71063a Merge pull request #126 from d3mlabs/jpd/hermetic-scrub-guard f911e38 Make the scrub-list guard hermetic: construct the bundler launch it measures 6c398b8 ai-flow /build: let's resolve conflicts cb68d60 Merge pull request #122 from d3mlabs/ai/118-pr-d-split-commandexecutor-into-a-dispat 4477c6b Update the manifest-loader contract note for the eager toolchain pass 3ad03c3 Constructor-inject CommandRunner; two messages replace the wait flag 2ac941a Route help through the command path; group and eager-load usage a78ba14 Add the help builtin c7ae57a Add Category trait to the Command hierarchy 2e05625 ai-flow /build: let's fix the fake classes, put them within the test class a29b5e6 Merge main: sealed-module Command hierarchy, super() convention, and bin/test.rb runner 5d9c57b Merge pull request #121 from d3mlabs/ai/117-pr-a-close-the-sealed-command-hierarchy b8ed631 Call super() in every initializer that derives from the Command hierarchy 5bcc76f Rework the seal: Command becomes a sealed module, BuiltinCommand the abstract open-edge class 5c68c85 Merge pull request #120 from d3mlabs/ai/116-pr-c-bin-test-rb-tee-suite-output-to-a-s c62efc8 ai-flow /build: PR B: Typed child-process failure taxonomy in CommandRunner (CommandFailedError / CommandKilledError / CommandSpawnError) mapped to exit codes in Runner#exit_for f09f845 ai-flow /build: PR D: Split CommandExecutor into a dispatching composite with injectable BuiltinExecutor / ProjectExecutor / OverriddenExecutor strategies (exec_into vs run_waiting) 0775515 ai-flow /build: PR A: Close the sealed Command hierarchy honestly — BuiltinBody interface, final BuiltinCommand holding a body, delete the sorbet-runtime ivar pokes, un-private CommandRepository 5a5fb41 ai-flow /build: PR C: bin/test.rb — tee suite output to a stable log artifact and pass file args through to rake TEST 4c89408 Merge pull request #115 from d3mlabs/ai/37-layer-the-dev-runner-application-service 04d461c Add the simplecov-cobertura gem RBI 81677f6 Upload cobertura to codecov instead of SimpleCov JSON 0a741f0 Cover the default factories, image credential providers, and nocov the sealed absurd arm fe7c94e ai-flow /build: Layer the dev Runner (application service + boundary coercion) d782b1a Merge pull request #107 from d3mlabs/ai/101-dev-clone-host-global-builtin-cloning-vi f305ac9 ai-flow /build: codecov coverage missing fac96ee ai-flow /build: dev clone: host-global builtin cloning via gh auth to the canonical $DEV_CD_ROOT path d16b757 Merge pull request #100 from d3mlabs/jpd/99-pin-homebrew-installer 2ad614e Pin the Homebrew installer to a commit SHA (dev#99) 53e3616 Merge pull request #90 from d3mlabs/ai/89-gemskilllinker-links-minted-under-a-sand 95ee372 Merge pull request #97 from d3mlabs/ai/learn-promote-rbenv-libruby-rpath-hijack f7edc33 chore: nudge origin-firing after ai-flow#57 (removal diffs skip green) 646f189 Merge pull request #98 from d3mlabs/jpd/proposal-checks-edited 50e913a proposal-checks: re-verify on PR body edits (ai-flow#54) 59a3146 ai-flow /learn: drop rbenv-libruby-rpath-hijack (promoted to the org tier) f119987 Merge pull request #96 from d3mlabs/jpd/ai-flow-knowledge-repo b0e7131 ai-flow config: opt dev into org-tier learning promotion d17b2ff Merge pull request #95 from d3mlabs/jpd/94-self-defending-entrypoint 29b2e16 Test readability: one aliased scrub list, one property per test f7197ac Drift guard: the unset list must cover what the running bundler exports 049bbc8 Probe the shim scrub with a stub ruby instead of a full dev command run 88fa953 bin/dev: scrub foreign bundler activation before Ruby boots 9cf868a Merge pull request #92 from d3mlabs/ai/60-plan-pull-mangles-files-with-an-empty-fr 6783469 Merge pull request #93 from d3mlabs/ai/learn-issue-60 991d46d ai-flow /build: capture learnings from the build pass fe64507 ai-flow /build: Plan pull mangles files with an empty frontmatter block above the real one (double frontmatter) d8db57d ai-flow /build: GemSkillLinker: links minted under a sandboxed session point into ephemeral sandbox cache paths b4526ea Merge pull request #88 from d3mlabs/jpd/ast-transform-3.1.1 de9beaf Bump ast_transform to 3.1.1 and drop the heredoc-emission workaround
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #118.
Requested by @JPDuchesne.
Closes #118