test(integration): gated ko-KR push-permission live suite (#660)#692
Conversation
Issue #660 plan item: "Add integration test gated by a ko-KR simulator availability check in CI". Encodes the three Exit criteria from the issue body as Jest assertions: 1. app_handle_alert { action: 'dismiss' } returns dismissed: true, strategy: 'ax-scan' 2. visibleButtons contains the localized labels (허용, 허용 안 함) 3. ax-bridge-native dump --debug exits 0 and emits the walker debug events added in #691 The suite is gated behind explicit opt-in (OPENSAFARI_KOKR_PUSH_DIALOG=1 + OSF_DEVICE_ID + OSF_PERMISSION_BUNDLE) because the cold-launch step requires the host shell to have Full Disk Access for the simulator's TCC sandbox so `simctl privacy reset notifications` succeeds. Without the FDA grant, the suite skips with a structured warning instead of failing CI on a host environment issue. See docs/recipes/transient-simctl-errors.md for the FDA setup the suite expects. Default behavior (no opt-in) is a single-test "suite skipped" assertion, so the file integrates cleanly into the existing tests/integration set without needing a CI flag flip. When a focused session has the FDA grant, opting in turns the same file into a binary verification of the issue's Exit criteria. The walker_* event assertions in the second test depend on the diagnostic events landed in #691; opt-in runs against a develop branch that includes #691 will pass cleanly, runs against an earlier develop will hit a partial-fail on those events but only when opt-in is set. Verified locally: - Default-skip run: 1 test passes, prints opt-in skip warning - ESLint clean Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new live integration test suite for issue #660, focusing on the Korean localization of the iOS notification permission sheet. The suite includes logic for resetting notification permissions, launching the target application, and verifying the presence and dismissibility of localized buttons. The review feedback correctly identifies that several critical failure points—such as a failed environment reset, an empty tool response, or the absence of the permission sheet—currently use console.warn and return early. This leads to false positives where the test passes without actually performing the verification. The reviewer recommends throwing explicit errors in these scenarios to ensure test reliability when the suite is opted into.
| if (!reset.ok) { | ||
| console.warn(`[issue-660] ${reset.reason}`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Once the test has opted in via OPENSAFARI_KOKR_PUSH_DIALOG=1, environmental failures like a simctl reset error should result in a test failure rather than a silent skip via console.warn. This ensures that the operator is alerted if the environment they've committed to providing is not actually ready.
if (!reset.ok) {
throw new Error(`[issue-660] ${reset.reason}`);
}| if (!lastBody) { | ||
| console.warn('[issue-660] tool handler returned no body — skipping'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Similar to the reset check, if the tool handler fails to return a body, the test should fail to indicate that the expected interaction did not occur.
if (!lastBody) {
throw new Error('[issue-660] tool handler returned no body');
}| !visibleButtons.includes(ACCEPT_LABEL) && | ||
| !visibleButtons.includes(DISMISS_LABEL) | ||
| ) { | ||
| console.warn( | ||
| `[issue-660] permission sheet did not appear within ${SHEET_WAIT_MS} ms; ` + | ||
| `visibleButtons=${JSON.stringify(visibleButtons)}. ` + | ||
| 'Skipping — likely a build flag or app behavior change.', | ||
| ); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
If the permission sheet does not appear within the timeout, the test has failed to verify the fix for issue #660. Using console.warn and returning allows the test to pass incorrectly. Throwing an error here ensures the test correctly reports a failure when the localized buttons are not found.
if (
!visibleButtons.includes(ACCEPT_LABEL) &&
!visibleButtons.includes(DISMISS_LABEL)
) {
throw new Error(
`[issue-660] permission sheet did not appear within ${SHEET_WAIT_MS} ms; ` +
`visibleButtons=${JSON.stringify(visibleButtons)}.`
);
}![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 319edbc001
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| expect(visibleButtons).toEqual( | ||
| expect.arrayContaining([ACCEPT_LABEL, DISMISS_LABEL]), |
There was a problem hiding this comment.
visibleButtons from app_handle_alert is a diagnostic field that may include whitespace annotations (e.g. "허용 안 함 (norm: 허용 안 함)") when SpringBoard uses NBSP or other non-ASCII whitespace, so asserting exact membership of raw "허용 안 함" will fail even when the correct button is present. In the iOS 26.4 ko-KR scenario this makes the live test report a regression for a passing behavior; compare against normalized/stripped labels before this assertion.
Useful? React with 👍 / 👎.
Address gemini-code-assist (3) and chatgpt-codex-connector (1) review items on PR #692: - gemini@:204, :232, :246: once OPENSAFARI_KOKR_PUSH_DIALOG=1 has been flipped, environmental failures (TCC reset error, empty tool body, missing permission sheet) are setup errors that the operator alone can resolve and must surface as test failures rather than silent console.warn skips. Skip-with-warn behavior is preserved for the default no-opt-in path. - codex@:253: collectVisibleButtonLabels in app_handle_alert annotates non-ASCII whitespace as "<original> (norm: <normalized>)" (slice 2 of #642), and SpringBoard ko-KR permission sheets use NBSP between the syllables of 허용 안 함. Strict equality against the raw constant would falsely fail when the diagnostic suffix is present or when whitespace is U+00A0. Introduces stripDiagnosticAnnotation + normalizeWhitespace + canonicalLabel mirroring the regex from src/tools/app-handle-alert.ts so the assertion compares against canonicalized labels. Verified locally: - Default-skip run passes 1 test with the structured warning - ESLint clean Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9f1599c52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function normalizeWhitespace(label: string): string { | ||
| return label.replace(/\s+/gu, ' ').trim(); |
There was a problem hiding this comment.
canonicalLabel() currently relies on label.replace(/\s+/gu, ' '), but JavaScript \s does not match U+2060 WORD JOINER. collectVisibleButtonLabels can emit labels containing U+2060 (it is explicitly treated as fancy whitespace in src/tools/app-handle-alert-labels.ts), so a value like "허용안함 (norm: 허용 안 함)" is stripped to "허용안함" and never equals DISMISS_LABEL. In that case this live suite reports a false regression even though the expected ko-KR button is present.
Useful? React with 👍 / 👎.
…odex P2) Address chatgpt-codex-connector P2 on PR #692 (commit d9f1599, line 80): JavaScript's `\s` regex class does NOT match U+2060 WORD JOINER, even though `src/tools/app-handle-alert-labels.ts` explicitly treats it as fancy whitespace alongside U+00A0 / U+202F / U+2007 / U+2028 / U+2029. A SpringBoard label like "허용안함" (with U+2060 between syllables) would slip past `\s+` normalization untouched and never equal the plain ASCII-spaced constant `허용 안 함`, producing a false-regression report on a passing behavior. Mirror the FANCY_WHITESPACE codepoint set from the source file 1:1 and strip those characters before applying the standard `\s+` collapse. The escape sequence form is used so the regex character class is unambiguous in source — avoids the editor-level invisibility of literal control characters. Verified locally: - Default-skip run passes 1 test - ESLint clean Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Bot review iteration summaryAll four review items from the initial round on
The skip-with-warn pattern is preserved for the default no-opt-in path only. Once Re-posted bot comments on CI: lint + test + build all green on 🤖 Generated with Claude Code |
1 participant
Summary
tests/integration/issue-660-ko-kr-push-permission.live.test.ts:app_handle_alert { action: 'dismiss' }returnsdismissed: true, strategy: 'ax-scan', andvisibleButtonscontains허용/허용 안 함.ax-bridge-native dump --debugexits 0 and emits the walker debug events introduced in feat(ax-bridge): walker candidate diagnostics on --debug (#660) #691.OPENSAFARI_KOKR_PUSH_DIALOG=1+OSF_DEVICE_ID+OSF_PERMISSION_BUNDLE. Default behavior (no opt-in) is a single "suite skipped" assertion that prints a structured warning. CI does not set the opt-in env var so this PR does not change CI behavior.Why opt-in instead of auto-detect
The cold-launch step requires
xcrun simctl privacy <udid> reset notifications <bundle>, which fails withNSPOSIXErrorDomain code 1("Operation not permitted") on hosts whose shell does not have Full Disk Access for the simulator's TCC sandbox — the same wall the previous autopilot session and this one both hit. Auto-running the suite on a non-FDA host would surface the simctl error as a hard test failure even though the failure is environmental, not regression-driven.The opt-in env var doubles as a contract: the operator who flips it accepts responsibility for ensuring the host environment is FDA-granted (recipe in
docs/recipes/transient-simctl-errors.md).Dependency on PR #691
Test B asserts the presence of
walker_app_windows_enumerated,walker_top_candidates, andwalker_overlay_roles_seenevents. Those events ship in #691. Opt-in runs against a develop branch that includes #691 pass cleanly. Opt-in runs against an earlier develop will hit a partial-fail on Test B's event-name assertions; default-skip runs are unaffected because the assertions only execute inside the opt-in branch.Test plan
npx jest tests/integration/issue-660-ko-kr-push-permission.live.test.ts --runInBandpasses 1 test with the structured skip warning🤖 Generated with Claude Code