Repository navigation
feat(prompt): Ask only blocking questions, and ask them with an action item - #2504
Merged
Merged
Conversation
A blocked child result, the child Guardian denial, and two system prompt rules still told the agent to ask a person in plain text. Each now names addReviewItem. A blocked child becomes one action item whose dedupe key comes from the child and the blocked action, and a non-blocking question becomes an action item while the work continues. Part of #2488 Co-Authored-By: David Cramer <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The parent now asks for missing person-only input with an action item, so the result Turn can end with only the card. The integration case keeps its contract: one result Turn after the block. asking.eval.ts owns how the parent asks. Co-Authored-By: David Cramer <[email protected]>
…estions Co-Authored-By: David Cramer <[email protected]>
… reply The reaction-only integration eval failed in 4 of 5 live runs on this branch. The model added the reaction, then posted a confirmation. The new wording "then deliver the requested work in this Turn" can read as a push to always reply, which conflicts with the NO_REPLY rule. Say "continue the work" instead. Co-Authored-By: David Cramer <[email protected]>
… off the desk Line 31 told the agent to add every open scope or follow-up question as an action item. Open action items now mark the person as waited on, so each "want me to also..." would flag the Conversation. Ask only for missing access or a blocking decision that is the person's. For a scope choice that does not block, choose the conventional option, deliver, and state it. Give each rule one owner. The system prompt owns when to ask. The addReviewItem description owns how to ask and when to wait, so drop its "when" sentence and the duplicate default and finish wording in the Automation run rule. Add an eval case for a non-blocking scope choice. Co-Authored-By: David Cramer <[email protected]>
Missing access to one part, such as one attendee's calendar, made the agent stop and ask with an item, so it gave no meeting time. Ask only when the work cannot be delivered without the answer. Otherwise deliver what is possible and state what could not be done. Co-Authored-By: David Cramer <[email protected]>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b5f2bb6. Configure here.
Partial delivery could leave a missing-access or person-owned decision off the desk whenever another part of the request was deliverable. Deliver the part that works, then ask for the blocked part with addReviewItem.
Ask only when the person can give what is missing. When part of the work is blocked, deliver the rest in the same 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. Co-Authored-By: David Cramer <[email protected]>
…estions # Conflicts: # packages/junior/src/chat/agent-invocations/README.md Co-Authored-By: David Cramer <[email protected]>
Contributor
|
3 screenshot changes — 3 changed · 0 added · 0 removed Review screenshots in Frameshift
|
dcramer
added a commit
that referenced
this pull request
Nov 2, 2026
…oval (#2505) 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`. <!-- 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]>
dcramer
approved these changes
Nov 2, 2026
| }) => { | ||
| const conversation = await run( | ||
| slackMention( | ||
| 'Use spawnAgent to start one child agent with the task "Write the deploy command for the deploy region that the user chose. The region is not in this task." When its result arrives, get what it needs so it can finish.', |
sentry-junior Bot
added a commit
that referenced
this pull request
Nov 2, 2026
The out-of-office asking eval failed on the #2504 head and on this branch. The agent wrote a reply without the vacation dates and asked nothing, because it read the dates as a scope choice. A fact that only the person knows is now a reason to ask, and the agent must not drop it or guess it. Co-Authored-By: David Cramer <[email protected]>
sentry-junior Bot
added a commit
that referenced
this pull request
Nov 2, 2026
The drafting rule allowed a placeholder or a question. So for a draft that needs a fact that only the person knows, the agent put in a placeholder and asked nothing. The out-of-office asking eval failed on the #2504 head and on this branch for this reason. Now the agent puts in the placeholder and asks for the facts with one addReviewItem item. Co-Authored-By: David Cramer <[email protected]>
sentry-junior Bot
added a commit
that referenced
this pull request
Nov 2, 2026
The case named spawnAgent, gave the child task text, and told the parent what to do with the result. Now Maya asks to hand off a deploy command for a region that only she knows. The assertions do not change. Follow-up to #2504, part of #2488 Co-Authored-By: David Cramer <[email protected]>
sentry-junior Bot
added a commit
that referenced
this pull request
Nov 2, 2026
The out-of-office asking eval failed on the #2504 head and on this branch. The agent wrote a reply without the vacation dates and asked nothing, because it read the dates as a scope choice. A fact that only the person knows is now a reason to ask, and the agent must not drop it or guess it. Co-Authored-By: David Cramer <[email protected]>
sentry-junior Bot
added a commit
that referenced
this pull request
Nov 2, 2026
The drafting rule allowed a placeholder or a question. So for a draft that needs a fact that only the person knows, the agent put in a placeholder and asked nothing. The out-of-office asking eval failed on the #2504 head and on this branch for this reason. Now the agent puts in the placeholder and asks for the facts with one addReviewItem item. Co-Authored-By: David Cramer <[email protected]>
dcramer
added a commit
that referenced
this pull request
Nov 2, 2026
…#2518) Follow-up to #2504, part of #2488 **Blocked-child case** (`asking.eval.ts`): the case was scripted. It named `spawnAgent`, wrote the child's task text ("The region is not in this task"), and told the parent "get what it needs so it can finish". Now Maya asks to hand off a deploy command for a Cloud Run region that she "picked last week". Only she knows the region. The prompt does not say how to delegate or how to ask, and it names no tool. The assertions do not change: one `spawnAgent` call, an `addReviewItem` action item in the result Turn, and no `?` in the replies of that Turn. **Out-of-office case** (`asking.eval.ts`): this case also failed on the final #2504 head. The agent wrote an out-of-office reply with no dates and asked nothing. That reply is complete, so the request did not need a fact from Maya. Maya now asks for a reply that says the day she is back, which only she knows. The assertion does not change. **Drafting rule** (`system-prompt-rules.ts`): after that request change, the agent put in a `[day/date]` placeholder and still asked nothing. The cause is the drafting rule, which said "use clear placeholders or ask one focused question", so a placeholder alone satisfied it. Now the agent puts a placeholder where a fact that only the person knows goes, and asks for those facts with one `addReviewItem` item. This matches the asking rule: deliver the rest of the work and ask about the blocked part. **Neutral prompt example** (`system-prompt-rules.ts`): the asking rule used "another person's calendar that returns notFound" as its example of a gap that nobody can fill. That is the Google Calendar integration eval scenario, and `policies/evals.md` does not allow scenario phrases in product prompts. The example is now "data that only someone outside the Conversation controls". The Google Calendar integration cases still pass. I checked the other two cases. The CSV case and the Automation case ("Check with me whether to cancel ... or move it") use requests that a person would write and name no tool. I left them unchanged. On b984016, all four `asking.eval.ts` cases pass in behavioral shard 4/4. <!-- 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]>
dcramer
added a commit
that referenced
this pull request
Nov 2, 2026
…2520) Integration evals on PRs off main with both #2514 and #2504 failed four cases. Each failure has a root cause in a tool contract, a turn input, or the eval fixture. None is a runtime double post. **Attachments: "when the vision model cannot read an image"** (0.50 in 4 of 4 runs on main; at the 0.75 floor in 9 of 13 earlier runs) The turn input said only that the image "could not be analyzed". `viewImage` said "unsupported image format; use PNG, JPEG, GIF, or WebP" for a `.png` with bad bytes. The agent then told the user that the attachment "didn't come through" and asked for a re-upload, as if no file arrived. Both messages now say that the file arrived but could not be read as an image. The case now scores 1.0. **Slack Thread Status and Title, both cases** ("expected 2 to have a length of 1") This is not a double post and not a second title. The failing assertion is `replies` (lines 27 and 42). Since #2514, a how-to answer is about 2,200–2,400 characters, which is over the 2,200-character Slack inline budget. Delivery splits it into two posts. #2514's own branch failed this way at 08f9f6a. The fixture counted each post as a reply, and the judge saw the answer three times. The fixture (`slackCallReplies` in `src/fixture/results.ts`) now treats the consecutive posts that make up one stored reply as one reply. A repeated post is still a second reply. The comparison removes the continuation marker and drops code fence lines. A split inside a code block closes the block in one post and reopens it, with its language tag, in the next. Dropping the fence lines also keeps a cut between two real code blocks matching. The fix is fixture-only, and Slack output code does not change. I checked it against the real split posts of the failing runs. Unit tests cover a plain split, a split inside a ```ts block, a cut between two code blocks, and a repeated post. **Schedule Creation: "simple one-off reminder"** (Guardian `ask`, then the agent asked to confirm) The `createAutomation` description said that the automation "delivers to the channel or DM of this conversation". Guardian believed it over the `task_creator` outcome and returned `unclear_destination`. The new Guardian snapshot reproduces this on the old description (run 41374373955: "schedules a reminder in the active public Slack channel") and allows on the new one (run 41374880826). The description now says that the automation belongs to the conversation and that its outcomes set where its messages go. A later cleanup removes the sentences that other surfaces already own. The `<tool-policy>` block owns creator credentials and subscribable tool results. The trigger `identifier` field and `searchEventTypes` own namespace-wide event reach. The cleanup also removes the provider example from core. The first invalid call (`timezone` beside `schedule`) is a model nesting slip that 8 earlier passing runs also had. The strict schema catches it, and the retry recovers. **Scheduled Delivery: the creator mention** (run 41382009284 on 60f4efe, all 3 attempts) `createAutomation` is in the tool list of every automation run. A change to its description stops the stored recordings from matching, so these cases run live. Live, a reminder for its creator often posted without the mention: "Reminder: The Automation creator, please do healthchecks.", "Reminder: Do your healthchecks.", and "Reminder: Do healthchecks.". Such a reminder posts to the channel without pinging its creator. `requireInstructionReferences` makes a stored instruction call the creator "the Automation creator". `renderTaskInput` passed that text through, and only the `Created by:` line told the run to write the mention instead. `renderTaskInput` now writes the creator mention in place of "the Automation creator" (any case, whole words). Scheduled and event automations pass a creator. Watches do not, so their input does not change. The `Created by:` line stays, because it still covers "me" and "my". `heartbeat-scheduled-automations.test.ts` now checks that a stored "the Automation creator" reaches the run as the mention in the `Instructions:` line. The test fails without the fix. All three failures were on "when a due reminder says me, mention its creator". Its legacy instruction says "me", not "the Automation creator", so this fix does not reach it. That case stays model choice. #2519, stacked on this PR, moves it to the behavioral suite. This PR does not move it. CI: pending on a35f17f. Part of #2488 <!-- junior-request-attribution:start --> via @dcramer. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3A7a1dc2757924a78a9104357ce0de2cbf --> -- [View Session](https://junior-prod.sentry.dev/conversations/agent%3A7a1dc2757924a78a9104357ce0de2cbf) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3A7a1dc2757924a78a9104357ce0de2cbf/?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]>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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):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
addReviewItemdescription (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 withaddReviewItemonly 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:
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. Itsdedupe_keycomes 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 withmessageAgent.guardian/gate.ts): only the wording about how the parent asks changes. Guardian decisions andACTION_CONFIRMATION_INSTRUCTIONS(A2, deferred) do not change.system-prompt-rules.ts): "ask one focused question" now namesaddReviewItem.agent-invocations/AGENTS.md): a delivery rule says that when Guardian blocks a child action, the parent adds one action item withaddReviewIteminstead of a plain-text question, deduped by child Conversation and action. (mainreplaced the oldREADME.mdwith thisAGENTS.md; the README edit moved here.)Evals (
asking.eval.ts):addReviewItem, and the reply text of the result Turn has no question.addReviewItemitem. 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_replyresult.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. Onmainit reads asother person. #2505 fixes that.via @dcramer.
--
View Session [Sentry]