Skip to content

feat(rpc): impl tools/enable + tools/disable (#228)#287

Merged
Destynova2 merged 1 commit intomainfrom
feat/rpc-tools-228
Apr 26, 2026
Merged

feat(rpc): impl tools/enable + tools/disable (#228)#287
Destynova2 merged 1 commit intomainfrom
feat/rpc-tools-228

Conversation

@Destynova2
Copy link
Copy Markdown
Contributor

Summary

Replaces the TODO(#228) stubs in server::rpc::tools_ns::enable and disable with working in-memory mutations of config.tool_layer.inject. Follows the P1 pattern (PR #286).

Behaviour

  • enable(tool) appends InjectRule { tool, if_absent: true } after validating the tool isn't already enabled.
  • disable(tool) removes the matching rule after confirming the tool is currently enabled.
  • Both rebuild Router + ProviderRegistry and atomic-swap the ReloadableState.
  • Disk persistence out of scope per Runtime config mutation for RPC namespaces (7 TODOs) #228.

Mutation logic split into pure sync helpers apply_enable / apply_disable so validation is unit-testable without an AppState.

Tests — 10/10 passing

  • enable_appends_inject_rule, enable_rejects_empty_tool_name, enable_rejects_duplicate
  • disable_removes_inject_rule, disable_rejects_not_enabled, disable_rejects_empty_tool_name
  • require_role_denies_observer_for_admin_methods (role check contract)
  • 3 pre-existing ToolInfo tests still passing

Test plan

  • cargo nextest run -E 'test(tools_ns)' --features mcp10/10 passed
  • cargo fmt --all -- --check clean
  • cargo clippy --all-features -- -D warnings clean
  • CI: full nextest + clippy + fmt + audit + deny

Implements P2 of the cli-forge-chef brigade plan for #228. P3 (pledge) and P4 (hit) follow.

🤖 Generated with Claude Code

…228)

Replaces the two `TODO(#228)` stubs in `server::rpc::tools_ns` with
working in-memory mutations of `config.tool_layer.inject`. Mirrors the
P1 pattern (config_ns::set) for atomic state swap.

Behaviour:
- `enable(tool)` appends `InjectRule { tool, if_absent: true }` to
  `tool_layer.inject` after validating the tool isn't already enabled.
- `disable(tool)` removes any matching `InjectRule` after confirming
  the tool is currently enabled.
- Both rebuild Router + ProviderRegistry from the mutated config and
  atomic-swap the ReloadableState (same primitive as `config_ns::set`
  and `server_ns::reload_config`).
- Disk persistence is explicitly out of scope per #228 — the change
  reverts on the next reload from disk.

Mutation logic is split into pure sync helpers `apply_enable` /
`apply_disable` so the validation paths are unit-testable without
constructing an `AppState`.

# Errors

- `ERR_FORBIDDEN` when the caller is below `Admin`.
- `INVALID_PARAMS_CODE` for empty names, duplicate enable, or disable
  on a tool that wasn't enabled.
- `ERR_INTERNAL` when the registry rebuild or atomic swap fails.

Tests: 10/10 passing — fixture-based mutation tests, role denial,
empty-name rejection, duplicate-enable rejection, not-enabled-disable
rejection, plus the pre-existing ToolInfo serialization tests.

Implements P2 of the cli-forge-chef brigade plan for issue #228.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Destynova2 Destynova2 enabled auto-merge April 26, 2026 16:35
@Destynova2 Destynova2 merged commit 2e40db5 into main Apr 26, 2026
42 checks passed
@Destynova2 Destynova2 deleted the feat/rpc-tools-228 branch April 26, 2026 16:43
Destynova2 pushed a commit that referenced this pull request Apr 26, 2026
…228)

Replaces the two `TODO(#228)` stubs in `server::rpc::hit_ns::set_policy`
and `resolve` with working implementations. Closes the last of the four
RPC namespaces tracked by issue #228.

set_policy(name, policy_json):
- Deserializes the JSON payload into `PolicyConfig`.
- The `name` argument always wins over `policy.name` (path-vs-payload).
- Upserts in `config.policies` (replace if same name, append otherwise).
- Compiles a fresh `PolicyMatcher` BEFORE the swap so a malformed glob
  is rejected without dirtying the running registry.
- Atomic-swaps the ReloadableState; the matcher is rebuilt by
  `ReloadableState::new`, so the new policy is live before the RPC
  returns.

resolve(context):
- Builds a `RequestContext` field-by-field from a JSON object (the type
  has no Deserialize derive — it is constructed imperatively in the
  dispatch path; we keep parity here).
- Missing fields fall back to permissive defaults so a partial probe
  ("just match on model") still works.
- Calls `PolicyMatcher::evaluate` and projects the merged
  `ResolvedPolicy` to JSON for transport.

Both functions are gated on `#[cfg(feature = "policies")]` with a
graceful no-op fallback when the feature is off.

# Errors

- `ERR_FORBIDDEN` when the caller is below `Admin` (set_policy) or
  `Operator` (resolve).
- `INVALID_PARAMS_CODE` for empty name, malformed policy JSON,
  malformed glob in match_rules, non-object context.
- `ERR_INTERNAL` when the registry rebuild or atomic swap fails.

Tests: 10/10 passing.

Implements P4 of the cli-forge-chef brigade plan for #228.
With #286 (P1), #287 (P2), #289 (P3), #228 closes when this lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant