diff --git a/.deepsource.toml b/.deepsource.toml index e175aaa7..f2dfafc7 100644 --- a/.deepsource.toml +++ b/.deepsource.toml @@ -1,10 +1,15 @@ version = 1 # `pkg/` is deliberately NOT excluded from DeepSource analysis. -# It holds hand-written module loaders (e.g. the Pyodide shim et_ws_pydata1.js) alongside build output. Only the -# int-gen outputs under `generated/` and the scenario `verification/` fixtures are excluded, since those are fully -# generated; if genuinely-generated glue under `pkg/` turns up as a finding, add a narrow per-file exclude then. -exclude_patterns = ["generated/**", "verification/**"] +# It holds hand-written module loaders (e.g. the Pyodide shim et_ws_pydata1.js) alongside build output. The +# int-gen outputs under `generated/` and the scenario `verification/` fixtures are excluded because they are +# fully generated; if genuinely-generated glue under `pkg/` turns up as a finding, add a narrow per-file +# exclude then. +# `**/error.rs` is excluded from coverage tracking: these files are thin error-type plumbing (thiserror +# `#[derive]` + `#[from]` conversions, plus the occasional unreachable defensive `From`), so their line +# coverage is noise. No Rust static analyzer is configured here -- DeepSource only tracks Rust coverage -- so +# the exclude costs no issue detection. (The matching Codecov exclusion is set in Codecov's web UI.) +exclude_patterns = ["**/error.rs", "generated/**", "verification/**"] # Repo convention: tests live in a `tests/` directory or in source files prefixed `test_`. test_patterns = ["**/test_*.py", "**/tests/**"] diff --git a/CLAUDE.md b/CLAUDE.md index aa404f30..fc1cf3e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -203,6 +203,12 @@ running the whole battery for every edit is slow, expensive, and almost always w types changed. Pick the targeted tasks from the tables below that match the extensions of the files you actually modified, and run only those. Same goes for `fmt`: run the per-file-type formatter, not the aggregate. +**Always run `mise run semgrep-check` before finishing a set of changes to any files** -- it is the one check to +run regardless of which file types you touched. It carries repo-wide, cross-cutting rules that the per-file-type +tasks don't (comment summary lines across TOML/YAML/JSON/C, the `no-non-ascii` ban, banned constructs in +`generic` mode over config text), and it is cheap. Skipping it is how comment-style and cross-file regressions +slip through to CI. + **Agents must not invoke formatter/linter binaries directly** (e.g. `mise exec -- taplo format`, raw `taplo`/`dprint`/`oxfmt`/`cargo fmt` calls). Always use the corresponding mise task (`mise run taplo-fmt`, `mise run taplo-check`, `mise run dprint-fmt`, etc.). The tasks carry the project's config-file paths @@ -883,6 +889,16 @@ When the rewrite can't be expressed as a single template (multiple match shapes, structural restructuring), keep the rule check-only and write a brief note in the rule body explaining why the autofix wasn't viable. +### Expand `doc-summary-ends-with-period` as you touch Rust files + +`config/ast-grep/rules/doc-summary-ends-with-period.yaml` enforces that a doc comment's first line is a +one-line summary ending in terminal punctuation (`.`, `!`, or `?`). The workspace had a large pre-existing +backlog of violations, so the rule is scoped by a `files:` allowlist rather than applied workspace-wide, and it +is rolled out incrementally. **Whenever you modify a Rust file, add it to that rule's `files:` list (keep the +list sorted) and fix any first-line-summary violations in it as part of the same change** -- so the rule's +coverage only ever grows. Once the `files:` list covers effectively everything, drop the `files:` scoping and +let it apply workspace-wide. + ### NEVER delete a lint rule without explicit user permission **Do not delete a rule from `config/ast-grep/rules/`, `config/semgrep/`, `config/conftest/policy/`, `config/taplo/`, diff --git a/Cargo.lock b/Cargo.lock index 23dd0fd1..108b9b1d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1351,7 +1351,7 @@ dependencies = [ "num-traits", "serde", "wasm-bindgen", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -1509,6 +1509,18 @@ dependencies = [ "memchr", ] +[[package]] +name = "command-error" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b6ac3abfcf15b4536b079bcee923683fe3dc1173b70be5e05ccf28ca112862e" +dependencies = [ + "dyn-clone", + "process-wrap", + "shell-words", + "utf8-command", +] + [[package]] name = "compact_str" version = "0.7.1" @@ -4043,7 +4055,9 @@ version = "0.1.0" dependencies = [ "asyncapi-rust", "base64 0.22.1", + "command-error", "et-path", + "et-test-helpers", "fs-err", "humantime-serde", "log", @@ -4058,7 +4072,6 @@ dependencies = [ "serde_path_to_error", "serde_yaml", "strum", - "temp-env", "tempfile", "testing_logger", "thiserror 2.0.18", @@ -4205,9 +4218,11 @@ dependencies = [ "asyncapi-rust", "clap", "clap-markdown", + "command-error", "edge-toolkit", "et-modules-service", "et-storage-service", + "et-test-helpers", "et-ws-server", "fs-err", "heck", @@ -4333,6 +4348,7 @@ version = "0.1.0" dependencies = [ "port_check", "retry", + "temp-env", ] [[package]] @@ -4518,6 +4534,7 @@ version = "0.1.0" dependencies = [ "backon", "base64 0.22.1", + "command-error", "edge-toolkit", "et-rest-client", "et-ws-runner-common", @@ -4542,6 +4559,7 @@ version = "0.1.0" dependencies = [ "edge-toolkit", "et-rest-client", + "et-test-helpers", "futures-util", "humantime-serde", "reqwest 0.13.4", @@ -4552,7 +4570,6 @@ dependencies = [ "serde_default", "serde_json", "serde_path_to_error", - "temp-env", "thiserror 2.0.18", "tokio", "tokio-tungstenite", @@ -4624,6 +4641,7 @@ dependencies = [ "bytesize", "chrono", "edge-toolkit", + "et-test-helpers", "fs-err", "futures-util", "opentelemetry 0.31.0", @@ -4633,7 +4651,6 @@ dependencies = [ "serde_default", "serde_json", "serde_yaml", - "temp-env", "tokio", "tracing", "uuid", @@ -4727,6 +4744,7 @@ version = "0.1.0" dependencies = [ "async-trait", "bytemuck", + "command-error", "edge-toolkit", "et-otlp", "et-rest-client", @@ -4786,6 +4804,7 @@ name = "et-ws-web-runner" version = "0.1.0" dependencies = [ "cc", + "command-error", "deno_core", "deno_error", "deno_resolver", @@ -5383,7 +5402,7 @@ dependencies = [ "log", "presser", "thiserror 2.0.18", - "windows", + "windows 0.62.2", ] [[package]] @@ -5699,7 +5718,7 @@ checksum = "617aaa3557aef3810a6369d0a99fac8a080891b68bd9f9812a1eeda0c0730cbd" dependencies = [ "cfg-if", "libc", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -5878,7 +5897,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core", + "windows-core 0.62.2", ] [[package]] @@ -6199,6 +6218,9 @@ dependencies = [ [[package]] name = "int-wasm-cov-wrapper" version = "0.1.0" +dependencies = [ + "command-error", +] [[package]] name = "inventory" @@ -6234,7 +6256,7 @@ dependencies = [ "socket2 0.6.4", "widestring", "windows-registry", - "windows-result", + "windows-result 0.4.1", "windows-sys 0.61.2", ] @@ -6326,7 +6348,7 @@ dependencies = [ "simd_cesu8", "thiserror 2.0.18", "walkdir", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -6625,7 +6647,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" dependencies = [ "cfg-if", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -7769,7 +7791,7 @@ dependencies = [ "libc", "redox_syscall 0.5.18", "smallvec 1.15.2", - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -8190,6 +8212,18 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "process-wrap" +version = "8.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a3ef4f2f0422f23a82ec9f628ea2acd12871c81a9362b02c43c1aa86acfc3ba1" +dependencies = [ + "indexmap", + "nix 0.30.1", + "tracing", + "windows 0.61.3", +] + [[package]] name = "profiling" version = "1.0.18" @@ -9818,6 +9852,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shell-words" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc6fe69c597f9c37bfeeeeeb33da3530379845f10be461a66d16d03eca2ded77" + [[package]] name = "shlex" version = "1.3.0" @@ -11559,6 +11599,12 @@ version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09cc8ee72d2a9becf2f2febe0205bbed8fc6615b7cb429ad062dc7b7ddd036a9" +[[package]] +name = "utf8-command" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4b151524c94cda49046b29e6d20b03092ff9363b02acc1bf3994da60910c55b" + [[package]] name = "utf8_iter" version = "1.0.4" @@ -12521,9 +12567,9 @@ dependencies = [ "web-sys", "wgpu-naga-bridge", "wgpu-types", - "windows", - "windows-core", - "windows-result", + "windows 0.62.2", + "windows-core 0.62.2", + "windows-result 0.4.1", ] [[package]] @@ -12686,16 +12732,38 @@ dependencies = [ "wasmtime-internal-cranelift", ] +[[package]] +name = "windows" +version = "0.61.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9babd3a767a4c1aef6900409f85f5d53ce2544ccdfaa86dad48c91782c6d6893" +dependencies = [ + "windows-collections 0.2.0", + "windows-core 0.61.2", + "windows-future 0.2.1", + "windows-link 0.1.3", + "windows-numerics 0.2.0", +] + [[package]] name = "windows" version = "0.62.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "527fadee13e0c05939a6a05d5bd6eec6cd2e3dbd648b9f8e447c6518133d8580" dependencies = [ - "windows-collections", - "windows-core", - "windows-future", - "windows-numerics", + "windows-collections 0.3.2", + "windows-core 0.62.2", + "windows-future 0.3.2", + "windows-numerics 0.3.1", +] + +[[package]] +name = "windows-collections" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3beeceb5e5cfd9eb1d76b381630e82c4241ccd0d27f1a39ed41b2760b255c5e8" +dependencies = [ + "windows-core 0.61.2", ] [[package]] @@ -12704,7 +12772,20 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23b2d95af1a8a14a3c7367e1ed4fc9c20e0a26e79551b1454d72583c97cc6610" dependencies = [ - "windows-core", + "windows-core 0.62.2", +] + +[[package]] +name = "windows-core" +version = "0.61.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0fdd3ddb90610c7638aa2b3a3ab2904fb9e5cdbecc643ddb3647212781c4ae3" +dependencies = [ + "windows-implement", + "windows-interface", + "windows-link 0.1.3", + "windows-result 0.3.4", + "windows-strings 0.4.2", ] [[package]] @@ -12715,9 +12796,20 @@ checksum = "b8e83a14d34d0623b51dce9581199302a221863196a1dde71a7663a4c2be9deb" dependencies = [ "windows-implement", "windows-interface", - "windows-link", - "windows-result", - "windows-strings", + "windows-link 0.2.1", + "windows-result 0.4.1", + "windows-strings 0.5.1", +] + +[[package]] +name = "windows-future" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc6a41e98427b19fe4b73c550f060b59fa592d7d686537eebf9385621bfbad8e" +dependencies = [ + "windows-core 0.61.2", + "windows-link 0.1.3", + "windows-threading 0.1.0", ] [[package]] @@ -12726,9 +12818,9 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e1d6f90251fe18a279739e78025bd6ddc52a7e22f921070ccdc67dde84c605cb" dependencies = [ - "windows-core", - "windows-link", - "windows-threading", + "windows-core 0.62.2", + "windows-link 0.2.1", + "windows-threading 0.2.1", ] [[package]] @@ -12753,20 +12845,36 @@ dependencies = [ "syn 2.0.118", ] +[[package]] +name = "windows-link" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e6ad25900d524eaabdbbb96d20b4311e1e7ae1699af4fb28c17ae66c80d798a" + [[package]] name = "windows-link" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-numerics" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9150af68066c4c5c07ddc0ce30421554771e528bde427614c61038bc2c92c2b1" +dependencies = [ + "windows-core 0.61.2", + "windows-link 0.1.3", +] + [[package]] name = "windows-numerics" version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6e2e40844ac143cdb44aead537bbf727de9b044e107a0f1220392177d15b0f26" dependencies = [ - "windows-core", - "windows-link", + "windows-core 0.62.2", + "windows-link 0.2.1", ] [[package]] @@ -12775,9 +12883,18 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "02752bf7fbdcce7f2a27a742f798510f3e5ad88dbe84871e5168e2120c3d5720" dependencies = [ - "windows-link", - "windows-result", - "windows-strings", + "windows-link 0.2.1", + "windows-result 0.4.1", + "windows-strings 0.5.1", +] + +[[package]] +name = "windows-result" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56f42bd332cc6c8eac5af113fc0c1fd6a8fd2aa08a0119358686e5160d0586c6" +dependencies = [ + "windows-link 0.1.3", ] [[package]] @@ -12786,7 +12903,16 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" dependencies = [ - "windows-link", + "windows-link 0.2.1", +] + +[[package]] +name = "windows-strings" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56e6c93f3a0c3b36176cb1327a4958a0353d5d166c2a35cb268ace15e91d3b57" +dependencies = [ + "windows-link 0.1.3", ] [[package]] @@ -12795,7 +12921,7 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7837d08f69c77cf6b07689544538e017c1bfcf57e34b4c0ff58e6c2cd3b37091" dependencies = [ - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -12831,7 +12957,7 @@ version = "0.61.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae137229bcbd6cdf0f7b80a31df61766145077ddf49416a728b02cb3921ff3fc" dependencies = [ - "windows-link", + "windows-link 0.2.1", ] [[package]] @@ -12856,7 +12982,7 @@ version = "0.53.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4945f9f551b88e0d65f3db0bc25c33b8acea4d9e41163edf90dcd0b19f9069f3" dependencies = [ - "windows-link", + "windows-link 0.2.1", "windows_aarch64_gnullvm 0.53.1", "windows_aarch64_msvc 0.53.1", "windows_i686_gnu 0.53.1", @@ -12867,13 +12993,22 @@ dependencies = [ "windows_x86_64_msvc 0.53.1", ] +[[package]] +name = "windows-threading" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b66463ad2e0ea3bbf808b7f1d371311c80e115c0b71d60efc142cafbcfb057a6" +dependencies = [ + "windows-link 0.1.3", +] + [[package]] name = "windows-threading" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3949bd5b99cafdf1c7ca86b43ca564028dfe27d66958f2470940f73d86d75b37" dependencies = [ - "windows-link", + "windows-link 0.2.1", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index ad5d5697..383f144d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ cc = "1" chrono = { version = "0.4", features = ["serde"] } clap = { version = "4.4", features = ["derive"] } clap-markdown = "0.1.5" +command-error = "0.8" deno_core = "0.407.0" deno_error = "=0.7.1" deno_resolver = "0.85.0" @@ -435,6 +436,28 @@ ignore = [ # Not in declared repo (gfx-rs/wgpu): a per-platform helper crate from the wgpu monorepo. # cargo-unmaintained can't find the individual member at the repo root -- false positive; pulled by wgpu-core. "wgpu-core-deps-windows-linux-android", + # windows-targets and the windows_* import-library crates are sunset false positives, not abandonment. + # They declare `repository = https://github.com/microsoft/windows-rs`, but Microsoft deleted them from that + # repo on 2026-07-20 (commit 201b9eaa7, "windows-targets" #4722), superseding them with `windows-link`. + # The final 0.52.x/0.53.x versions stay published on crates.io; since the declared repo no longer holds a + # Cargo.toml for them, cargo-unmaintained's "package in repo" heuristic reports "not in" -- a false positive + # for deliberately-sunset, still-functional crates, not abandonment. + # + # These can't be bumped out of the lock: they are pulled only transitively, via older windows-sys majors + # (0.52/0.59/0.60, all pre-windows-link) that upstream crates hard-require -- `ring` 0.17.14 pins + # windows-sys `^0.52` (via rcgen -> et-ws-server) and `notify` 8.2.0 pins `^0.60.1` (via deno_runtime), and + # neither has a windows-link-based release yet, so `cargo update` refuses windows-sys 0.61. Bumping + # windows-targets itself is futile -- no version of it exists in the repo any more. Delete these entries once + # the upstream stack adopts windows-link and the crates drop out of the lock. + "windows-targets", + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_gnullvm", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", # Declared repository WebAssembly/WASI is spec-only (no `crates/` tree). # The crate is effectively orphaned upstream. "witx", diff --git a/config/ast-grep/rules/command-requires-command-error.yaml b/config/ast-grep/rules/command-requires-command-error.yaml new file mode 100644 index 00000000..5e4a818b --- /dev/null +++ b/config/ast-grep/rules/command-requires-command-error.yaml @@ -0,0 +1,23 @@ +id: command-requires-command-error +language: Rust +severity: error +message: | + A file that imports `std::process::Command` via `use` must also `use command_error::CommandExt` (import it as + `use command_error::CommandExt as _;`) so command execution goes through the checked methods + (`status_checked` / `output_checked` / `spawn_checked`), whose errors carry the command line and exit status -- + a raw `std` `Command`'s `.status()` / `.output()` / `.spawn()` errors carry none of that. If a call genuinely + needs the raw outcome (e.g. a test that asserts a non-zero exit, or an exit-code proxy), fully-qualify + `std::process::Command` at the call site instead of importing it, so this rule does not apply. +rule: + kind: use_declaration + regex: 'std\s*::\s*process\s*::.*\bCommand\b' + not: + inside: + kind: source_file + stopBy: end + has: + kind: use_declaration + regex: "command_error" + stopBy: end +ignores: + - generated/** diff --git a/config/ast-grep/rules/doc-summary-ends-with-period.yaml b/config/ast-grep/rules/doc-summary-ends-with-period.yaml new file mode 100644 index 00000000..790d25d8 --- /dev/null +++ b/config/ast-grep/rules/doc-summary-ends-with-period.yaml @@ -0,0 +1,41 @@ +# This rule is rolled out incrementally, scoped by a `files:` allowlist rather than applied workspace-wide. +# The workspace had ~113 pre-existing docstrings whose first line does not end in terminal punctuation, so +# rather than rewrite them all at once, this rule only covers the files listed below. Grow the list (or drop +# the scoping once the backlog is cleared) as more files are brought into line. +id: doc-summary-ends-with-period +language: Rust +severity: error +message: | + A doc comment's first line must be a complete one-line summary ending in terminal punctuation (`.`, `!`, or + `?`). Put any elaboration in a following paragraph after a blank `///` line so the summary stays a single line + -- rustdoc and readers treat that first line as the item's summary. +rule: + all: + # A `///` outer-doc line with visible content (excludes an empty `///` and a `////` block comment). + - kind: line_comment + - regex: '^///\s*[^/\s]' + # ...whose text does not end in terminal punctuation (allowing a trailing `)`, quote, backtick, or `]`). + - not: + regex: '[.!?][")`\]]*\s*$' + # ...and which starts a doc block (is not preceded by another `///` doc line). + - not: + follows: + kind: line_comment + regex: "^///($|[^/])" + stopBy: neighbor +files: + - libs/edge-toolkit/src/config.rs + - libs/edge-toolkit/tests/no_mise.rs + - libs/test-helpers/src/lib.rs + - libs/ws-runner-common/tests/config.rs + - services/ws-pyo3-runner/tests/modules.rs + - services/ws-wasi-runner/tests/modules.rs + - services/ws-wasi-runner/tests/otel_propagation.rs + - services/ws-wasi-runner/tests/vector_otlp_relay.rs + - services/ws-web-runner/build.rs + - services/ws/tests/config.rs + - utilities/int-gen/src/error.rs + - utilities/int-gen/src/lib.rs + - utilities/int-gen/src/zig.rs + - utilities/int-gen/tests/generate_zig.rs + - utilities/wasm-cov-wrapper/src/main.rs diff --git a/generated/zig-rest/src/et_rest_client.zig b/generated/zig-rest/src/et_rest_client.zig index 6c48711d..f50bcfcc 100644 --- a/generated/zig-rest/src/et_rest_client.zig +++ b/generated/zig-rest/src/et_rest_client.zig @@ -1,3 +1,7 @@ +// +// this code was generated by openapi2zig 0.4.0 (1766c90) +// changes to this file may cause incorrect behavior and will be lost if the code is regenerated +// const std = @import("std"); /////////////////////////////////////////// @@ -61,6 +65,13 @@ pub fn ApiResult(comptime T: type) type { }; } +pub const HttpObserver = struct { + ctx: ?*anyopaque, + onRequest: ?*const fn (ctx: ?*anyopaque, method: std.http.Method, url: []const u8, headers: []const std.http.Header, body: ?[]const u8) void, + onResponse: ?*const fn (ctx: ?*anyopaque, method: std.http.Method, url: []const u8, status: std.http.Status, headers: []const std.http.Header, body: []const u8, duration_ns: u64) void, + onError: ?*const fn (ctx: ?*anyopaque, method: std.http.Method, url: []const u8, err_name: []const u8) void, +}; + pub const Client = struct { allocator: std.mem.Allocator, io: std.Io, @@ -70,6 +81,7 @@ pub const Client = struct { organization: ?[]const u8 = null, project: ?[]const u8 = null, default_headers: []const std.http.Header = &.{}, + http_observer: ?HttpObserver = null, pub fn init(allocator: std.mem.Allocator, io: std.Io, api_key: []const u8) Client { return .{ @@ -77,6 +89,7 @@ pub const Client = struct { .io = io, .http = .{ .allocator = allocator, .io = io }, .api_key = api_key, + .http_observer = null, }; } @@ -198,15 +211,37 @@ pub fn postJsonResult(comptime T: type, client: *Client, path: []const u8, paylo return parseRawResponse(T, try postJsonRaw(client, path, payload)); } +pub const CancellationToken = struct { + cancelled: std.atomic.Value(bool), + + pub fn init() CancellationToken { + return .{ .cancelled = std.atomic.Value(bool).init(false) }; + } + + pub fn cancel(self: *CancellationToken) void { + self.cancelled.store(true, .seq_cst); + } + + pub fn isCancelled(self: *CancellationToken) bool { + return self.cancelled.load(.seq_cst); + } +}; + +fn checkCancellation(token: ?*CancellationToken) !void { + if (token) |t| { + if (t.isCancelled()) return error.Cancelled; + } +} + const max_sse_line_size = 256 * 1024; const max_sse_event_size = 1024 * 1024; -pub fn parseSseBytes(allocator: std.mem.Allocator, bytes: []const u8, callback: anytype) !void { +pub fn parseSseBytes(allocator: std.mem.Allocator, bytes: []const u8, callback: anytype, cancellation_token: ?*CancellationToken) !void { var reader: std.Io.Reader = .fixed(bytes); - try parseSseReader(allocator, &reader, callback); + try parseSseReader(allocator, &reader, callback, cancellation_token); } -pub fn parseSseReader(allocator: std.mem.Allocator, reader: *std.Io.Reader, callback: anytype) !void { +pub fn parseSseReader(allocator: std.mem.Allocator, reader: *std.Io.Reader, callback: anytype, cancellation_token: ?*CancellationToken) !void { var line_buf: std.Io.Writer.Allocating = .init(allocator); defer line_buf.deinit(); @@ -214,6 +249,7 @@ pub fn parseSseReader(allocator: std.mem.Allocator, reader: *std.Io.Reader, call defer event_data.deinit(); while (true) { + try checkCancellation(cancellation_token); line_buf.clearRetainingCapacity(); _ = reader.streamDelimiterLimit(&line_buf.writer, '\n', .limited(max_sse_line_size)) catch |err| switch (err) { @@ -282,16 +318,16 @@ fn TypedSseCallback(comptime T: type, comptime Callback: type) type { }; } -pub fn parseSseBytesTyped(comptime T: type, allocator: std.mem.Allocator, bytes: []const u8, callback: anytype) !void { +pub fn parseSseBytesTyped(comptime T: type, allocator: std.mem.Allocator, bytes: []const u8, callback: anytype, cancellation_token: ?*CancellationToken) !void { const Callback = @TypeOf(callback.*); var typed_callback: TypedSseCallback(T, Callback) = .{ .allocator = allocator, .callback = callback }; - try parseSseBytes(allocator, bytes, &typed_callback); + try parseSseBytes(allocator, bytes, &typed_callback, cancellation_token); } -pub fn parseSseReaderTyped(comptime T: type, allocator: std.mem.Allocator, reader: *std.Io.Reader, callback: anytype) !void { +pub fn parseSseReaderTyped(comptime T: type, allocator: std.mem.Allocator, reader: *std.Io.Reader, callback: anytype, cancellation_token: ?*CancellationToken) !void { const Callback = @TypeOf(callback.*); var typed_callback: TypedSseCallback(T, Callback) = .{ .allocator = allocator, .callback = callback }; - try parseSseReader(allocator, reader, &typed_callback); + try parseSseReader(allocator, reader, &typed_callback, cancellation_token); } fn stringifyStreamRequest(allocator: std.mem.Allocator, requestBody: anytype) ![]u8 { @@ -312,13 +348,13 @@ fn stringifyStreamRequest(allocator: std.mem.Allocator, requestBody: anytype) ![ return try out.toOwnedSlice(); } -fn streamJsonTyped(comptime T: type, client: *Client, path: []const u8, requestBody: anytype, callback: anytype) !void { +fn streamJsonTyped(comptime T: type, client: *Client, path: []const u8, requestBody: anytype, callback: anytype, cancellation_token: ?*CancellationToken) !void { const Callback = @TypeOf(callback.*); var typed_callback: TypedSseCallback(T, Callback) = .{ .allocator = client.allocator, .callback = callback }; - try streamJson(client, path, requestBody, &typed_callback); + try streamJson(client, path, requestBody, &typed_callback, cancellation_token); } -fn streamJson(client: *Client, path: []const u8, requestBody: anytype, callback: anytype) !void { +fn streamJson(client: *Client, path: []const u8, requestBody: anytype, callback: anytype, cancellation_token: ?*CancellationToken) !void { const allocator = client.allocator; const payload = try stringifyStreamRequest(allocator, requestBody); defer allocator.free(payload); @@ -330,27 +366,55 @@ fn streamJson(client: *Client, path: []const u8, requestBody: anytype, callback: const url = try std.fmt.allocPrint(allocator, "{s}{s}", .{ client.base_url, path }); defer allocator.free(url); + + if (client.http_observer) |obs| { + if (obs.onRequest) |cb| cb(obs.ctx, .POST, url, headers.items, payload); + } + const uri = try std.Uri.parse(url); + try checkCancellation(cancellation_token); - var req = try client.http.request(.POST, uri, .{ + const start = std.Io.Clock.awake.now(client.io); + var req = client.http.request(.POST, uri, .{ .redirect_behavior = .unhandled, .headers = .{ .accept_encoding = .{ .override = "identity" } }, .extra_headers = headers.items, - }); + }) catch |err| { + if (client.http_observer) |obs| { + if (obs.onError) |cb| cb(obs.ctx, .POST, url, @errorName(err)); + } + return err; + }; defer req.deinit(); req.transfer_encoding = .{ .content_length = payload.len }; - var body = try req.sendBodyUnflushed(&.{}); - try body.writer.writeAll(payload); - try body.end(); + var request_body = try req.sendBodyUnflushed(&.{}); + try request_body.writer.writeAll(payload); + try request_body.end(); try req.connection.?.flush(); + try checkCancellation(cancellation_token); - var response = try req.receiveHead(&.{}); - if (response.head.status.class() != .success) return error.ResponseError; + var response = req.receiveHead(&.{}) catch |err| { + if (client.http_observer) |obs| { + if (obs.onError) |cb| cb(obs.ctx, .POST, url, @errorName(err)); + } + return err; + }; + const elapsed_ns = @as(u64, @intCast(start.untilNow(client.io, .awake).nanoseconds)); + if (response.head.status.class() != .success) { + if (client.http_observer) |obs| { + if (obs.onResponse) |cb| cb(obs.ctx, .POST, url, response.head.status, &.{}, "", elapsed_ns); + } + return error.ResponseError; + } + + if (client.http_observer) |obs| { + if (obs.onResponse) |cb| cb(obs.ctx, .POST, url, response.head.status, &.{}, "", elapsed_ns); + } var transfer_buffer: [8 * 1024]u8 = undefined; const reader = response.reader(&transfer_buffer); - parseSseReader(allocator, reader, callback) catch |err| switch (err) { + parseSseReader(allocator, reader, callback, cancellation_token) catch |err| switch (err) { error.ReadFailed => return response.bodyErr() orelse err, else => return err, }; diff --git a/libs/edge-toolkit/Cargo.toml b/libs/edge-toolkit/Cargo.toml index 692ed59c..a5ae8e97 100644 --- a/libs/edge-toolkit/Cargo.toml +++ b/libs/edge-toolkit/Cargo.toml @@ -12,6 +12,7 @@ doctest = false [dependencies] asyncapi-rust = { workspace = true, optional = true } base64.workspace = true +command-error.workspace = true et-path.workspace = true fs-err.workspace = true humantime-serde.workspace = true @@ -34,8 +35,8 @@ thiserror.workspace = true schema-export = ["dep:asyncapi-rust", "dep:schemars"] [dev-dependencies] +et-test-helpers.workspace = true rstest.workspace = true -temp-env.workspace = true tempfile.workspace = true testing_logger.workspace = true diff --git a/libs/edge-toolkit/src/config.rs b/libs/edge-toolkit/src/config.rs index 7032c240..234cf3af 100644 --- a/libs/edge-toolkit/src/config.rs +++ b/libs/edge-toolkit/src/config.rs @@ -1,6 +1,7 @@ use std::path::{Path, PathBuf}; use std::time::Duration; +use command_error::CommandExt as _; use fs_err as fs; use serde::Deserialize; use serde_default::DefaultFromSerde; @@ -13,8 +14,9 @@ use crate::ports::Services; /// Localhost address 127.0.0.1 . pub const LOCALHOST: &str = "127.0.0.1"; -/// Whether a config value names the "disabled" state: `none`, `off`, or -/// `disabled` (case-insensitive, surrounding whitespace ignored). +/// Whether a config value names the "disabled" state: `none`, `off`, or `disabled`. +/// +/// Matched case-insensitively, ignoring surrounding whitespace. /// /// These sentinels let an `Option<_>` env-var field be set to `None`. A blank /// value can't serve that role -- `serde-env` drops empty-valued vars, so a @@ -27,8 +29,7 @@ pub fn is_disabled_sentinel(value: &str) -> bool { || trimmed.eq_ignore_ascii_case("disabled") } -/// Deserialize `Option` where a disable sentinel ([`is_disabled_sentinel`]) -/// maps to `None` and any other value to `Some(T)` via `T`'s own `Deserialize`. +/// Deserialize `Option`, mapping a disable sentinel ([`is_disabled_sentinel`]) to `None`, else `Some(T)`. /// /// Generic over the inner type, for fields read from env vars via `serde-env`: /// the value arrives as a string, so this works for any `T` whose `Deserialize` @@ -55,8 +56,7 @@ where T::deserialize(inner).map(Some) } -/// [`deserialize_optional`] for `Option` fields, parsing the value as -/// a humantime duration (e.g. `15s`, `1m30s`). +/// [`deserialize_optional`] for `Option` fields, parsing a humantime duration (e.g. `15s`, `1m30s`). /// /// Separate from the generic [`deserialize_optional`] because `Duration`'s own /// `Deserialize` expects a `{secs, nanos}` struct, not a humantime string. @@ -174,12 +174,15 @@ pub fn default_modules_folders() -> Vec { paths } -/// Returns `true` if the `mise` binary is reachable on `PATH`. A failed -/// `Command::output()` indicates the binary couldn't be spawned -- -/// typically because it's not installed or the test is hiding `PATH`. +/// Returns `true` if the `mise` binary is reachable on `PATH`. +/// +/// It runs `mise --version` and checks the exit status is 0. #[must_use] pub fn mise_is_available() -> bool { - std::process::Command::new("mise").arg("--version").output().is_ok() + std::process::Command::new("mise") + .arg("--version") + .output_checked() + .is_ok() } /// Guest languages mise loads via `MISE_ENV`. @@ -206,8 +209,7 @@ pub enum Language { } impl Language { - /// Canonical lowercase name as used in `MISE_ENV` and the - /// `config..toml` filename. + /// Canonical lowercase name as used in `MISE_ENV` and the `config..toml` filename. #[must_use] pub fn as_str(self) -> &'static str { self.into() @@ -257,10 +259,12 @@ pub fn mise_env_includes(language: Language) -> bool { /// Returns the install path for a `mise` tool, e.g. `mise where npm:onnxruntime-web`. #[must_use] pub fn mise_where(tool: &str) -> Option { - let output = std::process::Command::new("mise").args(["where", tool]).output().ok()?; - if !output.status.success() { - return None; - } + // `output_checked` folds the spawn check and the non-zero-exit check together, so a missing tool or a + // failed `mise where` both short-circuit to `None` without a manual `status.success()` guard. + let output = std::process::Command::new("mise") + .args(["where", tool]) + .output_checked() + .ok()?; let stdout = std::str::from_utf8(&output.stdout).ok()?; let path = PathBuf::from(stdout.trim()); path.is_dir().then_some(path) @@ -289,9 +293,10 @@ pub fn mise_npm_package_path(package: &str) -> Option { mise_npm_modules_path(package).map(|node_modules| node_modules.join(package)) } -/// Pure-filesystem version of [`mise_npm_modules_path`]: given an -/// `` root and a `` name, return the `node_modules` -/// directory that contains ``. Supports the mise npm backends: +/// Pure-filesystem version of [`mise_npm_modules_path`], resolving a package's `node_modules` directory. +/// +/// Given an `` root and a `` name, return the `node_modules` directory that contains +/// ``. Supports the mise npm backends: /// /// 1. Classical npm/mise (Unix): `/lib/node_modules/` /// 2. npm on Windows: `/node_modules/` (no `lib/` segment -- @@ -324,8 +329,7 @@ pub fn find_npm_modules_path_in(install: &Path, package: &str) -> Option Vec { if !mise_is_available() { return Vec::new(); } - let output = std::process::Command::new("mise") + // `output_checked` errors on both a spawn failure and a non-zero exit, so the `mise ls` best-effort + // path collapses to a single fallible call -- no separate `status.success()` filter. + let Ok(output) = std::process::Command::new("mise") .args(["ls", "--current", "--json"]) - .output() - .ok(); - let Some(output) = output.filter(|out| out.status.success()) else { + .output_checked() + else { return Vec::new(); }; let Ok(tools) = serde_json::from_slice::>(&output.stdout) else { @@ -366,8 +371,7 @@ pub fn mise_python_site_packages() -> Vec { .collect() } -/// Pure-filesystem helper: given a mise `pipx:` `` root, return the -/// venv `site-packages` directory. +/// Pure-filesystem helper returning the venv `site-packages` directory for a mise `pipx:` `` root. /// /// pipx lays each tool out as `///site-packages`, /// where `` is `lib/python` on POSIX and `Lib` (no Python diff --git a/libs/edge-toolkit/tests/no_mise.rs b/libs/edge-toolkit/tests/no_mise.rs index fd1e9b31..bb611faf 100644 --- a/libs/edge-toolkit/tests/no_mise.rs +++ b/libs/edge-toolkit/tests/no_mise.rs @@ -9,7 +9,6 @@ use std::path::PathBuf; use edge_toolkit::config::default_modules_folders; -use tempfile::TempDir; #[test] fn returns_only_workspace_paths_when_mise_missing() { @@ -19,14 +18,9 @@ fn returns_only_workspace_paths_when_mise_missing() { // a parallel test runner. testing_logger::setup(); - // Point PATH at an empty tempdir, hiding mise (and every other - // binary) from the spawn in `mise_is_available`. `temp-env` - // restores PATH after the closure, so we don't poison sibling - // tests in the same binary. - let empty_dir = TempDir::new().unwrap(); - let path = empty_dir.path().to_string_lossy().into_owned(); - - let paths = temp_env::with_var("PATH", Some(path.as_str()), default_modules_folders); + // Empty PATH for the call, hiding mise (and every other binary) from the spawn in `mise_is_available`. + // `with_empty_path` restores PATH after the closure, so we don't poison sibling tests in the same binary. + let paths = et_test_helpers::with_empty_path(default_modules_folders); // Hardcoded workspace paths only, zero mise-resolved paths. let expected_suffixes = [ diff --git a/libs/test-helpers/Cargo.toml b/libs/test-helpers/Cargo.toml index 67b38598..7040ed6a 100644 --- a/libs/test-helpers/Cargo.toml +++ b/libs/test-helpers/Cargo.toml @@ -13,6 +13,7 @@ doctest = false [dependencies] port_check.workspace = true retry.workspace = true +temp-env.workspace = true [lints] workspace = true diff --git a/libs/test-helpers/src/lib.rs b/libs/test-helpers/src/lib.rs index e731056c..f9bc269a 100644 --- a/libs/test-helpers/src/lib.rs +++ b/libs/test-helpers/src/lib.rs @@ -14,6 +14,25 @@ use std::sync::{Arc, Mutex}; use retry::delay::Fixed; use retry::retry; +/// The workspace's canonical fake-environment test harness, re-exported for every test crate. +/// +/// `temp-env` runs a closure with environment variables temporarily set or unset and restores them afterward, +/// serialised by its own global lock. Prefer it (e.g. `et_test_helpers::temp_env::with_var("KEY", Some("v"), +/// || ...)`) over touching `std::env` by hand -- `set_var`/`remove_var` are `unsafe` on edition 2024 and race +/// other threads. For the common "make a `PATH`-resolved tool look absent" case, use [`with_empty_path`]. +pub use temp_env; + +/// Run `run` with `PATH` emptied so a bare-name `Command` spawn fails, restoring `PATH` afterward. +/// +/// Exercises "tool not found on `PATH`" fallbacks deterministically: the OS-level program resolution inside +/// `Command::new("")` sees the empty `PATH` and fails to find the binary. Thin wrapper over +/// [`temp_env::with_var`]; returns whatever `run` returns. +pub fn with_empty_path(run: Body) -> Out +where + Body: FnOnce() -> Out, +{ + temp_env::with_var("PATH", Some(""), run) +} /// Reserve a free loopback TCP port, bound then released for the caller to claim. /// diff --git a/libs/ws-runner-common/Cargo.toml b/libs/ws-runner-common/Cargo.toml index d8e62474..d9ee40ab 100644 --- a/libs/ws-runner-common/Cargo.toml +++ b/libs/ws-runner-common/Cargo.toml @@ -28,8 +28,8 @@ tokio-tungstenite = { workspace = true, features = ["connect"] } tracing.workspace = true [dev-dependencies] +et-test-helpers.workspace = true serde-env.workspace = true -temp-env.workspace = true [lints] workspace = true diff --git a/libs/ws-runner-common/tests/config.rs b/libs/ws-runner-common/tests/config.rs index 8e3f64e9..21ac05ae 100644 --- a/libs/ws-runner-common/tests/config.rs +++ b/libs/ws-runner-common/tests/config.rs @@ -10,6 +10,7 @@ use std::time::Duration; +use et_test_helpers::temp_env; use et_ws_runner_common::config::{RunnerConfig, WsConfig}; use serde::Deserialize; diff --git a/services/ws-pyo3-runner/Cargo.toml b/services/ws-pyo3-runner/Cargo.toml index 9e0782d5..e06c0232 100644 --- a/services/ws-pyo3-runner/Cargo.toml +++ b/services/ws-pyo3-runner/Cargo.toml @@ -39,6 +39,7 @@ pyo3-build-config.workspace = true [dev-dependencies] backon = { workspace = true, features = ["tokio-sleep"] } base64.workspace = true +command-error.workspace = true et-ws-test-server.workspace = true rstest.workspace = true diff --git a/services/ws-pyo3-runner/tests/modules.rs b/services/ws-pyo3-runner/tests/modules.rs index 65b326fc..6fb85176 100644 --- a/services/ws-pyo3-runner/tests/modules.rs +++ b/services/ws-pyo3-runner/tests/modules.rs @@ -23,6 +23,7 @@ use std::process::{Child, Command, Stdio}; use std::time::{Duration, Instant}; use backon::{ExponentialBuilder, RetryableWithContext as _}; +use command_error::CommandExt as _; use edge_toolkit::config::{Language, mise_env_includes}; use edge_toolkit::ws::{ClientMessage, ServerMessage}; use futures_util::{SinkExt as _, StreamExt as _}; @@ -37,15 +38,18 @@ type ControlSocket = tokio_tungstenite::WebSocketStream Result<(), Box> { + // Raw `.output()` (not `output_checked`) on purpose: a non-zero exit is the asserted-for outcome here, so + // we must inspect `output.status`/`output.stderr` rather than have a checked call turn it into an error. let output = Command::new(env!("CARGO_BIN_EXE_et-ws-pyo3-runner")) .env("RUNNER_MODULE", "no_hooks") .env("PYO3_PYTHONPATH", python_dir()) @@ -150,10 +157,12 @@ fn no_hooks_fails_to_load() -> Result<(), Box> { Ok(()) } -/// `RUNNER_TIMEOUT` must trigger a graceful shutdown, not an abrupt drop: `drive`'s shutdown arm returns so -/// `run` still executes its teardown (queue `on_shutdown`, join the Python worker, close the socket). `echo` -/// loads without any gated toolchain, so the runner reaches its connected run loop before the short timeout -/// trips; a clean (zero) exit code then confirms the teardown path ran to completion. +/// `RUNNER_TIMEOUT` must trigger a graceful shutdown, not an abrupt drop. +/// +/// `drive`'s shutdown arm returns so `run` still executes its teardown (queue `on_shutdown`, join the Python +/// worker, close the socket). `echo` loads without any gated toolchain, so the runner reaches its connected run +/// loop before the short timeout trips; a clean (zero) exit code then confirms the teardown path ran to +/// completion. #[tokio::test(flavor = "current_thread")] async fn shuts_down_gracefully_on_timeout() -> Result<(), Box> { let server = et_ws_test_server::start(); @@ -166,7 +175,8 @@ async fn shuts_down_gracefully_on_timeout() -> Result<(), Box> { .env("RUST_LOG", rust_log) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) - .spawn()?; + .spawn_checked()? + .into_child(); // The runner self-exits shortly after the 3s timeout; the outer bound only stops a regression from hanging // the suite. `try_wait` polls without blocking the current-thread runtime. @@ -278,8 +288,7 @@ fn check_torch(value: &serde_json::Value) -> Result<(), Box> { Ok(()) } -/// Directory of the bundled test Python modules -- the single source of truth -/// for `PYO3_PYTHONPATH` across every case. +/// Directory of the bundled test Python modules -- the single source of truth for `PYO3_PYTHONPATH`. fn python_dir() -> PathBuf { edge_toolkit::config::get_project_root().join("services/ws-pyo3-runner/python") } @@ -295,8 +304,9 @@ fn spawn_runner(module: &str, ws_url: &str) -> Child { .env("RUST_LOG", rust_log) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) - .spawn() + .spawn_checked() .unwrap() + .into_child() } /// Open a control client and drive et-connect until we have an `agent_id`. @@ -345,8 +355,9 @@ async fn peer_poll_step<'sock>( ((control, self_id), outcome) } -/// One registration poll round: request the roster, drain replies for `POLL_DRAIN_WINDOW`, and return -/// `Ok(())` once a peer that isn't us appears. `Err(())` is the "not yet" sentinel `backon` retries on. +/// One registration poll round: request the roster and drain replies for `POLL_DRAIN_WINDOW`. +/// +/// Returns `Ok(())` once a peer that isn't us appears. `Err(())` is the "not yet" sentinel `backon` retries on. async fn poll_for_peer_once(control: &mut ControlSocket, self_id: &str) -> Result<(), ()> { let Ok(req) = serde_json::to_string(&ClientMessage::ListAgents) else { return Err(()); @@ -434,8 +445,9 @@ async fn collect_binary(control: &mut ControlSocket, count: usize) -> Result bool { edge_toolkit::config::mise_python_site_packages() .iter() diff --git a/services/ws-wasi-runner/Cargo.toml b/services/ws-wasi-runner/Cargo.toml index ee11e9c7..a85307b7 100644 --- a/services/ws-wasi-runner/Cargo.toml +++ b/services/ws-wasi-runner/Cargo.toml @@ -73,6 +73,7 @@ ort.workspace = true wgpu.workspace = true [dev-dependencies] +command-error.workspace = true et-test-helpers.workspace = true et-ws-test-server.workspace = true int-otlp-mock.workspace = true diff --git a/services/ws-wasi-runner/tests/modules.rs b/services/ws-wasi-runner/tests/modules.rs index fde7856e..1cf3ac2e 100644 --- a/services/ws-wasi-runner/tests/modules.rs +++ b/services/ws-wasi-runner/tests/modules.rs @@ -5,6 +5,7 @@ #![cfg(test)] +use command_error::CommandExt as _; use edge_toolkit::config::{Language, mise_env_includes}; use rstest::rstest; @@ -30,12 +31,12 @@ fn module_runs_successfully(#[case] module: &str, #[case] language: Language) { // No-op on Linux/Windows. // ET_TEST_COVERAGE, when set by the coverage workflow, is inherited by the child (Command keeps the parent // env), so the runner preopens /cov and instrumented guests dump their .profraw -- no forwarding needed. - let status = std::process::Command::new(bin) + // `status_checked` turns a non-zero exit (or a spawn failure) into a panic carrying the command line and + // status; `unwrap_or_else` adds the `module` name that isn't otherwise on the command line. + let _: std::process::ExitStatus = std::process::Command::new(bin) .env("RUNNER_MODULE", module) .env("WS_SERVER_URL", &server.ws_url) .env("ET_TEST_WS_WASI_RUNNER_FAST_EXIT", "1") - .status() - .unwrap(); - - assert!(status.success(), "{module} exited with code {:?}", status.code()); + .status_checked() + .unwrap_or_else(|error| panic!("{module} runner failed: {error}")); } diff --git a/services/ws-wasi-runner/tests/otel_propagation.rs b/services/ws-wasi-runner/tests/otel_propagation.rs index f597870c..5298e4f2 100644 --- a/services/ws-wasi-runner/tests/otel_propagation.rs +++ b/services/ws-wasi-runner/tests/otel_propagation.rs @@ -30,6 +30,7 @@ use std::collections::HashSet; use std::time::Duration; +use command_error::CommandExt as _; use edge_toolkit::config::{Language, OtlpConfig, OtlpProtocol, mise_env_includes}; // Skipped on Windows: this test spawns the runner against the wasi-data1 @@ -68,17 +69,17 @@ fn trace_ids_propagate_between_runner_and_server() { // OTLP. Every `OTLP_*` env var is consumed by the runner's // `serde_env::from_env::()` call. let bin = env!("CARGO_BIN_EXE_et-ws-wasi-runner"); - let status = std::process::Command::new(bin) + // `status_checked` panics on a non-zero exit or spawn failure, with the command line + status baked in -- + // the same assertion the explicit `status.success()` check made, folded into the call. + let _: std::process::ExitStatus = std::process::Command::new(bin) .env("RUNNER_MODULE", "et-ws-wasi-data1") .env("WS_SERVER_URL", &server.ws_url) .env("OTLP_COLLECTOR_URL", mock.collector_url()) .env("OTLP_PROTOCOL", "JSON") .env("OTLP_SERVICE_LABEL", "et-ws-wasi-runner") - .status() + .status_checked() .unwrap(); - assert!(status.success(), "runner exited with code {:?}", status.code()); - // 4. Flush our own (server-side) batch exporter so any pending spans // land in the mock before we read it. server_handles.shutdown(); diff --git a/services/ws-wasi-runner/tests/vector_otlp_relay.rs b/services/ws-wasi-runner/tests/vector_otlp_relay.rs index d8461dd2..444d09de 100644 --- a/services/ws-wasi-runner/tests/vector_otlp_relay.rs +++ b/services/ws-wasi-runner/tests/vector_otlp_relay.rs @@ -28,6 +28,7 @@ use std::process::{Command, Stdio}; use std::sync::Mutex; use std::time::Duration; +use command_error::CommandExt as _; use et_test_helpers::{ChildGuard, drain_stderr, reserve_port, wait_for_port}; use opentelemetry_proto::tonic::collector::trace::v1::ExportTraceServiceRequest; use opentelemetry_proto::tonic::common::v1::{AnyValue, KeyValue, any_value}; @@ -73,8 +74,9 @@ fn vector_relays_buffered_otlp_after_backend_comes_online() { .env("RELAY_SINK_PORT", mock_port.to_string()) .stdout(Stdio::null()) .stderr(Stdio::piped()) - .spawn() - .unwrap(); + .spawn_checked() + .unwrap() + .into_child(); // Drain Vector's stderr into memory so it stays quiet on success but is // available for failure messages (a file-backed `Stdio` would need the // banned `std::fs::File`). diff --git a/services/ws-web-runner/Cargo.toml b/services/ws-web-runner/Cargo.toml index a7904029..139a0259 100644 --- a/services/ws-web-runner/Cargo.toml +++ b/services/ws-web-runner/Cargo.toml @@ -51,6 +51,7 @@ winapi.workspace = true # since that branch (and the functions it calls) don't compile there, which also keeps them out of the Linux # coverage report. cc.workspace = true +command-error.workspace = true fs-err.workspace = true [dev-dependencies] diff --git a/services/ws-web-runner/build.rs b/services/ws-web-runner/build.rs index fc737107..7f4b15a0 100644 --- a/services/ws-web-runner/build.rs +++ b/services/ws-web-runner/build.rs @@ -129,6 +129,8 @@ fn link_mingw_shim() { reason = "build script: a panic is cargo's only failure channel for a failed command" )] fn run(command: &mut std::process::Command) { - let status = command.status().unwrap(); - assert!(status.success(), "{command:?} exited with {status}"); + // Scoped `use` to inherit "Windows only". + use command_error::CommandExt as _; + + let _: std::process::ExitStatus = command.status_checked().unwrap(); } diff --git a/services/ws/Cargo.toml b/services/ws/Cargo.toml index 1362e005..5c4c67ec 100644 --- a/services/ws/Cargo.toml +++ b/services/ws/Cargo.toml @@ -28,8 +28,8 @@ tracing.workspace = true uuid.workspace = true [dev-dependencies] +et-test-helpers.workspace = true serde-env.workspace = true -temp-env.workspace = true [lints] workspace = true diff --git a/services/ws/tests/config.rs b/services/ws/tests/config.rs index 5bff5ada..fd28dbbf 100644 --- a/services/ws/tests/config.rs +++ b/services/ws/tests/config.rs @@ -8,6 +8,7 @@ reason = "test code: byte sizes read clearer as decimal MiB math than hex" )] +use et_test_helpers::temp_env; use et_ws_service::WsConfig; use serde::Deserialize; diff --git a/utilities/int-gen/Cargo.toml b/utilities/int-gen/Cargo.toml index e2e8853d..7ddc9e9f 100644 --- a/utilities/int-gen/Cargo.toml +++ b/utilities/int-gen/Cargo.toml @@ -17,6 +17,7 @@ path = "src/bin/int-gen.rs" asyncapi-rust.workspace = true clap.workspace = true clap-markdown = { workspace = true, optional = true } +command-error.workspace = true edge-toolkit = { workspace = true, features = ["schema-export"] } et-modules-service = { workspace = true, features = ["openapi-spec"] } et-storage-service = { workspace = true, features = ["openapi-spec"] } @@ -47,5 +48,8 @@ wit-parser.workspace = true [features] markdown-help = ["dep:clap-markdown"] +[dev-dependencies] +et-test-helpers.workspace = true + [lints] workspace = true diff --git a/utilities/int-gen/src/error.rs b/utilities/int-gen/src/error.rs new file mode 100644 index 00000000..513db269 --- /dev/null +++ b/utilities/int-gen/src/error.rs @@ -0,0 +1,54 @@ +//! The crate's error type and its foreign-error conversions. + +/// Errors raised by `et-int-gen`. +/// +/// Every external error type that fallible functions can produce is wrapped +/// transparently via `#[from]`, so call sites just use `?`. Domain errors +/// (malformed schemas, missing `AsyncAPI` nodes, etc.) sit alongside as +/// non-transparent variants with static messages. +#[expect( + clippy::exhaustive_enums, + clippy::error_impl_error, + reason = "internal crate (no SemVer); new variants land with their change, and crate::Error is the sole error type" +)] +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error(transparent)] + Io(#[from] std::io::Error), + #[error(transparent)] + Command(#[from] command_error::Error), + #[error(transparent)] + Json(#[from] serde_json::Error), + #[error(transparent)] + Yaml(#[from] serde_yaml::Error), + #[error(transparent)] + Http(#[from] reqwest::Error), + #[error(transparent)] + Semver(#[from] semver::Error), + #[error(transparent)] + Fmt(#[from] std::fmt::Error), + #[error(transparent)] + Regex(#[from] regex::Error), + + #[error("AsyncAPI spec missing required node: {0}")] + SpecNodeMissing(&'static str), + #[error("WS message JSON Schema malformed: {0}")] + SchemaMalformed(&'static str), + #[error("unsupported JSON Schema `type`: `{0}`")] + UnsupportedSchemaType(String), + #[error("enum value not a string in `{0}`")] + EnumValueNotString(String), + #[error("progenitor codegen: {0}")] + Progenitor(#[from] progenitor::Error), + #[error("zig codegen: {0}")] + ZigCodegen(String), +} + +// Left uncovered by design: `LanguageError` has a private field and no public constructor, and `set_language` +// only returns it on a tree-sitter/grammar ABI mismatch -- which would break every Zig-parsing test -- so this +// conversion cannot be exercised by a unit test. +impl From for Error { + fn from(err: tree_sitter::LanguageError) -> Self { + Self::ZigCodegen(format!("set Zig language: {err}")) + } +} diff --git a/utilities/int-gen/src/lib.rs b/utilities/int-gen/src/lib.rs index c83dafac..58cb4735 100644 --- a/utilities/int-gen/src/lib.rs +++ b/utilities/int-gen/src/lib.rs @@ -44,64 +44,13 @@ use fs_err as fs; use schemars::schema_for; pub mod asyncapi; +pub mod error; pub mod kdl; pub mod openapi; pub mod wit; pub mod zig; -/// Errors raised by `et-int-gen`. -/// -/// Every external error type that fallible functions can produce is wrapped -/// transparently via `#[from]`, so call sites just use `?`. Domain errors -/// (malformed schemas, missing `AsyncAPI` nodes, etc.) sit alongside as -/// non-transparent variants with static messages. -#[expect( - clippy::exhaustive_enums, - clippy::error_impl_error, - reason = "internal crate (no SemVer); new variants land with their change, and crate::Error is the sole error type" -)] -#[derive(Debug, thiserror::Error)] -pub enum Error { - #[error(transparent)] - Io(#[from] std::io::Error), - #[error(transparent)] - Json(#[from] serde_json::Error), - #[error(transparent)] - Yaml(#[from] serde_yaml::Error), - #[error(transparent)] - Http(#[from] reqwest::Error), - #[error(transparent)] - Semver(#[from] semver::Error), - #[error(transparent)] - Fmt(#[from] std::fmt::Error), - #[error(transparent)] - Regex(#[from] regex::Error), - - #[error("AsyncAPI spec missing required node: {0}")] - SpecNodeMissing(&'static str), - #[error("WS message JSON Schema malformed: {0}")] - SchemaMalformed(&'static str), - #[error("unsupported JSON Schema `type`: `{0}`")] - UnsupportedSchemaType(String), - #[error("enum value not a string in `{0}`")] - EnumValueNotString(String), - #[error("progenitor codegen: {0}")] - Progenitor(String), - #[error("zig codegen: {0}")] - ZigCodegen(String), -} - -impl From for Error { - fn from(err: progenitor::Error) -> Self { - Self::Progenitor(err.to_string()) - } -} - -impl From for Error { - fn from(err: tree_sitter::LanguageError) -> Self { - Self::ZigCodegen(format!("set Zig language: {err}")) - } -} +pub use self::error::Error; /// Emit every checked-in artifact under `generated/` (core + Rust + Zig). /// @@ -114,11 +63,10 @@ pub fn generate() -> Result<(), Error> { Ok(()) } -/// Emit the language-agnostic artifacts: `ws.yaml`, `rest.yaml`, the -/// `ws.schema.json` intermediates, `ws.kdl`, and the `et:ws-messages` WIT -/// package. +/// Emit the language-agnostic artifacts. /// -/// These feed every downstream client (the Dart/Python generators consume +/// Namely `ws.yaml`, `rest.yaml`, the `ws.schema.json` intermediates, `ws.kdl`, and the `et:ws-messages` WIT +/// package. These feed every downstream client (the Dart/Python generators consume /// `ws.kdl`/`*.schema.json`/`rest.yaml`), so this is the prerequisite step /// every per-language `gen:*` mise task depends on. #[expect( @@ -222,8 +170,7 @@ pub fn generate_zig() -> Result<(), Error> { Ok(()) } -/// Write only when the contents differ -- keeps `mise run check` quiet on -/// no-op regenerations. +/// Write only when the contents differ, to keep `mise run check` quiet on no-op regenerations. #[expect( clippy::print_stdout, reason = "et-int-gen is a CLI; `wrote ` per generated file is intended user-visible progress output" diff --git a/utilities/int-gen/src/zig.rs b/utilities/int-gen/src/zig.rs index f71aa3b6..ecdf927a 100644 --- a/utilities/int-gen/src/zig.rs +++ b/utilities/int-gen/src/zig.rs @@ -34,15 +34,16 @@ use std::path::Path; use std::process::Command; +use command_error::CommandExt as _; use fs_err as fs; use regex::Regex; use tree_sitter::{Node, Parser}; use crate::Error; -/// Name of the single shared request function whose body we replace with the -/// host-import dispatch; `requestRaw` and every per-operation wrapper funnel -/// through it. +/// Name of the single shared request function whose body we replace with the host-import dispatch. +/// +/// `requestRaw` and every per-operation wrapper funnel through it. const SHARED_REQUEST_FN: &str = "requestRawWithContentType"; /// The replacement body spliced into [`SHARED_REQUEST_FN`] by [`rewrite`]. @@ -51,27 +52,33 @@ const REQUEST_RAW_BODY: &str = include_str!("zig.in/request_raw_body.zig"); /// The `extern fn js_rest_request(...)` declaration appended to the generated client by [`rewrite`]. const JS_REST_REQUEST_EXTERN: &str = include_str!("zig.in/js_rest_request_extern.zig"); -/// Regex spanning the inlined request tail openapi2zig 0.3 emits for a binary/text body (its `.binary`/`.text` -/// arm, which -- unlike JSON bodies -- does not delegate to [`SHARED_REQUEST_FN`]). It anchors on the -/// `= requestBody;` payload assignment, which is unique to those ops (JSON ops assign `str.written()`, empty -/// bodies `null`), then skips to the two captures: 1 = content-type, 2 = HTTP method. `[A-Z]+` (not `\w`) keeps -/// it ASCII so no unicode regex feature is needed. [`reroute_inline_binary_ops`] splices both into a delegation. -const INLINE_FETCH_BLOCK_PATTERN: &str = - r#"(?s)requestBody;.*?"([^"]+)".*?Method\.([A-Z]+).*?toOwnedSlice\(\),\n \};"#; +/// Regex matching openapi2zig's inlined binary/text-body request tail, for rerouting it to [`SHARED_REQUEST_FN`]. +/// +/// That `.binary`/`.text` arm (unlike JSON bodies) does not delegate to [`SHARED_REQUEST_FN`]. The regex anchors +/// on the `= requestBody;` payload assignment, which is unique to those ops (JSON ops assign `str.written()`, +/// empty bodies `null`), then skips to the two captures: 1 = content-type, 2 = HTTP method. `[A-Z]+` (not `\w`) +/// keeps it ASCII so no unicode regex feature is needed, and ends on the `RawResponse` struct literal's closing +/// `.body = body,` field -- as of openapi2zig 0.4 that arm hoists the response body into a `const body` and +/// stores it via `.body = body,` (0.3 inlined `.body = ...toOwnedSlice(),` in the struct literal directly). +/// [`reroute_inline_binary_ops`] splices both captures into a delegation. +const INLINE_FETCH_BLOCK_PATTERN: &str = r#"(?s)requestBody;.*?"([^"]+)".*?Method\.([A-Z]+).*?\.body = body,\n \};"#; + +/// The volatile ` on UTC` suffix openapi2zig stamps into its `// ` header. +/// +/// It is a wall-clock time, so the committed client would differ on every regen and fail `gen-specs-check`. We +/// strip just this suffix (keeping the stable `openapi2zig ()` identity) so the output is +/// deterministic across runs of the same binary. +const GENERATED_TIMESTAMP_PATTERN: &str = r" on \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} UTC"; /// Return `true` if the `openapi2zig` binary is on `PATH`. /// /// Upstream doesn't publish a `linux/arm64` release (see `.mise/config.zig.toml`). #[must_use] pub fn is_available() -> bool { - Command::new("openapi2zig").arg("--version").output().is_ok() + Command::new("openapi2zig").arg("--version").output_checked().is_ok() } -/// Invoke `openapi2zig` against the `OpenAPI` JSON intermediate, post-process -/// the result, and return the final Zig source. -/// -/// Subprocess errors are flattened into `Error::ZigCodegen` since we don't -/// model them more precisely. +/// Invoke `openapi2zig` against the `OpenAPI` JSON intermediate, and post-process. pub fn render(rest_json: &Path, raw_out: &Path) -> Result { run_openapi2zig(rest_json, raw_out)?; let raw = fs::read_to_string(raw_out)?; @@ -83,14 +90,14 @@ pub fn render(rest_json: &Path, raw_out: &Path) -> Result { reason = "named helper; pairs with rewrite() as the two halves of render()" )] fn run_openapi2zig(rest_json: &Path, raw_out: &Path) -> Result<(), Error> { - if let Some(parent) = raw_out.parent() { - fs::create_dir_all(parent)?; - } - // Surface the spawn failure as `Error::Io` (the `#[from]` variant) - // rather than a `ZigCodegen(format!(...))` wrap -- same diagnostic - // detail (`std::io::Error` carries the "No such file or directory" - // text) with the typed source preserved. - let status = Command::new("openapi2zig") + // `parent()` is `None` only for a root/prefix/empty path -- never for a real output file (it yields + // `Some("")` for a bare filename, which `create_dir_all` treats as a no-op `Ok`), so the empty-path + // fallback keeps this unconditional without a guard. + fs::create_dir_all(raw_out.parent().unwrap_or_else(|| Path::new("")))?; + // `status_checked` inherits stdio (so openapi2zig's `info:` lines still stream live) and turns both a + // spawn failure and a non-zero exit into an `Error::Command` carrying the command line + status -- no + // manual `status.success()` check, and a richer diagnostic than the old `ZigCodegen("exited with ...")`. + let _: std::process::ExitStatus = Command::new("openapi2zig") .args([ "generate", "--resource-wrappers", @@ -100,18 +107,15 @@ fn run_openapi2zig(rest_json: &Path, raw_out: &Path) -> Result<(), Error> { "-o", &raw_out.display().to_string(), ]) - .status()?; - if !status.success() { - return Err(Error::ZigCodegen(format!("openapi2zig exited with {status}"))); - } + .status_checked()?; Ok(()) } -/// Replace the shared request function's body with our extern-backed -/// implementation, reroute the inlined binary operations through it, assert -/// no reachable `client.http.fetch` survived, and append the -/// `extern fn js_rest_request` declaration. Everything else passes through -/// verbatim. +/// Rewrite openapi2zig's output to funnel every request through the host import. +/// +/// Replaces the shared request function's body with our extern-backed implementation, reroutes the inlined +/// binary operations through it, asserts no reachable `client.http.fetch` survived, and appends the +/// `extern fn js_rest_request` declaration. Everything else passes through verbatim. #[expect( clippy::single_call_fn, reason = "named helper; pairs with run_openapi2zig() as the two halves of render()" @@ -159,11 +163,12 @@ fn rewrite(source: &str) -> Result { )); } out.push_str(JS_REST_REQUEST_EXTERN); + // Strip openapi2zig's volatile header timestamp so the committed client stays byte-identical across regens. + let out = Regex::new(GENERATED_TIMESTAMP_PATTERN)?.replace(&out, "").into_owned(); Ok(out) } -/// Reroute openapi2zig's inlined binary/text-body operations through the -/// shared, extern-backed request function. +/// Reroute openapi2zig's inlined binary/text-body operations through the shared request function. /// /// Since openapi2zig 0.3, an operation whose request body is not JSON/form /// (e.g. `application/octet-stream`) does not delegate to @@ -195,8 +200,7 @@ fn reroute_inline_binary_ops(source: &str) -> Result { .into_owned()) } -/// Recursive walk: return the first `function_declaration` whose `name` -/// child matches `wanted`. +/// Recursively return the first `function_declaration` whose `name` child matches `wanted`. fn find_fn<'tree>(node: Node<'tree>, source: &str, wanted: &str) -> Option> { // Indexing into `source` is safe because tree-sitter node byte spans // are always UTF-8 boundary-aligned for a UTF-8 input. diff --git a/utilities/int-gen/tests/generate_zig.rs b/utilities/int-gen/tests/generate_zig.rs new file mode 100644 index 00000000..ee15424e --- /dev/null +++ b/utilities/int-gen/tests/generate_zig.rs @@ -0,0 +1,25 @@ +//! Covers `generate_zig`'s "openapi2zig not on PATH" skip branch. +//! +//! `generate_zig` probes for the `openapi2zig` binary via [`zig::is_available`] and quietly skips Zig REST-client +//! generation when it is absent (upstream ships no linux/arm64 release, so that lane always takes this path). We +//! force the skip regardless of whether the tool is installed on the test runner by emptying `PATH` for the +//! duration of the call with `temp-env`, so the internal `Command::new("openapi2zig")` lookup finds nothing. +#![cfg(test)] + +use et_int_gen::zig; + +#[test] +fn generate_zig_skips_when_openapi2zig_is_absent() { + // `with_empty_path` empties the real process `PATH` for the closure and restores it after, so the OS-level + // program resolution inside `is_available` sees an empty `PATH` and can't find openapi2zig. + et_test_helpers::with_empty_path(|| { + assert!(!zig::is_available(), "openapi2zig must look absent with an empty PATH"); + // The skip branch prints a notice and returns `Ok(())` without invoking openapi2zig or touching the + // committed client, so this succeeds even on a runner that has the tool installed. + let result = et_int_gen::generate_zig(); + assert!( + result.is_ok(), + "generate_zig must succeed by skipping when openapi2zig is absent: {result:?}" + ); + }); +} diff --git a/utilities/wasm-cov-wrapper/Cargo.toml b/utilities/wasm-cov-wrapper/Cargo.toml index 7ea9727c..4e6a514e 100644 --- a/utilities/wasm-cov-wrapper/Cargo.toml +++ b/utilities/wasm-cov-wrapper/Cargo.toml @@ -13,5 +13,8 @@ name = "int-wasm-cov-wrapper" path = "src/main.rs" test = false +[dependencies] +command-error.workspace = true + [lints] workspace = true diff --git a/utilities/wasm-cov-wrapper/src/main.rs b/utilities/wasm-cov-wrapper/src/main.rs index b99bea83..963e3900 100644 --- a/utilities/wasm-cov-wrapper/src/main.rs +++ b/utilities/wasm-cov-wrapper/src/main.rs @@ -21,6 +21,8 @@ use std::ffi::OsString; use std::process::{Command, ExitCode}; +use command_error::CommandExt as _; + fn main() -> ExitCode { let mut args = std::env::args_os().skip(1); let Some(rustc) = args.next() else { @@ -47,9 +49,9 @@ fn main() -> ExitCode { ); } - match Command::new(&rustc).args(&rustc_args).status() { - Ok(status) if status.success() => ExitCode::SUCCESS, - Ok(_nonzero) => ExitCode::FAILURE, + match Command::new(&rustc).args(&rustc_args).status_checked() { + Ok(_) => ExitCode::SUCCESS, + Err(command_error::Error::Output(_)) => ExitCode::FAILURE, Err(error) => { eprintln!("int-wasm-cov-wrapper: failed to run rustc: {error}"); ExitCode::FAILURE