mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
perf(slack): avoid unused catalog revision scans (#16630)
Slack catalog discovery now checks live task, user, endpoint and credentials without calculating an unused compatibility revision. Full authority callers retain the unchanged revision and search-token path. Validation: 44 affected cases, full repository typecheck/build, current-head full CI/E2E and security checks, Greptile Base 5/5 with no unresolved findings. Identical production catalog output with revision statements reduced from two to zero in real-PostgreSQL comparisons. Hosted latency acceptance remains pending. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
32ee2e9a6a
commit
669d329f25
8 files changed
+187
-23
No files matched your search
@@ -924,6 +924,15 @@ clickable link to each task the agent creates. Agents use the public task URL
|
||||
from Paperclip tools or task context and report when a link is unavailable.
|
||||
Existing conversations keep their original communication guidance snapshot.
|
||||
|
||||
Slack catalog discovery uses `resolveSlackTaskAuthorization` to recheck the live
|
||||
run, task, agent, linked user, endpoint, and current bot credential. Discovery
|
||||
does not consume a compatibility revision, so it avoids hashing company policy
|
||||
and profile rows. Callers that need a revision continue to use
|
||||
`resolveSlackTaskAuthority`; connector skill snapshots and governed operations
|
||||
retain their existing fingerprint. Both paths share the same authorization
|
||||
implementation and perform fresh checks on every call. The real PostgreSQL
|
||||
controls are in `server/src/__tests__/slack-tool-preparation.test.ts`.
|
||||
|
||||
### Chat activity pagination and callback diagnostics
|
||||
|
||||
The connection Activity tab loads 25 records per page. `GET /api/chat-endpoints/:id/activity?limit=25`
|
||||
|
||||
@@ -138,6 +138,15 @@ the current bot token without resolving unrelated webhook credentials. See the
|
||||
and [batch failure controls](../../server/src/services/connectors/slack-endpoint-preparation.test.ts).
|
||||
These local controls do not establish live Slack latency or provider access.
|
||||
|
||||
Catalog authorization projection (2026-10-17): tool discovery uses the same live
|
||||
service-owned task, company, agent, requester, endpoint and credential checks as
|
||||
full authority resolution. It omits the revision digest and search-action token,
|
||||
which catalog descriptors do not consume. Connector assignments and governed
|
||||
operations retain full revision resolution; invocation policy is still checked
|
||||
by the gateway. Registry replacement and missing-service controls are in
|
||||
[authority registration tests](../../server/src/services/connectors/slack-authority.test.ts).
|
||||
This is a source and local-database contract; hosted latency needs live evidence.
|
||||
|
||||
New conversations resolve typed project/workspace defaults from their endpoint
|
||||
and destination under the task-creation transaction. Defaults do not authorize
|
||||
private sources: admission checks the linked user's authority and assigned agent.
|
||||
|
||||
@@ -68,6 +68,13 @@ and confirm that the next preparation excludes it. The
|
||||
uses real PostgreSQL with one connection and controlled credential resolution;
|
||||
it does not prove live provider latency or an end-to-end response target.
|
||||
|
||||
Catalog discovery rechecks live authority and current credentials without
|
||||
computing the unused compatibility fingerprint. Assignment snapshots and
|
||||
governed calls retain full revision resolution. The same regression verifies
|
||||
identical catalog membership, absence of digest SQL during discovery, and
|
||||
revocation on the next preparation. A faster component is not proof of a faster
|
||||
Slack reaction or final reply; measure those separately on the deployed revision.
|
||||
|
||||
### `endpoint-management`
|
||||
|
||||
Automated: [channel integration](../../server/src/__tests__/chat-channels.integration.test.ts) covers shared endpoint/delivery behavior; live credential repair and provider ownership remain manual checks.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { drizzle } from "drizzle-orm/postgres-js";
|
||||
import {
|
||||
agents, chatEndpoints, chatExternalPrincipals, chatIdentityLinks, companies,
|
||||
companyMemberships, createDb, heartbeatRuns, issues, toolApplications, toolConnections,
|
||||
@@ -79,6 +80,34 @@ describeDatabase("Slack tool preparation", () => {
|
||||
[...new Set((await slackToolsForSession(db, binding)).map(tool => String(tool.providerMetadata?.endpointId)))],
|
||||
};
|
||||
|
||||
it("does not compute an unused revision when preparing the session catalog", async () => {
|
||||
const f = await fixture();
|
||||
const queries: string[] = [];
|
||||
const measured = drizzle(db.$client, { logger: { logQuery: query => queries.push(query) } }) as typeof db;
|
||||
const registration = createSlackTaskAuthorityRegistration(measured, {
|
||||
resolveCredentials: createChatCredentials(measured, { secrets: { resolveSecretValue: f.resolveSecretValue } } as unknown as Parameters<typeof createChatCredentials>[1]).resolveCredentials,
|
||||
authorizeInboundWakeup: async () => { throw new Error("This is an ordinary task"); },
|
||||
});
|
||||
try {
|
||||
const tools = await slackToolsForSession(measured, f.binding);
|
||||
expect([...new Set(tools.map(tool => tool.providerMetadata?.endpointId))].sort()).toEqual(f.endpoints.map(endpoint => endpoint.id).sort());
|
||||
expect(queries.filter(query => query.includes("row_to_json(revision_row)"))).toHaveLength(0);
|
||||
queries.length = 0;
|
||||
const authority = await resolveSlackTaskAuthority(measured, { ...f.binding, endpointId: f.endpoints[1].id });
|
||||
expect(authority.revision).toMatch(/^[0-9a-f]{64}$/);
|
||||
expect(queries.filter(query => query.includes("row_to_json(revision_row)"))).toHaveLength(1);
|
||||
await db.update(chatIdentityLinks).set({ status: "revoked" }).where(eq(chatIdentityLinks.id, f.links[0].id));
|
||||
queries.length = 0;
|
||||
f.resolveSecretValue.mockClear();
|
||||
expect([...new Set((await slackToolsForSession(measured, f.binding)).map(tool => tool.providerMetadata?.endpointId))]).toEqual([f.endpoints[1].id]);
|
||||
expect(f.resolveSecretValue).toHaveBeenCalledTimes(1);
|
||||
expect(queries.filter(query => query.includes("row_to_json(revision_row)"))).toHaveLength(0);
|
||||
} finally {
|
||||
registration.unregisterSlackTaskAuthority();
|
||||
f.unregisterSlackTaskAuthority();
|
||||
}
|
||||
});
|
||||
|
||||
it.each(Object.entries(paths))("overlaps two independent endpoint checks for %s", async (_name, prepare) => {
|
||||
const f = await fixture();
|
||||
const gate = deferred();
|
||||
@@ -132,6 +161,22 @@ describeDatabase("Slack tool preparation", () => {
|
||||
} finally { f.unregisterSlackTaskAuthority(); }
|
||||
});
|
||||
|
||||
it.each(Object.entries(paths))("rechecks bot credential availability for %s", async (_name, prepare) => {
|
||||
const f = await fixture();
|
||||
try {
|
||||
expect((await prepare(f.binding)).sort()).toEqual(f.endpoints.map(endpoint => endpoint.id).sort());
|
||||
const error = new Error("Bot credential is revoked");
|
||||
f.resolveSecretValue.mockClear();
|
||||
f.resolveSecretValue.mockRejectedValue(error);
|
||||
await expect(prepare(f.binding)).rejects.toBe(error);
|
||||
expect(f.resolveSecretValue).toHaveBeenCalledTimes(2);
|
||||
f.resolveSecretValue.mockClear();
|
||||
f.resolveSecretValue.mockResolvedValue("");
|
||||
expect(await prepare(f.binding)).toEqual([]);
|
||||
expect(f.resolveSecretValue).toHaveBeenCalledTimes(2);
|
||||
} finally { f.unregisterSlackTaskAuthority(); }
|
||||
});
|
||||
|
||||
it.each(Object.entries(paths))("rechecks identity revocation for %s without touching another company", async (_name, prepare) => {
|
||||
const f = await fixture();
|
||||
const foreign = await fixture();
|
||||
|
||||
@@ -5,7 +5,7 @@ import { slackSearchActionToken } from "../../connectors/slack-search-context.js
|
||||
import { slackAuthorizationRevision } from "../../connectors/slack-revision.js";
|
||||
|
||||
import { instanceSettingsService } from "../../instance-settings.js";
|
||||
import { registerSlackTaskAuthority, slackRunOrigin } from "../../connectors/slack-authority.js";
|
||||
import { registerSlackTaskAuthority, slackRunOrigin, type SlackTaskBinding } from "../../connectors/slack-authority.js";
|
||||
import { measureRunPhase } from "../../execution-preparation-timing.js";
|
||||
import { measureRunTransaction } from "../../execution-transaction-timing.js";
|
||||
|
||||
@@ -53,7 +53,7 @@ export function createSlackTaskAuthorityRegistration(db: Db, options: SlackTaskA
|
||||
const { authorizeInboundWakeup, resolveCredentials } = options;
|
||||
|
||||
|
||||
const unregisterSlackTaskAuthority = registerSlackTaskAuthority(db, async (binding) => {
|
||||
const authorize = async (binding: SlackTaskBinding) => {
|
||||
if (!(await measureRunPhase("connectors.slack.settings", () => instanceSettingsService(db).getExperimental())).enableChatConnectors)
|
||||
throw forbidden("Chat connectors are disabled");
|
||||
const identity = await measureRunPhase("connectors.slack.identity", () => slackRunOrigin(db, binding));
|
||||
@@ -140,19 +140,29 @@ export function createSlackTaskAuthorityRegistration(db: Db, options: SlackTaskA
|
||||
const credentials = await measureRunPhase("connectors.slack.credentials", () => resolveCredentials(resolved.endpoint, db, ["botToken"]));
|
||||
if (!credentials.botToken) throw forbidden("Slack bot credential is unavailable");
|
||||
return {
|
||||
endpoint: resolved.endpoint, issueId: binding.issueId, conversation: resolved.conversation,
|
||||
principalId: resolved.principalId, slackUserId: resolved.slackUserId,
|
||||
userId: run.responsibleUserId, deliveryId: resolved.delivery?.id ?? null,
|
||||
identityContextId: identity.context?.id ?? null,
|
||||
workMode: resolved.issue.workMode,
|
||||
revision: createHash("sha256").update(JSON.stringify([
|
||||
resolved.endpoint.status, resolved.endpoint.allowDirectMessages, resolved.conversation?.sessionGeneration ?? null,
|
||||
run.responsibleUserId, credentials.botToken,
|
||||
await measureRunPhase("connectors.slack.revision", () => slackAuthorizationRevision(db, binding.companyId, resolved.endpoint.id, resolved.endpoint.connectionId, binding.agentId, run.responsibleUserId!)),
|
||||
])).digest("hex"),
|
||||
botToken: credentials.botToken,
|
||||
searchActionToken: resolved.delivery ? slackSearchActionToken(db, resolved.endpoint.id, resolved.endpoint.providerAccountId!, resolved.slackUserId, String((resolved.delivery.normalizedEvent.message as Record<string, unknown> | undefined)?.providerMessageId ?? "")) : null,
|
||||
authority: {
|
||||
endpoint: resolved.endpoint, issueId: binding.issueId, conversation: resolved.conversation,
|
||||
principalId: resolved.principalId, slackUserId: resolved.slackUserId,
|
||||
userId: run.responsibleUserId, deliveryId: resolved.delivery?.id ?? null,
|
||||
identityContextId: identity.context?.id ?? null,
|
||||
workMode: resolved.issue.workMode,
|
||||
botToken: credentials.botToken,
|
||||
},
|
||||
searchActionToken: () => resolved.delivery ? slackSearchActionToken(db, resolved.endpoint.id, resolved.endpoint.providerAccountId!, resolved.slackUserId, String((resolved.delivery.normalizedEvent.message as Record<string, unknown> | undefined)?.providerMessageId ?? "")) : null,
|
||||
};
|
||||
});
|
||||
};
|
||||
const unregisterSlackTaskAuthority = registerSlackTaskAuthority(db, async (binding) => {
|
||||
const resolved = await authorize(binding);
|
||||
const authority = resolved.authority;
|
||||
return {
|
||||
...authority,
|
||||
revision: createHash("sha256").update(JSON.stringify([
|
||||
authority.endpoint.status, authority.endpoint.allowDirectMessages, authority.conversation?.sessionGeneration ?? null,
|
||||
authority.userId, authority.botToken,
|
||||
await measureRunPhase("connectors.slack.revision", () => slackAuthorizationRevision(db, binding.companyId, authority.endpoint.id, authority.endpoint.connectionId, binding.agentId, authority.userId)),
|
||||
])).digest("hex"),
|
||||
searchActionToken: resolved.searchActionToken(),
|
||||
};
|
||||
}, async binding => (await authorize(binding)).authority);
|
||||
return { unregisterSlackTaskAuthority };
|
||||
}
|
||||
@@ -0,0 +1,68 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
registerSlackTaskAuthority,
|
||||
resolveSlackTaskAuthorization,
|
||||
resolveSlackTaskAuthority,
|
||||
type SlackTaskAuthority,
|
||||
type SlackTaskBinding,
|
||||
} from "./slack-authority.js";
|
||||
|
||||
const binding: SlackTaskBinding = {
|
||||
companyId: "company",
|
||||
agentId: "agent",
|
||||
issueId: "issue",
|
||||
runId: "run",
|
||||
};
|
||||
|
||||
describe("Slack authority registration", () => {
|
||||
it("requires a live registration for authorization and revision resolution", async () => {
|
||||
const db = {} as Db;
|
||||
await expect(resolveSlackTaskAuthorization(db, binding)).rejects.toMatchObject({ status: 403 });
|
||||
await expect(resolveSlackTaskAuthority(db, binding)).rejects.toMatchObject({ status: 403 });
|
||||
});
|
||||
|
||||
it("retains existing resolver compatibility without caching its result", async () => {
|
||||
const db = {} as Db;
|
||||
const first = { revision: "first" } as SlackTaskAuthority;
|
||||
const second = { revision: "second" } as SlackTaskAuthority;
|
||||
const resolve = vi.fn().mockResolvedValueOnce(first).mockResolvedValueOnce(second).mockResolvedValueOnce(second);
|
||||
const unregister = registerSlackTaskAuthority(db, resolve);
|
||||
try {
|
||||
expect(await resolveSlackTaskAuthorization(db, binding)).toBe(first);
|
||||
expect(await resolveSlackTaskAuthorization(db, binding)).toBe(second);
|
||||
expect(await resolveSlackTaskAuthority(db, binding)).toBe(second);
|
||||
expect(resolve.mock.calls).toEqual([[binding], [binding], [binding]]);
|
||||
expect(resolve.mock.contexts).toEqual([undefined, undefined, undefined]);
|
||||
} finally { unregister(); }
|
||||
});
|
||||
|
||||
it("uses live authorization separately from the revision resolver", async () => {
|
||||
const db = {} as Db;
|
||||
const full = { revision: "current" } as SlackTaskAuthority;
|
||||
const resolve = vi.fn(async () => full);
|
||||
const authorize = vi.fn(async () => full);
|
||||
const unregister = registerSlackTaskAuthority(db, resolve, authorize);
|
||||
try {
|
||||
expect(await resolveSlackTaskAuthorization(db, binding)).toBe(full);
|
||||
expect(authorize).toHaveBeenCalledWith(binding);
|
||||
expect(resolve).not.toHaveBeenCalled();
|
||||
expect(await resolveSlackTaskAuthority(db, binding)).toBe(full);
|
||||
expect(resolve).toHaveBeenCalledWith(binding);
|
||||
} finally { unregister(); }
|
||||
});
|
||||
|
||||
it("does not unregister a replacement service's authorization", async () => {
|
||||
const db = {} as Db;
|
||||
const old = { revision: "old" } as SlackTaskAuthority;
|
||||
const current = { revision: "current" } as SlackTaskAuthority;
|
||||
const unregisterOld = registerSlackTaskAuthority(db, async () => old, async () => old);
|
||||
const unregisterCurrent = registerSlackTaskAuthority(db, async () => current, async () => current);
|
||||
try {
|
||||
unregisterOld();
|
||||
expect(await resolveSlackTaskAuthorization(db, binding)).toBe(current);
|
||||
expect(await resolveSlackTaskAuthority(db, binding)).toBe(current);
|
||||
} finally { unregisterCurrent(); }
|
||||
await expect(resolveSlackTaskAuthorization(db, binding)).rejects.toMatchObject({ status: 403 });
|
||||
});
|
||||
});
|
||||
@@ -37,29 +37,44 @@ export interface SlackTaskAuthority {
|
||||
botToken: string;
|
||||
searchActionToken?: string | null;
|
||||
}
|
||||
export type SlackTaskAuthorization = Omit<SlackTaskAuthority, "revision" | "searchActionToken">;
|
||||
// Only the live chat service can attest to admission. JSON snapshots, model
|
||||
// arguments and credential possession cannot install an authority callback.
|
||||
const authorities = new WeakMap<
|
||||
Db,
|
||||
(binding: SlackTaskBinding) => Promise<SlackTaskAuthority>
|
||||
{
|
||||
resolve: (binding: SlackTaskBinding) => Promise<SlackTaskAuthority>;
|
||||
authorize?: (binding: SlackTaskBinding) => Promise<SlackTaskAuthorization>;
|
||||
}
|
||||
>();
|
||||
export function registerSlackTaskAuthority(
|
||||
db: Db,
|
||||
resolve: (binding: SlackTaskBinding) => Promise<SlackTaskAuthority>,
|
||||
authorize?: (binding: SlackTaskBinding) => Promise<SlackTaskAuthorization>,
|
||||
) {
|
||||
authorities.set(db, resolve);
|
||||
const registration = { resolve, authorize };
|
||||
authorities.set(db, registration);
|
||||
return () => {
|
||||
if (authorities.get(db) === resolve) authorities.delete(db);
|
||||
if (authorities.get(db) === registration) authorities.delete(db);
|
||||
};
|
||||
}
|
||||
export async function resolveSlackTaskAuthority(
|
||||
db: Db,
|
||||
binding: SlackTaskBinding,
|
||||
) {
|
||||
const resolve = authorities.get(db);
|
||||
if (!resolve) throw forbidden("Slack task authority is unavailable");
|
||||
const registration = authorities.get(db);
|
||||
if (!registration) throw forbidden("Slack task authority is unavailable");
|
||||
const resolve = registration.resolve;
|
||||
return resolve(binding);
|
||||
}
|
||||
export async function resolveSlackTaskAuthorization(
|
||||
db: Db,
|
||||
binding: SlackTaskBinding,
|
||||
): Promise<SlackTaskAuthorization> {
|
||||
const registration = authorities.get(db);
|
||||
if (!registration) throw forbidden("Slack task authority is unavailable");
|
||||
return (registration.authorize ?? registration.resolve)(binding);
|
||||
}
|
||||
|
||||
/** Follow only immutable controller provenance, stopping at the latest human message.
|
||||
* A board-authored message must not inherit an earlier Slack user's credentials. */
|
||||
|
||||
@@ -18,8 +18,9 @@ import type {
|
||||
ToolGatewaySession,
|
||||
} from "../tool-gateway.js";
|
||||
import {
|
||||
resolveSlackTaskAuthority,
|
||||
resolveSlackTaskAuthorization,
|
||||
slackEndpointCandidates,
|
||||
type SlackTaskAuthorization,
|
||||
} from "./slack-authority.js";
|
||||
const catalogDefinitions = SLACK_TOOLS.map(tool => ({
|
||||
tool,
|
||||
@@ -208,9 +209,9 @@ async function slackToolsForEndpoint(
|
||||
endpointId: string,
|
||||
): Promise<ToolGatewayDescriptor[]> {
|
||||
if (!session.agentId || !session.runId || !session.issueId) return [];
|
||||
let authority;
|
||||
let authority: SlackTaskAuthorization;
|
||||
try {
|
||||
authority = await resolveSlackTaskAuthority(db, {
|
||||
authority = await resolveSlackTaskAuthorization(db, {
|
||||
endpointId,
|
||||
companyId: session.companyId,
|
||||
agentId: session.agentId,
|
||||
|
||||
Reference in new issue
Block a user