From 441ac8bb88533219e1d83e166cb81b8fe8f0a665 Mon Sep 17 00:00:00 2001 From: John Pangas Date: Wed, 19 Aug 2026 02:51:12 -0600 Subject: [PATCH 1/3] Use the TestRail action directly in test-plan-generator The agent submitted its plan through a private submit_result MCP tool, and __main__ then copied that structured result into the TestRail action. The action existed but was never the thing the agent called. Expose testrail.submit_test_plan to the agent as a recordable action instead, and drop the result module: the action schema is now the only definition of a generated test plan. The per-case `context` label goes with it, and the case count and sequential-id checks move from TestPlanResult onto SubmitTestPlanInput, where they are enforced for every caller. Recording twice would create two TestRail suites, so a second call is now rejected. The agent's own findings become its closing prose, so the UI reads the plan from the recorded action's params, falling back to findings.result for runs recorded before this change. Fixes #6508 --- .../test_plan_generator/__main__.py | 3 +- .../test_plan_generator/agent.py | 40 ++--- .../test_plan_generator/config.py | 6 + .../test_plan_generator/prompts/system.md | 27 ++- .../test_plan_generator/result.py | 154 ------------------ .../hackbot_runtime/actions/testrail.py | 54 +++--- .../tests/test_testrail_action.py | 67 +++++++- .../tests/test_testrail_handler.py | 2 - .../hackbot-ui/components/FindingsView.tsx | 5 +- services/hackbot-ui/components/RunDetail.tsx | 13 +- .../hackbot-ui/components/TestPlanView.tsx | 14 +- 11 files changed, 161 insertions(+), 224 deletions(-) delete mode 100644 agents/test-plan-generator/hackbot_agents/test_plan_generator/result.py diff --git a/agents/test-plan-generator/hackbot_agents/test_plan_generator/__main__.py b/agents/test-plan-generator/hackbot_agents/test_plan_generator/__main__.py index aaae133ca7..68a11794e3 100644 --- a/agents/test-plan-generator/hackbot_agents/test_plan_generator/__main__.py +++ b/agents/test-plan-generator/hackbot_agents/test_plan_generator/__main__.py @@ -1,5 +1,4 @@ from hackbot_runtime import HackbotContext, run_async -from hackbot_runtime.actions.testrail import record_test_plan from pydantic_settings import BaseSettings, SettingsConfigDict from .agent import TestPlanGeneratorResult, run_test_plan_generator @@ -32,8 +31,8 @@ async def main(ctx: HackbotContext) -> TestPlanGeneratorResult: firefox_path=firefox_path, log=ctx.log_path, verbose=True, + actions_recorder=ctx.actions, ) - record_test_plan(ctx.actions, result.result.model_dump(mode="json")) return result diff --git a/agents/test-plan-generator/hackbot_agents/test_plan_generator/agent.py b/agents/test-plan-generator/hackbot_agents/test_plan_generator/agent.py index 08122e2495..3eaddcf70e 100644 --- a/agents/test-plan-generator/hackbot_agents/test_plan_generator/agent.py +++ b/agents/test-plan-generator/hackbot_agents/test_plan_generator/agent.py @@ -11,18 +11,14 @@ McpServerConfig, ResultMessage, ) -from hackbot_runtime import AgentError, HackbotAgentResult +from hackbot_runtime import ActionsRecorder, AgentError, HackbotAgentResult +from hackbot_runtime.actions import ACTIONS_SERVER_NAME +from hackbot_runtime.actions.claude_sdk import actions_server_for, actions_to_tool_names +from hackbot_runtime.actions.testrail import ACTION_TYPE as TESTRAIL_SUBMIT_TEST_PLAN from hackbot_runtime.claude import Reporter -from .config import DEVTOOLS_TOOLS +from .config import DEVTOOLS_TOOLS, ENABLED_ACTION_TYPES from .devtools_mcp import build_devtools_server -from .result import ( - RESULT_SERVER_NAME, - SUBMIT_RESULT_TOOL, - ResultCollector, - TestPlanResult, - build_result_server, -) HERE = Path(__file__).resolve().parent @@ -30,7 +26,7 @@ class TestPlanGeneratorResult(HackbotAgentResult): - result: TestPlanResult | None = None + result: str | None = None def load_system_prompt() -> str: @@ -45,12 +41,12 @@ def build_user_prompt( f"Feature name:\n{feature_name}\n\n" f"Feature description:\n{feature_description}\n\n" f"Test scope:\n{test_scope}\n\n" - "Use the provided feature name as the structured result feature. " + "Use the provided feature name as the TestRail action feature. " "Keep all generated test cases within the provided test scope.\n\n" "Follow the required workflow exactly: before execution, generate no more than 30 " "test cases to cover all distinct behaviors, meaningful variations, and " "negative scenarios. Run the cases in order, stop each case after its first " - "failed step, and submit exactly one structured result." + "failed step, and record exactly one TestRail test plan action." ) @@ -65,6 +61,7 @@ async def run_test_plan_generator( firefox_path: str | None = None, verbose: bool = False, log: Path | None = None, + actions_recorder: ActionsRecorder | None = None, ) -> TestPlanGeneratorResult: """Generate and run a Firefox QA test plan for one feature.""" subject = feature_name @@ -76,12 +73,14 @@ async def run_test_plan_generator( enable_script=True, ) - result_collector = ResultCollector() - result_server = build_result_server(result_collector) + actions_recorder, actions_server = actions_server_for( + actions_recorder, types=ENABLED_ACTION_TYPES + ) + enabled_action_tools = actions_to_tool_names(ENABLED_ACTION_TYPES) mcp_servers: dict[str, McpServerConfig] = { "firefox-devtools": devtools_server, - RESULT_SERVER_NAME: result_server, + ACTIONS_SERVER_NAME: actions_server, } options = ClaudeAgentOptions( @@ -90,7 +89,7 @@ async def run_test_plan_generator( permission_mode="bypassPermissions", allowed_tools=[ *DEVTOOLS_TOOLS, - SUBMIT_RESULT_TOOL, + *enabled_action_tools, ], model=model, max_turns=max_turns, @@ -118,13 +117,16 @@ async def run_test_plan_generator( f"{subject} test-plan generation failed: " f"{result_msg.result or result_msg.subtype}" ) - if result_collector.result is None: + if not any( + action["type"] == TESTRAIL_SUBMIT_TEST_PLAN + for action in actions_recorder.actions + ): raise AgentError( - f"{subject}: agent finished without submitting a result via submit_result" + f"{subject}: agent finished without recording a TestRail test plan action" ) return TestPlanGeneratorResult( - result=result_collector.result, + result=result_msg.result, num_turns=result_msg.num_turns, total_cost_usd=result_msg.total_cost_usd, ) diff --git a/agents/test-plan-generator/hackbot_agents/test_plan_generator/config.py b/agents/test-plan-generator/hackbot_agents/test_plan_generator/config.py index 3760e2ff42..b943d9ef01 100644 --- a/agents/test-plan-generator/hackbot_agents/test_plan_generator/config.py +++ b/agents/test-plan-generator/hackbot_agents/test_plan_generator/config.py @@ -1,3 +1,9 @@ +# Recordable action types the agent may take, by dotted id. It generates and runs +# test cases, then records them for TestRail; it takes no other action. +ENABLED_ACTION_TYPES = [ + "testrail.submit_test_plan", +] + # Firefox DevTools MCP tools (@mozilla/firefox-devtools-mcp-moz), exposed under # the "firefox-devtools" server name. Keep this focused on tools needed to # generate and execute Firefox QA cases: page interaction, observation, diff --git a/agents/test-plan-generator/hackbot_agents/test_plan_generator/prompts/system.md b/agents/test-plan-generator/hackbot_agents/test_plan_generator/prompts/system.md index bb508182b3..bab8eee10f 100644 --- a/agents/test-plan-generator/hackbot_agents/test_plan_generator/prompts/system.md +++ b/agents/test-plan-generator/hackbot_agents/test_plan_generator/prompts/system.md @@ -2,7 +2,8 @@ You are a Firefox QA test-plan generation and execution agent. Generate test cases from the provided Firefox feature name, feature description, and test scope, run them in Firefox with the available DevTools MCP tools, and -report only pass/fail/unsuitable results. Do not try to fix, patch or make changes. +record the generated test plan for TestRail. Do not try to fix, patch or make +changes. ## Required workflow @@ -10,19 +11,18 @@ report only pass/fail/unsuitable results. Do not try to fix, patch or make chang variations, and negative scenarios before running any case. 2. Each test case must have: - A title. - - A primary execution context label: `chrome` or `content`. - Ordered test steps, each with an `action` and optional `expectation`. 3. Run the generated cases and steps in order. -4. Submit one final structured result with `submit_result`. - - Use the provided feature name as the structured result feature. +4. Record one final TestRail action with `testrail_submit_test_plan`. + - Use the provided feature name as the action feature. ## Context guidance -Choose a primary context label per case: `content` for normal web page or -document behavior; `chrome` for Firefox UI, browser state, preferences, toolbar, -menus, panels, downloads, history, bookmarks, PDF viewer chrome behavior, or -uncertainty. The label describes what the case mainly exercises; it does not -restrict per-step tool choice. +Decide which context each case mainly exercises and pick tools accordingly: +`content` for normal web page or document behavior; `chrome` for Firefox UI, +browser state, preferences, toolbar, menus, panels, downloads, history, +bookmarks, PDF viewer chrome behavior, or uncertainty. This judgment guides your +tool selection and it does not restrict per-step tool choice. Use the most appropriate DevTools MCP tool for each step. Prefer content tools for page/DOM interaction and privileged-context tools for browser UI/state or @@ -68,9 +68,8 @@ Mark a case as `unsuitable` only if it requires: ## Reporting -The final answer must be submitted through `submit_result` exactly once. A prose -message is not enough. Include one case result for every generated test case. +Record the generated test plan through `testrail_submit_test_plan` exactly once. +A prose message is not enough. -For failed steps, set `failure_reason` to a short explanation of the observed -failure. For failed or unsuitable cases, set the case-level `failure_reason` as -well. Leave `failure_reason` empty for passed steps and passed cases. +Then close with a write-up of the execution: which cases passed, failed, or were +unsuitable, with concise observations for the failed and unsuitable ones. diff --git a/agents/test-plan-generator/hackbot_agents/test_plan_generator/result.py b/agents/test-plan-generator/hackbot_agents/test_plan_generator/result.py deleted file mode 100644 index 674650b5a4..0000000000 --- a/agents/test-plan-generator/hackbot_agents/test_plan_generator/result.py +++ /dev/null @@ -1,154 +0,0 @@ -"""Structured result reporting for the test-plan-generator agent.""" - -from __future__ import annotations - -from typing import Literal - -from claude_agent_sdk import McpServerConfig, create_sdk_mcp_server, tool -from pydantic import ( - BaseModel, - Field, - ValidationError, - model_validator, -) - -RESULT_SERVER_NAME = "test-plan-generator" -SUBMIT_RESULT_TOOL = f"mcp__{RESULT_SERVER_NAME}__submit_result" - - -class GeneratedTestStep(BaseModel): - action: str = Field(description="The action a QA engineer should perform.") - expectation: str | None = Field( - default=None, - description=("Expected result for this step."), - ) - - -class GeneratedTestCase(BaseModel): - id: int = Field(description="Sequential case id starting at 1.") - title: str - context: Literal["chrome", "content"] = Field( - description=( - "Primary context label for what the case mainly exercises. This is " - "guidance for tool selection, not a restriction on which available " - "DevTools MCP tools may be used for individual steps." - ) - ) - preconditions: str | None = None - steps: list[GeneratedTestStep] = Field( - description=( - "Concise ordered test steps. Each step has an action and an optional " - "expectation." - ) - ) - - @model_validator(mode="after") - def _validate_steps(self) -> "GeneratedTestCase": - if not self.steps: - raise ValueError("each generated test case must have at least one step") - if not any(step.expectation for step in self.steps): - raise ValueError( - "each generated test case must include an expected result on at " - "least one verification step" - ) - return self - - -class StepResult(BaseModel): - step_number: int - status: Literal["passed", "failed", "not_run"] - observation: str - failure_reason: str | None = Field( - default=None, - description=( - "Required when status is failed. A concise reason why the step failed, " - "based only on what was observed during execution." - ), - ) - - @model_validator(mode="after") - def _validate_failure_reason(self) -> "StepResult": - if self.status == "failed" and not self.failure_reason: - raise ValueError("failed steps must include failure_reason") - return self - - -class TestCaseResult(BaseModel): - id: int - status: Literal["passed", "failed", "unsuitable"] - step_results: list[StepResult] - summary: str - failure_reason: str | None = Field( - default=None, - description=( - "Required when status is failed or unsuitable. A concise reason why " - "the case failed or could not be run, useful for later developer review." - ), - ) - - @model_validator(mode="after") - def _validate_failure_reason(self) -> "TestCaseResult": - if self.status in {"failed", "unsuitable"} and not self.failure_reason: - raise ValueError("failed or unsuitable cases must include failure_reason") - return self - - -class TestPlanResult(BaseModel): - feature: str - generated_test_cases: list[GeneratedTestCase] - results: list[TestCaseResult] - summary: str - - @model_validator(mode="after") - def _validate_result(self) -> "TestPlanResult": - case_count = len(self.generated_test_cases) - if not 1 <= case_count <= 30: - raise ValueError("generated_test_cases must contain 1 to 30 cases") - - case_ids = [case.id for case in self.generated_test_cases] - expected_ids = list(range(1, case_count + 1)) - if case_ids != expected_ids: - raise ValueError("generated test case ids must be sequential starting at 1") - - result_ids = [result.id for result in self.results] - if result_ids != expected_ids: - raise ValueError( - "results must contain one result for each generated test case id" - ) - - return self - - -SUBMIT_RESULT_SCHEMA = { - **TestPlanResult.model_json_schema(), - "additionalProperties": False, -} - - -class ResultCollector: - """Holds the result submitted by the agent, if any.""" - - def __init__(self) -> None: - self.result: TestPlanResult | None = None - - -def build_result_server(collector: ResultCollector) -> McpServerConfig: - """Build an in-process MCP server exposing the ``submit_result`` tool.""" - - @tool( - "submit_result", - "Submit the final generated Firefox QA test plan and execution result. " - "Call exactly once, after all generated test cases have been run.", - SUBMIT_RESULT_SCHEMA, - ) - async def submit_result(args: dict) -> dict: - try: - collector.result = TestPlanResult.model_validate(args) - except ValidationError as exc: - return { - "content": [{"type": "text", "text": f"Invalid result: {exc}"}], - "is_error": True, - } - return {"content": [{"type": "text", "text": "Result recorded."}]} - - return create_sdk_mcp_server(name=RESULT_SERVER_NAME, tools=[submit_result]) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py index d009dd2974..65bb41022c 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py @@ -1,9 +1,4 @@ -"""TestRail-domain recordable actions. - -The test plan generator records this action deterministically after its -structured result has been validated. The external TestRail mutation still -happens only in the apply side handler. -""" +"""TestRail recordable actions.""" from __future__ import annotations @@ -34,9 +29,6 @@ class TestRailStepInput(BaseModel): class TestRailCaseInput(BaseModel): id: int title: str = Field(description="TestRail test case title.") - context: str | None = Field( - default=None, description="Optional context for where this case applies." - ) preconditions: str | None = Field( default=None, description="Optional setup required before running this case." ) @@ -75,7 +67,9 @@ def expectations_must_include_verification(self) -> "TestRailCaseInput": class SubmitTestPlanInput(BaseModel): feature: str = Field(description="Feature covered by the generated test cases.") generated_test_cases: list[TestRailCaseInput] = Field( - min_length=1, description="Generated test cases to upload to TestRail." + min_length=1, + max_length=30, + description="Generated test cases to upload to TestRail.", ) @field_validator("feature") @@ -86,6 +80,14 @@ def feature_must_not_be_blank(cls, value: str) -> str: raise ValueError("feature must not be blank") return value + @model_validator(mode="after") + def case_ids_must_be_sequential(self) -> "SubmitTestPlanInput": + case_ids = [case.id for case in self.generated_test_cases] + expected_ids = list(range(1, len(self.generated_test_cases) + 1)) + if case_ids != expected_ids: + raise ValueError("test case ids must be sequential starting at 1") + return self + def _confirm(recorder: ActionsRecorder, action_type: str) -> str: return f"Recorded {action_type} (#{len(recorder.actions) - 1})." @@ -118,30 +120,30 @@ async def submit_test_plan( ], generated_test_cases: Annotated[ list[TestRailCaseInput], - Field(description="Generated test cases to upload together to TestRail."), + # Bounds are repeated from SubmitTestPlanInput: this copy is the agent's + # schema, that one is the enforcement. + Field( + min_length=1, + max_length=30, + description="Generated test cases to upload together to TestRail.", + ), ], ) -> str: """Record a generated test plan for deferred TestRail submission. - This records one reviewed action. The apply step creates a new TestRail - suite, creates a section in it, and uploads all supplied test cases. - Nothing is sent to TestRail during the agent run. + Call this at most once per run: the apply step creates a new TestRail suite + for every recorded action, so a second call would duplicate the whole plan. + The apply step creates the suite, creates a section in it, and uploads all + supplied test cases. Nothing is sent to TestRail during the agent run. """ + if any(action["type"] == ACTION_TYPE for action in recorder.actions): + raise ToolError( + "a test plan is already recorded for this run; do not call " + "submit_test_plan again" + ) params = _validated_params(feature, generated_test_cases) recorder.record(ACTION_TYPE, params) return _confirm(recorder, ACTION_TYPE) -def record_test_plan( - recorder: ActionsRecorder, - test_plan: dict[str, Any], -) -> dict: - """Record validated generated cases for deferred TestRail submission.""" - params = _validated_params( - str(test_plan.get("feature") or ""), - list(test_plan.get("generated_test_cases") or []), - ) - return recorder.record(ACTION_TYPE, params) - - TOOLS = tools_in(__name__) diff --git a/libs/hackbot-runtime/tests/test_testrail_action.py b/libs/hackbot-runtime/tests/test_testrail_action.py index 75fce89617..e6232ed023 100644 --- a/libs/hackbot-runtime/tests/test_testrail_action.py +++ b/libs/hackbot-runtime/tests/test_testrail_action.py @@ -11,7 +11,6 @@ def _cases(): { "id": 1, "title": "The PDF opens", - "context": "content", "preconditions": "A PDF is available.", "steps": [ {"action": "Open the PDF", "expectation": "The PDF is displayed."} @@ -31,7 +30,16 @@ async def test_submit_test_plan_tool_records_deferred_action(): assert recorder.actions[0]["type"] == ACTION_TYPE assert recorder.actions[0]["params"] == { "feature": "Feature", - "generated_test_cases": _cases(), + "generated_test_cases": [ + { + "id": 1, + "title": "The PDF opens", + "preconditions": "A PDF is available.", + "steps": [ + {"action": "Open the PDF", "expectation": "The PDF is displayed."} + ], + } + ], } @@ -42,7 +50,13 @@ async def test_submit_test_plan_tool_rejects_invalid_input(): await testrail.submit_test_plan( recorder, feature=" ", - generated_test_cases=[{"id": 1, "title": "Case", "steps": []}], + generated_test_cases=[ + { + "id": 1, + "title": "Case", + "steps": [], + } + ], ) assert "invalid TestRail submission" in str(exc.value) @@ -69,6 +83,53 @@ async def test_submit_test_plan_tool_rejects_cases_without_expectation(): assert recorder.actions == [] +async def test_submit_test_plan_tool_rejects_non_sequential_case_ids(): + recorder = ActionsRecorder() + cases = _cases() + cases[0]["id"] = 2 + + with pytest.raises(ToolError) as exc: + await testrail.submit_test_plan( + recorder, + feature="Feature", + generated_test_cases=cases, + ) + + assert "invalid TestRail submission" in str(exc.value) + assert recorder.actions == [] + + +async def test_submit_test_plan_tool_rejects_a_second_submission(): + recorder = ActionsRecorder() + await testrail.submit_test_plan( + recorder, feature="Feature", generated_test_cases=_cases() + ) + + with pytest.raises(ToolError) as exc: + await testrail.submit_test_plan( + recorder, feature="Other feature", generated_test_cases=_cases() + ) + + assert "already recorded" in str(exc.value) + assert [action["params"]["feature"] for action in recorder.actions] == ["Feature"] + + +async def test_submit_test_plan_tool_rejects_more_than_thirty_cases(): + recorder = ActionsRecorder() + case = _cases()[0] + cases = [{**case, "id": index} for index in range(1, 32)] + + with pytest.raises(ToolError) as exc: + await testrail.submit_test_plan( + recorder, + feature="Feature", + generated_test_cases=cases, + ) + + assert "invalid TestRail submission" in str(exc.value) + assert recorder.actions == [] + + async def test_submit_test_plan_tool_preserves_blank_expectations(): recorder = ActionsRecorder() diff --git a/libs/hackbot-runtime/tests/test_testrail_handler.py b/libs/hackbot-runtime/tests/test_testrail_handler.py index 2751183864..0d1a08a3d6 100644 --- a/libs/hackbot-runtime/tests/test_testrail_handler.py +++ b/libs/hackbot-runtime/tests/test_testrail_handler.py @@ -17,7 +17,6 @@ def _plan(): { "id": 1, "title": "The PDF opens", - "context": "content", "preconditions": "A PDF is available.", "steps": [ {"action": "Open the PDF", "expectation": None}, @@ -30,7 +29,6 @@ def _plan(): { "id": 2, "title": "The toolbar remains available", - "context": "chrome", "preconditions": None, "steps": [ { diff --git a/services/hackbot-ui/components/FindingsView.tsx b/services/hackbot-ui/components/FindingsView.tsx index e3a981d1b2..e83c57b5ad 100644 --- a/services/hackbot-ui/components/FindingsView.tsx +++ b/services/hackbot-ui/components/FindingsView.tsx @@ -9,6 +9,7 @@ import { isStringArray, titleize, } from "@/lib/findings-format"; +import type { RunAction } from "@/lib/types"; import { Markdown } from "./Markdown"; import { parseTestPlan, TestPlanView } from "./TestPlanView"; @@ -212,12 +213,14 @@ function FriendlyFindings({ findings }: { findings: Record }) { export function FindingsView({ findings, agent, + actions = null, }: { findings: Record; agent: string; + actions?: RunAction[] | null; }) { const testPlan = - agent === "test-plan-generator" ? parseTestPlan(findings) : null; + agent === "test-plan-generator" ? parseTestPlan(findings, actions) : null; // Default to the friendly, readable view; raw JSON is opt-in. const [mode, setMode] = useState("friendly"); diff --git a/services/hackbot-ui/components/RunDetail.tsx b/services/hackbot-ui/components/RunDetail.tsx index b1351d1598..c1f3bcbde2 100644 --- a/services/hackbot-ui/components/RunDetail.tsx +++ b/services/hackbot-ui/components/RunDetail.tsx @@ -162,6 +162,15 @@ export function RunDetail({ runId }: { runId: string }) { const log = extractLog(run); const findings = run.summary?.findings ?? {}; const hasFindings = Object.keys(findings).length > 0; + const hasTestPlanAction = + run.agent === "test-plan-generator" && + Boolean( + actions?.some( + (action) => + action.type === "testrail.submit_test_plan" && + Array.isArray(action.params?.generated_test_cases) + ) + ); // Both pending and failed actions are (re)applied by the apply endpoint — it // skips only already-applied ones — so one button covers applying and retry. @@ -251,7 +260,9 @@ export function RunDetail({ runId }: { runId: string }) { )} - {hasFindings && } + {(hasFindings || hasTestPlanAction) && ( + + )} {actions && actions.length > 0 && (
diff --git a/services/hackbot-ui/components/TestPlanView.tsx b/services/hackbot-ui/components/TestPlanView.tsx index bcaa49c7fe..551cda2458 100644 --- a/services/hackbot-ui/components/TestPlanView.tsx +++ b/services/hackbot-ui/components/TestPlanView.tsx @@ -1,4 +1,5 @@ import { isPlainObject } from "@/lib/findings-format"; +import type { RunAction } from "@/lib/types"; type TestCaseStatus = "passed" | "failed" | "unsuitable"; @@ -53,9 +54,18 @@ function isTestStep(value: TestStep | null): value is TestStep { } export function parseTestPlan( - findings: Record + findings: Record, + actions: RunAction[] | null = null ): TestPlan | null { - const result = findings.result; + // The agent now records the plan straight onto the action, so that is the + // source of truth. Runs from before the switch still carry it in findings. + const testRailAction = actions?.find( + (action) => + action.type === "testrail.submit_test_plan" && + isPlainObject(action.params) && + Array.isArray(action.params.generated_test_cases) + ); + const result = testRailAction?.params ?? findings.result; if (!isPlainObject(result) || !Array.isArray(result.generated_test_cases)) { return null; } From f0cc8ae16a8a376cd8b31bf33fcfeb70c8b2f547 Mon Sep 17 00:00:00 2001 From: John Pangas Date: Wed, 19 Aug 2026 17:54:05 -0600 Subject: [PATCH 2/3] Use the TestPlan View --- .../hackbot_runtime/actions/testrail.py | 2 +- .../hackbot-ui/components/FindingsView.tsx | 15 +------------ services/hackbot-ui/components/RunDetail.tsx | 22 +++++++++---------- .../hackbot-ui/components/TestPlanView.tsx | 3 ++- 4 files changed, 15 insertions(+), 27 deletions(-) diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py index 65bb41022c..e429dd229c 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py @@ -85,7 +85,7 @@ def case_ids_must_be_sequential(self) -> "SubmitTestPlanInput": case_ids = [case.id for case in self.generated_test_cases] expected_ids = list(range(1, len(self.generated_test_cases) + 1)) if case_ids != expected_ids: - raise ValueError("test case ids must be sequential starting at 1") + raise ToolError("test case ids must be sequential starting at 1") return self diff --git a/services/hackbot-ui/components/FindingsView.tsx b/services/hackbot-ui/components/FindingsView.tsx index e83c57b5ad..bd3a3c2519 100644 --- a/services/hackbot-ui/components/FindingsView.tsx +++ b/services/hackbot-ui/components/FindingsView.tsx @@ -9,9 +9,7 @@ import { isStringArray, titleize, } from "@/lib/findings-format"; -import type { RunAction } from "@/lib/types"; import { Markdown } from "./Markdown"; -import { parseTestPlan, TestPlanView } from "./TestPlanView"; type ViewMode = "friendly" | "raw"; @@ -212,16 +210,9 @@ function FriendlyFindings({ findings }: { findings: Record }) { export function FindingsView({ findings, - agent, - actions = null, }: { findings: Record; - agent: string; - actions?: RunAction[] | null; }) { - const testPlan = - agent === "test-plan-generator" ? parseTestPlan(findings, actions) : null; - // Default to the friendly, readable view; raw JSON is opt-in. const [mode, setMode] = useState("friendly"); return ( @@ -250,11 +241,7 @@ export function FindingsView({
{mode === "friendly" ? ( - testPlan ? ( - - ) : ( - - ) + ) : ( 
{JSON.stringify(findings, null, 2)}
)} diff --git a/services/hackbot-ui/components/RunDetail.tsx b/services/hackbot-ui/components/RunDetail.tsx index c1f3bcbde2..68b9a67faa 100644 --- a/services/hackbot-ui/components/RunDetail.tsx +++ b/services/hackbot-ui/components/RunDetail.tsx @@ -15,6 +15,7 @@ import { import { FindingsView } from "./FindingsView"; import { Markdown } from "./Markdown"; import { StatusBadge } from "./StatusBadge"; +import { parseTestPlan, TestPlanView } from "./TestPlanView"; // Proposed bugzilla.add_comment actions carry the comment body in params.text; // pull it out so we can preview what would be posted to the bug. @@ -162,15 +163,12 @@ export function RunDetail({ runId }: { runId: string }) { const log = extractLog(run); const findings = run.summary?.findings ?? {}; const hasFindings = Object.keys(findings).length > 0; - const hasTestPlanAction = - run.agent === "test-plan-generator" && - Boolean( - actions?.some( - (action) => - action.type === "testrail.submit_test_plan" && - Array.isArray(action.params?.generated_test_cases) - ) - ); + // The QA agent gets its own purpose-built view; its plan lives on the + // TestRail action, so findings are usually empty (raw data is in summary.json). + const testPlan = + run.agent === "test-plan-generator" + ? parseTestPlan(findings, actions) + : null; // Both pending and failed actions are (re)applied by the apply endpoint — it // skips only already-applied ones — so one button covers applying and retry. @@ -260,8 +258,10 @@ export function RunDetail({ runId }: { runId: string }) { )} - {(hasFindings || hasTestPlanAction) && ( - + {testPlan ? ( + + ) : ( + hasFindings && )} {actions && actions.length > 0 && ( diff --git a/services/hackbot-ui/components/TestPlanView.tsx b/services/hackbot-ui/components/TestPlanView.tsx index 551cda2458..044a622dec 100644 --- a/services/hackbot-ui/components/TestPlanView.tsx +++ b/services/hackbot-ui/components/TestPlanView.tsx @@ -128,7 +128,8 @@ export function TestPlanView({ testPlan }: { testPlan: TestPlan }) { ); return ( -
+
+

Test plan

{testPlan.feature && (

Feature name

From f5870db0ff4ace956867caab6183ecdf2ca10ca5 Mon Sep 17 00:00:00 2001 From: John Pangas Date: Wed, 19 Aug 2026 18:18:34 -0600 Subject: [PATCH 3/3] Fix failing test --- libs/hackbot-runtime/tests/test_testrail_action.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/hackbot-runtime/tests/test_testrail_action.py b/libs/hackbot-runtime/tests/test_testrail_action.py index e6232ed023..4c17ed077c 100644 --- a/libs/hackbot-runtime/tests/test_testrail_action.py +++ b/libs/hackbot-runtime/tests/test_testrail_action.py @@ -95,7 +95,7 @@ async def test_submit_test_plan_tool_rejects_non_sequential_case_ids(): generated_test_cases=cases, ) - assert "invalid TestRail submission" in str(exc.value) + assert "test case ids must be sequential starting at 1" in str(exc.value) assert recorder.actions == []