Skip to content
Merged
4 changes: 2 additions & 2 deletions packages/junior-evals/evals/agent/slack-user-status.eval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ describeEval("Slack User Status", slackEvals, (it) => {

expect(toolCalls(result.session)).toEqual([
expect.objectContaining({
name: "slackUserLookup",
arguments: { mode: "user_id", value: "U0TEST" },
name: "userLookup",
arguments: { provider: "slack", query: "U0TEST" },
status: "ok",
}),
]);
Expand Down
1 change: 1 addition & 0 deletions packages/junior-github/src/credential-support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,7 @@ export async function resolveUserAccount(
const url =
typeof account.html_url === "string" ? account.html_url : undefined;
return {
handle: login.trim(),
id: String(id),
label: login.trim(),
...(url ? { url } : {}),
Expand Down
1 change: 1 addition & 0 deletions packages/junior-github/tests/github-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2090,6 +2090,7 @@ Conversation: \`local:test:old-conversation\`
});

expect(account).toEqual({
handle: "actor",
id: "12345",
label: "actor",
url: "https://github.com/actor",
Expand Down
2 changes: 2 additions & 0 deletions packages/junior-plugin-api/src/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export const pluginAuthorizationSchema = z
/** Runtime schema for a provider account attached to stored OAuth tokens. */
export const pluginProviderAccountSchema = z
.object({
displayName: nonBlankStringSchema.optional(),
handle: nonBlankStringSchema.optional(),
id: nonBlankStringSchema,
label: nonBlankStringSchema.optional(),
url: nonBlankStringSchema.optional(),
Expand Down
1 change: 1 addition & 0 deletions packages/junior/src/chat/agent/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ export async function wireAgentTools(
conversationId: args.conversationId,
sessionId: args.turnId,
actorId: credentialUserId,
actor: args.currentActor,
channelId: slackChannelId,
destination: args.routing.destination,
source: runSource,
Expand Down
14 changes: 14 additions & 0 deletions packages/junior/src/chat/credentials/unlink-provider.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,29 @@
import { getSqlExecutor } from "@/chat/db";
import { deleteProviderIdentityForSlackUser } from "@/chat/identities/sql";
import type { UserTokenStore } from "@/chat/credentials/user-token-store";
import {
deleteMcpAuthSessionsForUserProvider,
deleteMcpServerSessionId,
deleteMcpStoredOAuthCredentials,
} from "@/chat/mcp/auth-store";

/** Remove one provider connection and its exact stored account identity. */
export async function unlinkProvider(
userId: string,
provider: string,
userTokenStore: UserTokenStore,
slackTeamId?: string,
): Promise<void> {
const tokens = await userTokenStore.get(userId, provider);
if (tokens?.account && slackTeamId) {
await deleteProviderIdentityForSlackUser(
getSqlExecutor(),
slackTeamId,
userId,
provider,
tokens.account.id,
);
Comment thread
cursor[bot] marked this conversation as resolved.
}
await Promise.all([
userTokenStore.delete(userId, provider),
deleteMcpStoredOAuthCredentials(userId, provider),
Expand Down
98 changes: 92 additions & 6 deletions packages/junior/src/chat/identities/sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ async function existingIdentity(
return rows[0];
}

/** Persist one provider identity observation and link verified emails to users. */
export async function upsertIdentity(
async function upsertIdentityRecord(
executor: JuniorSqlDatabase,
identity: IdentityUpsert,
nowMs: number = Date.now(),
linkedUserId: string | undefined,
nowMs: number,
): Promise<StoredIdentity> {
const emailNormalized = normalizeIdentityEmail(identity.email);
const email = emailNormalized
Expand Down Expand Up @@ -113,7 +113,21 @@ export async function upsertIdentity(
) {
throw new Error("Identity verified email conflicts with linked user");
}
const userId = existing?.userId ?? verifiedUserId;
if (
linkedUserId &&
verifiedUserId &&
linkedUserId !== verifiedUserId
) {
throw new Error("Linked identity conflicts with verified email user");
}
if (
existing?.userId &&
linkedUserId &&
existing.userId !== linkedUserId
) {
throw new Error("Identity conflicts with linked user");
}
const userId = existing?.userId ?? linkedUserId ?? verifiedUserId;
const rows = await executor
.db()
.insert(juniorIdentities)
Expand Down Expand Up @@ -143,26 +157,98 @@ export async function upsertIdentity(
set: {
kind: sql`excluded.kind`,
userId: sql`coalesce(${juniorIdentities.userId}, excluded.user_id)`,
displayName: sql`coalesce(${juniorIdentities.displayName}, excluded.display_name)`,
handle: sql`coalesce(${juniorIdentities.handle}, excluded.handle)`,
displayName: linkedUserId
? sql`coalesce(excluded.display_name, ${juniorIdentities.displayName})`
: sql`coalesce(${juniorIdentities.displayName}, excluded.display_name)`,
handle: linkedUserId
? sql`coalesce(excluded.handle, ${juniorIdentities.handle})`
: sql`coalesce(${juniorIdentities.handle}, excluded.handle)`,
email: sql`case when ${juniorIdentities.emailVerified} then coalesce(${juniorIdentities.email}, excluded.email) when excluded.email_verified then excluded.email else coalesce(${juniorIdentities.email}, excluded.email) end`,
emailNormalized: sql`case when ${juniorIdentities.emailVerified} then coalesce(${juniorIdentities.emailNormalized}, excluded.email_normalized) when excluded.email_verified then excluded.email_normalized else coalesce(${juniorIdentities.emailNormalized}, excluded.email_normalized) end`,
emailVerified: sql`${juniorIdentities.emailVerified} OR excluded.email_verified`,
avatarUrl: sql`coalesce(${juniorIdentities.avatarUrl}, excluded.avatar_url)`,
metadata: sql`coalesce(${juniorIdentities.metadata}, excluded.metadata_json)`,
updatedAt: sql`excluded.updated_at`,
},
...(linkedUserId
? {
setWhere: sql`${juniorIdentities.userId} IS NULL OR ${juniorIdentities.userId} = excluded.user_id`,
}
: {}),
})
.returning({
id: juniorIdentities.id,
userId: juniorIdentities.userId,
});
const row = rows[0];
if (!row) {
if (linkedUserId) {
throw new Error("Identity conflicts with linked user");
}
throw new Error("Identity upsert returned no row");
}
return {
id: row.id,
...(row.userId ? { userId: row.userId } : {}),
};
}

/** Persist one provider identity observation and link verified emails to users. */
export async function upsertIdentity(
executor: JuniorSqlDatabase,
identity: IdentityUpsert,
nowMs: number = Date.now(),
): Promise<StoredIdentity> {
return await upsertIdentityRecord(executor, identity, undefined, nowMs);
}

/**
* Persist one provider identity after a trusted account-linking flow.
* Callers must use provider-verified account data, such as GitHub's authenticated
* user response, never identity claims inferred from content such as Git commits.
*/
export async function upsertLinkedIdentity(
executor: JuniorSqlDatabase,
userId: string,
identity: IdentityUpsert,
nowMs: number = Date.now(),
): Promise<StoredIdentity> {
return await upsertIdentityRecord(executor, identity, userId, nowMs);
}

/** Remove one exact provider identity owned by a workspace Slack user. */
export async function deleteProviderIdentityForSlackUser(
executor: JuniorSqlDatabase,
slackTeamId: string,
slackUserId: string,
provider: string,
providerSubjectId: string,
): Promise<void> {
const slackRows = await executor
.db()
.select({ userId: juniorIdentities.userId })
.from(juniorIdentities)
.where(
and(
eq(juniorIdentities.kind, "user"),
eq(juniorIdentities.provider, "slack"),
eq(juniorIdentities.providerTenantId, slackTeamId),
eq(juniorIdentities.providerSubjectId, slackUserId),
),
)
.limit(1);
const userId = slackRows[0]?.userId;
if (!userId) return;

await executor
.db()
.delete(juniorIdentities)
.where(
and(
eq(juniorIdentities.userId, userId),
eq(juniorIdentities.provider, provider),
eq(juniorIdentities.providerTenantId, ""),
eq(juniorIdentities.providerSubjectId, providerSubjectId),
),
);
}
12 changes: 10 additions & 2 deletions packages/junior/src/chat/ingress/slack-webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ import {
} from "@/chat/ingress/message-changed";
import { normalizeIncomingSlackThreadId } from "@/chat/ingress/message-router";
import { isExternalSlackUser } from "@/chat/ingress/workspace-membership";
import { runWithWorkspaceTeamId } from "@/chat/slack/workspace-context";
import {
getWorkspaceTeamId,
runWithWorkspaceTeamId,
} from "@/chat/slack/workspace-context";
import { parseSlackThreadId } from "@/chat/slack/context";
import { getStateAdapter } from "@/chat/state/adapter";
import { handleSlashCommand } from "@/chat/ingress/slash-command";
Expand Down Expand Up @@ -593,7 +596,12 @@ async function handleInteractivePayload(args: {
{ userId: userId },
async () => {
try {
await unlinkProvider(userId, provider, args.userTokenStore);
await unlinkProvider(
userId,
provider,
args.userTokenStore,
getWorkspaceTeamId(),
);
} catch (error) {
logException(error, "app_home.disconnect_unlink.failed", {
"app.credential.provider": provider,
Expand Down
13 changes: 10 additions & 3 deletions packages/junior/src/chat/ingress/slash-command.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import type { SlashCommandEvent } from "chat";
import { createUserTokenStore } from "@/chat/capabilities/factory";
import { unlinkProvider } from "@/chat/credentials/unlink-provider";
import { formatProviderLabel, startOAuthFlow } from "@/chat/oauth-flow";
import { pluginCatalogRuntime } from "@/chat/plugins/catalog-runtime";
import { logInfo } from "@/chat/logging";
import { getChatConfig } from "@/chat/config";
import { parseActorUserId } from "@/chat/actor";
import { createActor, parseActorUserId } from "@/chat/actor";

async function postEphemeral(
event: SlashCommandEvent,
Expand Down Expand Up @@ -43,9 +44,14 @@ async function handleLink(
return;
}

const raw = event.raw as { channel_id?: string };
const raw = event.raw as { channel_id?: string; team_id?: string };
const actor = createActor(
{ platform: "slack", teamId: raw.team_id, userId: actorId },
{ platform: "slack", teamId: raw.team_id, userId: actorId },
);
const result = await startOAuthFlow(provider, {
actorId,
...(actor ? { actor } : {}),
channelId: raw.channel_id,
});

Expand Down Expand Up @@ -86,7 +92,8 @@ async function handleUnlink(
}

const tokenStore = createUserTokenStore();
await tokenStore.delete(actorId, provider);
const teamId = (event.raw as { team_id?: string }).team_id;
await unlinkProvider(actorId, provider, tokenStore, teamId);

logInfo("slash_command.credential.unlinked", {
"app.credential.provider": provider,
Expand Down
13 changes: 13 additions & 0 deletions packages/junior/src/chat/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1644,6 +1644,19 @@ export function logException(
return log.exception(eventName, normalizedError, attributes);
}

/** Run an optional side effect without changing the successful primary outcome. */
export async function runBestEffort(
operation: () => Promise<void>,
eventName: string,
attributes: Record<string, unknown> = {},
): Promise<void> {
try {
await operation();
} catch (error) {
logException(error, eventName, attributes);
}
}

/** Add context to the current operation and Sentry scope. */
export function setTags(context: LogContext = {}): void {
updateLogContext(context, contextToAttributes(context));
Expand Down
11 changes: 11 additions & 0 deletions packages/junior/src/chat/oauth-flow.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { randomBytes } from "node:crypto";
import {
actorSchema,
sourceSchema,
type Actor,
type Destination,
type Source,
} from "@sentry/junior-plugin-api";
Expand Down Expand Up @@ -29,6 +31,7 @@ type PrivateDeliveryResult = "in_context" | "fallback_dm" | false;
export type OAuthStatePayload = {
userId: string;
provider: string;
actor?: Actor;
channelId?: string;
destination?: Destination;
source?: Source;
Expand All @@ -40,6 +43,7 @@ export type OAuthStatePayload = {

type OAuthFlowInput = {
actorId: string;
actor?: Actor;
channelId?: string;
destination?: Destination;
source?: Source;
Expand Down Expand Up @@ -67,6 +71,11 @@ export function parseOAuthStatePayload(
if (typeof value.userId !== "string" || typeof value.provider !== "string") {
return undefined;
}
const actor =
value.actor === undefined ? undefined : actorSchema.safeParse(value.actor);
if (value.actor !== undefined && (!actor || !actor.success)) {
return undefined;
}
const destination = parseDestination(value.destination);
if (value.destination !== undefined && !destination) {
return undefined;
Expand All @@ -81,6 +90,7 @@ export function parseOAuthStatePayload(
return {
userId: value.userId,
provider: value.provider,
...(actor?.success ? { actor: actor.data } : {}),
...(optionalString(value.channelId)
? { channelId: optionalString(value.channelId) }
: {}),
Expand Down Expand Up @@ -257,6 +267,7 @@ export async function startOAuthFlow(
{
userId: input.actorId,
provider,
...(input.actor ? { actor: input.actor } : {}),
...(input.channelId ? { channelId: input.channelId } : {}),
...(input.destination ? { destination: input.destination } : {}),
...(input.source ? { source: input.source } : {}),
Expand Down
7 changes: 7 additions & 0 deletions packages/junior/src/chat/plugins/credential-hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,13 @@ export interface IssueCredentialInput {
userTokenStore: UserTokenStore;
}

/** Return providers that can verify an OAuth account for identity storage. */
export function getOAuthAccountProviders(): string[] {
return getPlugins()
.filter((plugin) => Boolean(plugin.hooks?.resolveOAuthAccount))
.map((plugin) => plugin.manifest.name);
}

/** Ask a plugin which provider account belongs to an OAuth token. */
export async function resolvePluginOAuthAccount(input: {
provider: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* signal emitted by the egress proxy — never inferred from bash command text,
* stdout patterns, or exit codes.
*/
import type { Destination, Source } from "@sentry/junior-plugin-api";
import type { Actor, Destination, Source } from "@sentry/junior-plugin-api";
import type { UserTokenStore } from "@/chat/credentials/user-token-store";
import { formatProviderLabel, startOAuthFlow } from "@/chat/oauth-flow";
import {
Expand Down Expand Up @@ -56,6 +56,7 @@ export interface PluginAuthOrchestrationInput {
conversationId?: string;
sessionId?: string;
actorId?: string;
actor?: Actor;
channelId?: string;
destination?: Destination;
source?: Source;
Expand Down Expand Up @@ -166,6 +167,7 @@ export function createPluginAuthOrchestration(
if (!reusingPendingLink) {
const oauthResult = await startOAuthFlow(provider, {
actorId: input.actorId,
...(input.actor ? { actor: input.actor } : {}),
channelId: input.channelId,
destination: input.destination,
source: input.source,
Expand Down
Loading
Loading