You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
T5-P1-A5-WP1 Provide explicit regression coverage for the fail-closed validity guard added to the _is_deferred_recall=True branch of open_kitchen, preventing future regressions from silently re-opening a succes... #4002
Provide explicit regression coverage for the fail-closed validity guard added to the _is_deferred_recall=True branch of open_kitchen, preventing future regressions from silently re-opening a success:true path on invalid or empty recipe content.
Assignment: P1-A5 (C4: Deferred-recall open_kitchen fail-closed test — mock load_and_validate returning invalid/empty on the _is_deferred_recall branch, assert success:false envelope)
Depends on: None
Depended on by: None
Deliverables
tests/server/test_open_kitchen_deferred_recall.py — 2-3 new failure-path test functions covering valid:False + empty content, valid:False + non-empty content, and optionally absent valid key
Technical Steps
Read the guard block inserted by PR Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995 into the _is_deferred_recall=True branch of open_kitchen (tools_kitchen.py lines ~605-620 on that branch): if not result.get('valid', False) or not result.get('content', ''): returns an inline JSON envelope with success:False, kitchen:failed, stage:recipe_validation, errors list, and suggestions list.
In test_open_kitchen_deferred_recall.py, add test function test_deferred_recall_fails_closed_when_valid_false_empty_content: use _make_deferred_recall_ctx('test-recipe'), set load_and_validate.return_value to {valid: False, content: '', errors: ['structural error A'], suggestions: [], requires_packs: [], requires_features: [], content_hash: 'abc', composite_hash: 'def', recipe_version: '1.0'}, patch autoskillit.server._get_ctx, await open_kitchen(name='test-recipe', ctx=mock_ctx), parse JSON result, assert parsed['success'] is False, parsed['kitchen'] == 'failed', parsed['stage'] == 'recipe_validation'.
Add test function test_deferred_recall_fails_closed_when_valid_false_nonempty_content: identical setup but set content to a non-empty string such as 'some content', keeping valid: False; assert the same three envelope fields — verifying the guard fires on invalid flag alone regardless of content presence.
Optionally add a third test test_deferred_recall_fails_closed_when_valid_missing: omit the 'valid' key entirely from the mock return dict (content non-empty) to assert the guard treats absent valid as False — covering the default=False defensive branch in result.get('valid', False).
Ensure all new tests carry @pytest.mark.anyio and are grouped with the existing pytestmark = [pytest.mark.layer('server'), pytest.mark.small] at module level.
Do not add any imports beyond those already present in the test file (json, MagicMock, patch, pytest are all present).
Verify the assertions cover errors passthrough: assert 'recipe_validation' == parsed['stage'] and optionally assert parsed.get('errors') == ['structural error A'] to confirm the errors list is forwarded in the envelope.
Acceptance Criteria
task test-check passes with the new tests included and no existing tests broken.
test_deferred_recall_fails_closed_when_valid_false_empty_content: mocking load_and_validate to return {valid: False, content: '', errors: ['structural error A'], suggestions: []} causes open_kitchen to return a JSON envelope where success is False, kitchen is 'failed', and stage is 'recipe_validation'.
test_deferred_recall_fails_closed_when_valid_false_nonempty_content: mocking load_and_validate to return {valid: False, content: 'non-empty content', errors: ['structural error B'], suggestions: []} produces the same failure envelope fields.
No test in the file calls load_and_validate with side_effect=Exception to reach the except-path; all new tests exercise the post-return guard path only.
All new tests use _make_deferred_recall_ctx() (gate.enabled=True, recipe_name matches the name argument) so _is_deferred_recall evaluates to True inside open_kitchen.
The mock return value in each new test includes 'suggestions': [] alongside 'errors' so the guard code's result.get('suggestions', []) does not raise and the forwarded envelope is complete.
Each new test is decorated with @pytest.mark.anyio and inherits the module-level pytestmark layer('server') and small markers.
Goal
Provide explicit regression coverage for the fail-closed validity guard added to the _is_deferred_recall=True branch of open_kitchen, preventing future regressions from silently re-opening a success:true path on invalid or empty recipe content.
Context
Deliverables
tests/server/test_open_kitchen_deferred_recall.py — 2-3 new failure-path test functions covering valid:False + empty content, valid:False + non-empty content, and optionally absent valid keyTechnical Steps
if not result.get('valid', False) or not result.get('content', ''):returns an inline JSON envelope with success:False, kitchen:failed, stage:recipe_validation, errors list, and suggestions list.test_deferred_recall_fails_closed_when_valid_false_empty_content: use _make_deferred_recall_ctx('test-recipe'), set load_and_validate.return_value to {valid: False, content: '', errors: ['structural error A'], suggestions: [], requires_packs: [], requires_features: [], content_hash: 'abc', composite_hash: 'def', recipe_version: '1.0'}, patch autoskillit.server._get_ctx, await open_kitchen(name='test-recipe', ctx=mock_ctx), parse JSON result, assert parsed['success'] is False, parsed['kitchen'] == 'failed', parsed['stage'] == 'recipe_validation'.test_deferred_recall_fails_closed_when_valid_false_nonempty_content: identical setup but set content to a non-empty string such as 'some content', keeping valid: False; assert the same three envelope fields — verifying the guard fires on invalid flag alone regardless of content presence.test_deferred_recall_fails_closed_when_valid_missing: omit the 'valid' key entirely from the mock return dict (content non-empty) to assert the guard treats absent valid as False — covering the default=False defensive branch in result.get('valid', False).Acceptance Criteria