Add tool registry policy diagnostics by vaddisrinivas · Pull Request #2336 · tinyhumansai/openhuman

vaddisrinivas · May 20, 2026

Refs #2136

Summary

Adds tool_registry.diagnostics as a generic, redacted diagnostics RPC for tool inventory and policy visibility review.
Reports tool counts by transport, enabled count, possible write-capable tool ids, and known policy/approval/registry surfaces.
Adds focused unit coverage for diagnostics schema/handler behavior and policy surface reporting.

Checklist

Issue URL: Add tool-policy diagnostics and conformance reporting #2136
Branch: codex/oh-2136-policy-diagnostics
Commit SHA: a99bd5d9820899a22a618f80d4fb9e0f35057fc9
Files changed summary: src/openhuman/tool_registry/mod.rs, src/openhuman/tool_registry/ops.rs, src/openhuman/tool_registry/schemas.rs, src/openhuman/tool_registry/types.rs
Validation commands run: cargo fmt --manifest-path Cargo.toml --all --check; cargo test --manifest-path Cargo.toml tool_registry
Blocked validations: N/A - none
Behavior intentionally changed: adds a read-only diagnostics RPC/report surface under tool_registry
Duplicate/stale PR note: no existing open PR found for this branch or issue search before creating this PR

Summary by CodeRabbit

New Features
- Added a diagnostics endpoint that returns redacted tool-policy diagnostics: total and enabled tool counts, transport breakdown, possible write surfaces, and policy surfaces.
Tests
- Added coverage and tests validating the diagnostics output and schema (non-empty counts and expected policy-surface entries).

coderabbitai · May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3d02c108-f6bf-42c5-84a6-639beed74a67

📥 Commits

Reviewing files that changed from the base of the PR and between a99bd5d and 3c3905b.

📒 Files selected for processing (2)

src/openhuman/tool_registry/ops.rs
src/openhuman/tool_registry/schemas.rs

📝 Walkthrough

Walkthrough

Adds a diagnostics RPC to the tool registry: defines ToolPolicyDiagnostics, implements diagnostics() with helpers to count tools and discover write-capable and policy surfaces, and wires a diagnostics controller/handler with tests validating schema and JSON output.

Changes

Tool Registry Diagnostics Feature

Layer / File(s)	Summary
ToolPolicyDiagnostics type definition and export `src/openhuman/tool_registry/types.rs`, `src/openhuman/tool_registry/mod.rs`	Defines `ToolPolicyDiagnostics` (counts and redacted surface lists) and re-exports it from the registry module.
Diagnostics computation logic and unit test `src/openhuman/tool_registry/ops.rs`	Adds `pub fn diagnostics()` plus helpers (`looks_write_capable`, `policy_surface_ids`, `is_policy_surface`) and a unit test that validates inventory counts and expected policy/write-capable surfaces.
RPC schema and controller handler wiring `src/openhuman/tool_registry/schemas.rs`	Registers the `diagnostics` controller in `all_controller_schemas()` and `all_registered_controllers()`, defines the `diagnostics` schema I/O, implements `handle_diagnostics` handler, and updates/adds tests asserting schema presence and JSON output shape/content.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

tinyhumansai/openhuman#2003: Earlier work extending the tool_registry surface and APIs; this PR adds diagnostics on top of that registry surface.

Poem

🐰 I hopped through the registry night,
Counting tools by dim moonlight,
Surfaces that write, surfaces that show,
I list them all in tidy rows,
Diagnostics done — a rabbit's delight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 69.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add tool registry policy diagnostics' directly and clearly summarizes the main change: adding a new diagnostics feature to the tool registry focused on policy visibility.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/openhuman/tool_registry/ops.rs (1)

276-311: ⚡ Quick win

Deduplicate policy surface IDs to avoid future drift.

policy_surface_ids() and is_policy_surface() each carry the same hard-coded list. A single source of truth avoids mismatches when surfaces are added/removed.

Proposed refactor

