First-principles overhaul: correctness fixes, unified streaming architecture, UX/design polish, new capabilities#76
Conversation
…s endpoint, validation, auth hook, rate limiting, tests; remove dead assistant/upload code Co-Authored-By: Nader Dabit <dabit3@gmail.com>
…ll header, System theme, settings redesign, camera-roll save/share, a11y labels, ESLint + CI Co-Authored-By: Nader Dabit <dabit3@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| name: 'Light', | ||
| label: 'light', | ||
| quoteBackgroundColor: 'rgba(0, 0, 0, .05)', | ||
| codeBorderColor: 'rgba(255, 255, 255, .2)', |
There was a problem hiding this comment.
🟡 Code blocks and horizontal rules are invisible on all light-colored themes
The code border color for the light theme is set to near-transparent white (codeBorderColor: 'rgba(255, 255, 255, .2)' at app/src/theme.ts:38) on a white background, so code block borders, fenced code borders, and horizontal rules are invisible in chat messages.
Impact: Users on the Light, Hacker News, or Pink themes cannot see code blocks, code fences, or horizontal rules in AI responses.
Mechanism: light-based themes inherit an invisible border color
The codeBorderColor property was newly introduced in this PR. The dark theme correctly uses 'rgba(255, 255, 255, .2)' (light border on dark background) at app/src/theme.ts:58, but the light theme copies the same value instead of using a dark-on-light equivalent like 'rgba(0, 0, 0, .15)'.
Since hackerNews (app/src/theme.ts:74) and pink (app/src/theme.ts:127) both spread ...lightTheme, they inherit the broken value.
The property is used in three places in app/src/screens/chat.tsx:
code_inlineborder (app/src/screens/chat.tsx:782)hrbackground color (app/src/screens/chat.tsx:786) — horizontal rules become invisiblefenceborder (app/src/screens/chat.tsx:794) — code fences become invisible
| codeBorderColor: 'rgba(255, 255, 255, .2)', | |
| codeBorderColor: 'rgba(0, 0, 0, .15)', |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in db22ab6 — light theme now uses codeBorderColor: 'rgba(0, 0, 0, .15)', which HN and Pink themes inherit.
| sanitized[label] = msgs | ||
| .filter(m => !m.error) | ||
| .map(m => m.image ? { ...m, image: { ...m.image, base64: undefined } } : m) | ||
| } |
There was a problem hiding this comment.
🔍 Chat persistence strips base64 from image attachments, preventing re-send of images
In app/src/screens/chat.tsx:165, the persistence sanitizer sets base64: undefined on image attachments before writing to AsyncStorage. This is a reasonable space-saving measure, but it means that after app restart, user messages with images will display the image (via uri) but the base64 data needed to re-send the image to the server (via useChatStream.ts:28-30) will be missing. If a user retries or regenerates after restart, the image context will be silently lost from the API request. The uri may also become stale after restart depending on the platform's temporary file lifecycle.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Deliberate tradeoff: persisting base64 payloads in AsyncStorage would quickly blow past its size limits (a single photo can be several MB), so persisted history keeps the display uri but drops the raw bytes. After a restart, retrying a message with an image sends the text context without the image — degraded but not broken. A follow-up could copy attachments into the app's document directory and re-encode on demand.
| const token = process.env.API_AUTH_TOKEN | ||
| if (!token) return next() | ||
| const header = req.headers.authorization || '' | ||
| if (header === `Bearer ${token}`) return next() |
There was a problem hiding this comment.
🟨 Auth token compared with timing-unsafe string equality
The bearer token authentication in server/src/middleware.ts:12 uses JavaScript's === operator to compare the provided token against the secret. This is not constant-time and could theoretically leak information about the token value through timing side-channels, though exploitability over a network is low.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in db22ab6 — the comparison now uses crypto.timingSafeEqual with a length check.
| app.use(bodyParser.urlencoded({ extended: true })) | ||
| app.use(bodyParser.json()) | ||
| app.use(express.json({limit: '50mb'})) | ||
| app.use(cors({ origin: process.env.CORS_ORIGIN || '*' })) |
There was a problem hiding this comment.
🟨 Default CORS policy allows all origins
The server at server/src/index.ts:13 defaults to cors({ origin: '*' }) when CORS_ORIGIN is not set. This allows any website to make authenticated cross-origin requests to the API, which could be exploited if the API is deployed with API_AUTH_TOKEN set and a browser-based attacker can trick a user into visiting a malicious page.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional default: the primary client is a native mobile app (no browser cookies/ambient credentials), so * keeps local dev friction-free, and the auth scheme is an explicit bearer header that a cross-origin page can't attach without already knowing the token. Deployments that serve browser clients can lock this down via CORS_ORIGIN (documented in .env.example).
Co-Authored-By: Nader Dabit <dabit3@gmail.com>
1 participant
Summary
Full-stack overhaul of the chat app across correctness, architecture, UX, design/a11y, and capabilities.
Correctness (the important invisible bugs)
messages[]history.Human:/Assistant:prompt-string API (with 2,000-char history truncation) to the native/v1/messagesAPI.errorevents; the UI shows the error inline with a Retry button (previously console-only).expo-media-library(previously wrote a data-URI into an invisible app dir).Architecture
POST /chat/:provideraccepts{ model, messages: [{ role, content, image? }] }, streams normalized SSE{"content": "..."}tokens +[DONE]. OpenAI/GLM/Kimi share a singlestreamOpenAICompatible()handler.useChatStream()hook (provider routing, SSE parsing with chunk-boundary buffering, 50ms token batching, stop()).GET /modelsis the single source of truth for the model registry (app fetches on launch, falls back to local constants); models carry aproviderfield instead of name-substring matching.API_AUTH_TOKEN), rate limiting, CORS,/health, env-configurable port.noImplicitAnyon, all errors fixed), ESLint (expo config), vitest+supertest server tests (8 passing), and a GitHub Actions CI workflow (typecheck + lint + test for app and server).UX
Design & accessibility
useColorScheme(); themed StatusBar.New capabilities
expo-speech.Not included (and why)
Testing
pnpm typecheckclean in bothapp/andserver/;expo lintclean; server tests 8/8 passing.Link to Devin session: https://app.devin.ai/sessions/83f8a428d8d84271a022288548575d52
Requested by: @dabit3