Store visibility on each Conversation
I maintainer di solito rispondono entro 1 giorno
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Idoneità per principianti
- 22/100
Direzione di ricerca
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.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
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 becomespublic. - 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
visibilitycolumn tojunior_conversations. Values:public,private.NULLmeans not known yet. - Root Conversations own the value. A child Conversation reads the value of its root, as today (
root_conversation_id). junior_destinations.visibilitystays. 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.upsertConversationowns 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
privatetopublicis 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
publictoprivate(a Slack channel is converted to private), the same write sets allpublicConversations at that Destination toprivate. This keeps the current protection for converted channels. - A Destination that changes from
privatetopublicdoes 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
destinationVisibilityas the visibility of the Slack channel. Core sets it toprivatefor a DM. - Add an optional
conversationVisibilityto the dispatch record. It defaults todestinationVisibility. 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 asconversationVisibility.destinationVisibilitystaysprivate.recordActivityandrecordExecutiontake 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.resolveConversationPrivacyalready does this and logsconversation.visibility.defaulted.- Missing or invalid root metadata stays private, as today in
privacy.tsandaccess.ts. - Readers do not fall back to the Destination value after the reader cutover. A fallback would bring back the leak.
Migration and backfill
- Add the column. In the same migration, copy
publicorprivatefrom the Destination row to each root Conversation that has adestination_id. Leave other valuesNULL. This is correct now, because no deployed writer can make a DM public yet. - 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. - 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, andsearchConversationEventsuse 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.tsandapi/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.tsandchat/briefs/search.ts: public cross-Conversation search.chat/conversations/sql/purge.ts: public and private retention windows.policies/data-redaction.mdandchat/conversations/README.md: they say that Destination visibility is the privacy authority. Update them.- Rename
ConversationPrivacytoConversationVisibilityafter the cutover, per the TODO inconversation-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
- Column, migration with backfill, and the write rule in
upsertConversation.Conversation.visibilityreads 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. - Second backfill for
NULLrows. 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). - Split
conversationVisibilityfromdestinationVisibilityon dispatches and store writes. - 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 fillsNULL;public→private; no change fromprivate→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:generateand the migration metadata checks.
Open questions
- Is the rule "more private only, except explicit publish" correct? The visible change is that old Conversations stay private after a channel becomes public.
- 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?
- Should step 5 start public-origin Conversations as private until the credential question above has an answer?
via @BYK.
--
- Lingua principale
- TypeScript
- Stelle
- 385
- Fork
- 42
- Merge medio
- 10h 4m
- PR unite (30g)
- 361
Preparare l'ambiente
- Include un Dockerfile o un file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di getsentry/junior
-
documentation
Difficoltà 2/5 1-3 ore Idoneità per principianti 84/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 64/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 65/100
getsentry/junior#335 · 1 reazione ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di getsentry/junior
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 62/100
oxidecomputer/skepsis#102 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 62/100
voidzero-dev/void#677 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
kunchenguid/gh-axi#247 ·
I maintainer di solito rispondono entro 1 giorno
-
bug
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
lidge-jun/opencodex#8101 · 2 commenti ·
I maintainer di solito rispondono entro 1 giorno