Skip to content

send_message: steer target session by default - #172

Open
yourbuddyconner wants to merge 1 commit into
mainfrom
fix/send-message-steer-default
Open

send_message: steer target session by default#172
yourbuddyconner wants to merge 1 commit into
mainfrom
fix/send-message-steer-default

Conversation

@yourbuddyconner

Copy link
Copy Markdown
Collaborator

What

Flips the default interrupt value on the send_message sandbox tool from false (queue) to true (steer). When the orchestrator (or any session) sends a message to another session, the target's current turn is now aborted and the message is delivered immediately instead of sitting in the queue.

  • docker/opencode/tools/send_message.tsinterrupt defaults to true; description updated.
  • packages/worker/src/lib/orchestrator-persona.ts — persona docs describe the new default and how to opt back into queueing with interrupt: false.
  • backend/images/base.pyIMAGE_BUILD_VERSION bumped so redeployed sandboxes pick up the tool change.

notify_parent intentionally keeps interrupt: false — children shouldn't preempt the orchestrator's other work by default.

Why

Queued messages can sit unread for a long time when a child session is on a long-running tear. The orchestrator's follow-up prompts and course-corrections are almost always time-sensitive — a stuck child needs to see the redirect now, not after it finishes the wrong thing. The downstream /prompt handler in SessionAgentDO already routed interrupt: true to steer mode (handleInterruptPrompt, which aborts if busy and dispatches at elevated priority); only the caller default was wrong.

Test plan

  • After deploy, orchestrator's send_message to a busy child aborts the child's current turn and delivers the message immediately
  • send_message with interrupt: false still queues behind the target's current work
  • send_message to an idle child dispatches as a normal prompt (no spurious abort)
  • notify_parent continues to queue by default

Orchestrator (and any session) sending a message to another session
now aborts the target's current turn and delivers the message
immediately instead of queueing behind it. Pass interrupt: false to
opt back into queueing.

Queued messages can sit unread for a long time when the target is on
a long-running turn, which is not what the caller usually wants. The
downstream /prompt handler already routed interrupt:true to steer
mode; this just flips the tool default.

notify_parent keeps interrupt:false — children shouldn't preempt the
orchestrator's other work by default.
@github-actions

Copy link
Copy Markdown

Preview deployment: https://pr-172.dev-valet-turnkey-client.pages.dev

@xBalbinus

Copy link
Copy Markdown

@valet-valet-turnkey-dev review this PR

@valet-valet-turnkey-dev valet-valet-turnkey-dev 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.

The PR does exactly what it describes and the three-file change is internally consistent. The schema description for interrupt still says "If true (default)" which is accurate, and notify_parent is intentionally left alone per the description.

One real concern worth raising:

  1. docker/opencode/tools/send_message.ts:34 — The .describe() string for interrupt now reads "If true (default), abort…", but the Zod schema still marks the field .optional() with no .default(true). The runtime default is applied only at the ?? true call-site in execute. This is fine functionally, but any JSON-schema consumer (e.g. the LLM's function-calling layer) that reads the generated schema to decide what to pass will see interrupt as an optional boolean with no declared default — it won't infer true as the default value. If the model omits interrupt entirely (which is the likely behavior for an optional field with no schema-level default), the ?? true fallback fires correctly, so there is no bug in practice. But if you want the schema itself to advertise the default, consider .default(true) instead of .optional(). Worth a deliberate decision either way.

Created on behalf of Xiangan He xiangan@turnkey.io

@xBalbinus xBalbinus 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.

The default flip makes sense for the child-session case — a queued message behind a long turn really does sit unread, and read_messages already covers the passive check-in. One scoping problem inline that's worth resolving before merge, because it makes the actual behavior stronger than what the new description promises.

sessionId: args.session_id,
content: args.message,
interrupt: args.interrupt ?? false,
interrupt: args.interrupt ?? true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The description promises "its current turn is aborted", but the delivery path can't scope that narrowly. sendSessionMessage (session-cross.ts) POSTs the target DO /prompt with only {content, interrupt} — no channelType/channelId/threadId. In the DO, handleInterruptPrompt then computes an undefined abort scope and handleAbort(undefined, undefined) falls through to the fully unscoped, session-wide abort branch (runnerLink.send({ type: 'abort' }) — session-agent.ts even labels it "Fully unscoped abort — session-wide").

For a plain child session that's equivalent to "abort its current turn", so the common orchestrator→child case is fine. But the tool also targets siblings and the orchestrator itself, and orchestrators run concurrent per-thread turns — the DO grew thread-scoped abort logic (TKAI-106) precisely so one thread's interrupt can't kill the others. With interrupt now defaulting on, any routine cross-session message to a busy multi-threaded target aborts every in-flight turn on it.

Two ways out: have sendSessionMessage resolve and pass the target's main-channel/thread scope so the abort stays per-turn, or keep the steer default only when the target isn't an orchestrator (its session id prefix is already checked elsewhere in that function). Either keeps the new default honest with the description.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants