Fix mobile review pane scrolling and improve delta colors

[friuns2]

No description provided.

[qodo-code-review]

Review Summary by Qodo

Sync auth state, fix mobile scrolling, and improve GitHub API reliability

✨ Enhancement 🐞 Bug fix

Walkthroughs

Description

• Sync active account state with runtime authentication on account queries
• Fix mobile review pane scrolling with flexbox layout and touch scrolling
• Replace Node fetch with curl for GitHub tree API to prevent hangs
• Improve delta color styling in file change summaries
• Update version to 1.0.18

Diagram

flowchart LR
  A["Account Routes"] -->|"syncStoredActiveAccountWithAuth"| B["Auth State Sync"]
  B -->|"Update active account"| C["Stored State"]
  D["Skills Routes"] -->|"fetchGitHubJsonWithCurl"| E["GitHub Tree API"]
  E -->|"Fallback to fetch"| F["Tree Cache"]
  G["Review Pane"] -->|"Flexbox + touch scroll"| H["Mobile Scrolling Fix"]
  I["Thread Conversation"] -->|"Color styling"| J["Delta Colors"]

Loading

File Changes

1. src/server/accountRoutes.ts ✨ Enhancement +67/-1

Sync stored account state with runtime auth

• Added syncStoredActiveAccountWithAuth() function to synchronize stored account state with
 runtime authentication
• Updates active account ID and account metadata from auth file on each accounts query
• Persists state changes and snapshots when account details differ
• Integrated sync call into /codex-api/accounts and /codex-api/accounts/active endpoints

src/server/accountRoutes.ts

---

2. src/server/skillsRoutes.ts 🐞 Bug fix +24/-3

Use curl for GitHub tree API with fallback

• Added fetchGitHubJsonWithCurl() function using curl command for GitHub API requests
• Implements fallback mechanism: tries curl first, then falls back to Node fetch
• Addresses hanging issue with large Skills Hub tree in certain proxy setups
• Curl request includes timeout and proper headers for GitHub API compatibility

src/server/skillsRoutes.ts

---

3. src/App.vue ✨ Enhancement +10/-3

Resolve quota display for active accounts

• Added import for UiRateLimitSnapshot type
• Created resolveDisplayedQuota() function to show active account's live quota or fallback to
 snapshot
• Updated formatAccountQuota() to use resolved quota instead of always using snapshot
• Improves quota display accuracy for active accounts

src/App.vue

---

View more (4)

4. src/components/content/ReviewPane.vue 🐞 Bug fix +6/-4

Fix mobile review pane scrolling with flexbox

• Added flexbox display and flex-col to .review-pane-content and .review-pane-findings for
 proper layout
• Added flex-1 and -webkit-overflow-scrolling: touch to .review-pane-diff for mobile scrolling
• Added min-h-0 flex-1 to .review-pane-findings-list for proper flex child sizing
• Changed .review-pane-main to flexbox layout with h-full min-h-0 flex-col for mobile
 responsiveness

src/components/content/ReviewPane.vue

---

5. src/components/content/ThreadConversation.vue ✨ Enhancement +20/-4

Match file change summary delta colors

• Fixed indentation in file change summary status spans (two instances)
• Added file-change-summary-signed-count class to styled count badges in file summaries
• Added new CSS rules for .file-change-summary-signed-count with tone-based colors (add, remove,
 neutral)
• Matches delta colors between review pane and thread conversation components

src/components/content/ThreadConversation.vue

---

6. src/style.css ✨ Enhancement +12/-0

Add dark mode styles for delta colors

• Added dark mode styles for .file-change-summary-signed-count with tone variants
• Provides emerald, rose, and zinc colors for add, remove, and neutral tones in dark mode
• Ensures consistent color scheme across light and dark themes for file change badges

src/style.css

---

7. package.json ⚙️ Configuration changes +1/-1

Bump version to 1.0.18

• Updated version from 1.0.16 to 1.0.18

package.json

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3)   📘 Rule violations (0)   📎 Requirement gaps (0)

🐞\ ≡ Correctness (1) ☼ Reliability (2)

1. Accounts GET can 502 🐞 ☼

Description

/codex-api/accounts and /codex-api/accounts/active now await syncStoredActiveAccountWithAuth()
without handling potential filesystem write failures from
writeSnapshot()/writeStoredAccountsState(). If those writes fail (permissions, disk full, etc.),
the request throws and is converted to a 502 by the bridge middleware, breaking account loading in
the UI.

Code

src/server/accountRoutes.ts[R769-773]

  if (req.method === 'GET' && url.pathname === '/codex-api/accounts') {
+    await syncStoredActiveAccountWithAuth()
    const state = await scheduleAccountsBackgroundRefresh()
    setJson(res, 200, {
      data: {

Evidence

The new route-path calls to syncStoredActiveAccountWithAuth() are not wrapped in try/catch. Inside
the sync function, disk writes are performed unguarded. The server bridge wraps route handling in a
try/catch and returns a 502 JSON response on any thrown error, so a local FS failure becomes a
user-visible outage on these endpoints.

src/server/accountRoutes.ts[696-759]
src/server/accountRoutes.ts[769-789]
src/server/accountRoutes.ts[348-352]
src/server/codexAppServerBridge.ts[1436-1451]
src/server/codexAppServerBridge.ts[1840-1844]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Account listing endpoints now perform best-effort “active auth -> stored state” synchronization, but any filesystem write failure currently throws and turns the endpoint into a 502.

## Issue Context
- `syncStoredActiveAccountWithAuth()` can call `writeSnapshot()` and `writeStoredAccountsState()`.
- `/codex-api/accounts` and `/codex-api/accounts/active` call it without guarding errors.
- Unhandled errors are converted to 502 by the bridge middleware.

## Fix Focus Areas
- src/server/accountRoutes.ts[696-759]
- src/server/accountRoutes.ts[769-789]

## Suggested fix
Make the sync non-fatal for GET endpoints:
- Option A (preferred): catch and swallow write errors inside `syncStoredActiveAccountWithAuth()` (return the previously-read `state` if snapshot/state persistence fails).
- Option B: wrap the `await syncStoredActiveAccountWithAuth()` calls in try/catch in the GET handlers and proceed with `readStoredAccountsState()`/`scheduleAccountsBackgroundRefresh()`.

If you want observability without breaking UX, add a minimal log (server-side) when persistence fails, but do not fail the GET response.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Delta badge color overridden 🐞 ≡

Description

In ThreadConversation.vue, the new .file-change-summary-signed-count[data-tone=...] rules set text
colors, but the existing later .file-change-signed-count[data-tone=...] rule also matches the same
elements and overrides the summary text colors in light mode. This prevents the intended “improved
delta colors” from actually applying (dark mode is unaffected due to higher-specificity global
overrides).

Code

src/components/content/ThreadConversation.vue[R4425-4443]

+.file-change-summary-signed-count {
+  @apply rounded-full px-2 py-1;
+}
+
+.file-change-summary-signed-count[data-tone='add'] {
+  @apply bg-emerald-100 text-emerald-700;
+}
+
+.file-change-summary-signed-count[data-tone='remove'] {
+  @apply bg-rose-100 text-rose-700;
+}
+
+.file-change-summary-signed-count[data-tone='neutral'] {
+  @apply bg-zinc-100 text-zinc-600;
+}
+
.file-change-signed-count[data-tone='add'] {
  @apply text-emerald-600;
}

Evidence

The template now renders summary parts with both classes (file-change-signed-count and
file-change-summary-signed-count). In the scoped CSS, the summary tone rules appear before the
generic tone rules with equal specificity, so the later generic rule wins for color in light mode.
Dark-mode overrides live in global CSS with higher specificity (:root.dark ...) so the problem is
limited to light mode.

src/components/content/ThreadConversation.vue[116-136]
src/components/content/ThreadConversation.vue[4421-4447]
src/style.css[524-542]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Light-mode summary delta badge text colors are overridden because the element matches both `.file-change-summary-signed-count[data-tone=...]` and the later `.file-change-signed-count[data-tone=...]` rule.

## Issue Context
The summary count spans now include both classes:
- `file-change-signed-count`
- `file-change-summary-signed-count`

Because the generic tone rules are defined after the summary rules (and have equal specificity), they override `text-*` on the summary badges in light mode.

## Fix Focus Areas
- src/components/content/ThreadConversation.vue[4421-4447]

## Suggested fix
Use one of these approaches:
1) Restrict the generic tone rules so they *don’t* apply to summary badges:
```css
.file-change-signed-count:not(.file-change-summary-signed-count)[data-tone='add'] { ... }
.file-change-signed-count:not(.file-change-summary-signed-count)[data-tone='remove'] { ... }
```
2) Increase specificity of the summary tone rules so they always win:
```css
.file-change-signed-count.file-change-summary-signed-count[data-tone='add'] { ... }
```
3) Reorder: move `.file-change-signed-count[data-tone=...]` above the summary tone rules.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Fetch fallback can hang 🐞 ☼

Description

fetchSkillsTree() now tries curl first, but the fallback path uses fetch()/resp.json() without
any timeout/abort logic. Since the code comment explicitly notes Node fetch can hang reading the
large tree body, environments where curl is unavailable (or curl fails/parsing fails) can still hit
an indefinite hang.

Code

src/server/skillsRoutes.ts[R194-222]

+async function fetchGitHubJsonWithCurl(url: string): Promise<unknown> {
+  const token = await getGhToken()
+  const args = [
+    '-fsSL',
+    '--max-time', '20',
+    '-H', 'Accept: application/vnd.github+json',
+    '-H', 'User-Agent: codex-web-local',
+  ]
+  if (token) args.push('-H', `Authorization: Bearer ${token}`)
+  args.push(url)
+  const output = await runCommandWithOutput('curl', args)
+  return JSON.parse(output) as unknown
+}
+
async function fetchSkillsTree(): Promise<SkillsTreeEntry[]> {
  if (skillsTreeCache && Date.now() - skillsTreeCache.fetchedAt < TREE_CACHE_TTL_MS) {
    return skillsTreeCache.entries
  }

-  const resp = await ghFetch(`https://api.github.com/repos/${HUB_SKILLS_OWNER}/${HUB_SKILLS_REPO}/git/trees/main?recursive=1`)
-  if (!resp.ok) throw new Error(`GitHub tree API returned ${resp.status}`)
-  const data = (await resp.json()) as { tree?: Array<{ path: string; type: string }> }
+  const treeUrl = `https://api.github.com/repos/${HUB_SKILLS_OWNER}/${HUB_SKILLS_REPO}/git/trees/main?recursive=1`
+  let data: { tree?: Array<{ path: string; type: string }> }
+  try {
+    // The Skills Hub tree is large; Node fetch can hang while reading the body in some proxy setups.
+    data = await fetchGitHubJsonWithCurl(treeUrl) as { tree?: Array<{ path: string; type: string }> }
+  } catch {
+    const resp = await ghFetch(treeUrl)
+    if (!resp.ok) throw new Error(`GitHub tree API returned ${resp.status}`)
+    data = (await resp.json()) as { tree?: Array<{ path: string; type: string }> }
+  }

Evidence

The PR adds a curl-based fetch with --max-time 20, but on any curl error it falls back to
ghFetch() which is a thin wrapper around fetch() with no timeout. The nearby comment documents
that Node fetch can hang reading this response body, so the fallback reintroduces the hang risk when
triggered.

src/server/skillsRoutes.ts[184-222]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The curl-first approach mitigates hanging fetches, but the fallback fetch path still has no timeout and can hang indefinitely when triggered.

## Issue Context
- Primary: `curl --max-time 20`.
- Fallback: `fetch()` + `resp.json()` without AbortController.
- The code comment states Node fetch can hang reading this large body.

## Fix Focus Areas
- src/server/skillsRoutes.ts[184-222]

## Suggested fix
Add an explicit timeout to the fallback fetch path:
- Implement `ghFetch(url, { timeoutMs })` using `AbortController` and `setTimeout`.
- Apply the same timeout to reading/parsing (`resp.json()`) by ensuring the abort signal is used for the entire request.

Example pattern:
```ts
const controller = new AbortController()
const timer = setTimeout(() => controller.abort(), 20000)
try {
 const resp = await fetch(url, { headers, signal: controller.signal })
 ...
} finally {
 clearTimeout(timer)
}
```
This keeps the fallback from hanging forever while preserving the curl-first behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-code-review]

1. Accounts get can 502 🐞 Bug ☼ Reliability

/codex-api/accounts and /codex-api/accounts/active now await syncStoredActiveAccountWithAuth()
without handling potential filesystem write failures from
writeSnapshot()/writeStoredAccountsState(). If those writes fail (permissions, disk full, etc.),
the request throws and is converted to a 502 by the bridge middleware, breaking account loading in
the UI.

Agent Prompt

## Issue description
Account listing endpoints now perform best-effort “active auth -> stored state” synchronization, but any filesystem write failure currently throws and turns the endpoint into a 502.

## Issue Context
- `syncStoredActiveAccountWithAuth()` can call `writeSnapshot()` and `writeStoredAccountsState()`.
- `/codex-api/accounts` and `/codex-api/accounts/active` call it without guarding errors.
- Unhandled errors are converted to 502 by the bridge middleware.

## Fix Focus Areas
- src/server/accountRoutes.ts[696-759]
- src/server/accountRoutes.ts[769-789]

## Suggested fix
Make the sync non-fatal for GET endpoints:
- Option A (preferred): catch and swallow write errors inside `syncStoredActiveAccountWithAuth()` (return the previously-read `state` if snapshot/state persistence fails).
- Option B: wrap the `await syncStoredActiveAccountWithAuth()` calls in try/catch in the GET handlers and proceed with `readStoredAccountsState()`/`scheduleAccountsBackgroundRefresh()`.

If you want observability without breaking UX, add a minimal log (server-side) when persistence fails, but do not fail the GET response.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools