diff --git a/apps/web/src/routers/cli-sessions-v2-router.test.ts b/apps/web/src/routers/cli-sessions-v2-router.test.ts index 50e77912b9..0ce2d11141 100644 --- a/apps/web/src/routers/cli-sessions-v2-router.test.ts +++ b/apps/web/src/routers/cli-sessions-v2-router.test.ts @@ -857,6 +857,36 @@ describe('cli-sessions-v2-router', () => { expect(result.title).toBe('renamed by current member'); }); + it('rename always overwrites the title, whether it is still the creation placeholder or an existing (e.g. agent-generated) title', async () => { + const caller = await createCallerForUser(regularUser.id); + + // Session starts with title NULL (the creation placeholder) — rename must succeed. + const [beforeAnyTitle] = await db + .select({ title: cli_sessions_v2.title }) + .from(cli_sessions_v2) + .where(eq(cli_sessions_v2.session_id, organizationSessionId)); + expect(beforeAnyTitle?.title).toBeNull(); + + const firstRename = await caller.cliSessionsV2.rename({ + session_id: organizationSessionId, + title: 'agent-generated title', + }); + expect(firstRename.title).toBe('agent-generated title'); + + // A subsequent user rename must overwrite an already non-null (agent-generated) title too. + const secondRename = await caller.cliSessionsV2.rename({ + session_id: organizationSessionId, + title: 'user renamed title', + }); + expect(secondRename.title).toBe('user renamed title'); + + const [persisted] = await db + .select({ title: cli_sessions_v2.title }) + .from(cli_sessions_v2) + .where(eq(cli_sessions_v2.session_id, organizationSessionId)); + expect(persisted?.title).toBe('user renamed title'); + }); + it('rename rejects an organization session after its creator loses membership', async () => { const originalTitle = 'organization session title'; await db diff --git a/services/session-ingest/src/ingest/metadata.test.ts b/services/session-ingest/src/ingest/metadata.test.ts index cd8389d7e5..7879ec6b22 100644 --- a/services/session-ingest/src/ingest/metadata.test.ts +++ b/services/session-ingest/src/ingest/metadata.test.ts @@ -119,6 +119,8 @@ type ApplyMetadataDbOptions = { parentSessionId?: string | null; createsCycle?: boolean; scopeRootMissing?: boolean; + /** Title stored on the row before applyMetadataChanges runs. Defaults to the creation placeholder (NULL). */ + initialTitle?: string | null; }; /** @@ -148,6 +150,7 @@ function createApplyMetadataDb(options: ApplyMetadataDbOptions = {}) { function currentSessionState() { return { + title: options.initialTitle ?? null, status: options.initialStatus ?? 'idle', parentSessionId: options.parentSessionId ?? null, cloudAgentSessionScopeId: options.cloudAgentSessionScopeId ?? null, @@ -666,6 +669,65 @@ describe('applyMetadataChanges', () => { expect(notifyUserSessionEvent).not.toHaveBeenCalled(); }); + describe('agent-generated title vs. user rename race', () => { + it('applies the agent-generated title when the row still has the creation placeholder (NULL)', async () => { + const db = createApplyMetadataDb({ initialTitle: null }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + + await applyMetadataChanges(env, 'usr_1', 'ses_1', new Map([['title', 'Agent title']])); + + expect(db.updateSets).toEqual([expect.objectContaining({ title: 'Agent title' })]); + expect(notifyUserSessionEvent).toHaveBeenCalledWith( + env, + 'usr_1', + expect.objectContaining({ type: 'session.updated' }), + undefined + ); + }); + + it('skips the agent-generated title write when the user already renamed the session', async () => { + const db = createApplyMetadataDb({ initialTitle: 'User chosen title' }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + const warnSpy = vi.mocked(console.warn); + + await applyMetadataChanges(env, 'usr_1', 'ses_1', new Map([['title', 'Agent title']])); + + // Title write is dropped entirely; no update statement is issued for a title-only batch. + expect(db.applyUpdate).not.toHaveBeenCalled(); + expect(db.updateSets).toEqual([]); + expect(notifyUserSessionEvent).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalledWith( + 'Skipping agent-generated title write; title is no longer the placeholder', + expect.objectContaining({ kiloUserId: 'usr_1', sessionId: 'ses_1' }) + ); + }); + + it('still applies other metadata fields when the title write is skipped due to a prior user rename', async () => { + const db = createApplyMetadataDb({ initialTitle: 'User chosen title' }); + vi.mocked(getWorkerDb).mockReturnValue(db as never); + + await applyMetadataChanges( + env, + 'usr_1', + 'ses_1', + new Map([ + ['title', 'Agent title'], + ['platform', 'cli'], + ['status', 'busy'], + ]) + ); + + expect(db.updateSets).toEqual([ + expect.objectContaining({ + created_on_platform: 'cli', + status: 'busy', + }), + ]); + const written = db.updateSets[0] as Record; + expect(written).not.toHaveProperty('title'); + }); + }); + it('logs when a session scope root is missing during reparent', async () => { const db = createApplyMetadataDb({ cloudAgentSessionScopeId: 'cloud-agent-session-scope-1', diff --git a/services/session-ingest/src/ingest/metadata.ts b/services/session-ingest/src/ingest/metadata.ts index 6f7b49af58..9a24c85454 100644 --- a/services/session-ingest/src/ingest/metadata.ts +++ b/services/session-ingest/src/ingest/metadata.ts @@ -68,10 +68,14 @@ export async function applyMetadataChanges( /** True only when an organization_id write was actually applied (authorized claim or explicit null clear). */ let organizationIdWriteApplied = false; + /** True only when an agent-generated title write was actually applied (placeholder was still unset). */ + let titleWriteApplied = false; + const notification = await db.transaction(async tx => { const selectCurrentRow = () => tx .select({ + title: cli_sessions_v2.title, status: cli_sessions_v2.status, parentSessionId: cli_sessions_v2.parent_session_id, cloudAgentSessionId: cli_sessions_v2.cloud_agent_session_id, @@ -126,6 +130,23 @@ export async function applyMetadataChanges( return { changed: status !== previousStatus, previousStatus }; })(); + // Agent-generated titles arrive asynchronously and can race a user rename. A session's + // title starts out NULL (the placeholder set at row creation); only promote it from that + // placeholder here. Once the title is non-null — whether from a user rename or an earlier + // agent-generated write — leave it alone so a later user rename can never be clobbered by + // an in-flight ingest. + if (mergedChanges.has('title')) { + if (currentRow.title === null) { + titleWriteApplied = true; + } else { + console.warn('Skipping agent-generated title write; title is no longer the placeholder', { + kiloUserId, + sessionId, + }); + delete updates.title; + } + } + // Membership check only for non-null org claims; run on the same tx as the UPDATE. // Residual: SessionIngestDO.writeIngestMetaIfChanged records the claimed orgId in DO // SQLite and emits a change only when the value differs; after a refused write the DO @@ -281,9 +302,9 @@ export async function applyMetadataChanges( } } - // Refused org/parent claims must not emit phantom session.updated events. + // Refused org/parent/title claims must not emit phantom session.updated events. const changedNonStatus = - mergedChanges.has('title') || + titleWriteApplied || mergedChanges.has('platform') || organizationIdWriteApplied || mergedChanges.has('gitUrl') ||