Review desk second pass: one way to ask a person, and every scenario lands on the desk

Aperta
#2,488 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

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
10/100
Tipo di issue
Funzionalità
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
github, typescript

Direzione di ricerca

This is a design issue with five parallel tracks, so start by reading the Work tracks section and choosing one, such as the Automation prompt rule in system-prompt-rules.ts. Track 1 merges requestUserInput into addReviewItem and removes chat/slack/user-input-state.ts. Done means every question goes through junior_review_items and the old path is gone, but the scope and open decisions should be confirmed with maintainers before writing code.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

feature

Follow-up to #2401, which shipped the review desk. This issue tracks the second pass. It replaces #2187.

Decisions

  • Any request for input from a person goes on the desk (A1, A3).
  • A Junior pull request that is ready to merge goes on the desk (C1).
  • "Changes requested" does not go on the desk (C2). The agent does not need the person to do something.
  • Account connections (OAuth) do not go on the desk for now (B1).
  • Guardian confirmations stay out of this pass (A2, A4). #2490 is closed. See "Two goals" below.
  • The agent never asks a person in plain text. Every question is a Review item (A1, A5).
  • Ask only when the person can give what is missing: access that they control, or a decision that is truly theirs. When only part of the work is blocked, deliver the rest and ask about the blocked part. A scope choice that does not block the work is not a question: choose the conventional option and state the choice in the reply. Offers and follow-up ideas never become items (#2504).
  • Routine upkeep of a resource that Junior owns does not ask a person. A tool that proves the ownership in code and has only reversible effects skips Guardian review (#2516).
  • "Waiting on you" lives in the same junior_review_items table as decisions. There is no separate wait record. See "Lifecycle" below.
  • No wait kind for now. "Waiting on you" comes from open action items: an open action item, assigned to the User, linked to the Conversation (#2503). The only case that open action items do not cover is an unanswered Guardian ask, and Guardian is deferred. If a later case needs waits that close on their own, add a separate field for the source. Do not add a third kind.
  • Only the assignees of an item can act on it. Do not let any reader take a public item for now. An answer can carry the authority of the person who gives it (for example, approval for a blocked child agent), so a change to who can answer is a permission change. Revisit it when a real item goes stale.
Two goals

The desk has two goals. They are similar, but they are not the same:

  • Decisions: a question that is truly the person's to answer. The desk captures the question and the answer, so the answer stays on record.
  • Waiting on you: a Conversation that cannot continue until the person acts. A Guardian permission ask is this kind. It should only notify the person after it has waited for some time, not at once.

Problem

This is the state at the start of this pass. #2491 fixed the first three points, and #2492 fixes the last one. The Guardian point is deferred.

The review desk exists, but most work that needs a person does not reach it:

  • Junior has two ways to ask a person: requestUserInput and addReviewItem with kind: action. They use the same request shape (userInputRequestSchema) and a similar Slack card. But requestUserInput stores its open request in chat/slack/user-input-state.ts (one per thread, 30-day TTL). It has no assignee, no desk entry, and no record of who answered.
  • The system prompt does not tell the agent about the review desk. addReviewItem is a deferred tool in the review-desk source.
  • AUTOMATION_RUN_RULES in system-prompt-rules.ts says "Do not ask questions, ask for approval, or offer options." Automation runs are the main async case for the desk, but this rule pushes against it.
  • A Guardian ask decision (guardian/gate.ts) makes the model write a free-text confirmation question. There is no structured item.
  • Nothing adds an item when a Junior pull request is ready to merge. The plugin API has no way for a plugin to add or close a Review item.

Direction

Map each scenario, then pick how its item gets onto the desk. Scenarios can use different paths:

  • Harness: core adds the item deterministically.
  • Plugin: a provider plugin adds the item from an event.
  • Model: the agent decides. Core makes sure the item lands.
  • Derived: shown from other state. Not stored as an item.
One tool to ask a person

Use addReviewItem for every question. Remove requestUserInput.

  • The assignee defaults to the Actor. Named assignees stay optional.
  • Load the tool in every Turn. Do not keep it deferred.
  • Use one waiting rule: if the next step needs the answer, end the Turn and let the card be the reply. If other work does not depend on it, continue.
  • A reply typed in the thread closes the open item as "replied in thread". Quick questions in a chat then go to the archive and do not crowd the desk.
  • Delete request-user-input.ts, slack/user-input-state.ts, and the Slack user-input card path. This is a hard cutover.
  • The tool description must say plainly that this is how the agent asks a person anything. Models reach for "ask the user" more readily than "add a review item". An eval must check this.
  • Use the same bar as Claude Code's AskUserQuestion: ask only for a decision that is truly the person's. Make the question stand alone. Put the recommended option first.

Scenarios

Decisions (action items)
  • A1. The agent needs a decision in a Conversation. Path: Model, through the single tool above. Assignee: the Actor. Closes on an answer, a dismiss, or a reply in the thread.
  • A2. Guardian asks to confirm a risky write. Deferred. Guardian keeps its plain-text confirmation in the thread, as on main. #2490 made the item itself the approval, and that added too many rules. Revisit this as a "waiting on you" case (D1), not as a decision.
  • A3. An Automation run needs a judgment. Path: Model. AUTOMATION_RUN_RULES says: ask a person with addReviewItem only for a decision that is truly theirs and blocks the work (#2504). The tool description owns the waiting rule, so other work that does not need the answer continues. Assignee: the people the instruction names, or else the Automation creator.
  • A4. Guardian asks during an Automation run. Deferred with A2.
  • A5. A child agent is blocked on an approval. The child calls reportBlocked, as today. The parent must add one action item (A1, L1). It must not ask in plain text. #2504 changes the three prompt texts that told the agent to ask in plain text: agent-invocations/result.ts (blocked child result), guardian/gate.ts (child Guardian denial), and the drafting rule in system-prompt-rules.ts. Routine upkeep of a Junior pull request no longer blocks a child agent at all (#2516).
Access and setup
  • B1. An account connection (OAuth) is needed. Not on the desk for now. The link stays a private delivery, as today.
  • B2. A blocked Automation. Path: Derived. No change.
Waiting on you
  • D1. An idle Conversation waits on a person. Path: Derived. The "Waiting on you" marker shows when the Conversation has an open action item assigned to the User (#2503). A question closes on an answer, a dismiss, or a reply in the thread. A generic wait kind with its own delay is deferred with Guardian (A2), because Guardian is its only case today.
Notices
  • C1. A Junior pull request is ready to merge. Path: Plugin. The GitHub plugin adds a notice when a Junior pull request is open, not a draft, approved (each reviewer's latest review: at least one approval, no change requests), and GitHub's merge state is clean. clean covers required checks, required reviews and conflicts. A notice does not mark its Conversation as "Waiting on you". Plugin dedupe keys use a reserved plugin: prefix, so an agent cannot change or close them. Dedupe on the pull request. Assignee: the requester from the originating Conversation. The plugin closes the item when the pull request merges or closes, or when it is no longer ready to merge.
  • C2. Someone requests changes on a Junior pull request. Not on the desk. PR maintenance handles the feedback, and the requester gets a Slack mention as today.
  • C3. An agent finds work for a person outside Junior (for example, a CVE match). Path: Model, through a skill. This should be rare. Dedupe on the subject.
Not on the desk
  • Progress updates and "done" receipts.
  • Failures that need no action from a person.
  • Questions from child agents. These go to the parent.

Authority model

Keep these four things apart:

  • Principal: the person that the work is for. There is one for each tree of Runs. A child agent inherits the Actor of its parent Run, so it uses the access of that person. A child agent is not a system Actor.
  • Author: each message records who wrote it: a person, Junior, a child agent, or an event. A child agent result is an event.
  • Authority: only messages from the principal can approve an action. A message counts when its stored author Identity is one of the Identities of the principal's User (#2505). A desk answer from the web Identity of a person counts in a Slack thread for the same reason. Messages from other people, events, parent task text and agent text never approve.
  • Asking: only a root Run asks a person. A blocked child agent reports to its parent, and the parent adds one item (L1).

Guardian reads the root Conversation of a child agent only to find the messages of the principal. The root is not a special message.

Shared rules

  • One open item per subject (dedupe_key). A repeat updates the item.
  • An item closes through an answer, a dismiss, a reply in the thread (A1), its source event (C1), or an agent. It also closes when its Conversation is purged or its Automation is deleted (#2506). A purge hard-deletes private items, including their answers (#2477). This is the one exception to "the answer stays on record", because privacy wins.
  • Record how each item closed, and send that to the linked Conversation.
  • A trigger, or the agent's own text, is never approval.
  • C1 needs a small provider-neutral contract in junior-plugin-api to add, update, and close a Review item. GitHub behavior stays in packages/junior-github.

Lifecycle

All desk entries are rows in junior_review_items. Each kind owns its lifecycle in code. The desk does not own timers.

  • Columns: use the existing kind (action or notice) and created_at. Do not add expires_at or visible_at columns, and do not add a wait kind (see Decisions). If a later case needs its own delay or expiry, make it a rule in code, computed from created_at.
  • Visibility: every item shows at once.
  • Turn start: a new Turn does not close items. A decision stays open, because an unrelated Turn must not drop an open question. A reply in the thread already closes a question (A1).
  • Source gone: purge and Automation delete close the items (#2506). A plugin closes its own notices (#2492).
  • Later, not built: a prompt list of the open items that a Conversation added, so the agent can close stale ones. A sweep for items that expire. Neither is needed until an item kind expires or items go stale in practice.
  • Retention: not decided. Leave it for later.

Work tracks

These tracks run in parallel. Each track is one pull request.

  1. One tool to ask a person (A1, A3). Done in #2491. #2494 saves a reply in the thread as the answer.
  2. Guardian confirmation as an item (A2, A4). Dropped for now. #2490 is closed unmerged.
  3. Ready to merge (C1). Done in #2492.
  4. No plain-text questions (A5, L1). #2504. Only blocking questions become items. Scope choices are stated, not asked.
  5. Generic waits (D1). Deferred with Guardian. See Decisions.
  6. One "needs you" rule. Done in #2503. The count, the marker and the empty state use one rule.
  7. Items that stay open forever. Done in #2506. Purge, Automation delete and subscribed thread replies close items.
  8. One authority rule. #2505. A message approves only when its author Identity belongs to the principal's User. The local CLI keeps its old match, because CLI messages use the system Identity.
  9. No asks for routine upkeep (#2438). #2516. The feedback and resolve-thread tools prove in code that the pull request is Junior's own, and skip review.
  10. Reaction-only reply. #2515. The addReaction description says that the reaction can be the whole reply. Not a desk change, but it caused the eval failures on the prompt PRs.

Lessons from dcramer/terra

dcramer/terra runs its own review desk: decision D422, the review-desk skill and tools/review-desk/. It is a private claude.ai page for one developer. Agents write question and review items to its database. Most of its rules are already in the addReviewItem description: a question stands alone, and the recommended option comes first. These lessons are new:

  • L1. A blocked child agent becomes one desk item. In terra, a child agent never asks in chat. It ends its report with one FOR REVIEW: line per item, and the parent adds the items to the desk. Junior child agents already report to the parent with reportBlocked, but the parent then asks in chat. The Slack thread for this issue shows the problem: the parent asked for the same approval of bot-comment writes four times. Rule: when a child agent is blocked on a person, the parent adds one action item. Its dedupe key comes from the blocked action, so a repeat request updates the same item.
  • L2. Record what the agent did with an answer. After the agent acts on an answer, terra adds Picked up: <what I did> to the note of the item. Junior records who answered and the answer, but not what happened next. Add one outcome line to the closed item, so an archived item shows the full story.
  • L3. Keep answers as wake-ups. Terra answers do not reach the agent, so it needs a hook (tools/hooks/desk-answers.ts). The hook exists because agents missed answers. In Junior, an answer is a message that wakes the Conversation. Keep this behavior.
  • L4. Review items with images. Terra has a review kind. It carries stills and films for the developer to judge by eye. Junior items cannot hold images. Visual QA, such as the screenshots for #2487, fits this case.
  • L5. Every item says what the person must do. Terra requires an ask line on each item, shown as "What I need from you". Junior questions already do this. Notices do not. For example, the ready-to-merge notice (C1) should say "Merge it, or say why not."
  • L6. Every answer stays on its item. Terra writes a settled answer to a decision file, with who decided and when. For Junior, the review item row is the record: it keeps the question, the answer, who answered and when. Card and desk answers already save answer. #2494 also saves a reply in the thread as the answer. A later knowledge base can read these rows. Search over archived items covers lookup now, which agrees with #2487.

Not a fit: terra gives each developer a separate desk and shares nothing. The Junior desk is shared on purpose.

Related

  • #2401: the first review desk (closed)
  • #2487: replace the review desk filters with one search across open and archived items (merged)
  • #2187: user input as a first-class interaction (closed in favor of this issue). The structured request shape it asked for shipped with userInputRequestSchema. A1 answers its open questions about automations and about whether to replace requestUserInput.
  • #2105: coordinating child conversations
  • #2490: Guardian confirmation as an item (closed unmerged)
  • #2494: save a reply in the thread as the item answer
  • #2503: one "needs you" rule
  • #2506: close items whose source is gone
  • #2438: Guardian asks child agents before PR feedback upkeep (fixed by #2516)
  • #2504: only blocking questions become items
  • #2505: one authority rule for root messages
  • #2515: the reaction can be the whole reply
  • #2516: skip review for owned PR feedback upkeep

via @dcramer.

--

View Session [Sentry]

Lingua principale
TypeScript
Stelle
385
Fork
42
Merge medio
10h 4m
PR unite (30g)
361

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di getsentry/junior

Tutte le issue di getsentry/junior

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.