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
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions libs/edge-toolkit/tests/registry.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//! Covers the `AgentRegistry` persistence + session-lookup paths the ws-server integration tests skip:
//! save/load round-trip (including the missing-file and populated branches), reconnect, `agent_session`,
//! and the `with_pending_direct_messages` builder. `S = String` stands in for the runtime session handle
//! (which is `#[serde(skip)]`, so it is never persisted).
#![cfg(test)]

use std::collections::BTreeMap;

use edge_toolkit::ws::AgentConnectionState;
use edge_toolkit::ws_server::{AgentRecord, AgentRegistry};
use tempfile::tempdir;

#[test]
fn save_load_roundtrip_and_session_lookup() {
let registry = AgentRegistry::<String>::default();

// Fresh connection (Assigned), then a reconnect for the same id which swaps in a new session.
let (agent_id, _assigned) = registry.connect_agent(None, "agent-1".to_string(), "127.0.0.1", "sess-a".to_string());
let (_same_id, _reconnected) = registry.connect_agent(
Some(agent_id.clone()),
"agent-x".to_string(),
"127.0.0.2",
"sess-b".to_string(),
);

assert_eq!(
registry.agent_session(&agent_id).as_deref(),
Some("sess-b"),
"reconnect keeps the newest session"
);
assert_eq!(registry.agent_session("nobody"), None, "unknown agent has no session");

// Persist and reload. Sessions are #[serde(skip)], so they return as None, but the agent survives.
let dir = tempdir().unwrap();
let path = dir.path().join("registry.yaml");
registry.save(&path).unwrap();
let reloaded = AgentRegistry::<String>::load(&path).unwrap();
assert_eq!(reloaded.agent_session(&agent_id), None, "sessions are not persisted");
Comment on lines +33 to +38

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the reload assertion prove that the record was persisted.

reloaded.agent_session(&agent_id) == None is also true for an entirely empty registry, so this does not verify that the agent survived the save/load round trip. Assert a persisted field through the registry's record lookup API, or inspect the serialized YAML if no such API exists; keep the session assertion separately.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/edge-toolkit/tests/registry.rs` around lines 33 - 38, Update the
persistence test around AgentRegistry::load so it first verifies the reloaded
registry contains the agent’s persisted record using the available record lookup
API, or serialized YAML when no lookup exists. Keep the existing agent_session
assertion separately to confirm sessions remain unpersisted.


// load() on a missing file yields an empty registry rather than erroring.
let empty = AgentRegistry::<String>::load(&dir.path().join("absent.yaml")).unwrap();
assert_eq!(empty.agent_session(&agent_id), None);
}

#[test]
fn agent_record_with_pending_builder_replaces_the_map() {
let record = AgentRecord::<String>::new(AgentConnectionState::Disconnected, None, None)
.with_pending_direct_messages(BTreeMap::new());
assert!(record.pending_direct_messages.is_empty());
}
Comment on lines +45 to +50

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Exercise replacement with a non-empty pending-message map.

The newly constructed record and the replacement map are both empty, so this test passes even if with_pending_direct_messages ignores its argument or fails to replace an existing map. Seed a non-empty map, then replace it with a distinct map and assert the exact result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/edge-toolkit/tests/registry.rs` around lines 45 - 50, Update
agent_record_with_pending_builder_replaces_the_map to initialize a non-empty
pending-message map, replace it with a distinct non-empty map through
with_pending_direct_messages, and assert pending_direct_messages exactly matches
the replacement map. Ensure the test would fail if the builder ignores its
argument or retains the original map.

3 changes: 3 additions & 0 deletions services/ws-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,8 @@ tracing-subscriber.workspace = true
utoipa = { workspace = true, optional = true }
uuid.workspace = true

[dev-dependencies]
tempfile.workspace = true

[lints]
workspace = true
1 change: 1 addition & 0 deletions services/ws-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pub use et_ws_service::{AgentSession, WsAgentRegistry};

pub mod config;
pub mod routes;
pub mod tls;

pub use self::routes::health;
use crate::config::Config;
Expand Down
3 changes: 1 addition & 2 deletions services/ws-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@ use clap::Parser;
use et_modules_service::list_modules;
use et_ws_server::config::Config;
use et_ws_server::configure_app;
use et_ws_server::tls;
use et_ws_service::load_registry;
use tracing::{error, info};
use tracing_actix_web::TracingLogger;
use tracing_subscriber::{layer::SubscriberExt as _, util::SubscriberInitExt as _};

mod tls;

#[derive(Parser, Debug)]
#[command(author, version, about, long_about = None)]
struct Args {
Expand Down
6 changes: 4 additions & 2 deletions services/ws-server/src/tls.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#![expect(
clippy::single_call_fn,
clippy::unwrap_used,
reason = "TLS bootstrap helpers are single-use by main() and panic on invalid PEM / cert-gen failure intentionally"
reason = "TLS bootstrap helpers panic on invalid PEM / cert-gen failure intentionally; callers are main + tests"
)]

use std::path::Path;
Expand All @@ -13,12 +12,14 @@ type CertKeyPair = (
rustls::pki_types::PrivateKeyDer<'static>,
);

#[must_use]
pub fn load_tls_certs(cert_filename: &Path, key_filename: &Path) -> CertKeyPair {
let cert_der = rustls::pki_types::CertificateDer::from_pem_file(cert_filename).unwrap();
let key_der = rustls::pki_types::PrivateKeyDer::from_pem_file(key_filename).unwrap();
(cert_der, key_der)
}

#[must_use]
pub fn generate_tls_certs(cert_filename: &Path, key_filename: &Path) -> CertKeyPair {
let certified = rcgen::generate_simple_self_signed(vec![
"localhost".to_string(),
Expand All @@ -35,6 +36,7 @@ pub fn generate_tls_certs(cert_filename: &Path, key_filename: &Path) -> CertKeyP
(cert_der, key_der)
}

#[must_use]
pub fn build_tls_server_config(
cert_der: rustls::pki_types::CertificateDer<'static>,
key_der: rustls::pki_types::PrivateKeyDer<'static>,
Expand Down
32 changes: 32 additions & 0 deletions services/ws-server/tests/tls.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//! Covers the TLS bootstrap helpers: self-signed generation, PEM round-trip load, and rustls config build.
#![cfg(test)]

use et_ws_server::tls::{build_tls_server_config, generate_tls_certs, load_tls_certs};
use tempfile::tempdir;

#[test]
fn generate_write_load_and_build_server_config() {
let dir = tempdir().unwrap();
let cert = dir.path().join("cert.pem");
let key = dir.path().join("key.pem");

// Generate a self-signed pair, which also writes both PEM files to disk.
let (gen_cert, gen_key) = generate_tls_certs(&cert, &key);
assert!(
cert.exists() && key.exists(),
"generate_tls_certs must write both PEM files"
);
let from_generated = build_tls_server_config(gen_cert, gen_key);
assert!(
from_generated.alpn_protocols.is_empty(),
"no ALPN protocols are configured by default"
);

// The freshly-written PEMs must load back into a der pair that also builds a valid config.
let (loaded_cert, loaded_key) = load_tls_certs(&cert, &key);
let from_loaded = build_tls_server_config(loaded_cert, loaded_key);
assert!(
from_loaded.alpn_protocols.is_empty(),
"reloaded config also has no ALPN protocols"
);
}
6 changes: 2 additions & 4 deletions utilities/int-gen/src/wit/upstream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,13 +272,11 @@ fn fetch_and_trim_webgpu(deps_root: &Path) -> Result<(), Error> {
/// Parse the upstream `webgpu.wit` via `wit-parser`, filter the parsed AST
/// down to our compute-only subset, and re-emit using `wit-encoder`.
#[expect(
clippy::unnecessary_wraps,
clippy::single_call_fn,
clippy::unwrap_used,
clippy::unwrap_in_result,
reason = "Result lets caller use ? like fetch_* helpers; called once; wit-parser anyhow::Error, inputs literals"
reason = "inputs are trusted (upstream WIT or the committed fixture); a parse failure is a bug, so unwrap"
)]
fn strip_webgpu(raw: &str) -> Result<String, Error> {
pub fn strip_webgpu(raw: &str) -> Result<String, Error> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MEDIUM RISK

Suggestion: The function signature is misleading because it never returns an Err. All parsing and encoding failures are handled by panicking, so wrapping the result in a Result adds unnecessary complexity for callers.

Try running the following prompt in your coding agent:

Simplify strip_webgpu in utilities/int-gen/src/wit/upstream.rs to return String instead of Result<String, Error>, update the return statement at the end of the function to remove the Ok() wrapper, and remove the clippy::unwrap_in_result expectation from the function's attribute block.

let mut resolve = wit_parser::Resolve::default();
let _stub: wit_parser::PackageId = resolve.push_str("wasi-io-stub.wit", WASI_IO_STUB).unwrap();
let _stub: wit_parser::PackageId = resolve
Expand Down
27 changes: 27 additions & 0 deletions utilities/int-gen/tests/webgpu_trim.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//! Exercises the `strip_webgpu` WIT-trimming pipeline on the committed compute-subset fixture.
//!
//! The upstream fetch (`run` / `fetch_*`) is network-bound, but the parse-filter-reemit core is pure, so we
//! drive it directly against the trimmed `webgpu.wit` under `generated/` -- a self-contained `wasi:webgpu`
//! package with records, variants, resources, enums and flags, which walks every arm of `mutate_interface`
//! and `collect_type_refs`.
#![cfg(test)]

use et_int_gen::wit::upstream::strip_webgpu;

#[test]
fn strips_webgpu_wit_and_reemits_the_package() {
// Anchor to the repo root (no relative-path literal) and read the committed fixture at runtime.
let wit_path = edge_toolkit::config::get_project_root().join("generated/specs/wit/deps/wasi-webgpu/webgpu.wit");
let raw = fs_err::read_to_string(&wit_path).unwrap();

let out = strip_webgpu(&raw).unwrap();
assert!(
out.contains("package wasi:webgpu"),
"re-emitted WIT should still declare the wasi:webgpu package, got:\n{out}"
);
// The trimmer keeps the compute resources (e.g. gpu-device) while dropping cross-package glue.
assert!(
out.contains("resource gpu-device"),
"expected the gpu-device resource to survive, got:\n{out}"
Comment on lines +17 to +25

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert that trimming removes an unwanted declaration.

These assertions only verify retained content, so a no-op implementation could still pass. Add a negative assertion for a known member from WEBGPU_DROP_METHODS that exists in the fixture but must be absent from out.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@utilities/int-gen/tests/webgpu_trim.rs` around lines 17 - 25, Add a negative
assertion in the strip_webgpu test after producing out, checking that a known
declaration or method from WEBGPU_DROP_METHODS present in the fixture is absent
from the trimmed output. Keep the existing assertions for retained package and
resource content unchanged, and use the known dropped member rather than an
arbitrary string.

);
}
Loading