Skip to content

ci: quarantine cron-interactive from push E2E to nightly-only #6986

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

Merged
wenshao merged 1 commit into from
Jul 15, 2026
Merged

ci: quarantine cron-interactive from push E2E to nightly-only #6986

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 52 additions & 3 deletions .github/workflows/e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ jobs:
VERBOSE: 'true'
run: |-
if [[ "${{ matrix.sandbox }}" == "sandbox:docker" ]]; then
npm run test:integration:sandbox:docker
npm run test:integration:sandbox:docker -- --exclude '**/interactive/cron-interactive.test.ts'
else
npm run test:integration:sandbox:none
npm run test:integration:sandbox:none -- --exclude '**/interactive/cron-interactive.test.ts'
fi

e2e-test-macos:
Expand Down Expand Up @@ -143,7 +143,56 @@ jobs:
OPENAI_API_KEY: '${{ secrets.OPENAI_API_KEY }}'
OPENAI_BASE_URL: '${{ secrets.OPENAI_BASE_URL }}'
OPENAI_MODEL: '${{ secrets.OPENAI_MODEL }}'
run: 'npm run test:e2e'
run: 'npx cross-env VERBOSE=true KEEP_OUTPUT=true QWEN_SANDBOX=false vitest run --root ./integration-tests --exclude "**/interactive/cron-interactive.test.ts"'

@qwen-code-ci-bot qwen-code-ci-bot Jul 15, 2026

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] The macOS job inlines the full vitest command instead of delegating to the test:e2e npm script like every other job. This means future changes to test:e2e or test:integration:sandbox:none in package.json (new flags, reporters, pool options) will silently not apply to the macOS job.

Concrete cost: the Linux job uses npm run test:integration:sandbox:none -- --exclude '...' and automatically picks up script changes. The macOS job should follow the same pattern through test:e2e:

Suggested change
run: 'npx cross-env VERBOSE=true KEEP_OUTPUT=true QWEN_SANDBOX=false vitest run --root ./integration-tests --exclude "**/interactive/cron-interactive.test.ts"'
run: 'npm run test:e2e -- --exclude "**/interactive/cron-interactive.test.ts"'

This expands through test:e2e → test:integration:sandbox:none → vitest run --root ./integration-tests --exclude "**/interactive/cron-interactive.test.ts", preserving VERBOSE, KEEP_OUTPUT, and QWEN_SANDBOX from the script chain while picking up any future script modifications.

— qwen3.7-max via Qwen Code /review


cron-interactive-nightly:
name: 'cron-interactive E2E (nightly)'
Comment on lines +148 to +149

@doudouOUC doudouOUC Jul 15, 2026

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] continue-on-error: true combined with the existing failure-notification workflow (main-ci-failure-issue.yml) creates a silent-coverage gap. That workflow filters on conclusion == 'failure' (which continue-on-error prevents) AND event == 'push' (but this job fires on schedule). Net result: if a real cron regression lands, the nightly job fails silently — the workflow badge stays green, no issue is created, and nobody is alerted. The PR acknowledges the ~24h detection delay as an acceptable tradeoff, but there is currently no detection path, not even the delayed one.

Consider adding a conditional notification step:

      - name: 'Report nightly failure'
        if: failure()
        env:
          GH_TOKEN: '${{ secrets.GITHUB_TOKEN }}'
        run: |
          gh issue create --title "Nightly cron-interactive E2E failure ($(date +%F))" \
            --label "ci/nightly-failure" \
            --body "See: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"

This preserves continue-on-error (flakes don't block CI) while ensuring genuine regressions surface within ~24h.

— qwen3.7-max via Qwen Code /review

runs-on: 'ubuntu-latest'
if: |-
${{ github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' }}
Comment on lines +150 to +152

@doudouOUC doudouOUC Jul 15, 2026

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] The nightly job runs only on ubuntu-latest, but the PR description and linked issue evidence show the flakes were worst on macOS runners ("failed only on macOS"). A real cron regression that manifests only on macOS (e.g., a platform-specific timing issue in CronScheduler) would go undetected by the nightly job.

Consider adding macOS to the nightly matrix:

    strategy:
      matrix:
        os: [ubuntu-latest, macos-latest]
    runs-on: ${{ matrix.os }}

Or at minimum, document that macOS coverage is intentionally dropped for nightly.

— qwen3.7-max via Qwen Code /review

# This test is inherently timing-flaky (wall-clock cron fire + real model
# latency). Run it nightly only so flakes do not turn push CI red.
# continue-on-error prevents this job from marking the workflow as failed.
continue-on-error: true

@doudouOUC doudouOUC Jul 15, 2026

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] No timeout-minutes on this job. The cron-interactive test depends on wall-clock scheduling (*/1 at minute boundary) + real model latency with a 90s waitForScreen window. If the test hangs (e.g., the cron event never fires), the job runs until GitHub's default 360-minute runner timeout. No other job in this workflow sets timeout-minutes either, but this nightly job is the highest-risk candidate for hangs.

    timeout-minutes: 30

— qwen3.7-max via Qwen Code /review

steps:
- name: 'Checkout'
uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3

- name: 'Set up Node.js'
uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0
with:
node-version-file: '.nvmrc'
cache: 'npm'
cache-dependency-path: 'package-lock.json'
registry-url: 'https://registry.npmjs.org/'

- name: 'Configure npm for rate limiting'
run: |-
npm config set fetch-retry-mintimeout 20000
npm config set fetch-retry-maxtimeout 120000
npm config set fetch-retries 5
npm config set fetch-timeout 300000

- name: 'Install dependencies'
run: |-
npm ci --prefer-offline --no-audit --progress=false

- name: 'Build project'
run: |-
npm run build

- name: 'Bundle CLI for E2E tests'
run: |-
npm run bundle

- name: 'Run cron-interactive E2E tests'
env:
OPENAI_API_KEY: '${{ secrets.OPENAI_API_KEY }}'
OPENAI_BASE_URL: '${{ secrets.OPENAI_BASE_URL }}'
OPENAI_MODEL: '${{ secrets.OPENAI_MODEL }}'
KEEP_OUTPUT: 'true'
VERBOSE: 'true'
run: 'npx cross-env QWEN_SANDBOX=false vitest run --root ./integration-tests interactive/cron-interactive.test.ts'

web-shell-browser-regression:
name: 'web-shell Browser Regression'
Expand Down
Loading