Skip to content

fix(mobile): settle the new session branch row and explain an unreachable repository - #8729

Merged
iscekic merged 8 commits into
mainfrom
drain/new-session-branch-start-states
Oct 24, 2026
Merged

iscekic merged 8 commits into
mainfrom
drain/new-session-branch-start-states

Conversation

@iscekic

@iscekic iscekic commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Evidence

Evidence: demonstrated — on the local dev stack with an owned fake account on the Android emulator: with the selected repository's branch request held open so the server could not answer it, the branch row showed a single loading node and then, past the settle bound, a single retry row with no loading node anywhere in the hierarchy; pressing Retry issued a fresh branch request against the backend; Start session on a repository the connected installation does not own returned the runtime's classified integration_mismatch and the composer showed the actionable repository guidance; Start session on the installation's own repository created the session and opened its detail.

What changed

Branch row: one settled state, and loading is bounded (apps/mobile)

  • resolveBranchSectionState is a pure resolver over the branch query's flags and returns a single discriminant: bitbucket-organizations-only, loading, unavailable, retryable, empty, or list. RepositoryBranchSelector renders from that value only, so loading and a message cannot both be rendered.
  • Each state renders in its own native view (keyed by the state). React otherwise recycles the loading wrapper — the only state carrying an accessibility label — into the next state's row and keeps the stale "Loading branches" description on it, which is exactly what assistive tech and the recorded hierarchy reported alongside the error.
  • useRepositoryBranches gives a pending request a 15 s bound. Past it the row reports a retryable failure with an enabled Retry while the query keeps running: a refetch cancels the stuck request, so the control works exactly when it is needed. A late answer still replaces the row, and the expiry is tied to the repository and scope it was armed for, so a new selection starts loading immediately.

Start session: classify an unreachable repository (apps/web, apps/mobile)

  • The prepareSession handlers (personal and organization) resolve the GitHub connection that owns the selected repository for the caller's scope (resolveGitHubIntegrationIdForRepository) and forward its id when the client did not send one. An explicitly supplied id still wins, and a repository no connected installation names stays unqualified — the previous behaviour — rather than refusing a request the runtime can still serve.
  • With the id present, the runtime's existing repository preflight classifies a refusal (integration_mismatch, no_installation_found, repository_not_installed). cloudPrepareFailureMessage maps exactly those refusals to the existing localized agentChat.newSession.branchUnavailable guidance, so the form says what to fix instead of the generic service line. No cloud-agent runtime code changed.

End-to-end verification

Run on this VM at this head: local dev stack (web app, Cloud Agent worker, git-token service, session ingest, Metro) and the Android emulator omp36, signed in to an owned fake account. The GitHub connection fixture is one active personal installation whose cached repository inventory names both a repository its account owns and one owned by another account; the stack holds no GitHub App credentials, so provider calls are exercised unmodified and fail at the provider.

  • Branch row settles to exactly one state. Selecting a repository the row had not queried yet issued the real branch query to the backend. With the web server process paused so the request could not be answered — no provider timeout, no server-side error, and no socket-level abort can end it — the hierarchy contained the loading node alone. Past the settle bound the same row contained the retry row alone, with an enabled Retry, and no loading description anywhere in the dump: the only thing that ended the loading state was the client-side bound. Resuming the server let the same request complete (the server logged it) and the row stayed a single retry row. A provider refusal without any stall settles the row to the same single state, and pressing Retry then issues a fresh request while the row stays single.
  • Start session, repository the installation cannot serve. The prepare request reached the worker's repository preflight, which answered GitHub repository authorization failed (integration_mismatch); the router returned BAD_REQUEST, no session row was created, and the composer showed "Failed to create session" with the actionable repository guidance instead of the generic service line. The same result reproduced twice, once per repository change.
  • Why this exercises the change. The runtime returns integration_mismatch only when a prepare request names the integration expected to serve the repository, and the mobile body never sends one; the refusal therefore demonstrates that the new server-side resolution ran and forwarded the connection it found.
  • Start session, repository the installation owns. The same flow returned a successful prepare, a session row was created, and the app opened the session detail with the prompt as its first message.

Local-stack limits, stated plainly:

  • The branch list itself could not be rendered with real branches: the successful provider read needs live GitHub App credentials, which this VM does not have. Its loading, settle-bound, retry, and settled single-state behaviour were observed; the list state was not.
  • The created session's model turn fails on the local stack (no sandbox model). That is unrelated to this change and does not affect the create, which returned success and produced the session route.

Checks run on this VM: pnpm --filter kilo-app exec tsc --noEmit (clean), pnpm --filter kilo-app exec oxlint over changed files (clean), mobile vitest pure suites (branch state, repo hook, prepare error display) and the mounted branch-selector suite (all passing), pnpm --filter web exec tsc --noEmit (clean), web jest for cloud-agent-next-router and organization-cloud-agent-next-router (298 passing, including new cases for a resolved, an explicit, and an absent connection id).

Production prerequisites

None.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Create refusal: For the create failure: mobile's prepare body never sends githubIntegrationId, so the cloud-agent runtime's repository preflight is skipped and a repository the GitHub installation cannot reach fails later as an opaque error (generic copy). Where should the classified refusal come from?
    Answer: Web resolves the integration, forwards the id
    Confidence: 0.65 — fixes the missing integration identity at its cause, uses the runtime's existing classified preflight without changing runtime code, and avoids an extra provider call on every create.
  2. Question: Branch loader: The branch section's skeleton has no settle bound: a query that stays pending (stalled request, retries in flight) spins indefinitely with no Retry. How should that be fixed?
    Answer: Settle bound ~15s, query keeps running
    Confidence: 0.60 — a stuck end state is not acceptable and loading must never look permanent; the bound is a safety net, not the only fix.
  3. Question: State shape: How deep should the branch-state refactor go?
    Answer: Pure resolver + single discriminant
    Confidence: 0.75 — fix the class: a single discriminant makes the two-node state unrepresentable.
  4. Question: Refusal copy: When Start fails because the GitHub installation cannot reach the selected repository (classified BAD_REQUEST from the runtime preflight), which copy should the composer show?
    Answer: Reuse branchUnavailable key
    Confidence: 0.55 — the brief asks for the specific actionable message the branch section uses, without an 86-catalog translation pass.
  5. Question: Integration resolution: In the web prepareSession handlers, when a GitHub repository is selected but no integration can be resolved from the caller's connected installations (ambiguous, not in the cached inventory, or an unsupported path such as a public repo), what should the server do?
    Answer: Forward nothing, keep current behaviour
    Confidence: 0.65 — refusing would break public-repo and token paths that work today; classify only the real provider refusal.
  6. Question: Settle bound UX: The branch query gets a ~15 s settle bound (per the earlier answer). While a request is genuinely still in flight after the bound, what should the row show?
    Answer: Retryable error with Retry
    Confidence: 0.65 — the row must never look stuck, and a late success can still replace the row.

…on's repository

A prepare request that names no integration skips the repository preflight, so a repository the connected installation cannot reach fails later as an opaque error. Resolve the owning connection for the caller's scope and forward its id: an explicit id still wins, and a repository no connection names stays unqualified, which is the previous behaviour.
…able repository

The branch row now renders from one resolved state, so loading and a message can never share the screen, and a request that outlives a 15 second bound becomes a retry instead of an endless skeleton. Each state also gets its own native view, so the accessibility tree cannot keep the loading description on the row that replaced it. When the server refuses the selected repository, the form shows the actionable repository guidance instead of the generic service line.
@iscekic iscekic added drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer labels Oct 23, 2026
@iscekic

iscekic commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Local-stack verification on the Android emulator with an owned fake account and seeded GitHub connection fixtures (one repository under the connected account, one whose owner differs).

  1. Branch section while the branch request is held open at the network boundary: only the loading row, no message.
  2. The same row past the 15 s settle bound: a single retry row ("Couldn't load branches" + Retry), and the accessibility hierarchy no longer carries the loading description — the state the old tree kept next to the settled row.
  3. Start session on the repository the connected account does not own: the classified refusal is shown as actionable repository guidance instead of the generic service line, and no session is created.
  4. Start session on the repository the connected account owns: the session is created and its detail screen opens with the prompt as the first message. The local fake sandbox cannot run the turn, so "Response failed" below the prompt is a local-environment artifact, not part of the create evidence.

Branch section while the branch request is still in flight

Same row past the settle bound, one retry row

Start session refuses an unreachable repository with actionable guidance

Session created for a reachable repository and its detail screen opened

@kilo-code-bot

kilo-code-bot Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit 9b3522ee adds one Vitest case for the server-shaped form of the repository-refusal copy; it exercises the same readTrpcErrorField → regex path as the existing direct-error cases, so it is a correct and behavior-neutral test addition, and the three earlier inline findings are fixed in the current code.

Assumptions: [INFERENCE] I did not run the mobile Vitest suite or typecheck in this read-only session; the verdict is from reading cloudPrepareFailureMessage, readTrpcErrorField, and knownServerErrorMessage.

Files Reviewed (5 files)
  • apps/mobile/src/components/agents/cloud-prepare-error-display.test.ts - incremental change
  • apps/mobile/src/components/agents/cloud-prepare-error-display.ts - context
  • apps/mobile/src/lib/trpc-error.ts - context
  • apps/mobile/src/lib/server-error-copy.ts - context
  • apps/mobile/src/lib/use-new-session-repos.ts - targeted verification of the previously reported settle-bound findings
Previous Review Summaries (3 snapshots, latest commit cb8222d)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit cb8222d)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change (0e6cde88..cb8222da) replaces two line-scoped ESLint disables with a single documented file-level disable in github-integration-resolution.test.ts; it is behavior-neutral for the test and I found no new defects in the changed lines.

Assumptions: [INFERENCE] I did not run the web Jest suite or ESLint in this read-only session. The verdict relies on reading the full test file: both suppressed rules (@typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires) are still exercised by the two require calls at lines 17 and 20, so the file-level disable is not unused, and the surrounding test logic is unchanged.

Files Reviewed (1 file)
  • apps/web/src/lib/cloud-agent/github-integration-resolution.test.ts

Previous review (commit 0e6cde8)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental changes (fb18f927..0e6cde88) keep the bounded branch retry pressable while a request is stuck and make the GitHub integration enrichment genuinely best-effort; I found no new defects in the changed lines.

Assumptions: [INFERENCE] I did not run the mobile or web suites in this read-only session. The verdict on the retry relies on React Query v5's refetch() canceling an in-flight fetch by default and on Button deriving disabled from loading (apps/mobile/src/components/ui/button.tsx); the web verdict relies on the new unit test and the try/catch now wrapping the inventory read. Only the incremental range was analyzed in depth; unchanged parts of the PR were checked only where a prior finding could have persisted.

Files Reviewed (4 files)
  • apps/mobile/src/lib/use-new-session-repos.ts
  • apps/mobile/src/lib/use-new-session-repos.test.ts
  • apps/web/src/lib/cloud-agent/github-integration-resolution.ts
  • apps/web/src/lib/cloud-agent/github-integration-resolution.test.ts

Previous review (commit fb18f92)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The branch-row settle bound renders its Retry non-pressable exactly when a request is stalled, so the escape hatch it adds cannot be used; two hardening gaps accompany it in the bound reset and the new integration resolution.

Assumptions: [INFERENCE] I did not run the mobile suite or the local stack, so the disabled-Retry conclusion rests on Button deriving disabled from loading (apps/mobile/src/components/ui/button.tsx:153) and React Query reporting a stalled initial fetch as isPending && isFetching; the runtime preflight/role semantics were read from services/git-token-service/src/installation-lookup-service.ts. No cross-tenant exposure found: resolution is scoped to ctx.user.id for personal and to an organizationMemberMutationProcedure-validated organizationId for organizations.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/lib/use-new-session-repos.ts 529 After the settle bound, the still-pending request sets isRetrying: true; the selector passes that as both disabled and loading to Button, so the new Retry cannot be pressed while the request stays stuck.

SUGGESTION

File Line Issue
apps/mobile/src/lib/use-new-session-repos.ts 515 boundExpired is reset in an effect, so one render after a repository/scope change the previous deadline still applies and paints the retryable error for the newly selected repository.
apps/web/src/lib/cloud-agent/github-integration-resolution.ts 22 Resolution is documented as best effort but sits outside the handlers' try, so an inventory read failure fails the create with a generic internal error instead of forwarding no integration id.
Files Reviewed (12 files)
  • apps/mobile/src/components/agents/branch-section-state.test.ts - no issues
  • apps/mobile/src/components/agents/cloud-prepare-error-display.test.ts - no issues
  • apps/mobile/src/components/agents/cloud-prepare-error-display.ts - no issues
  • apps/mobile/src/components/agents/new-session-repository-state.ts - no issues
  • apps/mobile/src/components/agents/repository-branch-selector.tsx - no issues
  • apps/mobile/src/lib/use-new-session-repos.test.ts - pins the disabled-Retry state (see WARNING)
  • apps/mobile/src/lib/use-new-session-repos.ts - 2 issues
  • apps/web/src/lib/cloud-agent/github-integration-resolution.ts - 1 issue
  • apps/web/src/routers/cloud-agent-next-router.test.ts - no issues
  • apps/web/src/routers/cloud-agent-next-router.ts - no issues
  • apps/web/src/routers/organizations/organization-cloud-agent-next-router.test.ts - no issues
  • apps/web/src/routers/organizations/organization-cloud-agent-next-router.ts - no issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 57.8K · Output: 5.4K · Cached: 606K

Review guidance: REVIEW.md from base branch main

…ry to the repository

The settle bound reported a running request as retry progress, which disabled the retry control exactly when the stuck request needed replacing; a refetch cancels the in-flight request, so the control stays enabled and only a retry after a real failure reports progress. The expiry now records the repository and scope it was armed for, so a selection change starts loading again with no stale expiry frame. The GitHub connection lookup also treats a read failure as no connection, which is the behaviour its documentation promises.
@iscekic iscekic added merge:human Substantive functional risk requires human review. merge:blocked:insufficient-e2e Verification does not cover the change labels Oct 23, 2026
…nch-start-states

# Conflicts:
#	apps/mobile/src/components/agents/cloud-prepare-error-display.ts
@iscekic iscekic removed merge:blocked:insufficient-e2e Verification does not cover the change merge:human Substantive functional risk requires human review. labels Oct 24, 2026
@iscekic

iscekic commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Re-ran the affected flow on the merged head on the Android emulator, local dev stack, owned fake account with seeded GitHub connection fixtures.

  1. Branch section for the selected repository renders exactly one settled row: Couldn't load branches with Retry, and the accessibility hierarchy carries no loading description. Pressing Retry issued fresh branch requests and the row stayed single.
  2. Start session on the repository the connection's account does not own: the worker refused with GitHub repository authorization failed (integration_mismatch), the router returned BAD_REQUEST, and the form showed the actionable repository guidance instead of the generic service line. No session was created.
  3. Start session on the repository the connection's account owns passed the repository preflight and reached the worker's local model-catalog check, which this VM cannot satisfy; no repository refusal was returned.

Branch section settled to one row after retry

Start session refusing an unreachable repository with actionable guidance

@iscekic iscekic added merge:blocked:insufficient-e2e Verification does not cover the change merge:human Substantive functional risk requires human review. labels Oct 24, 2026
@iscekic

iscekic commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Re-ran the affected flow on the current head on the Android emulator, against the local dev stack with an owned fake account and seeded GitHub connection fixtures (one repository under the connection's account, one whose owner differs).

  1. Branch row with the server suspended before selecting a repository: exactly one state was rendered — the loading row. Past the settle bound the row became a single Couldn't load branches + Retry, and the accessibility hierarchy contained no loading description anywhere. Pressing Retry after the server resumed issued fresh branch requests and the row stayed single.
  2. Start session on the repository the connection's account does not own: the worker answered GitHub repository authorization failed (integration_mismatch), the router returned BAD_REQUEST, and the form showed the actionable repository guidance instead of the generic service line. No session row was created.
  3. Start session on the connection's own repository: the request passed the repository preflight, created the session, and the app opened its detail screen with the prompt as the first user message. The local fake sandbox cannot run the turn, so Response failed under the prompt is a local-environment artifact, not part of the create evidence.

Branch section settled to a single retry row

Start refused with actionable repository guidance

@iscekic

iscekic commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Android emulator run at this head against the local dev stack.

The branch row under a selected repository holds one settled state: with the branch request still in flight past the settle bound the row shows the retry row alone, and no loading description is present in the same hierarchy. The composer reports a repository the installation cannot serve with the actionable provider-access guidance instead of the generic service line.

Branch row showing one settled retry state

Composer showing actionable repository guidance

@iscekic iscekic removed the merge:blocked:insufficient-e2e Verification does not cover the change label Oct 24, 2026
@iscekic
iscekic added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 9a2c4d3 Oct 24, 2026
63 checks passed
@iscekic
iscekic deleted the drain/new-session-branch-start-states branch October 24, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer merge:human Substantive functional risk requires human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants