From d3b7bbf399e47e8df3db21239cb8ada768185240 Mon Sep 17 00:00:00 2001 From: Harry19081 <20519290+Harry19081@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:32:18 +0800 Subject: [PATCH] fix(collab): stamp imported replays with the source's activity time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking a cloud-org card in Kanban imports a local replay copy of the teammate's session, and the board then re-derives the card from that local row instead of the cloud row. The copy was stamped with the import moment, so Started / Last updated jumped to the click, the card read "Now", the row jumped to the top of List/Diary, and an old session was pulled back out of the auto-archived column. The copy now adopts the owner's `lastActivityAt` — the same value the pre-click cloud card renders — falling back to the import moment only for rows that carry none. `importedFrom.importedAt` still records now. `upsertSession` pins timestamps by policy, which meant an already- imported copy could never be corrected. Adds `applyImportedSessionTimestamps`, a fourth documented path in that policy, guarded to rows carrying `importedFrom`, so an imported mirror tracks its source while locally-owned sessions stay pinned. Reopening a previously-clicked card heals it in place. Pre-commit hook ran. Total eslint: 0, total circular: 0 --- .../engine/collabSessionImport.ts | 89 ++++++++++- .../engine/collabSyncEngineHelpers.test.ts | 142 ++++++++++++++++++ .../sessionAtom/__tests__/mutations.test.ts | 85 ++++++++++- src/store/session/sessionAtom/mutations.ts | 49 ++++++ 4 files changed, 360 insertions(+), 5 deletions(-) diff --git a/src/features/TeamCollaboration/engine/collabSessionImport.ts b/src/features/TeamCollaboration/engine/collabSessionImport.ts index b354a060e..f34b5b30b 100644 --- a/src/features/TeamCollaboration/engine/collabSessionImport.ts +++ b/src/features/TeamCollaboration/engine/collabSessionImport.ts @@ -12,7 +12,10 @@ import { buildCloudOrgSelectorValue } from "@src/features/Org2Cloud/org2CloudOrg import { createLogger } from "@src/hooks/logger"; import { sessionsAtom } from "@src/store/session/sessionAtom/atoms"; import { recordGuestImportedSession } from "@src/store/session/sessionAtom/guestImportRegistry"; -import { upsertSession } from "@src/store/session/sessionAtom/mutations"; +import { + applyImportedSessionTimestamps, + upsertSession, +} from "@src/store/session/sessionAtom/mutations"; import { persistSessions } from "@src/store/session/sessionAtom/persistence"; import type { Session, @@ -459,6 +462,42 @@ async function streamIncrementalRemoteSessionToCache( } } +/** + * Activity time of the OWNER's session, for the imported copy's timestamps. + * + * The replay copy describes someone else's work; stamping it with the moment + * the viewer clicked made every card jump its Started / Last updated to "Now" + * on first open, reordered List/Diary around the click, and pulled an old + * session back out of the auto-archived column. Cloud metadata carries no + * creation timestamp, so `lastActivityAt` is the only source-side time we + * have — the same proxy the pre-click cloud card itself renders. Undefined + * for pre-`lastActivityAt` rows, which keep whatever local stamp they have. + */ +function readSourceActivityAt( + remoteSession: ImportRemoteSessionOptions["remoteSession"] +): string | undefined { + const lastActivityAt = remoteSession.lastActivityAt; + if (!lastActivityAt) return undefined; + return Number.isFinite(Date.parse(lastActivityAt)) + ? lastActivityAt + : undefined; +} + +/** + * A session cannot have been created after its own last activity. Keeping the + * earlier of the two also heals rows imported before the fix above, whose + * `created_at` is the old import-click stamp. + */ +function resolveImportedCreatedAt( + existingCreatedAt: string | undefined, + activityAt: string +): string { + if (!existingCreatedAt) return activityAt; + const existingMs = Date.parse(existingCreatedAt); + if (!Number.isFinite(existingMs)) return activityAt; + return existingMs <= Date.parse(activityAt) ? existingCreatedAt : activityAt; +} + function resolveImportedSourceDisplay( remoteSession: ImportRemoteSessionOptions["remoteSession"], existing: Session | undefined @@ -526,8 +565,21 @@ function refreshImportedSessionPresentation( existing.orgId === importedFrom.orgId ? buildCloudOrgSelectorValue(importedFrom.orgId) : existing.orgId; + // Same healing rationale for the source-activity timestamps: a copy imported + // before they were tracked carries the old import-click stamp, and a + // cursor-current reopen never reaches the write path that would correct it. + const activityAt = readSourceActivityAt(remoteSession); + const createdAt = activityAt + ? resolveImportedCreatedAt(existing.created_at, activityAt) + : existing.created_at; + const timestampsUnchanged = + !activityAt || + (existing.created_at === createdAt && + existing.updated_at === activityAt && + existing.completed_at === activityAt); const unchanged = existing.orgId === normalizedOrgId && + timestampsUnchanged && existing.name === remoteSession.title && existing.repoPath === repoPath && existing.agentDisplayName === sourcePresentation.agentLabel && @@ -547,6 +599,13 @@ function refreshImportedSessionPresentation( const refreshed: Session = { ...existing, ...(normalizedOrgId !== undefined ? { orgId: normalizedOrgId } : {}), + ...(activityAt + ? { + created_at: createdAt, + updated_at: activityAt, + completed_at: activityAt, + } + : {}), name: remoteSession.title, repoPath, agentDisplayName: sourcePresentation.agentLabel, @@ -554,6 +613,14 @@ function refreshImportedSessionPresentation( importedFrom: refreshedImportedFrom, }; upsertSession(refreshed); + if (activityAt) { + // upsertSession pins timestamps; this row's clock is the source's. + applyImportedSessionTimestamps(existing.session_id, { + created_at: createdAt ?? activityAt, + updated_at: activityAt, + completed_at: activityAt, + }); + } recordGuestImportedSession(refreshed); persistSessions(getInstrumentedStore().get(sessionsAtom) as Session[]); } @@ -818,6 +885,13 @@ async function importRemoteSessionInner( try { throwIfAborted(options.signal); const now = new Date().toISOString(); + // Source-side activity time, NOT `now`: see readSourceActivityAt. + // `importedAt` below stays `now` — that one really is about this device. + const activityAt = readSourceActivityAt(remoteSession) ?? now; + const createdAt = resolveImportedCreatedAt( + existing?.created_at, + activityAt + ); const importedFrom: SessionImportedFrom = { orgId, sourceSessionId: remoteSession.sourceSessionId, @@ -847,9 +921,9 @@ async function importRemoteSessionInner( const importedRow: Session = { session_id: localSessionId, status: "completed", - created_at: existing?.created_at ?? now, - updated_at: now, - completed_at: now, + created_at: createdAt, + updated_at: activityAt, + completed_at: activityAt, name: remoteSession.title, repoPath: remoteSession.repoPath, category: "external_history", @@ -909,6 +983,13 @@ async function importRemoteSessionInner( // No await after the final abort check: the session row, guest registry // and persisted list commit synchronously as one local critical section. upsertSession(importedRow); + // Re-import of an existing copy: upsertSession pins timestamps against + // careless reconcile writes, but this row's clock belongs to the source. + applyImportedSessionTimestamps(localSessionId, { + created_at: createdAt, + updated_at: activityAt, + completed_at: activityAt, + }); recordGuestImportedSession(importedRow); persistSessions(store.get(sessionsAtom) as Session[]); } catch (error) { diff --git a/src/features/TeamCollaboration/engine/collabSyncEngineHelpers.test.ts b/src/features/TeamCollaboration/engine/collabSyncEngineHelpers.test.ts index 40c7c5b37..37bb53800 100644 --- a/src/features/TeamCollaboration/engine/collabSyncEngineHelpers.test.ts +++ b/src/features/TeamCollaboration/engine/collabSyncEngineHelpers.test.ts @@ -809,6 +809,148 @@ describe("importRemoteSession", () => { }); }); + it("stamps the imported copy with the source's activity time, not the click", async () => { + // Regression: a fresh import stamped created_at/updated_at/completed_at + // with `now`, so opening a cloud card in Kanban flipped its Started / + // Last updated to the moment of the click and dragged the row to the top + // of List/Diary. + const client = { + getSessionEventSegments: vi.fn(async () => sealSnapshot(makeSnapshot())), + } satisfies Pick; + + const result = await importRemoteSession({ + client, + orgId: "org-1", + remoteSession: makeRemote({ lastActivityAt: "2026-06-01T09:30:00.000Z" }), + }); + + const record = (store.get(sessionsAtom) as Session[]).find( + (session) => session.session_id === result!.localSessionId + )!; + expect(record.created_at).toBe("2026-06-01T09:30:00.000Z"); + expect(record.updated_at).toBe("2026-06-01T09:30:00.000Z"); + expect(record.completed_at).toBe("2026-06-01T09:30:00.000Z"); + // The import moment still belongs on the provenance cursor. + expect(record.importedFrom?.importedAt).not.toBe( + "2026-06-01T09:30:00.000Z" + ); + }); + + it("falls back to the import moment when the row carries no activity time", async () => { + const client = { + getSessionEventSegments: vi.fn(async () => sealSnapshot(makeSnapshot())), + } satisfies Pick; + + const result = await importRemoteSession({ + client, + orgId: "org-1", + remoteSession: makeRemote({ lastActivityAt: undefined }), + }); + + const record = (store.get(sessionsAtom) as Session[]).find( + (session) => session.session_id === result!.localSessionId + )!; + expect(record.updated_at).toBe(record.importedFrom?.importedAt); + expect(record.created_at).toBe(record.updated_at); + }); + + it("heals an import-click timestamp on a refresh-only reopen", async () => { + // Copies imported before the fix above carry the click stamp, and a + // cursor-current reopen never reaches the write path — heal them here or + // they show the wrong Started / Last updated forever. + const client = { + getSessionEventSegments: vi.fn(async () => sealSnapshot(makeSnapshot())), + } satisfies Pick; + const expectedId = await deriveImportedSessionId("org-1", "remote-1"); + store.set(sessionsAtom, [ + { + session_id: expectedId, + status: "completed", + created_at: "2026-07-20T12:00:00.000Z", + updated_at: "2026-07-20T12:00:00.000Z", + completed_at: "2026-07-20T12:00:00.000Z", + name: "Remote session", + orgId: "cloud:org-1", + importedFrom: { + orgId: "org-1", + sourceSessionId: "remote-1", + ownerMemberId: "m2", + ownerDisplayName: "Bob", + epoch: 1, + seq: 1, + count: 1, + frozenCount: 1, + tailHash: undefined, + importedAt: "2026-07-20T12:00:00.000Z", + }, + }, + ]); + eventStoreMock.getPersistedEvents.mockResolvedValue([ + { id: "e1" } as unknown as SessionEvent, + ]); + eventStoreMock.countPersistedEvents.mockResolvedValue(1); + + const result = await importRemoteSession({ + client, + orgId: "org-1", + remoteSession: makeRemote({ lastActivityAt: "2026-06-01T09:30:00.000Z" }), + }); + + const record = (store.get(sessionsAtom) as Session[]).find( + (session) => session.session_id === expectedId + )!; + expect(result?.updated).toBe(false); + expect(client.getSessionEventSegments).not.toHaveBeenCalled(); + expect(record.created_at).toBe("2026-06-01T09:30:00.000Z"); + expect(record.updated_at).toBe("2026-06-01T09:30:00.000Z"); + expect(record.completed_at).toBe("2026-06-01T09:30:00.000Z"); + }); + + it("keeps a created_at that predates the source's last activity", async () => { + const client = { + getSessionEventSegments: vi.fn(async () => sealSnapshot(makeSnapshot())), + } satisfies Pick; + const expectedId = await deriveImportedSessionId("org-1", "remote-1"); + store.set(sessionsAtom, [ + { + session_id: expectedId, + status: "completed", + created_at: "2026-05-01T08:00:00.000Z", + updated_at: "2026-06-01T09:30:00.000Z", + completed_at: "2026-06-01T09:30:00.000Z", + name: "Remote session", + orgId: "cloud:org-1", + importedFrom: { + orgId: "org-1", + sourceSessionId: "remote-1", + ownerMemberId: "m2", + ownerDisplayName: "Bob", + epoch: 1, + seq: 1, + count: 1, + frozenCount: 1, + tailHash: undefined, + importedAt: "2026-06-01T10:00:00.000Z", + }, + }, + ]); + eventStoreMock.getPersistedEvents.mockResolvedValue([ + { id: "e1" } as unknown as SessionEvent, + ]); + eventStoreMock.countPersistedEvents.mockResolvedValue(1); + + await importRemoteSession({ + client, + orgId: "org-1", + remoteSession: makeRemote({ lastActivityAt: "2026-06-01T09:30:00.000Z" }), + }); + + const record = (store.get(sessionsAtom) as Session[]).find( + (session) => session.session_id === expectedId + )!; + expect(record.created_at).toBe("2026-05-01T08:00:00.000Z"); + }); + it("stamps Session.orgId on a MEMBER import so the sidebar org filter matches", async () => { const client = { getSessionEventSegments: vi.fn(async () => sealSnapshot(makeSnapshot())), diff --git a/src/store/session/sessionAtom/__tests__/mutations.test.ts b/src/store/session/sessionAtom/__tests__/mutations.test.ts index 0874f942e..6eed640e0 100644 --- a/src/store/session/sessionAtom/__tests__/mutations.test.ts +++ b/src/store/session/sessionAtom/__tests__/mutations.test.ts @@ -3,10 +3,12 @@ * * Backend-owned fields (`created_at`, `updated_at`, and their `*_time` * aliases) MUST NOT drift through frontend-only writes. These tests - * lock that contract for the two mutation entry points: + * lock that contract for the mutation entry points: * * - `upsertSession` (insert + update) * - `updateSessionStatus` + * - `applyImportedSessionTimestamps` — the one sanctioned override, + * narrowed to imported replay copies whose clock is the source's * * They are paranoid by design: the regression they protect against * (clicking an old session in WorkStation makes it appear in the 6h @@ -31,11 +33,24 @@ async function loadModule() { return { upsertSession: mutations.upsertSession, updateSessionStatus: mutations.updateSessionStatus, + applyImportedSessionTimestamps: mutations.applyImportedSessionTimestamps, sessionsAtom: atoms.sessionsAtom, store: getInstrumentedStore(), }; } +const IMPORTED_FROM = { + orgId: "org-1", + sourceSessionId: "remote-1", + ownerMemberId: "m2", + ownerDisplayName: "Bob", + epoch: 1, + seq: 1, + count: 1, + frozenCount: 1, + importedAt: "2026-07-20T12:00:00.000Z", +} as const; + function makeSession(overrides: Partial = {}): Session { return { session_id: "sess-1", @@ -114,6 +129,74 @@ describe("upsertSession", () => { }); }); +describe("applyImportedSessionTimestamps", () => { + const SOURCE_TIMES = { + created_at: "2026-06-01T09:30:00.000Z", + updated_at: "2026-06-01T09:30:00.000Z", + completed_at: "2026-06-01T09:30:00.000Z", + }; + + it("overrides the pinned timestamps on an imported replay copy", async () => { + const { + upsertSession, + applyImportedSessionTimestamps, + sessionsAtom, + store, + } = await loadModule(); + // The pre-fix state: the copy carries the moment the viewer clicked it. + upsertSession( + makeSession({ + session_id: "imported-1", + created_at: "2026-07-20T12:00:00.000Z", + updated_at: "2026-07-20T12:00:00.000Z", + completed_at: "2026-07-20T12:00:00.000Z", + importedFrom: IMPORTED_FROM, + }) + ); + + applyImportedSessionTimestamps("imported-1", SOURCE_TIMES); + + expect(store.get(sessionsAtom)[0]).toMatchObject(SOURCE_TIMES); + }); + + it("leaves a locally-owned session's timestamps pinned", async () => { + const { + upsertSession, + applyImportedSessionTimestamps, + sessionsAtom, + store, + } = await loadModule(); + upsertSession(makeSession({ session_id: "local-1" })); + + applyImportedSessionTimestamps("local-1", SOURCE_TIMES); + + const after = store.get(sessionsAtom)[0]; + expect(after.created_at).toBe("2026-01-01T00:00:00.000Z"); + expect(after.updated_at).toBe("2026-01-02T00:00:00.000Z"); + }); + + it("returns the same array when nothing changes", async () => { + const { + upsertSession, + applyImportedSessionTimestamps, + sessionsAtom, + store, + } = await loadModule(); + upsertSession( + makeSession({ + session_id: "imported-1", + ...SOURCE_TIMES, + importedFrom: IMPORTED_FROM, + }) + ); + const before = store.get(sessionsAtom); + + applyImportedSessionTimestamps("imported-1", SOURCE_TIMES); + + expect(store.get(sessionsAtom)).toBe(before); + }); +}); + describe("updateSessionStatus", () => { it("flips status without touching updated_at", async () => { const { upsertSession, updateSessionStatus, sessionsAtom, store } = diff --git a/src/store/session/sessionAtom/mutations.ts b/src/store/session/sessionAtom/mutations.ts index b992f6fd4..4cda96054 100644 --- a/src/store/session/sessionAtom/mutations.ts +++ b/src/store/session/sessionAtom/mutations.ts @@ -18,6 +18,10 @@ * reconcile-driven write; it represents activity the user just * performed, so it's the correct signal for sidebar / Kanban * "recent activity" ordering. + * 4. `applyImportedSessionTimestamps()` — an imported collaboration + * replay mirrors a TEAMMATE's session, so its timestamps are the + * owner's and arrive on the cloud listing row. No local read or + * list refresh can supply them. * * On the *update* path of `upsertSession()` we deliberately preserve * the prior record's timestamps and ignore whatever the caller spread @@ -121,6 +125,51 @@ export const markSessionActive = (sessionId: string) => { ); }; +/** + * Adopt the SOURCE's activity timestamps on an imported replay copy. + * + * The only mutation that writes someone else's clock, and the reason it + * has to exist: an imported collaboration copy is a read-only mirror of a + * teammate's session, so its `created_at` / `updated_at` describe the + * OWNER's work and reach this device on the cloud listing row + * (`lastActivityAt`) — no `loadSessions()` refresh can correct them. + * `upsertSession()`'s pinning is precisely what this bypasses; without it + * the copy keeps the moment the viewer first clicked the card, which made + * every opened cloud card read "Now" in Kanban and jump to the top of + * List/Diary. + * + * No-op unless the row is in the store AND carries `importedFrom`: the + * pinning stays absolute for locally-owned sessions. + */ +export const applyImportedSessionTimestamps = ( + sessionId: string, + timestamps: { + created_at: string; + updated_at: string; + completed_at: string; + } +) => { + const store = getStore(); + store.set(sessionsAtom, (prev) => { + let changed = false; + const next = prev.map((session) => { + if (session.session_id !== sessionId || !session.importedFrom) { + return session; + } + if ( + session.created_at === timestamps.created_at && + session.updated_at === timestamps.updated_at && + session.completed_at === timestamps.completed_at + ) { + return session; + } + changed = true; + return { ...session, ...timestamps }; + }); + return changed ? next : prev; + }); +}; + /** * Remove a session from the store. */