Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ Additive fields are permitted. Nullable fields are emitted as `null`, not omitte
- `repo view`: `{repository:{full_name,url,api_url,description,private,archived,default_branch,has_actions,has_pull_requests,open_pull_requests}}`.
- `api` (single request): `{status,data}`. Paginated `api`: `{data,page_info,next?}`.
- `pr find`: `{found,pull_request,search_info:{complete,pages,fetched,total}}`; `pull_request` is an identity or `null`.
- `pr list`: `{pull_requests,page_info,next?}`. Default rows are `{number,title,state,head}`; `--fields` selects other identity fields and `--fields all` selects all of them.
- `pr list`: `{pull_requests,page_info,next?,field_info?}`. Default rows are `{number,title,state,head}`; `--fields` selects other identity fields and `--fields all` selects every field from the list route, never the derived fields. The opt-in derived fields are `checks_state`, `checks_passes`, and `review_decision`. Naming any of them adds `field_info:{per_row_fields,rows_fetched,failures:[{number,field,reason}]}`; each failure names the exact requested field emitted as `null`. Enrichment is limited to displayed TOON rows, while `--full` and JSON enrich every fetched row.
- `pr view`: `{pull_request:{...identity,body,body_length,body_truncated,edit_history_count},next?}`. `body` is a 500-Unicode-code-point preview by default and complete with `--full`; `body_length` is measured in Unicode code points. `edit_history_count` and the runnable `next` hint are present only when content history exists. The lookup is best-effort and never fails the view: a host without the content-history routes, or one that refuses or fails to answer them, returns the same document without those fields. `--fields` narrows only `pull_request` under the detail-field rules above.
- `pr history` and `issue history`: `overview` returns `{overview:{counts:[{comment_id,count}],total}}`; `list` returns `{comment_id,revisions:[{history_id,summary}]}` newest first; `detail` returns `{revision:{history_id,previous_history_id,can_soft_delete,before,after,diff_html?}}`; `soft-delete` returns `{deleted,already_deleted?,comment_id,history_id,message?}`. The default operation is `list` and the default comment id is `0`, the body. `before` and `after` are reconstructed from Forgejo's diff HTML with `gd` omitted from `after` and `gi` omitted from `before`; raw HTML is included only with `--raw`. Soft-delete requires `--yes`, never prompts, checks `can_soft_delete`, and exits `0` for an already-deleted no-op; a host that withholds that permission is refused with `CONTENT_HISTORY_DELETE_REFUSED` before anything is posted. A host too old to serve the routes reports `CONTENT_HISTORY_UNSUPPORTED`; a repository the web root itself cannot read reports `CONTENT_HISTORY_AUTHORIZATION`, since that interface authenticates by session rather than by API token and answers for a private repository with the same 404. A revision that does not exist on a host that does serve the routes is `CONTENT_HISTORY_NOT_FOUND`, and diff HTML that cannot be reconstructed is `CONTENT_HISTORY_MALFORMED_DIFF` rather than a silently partial `before` or `after`.
- `pr create`: `{created,updated,pull_request}`. `pr update`: `{updated,pull_request}`. Existing desired state is exit-0 and mutation-free. Creation refuses with `PAGINATION_INCOMPLETE` if either its initial duplicate search or its post-conflict race-recovery search reaches the pagination ceiling without finding the pull request.
Expand Down Expand Up @@ -135,6 +135,8 @@ Merged-state proof always has `{merged, number, url, head_sha, merge_commit_sha,

A review identity is `{id, api_url, user, team, state, stale, official, dismissed, commit_id, submitted_at, updated_at, body, body_length, body_truncated, comments}`. `api_url` is constructed from the configured canonical base URL and repository identity, not trusted response links. `user` names the reviewer; a review requested from a team names the team in `team` and leaves `user` `null`, so a record is never left with nobody on it. `state` carries Forgejo's verdict verbatim. The values Forgejo defines today are `APPROVED`, `REQUEST_CHANGES`, `COMMENT`, `PENDING`, and `REQUEST_REVIEW`; a Forgejo release that adds another passes it through rather than discarding it, so a consumer must tolerate a value outside that list. `state` is `null` only when Forgejo reports no verdict at all, which it signals with an empty string rather than by omitting the field. `submitted_at` is `null` when Forgejo has recorded no submission time, which is how a review requesting a reviewer is reported, and `updated_at` reports the last change to the record, including a dismissal. `body` follows the same preview rules as `pr view`, and `--full` expands review and comment bodies as well as displaying every review.

`review_decision` is `changes_requested|approved|stale|review_requested|commented|none`. Only each reviewer's latest non-dismissed approval or change request is decisive; change requests from different reviewers outrank approvals. A current verdict attached only to an older commit is `stale`. If review pagination is incomplete, the field is `null` and `field_info.failures` names `review_decision` rather than reporting a decision from partial history.

A review comment identity is `{id, api_url, path, position, original_position, commit_id, original_commit_id, diff_hunk, diff_hunk_length, diff_hunk_truncated, user, resolved_by, created_at, updated_at, body, body_length, body_truncated}`. `path` names the file the comment marks. A comment on a line the change adds is anchored by `position` and `commit_id`; a comment on a line the change removes is anchored by `original_position` and `original_commit_id` instead, so a consumer locating a comment reads whichever pair is non-null. Forgejo reports an anchor it does not have as an empty string or a zero rather than by omitting the field, and both are normalized to `null` here so that a missing anchor cannot be read as line `0`. `resolved_by` is a username or `null`. `diff_hunk` is the excerpt Forgejo anchors the comment to; it observes the same preview ceiling as a body, so the capped view cannot emit an unbounded hunk and `--full` returns it whole. `diff_hunk_length` and `diff_hunk_truncated` measure it the way `body_length` and `body_truncated` measure a body — the length in Unicode code points, reported whole in both views — so a capped hunk is never mistaken for a whole one. A review reporting no inline comments is not queried for them, so `comments: []` costs no request.

Reviews are read-only. Submitting, dismissing, and deleting reviews are reachable only through `api`, so no review write path exists in the command surface.
Expand Down
17 changes: 16 additions & 1 deletion skills/forgejo-axi/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,26 @@ Usage:
forgejo-axi pr list --repo OWNER/REPO [--state open|closed|all] [--limit N|--full] [--fields LIST|all] [connection flags]

Flags:
--fields LIST Comma-separated identity fields; defaults to number,title,state,head
--fields LIST Comma-separated fields; defaults to number,title,state,head

Three fields are not on Forgejo's list route and cost extra requests per
displayed row, so they are fetched only when named:
checks_state Commit-status state for the head, as `pr checks` reports it
checks_passes Whether those checks pass, required contexts included
review_decision changes_requested|approved|stale|review_requested|commented|none

`all` expands to the fields the list route itself returns and never to
these three, so an existing --fields all call keeps costing one request.

When any of them is requested the response carries field_info with the rows
fetched and any row whose fetch failed. A failed row's field is null, which
means unknown and never a state. Only displayed rows are fetched, so --limit
and the default display cap bound the cost; --full and --json do not.

Examples:
forgejo-axi pr list --repo owner/repo
forgejo-axi pr list --repo owner/repo --state all --full --fields all
forgejo-axi pr list --repo owner/repo --fields number,checks_passes,review_decision
```

#### pr view
Expand Down
79 changes: 72 additions & 7 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
type IssueInput,
type LabelInput,
type PullRequestIdentity,
type PullRequestListRow,
type RepositoryRef,
type RunIdentity,
} from './forgejo.js';
Expand Down Expand Up @@ -353,15 +354,61 @@ async function pullList(
const page = await service.listPulls(repo, state);
const limit = displayLimit(requestedLimit);
const showAll = full || json;
const fields = chooseFields<PullRequestIdentity>(
const fields = chooseFields<PullRequestListRow>(
stringFlag(parsed, '--fields'),
PULL_IDENTITY_FIELDS,
PULL_LIST_FIELDS,
DEFAULT_PULL_LIST_FIELDS,
PULL_IDENTITY_FIELDS,
);
const displayed = (showAll ? page.items : page.items.slice(0, limit)).map(
(pull) => selectFields(pull, fields),
const want = {
checksState: fields.includes('checks_state'),
checksPasses: fields.includes('checks_passes'),
reviews: fields.includes('review_decision'),
};

// Enrichment is per row, so it is done for the rows that will be displayed
// and no others. Fetching it for rows the caller asked not to see would be
// paid for and thrown away.
const visible = showAll ? page.items : page.items.slice(0, limit);
const rows: PullRequestListRow[] = [];
const failures: Array<{ number: number; field: string; reason: string }> = [];
if (!want.checksState && !want.checksPasses && !want.reviews) {
rows.push(...visible);
} else {
const batchSize = showAll ? 4 : 1;
for (let start = 0; start < visible.length; start += batchSize) {
const enriched = await Promise.all(
visible.slice(start, start + batchSize).map(async (pull) => ({
pull,
derived: await service.derivedPullFields(repo, pull, want),
})),
);
for (const { pull, derived } of enriched) {
failures.push(...derived.failures);
rows.push({ ...pull, ...derived.values });
}
}
}

const output = listOutput(
'pull_requests',
rows.map((row) => selectFields(row, fields)),
page,
showAll,
);
return listOutput('pull_requests', displayed, page, showAll);
if (want.checksState || want.checksPasses || want.reviews) {
// The multiplier is the caller's to see: these fields cost requests per
// row, and a list that hid that would make a slow call look like a cheap
// one.
output['field_info'] = {
per_row_fields: fields.filter((field) =>
DERIVED_PULL_LIST_FIELDS.includes(field),
),
rows_fetched: rows.length,
failures,
};
}
return output;
}

async function pullRead(
Expand Down Expand Up @@ -1270,7 +1317,20 @@ const PULL_IDENTITY_FIELDS: ReadonlyArray<keyof PullRequestIdentity> = [
'merged_at',
'merged_by',
];
const DEFAULT_PULL_LIST_FIELDS: ReadonlyArray<keyof PullRequestIdentity> = [
/**
* The `pr list` fields that are not on the list route's own response and cost
* a request per displayed row to fill in.
*/
const DERIVED_PULL_LIST_FIELDS: ReadonlyArray<keyof PullRequestListRow> = [
'checks_state',
'checks_passes',
'review_decision',
];
const PULL_LIST_FIELDS: ReadonlyArray<keyof PullRequestListRow> = [
...PULL_IDENTITY_FIELDS,
...DERIVED_PULL_LIST_FIELDS,
];
const DEFAULT_PULL_LIST_FIELDS: ReadonlyArray<keyof PullRequestListRow> = [
'number',
'title',
'state',
Expand Down Expand Up @@ -1363,9 +1423,14 @@ function chooseFields<T extends object>(
raw: string | undefined,
all: ReadonlyArray<keyof T & string>,
defaults: ReadonlyArray<keyof T & string>,
// What `all` expands to, when that is not the whole valid set. `pr list`
// keeps its per-row fields out of the expansion: they are valid names, but
// letting `all` pull them in would turn an existing one-request call into
// one request per row without the caller asking for it.
allExpansion: ReadonlyArray<keyof T & string> = all,
): ReadonlyArray<keyof T & string> {
if (raw === undefined) return defaults;
if (raw === 'all') return all;
if (raw === 'all') return allExpansion;
const fields = raw.split(',');
if (
fields.some(
Expand Down
110 changes: 109 additions & 1 deletion src/forgejo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import {
} from './checks.js';
import { appendPath, type ConnectionConfig } from './config.js';
import { positiveInteger } from './args.js';
import { ForgejoAxiError, usageError } from './errors.js';
import { asForgejoError, ForgejoAxiError, usageError } from './errors.js';
import {
ForgejoHttpClient,
redact,
type Paginated,
type HttpResponse,
type RequestInput,
} from './http.js';
import { evaluateReviewDecision, type ReviewDecision } from './reviews.js';

interface ApiUser {
login?: string;
Expand Down Expand Up @@ -193,6 +194,20 @@ export interface PullRequestIdentity {
merged_by: string | null;
}

/**
* The `pr list` fields Forgejo's list route does not carry, each fetched per
* row only when asked for. Null means the fetch for that row failed and the
* value is unknown, which is never the same as a definite state.
*/
export interface DerivedPullFields {
checks_state?: string | null;
checks_passes?: boolean | null;
review_decision?: ReviewDecision | null;
}

export interface PullRequestListRow
extends PullRequestIdentity, DerivedPullFields {}

export interface LabelIdentity {
id: number;
name: string;
Expand Down Expand Up @@ -1618,6 +1633,99 @@ export class ForgejoService {
};
}

/**
* The verdict of a pull request's reviews, without their inline comments.
*
* `listReviews` costs one further round trip per commented review because it
* returns those comments. A list only needs the verdict, so this reads the
* review pages and stops there without fetching comments.
*/
async reviewDecision(
repo: RepositoryRef,
number: number,
): Promise<ReviewDecision> {
const page = await this.http.paginate<ApiPullReview>(
`${repoPath(repo)}/pulls/${number}/reviews`,
);
if (!page.complete) {
throw new ForgejoAxiError(
'Pull request review search reached the pagination safety ceiling',
'PAGINATION_INCOMPLETE',
{ details: { pages: page.pages, fetched: page.items.length } },
);
}
return evaluateReviewDecision(
page.items.map((review) => ({
id: review.id ?? null,
reviewer: review.user?.login ?? null,
submitted_at: timestampOrNull(review.submitted_at),
state: review.state ?? null,
dismissed: review.dismissed ?? false,
stale: review.stale ?? false,
})),
);
}

/**
* The per-row fields `pr list` cannot get from the list route, fetched only
* for the rows and fields actually asked for.
*
* A row whose own fetch fails yields null for that field and a recorded
* reason, rather than failing the whole list or -- worse -- reporting an
* unread field as if it had been read.
*/
async derivedPullFields(
repo: RepositoryRef,
pull: PullRequestIdentity,
want: { checksState: boolean; checksPasses: boolean; reviews: boolean },
): Promise<{
values: DerivedPullFields;
failures: Array<{ number: number; field: string; reason: string }>;
}> {
const values: DerivedPullFields = {};
const failures: Array<{
number: number;
field: string;
reason: string;
}> = [];
if (want.checksState || want.checksPasses) {
try {
const checks = await this.checksForPull(repo, pull);
values.checks_state = checks.state;
values.checks_passes = checks.passes;
} catch (error) {
values.checks_state = null;
values.checks_passes = null;
const reason = asForgejoError(error).message;
if (want.checksState)
failures.push({
number: pull.number,
field: 'checks_state',
reason,
});
if (want.checksPasses)
failures.push({
number: pull.number,
field: 'checks_passes',
reason,
});
}
}
if (want.reviews) {
try {
values.review_decision = await this.reviewDecision(repo, pull.number);
} catch (error) {
values.review_decision = null;
failures.push({
number: pull.number,
field: 'review_decision',
reason: asForgejoError(error).message,
});
}
}
return { values, failures };
}

private async checksForPull(
repo: RepositoryRef,
pull: PullRequestIdentity,
Expand Down
Loading
Loading