diff --git a/AGENTS.md b/AGENTS.md index c577e80..30ac55e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,6 +11,7 @@ The CLI layer never knows which backend is active — it only talks to the `Stor - `src/context.ts` — `resolveTasksContext` builds the backend `Store` + `ResolvedConfig`; every command receives this `TasksContext`. - `src/store.ts` - the `Store` interface and `Capabilities`. Core contract: `create/get/update/remove/list/transition/addDep/removeDep/updatePublicFollowup`. `prune`/`render` are optional and capability-gated. - `src/model.ts` — the `Task` data model (report §5). +- `src/pr-url.ts` — `isPrUrl`, the one canonical PR-URL seam (GitHub `/pull/` on github.com, Forgejo `/pulls/` on any lowercase DNS host) shared by prose link derivation, `--pr` validation, and public-followup `pr_url`; near-misses derive as `doc` links, never `pr`. - `src/derive.ts` - worker `blocked` / `ready` / active `held` and public delivery readiness are derived in the CLI from `list` + the dep graph + hold date gates, never Store methods, so every backend gets them for free. - `src/backends/markdown*.ts` — the only P1 backend. - `src/public-followup.ts` - authoritative versioned schema, strict privacy-safe validation, canonical encoding, immutable-field checks, relation/event readiness, and terminal-state invariants for `kind=public-followup`; `src/commands/public-followup.ts` owns its dedicated CLI state machine. diff --git a/README.md b/README.md index 36c4ccc..a9221b3 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,7 @@ tasks-axi add lavish-foo-q9 "fix summary toggle" --kind ship --repo lavish-axi - # move through the workflow tasks-axi start firstmate-lease-adopt tasks-axi done sm-idle-handoff-q8 --pr https://github.com/owner/repo/pull/42 +tasks-axi done fj-task-q1 --pr https://forgejo.example.com/owner/repo/pulls/39 tasks-axi reopen some-task # dependencies, holds, and the ready queue diff --git a/src/backends/markdown-grammar.ts b/src/backends/markdown-grammar.ts index 22ec5d9..ef82a01 100644 --- a/src/backends/markdown-grammar.ts +++ b/src/backends/markdown-grammar.ts @@ -1,6 +1,7 @@ import { AxiError } from "../errors.js"; import type { Dep, Hold, HoldKind, State, Task, TaskLink } from "../model.js"; import { HOLD_KINDS } from "../model.js"; +import { isPrUrl } from "../pr-url.js"; import { PUBLIC_FOLLOWUP_KIND, assertPublicFollowupTaskState, @@ -125,7 +126,6 @@ const TAIL_HOLD_KIND = new RegExp( ); const TAIL_HOLD_UNTIL = new RegExp(`\\s*\\(hold-until:\\s*(${DATE})\\)\\s*$`); -const PR_LINK = /https?:\/\/\S+?\/pull\/\d+/g; const REPORT_LINK = /\bdata\/\S+?\/report\.md\b/g; const GENERIC_URL = /https?:\/\/\S+/g; @@ -162,10 +162,12 @@ export function deriveLinks(text: string): TaskLink[] { seen.add(url); links.push({ kind, url }); }; - for (const m of text.matchAll(PR_LINK)) add("pr", m[0]); + for (const m of text.matchAll(GENERIC_URL)) { + if (isPrUrl(trimUrl(m[0]))) add("pr", m[0]); + } for (const m of text.matchAll(REPORT_LINK)) add("report", m[0]); for (const m of text.matchAll(GENERIC_URL)) { - if (!/\/pull\/\d+/.test(m[0])) add("doc", m[0]); + if (!isPrUrl(trimUrl(m[0]))) add("doc", m[0]); } return links; } diff --git a/src/backends/markdown.ts b/src/backends/markdown.ts index aa9e612..cd1adfc 100644 --- a/src/backends/markdown.ts +++ b/src/backends/markdown.ts @@ -22,6 +22,7 @@ import type { TransitionOpts, } from "../model.js"; import { HOLD_KINDS } from "../model.js"; +import { PR_URL_EXPECTED } from "../pr-url.js"; import { PUBLIC_FOLLOWUP_KIND, assertPublicFollowupMutation, @@ -140,7 +141,9 @@ function normalizeLinkUrl(url: string): string { } function normalizeTypedLink(link: TaskLink): TaskLink { - const url = normalizeLinkUrl(link.url); + const normalized = normalizeLinkUrl(link.url); + // pr URLs must already be canonical; padded input is rejected, not trimmed + const url = link.kind === "pr" ? link.url : normalized; const derived = deriveLinks(url); if ( !derived.some( @@ -149,7 +152,7 @@ function normalizeTypedLink(link: TaskLink): TaskLink { ) { const expected = link.kind === "pr" - ? "an http(s) pull request URL ending in /pull/" + ? PR_URL_EXPECTED : link.kind === "report" ? "a data//report.md path" : "an http(s) URL"; diff --git a/src/commands/crud.ts b/src/commands/crud.ts index 8bcb36f..d075976 100644 --- a/src/commands/crud.ts +++ b/src/commands/crud.ts @@ -10,6 +10,7 @@ import { } from "../args.js"; import { takeBody } from "../body.js"; import { deriveLinks, extractTags } from "../backends/markdown-grammar.js"; +import { PR_URL_EXPECTED } from "../pr-url.js"; import { renderMutation, stateLabel, taskToJson } from "../confirm.js"; import { requireCtx, type TasksContext } from "../context.js"; import { blockedIds, heldTasks } from "../derive.js"; @@ -140,14 +141,13 @@ function requireTypedLinkUrl( `Pass ${flag}=... without line breaks`, ]); } - const url = checked.trim(); + // pr URLs must already be canonical; padded input is rejected, not trimmed + const url = kind === "pr" ? checked : checked.trim(); if ( !deriveLinks(url).some((link) => link.kind === kind && link.url === url) ) { const expected = - kind === "pr" - ? "an http(s) pull request URL ending in /pull/" - : "a data//report.md path"; + kind === "pr" ? PR_URL_EXPECTED : "a data//report.md path"; throw new AxiError(`${flag} must be ${expected}`, "VALIDATION_ERROR"); } return url; diff --git a/src/commands/state.ts b/src/commands/state.ts index 5126f22..d3ab086 100644 --- a/src/commands/state.ts +++ b/src/commands/state.ts @@ -57,6 +57,7 @@ flags: --json print the resulting task as a JSON object examples: tasks-axi done sm-idle-handoff-q8 --pr https://github.com/o/r/pull/42 + tasks-axi done fj-task-q1 --pr https://forgejo.example.com/o/r/pulls/39 tasks-axi done pr31-review-r6 --report data/pr31-review-r6/report.md`; export const REOPEN_HELP = `usage: tasks-axi reopen diff --git a/src/pr-url.ts b/src/pr-url.ts new file mode 100644 index 0000000..ac68948 --- /dev/null +++ b/src/pr-url.ts @@ -0,0 +1,32 @@ +/** + * Canonical pull request URL classification - the single seam shared by prose + * link derivation (deriveLinks), typed-link validation (`done --pr`, `add --pr`, + * backend normalization), and public-followup `pr_url` deliverables. + * + * Exactly two byte-for-byte shapes are PR URLs: + * - GitHub: https://github.com///pull/ + * - Forgejo: https://///pulls/ + * with a positive number without leading zeros. Anything else - issue URLs, + * singular/plural route confusion, trailing slash, query/fragment, whitespace, + * userinfo, ports, encoded separators - is not a PR URL. + */ + +const HOST_LABEL = "[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?"; +const SEGMENT = "[A-Za-z0-9._-]+"; +const PR_URL_RE = new RegExp( + `^https://(${HOST_LABEL}(?:\\.${HOST_LABEL})*)/(${SEGMENT})/(${SEGMENT})/(pull|pulls)/([1-9][0-9]*)$`, +); + +export const PR_URL_EXPECTED = + "a canonical pull request URL: https://github.com///pull/ (GitHub) or https://///pulls/ (Forgejo)"; + +/** True when url is byte-for-byte a canonical GitHub or Forgejo PR URL. */ +export function isPrUrl(url: string): boolean { + const m = PR_URL_RE.exec(url); + if (m === null) return false; + const [, host, owner, repo, route] = m; + if (owner === "." || owner === ".." || repo === "." || repo === "..") { + return false; + } + return route === "pull" ? host === "github.com" : host !== "github.com"; +} diff --git a/src/public-followup.ts b/src/public-followup.ts index ca205d8..cbd3b3a 100644 --- a/src/public-followup.ts +++ b/src/public-followup.ts @@ -1,4 +1,5 @@ import { AxiError } from "./errors.js"; +import { isPrUrl } from "./pr-url.js"; export const PUBLIC_FOLLOWUP_KIND = "public-followup"; export const PUBLIC_FOLLOWUP_SCHEMA_VERSION = 1 as const; @@ -209,7 +210,6 @@ const SHA256_RE = /^[a-f0-9]{64}$/; const DELIVERABLE_NAME_RE = /^[a-z][a-z0-9_]{0,63}$/; const SAFE_CODE_RE = /^[a-z][a-z0-9._-]{0,63}$/; const PROJECT_RE = /^[A-Za-z0-9][A-Za-z0-9._/-]{0,79}$/; -const PR_URL_RE = /^https:\/\/[^?#\s]+\/pull\/\d+$/; const REPORT_PATH_RE = /^data\/[A-Za-z0-9][A-Za-z0-9._-]*\/report\.md$/; const COMMIT_SHA_RE = /^[a-f0-9]{7,64}$/; const MAX_ENCODED_METADATA_LENGTH = 1_000_000; @@ -1367,15 +1367,7 @@ function deliverablesAreSafeForExpected( return false; } for (const [name, value] of Object.entries(deliverables)) { - if (name === "pr_url") { - if (!PR_URL_RE.test(value)) return false; - try { - const url = new URL(value); - if (url.username !== "" || url.password !== "") return false; - } catch { - return false; - } - } + if (name === "pr_url" && !isPrUrl(value)) return false; if (name === "report_path" && !REPORT_PATH_RE.test(value)) return false; if (name === "commit_sha" && !COMMIT_SHA_RE.test(value)) return false; if (name === "error_code" && !SAFE_CODE_RE.test(value)) return false; diff --git a/test/backends/markdown-grammar.test.ts b/test/backends/markdown-grammar.test.ts index 7fe8471..5989fe3 100644 --- a/test/backends/markdown-grammar.test.ts +++ b/test/backends/markdown-grammar.test.ts @@ -326,6 +326,42 @@ describe("markdown grammar", () => { }); expect(links).toContainEqual({ kind: "report", url: "data/x/report.md" }); }); + + it("derives a Forgejo pulls URL as the single pr link", () => { + const links = deriveLinks( + "merged https://forgejo.samesies.gay/eve/orchalycious/pulls/39", + ); + expect(links).toEqual([ + { + kind: "pr", + url: "https://forgejo.samesies.gay/eve/orchalycious/pulls/39", + }, + ]); + }); + + it("keeps non-canonical PR-ish URLs as doc links, never pr", () => { + for (const url of [ + "https://forgejo.samesies.gay/eve/orchalycious/pull/39", + "https://github.com/o/r/pulls/42", + "https://forgejo.samesies.gay/o/r/pulls/39?tab=files", + ]) { + expect(deriveLinks(`see ${url}`)).toEqual([{ kind: "doc", url }]); + } + }); + + it("round-trips a done bullet with a Forgejo pull URL byte-exactly", () => { + const src = + "## Queued\n\n## Done\n- [x] fj-done-q1 - merged https://forgejo.samesies.gay/eve/orchalycious/pulls/39 (merged 2026-08-07)\n"; + const doc = parseBacklog(src); + const task = tasksOf(doc)[0]; + expect(task.links).toEqual([ + { + kind: "pr", + url: "https://forgejo.samesies.gay/eve/orchalycious/pulls/39", + }, + ]); + expect(renderBacklog(doc)).toBe(src); + }); }); describe("canonical render", () => { diff --git a/test/backends/markdown.test.ts b/test/backends/markdown.test.ts index cfd06c6..3ff1b3a 100644 --- a/test/backends/markdown.test.ts +++ b/test/backends/markdown.test.ts @@ -530,6 +530,11 @@ describe("MarkdownStore", () => { addLinks: [{ kind: "pr", url: "https://github.com/o/r/issues/9" }], }), ).rejects.toMatchObject({ code: "VALIDATION_ERROR" }); + await expect( + b.store.update("cert-cleanup", { + addLinks: [{ kind: "pr", url: " https://github.com/o/r/pull/9 " }], + }), + ).rejects.toMatchObject({ code: "VALIDATION_ERROR" }); await expect( b.store.update("cert-cleanup", { addLinks: [{ kind: "report", url: "reports/cert/report.md" }], diff --git a/test/commands/public-followup.test.ts b/test/commands/public-followup.test.ts index f47a5d0..075fddb 100644 --- a/test/commands/public-followup.test.ts +++ b/test/commands/public-followup.test.ts @@ -361,6 +361,48 @@ describe("public-followup commands", () => { } }); + it("accepts a Forgejo pulls URL as a pr-merged deliverable", async () => { + const b = makeBacklog(EMPTY); + try { + await add(b); + await bind(b); + const landed = await acceptEvent( + b, + event("evt-forgejo-url", "rel-code", "work-code-q1", 1, { + deliverables: { + pr_url: "https://forgejo.samesies.gay/eve/orchalycious/pulls/39", + }, + }), + ); + expect( + landed.task.public_followup.work_relations[0].accepted_events[0] + .deliverables.pr_url, + ).toBe("https://forgejo.samesies.gay/eve/orchalycious/pulls/39"); + } finally { + b.cleanup(); + } + }); + + it("rejects a singular-route Forgejo PR URL before accepting work", async () => { + const b = makeBacklog(EMPTY); + try { + await add(b); + await bind(b); + await expect( + acceptEvent( + b, + event("evt-singular-url", "rel-code", "work-code-q1", 1, { + deliverables: { + pr_url: "https://forgejo.samesies.gay/eve/orchalycious/pull/39", + }, + }), + ), + ).rejects.toMatchObject({ code: "VALIDATION_ERROR" }); + } finally { + b.cleanup(); + } + }); + it("keeps a failed required relation actionable unless failure is expected", async () => { const b = makeBacklog(EMPTY); try { diff --git a/test/commands/state.test.ts b/test/commands/state.test.ts index 7744c5c..f311fff 100644 --- a/test/commands/state.test.ts +++ b/test/commands/state.test.ts @@ -125,6 +125,50 @@ describe("state commands", () => { } }); + it("closes with a Forgejo pulls URL and preserves it byte-for-byte", async () => { + const b = makeBacklog(); + try { + const out = await doneCommand( + [ + "cert-cleanup", + "--pr", + "https://forgejo.samesies.gay/eve/orchalycious/pulls/39", + "--no-prune", + ], + b.ctx, + ); + expect(out).toContain( + "done cert-cleanup -> Done (pr https://forgejo.samesies.gay/eve/orchalycious/pulls/39)", + ); + const read = b.read(); + expect(read).toContain( + "https://forgejo.samesies.gay/eve/orchalycious/pulls/39", + ); + expect(read).toContain("(merged 2026-07-01)"); + } finally { + b.cleanup(); + } + }); + + it("rejects non-canonical pull URLs without mutating", async () => { + const b = makeBacklog(); + try { + for (const url of [ + "https://forgejo.samesies.gay/eve/orchalycious/pull/39", + "https://github.com/o/r/pulls/9", + "https://github.com/o/r/pull/9?w=1", + " https://github.com/o/r/pull/9 ", + ]) { + await expect( + doneCommand(["cert-cleanup", "--pr", url, "--no-prune"], b.ctx), + ).rejects.toMatchObject({ code: "VALIDATION_ERROR" }); + } + expect(b.read()).toContain("- [ ] cert-cleanup"); + } finally { + b.cleanup(); + } + }); + it("emits a machine-readable task and pruned count with --json", async () => { const b = makeBacklog(); try { diff --git a/test/pr-url.test.ts b/test/pr-url.test.ts new file mode 100644 index 0000000..1a3b21c --- /dev/null +++ b/test/pr-url.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vitest"; + +import { isPrUrl } from "../src/pr-url.js"; + +describe("isPrUrl", () => { + it.each([ + "https://github.com/o/r/pull/42", + "https://github.com/some-owner/some.repo/pull/1", + "https://forgejo.samesies.gay/eve/orchalycious/pulls/39", + "https://codeberg.org/forgejo/forgejo/pulls/1234", + ])("accepts canonical PR URL %s", (url) => { + expect(isPrUrl(url)).toBe(true); + }); + + it.each([ + // singular/plural route confusion + "https://github.com/o/r/pulls/42", + "https://forgejo.samesies.gay/eve/orchalycious/pull/39", + // issue URLs + "https://github.com/o/r/issues/42", + "https://forgejo.samesies.gay/o/r/issues/42", + // number shape + "https://github.com/o/r/pull/0", + "https://github.com/o/r/pull/042", + "https://github.com/o/r/pull/42abc", + "https://forgejo.samesies.gay/o/r/pulls/0", + // scheme / decoration + "http://github.com/o/r/pull/42", + "https://github.com/o/r/pull/42/", + "https://github.com/o/r/pull/42?w=1", + "https://github.com/o/r/pull/42#top", + " https://github.com/o/r/pull/42", + "https://github.com/o/r/pull/42\n", + "https://github.com/o/r/pull/4\u00002", + "https://github.com/o/r/pull/4 2", + // authority shape + "https://user@forgejo.samesies.gay/o/r/pulls/39", + "https://token:secret@github.com/o/r/pull/519", + "https://forgejo.samesies.gay:8443/o/r/pulls/39", + "https://Forgejo.Samesies.Gay/o/r/pulls/39", + "https://-bad-.example.com/o/r/pulls/39", + // path shape + "https://forgejo.samesies.gay/o/r/extra/pulls/39", + "https://forgejo.samesies.gay/r/pulls/39", + "https://forgejo.samesies.gay//r/pulls/39", + "https://forgejo.samesies.gay/o%2Fx/r/pulls/39", + "https://forgejo.samesies.gay/../r/pulls/39", + "https://forgejo.samesies.gay/o/../pulls/39", + ])("rejects non-canonical URL %j", (url) => { + expect(isPrUrl(url)).toBe(false); + }); +});