Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions pkg/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,64 @@ func (a *App) Run(ctx context.Context, cancel context.CancelFunc, message string
}()
}

// buildQueuedMessage processes a text/attachment pair into a runtime.QueuedMessage
// suitable for either steering or follow-up. Attachments are rendered into
// MultiContent parts so the model can see images/PDFs alongside the text.
func (a *App) buildQueuedMessage(content string, attachments []messages.Attachment) runtime.QueuedMessage {
msg := runtime.QueuedMessage{Content: content}

if len(attachments) == 0 {
return msg
}

ctx := context.Background()
var textBuilder strings.Builder
textBuilder.WriteString(content)

var binaryParts []chat.MessagePart

for _, att := range attachments {
switch {
case att.FilePath != "":
a.processFileAttachment(ctx, att, &textBuilder, &binaryParts)
case att.Content != "":
a.processInlineAttachment(att, &textBuilder)
default:
slog.Debug("skipping attachment with no file path or content", "name", att.Name)
}
}

msg.Content = textBuilder.String()
if len(binaryParts) > 0 {
msg.MultiContent = binaryParts
}
return msg
}

// Steer enqueues a user message for mid-turn injection into the running agent
// loop. The agent will see the message at the next tool-round boundary. Returns
// an error if the steer queue is full.
func (a *App) Steer(content string, attachments []messages.Attachment) error {
return a.runtime.Steer(a.buildQueuedMessage(content, attachments))
}

// ClearSteerQueue drains all pending messages from the runtime's steer queue.
func (a *App) ClearSteerQueue() {
a.runtime.ClearSteerQueue()
}

// FollowUp enqueues a user message for end-of-turn processing. Each follow-up
// gets a full undivided agent turn after the current turn completes. Returns
// an error if the follow-up queue is full.
func (a *App) FollowUp(content string, attachments []messages.Attachment) error {
return a.runtime.FollowUp(a.buildQueuedMessage(content, attachments))
}

// ClearFollowUpQueue drains all pending messages from the runtime's follow-up queue.
func (a *App) ClearFollowUpQueue() {
a.runtime.ClearFollowUpQueue()
}

// processFileAttachment reads a file from disk, classifies it, and either
// appends its text content to textBuilder or adds a binary part to binaryParts.
func (a *App) processFileAttachment(ctx context.Context, att messages.Attachment, textBuilder *strings.Builder, binaryParts *[]chat.MessagePart) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ func (m *mockRuntime) TitleGenerator() *sessiontitle.Generator { return nil }
func (m *mockRuntime) Close() error { return nil }
func (m *mockRuntime) Stop() {}
func (m *mockRuntime) Steer(_ runtime.QueuedMessage) error { return nil }
func (m *mockRuntime) ClearSteerQueue() {}
func (m *mockRuntime) FollowUp(_ runtime.QueuedMessage) error { return nil }
func (m *mockRuntime) ClearFollowUpQueue() {}

