fix(assistant): address sidecar review follow-ups

packages/feature/assistant/src/infrastructure/model-boundary.lisp

@@ -8,6 +8,12 @@
   kind
   payload)
 
+(defun make-assistant-model-error-event (generation message)
+  (make-assistant-model-event
+   generation
+   :stream-error
+   (list (cons "message" (or message "assistant model stream error")))))
+
 (defstruct (assistant-model-boundary
             (:constructor %make-assistant-model-boundary
                 (start-fn request-fn poll-fn stop-fn status-fn)))

packages/feature/assistant/src/package.lisp

@@ -117,9 +117,11 @@ depend on one stable feature package.")
      #:assistant-model-event-generation
      #:assistant-model-event-kind
      #:assistant-model-event-payload
+     #:make-assistant-model-error-event
      #:next-assistant-turn-generation
      #:make-assistant-user-payload
      #:assistant-system-init-safe-p
      #:make-assistant-sidecar-boundary
      #:assistant-sidecar-command-arguments
+     #:+assistant-sidecar-response-timeout-seconds+
      #:make-assistant-fixture-boundary)))

src/infrastructure/acl/syscall-process-sidecar.lisp

@@ -91,20 +91,20 @@
     (if (null handle)
         (values nil status)
         (let* ((version nil)
+               (version-error nil)
                (reader
                 (sb-thread:make-thread
                  (lambda ()
                    (setf version
-                         (handler-case
-                             (with-output-to-string (stream)
+                         (with-output-to-string (stream)
+                           (handler-case
                                (loop for line = (read-line
                                                  (sidecar-handle-output handle)
                                                  nil nil)
                                      while line
-                                     do (write-line line stream)))
-                           (error (condition)
-                             (declare (ignore condition))
-                             nil)))
+                                     do (write-line line stream))
+                             (error (condition)
+                               (setf version-error (princ-to-string condition))))))
                    nil)
                  :name "nshell assistant sidecar version reader"))
               (deadline (+ (get-internal-real-time)
@@ -129,12 +129,18 @@
            (sb-thread:join-thread reader :default nil :timeout 0.2))
           (let ((exit-status (sidecar-exit-status handle)))
             (stop-sidecar handle)
-          (values (and version
+            (values (and version
                        (string-trim '(#\Space #\Tab #\Newline #\Return)
                                     version))
                   (cond (timed-out-p :timeout)
                         ((zerop (or exit-status 1)) :ok)
-                        (t :version-failed))))))))
+                        (t (list :version-failed
+                                 :output (and version
+                                              (string-trim
+                                               '(#\Space #\Tab #\Newline #\Return)
+                                               version))
+                                 :error version-error
+                                 :exit-status exit-status)))))))))
 
 (defun run-sidecar-version (command)
   (run-sidecar-version-cancellable command nil))

src/infrastructure/assistant-sidecar-stream.lisp

@@ -51,7 +51,9 @@
     (when (and (eql generation
                     (assistant-sidecar-state-startup-generation state))
                (eq handle (assistant-sidecar-state-handle state)))
-      (assistant-sidecar-state-pending state))))
+      (let ((pending (assistant-sidecar-state-pending state)))
+        (values pending
+                (and pending (assistant-pending-cell-generation pending)))))))
 
 (defun %assistant-sidecar-set-pending (state pending)
   (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
@@ -79,17 +81,15 @@
       (error () nil))))
 
 (defun %assistant-sidecar-error-event (generation message)
-  (make-assistant-model-event
-   generation
-   :stream-error
-   (list (cons "message" (or message "assistant sidecar stream error")))))
+  (make-assistant-model-error-event generation message))
 
 (defun %assistant-sidecar-reader-loop (state handle generation)
   (loop
     (multiple-value-bind (object status message)
         (%assistant-read-json-value
          (nshell.infrastructure.acl:sidecar-handle-output handle))
-      (let ((pending (%assistant-sidecar-reader-pending state handle generation)))
+      (multiple-value-bind (pending pending-generation)
+          (%assistant-sidecar-reader-pending state handle generation)
         (unless pending
           (return))
         (case status
@@ -101,11 +101,11 @@
                      (%assistant-sidecar-publish
                       pending
                       (%assistant-sidecar-error-event
-                       (assistant-pending-cell-generation pending)
+                       pending-generation
                        "assistant sidecar returned an unknown event"))
                      (%assistant-sidecar-complete pending))
                    (let ((event (make-assistant-model-event
-                                 (assistant-pending-cell-generation pending)
+                                 pending-generation
                                  kind
                                  object)))
                      (%assistant-sidecar-publish pending event)
@@ -122,9 +122,10 @@
              (%assistant-sidecar-publish
               pending
               (%assistant-sidecar-error-event
-               (assistant-pending-cell-generation pending)
+               pending-generation
                "assistant sidecar process exited before the pending request completed"))
              (%assistant-sidecar-complete pending))
+           (%assistant-sidecar-stop-state state :preserve-pending-p t)
            (return))
           (otherwise
            (when (and (eq status :error)
@@ -137,7 +138,7 @@
              (%assistant-sidecar-publish
               pending
               (%assistant-sidecar-error-event
-               (assistant-pending-cell-generation pending)
+               pending-generation
                message))
              (%assistant-sidecar-complete pending))
            (return)))))))
@@ -183,7 +184,8 @@
       (error () nil))))
 
 (defun %assistant-sidecar-join-thread (thread)
-  (when thread
+  (when (and thread
+             (not (eq thread sb-thread:*current-thread*)))
     (ignore-errors
      (sb-thread:join-thread thread :default nil :timeout 0.1))
     (when (sb-thread:thread-alive-p thread)
@@ -236,13 +238,14 @@
     (%assistant-sidecar-close-channel
      (assistant-pending-cell-events pending))))
 
-(defun %assistant-sidecar-stop-state (state &key preserve-pending-p)
+(defun %assistant-sidecar-stop-state (state &key preserve-pending-p preserve-starting-p)
   (let (pending write-channel handle startup reader writer error-thread)
     (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
-      (setf (assistant-sidecar-state-starting-p state) nil
-            (assistant-sidecar-state-startup-generation state)
-              (1+ (assistant-sidecar-state-startup-generation state))
-            pending (assistant-sidecar-state-pending state)
+      (unless preserve-starting-p
+        (setf (assistant-sidecar-state-starting-p state) nil
+              (assistant-sidecar-state-startup-generation state)
+                (1+ (assistant-sidecar-state-startup-generation state))))
+      (setf pending (assistant-sidecar-state-pending state)
             write-channel (assistant-sidecar-state-write-channel state)
             handle (assistant-sidecar-state-handle state)
             startup (assistant-sidecar-state-start-thread state)
@@ -445,47 +448,61 @@
                                                      :preserve-pending-p t))))))))))))
 
 (defun %assistant-sidecar-start (state)
-  (if (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
-        (assistant-sidecar-state-starting-p state))
-      t
-      (if (null (assistant-sidecar-state-command state))
-      (progn
-        (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
-          (setf (assistant-sidecar-state-starting-p state) nil
-                (assistant-sidecar-state-disabled-reason state)
-                :disabled-by-environment))
-        nil)
-      (let ((handle (%assistant-sidecar-handle state))
-            (init-p (%assistant-sidecar-init-p state))
-            (dead-p (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
-                      (assistant-sidecar-state-dead-p state))))
-        (if (and handle init-p (not dead-p)
-                 (nshell.infrastructure.acl:sidecar-alive-p handle))
-      t
-          (progn
-        (%assistant-sidecar-stop-state state)
-        (let ((startup-generation
-                (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
-                  (setf (assistant-sidecar-state-starting-p state) t
-                        (assistant-sidecar-state-disabled-reason state) nil
-                        (assistant-sidecar-state-dead-p state) nil
-                        (assistant-sidecar-state-dead-reason state) nil)
-                  (incf (assistant-sidecar-state-startup-generation state))))
-              (pending
-                (%make-assistant-pending-cell
-                 0
-                 (cl-concurrent-kit:make-channel :buffer-size 128)
-                 (cl-concurrent-kit:make-promise))))
-          (%assistant-sidecar-set-pending state pending)
-          (let ((thread (sb-thread:make-thread
-                         (lambda ()
-                           (unwind-protect
-                           (%assistant-sidecar-start-worker
-                            state pending startup-generation)
-                             (%assistant-sidecar-set-start-thread state nil)))
-                         :name "nshell assistant sidecar startup")))
-            (%assistant-sidecar-set-start-thread state thread))
-          t)))))))
+  (let ((startup-generation
+          (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
+            (cond
+              ((assistant-sidecar-state-starting-p state) :already-starting)
+              ((null (assistant-sidecar-state-command state))
+               (setf (assistant-sidecar-state-disabled-reason state)
+                     :disabled-by-environment)
+               :disabled)
+              (t
+               (setf (assistant-sidecar-state-starting-p state) t
+                     (assistant-sidecar-state-disabled-reason state) nil
+                     (assistant-sidecar-state-dead-p state) nil
+                     (assistant-sidecar-state-dead-reason state) nil)
+               (incf (assistant-sidecar-state-startup-generation state)))))))
+    (case startup-generation
+      (:already-starting t)
+      (:disabled nil)
+      (otherwise
+       (let ((handle (%assistant-sidecar-handle state))
+             (init-p (%assistant-sidecar-init-p state))
+             (dead-p (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
+                       (assistant-sidecar-state-dead-p state))))
+         (if (and handle init-p (not dead-p)
+                  (nshell.infrastructure.acl:sidecar-alive-p handle))
+             (progn
+               (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
+                 (setf (assistant-sidecar-state-starting-p state) nil))
+               t)
+             (progn
+               (let ((handoff-complete-p nil))
+                 (unwind-protect
+                      (progn
+                        (%assistant-sidecar-stop-state state :preserve-starting-p t)
+                        (let ((pending
+                                (%make-assistant-pending-cell
+                                 0
+                                 (cl-concurrent-kit:make-channel :buffer-size 128)
+                                 (cl-concurrent-kit:make-promise))))
+                          (%assistant-sidecar-set-pending state pending)
+                          (let ((thread (sb-thread:make-thread
+                                         (lambda ()
+                                           (unwind-protect
+                                                (%assistant-sidecar-start-worker
+                                                 state pending startup-generation)
+                                             (%assistant-sidecar-set-start-thread
+                                              state nil)))
+                                         :name "nshell assistant sidecar startup")))
+                            (%assistant-sidecar-set-start-thread state thread)
+                            (setf handoff-complete-p t)))
+                        t)
+                   (unless handoff-complete-p
+                     (sb-thread:with-mutex ((assistant-sidecar-state-lock state))
+                       (when (eql startup-generation
+                                  (assistant-sidecar-state-startup-generation state))
+                         (setf (assistant-sidecar-state-starting-p state) nil)))))))))))))
 
 (defun %assistant-sidecar-request (state generation payload)
   (let ((handle (%assistant-sidecar-handle state))

src/infrastructure/assistant-sidecar.lisp

@@ -3,6 +3,9 @@
 (defparameter +assistant-sidecar-handshake-timeout-seconds+ 10
   "Maximum time allowed for sidecar version and init handshakes.")
 
+(defparameter +assistant-sidecar-response-timeout-seconds+ 90
+  "Maximum silence allowed between response events; long generations may run longer.")
+
 (defstruct (assistant-sidecar-state
             (:constructor %make-assistant-sidecar-state (command arguments)))
   command

src/presentation/repl-agent.lisp

@@ -67,6 +67,7 @@
   (setf *agent-session* nil
         *assistant-model-event-handler* nil
         *assistant-turn-started-at* nil
+        *assistant-last-event-at* nil
         *assistant-request-kind* nil
         *assistant-command-origin* :typed
         *assistant-command-confirmed-p* nil

src/presentation/repl-ai.lisp

@@ -5,6 +5,7 @@
           (nshell.feature.assistant:next-assistant-turn-generation
            *assistant-turn-generation*)
         *assistant-turn-started-at* nil
+        *assistant-last-event-at* nil
         *last-assistant-model-event* nil
         *assistant-model-event-handler* nil
         *assistant-request-kind* nil

src/presentation/repl-assistant-poll.lisp

@@ -1,15 +1,33 @@
 ;;; REPL polling bridge for assistant model events
 (in-package #:nshell.presentation)
 
+(defun %assistant-response-deadline-expired-p (now)
+  (and *assistant-last-event-at*
+       (>= (- now *assistant-last-event-at*)
+           (* nshell.feature.assistant:+assistant-sidecar-response-timeout-seconds+
+              internal-time-units-per-second))))
+
 (defun %poll-assistant-model-event ()
-  (let ((result
-          (nshell.feature.assistant:assistant-model-poll
-           *assistant-turn-generation*)))
+  (let* ((now (boundary-monotonic))
+         (result
+           (if (%assistant-response-deadline-expired-p now)
+               (progn
+                 (nshell.feature.assistant:assistant-model-stop)
+                 (list :status :event
+                       :value
+                       (nshell.feature.assistant:make-assistant-model-error-event
+                        *assistant-turn-generation*
+                        (format nil
+                                "AI 応答がタイムアウトしました（~D 秒）"
+                                nshell.feature.assistant:+assistant-sidecar-response-timeout-seconds+))))
+               (nshell.feature.assistant:assistant-model-poll
+                *assistant-turn-generation*))))
     (when (eq :event
               (nshell.feature.assistant:assistant-boundary-status result))
       (let ((event (nshell.feature.assistant:assistant-boundary-value result)))
         (when (and (nshell.feature.assistant:assistant-model-event-p event)
                    (eql *assistant-turn-generation*
                         (nshell.feature.assistant:assistant-model-event-generation
                          event)))
+          (setf *assistant-last-event-at* now)
           event)))))

src/presentation/repl-output-handlers.lisp

@@ -224,6 +224,7 @@ wrapped line's other rows on screen as stale duplicates."
   (clear-rendered-transient-panel)
   (setf *assistant-model-event-handler* nil
         *assistant-turn-started-at* nil
+        *assistant-last-event-at* nil
         *assistant-request-kind* nil
         *assistant-explain-candidates* nil
         *assistant-explain-candidate-index* 0
@@ -341,6 +342,7 @@ wrapped line's other rows on screen as stale duplicates."
   (setf *input-state* (make-repl-input-state)
         *assistant-model-event-handler* nil
         *assistant-turn-started-at* nil
+        *assistant-last-event-at* nil
         *assistant-request-kind* nil
         *assistant-explain-candidates* nil
         *assistant-explain-candidate-index* 0
@@ -389,7 +391,8 @@ wrapped line's other rows on screen as stale duplicates."
               (multiple-value-bind (status classification reason)
                   (%assistant-proposal-assessment proposal)
                 (setf *assistant-model-event-handler* nil
-                      *assistant-turn-started-at* nil)
+                      *assistant-turn-started-at* nil
+                      *assistant-last-event-at* nil)
                 (clear-rendered-transient-panel)
                 (if (eq status :parse-error)
                     (progn
@@ -459,7 +462,6 @@ wrapped line's other rows on screen as stale duplicates."
     (setf *assistant-last-cancel-at* nil)
     (setf *assistant-explain-candidates* nil
           *assistant-explain-candidate-index* 0)
-    (setf *assistant-turn-started-at* (boundary-monotonic))
     (unless (%assistant-boundary-ok-p start-result)
       (return-from %process-ask-submit-output-event
         (%return-from-ask-with-message
@@ -480,6 +482,8 @@ wrapped line's other rows on screen as stale duplicates."
            (format nil "AI 要求を送信できません: ~a"
                    (%assistant-boundary-failure-message
                     request-result "assistant model request failed")))))
+      (setf *assistant-turn-started-at* (boundary-monotonic))
+      (setf *assistant-last-event-at* *assistant-turn-started-at*)
       (setf *assistant-model-event-handler* #'%handle-ask-model-event)
       (render-prompt-cont)
       (render-assistant-progress-panel *assistant-turn-started-at*)
@@ -540,6 +544,7 @@ wrapped line's other rows on screen as stale duplicates."
        (setf *input-state* (make-repl-input-state)
              *assistant-model-event-handler* nil
              *assistant-turn-started-at* nil
+             *assistant-last-event-at* nil
              *assistant-request-kind* nil
              *assistant-explain-candidates* nil
              *assistant-explain-candidate-index* 0

src/presentation/repl-session-init.lisp

@@ -159,6 +159,7 @@ entered during this session."
         *input-state* (make-repl-input-state)
         *assistant-turn-generation* 0
         *assistant-turn-started-at* nil
+        *assistant-last-event-at* nil
         *assistant-last-cancel-at* nil
         *last-assistant-model-event* nil
         *assistant-model-event-handler* nil