chore: migrate to master, enforce ci - #1
Closed
georgewhewell wants to merge 116 commits into
Closed
Conversation
Collapse the remote execution stack onto the canonical graph request shape, move quote discovery into the rpc crate, and remove the CLI-only discovery split. This also folds in the executor/client fixes needed to make the new path work end-to-end.
Document the deny-by-default serve flow, surface the active policy mode at startup, and add the Nix/docker packaging helpers that make the new deployment shape usable.
…s everywhere
OpenTelemetry plumbing is now opt-in via the `otel` feature on `hellas-cli`
(default off). With the feature off, none of opentelemetry / opentelemetry_sdk
/ opentelemetry-otlp / tracing-opentelemetry / reqwest compile, and the
trace-context propagation glue collapses to identity. Plain `tracing::info!`
/ `warn!` / etc. macros stay unconditional — they're no-op-cheap without a
subscriber.
Cfg surface is concentrated, not sprinkled:
- One function pair in `tracing_config.rs` (`install_with_otel`); registry
composition stays cfg-free. `TracerGuard` newtype hides the
`Option<SdkTracerProvider>` behind a cfg-gated field so `main.rs` drops the
`if let Some(provider) = ...` dance around shutdown.
- `execution.rs`: cfg-swap of the `TracedChannel` type alias plus a
`traced(channel)` helper collapses 8 `InterceptedService::new(channel,
TraceContextInjector)` sites and avoids spreading cfg across the file.
- `serve/node.rs`: a single `traced_service<S>` helper replaces 5
`trace_layer.layer(...)` sites.
iroh's internal `EndpointMetrics` are bridged into the existing
`prometheus-client` registry exposed at `/metrics`. The cli's `otel` feature
also enables `tonic-iroh-transport/metrics`, and `serve` attaches
`endpoint.metrics()` (clone of Arcs into live storage) via
`MetricsBundle::with_iroh`. The HTTP handler emits prometheus-client text
followed by iroh's OpenMetrics text in one well-formed response with a single
`# EOF` terminator. Verified end-to-end: `endpoint_socket_send_ipv4_total`
etc. show up alongside `hellas_*` counters.
Switch all TLS to rustls so the crate compiles in weird places (wasm,
cross-compile, no system openssl):
- Workspace `tonic-iroh-transport`: drop `["otel", "native-defaults"]`,
pin to v0.9.2, use granular features `["tls-ring", "portmapper",
"fast-apple-datapath"]`. v0.9.2 exposes the new passthroughs.
- Workspace `reqwest`: switch to `["rustls", "webpki-roots"]` (was
`["rustls-native-certs"]`, which lacks an actual TLS provider — the cause
of the `"invalid URL, scheme is not http"` symptom against
jaeger.lsd-ag.ch).
- `opentelemetry-otlp`: add `"reqwest-rustls-webpki-roots"` so its internal
`reqwest 0.12` (separate from our 0.13) gets a TLS provider too.
- `crates/executor/Cargo.toml`: `hf-hub = "0.5"` was secretly pulling
`native-tls` -> `openssl-sys` via default features. Pin to
`default-features = false, features = ["ureq"]`, matching `hellas-rpc`.
- Drop `pkgs.openssl` from `nix/default.nix`, `nix/docker.nix`,
`nix/package.nix`. `ldd target/debug/hellas-cli` is now empty for
`libssl`/`libcrypto` in both default and `candle,otel` builds.
Dev workflow:
- `rust-analyzer.toml` at workspace root pins RA's feature set to
`["candle", "otel"]` so type-checking covers gated modules across
editors. Replaces the abandoned `HELLAS_FEATURES` env var / cargo shim
approach.
- `nix/default.nix:88`: `hellas-run` wrapper drops
`--features "${HELLAS_FEATURES:-candle}"` in favor of explicit
`--features candle`.
Build matrix: all four cli feature combos compile (`{}`, `candle`, `otel`,
`candle,otel`); workspace check + clippy clean; HTTPS OTLP connect now
succeeds (DNS to jaeger.lsd-ag.ch is environmental).
- expose individual check-{fmt,clippy,sort,test} apps for matrix dispatch
- add `cargo test --workspace` check (default features)
- workflow enumerates check-* apps from the flake, runs each on
`[self-hosted, shared]`; gates with `CI passed` aggregate job
- opt-in `cache.hellas.ai` substituter via flake.nix nixConfig
writeShellApplication strips PATH down to runtimeInputs only, so cargo's default linker invocation (`cc`) was failing on the runner.
Single source of truth in nix/ci.nix is now an attrset
{ name -> { check, fix? } }. Exposed flat as `.#ci.<system>.commands`
for the GitHub Actions matrix to enumerate. CI runs each command via
`nix develop -c` so the dev shell is the runtime environment — no
more per-check writeShellApplication wrappers with hand-curated input
lists.
Workflow gains a `devshell` warmup job (clean failure surface for
env issues) and drops `--accept-flake-config` (runner daemon already
trusts cache.hellas.ai; flake nixConfig is for downstream users).
`nix run .#fix` previously fell back to running the check command for entries without a `fix` field — so it ran cargo test (slow) and cargo outdated (heuristic) during fix mode. Now those are filtered out entirely; fix only runs entries with an explicit fix variant.
Mechanical changes from `nix run .#fix`: rustfmt across the workspace, cargo-sort across all Cargo.toml files, and clippy's --fix for the auto-resolvable lints (collapsible if/let chains).
EnqueueError / StartExecutionError now wrap ExecuteJob in Box (was ~232 bytes inline). PreparedRoute / OpaquePreparedRoute box the RemoteDirect variant which held a ~1KB RemoteExecution. The remaining variant-size disparity in the route enums is annotated with `#[allow(clippy::large_enum_variant)]` — the variants are heterogeneous by nature and the enum lives only briefly during execution setup.
After `CI passed`, on push only, build the slow targets in parallel: static-x86_64 cross.x86_64-linux-musl.cli static-aarch64 cross.aarch64-linux-musl.cli docker-cpu default cpu image docker-cuda alias of docker-cuda12-sm89 (new) Matrix is driven from `.#ci.<sys>.builds` (same data-driven pattern as `commands`). `Extended builds passed` is a separate gate from `CI passed` so branch protection can require them independently.
georgewhewell
force-pushed
the
grw/feat/aiter
branch
7 times, most recently
from
May 11, 2026 15:39
120cd35 to
9f36f95
Compare
georgewhewell
force-pushed
the
grw/feat/aiter
branch
from
May 11, 2026 16:18
9f36f95 to
2f50145
Compare
|
Hydra reported failures for this PR.
Failed builds:
Update: hellas/node-pr-1, eval #760, head |
georgewhewell
force-pushed
the
master
branch
from
September 11, 2026 18:53
e78eefb to
5d88bf5
Compare
georgewhewell
added a commit
that referenced
this pull request
Sep 11, 2026
…_FINDINGS #1 + #5) The old `ExecutionRuntime` was a bag-of-Options that lied about what it supported. `default().with_secret_key(sk)` produced a remote- capable-looking value that couldn't dial — the `secret_key` field was stored but never used to build an endpoint. All three remote callers (llm, opaque, gateway) constructed exactly this dead shape. `with_registry` existed for "external code that builds its own registry", which nothing in-tree does. `RemoteNodeTarget` stored `Vec<SocketAddr>` as a parallel structure to iroh's own `EndpointAddr`, and the `endpoint_addr()` adapter built to bridge the two was `#[allow(dead_code)]` because no call site reached it. This commit collapses the three pieces: 1. `ExecutionRuntime` shrinks to two fields: ``` struct ExecutionRuntime { #[cfg(feature = "hellas-executor")] local_executor: Option<ExecutorHandle>, remote: Option<RemoteRpc { endpoint, registry }>, } ``` `RemoteRpc` owns the bound iroh `Endpoint` and the `ServiceRegistry` built atop it as one unit — no half-built state. Constructors: - `local(handle)` for local-only. - `remote(secret_key, seed_targets).await?` for remote-only. - `with_remote(self, sk, seed).await?` adds remote dialing to an existing (typically local) runtime; used by the verify-against-local path. Deleted: `secret_key` field, `with_secret_key`, `with_registry`, `require_registry`. They were the inert composition the review flagged. 2. New helper `remote_transport<S: ServiceMarker>(&self, target) -> anyhow::Result<IrohTransport>`. Every `*Direct::quote_*` / `*Direct::opaque_*` call site now goes through this one path instead of replicating `registry.pool::<S>().transport(target.node_id).await.map_err(|e| anyhow!(e).context(...))` boilerplate per service. ~7 sites collapse from 4-6 lines each to 1. 3. `RemoteNodeTarget` becomes just `{ addr: EndpointAddr }` — the address-bundle iroh expects at `Endpoint::connect` time. Direct CLI-supplied IPs go into `EndpointAddr::from_parts(id, ip_hints)` at the CLI parse boundary (in `ExecutionRoute::remote`) and never enter `PeerManager` / `PeerDirectory`. iroh owns routing — `presets::N0` already configures pkarr/DNS lookup, so `EndpointId`-only routing works when iroh can resolve the peer. `RemoteNodeTarget::node_id()` is the only accessor. Deleted: `node_addrs: Vec<SocketAddr>` field, the dead `endpoint_addr()` helper, the `TransportAddr` thread-through. 4. `wire/src/iroh/pool.rs`: `Pool::connection` and `Pool::transport` now take `impl Into<EndpointAddr>`. The internal `endpoint.connect` already accepted `impl Into<EndpointAddr>`, so this is a clean widening. Cache keys remain `EndpointId` — hints are dial-time only, not part of the cache identity (an `EndpointAddr` with stale IPs reuses a live connection cached under the same `EndpointId`). `From<EndpointId> for EndpointAddr` (existing in iroh-base) handles the migration so all old `pool.transport(id)` call sites still compile after the change. 5. Three broken call sites fixed: - `cli/commands/llm.rs:107`: `default().with_secret_key(sk)` → `ExecutionRuntime::remote(sk, vec![]).await?` (or `local(...).with_remote(sk, vec![]).await?` when verify_local is set). - `cli/commands/opaque.rs:53`: same. - `cli/commands/gateway/state.rs:100`: `from_options` is now `async fn`; `mod.rs` updated to `.await?` it. Behavioural change: `hellas execute --remote`, `hellas llm`, and `hellas gateway` now actually dial when given a `--node-id`. Before this commit they always errored out with "no iroh ServiceRegistry is configured on the runtime". Tests: - `cargo test -p hellas-cli --features hellas-executor` → 58 (unchanged — the runtime-construction tests stayed in shape through the refactor). - `cargo test -p hellas-wire --all-features` → 57. - `cargo test -p hellas-rpc --all-features --lib` → 73. - `cargo build --workspace` clean (no warnings from our crates). Diffstat: 6 files, +180 / -156. Net: **+24 LOC** — modestly subtractive after accounting for the new `with_remote` builder required by the verify-against-local composition. The "delete dead helpers + collapse boilerplate into `remote_transport`" budget delivered ~80 LOC reduction; that's mostly offset by the new builder + the dedicated `RemoteRpc` struct + doc comments. Net LOC isn't the only measure: the runtime can no longer be constructed in a broken state, and a previously-bug-prone area (pool dial boilerplate) is now factored through one helper.
georgewhewell
added a commit
that referenced
this pull request
Sep 11, 2026
… rule Adversarial panel (Codex/gpt-5.3-codex-spark, Grok, nemotron-3-ultra) reviewed 8b5cd03..HEAD. Confirmed findings fixed: - StakeBondTerms commits challenge_margin (window + inclusion + finality, blocks); JobAcceptanceContext::covered_by is the admission rule (1 <= p_j <= max_job_price, terminal_deadline + margin <= bond timeout) and FraudArtifact::binds re-checks it — closes the stall-past-timeout escape's policy gap (Grok #3, Codex #1). - Kernel open rejects party-controlled treasuries (TreasuryIsParty): treasury == provider would collapse the slash penalty from S to A (Grok #2). - Kernel open rejects max_job_price == 0 (JobPriceCapZero): kills the A = C_disp strict-incentive degeneracy (Grok #7). - CloseKindSet::decode refuses no-exit sets (defense in depth, Grok #12). - Bond e2e uses STAKE_BOND_PROTOCOL, not the payment code (Grok #13). - preverified-seals feature doc: process-local cache, single-node dev only, never multi-validator (Grok #1 — by design, now explicit). Rejected with evidence: cross-bond artifact replay (binds checks bond_edge == public.edge_id), seal-omits-treasury (acceptance digest commits bond_terms which commits it), SealPublicInputs malleability (terms hash-checked against edge pre-verify), VoucherBook stale race (kernel payload hash binds the edge; older voucher pays provider less), stake==net-value as fee bug (plan defines S net).
georgewhewell
added a commit
that referenced
this pull request
Sep 11, 2026
…_FINDINGS #1 + #5) The old `ExecutionRuntime` was a bag-of-Options that lied about what it supported. `default().with_secret_key(sk)` produced a remote- capable-looking value that couldn't dial — the `secret_key` field was stored but never used to build an endpoint. All three remote callers (llm, opaque, gateway) constructed exactly this dead shape. `with_registry` existed for "external code that builds its own registry", which nothing in-tree does. `RemoteNodeTarget` stored `Vec<SocketAddr>` as a parallel structure to iroh's own `EndpointAddr`, and the `endpoint_addr()` adapter built to bridge the two was `#[allow(dead_code)]` because no call site reached it. This commit collapses the three pieces: 1. `ExecutionRuntime` shrinks to two fields: ``` struct ExecutionRuntime { #[cfg(feature = "hellas-executor")] local_executor: Option<ExecutorHandle>, remote: Option<RemoteRpc { endpoint, registry }>, } ``` `RemoteRpc` owns the bound iroh `Endpoint` and the `ServiceRegistry` built atop it as one unit — no half-built state. Constructors: - `local(handle)` for local-only. - `remote(secret_key, seed_targets).await?` for remote-only. - `with_remote(self, sk, seed).await?` adds remote dialing to an existing (typically local) runtime; used by the verify-against-local path. Deleted: `secret_key` field, `with_secret_key`, `with_registry`, `require_registry`. They were the inert composition the review flagged. 2. New helper `remote_transport<S: ServiceMarker>(&self, target) -> anyhow::Result<IrohTransport>`. Every `*Direct::quote_*` / `*Direct::opaque_*` call site now goes through this one path instead of replicating `registry.pool::<S>().transport(target.node_id).await.map_err(|e| anyhow!(e).context(...))` boilerplate per service. ~7 sites collapse from 4-6 lines each to 1. 3. `RemoteNodeTarget` becomes just `{ addr: EndpointAddr }` — the address-bundle iroh expects at `Endpoint::connect` time. Direct CLI-supplied IPs go into `EndpointAddr::from_parts(id, ip_hints)` at the CLI parse boundary (in `ExecutionRoute::remote`) and never enter `PeerManager` / `PeerDirectory`. iroh owns routing — `presets::N0` already configures pkarr/DNS lookup, so `EndpointId`-only routing works when iroh can resolve the peer. `RemoteNodeTarget::node_id()` is the only accessor. Deleted: `node_addrs: Vec<SocketAddr>` field, the dead `endpoint_addr()` helper, the `TransportAddr` thread-through. 4. `wire/src/iroh/pool.rs`: `Pool::connection` and `Pool::transport` now take `impl Into<EndpointAddr>`. The internal `endpoint.connect` already accepted `impl Into<EndpointAddr>`, so this is a clean widening. Cache keys remain `EndpointId` — hints are dial-time only, not part of the cache identity (an `EndpointAddr` with stale IPs reuses a live connection cached under the same `EndpointId`). `From<EndpointId> for EndpointAddr` (existing in iroh-base) handles the migration so all old `pool.transport(id)` call sites still compile after the change. 5. Three broken call sites fixed: - `cli/commands/llm.rs:107`: `default().with_secret_key(sk)` → `ExecutionRuntime::remote(sk, vec![]).await?` (or `local(...).with_remote(sk, vec![]).await?` when verify_local is set). - `cli/commands/opaque.rs:53`: same. - `cli/commands/gateway/state.rs:100`: `from_options` is now `async fn`; `mod.rs` updated to `.await?` it. Behavioural change: `hellas execute --remote`, `hellas llm`, and `hellas gateway` now actually dial when given a `--node-id`. Before this commit they always errored out with "no iroh ServiceRegistry is configured on the runtime". Tests: - `cargo test -p hellas-cli --features hellas-executor` → 58 (unchanged — the runtime-construction tests stayed in shape through the refactor). - `cargo test -p hellas-wire --all-features` → 57. - `cargo test -p hellas-rpc --all-features --lib` → 73. - `cargo build --workspace` clean (no warnings from our crates). Diffstat: 6 files, +180 / -156. Net: **+24 LOC** — modestly subtractive after accounting for the new `with_remote` builder required by the verify-against-local composition. The "delete dead helpers + collapse boilerplate into `remote_transport`" budget delivered ~80 LOC reduction; that's mostly offset by the new builder + the dedicated `RemoteRpc` struct + doc comments. Net LOC isn't the only measure: the runtime can no longer be constructed in a broken state, and a previously-bug-prone area (pool dial boilerplate) is now factored through one helper.
georgewhewell
added a commit
that referenced
this pull request
Sep 11, 2026
… rule Adversarial panel (Codex/gpt-5.3-codex-spark, Grok, nemotron-3-ultra) reviewed 8b5cd03..HEAD. Confirmed findings fixed: - StakeBondTerms commits challenge_margin (window + inclusion + finality, blocks); JobAcceptanceContext::covered_by is the admission rule (1 <= p_j <= max_job_price, terminal_deadline + margin <= bond timeout) and FraudArtifact::binds re-checks it — closes the stall-past-timeout escape's policy gap (Grok #3, Codex #1). - Kernel open rejects party-controlled treasuries (TreasuryIsParty): treasury == provider would collapse the slash penalty from S to A (Grok #2). - Kernel open rejects max_job_price == 0 (JobPriceCapZero): kills the A = C_disp strict-incentive degeneracy (Grok #7). - CloseKindSet::decode refuses no-exit sets (defense in depth, Grok #12). - Bond e2e uses STAKE_BOND_PROTOCOL, not the payment code (Grok #13). - preverified-seals feature doc: process-local cache, single-node dev only, never multi-validator (Grok #1 — by design, now explicit). Rejected with evidence: cross-bond artifact replay (binds checks bond_edge == public.edge_id), seal-omits-treasury (acceptance digest commits bond_terms which commits it), SealPublicInputs malleability (terms hash-checked against edge pre-verify), VoucherBook stale race (kernel payload hash binds the edge; older voucher pays provider less), stake==net-value as fee bug (plan defines S net).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.