Enforce Sorbet typed sigil by d3mlabs-ai-flow[bot] · Pull Request #140 · d3mlabs/dev

d3mlabs-ai-flow · 2026-08-28T22:50:12Z

Implements #139.

Requested by @JPDuchesne.

Closes #139

Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com>

codecov · 2026-08-28T22:51:28Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

JPDuchesne

Some early feedback

JPDuchesne · 2026-08-28T23:42:02Z


      # Ensure the wrapper function + completer are in the user's shell RC.
      #
      # @return [Symbol, false] :added, :already_present, or false (unsupported shell)


This typing is actually pretty bad, we should likely model this as what the name of this method actually means which simplifies the return type:

ensure the hook is installed, idempotent. an idempotent method caller has no care for whether it was already done or not, thus get rid of :already_present, which leaves :added as a success case or false for unsupported shell. ensuring the hook is installed it the success case, so not raising is the success case. raising for UnsupportedShellError or something (likely caught at a higher level)

Let's create a GH issue to refactor

Created issue #144 for the refactor: idempotent ensure semantics, drop :already_present, success = not raising, typed UnsupportedShellError caught at a higher level — scoped to ShellRcHook, HookInstaller, and the three callers that branch on the tri-state.

Addressed in 5a3024a.

The refactor is tracked as issue #144 (idempotent ensure semantics, raise UnsupportedShellError, drop :already_present) — no code change in this PR.

Addressed in d195e32.

JPDuchesne · 2026-08-28T23:58:58Z

@@ -1,3 +1,4 @@
+# typed: false


why typed: false

dsl.rb's method_missing dispatch into a required-keyword method needs T.unsafe to typecheck, and T.unsafe is sorbet-runtime — forbidden in the pre-bundle chain — so it stays typed: false; documented exclude.

Addressed in 5a3024a.

dsl.rb is typed: false because its method_missing dispatch into required-keyword methods needs T.unsafe, which the pre-bundle no-sorbet-runtime constraint forbids; documented in the exclude comments.

Addressed in d195e32.

JPDuchesne · 2026-08-28T23:59:03Z

@@ -1,3 +1,4 @@
+# typed: false


why typed: false

fetcher.rb consumes a consumer-repo Lockfile API (parse, runtime_ref_map) that doesn't resolve against this repo's Lockfile class, so type checking it produces false errors; typed: false with a documented exclude, also pre-bundle.

Addressed in 5a3024a.

fetcher.rb is typed: false because it consumes a consumer-repo Lockfile API (parse, runtime_ref_map) that doesn't resolve against this repo's Lockfile; documented in the exclude comments.

Addressed in d195e32.

JPDuchesne · 2026-08-29T00:00:34Z

+        params(
+          targets: T.nilable(T::Array[T::Hash[String, T.untyped]]),
+          target_name: String,
+        ).returns(T.nilable(T::Hash[String, T.untyped]))
+      end
      def find_target(targets, target_name)
        return nil if targets.nil? || targets.empty?


why is targets even nilable if the method returns nil when targets is nil ? we should remove the representation of a nilable targets here

Removed the nilable representation: find_target now takes T::Array[Hash] and the one caller owns the API-boundary coercion (version_data["targets"] || []); the return stays nilable only for the genuinely-empty case, and a test covers the transport method the change touched.

Addressed in 5a3024a.

Already fixed on the branch: find_target now takes a non-nilable T::Array and the caller passes version_data["targets"] || [] — the nilable representation is gone.

Addressed in d195e32.

JPDuchesne · 2026-08-29T00:06:33Z

+      sig { params(body: T::Hash[Symbol, T.untyped]).returns(Net::HTTPResponse) }
      def post_graphql(body)
-        http = Net::HTTP.new(GRAPHQL_ENDPOINT.host, GRAPHQL_ENDPOINT.port)
+        http = Net::HTTP.new(T.must(GRAPHQL_ENDPOINT.host), GRAPHQL_ENDPOINT.port)


/ask Fine I guess, but is there a reason we do it like this rather than capturing the host as a constant too ? or better yet is there a better way to do this from the URI directly? I would imagine there's some interoperability between these modules?

✅ /ask

Why the T.must: Sorbet's stdlib payload types URI(...) as URI::Generic, whose #host is T.nilable(String) — host-less URIs (relative refs, mailto:) are representable, and Sorbet can't see that this particular literal always parses with a host. Verified against this repo's Sorbet: T.reveal_type(URI("https://x.dev/q").host) → T.nilable(String). So the narrow has to happen somewhere; the only question is where.

Capturing the host as a constant — yes, strictly better than the per-call T.must: GRAPHQL_HOST = T.let(T.must(GRAPHQL_ENDPOINT.host), String) next to the endpoint constant does the narrow once at load (fail-fast if the URL is ever edited into something host-less) and keeps call sites clean.

URI-direct interop — exists, and it's the nicer fix: Net::HTTP.post(GRAPHQL_ENDPOINT, JSON.generate(body), "Content-Type" => "application/json") (Ruby ≥ 2.4) takes the URI whole, derives use_ssl from the scheme, and deletes the entire new/use_ssl/Net::HTTP::Post dance — the response-code check stays. One caveat, also verified: the payload leaves Net::HTTP.post's return untyped, so under strict the result wants a T.let(..., Net::HTTPResponse) — one escape hatch traded for another, but ~5 fewer lines and no per-part URI plucking. Either variant is an improvement over the inline T.must; the Net::HTTP.post form is a small follow-up candidate (this method is untouched by the deps redesign in #142, so it can land on either branch).

let's record this in a GH issue

JPDuchesne · 2026-08-29T00:11:34Z

      ) do
+        extend T::Sig
+
+        # Sorbet's Data.define rewriter can't attach sigs to the generated


/ask there has to be a cleaner way to support Data, no? (i.e. tapioca or rbis?) otherwise I guess we use a class rather than Data.define ?

✅ /ask

No clean way exists today — the wall is sorbet/sorbet#7272 (cited in the code comment): Sorbet's Data.define rewriter synthesizes the member readers without sigs, strict demands a sig on every method, and re-opening the block to define sigged readers with the same names would replace the C-level originals. The escape routes you'd hope for don't apply:

tapioca: generates RBIs for gems (tapioca gem) and framework metaprogramming via DSL compilers (tapioca dsl) — there's no compiler for app-local Data.define, and tapioca doesn't process your own lib/ at all.

hand-written RBI shims: sorbet/rbi/shims is for methods Sorbet can't see. These readers ARE seen (the rewriter defines them), so a shim is a conflicting duplicate definition rather than a sig attachment — and it drifts silently when members change.

Hence the to_h-delegating explicit readers here: ugly, but honest about the constraint.

So yes — the pragmatic conclusion is the one you guessed: a class rather than Data.define. That's already the precedent on the stacked branch: #142's new domain value types (PackageId, PackageVersion, Artifact, …) are plain classes with attr_reader + sigs, because Data.define additionally hits error 4010 there once initialize carries keyword defaults.

For Entry specifically, T::Struct is the zero-boilerplate fit: registry.rb is not in the pre-bundle chain, so sorbet-runtime is available; const :symbol, Symbol (etc.) gives typed readers and keyword construction with defaults for free; and Entry uses none of what Data buys (value equality, #with) — it's config rows, not a value type. That conversion would delete this whole reader block. Best done as a follow-up on #142's branch, since this file is heavily reshaped there (scheme/locker slots) and doing it here would just manufacture a conflict.

ok, let's solve this with #142

JPDuchesne · 2026-09-04T16:28:14Z

/build let's swipe the current PR review feedback + fix codecov

✅ /build — committed 5a3024a.

Codecov fixed via the org's known Sorbet-coverage setup: simplecov-sorbet added (with simplecov ~> 1.0 / simplecov-cobertura ~> 4.0 bumps, regenerated gem RBIs, migrated :nocov:/add_filter to the 1.x forms) plus disable_search: true on the upload so the poisoned JSON never reaches Codecov — that erased 21 of the 27 patch misses (sig-block interiors), and the remaining 6 pre-existing uncovered boundary lines the sigil edits dragged into the patch got real tests (BuildWatcher run_once with real processes, SteamCmd ensure!/run, the capture/system executor wrappers, brew local-tap registration, credentials empty-paste), landing local patch coverage at 100% (958 tests green, rubocop clean, srb tc clean, tapioca verified). Review threads: one GitHub issue (#144), sorbet-runtime centralized into src/dev.rb, Cd::Repo now strict, real collaborator types in cd/clone accessors, splat escapes removed or documented (error 7019 is the root constraint), and the eight sigil questions answered by the pre-bundle stdlib-only constraint already documented in .rubocop.yml. Two learnings captured: [toolchain/sorbet-splat-unsafe] and [architecture/pre-bundle-stdlib-chain]; also added --ignore=/.ai-flow to sorbet/config so nested ai-flow checkouts stop breaking local srb tc.
FIX: lib/dev/deps/tap.rb + brew_integration.rb — the documented local-tap form file://./x parses to URI path "/x" (host "."), so resolve_file_url's project-relative "./" branch never fires for it and the tap resolves to a root-level absolute path; normalize the host-"." case (or the Tap constructor) to project-relative.

🧠 drafted 2 learnings in a learning proposal PR: #145

pre-bundle-stdlib-chain

sorbet-splat-unsafe

⚠️ The agent proposed changes under .github/workflows/ — the ai-flow App has no workflows permission (by design; see d3mlabs/ai-flow docs/attribution.md), so they were left out of the commit. Paste this in your clone to apply them onto the PR under your own credentials:
apply the workflow patch (one paste)
gh pr checkout https://github.com/d3mlabs/dev/pull/140 && git apply --index <<'PATCH' && git commit -m "Apply ai-flow's proposed workflow changes" && git push
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 9fe602b..b3baff6 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -70,4 +70,8 @@ jobs:
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: coverage/coverage.xml
+          # Upload only the cobertura report: auto-search also picks up
+          # SimpleCov's native coverage.json, whose "ignored" line entries
+          # (simplecov-sorbet's skipped ranges) error Codecov's processing.
+          disable_search: true
           fail_ci_if_error: false
PATCH

⚙️ workflow run · model: claude-fable-5-high

JPDuchesne · 2026-09-04T17:19:47Z

Two of this review's three /ask comments were silently dropped by the dispatcher — root cause (per-PR dispatch concurrency vs GitHub's batch-review event burst) and fix design filed as d3mlabs/ai-flow#73. Both lost questions were answered manually in their threads.

Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com>

JPDuchesne · 2026-09-04T17:54:46Z

+# Required here (not just in src/dev.rb) because consumer dependencies.rb
+# files load this hook standalone via bin/install-build-deps.rb, where the
+# Dev module root never loads.
+require "sorbet-runtime"


is there a more central place we should put this require?

The central place now exists — src/dev.rb carries the one sorbet-runtime require for the whole Dev tree — but the hooks are the deliberate exception: consumer dependencies.rb files load them standalone via bin/install-build-deps.rb, where the Dev root never loads, so each hook keeps its own require with a comment saying exactly that.

Addressed in d195e32.

JPDuchesne · 2026-09-04T17:55:22Z

+# Required here (not just in src/dev.rb) because consumer dependencies.rb
+# files load this hook standalone via bin/install-build-deps.rb, where the
+# Dev module root never loads.
+require "sorbet-runtime"


same, is there a more central place we should put this require?

Same as thread 13 — wwise_download.rb is loaded standalone by consumer repos, so the self-require stays, documented in place.

Addressed in d195e32.

JPDuchesne · 2026-09-04T18:14:22Z

+      sig { params(body: T::Hash[Symbol, T.untyped]).returns(Net::HTTPResponse) }
      def post_graphql(body)
-        http = Net::HTTP.new(GRAPHQL_ENDPOINT.host, GRAPHQL_ENDPOINT.port)
+        http = Net::HTTP.new(T.must(GRAPHQL_ENDPOINT.host), GRAPHQL_ENDPOINT.port)


let's record this in a GH issue

JPDuchesne · 2026-09-05T12:47:52Z

@@ -1,3 +1,4 @@
+# typed: true


typed: true? why not strict?

Pre-bundle chain: lockfile.rb is loaded by bin/setup.rb before the bundle exists, so typed: true is its ceiling (documented holdout).

Addressed in d195e32.

JPDuchesne · 2026-09-05T12:50:58Z

      ) do
+        extend T::Sig
+
+        # Sorbet's Data.define rewriter can't attach sigs to the generated


ok, let's solve this with #142

JPDuchesne · 2026-09-05T12:58:07Z


 require "fileutils"
+require "pathname"
+require "sorbet-runtime"


Same thing here with regards to namespacing and sorbet-runtime require

Done: Dev::ShadowenvPython, require dropped.

Addressed in d195e32.

JPDuchesne · 2026-09-05T12:58:12Z


 require "fileutils"
+require "pathname"
+require "sorbet-runtime"


Same thing here with regards to namespacing and sorbet-runtime require

