Repository navigation
fix(server): read host platform/arch via HostProcess references - #6
Merged
Merged
Conversation
Resolves the only three lint errors in the repo: the Copilot bundled binary resolver read `process.platform` and `process.arch` directly, violating t3code(no-global-process-runtime). `HostProcessPlatform` / `HostProcessArchitecture` are `Context.Reference`s with defaults, so they add nothing to `R` — no layer wiring or test changes are needed, and tests can now override the host platform. `resolveBundledCopilotBinaryPath` and `makeCopilotClientOptions` become Effects, so their four call sites gain `yield*`. In CopilotTextGeneration the options are hoisted out of the `Effect.tryPromise` callback, which cannot yield. `pnpm lint` now exits 0 (was 1). Co-Authored-By: Claude <noreply@anthropic.com>
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
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.
Summary
Fixes the only three lint errors in the repo. The Copilot bundled-binary resolver read
process.platformandprocess.archdirectly, violatingt3code(no-global-process-runtime).pnpm lintnow exits 0 (was exit 1).Why this was cheap
The rule's message ("inject the runtime reference … and provide it explicitly in tests") suggests layer plumbing, but
HostProcessPlatform/HostProcessArchitectureareContext.References with defaults. They contribute nothing toR, so there is no layer wiring and no test setup to change — tests simply gain the ability to override the host platform.Changes
resolveBundledCopilotBinaryPathandmakeCopilotClientOptionsbecome Effectsyield*CopilotTextGeneration, the options are hoisted out of theEffect.tryPromisecallback, which cannot yieldTwo details worth a reviewer's eye:
CopilotAdapter.ts(listAgents), the call is the right operand of??.yield*there still short-circuits, so the existing "reuse a live client" behavior is preserved — noted in a comment.makeCopilotClientOptionslost its explicit return-type annotation to become a generator;satisfies ConstructorParameters<typeof CopilotClient>[0]preserves the same checking.Verification
pnpm lint— exit 0, no errors (warnings unchanged)pnpm typecheck— clean across all packagespnpm test— 1690 passed, 7 skipped, 0 failedNote: a pre-existing flaky test
One full-suite run failed in
apps/web/src/lib/stashImageCompression.test.ts— "reports too-large when even the smallest encoding overflows the budget". It passes 4/4 in isolation and passed on a clean re-run, and this PR touches onlyapps/server. It appears to fail only under full-suite parallelism. Pre-existing and unrelated; worth a separate look.Not verified
The Copilot provider was not exercised at runtime. The refactor is behavior-preserving by construction (same values, same order), but no test covers
resolveBundledCopilotBinaryPath, so binary resolution is confirmed only by typecheck.🤖 Generated with Claude Code