diff --git a/.env.example b/.env.example index cabc5a0..d35705b 100644 --- a/.env.example +++ b/.env.example @@ -1,5 +1,4 @@ HTTP_PORT=3000 -BOT_NAME=Herald WEBHOOK_SIG_HEADER_SECRET= @@ -20,7 +19,6 @@ RUST_BACKTRACE=1 METRICS_BIND_ADDR= -# Sandboxed tool execution (optional) -SANDBOX_ENABLED=false +# Sandboxed tool execution CONTAINER_RUNTIME=docker -SANDBOX_MAX_ITERATIONS=8 \ No newline at end of file +SANDBOX_MAX_ITERATIONS=8 diff --git a/Cargo.lock b/Cargo.lock index 02d3b25..4444691 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -338,17 +338,6 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" -[[package]] -name = "chacha20" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "65c35e4b699c7e15ccbe7ee35c005e4fc0a278d22238a2857e6ce2dadeda1b06" -dependencies = [ - "cfg-if", - "cpufeatures", - "rand_core 0.10.1", -] - [[package]] name = "convert_case" version = "0.10.0" @@ -374,15 +363,6 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" -[[package]] -name = "cpufeatures" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ca28b0ae3115b884660db4118d803791fd6756b6e88f39c0f3f7859060d7566" -dependencies = [ - "libc", -] - [[package]] name = "crossbeam-epoch" version = "0.9.20" @@ -398,41 +378,6 @@ version = "0.8.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "61803da095bee82a81bb1a452ecc25d3b2f1416d1897eb86430c6159ef717c17" -[[package]] -name = "darling" -version = "0.20.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc7f46116c46ff9ab3eb1597a45688b6715c6e628b5c133e288e709a29bcb4ee" -dependencies = [ - "darling_core", - "darling_macro", -] - -[[package]] -name = "darling_core" -version = "0.20.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d00b9596d185e565c2207a0b01f8bd1a135483d02d9b7b0a54b11da8d53412e" -dependencies = [ - "fnv", - "ident_case", - "proc-macro2", - "quote", - "strsim", - "syn 2.0.119", -] - -[[package]] -name = "darling_macro" -version = "0.20.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" -dependencies = [ - "darling_core", - "quote", - "syn 2.0.119", -] - [[package]] name = "debugid" version = "0.8.0" @@ -459,37 +404,6 @@ version = "0.5.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7cd812cc2bc1d69d4764bd80df88b4317eaef9e773c75226407d9bc0876b211c" -[[package]] -name = "derive_builder" -version = "0.20.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "507dfb09ea8b7fa618fcf76e953f4f5e192547945816d5358edffe39f6f94947" -dependencies = [ - "derive_builder_macro", -] - -[[package]] -name = "derive_builder_core" -version = "0.20.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d5bcf7b024d6835cfb3d473887cd966994907effbe9227e8c8219824d06c4e8" -dependencies = [ - "darling", - "proc-macro2", - "quote", - "syn 2.0.119", -] - -[[package]] -name = "derive_builder_macro" -version = "0.20.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab63b0e2bf4d5928aff72e83a7dace85d7bba5fe12dcc3c5a572d78caffd3f3c" -dependencies = [ - "derive_builder_core", - "syn 2.0.119", -] - [[package]] name = "derive_more" version = "2.1.1" @@ -521,7 +435,7 @@ dependencies = [ "serde", "serde_json", "tempfile", - "thiserror 2.0.20", + "thiserror", "tokio", ] @@ -552,24 +466,6 @@ version = "0.15.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1aaf95b3e5c8f23aa320147307562d361db0ae0d51242340f558153b4eb2439b" -[[package]] -name = "dotenvy_macro" -version = "0.15.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb0235d912a8c749f4e0c9f18ca253b4c28cfefc1d2518096016d6e3230b6424" -dependencies = [ - "dotenvy", - "proc-macro2", - "quote", - "syn 1.0.109", -] - -[[package]] -name = "dyn-clone" -version = "1.0.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0881ea181b1df73ff77ffaaf9c7544ecc11e82fba9b5f27b262a3c73a332555" - [[package]] name = "encoding_rs" version = "0.8.35" @@ -592,7 +488,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -749,10 +645,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ff2abc00be7fca6ebc474524697ae276ad847ad0a6b3faa4bcb027e9a4614ad0" dependencies = [ "cfg-if", - "js-sys", "libc", "wasi", - "wasm-bindgen", ] [[package]] @@ -763,24 +657,10 @@ checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" dependencies = [ "cfg-if", "libc", - "r-efi 5.3.0", + "r-efi", "wasip2", ] -[[package]] -name = "getrandom" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" -dependencies = [ - "cfg-if", - "js-sys", - "libc", - "r-efi 6.0.0", - "rand_core 0.10.1", - "wasm-bindgen", -] - [[package]] name = "gimli" version = "0.32.3" @@ -833,24 +713,21 @@ version = "1.2.0" dependencies = [ "anyhow", "axum", - "bytes", "devcontainer-rs", "dotenvy", "futures-util", "hex", "metrics", "metrics-exporter-prometheus", - "openrouter-rs", - "reqwest 0.12.28", + "reqwest", "ring", "sentry", "sentry-anyhow", "serde", "serde_json", "tempfile", - "thiserror 2.0.20", + "thiserror", "tokio", - "tokio-stream", "tokio-util", "tower", "tower-http 0.7.1", @@ -953,22 +830,6 @@ dependencies = [ "want", ] -[[package]] -name = "hyper-rustls" -version = "0.27.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33ca68d021ef39cf6463ab54c1d0f5daf03377b70561305bb89a8f83aab66e0f" -dependencies = [ - "http 1.5.0", - "hyper", - "hyper-util", - "rustls", - "tokio", - "tokio-rustls", - "tower-service", - "webpki-roots", -] - [[package]] name = "hyper-tls" version = "0.6.0" @@ -1091,12 +952,6 @@ dependencies = [ "zerovec", ] -[[package]] -name = "ident_case" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" - [[package]] name = "idna" version = "1.1.0" @@ -1232,12 +1087,6 @@ dependencies = [ "tracing-subscriber", ] -[[package]] -name = "lru-slab" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "112b39cec0b298b6c1999fee3e31427f74f676e4cb9879ed1a121b43661a4154" - [[package]] name = "matchers" version = "0.2.0" @@ -1285,7 +1134,7 @@ dependencies = [ "metrics", "metrics-util", "quanta", - "thiserror 2.0.20", + "thiserror", "tokio", "tracing", ] @@ -1301,7 +1150,7 @@ dependencies = [ "hashbrown 0.16.1", "metrics", "quanta", - "rand 0.9.5", + "rand", "rand_xoshiro", "rapidhash", "sketches-ddsketch", @@ -1313,16 +1162,6 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" -[[package]] -name = "mime_guess" -version = "2.0.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7c44f8e672c00fe5308fa235f821cb4198414e1c77935c1ab6948d3fd78550e" -dependencies = [ - "mime", - "unicase", -] - [[package]] name = "miniz_oxide" version = "0.8.9" @@ -1562,26 +1401,6 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" -[[package]] -name = "openrouter-rs" -version = "0.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "042920aeb3955ae7c6c26118c646f9723c4ba39e65ceb663513c6973ebfc21dc" -dependencies = [ - "derive_builder", - "dotenvy_macro", - "futures-util", - "http 1.5.0", - "reqwest 0.12.28", - "schemars", - "serde", - "serde_json", - "thiserror 1.0.69", - "tokio", - "tokio-util", - "urlencoding", -] - [[package]] name = "openssl" version = "0.10.81" @@ -1765,62 +1584,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "quinn" -version = "0.11.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c1a41e437b6bbd489372cd4971de128e85c855f56c57f283d20ff016cf7c0a8" -dependencies = [ - "bytes", - "cfg_aliases", - "pin-project-lite", - "quinn-proto", - "quinn-udp", - "rustc-hash", - "rustls", - "socket2", - "thiserror 2.0.20", - "tokio", - "tracing", - "web-time", -] - -[[package]] -name = "quinn-proto" -version = "0.11.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04759210543be93709136e28212294a659ef5001836ff4eab4d663e4529bba83" -dependencies = [ - "bytes", - "getrandom 0.4.3", - "lru-slab", - "rand 0.10.2", - "rand_pcg", - "ring", - "rustc-hash", - "rustls", - "rustls-pki-types", - "slab", - "thiserror 2.0.20", - "tinyvec", - "tracing", - "web-time", -] - -[[package]] -name = "quinn-udp" -version = "0.5.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35a133f956daabe89a61a685c2649f13d82d5aa4bd5d12d1277e1072a21c0694" -dependencies = [ - "cfg_aliases", - "libc", - "once_cell", - "socket2", - "tracing", - "windows-sys 0.61.2", -] - [[package]] name = "quote" version = "1.0.47" @@ -1836,12 +1599,6 @@ version = "5.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" -[[package]] -name = "r-efi" -version = "6.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" - [[package]] name = "rand" version = "0.9.5" @@ -1849,18 +1606,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9ef1d0d795eb7d84685bca4f72f3649f064e6641543d3a8c415898726a57b41" dependencies = [ "rand_chacha", - "rand_core 0.9.5", -] - -[[package]] -name = "rand" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c7f5fa3a058cd35567ef9bfa5e75732bee0f9e4c55fa90477bef2dfcdbc4be80" -dependencies = [ - "chacha20", - "getrandom 0.4.3", - "rand_core 0.10.1", + "rand_core", ] [[package]] @@ -1870,7 +1616,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb" dependencies = [ "ppv-lite86", - "rand_core 0.9.5", + "rand_core", ] [[package]] @@ -1882,28 +1628,13 @@ dependencies = [ "getrandom 0.3.4", ] -[[package]] -name = "rand_core" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" - -[[package]] -name = "rand_pcg" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "caa0f4137e1c0a72f4c651489402276c8e8e1cf081f3b0ba156d2cbeef09e86a" -dependencies = [ - "rand_core 0.10.1", -] - [[package]] name = "rand_xoshiro" version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f703f4665700daf5512dcca5f43afa6af89f09db47fb56be587f80636bda2d41" dependencies = [ - "rand_core 0.9.5", + "rand_core", ] [[package]] @@ -1933,26 +1664,6 @@ dependencies = [ "bitflags", ] -[[package]] -name = "ref-cast" -version = "1.0.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7e440fb4e4b4147295338efb76001ab9e4efc0e5839df2c47fc5ac2381d365c3" -dependencies = [ - "ref-cast-impl", -] - -[[package]] -name = "ref-cast-impl" -version = "1.0.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "92ecd8964f8453721699a1ed72037b0db49ce2f5a5138486ee89bed6f67cdf3a" -dependencies = [ - "proc-macro2", - "quote", - "syn 3.0.4", -] - [[package]] name = "regex" version = "1.13.1" @@ -1990,53 +1701,11 @@ checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" [[package]] name = "reqwest" -version = "0.12.28" +version = "0.13.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eddd3ca559203180a307f12d114c268abf583f59b03cb906fd0b3ff8646c1147" +checksum = "16a1cfa75cc186dd73d5818e510e042e40927bccc9c236b061cea97e1eb08029" dependencies = [ - "base64 0.22.1", - "bytes", - "futures-core", - "futures-util", - "http 1.5.0", - "http-body", - "http-body-util", - "hyper", - "hyper-rustls", - "hyper-util", - "js-sys", - "log", - "mime_guess", - "percent-encoding", - "pin-project-lite", - "quinn", - "rustls", - "rustls-pki-types", - "serde", - "serde_json", - "serde_urlencoded", - "sync_wrapper", - "tokio", - "tokio-rustls", - "tokio-util", - "tower", - "tower-http 0.6.11", - "tower-service", - "url", - "wasm-bindgen", - "wasm-bindgen-futures", - "wasm-streams", - "web-sys", - "webpki-roots", -] - -[[package]] -name = "reqwest" -version = "0.13.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "219c5811de6525e5416c7d5d53bb656d3afdbc6c5af816e0802bcfa42dbdc1c3" -dependencies = [ - "base64 0.22.1", + "base64 0.23.1", "bytes", "futures-channel", "futures-core", @@ -2058,12 +1727,14 @@ dependencies = [ "sync_wrapper", "tokio", "tokio-native-tls", + "tokio-util", "tower", "tower-http 0.6.11", "tower-service", "url", "wasm-bindgen", "wasm-bindgen-futures", + "wasm-streams", "web-sys", ] @@ -2087,12 +1758,6 @@ version = "0.1.28" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b74b56ffa8bb2830709a538c2cbcae9aa062db0d2a42563bfb09bdaae44020eb" -[[package]] -name = "rustc-hash" -version = "2.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b1e7f9a428571be2dc5bc0505c13fb6bf936822b894ec87abf8a08a4e51742d" - [[package]] name = "rustc_version" version = "0.4.1" @@ -2112,21 +1777,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", -] - -[[package]] -name = "rustls" -version = "0.23.43" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0283386ce02abc0151e1761d08802dfe86c173b0b494af5cbc086574e453da06" -dependencies = [ - "once_cell", - "ring", - "rustls-pki-types", - "rustls-webpki", - "subtle", - "zeroize", + "windows-sys 0.52.0", ] [[package]] @@ -2135,21 +1786,9 @@ version = "1.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2f4925028c7eb5d1fcdaf196971378ed9d2c1c4efc7dc5d011256f76c99c0a96" dependencies = [ - "web-time", "zeroize", ] -[[package]] -name = "rustls-webpki" -version = "0.103.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3c3cf1d8b1e7d4927e2d154c3fcb02979afb9939629c62cd9048d4f07b60ac2" -dependencies = [ - "ring", - "rustls-pki-types", - "untrusted", -] - [[package]] name = "rustversion" version = "1.0.23" @@ -2171,31 +1810,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "schemars" -version = "1.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "687274d293b6cdc6e73e0fee520bf2049650090d7164f87672d212a3c530cf4a" -dependencies = [ - "dyn-clone", - "ref-cast", - "schemars_derive", - "serde", - "serde_json", -] - -[[package]] -name = "schemars_derive" -version = "1.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d98c67716b46af2f0b8cf752abc930f6f9aecfbf671ecfb531db8a31dbe4e2ba" -dependencies = [ - "proc-macro2", - "quote", - "serde_derive_internals", - "syn 3.0.4", -] - [[package]] name = "scoped-tls" version = "1.0.1" @@ -2246,7 +1860,7 @@ dependencies = [ "cfg_aliases", "httpdate", "native-tls", - "reqwest 0.13.4", + "reqwest", "sentry-actix", "sentry-backtrace", "sentry-contexts", @@ -2315,7 +1929,7 @@ version = "0.49.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d889520a375e5b93efb0a66def1630d21d168b0251eb55b286b03fa940ccfc84" dependencies = [ - "rand 0.9.5", + "rand", "sentry-types", "serde", "serde_json", @@ -2389,10 +2003,10 @@ checksum = "fc71f5ca55942d9b2901af95d5df5c5d65c164134c22a83682cac3d0b6c7ef2d" dependencies = [ "debugid", "hex", - "rand 0.9.5", + "rand", "serde", "serde_json", - "thiserror 2.0.20", + "thiserror", "time", "url", "uuid", @@ -2428,17 +2042,6 @@ dependencies = [ "syn 3.0.4", ] -[[package]] -name = "serde_derive_internals" -version = "0.30.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f852137cce035d6a4df67ccce505ff6b3e9fd3a10e3e52b24dc71e650bb1a9bd" -dependencies = [ - "proc-macro2", - "quote", - "syn 3.0.4", -] - [[package]] name = "serde_json" version = "1.0.151" @@ -2534,29 +2137,6 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" -[[package]] -name = "strsim" -version = "0.11.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" - -[[package]] -name = "subtle" -version = "2.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" - -[[package]] -name = "syn" -version = "1.0.109" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", -] - [[package]] name = "syn" version = "2.0.119" @@ -2606,19 +2186,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.4.3", + "getrandom 0.3.4", "once_cell", "rustix", - "windows-sys 0.61.2", -] - -[[package]] -name = "thiserror" -version = "1.0.69" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" -dependencies = [ - "thiserror-impl 1.0.69", + "windows-sys 0.52.0", ] [[package]] @@ -2627,18 +2198,7 @@ version = "2.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" dependencies = [ - "thiserror-impl 2.0.20", -] - -[[package]] -name = "thiserror-impl" -version = "1.0.69" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", + "thiserror-impl", ] [[package]] @@ -2701,21 +2261,6 @@ dependencies = [ "zerovec", ] -[[package]] -name = "tinyvec" -version = "1.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb4ebadaa0af04fab11ae01eb5f9fdb5f9c5b875506e210e71c07873528baa7f" -dependencies = [ - "tinyvec_macros", -] - -[[package]] -name = "tinyvec_macros" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" - [[package]] name = "tokio" version = "1.53.1" @@ -2754,27 +2299,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "tokio-rustls" -version = "0.26.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" -dependencies = [ - "rustls", - "tokio", -] - -[[package]] -name = "tokio-stream" -version = "0.1.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3d06f0b082ba57c26b79407372e57cf2a1e28124f78e9479fe80322cf53420b" -dependencies = [ - "futures-core", - "pin-project-lite", - "tokio", -] - [[package]] name = "tokio-util" version = "0.7.19" @@ -2929,12 +2453,6 @@ dependencies = [ "libc", ] -[[package]] -name = "unicase" -version = "2.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dbc4bc3a9f746d862c45cb89d705aa10f187bb96c76001afab07a0d35ce60142" - [[package]] name = "unicode-ident" version = "1.0.24" @@ -3001,12 +2519,6 @@ dependencies = [ "serde_derive", ] -[[package]] -name = "urlencoding" -version = "2.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "daf8dba3b7eb870caf1ddeed7bc9d2a049f3cfdfae7cb521b087cc33ae4c49da" - [[package]] name = "utf8-zero" version = "0.8.1" @@ -3123,9 +2635,9 @@ dependencies = [ [[package]] name = "wasm-streams" -version = "0.4.2" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "15053d8d85c7eccdbefef60f06769760a563c7f0a9d6902a13d35c7800b0ad65" +checksum = "9d1ec4f6517c9e11ae630e200b2b65d193279042e28edd4a2cda233e46670bbb" dependencies = [ "futures-util", "js-sys", @@ -3144,16 +2656,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "web-time" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a6580f308b1fad9207618087a65c04e7a10bc77e02c8e84e9b00dd4b12fa0bb" -dependencies = [ - "js-sys", - "wasm-bindgen", -] - [[package]] name = "webpki-root-certs" version = "1.0.9" @@ -3163,15 +2665,6 @@ dependencies = [ "rustls-pki-types", ] -[[package]] -name = "webpki-roots" -version = "1.0.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7dcd9d09a39985f5344844e66b0c530a33843579125f23e21e9f0f220850f22a" -dependencies = [ - "rustls-pki-types", -] - [[package]] name = "winapi" version = "0.3.9" diff --git a/Cargo.toml b/Cargo.toml index 672a89c..1fef14f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,16 +6,14 @@ members = [ resolver = "3" [workspace.dependencies] -reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } +reqwest = { version = "0.13", default-features = false, features = ["json", "native-tls", "stream"] } tokio = { version = "1.53", features = ["full"] } -tokio-stream = "0.1" tokio-util = "0.7" futures-util = "0.3" serde_json = "1.0" serde = { version = "1.0", features = ["derive"] } sentry = { version = "0.49", features = ["tower-axum-matched-path"] } sentry-anyhow = { version = "0.49", features = ["backtrace"] } -openrouter-rs = "0.14" dotenvy = "0.15" tower = "0.5" tower-http = { version = "0.7", features = ["trace"] } @@ -26,7 +24,6 @@ anyhow = { version = "1", features = ["backtrace"] } thiserror = "2.0" ring = "0.17" hex = "0.4" -bytes = "1.1" metrics = "0.24" metrics-exporter-prometheus = { version = "0.18", default-features = false, features = ["http-listener"] } diff --git a/README.md b/README.md index a749dfd..235c963 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,6 @@ Herald reads its configuration from environment variables (a `.env` file is supp | Variable | Description | |---|---| | `HTTP_PORT` | Port to listen on | -| `BOT_NAME` | The bot's Gitea username (used to detect mentions) | | `WEBHOOK_SIG_HEADER_SECRET` | Gitea webhook secret for signature verification | | `OPEN_ROUTER_API_KEY` | OpenRouter API key | | `OPEN_ROUTER_MODEL` | Model to use (e.g. `deepseek/deepseek-v4-flash`) | @@ -38,20 +37,24 @@ Herald reads its configuration from environment variables (a `.env` file is supp | `METRICS_BIND_ADDR` | *(optional)* Bind address for the Prometheus metrics endpoint (e.g. `0.0.0.0:9100`). If unset, the metrics exporter is disabled. | | `SENTRY_DSN` | *(optional)* Sentry DSN for error tracking | | `RUST_LOG` | *(optional)* Log level, defaults to `info` | -| `SANDBOX_ENABLED` | *(optional)* Run reviews inside a devcontainer sandbox so the model can explore the repository with tools. Defaults to `false` | | `CONTAINER_RUNTIME` | *(optional)* Container runtime binary used for the sandbox (`docker` or `podman`). Defaults to `docker` | | `SANDBOX_MAX_ITERATIONS` | *(optional)* Maximum number of tool-calling iterations per sandboxed review. Defaults to `8` | ## Sandboxed reviews -When `SANDBOX_ENABLED=true`, Herald reviews pull requests inside an ephemeral +Herald reviews pull requests inside an ephemeral [Dev Container](https://containers.dev/). For each review it: 1. clones the pull request head into a temporary directory, 2. builds and starts the repository's devcontainer (`devcontainer-rs`), -3. lets the model explore the repository with read-only tools (`ls`, `read_file`, - `grep`, `find`) executed inside the container, -4. posts the review and removes the container and the temporary clone. +3. reads the pull request diff and file list from the Gitea API with + `GITEA_TOKEN` (so private repositories work), tells the model which files and + lines changed — additions and deletions, with the line numbers of the new and + old versions of the file respectively — then lets it explore the repository + with read-only tools (`ls`, `read_file`, `grep`, `find`) run inside the + container: the code itself is not sent, so the model reads it at those lines, +4. posts the review, anchoring each comment on the added or removed line it + refers to, and removes the container and the temporary clone. The container runtime is selected with `CONTAINER_RUNTIME` (`docker` or `podman`). The repository must contain a `.devcontainer/devcontainer.json`. diff --git a/crates/herald-server/Cargo.toml b/crates/herald-server/Cargo.toml index 9635007..6a6ed9a 100644 --- a/crates/herald-server/Cargo.toml +++ b/crates/herald-server/Cargo.toml @@ -6,14 +6,12 @@ edition = "2024" [dependencies] reqwest = { workspace = true } tokio = { workspace = true } -tokio-stream = { workspace = true } tokio-util = { workspace = true } futures-util = { workspace = true } serde_json = { workspace = true } serde = { workspace = true } sentry = { workspace = true } sentry-anyhow = { workspace = true } -openrouter-rs = { workspace = true } dotenvy = { workspace = true } tower = { workspace = true } tower-http = { workspace = true } @@ -24,7 +22,6 @@ anyhow = { workspace = true } thiserror = { workspace = true } ring = { workspace = true } hex = { workspace = true } -bytes = { workspace = true } metrics = { workspace = true } metrics-exporter-prometheus = { workspace = true } devcontainer-rs = { path = "../devcontainer-rs" } diff --git a/crates/herald-server/src/bot.rs b/crates/herald-server/src/bot.rs index ae1029e..aca98ed 100644 --- a/crates/herald-server/src/bot.rs +++ b/crates/herald-server/src/bot.rs @@ -4,7 +4,7 @@ use crate::{ open_router::OpenRouterClient, sandbox::SandboxConfig, }; -use serde::Deserialize; +use serde::{Deserialize, Deserializer}; use std::{collections::HashSet, sync::Arc}; use tokio::sync::Mutex; use tokio_util::sync::CancellationToken; @@ -21,15 +21,52 @@ pub struct ReviewResult { pub struct ReviewItem { pub filename: String, pub line: Option, + #[serde(default, deserialize_with = "deserialize_side")] + pub side: Option, pub message: String, } +/// Which version of the file a review comment is anchored on. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReviewSide { + /// The line was added by the pull request: `line` is a line number of the + /// new version of the file. + Added, + /// The line was removed by the pull request: `line` is a line number of the + /// old version of the file. + Removed, +} + +impl ReviewSide { + /// Reads the side the model asked for, tolerating casing and synonyms. + fn parse(raw: &str) -> Option { + match raw.trim().to_ascii_lowercase().as_str() { + "added" | "add" | "new" | "right" => Some(Self::Added), + "removed" | "remove" | "deleted" | "delete" | "old" | "left" => Some(Self::Removed), + _ => None, + } + } +} + +/// Reads the side the model asked for. An unreadable value is ignored instead of +/// failing the whole review: the side is then resolved from the changed lines. +fn deserialize_side<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let raw = Option::::deserialize(deserializer)?; + + Ok(raw + .as_ref() + .and_then(serde_json::Value::as_str) + .and_then(ReviewSide::parse)) +} + #[derive(Clone)] pub struct Bot { bot_name: String, gitea_api: GiteaAPI, open_router_client: OpenRouterClient, - http_client: reqwest::Client, max_concurrent: usize, open_router_model: String, sandbox: SandboxConfig, @@ -41,7 +78,6 @@ impl Bot { bot_name: String, gitea_api: GiteaAPI, open_router_client: OpenRouterClient, - http_client: reqwest::Client, max_concurrent: usize, open_router_model: String, sandbox: SandboxConfig, @@ -50,7 +86,6 @@ impl Bot { bot_name, gitea_api, open_router_client, - http_client, max_concurrent, open_router_model, sandbox, @@ -123,7 +158,6 @@ impl Bot { WebhookType::Review(review_payload) => crate::bot_actions::review::exec_review( &self.gitea_api, &self.open_router_client, - &self.http_client, &self.open_router_model, &self.sandbox, tools, diff --git a/crates/herald-server/src/bot_actions/review.rs b/crates/herald-server/src/bot_actions/review.rs index fdb39bb..83d9424 100644 --- a/crates/herald-server/src/bot_actions/review.rs +++ b/crates/herald-server/src/bot_actions/review.rs @@ -1,30 +1,18 @@ -use futures_util::stream::TryStreamExt; -use openrouter_rs::types::Tool; -use tokio::io::AsyncReadExt; -use tokio_util::io::StreamReader; use tracing::{info, instrument, warn}; use crate::{ - bot::ReviewResult, - consts::{BOT_PROCESS_MSG, MAX_DIFF_SIZE, REVIEW_PROMPT, SANDBOX_SYSTEM_PROMPT}, - gitea::{GiteaAPI, ReviewPayload}, + bot::{ReviewResult, ReviewSide}, + consts::{BOT_PROCESS_MSG, REVIEW_PROMPT, SANDBOX_SYSTEM_PROMPT}, + gitea::{GiteaAPI, PullRequestFile, ReviewPayload}, metrics, - open_router::OpenRouterClient, + open_router::{OpenRouterClient, Tool}, sandbox::{Sandbox, SandboxConfig, agent}, }; -#[instrument(skip( - gitea_api, - open_router_client, - http_client, - sandbox_config, - tools, - review_payload -))] +#[instrument(skip(gitea_api, open_router_client, sandbox_config, tools, review_payload))] pub async fn exec_review( gitea_api: &GiteaAPI, open_router_client: &OpenRouterClient, - http_client: &reqwest::Client, model: &str, sandbox_config: &SandboxConfig, tools: Vec, @@ -46,32 +34,43 @@ pub async fn exec_review( .await?; let bot_result: Result = async { - let git_diff = - download_git_diff(http_client, &review_payload.pull_request.diff_url).await?; + let full_name = &review_payload.repository.full_name; + let index = review_payload.pull_request.number; - let diff_for_llm = format_diff_for_review(&git_diff); + let git_diff = gitea_api.pull_request_diff(full_name, index).await?; + + // The file list only refines the paths and describes the changes: a + // failure is not fatal, the diff is the source of truth for the lines. + let files = match gitea_api.pull_request_files(full_name, index).await { + Ok(files) => files, + Err(err) => { + warn!(%err, "Failed to list the pull request files"); + Vec::new() + } + }; + + let mut changed_lines = parse_changed_lines(&git_diff); + resolve_filenames(&mut changed_lines, &files); + + let changes = format_changes(&files, &changed_lines); let bot_request = REVIEW_PROMPT .replace("{subject}", &review_payload.pull_request.title) .replace("{comment}", &review_payload.comment.body) - .replace("{diff}", &diff_for_llm); + .replace("{changes}", &changes); - let (message, cost) = if sandbox_config.enabled { - run_sandboxed_review( - gitea_api, - open_router_client, - sandbox_config, - tools, - &review_payload, - &bot_request, - ) - .await? - } else { - let chat_result = open_router_client.chat(&bot_request).await?; - (chat_result.message, chat_result.cost) - }; + let (message, cost) = run_sandboxed_review( + gitea_api, + open_router_client, + sandbox_config, + tools, + &review_payload, + &bot_request, + ) + .await?; let mut review_result = serde_json::from_str::(&message)?; + resolve_review_sides(&mut review_result, &changed_lines); review_result.cost = cost; if let Some(cost) = review_result.cost { @@ -183,83 +182,350 @@ fn review_result_to_markdown(review_result: &ReviewResult) -> String { md } -async fn download_git_diff(http_client: &reqwest::Client, url: &str) -> anyhow::Result { - let response = http_client.get(url).send().await?; - let stream = response.bytes_stream().map_err(std::io::Error::other); +/// The lines a pull request changed, per file. +/// +/// Line numbers are the ones Gitea expects to place a review comment: +/// [`ReviewSide::Added`] numbers refer to the new version of the file (sent as +/// `new_position`), [`ReviewSide::Removed`] numbers to the old version (sent as +/// `old_position`). +type ChangedLines = Vec; - let mut buf = Vec::with_capacity(MAX_DIFF_SIZE); - StreamReader::new(stream) - .take((MAX_DIFF_SIZE + 1) as u64) - .read_to_end(&mut buf) - .await?; - - if buf.len() > MAX_DIFF_SIZE { - anyhow::bail!("Git diff exceeds the maximum allowed size of 1 Mo"); - } - - Ok(String::from_utf8_lossy(&buf).into_owned()) +struct ChangedFile { + filename: String, + added: Vec, + removed: Vec, } -fn format_diff_for_review(git_diff: &str) -> String { - let mut output = String::new(); - let mut current_file: Option<&str> = None; +impl ChangedFile { + fn new(filename: &str) -> Self { + Self { + filename: String::from(filename), + added: Vec::new(), + removed: Vec::new(), + } + } + + /// Lines that can be commented on for the given side. + fn lines(&self, side: ReviewSide) -> &[u64] { + match side { + ReviewSide::Added => &self.added, + ReviewSide::Removed => &self.removed, + } + } + + /// Side a line belongs to, used when the model did not state one. + fn side_of(&self, line: u64) -> Option { + if self.added.contains(&line) { + Some(ReviewSide::Added) + } else if self.removed.contains(&line) { + Some(ReviewSide::Removed) + } else { + None + } + } +} + +/// Lists the lines changed by the diff, per file, on both sides. +/// +/// Only the line numbers are kept: the model reads the code itself through the +/// sandbox tools. +fn parse_changed_lines(git_diff: &str) -> ChangedLines { + let mut files = Vec::new(); + let mut current_file: Option = None; + let mut in_hunk = false; + let mut old_line: u64 = 0; let mut new_line: u64 = 0; for line in git_diff.lines() { - if let Some(rest) = line.strip_prefix("diff --git a/") { - if let Some(end) = rest.find(' ') { - current_file = Some(&rest[..end]); - } - new_line = 0; + if line.starts_with("diff --git ") { + current_file = None; + in_hunk = false; continue; } - if line.starts_with("---") || line.starts_with("+++") { - continue; - } - - if line.starts_with("@@") && line.contains('+') { - if let Some(start) = parse_hunk_new_start(line) { - new_line = start; + // `--- a/x` and `+++ b/x` only appear before the first hunk of a file. + // Inside a hunk, a line may legitimately start with them: removing + // `--x` gives `---x`, adding `++i;` gives `+++i;`. + if !in_hunk && (line.starts_with("--- ") || line.starts_with("+++ ")) { + // A header is never a content line: `+++ /dev/null` on a deleted + // file keeps the path of the other side in place. + if let Some(path) = header_file_path(line) { + current_file = Some(path); } continue; } - let Some(filename) = current_file else { + if line.starts_with("@@") { + if let Some((old_start, new_start)) = parse_hunk_starts(line) { + old_line = old_start; + new_line = new_start; + in_hunk = true; + } + continue; + } + + let Some(filename) = current_file.as_deref() else { continue; }; - if line.starts_with(' ') { - new_line += 1; - continue; - } - - if let Some(code) = line.strip_prefix('+') { - use std::fmt::Write; - let _ = writeln!(output, "{filename}:{new_line}:{code}"); - new_line += 1; + match line.as_bytes().first() { + Some(b' ') => { + old_line += 1; + new_line += 1; + } + Some(b'-') => { + changed_file(&mut files, filename).removed.push(old_line); + old_line += 1; + } + Some(b'+') => { + changed_file(&mut files, filename).added.push(new_line); + new_line += 1; + } + // `\ No newline at end of file`, and anything unexpected: a line + // that advances neither side. + _ => {} } } - output + files } -fn parse_hunk_new_start(hunk_header: &str) -> Option { - let plus_part = hunk_header.split('+').nth(1)?; - let num_str = plus_part.split(|c: char| !c.is_ascii_digit()).next()?; - num_str.parse::().ok() +/// Path of the file on one side of the diff, from a `--- a/` or +/// `+++ b/` header line. +/// +/// These lines are the only unambiguous source for the path: the `diff --git` +/// line is cut at the first space, and it names the old path of a renamed file. +/// `None` for `/dev/null`, which leaves the path of the other side in place. +fn header_file_path(line: &str) -> Option { + let (prefix, raw) = match line.strip_prefix("--- ") { + Some(raw) => ("a/", raw), + None => ("b/", line.strip_prefix("+++ ")?), + }; + + let path = decode_git_path(raw); + + Some(String::from(path.strip_prefix(prefix)?)) +} + +/// Decodes a path as git writes it in a diff header: git wraps it in quotes and +/// escapes the bytes that need it (`\303\251` for `é`) when the path contains +/// non-printable or non-ASCII characters. +fn decode_git_path(raw: &str) -> String { + let Some(quoted) = raw.strip_prefix('"').and_then(|raw| raw.strip_suffix('"')) else { + return String::from(raw); + }; + + let bytes = quoted.as_bytes(); + let mut decoded = Vec::with_capacity(bytes.len()); + let mut index = 0; + + while let Some(byte) = bytes.get(index) { + index += 1; + + if *byte != b'\\' { + decoded.push(*byte); + continue; + } + + match bytes.get(index) { + // Octal escapes are the ones that matter for a path: git uses them + // for every non-ASCII byte. + Some(digit @ b'0'..=b'7') => { + let mut value = u32::from(digit - b'0'); + let mut digits = 1; + + while digits < 3 { + match bytes.get(index + digits) { + Some(next @ b'0'..=b'7') => { + value = value * 8 + u32::from(next - b'0'); + digits += 1; + } + _ => break, + } + } + + decoded.push(u8::try_from(value).unwrap_or(b'?')); + index += digits; + } + Some(escaped) => { + decoded.push(match escaped { + b't' => b'\t', + b'n' => b'\n', + b'r' => b'\r', + other => *other, + }); + index += 1; + } + None => decoded.push(b'\\'), + } + } + + String::from_utf8_lossy(&decoded).into_owned() +} + +/// Replaces the paths parsed from the diff with the exact paths reported by the +/// API, which are the ones the model sees in the sandbox. +fn resolve_filenames(changed_lines: &mut ChangedLines, files: &[PullRequestFile]) { + for changed in changed_lines.iter_mut() { + let parsed = changed.filename.as_str(); + + let Some(file) = files.iter().find(|file| file.filename == parsed) else { + if !files.is_empty() { + warn!(path = %parsed, "Changed file is not in the pull request file list"); + } + continue; + }; + + changed.filename = file.filename.clone(); + } +} + +/// Renders the changes for the model: the files the pull request touches, then +/// the lines to review per file. +fn format_changes(files: &[PullRequestFile], changed_lines: &ChangedLines) -> String { + let mut sections = Vec::new(); + + if !files.is_empty() { + let described = files + .iter() + .map(describe_file) + .collect::>() + .join(", "); + + sections.push(format!("Files changed by the pull request: {described}")); + } + + sections.push(format!( + "Lines to review, per file:\n{}", + format_changed_lines(changed_lines) + )); + + sections.join("\n\n") +} + +/// Describes a changed file for the model, including how it changed. +fn describe_file(file: &PullRequestFile) -> String { + match &file.previous_filename { + Some(previous) => format!("{} ({} from {})", file.filename, file.status, previous), + None => format!("{} ({})", file.filename, file.status), + } +} + +/// Borrows the entry of `files` for a file, creating it on its first change. +fn changed_file<'a>(files: &'a mut ChangedLines, filename: &str) -> &'a mut ChangedFile { + let index = match files.iter().position(|file| file.filename == filename) { + Some(index) => index, + None => { + files.push(ChangedFile::new(filename)); + files.len() - 1 + } + }; + + &mut files[index] +} + +/// Renders the changed lines as `filename: added 1, 2 / removed 3`, one file per +/// line, keeping only the sides the file actually has. +fn format_changed_lines(changed_lines: &ChangedLines) -> String { + changed_lines + .iter() + .map(|file| { + let mut sides = Vec::new(); + + for (label, lines) in [("added", &file.added), ("removed", &file.removed)] { + if !lines.is_empty() { + sides.push(format!("{label} {}", format_line_numbers(lines))); + } + } + + format!("{}: {}", file.filename, sides.join(" / ")) + }) + .collect::>() + .join("\n") +} + +fn format_line_numbers(lines: &[u64]) -> String { + lines + .iter() + .map(u64::to_string) + .collect::>() + .join(", ") +} + +/// Resolves the side each review is anchored on and drops the reviews that do +/// not match a line the pull request changes. +/// +/// The model is asked to pick a line and a side from the provided lists, but +/// nothing forces it to, and Gitea accepts any position: a wrong one places the +/// comment on an unrelated line of the file instead of failing. A review that +/// omits its side is resolved from the lists, and one that matches no changed +/// line is dropped. +fn resolve_review_sides(review_result: &mut ReviewResult, changed_lines: &ChangedLines) { + let mut dropped = Vec::new(); + + review_result.reviews.retain_mut(|review| { + let side = review.line.and_then(|line| { + let file = changed_lines + .iter() + .find(|file| file.filename == review.filename)?; + + let side = review.side.or_else(|| file.side_of(line))?; + + file.lines(side).contains(&line).then_some(side) + }); + + match side { + Some(side) => { + review.side = Some(side); + true + } + None => { + dropped.push(match review.line { + Some(line) => format!("{}:{line}", review.filename), + None => format!("{}:no line", review.filename), + }); + false + } + } + }); + + if !dropped.is_empty() { + warn!( + dropped = dropped.len(), + reviews = %dropped.join(", "), + "Dropped reviews that are not anchored on a changed line" + ); + } +} + +/// Extracts the old and new starting line numbers of a hunk header such as +/// `@@ -12,3 +12,5 @@`. The counts are optional and git may append a section +/// heading after the closing `@@`. +fn parse_hunk_starts(hunk_header: &str) -> Option<(u64, u64)> { + let body = hunk_header.strip_prefix("@@ ")?; + let body = body.split(" @@").next()?; + + let (old, new) = body.split_once(" +")?; + let old = old.strip_prefix('-')?.split(',').next()?; + let new = new.split(',').next()?; + + Some((old.parse().ok()?, new.parse().ok()?)) } #[cfg(test)] -#[test] -fn test_format_diff_for_review() { - let diff = concat!( +mod tests { + use super::*; + use crate::bot::ReviewItem; + + /// Additions, a removal and a line changed on both sides. + const DIFF: &str = concat!( "diff --git a/src/foo.rs b/src/foo.rs\n", "--- a/src/foo.rs\n", "+++ b/src/foo.rs\n", - "@@ -1,3 +1,6 @@\n", + "@@ -1,4 +1,6 @@\n", " fn main() {\n", "+ let x = 1;\n", + "- let removed = 0;\n", " println!(\"hello\");\n", "+ let y = 2;\n", "+ let z = 3;\n", @@ -274,14 +540,324 @@ fn test_format_diff_for_review() { "+ let b = 20;\n", ); - let result = format_diff_for_review(diff); - let expected = concat!( - "src/foo.rs:2: let x = 1;\n", - "src/foo.rs:4: let y = 2;\n", - "src/foo.rs:5: let z = 3;\n", - "src/bar.rs:11: let a = 10;\n", - "src/bar.rs:13: let b = 20;\n", + /// A pull request that only deletes a file. + const DELETION_ONLY: &str = concat!( + "diff --git a/src/old.rs b/src/old.rs\n", + "deleted file mode 100644\n", + "--- a/src/old.rs\n", + "+++ /dev/null\n", + "@@ -1,3 +0,0 @@\n", + "-fn a() {}\n", + "-fn b() {}\n", + "-fn c() {}\n", ); - assert_eq!(result, expected); + /// A hunk whose content starts with `+++` / `---`, which must not be taken + /// for the file headers. + const TRICKY_CONTENT: &str = concat!( + "diff --git a/src/tricky.js b/src/tricky.js\n", + "--- a/src/tricky.js\n", + "+++ b/src/tricky.js\n", + "@@ -1,4 +1,4 @@\n", + " let i = 0;\n", + "+++i;\n", + "---x;\n", + " console.log(i);\n", + ); + + /// A pull request that renames a file. + const RENAMED: &str = concat!( + "diff --git a/src/old.rs b/src/new.rs\n", + "similarity index 50%\n", + "rename from src/old.rs\n", + "rename to src/new.rs\n", + "--- a/src/old.rs\n", + "+++ b/src/new.rs\n", + "@@ -1,1 +1,1 @@\n", + "-fn old() {}\n", + "+fn new() {}\n", + ); + + /// A file whose path contains a space, which the `diff --git` line cannot + /// express without ambiguity. + const PATH_WITH_SPACE: &str = concat!( + "diff --git a/src/my file.rs b/src/my file.rs\n", + "--- a/src/my file.rs\n", + "+++ b/src/my file.rs\n", + "@@ -1,1 +1,2 @@\n", + " fn a() {}\n", + "+fn b() {}\n", + ); + + /// A file whose path git quotes and escapes (`caf\\303\\251.md` is + /// `caf\u{e9}.md`). + const QUOTED_PATH: &str = concat!( + "diff --git \"a/docs/caf\\303\\251.md\" \"b/docs/caf\\303\\251.md\"\n", + "--- \"a/docs/caf\\303\\251.md\"\n", + "+++ \"b/docs/caf\\303\\251.md\"\n", + "@@ -1,1 +1,2 @@\n", + " intro\n", + "+ajout\n", + ); + + fn review(filename: &str, line: Option, side: Option) -> ReviewItem { + ReviewItem { + filename: String::from(filename), + line, + side, + message: String::from("message"), + } + } + + fn review_result(reviews: Vec) -> ReviewResult { + ReviewResult { + reviews, + comment: String::new(), + cost: None, + } + } + + fn pull_request_file( + filename: &str, + previous_filename: Option<&str>, + status: &str, + ) -> PullRequestFile { + PullRequestFile { + filename: String::from(filename), + previous_filename: previous_filename.map(String::from), + status: String::from(status), + } + } + + #[test] + fn changed_lines_are_listed_per_file_and_side() { + let expected = concat!( + "src/foo.rs: added 2, 4, 5 / removed 2\n", + "src/bar.rs: added 11, 13" + ); + + assert_eq!(format_changed_lines(&parse_changed_lines(DIFF)), expected); + } + + #[test] + fn a_deletion_only_pull_request_lists_removed_lines() { + let expected = "src/old.rs: removed 1, 2, 3"; + + assert_eq!( + format_changed_lines(&parse_changed_lines(DELETION_ONLY)), + expected + ); + } + + #[test] + fn hunk_content_starting_with_plus_or_minus_is_counted() { + let expected = "src/tricky.js: added 2 / removed 2"; + + assert_eq!( + format_changed_lines(&parse_changed_lines(TRICKY_CONTENT)), + expected + ); + } + + #[test] + fn a_renamed_file_uses_its_new_path() { + let expected = "src/new.rs: added 1 / removed 1"; + + assert_eq!( + format_changed_lines(&parse_changed_lines(RENAMED)), + expected + ); + } + + #[test] + fn a_path_with_a_space_is_read_from_the_headers() { + let expected = "src/my file.rs: added 2"; + + assert_eq!( + format_changed_lines(&parse_changed_lines(PATH_WITH_SPACE)), + expected + ); + } + + #[test] + fn a_quoted_path_is_decoded() { + let expected = "docs/café.md: added 2"; + + assert_eq!( + format_changed_lines(&parse_changed_lines(QUOTED_PATH)), + expected + ); + } + + #[test] + fn filenames_are_resolved_against_the_api_list() { + let mut changed_lines = parse_changed_lines(DIFF); + let files = vec![ + pull_request_file("src/bar.rs", None, "modified"), + pull_request_file("src/foo.rs", Some("src/renamed.rs"), "renamed"), + ]; + + resolve_filenames(&mut changed_lines, &files); + + assert_eq!(changed_lines[0].filename, "src/foo.rs"); + assert_eq!(changed_lines[1].filename, "src/bar.rs"); + } + + #[test] + fn a_file_absent_from_the_api_list_keeps_the_diff_path() { + let mut changed_lines = parse_changed_lines(DIFF); + let files = vec![pull_request_file("src/bar.rs", None, "modified")]; + + resolve_filenames(&mut changed_lines, &files); + + assert_eq!(changed_lines[0].filename, "src/foo.rs"); + assert_eq!(changed_lines[1].filename, "src/bar.rs"); + } + + #[test] + fn changes_describe_the_files_then_the_lines() { + let changed_lines = parse_changed_lines(DELETION_ONLY); + let files = vec![pull_request_file("src/old.rs", None, "deleted")]; + + let expected = concat!( + "Files changed by the pull request: src/old.rs (deleted)\n", + "\n", + "Lines to review, per file:\n", + "src/old.rs: removed 1, 2, 3" + ); + + assert_eq!(format_changes(&files, &changed_lines), expected); + } + + #[test] + fn changes_without_the_api_list_only_hold_the_lines() { + let changed_lines = parse_changed_lines(DELETION_ONLY); + + let expected = "Lines to review, per file:\nsrc/old.rs: removed 1, 2, 3"; + + assert_eq!(format_changes(&[], &changed_lines), expected); + } + + #[test] + fn a_renamed_file_is_described_with_its_previous_path() { + let file = pull_request_file("src/new.rs", Some("src/old.rs"), "renamed"); + + assert_eq!(describe_file(&file), "src/new.rs (renamed from src/old.rs)"); + } + + #[test] + fn reviews_keep_their_changed_line_and_resolve_their_side() { + let changed_lines = parse_changed_lines(DIFF); + let mut review_result = review_result(vec![ + review("src/foo.rs", Some(4), Some(ReviewSide::Added)), + review("src/foo.rs", Some(2), Some(ReviewSide::Removed)), + review("src/bar.rs", Some(13), None), + ]); + + resolve_review_sides(&mut review_result, &changed_lines); + + let sides = review_result + .reviews + .iter() + .map(|review| review.side) + .collect::>(); + + assert_eq!( + sides, + vec![ + Some(ReviewSide::Added), + Some(ReviewSide::Removed), + Some(ReviewSide::Added) + ] + ); + } + + #[test] + fn a_line_changed_on_both_sides_defaults_to_added() { + let changed_lines = parse_changed_lines(DIFF); + let mut review_result = review_result(vec![review("src/foo.rs", Some(2), None)]); + + resolve_review_sides(&mut review_result, &changed_lines); + + assert_eq!(review_result.reviews[0].side, Some(ReviewSide::Added)); + } + + #[test] + fn reviews_outside_changed_lines_are_dropped() { + let changed_lines = parse_changed_lines(DIFF); + let mut review_result = review_result(vec![ + // valid + review("src/foo.rs", Some(2), Some(ReviewSide::Added)), + // a line that exists but is not part of the change + review("src/foo.rs", Some(3), None), + // a line beyond the change + review("src/foo.rs", Some(999), None), + // a changed line, but on the wrong side + review("src/foo.rs", Some(4), Some(ReviewSide::Removed)), + // a line changed in another file + review("src/foo.rs", Some(11), None), + // an unknown file + review("src/baz.rs", Some(1), None), + // no line at all + review("src/bar.rs", None, None), + ]); + + resolve_review_sides(&mut review_result, &changed_lines); + + assert_eq!(review_result.reviews.len(), 1); + assert_eq!(review_result.reviews[0].filename, "src/foo.rs"); + assert_eq!(review_result.reviews[0].line, Some(2)); + assert_eq!(review_result.reviews[0].side, Some(ReviewSide::Added)); + } + + #[test] + fn a_deleted_line_is_kept_as_a_removed_anchor() { + let changed_lines = parse_changed_lines(DELETION_ONLY); + let mut review_result = review_result(vec![ + review("src/old.rs", Some(2), Some(ReviewSide::Removed)), + review("src/old.rs", Some(2), Some(ReviewSide::Added)), + ]); + + resolve_review_sides(&mut review_result, &changed_lines); + + assert_eq!(review_result.reviews.len(), 1); + assert_eq!(review_result.reviews[0].side, Some(ReviewSide::Removed)); + } + + #[test] + fn odd_sides_from_the_model_are_tolerated() { + let changed_lines = parse_changed_lines(DIFF); + let mut review_result: ReviewResult = serde_json::from_str( + r#"{ + "reviews": [ + { "filename": "src/foo.rs", "line": 4, "side": "Added", "message": "a" }, + { "filename": "src/foo.rs", "line": 2, "side": "new", "message": "b" }, + { "filename": "src/foo.rs", "line": 2, "side": "REMOVED", "message": "c" }, + { "filename": "src/foo.rs", "line": 2, "side": "banana", "message": "d" }, + { "filename": "src/foo.rs", "line": 5, "message": "e" } + ], + "comment": "" + }"#, + ) + .unwrap(); + + resolve_review_sides(&mut review_result, &changed_lines); + + let sides = review_result + .reviews + .iter() + .map(|review| review.side) + .collect::>(); + + assert_eq!( + sides, + vec![ + Some(ReviewSide::Added), + Some(ReviewSide::Added), + Some(ReviewSide::Removed), + Some(ReviewSide::Added), + Some(ReviewSide::Added) + ] + ); + } } diff --git a/crates/herald-server/src/consts.rs b/crates/herald-server/src/consts.rs index 7dc6547..3d87a6a 100644 --- a/crates/herald-server/src/consts.rs +++ b/crates/herald-server/src/consts.rs @@ -27,25 +27,29 @@ pub const REVIEW_PROMPT: &str = " This is the user comment: \"{comment}\" - The code changes (only added lines, with line numbers): + The pull request changes these files and lines: - {diff} + {changes} - Please review the code changes and provide feedback. + `added` line numbers refer to the new version of the file, `removed` line + numbers to the old version, as they appear in the diff. - IMPORTANT: the `line` field must be the line number shown before each line. - The provided code has the format: `filename:line:code` + The code is not provided: read the files you need with the available tools + before answering. Review only the listed lines. Return your feedback, in french, with only this json format, reviews must contain each review All fields are mandatory. - (filename field must contain the full path with extension) and comment must contain a final summary: + (filename field must contain the full path with extension; line must be one of the + listed line numbers for that file, and side must be \"added\" when the line comes + from the `added` list or \"removed\" when it comes from the `removed` list) + and comment must contain a final summary: { \"reviews\": [ { \"filename\": \"\", \"line\": , - \"code\": \"\", + \"side\": \"\", \"message\": \"\" } ], diff --git a/crates/herald-server/src/env.rs b/crates/herald-server/src/env.rs index 6fa8ced..7cdf68d 100644 --- a/crates/herald-server/src/env.rs +++ b/crates/herald-server/src/env.rs @@ -13,7 +13,6 @@ pub struct EnvConfig { pub gitea_timeout: u64, pub metrics_bind_addr: Option, pub container_runtime: String, - pub sandbox_enabled: bool, pub sandbox_max_iterations: usize, } @@ -30,9 +29,6 @@ pub fn load_config() -> anyhow::Result { let metrics_bind_addr = std::env::var("METRICS_BIND_ADDR").ok(); let container_runtime = std::env::var("CONTAINER_RUNTIME").unwrap_or_else(|_| "docker".to_string()); - let sandbox_enabled = std::env::var("SANDBOX_ENABLED") - .map(|value| matches!(value.as_str(), "1" | "true" | "yes")) - .unwrap_or(false); let sandbox_max_iterations = std::env::var("SANDBOX_MAX_ITERATIONS") .ok() .and_then(|value| value.parse().ok()) @@ -50,7 +46,6 @@ pub fn load_config() -> anyhow::Result { gitea_timeout, metrics_bind_addr, container_runtime, - sandbox_enabled, sandbox_max_iterations, }) } diff --git a/crates/herald-server/src/gitea.rs b/crates/herald-server/src/gitea.rs index a86123c..13fac7e 100644 --- a/crates/herald-server/src/gitea.rs +++ b/crates/herald-server/src/gitea.rs @@ -1,10 +1,23 @@ use std::time::Duration; +use futures_util::stream::TryStreamExt; use serde::Deserialize; use serde_json::{Value, json}; -use tracing::instrument; +use tokio::io::AsyncReadExt; +use tokio_util::io::StreamReader; +use tracing::{instrument, warn}; -use crate::{bot::ReviewResult, errors::AppError}; +use crate::{ + bot::{ReviewResult, ReviewSide}, + consts::MAX_DIFF_SIZE, + errors::AppError, +}; + +/// Page size requested when listing the files of a pull request. +const FILE_PAGE_SIZE: u64 = 50; + +/// Maximum number of pages fetched for a pull request file list. +const MAX_FILE_PAGES: u64 = 10; #[derive(Clone)] pub struct GiteaAPI { @@ -135,6 +148,70 @@ impl GiteaAPI { Ok(()) } + /// Raw unified diff of a pull request. + /// + /// The API endpoint is used rather than the `diff_url` carried by the + /// webhook: that one points at a web route, which is session authenticated + /// and therefore does not serve private repositories to an API token. + #[instrument(skip(self))] + pub async fn pull_request_diff(&self, full_name: &str, index: u64) -> anyhow::Result { + let url = format!( + "{}/api/v1/repos/{}/pulls/{}.diff", + self.base_url, full_name, index + ); + + let res = self.client.get(url).send().await?; + + if !res.status().is_success() { + return Err(anyhow::anyhow!( + "Failed to download pull request diff: {}", + res.status() + )); + } + + read_capped(res).await + } + + /// Files changed by a pull request, with their exact path and status. + #[instrument(skip(self))] + pub async fn pull_request_files( + &self, + full_name: &str, + index: u64, + ) -> anyhow::Result> { + let mut files = Vec::new(); + + for page in 1..=MAX_FILE_PAGES { + let url = format!( + "{}/api/v1/repos/{}/pulls/{}/files?limit={FILE_PAGE_SIZE}&page={page}", + self.base_url, full_name, index + ); + + let res = self.client.get(url).send().await?; + + if !res.status().is_success() { + return Err(anyhow::anyhow!( + "Failed to list pull request files: {}", + res.status() + )); + } + + let page_files = res.json::>().await?; + + // The instance may cap the page size below the requested one, so a + // short page is not the end of the list: an empty one is. + if page_files.is_empty() { + return Ok(files); + } + + files.extend(page_files); + } + + warn!(files = files.len(), "Pull request file list was truncated"); + + Ok(files) + } + #[instrument(skip(self, review_result))] pub async fn post_pull_request_review( &self, @@ -148,19 +225,27 @@ impl GiteaAPI { self.base_url, full_name, index ); - let comments = &review_result + let comments = review_result .reviews .iter() - .filter(|r| r.line.is_some()) - .map(|r| { - let path = r.filename.clone(); - let line = r.line.unwrap_or(0); - let body = r.message.clone(); + .filter_map(|review| { + let line = review.line?; + let path = review.filename.clone(); + let body = review.message.clone(); - json!({ - "path": path, - "new_position": line, - "body": body + // A line removed by the pull request only exists in the old + // version of the file, so it is anchored with `old_position`. + Some(match review.side { + Some(ReviewSide::Removed) => json!({ + "path": path, + "old_position": line, + "body": body + }), + _ => json!({ + "path": path, + "new_position": line, + "body": body + }), }) }) .collect::>(); @@ -184,6 +269,23 @@ impl GiteaAPI { } } +/// Reads a response body, refusing to buffer more than [`MAX_DIFF_SIZE`]. +async fn read_capped(response: reqwest::Response) -> anyhow::Result { + let stream = response.bytes_stream().map_err(std::io::Error::other); + + let mut buf = Vec::with_capacity(MAX_DIFF_SIZE); + StreamReader::new(stream) + .take((MAX_DIFF_SIZE + 1) as u64) + .read_to_end(&mut buf) + .await?; + + if buf.len() > MAX_DIFF_SIZE { + anyhow::bail!("Pull request diff exceeds the maximum allowed size of 1 MiB"); + } + + Ok(String::from_utf8_lossy(&buf).into_owned()) +} + #[derive(Debug)] pub enum WebhookType { Review(ReviewPayload), @@ -213,7 +315,6 @@ pub struct ReviewPayload { #[derive(Deserialize, Debug)] pub struct PullRequest { - pub diff_url: String, pub number: u64, pub title: String, } @@ -234,6 +335,18 @@ pub struct Repository { pub full_name: String, } +/// A file changed by a pull request, as reported by the API. +#[derive(Deserialize, Debug)] +pub struct PullRequestFile { + /// Path of the file in the new version of the repository. + pub filename: String, + /// Previous path, for a renamed file. + #[serde(default)] + pub previous_filename: Option, + /// `added`, `modified`, `deleted`, `renamed`… + pub status: String, +} + impl WebhookType { pub fn from_event(event: &str, bot_name: &str, json: Value) -> Result { let wb = match event { diff --git a/crates/herald-server/src/main.rs b/crates/herald-server/src/main.rs index 2c313e4..d6d4b0b 100644 --- a/crates/herald-server/src/main.rs +++ b/crates/herald-server/src/main.rs @@ -79,15 +79,14 @@ async fn run() -> anyhow::Result<()> { let shutdown = CancellationToken::new(); let sandbox = SandboxConfig { - enabled: config.sandbox_enabled, runtime: devcontainer_rs::ContainerRuntime::new(config.container_runtime.clone()), max_iterations: config.sandbox_max_iterations, }; - if sandbox.enabled && !sandbox.runtime.available().await { + if !sandbox.runtime.available().await { warn!( runtime = sandbox.runtime.program(), - "Sandbox is enabled but the container runtime is not available" + "Container runtime is not available, every review will fail" ); } @@ -95,7 +94,6 @@ async fn run() -> anyhow::Result<()> { gitea_user.login, gitea_api, open_router_client, - reqwest::Client::new(), config.bot_max_concurrent, config.open_router_model.clone(), sandbox, diff --git a/crates/herald-server/src/open_router.rs b/crates/herald-server/src/open_router.rs index dd093ac..8709091 100644 --- a/crates/herald-server/src/open_router.rs +++ b/crates/herald-server/src/open_router.rs @@ -1,63 +1,253 @@ +//! Minimal OpenRouter chat-completions client. +//! +//! Herald only needs a non-streaming `POST /chat/completions` with optional +//! tool calling, so the wire types are implemented in-tree instead of pulling a +//! third-party SDK (and its own `reqwest` version) into the workspace. +//! +//! Only the response fields Herald consumes are modelled: `content`, +//! `tool_calls` and `usage.cost`. Unknown fields are ignored. + use std::time::Duration; -use openrouter_rs::{ - Message, - api::chat::ChatCompletionRequest, - types::{Tool, ToolCall}, -}; +use anyhow::Context; +use serde::{Deserialize, Serialize}; +use serde_json::Value; use tracing::instrument; -pub struct ChatResult { - pub message: String, - pub cost: Option, -} +/// OpenRouter API root, version prefix included. +const BASE_URL: &str = "https://openrouter.ai/api/v1"; +/// The model decides on its own which tool to call. +const TOOL_CHOICE_AUTO: &str = "auto"; + +/// Result of a completion that may contain tool calls. pub struct ToolChatResult { pub message: Option, pub tool_calls: Vec, pub cost: Option, } +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum Role { + System, + User, + Assistant, + Tool, +} + +/// A single turn of the conversation. +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct Message { + pub role: Role, + #[serde(skip_serializing_if = "Option::is_none")] + pub content: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub tool_calls: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub tool_call_id: Option, +} + +impl Message { + pub fn new(role: Role, content: impl Into) -> Self { + Self { + role, + content: Some(content.into()), + tool_calls: None, + tool_call_id: None, + } + } + + /// Assistant turn requesting tool calls. Models often answer with tool + /// calls but no text, in which case the content is sent as `null`. + pub fn assistant_with_tool_calls( + content: impl Into, + tool_calls: Vec, + ) -> Self { + let content = content.into(); + + Self { + role: Role::Assistant, + content: (!content.is_empty()).then_some(content), + tool_calls: Some(tool_calls), + tool_call_id: None, + } + } + + /// Result of a tool call, linked to the request by `tool_call_id`. + pub fn tool_response(tool_call_id: impl Into, content: impl Into) -> Self { + Self { + role: Role::Tool, + content: Some(content.into()), + tool_calls: None, + tool_call_id: Some(tool_call_id.into()), + } + } +} + +/// A tool the model may call. +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct Tool { + #[serde(rename = "type", default = "function_type")] + pub kind: String, + pub function: FunctionDefinition, +} + +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct FunctionDefinition { + pub name: String, + pub description: String, + /// JSON schema describing the accepted arguments. + pub parameters: Value, +} + +impl Tool { + pub fn new(name: impl Into, description: impl Into, parameters: Value) -> Self { + Self { + kind: function_type(), + function: FunctionDefinition { + name: name.into(), + description: description.into(), + parameters, + }, + } + } +} + +/// A tool call requested by the model. +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct ToolCall { + pub id: String, + #[serde(rename = "type", default = "function_type")] + pub kind: String, + pub function: FunctionCall, +} + +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct FunctionCall { + pub name: String, + /// Arguments as a JSON-encoded string. Kept verbatim so that echoing the + /// call back into the conversation does not re-encode or corrupt it. + pub arguments: String, +} + +impl ToolCall { + pub fn id(&self) -> &str { + &self.id + } + + pub fn name(&self) -> &str { + &self.function.name + } + + pub fn arguments_json(&self) -> &str { + &self.function.arguments + } +} + +fn function_type() -> String { + String::from("function") +} + +#[derive(Serialize)] +struct ChatRequest<'a> { + model: &'a str, + messages: &'a [Message], + reasoning: Reasoning, + tools: &'a [Tool], + tool_choice: &'a str, +} + +/// OpenRouter reasoning config; Herald always asks for reasoning. +#[derive(Serialize)] +struct Reasoning { + enabled: bool, +} + +#[derive(Deserialize)] +struct ChatResponse { + #[serde(default)] + choices: Vec, + #[serde(default)] + usage: Option, +} + +#[derive(Deserialize)] +struct Choice { + message: ResponseMessage, +} + +#[derive(Deserialize)] +struct ResponseMessage { + #[serde(default)] + content: Option, + #[serde(default)] + tool_calls: Option>, +} + +/// Message content is a plain string for most models and a list of typed parts +/// for others; both are collapsed to text. +#[derive(Deserialize)] +#[serde(untagged)] +enum Content { + Text(String), + Parts(Vec), +} + +#[derive(Deserialize)] +struct ContentPart { + #[serde(default)] + text: Option, +} + +impl Content { + fn into_text(self) -> String { + match self { + Self::Text(text) => text, + Self::Parts(parts) => parts + .into_iter() + .filter_map(|part| part.text) + .collect::>() + .join(""), + } + } +} + +#[derive(Deserialize)] +struct Usage { + #[serde(default)] + cost: Option, +} + +/// Error payload returned by OpenRouter for a failed request. +#[derive(Deserialize)] +struct ErrorResponse { + error: ErrorDetail, +} + +#[derive(Deserialize)] +struct ErrorDetail { + message: String, +} + #[derive(Clone)] pub struct OpenRouterClient { - client: openrouter_rs::OpenRouterClient, + client: reqwest::Client, + api_key: String, model: String, } impl OpenRouterClient { pub fn new(token: &str, model: &str, timeout: u64) -> anyhow::Result { Ok(Self { - client: openrouter_rs::OpenRouterClient::builder() - .api_key(token) - .http_client( - reqwest::Client::builder() - .timeout(Duration::from_secs(timeout)) - .build()?, - ) + client: reqwest::Client::builder() + .timeout(Duration::from_secs(timeout)) .build()?, + api_key: String::from(token), model: String::from(model), }) } - #[instrument(skip(self), err)] - pub async fn chat(&self, msg: &str) -> anyhow::Result { - let request = ChatCompletionRequest::builder() - .model(&self.model) - .enable_reasoning() - .messages(vec![Message::new(openrouter_rs::types::Role::User, msg)]) - .build()?; - - let response = self.client.chat().create(&request).await?; - - Ok(ChatResult { - message: response.choices[0] - .content() - .map(String::from) - .ok_or(anyhow::anyhow!("No content"))?, - cost: response.usage.and_then(|u| u.cost), - }) - } - /// Sends a conversation with tool definitions and returns either a final /// message or the tool calls requested by the model. #[instrument(skip(self, messages, tools), err)] @@ -66,27 +256,186 @@ impl OpenRouterClient { messages: Vec, tools: Vec, ) -> anyhow::Result { - let request = ChatCompletionRequest::builder() - .model(&self.model) - .enable_reasoning() - .messages(messages) - .tools(tools) - .tool_choice_auto() - .build()?; + let response = self.complete(&messages, &tools).await?; - let response = self.client.chat().create(&request).await?; - let choice = response + let cost = response.usage.and_then(|usage| usage.cost); + let message = response .choices - .first() - .ok_or_else(|| anyhow::anyhow!("No choices in response"))?; + .into_iter() + .next() + .ok_or_else(|| anyhow::anyhow!("No choices in response"))? + .message; Ok(ToolChatResult { - message: choice.content().map(String::from), - tool_calls: choice - .tool_calls() - .map(<[ToolCall]>::to_vec) - .unwrap_or_default(), - cost: response.usage.and_then(|u| u.cost), + message: message.content.map(Content::into_text), + tool_calls: message.tool_calls.unwrap_or_default(), + cost, }) } + + async fn complete(&self, messages: &[Message], tools: &[Tool]) -> anyhow::Result { + let request = ChatRequest { + model: &self.model, + messages, + reasoning: Reasoning { enabled: true }, + tools, + tool_choice: TOOL_CHOICE_AUTO, + }; + + let response = self + .client + .post(format!("{BASE_URL}/chat/completions")) + .bearer_auth(&self.api_key) + .json(&request) + .send() + .await + .context("failed to reach OpenRouter")?; + + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + anyhow::bail!("OpenRouter returned {status}: {}", error_message(&body)); + } + + response + .json::() + .await + .context("invalid OpenRouter response") + } +} + +/// Extracts the message from an OpenRouter error body, falling back to the raw +/// body when it is not the expected JSON shape. +fn error_message(body: &str) -> String { + serde_json::from_str::(body) + .map(|response| response.error.message) + .unwrap_or_else(|_| body.trim().to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + /// Builds a tool call the way the API returns one, so the fixture also + /// covers deserialization. + fn tool_call(id: &str, name: &str, arguments: &str) -> ToolCall { + serde_json::from_value(json!({ + "id": id, + "type": "function", + "function": { "name": name, "arguments": arguments } + })) + .unwrap() + } + + #[test] + fn tool_call_arguments_are_kept_verbatim() { + let call = tool_call("call_1", "grep", r#"{"pattern":"fn main"}"#); + let message = Message::assistant_with_tool_calls("", vec![call]); + + let serialized = serde_json::to_value(&message).unwrap(); + + assert_eq!(serialized["content"], Value::Null); + assert_eq!(serialized["tool_calls"][0]["type"], "function"); + assert_eq!(serialized["tool_calls"][0]["function"]["name"], "grep"); + assert_eq!( + serialized["tool_calls"][0]["function"]["arguments"], + r#"{"pattern":"fn main"}"# + ); + } + + #[test] + fn tool_response_carries_role_and_tool_call_id() { + let message = Message::tool_response("call_1", "src/main.rs"); + let serialized = serde_json::to_value(&message).unwrap(); + + assert_eq!(serialized["role"], "tool"); + assert_eq!(serialized["tool_call_id"], "call_1"); + assert_eq!(serialized["content"], "src/main.rs"); + } + + #[test] + fn request_serializes_tool_choice_and_reasoning() { + let messages = [Message::new(Role::User, "hi")]; + let tools = [Tool::new("ls", "List files", json!({"type": "object"}))]; + + let request = serde_json::to_value(ChatRequest { + model: "some/model", + messages: &messages, + reasoning: Reasoning { enabled: true }, + tools: &tools, + tool_choice: TOOL_CHOICE_AUTO, + }) + .unwrap(); + + assert_eq!(request["model"], "some/model"); + assert_eq!(request["reasoning"]["enabled"], true); + assert_eq!(request["tool_choice"], "auto"); + assert_eq!(request["tools"][0]["function"]["name"], "ls"); + assert_eq!(request["messages"][0]["role"], "user"); + } + + #[test] + fn response_parses_tool_calls_and_cost() { + let response: ChatResponse = serde_json::from_value(json!({ + "choices": [{ + "message": { + "role": "assistant", + "content": null, + "tool_calls": [{ + "id": "call_1", + "type": "function", + "function": { "name": "ls", "arguments": "{\"path\":\"src\"}" } + }] + } + }], + "usage": { "cost": 0.0021 } + })) + .unwrap(); + + assert_eq!(response.usage.and_then(|usage| usage.cost), Some(0.0021)); + + let message = response.choices.into_iter().next().unwrap().message; + assert!(message.content.is_none()); + + let calls = message.tool_calls.unwrap(); + assert_eq!(calls[0].name(), "ls"); + assert_eq!(calls[0].id(), "call_1"); + assert_eq!(calls[0].arguments_json(), r#"{"path":"src"}"#); + } + + #[test] + fn response_parses_parts_content() { + let response: ChatResponse = serde_json::from_value(json!({ + "choices": [{ + "message": { + "role": "assistant", + "content": [ + { "type": "text", "text": "hello " }, + { "type": "text", "text": "world" } + ] + } + }] + })) + .unwrap(); + + assert!(response.usage.is_none()); + + let message = response.choices.into_iter().next().unwrap().message; + assert_eq!(message.content.unwrap().into_text(), "hello world"); + } + + #[test] + fn error_message_prefers_api_message() { + let body = r#"{"error":{"message":"No auth credentials found","code":401}}"#; + assert_eq!(error_message(body), "No auth credentials found"); + } + + #[test] + fn error_message_falls_back_to_raw_body() { + assert_eq!( + error_message(" bad gateway "), + "bad gateway" + ); + } } diff --git a/crates/herald-server/src/sandbox/agent.rs b/crates/herald-server/src/sandbox/agent.rs index 35d53c4..bf9e1e2 100644 --- a/crates/herald-server/src/sandbox/agent.rs +++ b/crates/herald-server/src/sandbox/agent.rs @@ -6,15 +6,11 @@ //! exhausted. use anyhow::Context; -use openrouter_rs::{ - Message, - types::{Role, Tool, ToolCall}, -}; use serde_json::Value; use tracing::{debug, warn}; use crate::{ - open_router::OpenRouterClient, + open_router::{Message, OpenRouterClient, Role, Tool, ToolCall}, sandbox::{Sandbox, tools}, }; @@ -105,10 +101,22 @@ fn parse_args(call: &ToolCall) -> anyhow::Result { #[cfg(test)] mod tests { use super::*; + use serde_json::json; + + /// Builds a tool call the way the API returns one, so the fixture also + /// covers deserialization. + fn tool_call(name: &str, arguments: &str) -> ToolCall { + serde_json::from_value(json!({ + "id": "call_1", + "type": "function", + "function": { "name": name, "arguments": arguments } + })) + .unwrap() + } #[test] fn parse_args_accepts_empty_arguments() { - let call = ToolCall::new("id", "ls", ""); + let call = tool_call("ls", ""); assert_eq!( parse_args(&call).unwrap(), Value::Object(serde_json::Map::new()) @@ -117,13 +125,13 @@ mod tests { #[test] fn parse_args_parses_json_object() { - let call = ToolCall::new("id", "ls", r#"{"path":"src"}"#); + let call = tool_call("ls", r#"{"path":"src"}"#); assert_eq!(parse_args(&call).unwrap()["path"], "src"); } #[test] fn parse_args_rejects_invalid_json() { - let call = ToolCall::new("id", "ls", "not json"); + let call = tool_call("ls", "not json"); assert!(parse_args(&call).is_err()); } } diff --git a/crates/herald-server/src/sandbox/mod.rs b/crates/herald-server/src/sandbox/mod.rs index f31a507..362376a 100644 --- a/crates/herald-server/src/sandbox/mod.rs +++ b/crates/herald-server/src/sandbox/mod.rs @@ -25,8 +25,6 @@ const DEVCONTAINER_PATHS: [&str; 2] = [".devcontainer/devcontainer.json", ".devc /// Sandbox-related runtime configuration. #[derive(Clone)] pub struct SandboxConfig { - /// Whether the bot should run its tools inside a sandbox container. - pub enabled: bool, /// Container runtime binary to drive (e.g. `docker`, `podman`). pub runtime: ContainerRuntime, /// Maximum number of tool-calling iterations per agent run. diff --git a/crates/herald-server/src/sandbox/tools.rs b/crates/herald-server/src/sandbox/tools.rs index a595984..bf318e6 100644 --- a/crates/herald-server/src/sandbox/tools.rs +++ b/crates/herald-server/src/sandbox/tools.rs @@ -10,11 +10,10 @@ use std::path::Path; use anyhow::{Context, bail}; use devcontainer_rs::{ExecOutput, normalize}; -use openrouter_rs::types::Tool; use serde_json::{Value, json}; use super::Sandbox; -use crate::gitea::WebhookType; +use crate::{gitea::WebhookType, open_router::Tool}; /// Tools available to the model for a given webhook action. /// @@ -216,7 +215,6 @@ mod tests { WebhookType::Review(ReviewPayload { action: "created".to_string(), pull_request: PullRequest { - diff_url: "https://example.com/diff".to_string(), number: 1, title: "My PR".to_string(), },