Skip to content

Fix private REST inbox teammate lastSeenAt context #157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
seemayr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix private REST inbox teammate lastSeenAt context #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 45 additions & 1 deletion apps/api/src/rest/routers/conversation-auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ const safelyExtractRequestQueryMock = mock((async () => ({})) as (
...args: unknown[]
) => Promise<unknown>);
const validateResponseMock = mock(<T>(value: T) => value);
const resolvePrivateApiKeyActorUserMock = mock(
(async () => null) as (...args: unknown[]) => Promise<unknown>
);

const getVisitorMock = mock(
(async () => null) as (...args: unknown[]) => Promise<unknown>
Expand Down Expand Up @@ -55,6 +58,10 @@ mock.module("@api/utils/validate", () => ({
validateResponse: validateResponseMock,
}));

mock.module("@api/lib/private-api-key-actor", () => ({
resolvePrivateApiKeyActorUser: resolvePrivateApiKeyActorUserMock,
}));

mock.module("@api/db/queries", () => ({
getVisitor: getVisitorMock,
upsertVisitor: mock(async () => null),
Expand Down Expand Up @@ -317,15 +324,19 @@ describe("conversation auth and inbox routes", () => {
items: [createInboxItem()],
nextCursor: "cursor_2",
});
resolvePrivateApiKeyActorUserMock.mockResolvedValue(null);
mergeConversationMetadataMock.mockResolvedValue(createConversationRecord());
});

it("lists inbox conversations for private API keys", async () => {
resolvePrivateApiKeyActorUserMock.mockResolvedValue({
userId: "user-1",
});
safelyExtractRequestQueryMock.mockResolvedValue({
db: {},
apiKey: { keyType: APIKeyType.PRIVATE },
organization: { id: "org-1" },
website: { id: "site-1", organizationId: "org-1" },
website: { id: "site-1", organizationId: "org-1", teamId: "team-1" },
query: {
limit: 20,
cursor: null,
Expand All @@ -347,6 +358,39 @@ describe("conversation auth and inbox routes", () => {
expect(response.status).toBe(200);
expect(payload.nextCursor).toBe("cursor_2");
expect(payload.items[0]?.id).toBe(conversationId);
expect(listConversationsHeadersMock).toHaveBeenCalledWith(
{},
{
organizationId: "org-1",
websiteId: "site-1",
userId: "user-1",
limit: 20,
cursor: null,
}
);
});

it("keeps inbox lastSeenAt actorless when no teammate context is available", async () => {
resolvePrivateApiKeyActorUserMock.mockResolvedValue(null);
safelyExtractRequestQueryMock.mockResolvedValue({
db: {},
apiKey: { keyType: APIKeyType.PRIVATE },
organization: { id: "org-1" },
website: { id: "site-1", organizationId: "org-1", teamId: "team-1" },
query: {
limit: 20,
cursor: null,
},
});

const { conversationRouter } = await conversationRouterModulePromise;
const response = await conversationRouter.request(
new Request("http://localhost/inbox?limit=20", {
method: "GET",
})
);

expect(response.status).toBe(200);
expect(listConversationsHeadersMock).toHaveBeenCalledWith(
{},
{
Expand Down
11 changes: 10 additions & 1 deletion apps/api/src/rest/routers/conversation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2043,12 +2043,21 @@ conversationRouter.openapi(
return privateContext;
}

const actor = await requirePrivateConversationActor({
c,
db: extracted.db,
apiKey: privateContext.apiKey,
organizationId: privateContext.organization.id,
websiteTeamId: privateContext.website.teamId,
required: false,
});
Comment on lines +2046 to +2053
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 required: false doesn't prevent 403 for linked keys on teamless websites

resolvePrivateApiKeyActorUser only short-circuits to null when candidateUserId is falsy (unlinked key + no explicit actor header). When a key has linkedUserId set — or when an X-Actor-User-Id header is present — candidateUserId becomes truthy, and the next check if (!params.websiteTeamId) throws a 403 unconditionally, regardless of required. So any linked private key used against a website with teamId: null | undefined will now get a 403 from the inbox, where previously it always returned 200 with userId: null.

This contradicts the PR's stated goal of keeping actorless / no-team-context integrations working. The relevant path in resolvePrivateApiKeyActorUser (apps/api/src/lib/private-api-key-actor.ts lines 54-58):

if (!params.websiteTeamId) {
    throw new AuthValidationError(403, ...); // no required check here
}

A catch around the call in the inbox handler, or a separate guard, is needed to handle this gracefully.


const [planInfo, result] = await Promise.all([
getPlanForWebsite(privateContext.website),
listConversationsHeaders(extracted.db, {
organizationId: privateContext.organization.id,
websiteId: privateContext.website.id,
userId: null,
userId: actor?.userId ?? null,
limit: extracted.query.limit,
cursor: extracted.query.cursor ?? null,
}),
Comment on lines +2046 to 2063
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Actor resolution and plan fetch could be parallelized

requirePrivateConversationActor is awaited sequentially before the Promise.all, but actor resolution and getPlanForWebsite are fully independent of each other. Parallelizing them — e.g. const [actor, planInfo] = await Promise.all([requirePrivateConversationActor(...), getPlanForWebsite(...)]) — would reduce inbox latency on every request at the cost of zero correctness risk.

Expand Down