Skip to content

Move the mobile E2E harness out to the kilo-workflow repo - #4968

Merged
iscekic merged 3 commits into
mainfrom
fix/simulator-stale-claim-leak
Aug 3, 2026
Merged

Move the mobile E2E harness out to the kilo-workflow repo#4968
iscekic merged 3 commits into
mainfrom
fix/simulator-stale-claim-leak

Conversation

@iscekic

@iscekic iscekic commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Symptom

A weekend of workflow runs left the machine overloaded: orphaned Appium servers and a simctl recordVideo process alive for over two days, plus simulators that nothing could power off.

Cause

dev/local/mobile-simulator.ts existed in both this repo and kilo-workflow, drifted by 152 lines, each copy carrying fixes the other lacked. Both wrote the same claim files in the same machine-global $TMPDIR/kilo-mobile-simulator-claims.

The leak itself: a claim whose worktree is gone was discarded without powering the device off. The next claim saw a Booted device, recorded bootedByClaim: false, and release never shut it down — the simulator stayed booted forever. Fixing that once meant fixing it twice, in two places that kept diverging.

Change

E2E is agent tooling, not app code, so it moves out. apps/mobile/e2e/ and the simulator CLI now live in the kilo-workflow repo as mobile_e2e/, which is the single owner. The leak fix itself lands in kilo-workflow, not here.

  • Delete apps/mobile/e2e/ (22 files) and the duplicated mobile-simulator.ts + test.
  • Drop the dev:mobile:simulator script, and the appium/webdriverio deps and catalog entries — nothing here imports them now.
  • Split dev/local/mobile-workflow.test.ts: the 15 flow and preflight assertions move with the harness; the 7 app and dev-tooling assertions stay.
  • apps/mobile/AGENTS.md no longer points at an e2e runbook.
  • dev/seed/app/api-token.ts drops its path reference in an error message.

One deliberate coupling left

dev/local/mobile-ios-build.ts still reads the claim directory to prove a device belongs to this worktree. pnpm dev:mobile:ios build <udid> stays a cloud command that the harness calls, so the read is correct. The record shape is now a contract owned by kilo-workflow, and a comment says so.

Verification

  • pnpm test:mobile-workflow — 100 pass, 0 fail
  • pnpm test:dev-local — 184 pass, 0 fail
  • Harness smoke-tested from this worktree after the move: mobile_e2e/appium.sh <device> server status resolves the cloud worktree, re-execs through the harness tsx/process-lock, and answers.

Review note

Merge this together with the kilo-workflow commit. Until both land, mobile_e2e/preflight.sh and the role definitions point at paths that only exist on one side.

`apps/mobile/e2e/` and `dev/local/mobile-simulator.ts` now live in the
kilo-workflow repo as `mobile_e2e/`. Nothing in this repo drove them: they are
agent tooling, not app code.

The simulator claim CLI existed in both repos and had drifted 152 lines, each
copy carrying fixes the other lacked, while both wrote the same claim files in
the same machine-global temp directory. A stale-claim bug that leaves booted
simulators running forever had to be fixed twice. One owner removes that class
of problem; the fix landed in kilo-workflow.

- Delete apps/mobile/e2e/ (22 files) and the duplicated simulator CLI.
- Drop the dev:mobile:simulator script and the appium/webdriverio deps and
  catalog entries; nothing here imports them now.
- Split dev/local/mobile-workflow.test.ts: the flow and preflight assertions
  move with the harness, the app and dev-tooling assertions stay.
- apps/mobile/AGENTS.md no longer points at an e2e runbook.

dev/local/mobile-ios-build.ts still reads the claim directory to prove a device
belongs to this worktree. That directory is now a contract owned by
kilo-workflow, and the code says so.

test:mobile-workflow 100 pass, test:dev-local 184 pass.
@iscekic iscekic self-assigned this Aug 3, 2026
iscekic added 2 commits August 3, 2026 12:21
pnpm install --frozen-lockfile failed with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH:
the catalogs block still named appium and webdriverio after they left
pnpm-workspace.yaml. Every CI job died at the install step.

Drops the appium and webdriverio dependency trees, 1739 lines.
Splitting the harness tests out left a stray blank line that oxfmt rejects.
@kilo-code-bot

kilo-code-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

This is a deletion-only harness move plus small doc/comment edits; I verified the cloud repo retains no dangling references to the removed harness and that the dependency/catalog removals are internally consistent, with high confidence.

Verification performed

  • No remaining references anywhere in the repo (excluding pnpm-lock.yaml) to apps/mobile/e2e, mobile-simulator.ts, dev:mobile:simulator, appium-wrapper, appium, or webdriverio. The only Appium mentions left are prose comments in apps/mobile/src/components/agents/chat-composer.tsx and message-bubble-a11y.test.ts, and the only kilo-mobile-simulator-claims use is the intentional read in dev/local/mobile-ios-build.ts.
  • pnpm-lock.yaml no longer resolves appium/webdriverio, matching the removed root devDependencies and pnpm-workspace.yaml catalog, minimumReleaseAgeExclude, and allowBuilds entries. geckodriver: true is correctly retained for apps/extension Firefox e2e.
  • test:mobile-workflow now lists only test files that still exist; test:dev-local uses a glob, so the deleted appium-wrapper.test.ts / mobile-simulator.test.ts do not break it. No CI workflow referenced the deleted paths or the removed script.
  • The trimmed dev/local/mobile-workflow.test.ts drops exactly the assertions that read apps/mobile/e2e/** files; the remaining assertions read only files that still exist in this repo.
  • No new imports, runtime code paths, or subscriptions were added, so there is no new memory-leak surface.

Assumptions / notes for the author

  • validateSimulatorClaim in dev/local/mobile-ios-build.ts:663 still hard-fails when no claim record exists, and this repo no longer contains a writer for $TMPDIR/kilo-mobile-simulator-claims. So pnpm dev:mobile:ios build <udid> is now only usable when driven by the kilo-workflow harness. The PR description states this coupling is deliberate and the added comment documents the contract, so I did not flag it, but it does make the command unusable standalone from this repo.
  • Per the PR's own review note, this must land together with the kilo-workflow commit; I could not verify the other side of the move.
Files Reviewed (32 files)
  • apps/mobile/AGENTS.md
  • dev/local/mobile-ios-build.ts
  • dev/local/mobile-workflow.test.ts
  • dev/seed/app/api-token.ts
  • package.json
  • pnpm-workspace.yaml
  • pnpm-lock.yaml (generated)
  • apps/mobile/e2e/** (22 files, deleted)
  • dev/local/mobile-simulator.ts, dev/local/mobile-simulator.test.ts, dev/local/appium-wrapper.test.ts (deleted)

Reviewed by claude-opus-5 · Input: 50 · Output: 9.8K · Cached: 1.5M

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic requested a review from jeanduplessis August 3, 2026 10:29
@iscekic
iscekic merged commit 128e1fb into main Aug 3, 2026
72 checks passed
@iscekic
iscekic deleted the fix/simulator-stale-claim-leak branch August 3, 2026 10:42
iscekic added a commit that referenced this pull request Aug 3, 2026
Main moved the mobile E2E harness out to the kilo-workflow repo (#4968).
Accept every deletion: the harness files, dev/local/mobile-simulator.ts,
and the harness tests in dev/local/mobile-workflow.test.ts.

The branch's harness fixes are already in kilo-workflow. Verified by
diffing each file against its kilo-workflow copy; only path and
relocation lines differ.

apps/mobile/e2e/github-stub.sh was added on this branch after main
deleted the directory. Drop it too; mobile_e2e/github-stub.sh in
kilo-workflow is a superset.
iscekic added a commit that referenced this pull request Aug 3, 2026
main moved `apps/mobile/e2e/` and `dev/local/mobile-simulator.ts` to the
kilo-workflow repo in #4968. This branch had modified nine of those files and
added `apps/mobile/e2e/github-stub.sh`.

Accept the deletion for every moved path. The harness edits belong in
kilo-workflow now, not here.

`dev/local/mobile-workflow.test.ts` takes main's split version: both of this
branch's edits to it asserted on the moved flow files.
iscekic added a commit that referenced this pull request Aug 3, 2026
main moved the mobile E2E harness to the kilo-workflow repo (#4968) and
merged the harness refactor (#4960). This branch still carried both. Accept
main's deletions:

- apps/mobile/e2e/** — every change is already in kilo-workflow's mobile_e2e.
- dev/local/mobile-simulator.ts — kilo-workflow's copy is ahead of this one.
- dev/local/mobile-workflow.test.ts — keep main's non-e2e tests only.
- dev/seed/app/github-integration-copy.ts — kilo-workflow's
  github-installation.sh replaced it with the githubApps.devAddInstallation
  mutation main already ships.

What remains is the composer wrap fix plus one process-lock test.
iscekic added a commit that referenced this pull request Aug 3, 2026
main moved the mobile E2E harness to the kilo-workflow repo (#4968). This
branch had modified eight of the deleted files plus the simulator claim CLI.
Every one of those changes already exists in kilo-workflow, so the merge takes
main's deletion.

- Delete apps/mobile/e2e/ and dev/local/mobile-simulator.ts.
- Delete apps/mobile/e2e/github-stub.sh: kilo-workflow holds a newer copy.
- Delete dev/seed/app/github-integration-copy.ts: main dropped the seed tool
  and kilo-workflow replaced it with mobile_e2e/github-installation.sh.
- Take main's dev/local/mobile-workflow.test.ts: the harness assertions moved
  with the harness.

dev/local/tmux.ts and dev/local/mobile-android.ts now match main exactly, so
this branch changes only apps/mobile/src.
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.

2 participants