Done: Dev::ShadowenvRuby, require dropped.

Addressed in d195e32.

JPDuchesne · 2026-09-05T12:58:16Z


 require "fileutils"
+require "pathname"
+require "sorbet-runtime"


Same thing here with regards to namespacing and sorbet-runtime require

Done: Dev::ShadowenvUnreal, require dropped.

Addressed in d195e32.

JPDuchesne · 2026-09-05T12:58:20Z


 require "fileutils"
+require "pathname"
+require "sorbet-runtime"


Same thing here with regards to namespacing and sorbet-runtime require

Done: Dev::ShadowenvXcode, require dropped.

Addressed in d195e32.

JPDuchesne · 2026-09-05T13:01:39Z

+    $stdin.stubs(:tty?).returns(true)
+    $stdin.stubs(:noecho).returns("")
+    $stdout.stubs(:puts)
+    $stdout.stubs(:print)


we should be able to use RSpock interaction-based stubbing here

Converted to RSpock interaction stubbing in test/dev/credentials_test.rb: _ * $stdin.tty? >> true, _ * $stdin.noecho >> "", and the $stdout silencers are now declared in the Then block (installed before When runs), replacing the mocha .stubs calls.

Addressed in d195e32.

JPDuchesne · 2026-09-05T13:06:30Z

/build let's swipe the review feedback

✅ /build — committed d195e32.

Main change this pass: the eight in-process top-level helpers (build_container, build_watcher, six shadowenv_*) moved under the Dev namespace at lib/dev/ and dropped their sorbet-runtime self-requires — src/dev.rb is now the single require, with only the consumer-loaded deps hooks and bin/test.rb's rake_test_argv as documented standalone exceptions; their tests moved to test/dev/ and all references were qualified. Also converted the credentials empty-paste test to RSpock interaction stubbing. Threads 2, 3, 5, and 12 were already fixed by the previous iteration's commit; the sigil threads (4, 6–11, 15–17, 20) are all the documented pre-bundle stdlib-only constraint. Verified: srb tc clean, tapioca gem --verify in sync, rubocop 245 files no offenses, full suite 958 tests 0 failures. Learning drafts (pre-bundle-stdlib-chain, sorbet-splat-unsafe) refined to reflect the final namespacing; index line updated. Note: the dev CLI on this runner is broken by leaked gem env, worked around by running the repo's bin scripts with a scrubbed environment. FIX: bin/dev shim — scrub inherited GEM_HOME/GEM_PATH/RUBYLIB before resolving Ruby, so a CI harness's leaked gem environment (here: Ruby 3.3.10 paths plus dev-core's RUBYLIB) can't break dev's bootstrap.

🧠 refined 2 learnings in a learning proposal PR: #145

pre-bundle-stdlib-chain

sorbet-splat-unsafe

⚙️ workflow run · model: claude-fable-5-high

Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com>

…aded The codecov-action auto-search was picking up coverage/coverage.json (SimpleCov native JSON with skipped lines from simplecov-sorbet) alongside the pinned cobertura XML, causing Codecov to process every commit to an empty error report and never post statuses. Co-authored-by: Cursor <cursoragent@cursor.com>

Covers setup!, the Homebrew binary-fallback version detection, brew_prefix_for / path_with_brew_bin / homebrew_prefix, the fresh-install and not-found paths of ensure_ruby_installed!, ensure_ruby_build_deps!, the single-part gem_api_version fallback, and the shell-hook write-failure rescue - all via fake executables and unwritable real directories. Co-authored-by: Cursor <cursoragent@cursor.com>

Covers setup!, venv creation/reuse/failure in ensure_venv!, the ensurepip bootstrap and its failure in ensure_pip!, every outcome of ensure_homebrew_python! (installed, install, install-failure, prefix unresolvable, python3 fallback, no interpreter), and brew_prefix_for - all via fake brew/python executables on a restricted PATH. Co-authored-by: Cursor <cursoragent@cursor.com>

Covers wait_or_kill killing a silent idle build, container_cpu parsing the docker stats percentage, and the no-docker-executable idle fallback, all via fake docker executables on PATH. Co-authored-by: Cursor <cursoragent@cursor.com>

Covers prewarm_container_name, the system-delegating wrappers (remove_image, container_exists?, start_container, remove_container, local_image?, pull, push!, registry_has?), the docker ps name parsing in service_containers, container_running?, and run_watched's watcher delegation. Co-authored-by: Cursor <cursoragent@cursor.com>

Covers detect_llvm_prefix finding an installed formula's keg, the no-usable-keg nil path, and the brew-absent nil path, all via fake brew executables on PATH. Co-authored-by: Cursor <cursoragent@cursor.com>

A fake brew reports lua@5.1 installed but fails the luarocks install, so ensure_homebrew_lua! raises BrewInstallError naming luarocks. Co-authored-by: Cursor <cursoragent@cursor.com>

platform_subdir branched on the RUBY_PLATFORM constant, so any single CI platform could only ever cover its own branch. It now takes the platform as a defaulted parameter, and a Where-table exercises the darwin, linux, and windows mappings. Co-authored-by: Cursor <cursoragent@cursor.com>

Require blocks keep the redesign's package/package_id/package_version requires and adopt main's convention of loading sorbet-runtime once in src/dev.rb (applied to every new deps file, matching the sweep in 5a3024a). Ficsit's find keeps the redesign body with main's single target binding ported. Dead main-side requires (tempfile, digest/open3/ tmpdir, repository.rb's dependency pair) dropped with the code that used them. Co-authored-by: Cursor <cursoragent@cursor.com>

ai-flow /build: Enforce Sorbet typed sigil

a1093cf

Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com>

d3mlabs-ai-flow Bot assigned JPDuchesne Aug 28, 2026

d3mlabs-ai-flow Bot mentioned this pull request Aug 28, 2026

Enforce Sorbet typed sigil #139

Closed

JPDuchesne mentioned this pull request Sep 2, 2026

Deps redesign: package universe repositories, Resolver-owned selection #142

Open

JPDuchesne reviewed Sep 4, 2026

View reviewed changes

d3mlabs-ai-flow Bot mentioned this pull request Sep 4, 2026

Refactor shell-hook installation to idempotent ensure semantics (raise on unsupported shell) #144

Open

JPDuchesne mentioned this pull request Sep 4, 2026

Batch review submissions lose slash commands: per-PR dispatch concurrency evicts queued runs d3mlabs/ai-flow#73

Closed

ai-flow /build: let's swipe the current PR review feedback + fix codecov

5a3024a

Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com>

d3mlabs-ai-flow Bot mentioned this pull request Sep 4, 2026

ai-flow /learn: learnings from d3mlabs/dev#140 #145

Open

JPDuchesne mentioned this pull request Sep 4, 2026

Dispatch a submitted review as one unit — batch reviews lost commands d3mlabs/ai-flow#74

Merged

JPDuchesne reviewed Sep 5, 2026

View reviewed changes

ai-flow /build: let's swipe the review feedback

d195e32

Co-authored-by: JPDuchesne <2636122+JPDuchesne@users.noreply.github.com>

JPDuchesne approved these changes Sep 5, 2026

View reviewed changes

JPDuchesne and others added 8 commits September 5, 2026 09:42

test: cover ShadowenvLlvm's brew prefix detection

ff023c8

Covers detect_llvm_prefix finding an installed formula's keg, the no-usable-keg nil path, and the brew-absent nil path, all via fake brew executables on PATH. Co-authored-by: Cursor <cursoragent@cursor.com>

test: cover ShadowenvLua's luarocks install-failure branch

1ac395c

A fake brew reports lua@5.1 installed but fails the luarocks install, so ensure_homebrew_lua! raises BrewInstallError naming luarocks. Co-authored-by: Cursor <cursoragent@cursor.com>

JPDuchesne approved these changes Sep 5, 2026

View reviewed changes

JPDuchesne merged commit 5e4e255 into main Sep 5, 2026
8 checks passed

JPDuchesne deleted the ai/139-enforce-sorbet-typed-sigil branch September 5, 2026 16:07

Uh oh!

Conversation

d3mlabs-ai-flow Bot commented Aug 28, 2026

Uh oh!

codecov Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JPDuchesne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JPDuchesne commented Sep 4, 2026 • edited by d3mlabs-ai-flow Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JPDuchesne commented Sep 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d3mlabs-ai-flow Bot Sep 5, 2026

Choose a reason for hiding this comment

codecov Bot commented Aug 28, 2026 •

edited

Loading

JPDuchesne commented Sep 4, 2026 •

edited by d3mlabs-ai-flow Bot

Loading

JPDuchesne commented Sep 5, 2026 •

edited by d3mlabs-ai-flow Bot

Loading