Merge pull request #39 from Freudator86/feat/pull-request-url-addressing

feat: address a pull request by its URL as well as its number

Author: escidmore

docs/contract.md

@@ -13,16 +13,16 @@ forgejo-axi repo view --repo OWNER/REPO [connection flags]
 forgejo-axi api METHOD PATH [--data JSON] [--paginate [--limit N|--full]] [connection flags]
 forgejo-axi pr find --repo OWNER/REPO --head BRANCH [--base BRANCH] [--state STATE]
 forgejo-axi pr list --repo OWNER/REPO [--state STATE] [--limit N|--full] [--fields LIST|all]
-forgejo-axi pr view --repo OWNER/REPO NUMBER [--full] [--fields LIST|all]
-forgejo-axi pr history [overview|list|detail|soft-delete] --repo OWNER/REPO NUMBER [--comment-id ID] [--history-id ID] [--raw] [--yes]
-forgejo-axi pr reviews --repo OWNER/REPO NUMBER [--limit N|--full]
-forgejo-axi pr diff --repo OWNER/REPO NUMBER [--full]
+forgejo-axi pr view [--repo OWNER/REPO] NUMBER|URL [--full] [--fields LIST|all]
+forgejo-axi pr history [overview|list|detail|soft-delete] [--repo OWNER/REPO] NUMBER|URL [--comment-id ID] [--history-id ID] [--raw] [--yes]
+forgejo-axi pr reviews [--repo OWNER/REPO] NUMBER|URL [--limit N|--full]
+forgejo-axi pr diff [--repo OWNER/REPO] NUMBER|URL [--full]
 forgejo-axi pr create --repo OWNER/REPO --title TITLE --head BRANCH --base BRANCH [--body BODY | --body-file PATH|-] [--draft]
-forgejo-axi pr update --repo OWNER/REPO NUMBER [--title TITLE] [--body BODY | --body-file PATH|-] [--base BRANCH] [--state open|closed]
-forgejo-axi pr checks --repo OWNER/REPO NUMBER
-forgejo-axi pr mergeability --repo OWNER/REPO NUMBER
-forgejo-axi pr merge --repo OWNER/REPO NUMBER --expected-head SHA [--method merge|squash|rebase]
-forgejo-axi pr merged --repo OWNER/REPO NUMBER
+forgejo-axi pr update [--repo OWNER/REPO] NUMBER|URL [--title TITLE] [--body BODY | --body-file PATH|-] [--base BRANCH] [--state open|closed]
+forgejo-axi pr checks [--repo OWNER/REPO] NUMBER|URL
+forgejo-axi pr mergeability [--repo OWNER/REPO] NUMBER|URL
+forgejo-axi pr merge [--repo OWNER/REPO] NUMBER|URL --expected-head SHA [--method merge|squash|rebase]
+forgejo-axi pr merged [--repo OWNER/REPO] NUMBER|URL
 forgejo-axi label list --repo OWNER/REPO [--limit N|--full]
 forgejo-axi label create --repo OWNER/REPO NAME [--color HEX] [--description TEXT]
 forgejo-axi label edit --repo OWNER/REPO NAME [--name NEW] [--color HEX] [--description TEXT]
@@ -51,6 +51,8 @@ separate value beginning with `-` remains reserved for flags, except the exact
 
 For `pr create` and `pr update`, `--body` and `--body-file` are mutually exclusive. `--body-file PATH` reads a UTF-8 file and `--body-file -` reads stdin; body-file content is forwarded verbatim. Input that cannot be read, or whose bytes are not valid UTF-8, is refused with `BODY_FILE_ERROR` (exit `2`) before any request is made, because the invocation named an unusable body source rather than the host failing. No other command accepts `--body-file`: `issue create`, `issue edit`, and `issue comment` take `--body` only.
 
+A pull request `URL` supplies the repository and number for every command that accepts `NUMBER|URL`; it never changes the configured Forgejo base URL or where credentials are sent. The URL must use HTTP(S), contain no credentials, and carry an `OWNER/REPO/pulls/NUMBER` path, including beneath a path prefix or the API route. An explicit `--repo` may accompany a URL only when it names the same repository; a URL overrides `FORGEJO_REPOSITORY`.
+
 `--fields LIST|all` on `pr view`, `issue view`, and `run view` selects fields only from the command's primary `pull_request`, `issue`, or `run` object. Sibling comments, jobs, metadata, and hints are unchanged. Fields are emitted in requested order; a requested optional field that the host did not supply is omitted rather than fabricated as `null`. `all` selects every field that detail object can expose.
 
 Connection flags are `--base-url URL`, `--token-env NAME`, `--timeout-ms N`, `--ca-file PATH`, and `--json`. Environment defaults are `FORGEJO_BASE_URL`, `FORGEJO_REPOSITORY`, `FORGEJO_TIMEOUT_MS`, and `FORGEJO_CA_FILE`. `--ca-file`/`FORGEJO_CA_FILE` supplies a replacement CA trust bundle, matching Node's TLS `ca` behavior; it does not append to the platform trust store.

skills/forgejo-axi/SKILL.md

@@ -225,6 +225,10 @@ Commands:
   merge         Merge only the expected head and return merged proof
   merged        Return merged-state proof
 
+Wherever a command takes NUMBER it also accepts the pull request's URL, which
+carries the repository with it and makes --repo unnecessary. The request still
+goes to the configured base URL; the URL's host is never used to route it.
+
 Run `forgejo-axi pr <command> --help` for flags and examples.
 ```
 
@@ -262,7 +266,7 @@ Examples:
 forgejo-axi pr view — show canonical pull request identity and body
 
 Usage:
-  forgejo-axi pr view --repo OWNER/REPO NUMBER [--full] [--fields LIST|all] [connection flags]
+  forgejo-axi pr view [--repo OWNER/REPO] NUMBER|URL [--full] [--fields LIST|all] [connection flags]
 
 Flags:
   --full         Display the complete pull request body instead of its preview
@@ -272,7 +276,7 @@ A requested field the host does not supply is omitted, never emitted as null.
 
 Examples:
   forgejo-axi pr view --repo owner/repo 42 --full
-  forgejo-axi pr view --repo owner/repo 42 --fields state,head_sha
+  forgejo-axi pr view https://forgejo.example/owner/repo/pulls/42 --fields state,head_sha
 ```
 
 #### pr history
@@ -281,7 +285,7 @@ Examples:
 forgejo-axi pr history — inspect or soft-delete pull request content history
 
 Usage:
-  forgejo-axi pr history [overview|list|detail|soft-delete] --repo OWNER/REPO NUMBER [--comment-id ID] [--history-id ID] [--raw] [--yes] [connection flags]
+  forgejo-axi pr history [overview|list|detail|soft-delete] [--repo OWNER/REPO] NUMBER|URL [--comment-id ID] [--history-id ID] [--raw] [--yes] [connection flags]
 
 Commands:
   overview    Show edit counts for the body and comments
@@ -308,7 +312,7 @@ Examples:
 forgejo-axi pr reviews — list reviews with their inline comments
 
 Usage:
-  forgejo-axi pr reviews --repo OWNER/REPO NUMBER [--limit N|--full] [connection flags]
+  forgejo-axi pr reviews [--repo OWNER/REPO] NUMBER|URL [--limit N|--full] [connection flags]
 
 Flags:
   --limit N   Display this many reviews instead of the default 30
@@ -324,7 +328,7 @@ Example:
 forgejo-axi pr diff — print the unified diff
 
 Usage:
-  forgejo-axi pr diff --repo OWNER/REPO NUMBER [--full] [connection flags]
+  forgejo-axi pr diff [--repo OWNER/REPO] NUMBER|URL [--full] [connection flags]
 
 Flags:
   --full   Print every line instead of the first 30
@@ -360,7 +364,7 @@ Examples:
 forgejo-axi pr update — idempotently update a pull request
 
 Usage:
-  forgejo-axi pr update --repo OWNER/REPO NUMBER [--title TITLE] [--body BODY | --body-file PATH|-] [--base BRANCH] [--state open|closed] [connection flags]
+  forgejo-axi pr update [--repo OWNER/REPO] NUMBER|URL [--title TITLE] [--body BODY | --body-file PATH|-] [--base BRANCH] [--state open|closed] [connection flags]
 
 Flags:
   --body BODY           Pull request body text
@@ -380,7 +384,7 @@ Examples:
 forgejo-axi pr checks — normalize statuses and required contexts
 
 Usage:
-  forgejo-axi pr checks --repo OWNER/REPO NUMBER [connection flags]
+  forgejo-axi pr checks [--repo OWNER/REPO] NUMBER|URL [connection flags]
 
 Example:
   forgejo-axi pr checks --repo owner/repo 42 --json
@@ -392,7 +396,7 @@ Example:
 forgejo-axi pr mergeability — evaluate mergeability and required checks
 
 Usage:
-  forgejo-axi pr mergeability --repo OWNER/REPO NUMBER [connection flags]
+  forgejo-axi pr mergeability [--repo OWNER/REPO] NUMBER|URL [connection flags]
 
 Example:
   forgejo-axi pr mergeability --repo owner/repo 42
@@ -404,7 +408,7 @@ Example:
 forgejo-axi pr merge — expected-head merge with merged-state proof
 
 Usage:
-  forgejo-axi pr merge --repo OWNER/REPO NUMBER --expected-head SHA [--method merge|squash|rebase] [connection flags]
+  forgejo-axi pr merge [--repo OWNER/REPO] NUMBER|URL --expected-head SHA [--method merge|squash|rebase] [connection flags]
 
 Example:
   forgejo-axi pr merge --repo owner/repo 42 --expected-head abc123 --method squash
@@ -416,7 +420,7 @@ Example:
 forgejo-axi pr merged — return merged-state proof
 
 Usage:
-  forgejo-axi pr merged --repo OWNER/REPO NUMBER [connection flags]
+  forgejo-axi pr merged [--repo OWNER/REPO] NUMBER|URL [connection flags]
 
 Example:
   forgejo-axi pr merged --repo owner/repo 42

src/cli.ts

@@ -19,7 +19,7 @@ import {
   normalizeLabelColor,
   pageInfo,
   parseIssueNumber,
-  parsePullNumber,
+  parsePullTarget,
   parseRepository,
   parseRunId,
   type IssueIdentity,
@@ -379,10 +379,7 @@ async function pullRead(
     }),
     `pr ${command}`,
   );
-  const number = parsePullNumber(
-    requireOnePositional(parsed, 'pull request number'),
-  );
-  const repo = resolveRepo(parsed, env);
+  const { repo, number } = resolvePullTarget(parsed, env);
   const service = await serviceFor(parsed, env);
   if (command === 'view') {
     const fields = chooseDetailFields(
@@ -422,14 +419,11 @@ async function pullReviews(
     }),
     'pr reviews',
   );
-  const number = parsePullNumber(
-    requireOnePositional(parsed, 'pull request number'),
-  );
+  const { repo, number } = resolvePullTarget(parsed, env);
   const full = boolFlag(parsed, '--full');
   const requestedLimit = stringFlag(parsed, '--limit');
   const json = boolFlag(parsed, '--json');
   rejectDisplayFlagConflicts(full, requestedLimit, json);
-  const repo = resolveRepo(parsed, env);
   const service = await serviceFor(parsed, env);
   const page = await service.listReviews(repo, number, full);
   const showAll = full || json;
@@ -448,10 +442,7 @@ async function pullDiff(
     withFlags({ '--repo': 'value', '--full': 'boolean' }),
     'pr diff',
   );
-  const number = parsePullNumber(
-    requireOnePositional(parsed, 'pull request number'),
-  );
-  const repo = resolveRepo(parsed, env);
+  const { repo, number } = resolvePullTarget(parsed, env);
   const service = await serviceFor(parsed, env);
   const diff = await service.diffPull(repo, number);
   const showAll = boolFlag(parsed, '--full') || boolFlag(parsed, '--json');
@@ -532,10 +523,7 @@ async function pullUpdate(
     }),
     'pr update',
   );
-  const number = parsePullNumber(
-    requireOnePositional(parsed, 'pull request number'),
-  );
-  const repo = resolveRepo(parsed, env);
+  const { repo, number } = resolvePullTarget(parsed, env);
   const state = stringFlag(parsed, '--state');
   if (state !== undefined) stateFlag(state, false);
   const title = stringFlag(parsed, '--title');
@@ -611,10 +599,7 @@ async function pullMerge(
     }),
     'pr merge',
   );
