Skip to content

Commit 6b8805b

Browse files
cursoragentjuliusmarminge
cursoragent
and
juliusmarminge
committed
Fix duplicated utility functions and unsafe constructor fallback
- Extract shared command resolution helpers (stripWrappingQuotes, resolvePathEnvironmentVariable, resolveWindowsPathExtensions, resolveCommandCandidates, isExecutableFile) into a new commandResolution.ts module, eliminating duplication between processRunner.ts and open.ts. - Make CodexAppServerManager constructor require services parameter instead of using an unsafe cast that erases Effect service requirements when services are omitted. Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
1 parent b249442 commit 6b8805b

5 files changed

Lines changed: 101 additions & 152 deletions

File tree

‎apps/server/src/codexAppServerManager.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -169,15 +169,9 @@ export class CodexAppServerManager extends EventEmitter<CodexAppServerManagerEve
169169
options?: Effect.RunOptions | undefined,
170170
) => Promise<A>;
171171

172-
constructor(services?: ServiceMap.ServiceMap<NodeServices.NodeServices>) {
172+
constructor(services: ServiceMap.ServiceMap<NodeServices.NodeServices>) {
173173
super();
174-
this.runPromise = services
175-
? Effect.runPromiseWith(services)
176-
: ((effect, options) =>
177-
Effect.runPromise(
178-
effect as unknown as Effect.Effect<unknown, never>,
179-
options,
180-
)) as typeof this.runPromise;
174+
this.runPromise = Effect.runPromiseWith(services);
181175
}
182176

183177
async startSession(input: ProviderSessionStartInput): Promise<ProviderSession> {

‎apps/server/src/commandResolution.ts‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { accessSync, constants, statSync } from "node:fs";
2+
import { extname } from "node:path";
3+
4+
export function stripWrappingQuotes(value: string): string {
5+
return value.replace(/^"+|"+$/g, "");
6+
}
7+
8+
export function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string {
9+
return env.PATH ?? env.Path ?? env.path ?? "";
10+
}
11+
12+
export function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray<string> {
13+
const rawValue = env.PATHEXT;
14+
const fallback = [".COM", ".EXE", ".BAT", ".CMD"];
15+
if (!rawValue) return fallback;
16+
17+
const parsed = rawValue
18+
.split(";")
19+
.map((entry) => entry.trim())
20+
.filter((entry) => entry.length > 0)
21+
.map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`));
22+
return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback;
23+
}
24+
25+
export function resolveCommandCandidates(
26+
command: string,
27+
platform: NodeJS.Platform,
28+
windowsPathExtensions: ReadonlyArray<string>,
29+
): ReadonlyArray<string> {
30+
if (platform !== "win32") return [command];
31+
const extension = extname(command);
32+
const normalizedExtension = extension.toUpperCase();
33+
34+
if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) {
35+
const commandWithoutExtension = command.slice(0, -extension.length);
36+
return Array.from(
37+
new Set([
38+
command,
39+
`${commandWithoutExtension}${normalizedExtension}`,
40+
`${commandWithoutExtension}${normalizedExtension.toLowerCase()}`,
41+
]),
42+
);
43+
}
44+
45+
const candidates: string[] = [];
46+
for (const candidateExtension of windowsPathExtensions) {
47+
candidates.push(`${command}${candidateExtension}`);
48+
candidates.push(`${command}${candidateExtension.toLowerCase()}`);
49+
}
50+
return Array.from(new Set(candidates));
51+
}
52+
53+
export function isExecutableFile(
54+
filePath: string,
55+
platform: NodeJS.Platform,
56+
windowsPathExtensions: ReadonlyArray<string>,
57+
): boolean {
58+
try {
59+
const stat = statSync(filePath);
60+
if (!stat.isFile()) return false;
61+
if (platform === "win32") {
62+
const extension = extname(filePath);
63+
if (extension.length === 0) return false;
64+
return windowsPathExtensions.includes(extension.toUpperCase());
65+
}
66+
accessSync(filePath, constants.X_OK);
67+
return true;
68+
} catch {
69+
return false;
70+
}
71+
}

‎apps/server/src/open.ts‎

Lines changed: 8 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,17 @@
66
*
77
* @module Open
88
*/
9-
import { accessSync, constants, statSync } from "node:fs";
10-
import { extname, join } from "node:path";
9+
import { join } from "node:path";
1110

1211
import { EDITORS, type EditorId, type ServerRuntimeEnvironment } from "@t3tools/contracts";
1312
import { ServiceMap, Schema, Effect, Layer } from "effect";
13+
import {
14+
isExecutableFile,
15+
resolveCommandCandidates,
16+
resolvePathEnvironmentVariable,
17+
resolveWindowsPathExtensions,
18+
stripWrappingQuotes,
19+
} from "./commandResolution";
1420
import { spawnDetachedProcess, spawnProcessSync } from "./processRunner";
1521
import { detectServerRuntimeEnvironment } from "./runtimeEnvironment";
1622

@@ -72,75 +78,6 @@ function fileManagerCommandForPlatform(platform: NodeJS.Platform): string {
7278
}
7379
}
7480

75-
function stripWrappingQuotes(value: string): string {
76-
return value.replace(/^"+|"+$/g, "");
77-
}
78-
79-
function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string {
80-
return env.PATH ?? env.Path ?? env.path ?? "";
81-
}
82-
83-
function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray<string> {
84-
const rawValue = env.PATHEXT;
85-
const fallback = [".COM", ".EXE", ".BAT", ".CMD"];
86-
if (!rawValue) return fallback;
87-
88-
const parsed = rawValue
89-
.split(";")
90-
.map((entry) => entry.trim())
91-
.filter((entry) => entry.length > 0)
92-
.map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`));
93-
return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback;
94-
}
95-
96-
function resolveCommandCandidates(
97-
command: string,
98-
platform: NodeJS.Platform,
99-
windowsPathExtensions: ReadonlyArray<string>,
100-
): ReadonlyArray<string> {
101-
if (platform !== "win32") return [command];
102-
const extension = extname(command);
103-
const normalizedExtension = extension.toUpperCase();
104-
105-
if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) {
106-
const commandWithoutExtension = command.slice(0, -extension.length);
107-
return Array.from(
108-
new Set([
109-
command,
110-
`${commandWithoutExtension}${normalizedExtension}`,
111-
`${commandWithoutExtension}${normalizedExtension.toLowerCase()}`,
112-
]),
113-
);
114-
}
115-
116-
const candidates: string[] = [];
117-
for (const extension of windowsPathExtensions) {
118-
candidates.push(`${command}${extension}`);
119-
candidates.push(`${command}${extension.toLowerCase()}`);
120-
}
121-
return Array.from(new Set(candidates));
122-
}
123-
124-
function isExecutableFile(
125-
filePath: string,
126-
platform: NodeJS.Platform,
127-
windowsPathExtensions: ReadonlyArray<string>,
128-
): boolean {
129-
try {
130-
const stat = statSync(filePath);
131-
if (!stat.isFile()) return false;
132-
if (platform === "win32") {
133-
const extension = extname(filePath);
134-
if (extension.length === 0) return false;
135-
return windowsPathExtensions.includes(extension.toUpperCase());
136-
}
137-
accessSync(filePath, constants.X_OK);
138-
return true;
139-
} catch {
140-
return false;
141-
}
142-
}
143-
14481
function resolvePathDelimiter(platform: NodeJS.Platform): string {
14582
return platform === "win32" ? ";" : ":";
14683
}

