fix(server): resolve pre-existing typecheck drift#2
Merged
Berkay2002 merged 1 commit intomainfrom Apr 16, 2026
Merged
Conversation
- CodexProvider: use conditional spread for optional `skills` so `exactOptionalPropertyTypes` does not surface `undefined`. - PerfProviderRegistry: add `slashCommands: []` and `skills: []` to the codex and claudeAgent fixture snapshots so they satisfy ServerProvider. - PerfProviderAdapter: add a no-op `compactThread` stub (succeeds with `null` for known sessions) to satisfy ProviderAdapterShape. - PerfProviderAdapter: capture the runtime context once and use `Effect.runForkWith` inside the timer callback; yield tagged errors directly instead of wrapping them in `Effect.fail`. - PerfProviderLayers: switch to `Layer.provideMerge` for the session directory so the perf provider exposes it to downstream consumers (matches DefaultProviderLayerLive) — this also resolves the `ProviderSessionDirectory` context leak flagged by cli.test.ts and server.ts. - ProviderSessionReaper.test: add `stopSessionForProvider` and `compactThread` mocks to the mock `ProviderServiceShape`.
3 tasks
There was a problem hiding this comment.
Pull request overview
Restores clean TypeScript typechecking in the apps/server workspace by aligning server-side provider implementations, perf fixtures, and layer wiring with the current provider contracts and Effect runtime patterns.
Changes:
- Fixes
exactOptionalPropertyTypesissues by omitting optionalskillswhen undefined in Codex provider snapshots. - Updates perf provider fixture snapshots to include required
slashCommands/skillsarrays perServerProvidercontract. - Adjusts perf provider adapter/runtime and layer composition (adds
compactThread, captures Effect context forrunFork, and exposes session directory downstream viaLayer.provideMerge), plus updates test mocks accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/server/src/provider/Layers/ProviderSessionReaper.test.ts | Extends the mocked ProviderServiceShape to include newly-required methods (stopSessionForProvider, compactThread). |
| apps/server/src/provider/Layers/CodexProvider.ts | Avoids explicitly passing skills: undefined by conditionally spreading the property. |
| apps/server/src/perf/PerfProviderRegistry.ts | Adds slashCommands: [] and skills: [] to perf fixture ServerProvider literals to satisfy typing. |
| apps/server/src/perf/PerfProviderLayers.ts | Switches to Layer.provideMerge so ProviderSessionDirectory remains available to downstream consumers. |
| apps/server/src/perf/PerfProviderAdapter.ts | Captures runtime context for use in setTimeout callbacks, yields tagged errors directly, and adds a compactThread stub. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Apr 16, 2026
Berkay2002
added a commit
that referenced
this pull request
Apr 17, 2026
9 tasks
Berkay2002
added a commit
that referenced
this pull request
Apr 17, 2026
…/Linux IDs (#9) * feat(shared): add env dual-read shim (BCODE_* preferred, T3CODE_* fallback with warning) * feat(rebrand): flip server CLI env vars via Effect Config shim BCODE_* preferred with T3CODE_* fallback + per-key deprecation warning. Rename bootstrap envelope field t3Home -> bcodeHome. Default otlpServiceName flips from t3-server to bcode-server. * feat(rebrand): flip desktop + scripts env readers to new names Desktop app.ts, updateState, terminal Manager, perf harnesses, telemetry, projectScripts, vite/web, dev-runner, build-desktop-artifact, mock-update and misc tests move to BCODE_*. Scripts with user-facing env (dev-runner, build-desktop-artifact) keep a BCODE_* preferred / T3CODE_* fallback shim that warns once per legacy key. projectScriptRuntimeEnv writes both prefixes so user-authored project scripts continue reading legacy names. Terminal env strip filter now drops both BCODE_* and T3CODE_* keys. * feat(rebrand): flip shell capture sentinels __T3CODE_* to __BCODE_* Internal-only markers grep'd out of captured shell output. No persistence or external interface, safe to flip without a legacy-name fallback. * chore(rebrand): add BCODE_* keys to turbo globalEnv alongside T3CODE_* Dual-listing matches the env var shim window: cache invalidation fires when either prefix changes through v0.0.19. The T3CODE_* entries are removed together with the shim in v0.0.20. * feat(rebrand): rename desktop internal protocol scheme t3:// to bcode:// DESKTOP_SCHEME is an electron-internal asset protocol used for packaged UI loading. No OS-level handler is registered via setAsDefaultProtocolClient, so this flip has no external surface and needs no migration shim. * feat(rebrand): rename COM/bundle ID to com.berkayorhan.bcode and artifactName to BCode-* Flips APP_USER_MODEL_ID (Windows AUMID), APP_BUNDLE_ID (macOS plist patching in the electron launcher), and the electron-builder appId + artifactName pattern. Changes the installed-app identity on new installs; existing installs continue to resolve their own identity via electron's userData paths (deliberately kept under the legacy t3code name). * feat(rebrand): rename Linux entry, WM class, and internal t3code identifiers to bcode LINUX_DESKTOP_ENTRY_NAME, LINUX_WM_CLASS, executableName, StartupWMClass, the dev-electron pkill marker arg, the packaged package.json name, the temp stage dir prefixes, and the bcodeCommitHash metadata field all flip to bcode. USER_DATA_DIR_NAME remains 't3code' deliberately (per AGENTS.md) to preserve electron-managed state on existing installs. * test(rebrand): flip release fixture artifact names T3-Code-* to BCode-* Follows the commit 8 artifactName flip: update-manifest test fixtures and release-smoke-test fixture URLs must match the new artifact naming pattern or the tests fail on what's effectively stale hardcoded data. * docs(rebrand): rewrite current docs for BCODE_*, bcode://, ~/.bcode Updates observability, release, perf-benchmarks, quick-start, scripts, KEYBINDINGS, and debugging rule docs to use the new env var names and home directory. Adds an env var deprecation-window note at the top of observability.md. Historical plans and specs are left untouched. * docs(plan): add PR #2 execution breakdown * test(rebrand): assert dual-prefix env keys in project setup script The projectScriptRuntimeEnv helper writes both BCODE_* and T3CODE_* aliases through v0.0.19 so user-authored project scripts keep reading legacy names. The setup-script runner assertion must match that shape. Other projectScripts/ChatView assertions already use toMatchObject partial matching so they were unaffected. * fix(rebrand): address Copilot review on PR #9 - desktop main.ts:1412: write bcodeHome (not t3Home) to the bootstrap envelope so the server honors the desktop-selected base dir after the schema field rename. - turbo.json: add BCODE_WEB_SOURCEMAP and T3CODE_WEB_SOURCEMAP to globalEnv so vite sourcemap changes invalidate turbo cache. - build-desktop-artifact.ts: emit the one-time legacy-env deprecation warning for T3CODE_DESKTOP_UPDATE_REPOSITORY like the other shimmed reads. - observability.md / KEYBINDINGS.md / debugging.md / scripts.md: revert hardcoded ~/.bcode/... paths to ~/.t3/... with a note about the upcoming home-dir flip. The runtime default is still ~/.t3 until PR #3 lands the auto-migration, so user-facing docs must match the current on-disk reality. Env var name flips (BCODE_*) are retained since the shim honors both.
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 pre-existing
bun run --cwd apps/server typecheckbreakage so the server workspace typechecks cleanly again. Sibling to PR #1 (web typecheck drift) — allows the release-pipeline PR to rebase on a green main.Reduces server typecheck errors from 41 → 10 (remaining 10 are pre-existing perf-test drift in
integration/perf/serverLatency.perf.test.tsthat reference removed RPC methodsWS_METHODS.subscribeOrchestrationDomainEventsandWS_METHODS.gitStatus; those need a rewrite against the new RPC surface and are out of scope for this typecheck-drift patch).Changes
CodexProvider.ts:397(TS2379) — use conditional-spread for optionalskillssoexactOptionalPropertyTypesdoesn't surfaceundefined.PerfProviderRegistry.ts:49/70(TS2739) — addslashCommands: []andskills: []to the codex and claudeAgent fixture snapshots.PerfProviderAdapter.ts:455(TS1360) — add a no-opcompactThreadstub that succeeds withnullfor known sessions.PerfProviderAdapter.ts:192(effect-lsp TS32) — capture the runtime context once withEffect.contextand useEffect.runForkWithinside thesetTimeoutcallback.PerfProviderAdapter.ts:248/346/383/412(effect-lsp TS29) — yield tagged errors directly instead of wrapping them inEffect.fail.PerfProviderLayers.ts— switch toLayer.provideMergefor the session directory so the perf provider exposes it to downstream consumers (matchesDefaultProviderLayerLive). This resolves theProviderSessionDirectorycontext leak flagged bycli.test.tsandserver.tsunder the ternary-layer selection.ProviderSessionReaper.test.ts:134(TS2739) — addstopSessionForProviderandcompactThreadmocks to the mockProviderServiceShape.Test plan
bun run --cwd apps/server typecheck— allsrc/errors resolved (10 pre-existingintegration/perf/errors remain; see summary)bun run fmtbun run lint— 0 errors