Skip to content

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

Description

@Trecek

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

  • Phase: Hook Efficacy Probe + Fail-Closed Completions (Milestone: 1-hook-efficacy-probe-fail-closed-completions)
  • 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

  1. 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.
  2. Confirm that LoadRecipeResult (recipe/_recipe_ingredients.py line 110) after PR Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995 includes an 'errors' field (list of strings surfaced from validate_recipe_structure / _validate_no_dangling_routes), so the mock dict must include that key.
  3. 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'.
  4. 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.
  5. 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).
  6. 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.
  7. Do not add any imports beyond those already present in the test file (json, MagicMock, patch, pytest are all present).
  8. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    recipe:implementationRoute: proceed directly to implementationstagedImplementation staged and waiting for promotion to main

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions