Review call: 46 lines of documentation around a single constant, part of
it already stale. The worst paragraph explained why the earlier per-hive
shape had been justified wrongly — history of a design that never
shipped, written into the file within an hour of that design being
dropped. A file is not a changelog; why it was wrong belongs in the PR.
The constant moves to lib.rs beside the status bucket name, keeping only
the rationale that stays true: three crates must agree on the string, and
the one that agrees hardest speaks neither jetstream nor kv, which is why
it cannot sit behind a feature gate.
status earns a module of its own because it holds a bucket name AND the
functions that open it. This held a constant.
Review call: the event was addressed per hive — `$SWARM.events.<hive>.knowledge`,
published in a loop over the roster, granted through a wildcard. It does not
need to be. The payload is empty and the event means the same thing to every
hive, so one publish to one subject delivers exactly what N publishes to N
subjects did, and core NATS already fans out to whoever is subscribed. A hive
that was down misses it either way and reconciles on its next periodic pull.
That deletes rather than reshuffles: the roster loop, the wildcard, and the
shared subject-building function whose entire purpose was keeping the grant and
the publish from drifting apart. With one literal there is nothing to disagree
about.
The per-hive shape was justified by the callout policy's rule that an extra
subject must contain the hive name. That rule governs `extra_hive_subjects` —
what a HIVE may publish. This subject lives in the controller's reader grant,
which the rule does not constrain, so a real rule was carried across into a
decision it had no authority over.
Knowledge becomes its own category rather than a leaf under a general event
namespace, since a namespace shaped for events that do not exist yet is a
decision made before there is anything to decide from. The empty config-PR match
arm goes with it: an arm with no body claims this is where the deploy path is
handled, and it is not.
The deny test stays and matters more, not less: with one shared subject a forged
event would reach the whole swarm where a per-hive one reached a single hive.
The test's doc named knowledge::ensure_webhook as one of two hive-side
reapers. That function is gone; its replacement matches the full URL and
cannot touch another hive's hook.
The assertion arm stays. The hazard is not what this repository's source
says, it is what is deployed — a hive still running the previous version
reaps by suffix until it is upgraded, so the arm guards the transition
rather than a current code path. Recorded when to drop it.
A webhook has exactly one target URL, so every hive registering one
against the shared internal/knowledge repository was last-writer-wins
rather than idempotent: all but the most recent silently stopped
receiving deliveries. The swarm controller holds the single registration
and now addresses an event to each hive over the queue instead.
This is a migration, not a deletion. Not registering any more fixes
nothing on a hive that has already run — the hook it created persists on
the forge, so the contention would survive on exactly the deployments
that have it while fresh installs looked fixed. The hive that created a
hook removes it.
It removes only its OWN, matched on the full URL rather than the
/webhook/knowledge suffix. A hook with that suffix and a different base
belongs to another hive, possibly one not yet upgraded, and deleting it
would break that hive's knowledge sync until it caught up. Reaping a
neighbour's registration is the behaviour being removed here; doing it
while fixing it would only invert the direction.
The predecessor did reap by suffix, to clear loopback hooks left by an
older single-hive layout. That was safe when a hive was alone on its
forge and is not safe now. The hive-side registrars also acted as reapers
of hooks under their own path, which is why the swarm hook lives under
/webhook/forge/; removing this registrar removes that reaper too.
Intended, and stated because no reviewer would infer it from the diff.
The receive endpoint goes with it. A live HMAC-verified
/webhook/knowledge that nothing can legitimately reach would tell the
next reader that this is how a hive learns about knowledge changes.
Docs move in the same commit: docs/swarm/README.md said two hooks exist
per swarm-wide repo and neither should be deleted, which is now true for
agent-configs and wrong for internal/knowledge — a half-correct
description being worse than an uncorrected one.
A hive learned the knowledge repository had changed only by registering
its own forge webhook. This subscribes to the per-hive subject the
controller now publishes on and calls the pull this daemon already runs
at boot.
Shares the hive's ONE queue connection rather than opening a second: a
second connect would double the auth-callout traffic against authelia and
give the two paths independent reconnect state, so one could be serving
while the other was still down. Same argument as the controller side.
No payload is read, because there is none to read — the webhook handler
this replaces took two fields from Forgejo and used neither, then ran
`git pull`, which re-derives everything from the repository.
At-most-once, and that is not a regression: a webhook delivery to a hive
that is down is lost identically today, and the boot pull covers it.
JetStream would require this end to publish to
`$JS.API.CONSUMER.CREATE.<stream>`, which the callout policy does not
grant, so durability would cost grants on both sides to remove a failure
the boot pull already handles.
⚠️ Documented at the call site rather than left implicit: a refused
subscription is indistinguishable from a quiet one, because NATS reports
authorization violations asynchronously on the connection. If hives stop
hearing events, the server log is the thing that knows.
futures-util comes from the workspace (same version swarm-controller
already uses), not a new dependency version.
Every one of these carried a "SAFETY: single-threaded mutation of an env
var no other test asserts on" comment. Each claim was true of the module
and false of the process: env vars are one process-global and every
#[test] in this crate lands in the same binary at default parallelism,
so "no other test" has to mean no other test in the BINARY — and two of
them set HIVE_FORGE_URL outright.
They now take the crate lock #3483 added, including the two whose
variables nothing else touches: what makes a variable safe is that every
mutator routes through one lock, not that today's set happens not to
collide.
The doc comment on push_forwarded_var_options said the render-level
tests race each other; they serialise now, so it says that. It is
deliberately not an intra-doc link — test_env is #[cfg(test)], so
rustdoc cannot resolve it and -D rustdoc::broken-intra-doc-links fails
the docs check.
Review finding from argus. The new endpoint test carried a SAFETY comment
claiming no other test in its module asserts on the variables it perturbs —
the wrong boundary. The module is not the unit that shares the environment,
the process is: meta.rs's render_flake_injects_otel_when_signalled mutates
the same HYPERHIVE_OTEL_ENDPOINT, both land in the one hive-c0re test binary,
and cargo runs it at default parallelism with no serialisation anywhere in
the crate. Each test independently claimed exclusive ownership of shared
global state, which is the instrument-that-looks-solid class the endpoint
change's own gate reasoning warns about.
Adds test_env with a single ENV_LOCK, taken by both. No new dependency: this
is the pattern hive-bash-mcp and hive-agent already use, and hive-bash-mcp's
helper records why it has to be crate-wide rather than per-module — two
per-module mutexes serialise nothing against each other, which produced a
CI-only flake there.
The asymmetry that makes this hard to see locally is worth stating: an agent
container has the hyperhive variables ambient-set, so a losing race still
finds a plausible value and the test passes; the nix sandbox strips them, so
only there can one thread delete a variable out from under another. Verified
in that shape with `env -u HYPERHIVE_OTEL_ENDPOINT -u
OTEL_EXPORTER_OTLP_ENDPOINT`, five consecutive runs green — a sanity check,
not a proof, since a race cannot be shown absent by running. What makes it
correct is structural: both tests take the same lock.
Deliberately scoped to the pair that overlaps. meta.rs has three further
env-mutating tests (HIVE_FORGE_URL twice, the TLS CA pair) that race with
each other, untouched here and tracked separately, because the fix is not
the mechanical one it looks like: std::sync::Mutex is not reentrant, so
adding a lock to a test whose helpers also lock deadlocks. That needs
reading per test rather than a sweep.
hive-c0re's container-resource exporter has POSTed to a 404 for as long as
it has existed, silently: it passed the collector's base address to
`with_endpoint`, which the SDK takes verbatim, so every export went to `/`
instead of `/v1/metrics`. Nothing reported it — OTLP export failures go to
an error handler no binary here installs — so the daemon logged "exporter
enabled" and delivered nothing. VictoriaMetrics has never held a sample
under `service.name=hyperhive-c0re`.
Fix the way the rest of the repo already resolves an endpoint: an endpoint
option names a BASE, and the layer that knows the signal appends to it.
`hive-metric` — same SDK, same collector — never calls `with_endpoint`, and
`docs/observability.md` documents the append as system behaviour; the one
place a full path is spelled out is the VictoriaMetrics exporter, because
its far end is not a standard OTLP path.
So drop the call. The builder is now byte-identical to hive-metric's, and
hive-c0re's unit carries the standard `OTEL_EXPORTER_OTLP_ENDPOINT` for the
SDK to read. The address is bound once in nix and consumed twice, so what a
hive hands its agents and what it exports to itself cannot drift.
The enable signal moves to that same standard variable: "configured" and
"where it actually goes" become one string rather than two that agree by
convention. `HYPERHIVE_OTEL_*` keeps its own job, the agent-config
transport meta.rs reads — a name the SDK has never known, which is the bug.
The test changes shape with the fix. The old one asserted a URL this module
built; the new one pins that the exporter is gated on the variable the SDK
itself reads, because the fix is now an absence and an absence is what a
later "the endpoint is right there, just pass it" edit puts back.
Refs #3402
hive-c0re's container-resource exporter already targets this hive's own
collector (environment.nix derives the bridge address), so the upstream
header it was loaded with has nowhere to be presented: that hop is
unauthenticated for every producer on the host, and the credential
belongs to the swarm tier, which is the one that leaves the swarm.
Drop the LoadCredential entry and the auth_headers() reader with it.
The option itself stays -- swarm-otel.nix is its real consumer, via
EnvironmentFile on the collector unit.
Also corrects three descriptions that this makes false, or that were
already false: the module doc claimed to reuse the config "Claude Code's
in-container SDK export uses", which stopped being true when agents
moved off that path; the nix comment claimed the secret is "the same one
the agent containers get, forwarded via nspawn --load-credential", which
lost its last producer earlier; and docs/observability.md described an
Authorization header on a hop that will no longer send one. The
headersCredential option's own docs already said it reaches "neither an
agent container nor a hive's own collector" -- this makes that true
rather than aspirational.
The host-side collector is the only path telemetry leaves a hive, so
HYPERHIVE_OTEL_HEADERS_CREDENTIAL is never emitted and everything
downstream of it is unreachable. What made it worth removing rather than
leaving inert is what it looked like to a reader: a complete,
well-commented mechanism for writing the hive's upstream credential into
a file the agent can read, described in the present tense. Anyone auditing
"can an agent obtain the OTEL token?" had to reconstruct the whole env-var
chain to find out the answer is no.
Gone: the per-agent `hyperhive.otel.headersCredential` option, the
`hive-otel-header` oneshot that merged OTEL_EXPORTER_OTLP_HEADERS into the
agent's own settings.json, and meta.rs's field, env read and render.
⚠️ Scoped by NAMESPACE, not by name. `hyperhive.otel.headersCredential`
(per-agent) and `services.hyperhive.otel.headersCredential` (host) are
different options sharing a leaf name — the host one is read by
`stats/otel_metrics.rs` for c0re's own container-resource exporter and
stays. Sweeping the string would have taken out working code.
The comment above `otelSettingsEnv` now states the property rather than
the absence: there is no auth header and no mechanism to add one, because
an agent exports to the hive's own collector and nothing an agent can read
is a secret to the swarm. The old behaviour is named in the past tense so
it reads as removed rather than overlooked.
meta.rs's assertions that pinned the injection are deleted rather than
adjusted; the surrounding test keeps covering extraResourceAttributes and
the endpoint/protocol injection, which are live.
Ports the shadow-DOM <hive-jobq-graph> custom element
(frontend/packages/shared/src/jobq-graph/) to a Preact component
(JobqGraph.js) shared by the dashboard and swarm-ui, per hyperhive#3310.
- JobqGraph.js: written with plain h() calls (no JSX) so the same file
compiles unmodified under both the dashboard's text-loader CSS config
and swarm-ui's JSX config. Exports `JobqGraph` for JSX use and
`mountJobqGraph(container, props)` for the dashboard's non-JSX
imperative mount, returning a `{refresh(), update()}` handle matching
the old custom element's public surface. Same rendering contract as
before: indented state tree, payload.label verbatim, payload.data as
a generic key/value list, "waits on: <label>" text for Node-kind deps,
per-state filter checkboxes, optional cancel button.
- jobq-graph.css: light-DOM adaptation of the old shadow-scoped
stylesheet (:host -> .jg-root, otherwise unchanged).
- dashboard/src/builds.js: local mountJobqGraph() renamed to
mountRebuildQueue() to avoid colliding with the newly-imported shared
mountJobqGraph; cancel handling is now a plain onCancel callback
instead of a DOM CustomEvent listener (no shadow boundary to cross
anymore).
- dashboard + shared package.json: added preact as a dependency (matches
swarm-ui's existing pin, 10.29.8) - the dashboard was a vanilla-JS MPA
with no Preact/JSX pipeline before this.
- Removed the old hive-jobq-graph.js/.css entirely (confirmed via grep
it had exactly one consumer, dashboard/src/builds.js, so this is a
clean swap, not parallel maintenance of two implementations).
- Updated stale doc-comment references to the old element name in
builds.html, tabs.js, swarm.js, docs/web-ui/dashboard.md, and
hive-c0re/src/job_queue/mod.rs.
Verified: npm run build (whole frontend workspace) and npm run
typecheck (swarm-ui) both clean; cargo build/clippy/test -p hive-c0re
all clean (331 tests, 0 failures); headless-chromium screenshot of
/builds.html against a mock GET /api/jobq/graph payload confirms full
visual/behavioral parity with the old custom element (tree, filter
checkboxes, cancel buttons, error text, waits-on line, data list, live
build log panel).
This covers the dashboard-replacement half of hyperhive#3310 only. The
swarm-ui half (rendering the CreateAgent DAG on the agent-creation page)
is downstream of hyperhive#3306/#3124 landing - no swarm-ui page exists
yet to mount it in.
Same review catch as the crate side: these two format the queue client's
own error with `{:#}`, and thiserror's Display ignores the alternate flag,
so the source was silently dropped. The banners read "swarm status
publishing is off: swarm queue is half-configured" with no list of missing
variables, and "...: connecting to the swarm queue at <url>" with no nats
error saying why.
These are the two worst places to lose it. `set_boot_warning` is for a
one-shot startup step with no retry: the banner leaks until the process
restarts, so it is the operator's whole account of what went wrong.
The controller reads per-hive status out of a JetStream KV bucket and
nothing was writing one, so every hive rendered `never_reported`. This is
the half that makes the read path mean anything.
A hive offers; the controller never reaches down to collect. The gateway
has gone down in a way where every recovery channel ran through the one
broken thing, so a status path that depended on the controller would go
dark exactly when it is needed to diagnose the controller's own network.
What it publishes is what the hive already says about itself —
`warnings::readiness()`, the same value `/health/ready` serves. Nothing
here stamps a time: freshness is derived by the reader from when the value
landed, so a hive cannot make itself look fresher than it is, and a hive
with a wrong clock skews only its own payload.
The key is this hive's `hiveName`, which `swarm.nix` already asserts is a
key of `swarm.hives` — so a hive that evaluates at all publishes under a
name the roster knows, rather than by convention.
Publish first, then wait: a hive that has just come up is the one whose
status someone is looking at, and sleeping first would make every restart
read stale for a full interval. The interval is one decision with the
controller's staleness threshold, not two — a ratio of 2 means one lost
publish still reads fresh and two consecutive misses read stale.
Failures go to the dashboard banner through SweepHealth, debounced, at
`warn` and deliberately not `crit`: `crit` is what makes this hive report
itself degraded, and a hive that cannot reach the queue is not unhealthy —
the swarm's view of it is. Publishing `degraded` because the publish
failed would be both false and self-erasing on the next tick.
`get_health_ready` computed "degraded iff any warning is crit" inline and
wrapped it in a private `ReadyBody`. The swarm status publisher needs the
same verdict, and the warnings module's own doc already states why it must
not compute its own: two systems independently deciding what counts as
unhealthy is how they end up disagreeing.
The disagreement would also be silent. Each side would look internally
consistent, and the day a second degraded condition is added to one of
them, the dashboard and the swarm view would report different things about
the same host with nothing to flag it.
`warnings::readiness()` is now the single producer and `Readiness` the
single type. `ReadyBody` is deleted rather than made public: the endpoint
keeps the part that genuinely is its own, the mapping onto an HTTP status
code, and serves the shared document as its body.
Mara wanted the underlying plumbing gone too, not just the dashboard
display. Traced every consumer before cutting:
- certFingerprint (services.hyperhive.swarm.hives.<name>.certFingerprint):
removed the nix option entirely. Its only consumer was the dashboard
code removed in the previous commits.
- HYPERHIVE_PEERS: removed entirely — the env var itself, the whole
block that built it in hive-c0re/environment.nix, and its entry in
meta.rs's FORWARDED_VARS (which forwarded it into every agent
container). Turned out to have zero real consumers, not just one:
the docs claimed hive-agent::identity::peers() read it for qualified
agent labels, but no such function exists — identity.rs only
qualifies THIS agent's own label with HYPERHIVE_HIVE_DOMAIN, nothing
peer-list-related. Grepped the whole hive-agent crate to confirm
before removing.
services.hyperhive.swarm.peerHives (the nix option HYPERHIVE_PEERS was
built from) is untouched — swarm-wireguard.nix reads it directly for
the wg-hive mesh, a real and unrelated consumer.
Verified: cargo build/clippy/test -p hive-c0re -p swarm-controller all
clean (needed nix develop -c per the usual -lsqlite3 gap), all touched
nix files pass nix-instantiate --parse, and a throwaway nixosSystem
eval confirms the wireguard mesh still configures a peer's
wireguardAddress into wg-hive correctly with certFingerprint gone.
Removes the per-hive dashboard's "peer hives" display support:
`peer_hives` field on StateSnapshot, the `PeerHiveView` struct,
`parse_peer_hives()`, and `validate_cert_fingerprint()`. That surface
moved to swarm-ui's own hive roster page — no longer needed at the
hive level.
HYPERHIVE_PEERS itself is untouched: hive-agent::identity::peers()
still reads it for qualified agent labels, and the nix module still
forwards it to agent containers. Only this crate's dashboard-only
consumption is gone.
Verified: cargo build/clippy/test -p hive-c0re clean, grepped the
whole tree for stray peer_hives/PeerHiveView/parse_peer_hives
references after the removal — none left.
mara: comments to code ratio too high. It was 16 comment lines for one
line of feature.
Kept only the fact that stops the feature being trimmed away again - it
is not a transport, it is in `default`, so `default-features = false`
drops it - and, in hive-metric, the one reason its declaration
deliberately differs. Why an HTTP 200 is the failure that matters, and
what a stderr-reporting CLI would cost, are the PR's and the follow-up
issue's job, not the manifest's.
`default-features = false` on opentelemetry-otlp was written to trim
transports and dropped `internal-logs` with them, so every otel_warn! and
otel_debug! inside that crate compiled to nothing. The failure that
matters is the one which returns HTTP 200: the collector accepts the
request and rejects the data points, and HttpMetricsClient.PartialSuccess
is the only place the rejection count and the collector's reason are ever
surfaced.
The feature is per-crate, not per-workspace: the macros are exported by
the opentelemetry API crate but their cfg and CARGO_PKG_NAME resolve in
the calling crate, so the API crate and the SDK had internal logs on
while the exporter did not.
hive-metric keeps the identical declaration on purpose - it installs no
tracing subscriber, so the feature would be inert there and would only
mislead. Both files now record which side of that they are on, and the
stale "same features as hive-metric" comment is corrected.
Not sufficient on its own: a hard export failure is logged by the SDK at
debug on the compiled timer path, so it stays below the info filter.
That needs a level rather than a feature and is left to review.
A bare `git pull --ff-only` merges whatever `branch.<current>.merge`
lists. A clone carrying more than one such entry aborts with "Cannot
fast-forward to multiple branches", which takes /knowledge out for every
agent on the hive - and the daemon neither writes that config nor can
see it, so the call worked only for as long as it happened to stay
clean.
Reproduced against two throwaway bare repos: duplicate merge entries
give that exact fatal, exit 128. A detached HEAD gives a different
error, and a missing tracking config does not reproduce at all - the
clone of an empty repo does write the tracking entry, so my first
explanation was wrong.
Naming origin and main makes the pull independent of local branch
config: a stray entry degrades to "the pull did not pick it up" instead
of breaking the shared mount. Does not explain how a second entry
appeared; nothing in this tree writes branch config.
Takes the crate from 26 rustdoc warnings to 1, on top of the ten in the
previous commit.
argus's review findings:
- agent_sockets.rs: [`write`] was still ambiguous (function vs macro).
The previous change narrowed the qualifier and left the ambiguity;
[`write()`] is what resolves it.
- forge/users.rs <hex> and stats/container_stats.rs <name>: unclosed
HTML tags in prose, now backticked.
The rest of the crate, so the count actually reaches zero:
- job_queue/mod.rs: Queue::graph_snapshot -> JobQueue::graph_snapshot
(there is no Queue type), and super::scheduler -> scheduler (mod.rs
*is* job_queue, so super:: pointed outside it)
- job_queue/resource.rs: NodeKind -> super::model::NodeKind
- matrix.rs: password_path(name) -> password_path; and
forge::provision_user_token -> crate::forge::provision_user_token.
Note the path has no `users` segment: forge/mod.rs declares `mod
users` private and re-exports it, so the canonical path comes from the
re-export rather than the directory tree.
- socket_server/lifecycle_handlers.rs: InfraContainer ->
hive_priv_sock::InfraContainer
- stats/otel_metrics.rs: crate::meta::otel_config is a private fn no
path can name from another module, so it becomes prose
- main.rs: redundant explicit link target dropped
coordinator.rs:405 (CrashWatchGuard) is deliberately untouched: #3244
deletes that doc block, so fixing it here would conflict with an open PR
and repair a symbol that is about to stop existing.
Remove or fix broken documentation links that accumulate silently:
- container_view.rs: HiveEnv reference
- forge/mod.rs: READY_TIMEOUT and webhook handler links
- workers/knowledge.rs: webhook handler link
- job_queue/model.rs: Claim::deps and WireNode::data references
- stats/hive_stats.rs: read_skill_breakdown reference
- stores/audit_log.rs: global() reference
- workers/agent_sockets.rs: ambiguous agent_sockets::write reference
- coordinator.rs: systemd.services.<harness> formatting
- resource_limits.rs: ambiguous write/read references
Some broken links were to deleted functions/types; these are replaced
with prose descriptions. Others referenced items outside this crate or
were private; these are replaced with plain text references or qualified
paths as appropriate.
Fixes: #3245
argus caught these on review. `recent_transient`'s doc still pointed at
NO_NODE_LABEL, and `running_transients`' still described destroy as
having no queue node behind it and linked suppress_crash_watch -- all
three deleted in the parent commit, so the first two were broken
intra-doc links and the third was simply false.
Migration is now the only operation in that sentence.
Destroy was a straight-line async fn with no queue node behind it, so
nothing in the graph could answer "is this container going down on
purpose?". That gap is why an imperative crash-watch suppression guard
existed: an RAII handle held for the operation's duration, a second way
to say what every other lifecycle op already says through its node.
Reuse the existing Stop node rather than teaching a new node to stop
things:
Stop -> DestroyContainer -> (PurgeState) -> DestroyBookkeeping
Stop already declares takes_container_down honestly, so the suppression
is now derived from the graph like every other op's. It also turns the
precondition into an edge: DestroyContainer runs only under a completed
Stop, so it operates on an already-stopped container and carries
takes_container_down = false permanently. A container still alive at
that point is a real bug and stays loud instead of being absorbed by a
flag -- which matters because a wrong true silently swallows a crash
while a wrong false only costs a spurious event.
Removes suppress_crash_watch, CrashWatchSuppression, crash_suppressed,
crash_watch_suppressed and NO_NODE_LABEL. The migration call sites went
with the obsolete startup migrations, so destroy was the last caller and
intent now has exactly one home.
destroy() becomes a submit-and-return, matching every sibling endpoint
(rebuild, kill, restart, start, pause, resume) -- it was the only
lifecycle op that awaited its work. The container rescan moves into the
bookkeeping tail, so ContainerRemoved now arrives after the 200 rather
than before it.
Also drops an orphaned doc-comment in coordinator.rs: two stacked blocks
where only the second described crash_suppressed, the first documenting
a field that no longer exists. Removing the field would have re-pointed
it at recent_transient.
/agents/<name>/config bound the working clone a config change is staged
in, so an agent could see a proposal that was never approved -- a config
that does not govern its container. Both objects already exist; this
repoints the bind at the deployed one.
Both mounts (own + child) now resolve through config_bind_source() so
they cannot drift, and agent_proposed_dir's doc-comment is corrected:
it claimed to be manager-editable and bind-mounted, and neither is true.
Both said a whole-hive graceful stop costs ONE `GRACEFUL_STOP_TIMEOUT`
in total because drains overlap. That is only true for a power op. In a
rebuild subtree the brace holds the build slot across the whole subtree,
drain included, so the boot sweep's per-agent drains serialise and the
sweep costs one timeout per wave of `buildSlots`.
Deleted rather than corrected. The right cost statement depends on an
operator knob and belongs in docs/coordinator.md if it belongs anywhere;
a comment that has to hedge about a config value is the kind that goes
stale silently. A comment saying nothing beats one that lies.
graph_snapshot previously filtered which whole roots got projected
based on the root node's own state, so a group root that was still
Running but had already-Done internal steps couldn't be filtered
down to just its live nodes, and a filtered-out root hid its entire
subtree even when a descendant still matched.
Apply the states filter after GraphWire::wire_snapshot instead, over
every node in the flattened tree, not just roots. The jobq-graph
client already handles an orphaned node (parent filtered out) by
promoting it to a rendered root, so this is safe on the client side
with no changes needed there.
Fixes hyperhive#3210
The parent's copy is for reading a child's config; a change to it is a PR
on the child's repo, made from a clone and merged after review. A
writable mount is a second path to the same file that skips that review,
which makes the boundary a convention rather than a permission.
Confirmed with ruth before flipping: it clones from the forge and opens a
PR, including for a brand-new child's first config.
The prose was the larger half. docs/approvals.md did not merely describe
the old mount, it *instructed* agents to use it ("can therefore edit,
commit, and submit changes for any of its direct children directly inside
its container"), and the doc comment in host_config.rs asserted a
dependency that never existed: the InitConfig seed runs as hive-c0re
against the host path, and read_only on a bind constrains writers inside
the container only. That comment is what produced issue #3206, now closed
as invalid.
`agents.conf` and `gateway.htpasswd` move from /var/lib/hyperhive/gateway
to /var/lib/hive-gateway/conf, alongside the `tls/` the gateway already
kept there.
nginx reads both as an unprivileged user. Under c0re's state dir it could
only reach them by traversing a directory systemd re-declares `0750
hive-core` on every c0re start — so nginx was given `SupplementaryGroups
= [ "hive-core" ]`, which also handed it read access to everything else
group-readable in that tree. The tokens are individually 0600, but the
broker sqlite carries no explicit mode: every message between every agent
was readable by the process whose job is parsing untrusted network input.
Moving the files removes the need and the exposure together. The group is
gone, and its absence is now commented as load-bearing so it doesn't come
back as a fix for a symptom it would recreate.
Also drops this module's `/var/lib/hyperhive` tmpfiles rule. It declared
`0755 root root` and could never win against `StateDirectoryMode`, and a
losing declaration still reads as a guarantee — that is what sent the
first diagnosis of the outage looking for who had changed the mode.
Ordering is unchanged and still the thing that makes a fresh boot work:
tmpfiles runs before services and seeds both files empty-but-valid, nginx
names them (an `include` of a missing file is fatal, not empty), and
content arrives when c0re writes and reloads — which it does on every
topology change, so a boot against the empty seed resolves itself.
Folds in the mode fix: `write` now sets 0644 on the tmp file before the
rename, because a rename carries the source's mode and discards the
destination's, and the tmpfiles rule that declares 0644 is
create-if-absent so it never re-applies.
Per review: docs represent current state. Every "used to" / "no longer"
clause this branch introduced is gone — including the History section in
network.md, which was a whole subsection about a sync mechanism that
doesn't exist.
Where the removed clause was carrying a real constraint, the constraint
stays and is stated in the present tense instead of as a delta: nothing
narrows what the gateway's nginx can reach except the directory
permissions in front of a socket, and nothing bounds `ReloadGatewayNginx`
except the hard-coded unit name. Those read as rules now rather than as
the story of how they came to be rules.
The gateway's nginx + dnsmasq no longer run in their own nspawn container.
`nix/host-modules/hive-gateway/default.nix` loses the
`containers.hive-gateway` wrapper and everything that existed only to punch
holes in it: `privateNetwork = false`, `CAP_NET_ADMIN`, five bind mounts,
its own `stateVersion`, `networking.firewall.enable = false`,
`networking.resolvconf.enable = false`, and the `hive-gateway-resolv`
path+service pair. 465 -> 303 lines.
The container never bought isolation here. It shared the host netns by
necessity — nginx binds the host's :80/:443, dnsmasq answers on the bridge —
so each of those settings was undoing a boundary the gateway could not
afford in the first place.
Four things made it more than a deletion, none of them visible in the nix
diff:
- The self-signed cert service also imports the hive CA leaf, so removing it
with the container would have left nginx naming a missing cert file, which
it refuses to load at all.
- The nginx reload is a hive-priv verb. It still needs root, but no longer
for the reason its doc gave, and `--machine=` was both transport and
scope — so the unit name is now hard-coded in the helper as the
containment.
- The lifecycle verb named a container that stops existing.
- `journalctl -M hive-gateway` had no machine to enter.
Per the operator's ruling, the operator verb keeps working and agents lose
it. `InfraContainer` answered three questions that used to share an answer;
it now splits into `name()` (identity), `target()` (Container vs HostUnit),
`service_unit()` (the systemd unit), and `agent_restartable()`, which the
MCP restart path checks before the capability so the refusal cannot read as
"ask for infra_admin". `SIBLING_CONTAINERS` drops the gateway — it gates the
requests that name a container as a string — while `FromStr` still accepts
it, because that answers what a name is, not who may act on it. The
dashboard's gateway journal reads host journald filtered to `nginx.service`.
Prose was corrected where it only named a location, and re-argued where the
container was doing security work: a `0666` per-agent socket was safe
because only the gateway container had the directory bind-mounted. There is
no mount now, so the directory permissions are the whole of the access
control — the constraint holds, its mechanism doesn't.
Gate: nix fmt / clippy --all-targets -D warnings / cargo test all clean (710
tests); hivectl-cli.md regenerated from the clap tree. The nix eval was run
in both TLS shapes at this commit: every delta in the rendered
virtualHosts is one of the three intended path moves, dnsmasq settings are
byte-identical, and the absence probe flips true -> false with bindMounts
emptied.
is_running collapsed every non-active state into false, so a container that
exhausted its bounded restarts read as plain "down" -- indistinguishable
from one an operator stopped deliberately. Bounding the restarts made that
gap sharper: a slow-failing agent used to grind on visibly, now it can stop
quietly.
Adds UnitState + unit_state() beside is_running rather than widening it.
is_running has ~8 call sites and nearly all are reconcile/power logic asking
"is it up? if not, start it" -- a question with two answers. Only the view
builder needs more, and it gets both facts from one systemctl call, since
is-active prints the state when not passed --quiet.
Surfaces as a flat failed flag on ContainerView and AgentStatusRow, matching
the shape those types already document: independent, orthogonally-observed
facts rather than a state machine. serde(default) keeps it order-independent
with the frontend half.
No behaviour change: nothing acts on the flag, per the ruling.
Closes the #3110 split — lib.rs is now just the crate doc comment and
the pub mod list.
journal.rs's new doc comment fixes a pre-existing bug: the old
JournalPriority doc text in lib.rs was actually half Capability's doc
(a leftover from an earlier reorder that moved the code but not the
comment above it).
GET /api/jobq/graph gains a states query param (comma-separated
hive_jobq::State names): narrows the served root groups to the named
states, keeping a group whole (filtering by a root's own state, which
is already its subtree's rolled-up answer). Absent, empty, or fully
unrecognised is the identity filter, matching prior behaviour.
hive-jobq-graph.js gains a row of per-state checkboxes above the tree,
re-fetching the endpoint with the selection on toggle. Default
selection hides Done and Skipped.
Server-side filtering (not client-side hiding) so hive-jobq-graph-update's
node list, and everything downstream of it in builds.js (count pill,
live-log panel), only ever sees what's actually shown.
hive_sh4re::assets::branding_svg() resolved a server-side default
icon at runtime from HIVE_ASSETS_DIR — the only consumer was
serve_icon(), which fell back to it whenever the agent had no
`hyperhive.icon` override. Removed both the fallback and the
function: serve_icon() now 404s when /etc/hyperhive/icon.svg is
absent, and the per-agent web UI (app.js) picks up the existing
dashboard swarm.js pattern — swap the <img> src to the
frontend-bundled /favicon.svg on load failure, guarded against
looping if the fallback itself 404s.
Updated the doc/comment claims that said the server always returns
an image (docs/web-ui/agent.md, nix/agent-modules/default.nix, the
hive-c0re/forge/users.rs comment referencing the old shared-asset
set). forge-avatar-sync and the matrix avatar sync are unaffected —
both are gated on hyperhive.icon != null and never depended on the
removed fallback.
bind_child_agent_dirs looped over state, harness and config alike and
mounted all three read-write, while the doc comment above it defended
only state. The rationale covered one dir, the loop covered three — the
uniformity is what erased the fact that the three have three different
answers.
harness holds the child's own runtime material (bash-tasks, the
turn-stats and event sqlite dbs) and nothing argues for a parent
touching it. The only other reader is stats::hive_stats, which reads the
host path directly and needs no mount into anyone.
config stays read-write here on purpose. The ruling is that it becomes
read-only, but request_init_config still has the manager seed a new
child's config in place, so flipping the mount before relocating that
step breaks agent creation hive-wide. That ordering now lives in the doc
comment, where someone about to finish the job in one line will see it.
docs/persistence.md justified all three dirs as RW; it now states the
boundary as three answers and names the right source file.
