refactor(client-runtime): remove unused runtime exports and helpers - #10167

💭 I'm reviewing this cleanup PR that removes exports, deletes layer functions from service modules, and adds public annotations, checking it against the canonical module convention of imports, schema, Context.Service tag, make, then layer. I'm flagging the removal of layer export from credentialStore.ts as a potential concern worth examining further. I'm now checking whether removing service tag exports from barrels like connection/index.ts and rpc/index.ts (dropping ConnectionDriver, ConnectionResolver, RpcSessionFactory) actually breaks consumer code that depends on yield* access, or if those are re-exported elsewhere in the barrel. This looks like a broader dead-code cleanup pass, likely tool-driven, removing unused exports across vcsStatus.ts, credentialStore, profileStore, supervisor, and session modules. The key question remains whether stripping layer exports from these service modules violates the canonical "make, then layer" convention — notably, the PR itself adds /** @public Service construction is part of the canonical Effect module API. */ comments to several other modules (driver.ts, registry.ts, resolver.ts, discovery.ts, managedRelay.ts, session.ts), suggesting an intent to preserve those exports elsewhere while trimming them here. There's an inconsistency though: connectivity.ts and wakeups.ts privatize their make functions while driver.ts keeps make public with the annotation. I need to check whether those privatized make functions are still referenced internally in connectivity.ts and wakeups.ts, or if they've become dead code entirely — worth inspecting those files directly.

1. browse_code — packages/client-runtime/src/connection/connectivity.ts

2. browse_code — packages/client-runtime/src/connection/index.ts

💭 I'm weighing whether keeping make internal in connectivity.ts breaks convention, since the module still technically owns construction even though make is just a thin wrapper around Connectivity.of. The PR itself annotates some modules as @public for canonical construction, showing the author knows this convention but applied it inconsistently — privatizing make in connectivity.ts and wakeups.ts while deleting layer entirely from credentialStore, profileStore, supervisor, and session. I'm going through each case to see which ones genuinely break the "make, then layer" ordering rule versus which are defensible trims, starting with credentialStore where make stays public but layer is gone, forcing consumers to manually wrap it with Layer.succeed. Session's case is milder since layerWithOptions remains available as an alternative construction path, but supervisor and connectivity/wakeups still look like real violations since they strip out one canonical piece while leaving the tag exported. I'm weighing whether removing genuinely dead exports counts as legitimate cleanup rather than a convention breach, but since the check exists specifically to enforce this canonical shape, the clearest violations are the ones where the tag remains exported but the standard construction path (make or layer) is gone. I'm also looking at the connection barrel, which now hides the ConnectionDriver tag and layer behind only type exports rather than re-exporting the whole module namespace as the convention prefers — meaning outside consumers can't acquire that dependency anymore. I need to check whether anything outside this package actually depends on it before flagging it as a real issue. Since CI's type-check passed, these were genuinely unused exports, so this PR looks like a mechanical cleanup — but a few service modules lost their canonical make/layer construction API in the process, contradicting the convention that public service modules should expose these. That's the real finding worth flagging: connectivity.ts's make was privatized and should be restored as a public export. I'm listing the concrete offenders: wakeups.ts privatizing make similarly, credentialStore.ts/profileStore.ts/supervisor.ts dropping their layer exports, session.ts dropping its default layer (though layerWithOptions still remains, so maybe skip that one), and connection/index.ts removing the ConnectionDriver tag and ConnectionResolver from the barrel — better to re-export them as namespaces from their source files. I'm weighing whether to post all seven comments or narrow it down, and want to double-check each against the actual file contents before finalizing to keep the review precise.

3. browse_code — packages/client-runtime/src/connection/credentialStore.ts

4. browse_code — packages/client-runtime/src/connection/profileStore.ts

5. browse_code — packages/client-runtime/src/connection/wakeups.ts

6. browse_code — packages/client-runtime/src/connection/driver.ts

💭 The pattern across these service modules is that they export both make and layer, but this PR inconsistently privatizes make in some files while deleting layer in others, conflicting with the @public annotations added elsewhere. I want to check supervisor.ts and session.ts next to see if the same inconsistency shows up there.

7. browse_code — packages/client-runtime/src/connection/supervisor.ts

8. browse_code — packages/client-runtime/src/rpc/session.ts

