diff --git a/.claude/tasks/0002-issue-list-filters-and-fields.md b/.claude/tasks/0002-issue-list-filters-and-fields.md index 38c133a..40b26e9 100644 --- a/.claude/tasks/0002-issue-list-filters-and-fields.md +++ b/.claude/tasks/0002-issue-list-filters-and-fields.md @@ -13,9 +13,25 @@ Field selection: `--fields ` exposing the extra fields `body` (raw), `clo ## Acceptance criteria -- [ ] `--label`, `--assignee`, `--author`, and `--milestone` map to their Gitea API query params and filter server-side -- [ ] `--sort ` reorders descending client-side after full pagination; the count line still reports `T` from `X-Total-Count` -- [ ] `--fields` selects among the documented extra fields, each rendered via its FieldDef extractor (relative times, joined label names, milestone title) -- [ ] Output contains no `type` field -- [ ] `--search` fails with `VALIDATION_ERROR` (exit 2) and a help line pointing at `gitea-axi search issues ""` -- [ ] Fixture-server tests cover each filter, client-side sort with pagination, `--fields` extraction, and the forbidden `--search` +- [x] `--label`, `--assignee`, `--author`, and `--milestone` map to their Gitea API query params and filter server-side +- [x] `--sort ` reorders descending client-side after full pagination; the count line still reports `T` from `X-Total-Count` +- [x] `--fields` selects among the documented extra fields, each rendered via its FieldDef extractor (relative times, joined label names, milestone title) +- [x] Output contains no `type` field +- [x] `--search` fails with `VALIDATION_ERROR` (exit 2) and a help line pointing at `gitea-axi search issues ""` +- [x] Fixture-server tests cover each filter, client-side sort with pagination, `--fields` extraction, and the forbidden `--search` + +## Implementation Notes + +Exhaustive pagination landed as a shared `src/paginate.ts` (`fetchAllPages`, `readTotalCount`), since ADR 0005 makes it a policy that later slices (`pr list`, the dashboard) reuse rather than a detail of this command. +`lookup.ts`'s `listAllLabels` hand-rolled the same loop and now calls the shared helper, which removed its local `LABEL_PAGE_SIZE`/`LABEL_PAGE_LIMIT` constants. +The helper carries the 20-page cap those constants encoded, matching the 1000-item ceiling Principle 8 sets on exhaustive pagination — without it a server that ignores paging would loop forever. + +Two count-line details the acceptance criteria did not spell out. +Under `--sort`, `--limit` caps the *sorted* result rather than the fetch, so pages are always read at the full page size of 50 and the top `N` by the sort key is what the limit selects. +Also under `--sort`, when an instance omits `X-Total-Count`, the total falls back to the size of the fully paginated set instead of degrading to `count: N (showing first N)` — everything was fetched to sort it, so the total is known, and Principle 4 says a total is always reported. + +`--sort` and `--state` shared an enum-parsing shape, now extracted as `parseEnumFlag` in `flags.ts`. + +**Open question for the spec, deliberately not resolved here:** `--fields body` renders the body raw and untruncated, exactly as this task and the spec's command surface specify ("`body` (raw)"), matching the already-merged `issue create --fields body`. +This contradicts Principle 3 ("Body text is truncated at **500 characters** in all contexts (list and detail alike)"): a 30-row list with `--fields body` can now emit 30 full bodies, which is the cost Principle 3 exists to prevent. +Truncating here alone would make `issue list` disagree with `issue create`, so the conflict wants one ruling applied to both commands rather than a silent divergence in this slice. diff --git a/src/commands/issue.ts b/src/commands/issue.ts index 0a51db9..cc07409 100644 --- a/src/commands/issue.ts +++ b/src/commands/issue.ts @@ -15,8 +15,15 @@ import { selectExtraFields, type FieldDef, } from "../fields.js"; -import { flagValue, parseFlags, parsePositionalNumber } from "../flags.js"; +import { + flagValue, + parseEnumFlag, + parseFlags, + parsePositionalNumber, + splitFlag, +} from "../flags.js"; import { resolveLabelIds, resolveMilestoneId } from "../lookup.js"; +import { fetchAllPages, readTotalCount } from "../paginate.js"; import { formatCountLine, renderDetail, renderList, type DetailBlock } from "../render.js"; import { relativeTime } from "../time.js"; import { suggestCommand } from "../suggestions.js"; @@ -86,9 +93,15 @@ export const ISSUE_LIST_HELP = `usage: gitea-axi issue list [flags] List issues in the current repository. Pull requests are never included. flags: - --state Filter by state (default: open) - --limit Maximum number of issues to return (default: 30) - --help Show this help + --state Filter by state (default: open) + --label Filter by label name (comma-separated) + --assignee Filter by assignee + --author Filter by author + --milestone Filter by milestone name + --sort Sort descending (client-side) + --limit Maximum number of issues to return (default: 30) + --fields Append extra fields: body, closedAt, labels, milestone, updatedAt, url + --help Show this help global flags: -R, --repo Override the repository detected from the git origin remote @@ -103,27 +116,97 @@ const ISSUE_LIST_FIELDS: FieldDef[] = [ relativeTimeField("created", "created_at"), ]; +// Appended to the defaults on request via `--fields`, never replacing them. +const ISSUE_LIST_EXTRA_FIELDS: Record> = { + body: pluck("body"), + closedAt: relativeTimeField("closedAt", "closed_at"), + labels: joined("labels", "labels", "name"), + milestone: pluck("milestone", "milestone.title"), + updatedAt: relativeTimeField("updatedAt", "updated_at"), + url: pluck("url", "html_url"), +}; + const ISSUE_STATES = ["open", "closed", "all"] as const; type IssueState = (typeof ISSUE_STATES)[number]; +const ISSUE_SORTS = ["created", "updated", "comments"] as const; +type IssueSort = (typeof ISSUE_SORTS)[number]; + +/** Sort keys, read descending. A missing or unparseable value sorts last. */ +const ISSUE_SORT_KEYS: Record number> = { + created: (issue) => timestamp(issue.created_at), + updated: (issue) => timestamp(issue.updated_at), + comments: (issue) => issue.comments ?? 0, +}; + const DEFAULT_LIMIT = 30; const ISSUE_LIST_HELP_SUGGESTION = [ "Run `gitea-axi issue list --help` to see available flags", ]; +function timestamp(iso: string | undefined): number { + const value = Date.parse(iso ?? ""); + return Number.isNaN(value) ? 0 : value; +} + function parseState(value: string | true | undefined): IssueState { - if (value === undefined) { - return "open"; + return parseEnumFlag(value, "--state", ISSUE_STATES, ISSUE_LIST_HELP_SUGGESTION) ?? "open"; +} + +function parseSort(value: string | true | undefined): IssueSort | undefined { + return parseEnumFlag(value, "--sort", ISSUE_SORTS, ISSUE_LIST_HELP_SUGGESTION); +} + +/** + * Gitea's issue list has no sort parameter, so ordering happens here, over the + * fully paginated set (see ADR 0005). `sort` is stable, so equal keys keep the + * order the API returned them in. + */ +function sortIssues(issues: Issue[], sort: IssueSort): Issue[] { + const key = ISSUE_SORT_KEYS[sort]; + return [...issues].sort((a, b) => key(b) - key(a)); +} + +/** + * The filters Gitea's issue list accepts as query params, under its own names. + * All four filter server-side; none of them needs the client-side policy. + */ +function issueListFilters(flags: Record): Record { + const filters: Record = {}; + const label = flagValue(flags, "--label"); + if (label !== undefined) { + filters.labels = label; } - if (value === true || !ISSUE_STATES.includes(value as IssueState)) { - throw axiError( - `Invalid --state value: ${String(value)} (expected open, closed, or all)`, - "VALIDATION_ERROR", - ISSUE_LIST_HELP_SUGGESTION, - ); + const assignee = flagValue(flags, "--assignee"); + if (assignee !== undefined) { + filters.assigned_by = assignee; } - return value as IssueState; + const author = flagValue(flags, "--author"); + if (author !== undefined) { + filters.created_by = author; + } + const milestone = flagValue(flags, "--milestone"); + if (milestone !== undefined) { + filters.milestones = milestone; + } + return filters; +} + +/** + * `--search` is refused rather than quietly forwarded to the API's `q` param: + * full-text search is `search issues`, and a flag that half-worked here would be + * the wrong thing to learn. Checked ahead of `parseFlags` so every form of the + * flag — valued, inline, bare — lands on the redirect instead of a generic + * unknown-flag or missing-value error. + */ +function refuseSearchFlag(args: string[]): void { + if (!args.some((arg) => splitFlag(arg).name === "--search")) { + return; + } + throw axiError("issue list does not support --search", "VALIDATION_ERROR", [ + 'Use `gitea-axi search issues ""` for full-text search', + ]); } function parseLimit(value: string | true | undefined): number { @@ -168,9 +251,19 @@ async function issueList(deps: CliDeps, args: string[]): Promise { if (args.includes("--help")) { return ISSUE_LIST_HELP; } + refuseSearchFlag(args); const { flags, positionals } = parseFlags( args, - { "--state": { takesValue: true }, "--limit": { takesValue: true } }, + { + "--state": { takesValue: true }, + "--label": { takesValue: true }, + "--assignee": { takesValue: true }, + "--author": { takesValue: true }, + "--milestone": { takesValue: true }, + "--sort": { takesValue: true }, + "--limit": { takesValue: true }, + "--fields": { takesValue: true }, + }, "issue list", ); if (positionals.length > 0) { @@ -181,33 +274,58 @@ async function issueList(deps: CliDeps, args: string[]): Promise { ); } const state = parseState(flags["--state"]); + const sort = parseSort(flags["--sort"]); const limit = parseLimit(flags["--limit"]); + const extraFields = selectExtraFields( + flagValue(flags, "--fields"), + ISSUE_LIST_EXTRA_FIELDS, + "issue list", + ); + const query = { state, type: "issues" as const, ...issueListFilters(flags) }; const context = await resolveRepoContext(deps); const api = createClient(context); - let response; + let issues: Issue[]; + let total: number | undefined; try { - response = await api.repos.issueListIssues(context.owner, context.name, { - state, - type: "issues", - limit, - page: 1, - }); + if (sort === undefined) { + const response = await api.repos.issueListIssues(context.owner, context.name, { + ...query, + limit, + page: 1, + }); + issues = response.data ?? []; + total = readTotalCount(response.headers); + } else { + // Sorting client-side means holding the whole set first: the top `limit` + // by the sort key is only knowable once every page is in (see ADR 0005). + const result = await fetchAllPages((page, pageLimit) => + api.repos.issueListIssues(context.owner, context.name, { + ...query, + page, + limit: pageLimit, + }), + ); + issues = sortIssues(result.items, sort).slice(0, limit); + // Sorting reorders without changing membership, so the API's own total + // still describes this result set and the count line keeps reporting it. + // Having paginated everything, the set's own size is the fallback when an + // instance omits the header — a total is always reported (Principle 4). + total = result.total ?? result.items.length; + } } catch (error) { throw classifyHttpError(error); } - const issues = response.data ?? []; - const totalHeader = response.headers.get("x-total-count"); - const total = totalHeader !== null ? Number(totalHeader) : undefined; - const resolvedTotal = total !== undefined && Number.isFinite(total) ? total : undefined; const now = new Date(); - const rows = issues.map((issue) => extractRow(issue, ISSUE_LIST_FIELDS, { now })); + const rows = issues.map((issue) => + extractRow(issue, [...ISSUE_LIST_FIELDS, ...extraFields], { now }), + ); return renderList({ noun: "issues", rows, - countLine: formatCountLine(rows.length, resolvedTotal, rows.length >= limit), - help: issueListSuggestions(context, state, rows.length, resolvedTotal), + countLine: formatCountLine(rows.length, total, rows.length >= limit), + help: issueListSuggestions(context, state, rows.length, total), }); } diff --git a/src/flags.ts b/src/flags.ts index e2b3295..a12abd8 100644 --- a/src/flags.ts +++ b/src/flags.ts @@ -38,6 +38,39 @@ export function flagValue( return typeof value === "string" ? value : undefined; } +/** ["open", "closed", "all"] → "open, closed, or all". */ +function orList(values: readonly string[]): string { + if (values.length < 2) { + return values[0] ?? ""; + } + return `${values.slice(0, -1).join(", ")}, or ${values[values.length - 1]}`; +} + +/** + * Read a flag whose value must be one of a fixed set. Returns undefined when the + * flag was absent, leaving the default to the caller — a flag with no default + * (`--sort`) and one with a default (`--state`) then differ only in what they do + * with that undefined. + */ +export function parseEnumFlag( + value: string | true | undefined, + name: string, + allowed: readonly T[], + suggestions: string[], +): T | undefined { + if (value === undefined) { + return undefined; + } + if (value === true || !allowed.includes(value as T)) { + throw axiError( + `Invalid ${name} value: ${String(value)} (expected ${orList(allowed)})`, + "VALIDATION_ERROR", + suggestions, + ); + } + return value as T; +} + /** Split "--flag=value" into name and inline value; "--flag" has none. */ export function splitFlag(arg: string): SplitFlag { const equals = arg.indexOf("="); diff --git a/src/lookup.ts b/src/lookup.ts index b861571..5728632 100644 --- a/src/lookup.ts +++ b/src/lookup.ts @@ -2,36 +2,23 @@ import type { Label } from "gitea-js"; import type { GiteaClient } from "./client.js"; import type { RepoContext } from "./context.js"; import { axiError, classifyHttpError } from "./errors.js"; +import { fetchAllPages } from "./paginate.js"; /** * Name→ID resolution for the Gitea endpoints that only accept numeric ids. * Shared by every command that takes a `--label` or `--milestone` name. */ -const LABEL_PAGE_SIZE = 50; -/** Guard against an unbounded loop if a server ignores paging and always returns a full page. */ -const LABEL_PAGE_LIMIT = 20; - /** Fetch every label in the repository, paging until the API runs out. */ async function listAllLabels(api: GiteaClient, context: RepoContext): Promise { - const labels: Label[] = []; - for (let page = 1; page <= LABEL_PAGE_LIMIT; page++) { - let batch: Label[]; - try { - const response = await api.repos.issueListLabels(context.owner, context.name, { - page, - limit: LABEL_PAGE_SIZE, - }); - batch = response.data ?? []; - } catch (error) { - throw classifyHttpError(error); - } - labels.push(...batch); - if (batch.length < LABEL_PAGE_SIZE) { - break; - } + try { + const { items } = await fetchAllPages