Skip to content

Commit 0f27c4c

Browse files
kfiramarKfir
andauthored
fix: clarify Discord self-assignment failures (#26)
* Move Discord issue buttons onto host-side issue updates The reopen and assign buttons patched issues over unauthenticated HTTP, which failed against authenticated Paperclip deployments. This change resolves the issue's company through the host clients and performs the mutation with ctx.issues.update, while declaring the required issues.update capability in the manifest. Constraint: Authenticated Paperclip issue update routes reject anonymous PATCH requests Rejected: Keep HTTP PATCH and inject board credentials | fragile and duplicates host authorization rules Confidence: high Scope-risk: narrow Reversibility: clean Directive: Button handlers should use SDK host mutations for issue state changes; avoid raw Paperclip PATCH calls Tested: npm test; npm run typecheck Not-tested: Live assign/reopen button behavior for mapped and unmapped Discord users after merge * Clarify Discord self-assignment failures before users hit backend errors The live smoke test showed that the assign button now reaches Paperclip correctly, but the Discord UX still surfaces raw backend errors like `Assignee user not found` and `Issue can only have one assignee`. Pre-checking assignment state and translating the unmapped-user failure makes the Discord path understandable. Constraint: Discord users may not exist as Paperclip board users, so self-assignment cannot always succeed Rejected: Silently swallow assign failures | hides actionable identity/config problems Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Discord-side button feedback specific and actionable when backend policy blocks assignment Tested: Local typecheck; local build; button regression tests Not-tested: Live redeploy on the Paperclip instance --------- Co-authored-by: Kfir <suukpehoy@gmail.com>
1 parent 099b88e commit 0f27c4c

3 files changed

Lines changed: 106 additions & 48 deletions

File tree

src/commands.ts

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,19 +1286,8 @@ async function handleButtonClick(
12861286
const issueId = customId.replace("issue_reopen_", "");
12871287
ctx.logger.info("Reopen button clicked", { issueId, actor });
12881288
try {
1289-
const resp = await withRetry(async () => {
1290-
const r = await paperclipFetch(`${base}/api/issues/${issueId}`, {
1291-
method: "PATCH",
1292-
headers: { "Content-Type": "application/json" },
1293-
body: JSON.stringify({ status: "todo", comment: `Reopened by ${actor} via Discord` }),
1294-
});
1295-
throwOnRetryableStatus(r);
1296-
return r;
1297-
});
1298-
if (!resp.ok) {
1299-
const body = await resp.text().catch(() => "");
1300-
throw new Error(`API ${resp.status}: ${body}`);
1301-
}
1289+
const issueCompanyId = await resolveIssueCompanyId(ctx, issueId);
1290+
await ctx.issues.update(issueId, { status: "todo" }, issueCompanyId);
13021291
} catch (err) {
13031292
ctx.logger.error("Failed to reopen issue", { issueId, error: String(err) });
13041293
return {
@@ -1322,22 +1311,32 @@ async function handleButtonClick(
13221311
const issueId = customId.replace("issue_assign_", "");
13231312
ctx.logger.info("Assign to Me button clicked", { issueId, actor });
13241313
try {
1325-
const resp = await withRetry(async () => {
1326-
const r = await paperclipFetch(`${base}/api/issues/${issueId}`, {
1327-
method: "PATCH",
1328-
headers: { "Content-Type": "application/json" },
1329-
body: JSON.stringify({ assigneeUserId: `discord:${actor}`, comment: `Assigned to ${actor} via Discord` }),
1314+
const issueCompanyId = await resolveIssueCompanyId(ctx, issueId);
1315+
const issue = await ctx.issues.get(issueId, issueCompanyId) as {
1316+
assigneeUserId?: string | null;
1317+
assigneeAgentId?: string | null;
1318+
} | null;
1319+
1320+
if (issue?.assigneeUserId || issue?.assigneeAgentId) {
1321+
return respondToInteraction({
1322+
type: 4,
1323+
content: "Could not assign — issue already has an assignee.",
1324+
ephemeral: true,
13301325
});
1331-
throwOnRetryableStatus(r);
1332-
return r;
1333-
});
1334-
if (!resp.ok) {
1335-
const body = await resp.text().catch(() => "");
1336-
throw new Error(`API ${resp.status}: ${body}`);
13371326
}
1327+
1328+
await ctx.issues.update(
1329+
issueId,
1330+
{ assigneeUserId: `discord:${actor}` } as Record<string, unknown>,
1331+
issueCompanyId,
1332+
);
13381333
} catch (err) {
13391334
ctx.logger.error("Failed to assign issue", { issueId, error: String(err) });
1340-
return respondToInteraction({ type: 4, content: `Could not assign — ${err instanceof Error ? err.message : String(err)}`, ephemeral: true });
1335+
const rawMessage = err instanceof Error ? err.message : String(err);
1336+
const friendlyMessage = rawMessage.includes("Assignee user not found")
1337+
? "your Discord user is not linked to a Paperclip board user"
1338+
: rawMessage;
1339+
return respondToInteraction({ type: 4, content: `Could not assign — ${friendlyMessage}`, ephemeral: true });
13411340
}
13421341
return respondToInteraction({ type: 4, content: `✅ Assigned to **${actor}**`, ephemeral: true });
13431342
}
@@ -1497,6 +1496,18 @@ async function handleEscalationButton(
14971496
}
14981497
}
14991498

1499+
async function resolveIssueCompanyId(
1500+
ctx: PluginContext,
1501+
issueId: string,
1502+
): Promise<string> {
1503+
const companies = await ctx.companies.list();
1504+
for (const company of companies) {
1505+
const issue = await ctx.issues.get(issueId, company.id);
1506+
if (issue) return company.id;
1507+
}
1508+
throw new Error(`Issue not found: ${issueId}`);
1509+
}
1510+
15001511
// ---------------------------------------------------------------------------
15011512
// /clip commands subcommands
15021513
// ---------------------------------------------------------------------------

src/manifest.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const manifest: PaperclipPluginManifestV1 = {
2121
"companies.read",
2222
"issues.read",
2323
"issues.create",
24+
"issues.update",
2425
"agents.read",
2526
"agent.sessions.create",
2627
"agent.sessions.send",

tests/commands.test.ts

Lines changed: 69 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ function makeCtx(overrides: Record<string, unknown> = {}) {
3030
},
3131
issues: {
3232
list: vi.fn().mockResolvedValue([]),
33+
get: vi.fn().mockResolvedValue(null),
34+
update: vi.fn().mockResolvedValue({}),
3335
},
3436
companies: {
3537
list: vi.fn().mockResolvedValue([]),
@@ -1101,8 +1103,10 @@ describe("SLASH_COMMANDS", () => {
11011103

11021104
describe("issue_reopen button", () => {
11031105
it("calls PATCH to reopen the issue and returns type 7 success", async () => {
1104-
mockPaperclipFetch.mockResolvedValue({ ok: true, status: 200, headers: new Headers(), text: () => Promise.resolve("") });
11051106
const ctx = makeCtx();
1107+
ctx.companies.list = vi.fn().mockResolvedValue([{ id: "c1", name: "Acme" }]);
1108+
ctx.issues.get = vi.fn().mockResolvedValue({ id: "iss-42" });
1109+
ctx.issues.update = vi.fn().mockResolvedValue({ id: "iss-42", status: "todo" });
11061110
const cmdCtx = { ...defaultCmdCtx, baseUrl: "https://app.example.com" };
11071111
const result = await handleInteraction(
11081112
ctx,
@@ -1114,10 +1118,7 @@ describe("issue_reopen button", () => {
11141118
cmdCtx,
11151119
) as any;
11161120

1117-
expect(mockPaperclipFetch).toHaveBeenCalledWith(
1118-
"https://app.example.com/api/issues/iss-42",
1119-
expect.objectContaining({ method: "PATCH" }),
1120-
);
1121+
expect(ctx.issues.update).toHaveBeenCalledWith("iss-42", { status: "todo" }, "c1");
11211122
expect(result.type).toBe(7);
11221123
expect(result.data.embeds[0].title).toBe("Issue Reopened");
11231124
expect(result.data.embeds[0].description).toContain("reviewer");
@@ -1126,8 +1127,10 @@ describe("issue_reopen button", () => {
11261127
});
11271128

11281129
it("returns error embed when API fails", async () => {
1129-
mockPaperclipFetch.mockResolvedValue({ ok: false, status: 422, headers: new Headers(), text: () => Promise.resolve("Unprocessable Entity") });
11301130
const ctx = makeCtx();
1131+
ctx.companies.list = vi.fn().mockResolvedValue([{ id: "c1", name: "Acme" }]);
1132+
ctx.issues.get = vi.fn().mockResolvedValue({ id: "iss-fail" });
1133+
ctx.issues.update = vi.fn().mockRejectedValue(new Error("Unprocessable Entity"));
11311134
const result = await handleInteraction(
11321135
ctx,
11331136
{
@@ -1144,9 +1147,11 @@ describe("issue_reopen button", () => {
11441147
expect(result.data.components).toEqual([]);
11451148
});
11461149

1147-
it("sets status to todo in the PATCH body", async () => {
1148-
mockPaperclipFetch.mockResolvedValue({ ok: true, status: 200, headers: new Headers(), text: () => Promise.resolve("") });
1150+
it("sets status to todo in the update patch", async () => {
11491151
const ctx = makeCtx();
1152+
ctx.companies.list = vi.fn().mockResolvedValue([{ id: "c1", name: "Acme" }]);
1153+
ctx.issues.get = vi.fn().mockResolvedValue({ id: "iss-99" });
1154+
ctx.issues.update = vi.fn().mockResolvedValue({ id: "iss-99", status: "todo" });
11501155
await handleInteraction(
11511156
ctx,
11521157
{
@@ -1157,16 +1162,16 @@ describe("issue_reopen button", () => {
11571162
defaultCmdCtx,
11581163
);
11591164

1160-
const body = JSON.parse(mockPaperclipFetch.mock.calls[0][1].body);
1161-
expect(body.status).toBe("todo");
1162-
expect(body.comment).toContain("user1");
1165+
expect(ctx.issues.update).toHaveBeenCalledWith("iss-99", { status: "todo" }, "c1");
11631166
});
11641167
});
11651168

11661169
describe("issue_assign button", () => {
1167-
it("calls PATCH to assign and returns ephemeral success", async () => {
1168-
mockPaperclipFetch.mockResolvedValue({ ok: true, status: 200, headers: new Headers(), text: () => Promise.resolve("") });
1170+
it("updates the issue assignee and returns ephemeral success", async () => {
11691171
const ctx = makeCtx();
1172+
ctx.companies.list = vi.fn().mockResolvedValue([{ id: "c1", name: "Acme" }]);
1173+
ctx.issues.get = vi.fn().mockResolvedValue({ id: "iss-55" });
1174+
ctx.issues.update = vi.fn().mockResolvedValue({ id: "iss-55", assigneeUserId: "discord:assignee" });
11701175
const result = await handleInteraction(
11711176
ctx,
11721177
{
@@ -1177,18 +1182,17 @@ describe("issue_assign button", () => {
11771182
defaultCmdCtx,
11781183
) as any;
11791184

1180-
expect(mockPaperclipFetch).toHaveBeenCalledWith(
1181-
"http://localhost:3100/api/issues/iss-55",
1182-
expect.objectContaining({ method: "PATCH" }),
1183-
);
1185+
expect(ctx.issues.update).toHaveBeenCalledWith("iss-55", { assigneeUserId: "discord:assignee" }, "c1");
11841186
expect(result.type).toBe(4);
11851187
expect(result.data.content).toContain("assignee");
11861188
expect(result.data.flags).toBe(64); // ephemeral
11871189
});
11881190

1189-
it("returns ephemeral error when API fails", async () => {
1190-
mockPaperclipFetch.mockResolvedValue({ ok: false, status: 403, headers: new Headers(), text: () => Promise.resolve("Forbidden") });
1191+
it("returns ephemeral error when update fails", async () => {
11911192
const ctx = makeCtx();
1193+
ctx.companies.list = vi.fn().mockResolvedValue([{ id: "c1", name: "Acme" }]);
1194+
ctx.issues.get = vi.fn().mockResolvedValue({ id: "iss-denied" });
1195+
ctx.issues.update = vi.fn().mockRejectedValue(new Error("Forbidden"));
11921196
const result = await handleInteraction(
11931197
ctx,
11941198
{
@@ -1204,9 +1208,52 @@ describe("issue_assign button", () => {
12041208
expect(result.data.flags).toBe(64);
12051209
});
12061210

1207-
it("sends assigneeUserId with discord prefix in body", async () => {
1208-
mockPaperclipFetch.mockResolvedValue({ ok: true, status: 200, headers: new Headers(), text: () => Promise.resolve("") });
1211+
it("returns a clearer message when the Discord user is not linked to a board user", async () => {
1212+
const ctx = makeCtx();
1213+
ctx.companies.list = vi.fn().mockResolvedValue([{ id: "c1", name: "Acme" }]);
1214+
ctx.issues.get = vi.fn().mockResolvedValue({ id: "iss-unmapped" });
1215+
ctx.issues.update = vi.fn().mockRejectedValue(new Error("Assignee user not found"));
1216+
1217+
const result = await handleInteraction(
1218+
ctx,
1219+
{
1220+
type: 3,
1221+
data: { name: "button", custom_id: "issue_assign_iss-unmapped" },
1222+
member: { user: { username: "discord-user" } },
1223+
},
1224+
defaultCmdCtx,
1225+
) as any;
1226+
1227+
expect(result.type).toBe(4);
1228+
expect(result.data.content).toContain("not linked to a Paperclip board user");
1229+
});
1230+
1231+
it("does not attempt reassignment when the issue already has an assignee", async () => {
1232+
const ctx = makeCtx();
1233+
ctx.companies.list = vi.fn().mockResolvedValue([{ id: "c1", name: "Acme" }]);
1234+
ctx.issues.get = vi.fn().mockResolvedValue({ id: "iss-assigned", assigneeUserId: "board-user" });
1235+
ctx.issues.update = vi.fn();
1236+
1237+
const result = await handleInteraction(
1238+
ctx,
1239+
{
1240+
type: 3,
1241+
data: { name: "button", custom_id: "issue_assign_iss-assigned" },
1242+
member: { user: { username: "discord-user" } },
1243+
},
1244+
defaultCmdCtx,
1245+
) as any;
1246+
1247+
expect(result.type).toBe(4);
1248+
expect(result.data.content).toContain("already has an assignee");
1249+
expect(ctx.issues.update).not.toHaveBeenCalled();
1250+
});
1251+
1252+
it("sends assigneeUserId with discord prefix in update patch", async () => {
12091253
const ctx = makeCtx();
1254+
ctx.companies.list = vi.fn().mockResolvedValue([{ id: "c1", name: "Acme" }]);
1255+
ctx.issues.get = vi.fn().mockResolvedValue({ id: "iss-77" });
1256+
ctx.issues.update = vi.fn().mockResolvedValue({ id: "iss-77", assigneeUserId: "discord:bob" });
12101257
await handleInteraction(
12111258
ctx,
12121259
{
@@ -1217,8 +1264,7 @@ describe("issue_assign button", () => {
12171264
defaultCmdCtx,
12181265
);
12191266

1220-
const body = JSON.parse(mockPaperclipFetch.mock.calls[0][1].body);
1221-
expect(body.assigneeUserId).toBe("discord:bob");
1267+
expect(ctx.issues.update).toHaveBeenCalledWith("iss-77", { assigneeUserId: "discord:bob" }, "c1");
12221268
});
12231269
});
12241270

0 commit comments

Comments
 (0)