Skip to content

🔀 feat: Save Config Edits as One Change Batch Against the Loaded Revision - #175

Merged
danny-avila merged 9 commits into
mainfrom
pr/config-save-pipeline-v2
Oct 24, 2026
Merged

danny-avila merged 9 commits into
mainfrom
pr/config-save-pipeline-v2

Conversation

@danny-avila

@danny-avila danny-avila commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Pull Request Template

⚠️ Before Submitting a PR, Please Review:

  • Please ensure that you have thoroughly read and understood the Contributing Docs before submitting your Pull Request.

⚠️ Documentation Updates Notice:

  • Kindly note that documentation updates are managed in this repository: librechat.ai

Summary

Builds on the config contract PRs, and includes the latest of #174. A LibreChat server that reports both changesBatch and expectedRevision (GET /api/admin/config/meta) now gets each save as one atomic POST /api/admin/config/:principalType/:principalId/changes batch, sent with the revision the edits were made against. A server without either flag, or without the meta route, keeps the separate field writes with rollback and read-back. That path has no revision check, so "Changed elsewhere" needs a server with change batches. Both kinds of server were checked live. The older one does not prune emptied parents, so some saves leave a different stored document there (see Testing).

One batch per save

  • Pending edits to Base or the open profile are planned as before: keyed entries by identity, validation and write rules. The plan is then sent as one batch in the order the separate writes use: unhides, hides, sets, then resets (one unset per path).
  • The server applies a batch whole or not at all. This path no longer needs the hand-written rollback, and a refused batch leaves nothing to undo.
  • A plan with more changes than limits.maxChangeOps (1000 on current dev) is split into several batches, only between config sections. All changes to one section (a hide and the values that replace it, an entry and the settings that depend on it) go in the same batch, so no batch leaves a section half-changed.
    • Each batch is planned again from its own changes, and nothing is sent unless every batch is valid on its own.
    • A section with more changes than one batch takes is refused before anything is written. The error names the section and the limit.
    • Each batch is sent with the revision the previous one left. If a later batch fails, the error says that the earlier batches stay saved. This includes a 409 on a later batch, which is not reported as "Changed elsewhere" because part of the save was written.
  • Base batches carry priority: 0, as Base PATCHes did.
  • Imports into Base or a profile use the same path.

Revisions and "Changed elsewhere"

  • Base and profile reads keep the document's revision. A Base with no document yet uses absent.
  • Save sends the revision the target was at when the first pending edit was made. A re-read while edits are pending (opening the review, or a window-focus refetch) does not change it. Once the edits are saved or dropped, it follows the loaded revision again.
  • The server may find that the target was saved elsewhere since then, either when it re-reads the target or as a 409 CONFIG_VERSION_CONFLICT from the batch. In both cases nothing is written.
  • The review then shows a "Changed elsewhere" notice. The page already shows what is stored, and a setting that changed after the admin edited it is marked "Changed since you loaded it".
    • Reload drops the edits (with Undo) and closes the review.
    • Merge and save sends the edits over the latest version. Changes the other admin saved to fields these edits don't touch are kept.
    • The review's own Save stays disabled while the notice is shown. Closing the review hides the notice. Opening it again re-reads the target and compares the edits with what is stored now. Its Save still sends the revision the edits were made against, so it shows "Changed elsewhere" again until the admin reloads or merges.
  • A save that has no loaded revision (an import) and loses a race with another write is planned and sent once more.

Write reports

  • A batch reply always lists what it skipped and what the merge ignores, and the panel reads both lists on any server with change batches:
    • A change skipped as a whole, or stored without effect, is not counted as applied.
    • A skipped or ignored part of a change is reported without lowering the count.
    • A reply's ignored covers the whole stored document, and an entry list is sent as the collection's whole override, which can carry reported parts of entries the save did not change. So, as with 🧾 feat: Write Reports, Profile Priority, Pre-Auth Sign-In and Per-Field Titles from the Config Contract #174's field writes, a dropped credential (secret_rebinding), or a skipped or ignored part, is listed only when the save changed what it belongs to: its custom endpoint, its MCP server, the Azure groups, or the object that holds it (OCR, a speech provider). A later save of another setting, or of another entry in the same list, does not repeat it.
    • Such a part is listed apart from the skipped changes, once however many batches of a split save repeat it. An import counts only whole skips against its entries.
  • After a batch, the check for what did not take effect needs no second read of the stored document, because the reply returns it.
    • For Base, the effective config is read once, without the delayed retry, on servers that clear their caches before replying (awaitedInvalidation).
    • A finding at, inside or around a location the reply already reported is not listed twice.
  • A refused change (opIndex) is shown on the field it addresses. Validation issues keep the existing segment and parameter mapping.

Validation of the end state

  • With change batches, the pre-flight checks validate the end state, as the server does. Each section the save writes is checked against the layer beneath, minus the values the target hides before and after the save.
  • Hiding a librechat.yaml value on its own is now checked. For example, hiding the YAML memory agent ID without setting a provider and model is blocked before anything is sent.
  • Older servers keep the tombstones-first check.

