From 37cea5a8c2fff3e4e144e103d8614ce8fea07975 Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 16 Sep 2026 06:14:56 +0800 Subject: [PATCH] feat(web): add a Failed tab to the Needs-you Inbox (#3683) Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio --- apps/backend/internal/backendapp/helpers.go | 4 + apps/backend/internal/db/dialect/dialect.go | 13 + .../internal/db/dialect/dialect_test.go | 9 + apps/backend/internal/failedinbox/handlers.go | 169 ++++++ .../internal/failedinbox/handlers_test.go | 223 ++++++++ apps/backend/internal/failedinbox/reason.go | 63 +++ .../internal/failedinbox/reason_test.go | 78 +++ .../internal/office/testharness/routes.go | 3 + .../office/testharness/routes_test.go | 45 ++ .../internal/task/models/failed_inbox.go | 33 ++ .../repository/sqlite/failed_inbox_query.go | 102 ++++ .../failed_inbox_query_postgres_test.go | 126 +++++ .../sqlite/failed_inbox_query_test.go | 320 +++++++++++ .../needs-you-inbox-page-client.test.tsx | 130 ++++- .../needs-you-inbox-page-client.tsx | 82 ++- .../app-sidebar/app-sidebar-primary-nav.tsx | 1 + .../failed-inbox-empty-state.test.tsx | 36 ++ .../failed-inbox-empty-state.tsx | 27 + .../failed-inbox-error-state.test.tsx | 22 + .../failed-inbox-error-state.tsx | 22 + .../needs-you-inbox/failed-inbox-row.test.tsx | 104 ++++ .../needs-you-inbox/failed-inbox-row.tsx | 96 ++++ .../failed-inbox-tab-panel.test.tsx | 108 ++++ .../failed-inbox-tab-panel.tsx | 56 ++ .../needs-you-inbox/inbox-tab-strip.test.tsx | 95 ++++ .../needs-you-inbox/inbox-tab-strip.tsx | 64 +++ .../needs-you-inbox-empty-state.test.tsx | 34 ++ .../needs-you-inbox-empty-state.tsx | 16 + apps/web/e2e/helpers/api-client.ts | 2 + .../e2e/tests/chat/inbox-failed-tab.spec.ts | 239 ++++++++ .../chat/mobile-inbox-failed-tab.spec.ts | 83 +++ .../use-failed-inbox-controller.test.tsx | 143 +++++ .../use-failed-inbox-controller.ts | 73 +++ .../lib/api/domains/failed-inbox-api.test.ts | 47 ++ apps/web/lib/api/domains/failed-inbox-api.ts | 16 + apps/web/lib/failed-inbox/inbox-tab.test.ts | 45 ++ apps/web/lib/failed-inbox/inbox-tab.ts | 26 + .../lib/failed-inbox/row-presentation.test.ts | 48 ++ apps/web/lib/failed-inbox/row-presentation.ts | 24 + apps/web/lib/state/app-state-extra-actions.ts | 4 +- apps/web/lib/state/app-state-types.ts | 5 + apps/web/lib/state/default-state.test.ts | 13 + apps/web/lib/state/default-state.ts | 10 + .../failed-inbox/failed-inbox-slice.test.ts | 266 +++++++++ .../slices/failed-inbox/failed-inbox-slice.ts | 118 ++++ .../slices/failed-inbox/selectors.test.ts | 76 +++ .../state/slices/failed-inbox/selectors.ts | 48 ++ .../lib/state/slices/failed-inbox/types.ts | 50 ++ apps/web/lib/state/slices/index.ts | 8 + apps/web/lib/state/slices/workspace/types.ts | 8 + .../state/slices/workspace/workspace-slice.ts | 3 +- apps/web/lib/state/store.ts | 2 + .../lib/threads/thread-session-status.test.ts | 19 +- apps/web/lib/threads/thread-session-status.ts | 9 + apps/web/lib/types/failed-inbox.ts | 21 + apps/web/lib/ws/handlers/workspaces.test.ts | 69 ++- apps/web/lib/ws/handlers/workspaces.ts | 10 +- apps/web/src/locales/en/failedInbox.json | 18 + apps/web/src/locales/en/needsYouInbox.json | 6 +- apps/web/src/locales/pseudo/failedInbox.json | 18 + .../web/src/locales/pseudo/needsYouInbox.json | 6 +- apps/web/src/locales/pt-pt/failedInbox.json | 18 + apps/web/src/locales/pt-pt/needsYouInbox.json | 6 +- apps/web/src/locales/zh-cn/failedInbox.json | 18 + apps/web/src/locales/zh-cn/needsYouInbox.json | 6 +- apps/web/src/locales/zh-hk/failedInbox.json | 18 + apps/web/src/locales/zh-hk/needsYouInbox.json | 6 +- apps/web/src/locales/zh-tw/failedInbox.json | 18 + apps/web/src/locales/zh-tw/needsYouInbox.json | 6 +- apps/web/src/office-routes.tsx | 12 +- .../office-routes.workspace-revision.test.tsx | 92 +++ apps/web/src/settings-routes.tsx | 21 +- ...ettings-routes.workspace-revision.test.tsx | 105 ++++ docs/plans/needs-you-inbox/plan.md | 39 +- .../task-03-inbox-failed-tab.md | 121 ++++ docs/public/configuration.md | 2 +- docs/specs/ui/requirements/inbox-failed.md | 322 +++++++++++ .../system-design/inbox-failed-bucket-01.md | 526 ++++++++++++++++++ 78 files changed, 4895 insertions(+), 55 deletions(-) create mode 100644 apps/backend/internal/failedinbox/handlers.go create mode 100644 apps/backend/internal/failedinbox/handlers_test.go create mode 100644 apps/backend/internal/failedinbox/reason.go create mode 100644 apps/backend/internal/failedinbox/reason_test.go create mode 100644 apps/backend/internal/task/models/failed_inbox.go create mode 100644 apps/backend/internal/task/repository/sqlite/failed_inbox_query.go create mode 100644 apps/backend/internal/task/repository/sqlite/failed_inbox_query_postgres_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/failed_inbox_query_test.go create mode 100644 apps/web/components/needs-you-inbox/failed-inbox-empty-state.test.tsx create mode 100644 apps/web/components/needs-you-inbox/failed-inbox-empty-state.tsx create mode 100644 apps/web/components/needs-you-inbox/failed-inbox-error-state.test.tsx create mode 100644 apps/web/components/needs-you-inbox/failed-inbox-error-state.tsx create mode 100644 apps/web/components/needs-you-inbox/failed-inbox-row.test.tsx create mode 100644 apps/web/components/needs-you-inbox/failed-inbox-row.tsx create mode 100644 apps/web/components/needs-you-inbox/failed-inbox-tab-panel.test.tsx create mode 100644 apps/web/components/needs-you-inbox/failed-inbox-tab-panel.tsx create mode 100644 apps/web/components/needs-you-inbox/inbox-tab-strip.test.tsx create mode 100644 apps/web/components/needs-you-inbox/inbox-tab-strip.tsx create mode 100644 apps/web/e2e/tests/chat/inbox-failed-tab.spec.ts create mode 100644 apps/web/e2e/tests/chat/mobile-inbox-failed-tab.spec.ts create mode 100644 apps/web/hooks/domains/failed-inbox/use-failed-inbox-controller.test.tsx create mode 100644 apps/web/hooks/domains/failed-inbox/use-failed-inbox-controller.ts create mode 100644 apps/web/lib/api/domains/failed-inbox-api.test.ts create mode 100644 apps/web/lib/api/domains/failed-inbox-api.ts create mode 100644 apps/web/lib/failed-inbox/inbox-tab.test.ts create mode 100644 apps/web/lib/failed-inbox/inbox-tab.ts create mode 100644 apps/web/lib/failed-inbox/row-presentation.test.ts create mode 100644 apps/web/lib/failed-inbox/row-presentation.ts create mode 100644 apps/web/lib/state/slices/failed-inbox/failed-inbox-slice.test.ts create mode 100644 apps/web/lib/state/slices/failed-inbox/failed-inbox-slice.ts create mode 100644 apps/web/lib/state/slices/failed-inbox/selectors.test.ts create mode 100644 apps/web/lib/state/slices/failed-inbox/selectors.ts create mode 100644 apps/web/lib/state/slices/failed-inbox/types.ts create mode 100644 apps/web/lib/types/failed-inbox.ts create mode 100644 apps/web/src/locales/en/failedInbox.json create mode 100644 apps/web/src/locales/pseudo/failedInbox.json create mode 100644 apps/web/src/locales/pt-pt/failedInbox.json create mode 100644 apps/web/src/locales/zh-cn/failedInbox.json create mode 100644 apps/web/src/locales/zh-hk/failedInbox.json create mode 100644 apps/web/src/locales/zh-tw/failedInbox.json create mode 100644 apps/web/src/office-routes.workspace-revision.test.tsx create mode 100644 apps/web/src/settings-routes.workspace-revision.test.tsx create mode 100644 docs/plans/needs-you-inbox/task-03-inbox-failed-tab.md create mode 100644 docs/specs/ui/requirements/inbox-failed.md create mode 100644 docs/specs/ui/system-design/inbox-failed-bucket-01.md diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index b326f5bc62..488338ff41 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -46,6 +46,7 @@ import ( editorhandlers "github.com/kandev/kandev/internal/editors/handlers" "github.com/kandev/kandev/internal/entityrefs" "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/failedinbox" gateways "github.com/kandev/kandev/internal/gateway/websocket" "github.com/kandev/kandev/internal/github" "github.com/kandev/kandev/internal/gitlab" @@ -1427,6 +1428,9 @@ func registerSecondaryRoutes( ) p.log.Debug("Registered Clarification handlers (HTTP)") + failedinbox.RegisterRoutes(p.router, p.taskSvc, p.taskRepo, p.log, p.features.NeedsYouInbox) + p.log.Debug("Registered Failed Inbox handlers (HTTP)") + // Wire the plugin Host interaction write path (ADR 0052) onto the same // orchestrator permission resolution and the same clarification resolver // instance the REST route and the MCP handlers use, so a plugin's response diff --git a/apps/backend/internal/db/dialect/dialect.go b/apps/backend/internal/db/dialect/dialect.go index e49e42a855..d04bd4f31c 100644 --- a/apps/backend/internal/db/dialect/dialect.go +++ b/apps/backend/internal/db/dialect/dialect.go @@ -21,6 +21,19 @@ func AutoIncrementIDColumn(driver string) string { return "id INTEGER PRIMARY KEY AUTOINCREMENT" } +// ByteOrderedText wraps a column or expression reference so ORDER BY compares +// it byte-for-byte on every supported dialect, regardless of a database-level +// locale collation. SQLite's TEXT columns already default to the BINARY +// collation, so this is a no-op there; PostgreSQL's default template +// collation can be locale-aware, so this appends an explicit COLLATE "C" to +// force the same byte order there too. +func ByteOrderedText(driver, expr string) string { + if IsPostgres(driver) { + return expr + ` COLLATE "C"` + } + return expr +} + // BoolToInt converts a boolean to an integer for SQL storage. func BoolToInt(value bool) int { if value { diff --git a/apps/backend/internal/db/dialect/dialect_test.go b/apps/backend/internal/db/dialect/dialect_test.go index ac64cbeb4c..a2b1cde1ac 100644 --- a/apps/backend/internal/db/dialect/dialect_test.go +++ b/apps/backend/internal/db/dialect/dialect_test.go @@ -27,6 +27,15 @@ func TestTimestampType(t *testing.T) { } } +func TestByteOrderedText(t *testing.T) { + if got := ByteOrderedText(SQLite3, "t.id"); got != "t.id" { + t.Errorf("sqlite: expected bare expression, got %q", got) + } + if got := ByteOrderedText(PGX, "t.id"); got != `t.id COLLATE "C"` { + t.Errorf("pgx: expected explicit C collation, got %q", got) + } +} + func TestBoolToInt(t *testing.T) { if BoolToInt(true) != 1 { t.Error("expected 1 for true") diff --git a/apps/backend/internal/failedinbox/handlers.go b/apps/backend/internal/failedinbox/handlers.go new file mode 100644 index 0000000000..1d13a298ff --- /dev/null +++ b/apps/backend/internal/failedinbox/handlers.go @@ -0,0 +1,169 @@ +package failedinbox + +import ( + "context" + "errors" + "net/http" + "strconv" + "strings" + "time" + + "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/authz" + "github.com/kandev/kandev/internal/common/logger" + taskmodels "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository/repoerrors" + "github.com/kandev/kandev/internal/task/service" + "go.uber.org/zap" +) + +const ( + defaultFailedInboxLimit = 50 + maxFailedInboxLimit = 200 + + errFailedInboxInternal = "failed to read failed inbox" +) + +// workspaceAuthorizer is the workspace-scope check the Failed tab's read +// endpoint needs (design "Security"): the same rule AuthorizeWorkspaceScope +// already applies everywhere else. +type workspaceAuthorizer interface { + AuthorizeWorkspaceScope(ctx context.Context, workspaceID string, scope authz.Scope) error +} + +// failedTaskStore is the bounded read surface the Failed tab needs from the +// task repository. +type failedTaskStore interface { + ListFailedInboxTasks(ctx context.Context, opts taskmodels.ListFailedInboxOptions) (*taskmodels.FailedInboxPage, error) +} + +// Handlers serves the Inbox Failed tab's HTTP surface. +type Handlers struct { + authz workspaceAuthorizer + store failedTaskStore + logger *logger.Logger +} + +// NewHandlers constructs the Failed-tab handlers. +func NewHandlers(authz workspaceAuthorizer, store failedTaskStore, log *logger.Logger) *Handlers { + return &Handlers{ + authz: authz, + store: store, + logger: log.WithFields(zap.String("component", "failed-inbox-handlers")), + } +} + +// RegisterRoutes registers the Failed tab's HTTP route. enabled gates it +// behind the same Inbox feature flag that already governs the Inbox +// destination (AC-UI-INBOX-FAILED-001.3). +func RegisterRoutes(router *gin.Engine, authz workspaceAuthorizer, store failedTaskStore, log *logger.Logger, enabled bool) { + if !enabled { + return + } + h := NewHandlers(authz, store, log) + router.GET("/api/v1/failed-inbox", h.httpListFailedInbox) +} + +// failedInboxRow is one wire row. task_id, title, workspace_id, origin and +// reason are always present as an empty string rather than absent; +// failure_instant is omitted (never null, never a zero instant) when +// unresolvable -- the wire contract F22 fixes. +type failedInboxRow struct { + TaskID string `json:"task_id"` + Title string `json:"title"` + WorkspaceID string `json:"workspace_id"` + Origin string `json:"origin"` + FailureInstant *string `json:"failure_instant,omitempty"` + Reason string `json:"reason"` +} + +// failedInboxListResponse is GET /api/v1/failed-inbox's response envelope. +type failedInboxListResponse struct { + Rows []failedInboxRow `json:"rows"` + Count int `json:"count"` + Truncated bool `json:"truncated"` +} + +func respondFailedInboxError(c *gin.Context, status int, message string) { + c.JSON(status, gin.H{"error": message}) +} + +// httpListFailedInbox backs GET /api/v1/failed-inbox. +func (h *Handlers) httpListFailedInbox(c *gin.Context) { + ctx := c.Request.Context() + workspaceID := strings.TrimSpace(c.Query("workspace_id")) + if workspaceID == "" { + respondFailedInboxError(c, http.StatusBadRequest, "workspace_id is required") + return + } + limit, err := parseFailedInboxLimit(c.Query("limit")) + if err != nil { + respondFailedInboxError(c, http.StatusBadRequest, err.Error()) + return + } + if err := h.authz.AuthorizeWorkspaceScope(ctx, workspaceID, authz.ScopeWorkspaceRead); err != nil { + h.respondWorkspaceAuthzError(c, err) + return + } + + page, err := h.store.ListFailedInboxTasks(ctx, taskmodels.ListFailedInboxOptions{WorkspaceID: workspaceID, Limit: limit}) + if err != nil { + h.logger.Error("failed to list failed inbox tasks", zap.Error(err)) + respondFailedInboxError(c, http.StatusInternalServerError, errFailedInboxInternal) + return + } + + rows := make([]failedInboxRow, 0, len(page.Rows)) + for _, row := range page.Rows { + rows = append(rows, toFailedInboxRow(row)) + } + c.JSON(http.StatusOK, failedInboxListResponse{Rows: rows, Count: len(rows), Truncated: page.HasMore}) +} + +// toFailedInboxRow projects a store row onto the wire shape, applying +// AC-UI-INBOX-FAILED-001.19a's sanitize-then-truncate reason contract. +func toFailedInboxRow(row taskmodels.FailedInboxTaskRow) failedInboxRow { + out := failedInboxRow{ + TaskID: row.TaskID, + Title: row.Title, + WorkspaceID: row.WorkspaceID, + Origin: row.Origin, + Reason: SanitizeAndTruncateReason(row.Reason), + } + if row.FailureInstant != nil { + instant := row.FailureInstant.UTC().Format(time.RFC3339) + out.FailureInstant = &instant + } + return out +} + +// parseFailedInboxLimit mirrors the Needs-you bucket's limit contract +// (AC-UI-INBOX-FAILED-001.12): absent defaults to 50, non-numeric or <= 0 is +// rejected, and > 200 clamps to 200. +func parseFailedInboxLimit(raw string) (int, error) { + if raw == "" { + return defaultFailedInboxLimit, nil + } + n, err := strconv.Atoi(raw) + if err != nil || n <= 0 { + return 0, errors.New("limit must be a positive integer") + } + if n > maxFailedInboxLimit { + return maxFailedInboxLimit, nil + } + return n, nil +} + +// respondWorkspaceAuthzError maps AuthorizeWorkspaceScope's errors per +// AC-UI-INBOX-FAILED-001.24: an unreachable workspace is a non-disclosing 404. +func (h *Handlers) respondWorkspaceAuthzError(c *gin.Context, err error) { + switch { + case errors.Is(err, repoerrors.ErrWorkspaceNotFound): + respondFailedInboxError(c, http.StatusNotFound, "workspace not found") + case errors.Is(err, service.ErrForbidden): + respondFailedInboxError(c, http.StatusForbidden, "insufficient permissions") + default: + h.logger.Error("failed to authorize failed inbox workspace access", zap.Error(err)) + respondFailedInboxError(c, http.StatusInternalServerError, errFailedInboxInternal) + } +} diff --git a/apps/backend/internal/failedinbox/handlers_test.go b/apps/backend/internal/failedinbox/handlers_test.go new file mode 100644 index 0000000000..b76ee50331 --- /dev/null +++ b/apps/backend/internal/failedinbox/handlers_test.go @@ -0,0 +1,223 @@ +package failedinbox + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + "unicode/utf8" + + "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/authz" + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/repository/repoerrors" + "github.com/kandev/kandev/internal/task/service" + + taskmodels "github.com/kandev/kandev/internal/task/models" +) + +type fakeWorkspaceAuthorizer struct { + err error +} + +func (f *fakeWorkspaceAuthorizer) AuthorizeWorkspaceScope(context.Context, string, authz.Scope) error { + return f.err +} + +type fakeFailedTaskStore struct { + page *taskmodels.FailedInboxPage + err error + // lastOpts records the options of the most recent call, for assertions + // on what the handler passed through. + lastOpts taskmodels.ListFailedInboxOptions +} + +func (f *fakeFailedTaskStore) ListFailedInboxTasks(_ context.Context, opts taskmodels.ListFailedInboxOptions) (*taskmodels.FailedInboxPage, error) { + f.lastOpts = opts + if f.err != nil { + return nil, f.err + } + if f.page != nil { + return f.page, nil + } + return &taskmodels.FailedInboxPage{}, nil +} + +func newFailedInboxTestHandler(authorizer workspaceAuthorizer, store failedTaskStore) *Handlers { + gin.SetMode(gin.TestMode) + return NewHandlers(authorizer, store, logger.Default()) +} + +func runFailedInboxGet(h *Handlers, target string) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodGet, target, nil) + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = req + h.httpListFailedInbox(c) + return rec +} + +func decodeFailedInboxBody(t *testing.T, rec *httptest.ResponseRecorder, out interface{}) { + t.Helper() + if err := json.Unmarshal(rec.Body.Bytes(), out); err != nil { + t.Fatalf("decode response body %q: %v", rec.Body.String(), err) + } +} + +func TestHttpListFailedInbox_MissingWorkspaceIDIs400(t *testing.T) { + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, &fakeFailedTaskStore{}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox") + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +func TestHttpListFailedInbox_NonNumericLimitIs400(t *testing.T) { + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, &fakeFailedTaskStore{}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1&limit=abc") + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +func TestHttpListFailedInbox_NonPositiveLimitIs400(t *testing.T) { + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, &fakeFailedTaskStore{}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1&limit=0") + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +func TestHttpListFailedInbox_AbsentLimitDefaultsTo50(t *testing.T) { + store := &fakeFailedTaskStore{} + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, store) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + if store.lastOpts.Limit != 50 { + t.Errorf("limit = %d, want 50", store.lastOpts.Limit) + } +} + +func TestHttpListFailedInbox_LimitAbove200Clamps(t *testing.T) { + store := &fakeFailedTaskStore{} + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, store) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1&limit=999") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + if store.lastOpts.Limit != 200 { + t.Errorf("limit = %d, want clamped 200", store.lastOpts.Limit) + } +} + +func TestHttpListFailedInbox_UnreachableWorkspaceIs404NonDisclosing(t *testing.T) { + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{err: repoerrors.ErrWorkspaceNotFound}, &fakeFailedTaskStore{}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws-outside") + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", rec.Code) + } +} + +func TestHttpListFailedInbox_ForbiddenMapsTo403(t *testing.T) { + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{err: service.ErrForbidden}, &fakeFailedTaskStore{}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1") + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403", rec.Code) + } +} + +func TestHttpListFailedInbox_StoreErrorIs500(t *testing.T) { + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, &fakeFailedTaskStore{err: errors.New("boom")}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1") + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want 500", rec.Code) + } +} + +func TestHttpListFailedInbox_EmptyPageReturnsEmptyListNotAbsent(t *testing.T) { + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, &fakeFailedTaskStore{page: &taskmodels.FailedInboxPage{}}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1") + var body struct { + Rows []json.RawMessage `json:"rows"` + Count int `json:"count"` + Truncated bool `json:"truncated"` + } + decodeFailedInboxBody(t, rec, &body) + if body.Rows == nil { + t.Error("expected an empty array, got absent/null rows") + } + if body.Count != 0 || body.Truncated { + t.Errorf("expected count=0 truncated=false, got count=%d truncated=%v", body.Count, body.Truncated) + } +} + +func TestHttpListFailedInbox_CountIsRenderedRowSetSize(t *testing.T) { + instant := time.Date(2026, 9, 14, 1, 0, 0, 0, time.UTC) + page := &taskmodels.FailedInboxPage{ + Rows: []taskmodels.FailedInboxTaskRow{ + {TaskID: "t1", Title: "Task one", WorkspaceID: "ws1", Origin: "manual", FailureInstant: &instant, Reason: "boom"}, + {TaskID: "t2", Title: "Task two", WorkspaceID: "ws1", Origin: "", FailureInstant: nil, Reason: ""}, + }, + HasMore: true, + } + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, &fakeFailedTaskStore{page: page}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1") + + var body struct { + Rows []struct { + TaskID string `json:"task_id"` + Title string `json:"title"` + WorkspaceID string `json:"workspace_id"` + Origin string `json:"origin"` + FailureInstant *string `json:"failure_instant"` + Reason string `json:"reason"` + } `json:"rows"` + Count int `json:"count"` + Truncated bool `json:"truncated"` + } + decodeFailedInboxBody(t, rec, &body) + if body.Count != 2 { + t.Errorf("count = %d, want 2 (the rendered row set's own size)", body.Count) + } + if !body.Truncated { + t.Error("expected truncated=true") + } + if body.Rows[0].TaskID != "t1" || body.Rows[0].FailureInstant == nil || *body.Rows[0].FailureInstant != "2026-09-14T01:00:00Z" { + t.Errorf("row 0 = %+v", body.Rows[0]) + } + if body.Rows[1].FailureInstant != nil { + t.Errorf("row 1 failure_instant should be omitted (nil), got %v", *body.Rows[1].FailureInstant) + } + if body.Rows[1].Origin != "" || body.Rows[1].Reason != "" { + t.Errorf("row 1 = %+v, want empty-string origin and reason rather than absent", body.Rows[1]) + } +} + +func TestHttpListFailedInbox_ReasonSanitizedAndTruncated(t *testing.T) { + longReason := "" + for i := 0; i < 600; i++ { + longReason += "a" + } + page := &taskmodels.FailedInboxPage{ + Rows: []taskmodels.FailedInboxTaskRow{ + {TaskID: "t1", Title: "Task one", WorkspaceID: "ws1", Reason: longReason}, + }, + } + h := newFailedInboxTestHandler(&fakeWorkspaceAuthorizer{}, &fakeFailedTaskStore{page: page}) + rec := runFailedInboxGet(h, "/api/v1/failed-inbox?workspace_id=ws1") + + var body struct { + Rows []struct { + Reason string `json:"reason"` + } `json:"rows"` + } + decodeFailedInboxBody(t, rec, &body) + if got := utf8.RuneCountInString(body.Rows[0].Reason); got != 512 { + t.Errorf("expected the reason truncated to 512 code points, got length %d", got) + } +} diff --git a/apps/backend/internal/failedinbox/reason.go b/apps/backend/internal/failedinbox/reason.go new file mode 100644 index 0000000000..d619c95f68 --- /dev/null +++ b/apps/backend/internal/failedinbox/reason.go @@ -0,0 +1,63 @@ +// Package failedinbox implements the Inbox Failed tab: a workspace-scoped, +// bounded, read-only projection of failed tasks +// (docs/specs/ui/system-design/inbox-failed-bucket-01.md). It is a separate +// endpoint from the clarification bundle read -- a different table, a +// different question, never merged. +package failedinbox + +import ( + "strings" + "unicode/utf8" +) + +// maxReasonCodePoints is AC-UI-INBOX-FAILED-001.19a's bound: enough to stop a +// 4096-byte stored message multiplied by a 200-row page from becoming an +// 800KB payload, while still showing the actual triage content. +const maxReasonCodePoints = 512 + +// SanitizeAndTruncateReason produces the failure reason exactly as +// AC-UI-INBOX-FAILED-001.19a requires, in the order the AC states -- the +// steps are not commutative. FIRST, every byte that is not part of a +// well-formed UTF-8 sequence is replaced with the Unicode replacement +// character, one per invalid byte, so the result is valid UTF-8 for every +// input including one that is not. SECOND, the result is truncated to at +// most 512 Unicode code points, never bytes, never splitting one. +func SanitizeAndTruncateReason(raw string) string { + return truncateToCodePoints(sanitizeToValidUTF8(raw), maxReasonCodePoints) +} + +// sanitizeToValidUTF8 replaces each invalid byte with the replacement +// character individually (unlike strings.ToValidUTF8, which collapses a run +// of invalid bytes into a single replacement), matching the AC's literal +// "each byte" wording. +func sanitizeToValidUTF8(raw string) string { + if raw == "" { + return "" + } + var b strings.Builder + b.Grow(len(raw)) + for i := 0; i < len(raw); { + r, size := utf8.DecodeRuneInString(raw[i:]) + // utf8.DecodeRuneInString already advances by exactly one byte on a + // malformed sequence (size <= 1 whenever r == RuneError for + // non-empty input), so writing r as-is replaces each invalid byte + // individually rather than collapsing a run into one replacement. + b.WriteRune(r) + i += size + } + return b.String() +} + +// truncateToCodePoints cuts s at the byte offset where the (max+1)'th code +// point would start, so the result carries exactly min(max, count) code +// points and never splits one. +func truncateToCodePoints(s string, max int) string { + count := 0 + for i := range s { + if count == max { + return s[:i] + } + count++ + } + return s +} diff --git a/apps/backend/internal/failedinbox/reason_test.go b/apps/backend/internal/failedinbox/reason_test.go new file mode 100644 index 0000000000..9574c22b11 --- /dev/null +++ b/apps/backend/internal/failedinbox/reason_test.go @@ -0,0 +1,78 @@ +package failedinbox + +import ( + "strings" + "testing" + "unicode/utf8" +) + +func TestSanitizeAndTruncateReason_ValidShortReasonUnchanged(t *testing.T) { + if got := SanitizeAndTruncateReason("connection refused"); got != "connection refused" { + t.Errorf("got %q", got) + } +} + +func TestSanitizeAndTruncateReason_Empty(t *testing.T) { + if got := SanitizeAndTruncateReason(""); got != "" { + t.Errorf("got %q", got) + } +} + +// AC-UI-INBOX-FAILED-001.19a: invalid bytes become the replacement +// character, one per invalid byte, never a broken partial character. +func TestSanitizeAndTruncateReason_InvalidUTF8Replaced(t *testing.T) { + raw := "before\xff\xfeafter" + got := SanitizeAndTruncateReason(raw) + if !utf8.ValidString(got) { + t.Fatalf("result is not valid UTF-8: %q", got) + } + want := "before��after" + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +// AC-UI-INBOX-FAILED-001.19a: truncation is to at most 512 Unicode CODE +// POINTS, never bytes, and never splits a code point -- a multi-byte script +// must still yield exactly 512 runes and no partial trailing character. +func TestSanitizeAndTruncateReason_TruncatesToCodePointsNotBytes(t *testing.T) { + // Each "日" is 3 bytes in UTF-8; 600 of them is 1800 bytes but only 600 + // code points. + raw := strings.Repeat("日", 600) + got := SanitizeAndTruncateReason(raw) + if !utf8.ValidString(got) { + t.Fatalf("result is not valid UTF-8") + } + gotCount := utf8.RuneCountInString(got) + if gotCount != 512 { + t.Fatalf("expected exactly 512 code points, got %d", gotCount) + } + want := strings.Repeat("日", 512) + if got != want { + t.Errorf("got %q, want first 512 repeats of 日", got) + } +} + +func TestSanitizeAndTruncateReason_ExactBoundaryUnchanged(t *testing.T) { + raw := strings.Repeat("a", 512) + got := SanitizeAndTruncateReason(raw) + if got != raw { + t.Errorf("expected a 512-code-point reason to be returned unchanged") + } +} + +func TestSanitizeAndTruncateReason_SanitizesBeforeTruncating(t *testing.T) { + // A long run of invalid bytes followed by valid content: sanitizing + // FIRST means the invalid bytes count toward the 512-code-point budget + // as individual replacement characters, not as raw bytes that would + // otherwise never reach the truncation step in a byte-sliced + // implementation. + raw := strings.Repeat("\xff", 600) + got := SanitizeAndTruncateReason(raw) + if !utf8.ValidString(got) { + t.Fatalf("result is not valid UTF-8") + } + if utf8.RuneCountInString(got) != 512 { + t.Fatalf("expected exactly 512 code points, got %d", utf8.RuneCountInString(got)) + } +} diff --git a/apps/backend/internal/office/testharness/routes.go b/apps/backend/internal/office/testharness/routes.go index 523bcb733a..2688a57257 100644 --- a/apps/backend/internal/office/testharness/routes.go +++ b/apps/backend/internal/office/testharness/routes.go @@ -308,6 +308,7 @@ type seedTaskSessionRequest struct { AgentProfileID string `json:"agent_profile_id,omitempty"` StartedAt *string `json:"started_at,omitempty"` CompletedAt *string `json:"completed_at,omitempty"` + ErrorMessage string `json:"error_message,omitempty"` CommandCount int `json:"command_count,omitempty"` Metadata map[string]interface{} `json:"metadata,omitempty"` } @@ -380,6 +381,7 @@ func updateSeededSession( } existing.State = session.State existing.CompletedAt = session.CompletedAt + existing.ErrorMessage = session.ErrorMessage existing.UpdatedAt = time.Now().UTC() if existing.Metadata == nil { existing.Metadata = map[string]interface{}{} @@ -540,6 +542,7 @@ func buildSeededSession(req *seedTaskSessionRequest) (*models.TaskSession, error RepositoryID: req.RepositoryID, AgentProfileID: req.AgentProfileID, State: models.TaskSessionState(req.State), + ErrorMessage: req.ErrorMessage, Metadata: metadata, StartedAt: startedAt, CompletedAt: completedAt, diff --git a/apps/backend/internal/office/testharness/routes_test.go b/apps/backend/internal/office/testharness/routes_test.go index 7e7504f1b8..ed358aca9e 100644 --- a/apps/backend/internal/office/testharness/routes_test.go +++ b/apps/backend/internal/office/testharness/routes_test.go @@ -352,6 +352,51 @@ func TestSeedTaskSessionUsesRequestedSessionID(t *testing.T) { } } +func TestSeedTaskSessionUpdateAppliesErrorMessage(t *testing.T) { + repo, sqlxDB := newTestRepo(t) + taskID := uuid.New().String() + sessionID := uuid.New().String() + seedTask(t, sqlxDB, taskID) + r := newRouter(t, repo, nil) + + body := mustJSON(t, map[string]interface{}{ + "task_id": taskID, + "session_id": sessionID, + "state": "RUNNING", + }) + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/v1/_test/task-sessions", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String()) + } + + completedAt := time.Now().UTC().Format(time.RFC3339) + updateBody := mustJSON(t, map[string]interface{}{ + "task_id": taskID, + "session_id": sessionID, + "state": "FAILED", + "completed_at": completedAt, + "error_message": "boom", + }) + w = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodPost, "/api/v1/_test/task-sessions", bytes.NewReader(updateBody)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d body=%s", w.Code, w.Body.String()) + } + + session, err := repo.GetTaskSession(context.Background(), sessionID) + if err != nil { + t.Fatalf("get updated session: %v", err) + } + if session.ErrorMessage != "boom" { + t.Fatalf("error_message = %q, want %q", session.ErrorMessage, "boom") + } +} + func TestSeedTaskSessionUsesRequestedRepositoryID(t *testing.T) { repo, sqlxDB := newTestRepo(t) taskID := uuid.New().String() diff --git a/apps/backend/internal/task/models/failed_inbox.go b/apps/backend/internal/task/models/failed_inbox.go new file mode 100644 index 0000000000..d4b47410d1 --- /dev/null +++ b/apps/backend/internal/task/models/failed_inbox.go @@ -0,0 +1,33 @@ +package models + +import "time" + +// FailedInboxTaskRow is one row of the Inbox Failed-tab projection: a task +// whose own state is the terminal failed state (AC-UI-INBOX-FAILED-001.6), +// joined to the session AC-UI-INBOX-FAILED-001.30 selects for its failure +// instant and reason. FailureInstant is nil when unresolvable (no session, or +// the chosen session records no completion instant); Reason is "" when no +// reason can be resolved, mirroring the wire contract's own omit/empty rule. +type FailedInboxTaskRow struct { + TaskID string + Title string + WorkspaceID string + Origin string + FailureInstant *time.Time + Reason string +} + +// ListFailedInboxOptions bounds a single failed-inbox read +// (AC-UI-INBOX-FAILED-001.5, .12, .13). Limit must be >= 1. +type ListFailedInboxOptions struct { + WorkspaceID string + Limit int +} + +// FailedInboxPage is one bounded page of failed tasks, ordered per +// AC-UI-INBOX-FAILED-001.10/.10a. HasMore mirrors the bundle read's Limit+1 +// probe-row convention (AC-UI-INBOX-FAILED-001.12's truncation signal). +type FailedInboxPage struct { + Rows []FailedInboxTaskRow + HasMore bool +} diff --git a/apps/backend/internal/task/repository/sqlite/failed_inbox_query.go b/apps/backend/internal/task/repository/sqlite/failed_inbox_query.go new file mode 100644 index 0000000000..b0a8efc970 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/failed_inbox_query.go @@ -0,0 +1,102 @@ +package sqlite + +import ( + "context" + "database/sql" + "fmt" + + "github.com/kandev/kandev/internal/db/dialect" + "github.com/kandev/kandev/internal/task/models" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// ListFailedInboxTasks returns the bounded, workspace-scoped page of failed +// tasks for the Inbox Failed tab (docs/specs/ui/system-design/inbox-failed-bucket-01.md, +// "Ordering, paging, and the failure instant" and "Session resolution"). A +// row exists when tasks.state is the terminal failed state and +// tasks.archived_at is null (AC .6, .7); the correlated subquery selects the +// same session AC .30/.30a pick per task, and every ordering key is explicit +// rather than left to either engine's default (AC .10, .10a). +func (r *Repository) ListFailedInboxTasks(ctx context.Context, opts models.ListFailedInboxOptions) (*models.FailedInboxPage, error) { + if opts.Limit < 1 { + return nil, fmt.Errorf("ListFailedInboxTasks: limit must be >= 1, got %d", opts.Limit) + } + + drv := r.ro.DriverName() + query := failedInboxQuery(drv) + rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(query), string(v1.TaskStateFailed), opts.WorkspaceID, opts.Limit+1) + if err != nil { + return nil, err + } + defer func() { _ = rows.Close() }() + + taskRows, err := scanFailedInboxRows(rows) + if err != nil { + return nil, err + } + + page := &models.FailedInboxPage{Rows: taskRows} + if len(taskRows) > opts.Limit { + page.Rows = taskRows[:opts.Limit] + page.HasMore = true + } + return page, nil +} + +// failedInboxSessionOrder is the session-selection order AC .30/.30a specify: +// the primary session first, then most-recently-started, tied broken by the +// lowest session id compared as a byte-ordered string. +func failedInboxSessionOrder(drv string) string { + return fmt.Sprintf("s.is_primary DESC, s.started_at DESC, %s ASC", dialect.ByteOrderedText(drv, "s.id")) +} + +// failedInboxRowOrder is the row order AC .10/.10a specify: unresolvable +// failure instants first, then failure instant descending, then task id +// ascending as a byte-ordered string -- explicit rather than left to either +// engine's default null placement (design#Ordering-paging-and-the-failure-instant). +func failedInboxRowOrder(drv string) string { + return fmt.Sprintf( + "(CASE WHEN cs.completed_at IS NULL THEN 0 ELSE 1 END) ASC, cs.completed_at DESC, %s ASC", + dialect.ByteOrderedText(drv, "t.id"), + ) +} + +func failedInboxQuery(drv string) string { + return fmt.Sprintf(` + SELECT t.id, t.title, t.workspace_id, t.origin, cs.completed_at, cs.error_message + FROM tasks t + LEFT JOIN task_sessions cs ON cs.id = ( + SELECT s.id FROM task_sessions s + WHERE s.task_id = t.id + ORDER BY %s + LIMIT 1 + ) + WHERE t.state = ? AND t.archived_at IS NULL AND t.workspace_id = ? + ORDER BY %s + LIMIT ? + `, failedInboxSessionOrder(drv), failedInboxRowOrder(drv)) +} + +func scanFailedInboxRows(rows *sql.Rows) ([]models.FailedInboxTaskRow, error) { + var out []models.FailedInboxTaskRow + for rows.Next() { + var row models.FailedInboxTaskRow + var origin sql.NullString + var completedAt sql.NullTime + var errorMessage sql.NullString + if err := rows.Scan(&row.TaskID, &row.Title, &row.WorkspaceID, &origin, &completedAt, &errorMessage); err != nil { + return nil, err + } + row.Origin = origin.String + if completedAt.Valid { + completedAtCopy := completedAt.Time + row.FailureInstant = &completedAtCopy + } + row.Reason = errorMessage.String + out = append(out, row) + } + if err := rows.Err(); err != nil { + return nil, err + } + return out, nil +} diff --git a/apps/backend/internal/task/repository/sqlite/failed_inbox_query_postgres_test.go b/apps/backend/internal/task/repository/sqlite/failed_inbox_query_postgres_test.go new file mode 100644 index 0000000000..73d900a79d --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/failed_inbox_query_postgres_test.go @@ -0,0 +1,126 @@ +package sqlite + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/testutil" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// TestPostgresListFailedInboxTasksScansDirectCompletedAtColumn proves the +// nullable completed_at column (a direct column reference through the +// correlated-subquery join, not a MIN() aggregate) round-trips through +// PostgreSQL's driver both when present and when the joined session is +// absent -- the SQLite-only run never exercises PostgreSQL's own NULL +// timestamp handling on this path. +func TestPostgresListFailedInboxTasksScansDirectCompletedAtColumn(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("init postgres schema: %v", err) + } + ctx := context.Background() + + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-pg", Name: "ws-pg"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + seedFailedInboxTask(t, repo, "task-pg-resolved", "ws-pg", v1.TaskStateFailed, "") + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-pg-resolved", TaskID: "task-pg-resolved", IsPrimary: true, + StartedAt: mustTime(0), CompletedAt: timePtr(mustTime(10)), ErrorMessage: "boom", + }) + seedFailedInboxTask(t, repo, "task-pg-unresolved", "ws-pg", v1.TaskStateFailed, "") + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws-pg", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 2 { + t.Fatalf("expected 2 rows, got %+v", page.Rows) + } + // Unresolvable sorts first. + if page.Rows[0].TaskID != "task-pg-unresolved" || page.Rows[0].FailureInstant != nil { + t.Fatalf("expected the unresolvable row first with a nil instant, got %+v", page.Rows[0]) + } + if page.Rows[1].TaskID != "task-pg-resolved" || page.Rows[1].FailureInstant == nil { + t.Fatalf("expected the resolved row second with a non-nil instant, got %+v", page.Rows[1]) + } + if !page.Rows[1].FailureInstant.Equal(mustTime(10)) { + t.Fatalf("FailureInstant = %v, want %v", page.Rows[1].FailureInstant, mustTime(10)) + } +} + +// TestPostgresListFailedInboxTasksTieBreaksByteOrdered proves +// dialect.ByteOrderedText's explicit COLLATE "C" actually forces byte order +// on PostgreSQL for the task-id tiebreak (AC-UI-INBOX-FAILED-001.10a). The +// ids deliberately differ only in case: "B" (0x42) sorts before "a" (0x61) +// in byte/C-locale order, but a language-aware default collation typically +// compares letters primarily case-insensitively and would put "a" first -- +// two same-case ids (e.g. "a"/"b") would pass this assertion identically +// whether or not COLLATE "C" is applied, which is exactly the weak fixture +// this replaced. See TestPostgresListFailedInboxTasksSessionTieBreaksByteOrdered +// for the sibling session-id coverage (AC .30a) this file previously lacked +// entirely. +func TestPostgresListFailedInboxTasksTieBreaksByteOrdered(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("init postgres schema: %v", err) + } + ctx := context.Background() + + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-pg-tie", Name: "ws-pg-tie"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + seedFailedInboxTask(t, repo, "task-pg-a", "ws-pg-tie", v1.TaskStateFailed, "") + seedFailedInboxTask(t, repo, "task-pg-B", "ws-pg-tie", v1.TaskStateFailed, "") + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws-pg-tie", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 2 || page.Rows[0].TaskID != "task-pg-B" || page.Rows[1].TaskID != "task-pg-a" { + t.Fatalf("expected byte-ordered [task-pg-B, task-pg-a], got %+v", page.Rows) + } +} + +// TestPostgresListFailedInboxTasksSessionTieBreaksByteOrdered proves the same +// COLLATE "C" behavior for the session-id tiebreak (AC-UI-INBOX-FAILED-001.30a) +// -- the sibling test above only ever exercised the task-id tiebreak, so a +// missing collation on the session-id comparison had zero PostgreSQL coverage. +// Mirrors the SQLite-only +// TestListFailedInboxTasks_SessionResolutionTieBreaksByLowestSessionID fixture +// shape (two non-primary sessions on the same task, tied on started_at), with +// case-differing ids for the same reason as above. +func TestPostgresListFailedInboxTasksSessionTieBreaksByteOrdered(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("init postgres schema: %v", err) + } + ctx := context.Background() + + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-pg-sess-tie", Name: "ws-pg-sess-tie"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + seedFailedInboxTask(t, repo, "task-pg-sess-tie", "ws-pg-sess-tie", v1.TaskStateFailed, "") + tie := mustTime(10) + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-pg-a", TaskID: "task-pg-sess-tie", IsPrimary: false, + StartedAt: tie, CompletedAt: timePtr(mustTime(15)), ErrorMessage: "session a", + }) + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-pg-B", TaskID: "task-pg-sess-tie", IsPrimary: false, + StartedAt: tie, CompletedAt: timePtr(mustTime(20)), ErrorMessage: "session B", + }) + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws-pg-sess-tie", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 || page.Rows[0].Reason != "session B" { + t.Fatalf("expected the tie broken toward the byte-ordered lower session id (sess-pg-B), got %+v", page.Rows) + } +} diff --git a/apps/backend/internal/task/repository/sqlite/failed_inbox_query_test.go b/apps/backend/internal/task/repository/sqlite/failed_inbox_query_test.go new file mode 100644 index 0000000000..75b8ad7bd3 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/failed_inbox_query_test.go @@ -0,0 +1,320 @@ +package sqlite + +import ( + "context" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// seedFailedInboxTask creates a task in the given state/workspace/origin, +// satisfying the failed-task predicate's own fields +// (AC-UI-INBOX-FAILED-001.6, .7, .9a). +func seedFailedInboxTask(t *testing.T, repo *Repository, id, workspaceID string, state v1.TaskState, origin string) { + t.Helper() + if err := repo.CreateTask(context.Background(), &models.Task{ + ID: id, WorkspaceID: workspaceID, Title: "task " + id, State: state, Origin: origin, + }); err != nil { + t.Fatalf("create task %s: %v", id, err) + } +} + +type failedInboxSessionSeed struct { + ID string + TaskID string + IsPrimary bool + StartedAt time.Time + CompletedAt *time.Time + ErrorMessage string +} + +func seedFailedInboxSession(t *testing.T, repo *Repository, s failedInboxSessionSeed) { + t.Helper() + if err := repo.CreateTaskSession(context.Background(), &models.TaskSession{ + ID: s.ID, + TaskID: s.TaskID, + IsPrimary: s.IsPrimary, + StartedAt: s.StartedAt, + CompletedAt: s.CompletedAt, + ErrorMessage: s.ErrorMessage, + State: models.TaskSessionStateFailed, + }); err != nil { + t.Fatalf("create session %s: %v", s.ID, err) + } +} + +func mustTime(offsetSeconds int) time.Time { + return time.Date(2026, 9, 14, 0, 0, 0, 0, time.UTC).Add(time.Duration(offsetSeconds) * time.Second) +} + +func timePtr(tm time.Time) *time.Time { return &tm } + +func TestListFailedInboxTasks_OnlyTaskStateFailedListed(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-failed", "ws1", v1.TaskStateFailed, "") + seedFailedInboxTask(t, repo, "task-running", "ws1", v1.TaskStateInProgress, "") + // A task whose primary session is failed but whose own state is not + // terminal-failed must list zero rows for it (AC .6). + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-running", TaskID: "task-running", IsPrimary: true, + StartedAt: mustTime(0), CompletedAt: timePtr(mustTime(10)), + }) + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 || page.Rows[0].TaskID != "task-failed" { + t.Fatalf("expected exactly [task-failed], got %+v", page.Rows) + } +} + +func TestListFailedInboxTasks_ArchivedTaskNeverListed(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-archived", "ws1", v1.TaskStateFailed, "") + if err := repo.ArchiveTask(ctx, "task-archived"); err != nil { + t.Fatalf("ArchiveTask: %v", err) + } + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 0 { + t.Fatalf("expected no rows for an archived task, got %+v", page.Rows) + } +} + +func TestListFailedInboxTasks_WorkspaceScoped(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-ws1", "ws1", v1.TaskStateFailed, "") + seedFailedInboxTask(t, repo, "task-ws2", "ws2", v1.TaskStateFailed, "") + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 || page.Rows[0].TaskID != "task-ws1" { + t.Fatalf("expected exactly [task-ws1], got %+v", page.Rows) + } +} + +func TestListFailedInboxTasks_SessionResolutionPrefersPrimaryThenMostRecent(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-1", "ws1", v1.TaskStateFailed, "") + // Two non-primary sessions, one primary: primary must win regardless of + // recency (AC .30). + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-old", TaskID: "task-1", IsPrimary: false, + StartedAt: mustTime(0), CompletedAt: timePtr(mustTime(5)), ErrorMessage: "old non-primary", + }) + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-primary", TaskID: "task-1", IsPrimary: true, + StartedAt: mustTime(10), CompletedAt: timePtr(mustTime(15)), ErrorMessage: "primary reason", + }) + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-newer-non-primary", TaskID: "task-1", IsPrimary: false, + StartedAt: mustTime(20), CompletedAt: timePtr(mustTime(25)), ErrorMessage: "newer non-primary", + }) + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 { + t.Fatalf("expected 1 row, got %+v", page.Rows) + } + if page.Rows[0].Reason != "primary reason" { + t.Fatalf("expected primary session's reason, got %q", page.Rows[0].Reason) + } +} + +func TestListFailedInboxTasks_SessionResolutionMostRecentWhenNonePrimary(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-1", "ws1", v1.TaskStateFailed, "") + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-old", TaskID: "task-1", IsPrimary: false, + StartedAt: mustTime(0), CompletedAt: timePtr(mustTime(5)), ErrorMessage: "old", + }) + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-new", TaskID: "task-1", IsPrimary: false, + StartedAt: mustTime(10), CompletedAt: timePtr(mustTime(15)), ErrorMessage: "new", + }) + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 || page.Rows[0].Reason != "new" { + t.Fatalf("expected the most recently started session's reason, got %+v", page.Rows) + } +} + +func TestListFailedInboxTasks_SessionResolutionTieBreaksByLowestSessionID(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-1", "ws1", v1.TaskStateFailed, "") + tie := mustTime(10) + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-b", TaskID: "task-1", IsPrimary: false, + StartedAt: tie, CompletedAt: timePtr(mustTime(15)), ErrorMessage: "session b", + }) + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-a", TaskID: "task-1", IsPrimary: false, + StartedAt: tie, CompletedAt: timePtr(mustTime(20)), ErrorMessage: "session a", + }) + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 || page.Rows[0].Reason != "session a" { + t.Fatalf("expected the tie broken toward the lower session id (sess-a), got %+v", page.Rows) + } +} + +func TestListFailedInboxTasks_NoSessionLeavesBothFieldsUnresolvable(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-no-session", "ws1", v1.TaskStateFailed, "") + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 { + t.Fatalf("a failed task with no session must still be listed, got %+v", page.Rows) + } + row := page.Rows[0] + if row.FailureInstant != nil { + t.Errorf("expected unresolvable failure instant, got %v", row.FailureInstant) + } + if row.Reason != "" { + t.Errorf("expected unresolvable reason, got %q", row.Reason) + } +} + +func TestListFailedInboxTasks_SessionWithNoCompletionInstantIsUnresolvable(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-1", "ws1", v1.TaskStateFailed, "") + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-1", TaskID: "task-1", IsPrimary: true, + StartedAt: mustTime(0), CompletedAt: nil, ErrorMessage: "boom", + }) + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 || page.Rows[0].FailureInstant != nil { + t.Fatalf("expected unresolvable failure instant, got %+v", page.Rows) + } +} + +func TestListFailedInboxTasks_OrderingUnresolvableFirstThenInstantDescThenTaskIDAsc(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + // task-late failed most recently among resolved rows -> first of the + // resolved group. + seedFailedInboxTask(t, repo, "task-late", "ws1", v1.TaskStateFailed, "") + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-late", TaskID: "task-late", IsPrimary: true, + StartedAt: mustTime(0), CompletedAt: timePtr(mustTime(100)), + }) + + seedFailedInboxTask(t, repo, "task-early", "ws1", v1.TaskStateFailed, "") + seedFailedInboxSession(t, repo, failedInboxSessionSeed{ + ID: "sess-early", TaskID: "task-early", IsPrimary: true, + StartedAt: mustTime(0), CompletedAt: timePtr(mustTime(10)), + }) + + // task-unresolvable-b and task-unresolvable-a tie (both unresolvable) and + // must sort by task id ascending, ahead of every resolved row. + seedFailedInboxTask(t, repo, "task-unresolvable-b", "ws1", v1.TaskStateFailed, "") + seedFailedInboxTask(t, repo, "task-unresolvable-a", "ws1", v1.TaskStateFailed, "") + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + got := make([]string, len(page.Rows)) + for i, row := range page.Rows { + got[i] = row.TaskID + } + want := []string{"task-unresolvable-a", "task-unresolvable-b", "task-late", "task-early"} + if len(got) != len(want) { + t.Fatalf("expected %v, got %v", want, got) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("expected order %v, got %v", want, got) + } + } +} + +func TestListFailedInboxTasks_Pagination(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + for i := 0; i < 3; i++ { + seedFailedInboxTask(t, repo, "task-page-"+string(rune('a'+i)), "ws1", v1.TaskStateFailed, "") + } + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 2}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 2 { + t.Fatalf("expected exactly 2 rows (bound), got %d", len(page.Rows)) + } + if !page.HasMore { + t.Errorf("expected HasMore=true with 3 rows over a limit of 2") + } + + full, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 3}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if full.HasMore { + t.Errorf("expected HasMore=false when the bound exactly covers every row") + } +} + +func TestListFailedInboxTasks_RejectsSubOneLimit(t *testing.T) { + repo := newRepoForSessionTests(t) + if _, err := repo.ListFailedInboxTasks(context.Background(), models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 0}); err == nil { + t.Fatal("expected an error for Limit < 1") + } +} + +func TestListFailedInboxTasks_OriginCarriedThrough(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + seedFailedInboxTask(t, repo, "task-1", "ws1", v1.TaskStateFailed, models.TaskOriginAutomationRun) + + page, err := repo.ListFailedInboxTasks(ctx, models.ListFailedInboxOptions{WorkspaceID: "ws1", Limit: 50}) + if err != nil { + t.Fatalf("ListFailedInboxTasks: %v", err) + } + if len(page.Rows) != 1 || page.Rows[0].Origin != models.TaskOriginAutomationRun { + t.Fatalf("expected origin %q, got %+v", models.TaskOriginAutomationRun, page.Rows) + } +} 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 98fbef76a9..003becb3b9 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 @@ -2,15 +2,17 @@ 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 { ClarificationInboxBundle } from "@/lib/types/clarification-inbox"; +import type { FailedInboxRow as FailedInboxRowData } from "@/lib/types/failed-inbox"; const mocks = vi.hoisted(() => ({ bumpRefreshTick: vi.fn(), + useFailedInboxController: vi.fn(), })); const EMPTY_TESTID = "needs-you-inbox-empty"; const ERROR_TESTID = "needs-you-inbox-error"; -let state: { +let needsYouState: { status: "idle" | "loading" | "ready" | "error"; bundles: ClarificationInboxBundle[]; hiddenCount: number; @@ -20,12 +22,20 @@ let state: { }; let workspacesActiveId: string | null; +let failedState: { + rows: FailedInboxRowData[]; + count: number; + truncated: boolean; + status: "idle" | "loading" | "ready" | "error"; +}; + vi.mock("@/components/state-provider", () => ({ // eslint-disable-next-line @typescript-eslint/no-explicit-any useAppStore: (selector: (s: any) => unknown) => selector({ - needsYouInbox: { byWorkspaceId: { w1: state } }, - workspaces: { activeId: workspacesActiveId }, + needsYouInbox: { byWorkspaceId: { w1: needsYouState } }, + failedInbox: { byWorkspaceId: { w1: failedState } }, + workspaces: { activeId: workspacesActiveId, items: [{ id: "w1", name: "Kegmil V2" }] }, bumpNeedsYouInboxRefreshTick: mocks.bumpRefreshTick, }), })); @@ -53,6 +63,14 @@ vi.mock("@/components/needs-you-inbox/needs-you-inbox-hidden-panel", () => ({ ), })); +// The controller owns its own refresh triggers (mount, tab change, workspace +// change, foreground, periodic 60s) and is covered by its own test suite; +// this page-level suite is about which view renders, so the controller is a +// no-op here. +vi.mock("@/hooks/domains/failed-inbox/use-failed-inbox-controller", () => ({ + useFailedInboxController: (...args: unknown[]) => mocks.useFailedInboxController(...args), +})); + import { NeedsYouInboxPageClient } from "./needs-you-inbox-page-client"; function bundle(id: string): ClarificationInboxBundle { @@ -83,13 +101,34 @@ function bundle(id: string): ClarificationInboxBundle { }; } +function failedRow(id: string): FailedInboxRowData { + return { + task_id: id, + title: `Failed task ${id}`, + workspace_id: "w1", + origin: "manual", + failure_instant: "2026-09-14T00:00:00Z", + reason: "boom", + }; +} + +function setLocation(path: string) { + window.history.replaceState({}, "", path); +} + beforeEach(() => { mocks.bumpRefreshTick.mockReset(); - state = { status: "idle", bundles: [], hiddenCount: 0, hasMore: false }; + mocks.useFailedInboxController.mockReset(); + needsYouState = { status: "idle", bundles: [], hiddenCount: 0, hasMore: false }; + failedState = { rows: [], count: 0, truncated: false, status: "idle" }; workspacesActiveId = "w1"; + setLocation("/needs-you-inbox"); }); -afterEach(() => cleanup()); +afterEach(() => { + cleanup(); + setLocation("/needs-you-inbox"); +}); describe("NeedsYouInboxPageClient", () => { it("titles the route Inbox, which the app top bar owns (design-03#D4)", () => { @@ -99,7 +138,7 @@ describe("NeedsYouInboxPageClient", () => { }); it("renders a row per listed bundle", () => { - state = { + needsYouState = { status: "ready", bundles: [bundle("p1"), bundle("p2")], hiddenCount: 0, @@ -111,14 +150,14 @@ describe("NeedsYouInboxPageClient", () => { }); it("renders the empty state when the read succeeded with no rows and no truncation", () => { - state = { status: "ready", bundles: [], hiddenCount: 0, hasMore: false }; + needsYouState = { status: "ready", bundles: [], hiddenCount: 0, hasMore: false }; render(); expect(screen.getByTestId(EMPTY_TESTID)).not.toBeNull(); }); it("renders the error state, not the empty state, when the read failed (AC .21)", () => { - state = { status: "error", bundles: [], hiddenCount: 0, hasMore: false }; + needsYouState = { status: "error", bundles: [], hiddenCount: 0, hasMore: false }; render(); expect(screen.getByTestId(ERROR_TESTID)).not.toBeNull(); @@ -126,7 +165,7 @@ describe("NeedsYouInboxPageClient", () => { }); it("renders the error state for a zero-row truncated page rather than caught up (F42)", () => { - state = { status: "ready", bundles: [], hiddenCount: 0, hasMore: true }; + needsYouState = { status: "ready", bundles: [], hiddenCount: 0, hasMore: true }; render(); expect(screen.getByTestId(ERROR_TESTID)).not.toBeNull(); @@ -134,21 +173,21 @@ describe("NeedsYouInboxPageClient", () => { }); it("shows the truncation notice when the page is bounded (AC .11)", () => { - state = { status: "ready", bundles: [bundle("p1")], hiddenCount: 0, hasMore: true }; + needsYouState = { status: "ready", bundles: [bundle("p1")], hiddenCount: 0, hasMore: true }; render(); expect(screen.getByTestId("needs-you-inbox-truncated")).not.toBeNull(); }); it("does not show the truncation notice when the page is exhaustive", () => { - state = { status: "ready", bundles: [bundle("p1")], hiddenCount: 0, hasMore: false }; + needsYouState = { status: "ready", bundles: [bundle("p1")], hiddenCount: 0, hasMore: false }; render(); expect(screen.queryByTestId("needs-you-inbox-truncated")).toBeNull(); }); it("shows the hidden panel alongside a non-empty list when bundles are hidden", () => { - state = { status: "ready", bundles: [bundle("p1")], hiddenCount: 2, hasMore: false }; + needsYouState = { status: "ready", bundles: [bundle("p1")], hiddenCount: 2, hasMore: false }; render(); expect(screen.getByTestId("stub-hidden-panel").textContent).toBe("2"); @@ -157,12 +196,13 @@ describe("NeedsYouInboxPageClient", () => { describe("NeedsYouInboxPageClient loading vs. settled/refresh states", () => { it("shows a loading indicator on the very first read, not the empty state", () => { - state = { + needsYouState = { status: "loading", bundles: [], hiddenCount: 0, hasMore: false, appliedGeneration: 0, + lastAppliedOk: false, }; render(); @@ -171,7 +211,7 @@ describe("NeedsYouInboxPageClient loading vs. settled/refresh states", () => { }); it("keeps the settled empty state during a background refresh, instead of re-flashing loading", () => { - state = { + needsYouState = { status: "loading", bundles: [], hiddenCount: 0, @@ -186,7 +226,7 @@ describe("NeedsYouInboxPageClient loading vs. settled/refresh states", () => { }); it("keeps the settled list during a background refresh, instead of re-flashing loading", () => { - state = { + needsYouState = { status: "loading", bundles: [bundle("p1")], hiddenCount: 0, @@ -201,7 +241,7 @@ describe("NeedsYouInboxPageClient loading vs. settled/refresh states", () => { }); it("does not flash the error state for a truncated boot payload before the first read resolves", () => { - state = { + needsYouState = { status: "loading", bundles: [], hiddenCount: 0, @@ -222,7 +262,7 @@ describe("NeedsYouInboxPageClient loading vs. settled/refresh states", () => { // apart. Getting this wrong renders a false "Nothing needs your attention" // over a read that is still failing. it("shows the loading indicator, not a false empty state, during a refresh that follows a failed read", () => { - state = { + needsYouState = { status: "loading", bundles: [], hiddenCount: 0, @@ -242,7 +282,7 @@ describe("NeedsYouInboxPageClient loading vs. settled/refresh states", () => { // already set. That must read the same as any other pre-first-read state: // the honest spinner, not a false empty. it("shows the loading indicator, not a false empty state, for an idle truncated boot seed", () => { - state = { + needsYouState = { status: "idle", bundles: [], hiddenCount: 0, @@ -263,7 +303,7 @@ describe("NeedsYouInboxPageClient loading vs. settled/refresh states", () => { // resolve. This must render the empty state, not a permanent spinner. it("renders the empty state, not a permanent spinner, when no active workspace resolves", () => { workspacesActiveId = null; - state = { + needsYouState = { status: "idle", bundles: [], hiddenCount: 0, @@ -277,3 +317,55 @@ describe("NeedsYouInboxPageClient loading vs. settled/refresh states", () => { expect(screen.queryByRole("status")).toBeNull(); }); }); + +describe("NeedsYouInboxPageClient — Failed tab (REQ-UI-INBOX-FAILED-001)", () => { + it("renders the tab strip with Needs you selected by default", () => { + render(); + expect(screen.getByRole("tab", { name: /Needs you/ }).getAttribute("aria-selected")).toBe( + "true", + ); + }); + + it("renders the Failed tab directly from ?tab=failed, without first rendering Needs you (AC .2)", () => { + setLocation("/needs-you-inbox?tab=failed"); + needsYouState = { status: "ready", bundles: [bundle("p1")], hiddenCount: 0, hasMore: false }; + failedState = { rows: [failedRow("t1")], count: 1, truncated: false, status: "ready" }; + + render(); + + expect(screen.getByTestId("failed-inbox-row")).not.toBeNull(); + expect(screen.queryByTestId("stub-row")).toBeNull(); + }); + + it("renders Needs you for an unrecognised tab value rather than an error (AC .2)", () => { + setLocation("/needs-you-inbox?tab=bogus"); + needsYouState = { status: "ready", bundles: [bundle("p1")], hiddenCount: 0, hasMore: false }; + + render(); + + expect(screen.getByTestId("stub-row")).not.toBeNull(); + }); + + it("shows the Failed tab's own badge count once known, even while Needs you is selected (AC .16)", () => { + failedState = { rows: [failedRow("t1")], count: 1, truncated: false, status: "ready" }; + render(); + + expect(screen.getByTestId("inbox-tab-failed-badge").textContent).toBe("1"); + }); + + it("passes the failed count into the Needs-you empty state's AC .23 clause", () => { + needsYouState = { status: "ready", bundles: [], hiddenCount: 0, hasMore: false }; + failedState = { rows: [failedRow("t1")], count: 1, truncated: false, status: "ready" }; + + render(); + + expect(screen.getByText(/also failed/)).not.toBeNull(); + }); + + it("mounts the failed-inbox controller with the currently selected tab", () => { + setLocation("/needs-you-inbox?tab=failed"); + render(); + + expect(mocks.useFailedInboxController).toHaveBeenCalledWith("failed"); + }); +}); 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 3c43229631..d3341cbd14 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 @@ -2,22 +2,34 @@ import { useCallback } from "react"; import { useTranslation } from "react-i18next"; +import { TabsContent } from "@kandev/ui/tabs"; import { PageShell } from "@/components/page-shell"; import { useAppStore } from "@/components/state-provider"; +import { usePathname, useRouter, useSearchParams } from "@/lib/routing/client-router"; import { selectNeedsYouInboxBundles, + selectNeedsYouInboxCount, selectNeedsYouInboxHasMore, selectNeedsYouInboxHiddenCount, selectNeedsYouInboxLastAppliedOk, selectNeedsYouInboxRevision, selectNeedsYouInboxStatus, } from "@/lib/state/slices/needs-you-inbox/selectors"; +import { + selectFailedInboxCount, + selectFailedInboxCountIsKnown, + selectFailedInboxTruncated, +} from "@/lib/state/slices/failed-inbox/selectors"; import type { NeedsYouInboxReadStatus } from "@/lib/state/slices/needs-you-inbox/types"; import type { ClarificationInboxBundle } from "@/lib/types/clarification-inbox"; import { NeedsYouInboxRow } from "@/components/needs-you-inbox/needs-you-inbox-row"; import { NeedsYouInboxEmptyState } from "@/components/needs-you-inbox/needs-you-inbox-empty-state"; import { NeedsYouInboxErrorState } from "@/components/needs-you-inbox/needs-you-inbox-error-state"; import { NeedsYouInboxHiddenPanel } from "@/components/needs-you-inbox/needs-you-inbox-hidden-panel"; +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"; type ViewMode = "error" | "loading" | "empty" | "list"; @@ -77,21 +89,17 @@ function NeedsYouInboxList({ ); } -// v1 renders no tab strip and no in-page title (design-01#Components). The -// title belongs to the app top bar, which is PageShell's, so this route mounts -// the same chrome every other top-level route does rather than an unlabelled -// bare div -- that chrome also carries the phone nav trigger (design-03#D4). -export function NeedsYouInboxPageClient() { +function NeedsYouInboxTabContent({ retry }: { retry: () => void }) { const { t } = useTranslation(); const status = useAppStore(selectNeedsYouInboxStatus); const bundles = useAppStore(selectNeedsYouInboxBundles); const hiddenCount = useAppStore(selectNeedsYouInboxHiddenCount); const listRevision = useAppStore(selectNeedsYouInboxRevision); const hasMore = useAppStore(selectNeedsYouInboxHasMore); + const failedCount = useAppStore(selectFailedInboxCount); + const failedCountKnown = useAppStore(selectFailedInboxCountIsKnown); const lastAppliedOk = useAppStore(selectNeedsYouInboxLastAppliedOk); const hasActiveWorkspace = useAppStore((s) => s.workspaces.activeId !== null); - const bumpRefreshTick = useAppStore((s) => s.bumpNeedsYouInboxRefreshTick); - const retry = useCallback(() => bumpRefreshTick(), [bumpRefreshTick]); const viewMode = resolveViewMode( status, @@ -102,7 +110,7 @@ export function NeedsYouInboxPageClient() { ); return ( - + <> {viewMode === "error" && } {viewMode === "loading" && (

@@ -110,7 +118,12 @@ export function NeedsYouInboxPageClient() {

)} {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 +// top-level route does rather than an unlabelled bare div -- that chrome also +// carries the phone nav trigger (design-03#D4). +export function NeedsYouInboxPageClient() { + const { t } = useTranslation(); + const bumpRefreshTick = useAppStore((s) => s.bumpNeedsYouInboxRefreshTick); + const needsYouCount = useAppStore(selectNeedsYouInboxCount); + const needsYouHasMore = useAppStore(selectNeedsYouInboxHasMore); + const failedCount = useAppStore(selectFailedInboxCount); + const failedCountKnown = useAppStore(selectFailedInboxCountIsKnown); + const failedTruncated = useAppStore(selectFailedInboxTruncated); + + const pathname = usePathname(); + const searchParams = useSearchParams(); + const router = useRouter(); + const selectedTab = resolveInboxTab(searchParams); + + // Mounted only here, never app-wide (design-01#Control-flow): the Failed + // bucket's own refresh triggers, independent of the Needs-you controller + // mounted at the app shell. + useFailedInboxController(selectedTab); + + const retry = useCallback(() => bumpRefreshTick(), [bumpRefreshTick]); + const selectTab = useCallback( + (tab: InboxTab) => router.replace(buildInboxTabHref(pathname, tab, searchParams)), + [router, pathname, searchParams], + ); + + return ( + + + + + + + + + ); } diff --git a/apps/web/components/app-sidebar/app-sidebar-primary-nav.tsx b/apps/web/components/app-sidebar/app-sidebar-primary-nav.tsx index 9c0ce4a4e1..462075ef12 100644 --- a/apps/web/components/app-sidebar/app-sidebar-primary-nav.tsx +++ b/apps/web/components/app-sidebar/app-sidebar-primary-nav.tsx @@ -69,6 +69,7 @@ export function AppSidebarPrimaryNav({ collapsed }: AppSidebarPrimaryNavProps) { badge={needsYouInboxCount} badgeSuffix={needsYouInboxHasMore ? "+" : undefined} collapsed={collapsed} + testId="sidebar-needs-you-inbox" /> )} {workspaceId && collapsed && ( diff --git a/apps/web/components/needs-you-inbox/failed-inbox-empty-state.test.tsx b/apps/web/components/needs-you-inbox/failed-inbox-empty-state.test.tsx new file mode 100644 index 0000000000..a4b1f7dbb0 --- /dev/null +++ b/apps/web/components/needs-you-inbox/failed-inbox-empty-state.test.tsx @@ -0,0 +1,36 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +let workspaceItems: Array<{ id: string; name: string }> = [{ id: "w1", name: "Kegmil V2" }]; + +vi.mock("@/components/state-provider", () => ({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + useAppStore: (selector: (s: any) => unknown) => + selector({ workspaces: { activeId: "w1", items: workspaceItems } }), +})); + +import { FailedInboxEmptyState } from "./failed-inbox-empty-state"; + +afterEach(() => { + cleanup(); + workspaceItems = [{ id: "w1", name: "Kegmil V2" }]; +}); + +describe("FailedInboxEmptyState", () => { + it("names the active workspace rather than the whole instance (AC .21)", () => { + render(); + expect(screen.getByText("Nothing has failed in Kegmil V2.")).not.toBeNull(); + }); + + it("names no workspace when the active workspace cannot be resolved (AC .21)", () => { + workspaceItems = []; + render(); + expect(screen.getByText("Nothing has failed in this workspace.")).not.toBeNull(); + }); + + it("does not congratulate the operator (AC .21)", () => { + const { container } = render(); + expect(container.textContent).not.toContain("!"); + expect(container.textContent).not.toContain("caught up"); + }); +}); diff --git a/apps/web/components/needs-you-inbox/failed-inbox-empty-state.tsx b/apps/web/components/needs-you-inbox/failed-inbox-empty-state.tsx new file mode 100644 index 0000000000..ff79d25573 --- /dev/null +++ b/apps/web/components/needs-you-inbox/failed-inbox-empty-state.tsx @@ -0,0 +1,27 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import { useAppStore } from "@/components/state-provider"; + +// An empty Failed bucket is a normal state, not an achievement: no icon, no +// congratulation, and the workspace is named so the sentence is never read as +// a claim about the whole instance (AC-UI-INBOX-FAILED-001.21). +export function FailedInboxEmptyState() { + const { t } = useTranslation(); + const workspaceName = useAppStore( + (s) => s.workspaces?.items?.find((w) => w.id === s.workspaces.activeId)?.name, + ); + return ( +
+

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

+

{t("failedInbox:emptyDescription")}

+
+ ); +} diff --git a/apps/web/components/needs-you-inbox/failed-inbox-error-state.test.tsx b/apps/web/components/needs-you-inbox/failed-inbox-error-state.test.tsx new file mode 100644 index 0000000000..85c83408d2 --- /dev/null +++ b/apps/web/components/needs-you-inbox/failed-inbox-error-state.test.tsx @@ -0,0 +1,22 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; +import { FailedInboxErrorState } from "./failed-inbox-error-state"; + +afterEach(() => cleanup()); + +describe("FailedInboxErrorState", () => { + it("renders an alert distinct from the empty state and does not claim nothing failed (AC .22)", () => { + render(); + const alert = screen.getByTestId("failed-inbox-error"); + expect(alert.getAttribute("role")).toBe("alert"); + expect(alert.textContent).not.toContain("Nothing has failed"); + }); + + // design-01#Failure-and-recovery: recovery needs no operator action, so the + // error state carries no retry affordance -- the periodic and foreground + // triggers already re-read within the same bound the rows are. + it("renders no retry control", () => { + render(); + expect(screen.queryByRole("button")).toBeNull(); + }); +}); diff --git a/apps/web/components/needs-you-inbox/failed-inbox-error-state.tsx b/apps/web/components/needs-you-inbox/failed-inbox-error-state.tsx new file mode 100644 index 0000000000..35e6236a3f --- /dev/null +++ b/apps/web/components/needs-you-inbox/failed-inbox-error-state.tsx @@ -0,0 +1,22 @@ +"use client"; + +import { IconAlertTriangle } from "@tabler/icons-react"; +import { useTranslation } from "react-i18next"; + +// No retry control: recovery needs no operator action here (design-01#Failure-and-recovery) +// -- the periodic and foreground-return triggers already re-read within the +// same bound the rows are, so a retry button would offer nothing. +export function FailedInboxErrorState() { + const { t } = useTranslation(); + return ( +
+ +

{t("failedInbox:errorTitle")}

+

{t("failedInbox:errorDescription")}

+
+ ); +} diff --git a/apps/web/components/needs-you-inbox/failed-inbox-row.test.tsx b/apps/web/components/needs-you-inbox/failed-inbox-row.test.tsx new file mode 100644 index 0000000000..0a4a67f917 --- /dev/null +++ b/apps/web/components/needs-you-inbox/failed-inbox-row.test.tsx @@ -0,0 +1,104 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; +import type { FailedInboxRow as FailedInboxRowData } from "@/lib/types/failed-inbox"; +import { FailedInboxRow } from "./failed-inbox-row"; + +const ORIGIN_MARKER_TESTID = "failed-inbox-origin-marker"; + +function row(overrides: Partial = {}): FailedInboxRowData { + return { + task_id: "t1", + title: "Sync repository", + workspace_id: "w1", + origin: "manual", + failure_instant: "2026-09-14T00:00:00Z", + reason: "connection refused", + ...overrides, + }; +} + +afterEach(() => cleanup()); + +describe("FailedInboxRow", () => { + it("renders the title, reason, and an open-task control (AC .18)", () => { + render(); + + expect(screen.getByText("Sync repository")).not.toBeNull(); + expect(screen.getByText("connection refused")).not.toBeNull(); + const link = screen.getByTestId("failed-inbox-open-task") as HTMLAnchorElement; + expect(link.getAttribute("href")).toBe("/t/t1"); + }); + + it("renders the task-state failed marker, not the session-state one, from the shared state-icon module (AC .18a)", () => { + // TASK_STATE_ICONS.FAILED (IconX) and SESSION_STATE_ICONS.FAILED + // (IconAlertTriangle) share the same wrapper testid and red styling, so + // asserting only that a status icon exists cannot catch a regression to + // the forbidden session-state pairing -- the rendered glyph itself has + // to be checked. + const { container } = render(); + const icon = container.querySelector('[data-testid="failed-inbox-status-icon"]'); + expect(icon).not.toBeNull(); + expect(icon!.querySelector(".tabler-icon-x")).not.toBeNull(); + expect(icon!.querySelector(".tabler-icon-alert-triangle")).toBeNull(); + }); + + it("presents the shared thread-status vocabulary's failed status (AC .20/.20a)", () => { + render(); + expect(screen.getByRole("img", { name: "Failed" })).not.toBeNull(); + }); + + it("falls back to stated copy when the reason is empty (AC .19)", () => { + render(); + expect(screen.getByTestId("failed-inbox-reason-fallback")).not.toBeNull(); + }); + + it("falls back to stated copy when the reason is only whitespace (AC .19)", () => { + render(); + expect(screen.getByTestId("failed-inbox-reason-fallback")).not.toBeNull(); + }); + + it("renders no origin marker for the manual origin (AC .9a)", () => { + render(); + expect(screen.queryByTestId(ORIGIN_MARKER_TESTID)).toBeNull(); + }); + + it("renders no origin marker for an absent origin (AC .9a)", () => { + render(); + expect(screen.queryByTestId(ORIGIN_MARKER_TESTID)).toBeNull(); + }); + + it("renders a distinct marker for a non-person origin (AC .9a)", () => { + render(); + expect(screen.getByTestId(ORIGIN_MARKER_TESTID)).not.toBeNull(); + }); + + it("renders the generic marker for an unenumerated non-empty origin (AC .9a)", () => { + render(); + expect(screen.getByTestId(ORIGIN_MARKER_TESTID)).not.toBeNull(); + }); + + it("states the failure time is unknown rather than substituting one when unresolvable (AC .30)", () => { + render(); + expect(screen.getByTestId("failed-inbox-unknown-time")).not.toBeNull(); + }); + + it("states the failure time is unknown for a syntactically malformed wire value rather than rendering Date's lenient parse of it", () => { + // `new Date("0")` parses to 1970, not NaN, so a naive `formatRelativeTime` + // call would render a plausible-but-wrong age instead of the fallback. + render(); + expect(screen.getByTestId("failed-inbox-unknown-time")).not.toBeNull(); + }); + + it("states the failure time is unknown for a calendar-invalid wire value Date.parse would otherwise normalize", () => { + // `new Date("2026-02-30T10:00:00Z")` normalizes to March 2 instead of + // rejecting the nonexistent date. + render(); + expect(screen.getByTestId("failed-inbox-unknown-time")).not.toBeNull(); + }); + + it("carries a session id in the task href when the row has none, since the row model carries none", () => { + render(); + const link = screen.getByTestId("failed-inbox-open-task") as HTMLAnchorElement; + expect(link.getAttribute("href")).not.toContain("sessionId"); + }); +}); diff --git a/apps/web/components/needs-you-inbox/failed-inbox-row.tsx b/apps/web/components/needs-you-inbox/failed-inbox-row.tsx new file mode 100644 index 0000000000..12d01a5860 --- /dev/null +++ b/apps/web/components/needs-you-inbox/failed-inbox-row.tsx @@ -0,0 +1,96 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import { Button } from "@kandev/ui/button"; +import Link from "@/components/routing/app-link"; +import { formatRelativeTime } from "@/lib/i18n/formats"; +import { getTaskStateIcon } from "@/lib/ui/state-icons"; +import { FAILED_TASK_STATUS } from "@/lib/threads/thread-session-status"; +import { parseStrictRfc3339Timestamp } from "@/lib/utils/strict-timestamp"; +import { + failedInboxOriginMarkerKey, + hasResolvableFailedInboxReason, +} from "@/lib/failed-inbox/row-presentation"; +import type { FailedInboxRow as FailedInboxRowData } from "@/lib/types/failed-inbox"; + +// The row addresses a task, never a session (design-01#Session-resolution): +// the open-task control needs only the task id. +function taskHrefForRow(row: FailedInboxRowData): string { + return `/t/${row.task_id}`; +} + +export function FailedInboxRow({ row }: { row: FailedInboxRowData }) { + const { t } = useTranslation(); + const originMarkerKey = failedInboxOriginMarkerKey(row.origin); + const reasonText = hasResolvableFailedInboxReason(row.reason) + ? row.reason + : t("failedInbox:reasonUnknown"); + // Date.parse (which formatRelativeTime's `new Date(...)` uses internally) + // normalizes malformed or non-RFC3339 wire values -- e.g. "0" becomes 1970 + // and "2026-02-30" becomes March 2 -- into a plausible but wrong instant + // instead of failing, so shape validation must happen first or the unknown- + // time fallback below is silently bypassed. + const relativeTime = + row.failure_instant && parseStrictRfc3339Timestamp(row.failure_instant) !== null + ? formatRelativeTime(row.failure_instant) + : null; + + return ( +
+ + {getTaskStateIcon("FAILED")} + +
+ + {row.title} + + {!hasResolvableFailedInboxReason(row.reason) ? ( + {reasonText} + ) : ( + reasonText + )} + + {originMarkerKey && ( + + {t(originMarkerKey)} + + )} + + + {relativeTime !== null && relativeTime !== "" ? ( + relativeTime + ) : ( + + {t("failedInbox:unknownFailureTime")} + + )} + +
+ {/* Always visible at every viewport width -- AC-UI-INBOX-FAILED-001.18 + requires the open-task control be reachable where a fixed-width + button would otherwise be withheld, and this row has no actions + menu to fall back to like the Needs-you row does. */} + +
+ ); +} diff --git a/apps/web/components/needs-you-inbox/failed-inbox-tab-panel.test.tsx b/apps/web/components/needs-you-inbox/failed-inbox-tab-panel.test.tsx new file mode 100644 index 0000000000..05958e0536 --- /dev/null +++ b/apps/web/components/needs-you-inbox/failed-inbox-tab-panel.test.tsx @@ -0,0 +1,108 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { FailedInboxRow as FailedInboxRowData } from "@/lib/types/failed-inbox"; +import { FailedInboxTabPanel } from "./failed-inbox-tab-panel"; + +vi.mock("@/components/state-provider", async () => { + const actual = await vi.importActual( + "@/components/state-provider", + ); + return { ...actual, useAppStore: (selector: (s: unknown) => unknown) => selector(mockState) }; +}); + +let mockState: { + workspaces: { activeId: string; items: Array<{ id: string; name: string }> }; + failedInbox: { + byWorkspaceId: Record< + string, + { rows: FailedInboxRowData[]; count: number; truncated: boolean; status: string } + >; + }; +}; + +function row(id: string): FailedInboxRowData { + return { + task_id: id, + title: `Task ${id}`, + workspace_id: "w1", + origin: "manual", + failure_instant: "2026-09-14T00:00:00Z", + reason: "boom", + }; +} + +beforeEach(() => { + mockState = { + workspaces: { activeId: "w1", items: [{ id: "w1", name: "Kegmil V2" }] }, + failedInbox: { byWorkspaceId: {} }, + }; +}); + +afterEach(() => cleanup()); + +describe("FailedInboxTabPanel", () => { + it("shows a loading indicator on the very first read", () => { + mockState.failedInbox.byWorkspaceId.w1 = { + rows: [], + count: 0, + truncated: false, + status: "loading", + }; + render(); + expect(screen.getByRole("status")).not.toBeNull(); + }); + + it("shows a loading indicator, not the empty state, before the first read ever completes", () => { + // No entry under w1 yet -- the slice's idle default, not a successful + // empty read. A false "nothing has failed" would misrepresent an unread + // workspace as a confirmed-empty one. + render(); + expect(screen.getByRole("status")).not.toBeNull(); + expect(screen.queryByTestId("failed-inbox-empty")).toBeNull(); + }); + + it("renders the error state, not the empty state, when the read failed (AC .22)", () => { + mockState.failedInbox.byWorkspaceId.w1 = { + rows: [], + count: 0, + truncated: false, + status: "error", + }; + render(); + expect(screen.getByTestId("failed-inbox-error")).not.toBeNull(); + expect(screen.queryByTestId("failed-inbox-empty")).toBeNull(); + }); + + it("renders the empty state when the read succeeded with no rows", () => { + mockState.failedInbox.byWorkspaceId.w1 = { + rows: [], + count: 0, + truncated: false, + status: "ready", + }; + render(); + expect(screen.getByTestId("failed-inbox-empty")).not.toBeNull(); + }); + + it("renders a row per listed failed task", () => { + mockState.failedInbox.byWorkspaceId.w1 = { + rows: [row("t1"), row("t2")], + count: 2, + truncated: false, + status: "ready", + }; + render(); + expect(screen.getAllByTestId("failed-inbox-row")).toHaveLength(2); + }); + + it("shows a truncation notice when the page is bounded", () => { + mockState.failedInbox.byWorkspaceId.w1 = { + rows: [row("t1")], + count: 1, + truncated: true, + status: "ready", + }; + render(); + expect(screen.getByTestId("failed-inbox-truncated")).not.toBeNull(); + }); +}); diff --git a/apps/web/components/needs-you-inbox/failed-inbox-tab-panel.tsx b/apps/web/components/needs-you-inbox/failed-inbox-tab-panel.tsx new file mode 100644 index 0000000000..98d2188474 --- /dev/null +++ b/apps/web/components/needs-you-inbox/failed-inbox-tab-panel.tsx @@ -0,0 +1,56 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import { useAppStore } from "@/components/state-provider"; +import { + selectFailedInboxRows, + selectFailedInboxStatus, + selectFailedInboxTruncated, +} from "@/lib/state/slices/failed-inbox/selectors"; +import { FailedInboxRow } from "./failed-inbox-row"; +import { FailedInboxEmptyState } from "./failed-inbox-empty-state"; +import { FailedInboxErrorState } from "./failed-inbox-error-state"; + +type ViewMode = "error" | "loading" | "empty" | "list"; + +function resolveViewMode(status: string, rowCount: number): ViewMode { + if (status === "error") return "error"; + if ((status === "loading" || status === "idle") && rowCount === 0) return "loading"; + if (rowCount === 0) return "empty"; + return "list"; +} + +export function FailedInboxTabPanel() { + const { t } = useTranslation(); + const status = useAppStore(selectFailedInboxStatus); + const rows = useAppStore(selectFailedInboxRows); + const truncated = useAppStore(selectFailedInboxTruncated); + + const viewMode = resolveViewMode(status, rows.length); + + return ( +
+ {viewMode === "error" && } + {viewMode === "loading" && ( +

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

+ )} + {viewMode === "empty" && } + {viewMode === "list" && ( + <> +
+ {rows.map((row) => ( + + ))} +
+ {truncated && ( +

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

+ )} + + )} +
+ ); +} 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 new file mode 100644 index 0000000000..eb3f447699 --- /dev/null +++ b/apps/web/components/needs-you-inbox/inbox-tab-strip.test.tsx @@ -0,0 +1,95 @@ +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TabsContent } from "@kandev/ui/tabs"; +import type { ReactNode } from "react"; +import { InboxTabStrip } from "./inbox-tab-strip"; +import type { InboxTab } from "@/lib/failed-inbox/inbox-tab"; + +const FAILED_BADGE_TESTID = "inbox-tab-failed-badge"; + +function renderStrip( + overrides: Partial<{ + selectedTab: InboxTab; + onSelectTab: (tab: InboxTab) => void; + needsYouCount: number; + needsYouHasMore: boolean; + failedCount: number | undefined; + failedTruncated: boolean; + children?: ReactNode; + }> = {}, +) { + return render( + , + ); +} + +afterEach(() => cleanup()); + +describe("InboxTabStrip", () => { + it("renders exactly two tabs, Needs you then Failed (AC .1)", () => { + renderStrip(); + const tabs = screen.getAllByRole("tab"); + expect(tabs).toHaveLength(2); + expect(tabs[0].textContent).toContain("Needs you"); + expect(tabs[1].textContent).toContain("Failed"); + }); + + it("calls onSelectTab with the clicked tab's value", () => { + const onSelectTab = vi.fn(); + renderStrip({ onSelectTab }); + // Radix TabsTrigger switches tabs on mousedown, not click (@radix-ui/react-tabs). + fireEvent.mouseDown(screen.getByRole("tab", { name: /Failed/ })); + expect(onSelectTab).toHaveBeenCalledWith("failed"); + }); + + it("renders no failed badge before a response is known (AC .16)", () => { + renderStrip(); + expect(screen.queryByTestId(FAILED_BADGE_TESTID)).toBeNull(); + }); + + it("renders no failed badge when the known count is zero (AC .16)", () => { + renderStrip({ failedCount: 0 }); + expect(screen.queryByTestId(FAILED_BADGE_TESTID)).toBeNull(); + }); + + it("renders the failed badge once a non-zero count is known", () => { + renderStrip({ failedCount: 3 }); + expect(screen.getByTestId(FAILED_BADGE_TESTID).textContent).toBe("3"); + }); + + it("renders a capped indicator when the failed page is truncated (AC .17)", () => { + renderStrip({ failedCount: 200, failedTruncated: true }); + expect(screen.getByTestId(FAILED_BADGE_TESTID).textContent).toBe("200+"); + }); + + it("stays visible while the other tab is selected (AC .16)", () => { + renderStrip({ selectedTab: "failed", failedCount: 2 }); + expect(screen.getByTestId(FAILED_BADGE_TESTID)).not.toBeNull(); + }); + + it("carries a coarse-pointer/mobile 44px touch-target floor on every tab trigger", () => { + renderStrip(); + for (const tab of screen.getAllByRole("tab")) { + expect(tab.className).toContain("min-h-11"); + } + }); + + it("connects a tab trigger to its content panel", () => { + renderStrip({ + children: Needs you content, + }); + + const tab = screen.getByRole("tab", { name: /Needs you/ }); + const panel = screen.getByRole("tabpanel"); + expect(tab.getAttribute("aria-controls")).toBe(panel.getAttribute("id")); + expect(panel.getAttribute("aria-labelledby")).toBe(tab.getAttribute("id")); + }); +}); diff --git a/apps/web/components/needs-you-inbox/inbox-tab-strip.tsx b/apps/web/components/needs-you-inbox/inbox-tab-strip.tsx new file mode 100644 index 0000000000..e42ef46a35 --- /dev/null +++ b/apps/web/components/needs-you-inbox/inbox-tab-strip.tsx @@ -0,0 +1,64 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import { Tabs, TabsList, TabsTrigger } from "@kandev/ui/tabs"; +import type { ReactNode } from "react"; +import { Badge } from "@kandev/ui/badge"; +import type { InboxTab } from "@/lib/failed-inbox/inbox-tab"; + +// Absent for a not-yet-known or zero count; a truncated count carries a "+" +// suffix (AC-UI-INBOX-FAILED-001.16, .17) -- the same capped presentation the +// sidebar badge already uses. +function badgeText(count: number | undefined, truncated: boolean): string | null { + if (typeof count !== "number" || count <= 0) return null; + return truncated ? `${count}+` : `${count}`; +} + +export function InboxTabStrip({ + selectedTab, + onSelectTab, + needsYouCount, + needsYouHasMore, + failedCount, + failedTruncated, + children, +}: { + selectedTab: InboxTab; + onSelectTab: (tab: InboxTab) => void; + needsYouCount: number; + needsYouHasMore: boolean; + failedCount: number | undefined; + failedTruncated: boolean; + children?: ReactNode; +}) { + const { t } = useTranslation(); + const needsYouBadge = badgeText(needsYouCount, needsYouHasMore); + const failedBadge = badgeText(failedCount, failedTruncated); + + return ( + onSelectTab(value as InboxTab)}> + + + {t("needsYouInbox:tabLabel")} + {needsYouBadge && ( + + {needsYouBadge} + + )} + + + {t("failedInbox:tabLabel")} + {failedBadge && ( + + {failedBadge} + + )} + + + {children} + + ); +} diff --git a/apps/web/components/needs-you-inbox/needs-you-inbox-empty-state.test.tsx b/apps/web/components/needs-you-inbox/needs-you-inbox-empty-state.test.tsx index e10ef744ef..ec30614ee5 100644 --- a/apps/web/components/needs-you-inbox/needs-you-inbox-empty-state.test.tsx +++ b/apps/web/components/needs-you-inbox/needs-you-inbox-empty-state.test.tsx @@ -68,3 +68,37 @@ describe("NeedsYouInboxEmptyState", () => { expect(screen.getByTestId("needs-you-inbox-hidden-panel")).not.toBeNull(); }); }); + +// AC-UI-INBOX-FAILED-001.4: this is the single permitted delta to the +// Needs-you empty state; every case above (workspace naming, no +// congratulation, hidden-bundle disclosure) renders identically when the new +// props are omitted, which is this suite's own before/after proof. +describe("NeedsYouInboxEmptyState — AC-UI-INBOX-FAILED-001.23 failed-tasks clause", () => { + it("omits the clause entirely when the failed count is not yet known", () => { + render(); + expect(screen.queryByText(/Failed tab/)).toBeNull(); + }); + + it("omits the clause when the last failed read failed", () => { + render(); + expect(screen.queryByText(/Failed tab/)).toBeNull(); + }); + + it("says a failed task exists rather than implying the workspace is quiet, on first render", () => { + render(); + expect( + screen.getByText( + "This list does not count failed tasks. 2 tasks have also failed. See the Failed tab.", + ), + ).not.toBeNull(); + }); + + it("does not imply any failed task exists when the workspace holds none", () => { + render(); + expect( + screen.getByText( + "This list does not count failed tasks. None have failed in this workspace right now; check the Failed tab any time.", + ), + ).not.toBeNull(); + }); +}); diff --git a/apps/web/components/needs-you-inbox/needs-you-inbox-empty-state.tsx b/apps/web/components/needs-you-inbox/needs-you-inbox-empty-state.tsx index 61955df185..6feeacc36e 100644 --- a/apps/web/components/needs-you-inbox/needs-you-inbox-empty-state.tsx +++ b/apps/web/components/needs-you-inbox/needs-you-inbox-empty-state.tsx @@ -11,9 +11,15 @@ import { NeedsYouInboxHiddenPanel } from "./needs-you-inbox-hidden-panel"; export function NeedsYouInboxEmptyState({ hiddenCount, listRevision = 0, + failedCount, + failedCountKnown = false, }: { hiddenCount: number; listRevision?: number; + /** The Failed tab's own count, read only for the AC-UI-INBOX-FAILED-001.23 + * clause below -- never written from here (design-01#Count-separation). */ + failedCount?: number; + failedCountKnown?: boolean; }) { const { t } = useTranslation(); const workspaceName = useAppStore( @@ -31,6 +37,16 @@ export function NeedsYouInboxEmptyState({ : t("needsYouInbox:emptyTitleUnnamedWorkspace")}

{t("needsYouInbox:emptyDescription")}

+ {/* AC-UI-INBOX-FAILED-001.23: omitted while the failed count is not + known (no read applied yet, or the last one failed), so this never + asserts presence or absence it cannot back up. */} + {failedCountKnown && ( +

+ {failedCount && failedCount > 0 + ? t("needsYouInbox:failedClauseSome", { count: failedCount }) + : t("needsYouInbox:failedClauseNone")} +

+ )} {hiddenCount > 0 && ( diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 31fd2df606..72df236a14 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -1447,6 +1447,7 @@ export class ApiClient { repositoryId?: string; startedAt?: string; completedAt?: string; + errorMessage?: string; commandCount?: number; metadata?: Record; }, @@ -1460,6 +1461,7 @@ export class ApiClient { if (opts.repositoryId !== undefined) body.repository_id = opts.repositoryId; if (opts.startedAt !== undefined) body.started_at = opts.startedAt; if (opts.completedAt !== undefined) body.completed_at = opts.completedAt; + if (opts.errorMessage !== undefined) body.error_message = opts.errorMessage; if (opts.commandCount !== undefined) body.command_count = opts.commandCount; if (opts.metadata !== undefined) body.metadata = opts.metadata; return this.request("POST", "/api/v1/_test/task-sessions", body); diff --git a/apps/web/e2e/tests/chat/inbox-failed-tab.spec.ts b/apps/web/e2e/tests/chat/inbox-failed-tab.spec.ts new file mode 100644 index 0000000000..a7a8189de6 --- /dev/null +++ b/apps/web/e2e/tests/chat/inbox-failed-tab.spec.ts @@ -0,0 +1,239 @@ +// AC-UI-INBOX-FAILED-001.29: observing the Failed tab's count before that tab +// is ever selected, then selecting it, observing a failed task listed with +// its reason and relative failure time, opening the task from the row, and +// observing that the sidebar count did not change when the failed task +// appeared. +import { test, expect } from "../../fixtures/test-base"; +import { injectLatency, waitForHttp } from "../../helpers/causal-waits"; + +test.describe("Inbox Failed tab", () => { + test("lists a failed task without inflating the sidebar count (AC .29)", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const title = "Inbox Failed Tab Fixture"; + const reason = "Simulated failure for the Inbox Failed tab"; + + const { task_id: taskId } = await apiClient.seedTask(seedData.workspaceId, title, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await apiClient.seedTaskSession(taskId, { + state: "FAILED", + completedAt: new Date().toISOString(), + errorMessage: reason, + }); + await apiClient.updateTaskState(taskId, "FAILED"); + + await testPage.goto("/needs-you-inbox"); + + // The sidebar's "someone is blocked on you" count must never move when a + // failed task exists (AC .14) -- captured before the Failed tab is ever + // selected, so an accidental read into the shared count would show here. + const sidebarInbox = testPage.getByTestId("sidebar-needs-you-inbox"); + await expect(sidebarInbox).toBeVisible(); + const sidebarTextBeforeFailedTabSelected = await sidebarInbox.textContent(); + + // The tab strip's own Failed count becomes visible on Inbox mount -- + // before the Failed tab is ever the selected one (AC .29's ordering). + const failedBadge = testPage.getByTestId("inbox-tab-failed-badge"); + await expect(failedBadge).toHaveText("1", { timeout: 30_000 }); + + await testPage.getByRole("tab", { name: /Failed/ }).click(); + + const row = testPage.getByTestId("failed-inbox-row").filter({ hasText: title }); + await expect(row).toBeVisible({ timeout: 15_000 }); + await expect(row).toContainText(reason); + + // A real failure instant renders a relative time, not the "unknown" fallback. + await expect(row.getByTestId("failed-inbox-unknown-time")).toHaveCount(0); + + await row.getByTestId("failed-inbox-open-task").click(); + await expect(testPage).toHaveURL(new RegExp(`/t/${taskId}$`)); + + const sidebarTextAfterOpeningTask = await sidebarInbox.textContent(); + expect(sidebarTextAfterOpeningTask).toBe(sidebarTextBeforeFailedTabSelected); + }); + + // AC-UI-INBOX-FAILED-001.16: "Changing the active workspace shall clear it + // until a response for the new one is applied." Regression coverage for the + // Review-round-2 blocker: returning to a workspace visited earlier in the + // same session must not keep showing that workspace's stale ready badge + // while the fresh read for it is still in flight. + test("clears the stale badge while re-reading a re-visited workspace (AC .16)", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const title = "Workspace Switch Back Fixture"; + const { task_id: taskId } = await apiClient.seedTask(seedData.workspaceId, title, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await apiClient.seedTaskSession(taskId, { + state: "FAILED", + completedAt: new Date().toISOString(), + errorMessage: "Simulated failure for the workspace-switch regression", + }); + await apiClient.updateTaskState(taskId, "FAILED"); + + const workspaceB = await apiClient.createWorkspace("Failed Tab Switch Target"); + + const failedBadge = testPage.getByTestId("inbox-tab-failed-badge"); + + // First visit to workspace A's Failed tab: reads to "ready" with 1 row, + // the cached state this test later returns to. + await testPage.goto("/needs-you-inbox"); + await expect(failedBadge).toHaveText("1", { timeout: 30_000 }); + + // Switch to workspace B (client-side nav; unmounts the Inbox page and its + // controller) and visit its own empty Failed tab, so the slice's + // last-active-workspace tracking actually flips away from A. + await testPage.getByTestId("sidebar-workspace-trigger").click(); + await testPage.getByTestId(`sidebar-workspace-item-${workspaceB.id}`).click(); + await expect(testPage).toHaveURL((url) => url.pathname === "/"); + await testPage.getByTestId("sidebar-needs-you-inbox").click(); + await expect(testPage).toHaveURL(/\/needs-you-inbox$/); + // Anchor the absence check to a point after the tab strip has actually + // rendered -- checking `toHaveCount(0)` immediately after the URL change + // could pass trivially during the pre-mount window and never re-poll + // once satisfied, so it would not reliably catch a badge that renders + // (correctly or, if regressed, stale) a few milliseconds later. + await expect(testPage.getByRole("tab", { name: /Failed/ })).toBeVisible(); + await expect(failedBadge).toHaveCount(0); + + // Switch back to workspace A (still client-side; the store is never torn + // down) and delay only the next failed-inbox read, so the in-flight + // "loading" window is observable instead of resolving instantly. + await testPage.getByTestId("sidebar-workspace-trigger").click(); + await testPage.getByTestId(`sidebar-workspace-item-${seedData.workspaceId}`).click(); + await expect(testPage).toHaveURL((url) => url.pathname === "/"); + + let delayNextFailedInboxRead = false; + await testPage.route("**/api/v1/failed-inbox*", async (route) => { + if (delayNextFailedInboxRead) { + delayNextFailedInboxRead = false; + await injectLatency( + 1500, + "make the re-visited workspace's loading window observable before it resolves", + ); + } + await route.continue(); + }); + delayNextFailedInboxRead = true; + const rereadForA = waitForHttp(testPage, "GET", /\/failed-inbox$/, { + predicate: (response) => + new URL(response.url()).searchParams.get("workspace_id") === seedData.workspaceId, + }); + + await testPage.getByTestId("sidebar-needs-you-inbox").click(); + await expect(testPage).toHaveURL(/\/needs-you-inbox$/); + // Anchored the same way as the switch-to-B assertion above. + await expect(testPage.getByRole("tab", { name: /Failed/ })).toBeVisible(); + + // The bug this covers: workspace A's own cache was still "ready" from the + // first visit, so the badge kept showing its stale "1" immediately + // instead of clearing while this re-read is in flight. + await expect(failedBadge).toHaveCount(0); + + await rereadForA; + await expect(failedBadge).toHaveText("1", { timeout: 15_000 }); + }); + + // Regression coverage for the Review-round-3 blocker: the sibling test + // above visits workspace B's own Failed tab before switching back, which is + // exactly what let the stale cache go undetected -- `beginFailedInboxRead` + // is only ever called while the Inbox is mounted, so a switch that happens + // with the Inbox closed throughout must still be caught the next time this + // workspace is read. + test("clears a re-visited workspace's stale badge even when the switch away happened with the Inbox closed (AC .16)", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const title = "Workspace Switch Back While Closed Fixture"; + const { task_id: taskId } = await apiClient.seedTask(seedData.workspaceId, title, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await apiClient.seedTaskSession(taskId, { + state: "FAILED", + completedAt: new Date().toISOString(), + errorMessage: "Simulated failure for the closed-Inbox workspace-switch regression", + }); + await apiClient.updateTaskState(taskId, "FAILED"); + + const workspaceB = await apiClient.createWorkspace("Failed Tab Switch Target (Closed)"); + + const failedBadge = testPage.getByTestId("inbox-tab-failed-badge"); + + // First visit to workspace A's Failed tab: reads to "ready" with 1 row, + // the cached state this test later returns to. + await testPage.goto("/needs-you-inbox"); + await expect(failedBadge).toHaveText("1", { timeout: 30_000 }); + + // Switch to workspace B but never open its Inbox -- stay on the landing + // page. Nothing here calls the failed-inbox read for B at all, unlike + // the sibling test above. The switcher is a controlled Radix dropdown + // (app-sidebar-header.tsx wires its open state through the store) whose + // content has a CSS exit animation (dropdown-menu.tsx's + // `data-closed:animate-out ... duration-100`); Radix keeps the departing + // item mounted until that animation ends, after `aria-expanded` has + // already flipped. Both switches land on pathname "/" (only the + // `workspaceId` query param changes), so the routed page never remounts + // to force a settle the way every other picker test's route change does + // -- wait for the departing item to actually leave the DOM before + // reopening the same trigger, or the second open can race Radix's + // in-flight close and land unstable. + const workspaceTrigger = testPage.getByTestId("sidebar-workspace-trigger"); + const itemB = testPage.getByTestId(`sidebar-workspace-item-${workspaceB.id}`); + await workspaceTrigger.click(); + await itemB.click(); + await expect(testPage).toHaveURL((url) => url.pathname === "/"); + await expect(itemB).toHaveCount(0); + + // Switch back to workspace A, still without ever having opened the Inbox + // on B. + const itemA = testPage.getByTestId(`sidebar-workspace-item-${seedData.workspaceId}`); + await workspaceTrigger.click(); + await itemA.click(); + await expect(testPage).toHaveURL((url) => url.pathname === "/"); + await expect(itemA).toHaveCount(0); + await expect(workspaceTrigger).toHaveAttribute("aria-expanded", "false"); + + let delayNextFailedInboxRead = false; + await testPage.route("**/api/v1/failed-inbox*", async (route) => { + if (delayNextFailedInboxRead) { + delayNextFailedInboxRead = false; + await injectLatency( + 1500, + "make the re-visited workspace's loading window observable before it resolves", + ); + } + await route.continue(); + }); + delayNextFailedInboxRead = true; + const rereadForA = waitForHttp(testPage, "GET", /\/failed-inbox$/, { + predicate: (response) => + new URL(response.url()).searchParams.get("workspace_id") === seedData.workspaceId, + }); + + await testPage.getByTestId("sidebar-needs-you-inbox").click(); + await expect(testPage).toHaveURL(/\/needs-you-inbox$/); + await expect(testPage.getByRole("tab", { name: /Failed/ })).toBeVisible(); + + // The bug this covers: because the Inbox was never open while B was + // active, the slice's workspace-change tracking never saw the switch, + // so A's cache was still "ready" from the first visit and the badge + // kept showing its stale "1" instead of clearing while this re-read is + // in flight. + await expect(failedBadge).toHaveCount(0); + + await rereadForA; + await expect(failedBadge).toHaveText("1", { timeout: 15_000 }); + }); +}); diff --git a/apps/web/e2e/tests/chat/mobile-inbox-failed-tab.spec.ts b/apps/web/e2e/tests/chat/mobile-inbox-failed-tab.spec.ts new file mode 100644 index 0000000000..a34ba8f0ad --- /dev/null +++ b/apps/web/e2e/tests/chat/mobile-inbox-failed-tab.spec.ts @@ -0,0 +1,83 @@ +// AC-UI-INBOX-FAILED-001.29: the Failed tab golden path must also work on a +// phone-sized viewport, where the row's only control falls back to the 44px +// touch target (control-sizing.tsx's coarse-pointer breakpoint) instead of +// the desktop-density button. This mirrors inbox-failed-tab.spec.ts's +// desktop coverage rather than duplicating its assertions. +import { test, expect } from "../../fixtures/test-base"; + +const MIN_TOUCH_TARGET_PX = 44; + +test.describe("Inbox Failed tab on mobile", () => { + test("lists a failed task with a touch-sized open-task control (AC .29)", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const title = "Mobile Inbox Failed Tab Fixture"; + const reason = "Simulated failure for the Inbox Failed tab"; + + const { task_id: taskId } = await apiClient.seedTask(seedData.workspaceId, title, { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + }); + await apiClient.seedTaskSession(taskId, { + // Keep the session non-terminal so its completed_at remains NULL. The + // task is still failed below, which exercises the row's unknown-time + // fallback without violating the harness contract for terminal + // sessions. + state: "IDLE", + errorMessage: reason, + }); + await apiClient.updateTaskState(taskId, "FAILED"); + + await testPage.goto("/needs-you-inbox"); + + // The Failed count is visible before the Failed tab is ever selected + // (AC .29's ordering), same as the desktop coverage. + const failedBadge = testPage.getByTestId("inbox-tab-failed-badge"); + await expect(failedBadge).toHaveText("1", { timeout: 30_000 }); + + // The tab strip itself (not just the row's open-task control) is a + // coarse-pointer surface an operator must tap to reach this tab. + const failedTab = testPage.getByRole("tab", { name: /Failed/ }); + const failedTabBox = await failedTab.boundingBox(); + expect(failedTabBox, "Failed tab trigger should have a box").not.toBeNull(); + expect(failedTabBox!.height).toBeGreaterThanOrEqual(MIN_TOUCH_TARGET_PX); + + await failedTab.click(); + + const row = testPage.getByTestId("failed-inbox-row").filter({ hasText: title }); + await expect(row).toBeVisible({ timeout: 15_000 }); + await expect(row).toContainText(reason); + + const openTask = row.getByTestId("failed-inbox-open-task"); + const openTaskBox = await openTask.boundingBox(); + expect(openTaskBox, "open-task control should have a box").not.toBeNull(); + expect(openTaskBox!.height).toBeGreaterThanOrEqual(MIN_TOUCH_TARGET_PX); + + const unknownTime = row.getByTestId("failed-inbox-unknown-time"); + await expect(unknownTime).toBeVisible(); + const rowBox = await row.boundingBox(); + const unknownTimeBox = await unknownTime.boundingBox(); + expect(rowBox, "failed row should have a box").not.toBeNull(); + expect(unknownTimeBox, "unknown failure time should have a box").not.toBeNull(); + expect(openTaskBox!.x + openTaskBox!.width).toBeLessThanOrEqual(rowBox!.x + rowBox!.width); + expect(unknownTimeBox!.x + unknownTimeBox!.width).toBeLessThanOrEqual( + rowBox!.x + rowBox!.width, + ); + + // The Inbox/Failed-tab page itself must not scroll horizontally on a + // phone viewport -- checked here, before navigating away, so this + // measures the page the test is actually about rather than the + // task-detail page the next step opens. + const overflow = await testPage.evaluate(() => ({ + scrollWidth: document.documentElement.scrollWidth, + clientWidth: document.documentElement.clientWidth, + })); + expect(overflow.scrollWidth).toBeLessThanOrEqual(overflow.clientWidth); + + await openTask.click(); + await expect(testPage).toHaveURL(new RegExp(`/t/${taskId}$`)); + }); +}); diff --git a/apps/web/hooks/domains/failed-inbox/use-failed-inbox-controller.test.tsx b/apps/web/hooks/domains/failed-inbox/use-failed-inbox-controller.test.tsx new file mode 100644 index 0000000000..0259edb8ec --- /dev/null +++ b/apps/web/hooks/domains/failed-inbox/use-failed-inbox-controller.test.tsx @@ -0,0 +1,143 @@ +import { act, cleanup, 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 listFailedInboxMock = vi.fn(); +vi.mock("@/lib/api/domains/failed-inbox-api", () => ({ + listFailedInbox: (...args: unknown[]) => listFailedInboxMock(...args), +})); + +import { useFailedInboxController } from "./use-failed-inbox-controller"; + +const WORKSPACE_ID = "w1"; + +function page(overrides: Partial>> = {}) { + return { rows: [], count: 0, truncated: false, ...overrides }; +} + +function initialState(enabled: boolean, workspaceId = WORKSPACE_ID): HydrationState { + return { + features: { ...defaultFeatureFlags, needsYouInbox: enabled }, + workspaces: { activeId: workspaceId, byId: {}, allIds: [] }, + } as unknown as HydrationState; +} + +function renderController(selectedTab: "needs-you" | "failed" = "needs-you", enabled = true) { + let rerenderTab: (tab: "needs-you" | "failed") => void = () => {}; + const view = renderHook( + ({ tab }: { tab: "needs-you" | "failed" }) => { + useFailedInboxController(tab); + return useAppStoreApi(); + }, + { + initialProps: { tab: selectedTab }, + wrapper: ({ children }) => ( + {children} + ), + }, + ); + rerenderTab = (tab) => view.rerender({ tab }); + return { ...view, rerenderTab }; +} + +beforeEach(() => { + listFailedInboxMock.mockReset(); + listFailedInboxMock.mockResolvedValue(page()); +}); + +afterEach(() => { + cleanup(); + vi.useRealTimers(); +}); + +describe("useFailedInboxController", () => { + it("reads on mount for the active workspace regardless of selected tab", async () => { + renderController("needs-you"); + await waitFor(() => expect(listFailedInboxMock).toHaveBeenCalledWith(WORKSPACE_ID)); + }); + + it("never reads when the Inbox feature flag is disabled", async () => { + renderController("needs-you", false); + await act(async () => { + await Promise.resolve(); + }); + expect(listFailedInboxMock).not.toHaveBeenCalled(); + }); + + it("applies a successful read into the failed-inbox slice", async () => { + listFailedInboxMock.mockResolvedValue( + page({ + count: 1, + truncated: false, + rows: [{ task_id: "t1", title: "T", workspace_id: WORKSPACE_ID, origin: "", reason: "" }], + }), + ); + const { result } = renderController("failed"); + + await waitFor(() => { + const state = result.current.getState().failedInbox.byWorkspaceId[WORKSPACE_ID]; + expect(state?.status).toBe("ready"); + expect(state?.count).toBe(1); + expect(state?.rows).toHaveLength(1); + }); + }); + + it("applies an errored read into the failed-inbox slice", async () => { + listFailedInboxMock.mockRejectedValue(new Error("boom")); + const { result } = renderController("needs-you"); + + await waitFor(() => { + const state = result.current.getState().failedInbox.byWorkspaceId[WORKSPACE_ID]; + expect(state?.status).toBe("error"); + }); + }); + + it("re-reads when the selected tab changes, even though it already read at mount", async () => { + const { rerenderTab } = renderController("needs-you"); + await waitFor(() => expect(listFailedInboxMock).toHaveBeenCalledTimes(1)); + + rerenderTab("failed"); + + await waitFor(() => expect(listFailedInboxMock).toHaveBeenCalledTimes(2)); + }); + + it("re-reads once at the periodic 60s interval while the tab stays visible, regardless of selected tab", async () => { + vi.useFakeTimers(); + renderController("needs-you"); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(listFailedInboxMock).toHaveBeenCalledTimes(1); + + await act(async () => { + await vi.advanceTimersByTimeAsync(60_000); + }); + + expect(listFailedInboxMock).toHaveBeenCalledTimes(2); + }); + + it("does not re-read at the periodic interval while the tab is hidden", async () => { + vi.useFakeTimers(); + Object.defineProperty(document, "visibilityState", { + configurable: true, + get: () => "hidden", + }); + renderController("needs-you"); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + listFailedInboxMock.mockClear(); + + await act(async () => { + await vi.advanceTimersByTimeAsync(60_000); + }); + + expect(listFailedInboxMock).not.toHaveBeenCalled(); + Object.defineProperty(document, "visibilityState", { + configurable: true, + get: () => "visible", + }); + }); +}); diff --git a/apps/web/hooks/domains/failed-inbox/use-failed-inbox-controller.ts b/apps/web/hooks/domains/failed-inbox/use-failed-inbox-controller.ts new file mode 100644 index 0000000000..1b39a6dcb2 --- /dev/null +++ b/apps/web/hooks/domains/failed-inbox/use-failed-inbox-controller.ts @@ -0,0 +1,73 @@ +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 { listFailedInbox } from "@/lib/api/domains/failed-inbox-api"; +import type { InboxTab } from "@/lib/failed-inbox/inbox-tab"; + +// A maximum staleness, not a rate limit (design-01#Control-flow): the +// residual catch-all for an exit that emits no event. +const PERIODIC_REFRESH_MS = 60_000; + +/** + * Owns every Failed-bucket refresh trigger (design-01#Control-flow): Inbox + * mount, tab selection changing, active workspace changing, the browser tab + * returning to visible, and a 60s periodic re-read while visible. Every + * trigger fires regardless of which tab is currently selected. Deliberately + * NOT app-wide and NOT WS-subscribed: mounted only with the Inbox page, + * because nothing outside the Inbox renders this bucket's count, and it must + * not share the Needs-you slice's WS-triggered refresh machinery + * (design-01#Control-flow, "why it does not stop when Needs you is + * selected"). + */ +export function useFailedInboxController(selectedTab: InboxTab) { + const enabled = useFeature("needsYouInbox"); + const workspaceId = useAppStore((s) => s.workspaces.activeId); + const storeApi = useAppStoreApi(); + + const refresh = useCallback( + async (targetWorkspaceId: string) => { + const { beginFailedInboxRead, setFailedInboxPage, setFailedInboxError } = storeApi.getState(); + const generation = beginFailedInboxRead(targetWorkspaceId); + try { + const page = await listFailedInbox(targetWorkspaceId); + setFailedInboxPage(targetWorkspaceId, generation, { + rows: page.rows, + count: page.count, + truncated: page.truncated, + }); + } catch { + setFailedInboxError(targetWorkspaceId, generation); + } + }, + [storeApi], + ); + + // Triggers: Inbox mount, tab selection changing, active workspace changing. + // One effect covers all three -- it fires on mount and again whenever + // either dependency changes, regardless of which tab ends up selected. + useEffect(() => { + if (!enabled || !workspaceId) return; + void refresh(workspaceId); + // eslint-disable-next-line react-hooks/exhaustive-deps -- selectedTab is + // a trigger, not a read; re-running this effect is the only thing it does. + }, [enabled, workspaceId, selectedTab, refresh]); + + // Trigger: tab visibility regained (coalesced with focus/pageshow/online). + useForegroundRefresh( + () => { + if (workspaceId) void refresh(workspaceId); + }, + enabled && !!workspaceId, + workspaceId, + ); + + // Trigger: bounded periodic re-read, only while the browser tab is visible. + useEffect(() => { + if (!enabled || !workspaceId) return; + const interval = window.setInterval(() => { + if (document.visibilityState === "visible") void refresh(workspaceId); + }, PERIODIC_REFRESH_MS); + return () => window.clearInterval(interval); + }, [enabled, workspaceId, refresh]); +} diff --git a/apps/web/lib/api/domains/failed-inbox-api.test.ts b/apps/web/lib/api/domains/failed-inbox-api.test.ts new file mode 100644 index 0000000000..68c3d7a4ca --- /dev/null +++ b/apps/web/lib/api/domains/failed-inbox-api.test.ts @@ -0,0 +1,47 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const fetchJsonMock = vi.hoisted(() => vi.fn()); + +vi.mock("../client", () => ({ + fetchJson: fetchJsonMock, +})); + +import { listFailedInbox } from "./failed-inbox-api"; + +describe("listFailedInbox", () => { + beforeEach(() => fetchJsonMock.mockReset()); + + it("requests the failed-inbox endpoint with only the workspace id", async () => { + const response = { rows: [], count: 0, truncated: false }; + fetchJsonMock.mockResolvedValue(response); + + await expect(listFailedInbox("workspace-1")).resolves.toEqual(response); + expect(fetchJsonMock).toHaveBeenCalledWith( + "/api/v1/failed-inbox?workspace_id=workspace-1", + undefined, + ); + }); + + it("encodes the workspace id", async () => { + fetchJsonMock.mockResolvedValue({ rows: [], count: 0, truncated: false }); + + await listFailedInbox("workspace/needs encoding"); + + expect(fetchJsonMock).toHaveBeenCalledWith( + "/api/v1/failed-inbox?workspace_id=workspace%2Fneeds%20encoding", + undefined, + ); + }); + + it("forwards request options", async () => { + const options = { cache: "no-store" as const }; + fetchJsonMock.mockResolvedValue({ rows: [], count: 0, truncated: false }); + + await listFailedInbox("workspace-1", options); + + expect(fetchJsonMock).toHaveBeenCalledWith( + "/api/v1/failed-inbox?workspace_id=workspace-1", + options, + ); + }); +}); diff --git a/apps/web/lib/api/domains/failed-inbox-api.ts b/apps/web/lib/api/domains/failed-inbox-api.ts new file mode 100644 index 0000000000..ad730c9453 --- /dev/null +++ b/apps/web/lib/api/domains/failed-inbox-api.ts @@ -0,0 +1,16 @@ +import { fetchJson, type ApiRequestOptions } from "../client"; +import type { FailedInboxPage } from "@/lib/types/failed-inbox"; + +const BASE = "/api/v1/failed-inbox"; + +// One canonical parameter set (design-01#Control-flow): every caller passes +// only workspaceId. No cursor -- this read serves exactly one bounded page. +export function listFailedInbox( + workspaceId: string, + options?: ApiRequestOptions, +): Promise { + return fetchJson( + `${BASE}?workspace_id=${encodeURIComponent(workspaceId)}`, + options, + ); +} diff --git a/apps/web/lib/failed-inbox/inbox-tab.test.ts b/apps/web/lib/failed-inbox/inbox-tab.test.ts new file mode 100644 index 0000000000..239696c895 --- /dev/null +++ b/apps/web/lib/failed-inbox/inbox-tab.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; +import { INBOX_TAB_QUERY_PARAM, buildInboxTabHref, resolveInboxTab } from "./inbox-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. +describe("resolveInboxTab", () => { + it("selects failed for exactly one tab=failed value", () => { + expect(resolveInboxTab(new URLSearchParams("tab=failed"))).toBe("failed"); + }); + + it("selects needs-you when the param is absent", () => { + expect(resolveInboxTab(new URLSearchParams(""))).toBe("needs-you"); + }); + + it("selects needs-you when the param is empty", () => { + expect(resolveInboxTab(new URLSearchParams("tab="))).toBe("needs-you"); + }); + + it("selects needs-you when the param is repeated, even if both are failed", () => { + expect(resolveInboxTab(new URLSearchParams("tab=failed&tab=failed"))).toBe("needs-you"); + }); + + it("selects needs-you for an unrecognised value", () => { + expect(resolveInboxTab(new URLSearchParams("tab=something-else"))).toBe("needs-you"); + }); + + it("selects needs-you for the explicit needs-you value", () => { + expect(resolveInboxTab(new URLSearchParams("tab=needs-you"))).toBe("needs-you"); + }); +}); + +describe("buildInboxTabHref", () => { + it("carries the selected tab as the only query parameter", () => { + expect(buildInboxTabHref("/needs-you-inbox", "failed")).toBe( + `/needs-you-inbox?${INBOX_TAB_QUERY_PARAM}=failed`, + ); + }); + + it("preserves other existing query parameters", () => { + expect( + buildInboxTabHref("/needs-you-inbox?other=1", "failed", new URLSearchParams("other=1")), + ).toBe(`/needs-you-inbox?other=1&${INBOX_TAB_QUERY_PARAM}=failed`); + }); +}); diff --git a/apps/web/lib/failed-inbox/inbox-tab.ts b/apps/web/lib/failed-inbox/inbox-tab.ts new file mode 100644 index 0000000000..bbc17a5a30 --- /dev/null +++ b/apps/web/lib/failed-inbox/inbox-tab.ts @@ -0,0 +1,26 @@ +export type InboxTab = "needs-you" | "failed"; + +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. +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"; +} + +// Selecting a tab REPLACES the current history entry (AC-UI-INBOX-FAILED-001.2), +// carrying the selected tab alongside any other existing query parameters. +export function buildInboxTabHref( + pathname: string, + tab: InboxTab, + currentSearchParams: URLSearchParams = new URLSearchParams(), +): string { + const params = new URLSearchParams(currentSearchParams); + params.set(INBOX_TAB_QUERY_PARAM, tab); + return `${pathname.split("?")[0]}?${params.toString()}`; +} diff --git a/apps/web/lib/failed-inbox/row-presentation.test.ts b/apps/web/lib/failed-inbox/row-presentation.test.ts new file mode 100644 index 0000000000..eaa10d9ae4 --- /dev/null +++ b/apps/web/lib/failed-inbox/row-presentation.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { failedInboxOriginMarkerKey, hasResolvableFailedInboxReason } from "./row-presentation"; + +// AC-UI-INBOX-FAILED-001.9a: `manual` is the only origin treated as a +// person and renders no marker; each of the five non-person origins gets its +// own distinct marker; anything else (including absent/empty) either takes +// the `manual` path (no marker) or, if non-empty and unrecognised, the +// generic marker. +describe("failedInboxOriginMarkerKey", () => { + it("renders no marker for the manual (person) origin", () => { + expect(failedInboxOriginMarkerKey("manual")).toBeNull(); + }); + + it("renders no marker for an absent or empty origin", () => { + expect(failedInboxOriginMarkerKey("")).toBeNull(); + expect(failedInboxOriginMarkerKey(undefined)).toBeNull(); + }); + + it("gives each of the five non-person origins its own distinct marker key", () => { + const keys = [ + "agent_created", + "routine", + "onboarding", + "automation_run", + "automation_task", + ].map((origin) => failedInboxOriginMarkerKey(origin)); + expect(new Set(keys).size).toBe(5); + for (const key of keys) expect(key).toBeTruthy(); + }); + + it("renders the generic marker for a non-empty, unenumerated origin", () => { + expect(failedInboxOriginMarkerKey("some_future_origin")).toBe("failedInbox:originGeneric"); + }); +}); + +describe("hasResolvableFailedInboxReason", () => { + it("is false for an empty string", () => { + expect(hasResolvableFailedInboxReason("")).toBe(false); + }); + + it("is false for a whitespace-only string", () => { + expect(hasResolvableFailedInboxReason(" \n\t ")).toBe(false); + }); + + it("is true for a non-blank reason", () => { + expect(hasResolvableFailedInboxReason("connection refused")).toBe(true); + }); +}); diff --git a/apps/web/lib/failed-inbox/row-presentation.ts b/apps/web/lib/failed-inbox/row-presentation.ts new file mode 100644 index 0000000000..f13b24d5c5 --- /dev/null +++ b/apps/web/lib/failed-inbox/row-presentation.ts @@ -0,0 +1,24 @@ +// AC-UI-INBOX-FAILED-001.9a: `manual` is the only origin treated as a +// person and renders no marker; the five enumerated non-person values (mirrors +// apps/backend/internal/task/models/models.go's TaskOrigin* constants) each +// render their own distinct marker key, and anything else non-empty renders +// the generic marker rather than nothing, the raw value, or a broken row. +const ORIGIN_MARKER_KEYS: Record = { + agent_created: "failedInbox:originAgentCreated", + routine: "failedInbox:originRoutine", + onboarding: "failedInbox:originOnboarding", + automation_run: "failedInbox:originAutomationRun", + automation_task: "failedInbox:originAutomationTask", +}; + +export function failedInboxOriginMarkerKey(origin: string | undefined): string | null { + if (!origin || origin === "manual") return null; + return ORIGIN_MARKER_KEYS[origin] ?? "failedInbox:originGeneric"; +} + +// AC-UI-INBOX-FAILED-001.19: applies to the value the server returns, after +// its own truncation, so a reason whose first 512 code points are all +// whitespace still takes the fallback. +export function hasResolvableFailedInboxReason(reason: string): boolean { + return reason.trim().length > 0; +} diff --git a/apps/web/lib/state/app-state-extra-actions.ts b/apps/web/lib/state/app-state-extra-actions.ts index dfe848217b..1d946f8290 100644 --- a/apps/web/lib/state/app-state-extra-actions.ts +++ b/apps/web/lib/state/app-state-extra-actions.ts @@ -13,6 +13,7 @@ import type { PluginsSliceActions, ReviewSliceActions, NeedsYouInboxSliceActions, + FailedInboxSliceActions, } from "./slices"; // Split out of app-state-types.ts to stay under the file line limit. These @@ -31,4 +32,5 @@ export type AppStateExtraActions = Pick { expect(state.quickChat.selectionReadyByWorkspace).toEqual({ "workspace-1": true }); }); }); + +describe("failed Inbox hydration state", () => { + it("keeps the request revision map when older persisted state omits it", () => { + const state = mergeInitialState({ + failedInbox: { + byWorkspaceId: {}, + generationByWorkspaceId: {}, + }, + } as unknown as HydrationState); + + expect(state.failedInbox.readRevisionByWorkspaceId).toEqual({}); + }); +}); diff --git a/apps/web/lib/state/default-state.ts b/apps/web/lib/state/default-state.ts index 14aad51ac1..c0fb41fb62 100644 --- a/apps/web/lib/state/default-state.ts +++ b/apps/web/lib/state/default-state.ts @@ -17,6 +17,7 @@ import { defaultSystemState, defaultReviewState, defaultNeedsYouInboxState, + defaultFailedInboxState, } from "./slices"; import { mergeHydratedQuickChatSessions } from "@/lib/state/slices/ui/quick-chat-sync"; import type { AgentRuntimeAvailability } from "@/lib/types/agent-runtime"; @@ -69,6 +70,7 @@ export const defaultState = { walkthroughs: defaultSessionState.walkthroughs, taskReview: defaultReviewState.taskReview, needsYouInbox: defaultNeedsYouInboxState.needsYouInbox, + failedInbox: defaultFailedInboxState.failedInbox, queue: defaultSessionState.queue, terminal: defaultSessionRuntimeState.terminal, shell: defaultSessionRuntimeState.shell, @@ -526,6 +528,14 @@ export function mergeInitialState(initialState?: HydrationState): DefaultState { }, office: { ...defaultState.office, ...initialState.office }, needsYouInbox: { ...defaultState.needsYouInbox, ...initialState.needsYouInbox }, + failedInbox: { + ...defaultState.failedInbox, + ...initialState.failedInbox, + readRevisionByWorkspaceId: { + ...defaultState.failedInbox.readRevisionByWorkspaceId, + ...initialState.failedInbox?.readRevisionByWorkspaceId, + }, + }, features: { ...defaultState.features, ...initialState.features }, auth: { ...defaultState.auth, ...initialState.auth }, ...mergeSessionHostnamesState(initialState), diff --git a/apps/web/lib/state/slices/failed-inbox/failed-inbox-slice.test.ts b/apps/web/lib/state/slices/failed-inbox/failed-inbox-slice.test.ts new file mode 100644 index 0000000000..4aa33d4f81 --- /dev/null +++ b/apps/web/lib/state/slices/failed-inbox/failed-inbox-slice.test.ts @@ -0,0 +1,266 @@ +import { describe, expect, it } from "vitest"; +import { create } from "zustand"; +import { immer } from "zustand/middleware/immer"; +import type { FailedInboxRow } from "@/lib/types/failed-inbox"; +import { createFailedInboxSlice } from "./failed-inbox-slice"; +import type { FailedInboxSlice } from "./types"; +import { createWorkspaceSlice } from "../workspace/workspace-slice"; +import type { WorkspaceSlice } from "../workspace/types"; + +type TestStore = FailedInboxSlice & WorkspaceSlice; + +function newStore() { + // `createFailedInboxSlice` and `createWorkspaceSlice` both type `set`/`get` + // against the full `AppState` (the zustand slices pattern, so composing + // them in store.ts needs no cast); this isolated test store only composes + // these two slices, so the shapes need a cast here, in test-only code the + // store.ts architecture rule does not cover. `createWorkspaceSlice` is + // real, not a stub: `beginFailedInboxRead` now reads + // `workspaces.activeIdRevision`, the app-wide signal a genuine workspace + // switch bumps regardless of whether the Inbox is mounted to see it, so a + // test proving that has to drive a real `setActiveWorkspace`. + return create()( + immer((set, get, api) => ({ + ...createWorkspaceSlice( + set as unknown as Parameters[0], + get as unknown as Parameters[1], + api as unknown as Parameters[2], + ), + ...createFailedInboxSlice( + set as unknown as Parameters[0], + get as unknown as Parameters[1], + ), + })), + ); +} + +function row(overrides: Partial = {}): FailedInboxRow { + return { + task_id: "t1", + title: "Task", + workspace_id: "w1", + origin: "manual", + failure_instant: "2026-09-14T01:00:00Z", + reason: "boom", + ...overrides, + }; +} + +describe("failed-inbox slice", () => { + it("has no rows or count for a workspace before any read (AC .16: absent is not zero)", () => { + const store = newStore(); + expect(store.getState().failedInbox.byWorkspaceId.w1).toBeUndefined(); + }); + + it("applies a page read tagged with the current generation", () => { + const store = newStore(); + const generation = store.getState().beginFailedInboxRead("w1"); + + store.getState().setFailedInboxPage("w1", generation, { + rows: [row()], + count: 1, + truncated: true, + }); + + const state = store.getState().failedInbox.byWorkspaceId.w1; + expect(state.status).toBe("ready"); + expect(state.count).toBe(1); + expect(state.rows).toHaveLength(1); + expect(state.truncated).toBe(true); + }); + + // AC-UI-INBOX-FAILED-001.26: a fast workspace switch or overlapping refresh + // trigger must not let a slower, older response overwrite a newer one. + it("drops a page response whose generation was superseded by a newer read", () => { + const store = newStore(); + const staleGeneration = store.getState().beginFailedInboxRead("w1"); + const freshGeneration = store.getState().beginFailedInboxRead("w1"); + expect(freshGeneration).not.toBe(staleGeneration); + + store.getState().setFailedInboxPage("w1", freshGeneration, { + rows: [row({ task_id: "fresh" })], + count: 1, + truncated: false, + }); + // The stale response lands after the fresh one and must be dropped. + store.getState().setFailedInboxPage("w1", staleGeneration, { + rows: [row({ task_id: "stale" })], + count: 1, + truncated: false, + }); + + const state = store.getState().failedInbox.byWorkspaceId.w1; + expect(state.rows[0].task_id).toBe("fresh"); + }); + + it("clears rows and count in the same update on a failed read that follows a successful one", () => { + const store = newStore(); + const okGeneration = store.getState().beginFailedInboxRead("w1"); + store.getState().setFailedInboxPage("w1", okGeneration, { + rows: [row()], + count: 1, + truncated: false, + }); + + const failGeneration = store.getState().beginFailedInboxRead("w1"); + store.getState().setFailedInboxError("w1", failGeneration); + + const state = store.getState().failedInbox.byWorkspaceId.w1; + expect(state.status).toBe("error"); + expect(state.rows).toHaveLength(0); + expect(state.count).toBe(0); + }); + + it("drops a stale error response the same way it drops a stale page response", () => { + const store = newStore(); + const staleGeneration = store.getState().beginFailedInboxRead("w1"); + const freshGeneration = store.getState().beginFailedInboxRead("w1"); + store.getState().setFailedInboxPage("w1", freshGeneration, { + rows: [row()], + count: 1, + truncated: false, + }); + + store.getState().setFailedInboxError("w1", staleGeneration); + + const state = store.getState().failedInbox.byWorkspaceId.w1; + expect(state.status).toBe("ready"); + expect(state.count).toBe(1); + }); + + it("keys generations independently per workspace", () => { + const store = newStore(); + const genW1 = store.getState().beginFailedInboxRead("w1"); + const genW2 = store.getState().beginFailedInboxRead("w2"); + + store.getState().setFailedInboxPage("w2", genW2, { + rows: [row({ workspace_id: "w2" })], + count: 1, + truncated: false, + }); + store.getState().setFailedInboxPage("w1", genW1, { + rows: [], + count: 0, + truncated: false, + }); + + expect(store.getState().failedInbox.byWorkspaceId.w1.status).toBe("ready"); + expect(store.getState().failedInbox.byWorkspaceId.w2.rows).toHaveLength(1); + }); +}); + +describe("failed-inbox slice: response identity", () => { + it("drops a response after the active workspace changes", () => { + const store = newStore(); + store.getState().setActiveWorkspace("w1"); + const generation = store.getState().beginFailedInboxRead("w1"); + + store.getState().setActiveWorkspace("w2"); + store.getState().setFailedInboxPage("w1", generation, { + rows: [row()], + count: 1, + truncated: false, + }); + + expect(store.getState().failedInbox.byWorkspaceId.w1.status).toBe("loading"); + expect(store.getState().failedInbox.byWorkspaceId.w1.rows).toHaveLength(0); + }); + + it("drops a response after switching away and back to the same workspace", () => { + const store = newStore(); + store.getState().setActiveWorkspace("w1"); + const generation = store.getState().beginFailedInboxRead("w1"); + + store.getState().setActiveWorkspace("w2"); + store.getState().setActiveWorkspace("w1"); + store.getState().setFailedInboxError("w1", generation); + + expect(store.getState().failedInbox.byWorkspaceId.w1.status).toBe("loading"); + expect(store.getState().failedInbox.byWorkspaceId.w1.rows).toHaveLength(0); + }); +}); + +describe("failed-inbox slice: workspace-change status transitions (AC .16)", () => { + // The count "shall remain rendered" across a background refresh -- only a + // never-read or previously-errored workspace should show as not-yet-known + // while a request is in flight. + it("keeps a workspace's status ready while a background refresh of already-loaded data is in flight", () => { + const store = newStore(); + const firstGeneration = store.getState().beginFailedInboxRead("w1"); + store.getState().setFailedInboxPage("w1", firstGeneration, { + rows: [row()], + count: 1, + truncated: false, + }); + + store.getState().beginFailedInboxRead("w1"); + + const state = store.getState().failedInbox.byWorkspaceId.w1; + expect(state.status).toBe("ready"); + expect(state.rows).toHaveLength(1); + expect(state.count).toBe(1); + }); + + // "Changing the active workspace shall clear it until a response for the + // new one is applied" -- returning to a workspace visited earlier in the + // same session must not show its stale cache. Drives `setActiveWorkspace` + // (the real signal a switch produces) rather than inferring a switch from + // the `beginFailedInboxRead` call sequence, since the guard now keys off + // `workspaces.activeIdRevision`, not a copy local to this slice. + it("clears a workspace's stale ready state when the operator switches back to it", () => { + const store = newStore(); + store.getState().setActiveWorkspace("w1"); + const w1Generation = store.getState().beginFailedInboxRead("w1"); + store.getState().setFailedInboxPage("w1", w1Generation, { + rows: [row()], + count: 1, + truncated: false, + }); + expect(store.getState().failedInbox.byWorkspaceId.w1.status).toBe("ready"); + + // Switch away to w2 (Inbox stays mounted and reads it too), then back to + // w1, before w1's new read resolves. + store.getState().setActiveWorkspace("w2"); + store.getState().beginFailedInboxRead("w2"); + store.getState().setActiveWorkspace("w1"); + store.getState().beginFailedInboxRead("w1"); + + const state = store.getState().failedInbox.byWorkspaceId.w1; + expect(state.status).not.toBe("ready"); + expect(state.rows).toHaveLength(0); + expect(state.count).toBe(0); + }); + + // Regression coverage for the gap the switch-back fix above did not close: + // `beginFailedInboxRead` is only ever called while the Inbox is mounted, so + // a workspace switch that happens while it is closed must still be caught + // the next time this workspace is read -- it must not depend on the Inbox + // having been open (and reading) for every intermediate workspace. + it("clears a workspace's stale ready state when the switch away and back happens with no read in between", () => { + const store = newStore(); + store.getState().setActiveWorkspace("w1"); + const w1Generation = store.getState().beginFailedInboxRead("w1"); + store.getState().setFailedInboxPage("w1", w1Generation, { + rows: [row()], + count: 1, + truncated: false, + }); + expect(store.getState().failedInbox.byWorkspaceId.w1.status).toBe("ready"); + + // The active workspace changes twice (e.g. via the sidebar switcher on + // another page) with the Inbox closed throughout -- nothing here calls + // `beginFailedInboxRead` for w2 at all. + store.getState().setActiveWorkspace("w2"); + store.getState().setActiveWorkspace("w1"); + + // The Inbox reopens on w1 and its mount effect reads it. + const state0 = store.getState().failedInbox.byWorkspaceId.w1; + expect(state0.status).toBe("ready"); // still the stale cache, pre-read + store.getState().beginFailedInboxRead("w1"); + + const state = store.getState().failedInbox.byWorkspaceId.w1; + expect(state.status).not.toBe("ready"); + expect(state.rows).toHaveLength(0); + expect(state.count).toBe(0); + }); +}); diff --git a/apps/web/lib/state/slices/failed-inbox/failed-inbox-slice.ts b/apps/web/lib/state/slices/failed-inbox/failed-inbox-slice.ts new file mode 100644 index 0000000000..b3a3d22883 --- /dev/null +++ b/apps/web/lib/state/slices/failed-inbox/failed-inbox-slice.ts @@ -0,0 +1,118 @@ +import type { StateCreator } from "zustand"; +import type { AppState } from "../../app-state-types"; +import type { FailedInboxSlice, FailedInboxSliceState, FailedInboxWorkspaceState } from "./types"; + +export const defaultFailedInboxState: FailedInboxSliceState = { + failedInbox: { + byWorkspaceId: {}, + generationByWorkspaceId: {}, + readRevisionByWorkspaceId: {}, + }, +}; + +const emptyWorkspaceState = (readAtWorkspaceRevision = 0): FailedInboxWorkspaceState => ({ + rows: [], + count: 0, + truncated: false, + status: "idle", + appliedGeneration: 0, + readAtWorkspaceRevision, +}); + +// Typed against the full `AppState` (the zustand slices-pattern shape), not +// this slice's own narrower type -- that keeps `set`/`get` structurally +// identical to the root store's, so composing this slice in store.ts needs +// no `as any` escape (ARCH-FRONTEND-ROOT-STATE-CAST). +type ImmerSet = Parameters< + StateCreator +>[0]; +type ImmerGet = Parameters< + StateCreator +>[1]; + +/** + * Takes `get` (unlike the needs-you-inbox slice) so a genuine workspace + * switch can be detected even when it happened while nothing in this slice + * was called to observe it -- see `beginFailedInboxRead`. + */ +export const createFailedInboxSlice = (set: ImmerSet, get: ImmerGet): FailedInboxSlice => ({ + ...defaultFailedInboxState, + + beginFailedInboxRead: (workspaceId) => { + // Read from `workspaces.activeIdRevision`, the app-wide source of truth + // bumped on every actual workspace switch, rather than a copy local to + // this slice: this slice's own actions only ever run while the Inbox + // page is mounted, so a locally-shadowed "last workspace seen" value + // cannot detect a switch that happened while the Inbox was closed. + const currentRevision = get().workspaces.activeIdRevision ?? 0; + let generation = 0; + set((draft) => { + const next = (draft.failedInbox.generationByWorkspaceId[workspaceId] ?? 0) + 1; + draft.failedInbox.generationByWorkspaceId[workspaceId] = next; + draft.failedInbox.readRevisionByWorkspaceId[workspaceId] = currentRevision; + const existing = draft.failedInbox.byWorkspaceId[workspaceId] ?? emptyWorkspaceState(); + const isStaleWorkspaceRevision = existing.readAtWorkspaceRevision !== currentRevision; + // A same-workspace refresh trigger (periodic tick, tab change, + // foreground return) leaves already-`ready` data alone, so it cannot + // hide the badge behind rows it is still showing. Any read whose + // workspace revision has moved on always resets to `loading` and + // clears any cache from an earlier visit, even if that visit ended + // `ready` -- whether the switch happened while this workspace's tab + // was open or while the Inbox was closed entirely. + if (isStaleWorkspaceRevision || existing.status !== "ready") { + draft.failedInbox.byWorkspaceId[workspaceId] = { + ...emptyWorkspaceState(currentRevision), + status: "loading", + }; + } + generation = next; + }); + return generation; + }, + + setFailedInboxPage: (workspaceId, generation, page) => + set((draft) => { + if (draft.failedInbox.generationByWorkspaceId[workspaceId] !== generation) return; + const requestRevision = draft.failedInbox.readRevisionByWorkspaceId[workspaceId] ?? 0; + const activeWorkspaceId = get().workspaces.activeId; + const activeRevision = get().workspaces.activeIdRevision ?? 0; + if ( + (activeWorkspaceId !== null && activeWorkspaceId !== workspaceId) || + activeRevision !== requestRevision + ) { + return; + } + draft.failedInbox.byWorkspaceId[workspaceId] = { + rows: page.rows, + count: page.count, + truncated: page.truncated, + status: "ready", + appliedGeneration: generation, + readAtWorkspaceRevision: requestRevision, + }; + }), + + setFailedInboxError: (workspaceId, generation) => + set((draft) => { + if (draft.failedInbox.generationByWorkspaceId[workspaceId] !== generation) return; + const requestRevision = draft.failedInbox.readRevisionByWorkspaceId[workspaceId] ?? 0; + const activeWorkspaceId = get().workspaces.activeId; + const activeRevision = get().workspaces.activeIdRevision ?? 0; + if ( + (activeWorkspaceId !== null && activeWorkspaceId !== workspaceId) || + activeRevision !== requestRevision + ) { + return; + } + // A failed read clears the rows it was replacing in the SAME update + // (design-01#Failure-and-recovery) -- stale rows over an absent count + // is a disagreement the count and the list must never show. + draft.failedInbox.byWorkspaceId[workspaceId] = { + ...emptyWorkspaceState( + draft.failedInbox.byWorkspaceId[workspaceId]?.readAtWorkspaceRevision ?? 0, + ), + status: "error", + appliedGeneration: generation, + }; + }), +}); diff --git a/apps/web/lib/state/slices/failed-inbox/selectors.test.ts b/apps/web/lib/state/slices/failed-inbox/selectors.test.ts new file mode 100644 index 0000000000..dd5e0437ea --- /dev/null +++ b/apps/web/lib/state/slices/failed-inbox/selectors.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from "vitest"; +import { createAppStore } from "@/lib/state/store"; +import { + selectFailedInboxCount, + selectFailedInboxCountIsKnown, + selectFailedInboxRows, + selectFailedInboxStatus, + selectFailedInboxTruncated, +} from "./selectors"; + +const WORKSPACE_ID = "workspace-1"; + +function storeWithActiveWorkspace() { + const store = createAppStore(); + store.setState((state) => ({ + ...state, + workspaces: { ...state.workspaces, activeId: WORKSPACE_ID }, + })); + return store; +} + +describe("failed-inbox selectors", () => { + it("reports no count and unknown status before any read (AC .16, .23)", () => { + const store = storeWithActiveWorkspace(); + const state = store.getState(); + expect(selectFailedInboxCount(state)).toBe(0); + expect(selectFailedInboxCountIsKnown(state)).toBe(false); + expect(selectFailedInboxStatus(state)).toBe("idle"); + expect(selectFailedInboxRows(state)).toEqual([]); + }); + + it("reports no active workspace as the empty workspace state", () => { + const store = createAppStore(); + const state = store.getState(); + expect(selectFailedInboxCount(state)).toBe(0); + expect(selectFailedInboxCountIsKnown(state)).toBe(false); + }); + + it("reflects an applied page: known count, rows, and truncation", () => { + const store = storeWithActiveWorkspace(); + const generation = store.getState().beginFailedInboxRead(WORKSPACE_ID); + store.getState().setFailedInboxPage(WORKSPACE_ID, generation, { + rows: [ + { + task_id: "t1", + title: "Task", + workspace_id: WORKSPACE_ID, + origin: "manual", + failure_instant: "2026-09-14T01:00:00Z", + reason: "boom", + }, + ], + count: 1, + truncated: true, + }); + + const state = store.getState(); + expect(selectFailedInboxCount(state)).toBe(1); + expect(selectFailedInboxCountIsKnown(state)).toBe(true); + expect(selectFailedInboxTruncated(state)).toBe(true); + expect(selectFailedInboxRows(state)).toHaveLength(1); + }); + + // AC .23: the count is not "known" while the last read for the active + // workspace failed, even though a generation was applied. + it("reports the count as unknown after a failed read", () => { + const store = storeWithActiveWorkspace(); + const generation = store.getState().beginFailedInboxRead(WORKSPACE_ID); + store.getState().setFailedInboxError(WORKSPACE_ID, generation); + + const state = store.getState(); + expect(selectFailedInboxStatus(state)).toBe("error"); + expect(selectFailedInboxCountIsKnown(state)).toBe(false); + expect(selectFailedInboxCount(state)).toBe(0); + }); +}); diff --git a/apps/web/lib/state/slices/failed-inbox/selectors.ts b/apps/web/lib/state/slices/failed-inbox/selectors.ts new file mode 100644 index 0000000000..d686c5969d --- /dev/null +++ b/apps/web/lib/state/slices/failed-inbox/selectors.ts @@ -0,0 +1,48 @@ +import type { AppState } from "@/lib/state/store"; +import type { FailedInboxRow } from "@/lib/types/failed-inbox"; +import type { FailedInboxWorkspaceState } from "./types"; + +const EMPTY_ROWS: FailedInboxRow[] = []; + +const EMPTY_WORKSPACE_STATE: FailedInboxWorkspaceState = { + rows: EMPTY_ROWS, + count: 0, + truncated: false, + status: "idle", + appliedGeneration: 0, + readAtWorkspaceRevision: 0, +}; + +function selectActiveWorkspaceState(state: AppState): FailedInboxWorkspaceState { + const workspaceId = state.workspaces.activeId; + if (!workspaceId) return EMPTY_WORKSPACE_STATE; + return state.failedInbox.byWorkspaceId[workspaceId] ?? EMPTY_WORKSPACE_STATE; +} + +export function selectFailedInboxRows(state: AppState): FailedInboxRow[] { + return selectActiveWorkspaceState(state).rows; +} + +export function selectFailedInboxStatus(state: AppState) { + return selectActiveWorkspaceState(state).status; +} + +export function selectFailedInboxTruncated(state: AppState): boolean { + return selectActiveWorkspaceState(state).truncated; +} + +/** The Failed tab's own badge count (AC-UI-INBOX-FAILED-001.16): absent + * workspace or absent state renders no badge, not a zero -- distinguishing + * "not read yet" from "confirmed empty" at the presentation layer (callers + * gate visibility separately). */ +export function selectFailedInboxCount(state: AppState): number { + return selectActiveWorkspaceState(state).count; +} + +/** True only once a successful read has been applied for the active + * workspace -- neither "never read" nor "the last read failed" counts as + * known, since AC-UI-INBOX-FAILED-001.23's clause must be omitted in both + * cases rather than asserting presence or absence. */ +export function selectFailedInboxCountIsKnown(state: AppState): boolean { + return selectActiveWorkspaceState(state).status === "ready"; +} diff --git a/apps/web/lib/state/slices/failed-inbox/types.ts b/apps/web/lib/state/slices/failed-inbox/types.ts new file mode 100644 index 0000000000..4d6dbdaa91 --- /dev/null +++ b/apps/web/lib/state/slices/failed-inbox/types.ts @@ -0,0 +1,50 @@ +import type { FailedInboxRow } from "@/lib/types/failed-inbox"; + +export type FailedInboxReadStatus = "idle" | "loading" | "ready" | "error"; + +export type FailedInboxWorkspaceState = { + rows: FailedInboxRow[]; + count: number; + truncated: boolean; + status: FailedInboxReadStatus; + // The generation of the last response actually applied to this workspace's + // rows, for the stale-response guard (AC-UI-INBOX-FAILED-001.26). + appliedGeneration: number; + // `workspaces.activeIdRevision` as of the read that produced this entry's + // current status. Compared against the live revision (not a locally-shadowed + // copy) so a workspace switch that happens while nothing here is mounted to + // observe it is still detected the next time this workspace is read. + readAtWorkspaceRevision: number; +}; + +export type FailedInboxSliceState = { + failedInbox: { + 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. + // Keyed on workspace only, deliberately never on the selected tab + // (design-01#Control-flow). + generationByWorkspaceId: Record; + // Active-workspace revision captured when each request was issued. A + // response for an inactive or re-selected workspace must not mutate its + // cache after the selection boundary has moved. + readRevisionByWorkspaceId: Record; + }; +}; + +export type FailedInboxSliceActions = { + /** Bumps and returns the new request generation for a workspace read. */ + beginFailedInboxRead: (workspaceId: string) => number; + /** Applies a successful page read if its generation is still current. */ + setFailedInboxPage: ( + workspaceId: string, + generation: number, + page: { rows: FailedInboxRow[]; count: number; truncated: boolean }, + ) => void; + /** Applies a failed read if its generation is still current -- clears rows + * and count in the same update (design-01#Failure-and-recovery). */ + setFailedInboxError: (workspaceId: string, generation: number) => void; +}; + +export type FailedInboxSlice = FailedInboxSliceState & FailedInboxSliceActions; diff --git a/apps/web/lib/state/slices/index.ts b/apps/web/lib/state/slices/index.ts index d4df550361..5be9457fdc 100644 --- a/apps/web/lib/state/slices/index.ts +++ b/apps/web/lib/state/slices/index.ts @@ -24,6 +24,7 @@ export { createNeedsYouInboxSlice, defaultNeedsYouInboxState, } from "./needs-you-inbox/needs-you-inbox-slice"; +export { createFailedInboxSlice, defaultFailedInboxState } from "./failed-inbox/failed-inbox-slice"; // Export types export type { KanbanSlice, KanbanSliceState, KanbanSliceActions } from "./kanban/types"; @@ -104,6 +105,13 @@ export type { NeedsYouInboxReadStatus, NeedsYouInboxBootSeed, } from "./needs-you-inbox/types"; +export type { + FailedInboxSlice, + FailedInboxSliceActions, + FailedInboxSliceState, + FailedInboxWorkspaceState, + FailedInboxReadStatus, +} from "./failed-inbox/types"; // Re-export commonly used types from each domain export type { diff --git a/apps/web/lib/state/slices/workspace/types.ts b/apps/web/lib/state/slices/workspace/types.ts index 4da93df5ee..245ffcf651 100644 --- a/apps/web/lib/state/slices/workspace/types.ts +++ b/apps/web/lib/state/slices/workspace/types.ts @@ -28,6 +28,14 @@ export type WorkspaceState = { updated_at: string; }>; activeId: string | null; + // Bumped every time `activeId` is set to a different workspace (never on a + // re-select of the same one). Optional so every existing partial + // `workspaces` object (SSR page state, WS hydration payloads) still + // type-checks unchanged; absent means "never bumped", same as 0. Lets a + // consumer that only observes state intermittently -- e.g. a slice scoped + // to one route -- detect that the active workspace changed while it was + // not watching, without itself subscribing app-wide. + activeIdRevision?: number; }; export type RepositoriesState = { diff --git a/apps/web/lib/state/slices/workspace/workspace-slice.ts b/apps/web/lib/state/slices/workspace/workspace-slice.ts index f2ffc26b8c..ffc2d18531 100644 --- a/apps/web/lib/state/slices/workspace/workspace-slice.ts +++ b/apps/web/lib/state/slices/workspace/workspace-slice.ts @@ -3,7 +3,7 @@ import type { WorkspaceSlice, WorkspaceSliceState } from "./types"; import type { RepositoryBranchPolicy, RepositorySet } from "@/lib/types/http"; export const defaultWorkspaceState: WorkspaceSliceState = { - workspaces: { items: [], activeId: null }, + workspaces: { items: [], activeId: null, activeIdRevision: 0 }, repositories: { itemsByWorkspaceId: {}, loadingByWorkspaceId: {}, loadedByWorkspaceId: {} }, repositorySets: { itemsByWorkspaceId: {}, @@ -48,6 +48,7 @@ export const createWorkspaceSlice: StateCreator< (get() as WorkspaceSliceWithKanbanReset).resetKanbanWorkspaceContext?.(); set((draft) => { draft.workspaces.activeId = workspaceId; + draft.workspaces.activeIdRevision = (draft.workspaces.activeIdRevision ?? 0) + 1; }); }, setWorkspaces: (workspaces) => diff --git a/apps/web/lib/state/store.ts b/apps/web/lib/state/store.ts index c0d6556833..e4e07c0a2e 100644 --- a/apps/web/lib/state/store.ts +++ b/apps/web/lib/state/store.ts @@ -26,6 +26,7 @@ import { createPluginsSlice, createReviewSlice, createNeedsYouInboxSlice, + createFailedInboxSlice, } from "./slices"; // Re-export all types from slices for backwards compatibility. @@ -83,6 +84,7 @@ export function createAppStore(initialState?: HydrationState) { // eslint-disable-next-line @typescript-eslint/no-explicit-any ...createReviewSlice(set as any), ...createNeedsYouInboxSlice(set), + ...createFailedInboxSlice(set, get), // 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/threads/thread-session-status.test.ts b/apps/web/lib/threads/thread-session-status.test.ts index 527e25e219..ad2b9eda90 100644 --- a/apps/web/lib/threads/thread-session-status.test.ts +++ b/apps/web/lib/threads/thread-session-status.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from "vitest"; import type { TaskSession } from "@/lib/types/http"; import { sessionId, taskId } from "@/lib/types/ids"; -import { resolveThreadColumnStatus, resolveThreadSessionStatus } from "./thread-session-status"; +import { + FAILED_TASK_STATUS, + resolveThreadColumnStatus, + resolveThreadSessionStatus, +} from "./thread-session-status"; function session(overrides: Partial = {}): TaskSession { return { @@ -83,3 +87,16 @@ describe("resolveThreadColumnStatus", () => { ).toMatchObject({ kind: "needs-you", hasAttention: true }); }); }); + +// AC-UI-INBOX-FAILED-001.20: the Failed tab has no session to resolve status +// from, so it reaches the shared vocabulary's "failed" entry directly rather +// than fabricating a stand-in session for resolveThreadSessionStatus. +describe("FAILED_TASK_STATUS", () => { + it("is exactly the same entry resolveThreadSessionStatus returns for a failed session", () => { + expect(FAILED_TASK_STATUS).toEqual(resolveThreadSessionStatus(session({ state: "FAILED" }))); + }); + + it("carries no attention flag", () => { + expect(FAILED_TASK_STATUS.hasAttention).toBe(false); + }); +}); diff --git a/apps/web/lib/threads/thread-session-status.ts b/apps/web/lib/threads/thread-session-status.ts index 5964a4ee7e..7c0a10b386 100644 --- a/apps/web/lib/threads/thread-session-status.ts +++ b/apps/web/lib/threads/thread-session-status.ts @@ -52,6 +52,15 @@ const STATUS = { created: { kind: "created", labelKey: "threads:statusNotStarted", hasAttention: false }, } satisfies Record; +// The Inbox Failed tab has no session to resolve status from -- every row is +// defined by the task's own terminal failed state (AC-UI-INBOX-FAILED-001.6, +// .20). Exporting the existing `failed` entry lets that tab reach it +// directly instead of fabricating a stand-in session for +// resolveThreadSessionStatus, which would reintroduce the +// pending_action-first precedence AC-UI-INBOX-FAILED-001.20a excludes. This +// is a wider export of an existing entry, not a new or duplicated one. +export const FAILED_TASK_STATUS: ThreadStatus = STATUS.failed; + const REVIEW_READY: ThreadStatus = { kind: "review-ready", labelKey: "threads:statusReadyForReview", diff --git a/apps/web/lib/types/failed-inbox.ts b/apps/web/lib/types/failed-inbox.ts new file mode 100644 index 0000000000..9858ee53a4 --- /dev/null +++ b/apps/web/lib/types/failed-inbox.ts @@ -0,0 +1,21 @@ +// Inbox Failed tab wire types +// (docs/specs/ui/system-design/inbox-failed-bucket-01.md#Data-and-contracts). +// Mirrors apps/backend/internal/failedinbox/handlers.go's +// failedInboxRow / failedInboxListResponse. + +export type FailedInboxRow = { + task_id: string; + title: string; + workspace_id: string; + origin: string; + // Omitted (never null, never a zero instant) when unresolvable -- how the + // client tells an unresolved failure instant from a real one. + failure_instant?: string; + reason: string; +}; + +export type FailedInboxPage = { + rows: FailedInboxRow[]; + count: number; + truncated: boolean; +}; diff --git a/apps/web/lib/ws/handlers/workspaces.test.ts b/apps/web/lib/ws/handlers/workspaces.test.ts index bf1ecedea6..0490457ccb 100644 --- a/apps/web/lib/ws/handlers/workspaces.test.ts +++ b/apps/web/lib/ws/handlers/workspaces.test.ts @@ -14,6 +14,7 @@ function dispatch(handlers: WsHandlers, type: string, payload: unknown) { } type WorkspaceItem = AppState["workspaces"]["items"][number]; +const SECOND_WORKSPACE_ID = "workspace-2"; function storeWith(items: WorkspaceItem[]): StoreApi { return createStore( @@ -73,6 +74,16 @@ describe("workspace.updated placement", () => { expect(store.getState().workspaces.items[0].unit_id).toBe("unit-new"); }); + + it("bumps the active workspace revision when the first workspace becomes active", () => { + const store = storeWith([]); + const handlers = registerWorkspacesHandlers(store); + + dispatch(handlers, "workspace.created", { id: "ws-2", name: "Runtime" }); + + expect(store.getState().workspaces.activeId).toBe("ws-2"); + expect(store.getState().workspaces.activeIdRevision).toBe(1); + }); }); describe("workspace.deleted queue cleanup", () => { @@ -82,7 +93,7 @@ describe("workspace.deleted queue cleanup", () => { const clearQueueStatus = vi.fn(); let state = { workspaces: { - items: [{ id: WORKSPACE_ID }, { id: "workspace-2" }], + items: [{ id: WORKSPACE_ID }, { id: SECOND_WORKSPACE_ID }], activeId: WORKSPACE_ID, }, workflows: { items: [], activeId: null }, @@ -119,4 +130,60 @@ describe("workspace.deleted queue cleanup", () => { expect(clearQueueStatus).toHaveBeenCalledOnce(); expect(clearQueueStatus).toHaveBeenCalledWith("session-1"); }); + + it("bumps the active workspace revision when deletion selects a fallback", () => { + const clearQueueStatus = vi.fn(); + const first = { ...workspace(), id: WORKSPACE_ID }; + const second = { ...workspace(), id: SECOND_WORKSPACE_ID }; + const store = createStore( + () => + ({ + workspaces: { + items: [first, second], + activeId: WORKSPACE_ID, + activeIdRevision: 4, + }, + workflows: { items: [], activeId: null }, + kanban: { workflowId: null, steps: [], tasks: [] }, + kanbanMulti: { snapshots: {} }, + taskSessions: { items: {} }, + clearQueueStatus, + }) as unknown as AppState, + ); + + registerWorkspacesHandlers(store)["workspace.deleted"]!({ + payload: { id: WORKSPACE_ID }, + } as never); + + expect(store.getState().workspaces.activeId).toBe(SECOND_WORKSPACE_ID); + expect(store.getState().workspaces.activeIdRevision).toBe(5); + }); + + it("preserves the active workspace revision when deletion leaves the selection unchanged", () => { + const clearQueueStatus = vi.fn(); + const first = { ...workspace(), id: WORKSPACE_ID }; + const second = { ...workspace(), id: SECOND_WORKSPACE_ID }; + const store = createStore( + () => + ({ + workspaces: { + items: [first, second], + activeId: SECOND_WORKSPACE_ID, + activeIdRevision: 4, + }, + workflows: { items: [], activeId: null }, + kanban: { workflowId: null, steps: [], tasks: [] }, + kanbanMulti: { snapshots: {} }, + taskSessions: { items: {} }, + clearQueueStatus, + }) as unknown as AppState, + ); + + registerWorkspacesHandlers(store)["workspace.deleted"]!({ + payload: { id: WORKSPACE_ID }, + } as never); + + expect(store.getState().workspaces.activeId).toBe(SECOND_WORKSPACE_ID); + expect(store.getState().workspaces.activeIdRevision).toBe(4); + }); }); diff --git a/apps/web/lib/ws/handlers/workspaces.ts b/apps/web/lib/ws/handlers/workspaces.ts index 7b05b85f50..c2c76b547e 100644 --- a/apps/web/lib/ws/handlers/workspaces.ts +++ b/apps/web/lib/ws/handlers/workspaces.ts @@ -4,6 +4,11 @@ import type { WsHandlers } from "@/lib/ws/handlers/types"; type WorkspaceItem = WorkspaceState["items"][number]; +function nextActiveWorkspaceRevision(workspaces: WorkspaceState, activeId: string | null): number { + const revision = workspaces.activeIdRevision ?? 0; + return workspaces.activeId === activeId ? revision : revision + 1; +} + function handleWorkspaceDeleted(store: StoreApi, workspaceId: string): void { const currentState = store.getState(); const workspaceTaskIds = new Set( @@ -26,10 +31,11 @@ function handleWorkspaceDeleted(store: StoreApi, workspaceId: string): state.workspaces.activeId === workspaceId ? (items[0]?.id ?? null) : state.workspaces.activeId; + const activeIdRevision = nextActiveWorkspaceRevision(state.workspaces, activeId); const clearBoards = state.workspaces.activeId === workspaceId; return { ...state, - workspaces: { items, activeId }, + workspaces: { items, activeId, activeIdRevision }, workflows: clearBoards ? { items: [], activeId: null } : state.workflows, kanban: clearBoards ? { workflowId: null, steps: [], tasks: [] } : state.kanban, }; @@ -61,11 +67,13 @@ export function registerWorkspacesHandlers(store: StoreApi): WsHandler ) : [newWorkspace, ...state.workspaces.items]; const activeId = state.workspaces.activeId ?? payload.id; + const activeIdRevision = nextActiveWorkspaceRevision(state.workspaces, activeId); return { ...state, workspaces: { items, activeId, + activeIdRevision, }, }; }); diff --git a/apps/web/src/locales/en/failedInbox.json b/apps/web/src/locales/en/failedInbox.json new file mode 100644 index 0000000000..1ff7debc59 --- /dev/null +++ b/apps/web/src/locales/en/failedInbox.json @@ -0,0 +1,18 @@ +{ + "tabLabel": "Failed", + "openTask": "Open task", + "reasonUnknown": "No failure reason was recorded.", + "unknownFailureTime": "Failure time unknown", + "originAgentCreated": "Created by an agent", + "originRoutine": "Created by a routine", + "originOnboarding": "Created during onboarding", + "originAutomationRun": "Automation run", + "originAutomationTask": "Created by automation", + "originGeneric": "Automated", + "emptyTitle": "Nothing has failed in {{workspace}}.", + "emptyTitleUnnamedWorkspace": "Nothing has failed in this workspace.", + "emptyDescription": "Failed tasks show up here for triage. They are never counted toward the sidebar total.", + "errorTitle": "Could not load failed tasks", + "errorDescription": "Something went wrong loading the tasks that failed in this workspace. This will retry automatically.", + "truncatedNotice": "Showing the first failed tasks. More are waiting." +} diff --git a/apps/web/src/locales/en/needsYouInbox.json b/apps/web/src/locales/en/needsYouInbox.json index fa949056e3..239ba3464a 100644 --- a/apps/web/src/locales/en/needsYouInbox.json +++ b/apps/web/src/locales/en/needsYouInbox.json @@ -31,5 +31,9 @@ "openTask": "Open task", "questionCount_one": "{{count}} question in this bundle", "questionCount_other": "{{count}} questions in this bundle", - "emptyTitleUnnamedWorkspace": "Nothing is waiting on you in this workspace." + "emptyTitleUnnamedWorkspace": "Nothing is waiting on you in this workspace.", + "tabLabel": "Needs you", + "failedClauseSome_one": "This list does not count failed tasks. {{count}} task has also failed. See the Failed tab.", + "failedClauseSome_other": "This list does not count failed tasks. {{count}} tasks have also failed. See the Failed tab.", + "failedClauseNone": "This list does not count failed tasks. None have failed in this workspace right now; check the Failed tab any time." } diff --git a/apps/web/src/locales/pseudo/failedInbox.json b/apps/web/src/locales/pseudo/failedInbox.json new file mode 100644 index 0000000000..ae8d4b135a --- /dev/null +++ b/apps/web/src/locales/pseudo/failedInbox.json @@ -0,0 +1,18 @@ +{ + "tabLabel": "Ƒàĩĺēď", + "openTask": "Ōƥēń ţàśķ", + "reasonUnknown": "Ńō ƒàĩĺũŕē ŕēàśōń ŵàś ŕēćōŕďēď.", + "unknownFailureTime": "Ƒàĩĺũŕē ţĩḿē ũńķńōŵń", + "originAgentCreated": "Ćŕēàţēď ƀŷ àń àĝēńţ", + "originRoutine": "Ćŕēàţēď ƀŷ à ŕōũţĩńē", + "originOnboarding": "Ćŕēàţēď ďũŕĩńĝ ōńƀōàŕďĩńĝ", + "originAutomationRun": "Àũţōḿàţĩōń ŕũń", + "originAutomationTask": "Ćŕēàţēď ƀŷ àũţōḿàţĩōń", + "originGeneric": "Àũţōḿàţēď", + "emptyTitle": "Ńōţĥĩńĝ ĥàś ƒàĩĺēď ĩń {{workspace}}.", + "emptyTitleUnnamedWorkspace": "Ńōţĥĩńĝ ĥàś ƒàĩĺēď ĩń ţĥĩś ŵōŕķśƥàćē.", + "emptyDescription": "Ƒàĩĺēď ţàśķś śĥōŵ ũƥ ĥēŕē ƒōŕ ţŕĩàĝē. Ţĥēŷ àŕē ńēvēŕ ćōũńţēď ţōŵàŕď ţĥē śĩďēƀàŕ ţōţàĺ.", + "errorTitle": "Ćōũĺď ńōţ ĺōàď ƒàĩĺēď ţàśķś", + "errorDescription": "Śōḿēţĥĩńĝ ŵēńţ ŵŕōńĝ ĺōàďĩńĝ ţĥē ţàśķś ţĥàţ ƒàĩĺēď ĩń ţĥĩś ŵōŕķśƥàćē. Ţĥĩś ŵĩĺĺ ŕēţŕŷ àũţōḿàţĩćàĺĺŷ.", + "truncatedNotice": "Śĥōŵĩńĝ ţĥē ƒĩŕśţ ƒàĩĺēď ţàśķś. Ḿōŕē àŕē ŵàĩţĩńĝ." +} diff --git a/apps/web/src/locales/pseudo/needsYouInbox.json b/apps/web/src/locales/pseudo/needsYouInbox.json index c7538c6b44..cb7664ef29 100644 --- a/apps/web/src/locales/pseudo/needsYouInbox.json +++ b/apps/web/src/locales/pseudo/needsYouInbox.json @@ -31,5 +31,9 @@ "openTask": "Ōƥēń ţàśķ", "questionCount_one": "{{count}} qũēśţĩōń ĩń ţĥĩś ƀũńďĺē", "questionCount_other": "{{count}} qũēśţĩōńś ĩń ţĥĩś ƀũńďĺē", - "emptyTitleUnnamedWorkspace": "Ńōţĥĩńĝ ĩś ŵàĩţĩńĝ ōń ŷōũ ĩń ţĥĩś ŵōŕķśƥàćē." + "emptyTitleUnnamedWorkspace": "Ńōţĥĩńĝ ĩś ŵàĩţĩńĝ ōń ŷōũ ĩń ţĥĩś ŵōŕķśƥàćē.", + "tabLabel": "Ńēēďś ŷōũ", + "failedClauseSome_one": "Ţĥĩś ĺĩśţ ďōēś ńōţ ćōũńţ ƒàĩĺēď ţàśķś. {{count}} ţàśķ ĥàś àĺśō ƒàĩĺēď. Śēē ţĥē Ƒàĩĺēď ţàƀ.", + "failedClauseSome_other": "Ţĥĩś ĺĩśţ ďōēś ńōţ ćōũńţ ƒàĩĺēď ţàśķś. {{count}} ţàśķś ĥàvē àĺśō ƒàĩĺēď. Śēē ţĥē Ƒàĩĺēď ţàƀ.", + "failedClauseNone": "Ţĥĩś ĺĩśţ ďōēś ńōţ ćōũńţ ƒàĩĺēď ţàśķś. Ńōńē ĥàvē ƒàĩĺēď ĩń ţĥĩś ŵōŕķśƥàćē ŕĩĝĥţ ńōŵ; ćĥēćķ ţĥē Ƒàĩĺēď ţàƀ àńŷ ţĩḿē." } diff --git a/apps/web/src/locales/pt-pt/failedInbox.json b/apps/web/src/locales/pt-pt/failedInbox.json new file mode 100644 index 0000000000..f1ac33a955 --- /dev/null +++ b/apps/web/src/locales/pt-pt/failedInbox.json @@ -0,0 +1,18 @@ +{ + "tabLabel": "Falhadas", + "openTask": "Abrir tarefa", + "reasonUnknown": "Não foi registado nenhum motivo de falha.", + "unknownFailureTime": "Hora da falha desconhecida", + "originAgentCreated": "Criada por um agente", + "originRoutine": "Criada por uma rotina", + "originOnboarding": "Criada durante a integração", + "originAutomationRun": "Execução de automação", + "originAutomationTask": "Criada por automação", + "originGeneric": "Automatizada", + "emptyTitle": "Nada falhou em {{workspace}}.", + "emptyTitleUnnamedWorkspace": "Nada falhou neste espaço de trabalho.", + "emptyDescription": "As tarefas falhadas aparecem aqui para triagem. Nunca são contadas no total da barra lateral.", + "errorTitle": "Não foi possível carregar as tarefas falhadas", + "errorDescription": "Ocorreu um erro ao carregar as tarefas que falharam neste espaço de trabalho. Isto será repetido automaticamente.", + "truncatedNotice": "A mostrar as primeiras tarefas falhadas. Há mais à espera." +} diff --git a/apps/web/src/locales/pt-pt/needsYouInbox.json b/apps/web/src/locales/pt-pt/needsYouInbox.json index ef2077696c..97316a0f96 100644 --- a/apps/web/src/locales/pt-pt/needsYouInbox.json +++ b/apps/web/src/locales/pt-pt/needsYouInbox.json @@ -31,5 +31,9 @@ "openTask": "Abrir tarefa", "questionCount_one": "{{count}} pergunta neste conjunto", "questionCount_other": "{{count}} perguntas neste conjunto", - "emptyTitleUnnamedWorkspace": "Nada está à sua espera neste espaço de trabalho." + "emptyTitleUnnamedWorkspace": "Nada está à sua espera neste espaço de trabalho.", + "tabLabel": "Precisa de si", + "failedClauseSome_one": "Esta lista não conta tarefas falhadas. {{count}} tarefa também falhou. Consulte o separador Falhadas.", + "failedClauseSome_other": "Esta lista não conta tarefas falhadas. {{count}} tarefas também falharam. Consulte o separador Falhadas.", + "failedClauseNone": "Esta lista não conta tarefas falhadas. Nenhuma falhou neste espaço de trabalho neste momento; pode consultar o separador Falhadas a qualquer momento." } diff --git a/apps/web/src/locales/zh-cn/failedInbox.json b/apps/web/src/locales/zh-cn/failedInbox.json new file mode 100644 index 0000000000..4a6ad33012 --- /dev/null +++ b/apps/web/src/locales/zh-cn/failedInbox.json @@ -0,0 +1,18 @@ +{ + "tabLabel": "失败", + "openTask": "打开任务", + "reasonUnknown": "未记录失败原因。", + "unknownFailureTime": "失败时间未知", + "originAgentCreated": "由代理创建", + "originRoutine": "由例行程序创建", + "originOnboarding": "在引导过程中创建", + "originAutomationRun": "自动化运行", + "originAutomationTask": "由自动化创建", + "originGeneric": "自动化", + "emptyTitle": "{{workspace}} 中没有任何失败事项。", + "emptyTitleUnnamedWorkspace": "此工作区中没有任何失败事项。", + "emptyDescription": "失败的任务会显示在此处以供排查,永远不会计入侧边栏的总数。", + "errorTitle": "无法加载失败的任务", + "errorDescription": "加载此工作区中失败的任务时出错,系统将自动重试。", + "truncatedNotice": "仅显示最先出现的失败任务,还有更多等待处理。" +} diff --git a/apps/web/src/locales/zh-cn/needsYouInbox.json b/apps/web/src/locales/zh-cn/needsYouInbox.json index 57259d563d..a9c209f607 100644 --- a/apps/web/src/locales/zh-cn/needsYouInbox.json +++ b/apps/web/src/locales/zh-cn/needsYouInbox.json @@ -31,5 +31,9 @@ "openTask": "打开任务", "questionCount_one": "此问题集中有 {{count}} 个问题", "questionCount_other": "此问题集中有 {{count}} 个问题", - "emptyTitleUnnamedWorkspace": "此工作区中没有任何事项在等待您处理。" + "emptyTitleUnnamedWorkspace": "此工作区中没有任何事项在等待您处理。", + "tabLabel": "需要你处理", + "failedClauseSome_one": "此列表不计入失败的任务。{{count}} 个任务也失败了,请查看失败标签页。", + "failedClauseSome_other": "此列表不计入失败的任务。{{count}} 个任务也失败了,请查看失败标签页。", + "failedClauseNone": "此列表不计入失败的任务。此工作区目前没有失败的任务,您可以随时查看失败标签页。" } diff --git a/apps/web/src/locales/zh-hk/failedInbox.json b/apps/web/src/locales/zh-hk/failedInbox.json new file mode 100644 index 0000000000..0d86254642 --- /dev/null +++ b/apps/web/src/locales/zh-hk/failedInbox.json @@ -0,0 +1,18 @@ +{ + "tabLabel": "失敗", + "openTask": "開啟任務", + "reasonUnknown": "未記錄失敗原因。", + "unknownFailureTime": "失敗時間未知", + "originAgentCreated": "由代理建立", + "originRoutine": "由例行程序建立", + "originOnboarding": "在引導過程中建立", + "originAutomationRun": "自動化執行", + "originAutomationTask": "由自動化建立", + "originGeneric": "自動化", + "emptyTitle": "{{workspace}} 中沒有任何失敗事項。", + "emptyTitleUnnamedWorkspace": "此工作區中沒有任何失敗事項。", + "emptyDescription": "失敗的任務會顯示在此處以供排查,永遠不會計入側邊欄的總數。", + "errorTitle": "無法載入失敗的任務", + "errorDescription": "載入此工作區中失敗的任務時出錯,系統將自動重試。", + "truncatedNotice": "僅顯示最先出現的失敗任務,還有更多等待處理。" +} diff --git a/apps/web/src/locales/zh-hk/needsYouInbox.json b/apps/web/src/locales/zh-hk/needsYouInbox.json index 9300ee9385..9c89d9e2f3 100644 --- a/apps/web/src/locales/zh-hk/needsYouInbox.json +++ b/apps/web/src/locales/zh-hk/needsYouInbox.json @@ -31,5 +31,9 @@ "openTask": "開啟任務", "questionCount_one": "此問題集中有 {{count}} 個問題", "questionCount_other": "此問題集中有 {{count}} 個問題", - "emptyTitleUnnamedWorkspace": "此工作區中沒有任何事項在等待您處理。" + "emptyTitleUnnamedWorkspace": "此工作區中沒有任何事項在等待您處理。", + "tabLabel": "需要你處理", + "failedClauseSome_one": "此列表不計入失敗的任務。{{count}} 個任務也失敗了,請查看失敗標籤頁。", + "failedClauseSome_other": "此列表不計入失敗的任務。{{count}} 個任務也失敗了,請查看失敗標籤頁。", + "failedClauseNone": "此列表不計入失敗的任務。此工作區目前沒有失敗的任務,您可以隨時查看失敗標籤頁。" } diff --git a/apps/web/src/locales/zh-tw/failedInbox.json b/apps/web/src/locales/zh-tw/failedInbox.json new file mode 100644 index 0000000000..86c8578144 --- /dev/null +++ b/apps/web/src/locales/zh-tw/failedInbox.json @@ -0,0 +1,18 @@ +{ + "tabLabel": "失敗", + "openTask": "開啟任務", + "reasonUnknown": "未記錄失敗原因。", + "unknownFailureTime": "失敗時間未知", + "originAgentCreated": "由代理建立", + "originRoutine": "由例行程式建立", + "originOnboarding": "在引導過程中建立", + "originAutomationRun": "自動化執行", + "originAutomationTask": "由自動化建立", + "originGeneric": "自動化", + "emptyTitle": "{{workspace}} 中沒有任何失敗事項。", + "emptyTitleUnnamedWorkspace": "此工作區中沒有任何失敗事項。", + "emptyDescription": "失敗的任務會顯示在此處以供排查,永遠不會計入側邊欄的總數。", + "errorTitle": "無法載入失敗的任務", + "errorDescription": "載入此工作區中失敗的任務時出錯,系統將自動重試。", + "truncatedNotice": "僅顯示最先出現的失敗任務,還有更多等待處理。" +} diff --git a/apps/web/src/locales/zh-tw/needsYouInbox.json b/apps/web/src/locales/zh-tw/needsYouInbox.json index 3b4bc00a9b..793118ba8f 100644 --- a/apps/web/src/locales/zh-tw/needsYouInbox.json +++ b/apps/web/src/locales/zh-tw/needsYouInbox.json @@ -31,5 +31,9 @@ "openTask": "開啟任務", "questionCount_one": "此問題集中有 {{count}} 個問題", "questionCount_other": "此問題集中有 {{count}} 個問題", - "emptyTitleUnnamedWorkspace": "此工作區中沒有任何事項在等待您處理。" + "emptyTitleUnnamedWorkspace": "此工作區中沒有任何事項在等待您處理。", + "tabLabel": "需要你處理", + "failedClauseSome_one": "此清單不計入失敗的任務。{{count}} 個任務也失敗了,請檢視失敗標籤頁。", + "failedClauseSome_other": "此清單不計入失敗的任務。{{count}} 個任務也失敗了,請檢視失敗標籤頁。", + "failedClauseNone": "此清單不計入失敗的任務。此工作區目前沒有失敗的任務,您可以隨時檢視失敗標籤頁。" } diff --git a/apps/web/src/office-routes.tsx b/apps/web/src/office-routes.tsx index f1785f4ede..6ba07412b9 100644 --- a/apps/web/src/office-routes.tsx +++ b/apps/web/src/office-routes.tsx @@ -193,7 +193,7 @@ type OfficeBootstrapState = { onboardingComplete: boolean | null; }; -function useOfficeRouteBootstrap( +export function useOfficeRouteBootstrap( officeEnabled: boolean, routeWorkspaceId: string | null, ): OfficeBootstrapState { @@ -241,13 +241,21 @@ function useOfficeRouteBootstrap( userSettingsResponse?.settings?.workspace_id ?? null, ); + const workspaceBeforeHydration = store.getState().workspaces.activeId; store.getState().hydrate({ - workspaces: { items: workspaceItems, activeId: activeWorkspaceId }, + workspaces: { items: workspaceItems, activeId: workspaceBeforeHydration }, userSettings: { ...mapUserSettingsResponse(userSettingsResponse), workspaceId: activeWorkspaceId, }, }); + // Routing the actual switch through `setActiveWorkspace` (rather than + // letting `hydrate` overwrite `activeId` directly) keeps + // `activeIdRevision` accurate for consumers that key staleness off it, + // such as the Failed-inbox cache. + if (activeWorkspaceId !== workspaceBeforeHydration) { + store.getState().setActiveWorkspace(activeWorkspaceId); + } // Data loading is not this bootstrap's job: agents, projects, inbox and // meta follow the active workspace via `useOfficeWorkspaceData`, mounted // in the always-present `AppSidebar`. Setting the active workspace above diff --git a/apps/web/src/office-routes.workspace-revision.test.tsx b/apps/web/src/office-routes.workspace-revision.test.tsx new file mode 100644 index 0000000000..a9958d58a5 --- /dev/null +++ b/apps/web/src/office-routes.workspace-revision.test.tsx @@ -0,0 +1,92 @@ +import type { ReactNode } from "react"; +import { renderHook, waitFor } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { StoreApi } from "zustand"; +import { StateProvider, useAppStoreApi } from "@/components/state-provider"; +import type { AppState } from "@/lib/state/store"; + +vi.mock("@/lib/config", () => ({ + getBackendConfig: () => ({ apiBaseUrl: "http://localhost:8443" }), +})); + +const listWorkspacesMock = vi.fn(); +const fetchUserSettingsMock = vi.fn(); +const getOnboardingStateMock = vi.fn(); + +vi.mock("@/lib/api", () => ({ + listWorkspaces: (...args: unknown[]) => listWorkspacesMock(...args), + fetchUserSettings: (...args: unknown[]) => fetchUserSettingsMock(...args), +})); + +vi.mock("@/lib/api/domains/office-api", () => ({ + getOnboardingState: (...args: unknown[]) => getOnboardingStateMock(...args), +})); + +import { useOfficeRouteBootstrap } from "./office-routes"; + +const OFFICE_WORKSPACE_ID = "ws-office-1"; +const OFFICE_WORKSPACES_RESPONSE = { + workspaces: [ + { + id: OFFICE_WORKSPACE_ID, + name: "Office workspace", + owner_id: "user-1", + office_workflow_id: "office-flow-1", + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + }, + ], + total: 1, +}; + +function makeWrapper(initialActiveId: string | null) { + let captured: StoreApi | null = null; + function Capture() { + captured = useAppStoreApi(); + return null; + } + function Wrapper({ children }: { children: ReactNode }) { + return ( + + + {children} + + ); + } + return { Wrapper, getStore: () => captured }; +} + +// `useOfficeRouteBootstrap` used to hydrate `workspaces.activeId` directly, +// bypassing `setActiveWorkspace` and leaving `activeIdRevision` unbumped. +// Consumers keyed on that revision (the Failed-inbox cache) could then render +// a stale, pre-switch snapshot after navigating through Office. +describe("useOfficeRouteBootstrap", () => { + it("routes the resolved active workspace through setActiveWorkspace, bumping activeIdRevision", async () => { + getOnboardingStateMock.mockResolvedValue({ completed: true }); + listWorkspacesMock.mockResolvedValue(OFFICE_WORKSPACES_RESPONSE); + fetchUserSettingsMock.mockResolvedValue(null); + + const { Wrapper, getStore } = makeWrapper("ws-kanban-1"); + renderHook(() => useOfficeRouteBootstrap(true, null), { wrapper: Wrapper }); + + await waitFor(() => { + expect(getStore()?.getState().workspaces.activeId).toBe(OFFICE_WORKSPACE_ID); + }); + expect(getStore()?.getState().workspaces.activeIdRevision).toBe(1); + }); + + it("does not bump activeIdRevision when the resolved workspace matches the one already active", async () => { + getOnboardingStateMock.mockResolvedValue({ completed: true }); + listWorkspacesMock.mockResolvedValue(OFFICE_WORKSPACES_RESPONSE); + fetchUserSettingsMock.mockResolvedValue(null); + + const { Wrapper, getStore } = makeWrapper(OFFICE_WORKSPACE_ID); + renderHook(() => useOfficeRouteBootstrap(true, null), { wrapper: Wrapper }); + + await waitFor(() => { + expect(listWorkspacesMock).toHaveBeenCalled(); + }); + expect(getStore()?.getState().workspaces.activeId).toBe(OFFICE_WORKSPACE_ID); + expect(getStore()?.getState().workspaces.activeIdRevision ?? 0).toBe(0); + }); +}); diff --git a/apps/web/src/settings-routes.tsx b/apps/web/src/settings-routes.tsx index 38f65ea008..9ea6ea7385 100644 --- a/apps/web/src/settings-routes.tsx +++ b/apps/web/src/settings-routes.tsx @@ -555,7 +555,7 @@ function UpdatesRoute() { ); } -function SettingsRouteBootstrap({ pathname }: { pathname: string }) { +export function SettingsRouteBootstrap({ pathname }: { pathname: string }) { const store = useAppStoreApi(); const bootstrappedRef = useRef(false); @@ -568,8 +568,23 @@ function SettingsRouteBootstrap({ pathname }: { pathname: string }) { const initialState = await loadSettingsInitialState( () => store.getState().agentProfiles.version, ); - if (!cancelled && Object.keys(initialState).length > 0) { - store.getState().hydrate(initialState); + if (cancelled || Object.keys(initialState).length === 0) return; + const desiredWorkspaceId = initialState.workspaces?.activeId ?? null; + const workspaceBeforeHydration = store.getState().workspaces.activeId; + // Routing the actual switch through `setActiveWorkspace` (rather than + // letting `hydrate` overwrite `activeId` directly) keeps + // `activeIdRevision` accurate for consumers that key staleness off it, + // such as the Failed-inbox cache. + store.getState().hydrate( + initialState.workspaces + ? { + ...initialState, + workspaces: { ...initialState.workspaces, activeId: workspaceBeforeHydration }, + } + : initialState, + ); + if (initialState.workspaces && desiredWorkspaceId !== workspaceBeforeHydration) { + store.getState().setActiveWorkspace(desiredWorkspaceId); } } diff --git a/apps/web/src/settings-routes.workspace-revision.test.tsx b/apps/web/src/settings-routes.workspace-revision.test.tsx new file mode 100644 index 0000000000..d763064cad --- /dev/null +++ b/apps/web/src/settings-routes.workspace-revision.test.tsx @@ -0,0 +1,105 @@ +import type { ReactNode } from "react"; +import { render, waitFor } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { StoreApi } from "zustand"; +import { StateProvider, useAppStoreApi } from "@/components/state-provider"; +import type { AppState } from "@/lib/state/store"; + +vi.mock("@/lib/config", () => ({ + getBackendConfig: () => ({ apiBaseUrl: "http://localhost:8443" }), +})); + +const listWorkspacesMock = vi.fn(); +const listExecutorsMock = vi.fn(); +const listAgentsMock = vi.fn(); +const listAgentDiscoveryMock = vi.fn(); +const listAvailableAgentsMock = vi.fn(); +const fetchUserSettingsMock = vi.fn(); + +vi.mock("@/lib/api/domains/workspace-api", async (importOriginal) => ({ + ...(await importOriginal()), + listWorkspaces: (...args: unknown[]) => listWorkspacesMock(...args), +})); + +vi.mock("@/lib/api/domains/settings-api", async (importOriginal) => ({ + ...(await importOriginal()), + fetchUserSettings: (...args: unknown[]) => fetchUserSettingsMock(...args), + listAgentDiscovery: (...args: unknown[]) => listAgentDiscoveryMock(...args), + listAgents: (...args: unknown[]) => listAgentsMock(...args), + listAvailableAgents: (...args: unknown[]) => listAvailableAgentsMock(...args), + listExecutors: (...args: unknown[]) => listExecutorsMock(...args), +})); + +import { SettingsRouteBootstrap } from "./settings-routes"; + +const SETTINGS_WORKSPACE_ID = "ws-settings-1"; +const SETTINGS_WORKSPACES_RESPONSE = { + workspaces: [ + { + id: SETTINGS_WORKSPACE_ID, + name: "Settings workspace", + owner_id: "user-1", + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + }, + ], + total: 1, +}; + +function makeWrapper(initialActiveId: string | null) { + let captured: StoreApi | null = null; + function Capture() { + captured = useAppStoreApi(); + return null; + } + function Wrapper({ children }: { children: ReactNode }) { + return ( + + + {children} + + ); + } + return { Wrapper, getStore: () => captured }; +} + +function stubApis() { + listExecutorsMock.mockResolvedValue({ executors: [] }); + listAgentsMock.mockResolvedValue({ agents: [] }); + listAgentDiscoveryMock.mockResolvedValue({ agents: [] }); + listAvailableAgentsMock.mockResolvedValue({ agents: [], tools: [] }); + fetchUserSettingsMock.mockResolvedValue(null); +} + +// `SettingsRouteBootstrap` used to hydrate `workspaces.activeId` directly, +// bypassing `setActiveWorkspace` and leaving `activeIdRevision` unbumped. +// Consumers keyed on that revision (the Failed-inbox cache) could then render +// a stale, pre-switch snapshot after navigating through Settings. +describe("SettingsRouteBootstrap", () => { + it("routes the resolved active workspace through setActiveWorkspace, bumping activeIdRevision", async () => { + stubApis(); + listWorkspacesMock.mockResolvedValue(SETTINGS_WORKSPACES_RESPONSE); + + const { Wrapper, getStore } = makeWrapper("ws-kanban-1"); + render(, { wrapper: Wrapper }); + + await waitFor(() => { + expect(getStore()?.getState().workspaces.activeId).toBe(SETTINGS_WORKSPACE_ID); + }); + expect(getStore()?.getState().workspaces.activeIdRevision).toBe(1); + }); + + it("does not bump activeIdRevision when the resolved workspace matches the one already active", async () => { + stubApis(); + listWorkspacesMock.mockResolvedValue(SETTINGS_WORKSPACES_RESPONSE); + + const { Wrapper, getStore } = makeWrapper(SETTINGS_WORKSPACE_ID); + render(, { wrapper: Wrapper }); + + await waitFor(() => { + expect(listWorkspacesMock).toHaveBeenCalled(); + }); + expect(getStore()?.getState().workspaces.activeId).toBe(SETTINGS_WORKSPACE_ID); + expect(getStore()?.getState().workspaces.activeIdRevision ?? 0).toBe(0); + }); +}); diff --git a/docs/plans/needs-you-inbox/plan.md b/docs/plans/needs-you-inbox/plan.md index 0c6465dc94..11da806b78 100644 --- a/docs/plans/needs-you-inbox/plan.md +++ b/docs/plans/needs-you-inbox/plan.md @@ -3,10 +3,12 @@ created: 2026-09-12 status: complete requirements: - REQ-UI-NEEDS-YOU-INBOX-001 + - REQ-UI-INBOX-FAILED-001 system_design: - ../../specs/ui/system-design/needs-you-inbox-01.md - ../../specs/ui/system-design/needs-you-inbox-02.md - ../../specs/ui/system-design/needs-you-inbox-03.md + - ../../specs/ui/system-design/inbox-failed-bucket-01.md legacy_specs: [] --- @@ -30,6 +32,12 @@ 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. +Task 03 adds the Inbox's second bucket, the failed-task bucket, per +[requirements](../../specs/ui/requirements/inbox-failed.md) and +[system design](../../specs/ui/system-design/inbox-failed-bucket-01.md). It +was explicitly out of scope for Task 01 and is delivered as a separate work +order against the same Inbox surface. + ## Scope ### In scope @@ -41,34 +49,45 @@ that workflow when this feature's spec canon was authored. - In-place answer via the shipped `ClarificationPanelSection`. - Per-user dismiss/snooze/restore sidecar that never mutates the underlying clarification record. +- The Inbox's second, Failed tab: a bounded list of terminal-`FAILED` tasks, + deliberately excluded from the existing Needs-you badge count + (Task 03, `REQ-UI-INBOX-FAILED-001`). - i18n in all five shipped locales. ### Out of scope -Re-ask, the history bucket, the failed-task bucket, cross-workspace rows, and -any change to the current-turn predicate's meaning. See the requirements -document's own "Out of scope" section for the full list. +Re-ask, the history bucket, cross-workspace rows, and any change to the +current-turn predicate's meaning. See the requirements documents' own +"Out of scope" sections for the full list. ## Work orders - [x] [Task 01: Needs-you Inbox delivery](task-01-needs-you-inbox-delivery.md) - [x] [Task 02: Inbox feature-toggle terminology](task-02-inbox-label-alignment.md) +- [x] [Task 03: Inbox Failed tab delivery](task-03-inbox-failed-tab.md) - [x] [Task 03: Fix background-refresh loading state](task-03-fix-background-refresh-loading-state.md) Task 02 depends on Task 01 and records the follow-up display-name alignment. -Task 03 depends on Task 01 and fixes a `resolveViewMode` / WS-coalescing -regression against AC .19-.21; see its own file for scope and verification. +Task 03 depends on Task 01's shipped Inbox surface (tab strip host, route, +sidebar entry) but touches no file Task 01 did not already establish as +extensible; it is otherwise independent of Task 02's label change. + +The background-refresh work also depends on Task 01 and fixes a +`resolveViewMode` / WS-coalescing regression against AC .19-.21; see its own +file for scope and verification. ## Verification results -See the work order for the full verification record. Summary: backend and +See each work order for its 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 answer-in-place flow pass; -`pnpm run i18n:check` and `pnpm run i18n:ratchet` clean; `python3 -scripts/lint-spec-files.py --all` passes. +worktree); desktop and mobile E2E specs for the answer-in-place and +failed-tab flows 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/3641 +PRs: https://github.com/kdlbs/kandev/pull/3641 (Task 01), +https://github.com/kdlbs/kandev/pull/3673 (Task 02), +https://github.com/kdlbs/kandev/pull/3683 (Task 03) ## Task 02 results diff --git a/docs/plans/needs-you-inbox/task-03-inbox-failed-tab.md b/docs/plans/needs-you-inbox/task-03-inbox-failed-tab.md new file mode 100644 index 0000000000..caf7af1263 --- /dev/null +++ b/docs/plans/needs-you-inbox/task-03-inbox-failed-tab.md @@ -0,0 +1,121 @@ +--- +id: "03-inbox-failed-tab" +title: "Inbox Failed tab delivery" +status: done +wave: 3 +depends_on: ["01-needs-you-inbox-delivery"] +plan: "plan.md" +requirements: + - REQ-UI-INBOX-FAILED-001 +acceptance_criteria: + - AC-UI-INBOX-FAILED-001.1 + - AC-UI-INBOX-FAILED-001.2 + - AC-UI-INBOX-FAILED-001.3 + - AC-UI-INBOX-FAILED-001.4 + - AC-UI-INBOX-FAILED-001.5 + - AC-UI-INBOX-FAILED-001.6 + - AC-UI-INBOX-FAILED-001.7 + - AC-UI-INBOX-FAILED-001.8 + - AC-UI-INBOX-FAILED-001.9 + - AC-UI-INBOX-FAILED-001.10 + - AC-UI-INBOX-FAILED-001.11 + - AC-UI-INBOX-FAILED-001.12 + - AC-UI-INBOX-FAILED-001.13 + - AC-UI-INBOX-FAILED-001.14 + - AC-UI-INBOX-FAILED-001.15 + - AC-UI-INBOX-FAILED-001.16 + - AC-UI-INBOX-FAILED-001.17 + - AC-UI-INBOX-FAILED-001.18 + - AC-UI-INBOX-FAILED-001.19 + - AC-UI-INBOX-FAILED-001.20 + - AC-UI-INBOX-FAILED-001.21 + - AC-UI-INBOX-FAILED-001.22 + - AC-UI-INBOX-FAILED-001.23 + - AC-UI-INBOX-FAILED-001.24 + - AC-UI-INBOX-FAILED-001.25 + - AC-UI-INBOX-FAILED-001.26 + - AC-UI-INBOX-FAILED-001.27 + - AC-UI-INBOX-FAILED-001.28 + - AC-UI-INBOX-FAILED-001.29 + - AC-UI-INBOX-FAILED-001.30 +system_design: + - ../../specs/ui/system-design/inbox-failed-bucket-01.md +--- + +# Task 03: Inbox Failed Tab Delivery + +## Summary + +Add the Inbox's second bucket: a Failed tab that lists tasks in the terminal +`FAILED` state, deliberately off the existing Needs-you badge. The Needs-you +tab's rows, ordering, count, answer affordances, dismiss/snooze, and error +state are untouched; the only permitted delta to that tab is one added empty +state copy clause (`AC-UI-INBOX-FAILED-001.23` / `.4`). + +## In scope + +- Backend: `internal/failedinbox` (bounded list read, reason resolution), + `internal/task/repository/sqlite/failed_inbox_query.go` + (SQLite/Postgres-compatible query), and flag-gated route registration + (`features.needsYouInbox`, reusing the existing Inbox flag rather than + adding a new one). +- Frontend: `failedInbox` store slice and selectors, `inbox-tab-strip`, + `failed-inbox-tab-panel`/`-row`/`-empty-state`/`-error-state`, + `useFailedInboxController`, i18n in all five shipped locales. +- Desktop and mobile E2E coverage for listing a failed task without inflating + the sidebar's Needs-you count (AC .29). +- A workspace-revision fix (`office-routes.tsx`, `settings-routes.tsx`) for a + Zustand hydration race the tab-strip's workspace-switch handling surfaced: + capture `activeId` before `hydrate()`, keep `hydrate()`'s own `activeId` + unchanged, then call `setActiveWorkspace()` explicitly when it differs, so + `activeIdRevision` stays accurate. + +## Out of scope + +Re-ask, the history bucket, cross-workspace rows, and any change to the +current-turn predicate's meaning or the Needs-you tab's own row/ordering/count +behavior. See the requirements document's own "Out of scope" section for the +full list. + +## Acceptance + +All 30 numbered acceptance criteria under `REQ-UI-INBOX-FAILED-001`, plus the 6 +lettered sub-criteria the requirements document defines under them (.9a, .10a, +.18a, .19a, .20a, .30a — each refines its parent numbered AC rather than +standing as a separate tracked entry); see +[requirements](../../specs/ui/requirements/inbox-failed.md) for the +full text. Notable ones with dedicated regression coverage: + +- AC .14/.29: the sidebar's Needs-you count never moves when a failed task + exists, checked before the Failed tab is ever selected - + `apps/web/e2e/tests/chat/inbox-failed-tab.spec.ts`. +- AC .16: a re-visited workspace's stale Failed badge clears even when the + workspace switch happened while the Inbox was closed throughout - + `inbox-failed-tab.spec.ts` ("clears a re-visited workspace's stale badge... + Inbox closed"). +- AC .4/.23: the Needs-you empty state gains exactly one added copy clause and + no other behavior change - `needs-you-inbox-empty-state.test.tsx`. +- AC .18: the open-task control stays reachable at every viewport width (no + actions-menu fallback) - `failed-inbox-row.tsx` / `failed-inbox-row.test.tsx`. + +## Verification + +- `go build ./...`, `go vet ./...` clean (apps/backend). +- `go test ./internal/failedinbox/... ./internal/task/repository/sqlite/...` + - all packages `ok`. +- `make -C apps/backend lint` - 0 issues. +- `pnpm run typecheck`, `pnpm exec eslint` (touched files) - clean. +- `pnpm exec vitest run` (targeted `failed-inbox`, Office/Settings + workspace-revision suites) - all passing. +- `pnpm run i18n:check` / `pnpm run i18n:ratchet` - five locales complete, no + em-dash violations, guard allowlist intact. +- `pnpm e2e:run` - desktop `inbox-failed-tab.spec.ts` and mobile + `mobile-inbox-failed-tab.spec.ts` both pass with a full rebuild. +- `python3 scripts/lint-spec-files.py --all` - all specification files + passed. +- Full local-suite pre-existing failures (macOS `/var`-symlink worktree + issues, no local Docker daemon, wall-clock-timing tests under host load) + proven pre-existing against `git merge-base HEAD origin/main` in a scratch + worktree; unrelated to this branch's diff. + +PR: https://github.com/kdlbs/kandev/pull/3683 diff --git a/docs/public/configuration.md b/docs/public/configuration.md index 24c9947026..714b4d769c 100644 --- a/docs/public/configuration.md +++ b/docs/public/configuration.md @@ -508,7 +508,7 @@ Copying this entire file is unnecessary and can freeze old defaults in a deploym | `features.multiTenancy` | `KANDEV_FEATURES_MULTI_TENANCY` | off | Experimental organizations above authenticated users. Requires `features.auth`; startup is refused otherwise. | | `features.dynamicAgentRouting` | `KANDEV_FEATURES_DYNAMIC_AGENT_ROUTING` | off | Experimental dynamic profiles with ordered provider-error fallback. | | `features.canvases` | `KANDEV_FEATURES_CANVASES` | off | Experimental agent-authored isolated web-app canvases for tasks and workspaces. High risk. | -| `features.needsYouInbox` | `KANDEV_FEATURES_NEEDS_YOU_INBOX` | off | Inbox for clarification questions across tasks in the active workspace. | +| `features.needsYouInbox` | `KANDEV_FEATURES_NEEDS_YOU_INBOX` | off | Inbox for clarification questions and failed tasks in the active workspace. The Failed tab shows a bounded list with its own count. | | `features.officeSessionIdentity` | `KANDEV_FEATURES_OFFICE_SESSION_IDENTITY` | on | Office participant sessions: each participant agent gets its own session per task. The live `(task_id, agent_profile_id)` pair is guarded in-transaction, not by a table-level index. Pre-existing duplicate rows are retained and resolved by selection. Two Kandev processes must not write the same SQLite file. | | `features.agentSurvival` | `KANDEV_FEATURES_AGENT_SURVIVAL` | off | Experimental. Lets the `agentctl` control server and its running agents outlive a backend restart or upgrade, for the `local_pc` and worktree executors only. The restarted backend takes the surviving server over and reconnects to the sessions still running on it. **Unavailable on Windows**, where surviving the backend means giving up the Job Object that guarantees agent processes are cleaned up. | | `debug.devMode` | `KANDEV_DEBUG_DEV_MODE` | off | High-risk diagnostic endpoints and ACP frame logging. | diff --git a/docs/specs/ui/requirements/inbox-failed.md b/docs/specs/ui/requirements/inbox-failed.md new file mode 100644 index 0000000000..a6bb904763 --- /dev/null +++ b/docs/specs/ui/requirements/inbox-failed.md @@ -0,0 +1,322 @@ +--- +status: draft +system: ui +created: 2026-09-14 +owners: + - nova28 +--- + +# Inbox Failed Bucket Requirements + +## Overview + +The Inbox shipped with one bucket. Its badge counts questions blocking an agent +and nothing else, which is correct and is not changed here. What it leaves out is +the other way an agent stops being useful: it fails, and nothing says so. Measured +2026-09-14, archived excluded: 15 tasks sit in the terminal `FAILED` state, and none +appears on any list an operator opens. + +This capability adds the Inbox's second bucket: failed tasks, present and +deliberately off the badge. A failed task is triage, not a decision the operator +owes an agent, so it must be reachable without ever inflating the number that +means "someone is blocked on you". UI owns the bucket, the tab, the row, and the +exclusion from the count. Task failure itself, its state machine, and its retry +behaviour stay owned by the tasks and workflow systems, consumed here unchanged. + +This file states the contract. The +[system design](../system-design/inbox-failed-bucket-01.md) argues it, and carries +the input inventory, the prior art and its receipts, and the reasoning behind the +Out of scope entries that turn on a judgement rather than a scope boundary. + +## Terminology + +- **Failed task:** A task whose own state is the terminal `FAILED` state and which + is not archived. Defined at the task, not the session: AC-UI-INBOX-FAILED-001.6. +- **Failure instant:** The completion instant of the session + AC-UI-INBOX-FAILED-001.30 selects. Not the task's last-updated time. +- **Actionable count:** As the Inbox already defines it, the number of rows the + operator can act on now. Failed tasks are never part of it. + +## Requirements + +### REQ-UI-INBOX-FAILED-001: Inbox failed bucket + +**Intent:** Make every failed task in the active workspace reachable from the +Inbox in one place, ordered newest first, without adding a single unit to the +count that means an agent is blocked on the operator. + +**User story:** As an operator, I want the Inbox to show me what failed, so that +a dead agent run is discoverable without opening the board, and so that the +number on the sidebar still only ever means someone is waiting on my answer. + +#### Acceptance criteria + +**Tab strip and destination** + +- **AC-UI-INBOX-FAILED-001.1:** The Inbox shall render a tab strip with exactly two + tabs, in the order Needs you then Failed, and Needs you shall be selected when no + tab is otherwise specified. +- **AC-UI-INBOX-FAILED-001.2:** The selected tab shall be addressable through the + `tab` query parameter on the existing Inbox route, whose only recognised values are + `needs-you` and `failed`. Opening `?tab=failed` shall render the Failed tab + directly, without the operator selecting it and without first rendering Needs you. + An absent, empty, repeated or unrecognised value shall render Needs you rather than + an error. Selecting a tab shall REPLACE the current history entry, not push one. +- **AC-UI-INBOX-FAILED-001.3:** Where the Inbox feature flag is disabled, no tab + strip and no failed bucket shall render, by the same gating that already governs + the Inbox destination. +- **AC-UI-INBOX-FAILED-001.4:** The Needs you tab's row set, ordering, count, + answer affordances, dismiss and snooze behaviour, and error state shall be + unchanged by this capability. Its empty state shall be unchanged except for the + single copy clause AC-UI-INBOX-FAILED-001.23 adds: same render conditions, row + set, structure and workspace-naming rule, that clause the only observable + difference. A test shall assert the enumerated behaviour is identical before and + after, asserting behaviour rather than empty-state copy. + +**Row set** + +- **AC-UI-INBOX-FAILED-001.5:** The Failed tab shall list exactly those tasks + that are failed tasks and whose workspace is the active workspace, and no + others, subject to the bound in AC-UI-INBOX-FAILED-001.12. Switching the + active workspace shall replace the list with that workspace's failed tasks. +- **AC-UI-INBOX-FAILED-001.6:** A row shall be derived from the task's own terminal + failed state, never from a session state alone. Given a fixture of tasks whose + primary session is failed but whose task state is not terminal failed, the Failed + tab shall list zero rows for them. +- **AC-UI-INBOX-FAILED-001.7:** An archived task shall never be listed, whatever + its state. +- **AC-UI-INBOX-FAILED-001.8:** When a listed task stops being a failed task for any + reason, including retry, re-state or archive, the Failed tab shall remove its row + without the operator reloading or re-navigating. The row set shall converge by + re-reading the same path, so no exit can strand a row by emitting no event. While + the Inbox is open, a workspace is active and the browser tab is visible, that + convergence shall complete within 60 seconds of the change when that window's + re-read succeeds, and a tab returning to visible shall re-read at once rather than + waiting out that interval. A failing read falls to AC-UI-INBOX-FAILED-001.22. + Neither bound depends on which tab is selected. +- **AC-UI-INBOX-FAILED-001.9:** A failed task shall be listed regardless of origin. + Given a fixture in which most failed tasks are automation runs, the Failed tab + shall list all of them rather than suppressing them. +- **AC-UI-INBOX-FAILED-001.9a:** `manual` shall be the only origin treated as a + person; every other value shall be treated as not a person. A `manual` row shall + render no origin marker. Each of the five non-person origins shall render its own + distinct marker, not one shared word: `agent_created`, `routine`, `onboarding`, + `automation_run`, `automation_task`. A non-empty origin this capability does not + enumerate shall render a stated generic non-person marker, never nothing, never + the raw value, never a failed row. An absent or empty origin shall render no + marker, taking the `manual` path. + +**Ordering and boundaries** + +- **AC-UI-INBOX-FAILED-001.10:** Rows shall be ordered by three keys in this + order: first, whether the failure instant is unresolvable, unresolvable before + resolved; second, failure instant descending; third, `tasks.id` ascending, + compared as a byte-ordered string as in AC-UI-INBOX-FAILED-001.30a, never by a + collation the storage engine chooses. The order shall be total: the same set of + failed tasks shall always produce the same row order, so the unresolvable group is + itself ordered by `tasks.id`. +- **AC-UI-INBOX-FAILED-001.10a:** The placement of unresolvable failure instants + shall be an explicit sort key and shall never be left to the storage engine's + default placement of absent values, on which this product's two engines disagree. + The row order shall be identical under every engine this product supports, and a + test shall assert it under each rather than one. +- **AC-UI-INBOX-FAILED-001.11:** The failure instant shall be the completion instant + recorded on the session AC-UI-INBOX-FAILED-001.30 selects, never the task's + last-updated time. Given a fixture whose task was written after it failed, the row + shall report the instant it failed rather than when it was last written. Where that + session records no completion instant, the failure instant is unresolvable and + AC-UI-INBOX-FAILED-001.30 governs; no other field shall be substituted. +- **AC-UI-INBOX-FAILED-001.12:** A single read shall return at most a bounded + page, matching the bound the Needs you bucket already uses: an absent limit + shall default to 50, a limit above 200 shall be clamped to 200 rather than + rejected, and a limit of 200 or below shall be honoured exactly. When more + failed tasks exist than the page returns, the Failed tab shall indicate the list + is truncated rather than presenting a partial list as complete. The response + shall carry that truncation as its own explicit field, never inferred from the + presence of a cursor, a next-page link, or any other optional field. +- **AC-UI-INBOX-FAILED-001.13:** A read shall reject a missing or empty workspace + identifier and a non-numeric or non-positive limit, each as a client error that + lists no rows and renders the error state, never silently substituting a default. + A read matching nothing shall return an empty list rather than an absent one. The read shall accept and return no cursor: it serves + exactly one bounded page per AC-UI-INBOX-FAILED-001.12, whose truncation + indicator is all it says about rows beyond the bound. + +**Count** + +- **AC-UI-INBOX-FAILED-001.14:** The sidebar count shall be unchanged by this + capability. No failed task shall contribute to it through any producer, including + the boot-hydrated producer that renders it before the Inbox is opened. +- **AC-UI-INBOX-FAILED-001.15:** Given a fixture with one or more failed tasks and + no answerable bundles in the active workspace, the sidebar entry shall render no + count at all. A zero shall not be rendered in place of the absent count, and the + presence of failed tasks shall not make it appear. +- **AC-UI-INBOX-FAILED-001.16:** The Failed tab shall carry its own count, on the + tab and not in the page header, rendered as a secondary-variant badge inside the + tab control. That count shall be the number of rows carried by the most recent + response successfully applied for the active workspace, never a separately computed + total, a count from the start of the result set, or any number that response did + not carry alongside its rows. Whenever the Failed tab is selected that response is + also the one supplying its rows, so no rendered state exists in which the count and + the visible list disagree. It shall remain rendered while the other tab is selected, + since the tab strip is visible from both. No count shall be rendered before + a response has been applied for the active workspace, nor where it is zero; an + unread bucket shall not be presented as one known to hold zero. Changing the active + workspace shall clear it until a response for the new one is applied. +- **AC-UI-INBOX-FAILED-001.17:** When the failed page is truncated, the tab's count + shall present the bounded value as a capped indicator rather than an exact total, + by the same presentation the Needs you count already uses. Given a bound of 200 + and more than 200 failed tasks, the tab shall render the row count marked as + capped, never a bare 200 and never a total the response did not carry. + +**Row content** + +- **AC-UI-INBOX-FAILED-001.18:** A row shall present the task's title, the reason it + failed, how long ago it failed, and a control that opens the task, reachable at + every viewport width including where a fixed-width button would be withheld. +- **AC-UI-INBOX-FAILED-001.18a:** A row's failure marker shall be the shipped + TASK-state failed pairing from the shared state-icon module, not that module's + session-state one, which is a different pairing: this bucket is defined by task + state per AC-UI-INBOX-FAILED-001.6. This capability shall add no new colour; the + marker shall use the module's existing error style. A test shall assert the row + renders that module's task-state pairing rather than a local copy of it. +- **AC-UI-INBOX-FAILED-001.19:** The failure reason shall be the error the task's + session recorded, truncated by the server so one long message cannot inflate a + full page; a message longer than the bound shall be truncated at the boundary + rather than omitted or rejected. Where no reason can be resolved, or the value the + server returns is empty or only whitespace, the row shall render a stated fallback + rather than a blank line, and shall still render the rest of the row. That + emptiness test applies to the value the server returns, after + AC-UI-INBOX-FAILED-001.19a's truncation, so a reason whose first 512 code points + are all whitespace takes the fallback. +- **AC-UI-INBOX-FAILED-001.19a:** The server shall produce the returned reason in + this order, and the order is part of the contract because the steps are not + commutative. FIRST, the stored value shall be made valid UTF-8 by replacing each + byte that is not part of a well-formed sequence with the Unicode replacement + character, so what is measured and returned is valid UTF-8 for every input, + including one that is not. SECOND, the result shall be truncated to at most 512 + Unicode code points, not bytes, never splitting a code point. A reason already + valid UTF-8 and 512 code points or fewer shall be returned unchanged; that covers + this case only and is no licence to return invalid UTF-8 that happens to be short. + Given a 600-code-point reason in a script encoding to more than one byte per + character, the response shall carry exactly the first 512 and no partial trailing + character. The response shall carry no truncated flag for the reason and the row + shall render no marker. +- **AC-UI-INBOX-FAILED-001.30:** The failure instant and the failure reason shall + be resolved from exactly one of the task's sessions, by a rule total and + deterministic for any task: the session marked primary; where more than one is, + the one that started most recently; where none is, the session that started most + recently; and where the task has no session, neither field is resolvable. A failed task shall be listed in every one of those cases, including + the last: an unresolvable failure instant shall place the row first per + AC-UI-INBOX-FAILED-001.10a, with the row stating the failure time is unknown + rather than a substituted time, and an unresolvable reason shall render + AC-UI-INBOX-FAILED-001.19's fallback. No resolution failure + shall remove a failed task that AC-UI-INBOX-FAILED-001.12's bound would otherwise + have included; the bound is the only thing that may elide a row. +- **AC-UI-INBOX-FAILED-001.30a:** Where two or more candidate sessions tie at the + most recent start instant, the session with the lowest session identifier, + compared as an ascending byte-ordered string, shall be chosen, so repeating a read + against unchanged data always selects the same session as + AC-UI-INBOX-FAILED-001.25 requires. The start instant is non-nullable, so this + covers an equal value, not an absent one. +- **AC-UI-INBOX-FAILED-001.20:** Every row on the Failed tab shall present the + shared thread status vocabulary's failed status, the entry the rest of the + product already uses, which carries no attention flag. It shall be selected by + the task's own terminal failed state, which every listed row has by + AC-UI-INBOX-FAILED-001.6, and shall not be resolved per row from any session's + state. Because that vocabulary's resolvers take a session and this row model + carries none, the failed entry shall be reached directly, never by synthesising a + stand-in session to pass to a resolver. Exporting that existing entry from the + module that already defines it is permitted and is not a new entry. This + capability shall define no second status vocabulary, add no entry to the existing + one, and duplicate no entry's value locally. +- **AC-UI-INBOX-FAILED-001.20a:** No row on the Failed tab shall render a status + carrying an attention flag, under any resolution outcome of + AC-UI-INBOX-FAILED-001.30, including a task with no session at all. Given a + fixture whose resolved session carries a pending permission request or a pending + clarification, the row shall still present the failed status. + +**Empty and error** + +- **AC-UI-INBOX-FAILED-001.21:** When the Failed tab lists no rows for the active + workspace, it shall render an empty state that names the active workspace, does + not congratulate the operator, and does not claim they are caught up. Where the + workspace's name cannot be resolved from client state, the copy shall name no + workspace rather than the wrong one. +- **AC-UI-INBOX-FAILED-001.22:** If a failed-bucket read fails, then the Failed tab + shall render an error state distinct from its empty state, shall not state that + nothing failed, and shall render no count. A failure of one tab's read shall not + empty, error, or otherwise disturb the other tab, except for the single clause that + AC-UI-INBOX-FAILED-001.23 governs. +- **AC-UI-INBOX-FAILED-001.23:** The Needs you tab's empty state shall name + failed tasks among the things it is not counting, and shall say where they are. + Where the active workspace holds at least one failed task, it shall say so + rather than implying the workspace is quiet; where it holds none, it shall not + imply that any exist; and where the failed count is not known, because no failed + read has been applied for the active workspace or the last one failed, it shall + omit the clause entirely rather than assert either. This shall hold on the first + render of the Needs you empty state, without the operator selecting the Failed tab + first. + +**Security** + +- **AC-UI-INBOX-FAILED-001.24:** If a failed-bucket read names a workspace + outside the caller's visible workspace set, then the server shall reject it with + the same non-disclosing not-found outcome the Inbox already uses rather than an + empty success, and the Failed tab shall render its error state rather than its + empty state. Scoping shall be enforced server-side from the request identity; a + client-supplied workspace identifier shall not by itself grant access. Where + authentication is disabled, the single-user identity reaches every workspace. + +**Idempotency, concurrency, and staleness** + +- **AC-UI-INBOX-FAILED-001.25:** The failed-bucket read shall be free of side + effects and idempotent: repeating it with the same parameters against unchanged + data shall return the same rows in the same order and write nothing. Nothing in + this capability shall write task state. +- **AC-UI-INBOX-FAILED-001.26:** When two failed-bucket reads are in flight, only + the response matching the currently active workspace and the most recently + issued read shall be applied, so a slower earlier response can neither replace + the active list nor resurrect a row that a newer response dropped. That guard + shall NOT key on the selected tab: the failed read runs regardless of which tab + is selected per AC-UI-INBOX-FAILED-001.23, so a response that arrives while Needs + you is selected shall still be applied. +- **AC-UI-INBOX-FAILED-001.27:** While a task changes state concurrently with a + read, the read shall return either the row or no row and shall not fail. Two + operators reading one workspace at the same instant shall each receive a + self-consistent page. + +**Copy and coverage** + +- **AC-UI-INBOX-FAILED-001.28:** All copy added by this capability shall be + localized in the five shipped locales, shall contain no Unicode em dash, and the + repository i18n ratchet shall pass. +- **AC-UI-INBOX-FAILED-001.29:** An end-to-end specification shall cover observing + the Failed tab's count before that tab is ever selected, then selecting it, + observing a failed task listed with its reason and relative failure time, opening + the task from the row, and observing that the sidebar count did not change when + the failed task appeared. + +## Out of scope + +Each entry is a stated exclusion, not an omission. + +- **Pagination past the first bounded page.** The read serves one page of at most + 200 rows and says, through AC-UI-INBOX-FAILED-001.12's truncation indicator, + that more exist; it takes and returns no cursor. +- **Dismiss, snooze, and restore for failed rows.** A failed task already has three + real exits, all removing the row under AC-UI-INBOX-FAILED-001.8: retry, move, + archive. +- **Retrying or restarting a task from the Inbox.** The row's only control opens the + task. +- **The history bucket ("Asked, unanswered" in the deck).** A separate sibling + capability. The tab strip has room; the third tab is not built here. +- **Severity, ranking, or grouping of failures.** Every failed task is one row. No + dot, no chip, no rollup of repeated identical failures. +- **Notification on failure.** Enabling this emits no notification for a task that + already failed, and adds none for a task that fails later. +- **Retention, archival, or pruning of failed rows.** Archived tasks are excluded by + AC-UI-INBOX-FAILED-001.7. +- **Cross-workspace rows.** Workspace-scoped, by the same product decision and + permission barrier that scopes the Needs you bucket. +- **Any change to what makes a task failed**, to its state machine, or to the + workflow engine's behaviour on agent error. Consumed unchanged. diff --git a/docs/specs/ui/system-design/inbox-failed-bucket-01.md b/docs/specs/ui/system-design/inbox-failed-bucket-01.md new file mode 100644 index 0000000000..abce01bf71 --- /dev/null +++ b/docs/specs/ui/system-design/inbox-failed-bucket-01.md @@ -0,0 +1,526 @@ +--- +status: draft +system: ui +requirements: + - REQ-UI-INBOX-FAILED-001 +--- + +# Inbox Failed Bucket System Design + +The Inbox itself is designed in +[`needs-you-inbox-01.md`](needs-you-inbox-01.md), +[`needs-you-inbox-02.md`](needs-you-inbox-02.md) and +[`needs-you-inbox-03.md`](needs-you-inbox-03.md). This design adds one bucket to it, +and changes exactly one thing those three own: the Needs you empty state gains a +single copy clause, specified by `AC-UI-INBOX-FAILED-001.23` and carved out as the +sole permitted delta by `AC-UI-INBOX-FAILED-001.4`. Their behaviour, row set, +ordering, count, answer affordances, dismiss and snooze, and error state are +untouched. + +## Purpose and boundaries + +UI owns the bucket, the tab, the row, and the exclusion from the count. It does +not own what makes a task failed, when a failed session is retried, or how a +task is archived. Those are the tasks and workflow contracts, consumed here +through a read-only projection. + +Two boundaries are worth naming because getting either wrong turns this into a +different change: + +- **The badge producer is not touched.** The Inbox badge is produced by the + clarification-inbox read and its boot-hydration twin. This capability adds a second + read and a second count, rendered on a tab. No failed-task code path may write the + badge's per-workspace state key. +- **The failed read is a separate endpoint from the bundle read.** They answer + different questions over different tables, and merging them would make one + bucket's failure the other bucket's failure, which + AC-UI-INBOX-FAILED-001.22 forbids. + +## Prior art + +**Our own prior reasoning (wiki).** Receipt: the vault resolved through +`~/.obsidian-wiki/config` to `OBSIDIAN_VAULT_PATH=/Users/henry/Documents/henry/wiki`, +collection `wiki`. It could not then be read: `qmd` is not on `PATH`, its MCP server +is not exposed, `obsidian-wiki` is not installed, and the directory is unreadable +from this sandbox (`Operation not permitted`), so no grep fallback was possible. This leg returned nothing because the vault was +unreachable, not because it held nothing. + +**What other products shipped (saas-kb).** Receipt: `search_saas_docs` with +`category: "ai_sdlc"`, queries "inbox badge count actionable items failed runs +triage surface" and "sidebar badge unread count excluded from count failed runs do +not require action", then `get_saas_doc` on the top hit. The first returned +substantive Paperclip and Factory.ai hits; the second collapsed onto unrelated +vendors and was discarded. + +Paperclip's Inbox is the closest shipped analogue (tabs: Mine, Recent, Unread, +Blocked, All). Three decisions taken from it: tab switching navigates, since its +`/inbox/` "doesn't just hide content", so a tab is bookmarkable; failed runs +are a category rather than a queue, though we place them in a dedicated tab rather +than a filter inside a firehose, because two buckets are not six categories and a +filter must be discovered; and per-item hiding is scoped to one tab, which we reach +for the different reason in [Out-of-scope reasoning](#out-of-scope-reasoning). + +Where we differ deliberately: Paperclip's Blocked tab carries a severity dot and a +blocked-reason chip, and its Mine tab is "the queue that matters most". We rank no +failures by severity and do not treat this bucket as a queue. + +## Requirement mapping + +`REQ-UI-INBOX-FAILED-001` maps to the sections below, per acceptance criterion. + +| Acceptance criteria | Design section | +| --- | --- | +| .1-.4 | [Components and responsibilities](#components-and-responsibilities) | +| .5-.9 | [Input inventory](#input-inventory), [The failed-task predicate](#the-failed-task-predicate) | +| .9a | [Data and contracts](#data-and-contracts) | +| .10, .10a, .11-.13 | [Ordering, paging, and the failure instant](#ordering-paging-and-the-failure-instant) | +| .14-.17 | [Count separation](#count-separation), [Ordering, paging, and the failure instant](#ordering-paging-and-the-failure-instant) | +| .18, .18a, .19, .19a | [Data and contracts](#data-and-contracts), [Copy and coverage](#copy-and-coverage) | +| .20, .20a, .30, .30a | [Session resolution](#session-resolution) | +| .21-.23, .27 | [Failure and recovery](#failure-and-recovery), [Count separation](#count-separation), [Control flow](#control-flow) | +| .24 | [Security](#security) | +| .8, .25-.26 | [Control flow](#control-flow) | +| .28-.29 | [Copy and coverage](#copy-and-coverage) | + +## Input inventory + +Sampled from the live instance on 2026-09-14 against +`/Users/henry/.kandev/data/kandev.db` opened read-only. Every number below is a +measurement, not an estimate. + +| Candidate predicate, archived excluded | Rows | +| --- | --- | +| `tasks.state = 'FAILED'` | 15 | +| task has a primary session in state `FAILED` | 18 | +| task has any session in state `FAILED` | 22 | + +The three-row gap decomposes as: 15 tasks where both hold, plus one task in +`CREATED`, one in `REVIEW` and one in `SCHEDULING`, each with a failed primary +session. No task is in `FAILED` state without a failed primary session, so the first +set is a strict subset of the second and task state is reliably written. + +Other measured facts the contract depends on: + +- `task_sessions.error_message` on a failed session: never empty across all 111 + failed sessions, mean 199 bytes, maximum 4096. +- `task_sessions.completed_at` on a failed session: never null across those 111. +- `tasks.updated_at` drifts from the session completion instant: zero on 14 of the + 15 failed tasks, and 538,095 seconds (6.2 days) on one. +- Origin of the 15 failed tasks: 13 `automation_run`, 2 `manual`. `is_ephemeral` + is 0 on all 18 primary-failed tasks, so it does not discriminate and `origin` + does. +- `tasks.labels` is `[]` on every failed task, so the deck's row subtitle cannot be + sourced from labels. +- Workspace spread of the 15: 14 in the Kandev workspace, 1 elsewhere, so the + default page bound is never reached today. +- 42 archived tasks carry a failed primary session, which is why + AC-UI-INBOX-FAILED-001.7's exclusion is load-bearing rather than theoretical. + +## The failed-task predicate + +**Decision: a row exists when `tasks.state` is the terminal failed state and +`tasks.archived_at` is null.** Rejected alternatives: + +- **Primary session is `FAILED`.** What the deck measured, and wrong as a predicate. + It admits the `SCHEDULING` task, which the system is actively re-placing and which + will run again without anyone triaging it, and the `REVIEW` task, which has already + moved on. The deck's 17 was a volume-and-recency argument, not a definition, and the + deck does not own the data contract. +- **Any session is `FAILED`.** Admits every task that ever failed once and then + succeeded: 22 rows today, at least 7 of them recoveries. + +The chosen predicate is the one the rest of the product already uses: +`IsTerminalTaskState` in `apps/backend/internal/task/models/models.go` treats +completed, failed and cancelled alike. Deriving the bucket from a fact the task model +already asserts means a retry removes the row by changing task state, which is the +convergence AC-UI-INBOX-FAILED-001.8 requires, rather than by a session count the +Inbox would re-derive. + +The cost, stated plainly: a task that dies without its state being written to failed +will not appear. Today that set is empty. If it stops being empty, the fix is in the +task state machine, not a second predicate here. + +## Components and responsibilities + +| Component | Responsibility | +| --- | --- | +| Failed-bucket read endpoint | Workspace-scoped, bounded, read-only projection of failed tasks. New. | +| Failed-bucket store query | One query joining the task to the session [Session resolution](#session-resolution) selects, for the failure instant and reason. Usually the primary session but not always, so the join follows the selection rule, not `is_primary`. New. | +| Inbox page tab strip | Two tabs, `variant="line"` from the shared `Tabs` component; each count a secondary-variant `Badge` inside its tab control. New. | +| Failed row | Title, reason, relative failure time, open-task control, and the shared module's TASK-state failed marker. New. | +| Failed-bucket state slice | Per-workspace rows, count, truncation flag, read status, generation guard. New, modelled on the existing Needs-you slice. | +| Needs-you bucket, its slice, its controller, its badge | Unchanged. | +| Shared thread status vocabulary | Its `failed` entry is reused for row status, reached directly rather than through `resolveThreadSessionStatus`: see [Session resolution](#session-resolution), the resolvers take a session and this row has none. | + +The deck settles where the count goes: inside the tab, not the page header, because +the header is where a title would go and this page has none. + +## Data and contracts + +The failed row carries, per task: `task_id`, `title`, `workspace_id`, `origin`, +`failure_instant`, and `reason`. `failure_instant` is OMITTED when unresolvable, +never null and never a zero instant, which is how the client tells it from a real +one; the other five keys are always present, empty string rather than absent. It carries no session identifier and +no session state in the row model, because the row addresses a task, the +open-task control needs only the task, and the row's status is fixed rather than +resolved from a session (see [Session resolution](#session-resolution)). + +**Origin, and which values count as a person.** `manual` is the only origin that +is a person; the other five (`agent_created`, `routine`, `onboarding`, +`automation_run`, `automation_task`, at `models.go:1024-1030`) are not, and each +renders its own marker. The partition is stated rather than inferred because the +enum has six values while the live failed set has only two of them, so a builder +reading the input inventory alone would be guessing about the other four the day +one of them fails. They are not collapsed into a single "automated" word because +an agent creating a task and a schedule creating a task are different triage +facts and the task model already spends six constants distinguishing them. The +set is owned by the tasks system and can grow without this spec changing, so an +unrecognised value renders a generic non-person marker rather than nothing, the +raw stored string, or a broken row. + +**Failure reason.** The session's `error_message`, truncated server-side to 512 +Unicode code points before it enters the response. The bound stops a 4096-byte +message multiplied by a 200-row page from becoming an 800KB payload for a list the +operator scans. The unit is code points, not bytes, and truncation never splits one, +so the field is valid UTF-8 for every input including one that is not. This is stated +because the measured field is measured in bytes and the obvious Go implementation, +slicing at byte offset 512, produces invalid UTF-8 the first time a non-ASCII message +runs long. Code points cap the worst case at 4 bytes each, still far under the +payload the bound prevents. The client +renders it clamped to one line and never blank: an unresolvable reason becomes +stated fallback copy, per AC-UI-INBOX-FAILED-001.19. Nothing flags a truncated +reason to the client, per AC-UI-INBOX-FAILED-001.19a, because the row is already +clamped to one line and the full text is on the task the row opens. + +**Why the reason and not the deck's subtitle.** The deck draws a subsystem label +under each title. That label has no source: `tasks.labels` is empty on every failed +task, and subsystem tags are a plugin-owned, human-assigned dimension the read path +does not have. The failure reason is always present and is the actual triage +content, so it takes the slot. + +**The endpoint.** `GET /api/v1/failed-inbox`, taking `workspace_id` (required) and +`limit` (optional), parsed by the same rules the bundle read applies. It takes no +`cursor`, the one query parameter the sibling accepts and this read does not. + +**Response envelope: rows, `count`, and an explicit `truncated` boolean.** The +truncation signal is a FIELD OF ITS OWN, and this is the one place the sibling +cannot be copied. The bundle read carries no truncation field at all: it signals +"there is more" solely by emitting `next_cursor`, which the client turns into +`hasMore` and then into the capped `"+"` suffix on the count. That whole chain +hangs off the cursor. Since this read deliberately has none +(`AC-UI-INBOX-FAILED-001.13`), nothing in the sibling's envelope is left to mirror, +and a builder told to "mirror the bundle read" would either re-introduce the cursor +to get the signal back or invent a field silently. So it is named here: `truncated`, +set when the store had more rows than the bound returned, computed the way the +sibling computes `HasMore`. The CLIENT-side presentation +of a capped count is still shared with the sibling per +`AC-UI-INBOX-FAILED-001.17`; it is only the wire signal that differs, and it +differs because the cursor it used to ride on is gone. + +**The tab lives in the address as the `tab` query parameter on the existing Inbox +route**, not as a second route path: `?tab=failed` selects Failed, `?tab=needs-you` +and every other value, including an absent, empty, repeated or unrecognised one, +selects Needs you. The destination is one page with two buckets; giving Failed its +own path would make the sidebar entry's active-route match ambiguous and would +duplicate the page shell. Paperclip's `/inbox/` shape is the right instinct +about bookmarkability and the wrong shape for a route that already exists and is +matched exactly. + +Selecting a tab REPLACES the current history entry rather than pushing one. Both +forms are equally bookmarkable, so the only question is what the back control does, +and that follows from what the strip is: two views of one destination. Pushing would +make back walk the operator through their own tab clicks before letting them leave +the Inbox. Replacing means back returns where they came from, and the address still +names the tab for a bookmark or reload. + +## Session resolution + +The row's failure instant and reason come from one session, chosen by a rule that +must be total because the schema does not enforce what the data currently happens to +satisfy. `task_sessions.is_primary` has no unique index, so more than one primary +session per task is representable, and nothing requires a task to have a session. + +Measured 2026-09-14: zero tasks have more than one primary session, zero failed tasks +have no primary session, zero have no session at all. The rule below therefore costs +nothing today and exists so a builder is not forced to invent it, and so a future data +shape cannot silently drop a failed task. + +Chosen session, in order: the primary session; if several are marked primary, +the most recently started; if none is, the most recently started session of any +kind; and where two candidates tie on start instant, the lower +`task_sessions.id` compared as an ascending byte-ordered string. + +**That last tiebreak is load-bearing, not tidiness.** One of the fields read from +the chosen session is `completed_at`, which +[Ordering, paging, and the failure instant](#ordering-paging-and-the-failure-instant) +makes the primary sort key. Two sessions tying on `started_at` while carrying +different `completed_at` values would put the row in two different positions on two +reads of unchanged data, breaking both the total order and the idempotency +`AC-UI-INBOX-FAILED-001.25` requires. `TaskSession.StartedAt` is a `time.Time` and +not a pointer, so this covers an equal value rather than an absent one. + +Where the task has no session, both fields are unresolvable. The row is still listed. +It sorts FIRST and states the failure time is unknown, rather than borrowing +`tasks.updated_at` and presenting a drifted value as the failure time, the mistake +[Ordering, paging, and the failure instant](#ordering-paging-and-the-failure-instant) +exists to prevent. First rather than last is what lets +`AC-UI-INBOX-FAILED-001.30`'s promise survive a bounded page: a row sorted last +is the first row a bound discards, so "sorts last" and "no resolution failure +removes a failed task from the list" could not both hold. First is also right on +the merits, because a task that failed and cannot say when is the row most likely +to need a person. + +The status the row renders does not come from this chosen session. It is fixed to +the shared vocabulary's `failed` entry for every row on the tab, per +`AC-UI-INBOX-FAILED-001.20`. `resolveThreadSessionStatus` tests `pending_action` +before it tests `state`, and its `permission` and `clarification` entries both +carry `hasAttention: true`, so resolving status per row from whichever session +this rule picked could put an attention flag on the one bucket whose premise is +that it never carries one. Fixing the status is also why the row model needs no +session-state field, and why the no-session case needs no special handling here. + +**How the entry is reached, since it cannot be reached the usual way.** That module +exports two resolvers and no status value: both take a session-shaped argument, and +the `STATUS` table holding the entries is module-private. Fixing the status while +carrying no session field therefore leaves no way to call either resolver honestly. +The row must NOT fabricate a `{state: "FAILED"}` stand-in to pass to one: that +reintroduces exactly the `pending_action`-first precedence +`AC-UI-INBOX-FAILED-001.20a` exists to exclude. The entry is instead made directly +reachable by exporting it from the module that already defines it. Exporting an +existing value adds no entry and defines no second vocabulary, so +`AC-UI-INBOX-FAILED-001.20`'s prohibition is satisfied: it forbids a NEW or +DUPLICATED status, not a wider export of the one that ships. The module is not +reused unchanged: it gains an export. + +## Ordering, paging, and the failure instant + +**Order: unresolvable-instant first, then failure instant descending, then +`tasks.id` ascending as a byte-ordered string.** Descending because triage reads +newest-first, which is also what the deck draws. `tasks.id` is the final tiebreak +because it is the row's primary key: unique by construction, stable across reads, +and therefore enough to make the order total. An order that is only usually total is +a bug waiting for a fixture. + +**That last key names its comparison for the same reason the session tiebreak +does.** `AC-UI-INBOX-FAILED-001.30a` already pins the session-id tiebreak to an +ascending byte-ordered string; leaving the TASK-id tiebreak as bare "ascending" +leaves the comparison to the engine's text collation, the same class of +engine-dependent behaviour the null-placement key below exists to stamp out. A +locale- or case-sensitivity-dependent collation would order two rows differently on +SQLite and PostgreSQL while both satisfied the words "task identifier ascending", +and `AC-UI-INBOX-FAILED-001.10a` requires one order under every engine. The tiebreak +that makes the order total cannot itself be the key that varies. + +**The first key is explicit, and must not be left to the engine.** A row whose +failure instant is unresolvable sorts ahead of every resolved row, and that +placement is written as its own leading sort key rather than inherited from how +the store happens to place absent values. This matters because the product runs +two engines and they disagree: under `ORDER BY ... DESC`, SQLite places absent +values last and PostgreSQL places them first. An ordering that relies on the +default is therefore reproducible on one engine and not the other, which +`AC-UI-INBOX-FAILED-001.10`'s totality claim forbids outright. There is no +precedent to copy here: the backend contains no `NULLS FIRST` or `NULLS LAST` +anywhere today, because this is the first ordering key in it that can be absent. +The dual-dialect coverage that already exists for this layer +(`apps/backend/internal/db/dialect/` and the `*_postgres_test.go` twins under +`apps/backend/internal/task/repository/sqlite/`) is where the order gets asserted +on both, rather than on whichever one the developer ran. + +**The failure instant is the chosen session's `completed_at`.** Not +`tasks.updated_at`, which the input inventory shows drifting 6.2 days past the real +failure on a live row. A row saying "failed 6 days ago" when it failed 12 days ago is +a quiet lie in the one field the operator uses to decide whether to care. + +**Paging: default 50, cap 200**, the same bound the bundle read uses. Reusing the +bound rather than inventing a second one keeps AC-UI-INBOX-FAILED-001.17's capped +presentation identical across both tabs. The truncation SIGNAL is not shared; see +[Data and contracts](#data-and-contracts) for why this read carries its own +`truncated` field instead. + +**The bound is the only thing that may elide a row, and it says so.** Two ACs made +absolute-sounding promises a bounded page can break: `AC-UI-INBOX-FAILED-001.5` +lists "exactly those tasks ... and no others", and `AC-UI-INBOX-FAILED-001.30` +promises no resolution failure removes a failed task. Both are now subject to this +bound, because at 201 qualifying tasks the first is arithmetically false, and a +workspace with over 200 unresolvable instants pushes some past the bound despite +sorting first. That is not a flaw in sorting them first: first-of-N is still +discarded when N exceeds the bound, and no ordering fixes it. What sorting first +DOES guarantee is that unresolvable rows are the LAST thing the bound reaches, which +is why the key leads. The honest contract: the bound is the only force that elides a +row, and `truncated` discloses every time it did. + +**One page, and no cursor at all.** The read takes no cursor and returns none. +This is a decision, recorded in the requirements' `## Out of scope`, not an +omission: the truncation indicator is the whole of what this read says about rows +past the bound. Why it is left out rather than +finished: [Out-of-scope reasoning](#out-of-scope-reasoning). + +**Where the count comes from.** The tab's count is the length of the row array in the +same response, never a separately computed total. That makes +`AC-UI-INBOX-FAILED-001.16`'s "no rendered state exists in which the two disagree" +true by construction rather than by discipline: there is one number and the rows are +it. The sidebar badge's count is produced separately by the bundle read, untouched +here. + +## Count separation + +The badge and the tab count are produced by different reads and stored under +different per-workspace keys. The separation that matters is a WRITE separation, +and stating it as a write rule is what keeps it both strict and satisfiable: no +failed-bucket code path writes the Needs-you slice, and no Needs-you code path +writes the failed slice. + +Reading across the two slices is NOT forbidden, and one place needs it: the Needs +you empty state reads the failed slice's count for +`AC-UI-INBOX-FAILED-001.23`. Tightening this to forbid READS looks like extra safety +and is a contradiction: it forbids the only honest source for a fact another AC +demands. The invariant worth +protecting is that no failed task can ADD TO the badge's number, which is a property +of who WRITES the badge's key. A read cannot inflate a counter. + +The Needs-you empty state gains one clause naming failed tasks among what it does +not count, and pointing at the tab. That clause is conditional on the workspace +actually holding a failed task: a workspace with none must not be told that some +exist, which is the same rule the hidden-bundle disclosure already follows. + +## Control flow + +The failed bucket is read on exactly these triggers and no others: Inbox mount, +tab selection changing, +active workspace changing, the browser tab returning to visible, and a periodic +re-read every 60 seconds while the browser tab is visible and a workspace is active. +Every one of them fires REGARDLESS of which tab is currently selected. + +**The 60 seconds is a maximum staleness, not a rate limit.** It is the longest a +visible Failed tab may go without re-reading, which is what makes +`AC-UI-INBOX-FAILED-001.8`'s convergence and `AC-UI-INBOX-FAILED-001.23`'s +present-tense clause bounded rather than merely eventual. A ceiling on request +FREQUENCY would not do that: "at most once every 60 seconds" is equally satisfied by +reading once an hour, or once at mount and never again. A hidden tab is exempt while +hidden and re-reads AT ONCE on returning, not at the next tick, so the exemption +cannot turn an hour in the background into an hour of stale rows. One read per +return, not one per browser event; a return commonly fires several. + +It is never read outside the Inbox destination, so it does NOT subscribe to the +Needs-you count slice's five refresh triggers: `needs-you-inbox-02.md` makes those +slice-owned and app-wide precisely so the badge stays right while the Inbox is +closed, so reusing them would contradict the clause above and would also put +Needs-you-owned machinery in charge of failed-slice writes, which +[Count separation](#count-separation) forbids. What that severs is the SUBSCRIPTION, +not the capability: the visibility trigger above is this bucket's own, mounted with +the Inbox, and the browser-visibility hook it uses is generic and already shared by +unrelated consumers, so it carries no Needs-you machinery. This bucket runs only +while the Inbox is open because nothing outside the Inbox renders its count. + +**Why it does not stop when Needs you is selected.** The read follows the +DESTINATION, not the tab, because two things visible from Needs you depend on it. The +TAB STRIP carries the Failed count from both tabs, so a tab-scoped read leaves that +count absent or arbitrarily stale the whole time the operator is on Needs you. And +`AC-UI-INBOX-FAILED-001.23` requires the Needs you EMPTY STATE to say whether the +workspace holds a failed task; without the read it has no way to know, and the only +other source is the badge producer, which `AC-UI-INBOX-FAILED-001.14` puts off +limits. The cost is one extra bounded read-only request per Inbox open for an +operator who never opens the Failed tab. + +Reads are side-effect free and idempotent. Overlapping reads are resolved by the +same generation guard the Needs-you slice uses: each read issues a generation, and +a response whose generation is no longer current is dropped rather than applied. +The guard is keyed on the WORKSPACE and the generation, and deliberately NOT on the +selected tab. Keying it on the tab would discard exactly the responses the two +points above depend on, since those arrive while Needs you is selected; a guard +that drops them would reintroduce the stale count it was supposed to prevent. The +workspace remains part of the key because a response for a workspace the operator +has already left must never be applied, which is what +`AC-UI-INBOX-FAILED-001.26` requires. + +## Failure and recovery + +- **A failed-bucket read fails.** The Failed tab renders its error state, carries no + count, and does not claim nothing failed. The Needs you tab's rows, count and state + are untouched, owned by a different read that did not fail. Its EMPTY STATE is the + one place the two meet: `AC-UI-INBOX-FAILED-001.23`'s clause needs a failed count + that no longer exists, so the clause is OMITTED rather than asserting presence or + absence. Omitting is the honest option: claiming none exist is a lie the operator + would act on. Recovery needs no operator action and no retry affordance: the + periodic trigger re-reads within 60 seconds, and a return to visibility re-reads at + once, so the error state is bounded by the same guarantee the rows are. +- **A bundle read fails while Failed is selected.** The reverse holds. The + sidebar badge suppresses per the existing rule; the Failed tab keeps rendering. +- **A read that fails after a successful one clears the rows it was replacing**, + on the same rule and for the same reason the Inbox already applies: rows + surviving under an absent count is the disagreement the count contract forbids. +- **A task changes state during a read.** The read returns either the row or no + row. Convergence is by re-reading, never by an event: the row set must be + correct even for an exit that emits nothing. +- **A row's task is archived or deleted between read and click.** Opening it behaves + as opening any stale task reference already does; no special handling, no + optimistic removal. + +## Security + +The same rule the Inbox read already applies, through the same authorization call: +workspace scope is resolved server-side from the request identity, and a workspace +outside the caller's visible set produces the non-disclosing not-found outcome rather +than an empty success. The required scope is the workspace reach scope itself, so as +with the bundle read the forbidden outcome is unreachable by construction rather than +unimplemented. Where authentication is disabled, the single-user identity reaches +every workspace by identity resolution, not a bypass. + +No sidecar, no per-user state, and no writes at all, so there is nothing here +that a second caller can race into. + +## Out-of-scope reasoning + +The requirements list the exclusions; the argument lives here, so the contract file +stays under its size ceiling. + +- **Pagination.** Measured 2026-09-14 the instance held 15 failed tasks, 14 of them + in one workspace, so the default bound of 50 is not reached and the 200 cap is + more than an order of magnitude away. A cursor here is also harder than the + sibling's: `encodeInboxCursor` is keyed on `(created_at, pending_id)` where + `created_at` is never null, while this sort key is deliberately nullable, so its + cursor would have to encode and compare the unresolvable-instant case across a page + boundary, precisely where a row gets silently duplicated or dropped. Retry, move and + archive all remove rows under AC-UI-INBOX-FAILED-001.8, so the bucket drains by + being worked rather than scrolled. +- **Dismiss, snooze, restore.** Needs you needs per-user hiding because an + unanswered question has no exit but an answer. A failed task already has three + real exits. A fourth, cosmetic one would add a sidecar table, a hidden-count + disclosure and a restore affordance to hide something the operator can resolve. +- **Severity or grouping.** Grouping repeated identical failures is a real design + problem and belongs to a follow-up with its own evidence. A severity signal on an + uncounted bucket would also reintroduce urgency through the side door: the + Paperclip departure noted under [Prior art](#prior-art). +- **Retention.** Measured 2026-09-14, 42 archived tasks carried a failed primary + session, all excluded by AC-UI-INBOX-FAILED-001.7. Nothing here decides when the + unarchived ones age out. + +## Copy and coverage + +New copy: two tab labels, the failed empty state, the failed error state, the +relative-failure-time phrasing, six origin markers (one per non-person origin +value, plus the generic marker for an origin this capability does not enumerate), +the unknown-failure-time phrasing, the unresolvable reason fallback, and one added +clause in the Needs you empty state. + +**No new visual vocabulary.** The row's failure marker is the shared state-icon +module's TASK-state failed pairing; the tab counts are secondary-variant badges in +the tab controls. Neither is a fresh choice. The icon module carries a DIFFERENT +failed pairing under task state and under session state, so "the failed icon" is +ambiguous until the spec picks one: this bucket is defined by task state +(`AC-UI-INBOX-FAILED-001.6`), its row addresses a task, and its status is fixed from +task state, so the task-state pairing is the consistent one. The module's existing +error colour is reused as-is; no hue is added. The badge variant is load-bearing for +a similar reason: the deck chose `variant="line"` specifically BECAUSE a secondary +count badge has contrast on the resulting background and is invisible on the default +variant's track, so shipping `line` without that badge keeps the choice and discards +its justification. All of it goes through `t()`, in five +locales, with no Unicode em dash. The origin markers are six strings rather than +one because `AC-UI-INBOX-FAILED-001.9a` gives each non-person origin its own +marker, so the origin copy is five strings per locale wider than a single +"automated" word would be. + +End-to-end coverage asserts the tab strip, a listed failed row with its reason +and relative time, opening the task from the row, and, in the same run, that the +sidebar count did not move when the failed task appeared. That last assertion is +the one that would catch the most likely regression this capability can cause.