fix(mobile): bump app version to 1.0.4 - #4794
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe latest commits reverted every shared e2e-harness file, so the net PR diff against Verification performed in this pass
Files Reviewed (1 file)
Assumptions: review was static and read-only — no tests were executed and CI status at HEAD was not read. The version constant is assumed to be consumed by EAS build metadata only, with build numbers auto-incremented outside this file; the reverted dev-tooling changes are assumed to land via #4960 as the PR description states. Previous Review Summaries (6 snapshots, latest commit f79b0bd)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit f79b0bd)Status: No Issues Found | Recommendation: Merge Executive SummaryThe branch was force-pushed to Verification performed in this pass
Files Reviewed (5 files)
Assumptions: review was static and read-only — no tests were executed and CI status at HEAD was not read; local-only dev tooling under Previous review (commit 2b3f4e2)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of Verification performed in this pass
Files Reviewed (1 file in this increment)
Note: assertion/flow consistency was determined statically (read-only review; no tests were executed). CI status at HEAD was not read. Previous review (commit 135c1d4)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe Overview
Issue Details (click to expand)CRITICAL
Verified in this pass (3c1fda3..135c1d4)
Files Reviewed (10 files)
Note: incremental review of Fix these issues in Kilo Cloud Previous review (commit 3c1fda3)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the previous review ( Verified in this pass
Files Reviewed (1 file)
Note: incremental review of Previous review (commit 5db9a3c)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe PR grew from a version bump into the mobile E2E harness; the highest-risk item is Overview
Issue Details (click to expand)WARNING
SUGGESTION
Verified and not flagged
Files Reviewed (15 files)
Note: the previous review covered only Fix these issues in Kilo Cloud Previous review (commit 6fe409c)Status: No Issues Found | Recommendation: Merge Executive SummarySimple, isolated version bump in Files Reviewed (1 files)
Reviewed by claude-opus-5 · Input: 32 · Output: 4.9K · Cached: 924K Review guidance: REVIEW.md from base branch |
…olerant flows - rewrite e2e/AGENTS.md in simplified technical English: task-structured, scripts own the mechanics, 410 -> ~215 lines - new e2e/github-stub.sh: PR-review stub server + env line + token seed as one reversible command pair; failed starts clean up after themselves - emulator-start --wait retries a failed launch once itself (boot-envelope timeout keeps the GPU, other failures switch to software rendering); the runbook's manual GPU decision table is gone - appium.sh hierarchy writes to a file and prints the path; a raw XML dump into an agent session is large enough to kill it silently - raise iOS cold-launch/settle budgets to 120s (timed out under parallel dual-bundle load); document ANR-under-load handling - simulator claims record originalDeviceName and bootedByClaim intent in the initial write, so a lost post-boot rewrite can no longer leak a renamed or still-booted device on release
- emulator-start retry: rethrow the already-exists guard (a retry would tear down a live emulator) and keep attempt 1's error when the interim teardown itself fails - github-stub: curl transport failures now route through seed_fail, so a failed start still rolls back the session and env line - github-stub: guard against a .env.local without a trailing newline before appending, so the marker line can never merge into (and later delete) an existing variable
- tmux targets use exact-match (=name): bare -t prefix-matches, so a sibling worktree whose slug extends ours could be probed or killed in its place - simulator claims write atomically (temp + rename/link): in-place truncation let an unlocked reader see a partial file, judge it corrupt, and delete a live claim - a claim abandoned after its boot (rename failure) now powers the device off, or keeps the claim when even that fails - simctl bootstatus is bounded (15 min) so a wedged CoreSimulator cannot block a claim forever - emulator-start tears down after a failed second attempt too - github-stub start claims its port machine-globally and verifies its own session survived, so two worktrees cannot share one stub silently; stop releases the claim - appium: driver install re-checks inside the install lock; server startup verifies port ownership before adopting a listener; stop escalates to SIGKILL instead of orphaning a live server - record.sh serializes start/stop per device with the shared process lock - open-app consumes its launch budget in 30s slices so an ANR dialog is answered promptly instead of after the full wait - login.sh matches the recipient literally, not as a regex - hierarchy mktemp template puts Xs last (BSD mktemp leaves embedded Xs literal, making every bare call after the first fail) - AGENTS.md: per-platform mixed fixtures, pull-and-grep for ADB dumps, stub-stop token residue note - mobile-workflow tests updated for the raised launch/settle budgets
- appium: the recorded-server fast path now verifies the recorded pid owns the listener before adopting (a recycled pid plus a sibling's server on the same port answered /status and got adopted) - github-stub: seeding extracted into seed_token; new 'seed <email>' subcommand adds a token row for a second signed-in account (the other platform's verifier) without touching the running stub - github-stub: the port claim covers only the choose-to-bind window and an EXIT trap releases it, so an interrupted start cannot leak claims and erode the port range - github-stub: githubUserId carries the pid — concurrent seeds in the same second cannot collide on the unique github_user_id - AGENTS.md: the ADB dump example creates its own mktemp file instead of assuming $SCRATCH
devSeedUserGithubToken can answer 200 with success:false (row not upserted); the status code alone reported a seed that never landed.
A process that survives even SIGKILL kept its state file removed and the stop reported success; the caller then released device claims over a live server. stop now fails and keeps the state in that case.
- claim_port reaps an orphaned port claim (older than 120s with nothing listening) and the start path traps INT/TERM/HUP besides EXIT, so a killed start can no longer shrink the port pool permanently - githubUserId derives deterministically from the account email: the upsert's setWhere only updates a matching row, so the fresh random id made every re-seed of an already-seeded account report success:false and tear the stub down with a misleading hint - lsof ownership probes tolerate no-match exits; pipefail otherwise killed appium.sh before the fresh-start fallback could run
…ling - appium startup adopts a port only on proven ownership (listener pid equals ours); an empty lsof keeps polling instead of adopting a vanished or foreign server - github-stub signal traps exit instead of continuing a start that just released its port claim - success:false with 200 and no error means an account seeded under an older random id — keep that usable row instead of tearing the stub down
success:false can come from an older random-id seed or from a real GitHub connection on the dev account; the keep-message now says so instead of implying only legacy seeds, and notes the stub accepts any token.
…lback - a kept token row is verified to be the stub's (getUserAuthorization githubLogin == kilo-stub-user); a real GitHub connection row fails with a drop-the-row hint — under the stub its refresh bypasses GITHUB_API_BASE_URL, hits real github.com, and revokes itself - start rolls back everything (session, env line, state, port claim) via one EXIT trap when it dies before the final state write, so an aborted start no longer blocks the next one behind 'session already runs' - appium ownership checks degrade to pid+status when lsof is missing or cannot attribute the socket, instead of killing healthy servers across 50 port blocks
- the row probe checks HTTP status and error envelope before advising: a failed probe reports 'could not determine the existing row' instead of confidently telling the operator to delete a row it never examined; a kept row must also be connected (a revoked stub row gets the drop hint) - start's rollback undoes only what this run did (session-created and env-added flags), so a start losing the session-creation race cannot kill the winner's session, env line, or state - the recorded-server reuse path applies the same blind-lsof rule as the start loop: adopt on pid+status after three blind probes instead of orphaning a healthy server and consuming a port block per invocation - drop dead SEED_FAIL_MODE and the stale random-id comment
- github-stub creates its session under a pid-unique name armed before the create and renames to claim the canonical one: a signal at any instant tears down only our own session, and a lost rename race touches nothing but our port claim; the env-added flag arms before the append for the same reason - the appium reuse probe rechecks pid liveness and /status on every pass, so a server dying mid-probe is cleaned up (stop_server) instead of adopted; a foreign listener still only drops our state
…windows Concurrent stub commands raced over the session name, env line, and state dir; each guard narrowed the window without closing it. The shared process-lock (the appium.sh pattern) removes the class: one stub command per worktree at a time, so the entry check is authoritative, the rollback flags only order against signals, and the unique-name rename dance goes away.
Both row branches run after the account answered the seed call, so the sign-in hint contradicted what the script had just proved.
- every network call under the stub lock carries --max-time 30, so a hung local API cannot hold the lock to its 300s wait bound; status runs unlocked (read-only) and answers during a long start - a failed session create (a survivor of a SIGKILLed lock wrapper got there first) resets the created flag before exiting, so cleanup cannot touch the survivor's session - align the env-append comment with the lock (the rename step it cited is gone)
pnpm dev:seed app:github-integration-copy <email> finds the newest real integration in the shared dev database (fakes excluded: kilo-stub* logins and far-future expiries — a real refresh token expires within months), decrypts it with git-token-service's local dev key, re-encrypts for the target account under a synthetic github_user_id (the pair index is unique), and upserts. Scenarios that need a real integration (cloud agents) run against the copy; with no valid donor the command fails with an explicit BLOCKED message, and the runbook tells verifiers to report VERIFICATION BLOCKED rather than fake or silently skip. Verified against the dev db: copy mechanics end to end, and the blocked path on a database holding only seeded rows.
- donor filter excludes 13-digit synthetic github_user_ids (stub seeds and earlier copies): revocation lands only on the real row, so a surviving copy could hand out dead credentials as a fresh success - app:github-integration-copy --remove <email> deletes the account's token row, so an account can return to stub use; runbook and usage text now require separate accounts for stub and integration scenarios - verified live: remove clears the copied row and is idempotent
One instruction per sentence in the integration and stub sections; no rule changed meaning.
- the donor's access token is checked against api.github.com before the copy: database metadata cannot prove a rotated or GitHub-revoked token dead, so silent dead-credential copies reported success - the copy row carries a dummy, already-expired refresh token: GitHub refresh tokens rotate on use, so a refreshing copy would kill the donor's stored refresh token and poison every later copy - usage and runbook text updated to the access-token-only contract
The copy relocates real credentials, so every destructive path is now guarded and every shared-grant hazard is stated: - assertLocalDatabaseTarget: USE_PRODUCTION_DB=true and non-loopback hosts are refused, making the dev-only claim enforceable (verified live) - --remove deletes only a synthetic row (copy or stub seed) and names what it removed; a real connection is refused with instructions - a copy refuses to overwrite a real connection on the target account - donor lookup also matches google_user_email (normalized_email is nullable) - usage and runbook state the real blast radius: the copy shares the donor's OAuth grant, so disconnect on a copied account revokes the DONOR's authorization and no re-copy repairs it — disconnect scenarios are blocked on copied accounts; commit attribution carries a synthetic id - stub status reports 'starting' when a session has no state yet, instead of claiming up for a stub that is not usable - lock-bounded curls raised to 120s (30s was the budget this PR already found too small under full parallel load) - a failed tmux create distinguishes a concurrent session from a tmux error
A dead database dumped a driver AggregateError stack trace, and a dead network read as 'no valid donor'. Both now fail with one line naming the command that fixes them, so an agent does not spend a round guessing at ECONNREFUSED output or chasing a false BLOCKED. Verified live with postgres stopped.
- the synthetic-only rule is re-asserted in the upsert's setWhere, so a real OAuth callback landing between the pre-check and the write cannot be overwritten; zero updated rows reports that explicitly (postgres predicate verified against synthetic and real id shapes) - donors need >10 minutes of access-token life (twice git-token-service's 5-minute refresh buffer): a token inside the buffer passes the live probe, then triggers a refresh the copy cannot perform - the target lookup tries the exact sign-in email before the normalized fallback, since normalized_email is not unique
…dget - the 'can never refresh' claim was wrong: git-token-service gates refresh on the ACCESS token's expiry, so the dummy refresh token IS sent and the copy ends up revoked. Docs, usage text and the runbook now describe that end state, the tool reports usableUntil, and donors need 35 minutes of access life so a copy is usable for at least 30 - --remove reports what actually went away (.returning()), so a row replaced mid-command cannot read as removed - stub lock --wait raised to 900s, above the ~8 minute worst-case hold the 120s per-call budgets allow, so a queued stop or seed waits instead of failing - drop the now-unused 'or' import
The 35-minute SQL margin is measured before up to ten 15-second GitHub probes, so a copy could return with less than the promised 30 usable minutes. The selected donor is now re-checked against the clock after its probe, and one helper owns the usable-until arithmetic the filter, the check, and the report all share.
A campaign verifier died with no sentinel right after a doctor --json dump (the whole environment, PATH included). The runbook now states the general rule — bound anything that can print a lot — and names --json diagnostics alongside the Appium and hierarchy cases it already covered.
…1.0.4 # Conflicts: # apps/mobile/app.config.ts
2b3f4e2 to
f79b0bd
Compare
PR #4960 carries these changes and merges first. This branch keeps no copy. - dev/local/mobile-android.ts - dev/local/mobile-simulator.ts - dev/local/tmux.ts - dev/seed/app/github-integration-copy.ts
Bumps mobile app version to 1.0.4.
Workflow final report (gate-mobile-version-0ad2)
COMPLETE
PR: #4794
Head: 3c1fda3
What changed
origin/mainintochore/mobile-version-1.0.4; resolved the soleapps/mobile/app.config.tsconflict as main's full file withversion: '1.0.4'.origin/mobile-e2e-stecleanly, as required.Validation
pnpm exec oxfmt --check apps/mobile/app.config.tspassed.dev/seed/app/github-integration-copy.ts; both passed.git diff --checkpassed.No Issues Foundand recommends merge.GATE OK (mechanical items)with mergeability MERGEABLE, clean state, zero unresolved threads, assigneeiscekic, and labelhuman-ready.E2E
Assumptions used
Kilobot / CI history