Repository navigation
feat(api): add shared source-author and distributed-lock helpers - #17086
AtefBellaaj wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0493a54880
ℹ️ 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".
0493a54 to
a26b9fa
Compare
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
a26b9fa to
74d3ca3
Compare
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
74d3ca3 to
1a38bf5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a38bf54fe
ℹ️ 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".
1a38bf5 to
4b6d8bb
Compare
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
4b6d8bb to
8033c78
Compare
Lift makeSourceAuthorId out of the GitHub skill sync into packages/api/src/utils/author.ts and add createDistributedLock in packages/api/src/cache/lock.ts (Redis SET NX PX with owner-checked Lua extend/release, in-process Map fallback without Redis). The skill sync call site uses the shared helper; author ids are byte-identical.
8033c78 to
b4d2bf0
Compare
Pull Request
Summary
The upcoming Langfuse prompt sync needs a deterministic author id for records owned by an external source and a cross-instance lock so two admins or an admin and a scheduler cannot run the same sync at once. Today the first exists only as a private function inside the GitHub skill sync (
packages/api/src/skills/sync/github.ts) and the second only as a feature-local Redis lock in the principals cache (packages/api/src/cache/principals.ts). This PR lifts both into shared@librechat/apihelpers:makeSourceAuthorId(provider, sourceId, tenantId?)inpackages/api/src/utils/author.tsandcreateDistributedLock({ redisClient?, now? })inpackages/api/src/cache/lock.ts. Behavior is unchanged: the skill sync now calls the shared helper and produces byte-identical author ids (the unchangedgithub.spec.tsproves it), and nothing switches to the new lock yet. The Langfuse prompt stack (starting at PR #16854) will retarget onto this branch.Related to #16854
How it works
Ownership:
makeSourceAuthorIdjoins its components with:and does not escape them, so it throws whenprovider,sourceId, or a providedtenantIdcontains:— otherwise two distinct inputs could collide on the same seed. For valid inputs the seed (provider:sourceIdorprovider:sourceId:tenantId) is unchanged from the ids the GitHub skill sync already stores, so the encoding cannot change without re-keying existing records.The Redis path uses
SET key token PX ttl NXto acquire, plus the two owner-checked scripts above (extend and release) run through the existingevalScripthelper (EVALSHA with NOSCRIPT fallback); without a Redis client, an in-processMapwith an injectable clock is used, which is complete rather than degraded because a LibreChat deployment without Redis is single-node.acquire(key, ttlMs)andextend(key, token, ttlMs)throwRangeErrorunlessttlMsis a positive integer.release(key, token)returnstruewhen the caller still owned the key andfalsewhen the lease had already expired or another holder owns it. The in-process backend removes an expired entry when its key is touched again and sweeps the whole table once it grows past a threshold that doubles after each sweep, soacquirestays O(1) per call and keys that are never touched again do not accumulate.The module lives in
cache/next toredisScript.ts, whoseevalScriptit reuses, andprincipals.ts, whose owner-token lock shape it generalizes. The GitHub skill sync underpackages/api/src/skills/syncdoes not use the Mongo lease inpackages/data-schemas/src/methods/skillSync.ts(tryAcquireSkillSyncLock); it needs a lock that coordinates admin-triggered and scheduled runs without aSkillSyncStatusdocument round trip. The repository already has owner-token Redis locks with the same Lua shape incache/principals.ts,agents/files.ts,cluster/LeaderElection.ts, andmcp/catalog/store.ts, and multi-replica LibreChat already requires shared Redis (.env.example, scheduled chats comment), so the in-process backend only needs to cover the single-process case.Type of change
Testing
Tested environments/configuration:
packages/apiJest with a mockedioredisclient; no browser or provider involved.Automated tests:
packages/api:src/utils/author.spec.ts,src/cache/lock.spec.ts,src/skills/sync/github.spec.ts(3 suites, all passing)npx tsc --noEmitinpackages/apiauthor.spec.ts(12 tests),lock.spec.ts(36 tests: 20 plain cases plus 4test.eachblocks of 4 ttl values each)Screenshots / recordings
No user-facing change.
Risk / compatibility
Low. The only runtime change is the
github.tscall site, with parity proven by the unchanged spec. No schema, index, or migration change; no new Mongo-only operators; Redis commands are single-key, so this is Redis Cluster safe. Safe to deploy and roll back in any order.Checklist
🤖 Generated with Claude Code