fix: address bot review feedback on #35862

- Remove dead 'return false' in runServiceStart (Greptile)
- Include stack trace in run-loop crash guard error log (Greptile)
- Only catch startup errors on subsequent restarts, not initial start (Codex P1)
- Add JSDoc note about env var false positive edge case (Codex P1)
This commit is contained in:
merlin
2026-03-08 00:02:49 +08:00
committed by Vincent Koc
parent ce44b366db
commit 32f6501dbd
2 changed files with 19 additions and 8 deletions

View File

@@ -111,7 +111,11 @@ async function resolveServiceLoadedOrFail(params: {
/**
* Best-effort config validation. Returns a string describing the issues if
* config exists and is invalid, or null if config is valid/missing/unreadable.
* (#35862)
*
* Note: This reads the config file snapshot in the current CLI environment.
* Configs using env vars only available in the service context (launchd/systemd)
* may produce false positives, but the check is intentionally best-effort —
* a false positive here is safer than a crash on startup. (#35862)
*/
async function getConfigValidationError(): Promise<string | null> {
try {
@@ -214,7 +218,7 @@ export async function runServiceStart(params: {
fail(
`${params.serviceNoun} aborted: config is invalid.\n${configError}\nFix the config and retry, or run "openclaw doctor" to repair.`,
);
return false;
return;
}
}

View File

@@ -190,20 +190,27 @@ export async function runGatewayLoop(params: {
// Keep process alive; SIGUSR1 triggers an in-process restart (no supervisor required).
// SIGTERM/SIGINT still exit after a graceful shutdown.
let isFirstStart = true;
// eslint-disable-next-line no-constant-condition
while (true) {
onIteration();
try {
server = await params.start();
isFirstStart = false;
} catch (err) {
// If startup fails (e.g., invalid config after a config-triggered
// restart), keep the process alive and wait for the next SIGUSR1
// instead of crashing. A crash here would respawn a new process that
// loses macOS Full Disk Access (TCC permissions are PID-bound). (#35862)
// On initial startup, let the error propagate so the outer handler
// can report "Gateway failed to start" and exit non-zero. Only
// swallow errors on subsequent in-process restarts to keep the
// process alive (a crash would lose macOS TCC permissions). (#35862)
if (isFirstStart) {
throw err;
}
server = null;
const errMsg = err instanceof Error ? err.message : String(err);
const errStack = err instanceof Error && err.stack ? `\n${err.stack}` : "";
gatewayLog.error(
`gateway startup failed: ${err instanceof Error ? err.message : String(err)}. ` +
"Process will stay alive; fix the issue and restart.",
`gateway startup failed: ${errMsg}. ` +
`Process will stay alive; fix the issue and restart.${errStack}`,
);
}
await new Promise<void>((resolve) => {