Make Helpdesk email updates explicit

docs/helpdesk_smoke_test.md

@@ -6,9 +6,10 @@ redMCP work together on real Helpdesk issues.
 The test creates one inbound Helpdesk email for `fud-helpdesk`, triggers
 Helpdesk mail import, fetches the created issue through
 `RedMCP\RedmineClient::issueWithHelpdesk()`, performs a non-Helpdesk CRUD
-control in `fud-nohelpdesk`, sends a Helpdesk reply through the same mailer path
+control in `fud-nohelpdesk`, sends a Helpdesk reply with
-used by the UI's "Send note" checkbox, verifies the outbound message arrives in
+`updateIssue(..., ['send_helpdesk_email' => true])`, verifies the outbound
-Mailpit, and closes the Helpdesk issue.
+message arrives in Mailpit, verifies a default `updateIssue()` does not send
+mail, and closes the Helpdesk issue.
 
 ## Run
 
@@ -37,6 +38,7 @@ The final output should include:
 
 ```text
 [OK] redMCP issueWithHelpdesk returned ticket and message context
+[OK] redMCP default issue update did not send Helpdesk email
 [OK] redMCP non-Helpdesk CRUD control passed
 [OK] Mailpit received outbound Helpdesk/Redmine mail containing the reply token
 [OK] Closed smoke-test Helpdesk issue

helpdesk_smoke_test.py

@@ -122,13 +122,22 @@ def run_smoke(config: SmokeConfig) -> None:
     assert_helpdesk_context(context, issue_id, from_address, subject, require_messages=False)
     print("[OK] redMCP issueWithHelpdesk returned Helpdesk ticket context")
 
+    silent_token = f"redMCP silent internal note {token}"
+    redmcp_call(config, "updateIssue", {"issue_id": issue_id, "fields": {"notes": silent_token}})
+    assert_mailpit_text_absent(config, silent_token)
+    print("[OK] redMCP default issue update did not send Helpdesk email")
+
     control = redmcp_call(config, "createControlIssue", {"project": config.control_project, "token": token})
     control_id = int(control["id"])
     redmcp_call(config, "updateIssue", {"issue_id": control_id, "fields": {"notes": f"redMCP control update {token}"}})
     redmcp_call(config, "deleteIssue", {"issue_id": control_id})
     print(f"[OK] redMCP non-Helpdesk CRUD control passed with issue #{control_id}")
 
-    send_helpdesk_reply(config, issue_id, reply_token)
+    redmcp_call(
+        config,
+        "updateIssue",
+        {"issue_id": issue_id, "fields": {"notes": reply_token}, "options": {"send_helpdesk_email": True}},
+    )
     wait_for_mailpit_text(config, before_ids, reply_token)
     print("[OK] Mailpit received outbound Helpdesk/Redmine mail containing the reply token")
 
@@ -195,34 +204,6 @@ def trigger_helpdesk_import(config: SmokeConfig) -> None:
         raise SmokeError("Helpdesk import failed: " + (result.stderr or result.stdout).strip())
 
 
-def send_helpdesk_reply(config: SmokeConfig, issue_id: int, content: str) -> None:
-    ruby = (
-        "issue = Issue.find(%d); "
-        "user = User.find_by_login('rebot') || User.find_by_login('admin') || User.find(1); "
-        "User.current = user; "
-        "raise 'issue has no Helpdesk customer' if issue.customer.nil?; "
-        "journal = issue.init_journal(user); "
-        "journal.notes = %s; "
-        "issue.save!; "
-        "contact = issue.customer; "
-        "HelpdeskMailer.with_activated_perform_deliveries do; "
-        "msg = HelpdeskMailer.issue_response(contact, journal, {}).deliver; "
-        "JournalMessage.create!(:from_address => '', "
-        ":to_address => contact.primary_email.downcase, "
-        ":is_incoming => false, "
-        ":message_date => Time.now, "
-        ":message_id => msg.message_id.to_s.slice(0, 255), "
-        ":source => HelpdeskTicket::HELPDESK_EMAIL_SOURCE, "
-        ":contact => contact, "
-        ":journal => journal); "
-        "end; "
-        "puts 'ok'"
-    ) % (issue_id, ruby_string(content))
-    result = ssh_redmine(config, f"bin/rails runner -e production {shell_quote(ruby)}")
-    if result.returncode != 0:
-        raise SmokeError("Helpdesk reply failed: " + (result.stderr or result.stdout).strip())
-
-
 def wait_for_issue(config: SmokeConfig, subject: str, timeout: int = 60) -> int:
     deadline = time.time() + timeout
     while time.time() < deadline:
@@ -295,7 +276,7 @@ switch ($input['action']) {
         ]);
         break;
     case 'updateIssue':
-        $client->updateIssue((int) $payload['issue_id'], $payload['fields']);
+        $client->updateIssue((int) $payload['issue_id'], $payload['fields'], $payload['options'] ?? []);
         $result = ['ok' => true];
         break;
     case 'deleteIssue':
@@ -380,6 +361,16 @@ def wait_for_mailpit_text(config: SmokeConfig, before_ids: set[str], needle: str
     raise SmokeError(f"Timed out waiting for Mailpit message containing {needle!r}")
 
 
+def assert_mailpit_text_absent(config: SmokeConfig, needle: str, timeout: int = 5) -> None:
+    deadline = time.time() + timeout
+    while time.time() < deadline:
+        for message_id in mailpit_message_ids(config):
+            message = mailpit_json(config, f"/api/v1/message/{message_id}", {})
+            if needle in json.dumps(message):
+                raise SmokeError(f"Mailpit unexpectedly received message containing {needle!r}")
+        time.sleep(1)
+
+
 def mailpit_json(config: SmokeConfig, path: str, params: dict[str, str]) -> dict[str, Any]:
     query = urllib.parse.urlencode(params)
     url = f"http://{config.mailpit_host}:{config.mailpit_http_port}{path}"

redMCP/README.md

@@ -47,6 +47,25 @@ $client->updateIssue((int) $created['id'], ['notes' => 'Follow-up note']);
 $client->deleteIssue((int) $created['id']);
 ```
 
