Skip to content

fix gateway | prevent pairing state loss during concurrent DM pairing updates - #5572

Closed
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/pairing-concurrency-lock
Closed

Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/pairing-concurrency-lock

Conversation

@Dusk1e

@Dusk1e Dusk1e commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Summary

This fixes a concurrency bug in the DM pairing store where concurrent gateway/CLI operations could silently overwrite each other's state.

Changes in this PR:

  • add a cross-process, reentrant advisory lock for pairing state access
  • make pairing JSON writes atomic with temp-file + os.replace()
  • serialize read-modify-write flows for:
    • pending pairing codes
    • approved users
    • rate-limit state
    • lockout counters
  • add regression tests covering concurrent updates

Problem

gateway/pairing.py stored pairing state in JSON files, but the read/modify/write flow was not synchronized across processes.

That meant concurrent operations like:

  • generating pairing codes from the gateway while approving codes from the CLI
  • multiple pairing requests arriving close together
  • overlapping rate-limit or lockout updates

could race and cause silent state loss.

In practice, one writer could load stale JSON, modify it, and overwrite newer changes from another writer. This could drop:

  • pending codes
  • approved users
  • rate-limit entries
  • failed-attempt / lockout state

Fix

This PR introduces a shared pairing store lock and wraps pairing state mutations in serialized transactions.

It also switches _secure_write() to atomic persistence so state files are never left partially written if the process is interrupted mid-write.

Tests

Added regression coverage for:

  • concurrent rate-limit updates not losing entries
  • concurrent pairing code generation preserving all pending requests

Files changed

  • gateway/pairing.py
  • tests/gateway/test_pairing.py

Manual verification

I attempted to run:

python -m pytest tests/gateway/test_pairing.py -q

@teknium1

teknium1 commented Apr 6, 2026

Copy link
Copy Markdown
Collaborator

Not included in this batch — main-side commit e9b5864 already added atomic writes + threading.RLock to pairing.py, creating heavy conflicts. Your unique addition (cross-process fcntl/msvcrt locking) is still valuable as a follow-up. Leaving open.

@teknium1

teknium1 commented Apr 7, 2026

Copy link
Copy Markdown
Collaborator

Closing — main-side commit e9b5864 already added atomic writes (tempfile + os.replace) and threading.RLock protecting all read-modify-write operations in pairing.py. This handles the real-world concurrency case (multiple platform adapters in the same gateway process).

Your unique addition — cross-process file locking via fcntl/msvcrt — would protect against CLI and gateway writing simultaneously, but that's a very narrow race window, and the atomic writes already prevent corruption (worst case is last-writer-wins, which is acceptable for pairing operations). The complexity of platform-specific locking doesn't justify the marginal improvement.

Thanks for the thorough implementation @Dusk1e — the atomic write pattern and reentrant lock design were solid.

@teknium1 teknium1 closed this Apr 7, 2026
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.

2 participants