Skip to content

fix(server): read host platform/arch via HostProcess references - #6

Merged
JTBroad merged 1 commit into
mainfrom
fix-copilot-host-process-lint
Jul 28, 2026
Merged

JTBroad merged 1 commit into
mainfrom
fix-copilot-host-process-lint

Conversation

@JTBroad

@JTBroad JTBroad commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes 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).

pnpm lint now 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 / HostProcessArchitecture are Context.References with defaults. They contribute nothing to R, so there is no layer wiring and no test setup to change — tests simply gain the ability to override the host platform.

Changes

  • resolveBundledCopilotBinaryPath and makeCopilotClientOptions become Effects
  • Their four call sites gain yield*
  • In CopilotTextGeneration, the options are hoisted out of the Effect.tryPromise callback, which cannot yield

Two details worth a reviewer's eye:

  • At 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.
  • makeCopilotClientOptions lost 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 packages
  • ✅ pnpm test — 1690 passed, 7 skipped, 0 failed

Note: 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 only apps/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

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>
@github-actions github-actions Bot added size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Jul 28, 2026
@JTBroad
JTBroad merged commit 8dd20ba into main Jul 28, 2026
6 of 10 checks passed
@JTBroad
JTBroad deleted the fix-copilot-host-process-lint branch July 28, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant