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
134 changes: 134 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 @@ -1713,4 +1713,138 @@ describe('cli-sessions-v2-router', () => {
]);
});
});

describe('list / search hide never-ingested placeholders', () => {
// Bare POST /api/session placeholders: title/status/cost NULL and platform
// still at the column default 'unknown'. Content may still exist in DO/R2;
// the four-column conjunction is only a list/search visibility predicate.
const placeholderId = 'ses_hide_placeholder_bare_0001';
const titledUnknownId = 'ses_hide_placeholder_titled_0001';
const statusUnknownId = 'ses_hide_placeholder_status_0001';
const costOnlyZeroId = 'ses_hide_placeholder_cost0_0001';
const normalCliId = 'ses_hide_placeholder_cli_0001';
const allSessionIds = [
placeholderId,
titledUnknownId,
statusUnknownId,
costOnlyZeroId,
normalCliId,
];

beforeEach(async () => {
const baseTime = Date.parse('2026-06-01T12:00:00.000Z');
await db.insert(cli_sessions_v2).values([
{
session_id: placeholderId,
kilo_user_id: regularUser.id,
// defaults: created_on_platform 'unknown', title/status/cost NULL
created_at: new Date(baseTime).toISOString(),
updated_at: new Date(baseTime).toISOString(),
},
{
session_id: titledUnknownId,
kilo_user_id: regularUser.id,
created_on_platform: 'unknown',
title: 'titled but still unknown platform',
created_at: new Date(baseTime + 1000).toISOString(),
updated_at: new Date(baseTime + 1000).toISOString(),
},
{
session_id: statusUnknownId,
kilo_user_id: regularUser.id,
created_on_platform: 'unknown',
status: 'running',
created_at: new Date(baseTime + 2000).toISOString(),
updated_at: new Date(baseTime + 2000).toISOString(),
},
{
session_id: costOnlyZeroId,
kilo_user_id: regularUser.id,
created_on_platform: 'unknown',
// Metrics emission can persist 0 (writer clamps with Math.max(0, …))
// while no metadata projection ever succeeded.
total_cost_microdollars: 0,
created_at: new Date(baseTime + 3000).toISOString(),
updated_at: new Date(baseTime + 3000).toISOString(),
},
{
session_id: normalCliId,
kilo_user_id: regularUser.id,
created_on_platform: 'cli',
title: 'normal cli session',
status: 'completed',
created_at: new Date(baseTime + 4000).toISOString(),
updated_at: new Date(baseTime + 4000).toISOString(),
},
]);
});

afterEach(async () => {
await db.delete(cli_sessions_v2).where(inArray(cli_sessions_v2.session_id, allSessionIds));
});

it('list omits bare placeholder rows', async () => {
const caller = await createCallerForUser(regularUser.id);
const result = await caller.cliSessionsV2.list({});
const ids = result.cliSessions.map(session => session.session_id);

expect(ids).not.toContain(placeholderId);
});

it('list returns a row with a title even when platform is still unknown', async () => {
const caller = await createCallerForUser(regularUser.id);
const result = await caller.cliSessionsV2.list({});
const ids = result.cliSessions.map(session => session.session_id);

expect(ids).toContain(titledUnknownId);
});

it('list returns a row with a status even when title is null and platform is unknown', async () => {
const caller = await createCallerForUser(regularUser.id);
const result = await caller.cliSessionsV2.list({});
const ids = result.cliSessions.map(session => session.session_id);

expect(ids).toContain(statusUnknownId);
});

it('list returns a row with only total_cost_microdollars set (including zero)', async () => {
const caller = await createCallerForUser(regularUser.id);
const result = await caller.cliSessionsV2.list({});
const ids = result.cliSessions.map(session => session.session_id);

expect(ids).toContain(costOnlyZeroId);
const costOnly = result.cliSessions.find(session => session.session_id === costOnlyZeroId);
expect(costOnly?.total_cost_microdollars).toBe(0);
});

it('list returns a normal cli session and keeps pagination stable with placeholders interleaved', async () => {
const caller = await createCallerForUser(regularUser.id);
// Fixtures are ordered by created_at; placeholders sit between visible
// rows. limit=2 over created_at should page only visible rows.
const page1 = await caller.cliSessionsV2.list({ limit: 2, orderBy: 'created_at' });
const page1Ids = page1.cliSessions.map(session => session.session_id);

expect(page1Ids).toEqual([normalCliId, costOnlyZeroId]);
expect(page1Ids).not.toContain(placeholderId);
expect(page1.nextCursor).not.toBeNull();

const page2 = await caller.cliSessionsV2.list({
limit: 2,
orderBy: 'created_at',
cursor: page1.nextCursor!,
});
const page2Ids = page2.cliSessions.map(session => session.session_id);

expect(page2Ids).toEqual([statusUnknownId, titledUnknownId]);
expect(page2Ids).not.toContain(placeholderId);
});

it('search by exact session_id does not return a bare placeholder', async () => {
const caller = await createCallerForUser(regularUser.id);
const result = await caller.cliSessionsV2.search({ search_string: placeholderId });

expect(result.results.map(session => session.session_id)).not.toContain(placeholderId);
expect(result.total).toBe(0);
});
});
});
37 changes: 37 additions & 0 deletions apps/web/src/routers/cli-sessions-v2-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,41 @@ async function addOrganizationCondition(
whereConditions.push(eq(cli_sessions_v2.organization_id, organizationId));
}

/**
* Hide never-ingested placeholder rows from list/search.
*
* POST /api/session creates bare placeholders (title/status/cost NULL,
* created_on_platform default 'unknown') before any user turn. Metadata and
* cost arrive later via ingest; if the client dies in that window the row
* stays permanently unwritten.
*
* All four columns unwritten proves only that no metadata projection ever
* succeeded and no metrics emission ever persisted a cost. It does not prove
* the row has no content — content lives in the DO and R2 and commits
* independently of the metadata projection.
*
* total_cost_microdollars is written by the alarm-driven metrics emission in
* SessionIngestDO.emitSessionMetrics (best-effort: stays NULL when the metric
* is non-finite or the UPDATE throws and is swallowed), not per flush — so
* presence proves the session reached a metrics emission and must be shown;
* absence proves nothing.
*
* Invariant: any row whose four list columns are all unwritten is hidden,
* regardless of whether the DO holds content. The predicate is the definition
* of what gets hidden; no Postgres-visible signal can do better on a
* paginated list query.
*/
function addHideUningestedPlaceholderCondition(whereConditions: SQL[]): void {
whereConditions.push(
sql`(
${isNotNull(cli_sessions_v2.title)}
OR ${isNotNull(cli_sessions_v2.status)}
OR ${cli_sessions_v2.created_on_platform} != 'unknown'
OR ${isNotNull(cli_sessions_v2.total_cost_microdollars)}
)`
);
}

