Skip to content

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

Closed

Description

@sentry-junior

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). feat(guardian): Put Guardian asks on the review desk #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 (feat(prompt): Ask only blocking questions, and ask them with an action item #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 (fix(github): Skip review for owned PR feedback upkeep #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 (fix(review-desk): Use one rule for items that need a person #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. feat(guardian): Put Guardian asks on the review desk #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 (feat(prompt): Ask only blocking questions, and ask them with an action item #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. feat(prompt): Ask only blocking questions, and ask them with an action item #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 (fix(github): Skip review for owned PR feedback upkeep #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 (fix(review-desk): Use one rule for items that need a person #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 (fix(agent-invocations): Count a linked desk answer as the user's approval #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 (fix(review-desk): Close Review items that no agent can act on #2506). A purge hard-deletes private items, including their answers (fix(review-desk): Purge private Review items and add missing foreign keys #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 (fix(review-desk): Close Review items that no agent can act on #2506). A plugin closes its own notices (feat(review-desk): Add a ready to merge notice for Junior pull requests #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 feat(review-items): Ask with addReviewItem and close on thread reply #2491. feat(review-items): Save a thread reply as the item answer #2494 saves a reply in the thread as the answer.
  2. Guardian confirmation as an item (A2, A4). Dropped for now. feat(guardian): Put Guardian asks on the review desk #2490 is closed unmerged.
  3. Ready to merge (C1). Done in feat(review-desk): Add a ready to merge notice for Junior pull requests #2492.
  4. No plain-text questions (A5, L1). feat(prompt): Ask only blocking questions, and ask them with an action item #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 fix(review-desk): Use one rule for items that need a person #2503. The count, the marker and the empty state use one rule.
  7. Items that stay open forever. Done in fix(review-desk): Close Review items that no agent can act on #2506. Purge, Automation delete and subscribed thread replies close items.
  8. One authority rule. fix(agent-invocations): Count a linked desk answer as the user's approval #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 (Guardian asks child agents before PR feedback upkeep #2438). fix(github): Skip review for owned PR feedback upkeep #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. fix(slack): Say when a reaction is the whole 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 ref(review-desk): Replace view tabs with one search across open and archived items #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. feat(review-items): Save a thread reply as the item 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 ref(review-desk): Replace view tabs with one search across open and archived items #2487.

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

Related

via @dcramer.

--

View Session [Sentry]

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions