From b350f7f09ef40ab194c224bbd3d7da41032eb53f Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 12 Oct 2026 04:58:04 +0000 Subject: [PATCH] fix(server): item context's open_blocker uses the dashboard's predicate, so a reference blocker blocks (PLAN-3598 slice (b)) Lead ruling on #2094: PLAN-3535 PR 2 keeps an explicitly linked reference BLOCKER blocking, and firstActiveBlocker does the same. The brief now builds the dashboard's doneContext map and reuses its predicates instead of its own CollectionTracksWork-first cache: - open_blocker: the blocker is active when !isItemDone, firstActiveBlocker's test, so a draft doc linked as a blocker blocks until it is archived; - the subject item is open work by the dashboard's workItems rule (!isReferenceCollection && !isItemDone); - open_under_closed_parent: the parent is closed when isItemDone, in any collection, since the close guard refuses to close a parent over an open work child. A new test asserts the brief and the dashboard agree on a reference blocker, draft and archived. The lifecycle guard classifies the new isItemDone site. Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS --- .../plan3535_lifecycle_sites_guard_test.go | 4 +- internal/server/handlers_item_context.go | 86 ++++--------------- internal/server/plan3598_item_context_test.go | 70 +++++++++------ 3 files changed, 64 insertions(+), 96 deletions(-) diff --git a/internal/models/plan3535_lifecycle_sites_guard_test.go b/internal/models/plan3535_lifecycle_sites_guard_test.go index e446b777b..a1722868d 100644 --- a/internal/models/plan3535_lifecycle_sites_guard_test.go +++ b/internal/models/plan3535_lifecycle_sites_guard_test.go @@ -44,7 +44,6 @@ var lifecycleSites = map[string]string{ "internal/server/handlers_dashboard.go::isItemDone::IsTerminalItem": "keep: the done predicate itself; each caller below is classified", "internal/server/handlers_dashboard.go::childProgressState::IsAbandonedItem": "exclude: the progress predicate; isReferenceCollection answers first", "internal/server/handlers_dashboard.go::childProgressState::IsTerminalItem": "exclude: the progress predicate; isReferenceCollection answers first", - "internal/server/handlers_item_context.go::lifecycle::IsTerminalItem": "exclude: item-context conflicts (PLAN-3598); CollectionTracksWork answers first, so a reference item is neither an open blocker nor a closed parent", "internal/store/items.go::nonTerminalFilter::doneFiltersForWorkspace": "keep: the list filter for an item list's own open items (a doc's status filters its own list)", "internal/store/items.go::nonTerminalFilter::buildChildrenDoneExpr": "keep: as above", "internal/store/items.go::ListItems::nonTerminalFilter": "keep: `pad item list` / MCP list's default open filter, a list's own items", @@ -60,7 +59,8 @@ var lifecycleSites = map[string]string{ "internal/server/handlers_dashboard.go::buildDashboardResponse::childProgressState": "exclude: active-plan progress, childProgressState", // Dashboard open-work sections. - "internal/server/handlers_dashboard.go::buildDashboardResponse::isItemDone": "exclude: overdue, blocked, suggested_next and by_role read workItems; orphaned_task checks isReferenceCollection; the blocker check keeps a reference BLOCKER, an explicit link", + "internal/server/handlers_item_context.go::handleGetItemContext::isItemDone": "exclude: item-context conflicts (PLAN-3598) mirror the dashboard: the subject item checks isReferenceCollection first; a blocker keeps a reference BLOCKER, an explicit link; a closed parent of any collection is an anomaly (the close guard skips only reference children)", + "internal/server/handlers_dashboard.go::buildDashboardResponse::isItemDone": "exclude: overdue, blocked, suggested_next and by_role read workItems; orphaned_task checks isReferenceCollection; the blocker check keeps a reference BLOCKER, an explicit link", // Close guard. "internal/server/handlers_items_open_children_guard.go::runOpenChildrenGuard::isItemDone": "exclude: a reference child is skipped before the done check", diff --git a/internal/server/handlers_item_context.go b/internal/server/handlers_item_context.go index 2746dd19f..1a86478b5 100644 --- a/internal/server/handlers_item_context.go +++ b/internal/server/handlers_item_context.go @@ -204,7 +204,14 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) { } visible := s.contextRelatedVisible(r, workspaceID) - sc := newContextSchemaCache(s) + // Lifecycle reads use the dashboard's predicates (lead ruling, PLAN-3598 + // #2094): the brief must agree with firstActiveBlocker and workItems. + colls, err := s.store.ListCollectionsMinimal(workspaceID) + if err != nil { + writeInternalError(w, err) + return + } + ctxMap := buildDoneContextMap(colls) resp := itemContextResponse{ Intent: intent, Triggers: triggers, Role: role, Budget: budget, Parents: []itemContextParent{}, Conventions: []itemContextConvention{}, @@ -230,7 +237,9 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) { writeInternalError(w, err) return } - itemOpen := sc.openWork(item) + // The item is open work by the dashboard's workItems rule: a reference + // item (PLAN-3535) is not work, so it is never "open" here. + itemOpen := !isReferenceCollection(item.CollectionID, ctxMap) && !isItemDone(item.Fields, item.CollectionID, ctxMap) for i, a := range ancestors { p := itemContextParent{Ref: a.Ref, Title: a.Title, Collection: a.CollectionSlug, Seq: a.Seq, Status: contextFieldString(a.Fields, "status")} if ac := acceptance.Parse(a.Content); ac.Found { @@ -238,7 +247,10 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) { } if i == 0 { p.Content = a.Content - if itemOpen && sc.closedWork(&a) { + // A closed parent of any collection: the close guard refuses + // to close a parent over an open work child, so this pair is + // an anomaly whichever collection the parent is in. + if itemOpen && isItemDone(a.Fields, a.CollectionID, ctxMap) { resp.Conflicts = append(resp.Conflicts, itemContextConflict{Kind: "open_under_closed_parent", Refs: []string{item.Ref, a.Ref}, Detail: item.Ref + " is open but its parent " + a.Ref + " is closed"}) } @@ -306,7 +318,9 @@ func (s *Server) handleGetItemContext(w http.ResponseWriter, r *http.Request) { } depKind = append(depKind, kind) if !outgoing && itemOpen { - if other, gerr := s.store.GetItem(otherID); gerr == nil && other != nil && sc.openWork(other) { + if other, gerr := s.store.GetItem(otherID); gerr == nil && other != nil && !isItemDone(other.Fields, other.CollectionID, ctxMap) { + // firstActiveBlocker's predicate: an explicitly linked + // reference blocker still blocks (PLAN-3535 PR 2). resp.Conflicts = append(resp.Conflicts, itemContextConflict{Kind: "open_blocker", Refs: []string{item.Ref, ref}, Detail: item.Ref + " is blocked by " + ref + ", which is still open"}) } @@ -598,67 +612,3 @@ func contextFieldString(fieldsJSON, key string) string { v, _ := m[key].(string) return v } - -// contextSchemaCache answers "is this item open work, closed work, or not -// work at all?" per collection: CollectionTracksWork first, then the -// done-field-aware check (models.IsTerminalItem). -type contextSchemaCache struct { - s *Server - by map[string]*contextSchema -} - -type contextSchema struct { - schema models.CollectionSchema - settings models.CollectionSettings - ok bool -} - -func newContextSchemaCache(s *Server) *contextSchemaCache { - return &contextSchemaCache{s: s, by: map[string]*contextSchema{}} -} - -func (c *contextSchemaCache) load(it *models.Item) *contextSchema { - cs, ok := c.by[it.CollectionID] - if !ok { - cs = &contextSchema{} - if coll, err := c.s.store.GetCollection(it.CollectionID); err == nil && coll != nil { - cs.ok = models.UnmarshalItemFieldSchema([]byte(coll.Schema), &cs.schema) == nil - if coll.Settings != "" { - _ = json.Unmarshal([]byte(coll.Settings), &cs.settings) - } - } - c.by[it.CollectionID] = cs - } - return cs -} - -// lifecycle reports whether the item is work at all (a reference item, in a -// collection that does not track work, is neither open nor closed: PLAN-3535) -// and, if so, whether it is done. -func (c *contextSchemaCache) lifecycle(it *models.Item) (tracksWork, done bool) { - cs := c.load(it) - if !cs.ok { - // An unreadable collection keeps the conservative reading: open - // work. Only a collection positively known not to track work - // excludes its items. - return true, false - } - if !models.CollectionTracksWork(cs.settings) { - return false, false - } - var m map[string]any - if json.Unmarshal([]byte(it.Fields), &m) != nil { - return true, false - } - return true, models.IsTerminalItem(m, cs.schema, cs.settings) -} - -func (c *contextSchemaCache) openWork(it *models.Item) bool { - work, done := c.lifecycle(it) - return work && !done -} - -func (c *contextSchemaCache) closedWork(it *models.Item) bool { - work, done := c.lifecycle(it) - return work && done -} diff --git a/internal/server/plan3598_item_context_test.go b/internal/server/plan3598_item_context_test.go index 50410c463..860047a61 100644 --- a/internal/server/plan3598_item_context_test.go +++ b/internal/server/plan3598_item_context_test.go @@ -309,43 +309,61 @@ func TestPLAN3598_ItemContextArchivedFollowsGet(t *testing.T) { }) } -// A reference item (in a collection that does not track work, PLAN-3535) is -// neither open nor closed: a doc that blocks the item raises no open_blocker. -func TestPLAN3598_ItemContextReferenceItemIsNotOpenWork(t *testing.T) { +// The brief's open_blocker agrees with the dashboard's blocked attention on +// a REFERENCE blocker (lead ruling on #2094, PLAN-3535 PR 2): an explicitly +// linked doc blocks until it reaches its terminal status, in both places. +// The subject task has no other blocker, so the dashboard's first active +// blocker can only be the doc. +func TestPLAN3598_ItemContextReferenceBlockerAgreesWithDashboard(t *testing.T) { forEachDialect(t, func(t *testing.T, d store.DriverType) { f := newCtxFixture(t, d) - doc := f.itemIn("docs", map[string]any{"title": "Ctx reference doc"}) - if _, err := f.srv.store.CreateItemLink(f.wsID, models.ItemLinkCreate{TargetID: f.task.ID, LinkType: models.ItemLinkTypeBlocks}, doc.ID); err != nil { + task := f.itemIn("tasks", map[string]any{"title": "Ctx doc-blocked task", "fields": `{"status":"open"}`}) + doc := f.itemIn("docs", map[string]any{"title": "Ctx reference doc", "fields": `{"status":"draft"}`}) + if _, err := f.srv.store.CreateItemLink(f.wsID, models.ItemLinkCreate{TargetID: task.ID, LinkType: models.ItemLinkTypeBlocks}, doc.ID); err != nil { t.Fatal(err) } - resp, code, body := f.context(f.ownerTok, "?intent=implement") - if resp == nil { - t.Fatalf("status %d: %s", code, body) - } - var sawDoc, sawTask bool - for _, b := range resp.Deps.BlockedBy { - sawDoc = sawDoc || b.Ref == doc.Ref - } - for _, c := range resp.Conflicts { - if c.Kind != "open_blocker" { - continue - } - for _, r := range c.Refs { - if r == doc.Ref { - t.Errorf("a reference doc was reported as an open blocker: %+v", c) + briefBlocks := func() bool { + rr := f.do("GET", "/api/v1/workspaces/"+f.wsSlug+"/items/"+task.Ref+"/context", f.ownerTok, nil) + f.must(rr, http.StatusOK, "context") + var resp itemContextResponse + parseJSON(t, rr, &resp) + for _, c := range resp.Conflicts { + if c.Kind == "open_blocker" && len(c.Refs) == 2 && c.Refs[0] == task.Ref && c.Refs[1] == doc.Ref { + return true } - sawTask = sawTask || r == f.blocker.Ref } + return false } - if !sawDoc || !sawTask { - t.Errorf("precondition: the doc should be listed as a blocker (%v) and the open task blocker still conflict (%v)", sawDoc, sawTask) + dashboardBlocks := func() bool { + rr := f.do("GET", "/api/v1/workspaces/"+f.wsSlug+"/dashboard", f.ownerTok, nil) + f.must(rr, http.StatusOK, "dashboard") + var resp DashboardResponse + parseJSON(t, rr, &resp) + for _, a := range resp.Attention { + if a.Type == "blocked" && a.ItemRef == task.Ref { + return true + } + } + return false + } + + if b, dsh := briefBlocks(), dashboardBlocks(); !b || !dsh { + t.Errorf("a draft doc linked as a blocker: brief open_blocker=%v, dashboard blocked=%v; both must be true", b, dsh) + } + + archived := `{"status":"archived"}` + if _, err := f.srv.store.UpdateItem(doc.ID, models.ItemUpdate{Fields: &archived}); err != nil { + t.Fatal(err) + } + if b, dsh := briefBlocks(), dashboardBlocks(); b || dsh { + t.Errorf("an archived (terminal) doc blocker: brief open_blocker=%v, dashboard blocked=%v; both must be false", b, dsh) } }) } -// A blocker whose collection cannot be read keeps the conservative reading, -// open work, so it still raises open_blocker (Codex round 4): only a -// collection positively known not to track work is excluded. +// A blocker whose collection's schema cannot be read still raises +// open_blocker: isItemDone, the dashboard's predicate, reads an unparseable +// schema as zero-valued, so an open status stays open. func TestPLAN3598_ItemContextUnreadableCollectionStaysOpenWork(t *testing.T) { forEachDialect(t, func(t *testing.T, d store.DriverType) { f := newCtxFixture(t, d)