fix(timeline): 'commented on update' only for a comment an update carried (BUG-3437) (#1832)

* fix(timeline): 'commented on update' only for a comment an update carried (BUG-3437)

TimelineCommentCard showed 'commented on update' whenever the comment had
an activity_id, and every comment has one: a comment or reply written on
its own links to its own 'commented' activity, so a plain
'pad item comment' read as if it were attached to a change.

The store's comment list join (the one that already reads the agent name
off the linked activity) now also reads its action into
Comment.LinkedActivityAction (json:"-"), and the timeline sets an
additive, omitempty TimelineEntry.comment_on_update when that action is
anything but 'commented'. The card keys the label on it. The comments
API, the CLI and MCP keep their shape (the new field is never serialized
outside the timeline, which MCP does not expose).

Tests: the server test writes a standalone comment, a reply and an update
carrying a comment through the real routes (web, CLI and MCP share the
comments route) and checks the flag, with the old rule and an always-false
mutant each red; the web test pins the card and the entry list's
pass-through, each red with its half reverted.

Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS

* test(timeline): a comment that arrives live keeps 'commented on update' (BUG-3437)

The label comes from the server-set comment_on_update. Comments that arrive
while the timeline is open come through the SSE refresh, which re-reads the
timeline endpoint (nothing builds a comment entry client-side), so they carry
the flag. This pins it: red if the entry list stops passing the flag.

Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS

internal/models/activity.go

@@ -185,13 +185,19 @@ type TimelineEntry struct {
 	// do NOT get it: their name already lives in Activity.Metadata, and a
 	// second copy there would be a second source that can drift (TASK-2760,
 	// lead ruling on the trail).
-	AgentName string                  `json:"agent_name,omitempty"`
-	Source    string                  `json:"source"`
-	Comment   *Comment                `json:"comment,omitempty"`
-	Activity  *Activity               `json:"activity,omitempty"`
-	Version   *Version                `json:"version,omitempty"`
-	Note      *ItemImplementationNote `json:"note,omitempty"`
-	Decision  *ItemDecisionLogEntry   `json:"decision,omitempty"`
+	AgentName string `json:"agent_name,omitempty"`
+	// CommentOnUpdate is set on a "comment" entry whose comment an item update
+	// carried (`pad item update --comment`, the update's "updated" activity),
+	// and false for a comment written on its own, whose linked activity is its
+	// own "commented" row (BUG-3437). The card's "commented on update" label
+	// keys on it; it used to key on activity_id, which EVERY comment has.
+	CommentOnUpdate bool                    `json:"comment_on_update,omitempty"`
+	Source          string                  `json:"source"`
+	Comment         *Comment                `json:"comment,omitempty"`
+	Activity        *Activity               `json:"activity,omitempty"`
+	Version         *Version                `json:"version,omitempty"`
+	Note            *ItemImplementationNote `json:"note,omitempty"`
+	Decision        *ItemDecisionLogEntry   `json:"decision,omitempty"`
 	// AutosaveRun is set on a collab-snapshot version entry that stands for a
 	// collapsed run of autosaves (PLAN-2348 U3): the entry is the run's
 	// NEWEST row, and this says what the dropped older rows were.

internal/models/comment.go

@@ -33,8 +33,17 @@ type Comment struct {
 	Imported bool `json:"imported,omitempty"`
 
 	// Populated by joins (not stored)
-	ItemTitle string `json:"item_title,omitempty"`
-	ItemSlug  string `json:"item_slug,omitempty"`
+	//
+	// LinkedActivityAction is the action of the activity ActivityID points at,
+	// read by the same list join as AgentName (BUG-3437): "commented" for a
+	// comment or reply on its own, "updated" (or another change) for a comment
+	// an item update carried. Never serialized: the item timeline turns it into
+	// TimelineEntry.CommentOnUpdate, and nothing else needs it, so the
+	// comments API and MCP keep their shape. Empty on a comment read any other
+	// way, and on one with no linked activity.
+	LinkedActivityAction string `json:"-"`
+	ItemTitle            string `json:"item_title,omitempty"`
+	ItemSlug             string `json:"item_slug,omitempty"`
 	// AgentName is the display name the writing agent declared (the
 	// X-Pad-Agent header), read off the activity this comment's ActivityID
 	// points at — the `commented` row a comment or reply logs, or the

internal/server/bug3437_comment_on_update_test.go

@@ -0,0 +1,88 @@
+package server
+
+import (
+	"encoding/json"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+// BUG-3437: the timeline's "commented on update" label keyed on the comment's
+// activity_id, and EVERY comment has one: a comment or reply written on its own
+// links to its own "commented" activity. The timeline now says which: an
+// entry's comment_on_update is true only when an item update carried the
+// comment. The web, the CLI (`pad item comment`) and MCP all write a comment
+// through the same comments route, so the standalone leg covers all three.
+func TestBug3437_CommentOnUpdateOnlyForAnUpdatesComment(t *testing.T) {
+	t.Parallel()
+	srv := testServer(t)
+	ws := createTestWorkspaceViaAPI(t, srv)
+	item := timelineItemWithStructured(t, srv, ws, "", "")
+
+	standalone := postComment(t, srv, ws, item.Slug, "", "a plain comment")
+	reply := postReply(t, srv, ws, standalone.ID, "", "a plain reply")
+
+	// An update that carries a comment, the way `pad item update --comment` sends it.
+	rr := doAttributionRequest(t, srv, "", "PATCH", "/api/v1/workspaces/"+ws+"/items/"+item.Slug,
+		map[string]any{"title": item.Title + " (renamed)", "comment": "why I renamed it"})
+	if rr.Code != http.StatusOK {
+		t.Fatalf("update with comment: %d %s", rr.Code, rr.Body.String())
+	}
+	var updated struct {
+		Slug string `json:"slug"`
+	}
+	if err := json.Unmarshal(rr.Body.Bytes(), &updated); err != nil || updated.Slug == "" {
+		t.Fatalf("update response: %v %s", err, rr.Body.String())
+	}
+	item.Slug = updated.Slug // a rename moves the slug
+
+	resp := fetchTimeline(t, srv, ws, item.Slug, "")
+	var onUpdateID string
+	for _, e := range resp.Entries {
+		if e.Kind == "comment" && e.Comment != nil && e.Comment.Body == "why I renamed it" {
+			onUpdateID = e.ID
+			if !e.CommentOnUpdate {
+				t.Errorf("the update's comment: comment_on_update = false, want true")
+			}
+		}
+	}
+	if onUpdateID == "" {
+		t.Fatalf("the update's comment is not on the timeline: %+v", resp.Entries)
+	}
+	entry := commentEntryByID(resp.Entries, standalone.ID)
+	if entry == nil {
+		t.Fatalf("standalone comment not on the timeline")
+	}
+	if entry.CommentOnUpdate {
+		t.Errorf("a standalone comment: comment_on_update = true, want false")
+	}
+	if entry.Comment.ActivityID == "" {
+		t.Errorf("control: the standalone comment has no activity_id, so keying on it would not have failed")
+	}
+	// The reply is nested under its parent, and the label is a top-level
+	// entry's: a reply never becomes an entry with the flag.
+	for _, r := range entry.Comment.Replies {
+		if r.ID == reply.ID && r.ActivityID == "" {
+			t.Errorf("control: the reply has no activity_id")
+		}
+	}
+
+	// The comments API and MCP keep their shape: the linked action is never serialized.
+	rr = doAttributionRequest(t, srv, "", "GET", "/api/v1/workspaces/"+ws+"/items/"+item.Slug+"/comments", nil)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("list comments: %d", rr.Code)
+	}
+	body := rr.Body.String()
+	for _, k := range []string{"linked_activity_action", "LinkedActivityAction", "comment_on_update"} {
+		if strings.Contains(body, k) {
+			t.Errorf("the comments API now carries %q", k)
+		}
+	}
+	var list []map[string]any
+	if err := json.Unmarshal(rr.Body.Bytes(), &list); err != nil {
+		var wrapped map[string][]map[string]any
+		if err2 := json.Unmarshal(rr.Body.Bytes(), &wrapped); err2 != nil {
+			t.Fatalf("decode comments: %v / %v", err, err2)
+		}
+	}
+}

internal/server/handlers_timeline.go

@@ -547,8 +547,11 @@ func buildTimeline(comments []models.Comment, activities []models.Activity, vers
 			// Derived from the nested comment (see TimelineEntry.AgentName);
 			// the store's join is the only writer of the value being copied.
 			AgentName: c.AgentName,
-			Source:    c.Source,
-			Comment:   &comments[i],
+			// A comment an update carried links to the update's activity; one
+			// written on its own links to its own "commented" row (BUG-3437).
+			CommentOnUpdate: c.LinkedActivityAction != "" && c.LinkedActivityAction != "commented",
+			Source:          c.Source,
+			Comment:         &comments[i],
 		}
 		entries = append(entries, entry)
 	}

internal/store/comments.go

@@ -347,7 +347,7 @@ func (s *Store) getCommentQ(q Queryer, id string) (*models.Comment, error) {
 const commentListCols = `c.id, c.item_id, c.workspace_id, c.author, COALESCE(c.user_id, ''), c.body,
 		       c.created_by, c.source, COALESCE(c.activity_id, ''), COALESCE(c.parent_id, ''),
 		       c.created_at, c.updated_at,
-		       CASE WHEN c.deleted_at IS NULL THEN 0 ELSE 1 END, c.imported, a.metadata`
+		       CASE WHEN c.deleted_at IS NULL THEN 0 ELSE 1 END, c.imported, a.metadata, a.action`
 
 const commentAgentJoin = `LEFT JOIN activities a ON a.id = c.activity_id AND a.document_id = c.item_id`
 
@@ -356,12 +356,12 @@ func scanComments(rows *sql.Rows) ([]models.Comment, error) {
 	for rows.Next() {
 		var c models.Comment
 		var createdAt, updatedAt string
-		var activityMeta sql.NullString
+		var activityMeta, activityAction sql.NullString
 		var deleted, imported int
 		if err := rows.Scan(
 			&c.ID, &c.ItemID, &c.WorkspaceID, &c.Author, &c.UserID, &c.Body,
 			&c.CreatedBy, &c.Source, &c.ActivityID, &c.ParentID,
-			&createdAt, &updatedAt, &deleted, &imported, &activityMeta,
+			&createdAt, &updatedAt, &deleted, &imported, &activityMeta, &activityAction,
 		); err != nil {
 			return nil, fmt.Errorf("scan comment: %w", err)
 		}
@@ -372,6 +372,7 @@ func scanComments(rows *sql.Rows) ([]models.Comment, error) {
 		if activityMeta.Valid {
 			c.AgentName = models.AgentNameFromMetadata(activityMeta.String)
 		}
+		c.LinkedActivityAction = activityAction.String
 		comments = append(comments, c)
 	}
 	return comments, rows.Err()

web/src/lib/components/timeline/TimelineCommentCard.svelte

@@ -10,6 +10,12 @@
 
 	interface Props {
 		comment: Comment;
+		/**
+		 * The comment came with an item update (the timeline entry's
+		 * `comment_on_update`, BUG-3437). Only then is it "commented on
+		 * update": every comment has an activity_id, a standalone one included.
+		 */
+		onUpdate?: boolean;
 		wsSlug: string;
 		username?: string;
 		items: Item[];
@@ -57,7 +63,7 @@
 		onRemoveReaction: (commentId: string, emoji: string) => void;
 	}
 
-	let { comment, wsSlug, username = '', items, currentUserId = '', canEdit = true, frozen = false, attachmentResolver, isAdmin = false, hostToken = '', onDelete, onReply, onEdit, onReaction, onRemoveReaction }: Props = $props();
+	let { comment, onUpdate = false, wsSlug, username = '', items, currentUserId = '', canEdit = true, frozen = false, attachmentResolver, isAdmin = false, hostToken = '', onDelete, onReply, onEdit, onReaction, onRemoveReaction }: Props = $props();
 
 	let showReplyForm = $state(false);
 	let submittingReply = $state(false);
@@ -247,7 +253,7 @@
 		{/if}
 	</div>
 
-	{#if comment.activity_id}
+	{#if onUpdate}
 		<div class="activity-label">commented on update</div>
 	{/if}
 

web/src/lib/components/timeline/TimelineEntryList.svelte

@@ -135,6 +135,7 @@
 				{#if entry.kind === 'comment' && entry.comment}
 					<TimelineCommentCard
 						comment={entry.comment}
+						onUpdate={entry.comment_on_update === true}
 						{wsSlug}
 						{username}
 						{items}

web/src/lib/components/timeline/timelineCommentOnUpdate.svelte.test.ts

@@ -0,0 +1,67 @@
+import { describe, it, expect } from 'vitest';
+import { render } from '@testing-library/svelte';
+import TimelineCommentCard from './TimelineCommentCard.svelte';
+import TimelineEntryList from './TimelineEntryList.svelte';
+import type { Comment, TimelineEntry } from '$lib/types';
+
+/**
+ * BUG-3437. "commented on update" used to show for any comment with an
+ * activity_id, and every comment has one: a standalone comment links to its
+ * own "commented" activity. The label now follows the timeline entry's
+ * comment_on_update, which the server sets only for a comment an item update
+ * carried.
+ */
+function comment(overrides: Partial<Comment> = {}): Comment {
+	return {
+		id: 'c-1',
+		item_id: 'item-1',
+		workspace_id: 'ws-1',
+		author: 'Dave',
+		body: 'hello',
+		created_by: 'user',
+		source: 'cli',
+		activity_id: 'a-1', // every comment has one
+		created_at: new Date('2026-10-06T12:00:00Z').toISOString(),
+		updated_at: new Date('2026-10-06T12:00:00Z').toISOString(),
+		...overrides
+	};
+}
+
+const noop = () => {};
+const handlers = { onDelete: noop, onReply: noop, onEdit: noop, onReaction: noop, onRemoveReaction: noop };
+
+function entry(onUpdate: boolean | undefined): TimelineEntry {
+	return {
+		id: 'c-1',
+		kind: 'comment',
+		created_at: '2026-10-06T12:00:00Z',
+		actor: 'user',
+		source: 'cli',
+		comment: comment(),
+		...(onUpdate === undefined ? {} : { comment_on_update: onUpdate })
+	} as TimelineEntry;
+}
+
+describe('"commented on update" label (BUG-3437)', () => {
+	it('a standalone comment with an activity_id shows no label', () => {
+		const { queryByText } = render(TimelineCommentCard, { comment: comment(), wsSlug: 'ws', items: [], ...handlers });
+		expect(queryByText('commented on update')).toBeNull();
+	});
+
+	it('the card shows it when told the comment came with an update', () => {
+		const { getByText } = render(TimelineCommentCard, { comment: comment(), onUpdate: true, wsSlug: 'ws', items: [], ...handlers });
+		expect(getByText('commented on update')).toBeTruthy();
+	});
+
+	it('the entry list passes comment_on_update through, and only true counts', () => {
+		for (const [flag, shown] of [
+			[true, true],
+			[false, false],
+			[undefined, false]
+		] as const) {
+			const { queryByText, unmount } = render(TimelineEntryList, { entries: [entry(flag)], wsSlug: 'ws', items: [], ...handlers });
+			expect(queryByText('commented on update') !== null, `comment_on_update=${flag}`).toBe(shown);
+			unmount();
+		}
+	});
+});

web/src/lib/components/timeline/timelineCommentOnUpdateLive.svelte.test.ts

@@ -0,0 +1,189 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { flushSync, mount, unmount, tick } from 'svelte';
+import type { Comment, TimelineEntry, TimelineResponse } from '$lib/types';
+
+/**
+ * BUG-3437, the live half. "commented on update" comes from the timeline
+ * entry's comment_on_update, which only the SERVER sets. A comment that
+ * arrives while the timeline is open (an SSE comment_created, or an update
+ * with --comment from the CLI or MCP) reaches the view through the SSE
+ * refresh, which re-reads the timeline endpoint, so it carries the flag like
+ * a reload does. This pins that: if a future change built the entry
+ * client-side, the label would silently disappear until reload.
+ *
+ * Harness copied from timelineRefreshDeletion.svelte.test.ts (module mocks
+ * are per-file).
+ */
+
+type ListParams = { limit?: number; before?: string; before_id?: string; kinds?: readonly string[] } | undefined;
+
+const pages: TimelineResponse[] = [];
+const timelineListMock = vi.fn(async (_ws: string, _slug: string, _params: ListParams) => {
+	return pages.shift() ?? { entries: [], has_more: false };
+});
+
+vi.mock('$lib/api/client', () => ({
+	api: {
+		timeline: {
+			list: (ws: string, slug: string, params: ListParams) => timelineListMock(ws, slug, params),
+		},
+		comments: {
+			create: vi.fn(),
+			update: vi.fn(),
+			delete: vi.fn(),
+			addReaction: vi.fn(),
+			removeReaction: vi.fn(),
+		},
+		attachments: {
+			downloadUrl: (ws: string, id: string) => `/api/v1/workspaces/${ws}/attachments/${id}`,
+		},
+	},
+}));
+
+// FANS OUT rather than only recording: a mock that returns a disposer leaves
+// the component subscribed to nothing and every refresh assertion passes
+// vacuously (BUG-2509's lesson).
+let itemEventCb: ((event: { type: string }) => void) | undefined;
+vi.mock('$lib/services/sse.svelte', () => ({
+	sseService: {
+		onItemEvent: (cb: (event: { type: string }) => void) => {
+			itemEventCb = cb;
+			return () => {
+				itemEventCb = undefined;
+			};
+		},
+	},
+}));
+
+vi.mock('$lib/stores/auth.svelte', () => ({
+	authStore: {
+		userId: 'user-1',
+		user: { id: 'user-1', role: 'member' },
+		// ItemTimeline fences every load and handler on identity (BUG-3105);
+		// identity never changes in this suite.
+		identityFence: () => () => true,
+	},
+}));
+
+vi.mock('$lib/stores/workspace.svelte', () => ({
+	workspaceStore: { canEditItem: () => false },
+}));
+
+vi.mock('$lib/components/CommentEditor.svelte', async () => ({
+	default: (await import('./fixtures/InertCommentEditor.svelte')).default,
+}));
+
+const { default: ItemTimeline } = await import('./ItemTimeline.svelte');
+
+function entry(id: string, createdAt: string, body: string, onUpdate?: boolean): TimelineEntry {
+	const c: Comment = {
+		id,
+		item_id: 'item-a',
+		workspace_id: 'ws-1',
+		author: 'alice',
+		body,
+		created_by: 'alice',
+		source: 'web',
+		created_at: createdAt,
+		updated_at: createdAt,
+	};
+	return {
+		id,
+		kind: 'comment',
+		created_at: createdAt,
+		actor: 'alice',
+		source: 'cli',
+		comment: { ...c, activity_id: `act-${id}` }, // every comment has one
+		...(onUpdate ? { comment_on_update: true } : {})
+	};
+}
+
+let host: HTMLElement;
+let app: Record<string, unknown> | null = null;
+
+const props = $state({
+	wsSlug: 'ws',
+	username: 'alice',
+	itemSlug: 'TASK-1',
+	currentContent: '',
+	itemId: 'item-a',
+	collectionId: 'coll-1',
+	hostToken: 'host-1',
+	resourceGen: 0,
+	visibleKinds: undefined as Array<'comment' | 'activity' | 'version'> | undefined,
+	mutationsEnabled: false,
+});
+
+async function settle() {
+	for (let i = 0; i < 10; i++) {
+		await tick();
+		flushSync();
+	}
+}
+
+/**
+ * Fire a relevant SSE event and let the debounced refresh run. The component
+ * debounces these by 500ms, so the wait has to clear it — and the helper
+ * ASSERTS the refresh actually happened, because every expectation below is
+ * about what the refresh did and would pass vacuously if it never ran.
+ */
+async function fireRefresh() {
+	const before = timelineListMock.mock.calls.length;
+	itemEventCb?.({ type: 'comment_created' });
+	await new Promise((r) => setTimeout(r, 700));
+	await settle();
+	if (timelineListMock.mock.calls.length === before) {
+		throw new Error('the SSE refresh never fired — every assertion after this would be vacuous');
+	}
+}
+
+function shownBodies(): string[] {
+	return Array.from(host.querySelectorAll('.comment-card')).map((el) =>
+		(el.textContent ?? '').trim()
+	);
+}
+
+function shows(label: string): boolean {
+	return shownBodies().some((b) => b.includes(label));
+}
+
+beforeEach(() => {
+	host = document.createElement('div');
+	document.body.appendChild(host);
+	pages.length = 0;
+	timelineListMock.mockClear();
+});
+
+afterEach(() => {
+	if (app) unmount(app);
+	app = null;
+	host.remove();
+});
+
+function labelled(body: string): boolean {
+	const card = Array.from(host.querySelectorAll('.comment-card')).find((el) => (el.textContent ?? '').includes(body));
+	if (!card) throw new Error(`no card for "${body}"`);
+	return (card.textContent ?? '').includes('commented on update');
+}
+
+describe('"commented on update" on a comment that arrives live (BUG-3437)', () => {
+	it('an SSE refresh brings the label for an update\'s comment, and not for a plain one', async () => {
+		pages.push({ entries: [entry('e-plain', '2026-10-06T12:00:01Z', 'a plain comment')], has_more: false });
+		pages.push({
+			entries: [
+				entry('e-upd', '2026-10-06T12:00:09Z', 'why I renamed it', true),
+				entry('e-plain', '2026-10-06T12:00:01Z', 'a plain comment')
+			],
+			has_more: false
+		});
+
+		app = mount(ItemTimeline, { target: host, props }) as Record<string, unknown>;
+		await settle();
+		expect(labelled('a plain comment')).toBe(false);
+
+		await fireRefresh();
+
+		expect(labelled('why I renamed it'), 'the live update comment lost its label').toBe(true);
+		expect(labelled('a plain comment')).toBe(false);
+	});
+});

web/src/lib/types/index.ts

@@ -1730,6 +1730,8 @@ export interface TimelineEntry {
 	 * second copy here (TASK-2760).
 	 */
 	agent_name?: string;
+	/** A comment entry whose comment an item update carried (BUG-3437); absent for a standalone comment. */
+	comment_on_update?: boolean;
 	source: string;
 	comment?: Comment;
 	activity?: Activity;