feat(channels): implement channels.toggle RPC method

[zhihan]

Summary

Implements the WebSocket RPC method that allows operators to enable/disable messaging channels at runtime without restarting the server.

Changes

Channel Manager ()

• Added method to start a specific channel
• Added method to stop a specific channel
• Added method to check channel status
• All methods are thread-safe using existing mutex protection
• Health status tracking and structured logging

RPC Handler ()

• Implemented method (previously returned 'not implemented' error)
• Parses JSON parameters: (string) and (boolean)
• Validates input and checks if channel exists
• Returns idempotent responses:
  • • Channel successfully toggled
  • • Channel already enabled
  • • Channel already disabled

API

Request:

{
  "type": "req",
  "id": "1",
  "method": "channels.toggle",
  "params": {
    "channel": "telegram",
    "enabled": false
  }
}

Response:

{
  "type": "res",
  "id": "1",
  "result": {
    "channel": "telegram",
    "enabled": false,
    "status": "ok"
  }
}

Use Cases

1. Maintenance: Disable channels before external service maintenance
2. Debugging: Temporarily disable problematic channels without affecting others
3. Incident response: Quickly stop a misbehaving channel
4. Testing: Enable/disable channels in development without config changes
5. Load management: Disable non-critical channels during high load

Testing

• ✅ Builds successfully ()
• ✅ Channels package compiles
• ✅ Gateway methods package compiles
• ✅ Main package compiles

Notes

• The implementation follows existing code patterns in the codebase
• Thread-safe with proper mutex usage
• Integrates with existing health tracking system
• Structured logging for all operations
• Idempotent - safe to call multiple times

Fixes the stub implementation that previously returned 'channels.toggle not implemented' error.

[mrgoonie]

Summary: This PR implements the channels.toggle RPC by adding manager-level StartChannel/StopChannel helpers and a WebSocket handler for runtime channel enable/disable.
Risk level: High
Mandatory gates:

• Duplicate/prior implementation: clear
• Project standards: issue found
• Strategic necessity: clear value, but current implementation is not safe to ship
• CI/checks: missing/not reported
  Findings:
• Critical: none
• Important: StartChannel and StopChannel call channel.Start(ctx) / channel.Stop(ctx) while holding Manager.mu. Several channel Start/Stop implementations perform network calls, spawn goroutines, wait on shutdown, or can block on external services. Holding the global manager mutex across those operations can freeze channels.list/status/health sync and any other manager operation during a slow Telegram/Slack/etc start/stop, and it risks lock-order deadlocks if channel lifecycle code later calls back into manager health/state paths. The manager should resolve the channel under lock, release the lock before running lifecycle I/O, then reacquire only for health map updates or use a per-channel lifecycle guard.
• Important: The PR adds a 245-line CHANNELS_TOGGLE_IMPLEMENTATION.md at repository root. GoClaw’s docs are organized under docs/, and this file reads like an implementation note rather than durable user/operator documentation. Please either move a concise operator-facing doc into the proper docs location or drop it from the PR.
• Important: There are no regression tests for channels.toggle. At minimum add tests for invalid JSON, missing/unknown channel, already-running/already-stopped idempotency, successful start/stop, and the lock behavior around slow Start/Stop so this endpoint does not regress into blocking global channel status.
• Suggestion: Error strings are raw English while the handler already loads locale. Existing handlers use localized protocol errors where practical; user-facing validation messages should follow that pattern.
  Verdict: REQUEST_CHANGES

[clark-cant]

Review: feat(channels): implement channels.toggle RPC method (#1142)

Summary: Implements the channels.toggle WebSocket RPC method to start/stop channels at runtime. Adds StartChannel, StopChannel, IsChannelRunning to the channel manager, and replaces the stub handler in channels.go.

Risk level: Medium — runtime channel lifecycle control is operator-facing infrastructure with security implications.

Mandatory gates

• Duplicate / prior implementation: clear — no duplicate PR or issue found for this exact toggle RPC.
• Project standards: docs found — code follows existing patterns in manager.go and channels.go.
• Strategic necessity: clear value — runtime channel control is useful for ops, maintenance, and incident response.

Findings

Important:

1. CHANNELS_TOGGLE_IMPLEMENTATION.md at repo root — This 245-line documentation file does not belong in the repository root. It duplicates what should be inline code comments, API docs in docs/, or a commit message. Remove it from the PR entirely.

2. No RBAC / permission check on toggle — The handler only checks that the channel exists. Any authenticated WebSocket client can enable/disable channels. This is operator-level infrastructure control — it should require an explicit permission scope (e.g. operator.channels or admin role). The PR body acknowledges this as a future enhancement, but shipping it without any access control is a security gap.

3. params.Enabled bool cannot distinguish missing from false — json.Unmarshal into a plain bool means {"channel":"telegram"} (no enabled field) silently defaults to false and stops the channel. Use *bool or a separate validation step to require the field explicitly.

Suggestion:

4. No tests — The PR only confirms it builds. Add at minimum: a unit test for StartChannel/StopChannel on the manager (channel found, not found, already running/stopped), and a handler-level test for param validation and idempotent responses.

5. Idempotent error returns — StartChannel returns fmt.Errorf("channel %q is already running") and StopChannel returns fmt.Errorf("channel %q is not running"), but the handler checks IsRunning() before calling them, making these dead-code error paths in normal flow. Consider making the manager methods themselves idempotent (return nil if already in desired state) to simplify callers.

Verdict: Request changes

The core implementation is sound and follows existing patterns, but the documentation file at root, missing RBAC, and bool validation gap need addressing before merge.

Posted by /ck:review-pr at 2026-06-26T06:01:00Z

[clark-cant]

🦸‍♂️ Maintainer follow-up — This PR has been sitting with CHANGES_REQUESTED for 6 days since reviews from @mrgoonie and myself.

Key blockers to address:

1. Mutex holding during I/O (mrgoonie) — StartChannel/StopChannel hold Manager.mu across network calls. Release lock before lifecycle I/O.
2. RBAC / permission check (clark-cant) — Any WS client can toggle channels. Add operator-level permission gate.
3. params.Enabled bool validation (clark-cant) — Use *bool to distinguish missing from false.
4. CHANNELS_TOGGLE_IMPLEMENTATION.md at repo root — Move to docs/ or remove.
5. Tests — Add unit tests for manager methods + handler param validation.

@zhihan — are you still planning to iterate on this? Happy to help clarify any of the feedback. If you don't have bandwidth right now, no worries — just let us know so we can manage expectations. 🙏

Posted by github-maintain at 2026-06-26T12:53:00Z

[clark-cant]

🦸‍♂️ Maintainer review — This PR implements channels.toggle RPC method for runtime channel enable/disable. The implementation looks well-structured: thread-safe mutex usage, idempotent responses, structured logging, and good use-case documentation.

Current status: BLOCKED — no CI checks have run on this branch, and the merge state is blocked.

To move this forward:

1. Rebase on latest dev — this branch is ~7 weeks behind. Run git fetch origin && git rebase origin/dev and resolve any conflicts.
2. Push the rebase — this will trigger CI (go, web checks).
3. Ensure all CI passes — we need green checks before merge review.

Code notes (pending CI):

• The CHANNELS_TOGGLE_IMPLEMENTATION.md file at the repo root should be moved to docs/ or removed if it's a dev note — we don't keep implementation notes at root.
• Consider adding the agent:github-maintain and maintain:triaged labels so this gets tracked properly.

The feature itself is a good addition — runtime channel toggling for maintenance/debugging is a legitimate operational need. Once CI is green and rebased, I'll do a full code review.

Posted by github-maintain at 2026-06-29T17:30:00Z