fix(message-gatekeeper): allow quoting a tshow message from its parent channel (#432)

[chenjr0719]

Summary

Refs #432. Quoting a tshow ("also send to channel") thread reply from its
parent channel room was rejected with thread context mismatch, because the
quoted-parent's own ThreadParentID (non-empty, since it's a thread reply)
never matched the quoting message's thread (empty, since it's posted in the
channel).

Changes

• pkg/model/cassandra/message.go: add TShow to QuotedParentMessage.
• message-gatekeeper/fetcher_history.go: carry tshow through the
  quoted-parent projection fetched from history-service.
• message-gatekeeper/handler.go: checkQuoteThreadContext now takes the
  quoting message's room and allows the quote when the parent is tshow and
  the quote is posted from that same channel room (not from an unrelated room
  or a different thread).
• docs/client-api.md: documented the new tshow field on
  QuotedParentMessage and the carve-out in the thread-context-mismatch error
  row.

Verification

• New table-driven cases added to TestHandler_resolveQuoteSnapshot
  (message-gatekeeper/handler_test.go): tshow-from-channel allowed,
  non-tshow thread reply from channel still rejected, tshow parent from an
  unrelated room still rejected, tshow parent quoted from within its own
  thread still allowed.
• go build ./message-gatekeeper/... ./history-service/... ./pkg/... passes.
• go test -race ./message-gatekeeper/... ./history-service/... ./pkg/model/... passes.

Why surface tshow on the quote projection

The guard must distinguish a tshow ("also send to channel") parent — legitimately visible
in the channel — from a normal thread reply, which is not. Both carry a non-empty
ThreadParentID and a channel RoomID, so no existing snapshot field tells them apart;
tshow is the only discriminator.

tshow is not new data: it already exists on the messages_by_id row and is already
returned by history-service's get-by-id reply. The gatekeeper simply fetches a narrow
projection of the quoted parent (quotedParentProjection) that previously dropped it — so
this change stops discarding a field already on the wire, rather than adding state. No
history-service change, no schema/DDL.

The only alternative that avoids carrying the flag is to replace the thread-linkage rule
with a room-visibility check (resolve whether the quoted message is present in the quoter's
room timeline). That costs an extra read per quote and a larger re-architecture of the
same-conversation rule for no benefit here, so it was rejected in favor of the existing,
already-transmitted tshow flag.

Summary by CodeRabbit

• New Features

  • Added support for preserving an additional quote-context flag on quoted messages, improving how thread replies are represented.
  • Quoted replies can now be validated more accurately when sent from a thread or its parent channel.

• Bug Fixes

  • Tightened quote validation to reject mismatched thread-context quotes.
  • Allowed the documented exception for thread replies that are shown in the parent channel, while still blocking invalid cross-room quotes.

• Documentation

  • Updated API docs to reflect the new quote metadata and validation behavior.

[coderabbitai]

Warning

Review limit reached

@chenjr0719, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3ae9cdd5-127e-497e-8a68-0983b0b5315c

📥 Commits

Reviewing files that changed from the base of the PR and between bf4c348 and c803d3e.

📒 Files selected for processing (6)

• docs/client-api.md
• message-gatekeeper/fetcher_history.go
• message-gatekeeper/handler.go
• message-gatekeeper/handler_test.go
• pkg/model/cassandra/message.go
• pkg/model/cassandra/message_test.go

📝 Walkthrough

Walkthrough

Adds a TShow boolean field to QuotedParentMessage, decodes it from the history-service reply, and uses it to relax quote thread-context validation—allowing tshow: true thread replies to be quoted from their parent channel room. Includes model tests, handler tests, and documentation updates.

Changes

TShow Quoted-Parent Feature

Layer / File(s)	Summary
QuotedParentMessage model field pkg/model/cassandra/message.go, pkg/model/cassandra/message_test.go	Adds TShow bool field with json:"tshow,omitempty" and cql:"tshow" tags; adds a JSON round-trip test verifying presence/omission of tshow and value preservation.
History fetcher propagation message-gatekeeper/fetcher_history.go	quotedParentProjection gains a TShow field decoded from the reply, and FetchQuotedParent sets TShow on the returned snapshot.
Thread-context validation carve-out message-gatekeeper/handler.go, message-gatekeeper/handler_test.go	checkQuoteThreadContext gains a newMessageRoomID parameter and a tshowChannelQuote condition exempting TShow parents quoted from their own channel room; test matrix extended with four new scenarios covering allow/reject cases.
Client API docs docs/client-api.md	Documents the tshow field on QuotedParentMessage and a bad_request error case for quote thread-context mismatches, noting the tshow: true exception.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related issues

