Conversation
|
✅ Preview binaries are ready! To test with modules: |
mafredri
left a comment
mafredri
left a comment
There was a problem hiding this comment.
Nice work so far! I left a few comments and suggestions. Mainly about moving some concerns from httpapi to cmd/server. I think it would be helpful to have some tests based on real agent session restoration output to evaluate the screentracker changes.
lib/httpapi/server.go
Outdated
| currentStatus = st.ConversationStatusChanging | ||
| s.logger.Info("Initial prompt sent successfully") | ||
| if !s.stateLoadComplete && s.statePersistenceCfg.LoadState { | ||
| _, _ = s.conversation.LoadState(s.statePersistenceCfg.StateFile) |
There was a problem hiding this comment.
Not objecting, just curious. Why do we wait for stability to load the state?
There was a problem hiding this comment.
This is by design. We wait for the initial stable state to capture a baseline screen snapshot. This baseline allows us to clear any agent-generated messages or screen content before loading our saved state.
| func (s *Server) HandleSignals(ctx context.Context, process *termexec.Process) { | ||
| // Handle shutdown signals (SIGTERM, SIGINT only on Windows) | ||
| shutdownCh := make(chan os.Signal, 1) | ||
| signal.Notify(shutdownCh, os.Interrupt, syscall.SIGTERM) |
There was a problem hiding this comment.
Does this compile on Windows? IIRC we can only support os.Interrupt there.
There was a problem hiding this comment.
compiles - yes (since PR Preview Build / Build Release Binaries (pull_request) passes), but haven't tested it.
# Conflicts: # cmd/server/server.go # lib/httpapi/server.go # lib/screentracker/conversation.go # lib/screentracker/pty_conversation.go # lib/screentracker/pty_conversation_test.go
# Conflicts: # lib/httpapi/server.go # lib/screentracker/pty_conversation.go
3 participants
Closes: coder/internal#1256
MergeAfter: #172