Emptied parents (prunesEmptyParents)

  • On a server that removes the objects a delete leaves empty, a reset is sent at its own path. It is no longer widened to the emptied parent. Older servers keep the widening.

Hiding or showing an entry with edits inside it (both kinds of server, with one difference in what is stored)

  • An unhide removes everything stored at and under its path, so it is now written first, in a batch and as separate field writes. Showing an MCP server again for a profile and editing its URL in the same save keeps the URL. Before, the unhide was written last and removed the URL, and the save reported the URL as not stored and pointed to librechat.yaml.
  • Hiding an entry and editing a field inside it in the same save keeps the entry hidden. The edit is not written, since the hide removes it anyway. Before, the edit was written after the hide and cleared it. On a server without prunesEmptyParents, the hide still leaves the emptied parent beside the tombstone (mcpServers: {} when the profile stored only that server). The server stays hidden.
  • On a server without prunesEmptyParents, a reset now widens over a stored tombstone that the same save unhides. Switching the memory agent back to the librechat.yaml agent ID no longer leaves an empty memory.agent object.

Not in this PR:

  • Keyed entry ops in the batch (upsertEntry, removeEntry, rename). Entries are still sent as the collection's whole override, merged by identity before sending.
  • dryRun validation.
  • One existing review behaviour is unchanged: after another admin changes a custom endpoint this admin also edited, the review compares the edited copy of the entry, so a field only the other admin changed can read as reverted. The save itself applies only this admin's own changes, and the other admin's value stays.

Change Type

  • New feature (non-breaking change which adds functionality)

Testing

Unit and component tests:

  • Planner:
    • With prunesEmptyParents, a reset is not widened.
    • With change batches, a hide on its own is validated.
    • Batch ops keep their order (unhides, hides, sets, resets), with one change per path.
    • An unhide goes before the edits and hides inside it. An edit inside a path the same save hides is not written.
    • A reset widens over a stored tombstone the same save unhides.
  • Save path, against an in-memory admin API with revisions and change batches:
    • One batch with the loaded revision, or absent when there is no document.
    • A reset sent as an unset without widening.
    • A conflict found before writing, and on a 409.
    • The single retry for imports.
    • A refused batch, located by opIndex, with nothing rolled back.
    • Skipped and ignored reports, kept apart from the count.
    • A dropped credential listed for the entry the batch changed, not for an unrelated save or one of another entry sent in the same list, and once across split batches. A reported part inside another entry of a list sent whole is not listed.
    • Batches chained by revision above maxChangeOps, a partial failure, and a 409 on a later batch reported as a partial write.
    • Batches split only between sections, against an in-memory API that refuses a batch whose end state is invalid. The memory agent switch stays in one batch. A section above the limit is refused with nothing written.
    • Showing a server again with its URL edited, and hiding a server with its URL edited, in a batch and as separate field writes.
    • Switching the memory agent back to the librechat.yaml ID as separate field writes, leaving no empty object.
    • Separate field writes when the server has expectedRevision without changesBatch.
    • The memory agent switch as one batch, and a lone hide blocked.
  • Revision reads for Base (a document, no document, or no revision reported) and for profiles.
  • The batch report reader, by the locations a save changed: a dropped credential by its entry or the object that holds it, and a part of a collection sent whole by its entry.
  • Hooks:
    • The loaded revision is sent (absent without a document). Older servers get none.
    • A conflict keeps the edits. Merge sends no revision and clears them on success. Reload drops them and closes the review.
    • A partial write after a 409 on a later batch shows the "Save failed partway" error, not the conflict notice.
    • After a conflict and the page's re-read, the review's Save sends nothing and keeps the edits, and Merge still saves.
    • A re-read while edits are pending keeps the revision they were made against, so Save meets the conflict. After the review is closed and opened again, Save still sends that revision and conflicts again until Merge.
    • A re-read with no pending edits, and saving or discarding the edits, let the revision follow the loaded one.
    • A section above the batch limit is named in the save error, and the edits are kept.
  • Review: the "Changed elsewhere" notice, the target it names, and its actions. The footer Save is disabled while the notice is shown.

Live, with Playwright and MongoDB checks:

  1. Current dev:
    • Base:
      • Two interface settings saved as one write (configVersion +1).
      • Resetting one setting and then the other left {}, without an empty interface object.
    • Conflict, then Merge:
      • Edited "Model select". Another write set "Presets" while the review was open.
      • Save showed "Changed elsewhere" and wrote nothing. "Merge and save" kept both values.
    • Conflict, then Reload: the edits were dropped with Undo, the review closed, and the page showed the other write's value.
    • Changed field: the admin set "Message window size" to 5 while another write set 7. The review marked "Changed since you loaded it; now 7", and Merge stored 5.
    • Role USER profile:
      • The same flow, with the notice naming "Role: USER".
      • An edit that equalled the other admin's value dropped out of the review. Merge stored both settings.
    • Custom endpoint: edited Moonshot's display label while another write set its title model. Merge stored both fields on the entry.
    • Importing three settings into Base made one write.
    • Switching the memory agent from the librechat.yaml agent ID to a provider and model made one write, with the hide and both values. Switching back made one write and left no override or tombstone.
    • Role USER hides a Base MCP server. Clicking Unhide and changing its Server URL stored the URL with no tombstone, and the toast said "Changes saved". Changing the URL and then hiding the server stored only the tombstone.
    • The notice was checked in light mode, dark mode and at phone width.
    • Two tabs on Base: tab A set "Presets" and opened the review. Tab B saved "Parameters". Tab A's Save showed "Changed elsewhere", and its footer Save was disabled. Clicking it and pressing Enter wrote nothing (same revision). Then:
      • "Merge and save" stored both values.
      • Or: Cancel, then the review opened again listed only tab A's edit against what tab B stored. Its Save showed "Changed elsewhere" again and wrote nothing, and "Merge and save" stored both.
      • Or: "Reload" discarded the edit with Undo, and the stored document kept tab B's value. Setting "Presets" again then saved without a conflict.
    • Refetch while editing, two tabs on Base: tab A set "Presets". Tab B saved "Parameters". Tab A refetched on window focus (after the 30-second stale time), and its review re-read the target again. Tab A's Save showed "Changed elsewhere" and wrote nothing. The build before this fix saved there without a notice.
    • The same with no pending edit in tab A at the refetch: setting "Presets" afterwards saved without a conflict, and both values were stored.
    • On dev, a batch that hides the librechat.yaml memory agent ID alone is accepted, and so is a following batch that sets only its provider. That leaves memory.agent: { provider } stored without a model, the kind of intermediate state a split by position could send. The split by sections was checked in unit tests only, since dev takes 1000 changes per batch.
    • Dropped credentials in role USER:
      • Changing GPT's base URL saved with "API key: not sent: …" in the toast.
      • Then the OpenAI title model, and ClickHouse's display label, each said "Changes saved". The ClickHouse save writes the whole endpoints.custom override, GPT's entry included. A dry-run batch afterwards showed that the server still reports GPT's dropped key for that document.
      • Changing GPT's display label listed GPT's API key again.
      • Importing a new GPT base URL said "Imported 1 value into Role: USER", with the dropped key in the notice. It made one write.
  2. An older LibreChat without the meta route (the same panel build):
    • Saves used separate PATCH, tombstone and DELETE field writes, with no revision. Nothing checks for a write elsewhere, so "Changed elsewhere" never shows there. Conflicts, split batches and write reports were not checked on this server, since it has none of them.
    • Base: two interface settings saved. Resetting one and then the other left {}, because the reset was widened to the emptied interface object.
    • Role USER hides a Base MCP server. Unhide with its Server URL changed stored the URL with no tombstone, and the toast said "Changes saved", as on dev.
    • Changing the URL and then hiding the server stored the tombstone, and also an empty mcpServers object. On dev the document had no mcpServers object. The server stayed hidden on both.
    • Switching the memory agent back from a provider and model to the librechat.yaml agent ID left no memory override and no tombstone, as on dev.

Test Configuration:

  • LibreChat dev, and an older LibreChat (f7a56e8, #17204) without GET /api/admin/config/meta.
  • MongoDB 8.2 replica set, production-like librechat.yaml.
  • npx tsc --noEmit, npx eslint src/ e2e/, npx prettier --check on the changed files, bun run test:lint-rules, npx vitest run (2707 tests).

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

…nd Accept Dotted Keys

Adds a gating layer that reads the admin config contract (GET /api/admin/config/meta) with bundled
fallbacks for older servers, accepts dotted record keys where the server keeps them, and fixes a
set of smaller configuration issues.
…ld Titles from the Config Contract

Shows what a save skipped or ignored and why, maps issues by segments and params, edits profile
priority and creates profiles create-only, makes the pre-auth Base fields editable, and resolves
titles per field — each gated on the server contract with today's behaviour as the fallback.
…aths in Profile Merges, Count Partial Reports Apart
…sion

On servers with change batches and revisions, saves each principal's pending edits as one POST
/changes batch against the loaded revision, with a changed-elsewhere flow on conflict; older servers
keep the field-by-field save path.
@danny-avila

Copy link
Copy Markdown
Contributor Author

@codex review

Pass 1. Head: 5c7ccaa

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T19:50:37.184550Z 3e25117 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c7ccaabd0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/hooks/config/save.ts Outdated
Comment thread src/server/config/save.ts Outdated
@danny-avila

Copy link
Copy Markdown
Contributor Author

@codex review

Pass 2. Head: 3e25117

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 3e25117616

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danny-avila
danny-avila added this pull request to stack #177 October 24, 2026 06:53
@danny-avila
danny-avila removed this pull request from stack #177 October 24, 2026 06:55
@danny-avila
danny-avila changed the base branch from pr/config-backend-feature-ui to main October 24, 2026 07:00
@danny-avila
danny-avila merged commit a202197 into main Oct 24, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant