Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions apps/web/src/routers/cli-sessions-v2-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 62 additions & 0 deletions services/session-ingest/src/ingest/metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

/**
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<string, unknown>;
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',
Expand Down
25 changes: 23 additions & 2 deletions services/session-ingest/src/ingest/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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') ||
Expand Down