fix(ci): stabilize TUI PTY proof ownership (#119420)

* fix(ci): stabilize TUI PTY proof ownership

Punchcard-Session: cobalt-brook-meadow-m9

* fix(test): align TUI PTY event typings

Punchcard-Session: cobalt-brook-meadow-m9
This commit is contained in:
Vincent Koc
2026-08-05 11:08:56 +08:00
committed by GitHub
parent 23c7981a73
commit 17ef7771a6
7 changed files with 276 additions and 35 deletions
+6 -3
View File
@@ -1291,15 +1291,18 @@ jobs:
wait_checks
fi
# The built local TUI lane owns a real Gateway fixture. Keep it
# exclusive so sibling artifact shards cannot starve its requests.
# The dedicated Node shard owns the full built-CLI PTY suite. Keep
# only the local roundtrip and Gateway-connect artifact canaries here.
if [ "$RUN_TUI_PTY" = "true" ]; then
start_check "tui-pty" env \
NODE_OPTIONS=--max-old-space-size=8192 \
OPENCLAW_TUI_PTY_INCLUDE_LOCAL=1 \
OPENCLAW_TUI_PTY_USE_BUILT_CLI=1 \
OPENCLAW_VITEST_MAX_WORKERS=2 \
node scripts/run-vitest.mjs run --config test/vitest/vitest.tui-pty.config.ts
node scripts/run-vitest.mjs run \
--config test/vitest/vitest.tui-pty.config.ts \
src/tui/tui-pty-local.e2e.test.ts \
--testNamePattern "launches openclaw (chat as local mode|tui against a real Gateway) through a real PTY"
wait_checks
fi
+2 -2
View File
@@ -101,7 +101,7 @@ Separate iOS and macOS Periphery workflows enforce a zero-findings dead-code pol
- **CI workflow edits** validate the Node CI graph, workflow linting, and the Windows lane (`ci.yml` executes it), but do not force iOS, Android, or macOS native builds by themselves; those platform lanes stay scoped to platform source changes.
- **Workflow Sanity** runs `actionlint`, `zizmor` over all workflow YAML files, the composite-action interpolation guard, and the conflict-marker guard. The PR-scoped `security-fast` job also runs `zizmor` over changed workflow files so workflow security findings fail early in the main CI graph.
- **Docs on `main` pushes** are checked by the standalone `Docs` workflow with the same ClawHub docs mirror used by CI, so mixed code+docs pushes do not also queue the CI `check-docs` shard. Pull requests and manual CI still run `check-docs` from CI when docs changed.
- **TUI PTY** splits by runtime ownership. The Linux Node shard runs the deterministic source-level `TuiBackend` fixture lane. The `build-artifacts` job reruns `test/vitest/vitest.tui-pty.config.ts` with `OPENCLAW_TUI_PTY_INCLUDE_LOCAL=1` and `OPENCLAW_TUI_PTY_USE_BUILT_CLI=1`, so the slower `tui --local` smoke exercises the exact-head built CLI while mocking only the external model endpoint. That built local lane runs after the other artifact checks because it owns a real Gateway fixture whose request deadlines must not compete with sibling shards.
- **TUI PTY** splits by proof ownership. The dedicated `core-runtime-tui-pty` Node shard owns the full real-backend suite against the exact-head built CLI. The `build-artifacts` job keeps only a local model roundtrip and a real Gateway connection canary, so every artifact boundary proves the built launcher without duplicating the full serial suite inside the build job.
- **CI routing-only edits, the small set of core-test fixtures the fast task runs directly, and narrow plugin contract helper edits** use a fast Node-only manifest path: `preflight`, `security-fast`, and only the fast lanes the change touches — a single `checks-fast-core` CI-routing task, the two plugin contract shards, or both. That path skips build artifacts, Node 22 compatibility, channel contracts, full core shards, bundled-plugin shards, and additional guard matrices.
- **Windows Node checks** are scoped to Windows-specific process/path wrappers, npm/pnpm/UI runner helpers, package manager config, and the CI workflow surfaces that execute that lane; unrelated source, plugin, install-smoke, and test-only changes stay on the Linux Node lanes.
@@ -121,7 +121,7 @@ The slowest Node test families are split or balanced so each job stays small wit
- The build-artifact job also persists content-fingerprinted `build-all` step outputs. CI's self-built plugin SDK declarations hash the complete repository-owned TypeScript/JSON source graph, exclude installed and generated directories, and restore both flat declarations and package bridges after `tsdown` clears `dist`. Documentation, workflow, plugin, and other changes outside that graph can reuse the declaration snapshot; source changes rebuild it before the export gate runs.
- Full declaration builds split `tsdown` into AI, workspace-package, and unified groups. Each group caches declarations only, then still rebuilds runtime JavaScript before restoring those declarations. Core or plugin changes therefore invalidate only the large unified graph, while workspace-package changes conservatively invalidate every dependent declaration group. Public full builds generally use an immutable Actions cache; coarse restore keys seed partial changes, per-group content fingerprints reject stale data, and GitHub's cache quota evicts old generations. The weekly Node 22 lane instead publishes a 14-day artifact after successful `main` runs and restores only artifacts whose immutable producer identity resolves to that workflow on `main`, avoiding quota churn without allowing PR code to write a shared cache. Private-QA declarations are never persisted in Actions caches because cache namespaces are not confidentiality boundaries.
- `check-additional-*` stripes the supplemental boundary guard list (`scripts/run-additional-boundary-checks.mjs`) into one prompt-heavy shard (`check-additional-boundaries-a`, which includes the Codex prompt snapshot drift check) and one combined shard for the remaining stripes (`check-additional-boundaries-bcd`), each running independent guards concurrently and printing per-check timings. Package-boundary compile/canary work stays together, and runtime topology architecture runs separately from the gateway watch coverage embedded in `build-artifacts`.
- On the 32-vCPU self-hosted build runner, Gateway watch, channel tests, and the core support-boundary shard start together inside `build-artifacts` after `dist/` and `dist-runtime/` are already built. GitHub-hosted fallback runs keep Gateway watch serial so low-core contention cannot consume its readiness deadline. Both paths then run built local TUI PTY alone so its real Gateway fixture has exclusive process and event-loop capacity.
- On the 32-vCPU self-hosted build runner, Gateway watch, channel tests, and the core support-boundary shard start together inside `build-artifacts` after `dist/` and `dist-runtime/` are already built. GitHub-hosted fallback runs keep Gateway watch serial so low-core contention cannot consume its readiness deadline. Both paths then run the two built TUI PTY artifact canaries alone; the dedicated Node shard owns the full serial suite.
Once admitted, canonical Linux CI permits up to 28 concurrent Node test jobs and
12 for the smaller fast/check lanes; Windows and Android stay at two because
+95 -1
View File
@@ -1,12 +1,106 @@
import { describe, expect, it } from "vitest";
import { createDeferred } from "../test-utils/deferred.js";
import { cleanupStartedFixture, createFreshSession } from "./tui-pty-local-test-support.js";
import {
cleanupStartedFixture,
createChatTerminalObserver,
createFreshSession,
registerIdempotentCleanup,
} from "./tui-pty-local-test-support.js";
import type { PtyRun } from "./tui-pty-test-support.js";
const SUBMISSION_SETTLE_MS = 150;
const SESSION_ROLLOVER_BUSY_MESSAGE = "abort the current run before /new";
describe("local TUI PTY fixture support", () => {
it("registers idempotent cleanup before fallible fixture setup", async () => {
const order: string[] = [];
let registeredCleanup: (() => Promise<void>) | undefined;
const setupError = new Error("setup failed");
let cleanupCalls = 0;
await expect(
(async () => {
const cleanup = registerIdempotentCleanup(
(registered) => {
order.push("registered");
registeredCleanup = registered;
},
async () => {
cleanupCalls += 1;
order.push("cleanup");
},
);
try {
order.push("setup");
throw setupError;
} finally {
await cleanup();
}
})(),
).rejects.toBe(setupError);
await registeredCleanup!();
expect(order).toEqual(["registered", "setup", "cleanup"]);
expect(cleanupCalls).toBe(1);
});
it("waits for the matching successful terminal event before history observation", async () => {
const observer = createChatTerminalObserver();
const terminal = observer.waitForFinal({
runId: "run-history",
sessionKey: "agent:main:history",
timeoutMs: 1_000,
});
observer.onEvent({
event: "chat",
payload: {
runId: "run-history",
sessionKey: "agent:main:history",
state: "delta",
},
});
observer.onEvent({
event: "chat",
payload: {
runId: "run-other",
sessionKey: "agent:main:history",
state: "final",
},
});
observer.onEvent({
event: "chat",
payload: {
runId: "run-history",
sessionKey: "agent:main:history",
state: "final",
},
});
await expect(terminal).resolves.toMatchObject({ state: "final" });
});
it("fails promptly when the observed chat run terminates with an error", async () => {
const observer = createChatTerminalObserver();
observer.onEvent({
event: "chat",
payload: {
errorMessage: "provider failed",
runId: "run-history",
sessionKey: "agent:main:history",
state: "error",
},
});
await expect(
observer.waitForFinal({
runId: "run-history",
sessionKey: "agent:main:history",
timeoutMs: 1_000,
}),
).rejects.toThrow("chat run run-history ended as error: provider failed");
});
it("owns late fixture startup without swallowing cleanup failures", async () => {
await expect(cleanupStartedFixture(Promise.reject(new Error("setup failed")))).resolves.toBe(
undefined,
+79
View File
@@ -3,6 +3,85 @@ import { waitFor, type PtyRun } from "./tui-pty-test-support.js";
const STARTUP_TIMEOUT_MS = 60_000;
const OUTPUT_TIMEOUT_MS = 120_000;
type CleanupRegistrar = (cleanup: () => Promise<void>) => void;
export function createIdempotentCleanup(cleanup: () => Promise<void>) {
let cleanupPromise: Promise<void> | undefined;
return () => (cleanupPromise ??= cleanup());
}
// Register before setup starts so a timed-out test still owns partial resources.
export function registerIdempotentCleanup(
registerCleanup: CleanupRegistrar,
cleanup: () => Promise<void>,
) {
const registeredCleanup = createIdempotentCleanup(cleanup);
registerCleanup(registeredCleanup);
return registeredCleanup;
}
type ObservedChatTerminal = {
errorMessage?: string;
runId: string;
sessionKey: string;
state: "aborted" | "error" | "final";
};
// A completed-history assertion must wait for the run's terminal event first.
// Polling history during the run can consume the RPC deadline and leak that run.
export function createChatTerminalObserver() {
const terminals = new Map<string, ObservedChatTerminal>();
const keyFor = (sessionKey: string, runId: string) => `${sessionKey}\u0000${runId}`;
return {
onEvent: ({ event, payload }: { event: string; payload?: unknown }) => {
if (event !== "chat" || !payload || typeof payload !== "object") {
return;
}
const chatEvent = payload as {
errorMessage?: unknown;
runId?: unknown;
sessionKey?: unknown;
state?: unknown;
};
if (
typeof chatEvent.runId !== "string" ||
typeof chatEvent.sessionKey !== "string" ||
(chatEvent.state !== "aborted" &&
chatEvent.state !== "error" &&
chatEvent.state !== "final")
) {
return;
}
terminals.set(keyFor(chatEvent.sessionKey, chatEvent.runId), {
...(typeof chatEvent.errorMessage === "string"
? { errorMessage: chatEvent.errorMessage }
: {}),
runId: chatEvent.runId,
sessionKey: chatEvent.sessionKey,
state: chatEvent.state,
});
},
waitForFinal: async (params: { runId: string; sessionKey: string; timeoutMs: number }) => {
const terminal = await waitFor({
timeoutMs: params.timeoutMs,
read: () => terminals.get(keyFor(params.sessionKey, params.runId)) ?? null,
onTimeout: () =>
new Error(`chat run ${params.runId} did not reach a terminal event before history load`),
});
terminals.delete(keyFor(params.sessionKey, params.runId));
if (terminal.state !== "final") {
throw new Error(
`chat run ${params.runId} ended as ${terminal.state}${
terminal.errorMessage ? `: ${terminal.errorMessage}` : ""
}`,
);
}
return terminal;
},
};
}
export async function waitForOutputAfter(
run: PtyRun,
needle: string,
+86 -27
View File
@@ -21,8 +21,11 @@ import { GatewayChatClient } from "./gateway-chat.js";
import { synchronizedFrameRows } from "./tui-pty-harness-assertion-test-support.js";
import {
cleanupStartedFixture,
createChatTerminalObserver,
createIdempotentCleanup,
createFreshSession,
lastOutputIndexAfter,
registerIdempotentCleanup,
waitForOutputAfter,
} from "./tui-pty-local-test-support.js";
import { sleep, startPty, waitFor, type PtyRun } from "./tui-pty-test-support.js";
@@ -159,11 +162,6 @@ async function requestWithUnavailableRetry<T>(request: () => Promise<T>): Promis
}
}
function createIdempotentCleanup(cleanup: () => Promise<void>) {
let cleanupPromise: Promise<void> | undefined;
return () => (cleanupPromise ??= cleanup());
}
type CleanupRegistrar = (cleanup: () => Promise<void>) => void;
async function readRequestBody(req: IncomingMessage): Promise<string> {
@@ -845,8 +843,37 @@ async function startGatewayModeTui(
const rejectedRequestOffset = shared.mockModel.rejectedRequests().length;
const sessionKey = `agent:${scenario.agentId}:tui-pty-${++gatewaySessionSequence}`;
const sessionKeys = new Set([sessionKey]);
await shared.controlClient.createSession({ key: sessionKey, agentId: scenario.agentId });
await shared.controlClient.patchSession({
const controlClient = new GatewayChatClient({
url: shared.gateway.url,
token: shared.gateway.gatewayToken,
allowInsecureLocalOperatorUi: false,
});
let controlClientConnected = false;
controlClient.onConnected = () => {
controlClientConnected = true;
};
// A timed-out RPC drops its pending response while leaving the socket open.
// Case-local ownership prevents that late work from crossing into the next test.
const cleanup = registerIdempotentCleanup(registerCleanup, async () => {
shared.mockModel.releaseFirstResponse(scenario.modelId);
try {
if (controlClientConnected) {
for (const key of sessionKeys) {
await controlClient.abortChat({ sessionKey: key });
}
}
} finally {
await controlClient.stop();
}
});
controlClient.start();
await waitFor({
timeoutMs: LOCAL_STARTUP_TIMEOUT_MS,
read: () => (controlClientConnected ? true : null),
onTimeout: () => new Error("Gateway case control client did not connect"),
});
await controlClient.createSession({ key: sessionKey, agentId: scenario.agentId });
await controlClient.patchSession({
key: sessionKey,
agentId: scenario.agentId,
model: `tui-pty-mock/${scenario.modelId}`,
@@ -865,13 +892,6 @@ async function startGatewayModeTui(
onTimeout: () => new Error("adopted Gateway session did not reach an idle final screen"),
});
const outputOffset = run.visibleOutput().length;
const cleanup = createIdempotentCleanup(async () => {
shared.mockModel.releaseFirstResponse(scenario.modelId);
for (const key of sessionKeys) {
await shared.controlClient.abortChat({ sessionKey: key });
}
});
registerCleanup(cleanup);
return {
kind: "gateway" as const,
run,
@@ -1593,27 +1613,62 @@ export default {
const shared = await requireSharedGatewayFixture();
const agentId = SHARED_GATEWAY_AGENT_ID;
const sessionKey = `agent:${agentId}:tui-pty-history`;
const runId = randomUUID();
const userMarker = "T02_HISTORY_USER";
const assistantMarker = GATEWAY_SCENARIOS.history.replyText;
const model = `tui-pty-mock/${GATEWAY_SCENARIOS.history.modelId}`;
await shared.controlClient.createSession({ key: sessionKey, agentId });
await shared.controlClient.patchSession({ key: sessionKey, agentId, model });
await shared.controlClient.sendChat({ sessionKey, message: userMarker });
await waitForHistoryMessages(shared.controlClient, sessionKey, ({ messages }) =>
hasOrderedTurn(messages, userMarker, assistantMarker),
);
const attached = await startIsolatedGatewayPty({
gateway: shared.gateway,
registerCleanup: onTestFinished,
sessionKey,
const terminalObserver = createChatTerminalObserver();
const historyClient = new GatewayChatClient({
url: shared.gateway.url,
token: shared.gateway.gatewayToken,
allowInsecureLocalOperatorUi: false,
});
let historyClientConnected = false;
historyClient.onConnected = () => {
historyClientConnected = true;
};
historyClient.onEvent = terminalObserver.onEvent;
const cleanup = registerIdempotentCleanup(onTestFinished, async () => {
try {
if (historyClientConnected) {
await historyClient.abortChat({ sessionKey, runId });
}
} finally {
await historyClient.stop();
}
});
let attached: Awaited<ReturnType<typeof startIsolatedGatewayPty>> | undefined;
try {
await attached.run.waitForOutput(assistantMarker, LOCAL_STARTUP_TIMEOUT_MS);
historyClient.start();
await waitFor({
timeoutMs: LOCAL_STARTUP_TIMEOUT_MS,
read: () => (historyClientConnected ? true : null),
onTimeout: () => new Error("history Gateway client did not connect"),
});
await historyClient.subscribeSessionEvents();
await historyClient.createSession({ key: sessionKey, agentId });
await historyClient.patchSession({ key: sessionKey, agentId, model });
await historyClient.sendChat({ sessionKey, message: userMarker, runId });
await terminalObserver.waitForFinal({
runId,
sessionKey,
timeoutMs: LOCAL_OUTPUT_TIMEOUT_MS,
});
await waitForHistoryMessages(historyClient, sessionKey, ({ messages }) =>
hasOrderedTurn(messages, userMarker, assistantMarker),
);
attached = await startIsolatedGatewayPty({
gateway: shared.gateway,
registerCleanup: onTestFinished,
sessionKey,
});
const attachedRun = attached.run;
await attachedRun.waitForOutput(assistantMarker, LOCAL_STARTUP_TIMEOUT_MS);
const output = await waitFor({
timeoutMs: LOCAL_STARTUP_TIMEOUT_MS,
read: () => {
const screen =
synchronizedFrameRows(attached.run.output(), attached.run)[0]?.join("\n") ?? "";
synchronizedFrameRows(attachedRun.output(), attachedRun)[0]?.join("\n") ?? "";
return screen.includes(userMarker) && screen.includes(assistantMarker) ? screen : null;
},
onTimeout: () => new Error("history did not reach a final synchronized TUI screen"),
@@ -1622,7 +1677,11 @@ export default {
expect(output.split(assistantMarker)).toHaveLength(2);
expect(output.indexOf(userMarker)).toBeLessThan(output.indexOf(assistantMarker));
} finally {
await attached.cleanup();
try {
await attached?.cleanup();
} finally {
await cleanup();
}
}
},
LOCAL_TEST_TIMEOUT_MS,
+2 -1
View File
@@ -727,7 +727,7 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => {
]);
});
it("runs the TUI PTY local smoke against built CLI artifacts", () => {
it("keeps the full TUI PTY suite in its dedicated built-CLI shard", () => {
const tuiPtyShard = createNodeTestShards().find(
(shard) => shard.shardName === "core-runtime-tui-pty",
);
@@ -741,6 +741,7 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => {
},
requiresDist: true,
});
expect(tuiPtyShard?.includePatterns).toBeUndefined();
});
it("covers every infra test exactly once across core runtime infra shards", () => {
+6 -1
View File
@@ -4890,7 +4890,7 @@ printf '%s\n' "\${CURL_SUCCESS_IP:-203.0.113.7}"
expect(saveStep.with.key).toContain("dist-build-v2-");
});
it("parallelizes gateway watch only on the large self-hosted runner and isolates TUI PTY", () => {
it("keeps the full built TUI PTY suite out of the artifact canary gate", () => {
const workflow = readCiWorkflow();
const buildArtifactSteps = workflow.jobs["build-artifacts"].steps;
const builtArtifactChecks = buildArtifactSteps.find(
@@ -4922,6 +4922,11 @@ printf '%s\n' "\${CURL_SUCCESS_IP:-203.0.113.7}"
expect(hostedGatewayWait).toBeGreaterThan(hostedGatewayWatch);
expect(tuiPty).toBeGreaterThan(hostedGatewayWait);
expect(tuiPtyWait).toBeGreaterThan(tuiPty);
expect(run.slice(tuiPty, tuiPtyWait)).toContain("src/tui/tui-pty-local.e2e.test.ts");
expect(run.slice(tuiPty, tuiPtyWait)).toContain("--testNamePattern");
expect(run.slice(tuiPty, tuiPtyWait)).toContain(
"launches openclaw (chat as local mode|tui against a real Gateway) through a real PTY",
);
expect(run).toContain("wait_checks()");
expect(run.match(/wait_checks$/gmu)).toHaveLength(3);
});