Skip to content

Nightly regression: tests/integration/test_sdlc_multi_lineage.py::TestConcurrentMultiLineageContention::test_concurrent_bare_ensures_mint_exactly_one_owner #3323

Description

@valorengels

Failing node

tests/integration/test_sdlc_multi_lineage.py::TestConcurrentMultiLineageContention::test_concurrent_bare_ensures_mint_exactly_one_owner

Surfaced by nightly regression triage (lane nightly-triage-f7ec48e1).

Observed failure

Reproduced locally with ./scripts/pytest-clean.sh <node>:

tests/integration/test_sdlc_multi_lineage.py:85: in test_concurrent_bare_ensures_mint_exactly_one_owner
    assert len(winners) == 1, f"exactly one lineage must mint; got {results!r}"
E   AssertionError: exactly one lineage must mint; got [{'blocked': True, 'reason': 'ISSUE_LOCKED', 'owner_run_id': 'ca7dd5528c4549c5a49a0eff70431ab9', 'owner_session_id': 'sdlc-local-92026', 'orphaned_lock': False}, {'blocked': True, 'reason': 'ISSUE_LOCKED', 'owner_run_id': 'ca7dd5528c4549c5a49a0eff70431ab9', 'owner_session_id': 'sdlc-local-92026', 'orphaned_lock': False}, {'error': 'RUN_BIND_FAILED', 'reason': 'post-save readback mismatch', 'session_id': 'sdlc-local-92026'}, {'error': 'RUN_BIND_FAILED', 'reason': 'post-save readback mismatch', 'session_id': 'sdlc-local-92026'}]
E   assert 0 == 1
E    +  where 0 = len([])

Two of the four racing lineages lose the SET NX contest and are correctly refused with ISSUE_LOCKED; the two that acquire the lock both fail the post-save readback and release it, so winners is empty.

Likely cause (shared across all four nodes in this class)

Stale test double, not broken production behavior. The readback in _acquire_run_lock_and_bind was migrated to a resolver the test's AgentSession mock does not stub.

  • tools/sdlc_session_ensure.py:613-647 — after saving active_run_id, the bind does a post-save readback:
    from models.agent_session import AgentSession
    fresh = AgentSession.newest_for_session_id(session_id)
    readback_run_id = getattr(fresh, "active_run_id", None) if fresh is not None else None
    ...
    if readback_run_id != candidate:
        release_issue_lock(issue_number, candidate)
        return None, {"error": "RUN_BIND_FAILED", "reason": "post-save readback mismatch", ...}
  • tests/integration/test_sdlc_multi_lineage.py:36-40 — the test's _readback_as() helper stubs only the old access path:
    def _readback_as(session: MagicMock) -> MagicMock:
        mock_as = MagicMock()
        mock_as.query.filter.return_value = [session]
        return mock_as
    newest_for_session_id is never configured, so it returns an auto-generated MagicMock, whose .active_run_id is another auto-MagicMock that can never equal candidate. Every bind therefore fails the readback, releases the lock via compare-and-delete, and returns RUN_BIND_FAILED — so no lineage ever mints and no lease is ever held.
  • The readback moved to AgentSession.newest_for_session_id (models/agent_session.py:1232-1235) in commit 3c77e1eab ("AgentSession: one newest-wins resolver for every session_id read"). The mock helper was not updated with it.

All four tests in TestConcurrentMultiLineageContention share this single cause; they fail at four different assertions because each consumes the broken result differently.

A secondary, independent weakness this exposed: assert not supervisor.get("blocked") (lines 105, 122, 142) is satisfied by a RUN_BIND_FAILED dict, which carries error but no blocked key. The guard passes on an error result and the test then dies on a bare KeyError: 'run_id' instead of reporting the actual failure.

Suggested next steps

  1. Fix _readback_as (tests/integration/test_sdlc_multi_lineage.py:36-40) to stub the resolver the code under test actually calls — configure mock_as.newest_for_session_id.return_value = session (keeping query.filter only if some other path still needs it). Because session.active_run_id is assigned by the bind before the readback, a mock returning the same object naturally reads back the candidate.
  2. Strengthen the supervisor guards at lines 105, 122 and 142 to assert not supervisor.get("blocked") and not supervisor.get("error"), supervisor so an error result fails loudly at its origin instead of as a downstream KeyError.
  3. Consider whether the readback deserves a mock-independent guard: this class is the acceptance test for exactly-one-owner semantics (Reliability risk: /do-sdlc fork vs supervisor — forked executions merge past blocked gates, record unearned/contradictory verdicts, and lease TTL churn deadlocks the router #2026 WS1), and it silently stopped exercising the lock contest the moment the mock drifted — every lineage was refused for the wrong reason while the assertions still looked meaningful.

Related

All four nodes in TestConcurrentMultiLineageContention failed in the same nightly batch and share the root cause above:

  • test_concurrent_bare_ensures_mint_exactly_one_owner
  • test_second_lineage_inherits_owner_run_id_via_named_refusal
  • test_release_frees_issue_for_a_fresh_lineage
  • test_non_owner_lineage_cannot_pass_single_owner_merge_gate

Triage 2026-09-15

Status: confirmed still failing on main 205344717. This issue now owns the full cluster (duplicates closed into it).

Single-line hotfix: Yes, and it fixes all four nodes with one change. _readback_as() in tests/integration/test_sdlc_multi_lineage.py:34-37 only stubs mock_as.query.filter.return_value = [session], but _acquire_run_lock_and_bind's post-save readback (tools/sdlc_session_ensure.py:617) calls AgentSession.newest_for_session_id(session_id) instead — an unstubbed method on the patched class, so it returns an unrelated MagicMock whose active_run_id never matches the just-written candidate. That readback "mismatch" makes every bind look like a Race-3 failure: RUN_BIND_FAILED is returned and the lock is released via compare-and-delete. This explains both reported symptoms as one root cause — the KeyError on result["run_id"] (a RUN_BIND_FAILED dict has no run_id key and no blocked key, so assert not result.get("blocked") even passes silently), and the refusal-string mismatch (once the lock is released, _check_lease_ownership sees no owner at all and takes the "no issue lease held for #N" branch instead of the "does not hold the issue lease" branch it hits when a genuine owner exists). Fix: add mock_as.newest_for_session_id.return_value = session inside _readback_as. No separate fix needed for the refusal-string node.

Reduce-complexity option: The concurrency being exercised (real touch_issue_lock / Redis lock contention) is genuine and worth keeping mocked-minimally. But the AgentSession double is a hand-rolled partial stand-in for the whole class, and it silently drifts whenever production adds a new read path (as happened here with newest_for_session_id) — the mock becomes the thing under test instead of a faithful double. Recommend replacing the blanket patch("models.agent_session.AgentSession", MagicMock()) with a small fake object that implements exactly the methods ensure_session's call graph uses (rows_for_session_id, newest_for_session_id) and always returns the shared session, so any future new read path raises AttributeError loudly instead of returning a plausible-looking wrong value.

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

    testingRelated to the test suite (tests/)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions