Skip to content

bug(tests): api.terminal.router enumerates zero routes under python-suite shard 12/12 (unidentified sys.modules pollution) #15087

Closed
Closed
bug(tests): api.terminal.router enumerates zero routes under python-suite shard 12/12 (unidentified sys.modules pollution)#15087
Labels
testing

Description

@mrveiss
mrveiss
opened on Aug 26, 2026

What

Under the python-suite shard 12/12 CI job, api.terminal.router.routes (and
the merged admin_router routes on it) enumerated as empty when read
in-process from autobot-backend/api/terminal_websocket_route_test.py, even
though the same in-process reads pass when that file runs in isolation or in
smaller combinations locally. Evidence: job 98074498060.

FAILED test_every_tool_route_is_present - AssertionError: expected
  ['/terminal/check-tool', '/terminal/install-tool', '/terminal/package-managers',
   '/terminal/validate-command'], found []
FAILED test_tool_route_keeps_the_admin_dependency[...] - StopIteration  (x4)
FAILED TestTerminalRouterDependencyWiring::test_http_routes_keep_the_admin_dependency
  - AssertionError: expected exactly one route for /, found 0
==== 6 failed, 2447 passed, 33 skipped ====

found [] / found 0 means the router object these tests read had no routes
at all -- not that the admin gate was missing. This is a shared-state
collision: some earlier-collected test in the same pytest-xdist worker leaves
either sys.modules['api.terminal'] or the router/admin_router objects
themselves in a state this file's own module-level import inherits.

What I ruled out

  • Not reproducible serially (single process, no -n) across the full
    --shard-splits 12 --shard-group 12 module set locally -- no sys.modules
    leak-guard warning fired for anything touching api/api.terminal
    (repo_tests/sys_modules_leak_guard.py's known-leak baseline lists none).
  • Not reproducible with -n 2 --dist loadscope locally against the same
    shard, either -- all terminal_websocket_route_test.py tests passed.
  • Likely environment-sensitive: CI runs Python 3.14.7
    (/opt/hostedtoolcache/Python/3.14.7/x64), this machine's dev interpreter
    is 3.10.12, and neither pip nor a venv install is permitted here to match
    it. xdist's --dist loadscope worker assignment is a function of
    collection order and per-file item counts, both of which can differ across
    Python versions when optional dependencies are unavailable (several
    collection errors were observed locally in files unrelated to
    api.terminal, e.g. llc/scheduler/*, security/enterprise/threat_detection).

The polluter is unidentified. I did not narrow it to a specific file
before switching strategy (see below) rather than continuing to burn CI/local
cycles bisecting a Python-3.14-only reproduction.

Why this matters beyond one test file

This is the same family as #15076 (a confirmed sys.modules leak from
sdk_request_url_test.py into audit_api_wiring, exit 1 with zero failing
tests) and #15069 (python-suite shard triage) -- tests that corrupt shared
interpreter state for everything scheduled after them in the same xdist
worker. api.terminal_websocket_route_test.py was fixed in #15085/#14998 by
moving the affected assertions to a clean-interpreter subprocess (mirroring
autobot_shared/user_management/models/core_test.py's per-class subprocess
pattern), which is immune to the mechanism regardless of what it is -- so this
issue is the pollution finding on its own, not a blocker on that PR.

Suggested approach

  • A pytest plugin hook (pytest_runtest_teardown or similar) that snapshots
    id(sys.modules.get('api.terminal')) and id(getattr(mod, 'router', None))
    across item boundaries within a -n 2 --dist loadscope run on a 3.14
    interpreter would catch the exact transition point; this needs to run where
    Python 3.14 + the full dependency set are available (CI, or a maintainer
    machine with that toolchain), not this box.
  • Once found, the fix is almost certainly: wrap whatever installs the
    substitute state in the same try/finally shape repo_tests/sys_modules_leak_guard.py
    already enforces for other packages, scoped to the nodes that need it.

Acceptance criteria

  • The polluting test (or the mechanism, if it's not sys.modules
    identity but in-place mutation of the router's .routes list) is
    identified by file:line.
  • It installs/mutates shared state inside a try/finally (or is added to
    repo_tests/sys_modules_leak_baseline.txt if the leak guard already
    covers this shape and just needs the entry).
  • python-suite shard 12/12 passes api/terminal_websocket_route_test.py
    cleanly on CI's own Python 3.14 runner.

Related

Activity

  1. github-actions
    added
    testing
    on Aug 26, 2026
  2. mrveiss
    added 2 commits that reference this issue on Aug 26, 2026

    fix(terminal): read the terminal route/dependency spec from a clean s…

    016739e

    fix(terminal): read the terminal route/dependency spec from a clean s…

    88172eb

  3. mrveiss commented on Aug 26, 2026

    @mrveiss
    mrveiss
    on Aug 26, 2026
    OwnerAuthor

    Root cause: there is no polluter

    The "zero routes" is fastapi >= 0.139's deferred include_router (#15093), not sys.modules pollution.

    The read site. Job 98074498060 ran 85bbb2608767, where the enumeration was a flat .routes read filtered by path:

    return [r for r in merged_router.routes if getattr(r, "path", "") in _TOOL_PATHS]

    What that returns on CI's version. api/terminal.py:1299 is router.include_router(admin_router); api/terminal.py:198 is admin_router.include_router(tools_router, prefix="/terminal"). Under fastapi 0.141.1 / starlette 1.6.0 include_router appends one _IncludedRouter wrapper with path = None rather than copying the child's routes up, so the top level of api.terminal.router is three entries — job 98088603289 dumped them as [APIWebSocketRoute /ws/{session_id}, APIWebSocketRoute /ws/ssh/{host_id}, _IncludedRouter path=null]. Every HTTP route is one level down; the four tool routes are two. Filtering those three by path gives exactly the reported found [], the four StopIterations from next(...), and found 0 for /.

    "Only under shard 12/12" was not a neighbour effect. repo_tests/stable_shard.py assigns a module by sha256 of its own path, independent of what else was collected. Against .test_durations at 9b88068b5:

    module shard
    autobot-backend/api/terminal_websocket_route_test.py 12
    autobot-backend/api/terminal_tools_test.py 12
    autobot-backend/api/terminal_router_dependency_wiring_test.py 2

    Shard 12 is the only shard that ever runs those files, so there was no other shard for the result to differ from. The local/CI split is the fastapi version (#15091: this box resolves 0.135.2, below the declared >=0.141.1 floor, and flattens eagerly) — not xdist scheduling.

    AC status

    What was still broken

    The dependency sweep in terminal_router_dependency_wiring_test.py was vacuous. test_websocket_routes_carry_no_admin_dependency asserts an absence (_ADMIN_DEP not in spec[path]); with every route's dependency list forced to [] it passed while its positive sibling failed. That is not hypothetical — fastapi 0.141.1 reports an inherited include_router(dependencies=) as empty on the route object (job 98114928835), the same deferral. The assertion that is the #14998 fix was reporting the fix intact without checking it.

    PR #15113 adds the witness, adds a per-nesting-level completeness guard for the dump, and replaces the wrong root-cause note in that module. It uses Refs rather than Closes solely because AC2 is refuted rather than satisfied.

  4. mrveiss
    added 5 commits that reference this issue on Aug 26, 2026

    test(terminal): match the tool route by suffix and pin the unrecovera…

    92feb84

    fix(sdk): stop sending query parameters the routes do not accept (#15119

    362cb4f

    docs(terminal): cite #15112's own wording for the unrecoverable inclu…

    dbabe8a

    fix(sdk): stop sending query parameters the routes do not accept (#15119

    b2de8e6

    test(terminal): fail the terminal route sweep when it enumerates noth…

    a1234d4

  5. mrveiss commented on Aug 26, 2026

    @mrveiss
    mrveiss
    on Aug 26, 2026
    OwnerAuthor

    Closing — but one criterion is refuted rather than met, and that is the substance of this issue, so it is stated first rather than buried.

    AC2 cannot be satisfied because there is no polluter. It asks that the polluting test install or mutate shared state inside a try/finally, or be added to sys_modules_leak_baseline.txt. Nothing mutates shared state here. Doing either would have been a fix to a mechanism that does not exist, and the guard entry would have been a permanent false claim about this file. PR #15113 is therefore Refs, not Closes.

    AC Kind Outcome
    1 — identify the mechanism at file:line forensic (CI dumps + git object) Met — and it is not what the title says
    2 — wrap the mutation in try/finally, or baseline it — Refuted: no shared-state mutation exists
    3 — shard 12/12 passes the file cleanly on CI's Python 3.14 CI Met

    AC1 — the real mechanism. fastapi >= 0.139 defers include_router. api/terminal.py:1299 (router.include_router(admin_router)) and :198 (admin_router.include_router(tools_router, prefix="/terminal")) put every HTTP route one level down and the four tool routes two levels down, so a flat .routes read of api.terminal.router returns three entries. CI job 98088603289 dumped exactly that: [APIWebSocketRoute /ws/{session_id}, APIWebSocketRoute /ws/ssh/{host_id}, _IncludedRouter path=null]. Filtering those three reproduces found [], four StopIterations and found 0 — term for term. The read site was recovered from git at 85bbb2608767: a flat .routes walk filtered by path.

    "Only under shard 12/12" was never a neighbour effect. repo_tests/stable_shard.py assigns by sha256 of the module path, and both terminal_websocket_route_test.py and terminal_tools_test.py land in shard 12 — so shard 12 is the only shard that runs those files. There was no other shard to differ from, and therefore nothing about shard 12 to explain.

    The vacuity that was actually live was not the one the title points at. The route sweep was already guarded. The dependency sweep was not — and that is what test_websocket_routes_carry_no_admin_dependency reads, which is the #14998 fix itself, stated as an absence. With every dependency list forced to [] it passed while its positive sibling failed. Not hypothetical: fastapi 0.141.1 reports an inherited include_router(dependencies=) as empty on the route object (job 98114928835). It now carries a witness inside the absence assertion, plus a per-nesting-level completeness test that names which inclusion was lost.

    AC3 — met, on the runner that matters. python-suite shard 12/12 green on Python 3.14.7, the version that produces the deferral; head dbabe8a04, 59 success / 12 skipped / 0 failures across all 12 shards. Everything before that was local, and per #15091 a local pass carries no information about the deferral — this one does.

    A correction on the way there worth recording. The first revision's own new test failed on CI, and its expectation was what was wrong, not the code: it expected /terminal/package-managers, while the dump held /package-managers, /install-tool and /check-tool unprefixed. The walk had not stopped before flattening the inclusion — it flattened it and dropped the include-time prefix, which is not recoverable by runtime introspection. The assertion message said "the enumeration stopped before flattening", which would have sent the next reader hunting a truncation that was not there; it now describes the real condition.

    Left open deliberately: #15126 — the include-prefix gap, gated on #15112's AST mount graph, whose acceptance criteria require deleting this PR's suffix-match workaround when it lands. A workaround must not outlive its reason.

    Not proved: the deferral was never observed first-hand on this box (fastapi 0.135.2, #15091) — it is established from CI dumps plus documented semantics. And shard 2's collected items moved 2563 → 2572 where the diff accounts for +4; the remaining +5 is consistent with the collection variance stable_shard.py documents, but it was not chased and is not claimed.

    Also filed from this work: #15139 — a starved run and a superseded run both surface as failure, and each cost a full root-cause pass on a clean diff.

  6. mrveiss
    closed this as completedon Aug 26, 2026
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

    testing

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions