Skip to content

Commit 63f1c4e

Browse files
ericdalloeca-agent
andcommitted
Lock OAuth refresh across concurrent ECA processes (#462)
Multiple eca server processes sharing ~/.cache/eca/db.transit.json all read the same refresh token at startup, then each independently POSTs to the provider when the access token nears expiry. OAuth refresh tokens are single-use, so the first request wins, the others receive invalid_grant, and the losers' in-memory state never resyncs from disk. Wrap renew-auth! in a cross-process advisory lock (JVM mutex plus FileChannel.lock on a sidecar of the global cache file). Inside the lock, re-read the :auth slice from disk via sync-auth-from-cache! and only call login-step :login/renew-token when the freshly-loaded expires-at is still in the past. Losers now silently adopt the winner's rotated tokens instead of POSTing with a stale refresh token. Generic across all providers that route through the shared renew-auth! entry point (Anthropic Max/Console, OpenAI ChatGPT, Copilot). MCP token refresh is unchanged. 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca-agent <git@eca.dev>
1 parent 165b3e2 commit 63f1c4e

5 files changed

Lines changed: 257 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## Unreleased
44

5+
- Fix `invalid_grant` errors when multiple ECA processes race to refresh the same OAuth token. #462
6+
57
## 0.135.0
68

79
- Persist chats more durably: save after every assistant segment / tool output / error / rollback, and write the cache via atomic tmp+rename so long chats no longer get lost on crash or restart.

src/eca/db.clj

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
[eca.metrics :as metrics]
99
[eca.shared :as shared])
1010
(:import
11-
[java.io OutputStream]
11+
[java.io OutputStream RandomAccessFile]
12+
[java.nio.channels FileChannel FileLock]
1213
[java.nio.file AtomicMoveNotSupportedException CopyOption Files StandardCopyOption]))
1314

1415
(set! *warn-on-reflection* true)
@@ -262,6 +263,63 @@
262263
(assoc :version version)
263264
(upsert-cache! (transit-global-db-file) metrics)))
264265

