Skip to content

fix(release): bound the Windows process probe so its poll deadlines hold #3241

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

Closed
Joob1n wants to merge 11 commits into from
Closed

fix(release): bound the Windows process probe so its poll deadlines hold #3241

Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7f61f1e
fix(release): read the CDP port Chromium bound instead of reserving one
Joob1n Aug 19, 2026
3ecb005
fix(release): delete the previous instance's DevToolsActivePort first
Joob1n Aug 19, 2026
6532f7a
fix(release): give CDP discovery the time a cold runner actually needs
Joob1n Aug 19, 2026
090158d
chore: restart CI — test_workspaces hung past three hours
Joob1n Aug 19, 2026
8a4a8d6
Merge remote-tracking branch 'origin/main' into fix/cdp-smoke-port-race
Joob1n Aug 20, 2026
119d9da
fix(release): bound each CDP poll attempt so the deadline stays honest
Joob1n Aug 20, 2026
55ce142
fix(release): bound the Windows process probe so its poll deadlines hold
Joob1n Aug 20, 2026
af023a7
fix(release): let the process-poll deadline outrank a stalled probe
Joob1n Aug 20, 2026
5543fba
Merge remote-tracking branch 'origin/main' into fix/cdp-smoke-port-race
Joob1n Aug 20, 2026
7eeb843
fix(release): size the process probe to its budget and say what was o…
Joob1n Aug 20, 2026
c611cfd
refactor(release): hand CDP port discovery to #3265
Joob1n Aug 20, 2026
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
94 changes: 58 additions & 36 deletions scripts/verify-packaged-app.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { spawn } from 'node:child_process';
import { createHash } from 'node:crypto';
import { createReadStream } from 'node:fs';
import { access, mkdir } from 'node:fs/promises';
import { createServer } from 'node:net';
import { access, mkdir, readFile, rm } from 'node:fs/promises';
import { join } from 'node:path';

// `timeoutMs` is opt-in, for the commands that have actually hung: node-pty
Expand Down Expand Up @@ -74,53 +73,65 @@ export async function assertMissing(path) {
throw new Error(`Forbidden release resource exists: ${path}`);
}

async function reserveTcpPort() {
const server = createServer();
await new Promise((resolvePromise, reject) => {
server.once('error', reject);
server.listen(0, '127.0.0.1', resolvePromise);
});
const address = server.address();
if (!address || typeof address === 'string') {
server.close();
throw new Error('Could not reserve a CDP port.');
}
await new Promise((resolvePromise, reject) => {
server.close((error) => (error ? reject(error) : resolvePromise()));
});
return address.port;
}

function delay(milliseconds) {
return new Promise((resolvePromise) => {
setTimeout(resolvePromise, milliseconds);
});
}

async function findRendererTarget(port, child) {
const deadline = Date.now() + 30_000;
/**
* The port Chromium actually bound, read from the DevToolsActivePort file it
* writes into the user-data directory. The verifier used to reserve a port,
* release it, and hand the number to Electron — leaving a window in which
* anything else on the runner could take it, and a timeout log that could not
* say whether the app was even listening where the poll looked (issue #3196).
*/
async function readDevToolsPort(userDataDirectory) {
try {
const content = await readFile(join(userDataDirectory, 'DevToolsActivePort'), 'utf8');
const port = Number.parseInt(content.split('\n')[0] ?? '', 10);
return Number.isInteger(port) && port > 0 ? port : null;
} catch {
return null;
}
}

async function findRendererTarget(userDataDirectory, child) {
const startedAt = Date.now();
const deadline = startedAt + 30_000;
let port = null;
let lastError;
while (Date.now() < deadline) {
if (child.exitCode !== null) {
throw new Error(`Packaged Maka exited before its renderer was ready.`);
}
try {
const response = await fetch(`http://127.0.0.1:${port}/json/list`);
if (response.ok) {
const targets = await response.json();
const page = targets.find(
(target) => target.type === 'page' && target.webSocketDebuggerUrl,
);
if (page) return page;
port ??= await readDevToolsPort(userDataDirectory);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Outdated

@Astro-Han Astro-Han Aug 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] Make findRendererTarget enforce the freshness it now depends on, rather than trusting each caller to have deleted the file. port ??= caches the first value read and never re-reads, so a DevToolsActivePort left by a previous instance is latched for the entire 120-second window and the run ends with port <dead> did not answer — which reads as a hung app rather than as a stale file, the precise misdiagnosis this PR exists to eliminate. All three current call sites do delete the file first, so nothing is broken today; what is fragile is that the requirement lives only in three separate comments, on an exported function whose signature gives no hint of it. The fourth caller will not know. Passing the spawn timestamp and ignoring a port file older than it, or having this function do the deletion as part of a startRendererTarget(userDataDirectory) that owns both halves, puts the invariant where it cannot be forgotten. Inference about the future caller; the caching behaviour itself is confirmed by reading the code at this head.

if (port !== null) {
try {
const response = await fetch(`http://127.0.0.1:${port}/json/list`);
if (response.ok) {
const targets = await response.json();

@coderabbitai coderabbitai Bot Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- file outline ---'
ast-grep outline scripts/verify-packaged-app.mjs --view expanded
printf '%s\n' '--- relevant source ---'
sed -n '1,145p' scripts/verify-packaged-app.mjs
printf '%s\n' '--- package/runtime declarations ---'
rg -n --glob 'package.json' --glob '*.json' '"(engines|node|electron)"' . | head -80
printf '%s\n' '--- usages and tests ---'
rg -n 'findRendererTarget|DevToolsActivePort|verify-packaged-app|AbortSignal|fetch\\(' scripts test tests package.json .github 2>/dev/null | head -200

Repository: maka-agent/maka-agent

Length of output: 13651

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- runtime and workflow versions ---'
node --version
rg -n -C 3 'node-version|setup-node|verify-packaged-app|smokePackagedRenderer' .github scripts package.json 2>/dev/null | head -160
printf '%s\n' '--- launch and cleanup context ---'
sed -n '257,310p' scripts/verify-packaged-app.mjs
sed -n '186,245p' scripts/verify-packaged-app.mjs
printf '%s\n' '--- standalone cancellation probe ---'
node - <<'JS'
const events = [];
const signal = AbortSignal.timeout(30);
const stalledFetch = (_url, { signal }) =>
  new Promise((resolve, reject) => {
    const onAbort = () => {
      events.push(`aborted:${signal.reason?.name}`);
      reject(signal.reason);
    };
    if (signal.aborted) return onAbort();
    signal.addEventListener('abort', onAbort, { once: true });
  });

globalThis.fetch = stalledFetch;
const started = Date.now();
try {
  await fetch('http://127.0.0.1:1/json/list', { signal });
} catch (error) {
  console.log(JSON.stringify({
    apiAvailable: typeof AbortSignal.timeout === 'function',
    elapsedMs: Date.now() - started,
    errorName: error?.name,
    events,
  }));
}
JS

Repository: maka-agent/maka-agent

Length of output: 16284

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- package engine ---'
sed -n '1,18p' package.json
printf '%s\n' '--- smoke renderer callers and working-directory setup ---'
rg -n -C 5 'smokePackagedRenderer\\(|workingDirectory:|mkdtemp|temporaryDirectory|rm\\(' scripts .github | head -240
printf '%s\n' '--- deterministic abort probe ---'
node - <<'JS'
(async () => {
  const controller = new AbortController();
  const keepAlive = setTimeout(() => controller.abort(new DOMException('deadline', 'TimeoutError')), 30);
  const events = [];
  const stalledFetch = (_url, { signal }) =>
    new Promise((resolve, reject) => {
      const onAbort = () => {
        events.push(`fetch-aborted:${signal.reason?.name}`);
        reject(signal.reason);
      };
      if (signal.aborted) return onAbort();
      signal.addEventListener('abort', onAbort, { once: true });
    });

  globalThis.fetch = stalledFetch;
  const started = Date.now();
  try {
    await fetch('http://127.0.0.1:1/json/list', { signal: controller.signal });
  } catch (error) {
    console.log(JSON.stringify({
      apiAvailable: typeof AbortSignal.timeout === 'function',
      elapsedMs: Date.now() - started,
      errorName: error?.name,
      events,
    }));
  } finally {
    clearTimeout(keepAlive);
  }
})();
JS

Repository: maka-agent/maka-agent

Length of output: 911

Fix now: bound each CDP request by the remaining deadline.

Pass a deadline-based AbortSignal to fetch(); it also cancels response.json() body reads. Skip the 250 ms delay when the deadline expires so the timeout and finally cleanup run promptly.

Source: Path instructions

@Astro-Han Astro-Han Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmed on exact head 090158d8d; I agree with this finding and calibrate it as P2. The outer 120-second loop does not bound an individual fetch() or response.json(). A peer that accepts the TCP connection and then never responds can therefore pin the verifier until the workflow timeout, bypassing the intended diagnostic deadline and delaying stopChild(). The smallest fix is to pass an AbortSignal based on the remaining deadline to the request/body read and add a hanging-server regression. I am not opening a duplicate inline thread.

中文说明

已在当前 head 复现并确认，校准为 P2。外层 120 秒循环并不能限制单次 fetch/response.json；如果对端建立连接后不响应，验证会一直挂到 workflow 超时，finally/stopChild 也无法及时执行。请用剩余 deadline 创建 AbortSignal，并补“连接成功但不响应”的测试。

@coderabbitai coderabbitai Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@Astro-Han, agreed. The finding remains valid and open.

The fix should derive an AbortSignal from the remaining findRendererTarget() deadline for both fetch() and response.json(). The regression should use a server that accepts the connection but does not send a response. It should verify that the verifier reaches its diagnostic timeout and executes cleanup without waiting for the workflow timeout.

You are interacting with an AI system.

const page = targets.find(
(target) => target.type === 'page' && target.webSocketDebuggerUrl,
);
if (page) return page;
}
} catch (error) {
lastError = error;
}
} catch (error) {
lastError = error;
}
await delay(250);
}
// Say which of the three causes this is, so classifying the failure does not
// take a re-run: no port file means the browser never opened an endpoint;
// a port plus fetch errors means it bound somewhere the poll could reach
// but never answered.
const elapsedSeconds = Math.round((Date.now() - startedAt) / 1000);
throw new Error(
`Packaged Maka renderer did not expose CDP within 30 seconds${
lastError ? `: ${lastError.message}` : ''
`Packaged Maka renderer did not expose CDP within ${elapsedSeconds} seconds: ${
port === null
? 'DevToolsActivePort was never written under the user-data directory'
: `polled port ${port} from DevToolsActivePort${lastError ? `; last error: ${lastError.message}` : ''}`
}.`,
);
}
Expand Down Expand Up @@ -244,17 +255,28 @@ export function isolatedUserEnv(homeDirectory, { temporaryDirectory = homeDirect
}

export async function smokePackagedRenderer(executable, { workingDirectory } = {}) {
const port = await reserveTcpPort();
const home = join(workingDirectory, 'home');
const userData = join(workingDirectory, 'user-data');
const userEnv = isolatedUserEnv(home);
await mkdir(home, { recursive: true });
await mkdir(userData, { recursive: true });
// Chromium removes DevToolsActivePort only on a clean exit, and the
// upgrade-lifecycle check reuses one user-data directory across two app
// versions with a SIGKILL between them — so a file found here can belong to
// the previous instance, pointing the poll at a port nothing listens on
// anymore. Deleting it first means whatever appears was written by this
// child. This is what the first run of the port-file diagnostics caught:
// the poll read a bound-looking port and fetch failed for the full window.
await rm(join(userData, 'DevToolsActivePort'), { force: true });
await mkdir(userEnv.APPDATA, { recursive: true });
await mkdir(userEnv.LOCALAPPDATA, { recursive: true });
const child = spawn(
executable,
[`--remote-debugging-port=${port}`, `--user-data-dir=${userData}`, '--enable-logging=stderr'],
// Port 0: Chromium binds a free port itself and records it in the
// user-data directory's DevToolsActivePort file, which is where the poll
// reads it back — no reserve-then-release window for another process on
// the runner to take the number first.
['--remote-debugging-port=0', `--user-data-dir=${userData}`, '--enable-logging=stderr'],
{
cwd: workingDirectory,
env: {
Expand All @@ -272,7 +294,7 @@ export async function smokePackagedRenderer(executable, { workingDirectory } = {
});

try {
const target = await findRendererTarget(port, child);
const target = await findRendererTarget(userData, child);
const deadline = Date.now() + 30_000;
let rendererState;
while (Date.now() < deadline) {
Expand Down
Loading