Skip to content

feat(datasource): reply to a Pylon issue from Forest on the customer's channel [PRD-1873] - #420

Merged
christophebrun-forest merged 9 commits into
mainfrom
feature/prd-1873-pylon-datasource-reply-to-an-issue-from-forest-on-the
Nov 4, 2026
Merged

christophebrun-forest merged 9 commits into
mainfrom
feature/prd-1873-pylon-datasource-reply-to-an-issue-from-forest-on-the

Conversation

@christophebrun-forest

@christophebrun-forest christophebrun-forest commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

What

Adds a ReplyToIssue action plugin to forest_admin_datasource_pylon. An operator can answer a Pylon issue from Forest, and the reply reaches the customer on the channel they used: Slack stays Slack, email stays email. The operator can post an internal note instead.

Linear: PRD-1873

How

Pylon keeps the channel: POST /issues/{id}/reply delivers the reply into the conversation of the message named by message_id. The plugin picks that message and, for email, the recipients, which Pylon does not infer.

  • Channel rule (ReplyTarget): the channel is the source of the first customer-visible message. The reply targets the latest customer-visible message on that channel. With no customer-visible message, only an internal note is offered.
  • Email recipients are pre-filled from that message and stay editable:
    • when the customer wrote the message, To is its sender only, since the customer's own To holds the support inbox; when the team wrote it, To is whom it went to;
    • support_emails and the addresses the team wrote from in the thread are left out, from the pre-fill and from what the operator types.
  • Form (FormBuilder): reply type, channel (read-only), To and CC (email only), rich-text message. The thread is read once per render.
  • Execution:
    • the thread is read again with fetch_complete_issue_messages, which raises instead of degrading to nil, so an unreadable thread is never taken for an issue without a customer message;
    • an internal note needs no thread;
    • a Pylon 4xx is shown to the operator as Pylon words it;
    • any other failure asks the operator to check Pylon before resending, since the message may already have been delivered.
  • post_as_operator (off by default):
    • a public reply passes the user_id of the active Pylon user whose email matches the Forest caller's;
    • no match, several matches or a caller without an email refuse the reply. It never falls back to signing with the token;
    • internal notes are always signed by the token.
  • Client: reply_to_issue and create_issue_note, each with its own 30/min RateLimits rule. A 2xx is taken as delivered whatever its body.
  • Message thread: embedded messages now carry email_from, email_to and email_cc.

Reviewer notes

  • The channel is deliberately the one of the first customer-visible message, as decided in PRD-1873. An issue that started on another channel and moved to email is answered on the first channel.
  • The per-render cache is one strongly held entry per thread. ObjectSpace::WeakMap lets values be collected mid-render, and WeakKeyMap needs Ruby 3.3 while the gem supports 3.0.
  • Clicking a row of the embedded messages list shows an unrelated front error, tracked in PRD-1875.

Testing

  • bundle exec rspec in the package: 902 examples, 0 failures, 100% line coverage. bundle exec rubocop: clean.
  • Form checked manually from a local app on a real Pylon workspace.
  • Real sends tested on a Slack issue and an email issue.
  • Chat widget not tested and out of scope: it is not enabled on the client instance.

🤖 Generated with Claude Code

Note

Add ReplyToIssue plugin to reply to Pylon issues from Forest

  • Adds a configurable single-scope action in reply_to_issue.rb that posts a public reply or an internal note to a Pylon issue, with a dynamic form built by form_builder.rb.
  • Resolves the reply target as the latest customer-visible message on the channel of the first customer-visible message, filters support and team addresses from email recipients, and prefills To/CC from the target message in reply_target.rb.
  • Optional operator signing matches the Forest caller's email to an active Pylon user and attaches its user ID; zero or multiple matches block the reply (operator_lookup.rb).
  • Adds strict Client#fetch_complete_issue_messages for whole-thread reads, Client::Writes#reply_to_issue and #create_issue_note write calls, email envelope fields (email_from, email_to, email_cc) in the embedded message schema, and 30-req/min rate-limit buckets for the reply and note endpoints.
  • Behavioral Change: the truncated-walk APIError text in Client#fetch_all is now shorter; messages expose email sender/recipients (with nil for non-email messages), while bcc_emails and message_id are excluded; write failures other than Pylon 4xx return an uncertain-delivery warning.

Changes since #420 opened

  • Strengthened the client.reply_to_issue expectations in two ForestAdminDatasourcePylon::Plugins::ReplyToIssue specs so they assert the call arguments. [20ff8e1]

Macroscope summarized 50928c7.

christophebrun-forest and others added 6 commits October 30, 2026 15:00
Client::Writes gains reply_to_issue and create_issue_note, posting to
/issues/{id}/reply and /issues/{id}/note with the id escaped. Both get
their own 30/min window in RateLimits instead of the shared fallback.

fetch_complete_issue_messages reads the thread for a caller acting on
it: a failed or truncated read raises instead of degrading to nil, so
an unreadable thread is never taken for one with no customer message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… [PRD-1873]

Each embedded message now carries email_from, email_to and email_cc,
flattened from Pylon's email_info and null outside email. A reply to an
email issue must name its recipients, which Pylon does not infer.

Blind copies and the RFC 5322 message id header stay out. Also corrects
the thread endpoint budget in the MessagesEmbedder comment (120/min).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Answers a Pylon issue from Forest on the channel the customer used, or
posts an internal note. The channel is the source of the first
customer-visible message; the reply targets the latest message on that
channel. Email recipients are pre-filled from that message, support
addresses left out, since Pylon does not infer them.

The form reads the thread once per render. The executor reads it again,
strictly: an unreadable thread is reported as such, never as an issue
without customer message. Pylon 4xx reach the operator as the action
error. Replies are signed by the API token account for now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With post_as_operator, a public reply passes the user_id of the active
Pylon user whose email or emails match the Forest caller email, case
insensitive. No match, several matches or a caller without email
refuse the reply and send nothing: falling back to the token would sign
some replies in the operator name and others not, out of their sight.

Internal notes stay signed by the token account. Off by default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Options, the channel rule, how email recipients are pre-filled, who
signs a reply, and what to know before granting the action: the
issue_id_field column is the authority, and post_as_operator trusts
the emails shared between Forest and Pylon.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…873]

- Recipients: the sender of the latest email joins To only when the
  customer wrote it, and the addresses the team wrote from in the thread
  are left out even when support_emails misses them.
- An internal note no longer needs the thread, so an unreadable thread
  still lets the operator leave one.
- A reply or note answered 2xx is taken as delivered whatever its body,
  and a non-4xx failure asks the operator to check Pylon before sending
  again instead of surfacing a bare 500.
- The per-render thread cache holds its state strongly: WeakMap values
  could be collected mid-render.
- An emptied rich-text editor (<p><br></p>) is refused as empty.
- The operator lookup asks Pylon for active users only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 30, 2026

Copy link
Copy Markdown

PRD-1873

@qltysh

qltysh Bot commented Oct 30, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

⬆️ Merging this pull request will increase total coverage on main by 0.1%.

Modified Files with Diff Coverage (8)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
..._admin_datasource_pylon/collections/issue/messages_embedder.rb100.0%
Coverage rating: A Coverage rating: A
...n_datasource_pylon/lib/forest_admin_datasource_pylon/client.rb100.0%
Coverage rating: A Coverage rating: A
...ource_pylon/lib/forest_admin_datasource_pylon/client/writes.rb100.0%
New file Coverage rating: A
..._admin_datasource_pylon/plugins/reply_to_issue/reply_target.rb100.0%
New file Coverage rating: A
...admin_datasource_pylon/plugins/reply_to_issue/thread_lookup.rb100.0%
New file Coverage rating: A
...on/lib/forest_admin_datasource_pylon/plugins/reply_to_issue.rb100.0%
New file Coverage rating: A
..._admin_datasource_pylon/plugins/reply_to_issue/form_builder.rb100.0%
New file Coverage rating: A
...min_datasource_pylon/plugins/reply_to_issue/operator_lookup.rb100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@christophebrun-forest
christophebrun-forest marked this pull request as ready for review October 30, 2026 15:22
…D-1873]

- Answering a customer email writes to its sender only: the customer's
  own To holds the support inbox, which nothing else names before the
  team's first email when support_emails is not set.
- The support and team addresses are also removed from what the
  operator types in To and CC, and CC is deduplicated against To.
- An email_info that is not an object no longer raises out of the
  thread lookup.
- The per-render state is kept on the render context rather than in a
  thread-local slot that outlived the request.
- A literal non-breaking space or zero-width character no longer passes
  as a message, and a blank Pylon user name falls back to the email.
- issue_messages_path is private; the README states what a submit costs
  in thread reads, the agent rendering the form again before executing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@qltysh

qltysh Bot commented Oct 30, 2026

Copy link
Copy Markdown

7 new issues

Tool Category Rule Count
qlty Structure Function with high complexity (count = 6): run 4
qlty Structure Function with many parameters (count = 5): reply 2
qlty Structure Function with many returns (count = 4): reply 1

support_emails: normalize_emails(options[:support_emails])
}

collection_customizer.add_action((options[:action_name] || NAME).to_s, build_action(datasource, opts))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function with high complexity (count = 6): run [qlty:function-complexity]

note(datasource, issue_id, body)
end
result_builder.public_send(status, message: message)
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function with high complexity (count = 9): executor [qlty:function-complexity]

text.gsub(/[[:space:]\u200B-\u200D\u2060\uFEFF]/, '').empty?
end

def reply(datasource, state, body, context, opts)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function with many parameters (count = 5): reply [qlty:function-parameters]

opts[:allow_internal_note] ? "#{no_target} Post an internal note instead." : no_target
end

def recipients_field(label, state, opts, side)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function with many parameters (count = 4): recipients_field [qlty:function-parameters]


