Skip to content

guard(ansible): the slm_agent deploy task list is hand-maintained and unchecked against its directory #16100

Description

@mrveiss

Problem

autobot-slm-backend/ansible/roles/slm_agent/tasks/main.yml deploys the SLM agent one module at a
time — a hand-written copy: task per file:

- name: "SLM Agent | Copy agent source - health_collector.py"
  copy:
    src: slm/agent/health_collector.py
    dest: "{{ slm_agent_dir }}/slm/agent/health_collector.py"

Nothing checks that list against the directory it is supposed to mirror. A module added to
slm/agent/ without a matching task is mirrored but never shipped, and the mirror is exactly
what hides it: pipeline-scripts/detect-agent-code-drift.sh passes (the file is byte-identical in
both trees), the file-size ratchet passes, and the module is simply absent on every node. The first
symptom is an ImportError in the agent at runtime, after deploy, on the fleet.

The reverse is equally silent: a task naming a module that no longer exists fails the play on the
node, not in CI.

Why now

Hit while adding slm/agent/heartbeat_payload.py in #16099. The drift check went green with the
module mirrored and not deployed — the guard that exists for this directory pair does not cover
shipping, only content. I verified by hand that all 7 production modules are currently covered:

ok  __init__.py   ok  agent.py         ok  health_collector.py   ok  heartbeat_payload.py
ok  port_scanner.py   ok  role_detector.py   ok  version.py

That check is three lines of shell and should not be a thing anyone does by hand.

Scope

A guard asserting the deploy task list and the canonical agent directory describe the same set.

Acceptance criteria

  • A test asserts every non-test .py under autobot-slm-backend/slm/agent/ has a matching
    src: slm/agent/<name> task in roles/slm_agent/tasks/main.yml.
  • The same test asserts the converse — every src: slm/agent/<name> task names a file that
    exists — so a deleted module fails in CI rather than on a node.
  • The test carries a positive control: it fails when given a synthetic tree with a module and
    no task. An empty or non-resolving file set must not read as a clean pass.
  • It lives beside the existing pair guard in autobot-slm-backend/ansible/tests/, following
    detect_agent_code_drift_test.py, which builds a synthetic tree under tmp_path rather than
    asserting against this checkout.
  • health_collector_probe_test.py's exemption is not special-cased here — test files are out
    of scope for deployment by definition, so the filter is "non-test", not a name list.

Notes

Same defect class as #13148 (hand-synchronised duplications that cannot drift-detect) and #15684
(a manifest and an inline list filling the same venvs). The pattern each time is one fact recorded
in two places with nothing comparing them; the cost is always that the divergence is invisible
until something is wrong on a node.

Related: detect-agent-code-drift.sh already owns "these two directories agree". This is the
missing third leg — "and the deploy list agrees with both".

Activity

  1. mrveiss commented on Sep 9, 2026

    @mrveiss
    OwnerAuthor

    Measured on the current tree — the invariant holds by accident, which is the dangerous state

    modules in autobot-slm-backend/slm/agent/     10
    named in roles/slm_agent/tasks/main.yml        6
    in the directory but not shipped               4   — all *_test.py
    

    So there is no live defect today: the four unshipped files are tests and correctly do not ship. Every non-test module happens to be named.

    That is worth stating precisely, because it is the harder version of the problem rather than the easier one. A rule satisfied by coincidence is indistinguishable from a rule that is enforced, and the first new non-test module breaks it with nothing reporting — which is exactly what #16099 does by adding heartbeat_payload.py.

    The check that would catch it is cheap: every slm/agent/*.py that is not a test must be named by a deploy task, and every deploy task must name a file that exists. The second half matters as much as the first — a task shipping a module that was renamed away fails at apply time on a node rather than in CI.

    Cross-referenced from #13148, where I had cited detect-agent-code-drift.sh as the model to copy for the vnc/ansible duplications. That citation needed this caveat: it is a good model for content parity and silent on reachability, and a guard built from it would answer only one of the two questions an operator needs answered.

  2. mrveiss commented on Sep 9, 2026

    @mrveiss
    OwnerAuthor

    One correction to the numbers recorded above, because it inverts what the example shows.

    #16099 does not break the coincidence — it is the case where the rule was followed. Measured on both refs:

    ref src: slm/agent/ tasks non-test modules
    origin/Dev_new_gui 6 6
    issue-15495-split-frozen-modules 7 7

    heartbeat_payload.py ships at roles/slm_agent/tasks/main.yml:232. I added the module and its copy: task together, because I checked by hand — which is the actual argument for this issue, and a weaker one than "a live defect just landed".

    The 10-vs-6 framing above compares all files to non-test tasks; the 4 unshipped are test files, which are out of scope for deployment by definition. Like-for-like it is 6/6 on base and 7/7 on the branch. The rule holds on both refs and nothing checks it on either — that is the whole finding, and it is enough. A rule satisfied by coincidence being indistinguishable from an enforced one is exactly right; it just does not need a breach to make the case, and citing one that did not happen would be the first thing a reader disproves.

    The load-bearing evidence stays what it was: detect-agent-code-drift.sh reports clean on a module that is byte-identical in both trees and installed on no node. Content parity and shipping parity are different claims, and only one is checked.

  3. mrveiss commented on Sep 9, 2026

    @mrveiss
    OwnerAuthor

    Correcting my own numbers above — the comparison was not like-for-like, and the breach I cited did not happen

    I wrote "modules 10, named 6, unshipped 4" and concluded that #16099 breaks the rule silently by adding heartbeat_payload.py. Both halves are wrong.

    The counts compared different populations. 10 was every .py in slm/agent/, including tests; 6 was the non-test task count. Like-for-like, verified on both refs:

    origin/Dev_new_gui                       src: slm/agent/ tasks = 6   non-test modules = 6
    issue-15495-split-frozen-modules         src: slm/agent/ tasks = 7   non-test modules = 7
    

    And #16099 is the case where the rule was followed, not broken. heartbeat_payload.py ships at tasks/main.yml:232 — the author added the module and its task together, by hand.

    So the correct statement is narrower and better: the rule holds on both refs, and nothing checks it on either. It does not need a breach to be worth fixing, and citing one that did not occur is the first thing a reader disproves — which would have cost this issue its credibility over a detail that was never load-bearing.

    What survives, and it is the whole finding: the invariant is maintained by an author remembering. That hand-check is the argument for this issue. A rule held by discipline and a rule held by a guard produce the same output right up until the moment someone is in a hurry, and nothing here can tell them apart.

    The acceptance criteria are unaffected — a check that every non-test slm/agent/*.py is named by a deploy task, and that every deploy task names a file that exists.

    I have also corrected the cross-reference on #13148, which repeated the same wrong figure.

  4. modified the milestones: Backlog, v0.12.0 on Sep 12, 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

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions