Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/qwen-code-pr-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ jobs:
fi
done
git worktree prune -v || true
echo "stale review worktrees cleaned"
echo "stale agent state cleaned"

# SECURITY: checkout trusted base code; /review fetches PR diff context.
- name: 'Checkout base branch'
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/config/config-session-env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const baseParams: ConfigParameters = {
// flag resets). That cold transform+evaluate runs several seconds and, under
// a contended CI runner, crosses the 5s default — a flaky timeout, not a hang.
// The reset is load-bearing for what these tests check, so give them headroom.
vi.setConfig({ testTimeout: 30_000 });
vi.setConfig({ testTimeout: 30_000, hookTimeout: 30_000 });

describe('Config sessionEnvClaimed guard', () => {
let originalEnv: string | undefined;
Expand Down
16 changes: 7 additions & 9 deletions packages/core/src/skills/skill-activation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,14 @@
*/

import * as path from 'node:path';
import { describe, expect, it, vi } from 'vitest';
import { describe, expect, it } from 'vitest';
import {
SkillActivationRegistry,
resolveProjectRelativePath,
splitConditionalSkills,
} from './skill-activation.js';
import type { SkillConfig } from './types.js';

// The integration tests below `await import('../core/coreToolScheduler.js')`
// just to reach one pure helper, but that drags in the whole scheduler module
// graph cold. The first such import runs a few seconds and, under a contended
// CI runner, crosses the 5s default — a flaky timeout, not a hang.
vi.setConfig({ testTimeout: 30_000 });

function makeSkill(overrides: Partial<SkillConfig>): SkillConfig {
return {
name: overrides.name ?? 'test-skill',
Expand Down Expand Up @@ -248,6 +242,10 @@ describe('resolveProjectRelativePath', () => {
});

describe('extractToolFilePaths → SkillActivationRegistry integration', () => {
// These tests `await import('../core/coreToolScheduler.js')` just to reach
// one pure helper, but that drags in the whole scheduler module graph cold.
// Under a contended CI runner, that can cross the 5s default timeout.

// Regression: feed the real candidate output for a `glob` call into
// the registry and assert end-to-end activation. The earlier per-field
// extraction (path + pattern as separate candidates) silently failed
Expand All @@ -272,7 +270,7 @@ describe('extractToolFilePaths → SkillActivationRegistry integration', () => {
for (const n of reg.matchAndConsume(c)) activated.add(n);
}
expect(Array.from(activated)).toEqual(['tsx-helper']);
});
}, 30_000);

it('does NOT activate from external glob.path (project-root guard wins)', async () => {
const { extractToolFilePaths } = await import(
Expand All @@ -291,5 +289,5 @@ describe('extractToolFilePaths → SkillActivationRegistry integration', () => {
for (const n of reg.matchAndConsume(c)) activated.add(n);
}
expect(activated.size).toBe(0);
});
}, 30_000);
});
38 changes: 38 additions & 0 deletions scripts/tests/qwen-resolve-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,31 @@ const repoRoot = path.resolve(
'../..',
);

function escapeRegExp(value) {
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}

function job(workflow, name) {
const start = workflow.indexOf(`\n ${name}:`);
if (start === -1) {
return '';
}
const nextJob = workflow.slice(start + 1).search(/\n {2}\S/);
return nextJob === -1
? workflow.slice(start)
: workflow.slice(start, start + 1 + nextJob);
}

function step(section, name) {
const escaped = escapeRegExp(name);
const match = section.match(
new RegExp(
`\\n\\s+- name:\\s*(['"])${escaped}\\1[\\s\\S]*?(?=\\n\\s+- name:\\s*['"]|\\n\\s{2}[a-zA-Z0-9_-]+:|$)`,
),
);
return match?.[0] ?? '';
}

describe('qwen resolve workflow', () => {
const workflow = readFileSync(
path.join(repoRoot, '.github/workflows/qwen-code-pr-review.yml'),
Expand Down Expand Up @@ -88,6 +113,18 @@ describe('qwen resolve workflow', () => {
expect(workflow).not.toContain('qwen-fix-conflicts');
});

it('isolates review agent state per run', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] These regression tests verify the cleanup step contents and the agent step QWEN_HOME, but they do not assert that cleanup runs before the agent. If Clean stale agent state is later moved below Run review or Run Qwen Triage, stale state would still be used while both tests keep passing. Please add a relative-order assertion in both workflow tests, for example by comparing the scoped indices of the cleanup and agent steps.

expect(reviewJob.indexOf("- name: 'Clean stale agent state'"))
  .toBeLessThan(reviewJob.indexOf("- name: 'Run review'"));

— GPT-5 via Qwen Code /review

const cleanStep = step(reviewJob, 'Clean stale agent state');
const agentStep = step(reviewJob, 'Run review');

expect(cleanStep).toContain('QWEN_HOME="${RUNNER_TEMP:?}/qwen-home"');
expect(cleanStep).toContain('rm -rf "$QWEN_HOME"');
expect(cleanStep).toContain('mkdir -p "$QWEN_HOME"');
expect(cleanStep).toContain('rm -f /tmp/stage-*.md');
expect(cleanStep).toContain('echo "stale agent state cleaned"');
expect(agentStep).toContain("QWEN_HOME: '${{ runner.temp }}/qwen-home'");
});

// Whole-file `toContain` cannot tell which job a guard lives on. Slice the
// resolve-pr job so these assertions fail if a future edit drops a guard
// specifically from the credentialed conflict-resolution path. Bound the slice
Expand All @@ -101,6 +138,7 @@ describe('qwen resolve workflow', () => {
nextJob === -1
? workflow.slice(resolveJobStart)
: workflow.slice(resolveJobStart, resolveJobStart + 1 + nextJob);
const reviewJob = job(workflow, 'review-pr');

it('keeps the authorization and scope guards on resolve-pr', () => {
// /resolve must require write+ permission before any credentialed push.
Expand Down
12 changes: 12 additions & 0 deletions scripts/tests/qwen-triage-workflow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@ describe('qwen-triage tmux workflow', () => {
expect(runStep).toContain('"OPENAI_MODEL=$OPENAI_MODEL"');
});

it('isolates agent state per run', () => {
const cleanStep = step('Clean stale agent state');
const runStep = step('Run Qwen Triage');

expect(cleanStep).toContain('QWEN_HOME="${RUNNER_TEMP:?}/qwen-home"');
expect(cleanStep).toContain('rm -rf "$QWEN_HOME"');
expect(cleanStep).toContain('mkdir -p "$QWEN_HOME"');
expect(cleanStep).toContain('rm -f /tmp/stage-*.md');
expect(cleanStep).toContain('echo "stale agent state cleaned"');
expect(runStep).toContain("QWEN_HOME: '${{ runner.temp }}/qwen-home'");
});

it('reports timeout and infra-error without claiming the flow was exercised', () => {
const postStep = step('Post tmux result comment');

Expand Down
Loading