266+
(def ^:private global-cache-lock-sentinel (Object.))
267+
268+
(defn ^:private global-cache-lock-file []
269+
(io/file (cache/global-dir) "db.transit.json.lock"))
270+
271+
(defn with-global-cache-lock-fn
272+
"Run `f` while holding both a JVM-wide mutex and an OS advisory exclusive
273+
lock on a sidecar of the global cache file. The JVM mutex avoids
274+
`OverlappingFileLockException` when two threads in the same ECA server
275+
race a renew; the file lock serializes across `eca server` processes
276+
that share `~/.cache/eca/`. Blocks until both are acquired."
277+
[f]
278+
(locking global-cache-lock-sentinel
279+
(let [^java.io.File lock-file (global-cache-lock-file)
280+
_ (io/make-parents lock-file)
281+
^RandomAccessFile raf (RandomAccessFile. lock-file "rw")
282+
^FileChannel channel (.getChannel raf)
283+
lock-ref (volatile! nil)]
284+
(try
285+
(vreset! lock-ref ^FileLock (.lock channel))
286+
(f)
287+
(finally
288+
(when-let [^FileLock lock @lock-ref]
289+
(try (.release lock)
290+
(catch Throwable e
291+
(logger/warn logger-tag "Could not release global cache lock" e))))
292+
(try (.close channel) (catch Throwable _))
293+
(try (.close raf) (catch Throwable _)))))))
294+
295+
(defmacro with-global-cache-lock
296+
"See `with-global-cache-lock-fn`. Runs `body` while holding the lock."
297+
[& body]
298+
`(with-global-cache-lock-fn (fn [] ~@body)))
299+
300+
(defn sync-auth-from-cache!
301+
"Re-read the global cache from disk and, if its `:auth` entry for `provider`
302+
has a different `:expires-at` than the in-memory copy, overwrite the
303+
in-memory `[:auth provider]` with the disk version. This lets a process
304+
that lost a token-refresh race adopt the winner's freshly rotated tokens
305+
instead of POSTing with a stale refresh token.
306+
307+
Returns a truthy value when in-memory state was updated."
308+
[db* provider metrics]
309+
(try
310+
(when-let [disk-auth (some-> (read-global-cache metrics) :auth (get provider))]
311+
(let [in-mem-auth (get-in @db* [:auth provider])]
312+
(when (and (:expires-at disk-auth)
313+
(not= (:expires-at disk-auth) (:expires-at in-mem-auth)))
314+
(logger/info logger-tag
315+
(format "Adopting %s auth tokens refreshed by peer process (expires-at %s)"
316+
provider (:expires-at disk-auth)))
317+
(swap! db* assoc-in [:auth provider] disk-auth)
318+
true)))
319+
(catch Throwable e
320+
(logger/warn logger-tag "Could not sync auth from cache" e)
321+
false)))
322+
265323
(defn cleanup-old-chats!
266324
"Deletes chats older than retention-days from the db and flushes the workspace cache.
267325
When retention-days is non-positive, cleanup is disabled."

src/eca/features/login.clj

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33
[clojure.string :as string]
44
[eca.config :as config]
55
[eca.db :as db]
6+
[eca.logger :as logger]
67
[eca.messenger :as messenger]
78
[eca.models :as models]
89
[eca.shared :refer [multi-str]]))
910

11+
(def ^:private logger-tag "[LOGIN]")
12+
1013
(defmulti login-step (fn [ctx] [(:provider ctx) (:step ctx)]))
1114

1215
(defmethod login-step :default [{:keys [send-msg!]}]
@@ -78,14 +81,27 @@
7881
{:keys [db* messenger config metrics]}
7982
{:keys [on-error]}]
8083
(try
81-
(login-step
82-
{:provider provider
83-
:metrics metrics
84-
:messenger messenger
85-
:config config
86-
:step :login/renew-token
87-
:db* db*})
88-
(db/update-global-cache! @db* metrics)
84+
;; Serialize across ECA processes that share `~/.cache/eca/db.transit.json`.
85+
;; OAuth refresh tokens are single-use, so two concurrent processes
86+
;; both POSTing with the same token would have one win and the other
87+
;; receive `invalid_grant`. Holding the cache lock + re-reading disk
88+
;; lets the loser adopt the winner's rotated tokens instead. #462
89+
(db/with-global-cache-lock
90+
(db/sync-auth-from-cache! db* provider metrics)
91+
(let [expires-at (get-in @db* [:auth provider :expires-at])
92+
now-plus-60 (+ 60 (quot (System/currentTimeMillis) 1000))]
93+
(if (and expires-at (> (long expires-at) now-plus-60))
94+
(logger/info logger-tag
95+
(format "Skipping %s renew; peer process already refreshed." provider))
96+
(do
97+
(login-step
98+
{:provider provider
99+
:metrics metrics
100+
:messenger messenger
101+
:config config
102+
:step :login/renew-token
103+
:db* db*})
104+
(db/update-global-cache! @db* metrics)))))
89105
(catch Exception e
90106
(on-error (.getMessage e)))))
91107

test/eca/db_test.clj

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
[clojure.java.io :as io]
55
[clojure.test :refer [deftest is testing]]
66
[cognitect.transit :as transit]
7+
[eca.cache :as cache]
78
[eca.db :as db])
89
(:import
910
[java.io File]))
@@ -97,6 +98,94 @@
9798
(is (= db/version (:version (read-transit-file cache-file))))
9899
(finally (fs/delete-tree tmpdir))))))
99100

101+
(deftest sync-auth-from-cache!-adopts-fresher-disk-tokens-test
102+
(testing "when on-disk :auth has a different :expires-at, in-memory state is overwritten"
103+
(let [tmpdir (str (fs/create-temp-dir))]
104+
(with-redefs [cache/global-dir (constantly (io/file tmpdir))]
105+
(try
106+
(let [fresh-auth {:type :auth/oauth
107+
:mode :max
108+
:step :login/done
109+
:refresh-token "fresh-refresh"
110+
:api-key "fresh-access"
111+
:expires-at 9999999999}
112+
disk-db (atom {:auth {"anthropic" fresh-auth}})
113+
_ (db/update-global-cache! @disk-db nil)
114+
stale-mem (atom {:auth {"anthropic" {:type :auth/oauth
115+
:mode :max
116+
:step :login/done
117+
:refresh-token "stale-refresh"
118+
:api-key "stale-access"
119+
:expires-at 1000}}})
120+
updated? (db/sync-auth-from-cache! stale-mem "anthropic" nil)]
121+
(is updated?)
122+
(is (= "fresh-refresh" (get-in @stale-mem [:auth "anthropic" :refresh-token])))
123+
(is (= "fresh-access" (get-in @stale-mem [:auth "anthropic" :api-key])))
124+
(is (= 9999999999 (get-in @stale-mem [:auth "anthropic" :expires-at]))))
125+
(finally (fs/delete-tree tmpdir)))))))
126+
127+
(deftest sync-auth-from-cache!-noop-when-disk-matches-memory-test
128+
(testing "when on-disk :expires-at matches memory, no update happens"
129+
(let [tmpdir (str (fs/create-temp-dir))]
130+
(with-redefs [cache/global-dir (constantly (io/file tmpdir))]
131+
(try
132+
(let [auth {:type :auth/oauth
133+
:refresh-token "same"
134+
:api-key "same"
135+
:expires-at 7777}
136+
disk-db (atom {:auth {"anthropic" auth}})
137+
_ (db/update-global-cache! @disk-db nil)
138+
mem (atom {:auth {"anthropic" auth}})
139+
updated? (db/sync-auth-from-cache! mem "anthropic" nil)]
140+
(is (not updated?))
141+
(is (= auth (get-in @mem [:auth "anthropic"]))))
142+
(finally (fs/delete-tree tmpdir)))))))
143+
144+
(deftest sync-auth-from-cache!-noop-when-no-disk-cache-test
145+
(testing "when no global cache file exists, returns falsy and leaves memory untouched"
146+
(let [tmpdir (str (fs/create-temp-dir))]
147+
(with-redefs [cache/global-dir (constantly (io/file tmpdir))]
148+
(try
149+
(let [mem (atom {:auth {"anthropic" {:refresh-token "x" :expires-at 1}}})
150+
updated? (db/sync-auth-from-cache! mem "anthropic" nil)]
151+
(is (not updated?))
152+
(is (= "x" (get-in @mem [:auth "anthropic" :refresh-token]))))
153+
(finally (fs/delete-tree tmpdir)))))))
154+
155+
(deftest with-global-cache-lock-runs-body-and-releases-test
156+
(testing "the lock can be acquired sequentially without leaking handles"
157+
(let [tmpdir (str (fs/create-temp-dir))]
158+
(with-redefs [cache/global-dir (constantly (io/file tmpdir))]
159+
(try
160+
(let [ran (atom 0)]
161+
(db/with-global-cache-lock (swap! ran inc))
162+
(db/with-global-cache-lock (swap! ran inc))
163+
(is (= 2 @ran)))
164+
(finally (fs/delete-tree tmpdir)))))))
165+
166+
(deftest with-global-cache-lock-serializes-concurrent-threads-test
167+
(testing "two threads acquiring the lock cannot interleave inside the body"
168+
(let [tmpdir (str (fs/create-temp-dir))]
169+
(with-redefs [cache/global-dir (constantly (io/file tmpdir))]
170+
(try
171+
(let [inside (atom 0)
172+
max-inside (atom 0)
173+
iterations 50
174+
worker (fn []
175+
(dotimes [_ iterations]
176+
(db/with-global-cache-lock
177+
(let [v (swap! inside inc)]
178+
(swap! max-inside max v)
179+
;; small spin so a non-locking impl would observe overlap
180+
(Thread/sleep 1)
181+
(swap! inside dec)))))
182+
f1 (future (worker))
183+
f2 (future (worker))]
184+
@f1 @f2
185+
(is (= 1 @max-inside)
186+
"no two threads should ever be inside the locked body at the same time"))
187+
(finally (fs/delete-tree tmpdir)))))))
188+
100189
(deftest normalize-preserves-empty-message-chats-test
101190
(let [normalize @#'db/normalize-db-for-workspace-write
102191
result (normalize {:chats {"empty" {:id "empty" :messages []}

test/eca/features/login_test.clj

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
(ns eca.features.login-test
22
(:require
3+
[babashka.fs :as fs]
4+
[clojure.java.io :as io]
35
[clojure.test :refer [deftest is testing]]
6+
[eca.cache :as cache]
7+
[eca.db :as db]
48
[eca.features.login :as login]
59
[hato.client :as http]
610
[matcher-combinators.test :refer [match?]]))
@@ -67,3 +71,82 @@
6771
1 {}}}
6872

6973
@db*)))))))
74+
75+
(def ^:private renew-auth!* @#'login/renew-auth!)
76+
77+
(deftest renew-auth!-skips-refresh-when-peer-already-refreshed-test
78+
(testing "if disk holds fresher tokens than memory, renew-auth! adopts them and does NOT call login-step"
79+
(let [tmpdir (str (fs/create-temp-dir))]
80+
(with-redefs [cache/global-dir (constantly (io/file tmpdir))]
81+
(try
82+
(let [fresh {:type :auth/oauth :mode :max :step :login/done
83+
:refresh-token "peer-fresh" :api-key "peer-access"
84+
:expires-at 9999999999}
85+
stale {:type :auth/oauth :mode :max :step :login/done
86+
:refresh-token "mine-stale" :api-key "mine-access"
87+
:expires-at 1000}
88+
_ (db/update-global-cache! {:auth {"anthropic" fresh}} nil)
89+
db* (atom {:auth {"anthropic" stale}})
90+
step-calls (atom 0)
91+
on-error-msgs (atom [])]
92+
(with-redefs [login/login-step (fn [_ctx] (swap! step-calls inc))]
93+
(renew-auth!* "anthropic"
94+
{:db* db* :messenger nil :config nil :metrics nil}
95+
{:on-error #(swap! on-error-msgs conj %)}))
96+
(is (zero? @step-calls)
97+
"login-step should not be invoked when disk has fresh tokens")
98+
(is (empty? @on-error-msgs))
99+
(is (= "peer-fresh" (get-in @db* [:auth "anthropic" :refresh-token])))
100+
(is (= "peer-access" (get-in @db* [:auth "anthropic" :api-key])))
101+
(is (= 9999999999 (get-in @db* [:auth "anthropic" :expires-at]))))
102+
(finally (fs/delete-tree tmpdir)))))))
103+
104+
(deftest renew-auth!-refreshes-when-disk-also-stale-test
105+
(testing "when memory and disk are both expired, renew-auth! invokes login-step exactly once and persists"
106+
(let [tmpdir (str (fs/create-temp-dir))]
107+
(with-redefs [cache/global-dir (constantly (io/file tmpdir))]
108+
(try
109+
(let [stale {:type :auth/oauth :mode :max :step :login/done
110+
:refresh-token "stale" :api-key "stale-access"
111+
:expires-at 1000}
112+
_ (db/update-global-cache! {:auth {"anthropic" stale}} nil)
113+
db* (atom {:auth {"anthropic" stale}})
114+
step-calls (atom 0)
115+
on-error-msgs (atom [])]
116+
(with-redefs [login/login-step
117+
(fn [{:keys [db* provider]}]
118+
(swap! step-calls inc)
119+
(swap! db* update-in [:auth provider] merge
120+
{:refresh-token "rotated"
121+
:api-key "rotated-access"
122+
:expires-at 9999999999}))]
123+
(renew-auth!* "anthropic"
124+
{:db* db* :messenger nil :config nil :metrics nil}
125+
{:on-error #(swap! on-error-msgs conj %)}))
126+
(is (= 1 @step-calls))
127+
(is (empty? @on-error-msgs))
128+
(is (= "rotated" (get-in @db* [:auth "anthropic" :refresh-token])))
129+
;; And the rotated tokens should have been written to disk so the next
130+
;; process in the race sees them.
131+
(let [reloaded (atom {:auth {"anthropic" {}}})]
132+
(db/sync-auth-from-cache! reloaded "anthropic" nil)
133+
(is (= "rotated" (get-in @reloaded [:auth "anthropic" :refresh-token])))))
134+
(finally (fs/delete-tree tmpdir)))))))
135+
136+
(deftest renew-auth!-calls-on-error-when-login-step-throws-test
137+
(testing "exceptions from the provider refresh propagate to on-error and do not persist"
138+
(let [tmpdir (str (fs/create-temp-dir))]
139+
(with-redefs [cache/global-dir (constantly (io/file tmpdir))]
140+
(try
141+
(let [stale {:type :auth/oauth :refresh-token "stale" :expires-at 1000}
142+
_ (db/update-global-cache! {:auth {"anthropic" stale}} nil)
143+
db* (atom {:auth {"anthropic" stale}})
144+
on-error-msgs (atom [])]
145+
(with-redefs [login/login-step
146+
(fn [_ctx] (throw (ex-info "Anthropic refresh token failed" {})))]
147+
(renew-auth!* "anthropic"
148+
{:db* db* :messenger nil :config nil :metrics nil}
149+
{:on-error #(swap! on-error-msgs conj %)}))
150+
(is (= 1 (count @on-error-msgs)))
151+
(is (= "Anthropic refresh token failed" (first @on-error-msgs))))
152+
(finally (fs/delete-tree tmpdir)))))))

0 commit comments

Comments
 (0)