// Verify mockRuntime implements runtime.Runtime
var _ runtime.Runtime = (*mockRuntime)(nil)
Expand Down
2 changes: 2 additions & 0 deletions pkg/cli/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ func (m *mockRuntime) UpdateSessionTitle(context.Context, *session.Session, stri
func (m *mockRuntime) TitleGenerator() *sessiontitle.Generator { return nil }
func (m *mockRuntime) Close() error { return nil }
func (m *mockRuntime) Steer(runtime.QueuedMessage) error { return nil }
func (m *mockRuntime) ClearSteerQueue() {}
func (m *mockRuntime) FollowUp(runtime.QueuedMessage) error { return nil }
func (m *mockRuntime) ClearFollowUpQueue() {}
func (m *mockRuntime) RegenerateTitle(context.Context, *session.Session, chan runtime.Event) {}

func (m *mockRuntime) Resume(_ context.Context, req runtime.ResumeRequest) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/runtime/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ func (m *mockRuntime) UpdateSessionTitle(context.Context, *session.Session, stri
func (m *mockRuntime) TitleGenerator() *sessiontitle.Generator { return nil }
func (m *mockRuntime) Close() error { return nil }
func (m *mockRuntime) Steer(QueuedMessage) error { return nil }
func (m *mockRuntime) ClearSteerQueue() {}
func (m *mockRuntime) FollowUp(QueuedMessage) error { return nil }
func (m *mockRuntime) ClearFollowUpQueue() {}

func (m *mockRuntime) RegenerateTitle(context.Context, *session.Session, chan Event) {
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/runtime/loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ func (r *LocalRuntime) RunStream(ctx context.Context, sess *session.Session) <-c
events := make(chan Event, 128)

go func() {
// Drain any orphaned messages from a previous cancelled run so
// they don't leak into this new stream. Both queues are drained:
// the user may have switched modes between sessions, or pending
// messages of either kind may have survived a cancel boundary.
r.steerQueue.Drain(context.Background())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

HIGH: followUpQueue not drained at RunStream start — orphaned follow-ups leak into the next session

RunStream explicitly drains steerQueue at startup (line 80) to clear orphaned messages from a previous cancelled session, but followUpQueue receives no equivalent drain. If the user cancels a session while follow-up messages are pending (in followup mode), those messages remain in followUpQueue. The next RunStream call — for a completely different conversation — will dequeue and process them at the end of the first agent turn (loop.go:454: r.followUpQueue.Dequeue(ctx)), silently injecting stale messages into the new session as if the user sent them there. This works in tandem with Finding 2 below.

Fix: Add r.followUpQueue.Drain(context.Background()) immediately after the steer drain on line 80.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed in 4f823e4. RunStream now drains both queues at entry, so a follow-up enqueued in a previously cancelled run can't leak into the next session.

r.followUpQueue.Drain(context.Background())

telemetry.RecordSessionStart(ctx, r.CurrentAgentName(), sess.ID)

ctx, sessionSpan := r.startSpan(ctx, "runtime.session", trace.WithAttributes(
Expand Down
8 changes: 8 additions & 0 deletions pkg/runtime/remote_runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,14 @@ func (r *RemoteRuntime) Steer(msg QueuedMessage) error {
})
}

// ClearSteerQueue is a no-op for remote runtimes — steered messages are
// forwarded to the server and there is no local queue to drain.
func (r *RemoteRuntime) ClearSteerQueue() {}

// ClearFollowUpQueue is a no-op for remote runtimes — follow-up messages are
// forwarded to the server and there is no local queue to drain.
func (r *RemoteRuntime) ClearFollowUpQueue() {}

// FollowUp enqueues a message for end-of-turn processing on the remote server.
func (r *RemoteRuntime) FollowUp(msg QueuedMessage) error {
if r.sessionID == "" {
Expand Down
18 changes: 18 additions & 0 deletions pkg/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,15 @@ type Runtime interface {
// running agent loop. Returns an error if the queue is full or steering
// is not available.
Steer(msg QueuedMessage) error
// ClearSteerQueue drains all pending messages from the steer queue,
// discarding them. Used when the user explicitly clears the queue.
ClearSteerQueue()
// FollowUp enqueues a message for end-of-turn processing. Each follow-up
// gets a full undivided agent turn. Returns an error if the queue is full.
FollowUp(msg QueuedMessage) error
// ClearFollowUpQueue drains all pending messages from the follow-up queue,
// discarding them. Used when the user explicitly clears the queue.
ClearFollowUpQueue()

// Close releases resources held by the runtime (e.g., session store connections).
Close() error
Expand Down Expand Up @@ -1059,6 +1065,12 @@ func (r *LocalRuntime) Steer(msg QueuedMessage) error {
return nil
}

// ClearSteerQueue drains all pending messages from the steer queue,
// discarding them. This is safe to call concurrently with the agent loop.
func (r *LocalRuntime) ClearSteerQueue() {
r.steerQueue.Drain(context.Background())
}

// FollowUp enqueues a message to be processed after the current agent turn
// finishes. Unlike Steer, follow-ups are popped one at a time and each gets
// a full undivided agent turn.
Expand All @@ -1069,6 +1081,12 @@ func (r *LocalRuntime) FollowUp(msg QueuedMessage) error {
return nil
}

// ClearFollowUpQueue drains all pending messages from the follow-up queue,
// discarding them. This is safe to call concurrently with the agent loop.
func (r *LocalRuntime) ClearFollowUpQueue() {
r.followUpQueue.Drain(context.Background())
}

// Run starts the agent's interaction loop

func (r *LocalRuntime) startSpan(ctx context.Context, name string, opts ...trace.SpanStartOption) (context.Context, trace.Span) {
Expand Down
11 changes: 11 additions & 0 deletions pkg/tui/commands/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,17 @@ func builtInSettingsCommands() []Item {
return core.CmdHandler(messages.OpenThemePickerMsg{})
},
},
{
ID: "settings.followup-behavior",
Label: "Follow-up Behavior",
SlashCommand: "/followup-behavior",
Description: "Set behavior for messages sent while agent is working (usage: /followup-behavior steer|followup)",
Category: "Settings",
Immediate: true,
Execute: func(arg string) tea.Cmd {
return core.CmdHandler(messages.SetFollowupBehaviorMsg{Mode: strings.TrimSpace(arg)})
},
},
}
}

Expand Down
55 changes: 55 additions & 0 deletions pkg/tui/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,61 @@ func (m *appModel) handleToggleSplitDiff() (tea.Model, tea.Cmd) {
return m, tea.Batch(cmds...)
}

// handleSetFollowupBehavior sets the follow-up behavior persistently. When
// called with an empty mode it reports the current value. Invalid modes
// produce a warning notification listing the accepted values.
func (m *appModel) handleSetFollowupBehavior(mode string) (tea.Model, tea.Cmd) {
mode = strings.ToLower(strings.TrimSpace(mode))

current := userconfig.Get().GetFollowupBehavior()
if mode == "" {
return m, notification.InfoCmd(fmt.Sprintf(
"Follow-up behavior: %s · usage: /followup-behavior steer|followup", current))
}

if mode != userconfig.FollowupBehaviorSteer && mode != userconfig.FollowupBehaviorFollowUp {
return m, notification.WarningCmd(fmt.Sprintf(
"Unknown follow-up behavior %q. Valid values: %s, %s",
mode, userconfig.FollowupBehaviorSteer, userconfig.FollowupBehaviorFollowUp))
}

if mode == current {
return m, notification.InfoCmd(fmt.Sprintf("Follow-up behavior already set to %q", mode))
}

// Apply the change in-memory synchronously so userconfig.Get returns the
// new value immediately. Without this the send-dispatch and
// working-indicator readers below would race the background file write
// and keep returning the old mode until it commits — meaning a message
// sent right after the command could be dispatched under the old mode.
userconfig.SetFollowupBehaviorOverride(mode)

// Persist to global userconfig (fire-and-forget, matching the split-diff pattern).
go func() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MEDIUM: /followup-behavior setting is persisted asynchronously — the new mode is not active until the file write completes

handleSetFollowupBehavior fires a goroutine to write the new value to disk, but userconfig.Get() calls Load()os.ReadFile on every invocation with no in-memory cache. Between when the goroutine starts and when the file write commits, every call to userconfig.Get().GetFollowupBehavior() — including the dispatch switch in handleSendMsg and the queueLabelForMode() label in the working indicator — still returns the old value.

A user who types /followup-behavior followup and immediately sends a message while the agent is busy will see the success toast but have their message incorrectly steered (old behavior) rather than queued as a follow-up. The split-diff toggle uses the same fire-and-forget pattern, but its state is driven by an in-memory field (sessionState.ToggleSplitDiffView()), so the race doesn't affect it.

Fix: Apply the change to an in-memory field (e.g. update a package-level variable via a Set function) synchronously before launching the goroutine, so the in-flight setting is visible immediately.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 4f823e4. Added userconfig.SetFollowupBehaviorOverride, a mutex-guarded package-level override that Get applies on top of whatever it reads from disk. handleSetFollowupBehavior now calls it synchronously before launching the persistence goroutine, so the dispatch switch in handleSendMsg and the queueLabelForMode label see the new value immediately — no race against the file write. Covered by TestSetFollowupBehaviorOverride.

cfg, err := userconfig.Load()
if err != nil {
slog.Warn("Failed to load userconfig for follow-up behavior change", "error", err)
return
}
if cfg.Settings == nil {
cfg.Settings = &userconfig.Settings{}
}
cfg.Settings.FollowupBehavior = mode
if err := cfg.Save(); err != nil {
slog.Warn("Failed to persist follow-up behavior to userconfig", "error", err)
}
}()

var description string
switch mode {
case userconfig.FollowupBehaviorSteer:
description = "messages sent while working will be injected mid-turn"
case userconfig.FollowupBehaviorFollowUp:
description = "messages sent while working will queue as their own turn"
}
return m, notification.SuccessCmd(fmt.Sprintf("Follow-up behavior set to %q · %s", mode, description))
}

// --- Dialogs ---

func (m *appModel) handleShowCostDialog() (tea.Model, tea.Cmd) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/tui/messages/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ type (
// ToggleSplitDiffMsg toggles split diff view mode.
ToggleSplitDiffMsg struct{}

// SetFollowupBehaviorMsg sets the follow-up behavior (how messages
// submitted while the agent is working are dispatched). Mode is the raw
// user-supplied value; empty string means "show current value". Unknown
// values are rejected by the handler.
SetFollowupBehaviorMsg struct{ Mode string }

// SendMsg contains the content sent to the agent.
SendMsg struct {
Content string // Full content sent to the agent (with file contents expanded)
Expand Down
Loading
Loading