Store visibility on each Conversation

オープン
#2,380 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
5/5
見積もり時間
1週間以上
初心者へのやさしさ
22/100
issue の種類
機能追加
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
sql, typescript

調査の方向性

Start with chat/conversations/sql/store.ts and the existing tests/component/conversation-sql-store.test.ts scenario named “persists visibility from source signals...”; the issue describes the proposed storage rule and the reader cutover across many files. Follow the listed slices and migrations, then run the component and integration scenarios named in the test plan. Done means root Conversations retain their own visibility, readers use it, and channel privacy changes cascade as specified.

索引モデルが issue の本文から書いたものです。

説明

Step 4 of #2207. It must land before step 5 (GitHub webhook). Step 2 is #2217.

Problem

Conversation visibility is not stored on the Conversation. Readers get it from the shared Destination row of the Slack channel (junior_destinations.visibility). All Conversations in one channel or DM share that one value.

Each write with a visibility signal replaces that value. recordActivity and recordExecution call upsertDestination with refreshVisibility. Dispatches pass destinationVisibility. Slack messages pass the channel type.

This blocks the #2207 use case: work that starts from a public source, such as a comment on a public repository, and runs in the person's private DM.

  • If a dispatch marks the DM public, the DM Destination row becomes public.
  • Then every other Conversation in that DM shows as public. The dashboard shows their transcript and tool data (canExposeConversationPayload). Search, Briefs, and retention also use the public rules.
  • The next normal DM message sets the row back to private. The value keeps changing between the two.

Proposal

Store visibility on each root Conversation. The Destination row keeps the visibility of the Slack channel only.

Storage
  • Add a nullable visibility column to junior_conversations. Values: public, private. NULL means not known yet.
  • Root Conversations own the value. A child Conversation reads the value of its root, as today (root_conversation_id).
  • junior_destinations.visibility stays. It is a fact about the Slack channel. Channel tools (channel-access.ts), Automations, and the Location pages that decide if a Location is named use it. It is no longer the visibility of a Conversation.
Write rule

A Conversation may become more private. It never becomes more public by itself.

Stored Signal Result
NULL public or private the signal
NULL none the current Destination value if it is public or private, else NULL
public private private
public public or none public
private any private
  • One SQL upsert in SqlStore.upsertConversation owns this rule. Other code does not decide it.
  • Why: when the person replies in a public-origin DM Conversation, the reply can contain private data. The Conversation becomes private. A later public event in the same thread must not make it public again.
  • The only path from private to public is an explicit action by a participant, such as the "make public" action in #1545. That action writes the Conversation column, not the Destination row.
  • When a Destination changes from public to private (a Slack channel is converted to private), the same write sets all public Conversations at that Destination to private. This keeps the current protection for converted channels.
  • A Destination that changes from private to public does not change its old Conversations. They stay private. New Conversations there start public. Today they all become public. This is a deliberate fail-closed change.
Dispatch input

Today one dispatch field, destinationVisibility, sets three things: the Destination row, the Conversation, and the Slack thread channelVisibility in slack/dispatch-turn.ts. For a DM these must differ.

  • Keep destinationVisibility as the visibility of the Slack channel. Core sets it to private for a DM.
  • Add an optional conversationVisibility to the dispatch record. It defaults to destinationVisibility. It writes only the Conversation column.
  • dispatchForIdentity (step 2, #2217) stays private-only now. In step 5, it accepts the origin visibility from the plugin, for example the visibility of the GitHub repository. It passes that as conversationVisibility. destinationVisibility stays private.
  • recordActivity and recordExecution take the two values as separate inputs. A dispatch must not write a public value to a DM Destination row.
Fail closed
  • NULL, unknown, or invalid values read as private. resolveConversationPrivacy already does this and logs conversation.visibility.defaulted.
  • Missing or invalid root metadata stays private, as today in privacy.ts and access.ts.
  • Readers do not fall back to the Destination value after the reader cutover. A fallback would bring back the leak.

Migration and backfill

  1. Add the column. In the same migration, copy public or private from the Destination row to each root Conversation that has a destination_id. Leave other values NULL. This is correct now, because no deployed writer can make a DM public yet.
  2. Old workers that run during a deploy do not write the column. Their new rows stay NULL. They read as private, and the next write from a new worker fills them by the rule above.
  3. Before the reader cutover, run the same copy again for rows that are still NULL.

Readers that must change

Each reader below gets visibility from the Destination row of the root Conversation. Each must read the column of the root Conversation instead:

  • chat/conversations/sql/store.ts: conversationFromRow (Conversation.visibility). The agent Turn privacy (chat/agent/index.ts), Briefs input, forks, plugin tasks, resume, and searchConversationEvents use this value.
  • chat/conversations/sql/privacy.ts: resolveRootVisibility (Brief storage and tasks, retention).
  • api/conversations/access.ts: readConversationAccessFromSql (dashboard payload access).
  • api/conversations/list.ts: feed filters and Brief search.
  • api/conversations/stats.query.ts and api/people/profile.query.ts: private titles and counts.
  • api/locations/query.ts: Conversation rows and counts at a Location. The Location itself can still use the Destination value to decide if it is named.
  • chat/conversations/sql/message-search.ts and chat/briefs/search.ts: public cross-Conversation search.
  • chat/conversations/sql/purge.ts: public and private retention windows.
  • policies/data-redaction.md and chat/conversations/README.md: they say that Destination visibility is the privacy authority. Update them.
  • Rename ConversationPrivacy to ConversationVisibility after the cutover, per the TODO in conversation-privacy.ts.

Readers that stay on the Destination value: slack/tool-support/channel-access.ts, automations/read.ts, automations/collection.ts, and resolveDestinationVisibility (it supplies the channel signal for a Slack message).

Slices

  1. Column, migration with backfill, and the write rule in upsertConversation. Conversation.visibility reads the column. Dashboard SQL readers do not change yet. Visible change: an old Conversation stays private in the runtime after its channel becomes public. PR: #2382.
  2. Second backfill for NULL rows. The cascade for channels that become private. Hard cutover of all SQL readers above. Docs and policy update. The cascade writes rows of other Conversations, so it must keep one lock order (Destination row, then Conversation rows).
  3. Split conversationVisibility from destinationVisibility on dispatches and store writes.
  4. Then step 5 of #2207 can pass origin visibility.

Out of scope

  • The reverse case: a private DM run writes to a public issue or PR. Step 3 (#773) and the "no private data in public output" requirement on #2207 track it.
  • Which data a public-origin Conversation may use. In a public-origin Conversation, the person's own credentials can read private data. Tool results then appear in a public transcript. Step 5 must decide if a public-origin Conversation starts private until this is addressed. See open questions.
  • A UI to change visibility (#1545, #1200).

Test plan

  • Extend the primary owning scenario in tests/component/conversation-sql-store.test.ts ("persists visibility from source signals..."). Cover: a signal fills NULL; public → private; no change from private → public; a channel converted to private makes its Conversations private; two Conversations in one DM keep different values.
  • Slice 2: extend the existing dashboard access and feed scenarios (tests/integration/api/conversations/*, tests/integration/dashboard-reporting.test.ts) with one Conversation that differs from its Destination.
  • Slice 3 and step 5: extend tests/integration/identity-dispatch.test.ts (#2217). A public-origin dispatch in a DM shows its own transcript as public. Another Conversation in the same DM stays private.
  • No tests for migration SQL. Use pnpm --filter @sentry/junior db:generate and the migration metadata checks.

Open questions

  1. Is the rule "more private only, except explicit publish" correct? The visible change is that old Conversations stay private after a channel becomes public.
  2. Should a public-origin DM Conversation become private at the first reply by the person (current proposal), or only when private data enters by another path?
  3. Should step 5 start public-origin Conversations as private until the credential question above has an answer?

via @BYK.

--

View Junior Session [Sentry]

主要言語
TypeScript
スター
385
フォーク
42
平均マージ
9時間 38分
マージ済み PR(30日)
408

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

getsentry/junior のほかの issue

getsentry/junior の issue をすべて見る

似ている issue

TypeScript の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。