feat: core CLI completion — all 21 services wired#2
Open
Conversation
TDD: wiremock test verifies ApiKey header injection and JSON decode; replaces the stub implementation that always returned ApiError::Internal.
Replace stub implementation with real HttpClient::get_json call. TDD: radarr_health::system_status_ok test goes red then green.
Wire RadarrClient::health() to call system_status(), implement the ServiceClient trait with proper reachable/auth_ok/version/latency_ms mapping, and handle all RadarrError variants (Api(Auth), Api(_), NotFound).
- Root CLAUDE.md: add api/ to nested guides, fix cargo flags (--features all → --workspace --all-features), add missing just targets (build-release, fmt, clean), and expand the structure tree to include api/, api.rs, catalog.rs, mcp.rs, tui.rs, cli.rs - bin/CLAUDE.md: replace placeholder with actual script table and rules
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds per-service HTTP route groups and MCP dispatch stubs, implements a reqwest-backed HttpClient and many Radarr SDK methods, expands CLI subcommands and serve/stdio dispatch, populates catalog actions from service ACTIONS, and changes ToolError/envelope serialization plus tracing/logging behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as HTTP Client
participant Router as Axum Router
participant Service as Service Handler
participant MCP as MCP Dispatcher
participant SDK as Service SDK (e.g., RadarrClient)
participant Http as HttpClient
participant Backend as Remote API
Client->>Router: POST /v1/radarr { action, params }
Router->>Service: handler(ActionRequest)
Service->>MCP: dispatch(action, params)
MCP->>SDK: client_from_env()
SDK->>Http: get_json("/api/v3/system/status")
Http->>Backend: HTTP GET /api/v3/system/status (+auth)
Backend-->>Http: 200 + JSON
Http->>SDK: parsed JSON
SDK-->>MCP: Result<Value>
MCP-->>Service: dispatch result
Service-->>Router: Json<Value>
Router-->>Client: HTTP 200 Json
sequenceDiagram
participant User as MCP client (stdio)
participant Serve as lab serve (MCP server)
participant Registry as ToolRegistry
participant Dispatch as MCP dispatcher
participant SDK as Service SDK
participant Backend as Remote API
User->>Serve: newline JSON { service:"radarr", action:"movie.list", params:{} }
Serve->>Registry: lookup service handler
Serve->>Dispatch: dispatch(action, params)
Dispatch->>SDK: client_from_env(); SDK->>Backend: HTTP calls
Backend-->>SDK: data
SDK-->>Dispatch: Result<Value>
Dispatch-->>Serve: envelope { ok: data }
Serve-->>User: newline JSON { ok: data }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74dd627d73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
crates/lab/Cargo.toml
Outdated
|
|
||
| [features] | ||
| default = ["radarr", "sonarr", "prowlarr", "plex", "sabnzbd", "qbittorrent"] | ||
| default = ["radarr"] |
There was a problem hiding this comment.
Changing default to only radarr silently drops Sonarr, Prowlarr, Plex, SABnzbd, and qBittorrent from a normal cargo build/release build, which is a breaking regression for existing users who rely on the previous default binary behavior without passing explicit --features. If this reduction is intentional it needs a coordinated breaking-change path, but as-is it removes substantial functionality by default.
Useful? React with 👍 / 👎.
crates/lab/src/cli/serve.rs
Outdated
Comment on lines
+70
to
+73
| let mut reader = BufReader::new(stdin).lines(); | ||
| let mut stdout = tokio::io::stdout(); | ||
|
|
||
| while let Some(line) = reader.next_line().await? { |
There was a problem hiding this comment.
This loop reads newline-delimited JSON via BufRead::lines, but MCP stdio uses JSON-RPC messages framed with Content-Length headers, so standard MCP clients won't be able to communicate with this server and lab serve --transport stdio will fail interoperability. The current protocol shape (service/action/params) is also not MCP JSON-RPC request format.
Useful? React with 👍 / 👎.
crates/lab/src/cli/serve.rs
Outdated
Comment on lines
+92
to
+95
| let body = match result { | ||
| Ok(v) => serde_json::json!({ "data": v }), | ||
| Err(e) => serde_json::json!({ "kind": "internal_error", "message": e.to_string() }), | ||
| }; |
There was a problem hiding this comment.
All dispatch failures are currently rewritten to {"kind":"internal_error"...}, which erases actionable error kinds like unknown_action, missing_param, auth_failed, or rate_limited; clients can no longer distinguish user/input errors from true server faults. This breaks the stable error-envelope contract and makes automated recovery logic unreliable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates operator/developer documentation (CLAUDE guides) but also introduces the first slice of real service wiring: a functional Radarr HTTP client call path, MCP/CLI dispatch scaffolding, and an axum endpoint backed by shared app state.
Changes:
- Update root and
bin/CLAUDE.md docs (repo structure,justtargets, operator script guidance). - Implement
lab-apisHttpClientJSON helpers and Radarrsystem_status+ health plumbing, with integration tests. - Add initial
labruntime wiring for Radarr across registry/catalog, CLI (serve,health,doctor), TUI plugin metadata, and HTTP API routing/state.
Reviewed changes
Copilot reviewed 55 out of 56 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/lab/src/tui/metadata.rs | Start populating TUI plugin rows from compiled-in service metadata (Radarr). |
| crates/lab/src/mcp/services/sonarr.rs | Placeholder MCP dispatch module. |
| crates/lab/src/mcp/services/radarr.rs | New Radarr MCP dispatcher + env-based client construction. |
| crates/lab/src/mcp/services/prowlarr.rs | Placeholder MCP dispatch module. |
| crates/lab/src/mcp/services/plex.rs | Placeholder MCP dispatch module. |
| crates/lab/src/mcp/services/openai.rs | Placeholder MCP dispatch module. |
| crates/lab/src/mcp/services/arcane.rs | Placeholder MCP dispatch module. |
| crates/lab/src/mcp/services.rs | Wire service dispatch modules (extract + feature-gated radarr). |
| crates/lab/src/mcp/registry.rs | Register Radarr in the default tool registry (feature-gated). |
| crates/lab/src/cli/sonarr.rs | Placeholder CLI module. |
| crates/lab/src/cli/serve.rs | Implement basic stdio “serve” loop with registry filtering + dispatch. |
| crates/lab/src/cli/prowlarr.rs | Placeholder CLI module. |
| crates/lab/src/cli/plex.rs | Placeholder CLI module. |
| crates/lab/src/cli/openai.rs | Placeholder CLI module. |
| crates/lab/src/cli/health.rs | Add Radarr health row using the shared ServiceClient trait. |
| crates/lab/src/cli/doctor.rs | Add Radarr env-var checks using plugin metadata required env list. |
| crates/lab/src/cli/arcane.rs | Placeholder CLI module. |
| crates/lab/src/catalog.rs | Populate per-service catalog actions (Radarr) instead of empty list. |
| crates/lab/src/api/state.rs | Store optional Radarr client in shared axum state (from env). |
| crates/lab/src/api/router.rs | Add feature-gated Radarr HTTP route for /v1/radarr/system/status. |
| crates/lab/Cargo.toml | Change default features to only radarr. |
| crates/lab-apis/tests/radarr_health.rs | Wiremock integration test for Radarr system_status request/decoding. |
| crates/lab-apis/tests/http_client.rs | Wiremock integration test for auth header injection + JSON decoding. |
| crates/lab-apis/src/unraid.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/unifi.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/tautulli.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/tailscale.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/sonarr/types.rs | Placeholder Sonarr types module. |
| crates/lab-apis/src/sonarr/error.rs | Placeholder Sonarr error module. |
| crates/lab-apis/src/sonarr/client.rs | Placeholder Sonarr client module. |
| crates/lab-apis/src/sonarr.rs | Add Sonarr plugin metadata (feature compiles). |
| crates/lab-apis/src/sabnzbd.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/radarr/client/system.rs | Implement system_status() via HttpClient::get_json. |
| crates/lab-apis/src/radarr/client.rs | Implement Radarr health() by calling system_status(). |
| crates/lab-apis/src/radarr.rs | Implement ServiceClient for RadarrClient returning ServiceStatus. |
| crates/lab-apis/src/qbittorrent.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/prowlarr/types.rs | Placeholder Prowlarr types module. |
| crates/lab-apis/src/prowlarr/error.rs | Placeholder Prowlarr error module. |
| crates/lab-apis/src/prowlarr/client.rs | Placeholder Prowlarr client module. |
| crates/lab-apis/src/prowlarr.rs | Add Prowlarr plugin metadata (feature compiles). |
| crates/lab-apis/src/plex/types.rs | Placeholder Plex types module. |
| crates/lab-apis/src/plex/error.rs | Placeholder Plex error module. |
| crates/lab-apis/src/plex/client.rs | Placeholder Plex client module. |
| crates/lab-apis/src/plex.rs | Add Plex plugin metadata (feature compiles). |
| crates/lab-apis/src/paperless.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/memos.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/linkding.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/core/http.rs | Implement shared reqwest wrapper with auth injection + JSON helpers. |
| crates/lab-apis/src/bytestash.rs | Replace stub with metadata-only module (feature compiles). |
| crates/lab-apis/src/arcane/types.rs | Placeholder Arcane types module. |
| crates/lab-apis/src/arcane/error.rs | Placeholder Arcane error module. |
| crates/lab-apis/src/arcane/client.rs | Placeholder Arcane client module. |
| crates/lab-apis/src/arcane.rs | Add Arcane plugin metadata (feature compiles). |
| CLAUDE.md | Update repo structure + just command docs (adds api/ references). |
| Cargo.toml | Enable tokio io-std feature for stdio server support. |
| bin/CLAUDE.md | Replace placeholder with operator script table + rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/lab/src/api/router.rs
Outdated
Comment on lines
+49
to
+56
| .map_err(|e| match e { | ||
| lab_apis::radarr::RadarrError::Api(sdk_err) => super::error::ApiError::Sdk(sdk_err), | ||
| lab_apis::radarr::RadarrError::NotFound { kind, id } => { | ||
| super::error::ApiError::UnknownAction(format!("{kind} id {id} not found")) | ||
| } | ||
| })?; | ||
| Ok(Json(serde_json::to_value(status).unwrap_or_default())) | ||
| } |
There was a problem hiding this comment.
radarr_system_status is swallowing serialization failures with unwrap_or_default(), which can silently return {} and will also likely trip the repo’s clippy::unwrap_used lint. Prefer propagating the serde_json::to_value error into an ApiError (e.g. map to ApiError::Sdk(SdkError::Internal(...))) so the handler returns a 5xx instead of an empty payload.
crates/lab/src/api/router.rs
Outdated
Comment on lines
+49
to
+53
| .map_err(|e| match e { | ||
| lab_apis::radarr::RadarrError::Api(sdk_err) => super::error::ApiError::Sdk(sdk_err), | ||
| lab_apis::radarr::RadarrError::NotFound { kind, id } => { | ||
| super::error::ApiError::UnknownAction(format!("{kind} id {id} not found")) | ||
| } |
There was a problem hiding this comment.
Mapping lab_apis::radarr::RadarrError::NotFound { .. } to ApiError::UnknownAction results in a 400 for a resource-missing condition. This should likely map to a 404 (ApiError::Sdk(SdkError::NotFound)), or to a dedicated HTTP-layer error that preserves the {kind,id} context but still returns NOT_FOUND.
crates/lab-apis/src/core/http.rs
Outdated
Comment on lines
19
to
28
| pub fn new(base_url: impl Into<String>, auth: Auth) -> Self { | ||
| let inner = Client::builder() | ||
| .user_agent(concat!("lab-apis/", env!("CARGO_PKG_VERSION"))) | ||
| .build() | ||
| .expect("reqwest::Client::build"); | ||
| Self { | ||
| base_url: base_url.into(), | ||
| auth, | ||
| inner, | ||
| } |
There was a problem hiding this comment.
HttpClient::new uses .expect("reqwest::Client::build"), but the workspace enables clippy::expect_used and CI runs clippy with -D warnings, so this will fail linting. Consider switching to an infallible constructor (reqwest::Client::new() + setting the User-Agent per request), or make the constructor fallible (try_new -> Result<Self, ApiError>), or explicitly #[allow(clippy::expect_used)] with justification.
crates/lab/src/mcp/registry.rs
Outdated
Comment on lines
+67
to
+70
| #[cfg(feature = "radarr")] | ||
| const fn category_slug(cat: lab_apis::core::Category) -> &'static str { | ||
| use lab_apis::core::Category; | ||
| match cat { |
There was a problem hiding this comment.
category_slug is gated behind #[cfg(feature = "radarr")], but it maps the shared lab_apis::core::Category enum and will likely be needed by other services as they’re registered. Making it unconditional now avoids future feature-combo build failures (e.g. enabling sonarr without radarr).
Comment on lines
+25
to
+31
| "help" => Ok(json!({ | ||
| "service": "radarr", | ||
| "actions": [ | ||
| { "name": "system.status", "description": "Return Radarr system status", "destructive": false }, | ||
| { "name": "help", "description": "Show this catalog", "destructive": false }, | ||
| ] | ||
| })), |
There was a problem hiding this comment.
The help action response duplicates the action catalog that also exists in crates/lab/src/catalog.rs. To prevent drift between CLI/MCP/HTTP surfaces, consider sourcing this list from the shared catalog (or another shared action-spec table) rather than duplicating literals here.
Comment on lines
13
to
+16
| - `crates/lab/src/mcp/` — dispatch, envelopes, elicitation, catalog | ||
| - `crates/lab/src/cli/` — thin-shim pattern, destructive flags, batch commands | ||
| - `crates/lab/src/tui/` — plugin manager UX, `.mcp.json` patching | ||
| - `crates/lab/src/api/` — axum HTTP surface, status code mapping, middleware stack |
There was a problem hiding this comment.
PR title/description indicate a docs-only CLAUDE.md audit, but this diff introduces substantial functional changes (Radarr HTTP client + tests, MCP/CLI dispatch wiring, axum routes, default feature changes). Please update the PR title/description (or split into separate PRs) so reviewers can assess scope and risk accurately.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 issues found across 56 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/lab/src/catalog.rs">
<violation number="1" location="crates/lab/src/catalog.rs:51">
P2: Catalog action discovery is hardcoded by service string, creating a drift-prone second source of truth for service actions.</violation>
</file>
<file name="crates/lab/src/cli/doctor.rs">
<violation number="1" location="crates/lab/src/cli/doctor.rs:53">
P2: Environment check marks empty required env vars as valid, producing false-positive doctor results.</violation>
</file>
<file name="crates/lab/src/api/router.rs">
<violation number="1" location="crates/lab/src/api/router.rs:55">
P3: Return the typed `SystemStatus` directly (e.g., `Json(status)`) and let serialization errors surface through `ApiError` instead of defaulting to an empty value.</violation>
</file>
<file name="crates/lab/src/cli/serve.rs">
<violation number="1" location="crates/lab/src/cli/serve.rs:64">
P2: This command now contains MCP server and dispatch business logic in `cli/`, which violates the directory’s thin-shim rule and makes the CLI layer responsible for protocol/runtime behavior.</violation>
<violation number="2" location="crates/lab/src/cli/serve.rs:70">
P1: MCP stdio transport uses `Content-Length`-header framing (same as LSP) with JSON-RPC 2.0 messages, not newline-delimited JSON. Reading via `BufReader::lines()` and using a custom `{"service","action","params"}` envelope means standard MCP clients will fail to communicate with this server. Consider using the `rmcp` crate (already in workspace deps) or implementing the Content-Length framing with proper JSON-RPC request/response shapes.</violation>
<violation number="3" location="crates/lab/src/cli/serve.rs:94">
P2: All dispatch errors are mapped to `"kind": "internal_error"`, which erases actionable error kinds like `unknown_service`, `unknown_action`, or `auth_failed`. Clients cannot distinguish user/input errors from true server faults. Consider propagating a structured error kind from the dispatch layer instead of collapsing everything into one bucket.</violation>
</file>
<file name="crates/lab/src/cli/health.rs">
<violation number="1" location="crates/lab/src/cli/health.rs:33">
P2: `health.rs` now contains service orchestration/business logic (`radarr_row`) instead of remaining a thin CLI shim. Move this logic into `lab-apis` and keep the CLI command to invoking that API + printing.</violation>
</file>
<file name="crates/lab/src/mcp/services.rs">
<violation number="1" location="crates/lab/src/mcp/services.rs:3">
P2: `extract` is added to `services` but never registered in `build_default_registry`, so the MCP tool is unreachable. Either register it in the registry or drop the module include if it’s intentionally inactive.</violation>
</file>
<file name="crates/lab-apis/src/radarr.rs">
<violation number="1" location="crates/lab-apis/src/radarr.rs:104">
P1: This maps all non-auth errors to `unreachable`, which misclassifies HTTP-level failures (e.g., 404/5xx/decode) as connectivity failures.</violation>
</file>
<file name="crates/lab/Cargo.toml">
<violation number="1" location="crates/lab/Cargo.toml:52">
P1: Changing `default` features from six services to only `radarr` silently drops Sonarr, Prowlarr, Plex, SABnzbd, and qBittorrent from a normal `cargo build` / release build. Existing users who rely on the previous default binary without explicit `--features` will lose substantial functionality. If the reduction is intentional it should be called out as a breaking change; otherwise restore the previous default set.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| #[cfg(feature = "radarr")] | ||
| async fn radarr_row() -> HealthRow { |
There was a problem hiding this comment.
P2: health.rs now contains service orchestration/business logic (radarr_row) instead of remaining a thin CLI shim. Move this logic into lab-apis and keep the CLI command to invoking that API + printing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/lab/src/cli/health.rs, line 33:
<comment>`health.rs` now contains service orchestration/business logic (`radarr_row`) instead of remaining a thin CLI shim. Move this logic into `lab-apis` and keep the CLI command to invoking that API + printing.</comment>
<file context>
@@ -10,17 +10,56 @@ use crate::output::{OutputFormat, print};
}
+
+#[cfg(feature = "radarr")]
+async fn radarr_row() -> HealthRow {
+ use lab_apis::core::ServiceClient;
+
</file context>
Batched implementation of docs/superpowers/plans/2026-04-08-clippy-cleanup.md.
- Relax missing_docs and clippy::unused_async workspace-wide with TODOs
- Add readme/keywords/categories metadata to both crate manifests
- Backtick product names for clippy::doc_markdown (core, servarr, radarr, openai)
- http.rs: add # Panics doc, allow startup expect, require Sync on post_json body
- radarr.rs: saturate latency_ms conversion instead of truncating
- traits.rs: return &'static str from ServiceClient::{name,service_type}
- cli/install, cli/completions, cli/plugins: drop Result wrappers, take args by ref
- tests: allow expect_used/unwrap_used at the test-file level
- api/error.rs: merge duplicate match arms, const fn on kind()
- mcp/envelope.rs, extract/types.rs: const fn on trivial getters
- config.rs: collapse nested ifs into let-chains
- extract/transport.rs: drop underscore prefix from used bindings
- servarr/{notification,system}: allow struct_excessive_bools on DTOs
- extract/types.rs: ok_or -> ok_or_else (or_fun_call)
- radarr/types/filesystem: split too-long first doc paragraph
- api/router.rs: drop redundant must_use on build_router
- mcp/meta.rs: drop unnecessary Result wrapper
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 28
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/lab-apis/src/arcane.rs`:
- Around line 6-18: The arcane.rs file currently only exports META but is
missing the required per-service submodule declarations; add module declarations
for client, types, and error by adding mod client; mod types; mod error; (and
optionally pub use client::.../types::.../error::... as appropriate) so the
crate follows the project structure where arcane.rs sits alongside an arcane/
folder containing client.rs, types.rs, and error.rs; ensure the module names
match exactly (client, types, error) so the compiler can find arcane::client,
arcane::types, and arcane::error.
In `@crates/lab-apis/src/arcane/error.rs`:
- Line 1: Add a proper typed error enum for this service using thiserror: create
an ArcaneError enum that derives Debug and thiserror::Error and includes a
transparent wrapper variant for the shared ApiError (e.g., Api(#[from]
#[transparent] ApiError)) so every service error wraps ApiError transparently;
include any service-specific variants you need (with clear thiserror display
messages) and export ArcaneError for use throughout the arcane module.
In `@crates/lab-apis/src/arcane/types.rs`:
- Line 1: Populate this module with the Arcane API request/response models using
serde derives: add structs like ArcaneRequest/ArcaneResponse (or domain-specific
names used elsewhere such as CreateSpellRequest, UpdateSpellRequest,
SpellResponse, PaginatedSpells, ErrorResponse) annotated with
#[derive(Serialize, Deserialize, Debug, Clone)] and any necessary serde
attributes for field renaming/optional fields; ensure all types currently
defined in client.rs are moved here and re-exported for client usage, and update
client.rs to reference these types (e.g., ArcaneRequest, SpellResponse,
ErrorResponse) rather than defining them inline.
In `@crates/lab-apis/src/core/http.rs`:
- Around line 43-50: The url(&self, path: &str) -> String currently allows
absolute URLs and can leak creds; change it to reject absolute URLs and
construct the final URL only from self.base_url and a relative path: update
signature to return Result<String, Error> (or a custom error) and use
url::Url::parse(path) to detect absolute URLs (if parse succeeds and has a
scheme, return Err), otherwise use Url::parse(&self.base_url)?.join(path)? to
build the final URL; reference the url function and self.base_url and use
url::Url::parse and Url::join to implement the checks and safe join.
- Around line 20-23: The shared reqwest client built in Client::builder()
(variable inner) lacks connect/request timeouts; update the builder chain in the
HttpClient creation to set sensible defaults (e.g.,
.connect_timeout(Duration::from_secs(10)) and .timeout(Duration::from_secs(30)))
and add the needed import for std::time::Duration so outbound HTTP calls cannot
hang indefinitely.
In `@crates/lab-apis/src/linkding.rs`:
- Around line 6-18: The file declares META for the linkding plugin but is
missing the per-service module declarations; add module declarations for the
service by adding pub mod client, pub mod types, and pub mod error in
linkding.rs (near the META const) so the compiler and discovery conventions pick
up the service modules, and ensure a linkding/ directory exists with client.rs,
types.rs, and error.rs implementing the service API types and client logic
referenced by the crate.
In `@crates/lab-apis/src/memos.rs`:
- Around line 6-18: The memos service entry-point (the PluginMeta constant META)
is missing its submodule declarations; add `pub mod client;`, `pub mod types;`,
and `pub mod error;` to this file so the module layout for the memos service is
complete and aligns with the expected per-service structure (so the `memos`
module exposes the client, types, and error submodules referenced by the
service).
In `@crates/lab-apis/src/paperless.rs`:
- Around line 6-18: The file defines the PluginMeta constant META for the
paperless service but is missing the required per-service submodule
declarations; add module declarations for the service's client, types, and error
modules (e.g., declare mod client; mod types; mod error;) in the paperless.rs so
the compiler can find paperless/{client.rs, types.rs, error.rs} and their
symbols referenced by META or other crate code; ensure the declarations appear
alongside the META constant and follow the crate's per-service module pattern.
In `@crates/lab-apis/src/plex/client.rs`:
- Line 1: The Plex client module currently contains only a placeholder and must
implement the service contract: implement the ServiceClient trait for the Plex
struct (provide the required health_check method and any associated trait
items), add a constructor (e.g., Plex::new or Plex::from_config) that
initializes internal http client/config, and expose business methods declared in
the spec such as get_library, search_media, and play_media (or whichever
business methods the service requires) as impl Plex methods; ensure the impl
block for Plex and the ServiceClient impl are added to client.rs and that Plex
is publicly exported so callers can instantiate and use it for health checks and
business flows.
In `@crates/lab-apis/src/plex/error.rs`:
- Line 1: Add a typed Plex service error in error.rs by defining a public
PlexError enum that derives thiserror::Error and Debug and includes a
transparent wrapper variant for the shared ApiError (use #[error(transparent)]
and #[from] on the variant that holds ApiError) so all Plex service errors wrap
ApiError without panicking; ensure the module exports PlexError and use it as
the error type in service Result<T, PlexError> signatures per the repository
convention.
In `@crates/lab-apis/src/plex/types.rs`:
- Line 1: types.rs is currently an empty placeholder; you must define the Plex
API request/response models here (instead of in client.rs). Create
strongly-typed structs/enums for each API payload used by the client (e.g.,
PlexAuthRequest, PlexAuthResponse, PlexMediaItem, PlexError) and derive serde
Serialize/Deserialize plus Debug/Clone/PartialEq as appropriate, add any
necessary serde field attributes (rename, default, skip_serializing_if) to match
JSON, and re-export them (pub) so client.rs imports the types from the types
module rather than declaring them inline. Also remove or move any type
definitions currently in client.rs into types.rs and update client.rs imports to
use crate::plex::types::{...}.
In `@crates/lab-apis/src/prowlarr/client.rs`:
- Line 1: Add a minimal ProwlarrClient scaffold that implements the
ServiceClient trait so the module exposes a testable service client surface:
create a struct ProwlarrClient and implement ServiceClient for it (including a
health_check async method returning Result<...> per the trait), and expose any
basic constructor/new method (e.g., ProwlarrClient::new) and a placeholder
business method (e.g., get_indexers or ping) in client.rs; alternatively, if you
don't want to provide a scaffold, gate or remove the module by deleting the
empty file or adding cfg(feature = "prowlarr") around it so it doesn't violate
the project guideline that all service client modules must implement
ServiceClient.
In `@crates/lab-apis/src/prowlarr/error.rs`:
- Line 1: Create a real ProwlarrError type using thiserror: define pub enum
ProwlarrError with #[derive(Debug, thiserror::Error)] and include a transparent
wrapper variant for the shared ApiError (e.g., #[error(transparent)] Api(#[from]
ApiError)), plus any service-specific variants you need (e.g., Http(#[from]
reqwest::Error), Parse(#[from] serde_json::Error), NotFound(String)) so the
module implements the required typed errors and transparently wraps ApiError per
the project guideline.
In `@crates/lab-apis/src/radarr.rs`:
- Around line 96-106: The current Err(RadarrError::Api(e)) arm treats all API
errors as host-down by returning ServiceStatus::unreachable; instead return a
ServiceStatus that marks the service as reachable (reachable: true), set auth_ok
appropriately (keep the Auth branch handling for
Err(RadarrError::Api(ApiError::Auth)) which sets auth_ok: false; for other Api
errors set auth_ok: true), include the error text in message (e.to_string()),
keep version None and preserve latency_ms from start.elapsed(), and remove use
of ServiceStatus::unreachable for non-auth API errors so HTTP errors like
NotFound/RateLimited are not misclassified.
In `@crates/lab-apis/src/sabnzbd.rs`:
- Around line 6-18: The file exposes only the META constant but must also
declare the service submodules; add module declarations for the SABnzbd service
entry-point by adding pub mod client;, pub mod types;, and pub mod error; to
crates/lab-apis/src/sabnzbd.rs (keeping the existing META constant intact) so
the service follows the repository layout with sabnzbd/client.rs,
sabnzbd/types.rs, and sabnzbd/error.rs; ensure the module names match exactly
(client, types, error) so consumers can import the client and types alongside
META.
In `@crates/lab-apis/src/sonarr/client.rs`:
- Line 1: sonarr/client.rs currently contains only a doc stub and must define a
concrete SonarrClient type and implement the required ServiceClient trait plus
business methods. Add a SonarrClient struct (including config fields like
base_url and an HTTP client), implement ServiceClient for SonarrClient with the
required health_check (or check_health) method that performs an HTTP health
probe and returns the expected result type, and expose business methods such as
new (constructor), get_series, add_series and remove_series (or other
Sonarr-specific RPCs the service needs) that encapsulate request/response
handling and surface crate error types; make sure method names match the trait
and calling code (ServiceClient::health_check) and reuse a shared reqwest client
for requests. Ensure all public signatures are in client.rs so consumers can
perform health checks and business operations.
In `@crates/lab-apis/src/sonarr/error.rs`:
- Line 1: Create a typed Sonarr error enum using thiserror: add pub enum
SonarrError with #[derive(thiserror::Error, Debug)] and include a transparent
wrapper variant like #[error(transparent)] Api(#[from] crate::ApiError) to wrap
the shared ApiError, plus any service-specific variants (e.g., NotFound,
Validation(#[from] SomeValidationError) or Io(#[from] std::io::Error)) as
needed; also add a pub type Result<T> = std::result::Result<T, SonarrError> and
ensure no panics are introduced—return Result<T> everywhere.
In `@crates/lab/src/api/router.rs`:
- Around line 43-53: The current handler maps a missing Radarr client and
RadarrError::NotFound into client-side 400s; instead, change the mapping so
state.radarr() absence returns a server-side availability/configuration error
(e.g., ApiError::ServiceUnavailable or ApiError::Internal) rather than
UnknownInstance("radarr"), and map lab_apis::radarr::RadarrError::NotFound {..}
to the API's not-found variant (e.g., ApiError::NotFound) instead of
UnknownAction; keep the SDK passthrough for RadarrError::Api(sdk_err) to
ApiError::Sdk(sdk_err). Locate the logic around state.radarr(), system_status(),
and the match on lab_apis::radarr::RadarrError to apply these replacements.
In `@crates/lab/src/catalog.rs`:
- Around line 51-74: The actions_for function currently only returns hardcoded
entries for "radarr", causing the always-on "extract" service to have an empty
actions list; update actions_for to recognize "extract" and return the real
action entries by reusing the existing ACTIONS constant from
crates::lab::mcp::services::extract (or by mapping ACTIONS into
Vec<ActionEntry>), so Catalog { services } is populated with the extract
actions; reference the actions_for function, the ACTIONS constant in the extract
module, the ActionEntry struct, and the Catalog services population when making
the change.
In `@crates/lab/src/cli/doctor.rs`:
- Around line 49-65: The check using std::env::var(...).is_ok() incorrectly
treats empty values as present; update the present detection in the radarr block
(around lab_apis::radarr::META and meta.required_env) to ensure the env var is
non-empty and not just set: fetch the value and consider it present only if
value.trim().is_empty() is false (or preferably call the shared runtime env
validator used by bootstrap if one exists) so Finding entries use Severity::Ok
only for non-blank values and Severity::Fail for missing/empty/whitespace-only
values.
In `@crates/lab/src/cli/openai.rs`:
- Line 1: The file currently lacks the CLI shim: add code that destructures the
clap CLI types (e.g., OpenAiArgs and OpenAiSubcommand) for the openai
subcommand, construct/obtain the lab-apis client
(lab_apis::openai::OpenAiClient) and call the appropriate client method (e.g.,
OpenAiClient::call or the specific method for the requested subcommand) with the
extracted args, then format and print the result (use the existing
format_output/print_json helper used by other CLI shims). Ensure the module
exports a thin runner function (e.g., run_openai_command or handle_openai) that
only maps clap data -> lab-apis client call -> formatted output.
In `@crates/lab/src/cli/serve.rs`:
- Around line 91-95: The stdio loop currently collapses every dispatch failure
in dispatch(®istry, service, action, params).await into a generic
internal_error JSON; instead inspect the Err(e) returned from dispatch (or from
the Result named result) and serialize its structured error information
(kind/code/message) into body so service-specific errors like unknown_action,
unknown_instance, rate_limited are preserved; locate the match that builds body
(the Ok(v) / Err(e) arms around body) and replace the Err arm with logic that
either pattern-matches on the dispatched error type or calls a serialization
method on the error to produce serde_json with its real kind and message,
ensuring existing consumers receive the original error kind rather than always
"internal_error".
- Around line 51-61: filter_registry currently silently drops unknown or
misspelled service names; change it to validate the provided services list
against the available registry.services() names and fail fast if any requested
names are not found. Specifically, in filter_registry (and its callers) either
(A) return a Result<ToolRegistry, Error> instead of ToolRegistry and return Err
listing the unknown service names when services contains entries not present in
registry.services().map(|e| e.name), or (B) if you prefer a simpler change,
detect unknown names and immediately exit with a clear error message (e.g., via
panic! or process::exit with an explanatory string like "Unknown service(s):
..."). Ensure you reference the registry.services() entries and entry.name when
computing the difference so typos/disabled services surface as an error rather
than being silently dropped.
In `@crates/lab/src/cli/sonarr.rs`:
- Line 1: Replace the stub in sonarr.rs with a thin shim that destructures the
clap args struct (e.g., SonarrArgs or SonarrCommand) passed to the CLI,
constructs or obtains the Sonarr API client (e.g.,
lab_apis::sonarr::SonarrClient or client from context), calls the appropriate
client method (e.g., SonarrClient::list_series / get_series / whatever matches
the subcommand name), awaits the result, handles errors by mapping them to
stderr logging, and formats the successful response to stdout (JSON or
human-readable per other CLI modules) so the module matches the pattern used by
other files in crates/lab/src/cli (use the same helper functions/utilities for
printing and error handling).
In `@crates/lab/src/mcp/registry.rs`:
- Around line 51-64: build_default_registry() never registers the always-on
"extract" tool so it cannot be discovered; add a RegisteredService entry for the
extract service before returning reg. Locate build_default_registry and call
reg.register(...) with the extract service metadata (preferably using
lab_apis::extract::META if available, otherwise create a RegisteredService with
name "extract", a concise description and category via
category_slug(<category>)); keep the existing feature-gated blocks unchanged.
In `@crates/lab/src/mcp/services/plex.rs`:
- Line 1: The plex module currently is a placeholder and must implement MCP
dispatch and JSON error-envelope handling before being exposed: add a dispatch
entry point (e.g., dispatch_plex_mcp or handle_plex_request) that validates
incoming MCP actions and parameters and returns a Result<McpResponse,
PlexMcpError>; create a PlexMcpError enum/struct (e.g., PlexMcpError with
variants unknown_action, missing_param, unknown_instance, rate_limited,
internal) that serializes to the stable JSON envelope { "kind": "<variant>",
"message": "...", "details": ... } and map all internal errors to these
variants; ensure any public function (dispatch_plex_mcp / handle_plex_request)
converts errors into that JSON envelope so callers always receive the MCP-stable
failure shape; add tests exercising each error kind to validate the envelope
format.
In `@crates/lab/src/mcp/services/radarr.rs`:
- Around line 23-40: Replace the opaque anyhow errors in dispatch with MCP JSON
error envelopes: instead of ok_or_else(...anyhow!) or anyhow::bail! return an
Ok(serde_json::json!(...)) containing a stable "error" object with a "kind" (use
"unknown_instance" or "missing_instance" for client_from_env failures and
"unknown_action" for the unknown=> branch) and a "message" string; update the
client_from_env branch (the error from client_from_env() used in the
"system.status" arm) and the unknown arm (the unknown => anyhow::bail! branch)
to produce these typed envelopes so callers get stable error kinds rather than
free-form strings.
- Around line 11-17: client_from_env currently treats empty RADARR_URL or
RADARR_API_KEY as valid because ok()? accepts empty strings; update
client_from_env to treat blank/whitespace values as unconfigured by reading
RADARR_URL and RADARR_API_KEY, trimming them and returning None if either is
empty/whitespace before calling RadarrClient::new; specifically guard the values
obtained from std::env::var(...) (for RADARR_URL and RADARR_API_KEY) with a
trim().is_empty() check and only construct RadarrClient::new(..., Auth::ApiKey {
header: "X-Api-Key".into(), key }) when both are non-empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1aa914ae-cc92-4892-8cef-78fc08c5ce2a
📒 Files selected for processing (56)
CLAUDE.mdCargo.tomlbin/CLAUDE.mdcrates/lab-apis/src/arcane.rscrates/lab-apis/src/arcane/client.rscrates/lab-apis/src/arcane/error.rscrates/lab-apis/src/arcane/types.rscrates/lab-apis/src/bytestash.rscrates/lab-apis/src/core/http.rscrates/lab-apis/src/linkding.rscrates/lab-apis/src/memos.rscrates/lab-apis/src/paperless.rscrates/lab-apis/src/plex.rscrates/lab-apis/src/plex/client.rscrates/lab-apis/src/plex/error.rscrates/lab-apis/src/plex/types.rscrates/lab-apis/src/prowlarr.rscrates/lab-apis/src/prowlarr/client.rscrates/lab-apis/src/prowlarr/error.rscrates/lab-apis/src/prowlarr/types.rscrates/lab-apis/src/qbittorrent.rscrates/lab-apis/src/radarr.rscrates/lab-apis/src/radarr/client.rscrates/lab-apis/src/radarr/client/system.rscrates/lab-apis/src/sabnzbd.rscrates/lab-apis/src/sonarr.rscrates/lab-apis/src/sonarr/client.rscrates/lab-apis/src/sonarr/error.rscrates/lab-apis/src/sonarr/types.rscrates/lab-apis/src/tailscale.rscrates/lab-apis/src/tautulli.rscrates/lab-apis/src/unifi.rscrates/lab-apis/src/unraid.rscrates/lab-apis/tests/http_client.rscrates/lab-apis/tests/radarr_health.rscrates/lab/Cargo.tomlcrates/lab/src/api/router.rscrates/lab/src/api/state.rscrates/lab/src/catalog.rscrates/lab/src/cli/arcane.rscrates/lab/src/cli/doctor.rscrates/lab/src/cli/health.rscrates/lab/src/cli/openai.rscrates/lab/src/cli/plex.rscrates/lab/src/cli/prowlarr.rscrates/lab/src/cli/serve.rscrates/lab/src/cli/sonarr.rscrates/lab/src/mcp/registry.rscrates/lab/src/mcp/services.rscrates/lab/src/mcp/services/arcane.rscrates/lab/src/mcp/services/openai.rscrates/lab/src/mcp/services/plex.rscrates/lab/src/mcp/services/prowlarr.rscrates/lab/src/mcp/services/radarr.rscrates/lab/src/mcp/services/sonarr.rscrates/lab/src/tui/metadata.rs
crates/lab-apis/src/arcane/error.rs
Outdated
| @@ -0,0 +1 @@ | |||
| //! Not yet implemented. | |||
There was a problem hiding this comment.
Define the Arcane typed error instead of leaving error.rs empty.
With only a placeholder on Line 1, this service has no error contract and cannot transparently wrap shared API failures.
As per coding guidelines, crates/lab-apis/src/*/error.rs: “Use thiserror for typed errors per service in lab-apis. Every service error must wrap ApiError transparently.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/lab-apis/src/arcane/error.rs` at line 1, Add a proper typed error enum
for this service using thiserror: create an ArcaneError enum that derives Debug
and thiserror::Error and includes a transparent wrapper variant for the shared
ApiError (e.g., Api(#[from] #[transparent] ApiError)) so every service error
wraps ApiError transparently; include any service-specific variants you need
(with clear thiserror display messages) and export ArcaneError for use
throughout the arcane module.
crates/lab-apis/src/arcane/types.rs
Outdated
| @@ -0,0 +1 @@ | |||
| //! Not yet implemented. | |||
There was a problem hiding this comment.
Populate types.rs with Arcane request/response models.
Line 1 keeps this module as a placeholder, so the service lacks the required typed API surface.
As per coding guidelines, crates/lab-apis/src/*/types.rs: “Request and response types must be defined in types.rs with serde derive macros. Never put type definitions in client.rs.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/lab-apis/src/arcane/types.rs` at line 1, Populate this module with the
Arcane API request/response models using serde derives: add structs like
ArcaneRequest/ArcaneResponse (or domain-specific names used elsewhere such as
CreateSpellRequest, UpdateSpellRequest, SpellResponse, PaginatedSpells,
ErrorResponse) annotated with #[derive(Serialize, Deserialize, Debug, Clone)]
and any necessary serde attributes for field renaming/optional fields; ensure
all types currently defined in client.rs are moved here and re-exported for
client usage, and update client.rs to reference these types (e.g.,
ArcaneRequest, SpellResponse, ErrorResponse) rather than defining them inline.
crates/lab-apis/src/core/http.rs
Outdated
Comment on lines
+20
to
+23
| let inner = Client::builder() | ||
| .user_agent(concat!("lab-apis/", env!("CARGO_PKG_VERSION"))) | ||
| .build() | ||
| .expect("reqwest::Client::build"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current timeout coverage for reqwest client builders.
# Expected: shared HttpClient builder should include both connect_timeout and timeout.
set -euo pipefail
rg -n --type=rust 'Client::builder\(|\.connect_timeout\(|\.timeout\(' crates/lab-apis/srcRepository: jmagar/lab
Length of output: 127
🏁 Script executed:
cat -n crates/lab-apis/src/core/http.rs | head -40Repository: jmagar/lab
Length of output: 1400
🏁 Script executed:
rg -n 'timeout|Timeout' crates/lab-apis/src --type=rust -iRepository: jmagar/lab
Length of output: 170
🏁 Script executed:
rg -n 'middleware|Middleware' crates/lab-apis/src --type=rustRepository: jmagar/lab
Length of output: 36
🏁 Script executed:
head -10 crates/lab-apis/src/core/http.rsRepository: jmagar/lab
Length of output: 388
Add default timeouts for outbound HTTP calls.
Line 20–23 constructs the shared client without request/connect timeouts. The comment in status.rs explicitly references HttpClient defaults, confirming timeouts are expected. A slow/hung upstream can block calls indefinitely and degrade service health.
Proposed fix
+use std::time::Duration;
use reqwest::{Client, RequestBuilder};
@@
let inner = Client::builder()
+ .connect_timeout(Duration::from_secs(5))
+ .timeout(Duration::from_secs(30))
.user_agent(concat!("lab-apis/", env!("CARGO_PKG_VERSION")))
.build()
.expect("reqwest::Client::build");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let inner = Client::builder() | |
| .user_agent(concat!("lab-apis/", env!("CARGO_PKG_VERSION"))) | |
| .build() | |
| .expect("reqwest::Client::build"); | |
| use std::time::Duration; | |
| use reqwest::{Client, RequestBuilder}; | |
| let inner = Client::builder() | |
| .connect_timeout(Duration::from_secs(5)) | |
| .timeout(Duration::from_secs(30)) | |
| .user_agent(concat!("lab-apis/", env!("CARGO_PKG_VERSION"))) | |
| .build() | |
| .expect("reqwest::Client::build"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/lab-apis/src/core/http.rs` around lines 20 - 23, The shared reqwest
client built in Client::builder() (variable inner) lacks connect/request
timeouts; update the builder chain in the HttpClient creation to set sensible
defaults (e.g., .connect_timeout(Duration::from_secs(10)) and
.timeout(Duration::from_secs(30))) and add the needed import for
std::time::Duration so outbound HTTP calls cannot hang indefinitely.
crates/lab/src/cli/sonarr.rs
Outdated
| @@ -0,0 +1 @@ | |||
| //! Not yet implemented. | |||
There was a problem hiding this comment.
Sonarr CLI module is still a stub and misses shim behavior (Line 1).
This needs argument destructuring + client call + output formatting to match CLI module conventions.
As per coding guidelines, “CLI subcommands in crates/lab/src/cli/<service>.rs are thin shims that destructure clap arguments and call the corresponding client method from lab-apis, then format output.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/lab/src/cli/sonarr.rs` at line 1, Replace the stub in sonarr.rs with a
thin shim that destructures the clap args struct (e.g., SonarrArgs or
SonarrCommand) passed to the CLI, constructs or obtains the Sonarr API client
(e.g., lab_apis::sonarr::SonarrClient or client from context), calls the
appropriate client method (e.g., SonarrClient::list_series / get_series /
whatever matches the subcommand name), awaits the result, handles errors by
mapping them to stderr logging, and formats the successful response to stdout
(JSON or human-readable per other CLI modules) so the module matches the pattern
used by other files in crates/lab/src/cli (use the same helper
functions/utilities for printing and error handling).
crates/lab/src/mcp/services/plex.rs
Outdated
| @@ -0,0 +1 @@ | |||
| //! Not yet implemented. | |||
There was a problem hiding this comment.
Add Plex MCP dispatch/error envelope handling before exposing this module (Line 1).
A placeholder module provides no stable JSON failure kinds for tool errors, which breaks MCP error-contract expectations.
As per coding guidelines, “All MCP tool failures must return JSON envelopes with a stable kind tag (unknown_action, missing_param, unknown_instance, rate_limited, etc.) so agents can react programmatically.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/lab/src/mcp/services/plex.rs` at line 1, The plex module currently is
a placeholder and must implement MCP dispatch and JSON error-envelope handling
before being exposed: add a dispatch entry point (e.g., dispatch_plex_mcp or
handle_plex_request) that validates incoming MCP actions and parameters and
returns a Result<McpResponse, PlexMcpError>; create a PlexMcpError enum/struct
(e.g., PlexMcpError with variants unknown_action, missing_param,
unknown_instance, rate_limited, internal) that serializes to the stable JSON
envelope { "kind": "<variant>", "message": "...", "details": ... } and map all
internal errors to these variants; ensure any public function (dispatch_plex_mcp
/ handle_plex_request) converts errors into that JSON envelope so callers always
receive the MCP-stable failure shape; add tests exercising each error kind to
validate the envelope format.
Comment on lines
+11
to
+17
| pub fn client_from_env() -> Option<RadarrClient> { | ||
| let url = std::env::var("RADARR_URL").ok()?; | ||
| let key = std::env::var("RADARR_API_KEY").ok()?; | ||
| Some(RadarrClient::new( | ||
| &url, | ||
| Auth::ApiKey { header: "X-Api-Key".into(), key }, | ||
| )) |
There was a problem hiding this comment.
Treat empty RADARR_* values as unconfigured.
ok()? accepts empty strings, so this builds a Radarr client that can only fail later. Reject blank/whitespace values here so serve, health, doctor, and AppState all agree on when Radarr is actually configured.
🔧 Minimal fix
pub fn client_from_env() -> Option<RadarrClient> {
- let url = std::env::var("RADARR_URL").ok()?;
- let key = std::env::var("RADARR_API_KEY").ok()?;
+ let url = std::env::var("RADARR_URL")
+ .ok()
+ .filter(|v| !v.trim().is_empty())?;
+ let key = std::env::var("RADARR_API_KEY")
+ .ok()
+ .filter(|v| !v.trim().is_empty())?;
Some(RadarrClient::new(
&url,
Auth::ApiKey { header: "X-Api-Key".into(), key },
))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/lab/src/mcp/services/radarr.rs` around lines 11 - 17, client_from_env
currently treats empty RADARR_URL or RADARR_API_KEY as valid because ok()?
accepts empty strings; update client_from_env to treat blank/whitespace values
as unconfigured by reading RADARR_URL and RADARR_API_KEY, trimming them and
returning None if either is empty/whitespace before calling RadarrClient::new;
specifically guard the values obtained from std::env::var(...) (for RADARR_URL
and RADARR_API_KEY) with a trim().is_empty() check and only construct
RadarrClient::new(..., Auth::ApiKey { header: "X-Api-Key".into(), key }) when
both are non-empty.
…drop-inside-async panic
RegisteredService now carries a `dispatch: DispatchFn` field. Defines `DispatchFn` type alias and `dispatch_fn!` macro to bridge async fn dispatch signatures into owned-String fn pointers stored in the registry. All 22 service registrations wired with their dispatch functions. Adds round-trip test confirming extract help dispatch works through the registry.
The derived Deserialize with #[serde(tag = "kind")] expected
{"kind":"sdk","sdk_kind":"auth_failed"} but Serialize (hand-written)
produces {"kind":"auth_failed","message":"..."}. Since Deserialize is
unused in production, remove it and the serde attributes to eliminate
the wire-format disagreement. Add snapshot test verifying kind() matches
serialized kind for every variant.
…ce/service/action to child spans
…http error mapping
…spatch, correct error ordering
ApiError in api/error.rs serialized a bare {kind, message} envelope,
dropping structured fields (valid, hint, param) that ToolError preserves.
Since all HTTP handlers already use ToolError, remove the dead ApiError
type and re-export ToolError from the api error module. MCP and HTTP
surfaces now share the exact same error serialization path.
…t/compression/cors
Add serde(deny_unknown_fields) to AuthCredentials, SnippetWriteRequest, and ShareCreateRequest — callers can no longer inject undeclared fields into upstream API requests. Change snippets.update to deserialize into typed SnippetWriteRequest instead of forwarding a free Value via body_from_params. Add upfront unknown-action validation before client construction so callers get a clear "unknown_action" error rather than "not configured".
…s/error.rs Move all From<XError> for ToolError impls from mcp/services/ and services/bytestash.rs into services/error.rs with feature gates. Both MCP and HTTP surfaces now share a single conversion path. Document the pattern in docs/ERRORS.md so new services follow it consistently.
AppState now holds Arc<ToolRegistry> alongside the catalog. A generic
POST /v1/{service} handler looks up the service in the registry and
calls its DispatchFn through handle_action (unknown-action gate,
destructive confirmation, timing, structured logging). This ensures
MCP and HTTP surfaces use the same dispatch functions.
All HTTP service handlers previously called mcp::dispatch directly, bypassing the unknown-action gate and destructive confirmation gate. Migrate all 21 handlers to use handle_action which provides: - unknown-action rejection (fail-closed) - destructive confirmation gate (params["confirm"] == true) - confirm key stripping before dispatch - timed dispatch with structured logging
Extract service routes into a sub-router nested under /v1 instead of flat routes with /v1 prefix. Health/ready probes stay at root. This makes it trivial to add /v2 routes later without touching existing service handlers.
print() was ignoring the format parameter and always emitting pretty-printed JSON. Now Human emits pretty-printed JSON (multi-line) and Json emits compact single-line JSON. Extract render() helper for testability. Add 4 tests verifying format differentiation.
Extract home_dir() helper that checks HOME (Unix) then USERPROFILE (Windows). No new dependency needed. Both dotenv_path and toml_path now use it instead of hardcoding HOME.
The old test re-implemented the filter predicate inline and tested Option::filter, not client_from_env. Extract client_from_vars(url, token) so the real logic is testable without env mutation. Add 4 tests covering empty url, empty token, missing values, and valid inputs.
…fields Add payload ParamSpec to all bytestash actions that use body_from_params so MCP callers discover the alternative JSON body input path. Add missing body field params (title, description, language, fragments, categories) to snippets.update. Also fix spawn_health! macro to accept any Ok type (not just ()) so services returning ServiceStatus compile.
Tighten language from "should" to "must" throughout ERRORS.md to match the prescriptive tone of the error contract. The From<ServiceError> for ToolError placement convention was already documented in the prior commit; this finalizes the wording.
On every PR to main, Claude reviews the diff and posts a comment listing which docs/ and CLAUDE.md files may be stale. Concurrency group cancels stale runs per PR.
…in CICD.md Add rows to the CI Checks table and a new Claude-Powered PR Checks section documenting both workflows, the ANTHROPIC_API_KEY requirement, and concurrency group behavior.
- Add doc-freshness workflow: detects stale docs from PR diff, posts sticky PR comment; pre-computes diff/find to avoid prompt injection - Add code-conventions workflow: checks changed lines against locked rules; no Bash tool, debounced, failure annotation on API errors - Update .github/CLAUDE.md with new workflow rows and secret note - Broad branch work: sabnzbd client/types, radarr/unifi MCP dispatch refactors, TUI metadata/marketplace, CLI params/serve, docs refresh Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the core CLI infrastructure so adding a new service only requires implementing it — no core file changes needed.
category_slugmade always-available (was radarr-gated)ActionSpec/ParamSpecstand-ins to reallab_apis::core::actiontypespub const ACTIONSto radarr dispatcher; drive catalog viaconvert_actions()helpercli.rsdoctorandhealthnow iterate all services genericallyPOST /v1/<service>action dispatch for all 21 services;AppStatesimplified to unit structWhat's left (per-service work only)
For each service, implementing it means:
lab-apis/src/<s>/client.rs— implement client +pub const ACTIONSlab/src/cli/<s>.rs— replace stub with thin shim (~20 lines)lab/src/mcp/services/<s>.rs— replace stub with action dispatchlab/src/api/services/<s>.rs— already delegates to MCP dispatch (no change needed)Test plan
cargo test --workspace --all-features— 8 tests passcargo check --workspace --all-features— zero errorslab helplists all 21 servicesecho '{"service":"extract","action":"help","params":{}}' | lab serve --transport stdioreturns extract catalogSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Refactor