+`updateIssue()` is intentionally safe by default: on Helpdesk-backed issues, a
+normal Redmine note does **not** send an email to the customer. To send through
+the Helpdesk plugin, opt in explicitly:
+
+```php
+$client->updateIssue(39858, [
+    'notes' => 'Customer-visible response.',
+], [
+    'send_helpdesk_email' => true,
+]);
+
+// Equivalent explicit form:
+$client->sendHelpdeskIssueResponse(39858, 'Customer-visible response.');
+```
+
+Use the default non-email update for internal notes, status/category/assignee
+changes, and automation cleanup. Use the Helpdesk email path only when the
+caller deliberately wants the customer to receive mail.
+
 ## Test instance
 
 A working test copy of Redmine is available on the LAN at `192.168.50.170`.

redMCP/app/RedmineClient.php

@@ -101,17 +101,41 @@ final class RedmineClient
     /**
      * Update a Redmine issue.
      *
+     * By default this uses the normal Redmine issue REST API and does not send
+     * a Helpdesk email. To send a customer-visible Helpdesk response, pass
+     * ['send_helpdesk_email' => true] with a notes field, or call
+     * sendHelpdeskIssueResponse() directly.
+     *
      * Typical fields include notes, subject, status_id, priority_id,
      * assigned_to_id, private_notes, due_date, and tracker_id.
      *
      * @param array<string,mixed> $fields
      */
-    public function updateIssue(int $issueId, array $fields): bool
+    public function updateIssue(int $issueId, array $fields, array $options = []): bool
     {
         if ($fields === []) {
             throw new RuntimeException('Updating an issue requires at least one field.');
         }
 
+        if (!empty($options['send_helpdesk_email'])) {
+            if (!isset($fields['notes']) || trim((string) $fields['notes']) === '') {
+                throw new RuntimeException('Sending a Helpdesk email requires a non-empty notes field.');
+            }
+            if (!empty($fields['private_notes'])) {
+                throw new RuntimeException('A private note cannot be sent as a Helpdesk email.');
+            }
+
+            $issueFields = $fields;
+            unset($issueFields['notes'], $issueFields['private_notes']);
+            if ($issueFields !== []) {
+                $this->updateIssue($issueId, $issueFields);
+            }
+
+            $this->sendHelpdeskIssueResponse($issueId, (string) $fields['notes'], $options);
+
+            return true;
+        }
+
         $issueApi = $this->client->getApi('issue');
         $issueApi->update($issueId, $fields);
         $this->assertLastApiResponseSucceeded($issueApi, 'update issue #' . $issueId);
@@ -119,6 +143,46 @@ final class RedmineClient
         return true;
     }
 
+    /**
+     * Send a Helpdesk email response for an existing Helpdesk-backed issue.
+     *
+     * This uses the RedmineUP Helpdesk API endpoint that corresponds to sending
+     * a note with the Helpdesk UI's "Send note" checkbox enabled.
+     *
+     * @param array<string,mixed> $options Optional to_address, cc_address,
+     *                                     bcc_address, and status_id fields.
+     *
+     * @return array<string,mixed>
+     */
+    public function sendHelpdeskIssueResponse(int $issueId, string $content, array $options = []): array
+    {
+        if (trim($content) === '') {
+            throw new RuntimeException('Sending a Helpdesk response requires non-empty content.');
+        }
+
+        $message = [
+            'issue_id' => $issueId,
+            'content' => $content,
+        ];
+        if (isset($options['status_id'])) {
+            $message['status_id'] = $options['status_id'];
+        }
+
+        $payload = ['message' => $message];
+        foreach (['to_address', 'cc_address', 'bcc_address'] as $key) {
+            if (isset($options[$key])) {
+                $payload[$key] = $options[$key];
+            }
+        }
+
+        $response = $this->postJson('/helpdesk/email_note', $payload);
+        if (!isset($response['message']) || !is_array($response['message'])) {
+            throw new RuntimeException('Helpdesk email response returned unexpected JSON.');
+        }
+
+        return $response['message'];
+    }
+
     /**
      * Delete a Redmine issue.
      */
@@ -269,6 +333,47 @@ final class RedmineClient
         return $decoded;
     }
 
+    /**
+     * @param array<string,mixed> $payload
+     *
+     * @return array<string,mixed>
+     */
+    private function postJson(string $path, array $payload): array
+    {
+        if (!$this->client instanceof HttpClient) {
+            throw new RuntimeException('Configured Redmine client cannot issue raw HTTP requests.');
+        }
+
+        $requestPath = $this->buildPath($path, []);
+        $encoded = json_encode($payload);
+        if ($encoded === false) {
+            throw new RuntimeException('Could not encode Redmine POST payload.');
+        }
+
+        $response = $this->client->request(HttpFactory::makeJsonRequest('POST', $requestPath, $encoded));
+        $status = $response->getStatusCode();
+        if ($status >= 400) {
+            throw new RuntimeException('Redmine POST ' . $requestPath . ' failed with HTTP ' . $status . ': ' . $response->getContent());
+        }
+
+        $body = $response->getContent();
+        if ($body === '') {
+            return [];
+        }
+
+        try {
+            $decoded = json_decode($body, true, 512, JSON_THROW_ON_ERROR);
+        } catch (Throwable $exception) {
+            throw new RuntimeException('Redmine POST ' . $requestPath . ' returned invalid JSON.', 0, $exception);
+        }
+
+        if (!is_array($decoded)) {
+            throw new RuntimeException('Redmine POST ' . $requestPath . ' returned invalid JSON.');
+        }
+
+        return $decoded;
+    }
+
     /**
      * @param array<string,mixed> $params
      */