Skip to content

fix(desktop): don't nuke host services on app update #3620

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 1 commit into from
Apr 21, 2026
+33 −16
Merged

fix(desktop): don't nuke host services on app update #3620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions apps/desktop/src/main/host-service/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
resolveTerminalBaseEnv,
} from "@superset/host-service/terminal-env";
import { connectRelay } from "@superset/host-service/tunnel";
import { removeManifest, writeManifest } from "main/lib/host-service-manifest";
import { writeManifest } from "main/lib/host-service-manifest";
import { env } from "./env";

async function main(): Promise<void> {
Expand Down Expand Up @@ -81,10 +81,8 @@ async function main(): Promise<void> {
);
injectWebSocket(server);

// Manifest lifecycle belongs to the coordinator, not the child.
const shutdown = () => {
if (env.ORGANIZATION_ID) {
removeManifest(env.ORGANIZATION_ID);
}
server.close();
process.exit(0);
};
Expand Down
43 changes: 31 additions & 12 deletions apps/desktop/src/main/lib/host-service-coordinator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ const HEALTH_POLL_INTERVAL = 200;
const HEALTH_POLL_TIMEOUT = 10_000;
const ADOPTED_LIVENESS_INTERVAL = 5_000;

function openLogFile(organizationId: string): number {
try {
const dir = manifestDir(organizationId);
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
}
return fs.openSync(path.join(dir, "host-service.log"), "a", 0o600);
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unbounded log file growth

host-service.log is opened in append mode ("a") with no size cap or rotation. Over time — especially if the host-service restarts frequently or is verbose — this file will grow without bound.

Consider either:

  • Rotating at a fixed size (e.g., rename to .log.1 when it exceeds a few MBs), or
  • Truncating on each new spawn by opening with "w" instead of "a" (acceptable since each spawn is a fresh process whose history can be considered independent).
Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/host-service-coordinator.ts
Line: 64

Comment:
**Unbounded log file growth**

`host-service.log` is opened in append mode (`"a"`) with no size cap or rotation. Over time — especially if the host-service restarts frequently or is verbose — this file will grow without bound.

Consider either:
- Rotating at a fixed size (e.g., rename to `.log.1` when it exceeds a few MBs), or
- Truncating on each new spawn by opening with `"w"` instead of `"a"` (acceptable since each spawn is a fresh process whose history can be considered independent).

How can I resolve this? If you propose a fix, please make it concise.

} catch {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Apr 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Empty catch {} silently swallows log-open failures — if this path is hit, all child stdout/stderr is discarded (stdio: "ignore" fallback) with zero diagnostic trace. Add at minimum a console.warn with the organization ID and error so operators can tell why logs are missing.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/lib/host-service-coordinator.ts, line 65:

<comment>Empty `catch {}` silently swallows log-open failures — if this path is hit, all child stdout/stderr is discarded (`stdio: "ignore"` fallback) with zero diagnostic trace. Add at minimum a `console.warn` with the organization ID and error so operators can tell why logs are missing.</comment>

<file context>
@@ -55,6 +55,18 @@ const HEALTH_POLL_INTERVAL = 200;
+			fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
+		}
+		return fs.openSync(path.join(dir, "host-service.log"), "a", 0o600);
+	} catch {
+		return -1;
+	}
</file context>
Suggested change
} catch {
} catch (err) {
console.warn(
`[host-service:${organizationId}] Failed to open log file, child output will be discarded:`,
err,
);
Fix with Cubic

return -1;
}
}
Comment on lines +58 to +68
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent log-open failure drops all child output

When openLogFile fails (e.g., permissions error, unexpected filesystem issue), it returns -1 with no warning. The spawn call then falls back to stdio: "ignore", silently discarding all stdout/stderr from the host-service child. Operators will have no indication that logs are missing.

Consider emitting at least a console.warn when the file cannot be opened:

Suggested change
function openLogFile(organizationId: string): number {
try {
const dir = manifestDir(organizationId);
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
}
return fs.openSync(path.join(dir, "host-service.log"), "a", 0o600);
} catch {
return -1;
}
}
function openLogFile(organizationId: string): number {
try {
const dir = manifestDir(organizationId);
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
}
return fs.openSync(path.join(dir, "host-service.log"), "a", 0o600);
} catch (err) {
console.warn(
`[host-service:${organizationId}] Failed to open log file, child output will be discarded:`,
err,
);
return -1;
}
}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/main/lib/host-service-coordinator.ts
Line: 58-68

Comment:
**Silent log-open failure drops all child output**

When `openLogFile` fails (e.g., permissions error, unexpected filesystem issue), it returns `-1` with no warning. The spawn call then falls back to `stdio: "ignore"`, silently discarding all stdout/stderr from the host-service child. Operators will have no indication that logs are missing.

Consider emitting at least a `console.warn` when the file cannot be opened:

```suggestion
function openLogFile(organizationId: string): number {
	try {
		const dir = manifestDir(organizationId);
		if (!fs.existsSync(dir)) {
			fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
		}
		return fs.openSync(path.join(dir, "host-service.log"), "a", 0o600);
	} catch (err) {
		console.warn(
			`[host-service:${organizationId}] Failed to open log file, child output will be discarded:`,
			err,
		);
		return -1;
	}
}
```

How can I resolve this? If you propose a fix, please make it concise.


async function findFreePort(): Promise<number> {
return new Promise((resolve, reject) => {
const server = createServer();
Expand Down Expand Up @@ -402,10 +414,25 @@ export class HostServiceCoordinator extends EventEmitter {
this.emitStatus(organizationId, "starting", null);

const env = await this.buildEnv(organizationId, port, secret, config);
const child = childProcess.spawn(process.execPath, [this.scriptPath], {
stdio: ["ignore", "pipe", "pipe"],
env,
});

// Detached + file-backed stdio so the child outlives the parent's process
// group (Squirrel.Mac SIGTERMs it during updates) and doesn't depend on
// parent-held stdout/stderr pipes.
const logFd = openLogFile(organizationId);
let child: childProcess.ChildProcess;
try {
child = childProcess.spawn(process.execPath, [this.scriptPath], {
detached: true,
stdio: logFd >= 0 ? ["ignore", logFd, logFd] : "ignore",
env,
});
} finally {
if (logFd >= 0) {
try {
fs.closeSync(logFd);
} catch {}
}
}

const childPid = child.pid;
if (!childPid) {
Expand All @@ -415,14 +442,6 @@ export class HostServiceCoordinator extends EventEmitter {

instance.pid = childPid;

child.stdout?.on("data", (data: Buffer) => {
console.log(`[host-service:${organizationId}] ${data.toString().trim()}`);
});
child.stderr?.on("data", (data: Buffer) => {
console.error(
`[host-service:${organizationId}] ${data.toString().trim()}`,
);
});
child.on("exit", (code) => {
console.log(`[host-service:${organizationId}] exited with code ${code}`);
const current = this.instances.get(organizationId);
Expand Down
Loading