Skip to content

fix(agent-invocations): Count a linked desk answer as the user's approval - #2505

Merged
dcramer merged 4 commits into
mainfrom
fix/desk-answer-root-authority
Nov 2, 2026
Merged

dcramer merged 4 commits into
mainfrom
fix/desk-answer-root-authority

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Part of #2488 (A5, L1). Companion to #2504.

A review desk answer to a Slack thread arrives in the root Conversation as a web Message from the person's web Identity (review-items/messages.ts, webActorFromEmail). readRootConversation compared its author dashboard:<hash> with the child's Slack Actor and labeled it other person. So a child's action review never accepted a desk answer as approval, and the blocked-child flow in #2504 worked only with a click on the Slack card.

One rule. readRootConversation reads the Run Actor's person once (readActorIdentity). A root message is user only when its stored author Identity is one of that person's Identities: every Identity of their User, or only the Actor's own Identity when it has no User. The match on meta.author for Slack and web messages is removed. A message without a stored author Identity is other person (fail closed). The automation label for an Automation run's own instruction does not change.

Links that the code trusts. It trusts only these links. Nothing is matched by name, email, or display data at read time.

  1. The stored author Identity of the message event: authorIdentityId, the actor_identity_id column. Slack ingress (slack-webhook.ts → resolveActorIdentityId) and web ingress (appendAndEnqueueWebMessage → resolveActorIdentityId) set it. The event store writes it at append (resolveEventActorIdentityId), and it lifts it into the payload at read (sql/history.ts eventFromRow).
  2. The Identities of the Run Actor's User (user.identities). An Identity gets a User only through a verified email, and upsertIdentityRecord rejects a link that conflicts with an existing one.

Rows without an author Identity. I checked where stored human messages can lack one. They now read as other person:

  • Rows from before migration 0026, when the backfill found no matching Identity for the author.
  • A Slack message written without ingress binding, where the Conversation has no known workspace and no Slack Identity existed yet. resolveEventActorIdentityId only looks up in that case.

Slack and web messages from current ingress always have one.

Local CLI exception. The local runner has no per-person Identity. It writes CLI messages with the system Identity (local/runner.ts), and events use that Identity too. An Identity match cannot tell the CLI person apart from system work. So a local message is still user when its author is the same local Actor. Without this, local pnpm cli -- chat runs would lose user authority in action review.

Tests (agent-invocation-user-approval.test.ts): the test helper now writes the author Identity as Slack ingress does. The cases are:

  • A desk answer from the Identity linked to the Slack user's User is user.
  • A desk answer from another person is other person.
  • A message with the user's id in meta.author but no stored Identity is other person.
  • A local CLI message is user for the local Actor and other person for a Slack Actor.
  • The Automation instruction stays automation.

via @dcramer.

--

View Session [Sentry]

@vercel

vercel Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
junior-docs Ready Ready Preview Nov 2, 2026 12:13am UTC

Request Review

@dcramer dcramer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like we're duplicating abstractions - is there not a simpler way to address this that's more aligned with our architecture

sentry-junior Bot and others added 3 commits November 1, 2026 23:58
…oval

A review desk answer to a Slack thread arrives as a web Message from the
person's web Identity. Root Conversation reads compared it with the child's
Slack Actor and labeled it as another person, so a child's action review
could not accept the answer.

A web message is now labeled user when its stored author Identity links to
the same User as the Run's Actor. Only the stored Identity-to-User links
count. A message without a linked User stays other person.

Part of #2488

Co-Authored-By: David Cramer <[email protected]>

Co-Authored-By: David Cramer <[email protected]>
Drop the web-only special case and the per-message Identity reads. Read the
Run Actor's User once and label a root message as the user when its stored
author Identity is one of that User's Identities. This uses the existing
User and Identity model instead of a parallel matching path.

Co-Authored-By: David Cramer <[email protected]>
Remove the meta.author Actor match for Slack and web messages. A root
message is the user only when its stored author Identity is one of the
Identities of the Run Actor's person. A message without one fails closed as
other person. The local CLI keeps its Actor match because its messages use
the system Identity. The Automation instruction label does not change.

Co-Authored-By: David Cramer <[email protected]>

Co-Authored-By: David Cramer <[email protected]>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9c53eb5. Configure here.

