Conversation
There was a problem hiding this comment.
Lots of cruft was gathering in the other PR, thanks for fixing and getting this up on its feet!
I realize this is still in Draft mode, not yet ready for review, so leaving this as a comment.
Just one structural thing - kindof a nit but makes it a lot easier to grok the repo and see when changes are just visual, can you please move all the gui things into one top-level gui directory?
So, /app becomes /gui (or a subdirectory) and /packages/gui also moves into /gui somewhere?
We'll still need some other top-level changes in this PR, so we can hook it all together, but that'll make it much easier to review and understand going forward
I'm also not a big fan of their default naming, src-tauri is confusing, I'm biased towards the way I named things in https://github.com/Lay3rLabs/WAVS/tree/c097a189082e04b12be942e5f2dc67253c7e325a/packages/gui with:
- backend
- frontend
- media
- shared
but it's not a showstopper... and as long as all this stuff is self-contained in /gui, we can use the tauri defaults and vibecode it and treat it as "if it looks right it is right"
685c2a1 to
4f8a0a0
Compare
dakom
left a comment
There was a problem hiding this comment.
This was introduced in #1126.
@copilot Not requesting code changes — looking for an architectural evaluation of packages/wavs-mcp
Please comment with your review, do not make any changes:
- How much of the MCP server's functionality duplicates what a generic OpenAPI/Swagger-to-MCP bridge would provide, given that the WAVS HTTP API is already documented via OpenAPI?
- Whether embedding private key handling (WAVS_CHAIN_WRITE_CREDENTIAL, WAVS_SIGNING_MNEMONIC) in an MCP server — where an AI can autonomously send on-chain transactions — is a good security posture.
- The scaffold tool generates WASM component boilerplate, but components are generic WASM concepts — it's services (which tie triggers to workflows and components) that are WAVS-specific. Does the scaffolding actually help with the WAVS-specific part, or is it just generating generic WASM component code that doesn't reflect how services are configured and deployed?
- Whether this package would be better maintained separately from the core WAVS repo, to avoid coupling MCP tooling to the core release cycle.
|
On 1 — OpenAPI bridge overlap A generic OpenAPI-to-MCP bridge exposes raw HTTP semantics: JSON request bodies, path parameters, auth headers. That works for exploration, but it means the AI is constructing AddServiceRequest JSON blobs and managing auth headers manually — not a great experience. The MCP tools provide a curated layer above that: each tool has a typed schema, a description that communicates intent and preconditions, and unified credential management across all operations (the distinction between --token for node auth vs --chain-write-credential for on-chain ops isn't obvious from an OpenAPI spec). wavs_deploy_dev_service is a concrete example: it orchestrates a two-step save-then-register flow that the raw API exposes as separate calls with no obvious ordering guidance. More importantly, the chain-write and local tooling (7 of 22 tools) can't be bridged from OpenAPI at all — they're EVM contract interactions and local build operations. Users need this package anyway. Splitting "use the generic bridge for these 15, use the MCP package for these 7" is a worse DX than a single coherent toolset. On 2 — Private key handling This is opt-in. The MCP server operates fully without credentials for read-only use. Chain-write capabilities are only active if you explicitly provide --chain-write-credential. Same posture as any dev tool that can optionally sign transactions. On 3 — Scaffolding scope Fair point. On 4 — Repo coupling The MCP package imports wavs-types and utils from the workspace and embeds WIT files from the project tree. In the same repo, staying in sync is free — any API change that breaks the MCP tools fails immediately. Moving it to a separate repo turns that into a version matrix: publish wavs-types, cut a new MCP release, update the pin. That overhead only makes sense once there's a concrete reason (different release cadence, active external contributors who need isolation). Right now there isn't one IMO. |
I don't understand this point. There is no other way to communicate to the server except with these "raw HTTP semantics"... it's not a great experience for a robot, but that's okay. For a human using an agent - yeah, the API alone doesn't document intent - that's what OpenAPI/Swagger is for. Agents can consume that perfectly fine. If MCP is helpful here, my point isn't "don't add MCP", it's "don't re-invent the wheel, OpenAPI->MCP solutions already exist" But tbh it's not clear to me that MCP is actually more helpful than a simple markdown file telling the agent to consume the OpenAPI docs. (again - not saying "don't expose an MCP server", it's just that the http part of it isn't a huge win by itself) Is there something you wanted your agent to do, in terms of talking to the wavs http server, that it failed to do properly? Odds are the fix there is just better docs and publishing the OpenAPI docs cleanly.
What are these tools? WAVS doesn't know about any specific contract shapes. We deliberately decoupled that and it sits in different middleware repos, wavs-tools, etc. This was intentional to make sure we didn't accidentally bake in any assumptions in WAVS itself. In general our approach is not to talk to middleware contracts directly, but rather use the docker commands - and those docker images exist outside this repo. Similar but different to above, my point isn't "don't add MCP". This is a use-case where the agent legitimately won't know what the middleware images are capable of in a standard way (there's no OpenAPI). My point is, "that's a great thing to have, so let's add MCP in its own repo where it can submodule and access other projects"
Fair enough, as long as the MCP keys aren't conflated with aggregator/signing keys, you're right that it's just the risk users take and that's their choice 👍
Much of what your MCP server is doing is not defined in this repo. Some examples:
For better or worse we are not structured as a monorepo. @reecepbcups gave the green light to deprecate the separate wavs-wasi repo and move it here, but that won't change much - the MCP server wants to do a lot more than just hit the wavs http api and look at the wit definitions - in fact the main thing it needs to do that can't be done by consuming OpenAPI docs is talk to the middleware stuff, which is not here I think the more elegant and poweful solution is to start a new We can then make changes in the various repos as needed to make it easier for the mcp to work with and use, which would make more sense to then make corresponding changes to the For example, let's say we add new commands to the poa middleware that aren't needed in wavs at all but are useful for agents working with the contracts, or eigenlayer makes changes that don't affect wavs core. It'll be odd to have to make code changes here just to update the mcp when it really has nothing to do with wavs itself. |
194966a to
ba59f12
Compare
|
Setting aside where the MCP should live for a second, I know there is a lot to review here but the main thing I would like looked at is anything that touches wavs core packages. Had to make few additions to the http server which I think are sensible, but some other files we touched as well, to better handle restarting the app and reloading services or being able to show which submissions were successful. Open to other approaches here. |
There was a problem hiding this comment.
Pull request overview
Introduces a new WAVS desktop (Tauri) app and MCP tooling, alongside node/API enhancements to support GUI workflows (service lifecycle, persistence, and developer inspection endpoints).
Changes:
- Added a Tauri + React desktop app (
app/) with a Rust backend bridge (app/src-tauri/) and shared GUI types/events (packages/gui/shared). - Added
wavs-mcp(Rust binary + npm package) for Model Context Protocol integrations, plus Claude skill documentation and setup automation. - Enhanced WAVS node HTTP/dev capabilities: service pause/resume, persisted service registry status, dev-service persistence across restarts, KV listing, and filesystem browsing.
Reviewed changes
Copilot reviewed 158 out of 229 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| wit-definitions/aggregator/wit/deps/wavs-operator-2.7.0/package.wit | Adds WIT package definition for wavs:operator@2.7.0 used by components/tooling. |
| packages/wavs/tests/wavs_systems/mock_app.rs | Updates dispatcher init to include GUI/Tauri handle. |
| packages/wavs/src/subsystems/aggregator.rs | Emits dispatcher event on submission confirmation for UI/activity updates. |
| packages/wavs/src/service_registry.rs | Persists service status (active/paused) and adds status mutation + tests. |
| packages/wavs/src/main.rs | Updates dispatcher construction with Tauri handle. |
| packages/wavs/src/lib.rs | Delays dispatcher restore until HTTP port bound; improves thread panic logging. |
| packages/wavs/src/http/state.rs | Persists dev services to disk and reloads them on restart. |
| packages/wavs/src/http/server.rs | Adds pause/resume + KV list + FS dev routes and readiness signaling. |
| packages/wavs/src/http/handlers/service/pause.rs | Implements pause/resume service HTTP handlers. |
| packages/wavs/src/http/handlers/service.rs | Exposes pause module in service handler tree. |
| packages/wavs/src/http/handlers/mod.rs | Exposes fs handler module. |
| packages/wavs/src/http/handlers/kv.rs | Adds KV listing endpoint returning base64 values. |
| packages/wavs/src/http/handlers/fs.rs | Adds dev filesystem browse/read endpoints with traversal protection. |
| packages/wavs/benches/dev_triggers/setup.rs | Updates benchmark dispatcher init with Tauri handle. |
| packages/wavs/Cargo.toml | Adds GUI feature/deps and base64 dependency for new endpoints. |
| packages/wavs-mcp/src/scaffold.rs | Adds WIT export + component scaffolding helpers for MCP local tools. |
| packages/wavs-mcp/src/main.rs | Adds MCP server CLI with safer credential lookup behavior. |
| packages/wavs-mcp/scripts/copy-skill-files.mjs | Prepack helper to ship skill files in npm package. |
| packages/wavs-mcp/postinstall.mjs | Downloads platform-specific wavs-mcp binary after npm install. |
| packages/wavs-mcp/package.json | Defines npm package metadata, bin entry, and lifecycle scripts. |
| packages/wavs-mcp/README.md | Documents installing/running wavs-mcp and MCP client integration. |
| packages/wavs-mcp/Cargo.toml | Adds Rust crate for wavs-mcp server binary. |
| packages/utils/src/service.rs | Adds logging + IPFS local gateway fallback during service fetch. |
| packages/utils/src/evm_client.rs | Switches nonce reads to pending nonce for safer tx submission. |
| packages/utils/src/context.rs | Introduces AnyRuntime to support embedding into an existing Tokio runtime. |
| packages/types/src/http.rs | Adds PauseServiceRequest request type. |
| packages/layer-tests/src/e2e/handles.rs | Updates dispatcher init signature for tests. |
| packages/gui/shared/src/settings.rs | Adds GUI settings persistence schema (registries/services/MCP flags). |
| packages/gui/shared/src/lib.rs | Exposes gui shared modules. |
| packages/gui/shared/src/event.rs | Adds shared event payloads and emitter trait for Tauri. |
| packages/gui/shared/src/error.rs | Adds shared GUI error type used across frontend/backend. |
| packages/gui/shared/src/command.rs | Adds shared command response types for frontend/backend. |
| packages/gui/shared/Cargo.toml | Adds wavs-gui-shared crate definition and backend feature flags. |
| justfile | Adds app dev/build recipes + Claude/MCP setup commands and ABI copy. |
| examples/contracts/solidity/abi/ISimpleTrigger.sol/ISimpleTrigger.json | Updates ABI JSON metadata (evmVersion change). |
| checksums.txt | Updates component build artifact checksums. |
| app/vite.config.ts | Adds Vite config for Tauri dev/build behavior. |
| app/tsconfig.node.json | Adds TS config for Vite config typechecking. |
| app/tsconfig.json | Adds TS config for React/Vite frontend. |
| app/tailwind.config.js | Adds Tailwind theme/config for GUI styling. |
| app/src/utils/error.ts | Adds helper to extract human-readable errors from Tauri responses. |
| app/src/tauri/listeners.ts | Adds Tauri event listeners (settings/log/trigger/submission/service). |
| app/src/tauri/index.ts | Re-exports Tauri command/listener utilities. |
| app/src/tauri/commands.ts | Defines frontend invoke wrappers for backend commands. |
| app/src/stores/walletStore.ts | Adds mnemonic/keychain state management + address derivation. |
| app/src/stores/poaStore.ts | Adds PoA registry state + persistence helpers. |
| app/src/stores/appStore.ts | Adds global app state (settings, logs, activity, services). |
| app/src/pages/services/index.ts | Exposes services pages from a central index. |
| app/src/pages/services/ServicesLayout.tsx | Loads services/registries and builds breadcrumbs for service navigation. |
| app/src/pages/services/ServiceListPage.tsx | Adds services list UI and registry/service correlation. |
| app/src/pages/services/ServiceBuilderPage.tsx | Adds service builder page routing and query-param preselection. |
| app/src/pages/index.ts | Exposes top-level pages index. |
| app/src/pages/NotFound.tsx | Adds 404 page. |
| app/src/pages/Health.tsx | Adds health + MCP status UI page. |
| app/src/pages/Activity.tsx | Adds activity page wrapper for activity feed component. |
| app/src/main.tsx | Adds React app bootstrap. |
| app/src/index.css | Adds global styles and scrollbar styling. |
| app/src/hooks/useViemClient.ts | Adds viem clients using keychain mnemonic for signing. |
| app/src/components/service/index.ts | Exposes service components barrel exports. |
| app/src/components/service/WorkflowViewer.tsx | Adds workflow viewer UI for service/workflow details. |
| app/src/components/service/WorkflowEditor.tsx | Adds workflow editor UI for service builder. |
| app/src/components/service/SubmitEditor.tsx | Adds submit configuration editor in service builder flow. |
| app/src/components/service/ServiceBuilder.tsx | Adds multi-step service builder UI flow. |
| app/src/components/service/ServiceActivity.tsx | Adds per-service activity feed wrapper. |
| app/src/components/service/ContractStep.tsx | Adds contract selection step for service builder. |
| app/src/components/poa/index.ts | Exposes PoA components barrel exports. |
| app/src/components/poa/RegistryInfo.tsx | Adds PoA registry info UI component. |
| app/src/components/layout/index.ts | Exposes layout components barrel exports. |
| app/src/components/layout/HealthIndicator.tsx | Adds header health indicator with polling and tooltip. |
| app/src/components/layout/Header.tsx | Adds top navigation header with health indicator and routing. |
| app/src/components/layout/Body.tsx | Adds layout body container for routed content. |
| app/src/components/atoms/index.ts | Exposes atom components barrel exports. |
| app/src/components/atoms/TomlEditor.tsx | Adds CodeMirror TOML editor component for config editing. |
| app/src/components/atoms/Toast.tsx | Adds toast notification system. |
| app/src/components/atoms/TextInput.tsx | Adds styled text input component with password reveal. |
| app/src/components/atoms/TextArea.tsx | Adds styled textarea component. |
| app/src/components/atoms/Tabs.tsx | Adds tabs component. |
| app/src/components/atoms/Modal.tsx | Adds global modal system. |
| app/src/components/atoms/Expander.tsx | Adds expandable section component. |
| app/src/components/atoms/DropdownMenu.tsx | Adds dropdown menu component. |
| app/src/components/atoms/Dropdown.tsx | Adds dropdown select component. |
| app/src/components/atoms/CloseX.tsx | Adds reusable close button component. |
| app/src/components/atoms/Button.tsx | Adds button component with variants/sizes/selection. |
| app/src/components/atoms/Breadcrumb.tsx | Adds breadcrumb navigation component. |
| app/src/components/atoms/AddressDisplay.tsx | Adds address display with click-to-copy. |
| app/src/App.tsx | Adds main router, initialization, listeners, and boot flow (wallet/settings). |
| app/src-tauri/tauri.conf.json | Adds Tauri v2 app configuration and CSP/asset protocol scope. |
| app/src-tauri/src/state.rs | Adds persisted settings + WAVS instance state + mnemonic & MCP process state. |
| app/src-tauri/src/main.rs | Adds Tauri binary entrypoint. |
| app/src-tauri/src/logger.rs | Adds tracing layer that forwards logs to frontend events. |
| app/src-tauri/src/lib.rs | Builds Tauri app, wires commands/plugins, and configures window/logging/states. |
| app/src-tauri/icons/ios/wavs.icon/icon.json | Adds iOS icon configuration for Tauri bundle assets. |
| app/src-tauri/icons/android/values/ic_launcher_background.xml | Adds Android launcher background color resource. |
| app/src-tauri/icons/android/mipmap-anydpi-v26/ic_launcher.xml | Adds Android adaptive icon config. |
| app/src-tauri/capabilities/default.json | Adds Tauri capability permissions definition. |
| app/src-tauri/build.rs | Adds Tauri build script entry. |
| app/src-tauri/Cargo.toml | Adds Tauri backend crate definition and dependencies. |
| app/src-tauri/.gitignore | Ignores Tauri/Cargo generated artifacts under src-tauri. |
| app/postcss.config.js | Adds Tailwind/PostCSS config. |
| app/package.json | Adds frontend dependencies/scripts for Tauri+React app build. |
| app/index.html | Adds Vite HTML entry. |
| app/README.md | Adds basic frontend template README. |
| app/PLAN.md | Adds implementation plan/notes for app and MCP features. |
| app/.vite/deps/package.json | Adds Vite optimized deps metadata (build cache). |
| app/.vite/deps/_metadata.json | Adds Vite optimized deps metadata (build cache). |
| app/.gitignore | Adds frontend gitignore (node_modules/dist/etc.). |
| README.md | Adds Claude Code integration instructions for skill + MCP registration. |
| Dockerfile | Narrows cargo chef/build to specific crates for faster CI images. |
| Cargo.toml | Expands workspace members and adds shared deps for Tauri/MCP/GUI. |
| CLAUDE.md | Adds extensive repo guidance for Claude Code (commands/architecture/app). |
| .github/workflows/tauri.yml | Adds GitHub Actions build/release workflow for Tauri app artifacts. |
| .github/workflows/release.yml | Uses shared setup action across release jobs. |
| .github/workflows/publish.yml | Uses shared setup action across publish jobs. |
| .github/workflows/publish-mcp-npm.yml | Adds CI to build wavs-mcp binaries and publish npm package. |
| .github/workflows/basic.yml | Refactors CI to use shared setup composite action. |
| .github/actions/setup/action.yml | Adds shared CI setup (deps/toolchain/cache/foundry) for workflows. |
| .claude/skills/wavs/reference/mcp-tools.md | Adds MCP tool reference documentation for skill distribution. |
| .claude/skills/wavs/install.sh | Adds global skill install script via sparse checkout. |
| .claude/skills/wavs/flows/update-service.md | Adds update-service operational flow for Claude skill. |
| .claude/skills/wavs/flows/component-dev.md | Adds component dev flow for Claude skill. |
| .claude/skills/wavs/SKILL.md | Adds WAVS skill entrypoint and setup guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| new Extract({ | ||
| cwd: binDir, | ||
| filter: (p) => path.basename(p) === 'wavs-mcp' || path.basename(p) === 'wavs-mcp.exe', | ||
| strip: 1, |
There was a problem hiding this comment.
strip: 1 will likely break extraction because the CI tarball (as packaged in publish-mcp-npm.yml) appears to contain only the binary at the archive root (e.g. wavs-mcp). Stripping 1 path component can result in an empty output path and a failed/incorrect extract. Fix by removing strip (or setting it to 0), or alternatively change the release tarball to include a top-level directory so strip: 1 is valid.
| strip: 1, |
| tracing::error!("HTTP server thread panicked: {:?}", e); | ||
| } | ||
| if let Err(e) = dispatcher_handle.join() { | ||
| tracing::error!("Dispatcher thread panicked: {:?}", e); |
There was a problem hiding this comment.
If the HTTP server thread panics, the process can continue with the dispatcher still running, and dispatcher_handle.join() may block indefinitely, leaving the node in a broken state (no HTTP server, but still running). Consider propagating the panic (re-panicking after logging) or triggering a coordinated shutdown/exit when either critical thread panics so the process doesn't silently hang in a degraded mode.
| tracing::error!("HTTP server thread panicked: {:?}", e); | |
| } | |
| if let Err(e) = dispatcher_handle.join() { | |
| tracing::error!("Dispatcher thread panicked: {:?}", e); | |
| tracing::error!("HTTP server thread panicked: {:?}", e); | |
| // Exit to avoid running in a degraded state or blocking on the dispatcher join. | |
| std::process::exit(1); | |
| } | |
| if let Err(e) = dispatcher_handle.join() { | |
| tracing::error!("Dispatcher thread panicked: {:?}", e); | |
| // Exit to avoid leaving the process running without a functioning dispatcher. | |
| std::process::exit(1); |
| // Persist to disk so service definitions survive node restarts (atomic write) | ||
| let services_dir = self.config.data.join("dev_services"); | ||
| std::fs::create_dir_all(&services_dir)?; | ||
| let dest = services_dir.join(format!("{service_hash}.json")); | ||
| let tmp = tempfile::NamedTempFile::new_in(&services_dir)?; | ||
| std::fs::write(tmp.path(), serde_json::to_vec(service)?)?; | ||
| tmp.persist(dest)?; |
There was a problem hiding this comment.
This code performs blocking filesystem work (std::fs::*, tempfile persist) on the request path (likely inside an async HTTP handler). That can block the Tokio runtime worker threads under load. Prefer using tokio::fs equivalents where possible, or wrap the whole persistence block in tokio::task::spawn_blocking so it can't stall async execution.
| let prefix = format!("{}/", service_id); | ||
| let entries: Vec<KvEntry> = state | ||
| .db_storage | ||
| .kv_store | ||
| .iter() | ||
| .filter_map(|e| { | ||
| let (full_key, value) = e.pair(); | ||
| let rest = full_key.strip_prefix(&prefix)?; | ||
| let (bucket, key) = rest.split_once('/')?; | ||
| Some(KvEntry { | ||
| bucket: bucket.to_string(), | ||
| key: key.to_string(), | ||
| value_b64: base64::engine::general_purpose::STANDARD.encode(value.as_slice()), | ||
| }) | ||
| }) | ||
| .collect(); | ||
| Json(entries).into_response() |
There was a problem hiding this comment.
This endpoint scans the entire kv_store and filters in-memory by prefix, which is O(N) over all services/buckets/keys and can get expensive as data grows. If the underlying store supports it, prefer a prefix/range iterator (e.g., iterate only keys starting with {service_id}/) or maintain a per-service index to avoid full scans on each request.
There was a problem hiding this comment.
eh, dev-only, it's good enough - fine just leaving CoPilot's notes as a comment
| // Read file bytes into memory and return them | ||
| let bytes = match tokio::fs::read(&target).await { | ||
| Ok(b) => b, | ||
| Err(_) => { | ||
| return (StatusCode::INTERNAL_SERVER_ERROR, "failed to read file").into_response() | ||
| } | ||
| }; | ||
| ( | ||
| StatusCode::OK, | ||
| [(header::CONTENT_TYPE, "application/octet-stream")], | ||
| Body::from(bytes), | ||
| ) | ||
| .into_response() |
There was a problem hiding this comment.
This reads the entire file into memory before responding, which can be a problem for large service files (memory spikes and latency). Consider streaming the file (e.g., using a tokio::fs::File and an Axum body stream) and optionally setting Content-Length when available.
There was a problem hiding this comment.
as long as this endpoint is gated behind dev-only, I don't think Copilot's suggestion to stream is really necessary. It's nice, but if a dev is hitting their own server, meh, this is fine imho
If it's easy to implement, I'd go with what CoPilot suggested though
| "hash": "d53dab46", | ||
| "configHash": "74d151e1", | ||
| "lockfileHash": "7bd08522", | ||
| "browserHash": "aa0c712e", |
There was a problem hiding this comment.
app/.vite/ contains Vite's generated dependency optimization cache and is typically not intended to be committed. This will cause noisy diffs and potential cross-machine issues. Consider removing app/.vite/** from the repo and adding .vite/ to app/.gitignore.
| "hash": "d53dab46", | |
| "configHash": "74d151e1", | |
| "lockfileHash": "7bd08522", | |
| "browserHash": "aa0c712e", | |
| "hash": "static", | |
| "configHash": "static-config", | |
| "lockfileHash": "static-lockfile", | |
| "browserHash": "static-browser", |
dakom
left a comment
There was a problem hiding this comment.
As noted on Slack, I deliberately ignored all Tauri and MCP stuff here, just focused on core WAVS
It mostly looks good, just a few superficial comments, but a couple blockers and questions too
| // Read file bytes into memory and return them | ||
| let bytes = match tokio::fs::read(&target).await { | ||
| Ok(b) => b, | ||
| Err(_) => { | ||
| return (StatusCode::INTERNAL_SERVER_ERROR, "failed to read file").into_response() | ||
| } | ||
| }; | ||
| ( | ||
| StatusCode::OK, | ||
| [(header::CONTENT_TYPE, "application/octet-stream")], | ||
| Body::from(bytes), | ||
| ) | ||
| .into_response() |
There was a problem hiding this comment.
as long as this endpoint is gated behind dev-only, I don't think Copilot's suggestion to stream is really necessary. It's nice, but if a dev is hitting their own server, meh, this is fine imho
If it's easy to implement, I'd go with what CoPilot suggested though
| canonical.starts_with(base).then_some(canonical) | ||
| } | ||
|
|
||
| async fn handle_fs_inner(state: &HttpState, service_id: &str, rel_path: &str) -> impl IntoResponse { |
There was a problem hiding this comment.
ServiceId should be validated with actual ServiceId type
It's not just a niceness, plain strings can sneak into paths even after validation, better to prevent the footgun
| let prefix = format!("{}/", service_id); | ||
| let entries: Vec<KvEntry> = state | ||
| .db_storage | ||
| .kv_store | ||
| .iter() | ||
| .filter_map(|e| { | ||
| let (full_key, value) = e.pair(); | ||
| let rest = full_key.strip_prefix(&prefix)?; | ||
| let (bucket, key) = rest.split_once('/')?; | ||
| Some(KvEntry { | ||
| bucket: bucket.to_string(), | ||
| key: key.to_string(), | ||
| value_b64: base64::engine::general_purpose::STANDARD.encode(value.as_slice()), | ||
| }) | ||
| }) | ||
| .collect(); | ||
| Json(entries).into_response() |
There was a problem hiding this comment.
eh, dev-only, it's good enough - fine just leaving CoPilot's notes as a comment
|
|
||
| let db_storage = dispatcher.db_storage.clone(); | ||
|
|
||
| // Load previously saved service definitions from disk so GET /dev/services/{hash} works after restart |
There was a problem hiding this comment.
This is confusing, but not because of this PR. The whole flow here for "save/load by hash" is only used for e2e tests.
Instead of duplicating the save/restore logic, we should get rid of this flow entirely.
Opened #1133 to track.
In the meantime, I think we should revert the change here - errors that come up this way would mean the direct calls are being used when they shouldn't, i.e. it's a misuse of the API
- Fix typo: DEFAULT_LOG_BUFFER_CAPCITY → DEFAULT_LOG_BUFFER_CAPACITY - Clamp buffer/broadcast capacity to minimum 1 in with_capacity to prevent panic - Validate level query param in HTTP handlers; return 400 for invalid values - Fix SSE duplicate-event race: skip live entries already covered by replay - Use reqwest .query() for URL-safe parameter encoding in MCP client - Match both quoted and unquoted field formats in wavs_query_component_logs filter - Remove hardcoded "10,000" from wavs_query_logs tool description Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Clamp limit to minimum 1 via .clamp(1, MAX_LIMIT) so polling always advances - Add dev_endpoints_enabled note to OpenAPI description so Swagger UI doesn't advertise endpoints that return 404 in production configs - Add SSE integration tests: replay ordering, level filter on replay, invalid level returns 400, and limit=0 clamping Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix OTLP gRPC exporter endpoint: remove /v1/traces suffix when using with_tonic() since gRPC routing is handled internally, not by URL path. Also fixes the same bug in utils/src/telemetry.rs (shared helper). - Fix tool_query_component_logs field filtering: replace raw substring search with boundary-aware matching (start-of-string or ", " prefix) to prevent false positives when the message field contains text that looks like a key=value pair. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ensure the EnvFilter (filters) is added last (.with(filters) outermost) in both the Jaeger and non-Jaeger branches so it gates all inner layers — fmt, telemetry, and InMemoryLogLayer — consistently regardless of whether Jaeger is configured. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…uest - Replace to_lowercase() in level_value() with eq_ignore_ascii_case() to avoid a heap allocation on every per-entry comparison. - Precompute the minimum level value once before the filter iterator in since(), so level_value(min_level) is not recomputed for every entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move ID assignment into push() under the write lock so entries are always stored in id order, preventing out-of-order IDs from concurrent callers (alloc_id() outside the lock). InMemoryLogLayer now passes id: 0 as a placeholder; push() overwrites it atomically. - Replace SSE max-id cutoff with a HashSet<u64> of exactly the replayed entry IDs, so entries that arrived between subscribe() and last_n() but were not captured in the replay snapshot are not incorrectly dropped from the live stream. - Compute poll next_id via entries.iter().map(|e| e.id).max() rather than entries.last() for explicitness and robustness. - Update tests to pass id: 0 (push now assigns the real id). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- next_id falls back to buffer watermark when no entries match a filter, so polling clients always advance even on empty filtered responses. - push() now returns the assigned u64 id; remove alloc_id() which had misleading semantics (push() overwrites any caller-supplied id anyway). - Fix last remaining alloc_id() call in http_logs_zero_limit_clamped test. - Extract build_tracer_provider() from utils::telemetry so main.rs can reuse it without duplicating the OTLP provider construction code. - Gate InMemoryLogLayer behind config.dev_endpoints_enabled using Option<InMemoryLogLayer> (.then(|| ...)); avoids per-event clone and broadcast overhead in production configs where /dev/logs is not served. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…y-jaeger-propagator from wavs/Cargo.toml These three crates were direct dependencies when main.rs contained inline OTLP tracer-provider setup. Since commit 955b873 extracted that logic into utils::telemetry::build_tracer_provider, wavs/src/ no longer imports them directly; they are now used only by packages/utils. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pause/resume -> update service
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Opened a new issue for some follow-up tasks, but not blocking - enabled auto-merge for now |
New Tauri app for WAVS!
Easily: