mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
* fix(daemon): restore startServer Promise contract — return url / { url, server }
PR #258 ("Standardize agent communication via stdin and remove
Windows-specific shims logic") removed the Promise wrap around
`startServer`'s `app.listen()` call. Two regressions shipped to main as
a result:
1. **`returnServer: true` returns just `server`, not `{ url, server }`.**
Both `apps/daemon/sidecar/server.ts:65` and
`apps/daemon/tests/version-route.test.ts:10` cast the return value to
`{ url, server }` and read `started.url`. Post-#258 those reads land
on `undefined`, which becomes `"undefined/api/version"` URLs at
runtime — the sidecar can no longer report its bound port to the
parent process, and `version-route.test.ts` fails with
`ERR_INVALID_URL`.
2. **`returnServer: false` returns nothing.** `apps/daemon/src/cli.ts:77`
calls `startServer({ port }).then(url => console.log(\`[od] listening on \${url}\`))`
— post-#258 that prints `[od] listening on undefined`, and the
subsequent `spawn(opener, [url], …)` to open the browser also gets
`undefined`. The daemon still binds, but the two paths that *consume*
the URL are silently broken.
Both stem from the same change: the new code synchronously kicks off
`app.listen` and returns either the server object or `undefined` before
`listen`'s callback fires, so neither the resolved port nor the URL
string ever reach the caller.
This restores the pre-#258 Promise wrap, which already had the right
shape for all three call sites:
```ts
return await new Promise((resolve) => {
const server = app.listen(port, () => {
const url = `http://127.0.0.1:${server.address().port}`;
resolve(returnServer ? { url, server } : url);
});
});
```
Behavior contract restored:
| `returnServer` | resolves with |
|---|---|
| `false` (default) | `string` — `http://127.0.0.1:<port>` |
| `true` | `{ url: string, server: http.Server }` |
The Promise resolution waits for `listen`'s callback so `port=0`
(ephemeral) callers always see the actual bound port, never the
placeholder.
Test coverage: `tests/version-route.test.ts` already exercises the
`{ url, server }` path via `await fetch(\`\${baseUrl}/api/version\`)`.
After this fix the suite is back to 15 files / 225 tests green
(2 of those — both in `version-route` — failed against current main).
Verification:
- `pnpm --filter @open-design/daemon test` — 15 files / 225 tests pass
- `tsc -p apps/daemon/tsconfig.json --noEmit` — clean
- `tsc -p apps/daemon/tsconfig.tests.json --noEmit` — clean (was failing
on the `as { url, server }` cast before)
* fix(daemon): reject startServer Promise on bind failure / null address
Folds in the two non-blocking improvements @lefarcen flagged on the
review of the parent commit:
1. **Explicit `error` event listener.** `app.listen` throws synchronously
for some failure modes (e.g. invalid port) and emits an `error` event
for others (EADDRINUSE on certain Node versions, EACCES, EADDRNOTAVAIL).
Without an `error` listener the returned Promise would hang forever
on the event-emitting paths instead of surfacing the failure to the
caller.
2. **Null-`address()` guard.** `server.address()` typing allows
`string | AddressInfo | null`. For TCP listeners it's always
AddressInfo, but if a future code path ever returns null (or a Unix
socket path string), the previous fallback `port` would still be
`0` for ephemeral-port callers and quietly produce a
`http://127.0.0.1:0` URL that fetches would fail on. Reject the
Promise instead with an explicit diagnostic.
Both keep the success path identical — `version-route.test.ts` still
exercises the same `{ url, server }` contract on `port: 0` and passes
unchanged. 15/15 daemon test files / 225/225 tests pass.
Refs review feedback on #268 (P2 suggestions, non-blocking).
* fix(daemon): drop a93246d workaround for restored startServer contract
After rebasing onto current `main`, this PR's restored
`{ url, server }` return shape from `startServer` collides with the
workaround applied in `a93246d` (PR #271), which had patched the two
consumers — `apps/daemon/sidecar/server.ts` and
`apps/daemon/tests/version-route.test.ts` — to call `started.address()`
on a bare `Server` handle.
Revert both consumers to the pre-`a93246d` `{ url, server }` shape so
they line up with the contract restored in `5c2f520`. With this:
- `pnpm --filter @open-design/daemon test` — 246/246 pass
- `pnpm --filter @open-design/daemon build` — tsc clean for both
tsconfig.json and tsconfig.sidecar.json
- `apps/daemon/src/cli.ts` (which expects a URL string and was
printing "listening on undefined" on `main`) is now correct without
any further change.
* fix(i18n): add missing chat.comments.* keys to hu.ts
PR #288 added Hungarian without the 12 `chat.comments.*` keys that
PR #284 introduced — the two landed in parallel and only en + the
older locales got both. Without these, `pnpm --filter @open-design/web
typecheck` errors on `src/i18n/locales/hu.ts(3,14): TS2740 ... missing
the following properties from type 'Dict': 'chat.comments.attached',
'chat.comments.emptyAttached', 'chat.comments.saved', and 9 more`,
which blocks every PR's CI.
Drive-by unblock with English fallback strings — proper Hungarian
translations should follow in a separate i18n pass.
---------
Co-authored-by: lefarcen <935902669@qq.com>
129 lines
3.5 KiB
TypeScript
129 lines
3.5 KiB
TypeScript
import type { Server } from "node:http";
|
|
|
|
import {
|
|
SIDECAR_ENV,
|
|
SIDECAR_MESSAGES,
|
|
normalizeDaemonSidecarMessage,
|
|
type DaemonStatusSnapshot,
|
|
type SidecarStamp,
|
|
} from "@open-design/sidecar-proto";
|
|
import {
|
|
createJsonIpcServer,
|
|
type JsonIpcServerHandle,
|
|
type SidecarRuntimeContext,
|
|
} from "@open-design/sidecar";
|
|
|
|
import { startServer } from "../src/server.js";
|
|
|
|
const DAEMON_PORT_ENV = SIDECAR_ENV.DAEMON_PORT;
|
|
const TOOLS_DEV_PARENT_PID_ENV = SIDECAR_ENV.TOOLS_DEV_PARENT_PID;
|
|
|
|
export type DaemonSidecarHandle = {
|
|
status(): Promise<DaemonStatusSnapshot>;
|
|
stop(): Promise<void>;
|
|
waitUntilStopped(): Promise<void>;
|
|
};
|
|
|
|
function parsePort(value: string | undefined): number {
|
|
if (value == null || value.trim().length === 0) return 0;
|
|
const port = Number(value);
|
|
if (!Number.isInteger(port) || port < 0 || port > 65535) {
|
|
throw new Error(`${DAEMON_PORT_ENV} must be an integer between 0 and 65535`);
|
|
}
|
|
return port;
|
|
}
|
|
|
|
async function closeHttpServer(server: Server): Promise<void> {
|
|
if (!server.listening) return;
|
|
await new Promise<void>((resolveClose, rejectClose) => {
|
|
server.close((error) => (error == null ? resolveClose() : rejectClose(error)));
|
|
});
|
|
}
|
|
|
|
function isProcessAlive(pid: number): boolean {
|
|
try {
|
|
process.kill(pid, 0);
|
|
return true;
|
|
} catch {
|
|
return false;
|
|
}
|
|
}
|
|
|
|
function attachParentMonitor(stop: () => Promise<void>): void {
|
|
const parentPid = Number(process.env[TOOLS_DEV_PARENT_PID_ENV]);
|
|
if (!Number.isInteger(parentPid) || parentPid <= 0) return;
|
|
|
|
const timer = setInterval(() => {
|
|
if (isProcessAlive(parentPid)) return;
|
|
clearInterval(timer);
|
|
void stop().finally(() => process.exit(0));
|
|
}, 1000);
|
|
timer.unref();
|
|
}
|
|
|
|
export async function startDaemonSidecar(runtime: SidecarRuntimeContext<SidecarStamp>): Promise<DaemonSidecarHandle> {
|
|
const started = await startServer({ port: parsePort(process.env[DAEMON_PORT_ENV]), returnServer: true }) as
|
|
| string
|
|
| { server: Server; url: string };
|
|
if (typeof started === "string") {
|
|
throw new Error("daemon startServer did not return a server handle");
|
|
}
|
|
const serverHandle = started;
|
|
|
|
const state: DaemonStatusSnapshot = {
|
|
pid: process.pid,
|
|
state: "running",
|
|
updatedAt: new Date().toISOString(),
|
|
url: serverHandle.url,
|
|
};
|
|
let ipcServer: JsonIpcServerHandle | null = null;
|
|
let stopped = false;
|
|
let resolveStopped!: () => void;
|
|
const stoppedPromise = new Promise<void>((resolveStop) => {
|
|
resolveStopped = resolveStop;
|
|
});
|
|
|
|
async function stop(): Promise<void> {
|
|
if (stopped) return;
|
|
stopped = true;
|
|
state.state = "stopped";
|
|
state.updatedAt = new Date().toISOString();
|
|
await ipcServer?.close().catch(() => undefined);
|
|
await closeHttpServer(serverHandle.server).catch(() => undefined);
|
|
resolveStopped();
|
|
}
|
|
|
|
attachParentMonitor(stop);
|
|
|
|
ipcServer = await createJsonIpcServer({
|
|
socketPath: runtime.ipc,
|
|
handler: async (message: unknown) => {
|
|
const request = normalizeDaemonSidecarMessage(message);
|
|
switch (request.type) {
|
|
case SIDECAR_MESSAGES.STATUS:
|
|
return { ...state };
|
|
case SIDECAR_MESSAGES.SHUTDOWN:
|
|
setImmediate(() => {
|
|
void stop().finally(() => process.exit(0));
|
|
});
|
|
return { accepted: true };
|
|
}
|
|
},
|
|
});
|
|
|
|
for (const signal of ["SIGINT", "SIGTERM"] as const) {
|
|
process.on(signal, () => {
|
|
void stop().finally(() => process.exit(0));
|
|
});
|
|
}
|
|
|
|
return {
|
|
async status() {
|
|
return { ...state };
|
|
},
|
|
stop,
|
|
waitUntilStopped() {
|
|
return stoppedPromise;
|
|
},
|
|
};
|
|
}
|