Resolve actor identity once and store it explicitly
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
- 20/100
- Tipo di issue
- Refactoring
- Chiarezza
- Abbastanza chiara
- Stato di attività
- Ferma
- Stack tecnologico
- github, redis, typescript
Direzione di ricerca
Start with the merged spec in openspec/changes/actor-identity-end-state/ and its design.md; chat/actor.ts is the runtime Actor entry point named in the issue. This issue coordinates several independently shipped steps, so choose a specific step and read its dependency issues before starting. Done means that step meets the spec and release rules; implementation details and tests are settled in its PR.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Junior's runtime Actor is typed (chat/actor.ts), but stored Conversation state flattens it to a Slack-only shape. Later write and read paths then guess the Identity again from platform strings, the Source, the Location, and id patterns. Each guess can drift, and they caused #2431 and its follow-up audit.
Current state
- Conversations and Redis work state store a Slack-only Actor. Web Actors lose their kind. System and local Actors are left out.
- #2443 merged the Actor-to-Identity mapping into one entry point. #2473 added the readers for the Identity id (released in 0.268.0). #2482 added the writers (merged, not released yet).
- In released code (0.268.0), the Conversation write lets the newest Identity win, so a human Turn can replace a system root's Identity. This is the root cause of #2431. #2436 works around one case. #2482 (merged) makes the write keep the first value. Until a release with #2482 runs on every worker, the damage can keep happening.
- The Conversation Identity column (
junior_conversations.actor_identity_id) does four jobs: the root that the dashboard shows, private access, spend, and the personal feed. Child rows store their own copy, and spend reads it. Two other Identity columns are always empty. - No inbound event Message has an author Identity.
- Session logs, activity metadata, Guardian, and root Conversation Message authors also guess the Identity.
End state
The source of truth is the merged spec on main: openspec/changes/actor-identity-end-state/ (#2444, updated by #2489). It matches the plan below. It fixes the target and the rules. Each step settles its own implementation details in its PR, after it re-checks the spec against current main. A change to a decided rule needs David's OK.
- Owner: the Identity that started the root Conversation: the human for Slack, web, and fork; the one system Identity for automations, plugin dispatch, and the local CLI. Set once at insert. It never transfers. An Automation has a creator, not an Owner.
- The Owner column is the existing
junior_conversations.actor_identity_id. Code and docs call it the Owner. No new column and no rename. - Children store no Owner. They read the root Owner, and child spend rolls up to it.
- One system Identity, with the fixed id
system(#2482). "Which system" is provenance (Source and #2211). Dashboard labels come from the Source and do not change to "System". - Users have many Identities. Spend, People, participants, and Locations roll up by User.
- Web users use their sign-in Identity. The mock viewer has a mock Identity. Junior stops writing
junior/<email>. - Bots are kind
botwith no User. - Every new user Message has an author Identity. Assistant output has none.
- Events are user Messages. The author is the person or bot who wrote the content, or the system Identity for a system event. Event authors are not participants.
- The event author is never the Turn Actor. Event Turns stay system-acted, and credentials come only from the Watch or Automation grant.
design.mdexplains the confused deputy risk. Junior still resolves the author to its Identity and linked User, for the Message label and the trust check. - Trust: an event author is trusted when Junior verifies them on an allowed domain (the #2260 / #2217 check) or their login is on
trustedAuthors. Trusted text counts like a chat message. Untrusted or unknown authors do not run. Plan reading: Junior drops their events before batching, so the agent never sees them. System events run as today, with the current untrusted-text handling. - Legacy data rule (plan decision): migrate an old data shape only when special code exists because of it, and only with a rule that a read-only dry run proves. Then delete that code.
- Needs migration: the stored Actor JSON (
actor_json), user Messages with no author Identity, the payload copy of the author id, the per-namescheduler/local-cli/eventsystem rows (merge intosystem, after the labels come from the Source), Owner copies on child Conversations, and the two unused columnscreator_identity_id/credential_subject_identity_id. - No migration: old
junior/<email>web Identities (valid Identities linked to the right User; a merge into the sign-in Identity is optional), and Redis and Turn checkpoint data (Junior waits for it to expire: 7-day Redis TTL; the plan waits 14 days). - Old event Messages get the system Identity as author. Plan reading of David's answer 2: old user Messages whose author cannot be proven stay with no author, with no guess. A human Message stored as system could later read as a system or automation Message in history.
- Needs migration: the stored Actor JSON (
- Owner repair (#2431) is separate (plan decision): it fixes wrong data, not a code path. A root with a system Source (
scheduler,event,local) that a human took over goes back tosystem; this is provable because the Source is set-once. A Slack or web root with a system Owner is repaired only if the dry run proves the rule. Humans who lose a root keep access as participants.
Decided by David Cramer in the Slack thread
- The Owner is the person who started the Conversation, or the system for an automation. Owners never transfer.
- There should be no unknown actors on any Message from now on.
- The local CLI user can be the system Identity.
- A web user is the User they sign in with, through the normal User and Identity tables.
- The schema should be normalized.
- Assistant output is just an event. It needs no author Identity.
- Web users always have an Identity. The mock viewer mocks one.
- Child spend rolls up to the root Owner.
- Slack bots have an Identity but no User.
- Inbound event authors: Junior finds the correct Identity and verifies it the same way as everywhere else. Junior ignores the instructions of someone without access.
- Most events are system events, with the system Identity as author. PR feedback and comments are authored by the person or bot who wrote them.
- Legacy
junior/<email>rows: leave them as they are, unless a migration path is clearly correct. - Participants: "same as today: users." Verified event authors as participants: "not right now maybe we will change our mind later."
- "also we'd allow list usernames in juniors config for github events (so we could allow list some bots)"
- "call it trusted authors or sometihng", then: "yes "trusted" is what we want here, but you can use the suffix that makes sense"
- "any sender who doesnt have the allowed domain email address or is unknown isnt trusted. bots and or other users could be listed under trusted"
- On what trusted text may do: "trusted authors are the same as normal authors... its just that untrusted authors we dont run"
- On the event author as Turn Actor: "I agree, lets not have it be the turn actor as it expands the security concerns. you need to document that (confused deputy concerns for example). knowing who it is that maps to our internal users is valuable tho so keep that"
- On splitting batches by trust and hiding untrusted entries: "untrusted events wont actually ever be seen so i think (2/3) is not relevant? system events might have untrusted text but we already handle that"
- On Cursor Bugbot: "yeah Bugbot and sentry will end up on trusted actors"
- His answers to the 14 proposals in #2444: (1) bot kind: "bots need to be annotated as bots, thats important"; (2) bypass Identity: "you can find what makes sense"; (3)
doneByfields: "yes"; (4) upsert keeps kind and user: "correct"; (5) events: "yes but an event is a message so"; (6) case-insensitive match: "yes"; (7) Owner repair: "sure defer to you but needs to be correct"; (8) Locations by User: "yep"; (9) author shape: "yep. and users have many identities associated to them"; (10) system key: "thats meaningless to me, make sure its good arch and ill trust you"; (11) Slack bot subject: "yep"; (12) mock key: "idk up to you make it correct"; (13) Automation creator: "sure, we dont really have "owners" on automations anyways"; (14) instruction record: "ok". - His answers on old data and the Owner column: (1) old event Messages get the system Identity as author: "(1) yes"; (2) old user Messages whose author cannot be proven: "(2), why not just make them system? if they dont have an author just make them without an author? should be uncommon"; (3) keep
junior_conversations.actor_identity_idas the Owner column: "(3) i guess? usually theres just like one actor.. everything else is metadata".
Handed off by David (2, 7, 10, 12): each is a current proposal in the spec that its step confirms. The Owner repair runs only for the rules that the dry run proves, after David's OK.
Resolved questions
- Mixed event batch (drop untrusted entries, or keep them as context): resolved by his "untrusted events wont actually ever be seen" answer. Plan reading: Junior drops untrusted authored events before batching. Only trusted and system entries are batched.
- Cursor Bugbot on
trustedAuthors: resolved by his "Bugbot and sentry" answer. Plan reading: he wrote "trusted actors", and the plan reads this as the GitHub plugintrustedAuthorslist. Cursor Bugbot and Sentry's GitHub bot go on that list in the deployed app config. Step 4 confirms the exact logins from real event senders. That config ships with the trust rule, or their PR feedback stops starting runs. - Old data and the Owner column: resolved by his three answers above. Plan decision: the new Owner column work (old steps 4a/4b) is removed, because #2482 makes the existing column keep its first value, which is the Owner rule.
No open questions remain.
Steps (each ships on its own)
Next: step 3.
- One Actor-to-Identity mapping: #2443 (merged).
- Carry the Identity id from ingress. Readers: #2473 (merged). Writers: #2482 (merged, not released yet). The step 8 Redis window starts when a release with #2482 runs on every worker.
- System root labels come from the Source, new system Conversations use
system, code calls the column the Owner, every reader uses the root Owner (including child spend and Locations by User), and children stop storing an Owner. Readers ship before children stop storing it. - Real Identities and trust: web uses the sign-in Identity, mock and bypass Identities, bots get the
botkind, event authors, the trust rule, andtrustedAuthorswith Bugbot and Sentry. Needs #2260. - Read-only dry run of every migration and repair rule. Post the counts on #2439, then David's OK.
- One data migration release, only for the rules the dry run proved.
- Owner repair (#2431), only for the rules the dry run proved.
- After the Redis window (14 days after step 2 runs on every worker): delete the fallbacks, require an author on new user Messages, and remove the stored Slack-shaped Actor from Conversations and Redis. Also the Guardian, session log, and dashboard author shape changes. Needs #2181.
- One release later: drop the old columns, move the durable rules to the owning READMEs, and delete the spec.
Release rules: readers before writers, safe under rolling deploys, column drops one release after the code stops using them, no production backfill without explicit approval, and a dry run before any migration or repair.
Dependencies: #2260 before 4, #2181 before 8. #2211 owns which Automation or dispatch did the work.
Not covered: keying user credentials by User, the closed Actor schema, and naming the local CLI human. Visibility rows written wrong before #2440 belong to #2382. Known gaps are listed in design.md.
Related: #2260, #2431, #2432, #2436, #2437, #2440, #2443, #2444, #2473, #2482, #2489.
via David Cramer.
--
- 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