function joinWithAnd(fragments: SQL[]): SQL {
return sql.join(fragments, sql` AND `);
}
Expand Down Expand Up @@ -506,6 +541,7 @@ export const cliSessionsV2Router = createTRPCRouter({
await addOrganizationCondition(whereConditions, ctx, organizationId);
addCreatedOnPlatformConditions(whereConditions, createdOnPlatform);
addGitUrlConditions(whereConditions, gitUrl);
addHideUningestedPlaceholderCondition(whereConditions);

if (cursor) {
whereConditions.push(lt(orderColumn, cursor));
Expand Down Expand Up @@ -586,6 +622,7 @@ export const cliSessionsV2Router = createTRPCRouter({
await addOrganizationCondition(whereConditions, ctx, organizationId);
addCreatedOnPlatformConditions(whereConditions, createdOnPlatform);
addGitUrlConditions(whereConditions, gitUrl);
addHideUningestedPlaceholderCondition(whereConditions);

if (!includeChildren) {
whereConditions.push(isNull(cli_sessions_v2.parent_session_id));
Expand Down
2 changes: 2 additions & 0 deletions packages/worker-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,5 @@ export type {
SecurityFindingAuditSnapshotSource,
SecurityFindingAuditWriterDb,
} from './security-finding-audit.js';

export { hasOrganizationAccess } from './organization-membership.js';
59 changes: 59 additions & 0 deletions packages/worker-utils/src/organization-membership.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { describe, expect, it, vi } from 'vitest';

import { hasOrganizationAccess } from './organization-membership.js';

type MembershipFixture = { kind: 'member' } | { kind: 'non-member' } | { kind: 'soft-deleted' };

function createMembershipDb(fixture: MembershipFixture) {
const rows =
fixture.kind === 'member'
? [{ id: 'mem_1' }]
: // non-member and soft-deleted org both yield no row: the join requires
// membership + organizations.deleted_at IS NULL.
[];

const limit = vi.fn(async () => rows);
const where = vi.fn(() => ({ limit }));
const innerJoin = vi.fn(() => ({ where }));
const from = vi.fn(() => ({ innerJoin }));
const select = vi.fn(() => ({ from }));

return { select, from, innerJoin, where, limit, rows };
}

describe('hasOrganizationAccess', () => {
it('returns true when the user has a direct membership in a live org', async () => {
const db = createMembershipDb({ kind: 'member' });

await expect(
hasOrganizationAccess(db as never, { kiloUserId: 'usr_1', organizationId: 'org_1' })
).resolves.toBe(true);

expect(db.select).toHaveBeenCalledOnce();
expect(db.from).toHaveBeenCalledOnce();
expect(db.innerJoin).toHaveBeenCalledOnce();
expect(db.where).toHaveBeenCalledOnce();
expect(db.limit).toHaveBeenCalledWith(1);
});

it('returns false when the user has no membership row', async () => {
const db = createMembershipDb({ kind: 'non-member' });

await expect(
hasOrganizationAccess(db as never, { kiloUserId: 'usr_1', organizationId: 'org_1' })
).resolves.toBe(false);
});

it('returns false when the user is a member of a soft-deleted org', async () => {
// Soft-deleted orgs are filtered by isNull(organizations.deleted_at) on the
// join, so the query returns no row — same as non-membership to the caller.
const db = createMembershipDb({ kind: 'soft-deleted' });

await expect(
hasOrganizationAccess(db as never, {
kiloUserId: 'usr_1',
organizationId: 'org_deleted',
})
).resolves.toBe(false);
});
});
46 changes: 46 additions & 0 deletions packages/worker-utils/src/organization-membership.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import type { WorkerDb } from '@kilocode/db/client';
import { organization_memberships, organizations } from '@kilocode/db/schema';
import { and, eq, isNull } from 'drizzle-orm';

type OrganizationMembershipDb = Pick<WorkerDb, 'select'>;

/**
* True when the user has a direct membership row for the organization and the
* organization is not soft-deleted. Mirrors worker session-access predicates in
* `cloud-agent-session-access.ts` as a standalone query (no session join).
*
* Deliberately excluded:
* - Parent-organization inherited roles — honoured only by the tRPC path
* (`apps/web/src/routers/organizations/utils.ts`), restricted to
* owner/billing_manager. No worker-side check considers them; adding
* inheritance here would make this the single most permissive worker check in
* the repo, in a security fix. The pre-existing gap is uniform across every
* worker path and is not this PR's to change.
* - `kilocode_users.is_admin` — no worker session-access path consults it, and
* session-ingest JWT auth deliberately discards every JWT claim except
* `kiloUserId`.
*/
export async function hasOrganizationAccess(
db: OrganizationMembershipDb,
params: { kiloUserId: string; organizationId: string }
): Promise<boolean> {
const rows = await db
.select({ id: organization_memberships.id })
.from(organization_memberships)
.innerJoin(
organizations,
and(
eq(organizations.id, organization_memberships.organization_id),
isNull(organizations.deleted_at)
)
)
.where(
and(
eq(organization_memberships.organization_id, params.organizationId),
eq(organization_memberships.kilo_user_id, params.kiloUserId)
)
)
.limit(1);

return rows[0] !== undefined;
}
Loading
Loading