‎apps/server/src/processRunner.ts‎

Lines changed: 10 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,19 @@ import {
55
spawnSync,
66
type StdioOptions,
77
} from "node:child_process";
8-
import { statSync } from "node:fs";
98
import { extname, join } from "node:path";
109

1110
import type { ServerRuntimeEnvironment } from "@t3tools/contracts";
1211
import { Effect, Exit, Scope } from "effect";
1312
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
1413

14+
import {
15+
isExecutableFile,
16+
resolveCommandCandidates,
17+
resolvePathEnvironmentVariable,
18+
resolveWindowsPathExtensions,
19+
stripWrappingQuotes,
20+
} from "./commandResolution";
1521
import { detectServerRuntimeEnvironment } from "./runtimeEnvironment";
1622

1723
interface ProcessSpawnBaseOptions {
@@ -93,71 +99,6 @@ function resolveRuntimeEnvironment(
9399
return runtimeEnvironment ?? detectServerRuntimeEnvironment();
94100
}
95101

96-
function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string {
97-
return env.PATH ?? env.Path ?? env.path ?? "";
98-
}
99-
100-
function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray<string> {
101-
const rawValue = env.PATHEXT;
102-
const fallback = [".COM", ".EXE", ".BAT", ".CMD"];
103-
if (!rawValue) return fallback;
104-
105-
const parsed = rawValue
106-
.split(";")
107-
.map((entry) => entry.trim())
108-
.filter((entry) => entry.length > 0)
109-
.map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`));
110-
return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback;
111-
}
112-
113-
function resolveCommandCandidates(
114-
command: string,
115-
windowsPathExtensions: ReadonlyArray<string>,
116-
): ReadonlyArray<string> {
117-
const extension = extname(command);
118-
const normalizedExtension = extension.toUpperCase();
119-
120-
if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) {
121-
const commandWithoutExtension = command.slice(0, -extension.length);
122-
return Array.from(
123-
new Set([
124-
command,
125-
`${commandWithoutExtension}${normalizedExtension}`,
126-
`${commandWithoutExtension}${normalizedExtension.toLowerCase()}`,
127-
]),
128-
);
129-
}
130-
131-
const candidates: string[] = [command];
132-
for (const candidateExtension of windowsPathExtensions) {
133-
candidates.push(`${command}${candidateExtension}`);
134-
candidates.push(`${command}${candidateExtension.toLowerCase()}`);
135-
}
136-
return Array.from(new Set(candidates));
137-
}
138-
139-
function stripWrappingQuotes(value: string): string {
140-
return value.replace(/^"+|"+$/g, "");
141-
}
142-
143-
function isExecutableFile(
144-
filePath: string,
145-
windowsPathExtensions: ReadonlyArray<string>,
146-
): boolean {
147-
try {
148-
const stat = statSync(filePath);
149-
if (!stat.isFile()) return false;
150-
const extension = extname(filePath);
151-
if (extension.length === 0) return false;
152-
if (windowsPathExtensions.length === 0) {
153-
return true;
154-
}
155-
return windowsPathExtensions.includes(extension.toUpperCase());
156-
} catch {
157-
return false;
158-
}
159-
}
160-
161102
function resolveEffectiveEnvironment(options: ProcessLaunchPlanOptions): NodeJS.ProcessEnv {
162103
const env = (options.env ?? {}) as NodeJS.ProcessEnv;
163104
if (options.inheritParentEnv === false) {
@@ -175,7 +116,7 @@ function resolveWindowsCommand(
175116
env: NodeJS.ProcessEnv,
176117
): ResolvedWindowsCommand | null {
177118
const windowsPathExtensions = resolveWindowsPathExtensions(env);
178-
const candidates = resolveCommandCandidates(command, windowsPathExtensions);
119+
const candidates = resolveCommandCandidates(command, "win32", windowsPathExtensions);
179120

180121
const classify = (filePath: string): ResolvedWindowsCommand => {
181122
const extension = extname(filePath).toUpperCase();
@@ -187,7 +128,7 @@ function resolveWindowsCommand(
187128

188129
if (command.includes("/") || command.includes("\\")) {
189130
for (const candidate of candidates) {
190-
if (isExecutableFile(candidate, windowsPathExtensions)) {
131+
if (isExecutableFile(candidate, "win32", windowsPathExtensions)) {
191132
return classify(candidate);
192133
}
193134
}
@@ -202,7 +143,7 @@ function resolveWindowsCommand(
202143
for (const pathEntry of pathEntries) {
203144
for (const candidate of candidates) {
204145
const candidatePath = join(pathEntry, candidate);
205-
if (isExecutableFile(candidatePath, windowsPathExtensions)) {
146+
if (isExecutableFile(candidatePath, "win32", windowsPathExtensions)) {
206147
return classify(candidatePath);
207148
}
208149
}

‎apps/server/src/provider/Layers/CodexAdapter.test.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import * as NodeServices from "@effect/platform-node/NodeServices";
1515
import { afterAll, assert, it, vi } from "@effect/vitest";
1616
import { assertFailure } from "@effect/vitest/utils";
1717

18-
import { Effect, Fiber, Layer, Option, Stream } from "effect";
18+
import { Effect, Fiber, Layer, Option, ServiceMap, Stream } from "effect";
1919

2020
import {
2121
CodexAppServerManager,
@@ -132,7 +132,9 @@ const providerSessionDirectoryTestLayer = Layer.succeed(ProviderSessionDirectory
132132
listSessionIds: () => Effect.succeed([]),
133133
});
134134

135-
const validationManager = new FakeCodexManager();
135+
const validationManager = new FakeCodexManager(
136+
ServiceMap.empty() as ServiceMap.ServiceMap<NodeServices.NodeServices>,
137+
);
136138
const validationLayer = it.layer(
137139
makeCodexAdapterLive({ manager: validationManager }).pipe(
138140
Layer.provideMerge(ServerConfig.layerTest(process.cwd(), process.cwd())),
@@ -164,7 +166,9 @@ validationLayer("CodexAdapterLive validation", (it) => {
164166
);
165167
});
166168

167-
const sessionErrorManager = new FakeCodexManager();
169+
const sessionErrorManager = new FakeCodexManager(
170+
ServiceMap.empty() as ServiceMap.ServiceMap<NodeServices.NodeServices>,
171+
);
168172
sessionErrorManager.sendTurnImpl.mockImplementation(async () => {
169173
throw new Error("Unknown session: sess-missing");
170174
});
@@ -204,7 +208,9 @@ sessionErrorLayer("CodexAdapterLive session errors", (it) => {
204208
);
205209
});
206210

207-
const lifecycleManager = new FakeCodexManager();
211+
const lifecycleManager = new FakeCodexManager(
212+
ServiceMap.empty() as ServiceMap.ServiceMap<NodeServices.NodeServices>,
213+
);
208214
const lifecycleLayer = it.layer(
209215
makeCodexAdapterLive({ manager: lifecycleManager }).pipe(
210216
Layer.provideMerge(ServerConfig.layerTest(process.cwd(), process.cwd())),

0 commit comments

Comments
 (0)