Skip to content

Integration tool tests assert against a superseded flat return shape (29 failed, 11 errors) #433

Description

@jeremymanning

Symptom

live-testsIntegration tests (local services) fails:

29 failed, 12 passed, 36 skipped, 3171 deselected, 11 errors in 150.59s

Run: https://github.com/ContextLab/orchestrator/actions/runs/30681083746

Failures are almost all KeyError on a key that does exist — just one level down:

TestTerminalTool::test_working_directory      - KeyError: 'stdout'
TestFileSystemTool::test_file_operations      - KeyError: 'content'
TestFileSystemTool::test_directory_operations - KeyError: 'items'
TestDataProcessingTool::test_json_processing  - KeyError: 'original_count'
TestDataProcessingTool::test_csv_processing   - KeyError: 'target_format'
TestPDFCompilerTool::test_pdf_generation      - KeyError: 'file_size'
TestToolIntegration::test_file_processing_pipeline - KeyError: 'valid'

Root cause (verified, not inferred)

Tools return a {success, result, error} envelope; the tests index the
envelope as though it were the payload. The report-generator assertion shows
both shapes in one line:

assert 'markdown' in {'result': {'markdown': '# AI Applications Report...',
                                 'word_count': 123, ...},
                      'success': True, 'error': None}

Confirmed directly against TerminalTool:

r = await TerminalTool().execute(command="echo hi")
sorted(r.keys())            # ['error', 'result', 'success']
'stdout' in r               # False
sorted(r['result'].keys())  # ['command', 'execution_time', 'return_code',
                            #  'stderr', 'stdout', 'working_dir']

So tests/integration/test_tools_real_world.py encodes a flat contract
that the tools no longer implement.

The decision this needs

Which contract is correct? These are not interchangeable:

  1. The envelope is intended (it matches the documented resilience layer —
    "user-friendly messages through resilience layer"). Then these tests are
    stale and must be updated to unwrap ['result'].
  2. The flat shape is intended and the envelope is a regression. Then the
    tools must be fixed, and this is a much larger, user-visible break.

I did not guess. Changing 29 tests to match current behaviour would be
exactly the "weaken the test until it passes" move the contributing guide
forbids, if (2) turns out to be the truth. Someone needs to state which shape
is the contract, ideally by writing it down in the ADR so this cannot drift
again.

Note this failure predates the current security work — it is visible in the
review of the recovery branch and is unrelated to the Dartmouth changes. The
11 additional errors in the same run have not been separately triaged.

Not covered here

test_web_scraping_pipeline fails differently (Task 'search_web' failed and policy is 'fail') and may be a genuine network/tool issue rather than the
shape mismatch.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions