Skip to content

Remove full server contexts from audit-only headless tests #4959

Description

@Trecek

Tracking issue: #4956#4956

Suggested implementation order: 3 of 3 (soft sequence; see tracker).

Outcome

Keep the headless result/audit tests and every assertion, but stop building a full server context where it is unused or where the test needs only the real in-memory audit log.

This is the third recommended implementation slice in the local-test-performance tracking effort. It is intentionally limited to eleven tests in one file; it is not a context-factory redesign.

Investigation

At snapshot 5f49d3761, source inspection and two independent adversarial reviewers verified the following. These target paths were unchanged through filing checkout 94e3d18e0.

Three unused fixture requests

In tests/execution/test_headless_core.py, these tests request tool_ctx but never read it:

  • test_success_empty_completed_returns_needs_retry_true
  • test_build_skill_result_subtype_never_contradicts_success
  • test_build_skill_result_channel_b_empty_stdout_is_retriable

Remove only their unused fixture parameters.

Eight audit-only requests

test_success_empty_completed_subtype_captured_in_audit_log and the seven tests in TestFailureCaptureInBuildSkillResult use only tool_ctx.audit:

  • nonzero-exit capture;
  • clean-success absence;
  • recorded skill command;
  • timestamp;
  • stale termination;
  • retry indication;
  • stderr truncation.

Supply a fresh real DefaultAuditLog per test, directly or through a small function-scoped fixture. Do not share a log across tests and do not replace its behavior with a mock.

Why the cost is unnecessary

make_tool_ctx calls the full server factory, recovers a real audit-admission ledger, probes an ephemeral session root, constructs services, writes ten local fixture SKILL.md files, and patches server state.

The server factory already uses DefaultAuditLog. That class is the production in-memory implementation, not a fake merely because it stores records in a list. The relevant failure writer uses the passed audit protocol, not the server singleton.

Consequently this slice avoids 11 full factory calls, 11 audit-ledger recoveries, and 110 skill-file writes. Those are supported work counts, not measured elapsed-time savings.

Using minimal_ctx is not the best substitute: it still performs ledger recovery. The narrow tests need the audit object itself.

Coverage and isolation boundaries

  • Retain all eleven tests and their current assertions.
  • Use the real log so record construction, truncation, report copying, and retry behavior remain exercised.
  • Keep normal root environment/home/logging fixtures in place.
  • Do not remove tool_ctx from actual server routing, recipe initialization, run_headless_core, or context-factory tests.
  • Do not modify make_context, ledger implementation, factory service wiring, or server-global cleanup.
  • The direct audit makes an accidental global-audit dependency easier to catch: writing to _state._ctx.audit instead of the explicitly passed audit must not satisfy these assertions.

Acceptance and validation

  • The three unused tests no longer request a full context.
  • The eight audit-only tests use isolated real logs and retain their assertions.
  • No target case depends on full factory/ledger/fixture-skill setup after the change.
  • Repeated and parallel runs do not share failure history or change retry results.
  • Existing context/factory and server-routing tests continue to exercise their original paths.
  • Run the focused existing headless tests through approved task targets, then required task test-check validation. Do not invoke pytest directly.
  • Record operation reduction and, when a quiet host is available, controlled elapsed-time results without claiming a suite-wide percentage.

Estimated impact

A small, low-risk runtime win, plausibly sub-second to a few seconds depending on scope and cache state; no clean timing estimate has been measured. The maintenance benefit is clearer: the tests state their real collaborator and stop depending on unrelated server construction.

Implementation order / related work

Recommended third, though it can be developed independently of the first two tickets. The order is soft, for clean measurements and manageable review, not a hard blocker.

#927 introduced minimal_ctx; this ticket does not redo that restructuring. Open #4819 concerns server recipe-cache fixtures and is a different scope. Doctor check narrowing is deliberately deferred because config/backend and aggregate-check behavior need separate validation.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions