Repository navigation
feat(mobile): name the organization on the PR review connect gate - #8082
Conversation
The provider connect gate and the PR review inbox never said which
account or organization the connection would run under. A reviewer in
two organizations saw only 'Connect your GitLab account' and had no way
to change the scope without leaving the flow.
Add PrReviewContextLine, a muted 'For {{organization}}' line rendered by
the connect, reconnect and Bitbucket org-only gate arms and the PR review
inbox. The whole line is the account switcher when the memberships offer
somewhere else to go — it opens the same account picker sheet the Profile
credits card uses — and plain text when there is nothing to switch to. A
GitLab MR pasted from a self-managed instance also names the instance
host above the description, since gitlab.com is the default a reader
assumes.
Copy ships in en.json plus all 86 catalogs in one commit, translated
from each catalog's sibling strings.
Verified on the Android emulator against the local stack with a fake
account in two organizations: the inbox names the active context, the
switcher opens the picker and the gate follows the switch, the
self-managed GitLab gate names the instance host while the gitlab.com
gate does not, and the Arabic RTL pass renders the line mirrored.
gate: drain-2026-10-19T01-16-34-130Z-batch-17-w22
iscekic
added
drain
|
bot: Android E2E proof on the local stack with a fake account in two seeded test organizations (test org one, test org two).
All captures show only owned fake-account test data from the non-production local stack.
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the previous review at Files Reviewed (1 files)
Previous Review Summaries (7 snapshots, latest commit 115ec52)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 115ec52)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the previous review at Files Reviewed (1 files)
Previous review (commit 1a9a79d)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the previous review at Files Reviewed (1 file)
Targeted verification at HEAD Previous review (commit e9ce7fc)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit at Files Reviewed (5 files + 87 locale catalogs)
Previous review (commit 122e558)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental change drops the Files Reviewed (2 files)
Previous review (commit ebdadcc)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe org-scope load-failure and stale-membership states are now handled, but the new status/retry surface still excludes the personal scope. Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 04253f7)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe org-scope load-failure retry now works as intended, but the new gate excludes the personal scope, so a failed memberships fetch there still leaves the user unable to switch or retry. Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit ab3a029)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe org-context feature is wired correctly through the gate and inbox, but the new context line has no error/retry path when the memberships query fails, and an unused re-export remains in the connect gate. Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (95 files)
Reviewed by deepseek-v4.1-flash-1017 · Input: 40.5K · Output: 3.8K · Cached: 360.1K Review guidance: REVIEW.md from base branch |
|
bot: RTL inbox and RTL picker captures, completing set 6 of the proof above (Arabic device locale; the mirrored context line and the working select-account sheet).
|
…est under the line cap The provider gate module exported its entry href to a file with no consumers, and the gate view test grew past the 300-line lint cap after the context line landed. Keep both files under the cap.
…line A first-load failure of the memberships query left the context line on the generic noun forever with no retry, and a persisted organization that left the list read as an unnamed scope. Mirror ContextControl's load-failure handling: a retry under the line when the list fails, and the unavailable-organization message when the persisted scope is no longer a membership.
A failed first load strands a personal-scope reader just as much as an organization one: the picker the line exists to provide stays hidden. Drop the organization gate from the load-error handling so the retry renders on both scopes.
… and tidy the review gate surfaces The org-only view now renders the account switcher above its description, so the copy still telling the reader to select an organization in their profile pointed one screen away from the control that does it. Rewrite the description to name the switcher above it, retranslated across all catalogs from each one's sibling strings. Also: one entry href constant shared by both gate modules, the gate view test restored to one branch per platform instead of a unioned launch helper, an entry-screen assertion that the context line mounts, and the mounted tests on the harness's named result type.
iscekic
added
merge:bot
…ct-gate-org-context # Conflicts: # apps/mobile/src/components/pr-review/pr-review-entry-screen.test.ts
iscekic
added
merge:blocked
merge:bot
…reen The inbox runs its screens as plain function calls, so the earlier coverage could only see a same-named host stub where the context line goes. Mount the inbox screen with the real context line and the organization provider: the active organization name renders between the header and the inbox list, and the line opens the account picker sheet and moves the selection.
iscekic
removed
merge:bot
Compare against the same typed element constants the sibling mounted suites use, so the renderer's element type check passes the app's typecheck.
iscekic
added
the
merge:bot
bot: ## Change
The provider connect gate on the PR-review surface (
pr-review-connect-gate.tsx) and the PR-review inbox never said which account or organization the connection would run under. A reviewer in two organizations saw only "Connect your GitLab account" with no way to name or change the scope without leaving the flow.PrReviewContextLine(apps/mobile/src/components/pr-review/pr-review-context-line.tsx): a mutedFor {{organization}}line naming the active account —common.personalfor the personal scope, the membership name otherwise. When the memberships offer somewhere else to go, the whole line is a switcher that opens the existing account picker sheet (useContextPicker/ContextPickerSheet, the one the Profile credits card uses); with nothing to switch to it renders as plain text with no chevron.instanceHint, display-only) names the instance host on a second line; gitlab.com links show no host line, matching the default a reader assumes.pr-review-provider-connect-gate.tsxto keep both files under the 300-line lint cap; the view ladder (selectPrReviewGateView) is untouched.prReview.connect.contextLine, shipped inen.jsonplus all 86 catalogs in the same commit, each translated from its catalog's sibling strings (no English fill).End-to-end evidence (Android emulator, local stack, fake account)
Local stack on this VM (web on localhost, Metro on localhost), CI dev-client APK, fake OTP account seeded into two organizations (Nordwind Beratung GmbH, Blaues Band Studio):
useOrganization()).ar-AE) device locale: the line renders mirrored ("لـBlaues Band Studio", chevron on the correct side), the picker opens and switching works under RTL.Checks:
pnpm --filter kilo-app run typecheck,lint,check:i18n(87 catalogs match en.json),check:plurals, and the mobile vitest suites forsrc/components/pr-review+src/i18n(2196 tests green), including new mounted tests for the context line (name, switch flow, no-chevron fallback, resolving fallback, instance hint) and the gate (line present on every provider's connect view, instance hint passed only for a self-managed GitLab host), plus a mounted inbox suite that renders the real context line inside the entry screen: the name sits between the header and the inbox list, and the line opens the picker sheet and moves the selection.Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Gate + inbox
Confidence: 0.50 · Precedent: Autonomy: fix the root cause and the class on every affected surface — an earlier session, "fix this for all shared services model pickers on all surfaces"; an earlier session, "if you saw a gap, fix it"
Answer: Muted line inside EmptyState (Recommended)
Confidence: 0.70 · Precedent: UX: keep similar elements consistent across surfaces — an earlier session, owner asked that the thought row match the tool call row
Answer: Switcher when >1 option (Recommended)
Confidence: 0.75 · Precedent: UX: remove redundant UI and make every surface pleasant to look at — an earlier session, owner asked to remove the redundant "context" copy
Answer: Show host when present (Recommended)
Confidence: 0.80 · Precedent: none
Answer: common.personal (Recommended)
Confidence: 0.75 · Precedent: Speed and least work: "do the least amount of work possible, with no drop in quality" (an earlier session); the brief limits copy to en.json
Answer: Tappable context line (Recommended)
Confidence: 0.75 · Precedent: UX: keep similar elements consistent across surfaces; the thought row should match the tool call row (an earlier session)
Answer: Connect + reconnect + org-only (Recommended)
Confidence: 0.65 · Precedent: Fix the class on every affected surface: "fix this for all shared services model pickers on all surfaces" (an earlier session)
Answer: Shared ContextLine component (Recommended)
Confidence: 0.55 · Precedent: If you see a gap, fix it: "if you saw a gap, fix it" (an earlier session); keep similar elements consistent across surfaces
Answer: Self-contained component (Recommended)
Confidence: 0.80 · Precedent: Call services directly and extract shared code: an earlier session, "extract any common code to an internal library"; Keep similar elements consistent across surfaces: an earlier session
Answer: One key with placeholder (Recommended)
Confidence: 0.65 · Precedent: Least work with no drop in quality: an earlier session, "do the least amount of work possible, with no drop in quality"
Answer: Gate + inbox mounted tests (Recommended)
Confidence: 0.70 · Precedent: Keep CI checks that catch real errors in PRs: an earlier session, "we want to catch errors in PRs, as early as possible"
Answer: Arabic screenshot spot check (Recommended)
Confidence: 0.85 · Precedent: Require valid end-to-end proof on every PR: an earlier session, "logs and/or screenshots are mandatory proof"; RTL alignment check: an earlier session
Answer: Fix the end check, not the branch. A main-only merge commit adds no feature code, so the check should accept the head when it is a merge from main and Kilobot reviewed the last non-merge head. Make that change in the workflow so every item gets it. Do not push an empty commit and do not invent a change. Then re-run the end check and continue to ready-for-review.
Confidence: 0.60 · Precedent: Autonomy: never accept blocked, fix the class; an earlier session: "a work item must converge". Speed: loosen a strict requirement that stalls items; an earlier session: "let's loosen it".
Answer: Fix the class yourself; don't wait on me. Land the end-check change in the workflow repo directly on main. The change: accept a head that only merges main when Kilobot reviewed the last non-merge head and the merge adds no non-generated diff beyond conflict resolution. Let it propagate (git pull and restart), then retry drain_session_end. Leave the branch untouched and push no empty commit. Only if you cannot write to the workflow repo, add the mounted inbox test that renders the real context line as one substantive commit, so Kilobot reviews a normal head. Blocked is not an end state.
Confidence: 0.55 · Precedent: Never accept blocked: treat it as a workflow bug and fix it until the item converges. An earlier session: 'a work item must converge'. Fix the class: an earlier session, 'fix the class'. Push directly to main in the workflow repo: an earlier session, 'push directly to main'.