feat(lora-studio): LoRA Studio v1 - Civitai ingest and archive - #2374
Conversation
Ingest and browse Civitai LoRA adapters: card grid with preview/base-model/type badges and tag chips, a detail panel with click-to-copy trigger words, and an Add-by-URL form that posts form-encoded (matching the backend's Form field, not JSON like the existing library.ts ingest bug). The list auto-refreshes while any row is pending/downloading and shows a failed row's error reason inline, including the geo-block/proxy message.
New LoraStore + /api/loras/* routes archive Civitai LoRA/LoCon/DoRA models: safetensors download (SHA256-verified via the existing download_file), up to 4 preview images, and metadata from the Civitai API. Civitai's edge geo-blocks some regions with HTTP 451, so a new lora_ingest_proxy_url config key lets the fetcher (and only the fetcher) go out through an explicit proxy. Every failure mode -- 451, connect error, SHA256 mismatch, non-LoRA model type -- sets status=failed with a specific reason and leaves no partial file on disk. LoRA files live under models_root()/loras/ and routes/models.py's disk scan now excludes that subtree so adapters never show up as loadable models. detect_kind() gains url:civitai and a CivitaiProcessor so /api/library/ingest routes Civitai URLs into the same ingest job.
The registry row was marked optional: true, but getLaunchableApps filters optional apps against the installed set from /api/apps/optional/installed, which is gated by the hardcoded OPTIONAL_FRONTEND_APPS allowlist in routes/apps.py. lora-studio is not in that allowlist, so the app never rendered in the launcher while every component test still passed. Drop the flag so it is an always-on app, and lock it with two tests that fail against the optional row.
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 15 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds LoRA Studio support for Civitai URL ingestion, persistent metadata and files, API routes, Library integration, model-scan exclusion, and a desktop interface for monitoring and managing LoRAs. ChangesLoRA Studio
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LoRAStudioApp
participant LoRAAPI
participant Civitai
participant LoraStore
participant LibraryPipeline
User->>LoRAStudioApp: Enter Civitai URL
LoRAStudioApp->>LoRAAPI: POST /api/loras/ingest
LoRAAPI->>LoraStore: Create pending record
LoRAAPI->>Civitai: Fetch metadata and download files
LoRAAPI->>LoraStore: Update status and metadata
LoRAStudioApp->>LoRAAPI: Poll LoRA list
LibraryPipeline->>LoRAAPI: Process Civitai Library item
LibraryPipeline->>LoraStore: Link LoRA artifact and status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
tests/test_lora_studio.py (1)
310-366: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a test for a hostile
files[].namevalue.The suite covers 451, connect errors, a non-LoRA type, and a checksum mismatch. It does not cover a Civitai response whose file
namecontains path separators. That is the traversal case raised ontinyagentos/routes/lora_studio.pyLines 285-293.Add a case with
"name": "../../evil.safetensors"and assert the written file stays insideisolated_loras_root.💚 Proposed test
`@pytest.mark.asyncio` async def test_traversal_filename_stays_inside_root( self, lora_store, isolated_loras_root, monkeypatch ): hostile = { **CIVITAI_MODEL_JSON, "modelVersions": [{ **CIVITAI_MODEL_JSON["modelVersions"][0], "files": [{ "primary": True, "name": "../../evil.safetensors", "downloadUrl": "https://civitai.com/api/download/models/999", "hashes": {}, }], }], } lora_id = "lora-test-2851174" await lora_store.create_pending( lora_id, source_url="https://civitai.com/models/2851174", civitai_model_id=2851174, civitai_version_id=None, ) _patch_get(monkeypatch, _civitai_response(200, hostile)) monkeypatch.setattr(ls, "download_file", _fake_download_ok) await run_civitai_ingest(lora_store, lora_id, proxy_url="") row = await lora_store.get(lora_id) written = Path(row["file_path"]).resolve() written.relative_to(isolated_loras_root.resolve())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_lora_studio.py` around lines 310 - 366, Add a pytest case alongside TestRunCivitaiIngestHappyPath that supplies a primary Civitai file named "../../evil.safetensors", runs run_civitai_ingest, resolves the resulting row["file_path"], and asserts it is relative to isolated_loras_root. Reuse the existing pending-row setup, response patching, and _fake_download_ok helper.tinyagentos/routes/lora_studio.py (1)
43-66: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider a concurrency limit for ingest jobs.
POST /api/loras/ingeststarts one background download per request with no cap. LoRA safetensors files are commonly hundreds of megabytes. Repeated submissions can saturate disk and network bandwidth.An
asyncio.Semaphorearoundrun_civitai_ingest, or a small worker queue, would bound the load. Rows already carry apendingstatus that a queue can drain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/routes/lora_studio.py` around lines 43 - 66, Bound concurrent LoRA ingest jobs scheduled by _schedule and executed via run_civitai_ingest, using an asyncio.Semaphore or small worker queue. Ensure each job acquires the limit before downloading and releases it afterward, while preserving the existing pending-status flow and background-task tracking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@desktop/src/lib/loras.ts`:
- Around line 38-49: Update fetchJson and LoRAStudioApp.fetchLoras so failed
LoRA list requests are distinguishable from successful empty results, allowing
fetchLoras to set listError and preserve the existing items when refreshes fail.
Do not return the fallback for HTTP, invalid-content-type, or transport failures
in the list path; propagate an error or use an explicit error result while
retaining fallback behavior where appropriate.
- Around line 111-120: Update desktop/src/lib/loras.ts lines 111-120 so
retryLora returns a dedicated result containing only id and status, rather than
declaring the response as LoraItem. Update desktop/src/apps/LoRAStudioApp.tsx
lines 190-197 to merge that retry result into the existing selected item, or
refresh the list immediately, preserving the item’s name, previews, tags, and
metadata.
In `@tinyagentos/config.py`:
- Around line 100-101: Update AppConfig.to_dict() to omit lora_ingest_proxy_url
from the dictionary returned by GET /api/config, while retaining the
configuration field for internal persistence and LoRA ingest behavior. Remove
only its serialization in to_dict(); do not change WebhookNotifier or the
underlying AppConfig storage.
In `@tinyagentos/library_pipeline.py`:
- Around line 647-650: Update the proxy URL lookup in the library pipeline to
avoid calling load_config, since that read can rewrite config.yaml. Reuse the
already-loaded application configuration when available, or parse only
lora_ingest_proxy_url directly from config_path without invoking save_config or
other mutating configuration logic.
- Around line 638-657: Update the pipeline flow around the LoraStore creation in
the library processor to accept and reuse the application-owned
app.state.lora_store instead of constructing and initializing a per-item store.
Thread this shared store through the relevant pipeline entry points and use it
for create_pending, run_civitai_ingest, and get; remove the per-item close so
ownership remains with the application.
In `@tinyagentos/routes/lora_studio.py`:
- Around line 285-293: Sanitize the API-provided filename before constructing
dest in the download flow: reduce it to its basename, then reject empty or
dot-only results and use the existing slug-based fallback when appropriate.
Ensure the resulting path always remains under lora_dir while preserving the
current download and cleanup behavior.
---
Nitpick comments:
In `@tests/test_lora_studio.py`:
- Around line 310-366: Add a pytest case alongside TestRunCivitaiIngestHappyPath
that supplies a primary Civitai file named "../../evil.safetensors", runs
run_civitai_ingest, resolves the resulting row["file_path"], and asserts it is
relative to isolated_loras_root. Reuse the existing pending-row setup, response
patching, and _fake_download_ok helper.
In `@tinyagentos/routes/lora_studio.py`:
- Around line 43-66: Bound concurrent LoRA ingest jobs scheduled by _schedule
and executed via run_civitai_ingest, using an asyncio.Semaphore or small worker
queue. Ensure each job acquires the limit before downloading and releases it
afterward, while preserving the existing pending-status flow and background-task
tracking.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 38500c7f-09dc-455d-bf86-6007599f83e0
📒 Files selected for processing (16)
changelog.d/lora-studio-backend.mddesktop/src/apps/LoRAStudioApp.test.tsxdesktop/src/apps/LoRAStudioApp.tsxdesktop/src/lib/loras.tsdesktop/src/registry/app-registry.test.tsdesktop/src/registry/app-registry.tstests/conftest.pytests/test_lora_studio.pytinyagentos/app.pytinyagentos/config.pytinyagentos/installers/download_installer.pytinyagentos/library_pipeline.pytinyagentos/lora_store.pytinyagentos/routes/__init__.pytinyagentos/routes/lora_studio.pytinyagentos/routes/models.py
| row = await store.get(lora_id) | ||
| if not row: | ||
| return JSONResponse({"error": f"LoRA {lora_id!r} not found"}, status_code=404) | ||
| if row.get("status") != "failed": |
There was a problem hiding this comment.
WARNING: Race condition in retry endpoint — the read-then-check-then-update sequence is not atomic.
If two retry requests arrive concurrently for the same failed LoRA, both can read status == "failed" before either writes, allowing duplicate background tasks to be scheduled. Both tasks then download to the same loras_root()/<slug>/ directory, racing on file writes and directory cleanup.
An atomic UPDATE ... WHERE status = 'failed' with a rows-affected check (or a per-LoRA lock) would serialize retries safely.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Confirmed and fixed with an atomic transition rather than a lock. LoraStore.claim_retry() is a single UPDATE ... WHERE id = ? AND status = 'failed' and returns whether it won; the route schedules the job only for the winner and 409s the loser. Red first, two retries in flight:
> assert codes == [202, 409]
E assert [202, 202] == [202, 409]
Now [202, 409] with exactly one background job scheduled.
| except Exception as exc: | ||
| # Loud failure: never leave a partial/error-page file on disk, and | ||
| # never report success on a fetch that could not be verified. | ||
| if lora_dir.exists(): |
There was a problem hiding this comment.
WARNING: Failure cleanup in run_civitai_ingest is not safe against concurrent tasks for the same LoRA.
shutil.rmtree(lora_dir) on any failure will delete the entire LoRA directory. If another background task for the same lora_id is mid-download (possible because ingest_lora/retry_lora are not serialized), this cleanup deletes files the other task is actively writing, causing it to fail as well.
Consider holding a per-LoRA lock, or at minimum checking that no other task is actively working on the same slug before removing the directory.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Partly addressed. The retry path — the one a user can actually trigger twice, by double-clicking Retry — is now serialised by an atomic failed → pending transition (LoraStore.claim_retry), so two jobs can no longer be scheduled for the same LoRA that way; see the reply on the retry-route thread for the red proof.
The remaining window needs a deliberate re-POST of the same URL while a download is running. I am not adding a per-LoRA lock for it in this PR: the cleanup only ever removes loras_root()/<slug>/, the ingest then fails loud and records the error on the row, and a retry recovers it. Noted rather than silently dropped.
| def _pick_file(version: dict) -> dict: | ||
| files = version.get("files") or [] | ||
| for f in files: | ||
| if f.get("primary"): |
There was a problem hiding this comment.
WARNING: _pick_file returns the primary file without verifying it is a .safetensors file.
The first loop returns any file marked primary regardless of extension, and the .safetensors fallback loop only runs when no primary file exists. If Civitai ever marks a .bin (or other non-.safetensors) file as primary for a LoRA, it would be downloaded and archived instead of a .safetensors file.
Validate the extension in the primary-file check, or prefer .safetensors over the primary flag to match the error message's contract ("No .safetensors file found").
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Confirmed and fixed. _pick_file now checks the extension on the primary pass too, so a primary non-safetensors file no longer wins over a real adapter. Red first:
> assert ls._pick_file(version)["name"] == "real-lora.safetensors"
E AssertionError: assert 'training-data.zip' == 'real-lora.safetensors'
A primary .safetensors still takes priority over a non-primary one, and the "No .safetensors file found" error now matches what the function actually enforces.
| /* Helpers */ | ||
| /* ------------------------------------------------------------------ */ | ||
|
|
||
| async function fetchJson<T>(url: string, fallback: T, init?: RequestInit): Promise<T> { |
There was a problem hiding this comment.
SUGGESTION: fetchJson returns a fallback on any HTTP error or network exception, so listLoras cannot distinguish "no LoRAs" from "server error".
On a 500 or network failure the UI silently shows the empty state ("No LoRAs yet") instead of surfacing the error. The backend is explicitly designed to fail loud; the frontend should match that by propagating or surfacing errors rather than swallowing them.
Consider returning a discriminated result (e.g. {data, error}) or adding a separate error path so the component can show an actionable error.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Confirmed and fixed. listLoras now throws on HTTP, content-type, and transport failures instead of falling back to an empty list, and the app's existing listError path finally becomes reachable; the "No LoRAs yet" empty state is suppressed while an error is showing, so a failed load can no longer be mistaken for an empty archive. Red first:
× surfaces a list failure instead of the empty state
TestingLibraryElementError: Unable to find an element with the text: /Failed to load LoRAs/i
The now-unused fetchJson helper is gone; the other callers already had their own explicit handling.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous Review Summaries (2 snapshots, latest commit e2fb869)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit e2fb869)Status: No Issues Found | Recommendation: Merge Files Reviewed (11 files)
Previous review (commit be77bd2)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (16 files)
Reviewed by step-3.7-flash · Input: 48.8K · Output: 6.7K · Cached: 576K |
…try, config round-trip Adjudicated review findings on the LoRA Studio PR plus one the review missed. - POST /api/config rebuilt AppConfig without lora_ingest_proxy_url at both rebuild sites, so saving any setting through the config editor wiped the proxy. Same class as the taosmd hooks fix in #2368. - The safetensors file name comes from the Civitai API response and was joined to the LoRA directory unsanitised, so '../..' or an absolute path escaped the archive root, where neither the failure cleanup nor the delete route can see it. - _pick_file returned the primary file whatever its extension, contradicting its own 'No .safetensors file found' contract. - The retry route read status then updated, so two retries could schedule two downloads into one directory. The transition is now a single atomic UPDATE. - CivitaiProcessor read the proxy through load_config, which persists a legacy litellm_port pin, letting a background ingest rewrite the user's config. - listLoras swallowed HTTP and transport failures into an empty list, so a server error rendered as 'No LoRAs yet' and dropped the visible rows. - retryLora was typed as a full row but the route returns {id, status}, so a retry blanked the card's name, previews, and tags until the next poll. Docs: README carries LoRA Studio and the bundled-app counts are corrected against the registry (Archive and Hub were missing); the /api/loras surface and its session-only posture are documented in agent-coordination.md.
The error banner and 'No LoRAs yet' rendered together, which still let a server error read as an empty archive. Also moves the two config-side-effect tests into their own class rather than trailing the retry-atomicity one.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tinyagentos/lora_store.py (1)
75-82: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve the current archive until the replacement ingest succeeds.
Lines 75-82 replace a ready row with a pending row, but they do not isolate or remove the existing archive.
run_civitai_ingest()then writes into the same deterministic directory and removes that whole directory on failure. A repeated submission can therefore delete a valid ready archive after a transient Civitai or download failure.Download into a unique staging directory. Replace the archive and ready metadata only after checksum verification and metadata persistence succeed. Add a regression test that re-ingests a ready LoRA and forces the replacement download to fail.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/lora_store.py` around lines 75 - 82, Update the re-ingest flow around the lora metadata insertion and run_civitai_ingest() to preserve the existing ready archive and metadata until replacement ingest fully succeeds. Stage each replacement download in a unique temporary directory, perform checksum verification and metadata persistence there, then atomically replace the archive and mark the row ready; ensure failure cleanup only removes staging data. Add a regression test covering re-ingesting a ready LoRA with a forced replacement download failure and assert the original archive and ready metadata remain intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/agent-coordination.md`:
- Around line 551-555: Update the ingest behavior statement near the
direct-connection description to clarify that an empty lora_ingest_proxy_url
uses a direct request, which may succeed; ingestion fails loudly only if that
request fails or is rejected, including HTTP 451, while preserving the existing
trust_env=False and no-file behavior.
---
Outside diff comments:
In `@tinyagentos/lora_store.py`:
- Around line 75-82: Update the re-ingest flow around the lora metadata
insertion and run_civitai_ingest() to preserve the existing ready archive and
metadata until replacement ingest fully succeeds. Stage each replacement
download in a unique temporary directory, perform checksum verification and
metadata persistence there, then atomically replace the archive and mark the row
ready; ensure failure cleanup only removes staging data. Add a regression test
covering re-ingesting a ready LoRA with a forced replacement download failure
and assert the original archive and ready metadata remain intact.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f766b15b-a0c5-4919-a0a2-02ab0ec25f85
📒 Files selected for processing (11)
README.mddesktop/src/apps/LoRAStudioApp.test.tsxdesktop/src/apps/LoRAStudioApp.tsxdesktop/src/lib/loras.tsdocs/agent-coordination.mdtests/test_lora_studio.pytests/test_routes_config.pytinyagentos/library_pipeline.pytinyagentos/lora_store.pytinyagentos/routes/lora_studio.pytinyagentos/routes/settings.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tinyagentos/library_pipeline.py
A direct Civitai request succeeds from an unblocked host; the loud failure is the refused-and-no-proxy case, not every unconfigured install.
|
nemotron-super review VERDICT: Blocking issues found
Automated first-pass review by the nemotron-super lane. The lead still reviews before merge. |
Conflict was at both AppConfig rebuild sites in routes/settings.py: this branch adds github_app_id there, dev added lora_ingest_proxy_url in #2374. Same class, same two lines - resolved by keeping BOTH at both sites.
…ld-site rule doc-gate went red on the merge: this branch modifies a route module and changes user-visible behaviour, and dev's gate now demands both a route doc and a changelog fragment. The doc records the actual trap rather than just listing endpoints: both write paths rebuild AppConfig field by field, so a field missing from either is silently dropped on the next save. That has now happened twice in two days (#2375 and #2374), and the parity test is what stops the third.
CARD TITLE (intent, not commit subject): LoRA Studio v1: Civitai ingest + archive
Jay request 2026-08-12 (card tsk-6bg6ra, lead-held). Share a Civitai model URL to taOS, have it ingested and archived with its safetensors file, name, description, previews, tags and trigger words. Spec:
~/.taos-team/specs/lora-studio-spec.md.The premise was measured, not assumed
From the Pi (egress GB):
451 = Civitai's own edge geo-blocking the UK. It is NOT DNS blocking, NOT ISP interception, and NOT a JS/anti-bot wall - the documented REST API answers normally from a non-UK IP. So no browser container and no scraping are needed; this is one proxied HTTP hop. A browser+VPN container would have been a large fragile mechanism for a problem that isn't there.
Design
Opt-in config key
lora_ingest_proxy_url(empty = direct), passed per-request to the Civitai fetcher only. Nothing else in taOS changes egress.trust_env=Falseon those calls specifically, so a strayHTTPS_PROXYcannot silently override the explicit choice.Rejected: routing all taOS egress through the VPN (breaks LAN/Tailscale/local backends); relying on
HTTPS_PROXYenv (httpxtrust_env=Truewould affect every other client - an accident, not a mechanism).Storage:
models_root()/loras/<slug>/withpreviews/beside it.routes/models.pydisk scan now excludes that subtree - it accepts any.safetensorsit finds, so without the exclusion adapters would list as loadable models. taOS has no model-kind concept at all today; that is a real gap this works around rather than solves.Failure is loud by design
Until a proxy is configured every live fetch fails with an explicit, actionable error. It will NEVER write a 451 error page to a
.safetensorspath and report success. Red-proven:Same treatment for connect errors, sha256 mismatch, and non-LoRA model types (a 6 GB checkpoint is refused, not silently archived).
Lead-found defect that the green suite could not catch
The frontend registry row was
optional: true.getLaunchableAppsfilters optional apps against/api/apps/optional/installed, gated by the hardcodedOPTIONAL_FRONTEND_APPSallowlist inroutes/apps.py, which has nolora-studioentry - the app would never have appeared in the launcher, while 3375 component tests passed, because they mount the component directly and nothing exercised launcher visibility. Fixed in be77bd2 and locked with two tests, red at the merge ref:Verification on the MERGED branch (neither half was ever tested against the other)
pytest tests/test_lora_studio.py tests/test_library.py tests/test_routes_models.py tests/test_config.py tests/test_routes_config.py tests/installers/-> 268 passednpx vitest run(full suite) -> 393 files, 3377 passednpx tsc --noEmit-> clean;compileall tinyagentos/-> rc 0{loras: [...], count: N}, frontend reads that shape andmeta_json.typefrom the verbatim stored API response.Known follow-ups (deliberately not in v1)
routes/apps.pyif wanted later.Review round: 10 bot findings adjudicated, plus one they missed (2026-08-12)
Every finding was checked against source before acting. Seven fixed, three declined with reasoning on-thread. Each fix was proven red against the pre-fix code first; output is fenced below, produced by reverting the four source files to
be77bd22and running the new tests.Lead-found, not reported by either bot:
PUT /api/configrebuiltAppConfigwithoutlora_ingest_proxy_urlat both rebuild sites, so the config editor's read-edit-write loop silently wiped the proxy on the next save. Same class as the taosmd hooks fix in #2368.Untrusted file name (CodeRabbit,
lora_studio.py:293) - the safetensors name comes from the Civitai API response and was joined to the LoRA directory unsanitised, so it could be written outside the archive root, where neither the failure cleanup norDELETE /api/loras/{id}can see it:File-type contract (Kilo,
lora_studio.py:200) -_pick_filereturned the primary file whatever its extension, contradicting its own error message:Retry race (Kilo,
lora_studio.py:488) - read-then-check-then-update let two retries schedule two downloads into one directory. Now one atomicUPDATE ... WHERE status = 'failed':Background config write (CodeRabbit,
library_pipeline.py:650) -load_config()persists a legacylitellm_portpin, so a Library ingest rewrote the user'sconfig.yaml:Frontend error handling (CodeRabbit
loras.ts:49+ Kiloloras.ts:38, and CodeRabbitloras.ts:120) - a failed list rendered as "No LoRAs yet", and a retry blanked the card:Declined, with reasoning on-thread: redacting
lora_ingest_proxy_urlfromAppConfig.to_dict()(that method is whatsave_configwrites, so it would make the key unpersistable; the endpoint is admin-session-only and already returns webhook URLs); a per-LoRA lock for the ingest-vs-cleanup window (the reachable half is fixed atomically, the rest fails loud and is recoverable); and reusing the app-ownedLoraStorein the processor (needs an app handle threaded through the whole processor registry; same idiom already exists indisk_quota.py).Docs (closes the doc-gate red): README carries LoRA Studio, and the bundled-app counts are corrected against the registry - they read 40/27 while the registry has 43 non-optional apps, because Archive and Hub were never added.
docs/agent-coordination.mddocuments the six/api/loras*routes and their session-only posture (no registry scope reaches them).Pre-existing, not fixed here: both
AppConfigrebuild sites inroutes/settings.pyalso droparchive,archived_agents, andgithub_app_id. Same data-loss class, older than this PR - carded separately rather than widened into this one.Summary by CodeRabbit
New Features
Bug Fixes