From 9e3d8bd7f55eed2f61d324566f4f956d493a0295 Mon Sep 17 00:00:00 2001 From: John Pangas Date: Wed, 19 Aug 2026 02:51:51 -0600 Subject: [PATCH 1/3] Report each TestRail case result with the case itself Execution results travelled as a parallel `results` list keyed by case id, so reading a case meant looking up its outcome somewhere else, and the two lists could disagree about which cases exist. Nest the result inside each generated test case instead. Every case now carries a required `result` with its status, summary and failure reason, which makes an unexecuted case unrepresentable rather than merely absent from a second list. The run-wide write-up moves to a `summary` on the action, where it describes the plan as a whole instead of competing with the per-case summaries. The UI reads the nested result and still falls back to the old `results` list so previously recorded runs render unchanged. Fixes #6511 --- .../test_plan_generator/prompts/system.md | 18 ++++-- .../hackbot_runtime/actions/testrail.py | 51 +++++++++++++-- .../tests/test_testrail_action.py | 63 +++++++++++++++++++ .../tests/test_testrail_handler.py | 46 +++----------- .../hackbot-ui/components/TestPlanView.tsx | 61 +++++++++--------- 5 files changed, 163 insertions(+), 76 deletions(-) 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 bab8eee10f..edfefa21f5 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 @@ -12,9 +12,13 @@ changes. 2. Each test case must have: - A title. - Ordered test steps, each with an `action` and optional `expectation`. + - After execution, one nested `result` containing the case status, a concise + summary, and any failure reason. 3. Run the generated cases and steps in order. 4. Record one final TestRail action with `testrail_submit_test_plan`. - Use the provided feature name as the action feature. + - Include the execution result inside each generated test case. + - Set `summary` to a short overview of how the run went as a whole. ## Context guidance @@ -35,8 +39,7 @@ bypass a failing content interaction. - Call only the tools needed for the current step. - If a step fails, mark that step failed, mark the case failed, stop that case, and move to the next case. -- When a step fails, include a concise failure reason based only on observed - behavior. +- When a step fails, include the observed behavior in the case result summary. - When a case fails or is unsuitable, include a concise case-level reason. - Do not try alternate approaches to make a failing step pass. @@ -68,8 +71,11 @@ Mark a case as `unsuitable` only if it requires: ## Reporting -Record the generated test plan through `testrail_submit_test_plan` exactly once. -A prose message is not enough. +Record the generated test plan and execution outcomes through +`testrail_submit_test_plan` exactly once. A prose message is not enough. Include +one nested `result` for every generated test case. -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. +Write the overall write-up once, in the action's `summary`: which cases passed, +failed, or were unsuitable, with concise observations for the failed and +unsuitable ones. It becomes the description of the TestRail run, so it is what a +QA engineer reads first. diff --git a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py index e429dd229c..72fe1d9286 100644 --- a/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py +++ b/libs/hackbot-runtime/hackbot_runtime/actions/testrail.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import Annotated, Any +from typing import Annotated, Any, Literal from agent_tools.registry import ToolError, tool, tools_in from pydantic import ( @@ -26,6 +26,23 @@ class TestRailStepInput(BaseModel): ) +class TestRailCaseResultInput(BaseModel): + status: Literal["passed", "failed", "unsuitable"] + summary: str + failure_reason: str | None = Field( + default=None, + description="Required when status is failed or unsuitable.", + ) + + @model_validator(mode="after") + def failure_reason_required_for_non_passing_cases( + self, + ) -> "TestRailCaseResultInput": + if self.status in {"failed", "unsuitable"} and not self.failure_reason: + raise ValueError("failed or unsuitable cases must include failure_reason") + return self + + class TestRailCaseInput(BaseModel): id: int title: str = Field(description="TestRail test case title.") @@ -39,6 +56,11 @@ class TestRailCaseInput(BaseModel): "and an optional expectation." ), ) + result: TestRailCaseResultInput = Field( + description=( + "Execution result for this generated test case after the agent ran it." + ) + ) @field_validator("title") @classmethod @@ -71,6 +93,10 @@ class SubmitTestPlanInput(BaseModel): max_length=30, description="Generated test cases to upload to TestRail.", ) + summary: str | None = Field( + default=None, + description="Optional summary of the generated test-plan execution.", + ) @field_validator("feature") @classmethod @@ -93,10 +119,19 @@ def _confirm(recorder: ActionsRecorder, action_type: str) -> str: return f"Recorded {action_type} (#{len(recorder.actions) - 1})." -def _validated_params(feature: str, generated_test_cases: list[Any]) -> dict[str, Any]: +def _validated_params( + feature: str, + generated_test_cases: list[Any], + *, + summary: str | None = None, +) -> dict[str, Any]: try: validated = SubmitTestPlanInput.model_validate( - {"feature": feature, "generated_test_cases": generated_test_cases} + { + "feature": feature, + "generated_test_cases": generated_test_cases, + "summary": summary, + } ) except ValidationError as exc: raise ToolError( @@ -128,6 +163,10 @@ async def submit_test_plan( description="Generated test cases to upload together to TestRail.", ), ], + summary: Annotated[ + str | None, + Field(description="Short overview of how the run went as a whole."), + ] = None, ) -> str: """Record a generated test plan for deferred TestRail submission. @@ -141,7 +180,11 @@ async def submit_test_plan( "a test plan is already recorded for this run; do not call " "submit_test_plan again" ) - params = _validated_params(feature, generated_test_cases) + params = _validated_params( + feature, + generated_test_cases, + summary=summary, + ) recorder.record(ACTION_TYPE, params) return _confirm(recorder, ACTION_TYPE) diff --git a/libs/hackbot-runtime/tests/test_testrail_action.py b/libs/hackbot-runtime/tests/test_testrail_action.py index 4c17ed077c..205bfaa803 100644 --- a/libs/hackbot-runtime/tests/test_testrail_action.py +++ b/libs/hackbot-runtime/tests/test_testrail_action.py @@ -15,6 +15,10 @@ def _cases(): "steps": [ {"action": "Open the PDF", "expectation": "The PDF is displayed."} ], + "result": { + "status": "passed", + "summary": "Worked.", + }, } ] @@ -38,8 +42,14 @@ async def test_submit_test_plan_tool_records_deferred_action(): "steps": [ {"action": "Open the PDF", "expectation": "The PDF is displayed."} ], + "result": { + "status": "passed", + "summary": "Worked.", + "failure_reason": None, + }, } ], + "summary": None, } @@ -55,6 +65,7 @@ async def test_submit_test_plan_tool_rejects_invalid_input(): "id": 1, "title": "Case", "steps": [], + "result": {"status": "passed", "summary": "Worked."}, } ], ) @@ -75,6 +86,7 @@ async def test_submit_test_plan_tool_rejects_cases_without_expectation(): "id": 1, "title": "Case", "steps": [{"action": "Open the PDF", "expectation": None}], + "result": {"status": "passed", "summary": "Worked."}, } ], ) @@ -144,6 +156,7 @@ async def test_submit_test_plan_tool_preserves_blank_expectations(): {"action": "Open the PDF", "expectation": ""}, {"action": "Select text", "expectation": "Text is selected."}, ], + "result": {"status": "passed", "summary": "Worked."}, } ], ) @@ -154,5 +167,55 @@ async def test_submit_test_plan_tool_preserves_blank_expectations(): ] +async def test_submit_test_plan_tool_records_execution_results(): + recorder = ActionsRecorder() + + await testrail.submit_test_plan( + recorder, + feature="Feature", + generated_test_cases=_cases(), + summary="All executable cases passed.", + ) + + assert recorder.actions[0]["params"]["generated_test_cases"][0]["result"] == { + "status": "passed", + "summary": "Worked.", + "failure_reason": None, + } + assert recorder.actions[0]["params"]["summary"] == "All executable cases passed." + + +async def test_submit_test_plan_tool_rejects_missing_case_result(): + recorder = ActionsRecorder() + cases = _cases() + del cases[0]["result"] + + 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_not_run_results(): + recorder = ActionsRecorder() + cases = _cases() + cases[0]["result"] = {"status": "not_run", "summary": "Not run."} + + 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 == [] + + def test_submit_test_plan_handler_is_registered(): assert isinstance(get_handler(ACTION_TYPE), SubmitTestPlanHandler) diff --git a/libs/hackbot-runtime/tests/test_testrail_handler.py b/libs/hackbot-runtime/tests/test_testrail_handler.py index 9fae0d8f56..3640b65673 100644 --- a/libs/hackbot-runtime/tests/test_testrail_handler.py +++ b/libs/hackbot-runtime/tests/test_testrail_handler.py @@ -25,6 +25,11 @@ def _plan(): "expectation": "Text selection is highlighted in the PDF.", }, ], + "result": { + "status": "passed", + "summary": "The PDF behaved as expected.", + "failure_reason": None, + }, }, { "id": 2, @@ -36,42 +41,11 @@ def _plan(): "expectation": "The toolbar remains visible and usable.", }, ], - }, - ], - "results": [ - { - "id": 1, - "status": "passed", - "summary": "The PDF behaved as expected.", - "failure_reason": None, - "step_results": [ - { - "step_number": 1, - "status": "passed", - "observation": "The PDF opened.", - "failure_reason": None, - }, - { - "step_number": 2, - "status": "passed", - "observation": "Text was selected.", - "failure_reason": None, - }, - ], - }, - { - "id": 2, - "status": "unsuitable", - "summary": "The toolbar could not be inspected.", - "failure_reason": "No available tool can inspect it.", - "step_results": [ - { - "step_number": 1, - "status": "not_run", - "observation": "Not run.", - "failure_reason": None, - } - ], + "result": { + "status": "unsuitable", + "summary": "The toolbar could not be inspected.", + "failure_reason": "No available tool can inspect it.", + }, }, ], "summary": "One passed and one was unsuitable.", diff --git a/services/hackbot-ui/components/TestPlanView.tsx b/services/hackbot-ui/components/TestPlanView.tsx index 044a622dec..2d5d5ba41f 100644 --- a/services/hackbot-ui/components/TestPlanView.tsx +++ b/services/hackbot-ui/components/TestPlanView.tsx @@ -8,6 +8,7 @@ interface GeneratedTestCase { title: string; preconditions: string | null; steps: TestStep[]; + result: TestCaseResult | null; } interface TestStep { @@ -25,7 +26,6 @@ interface TestCaseResult { export interface TestPlan { feature: string; generatedTestCases: GeneratedTestCase[]; - results: TestCaseResult[]; summary: string; } @@ -53,12 +53,23 @@ function isTestStep(value: TestStep | null): value is TestStep { return value !== null; } +function parseCaseResult(value: unknown, id: number): TestCaseResult | null { + if (!isPlainObject(value) || !isStatus(value.status)) { + return null; + } + return { + id, + status: value.status, + summary: typeof value.summary === "string" ? value.summary : "", + failureReason: + typeof value.failure_reason === "string" ? value.failure_reason : null, + }; +} + export function parseTestPlan( findings: Record, actions: RunAction[] | null = null ): TestPlan | null { - // 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" && @@ -70,6 +81,18 @@ export function parseTestPlan( return null; } + const legacyResultsById = new Map(); + if (Array.isArray(result.results)) { + for (const value of result.results) { + if (isPlainObject(value) && typeof value.id === "number") { + const parsed = parseCaseResult(value, value.id); + if (parsed) { + legacyResultsById.set(value.id, parsed); + } + } + } + } + const generatedTestCases: GeneratedTestCase[] = []; for (const value of result.generated_test_cases) { if ( @@ -90,43 +113,21 @@ export function parseTestPlan( preconditions: typeof value.preconditions === "string" ? value.preconditions : null, steps, + result: + parseCaseResult(value.result, value.id) ?? + legacyResultsById.get(value.id) ?? + null, }); } - const results: TestCaseResult[] = []; - if (Array.isArray(result.results)) { - for (const value of result.results) { - if ( - isPlainObject(value) && - typeof value.id === "number" && - isStatus(value.status) - ) { - results.push({ - id: value.id, - status: value.status, - summary: typeof value.summary === "string" ? value.summary : "", - failureReason: - typeof value.failure_reason === "string" - ? value.failure_reason - : null, - }); - } - } - } - return { feature: typeof result.feature === "string" ? result.feature : "", generatedTestCases, - results, summary: typeof result.summary === "string" ? result.summary : "", }; } export function TestPlanView({ testPlan }: { testPlan: TestPlan }) { - const resultsById = new Map( - testPlan.results.map((result) => [result.id, result]) - ); - return (

Test plan

@@ -139,7 +140,7 @@ export function TestPlanView({ testPlan }: { testPlan: TestPlan }) {

Test cases

    {testPlan.generatedTestCases.map((testCase) => { - const result = resultsById.get(testCase.id); + const result = testCase.result; const failureReason = result && result.status !== "passed" ? result.failureReason || result.summary From 7588cbce8bace31c0ba403b4c27fd7fc580d7dfe Mon Sep 17 00:00:00 2001 From: John Pangas Date: Wed, 19 Aug 2026 03:36:49 -0600 Subject: [PATCH 2/3] Name the case statuses in the test-plan-generator prompt The opening paragraph described the agent as recording a test plan, dropping the reporting half of the job that the old "report only pass/fail/unsuitable results" line carried. The status vocabulary is still enforced by the tool schema, which exposes status as an enum and rejects anything else, so this is not a correctness fix. It just means the first thing the agent reads describes what it actually does. --- .../hackbot_agents/test_plan_generator/prompts/system.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 edfefa21f5..1ba195f7e3 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,8 +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 -record the generated test plan for TestRail. Do not try to fix, patch or make -changes. +record the generated test plan for TestRail, each case carrying its `passed`, +`failed` or `unsuitable` result. Do not try to fix, patch or make changes. ## Required workflow From 3c1c408d3d7770daebad461980ea5fcf7bad3279 Mon Sep 17 00:00:00 2001 From: John Pangas Date: Fri, 21 Aug 2026 17:59:00 -0600 Subject: [PATCH 3/3] Stop asking the agent to mark steps failed, since steps carry no result The step-level `StepResult` model went away with result.py in #6657, so `TestRailStepInput` now holds only an action and an expectation. The execution rules still told the agent to mark a failing step, which it has no field to do. Point that failure at the case result instead, and have the agent name the offending step in the summary so the attribution the step status used to carry is not lost. --- .../hackbot_agents/test_plan_generator/prompts/system.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 1ba195f7e3..c65e4db8d1 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 @@ -37,9 +37,10 @@ bypass a failing content interaction. - Do not skip, reorder, combine, or rewrite steps after generation. - Call only the tools needed for the current step. -- If a step fails, mark that step failed, mark the case failed, stop that case, - and move to the next case. -- When a step fails, include the observed behavior in the case result summary. +- If a step fails, mark the case failed, stop that case, and move to the next + case. +- When a step fails, name the step and include the observed behavior in the + case result summary. - When a case fails or is unsuitable, include a concise case-level reason. - Do not try alternate approaches to make a failing step pass.