feat: add Slack-style unread divider to session transcript (#1922)
This commit is contained in:
@@ -969,7 +969,7 @@ func (m *Manager) Start(ctx context.Context) error {
|
||||
|
||||
// Forward adapter updates to our channel
|
||||
m.wg.Add(1)
|
||||
go m.forwardUpdates()
|
||||
go m.forwardUpdates(m.adapter, m.stopCh)
|
||||
|
||||
// Start workspace tracker with background context (not tied to HTTP request)
|
||||
root, trackers := m.snapshotTrackers()
|
||||
@@ -1000,7 +1000,7 @@ func (m *Manager) startOneShot() error {
|
||||
|
||||
// Forward adapter updates to our channel
|
||||
m.wg.Add(1)
|
||||
go m.forwardUpdates()
|
||||
go m.forwardUpdates(m.adapter, m.stopCh)
|
||||
|
||||
// Start workspace tracker with background context (not tied to HTTP request)
|
||||
root, trackers := m.snapshotTrackers()
|
||||
@@ -1385,10 +1385,10 @@ func (m *Manager) createAdapter() error {
|
||||
}
|
||||
|
||||
// forwardUpdates forwards updates from the adapter to the manager's channel
|
||||
func (m *Manager) forwardUpdates() {
|
||||
func (m *Manager) forwardUpdates(agentAdapter adapter.AgentAdapter, stopCh <-chan struct{}) {
|
||||
defer m.wg.Done()
|
||||
|
||||
updatesCh := m.adapter.Updates()
|
||||
updatesCh := agentAdapter.Updates()
|
||||
for {
|
||||
select {
|
||||
case update, ok := <-updatesCh:
|
||||
@@ -1400,7 +1400,7 @@ func (m *Manager) forwardUpdates() {
|
||||
default:
|
||||
m.logger.Warn("updates channel full, dropping notification")
|
||||
}
|
||||
case <-m.stopCh:
|
||||
case <-stopCh:
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"strings"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agentctl/server/adapter"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/config"
|
||||
@@ -72,6 +73,35 @@ func TestPublishMCPAttachmentAttemptCarriesBackendOwnedIdentity(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestForwardUpdatesUsesItsGenerationStopChannel(t *testing.T) {
|
||||
stopCh := make(chan struct{})
|
||||
m := &Manager{
|
||||
adapter: newStubAdapter(),
|
||||
updatesCh: make(chan adapter.AgentEvent, 1),
|
||||
stopCh: stopCh,
|
||||
logger: newTestLogger(t),
|
||||
}
|
||||
m.wg.Add(1)
|
||||
go m.forwardUpdates(m.adapter, stopCh)
|
||||
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
m.wg.Wait()
|
||||
close(done)
|
||||
}()
|
||||
|
||||
m.stopCh = make(chan struct{})
|
||||
close(stopCh)
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
close(m.stopCh)
|
||||
<-done
|
||||
t.Fatal("forwardUpdates did not stop with its original lifecycle channel")
|
||||
}
|
||||
}
|
||||
|
||||
// stubAdapter is a minimal AgentAdapter for testing the startup sequence.
|
||||
type stubAdapter struct {
|
||||
connectCalled bool
|
||||
|
||||
@@ -456,6 +456,7 @@ func mapUserSettingsState(response userdto.UserSettingsResponse, workspaceID str
|
||||
"chatSubmitKey": defaultString(settings.ChatSubmitKey, "cmd_enter"),
|
||||
"reviewAutoMarkOnScroll": settings.ReviewAutoMarkOnScroll,
|
||||
"confirmTaskArchive": settings.ConfirmTaskArchive,
|
||||
"unreadDivider": settings.UnreadDivider,
|
||||
"mcpTaskAgentProfileDefault": usermodels.NormalizeMCPTaskAgentProfileDefault(settings.MCPTaskAgentProfileDefault),
|
||||
"showAnchoredPromptBar": settings.ShowAnchoredPromptBar,
|
||||
"showScrollToLastPrompt": settings.ShowScrollToLastPrompt,
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"net/http"
|
||||
"os"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"go.uber.org/zap"
|
||||
@@ -55,6 +56,11 @@ func registerE2EResetRoutes(
|
||||
// so tests can assert on session-state-derived behavior without a full
|
||||
// agent stop/resume cycle.
|
||||
api.POST("/task-sessions", handleE2ECreateTaskSession(repo, log))
|
||||
// Force-sets a session's read cursor, bypassing the production mark-read
|
||||
// endpoint's monotonic (forward-only) guard, so specs can deterministically
|
||||
// seed "the user last read up through an earlier message" without a real
|
||||
// background agent turn.
|
||||
api.PATCH("/task-sessions/:id/read-cursor", handleE2ESetSessionReadCursor(repo, log))
|
||||
|
||||
log.Info("registered E2E endpoints (test-only)")
|
||||
}
|
||||
@@ -441,3 +447,40 @@ func handleE2ECreateTaskSession(repo *sqliterepo.Repository, log *logger.Logger)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
type e2eSetSessionReadCursorRequest struct {
|
||||
MessageID string `json:"message_id"`
|
||||
}
|
||||
|
||||
// handleE2ESetSessionReadCursor force-sets a session's last_read_message_id
|
||||
// via a raw update, bypassing UpdateTaskSessionLastReadMessageID's monotonic
|
||||
// guard (production only ever advances the cursor forward — see
|
||||
// repository/sqlite/session.go). E2E specs use this to seed "the user last
|
||||
// read up through an earlier message" deterministically, since replaying an
|
||||
// older messageId through the real mark-read endpoint is now a rejected
|
||||
// no-op rather than a rewind.
|
||||
func handleE2ESetSessionReadCursor(repo *sqliterepo.Repository, log *logger.Logger) gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
sessionID := c.Param("id")
|
||||
var body e2eSetSessionReadCursorRequest
|
||||
if err := c.ShouldBindJSON(&body); err != nil || body.MessageID == "" {
|
||||
c.JSON(http.StatusBadRequest, gin.H{errKey: "message_id is required"})
|
||||
return
|
||||
}
|
||||
result, err := repo.DB().ExecContext(c.Request.Context(),
|
||||
`UPDATE task_sessions SET last_read_message_id = ?, updated_at = ? WHERE id = ?`,
|
||||
body.MessageID, time.Now().UTC(), sessionID,
|
||||
)
|
||||
if err != nil {
|
||||
log.Error("e2e: failed to force-set session read cursor", zap.Error(err))
|
||||
c.JSON(http.StatusInternalServerError, gin.H{errKey: err.Error()})
|
||||
return
|
||||
}
|
||||
rows, _ := result.RowsAffected()
|
||||
if rows == 0 {
|
||||
c.JSON(http.StatusNotFound, gin.H{errKey: "session not found: " + sessionID})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"id": sessionID, "last_read_message_id": body.MessageID})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -130,7 +130,6 @@ func TestApplyStatesToConfigSetsClaudeBackgroundPromptHandoff(t *testing.T) {
|
||||
t.Fatal("ApplyStatesToConfig did not set Features.ClaudeBackgroundPromptHandoff = true")
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyStatesToConfigMarksImpliedDebugEnvAsApplied(t *testing.T) {
|
||||
for _, name := range []string{
|
||||
"KANDEV_DEBUG_PPROF_ENABLED",
|
||||
|
||||
@@ -75,7 +75,6 @@ func TestDefinitionsIncludeClaudeBackgroundPromptHandoffMetadata(t *testing.T) {
|
||||
t.Fatal("Mutable = false, want true")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefinitionsExposeSingleUserFacingDebugToggle(t *testing.T) {
|
||||
def, ok := DefinitionByKey("debug.devMode")
|
||||
if !ok {
|
||||
|
||||
@@ -264,6 +264,10 @@ type TaskSessionDTO struct {
|
||||
// EnrichForegroundActivity, never by FromTaskSession.
|
||||
ForegroundActivity v1.ForegroundActivity `json:"foreground_activity,omitempty"`
|
||||
ActiveSubagentCount int `json:"active_subagent_count"`
|
||||
// LastReadMessageID is the session's Slack-style read cursor — the id of
|
||||
// the newest message the frontend has marked as read. Used by the
|
||||
// transcript to position the unread ("New") divider.
|
||||
LastReadMessageID string `json:"last_read_message_id,omitempty"`
|
||||
}
|
||||
|
||||
// TaskSessionSummaryDTO is a lightweight version of TaskSessionDTO without snapshot fields.
|
||||
@@ -304,6 +308,7 @@ type TaskSessionSummaryDTO struct {
|
||||
// (ADR-0049); see TaskSessionDTO.
|
||||
ForegroundActivity v1.ForegroundActivity `json:"foreground_activity,omitempty"`
|
||||
ActiveSubagentCount int `json:"active_subagent_count"`
|
||||
LastReadMessageID string `json:"last_read_message_id,omitempty"`
|
||||
// CommandCount is the number of tool_call messages on this session,
|
||||
// surfaced inline in the timeline entry header ("ran N commands").
|
||||
// Populated by ListTaskSessions; defaults to 0 for callers that don't
|
||||
@@ -321,6 +326,16 @@ type GetTaskSessionResponse struct {
|
||||
Session TaskSessionDTO `json:"session"`
|
||||
}
|
||||
|
||||
// MarkSessionReadResponse is intentionally narrow: it carries only the
|
||||
// updated read cursor, never a full session snapshot. A full-session
|
||||
// response would be frozen at request time and, if applied verbatim by a
|
||||
// client, could overwrite a newer field (e.g. state) that arrived via a
|
||||
// concurrent WebSocket update while this request was still in flight.
|
||||
type MarkSessionReadResponse struct {
|
||||
SessionID string `json:"session_id"`
|
||||
LastReadMessageID string `json:"last_read_message_id"`
|
||||
}
|
||||
|
||||
type ListTaskSessionsResponse struct {
|
||||
Sessions []TaskSessionDTO `json:"sessions"`
|
||||
Total int `json:"total"`
|
||||
@@ -723,6 +738,7 @@ func FromTaskSessionSummary(session *models.TaskSession) TaskSessionSummaryDTO {
|
||||
IsPassthrough: session.IsPassthrough,
|
||||
ReviewStatus: session.ReviewStatus,
|
||||
TaskEnvironmentID: session.TaskEnvironmentID,
|
||||
LastReadMessageID: session.LastReadMessageID,
|
||||
}
|
||||
if len(session.Worktrees) > 0 {
|
||||
result.WorktreeID = session.Worktrees[0].WorktreeID
|
||||
@@ -762,6 +778,7 @@ func FromTaskSession(session *models.TaskSession) TaskSessionDTO {
|
||||
IsPassthrough: session.IsPassthrough,
|
||||
ReviewStatus: session.ReviewStatus,
|
||||
TaskEnvironmentID: session.TaskEnvironmentID,
|
||||
LastReadMessageID: session.LastReadMessageID,
|
||||
}
|
||||
if len(session.Worktrees) > 0 {
|
||||
result.WorktreeID = session.Worktrees[0].WorktreeID
|
||||
|
||||
@@ -593,6 +593,9 @@ func (m *mockRepository) DismissLastAgentError(_ context.Context, _ string, _ mo
|
||||
func (m *mockRepository) GetLastAgentMessage(_ context.Context, _ string) (string, error) {
|
||||
return "", nil
|
||||
}
|
||||
func (m *mockRepository) UpdateTaskSessionLastReadMessageID(_ context.Context, _ string, _ string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Task Plan operations
|
||||
func (m *mockRepository) CreateTaskPlan(ctx context.Context, plan *models.TaskPlan) error {
|
||||
|
||||
@@ -126,6 +126,7 @@ func (h *TaskHandlers) registerHTTP(router *gin.Engine) {
|
||||
api.GET("/tasks/:id/context", h.httpGetTaskContext)
|
||||
api.GET("/task-sessions/:id", h.httpGetTaskSession)
|
||||
api.POST("/task-sessions/:id/last-agent-error/dismiss", h.httpDismissLastAgentError)
|
||||
api.POST("/task-sessions/:id/mark-read", h.httpMarkSessionRead)
|
||||
api.GET("/tasks/:id/sessions", h.httpListTaskSessions)
|
||||
api.POST("/tasks/:id/sessions/ensure", h.httpEnsureTaskSession)
|
||||
api.GET("/tasks/:id/environment", h.httpGetTaskEnvironment)
|
||||
|
||||
@@ -471,6 +471,45 @@ func (h *TaskHandlers) httpDismissLastAgentError(c *gin.Context) {
|
||||
})
|
||||
}
|
||||
|
||||
type markSessionReadRequest struct {
|
||||
MessageID string `json:"message_id"`
|
||||
}
|
||||
|
||||
// httpMarkSessionRead advances the session's Slack-style unread-divider read
|
||||
// cursor to req.MessageID. A missing session is a 404; bad caller input
|
||||
// (empty ids, an unknown message, or a message belonging to a different
|
||||
// session — see service.ErrInvalidMarkSessionRead) is a 400; any other,
|
||||
// unexpected failure (a repository/DB error) is logged and returned as a
|
||||
// sanitized 500 rather than leaking the raw internal error to the caller.
|
||||
func (h *TaskHandlers) httpMarkSessionRead(c *gin.Context) {
|
||||
var req markSessionReadRequest
|
||||
if err := c.ShouldBindJSON(&req); err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid JSON: " + err.Error()})
|
||||
return
|
||||
}
|
||||
sessionID := c.Param("id")
|
||||
session, err := h.service.MarkSessionRead(c.Request.Context(), sessionID, req.MessageID)
|
||||
if err != nil {
|
||||
if errors.Is(err, models.ErrTaskSessionNotFound) {
|
||||
c.JSON(http.StatusNotFound, gin.H{"error": "task session not found"})
|
||||
return
|
||||
}
|
||||
if errors.Is(err, service.ErrInvalidMarkSessionRead) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
h.logger.Error("failed to mark session read",
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(err))
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to mark session read"})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, dto.MarkSessionReadResponse{
|
||||
SessionID: session.ID,
|
||||
LastReadMessageID: session.LastReadMessageID,
|
||||
})
|
||||
}
|
||||
|
||||
func (h *TaskHandlers) httpListSessionTurns(c *gin.Context) {
|
||||
sessionID := c.Param("id")
|
||||
if sessionID == "" {
|
||||
|
||||
@@ -2,6 +2,7 @@ package handlers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -1171,6 +1172,212 @@ func TestHTTPMoveTaskAllowsRunningPrimarySession(t *testing.T) {
|
||||
assert.Equal(t, "step-target", repo.task.WorkflowStepID)
|
||||
}
|
||||
|
||||
// markSessionReadRepo layers message storage and a mutable
|
||||
// UpdateTaskSessionLastReadMessageID on top of mockRepository's session map,
|
||||
// mirroring the real repository's narrow single-column write and
|
||||
// not-found behavior closely enough to exercise httpMarkSessionRead's error
|
||||
// classification (404 for a missing session, 400 for everything else).
|
||||
type markSessionReadRepo struct {
|
||||
mockRepository
|
||||
messages map[string]*models.Message
|
||||
// getMessageErr, when set, simulates an unexpected repository/DB failure
|
||||
// from GetMessage (as opposed to the ordinary "message not found" case).
|
||||
getMessageErr error
|
||||
}
|
||||
|
||||
func (m *markSessionReadRepo) GetMessage(_ context.Context, id string) (*models.Message, error) {
|
||||
if m.getMessageErr != nil {
|
||||
return nil, m.getMessageErr
|
||||
}
|
||||
msg, ok := m.messages[id]
|
||||
if !ok {
|
||||
return nil, sql.ErrNoRows
|
||||
}
|
||||
return msg, nil
|
||||
}
|
||||
|
||||
func (m *markSessionReadRepo) UpdateTaskSessionLastReadMessageID(_ context.Context, id, messageID string) error {
|
||||
session, ok := m.sessions[id]
|
||||
if !ok {
|
||||
return fmt.Errorf("%w: agent session not found: %s", models.ErrTaskSessionNotFound, id)
|
||||
}
|
||||
session.LastReadMessageID = messageID
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *markSessionReadRepo) CountToolCallMessagesBySession(context.Context, []string) (map[string]int, error) {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func newMarkSessionReadService(t *testing.T, repo *markSessionReadRepo, log *logger.Logger) *service.Service {
|
||||
t.Helper()
|
||||
return service.NewService(service.Repos{
|
||||
Workspaces: repo, Tasks: repo, TaskRepos: repo,
|
||||
Workflows: repo, Messages: repo, Turns: repo,
|
||||
Sessions: repo, GitSnapshots: repo, RepoEntities: repo,
|
||||
Executors: repo, Environments: repo, TaskEnvironments: repo,
|
||||
Reviews: repo,
|
||||
}, nil, log, service.RepositoryDiscoveryConfig{})
|
||||
}
|
||||
|
||||
func newMarkSessionReadRouter(t *testing.T, repo *markSessionReadRepo, log *logger.Logger) *gin.Engine {
|
||||
t.Helper()
|
||||
router := gin.New()
|
||||
NewTaskHandlers(newMarkSessionReadService(t, repo, log), nil, repo, nil, log).registerHTTP(router)
|
||||
return router
|
||||
}
|
||||
|
||||
func TestHTTPMarkSessionReadAdvancesCursor(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
log := newTestLogger(t)
|
||||
repo := &markSessionReadRepo{
|
||||
mockRepository: mockRepository{
|
||||
sessions: map[string]*models.TaskSession{
|
||||
"session-1": {ID: "session-1", TaskID: "task-1"},
|
||||
},
|
||||
},
|
||||
messages: map[string]*models.Message{
|
||||
"msg-1": {ID: "msg-1", TaskSessionID: "session-1"},
|
||||
},
|
||||
}
|
||||
router := newMarkSessionReadRouter(t, repo, log)
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/task-sessions/session-1/mark-read",
|
||||
strings.NewReader(`{"message_id":"msg-1"}`))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
require.Equal(t, http.StatusOK, rec.Code, "body: %s", rec.Body.String())
|
||||
assert.Equal(t, "msg-1", repo.sessions["session-1"].LastReadMessageID)
|
||||
var body dto.MarkSessionReadResponse
|
||||
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body))
|
||||
assert.Equal(t, "session-1", body.SessionID)
|
||||
assert.Equal(t, "msg-1", body.LastReadMessageID)
|
||||
assert.JSONEq(t, `{"session_id":"session-1","last_read_message_id":"msg-1"}`, rec.Body.String())
|
||||
}
|
||||
|
||||
func TestHTTPMarkSessionReadMissingSessionReturns404(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
log := newTestLogger(t)
|
||||
repo := &markSessionReadRepo{
|
||||
mockRepository: mockRepository{sessions: map[string]*models.TaskSession{}},
|
||||
messages: map[string]*models.Message{
|
||||
"msg-1": {ID: "msg-1", TaskSessionID: "session-missing"},
|
||||
},
|
||||
}
|
||||
h := &TaskHandlers{service: newMarkSessionReadService(t, repo, log), logger: log}
|
||||
rec := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(rec)
|
||||
c.Params = gin.Params{{Key: "id", Value: "session-missing"}}
|
||||
c.Request = httptest.NewRequest(http.MethodPost, "/task-sessions/session-missing/mark-read",
|
||||
strings.NewReader(`{"message_id":"msg-1"}`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.httpMarkSessionRead(c)
|
||||
|
||||
assert.Equal(t, http.StatusNotFound, rec.Code)
|
||||
}
|
||||
|
||||
func TestHTTPMarkSessionReadRejectsCrossSessionMessage(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
log := newTestLogger(t)
|
||||
repo := &markSessionReadRepo{
|
||||
mockRepository: mockRepository{
|
||||
sessions: map[string]*models.TaskSession{
|
||||
"session-1": {ID: "session-1", TaskID: "task-1"},
|
||||
},
|
||||
},
|
||||
messages: map[string]*models.Message{
|
||||
"msg-other": {ID: "msg-other", TaskSessionID: "session-2"},
|
||||
},
|
||||
}
|
||||
h := &TaskHandlers{service: newMarkSessionReadService(t, repo, log), logger: log}
|
||||
rec := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(rec)
|
||||
c.Params = gin.Params{{Key: "id", Value: "session-1"}}
|
||||
c.Request = httptest.NewRequest(http.MethodPost, "/task-sessions/session-1/mark-read",
|
||||
strings.NewReader(`{"message_id":"msg-other"}`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.httpMarkSessionRead(c)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, rec.Code)
|
||||
assert.Empty(t, repo.sessions["session-1"].LastReadMessageID)
|
||||
}
|
||||
|
||||
func TestHTTPMarkSessionReadRejectsUnknownMessage(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
log := newTestLogger(t)
|
||||
repo := &markSessionReadRepo{
|
||||
mockRepository: mockRepository{
|
||||
sessions: map[string]*models.TaskSession{
|
||||
"session-1": {ID: "session-1", TaskID: "task-1"},
|
||||
},
|
||||
},
|
||||
messages: map[string]*models.Message{},
|
||||
}
|
||||
h := &TaskHandlers{service: newMarkSessionReadService(t, repo, log), logger: log}
|
||||
rec := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(rec)
|
||||
c.Params = gin.Params{{Key: "id", Value: "session-1"}}
|
||||
c.Request = httptest.NewRequest(http.MethodPost, "/task-sessions/session-1/mark-read",
|
||||
strings.NewReader(`{"message_id":"does-not-exist"}`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.httpMarkSessionRead(c)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, rec.Code)
|
||||
assert.Empty(t, repo.sessions["session-1"].LastReadMessageID)
|
||||
}
|
||||
|
||||
// TestHTTPMarkSessionReadSanitizesUnexpectedRepositoryError verifies an
|
||||
// unexpected repository/DB failure (as opposed to bad caller input) never
|
||||
// leaks its raw error text to the client and is reported as a 500, not a
|
||||
// 400 — the caller did nothing wrong here.
|
||||
func TestHTTPMarkSessionReadSanitizesUnexpectedRepositoryError(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
log := newTestLogger(t)
|
||||
repo := &markSessionReadRepo{
|
||||
mockRepository: mockRepository{
|
||||
sessions: map[string]*models.TaskSession{
|
||||
"session-1": {ID: "session-1", TaskID: "task-1"},
|
||||
},
|
||||
},
|
||||
messages: map[string]*models.Message{},
|
||||
getMessageErr: errors.New("connection refused: dial tcp 127.0.0.1:5432"),
|
||||
}
|
||||
h := &TaskHandlers{service: newMarkSessionReadService(t, repo, log), logger: log}
|
||||
rec := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(rec)
|
||||
c.Params = gin.Params{{Key: "id", Value: "session-1"}}
|
||||
c.Request = httptest.NewRequest(http.MethodPost, "/task-sessions/session-1/mark-read",
|
||||
strings.NewReader(`{"message_id":"msg-1"}`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.httpMarkSessionRead(c)
|
||||
|
||||
assert.Equal(t, http.StatusInternalServerError, rec.Code)
|
||||
assert.NotContains(t, rec.Body.String(), "connection refused")
|
||||
assert.NotContains(t, rec.Body.String(), "5432")
|
||||
}
|
||||
|
||||
func TestHTTPMarkSessionReadRejectsInvalidJSON(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
log := newTestLogger(t)
|
||||
h := &TaskHandlers{logger: log}
|
||||
rec := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(rec)
|
||||
c.Params = gin.Params{{Key: "id", Value: "session-1"}}
|
||||
c.Request = httptest.NewRequest(http.MethodPost, "/task-sessions/session-1/mark-read",
|
||||
strings.NewReader(`not json`))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
h.httpMarkSessionRead(c)
|
||||
|
||||
assert.Equal(t, http.StatusBadRequest, rec.Code)
|
||||
}
|
||||
|
||||
func TestResolveFreshBranchName(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
@@ -949,6 +949,13 @@ type TaskSession struct {
|
||||
IsPrimary bool `json:"is_primary"` // Whether this is the primary session for the task
|
||||
IsPassthrough bool `json:"is_passthrough"` // Whether this session uses passthrough (PTY) mode
|
||||
ReviewStatus ReviewStatus `json:"review_status,omitempty"` // zero value = no review needed
|
||||
// LastReadMessageID is the id of the newest message the frontend has
|
||||
// marked as read for this session — the Slack-style "read cursor" used
|
||||
// to position the unread ("New") divider in the transcript. Advanced to
|
||||
// the latest message id whenever the session becomes the visible chat
|
||||
// panel; the frontend snapshots the PRIOR value at that moment to draw
|
||||
// the divider before overwriting it.
|
||||
LastReadMessageID string `json:"last_read_message_id,omitempty"`
|
||||
}
|
||||
|
||||
// ToAPI converts internal TaskSession to API type
|
||||
@@ -1011,6 +1018,9 @@ func (s *TaskSession) ToAPI() map[string]interface{} {
|
||||
if s.AgentProfileID != "" {
|
||||
result["agent_profile_id"] = s.AgentProfileID
|
||||
}
|
||||
if s.LastReadMessageID != "" {
|
||||
result["last_read_message_id"] = s.LastReadMessageID
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
|
||||
@@ -232,6 +232,7 @@ type SessionRepository interface {
|
||||
SetSessionACPSessionID(ctx context.Context, sessionID, acpSessionID string) (bool, error)
|
||||
DismissLastAgentError(ctx context.Context, sessionID string, expected models.LastAgentError, dismissedAt time.Time) (bool, error)
|
||||
GetLastAgentMessage(ctx context.Context, sessionID string) (string, error)
|
||||
UpdateTaskSessionLastReadMessageID(ctx context.Context, id, messageID string) error
|
||||
}
|
||||
|
||||
// SessionWorktreeRepository handles the task session↔worktree association.
|
||||
|
||||
@@ -193,6 +193,12 @@ func (r *Repository) runMigrations() error {
|
||||
// repo hasn't run yet.
|
||||
r.ensureRunnerProjectionTables()
|
||||
|
||||
// Slack-style unread divider: the read cursor a session advances to the
|
||||
// latest message id whenever it becomes the visible chat panel. The
|
||||
// frontend snapshots the prior value before the advance to position the
|
||||
// "New" divider (see models.TaskSession.LastReadMessageID).
|
||||
r.migrate.Apply("task_sessions.last_read_message_id", `ALTER TABLE task_sessions ADD COLUMN last_read_message_id TEXT DEFAULT ''`)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -110,6 +110,72 @@ func TestPostgresImproveKandevWorkflowIndexMigration(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestPostgresUpdateTaskSessionLastReadMessageIDMonotonic is the Postgres
|
||||
// counterpart to TestUpdateTaskSessionLastReadMessageID (SQLite): the
|
||||
// mark-read cursor guard used to compare SQLite's rowid pseudo-column, which
|
||||
// does not exist on Postgres and would fail this query outright. It now
|
||||
// compares (created_at, id), which is portable — this test proves the
|
||||
// forward-advance and stale-rejection behavior against a real Postgres
|
||||
// backend, not just SQLite. Skips unless KANDEV_TEST_POSTGRES_DSN is set.
|
||||
func TestPostgresUpdateTaskSessionLastReadMessageIDMonotonic(t *testing.T) {
|
||||
db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t))
|
||||
repo, err := NewWithDB(db, db, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("init postgres schema: %v", err)
|
||||
}
|
||||
ctx := context.Background()
|
||||
|
||||
// seedForMsgTest uses SQLite's `INSERT OR IGNORE`, which Postgres
|
||||
// rejects outright — seed directly with plain INSERTs instead (safe
|
||||
// here since each test gets a freshly created, empty isolated schema).
|
||||
seedNow := time.Now().UTC()
|
||||
if _, err := db.Exec(db.Rebind(`
|
||||
INSERT INTO tasks (id, workspace_id, title, created_at, updated_at)
|
||||
VALUES (?, '', 'test task', ?, ?)
|
||||
`), "task-pg-read", seedNow, seedNow); err != nil {
|
||||
t.Fatalf("seed task: %v", err)
|
||||
}
|
||||
if err := repo.CreateTaskSession(ctx, &models.TaskSession{
|
||||
ID: "session-pg-read", TaskID: "task-pg-read", State: models.TaskSessionStateWaitingForInput,
|
||||
}); err != nil {
|
||||
t.Fatalf("CreateTaskSession: %v", err)
|
||||
}
|
||||
if _, err := db.Exec(db.Rebind(`
|
||||
INSERT INTO task_session_turns (id, task_session_id, task_id, started_at, created_at, updated_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?)
|
||||
`), "turn-pg-read", "session-pg-read", "task-pg-read", seedNow, seedNow, seedNow); err != nil {
|
||||
t.Fatalf("seed turn: %v", err)
|
||||
}
|
||||
|
||||
now := time.Now().UTC()
|
||||
insertAgentMsg(t, repo, "msg-pg-1", "session-pg-read", "turn-pg-read", "user", "hi", now)
|
||||
insertAgentMsg(t, repo, "msg-pg-2", "session-pg-read", "turn-pg-read", "agent", "hello", now.Add(time.Second))
|
||||
|
||||
if err := repo.UpdateTaskSessionLastReadMessageID(ctx, "session-pg-read", "msg-pg-2"); err != nil {
|
||||
t.Fatalf("UpdateTaskSessionLastReadMessageID advance on postgres: %v", err)
|
||||
}
|
||||
session, err := repo.GetTaskSession(ctx, "session-pg-read")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "msg-pg-2" {
|
||||
t.Fatalf("session.LastReadMessageID = %q, want %q", session.LastReadMessageID, "msg-pg-2")
|
||||
}
|
||||
|
||||
// A delayed/retried request for the older message must not regress the
|
||||
// cursor on Postgres either — silent no-op, not an error.
|
||||
if err := repo.UpdateTaskSessionLastReadMessageID(ctx, "session-pg-read", "msg-pg-1"); err != nil {
|
||||
t.Fatalf("UpdateTaskSessionLastReadMessageID stale update on postgres: %v", err)
|
||||
}
|
||||
session, err = repo.GetTaskSession(ctx, "session-pg-read")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession after stale update: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "msg-pg-2" {
|
||||
t.Fatalf("session.LastReadMessageID = %q, want unchanged %q after stale update", session.LastReadMessageID, "msg-pg-2")
|
||||
}
|
||||
}
|
||||
|
||||
func TestPostgresExecutionProfileMigration(t *testing.T) {
|
||||
db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t))
|
||||
repo, err := NewWithDB(db, db, nil)
|
||||
|
||||
@@ -198,7 +198,7 @@ const taskSessionSelectCols = `ts.id, ts.task_id,
|
||||
ts.repository_id, ts.base_branch, ts.base_commit_sha, ts.workspace_path,
|
||||
ts.agent_profile_snapshot, ts.executor_snapshot, ts.environment_snapshot, ts.repository_snapshot,
|
||||
ts.state, ts.error_message, ts.metadata, ts.started_at, ts.completed_at, ts.updated_at,
|
||||
ts.is_primary, ts.review_status, ts.is_passthrough, ts.task_environment_id, ts.name`
|
||||
ts.is_primary, ts.review_status, ts.is_passthrough, ts.task_environment_id, ts.name, ts.last_read_message_id`
|
||||
|
||||
// taskSessionFromClause is the FROM clause that pairs with taskSessionSelectCols.
|
||||
// Always reference task_sessions as `ts` and executors_running as `er` in WHERE/ORDER.
|
||||
@@ -307,6 +307,7 @@ func (r *Repository) scanTaskSession(ctx context.Context, row *sql.Row, noRowsEr
|
||||
// via NullString so the empty case maps to "" on the model.
|
||||
var agentProfileID sql.NullString
|
||||
var name sql.NullString
|
||||
var lastReadMessageID sql.NullString
|
||||
|
||||
err := row.Scan(
|
||||
&session.ID, &session.TaskID, &session.AgentExecutionID, &session.ContainerID, &agentProfileID,
|
||||
@@ -314,7 +315,7 @@ func (r *Repository) scanTaskSession(ctx context.Context, row *sql.Row, noRowsEr
|
||||
&session.RepositoryID, &session.BaseBranch, &session.BaseCommitSHA, &session.WorkspacePath,
|
||||
&agentProfileSnapshotJSON, &executorSnapshotJSON, &environmentSnapshotJSON, &repositorySnapshotJSON,
|
||||
&state, &session.ErrorMessage, &metadataJSON, &session.StartedAt, &completedAt, &session.UpdatedAt,
|
||||
&isPrimary, &reviewStatus, &isPassthrough, &session.TaskEnvironmentID, &name,
|
||||
&isPrimary, &reviewStatus, &isPassthrough, &session.TaskEnvironmentID, &name, &lastReadMessageID,
|
||||
)
|
||||
|
||||
if err == sql.ErrNoRows {
|
||||
@@ -336,6 +337,9 @@ func (r *Repository) scanTaskSession(ctx context.Context, row *sql.Row, noRowsEr
|
||||
if name.Valid {
|
||||
session.Name = name.String
|
||||
}
|
||||
if lastReadMessageID.Valid {
|
||||
session.LastReadMessageID = lastReadMessageID.String
|
||||
}
|
||||
if completedAt.Valid {
|
||||
session.CompletedAt = &completedAt.Time
|
||||
}
|
||||
@@ -1264,6 +1268,66 @@ func (r *Repository) UpdateTaskSessionBaseCommit(ctx context.Context, id string,
|
||||
return nil
|
||||
}
|
||||
|
||||
// UpdateTaskSessionLastReadMessageID persists messageID as the session's
|
||||
// read cursor — deliberately a narrow single-column write (like
|
||||
// RenameTaskSession/UpdateTaskSessionBaseCommit) so it never collides with a
|
||||
// concurrent metadata or full-row write, and vice versa.
|
||||
//
|
||||
// The write is monotonic: it only advances the cursor when messageID is not
|
||||
// older than the currently persisted one, comparing each message's
|
||||
// (created_at, id) — created_at as the primary ordering (matches every
|
||||
// other transcript ordering in this codebase, e.g. ListMessages' `ORDER BY
|
||||
// created_at ASC`), with id as a deterministic tiebreaker for messages
|
||||
// created in the same instant. Portable across SQLite and Postgres — no
|
||||
// dialect branching needed, unlike the rowid-based approach this replaced
|
||||
// (SQLite's rowid pseudo-column doesn't exist on Postgres). This guards
|
||||
// against out-of-order delivery — e.g. two overlapping mark-read requests
|
||||
// where the response for an older message is processed after a newer one —
|
||||
// silently regressing last_read_message_id and resurrecting the "New"
|
||||
// divider over transcript the user already read. A cursor referencing a
|
||||
// message that no longer exists (deleted) is treated as having no rank, so
|
||||
// the guard never wedges: NOT EXISTS is satisfied and the update proceeds.
|
||||
func (r *Repository) UpdateTaskSessionLastReadMessageID(ctx context.Context, id, messageID string) error {
|
||||
now := time.Now().UTC()
|
||||
result, err := r.db.ExecContext(ctx, r.db.Rebind(`
|
||||
UPDATE task_sessions
|
||||
SET last_read_message_id = ?, updated_at = ?
|
||||
WHERE id = ?
|
||||
AND (
|
||||
last_read_message_id IS NULL OR last_read_message_id = ''
|
||||
OR NOT EXISTS (
|
||||
SELECT 1 FROM task_session_messages cur, task_session_messages incoming
|
||||
WHERE cur.id = task_sessions.last_read_message_id
|
||||
AND incoming.id = ?
|
||||
AND (
|
||||
cur.created_at > incoming.created_at
|
||||
OR (cur.created_at = incoming.created_at AND cur.id > incoming.id)
|
||||
)
|
||||
)
|
||||
)
|
||||
`), messageID, now, id, messageID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
rows, _ := result.RowsAffected()
|
||||
if rows > 0 {
|
||||
return nil
|
||||
}
|
||||
// No row was updated — either the session doesn't exist, or the guard
|
||||
// above correctly rejected a stale/out-of-order cursor (messageID is not
|
||||
// newer than what's already persisted). Distinguish the two so a stale
|
||||
// update is a silent no-op rather than a spurious not-found error.
|
||||
var exists int
|
||||
err = r.db.QueryRowContext(ctx, r.db.Rebind(`SELECT 1 FROM task_sessions WHERE id = ?`), id).Scan(&exists)
|
||||
if err != nil {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return fmt.Errorf("%w: agent session not found: %s", models.ErrTaskSessionNotFound, id)
|
||||
}
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ResetTaskSessionBasesForRepository rewrites base_branch and clears
|
||||
// base_commit_sha on every task_session belonging to (taskID, repositoryID).
|
||||
// Used by service.UpdateRepositoryBaseBranch after the changes-panel
|
||||
@@ -1584,6 +1648,7 @@ func scanTaskSessionRow(rows *sql.Rows) (*models.TaskSession, error) {
|
||||
var reviewStatus sql.NullString
|
||||
var agentProfileID sql.NullString
|
||||
var name sql.NullString
|
||||
var lastReadMessageID sql.NullString
|
||||
|
||||
err := rows.Scan(
|
||||
&session.ID, &session.TaskID, &session.AgentExecutionID, &session.ContainerID, &agentProfileID,
|
||||
@@ -1591,7 +1656,7 @@ func scanTaskSessionRow(rows *sql.Rows) (*models.TaskSession, error) {
|
||||
&session.RepositoryID, &session.BaseBranch, &session.BaseCommitSHA, &session.WorkspacePath,
|
||||
&agentProfileSnapshotJSON, &executorSnapshotJSON, &environmentSnapshotJSON, &repositorySnapshotJSON,
|
||||
&state, &session.ErrorMessage, &metadataJSON, &session.StartedAt, &completedAt, &session.UpdatedAt,
|
||||
&isPrimary, &reviewStatus, &isPassthrough, &session.TaskEnvironmentID, &name,
|
||||
&isPrimary, &reviewStatus, &isPassthrough, &session.TaskEnvironmentID, &name, &lastReadMessageID,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -1609,6 +1674,9 @@ func scanTaskSessionRow(rows *sql.Rows) (*models.TaskSession, error) {
|
||||
if name.Valid {
|
||||
session.Name = name.String
|
||||
}
|
||||
if lastReadMessageID.Valid {
|
||||
session.LastReadMessageID = lastReadMessageID.String
|
||||
}
|
||||
if completedAt.Valid {
|
||||
session.CompletedAt = &completedAt.Time
|
||||
}
|
||||
|
||||
@@ -141,6 +141,123 @@ func TestRenameTaskSession(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestUpdateTaskSessionLastReadMessageID verifies the read-cursor setter
|
||||
// rejects a missing session, persists the message id (round-tripping through
|
||||
// both GetTaskSession and ListTaskSessions, which scan via the single-row and
|
||||
// multi-row helpers respectively), that ToAPI omits an empty cursor, and that
|
||||
// the cursor never regresses when a stale/out-of-order messageID is applied
|
||||
// after a newer one already landed.
|
||||
func TestUpdateTaskSessionLastReadMessageID(t *testing.T) {
|
||||
repo := newRepoForSessionTests(t)
|
||||
ctx := context.Background()
|
||||
|
||||
if err := repo.UpdateTaskSessionLastReadMessageID(ctx, "missing-session", "msg-1"); !errors.Is(err, models.ErrTaskSessionNotFound) {
|
||||
t.Fatalf("UpdateTaskSessionLastReadMessageID error = %v, want ErrTaskSessionNotFound", err)
|
||||
}
|
||||
|
||||
seedForMsgTest(t, repo, "task-read", "session-read", "turn-read")
|
||||
now := time.Now().UTC()
|
||||
insertAgentMsg(t, repo, "msg-1", "session-read", "turn-read", "user", "hi", now)
|
||||
insertAgentMsg(t, repo, "msg-2", "session-read", "turn-read", "agent", "hello", now.Add(time.Second))
|
||||
|
||||
session, err := repo.GetTaskSession(ctx, "session-read")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession before mark-read: %v", err)
|
||||
}
|
||||
if _, ok := session.ToAPI()["last_read_message_id"]; ok {
|
||||
t.Fatalf("ToAPI() should omit last_read_message_id when empty")
|
||||
}
|
||||
|
||||
if err := repo.UpdateTaskSessionLastReadMessageID(ctx, "session-read", "msg-1"); err != nil {
|
||||
t.Fatalf("UpdateTaskSessionLastReadMessageID: %v", err)
|
||||
}
|
||||
session, err = repo.GetTaskSession(ctx, "session-read")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession after mark-read: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "msg-1" {
|
||||
t.Fatalf("session.LastReadMessageID = %q, want %q", session.LastReadMessageID, "msg-1")
|
||||
}
|
||||
if got := session.ToAPI()["last_read_message_id"]; got != "msg-1" {
|
||||
t.Fatalf(`ToAPI()["last_read_message_id"] = %v, want "msg-1"`, got)
|
||||
}
|
||||
|
||||
// Round-trips through the multi-row scan path (ListTaskSessions) too.
|
||||
sessions, err := repo.ListTaskSessions(ctx, "task-read")
|
||||
if err != nil {
|
||||
t.Fatalf("ListTaskSessions: %v", err)
|
||||
}
|
||||
if len(sessions) != 1 || sessions[0].LastReadMessageID != "msg-1" {
|
||||
t.Fatalf("ListTaskSessions did not carry LastReadMessageID: %#v", sessions)
|
||||
}
|
||||
|
||||
// Advancing to a newer message overwrites the cursor.
|
||||
if err := repo.UpdateTaskSessionLastReadMessageID(ctx, "session-read", "msg-2"); err != nil {
|
||||
t.Fatalf("UpdateTaskSessionLastReadMessageID advance: %v", err)
|
||||
}
|
||||
session, err = repo.GetTaskSession(ctx, "session-read")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession after advance: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "msg-2" {
|
||||
t.Fatalf("session.LastReadMessageID = %q, want %q", session.LastReadMessageID, "msg-2")
|
||||
}
|
||||
|
||||
// A delayed/retried request for the older message (out-of-order arrival)
|
||||
// must not regress the cursor — it's a silent no-op, not an error.
|
||||
if err := repo.UpdateTaskSessionLastReadMessageID(ctx, "session-read", "msg-1"); err != nil {
|
||||
t.Fatalf("UpdateTaskSessionLastReadMessageID stale update: %v", err)
|
||||
}
|
||||
session, err = repo.GetTaskSession(ctx, "session-read")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession after stale update: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "msg-2" {
|
||||
t.Fatalf("session.LastReadMessageID = %q, want unchanged %q after stale update", session.LastReadMessageID, "msg-2")
|
||||
}
|
||||
}
|
||||
|
||||
// TestUpdateTaskSessionLastReadMessageIDTiebreaksEqualTimestampsByID verifies
|
||||
// the monotonic guard's deterministic tiebreaker: when two messages share the
|
||||
// exact same created_at (e.g. a burst of messages persisted in the same
|
||||
// instant), ordering falls back to comparing message id, and a "stale" id in
|
||||
// that ordering is still rejected rather than silently accepted just because
|
||||
// the timestamps tie.
|
||||
func TestUpdateTaskSessionLastReadMessageIDTiebreaksEqualTimestampsByID(t *testing.T) {
|
||||
repo := newRepoForSessionTests(t)
|
||||
ctx := context.Background()
|
||||
|
||||
seedForMsgTest(t, repo, "task-tie", "session-tie", "turn-tie")
|
||||
sameInstant := time.Now().UTC()
|
||||
insertAgentMsg(t, repo, "msg-a", "session-tie", "turn-tie", "user", "a", sameInstant)
|
||||
insertAgentMsg(t, repo, "msg-b", "session-tie", "turn-tie", "agent", "b", sameInstant)
|
||||
|
||||
// "msg-b" > "msg-a" lexically, so it's the tiebreak winner at this
|
||||
// shared timestamp — advancing to it must succeed.
|
||||
if err := repo.UpdateTaskSessionLastReadMessageID(ctx, "session-tie", "msg-b"); err != nil {
|
||||
t.Fatalf("UpdateTaskSessionLastReadMessageID to tiebreak winner: %v", err)
|
||||
}
|
||||
|
||||
// Falling back to the tiebreak loser at the same timestamp must be
|
||||
// rejected as stale, exactly like a strictly-older timestamp would be.
|
||||
if err := repo.UpdateTaskSessionLastReadMessageID(ctx, "session-tie", "msg-a"); err != nil {
|
||||
t.Fatalf("UpdateTaskSessionLastReadMessageID stale tiebreak: %v", err)
|
||||
}
|
||||
session, err := repo.GetTaskSession(ctx, "session-tie")
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "msg-b" {
|
||||
t.Fatalf("session.LastReadMessageID = %q, want unchanged %q after stale tiebreak", session.LastReadMessageID, "msg-b")
|
||||
}
|
||||
}
|
||||
|
||||
// TestTaskSessionNotFoundErrorsAreTyped verifies that GetTaskSession,
|
||||
// UpdateTaskSession, UpdateTaskSessionState, and UpdateTaskSessionBaseCommit
|
||||
// all return ErrTaskSessionNotFound for a missing session, that
|
||||
// GetTaskSessionByTaskAndAgent translates not-found to (nil, nil), and that
|
||||
// GetPrimarySessionByTaskID returns ErrNoPrimarySession, while operations on
|
||||
// an existing session succeed.
|
||||
func TestTaskSessionNotFoundErrorsAreTyped(t *testing.T) {
|
||||
repo := newRepoForSessionTests(t)
|
||||
ctx := context.Background()
|
||||
|
||||
@@ -2,6 +2,8 @@ package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"time"
|
||||
|
||||
@@ -10,6 +12,15 @@ import (
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
// ErrInvalidMarkSessionRead marks a MarkSessionRead failure as caused by bad
|
||||
// caller input (empty ids, an unknown message, a message belonging to a
|
||||
// different session) rather than an unexpected internal failure. The HTTP
|
||||
// handler uses errors.Is against this to decide 400 vs a sanitized 500 —
|
||||
// without it, a genuine database/repository error would otherwise be
|
||||
// indistinguishable from a validation failure and leak as a 400 with the
|
||||
// raw internal error text.
|
||||
var ErrInvalidMarkSessionRead = errors.New("invalid mark-session-read request")
|
||||
|
||||
// SessionReadyPollInterval is how often WaitForSessionReady polls session state.
|
||||
const SessionReadyPollInterval = 1 * time.Second
|
||||
|
||||
@@ -117,6 +128,37 @@ func (s *Service) DismissLastAgentError(ctx context.Context, sessionID, stamp st
|
||||
return s.sessions.GetTaskSession(ctx, sessionID)
|
||||
}
|
||||
|
||||
// MarkSessionRead advances sessionID's Slack-style read cursor to messageID.
|
||||
// messageID must belong to sessionID — a stale id, a synthetic frontend-only
|
||||
// row (e.g. the task-description placeholder), or a cross-session id is
|
||||
// rejected rather than persisted, since a wrong cursor would hide genuinely
|
||||
// unread messages behind a mispositioned "New" divider. Every rejection
|
||||
// caused by bad caller input (as opposed to an unexpected repository/DB
|
||||
// failure) wraps ErrInvalidMarkSessionRead so the HTTP handler can return
|
||||
// 400 for the former and a sanitized, logged 500 for the latter.
|
||||
func (s *Service) MarkSessionRead(ctx context.Context, sessionID, messageID string) (*models.TaskSession, error) {
|
||||
if sessionID == "" {
|
||||
return nil, fmt.Errorf("%w: session_id is required", ErrInvalidMarkSessionRead)
|
||||
}
|
||||
if messageID == "" {
|
||||
return nil, fmt.Errorf("%w: message_id is required", ErrInvalidMarkSessionRead)
|
||||
}
|
||||
message, err := s.messages.GetMessage(ctx, messageID)
|
||||
if err != nil {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return nil, fmt.Errorf("%w: message %s not found", ErrInvalidMarkSessionRead, messageID)
|
||||
}
|
||||
return nil, fmt.Errorf("failed to look up message %s: %w", messageID, err)
|
||||
}
|
||||
if message.TaskSessionID != sessionID {
|
||||
return nil, fmt.Errorf("%w: message %s does not belong to session %s", ErrInvalidMarkSessionRead, messageID, sessionID)
|
||||
}
|
||||
if err := s.sessions.UpdateTaskSessionLastReadMessageID(ctx, sessionID, messageID); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return s.sessions.GetTaskSession(ctx, sessionID)
|
||||
}
|
||||
|
||||
// GetPrimarySession returns the primary session for a task.
|
||||
func (s *Service) GetPrimarySession(ctx context.Context, taskID string) (*models.TaskSession, error) {
|
||||
if err := s.authorizeTaskID(ctx, taskID); err != nil {
|
||||
|
||||
@@ -2984,6 +2984,129 @@ func setupTestTurn(t *testing.T, repo *sqliterepo.Repository, sessionID, taskID,
|
||||
return turn.ID
|
||||
}
|
||||
|
||||
// TestService_MarkSessionRead verifies MarkSessionRead advances the session's
|
||||
// read cursor when messageID is a real message belonging to sessionID, and
|
||||
// round-trips through GetTaskSession.
|
||||
func TestService_MarkSessionRead(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
setupTestTask(t, repo)
|
||||
sessionID := setupTestSession(t, repo)
|
||||
turnID := setupTestTurn(t, repo, sessionID, "task-123", "turn-123")
|
||||
if err := repo.CreateMessage(ctx, &models.Message{
|
||||
ID: "msg-1", TaskSessionID: sessionID, TaskID: "task-123", TurnID: turnID,
|
||||
AuthorType: models.MessageAuthorAgent, Content: "hello",
|
||||
}); err != nil {
|
||||
t.Fatalf("seed message: %v", err)
|
||||
}
|
||||
|
||||
session, err := svc.MarkSessionRead(ctx, sessionID, "msg-1")
|
||||
if err != nil {
|
||||
t.Fatalf("MarkSessionRead: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "msg-1" {
|
||||
t.Fatalf("session.LastReadMessageID = %q, want %q", session.LastReadMessageID, "msg-1")
|
||||
}
|
||||
|
||||
reloaded, err := svc.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
if reloaded.LastReadMessageID != "msg-1" {
|
||||
t.Fatalf("reloaded LastReadMessageID = %q, want %q", reloaded.LastReadMessageID, "msg-1")
|
||||
}
|
||||
}
|
||||
|
||||
// TestService_MarkSessionReadRejectsCrossSessionMessage verifies a message
|
||||
// belonging to a different session is rejected rather than silently
|
||||
// persisted as the cursor.
|
||||
func TestService_MarkSessionReadRejectsCrossSessionMessage(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
setupTestTask(t, repo)
|
||||
sessionID := setupTestSession(t, repo)
|
||||
if err := repo.CreateTaskSession(ctx, &models.TaskSession{ID: "session-other", TaskID: "task-123"}); err != nil {
|
||||
t.Fatalf("seed other session: %v", err)
|
||||
}
|
||||
turnID := setupTestTurn(t, repo, "session-other", "task-123", "turn-other")
|
||||
if err := repo.CreateMessage(ctx, &models.Message{
|
||||
ID: "msg-other", TaskSessionID: "session-other", TaskID: "task-123", TurnID: turnID,
|
||||
AuthorType: models.MessageAuthorAgent, Content: "from another session",
|
||||
}); err != nil {
|
||||
t.Fatalf("seed message: %v", err)
|
||||
}
|
||||
|
||||
_, markErr := svc.MarkSessionRead(ctx, sessionID, "msg-other")
|
||||
if !errors.Is(markErr, ErrInvalidMarkSessionRead) {
|
||||
t.Fatalf("MarkSessionRead error = %v, want ErrInvalidMarkSessionRead", markErr)
|
||||
}
|
||||
|
||||
session, err := svc.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "" {
|
||||
t.Fatalf("LastReadMessageID = %q, want unchanged empty cursor", session.LastReadMessageID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestService_MarkSessionReadRejectsUnknownMessage verifies a nonexistent
|
||||
// (e.g. frontend-synthetic) message id is rejected without touching the
|
||||
// session's cursor.
|
||||
func TestService_MarkSessionReadRejectsUnknownMessage(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
setupTestTask(t, repo)
|
||||
sessionID := setupTestSession(t, repo)
|
||||
|
||||
_, err := svc.MarkSessionRead(ctx, sessionID, "task-description")
|
||||
if !errors.Is(err, ErrInvalidMarkSessionRead) {
|
||||
t.Fatalf("MarkSessionRead error = %v, want ErrInvalidMarkSessionRead", err)
|
||||
}
|
||||
|
||||
session, err := svc.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "" {
|
||||
t.Fatalf("LastReadMessageID = %q, want unchanged empty cursor", session.LastReadMessageID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestService_MarkSessionReadRejectsEmptyIDs verifies MarkSessionRead
|
||||
// validates both ids up front, before touching the repository, and leaves
|
||||
// no cursor mutation behind.
|
||||
func TestService_MarkSessionReadRejectsEmptyIDs(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
setupTestTask(t, repo)
|
||||
sessionID := setupTestSession(t, repo)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
sessionID string
|
||||
messageID string
|
||||
}{
|
||||
{name: "empty session id", sessionID: "", messageID: "msg-1"},
|
||||
{name: "empty message id", sessionID: sessionID, messageID: ""},
|
||||
}
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if _, err := svc.MarkSessionRead(ctx, tc.sessionID, tc.messageID); err == nil {
|
||||
t.Fatal("expected MarkSessionRead to reject the request")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
session, err := svc.GetTaskSession(ctx, sessionID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetTaskSession: %v", err)
|
||||
}
|
||||
if session.LastReadMessageID != "" {
|
||||
t.Fatalf("LastReadMessageID = %q, want unchanged empty cursor", session.LastReadMessageID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestService_CreateMessage(t *testing.T) {
|
||||
svc, eventBus, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
|
||||
@@ -58,6 +58,7 @@ func (c *Controller) UpdateUserSettings(ctx context.Context, req dto.UpdateUserS
|
||||
ChatSubmitKey: req.ChatSubmitKey,
|
||||
ReviewAutoMarkOnScroll: req.ReviewAutoMarkOnScroll,
|
||||
ConfirmTaskArchive: req.ConfirmTaskArchive,
|
||||
UnreadDivider: req.UnreadDivider,
|
||||
MCPTaskAgentProfileDefault: req.MCPTaskAgentProfileDefault,
|
||||
ShowAnchoredPromptBar: req.ShowAnchoredPromptBar,
|
||||
ShowScrollToLastPrompt: req.ShowScrollToLastPrompt,
|
||||
|
||||
@@ -30,6 +30,7 @@ type UserSettingsDTO struct {
|
||||
ChatSubmitKey string `json:"chat_submit_key"`
|
||||
ReviewAutoMarkOnScroll bool `json:"review_auto_mark_on_scroll"`
|
||||
ConfirmTaskArchive bool `json:"confirm_task_archive"`
|
||||
UnreadDivider bool `json:"unread_divider"`
|
||||
MCPTaskAgentProfileDefault string `json:"mcp_task_agent_profile_default"`
|
||||
ShowAnchoredPromptBar bool `json:"show_anchored_prompt_bar"`
|
||||
ShowScrollToLastPrompt bool `json:"show_scroll_to_last_prompt"`
|
||||
@@ -94,6 +95,7 @@ type UpdateUserSettingsRequest struct {
|
||||
ChatSubmitKey *string `json:"chat_submit_key,omitempty"`
|
||||
ReviewAutoMarkOnScroll *bool `json:"review_auto_mark_on_scroll,omitempty"`
|
||||
ConfirmTaskArchive *bool `json:"confirm_task_archive,omitempty"`
|
||||
UnreadDivider *bool `json:"unread_divider,omitempty"`
|
||||
MCPTaskAgentProfileDefault *string `json:"mcp_task_agent_profile_default,omitempty"`
|
||||
ShowAnchoredPromptBar *bool `json:"show_anchored_prompt_bar,omitempty"`
|
||||
ShowScrollToLastPrompt *bool `json:"show_scroll_to_last_prompt,omitempty"`
|
||||
@@ -219,6 +221,7 @@ func FromUserSettings(settings *models.UserSettings) UserSettingsDTO {
|
||||
ChatSubmitKey: settings.ChatSubmitKey,
|
||||
ReviewAutoMarkOnScroll: settings.ReviewAutoMarkOnScroll,
|
||||
ConfirmTaskArchive: settings.ConfirmTaskArchive,
|
||||
UnreadDivider: settings.UnreadDivider,
|
||||
MCPTaskAgentProfileDefault: models.NormalizeMCPTaskAgentProfileDefault(settings.MCPTaskAgentProfileDefault),
|
||||
ShowAnchoredPromptBar: settings.ShowAnchoredPromptBar,
|
||||
ShowScrollToLastPrompt: settings.ShowScrollToLastPrompt,
|
||||
|
||||
@@ -58,6 +58,7 @@ type UserSettings struct {
|
||||
ChatSubmitKey string `json:"chat_submit_key"` // "enter" | "cmd_enter"
|
||||
ReviewAutoMarkOnScroll bool `json:"review_auto_mark_on_scroll"`
|
||||
ConfirmTaskArchive bool `json:"confirm_task_archive"`
|
||||
UnreadDivider bool `json:"unread_divider"`
|
||||
MCPTaskAgentProfileDefault string `json:"mcp_task_agent_profile_default"`
|
||||
ShowAnchoredPromptBar bool `json:"show_anchored_prompt_bar"` // desktop-only sticky last-prompt bar
|
||||
ShowScrollToLastPrompt bool `json:"show_scroll_to_last_prompt"`
|
||||
|
||||
@@ -49,6 +49,7 @@ type UpdateUserSettingsRequest struct {
|
||||
ChatSubmitKey *string
|
||||
ReviewAutoMarkOnScroll *bool
|
||||
ConfirmTaskArchive *bool
|
||||
UnreadDivider *bool
|
||||
MCPTaskAgentProfileDefault *string
|
||||
ShowAnchoredPromptBar *bool
|
||||
ShowScrollToLastPrompt *bool
|
||||
@@ -263,6 +264,9 @@ func applyTaskActionPreferences(settings *models.UserSettings, req *UpdateUserSe
|
||||
if req.ConfirmTaskArchive != nil {
|
||||
settings.ConfirmTaskArchive = *req.ConfirmTaskArchive
|
||||
}
|
||||
if req.UnreadDivider != nil {
|
||||
settings.UnreadDivider = *req.UnreadDivider
|
||||
}
|
||||
if err := applyMCPTaskAgentProfileDefault(settings, req.MCPTaskAgentProfileDefault); err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -656,6 +660,7 @@ func (s *Service) publishUserSettingsEvent(ctx context.Context, settings *models
|
||||
"chat_submit_key": settings.ChatSubmitKey,
|
||||
"review_auto_mark_on_scroll": settings.ReviewAutoMarkOnScroll,
|
||||
"confirm_task_archive": settings.ConfirmTaskArchive,
|
||||
"unread_divider": settings.UnreadDivider,
|
||||
"mcp_task_agent_profile_default": models.NormalizeMCPTaskAgentProfileDefault(settings.MCPTaskAgentProfileDefault),
|
||||
"show_anchored_prompt_bar": settings.ShowAnchoredPromptBar,
|
||||
"show_scroll_to_last_prompt": settings.ShowScrollToLastPrompt,
|
||||
|
||||
@@ -456,6 +456,7 @@ func marshalUserSettingsPayload(settings *models.UserSettings) ([]byte, error) {
|
||||
"chat_submit_key": settings.ChatSubmitKey,
|
||||
"review_auto_mark_on_scroll": settings.ReviewAutoMarkOnScroll,
|
||||
"confirm_task_archive": settings.ConfirmTaskArchive,
|
||||
"unread_divider": settings.UnreadDivider,
|
||||
"mcp_task_agent_profile_default": models.NormalizeMCPTaskAgentProfileDefault(settings.MCPTaskAgentProfileDefault),
|
||||
"show_anchored_prompt_bar": settings.ShowAnchoredPromptBar,
|
||||
"show_scroll_to_last_prompt": settings.ShowScrollToLastPrompt,
|
||||
@@ -579,6 +580,7 @@ func scanUserSettings(scanner interface{ Scan(dest ...any) error }, userID strin
|
||||
settings.ShowReleaseNotification = true
|
||||
settings.ReviewAutoMarkOnScroll = true
|
||||
settings.ConfirmTaskArchive = true
|
||||
settings.UnreadDivider = true
|
||||
settings.MCPTaskAgentProfileDefault = models.MCPTaskAgentProfileDefaultCurrentTask
|
||||
settings.ShowAnchoredPromptBar = false
|
||||
settings.ShowScrollToLastPrompt = true
|
||||
@@ -609,6 +611,7 @@ func scanUserSettings(scanner interface{ Scan(dest ...any) error }, userID strin
|
||||
ChatSubmitKey string `json:"chat_submit_key"`
|
||||
ReviewAutoMarkOnScroll *bool `json:"review_auto_mark_on_scroll"`
|
||||
ConfirmTaskArchive *bool `json:"confirm_task_archive"`
|
||||
UnreadDivider *bool `json:"unread_divider"`
|
||||
MCPTaskAgentProfileDefault string `json:"mcp_task_agent_profile_default"`
|
||||
ShowAnchoredPromptBar *bool `json:"show_anchored_prompt_bar"`
|
||||
ShowScrollToLastPrompt *bool `json:"show_scroll_to_last_prompt"`
|
||||
@@ -671,6 +674,11 @@ func scanUserSettings(scanner interface{ Scan(dest ...any) error }, userID strin
|
||||
} else {
|
||||
settings.ConfirmTaskArchive = true
|
||||
}
|
||||
if payload.UnreadDivider != nil {
|
||||
settings.UnreadDivider = *payload.UnreadDivider
|
||||
} else {
|
||||
settings.UnreadDivider = true
|
||||
}
|
||||
settings.MCPTaskAgentProfileDefault = models.NormalizeMCPTaskAgentProfileDefault(payload.MCPTaskAgentProfileDefault)
|
||||
if payload.ShowAnchoredPromptBar != nil {
|
||||
settings.ShowAnchoredPromptBar = *payload.ShowAnchoredPromptBar
|
||||
|
||||
@@ -92,6 +92,30 @@ func TestScanUserSettingsConfirmTaskArchiveDefault(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestScanUserSettingsUnreadDividerDefault(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
raw string
|
||||
want bool
|
||||
}{
|
||||
{name: "empty settings enable the divider", raw: `{}`, want: true},
|
||||
{name: "missing setting enables the divider", raw: `{"chat_submit_key":"enter"}`, want: true},
|
||||
{name: "explicit false disables the divider", raw: `{"unread_divider":false}`, want: false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
settings, err := scanUserSettings(settingsScanner{raw: tt.raw}, DefaultUserID)
|
||||
if err != nil {
|
||||
t.Fatalf("scan settings: %v", err)
|
||||
}
|
||||
if settings.UnreadDivider != tt.want {
|
||||
t.Fatalf("UnreadDivider = %v, want %v", settings.UnreadDivider, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestScanUserSettingsMCPTaskAgentProfileDefault(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
@@ -247,6 +247,7 @@ function mapEditorBehaviorFields(
|
||||
chatSubmitKey: s.chat_submit_key ?? "cmd_enter",
|
||||
reviewAutoMarkOnScroll: s.review_auto_mark_on_scroll ?? true,
|
||||
confirmTaskArchive: s.confirm_task_archive ?? true,
|
||||
unreadDivider: s.unread_divider ?? true,
|
||||
mcpTaskAgentProfileDefault: parseMCPTaskAgentProfileDefault(s.mcp_task_agent_profile_default),
|
||||
showAnchoredPromptBar: s.show_anchored_prompt_bar ?? false,
|
||||
showScrollToLastPrompt: s.show_scroll_to_last_prompt ?? true,
|
||||
|
||||
@@ -56,7 +56,7 @@ export const GENERAL_NAV_ITEMS: GeneralNavItem[] = [
|
||||
{
|
||||
href: "/settings/general/task-actions",
|
||||
label: "Task Actions",
|
||||
description: "MCP task defaults and archive safeguards",
|
||||
description: "MCP task defaults, archive safeguards, and transcript preferences",
|
||||
icon: IconArchive,
|
||||
},
|
||||
];
|
||||
|
||||
@@ -27,6 +27,7 @@ import type { Theme } from "@/lib/settings/types";
|
||||
import type { UserSettingsState } from "@/lib/state/slices/settings/types";
|
||||
import { ArchiveConfirmationSettings } from "@/components/settings/archive-confirmation-settings";
|
||||
import { MCPTaskAgentProfileDefaultSettings } from "@/components/settings/mcp-task-agent-profile-default-settings";
|
||||
import { UnreadDividerSettings } from "@/components/settings/unread-divider-settings";
|
||||
import { AnchoredPromptBarSettings } from "@/components/settings/anchored-prompt-bar-settings";
|
||||
import { useSettingsSaveContributor } from "@/components/settings/settings-save-provider";
|
||||
import type { StoredShortcutOverrides } from "@/lib/keyboard/shortcut-overrides";
|
||||
@@ -189,6 +190,7 @@ export function TaskActionsSettings() {
|
||||
<div className="space-y-4">
|
||||
<MCPTaskAgentProfileDefaultSettings />
|
||||
<ArchiveConfirmationSettings />
|
||||
<UnreadDividerSettings />
|
||||
</div>
|
||||
</SettingsSection>
|
||||
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||
import { StateProvider } from "@/components/state-provider";
|
||||
import { defaultState } from "@/lib/state/default-state";
|
||||
import { SettingsSaveProvider } from "./settings-save-provider";
|
||||
|
||||
const updateUserSettings = vi.fn();
|
||||
const DIVIDER_LABEL = "Show New divider in transcripts";
|
||||
|
||||
vi.mock("@/lib/api", () => ({
|
||||
updateUserSettings: (...args: unknown[]) => updateUserSettings(...args),
|
||||
}));
|
||||
|
||||
import { UnreadDividerSettings } from "./unread-divider-settings";
|
||||
|
||||
beforeEach(() => {
|
||||
updateUserSettings.mockReset().mockResolvedValue({ settings: {} });
|
||||
});
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
describe("UnreadDividerSettings", () => {
|
||||
it("keeps a disabled divider local until Save changes persists the preference", async () => {
|
||||
render(
|
||||
<StateProvider
|
||||
initialState={{ userSettings: { ...defaultState.userSettings, unreadDivider: true } }}
|
||||
>
|
||||
<SettingsSaveProvider>
|
||||
<UnreadDividerSettings />
|
||||
</SettingsSaveProvider>
|
||||
</StateProvider>,
|
||||
);
|
||||
const toggle = screen.getByRole("switch", { name: DIVIDER_LABEL });
|
||||
|
||||
expect(toggle.getAttribute("data-state")).toBe("checked");
|
||||
fireEvent.click(toggle);
|
||||
|
||||
expect(updateUserSettings).not.toHaveBeenCalled();
|
||||
expect(toggle.getAttribute("data-state")).toBe("unchecked");
|
||||
expect(
|
||||
screen.getByTestId("unread-divider-settings-card").getAttribute("data-settings-dirty"),
|
||||
).toBe("true");
|
||||
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Save changes" }));
|
||||
|
||||
await waitFor(() => expect(updateUserSettings).toHaveBeenCalledWith({ unread_divider: false }));
|
||||
await waitFor(() => expect(toggle.getAttribute("data-settings-dirty")).toBe("false"));
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,71 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { CardContent, CardHeader, CardTitle } from "@kandev/ui/card";
|
||||
import { Label } from "@kandev/ui/label";
|
||||
import { Switch } from "@kandev/ui/switch";
|
||||
import { useAppStore, useAppStoreApi } from "@/components/state-provider";
|
||||
import { updateUserSettings } from "@/lib/api";
|
||||
import { SettingsCard } from "./settings-card";
|
||||
import { useSettingsSaveContributor } from "./settings-save-provider";
|
||||
|
||||
/**
|
||||
* Edits the per-user preference that controls transcript unread dividers and
|
||||
* read-cursor updates. It participates in the shared Settings save/discard
|
||||
* lifecycle and commits the persisted preference to the app store on save.
|
||||
*/
|
||||
export function UnreadDividerSettings() {
|
||||
const unreadDivider = useAppStore((state) => state.userSettings.unreadDivider);
|
||||
const setUserSettings = useAppStore((state) => state.setUserSettings);
|
||||
const storeApi = useAppStoreApi();
|
||||
const [saved, setSaved] = useState(unreadDivider);
|
||||
const [draft, setDraft] = useState(unreadDivider);
|
||||
const draftRef = useRef(draft);
|
||||
draftRef.current = draft;
|
||||
const isDirty = draft !== saved;
|
||||
|
||||
useEffect(() => {
|
||||
setSaved((previous) => {
|
||||
if (draftRef.current === previous) setDraft(unreadDivider);
|
||||
return unreadDivider;
|
||||
});
|
||||
}, [unreadDivider]);
|
||||
|
||||
useSettingsSaveContributor({
|
||||
id: "general-unread-divider",
|
||||
revision: Number(draft),
|
||||
isDirty,
|
||||
save: async (revision) => {
|
||||
const submitted = Boolean(revision);
|
||||
await updateUserSettings({ unread_divider: submitted });
|
||||
setSaved(submitted);
|
||||
setUserSettings({ ...storeApi.getState().userSettings, unreadDivider: submitted });
|
||||
},
|
||||
discard: () => setDraft(saved),
|
||||
});
|
||||
|
||||
return (
|
||||
<SettingsCard isDirty={isDirty} data-testid="unread-divider-settings-card">
|
||||
<CardHeader>
|
||||
<CardTitle className="text-base">Unread Messages</CardTitle>
|
||||
</CardHeader>
|
||||
<CardContent>
|
||||
<div className="flex min-h-11 items-center justify-between gap-4">
|
||||
<div className="min-w-0 space-y-0.5">
|
||||
<Label htmlFor="show-unread-divider">Show New divider in transcripts</Label>
|
||||
<p className="text-xs text-muted-foreground">
|
||||
Mark messages that arrived while a task was out of view. Takes effect immediately.
|
||||
</p>
|
||||
</div>
|
||||
<Switch
|
||||
id="show-unread-divider"
|
||||
checked={draft}
|
||||
data-settings-dirty={isDirty}
|
||||
onCheckedChange={setDraft}
|
||||
className="shrink-0 cursor-pointer"
|
||||
/>
|
||||
</div>
|
||||
</CardContent>
|
||||
</SettingsCard>
|
||||
);
|
||||
}
|
||||
@@ -9,17 +9,20 @@ import { useSessionTurn } from "@/hooks/domains/session/use-session-turn";
|
||||
import { MessageListFooter } from "./message-list-footer";
|
||||
import { useNativeScrollManagement } from "./message-list-native-scroll";
|
||||
import { useTranscriptAutoScrollEnabled } from "./use-transcript-auto-scroll-enabled";
|
||||
import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import {
|
||||
type MessageListProps,
|
||||
type MessageListHandle,
|
||||
type LastPromptEdge,
|
||||
MessageListStatus,
|
||||
MessageItem,
|
||||
UnreadDivider,
|
||||
getItemKey,
|
||||
getConversationLoadingState,
|
||||
getEffectiveActiveTurnId,
|
||||
getLastTurnGroupId,
|
||||
getStreamingAgentMessageId,
|
||||
canReassertDividerScroll,
|
||||
resolveLastPromptEdge,
|
||||
isElementFullyVisible,
|
||||
} from "./message-list-shared";
|
||||
@@ -80,6 +83,7 @@ type MessageRowProps = {
|
||||
activeTurnId: string | null;
|
||||
streamingMessageId: string | null;
|
||||
onScrollToMessage: (messageId: string, options?: { align?: "start" | "center" }) => void;
|
||||
dividerBeforeItemKey?: string | null;
|
||||
};
|
||||
|
||||
/** One transcript row, keyed and DOM-id'd by `getItemKey` so the scroll
|
||||
@@ -96,10 +100,16 @@ function MessageRow({
|
||||
activeTurnId,
|
||||
streamingMessageId,
|
||||
onScrollToMessage,
|
||||
dividerBeforeItemKey,
|
||||
}: MessageRowProps) {
|
||||
const key = getItemKey(item);
|
||||
return (
|
||||
<div id={`msg-${key}`} className="pb-2" style={{ overflowAnchor: "none" }}>
|
||||
<div
|
||||
id={`msg-${key}`}
|
||||
className="pb-2 scroll-mt-[calc(4rem+env(safe-area-inset-top))] sm:scroll-mt-0"
|
||||
style={{ overflowAnchor: "none" }}
|
||||
>
|
||||
{dividerBeforeItemKey === key && <UnreadDivider />}
|
||||
<MessageItem
|
||||
item={item}
|
||||
sessionId={sessionId}
|
||||
@@ -141,8 +151,110 @@ type NativeMessageListBodyProps = {
|
||||
streamingMessageId: string | null;
|
||||
onScrollToMessage: (messageId: string, options?: { align?: "start" | "center" }) => void;
|
||||
autoScrollEnabled: boolean;
|
||||
dividerBeforeItemKey?: string | null;
|
||||
};
|
||||
|
||||
/**
|
||||
* Scroll to bottom on initial load — or, if this visit's Slack-style "New"
|
||||
* boundary lands on a currently-loaded item, straight to the divider
|
||||
* instead (mirrors Slack drawing the line where you left off rather than
|
||||
* always jumping to the newest message).
|
||||
*
|
||||
* - dividerBeforeItemKey is derived from usePanelActive (Dockview's
|
||||
* active-tab signal), backed by useSyncExternalStore, which only
|
||||
* resolves true on a render *after* this component's own mount — so on
|
||||
* the very first run here it's still null even for a session that does
|
||||
* have an unread divider.
|
||||
* - The initial messages fetch can itself arrive in more than one wave
|
||||
* (e.g. a WebSocket-delivered backfill continuing after this
|
||||
* component's first commit, unrelated to user-triggered pagination),
|
||||
* which can also retroactively shift where useScrollPositionOnPrepend
|
||||
* lands the scroll. Rather than trying to classify every wave as
|
||||
* "prepend" or "append" up front, the correction below keeps
|
||||
* re-asserting the divider's position on every relevant change, bounded
|
||||
* by BOTH of: the reader hasn't started scrolling yet (isUserScrolling
|
||||
* — wheel/touchstart/keydown, since a plain 'scroll' event can't tell
|
||||
* user intent apart from our own programmatic writes), AND still being
|
||||
* within a short settling window since mount (isWithinSettlingWindow).
|
||||
* The window exists so a live message arriving long after the visit has
|
||||
* genuinely settled — with no wheel/touch/key event to catch, e.g. a
|
||||
* scrollbar drag — can never re-trigger a correction; once either gate
|
||||
* trips, it's the user's scroll position to own, same as Slack never
|
||||
* re-snapping you to the unread line once you've started reading.
|
||||
* - didScrollToDivider and didInitialScroll are separate latches so the
|
||||
* bottom-fallback firing first (before dividerBeforeItemKey resolves)
|
||||
* doesn't block the divider correction from still applying once it
|
||||
* does. Embedded, always-invisible previews (isVisible hardcoded false,
|
||||
* see TaskChatPanel) never resolve a divider, so they keep the
|
||||
* original, unconditional scroll-to-bottom-on-mount behavior untouched.
|
||||
*/
|
||||
function useScrollToDividerOrBottom(
|
||||
scrollRef: React.RefObject<HTMLDivElement | null>,
|
||||
itemCount: number,
|
||||
dividerBeforeItemKey: string | null | undefined,
|
||||
) {
|
||||
const isUserScrollingRef = useRef(false);
|
||||
useEffect(() => {
|
||||
const el = scrollRef.current;
|
||||
if (!el) return;
|
||||
const markUserScrolling = () => {
|
||||
isUserScrollingRef.current = true;
|
||||
};
|
||||
el.addEventListener("wheel", markUserScrolling, { passive: true });
|
||||
el.addEventListener("touchstart", markUserScrolling, { passive: true });
|
||||
el.addEventListener("keydown", markUserScrolling);
|
||||
return () => {
|
||||
el.removeEventListener("wheel", markUserScrolling);
|
||||
el.removeEventListener("touchstart", markUserScrolling);
|
||||
el.removeEventListener("keydown", markUserScrolling);
|
||||
};
|
||||
}, [scrollRef]);
|
||||
|
||||
// Bounds how long the divider correction below can keep re-asserting
|
||||
// itself after mount, independent of user interaction: a scrollbar drag
|
||||
// (no wheel/touch/key event) or a live message arriving long after the
|
||||
// visit has settled must never be able to re-trigger it. 4s comfortably
|
||||
// covers the slowest observed multi-wave initial load (WS backfill
|
||||
// continuing after the REST fetch) without lingering into the range
|
||||
// where the user has plausibly started reading and scrolling normally.
|
||||
const mountedAtRef = useRef<number | null>(null);
|
||||
if (mountedAtRef.current === null) mountedAtRef.current = Date.now();
|
||||
const isWithinSettlingWindow = () => Date.now() - (mountedAtRef.current ?? 0) < 4000;
|
||||
|
||||
const didInitialScroll = useRef(false);
|
||||
const didScrollToDivider = useRef(false);
|
||||
useEffect(() => {
|
||||
const el = scrollRef.current;
|
||||
if (!el || itemCount === 0) return;
|
||||
const canReassertDivider = canReassertDividerScroll({
|
||||
hasDividerTarget: Boolean(dividerBeforeItemKey),
|
||||
didScrollToDivider: didScrollToDivider.current,
|
||||
isUserScrolling: isUserScrollingRef.current,
|
||||
isWithinSettlingWindow: isWithinSettlingWindow(),
|
||||
});
|
||||
if (canReassertDivider) {
|
||||
if (useDockviewStore.getState().pendingChatScrollTop === null) {
|
||||
const dividerEl = el.querySelector<HTMLElement>(`[id="msg-${dividerBeforeItemKey}"]`);
|
||||
if (dividerEl) {
|
||||
dividerEl.scrollIntoView({ block: "start" });
|
||||
didScrollToDivider.current = true;
|
||||
didInitialScroll.current = true;
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (didInitialScroll.current) return;
|
||||
// If a layout rebuild scroll restore is pending, skip initial scroll
|
||||
// (the restore handler will set the correct position)
|
||||
if (useDockviewStore.getState().pendingChatScrollTop !== null) {
|
||||
didInitialScroll.current = true;
|
||||
return;
|
||||
}
|
||||
el.scrollTop = el.scrollHeight;
|
||||
didInitialScroll.current = true;
|
||||
}, [itemCount, dividerBeforeItemKey]);
|
||||
}
|
||||
|
||||
/** Sentinel, status/footer, and transcript rows — everything below the
|
||||
* (optional) anchored prompt bar inside the scroll container. */
|
||||
function NativeMessageListBody({
|
||||
@@ -169,6 +281,7 @@ function NativeMessageListBody({
|
||||
streamingMessageId,
|
||||
onScrollToMessage,
|
||||
autoScrollEnabled,
|
||||
dividerBeforeItemKey,
|
||||
}: NativeMessageListBodyProps) {
|
||||
return (
|
||||
<div className="p-4">
|
||||
@@ -199,6 +312,7 @@ function NativeMessageListBody({
|
||||
activeTurnId={activeTurnId}
|
||||
streamingMessageId={streamingMessageId}
|
||||
onScrollToMessage={onScrollToMessage}
|
||||
dividerBeforeItemKey={dividerBeforeItemKey}
|
||||
/>
|
||||
))}
|
||||
|
||||
@@ -246,6 +360,7 @@ export const NativeMessageList = memo(
|
||||
firstMessageId,
|
||||
onFirstMessageHiddenChange,
|
||||
stickyPromptBar,
|
||||
dividerBeforeItemKey,
|
||||
}: MessageListProps,
|
||||
ref,
|
||||
) {
|
||||
@@ -274,6 +389,7 @@ export const NativeMessageList = memo(
|
||||
isLoadingMore,
|
||||
loadMore,
|
||||
});
|
||||
useScrollToDividerOrBottom(scrollRef, items.length, dividerBeforeItemKey);
|
||||
useImperativeHandle(ref, () => ({ scrollToMessage: handleScrollToMessage }), [
|
||||
handleScrollToMessage,
|
||||
]);
|
||||
@@ -317,6 +433,7 @@ export const NativeMessageList = memo(
|
||||
streamingMessageId={streamingMessageId}
|
||||
onScrollToMessage={handleScrollToMessage}
|
||||
autoScrollEnabled={autoScrollEnabled}
|
||||
dividerBeforeItemKey={dividerBeforeItemKey}
|
||||
/>
|
||||
</SessionPanelContent>
|
||||
);
|
||||
|
||||
@@ -74,10 +74,12 @@ vi.mock("@/lib/api/domains/session-api", () => ({
|
||||
import {
|
||||
MessageItem,
|
||||
MessageListStatus,
|
||||
UnreadDivider,
|
||||
getConversationLoadingState,
|
||||
getEffectiveActiveTurnId,
|
||||
getItemKey,
|
||||
getStreamingAgentMessageId,
|
||||
canReassertDividerScroll,
|
||||
getLastUserMessageId,
|
||||
getFirstUserMessageId,
|
||||
isElementFullyVisible,
|
||||
@@ -102,6 +104,66 @@ describe("getEffectiveActiveTurnId", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("canReassertDividerScroll", () => {
|
||||
it("never applies without a divider target", () => {
|
||||
expect(
|
||||
canReassertDividerScroll({
|
||||
hasDividerTarget: false,
|
||||
didScrollToDivider: false,
|
||||
isUserScrolling: false,
|
||||
isWithinSettlingWindow: true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("always applies the first time, regardless of the other gates", () => {
|
||||
expect(
|
||||
canReassertDividerScroll({
|
||||
hasDividerTarget: true,
|
||||
didScrollToDivider: false,
|
||||
isUserScrolling: true,
|
||||
isWithinSettlingWindow: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("re-asserts while still settling and the reader hasn't scrolled", () => {
|
||||
expect(
|
||||
canReassertDividerScroll({
|
||||
hasDividerTarget: true,
|
||||
didScrollToDivider: true,
|
||||
isUserScrolling: false,
|
||||
isWithinSettlingWindow: true,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("stops re-asserting once the reader has scrolled, even mid-settling-window", () => {
|
||||
expect(
|
||||
canReassertDividerScroll({
|
||||
hasDividerTarget: true,
|
||||
didScrollToDivider: true,
|
||||
isUserScrolling: true,
|
||||
isWithinSettlingWindow: true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("stops re-asserting once the settling window elapses, even without any interaction", () => {
|
||||
// Covers a scrollbar drag or a live message arriving long after the
|
||||
// visit settled — neither fires wheel/touchstart/keydown, so the
|
||||
// settling window is the only thing bounding this case.
|
||||
expect(
|
||||
canReassertDividerScroll({
|
||||
hasDividerTarget: true,
|
||||
didScrollToDivider: true,
|
||||
isUserScrolling: false,
|
||||
isWithinSettlingWindow: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
function row(onOpenFile: (p: string) => void) {
|
||||
return (
|
||||
<MessageItem
|
||||
@@ -578,3 +640,13 @@ describe("MessageListStatus", () => {
|
||||
expect(screen.queryByText("Loading conversation...")).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("UnreadDivider", () => {
|
||||
it("renders a labeled separator marking new messages", () => {
|
||||
render(<UnreadDivider />);
|
||||
|
||||
const divider = screen.getByTestId("unread-divider");
|
||||
expect(divider.getAttribute("role")).toBe("separator");
|
||||
expect(screen.getByText("New")).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -31,6 +31,9 @@ export type MessageListProps = {
|
||||
sessionState?: TaskSessionState;
|
||||
worktreePath?: string;
|
||||
onOpenFile?: (path: string) => void;
|
||||
/** Render item key (see getItemKey) the unread "New" divider should
|
||||
* appear immediately before; null/undefined renders no divider. */
|
||||
dividerBeforeItemKey?: string | null;
|
||||
/** Id of the most recent user-sent message, when known. Drives the
|
||||
* scroll-to-last-prompt button (always active) and the anchored-bar
|
||||
* affordance (opt-in, desktop only). */
|
||||
@@ -232,6 +235,34 @@ export function getConversationLoadingState(params: {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Decides whether message-list-native's initial divider-scroll effect may
|
||||
* (re-)apply `scrollIntoView` on the divider this render, versus leaving
|
||||
* whatever position the reader (or a prior correction) already settled on.
|
||||
*
|
||||
* True on the very first successful application (didScrollToDivider is
|
||||
* false) regardless of the other two gates — that first placement must
|
||||
* always happen once a divider target exists. After that, a re-assertion
|
||||
* (e.g. the transcript's initial data arriving in more than one wave, which
|
||||
* can shift where the divider actually lands) is only allowed while BOTH
|
||||
* the reader hasn't interacted yet (isUserScrolling — wheel, touch, or a
|
||||
* key press) AND the visit is still within its short settling window since
|
||||
* mount. Either gate tripping freezes the position for good: a live
|
||||
* message arriving well after the visit has genuinely settled — with no
|
||||
* interaction event to catch, e.g. a scrollbar drag — must never yank the
|
||||
* reader back to the divider.
|
||||
*/
|
||||
export function canReassertDividerScroll(params: {
|
||||
hasDividerTarget: boolean;
|
||||
didScrollToDivider: boolean;
|
||||
isUserScrolling: boolean;
|
||||
isWithinSettlingWindow: boolean;
|
||||
}): boolean {
|
||||
if (!params.hasDividerTarget) return false;
|
||||
if (!params.didScrollToDivider) return true;
|
||||
return !params.isUserScrolling && params.isWithinSettlingWindow;
|
||||
}
|
||||
|
||||
// The chat banner stays visible until the user explicitly dismisses it, even
|
||||
// after the agent resumes — so the user can read the full error message at
|
||||
// their own pace. The sidebar icon, by contrast, also auto-hides once the
|
||||
@@ -296,6 +327,27 @@ export function LastAgentErrorNotice({
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Slack-style unread-messages divider: a red rule with a "New" label,
|
||||
* positioned by the caller immediately before the first unread render item
|
||||
* (see hooks/use-processed-messages.ts's findUnreadDividerItemId).
|
||||
*/
|
||||
export function UnreadDivider() {
|
||||
return (
|
||||
<div
|
||||
data-testid="unread-divider"
|
||||
role="separator"
|
||||
aria-label="New messages"
|
||||
className="relative my-3 flex items-center"
|
||||
>
|
||||
<div className="h-px flex-1 bg-destructive" />
|
||||
<span className="ml-2 shrink-0 text-[10px] font-semibold uppercase tracking-wide text-destructive">
|
||||
New
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export function MessageListStatus({
|
||||
isLoadingMore,
|
||||
hasMore,
|
||||
|
||||
@@ -36,6 +36,7 @@ import {
|
||||
type LastPromptEdge,
|
||||
MessageListStatus,
|
||||
MessageItem,
|
||||
UnreadDivider,
|
||||
getItemKey,
|
||||
getConversationLoadingState,
|
||||
getEffectiveActiveTurnId,
|
||||
@@ -165,6 +166,7 @@ type RenderItemArgs = {
|
||||
activeTurnId: string | null;
|
||||
streamingMessageId: string | null;
|
||||
onScrollToMessage: (messageId: string, options?: { align?: "start" | "center" }) => void;
|
||||
dividerBeforeItemKey?: string | null;
|
||||
};
|
||||
|
||||
/** Renders one transcript row for a given (rebased) Virtuoso item index. */
|
||||
@@ -184,6 +186,7 @@ function useVirtuosoRenderItem(args: RenderItemArgs) {
|
||||
lastPromptMessageId,
|
||||
firstMessageId,
|
||||
onScrollToMessage,
|
||||
dividerBeforeItemKey,
|
||||
} = args;
|
||||
return useCallback(
|
||||
(index: number) => {
|
||||
@@ -205,6 +208,7 @@ function useVirtuosoRenderItem(args: RenderItemArgs) {
|
||||
data-last-prompt-row={isLastPromptRow || undefined}
|
||||
data-first-message-row={isFirstMessageRow || undefined}
|
||||
>
|
||||
{dividerBeforeItemKey === getItemKey(item) && <UnreadDivider />}
|
||||
<MessageItem
|
||||
item={item}
|
||||
sessionId={sessionId}
|
||||
@@ -236,10 +240,43 @@ function useVirtuosoRenderItem(args: RenderItemArgs) {
|
||||
lastPromptMessageId,
|
||||
firstMessageId,
|
||||
onScrollToMessage,
|
||||
dividerBeforeItemKey,
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Corrects the initial scroll position to this visit's Slack-style unread
|
||||
* divider (see findUnreadDividerItemId) once it resolves, mirroring Slack
|
||||
* drawing the line where you left off instead of always landing on the
|
||||
* newest message.
|
||||
*
|
||||
* Deliberately NOT done via Virtuoso's own `initialTopMostItemIndex` prop
|
||||
* (which only takes effect on Virtuoso's first render): dividerBeforeItemKey
|
||||
* is derived from usePanelActive (Dockview's active-tab signal), which is
|
||||
* backed by useSyncExternalStore and typically only resolves true one
|
||||
* render *after* this component's own mount — so on Virtuoso's actual first
|
||||
* render it's still null even for a session that does have an unread
|
||||
* divider, and a value frozen at that first render could never be
|
||||
* corrected. Scrolling here imperatively, gated on a change to
|
||||
* dividerBeforeItemKey rather than mount, catches it whenever it resolves.
|
||||
*/
|
||||
function useScrollToDividerOnceResolved(
|
||||
virtuosoRef: React.RefObject<VirtuosoHandle | null>,
|
||||
items: RenderItem[],
|
||||
firstItemIndex: number,
|
||||
dividerBeforeItemKey: string | null | undefined,
|
||||
) {
|
||||
const didScrollRef = useRef(false);
|
||||
useEffect(() => {
|
||||
if (didScrollRef.current || !dividerBeforeItemKey) return;
|
||||
const dividerIndex = items.findIndex((item) => getItemKey(item) === dividerBeforeItemKey);
|
||||
if (dividerIndex < 0) return;
|
||||
virtuosoRef.current?.scrollToIndex({ index: firstItemIndex + dividerIndex, align: "start" });
|
||||
didScrollRef.current = true;
|
||||
}, [virtuosoRef, items, firstItemIndex, dividerBeforeItemKey]);
|
||||
}
|
||||
|
||||
function useVirtuosoCallbacks(props: VirtuosoBodyProps) {
|
||||
const {
|
||||
items,
|
||||
@@ -249,12 +286,13 @@ function useVirtuosoCallbacks(props: VirtuosoBodyProps) {
|
||||
childrenByParentToolCallId,
|
||||
taskId,
|
||||
} = props;
|
||||
const { worktreePath, onOpenFile, lastTurnGroupId, activeTurnId } = props;
|
||||
const { worktreePath, onOpenFile, lastTurnGroupId, activeTurnId, dividerBeforeItemKey } = props;
|
||||
const { hasMore, isLoadingMore, loadMore } = props;
|
||||
const virtuosoRef = useRef<VirtuosoHandle>(null);
|
||||
const itemCount = items.length;
|
||||
const streamingMessageId = getStreamingAgentMessageId(messages);
|
||||
const firstItemIndex = useStableFirstItemIndex(items);
|
||||
useScrollToDividerOnceResolved(virtuosoRef, items, firstItemIndex, dividerBeforeItemKey);
|
||||
const { isLocked, runLocked } = useProgrammaticScrollLock(props.scrollParent);
|
||||
|
||||
const loadCooldownRef = useRef(false);
|
||||
@@ -330,6 +368,7 @@ function useVirtuosoCallbacks(props: VirtuosoBodyProps) {
|
||||
lastPromptMessageId: props.lastPromptMessageId,
|
||||
firstMessageId: props.firstMessageId,
|
||||
onScrollToMessage: handleScrollToMessage,
|
||||
dividerBeforeItemKey,
|
||||
});
|
||||
|
||||
return {
|
||||
|
||||
@@ -426,6 +426,7 @@ function useSessionData(
|
||||
const {
|
||||
messages,
|
||||
isLoading: messagesLoading,
|
||||
isInitialMessagesLoading,
|
||||
hasMore: hasOlderMessages,
|
||||
} = useSessionMessages(resolvedSessionId);
|
||||
const lastAgentError = useMemo(() => readLastAgentError(session?.metadata), [session?.metadata]);
|
||||
@@ -450,6 +451,7 @@ function useSessionData(
|
||||
return {
|
||||
messages,
|
||||
messagesLoading,
|
||||
isInitialMessagesLoading,
|
||||
...processed,
|
||||
sessionModel,
|
||||
activeModel,
|
||||
|
||||
@@ -0,0 +1,297 @@
|
||||
import { act, cleanup, renderHook, waitFor } from "@testing-library/react";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { TaskSession } from "@/lib/types/http";
|
||||
|
||||
const mockMarkSessionRead = vi.fn();
|
||||
const mockUpdateSessionReadCursor = vi.fn();
|
||||
|
||||
type MockState = {
|
||||
userSettings: { unreadDivider: boolean };
|
||||
taskSessions: { items: Record<string, TaskSession> };
|
||||
updateSessionReadCursor: typeof mockUpdateSessionReadCursor;
|
||||
};
|
||||
|
||||
let mockState: MockState;
|
||||
|
||||
function session(overrides: Partial<TaskSession> = {}): TaskSession {
|
||||
return {
|
||||
id: "session-1",
|
||||
task_id: "task-1",
|
||||
state: "RUNNING",
|
||||
started_at: "2026-06-27T00:00:00Z",
|
||||
updated_at: "2026-06-27T00:00:00Z",
|
||||
...overrides,
|
||||
} as TaskSession;
|
||||
}
|
||||
|
||||
const mockStoreApi = { getState: () => mockState };
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (state: MockState) => unknown) => selector(mockState),
|
||||
useAppStoreApi: () => mockStoreApi,
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api/domains/session-api", () => ({
|
||||
markSessionRead: (...args: unknown[]) => mockMarkSessionRead(...args),
|
||||
}));
|
||||
|
||||
import { useSessionReadTracking } from "./use-session-read-tracking";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockState = {
|
||||
userSettings: { unreadDivider: true },
|
||||
taskSessions: { items: {} },
|
||||
updateSessionReadCursor: mockUpdateSessionReadCursor,
|
||||
};
|
||||
mockMarkSessionRead.mockResolvedValue({ session_id: "session-1", last_read_message_id: "m2" });
|
||||
});
|
||||
|
||||
describe("useSessionReadTracking", () => {
|
||||
it("returns null while not visible and does not mark the session read", () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
const { result } = renderHook(() => useSessionReadTracking("session-1", false, "m2"));
|
||||
|
||||
expect(result.current).toBeNull();
|
||||
expect(mockMarkSessionRead).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns null and does not mark the session read when the user disables the divider", async () => {
|
||||
mockState.userSettings.unreadDivider = false;
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
const { result } = renderHook(() => useSessionReadTracking("session-1", true, "m2"));
|
||||
|
||||
expect(result.current).toBeNull();
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(mockMarkSessionRead).not.toHaveBeenCalled();
|
||||
});
|
||||
it("does not capture an anchor before the session record has loaded into the store, capturing correctly once it does", async () => {
|
||||
// Session absent entirely (still fetching) — must not treat this the
|
||||
// same as "loaded, and legitimately has no prior cursor" (see the
|
||||
// next test). Locking in a capture here would use undefined?.last_
|
||||
// read_message_id ?? null as the answer, when the real answer isn't
|
||||
// known yet.
|
||||
const { result, rerender } = renderHook(
|
||||
({ latest }: { latest: string | null }) => useSessionReadTracking("session-1", true, latest),
|
||||
{ initialProps: { latest: null as string | null } },
|
||||
);
|
||||
expect(result.current).toBeNull();
|
||||
expect(mockMarkSessionRead).not.toHaveBeenCalled();
|
||||
|
||||
// The session finishes loading with a real prior cursor already set
|
||||
// (e.g. a fetch that resolved after this hook's first render).
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
rerender({ latest: "m3" });
|
||||
|
||||
expect(result.current).toBe("m1");
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledWith("session-1", "m3"));
|
||||
});
|
||||
|
||||
it("does not dispatch mark-read while the session hasn't loaded yet, even if latestMessageId already resolved", async () => {
|
||||
// Reproduces a real mobile bug: unlike the dockview panel path (isVisible
|
||||
// lags behind usePanelActive's async resolution, giving the session
|
||||
// fetch a head start), mobile's isVisible is true from render one — so
|
||||
// if the *messages* list resolves before the *session metadata* fetch
|
||||
// does, latestMessageId can already be non-null while the session
|
||||
// record itself is still missing from the store. The mark-read
|
||||
// dispatch effect must not race ahead of the capture above and advance
|
||||
// the cursor before this visit's real "prior" boundary is ever read —
|
||||
// that would silently swallow the divider with no error.
|
||||
const { rerender } = renderHook(
|
||||
({ latest }: { latest: string | null }) => useSessionReadTracking("session-1", true, latest),
|
||||
{ initialProps: { latest: "m5" as string | null } },
|
||||
);
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(mockMarkSessionRead).not.toHaveBeenCalled();
|
||||
|
||||
// Session loads with the real prior cursor — the capture (during
|
||||
// render, before this commit's effects) must see this value, not
|
||||
// whatever a premature dispatch might have already advanced it to.
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
rerender({ latest: "m5" });
|
||||
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledWith("session-1", "m5"));
|
||||
});
|
||||
|
||||
it("captures null when the session has loaded but genuinely has no prior cursor (first-ever visit)", () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: undefined });
|
||||
const { result } = renderHook(() => useSessionReadTracking("session-1", true, "m1"));
|
||||
|
||||
expect(result.current).toBeNull();
|
||||
});
|
||||
|
||||
it("does not activate a divider when a new message arrives after opening at the cursor", () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
const { result, rerender } = renderHook(
|
||||
({ latest }: { latest: string }) => useSessionReadTracking("session-1", true, latest),
|
||||
{ initialProps: { latest: "m1" } },
|
||||
);
|
||||
|
||||
// Opening at the transcript tail has no unread boundary.
|
||||
expect(result.current).toBeNull();
|
||||
|
||||
// The user stays on this chat panel and sends a new prompt. It must not
|
||||
// turn the tail cursor captured at visit start into a "New" divider.
|
||||
rerender({ latest: "m2" });
|
||||
expect(result.current).toBeNull();
|
||||
});
|
||||
|
||||
it("waits for the initial message load before preserving a genuine unread boundary", () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
const trackRead = useSessionReadTracking as unknown as (
|
||||
sessionId: string | null,
|
||||
isVisible: boolean,
|
||||
latestMessageId: string | null,
|
||||
initialMessagesLoading: boolean,
|
||||
) => string | null;
|
||||
const { result, rerender } = renderHook(
|
||||
({
|
||||
latest,
|
||||
initialMessagesLoading,
|
||||
}: {
|
||||
latest: string | null;
|
||||
initialMessagesLoading: boolean;
|
||||
}) => trackRead("session-1", true, latest, initialMessagesLoading),
|
||||
{ initialProps: { latest: null as string | null, initialMessagesLoading: true } },
|
||||
);
|
||||
|
||||
expect(result.current).toBeNull();
|
||||
|
||||
// The first complete transcript includes messages that pre-date this visit.
|
||||
rerender({ latest: "m3", initialMessagesLoading: false });
|
||||
expect(result.current).toBe("m1");
|
||||
});
|
||||
|
||||
it("freezes the divider anchor at the cursor value from before this visit's advance", async () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
const { result } = renderHook(() => useSessionReadTracking("session-1", true, "m3"));
|
||||
|
||||
// Anchor is the value that was current the instant the session became
|
||||
// visible — "m1" — not wherever the cursor ends up after advancing.
|
||||
expect(result.current).toBe("m1");
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledWith("session-1", "m3"));
|
||||
});
|
||||
|
||||
it("advances the cursor as new messages arrive while still visible, without moving the divider", async () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
const { result, rerender } = renderHook(
|
||||
({ latest }: { latest: string }) => useSessionReadTracking("session-1", true, latest),
|
||||
{ initialProps: { latest: "m2" } },
|
||||
);
|
||||
expect(result.current).toBe("m1");
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledWith("session-1", "m2"));
|
||||
|
||||
// Simulate the store reflecting the server's response to the first call.
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m2" });
|
||||
rerender({ latest: "m4" });
|
||||
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledWith("session-1", "m4"));
|
||||
// Divider anchor is unchanged — still the value from when the visit started.
|
||||
expect(result.current).toBe("m1");
|
||||
});
|
||||
});
|
||||
|
||||
describe("useSessionReadTracking — mark-read dispatch", () => {
|
||||
it("does not call markSessionRead again once the cursor already matches the latest message", async () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m2" });
|
||||
renderHook(() => useSessionReadTracking("session-1", true, "m2"));
|
||||
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(mockMarkSessionRead).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("re-captures a fresh anchor after leaving and re-entering the session", async () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
const { result, rerender } = renderHook(
|
||||
({ visible, latest }: { visible: boolean; latest: string }) =>
|
||||
useSessionReadTracking("session-1", visible, latest),
|
||||
{ initialProps: { visible: true, latest: "m2" } },
|
||||
);
|
||||
expect(result.current).toBe("m1");
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledTimes(1));
|
||||
|
||||
// Leave: cursor is now advanced to m2 server-side. Advance the clock
|
||||
// past the hide-debounce (see use-session-read-tracking.ts) so the
|
||||
// reset that lets the next show re-capture actually commits — a real
|
||||
// navigate-away is far slower than the debounce window, and fake
|
||||
// timers drive that deterministically rather than a real sleep.
|
||||
vi.useFakeTimers();
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m2" });
|
||||
rerender({ visible: false, latest: "m2" });
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(350);
|
||||
});
|
||||
vi.useRealTimers();
|
||||
expect(result.current).toBeNull();
|
||||
|
||||
// More messages arrive while away, then the user navigates back in.
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m2" });
|
||||
rerender({ visible: true, latest: "m4" });
|
||||
|
||||
// New anchor reflects where the user actually left off (m2), not the
|
||||
// stale m1 from the first visit.
|
||||
expect(result.current).toBe("m2");
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledWith("session-1", "m4"));
|
||||
});
|
||||
|
||||
it("logs and swallows a failed mark-read call instead of throwing", async () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
mockMarkSessionRead.mockRejectedValue(new Error("network error"));
|
||||
const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
renderHook(() => useSessionReadTracking("session-1", true, "m2"));
|
||||
|
||||
await waitFor(() => expect(consoleError).toHaveBeenCalled());
|
||||
consoleError.mockRestore();
|
||||
});
|
||||
|
||||
it("discards a stale mark-read response that resolves after a newer one, so the local cursor never regresses", async () => {
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
|
||||
// Two overlapping requests: an older m2 whose response we control via a
|
||||
// deferred promise, and a newer m3 that resolves immediately. m3's
|
||||
// response lands first, then m2's stale response resolves last.
|
||||
type MarkReadResult = { session_id: string; last_read_message_id: string };
|
||||
let resolveM2: ((value: MarkReadResult) => void) | undefined;
|
||||
const m2Response = new Promise<MarkReadResult>((resolve) => {
|
||||
resolveM2 = resolve;
|
||||
});
|
||||
mockMarkSessionRead.mockImplementation((_sessionId: string, messageId: string) => {
|
||||
if (messageId === "m2") return m2Response;
|
||||
return Promise.resolve({ session_id: "session-1", last_read_message_id: messageId });
|
||||
});
|
||||
|
||||
const { rerender } = renderHook(
|
||||
({ latest }: { latest: string }) => useSessionReadTracking("session-1", true, latest),
|
||||
{ initialProps: { latest: "m2" } },
|
||||
);
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledWith("session-1", "m2"));
|
||||
|
||||
// A newer message arrives while m2's request is still in flight — this
|
||||
// dispatches and resolves the m3 request before m2 settles.
|
||||
mockState.taskSessions.items["session-1"] = session({ last_read_message_id: "m1" });
|
||||
rerender({ latest: "m3" });
|
||||
await waitFor(() => expect(mockMarkSessionRead).toHaveBeenCalledWith("session-1", "m3"));
|
||||
await waitFor(() =>
|
||||
expect(mockUpdateSessionReadCursor).toHaveBeenCalledWith("session-1", "m3"),
|
||||
);
|
||||
mockUpdateSessionReadCursor.mockClear();
|
||||
|
||||
// The delayed, now-stale m2 response finally resolves. It must be
|
||||
// discarded rather than regressing the store back to m2.
|
||||
resolveM2?.({ session_id: "session-1", last_read_message_id: "m2" });
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(mockUpdateSessionReadCursor).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,169 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { useAppStore, useAppStoreApi } from "@/components/state-provider";
|
||||
import { markSessionRead } from "@/lib/api/domains/session-api";
|
||||
|
||||
type Anchor = { sessionId: string; messageId: string | null };
|
||||
type Visit = {
|
||||
sessionId: string;
|
||||
priorCursor: string | null;
|
||||
initialMessagesReady: boolean;
|
||||
};
|
||||
|
||||
function visitAnchor(
|
||||
sessionId: string,
|
||||
priorCursor: string | null,
|
||||
latestMessageId: string | null,
|
||||
) {
|
||||
return latestMessageId !== null && priorCursor !== null && priorCursor !== latestMessageId
|
||||
? { sessionId, messageId: priorCursor }
|
||||
: null;
|
||||
}
|
||||
|
||||
function visibleAnchor(
|
||||
unreadDividerEnabled: boolean,
|
||||
isVisible: boolean,
|
||||
anchor: Anchor | null,
|
||||
sessionId: string | null,
|
||||
): string | null {
|
||||
if (!unreadDividerEnabled || !isVisible || anchor?.sessionId !== sessionId) return null;
|
||||
return anchor.messageId;
|
||||
}
|
||||
|
||||
/**
|
||||
* Tracks a session's Slack-style read cursor and derives the frozen
|
||||
* unread-divider anchor for the current "visit".
|
||||
*
|
||||
* - While the session is the visible chat panel, the cursor advances to
|
||||
* latestMessageId immediately (no debounce) whenever it changes — mirrors
|
||||
* Slack's continuous mark-as-read-while-open behavior, so a quick
|
||||
* navigate-away-and-back still sees the correct boundary next time.
|
||||
* - A visible visit captures its persisted cursor synchronously, but decides
|
||||
* divider eligibility only once the initial message load settles. If that
|
||||
* cursor is already the initial transcript tail, the visit is permanently
|
||||
* divider-ineligible: later live messages cannot create a "New" boundary.
|
||||
* A genuine initial unread boundary remains frozen until the next
|
||||
* visibility transition (leaving and coming back, or switching sessions).
|
||||
* - The visit state is captured during render (via the React "adjust state
|
||||
* while rendering" pattern), not inside a useEffect. It retains the cursor
|
||||
* from before live mark-read can overwrite it, while avoiding a divider
|
||||
* decision from a partial transcript.
|
||||
* - The capture is additionally gated on the session record already
|
||||
* existing in the store (a reactive selector, not store.getState()). This
|
||||
* prevents a host where isVisible is true from its first render (such as
|
||||
* mobile) from treating an unloaded session as a real empty cursor.
|
||||
*
|
||||
* Returns the render-item key (see hooks/use-processed-messages.ts's
|
||||
* findUnreadDividerItemId) the "New" divider should render immediately
|
||||
* before, or null when there's nothing to mark.
|
||||
*/
|
||||
export function useSessionReadTracking(
|
||||
sessionId: string | null,
|
||||
isVisible: boolean,
|
||||
latestMessageId: string | null,
|
||||
initialMessagesLoading = false,
|
||||
): string | null {
|
||||
const store = useAppStoreApi();
|
||||
const updateSessionReadCursor = useAppStore((state) => state.updateSessionReadCursor);
|
||||
const unreadDividerEnabled = useAppStore((state) => state.userSettings.unreadDivider);
|
||||
// Reactive (not store.getState()) so a session that hasn't loaded into the
|
||||
// store yet at mount — e.g. mobile, where isVisible defaults to true from
|
||||
// the very first render (see TaskChatPanelProps), unlike the dockview host
|
||||
// path where usePanelActive's async lag incidentally gives the session
|
||||
// fetch a head start — triggers a re-render once it does, instead of the
|
||||
// capture below racing ahead of it and locking in undefined?.last_read_
|
||||
// message_id ?? null as if that were a real "no prior cursor" answer.
|
||||
const sessionLoaded = useAppStore(
|
||||
(state) => sessionId !== null && sessionId in state.taskSessions.items,
|
||||
);
|
||||
const [anchor, setAnchor] = useState<Anchor | null>(null);
|
||||
// A visit starts at a visibility transition but only becomes eligible for
|
||||
// a divider after its initial message load settles. Its prior cursor stays
|
||||
// immutable so live messages cannot create a new boundary mid-visit.
|
||||
const [visit, setVisit] = useState<Visit | null>(null);
|
||||
|
||||
if (unreadDividerEnabled && isVisible && sessionId && sessionLoaded) {
|
||||
if (visit?.sessionId !== sessionId) {
|
||||
const priorCursor =
|
||||
store.getState().taskSessions.items[sessionId]?.last_read_message_id ?? null;
|
||||
const initialMessagesReady = !initialMessagesLoading;
|
||||
setVisit({ sessionId, priorCursor, initialMessagesReady });
|
||||
setAnchor(initialMessagesReady ? visitAnchor(sessionId, priorCursor, latestMessageId) : null);
|
||||
} else if (!visit.initialMessagesReady && !initialMessagesLoading) {
|
||||
setVisit({ ...visit, initialMessagesReady: true });
|
||||
setAnchor(visitAnchor(sessionId, visit.priorCursor, latestMessageId));
|
||||
}
|
||||
}
|
||||
|
||||
// The *hide* transition is debounced rather than immediate: Dockview
|
||||
// briefly disposes and re-registers a panel's underlying api during a
|
||||
// layout-driven remount (see PanelPortalManager's class doc and
|
||||
// usePanelActive), which can make isVisible read false for a render or
|
||||
// two even though the user never actually left.
|
||||
useEffect(() => {
|
||||
if (visit === null) return;
|
||||
if (!unreadDividerEnabled) {
|
||||
setVisit(null);
|
||||
setAnchor(null);
|
||||
latestDispatchRef.current = null;
|
||||
return;
|
||||
}
|
||||
if (isVisible) return;
|
||||
const timer = setTimeout(() => {
|
||||
setVisit(null);
|
||||
setAnchor(null);
|
||||
latestDispatchRef.current = null;
|
||||
}, 300);
|
||||
return () => clearTimeout(timer);
|
||||
}, [isVisible, unreadDividerEnabled, visit]);
|
||||
|
||||
// Tracks the (sessionId, messageId) of the most recently *dispatched*
|
||||
// mark-read request. The backend's cursor write is atomically monotonic
|
||||
// (see UpdateTaskSessionLastReadMessageID), but that only protects the
|
||||
// persisted row — not an in-flight HTTP response, which is a snapshot
|
||||
// frozen at the moment its own GetTaskSession ran. Two overlapping
|
||||
// requests (an older m2, then a newer m3) can still have their responses
|
||||
// arrive in either order; if m2's (now-stale) response resolves after
|
||||
// m3's, applying it verbatim would regress the local store back to m2
|
||||
// even though the database is correctly at m3. Comparing against this
|
||||
// ref when a response resolves discards any response that's no longer
|
||||
// the latest dispatched request for this session.
|
||||
const latestDispatchRef = useRef<{ sessionId: string; messageId: string } | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (
|
||||
!unreadDividerEnabled ||
|
||||
!sessionId ||
|
||||
!isVisible ||
|
||||
!latestMessageId ||
|
||||
visit?.sessionId !== sessionId ||
|
||||
!visit.initialMessagesReady
|
||||
)
|
||||
return;
|
||||
const currentCursor = store.getState().taskSessions.items[sessionId]?.last_read_message_id;
|
||||
if (currentCursor === latestMessageId) return;
|
||||
latestDispatchRef.current = { sessionId, messageId: latestMessageId };
|
||||
void markSessionRead(sessionId, latestMessageId)
|
||||
.then((response) => {
|
||||
const dispatch = latestDispatchRef.current;
|
||||
const isStale =
|
||||
!dispatch || dispatch.sessionId !== sessionId || dispatch.messageId !== latestMessageId;
|
||||
if (isStale) return;
|
||||
updateSessionReadCursor(response.session_id, response.last_read_message_id);
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
console.error("Failed to mark session read", err);
|
||||
});
|
||||
}, [
|
||||
sessionId,
|
||||
isVisible,
|
||||
visit,
|
||||
latestMessageId,
|
||||
store,
|
||||
unreadDividerEnabled,
|
||||
updateSessionReadCursor,
|
||||
]);
|
||||
|
||||
return visibleAnchor(unreadDividerEnabled, isVisible, anchor, sessionId);
|
||||
}
|
||||
@@ -8,6 +8,7 @@ import { useSessionChangesCount } from "@/hooks/domains/session/use-session-chan
|
||||
import type { ReviewSource } from "@/hooks/domains/session/use-review-sources";
|
||||
import { useEnvironmentSessionId } from "@/hooks/use-environment-session-id";
|
||||
import { useFileEditors } from "@/hooks/use-file-editors";
|
||||
import { usePanelActive } from "@/hooks/use-panel-active";
|
||||
import { setPanelTitle } from "@/lib/layout/panel-portal-manager";
|
||||
import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import { BrowserPanel } from "./browser-panel";
|
||||
@@ -60,6 +61,7 @@ function ChatContent({ panelId, params }: { panelId: string; params: Record<stri
|
||||
sessionId ? state.taskSessions.items[sessionId]?.is_passthrough === true : false,
|
||||
);
|
||||
useChatSessionTitle(panelId, sessionId);
|
||||
const isVisible = usePanelActive(panelId);
|
||||
|
||||
if (isPassthrough) {
|
||||
return (
|
||||
@@ -77,6 +79,7 @@ function ChatContent({ panelId, params }: { panelId: string; params: Record<stri
|
||||
onOpenFile={openFile}
|
||||
onOpenFileAtLine={openFile}
|
||||
hideSessionsDropdown
|
||||
isVisible={isVisible}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
import { useDockviewStore } from "@/lib/state/dockview-store";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { useFileEditors } from "@/hooks/use-file-editors";
|
||||
import { usePanelActive } from "@/hooks/use-panel-active";
|
||||
import { useSessionGitStatus } from "@/hooks/domains/session/use-session-git-status";
|
||||
import { useSessionCommits } from "@/hooks/domains/session/use-session-commits";
|
||||
import { useEnvironmentSessionId } from "@/hooks/use-environment-session-id";
|
||||
@@ -188,6 +189,7 @@ function ChatContent({ panelId, params }: { panelId: string; params: Record<stri
|
||||
sessionId ? state.taskSessions.items[sessionId]?.is_passthrough === true : false,
|
||||
);
|
||||
useChatSessionTitle(panelId, sessionId, !!paramSessionId);
|
||||
const isVisible = usePanelActive(panelId);
|
||||
|
||||
if (isPassthrough) {
|
||||
return (
|
||||
@@ -205,6 +207,7 @@ function ChatContent({ panelId, params }: { panelId: string; params: Record<stri
|
||||
onOpenFile={openFile}
|
||||
onOpenFileAtLine={openFile}
|
||||
hideSessionsDropdown
|
||||
isVisible={isVisible}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -177,6 +177,9 @@ export function PreviewSessionBody({ session, taskId }: { session: TaskSession;
|
||||
sessionId={session.id}
|
||||
taskId={taskId}
|
||||
hideSessionsDropdown
|
||||
// Read-only kanban hover preview — a transient glance, not "opening"
|
||||
// the task. Never advances the Slack-style read cursor.
|
||||
isVisible={false}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -30,6 +30,8 @@ import { SessionSearchHits } from "@/components/task/chat/session-search-hits";
|
||||
import { usePanelSearch } from "@/hooks/use-panel-search";
|
||||
import { useSessionSearch } from "@/hooks/domains/session/use-session-search";
|
||||
import { useLazyLoadMessages } from "@/hooks/use-lazy-load-messages";
|
||||
import { findUnreadDividerItemId, lastRenderedMessageId } from "@/lib/session-unread-divider";
|
||||
import { useSessionReadTracking } from "./chat/use-session-read-tracking";
|
||||
import { useDrainOlderMessages } from "@/components/task/chat/use-drain-older-messages";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import type { Message } from "@/lib/types/http";
|
||||
@@ -55,6 +57,25 @@ function useClarificationKey(agentMessageCount: number) {
|
||||
return { clarificationKey, handleClarificationResolved };
|
||||
}
|
||||
|
||||
function useUnreadDividerBeforeItemKey(
|
||||
sessionId: string | null,
|
||||
isVisible: boolean,
|
||||
groupedItems: Parameters<typeof lastRenderedMessageId>[0],
|
||||
isInitialMessagesLoading: boolean,
|
||||
) {
|
||||
const latestMessageId = useMemo(() => lastRenderedMessageId(groupedItems), [groupedItems]);
|
||||
const dividerAnchor = useSessionReadTracking(
|
||||
sessionId,
|
||||
isVisible,
|
||||
latestMessageId,
|
||||
isInitialMessagesLoading,
|
||||
);
|
||||
return useMemo(
|
||||
() => findUnreadDividerItemId(groupedItems, dividerAnchor),
|
||||
[groupedItems, dividerAnchor],
|
||||
);
|
||||
}
|
||||
|
||||
function SessionSearchOverlay({
|
||||
search,
|
||||
agentLabel,
|
||||
@@ -154,9 +175,18 @@ type TaskChatPanelProps = {
|
||||
onOpenFileAtLine?: (filePath: string) => void;
|
||||
/** Hide the sessions dropdown (session tabs in dockview replace it) */
|
||||
hideSessionsDropdown?: boolean;
|
||||
/**
|
||||
* Whether this panel is the one actually on screen — gates the
|
||||
* Slack-style unread-divider read tracking (see
|
||||
* chat/use-session-read-tracking.ts). Dockview-hosted callers must pass
|
||||
* real tab-activation state (see hooks/use-panel-active.ts); other hosts
|
||||
* (mobile, quick chat, kanban preview) default to true since mounting
|
||||
* already implies visibility for them.
|
||||
*/
|
||||
isVisible?: boolean;
|
||||
};
|
||||
|
||||
// eslint-disable-next-line max-lines-per-function -- composes many sub-panels; each concern already factored into its own hook
|
||||
// eslint-disable-next-line complexity, max-lines-per-function -- composes many sub-panels; each concern already factored into its own hook
|
||||
export const TaskChatPanel = memo(function TaskChatPanel({
|
||||
onSend,
|
||||
sessionId = null,
|
||||
@@ -166,6 +196,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({
|
||||
onRequestChangesTooltipDismiss,
|
||||
onOpenFileAtLine,
|
||||
hideSessionsDropdown,
|
||||
isVisible = true,
|
||||
}: TaskChatPanelProps) {
|
||||
const isArchived = useIsTaskArchived();
|
||||
const chatInputRef = useRef<ChatInputContainerHandle>(null);
|
||||
@@ -184,6 +215,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({
|
||||
taskId,
|
||||
isWorking,
|
||||
messagesLoading,
|
||||
isInitialMessagesLoading,
|
||||
groupedItems,
|
||||
allMessages,
|
||||
footerActionMessages,
|
||||
@@ -193,6 +225,12 @@ export const TaskChatPanel = memo(function TaskChatPanel({
|
||||
pendingClarification,
|
||||
pendingClarificationGroup,
|
||||
} = panelState;
|
||||
const dividerBeforeItemKey = useUnreadDividerBeforeItemKey(
|
||||
resolvedSessionId,
|
||||
isVisible,
|
||||
groupedItems,
|
||||
isInitialMessagesLoading,
|
||||
);
|
||||
const { handleCancelTurn } = useChatPanelHandlers(resolvedSessionId, chatInputRef);
|
||||
const { clarificationKey, handleClarificationResolved } = useClarificationKey(agentMessageCount);
|
||||
const {
|
||||
@@ -326,6 +364,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({
|
||||
sessionState={session?.state}
|
||||
worktreePath={session?.worktree_path}
|
||||
onOpenFile={onOpenFile}
|
||||
dividerBeforeItemKey={dividerBeforeItemKey}
|
||||
lastPromptMessageId={lastPromptMessageId}
|
||||
onLastPromptEdgeChange={setLastPromptEdge}
|
||||
firstMessageId={firstMessageId}
|
||||
|
||||
@@ -1718,6 +1718,28 @@ export class ApiClient {
|
||||
return this.request("GET", `/api/v1/task-sessions/${sessionId}/turns`);
|
||||
}
|
||||
|
||||
async getTaskSession(sessionId: string): Promise<{
|
||||
session: { id: string; last_read_message_id?: string };
|
||||
}> {
|
||||
return this.request("GET", `/api/v1/task-sessions/${sessionId}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Force-sets the session's read cursor directly, bypassing the production
|
||||
* mark-read endpoint's forward-only (monotonic) guard. Used by e2e specs
|
||||
* that need to seed "the user last read up through an earlier message"
|
||||
* deterministically — rewinding through the real endpoint would now be
|
||||
* rejected as a stale/out-of-order update.
|
||||
*/
|
||||
async forceSetSessionReadCursor(
|
||||
sessionId: string,
|
||||
messageId: string,
|
||||
): Promise<{ id: string; last_read_message_id: string }> {
|
||||
return this.request("PATCH", `/api/v1/e2e/task-sessions/${sessionId}/read-cursor`, {
|
||||
message_id: messageId,
|
||||
});
|
||||
}
|
||||
|
||||
async listTasks(
|
||||
workspaceId: string,
|
||||
): Promise<{ tasks: Array<{ id: string; title: string; workflow_step_id?: string }> }> {
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
import { type Locator } from "@playwright/test";
|
||||
import type { ApiClient } from "./api-client";
|
||||
import type { SeedData } from "../fixtures/test-base";
|
||||
import { multiMessageScript } from "./seed-session-messages";
|
||||
|
||||
/** True only when element is actually scrolled into container's visible
|
||||
* viewport. Unlike Playwright's toBeVisible(), which only checks CSS
|
||||
* visibility, this also verifies the element is scrolled into the
|
||||
* container's viewport, not merely present somewhere in its (taller)
|
||||
* overflow area. Callers must root both locators at
|
||||
* SessionPage.activeChat() — Dockview keeps background chat panels
|
||||
* mounted, and an unscoped locator could otherwise resolve a stale,
|
||||
* hidden panel's elements instead of the one the user is actually
|
||||
* looking at. */
|
||||
export async function isScrolledIntoView(container: Locator, element: Locator): Promise<boolean> {
|
||||
const [containerBox, elementBox] = await Promise.all([
|
||||
container.boundingBox(),
|
||||
element.boundingBox(),
|
||||
]);
|
||||
if (!containerBox || !elementBox) return false;
|
||||
return (
|
||||
elementBox.y >= containerBox.y &&
|
||||
elementBox.y + elementBox.height <= containerBox.y + containerBox.height
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Seeds a task whose transcript buries a read cursor under enough messages
|
||||
* both before and after it that, scrolled to the bottom (the naive,
|
||||
* unconditional behavior this feature replaces), the divider would land
|
||||
* well outside the visible viewport. Shared by the desktop and mobile
|
||||
* scroll-to-divider specs — same setup, different viewport/project.
|
||||
*/
|
||||
export async function seedScrollTestConversation(
|
||||
apiClient: ApiClient,
|
||||
seedData: SeedData,
|
||||
title: string,
|
||||
): Promise<{
|
||||
taskId: string;
|
||||
sessionId: string;
|
||||
cursorMessageId: string;
|
||||
newestMessageId: string;
|
||||
}> {
|
||||
const task = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
title,
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: multiMessageScript(["chat ready"], 5),
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
|
||||
const sessionId = task.session_id;
|
||||
|
||||
await apiClient.seedAgentMessages(sessionId, 40, "before cursor");
|
||||
const beforeCursor = await apiClient.listSessionMessages(sessionId);
|
||||
const cursorMessageId = beforeCursor.messages[beforeCursor.messages.length - 1].id;
|
||||
await apiClient.seedAgentMessages(sessionId, 40, "after cursor");
|
||||
const fullTranscript = await apiClient.listSessionMessages(sessionId);
|
||||
const newestMessageId = fullTranscript.messages[fullTranscript.messages.length - 1].id;
|
||||
|
||||
await apiClient.forceSetSessionReadCursor(sessionId, cursorMessageId);
|
||||
|
||||
return { taskId: task.id, sessionId, cursorMessageId, newestMessageId };
|
||||
}
|
||||
@@ -0,0 +1,85 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { openTaskSession } from "../../helpers/session";
|
||||
import { isScrolledIntoView, seedScrollTestConversation } from "../../helpers/unread-divider";
|
||||
|
||||
/**
|
||||
* Mobile parity for the Slack-style unread divider: the same
|
||||
* useSessionReadTracking/NativeMessageList code renders on mobile — this
|
||||
* confirms the divider still appears and the visit-start scroll still lands
|
||||
* on it correctly under the mobile chat layout, not just desktop's Dockview
|
||||
* panel. Setup is shared with the desktop scroll spec (see
|
||||
* helpers/unread-divider.ts) — same conversation shape, different viewport.
|
||||
*/
|
||||
test.describe("Mobile unread divider", () => {
|
||||
test("scrolls to the New divider on visit start under the mobile chat layout", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(60_000);
|
||||
|
||||
const { taskId, newestMessageId } = await seedScrollTestConversation(
|
||||
apiClient,
|
||||
seedData,
|
||||
"Mobile Unread Divider Scroll Test",
|
||||
);
|
||||
|
||||
// First visit to a task that was running off-screen. The seeded cursor
|
||||
// must remain unread until this visit, so do not make a throwaway visit
|
||||
// before opening the task under test.
|
||||
const session = await openTaskSession(testPage, taskId);
|
||||
const activeChat = session.activeChat();
|
||||
const scrollContainer = activeChat.locator(".chat-message-list");
|
||||
|
||||
const divider = activeChat.getByTestId("unread-divider");
|
||||
await expect(divider).toBeVisible();
|
||||
|
||||
await expect.poll(() => isScrolledIntoView(scrollContainer, divider)).toBe(true);
|
||||
|
||||
// The newest message — where a naive scroll-to-bottom would have left
|
||||
// the viewport — must NOT be in view; the mobile layout scrolled up to
|
||||
// the divider too.
|
||||
const newestRow = activeChat.locator(`[id="msg-${newestMessageId}"]`);
|
||||
expect(await isScrolledIntoView(scrollContainer, newestRow)).toBe(false);
|
||||
});
|
||||
|
||||
test("does not add a New divider when a visible session advances from its current tail", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(150_000);
|
||||
const task = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Mobile Unread Divider Active Visit Test",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: "/e2e:simple-message",
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
|
||||
|
||||
let session = await openTaskSession(testPage, task.id);
|
||||
await session.waitForChatIdle({ timeout: 60_000, attemptTimeout: 60_000 });
|
||||
const initialMessages = await apiClient.listSessionMessages(task.session_id);
|
||||
const initialTail = initialMessages.messages[initialMessages.messages.length - 1];
|
||||
if (!initialTail) throw new Error("expected the initial transcript to contain a message");
|
||||
await expect
|
||||
.poll(
|
||||
async () => (await apiClient.getTaskSession(task.session_id!)).session.last_read_message_id,
|
||||
)
|
||||
.toBe(initialTail.id);
|
||||
|
||||
await testPage.goto("/");
|
||||
await testPage.waitForLoadState("networkidle");
|
||||
session = await openTaskSession(testPage, task.id);
|
||||
await expect(session.activeChat().getByTestId("unread-divider")).toHaveCount(0);
|
||||
|
||||
await session.sendMessageViaButton("prompt while actively reading");
|
||||
await session.waitForChatIdle({ timeout: 60_000, attemptTimeout: 60_000 });
|
||||
await expect(session.activeChat().getByTestId("unread-divider")).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,186 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { openTaskSession } from "../../helpers/session";
|
||||
import { isScrolledIntoView, seedScrollTestConversation } from "../../helpers/unread-divider";
|
||||
|
||||
test.describe("Unread divider", () => {
|
||||
test("shows the Slack-style New divider only for messages read before a rewound cursor, and clears after the next visit", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(150_000);
|
||||
|
||||
const task = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Unread Divider Test",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: "/e2e:simple-message",
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
|
||||
const sessionId = task.session_id;
|
||||
|
||||
let session = await openTaskSession(testPage, task.id);
|
||||
// attemptTimeout must be raised explicitly, not just `timeout` — its
|
||||
// default is capped at min(15_000, ...) regardless of `timeout` (see
|
||||
// SessionPage.waitForChatIdle). Past that cap it does a page.reload()
|
||||
// to recover from a stuck WS subscription, which is a *real*
|
||||
// navigate-away-and-back from this feature's own perspective — it
|
||||
// would legitimately (and correctly) re-capture a fresh divider
|
||||
// anchor, making later assertions in this test fail for a reason
|
||||
// unrelated to what they're testing (the cursor advancing live while
|
||||
// the session stays genuinely, continuously visible in one browser
|
||||
// session).
|
||||
await session.waitForChatIdle({ timeout: 60_000, attemptTimeout: 60_000 });
|
||||
|
||||
// First-ever visit: there is no prior read cursor, so nothing renders as
|
||||
// "New" — but the cursor must still advance to the latest message so a
|
||||
// later visit has a real boundary to compare against.
|
||||
await expect(session.activeChat().getByTestId("unread-divider")).toHaveCount(0);
|
||||
await expect
|
||||
.poll(async () => (await apiClient.getTaskSession(sessionId)).session.last_read_message_id)
|
||||
.not.toBeUndefined();
|
||||
|
||||
const firstExchange = await apiClient.listSessionMessages(sessionId);
|
||||
expect(firstExchange.messages.length).toBeGreaterThanOrEqual(2);
|
||||
const readCursorMessageId = firstExchange.messages[firstExchange.messages.length - 1].id;
|
||||
|
||||
// Grow the transcript with a second exchange while still visible — the
|
||||
// cursor keeps advancing live while the session stays in view, so this
|
||||
// alone must not produce a divider either.
|
||||
await session.sendMessageViaButton("second question");
|
||||
await session.waitForChatIdle({ timeout: 60_000, attemptTimeout: 60_000 });
|
||||
await expect(session.activeChat().getByTestId("unread-divider")).toHaveCount(0);
|
||||
|
||||
const fullTranscript = await apiClient.listSessionMessages(sessionId);
|
||||
expect(fullTranscript.messages.length).toBeGreaterThan(firstExchange.messages.length);
|
||||
const firstUnreadMessage = fullTranscript.messages[firstExchange.messages.length];
|
||||
|
||||
// Rewind the persisted cursor back to the end of the first exchange —
|
||||
// deterministically simulating "the last time the user actually looked
|
||||
// at this task, only the first exchange existed." The newer messages
|
||||
// above are real, already-persisted transcript rows produced by the
|
||||
// mock agent above; only the read cursor is synthetically rewound here
|
||||
// (driving a second live agent turn from a truly backgrounded task
|
||||
// isn't practical from a single Playwright page). What this proves is
|
||||
// the navigation → hydration → divider-positioning path end to end;
|
||||
// cursor persistence and message-ownership validation are covered by
|
||||
// the backend repository/service/handler tests. Uses the e2e-only
|
||||
// force-set backdoor rather than the production mark-read endpoint:
|
||||
// the real endpoint is monotonic (it never regresses the persisted
|
||||
// cursor), so replaying an older messageId through it would now be a
|
||||
// rejected no-op instead of rewinding state for this test.
|
||||
await apiClient.forceSetSessionReadCursor(sessionId, readCursorMessageId);
|
||||
|
||||
// Navigate away, then back — the actual "navigate into a task that was
|
||||
// running outside of active view" trigger this feature targets.
|
||||
await testPage.goto("/");
|
||||
await testPage.waitForLoadState("networkidle");
|
||||
session = await openTaskSession(testPage, task.id);
|
||||
|
||||
const activeChat = session.activeChat();
|
||||
const divider = activeChat.getByTestId("unread-divider");
|
||||
await expect(divider).toBeVisible();
|
||||
|
||||
// Positioned as part of the first unread message's own row (the
|
||||
// renderer nests the divider inside that message's wrapper, immediately
|
||||
// before its content) — never inside or before an already-read row.
|
||||
const firstUnreadRow = activeChat.locator(`[id="msg-${firstUnreadMessage.id}"]`);
|
||||
const readRow = activeChat.locator(`[id="msg-${readCursorMessageId}"]`);
|
||||
await expect(firstUnreadRow).toBeVisible();
|
||||
await expect(readRow).toBeVisible();
|
||||
await expect(firstUnreadRow.getByTestId("unread-divider")).toHaveCount(1);
|
||||
await expect(readRow.getByTestId("unread-divider")).toHaveCount(0);
|
||||
|
||||
const [dividerBox, readBox] = await Promise.all([divider.boundingBox(), readRow.boundingBox()]);
|
||||
if (!dividerBox || !readBox) {
|
||||
throw new Error("expected the divider and the read row to be measurable");
|
||||
}
|
||||
expect(dividerBox.y).toBeGreaterThanOrEqual(readBox.y + readBox.height);
|
||||
|
||||
// Re-visiting already advanced the cursor to the latest message, so a
|
||||
// second navigate-away-and-back shows no divider — it doesn't linger.
|
||||
await testPage.goto("/");
|
||||
await testPage.waitForLoadState("networkidle");
|
||||
session = await openTaskSession(testPage, task.id);
|
||||
await expect(session.activeChat().getByTestId("unread-divider")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("does not add a New divider when a visible session advances from its current tail", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(150_000);
|
||||
const task = await apiClient.createTaskWithAgent(
|
||||
seedData.workspaceId,
|
||||
"Unread Divider Active Visit Test",
|
||||
seedData.agentProfileId,
|
||||
{
|
||||
description: "/e2e:simple-message",
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
},
|
||||
);
|
||||
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
|
||||
|
||||
let session = await openTaskSession(testPage, task.id);
|
||||
await session.waitForChatIdle({ timeout: 60_000, attemptTimeout: 60_000 });
|
||||
const initialMessages = await apiClient.listSessionMessages(task.session_id);
|
||||
const initialTail = initialMessages.messages[initialMessages.messages.length - 1];
|
||||
if (!initialTail) throw new Error("expected the initial transcript to contain a message");
|
||||
await expect
|
||||
.poll(
|
||||
async () => (await apiClient.getTaskSession(task.session_id!)).session.last_read_message_id,
|
||||
)
|
||||
.toBe(initialTail.id);
|
||||
|
||||
// Returning at the persisted tail starts a clean visit: it has no visible
|
||||
// divider, but the hook must also discard its latent tail anchor.
|
||||
await testPage.goto("/");
|
||||
await testPage.waitForLoadState("networkidle");
|
||||
session = await openTaskSession(testPage, task.id);
|
||||
await expect(session.activeChat().getByTestId("unread-divider")).toHaveCount(0);
|
||||
|
||||
await session.sendMessageViaButton("prompt while actively reading");
|
||||
await session.waitForChatIdle({ timeout: 60_000, attemptTimeout: 60_000 });
|
||||
await expect(session.activeChat().getByTestId("unread-divider")).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("scrolls straight to the New divider on visit start when it's outside the initial viewport, instead of jumping to the newest message", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(60_000);
|
||||
|
||||
const { taskId, newestMessageId } = await seedScrollTestConversation(
|
||||
apiClient,
|
||||
seedData,
|
||||
"Unread Divider Scroll Test",
|
||||
);
|
||||
|
||||
// First visit to a task that was running off-screen. The seeded cursor
|
||||
// must remain unread until this visit, so do not make a throwaway visit
|
||||
// before opening the task under test.
|
||||
const session = await openTaskSession(testPage, taskId);
|
||||
const activeChat = session.activeChat();
|
||||
const scrollContainer = activeChat.locator(".chat-message-list");
|
||||
|
||||
const divider = activeChat.getByTestId("unread-divider");
|
||||
await expect(divider).toBeVisible();
|
||||
|
||||
await expect.poll(() => isScrolledIntoView(scrollContainer, divider)).toBe(true);
|
||||
|
||||
// The newest message — where a naive scroll-to-bottom would have left
|
||||
// the viewport — must NOT be in view; we scrolled up to the divider
|
||||
// instead.
|
||||
const newestRow = activeChat.locator(`[id="msg-${newestMessageId}"]`);
|
||||
expect(await isScrolledIntoView(scrollContainer, newestRow)).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,23 @@
|
||||
import { expect, type Page } from "@playwright/test";
|
||||
import type { PrAssetCapture } from "../../helpers/pr-asset-capture";
|
||||
|
||||
export async function assertUnreadDividerSetting(
|
||||
page: Page,
|
||||
prCapture: PrAssetCapture,
|
||||
projectName: string,
|
||||
): Promise<void> {
|
||||
await page.goto("/settings/system/feature-toggles");
|
||||
|
||||
const unreadDivider = page.getByTestId("feature-toggle-features.unreadDivider");
|
||||
await expect(unreadDivider).toBeVisible();
|
||||
await expect(unreadDivider.getByText("Unread divider", { exact: true })).toBeVisible();
|
||||
await expect(unreadDivider.getByRole("switch", { name: "Toggle Unread divider" })).toBeChecked();
|
||||
await expect(unreadDivider).toContainText("Source: Default");
|
||||
await expect(unreadDivider).toContainText("Requires restart");
|
||||
|
||||
await unreadDivider.scrollIntoViewIfNeeded();
|
||||
await prCapture.screenshot(`unread-divider-setting-${projectName}`, {
|
||||
caption: `Unread divider feature toggle (${projectName})`,
|
||||
fullPage: true,
|
||||
});
|
||||
}
|
||||
@@ -0,0 +1,33 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
|
||||
test.describe("Unread divider preference on mobile", () => {
|
||||
test("persists a disabled New divider from Task Actions", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
prCapture,
|
||||
}, testInfo) => {
|
||||
await testPage.goto("/settings/general/task-actions");
|
||||
const toggle = testPage.getByRole("switch", { name: "Show New divider in transcripts" });
|
||||
|
||||
await expect(toggle).toBeChecked();
|
||||
|
||||
await toggle.scrollIntoViewIfNeeded();
|
||||
|
||||
await prCapture.screenshot(`unread-divider-preference-${testInfo.project.name}`, {
|
||||
caption: `Unread divider preference (${testInfo.project.name})`,
|
||||
fullPage: true,
|
||||
});
|
||||
await toggle.click();
|
||||
await expect(toggle).not.toBeChecked();
|
||||
expect((await apiClient.getUserSettings()).settings.unread_divider).toBe(true);
|
||||
|
||||
await testPage
|
||||
.getByTestId("settings-floating-save")
|
||||
.getByRole("button", { name: "Save changes" })
|
||||
.click();
|
||||
|
||||
await expect
|
||||
.poll(async () => (await apiClient.getUserSettings()).settings.unread_divider)
|
||||
.toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,33 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
|
||||
test.describe("Unread divider preference", () => {
|
||||
test("persists a disabled New divider from Task Actions", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
prCapture,
|
||||
}, testInfo) => {
|
||||
await testPage.goto("/settings/general/task-actions");
|
||||
const toggle = testPage.getByRole("switch", { name: "Show New divider in transcripts" });
|
||||
|
||||
await expect(toggle).toBeChecked();
|
||||
|
||||
await toggle.scrollIntoViewIfNeeded();
|
||||
|
||||
await prCapture.screenshot(`unread-divider-preference-${testInfo.project.name}`, {
|
||||
caption: `Unread divider preference (${testInfo.project.name})`,
|
||||
fullPage: true,
|
||||
});
|
||||
await toggle.click();
|
||||
await expect(toggle).not.toBeChecked();
|
||||
expect((await apiClient.getUserSettings()).settings.unread_divider).toBe(true);
|
||||
|
||||
await testPage
|
||||
.getByTestId("settings-floating-save")
|
||||
.getByRole("button", { name: "Save changes" })
|
||||
.click();
|
||||
|
||||
await expect
|
||||
.poll(async () => (await apiClient.getUserSettings()).settings.unread_divider)
|
||||
.toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -132,6 +132,7 @@ function summarizeMessages(messages: Message[]): {
|
||||
|
||||
interface UseSessionMessagesReturn {
|
||||
isLoading: boolean;
|
||||
isInitialMessagesLoading: boolean;
|
||||
messages: Message[];
|
||||
hasMore: boolean;
|
||||
oldestCursor: string | null;
|
||||
@@ -700,6 +701,7 @@ export function useSessionMessages(taskSessionId: string | null): UseSessionMess
|
||||
|
||||
return {
|
||||
isLoading: isLoading || isWaitingForInitialMessages || messagesMeta.isLoading,
|
||||
isInitialMessagesLoading: isWaitingForInitialMessages,
|
||||
messages,
|
||||
hasMore: messagesMeta.hasMore,
|
||||
oldestCursor: messagesMeta.oldestCursor,
|
||||
|
||||
@@ -60,6 +60,7 @@ function makeUnloadedSettings(): UserSettingsState {
|
||||
chatSubmitKey: "cmd_enter",
|
||||
reviewAutoMarkOnScroll: true,
|
||||
confirmTaskArchive: true,
|
||||
unreadDivider: true,
|
||||
mcpTaskAgentProfileDefault: "current_task",
|
||||
showAnchoredPromptBar: false,
|
||||
showScrollToLastPrompt: true,
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
import { act, cleanup, renderHook } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import type { DockviewPanelApi } from "dockview-react";
|
||||
import { panelPortalManager } from "@/lib/layout/panel-portal-manager";
|
||||
import { usePanelActive } from "./use-panel-active";
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
type FakePanelApi = {
|
||||
isActive: boolean;
|
||||
onDidActiveChange: (cb: (event: { isActive: boolean }) => void) => { dispose: () => void };
|
||||
};
|
||||
|
||||
type FakeApiHandle = {
|
||||
fakeApi: FakePanelApi;
|
||||
fireActiveChange: (isActive: boolean) => void;
|
||||
};
|
||||
|
||||
function makeFakeApi(initialIsActive: boolean): FakeApiHandle {
|
||||
let listener: ((event: { isActive: boolean }) => void) | null = null;
|
||||
const fakeApi: FakePanelApi = {
|
||||
isActive: initialIsActive,
|
||||
onDidActiveChange: (cb) => {
|
||||
listener = cb;
|
||||
return { dispose: () => (listener = null) };
|
||||
},
|
||||
};
|
||||
return {
|
||||
fakeApi,
|
||||
fireActiveChange: (isActive: boolean) => {
|
||||
fakeApi.isActive = isActive;
|
||||
listener?.({ isActive });
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function acquirePanel(panelId: string, initialIsActive: boolean): FakeApiHandle {
|
||||
const handle = makeFakeApi(initialIsActive);
|
||||
// Test double: only the subset of DockviewPanelApi this hook actually
|
||||
// reads/calls is implemented. Unchecked cast is intentional here — the
|
||||
// full interface has 30+ unrelated members no test in this file exercises.
|
||||
const dockviewApi = handle.fakeApi as unknown as DockviewPanelApi;
|
||||
panelPortalManager.acquire(panelId, "chat", {}, dockviewApi);
|
||||
return handle;
|
||||
}
|
||||
|
||||
describe("usePanelActive", () => {
|
||||
afterEach(() => {
|
||||
// Release any panels acquired by a test so state doesn't leak across cases.
|
||||
for (const id of panelPortalManager.ids()) panelPortalManager.release(id);
|
||||
});
|
||||
|
||||
it("returns false before a portal entry/api has been registered for the panel", () => {
|
||||
const { result } = renderHook(() => usePanelActive("panel-unregistered"));
|
||||
expect(result.current).toBe(false);
|
||||
});
|
||||
|
||||
it("reflects the panel's initial isActive state once registered", () => {
|
||||
acquirePanel("panel-active", true);
|
||||
const { result } = renderHook(() => usePanelActive("panel-active"));
|
||||
expect(result.current).toBe(true);
|
||||
|
||||
acquirePanel("panel-inactive", false);
|
||||
const { result: result2 } = renderHook(() => usePanelActive("panel-inactive"));
|
||||
expect(result2.current).toBe(false);
|
||||
});
|
||||
|
||||
it("updates when the panel's active tab changes via onDidActiveChange", () => {
|
||||
const handle = acquirePanel("panel-toggle", false);
|
||||
const { result } = renderHook(() => usePanelActive("panel-toggle"));
|
||||
expect(result.current).toBe(false);
|
||||
|
||||
act(() => handle.fireActiveChange(true));
|
||||
expect(result.current).toBe(true);
|
||||
|
||||
act(() => handle.fireActiveChange(false));
|
||||
expect(result.current).toBe(false);
|
||||
});
|
||||
|
||||
it("falls back to false again if the panel is released", () => {
|
||||
const PANEL_ID = "panel-released";
|
||||
acquirePanel(PANEL_ID, true);
|
||||
const { result, rerender } = renderHook(({ id }: { id: string }) => usePanelActive(id), {
|
||||
initialProps: { id: PANEL_ID },
|
||||
});
|
||||
expect(result.current).toBe(true);
|
||||
|
||||
panelPortalManager.release(PANEL_ID);
|
||||
rerender({ id: PANEL_ID });
|
||||
expect(result.current).toBe(false);
|
||||
});
|
||||
|
||||
it("rebinds to the new panel api when Dockview remounts (replaces) an existing portal entry", () => {
|
||||
const PANEL_ID = "panel-remount";
|
||||
const stale = acquirePanel(PANEL_ID, false);
|
||||
const { result } = renderHook(() => usePanelActive(PANEL_ID));
|
||||
expect(result.current).toBe(false);
|
||||
|
||||
// Simulate Dockview calling api.fromJSON() during a layout restore: the
|
||||
// same panelId is re-acquired with a brand-new DockviewPanelApi,
|
||||
// without the old one ever being released.
|
||||
const fresh = acquirePanel(PANEL_ID, false);
|
||||
|
||||
act(() => fresh.fireActiveChange(true));
|
||||
expect(result.current).toBe(true);
|
||||
|
||||
// The stale api's listener must have been torn down — firing it no
|
||||
// longer moves the hook (it would incorrectly flip back to false
|
||||
// without the resubscribe fix).
|
||||
act(() => stale.fireActiveChange(false));
|
||||
expect(result.current).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,70 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useSyncExternalStore } from "react";
|
||||
import type { DockviewPanelApi } from "dockview-react";
|
||||
import { panelPortalManager } from "@/lib/layout/panel-portal-manager";
|
||||
|
||||
/**
|
||||
* Tracks whether the dockview panel identified by panelId is the active tab
|
||||
* in its dockview group. Portal-hosted panel content (see
|
||||
* `lib/layout/panel-portal-manager.ts`) stays mounted while its tab is
|
||||
* inactive — e.g. the Chat tab keeps running behind an active Files/Changes
|
||||
* tab — so components that must distinguish "mounted" from "actually on
|
||||
* screen" (like the unread-divider read tracker) need this signal rather
|
||||
* than assuming mount implies visibility.
|
||||
*
|
||||
* Backed by `panelPortalManager` since portal-hosted content only receives
|
||||
* its `panelId`, not the dockview api directly; `usePortalSlot`'s mount
|
||||
* effect acquires the portal entry (with its api) before any child content
|
||||
* renders, so the entry is virtually always present by the time this runs.
|
||||
* Subscribes to both the manager's own add/remove notifications (a panel
|
||||
* acquired after this hook's first mount, or released while it stays
|
||||
* mounted) and the panel's own `onDidActiveChange` (tab switches within an
|
||||
* existing group).
|
||||
*
|
||||
* Only for dockview-hosted panels — pass the panel's real id. Defaults to
|
||||
* `false` until a portal entry/api is actually registered for it, rather
|
||||
* than assuming visibility during the brief acquire/mount race or after a
|
||||
* panel has been detached: a false negative just delays marking the session
|
||||
* read by a render, while a false positive could mark it read too early.
|
||||
* Non-dockview hosts (mobile, quick chat, kanban preview) never acquire a
|
||||
* portal entry — they should pass a literal `true`/`false` instead of
|
||||
* calling this hook.
|
||||
*/
|
||||
export function usePanelActive(panelId: string): boolean {
|
||||
const subscribe = useCallback(
|
||||
(onStoreChange: () => void) => {
|
||||
// Tracks which api our onDidActiveChange listener is currently bound
|
||||
// to, so a manager notification (panel acquired/released/remounted
|
||||
// anywhere) can detect when *this* panelId's api was replaced —
|
||||
// Dockview does this during a layout restore/switch — and rebind
|
||||
// rather than keep listening on the now-stale api.
|
||||
let boundApi: DockviewPanelApi | null = null;
|
||||
let disposable: { dispose: () => void } | null = null;
|
||||
|
||||
const rebindIfApiChanged = () => {
|
||||
const api = panelPortalManager.get(panelId)?.api ?? null;
|
||||
if (api === boundApi) return;
|
||||
disposable?.dispose();
|
||||
boundApi = api;
|
||||
disposable = api?.onDidActiveChange(onStoreChange) ?? null;
|
||||
};
|
||||
|
||||
rebindIfApiChanged();
|
||||
const unsubscribeManager = panelPortalManager.subscribe(() => {
|
||||
rebindIfApiChanged();
|
||||
onStoreChange();
|
||||
});
|
||||
return () => {
|
||||
unsubscribeManager();
|
||||
disposable?.dispose();
|
||||
};
|
||||
},
|
||||
[panelId],
|
||||
);
|
||||
const getSnapshot = useCallback(
|
||||
() => panelPortalManager.get(panelId)?.api?.isActive ?? false,
|
||||
[panelId],
|
||||
);
|
||||
return useSyncExternalStore(subscribe, getSnapshot, () => false);
|
||||
}
|
||||
@@ -98,6 +98,7 @@ function carryForwardAppStatusSettings(current: DisplaySettings) {
|
||||
function carryForwardTaskActionSettings(current: DisplaySettings) {
|
||||
return {
|
||||
confirmTaskArchive: current.confirmTaskArchive ?? true,
|
||||
unreadDivider: current.unreadDivider ?? true,
|
||||
mcpTaskAgentProfileDefault: current.mcpTaskAgentProfileDefault ?? "current_task",
|
||||
};
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import { fetchJson, type ApiRequestOptions } from "../client";
|
||||
import type {
|
||||
TaskSessionsResponse,
|
||||
TaskSessionResponse,
|
||||
MarkSessionReadResponse,
|
||||
ListMessagesResponse,
|
||||
ListTurnsResponse,
|
||||
} from "@/lib/types/http";
|
||||
@@ -84,6 +85,30 @@ export async function dismissLastAgentError(
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Advances the session's Slack-style read cursor to messageId. Called when a
|
||||
* session becomes the visible chat panel — the caller must snapshot the
|
||||
* session's PRIOR last_read_message_id (to position the unread divider)
|
||||
* before invoking this. The response is intentionally narrow (just the
|
||||
* updated cursor, not a full session) — apply it with a narrow store update
|
||||
* (e.g. updateSessionReadCursor), never a full-session merge, so it can't
|
||||
* clobber a newer field written by a concurrent WebSocket update.
|
||||
*/
|
||||
export async function markSessionRead(
|
||||
taskSessionId: string,
|
||||
messageId: string,
|
||||
options?: ApiRequestOptions,
|
||||
) {
|
||||
return fetchJson<MarkSessionReadResponse>(`/api/v1/task-sessions/${taskSessionId}/mark-read`, {
|
||||
...options,
|
||||
init: {
|
||||
...(options?.init ?? {}),
|
||||
method: "POST",
|
||||
body: JSON.stringify({ message_id: messageId }),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
export async function listTaskSessionMessages(
|
||||
taskSessionId: string,
|
||||
params?: { limit?: number; before?: string; after?: string; sort?: "asc" | "desc" },
|
||||
|
||||
@@ -43,3 +43,34 @@ describe("PanelPortalManager.reconcile", () => {
|
||||
expect(listener).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
|
||||
describe("PanelPortalManager.acquire", () => {
|
||||
it("notifies listeners when a remount replaces an existing entry's api", () => {
|
||||
const mgr = new PanelPortalManager();
|
||||
const listener = vi.fn();
|
||||
const firstApi = mockApi();
|
||||
mgr.acquire("panel-a", "chat", {}, firstApi);
|
||||
mgr.subscribe(listener);
|
||||
listener.mockClear();
|
||||
|
||||
const secondApi = mockApi();
|
||||
mgr.acquire("panel-a", "chat", {}, secondApi);
|
||||
|
||||
expect(listener).toHaveBeenCalledOnce();
|
||||
expect(mgr.get("panel-a")?.api).toBe(secondApi);
|
||||
});
|
||||
|
||||
it("does not notify when a redundant acquire reuses the same api instance", () => {
|
||||
const mgr = new PanelPortalManager();
|
||||
const listener = vi.fn();
|
||||
const api = mockApi();
|
||||
mgr.acquire("panel-a", "chat", {}, api);
|
||||
mgr.subscribe(listener);
|
||||
listener.mockClear();
|
||||
|
||||
mgr.acquire("panel-a", "chat", { updated: true }, api);
|
||||
|
||||
expect(listener).not.toHaveBeenCalled();
|
||||
expect(mgr.get("panel-a")?.params).toEqual({ updated: true });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -96,10 +96,20 @@ export class PanelPortalManager {
|
||||
this.version++;
|
||||
this.notify();
|
||||
} else {
|
||||
// Panel remounted after fromJSON — update api & params
|
||||
// Panel remounted after fromJSON — update api & params. Notify only
|
||||
// when the api handle itself was replaced (not on every redundant
|
||||
// acquire with the same api): consumers like usePanelActive bind an
|
||||
// onDidActiveChange listener directly to the api instance and must
|
||||
// learn about a swap to rebind, since the api itself carries no
|
||||
// "I've been replaced" signal of its own.
|
||||
const apiChanged = entry.api !== api;
|
||||
entry.api = api;
|
||||
entry.params = params;
|
||||
entry.component = component;
|
||||
if (apiChanged) {
|
||||
this.version++;
|
||||
this.notify();
|
||||
}
|
||||
}
|
||||
return entry;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { sessionId as toSessionId, taskId as toTaskId, type Message } from "@/lib/types/http";
|
||||
import type { RenderItem } from "@/hooks/use-processed-messages";
|
||||
import { findUnreadDividerItemId, lastRenderedMessageId } from "./session-unread-divider";
|
||||
|
||||
function makeMessage(id: string, content = ""): Message {
|
||||
return {
|
||||
id,
|
||||
session_id: toSessionId("s1"),
|
||||
task_id: toTaskId("t1"),
|
||||
author_type: "agent",
|
||||
content,
|
||||
type: "message",
|
||||
created_at: "",
|
||||
};
|
||||
}
|
||||
|
||||
const messageItem = (message: Message): RenderItem => ({ type: "message", message });
|
||||
const turnGroup = (id: string, messages: Message[]): RenderItem => ({
|
||||
type: "turn_group",
|
||||
id,
|
||||
turnId: messages[0]?.turn_id ?? null,
|
||||
messages,
|
||||
});
|
||||
const prepareProgress = (id: string): RenderItem => ({
|
||||
type: "prepare_progress",
|
||||
id,
|
||||
sessionId: "s1",
|
||||
});
|
||||
|
||||
describe("findUnreadDividerItemId", () => {
|
||||
it("returns null when there is no read cursor yet", () => {
|
||||
const items = [messageItem(makeMessage("m1"))];
|
||||
expect(findUnreadDividerItemId(items, null)).toBeNull();
|
||||
expect(findUnreadDividerItemId(items, undefined)).toBeNull();
|
||||
expect(findUnreadDividerItemId(items, "")).toBeNull();
|
||||
});
|
||||
|
||||
it("places the divider before the next standalone message after the cursor", () => {
|
||||
const items = [
|
||||
messageItem(makeMessage("m1")),
|
||||
messageItem(makeMessage("m2")),
|
||||
messageItem(makeMessage("m3")),
|
||||
];
|
||||
expect(findUnreadDividerItemId(items, "m1")).toBe("m2");
|
||||
});
|
||||
|
||||
it("returns null when the cursor already points at the newest message", () => {
|
||||
const items = [messageItem(makeMessage("m1")), messageItem(makeMessage("m2"))];
|
||||
expect(findUnreadDividerItemId(items, "m2")).toBeNull();
|
||||
});
|
||||
|
||||
it("treats a turn_group as a single unit — a cursor inside the group places the divider after the whole group, not mid-group", () => {
|
||||
const a1 = makeMessage("a1");
|
||||
const a2 = makeMessage("a2");
|
||||
const a3 = makeMessage("a3");
|
||||
const items = [turnGroup("turn-group-a1", [a1, a2, a3]), messageItem(makeMessage("m-after"))];
|
||||
// Cursor sits on the middle message of the group — the divider must not
|
||||
// split the group, so it lands before the next item entirely.
|
||||
expect(findUnreadDividerItemId(items, "a2")).toBe("m-after");
|
||||
});
|
||||
|
||||
it("returns null when the cursor is the last message inside the newest turn_group", () => {
|
||||
const a1 = makeMessage("a1");
|
||||
const a2 = makeMessage("a2");
|
||||
const items = [turnGroup("turn-group-a1", [a1, a2])];
|
||||
expect(findUnreadDividerItemId(items, "a2")).toBeNull();
|
||||
});
|
||||
|
||||
it("skips non-message items (prepare_progress) when locating the next unread message", () => {
|
||||
const items = [
|
||||
messageItem(makeMessage("m1")),
|
||||
prepareProgress("prepare-1"),
|
||||
messageItem(makeMessage("m2")),
|
||||
];
|
||||
expect(findUnreadDividerItemId(items, "m1")).toBe("m2");
|
||||
});
|
||||
|
||||
it("treats a cursor missing from the loaded window as the whole window being unread, placing the divider before its first message", () => {
|
||||
// Simulates returning to an old task: the persisted cursor points at a
|
||||
// message far older than what pagination has loaded so far.
|
||||
const items = [messageItem(makeMessage("m10")), messageItem(makeMessage("m11"))];
|
||||
expect(findUnreadDividerItemId(items, "m1-not-loaded")).toBe("m10");
|
||||
});
|
||||
|
||||
it("ignores the synthetic task-description placeholder as a cursor boundary", () => {
|
||||
// A CREATED session with no real messages yet renders only the
|
||||
// frontend-only task-description placeholder. It must never satisfy a
|
||||
// real lastReadMessageId lookup or be treated as a message to place the
|
||||
// divider after.
|
||||
const items = [messageItem(makeMessage("task-description"))];
|
||||
expect(findUnreadDividerItemId(items, "m1")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("lastRenderedMessageId", () => {
|
||||
it("returns null when there are no items", () => {
|
||||
expect(lastRenderedMessageId([])).toBeNull();
|
||||
});
|
||||
|
||||
it("returns the last standalone message's id", () => {
|
||||
const items = [messageItem(makeMessage("m1")), messageItem(makeMessage("m2"))];
|
||||
expect(lastRenderedMessageId(items)).toBe("m2");
|
||||
});
|
||||
|
||||
it("skips trailing non-message items (prepare_progress) to find the last real message", () => {
|
||||
const items = [messageItem(makeMessage("m1")), prepareProgress("prepare-1")];
|
||||
expect(lastRenderedMessageId(items)).toBe("m1");
|
||||
});
|
||||
|
||||
it("returns the last message inside a trailing turn_group", () => {
|
||||
const items = [
|
||||
messageItem(makeMessage("m1")),
|
||||
turnGroup("turn-group-a1", [makeMessage("a1"), makeMessage("a2")]),
|
||||
];
|
||||
expect(lastRenderedMessageId(items)).toBe("a2");
|
||||
});
|
||||
|
||||
it("is unaffected by a trailing item that never rendered (e.g. filtered from the backend list)", () => {
|
||||
// Mirrors the bug findUnreadDividerItemId's caller must avoid: the raw
|
||||
// backend list's newest row (a hidden clarification/status/setup-output
|
||||
// message) never became a render item, so it must not be reported here.
|
||||
const items = [messageItem(makeMessage("m1")), messageItem(makeMessage("m2"))];
|
||||
expect(lastRenderedMessageId(items)).toBe("m2");
|
||||
expect(lastRenderedMessageId(items)).not.toBe("m3-hidden-from-render");
|
||||
});
|
||||
|
||||
it("returns null for a CREATED session with only the synthetic task-description placeholder", () => {
|
||||
// use-processed-messages.ts injects this placeholder only when there are
|
||||
// zero real messages. It must never be reported as "the latest message"
|
||||
// — doing so would make the mark-read call target a message id that
|
||||
// doesn't exist in the backend on every visit to a brand-new session.
|
||||
const items = [messageItem(makeMessage("task-description"))];
|
||||
expect(lastRenderedMessageId(items)).toBeNull();
|
||||
});
|
||||
|
||||
it("skips a trailing synthetic task-description placeholder to find the last real message", () => {
|
||||
const items = [messageItem(makeMessage("m1")), messageItem(makeMessage("task-description"))];
|
||||
expect(lastRenderedMessageId(items)).toBe("m1");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,92 @@
|
||||
import type { RenderItem } from "@/hooks/use-processed-messages";
|
||||
|
||||
/** Message ids carried by a render item — a "message" item carries its own
|
||||
* id, a "turn_group" carries every message bundled into that visual group,
|
||||
* and non-message items (prepare_progress, agent_error_notice) carry none.
|
||||
*
|
||||
* The frontend-only "task-description" placeholder (injected by
|
||||
* use-processed-messages.ts when a session has no real messages yet) is
|
||||
* excluded too: it never exists as a row in the backend messages table, so
|
||||
* treating it as a real message id would make lastRenderedMessageId return
|
||||
* "task-description" as the cursor target — which the backend's
|
||||
* MarkSessionRead correctly rejects as an unknown message id (400), but
|
||||
* only after a wasted round trip logged as a console error on every visit
|
||||
* to a session with no messages yet. */
|
||||
function renderItemMessageIds(item: RenderItem): string[] {
|
||||
if (item.type === "message") {
|
||||
return item.message.id === "task-description" ? [] : [item.message.id];
|
||||
}
|
||||
if (item.type === "turn_group") {
|
||||
return item.messages.map((message) => message.id).filter((id) => id !== "task-description");
|
||||
}
|
||||
return [];
|
||||
}
|
||||
|
||||
/**
|
||||
* Locates the Slack-style unread ("New") divider boundary: the key of the
|
||||
* render item the divider should appear immediately before.
|
||||
*
|
||||
* Finds the item containing lastReadMessageId, then returns the key of the
|
||||
* next item that carries a real message. When lastReadMessageId falls inside
|
||||
* a turn_group (several messages bundled into one visual group), the whole
|
||||
* group counts as read and the divider lands after it — a group is never
|
||||
* split into read/unread halves.
|
||||
*
|
||||
* When lastReadMessageId isn't found among the currently loaded items, the
|
||||
* loaded window is treated as entirely unread — the divider goes before its
|
||||
* first real message — rather than suppressed. The common cause is an older
|
||||
* cursor pointing past the currently paginated window (the loaded messages
|
||||
* are the transcript's tail; older history hasn't been fetched yet), which
|
||||
* means everything visible really is unread. Returns null only when there is
|
||||
* no cursor at all (a brand-new session has nothing to mark "New" against),
|
||||
* or the cursor already points at the newest loaded message.
|
||||
*
|
||||
* The returned key mirrors message-list-shared.tsx's getItemKey format
|
||||
* (bare message id, or the item's own id for non-message items) — duplicated
|
||||
* inline rather than imported to avoid a circular import, since that module
|
||||
* imports RenderItem from hooks/use-processed-messages.ts. Keep the two in
|
||||
* sync.
|
||||
*/
|
||||
export function findUnreadDividerItemId(
|
||||
items: RenderItem[],
|
||||
lastReadMessageId: string | null | undefined,
|
||||
): string | null {
|
||||
if (!lastReadMessageId) return null;
|
||||
|
||||
const boundaryIndex = items.findIndex((item) =>
|
||||
renderItemMessageIds(item).includes(lastReadMessageId),
|
||||
);
|
||||
const searchFrom = boundaryIndex === -1 ? 0 : boundaryIndex + 1;
|
||||
|
||||
for (let i = searchFrom; i < items.length; i++) {
|
||||
const item = items[i];
|
||||
if (renderItemMessageIds(item).length > 0) {
|
||||
return item.type === "message" ? item.message.id : item.id;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the id of the newest message actually represented among items —
|
||||
* i.e. the last message that made it into the rendered transcript — or null
|
||||
* if items carries no message at all.
|
||||
*
|
||||
* Callers deriving the Slack-style read-cursor's "latest message" must use
|
||||
* this instead of the raw backend message list's last row: some backend
|
||||
* rows never render (a pending `clarification_request`, a hidden "Session
|
||||
* resumed" status, setup script output, a collapsed todo snapshot, ...), so
|
||||
* the raw list's tail can be a message findUnreadDividerItemId will never
|
||||
* find among items — which it then (correctly, per its own contract) treats
|
||||
* as "the whole loaded window is unread", drawing the divider before
|
||||
* everything even though nothing visible is actually new. Deriving the
|
||||
* cursor from the same render-item universe findUnreadDividerItemId
|
||||
* searches keeps both sides in sync.
|
||||
*/
|
||||
export function lastRenderedMessageId(items: RenderItem[]): string | null {
|
||||
for (let i = items.length - 1; i >= 0; i--) {
|
||||
const ids = renderItemMessageIds(items[i]);
|
||||
if (ids.length > 0) return ids[ids.length - 1];
|
||||
}
|
||||
return null;
|
||||
}
|
||||
@@ -248,6 +248,24 @@ describe("mapUserSettingsResponse", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("mapUserSettingsResponse unread divider", () => {
|
||||
it("defaults the preference to enabled and preserves an explicit disable", () => {
|
||||
const fallback = mapUserSettingsResponse(null) as { unreadDivider?: boolean };
|
||||
const disabled = mapUserSettingsResponse({
|
||||
settings: {
|
||||
user_id: DEFAULT_USER_ID,
|
||||
workspace_id: toWorkspaceId(""),
|
||||
repository_ids: [],
|
||||
unread_divider: false,
|
||||
updated_at: UPDATED_AT,
|
||||
} as unknown as NonNullable<Parameters<typeof mapUserSettingsResponse>[0]>["settings"],
|
||||
}) as { unreadDivider?: boolean };
|
||||
|
||||
expect(fallback.unreadDivider).toBe(true);
|
||||
expect(disabled.unreadDivider).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("transcript navigation settings", () => {
|
||||
it("uses the requested defaults when settings are unavailable", () => {
|
||||
const settings = mapUserSettingsResponse(null);
|
||||
|
||||
@@ -126,6 +126,7 @@ function buildBehaviorFields(s: UserSettingsData) {
|
||||
chatSubmitKey: s.chat_submit_key ?? "cmd_enter",
|
||||
reviewAutoMarkOnScroll: s.review_auto_mark_on_scroll ?? true,
|
||||
confirmTaskArchive: s.confirm_task_archive ?? true,
|
||||
unreadDivider: s.unread_divider ?? true,
|
||||
mcpTaskAgentProfileDefault: parseMCPTaskAgentProfileDefault(s.mcp_task_agent_profile_default),
|
||||
showAnchoredPromptBar: s.show_anchored_prompt_bar ?? false,
|
||||
showScrollToLastPrompt: s.show_scroll_to_last_prompt ?? true,
|
||||
@@ -192,6 +193,7 @@ export function mapUserSettingsResponse(response: UserSettingsResponse | null) {
|
||||
chatSubmitKey: "cmd_enter" as const,
|
||||
reviewAutoMarkOnScroll: true,
|
||||
confirmTaskArchive: true,
|
||||
unreadDivider: true,
|
||||
mcpTaskAgentProfileDefault: "current_task" as const,
|
||||
showAnchoredPromptBar: false,
|
||||
showScrollToLastPrompt: true,
|
||||
|
||||
@@ -483,6 +483,17 @@ function buildTaskSessionActions(set: ImmerSet) {
|
||||
syncEnvironmentMapping(draft, session.id, mergedSession.task_environment_id);
|
||||
reconcileActiveTurnForIdleSession(draft, mergedSession);
|
||||
}),
|
||||
updateSessionReadCursor: (sessionId: string, lastReadMessageId: string) =>
|
||||
set((draft) => {
|
||||
const session = draft.taskSessions.items[sessionId];
|
||||
if (!session) return;
|
||||
session.last_read_message_id = lastReadMessageId;
|
||||
const sessionsByTask = draft.taskSessionsByTask.itemsByTaskId[session.task_id];
|
||||
if (sessionsByTask) {
|
||||
const match = sessionsByTask.find((s) => s.id === sessionId);
|
||||
if (match) match.last_read_message_id = lastReadMessageId;
|
||||
}
|
||||
}),
|
||||
removeTaskSession: (taskId: string, sessionId: string) =>
|
||||
set((draft) => {
|
||||
delete draft.taskSessions.items[sessionId];
|
||||
|
||||
@@ -209,6 +209,15 @@ export type SessionSliceActions = {
|
||||
/** Source adoption is an authoritative idle boundary for the listed sessions. */
|
||||
reconcileWorkspaceSourcesAdopted: (sessionIds: string[]) => void;
|
||||
setTaskSession: (session: TaskSession) => void;
|
||||
/**
|
||||
* Narrowly updates only a session's Slack-style read cursor
|
||||
* (last_read_message_id) — never the full session object. Used for the
|
||||
* mark-read HTTP response, whose full-session snapshot is frozen at
|
||||
* write time and can otherwise clobber a newer WS session.state_changed
|
||||
* update that arrives while the request is still in flight. No-ops if
|
||||
* the session isn't in the store (never creates a bare session record).
|
||||
*/
|
||||
updateSessionReadCursor: (sessionId: string, lastReadMessageId: string) => void;
|
||||
removeTaskSession: (taskId: string, sessionId: string) => void;
|
||||
setTaskSessionsForTask: (taskId: string, sessions: TaskSession[]) => void;
|
||||
upsertTaskSessionFromEvent: (taskId: string, session: TaskSession) => void;
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { create } from "zustand";
|
||||
import { immer } from "zustand/middleware/immer";
|
||||
import { createSessionSlice } from "./session-slice";
|
||||
import { createSessionRuntimeSlice } from "../session-runtime/session-runtime-slice";
|
||||
import type { SessionSlice } from "./types";
|
||||
import type { SessionRuntimeSlice } from "../session-runtime/types";
|
||||
import { sessionId as toSessionId, taskId as toTaskId, type TaskSession } from "@/lib/types/http";
|
||||
|
||||
type CombinedSlice = SessionSlice & SessionRuntimeSlice;
|
||||
|
||||
function makeStore() {
|
||||
return create<CombinedSlice>()(
|
||||
immer((set) => ({
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
...(createSessionSlice as any)(set),
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
...(createSessionRuntimeSlice as any)(set),
|
||||
})),
|
||||
);
|
||||
}
|
||||
|
||||
const TASK_ID = toTaskId("task-1");
|
||||
const SESSION_ID = toSessionId("session-1");
|
||||
const TS = "2026-04-20T00:00:00Z";
|
||||
|
||||
function makeSession(overrides: Partial<TaskSession> = {}): TaskSession {
|
||||
return {
|
||||
id: SESSION_ID,
|
||||
task_id: TASK_ID,
|
||||
state: "RUNNING",
|
||||
started_at: TS,
|
||||
updated_at: TS,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
// updateSessionReadCursor exists specifically so the mark-read HTTP response
|
||||
// — a full-session snapshot frozen at request time — never overwrites a
|
||||
// field (e.g. state) that a concurrent WebSocket update wrote to the store
|
||||
// while the request was still in flight. It must touch ONLY
|
||||
// last_read_message_id.
|
||||
describe("updateSessionReadCursor", () => {
|
||||
it("updates only last_read_message_id, leaving every other field untouched", () => {
|
||||
const store = makeStore();
|
||||
store.setState((draft) => {
|
||||
draft.taskSessions.items[SESSION_ID] = makeSession({ last_read_message_id: "m1" });
|
||||
});
|
||||
|
||||
store.getState().updateSessionReadCursor(SESSION_ID, "m2");
|
||||
|
||||
const session = store.getState().taskSessions.items[SESSION_ID];
|
||||
expect(session.last_read_message_id).toBe("m2");
|
||||
expect(session.state).toBe("RUNNING");
|
||||
expect(session.updated_at).toBe(TS);
|
||||
});
|
||||
|
||||
it("does not clobber a newer field written by a concurrent update while the request was in flight", () => {
|
||||
const store = makeStore();
|
||||
store.setState((draft) => {
|
||||
draft.taskSessions.items[SESSION_ID] = makeSession({ last_read_message_id: "m1" });
|
||||
});
|
||||
|
||||
// Simulate a WS session.state_changed landing after the mark-read
|
||||
// request was dispatched but before its (now-stale) response resolves.
|
||||
store.getState().setTaskSession(makeSession({ state: "WAITING_FOR_INPUT" }));
|
||||
// The mark-read response resolves afterward, carrying only the cursor.
|
||||
store.getState().updateSessionReadCursor(SESSION_ID, "m1");
|
||||
|
||||
const session = store.getState().taskSessions.items[SESSION_ID];
|
||||
expect(session.state).toBe("WAITING_FOR_INPUT");
|
||||
expect(session.last_read_message_id).toBe("m1");
|
||||
});
|
||||
|
||||
it("is a no-op when the session isn't in the store (never creates a bare record)", () => {
|
||||
const store = makeStore();
|
||||
|
||||
store.getState().updateSessionReadCursor(SESSION_ID, "m2");
|
||||
|
||||
expect(store.getState().taskSessions.items[SESSION_ID]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("mirrors the update into the per-task session list", () => {
|
||||
const store = makeStore();
|
||||
const session = makeSession({ last_read_message_id: "m1" });
|
||||
store.setState((draft) => {
|
||||
draft.taskSessions.items[SESSION_ID] = session;
|
||||
draft.taskSessionsByTask.itemsByTaskId[TASK_ID] = [session];
|
||||
});
|
||||
|
||||
store.getState().updateSessionReadCursor(SESSION_ID, "m2");
|
||||
|
||||
const listed = store.getState().taskSessionsByTask.itemsByTaskId[TASK_ID]?.[0];
|
||||
expect(listed?.last_read_message_id).toBe("m2");
|
||||
});
|
||||
});
|
||||
@@ -38,6 +38,7 @@ export const defaultSettingsState: SettingsSliceState = {
|
||||
chatSubmitKey: "cmd_enter",
|
||||
reviewAutoMarkOnScroll: true,
|
||||
confirmTaskArchive: true,
|
||||
unreadDivider: true,
|
||||
mcpTaskAgentProfileDefault: "current_task",
|
||||
showAnchoredPromptBar: false,
|
||||
showScrollToLastPrompt: true,
|
||||
|
||||
@@ -180,6 +180,7 @@ export type UserSettingsState = {
|
||||
chatSubmitKey: "enter" | "cmd_enter";
|
||||
reviewAutoMarkOnScroll: boolean;
|
||||
confirmTaskArchive: boolean;
|
||||
unreadDivider: boolean;
|
||||
mcpTaskAgentProfileDefault: MCPTaskAgentProfileDefault;
|
||||
showAnchoredPromptBar: boolean;
|
||||
showScrollToLastPrompt: boolean;
|
||||
|
||||
@@ -383,6 +383,7 @@ export type AppState = KanbanSlice & {
|
||||
) => void;
|
||||
setMessagesLoading: (sessionId: string, loading: boolean) => void;
|
||||
setTaskSession: (session: TaskSession) => void;
|
||||
updateSessionReadCursor: (sessionId: string, lastReadMessageId: string) => void;
|
||||
removeTaskSession: (taskId: string, sessionId: string) => void;
|
||||
setTaskSessionsForTask: (taskId: string, sessions: TaskSession[]) => void;
|
||||
upsertTaskSessionFromEvent: (taskId: string, session: TaskSession) => void;
|
||||
|
||||
@@ -415,6 +415,7 @@ export type UserSettingsUpdatedPayload = {
|
||||
show_transcript_auto_scroll_control?: boolean;
|
||||
review_auto_mark_on_scroll?: boolean;
|
||||
confirm_task_archive?: boolean;
|
||||
unread_divider?: boolean;
|
||||
mcp_task_agent_profile_default?: MCPTaskAgentProfileDefault;
|
||||
show_release_notification?: boolean;
|
||||
release_notes_last_seen_version?: string;
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
import type { TaskSession } from "./http";
|
||||
import type { WorkflowStepDTO } from "./http";
|
||||
|
||||
export type TaskSessionsResponse = {
|
||||
sessions: TaskSession[];
|
||||
total: number;
|
||||
};
|
||||
|
||||
export type TaskSessionResponse = {
|
||||
session: TaskSession;
|
||||
};
|
||||
|
||||
/**
|
||||
* Response for POST /task-sessions/:id/mark-read. Intentionally narrow —
|
||||
* only the advanced read cursor, never a full session snapshot — so
|
||||
* applying it can never clobber a newer field (e.g. state) written by a
|
||||
* concurrent WebSocket update while the request was in flight.
|
||||
*/
|
||||
export type MarkSessionReadResponse = {
|
||||
session_id: string;
|
||||
last_read_message_id: string;
|
||||
};
|
||||
|
||||
export type ApproveSessionResponse = {
|
||||
success: boolean;
|
||||
session: TaskSession;
|
||||
workflow_step?: WorkflowStepDTO;
|
||||
};
|
||||
@@ -65,6 +65,7 @@ export type UserSettings = {
|
||||
show_transcript_auto_scroll_control?: boolean;
|
||||
review_auto_mark_on_scroll?: boolean;
|
||||
confirm_task_archive?: boolean;
|
||||
unread_divider?: boolean;
|
||||
mcp_task_agent_profile_default?: MCPTaskAgentProfileDefault;
|
||||
show_release_notification?: boolean;
|
||||
release_notes_last_seen_version?: string;
|
||||
@@ -118,6 +119,7 @@ export type UserSettingsUpdatePayload = {
|
||||
show_transcript_auto_scroll_control?: boolean;
|
||||
review_auto_mark_on_scroll?: boolean;
|
||||
confirm_task_archive?: boolean;
|
||||
unread_divider?: boolean;
|
||||
mcp_task_agent_profile_default?: MCPTaskAgentProfileDefault;
|
||||
show_release_notification?: boolean;
|
||||
release_notes_last_seen_version?: string;
|
||||
|
||||
@@ -436,22 +436,17 @@ export type TaskSession = ActiveSubagentCountFields & {
|
||||
review_status?: WorkflowReviewStatus;
|
||||
// Server-resolved tool_call count, populated by ListTaskSessions.
|
||||
command_count?: number;
|
||||
// Slack-style read cursor: the id of the newest message the frontend has
|
||||
// marked as read. Used to position the unread ("New") divider.
|
||||
last_read_message_id?: string;
|
||||
};
|
||||
|
||||
export type TaskSessionsResponse = {
|
||||
sessions: TaskSession[];
|
||||
total: number;
|
||||
};
|
||||
|
||||
export type TaskSessionResponse = {
|
||||
session: TaskSession;
|
||||
};
|
||||
|
||||
export type ApproveSessionResponse = {
|
||||
success: boolean;
|
||||
session: TaskSession;
|
||||
workflow_step?: WorkflowStepDTO;
|
||||
};
|
||||
export type {
|
||||
TaskSessionsResponse,
|
||||
TaskSessionResponse,
|
||||
MarkSessionReadResponse,
|
||||
ApproveSessionResponse,
|
||||
} from "./http-session-responses";
|
||||
|
||||
export type NotificationProviderType = "local" | "apprise" | "system";
|
||||
|
||||
|
||||
@@ -74,6 +74,16 @@ function buildSyncedLocalSettings(state: AppState, payload: UserSettingsUpdatedP
|
||||
};
|
||||
}
|
||||
|
||||
function buildTranscriptBehaviorSettings(payload: UserSettingsUpdatedPayload) {
|
||||
return {
|
||||
unreadDivider: payload.unread_divider ?? true,
|
||||
showAnchoredPromptBar: payload.show_anchored_prompt_bar ?? false,
|
||||
showScrollToLastPrompt: payload.show_scroll_to_last_prompt ?? true,
|
||||
showScrollToStart: payload.show_scroll_to_start ?? false,
|
||||
showTranscriptAutoScrollControl: payload.show_transcript_auto_scroll_control ?? true,
|
||||
};
|
||||
}
|
||||
|
||||
function buildBehaviorSettings(state: AppState, payload: UserSettingsUpdatedPayload) {
|
||||
return {
|
||||
preferredShell: payload.preferred_shell || null,
|
||||
@@ -85,10 +95,7 @@ function buildBehaviorSettings(state: AppState, payload: UserSettingsUpdatedPayl
|
||||
mcpTaskAgentProfileDefault: parseMCPTaskAgentProfileDefault(
|
||||
payload.mcp_task_agent_profile_default,
|
||||
),
|
||||
showAnchoredPromptBar: payload.show_anchored_prompt_bar ?? false,
|
||||
showScrollToLastPrompt: payload.show_scroll_to_last_prompt ?? true,
|
||||
showScrollToStart: payload.show_scroll_to_start ?? false,
|
||||
showTranscriptAutoScrollControl: payload.show_transcript_auto_scroll_control ?? true,
|
||||
...buildTranscriptBehaviorSettings(payload),
|
||||
showReleaseNotification: payload.show_release_notification ?? true,
|
||||
releaseNotesLastSeenVersion: (payload.release_notes_last_seen_version as string) || null,
|
||||
terminalLinkBehavior:
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
---
|
||||
spec: docs/specs/office/unread-divider.md
|
||||
created: 2026-07-31
|
||||
status: pending-approval
|
||||
---
|
||||
|
||||
# Implementation Plan: Suppress markers created during active visits
|
||||
|
||||
## Root cause
|
||||
|
||||
`useSessionReadTracking` captures a frozen visit anchor even when the
|
||||
persisted cursor already equals the transcript tail. `findUnreadDividerItemId`
|
||||
correctly returns no divider at that instant because there is no following
|
||||
message. When the user sends a prompt or the agent emits a message while the
|
||||
same panel remains visible, that stale tail anchor gains a following row and
|
||||
the renderer places a "New" divider before it. No visibility transition or
|
||||
late session metadata fetch is required.
|
||||
|
||||
## Decision
|
||||
|
||||
Determine divider eligibility from the immutable cursor captured at visit
|
||||
entry and the first fully loaded rendered transcript. A visit that begins at
|
||||
the current tail, or without a cursor, is permanently divider-ineligible
|
||||
until the user leaves and returns. Preserve a frozen anchor only when unread
|
||||
content existed in that initial, loaded transcript. The live read cursor
|
||||
continues advancing independently. Readiness must be a one-time
|
||||
initial-message-load signal, not the broad `messagesLoading` flag, which also
|
||||
covers pagination and refreshes.
|
||||
|
||||
## Frontend
|
||||
|
||||
- `apps/web/hooks/domains/session/use-session-messages.ts`: expose the
|
||||
existing `isWaitingForInitialMessages` state as a dedicated
|
||||
initial-message readiness result without conflating later pagination or
|
||||
background fetches.
|
||||
- `apps/web/components/task/chat/use-chat-panel-state.ts` and
|
||||
`apps/web/components/task/task-chat-panel.tsx`: propagate that dedicated
|
||||
initial-load signal to read tracking alongside the latest rendered message
|
||||
id.
|
||||
- `apps/web/components/task/chat/use-session-read-tracking.ts`: retain the
|
||||
prior cursor at the visibility transition and latch eligibility exactly once
|
||||
when the initial message load settles. Create an anchor only if that cursor
|
||||
precedes the initial tail; make no anchor for an empty cursor, an empty
|
||||
initial transcript, or a cursor equal to the tail. Keep the stale-response
|
||||
and leave/re-enter behavior. Do not derive a new divider from later
|
||||
`latestMessageId` changes.
|
||||
- `apps/web/components/task/chat/use-session-read-tracking.test.ts`: add
|
||||
regressions for (a) cursor-at-tail followed by a user prompt while
|
||||
continuously visible, and (b) a delayed initial message load that settles
|
||||
at the captured cursor. Both must remain divider-ineligible. Retain the
|
||||
existing true-unread and stale-response tests.
|
||||
|
||||
## E2E Tests
|
||||
|
||||
- `apps/web/e2e/tests/chat/unread-divider.spec.ts`: seed a session whose
|
||||
persisted cursor is already the visible tail, open it, send a prompt while
|
||||
staying on that chat panel, and assert no divider appears before the prompt.
|
||||
Retain the existing rewound-cursor flow to prove real pre-existing unread
|
||||
messages still render the divider.
|
||||
- `apps/web/e2e/tests/chat/mobile-unread-divider.spec.ts`: add the same
|
||||
tail-then-prompt assertion under the native mobile chat layout. This is the
|
||||
same user outcome on the shared hook, with a distinct mobile composition.
|
||||
|
||||
## Verification
|
||||
|
||||
1. Run the focused hook test file during Red-Green-Refactor.
|
||||
2. Run the desktop and mobile unread-divider Playwright specs through
|
||||
`pnpm e2e:run` after a production rebuild.
|
||||
3. Run `pnpm run typecheck` from `apps/web`.
|
||||
|
||||
## Implementation wave
|
||||
|
||||
1. [task-01-clear-active-visit-marker](task-01-clear-active-visit-marker.md)
|
||||
— sequential; one readiness-aware shared-hook change with desktop and
|
||||
mobile regression coverage.
|
||||
@@ -0,0 +1,44 @@
|
||||
---
|
||||
status: pending-approval
|
||||
---
|
||||
|
||||
# Task 01: Suppress markers created during active visits
|
||||
|
||||
## Scope
|
||||
|
||||
Repair the unread-divider eligibility lifecycle only. Do not change the
|
||||
read-cursor schema, API, feature setting, pagination behavior, or unrelated
|
||||
transcript scrolling.
|
||||
|
||||
## Steps
|
||||
|
||||
1. Extend `use-session-read-tracking.test.ts` with the exact regression:
|
||||
start a visible visit where the stored cursor and latest rendered message
|
||||
are both `m1`, then add `m2` while visibility never changes. Assert the
|
||||
derived divider remains absent. Add the delayed-initial-load variant and
|
||||
run the focused test to observe both failures before implementation.
|
||||
2. Expose the existing `isWaitingForInitialMessages` state through
|
||||
`useSessionMessages` and `useChatPanelState` as a dedicated initial-load
|
||||
signal. Do not reuse `messagesLoading`, because it also covers pagination
|
||||
and later background refreshes.
|
||||
3. Pass that signal to `useSessionReadTracking`. Retain the prior cursor at
|
||||
the visibility transition, then latch visit eligibility once the initial
|
||||
load settles: empty cursor, empty initial transcript, and cursor-equals-tail
|
||||
must remain divider-ineligible for the visit. Preserve frozen anchors for
|
||||
genuine pre-existing unread content and the stale-response guard.
|
||||
4. Add deterministic desktop and mobile Playwright coverage for opening at
|
||||
the current tail, sending a prompt without leaving the chat panel, and
|
||||
observing no divider before that prompt. Keep the existing genuine-unread
|
||||
visit-start coverage.
|
||||
5. Run the focused unit tests, both E2E specs, and `pnpm run typecheck` from
|
||||
`apps/web`.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- A session opened at its current tail does not render a divider when a user
|
||||
prompt or agent message is subsequently added during the same visible visit.
|
||||
- A true pre-existing unread boundary still renders after navigation into the
|
||||
session.
|
||||
- An initially loading or empty transcript cannot later activate a stale
|
||||
captured cursor as a divider during the same visit.
|
||||
- Desktop and mobile prove the tail-then-prompt flow has no divider.
|
||||
@@ -307,6 +307,8 @@ UI changes are persisted in the database and require a restart. An explicitly se
|
||||
The source checkout's `make dev` activates the embedded development profile, which enables Office, debug surfaces, ACP logging, and a mock agent; the App status bar and Claude background prompt handoff remain opt-in. Installed `run`/desktop builds select the safe production profile unless the environment explicitly opts in. E2E mock variables and routes are test-only and must never be enabled on a public deployment.
|
||||
|
||||
## Credentials and product settings
|
||||
The **Unread Messages** preference in **Settings > General > Task Actions** controls the Slack-style **New** divider in session transcripts. It defaults on for each user, persists with user settings, and takes effect immediately. Disabling it also stops that user's active transcript view from advancing the session read cursor.
|
||||
|
||||
|
||||
Most integrations, executor profiles, agent profiles, MCP servers, repository settings, and UI preferences are persistent database records edited under **Settings**. They are not fields in `config.yaml`. Secret values use an encrypted secret store backed by `<home>/data/master.key`; filesystem permissions, database backups, and key backup are part of the security boundary.
|
||||
|
||||
|
||||
@@ -285,9 +285,10 @@ Kandev warns when its live WebSocket connection has not recovered for three seco
|
||||
- **Office mode** — experimental, medium risk, and off in the production profile by default.
|
||||
- **App status bar** — stable, low risk, and off in the production profile by default. Enabling it adds the desktop/tablet bar and phone Status entry after restart; disabling it again does not stop connections, metrics collection requested by other clients, or plugins. Urgent WebSocket connectivity warnings still remain visible while the feature is off.
|
||||
- **Claude background prompt handoff** — experimental, high risk, and off in every profile by default. Enabling it lets Claude Code accept another prompt after its foreground yields while recognized async subagent, `run_in_background` shell, or Monitor work remains active. ACP lifecycle gaps can misclassify activity or overlap prompts; use it only for controlled testing.
|
||||
- **Unread divider** — a per-user setting at **Settings > General > Task Actions**. It defaults on, takes effect immediately, and controls both the Slack-style **New** divider and read-cursor updates while that user's transcript view is visible.
|
||||
- **Debug mode** — high risk; enables diagnostic endpoints and agent-message logging that can contain sensitive content.
|
||||
|
||||
Each requires restart. A value supplied explicitly by its environment variable locks the UI control; the debug toggle is also locked by explicit legacy/debug-message environment variables. Otherwise the UI stores an override in the database. The page can request restart only when the native local supervisor is available. A normal Unix `kandev` terminal launch is supervised; Desktop, a service, a container, a directly started backend, a deploy preview, or Windows requires a manual application restart.
|
||||
Each feature toggle requires restart. A value supplied explicitly by its environment variable locks the UI control; the debug toggle is also locked by explicit legacy/debug-message environment variables. Otherwise the UI stores an override in the database. The page can request restart only when the native local supervisor is available. A normal Unix `kandev` terminal launch is supervised; Desktop, a service, a container, a directly started backend, a deploy preview, or Windows requires a manual application restart.
|
||||
|
||||
Status-bar layout is a separate per-user preference. Hold Cmd on macOS or Ctrl
|
||||
elsewhere while mouse-dragging an item to move it across the desktop/tablet bar.
|
||||
|
||||
@@ -29,6 +29,7 @@ The office umbrella covers kandev's autonomous-agent product surface: workspaces
|
||||
| [assistant](office/assistant.md) | draft |
|
||||
| [automations-settings](office/automations-settings.md) | draft |
|
||||
| [testing](office/testing.md) | shipped |
|
||||
| [unread-divider](office/unread-divider.md) | shipped |
|
||||
|
||||
## platform/ — cross-cutting capabilities
|
||||
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
---
|
||||
status: shipped
|
||||
created: 2026-07-27
|
||||
owner: cfl
|
||||
---
|
||||
|
||||
# Office: Slack-Style Unread Divider
|
||||
|
||||
## Why
|
||||
|
||||
Navigating back into a task session that kept running in the background
|
||||
previously dropped the reader at the bottom of the transcript with no
|
||||
indication of what was new. A "New" divider identifies that pre-existing
|
||||
unread boundary, but it must never first emerge solely because messages arrive
|
||||
after the session is already visible. This spec documents the persisted cursor
|
||||
this feature is built on, since it's new product state whose scope (global vs.
|
||||
per-user) and edge-case behavior (first visit, pagination, initial scroll)
|
||||
aren't otherwise obvious from the UI alone.
|
||||
|
||||
## What
|
||||
|
||||
- Each user can enable or disable the divider from **Settings > General > Task
|
||||
Actions > Unread Messages**. It defaults **on** and takes effect immediately:
|
||||
when disabled, the frontend does not compute or render a divider, the message
|
||||
list performs its normal scroll behavior, and `useSessionReadTracking` does
|
||||
not dispatch `POST /task-sessions/:id/mark-read`; existing
|
||||
`last_read_message_id` values remain stored for a future re-enable.
|
||||
- Each `task_sessions` row carries one `last_read_message_id` column — the
|
||||
ID of the newest message the session has "seen." This is **session-global,
|
||||
not per-user or per-device**: kandev tasks do not currently model multiple
|
||||
independent viewers of the same session, so there is exactly one read
|
||||
cursor per session, shared by whoever opens it. If a second viewer (or the
|
||||
same user on a second device/tab) opens the same session, they observe and
|
||||
advance the same cursor as the first.
|
||||
- The cursor advances **live** while the session is the visible chat panel:
|
||||
as new messages render, `useSessionReadTracking` dispatches
|
||||
`POST /task-sessions/:id/mark-read` with the newest currently-rendered
|
||||
message id, and the backend persists it via
|
||||
`UpdateTaskSessionLastReadMessageID` (monotonic — see below).
|
||||
- The divider's position is decided once per **visit** (a transition from
|
||||
hidden/inactive to the active, visible chat panel), not on every render:
|
||||
the hook captures the cursor's value at the instant the session becomes
|
||||
visible (before this visit's live-advance can touch it) and freezes it as
|
||||
the anchor. Re-rendering while still visible never moves the divider;
|
||||
leaving and returning (or switching sessions and back) captures a fresh
|
||||
anchor from whatever the cursor advanced to since.
|
||||
- A visit that opens at the current transcript tail is **divider-ineligible**:
|
||||
if the captured cursor equals the newest initially-rendered message, the
|
||||
hook discards the anchor for that visible visit. Messages subsequently
|
||||
rendered while the panel remains visible must not make a "New" divider
|
||||
appear. The live cursor still advances normally.
|
||||
- **First visit**: a session with no persisted cursor (`last_read_message_id`
|
||||
is empty) has no divider — there's nothing to distinguish as "already
|
||||
read." The first visit begins live-advancing the cursor as normal.
|
||||
- **Initial scroll**: on visit start, if the divider falls on a
|
||||
currently-loaded message that's outside the initial viewport, the message
|
||||
list scrolls to bring it into view (`block: "start"`) instead of the
|
||||
default scroll-to-bottom. This is a one-time positioning decision from
|
||||
the reader's perspective: the transcript's initial data can itself
|
||||
arrive in more than one wave (e.g. a WebSocket-delivered backfill
|
||||
continuing after the first paint), which can shift where the divider
|
||||
actually lands — so the correction re-asserts itself across such
|
||||
changes rather than freezing on a possibly-incomplete first render. It
|
||||
stops permanently the moment either of two things happens, whichever
|
||||
comes first: the reader scrolls (wheel, touch, or a key press), or a
|
||||
short (4s) settling window since mount elapses — bounding how long a
|
||||
live message arriving well after the visit has genuinely settled (with
|
||||
no interaction event to catch, e.g. a scrollbar drag) could otherwise
|
||||
re-trigger it. It does not fight a pending explicit layout scroll
|
||||
restoration (`pendingChatScrollTop`), which always wins when set.
|
||||
- **Pagination**: `findUnreadDividerItemId` looks for the divider's anchor
|
||||
message among the *currently loaded* render items. If the anchor is older
|
||||
than everything loaded so far (paginated out), the whole loaded window is
|
||||
treated as unread and the divider renders before the first loaded item —
|
||||
matching Slack's behavior of showing the marker at the top of what's
|
||||
visible rather than requiring a full backward fetch before showing
|
||||
anything. The synthetic frontend-only `task-description` placeholder
|
||||
(rendered for a session with zero real messages yet) is never a valid
|
||||
anchor or divider boundary — it doesn't exist in the backend messages
|
||||
table.
|
||||
|
||||
## Persistence guarantees
|
||||
|
||||
- `UpdateTaskSessionLastReadMessageID` is a narrow, single-column write
|
||||
(like `RenameTaskSession`) so it never collides with a concurrent
|
||||
full-row or metadata write.
|
||||
- The write is **monotonic**, comparing each message's `(created_at, id)` —
|
||||
portable across SQLite and Postgres, unlike a rowid-based comparison. A
|
||||
delayed or out-of-order mark-read request (e.g. two overlapping requests
|
||||
where an older message's response is processed after a newer one) can
|
||||
never regress the cursor and resurrect a stale divider.
|
||||
- A cursor referencing a deleted message is treated as having no rank, so
|
||||
the monotonic guard never wedges.
|
||||
- The frontend applies the mark-read response narrowly
|
||||
(`updateSessionReadCursor`), never as a full-session merge — a delayed
|
||||
HTTP response can't clobber a newer `session.state_changed` WebSocket
|
||||
update to unrelated fields.
|
||||
|
||||
## Failure modes
|
||||
|
||||
- **Unexpected repository/DB failure marking a session read**: logged and
|
||||
returned as a sanitized `500`, distinct from `400` for genuine bad input
|
||||
(empty id, unknown message, message belonging to a different session).
|
||||
- **Mark-read targeting the synthetic placeholder**: prevented client-side —
|
||||
`lastRenderedMessageId` skips it, so an empty session never issues a
|
||||
mark-read request the backend would reject.
|
||||
|
||||
## Scenarios
|
||||
|
||||
- **GIVEN** a session with messages read before a rewound cursor, **WHEN**
|
||||
the user navigates away and back, **THEN** the divider appears before the
|
||||
first message added since the frozen anchor, and clears (no anchor) on
|
||||
the *next* visit after that.
|
||||
- **GIVEN** a session opens with its persisted cursor at the newest rendered
|
||||
message, **WHEN** the user sends a new prompt or an agent message arrives
|
||||
while the session remains visible, **THEN** no "New" divider appears before
|
||||
that new message.
|
||||
- **GIVEN** two overlapping mark-read requests for the same session where
|
||||
the response for an older message resolves after a newer one, **WHEN**
|
||||
both complete, **THEN** the persisted cursor reflects the newer message,
|
||||
never regressing.
|
||||
- **GIVEN** a brand-new session with no messages yet, **WHEN** the chat
|
||||
panel renders, **THEN** no mark-read request is issued for the synthetic
|
||||
task-description placeholder.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Per-user or per-device read state. Would require a join table
|
||||
(`session_id`, `user_id`) → `last_read_message_id`; not needed while
|
||||
kandev has no multi-viewer session model.
|
||||
- Unread *counts* (e.g. a badge showing "12 new") — this feature only
|
||||
positions a single boundary marker, matching Slack's divider, not its
|
||||
unread-count badges.
|
||||
Reference in New Issue
Block a user