Skip to content

Commit e25f03e

Browse files
committed
Update documentation for AzureDevOpsApi trait convention
Add trait convention to CLAUDE.md, code-reviewer, and plan-reviewer. Update PROJECT.md tool function signature and ARCHITECTURE.md with api_trait.rs and tests/ directory listing. Update plan checkmarks.
1 parent 839f1de commit e25f03e

File tree

6 files changed

+44
-24
lines changed

6 files changed

+44
-24
lines changed

.claude/agents/code-reviewer.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ You MUST verify ALL of the following in changed code:
8686
- **Async safety**: No blocking calls in async context. `tokio::task::spawn_blocking` for blocking operations. No fire-and-forget spawns without shutdown path. Flag violations as CRITICAL.
8787
- **Shared state**: `Arc<T>` for shared immutable state across tasks. `Arc<Mutex<T>>` or `tokio::sync::Mutex<T>` for shared mutable state. Flag unprotected shared mutable state as CRITICAL.
8888
- **MCP tool pattern**: Tools use `#[mcp_tool(name, description)]`, args struct with `Deserialize + JsonSchema`, return `Result<CallToolResult, McpError>`, convert domain errors via `.map_err()`. Flag deviations.
89+
- **AzureDevOpsApi trait**: Tool functions MUST accept `&(dyn AzureDevOpsApi + Send + Sync)`, not `&AzureDevOpsClient`. API calls MUST go through trait methods, not standalone functions. Flag deviations.
8990
- **Logging**: Use `log` crate. Correct log levels (trace/debug/info/warn/error). Never log secrets or tokens. Flag violations.
9091
- **CLI conventions**: All config via clap CLI flags. No env vars for config (except `RUST_LOG` for logging). Flag deviations.
9192

.claude/agents/plan-reviewer.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ You MUST verify ALL of the following for EVERY action's planned code:
105105
- **Async safety**: No blocking calls in async context. `tokio::task::spawn_blocking` for blocking operations. No fire-and-forget spawns without shutdown path. Flag violations as CRITICAL.
106106
- **Shared state**: `Arc<T>` for shared immutable state. `Arc<Mutex<T>>` for shared mutable state. Flag unprotected shared mutable state as CRITICAL.
107107
- **MCP tool pattern**: Tools use `#[mcp_tool(name, description)]`, args struct with `Deserialize + JsonSchema`, return `Result<CallToolResult, McpError>`, convert domain errors via `.map_err()`. Flag deviations.
108+
- **AzureDevOpsApi trait**: Planned tool functions MUST accept `&(dyn AzureDevOpsApi + Send + Sync)`, not `&AzureDevOpsClient`. API calls MUST go through trait methods. Flag deviations.
108109
- **Logging**: Use `log` crate. Correct log levels (trace/debug/info/warn/error). Never log secrets. Flag violations.
109110
- **CLI conventions**: All config via clap CLI flags. No env vars for config (except `RUST_LOG`). Flag deviations.
110111

CLAUDE.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,15 @@ This project uses specialized subagents (defined in `.claude/agents/`) to enforc
246246
- Use `mockall` (in dev-dependencies) for generating mock implementations.
247247
- Keep traits small (1–3 methods). Prefer composing small traits over large ones.
248248

249+
### AzureDevOpsApi trait
250+
- All MCP tool functions accept `&(dyn AzureDevOpsApi + Send + Sync)` instead of `&AzureDevOpsClient`.
251+
- `AzureDevOpsApi` is defined in `src/azure/api_trait.rs` with `#[cfg_attr(feature = "test-support", mockall::automock)]`.
252+
- `AzureDevOpsClient` implements the trait by delegating to the standalone API functions.
253+
- Integration tests use `MockAzureDevOpsApi` (enabled via `test-support` feature) to verify tool behavior without external services.
254+
249255
### Dependency injection
250256
- Pass dependencies explicitly via constructor parameters (`new()`).
251-
- Use `Arc<T>` for shared ownership across async tasks (e.g., `Arc<AzureDevOpsClient>` in `AzureMcpServer`).
257+
- Use `Arc<T>` for shared ownership across async tasks (e.g., `Arc<dyn AzureDevOpsApi + Send + Sync>` in `AzureMcpServer`).
252258
- Do NOT rely on mutable global state or `lazy_static!` for wiring dependencies (compile-time constants via `once_cell::sync::Lazy` for regex patterns are acceptable).
253259

254260
### Concurrency and safety

docs/ARCHITECTURE.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,25 @@ graph LR
2020
├── build.rs # Scans #[mcp_tool] → generates tool router
2121
├── Makefile # build, test, lint, fmt targets
2222
├── Dockerfile # Multi-stage: rust:alpine → alpine
23+
├── tests/ # Integration tests (anti-prompt-injection, tool behavior)
24+
│ ├── common/
25+
│ │ └── mod.rs # Shared helpers (assert_tool_output_has_warning)
26+
│ ├── test_tools_organizations.rs
27+
│ ├── test_tools_projects.rs
28+
│ ├── test_tools_teams.rs
29+
│ ├── test_tools_boards.rs
30+
│ ├── test_tools_tags.rs
31+
│ ├── test_tools_work_item_types.rs
32+
│ ├── test_tools_classification_nodes.rs
33+
│ └── test_tools_work_items.rs
2334
├── src/
2435
│ ├── main.rs # CLI entry (clap), transport selection
2536
│ ├── lib.rs # Library root: re-exports modules
2637
│ ├── compact_llm.rs # Compact JSON serializer for LLM output
2738
│ ├── azure/ # Azure DevOps API client layer
2839
│ │ ├── mod.rs
2940
│ │ ├── client.rs # AzureDevOpsClient, AzureError, auth, HTTP helpers
41+
│ │ ├── api_trait.rs # AzureDevOpsApi trait + MockAzureDevOpsApi (test-support feature)
3042
│ │ ├── models.rs # Shared data types (WorkItem, Board, Comment, etc.)
3143
│ │ ├── boards.rs # Boards API
3244
│ │ ├── classification_nodes.rs # Area/Iteration paths API
@@ -191,7 +203,7 @@ classDiagram
191203
}
192204
193205
class AzureMcpServer {
194-
-Arc~AzureDevOpsClient~ client
206+
-Arc~dyn AzureDevOpsApi~ client
195207
-ToolRouter~Self~ tool_router
196208
+new(client) Self
197209
}

docs/PROJECT.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,6 @@ MCP tool responses are optimized for LLM consumption:
154154
Each tool follows this pattern:
155155
1. Define `Args` struct with `Deserialize` + `JsonSchema` (schemars).
156156
2. Annotate the async function with `#[mcp_tool(name = "...", description = "...")]`.
157-
3. Function signature: `pub async fn tool_name(client: &AzureDevOpsClient, args: ArgsType) -> Result<CallToolResult, McpError>`.
157+
3. Function signature: `pub async fn tool_name(client: &(dyn AzureDevOpsApi + Send + Sync), args: ArgsType) -> Result<CallToolResult, McpError>`.
158158
4. Convert domain errors to `McpError` via `.map_err()`.
159159
5. Return `tool_text_success(content)` (from `support/tool_text_success.rs`) — this automatically prepends the anti-prompt-injection warning to all tool responses. Never use `CallToolResult::success(vec![Content::text(...)])` directly.

docs/plans/1_azure_devops_api_trait_integration_tests_20260311142611.md

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ Tools currently take `&AzureDevOpsClient` directly, making mocking impossible. T
1414

1515
Establishes the mockable API boundary so tools can be tested without external services.
1616

17-
- [ ] `list_team_members` normalized from `impl AzureDevOpsClient` method to standalone fn in `teams.rs`
18-
- [ ] `create_work_item`/`update_work_item` in `work_items.rs` accept owned types: `&[(String, Value)]`, `&[(String, String)]`
19-
- [ ] `AzureDevOpsApi` trait in `src/azure/api_trait.rs` with 23 async methods and `#[cfg_attr(feature = "test-support", mockall::automock)]`
20-
- [ ] `impl AzureDevOpsApi for AzureDevOpsClient` delegating to existing standalone functions
21-
- [ ] `mockall` moved to optional dependency, `test-support` feature added to `Cargo.toml`
22-
- [ ] Module registered in `src/azure/mod.rs`
23-
- [ ] `UNTRUSTED_CONTENT_WARNING` exported from `src/mcp/tools/support/mod.rs`
24-
- [ ] Makefile updated to pass `--features test-support` to test/lint targets
17+
- [x] `list_team_members` normalized from `impl AzureDevOpsClient` method to standalone fn in `teams.rs`
18+
- [x] `create_work_item`/`update_work_item` in `work_items.rs` accept owned types: `&[(String, Value)]`, `&[(String, String)]`
19+
- [x] `AzureDevOpsApi` trait in `src/azure/api_trait.rs` with 23 async methods and `#[cfg_attr(feature = "test-support", mockall::automock)]`
20+
- [x] `impl AzureDevOpsApi for AzureDevOpsClient` delegating to existing standalone functions
21+
- [x] `mockall` moved to optional dependency, `test-support` feature added to `Cargo.toml`
22+
- [x] Module registered in `src/azure/mod.rs`
23+
- [x] `UNTRUSTED_CONTENT_WARNING` exported from `src/mcp/tools/support/mod.rs`
24+
- [x] Makefile updated to pass `--features test-support` to test/lint targets
2525

2626
### Task 1.1: Normalize `list_team_members` in `src/azure/teams.rs` (modify)
2727

@@ -528,11 +528,11 @@ Pass `--features test-support` to test and lint targets so `MockAzureDevOpsApi`
528528

529529
Replace concrete `AzureDevOpsClient` references with the trait throughout the tool stack so mocks can be injected.
530530

531-
- [ ] `AzureMcpServer.client` is `Arc<dyn AzureDevOpsApi + Send + Sync>`
532-
- [ ] `build.rs` delegates via `&*self.client`
533-
- [ ] All 24 tool functions accept `&(dyn AzureDevOpsApi + Send + Sync)`
534-
- [ ] All API calls go through trait methods
535-
- [ ] `main.rs` unchanged
531+
- [x] `AzureMcpServer.client` is `Arc<dyn AzureDevOpsApi + Send + Sync>`
532+
- [x] `build.rs` delegates via `&*self.client`
533+
- [x] All 24 tool functions accept `&(dyn AzureDevOpsApi + Send + Sync)`
534+
- [x] All API calls go through trait methods
535+
- [x] `main.rs` unchanged
536536

537537
### Task 2.1: Update `src/mcp/server.rs` (modify)
538538

@@ -687,9 +687,9 @@ All tool files MUST keep `CallToolResult` in the `rmcp::model` import — it is
687687

688688
Proves every tool prepends the anti-prompt-injection warning by exercising real tool functions against mocked API.
689689

690-
- [ ] Every MCP tool has an integration test verifying anti-prompt-injection warning prefix
691-
- [ ] Tests use `MockAzureDevOpsApi` from `#[automock]`
692-
- [ ] Tests exercise full tool function → `CallToolResult` path
690+
- [x] Every MCP tool has an integration test verifying anti-prompt-injection warning prefix
691+
- [x] Tests use `MockAzureDevOpsApi` from `#[automock]`
692+
- [x] Tests exercise full tool function → `CallToolResult` path
693693

694694
### Task 3.1: Create `tests/common/mod.rs` (create)
695695

@@ -799,11 +799,11 @@ Note: the `extract_text_from_result` implementation depends on rmcp's `Content`
799799

800800
Ensure project docs and reviewer agents are aware of the new `AzureDevOpsApi` trait convention.
801801

802-
- [ ] `CLAUDE.md` updated with trait convention
803-
- [ ] `.claude/agents/code-reviewer.md` updated
804-
- [ ] `.claude/agents/plan-reviewer.md` updated
805-
- [ ] `docs/PROJECT.md` updated
806-
- [ ] `docs/ARCHITECTURE.md` updated
802+
- [x] `CLAUDE.md` updated with trait convention
803+
- [x] `.claude/agents/code-reviewer.md` updated
804+
- [x] `.claude/agents/plan-reviewer.md` updated
805+
- [x] `docs/PROJECT.md` updated
806+
- [x] `docs/ARCHITECTURE.md` updated
807807

808808
### Task 4.1: Update `CLAUDE.md` (modify)
809809

0 commit comments

Comments
 (0)