💭 I notice the rpc barrel dropped RpcSessionFactory from its exports while keeping the RpcSession type, so consumers like driver.ts now import directly from session.ts instead of the barrel. This suggests a convention: when a barrel exposes a whole service module, prefer namespace-style exports rather than re-exporting individual members. The more interesting question is whether making make private in connectivity.ts/wakeups.ts violates the pattern where canonical service modules should export both make and layer. Since the PR uses @public annotations elsewhere to preserve exports flagged by an unused-export lint, the same reasoning should apply here rather than narrowing the API — I want to check the repo's Effect conventions doc to confirm this is a real pattern worth flagging.

9. browse_code — Are there repository conventions documented (AGENTS.md, docs/, CONTRIBUTING) about Effect service modules exporting make and layer, and about the /** @public */ annotation used to keep exports that an unused-export linter would otherwise flag? Quote the relevant doc text and name the lint/tool that reads @public.

💭 I notice the repo already privatizes make behind an exported layer elsewhere, which supports doing the same in connectivity.ts and wakeups.ts for knip cleanup — though my conventions technically call for exporting a real make, the underlying concern is really about not faking construction just to satisfy Layer.effect, so this case seems consistent with the exception for abstract-port implementations. I want to avoid over-flagging a pure dead-export-removal PR, since the check explicitly discourages demanding repo-wide cleanup or nitpicking style preferences. The bigger question is whether removing layer from credentialStore/profileStore/supervisor forces consumers to inline Layer.succeed themselves at call sites — I should grep for how those are actually used now to see if that violates the convention that canonical modules own their layer.

10. browse_code — Where are the layers for ConnectionCredentialStore, ConnectionProfileStore, and EnvironmentSupervisor provided/constructed now (after removing the exported layer helpers from credentialStore.ts, profileStore.ts, supervisor.ts)? Show the call sites that build these layers (e.g. Layer.succeed/Layer.effect/Layer.scoped with these tags) across apps/ and packages/, with file paths and code snippets. Also show where RpcSessionFactory's layer/layerWithOptions is used and where EnvironmentRpc config and ConnectionResolver were consumed.

💭 These removals align with actual usage patterns—apps construct via Layer.effectContext and Context.add with make(), so the standalone layer helpers were dead code, and the supervisor is built per-environment without needing a layer. This isn't a convention violation but simply removing unused exports, with the canonical make() API staying intact where modules own construction, and the private make functions in connectivity.ts/wakeups.ts matching the existing repo pattern of private make plus exported layer. Now I'm scanning the rest of the diff for genuine convention issues rather than style nitpicks. In errorPresentation.ts, dpopFailureHint being privatized is fine. In state/gitActions.ts, several functions were removed along with unused imports — I want to confirm resolveLiveThreadBranchUpdate and resolveThreadBranchUpdate aren't used elsewhere, but since CI/typecheck passed, it's likely they were genuinely dead code rather than a behavioral regression, though this is somewhat outside the scope of the Effect service conventions I'm focused on. Checking the remaining files: pullRequests.ts's @public annotation looks fine, no new lint-disable directives or bare "effect" imports appear, and the removal of Layer imports matches the removed code. The vcs.ts barrel dropped its re-export of vcsStatus.ts along with the deleted VcsStatusTarget type, which passed typechecking so it's not an issue. The connection and rpc barrels only export type-level items rather than whole service modules, so the "prefer export * as X" convention doesn't apply there either — I'm not finding a real violation so far. I'm reconsidering whether the @public comments actually count as lint-disable directives, since they suppress knip's unused-export warnings; but each one does include an adjacent explanation like "Service construction is part of the canonical Effect module API," so they satisfy the documentation requirement either way. Now I'm verifying that each of the six @public tags is placed directly above the correct export it's meant to justify, checking registry.ts, resolver.ts, and discovery.ts one by one.

11. browse_code — packages/client-runtime/src/connection/registry.ts

12. browse_code — packages/client-runtime/src/relay/discovery.ts

💭 The @public annotations look correctly placed with explanations, and dependencies are acquired consistently via the yield pattern. I'm also checking whether privatizing make while keeping layer exported in connectivity/wakeups creates any issue, since the layer passes the service instance as a parameter to make — but this is pre-existing code where only the export keyword changed, and it's platform-provided implementation injection. Doing a final scan of remaining changed items: state/runtime.ts removed dead code and privatized runInEnvironment, fine; state/environmentHttpAuth.ts privatized two functions that are still used internally, so the lint check passes. Everything looks clear — no findings to report, so I'll set the final verdict and summary fields to indicate "All clear."