fix: follow up on the crete-session hook PR (#206)

Author: carlos-algms

.editorconfig

@@ -13,9 +13,14 @@ end_of_line = lf
 [*.md]
 trim_trailing_whitespace = false
 
+# insert_final_newline is safe for file types prettier does not format
 [*.lua]
 indent_size = 4
 insert_final_newline = true
 
+[*.txt]
+insert_final_newline = true
+
 [Makefile]
-indent_style = tab
+indent_style = tab
+insert_final_newline = true

README.md

@@ -839,16 +839,25 @@ integrating with other plugins.
   --- @type agentic.PartialUserConfig
   opts = {
     hooks = {
-      -- Called when a new ACP session is created (or fails to create)
+      -- Called when a new ACP session is created (or fails to create).
+      -- Fires on both success and failure; check `data.err` first.
       on_create_session_response = function(data)
         -- data.session_id: string|nil - The ACP session ID (nil if err is set)
         -- data.tab_page_id: number - The Neovim tabpage ID
-        -- data.response: table|nil - The ACP session creation response (nil if err is set)
+        -- data.response: table|nil - The ACP session creation response
+        --   (nil if err is set)
         -- data.err: table|nil - Error details if session creation failed
-        if data.response then
-          vim.notify("New session: " .. data.response.sessionId)
+        if data.err then
+          vim.notify(
+            "Session failed: " .. vim.inspect(data.err),
+            vim.log.levels.ERROR
+          )
+          return
         end
+        vim.notify("New session: " .. data.response.sessionId)
 
+        -- Reset the agentic_usage tabpage var (set by the on_session_update
+        -- example below) so a new session starts with a clean usage counter.
         if vim.api.nvim_tabpage_is_valid(data.tab_page_id) then
           vim.t[data.tab_page_id].agentic_usage = nil
         end

doc/agentic.txt

@@ -602,10 +602,12 @@ Event hooks ~
   opts = {
     hooks = {
       on_create_session_response = function(data)
-        -- data.response: table|nil - The ACP session creation response (nil if err is set)
-        -- data.err: table|nil - Error details if session creation failed
-        -- data.session_id: string
-        -- data.tab_page_id: number
+        -- Fires on both success and failure. Check `data.err` first.
+        -- data.session_id: string|nil (nil if err is set)
+        -- data.tab_page_id: number (may be invalid if tab was closed;
+        --   guard with vim.api.nvim_tabpage_is_valid before use)
+        -- data.response: table|nil (nil if err is set)
+        -- data.err: table|nil (nil on success)
       end,
       on_prompt_submit = function(data)
         -- data.prompt: string
@@ -940,4 +942,4 @@ Persistent ACP message log:
   `~/.cache/nvim/agentic_debug.log`
 
 ==============================================================================
-vim:tw=78:ts=8:ft=help:norl:
+vim:tw=78:ts=8:ft=help:norl:

lua/agentic/config_default.lua

@@ -25,10 +25,10 @@
 
 --- Data passed to the on_create_session_response hook
 --- @class agentic.UserConfig.CreateSessionResponseData
---- @field session_id string|nil
---- @field tab_page_id number
---- @field response agentic.acp.SessionCreationResponse|nil
---- @field err? agentic.acp.ACPError
+--- @field session_id? string Convenience field; equals response.sessionId when response is non-nil, nil if creation failed
+--- @field tab_page_id number The tabpage ID for this session
+--- @field response? agentic.acp.SessionCreationResponse Raw ACP create-session response, nil on error
+--- @field err? agentic.acp.ACPError Error details if session creation failed
 
 --- Data passed to the on_prompt_submit hook
 --- @class agentic.UserConfig.PromptSubmitData

lua/agentic/session_manager.test.lua

@@ -1238,4 +1238,128 @@ describe("agentic.SessionManager", function()
             assert.spy(write_message_spy).was.called(0)
         end)
     end)
+
+    describe("new_session: on_create_session_response hook", function()
+        local Config = require("agentic.config")
+        --- @type TestStub
+        local schedule_stub
+
+        before_each(function()
+            schedule_stub = spy.stub(vim, "schedule")
+            schedule_stub:invokes(function(fn)
+                fn()
+            end)
+        end)
+
+        --- Build a session mock with just enough surface for the error path.
+        --- new_session calls self:_cancel_session and self:_build_handlers
+        --- before agent:create_session. Both are stubbed to no-ops so the
+        --- test focuses on the hook invocation. The error path returns
+        --- immediately after the hook, so success-path collaborators are
+        --- not needed.
+        --- @return agentic.SessionManager
+        local function make_session()
+            return {
+                tab_page_id = 99,
+                session_id = nil,
+                status_animation = {
+                    start = function() end,
+                    stop = function() end,
+                },
+                _cancel_session = function() end,
+                _build_handlers = function()
+                    return {}
+                end,
+                new_session = SessionManager.new_session,
+                agent = {
+                    provider_config = { name = "Test" },
+                },
+            } --[[@as agentic.SessionManager]]
+        end
+
+        --- Stub agent:create_session to fire its callback synchronously
+        --- with the given response/err pair.
+        --- @param session agentic.SessionManager
+        --- @param response agentic.acp.SessionCreationResponse|nil
+        --- @param err agentic.acp.ACPError|nil
+        local function fake_create_session(session, response, err)
+            session.agent.create_session = function(_self, _handlers, callback)
+                callback(response, err)
+            end
+        end
+
+        after_each(function()
+            schedule_stub:revert()
+            Config.hooks = Config.hooks or {}
+            Config.hooks.on_create_session_response = nil
+        end)
+
+        it("fires on error with err set and response nil", function()
+            local hook_spy = spy.new(function() end)
+            Config.hooks = Config.hooks or {}
+            Config.hooks.on_create_session_response = function(data)
+                hook_spy(data)
+            end
+
+            local session = make_session()
+            local err = { code = -32000, message = "boom" }
+            fake_create_session(
+                session,
+                nil,
+                err --[[@as agentic.acp.ACPError]]
+            )
+
+            SessionManager.new_session(session)
+
+            assert.spy(hook_spy).was.called(1)
+            local data = hook_spy.calls[1][1]
+            assert.is_nil(data.session_id)
+            assert.equal(99, data.tab_page_id)
+            assert.is_nil(data.response)
+            assert.equal(err, data.err)
+            assert.is_nil(session.session_id)
+        end)
+
+        it("does not fire when no hook is configured", function()
+            Config.hooks = Config.hooks or {}
+            Config.hooks.on_create_session_response = nil
+
+            local session = make_session()
+            fake_create_session(session, nil, {
+                code = -32000,
+                message = "boom",
+            } --[[@as agentic.acp.ACPError]])
+
+            assert.has_no_errors(function()
+                SessionManager.new_session(session)
+            end)
+        end)
+
+        it("fires before the early return on error", function()
+            -- Verifies the contract: hook receives err, then session_id is
+            -- cleared. If the hook fired AFTER the early return, it would
+            -- never run on the error path.
+            local hook_call_order = {}
+            Config.hooks = Config.hooks or {}
+            Config.hooks.on_create_session_response = function(data)
+                table.insert(hook_call_order, {
+                    err = data.err,
+                    session_id_at_fire = data.session_id,
+                })
+            end
+
+            local session = make_session()
+            session.session_id = "stale-id"
+            fake_create_session(session, nil, {
+                code = -32000,
+                message = "boom",
+            } --[[@as agentic.acp.ACPError]])
+
+            SessionManager.new_session(session)
+
+            assert.equal(1, #hook_call_order)
+            assert.is_not_nil(hook_call_order[1].err)
+            assert.is_nil(session.session_id)
+        end)
+    end)
 end)