-  const number = parsePullNumber(
-    requireOnePositional(parsed, 'pull request number'),
-  );
-  const repo = resolveRepo(parsed, env);
+  const { repo, number } = resolvePullTarget(parsed, env);
   const expectedHead = requireFlag(parsed, '--expected-head');
   const rawMethod = stringFlag(parsed, '--method') ?? 'merge';
   if (!['merge', 'squash', 'rebase'].includes(rawMethod)) {
@@ -887,11 +872,14 @@ async function contentHistoryRead(
   if (operation === 'soft-delete' && historyId === undefined)
     throw usageError('--history-id is required for soft-delete');
 
-  const number =
-    family === 'pr'
-      ? parsePullNumber(requireOnePositional(parsed, 'pull request number'))
-      : parseIssueNumber(requireOnePositional(parsed, 'issue number'));
-  const repo = resolveRepo(parsed, env);
+  let repo: RepositoryRef;
+  let number: number;
+  if (family === 'pr') {
+    ({ repo, number } = resolvePullTarget(parsed, env));
+  } else {
+    number = parseIssueNumber(requireOnePositional(parsed, 'issue number'));
+    repo = resolveRepo(parsed, env);
+  }
   const service = await serviceFor(parsed, env);
   if (operation === 'overview') {
     return { overview: await service.contentHistoryOverview(repo, number) };
@@ -1225,6 +1213,36 @@ function resolveRepo(
   return parseRepository(raw);
 }
 
+/**
+ * Resolve the repository and number for a pull request addressed by either
+ * form. A `--repo` that contradicts the URL is a usage error, because two
+ * explicit addresses disagreeing means one of them is wrong and guessing which
+ * would act on a repository the caller did not name. `FORGEJO_REPOSITORY` is
+ * ambient rather than explicit, so an explicit URL simply overrides it.
+ */
+function resolvePullTarget(
+  parsed: ParsedArgs,
+  env: NodeJS.ProcessEnv,
+): { repo: RepositoryRef; number: number } {
+  const target = parsePullTarget(
+    requireOnePositional(parsed, 'pull request number'),
+  );
+  if (target.repo === null) {
+    return { repo: resolveRepo(parsed, env), number: target.number };
+  }
+  const flagRepo = stringFlag(parsed, '--repo');
+  if (
+    flagRepo !== undefined &&
+    parseRepository(flagRepo).fullName !== target.repo.fullName
+  ) {
+    throw usageError(
+      `--repo ${flagRepo} contradicts the pull request URL (${target.repo.fullName})`,
+      ['Drop --repo, or pass the number instead of the URL'],
+    );
+  }
+  return { repo: target.repo, number: target.number };
+}
+
 function withFlags(flags: FlagSpec): FlagSpec {
   return { ...CONNECTION_FLAGS, ...flags };
 }

src/forgejo.ts

@@ -1794,6 +1794,63 @@ export function parsePullNumber(raw: string): number {
   return positiveInteger(raw, 'Pull request number');
 }
 
+/**
+ * A pull request addressed either by number, leaving the repository to
+ * `--repo`, or by a URL that carries the repository with it.
+ */
+export interface PullRequestTarget {
+  /** Null when the caller gave a bare number and the repository is elsewhere. */
+  repo: RepositoryRef | null;
+  number: number;
+}
+
+/**
+ * Accept the URL Forgejo prints for a pull request wherever a number is taken.
+ *
+ * A scheme is the discriminator, so anything without one still reaches
+ * `parsePullNumber` and fails with the same message it always did.
+ *
+ * The URL supplies the repository and the number and nothing else. Its host is
+ * deliberately ignored rather than adopted as the base URL: routing a
+ * credentialled request at whatever host appeared in an argument is the same
+ * hazard `CROSS_ORIGIN_REDIRECT` exists to refuse, and it would also break
+ * every host reached through a forward or a proxy, where the printed URL and
+ * the reachable one legitimately differ.
+ */
+export function parsePullTarget(raw: string): PullRequestTarget {
+  if (!/^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//.test(raw)) {
+    return { repo: null, number: parsePullNumber(raw) };
+  }
+  let url: URL;
+  try {
+    url = new URL(raw);
+  } catch {
+    throw usageError('Pull request URL is not a valid URL');
+  }
+  if (url.protocol !== 'http:' && url.protocol !== 'https:')
+    throw usageError('Pull request URL must use http or https');
+  if (url.username !== '' || url.password !== '')
+    throw usageError('Pull request URL must not carry credentials');
+
+  // Scanned from the end so a repository named `pulls` cannot capture the
+  // match, and anchored on the `pulls/N` pair so the web form, the
+  // `/api/v1/repos/` form, a host served under a path prefix, and a trailing
+  // `/files` or `/commits` sub-page all resolve to the same target.
+  const segments = url.pathname.split('/').filter((segment) => segment !== '');
+  for (let index = segments.length - 2; index >= 2; index -= 1) {
+    if (segments[index] !== 'pulls') continue;
+    const candidate = segments[index + 1] ?? '';
+    if (!/^[1-9]\d*$/.test(candidate)) continue;
+    return {
+      repo: parseRepository(`${segments[index - 2]}/${segments[index - 1]}`),
+      number: parsePullNumber(candidate),
+    };
+  }
+  throw usageError(`Pull request URL has no OWNER/REPO/pulls/NUMBER: ${raw}`, [
+    'Expected a URL like https://forgejo.example/owner/repo/pulls/42',
+  ]);
+}
+
 export function parseIssueNumber(raw: string): number {
   return positiveInteger(raw, 'Issue number');
 }