mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
feat: store plan.md file in appropriate configuration folders (#12713)
This commit is contained in:
@@ -233,7 +233,13 @@ class SaasSQLAppConversationInfoService(SQLAppConversationInfoService):
|
||||
result = result_set.first()
|
||||
if result:
|
||||
stored_metadata, saas_metadata = result
|
||||
return self._to_info_with_user_id(stored_metadata, saas_metadata)
|
||||
# Fetch sub-conversation IDs
|
||||
sub_conversation_ids = await self.get_sub_conversation_ids(conversation_id)
|
||||
return self._to_info_with_user_id(
|
||||
stored_metadata,
|
||||
saas_metadata,
|
||||
sub_conversation_ids=sub_conversation_ids,
|
||||
)
|
||||
return None
|
||||
|
||||
async def batch_get_app_conversation_info(
|
||||
@@ -262,8 +268,16 @@ class SaasSQLAppConversationInfoService(SQLAppConversationInfoService):
|
||||
for conversation_id in conversation_id_strs:
|
||||
if conversation_id in info_by_id:
|
||||
stored_metadata, saas_metadata = info_by_id[conversation_id]
|
||||
# Fetch sub-conversation IDs for each conversation
|
||||
sub_conversation_ids = await self.get_sub_conversation_ids(
|
||||
UUID(conversation_id)
|
||||
)
|
||||
results.append(
|
||||
self._to_info_with_user_id(stored_metadata, saas_metadata)
|
||||
self._to_info_with_user_id(
|
||||
stored_metadata,
|
||||
saas_metadata,
|
||||
sub_conversation_ids=sub_conversation_ids,
|
||||
)
|
||||
)
|
||||
else:
|
||||
results.append(None)
|
||||
@@ -316,10 +330,11 @@ class SaasSQLAppConversationInfoService(SQLAppConversationInfoService):
|
||||
self,
|
||||
stored: StoredConversationMetadata,
|
||||
saas_metadata: StoredConversationMetadataSaas,
|
||||
sub_conversation_ids: list[UUID] | None = None,
|
||||
) -> AppConversationInfo:
|
||||
"""Convert stored metadata to AppConversationInfo with user_id from SAAS metadata."""
|
||||
# Use the base _to_info method to get the basic info
|
||||
info = self._to_info(stored)
|
||||
info = self._to_info(stored, sub_conversation_ids=sub_conversation_ids)
|
||||
|
||||
# Override the created_by_user_id with the user_id from SAAS metadata
|
||||
info.created_by_user_id = (
|
||||
|
||||
27
frontend/__tests__/api/v1-conversation-service.test.ts
Normal file
27
frontend/__tests__/api/v1-conversation-service.test.ts
Normal file
@@ -0,0 +1,27 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import V1ConversationService from "#/api/conversation-service/v1-conversation-service.api";
|
||||
|
||||
const { mockGet } = vi.hoisted(() => ({ mockGet: vi.fn() }));
|
||||
vi.mock("#/api/open-hands-axios", () => ({
|
||||
openHands: { get: mockGet },
|
||||
}));
|
||||
|
||||
describe("V1ConversationService", () => {
|
||||
describe("readConversationFile", () => {
|
||||
it("uses default plan path when filePath is not provided", async () => {
|
||||
// Arrange
|
||||
const conversationId = "conv-123";
|
||||
mockGet.mockResolvedValue({ data: "# PLAN content" });
|
||||
|
||||
// Act
|
||||
await V1ConversationService.readConversationFile(conversationId);
|
||||
|
||||
// Assert
|
||||
expect(mockGet).toHaveBeenCalledTimes(1);
|
||||
const callUrl = mockGet.mock.calls[0][0] as string;
|
||||
expect(callUrl).toContain(
|
||||
"file_path=%2Fworkspace%2Fproject%2F.agents_tmp%2FPLAN.md",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -170,7 +170,7 @@ describe("PlanPreview", () => {
|
||||
// Arrange
|
||||
const user = userEvent.setup();
|
||||
const expectedPrompt =
|
||||
"Execute the plan based on the workspace/project/PLAN.md file.";
|
||||
"Execute the plan based on the .agents_tmp/PLAN.md file.";
|
||||
renderPlanPreview(<PlanPreview planContent="Plan content" />);
|
||||
const buildButton = screen.getByTestId("plan-preview-build-button");
|
||||
|
||||
@@ -201,7 +201,7 @@ describe("PlanPreview", () => {
|
||||
useOptimisticUserMessageStore.setState({ optimisticUserMessage: null });
|
||||
const user = userEvent.setup();
|
||||
const expectedPrompt =
|
||||
"Execute the plan based on the workspace/project/PLAN.md file.";
|
||||
"Execute the plan based on the .agents_tmp/PLAN.md file.";
|
||||
renderPlanPreview(<PlanPreview planContent="Plan content" />);
|
||||
const buildButton = screen.getByTestId("plan-preview-build-button");
|
||||
|
||||
|
||||
@@ -41,8 +41,7 @@ describe("useHandleBuildPlanClick", () => {
|
||||
(createChatMessage as unknown as ReturnType<typeof vi.fn>).mockReturnValue({
|
||||
action: "message",
|
||||
args: {
|
||||
content:
|
||||
"Execute the plan based on the workspace/project/PLAN.md file.",
|
||||
content: "Execute the plan based on the .agents_tmp/PLAN.md file.",
|
||||
image_urls: [],
|
||||
file_urls: [],
|
||||
timestamp: expect.any(String),
|
||||
@@ -78,7 +77,7 @@ describe("useHandleBuildPlanClick", () => {
|
||||
// Arrange
|
||||
const { result } = renderHook(() => useHandleBuildPlanClick());
|
||||
const expectedPrompt =
|
||||
"Execute the plan based on the workspace/project/PLAN.md file.";
|
||||
"Execute the plan based on the .agents_tmp/PLAN.md file.";
|
||||
|
||||
// Act
|
||||
act(() => {
|
||||
@@ -109,7 +108,7 @@ describe("useHandleBuildPlanClick", () => {
|
||||
useOptimisticUserMessageStore.setState({ optimisticUserMessage: null });
|
||||
const { result } = renderHook(() => useHandleBuildPlanClick());
|
||||
const expectedPrompt =
|
||||
"Execute the plan based on the workspace/project/PLAN.md file.";
|
||||
"Execute the plan based on the .agents_tmp/PLAN.md file.";
|
||||
|
||||
// Act
|
||||
act(() => {
|
||||
@@ -155,7 +154,7 @@ describe("useHandleBuildPlanClick", () => {
|
||||
expect(useConversationStore.getState().conversationMode).toBe("code");
|
||||
expect(mockSend).toHaveBeenCalledTimes(1);
|
||||
expect(useOptimisticUserMessageStore.getState().optimisticUserMessage).toBe(
|
||||
"Execute the plan based on the workspace/project/PLAN.md file.",
|
||||
"Execute the plan based on the .agents_tmp/PLAN.md file.",
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -319,12 +319,12 @@ class V1ConversationService {
|
||||
/**
|
||||
* Read a file from a specific conversation's sandbox workspace
|
||||
* @param conversationId The conversation ID
|
||||
* @param filePath Path to the file to read within the sandbox workspace (defaults to /workspace/project/PLAN.md)
|
||||
* @param filePath Path to the file to read within the sandbox workspace (defaults to /workspace/project/.agents_tmp/PLAN.md)
|
||||
* @returns The content of the file or an empty string if the file doesn't exist
|
||||
*/
|
||||
static async readConversationFile(
|
||||
conversationId: string,
|
||||
filePath: string = "/workspace/project/PLAN.md",
|
||||
filePath: string = "/workspace/project/.agents_tmp/PLAN.md",
|
||||
): Promise<string> {
|
||||
const params = new URLSearchParams();
|
||||
params.append("file_path", filePath);
|
||||
|
||||
@@ -55,7 +55,9 @@ export function ChatMessage({
|
||||
"flex flex-col gap-2",
|
||||
type === "user" && "p-4 bg-tertiary self-end",
|
||||
type === "agent" && "mt-6 w-full max-w-full bg-transparent",
|
||||
isFromPlanningAgent && "border border-[#597ff4] bg-tertiary p-4 mt-2",
|
||||
isFromPlanningAgent &&
|
||||
type === "agent" &&
|
||||
"border border-[#597ff4] bg-tertiary p-4 mt-2",
|
||||
)}
|
||||
>
|
||||
<div
|
||||
|
||||
@@ -39,14 +39,14 @@ export function PlanPreview({
|
||||
isBuildDisabled,
|
||||
}: PlanPreviewProps) {
|
||||
const { t } = useTranslation();
|
||||
const { selectTab } = useSelectConversationTab();
|
||||
const { navigateToTab } = useSelectConversationTab();
|
||||
const { handleBuildPlanClick } = useHandleBuildPlanClick();
|
||||
const { scrollDomToBottom } = useScrollContext();
|
||||
|
||||
const shouldUsePlanningAgent = USE_PLANNING_AGENT();
|
||||
|
||||
const handleViewClick = () => {
|
||||
selectTab("planner");
|
||||
navigateToTab("planner");
|
||||
};
|
||||
|
||||
// Handle Build action with scroll to bottom
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import { OpenHandsEvent } from "#/types/v1/core";
|
||||
import { GenericEventMessage } from "../../../features/chat/generic-event-message";
|
||||
import { ChatMessage } from "../../../features/chat/chat-message";
|
||||
import { getEventContent } from "../event-content-helpers/get-event-content";
|
||||
import { getObservationResult } from "../event-content-helpers/get-observation-result";
|
||||
import { isObservationEvent } from "#/types/v1/type-guards";
|
||||
@@ -14,13 +13,11 @@ import { ObservationResultStatus } from "../../../features/chat/event-content-he
|
||||
interface GenericEventMessageWrapperProps {
|
||||
event: OpenHandsEvent | SkillReadyEvent;
|
||||
isLastMessage: boolean;
|
||||
isFromPlanningAgent?: boolean;
|
||||
}
|
||||
|
||||
export function GenericEventMessageWrapper({
|
||||
event,
|
||||
isLastMessage,
|
||||
isFromPlanningAgent = false,
|
||||
}: GenericEventMessageWrapperProps) {
|
||||
const { title, details } = getEventContent(event);
|
||||
|
||||
@@ -30,17 +27,6 @@ export function GenericEventMessageWrapper({
|
||||
if (event.observation.kind === "TaskTrackerObservation") {
|
||||
return <div>{details}</div>;
|
||||
}
|
||||
if (event.observation.kind === "FinishObservation") {
|
||||
const message = typeof details === "string" ? details : String(details);
|
||||
// Use ChatMessage for proper styling (blue border for planning agent, text-sm)
|
||||
return (
|
||||
<ChatMessage
|
||||
type="agent"
|
||||
message={message}
|
||||
isFromPlanningAgent={isFromPlanningAgent}
|
||||
/>
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -121,7 +121,6 @@ const renderUserMessageWithSkillReady = (
|
||||
<GenericEventMessageWrapper
|
||||
event={skillReadyEvent}
|
||||
isLastMessage={isLastMessage}
|
||||
isFromPlanningAgent={commonProps.isFromPlanningAgent}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
@@ -212,7 +211,6 @@ export function EventMessage({
|
||||
<GenericEventMessageWrapper
|
||||
event={event}
|
||||
isLastMessage={isLastMessage}
|
||||
isFromPlanningAgent={isFromPlanningAgent}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
@@ -261,7 +259,6 @@ export function EventMessage({
|
||||
<GenericEventMessageWrapper
|
||||
event={event}
|
||||
isLastMessage={isLastMessage}
|
||||
isFromPlanningAgent={isFromPlanningAgent}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
@@ -292,10 +289,6 @@ export function EventMessage({
|
||||
|
||||
// Generic fallback for all other events
|
||||
return (
|
||||
<GenericEventMessageWrapper
|
||||
event={event}
|
||||
isLastMessage={isLastMessage}
|
||||
isFromPlanningAgent={isFromPlanningAgent}
|
||||
/>
|
||||
<GenericEventMessageWrapper event={event} isLastMessage={isLastMessage} />
|
||||
);
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ export const useHandleBuildPlanClick = () => {
|
||||
setConversationMode("code");
|
||||
|
||||
// Create the build prompt to execute the plan
|
||||
const buildPrompt = `Execute the plan based on the workspace/project/PLAN.md file.`;
|
||||
const buildPrompt = `Execute the plan based on the .agents_tmp/PLAN.md file.`;
|
||||
|
||||
// Send the message to the code agent
|
||||
const timestamp = new Date().toISOString();
|
||||
|
||||
@@ -48,6 +48,19 @@ export function useSelectConversationTab() {
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Navigates to a tab without toggle behavior.
|
||||
* Always shows the panel and selects the tab, even if already selected.
|
||||
* Use this for "View" or "Read More" buttons that should always navigate.
|
||||
*/
|
||||
const navigateToTab = (tab: ConversationTab) => {
|
||||
onTabChange(tab);
|
||||
if (!isRightPanelShown) {
|
||||
setHasRightPanelToggled(true);
|
||||
setPersistedRightPanelShown(true);
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Checks if a specific tab is currently active (selected and panel is visible).
|
||||
*/
|
||||
@@ -56,6 +69,7 @@ export function useSelectConversationTab() {
|
||||
|
||||
return {
|
||||
selectTab,
|
||||
navigateToTab,
|
||||
isTabActive,
|
||||
onTabChange,
|
||||
selectedTab,
|
||||
|
||||
@@ -19,6 +19,13 @@ export const handleEventForUI = (
|
||||
return newUiEvents;
|
||||
}
|
||||
|
||||
// Don't add FinishObservation at all - we keep the FinishAction instead
|
||||
// Both contain the same message content, so we only need to display one
|
||||
// This also prevents duplicate messages when events arrive out of order due to React batching
|
||||
if (event.observation.kind === "FinishObservation") {
|
||||
return newUiEvents;
|
||||
}
|
||||
|
||||
// Find and replace the corresponding action from uiEvents
|
||||
const actionIndex = newUiEvents.findIndex(
|
||||
(uiEvent) => uiEvent.id === event.action_id,
|
||||
|
||||
@@ -569,6 +569,28 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
if not request.llm_model and parent_info.llm_model:
|
||||
request.llm_model = parent_info.llm_model
|
||||
|
||||
def _compute_plan_path(
|
||||
self,
|
||||
working_dir: str,
|
||||
git_provider: ProviderType | None,
|
||||
) -> str:
|
||||
"""Compute the PLAN.md path based on provider type.
|
||||
|
||||
Args:
|
||||
working_dir: The workspace working directory
|
||||
git_provider: The git provider type (GitHub, GitLab, Azure DevOps, etc.)
|
||||
|
||||
Returns:
|
||||
Absolute path to PLAN.md file in the appropriate config directory
|
||||
"""
|
||||
# GitLab and Azure DevOps use agents-tmp-config (since .agents_tmp is invalid)
|
||||
if git_provider in (ProviderType.GITLAB, ProviderType.AZURE_DEVOPS):
|
||||
config_dir = 'agents-tmp-config'
|
||||
else:
|
||||
config_dir = '.agents_tmp'
|
||||
|
||||
return f'{working_dir}/{config_dir}/PLAN.md'
|
||||
|
||||
async def _setup_secrets_for_git_providers(self, user: UserInfo) -> dict:
|
||||
"""Set up secrets for all git provider authentication.
|
||||
|
||||
@@ -855,6 +877,8 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
mcp_config: dict,
|
||||
condenser_max_size: int | None,
|
||||
secrets: dict[str, SecretValue] | None = None,
|
||||
git_provider: ProviderType | None = None,
|
||||
working_dir: str | None = None,
|
||||
) -> Agent:
|
||||
"""Create an agent with appropriate tools and context based on agent type.
|
||||
|
||||
@@ -865,6 +889,8 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
mcp_config: MCP configuration dictionary
|
||||
condenser_max_size: condenser_max_size setting
|
||||
secrets: Optional dictionary of secrets for authentication
|
||||
git_provider: Optional git provider type for computing plan path
|
||||
working_dir: Optional working directory for computing plan path
|
||||
|
||||
Returns:
|
||||
Configured Agent instance with context
|
||||
@@ -874,9 +900,14 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
|
||||
# Create agent based on type
|
||||
if agent_type == AgentType.PLAN:
|
||||
# Compute plan path if working_dir is provided
|
||||
plan_path = None
|
||||
if working_dir:
|
||||
plan_path = self._compute_plan_path(working_dir, git_provider)
|
||||
|
||||
agent = Agent(
|
||||
llm=llm,
|
||||
tools=get_planning_tools(),
|
||||
tools=get_planning_tools(plan_path=plan_path),
|
||||
system_prompt_filename='system_prompt_planning.j2',
|
||||
system_prompt_kwargs={'plan_structure': format_plan_structure()},
|
||||
condenser=condenser,
|
||||
@@ -1153,6 +1184,8 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
mcp_config,
|
||||
user.condenser_max_size,
|
||||
secrets=secrets,
|
||||
git_provider=git_provider,
|
||||
working_dir=working_dir,
|
||||
)
|
||||
|
||||
# Finalize and return the conversation request
|
||||
|
||||
@@ -682,6 +682,41 @@ class TestLiveStatusAppConversationService:
|
||||
== 'https://mcp.tavily.com/mcp/?tavilyApiKey=env_tavily_key'
|
||||
)
|
||||
|
||||
def test_compute_plan_path_default_uses_agents_tmp(self):
|
||||
"""Test _compute_plan_path returns .agents_tmp/PLAN.md for default/GitHub."""
|
||||
# Arrange
|
||||
working_dir = '/workspace/project'
|
||||
|
||||
# Act
|
||||
path_none = self.service._compute_plan_path(working_dir, None)
|
||||
path_github = self.service._compute_plan_path(working_dir, ProviderType.GITHUB)
|
||||
|
||||
# Assert
|
||||
assert path_none == '/workspace/project/.agents_tmp/PLAN.md'
|
||||
assert path_github == '/workspace/project/.agents_tmp/PLAN.md'
|
||||
|
||||
def test_compute_plan_path_gitlab_uses_agents_tmp_config(self):
|
||||
"""Test _compute_plan_path returns agents-tmp-config/PLAN.md for GitLab."""
|
||||
# Arrange
|
||||
working_dir = '/workspace/project'
|
||||
|
||||
# Act
|
||||
path = self.service._compute_plan_path(working_dir, ProviderType.GITLAB)
|
||||
|
||||
# Assert
|
||||
assert path == '/workspace/project/agents-tmp-config/PLAN.md'
|
||||
|
||||
def test_compute_plan_path_azure_uses_agents_tmp_config(self):
|
||||
"""Test _compute_plan_path returns agents-tmp-config/PLAN.md for Azure."""
|
||||
# Arrange
|
||||
working_dir = '/workspace/project'
|
||||
|
||||
# Act
|
||||
path = self.service._compute_plan_path(working_dir, ProviderType.AZURE_DEVOPS)
|
||||
|
||||
# Assert
|
||||
assert path == '/workspace/project/agents-tmp-config/PLAN.md'
|
||||
|
||||
@patch(
|
||||
'openhands.app_server.app_conversation.live_status_app_conversation_service.get_planning_tools'
|
||||
)
|
||||
@@ -704,6 +739,8 @@ class TestLiveStatusAppConversationService:
|
||||
mock_format_plan.return_value = 'test_plan_structure'
|
||||
mcp_config = {'default': {'url': 'test'}}
|
||||
system_message_suffix = 'Test suffix'
|
||||
working_dir = '/workspace/project'
|
||||
git_provider = ProviderType.GITHUB
|
||||
|
||||
# Act
|
||||
with patch(
|
||||
@@ -719,9 +756,14 @@ class TestLiveStatusAppConversationService:
|
||||
system_message_suffix,
|
||||
mcp_config,
|
||||
self.mock_user.condenser_max_size,
|
||||
git_provider=git_provider,
|
||||
working_dir=working_dir,
|
||||
)
|
||||
|
||||
# Assert
|
||||
mock_get_tools.assert_called_once_with(
|
||||
plan_path='/workspace/project/.agents_tmp/PLAN.md'
|
||||
)
|
||||
mock_agent_class.assert_called_once()
|
||||
call_kwargs = mock_agent_class.call_args[1]
|
||||
assert call_kwargs['llm'] == mock_llm
|
||||
@@ -1006,6 +1048,8 @@ class TestLiveStatusAppConversationService:
|
||||
mock_mcp_config,
|
||||
self.mock_user.condenser_max_size,
|
||||
secrets=mock_secrets,
|
||||
git_provider=ProviderType.GITHUB,
|
||||
working_dir='/test/dir',
|
||||
)
|
||||
self.service._finalize_conversation_request.assert_called_once()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user