Skip to content

test: strengthen CLI and mobile behavior checks - #33526

Merged
OrcaWin merged 2 commits into
mainfrom
test-audit/364-cli-mobile-behavior
Oct 25, 2026
Merged

OrcaWin merged 2 commits into
mainfrom
test-audit/364-cli-mobile-behavior

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​171 $\color{#cf222e}{\Huge{\mathbf{−}}}$​136 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​35
Prod 1 0 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1

Several CLI and mobile tests accepted unrelated handler failures, mocked their own session migration, or relied on unrealistic persistence fixtures. Repair 18 cases and remove three duplicate or inventory declarations across nine test files.

CLI tests now require successful caller dispatch, coordinate timeouts with actual socket receipt, and check skill links with platform-aware path semantics. Mobile tests exercise actual stable-session cutover and withdrawal, verify stale socket closure, and retain keyed storage while checking host edits and probe cleanup. Prune the two obsolete host-edit wall-clock findings from the ratchet baseline. Application code is unchanged.

Validation: independently checked original/candidate runs, retained-case witnesses, real-owner fault controls and sealed source/support evidence for 14 reviewed files (135 original declarations). Current scoped formatting, lint and the changed flaky-test ratchet pass; full fresh PR CI supplies aggregate validation before merge. The installed local pnpm wrapper cannot complete the required aggregate command, and no dependency/native builds were run locally. Linux runtime evidence uses explicit native substitutes; upstream Hermes capability data drift was reviewed separately and is validated against current code by CI.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 09afb374-950d-4f6a-8005-343a1e57c719
Commits

Reviewing files that changed from the base of the PR and between b12b703 and 9905553.

Files selected for processing: 1
  • config/flaky-test-pattern-baseline.txt
Files with no reviewable changes: 1
  • config/flaky-test-pattern-baseline.txt

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The pull request updates mobile transport and CLI tests. Mobile tests add checks for socket and relay state, scheduled recovery, request routing, listener cleanup, and pending timers. Lifecycle tests replace polling with fake-timer advancement and use mutable secure-store mock data. CLI tests strengthen caller-flag and runtime-timeout assertions, adjust symlink expectations by platform, and remove a skills-selector output test.

Priority: Low

Merge Risk: Minimal · up to 99055

The stale test baseline entry has been removed. No remaining merge-blocking issue is identified in this change.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Description check Warning The description explains the main test changes and validation results, but it omits most required template sections, including the linked issue, visual proof or an N/A explanation, testing checkboxes,… Update the description to use the repository template. Add the required issue link, visual proof or an explicit N/A reason, testing details and checkboxes, AI disclosure if applicable, review and agent-boundary sections, notes, and the chec…
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly summarizes the main change: strengthening CLI and mobile behavior tests.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the main test changes and validation results, but it omits most required template sections, including the linked issue, visual proof or an N/A explanation, testing checkboxes, review notes, agent boundary, notes, and checklist.

Resolution

Update the description to use the repository template. Add the required issue link, visual proof or an explicit N/A reason, testing details and checkboxes, AI disclosure if applicable, review and agent-boundary sections, notes, and the checklist. Keep the existing change summary and validation details.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ef1d2874-11e2-4e17-9d0a-c2590b79a417
Commits

Reviewing files that changed from the base of the PR and between 526ffe3 and b12b703.

Files selected for processing: 9
  • mobile/src/transport/foreground-stale-dial-restart.test.ts
  • mobile/src/transport/mobile-direct-endpoint-probe.test.ts
  • mobile/src/transport/mobile-endpoint-lifecycle-host-edit.test.ts
  • mobile/src/transport/mobile-endpoint-supervisor-direct-probe.test.ts
  • mobile/src/transport/mobile-endpoint-supervisor-nudge.test.ts
  • src/cli/orchestration-session-caller-cli.test.ts
  • src/cli/runtime-client.test.ts
  • src/cli/skills-install.test.ts
  • src/cli/skills-reference-selector.test.ts
Files with no reviewable changes: 1
  • src/cli/skills-reference-selector.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

lifecycle.stop()
finishCredentialWrite()
await new Promise((resolve) => setTimeout(resolve, 0))
await vi.advanceTimersByTimeAsync(0)

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.

Major · Maintainability & Code Quality · Quick win

Prune the wall-clock test ratchet baseline.

After this PR replaces real-time waits with fake-timer advancement, the required validation job fails. The test-no-wall-clock count is 0, but its checked-in baseline is 2. Run pnpm check:flaky-test-pattern-ratchet --prune and include the baseline update so the job can pass.

Source: Pipeline failures

@OrcaWin
OrcaWin merged commit 7d18e19 into main Oct 25, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant