Review on diff lines and explain merge blockers before merging #283

Merged

stephen merged 5 commits from feat/spec-parity-pr into feat/spec-parity-issues

2026-10-08 20:22:51 +00:00

stephen commented

2026-10-08 19:18:55 +00:00

Owner

Two pull-requests requirements of the fjord-client-spec (#213).

Inline review comments. `fj pr review N --line-comment

` is repeatable, and `--line-comments-file` takes the same as a JSON array. LINE is `N` or `new:N` for the PR's version of the file and `old:N` for a removed line. Comments given with --event are batched into that review in one request; without --event they post as a plain comment review. Each anchor is checked against the PR diff first, so a line that is not commentable is refused locally with the commentable lines named. Only single-line anchors are supported: range fields are rejected, which is also what Gitea needs.

--event comment with nothing to say and --event request-changes with
no body now fail locally instead of as a server 422.

Merge preflight. fj pr merge used to explain a blocked merge only
after the server refused it, and through the admin-only branch
protection endpoint. It now reads the base branch's effective protection
from GET /branches/{branch}, which needs read access only, before the
POST. A certain blocker (a required check failing, pending or not
reported, a missing required approval, no merge permission) stops the
merge with the reason and, for checks still running, a pointer to
--auto. Evidence that could not be read stays "unknown": it never blocks
and never reads as passing, and the server decides.

fj pr checks shows the same verdict, gains a REQUIRED column, and
lists required checks that have not reported. Its --json is additive:
required per status, statuses_complete and merge_preflight.

Covered by unit and wiremock tests only; the preflight was not run against a live protected branch. fj pr checks gains a REQUIRED column, which shifts the table for anything parsing it.

Part 6 of 8 in the spec-parity stack. Based on feat/spec-parity-org, so the diff here is this slice alone; it needs a rebase onto main once the PR below it is squash-merged.

Closes #213

Verified on macOS at this commit: cargo fmt --all clean, cargo clippy --all-targets --all-features -- -D warnings clean, cargo test --all green.

Two pull-requests requirements of the fjord-client-spec (#213). Inline review comments. `fj pr review N --line-comment <PATH> <LINE> <BODY>` is repeatable, and `--line-comments-file` takes the same as a JSON array. LINE is `N` or `new:N` for the PR's version of the file and `old:N` for a removed line. Comments given with --event are batched into that review in one request; without --event they post as a plain comment review. Each anchor is checked against the PR diff first, so a line that is not commentable is refused locally with the commentable lines named. Only single-line anchors are supported: range fields are rejected, which is also what Gitea needs. `--event comment` with nothing to say and `--event request-changes` with no body now fail locally instead of as a server 422. Merge preflight. `fj pr merge` used to explain a blocked merge only after the server refused it, and through the admin-only branch protection endpoint. It now reads the base branch's effective protection from GET /branches/{branch}, which needs read access only, before the POST. A certain blocker (a required check failing, pending or not reported, a missing required approval, no merge permission) stops the merge with the reason and, for checks still running, a pointer to --auto. Evidence that could not be read stays "unknown": it never blocks and never reads as passing, and the server decides. `fj pr checks` shows the same verdict, gains a REQUIRED column, and lists required checks that have not reported. Its --json is additive: `required` per status, `statuses_complete` and `merge_preflight`. Covered by unit and wiremock tests only; the preflight was not run against a live protected branch. `fj pr checks` gains a `REQUIRED` column, which shifts the table for anything parsing it. Part 6 of 8 in the spec-parity stack. Based on `feat/spec-parity-org`, so the diff here is this slice alone; it needs a rebase onto main once the PR below it is squash-merged. Closes #213 Verified on macOS at this commit: `cargo fmt --all` clean, `cargo clippy --all-targets --all-features -- -D warnings` clean, `cargo test --all` green.

stephen added 1 commit

2026-10-08 19:18:55 +00:00

Review on diff lines and explain merge blockers before merging 9ae0ad1c94

Two pull-requests requirements of the fjord-client-spec (#213).

Inline review comments. `fj pr review N --line-comment <PATH> <LINE>
<BODY>` is repeatable, and `--line-comments-file` takes the same as a
JSON array. LINE is `N` or `new:N` for the PR's version of the file and
`old:N` for a removed line. Comments given with --event are batched into
that review in one request; without --event they post as a plain comment
review. Each anchor is checked against the PR diff first, so a line that
is not commentable is refused locally with the commentable lines named.
Only single-line anchors are supported: range fields are rejected, which
is also what Gitea needs.

`--event comment` with nothing to say and `--event request-changes` with
no body now fail locally instead of as a server 422.

Merge preflight. `fj pr merge` used to explain a blocked merge only
after the server refused it, and through the admin-only branch
protection endpoint. It now reads the base branch's effective protection
from GET /branches/{branch}, which needs read access only, before the
POST. A certain blocker (a required check failing, pending or not
reported, a missing required approval, no merge permission) stops the
merge with the reason and, for checks still running, a pointer to
--auto. Evidence that could not be read stays "unknown": it never blocks
and never reads as passing, and the server decides.

`fj pr checks` shows the same verdict, gains a REQUIRED column, and
lists required checks that have not reported. Its --json is additive:
`required` per status, `statuses_complete` and `merge_preflight`.

Closes #213

stephen changed target branch from feat/spec-parity-org to feat/spec-parity-issues

2026-10-08 19:42:48 +00:00

stephen added 1 commit

2026-10-08 19:42:59 +00:00

Browse commits and files at a ref, show signature state, manage org labels (#284 ) c98fb7ec34

Four fjord-client-spec requirements: ssh-gpg-keys (commit signature
state), code-browsing (tags and browsing a tag ref), actions-ci
(Actions variables) and labels (organization labels and exclusive
groups).

`fj repo commit list|view` read history at --ref, --branch or --tag and
show each commit's signature state from the `verification` object:
verified with the signer, or nothing when the commit is unsigned. The
SIGNATURE column is dropped when no commit reports one.

`fj repo contents [PATH]` reads a file or directory at a ref. A tag is
pinned to its commit and labelled read-only. `fj browse --tag` opens the
tag route.

`fj label list|create|edit|delete --org ORG` work on organization
labels, with --exclusive and --no-exclusive for `group/name` labels.
Assigning labels on issue and pr create or edit now falls back to the
org's labels for names the repository does not have, and when an
exclusive label displaces a sibling the message is taken from the set
the forge confirms.

`fj label create` and `fj label edit` panicked at parse time on main,
with or without the flag: their `--color` shared an arg id with the
global ANSI `--color`. The label flag is now `--label-color`.

`fj variable set` could not create a variable (PUT only) and a
non-empty `fj variable list` failed to decode, because the model
required a `created_at` the API does not send. `set` now reads, then
PUTs or POSTs, then confirms. The list table is NAME / VALUE / SCOPE and
its JSON follows the ActionVariable shape. Variable values no longer
reach --debug output.

Covered by unit and wiremock tests only. Two things deserve a live check. The `variable list` model was rewritten to the swagger `ActionVariable` shape, and the test `list_variables_parses_live_bare_array` was rewritten with it, so the claim that the old shape never decoded rests on the swagger document and not on a recorded response. And `--color` to `--label-color` on `label create|edit` is a flag rename, though the old spelling could not be used: both commands panic at `src/cli/mod.rs:105` on main ("Mismatch between definition and access of `color`").

Part 7 of 8 in the spec-parity stack. Based on `feat/spec-parity-pr`, so the diff here is this slice alone; it needs a rebase onto main once the PR below it is squash-merged.

Verified on macOS at this commit: `cargo fmt --all` clean, `cargo clippy --all-targets --all-features -- -D warnings` clean, `cargo test --all` green.

Reviewed-on: #284

stephen added 1 commit

2026-10-08 19:48:17 +00:00

Merge feat/spec-parity-issues into feat/spec-parity-pr f3a3ff8f6a

stephen added 1 commit

2026-10-08 20:22:45 +00:00

Identify the forge before gating, read logs by token only, support Basic-auth proxies (#285 ) 4ab2c1398a

Four fjord-client-spec requirements: two under gitea-compatibility, one
under api-contract (#209) and one under authentication.

Flavor before floor. fj warned "older than Forgejo 7" on any Gitea 1.x
host because it compared the version before knowing what it was talking
to. `client::capability` now probes /api/forgejo/v1/version and then
/api/v1/version: the first answering is Forgejo, only the second is
Gitea, anything else stays unidentified with its raw version. The floor
applies to Forgejo only. Flavor and version are stored in hosts.toml at
login and refresh and shown by `fj auth status`.

Capability gates. One table decides whether a route is offered, by
flavor and version, and replaces the ad hoc Forgejo version check.
`fj run rerun` uses the Gitea 1.26+ routes and is withheld elsewhere;
`fj run cancel` uses the Forgejo 16 route and is withheld on Gitea and
older Forgejo. A withheld action sends nothing and names the run URL.
On Forgejo, `fj run rerun` now reports that limitation: the web POST it
used to attempt could never succeed with a token.

Token-only log reads. Job logs are read with Range requests in 1 MiB
chunks up to a 64 MiB ceiling. A server without the log routes gets a
typed limitation with the run URL, or a Loki the user configured. The
token no longer reaches any web route: `fj run view`, `fj run watch` and
artifact downloads use the API where the server has it and fall back to
an anonymous web request elsewhere, so on older servers they work for
publicly readable runs only.

Reverse proxies. `fj auth proxy` stores a separate HTTP Basic credential
for a proxy in front of the forge. A 407 proxy takes it in
Proxy-Authorization. A 401 Basic proxy takes it in Authorization, and
the forge token moves to X-Forge-Authorization, which the proxy has to
forward (one line in nginx or Caddy). `--pat-transport query` works with
an unmodified proxy at the cost of the token appearing in access logs.
The credential is sent to the configured host only, over HTTPS or
loopback, and neither secret appears in --debug output or errors. Git
over HTTPS behind such a proxy is out of scope; the docs recommend SSH.

Covered by unit and wiremock tests only. Nothing was run against a real Gitea, an older Forgejo, or an nginx or Caddy Basic-auth proxy, and the Gitea version gates come second-hand from the fjord-ios audit document.

Two behavior changes to weigh. Private repositories on Forgejo older than 16 lose `run view`, `run watch` and artifact downloads, because those used to work by sending the token to a web route. And `fj run rerun` on Forgejo now reports a limitation instead of attempting a POST that could not succeed.

`DEFAULT_LOKI_URL` is still used in one case, when a server that has the log routes reports a single job's log as missing and no Loki variable is set. `docs/architecture.md` says so.

Part 8 of 8 in the spec-parity stack. Based on `feat/spec-parity-browse`, so the diff here is this slice alone; it needs a rebase onto main once the PR below it is squash-merged.

Closes #209
Refs #103

Verified on macOS: `cargo fmt --all` clean, `cargo clippy --all-targets --all-features -- -D warnings` clean, `cargo test --all` green (1298 + 5 + 1), pre-push hook passed.

Reviewed-on: #285