Skip to content

fix(db): preserve explicit provider priorities on edit - #15485

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.52from
piyush97:fix/preserve-provider-priority
Oct 6, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.52from
piyush97:fix/preserve-provider-priority

Conversation

@piyush97

@piyush97 piyush97 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Saving a connection's priority succeeds but silently changes the value in SQLite. With one connection, saving 5 returns 5 in the response but stores 1; with multiple connections, the same edit renumbers every sibling. This also happens when an unrelated dashboard edit resubmits the existing priority.

The update path now preserves explicit positive priorities. The existing internal priority: 0 move-to-top operation still compacts the provider's order, but returns the reread connection so its response agrees with the database.

Related Issues

Closes #15014.

Validation

  • Change type: DB
  • Focused tests and category gates from the golden path
  • npm run lint
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Verified against release/v3.8.52 at 23a11484862b3bb589a55e85b00e4ac53ffeb234, with Node 22.23.3:

  • Existing provider CRUD baseline: 15/15 passed before adding regressions.
  • Added regressions failed before the fix: singleton persisted 1 instead of 5; editing the first of two connections changed both priorities to ranks instead of keeping 2 and 10.
  • After the fix, 24/24 tests pass across db-providers-crud.test.ts, codex-connection-edit-6562.test.ts, and codex-bulk-import-preserve-state-11954.test.ts, run as separate processes with the repo's data-directory isolation and DISABLE_SQLITE_AUTO_BACKUP=true.
  • npm run lint: passed.
  • npm run typecheck:core: passed.
  • Pre-commit and commit-message hooks: passed; not bypassed.
  • npm run typecheck:noimplicit:core: blocked by the existing open-sse/services/combo/autoStrategy.ts:519 TS7006 error. I reran it with the original, unmodified providers.ts and got the identical failure. No changes to that unrelated file.

The full unit matrix, coverage, and production build are left to PR CI, as the contribution guide recommends.

Tests Added Or Updated

  • tests/unit/db-providers-crud.test.ts: exact priority survives update, read-by-ID, cached list, and unrelated rename; sibling priorities remain unchanged while ordering follows the new value; internal zero/move-to-top still works and its response matches the final rank.
  • tests/unit/codex-connection-edit-6562.test.ts: real authenticated PUT preserves priority 142 in both its response and SQLite instead of silently normalizing to 1.

Coverage Notes

The added tests exercise the real SQLite write/read path, including cached reads, and the existing management API test drives the real PUT handler. No mocks replace the priority persistence behavior. Existing creation, deletion, and bulk-import tests still pass.

Reviewer Notes

This is deliberately an update-path fix. Creation and single-deletion retain their existing priority compaction behavior; it does not promise lifecycle-wide preservation of sparse priorities. No schema migration, feature flag, or live-provider credentials are needed. My running gateway was not modified.

@piyush97
piyush97 requested a review from diegosouzapw as a code owner October 3, 2026 23:15
@diegosouzapw
diegosouzapw merged commit 511ea7e into diegosouzapw:release/v3.8.52 Oct 6, 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.

fix(providers): Unable to set provider priority

2 participants