diff --git a/apps/backend/internal/clarification/handlers.go b/apps/backend/internal/clarification/handlers.go index ae66033861..fbe47df12d 100644 --- a/apps/backend/internal/clarification/handlers.go +++ b/apps/backend/internal/clarification/handlers.go @@ -29,6 +29,7 @@ const ( metaTaskIDKey = "task_id" metaPendingIDKey = "pending_id" metaRejectedKey = "rejected" + metaRequestIDKey = "request_id" clarificationPersistenceTimeout = 30 * time.Second @@ -239,6 +240,7 @@ func RegisterRoutes( inbox := router.Group("/api/v1/clarification-inbox") inbox.GET("", h.httpListInbox) inbox.GET("/hidden", h.httpListInboxHidden) + inbox.GET("/history", h.httpListInboxHistory) inbox.PUT("/sidecar/:pendingID", h.httpUpsertInboxSidecar) inbox.DELETE("/sidecar/:pendingID", h.httpDeleteInboxSidecar) } diff --git a/apps/backend/internal/clarification/inbox_handlers.go b/apps/backend/internal/clarification/inbox_handlers.go index a163f51aa3..99924aad36 100644 --- a/apps/backend/internal/clarification/inbox_handlers.go +++ b/apps/backend/internal/clarification/inbox_handlers.go @@ -17,6 +17,7 @@ import ( "github.com/kandev/kandev/internal/task/repository/repoerrors" "github.com/kandev/kandev/internal/task/service" userstore "github.com/kandev/kandev/internal/user/store" + wfmodels "github.com/kandev/kandev/internal/workflow/models" v1 "github.com/kandev/kandev/pkg/api/v1" "go.uber.org/zap" ) @@ -38,16 +39,25 @@ type inboxTaskLookup interface { BatchGetSessionsForTasks(ctx context.Context, taskIDs []string) (map[string][]*taskmodels.TaskSession, error) } -// inboxTaskService is the combined dependency the Needs-you Inbox handlers -// need from the task service. +// inboxWorkflowStepReader resolves a task's current workflow step for the +// History tab's step-starts-no-agent label. The clarification package has no +// other workflow dependency; this is that one, narrow read. +type inboxWorkflowStepReader interface { + GetWorkflowStep(ctx context.Context, stepID string) (*wfmodels.WorkflowStep, error) +} + +// inboxTaskService is the combined dependency the Needs-you Inbox and +// Inbox History handlers need from the task service. type inboxTaskService interface { inboxWorkspaceAuthorizer inboxTaskLookup + inboxWorkflowStepReader } -// inboxBundleStore is the bounded read/write surface the Needs-you Inbox -// endpoints need: the existing bundle-query machinery plus the new per-user -// dismiss/snooze sidecar (needs-you-inbox design, "Persistence"). +// inboxBundleStore is the bounded read/write surface the Needs-you Inbox and +// Inbox History endpoints need: the existing bundle-query machinery, the +// per-user dismiss/snooze sidecar (needs-you-inbox design, "Persistence"), +// and the additive, isolated history read. type inboxBundleStore interface { ListUnresolvedClarificationBundles(ctx context.Context, opts taskmodels.ListClarificationBundlesOptions) (*taskmodels.ClarificationBundlePage, error) FindMessagesByPendingIDs(ctx context.Context, pendingIDs []string) (map[string][]*taskmodels.Message, error) @@ -55,6 +65,8 @@ type inboxBundleStore interface { DeleteClarificationInboxSidecar(ctx context.Context, userID, pendingID string) error CountHiddenClarificationBundles(ctx context.Context, opts taskmodels.ListClarificationBundlesOptions) (taskmodels.ClarificationInboxHiddenSummary, error) GetClarificationInboxSidecarStates(ctx context.Context, userID string, pendingIDs []string) (map[string]taskmodels.ClarificationInboxHiddenBundle, error) + ListInboxHistoryBundles(ctx context.Context, opts taskmodels.ListClarificationHistoryOptions) (*taskmodels.ClarificationHistoryPage, error) + CountInboxHistoryBundles(ctx context.Context, opts taskmodels.ListClarificationHistoryOptions) (int, error) } const ( diff --git a/apps/backend/internal/clarification/inbox_handlers_test.go b/apps/backend/internal/clarification/inbox_handlers_test.go index 5a4a7df2f6..064e4f251f 100644 --- a/apps/backend/internal/clarification/inbox_handlers_test.go +++ b/apps/backend/internal/clarification/inbox_handlers_test.go @@ -16,6 +16,7 @@ import ( taskmodels "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/task/repository/repoerrors" "github.com/kandev/kandev/internal/task/service" + wfmodels "github.com/kandev/kandev/internal/workflow/models" ) var testInboxNow = time.Date(2026, time.September, 12, 12, 0, 0, 0, time.UTC) @@ -60,6 +61,13 @@ func (f *fakeInboxTasks) GetTasksByIDs(_ context.Context, ids []string) ([]*task return result, nil } +// GetWorkflowStep is a no-op default so fakeInboxTasks alone still satisfies +// inboxTaskService for every pre-existing test; History-specific tests +// override it via fakeInboxTasksWithWorkflow (inbox_history_handlers_test.go). +func (f *fakeInboxTasks) GetWorkflowStep(context.Context, string) (*wfmodels.WorkflowStep, error) { + return nil, nil +} + func (f *fakeInboxTasks) BatchGetSessionsForTasks(_ context.Context, taskIDs []string) (map[string][]*taskmodels.TaskSession, error) { f.sessionBatchCalls++ result := make(map[string][]*taskmodels.TaskSession) @@ -170,6 +178,22 @@ func (f *fakeInboxBundleStore) GetClarificationInboxSidecarStates( return f.states, nil } +// ListInboxHistoryBundles/CountInboxHistoryBundles are no-op defaults so +// fakeInboxBundleStore alone still satisfies inboxBundleStore for every +// pre-existing test; History-specific tests override them via +// fakeInboxBundleStoreWithHistory (inbox_history_handlers_test.go). +func (f *fakeInboxBundleStore) ListInboxHistoryBundles( + context.Context, taskmodels.ListClarificationHistoryOptions, +) (*taskmodels.ClarificationHistoryPage, error) { + return &taskmodels.ClarificationHistoryPage{}, nil +} + +func (f *fakeInboxBundleStore) CountInboxHistoryBundles( + context.Context, taskmodels.ListClarificationHistoryOptions, +) (int, error) { + return 0, nil +} + // alwaysAllowAuthorizer is a taskAccessAuthorizer double that authorizes // every task_id, for tests that only care about the inbox layer's own // validation/response behavior. diff --git a/apps/backend/internal/clarification/inbox_history_handlers.go b/apps/backend/internal/clarification/inbox_history_handlers.go new file mode 100644 index 0000000000..daa5253d2e --- /dev/null +++ b/apps/backend/internal/clarification/inbox_history_handlers.go @@ -0,0 +1,302 @@ +package clarification + +import ( + "context" + "net/http" + "sort" + "time" + + "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/authz" + taskmodels "github.com/kandev/kandev/internal/task/models" + wfmodels "github.com/kandev/kandev/internal/workflow/models" + "go.uber.org/zap" +) + +// inboxHistoryBundleKind's two wire values. +const ( + inboxHistoryBundleKindPermission = "permission" + inboxHistoryBundleKindClarification = "clarification" +) + +// inboxHistoryBundleView is one row of the History tab's bundle listing: the +// same shape as inboxBundleView (the message payload carries every field a +// row needs to render a clarification or permission bundle) plus the +// exclusion reason, the turn identity, the kind label, and the +// step-starts-no-agent flag. +type inboxHistoryBundleView struct { + inboxBundleView + // Kind distinguishes a permission bundle from a clarification bundle, + // derived from the bundle's own messages rather than carried as a second + // source of truth. + Kind string `json:"kind"` + // Reason is the exclusion reason: superseded, session_ended, or + // unreadable. + Reason string `json:"reason"` + // AskingTurnID is the turn that asked the bundle's question(s). + AskingTurnID string `json:"asking_turn_id"` + // SupersedingTurnID is set only when Reason is superseded via a + // turn-level supersession; nil (omitted) for every other case, including + // a same-turn permission supersession. + SupersedingTurnID *string `json:"superseding_turn_id,omitempty"` + // StepStartsNoAgent is true when the task's current workflow step starts + // no agent, false when it does, and nil (omitted) when the step cannot + // be read -- never a guess. + StepStartsNoAgent *bool `json:"step_starts_no_agent,omitempty"` +} + +// inboxHistoryListResponse is GET /api/v1/clarification-inbox/history's +// response envelope. total is the workspace-wide bundle count, distinct +// from count, the page size. +type inboxHistoryListResponse struct { + Bundles []inboxHistoryBundleView `json:"bundles"` + Count int `json:"count"` + Total int `json:"total"` + NextCursor string `json:"next_cursor,omitempty"` +} + +// httpListInboxHistory backs GET /api/v1/clarification-inbox/history, +// modeled on httpListInboxHidden: parse and authorize identically, then read +// through the additive history path instead of the operational one. +func (h *Handlers) httpListInboxHistory(c *gin.Context) { + ctx := c.Request.Context() + workspaceID, limit, cursorCreatedAt, cursorPendingID, ok := h.parseInboxListQuery(c) + if !ok { + return + } + if err := h.inboxTasks.AuthorizeWorkspaceScope(ctx, workspaceID, authz.ScopeWorkspaceRead); err != nil { + h.respondInboxWorkspaceAuthzError(c, err) + return + } + + page, err := h.inboxBundles.ListInboxHistoryBundles(ctx, taskmodels.ListClarificationHistoryOptions{ + WorkspaceID: workspaceID, + CursorCreatedAt: cursorCreatedAt, + CursorPendingID: cursorPendingID, + Limit: limit, + }) + if err != nil { + h.logger.Error("failed to list inbox history bundles", zap.Error(err)) + respondInboxError(c, http.StatusInternalServerError, errInboxInternal) + return + } + views, err := h.buildInboxHistoryBundleViews(ctx, page.Bundles) + if err != nil { + h.logger.Error("failed to hydrate inbox history bundles", zap.Error(err)) + respondInboxError(c, http.StatusInternalServerError, errInboxInternal) + return + } + + // The workspace-wide total is its own query: a failure here fails the + // whole read rather than defaulting the badge to zero or a stale value. + total, err := h.inboxBundles.CountInboxHistoryBundles(ctx, taskmodels.ListClarificationHistoryOptions{WorkspaceID: workspaceID}) + if err != nil { + h.logger.Error("failed to count inbox history bundles", zap.Error(err)) + respondInboxError(c, http.StatusInternalServerError, errInboxInternal) + return + } + + resp := inboxHistoryListResponse{Bundles: views, Count: len(views), Total: total} + if page.HasMore && len(page.Bundles) > 0 { + last := page.Bundles[len(page.Bundles)-1] + resp.NextCursor = encodeInboxCursor(last.CreatedAt, last.PendingID) + } + c.JSON(http.StatusOK, resp) +} + +// buildInboxHistoryBundleViews hydrates each bundle's durable messages and +// task_title/session_state/step-starts-no-agent enrichment. A bundle whose +// messages cannot be read fails the whole page (matching +// buildInboxBundleViews); a bundle with zero resolvable messages is skipped +// and logged. +func (h *Handlers) buildInboxHistoryBundleViews( + ctx context.Context, bundles []taskmodels.ClarificationHistoryBundleSummary, +) ([]inboxHistoryBundleView, error) { + if len(bundles) == 0 { + return []inboxHistoryBundleView{}, nil + } + pendingIDs, taskIDs := inboxHistoryBundleKeys(bundles) + messagesByPendingID, err := h.inboxBundles.FindMessagesByPendingIDs(ctx, pendingIDs) + if err != nil { + return nil, err + } + taskTitles, stepStartsNoAgent := h.batchInboxHistoryTaskInfo(ctx, taskIDs) + sessionStates := h.batchInboxSessionStates(ctx, taskIDs) + + views := make([]inboxHistoryBundleView, 0, len(bundles)) + for _, b := range bundles { + msgs := filterMessagesByPermissionGroupKey(messagesByPendingID[b.PendingID], b.PermissionGroupKey) + if len(msgs) == 0 { + h.logger.Warn("inbox history bundle has no resolvable messages; omitting from page", + zap.String("pending_id", b.PendingID)) + continue + } + views = append(views, buildInboxHistoryBundleView(b, msgs, taskTitles, sessionStates, stepStartsNoAgent)) + } + return views, nil +} + +func inboxHistoryBundleKeys(bundles []taskmodels.ClarificationHistoryBundleSummary) (pendingIDs, taskIDs []string) { + seenPendingIDs := make(map[string]struct{}, len(bundles)) + seenTaskIDs := make(map[string]struct{}, len(bundles)) + for _, bundle := range bundles { + if _, seen := seenPendingIDs[bundle.PendingID]; !seen { + seenPendingIDs[bundle.PendingID] = struct{}{} + pendingIDs = append(pendingIDs, bundle.PendingID) + } + if _, seen := seenTaskIDs[bundle.TaskID]; !seen && bundle.TaskID != "" { + seenTaskIDs[bundle.TaskID] = struct{}{} + taskIDs = append(taskIDs, bundle.TaskID) + } + } + return pendingIDs, taskIDs +} + +func buildInboxHistoryBundleView( + b taskmodels.ClarificationHistoryBundleSummary, + msgs []*taskmodels.Message, + taskTitles, sessionStates map[string]string, + stepStartsNoAgent map[string]*bool, +) inboxHistoryBundleView { + ordered := orderInboxHistoryMessages(msgs) + view := inboxHistoryBundleView{ + inboxBundleView: inboxBundleView{ + PendingID: b.PendingID, + TaskID: b.TaskID, + SessionID: b.SessionID, + CreatedAt: b.CreatedAt.UTC().Format(time.RFC3339), + Context: inboxBundleContext(ordered), + Messages: renderInboxMessages(ordered), + TaskTitle: taskTitles[b.TaskID], + SessionState: sessionStates[b.SessionID], + }, + Kind: inboxHistoryBundleKind(ordered), + Reason: string(b.Reason), + AskingTurnID: b.AskingTurnID, + } + if b.SupersedingTurnID != "" { + superseding := b.SupersedingTurnID + view.SupersedingTurnID = &superseding + } + if flag, ok := stepStartsNoAgent[b.TaskID]; ok { + view.StepStartsNoAgent = flag + } + return view +} + +// filterMessagesByPermissionGroupKey keeps only the messages belonging to +// this bundle's own logical request. FindMessagesByPendingIDs returns every +// message sharing a raw pending_id, including an unrelated request's when a +// permission pending_id has been reused (clarification_history_query.go's +// permissionGroupKeyExpr comment); groupKey is empty for a clarification +// bundle, whose pending_id is never reused, so every message passes +// unfiltered. +func filterMessagesByPermissionGroupKey(msgs []*taskmodels.Message, groupKey string) []*taskmodels.Message { + if groupKey == "" { + return msgs + } + filtered := make([]*taskmodels.Message, 0, len(msgs)) + for _, m := range msgs { + if permissionGroupKeyFromMessage(m) == groupKey { + filtered = append(filtered, m) + } + } + return filtered +} + +// permissionGroupKeyFromMessage mirrors clarification_history_query.go's +// permissionGroupKeyExpr exactly: a permission message's own request_id, +// falling back to its message id when request_id is absent; empty for any +// other message type. +func permissionGroupKeyFromMessage(m *taskmodels.Message) string { + if m.Type != taskmodels.MessageTypePermissionRequest { + return "" + } + if requestID := stringFromMetadata(m.Metadata, metaRequestIDKey); requestID != "" { + return requestID + } + return m.ID +} + +// inboxHistoryBundleKind derives the kind label from the bundle's own first +// message rather than carrying it as a second source of truth. +func inboxHistoryBundleKind(ordered []*taskmodels.Message) string { + if len(ordered) == 0 { + return "" + } + if ordered[0].Type == taskmodels.MessageTypePermissionRequest { + return inboxHistoryBundleKindPermission + } + return inboxHistoryBundleKindClarification +} + +// orderInboxHistoryMessages orders messages by question_index ascending +// (normalized per normalizedInboxQuestionIndex), then question_id ascending, +// then message id ascending. The third key is NOT part of orderInboxMessages: +// it only breaks a remaining tie for the empty-question_id bundles History +// uniquely admits, which Needs-you never lists, so the two Inbox tabs still +// never order a shared bundle differently. +func orderInboxHistoryMessages(msgs []*taskmodels.Message) []*taskmodels.Message { + sorted := make([]*taskmodels.Message, len(msgs)) + copy(sorted, msgs) + sort.SliceStable(sorted, func(i, j int) bool { + qi, qj := normalizedInboxQuestionIndex(sorted[i].Metadata), normalizedInboxQuestionIndex(sorted[j].Metadata) + if qi != qj { + return qi < qj + } + qidI, qidJ := questionIDFromMetadata(sorted[i].Metadata), questionIDFromMetadata(sorted[j].Metadata) + if qidI != qidJ { + return qidI < qidJ + } + return sorted[i].ID < sorted[j].ID + }) + return sorted +} + +// batchInboxHistoryTaskInfo fetches every bundle's owning task once, and +// resolves the step-starts-no-agent flag per unique workflow step (cached; +// History pages are bounded, and a task's steps repeat across bundles more +// often than not). A read failure at either layer omits the affected field +// rather than guessing. +func (h *Handlers) batchInboxHistoryTaskInfo(ctx context.Context, taskIDs []string) (titles map[string]string, stepStartsNoAgent map[string]*bool) { + titles = make(map[string]string, len(taskIDs)) + stepStartsNoAgent = make(map[string]*bool, len(taskIDs)) + if len(taskIDs) == 0 { + return titles, stepStartsNoAgent + } + tasks, err := h.inboxTasks.GetTasksByIDs(ctx, taskIDs) + if err != nil { + h.logger.Warn("failed to batch-load inbox history task info", zap.Error(err)) + return titles, stepStartsNoAgent + } + stepCache := make(map[string]*bool) + for _, task := range tasks { + if task == nil { + continue + } + titles[task.ID] = task.Title + if task.WorkflowStepID == "" { + continue + } + flag, cached := stepCache[task.WorkflowStepID] + if !cached { + flag = h.resolveStepStartsNoAgent(ctx, task.WorkflowStepID) + stepCache[task.WorkflowStepID] = flag + } + stepStartsNoAgent[task.ID] = flag + } + return titles, stepStartsNoAgent +} + +// resolveStepStartsNoAgent uses presence of an auto_start_agent on_enter +// action as the only test -- there is no disabled-entry concept to discount. +// A read failure (step not found, or the workflow cannot be read) omits the +// label rather than guessing. +func (h *Handlers) resolveStepStartsNoAgent(ctx context.Context, stepID string) *bool { + step, err := h.inboxTasks.GetWorkflowStep(ctx, stepID) + if err != nil || step == nil { + return nil + } + startsNoAgent := !step.HasOnEnterAction(wfmodels.OnEnterAutoStartAgent) + return &startsNoAgent +} diff --git a/apps/backend/internal/clarification/inbox_history_handlers_test.go b/apps/backend/internal/clarification/inbox_history_handlers_test.go new file mode 100644 index 0000000000..3f6f61c3d4 --- /dev/null +++ b/apps/backend/internal/clarification/inbox_history_handlers_test.go @@ -0,0 +1,439 @@ +package clarification + +import ( + "context" + "net/http" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/common/logger" + taskmodels "github.com/kandev/kandev/internal/task/models" + wfmodels "github.com/kandev/kandev/internal/workflow/models" +) + +// fakeInboxBundleStoreWithHistory adds the History read's two entry points on +// top of the existing fakeInboxBundleStore double, so the operational-inbox +// fixtures stay untouched. +type fakeInboxBundleStoreWithHistory struct { + *fakeInboxBundleStore + + historyPage *taskmodels.ClarificationHistoryPage + historyListErr error + historyLastOpts taskmodels.ListClarificationHistoryOptions + + historyTotal int + historyTotalErr error +} + +func (f *fakeInboxBundleStoreWithHistory) ListInboxHistoryBundles( + _ context.Context, opts taskmodels.ListClarificationHistoryOptions, +) (*taskmodels.ClarificationHistoryPage, error) { + f.historyLastOpts = opts + if f.historyListErr != nil { + return nil, f.historyListErr + } + if f.historyPage == nil { + return &taskmodels.ClarificationHistoryPage{}, nil + } + return f.historyPage, nil +} + +func (f *fakeInboxBundleStoreWithHistory) CountInboxHistoryBundles( + context.Context, taskmodels.ListClarificationHistoryOptions, +) (int, error) { + if f.historyTotalErr != nil { + return 0, f.historyTotalErr + } + return f.historyTotal, nil +} + +// fakeInboxTasksWithWorkflow adds GetWorkflowStep on top of the existing +// fakeInboxTasks double. +type fakeInboxTasksWithWorkflow struct { + *fakeInboxTasks + + steps map[string]*wfmodels.WorkflowStep + stepErr error +} + +func (f *fakeInboxTasksWithWorkflow) GetWorkflowStep(_ context.Context, stepID string) (*wfmodels.WorkflowStep, error) { + if f.stepErr != nil { + return nil, f.stepErr + } + return f.steps[stepID], nil +} + +func newInboxHistoryTestHandler( + t *testing.T, + msgs map[string][]*taskmodels.Message, + tasks *fakeInboxTasksWithWorkflow, + bundles *fakeInboxBundleStoreWithHistory, +) *Handlers { + t.Helper() + gin.SetMode(gin.TestMode) + bundles.stubMessageStore = stubMessageStore{messages: msgs} + h := NewHandlers(NewStore(time.Minute), nil, &stubMessageCreator{}, &stubMessageStore{messages: msgs}, + &stubEventBus{}, nil, logger.Default(), tasks, bundles) + h.now = func() time.Time { return testInboxNow } + return h +} + +func newHistoryQuestionMessage(pendingID, taskID, sessionID, questionID string, index int, created time.Time) *taskmodels.Message { + return &taskmodels.Message{ + ID: pendingID + "-msg", + TaskSessionID: sessionID, + TaskID: taskID, + Type: taskmodels.MessageTypeClarificationRequest, + Content: "what would you like?", + CreatedAt: created, + Metadata: map[string]any{ + "question_id": questionID, + "question_index": float64(index), + }, + } +} + +func TestHttpListInboxHistory_ReturnsHydratedBundles(t *testing.T) { + created := testInboxNow.Add(-time.Hour) + msgs := map[string][]*taskmodels.Message{ + "pend-1": {newHistoryQuestionMessage("pend-1", "task-1", "sess-1", "q-1", 0, created)}, + } + tasks := &fakeInboxTasksWithWorkflow{ + fakeInboxTasks: &fakeInboxTasks{ + tasks: map[string]*taskmodels.Task{ + "task-1": {ID: "task-1", Title: "Do the thing", WorkflowStepID: "step-1"}, + }, + }, + steps: map[string]*wfmodels.WorkflowStep{ + "step-1": {ID: "step-1"}, + }, + } + bundles := &fakeInboxBundleStoreWithHistory{ + fakeInboxBundleStore: &fakeInboxBundleStore{}, + historyPage: &taskmodels.ClarificationHistoryPage{ + Bundles: []taskmodels.ClarificationHistoryBundleSummary{ + { + PendingID: "pend-1", + TaskID: "task-1", + SessionID: "sess-1", + CreatedAt: created, + Reason: taskmodels.ClarificationHistoryReasonSessionEnded, + AskingTurnID: "turn-1", + }, + }, + }, + historyTotal: 1, + } + h := newInboxHistoryTestHandler(t, msgs, tasks, bundles) + + rec := runInboxGet(h, "/api/v1/clarification-inbox/history?workspace_id=ws-1", h.httpListInboxHistory) + + if rec.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) + } + var resp inboxHistoryListResponse + decodeInboxBody(t, rec, &resp) + if len(resp.Bundles) != 1 { + t.Fatalf("expected 1 bundle, got %d", len(resp.Bundles)) + } + got := resp.Bundles[0] + if got.PendingID != "pend-1" || got.Reason != "session_ended" || got.Kind != inboxHistoryBundleKindClarification { + t.Fatalf("unexpected bundle view: %+v", got) + } + if got.AskingTurnID != "turn-1" { + t.Fatalf("expected asking_turn_id turn-1, got %q", got.AskingTurnID) + } + if got.SupersedingTurnID != nil { + t.Fatalf("expected no superseding turn id, got %v", *got.SupersedingTurnID) + } + if got.StepStartsNoAgent == nil || *got.StepStartsNoAgent != true { + t.Fatalf("expected step_starts_no_agent=true, got %+v", got.StepStartsNoAgent) + } + if got.TaskTitle != "Do the thing" { + t.Fatalf("expected task title hydrated, got %q", got.TaskTitle) + } + if resp.Total != 1 || resp.Count != 1 { + t.Fatalf("expected total=1 count=1, got total=%d count=%d", resp.Total, resp.Count) + } +} + +func TestHttpListInboxHistory_SupersedingTurnIDOmittedWhenEmpty(t *testing.T) { + created := testInboxNow.Add(-time.Hour) + msgs := map[string][]*taskmodels.Message{ + "pend-1": {newHistoryQuestionMessage("pend-1", "task-1", "sess-1", "q-1", 0, created)}, + } + tasks := &fakeInboxTasksWithWorkflow{fakeInboxTasks: &fakeInboxTasks{tasks: map[string]*taskmodels.Task{ + "task-1": {ID: "task-1", Title: "T"}, + }}} + bundles := &fakeInboxBundleStoreWithHistory{ + fakeInboxBundleStore: &fakeInboxBundleStore{}, + historyPage: &taskmodels.ClarificationHistoryPage{ + Bundles: []taskmodels.ClarificationHistoryBundleSummary{ + { + PendingID: "pend-1", + TaskID: "task-1", + SessionID: "sess-1", + CreatedAt: created, + Reason: taskmodels.ClarificationHistoryReasonSuperseded, + AskingTurnID: "turn-1", + SupersedingTurnID: "turn-2", + }, + }, + }, + } + h := newInboxHistoryTestHandler(t, msgs, tasks, bundles) + + rec := runInboxGet(h, "/api/v1/clarification-inbox/history?workspace_id=ws-1", h.httpListInboxHistory) + + var resp inboxHistoryListResponse + decodeInboxBody(t, rec, &resp) + if resp.Bundles[0].SupersedingTurnID == nil || *resp.Bundles[0].SupersedingTurnID != "turn-2" { + t.Fatalf("expected superseding_turn_id turn-2, got %+v", resp.Bundles[0].SupersedingTurnID) + } + + // A different bundle, same-turn permission supersession, carries no + // separate superseding turn (AC .10): the field stays nil/omitted. + bundles.historyPage.Bundles[0].SupersedingTurnID = "" + rec2 := runInboxGet(h, "/api/v1/clarification-inbox/history?workspace_id=ws-1", h.httpListInboxHistory) + var resp2 inboxHistoryListResponse + decodeInboxBody(t, rec2, &resp2) + if resp2.Bundles[0].SupersedingTurnID != nil { + t.Fatalf("expected omitted superseding_turn_id, got %v", *resp2.Bundles[0].SupersedingTurnID) + } +} + +func TestHttpListInboxHistory_MissingWorkspaceIDRejectedWith400(t *testing.T) { + h := newInboxHistoryTestHandler(t, nil, + &fakeInboxTasksWithWorkflow{fakeInboxTasks: &fakeInboxTasks{}}, + &fakeInboxBundleStoreWithHistory{fakeInboxBundleStore: &fakeInboxBundleStore{}}) + + rec := runInboxGet(h, "/api/v1/clarification-inbox/history", h.httpListInboxHistory) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400 for missing workspace_id, got %d: %s", rec.Code, rec.Body.String()) + } +} + +func TestHttpListInboxHistory_StepReadFailureOmitsLabel(t *testing.T) { + created := testInboxNow.Add(-time.Hour) + msgs := map[string][]*taskmodels.Message{ + "pend-1": {newHistoryQuestionMessage("pend-1", "task-1", "sess-1", "q-1", 0, created)}, + } + tasks := &fakeInboxTasksWithWorkflow{ + fakeInboxTasks: &fakeInboxTasks{tasks: map[string]*taskmodels.Task{ + "task-1": {ID: "task-1", Title: "T", WorkflowStepID: "step-missing"}, + }}, + stepErr: context.DeadlineExceeded, + } + bundles := &fakeInboxBundleStoreWithHistory{ + fakeInboxBundleStore: &fakeInboxBundleStore{}, + historyPage: &taskmodels.ClarificationHistoryPage{ + Bundles: []taskmodels.ClarificationHistoryBundleSummary{ + {PendingID: "pend-1", TaskID: "task-1", SessionID: "sess-1", CreatedAt: created, Reason: taskmodels.ClarificationHistoryReasonUnreadable, AskingTurnID: "turn-1"}, + }, + }, + } + h := newInboxHistoryTestHandler(t, msgs, tasks, bundles) + + rec := runInboxGet(h, "/api/v1/clarification-inbox/history?workspace_id=ws-1", h.httpListInboxHistory) + + var resp inboxHistoryListResponse + decodeInboxBody(t, rec, &resp) + if resp.Bundles[0].StepStartsNoAgent != nil { + t.Fatalf("expected step_starts_no_agent omitted on read failure, got %v", *resp.Bundles[0].StepStartsNoAgent) + } +} + +func TestHttpListInboxHistory_ListErrorReturns500(t *testing.T) { + h := newInboxHistoryTestHandler(t, nil, + &fakeInboxTasksWithWorkflow{fakeInboxTasks: &fakeInboxTasks{}}, + &fakeInboxBundleStoreWithHistory{fakeInboxBundleStore: &fakeInboxBundleStore{}, historyListErr: context.DeadlineExceeded}) + + rec := runInboxGet(h, "/api/v1/clarification-inbox/history?workspace_id=ws-1", h.httpListInboxHistory) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", rec.Code, rec.Body.String()) + } +} + +func TestHttpListInboxHistory_CountErrorReturns500(t *testing.T) { + h := newInboxHistoryTestHandler(t, nil, + &fakeInboxTasksWithWorkflow{fakeInboxTasks: &fakeInboxTasks{}}, + &fakeInboxBundleStoreWithHistory{fakeInboxBundleStore: &fakeInboxBundleStore{}, historyTotalErr: context.DeadlineExceeded}) + + rec := runInboxGet(h, "/api/v1/clarification-inbox/history?workspace_id=ws-1", h.httpListInboxHistory) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("expected 500, got %d: %s", rec.Code, rec.Body.String()) + } +} + +func TestOrderInboxHistoryMessages_ThreeKeySort(t *testing.T) { + a := &taskmodels.Message{ID: "a", Metadata: map[string]any{"question_id": "z", "question_index": float64(0)}} + b := &taskmodels.Message{ID: "b", Metadata: map[string]any{"question_id": "y", "question_index": float64(0)}} + c := &taskmodels.Message{ID: "c", Metadata: map[string]any{"question_index": float64(1)}} + + got := orderInboxHistoryMessages([]*taskmodels.Message{c, a, b}) + + if got[0].ID != "b" || got[1].ID != "a" || got[2].ID != "c" { + t.Fatalf("unexpected order: %v, %v, %v", got[0].ID, got[1].ID, got[2].ID) + } +} + +// TestBuildInboxHistoryBundleViews_FiltersReusedPendingIDMessagesByPermissionGroupKey +// pins that FindMessagesByPendingIDs returning both an old (unrelated, +// already-resolved) and a new permission message under one reused pending_id +// does not leak the old request's content into the new request's bundle: +// filterMessagesByPermissionGroupKey must narrow hydration down to the +// bundle's own PermissionGroupKey before rendering. +func TestBuildInboxHistoryBundleViews_FiltersReusedPendingIDMessagesByPermissionGroupKey(t *testing.T) { + created := testInboxNow.Add(-time.Hour) + oldMsg := &taskmodels.Message{ + ID: "perm-old-msg", TaskSessionID: "sess-1", TaskID: "task-1", + Type: taskmodels.MessageTypePermissionRequest, Content: "old: delete /tmp?", + CreatedAt: created, + Metadata: map[string]any{"request_id": "req-old"}, + } + newMsg := &taskmodels.Message{ + ID: "perm-new-msg", TaskSessionID: "sess-1", TaskID: "task-1", + Type: taskmodels.MessageTypePermissionRequest, Content: "new: run tests?", + CreatedAt: created.Add(time.Minute), + Metadata: map[string]any{"request_id": "req-new"}, + } + // FindMessagesByPendingIDs returns every message sharing the raw + // pending_id, including the old, unrelated request's once a provider + // reuses it. + msgs := map[string][]*taskmodels.Message{ + "pend-reused": {oldMsg, newMsg}, + } + tasks := &fakeInboxTasksWithWorkflow{fakeInboxTasks: &fakeInboxTasks{ + tasks: map[string]*taskmodels.Task{"task-1": {ID: "task-1", Title: "Do the thing"}}, + }} + bundles := &fakeInboxBundleStoreWithHistory{fakeInboxBundleStore: &fakeInboxBundleStore{}} + h := newInboxHistoryTestHandler(t, msgs, tasks, bundles) + + views, err := h.buildInboxHistoryBundleViews(context.Background(), []taskmodels.ClarificationHistoryBundleSummary{ + { + PendingID: "pend-reused", + TaskID: "task-1", + SessionID: "sess-1", + CreatedAt: created.Add(time.Minute), + Reason: taskmodels.ClarificationHistoryReasonSuperseded, + AskingTurnID: "turn-b", + PermissionGroupKey: "req-new", + }, + }) + if err != nil { + t.Fatalf("buildInboxHistoryBundleViews: %v", err) + } + if len(views) != 1 { + t.Fatalf("expected 1 view, got %d: %+v", len(views), views) + } + got := views[0] + if len(got.Messages) != 1 { + t.Fatalf("expected the reused pending_id's hydration to keep only the bundle's own request's message, got %d: %+v", len(got.Messages), got.Messages) + } + if got.Messages[0].ID != "perm-new-msg" { + t.Fatalf("expected only the new request's message to render, got %q (old, unrelated request's content leaked through)", got.Messages[0].ID) + } +} + +// TestBuildInboxHistoryBundleViews_OmitsBundleWhenPermissionGroupKeyFiltersAllMessages +// pins the zero-message-after-filtering branch: a bundle whose +// PermissionGroupKey matches none of the messages FindMessagesByPendingIDs +// returned for its pending_id (a misaligned group key) is omitted from the +// page rather than rendered empty or with the wrong request's content. +func TestBuildInboxHistoryBundleViews_OmitsBundleWhenPermissionGroupKeyFiltersAllMessages(t *testing.T) { + created := testInboxNow.Add(-time.Hour) + unrelatedMsg := &taskmodels.Message{ + ID: "perm-unrelated-msg", TaskSessionID: "sess-1", TaskID: "task-1", + Type: taskmodels.MessageTypePermissionRequest, Content: "unrelated request", + CreatedAt: created, + Metadata: map[string]any{"request_id": "req-unrelated"}, + } + msgs := map[string][]*taskmodels.Message{ + "pend-reused": {unrelatedMsg}, + } + tasks := &fakeInboxTasksWithWorkflow{fakeInboxTasks: &fakeInboxTasks{ + tasks: map[string]*taskmodels.Task{"task-1": {ID: "task-1", Title: "Do the thing"}}, + }} + bundles := &fakeInboxBundleStoreWithHistory{fakeInboxBundleStore: &fakeInboxBundleStore{}} + h := newInboxHistoryTestHandler(t, msgs, tasks, bundles) + + views, err := h.buildInboxHistoryBundleViews(context.Background(), []taskmodels.ClarificationHistoryBundleSummary{ + { + PendingID: "pend-reused", + TaskID: "task-1", + SessionID: "sess-1", + CreatedAt: created, + Reason: taskmodels.ClarificationHistoryReasonSuperseded, + AskingTurnID: "turn-a", + PermissionGroupKey: "req-does-not-match-anything", + }, + }) + if err != nil { + t.Fatalf("buildInboxHistoryBundleViews: %v", err) + } + if len(views) != 0 { + t.Fatalf("expected the bundle omitted when its group key filters out every message, got %d: %+v", len(views), views) + } +} + +// TestPermissionGroupKeyFromMessage_FallsBackToMessageIDWhenRequestIDAbsent +// pins that the Go-side fallback (absent request_id -> message id) matches +// clarification_history_query.go's permissionGroupKeyExpr SQL fallback +// exactly, end to end through filterMessagesByPermissionGroupKey: a bundle +// whose PermissionGroupKey is the message's own id (because the SQL side +// took the same fallback) must still keep that message. +func TestPermissionGroupKeyFromMessage_FallsBackToMessageIDWhenRequestIDAbsent(t *testing.T) { + created := testInboxNow.Add(-time.Hour) + msgWithoutRequestID := &taskmodels.Message{ + ID: "perm-no-request-id-msg", TaskSessionID: "sess-1", TaskID: "task-1", + Type: taskmodels.MessageTypePermissionRequest, Content: "delete the branch?", + CreatedAt: created, + } + msgs := map[string][]*taskmodels.Message{ + "pend-1": {msgWithoutRequestID}, + } + tasks := &fakeInboxTasksWithWorkflow{fakeInboxTasks: &fakeInboxTasks{ + tasks: map[string]*taskmodels.Task{"task-1": {ID: "task-1", Title: "Do the thing"}}, + }} + bundles := &fakeInboxBundleStoreWithHistory{fakeInboxBundleStore: &fakeInboxBundleStore{}} + h := newInboxHistoryTestHandler(t, msgs, tasks, bundles) + + views, err := h.buildInboxHistoryBundleViews(context.Background(), []taskmodels.ClarificationHistoryBundleSummary{ + { + PendingID: "pend-1", + TaskID: "task-1", + SessionID: "sess-1", + CreatedAt: created, + Reason: taskmodels.ClarificationHistoryReasonSuperseded, + AskingTurnID: "turn-a", + PermissionGroupKey: "perm-no-request-id-msg", + }, + }) + if err != nil { + t.Fatalf("buildInboxHistoryBundleViews: %v", err) + } + if len(views) != 1 { + t.Fatalf("expected 1 view, got %d: %+v", len(views), views) + } + if len(views[0].Messages) != 1 || views[0].Messages[0].ID != "perm-no-request-id-msg" { + t.Fatalf("expected the request_id-absent message to survive the message-id fallback, got %+v", views[0].Messages) + } +} + +func TestInboxHistoryBundleKind_PermissionVsClarification(t *testing.T) { + perm := []*taskmodels.Message{{Type: taskmodels.MessageTypePermissionRequest}} + if got := inboxHistoryBundleKind(perm); got != inboxHistoryBundleKindPermission { + t.Fatalf("expected permission, got %q", got) + } + clar := []*taskmodels.Message{{Type: taskmodels.MessageTypeClarificationRequest}} + if got := inboxHistoryBundleKind(clar); got != inboxHistoryBundleKindClarification { + t.Fatalf("expected clarification, got %q", got) + } + if got := inboxHistoryBundleKind(nil); got != "" { + t.Fatalf("expected empty for no messages, got %q", got) + } +} diff --git a/apps/backend/internal/task/models/clarification_history.go b/apps/backend/internal/task/models/clarification_history.go new file mode 100644 index 0000000000..6c3ed2049e --- /dev/null +++ b/apps/backend/internal/task/models/clarification_history.go @@ -0,0 +1,78 @@ +package models + +import "time" + +// ClarificationHistoryReason is the one, precedence-ordered explanation for +// why a bundle is unanswerable rather than live. Exactly these three values +// exist; a bundle matching none of them is live and is not a history bundle. +type ClarificationHistoryReason string + +const ( + // ClarificationHistoryReasonSuperseded: the bundle's turn is no longer + // its session's current conversational turn, or it is a permission + // request superseded by a newer one on the same turn. + ClarificationHistoryReasonSuperseded ClarificationHistoryReason = "superseded" + // ClarificationHistoryReasonSessionEnded: the owning session ended + // (COMPLETED/FAILED/CANCELLED) and no later turn superseded it. + ClarificationHistoryReasonSessionEnded ClarificationHistoryReason = "session_ended" + // ClarificationHistoryReasonUnreadable: a clarification-only reason -- at + // least one question in the bundle carries no resolvable identifier. + ClarificationHistoryReasonUnreadable ClarificationHistoryReason = "unreadable" +) + +// ClarificationHistoryBundleSummary is one row of ListInboxHistoryBundles: +// the same bundle identity as ClarificationBundleSummary plus the exclusion +// reason and the turn identity fields. +type ClarificationHistoryBundleSummary struct { + PendingID string + SessionID string + TaskID string + CreatedAt time.Time + // Reason is the exclusion reason, already resolved by precedence + // (superseded, then session_ended, then unreadable). + Reason ClarificationHistoryReason + // AskingTurnID is the turn that asked the bundle's question(s). + AskingTurnID string + // SupersedingTurnID is the turn that superseded AskingTurnID, set only + // when Reason is superseded AND the supersession is a turn-level + // supersession (a later turn started). A same-turn permission + // supersession shares AskingTurnID and has no separate superseding turn + // to name, so this stays empty rather than a fabricated identifier. + SupersedingTurnID string + // PermissionGroupKey identifies which logical request this bundle's rows + // belong to when a permission pending_id is reused across two distinct + // requests: the request's own request_id, falling back to a message id + // when request_id is absent. Empty for a clarification bundle, whose + // pending_id is never reused. Message hydration must filter a + // pending_id's messages down to this key before rendering, since + // FindMessagesByPendingIDs returns every message sharing the raw + // pending_id regardless of which logical request it belongs to. + PermissionGroupKey string +} + +// ListClarificationHistoryOptions filters and paginates +// ListInboxHistoryBundles / CountInboxHistoryBundles. WorkspaceID is +// required: the history read is always workspace-scoped by an +// already-authorized caller, mirroring the shipped Needs-you Inbox reads. +type ListClarificationHistoryOptions struct { + WorkspaceID string + // CursorCreatedAt/CursorPendingID are the last returned (created_at, + // pending_id) pair. An empty CursorPendingID means the first page. + // Ignored by CountInboxHistoryBundles, which is always the unbounded + // workspace-wide total. + CursorCreatedAt time.Time + CursorPendingID string + // Limit is the page size, already resolved by the caller to the default + // (default 50, capped at 200). Must be >= 1. Ignored by + // CountInboxHistoryBundles. + Limit int +} + +// ClarificationHistoryPage is one page of ListInboxHistoryBundles, ordered +// created_at ascending, then pending_id ascending. +type ClarificationHistoryPage struct { + Bundles []ClarificationHistoryBundleSummary + // HasMore is true when at least one further bundle exists beyond this + // page under the same filters. + HasMore bool +} diff --git a/apps/backend/internal/task/repository/sqlite/clarification_history_query.go b/apps/backend/internal/task/repository/sqlite/clarification_history_query.go new file mode 100644 index 0000000000..b99ad16602 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/clarification_history_query.go @@ -0,0 +1,254 @@ +package sqlite + +import ( + "context" + "database/sql" + "fmt" + "time" + + "github.com/kandev/kandev/internal/db/dialect" + "github.com/kandev/kandev/internal/task/models" +) + +// ListInboxHistoryBundles is the History tab's exported entry point: a +// separate, additive read from the operational pending-interaction path. It +// reuses this package's unexported +// currentTurnAuthority and nonTerminalSessionPredicate machinery only +// through turnAuthorityCurrentTurnIDExpr, and is never called from any sink +// that gates a workflow transition, emits a pending-action event, resolves +// an answer, or contributes to the pending-action projection -- +// ARCH-INBOX-HISTORY-ISOLATION enforces that statically. +func (r *Repository) ListInboxHistoryBundles(ctx context.Context, opts models.ListClarificationHistoryOptions) (*models.ClarificationHistoryPage, error) { + if opts.Limit < 1 { + return nil, fmt.Errorf("ListInboxHistoryBundles: limit must be >= 1, got %d", opts.Limit) + } + if opts.WorkspaceID == "" { + return nil, fmt.Errorf("ListInboxHistoryBundles: workspace_id is required") + } + + drv := r.ro.DriverName() + args := []interface{}{opts.WorkspaceID} + query := inboxHistoryQueryBase(drv) + "\nSELECT " + inboxHistorySelectColumns + "\nFROM reasoned r\nJOIN tasks t ON t.id = r.task_id\n" + inboxHistoryWhereClause + if opts.CursorPendingID != "" { + query += "\n AND (r.created_at > ? OR (r.created_at = ? AND r.pending_id > ?))" + args = append(args, opts.CursorCreatedAt, opts.CursorCreatedAt, opts.CursorPendingID) + } + query += "\nORDER BY r.created_at ASC, r.pending_id ASC\nLIMIT ?" + args = append(args, opts.Limit+1) + + rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(query), args...) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + bundles, err := scanInboxHistoryBundleRows(rows, dialect.IsPostgres(drv)) + if err != nil { + return nil, err + } + + page := &models.ClarificationHistoryPage{Bundles: bundles} + if len(bundles) > opts.Limit { + page.Bundles = bundles[:opts.Limit] + page.HasMore = true + } + return page, nil +} + +// CountInboxHistoryBundles is the History tab's second exported entry +// point: the unbounded, workspace-wide bundle total, independent of page +// size and of any cursor. Subject to the same ARCH-INBOX-HISTORY-ISOLATION +// isolation as ListInboxHistoryBundles. +func (r *Repository) CountInboxHistoryBundles(ctx context.Context, opts models.ListClarificationHistoryOptions) (int, error) { + if opts.WorkspaceID == "" { + return 0, fmt.Errorf("CountInboxHistoryBundles: workspace_id is required") + } + drv := r.ro.DriverName() + query := inboxHistoryQueryBase(drv) + "\nSELECT COUNT(*)\nFROM reasoned r\nJOIN tasks t ON t.id = r.task_id\n" + inboxHistoryWhereClause + + var total int + if err := r.ro.QueryRowContext(ctx, r.ro.Rebind(query), opts.WorkspaceID).Scan(&total); err != nil { + return 0, err + } + return total, nil +} + +// inboxHistoryWhereClause is shared by the page and count queries: the +// eligibility gate (non-terminal, unarchived, workspace-scoped) AND +// (superseded OR session_ended OR unreadable) -- a bundle matching none of +// the three is live and excluded from both queries. +const inboxHistoryWhereClause = `WHERE r.has_pending = 1 + AND t.archived_at IS NULL + AND t.workspace_id = ? + AND (r.turn_mismatch = 1 + OR (r.bundle_type = 'permission_request' AND r.permission_not_newest = 1) + OR r.is_session_ended = 1 + OR r.is_unreadable = 1)` + +// inboxHistorySelectColumns is the page query's column list; the count +// query only needs the WHERE clause's inputs, which inboxHistoryWhereClause +// already references by CTE column name. +const inboxHistorySelectColumns = `r.pending_id, r.session_id, r.task_id, r.created_at, r.bundle_type, + r.turn_id, r.current_turn_id, r.turn_mismatch, r.permission_not_newest, + r.is_session_ended, r.is_unreadable, r.permission_group_key` + +// inboxHistoryQueryBase builds the two CTEs shared by the page and count +// queries: `bundles` groups messages into per-pending_id aggregates exactly +// like clarificationBundleTableExpr's inner subquery, but across BOTH +// clarification_request and permission_request (eligibility is evaluated per +// bundle regardless of kind), additionally split by request_id for a +// permission_request row so a reused pending_id never merges two distinct +// requests; `reasoned` resolves the three raw exclusion signals once per +// bundle so the outer WHERE and SELECT never recompute the current-turn +// correlated subquery more than once. +func inboxHistoryQueryBase(drv string) string { + pendingIDExpr := dialect.JSONExtract(drv, "m.metadata", "pending_id") + statusExpr := dialect.JSONExtract(drv, "m.metadata", "status") + questionIDExpr := fmt.Sprintf( + "COALESCE(NULLIF(%s, ''), NULLIF(%s, ''), '')", + dialect.JSONExtract(drv, "m.metadata", "question_id"), + dialect.JSONExtractPath(drv, "m.metadata", "question", "id"), + ) + notParentQuestion := dialect.ExcludeTruthyMetadataPredicate(drv, "m.metadata", "parent_question") + permissionOrder := pendingActionMessageOrder(drv, "m2") + currentTurnIDExpr := turnAuthorityCurrentTurnIDExpr(drv, "bundles.session_id") + terminalStates := terminalSessionStatesExpr("s.state") + // A provider may reuse a permission's pending_id for a later, unrelated + // request once the original resolves (message.go's + // GetPermissionMessageByIdentity, service_messages.go's + // UpdatePermissionMessage, event_handlers_streaming.go's + // handlePermissionCancelledEvent). Grouping permission rows by pending_id + // alone would merge that later request with the resolved original into + // one bundle. request_id disambiguates them the same way those three + // sites do; message id is the fallback for a row with no request_id. + requestIDExpr := dialect.JSONExtract(drv, "m.metadata", "request_id") + permissionGroupKeyExpr := fmt.Sprintf( + "CASE WHEN m.type = 'permission_request' THEN COALESCE(NULLIF(%s, ''), m.id) ELSE '' END", + requestIDExpr, + ) + + return fmt.Sprintf(`WITH bundles AS ( + SELECT + %[1]s AS pending_id, + m.task_session_id AS session_id, + COALESCE(NULLIF(MIN(m.task_id), ''), MIN(ts.task_id)) AS task_id, + MIN(m.created_at) AS created_at, + MIN(m.type) AS bundle_type, + MIN(m.turn_id) AS turn_id, + %[8]s AS permission_group_key, + MAX(CASE WHEN COALESCE(%[2]s, '') IN ('', 'pending') THEN 1 ELSE 0 END) AS has_pending, + MAX(CASE WHEN %[3]s = '' THEN 1 ELSE 0 END) AS has_missing_question_id, + MAX(CASE WHEN pr.rn IS NOT NULL AND pr.rn > 1 THEN 1 ELSE 0 END) AS permission_not_newest + FROM task_session_messages m + JOIN task_sessions ts ON ts.id = m.task_session_id + LEFT JOIN ( + SELECT m2.id AS message_id, + ROW_NUMBER() OVER ( + PARTITION BY m2.task_session_id, m2.turn_id + ORDER BY m2.created_at DESC, %[6]s DESC + ) AS rn + FROM task_session_messages m2 + WHERE m2.type = 'permission_request' + ) pr ON pr.message_id = m.id + WHERE m.type IN ('clarification_request', 'permission_request') + AND %[4]s + GROUP BY %[1]s, m.task_session_id, %[8]s +), +reasoned AS ( + SELECT + bundles.*, + %[5]s AS current_turn_id, + CASE WHEN COALESCE(bundles.turn_id, '') <> COALESCE(%[5]s, '') THEN 1 ELSE 0 END AS turn_mismatch, + CASE WHEN %[7]s THEN 1 ELSE 0 END AS is_session_ended, + CASE WHEN bundles.bundle_type = 'clarification_request' AND bundles.has_missing_question_id = 1 THEN 1 ELSE 0 END AS is_unreadable + FROM bundles + JOIN task_sessions s ON s.id = bundles.session_id +) +`, pendingIDExpr, statusExpr, questionIDExpr, notParentQuestion, currentTurnIDExpr, permissionOrder, terminalStates, permissionGroupKeyExpr) +} + +// turnAuthorityCurrentTurnIDExpr resolves the session's current turn by +// currentTurnAuthority ALONE, never composed with +// nonTerminalSessionPredicate: composing the two would give a +// terminated session no current turn at all, making every one of its rows +// read superseded and making session_ended unreachable (system design, "Why +// superseded resolves on turn authority alone"). +func turnAuthorityCurrentTurnIDExpr(drv, sessionExpr string) string { + predicate, orderBy := currentTurnAuthority(drv, "turn_row") + return fmt.Sprintf(`( + SELECT turn_row.id FROM task_session_turns turn_row + WHERE turn_row.task_session_id = %s AND %s + ORDER BY %s LIMIT 1 + )`, sessionExpr, predicate, orderBy) +} + +// terminalSessionStatesExpr builds the session_ended raw signal: the same +// three terminal states nonTerminalSessionPredicate excludes, applied +// directly to a session-state column rather than as an EXISTS negation, +// since reasoned already joins task_sessions once per bundle. +func terminalSessionStatesExpr(stateColumn string) string { + return fmt.Sprintf("%s IN ('%s', '%s', '%s')", + stateColumn, + models.TaskSessionStateCompleted, + models.TaskSessionStateFailed, + models.TaskSessionStateCancelled, + ) +} + +// scanInboxHistoryBundleRows scans inboxHistorySelectColumns. created_at is +// a MIN() aggregate, which loses SQLite driver type affinity exactly like +// scanClarificationBundleRows's identical case. +func scanInboxHistoryBundleRows(rows *sql.Rows, isPostgres bool) ([]models.ClarificationHistoryBundleSummary, error) { + var bundles []models.ClarificationHistoryBundleSummary + for rows.Next() { + var ( + pendingID, sessionID, taskID, bundleType, turnID string + currentTurnID sql.NullString + turnMismatch, permissionNotNewest int + isSessionEnded, isUnreadable int + permissionGroupKey string + createdAt time.Time + ) + if isPostgres { + if err := rows.Scan(&pendingID, &sessionID, &taskID, &createdAt, &bundleType, + &turnID, ¤tTurnID, &turnMismatch, &permissionNotNewest, + &isSessionEnded, &isUnreadable, &permissionGroupKey); err != nil { + return nil, err + } + } else { + var createdAtRaw string + if err := rows.Scan(&pendingID, &sessionID, &taskID, &createdAtRaw, &bundleType, + &turnID, ¤tTurnID, &turnMismatch, &permissionNotNewest, + &isSessionEnded, &isUnreadable, &permissionGroupKey); err != nil { + return nil, err + } + createdAt = parseLegacyTimestamp(createdAtRaw) + } + + isSuperseded := turnMismatch == 1 || (bundleType == "permission_request" && permissionNotNewest == 1) + summary := models.ClarificationHistoryBundleSummary{ + PendingID: pendingID, + SessionID: sessionID, + TaskID: taskID, + CreatedAt: createdAt, + AskingTurnID: turnID, + PermissionGroupKey: permissionGroupKey, + } + switch { + case isSuperseded: + summary.Reason = models.ClarificationHistoryReasonSuperseded + if turnMismatch == 1 && currentTurnID.Valid { + summary.SupersedingTurnID = currentTurnID.String + } + case isSessionEnded == 1: + summary.Reason = models.ClarificationHistoryReasonSessionEnded + case isUnreadable == 1: + summary.Reason = models.ClarificationHistoryReasonUnreadable + } + bundles = append(bundles, summary) + } + if err := rows.Err(); err != nil { + return nil, err + } + return bundles, nil +} diff --git a/apps/backend/internal/task/repository/sqlite/clarification_history_query_postgres_test.go b/apps/backend/internal/task/repository/sqlite/clarification_history_query_postgres_test.go new file mode 100644 index 0000000000..172f860eb4 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/clarification_history_query_postgres_test.go @@ -0,0 +1,258 @@ +package sqlite + +import ( + "context" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/testutil" +) + +// newInboxHistoryPostgresRepo opens an isolated Postgres schema and mirrors +// newRepoForSessionTests, whose SQLite-only newRepoForSessionTests is what +// every other clarification_history_query_test.go case runs against. +// Skips unless KANDEV_TEST_POSTGRES_DSN is set. It also creates the "ws-1" +// workspace every case below scopes its bundles to: unlike SQLite, +// PostgreSQL enforces the tasks.workspace_id foreign key (see +// seedMetadataCASTask's comment on the same gap), so seedInboxHistorySession +// would otherwise fail before exercising any dialect-sensitive SQL at all. +func newInboxHistoryPostgresRepo(t *testing.T) *Repository { + t.Helper() + 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() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "ws-1"}); err != nil { + t.Fatalf("create workspace ws-1: %v", err) + } + return repo +} + +// TestListInboxHistoryBundlesRealSequenceSupersededVsOperationalPostgres is +// the Postgres counterpart of the SQLite test of the same name (minus the +// suffix): AC .8's regression sequence, and AC .5's read path, both depend on +// dialect-sensitive SQL (dialect.JSONExtract/JSONExtractPath, +// dialect.ExcludeTruthyMetadataPredicate, and the permission-ordering window +// function in inboxHistoryQueryBase) that has never run against Postgres +// syntax before this file. +func TestListInboxHistoryBundlesRealSequenceSupersededVsOperationalPostgres(t *testing.T) { + repo := newInboxHistoryPostgresRepo(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 9, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-superseded-pg", "session-superseded-pg", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-superseded-pg", "session-superseded-pg", "turn-old-pg", base, base) + createClarificationBundleMessage(t, repo, "clar-old-pg", "task-superseded-pg", "session-superseded-pg", "turn-old-pg", "pending-superseded-pg", "q1", base) + + // Start a new turn without answering the question. + createPendingActionTurn(t, repo, "task-superseded-pg", "session-superseded-pg", "turn-new-pg", base.Add(time.Minute), base.Add(time.Minute)) + createPendingActionMessage(t, repo, "ordinary-new-pg", "task-superseded-pg", "session-superseded-pg", "turn-new-pg", + models.MessageTypeMessage, "", base.Add(time.Minute)) + + operational, err := repo.ListUnresolvedClarificationBundles(ctx, models.ListClarificationBundlesOptions{ + Unscoped: true, WorkspaceID: "ws-1", Limit: 50, + }) + if err != nil { + t.Fatalf("ListUnresolvedClarificationBundles: %v", err) + } + if _, found := findBundleByPendingID(operational.Bundles, "pending-superseded-pg"); found { + t.Fatal("operational listing returned a superseded bundle") + } + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + bundle, found := findHistoryBundle(page.Bundles, "pending-superseded-pg") + if !found { + t.Fatalf("history listing did not return the superseded bundle: %+v", page.Bundles) + } + if bundle.Reason != models.ClarificationHistoryReasonSuperseded { + t.Fatalf("reason = %q, want superseded", bundle.Reason) + } + if bundle.AskingTurnID != "turn-old-pg" { + t.Fatalf("asking turn = %q, want turn-old-pg", bundle.AskingTurnID) + } + if bundle.SupersedingTurnID != "turn-new-pg" { + t.Fatalf("superseding turn = %q, want turn-new-pg", bundle.SupersedingTurnID) + } +} + +// TestListInboxHistoryBundlesSessionEndedOmitsSupersedingTurnPostgres pins +// the session_ended reason and AC .10's omission rule for it against +// Postgres's terminalSessionStatesExpr branch. +func TestListInboxHistoryBundlesSessionEndedOmitsSupersedingTurnPostgres(t *testing.T) { + repo := newInboxHistoryPostgresRepo(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 10, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-ended-pg", "session-ended-pg", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-ended-pg", "session-ended-pg", "turn-ended-pg", base, base) + createClarificationBundleMessage(t, repo, "clar-ended-pg", "task-ended-pg", "session-ended-pg", "turn-ended-pg", "pending-ended-pg", "q1", base) + + session, err := repo.GetTaskSession(ctx, "session-ended-pg") + if err != nil { + t.Fatalf("GetTaskSession: %v", err) + } + session.State = models.TaskSessionStateCompleted + if err := repo.UpdateTaskSession(ctx, session); err != nil { + t.Fatalf("UpdateTaskSession: %v", err) + } + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + bundle, found := findHistoryBundle(page.Bundles, "pending-ended-pg") + if !found { + t.Fatalf("history listing did not return the session_ended bundle: %+v", page.Bundles) + } + if bundle.Reason != models.ClarificationHistoryReasonSessionEnded { + t.Fatalf("reason = %q, want session_ended", bundle.Reason) + } + if bundle.SupersedingTurnID != "" { + t.Fatalf("superseding turn = %q, want omitted", bundle.SupersedingTurnID) + } +} + +// TestListInboxHistoryBundlesUnreadableClarificationOnlyPostgres pins that +// unreadable applies only to clarification bundles on Postgres's +// dialect.JSONExtract/dialect.ExcludeTruthyMetadataPredicate branches, the +// same case the SQLite test of the same name (minus the suffix) covers. +func TestListInboxHistoryBundlesUnreadableClarificationOnlyPostgres(t *testing.T) { + repo := newInboxHistoryPostgresRepo(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 11, 0, 0, 0, time.UTC) + + // Unreadable clarification: current turn, missing question_id, so it's + // otherwise live except for the missing identifier. + seedInboxHistorySession(t, repo, "task-unreadable-pg", "session-unreadable-pg", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-unreadable-pg", "session-unreadable-pg", "turn-unreadable-pg", base, base) + createClarificationBundleMessage(t, repo, "clar-unreadable-pg", "task-unreadable-pg", "session-unreadable-pg", "turn-unreadable-pg", "pending-unreadable-pg", "", base) + + // Live, current-turn, non-terminal-session permission request: must + // NOT be listed at all (it is live -- none of the three reasons hold). + seedInboxHistorySession(t, repo, "task-live-perm-pg", "session-live-perm-pg", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-live-perm-pg", "session-live-perm-pg", "turn-live-perm-pg", base, base) + createInteractionMessage(t, repo, "perm-live-pg", "task-live-perm-pg", "session-live-perm-pg", "turn-live-perm-pg", + models.MessageTypePermissionRequest, map[string]interface{}{"pending_id": "pending-live-perm-pg"}, base) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + bundle, found := findHistoryBundle(page.Bundles, "pending-unreadable-pg") + if !found { + t.Fatalf("history listing did not return the unreadable bundle: %+v", page.Bundles) + } + if bundle.Reason != models.ClarificationHistoryReasonUnreadable { + t.Fatalf("reason = %q, want unreadable", bundle.Reason) + } + if _, found := findHistoryBundle(page.Bundles, "pending-live-perm-pg"); found { + t.Fatal("a live, current-turn, non-terminal-session permission request must not be listed") + } +} + +// TestListInboxHistoryBundlesPermissionSameTurnSupersessionPostgres pins +// AC .2's second superseded clause on Postgres: an older permission request +// superseded by a newer one on the SAME turn is superseded with no separate +// superseding turn to name (AC .10), exercising the +// ROW_NUMBER() OVER (PARTITION BY ... ORDER BY ...) window function that +// inboxHistoryQueryBase builds for permission_not_newest. +func TestListInboxHistoryBundlesPermissionSameTurnSupersessionPostgres(t *testing.T) { + repo := newInboxHistoryPostgresRepo(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 13, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-multi-perm-pg", "session-multi-perm-pg", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-multi-perm-pg", "session-multi-perm-pg", "turn-multi-perm-pg", base, base) + createInteractionMessage(t, repo, "perm-older-pg", "task-multi-perm-pg", "session-multi-perm-pg", "turn-multi-perm-pg", + models.MessageTypePermissionRequest, map[string]interface{}{"pending_id": "pending-perm-older-pg"}, base) + createInteractionMessage(t, repo, "perm-newer-pg", "task-multi-perm-pg", "session-multi-perm-pg", "turn-multi-perm-pg", + models.MessageTypePermissionRequest, map[string]interface{}{"pending_id": "pending-perm-newer-pg"}, base.Add(time.Second)) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + older, found := findHistoryBundle(page.Bundles, "pending-perm-older-pg") + if !found { + t.Fatalf("history listing did not return the superseded permission: %+v", page.Bundles) + } + if older.Reason != models.ClarificationHistoryReasonSuperseded { + t.Fatalf("older permission reason = %q, want superseded", older.Reason) + } + if older.SupersedingTurnID != "" { + t.Fatalf("same-turn superseding turn = %q, want omitted", older.SupersedingTurnID) + } + if _, found := findHistoryBundle(page.Bundles, "pending-perm-newer-pg"); found { + t.Fatal("the newest permission on the current turn is live and must not be listed") + } +} + +// TestListInboxHistoryBundlesPendingIDReuseAcrossTurnsPostgres is the +// Postgres counterpart of the SQLite test of the same name (minus the +// suffix): the GROUP BY that keys a permission bundle's identity is +// dialect-sensitive SQL exercised for the first time here. +func TestListInboxHistoryBundlesPendingIDReuseAcrossTurnsPostgres(t *testing.T) { + repo := newInboxHistoryPostgresRepo(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 15, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-reused-pending-pg", "session-reused-pending-pg", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-reused-pending-pg", "session-reused-pending-pg", "turn-a-old-pg", base, base) + createInteractionMessage(t, repo, "perm-reused-old-pg", "task-reused-pending-pg", "session-reused-pending-pg", "turn-a-old-pg", + models.MessageTypePermissionRequest, + map[string]interface{}{"pending_id": "pending-reused-pg", "request_id": "req-old-pg", "status": "approved"}, + base) + + createPendingActionTurn(t, repo, "task-reused-pending-pg", "session-reused-pending-pg", "turn-b-new-pg", base.Add(time.Minute), base.Add(time.Minute)) + createInteractionMessage(t, repo, "perm-reused-new-pg", "task-reused-pending-pg", "session-reused-pending-pg", "turn-b-new-pg", + models.MessageTypePermissionRequest, + map[string]interface{}{"pending_id": "pending-reused-pg", "request_id": "req-new-pg"}, + base.Add(time.Minute)) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + if bundle, found := findHistoryBundle(page.Bundles, "pending-reused-pg"); found { + t.Fatalf("a live current-turn permission must not be merged with a resolved reused-pending_id request into a superseded bundle: %+v", bundle) + } +} + +// TestCountInboxHistoryBundlesIsWorkspaceWideTotalPostgres pins AC .20's +// total on Postgres: independent of page size, same as the SQLite test of +// the same name (minus the suffix). +func TestCountInboxHistoryBundlesIsWorkspaceWideTotalPostgres(t *testing.T) { + repo := newInboxHistoryPostgresRepo(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 16, 0, 0, 0, time.UTC) + + for i, suffix := range []string{"a", "b", "c"} { + taskID, sessionID, turnID := "task-count-pg-"+suffix, "session-count-pg-"+suffix, "turn-count-pg-"+suffix + seedInboxHistorySession(t, repo, taskID, sessionID, "ws-1", models.TaskSessionStateCompleted, false) + createdAt := base.Add(time.Duration(i) * time.Minute) + createPendingActionTurn(t, repo, taskID, sessionID, turnID, createdAt, createdAt) + createClarificationBundleMessage(t, repo, "clar-count-pg-"+suffix, taskID, sessionID, turnID, "pending-count-pg-"+suffix, "q1", createdAt) + } + + total, err := repo.CountInboxHistoryBundles(ctx, models.ListClarificationHistoryOptions{WorkspaceID: "ws-1"}) + if err != nil { + t.Fatalf("CountInboxHistoryBundles: %v", err) + } + if total != 3 { + t.Fatalf("total = %d, want 3", total) + } + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 1)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + if len(page.Bundles) != 1 { + t.Fatalf("page bundles = %d, want 1 (total must stay independent of page size)", len(page.Bundles)) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/clarification_history_query_test.go b/apps/backend/internal/task/repository/sqlite/clarification_history_query_test.go new file mode 100644 index 0000000000..191a5a5fb4 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/clarification_history_query_test.go @@ -0,0 +1,435 @@ +package sqlite + +import ( + "context" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" +) + +// seedInboxHistorySession creates a workspace-scoped task and a session in +// the given state, mirroring seedPendingActionSession but with an explicit +// workspace and (optionally) archived task, both of which +// ListInboxHistoryBundles must filter on (AC .2 workspace scope, AC .4 +// archived exclusion). +func seedInboxHistorySession(t *testing.T, repo *Repository, taskID, sessionID, workspaceID string, sessionState models.TaskSessionState, archived bool) { + t.Helper() + ctx := context.Background() + task := &models.Task{ID: taskID, Title: taskID, WorkspaceID: workspaceID} + if err := repo.CreateTask(ctx, task); err != nil { + t.Fatalf("CreateTask(%s): %v", taskID, err) + } + if err := repo.CreateTaskSession(ctx, &models.TaskSession{ + ID: sessionID, TaskID: taskID, State: sessionState, + }); err != nil { + t.Fatalf("CreateTaskSession(%s): %v", sessionID, err) + } + if archived { + if err := repo.ArchiveTask(ctx, taskID); err != nil { + t.Fatalf("ArchiveTask(%s): %v", taskID, err) + } + } +} + +func unscopedHistoryOpts(workspaceID string, limit int) models.ListClarificationHistoryOptions { + return models.ListClarificationHistoryOptions{WorkspaceID: workspaceID, Limit: limit} +} + +func findHistoryBundle(bundles []models.ClarificationHistoryBundleSummary, pendingID string) (models.ClarificationHistoryBundleSummary, bool) { + for _, b := range bundles { + if b.PendingID == pendingID { + return b, true + } + } + return models.ClarificationHistoryBundleSummary{}, false +} + +// TestListInboxHistoryBundlesRealSequenceSupersededVsOperational drives +// AC .8's exact regression sequence: ask a question, then start a new turn +// without answering it. The operational listing must not return it and the +// history listing must. +func TestListInboxHistoryBundlesRealSequenceSupersededVsOperational(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 9, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-superseded", "session-superseded", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-superseded", "session-superseded", "turn-old", base, base) + createClarificationBundleMessage(t, repo, "clar-old", "task-superseded", "session-superseded", "turn-old", "pending-superseded", "q1", base) + + // Start a new turn without answering the question. + createPendingActionTurn(t, repo, "task-superseded", "session-superseded", "turn-new", base.Add(time.Minute), base.Add(time.Minute)) + createPendingActionMessage(t, repo, "ordinary-new", "task-superseded", "session-superseded", "turn-new", + models.MessageTypeMessage, "", base.Add(time.Minute)) + + operational, err := repo.ListUnresolvedClarificationBundles(ctx, models.ListClarificationBundlesOptions{ + Unscoped: true, WorkspaceID: "ws-1", Limit: 50, + }) + if err != nil { + t.Fatalf("ListUnresolvedClarificationBundles: %v", err) + } + if _, found := findBundleByPendingID(operational.Bundles, "pending-superseded"); found { + t.Fatal("operational listing returned a superseded bundle") + } + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + bundle, found := findHistoryBundle(page.Bundles, "pending-superseded") + if !found { + t.Fatalf("history listing did not return the superseded bundle: %+v", page.Bundles) + } + if bundle.Reason != models.ClarificationHistoryReasonSuperseded { + t.Fatalf("reason = %q, want superseded", bundle.Reason) + } + if bundle.AskingTurnID != "turn-old" { + t.Fatalf("asking turn = %q, want turn-old", bundle.AskingTurnID) + } + if bundle.SupersedingTurnID != "turn-new" { + t.Fatalf("superseding turn = %q, want turn-new", bundle.SupersedingTurnID) + } +} + +func findBundleByPendingID(bundles []models.ClarificationBundleSummary, pendingID string) (models.ClarificationBundleSummary, bool) { + for _, b := range bundles { + if b.PendingID == pendingID { + return b, true + } + } + return models.ClarificationBundleSummary{}, false +} + +// TestListInboxHistoryBundlesSessionEndedOmitsSupersedingTurn pins the +// session_ended reason and AC .10's omission rule for it. +func TestListInboxHistoryBundlesSessionEndedOmitsSupersedingTurn(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 10, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-ended", "session-ended", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-ended", "session-ended", "turn-ended", base, base) + createClarificationBundleMessage(t, repo, "clar-ended", "task-ended", "session-ended", "turn-ended", "pending-ended", "q1", base) + + session, err := repo.GetTaskSession(ctx, "session-ended") + if err != nil { + t.Fatalf("GetTaskSession: %v", err) + } + session.State = models.TaskSessionStateCompleted + if err := repo.UpdateTaskSession(ctx, session); err != nil { + t.Fatalf("UpdateTaskSession: %v", err) + } + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + bundle, found := findHistoryBundle(page.Bundles, "pending-ended") + if !found { + t.Fatalf("history listing did not return the session_ended bundle: %+v", page.Bundles) + } + if bundle.Reason != models.ClarificationHistoryReasonSessionEnded { + t.Fatalf("reason = %q, want session_ended", bundle.Reason) + } + if bundle.SupersedingTurnID != "" { + t.Fatalf("superseding turn = %q, want omitted", bundle.SupersedingTurnID) + } +} + +// TestListInboxHistoryBundlesUnreadableClarificationOnly pins that +// unreadable applies only to clarification bundles and never to a +// permission bundle, which carries no resolvable question identifier by +// construction (system design "A permission record is a different shape"). +func TestListInboxHistoryBundlesUnreadableClarificationOnly(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 11, 0, 0, 0, time.UTC) + + // Unreadable clarification: current turn, missing question_id, so it's + // otherwise live except for the missing identifier. + seedInboxHistorySession(t, repo, "task-unreadable", "session-unreadable", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-unreadable", "session-unreadable", "turn-unreadable", base, base) + createClarificationBundleMessage(t, repo, "clar-unreadable", "task-unreadable", "session-unreadable", "turn-unreadable", "pending-unreadable", "", base) + + // Live, current-turn, non-terminal-session permission request: must + // NOT be listed at all (it is live -- none of the three reasons hold). + seedInboxHistorySession(t, repo, "task-live-perm", "session-live-perm", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-live-perm", "session-live-perm", "turn-live-perm", base, base) + createInteractionMessage(t, repo, "perm-live", "task-live-perm", "session-live-perm", "turn-live-perm", + models.MessageTypePermissionRequest, map[string]interface{}{"pending_id": "pending-live-perm"}, base) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + bundle, found := findHistoryBundle(page.Bundles, "pending-unreadable") + if !found { + t.Fatalf("history listing did not return the unreadable bundle: %+v", page.Bundles) + } + if bundle.Reason != models.ClarificationHistoryReasonUnreadable { + t.Fatalf("reason = %q, want unreadable", bundle.Reason) + } + if _, found := findHistoryBundle(page.Bundles, "pending-live-perm"); found { + t.Fatal("a live, current-turn, non-terminal-session permission request must not be listed") + } +} + +// TestListInboxHistoryBundlesPrecedenceSupersededOverUnreadable pins AC .2's +// precedence test: a bundle that is both superseded and unreadable reports +// superseded. +func TestListInboxHistoryBundlesPrecedenceSupersededOverUnreadable(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 12, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-both", "session-both", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-both", "session-both", "turn-both-old", base, base) + createClarificationBundleMessage(t, repo, "clar-both", "task-both", "session-both", "turn-both-old", "pending-both", "", base) + createPendingActionTurn(t, repo, "task-both", "session-both", "turn-both-new", base.Add(time.Minute), base.Add(time.Minute)) + createPendingActionMessage(t, repo, "ordinary-both", "task-both", "session-both", "turn-both-new", + models.MessageTypeMessage, "", base.Add(time.Minute)) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + bundle, found := findHistoryBundle(page.Bundles, "pending-both") + if !found { + t.Fatalf("history listing did not return the bundle: %+v", page.Bundles) + } + if bundle.Reason != models.ClarificationHistoryReasonSuperseded { + t.Fatalf("reason = %q, want superseded (precedence over unreadable)", bundle.Reason) + } +} + +// TestListInboxHistoryBundlesPermissionSameTurnSupersession pins AC .2's +// second superseded clause: an older permission request superseded by a +// newer one on the SAME turn is superseded with no separate superseding +// turn to name (AC .10), while the newest stays live and unlisted. +func TestListInboxHistoryBundlesPermissionSameTurnSupersession(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 13, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-multi-perm", "session-multi-perm", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-multi-perm", "session-multi-perm", "turn-multi-perm", base, base) + createInteractionMessage(t, repo, "perm-older", "task-multi-perm", "session-multi-perm", "turn-multi-perm", + models.MessageTypePermissionRequest, map[string]interface{}{"pending_id": "pending-perm-older"}, base) + createInteractionMessage(t, repo, "perm-newer", "task-multi-perm", "session-multi-perm", "turn-multi-perm", + models.MessageTypePermissionRequest, map[string]interface{}{"pending_id": "pending-perm-newer"}, base.Add(time.Second)) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + older, found := findHistoryBundle(page.Bundles, "pending-perm-older") + if !found { + t.Fatalf("history listing did not return the superseded permission: %+v", page.Bundles) + } + if older.Reason != models.ClarificationHistoryReasonSuperseded { + t.Fatalf("older permission reason = %q, want superseded", older.Reason) + } + if older.SupersedingTurnID != "" { + t.Fatalf("same-turn superseding turn = %q, want omitted", older.SupersedingTurnID) + } + if _, found := findHistoryBundle(page.Bundles, "pending-perm-newer"); found { + t.Fatal("the newest permission on the current turn is live and must not be listed") + } +} + +// TestListInboxHistoryBundlesPendingIDReuseAcrossTurns pins the message.go / +// service_messages.go / event_handlers_streaming.go documented behavior that +// a provider may reuse a permission's pending_id for a later, unrelated +// request once the original resolves. The reused id must not let an old, +// already-resolved permission (a different turn, a different request) merge +// with a live, currently-answerable one on the session's current turn: the +// live request must stay unlisted rather than being reported superseded. +func TestListInboxHistoryBundlesPendingIDReuseAcrossTurns(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 15, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-reused-pending", "session-reused-pending", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-reused-pending", "session-reused-pending", "turn-a-old", base, base) + createInteractionMessage(t, repo, "perm-reused-old", "task-reused-pending", "session-reused-pending", "turn-a-old", + models.MessageTypePermissionRequest, + map[string]interface{}{"pending_id": "pending-reused", "request_id": "req-old", "status": "approved"}, + base) + + createPendingActionTurn(t, repo, "task-reused-pending", "session-reused-pending", "turn-b-new", base.Add(time.Minute), base.Add(time.Minute)) + createInteractionMessage(t, repo, "perm-reused-new", "task-reused-pending", "session-reused-pending", "turn-b-new", + models.MessageTypePermissionRequest, + map[string]interface{}{"pending_id": "pending-reused", "request_id": "req-new"}, + base.Add(time.Minute)) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + if bundle, found := findHistoryBundle(page.Bundles, "pending-reused"); found { + t.Fatalf("a live current-turn permission must not be merged with a resolved reused-pending_id request into a superseded bundle: %+v", bundle) + } +} + +// TestListInboxHistoryBundlesPendingIDReusePermissionGroupKeyDistinguishesRequests +// pins that once a reused pending_id's second request is itself superseded and +// returned, its PermissionGroupKey is its own request_id rather than the first +// (already-resolved) request's -- message hydration keys off this field to +// keep the two requests' rendered content apart even though +// FindMessagesByPendingIDs returns every message sharing the raw pending_id. +func TestListInboxHistoryBundlesPendingIDReusePermissionGroupKeyDistinguishesRequests(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 17, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-reused-group-key", "session-reused-group-key", "ws-1", models.TaskSessionStateWaitingForInput, false) + createPendingActionTurn(t, repo, "task-reused-group-key", "session-reused-group-key", "turn-a-old", base, base) + createInteractionMessage(t, repo, "perm-group-key-old", "task-reused-group-key", "session-reused-group-key", "turn-a-old", + models.MessageTypePermissionRequest, + map[string]interface{}{"pending_id": "pending-reused-group-key", "request_id": "req-old-group-key", "status": "approved"}, + base) + + createPendingActionTurn(t, repo, "task-reused-group-key", "session-reused-group-key", "turn-b-new", base.Add(time.Minute), base.Add(time.Minute)) + createInteractionMessage(t, repo, "perm-group-key-new", "task-reused-group-key", "session-reused-group-key", "turn-b-new", + models.MessageTypePermissionRequest, + map[string]interface{}{"pending_id": "pending-reused-group-key", "request_id": "req-new-group-key"}, + base.Add(time.Minute)) + + // A third turn supersedes turn-b-new without answering it, making the new + // request itself History-eligible. + createPendingActionTurn(t, repo, "task-reused-group-key", "session-reused-group-key", "turn-c", base.Add(2*time.Minute), base.Add(2*time.Minute)) + createPendingActionMessage(t, repo, "ordinary-group-key", "task-reused-group-key", "session-reused-group-key", "turn-c", + models.MessageTypeMessage, "", base.Add(2*time.Minute)) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + bundle, found := findHistoryBundle(page.Bundles, "pending-reused-group-key") + if !found { + t.Fatalf("history listing did not return the superseded reused-pending_id bundle: %+v", page.Bundles) + } + if bundle.Reason != models.ClarificationHistoryReasonSuperseded { + t.Fatalf("reason = %q, want superseded", bundle.Reason) + } + if bundle.PermissionGroupKey != "req-new-group-key" { + t.Fatalf("PermissionGroupKey = %q, want req-new-group-key (must not resolve to the old, already-resolved request's key)", bundle.PermissionGroupKey) + } +} + +// TestListInboxHistoryBundlesExcludesArchivedAndParentQuestion pins AC .4 +// (archived task) and AC .2's parent_question eligibility clause. +func TestListInboxHistoryBundlesExcludesArchivedAndParentQuestion(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 14, 0, 0, 0, time.UTC) + + seedInboxHistorySession(t, repo, "task-archived", "session-archived", "ws-1", models.TaskSessionStateCompleted, true) + createPendingActionTurn(t, repo, "task-archived", "session-archived", "turn-archived", base, base) + createClarificationBundleMessage(t, repo, "clar-archived", "task-archived", "session-archived", "turn-archived", "pending-archived", "q1", base) + + seedInboxHistorySession(t, repo, "task-parent-question", "session-parent-question", "ws-1", models.TaskSessionStateCompleted, false) + createPendingActionTurn(t, repo, "task-parent-question", "session-parent-question", "turn-parent-question", base, base) + if err := repo.CreateMessage(ctx, &models.Message{ + ID: "clar-parent-question", TaskSessionID: "session-parent-question", TaskID: "task-parent-question", + TurnID: "turn-parent-question", AuthorType: models.MessageAuthorAgent, + Type: models.MessageTypeClarificationRequest, + Metadata: map[string]interface{}{ + "pending_id": "pending-parent-question", "question_id": "q1", "status": "pending", + "parent_question": true, + }, + CreatedAt: base, + }); err != nil { + t.Fatalf("CreateMessage: %v", err) + } + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 50)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + if _, found := findHistoryBundle(page.Bundles, "pending-archived"); found { + t.Fatal("archived task's bundle must not be listed") + } + if _, found := findHistoryBundle(page.Bundles, "pending-parent-question"); found { + t.Fatal("parent_question bundle must not be listed") + } +} + +// TestListInboxHistoryBundlesOrderingAndPagination pins AC .18 (ordering) +// and AC .20 (limit+1 detection, exclusive keyset cursor, workspace scope). +func TestListInboxHistoryBundlesOrderingAndPagination(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 15, 0, 0, 0, time.UTC) + + for i, suffix := range []string{"a", "b", "c"} { + taskID, sessionID, turnID := "task-order-"+suffix, "session-order-"+suffix, "turn-order-"+suffix + seedInboxHistorySession(t, repo, taskID, sessionID, "ws-1", models.TaskSessionStateCompleted, false) + createdAt := base.Add(time.Duration(i) * time.Minute) + createPendingActionTurn(t, repo, taskID, sessionID, turnID, createdAt, createdAt) + createClarificationBundleMessage(t, repo, "clar-order-"+suffix, taskID, sessionID, turnID, "pending-order-"+suffix, "q1", createdAt) + } + + // Another workspace's bundle must never appear. + seedInboxHistorySession(t, repo, "task-order-other-ws", "session-order-other-ws", "ws-2", models.TaskSessionStateCompleted, false) + createPendingActionTurn(t, repo, "task-order-other-ws", "session-order-other-ws", "turn-order-other-ws", base, base) + createClarificationBundleMessage(t, repo, "clar-order-other-ws", "task-order-other-ws", "session-order-other-ws", "turn-order-other-ws", "pending-order-other-ws", "q1", base) + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 2)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + if len(page.Bundles) != 2 || !page.HasMore { + t.Fatalf("page = %+v, want 2 bundles with HasMore", page) + } + if page.Bundles[0].PendingID != "pending-order-a" || page.Bundles[1].PendingID != "pending-order-b" { + t.Fatalf("bundles = %v, want [pending-order-a pending-order-b]", []string{page.Bundles[0].PendingID, page.Bundles[1].PendingID}) + } + + last := page.Bundles[len(page.Bundles)-1] + next, err := repo.ListInboxHistoryBundles(ctx, models.ListClarificationHistoryOptions{ + WorkspaceID: "ws-1", Limit: 50, CursorCreatedAt: last.CreatedAt, CursorPendingID: last.PendingID, + }) + if err != nil { + t.Fatalf("ListInboxHistoryBundles (cursor): %v", err) + } + if len(next.Bundles) != 1 || next.Bundles[0].PendingID != "pending-order-c" { + t.Fatalf("next page = %v, want exactly [pending-order-c]", next.Bundles) + } + if next.HasMore { + t.Fatal("final page reported HasMore") + } +} + +// TestCountInboxHistoryBundlesIsWorkspaceWideTotal pins AC .20's total: it +// counts every eligible bundle for the workspace, independent of page size. +func TestCountInboxHistoryBundlesIsWorkspaceWideTotal(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + base := time.Date(2026, time.September, 1, 16, 0, 0, 0, time.UTC) + + for i, suffix := range []string{"a", "b", "c"} { + taskID, sessionID, turnID := "task-count-"+suffix, "session-count-"+suffix, "turn-count-"+suffix + seedInboxHistorySession(t, repo, taskID, sessionID, "ws-1", models.TaskSessionStateCompleted, false) + createdAt := base.Add(time.Duration(i) * time.Minute) + createPendingActionTurn(t, repo, taskID, sessionID, turnID, createdAt, createdAt) + createClarificationBundleMessage(t, repo, "clar-count-"+suffix, taskID, sessionID, turnID, "pending-count-"+suffix, "q1", createdAt) + } + + total, err := repo.CountInboxHistoryBundles(ctx, models.ListClarificationHistoryOptions{WorkspaceID: "ws-1"}) + if err != nil { + t.Fatalf("CountInboxHistoryBundles: %v", err) + } + if total != 3 { + t.Fatalf("total = %d, want 3", total) + } + + page, err := repo.ListInboxHistoryBundles(ctx, unscopedHistoryOpts("ws-1", 1)) + if err != nil { + t.Fatalf("ListInboxHistoryBundles: %v", err) + } + if len(page.Bundles) != 1 { + t.Fatalf("page bundles = %d, want 1 (total must stay independent of page size)", len(page.Bundles)) + } +} diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index 301f9af42c..8dfd9dcca4 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -1892,6 +1892,18 @@ func (s *Service) GetTasksByIDs(ctx context.Context, ids []string) ([]*models.Ta return s.tasks.GetTasksByIDs(ctx, ids) } +// GetWorkflowStep resolves one workflow step by ID for a caller that has +// already authorized the owning task/workspace, mirroring GetTasksByIDs. +// The Inbox History read uses this to test whether a task's current step +// starts an agent; s.workflowStepGetter is always wired in production, but a +// nil getter omits the label rather than panicking. +func (s *Service) GetWorkflowStep(ctx context.Context, stepID string) (*wfmodels.WorkflowStep, error) { + if s.workflowStepGetter == nil { + return nil, nil + } + return s.workflowStepGetter.GetStep(ctx, stepID) +} + func (s *Service) tryUpdateTaskPriorityOnly( ctx context.Context, id string, diff --git a/apps/web/app/needs-you-inbox/needs-you-inbox-page-client.test.tsx b/apps/web/app/needs-you-inbox/needs-you-inbox-page-client.test.tsx index 003becb3b9..0261755b64 100644 --- a/apps/web/app/needs-you-inbox/needs-you-inbox-page-client.test.tsx +++ b/apps/web/app/needs-you-inbox/needs-you-inbox-page-client.test.tsx @@ -1,4 +1,4 @@ -import { cleanup, render, screen } from "@testing-library/react"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { sessionId as toSessionId, taskId as toTaskId } from "@/lib/types/http"; import type { ClarificationInboxBundle } from "@/lib/types/clarification-inbox"; @@ -7,10 +7,13 @@ import type { FailedInboxRow as FailedInboxRowData } from "@/lib/types/failed-in const mocks = vi.hoisted(() => ({ bumpRefreshTick: vi.fn(), useFailedInboxController: vi.fn(), + inboxHistoryRefresh: vi.fn(), + inboxHistoryLoadMore: vi.fn(), })); const EMPTY_TESTID = "needs-you-inbox-empty"; const ERROR_TESTID = "needs-you-inbox-error"; +const HISTORY_TAB_TESTID = "inbox-tab-history"; let needsYouState: { status: "idle" | "loading" | "ready" | "error"; @@ -29,17 +32,55 @@ let failedState: { status: "idle" | "loading" | "ready" | "error"; }; +let historyState: { + status: "idle" | "loading" | "ready" | "error"; + bundles: unknown[]; + total: number; + hasMore: boolean; + isLoadingMore?: boolean; + loadMoreError?: boolean; +}; + vi.mock("@/components/state-provider", () => ({ // eslint-disable-next-line @typescript-eslint/no-explicit-any useAppStore: (selector: (s: any) => unknown) => selector({ needsYouInbox: { byWorkspaceId: { w1: needsYouState } }, failedInbox: { byWorkspaceId: { w1: failedState } }, + inboxHistory: { byWorkspaceId: { w1: historyState } }, workspaces: { activeId: workspacesActiveId, items: [{ id: "w1", name: "Kegmil V2" }] }, bumpNeedsYouInboxRefreshTick: mocks.bumpRefreshTick, }), })); +vi.mock("@/hooks/domains/inbox-history/use-inbox-history-controller", () => ({ + useInboxHistoryController: () => ({ + refresh: mocks.inboxHistoryRefresh, + loadMore: mocks.inboxHistoryLoadMore, + }), +})); + +vi.mock("@/components/inbox-history/inbox-history-list", () => ({ + InboxHistoryList: ({ bundles, onLoadMore }: { bundles: unknown[]; onLoadMore: () => void }) => ( + <> +
{bundles.length}
+ + ), +})); + // The shell is the app's own chrome (topbar, nav trigger, scroll container) // and pulls the whole nav context in with it; these cases are about which view // the page resolves to, so it is stubbed down to the title it is handed. @@ -119,8 +160,11 @@ function setLocation(path: string) { beforeEach(() => { mocks.bumpRefreshTick.mockReset(); mocks.useFailedInboxController.mockReset(); + mocks.inboxHistoryRefresh.mockReset(); + mocks.inboxHistoryLoadMore.mockReset(); needsYouState = { status: "idle", bundles: [], hiddenCount: 0, hasMore: false }; failedState = { rows: [], count: 0, truncated: false, status: "idle" }; + historyState = { status: "idle", bundles: [], total: 0, hasMore: false }; workspacesActiveId = "w1"; setLocation("/needs-you-inbox"); }); @@ -369,3 +413,64 @@ describe("NeedsYouInboxPageClient — Failed tab (REQ-UI-INBOX-FAILED-001)", () expect(mocks.useFailedInboxController).toHaveBeenCalledWith("failed"); }); }); + +// AC .1: a variant="line" tab strip, "Needs you" first and default-selected, +// "History" second. AC .16: the History tab's own bundle count. +describe("NeedsYouInboxPageClient — tab strip", () => { + it("selects Needs you by default, with its row set, count and behavior unchanged (AC .1)", () => { + needsYouState = { status: "ready", bundles: [bundle("p1")], hiddenCount: 0, hasMore: false }; + render(); + + expect(screen.getAllByTestId("stub-row")).toHaveLength(1); + expect(screen.queryByTestId("stub-history-list")).toBeNull(); + }); + + it("renders History as the second tab and switches content on selection", () => { + historyState = { status: "ready", bundles: [{ pending_id: "h1" }], total: 1, hasMore: false }; + render(); + + // Radix TabsTrigger switches tabs on mousedown, not click. + fireEvent.mouseDown(screen.getByTestId(HISTORY_TAB_TESTID)); + + expect(screen.getByTestId("stub-history-list").textContent).toBe("1"); + }); + + it("loads another History page from the tab list", () => { + historyState = { status: "ready", bundles: [{ pending_id: "h1" }], total: 2, hasMore: true }; + render(); + + fireEvent.mouseDown(screen.getByTestId(HISTORY_TAB_TESTID)); + fireEvent.click(screen.getByTestId("stub-history-load-more")); + + expect(mocks.inboxHistoryLoadMore).toHaveBeenCalledWith("w1"); + }); + + it("shows the History count badge in the Badge variant=secondary idiom, omitted at zero (AC .16)", () => { + historyState = { status: "ready", bundles: [], total: 0, hasMore: false }; + const { rerender } = render(); + expect(screen.getByTestId(HISTORY_TAB_TESTID).querySelector("[data-slot=badge]")).toBeNull(); + + historyState = { status: "ready", bundles: [], total: 3, hasMore: false }; + rerender(); + const badge = screen.getByTestId(HISTORY_TAB_TESTID).querySelector("[data-slot=badge]"); + expect(badge).not.toBeNull(); + expect(badge?.textContent).toBe("3"); + }); + + it("renders the History error state, not the empty state, when the history read failed (AC .25)", () => { + historyState = { status: "error", bundles: [], total: 0, hasMore: false }; + render(); + fireEvent.mouseDown(screen.getByTestId(HISTORY_TAB_TESTID)); + + expect(screen.getByTestId("inbox-history-error")).not.toBeNull(); + expect(screen.queryByTestId("inbox-history-empty")).toBeNull(); + }); + + it("renders the History empty state naming the bucket when there is nothing to show (AC .24)", () => { + historyState = { status: "ready", bundles: [], total: 0, hasMore: false }; + render(); + fireEvent.mouseDown(screen.getByTestId(HISTORY_TAB_TESTID)); + + expect(screen.getByTestId("inbox-history-empty")).not.toBeNull(); + }); +}); diff --git a/apps/web/app/needs-you-inbox/needs-you-inbox-page-client.tsx b/apps/web/app/needs-you-inbox/needs-you-inbox-page-client.tsx index d3341cbd14..79acf66b27 100644 --- a/apps/web/app/needs-you-inbox/needs-you-inbox-page-client.tsx +++ b/apps/web/app/needs-you-inbox/needs-you-inbox-page-client.tsx @@ -30,6 +30,19 @@ import { InboxTabStrip } from "@/components/needs-you-inbox/inbox-tab-strip"; import { FailedInboxTabPanel } from "@/components/needs-you-inbox/failed-inbox-tab-panel"; import { useFailedInboxController } from "@/hooks/domains/failed-inbox/use-failed-inbox-controller"; import { buildInboxTabHref, resolveInboxTab, type InboxTab } from "@/lib/failed-inbox/inbox-tab"; +import { useInboxHistoryController } from "@/hooks/domains/inbox-history/use-inbox-history-controller"; +import type { InboxHistoryController } from "@/hooks/domains/inbox-history/use-inbox-history-controller"; +import { + selectInboxHistoryBundles, + selectInboxHistoryCount, + selectInboxHistoryHasMore, + selectInboxHistoryIsLoadingMore, + selectInboxHistoryLoadMoreError, + selectInboxHistoryStatus, +} from "@/lib/state/slices/inbox-history/selectors"; +import { InboxHistoryList } from "@/components/inbox-history/inbox-history-list"; +import { InboxHistoryEmptyState } from "@/components/inbox-history/inbox-history-empty-state"; +import { InboxHistoryErrorState } from "@/components/inbox-history/inbox-history-error-state"; type ViewMode = "error" | "loading" | "empty" | "list"; @@ -89,6 +102,8 @@ function NeedsYouInboxList({ ); } +// "Needs you" is the tab strip's default-selected tab, and this content, +// count and behavior are unchanged by the strip's presence. function NeedsYouInboxTabContent({ retry }: { retry: () => void }) { const { t } = useTranslation(); const status = useAppStore(selectNeedsYouInboxStatus); @@ -137,6 +152,60 @@ function NeedsYouInboxTabContent({ retry }: { retry: () => void }) { ); } +// A truncated-to-zero page (bundleCount === 0 with hasMore) still resolves +// to the retryable error state, matching the Needs-you sibling's own +// handling of the same shape. +function resolveHistoryViewMode(status: string, bundleCount: number, hasMore: boolean): ViewMode { + if (status === "error" || (bundleCount === 0 && hasMore)) return "error"; + if (status === "loading" && bundleCount === 0) return "loading"; + if (bundleCount === 0) return "empty"; + return "list"; +} + +// The History tab's own read-only content, driven by its own isolated slice -- +// never the Needs-you slice or its refresh trigger. The controller stays in +// the parent so its lifecycle is mounted exactly once per page, not once per +// tab activation. +function InboxHistoryTabContent({ controller }: { controller: InboxHistoryController }) { + const { t } = useTranslation(); + const status = useAppStore(selectInboxHistoryStatus); + const bundles = useAppStore(selectInboxHistoryBundles); + const hasMore = useAppStore(selectInboxHistoryHasMore); + const isLoadingMore = useAppStore(selectInboxHistoryIsLoadingMore); + const loadMoreError = useAppStore(selectInboxHistoryLoadMoreError); + const workspaceId = useAppStore((s) => s.workspaces.activeId); + const { refresh, loadMore } = controller; + const retry = useCallback(() => { + if (workspaceId) void refresh(workspaceId); + }, [refresh, workspaceId]); + const loadNextPage = useCallback(() => { + if (workspaceId) void loadMore(workspaceId); + }, [loadMore, workspaceId]); + + const viewMode = resolveHistoryViewMode(status, bundles.length, hasMore); + + return ( + <> + {viewMode === "error" && } + {viewMode === "loading" && ( +

+ {t("common:loading")} +

+ )} + {viewMode === "empty" && } + {viewMode === "list" && ( + + )} + + ); +} + // v1 renders no tab strip and no in-page title (design-01#Components). This // capability adds the tab strip; the page title still belongs to the app top // bar, which is PageShell's, so this route mounts the same chrome every other @@ -150,6 +219,9 @@ export function NeedsYouInboxPageClient() { const failedCount = useAppStore(selectFailedInboxCount); const failedCountKnown = useAppStore(selectFailedInboxCountIsKnown); const failedTruncated = useAppStore(selectFailedInboxTruncated); + // The History tab's own bundle count, populated the moment the Inbox page + // opens, without requiring a click into the tab. + const historyCount = useAppStore(selectInboxHistoryCount); const pathname = usePathname(); const searchParams = useSearchParams(); @@ -160,6 +232,9 @@ export function NeedsYouInboxPageClient() { // bucket's own refresh triggers, independent of the Needs-you controller // mounted at the app shell. useFailedInboxController(selectedTab); + // The History controller is mounted once at the page level (not gated by + // which tab is active), never from the Needs-you slice or its refresh tick. + const historyController = useInboxHistoryController(); const retry = useCallback(() => bumpRefreshTick(), [bumpRefreshTick]); const selectTab = useCallback( @@ -176,6 +251,7 @@ export function NeedsYouInboxPageClient() { needsYouHasMore={needsYouHasMore} failedCount={failedCountKnown ? failedCount : undefined} failedTruncated={failedTruncated} + historyCount={historyCount} > @@ -183,6 +259,9 @@ export function NeedsYouInboxPageClient() { + + + ); diff --git a/apps/web/components/app-sidebar/app-sidebar-primary-nav.test.tsx b/apps/web/components/app-sidebar/app-sidebar-primary-nav.test.tsx index 2d589d4044..c1ebfaaceb 100644 --- a/apps/web/components/app-sidebar/app-sidebar-primary-nav.test.tsx +++ b/apps/web/components/app-sidebar/app-sidebar-primary-nav.test.tsx @@ -14,6 +14,7 @@ const state = { features: { ...defaultFeatureFlags } as FeatureFlags, office: { inboxCountByWorkspaceId: {} as Record }, needsYouInbox: { byWorkspaceId: {} as Record }, + inboxHistory: { byWorkspaceId: {} as Record }, quickChat: { isOpen: false, sessions: [] as Array<{ @@ -72,6 +73,7 @@ describe("AppSidebarPrimaryNav", () => { state.userSettings = { ...defaultState.userSettings }; state.features = { ...defaultFeatureFlags }; state.needsYouInbox = { byWorkspaceId: {} }; + state.inboxHistory = { byWorkspaceId: {} }; state.office.inboxCountByWorkspaceId = {}; state.quickChat.isOpen = false; state.quickChat.sessions = []; @@ -174,6 +176,7 @@ describe("AppSidebarPrimaryNav — Needs-you Inbox nav entry", () => { state.userSettings = { ...defaultState.userSettings }; state.features = { ...defaultFeatureFlags }; state.needsYouInbox = { byWorkspaceId: {} }; + state.inboxHistory = { byWorkspaceId: {} }; state.office.inboxCountByWorkspaceId = {}; mode = "kanban"; pathname = "/"; @@ -227,4 +230,18 @@ describe("AppSidebarPrimaryNav — Needs-you Inbox nav entry", () => { const link = screen.getByRole("link", { name: "Inbox" }); expect(link.textContent).toContain("50+"); }); + + // @covers AC-UI-INBOX-HISTORY-001.17 + it("leaves the Needs-you badge unset when History is populated and Needs-you is empty", () => { + state.features.needsYouInbox = true; + state.needsYouInbox.byWorkspaceId["ws-1"] = { count: 0 }; + state.inboxHistory.byWorkspaceId["ws-1"] = { total: 12 }; + renderNav(false); + + // A populated History total renders no badge at all (accessible name + // stays exactly "Inbox", with no count suffix) rather than a "0" badge or + // History's own total leaking into the Needs-you count. + const link = screen.getByRole("link", { name: "Inbox" }); + expect(link.textContent).not.toContain("12"); + }); }); diff --git a/apps/web/components/inbox-history/inbox-history-empty-state.tsx b/apps/web/components/inbox-history/inbox-history-empty-state.tsx new file mode 100644 index 0000000000..1fe88e5d8d --- /dev/null +++ b/apps/web/components/inbox-history/inbox-history-empty-state.tsx @@ -0,0 +1,25 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import { useAppStore } from "@/components/state-provider"; + +// Names what the bucket contains rather than a bare success message. +export function InboxHistoryEmptyState() { + const { t } = useTranslation(); + const workspaceName = useAppStore( + (s) => s.workspaces?.items?.find((w) => w.id === s.workspaces.activeId)?.name, + ); + return ( +
+

+ {workspaceName + ? t("inboxHistory:emptyTitle", { workspace: workspaceName }) + : t("inboxHistory:emptyTitleUnnamedWorkspace")} +

+

{t("inboxHistory:emptyDescription")}

+
+ ); +} diff --git a/apps/web/components/inbox-history/inbox-history-error-state.tsx b/apps/web/components/inbox-history/inbox-history-error-state.tsx new file mode 100644 index 0000000000..1094164f35 --- /dev/null +++ b/apps/web/components/inbox-history/inbox-history-error-state.tsx @@ -0,0 +1,31 @@ +"use client"; + +import { IconAlertTriangle } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { useTranslation } from "react-i18next"; + +// An error, including a workspace the caller cannot see, renders this state +// rather than the empty state, and the History badge shows no count. +export function InboxHistoryErrorState({ onRetry }: { onRetry: () => void }) { + const { t } = useTranslation(); + return ( +
+ +

{t("inboxHistory:errorTitle")}

+

{t("inboxHistory:errorDescription")}

+ +
+ ); +} diff --git a/apps/web/components/inbox-history/inbox-history-list.test.tsx b/apps/web/components/inbox-history/inbox-history-list.test.tsx new file mode 100644 index 0000000000..2eb6062cc9 --- /dev/null +++ b/apps/web/components/inbox-history/inbox-history-list.test.tsx @@ -0,0 +1,63 @@ +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; + +vi.mock("./inbox-history-row", () => ({ + InboxHistoryRow: () => null, +})); + +import { InboxHistoryList } from "./inbox-history-list"; + +function bundle(): InboxHistoryBundle { + return { + pending_id: "p1", + task_id: "t1", + session_id: "s1", + session_state: "COMPLETED", + task_title: "Task", + created_at: "2026-09-03T04:54:02Z", + context: "", + messages: [], + kind: "clarification", + reason: "session_ended", + asking_turn_id: "turn-1", + }; +} + +afterEach(() => cleanup()); + +describe("InboxHistoryList pagination", () => { + it("offers a load-more action for a truncated page", () => { + const onLoadMore = vi.fn(); + render( + , + ); + + fireEvent.click(screen.getByTestId("inbox-history-load-more")); + + expect(onLoadMore).toHaveBeenCalledOnce(); + }); + + it("keeps the action disabled and reports a page error while loading more", () => { + render( + , + ); + + expect((screen.getByTestId("inbox-history-load-more") as HTMLButtonElement).disabled).toBe( + true, + ); + expect(screen.getByTestId("inbox-history-load-more-error").getAttribute("role")).toBe("alert"); + }); +}); diff --git a/apps/web/components/inbox-history/inbox-history-list.tsx b/apps/web/components/inbox-history/inbox-history-list.tsx new file mode 100644 index 0000000000..988bcde189 --- /dev/null +++ b/apps/web/components/inbox-history/inbox-history-list.tsx @@ -0,0 +1,59 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import { Button } from "@kandev/ui/button"; +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; +import { InboxHistoryRow } from "./inbox-history-row"; + +export function InboxHistoryList({ + bundles, + hasMore, + isLoadingMore, + loadMoreError, + onLoadMore, +}: { + bundles: readonly InboxHistoryBundle[]; + hasMore: boolean; + isLoadingMore: boolean; + loadMoreError: boolean; + onLoadMore: () => void; +}) { + const { t } = useTranslation(); + return ( + <> +
+ {bundles.map((bundle) => ( + + ))} +
+ {hasMore && ( +
+

+ {t("inboxHistory:truncatedNotice")} +

+ + {loadMoreError && ( +

+ {t("inboxHistory:loadMoreError")} +

+ )} +
+ )} + + ); +} diff --git a/apps/web/components/inbox-history/inbox-history-row.mobile.test.tsx b/apps/web/components/inbox-history/inbox-history-row.mobile.test.tsx new file mode 100644 index 0000000000..096f0e603e --- /dev/null +++ b/apps/web/components/inbox-history/inbox-history-row.mobile.test.tsx @@ -0,0 +1,115 @@ +// Mobile-viewport coverage for AC-UI-INBOX-HISTORY-001.14/.29: on a phone or +// coarse-pointer viewport, both AC .14 actions (open task, copy id) must stay +// reachable with a 44px touch target, and the row must not clip. +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { sessionId as toSessionId, taskId as toTaskId } from "@/lib/types/http"; +import type { Message } from "@/lib/types/http"; +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; + +const mocks = vi.hoisted(() => ({ + copyToClipboard: vi.fn(), + toast: vi.fn(), + toastError: vi.fn(), +})); + +vi.mock("@/lib/utils/copy-to-clipboard", () => ({ + copyToClipboard: (...args: unknown[]) => mocks.copyToClipboard(...args), +})); + +vi.mock("@/lib/toast/sonner", () => ({ + toast: Object.assign((...args: unknown[]) => mocks.toast(...args), { + error: (...args: unknown[]) => mocks.toastError(...args), + }), +})); + +vi.mock("@/hooks/use-responsive-breakpoint", () => ({ + useResponsiveBreakpoint: () => ({ isMobile: true }), +})); + +import { InboxHistoryRow } from "./inbox-history-row"; + +const TASK_ID = "t1"; +const SESSION_ID = "s1"; + +function message(overrides: Partial = {}): Message { + return { + id: "m1", + session_id: toSessionId(SESSION_ID), + task_id: toTaskId(TASK_ID), + author_type: "agent", + content: "", + type: "clarification_request", + created_at: new Date().toISOString(), + ...overrides, + }; +} + +function bundle(overrides: Partial = {}): InboxHistoryBundle { + return { + pending_id: "p1", + task_id: TASK_ID, + session_id: SESSION_ID, + session_state: "COMPLETED", + task_title: "Fix the deploy", + created_at: new Date().toISOString(), + context: "shared context", + messages: [ + message({ + metadata: { + question: { + id: "q1", + title: "Pick a branch", + prompt: "Which branch should I use?", + options: [{ option_id: "o1", label: "main", description: "" }], + }, + }, + }), + ], + kind: "clarification", + reason: "session_ended", + asking_turn_id: "turn-1", + ...overrides, + }; +} + +beforeEach(() => { + mocks.copyToClipboard.mockReset().mockResolvedValue(true); + mocks.toast.mockReset(); + mocks.toastError.mockReset(); +}); + +afterEach(() => cleanup()); + +describe("InboxHistoryRow — mobile actions (AC .14, AC .29)", () => { + it("keeps the open-task action reachable, not display:none, on a phone viewport", () => { + render(); + const openTask = screen.getByTestId("inbox-history-open-task"); + expect(openTask.className).not.toMatch(/\bhidden\b/); + expect(openTask.getAttribute("href")).toBe("/t/t-9?sessionId=s-9"); + }); + + it("labels the icon-only open-task and copy-id actions for assistive tech", () => { + render(); + expect(screen.getByTestId("inbox-history-open-task").getAttribute("aria-label")).toBe( + "Open task", + ); + expect(screen.getByTestId("inbox-history-copy-id").getAttribute("aria-label")).toBe( + "Copy identifier", + ); + }); + + it("sizes the copy-id action to the 44px touch-target minimum", () => { + render(); + const copyId = screen.getByTestId("inbox-history-copy-id"); + expect(copyId.className).toContain("min-h-11"); + expect(copyId.className).toContain("min-w-11"); + }); + + it("still copies the bundle's pending_id from the icon-only action", async () => { + render(); + fireEvent.click(screen.getByTestId("inbox-history-copy-id")); + await Promise.resolve(); + expect(mocks.copyToClipboard).toHaveBeenCalledWith("bundle-42"); + }); +}); diff --git a/apps/web/components/inbox-history/inbox-history-row.tablet.test.tsx b/apps/web/components/inbox-history/inbox-history-row.tablet.test.tsx new file mode 100644 index 0000000000..28a51999ff --- /dev/null +++ b/apps/web/components/inbox-history/inbox-history-row.tablet.test.tsx @@ -0,0 +1,105 @@ +// Coarse-pointer tablet coverage: apps/web/AGENTS.md ("Responsive and touch +// surfaces") defines tablet as a coarse-pointer fallback between md and lg, +// so a tablet viewport (isMobile false, isFinePointer false) must get the +// same compact/touch-target action treatment as a phone. +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { sessionId as toSessionId, taskId as toTaskId } from "@/lib/types/http"; +import type { Message } from "@/lib/types/http"; +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; + +const mocks = vi.hoisted(() => ({ + copyToClipboard: vi.fn(), + toast: vi.fn(), + toastError: vi.fn(), +})); + +vi.mock("@/lib/utils/copy-to-clipboard", () => ({ + copyToClipboard: (...args: unknown[]) => mocks.copyToClipboard(...args), +})); + +vi.mock("@/lib/toast/sonner", () => ({ + toast: Object.assign((...args: unknown[]) => mocks.toast(...args), { + error: (...args: unknown[]) => mocks.toastError(...args), + }), +})); + +vi.mock("@/hooks/use-responsive-breakpoint", () => ({ + useResponsiveBreakpoint: () => ({ isMobile: false, isFinePointer: false }), +})); + +import { InboxHistoryRow } from "./inbox-history-row"; + +const TASK_ID = "t1"; +const SESSION_ID = "s1"; + +function message(overrides: Partial = {}): Message { + return { + id: "m1", + session_id: toSessionId(SESSION_ID), + task_id: toTaskId(TASK_ID), + author_type: "agent", + content: "", + type: "clarification_request", + created_at: new Date().toISOString(), + ...overrides, + }; +} + +function bundle(overrides: Partial = {}): InboxHistoryBundle { + return { + pending_id: "p1", + task_id: TASK_ID, + session_id: SESSION_ID, + session_state: "COMPLETED", + task_title: "Fix the deploy", + created_at: new Date().toISOString(), + context: "shared context", + messages: [ + message({ + metadata: { + question: { + id: "q1", + title: "Pick a branch", + prompt: "Which branch should I use?", + options: [{ option_id: "o1", label: "main", description: "" }], + }, + }, + }), + ], + kind: "clarification", + reason: "session_ended", + asking_turn_id: "turn-1", + ...overrides, + }; +} + +beforeEach(() => { + mocks.copyToClipboard.mockReset().mockResolvedValue(true); + mocks.toast.mockReset(); + mocks.toastError.mockReset(); +}); + +afterEach(() => cleanup()); + +describe("InboxHistoryRow — coarse-pointer tablet actions", () => { + it("renders icon-only actions with an aria-label, not the desktop text-labelled pair", () => { + render(); + expect(screen.getByTestId("inbox-history-open-task").getAttribute("aria-label")).toBe( + "Open task", + ); + expect(screen.getByTestId("inbox-history-copy-id").getAttribute("aria-label")).toBe( + "Copy identifier", + ); + }); + + it("sizes both actions to the 44px touch-target minimum", () => { + render(); + const openTask = screen.getByTestId("inbox-history-open-task"); + const copyId = screen.getByTestId("inbox-history-copy-id"); + expect(openTask.className).toContain("min-h-11"); + expect(openTask.className).toContain("min-w-11"); + expect(copyId.className).toContain("min-h-11"); + expect(copyId.className).toContain("min-w-11"); + }); +}); diff --git a/apps/web/components/inbox-history/inbox-history-row.test.tsx b/apps/web/components/inbox-history/inbox-history-row.test.tsx new file mode 100644 index 0000000000..8638915972 --- /dev/null +++ b/apps/web/components/inbox-history/inbox-history-row.test.tsx @@ -0,0 +1,202 @@ +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { sessionId as toSessionId, taskId as toTaskId } from "@/lib/types/http"; +import type { Message } from "@/lib/types/http"; +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; + +const mocks = vi.hoisted(() => ({ + copyToClipboard: vi.fn(), + toast: vi.fn(), + toastError: vi.fn(), +})); + +vi.mock("@/lib/utils/copy-to-clipboard", () => ({ + copyToClipboard: (...args: unknown[]) => mocks.copyToClipboard(...args), +})); + +vi.mock("@/lib/toast/sonner", () => ({ + toast: Object.assign((...args: unknown[]) => mocks.toast(...args), { + error: (...args: unknown[]) => mocks.toastError(...args), + }), +})); + +import { InboxHistoryRow } from "./inbox-history-row"; + +const TASK_ID = "t1"; +const SESSION_ID = "s1"; +const TOGGLE_TESTID = "inbox-history-row-toggle"; + +function message(overrides: Partial = {}): Message { + return { + id: "m1", + session_id: toSessionId(SESSION_ID), + task_id: toTaskId(TASK_ID), + author_type: "agent", + content: "", + type: "clarification_request", + created_at: new Date().toISOString(), + ...overrides, + }; +} + +function bundle(overrides: Partial = {}): InboxHistoryBundle { + return { + pending_id: "p1", + task_id: TASK_ID, + session_id: SESSION_ID, + session_state: "COMPLETED", + task_title: "Fix the deploy", + created_at: new Date().toISOString(), + context: "shared context", + messages: [ + message({ + metadata: { + question: { + id: "q1", + title: "Pick a branch", + prompt: "Which branch should I use?", + options: [{ option_id: "o1", label: "main", description: "" }], + }, + }, + }), + ], + kind: "clarification", + reason: "session_ended", + asking_turn_id: "turn-1", + ...overrides, + }; +} + +function permissionBundle(overrides: Partial = {}): InboxHistoryBundle { + return bundle({ + kind: "permission", + messages: [ + message({ + content: "Allow deleting tmp/?", + type: "permission_request", + metadata: { options: [{ kind: "allow", name: "Allow", option_id: "o1" }] }, + }), + ], + ...overrides, + }); +} + +function expandRow() { + fireEvent.click(screen.getByTestId(TOGGLE_TESTID)); +} + +beforeEach(() => { + mocks.copyToClipboard.mockReset().mockResolvedValue(true); + mocks.toast.mockReset(); + mocks.toastError.mockReset(); +}); + +afterEach(() => cleanup()); + +describe("InboxHistoryRow — kind and content", () => { + it("renders a clarification icon, not the permission icon, for a clarification bundle", () => { + render(); + expect(screen.queryByTestId("inbox-history-permission-icon")).toBeNull(); + }); + + it("labels a permission bundle with the shipped IconShieldQuestion/text-amber-500 pairing (AC .12)", () => { + render(); + const icon = screen.getByTestId("inbox-history-permission-icon"); + expect(icon).not.toBeNull(); + expect(icon.querySelector("svg")?.getAttribute("class")).toContain("text-amber-500"); + }); + + it("shows the reason badge, not a raw pending status (AC .9)", () => { + render(); + expect(screen.getByTestId("inbox-history-row-reason").textContent).toMatch( + /could not be read/i, + ); + }); + + it("reveals every question's title, prompt and option labels on expand (AC .30, AC .13)", () => { + render(); + expandRow(); + + const detail = screen.getByTestId("inbox-history-clarification-detail"); + expect(detail.textContent).toContain("Pick a branch"); + expect(detail.textContent).toContain("Which branch should I use?"); + expect(detail.textContent).toContain("main"); + expect(detail.textContent).toContain("shared context"); + }); + + it("renders permission content and options[].name, not a question object (AC .30)", () => { + render(); + expandRow(); + + const detail = screen.getByTestId("inbox-history-permission-detail"); + expect(detail.textContent).toContain("Allow deleting tmp/?"); + expect(detail.textContent).toContain("Allow"); + }); +}); + +describe("InboxHistoryRow — turn identity and step label", () => { + it("shows the step-starts-no-agent label only when the field is true (AC .11)", () => { + render(); + expandRow(); + expect(screen.getByTestId("inbox-history-step-starts-no-agent")).not.toBeNull(); + }); + + it("omits the step-starts-no-agent label when the field is absent (unknown, not guessed)", () => { + render(); + expandRow(); + expect(screen.queryByTestId("inbox-history-step-starts-no-agent")).toBeNull(); + }); + + it("identifies the superseding turn only when the reason is superseded (AC .10)", () => { + render( + , + ); + expandRow(); + const turnIdentity = screen.getByTestId("inbox-history-turn-identity"); + expect(turnIdentity.textContent).toContain("t1"); + expect(turnIdentity.textContent).toContain("t2"); + }); + + it("omits the superseding turn for a same-turn permission supersession (AC .10)", () => { + render( + , + ); + expandRow(); + const turnIdentity = screen.getByTestId("inbox-history-turn-identity"); + expect(turnIdentity.textContent).toContain("t1"); + expect(turnIdentity.textContent).not.toContain("undefined"); + }); +}); + +describe("InboxHistoryRow — actions", () => { + it("exposes only open-task and copy-id actions, no answer/dismiss/snooze affordance (AC .14)", () => { + render(); + expect(screen.getByTestId("inbox-history-open-task")).not.toBeNull(); + expect(screen.getByTestId("inbox-history-copy-id")).not.toBeNull(); + expect(screen.queryByRole("button", { name: /dismiss/i })).toBeNull(); + expect(screen.queryByRole("button", { name: /snooze/i })).toBeNull(); + expect(screen.queryByRole("button", { name: /^answer/i })).toBeNull(); + }); + + it("copies the bundle's pending_id when the copy action is used", async () => { + render(); + fireEvent.click(screen.getByTestId("inbox-history-copy-id")); + await Promise.resolve(); + expect(mocks.copyToClipboard).toHaveBeenCalledWith("bundle-42"); + }); + + it("links to the owning task with the session id", () => { + render(); + expect(screen.getByTestId("inbox-history-open-task").getAttribute("href")).toBe( + "/t/t-9?sessionId=s-9", + ); + }); +}); diff --git a/apps/web/components/inbox-history/inbox-history-row.tsx b/apps/web/components/inbox-history/inbox-history-row.tsx new file mode 100644 index 0000000000..339454b7c7 --- /dev/null +++ b/apps/web/components/inbox-history/inbox-history-row.tsx @@ -0,0 +1,262 @@ +"use client"; + +import { useState } from "react"; +import { + IconChevronDown, + IconChevronRight, + IconCopy, + IconExternalLink, + IconMessageQuestion, + IconShieldQuestion, +} from "@tabler/icons-react"; +import { Badge } from "@kandev/ui/badge"; +import { Button } from "@kandev/ui/button"; +import { useTranslation } from "react-i18next"; +import Link from "@/components/routing/app-link"; +import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; +import { toast } from "@/lib/toast/sonner"; +import { formatRelativeTime } from "@/lib/i18n/formats"; +import { copyToClipboard } from "@/lib/utils/copy-to-clipboard"; +import { + inboxHistoryClarificationQuestions, + inboxHistoryPermissionContent, + inboxHistoryReasonLabelKey, + inboxHistorySecondaryText, +} from "@/lib/inbox-history/row-presentation"; +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; + +// Standalone touch icons need both dimensions held to the 44px minimum, not +// just min-height (control-sizing.md, "Standalone touch icons need both +// height and width checks"). +const TOUCH_ICON_BUTTON = + "max-md:min-h-11 max-md:min-w-11 [@media(pointer:coarse)]:min-h-11 [@media(pointer:coarse)]:min-w-11"; + +function taskHrefForBundle(bundle: InboxHistoryBundle): string { + if (!bundle.session_id) return `/t/${bundle.task_id}`; + return `/t/${bundle.task_id}?sessionId=${encodeURIComponent(bundle.session_id)}`; +} + +function InboxHistoryQuestionDetail({ bundle }: { bundle: InboxHistoryBundle }) { + const { t } = useTranslation(); + if (bundle.kind === "permission") { + const { content, optionNames } = inboxHistoryPermissionContent(bundle); + return ( +
+

{content || t("inboxHistory:noContent")}

+ {optionNames.length > 0 && ( +

{optionNames.join(", ")}

+ )} +
+ ); + } + const questions = inboxHistoryClarificationQuestions(bundle); + return ( +
+ {bundle.context &&

{bundle.context}

} + {questions.map((question) => ( +
+

+ {question.title || question.prompt || t("inboxHistory:noContent")} +

+ {question.title && question.prompt && ( +

{question.prompt}

+ )} + {question.optionLabels.length > 0 && ( +

{question.optionLabels.join(", ")}

+ )} +
+ ))} +
+ ); +} + +function InboxHistoryRowIcon({ bundle }: { bundle: InboxHistoryBundle }) { + const { t } = useTranslation(); + if (bundle.kind === "permission") { + return ( + + + + ); + } + return ( + + + + ); +} + +function inboxHistoryPrimaryText(bundle: InboxHistoryBundle, fallback: string): string { + if (bundle.kind === "permission") { + return inboxHistoryPermissionContent(bundle).content || fallback; + } + const questions = inboxHistoryClarificationQuestions(bundle); + return questions[0]?.title || questions[0]?.prompt || bundle.context || fallback; +} + +// The tab's two actions. Icon-only with an aria-label on a phone or +// coarse-pointer viewport, where the labelled desktop pair does not fit +// alongside the reason badge and asked time without clipping; text-labelled +// otherwise. Rendering one pair rather than both keeps a hidden duplicate +// out of the accessibility tree at every breakpoint. +function InboxHistoryRowActions({ + taskHref, + onCopyId, + compact, +}: { + taskHref: string; + onCopyId: () => void; + compact: boolean; +}) { + const { t } = useTranslation(); + if (compact) { + return ( + <> + + + + ); + } + return ( + <> + + + + ); +} + +export function InboxHistoryRow({ bundle }: { bundle: InboxHistoryBundle }) { + const { t } = useTranslation(); + const [expanded, setExpanded] = useState(false); + const { isMobile, isFinePointer } = useResponsiveBreakpoint(); + // Tablet (md-lg) is a coarse-pointer breakpoint (apps/web/AGENTS.md, + // "Responsive and touch surfaces"): its actions need the same touch + // treatment as phone, not the fine-pointer desktop pair. + const compactActions = isMobile || !isFinePointer; + + const primaryText = inboxHistoryPrimaryText(bundle, t("inboxHistory:noContent")); + const secondaryText = inboxHistorySecondaryText(bundle); + const taskHref = taskHrefForBundle(bundle); + const relativeTime = formatRelativeTime(bundle.created_at); + + const handleCopyId = () => { + void copyToClipboard(bundle.pending_id).then((ok) => { + if (ok) toast(t("inboxHistory:copyIdSuccess")); + else toast.error(t("inboxHistory:copyIdFailed")); + }); + }; + + return ( +
+
+ + {/* Second line on a narrow viewport: the icon/text row above already + uses the full width, so the reason, asked time and the two + actions would clip if forced onto one row with it. */} +
+
+ + {t(inboxHistoryReasonLabelKey(bundle.reason))} + + + {relativeTime} + +
+
+ +
+
+
+ {expanded && ( +
+

+ {bundle.reason === "superseded" && bundle.superseding_turn_id + ? t("inboxHistory:turnSupersededBy", { + askingTurn: bundle.asking_turn_id, + supersedingTurn: bundle.superseding_turn_id, + }) + : t("inboxHistory:turnAsked", { turn: bundle.asking_turn_id })} +

+ {bundle.step_starts_no_agent === true && ( +

+ {t("inboxHistory:stepStartsNoAgent")} +

+ )} + +
+ )} +
+ ); +} diff --git a/apps/web/components/needs-you-inbox/inbox-tab-strip.test.tsx b/apps/web/components/needs-you-inbox/inbox-tab-strip.test.tsx index eb3f447699..3a50602438 100644 --- a/apps/web/components/needs-you-inbox/inbox-tab-strip.test.tsx +++ b/apps/web/components/needs-you-inbox/inbox-tab-strip.test.tsx @@ -6,6 +6,7 @@ import { InboxTabStrip } from "./inbox-tab-strip"; import type { InboxTab } from "@/lib/failed-inbox/inbox-tab"; const FAILED_BADGE_TESTID = "inbox-tab-failed-badge"; +const HISTORY_BADGE_TESTID = "inbox-tab-history-badge"; function renderStrip( overrides: Partial<{ @@ -15,6 +16,7 @@ function renderStrip( needsYouHasMore: boolean; failedCount: number | undefined; failedTruncated: boolean; + historyCount: number; children?: ReactNode; }> = {}, ) { @@ -26,6 +28,7 @@ function renderStrip( needsYouHasMore={false} failedCount={undefined} failedTruncated={false} + historyCount={0} {...overrides} />, ); @@ -34,12 +37,13 @@ function renderStrip( afterEach(() => cleanup()); describe("InboxTabStrip", () => { - it("renders exactly two tabs, Needs you then Failed (AC .1)", () => { + it("renders exactly three tabs, Needs you, Failed, then History (AC .1)", () => { renderStrip(); const tabs = screen.getAllByRole("tab"); - expect(tabs).toHaveLength(2); + expect(tabs).toHaveLength(3); expect(tabs[0].textContent).toContain("Needs you"); expect(tabs[1].textContent).toContain("Failed"); + expect(tabs[2].textContent).toContain("History"); }); it("calls onSelectTab with the clicked tab's value", () => { @@ -75,6 +79,16 @@ describe("InboxTabStrip", () => { expect(screen.getByTestId(FAILED_BADGE_TESTID)).not.toBeNull(); }); + it("renders no history badge when the count is zero (AC .16)", () => { + renderStrip({ historyCount: 0 }); + expect(screen.queryByTestId(HISTORY_BADGE_TESTID)).toBeNull(); + }); + + it("renders the history badge once a non-zero count is known", () => { + renderStrip({ historyCount: 3 }); + expect(screen.getByTestId(HISTORY_BADGE_TESTID).textContent).toBe("3"); + }); + it("carries a coarse-pointer/mobile 44px touch-target floor on every tab trigger", () => { renderStrip(); for (const tab of screen.getAllByRole("tab")) { diff --git a/apps/web/components/needs-you-inbox/inbox-tab-strip.tsx b/apps/web/components/needs-you-inbox/inbox-tab-strip.tsx index e42ef46a35..ed50c4b066 100644 --- a/apps/web/components/needs-you-inbox/inbox-tab-strip.tsx +++ b/apps/web/components/needs-you-inbox/inbox-tab-strip.tsx @@ -21,6 +21,7 @@ export function InboxTabStrip({ needsYouHasMore, failedCount, failedTruncated, + historyCount, children, }: { selectedTab: InboxTab; @@ -29,11 +30,13 @@ export function InboxTabStrip({ needsYouHasMore: boolean; failedCount: number | undefined; failedTruncated: boolean; + historyCount: number; children?: ReactNode; }) { const { t } = useTranslation(); const needsYouBadge = badgeText(needsYouCount, needsYouHasMore); const failedBadge = badgeText(failedCount, failedTruncated); + const historyBadge = badgeText(historyCount, false); return ( onSelectTab(value as InboxTab)}> @@ -57,6 +60,18 @@ export function InboxTabStrip({ )} + + {t("inboxHistory:tabLabel")} + {historyBadge && ( + + {historyBadge} + + )} + {children} diff --git a/apps/web/e2e/tests/chat/inbox-history-superseded.spec.ts b/apps/web/e2e/tests/chat/inbox-history-superseded.spec.ts new file mode 100644 index 0000000000..cc057cff94 --- /dev/null +++ b/apps/web/e2e/tests/chat/inbox-history-superseded.spec.ts @@ -0,0 +1,96 @@ +// AC-UI-INBOX-HISTORY-001.29: end-to-end coverage for the History tab -- +// selecting it, observing a `superseded` bundle with its asked time, the +// text of the question it actually asked (AC .30), and its +// step-starts-no-agent label (AC .11, named by that concept, never +// "parked") -- and confirming no answer affordance is present (AC .14). +import { test, expect } from "../../fixtures/test-base"; + +const QUESTION_TITLE = "Pick a deployment target"; +const QUESTION_PROMPT = "Which environment should this ship to?"; +const OPTION_LABEL = "Staging"; + +test.describe("Inbox History tab", () => { + test("lists a superseded question with its asked time, question text and step label, with no answer affordance (AC .29)", async ({ + testPage, + apiClient, + seedData, + }) => { + // A step whose on_enter carries no auto_start_agent entry, so the row + // must render the AC .11 step-starts-no-agent label. + const noAgentStep = await apiClient.createWorkflowStep( + seedData.workflowId, + "Inbox History No-Agent Step", + 9, + { events: { on_enter: [] } }, + ); + + const title = "Inbox History Superseded Flow"; + const task = await apiClient.createTask(seedData.workspaceId, title, { + workflow_id: seedData.workflowId, + workflow_step_id: noAgentStep.id, + }); + const { session_id: sessionId } = await apiClient.seedTaskSession(task.id, { + state: "WAITING_FOR_INPUT", + }); + + const turnAStartedAt = new Date(Date.now() - 120_000).toISOString(); + const turnBStartedAt = new Date(Date.now() - 60_000).toISOString(); + + // AC .8's real sequence: ask a question on turn A, then start turn B + // without ever answering it. Turn A is completed (the agent's turn ends + // blocked on the question) so turn B, which starts later, wins current- + // turn authority outright -- exercising `superseded`, not a shadowed tie. + await apiClient.seedSessionMessage(sessionId, { + type: "clarification_request", + newTurn: true, + turnStartedAt: turnAStartedAt, + turnCompletedAt: turnAStartedAt, + metadata: { + pending_id: "pend-history-superseded-1", + session_id: sessionId, + question_id: "q1", + question_index: 0, + question_total: 1, + status: "pending", + question: { + id: "q1", + title: QUESTION_TITLE, + prompt: QUESTION_PROMPT, + options: [{ option_id: "opt-staging", label: OPTION_LABEL, description: "" }], + }, + }, + }); + await apiClient.seedSessionMessage(sessionId, { + type: "message", + content: "Starting a different approach.", + newTurn: true, + turnStartedAt: turnBStartedAt, + }); + + await testPage.goto("/needs-you-inbox"); + await testPage.getByTestId("inbox-tab-history").click(); + + const row = testPage.getByTestId("inbox-history-row").filter({ hasText: title }); + await expect(row).toBeVisible({ timeout: 30_000 }); + + await expect(row.getByTestId("inbox-history-row-reason")).toContainText(/superseded/i); + await expect(row.getByTestId("inbox-history-row-asked-time")).not.toBeEmpty(); + + await row.getByTestId("inbox-history-row-toggle").click(); + const detail = row.getByTestId("inbox-history-clarification-detail"); + await expect(detail).toBeVisible(); + await expect(detail).toContainText(QUESTION_TITLE); + await expect(detail).toContainText(QUESTION_PROMPT); + await expect(detail).toContainText(OPTION_LABEL); + + await expect(row.getByTestId("inbox-history-step-starts-no-agent")).toBeVisible(); + + // Read-only: opening the task and copying the identifier are the only + // actions, and none of them answers, dismisses or snoozes anything. + await expect(row.getByTestId("inbox-history-open-task")).toBeVisible(); + await expect(row.getByTestId("inbox-history-copy-id")).toBeVisible(); + await expect(row.getByRole("button", { name: /^answer/i })).toHaveCount(0); + await expect(row.getByRole("button", { name: /dismiss/i })).toHaveCount(0); + await expect(row.getByRole("button", { name: /snooze/i })).toHaveCount(0); + }); +}); diff --git a/apps/web/e2e/tests/chat/mobile-inbox-history-superseded.spec.ts b/apps/web/e2e/tests/chat/mobile-inbox-history-superseded.spec.ts new file mode 100644 index 0000000000..96d9241b69 --- /dev/null +++ b/apps/web/e2e/tests/chat/mobile-inbox-history-superseded.spec.ts @@ -0,0 +1,84 @@ +// AC-UI-INBOX-HISTORY-001.14/.29: the History tab's read-only actions must +// also work on a phone-sized viewport, where the row falls back to icon-only +// AC .14 actions with the 44px touch target (control-sizing.tsx's +// coarse-pointer breakpoint) instead of the desktop text-labelled pair. This +// mirrors inbox-history-superseded.spec.ts's desktop coverage rather than +// duplicating its assertions. +import { test, expect } from "../../fixtures/test-base"; + +const MIN_TOUCH_TARGET_PX = 44; +const QUESTION_TITLE = "Pick a deployment target"; +const QUESTION_PROMPT = "Which environment should this ship to?"; +const OPTION_LABEL = "Staging"; + +test.describe("Inbox History tab on mobile", () => { + test("open-task and copy-id stay reachable with a 44px touch target, with no clipping (AC .14, AC .29)", async ({ + testPage, + apiClient, + seedData, + }) => { + const title = "Mobile Inbox History Superseded Flow"; + const task = await apiClient.createTask(seedData.workspaceId, title, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + const { session_id: sessionId } = await apiClient.seedTaskSession(task.id, { + state: "WAITING_FOR_INPUT", + }); + + const turnAStartedAt = new Date(Date.now() - 120_000).toISOString(); + const turnBStartedAt = new Date(Date.now() - 60_000).toISOString(); + + await apiClient.seedSessionMessage(sessionId, { + type: "clarification_request", + newTurn: true, + turnStartedAt: turnAStartedAt, + turnCompletedAt: turnAStartedAt, + metadata: { + pending_id: "pend-mobile-history-superseded-1", + session_id: sessionId, + question_id: "q1", + question_index: 0, + question_total: 1, + status: "pending", + question: { + id: "q1", + title: QUESTION_TITLE, + prompt: QUESTION_PROMPT, + options: [{ option_id: "opt-staging", label: OPTION_LABEL, description: "" }], + }, + }, + }); + await apiClient.seedSessionMessage(sessionId, { + type: "message", + content: "Starting a different approach.", + newTurn: true, + turnStartedAt: turnBStartedAt, + }); + + await testPage.goto("/needs-you-inbox"); + await testPage.getByTestId("inbox-tab-history").click(); + + const row = testPage.getByTestId("inbox-history-row").filter({ hasText: title }); + await expect(row).toBeVisible({ timeout: 30_000 }); + + const openTask = row.getByTestId("inbox-history-open-task"); + const copyId = row.getByTestId("inbox-history-copy-id"); + await expect(openTask).toBeVisible(); + await expect(copyId).toBeVisible(); + + const copyIdBox = await copyId.boundingBox(); + expect(copyIdBox, "copy-id action should have a box").not.toBeNull(); + expect(copyIdBox!.height).toBeGreaterThanOrEqual(MIN_TOUCH_TARGET_PX); + expect(copyIdBox!.width).toBeGreaterThanOrEqual(MIN_TOUCH_TARGET_PX); + + // The document itself must not scroll horizontally on a phone viewport + // (the row's reason badge, asked time and both actions must compose + // without clipping). + const overflow = await testPage.evaluate(() => ({ + scrollWidth: document.documentElement.scrollWidth, + clientWidth: document.documentElement.clientWidth, + })); + expect(overflow.scrollWidth).toBeLessThanOrEqual(overflow.clientWidth); + }); +}); diff --git a/apps/web/hooks/domains/inbox-history/use-inbox-history-controller.test.tsx b/apps/web/hooks/domains/inbox-history/use-inbox-history-controller.test.tsx new file mode 100644 index 0000000000..a223fef0f1 --- /dev/null +++ b/apps/web/hooks/domains/inbox-history/use-inbox-history-controller.test.tsx @@ -0,0 +1,164 @@ +import { act, cleanup, fireEvent, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { StateProvider, useAppStoreApi } from "@/components/state-provider"; +import { defaultFeatureFlags } from "@/lib/state/slices/features/types"; +import type { HydrationState } from "@/lib/state/store"; + +const listInboxHistoryMock = vi.fn(); +vi.mock("@/lib/api/domains/inbox-history-api", () => ({ + listInboxHistory: (...args: unknown[]) => listInboxHistoryMock(...args), +})); + +import { useInboxHistoryController } from "./use-inbox-history-controller"; + +const WORKSPACE_ID = "w1"; + +function page(overrides: Partial>> = {}) { + return { bundles: [], count: 0, total: 0, ...overrides }; +} + +function initialState(enabled: boolean): HydrationState { + return { + features: { ...defaultFeatureFlags, needsYouInbox: enabled }, + workspaces: { activeId: WORKSPACE_ID, byId: {}, allIds: [] }, + } as unknown as HydrationState; +} + +function renderController(enabled = true) { + return renderHook( + () => { + const controller = useInboxHistoryController(); + return { controller, store: useAppStoreApi() }; + }, + { + wrapper: ({ children }) => ( + {children} + ), + }, + ); +} + +beforeEach(() => { + listInboxHistoryMock.mockReset(); + listInboxHistoryMock.mockResolvedValue(page()); +}); + +afterEach(() => { + cleanup(); +}); + +describe("useInboxHistoryController", () => { + it("reads on mount for the active workspace when the flag is enabled", async () => { + renderController(true); + + await waitFor(() => expect(listInboxHistoryMock).toHaveBeenCalledWith(WORKSPACE_ID)); + }); + + it("never reads when the flag is disabled", async () => { + renderController(false); + + await act(async () => { + await Promise.resolve(); + }); + expect(listInboxHistoryMock).not.toHaveBeenCalled(); + }); + + it("applies a successful page to the workspace's slice state", async () => { + listInboxHistoryMock.mockResolvedValue(page({ bundles: [], total: 3 })); + const { result } = renderController(true); + + await waitFor(() => { + const state = result.current.store.getState().inboxHistory.byWorkspaceId[WORKSPACE_ID]; + expect(state?.status).toBe("ready"); + expect(state?.total).toBe(3); + }); + }); + + it("applies an error state when the read rejects", async () => { + listInboxHistoryMock.mockRejectedValue(new Error("boom")); + const { result } = renderController(true); + + await waitFor(() => { + expect(result.current.store.getState().inboxHistory.byWorkspaceId[WORKSPACE_ID]?.status).toBe( + "error", + ); + }); + }); + + it("re-reads when the browser regains foreground", async () => { + renderController(true); + await waitFor(() => expect(listInboxHistoryMock).toHaveBeenCalledTimes(1)); + + listInboxHistoryMock.mockClear(); + Object.defineProperty(document, "visibilityState", { + configurable: true, + get: () => "visible", + }); + fireEvent(document, new Event("visibilitychange")); + + await waitFor(() => expect(listInboxHistoryMock).toHaveBeenCalledTimes(1)); + }); + + it("subscribes to no WebSocket client (AC .22: point-in-time projection only)", async () => { + const wsModule = await import("@/lib/ws/connection"); + const spy = vi.spyOn(wsModule, "getWebSocketClient"); + renderController(true); + + await waitFor(() => expect(listInboxHistoryMock).toHaveBeenCalled()); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + it("loads the next page with the cursor and appends it", async () => { + listInboxHistoryMock + .mockResolvedValueOnce( + page({ + bundles: [{ pending_id: "first" }], + total: 2, + next_cursor: "cursor-1", + }), + ) + .mockResolvedValueOnce(page({ bundles: [{ pending_id: "second" }], total: 2 })); + const { result } = renderController(true); + + await waitFor(() => { + expect( + result.current.store.getState().inboxHistory.byWorkspaceId[WORKSPACE_ID]?.hasMore, + ).toBe(true); + }); + + await act(async () => { + await result.current.controller.loadMore(WORKSPACE_ID); + }); + + expect(listInboxHistoryMock).toHaveBeenNthCalledWith(2, WORKSPACE_ID, { cursor: "cursor-1" }); + const state = result.current.store.getState().inboxHistory.byWorkspaceId[WORKSPACE_ID]; + expect(state?.bundles.map((item) => item.pending_id)).toEqual(["first", "second"]); + expect(state?.hasMore).toBe(false); + }); + + it("keeps the current rows and exposes a retryable load-more error", async () => { + listInboxHistoryMock + .mockResolvedValueOnce( + page({ bundles: [{ pending_id: "first" }], total: 2, next_cursor: "cursor-1" }), + ) + .mockRejectedValueOnce(new Error("boom")); + const { result } = renderController(true); + + await waitFor(() => { + expect( + result.current.store.getState().inboxHistory.byWorkspaceId[WORKSPACE_ID]?.hasMore, + ).toBe(true); + }); + + await act(async () => { + await result.current.controller.loadMore(WORKSPACE_ID); + }); + + const state = result.current.store.getState().inboxHistory.byWorkspaceId[WORKSPACE_ID]; + expect(state?.bundles).toHaveLength(1); + expect(state?.loadMoreError).toBe(true); + expect(state?.isLoadingMore).toBe(false); + expect(state?.hasMore).toBe(true); + }); +}); diff --git a/apps/web/hooks/domains/inbox-history/use-inbox-history-controller.ts b/apps/web/hooks/domains/inbox-history/use-inbox-history-controller.ts new file mode 100644 index 0000000000..d2a3e9b9e5 --- /dev/null +++ b/apps/web/hooks/domains/inbox-history/use-inbox-history-controller.ts @@ -0,0 +1,80 @@ +import { useCallback, useEffect } from "react"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; +import { useFeature } from "@/hooks/domains/features/use-feature"; +import { useForegroundRefresh } from "@/hooks/use-foreground-refresh"; +import { listInboxHistory } from "@/lib/api/domains/inbox-history-api"; + +export type InboxHistoryController = { + refresh: (targetWorkspaceId: string) => Promise; + loadMore: (targetWorkspaceId: string) => Promise; +}; + +/** + * Owns the History tab's single read: a point-in-time projection. Unlike the + * Needs-you controller, this hook subscribes to no WebSocket event and + * re-reads only on mount, workspace change, and the browser regaining + * foreground, or an explicit page request -- never on a server-pushed signal. + */ +export function useInboxHistoryController(): InboxHistoryController { + const enabled = useFeature("needsYouInbox"); + const workspaceId = useAppStore((s) => s.workspaces.activeId); + const storeApi = useAppStoreApi(); + + const refresh = useCallback( + async (targetWorkspaceId: string) => { + const { beginInboxHistoryRead, setInboxHistoryPage, setInboxHistoryError } = + storeApi.getState(); + const generation = beginInboxHistoryRead(targetWorkspaceId); + try { + const page = await listInboxHistory(targetWorkspaceId); + setInboxHistoryPage(targetWorkspaceId, generation, { + bundles: page.bundles, + total: page.total, + hasMore: page.next_cursor !== undefined, + nextCursor: page.next_cursor, + }); + } catch { + setInboxHistoryError(targetWorkspaceId, generation); + } + }, + [storeApi], + ); + + const loadMore = useCallback( + async (targetWorkspaceId: string) => { + const state = storeApi.getState(); + const workspace = state.inboxHistory.byWorkspaceId[targetWorkspaceId]; + const generation = state.inboxHistory.generationByWorkspaceId[targetWorkspaceId] ?? 0; + if (!workspace?.nextCursor || !workspace.hasMore || workspace.status !== "ready") return; + if (!state.beginInboxHistoryLoadMore(targetWorkspaceId, generation)) return; + + try { + const page = await listInboxHistory(targetWorkspaceId, { cursor: workspace.nextCursor }); + storeApi.getState().appendInboxHistoryPage(targetWorkspaceId, generation, { + bundles: page.bundles, + total: page.total, + hasMore: page.next_cursor !== undefined, + nextCursor: page.next_cursor, + }); + } catch { + storeApi.getState().setInboxHistoryLoadMoreError(targetWorkspaceId, generation); + } + }, + [storeApi], + ); + + useEffect(() => { + if (!enabled || !workspaceId) return; + void refresh(workspaceId); + }, [enabled, workspaceId, refresh]); + + useForegroundRefresh( + () => { + if (workspaceId) void refresh(workspaceId); + }, + enabled && !!workspaceId, + workspaceId, + ); + + return { refresh, loadMore }; +} diff --git a/apps/web/lib/api/domains/inbox-history-api.ts b/apps/web/lib/api/domains/inbox-history-api.ts new file mode 100644 index 0000000000..3c0997924a --- /dev/null +++ b/apps/web/lib/api/domains/inbox-history-api.ts @@ -0,0 +1,20 @@ +import { fetchJson, type ApiRequestOptions } from "../client"; +import type { InboxHistoryPage } from "@/lib/types/inbox-history"; + +const BASE = "/api/v1/clarification-inbox/history"; + +export type ListInboxHistoryOptions = ApiRequestOptions & { + cursor?: string; +}; + +// The read-only History endpoint. Cursor forwarding is explicit, and this +// module has no mutating call. +export function listInboxHistory( + workspaceId: string, + options?: ListInboxHistoryOptions, +): Promise { + const { cursor, ...requestOptions } = options ?? {}; + const query = new URLSearchParams({ workspace_id: workspaceId }); + if (cursor) query.set("cursor", cursor); + return fetchJson(`${BASE}?${query.toString()}`, requestOptions); +} diff --git a/apps/web/lib/failed-inbox/inbox-tab.ts b/apps/web/lib/failed-inbox/inbox-tab.ts index bbc17a5a30..6fe6262444 100644 --- a/apps/web/lib/failed-inbox/inbox-tab.ts +++ b/apps/web/lib/failed-inbox/inbox-tab.ts @@ -1,16 +1,18 @@ -export type InboxTab = "needs-you" | "failed"; +export type InboxTab = "needs-you" | "failed" | "history"; export const INBOX_TAB_QUERY_PARAM = "tab"; // AC-UI-INBOX-FAILED-001.2: the `tab` query parameter's only recognised -// values are `needs-you` and `failed`. An absent, empty, repeated, or -// unrecognised value renders Needs you rather than an error -- "repeated" -// means exactly one occurrence is required, even if every repeated value is -// individually valid. +// values are `needs-you`, `failed`, and `history`. An absent, empty, +// repeated, or unrecognised value renders Needs you rather than an error -- +// "repeated" means exactly one occurrence is required, even if every +// repeated value is individually valid. export function resolveInboxTab(searchParams: URLSearchParams): InboxTab { const values = searchParams.getAll(INBOX_TAB_QUERY_PARAM); if (values.length !== 1) return "needs-you"; - return values[0] === "failed" ? "failed" : "needs-you"; + if (values[0] === "failed") return "failed"; + if (values[0] === "history") return "history"; + return "needs-you"; } // Selecting a tab REPLACES the current history entry (AC-UI-INBOX-FAILED-001.2), diff --git a/apps/web/lib/inbox-history/row-presentation.test.ts b/apps/web/lib/inbox-history/row-presentation.test.ts new file mode 100644 index 0000000000..851b96e53e --- /dev/null +++ b/apps/web/lib/inbox-history/row-presentation.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it } from "vitest"; +import { sessionId as toSessionId, taskId as toTaskId } from "@/lib/types/http"; +import type { Message } from "@/lib/types/http"; +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; +import { + inboxHistoryClarificationQuestions, + inboxHistoryPermissionContent, + inboxHistoryReasonLabelKey, + inboxHistorySecondaryText, +} from "./row-presentation"; + +const TASK_ID = "t1"; +const SESSION_ID = "s1"; +const CREATED_AT = "2026-09-03T04:54:02Z"; + +function bundle(overrides: Partial = {}): InboxHistoryBundle { + return { + pending_id: "p1", + task_id: TASK_ID, + session_id: SESSION_ID, + session_state: "COMPLETED", + task_title: "Task", + created_at: CREATED_AT, + context: "", + messages: [], + kind: "clarification", + reason: "session_ended", + asking_turn_id: "turn-1", + ...overrides, + }; +} + +function message(overrides: Partial = {}): Message { + return { + id: "m1", + session_id: toSessionId(SESSION_ID), + task_id: toTaskId(TASK_ID), + author_type: "agent", + content: "", + type: "clarification_request", + created_at: CREATED_AT, + ...overrides, + }; +} + +describe("inboxHistoryClarificationQuestions", () => { + it("extracts title, prompt and option labels per question, in message order (AC .30, AC .19)", () => { + const b = bundle({ + messages: [ + message({ + metadata: { + question: { + id: "q1", + title: "Pick a branch", + prompt: "Which branch should I use?", + options: [ + { option_id: "o1", label: "main", description: "" }, + { option_id: "o2", label: "dev", description: "" }, + ], + }, + }, + }), + ], + }); + + const questions = inboxHistoryClarificationQuestions(b); + + expect(questions).toHaveLength(1); + expect(questions[0]).toMatchObject({ + id: "q1", + title: "Pick a branch", + prompt: "Which branch should I use?", + optionLabels: ["main", "dev"], + }); + }); + + it("says so rather than render blank when a question has no resolvable content (AC .13)", () => { + const b = bundle({ messages: [message({ metadata: {} })] }); + + const questions = inboxHistoryClarificationQuestions(b); + + expect(questions[0].title).toBe(""); + expect(questions[0].prompt).toBe(""); + expect(questions[0].id).toBe("p1-0"); + }); +}); + +describe("inboxHistoryPermissionContent", () => { + it("reads content and options[].name, never .label, for a permission bundle (AC .30)", () => { + const b = bundle({ + kind: "permission", + messages: [ + message({ + content: "Allow running `rm -rf tmp/`?", + type: "permission_request", + metadata: { + options: [ + { kind: "allow", name: "Allow", option_id: "o1" }, + { kind: "reject", name: "Reject", option_id: "o2" }, + ], + }, + }), + ], + }); + + const result = inboxHistoryPermissionContent(b); + + expect(result.content).toBe("Allow running `rm -rf tmp/`?"); + expect(result.optionNames).toEqual(["Allow", "Reject"]); + }); +}); + +describe("inboxHistoryReasonLabelKey", () => { + it("maps each of the three reasons to a distinct i18n key (AC .9)", () => { + expect(inboxHistoryReasonLabelKey("superseded")).toBe("inboxHistory:reasonSuperseded"); + expect(inboxHistoryReasonLabelKey("session_ended")).toBe("inboxHistory:reasonSessionEnded"); + expect(inboxHistoryReasonLabelKey("unreadable")).toBe("inboxHistory:reasonUnreadable"); + }); +}); + +describe("inboxHistorySecondaryText", () => { + it("prefers task_title, falling back to task_id", () => { + expect(inboxHistorySecondaryText(bundle({ task_title: "My task" }))).toBe("My task"); + expect(inboxHistorySecondaryText(bundle({ task_title: "", task_id: "t-42" }))).toBe("t-42"); + }); +}); diff --git a/apps/web/lib/inbox-history/row-presentation.ts b/apps/web/lib/inbox-history/row-presentation.ts new file mode 100644 index 0000000000..0134e5efb1 --- /dev/null +++ b/apps/web/lib/inbox-history/row-presentation.ts @@ -0,0 +1,65 @@ +import type { ClarificationOption, ClarificationRequestMetadata } from "@/lib/types/http-agents"; +import type { + InboxHistoryBundle, + InboxHistoryPermissionMetadata, + InboxHistoryReason, +} from "@/lib/types/inbox-history"; + +export type InboxHistoryQuestionView = { + id: string; + title: string; + prompt: string; + optionLabels: string[]; +}; + +// A clarification bundle renders each question's title/prompt and option +// labels, in the order the API already returned them -- including a +// question whose own status is already terminal. +export function inboxHistoryClarificationQuestions( + bundle: InboxHistoryBundle, +): InboxHistoryQuestionView[] { + return bundle.messages.map((message, index) => { + const metadata = message.metadata as ClarificationRequestMetadata | undefined; + const question = metadata?.question; + return { + id: question?.id || `${bundle.pending_id}-${index}`, + title: question?.title ?? "", + prompt: question?.prompt ?? "", + optionLabels: (question?.options ?? []).map((option: ClarificationOption) => option.label), + }; + }); +} + +// A permission bundle carries no `question` object -- its text is the +// message's own `content` and its choices are `metadata.options[].name`, not +// `.label`. A permission bundle is always a bundle of one (system design +// "Record shape"). +export function inboxHistoryPermissionContent(bundle: InboxHistoryBundle): { + content: string; + optionNames: string[]; +} { + const message = bundle.messages[0]; + const metadata = message?.metadata as InboxHistoryPermissionMetadata | undefined; + return { + content: message?.content ?? "", + optionNames: (metadata?.options ?? []).map((option) => option.name), + }; +} + +const REASON_LABEL_KEYS: Record = { + superseded: "inboxHistory:reasonSuperseded", + session_ended: "inboxHistory:reasonSessionEnded", + unreadable: "inboxHistory:reasonUnreadable", +}; + +// The three reasons must read as distinguishable statements, never the raw +// `pending` status. +export function inboxHistoryReasonLabelKey(reason: InboxHistoryReason): string { + return REASON_LABEL_KEYS[reason]; +} + +// Secondary text: task title then task identifier, matching the Needs-you +// sibling's row-presentation convention. +export function inboxHistorySecondaryText(bundle: InboxHistoryBundle): string { + return bundle.task_title || bundle.task_id; +} diff --git a/apps/web/lib/state/app-state-extra-actions.ts b/apps/web/lib/state/app-state-extra-actions.ts index 1d946f8290..1ac336d632 100644 --- a/apps/web/lib/state/app-state-extra-actions.ts +++ b/apps/web/lib/state/app-state-extra-actions.ts @@ -14,6 +14,7 @@ import type { ReviewSliceActions, NeedsYouInboxSliceActions, FailedInboxSliceActions, + InboxHistorySliceActions, } from "./slices"; // Split out of app-state-types.ts to stay under the file line limit. These @@ -33,4 +34,5 @@ export type AppStateExtraActions = Pick()( + immer((set) => + createInboxHistorySlice(set as unknown as Parameters[0]), + ), + ); +} + +function bundle(overrides: Partial = {}): InboxHistoryBundle { + return { + pending_id: "p1", + task_id: "t1", + session_id: "s1", + session_state: "COMPLETED", + task_title: "Task", + created_at: "2026-09-03T04:54:02Z", + context: "", + messages: [], + kind: "clarification", + reason: "session_ended", + asking_turn_id: "turn-1", + ...overrides, + }; +} + +describe("inbox-history slice", () => { + it("has no rows or total for a workspace before any read", () => { + const store = newStore(); + expect(store.getState().inboxHistory.byWorkspaceId.w1).toBeUndefined(); + }); + + it("applies a page read tagged with the current generation", () => { + const store = newStore(); + const generation = store.getState().beginInboxHistoryRead("w1"); + + store.getState().setInboxHistoryPage("w1", generation, { + bundles: [bundle()], + total: 1, + hasMore: true, + nextCursor: "cursor-1", + }); + + const state = store.getState().inboxHistory.byWorkspaceId.w1; + expect(state.status).toBe("ready"); + expect(state.total).toBe(1); + expect(state.bundles).toHaveLength(1); + expect(state.hasMore).toBe(true); + expect(state.nextCursor).toBe("cursor-1"); + }); + + it("appends a cursor page and clears its loading state", () => { + const store = newStore(); + const generation = store.getState().beginInboxHistoryRead("w1"); + store.getState().setInboxHistoryPage("w1", generation, { + bundles: [bundle({ pending_id: "first" })], + total: 2, + hasMore: true, + nextCursor: "cursor-1", + }); + + expect(store.getState().beginInboxHistoryLoadMore("w1", generation)).toBe(true); + expect(store.getState().inboxHistory.byWorkspaceId.w1.isLoadingMore).toBe(true); + + store.getState().appendInboxHistoryPage("w1", generation, { + bundles: [bundle({ pending_id: "second" })], + total: 2, + hasMore: false, + }); + + const state = store.getState().inboxHistory.byWorkspaceId.w1; + expect(state.bundles.map((item) => item.pending_id)).toEqual(["first", "second"]); + expect(state.hasMore).toBe(false); + expect(state.nextCursor).toBeUndefined(); + expect(state.isLoadingMore).toBe(false); + expect(state.loadMoreError).toBe(false); + }); + + it("keeps loaded rows when a cursor page fails", () => { + const store = newStore(); + const generation = store.getState().beginInboxHistoryRead("w1"); + store.getState().setInboxHistoryPage("w1", generation, { + bundles: [bundle()], + total: 2, + hasMore: true, + nextCursor: "cursor-1", + }); + store.getState().beginInboxHistoryLoadMore("w1", generation); + store.getState().setInboxHistoryLoadMoreError("w1", generation); + + const state = store.getState().inboxHistory.byWorkspaceId.w1; + expect(state.bundles).toHaveLength(1); + expect(state.hasMore).toBe(true); + expect(state.nextCursor).toBe("cursor-1"); + expect(state.loadMoreError).toBe(true); + expect(state.isLoadingMore).toBe(false); + }); +}); + +describe("inbox-history slice stale responses", () => { + // AC .23 (mirrors AC-UI-NEEDS-YOU-INBOX-001.38): a fast workspace switch or + // overlapping refresh must not let a slower, older response overwrite what + // a newer one already wrote. + it("drops a page response whose generation was superseded by a newer read", () => { + const store = newStore(); + const staleGeneration = store.getState().beginInboxHistoryRead("w1"); + const freshGeneration = store.getState().beginInboxHistoryRead("w1"); + expect(freshGeneration).not.toBe(staleGeneration); + + store.getState().setInboxHistoryPage("w1", freshGeneration, { + bundles: [bundle({ pending_id: "fresh" })], + total: 1, + hasMore: false, + }); + store.getState().setInboxHistoryPage("w1", staleGeneration, { + bundles: [bundle({ pending_id: "stale" })], + total: 1, + hasMore: false, + }); + + const state = store.getState().inboxHistory.byWorkspaceId.w1; + expect(state.bundles[0].pending_id).toBe("fresh"); + }); + + it("clears rows and total in the same update on a failed read that follows a successful one", () => { + const store = newStore(); + const okGeneration = store.getState().beginInboxHistoryRead("w1"); + store.getState().setInboxHistoryPage("w1", okGeneration, { + bundles: [bundle()], + total: 1, + hasMore: false, + }); + + const failGeneration = store.getState().beginInboxHistoryRead("w1"); + store.getState().setInboxHistoryError("w1", failGeneration); + + const state = store.getState().inboxHistory.byWorkspaceId.w1; + expect(state.status).toBe("error"); + expect(state.bundles).toHaveLength(0); + expect(state.total).toBe(0); + }); + + it("drops a stale error response the same way it drops a stale page response", () => { + const store = newStore(); + const staleGeneration = store.getState().beginInboxHistoryRead("w1"); + const freshGeneration = store.getState().beginInboxHistoryRead("w1"); + store.getState().setInboxHistoryPage("w1", freshGeneration, { + bundles: [bundle()], + total: 1, + hasMore: false, + }); + + store.getState().setInboxHistoryError("w1", staleGeneration); + + const state = store.getState().inboxHistory.byWorkspaceId.w1; + expect(state.status).toBe("ready"); + expect(state.total).toBe(1); + }); +}); diff --git a/apps/web/lib/state/slices/inbox-history/inbox-history-slice.ts b/apps/web/lib/state/slices/inbox-history/inbox-history-slice.ts new file mode 100644 index 0000000000..65b2308c78 --- /dev/null +++ b/apps/web/lib/state/slices/inbox-history/inbox-history-slice.ts @@ -0,0 +1,119 @@ +import type { StateCreator } from "zustand"; +import type { AppState } from "../../app-state-types"; +import type { + InboxHistorySlice, + InboxHistorySliceState, + InboxHistoryWorkspaceState, +} from "./types"; + +export const defaultInboxHistoryState: InboxHistorySliceState = { + inboxHistory: { + byWorkspaceId: {}, + generationByWorkspaceId: {}, + }, +}; + +const emptyWorkspaceState = (): InboxHistoryWorkspaceState => ({ + bundles: [], + total: 0, + hasMore: false, + isLoadingMore: false, + loadMoreError: false, + status: "idle", + appliedGeneration: 0, +}); + +// Typed against the full `AppState` (the zustand slices-pattern shape), not +// this slice's own narrower type -- keeps `set` structurally identical to the +// root store's `set`, so composing this slice in store.ts needs no `as any` +// escape (ARCH-FRONTEND-ROOT-STATE-CAST), mirroring the needs-you-inbox slice. +type ImmerSet = Parameters< + StateCreator +>[0]; + +export const createInboxHistorySlice = (set: ImmerSet): InboxHistorySlice => ({ + ...defaultInboxHistoryState, + + beginInboxHistoryRead: (workspaceId) => { + let generation = 0; + set((draft) => { + const next = (draft.inboxHistory.generationByWorkspaceId[workspaceId] ?? 0) + 1; + draft.inboxHistory.generationByWorkspaceId[workspaceId] = next; + const workspace = draft.inboxHistory.byWorkspaceId[workspaceId] ?? emptyWorkspaceState(); + workspace.status = "loading"; + workspace.isLoadingMore = false; + workspace.loadMoreError = false; + draft.inboxHistory.byWorkspaceId[workspaceId] = workspace; + generation = next; + }); + return generation; + }, + + setInboxHistoryPage: (workspaceId, generation, page) => + set((draft) => { + if (draft.inboxHistory.generationByWorkspaceId[workspaceId] !== generation) return; + draft.inboxHistory.byWorkspaceId[workspaceId] = { + bundles: page.bundles, + total: page.total, + hasMore: page.hasMore, + nextCursor: page.nextCursor, + isLoadingMore: false, + loadMoreError: false, + status: "ready", + appliedGeneration: generation, + }; + }), + + beginInboxHistoryLoadMore: (workspaceId, generation) => { + let accepted = false; + set((draft) => { + if (draft.inboxHistory.generationByWorkspaceId[workspaceId] !== generation) return; + const workspace = draft.inboxHistory.byWorkspaceId[workspaceId]; + if ( + !workspace || + workspace.status !== "ready" || + !workspace.hasMore || + !workspace.nextCursor || + workspace.isLoadingMore + ) { + return; + } + workspace.isLoadingMore = true; + workspace.loadMoreError = false; + accepted = true; + }); + return accepted; + }, + + appendInboxHistoryPage: (workspaceId, generation, page) => + set((draft) => { + if (draft.inboxHistory.generationByWorkspaceId[workspaceId] !== generation) return; + const workspace = draft.inboxHistory.byWorkspaceId[workspaceId]; + if (!workspace) return; + workspace.bundles.push(...page.bundles); + workspace.total = page.total; + workspace.hasMore = page.hasMore; + workspace.nextCursor = page.nextCursor; + workspace.isLoadingMore = false; + workspace.loadMoreError = false; + }), + + setInboxHistoryLoadMoreError: (workspaceId, generation) => + set((draft) => { + if (draft.inboxHistory.generationByWorkspaceId[workspaceId] !== generation) return; + const workspace = draft.inboxHistory.byWorkspaceId[workspaceId]; + if (!workspace) return; + workspace.isLoadingMore = false; + workspace.loadMoreError = true; + }), + + setInboxHistoryError: (workspaceId, generation) => + set((draft) => { + if (draft.inboxHistory.generationByWorkspaceId[workspaceId] !== generation) return; + draft.inboxHistory.byWorkspaceId[workspaceId] = { + ...emptyWorkspaceState(), + status: "error", + appliedGeneration: generation, + }; + }), +}); diff --git a/apps/web/lib/state/slices/inbox-history/index.ts b/apps/web/lib/state/slices/inbox-history/index.ts new file mode 100644 index 0000000000..1ba330005d --- /dev/null +++ b/apps/web/lib/state/slices/inbox-history/index.ts @@ -0,0 +1,8 @@ +export { createInboxHistorySlice, defaultInboxHistoryState } from "./inbox-history-slice"; +export type { + InboxHistorySlice, + InboxHistorySliceActions, + InboxHistorySliceState, + InboxHistoryWorkspaceState, + InboxHistoryReadStatus, +} from "./types"; diff --git a/apps/web/lib/state/slices/inbox-history/selectors.ts b/apps/web/lib/state/slices/inbox-history/selectors.ts new file mode 100644 index 0000000000..3b954495d6 --- /dev/null +++ b/apps/web/lib/state/slices/inbox-history/selectors.ts @@ -0,0 +1,47 @@ +import type { AppState } from "@/lib/state/store"; +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; +import type { InboxHistoryWorkspaceState } from "./types"; + +const EMPTY_BUNDLES: InboxHistoryBundle[] = []; + +const EMPTY_WORKSPACE_STATE: InboxHistoryWorkspaceState = { + bundles: EMPTY_BUNDLES, + total: 0, + hasMore: false, + isLoadingMore: false, + loadMoreError: false, + status: "idle", + appliedGeneration: 0, +}; + +function selectActiveWorkspaceState(state: AppState): InboxHistoryWorkspaceState { + const workspaceId = state.workspaces.activeId; + if (!workspaceId) return EMPTY_WORKSPACE_STATE; + return state.inboxHistory.byWorkspaceId[workspaceId] ?? EMPTY_WORKSPACE_STATE; +} + +/** The History tab's own bundle count (AC .16), unrelated to and never + * reading the sidebar/Needs-you count state (AC .17). */ +export function selectInboxHistoryCount(state: AppState): number { + return selectActiveWorkspaceState(state).total; +} + +export function selectInboxHistoryBundles(state: AppState): InboxHistoryBundle[] { + return selectActiveWorkspaceState(state).bundles; +} + +export function selectInboxHistoryStatus(state: AppState) { + return selectActiveWorkspaceState(state).status; +} + +export function selectInboxHistoryHasMore(state: AppState): boolean { + return selectActiveWorkspaceState(state).hasMore; +} + +export function selectInboxHistoryIsLoadingMore(state: AppState): boolean { + return selectActiveWorkspaceState(state).isLoadingMore; +} + +export function selectInboxHistoryLoadMoreError(state: AppState): boolean { + return selectActiveWorkspaceState(state).loadMoreError; +} diff --git a/apps/web/lib/state/slices/inbox-history/types.ts b/apps/web/lib/state/slices/inbox-history/types.ts new file mode 100644 index 0000000000..9c5be65a92 --- /dev/null +++ b/apps/web/lib/state/slices/inbox-history/types.ts @@ -0,0 +1,62 @@ +import type { InboxHistoryBundle } from "@/lib/types/inbox-history"; + +export type InboxHistoryReadStatus = "idle" | "loading" | "ready" | "error"; + +export type InboxHistoryWorkspaceState = { + bundles: InboxHistoryBundle[]; + total: number; + hasMore: boolean; + nextCursor?: string; + isLoadingMore: boolean; + loadMoreError: boolean; + status: InboxHistoryReadStatus; + // The generation of the last response actually applied to this workspace's + // rows, for the stale-response guard. + appliedGeneration: number; +}; + +export type InboxHistorySliceState = { + inboxHistory: { + byWorkspaceId: Record; + // Latest generation ISSUED per workspace (bumped once per read this + // client starts). A response is applied only when its generation still + // equals this counter -- anything older lost the race and is dropped. + generationByWorkspaceId: Record; + }; +}; + +export type InboxHistorySliceActions = { + /** Bumps and returns the new request generation for a workspace read. */ + beginInboxHistoryRead: (workspaceId: string) => number; + /** Starts a cursor read if the workspace is ready and has another page. */ + beginInboxHistoryLoadMore: (workspaceId: string, generation: number) => boolean; + /** Applies a successful page read if its generation is still current. */ + setInboxHistoryPage: ( + workspaceId: string, + generation: number, + page: { + bundles: InboxHistoryBundle[]; + total: number; + hasMore: boolean; + nextCursor?: string; + }, + ) => void; + /** Appends a cursor page if its generation is still current. */ + appendInboxHistoryPage: ( + workspaceId: string, + generation: number, + page: { + bundles: InboxHistoryBundle[]; + total: number; + hasMore: boolean; + nextCursor?: string; + }, + ) => void; + /** Leaves existing rows visible when a cursor read fails. */ + setInboxHistoryLoadMoreError: (workspaceId: string, generation: number) => void; + /** Applies a failed read if its generation is still current -- clears rows + * and total in the same update, so the badge never shows a stale count. */ + setInboxHistoryError: (workspaceId: string, generation: number) => void; +}; + +export type InboxHistorySlice = InboxHistorySliceState & InboxHistorySliceActions; diff --git a/apps/web/lib/state/slices/index.ts b/apps/web/lib/state/slices/index.ts index 5be9457fdc..e0eca79b1c 100644 --- a/apps/web/lib/state/slices/index.ts +++ b/apps/web/lib/state/slices/index.ts @@ -25,6 +25,10 @@ export { defaultNeedsYouInboxState, } from "./needs-you-inbox/needs-you-inbox-slice"; export { createFailedInboxSlice, defaultFailedInboxState } from "./failed-inbox/failed-inbox-slice"; +export { + createInboxHistorySlice, + defaultInboxHistoryState, +} from "./inbox-history/inbox-history-slice"; // Export types export type { KanbanSlice, KanbanSliceState, KanbanSliceActions } from "./kanban/types"; @@ -112,6 +116,13 @@ export type { FailedInboxWorkspaceState, FailedInboxReadStatus, } from "./failed-inbox/types"; +export type { + InboxHistorySlice, + InboxHistorySliceActions, + InboxHistorySliceState, + InboxHistoryWorkspaceState, + InboxHistoryReadStatus, +} from "./inbox-history/types"; // Re-export commonly used types from each domain export type { diff --git a/apps/web/lib/state/store.ts b/apps/web/lib/state/store.ts index e4e07c0a2e..c7e425c485 100644 --- a/apps/web/lib/state/store.ts +++ b/apps/web/lib/state/store.ts @@ -27,6 +27,7 @@ import { createReviewSlice, createNeedsYouInboxSlice, createFailedInboxSlice, + createInboxHistorySlice, } from "./slices"; // Re-export all types from slices for backwards compatibility. @@ -85,6 +86,7 @@ export function createAppStore(initialState?: HydrationState) { ...createReviewSlice(set as any), ...createNeedsYouInboxSlice(set), ...createFailedInboxSlice(set, get), + ...createInboxHistorySlice(set), // Re-assert merged initial state so caller-supplied values win over slice defaults. ...buildStateOverrides(merged), // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/apps/web/lib/types/inbox-history.ts b/apps/web/lib/types/inbox-history.ts new file mode 100644 index 0000000000..d2a01b75f4 --- /dev/null +++ b/apps/web/lib/types/inbox-history.ts @@ -0,0 +1,43 @@ +// Inbox History wire types (docs/specs/ui/requirements/inbox-history.md, +// docs/specs/ui/system-design/inbox-history.md). Mirrors +// apps/backend/internal/clarification/inbox_history_handlers.go's +// inboxHistoryBundleView / inboxHistoryListResponse. +import type { ClarificationInboxBundle } from "@/lib/types/clarification-inbox"; + +export type InboxHistoryReason = "superseded" | "session_ended" | "unreadable"; + +export type InboxHistoryBundleKind = "clarification" | "permission"; + +// Extends the shipped bundle shape (clarification fields reuse the same +// `context`/`messages` projection) with the exclusion reason and the turn +// identity. +export type InboxHistoryBundle = ClarificationInboxBundle & { + kind: InboxHistoryBundleKind; + reason: InboxHistoryReason; + asking_turn_id: string; + superseding_turn_id?: string; + // Absent when the owning task's current step could not be read: the row + // omits the label rather than guessing. + step_starts_no_agent?: boolean; +}; + +export type InboxHistoryPage = { + bundles: InboxHistoryBundle[]; + count: number; + total: number; + // Present exactly when the page was truncated; absent means exhausted. + next_cursor?: string; +}; + +// A permission message's own metadata shape (system design "A permission +// record is a different shape"): no `question`, choices are `options[]` +// entries carrying `name`, not `label`. +export type InboxHistoryPermissionOption = { + kind: string; + name: string; + option_id: string; +}; + +export type InboxHistoryPermissionMetadata = { + options?: InboxHistoryPermissionOption[]; +}; diff --git a/apps/web/src/locales/en/inboxHistory.json b/apps/web/src/locales/en/inboxHistory.json new file mode 100644 index 0000000000..640e810c64 --- /dev/null +++ b/apps/web/src/locales/en/inboxHistory.json @@ -0,0 +1,25 @@ +{ + "tabLabel": "History", + "emptyTitle": "No history in {{workspace}}.", + "emptyTitleUnnamedWorkspace": "No history in this workspace.", + "emptyDescription": "This tab lists questions an agent asked that nobody answered: superseded by later work, left behind when a session ended, or unreadable. It is read-only audit evidence, not a queue to clear.", + "errorTitle": "Could not load history", + "errorDescription": "Something went wrong loading the questions in this workspace's history.", + "retry": "Retry", + "reasonSuperseded": "Superseded by later work", + "reasonSessionEnded": "Session ended without an answer", + "reasonUnreadable": "Could not be read", + "clarificationLabel": "Clarification question", + "permissionLabel": "Permission request", + "noContent": "This question could not be read.", + "openTask": "Open task", + "copyId": "Copy identifier", + "copyIdSuccess": "Copied the bundle identifier.", + "copyIdFailed": "Could not copy the bundle identifier.", + "turnAsked": "Asked in turn {{turn}}", + "turnSupersededBy": "Asked in turn {{askingTurn}}, superseded by turn {{supersedingTurn}}", + "stepStartsNoAgent": "This task's current step will not start an agent.", + "truncatedNotice": "More historical questions are available.", + "loadMore": "Load more history", + "loadMoreError": "Could not load more history. Try again." +} diff --git a/apps/web/src/locales/pseudo/inboxHistory.json b/apps/web/src/locales/pseudo/inboxHistory.json new file mode 100644 index 0000000000..a6d96b35ea --- /dev/null +++ b/apps/web/src/locales/pseudo/inboxHistory.json @@ -0,0 +1,25 @@ +{ + "tabLabel": "Ĥĩśţōŕŷ", + "emptyTitle": "Ńō ĥĩśţōŕŷ ĩń {{workspace}}.", + "emptyTitleUnnamedWorkspace": "Ńō ĥĩśţōŕŷ ĩń ţĥĩś ŵōŕķśƥàćē.", + "emptyDescription": "Ţĥĩś ţàƀ ĺĩśţś qũēśţĩōńś àń àĝēńţ àśķēď ţĥàţ ńōƀōďŷ àńśŵēŕēď: śũƥēŕśēďēď ƀŷ ĺàţēŕ ŵōŕķ, ĺēƒţ ƀēĥĩńď ŵĥēń à śēśśĩōń ēńďēď, ōŕ ũńŕēàďàƀĺē. Ĩţ ĩś ŕēàď-ōńĺŷ àũďĩţ ēvĩďēńćē, ńōţ à qũēũē ţō ćĺēàŕ.", + "errorTitle": "Ćōũĺď ńōţ ĺōàď ĥĩśţōŕŷ", + "errorDescription": "Śōḿēţĥĩńĝ ŵēńţ ŵŕōńĝ ĺōàďĩńĝ ţĥē qũēśţĩōńś ĩń ţĥĩś ŵōŕķśƥàćē'ś ĥĩśţōŕŷ.", + "retry": "Ŕēţŕŷ", + "reasonSuperseded": "Śũƥēŕśēďēď ƀŷ ĺàţēŕ ŵōŕķ", + "reasonSessionEnded": "Śēśśĩōń ēńďēď ŵĩţĥōũţ àń àńśŵēŕ", + "reasonUnreadable": "Ćōũĺď ńōţ ƀē ŕēàď", + "clarificationLabel": "Ćĺàŕĩƒĩćàţĩōń qũēśţĩōń", + "permissionLabel": "Ƥēŕḿĩśśĩōń ŕēqũēśţ", + "noContent": "Ţĥĩś qũēśţĩōń ćōũĺď ńōţ ƀē ŕēàď.", + "openTask": "Ōƥēń ţàśķ", + "copyId": "Ćōƥŷ ĩďēńţĩƒĩēŕ", + "copyIdSuccess": "Ćōƥĩēď ţĥē ƀũńďĺē ĩďēńţĩƒĩēŕ.", + "copyIdFailed": "Ćōũĺď ńōţ ćōƥŷ ţĥē ƀũńďĺē ĩďēńţĩƒĩēŕ.", + "turnAsked": "Àśķēď ĩń ţũŕń {{turn}}", + "turnSupersededBy": "Àśķēď ĩń ţũŕń {{askingTurn}}, śũƥēŕśēďēď ƀŷ ţũŕń {{supersedingTurn}}", + "stepStartsNoAgent": "Ţĥĩś ţàśķ'ś ćũŕŕēńţ śţēƥ ŵĩĺĺ ńōţ śţàŕţ àń àĝēńţ.", + "truncatedNotice": "Ḿōŕē ĥĩśţōŕĩćàĺ qũēśţĩōńś àŕē àvàĩĺàƀĺē.", + "loadMore": "Ĺōàď ḿōŕē ĥĩśţōŕŷ", + "loadMoreError": "Ćōũĺď ńōţ ĺōàď ḿōŕē ĥĩśţōŕŷ. Ţŕŷ àĝàĩń." +} diff --git a/apps/web/src/locales/pt-pt/inboxHistory.json b/apps/web/src/locales/pt-pt/inboxHistory.json new file mode 100644 index 0000000000..4215661242 --- /dev/null +++ b/apps/web/src/locales/pt-pt/inboxHistory.json @@ -0,0 +1,25 @@ +{ + "tabLabel": "Histórico", + "emptyTitle": "Sem histórico em {{workspace}}.", + "emptyTitleUnnamedWorkspace": "Sem histórico nesta área de trabalho.", + "emptyDescription": "Este separador lista perguntas que um agente fez e a que ninguém respondeu: substituídas por trabalho posterior, deixadas para trás quando uma sessão terminou, ou impossíveis de ler. É uma prova de auditoria só de leitura, não uma fila para limpar.", + "errorTitle": "Não foi possível carregar o histórico", + "errorDescription": "Ocorreu um erro ao carregar as perguntas do histórico desta área de trabalho.", + "retry": "Tentar novamente", + "reasonSuperseded": "Substituída por trabalho posterior", + "reasonSessionEnded": "Sessão terminada sem resposta", + "reasonUnreadable": "Não foi possível ler", + "clarificationLabel": "Pergunta de esclarecimento", + "permissionLabel": "Pedido de permissão", + "noContent": "Não foi possível ler esta pergunta.", + "openTask": "Abrir tarefa", + "copyId": "Copiar identificador", + "copyIdSuccess": "Identificador do conjunto copiado.", + "copyIdFailed": "Não foi possível copiar o identificador do conjunto.", + "turnAsked": "Perguntado no turno {{turn}}", + "turnSupersededBy": "Perguntado no turno {{askingTurn}}, substituído pelo turno {{supersedingTurn}}", + "stepStartsNoAgent": "A etapa atual desta tarefa não iniciará um agente.", + "truncatedNotice": "Há mais perguntas históricas disponíveis.", + "loadMore": "Carregar mais histórico", + "loadMoreError": "Não foi possível carregar mais histórico. Tente novamente." +} diff --git a/apps/web/src/locales/zh-cn/inboxHistory.json b/apps/web/src/locales/zh-cn/inboxHistory.json new file mode 100644 index 0000000000..e31e8ae362 --- /dev/null +++ b/apps/web/src/locales/zh-cn/inboxHistory.json @@ -0,0 +1,25 @@ +{ + "tabLabel": "历史记录", + "emptyTitle": "{{workspace}}中没有历史记录。", + "emptyTitleUnnamedWorkspace": "此工作区中没有历史记录。", + "emptyDescription": "此标签列出了代理提出但无人回答的问题：因后续工作被取代、会话结束时被遗留，或无法读取。这是只读的审计证据，不是待清理的队列。", + "errorTitle": "无法加载历史记录", + "errorDescription": "加载此工作区历史记录中的问题时出错。", + "retry": "重试", + "reasonSuperseded": "已被后续工作取代", + "reasonSessionEnded": "会话结束时未获回答", + "reasonUnreadable": "无法读取", + "clarificationLabel": "澄清问题", + "permissionLabel": "权限请求", + "noContent": "无法读取此问题。", + "openTask": "打开任务", + "copyId": "复制标识符", + "copyIdSuccess": "已复制该批次的标识符。", + "copyIdFailed": "无法复制该批次的标识符。", + "turnAsked": "在第 {{turn}} 轮提出", + "turnSupersededBy": "在第 {{askingTurn}} 轮提出，被第 {{supersedingTurn}} 轮取代", + "stepStartsNoAgent": "此任务的当前步骤不会启动代理。", + "truncatedNotice": "还有更多历史问题可查看。", + "loadMore": "加载更多历史记录", + "loadMoreError": "无法加载更多历史记录。请重试。" +} diff --git a/apps/web/src/locales/zh-hk/inboxHistory.json b/apps/web/src/locales/zh-hk/inboxHistory.json new file mode 100644 index 0000000000..fd1864218b --- /dev/null +++ b/apps/web/src/locales/zh-hk/inboxHistory.json @@ -0,0 +1,25 @@ +{ + "tabLabel": "歷史記錄", + "emptyTitle": "{{workspace}}中沒有歷史記錄。", + "emptyTitleUnnamedWorkspace": "此工作區中沒有歷史記錄。", + "emptyDescription": "此標籤列出了代理提出但無人回答的問題：因後續工作被取代、工作階段結束時被遺留，或無法讀取。這是隻讀的審計證據，不是待清理的佇列。", + "errorTitle": "無法載入歷史記錄", + "errorDescription": "載入此工作區歷史記錄中的問題時出錯。", + "retry": "重試", + "reasonSuperseded": "已被後續工作取代", + "reasonSessionEnded": "工作階段結束時未獲回答", + "reasonUnreadable": "無法讀取", + "clarificationLabel": "澄清問題", + "permissionLabel": "權限請求", + "noContent": "無法讀取此問題。", + "openTask": "開啟任務", + "copyId": "複製標識符", + "copyIdSuccess": "已複製該批次的標識符。", + "copyIdFailed": "無法複製該批次的標識符。", + "turnAsked": "在第 {{turn}} 輪提出", + "turnSupersededBy": "在第 {{askingTurn}} 輪提出，被第 {{supersedingTurn}} 輪取代", + "stepStartsNoAgent": "此任務的目前步驟不會啓動代理。", + "truncatedNotice": "還有更多歷史問題可查看。", + "loadMore": "載入更多歷史記錄", + "loadMoreError": "無法載入更多歷史記錄。請重試。" +} diff --git a/apps/web/src/locales/zh-tw/inboxHistory.json b/apps/web/src/locales/zh-tw/inboxHistory.json new file mode 100644 index 0000000000..8859411c1f --- /dev/null +++ b/apps/web/src/locales/zh-tw/inboxHistory.json @@ -0,0 +1,25 @@ +{ + "tabLabel": "歷史記錄", + "emptyTitle": "{{workspace}}中沒有歷史記錄。", + "emptyTitleUnnamedWorkspace": "此工作區中沒有歷史記錄。", + "emptyDescription": "此標籤列出了代理提出但無人回答的問題：因後續工作被取代、工作階段結束時被遺留，或無法讀取。這是隻讀的審計證據，不是待清理的佇列。", + "errorTitle": "無法載入歷史記錄", + "errorDescription": "載入此工作區歷史記錄中的問題時出錯。", + "retry": "重試", + "reasonSuperseded": "已被後續工作取代", + "reasonSessionEnded": "工作階段結束時未獲回答", + "reasonUnreadable": "無法讀取", + "clarificationLabel": "澄清問題", + "permissionLabel": "權限請求", + "noContent": "無法讀取此問題。", + "openTask": "開啟任務", + "copyId": "複製識別符號", + "copyIdSuccess": "已複製該批次的識別符號。", + "copyIdFailed": "無法複製該批次的識別符號。", + "turnAsked": "在第 {{turn}} 輪提出", + "turnSupersededBy": "在第 {{askingTurn}} 輪提出，被第 {{supersedingTurn}} 輪取代", + "stepStartsNoAgent": "此任務的目前步驟不會啟動代理。", + "truncatedNotice": "還有更多歷史問題可查看。", + "loadMore": "載入更多歷史記錄", + "loadMoreError": "無法載入更多歷史記錄。請重試。" +} diff --git a/config/architecture-lint/inbox_history_isolation.json b/config/architecture-lint/inbox_history_isolation.json new file mode 100644 index 0000000000..57ca6a6b45 --- /dev/null +++ b/config/architecture-lint/inbox_history_isolation.json @@ -0,0 +1,5 @@ +{ + "version": 1, + "rule": "ARCH-INBOX-HISTORY-ISOLATION", + "entries": [] +} diff --git a/docs/architecture-lint.md b/docs/architecture-lint.md index cee76bc48d..40a478d024 100644 --- a/docs/architecture-lint.md +++ b/docs/architecture-lint.md @@ -13,6 +13,7 @@ the GitHub Actions workflow only invokes those repository-owned entry points. | `ARCH-RUNTIME-IMPORT` | Production Go outside `internal/agent/runtime/` must not add direct imports of `runtime/lifecycle` or `runtime/agentctl`. | Depend on `internal/agent/runtime` or an explicitly approved low-level adapter. | | `ARCH-TASK-OFFICE-IMPORT` | Production Go under `internal/task/` must not add imports of `internal/office`. | The shared task model owns task concepts; Office consumes or adapts them. | | `ARCH-FRONTEND-ROOT-STATE-CAST` | `apps/web/lib/state/store.ts` must not add `as any` or `as unknown as` escapes. | Derive typed domain state, actions, and defaults. | +| `ARCH-INBOX-HISTORY-ISOLATION` | A closed set of backend pending-action sinks must not reference the Inbox History read's exported entry points, and the Inbox History read/render modules must not reference the Needs-you sidebar-badge state or subscribe to an event stream. | Keep the additive History read (AC-UI-INBOX-HISTORY-001.5/.17/.22) out of the operational pending-action path in both directions. | Each rule owns its exact grandfathered finding set under `config/architecture-lint/`. A current finding absent from that rule's baseline fails. When cleanup removes a finding, the now-stale entry diff --git a/docs/plans/inbox-history/plan.md b/docs/plans/inbox-history/plan.md new file mode 100644 index 0000000000..66fd3ae30f --- /dev/null +++ b/docs/plans/inbox-history/plan.md @@ -0,0 +1,66 @@ +--- +created: 2026-09-15 +status: complete +requirements: + - REQ-UI-INBOX-HISTORY-001 +system_design: + - ../../specs/ui/system-design/inbox-history.md +legacy_specs: [] +--- + +# Implementation Plan: Inbox History + +## Overview + +Add a second, read-only "History" tab to the Inbox that surfaces clarification +and permission bundles that became unanswerable (superseded, session ended, or +unreadable), per [requirements](../../specs/ui/requirements/inbox-history.md) +and [system design](../../specs/ui/system-design/inbox-history.md). + +This feature was built through Kandev's own spec-driven-development workflow +(4 spec-review rounds, 4 build rounds, security/test/code review across +multiple rounds, then PR integration) with its running record kept in the +task's own plan store rather than as narrative here. This document exists to +satisfy the repository's PR documentation coverage gate +(`ci: enforce pull request documentation coverage`), which was not wired into +that workflow when this feature's spec canon was authored. + +## Scope + +### In scope + +- A strictly additive backend read path in + `apps/backend/internal/task/repository/sqlite`, reusing that package's + unexported predicates (`currentTurnAuthority`, `nonTerminalSessionPredicate`). +- A new `ARCH-INBOX-HISTORY-ISOLATION` architecture-lint rule enforcing the + read path's isolation via a source-text scan (closed sink-file set plus a + scoped source-file set). +- A read-only History tab strip, row list, and clarification-detail expansion + on the existing Inbox surface, with open-task and copy-id as the only + actions. +- i18n in all five shipped locales. +- Desktop and mobile E2E coverage for a superseded bundle. + +### Out of scope + +Re-asking or resurfacing an unanswerable question, any change to the sidebar +or Needs-you unread badge, and any migration or backfill for bundles that went +stale before this shipped. See the requirements document's own "Out of scope" +section for the full list. + +## Work orders + +- [x] [Task 01: Inbox History delivery](task-01-inbox-history-delivery.md) + +Single work order; no dependents. + +## Verification results + +See the work order for the full verification record. Summary: backend and +frontend build/lint/typecheck clean; targeted and full package test suites +pass (pre-existing, unrelated failures proven against merge-base in a scratch +worktree); desktop and mobile E2E specs for the History tab pass; `pnpm run +i18n:check` and `pnpm run i18n:ratchet` clean; `python3 +scripts/lint-spec-files.py --all` passes. + +PR: https://github.com/kdlbs/kandev/pull/3679 diff --git a/docs/plans/inbox-history/task-01-inbox-history-delivery.md b/docs/plans/inbox-history/task-01-inbox-history-delivery.md new file mode 100644 index 0000000000..7c96df69d6 --- /dev/null +++ b/docs/plans/inbox-history/task-01-inbox-history-delivery.md @@ -0,0 +1,122 @@ +--- +id: "01-inbox-history-delivery" +title: "Inbox History delivery" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-UI-INBOX-HISTORY-001 +acceptance_criteria: + - AC-UI-INBOX-HISTORY-001.1 + - AC-UI-INBOX-HISTORY-001.2 + - AC-UI-INBOX-HISTORY-001.3 + - AC-UI-INBOX-HISTORY-001.4 + - AC-UI-INBOX-HISTORY-001.5 + - AC-UI-INBOX-HISTORY-001.6 + - AC-UI-INBOX-HISTORY-001.7 + - AC-UI-INBOX-HISTORY-001.8 + - AC-UI-INBOX-HISTORY-001.9 + - AC-UI-INBOX-HISTORY-001.10 + - AC-UI-INBOX-HISTORY-001.11 + - AC-UI-INBOX-HISTORY-001.12 + - AC-UI-INBOX-HISTORY-001.13 + - AC-UI-INBOX-HISTORY-001.14 + - AC-UI-INBOX-HISTORY-001.15 + - AC-UI-INBOX-HISTORY-001.16 + - AC-UI-INBOX-HISTORY-001.17 + - AC-UI-INBOX-HISTORY-001.18 + - AC-UI-INBOX-HISTORY-001.19 + - AC-UI-INBOX-HISTORY-001.20 + - AC-UI-INBOX-HISTORY-001.21 + - AC-UI-INBOX-HISTORY-001.22 + - AC-UI-INBOX-HISTORY-001.23 + - AC-UI-INBOX-HISTORY-001.24 + - AC-UI-INBOX-HISTORY-001.25 + - AC-UI-INBOX-HISTORY-001.26 + - AC-UI-INBOX-HISTORY-001.27 + - AC-UI-INBOX-HISTORY-001.28 + - AC-UI-INBOX-HISTORY-001.29 + - AC-UI-INBOX-HISTORY-001.30 + - AC-UI-INBOX-HISTORY-001.31 +system_design: + - ../../specs/ui/system-design/inbox-history.md +--- + +# Task 01: Inbox History Delivery + +## Summary + +Add a second, read-only "History" tab to the Inbox listing bundles that are +non-terminal, unarchived, not `parent_question` records, and unanswerable for +exactly one of three named reasons (`superseded`, `session_ended`, +`unreadable`). The read path is strictly additive, lives in +`apps/backend/internal/task/repository/sqlite` reusing that package's +unexported predicates, and its isolation is enforced by a new +source-text-scan architecture-lint rule. The count is a bundle count that +renders on the tab and never feeds the sidebar/Needs-you badge. + +## In scope + +- Backend: an additive Inbox History bundle read path + (`ListInboxHistoryBundles`), a new HTTP read endpoint, and Postgres coverage + for the dialect-sensitive query. +- `ARCH-INBOX-HISTORY-ISOLATION` architecture-lint rule scanning a closed + sink-file set and a scoped source-file set. +- Frontend: History tab strip, state slice, row/detail components, and i18n + in all five shipped locales. +- Desktop and mobile E2E coverage for a superseded bundle. + +## Out of scope + +Re-asking or resurfacing an unanswerable question, any change to the sidebar +or Needs-you unread badge, and any migration or backfill for bundles that went +stale before this shipped. + +## Acceptance + +All 31 acceptance criteria under `REQ-UI-INBOX-HISTORY-001`; see +[requirements](../../specs/ui/requirements/inbox-history.md) for the full +text. Notable ones with dedicated regression coverage: + +- AC .2/.3: bundle-level eligibility and mutual exclusion between the Needs-you + and History buckets, including `permission_group_key` disambiguation of a + reused permission `pending_id` at both the SQL classification layer and the + Go hydration layer. +- AC .5/.17/.22: the read's isolation, enforced by the architecture-lint rule + plus a same-package fixture and a runtime test asserting the History + controller never subscribes to the live WebSocket event stream. +- AC .14/.29: open-task and copy-id are the only actions, with no + answer/dismiss/snooze affordance — `inbox-history-superseded.spec.ts` and + `mobile-inbox-history-superseded.spec.ts` (44px touch target, no horizontal + clipping). +- AC .16/.17: the History tab's own bundle count never feeds the sidebar or + Needs-you unread badge — `app-sidebar-primary-nav.test.tsx`. +- AC .30: a row presents what was actually asked, keyed by + `permission_group_key` so a reused `pending_id` cannot render merged or + wrong content. + +## Verification + +- `make fmt`, `make typecheck` — clean. +- `make test` (backend Go) — two pre-existing failure clusters + (`internal/worktree`, one `internal/task/service` gitflow test) proven + pre-existing against `git merge-base HEAD origin/main` in a scratch + worktree; unrelated to this branch's diff (macOS `/var/folders` + symlink-resolution artifact). +- `make lint` — golangci-lint 0 issues, eslint clean, harness/spec/architecture + lints clean. +- `pnpm --filter @kandev/web test`, `pnpm --filter kandev test` (CLI) — clean + aside from one pre-existing, unrelated failure requiring a local Docker + daemon. +- `make test-scripts` — one pre-existing failure (`scripts/pr-await.test.sh`), + proven against the merge-base. +- `pnpm run i18n:check` / `pnpm run i18n:ratchet` — five locales complete, no + em-dash violations, guard allowlist intact. +- `pnpm e2e:run` — desktop `inbox-history-superseded.spec.ts`, + `needs-you-inbox-answer.spec.ts`, and mobile + `mobile-inbox-history-superseded.spec.ts` all pass. +- `python3 scripts/lint-spec-files.py --all` — all specification files + passed. + +PR: https://github.com/kdlbs/kandev/pull/3679 diff --git a/docs/specs/ui/requirements/inbox-history.md b/docs/specs/ui/requirements/inbox-history.md new file mode 100644 index 0000000000..9c8a770cd6 --- /dev/null +++ b/docs/specs/ui/requirements/inbox-history.md @@ -0,0 +1,327 @@ +--- +status: draft +system: ui +created: 2026-09-14 +owners: + - nova28 +--- + +# Inbox History Requirements + +## Overview + +`REQ-UI-NEEDS-YOU-INBOX-001` shipped the Inbox with one bucket: the answerable +clarification bundles for the active workspace. A bundle leaves it the moment it +stops being answerable, most often when a new turn starts with the old question +unanswered. Those rows are not deleted. They stay in +`task_session_messages` with a non-terminal status, and +[`ADR 2026-08-14`](../../../decisions/2026-08-14-current-turn-clarification-ownership.md) +calls them, at line 76, "useful audit evidence but not operational state by +itself". The gap this capability closes is that the audit evidence is unreadable: +every shipped read path is turn-scoped by design, so nothing returns a question +whose turn has been superseded. This adds a second, read-only bucket that can. + +Neither expiry nor the operational predicate changes; the ADR rejects both +alternatives that would close the same gap. The companion +[system design](../system-design/inbox-history.md) carries those alternatives, the +input inventory, the per-read filter table, the reasoning behind each criterion +below, and the code citation for every symbol named here. UI owns this contract +because the observable outcome is a tab, a list and a label; the durable +clarification record, its resolution semantics and the current-turn predicate +remain owned by the integrations system, consumed here unchanged. + +## Input inventory + +Sampled from the running instance on 2026-09-14, SQLite `mode=ro`. Record shapes, +the status histogram, both population samples and every code citation live in the +companion system design. + +**Units, fixed for the whole document.** A *row* is one `task_session_messages` +record. A *bundle* is the rows sharing a `pending_id`; a permission request is +always a bundle of one. A *bundle row* is the rendered list item for one bundle, +and where a criterion below says "row" it means a bundle row. Every count required +below is in **bundles** unless the criterion names another unit. **Terminal +status** is already decided in code by `InteractionStatus.IsTerminal()` +(`interactions.go`): any status other than absent or `pending`. + +**Population.** 46 rows, in 41 bundles across 13 tasks, sit in a step that starts +no agent. That is the number justifying the capability, and the only figure that +reproduced across two samples. + +## Requirements + +### REQ-UI-INBOX-HISTORY-001: Inbox history bucket + +**Intent:** Make the interactions the ADR preserves as audit evidence readable, +without changing what counts as operational. + +**User story:** As an operator, I want to see the questions an agent asked and +nobody answered, so a blocked or abandoned line of work stops being invisible. + +#### Acceptance criteria + +**The bucket and its boundary** + +- **AC-UI-INBOX-HISTORY-001.1:** The Inbox shall render a tab strip using the + shipped `variant="line"` tabs, `Needs you` first and `History` second. + `Needs you` shall remain the default selected tab, its row set, count and + behavior unchanged by the strip's presence. +- **AC-UI-INBOX-HISTORY-001.2:** Eligibility shall be evaluated per BUNDLE. The + History tab shall list a bundle, scoped to the active workspace, when all four + hold: at least one of its rows carries a non-terminal status per the shipped + `InteractionStatus.IsTerminal()`; its owning task is not archived; no row in it + carries a truthy `metadata.parent_question`; and it is unanswerable for at least + one of exactly three reasons. Each listed bundle shall carry its **exclusion + reason**, these and no others: + - **`superseded`**: the bundle's turn is no longer its session's current + conversational turn, or it is a permission request and a newer permission + request exists on that same turn, which the operational projection suppresses. + "Current conversational turn" shall be resolved by `currentTurnAuthority` + ALONE, never composed with `nonTerminalSessionPredicate`, which would make + `session_ended` unreachable. + - **`session_ended`**: the owning session's state is `COMPLETED`, `FAILED` or + `CANCELLED`, and no later turn superseded it. + - **`unreadable`**: the bundle is a **clarification** bundle and at least one of + its questions carries no resolvable question identifier, evaluated per bundle + to match the shipped mechanism. It shall NOT be applied to a permission + request, which carries no question identifier by construction. A permission + bundle shall reach History only via `superseded` or `session_ended`. + + When more than one reason holds, they shall be evaluated in the order listed and + exactly one reported; a test shall assert that a bundle which is both superseded + and unreadable reports `superseded`. A bundle matching none is still live and + shall NOT be listed. The Needs-you per-user dismiss/snooze sidecar shall NOT be + applied to History. +- **AC-UI-INBOX-HISTORY-001.3:** A bundle shall appear in at most one of the two + tabs. A bundle is non-terminal when any of its rows is, matching the shipped + `has_pending`. For any fixture, the intersection of the two bundle sets shall be + empty, and their union shall equal the non-terminal, unarchived bundles LESS the + parent-question records and LESS the live bundles Needs-you itself excludes. A + test shall assert both halves on a fixture holding at least one ordinary live + answerable clarification bundle AND one live current-turn permission request. + Exactly three reasons shall permit a non-terminal bundle to + appear in neither tab, and a test shall assert no fourth arises: the task is + archived (AC .4); it is a parent-question record (AC .2); or it is live (AC .2) + AND Needs-you excludes it by a filter of its own, those being permission-type + and the per-user dismiss/snooze sidecar. +- **AC-UI-INBOX-HISTORY-001.4:** When the owning task is archived, the bundle + shall not be listed in either tab. + +**The additive read path** + +- **AC-UI-INBOX-HISTORY-001.5:** The history read shall be a separate path from + the operational read. It shall not be reachable from any code path that gates a + workflow transition, emits a pending-action event, resolves an answer, or + contributes to a pending-action projection. The read shall live in the shipped + `apps/backend/internal/task/repository/sqlite` package and shall REUSE that + package's unexported `currentTurnAuthority` and `nonTerminalSessionPredicate` + rather than duplicating either. Because it shares a package with most of those + sinks, where no import statement exists to forbid, unreachability shall be + enforced STATICALLY by a SOURCE-TEXT scan rather than an import scan: a new + architecture-lint rule `ARCH-INBOX-HISTORY-ISOLATION` + (`docs/architecture-lint.md`, `make lint-architecture`) shall forbid a closed set + of sink files from referencing the history read's exported entry-point + identifier. That set shall be exactly the five files named by path in the + companion system design; a rule scanning fewer does not satisfy this criterion, + and neither does a runtime test. The rule shall ship with an EMPTY baseline under + `config/architecture-lint/`. A test shall assert both that the rule is registered + with no baseline entries AND that it flags a deliberately violating fixture + placed in the SAME package as the read. +- **AC-UI-INBOX-HISTORY-001.6:** The persisted `status` of a listed bundle shall + be unchanged by listing it, and reading the tab shall perform no write. +- **AC-UI-INBOX-HISTORY-001.7:** + `TestListPendingInteractionsAgreesWithPendingActionProjection` shall pass + unmodified. +- **AC-UI-INBOX-HISTORY-001.8:** A regression test shall drive the real sequence: + ask a question, then start a new turn without answering it, asserting both that + the operational listing does not return the bundle and that the history listing + does. + +**What a row says** + +- **AC-UI-INBOX-HISTORY-001.30:** A row shall present what was actually asked. + For a **clarification** bundle it shall render each + question's `title` and `prompt`, the option labels from `options[]` where the + question carried any, and the bundle's shared `context`. Every question in a + listed bundle shall be reachable, ordered per AC .19, INCLUDING a question whose + own status is already terminal: eligibility is a bundle property (AC .2) and + shall not filter a bundle's questions. A **permission** bundle carries no + `question` object and so no `title`, `prompt`, `question_index` or `context`; it + shall render the message `content` as the request text and the `name` values of + `metadata.options[]` as the offered choices, omitting the clarification-only + fields rather than rendering them empty. Long `context` may be truncated with the + full value reachable in the row, never dropped. +- **AC-UI-INBOX-HISTORY-001.9:** A row shall state the time the question was + asked, and its AC .2 exclusion reason rather than the raw `pending` status, so + the row does not claim an answer is still awaited. The three reasons shall be + distinguishable: `superseded` says the question was superseded by later work, + `session_ended` says the session ended without an answer, and `unreadable` says + the question itself could not be read. No row shall assert supersession unless + its reason is `superseded`. +- **AC-UI-INBOX-HISTORY-001.10:** A row shall identify the turn that asked it. + A row whose exclusion reason is `superseded` shall additionally identify the + turn that superseded it, EXCEPT a permission superseded within its own turn + (AC .2), where the superseding request shares that turn and there is none. That + case, and a reason of `session_ended` or `unreadable`, shall omit the field + rather than render a blank, a placeholder or a fabricated identifier; a test + shall assert the omission for each of those three cases. +- **AC-UI-INBOX-HISTORY-001.11:** When the owning task's current workflow step + starts no agent, the row shall say so. A step starts no agent when its `on_enter` + action list contains no entry whose type is `auto_start_agent`; an **absent or + empty** `on_enter` therefore satisfies it. Presence of that entry is the only + test: the shipped `OnEnterAction` carries a type and a config and no disablement + field, so no disabled entry exists to discount, and an unrecognized type string + is absent by construction. The step evaluated is + the task's current step at read time; if the task is mid-transition or its + workflow cannot be read, the row shall omit the label rather than guess, and + omission shall never read as a claim that an agent will start. This concept shall + not reuse the `parkedOnBackgroundWork` symbol, and the label shall say the step + will not start an agent, not that no agent can ever run. +- **AC-UI-INBOX-HISTORY-001.12:** A listed permission bundle shall be visually + labelled as a permission using the shipped `IconShieldQuestion` and + `text-amber-500` pairing. No new hue shall be introduced. +- **AC-UI-INBOX-HISTORY-001.13:** A clarification bundle holding at least one + question with no resolvable identifier shall still be listed, whole, and shall + not prevent the remaining bundles from rendering. This is a deliberate divergence + from the Needs-you listing, which drops such bundles; a test shall assert it. + This criterion governs LISTING only: the reason reported follows AC .2's + precedence. The bundle shall render whatever question content survives under + AC .30; when none survives it shall say so rather than render blank. + +**Read-only** + +- **AC-UI-INBOX-HISTORY-001.14:** The History tab shall expose no affordance that + answers, rejects, re-asks, dismisses, snoozes or otherwise mutates an + interaction. The only actions shall be opening the owning task and copying its + identifier. +- **AC-UI-INBOX-HISTORY-001.15:** The history read path shall expose no endpoint + accepting a request method other than a read, and the History tab shall issue no + request to any mutating endpoint. A test shall assert that rendering, paging and + acting within the History tab issues no call to the Needs-you sidecar dismiss and + snooze routes. Those shipped routes shall NOT be modified and shall keep + accepting any durable bundle identifier the caller can already reach: this + criterion constrains what the History surface CALLS, not what they ACCEPT. + +**Counting** + +- **AC-UI-INBOX-HISTORY-001.16:** The History count shall render on its tab in + the `Badge variant="secondary"` idiom. It shall count **bundles**, not rows and + not tasks, and shall be the workspace-wide total of AC .20 rather than the + current page's size, so paging does not change it. At zero the badge shall be + omitted rather than rendered as `0`. +- **AC-UI-INBOX-HISTORY-001.17:** No history code path shall write the sidebar + badge state key or the Needs-you count key. Enforced BOTH by the + `ARCH-INBOX-HISTORY-ISOLATION` rule of AC .5, extended to forbid the history + modules scoped in the system design from referencing those two keys, AND by a + runtime test asserting the badge is unchanged by a fixture that populates History + and leaves `Needs you` empty; the rule covers paths the fixture misses. + +**Ordering, bounds and pagination** + +- **AC-UI-INBOX-HISTORY-001.18:** Bundles shall be ordered by `created_at` + ascending, with ties broken by `pending_id` ascending, matching the shipped + sibling listing, so the oldest neglected question appears first. +- **AC-UI-INBOX-HISTORY-001.19:** Within a bundle, questions shall be ordered by + `question_index` ascending, ties broken by `question_id` ascending, then by the + message `id` ascending. The first two keys are the Inbox's shipped + `orderInboxMessages` and the sibling's `AC-UI-NEEDS-YOU-INBOX-001.10`, NOT the + MCP-resolution order of `orderBundleMessages`. The third key breaks the + remaining tie for the empty-`question_id` bundles AC .13 admits, which Needs-you + never lists, so the two Inbox tabs still never order a shared bundle differently. + An index that is absent, negative or not a number shall sort as zero and then by + the remaining keys, so the order is total. +- **AC-UI-INBOX-HISTORY-001.20:** The read shall be bounded, matching the + sibling's parse behavior exactly. An absent limit shall default to 50; a limit + above 200 shall be clamped to 200; a limit that is zero, negative or not a + base-10 integer shall be **rejected with 400, not clamped and not defaulted** + (`TestHttpListInbox_ZeroLimit_RejectedNotClamped`). The response shall carry a + workspace-wide total, in bundles, distinct from the page count. The cursor shall + be the keyset pair (`created_at`, `pending_id`) matching the AC .18 sort; + forward-only in that same ascending direction; **exclusive** of the bundle it + names; opaque to the client; and absent on the final page, which is how a caller + detects the walk's end. Matching the shipped sibling encoder, it encodes + that pair ONLY and carries no workspace: the request's own `workspace_id` scopes + every page, so a cursor minted elsewhere shall merely position the walk inside + the caller's own workspace. A malformed or undecodable cursor shall be rejected + with 400 and shall never silently restart from page one. A cursor naming a bundle + that has since left History is not an error: the walk resumes at the next bundle + greater than that keyset position. +- **AC-UI-INBOX-HISTORY-001.21:** The read shall be idempotent. Two identical + requests with no intervening data change shall return identical bundle sets in + identical order, and neither shall have a side effect. + +**Concurrency, empty and error** + +- **AC-UI-INBOX-HISTORY-001.22:** The listing shall be a point-in-time projection. + When a bundle is answered, or its turn becomes current again, after a response is + served, the rendered row shall persist until the next read; acting on it is + impossible because the tab offers no mutating action. No history row shall + subscribe to an event stream, enforced by the `ARCH-INBOX-HISTORY-ISOLATION` rule + of AC .5, extended to forbid those same history modules from importing the + event-stream subscription entry points. +- **AC-UI-INBOX-HISTORY-001.31:** Pagination shall be consistent across a walk + without holding a transaction open. Because each page is an independent + point-in-time read (AC .22), a bundle crossing the History boundary mid-walk may + be skipped or repeated. The keyset cursor of AC .20 shall bound this: a walk + shall never return the same bundle twice on one page, and shall never skip a + bundle whose (`created_at`, `pending_id`) exceeds the cursor and which was + present throughout the walk. The tab shall not present the page sequence as a + transactional snapshot. +- **AC-UI-INBOX-HISTORY-001.23:** When two callers read the same bundle + concurrently, both shall receive it, and neither read shall block on the other, + take a write lock, or mutate any row. Where a change lands between them the + results are permitted to differ, and no shared snapshot or cross-request + transaction shall be introduced to hide it. On the client, when two + history reads for different workspaces are in flight, only the response matching + the currently active workspace shall be applied, so a slower earlier response + cannot replace the active workspace's list or reintroduce another workspace's + bundles. Mirrors `AC-UI-NEEDS-YOU-INBOX-001.38`. +- **AC-UI-INBOX-HISTORY-001.24:** When the workspace holds no history bundles, + the tab shall render an empty state naming what the bucket contains rather than a + bare success message. +- **AC-UI-INBOX-HISTORY-001.25:** When a read names a workspace outside the + caller's visible workspace set, the server shall reject it with a non-disclosing + not-found outcome rather than an empty success, and the tab shall render an error + state rather than the empty state, so a permission boundary is never reported as + "nothing is here". When a read fails for any other reason, the tab shall render + that same error state, shall not state that nothing is waiting, and the History + badge shall render NO count rather than a stale one or a zero. Mirrors + `AC-UI-NEEDS-YOU-INBOX-001.31` and `.21`; neither shall be weakened. +- **AC-UI-INBOX-HISTORY-001.26:** An absent `metadata.status` shall be treated as + pending, retaining the existing legacy meaning. + +**Delivery constraints** + +- **AC-UI-INBOX-HISTORY-001.27:** No schema migration, no data backfill and no + notification shall be introduced. +- **AC-UI-INBOX-HISTORY-001.28:** All copy shall be localized in the five shipped + locales, shall contain no Unicode em dash, and the repository i18n ratchet shall + pass. +- **AC-UI-INBOX-HISTORY-001.29:** An end-to-end specification shall cover + selecting the History tab; observing a `superseded` bundle listed with its asked + time, **the text of the question it asked** (AC .30), and its + step-starts-no-agent label (AC .11); and confirming no answer affordance is + present. The question text is not optional coverage, and the label shall be named + by the AC .11 concept, not "parked". + +## Out of scope + +- **Re-ask.** Cut from Needs-you on 2026-09-11 and cut here too: re-asking + restarts an agent on a question events may have settled. +- **Clearing, bulk-dismissing or expiring history rows.** The tab makes the + population visible; it does not drain it. Draining mutates durable audit evidence + and needs its own decision. +- **Gating the shipped Needs-you sidecar routes.** `PUT` and `DELETE` on + `/api/v1/clarification-inbox/sidecar/:pendingID` keep accepting any durable + bundle identifier whose task the caller can reach, including one now in History. + Gating them would be a write-path change to an operational endpoint, which AC .5 + forbids, and History eligibility is not a stable property of a `pending_id`, so a + gate must first decide the retroactivity question the system design sets out. +- **The failed-task bucket**, the third tab in the revision-4 mockup deck. +- **Parent-question records.** An agent's question to its parent task, excluded + from both tabs per AC .2 on an ownership boundary rather than a staleness one. +- **Archived tasks**, per AC .4. +- **Cross-workspace rows.** Workspace-scoped, like Needs-you. +- **Any change to the current-turn predicate, expiry, bundle visibility rules or + resolution semantics.** Consumed unchanged; they belong to integrations. +- **Retention or archival of the rows this tab reads.** diff --git a/docs/specs/ui/system-design/inbox-history.md b/docs/specs/ui/system-design/inbox-history.md new file mode 100644 index 0000000000..520946e5df --- /dev/null +++ b/docs/specs/ui/system-design/inbox-history.md @@ -0,0 +1,382 @@ +--- +status: draft +system: ui +requirements: + - REQ-UI-INBOX-HISTORY-001 +--- +# Inbox History System Design + +This part carries the sampled input inventory and the code-level evidence behind +[`REQ-UI-INBOX-HISTORY-001`](../requirements/inbox-history.md). The requirement +holds the contract; this document holds the measurements, the citations and the +reasoning the criteria are grounded in, so a reader can check the contract rather +than trust it. One section is load-bearing for the contract rather than merely +explanatory: **Isolation mechanism** names the closed sink set AC .5 requires. + +All figures were sampled from the running instance on 2026-09-14, SQLite +`mode=ro`, rather than assumed. + +## Record shape + +A clarification or permission request is a row in `task_session_messages` with +`type` of `clarification_request` or `permission_request`. The fields this +capability reads are all in `metadata`: `pending_id` (groups a bundle), `status`, +`question` (an object carrying `id`, `title`, `prompt` and `options[]`), +`question_index`, `question_total`, `context` (free prose, observed over 900 +characters), and `agent_disconnected`. Turn identity is the column `turn_id`, +joined to `task_session_turns`. + +Two further shapes are load-bearing for the requirement's AC .3 fixture, because +without them the "no fourth reason arises" test cannot construct its +counterexamples: + +- `metadata.parent_question` (`models.MetaKeyParentQuestion`), the truthy marker on + an autopilot agent's question to its PARENT task rather than to the operator. +- the per-user dismiss/snooze sidecar rows behind `ClarificationSidecarFilter`, + which are keyed by user and not by bundle alone. + +### A permission record is a different shape, and that drives three criteria + +A `permission_request` carries NO `question` object. Its human-readable text is +the message's own `content`; its choices are `metadata.options[]`, whose entries +are `{kind, name, option_id}` and carry a `name`, not a `label`; it also holds +`action_type`, `action_details`, `request_id` and `tool_call_id`. It has no +`title`, no `prompt`, no `question_index` and no `context`. `buildPermissionInteraction` +(`service_interactions.go`) projects exactly that: `Title` from `row.Content`, +`Options` from `permissionOptionsFromMetadata`. + +Measured over the whole table: **52 of 52 permission rows carry no resolvable +question identifier** (neither `metadata.question_id` nor nested +`metadata.question.id`), while **0 of the clarification rows lack one**. 12 of the +52 are non-terminal. Each of the 52 has a distinct `pending_id`, so a permission +request is always a bundle of one. + +Three contract consequences follow, and they are why review round 3 returned +NEEDS RETHINK: + +1. **AC .2 scopes `unreadable` to clarification bundles.** The `unreadable` reason + exists to mirror the shipped `has_missing_question_id` drop, which is itself + hard-scoped to `WHERE m.type = 'clarification_request'`. Applied + type-agnostically it would match every permission row, so a live, current-turn, + newest (`rn = 1`), non-terminal-session permission request, one the operator is + genuinely being asked to answer right now, would be listed in an audit tab that + states no answer is awaited. It would also break AC .3: the union subtracts the + live permission-type bundles Needs-you excludes, while `unreadable` would + simultaneously admit them to History. +2. **AC .3's fixture needs a live current-turn permission request.** That row is + the one that fails if `unreadable` is ever widened back past clarifications. + The ordinary live answerable clarification bundle in the same fixture is the row + that fails if the union's subtraction term is ever restated as the complement of + AC .2 (see "Why AC .3's union is not stated as a complement" below). Both rows + are required; either alone leaves half the algebra unpinned. +3. **AC .30 needs a permission clause.** With permissions still reaching History + via `superseded` and `session_ended`, and AC .12 requiring them labelled, a + criterion mandating `title`, `prompt`, `options[].label` and `context` has no + valid projection for them. AC .30 therefore names `content` and `options[].name` + for a permission bundle and omits the clarification-only fields, rather than + rendering them empty, which AC .13's "say so rather than render blank" would + otherwise trigger on every permission row. + +## There is not one operational read; there are two + +This is the evidence behind the requirement's AC .2 naming three concrete +exclusion reasons instead of the phrase "excluded by the operational predicate". + +- `ListPendingInteractions` (`pending_interactions.go`) covers BOTH + `clarification_request` and `permission_request`, composing `currentTurnAuthority` + with `nonTerminalSessionPredicate` (lines 64-70). It backs the pending-action + projection. Its `ranked_permissions` CTE keeps only `rn = 1` per session within + the current turn, which is the second clause of AC .2's `superseded`. +- The Needs-you tab renders something narrower: + `ListUnresolvedClarificationBundles` (`inbox_handlers.go`). Its query + hard-filters `WHERE m.type = 'clarification_request'` and additionally drops any + bundle whose question identifier cannot be resolved + (`clarification_bundle_query.go`, the flat `metadata.question_id` falling back to + nested `metadata.question.id`, aggregated with `MAX(...)` so the drop is + bundle-granular). Both filters are independent of turn currency. + +**Every filter each read applies, and where it lands in the contract.** The +requirement's AC .2 and AC .3 were written against this enumeration; a filter +missing from it is a hole in the contract. + +| Filter | Read | Contract home | +|---|---|---| +| current turn (`currentTurnAuthority`) | both | AC .2 `superseded` | +| non-terminal session | both | AC .2 `session_ended` | +| unresolvable question id (bundle-granular, `MAX(...)`) | Needs-you | AC .2 `unreadable`, clarifications only | +| `type = 'clarification_request'` | Needs-you | AC .3, live-and-excluded | +| `parent_question` truthy | Needs-you | AC .2 eligibility, AC .3 | +| per-user dismiss/snooze sidecar | Needs-you | AC .3, live-and-excluded | +| newest permission per turn only (`ranked_permissions`, `rn = 1`) | projection | AC .2 `superseded`, second clause | + +Three consequences the contract depends on. A permission request never appears in +the Needs-you tab at any turn. An unresolvable-question-id bundle is dropped from +Needs-you even when its turn is current, which is why AC .13's divergence is real. +And a `parent_question` record is excluded on an OWNERSHIP boundary, not a +staleness one: it is a question to a parent agent, so History excludes it too +rather than surfacing agent-to-agent traffic the operator was never asked to +answer. The sibling requirement already encodes this at `needs-you-inbox.md:38`. + +**Why AC .3's union is not stated as a complement.** Writing it as "the +non-terminal unarchived bundles less those AC .2 classifies as live" is +degenerate: that expression IS AC .2's definition of History, so the formula would +assert `Union(History, Needs-you) = History` and, combined with AC .3's own +disjointness clause, force Needs-you to be empty. The subtraction term must name +only the live bundles Needs-you ITSELF excludes. + +**Why `superseded` resolves on turn authority alone.** `currentTurnAuthority` +(`turn_authority.go`) is defined purely from turn lifecycle and ordering, with no +session-state term, matching the ADR. Composed with `nonTerminalSessionPredicate` +(as the two shipped reads compose it) a terminal session has NO current turn, so +every row in it would read `superseded`; since AC .2 evaluates `superseded` first, +`session_ended` would become unreachable and AC .10's omission test unwritable. + +`nonTerminalSessionPredicate` (`message_clarification_response.go`) excludes on +SESSION state (`COMPLETED`, `FAILED`, `CANCELLED`), a different axis from turn +supersession. A row under an abandoned session therefore has no superseding turn, +which is why AC .10 scopes the superseding-turn field to the `superseded` reason. + +## Row, bundle and the unit the criteria use + +The requirement fixes three terms because review round 3 found them conflated: a +*row* is a `task_session_messages` record, a *bundle* is the rows sharing a +`pending_id`, and a *bundle row* is the rendered list item. + +Eligibility is a BUNDLE property, not a row property. The shipped sibling settles +this by construction: `clarificationBundleTableExpr` computes `has_pending` as +`MAX(CASE WHEN ... THEN 1 ELSE 0 END)` over the bundle's rows, and +`buildInboxBundleViews` then hydrates the bundle through `FindMessagesByPendingIDs`, +which returns EVERY message for that `pending_id` including already-answered ones. +So a partially answered bundle is listed whole and rendered whole. AC .2 and AC .30 +adopt that behavior rather than inventing a row-level filter that would drop a +terminal question out of the middle of a bundle the operator is reading. + +## Terminal status + +Already decided in code by `InteractionStatus.IsTerminal()` (`interactions.go`): +terminal is any status other than absent or `pending`. This capability consumes +that helper and does not restate the set. The model also carries `cancelled`, +which the histogram below does not observe in this instance but which +`IsTerminal()` treats as terminal. `InteractionStatusFromMetadata` normalizes an +absent or empty value to pending, which is AC .26. + +## Status values in use + +`clarification_request`: `answered` 477, `pending` 359, `expired` 25, `rejected` +14. `permission_request`: `approved` 38, absent 12, `expired` 2. Absent status +retains its existing legacy meaning of pending. + +## Population and distribution + +Non-terminal status, task not archived, turn no longer the session's current +conversational turn. Archived tasks hold 302 of the 371 non-terminal rows +instance-wide and are excluded by AC .4. + +Two samples, two days apart, because the population is live and moving: + +| Bucket | Rows | Bundles | Tasks | Sample | +|---|---|---|---|---| +| Step starts no agent | 46 | 41 | 13 | 2026-09-12 | +| Step starts an agent | 18 | 16 | 7 | 2026-09-12 | +| **Total** | **64** | **57** | **20** | 2026-09-12 | +| Step starts no agent | 46 | 41 | 13 | 2026-09-14 | +| Step starts an agent | 22 | 18 | 8 | 2026-09-14 | +| **Total** | **68** | **59** | **21** | 2026-09-14 | + +An earlier revision of this document reported the first sample's task total as 19. +That was an arithmetic error, not a measurement: the two buckets are disjoint, +because AC .11 classifies by the task's single current step, so the column must +sum. It is 20. + +**The 46 is the load-bearing figure and it is the one that reproduces.** The +no-agent bucket is identical across both samples while the agent-started bucket +grew, which is what a capability justified by "questions sitting where nothing will +re-raise them" should predict. Implementation must cite 46, not a total. + +## What "starts no agent" can and cannot mean + +The split is grounded in real config: a step whose `events.on_enter` holds no +`auto_start_agent` entry. Verified per step on this card's own workflow: `PR Review` +and `Done` are `{}`; `Spec`, `Build`, `Testing`, `Review`, `Create PR` and +`PR Fixup` all auto-start. The empty-`on_enter` case is therefore load-bearing for +the 46, which is why AC .11 states it explicitly. + +AC .11 no longer speaks of a "disabled" entry. The persisted shape is +`OnEnterAction{ Type OnEnterActionType; Config map[string]interface{} }` +(`workflow/models/models.go`) and `HasOnEnterAction` is a pure `Type` equality +check that never inspects `Config`. There is no enabled/disabled field on the +action, on `StepEvents.OnEnter`, or in the loader's known-type map +(`config/workflows/loader.go`). The only shipped "prevent auto start" concept, +`PreventAutoStartAgentOnOpen`, is a global per-user setting applied at session +launch and is not readable per step. So no disabled entry exists to discount, and +an entry whose type string the loader does not recognize simply is not +`auto_start_agent`. Presence is the whole test. + +The `clarification` package has no workflow dependency today: `Handlers` carries +store, hub, messageCreator, repo, eventBus, resolver, logger, `inboxTasks`, +`inboxBundles` and `now`. AC .11 therefore adds a new clarification-to-workflow +read. That is a build cost, not a contract gap. + +## Shipped primitives reused, all confirmed present + +- `IconShieldQuestion` and `STYLE_PERMISSION = "text-amber-500"` + (`apps/web/lib/ui/state-icons.tsx`). A `PENDING_PERMISSION_ICON` pairing already + exists, so AC .12 reuses rather than introduces. +- The `variant="line"` tab strip (`apps/packages/ui/src/tabs.tsx`) and the + `secondary` badge variant (`apps/packages/ui/src/badge.tsx`). +- Bundle ordering `ORDER BY b.created_at ASC, b.pending_id ASC` + (`clarification_bundle_query.go`). +- Limit bounds `defaultInboxLimit = 50` / `maxInboxLimit = 200` + (`inbox_handlers.go`), with `limit=0` REJECTED 400 rather than clamped + (`TestHttpListInbox_ZeroLimit_RejectedNotClamped`). +- `encodeInboxCursor` / `decodeInboxCursor`, the opaque `(created_at, pending_id)` + codec AC .20 adopts unchanged. +- The `/hidden` sibling endpoint (`httpListInboxHidden`) is the closest precedent + for the new read: same bundle view plus a reason field, page `count` vs workspace + `total`, cursor pagination. +- The shipped stale-response guard AC .23 mirrors: `generationByWorkspaceId` and + `appliedGeneration` in `apps/web/lib/state/slices/needs-you-inbox/types.ts`. +- The whole `/api/v1/clarification-inbox` route group is already gated on + `needsYouInboxEnabled` (`handlers.go`). Riding that flag is the path of least + resistance; the requirement mandates neither, deliberately leaving it to Build. + +### Cursor uniqueness, stated correctly + +`pending_id` is `uuid.New().String()` on the clarification path +(`clarification/store.go`), but a PERMISSION `pending_id` is a composite of the +form `--`. It is therefore not a UUID across the +whole population History reads. Uniqueness still holds (52 distinct values over 52 +permission rows, and the composite embeds a nanosecond timestamp), so +`(created_at, pending_id)` remains a total order over bundles and AC .20 needs no +further bundle-level tiebreak. The earlier "it is a UUID" justification was simply +narrower than the population the contract now covers. + +## Why AC .19 needs a third key + +`orderInboxMessages` (`inbox_handlers.go`) sorts by normalized `question_index` +then `questionIDFromMetadata`, using `sort.SliceStable`. For the bundles AC .13 +uniquely admits, `questionIDFromMetadata` returns `""` for every unresolvable +question, so two such questions with absent or equal indexes tie on BOTH keys and +the result falls back to hydration order, which is not a named column. AC .21's +"identical order" would then rest on something the contract never named. AC .19 +adds the message `id` as a third key. Needs-you never lists such a bundle, so the +two tabs still never order a shared bundle differently, and AC .19's "NOT +`orderBundleMessages`" distinction stands: that function tie-breaks by +`created_at` then `id` for MCP resolution and does not clamp a negative index. + +## Isolation mechanism + +**This section is normative by reference: AC .5 requires the rule to scan exactly +the set enumerated here.** + +AC .5, .17 and .22 are enforced by a new architecture-lint rule, +`ARCH-INBOX-HISTORY-ISOLATION`, per `docs/architecture-lint.md`. Rules are +implemented under `scripts/architecture_lint/rules` and their grandfathered +finding sets live under `config/architecture-lint/.json` in the shape +`{"version": 1, "rule": "ARCH-...", "entries": [...]}`. This rule ships with an +EMPTY `entries` list, so any violation fails; baselines can only shrink. + +**Why a source-text scan rather than an import scan.** Every shipped rule is a +per-file scanner: a `Rule` carries an `applies_to(path)` predicate and a +`scan(path, source)` function. Two shapes exist in-tree. `task_office_import.py` +and `runtime_import.py` scan IMPORTS, which in Go are package-granular. +`frontend_root_state_cast.py` scans SOURCE TEXT within one named file. AC .5 needs +the second shape, because the history read must reuse the package-private +`currentTurnAuthority` and `nonTerminalSessionPredicate` and therefore must live in +`apps/backend/internal/task/repository/sqlite`, alongside most of its own sinks. +Within one Go package there is no import statement to forbid, so an import rule +would be structurally vacuous for them, and a cross-package violating fixture would +pass AC .5's positive test while proving nothing about the real risk. That is why +AC .5 requires the fixture to sit in the SAME package as the read. + +**The closed sink set.** Five files, realizing all four sink categories AC .5 +names. A rule scanning fewer does not satisfy AC .5: + +| Path | Sink category | +|---|---| +| `apps/backend/internal/task/repository/sqlite/pending_interactions.go` | pending-action projection (`ListPendingInteractions`) | +| `apps/backend/internal/task/repository/sqlite/message.go` | pending-action projection (`GetPendingActionsBySessionIDs`) | +| `apps/backend/internal/task/repository/sqlite/pending_action_projection.go` | pending-action projection (epoch allocation) | +| `apps/backend/internal/task/repository/sqlite/message_clarification_response.go` | answer and resolution path | +| `apps/backend/internal/task/service/service_events.go` | pending-action event emitters (`addTaskPendingActionEventField`, `addPrimarySessionPendingActionEventField`) | + +No workflow-engine file reads the operational interaction listing directly today, +so the "gates a workflow transition" category is realized transitively through the +projection files above rather than by a file of its own. If a direct reader +appears, it joins this table and the rule's set in the same change. + +**The history-module source set, for AC .17 and AC .22.** Those two criteria +invert the direction: the forbidden SOURCES are the history modules and the +forbidden TARGETS are the sidebar badge state key and the Needs-you count key in +the web store slice (AC .17), and the event-stream subscription entry points +(AC .22). The rule's `applies_to` predicate for that half shall be the History +feature directory Build adds under `apps/web/`, plus the backend file declaring +the history read. Build owns both paths, since neither exists yet, and owns naming +the read's exported entry point, which is the identifier the sink half of the rule +forbids. The rule's own module shall enumerate all of them rather than infer them +from a substring match on "history", so the set is auditable. A source predicate +that matches nothing is the same vacuity AC .5's positive fixture test guards +against on the sink half, and the fixture requirement applies to this half too. + +## Why the shipped sidecar routes are not gated + +AC .15 constrains what the History surface CALLS, not what the Needs-you sidecar +routes ACCEPT, and `## Out of scope` records that those routes are unchanged. + +`httpUpsertInboxSidecar` and `httpDeleteInboxSidecar` (`inbox_handlers.go`) +validate the payload, call `authorizeBundleAccessOrRespond(pendingID)`, which +checks durable bundle existence plus task access, and then write. A History +bundle's `pending_id` is a real durable bundle, so those routes accept one TODAY. +Making them reject one would mean two things the requirement forbids itself. + +First, it is a write-path change to an operational endpoint, which AC .5's +additive-read-path constraint exists to prevent. + +Second, and harder: History eligibility is not a stable property of a `pending_id`. +A live bundle becomes a History bundle the moment its turn is superseded, with no +write to the sidecar row and no event. So an operator who legitimately dismissed a +live bundle at T0 would, at T1, hold a sidecar row that a gate now classifies as an +illegal call. A gate must therefore answer what happens to that already-written +row: leave it (the gate is cosmetic), delete it (the capability mutates operational +state), or fail the read (a read-only tab breaking an operational one). None of +those is decidable inside a read-only audit view, which is why it is deferred +whole rather than half-answered. + +## Rejected alternatives, recorded because they will be proposed again + +The ADR (accepted, amended 2026-08-15 and 2026-08-24) already rejects the two +changes that would close the same visible gap: + +- **Rewriting old rows when a new turn starts**, rejected at ADR line 103: it + *"adds a mutation to every turn-entry path."* +- **Treating every historical pending row as active**, rejected at ADR line 93: it + *"preserves unlimited late answers ... and block[s] workflow progress after newer + turns."* + +The requirement therefore changes neither expiry nor the operational predicate. + +## Correction to the originating card + +The card recorded 109 clarification rows across 25 live tasks plus 6 permission +rows, and stated that 85 sat in `PR Review` (70) and `PR Fixup` (15). Re-measured +on 2026-09-14 those figures do not reproduce: totals are lower and the +concentration has moved. The largest single step is now `Backlog` (18 rows), with +`PR Review` at 17 and `PR Fixup` at 4. The instance-wide histogram does reproduce +and has grown consistently, so this is real movement in the population between +2026-09-12 and 2026-09-14, not a measurement error. The capability is justified by +the 46 rows sitting where nothing will re-raise them, not by the card's 85. +Implementation must not re-copy the card's numbers. + +## Honest limit + +This makes the 46 rows sitting where nothing will re-raise them visible. With +re-ask and clearing both out of scope, it does not resolve them. "The Inbox fixes +the parked-question problem" would be an overclaim. "The Inbox stops them being +invisible" is the claim. + +## A naming collision to avoid + +`parked` already has an unrelated meaning in `state-icons.tsx`: +`parkedOnBackgroundWork` is a session waiting on background work. It is not this +capability's "the owning task sits in a step that starts no agent". These must not +share a symbol, and AC .29 forbids the E2E from naming the label "parked". diff --git a/scripts/architecture_lint/rules/__init__.py b/scripts/architecture_lint/rules/__init__.py index 136d09aec2..bd99473540 100644 --- a/scripts/architecture_lint/rules/__init__.py +++ b/scripts/architecture_lint/rules/__init__.py @@ -1,8 +1,9 @@ """Explicit registry of architecture rules.""" from .frontend_root_state_cast import RULE as FRONTEND_ROOT_STATE_CAST +from .inbox_history_isolation import RULE as INBOX_HISTORY_ISOLATION from .runtime_import import RULE as RUNTIME_IMPORT from .task_office_import import RULE as TASK_OFFICE_IMPORT -RULES = (RUNTIME_IMPORT, TASK_OFFICE_IMPORT, FRONTEND_ROOT_STATE_CAST) +RULES = (RUNTIME_IMPORT, TASK_OFFICE_IMPORT, FRONTEND_ROOT_STATE_CAST, INBOX_HISTORY_ISOLATION) diff --git a/scripts/architecture_lint/rules/inbox_history_isolation.py b/scripts/architecture_lint/rules/inbox_history_isolation.py new file mode 100644 index 0000000000..6dbcd40c2b --- /dev/null +++ b/scripts/architecture_lint/rules/inbox_history_isolation.py @@ -0,0 +1,159 @@ +"""Isolate the Inbox History read path from operational sinks and UI state. + +AC-UI-INBOX-HISTORY-001.5/.17/.22 need two, opposite-direction halves of one +rule: + +- The SINK half (AC .5): a closed set of backend files that must never + reference the history read's exported entry points, because the read + shares a package with most of its own sinks and there is no import + statement to forbid within one Go package. +- The SOURCE half (AC .17/.22): the history modules themselves must never + reference the sidebar/Needs-you count state or an event-stream + subscription entry point, so History can never (even accidentally) feed + the sidebar badge or start reacting to live updates. + +Both sets and both forbidden-target lists are enumerated explicitly rather +than inferred from a substring match on "history" or "count" -- per the +system design's "The rule's own module shall enumerate all of them rather +than infer them from a substring match", and AC .16 requires the History +tab to render its own bundle count, so the bare identifier "count" can never +be a forbidden target. + +Matching is a raw source-text scan (see _find_all below), so it also matches +a forbidden target's name inside a comment, not just live code. Do not name +a forbidden target in an explanatory comment in a history source file -- +there is no way to except it short of removing the comment. +""" + +from pathlib import Path + +from architecture_lint.model import Finding, Rule + + +RULE_ID = "ARCH-INBOX-HISTORY-ISOLATION" + +# AC .5's closed sink set, normative by reference to the system design's +# "Isolation mechanism" > "The closed sink set" table. A rule scanning fewer +# files does not satisfy AC .5. +SINK_FILES = frozenset( + { + "apps/backend/internal/task/repository/sqlite/pending_interactions.go", + "apps/backend/internal/task/repository/sqlite/message.go", + "apps/backend/internal/task/repository/sqlite/pending_action_projection.go", + "apps/backend/internal/task/repository/sqlite/message_clarification_response.go", + "apps/backend/internal/task/service/service_events.go", + } +) + +# The history read's two exported entry points (task/repository/sqlite/ +# clarification_history_query.go): forbidden anywhere in SINK_FILES. +HISTORY_READ_ENTRY_POINTS = ("ListInboxHistoryBundles", "CountInboxHistoryBundles") + +# AC .17/.22's history-module SOURCE set: every new module Build added for +# the History read/render path. Prefixes end in "/" and match a directory; +# exact entries match one file. +HISTORY_SOURCE_PREFIXES = ( + "apps/web/lib/state/slices/inbox-history/", + "apps/web/hooks/domains/inbox-history/", + "apps/web/components/inbox-history/", + "apps/web/lib/inbox-history/", +) +HISTORY_SOURCE_FILES = frozenset( + { + "apps/web/lib/api/domains/inbox-history-api.ts", + "apps/web/lib/types/inbox-history.ts", + "apps/backend/internal/task/repository/sqlite/clarification_history_query.go", + } +) + +# AC .17's forbidden targets. NOT the bare identifier "count": AC .16 +# requires History to render its own bundle count, so that target could +# never pass. These are the Needs-you slice's own symbols instead -- the +# sidebar-badge selector, the slice's own workspace-state field access, and +# the slice's write actions (lib/state/slices/needs-you-inbox/ +# needs-you-inbox-slice.ts) -- verified against lib/state/slices/ +# needs-you-inbox/{selectors,types,needs-you-inbox-slice}.ts and +# app-sidebar-primary-nav.tsx:30. +NEEDS_YOU_TARGETS = ( + "selectNeedsYouInboxCount", + "needsYouInbox.byWorkspaceId", + "setNeedsYouInboxPage", + "setNeedsYouInboxError", + "seedNeedsYouInboxBoot", + "beginNeedsYouInboxRead", +) + +# AC .22's forbidden targets. There is no dedicated event-stream +# subscription entry-point symbol -- client.on is a generic method on a +# shared WS client -- so the enumerable thing is the two event-name string +# literals use-needs-you-inbox-controller.ts:114-115 subscribes to. +EVENT_STREAM_TARGETS = ("session.pending_action_changed", "session.state_changed") + +SOURCE_HALF_TARGETS = NEEDS_YOU_TARGETS + EVENT_STREAM_TARGETS + + +def _matches_source_set(path: str) -> bool: + if path in HISTORY_SOURCE_FILES: + return True + return any(path.startswith(prefix) for prefix in HISTORY_SOURCE_PREFIXES) + + +def applies_to(path: str) -> bool: + return path in SINK_FILES or _matches_source_set(path) + + +def _line_number(source: str, index: int) -> int: + return source.count("\n", 0, index) + 1 + + +def _find_all(source: str, needle: str) -> list[int]: + positions: list[int] = [] + start = 0 + while True: + index = source.find(needle, start) + if index < 0: + return positions + positions.append(index) + start = index + 1 + + +def scan(path: str, source: str) -> list[Finding]: + if path in SINK_FILES: + targets = HISTORY_READ_ENTRY_POINTS + message = ( + "references the Inbox History read's exported entry point `{target}`; the " + "additive history read must never be reachable from an operational sink " + "(AC-UI-INBOX-HISTORY-001.5)" + ) + else: + targets = SOURCE_HALF_TARGETS + message = ( + "references `{target}`, forbidden for the Inbox History read/render path: " + "History must never write the sidebar badge or Needs-you count state and must " + "never subscribe to an event stream (AC-UI-INBOX-HISTORY-001.17/.22)" + ) + + findings: list[Finding] = [] + occurrences: dict[str, int] = {} + for target in targets: + for index in _find_all(source, target): + occurrences[target] = occurrences.get(target, 0) + 1 + findings.append( + Finding.create( + RULE_ID, + path, + _line_number(source, index), + {"path": path, "target": target, "occurrence": occurrences[target]}, + f"{path} " + message.format(target=target), + ) + ) + return findings + + +RULE = Rule( + id=RULE_ID, + slug="inbox_history_isolation", + baseline_path=Path("config/architecture-lint/inbox_history_isolation.json"), + applies_to=applies_to, + scan=scan, +) diff --git a/scripts/architecture_lint_tests/support.py b/scripts/architecture_lint_tests/support.py index 34fd2a7aa0..0f0bebecf0 100644 --- a/scripts/architecture_lint_tests/support.py +++ b/scripts/architecture_lint_tests/support.py @@ -18,6 +18,7 @@ from architecture_lint.rules import RULES # noqa: E402 RUNTIME_RULE = "ARCH-RUNTIME-IMPORT" TASK_OFFICE_RULE = "ARCH-TASK-OFFICE-IMPORT" ROOT_STATE_RULE = "ARCH-FRONTEND-ROOT-STATE-CAST" +INBOX_HISTORY_ISOLATION_RULE = "ARCH-INBOX-HISTORY-ISOLATION" RUNTIME_IMPORT = "github.com/kandev/kandev/internal/agent/runtime/lifecycle" OFFICE_IMPORT = "github.com/kandev/kandev/internal/office/models" RULE_FILES = {rule.id: rule.baseline_path.name for rule in RULES} @@ -59,6 +60,7 @@ class ArchitectureFixture(unittest.TestCase): runtime: list[dict[str, object]] | None = None, task_office: list[dict[str, object]] | None = None, root_state: list[dict[str, object]] | None = None, + inbox_history_isolation: list[dict[str, object]] | None = None, ) -> None: entries = {rule.id: [] for rule in RULES} entries.update( @@ -66,6 +68,7 @@ class ArchitectureFixture(unittest.TestCase): RUNTIME_RULE: runtime or [], TASK_OFFICE_RULE: task_office or [], ROOT_STATE_RULE: root_state or [], + INBOX_HISTORY_ISOLATION_RULE: inbox_history_isolation or [], } ) for rule in RULES: diff --git a/scripts/architecture_lint_tests/test_inbox_history_isolation.py b/scripts/architecture_lint_tests/test_inbox_history_isolation.py new file mode 100644 index 0000000000..20ff8b540b --- /dev/null +++ b/scripts/architecture_lint_tests/test_inbox_history_isolation.py @@ -0,0 +1,241 @@ +"""Tests for ARCH-INBOX-HISTORY-ISOLATION.""" + +from support import ArchitectureFixture, INBOX_HISTORY_ISOLATION_RULE + + +class InboxHistoryIsolationSinkTest(ArchitectureFixture): + def test_sink_file_referencing_read_entry_point_is_flagged(self) -> None: + path = "apps/backend/internal/task/repository/sqlite/pending_interactions.go" + self.write( + path, + """ + package sqlite + + func buildPendingInteractionQuery() { + bundles, _ := ListInboxHistoryBundles(nil, nil) + _ = bundles + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assert_diagnostic_location(result, path, 4) + self.assertIn(INBOX_HISTORY_ISOLATION_RULE, result.stdout) + self.assertIn("ListInboxHistoryBundles", result.stdout) + + def test_sink_file_referencing_count_entry_point_is_flagged(self) -> None: + path = "apps/backend/internal/task/service/service_events.go" + self.write( + path, + """ + package service + + func addTaskPendingActionEventField() { + total, _ := CountInboxHistoryBundles(nil, nil) + _ = total + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assertIn("CountInboxHistoryBundles", result.stdout) + + def test_unrelated_backend_file_is_not_scanned(self) -> None: + path = "apps/backend/internal/task/repository/sqlite/clarification_bundle_query.go" + self.write( + path, + """ + package sqlite + + func unrelated() { + bundles, _ := ListInboxHistoryBundles(nil, nil) + _ = bundles + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 0, result.stdout) + + +class InboxHistoryIsolationSourceTest(ArchitectureFixture): + def test_history_hook_referencing_needs_you_selector_is_flagged(self) -> None: + path = "apps/web/hooks/domains/inbox-history/use-inbox-history-controller.ts" + self.write( + path, + """ + import { selectNeedsYouInboxCount } from "@/lib/state/slices/needs-you-inbox/selectors"; + export function useInboxHistoryController() { + return selectNeedsYouInboxCount; + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assert_diagnostic_location(result, path, 1) + self.assertIn(INBOX_HISTORY_ISOLATION_RULE, result.stdout) + self.assertIn("selectNeedsYouInboxCount", result.stdout) + + def test_history_slice_reaching_into_needs_you_state_is_flagged(self) -> None: + path = "apps/web/lib/state/slices/inbox-history/inbox-history-slice.ts" + self.write( + path, + """ + export function bumpCount(state: any) { + return state.needsYouInbox.byWorkspaceId; + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assertIn("needsYouInbox.byWorkspaceId", result.stdout) + + def test_history_slice_calling_needs_you_write_action_is_flagged(self) -> None: + path = "apps/web/lib/state/slices/inbox-history/inbox-history-slice.ts" + self.write( + path, + """ + import { setNeedsYouInboxPage } from "@/lib/state/slices/needs-you-inbox/needs-you-inbox-slice"; + export function bumpPage(state: any) { + return setNeedsYouInboxPage(state); + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assertIn("setNeedsYouInboxPage", result.stdout) + + def test_history_presentation_module_is_scanned(self) -> None: + path = "apps/web/lib/inbox-history/row-presentation.ts" + self.write( + path, + """ + import { selectNeedsYouInboxCount } from "@/lib/state/slices/needs-you-inbox/selectors"; + export const x = selectNeedsYouInboxCount; + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assert_diagnostic_location(result, path, 1) + self.assertIn("selectNeedsYouInboxCount", result.stdout) + + def test_history_types_file_is_scanned(self) -> None: + path = "apps/web/lib/types/inbox-history.ts" + self.write( + path, + """ + export const x = "needsYouInbox.byWorkspaceId"; + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assertIn("needsYouInbox.byWorkspaceId", result.stdout) + + def test_history_component_subscribing_to_pending_action_event_is_flagged(self) -> None: + path = "apps/web/components/inbox-history/inbox-history-list.tsx" + self.write( + path, + """ + export function wire(client: any) { + return client.on("session.pending_action_changed", () => {}); + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assertIn("session.pending_action_changed", result.stdout) + + def test_history_api_file_subscribing_to_state_changed_event_is_flagged(self) -> None: + path = "apps/web/lib/api/domains/inbox-history-api.ts" + self.write( + path, + """ + export function wire(client: any) { + return client.on("session.state_changed", () => {}); + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assertIn("session.state_changed", result.stdout) + + def test_backend_history_read_file_referencing_event_target_is_flagged(self) -> None: + path = "apps/backend/internal/task/repository/sqlite/clarification_history_query.go" + self.write( + path, + """ + package sqlite + + // session.pending_action_changed + func placeholder() {} + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 1) + self.assertIn("session.pending_action_changed", result.stdout) + + def test_history_own_bundle_count_is_not_flagged(self) -> None: + """AC .16 requires History to render its own bundle count, so the + bare identifier "count" must never be a forbidden target -- only the + Needs-you slice's own symbols are.""" + path = "apps/web/components/inbox-history/inbox-history-tab.tsx" + self.write( + path, + """ + export function InboxHistoryTab({ count }: { count: number }) { + return count; + } + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 0, result.stdout) + + def test_unrelated_web_file_is_not_scanned(self) -> None: + path = "apps/web/components/needs-you-inbox/needs-you-inbox-row.tsx" + self.write( + path, + """ + import { selectNeedsYouInboxCount } from "@/lib/state/slices/needs-you-inbox/selectors"; + export const x = selectNeedsYouInboxCount; + """, + ) + self.track_all() + + result = self.run_cli("--all") + + self.assertEqual(result.returncode, 0, result.stdout)