+const POLICY_SURFACES: &[&str] = &[
+    "security.policy_info",
+    "approval.list_pending",
+    "approval.list_recent_decisions",
+    "approval.decide",
+    "tool_registry.list",
+    "tool_registry.get",
+    "tool_registry.diagnostics",
+];
+
 fn policy_surface_ids() -> Vec<String> {
-    let mut ids = [
-        "security.policy_info",
-        "approval.list_pending",
-        "approval.list_recent_decisions",
-        "approval.decide",
-        "tool_registry.list",
-        "tool_registry.get",
-        "tool_registry.diagnostics",
-    ]
+    let mut ids = POLICY_SURFACES
     .into_iter()
-    .map(String::from)
+    .map(|id| (*id).to_string())
     .collect::<BTreeSet<_>>();
@@
 fn is_policy_surface(tool_id: &str) -> bool {
-    matches!(
-        tool_id,
-        "security.policy_info"
-            | "approval.list_pending"
-            | "approval.list_recent_decisions"
-            | "approval.decide"
-            | "tool_registry.list"
-            | "tool_registry.get"
-            | "tool_registry.diagnostics"
-    )
+    POLICY_SURFACES.contains(&tool_id)
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/tool_registry/ops.rs` around lines 276 - 311, The hard-coded
policy surface list is duplicated between policy_surface_ids() and
is_policy_surface(), so centralize the list into a single source of truth (e.g.,
a static array or BTreeSet constant like POLICY_SURFACES) and update both
functions to use it: have policy_surface_ids() build its BTreeSet/Vec from
POLICY_SURFACES and have is_policy_surface(tool_id: &str) check membership
against POLICY_SURFACES (or the BTreeSet) instead of repeating the string
literals; keep the existing return types for policy_surface_ids() and the
signature of is_policy_surface().

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/tool_registry/schemas.rs`:
- Around line 107-115: The diagnostics RPC currently logs only on entry inside
handle_diagnostics; after calling
crate::openhuman::tool_registry::ops::diagnostics() and before returning the
JSON via to_json(...), add a debug-level exit log (e.g., log::debug!) that
indicates the diagnostics RPC completed (optionally include params.len() or a
brief response summary) so the handler has both entry and exit traces; place the
log immediately after the response is constructed and before the
ControllerFuture is returned from handle_diagnostics.

---

Nitpick comments:
In `@src/openhuman/tool_registry/ops.rs`:
- Around line 276-311: The hard-coded policy surface list is duplicated between
policy_surface_ids() and is_policy_surface(), so centralize the list into a
single source of truth (e.g., a static array or BTreeSet constant like
POLICY_SURFACES) and update both functions to use it: have policy_surface_ids()
build its BTreeSet/Vec from POLICY_SURFACES and have is_policy_surface(tool_id:
&str) check membership against POLICY_SURFACES (or the BTreeSet) instead of
repeating the string literals; keep the existing return types for
policy_surface_ids() and the signature of is_policy_surface().

🪄 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: CHILL

Plan: Pro

Run ID: 9cce37fe-5ad1-4ecd-ae8e-17886595aa5c

📥 Commits

Reviewing files that changed from the base of the PR and between ebd6457 and a99bd5d.

📒 Files selected for processing (4)

src/openhuman/tool_registry/mod.rs
src/openhuman/tool_registry/ops.rs
src/openhuman/tool_registry/schemas.rs
src/openhuman/tool_registry/types.rs

M3gA-Mind · May 20, 2026

@vaddisrinivas CI is failing on changes in this PR — please fix before review.

M3gA-Mind · May 20, 2026

@vaddisrinivas CI is failing on changes in this PR — please fix before review.

vaddisrinivas · May 20, 2026

Thanks for the heads-up. CI is green now on the latest run; re-review welcome.

… pr-2336-fix-comments # Conflicts: # src/openhuman/tool_registry/ops.rs

graycyrus

Review — tool_registry policy diagnostics

Well-structured PR. Clean module layout, good test coverage, and the diagnostics RPC fits the existing tool_registry pattern nicely. Both CodeRabbit findings (exit log, deduplication) are already addressed in the latest commits.

Two issues below — one logic bug and one heuristic gap.

Area	Files	Verdict
Rust core (ops)	`ops.rs`	1 major
Rust core (ops)	`ops.rs`	1 minor
Rust core (schemas)	`schemas.rs`	Clean
Rust core (types)	`types.rs`	Clean
Rust core (mod)	`mod.rs`	Clean

vaddisrinivas · May 21, 2026

@graycyrus Thanks again for the review. I addressed both requested diagnostics fixes in 8d25d1a (dynamic policy namespaces and {marker}. write-capability detection) and resolved the threads; current checks are green. Could you please take another look when you have a chance?

graycyrus

Re-review — all prior changes addressed ✓

Both findings from my previous review are resolved in 8d25d1a8:

File	Finding	Status
`ops.rs` — `policy_surface_ids()`	Dynamic extension was a no-op (filtered against same const that seeded the set)	Fixed — `is_policy_surface()` now matches `security.` / `approval.` namespaces dynamically, so newly registered policy schemas are picked up
`ops.rs` — `looks_write_capable()`	Missing `{marker}.suffix` pattern (e.g. `create.user`)	Fixed — added `{marker}.` check, test updated to cover `create.user`

No new issues found. Clean, well-structured addition — nice work @vaddisrinivas.

graycyrus

Looks good, nice work!

Add tool registry policy diagnostics

a99bd5d

vaddisrinivas marked this pull request as ready for review May 20, 2026 13:00

vaddisrinivas requested a review from a team May 20, 2026 13:00

coderabbitai Bot added the working A PR that is being worked on by the team. label May 20, 2026

coderabbitai Bot requested changes May 20, 2026

View reviewed changes

Comment thread src/openhuman/tool_registry/schemas.rs

vaddisrinivas changed the title ~~[codex] Add tool registry policy diagnostics~~ Add tool registry policy diagnostics May 20, 2026

Address tool registry diagnostics review

3c3905b

coderabbitai Bot previously approved these changes May 20, 2026

View reviewed changes

vaddisrinivas added 2 commits May 21, 2026 13:24

Merge remote-tracking branch 'origin/main' into pr-2336-fix-comments

b5c9189

Merge branch 'main' of https://github.com/tinyhumansai/openhuman into…

b586287

… pr-2336-fix-comments # Conflicts: # src/openhuman/tool_registry/ops.rs

vaddisrinivas dismissed coderabbitai[bot]’s stale review via b586287 May 21, 2026 17:35

graycyrus requested changes May 21, 2026

View reviewed changes

Comment thread src/openhuman/tool_registry/ops.rs

Comment thread src/openhuman/tool_registry/ops.rs

Broaden tool registry diagnostics heuristics

8d25d1a

coderabbitai Bot approved these changes May 21, 2026

View reviewed changes

vaddisrinivas requested a review from graycyrus May 21, 2026 19:48

graycyrus reviewed May 22, 2026

View reviewed changes

graycyrus approved these changes May 22, 2026

View reviewed changes

graycyrus merged commit 607a6a1 into tinyhumansai:main May 22, 2026
26 checks passed

senamakel pushed a commit to aqilaziz/openhuman that referenced this pull request May 23, 2026

Add tool registry policy diagnostics (tinyhumansai#2336)

cd956c5

This was referenced May 23, 2026

feat(tool-registry): add trusted capability providers #2551

Merged

feat(tool_policy): diagnostics RPC and Developer Options panel #2715

Merged

CodeGhost21 mentioned this pull request May 27, 2026

Feature Request : Global MCP tool registry - centralised discovery, versioning, and routing for all MCP tools across the swarm #1848

Closed

11 tasks

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

vaddisrinivas commented May 20, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Review ran into problems

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

M3gA-Mind commented May 20, 2026

Uh oh!

M3gA-Mind commented May 20, 2026

Uh oh!

vaddisrinivas commented May 20, 2026

Uh oh!

graycyrus left a comment

Choose a reason for hiding this comment

Review — tool_registry policy diagnostics

Uh oh!

Uh oh!

Uh oh!

vaddisrinivas commented May 21, 2026

Uh oh!

graycyrus left a comment

Choose a reason for hiding this comment

Re-review — all prior changes addressed ✓

Uh oh!

graycyrus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vaddisrinivas commented May 20, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 20, 2026 •

edited

Loading