Skip to content

test(ios): isolate smoke contracts and synchronize observable state - #3786

Merged
thymikee merged 12 commits into
mainfrom
t3/stabilize-ios-smoke-tests
Oct 17, 2026
Merged

thymikee merged 12 commits into
mainfrom
t3/stabilize-ios-smoke-tests

Conversation

@thymikee

@thymikee thymikee commented Oct 17, 2026 •

Copy link
Copy Markdown
Member

Summary

Give iOS smoke distinct sessions, declared starting states, and independent subtests. Use one built CLI and fixture controls for depth/WebView setup. Keep deep-link delivery and exact confirmation handling separate; readable misses remain fatal.

Masked XCTests require app-observed complete-suffix delivery and zero toggle presses. Readable fields retain exact replacement and dropped-clear checks. Cleanup captures exhaustion evidence without reissuing commands or losing typed already-clean handling. The prompt test now requires the intended AssertionError.

Scope: 22 files across iOS tests/fixtures, workflow, shared failure recording, and authoring guidance. Rebased onto 6e8c2a046c; preserved upstream documentation. Detailed evidence.

Validation

  • Review-fix source 360ae21bfb: focused 32/32; independent review passed. Prompt mutation: old test 1 false pass, new matcher 1 failure, repaired 1 pass.
  • Historical harness source 316052ba2e: simulator 22/22, 102s, no skips; iPhone 17 Pro/iOS27.0, current Release fixture. Sessions/daemon cleaned; keyboard preference restored. Prior focused suite 49/49; revised native tests 5/5.
  • Original native 92db564a30: masked pair 1 failure/1 pass. Cleanup base 5c7efc83ea: 1 failure → patched 1 pass.
  • pnpm check:affected --run --head HEAD passed at 1067255afada175396fbbf98db710f80938f9d14, exactCommit=true, receipt run-U6l3D6.

CI at 1067255af completed without failures, including iOS 26.2/macOS smoke, Integration Tests, and Coverage. The fixture-build matrix was skipped; Cubic was neutral.

View guided diff Turn on auto-fix

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.38 MB 5.38 MB +832 B
Package (unpacked) 5.38 MB 5.38 MB +832 B
Package (download) 1.62 MB 1.62 MB +122 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.1 ms 27.6 ms -0.5 ms
CLI --help 79.2 ms 78.2 ms -1.0 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 15 files

You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread test/integration/ios-simulator-e2e-deep-link-destination.test.ts
Comment thread test/integration/ios-simulator-e2e/live-webview-remote-content.ts
Comment thread test/integration/ios-simulator-e2e/live-webview-remote-content.ts
@thymikee

Copy link
Copy Markdown
Member Author

At 5c7efc8 one thing should change before this is ready: the unrelated-prompt assert.rejects in ios-simulator-e2e-deep-link-destination.test.ts has no matcher, so it passes for any rejection, not only the prompt-title mismatch it is meant to catch (thread). Adding a matcher for that AssertionError fixes it. I also have a design question below.

Smoke Tests on iOS 26.2 is still queued, not failed. It runs the ios.yml steps, the runner XCTests and the smoke harness this PR changes, so its result at this head is the signal we still need. I read the diff and took the retry-policy mutation result and the local 22/22 and 5/5 runs from the PR body and evidence doc. I did not reproduce them. I know of no conflicts.

Not blocking: live-runner.ts:159 decides whether a scenario owns its launch by comparing three hard-coded scenario ids, and SCENARIO_DEFINITIONS in scenarios.ts already lists every scenario. You can take this or leave it.

Could ScenarioDefinition declare the launch rule (for example setup: 'fixture-home' | 'owns-launch'), so executeLiveScenarios reads that trait instead of comparing ids? For the conformance probe, could it use the built client's own transport (built sendToDaemon and the attach lifecycle) instead of the source-side sendRequest call? I did not check whether the built dist exposes an import path for that, and dist is not built in my worktree. The rest of the split (deep-link journey, in-app navigation for depth and WebView, masked delivery canary) is already about as small as it can be.

Two smaller threads from the other reviewer still apply: one P3 on the delivery assertion in RunnerTests+SynthesizedTextEntryTests.swift:173, and one P3 on the stale "deep link landed" comment. The P2 thread on the /webview URL does not apply, because that URL was a setup handoff and not a declared contract. The webview-remote-content scenario asserts WebView snapshot content, and smoke:deep-link-navigation still covers deep-link delivery, so please resolve that thread.

Next: add the matcher, answer the design question, and let Smoke Tests (iOS 26.2) pass at the new head.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 12 files (changes from recent commits).

