test(integration): give the skill-hook parity E2E the docker startup budget (#15226) (#15237)

* test(integration): give the skill-hook parity E2E the docker startup budget (#15226)

The parity suite hardcoded a 30s TUI-readiness wait, but the sandbox:docker
leg boots a fresh container and has stalled past that before the prompt
renders (#13552, #14191). The model-invoked case runs first in the file, so
its cold boot was the one that timed out while the second case started warm.

Use the measured readyPromptBudgetMs for the readiness wait (180s on the
self-hosted docker leg) and the harness wait default for the shared waitFor,
then size each case's inline ceiling to those budgets. No assertion or
gate-hit expectation changes.

* docs(integration): correct ready-prompt budget consumer note (#15226)

Co-authored-by: Qwen-Coder <[email protected]>

* test(integration): harden skill-hook parity startup waits (#15226)

* test(integration): stop abandoned polls and pin budget invariants (#15226)

Co-authored-by: Qwen-Coder <[email protected]>

* fix(integration-tests): re-arm a reused TestRig after cleanup (#15226)

---------

Co-authored-by: qwen-code-dev-bot <[email protected]>
Co-authored-by: Qwen-Coder <[email protected]>
Co-authored-by: 易良 <[email protected]>

integration-tests/ready-prompt-budget.ts

@@ -18,13 +18,26 @@
  * a genuinely broken boot failing fast. Any budget here must stay under the
  * suite's testTimeout (TB_TIMEOUT_MINUTES, default 5min).
  *
- * Only InteractiveSession.start was observed to stall; the TestRig readiness
- * waits keep their own budgets and are tracked separately.
+ * InteractiveSession.start and the skill-hooks invocation-parity suite's
+ * TestRig readiness wait use this budget; other TestRig readiness waits keep
+ * their own budgets and are tracked separately.
  *
  * Pure and read at call time so the decision is pinned state-by-state with a
  * plain object instead of real time (scripts/tests/ready-prompt-budget.test.ts).
  */
+
+const TIERS = {
+  default: 30_000,
+  selfHosted: 90_000,
+  selfHostedDocker: 180_000,
+} as const;
+
+/** The widest branch, so a suite ceiling can be derived instead of copied. */
+export const MAX_READY_PROMPT_BUDGET_MS = Math.max(...Object.values(TIERS));
+
 export function readyPromptBudgetMs(env: NodeJS.ProcessEnv): number {
-  if (env['RUNNER_ENVIRONMENT'] !== 'self-hosted') return 30_000;
-  return env['QWEN_SANDBOX'] === 'docker' ? 180_000 : 90_000;
+  if (env['RUNNER_ENVIRONMENT'] !== 'self-hosted') return TIERS.default;
+  return env['QWEN_SANDBOX'] === 'docker'
+    ? TIERS.selfHostedDocker
+    : TIERS.selfHosted;
 }

integration-tests/skill-hooks-invocation-parity.test.ts

@@ -13,7 +13,11 @@
  */
 
 import { describe, it, expect, afterEach } from 'vitest';
-import { fakeServerHostOptions, TestRig } from './test-helper.js';
+import {
+  CI_DEFAULT_WAIT_MS,
+  fakeServerHostOptions,
+  TestRig,
+} from './test-helper.js';
 import { fakeToolCall, startFakeOpenAIServer } from './fake-openai-server.js';
 import type { FakeOpenAIServer } from './fake-openai-server.js';
 import {
@@ -27,9 +31,25 @@ import {
   makeWaitFor,
   stubFakeModelEnv,
 } from './helpers/gated-skill-fixture.js';
+import {
+  MAX_READY_PROMPT_BUDGET_MS,
+  readyPromptBudgetMs,
+} from './ready-prompt-budget.js';
 import { join } from 'node:path';
 import { existsSync } from 'node:fs';
 
+// Per-case ceilings over a stalled boot: the ready-prompt budget plus 3/5
+// waits at the harness's 60s CI default. Derived from the budget so a raise
+// moves both ceilings. retry: 0 because the suite retries twice by default,
+// and a stall that exhausts the ready budget cannot recover on a retry —
+// three attempts would burn 18 minutes of the docker leg's 60-minute job.
+const CI_WAIT_MS = CI_DEFAULT_WAIT_MS;
+const SLACK_MS = 60_000;
+const MODEL_CASE_TIMEOUT =
+  MAX_READY_PROMPT_BUDGET_MS + 3 * CI_WAIT_MS + SLACK_MS;
+const USER_CASE_TIMEOUT =
+  MAX_READY_PROMPT_BUDGET_MS + 5 * CI_WAIT_MS + SLACK_MS;
+
 describe('skill hooks fire on both invocation paths', () => {
   let rig: TestRig;
   let fakeServer: FakeOpenAIServer;
@@ -85,7 +105,7 @@ describe('skill hooks fire on both invocation paths', () => {
 
     restoreNoProxy = stubFakeModelEnv(rig, fakeServer);
 
-    const { ptyProcess } = rig.runInteractive(
+    const { ptyProcess, promise: exited } = rig.runInteractive(
       ...fakeModelLaunchArgs(fakeServer),
     );
 
@@ -94,7 +114,26 @@ describe('skill hooks fire on both invocation paths', () => {
       output += d;
     });
 
-    const ready = await rig.waitForText('Type your message', 30000);
+    // The docker leg's container boot has stalled past 30s before the prompt
+    // renders (#13552, #14191); use the measured self-hosted budget instead of
+    // the old fixed 30s. A child that dies during startup fails fast with its
+    // exit code, and cleanup() stops the abandoned waitForText poll within one
+    // tick instead of leaving it to scan the dead pty for the whole budget;
+    // only a live-but-slow boot pays that budget.
+    let startupUndecided = true;
+    const ready = await Promise.race([
+      rig.waitForText('Type your message', readyPromptBudgetMs(process.env)),
+      exited.then(({ exitCode, signal }) => {
+        // cleanup() kills the child on every healthy session's teardown, when
+        // the race is already settled: the abandoned arm must not reject.
+        if (!startupUndecided) return false;
+        throw new Error(
+          `CLI exited during startup (code ${exitCode}, signal ${signal})\n` +
+            `Output:\n${output}`,
+        );
+      }),
+    ]);
+    startupUndecided = false;
     expect(ready, `CLI did not start. Output:\n${output}`).toBe(true);
 
     const waitFor = makeWaitFor(rig, () => output);
@@ -144,27 +183,35 @@ describe('skill hooks fire on both invocation paths', () => {
     return { output, executed: existsSync(flagPath) };
   }
 
-  it('model-invoked skill: the gate fires and blocks the shell call', async () => {
-    const { output, executed } = await driveSession({
-      testName: 'skill hooks model invoked',
-      modelInvokesSkill: true,
-    });
-    expect(
-      output,
-      `gate did not fire. Output tail:\n${output.slice(-1500)}`,
-    ).toContain(GATE_MARKER);
-    expect(executed).toBe(false);
-  }, 240_000); // CI: 30s startup + 3 waits at the 60s default.
+  it(
+    'model-invoked skill: the gate fires and blocks the shell call',
+    { timeout: MODEL_CASE_TIMEOUT, retry: 0 },
+    async () => {
+      const { output, executed } = await driveSession({
+        testName: 'skill hooks model invoked',
+        modelInvokesSkill: true,
+      });
+      expect(
+        output,
+        `gate did not fire. Output tail:\n${output.slice(-1500)}`,
+      ).toContain(GATE_MARKER);
+      expect(executed).toBe(false);
+    },
+  );
 
-  it('user-invoked skill (/<skill-name>): the gate fires and blocks the shell call', async () => {
-    const { output, executed } = await driveSession({
-      testName: 'skill hooks slash invoked',
-      modelInvokesSkill: false,
-    });
-    expect(
-      output,
-      `gate did not fire. Output tail:\n${output.slice(-1500)}`,
-    ).toContain(GATE_MARKER);
-    expect(executed).toBe(false);
-  }, 360_000); // CI: 30s startup + 5 waits at the 60s default.
+  it(
+    'user-invoked skill (/<skill-name>): the gate fires and blocks the shell call',
+    { timeout: USER_CASE_TIMEOUT, retry: 0 },
+    async () => {
+      const { output, executed } = await driveSession({
+        testName: 'skill hooks slash invoked',
+        modelInvokesSkill: false,
+      });
+      expect(
+        output,
+        `gate did not fire. Output tail:\n${output.slice(-1500)}`,
+      ).toContain(GATE_MARKER);
+      expect(executed).toBe(false);
+    },
+  );
 });

integration-tests/test-helper.test.ts

@@ -133,6 +133,36 @@ describe('TestRig', () => {
       .toBe(false);
   });
 
+  it('re-arms a reused rig after cleanup', async () => {
+    const rig = new TestRig();
+    await rig.setup('reused after cleanup');
+
+    await rig.cleanup();
+    await rig.setup('reused after cleanup');
+
+    await expect(rig.poll(() => true, 1_000, 10)).resolves.toBe(true);
+  });
+
+  it('stops a pending readiness poll when the rig is cleaned up', async () => {
+    const rig = new TestRig();
+    await rig.setup('cleanup stops readiness poll');
+
+    const settled = rig.waitForText('never-rendered', 60_000);
+    await rig.cleanup();
+
+    // Without the stop flag the abandoned poll keeps scanning until its 60s
+    // budget expires; the race bounds the failure so a regression reddens
+    // this case instead of stalling the lane.
+    await expect(
+      Promise.race([
+        settled,
+        new Promise<'still polling'>((resolve) =>
+          setTimeout(() => resolve('still polling'), 5_000).unref(),
+        ),
+      ]),
+    ).resolves.toBe(false);
+  });
+
   it.each([
     [
       'telemetry events',

integration-tests/test-helper.ts

@@ -200,12 +200,21 @@ interface ParsedLog {
   }[];
 }
 
+/**
+ * `TestRig.getDefaultTimeout()`'s CI branch. Exported so a suite that derives
+ * its own ceilings multiplies this value instead of re-spelling it.
+ */
+export const CI_DEFAULT_WAIT_MS = 60_000;
+
 export class TestRig {
   bundlePath: string;
   testDir: string | null;
   testName?: string;
   _lastRunStdout?: string;
   _interactiveOutput = '';
+  // cleanup() sets this so an abandoned poll stops instead of scanning a dead
+  // pty until its own budget expires.
+  _closed = false;
   private readonly interactiveProcesses: pty.IPty[] = [];
 
   constructor() {
@@ -215,7 +224,7 @@ export class TestRig {
 
   // Get timeout based on environment
   getDefaultTimeout() {
-    if (env['CI']) return 60000; // 1 minute in CI
+    if (env['CI']) return CI_DEFAULT_WAIT_MS; // 1 minute in CI
     if (env['QWEN_SANDBOX']) return 30000; // 30s in containers
     return 15000; // 15s locally
   }
@@ -225,6 +234,8 @@ export class TestRig {
     options: { settings?: Record<string, unknown> } = {},
   ) {
     this.testName = testName;
+    // setup() re-arms a rig reused after cleanup().
+    this._closed = false;
     const sanitizedName = sanitizeTestName(testName);
     this.testDir = join(
       env['INTEGRATION_TEST_FILE_DIR']!,
@@ -520,6 +531,7 @@ export class TestRig {
   }
 
   async cleanup() {
+    this._closed = true;
     // A session a test never closed keeps its CLI child forwarding PTY bytes
     // into this worker's stdout; after vitest tears the worker down those
     // writes EPIPE and fail an otherwise all-green run (#10969).
@@ -634,7 +646,7 @@ export class TestRig {
     const startTime = Date.now();
     let attempts = 0;
     let lastError: unknown;
-    while (Date.now() - startTime < timeout) {
+    while (!this._closed && Date.now() - startTime < timeout) {
       attempts++;
       try {
         const result = predicate();

scripts/tests/ready-prompt-budget.test.ts

@@ -5,7 +5,10 @@
  */
 
 import { describe, expect, it } from 'vitest';
-import { readyPromptBudgetMs } from '../../integration-tests/ready-prompt-budget.js';
+import {
+  MAX_READY_PROMPT_BUDGET_MS,
+  readyPromptBudgetMs,
+} from '../../integration-tests/ready-prompt-budget.js';
 
 // The interactive real-spawn cases sit in a file no PR-triggered job
 // collects: ci.yml's PR-gated integration_no_ak leg runs an explicit
@@ -14,6 +17,26 @@ import { readyPromptBudgetMs } from '../../integration-tests/ready-prompt-budget
 // is therefore pinned here, where test:scripts collects it on every PR — the
 // same shape as integration-vitest-config.test.ts's RUNNER_ENVIRONMENT pin.
 describe('readyPromptBudgetMs', () => {
+  it('exports the widest budget so suite ceilings derive from it', () => {
+    const branches = [
+      readyPromptBudgetMs({}),
+      readyPromptBudgetMs({ RUNNER_ENVIRONMENT: 'github-hosted' }),
+      readyPromptBudgetMs({ RUNNER_ENVIRONMENT: 'self-hosted' }),
+      readyPromptBudgetMs({
+        RUNNER_ENVIRONMENT: 'self-hosted',
+        QWEN_SANDBOX: 'docker',
+      }),
+    ];
+    // MAX derives from every TIER, so this cross-check reddens when a new
+    // tier is added without being sampled here. The parity suite sizes its
+    // per-case timeouts from MAX, so a raise moves those ceilings too.
+    expect(Math.max(...branches)).toBe(MAX_READY_PROMPT_BUDGET_MS);
+    // "Any budget here must stay under the suite's testTimeout"
+    // (ready-prompt-budget.ts): interactive-session.test.ts sets no per-case
+    // timeout and inherits integration-tests/vitest.config.ts's 300s default.
+    expect(MAX_READY_PROMPT_BUDGET_MS).toBeLessThan(300_000);
+  });
+
   it('widens the ready-prompt budget on shared self-hosted runners (#13552)', () => {
     expect(readyPromptBudgetMs({ RUNNER_ENVIRONMENT: 'self-hosted' })).toBe(
       90_000,

scripts/tests/skill-hooks-parity-ready-budget.test.ts

@@ -0,0 +1,81 @@
+/**
+ * @license
+ * Copyright 2026 Qwen Team
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { readFileSync } from 'node:fs';
+import { join, resolve } from 'node:path';
+import { describe, expect, it } from 'vitest';
+
+// The parity suite runs in the PR-gated no-AK lane, but neither property here
+// is observable there on a green run: the ubuntu-latest fallback resolves the
+// budget to the same 30s the deleted literal was, while on the ecs-qwen
+// routing the runner injects RUNNER_ENVIRONMENT=self-hosted and
+// QWEN_SANDBOX=false selects the 90s tier. Neither value is asserted on the
+// lane, a healthy boot renders the prompt in seconds and never exercises the
+// dead-child race, and only a non-deterministic boot stall would surface a
+// revert to 30_000. These byte-shape pins give each revert a ~0ms witness
+// through test:scripts, which runs on every PR.
+const source = readFileSync(
+  join(
+    resolve(import.meta.dirname, '../..'),
+    'integration-tests/skill-hooks-invocation-parity.test.ts',
+  ),
+  'utf8',
+);
+
+describe('skill-hooks invocation-parity startup source pins (#15226)', () => {
+  it('threads the resolved ready-prompt budget into the waitFor', () => {
+    // A full revert to the literal 30_000 drops the readyPromptBudgetMs call
+    // and its import together, and otherwise passes every PR-gated lane. The
+    // call fits on one line today, but a reflow past 80 columns emits a
+    // trailing comma, so the pattern tolerates both forms.
+    expect(source).toMatch(
+      /rig\.waitForText\(\s*'Type your message',\s*readyPromptBudgetMs\(process\.env\),?\s*\)/,
+    );
+  });
+
+  it('races the ready-prompt wait against child exit, keeping the tail', () => {
+    // Reverting the race returns a dead-child boot to a full-budget poll of a
+    // dead pty and discards the exit code; dropping the throw's tail loses the
+    // host-side boot log that made #15226 diagnosable from the job log. The
+    // latch keeps the race's abandoned arm from rejecting on teardown. The
+    // same `Output:\n${output}` substring also appears in the untouched
+    // expect below, so the tail must be pinned as part of the throw.
+    expect(source).toContain('Promise.race([');
+    expect(source).toContain('if (!startupUndecided) return false;');
+    expect(source).toContain(
+      'CLI exited during startup (code ${exitCode}, signal ${signal})',
+    );
+    expect(source).toContain('Output:\\n${output}');
+    expect(source).toMatch(
+      /CLI exited during startup[\s\S]{0,120}?Output:\\n\$\{output\}/,
+    );
+  });
+
+  it('derives the CI wait from the harness default', () => {
+    // The ceilings multiply this value, and the harness derives every
+    // makeWaitFor budget from the same default. Re-spelling 60_000 here lets
+    // a harness change move the real waits without moving the ceilings.
+    expect(source).toMatch(/CI_WAIT_MS\s*=\s*CI_DEFAULT_WAIT_MS/);
+  });
+
+  it('derives both case ceilings from the budget and bounds retries', () => {
+    // Hand-derived ceilings go stale on the next budget raise; deriving them
+    // makes the raise move both. The explicit retry: 0 stops the root
+    // config's retry: 2 from tripling a stall that cannot recover.
+    expect(source).toContain(
+      'MAX_READY_PROMPT_BUDGET_MS + 3 * CI_WAIT_MS + SLACK_MS',
+    );
+    expect(source).toContain(
+      'MAX_READY_PROMPT_BUDGET_MS + 5 * CI_WAIT_MS + SLACK_MS',
+    );
+    expect(source).toMatch(
+      /\{\s*timeout: MODEL_CASE_TIMEOUT,\s*retry: 0,?\s*\}/,
+    );
+    expect(source).toMatch(
+      /\{\s*timeout: USER_CASE_TIMEOUT,\s*retry: 0,?\s*\}/,
+    );
+  });
+});