@dcramer
dcramer merged commit 198cbbc into main Nov 2, 2026
44 checks passed
@dcramer
dcramer deleted the fix/desk-answer-root-authority branch November 2, 2026 00:17
dcramer added a commit that referenced this pull request Nov 2, 2026
…n item (#2504)

Part of #2488 (track 4: A5, L1).

Prompt texts told the agent to ask a person in plain text. They now ask
with `addReviewItem`, and only when the person can give what is missing.
A non-blocking scope choice does not go on the desk. Since #2503, each
open action item that is assigned to a person marks the Conversation
"Waiting on you". So the desk must hold only decisions that need the
person.

**The rule (one owner each, per `policies/agent-steering.md`):**

- **When to ask: the task execution rule** (`system-prompt-rules.ts`):
> Ask the user a question only when they can give what is missing:
access that they control, or a decision that is truly theirs and blocks
safe progress. Ask it with `addReviewItem`, never in plain text. When
only part of the work is blocked, deliver the rest in this Turn and ask
about the blocked part with `addReviewItem`. When nobody in the
Conversation can give what is missing, such as another person's calendar
that returns notFound, state the gap and add no item. A scope choice
that does not block the work is not a question: choose the conventional
option, deliver the work, and state the choice in your reply. Do not add
an item for a scope choice, an offer, or a follow-up idea.

The existing rule against ending with an offer stays as it is. Partial
delivery and the ask go together: a blocked part never stops the rest of
the work, and a blocked part that the person can unblock still goes on
the desk (Bugbot). A gap that nobody in the Conversation can fill, such
as one attendee's calendar that returns `notFound`, is stated in the
reply with no item. The Google Calendar integration eval covers that
case.
- **How to ask and when to wait: the `addReviewItem` description**
(`tools/review-items.ts`). It drops its own "ask only for a decision
that is truly the person's" sentence, because the system prompt owns
that now.
- **Automation run rule** (`system-prompt-rules.ts`): it now reads "Ask
a person with `addReviewItem` only for a decision that is truly theirs
and blocks the work." It drops "with a recommended default, then finish
the run". The tool description owns that (recommended option first, and
finish only when the next step needs the answer).

**Other prompt changes:**

- **Blocked child result** (`agent-invocations/result.ts`): when only a
person can give what the child needs, such as approval, the parent adds
one action item that names the exact action. Its `dedupe_key` comes from
the child conversation and the blocked action, so a repeat block updates
the same item. Two rules stay: the parent's own text is not approval,
and after the answer the parent tells the child to try again with
`messageAgent`.
- **Child Guardian denial** (`guardian/gate.ts`): only the wording about
how the parent asks changes. Guardian decisions and
`ACTION_CONFIRMATION_INSTRUCTIONS` (A2, deferred) do not change.
- **Drafting rule** (`system-prompt-rules.ts`): "ask one focused
question" now names `addReviewItem`.
- **Agent invocations guidance** (`agent-invocations/AGENTS.md`): a
delivery rule says that when Guardian blocks a child action, the parent
adds one action item with `addReviewItem` instead of a plain-text
question, deduped by child Conversation and action. (`main` replaced the
old `README.md` with this `AGENTS.md`; the README edit moved here.)

**Evals** (`asking.eval.ts`):

- A blocked child result makes the parent call `addReviewItem`, and the
reply text of the result Turn has no question.
- A non-blocking scope choice (a list-of-dicts to CSV function) adds no
`addReviewItem` item. The reply gives the function and states a choice
it made, and has no ask or offer.

The blocked-child integration eval and the child Guardian approval eval
accept the card-only `no_reply` result.

**Card answers and child approval.** A click on the Slack card in the
root thread reaches the root Conversation as Slack input from the person
who clicked. When that person is the child's Actor, the child's action
review reads it as `user`. A desk answer on the dashboard to a Slack
root arrives as a web Message. On `main` it reads as `other person`.
#2505 fixes that.

<!-- junior-request-attribution:start -->
via @dcramer.
<!-- junior-request-attribution:end -->

<!-- junior-session-footer:start -->
<!-- junior-conversation-id:agent%3A45b85781e491a1a70c491c85bc40ba09 -->

--

[View
Session](https://junior-prod.sentry.dev/conversations/agent%3A45b85781e491a1a70c491c85bc40ba09)
[[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A45b85781e491a1a70c491c85bc40ba09/?project=4510944073809921)

<!-- junior-session-footer:end -->

---------

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Co-authored-by: David Cramer <[email protected]>
Co-authored-by: David Cramer <[email protected]>

This branch was successfully deployed

1 active deployment
Preview – junior-docs — 07489e13 Deployed Nov 2, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant