[kangfenmao]

What this PR does

Before this PR:

Hosted macOS development startup could exceed the 60-second readiness budget. Git Bash stripped TZ=Asia/Shanghai before launching the Windows controller, and expectVisible failed in strict mode when a selector matched several elements.

After this PR:

Both branch and tag modes have a 120-second readiness budget. The controller restores an absent TZ to Asia/Shanghai before reporting and native application launch, preserving explicitly supplied timezones. expectVisible checks the first match, consistent with expectText. The controller README documents these contracts.

Fixes #22069

Why we need it and why it was done in this way

The following tradeoffs were made:

• Keep one startup budget without adding a dispatch input.
• Set the missing timezone in the native Node controller: another workflow environment declaration would still be stripped by Git Bash. The existing native launch chain passes the environment to the application; actual application timezones remain measured.
• A hidden first match still fails even when later matches are visible. Actions and waitFor keep their unique-match requirements.

The following alternatives were considered:

A branch-only timeout increase and clearer strict-selector errors were considered; the requester selected a uniform 120-second budget and first-match visibility assertions. Changing the runner's operating-system timezone would unnecessarily broaden the repair.

Links to places where the discussion took place: #22069

Agent Note / Spec

Agent Note: N/A — the requester explicitly excludes .agents/notes/implemented/ from this PR. The behavior and usage contracts are documented in scripts/e2e/ondemand/README.md.

Spec PR: N/A — focused follow-up bug fix with requester-confirmed behavior.

Acceptance criteria: Both startup modes use 120 seconds; missing TZ is restored before reporting/application launch while explicit TZ is preserved; visibility assertions check the first match. Local verification passes; hosted Windows/macOS acceptance remains pending.

Verification

• Regression — pnpm test:scripts scripts/e2e/ondemand/__tests__/cli.test.ts scripts/e2e/ondemand/__tests__/steps.test.ts scripts/e2e/ondemand/__tests__/environment.test.ts scripts/e2e/ondemand/__tests__/workflow.test.ts scripts/e2e/ondemand/__tests__/report.test.ts: 5 files / 22 tests passed. The new missing-TZ subprocess test reproduced the failure before repair; explicit UTC is preserved.
• Regression — real Chrome page: reproduced the old strict-mode failure, then verified multiple visible matches pass, a hidden first match fails, and no match fails.
• Regression — real isolated macOS Electron branch/clean smoke with parent TZ removed and requiredTimezone=Asia/Shanghai: exit 0, three steps, readiness 16.165 seconds; inherited/controller/main/renderer timezone fields all report Asia/Shanghai. Chinese UI verification passed. Agent-owned Electron/runner stopped and port 9222 released.
• pnpm typecheck:e2e, scoped pnpm exec oxlint --deny-warnings, pnpm exec eslint, pnpm exec oxfmt --check, pnpm docs:check, and git diff --check: passed.
• pnpm change:scope --base origin/main, pnpm docs:affected --base origin/main, and pnpm check --plan: inspected. No documentation source prefixes matched; the owning controller README was updated. No full local gate was run.
• Local evidence: .context/qa22069/report.md and .context/cherry-electron-dev/issue-22069/run/report/. These are local artifacts, not committed files.

Breaking changes

No application-facing changes. On-demand QA's startup limit increases from 60 to 120 seconds for both modes. expectVisible now checks the first matching element. No dispatch input or result schema change.

Special notes for your reviewer

Hosted Windows/macOS acceptance is pending. Local macOS verification does not establish Windows Chromium timezone behavior, release-installer behavior, or startup taking longer than the former 60-second budget. Validate the controller branch via qa-ondemand.yml before or after merge; the workflow already exists on main.

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

• Branch: This PR targets main
• PR: The PR description is expressive enough and will help future contributors
• Code: Write code that humans can understand and Keep it simple
• Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
• Upgrade: Impact of this change on upgrade flows was considered and addressed if required
• Documentation: A user-guide update was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior.
• Self-review: I have reviewed my own code (e.g., via /gh-pr-review, gh pr diff, or GitHub UI) before requesting review from others

Release note

NONE

[cherry-qa]

Reviewed at head dfd3818 (QA toolchain change, not a product change).

What the change does: the on-demand controller now defaults TZ to Asia/Shanghai before it builds the report or launches the app (so a Git Bash launch on Windows stops dropping it), the application readiness budget goes 60s -> 120s for both branch and tag modes, and expectVisible now asserts on the first match only.

The three edits are internally consistent: the README describes all three behaviours, the readiness message carries the new number, and the timezone default runs before the report object is created, so the recorded inherited value reflects the default. environment.timezone is resolved at call time from Intl, so it sees the defaulted value.

Two things I would like before merge:

1. A plan-level test for the documented expectVisible rule: a selector that matches several elements where the first match is hidden must fail. Today the README states the rule but only the happy path is exercised, so a future refactor could quietly go back to matching any node.
2. Confirmation that the readiness increase still fits the resolved job timeout in tag mode, where the controller and the application startup both run inside the same step. A 120s readiness budget plus plan steps is close to the previous worst case, and I would rather see the number than assume it.

Nothing else blocking. I did not run the controller locally for this review; the checks above come from the diff and the files at this commit.

[cherry-pr-review]

Technical review supports approval at dfd38183cf0cf696ca37ccb8c3baaf73b1e070f3. No confirmed actionable regression was found in the timezone default, shared 120-second readiness budget, or documented first-match visibility behavior.

Head CI: basic-checks, general-test, package-test, render-test, and changes succeeded; main/renderer/platform matrix jobs were skipped. The synthetic merge has no check runs; legacy pending statuses contain zero contexts.

Lead validation was static; no local test suite was run. One independent reviewer reports successful base/head CLI probes. Hosted Windows timezone behavior, release installation, startup beyond 60 seconds, and browser visibility edge cases were not independently exercised successfully. These remain coverage limitations.

Cherry Review · Passed

no findings

[cherry-qa]

Tested at head dfd3818 (tooling change, not a product change).

The timezone fix works: with TZ unset the controller records and launches with Asia/Shanghai, and an explicit TZ (UTC, America/New_York) is kept exactly as supplied. Invalid dispatches are still rejected before a runner is used.

What I ran locally at this head:

Check	Result
initialize, TZ unset	pass — recorded inherited and controller timezone are both Asia/Shanghai
initialize, TZ=UTC	pass — both UTC
initialize, TZ=America/New_York	pass — both America/New_York
resolve with platform=linux, mode=bogus, profile=bogus, spec=../escape, unknown command	pass — rejected with exit 2, stage input
resolve with timeout_minutes 0 and 99999 (branch)	pass — rejected with the 15–360 message
scripts/e2e/ondemand tests at this head	pass — 7/7

Two things I would still like before merge:

1. expectVisible has no test for the rule you documented (several matches, the first one hidden and a later one visible must fail). The added test only covers the timezone default, so a later change to that .first() call can silently bring back the old whole-selector behaviour.

2. Tag mode now has a 120-second readiness budget, but a tag run accepts timeout_minutes: 1. Readiness happens inside the job step whose timeout is that value, so a 1–2 minute tag dispatch gets killed by the job timeout before the controller can write its report — you would get no result instead of an explicit "application not ready". Either raise the minimum tag timeout to 3 minutes or state in the README that very short tag timeouts cannot observe the readiness budget.

Nothing else blocking.