From ac18448ccc67e806229ce4a82c0bf217b55a678c Mon Sep 17 00:00:00 2001 From: sirmacik Date: Thu, 30 Apr 2026 18:13:09 +0200 Subject: [PATCH 1/3] Fix Codex ACP authentication --- agent-shell-openai.el | 31 +++++++++++++++++++++++-------- tests/agent-shell-openai-tests.el | 31 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/agent-shell-openai.el b/agent-shell-openai.el index a1c8efcc..3f48cd61 100644 --- a/agent-shell-openai.el +++ b/agent-shell-openai.el @@ -125,6 +125,27 @@ when starting a new Codex shell." :type '(choice (const nil) string) :group 'agent-shell) +(defun agent-shell-openai--codex-api-key-authenticate-request (api-key) + "Create a Codex `api-key' authentication request for API-KEY." + `((:method . "authenticate") + (:params . ((methodId . "api-key") + (_meta . (("api-key" . ((apiKey . ,api-key))))))))) + +(defun agent-shell-openai--codex-authenticate-request () + "Create the authenticate request for the current Codex auth config." + (cond ((map-elt agent-shell-openai-authentication :api-key) + (let ((api-key (agent-shell-openai-key))) + (unless api-key + (user-error "Please set your `agent-shell-openai-authentication'")) + (agent-shell-openai--codex-api-key-authenticate-request api-key))) + ((map-elt agent-shell-openai-authentication :codex-api-key) + (let ((codex-key (agent-shell-openai-key))) + (unless codex-key + (user-error "Please set your `agent-shell-openai-authentication'")) + (agent-shell-openai--codex-api-key-authenticate-request codex-key))) + (t + (acp-make-authenticate-request :method-id "chat-gpt")))) + (defun agent-shell-openai-make-codex-config () "Create a Codex agent configuration. @@ -144,16 +165,10 @@ Returns an agent configuration alist using `agent-shell-make-agent-config'." (funcall agent-shell-openai-default-model-id) agent-shell-openai-default-model-id)) :default-session-mode-id (lambda () agent-shell-openai-default-session-mode-id) - :authenticate-request-maker (lambda () - (cond ((map-elt agent-shell-openai-authentication :api-key) - (acp-make-authenticate-request :method-id "openai-api-key")) - ((map-elt agent-shell-openai-authentication :codex-api-key) - (acp-make-authenticate-request :method-id "codex-api-key")) - (t - (acp-make-authenticate-request :method-id "chatgpt")))) + :authenticate-request-maker #'agent-shell-openai--codex-authenticate-request :client-maker (lambda (buffer) (agent-shell-openai-make-codex-client :buffer buffer)) - :install-instructions "See https://github.com/zed-industries/codex-acp for installation.")) + :install-instructions "See https://github.com/agentclientprotocol/codex-acp for installation.")) (defun agent-shell-openai-start-codex () "Start an interactive Codex agent shell." diff --git a/tests/agent-shell-openai-tests.el b/tests/agent-shell-openai-tests.el index e96f8300..f3d3566a 100644 --- a/tests/agent-shell-openai-tests.el +++ b/tests/agent-shell-openai-tests.el @@ -31,5 +31,36 @@ (let ((agent-shell-openai-default-session-mode-id "full-access")) (should (string= (funcall default-session-mode-id-fn) "full-access"))))) +(ert-deftest agent-shell-openai-codex-login-auth-request-test () + "Test that Codex login auth uses the current chat-gpt method id." + (let* ((agent-shell-openai-authentication + (agent-shell-openai-make-authentication :login t)) + (request (funcall (map-elt (agent-shell-openai-make-codex-config) + :authenticate-request-maker)))) + (should (string= (map-nested-elt request '(:params methodId)) + "chat-gpt")))) + +(ert-deftest agent-shell-openai-codex-api-key-auth-request-test () + "Test that Codex API key auth sends key metadata." + (let* ((agent-shell-openai-authentication + (agent-shell-openai-make-authentication :api-key "openai-secret")) + (request (funcall (map-elt (agent-shell-openai-make-codex-config) + :authenticate-request-maker)))) + (should (string= (map-nested-elt request '(:params methodId)) + "api-key")) + (should (string= (map-nested-elt request '(:params _meta "api-key" apiKey)) + "openai-secret")))) + +(ert-deftest agent-shell-openai-codex-key-auth-request-test () + "Test that Codex-specific API key auth sends key metadata." + (let* ((agent-shell-openai-authentication + (agent-shell-openai-make-authentication :codex-api-key "codex-secret")) + (request (funcall (map-elt (agent-shell-openai-make-codex-config) + :authenticate-request-maker)))) + (should (string= (map-nested-elt request '(:params methodId)) + "api-key")) + (should (string= (map-nested-elt request '(:params _meta "api-key" apiKey)) + "codex-secret")))) + (provide 'agent-shell-openai-tests) ;;; agent-shell-openai-tests.el ends here From 5a43afb08def3adc2dd8500a57ab41dd3bebbbf5 Mon Sep 17 00:00:00 2001 From: sirmacik Date: Thu, 30 Apr 2026 20:03:14 +0200 Subject: [PATCH 2/3] Fix prompt loading message race --- agent-shell.el | 84 ++++++++++++++++++++++---------------- tests/agent-shell-tests.el | 42 +++++++++++++++++++ 2 files changed, 90 insertions(+), 36 deletions(-) diff --git a/agent-shell.el b/agent-shell.el index a53eb838..23a8b52b 100644 --- a/agent-shell.el +++ b/agent-shell.el @@ -2792,31 +2792,6 @@ variable (see makunbound)")) :config shell-maker--config :output (funcall (map-elt config :welcome-function) shell-maker--config))) - (if (eq agent-shell-session-strategy 'new-deferred) - ;; Show prompt now (unbootstrapped). - (shell-maker-finish-output - :config shell-maker--config - :success nil) - ;; Kick off ACP session bootstrapping. - (agent-shell--handle :shell-buffer shell-buffer)) - ;; State should be available after kicking off - ;; `agent-shell--handle'. Fire mode hook so initial - ;; state is available to agent-shell-mode-hook(s). - (run-hooks 'agent-shell-mode-hook) - ;; Refresh the session topic from the agent. `init-finished' fires - ;; once the session is established (covers resumed sessions whose - ;; title is already known) and `turn-complete' covers ongoing - ;; refinement for agents that summarize as the conversation grows. - ;; `session-selected' is too early -- it fires synchronously inside - ;; `agent-shell--handle' before this subscription can register. - (agent-shell-subscribe-to - :shell-buffer shell-buffer - :event 'init-finished - :on-event #'agent-shell--refresh-topic-from-session-list) - (agent-shell-subscribe-to - :shell-buffer shell-buffer - :event 'turn-complete - :on-event #'agent-shell--refresh-topic-from-session-list) ;; Subscribe to session selection events (needed regardless of focus). (when (eq agent-shell-session-strategy 'prompt) (agent-shell-subscribe-to @@ -2843,26 +2818,63 @@ variable (see makunbound)")) (agent-shell-subscribe-to :shell-buffer shell-buffer :event 'error + :on-event (lambda (_event) + (agent-shell-active-message-hide :active-message active-message))) + (agent-shell-subscribe-to + :shell-buffer shell-buffer + :event 'init-finished + :on-event (lambda (_event) + (agent-shell-active-message-hide :active-message active-message))) + (agent-shell-subscribe-to + :shell-buffer shell-buffer + :event 'prompt-ready :on-event (lambda (_event) (agent-shell-active-message-hide :active-message active-message))) (agent-shell-subscribe-to :shell-buffer shell-buffer :event 'clean-up :on-event (lambda (_event) - (agent-shell-active-message-hide :active-message active-message))))) + (agent-shell-active-message-hide :active-message active-message)))) + (unless no-focus + ;; Defer display until user selects a session. + ;; Why? The experience is janky to display a buffer + ;; and soon after that prompt the user for input. + ;; Better to prompt the user for input and then + ;; display the buffer. + (agent-shell-subscribe-to + :shell-buffer shell-buffer + :event 'session-selected + :on-event (lambda (_event) + (agent-shell--display-buffer shell-buffer))))) + (if (eq agent-shell-session-strategy 'new-deferred) + ;; Show prompt now (unbootstrapped). + (shell-maker-finish-output + :config shell-maker--config + :success nil) + ;; Kick off ACP session bootstrapping. + (agent-shell--handle :shell-buffer shell-buffer)) + ;; State should be available after kicking off + ;; `agent-shell--handle'. Fire mode hook so initial + ;; state is available to agent-shell-mode-hook(s). + (run-hooks 'agent-shell-mode-hook) + ;; Refresh the session topic from the agent. `init-finished' fires + ;; once the session is established (covers resumed sessions whose + ;; title is already known) and `turn-complete' covers ongoing + ;; refinement for agents that summarize as the conversation grows. + ;; `session-selected' is too early -- it fires synchronously inside + ;; `agent-shell--handle' before this subscription can register. + (agent-shell-subscribe-to + :shell-buffer shell-buffer + :event 'init-finished + :on-event #'agent-shell--refresh-topic-from-session-list) + (agent-shell-subscribe-to + :shell-buffer shell-buffer + :event 'turn-complete + :on-event #'agent-shell--refresh-topic-from-session-list) ;; Display buffer if no-focus was nil, respecting agent-shell-display-action (unless no-focus (if (eq agent-shell-session-strategy 'prompt) - ;; Defer display until user selects a session. - ;; Why? The experience is janky to display a buffer - ;; and soon after that prompt the user for input. - ;; Better to prompt the user for input and then - ;; display the buffer. - (agent-shell-subscribe-to - :shell-buffer shell-buffer - :event 'session-selected - :on-event (lambda (_event) - (agent-shell--display-buffer shell-buffer))) + nil (agent-shell--display-buffer shell-buffer)))) shell-buffer)) diff --git a/tests/agent-shell-tests.el b/tests/agent-shell-tests.el index c11d1b92..26b538a4 100644 --- a/tests/agent-shell-tests.el +++ b/tests/agent-shell-tests.el @@ -1376,6 +1376,48 @@ code block content (when (and test-buffer (buffer-live-p test-buffer)) (kill-buffer test-buffer))))) +(ert-deftest agent-shell-prompt-active-message-hidden-after-early-session-selection () + "Session selection during startup should hide prompt loading message." + (let ((test-buffer nil) + (active-message-hidden nil) + (displayed-buffer nil) + (fake-process (start-process "fake-agent" nil "cat")) + (agent-shell-session-strategy 'prompt) + (agent-shell-show-welcome-message nil) + (agent-shell-file-completion-enabled nil) + (config (list (cons :buffer-name "test-agent") + (cons :mode-line-name "Test Agent") + (cons :client-maker + (lambda (_buf) + (list (cons :command "cat"))))))) + (unwind-protect + (cl-letf (((symbol-function 'shell-maker-start) + (lambda (_config &rest _args) + (setq test-buffer (get-buffer-create "*test-agent-shell*")) + (with-current-buffer test-buffer + (setq major-mode 'agent-shell-mode)) + test-buffer)) + ((symbol-function 'shell-maker--process) (lambda () fake-process)) + ((symbol-function 'shell-maker-write-output) #'ignore) + ((symbol-function 'shell-maker-finish-output) #'ignore) + ((symbol-function 'agent-shell-active-message-show) + (lambda (&rest _args) '((:active . t)))) + ((symbol-function 'agent-shell-active-message-hide) + (lambda (&rest _args) (setq active-message-hidden t))) + ((symbol-function 'agent-shell--display-buffer) + (lambda (buffer) (setq displayed-buffer buffer))) + ((symbol-function 'agent-shell--handle) + (lambda (&rest _args) + (with-current-buffer test-buffer + (agent-shell--emit-event :event 'session-selected))))) + (agent-shell--start :config config) + (should active-message-hidden) + (should (eq displayed-buffer test-buffer))) + (when (process-live-p fake-process) + (delete-process fake-process)) + (when (and test-buffer (buffer-live-p test-buffer)) + (kill-buffer test-buffer))))) + (ert-deftest agent-shell--initiate-session-prefers-list-and-load-when-supported () "Test `agent-shell--initiate-session' prefers session/list + session/load." (with-temp-buffer From 3f2cda8a77204cde17a322a59c2b2ba760771349 Mon Sep 17 00:00:00 2001 From: sirmacik Date: Thu, 30 Apr 2026 20:57:07 +0200 Subject: [PATCH 3/3] Let Codex ACP reuse existing auth --- agent-shell-openai.el | 41 +++++++++++++++++-------------- tests/agent-shell-openai-tests.el | 40 +++++++++++++++++------------- 2 files changed, 46 insertions(+), 35 deletions(-) diff --git a/agent-shell-openai.el b/agent-shell-openai.el index 3f48cd61..98c44638 100644 --- a/agent-shell-openai.el +++ b/agent-shell-openai.el @@ -27,6 +27,7 @@ (eval-when-compile (require 'cl-lib)) +(require 'json) (require 'shell-maker) (require 'acp) @@ -125,26 +126,28 @@ when starting a new Codex shell." :type '(choice (const nil) string) :group 'agent-shell) -(defun agent-shell-openai--codex-api-key-authenticate-request (api-key) - "Create a Codex `api-key' authentication request for API-KEY." - `((:method . "authenticate") - (:params . ((methodId . "api-key") - (_meta . (("api-key" . ((apiKey . ,api-key))))))))) - -(defun agent-shell-openai--codex-authenticate-request () - "Create the authenticate request for the current Codex auth config." +(defun agent-shell-openai--codex-default-auth-request () + "Create the Codex default auth request for the current auth config." (cond ((map-elt agent-shell-openai-authentication :api-key) (let ((api-key (agent-shell-openai-key))) (unless api-key (user-error "Please set your `agent-shell-openai-authentication'")) - (agent-shell-openai--codex-api-key-authenticate-request api-key))) + `((methodId . "api-key") + (_meta . ((api-key . ((apiKey . ,api-key)))))))) ((map-elt agent-shell-openai-authentication :codex-api-key) (let ((codex-key (agent-shell-openai-key))) (unless codex-key (user-error "Please set your `agent-shell-openai-authentication'")) - (agent-shell-openai--codex-api-key-authenticate-request codex-key))) + `((methodId . "api-key") + (_meta . ((api-key . ((apiKey . ,codex-key)))))))) (t - (acp-make-authenticate-request :method-id "chat-gpt")))) + '((methodId . "chat-gpt"))))) + +(defun agent-shell-openai--codex-default-auth-environment () + "Return DEFAULT_AUTH_REQUEST environment for Codex ACP." + (list (concat "DEFAULT_AUTH_REQUEST=" + (json-serialize + (agent-shell-openai--codex-default-auth-request))))) (defun agent-shell-openai-make-codex-config () "Create a Codex agent configuration. @@ -160,12 +163,11 @@ Returns an agent configuration alist using `agent-shell-make-agent-config'." :shell-prompt-regexp "Codex> " :welcome-function #'agent-shell-openai--codex-welcome-message :icon-name "openai.png" - :needs-authentication t + :needs-authentication nil :default-model-id (lambda () (if (functionp agent-shell-openai-default-model-id) (funcall agent-shell-openai-default-model-id) agent-shell-openai-default-model-id)) :default-session-mode-id (lambda () agent-shell-openai-default-session-mode-id) - :authenticate-request-maker #'agent-shell-openai--codex-authenticate-request :client-maker (lambda (buffer) (agent-shell-openai-make-codex-client :buffer buffer)) :install-instructions "See https://github.com/agentclientprotocol/codex-acp for installation.")) @@ -192,6 +194,7 @@ Uses `agent-shell-openai-authentication' for authentication configuration." (agent-shell--make-acp-client :command (car agent-shell-openai-codex-acp-command) :command-params (cdr agent-shell-openai-codex-acp-command) :environment-variables (append (list (format "OPENAI_API_KEY=%s" api-key)) + (agent-shell-openai--codex-default-auth-environment) agent-shell-openai-codex-environment) :context-buffer buffer))) ((map-elt agent-shell-openai-authentication :codex-api-key) @@ -201,14 +204,16 @@ Uses `agent-shell-openai-authentication' for authentication configuration." (agent-shell--make-acp-client :command (car agent-shell-openai-codex-acp-command) :command-params (cdr agent-shell-openai-codex-acp-command) :environment-variables (append (list (format "CODEX_API_KEY=%s" codex-key)) + (agent-shell-openai--codex-default-auth-environment) agent-shell-openai-codex-environment) :context-buffer buffer))) ((map-elt agent-shell-openai-authentication :login) - (agent-shell--make-acp-client :command (car agent-shell-openai-codex-acp-command) - :command-params (cdr agent-shell-openai-codex-acp-command) - :environment-variables (append '("OPENAI_API_KEY=") - agent-shell-openai-codex-environment) - :context-buffer buffer)) + (agent-shell--make-acp-client :command (car agent-shell-openai-codex-acp-command) + :command-params (cdr agent-shell-openai-codex-acp-command) + :environment-variables (append '("OPENAI_API_KEY=") + (agent-shell-openai--codex-default-auth-environment) + agent-shell-openai-codex-environment) + :context-buffer buffer)) (t (error "Invalid authentication configuration")))) diff --git a/tests/agent-shell-openai-tests.el b/tests/agent-shell-openai-tests.el index f3d3566a..74370058 100644 --- a/tests/agent-shell-openai-tests.el +++ b/tests/agent-shell-openai-tests.el @@ -31,35 +31,41 @@ (let ((agent-shell-openai-default-session-mode-id "full-access")) (should (string= (funcall default-session-mode-id-fn) "full-access"))))) -(ert-deftest agent-shell-openai-codex-login-auth-request-test () +(ert-deftest agent-shell-openai-codex-does-not-eagerly-authenticate-test () + "Test that Codex lets codex-acp decide when auth is needed." + (let ((config (agent-shell-openai-make-codex-config))) + (should-not (map-elt config :needs-authentication)) + (should-not (map-elt config :authenticate-request-maker)))) + +(ert-deftest agent-shell-openai-codex-login-default-auth-request-test () "Test that Codex login auth uses the current chat-gpt method id." (let* ((agent-shell-openai-authentication (agent-shell-openai-make-authentication :login t)) - (request (funcall (map-elt (agent-shell-openai-make-codex-config) - :authenticate-request-maker)))) - (should (string= (map-nested-elt request '(:params methodId)) - "chat-gpt")))) + (env (car (agent-shell-openai--codex-default-auth-environment))) + (request (json-parse-string (string-remove-prefix "DEFAULT_AUTH_REQUEST=" env) + :object-type 'alist))) + (should (string= (map-elt request 'methodId) "chat-gpt")))) -(ert-deftest agent-shell-openai-codex-api-key-auth-request-test () +(ert-deftest agent-shell-openai-codex-api-key-default-auth-request-test () "Test that Codex API key auth sends key metadata." (let* ((agent-shell-openai-authentication (agent-shell-openai-make-authentication :api-key "openai-secret")) - (request (funcall (map-elt (agent-shell-openai-make-codex-config) - :authenticate-request-maker)))) - (should (string= (map-nested-elt request '(:params methodId)) - "api-key")) - (should (string= (map-nested-elt request '(:params _meta "api-key" apiKey)) + (env (car (agent-shell-openai--codex-default-auth-environment))) + (request (json-parse-string (string-remove-prefix "DEFAULT_AUTH_REQUEST=" env) + :object-type 'alist))) + (should (string= (map-elt request 'methodId) "api-key")) + (should (string= (map-nested-elt request '(_meta api-key apiKey)) "openai-secret")))) -(ert-deftest agent-shell-openai-codex-key-auth-request-test () +(ert-deftest agent-shell-openai-codex-key-default-auth-request-test () "Test that Codex-specific API key auth sends key metadata." (let* ((agent-shell-openai-authentication (agent-shell-openai-make-authentication :codex-api-key "codex-secret")) - (request (funcall (map-elt (agent-shell-openai-make-codex-config) - :authenticate-request-maker)))) - (should (string= (map-nested-elt request '(:params methodId)) - "api-key")) - (should (string= (map-nested-elt request '(:params _meta "api-key" apiKey)) + (env (car (agent-shell-openai--codex-default-auth-environment))) + (request (json-parse-string (string-remove-prefix "DEFAULT_AUTH_REQUEST=" env) + :object-type 'alist))) + (should (string= (map-elt request 'methodId) "api-key")) + (should (string= (map-nested-elt request '(_meta api-key apiKey)) "codex-secret")))) (provide 'agent-shell-openai-tests)