• [Fix] Quoting an also-send-to-channel (tshow) message from the channel fails with thread context mismatch #432: Matches the retrieved issue's quoting bug fix by adding tshow to the quoted-parent snapshot and relaxing thread-context validation.
• [Fix] message-worker re-projection drops the quote when quoting a tshow parent from the channel (sibling of #432) #434: Addresses the same tshow quote rule for thread/channel context, covering the gatekeeper/model/docs side.

Possibly related PRs

• hmchangw/chat#90: Both modify the shared QuotedParentMessage model, with this PR adding the TShow field on top of the original UDT support.
• hmchangw/chat#134: Both update the quote resolution path via resolveQuoteSnapshot/checkQuoteThreadContext, this PR adding the tshow/room-based carve-out.
• hmchangw/chat#400: Both change the same handler functions for quoted-parent resolution/validation, though for different purposes (tshow carve-out vs. outage degradation).

Suggested reviewers: mliu33

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: allowing quotes of tshow thread replies from their parent channel.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/432-tshow-quote-from-channel

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@pkg/model/cassandra/message.go`:
- Around line 59-61: The Cassandra quoted-parent message model is now missing
the new tshow field in its UDT mirrors, so update the QuotedParentMessage
definitions to stay aligned with message.go’s TShow mapping. Add the tshow
BOOLEAN field to the QuotedParentMessage schema in both the documentation mirror
and the Cassandra init UDT, keeping the field names and ordering consistent with
the existing quoted message structure.

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

Review profile: CHILL

Plan: Pro

Run ID: 6eb73fd9-3c79-47dd-9e72-65c4ef045396

📥 Commits

Reviewing files that changed from the base of the PR and between ad6f8fc and bf4c348.

📒 Files selected for processing (6)

• docs/client-api.md
• message-gatekeeper/fetcher_history.go
• message-gatekeeper/handler.go
• message-gatekeeper/handler_test.go
• pkg/model/cassandra/message.go
• pkg/model/cassandra/message_test.go

[mliu33]

Can you check if we store tshow in quotedParentMsg in current behavior ?

[chenjr0719]

Good question — checked on current `main`: no, `tshow` is not stored in the `quoted_parent_message` snapshot. The UDT has no `tshow` field, and nothing writes one into it.

What is persisted is the message's own `tshow` — the `tshow` column on `messages_by_id` (`Message.TShow cql:"tshow"`). So this PR adds `TShow` to the quote projection as transient (`cql:"-"`, never persisted) and populates it per-request from history's by-id reply (fetcher_history.go:105 ← parent.TShow, i.e. the parent row's own persisted flag), not from the stored snapshot. That keeps it schema/UDT-change-free and migration-free — the snapshot stays as-is.

[chenjr0719]

Checked — no, current (main) behavior does not store tshow on the quoted-parent snapshot, and this PR doesn't persist it either:

• On main, neither quotedParentProjection nor the cassandra.QuotedParentMessage UDT has a tshow field. The only tshow-related data captured on the parent snapshot is thread_parent_id / thread_parent_created_at (set by message-worker when the quoted message is a TShow reply), which history-service's access-window redaction reads together with the outer message's own m.TShow (history-service/internal/service/utils.go quoteInaccessible).
• The TShow field this PR adds to QuotedParentMessage is tagged cql:"-" — it is JSON-only, not written to the quoted_parent_message UDT column, so there's no schema/migration change. It's populated transiently at quote-resolution time from the live parent row (FetchQuotedParent reads it via history-service GetMessageByID, whose reply is a full cassandra.Message and already carries tshow) and consumed only in-process by the gatekeeper carve-out in resolveQuoteSnapshot (tshowChannelQuote := snap.TShow && newMessageThreadID == "" && snap.RoomID == newMessageRoomID).

So the flag is authoritative from the live parent at send time; nothing new is stored, and on a later read-back of the persisted snapshot TShow is simply absent (false), which no read path depends on. Happy to drop the field from the UDT struct and thread it through a local var instead if you'd prefer it not live on QuotedParentMessage at all.