You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread test/integration/ios-simulator-e2e/live-harness.ts
Comment thread test/integration/ios-simulator-e2e/live-snapshot-depth-frontier.ts
Comment thread docs/evidence/ios-smoke-contracts-2026-10-17.md
Comment thread test/integration/ios-simulator-e2e/README.md Outdated
@thymikee

Copy link
Copy Markdown
Member Author

The earlier findings from 5c7efc8 are mostly addressed at b78ff23, but one ask is still open, so this needs one more change before merge. CI is green: 18/18 checks pass at b78ff23, including the iOS workflow that runs the changed keyboard step, the runner XCTests and the smoke harness. There are no conflicts.

The unrelated-prompt assert.rejects at test/integration/ios-simulator-e2e-deep-link-destination.test.ts:107 still has no matcher, so any rejection passes. The previous review asked for this one change. Please add a matcher that names the prompt-title AssertionError, so only that rejection passes.

Not blocking, and you can take or leave it: the conformance probe in live-runner.ts:56 imports the built SDK but sends requests through the source-side transport. Your answer that the injected transport has timeoutRecovery:'none' and owns no daemon lifecycle is fine, so I consider that question closed.

On the open inline threads, these still apply: the missing matcher above, and the cleanup failure artifacts thread (allowFailure:true skips the screenshot and report). Three lower-priority threads also still apply. These two do not apply, so please resolve them: the /webview handoff thread, because /webview was a setup handoff and smoke:deep-link-navigation still covers URL delivery; and the README "extension" wording thread, because it means extending the smoke suite. The depth-1 Other node thread also does not apply, because the stricter check is deliberate there.

I did not run the simulator smoke, the focused 49/49 tests or the cleanup base-vs-patch experiment. Those results come from the PR body and the evidence doc. I could not confirm that the 22/22 local run used exactly b78ff23, since the evidence doc names harness source 316052b. I also did not check whether the built dist exposes an importable daemon transport.

@thymikee
thymikee force-pushed the t3/stabilize-ios-smoke-tests branch from b78ff23 to 1067255 Compare October 17, 2026 17:20
@thymikee

Copy link
Copy Markdown
Member Author

Rebased onto 6e8c2a046c and pushed 1067255af with the remaining review fixes. All eight existing inline threads are answered and resolved.

The unrelated-prompt test now requires its intended AssertionError. A mutation confirms the old matcher accepted an unrelated Error, while the new matcher fails it. Cleanup captures screenshot/snapshot/report artifacts after exhausting its own budget, without reissuing the command or losing typed already-clean handling on attempt three. Restored the masked prefilled diagnostic, corrected the WebView comment, and restored serial-command/signature rationale in the authoring guide.

The conformance probe retains the agreed existing-daemon transport (timeoutRecovery: 'none', no daemon lifecycle). Scenario setup is declared in the registry.

Focused checks: 32/32. Independent review: no findings. pnpm check:affected --run --head HEAD passed at 1067255af with exactCommit=true, receipt run-U6l3D6. CI is rerunning at this head; the earlier simulator results remain attributed to their original source in the evidence document.

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed 1067255. The code looks good, and the findings from b78ff23 (#3786 (comment)) are fixed: exhausted cleanup now records its failure evidence, and the deep-link destination test matches the assertion that names the prompt. All 18 checks pass, including the iOS job that runs the changed harness. There are no conflicts.

Not blocking: no test drives cleanupSession (live-harness.ts:117-118), so removing its recordFailure call keeps every test green; a fake runCli that fails close three times could pin it. Also, this commit removes the links to testing.md and test-audit.md from docs/agents/test-authoring.md, which is unrelated to this PR.

The local simulator run in the evidence doc is from harness source 316052b, so at this head only the CI iOS smoke covers the rebased code.

The remaining open threads can be resolved. Fixed at this head: r4303084766, r4303084772, r4303084778, r4303318244, r4303318255 and r4303318258. Not applicable: r4303084769 (smoke:deep-link-navigation still covers URL delivery) and r4303318250 (the stricter Application/Window-only frontier is deliberate for the truncated raw-depth contract).

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 17, 2026
@thymikee
thymikee merged commit 1089dba into main Oct 17, 2026
18 checks passed
@thymikee
thymikee deleted the t3/stabilize-ios-smoke-tests branch October 17, 2026 18:23
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-17 18:23 UTC

thymikee added a commit that referenced this pull request Oct 17, 2026
…me-no-process-globals

* origin/main:
  fix(help): correct input and MCP descriptions, add a first-screen help gate (#3788)
  test(ios): isolate smoke contracts and synchronize observable state (#3786)
thymikee added a commit that referenced this pull request Oct 17, 2026
…ment

* origin/main:
  fix(help): correct input and MCP descriptions, add a first-screen help gate (#3788)
  test(ios): isolate smoke contracts and synchronize observable state (#3786)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant