Skip to content

Commit 36c1db5

Browse files
committed
Merge: orphan-fix + isHealthy 401 + Makefile install targets
Three atomic commits delivering production fixes the user found after PR #2 merged: - fix(opencode) a29de6d: prevent orphan opencode-serve CPU spin on IDE quit. Removes '--print-logs' (which fueled an EPIPE storm in bun's logger callback when parent stdio pipes broke) and adds killStaleServer() pre-spawn guard that lsof/netstat-discovers and SIGKILLs any orphan holding port 5888 from a prior IDE run. Mirrors the well-behaved opencode-web-for-vscode ProcessManager. - build 5594814: add make build/install/uninstall targets so the workflow 'make install' replaces the long manual gulp + cp dance. - fix(opencode) b957102: isHealthy() must reject 401. Previously 'response.statusCode === 401 || isHealthResponse(body)' allowed the manager to falsely adopt an orphan opencode-serve from a prior IDE session whose password no longer matched the new manager's password, causing the SPA to load with 'Error: Unauthorized'. Strict 200-only health check makes doStart() fall through to killStaleServer() in this case. Local: 31/31 unit tests pass; user verified 'quit + restart' no longer leaves orphans or 401s.
2 parents cf9dfe6 + b957102 commit 36c1db5

3 files changed

Lines changed: 119 additions & 6 deletions

File tree

Makefile

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ REPO := $(abspath $(ROOT)/..)
66
SPA_SRC := $(REPO)/packages/app
77
SPA_DIST := $(SPA_SRC)/dist
88
SPA_DST := $(ROOT)/src/vs/workbench/contrib/opencode/media/spa
9+
APP_NAME := OpenCode IDE
10+
APP_BUNDLE := $(APP_NAME).app
11+
BUILD_OUT := $(REPO)/VSCode-darwin-arm64/$(APP_BUNDLE)
12+
INSTALL_DST := /Applications/$(APP_BUNDLE)
913
NVM_SETUP := source $$HOME/.nvm/nvm.sh && nvm use 22
1014

1115
.PHONY: help
@@ -25,6 +29,11 @@ help:
2529
@echo " Run:"
2630
@echo " make run launch dev build via scripts/code.sh"
2731
@echo ""
32+
@echo " Install:"
33+
@echo " make build build .app via gulp vscode-darwin-arm64"
34+
@echo " make install compile + build + cp .app to /Applications (with backup)"
35+
@echo " make uninstall restore the most recent /Applications/<app>.bak.*"
36+
@echo ""
2837
@echo " Test:"
2938
@echo " make smoke run full smoke suite"
3039
@echo " make smoke-opencode smoke filtered to OpenCode tests"
@@ -82,3 +91,33 @@ typecheck:
8291
clean:
8392
rm -rf out/ node_modules/*cache
8493
@echo ">> Removed out/ and node_modules/*cache"
94+
95+
.PHONY: build
96+
build:
97+
@echo ">> gulp vscode-darwin-arm64..."
98+
bash -lc '$(NVM_SETUP) && NODE_OPTIONS=--max-old-space-size=8192 npx gulp vscode-darwin-arm64'
99+
@test -d "$(BUILD_OUT)" || { echo "ERROR: $(BUILD_OUT) not produced"; exit 1; }
100+
@echo ">> Built: $(BUILD_OUT)"
101+
102+
.PHONY: install
103+
install: compile build
104+
@if [ -d "$(INSTALL_DST)" ]; then \
105+
BACKUP="$(INSTALL_DST).bak.$$(date +%Y%m%d-%H%M%S)"; \
106+
echo ">> Backing up $(INSTALL_DST)$$BACKUP"; \
107+
mv "$(INSTALL_DST)" "$$BACKUP"; \
108+
fi
109+
@echo ">> Installing $(BUILD_OUT)$(INSTALL_DST)"
110+
cp -R "$(BUILD_OUT)" "$(INSTALL_DST)"
111+
@stat -f ">> Installed at: %Sm" "$(INSTALL_DST)/Contents/Info.plist"
112+
@/usr/libexec/PlistBuddy -c "Print :CFBundleShortVersionString" "$(INSTALL_DST)/Contents/Info.plist" | xargs -I{} echo ">> Version: {}"
113+
114+
.PHONY: uninstall
115+
uninstall:
116+
@LATEST=$$(ls -1dt "$(INSTALL_DST).bak."* 2>/dev/null | head -1); \
117+
if [ -z "$$LATEST" ]; then \
118+
echo "ERROR: No backup found matching $(INSTALL_DST).bak.*"; exit 1; \
119+
fi; \
120+
echo ">> Restoring backup: $$LATEST"; \
121+
rm -rf "$(INSTALL_DST)"; \
122+
mv "$$LATEST" "$(INSTALL_DST)"; \
123+
echo ">> Restored: $(INSTALL_DST)"

src/vs/workbench/contrib/opencode/electron-main/opencodeServeManager.ts

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ export class OpencodeServeManager
182182
this.password ??
183183
randomBytes(16).toString("hex");
184184

185+
const port = this.getPort();
185186
const backendUrl = this.getConfiguredBackendUrl();
186187
if (await this.isHealthy(backendUrl, this.password)) {
187188
this.state = "running";
@@ -194,14 +195,15 @@ export class OpencodeServeManager
194195
return backendUrl;
195196
}
196197

198+
await this.killStaleServer(port);
199+
197200
const binaryPath = this.findBinaryPath();
198201
if (!binaryPath) {
199202
throw new Error(
200203
"Failed to find an opencode binary. Configure opencode.binaryPath or add opencode to PATH.",
201204
);
202205
}
203206

204-
const port = this.getPort();
205207
const deadline = Date.now() + STARTUP_TIMEOUT;
206208
const env = { ...process.env };
207209
delete env.OPENCODE_SERVER_PASSWORD;
@@ -214,7 +216,6 @@ export class OpencodeServeManager
214216
"127.0.0.1",
215217
"--port",
216218
String(port),
217-
"--print-logs",
218219
],
219220
{
220221
env,
@@ -393,7 +394,7 @@ export class OpencodeServeManager
393394
): Promise<boolean> {
394395
try {
395396
const response = await this.readHealth(url, password);
396-
return response.statusCode === 401 || isHealthResponse(response.body);
397+
return isHealthResponse(response.body);
397398
} catch {
398399
return false;
399400
}
@@ -494,6 +495,73 @@ export class OpencodeServeManager
494495
return undefined;
495496
}
496497

498+
protected async killStaleServer(port: number): Promise<void> {
499+
const url = `http://127.0.0.1:${port}/`;
500+
501+
try {
502+
if (await this.isHealthy(url, this.password)) {
503+
return;
504+
}
505+
} catch {
506+
// Treat any error as not healthy.
507+
}
508+
509+
const pid = await this.findProcessOnPort(port);
510+
if (pid === undefined) {
511+
return;
512+
}
513+
514+
this.logService.warn("[opencode] killing stale process holding port", {
515+
pid,
516+
port,
517+
});
518+
try {
519+
process.kill(pid, "SIGKILL");
520+
} catch (error) {
521+
this.logService.warn("[opencode] failed to kill stale process", {
522+
pid,
523+
error,
524+
});
525+
return;
526+
}
527+
528+
await this.sleep(500);
529+
}
530+
531+
protected async findProcessOnPort(port: number): Promise<number | undefined> {
532+
if (platform() === "win32") {
533+
try {
534+
const output = execSync("netstat -ano -p tcp", { encoding: "utf8" });
535+
const re = new RegExp(
536+
`\\s127\\.0\\.0\\.1:${port}\\s+\\S+\\s+LISTENING\\s+(\\d+)$`,
537+
);
538+
for (const line of output.split(/\r?\n/)) {
539+
const match = re.exec(line);
540+
if (match) {
541+
return Number(match[1]);
542+
}
543+
}
544+
return undefined;
545+
} catch {
546+
return undefined;
547+
}
548+
}
549+
550+
try {
551+
const output = execSync(`lsof -t -iTCP:${port} -sTCP:LISTEN`, {
552+
encoding: "utf8",
553+
}).trim();
554+
if (!output) {
555+
return undefined;
556+
}
557+
558+
const pid = Number(output.split(/\r?\n/)[0]);
559+
return Number.isFinite(pid) ? pid : undefined;
560+
} catch {
561+
return undefined;
562+
}
563+
}
564+
497565
private resolveConfiguredBinaryPath(
498566
configuredPath: string,
499567
): string | undefined {

src/vs/workbench/contrib/opencode/test/electron-main/opencodeServeManager.test.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,12 @@ class TestableOpencodeServeManager extends OpencodeServeManager {
148148

149149
return this.nextProcess as unknown as ChildProcess;
150150
}
151+
protected override async killStaleServer(_port: number): Promise<void> {
152+
// No-op in tests: real lsof/netstat lookup would find the mock HTTP backend in this process.
153+
}
154+
protected override async findProcessOnPort(_port: number): Promise<number | undefined> {
155+
return undefined;
156+
}
151157
get testState() { return this._testState; }
152158
get testWeStarted() { return this._testWeStarted; }
153159
}
@@ -263,7 +269,7 @@ suite('OpencodeServeManager / lifecycle', () => {
263269

264270
assert.strictEqual(url, `http://127.0.0.1:${backend.port}`);
265271
assert.strictEqual(manager.spawnCalls.length, 1);
266-
assert.deepStrictEqual(manager.spawnCalls[0].args, ['serve', '--hostname', '127.0.0.1', '--port', String(backend.port), '--print-logs']);
272+
assert.deepStrictEqual(manager.spawnCalls[0].args, ['serve', '--hostname', '127.0.0.1', '--port', String(backend.port)]);
267273
assert.strictEqual(manager.spawnCalls[0].options.env?.OPENCODE_SERVER_PASSWORD, manager.getPassword());
268274
assert.deepStrictEqual(manager.spawnCalls[0].options.stdio, ['ignore', 'pipe', 'pipe']);
269275
assert.strictEqual(spawn.calls.length, 0);
@@ -415,11 +421,11 @@ suite('OpencodeServeManager / health checks', () => {
415421
assert.strictEqual(await internals(manager).isHealthy(`http://127.0.0.1:${backend.port}`, undefined), true);
416422
});
417423

418-
test('isHealthy returns true on HTTP 401 (auth required = server alive)', async () => {
424+
test('isHealthy returns false on HTTP 401 (auth required but wrong credentials)', async () => {
419425
backend = await mockBackend({ status: 401 });
420426
manager = new TestableOpencodeServeManager(configuration(), new NullLogService());
421427

422-
assert.strictEqual(await internals(manager).isHealthy(`http://127.0.0.1:${backend.port}`, undefined), true);
428+
assert.strictEqual(await internals(manager).isHealthy(`http://127.0.0.1:${backend.port}`, undefined), false);
423429
});
424430

425431
test('isHealthy returns false on HTTP 500', async () => {

0 commit comments

Comments
 (0)