From 0cf3d299725bfc04575f3e443fa92895bf3a6c24 Mon Sep 17 00:00:00 2001 From: Barrett Ruth <62671086+barrettruth@users.noreply.github.com> Date: Fri, 6 Mar 2026 15:47:42 -0500 Subject: [PATCH] fix(oauth): resolve re-auth deadlock and improve flow robustness (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(oauth): resolve re-auth deadlock and improve flow robustness Problem: in-flight TCP server held port 18392 for up to 120 seconds. Calling `auth()` again caused `bind()` to fail silently — the browser opened but no listener could receive the OAuth callback. `_wipe()` on exchange failure also destroyed credentials, forcing full re-setup. Solution: `_active_close` at module scope cancels any in-flight server when `auth()` or `clear_tokens()` is called. Binding is guarded with `pcall`; the browser only opens after the server is listening. Swapped `_wipe()` for `clear_tokens()` in `_exchange_code` to preserve credentials on failure. Added `select_account` to `prompt` so Google always shows the account picker on re-auth. * test(oauth): isolate bundled-credentials fallback from real filesystem Problem: `resolve_credentials` reads from `vim.fn.stdpath('data')`, the real Neovim data dir. The test passed only because `_wipe()` was incidentally deleting the user's credential file mid-run. Solution: stub `oauth.load_json_file` for the duration of the test so real credential files cannot interfere with the fallback assertion. * ci: format --- lua/pending/sync/oauth.lua | 35 ++++++++++++++++++++++++++++------- spec/oauth_spec.lua | 5 +++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/lua/pending/sync/oauth.lua b/lua/pending/sync/oauth.lua index 8539ea6..fd6e88d 100644 --- a/lua/pending/sync/oauth.lua +++ b/lua/pending/sync/oauth.lua @@ -25,6 +25,8 @@ local BUNDLED_CLIENT_SECRET = 'PLACEHOLDER' local OAuthClient = {} OAuthClient.__index = OAuthClient +local _active_close = nil + ---@class pending.oauth local M = {} @@ -348,6 +350,11 @@ end ---@param on_complete? fun(): nil ---@return nil function OAuthClient:auth(on_complete) + if _active_close then + _active_close() + _active_close = nil + end + local creds = self:resolve_credentials() if creds.client_id == BUNDLED_CLIENT_ID then log.error(self.name .. ': no credentials configured — run :Pending auth') @@ -379,14 +386,11 @@ function OAuthClient:auth(on_complete) .. '&scope=' .. M.url_encode(self.scope) .. '&access_type=offline' - .. '&prompt=consent' + .. '&prompt=select_account%20consent' .. '&code_challenge=' .. M.url_encode(code_challenge) .. '&code_challenge_method=S256' - vim.ui.open(auth_url) - log.info('Opening browser for Google authorization...') - local server = vim.uv.new_tcp() local server_closed = false local function close_server() @@ -394,10 +398,20 @@ function OAuthClient:auth(on_complete) return end server_closed = true + if _active_close == close_server then + _active_close = nil + end server:close() end + _active_close = close_server + + local bind_ok, bind_err = pcall(server.bind, server, '127.0.0.1', port) + if not bind_ok or bind_err == nil then + close_server() + log.error(self.name .. ': port ' .. port .. ' already in use — try again in a moment') + return + end - server:bind('127.0.0.1', port) server:listen(1, function(err) if err then return @@ -430,6 +444,9 @@ function OAuthClient:auth(on_complete) end) end) + vim.ui.open(auth_url) + log.info('Opening browser for Google authorization...') + vim.defer_fn(function() if not server_closed then close_server() @@ -470,14 +487,14 @@ function OAuthClient:_exchange_code(creds, code, code_verifier, port, on_complet }, { text = true }) if result.code ~= 0 then - self:_wipe() + self:clear_tokens() log.error('Token exchange failed.') return end local ok, decoded = pcall(vim.json.decode, result.stdout or '') if not ok or not decoded.access_token then - self:_wipe() + self:clear_tokens() log.error('Invalid token response.') return end @@ -498,6 +515,10 @@ end ---@return nil function OAuthClient:clear_tokens() + if _active_close then + _active_close() + _active_close = nil + end os.remove(self:token_path()) end diff --git a/spec/oauth_spec.lua b/spec/oauth_spec.lua index 520227d..a4a6f1d 100644 --- a/spec/oauth_spec.lua +++ b/spec/oauth_spec.lua @@ -142,8 +142,13 @@ describe('oauth', function() it('falls back to bundled credentials', function() config.reset() vim.g.pending = { data_path = tmpdir .. '/tasks.json' } + local orig_load = oauth.load_json_file + oauth.load_json_file = function() + return nil + end local c = oauth.new({ name = 'gtasks', scope = 'x', port = 0, config_key = 'gtasks' }) local creds = c:resolve_credentials() + oauth.load_json_file = orig_load assert.equals(oauth._BUNDLED_CLIENT_ID, creds.client_id) assert.equals(oauth._BUNDLED_CLIENT_SECRET, creds.client_secret) end)