From 720bc516e63e8340d7beb2d199d18650d49e4381 Mon Sep 17 00:00:00 2001 From: Garfie Date: Wed, 22 Apr 2026 22:15:01 -0500 Subject: [PATCH] chore(sync-upstream): fix skill bugs and expand upstream scan coverage --- .claude/skills/sync-upstream/SKILL.md | 281 ++++++++++++------ .../reference/android-architecture.md | 149 +++++++--- .../sync-upstream/reference/api-mapping.md | 1 + .../sync-upstream/reference/model-mapping.md | 4 + .../sync-upstream/reference/ui-mapping.md | 2 +- .../sync-upstream/reference/upstream-paths.md | 98 +++++- 6 files changed, 395 insertions(+), 140 deletions(-) diff --git a/.claude/skills/sync-upstream/SKILL.md b/.claude/skills/sync-upstream/SKILL.md index cf181ae..4ab64eb 100644 --- a/.claude/skills/sync-upstream/SKILL.md +++ b/.claude/skills/sync-upstream/SKILL.md @@ -4,7 +4,7 @@ description: > Sync the LibreChat Mobile client with a newer official LibreChat server release. Diffs upstream tags, identifies gaps in the mobile app, proposes changes with user approval, then implements them. Use when a new LibreChat version is released. -allowed-tools: Bash, Read, Glob, Grep, Write, Edit, Agent, TeamCreate, SendMessage, TaskCreate, TaskUpdate, TaskGet, TaskList +allowed-tools: Bash, Read, Glob, Grep, Write, Edit, WebFetch, WebSearch, Agent, TeamCreate, SendMessage, TaskCreate, TaskUpdate, TaskGet, TaskList argument-hint: "[target-tag] (optional, defaults to latest stable)" --- @@ -18,15 +18,15 @@ or grep the codebase yourself. All heavy lifting is delegated to teammates. ## Team Setup -Create an Agent Team with 4 teammates at the start: +Create an Agent Team with 5 teammates at the start: | Teammate | Role | Prompt Summary | |----------|------|----------------| -| **investigator** | Reads upstream diffs, analyzes API/type/UI changes, cross-references with mobile codebase | "You are the investigator for a LibreChat Mobile upstream sync. Your job is to analyze diffs between upstream tags and identify gaps in the mobile client. You have read-only responsibilities — never write code." | -| **android-expert** | Technical advisor on Android architecture and best practices. Cross-checks all guidance with online research before advising. | "You are the Android tech lead for a LibreChat Mobile upstream sync. You provide guidance on Android best practices, Jetpack Compose patterns, Kotlin idioms, and architectural decisions. CRITICAL: You MUST cross-check your knowledge by researching online (WebSearch/WebFetch) before giving advice. Never rely solely on training data — always verify current best practices, especially for Compose, Ktor, and Material 3. Return researched guidance to teammates who ask." | -| **ios-kmp-expert** | Technical advisor on iOS and Kotlin Multiplatform architecture. Ensures KMP shared code and iOS-specific implementations follow platform conventions. | "You are the iOS/KMP tech lead for a LibreChat Mobile upstream sync. You provide guidance on Compose Multiplatform patterns, KMP expect/actual declarations, iOS platform APIs (UIKit, Foundation, Keychain), SKIE interop, and ensuring shared code compiles correctly for both targets. CRITICAL: You MUST cross-check your knowledge by researching online (WebSearch/WebFetch) before giving advice. Never rely solely on training data — always verify current best practices for CMP, KMP, and iOS platform conventions. Return researched guidance to teammates who ask." | -| **implementer** | Writes code changes to mobile app following existing patterns and conventions | "You are the implementer for a LibreChat Mobile upstream sync. Your job is to write code changes following existing architecture patterns. Always read the module's CLAUDE.md before touching it. Follow safeApiCall, Koin DI, UDF, Ktor, @Serializable patterns. Before making significant architectural decisions, consult the android-expert or ios-kmp-expert teammate." | -| **verifier** | Checks implementation correctness, runs builds, validates changes match proposal | "You are the verifier for a LibreChat Mobile upstream sync. Your job is to independently validate that implementation matches the approved proposal. Run builds, review changes, and flag issues. Message the implementer directly if fixes are needed." | +| **investigator** | Reads upstream diffs, analyzes API/type/UI changes, cross-references with mobile codebase | "You are the investigator for a LibreChat Mobile upstream sync. Your job is to analyze diffs between upstream tags and identify gaps in the mobile client. You have read-only responsibilities — never write code. You have WebFetch and WebSearch access so you can read GitHub release notes and changelogs in addition to local diffs." | +| **android-expert** | Technical advisor on Android architecture and best practices. Cross-checks all guidance with online research before advising. | "You are the Android tech lead for a LibreChat Mobile upstream sync. You provide guidance on Android best practices, Jetpack Compose patterns, Kotlin idioms, and architectural decisions. CRITICAL: You MUST cross-check your knowledge by researching online (WebSearch/WebFetch) before giving advice. Never rely solely on training data — always verify current best practices, especially for Compose, Ktor, Koin, and Material 3. Return researched guidance to teammates who ask." | +| **ios-kmp-expert** | Technical advisor on iOS and Kotlin Multiplatform architecture. Ensures KMP shared code and iOS-specific implementations follow platform conventions. | "You are the iOS/KMP tech lead for a LibreChat Mobile upstream sync. You provide guidance on Compose Multiplatform patterns, KMP expect/actual declarations, iOS platform APIs (UIKit, Foundation, Keychain, Network.framework), SKIE interop, and ensuring shared code compiles correctly for both targets. CRITICAL: You MUST cross-check your knowledge by researching online (WebSearch/WebFetch) before giving advice. Never rely solely on training data — always verify current best practices for CMP, KMP, and iOS platform conventions. Return researched guidance to teammates who ask." | +| **implementer** | Writes code changes to mobile app following existing patterns and conventions | "You are the implementer for a LibreChat Mobile upstream sync. Your job is to write code changes following existing architecture patterns. Always read the module's CLAUDE.md before touching it. This project uses **Koin** for DI (NOT Hilt) — use `koinViewModel()`, `viewModelOf(::X)`, `singleOf(::X)`, and Koin modules in `feature/*/di/`. Follow safeApiCall, UDF, Ktor, @Serializable patterns. Whenever you touch a KMP module, check BOTH commonMain AND iosMain source sets — expect declarations in commonMain need actual implementations in both androidMain and iosMain. Before making significant architectural decisions, consult the android-expert or ios-kmp-expert teammate." | +| **verifier** | Checks implementation correctness, runs builds, validates changes match proposal | "You are the verifier for a LibreChat Mobile upstream sync. Your job is to independently validate that implementation matches the approved proposal. Run Android build, iOS framework link, detekt, and tests. Review changes file-by-file. Flag issues. Message the implementer directly if fixes are needed — they retain context of what they changed." | **Teammate interaction pattern:** - **investigator** works independently on Phase A and B (diff + gap analysis) @@ -43,11 +43,14 @@ spawn new sub-agents (sub-agents lose context). Sub-agents are a last resort fal ## Prerequisites -Run these checks yourself (lightweight, no codebase reading): +Run these checks yourself (lightweight, no codebase reading). The working directory is already +the mobile repo root — do **not** `cd` into a different directory. The local checkout directory +name can vary (historical: `LibreChat-Android`; current upstream: `Librechat-Mobile`), so rely +on relative paths from CWD rather than any hardcoded repo-dir name. ### 1. Submodule check ```bash -cd Librechat-Mobile && git submodule status +git submodule status ``` If `upstream/` is missing: ```bash @@ -61,7 +64,7 @@ cat UPSTREAM_VERSION ``` If missing, create from current state: ```bash -VERSION=$(grep 'SUPPORTED_BACKEND_VERSION' core/common/src/main/kotlin/com/garfiec/librechat/core/common/BackendVersion.kt | grep -o '"[^"]*"' | tr -d '"') +VERSION=$(grep 'SUPPORTED_BACKEND_VERSION' core/common/src/commonMain/kotlin/com/garfiec/librechat/core/common/BackendVersion.kt | grep -o '"[^"]*"' | tr -d '"') COMMIT=$(cd upstream && git rev-parse HEAD) echo "# Upstream LibreChat version this mobile build tracks." > UPSTREAM_VERSION echo "# Updated by the sync-upstream skill. Do not edit manually." >> UPSTREAM_VERSION @@ -80,13 +83,8 @@ If dirty, ask the user to commit or stash before proceeding. Check if a prior sync was partially done in another session: ```bash -# Get version from tracking file TRACKED_TAG=$(grep '^tag=' UPSTREAM_VERSION | cut -d= -f2) - -# Get version from BackendVersion.kt -CODE_VERSION=$(grep 'SUPPORTED_BACKEND_VERSION' core/common/src/main/kotlin/com/garfiec/librechat/core/common/BackendVersion.kt | grep -o '"[^"]*"' | tr -d '"') - -# Get submodule current position +CODE_VERSION=$(grep 'SUPPORTED_BACKEND_VERSION' core/common/src/commonMain/kotlin/com/garfiec/librechat/core/common/BackendVersion.kt | grep -o '"[^"]*"' | tr -d '"') SUBMODULE_HEAD=$(cd upstream && git rev-parse HEAD) TRACKED_COMMIT=$(grep '^commit=' UPSTREAM_VERSION | cut -d= -f2) ``` @@ -98,6 +96,20 @@ If `TRACKED_TAG` (without `v` prefix) != `CODE_VERSION`, or `SUBMODULE_HEAD` != Wait for user response before continuing. +Also check for existing artifacts from a previous partial run: +```bash +ls -la .claude/sync-upstream/artifacts/ 2>/dev/null +``` +If a previous proposal or phase report exists, offer to resume from that state. + +### 5. Prepare artifact directory + +```bash +mkdir -p .claude/sync-upstream/artifacts +``` +All phase outputs (diff summaries, gap reports, proposals) are persisted here so the skill +is resumable if the session is compacted or dies mid-run. + --- ## Phase A: Diff Analysis @@ -108,29 +120,50 @@ Wait for user response before continuing. Create a task for the investigator with this prompt: -> Analyze upstream changes for a version sync. +> Analyze upstream changes for a version sync. **Persist all outputs to files under +> `.claude/sync-upstream/artifacts/` as you go** so the team lead can recover state if +> the session is compacted. > > 1. Read `UPSTREAM_VERSION` at the repo root to get the current tracked tag and commit. > 2. Fetch latest tags: `cd upstream && git fetch --tags origin` > 3. List all tags newer than the current tracked tag: > `cd upstream && git tag --sort=-v:refname` > Filter OUT any tags containing `-rc`, `-beta`, `-alpha`. -> 4. Generate diffs between the current tag and {target_tag} for these paths -> (read `${CLAUDE_SKILL_DIR}/reference/upstream-paths.md` for the full list): -> - `api/server/routes/` -> - `api/server/controllers/` -> - `packages/data-provider/src/` +> 4. **Read GitHub release notes for every stable tag** between the current tracked tag and +> the target tag (inclusive). Release notes almost always flag breaking changes that +> diffs hide. For each tag: +> ```bash +> gh api repos/danny-avila/LibreChat/releases/tags/{tag} --jq .body +> ``` +> Fall back to `WebFetch` on `https://github.com/danny-avila/LibreChat/releases/tag/{tag}` +> if `gh` is unavailable. Also scan commit messages: +> `cd upstream && git log --oneline {current_tag}..{target_tag}` — commit subjects +> from upstream maintainers are often the most candid description of what broke. +> 5. Generate diffs between the current tag and {target_tag} for all paths listed in +> `${CLAUDE_SKILL_DIR}/reference/upstream-paths.md`. Use `--stat` for an overview, +> then request per-path file-level diffs for anything non-trivial. Paths include: +> - `api/server/routes/` (including `routes/agents/`, `routes/files/`, `routes/admin/`) +> - `api/server/controllers/`, `api/server/middleware/`, `api/server/services/` +> - `api/models/` (Mongoose models that shape API responses) +> - `packages/data-provider/src/` — especially `api-endpoints.ts`, `config.ts`, +> `data-service.ts`, `parsers.ts`, `permissions.ts`, `types/`, `react-query/` > - `packages/data-schemas/src/` -> - `client/src/components/` -> - `client/src/hooks/` -> - `client/src/store/` -> 5. Categorize all changes into: -> - **API changes**: New/modified/removed routes, controller logic -> - **Type/schema changes**: New/modified data types, request/response shapes +> - `packages/api/src/` (newer workspace introduced recently) +> - `client/src/components/`, `client/src/hooks/`, `client/src/data-provider/`, +> `client/src/store/`, `client/src/Providers/` +> - `librechat.example.yaml`, `.env.example` (server config schema — affects `/api/config` surface) +> 6. Categorize every change into exactly one of: +> - **API changes**: New or modified routes, controller logic +> - **Removed/deprecated endpoints**: Routes or fields that were removed or deprecated (BREAK mobile hard — cannot be deferred) +> - **Type/schema changes**: New or modified data types, request/response shapes +> - **New config / feature flags**: New keys in `/api/config` response that gate UI > - **UI changes**: New components, modified user flows -> - **Bug fixes**: Fixes that may need mobile equivalents +> - **Security fixes**: CVEs, auth changes, token handling — cannot be deferred +> - **Bug fixes**: Non-security fixes that may need mobile equivalents > - **Infrastructure**: Build, config, deps (usually not relevant) -> 6. Report back a structured summary table. +> 7. Write the categorized summary to +> `.claude/sync-upstream/artifacts/phase-a-diff-summary.md` and report a short +> version back. Include representative file paths and release-note excerpts. If `$ARGUMENTS` was provided, use it as the target tag. @@ -141,7 +174,8 @@ Once investigator reports back: - If no `$ARGUMENTS`, present the list of available tags and ask user to pick one (default: latest stable). - Show the investigator's categorized summary. -**Done when:** User has seen the diff summary and confirmed the target tag. +**Done when:** User has seen the diff summary and confirmed the target tag. The file +`.claude/sync-upstream/artifacts/phase-a-diff-summary.md` exists. --- @@ -152,32 +186,58 @@ Once investigator reports back: ### B1. Send follow-up to investigator > Now cross-reference the upstream changes you found with the mobile codebase. +> Persist the gap report to `.claude/sync-upstream/artifacts/phase-b-gap-report.md`. > > 1. Read these mapping files from `${CLAUDE_SKILL_DIR}/reference/`: -> - `api-mapping.md` — official routes to mobile *Api.kt files +> - `api-mapping.md` — official routes to mobile `*Api.kt` files > - `model-mapping.md` — official TS types to Kotlin data classes > - `ui-mapping.md` — web components to mobile feature modules -> 2. For each API change from Phase A: -> - Grep the mobile codebase (`core/network/src/`) for the corresponding endpoint -> - Note if it exists, needs updating, or is missing -> 3. For each type/schema change: -> - Check `core/model/src/` for matching Kotlin data class -> - Note new fields, removed fields, type changes -> 4. For each UI change: -> - Check the corresponding `feature/*/` module +> 2. Read `DISCOVERY.md` at the repo root — it catalogs already-discovered backend +> endpoints and their quirks. Any new endpoints must be appended there by the +> implementer in Phase D. +> 3. For each **API change**: +> - Grep `core/network/src/commonMain/` for the corresponding endpoint (`*Api.kt` files) +> - Also grep `core/data/src/commonMain/` — some repositories inline endpoint paths +> - Note: exists / needs updating / missing +> 4. For each **removed / deprecated endpoint**: +> - Find every mobile call site and flag it as a Breaking gap +> 5. For each **type / schema change**: +> - Check `core/model/src/commonMain/` for the matching Kotlin data class +> - Note new, removed, or type-changed fields +> - **SSE-specific**: when upstream modifies streaming payloads or run events, also +> check `core/model/src/.../StreamEvent.kt`, `SseContentEvent.kt`, `ToolCallRecord.kt`, +> `ToolCallResult.kt`, `ToolAuthStatus.kt`. SSE shape drift is a frequent source of +> silent breakage that compiles but produces wrong UI at runtime. +> 6. For each **UI change**: +> - Check the corresponding `feature/*/` module (commonMain + platform sources) > - Note if the feature exists, needs updating, or is missing -> 5. Categorize all gaps: -> - **Breaking** (must fix): Changed request/response shapes, removed/renamed endpoints, auth changes -> - **Additive** (new feature): New endpoints, new UI features, new optional fields -> - **Cosmetic** (polish): UI improvements, a11y, i18n -> 6. Report the categorized gap list with specific file paths for both upstream and mobile. +> 7. For each **new feature flag in `/api/config`**: +> - Check `core/model/src/commonMain/.../StartupConfig.kt` — new keys must be added +> for the mobile app to detect the flag and gate UI accordingly +> 8. **Version-gating check.** Read `core/common/src/commonMain/.../BackendVersion.kt`. +> It exposes `parse()`, `isCompatible()`, and `extractVersionFromFooter()`. For any +> new upstream feature that requires the target tag or higher, flag it so the +> implementer adds a `BackendVersion.isCompatible(...)` check on the mobile side. +> 9. **iosMain coverage check.** For every touched module under `core/*` or `feature/*`, +> confirm whether `src/iosMain/kotlin/...` sources exist. If they do, the implementer +> must update both commonMain AND iosMain (expect/actual pairs, platform-specific +> bridges like `NWConnection` SSE transport, Keychain, etc.). Flag modules where +> iosMain parity is easily missed. +> 10. Categorize all gaps: +> - **Breaking** (must fix): Changed or removed request/response shapes, removed/renamed +> endpoints, auth changes, security fixes +> - **Additive** (new feature): New endpoints, new UI features, new optional fields, +> new feature flags +> - **Cosmetic** (polish): UI improvements, a11y, i18n +> 11. Report the categorized gap list with specific file paths for both upstream and mobile. ### B2. Receive gap report The investigator now has full context of both the upstream diff AND the mobile gaps. Keep the investigator alive for follow-up questions during Phase C. -**Done when:** You have the categorized gap list from the investigator. +**Done when:** You have the categorized gap list, and +`.claude/sync-upstream/artifacts/phase-b-gap-report.md` exists on disk. --- @@ -185,28 +245,31 @@ Keep the investigator alive for follow-up questions during Phase C. **Lead presents to user — no delegation needed.** -Take the investigator's gap report and present it as a structured proposal: +Take the investigator's gap report and produce a structured proposal. +**Persist the proposal to `.claude/sync-upstream/artifacts/proposal-{target_tag}.md` +before presenting it** so the user has a reviewable artifact and the skill can +resume if the session dies. ``` ## Sync Proposal: {current_tag} → {target_tag} ### Breaking Changes ({count}) — must fix before updating version -For each: -- What changed upstream (with file reference) -- What Mobile file(s) need updating -- Proposed change +For each: what changed upstream (file reference), mobile file(s) needing updates, proposed change. + +### Security Fixes ({count}) — must fix, cannot be deferred +For each: CVE / patch description, mobile-side impact (if any), proposed change. + +### Removed / Deprecated Endpoints ({count}) — mobile call sites must migrate +For each: removed/deprecated route, mobile call sites (file paths), replacement route and migration plan. ### New Features ({count}) — recommended -For each: -- What was added upstream -- Proposed mobile implementation approach -- Which module(s) would be affected +For each: what was added upstream, proposed mobile implementation approach, affected modules. + +### New Config / Feature Flags ({count}) +For each: new key in `/api/config`, proposed `StartupConfig.kt` update, whether a `BackendVersion` gate is needed. ### UI Changes Needing User Input ({count}) -For each: -- What the web app does (with file reference) -- Why it doesn't translate directly to mobile/Compose -- 2-3 concrete options for the user to choose from +For each: what the web app does (file reference), why it doesn't translate directly to Compose, 2-3 concrete options. ### Deferred Items ({count}) — can wait for a future sync Items that are low priority or require significant new infrastructure. @@ -215,27 +278,28 @@ Items that are low priority or require significant new infrastructure. ### Consult experts on non-trivial items Before presenting to the user, for any proposed change that involves: -- New architectural patterns (e.g., adding a new module, new DI scope) -- UI patterns that don't have an existing equivalent in the codebase -- Performance-sensitive changes (e.g., new list rendering, image loading) -- Navigation changes +- New architectural patterns (new module, new DI scope, new navigation key) +- UI patterns without an existing equivalent in the codebase +- Performance-sensitive changes (list rendering, image loading, SSE throughput) - KMP shared code or expect/actual patterns +- iOS-specific bridges (Network.framework, Keychain, WKWebView) Send Android-specific items to **android-expert** and KMP/iOS items to **ios-kmp-expert**: > Review these proposed mobile changes for best practices. For each item, > research online to verify the recommended approach is current (especially -> for Compose, Material 3, Koin, and Ktor). Flag any items where modern -> best practices differ from what we're proposing. +> for Compose, Material 3, Koin, Ktor, CMP, and iOS platform APIs). Flag any +> items where modern best practices differ from what we're proposing. > {list of non-trivial items} -Incorporate the experts' feedback into the proposal before showing the user. +Incorporate the experts' feedback into the proposal **and update the artifact file** before showing the user. ### Approval gate Ask the user: -> Review the proposal above. Reply with: +> Review the proposal above (also saved at `.claude/sync-upstream/artifacts/proposal-{target_tag}.md`). +> Reply with: > - **approve all** — proceed with full implementation > - **approve with changes** — tell me what to modify in the proposal > - **approve partial** — list which items to implement now (rest deferred) @@ -243,7 +307,7 @@ Ask the user: **Do NOT proceed to Phase D until the user explicitly approves.** -If the user requests changes, update the proposal and re-present. +If the user requests changes, update the proposal file AND re-present. --- @@ -263,22 +327,24 @@ Create a task for the implementer with the approved change list: > 3. Read the specific module's `CLAUDE.md` (e.g., `core/network/CLAUDE.md`) > 4. Read existing similar code as a pattern reference > -> **Conventions to follow:** +> **Conventions (THIS PROJECT USES KOIN, NOT HILT):** > - `safeApiCall` for all network calls in repositories -> - Hilt `@Inject` for dependency injection +> - **Koin** for DI — use `viewModelOf(::X)`, `singleOf(::X)`, `factoryOf(::X)` in Koin modules at `feature/*/di/` and `core/*/di/`. Register the module in `LibreChatApplication.kt`'s `startKoin { modules(...) }` when creating a new one. +> - Use `koinViewModel()` in Composables — never `hiltViewModel()` or `@HiltViewModel`. > - Unidirectional data flow: UI → ViewModel → Repository → API/Room > - `@Serializable` data classes in `core/model/` > - Ktor client patterns in `core/network/` +> - For KMP modules, update BOTH `src/commonMain/` AND `src/iosMain/` (and `src/androidMain/` when relevant). `expect` declarations in commonMain REQUIRE matching `actual` implementations in every target's source set. > > **Before making non-trivial architectural decisions** (new patterns, new modules, -> complex UI components), message the **android-expert** or **ios-kmp-expert** teammate for guidance. +> complex UI components), message the **android-expert** or **ios-kmp-expert** teammate. > They will research current best practices online and advise. > > **Approved changes:** > {paste the approved change list here} > > **After all code changes:** -> 1. Update `SUPPORTED_BACKEND_VERSION` in `core/common/src/main/kotlin/com/garfiec/librechat/core/common/BackendVersion.kt` to "{new_version}" (without v prefix) +> 1. Update `SUPPORTED_BACKEND_VERSION` in `core/common/src/commonMain/kotlin/com/garfiec/librechat/core/common/BackendVersion.kt` to "{new_version}" (without `v` prefix) > 2. Advance submodule: `cd upstream && git checkout {target_tag}` > 3. Update `UPSTREAM_VERSION` at repo root: > ``` @@ -286,7 +352,9 @@ Create a task for the implementer with the approved change list: > commit={full SHA of target_tag} > date={today's date YYYY-MM-DD} > ``` -> 4. Report back the full list of files you created or modified. +> 4. Update `DISCOVERY.md` at the repo root: append newly-discovered endpoints, removed +> endpoints, and revised response shapes. Keep it accurate — future syncs depend on it. +> 5. Report back the full list of files created or modified, grouped by module. ### D2. Assign verification task @@ -294,21 +362,38 @@ After implementer reports done, create a task for the verifier: > Verify the upstream sync implementation from {current_tag} to {target_tag}. > -> 1. Run `./gradlew assembleDebug` to verify the project compiles -> 2. Review all files the implementer changed against the approved proposal: -> - Are all approved changes implemented? -> - Are there any changes that weren't in the proposal? -> - Do the changes follow existing code patterns? -> 3. Check version consistency: -> - `SUPPORTED_BACKEND_VERSION` in BackendVersion.kt matches target -> - `UPSTREAM_VERSION` file has correct tag, commit, date -> - Submodule is at the correct tag: `cd upstream && git describe --tags` -> 4. If you find issues, message the **implementer** directly to fix them. -> The implementer retains context of what it changed. -> 5. Report back: -> - Build status (pass/fail + any errors) -> - Issues found (if any) -> - List of user-testable behaviors to verify on device +> **Build verification (run all four):** +> 1. `./gradlew assembleDebug` — Android app compiles +> 2. `./gradlew detekt` — static analysis passes on ALL source sets (commonMain, +> androidMain, iosMain). The CI gate currently only covers commonMain, so do +> not rely on CI here. If failures are found in iosMain/androidMain, report them. +> 3. `./gradlew :shared:linkDebugFrameworkIosSimulatorArm64` — iOS framework links. +> This catches iosMain compile errors that `assembleDebug` silently skips. +> 4. `./gradlew check` — unit tests pass for touched modules +> +> **Correctness review:** +> 5. Read the approved proposal at `.claude/sync-upstream/artifacts/proposal-{target_tag}.md`. +> 6. Review every file the implementer changed: +> - All approved items implemented? +> - Any unapproved changes (scope creep)? +> - Koin DI used (not Hilt / `@Inject`)? +> - `safeApiCall` wrapping new network calls? +> - For KMP modules: did the implementer touch iosMain where needed, not just commonMain? +> - For new endpoints: was `DISCOVERY.md` updated? +> +> **Version consistency:** +> 7. `SUPPORTED_BACKEND_VERSION` in `core/common/src/commonMain/.../BackendVersion.kt` matches target +> 8. `UPSTREAM_VERSION` file has correct tag, commit, date +> 9. Submodule is at the correct tag: `cd upstream && git describe --tags` +> 10. `DISCOVERY.md` reflects endpoint additions/removals +> +> **If you find issues**, message the **implementer** directly to fix them. +> The implementer retains context of what it changed. +> +> **Report back:** +> - Pass/fail for each of the four build commands + any errors +> - Issues found (if any) and who fixed them +> - List of user-testable behaviors to verify on device — separate lists for Android and iOS where applicable ### D3. Present results to user @@ -321,17 +406,22 @@ Once verifier confirms everything passes: {grouped by module} ### What to Test -{specific user-facing behaviors from verifier} +{Android behaviors} +{iOS behaviors} ### How to Test -1. Build: `./gradlew assembleDebug` -2. Install on device/emulator +1. Build: `./gradlew assembleDebug` (Android), `./gradlew :shared:linkDebugFrameworkIosSimulatorArm64` (iOS) +2. Install on device/emulator (both platforms where relevant) 3. Test each behavior listed above ### Version Updates - SUPPORTED_BACKEND_VERSION: {old} → {new} - UPSTREAM_VERSION tag: {old_tag} → {new_tag} - Submodule: pinned to {target_tag} +- DISCOVERY.md: {brief summary of what was appended} + +### Artifacts +Proposal and phase reports saved under `.claude/sync-upstream/artifacts/`. ``` --- @@ -345,7 +435,10 @@ Once verifier confirms everything passes: | No newer tags | Report "up to date" and stop | | Dirty working tree | Ask user to commit/stash | | Git fetch fails | Show `git remote -v`, check network | -| Build fails after changes | Verifier messages implementer to fix (both retain context) | -| User cancels at Phase C | Stop cleanly, no changes made | -| Version mismatch | Ask user which is the true baseline | +| Android build fails after changes | Verifier messages implementer to fix (both retain context) | +| iOS framework link fails (but Android passes) | Implementer likely forgot iosMain updates — message them to audit iosMain source sets for affected modules | +| Detekt fails (any source set) | Implementer fixes findings. Do not use `--ignore-failures`. | +| User cancels at Phase C | Delete or archive the proposal artifact, tear down the team (`TeamDelete`), stop cleanly | +| Version mismatch (tracked tag ≠ BackendVersion.kt) | Ask user which is the true baseline | +| Session compacted mid-run | List `.claude/sync-upstream/artifacts/` — newest file indicates phase reached; resume from there | | Teammate unresponsive | Check task status, re-send message; sub-agent as last resort | diff --git a/.claude/skills/sync-upstream/reference/android-architecture.md b/.claude/skills/sync-upstream/reference/android-architecture.md index 348ee00..b2f2fd6 100644 --- a/.claude/skills/sync-upstream/reference/android-architecture.md +++ b/.claude/skills/sync-upstream/reference/android-architecture.md @@ -6,20 +6,22 @@ Condensed architecture guide for the LibreChat Mobile app. Read module-specific ``` app -├── feature/auth → core/common, core/model, core/network, core/data, core/ui -├── feature/chat → core/common, core/model, core/network, core/data, core/ui -├── feature/conversations → core/common, core/model, core/data, core/ui -├── feature/settings → core/common, core/model, core/data, core/ui -├── feature/agents → core/common, core/model, core/data, core/ui -├── feature/files → core/common, core/model, core/data, core/ui -└── core/data → core/common, core/model, core/network - core/network → core/common, core/model - core/ui → core/common, core/model - core/model → (none — pure Kotlin) - core/common → (none — lowest layer) +├── feature/auth → core/common, core/model, core/network, core/data, core/ui +├── feature/chat → core/common, core/model, core/network, core/data, core/ui +├── feature/conversations → core/common, core/model, core/network, core/data, core/ui +├── feature/settings → core/common, core/model, core/data, core/ui +├── feature/agents → core/common, core/model, core/data, core/ui +├── feature/files → core/common, core/model, core/data, core/ui +└── core/data → core/common, core/model, core/network + core/network → core/common, core/model + core/ui → core/common, core/model + core/model → (none — pure Kotlin, no Android deps) + core/common → (none — lowest layer) ``` -**Rules:** Feature modules depend on `:core:*` only, never on each other. +**Rules:** +- Feature modules depend on `:core:*` only, never on each other. +- Feature modules currently live under `feature/{auth, chat, conversations, settings, agents, files}`. There is no `feature/prompts` yet — prompts UI is not implemented. ## Data Flow Pattern (Unidirectional) @@ -28,29 +30,37 @@ UI (Composable) → ViewModel (StateFlow) → Repository (interface) → API (Kt ``` - ViewModels expose `StateFlow` to Compose -- Repositories define interfaces in `:core:data`, implemented with `@Inject` +- Repositories define interfaces in `:core:data`, implemented with Koin DI - Room is a read-through cache; server is always source of truth ## safeApiCall Pattern -All repository network calls use this wrapper (defined in `core/common`): +All repository network calls use this wrapper (defined in `:core:common`): ```kotlin suspend fun safeApiCall(block: suspend () -> T): Result = - try { Result.Success(block()) } - catch (e: ClientRequestException) { Result.Error(e, parseErrorMessage(e.response.bodyAsText())) } - catch (e: ServerResponseException) { Result.Error(e, "Server error.") } - catch (e: HttpRequestTimeoutException) { Result.Error(e, "Request timed out.") } - catch (e: IOException) { Result.Error(e, "Network unavailable.") } - catch (e: SerializationException) { Result.Error(e, "Unexpected response format.") } + try { + Result.Success(block()) + } catch (e: ClientRequestException) { // 4xx + Result.Error(e, parseErrorMessage(e.response.bodyAsText())) + } catch (e: ServerResponseException) { // 5xx + Result.Error(e, "Server error. Please try again.") + } catch (e: HttpRequestTimeoutException) { + Result.Error(e, "Request timed out.") + } catch (e: IOException) { + Result.Error(e, "Network unavailable. Check your connection.") + } catch (e: SerializationException) { + Result.Error(e, "Unexpected response format.") + } ``` ## API Service Pattern (core/network) -Each API service is a Ktor-backed class with `@Inject constructor(client: HttpClient)`: +Each API service is a Ktor-backed class with a primary constructor that takes an `HttpClient`. +It is registered in a Koin module — no annotations. ```kotlin -class ConversationsApi @Inject constructor(private val client: HttpClient) { +class ConversationsApi(private val client: HttpClient) { suspend fun getConversations(pageNumber: Int, limit: Int, isArchived: Boolean): PaginatedConversations = client.get("/api/convos") { parameter("pageNumber", pageNumber.toString()) @@ -60,11 +70,19 @@ class ConversationsApi @Inject constructor(private val client: HttpClient) { } ``` +DI registration (in `core/network/src/commonMain/.../di/NetworkModule.kt`): +```kotlin +val networkModule = module { + singleOf(::ConversationsApi) + // ... +} +``` + **Backend quirk:** Mutation endpoints wrap body in `arg` field: `{ "arg": { ... } }` ## Repository Pattern (core/data) -Interface + implementation, always with Hilt injection: +Interface in `:core:data`, implementation in the same module, registered with Koin: ```kotlin interface ConversationRepository { @@ -72,14 +90,30 @@ interface ConversationRepository { suspend fun updateTitle(id: String, title: String): Result } -class ConversationRepositoryImpl @Inject constructor( +class ConversationRepositoryImpl( private val api: ConversationsApi, private val dao: ConversationDao, private val mapper: EntityMapper, - @Dispatcher(IO) private val dispatcher: CoroutineDispatcher, + private val ioDispatcher: CoroutineDispatcher, ) : ConversationRepository { ... } ``` +DI registration: +```kotlin +val dataModule = module { + single { + ConversationRepositoryImpl( + api = get(), + dao = get(), + mapper = get(), + ioDispatcher = get(qualifier = named("io")), + ) + } +} +``` + +Dispatchers come from `:core:common` via Koin named qualifiers (`named("io")`, `named("default")`, `named("main")`). + ## Room Entity Pattern (core/data) ```kotlin @@ -88,8 +122,7 @@ data class ConversationEntity( @PrimaryKey val conversationId: String, val title: String?, val endpoint: String?, - // Complex fields stored as JSON strings via TypeConverters - val tagsJson: String?, + val tagsJson: String?, // Complex fields stored as JSON strings via TypeConverters ) ``` @@ -97,9 +130,11 @@ DAO read methods return `Flow` for reactive observation. ## ViewModel Pattern (feature modules) +ViewModels extend `ViewModel` (or `androidx.lifecycle.ViewModel` for KMP) and are registered +with `viewModelOf(::X)` in the feature's Koin module: + ```kotlin -@HiltViewModel -class ConversationsViewModel @Inject constructor( +class ConversationsViewModel( private val conversationRepository: ConversationRepository, ) : ViewModel() { @@ -123,6 +158,32 @@ class ConversationsViewModel @Inject constructor( } ``` +DI: +```kotlin +val conversationsModule = module { + viewModelOf(::ConversationsViewModel) +} +``` + +Registered in `LibreChatApplication.kt`: +```kotlin +startKoin { + modules( + commonModule, networkModule, dataModule, uiModule, + authModule, chatModule, conversationsModule, settingsModule, + agentsModule, filesModule, + ) +} +``` + +In the screen composable: +```kotlin +@Composable +fun ConversationsScreen(viewModel: ConversationsViewModel = koinViewModel()) { ... } +``` + +**Do NOT use Hilt (`@HiltViewModel`, `@Inject`, `hiltViewModel()`) anywhere — the project is Koin-only and KMP-ready.** + ## SSE Streaming (core/network) Custom line parser over raw `ByteReadChannel` (do NOT use Ktor SSE plugin). @@ -133,6 +194,23 @@ Two-phase protocol: `SseConnectionManager` handles lifecycle: start, reconnect (exponential backoff), abort, `StateFlow`. +**iOS note:** iOS uses a custom `NWConnection`-based HTTP/1.1 transport +(`core/network/src/iosMain/.../sse/SseHttpTransport.ios.kt`) to bypass NSURLSession's +undocumented `text/*` content-type buffering. When editing SSE logic, always check iosMain +sources — the iOS path is NOT just commonMain + a thin shim. + +## KMP Source Set Layout + +Every KMP module has (minimally): +- `src/commonMain/kotlin/` — shared logic +- `src/androidMain/kotlin/` — Android-specific actuals / platform bridges +- `src/iosMain/kotlin/` — iOS-specific actuals / platform bridges (Network.framework, Keychain, etc.) + +When an `expect` class or function is declared in commonMain, matching `actual` declarations +must exist in BOTH `androidMain` and `iosMain`. Forgetting iosMain compiles locally +(because `./gradlew assembleDebug` only runs the Android targets) but breaks +`./gradlew :shared:linkDebugFrameworkIosSimulatorArm64`. + ## Naming Conventions - API classes: `{Domain}Api.kt` (e.g., `ConversationsApi.kt`) @@ -142,12 +220,15 @@ Two-phase protocol: - UI state: `{Feature}UiState` data class - Room entity: `{Domain}Entity.kt` - Room DAO: `{Domain}Dao.kt` -- DI module: `{Domain}Module.kt` -- Convention plugins: `librechat.mobile.{type}` (e.g., `librechat.mobile.feature`) +- Koin module: `{Feature}Module.kt` (feature) / `{Layer}Module.kt` (core) +- Convention plugins: `librechat.kmp.{type}` (KMP) or `librechat.mobile.{type}` (Android-only) ## Build System -- Gradle 8.11.1, AGP 8.7.3, Kotlin 2.1.0, compileSdk 35, minSdk 26 -- 7 convention plugins in `build-logic/convention/` +- Gradle 9.4.1, AGP 9.1.0, Kotlin 2.3.20, compileSdk 36, minSdk 26 +- Compose Multiplatform 1.11.0-beta01, Compose BOM 2025.05.00 +- 13 convention plugins under `build-logic/convention/src/main/kotlin/` - Version catalog: `gradle/libs.versions.toml` -- Convention plugin `apply()` must use `override fun apply(target: Project) { with(target) { ... } }` +- Navigation 3 (Nav3): `NavDisplay` + `NavBackStack` + `entryProvider` +- DI: Koin (KMP-ready — no Hilt anywhere) +- Convention plugin `apply()` uses `override fun apply(target: Project) { with(target) { ... } }` diff --git a/.claude/skills/sync-upstream/reference/api-mapping.md b/.claude/skills/sync-upstream/reference/api-mapping.md index 8535a49..be84d52 100644 --- a/.claude/skills/sync-upstream/reference/api-mapping.md +++ b/.claude/skills/sync-upstream/reference/api-mapping.md @@ -32,6 +32,7 @@ Maps official LibreChat API route files to their mobile `*Api.kt` counterparts. | Official Route | What It Does | Priority | |---------------|-------------|----------| +| `routes/admin/auth.js` | Admin-only authentication routes | Low — admin feature, not exposed in mobile | | `routes/categories.js` | Agent categories | Low — categories embedded in agent responses | | `routes/endpoints.js` | List available endpoints | Medium — Mobile uses config response instead | | `routes/models.js` | List available models per endpoint | Medium — Mobile uses config response instead | diff --git a/.claude/skills/sync-upstream/reference/model-mapping.md b/.claude/skills/sync-upstream/reference/model-mapping.md index d0caea4..34a2ed2 100644 --- a/.claude/skills/sync-upstream/reference/model-mapping.md +++ b/.claude/skills/sync-upstream/reference/model-mapping.md @@ -63,3 +63,7 @@ Maps official TypeScript types from `packages/data-provider/src/` to Android Kot | `packages/data-provider/src/types/runs.ts` | Run/streaming types | | `packages/data-provider/src/schemas.ts` | Zod validation schemas | | `packages/data-provider/src/config.ts` | Config types and VERSION constant | +| `packages/data-provider/src/api-endpoints.ts` | Canonical endpoint URL builders — check here first for renames/new routes | +| `packages/data-provider/src/data-service.ts` | HTTP client functions — actual request shapes sent by the web client | +| `packages/data-provider/src/parsers.ts` | Request/response parsers; hints at shape normalization | +| `packages/data-provider/src/permissions.ts` | Permission schemas and role gating | diff --git a/.claude/skills/sync-upstream/reference/ui-mapping.md b/.claude/skills/sync-upstream/reference/ui-mapping.md index 66ad025..df5e875 100644 --- a/.claude/skills/sync-upstream/reference/ui-mapping.md +++ b/.claude/skills/sync-upstream/reference/ui-mapping.md @@ -43,7 +43,7 @@ Maps web client component directories to mobile Compose feature modules. |------------|-------------------| | Recoil atoms (`store/`) | ViewModel state (StateFlow) | | React Query hooks (`hooks/`) | Repository + ViewModel | -| React Context (`Providers/`) | Hilt DI + CompositionLocal | +| React Context (`Providers/`) | Koin DI + CompositionLocal | | Radix UI components | Material 3 components | | Tailwind CSS | Material 3 theme + Modifier | | react-router-dom | Compose Navigation | diff --git a/.claude/skills/sync-upstream/reference/upstream-paths.md b/.claude/skills/sync-upstream/reference/upstream-paths.md index 1dbf4b0..939ef3a 100644 --- a/.claude/skills/sync-upstream/reference/upstream-paths.md +++ b/.claude/skills/sync-upstream/reference/upstream-paths.md @@ -1,31 +1,107 @@ # Upstream Paths to Watch -These are the directories in the `upstream/` submodule that matter for mobile parity. +These are the directories and files in the `upstream/` submodule that matter for mobile parity. Use these paths when generating focused diffs between tags. +## Server: Routes, Controllers, Middleware, Services + | Path | What It Contains | Why It Matters | -|------|-----------------|----------------| +|------|------------------|----------------| | `api/server/routes/` | Express route definitions (REST endpoints) | Defines the API contract the mobile app calls | -| `api/server/controllers/` | Request handlers and business logic | Shows request/response shapes and validation | -| `packages/data-provider/src/` | Shared types, schemas, API client, config | Canonical type definitions; Mobile models must match | -| `packages/data-provider/src/types/` | TypeScript type subdirectory (queries, mutations, agents, etc.) | React Query hook types that define API shapes | -| `packages/data-schemas/src/` | Mongoose schemas and DB models | Database schema changes that affect API responses | -| `client/src/components/` | React UI components by feature area | Reference for Android UI feature parity | +| `api/server/routes/agents/` | Agent chat, actions, tools, v1, OpenAI routes | Agents API surface — mobile's `AgentsApi.kt` + `ChatApi.kt` | +| `api/server/routes/files/` | File upload, avatar, images, speech subroutes | Mobile's `FilesApi.kt` / `FilesExtApi.kt` | +| `api/server/routes/admin/` | Admin-only routes (currently `auth.js`) | Admin surface — likely deferrable but flag any changes | +| `api/server/controllers/` | Request handlers and business logic | Reveals exact request/response shapes and validation | +| `api/server/middleware/` | Auth, rate limiting, abort, request validation | Changes here can alter headers, error shapes, and auth flow the mobile client relies on | +| `api/server/services/` | AuthService, MCP, Endpoints, Files, Runs, Tools, Artifacts, etc. | Business logic that shapes responses and implements features | +| `api/models/` | Mongoose models (Agent, Conversation, Message, File, etc.) | DB schema drives API response shapes — especially fields, defaults, enums | + +## Data Provider Package (canonical types + API client) + +| Path | What It Contains | Why It Matters | +|------|------------------|----------------| +| `packages/data-provider/src/api-endpoints.ts` | **Canonical list of endpoint URL builders** | Route renames and new endpoints land here first | +| `packages/data-provider/src/config.ts` | `VERSION` constant, config types | Source of truth for backend version we track | +| `packages/data-provider/src/data-service.ts` | HTTP client functions | Actual request shapes the web client sends | +| `packages/data-provider/src/parsers.ts` | Request/response parsers | Normalizes shapes before/after the wire | +| `packages/data-provider/src/permissions.ts` | Permission schemas | Role gating for endpoints | +| `packages/data-provider/src/types/` | TypeScript types (queries, mutations, agents, files, runs, mcpServers, web) | React Query hook types — canonical request/response shapes | +| `packages/data-provider/src/react-query/` | React Query service + hook exports | Links endpoints to hooks | +| `packages/data-provider/src/schemas.ts` | Zod validation schemas | Request validation contracts | +| `packages/data-provider/src/` (other files) | actions, artifacts, azure, bedrock, feedback, file-config, generate, keys, mcp, messages, models, roles | Feature-specific type sources | + +## Data Schemas Package (DB-side) + +| Path | What It Contains | Why It Matters | +|------|------------------|----------------| +| `packages/data-schemas/src/schema/` | Database schema definitions | DB schema changes → API response shape changes | +| `packages/data-schemas/src/models/` | Typed model wrappers | Typed facade over DB schemas | +| `packages/data-schemas/src/types/` | TypeScript type exports | Consumed by controllers | + +## API Package (newer, recent versions) + +| Path | What It Contains | Why It Matters | +|------|------------------|----------------| +| `packages/api/src/` | acl, agents, apiKeys, app, auth, cache, cdn, cluster, crypto, db, ... | New monorepo workspace introduced in recent versions — scan for any newly-exposed public surface | + +## Web Client (reference for feature parity) + +| Path | What It Contains | Why It Matters | +|------|------------------|----------------| +| `client/src/components/` | React UI components by feature area | Reference for mobile UI feature parity | | `client/src/hooks/` | Custom React hooks (data fetching, state) | Shows how the web app consumes APIs — informs ViewModel design | -| `client/src/store/` | Recoil/Jotai state atoms | Global state patterns — informs Android state management | -| `client/src/Providers/` | React Context providers | Feature flags, config-driven UI — informs Android feature gating | +| `client/src/data-provider/` | Query/mutation modules by domain (Agents, Auth, Endpoints, Files, MCP, Memories, Messages, SSE, Tools, ...) | **Often the most accurate mapping of feature → endpoint** — prefer over guessing from `hooks/` | +| `client/src/store/` | Recoil/Jotai state atoms | Global state patterns — informs mobile state management | +| `client/src/Providers/` | React Context providers | Feature flags, config-driven UI — informs mobile feature gating | + +## Server Config Schema + +| Path | What It Contains | Why It Matters | +|------|------------------|----------------| +| `librechat.example.yaml` | Reference server config | New keys gate new `/api/config` response fields | +| `.env.example` | Required/optional env vars | Changes hint at new feature availability | + +## Release Notes (authoritative) + +GitHub Releases often explicitly flag breaking changes and migrations that diffs hide. +For every stable tag between current and target (inclusive): +```bash +gh api repos/danny-avila/LibreChat/releases/tags/{tag} --jq .body +``` +Falls back to `WebFetch` on `https://github.com/danny-avila/LibreChat/releases/tag/{tag}`. + +Upstream has no repo-root `CHANGELOG.md` — release notes on GitHub are the canonical changelog. ## Diff Command Template +Overview: ```bash cd upstream && git diff {old_tag}..{new_tag} --stat -- \ api/server/routes/ \ api/server/controllers/ \ + api/server/middleware/ \ + api/server/services/ \ + api/models/ \ packages/data-provider/src/ \ packages/data-schemas/src/ \ + packages/api/src/ \ client/src/components/ \ client/src/hooks/ \ - client/src/store/ + client/src/data-provider/ \ + client/src/store/ \ + client/src/Providers/ \ + librechat.example.yaml \ + .env.example ``` -For detailed diffs, run each path separately to keep output manageable. +For detailed diffs, run each path separately to keep output manageable: +```bash +cd upstream && git diff {old_tag}..{new_tag} -- packages/data-provider/src/api-endpoints.ts +``` + +## Commit-message scan (complements the diff) + +```bash +cd upstream && git log --oneline {old_tag}..{new_tag} +``` +Breaking changes are frequently summarized in subject lines (e.g., `BREAKING CHANGE:`, `refactor!:`).