Skip to content

revert /reply to old behavior when full conversation provided#6058

Merged
tlongwell-block merged 3 commits intomainfrom
reply_context_regression
Dec 18, 2025
Merged

revert /reply to old behavior when full conversation provided#6058
tlongwell-block merged 3 commits intomainfrom
reply_context_regression

Conversation

@tlongwell-block
Copy link
Collaborator

@tlongwell-block tlongwell-block commented Dec 10, 2025

Allow passing full conversation to /reply if needed. This supports the previous behavior of the /reply endpoint before the session required overhaul as part of the agent manager work. This is required for some uses of goose-server where session contents may have more than one source of truth, like the goose slackbot.

@tlongwell-block tlongwell-block marked this pull request as ready for review December 17, 2025 15:40
@tlongwell-block tlongwell-block force-pushed the reply_context_regression branch from 4a99bd3 to 6aafb17 Compare December 17, 2025 17:29
Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

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

I think we do need to keep the full conversation in the loop

};

let mut all_messages = messages.clone();
let mut all_messages = Conversation::new_unvalidated(vec![user_message.clone()]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this doesn't strike me as quite right; we keep a shadow administration of the messages here I think just for tracking purposes. we should probably move that to agent, but until we do, we should in the block above retrieve the conversation so far from the db if it is not given and then call the agent only with the user message

@tlongwell-block
Copy link
Collaborator Author

/goose

@github-actions
Copy link
Contributor

Summary: This PR restores the ability for external integrations (like the Slack bot) to provide their own conversation history when calling /reply, while maintaining backward compatibility for the desktop UI. The API change from messages to user_message + optional conversation_so_far is well-designed and the implementation is correct.

🟡 Warnings

  1. Silent failure on replace_conversation may cause stale context (crates/goose-server/src/routes/reply.rs:287-293)

    When conversation_so_far is provided but SessionManager::replace_conversation fails, the code logs a warning and continues:

    if let Err(e) = SessionManager::replace_conversation(&session_id, &conv).await {
        tracing::warn!(
            "Failed to replace session conversation for {}: {}",
            session_id,
            e
        );
    }

    However, agent.reply() subsequently reads conversation from SessionManager::get_session() (see crates/goose/src/agents/agent.rs). If the replace failed, the agent will use the old session data instead of the caller-provided conversation.

    For external integrations where conversation_so_far is the source of truth, this could lead to the agent responding with incorrect context. Consider either:

    • Returning an error to the caller when replace fails, or
    • Documenting this as a known limitation for callers

✅ Highlights

  • Clean API design: Separating user_message from conversation_so_far makes the intent clear - the new message vs the history
  • Backward compatible: Desktop UI continues working with just user_message while external integrations can use conversation_so_far
  • Test updated correctly: The integration test at reply.rs:482-488 properly uses the new API structure
  • OpenAPI spec regenerated: Following project conventions

Review generated by goose

@tlongwell-block tlongwell-block merged commit d629fec into main Dec 18, 2025
18 checks passed
@tlongwell-block tlongwell-block deleted the reply_context_regression branch December 18, 2025 17:55
zanesq added a commit that referenced this pull request Dec 18, 2025
* 'main' of github.com:block/goose: (28 commits)
  Clean PR preview sites from gh-pages branch history (#6161)
  fix: make goose reviewer less sycophantic (#6171)
  revert /reply to previous behavior (replacing session history) when full conversation provided (#6058)
  chore: manually update version (#6166)
  Integrate pricing with canonical model (#6130)
  Regenerate canonical models when release branch is created. (#6127)
  fix: use correct parameter name in read_module handler (#6148)
  docs: blog for code mode MCP (#6126)
  test: add ACP integration test (#6150)
  docs: auto download updates (#6163)
  fix: respect default_enabled value of platform extensions (#6159)
  docs: skills (#6062)
  fix: add conditional configuration for GOOSE_BIN_DIR in PATH (#5940)
  Update dependencies to help in Fedora packaging (#5835)
  fix: make goose reviewer less bad (#6154)
  docs: create/edit recipe button (#6145)
  fix(google): Fix 400 Bad Request error with Gemini 3 thought signatures (#6035)
  fix: we don't need to warn about tool count when in code mode (#6149)
  deps: upgrade agent-client-protocol to 0.9.0 (#6109)
  fix(providers): fix for gemini-cli on windows to work around cmd's multiline prompt limitations #5911 (#5966)
  ...

# Conflicts:
#	ui/desktop/src/api/sdk.gen.ts
#	ui/desktop/src/hooks/useAgent.ts
dianed-square pushed a commit that referenced this pull request Dec 19, 2025
bkvarda pushed a commit to bkvarda/goose that referenced this pull request Dec 19, 2025
…ull conversation provided (block#6058)

Signed-off-by: Brandon Kvarda <brandon.kvarda@databricks.com>
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