Rust's runtime sets SIGPIPE to SIG_IGN at startup, so writing to a pipe
whose reader has gone away returns EPIPE and println! panics. `hive-forge
<verb> | head` printed a panic and exited 101 where cat, grep and every
other pipeline member exit quietly.
101 is not cosmetic: to a caller running `set -o pipefail` it is a real
failure, so a shell script that pipes our output stops on a condition
that is not an error.
Restore SIG_DFL first thing in main, before any output. Measured against
the same command: piped to head, 141 (killed by the signal) with empty
stderr; unpiped, 0; the pre-change binary, 101 with a panic.
Closes#3972
Seven hidden aliases are pure renames — `pr status` does exactly what
`pr-status` did. They now refuse and name the replacement instead of
running, which is the deprecation step before removing them outright.
Deliberately not all of the hidden verbs. The other ten (`view`,
`comment`, `comments`, `close`, `reopen`, `assign`, `labels`, `timeline`,
`dependency`, `reaction`) exist as BOTH `issue <verb>` and `pr <verb>`,
so a caller holding only a number cannot be told which to run. Refusing
those removes a capability rather than renaming one, and there is no
message that would help; they keep working pending a decision on what
replaces them.
Measured: each of the seven exits 1 naming its replacement; `view` still
exits 0; `pr status` still returns its merge-readiness verdict (0 on a
ready PR, 1 on one with CI pending).
Refs #3974
The runs-list query's `ref` filter matches only a fully-qualified ref,
while the listing prints `prettyref` (`main`, `#3967`). So no value a
caller can read off the output was accepted by the filter, and the
rejection rendered as "no matching runs" — indistinguishable from a
branch that has never been built.
Qualify the value before it reaches the query: `refs/...` passes
through, `#N`/`N` becomes the pull head ref, anything else becomes a
branch. A slash cannot mark an already-qualified ref, since branch
names contain them.
Not shared with ci-rerun: its `ref` is a workflow_dispatch body field
taking a bare branch name, so the same normalisation would break it.
Closes#3970
Pure `nix fmt` output from the commit before this one — no hand edits.
203 files: 52 md, 42 tsx, 32 js, 32 css, 21 ts, 13 html, 8 json, 3 mjs.
Reproduce with `nix develop -c nix fmt` on the parent commit; the result
should be byte-identical to this tree.
None of the 13 `.prettierignore` entries appears here — verified by
intersecting the changed-file list against the ignore file, with a
control proving the intersection finds a match when one exists.
clippy::items_after_test_module (pedantic, denied) — a #[cfg(test)] mod
has to be the last item in its file. Pure move: same line count, same
line multiset, no content change.
`cargo test` was green while this was broken; only clippy saw it.
Per mara's go-ahead on hyperhive#3902 ("getting started is good, but
terminal rendering does not go in there i think"):
Moved 21 top-level docs/*.md files into 7 new topic subdirectories
(existing web-ui/, turn-loop/, swarm/, tools/, crates/ untouched):
getting-started/ setup.md
agent-lifecycle/ agent-hierarchy.md, approvals.md, persistence.md
trust-boundary/ boundary.md, security.md
integrations/ forge.md, matrix.md, github.md, knowledge.md
networking/ gateway.md, network.md, snapshot-store.md
scheduler/ jobq.md, coordinator.md, ci.md, observability.md
process/ conventions.md, gotchas.md, pr-review-gate.md
web-ui/ terminal-rendering.md (moved into the EXISTING dir,
per mara's correction to the original getting-started
guess -- it's UI implementation detail, not onboarding)
The physical layout now matches docs/README.md's own topical headers,
which already amounted to this taxonomy -- see the scoping comment on
the issue for the two findings that motivated this (a genuine
duplication between CLAUDE.md's old "Reading paths" list and
docs/README.md's grouped one, since drifted out of sync with each
other; and the flat layout not matching the grouping we already had).
Fixed every cross-reference this moved across the whole repo (~120
files: docs/ internal links at every depth, Rust doc comments, nix
module option docs, crate READMEs) -- verified two ways: a grep sweep
confirming zero remaining references to any old path, and a script
that resolves every markdown link in docs/**/*.md + CLAUDE.md +
README.md against the filesystem and reports anything that doesn't
exist (zero broken links).
Collapsed CLAUDE.md's "Reading paths" section (the duplicate) down to
a pointer at docs/README.md, now the single index. Rewrote
docs/README.md itself to use the new subdirectory paths and added the
one doc it was missing that CLAUDE.md's old copy had (pr-review-gate.md).
Classified all 22 docs/*.md files first via a haiku subagent (mara's
suggestion) on two axes -- proposed grouping and operator-vs-
implementation focus -- before finalizing the taxonomy; spot-checked
the report and found internal inconsistencies (its classification
table disagreed with its own summary section for a few files), so this
taxonomy is my original proposal + the one correction mara gave
directly, not a blind application of the subagent's table. The
operator-focus data it gathered is still useful for a follow-up
content pass (docs skewing 'mixed' rather than pure operator-facing),
not addressed in this PR -- structure only.
nix fmt clean, both pre-push lints clean.
Nine of the 38 .nix tokens mentioned anywhere in *.rs did not name
anything that exists. Twelve mentions, five distinct targets:
hive-c0re.nix, hive-gateway.nix and hive-forge.nix are all directories
now; nix/modules/ is not a directory we have; hive-forge-tools.nix was
a bash script the binary replaced and is gone.
Where the reference is load-bearing it is corrected rather than
deleted, because the reference is the point: a comment saying a
constant must match a nix literal is only useful if you can open the
file it names. Where the module member was unambiguous the path now
names it exactly.
paths.rs's STATE_ROOT marker was the worst of them: it claimed the
value came from services.hyperhive.c0re.statePath, in hive-c0re.nix.
Neither exists. The option is not declared anywhere and the file is a
directory, so a "must match" contract pointed at two things that
cannot be opened. /var/lib/hyperhive is hardcoded on both sides, which
is what the comment now says.
hive-forge-tools.nix keeps no replacement: naming a file that was
deliberately deleted helps nobody, and "replaces a prior bash script"
is complete without it.
Measured before and after with the same command: 9 unresolved of 38
before, 4 of 35 after. The remaining four are an example path in a doc
comment, an upstream nixpkgs path, and two from one synthetic test
fixture.
Forgejo's issue-list endpoint answers 'was this merged?' only inside the
nested pull_request object, while state says closed for a merged PR and
for one closed without merging alike. So the obvious top-level query is
null or ambiguous for every row, and with a // default it renders as a
confident 'nothing merged' that cannot ever be right -- a wrong answer
shaped exactly like a clean one.
Copy merged and merged_at up to the top level of each PR row so the
obvious query is the correct one. Additive: the nested object is left
untouched so an existing consumer keeps working, and issue rows have no
pull_request and pass through unchanged.
The head branch is deliberately not hoisted: this endpoint does not carry
it at all -- the row's ref is an empty string, not the branch -- so there
is nothing to lift. pr show has head_branch.
forgejo-api 0.11.0 links reqwest 0.12 while the workspace is on 0.13, and
cargo unifies features per (crate, VERSION) — not per crate. So forgejo-api's
internal client was a *second* reqwest, built with neither the workspace's TLS
features nor the system trust store, and every https call it made to a gateway
name failed `invalid peer certificate: UnknownIssuer` while a client built in
this workspace succeeded against the same host.
Pinning to 0.11.1 puts one reqwest in the tree, which is what makes the
existing "feature unification gives forgejo-api our TLS backend" comment true
rather than aspirational — the claim is a fact about the lockfile, so the pin
is load-bearing and says so.
The reqwest feature list moves with it: `rustls-native-certs` and
`webpki-roots` were 0.13.1-era names that no longer exist, and `rustls` now
carries the platform verifier (OS trust store, honours SSL_CERT_FILE). Naming
a feature that is gone is a hard resolution error rather than a silent no-op,
so the list had to change in the same commit as the pin.
Refs #3391
Forgejo returns a bare `null` body (not `[]`) for a reactions list when
nothing has reacted yet. issue_reactions/comment_reactions deserialized
straight into forgejo-api's typed Vec<Reaction>, which has no null
tolerance, so issue show / pr show / view failed on every item with zero
reactions - i.e. nearly everything.
Generalize pr_status's existing null_as_empty (same quirk, hit earlier
on combined-status statuses) into a shared helper in verbs/mod.rs, and
fetch reactions via the raw JSON path (Client::get_api_json) with a
NullableVec<T> wrapper instead of the typed client's bare Vec<Reaction>.
Also names the failing request in errors going forward, since
get_api_json's error context includes the URL - closes the gap the
issue itself flagged (the old error said what didn't parse but not
what was fetched).
argus's review note: the emptiness guard was one-sided. A reviewed sha
of "" was already treated as unknown, but a head of "" was not — so a
forge returning an empty string rather than omitting the field would
make every review compare unequal and mark the whole PR stale. That is
the wrong-direction failure this function exists to prevent, and the
asymmetry contradicted its own doc comment.
Both sides now check emptiness, for the same reason: a blank string is
a value the forge sent, not a sha it has.
Forgejo's per-review `stale` flag is eventually consistent: seconds
after a push it still reports the pre-push answer, so a verdict against
the previous head reads as current in exactly the window where the CLI
gets run right after pushing.
latest_reviews now fetches the PR head once and ORs a direct comparison
of the review's own commit_id into the flag. Fixing it at construction
rather than at the call sites means superseded() is unchanged and all
three consumers are corrected together: assign-reviewer's refusal,
pr_merge's changes-requested gate, and — the one that matters most —
pr_status's readiness verdict, which could otherwise report a dead
approval as valid.
Unknowns fail toward keeping the verdict: a missing head or commit_id
degrades to today's behaviour instead of voiding every review on the PR.
The third bullet of the issue's ask, dropped in the first pass and caught
in review: a filter value that does not resolve is a near-miss far more
often than an invention, and an error that only lists all 18 available
names makes the reader do the diff by eye -- on the one occasion they
already know they mistyped something.
Thresholded rather than always suggesting the minimum-distance
candidate: a wrong suggestion is worse than none, because it invites a
second failed attempt at a name that was never there. The bound scales
with the needle (a third of its length, capped at 3), so a short name
does not match half the repo and a long one still tolerates a typo or
two, and an unrelated word falls back to the full list.
Tie-break is on length then alphabetical, so the suggestion does not
depend on the order the forge happened to return its labels in.
A filter value the forge cannot resolve is DISCARDED, not rejected, so a
typo does not narrow the result set -- it returns the unfiltered one.
That does not waste a query, it inverts the answer: "is anything open in
this milestone" comes back as every open issue and reads as yes, and a
duplicate check gets a list that never narrowed.
`list` now resolves both before querying. Labels reuse the write side's
resolver; the ids are discarded because this endpoint filters by name, so
resolution here is a spell-check rather than a lookup -- reusing it keeps
the message identical to the one the write side has always produced.
Milestones accept a title or an id and are checked against the ALL-state
set: filtering on a closed milestone is a normal query, and validating
against open-only would reject exactly the retrospective ones.
Both fetchers paginate. `repo_labels` asked for one page of 100 and
treated it as the population -- the inverse of the trailer bug, same
root: a valid label past the cut fails to resolve, and the error then
prints an "available labels" list that is itself truncated, so the
message argues for the typo.
`--assignee` / `--author` stay unvalidated on purpose: someone who has
left still legitimately appears on old issues, so a login that is not a
current member is not necessarily a typo.
Also drops the docs paragraph claiming unknown labels are silently
dropped on the write side; that has not been true since the resolver
landed.
`list` already built its query with `q: None, milestones: None` — both
fields were on the request it was sending. So full-text search over
title and body is a flag, not a new verb, and a text match is only
useful composed with the other filters anyway.
The trailer was the real defect. It fired on `count == limit`, but the
forge clamps page size to its own `api.MAX_RESPONSE_ITEMS`: ask for 400,
get a full 50, and `50 != 400` kept it silent — suppressing the warning
in precisely the case where the truncation is invisible. It now reports
the real total from `X-Total-Count`, which the response header struct
already parsed and the call site discarded. The requested limit is not
clamped client-side: that ceiling is the remote's configuration, not
ours.
Wraps the whole verb dispatch in run() with .with_context(|| format!("repo {repo}"))
instead of threading context through ~34 individual verb files. A body-decode
failure surfaces from forgejo-api as a bare ReqwestError with no status code or
URL retained (no client-injection point to capture more), so without this the
error alone can't distinguish a mistyped org/repo from a transient flake — see
the recent hive-forge triage-automation thread this was filed from.
Split run()'s match into a dispatch() fn so the repo can be captured once before
dispatching and the with_context wrap applied once after, uniformly, regardless
of which verb failed.
fixeshyperhive/hyperhive#2839. pr_assign_reviewer's own doc-comment
claimed full idempotency (already-requested = no-op), but conflated
'still pending' with 'already reviewed' - Forgejo clears a fulfilled
reviewer from requested_reviewers, so re-requesting them isn't a
no-op, it dismisses the standing review. now checks latest_reviews
for a non-superseded review from the target user first and skips
the request instead of blindly posting it.