Result.new(error: refusal(email, matches.size))
rescue APIError => e
Result.new(error: "Could not read the Pylon users to sign the reply in your name: #{e.message}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function with high complexity (count = 5): call [qlty:function-complexity]

@christophebrun-forest christophebrun-forest self-assigned this Nov 2, 2026
@matthv

matthv commented Nov 3, 2026

Copy link
Copy Markdown
Member

Overall this looks solid. I found no confirmed bug, the pylon suite is green, and reply POSTs are never replayed. These are the points I'd address before merging. Line numbers refer to the current head.

Tests that can hide real bugs

  1. Customer messages in the specs have no author. Real Pylon payloads always carry { 'contact' => …, 'user' => nil }. Changing ReplyTarget.team_authored? to author.is_a?(Hash) still passes all of reply_to_issue_spec.rb. In production that change would treat every customer email as team-written: To would be pre-filled with the support inbox, and the customer would be excluded from their own reply. Making the message / email_message helpers use the real author shape would close this.
  2. No test covers a write failure without an HTTP status (timeout). Replacing (400..499).cover?(e.status.to_i) with e.status.to_i < 500 still passes. A timeout would then show the raw error instead of "may have been delivered, check Pylon". Worth adding a plugin test raising APIError with status: nil, and a writes_spec test with .to_timeout asserting the reply POST is sent only once.

Behaviour
3. A failed thread read hides "Public reply". form_builder.rb:42-43 only looks at state.target, so a transient 429/5xx looks the same as "no customer-visible message", and only the internal note is offered. Keeping both options when state.error is set would let the executor decide, since it re-reads the thread anyway.
4. Typed recipients are dropped silently (reply_to_issue.rb:74-79). An excluded address in CC disappears with no message. If To only holds excluded addresses, the error says a To is required although one was typed. Naming the dropped addresses would avoid both confusions.
5. A matched Pylon user without id sends user_id: nil (reply_to_issue.rb:86), so the token signs while the success message says "signed by ". Treating that case as a refusal in OperatorLookup would keep the "never fall back to the token" guarantee.
6. The Channel field reads "Reply sent via …" (form_builder.rb:57) before anything is sent. Something like "Replies via email" would be accurate.

Stale comments
7. client.rb:181-186 (fetch_all) still says a walk past the cap is refused "unlike the thread", but fetch_complete_issue_messages now refuses past the cap too. client.rb:206 (collect_pages) should mention both callers that refuse.

Smaller, optional

  • Recipient exclusion is written twice, in ReplyTarget#recipients and again in the executor. A ReplyTarget#sanitize(to:, cc:) would keep one copy.
  • Address normalisation differs: team_addresses downcases but does not strip.
  • README: the request-cost bullet leaves out the GET /users made when post_as_operator is on. "Shown as Pylon words it" overstates it a little, since Pylon's text arrives inside the client's error prefix.

- give spec messages the real Pylon author shape, so a wrong team check fails
- cover a reply that timed out with no status, in the plugin and the client
- keep the public reply offered when the thread could not be read
- name the typed addresses left out as the team's, via ReplyTarget#sanitize
- refuse to sign as a matched Pylon user that has no id
- word the channel field as what will happen, not what happened
- refresh the stale fetch_all / collect_pages comments and the README

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
payload['user_id'] = operator.user['id'] if operator
write(state.issue_id, 'reply', reply_success(state, payload, operator&.user) + left_out(dropped)) do
datasource.client.reply_to_issue(state.issue_id, payload)
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 2 issues:

1. Function with many returns (count = 4): reply [qlty:return-statements]


2. Function with high complexity (count = 6): reply [qlty:function-complexity]

@christophebrun-forest

Copy link
Copy Markdown
Member Author

Thanks for the review. Everything is addressed in 50928c7:

  1. Author shape in specs: the message / email_message helpers now carry the real Pylon author (contact set, user nil for customers; author: team for team messages). Your author.is_a?(Hash) mutation now fails 8 examples.
  2. Timeout without status: added a plugin test raising APIError with status: nil (expects the "may have been delivered" message), and a writes_spec test with .to_timeout asserting the reply POST is sent once. The < 500 mutation now fails.
  3. Failed thread read: both reply types stay offered. The default stays "Internal note". Picking "Public reply" renders the form again, which reads the thread again.
  4. Dropped recipients: new ReplyTarget#sanitize(to:, cc:) returns the excluded addresses too. The success message and the "To required" error both name them. This also leaves a single copy of the exclusion logic.
  5. Matched user without id: OperatorLookup now refuses, so the token never signs silently.
  6. Channel field: reads "Replies via …".
  7. Stale comments: fetch_all and collect_pages now mention fetch_complete_issue_messages.

Smaller points: team_addresses now strips as well as downcases. The README mentions the GET /users cost with post_as_operator, and "as Pylon words it" became "with Pylon's own reason".

@matthv matthv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec (PRD-1873): conforms.

…specs [PRD-1873]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@matthv

matthv commented Nov 4, 2026

Copy link
Copy Markdown
Member

Forest Validator · Verify fixes: ✅ 1 closed

When (UTC) Commit Step Result
04/11 10:10 20ff8e1 Verify fixes ✅ 1 closed
1 finding✅ closed · Violates conventions: the image-only test would still pass if the reply body were stripped or rewri…
· run 420-20261104100838-matthv-verify

@matthv matthv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after /verify-fixes run 420-20261104100838-matthv-verify: 1 findings of 420-20261103142041-matthv closed or accepted, none reopened.

@christophebrun-forest
christophebrun-forest merged commit fcf628c into main Nov 4, 2026
33 checks passed
@christophebrun-forest
christophebrun-forest deleted the feature/prd-1873-pylon-datasource-reply-to-an-issue-from-forest-on-the branch November 4, 2026 10:45
forest-bot added a commit that referenced this pull request Nov 4, 2026
# [1.47.0](v1.46.4...v1.47.0) (2026-11-04)

### Features

* **datasource:** reply to a Pylon issue from Forest on the customer's channel [PRD-1873] ([#420](#420)) ([fcf628c](fcf628c))
@forest-bot

Copy link
Copy Markdown
Member

🎉 This PR is included in version 1.47.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants