From 2d0e20f19e70bd6b0a5dfc12058b1375f231b01c Mon Sep 17 00:00:00 2001 From: betegon Date: Fri, 2 Oct 2026 18:28:23 +0200 Subject: [PATCH] feat(cli): add issue unlink command Remove stored external issue associations through @sentry/api, reusing link discovery and URL matching. Preserve standard deletion confirmation, dry-run previews, and successful no-ops when an association is absent. Co-Authored-By: GPT-6 --- .../cli-docs/src/content/docs/contributing.md | 2 +- apps/cli-docs/src/fragments/commands/issue.md | 34 ++- .../sentry-cli/skills/sentry-cli/SKILL.md | 1 + .../skills/sentry-cli/references/issue.md | 20 ++ packages/cli/src/commands/issue/index.ts | 5 +- packages/cli/src/commands/issue/link-utils.ts | 4 +- packages/cli/src/commands/issue/unlink.ts | 75 ++++++ packages/cli/src/lib/api/issue-app-links.ts | 25 ++ .../cli/src/lib/api/issue-integrations.ts | 33 +++ packages/cli/src/lib/complete.ts | 1 + .../cli/src/lib/formatters/issue-links.ts | 19 +- packages/cli/src/lib/issue-links.ts | 69 +++++- .../test/commands/issue/unlink.func.test.ts | 215 ++++++++++++++++++ .../cli/test/lib/api/issue-app-links.test.ts | 38 +++- .../test/lib/api/issue-integrations.test.ts | 45 +++- .../test/lib/formatters/issue-links.test.ts | 16 ++ packages/cli/test/lib/issue-links.test.ts | 71 +++++- packages/cli/test/lib/sdk-positionals.test.ts | 14 ++ 18 files changed, 663 insertions(+), 24 deletions(-) create mode 100644 packages/cli/src/commands/issue/unlink.ts create mode 100644 packages/cli/test/commands/issue/unlink.func.test.ts diff --git a/apps/cli-docs/src/content/docs/contributing.md b/apps/cli-docs/src/content/docs/contributing.md index d50f47fd8..00de8dc49 100644 --- a/apps/cli-docs/src/content/docs/contributing.md +++ b/apps/cli-docs/src/content/docs/contributing.md @@ -68,7 +68,7 @@ toolkit/ │ │ │ ├── dsn/ # list │ │ │ ├── event/ # list, send, view │ │ │ ├── feedback/ # list, resolve, spam, unresolve, view -│ │ │ ├── issue/ # archive, events, explain, link, list, merge, plan, resolve, unresolve, view +│ │ │ ├── issue/ # archive, events, explain, link, list, merge, plan, resolve, unlink, unresolve, view │ │ │ ├── local/ # run, serve │ │ │ ├── log/ # list, view │ │ │ ├── monitor/ # list, run diff --git a/apps/cli-docs/src/fragments/commands/issue.md b/apps/cli-docs/src/fragments/commands/issue.md index 9cff62d1c..8a5e195a8 100644 --- a/apps/cli-docs/src/fragments/commands/issue.md +++ b/apps/cli-docs/src/fragments/commands/issue.md @@ -352,13 +352,13 @@ sentry issue link my-org/FRONT-123 https://github.com/example/app/issues/42 --js `--dry-run` discovers the integration and prepares the link without submitting a write. The provider validates the remote issue when the link is submitted. An existing matching link succeeds with `changed: false`. A Sentry App that -already links this issue to a different resource must be unlinked in Sentry first. +already links this issue to a different resource must be unlinked first. App callbacks must return the exact supplied URL; use the issue URL copied from the tracker, including its title suffix. A mismatch fails without saving the link. GitHub and GitHub Enterprise pull requests are stored as external references. Their `/pull/NUMBER` and `/issues/NUMBER` URLs identify the same resource for -duplicate detection. Linking a PR does not mark it as a fix or +duplicate detection and unlinking. Linking a PR does not mark it as a fix or resolve the Sentry issue. This command does not create a tracker issue or link a commit. Existing @@ -372,3 +372,33 @@ OAuth login. If an older OAuth session lacks the requested scopes, the CLI offers reauthorization after a permission error. Use `sentry auth login` to request the current default scopes. Environment tokens must be updated separately. + +### Unlink an external issue + +Remove an association without deleting either issue: + +```bash +sentry issue unlink FRONT-123 https://github.com/example/app/issues/42 +sentry issue unlink FRONT-123 https://github.com/example/app/pull/43 --yes +sentry issue unlink my-org/FRONT-123 https://example.atlassian.net/browse/APP-42 --yes +sentry issue unlink FRONT-123 https://linear.app/example/issue/APP-42/fix-error --dry-run +``` + +Use `--yes` for non-interactive execution. `--dry-run` shows whether the link +exists without removing it. If the association is already absent, the command +succeeds with `changed: false`. + +Unlink matches the URL against stored associations and sends Sentry's internal +link ID to the existing DELETE endpoint. It does not require fetching the ticket +from the remote tracker, so a deleted remote ticket can still be unlinked. +For a custom Sentry App, select it with `--app `; unlink does not require +the app to expose a link form. Use `--integration ` to disambiguate native +integration links. + +#### Unlink permissions + +Unlink requires **`event:write` and access to the Sentry project**; `event:admin` +is also accepted. The organization's “Let Members Delete Events” setting does +not restrict unlinking on updated Sentry versions. + +Granting a token more scopes does not override project-access policy. diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 771c666c2..1a3c9379f 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -418,6 +418,7 @@ Manage Sentry issues - `sentry issue archive ` — Archive (ignore) an issue - `sentry issue merge ` — Merge 2+ issues into a single canonical group - `sentry issue link ` — Link an existing external issue +- `sentry issue unlink ` — Unlink an external issue → Full flags and examples: `references/issue.md` diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/issue.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/issue.md index ccda75288..0712658b7 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/issue.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/issue.md @@ -392,4 +392,24 @@ sentry issue link my-org/FRONT-123 https://github.com/example/app/issues/42 --dr sentry issue link my-org/FRONT-123 https://github.com/example/app/issues/42 --json ``` +### `sentry issue unlink ` + +Unlink an external issue + +**Flags:** +- `--integration - Native integration ID, when multiple installations match` +- `--app - Sentry App slug (automatically detected for Linear URLs)` +- `-y, --yes - Skip confirmation prompt` +- `-f, --force - Force the operation without confirmation` +- `-n, --dry-run - Show what would happen without making changes` + +**Examples:** + +```bash +sentry issue unlink FRONT-123 https://github.com/example/app/issues/42 +sentry issue unlink FRONT-123 https://github.com/example/app/pull/43 --yes +sentry issue unlink my-org/FRONT-123 https://example.atlassian.net/browse/APP-42 --yes +sentry issue unlink FRONT-123 https://linear.app/example/issue/APP-42/fix-error --dry-run +``` + All commands also support `--json`, `--fields`, `--help`, `--log-level`, and `--verbose` flags. diff --git a/packages/cli/src/commands/issue/index.ts b/packages/cli/src/commands/issue/index.ts index 2bf86f007..64538c81d 100644 --- a/packages/cli/src/commands/issue/index.ts +++ b/packages/cli/src/commands/issue/index.ts @@ -7,6 +7,7 @@ import { listCommand } from "./list.js"; import { mergeCommand } from "./merge.js"; import { planCommand } from "./plan.js"; import { resolveCommand } from "./resolve.js"; +import { unlinkCommand } from "./unlink.js"; import { unresolveCommand } from "./unresolve.js"; import { viewCommand } from "./view.js"; @@ -22,6 +23,7 @@ export const issueRoute = buildRouteMap({ archive: archiveCommand, merge: mergeCommand, link: linkCommand, + unlink: unlinkCommand, }, // `reopen` is a friendlier synonym for `unresolve`, `ignore` for `archive`. aliases: { reopen: "unresolve", ignore: "archive" }, @@ -40,7 +42,8 @@ export const issueRoute = buildRouteMap({ " unresolve Reopen a resolved issue (alias: reopen)\n" + " archive Archive/ignore an issue (alias: ignore)\n" + " merge Merge 2+ issues into a single group\n" + - " link Link an existing external issue\n\n" + + " link Link an existing external issue\n" + + " unlink Remove an external issue link\n\n" + "Magic selectors (available for view, events, explain, plan, resolve, unresolve, archive):\n" + " @latest Most recent unresolved issue\n" + " @most_frequent Issue with the highest event frequency\n\n" + diff --git a/packages/cli/src/commands/issue/link-utils.ts b/packages/cli/src/commands/issue/link-utils.ts index 35f1f04d7..107bc71ee 100644 --- a/packages/cli/src/commands/issue/link-utils.ts +++ b/packages/cli/src/commands/issue/link-utils.ts @@ -1,9 +1,9 @@ -/** Arguments for linking external issues. */ +/** Shared arguments for external issue association commands. */ import { ValidationError } from "../../lib/errors.js"; import { issueIdPositional } from "./utils.js"; -/** Required source issue and existing external resource URL for linking. */ +/** Required source issue and existing external resource URL for link and unlink. */ export const EXTERNAL_ISSUE_POSITIONALS = { kind: "tuple", parameters: [ diff --git a/packages/cli/src/commands/issue/unlink.ts b/packages/cli/src/commands/issue/unlink.ts new file mode 100644 index 000000000..decf8a996 --- /dev/null +++ b/packages/cli/src/commands/issue/unlink.ts @@ -0,0 +1,75 @@ +/** Remove an external issue association without deleting the external issue. */ + +import type { SentryContext } from "../../context.js"; +import { formatIssueLinkResult } from "../../lib/formatters/issue-links.js"; +import { CommandOutput } from "../../lib/formatters/output.js"; +import { unlinkExternalIssue } from "../../lib/issue-links.js"; +import { + buildDeleteCommand, + confirmByTyping, + isConfirmationBypassed, +} from "../../lib/mutate-command.js"; +import { + EXTERNAL_ISSUE_FLAGS, + EXTERNAL_ISSUE_POSITIONALS, +} from "./link-utils.js"; +import { resolveOrgAndIssueId } from "./utils.js"; + +type UnlinkFlags = { + readonly integration?: string; + readonly app?: string; + readonly "dry-run": boolean; + readonly yes: boolean; + readonly force: boolean; +}; + +export const unlinkCommand = buildDeleteCommand({ + docs: { + brief: "Unlink an external issue", + fullDescription: + "Remove an external tracker issue or GitHub pull request reference from a Sentry issue.\n" + + "This does not delete the external issue or change the Sentry issue's status.\n\n" + + "Requires event:write and access to the Sentry project.\n" + + "Older Sentry versions may still require event:admin.\n\n" + + "Examples:\n" + + " sentry issue unlink FRONT-123 https://github.com/example/app/issues/42\n" + + " sentry issue unlink FRONT-123 https://github.com/example/app/pull/43 --yes\n" + + " sentry issue unlink my-org/FRONT-123 https://example.atlassian.net/browse/APP-42 --yes\n" + + " sentry issue unlink FRONT-123 https://linear.app/example/issue/APP-42/fix-error --dry-run", + }, + output: { human: formatIssueLinkResult }, + parameters: { + positional: EXTERNAL_ISSUE_POSITIONALS, + flags: EXTERNAL_ISSUE_FLAGS, + }, + async *func( + this: SentryContext, + flags: UnlinkFlags, + issueArg: string, + url: string + ) { + const { org, issueId } = await resolveOrgAndIssueId({ + issueArg, + cwd: this.cwd, + command: "unlink", + }); + if (!(flags["dry-run"] || isConfirmationBypassed(flags))) { + const confirmed = await confirmByTyping( + issueArg, + `Type '${issueArg}' to unlink ${url}:` + ); + if (!confirmed) { + return { hint: "Cancelled." }; + } + } + const result = await unlinkExternalIssue({ + orgSlug: org, + issueId, + url, + integrationId: flags.integration, + appSlug: flags.app, + dryRun: flags["dry-run"], + }); + yield new CommandOutput(result); + }, +}); diff --git a/packages/cli/src/lib/api/issue-app-links.ts b/packages/cli/src/lib/api/issue-app-links.ts index 4378dac84..090d0bc38 100644 --- a/packages/cli/src/lib/api/issue-app-links.ts +++ b/packages/cli/src/lib/api/issue-app-links.ts @@ -4,6 +4,7 @@ */ import { + deleteOrganizationIssueExternalIssue, executeSentryAppInstallationExternalIssueAction, type GroupExternalIssueResponse, getSentryAppInstallationExternalRequestOptions, @@ -694,3 +695,27 @@ export async function linkAppIssue( changed: result.response?.status === 201, }; } + +/** Remove only the selected local app association, using event:write or event:admin. */ +export async function unlinkAppIssueLink( + orgSlug: string, + issueId: string, + linkId: string +): Promise { + if (!isAllDigits(linkId)) { + throw new ValidationError( + "App unlink requires the numeric association ID", + "linkId" + ); + } + requireIssueTarget(orgSlug, issueId); + const result = await deleteOrganizationIssueExternalIssue({ + ...getSdkConfig(await resolveOrgRegion(orgSlug)), + path: { + organization_id_or_slug: orgSlug, + issue_id: issueId, + external_issue_id: linkId, + }, + }); + unwrapResult(result, "Failed to unlink app issue"); +} diff --git a/packages/cli/src/lib/api/issue-integrations.ts b/packages/cli/src/lib/api/issue-integrations.ts index fae54ffb4..fcc2db348 100644 --- a/packages/cli/src/lib/api/issue-integrations.ts +++ b/packages/cli/src/lib/api/issue-integrations.ts @@ -1,5 +1,6 @@ /** Existing issue-tracker links through Sentry's native integrations. */ import { + deleteOrganizationIssueIntegration, type ExternalIssueLinkResponse, type IssueIntegrationsResponse, listOrganizationIssueIntegrations, @@ -239,6 +240,14 @@ function flattenLinks(integrations: NativeIntegration[]): NativeIssueLink[] { ); } +/** List every link, including links whose provider is no longer supported. */ +export async function listNativeIssueLinks( + orgSlug: string, + issueId: string +): Promise { + return flattenLinks(await listIntegrations(orgSlug, issueId)); +} + function matchesNativeUrl(link: NativeIssueLink, target: URL): boolean { const existing = storedUrl(link.url); if (!existing) { @@ -377,3 +386,27 @@ export async function linkNativeIssue( changed: result.response.status === 201, }; } + +/** The DELETE identifier is Sentry's ExternalIssue ID, not the provider key. */ +export async function unlinkNativeIssueLink( + orgSlug: string, + issueId: string, + link: NativeIssueLink +): Promise { + const externalIssue = Number(link.id); + if (!Number.isSafeInteger(externalIssue) || externalIssue <= 0) { + throw new ValidationError( + "External issue link ID must be a safe positive integer." + ); + } + const result = await deleteOrganizationIssueIntegration({ + ...getSdkConfig(await resolveOrgRegion(orgSlug)), + path: { + organization_id_or_slug: orgSlug, + issue_id: issueId, + integration_id: link.integrationId, + }, + query: { externalIssue }, + }); + unwrapResult(result, "Failed to unlink external issue"); +} diff --git a/packages/cli/src/lib/complete.ts b/packages/cli/src/lib/complete.ts index 873ab8ae2..7f735530a 100644 --- a/packages/cli/src/lib/complete.ts +++ b/packages/cli/src/lib/complete.ts @@ -99,6 +99,7 @@ export const ORG_PROJECT_COMMANDS = new Set([ "issue plan", "issue resolve", "issue link", + "issue unlink", "issue unresolve", "issue archive", "issue merge", diff --git a/packages/cli/src/lib/formatters/issue-links.ts b/packages/cli/src/lib/formatters/issue-links.ts index f56f556fa..e033ec0a0 100644 --- a/packages/cli/src/lib/formatters/issue-links.ts +++ b/packages/cli/src/lib/formatters/issue-links.ts @@ -1,4 +1,4 @@ -/** Human-readable results for linking existing external issues. */ +/** Human-readable results for linking and unlinking existing external issues. */ import type { ExternalIssueLinkResult } from "../issue-links.js"; import { renderMarkdown, safeCodeSpan } from "./markdown.js"; @@ -8,15 +8,22 @@ export function formatIssueLinkResult(result: ExternalIssueLinkResult): string { const external = safeCodeSpan(result.externalIssue.url); const issue = safeCodeSpan(`${result.org}/${result.issueId}`); if (result.dryRun) { + const needsChange = + result.action === "link" ? !result.linked : result.linked; return renderMarkdown( - result.linked - ? `Already linked: ${external}. (dry run)` - : `Would link ${external} to ${issue}. (dry run)` + needsChange + ? `Would ${result.action} ${external} ${result.action === "link" ? "to" : "from"} ${issue}. (dry run)` + : `Already ${result.linked ? "linked" : "unlinked"}: ${external}. (dry run)` + ); + } + if (!result.changed) { + return renderMarkdown( + `Already ${result.linked ? "linked" : "unlinked"}: ${external}.` ); } return renderMarkdown( - result.changed + result.linked ? `Linked ${external} to ${issue}.` - : `Already linked: ${external}.` + : `Unlinked ${external} from ${issue}. The external issue was not deleted.` ); } diff --git a/packages/cli/src/lib/issue-links.ts b/packages/cli/src/lib/issue-links.ts index 1fccada1f..39c4f89a5 100644 --- a/packages/cli/src/lib/issue-links.ts +++ b/packages/cli/src/lib/issue-links.ts @@ -1,17 +1,23 @@ /** - * Link existing external issues through Sentry's native integrations + * Link and unlink existing external issues through Sentry's native integrations * and Sentry Apps. These operations leave the Sentry issue's status unchanged. */ import { type AppIssueLink, + findAppIssueLink, linkAppIssue, + listAppIssueLinks, resolveAppIssueLink, + unlinkAppIssueLink, } from "./api/issue-app-links.js"; import { + findNativeIssueLink, linkNativeIssue, + listNativeIssueLinks, type NativeIssueLink, resolveNativeIssueLink, + unlinkNativeIssueLink, } from "./api/issue-integrations.js"; import { ValidationError } from "./errors.js"; import { resolveOrgRegion } from "./region.js"; @@ -46,7 +52,7 @@ export type ExternalIssueLinkResult = { /** Numeric Sentry issue ID. */ issueId: string; /** Requested operation. */ - action: "link"; + action: "link" | "unlink"; /** Whether the external issue remains linked after the operation. */ linked: boolean; /** Whether this invocation changed an association. */ @@ -78,6 +84,13 @@ type LinkPlan = { submit: () => Promise<{ ref: ExternalIssueRef; changed: boolean }>; }; +/** A stored association matching the requested URL. */ +type StoredLink = { + ref: ExternalIssueRef; + /** Delete the association, leaving the remote issue untouched. */ + remove: () => Promise; +}; + /** Validate the URL and return the Sentry App slug, or undefined for a native integration. */ function selectSentryApp( options: ExternalIssueLinkOptions @@ -164,14 +177,43 @@ async function planLink( }; } +async function findStoredLink( + options: ExternalIssueLinkOptions, + appSlug: string | undefined +): Promise { + const { orgSlug, issueId, url } = options; + if (appSlug) { + const links = await listAppIssueLinks(orgSlug, issueId); + // Only an explicit --app narrows the match: another App may store a Linear URL. + const link = findAppIssueLink(links, url, options.appSlug); + if (!link) { + return; + } + return { + ref: appRef(link), + remove: () => unlinkAppIssueLink(orgSlug, issueId, link.id), + }; + } + const links = await listNativeIssueLinks(orgSlug, issueId); + const link = findNativeIssueLink(links, url, options.integrationId); + if (!link) { + return; + } + return { + ref: nativeRef(link), + remove: () => unlinkNativeIssueLink(orgSlug, issueId, link), + }; +} + function toResult( options: ExternalIssueLinkOptions, + action: ExternalIssueLinkResult["action"], outcome: Pick ): ExternalIssueLinkResult { return { org: options.orgSlug, issueId: options.issueId, - action: "link", + action, dryRun: options.dryRun, ...outcome, }; @@ -200,7 +242,7 @@ export async function linkExternalIssue( ): Promise { const plan = await planLink(options, selectSentryApp(options)); if (options.dryRun) { - return toResult(options, { + return toResult(options, "link", { linked: plan.linked, changed: false, externalIssue: plan.preview, @@ -210,9 +252,26 @@ export async function linkExternalIssue( if (changed) { await invalidateIssueLinks(options); } - return toResult(options, { + return toResult(options, "link", { linked: true, changed, externalIssue: ref, }); } + +/** Remove a stored association without contacting or deleting the remote ticket. */ +export async function unlinkExternalIssue( + options: ExternalIssueLinkOptions +): Promise { + const appSlug = selectSentryApp(options); + const link = await findStoredLink(options, appSlug); + if (link && !options.dryRun) { + await link.remove(); + await invalidateIssueLinks(options); + } + return toResult(options, "unlink", { + linked: Boolean(link && options.dryRun), + changed: Boolean(link && !options.dryRun), + externalIssue: link?.ref ?? { url: options.url, provider: appSlug }, + }); +} diff --git a/packages/cli/test/commands/issue/unlink.func.test.ts b/packages/cli/test/commands/issue/unlink.func.test.ts new file mode 100644 index 000000000..7a28c16ae --- /dev/null +++ b/packages/cli/test/commands/issue/unlink.func.test.ts @@ -0,0 +1,215 @@ +/** Tests the issue unlink command with its real destructive-command guard. */ + +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { unlinkCommand } from "../../../src/commands/issue/unlink.js"; +import { resolveOrgAndIssueId } from "../../../src/commands/issue/utils.js"; +import { + type ExternalIssueLinkResult, + unlinkExternalIssue, +} from "../../../src/lib/issue-links.js"; +import { confirmByTyping } from "../../../src/lib/mutate-command.js"; + +const { mockIsatty } = vi.hoisted(() => ({ mockIsatty: vi.fn(() => false) })); + +vi.mock("node:tty", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + isatty: mockIsatty, + default: { ...actual, isatty: mockIsatty }, + }; +}); + +vi.mock("../../../src/commands/issue/utils.js", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../src/commands/issue/utils.js") + >()), + resolveOrgAndIssueId: vi.fn(), +})); + +vi.mock("../../../src/lib/issue-links.js", () => ({ + unlinkExternalIssue: vi.fn(), +})); + +vi.mock("../../../src/lib/mutate-command.js", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../src/lib/mutate-command.js") + >()), + confirmByTyping: vi.fn(), +})); + +const externalUrl = "https://github.com/example/app/issues/42"; +const defaultFlags = { + "dry-run": false, + yes: false, + force: false, + json: false, +}; +const unlinkedResult: ExternalIssueLinkResult = { + org: "test-org", + issueId: "123456789", + action: "unlink", + linked: false, + changed: true, + externalIssue: { + id: "789", + identifier: "example/app#42", + url: externalUrl, + provider: "github", + }, +}; + +function createMockContext() { + const stdoutWrite = vi.fn((_chunk: string) => true); + return { + context: { + stdout: { write: stdoutWrite }, + stderr: { write: vi.fn((_chunk: string) => true) }, + cwd: "/tmp/example-project", + }, + output: () => stdoutWrite.mock.calls.map(([chunk]) => chunk).join(""), + }; +} + +describe("issue unlink", () => { + beforeEach(() => { + mockIsatty.mockReset().mockReturnValue(false); + vi.mocked(resolveOrgAndIssueId).mockReset(); + vi.mocked(unlinkExternalIssue).mockReset(); + vi.mocked(confirmByTyping).mockReset().mockResolvedValue(true); + vi.mocked(resolveOrgAndIssueId).mockResolvedValue({ + org: "test-org", + issueId: "123456789", + }); + vi.mocked(unlinkExternalIssue).mockResolvedValue(unlinkedResult); + }); + + test.each([ + { + selector: { integration: "99" }, + expected: { integrationId: "99", appSlug: undefined }, + }, + { + selector: { app: "custom-tracker" }, + expected: { integrationId: undefined, appSlug: "custom-tracker" }, + }, + ])("forwards resolved issue context and selector $selector", async ({ + selector, + expected, + }) => { + const { context, output } = createMockContext(); + const func = await unlinkCommand.loader(); + await func.call( + context, + { ...defaultFlags, ...selector, yes: true }, + "test-org/APP-42", + externalUrl + ); + + expect(resolveOrgAndIssueId).toHaveBeenCalledExactlyOnceWith({ + issueArg: "test-org/APP-42", + cwd: "/tmp/example-project", + command: "unlink", + }); + expect(unlinkExternalIssue).toHaveBeenCalledExactlyOnceWith({ + orgSlug: "test-org", + issueId: "123456789", + url: externalUrl, + ...expected, + dryRun: false, + }); + expect(confirmByTyping).not.toHaveBeenCalled(); + expect(output()).toContain("Unlinked"); + expect(output()).toContain("external issue was not deleted"); + }); + + test("refuses non-interactive mutation without explicit confirmation before resolving", async () => { + const { context, output } = createMockContext(); + const func = await unlinkCommand.loader(); + + await expect( + func.call(context, defaultFlags, "APP-42", externalUrl) + ).rejects.toThrow("Use --yes or --force to confirm."); + + expect(resolveOrgAndIssueId).not.toHaveBeenCalled(); + expect(confirmByTyping).not.toHaveBeenCalled(); + expect(unlinkExternalIssue).not.toHaveBeenCalled(); + expect(output()).toBe(""); + }); + + test.each([ + "yes", + "force", + ] as const)("allows non-interactive --%s without prompting", async (flag) => { + const { context } = createMockContext(); + const func = await unlinkCommand.loader(); + await func.call( + context, + { ...defaultFlags, [flag]: true }, + "APP-42", + externalUrl + ); + + expect(confirmByTyping).not.toHaveBeenCalled(); + expect(unlinkExternalIssue).toHaveBeenCalledExactlyOnceWith({ + orgSlug: "test-org", + issueId: "123456789", + url: externalUrl, + integrationId: undefined, + appSlug: undefined, + dryRun: false, + }); + }); + + test("confirms the selected issue and external URL before unlinking interactively", async () => { + mockIsatty.mockReturnValue(true); + const { context } = createMockContext(); + const func = await unlinkCommand.loader(); + await func.call(context, defaultFlags, "test-org/APP-42", externalUrl); + + expect(confirmByTyping).toHaveBeenCalledExactlyOnceWith( + "test-org/APP-42", + `Type 'test-org/APP-42' to unlink ${externalUrl}:` + ); + expect(unlinkExternalIssue).toHaveBeenCalledOnce(); + expect(vi.mocked(confirmByTyping).mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(unlinkExternalIssue).mock.invocationCallOrder[0] + ); + }); + + test("cancelling confirmation leaves the association untouched", async () => { + mockIsatty.mockReturnValue(true); + vi.mocked(confirmByTyping).mockResolvedValue(false); + const { context, output } = createMockContext(); + const func = await unlinkCommand.loader(); + await func.call(context, defaultFlags, "APP-42", externalUrl); + + expect(confirmByTyping).toHaveBeenCalledOnce(); + expect(unlinkExternalIssue).not.toHaveBeenCalled(); + expect(output()).toContain("Cancelled."); + }); + + test("permits --dry-run without a TTY and preserves current link state in JSON", async () => { + const result = { + ...unlinkedResult, + linked: true, + changed: false, + dryRun: true, + }; + vi.mocked(unlinkExternalIssue).mockResolvedValue(result); + const { context, output } = createMockContext(); + const func = await unlinkCommand.loader(); + await func.call( + context, + { ...defaultFlags, "dry-run": true, json: true }, + "APP-42", + externalUrl + ); + + expect(confirmByTyping).not.toHaveBeenCalled(); + expect(unlinkExternalIssue).toHaveBeenCalledWith( + expect.objectContaining({ dryRun: true }) + ); + expect(JSON.parse(output())).toEqual(result); + }); +}); diff --git a/packages/cli/test/lib/api/issue-app-links.test.ts b/packages/cli/test/lib/api/issue-app-links.test.ts index 0d4e2d991..d37ca3674 100644 --- a/packages/cli/test/lib/api/issue-app-links.test.ts +++ b/packages/cli/test/lib/api/issue-app-links.test.ts @@ -1,4 +1,4 @@ -/** Contract tests for installed app callbacks, singleton protection, and regional discovery. */ +/** Contract tests for installed app callbacks, singleton protection, and regional unlinking. */ import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { type AppIssueLink, @@ -6,6 +6,7 @@ import { linkAppIssue, listAppIssueLinks, resolveAppIssueLink, + unlinkAppIssueLink, } from "../../../src/lib/api/issue-app-links.js"; import { setAuthToken } from "../../../src/lib/db/auth.js"; import { setOrgRegion } from "../../../src/lib/db/regions.js"; @@ -99,6 +100,9 @@ beforeEach(async () => { actionStatus ); } + if (request.method === "DELETE") { + return new Response(null, { status: 204 }); + } throw new Error(`Unexpected request: ${request.method} ${request.url}`); }); }); @@ -613,7 +617,7 @@ describe("app issue-link action", () => { }); }); -describe("list and match app associations", () => { +describe("list and unlink app associations", () => { test("follows cursor pages and never uses a pagination URL as a request target", async () => { globalThis.fetch = mockFetch(async (input, init) => { const request = new Request(input, init); @@ -642,6 +646,36 @@ describe("list and match app associations", () => { ); }); + test("deletes the group association ID in its region without an app schema", async () => { + await unlinkAppIssueLink(ORG, ISSUE, LINK.id); + expect(calls).toHaveLength(1); + expect(calls[0]?.url).toBe( + "https://de.sentry.io/api/0/organizations/example-org/issues/123/external-issues/99/" + ); + expect(calls[0]?.method).toBe("DELETE"); + }); + + test("rejects relative path segments before issuing an unlink", async () => { + await expect(unlinkAppIssueLink(ORG, ISSUE, "..")).rejects.toThrow( + ValidationError + ); + await expect(unlinkAppIssueLink(ORG, "..", LINK.id)).rejects.toThrow( + ValidationError + ); + expect(calls).toHaveLength(0); + }); + + test("propagates unlink permissions without falling back to installation registration", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + calls.push(new Request(input, init)); + return json({ detail: "Requires event:admin" }, 403); + }); + await expect( + unlinkAppIssueLink(ORG, ISSUE, LINK.id) + ).rejects.toBeInstanceOf(ApiError); + expect(calls).toHaveLength(1); + }); + test("matches generic URLs and refuses ambiguity across apps", () => { const link = { ...LINK, diff --git a/packages/cli/test/lib/api/issue-integrations.test.ts b/packages/cli/test/lib/api/issue-integrations.test.ts index 19ea49bb0..dd7cd7536 100644 --- a/packages/cli/test/lib/api/issue-integrations.test.ts +++ b/packages/cli/test/lib/api/issue-integrations.test.ts @@ -5,11 +5,15 @@ import { type NativeIssueLink, resolveNativeIssueLink, selectNativeIntegration, + unlinkNativeIssueLink, } from "../../../src/lib/api/issue-integrations.js"; import { setAuthToken } from "../../../src/lib/db/auth.js"; import { setOrgRegion } from "../../../src/lib/db/regions.js"; import { ApiError } from "../../../src/lib/errors.js"; -import { linkExternalIssue } from "../../../src/lib/issue-links.js"; +import { + linkExternalIssue, + unlinkExternalIssue, +} from "../../../src/lib/issue-links.js"; import { mockFetch, useTestConfigDir } from "../../helpers.js"; const REGION = "https://eu.sentry.io"; @@ -443,6 +447,26 @@ describe("native link API", () => { expect(prepared.existing).toEqual(LINK); }); + test("unlinks by Sentry's association ID in the organization's region", async () => { + const requests = mockApi(() => new Response(null, { status: 204 })); + await unlinkNativeIssueLink(SOURCE.orgSlug, SOURCE.issueId, LINK); + const url = new URL(requests[0]?.url ?? ""); + expect(requests.map((request) => request.method)).toEqual(["DELETE"]); + expect(`${url.origin}${url.pathname}`).toBe(`${REGION}${INTEGRATIONS}10/`); + expect(url.searchParams.get("externalIssue")).toBe("1234"); + }); + + test("rejects an unlink ID that the SDK numeric query cannot represent exactly", async () => { + const requests = mockApi(() => new Response(null, { status: 204 })); + await expect( + unlinkNativeIssueLink(SOURCE.orgSlug, SOURCE.issueId, { + ...LINK, + id: "9007199254740993", + }) + ).rejects.toThrow("safe positive integer"); + expect(requests).toHaveLength(0); + }); + test.each([ "https://username:secret@tracker.example.com/browse/PROJ-7", "javascript:alert(1)", @@ -453,7 +477,7 @@ describe("native link API", () => { expect(requests).toHaveLength(0); }); - test("links a GitHub PR and returns its canonical URL", async () => { + test("links a GitHub PR and unlinks it through its other URL form", async () => { const pullUrl = "https://github.com/Owner/Repo/pull/7"; const storedLink = { ...LINK, @@ -462,19 +486,25 @@ describe("native link API", () => { displayName: "Owner/Repo#7", url: "https://github.com/Owner/Repo/issues/7", }; + let linked = false; const requests = mockApi((request) => { if (request.method === "PUT") { + linked = true; // The mutation returns GitHub's html_url; listing reconstructs /issues/N. return Response.json( { ...storedLink, id: 1234, integrationId: 10, url: pullUrl }, { status: 201 } ); } + if (request.method === "DELETE") { + linked = false; + return new Response(null, { status: 204 }); + } return json([ integration({ provider: "github", domainName: "github.com/owner", - externalIssues: [], + externalIssues: linked ? [storedLink] : [], }), ]); }); @@ -487,10 +517,17 @@ describe("native link API", () => { changed: true, externalIssue: { id: "1234", identifier: "Owner/Repo#7", url: pullUrl }, }); + expect(await unlinkExternalIssue(options)).toMatchObject({ + changed: true, + externalIssue: { id: "1234" }, + }); + expect(await unlinkExternalIssue(options)).toMatchObject({ + changed: false, + }); expect( requests .filter((request) => request.method !== "GET") .map((request) => request.method) - ).toEqual(["PUT"]); + ).toEqual(["PUT", "DELETE"]); }); }); diff --git a/packages/cli/test/lib/formatters/issue-links.test.ts b/packages/cli/test/lib/formatters/issue-links.test.ts index 6b6ffd532..950f00e01 100644 --- a/packages/cli/test/lib/formatters/issue-links.test.ts +++ b/packages/cli/test/lib/formatters/issue-links.test.ts @@ -33,6 +33,22 @@ describe("formatIssueLinkResult", () => { { action: "link", linked: true, changed: false }, `Already linked: ${URL}.`, ], + [ + { action: "unlink", linked: true, changed: false, dryRun: true }, + `Would unlink ${URL} from test-org/123. (dry run)`, + ], + [ + { action: "unlink", linked: false, changed: false, dryRun: true }, + `Already unlinked: ${URL}. (dry run)`, + ], + [ + { action: "unlink", linked: false, changed: true }, + `Unlinked ${URL} from test-org/123. The external issue was not deleted.`, + ], + [ + { action: "unlink", linked: false, changed: false }, + `Already unlinked: ${URL}.`, + ], ] as const)("renders %j", (state, expected) => { const result: ExternalIssueLinkResult = { org: "test-org", diff --git a/packages/cli/test/lib/issue-links.test.ts b/packages/cli/test/lib/issue-links.test.ts index 34582fb64..33cba6d8f 100644 --- a/packages/cli/test/lib/issue-links.test.ts +++ b/packages/cli/test/lib/issue-links.test.ts @@ -2,15 +2,24 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; import { + findAppIssueLink, linkAppIssue, + listAppIssueLinks, resolveAppIssueLink, + unlinkAppIssueLink, } from "../../src/lib/api/issue-app-links.js"; import { + findNativeIssueLink, linkNativeIssue, + listNativeIssueLinks, resolveNativeIssueLink, + unlinkNativeIssueLink, } from "../../src/lib/api/issue-integrations.js"; import { ApiError } from "../../src/lib/errors.js"; -import { linkExternalIssue } from "../../src/lib/issue-links.js"; +import { + linkExternalIssue, + unlinkExternalIssue, +} from "../../src/lib/issue-links.js"; import { invalidateCachedResponsesMatching } from "../../src/lib/response-cache.js"; vi.mock("../../src/lib/api/issue-app-links.js"); @@ -56,6 +65,8 @@ beforeEach(() => { link: nativeLink, changed: true, }); + vi.mocked(listNativeIssueLinks).mockResolvedValue([nativeLink]); + vi.mocked(findNativeIssueLink).mockReturnValue(nativeLink); vi.mocked(resolveAppIssueLink).mockResolvedValue({ ...options, url: appLink.webUrl, @@ -65,6 +76,8 @@ beforeEach(() => { fields: { issueId: "remote-uuid" }, }); vi.mocked(linkAppIssue).mockResolvedValue({ link: appLink, changed: true }); + vi.mocked(listAppIssueLinks).mockResolvedValue([appLink]); + vi.mocked(findAppIssueLink).mockReturnValue(appLink); }); describe("external issue associations", () => { @@ -161,6 +174,62 @@ describe("external issue associations", () => { expect(invalidateCachedResponsesMatching).not.toHaveBeenCalled(); }); + test("unlink selects a stored native association, without resolving the remote issue", async () => { + const result = await unlinkExternalIssue(options); + expect(unlinkNativeIssueLink).toHaveBeenCalledWith( + "example", + "123", + nativeLink + ); + expect(resolveNativeIssueLink).not.toHaveBeenCalled(); + expect(resolveAppIssueLink).not.toHaveBeenCalled(); + expect(result).toMatchObject({ linked: false, changed: true }); + }); + + test("unlink uses the stored app record ID, without invoking a link workflow", async () => { + const result = await unlinkExternalIssue({ + ...options, + url: appLink.webUrl, + }); + expect(unlinkAppIssueLink).toHaveBeenCalledWith("example", "123", "910"); + expect(resolveAppIssueLink).not.toHaveBeenCalled(); + expect(linkAppIssue).not.toHaveBeenCalled(); + expect(result).toMatchObject({ linked: false, changed: true }); + }); + + test.each([ + nativeLink.url, + appLink.webUrl, + ])("dry-run unlink preserves the link: %s", async (url) => { + const result = await unlinkExternalIssue({ ...options, url, dryRun: true }); + expect(result).toMatchObject({ + linked: true, + changed: false, + dryRun: true, + }); + expect(unlinkNativeIssueLink).not.toHaveBeenCalled(); + expect(unlinkAppIssueLink).not.toHaveBeenCalled(); + }); + + test.each([ + nativeLink.url, + appLink.webUrl, + ])("missing association is already unlinked: %s", async (url) => { + vi.mocked(findNativeIssueLink).mockReturnValue(undefined); + vi.mocked(findAppIssueLink).mockReturnValue(undefined); + const result = await unlinkExternalIssue({ ...options, url }); + expect(result).toMatchObject({ linked: false, changed: false }); + expect(unlinkNativeIssueLink).not.toHaveBeenCalled(); + expect(unlinkAppIssueLink).not.toHaveBeenCalled(); + }); + + test("a failed link read is not treated as an empty list", async () => { + const error = new ApiError("Forbidden", 403); + vi.mocked(listNativeIssueLinks).mockRejectedValue(error); + await expect(unlinkExternalIssue(options)).rejects.toBe(error); + expect(unlinkNativeIssueLink).not.toHaveBeenCalled(); + }); + test("a failed write propagates without claiming success or falling back to another provider", async () => { const error = new ApiError("Forbidden", 403); vi.mocked(linkNativeIssue).mockRejectedValue(error); diff --git a/packages/cli/test/lib/sdk-positionals.test.ts b/packages/cli/test/lib/sdk-positionals.test.ts index 87b694994..ea4b5f119 100644 --- a/packages/cli/test/lib/sdk-positionals.test.ts +++ b/packages/cli/test/lib/sdk-positionals.test.ts @@ -46,6 +46,20 @@ describe("generated SDK positional arguments", () => { }); }); + test("issue unlink forwards the issue and URL as separate positionals", async () => { + const { calls, sdk } = createRecordingSDK(); + await sdk.issue.unlink({ + issue: "example/APP-42", + url: "https://github.com/example/app/pull/123", + yes: true, + }); + expect(calls[0]).toMatchObject({ + path: ["issue", "unlink"], + positional: ["example/APP-42", "https://github.com/example/app/pull/123"], + flags: { yes: true }, + }); + }); + test("release deploy passes version, environment and name as separate tokens", async () => { const { calls, sdk } = createRecordingSDK();