From e276358c21593a4e3d58d657eeb1a2142d7f149a Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 19 Jul 2026 18:09:37 +0800 Subject: [PATCH 1/2] =?UTF-8?q?test(autofix):=20exercise=20the=20SKILL=20s?= =?UTF-8?q?tage=E2=86=94resolve=20contract=20end-to-end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #7225, implementing the reviewer's non-blocking suggestions. The staging guard #7225 added pins the mirrored LAYOUT but re-implements run-agent.mjs's `/../SKILL.md` convention in the test. If that coupling ever moves in the RUNNER (e.g. ../../SKILL.md), the string test stays green while prod breaks again — the same class of blind spot that let #7165 ship. This adds the one check that exercises the contract for real: stage the actual runner into a mirrored tmp layout, run it with --print-prompt, and assert it reads the staged SKILL (sentinel body + resolved skill dir). The negative case — the flat layout #7165 shipped — is asserted to crash with ENOENT, proving the test catches that regression. Also replaces the brittle fixed-width `[\s\S]{0,200}` bound between `core.hooksPath .husky` and the runner invocation with a direct ordering assertion (indexOf), so adding a comment between the two lines can no longer fail the test spuriously. 61/61 + 12/12. --- scripts/tests/qwen-autofix-workflow.test.js | 87 ++++++++++++++++++++- 1 file changed, 85 insertions(+), 2 deletions(-) diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index 7f1e33269d4..a7f444924dc 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -8,6 +8,7 @@ import { execFileSync, spawnSync } from 'node:child_process'; import { chmodSync, existsSync, + mkdirSync, mkdtempSync, readFileSync, rmSync, @@ -1855,6 +1856,80 @@ describe('qwen-autofix workflow', () => { ); }); + it('resolves the staged SKILL end-to-end by running the real runner (stage↔resolve contract)', () => { + // The string test above pins the mirrored LAYOUT, but it re-implements + // run-agent.mjs's `/../SKILL.md` convention. If that coupling ever + // moves in the RUNNER, the string test stays green while prod breaks — + // the same class of blind spot that let #7165 ship. This test runs the + // ACTUAL runner against the staged layout and asserts it reads the + // staged SKILL, exercising the stage↔resolve contract for real. + const runner = readFileSync(autofixRunnerScriptPath, 'utf8'); + const dir = mkdtempSync(join(tmpdir(), 'autofix-stage-')); + try { + // Mirror the workflow's staging: autofix-skill/{SKILL.md,scripts/run-agent.mjs}. + mkdirSync(join(dir, 'autofix-skill', 'scripts'), { recursive: true }); + writeFileSync( + join(dir, 'autofix-skill', 'SKILL.md'), + '---\nname: autofix\n---\nSTAGED_SKILL_SENTINEL\n', + ); + const stagedRunner = join( + dir, + 'autofix-skill', + 'scripts', + 'run-agent.mjs', + ); + writeFileSync(stagedRunner, runner); + const ok = spawnSync( + 'node', + [ + stagedRunner, + '--mode', + 'address-review', + '--pr', + '1', + '--issue', + '1', + '--workdir', + dir, + '--print-prompt', + ], + { encoding: 'utf8' }, + ); + expect(ok.status).toBe(0); + // The real runner resolved ../SKILL.md to the STAGED copy and inlined it. + expect(ok.stdout).toContain('STAGED_SKILL_SENTINEL'); + // Skill directory ends in the mirrored dir name (basename, not the full + // temp path — macOS canonicalizes /var → /private/var). + expect(ok.stdout).toMatch(/Skill directory: \S*[/\\]autofix-skill\n/); + + // And the FLAT layout #7165 shipped (runner alone, no ../SKILL.md) must + // crash with ENOENT — proving this test catches that regression. + const flatRunner = join(dir, 'run-agent.mjs'); + writeFileSync(flatRunner, runner); + const flat = spawnSync( + 'node', + [ + flatRunner, + '--mode', + 'address-review', + '--pr', + '1', + '--issue', + '1', + '--workdir', + dir, + '--print-prompt', + ], + { encoding: 'utf8' }, + ); + expect(flat.status).not.toBe(0); + expect(flat.stderr).toContain('ENOENT'); + expect(flat.stderr).toContain("SKILL.md'"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it('runs heavy autofix jobs on hosted runners with sandbox images', () => { const workflowAndSkill = `${workflow}\n${readAutofixSkill()}`; @@ -2304,9 +2379,17 @@ describe('qwen-autofix workflow', () => { expect(workflow).toMatch( /git config core\.hooksPath \/dev\/null\n\s+git checkout -B "\$\{BRANCH\}"/, ); - expect(workflow).toMatch( - /git config core\.hooksPath \.husky\n[\s\S]{0,200}node "\$\{RUNNER_TEMP\}\/autofix-skill\/scripts\/run-agent\.mjs"/, + // The agent step re-points hooks to .husky BEFORE invoking the runner. + // Assert the ordering directly (not a fixed-width window) so adding a + // comment between the two lines can't fail the test spuriously. + const huskyAt = triageAndAddressStep.indexOf( + 'git config core.hooksPath .husky', + ); + const stagedNodeAt = triageAndAddressStep.indexOf( + 'node "${RUNNER_TEMP}/autofix-skill/scripts/run-agent.mjs"', ); + expect(huskyAt).toBeGreaterThanOrEqual(0); + expect(stagedNodeAt).toBeGreaterThan(huskyAt); }); it('keeps sandbox image fallback covered by a reusable script', () => { From b7d118d8c8b11fae3a336622a1d540fd462242c6 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 19 Jul 2026 20:18:43 +0800 Subject: [PATCH 2/2] =?UTF-8?q?test(autofix):=20harden=20the=20stage?= =?UTF-8?q?=E2=86=94resolve=20integration=20test=20per=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies all four inline suggestions on #7227: - spawn process.execPath, not the bare 'node' string, so a version-manager shim or a PATH without node can't turn the test into an opaque 'null !== 0'. - nest the flat-layout runner under dir/flat/ so its ../SKILL.md resolves to dir/SKILL.md (never created) instead of a shared tmpdir()/SKILL.md that a concurrent job could leave behind and make the negative case pass spuriously — a real flake in the deflake-test itself. - reuse the existing withRunnerDir helper instead of duplicating its mkdtemp/try/finally/rmSync. - bound each spawnSync with timeout: 10_000 so a hung runner fails the test instead of the whole CI job (spawnSync blocks the event loop, so vitest's async timeout can't fire). 61/61 + 12/12. --- scripts/tests/qwen-autofix-workflow.test.js | 68 +++++++++------------ 1 file changed, 29 insertions(+), 39 deletions(-) diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index a7f444924dc..b36d397e6de 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -1864,8 +1864,26 @@ describe('qwen-autofix workflow', () => { // ACTUAL runner against the staged layout and asserts it reads the // staged SKILL, exercising the stage↔resolve contract for real. const runner = readFileSync(autofixRunnerScriptPath, 'utf8'); - const dir = mkdtempSync(join(tmpdir(), 'autofix-stage-')); - try { + const printPrompt = (scriptPath, dir) => + spawnSync( + process.execPath, + [ + scriptPath, + '--mode', + 'address-review', + '--pr', + '1', + '--issue', + '1', + '--workdir', + dir, + '--print-prompt', + ], + // spawnSync blocks the event loop, so vitest's async timeout can't + // fire — bound each subprocess directly against a hung runner. + { encoding: 'utf8', timeout: 10_000 }, + ); + withRunnerDir((dir) => { // Mirror the workflow's staging: autofix-skill/{SKILL.md,scripts/run-agent.mjs}. mkdirSync(join(dir, 'autofix-skill', 'scripts'), { recursive: true }); writeFileSync( @@ -1879,22 +1897,7 @@ describe('qwen-autofix workflow', () => { 'run-agent.mjs', ); writeFileSync(stagedRunner, runner); - const ok = spawnSync( - 'node', - [ - stagedRunner, - '--mode', - 'address-review', - '--pr', - '1', - '--issue', - '1', - '--workdir', - dir, - '--print-prompt', - ], - { encoding: 'utf8' }, - ); + const ok = printPrompt(stagedRunner, dir); expect(ok.status).toBe(0); // The real runner resolved ../SKILL.md to the STAGED copy and inlined it. expect(ok.stdout).toContain('STAGED_SKILL_SENTINEL'); @@ -1903,31 +1906,18 @@ describe('qwen-autofix workflow', () => { expect(ok.stdout).toMatch(/Skill directory: \S*[/\\]autofix-skill\n/); // And the FLAT layout #7165 shipped (runner alone, no ../SKILL.md) must - // crash with ENOENT — proving this test catches that regression. - const flatRunner = join(dir, 'run-agent.mjs'); + // crash with ENOENT — proving this test catches that regression. Nest it + // under dir/flat/ so its ../SKILL.md resolves to dir/SKILL.md (which this + // test never creates) rather than a shared tmpdir()/SKILL.md a concurrent + // job could leave behind and make the runner exit 0 spuriously. + mkdirSync(join(dir, 'flat'), { recursive: true }); + const flatRunner = join(dir, 'flat', 'run-agent.mjs'); writeFileSync(flatRunner, runner); - const flat = spawnSync( - 'node', - [ - flatRunner, - '--mode', - 'address-review', - '--pr', - '1', - '--issue', - '1', - '--workdir', - dir, - '--print-prompt', - ], - { encoding: 'utf8' }, - ); + const flat = printPrompt(flatRunner, dir); expect(flat.status).not.toBe(0); expect(flat.stderr).toContain('ENOENT'); expect(flat.stderr).toContain("SKILL.md'"); - } finally { - rmSync(dir, { recursive: true, force: true }); - } + }); }); it('runs heavy autofix jobs on hosted runners with sandbox images', () => {