feat(ansible): per-variable fields in the playbook run form

When you click Run, Steward now parses the playbook and renders a field
for each declared variable instead of a blank extra-vars textarea. The
form loads on demand via HTMX (/ansible/run-form/<source>/<playbook>).

- sources.discover_playbook_variables: parse vars: defaults + vars_prompt:
  (vars_prompt wins on name collision; non-scalar vars skipped; role/include
  vars not traversed). Flags secret-looking names + vars_prompt private.
- Run-time values flow through a JSON extra-vars file (-e @file), which is
  space/quote-safe — fixes a latent shlex-split bug in the old -e key=value
  textarea path. executor.build_extra_vars_file (pure) + start_run merge.
- Secret-flagged fields are masked AND routed through an unpersisted
  secret_vars channel (runner.trigger_run → start_run), so passwords entered
  at run time never land in the DB / run history.
- Defaults shown as placeholders (not prefilled): an untouched field falls
  through to the inventory/play default instead of overriding it.
- routes: run_form HTMX endpoint; _parse_run_params now returns
  (params, secret_vars, err) and reads var__/secret__ fields. Schedules drop
  secret vars (can't prompt unattended).
- templates: ansible/_run_form.html fragment; browse.html rewired to HTMX,
  static JS run-form removed. Advanced section keeps limit/tags/check + a
  free-form extra-vars escape hatch.
- tests: test_playbook_variables.py (discovery + extra-vars file).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

steward/ansible/executor.py

@@ -186,6 +186,20 @@ def build_bootstrap(connection: dict | None, tmpdir: str) -> tuple[list[str], li
     return args, files
 
 
+def build_extra_vars_file(merged: dict | None, tmpdir: str) -> tuple[list[str], list[tuple[str, str]]]:
+    """Run-time variables → a JSON extra-vars file (``-e @file``).
+
+    Pure: returns (argv, files). JSON is valid YAML to Ansible and, unlike
+    ``-e key=value`` strings (which Ansible shlex-splits on whitespace), keeps
+    values with spaces/quotes intact. Used for both operator-entered playbook
+    variables and the unpersisted secret subset (merged by the caller).
+    """
+    if not merged:
+        return [], []
+    path = os.path.join(tmpdir, "extravars.json")
+    return ["-e", "@" + path], [(path, json.dumps(merged))]
+
+
 def ansible_env(creds: dict | None, base_env) -> dict:
     """Return a copy of base_env with ANSIBLE_HOST_KEY_CHECKING set from creds."""
     env = dict(base_env)
@@ -282,6 +296,7 @@ async def start_run(
     params: dict | None = None,
     inventory_content: str | None = None,
     connection: dict | None = None,
+    secret_vars: dict | None = None,
 ) -> None:
     """Execute ansible-playbook as a subprocess and update the DB run row.
 
@@ -292,6 +307,9 @@ async def start_run(
     connection is an optional per-run SSH override (user/password) for
     first-contact provisioning. It is applied to the subprocess only — it is
     never persisted on the AnsibleRun row, never placed on argv, and not logged.
+    secret_vars are run-time playbook variables flagged sensitive; like
+    connection they reach the subprocess (merged into the extra-vars file) but
+    are never persisted.
     """
     from steward.models.ansible import AnsibleRunStatus
 
@@ -374,7 +392,10 @@ async def start_run(
 
             cred_args, cred_files = build_credentials(effective_creds, tmpdir)
             boot_args, boot_files = build_bootstrap(conn, tmpdir)
-            for cred_path, content in cred_files + boot_files:
+            # Operator-entered run-time vars (persisted) + secret vars (not).
+            merged_vars = {**((params or {}).get("extra_vars_map") or {}), **(secret_vars or {})}
+            ev_args, ev_files = build_extra_vars_file(merged_vars, tmpdir)
+            for cred_path, content in cred_files + boot_files + ev_files:
                 with open(cred_path, "w", encoding="utf-8") as cf:
                     cf.write(content)
                 os.chmod(cred_path, 0o600)
@@ -388,7 +409,7 @@ async def start_run(
                     user_args += ["--user", ssh_user]
 
             proc = await asyncio.create_subprocess_exec(
-                *cmd, *cred_args, *boot_args, *user_args,
+                *cmd, *cred_args, *boot_args, *user_args, *ev_args,
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.STDOUT,
                 cwd=cwd,

steward/ansible/routes.py

@@ -95,19 +95,43 @@ async def view_playbook(source_name: str, playbook_path: str):
     )
 
 
-def _parse_run_params(form) -> tuple[dict | None, str | None]:
+def _parse_run_params(form) -> tuple[dict | None, dict | None, str | None]:
-    """Build the executor params dict from form fields. Returns (params, error)."""
+    """Build executor params + the unpersisted secret-vars dict from form fields.
-    extra_vars: list[str] = []
+
+    Returns (params, secret_vars, error). Discovered variable fields are named
+    ``var__<name>``; a sibling hidden ``secret__<name>`` marks the sensitive
+    ones, which go into secret_vars (never persisted) instead of params. A
+    free-form ``extra_vars`` textarea (key=value per line) is also folded in.
+    All run-time values flow through a JSON extra-vars file downstream, so
+    values may contain spaces/quotes safely.
+    """
+    extra_vars_map: dict[str, str] = {}
+    secret_vars: dict[str, str] = {}
+
+    for key in set(form.keys()):
+        if not key.startswith("var__"):
+            continue
+        name = key[len("var__"):]
+        val = (form.get(key) or "").strip()
+        if not val:
+            continue  # untouched → fall through to inventory/play default
+        if f"secret__{name}" in form:
+            secret_vars[name] = val
+        else:
+            extra_vars_map[name] = val
+
     for line in (form.get("extra_vars", "") or "").splitlines():
         line = line.strip()
         if not line:
             continue
         if "=" not in line:
-            return None, f"Invalid extra var (expected key=value): {line!r}"
+            return None, None, f"Invalid extra var (expected key=value): {line!r}"
-        extra_vars.append(line)
+        k, v = line.split("=", 1)
+        extra_vars_map[k.strip()] = v
+
     params: dict = {}
-    if extra_vars:
+    if extra_vars_map:
-        params["extra_vars"] = extra_vars
+        params["extra_vars_map"] = extra_vars_map
     limit = (form.get("limit", "") or "").strip()
     if limit:
         params["limit"] = limit
@@ -116,7 +140,37 @@ def _parse_run_params(form) -> tuple[dict | None, str | None]:
         params["tags"] = tags
     if "check" in form:
         params["check"] = True
-    return (params or None), None
+    return (params or None), (secret_vars or None), None
+
+
+@ansible_bp.get("/run-form/<source_name>/<path:playbook_path>")
+@require_role(UserRole.operator)
+async def run_form(source_name: str, playbook_path: str):
+    """HTMX fragment: the run form for one playbook, with a field per declared
+    variable (vars/vars_prompt) discovered from the playbook itself."""
+    from steward.models.ansible_inventory import AnsibleTarget, AnsibleGroup
+
+    source = next((s for s in _get_sources() if s["name"] == source_name), None)
+    if source is None:
+        return "Source not found", 404
+    contents = src_module.read_playbook(source["path"], playbook_path)
+    if contents is None:
+        return "Playbook not found", 404
+    variables = src_module.discover_playbook_variables(contents)
+    inventories = src_module.discover_inventories(source["path"])
+
+    async with current_app.db_sessionmaker() as db:
+        targets = (await db.execute(
+            select(AnsibleTarget).order_by(AnsibleTarget.name))).scalars().all()
+        groups = (await db.execute(
+            select(AnsibleGroup).order_by(AnsibleGroup.name))).scalars().all()
+
+    return await render_template(
+        "ansible/_run_form.html",
+        source_name=source_name, playbook_path=playbook_path,
+        variables=variables, targets=targets, groups=groups,
+        inventories=inventories,
+    )
 
 
 @ansible_bp.post("/runs")
@@ -132,7 +186,7 @@ async def create_run():
     if not playbook_path:
         return "playbook_path is required", 400
 
-    params_or_none, err = _parse_run_params(form)
+    params_or_none, secret_vars, err = _parse_run_params(form)
     if err:
         return err, 400
 
@@ -142,6 +196,7 @@ async def create_run():
         playbook_path=playbook_path,
         inventory_scope=inventory_scope,
         params=params_or_none,
+        secret_vars=secret_vars,
         triggered_by=session["user_id"],
     )
     if err == "Source not found":
@@ -304,7 +359,9 @@ def _schedule_form_fields(form) -> tuple[dict | None, str | None]:
         interval = 0
     if not name or not source_name or not playbook_path or interval <= 0:
         return None, "name, source, playbook, and a positive interval are required"
-    params, err = _parse_run_params(form)
+    # Scheduled runs can't prompt, so secret vars are intentionally dropped —
+    # automation should rely on global creds / inventory vars, not run-time secrets.
+    params, _secret_vars, err = _parse_run_params(form)
     if err:
         return None, err
     return {

steward/ansible/runner.py

@@ -26,6 +26,7 @@ async def trigger_run(
     triggered_by: str | None = None,
     inventory_content: str | None = None,
     connection: dict | None = None,
+    secret_vars: dict | None = None,
 ):
     """Resolve inventory for the scope, persist an AnsibleRun, and launch it.
 
@@ -36,6 +37,8 @@ async def trigger_run(
     connection is an optional per-run SSH override (user/password) for
     first-contact provisioning — passed to the executor but deliberately NOT
     stored on the AnsibleRun row (params), so the password never lands in the DB.
+    secret_vars (sensitive run-time playbook variables) are likewise passed
+    through to the executor but never persisted.
     Returns (run, source, error): on success error is None; on failure run is
     None and error is a short human-readable reason.
     """
@@ -77,7 +80,7 @@ async def trigger_run(
         executor.start_run(
             app, run_id, playbook_path, inventory_path or "",
             source["path"], params or None, inventory_content,
-            connection=connection,
+            connection=connection, secret_vars=secret_vars,
         )
     )
     task.add_done_callback(

steward/ansible/sources.py

@@ -2,6 +2,7 @@ from __future__ import annotations
 import asyncio
 import logging
 import os
+import re
 import stat
 import tempfile
 from pathlib import Path
@@ -10,6 +11,12 @@ logger = logging.getLogger(__name__)
 
 INVENTORY_NAMES = {"hosts", "inventory", "inventory.yml", "inventory.ini"}
 
+# Variable names that should be entered masked + kept out of the DB. Matched
+# case-insensitively as a substring of the variable name.
+_SECRET_VAR_RE = re.compile(
+    r"(password|passwd|secret|token|api[_-]?key|private[_-]?key|credential)", re.I
+)
+
 # Name of the always-present, read-only source of first-party playbooks shipped
 # inside the app (maintenance tasks, host-agent install). Not operator-editable.
 BUILTIN_SOURCE_NAME = "steward-builtin"
@@ -188,6 +195,64 @@ def delete_playbook(source_path: str, relative_path: str) -> tuple[bool, str | N
     return True, None
 
 
+def discover_playbook_variables(content: str) -> list[dict]:
+    """Parse a playbook and list the variables an operator can set at run time.
+
+    Surfaces two sources, in this precedence (first wins on name collision):
+      - ``vars_prompt:`` — Ansible's explicit "ask me" mechanism. ``required``
+        when it has no default; ``secret`` when ``private`` (Ansible's default
+        is private=yes) or the name looks sensitive.
+      - ``vars:`` scalar entries — shown with their default as a placeholder
+        (NOT prefilled, so an untouched field falls through to inventory/play
+        defaults rather than overriding them).
+
+    Only top-level plays are inspected — role defaults and included var files are
+    not traversed (kept simple + predictable). Returns a list of dicts:
+    {name, default, secret, required, prompt}.
+    """
+    import yaml
+    try:
+        plays = yaml.safe_load(content)
+    except yaml.YAMLError:
+        return []
+    if not isinstance(plays, list):
+        return []
+
+    out: list[dict] = []
+    seen: set[str] = set()
+
+    def add(name, default, secret, required, prompt=None):
+        if not isinstance(name, str) or name in seen:
+            return
+        seen.add(name)
+        out.append({
+            "name": name,
+            "default": "" if default is None else default,
+            "secret": secret,
+            "required": required,
+            "prompt": prompt,
+        })
+
+    for play in plays:
+        if not isinstance(play, dict):
+            continue
+        for vp in play.get("vars_prompt") or []:
+            if not isinstance(vp, dict) or "name" not in vp:
+                continue
+            name = vp["name"]
+            default = vp.get("default")
+            private = bool(vp.get("private", True))  # Ansible defaults private=yes
+            add(name, default, private or bool(_SECRET_VAR_RE.search(str(name))),
+                default is None, vp.get("prompt"))
+        play_vars = play.get("vars")
+        if isinstance(play_vars, dict):
+            for name, default in play_vars.items():
+                # Only scalar defaults map cleanly to a single input field.
+                if isinstance(default, (str, int, float, bool)) or default is None:
+                    add(name, default, bool(_SECRET_VAR_RE.search(str(name))), False)
+    return out
+
+
 def validate_playbook_yaml(content: str) -> tuple[bool, str | None]:
     """Cheap save-time validation: parses as YAML and is a non-empty play list."""
     import yaml

steward/templates/ansible/_run_form.html

@@ -0,0 +1,96 @@
+{# Run form for a single playbook — loaded via HTMX from /ansible/run-form/… #}
+<div style="background:var(--bg-elevated);border-radius:6px;padding:1rem;border:1px solid var(--border-mid);">
+  <div style="display:flex;align-items:center;justify-content:space-between;margin-bottom:0.75rem;">
+    <h4 class="section-title" style="margin-bottom:0;">Run <span style="font-family:ui-monospace,monospace;font-weight:normal;">{{ playbook_path }}</span></h4>
+    <a href="#" onclick="this.closest('div[id^=run-form-]').innerHTML='';return false;"
+       class="btn btn-ghost btn-sm">Cancel</a>
+  </div>
+  <form hx-post="/ansible/runs" hx-target="next .run-result" hx-swap="innerHTML">
+    <input type="hidden" name="source_name" value="{{ source_name }}">
+    <input type="hidden" name="playbook_path" value="{{ playbook_path }}">
+
+    <div class="form-group">
+      <label>Run against</label>
+      <select name="inventory_scope">
+        {% if targets %}
+        <option value="steward:all">All targets ({{ targets | length }})</option>
+        {% if groups %}
+        <optgroup label="Group">
+          {% for grp in groups %}<option value="steward:group:{{ grp.id }}">Group: {{ grp.name }}</option>{% endfor %}
+        </optgroup>
+        {% endif %}
+        <optgroup label="Single target">
+          {% for tgt in targets %}<option value="steward:target:{{ tgt.id }}">{{ tgt.name }}</option>{% endfor %}
+        </optgroup>
+        {% endif %}
+        {% if inventories %}
+        <optgroup label="Repo inventory file">
+          {% for inv in inventories %}<option value="repo:{{ source_name }}:{{ inv }}">{{ source_name }}/{{ inv }}</option>{% endfor %}
+        </optgroup>
+        {% endif %}
+        {% if not targets and not inventories %}
+        <option value="" disabled>No targets or inventory files — add targets at /ansible/inventory/targets</option>
+        {% endif %}
+      </select>
+    </div>
+
+    {% if variables %}
+    <div style="margin:0.5rem 0 0.25rem;font-size:0.82rem;color:var(--text-muted);font-weight:600;">
+      Playbook variables
+    </div>
+    <p style="color:var(--text-dim);font-size:0.76rem;margin:0 0 0.6rem;">
+      Leave a field blank to use the playbook/inventory default. Secret fields are
+      never stored.
+    </p>
+    {% for v in variables %}
+    <div class="form-group">
+      <label>
+        {{ v.prompt or v.name }}
+        <span style="color:var(--text-muted);font-weight:normal;font-size:0.76rem;">
+          ({{ v.name }}{% if v.required %}, required{% endif %}{% if v.secret %}, secret{% endif %})
+        </span>
+      </label>
+      {% if v.secret %}<input type="hidden" name="secret__{{ v.name }}" value="1">{% endif %}
+      <input type="{{ 'password' if v.secret else 'text' }}"
+             name="var__{{ v.name }}"
+             {% if v.required %}required{% endif %}
+             {% if v.secret %}autocomplete="new-password"{% endif %}
+             placeholder="{% if v.secret %}(hidden){% elif v.default not in (None, '') %}default: {{ v.default }}{% else %}no default{% endif %}"
+             style="width:100%;font-family:ui-monospace,monospace;font-size:0.85rem;">
+    </div>
+    {% endfor %}
+    {% else %}
+    <p style="color:var(--text-dim);font-size:0.8rem;margin:0.25rem 0 0.6rem;">
+      No declared variables found in this playbook.
+    </p>
+    {% endif %}
+
+    <details style="margin:0.25rem 0 0.75rem;">
+      <summary style="cursor:pointer;font-size:0.82rem;color:var(--text-muted);">Advanced</summary>
+      <div class="form-group" style="margin-top:0.6rem;">
+        <label>Extra vars <span style="color:var(--text-muted);font-weight:normal;">(one key=value per line)</span></label>
+        <textarea name="extra_vars" rows="2" placeholder="custom_var=value"
+                  style="width:100%;font-family:ui-monospace,monospace;font-size:0.85rem;"></textarea>
+      </div>
+      <div class="form-group">
+        <label>Limit to host(s) <span style="color:var(--text-muted);font-weight:normal;">(--limit)</span></label>
+        <input type="text" name="limit" placeholder="web01,db*">
+      </div>
+      <div class="form-group">
+        <label>Tags <span style="color:var(--text-muted);font-weight:normal;">(--tags)</span></label>
+        <input type="text" name="tags" placeholder="deploy,config">
+      </div>
+      <div class="form-group">
+        <label style="display:flex;align-items:center;gap:0.5rem;font-weight:normal;">
+          <input type="checkbox" name="check">
+          Dry-run (--check --diff) — preview changes without applying
+        </label>
+      </div>
+    </details>
+
+    <div style="display:flex;gap:0.75rem;align-items:center;">
+      <button type="submit" class="btn">Execute</button>
+    </div>
+  </form>
+  <div class="run-result" style="margin-top:1rem;"></div>
+</div>

steward/templates/ansible/browse.html

@@ -76,88 +76,17 @@
                 {% if sd.editable and session.user_role == 'admin' %}
                 <a href="/ansible/playbooks/edit/{{ sd.source.name }}/{{ pb }}" class="btn btn-sm btn-ghost" style="margin-right:0.5rem;">Edit</a>
                 {% endif %}
-                <a href="#run-form-{{ sd.source.name }}"
+                <a hx-get="/ansible/run-form/{{ sd.source.name }}/{{ pb }}"
-                   onclick="document.getElementById('run-form-{{ sd.source.name }}').style.display='block';
+                   hx-target="#run-form-{{ sd.source.name }}" hx-swap="innerHTML"
-                            document.getElementById('run-playbook-{{ sd.source.name }}').value='{{ pb }}';
+                   style="cursor:pointer;" class="btn btn-sm btn-ghost">Run</a>
-                            document.getElementById('run-playbook-display-{{ sd.source.name }}').value='{{ pb }}';
-                            return false;"
-                   class="btn btn-sm btn-ghost">Run</a>
             </td>
         </tr>
         {% endfor %}
         </tbody>
     </table>
 
-    {# Run trigger form for this source #}
+    {# Run form (loaded on demand via HTMX from /ansible/run-form/<source>/<pb>) #}
-    <div id="run-form-{{ sd.source.name }}" style="display:none;background:var(--bg-elevated);border-radius:6px;padding:1rem;border:1px solid var(--border-mid);">
+    <div id="run-form-{{ sd.source.name }}"></div>
-        <h4 class="section-title" style="margin-bottom:0.75rem;">Trigger Run</h4>
-        <form hx-post="/ansible/runs" hx-target="#run-result-{{ sd.source.name }}" hx-swap="innerHTML">
-            <input type="hidden" name="source_name" value="{{ sd.source.name }}">
-            <input type="hidden" id="run-playbook-{{ sd.source.name }}" name="playbook_path" value="">
-            <div class="form-group">
-                <label>Playbook</label>
-                <input type="text" id="run-playbook-display-{{ sd.source.name }}"
-                       readonly style="color:var(--text-muted);" value="">
-            </div>
-            <div class="form-group">
-                <label>Run Against</label>
-                <select name="inventory_scope">
-                    {% if targets %}
-                    <optgroup label="All Targets">
-                        <option value="steward:all">All targets ({{ targets | length }})</option>
-                    </optgroup>
-                    {% if groups %}
-                    <optgroup label="Group">
-                        {% for grp in groups %}
-                        <option value="steward:group:{{ grp.id }}">Group: {{ grp.name }}</option>
-                        {% endfor %}
-                    </optgroup>
-                    {% endif %}
-                    <optgroup label="Single Target">
-                        {% for tgt in targets %}
-                        <option value="steward:target:{{ tgt.id }}">{{ tgt.name }}</option>
-                        {% endfor %}
-                    </optgroup>
-                    {% endif %}
-                    {% if sd.inventories %}
-                    <optgroup label="Repo Inventory File">
-                        {% for inv in sd.inventories %}
-                        <option value="repo:{{ sd.source.name }}:{{ inv }}">{{ sd.source.name }}/{{ inv }}</option>
-                        {% endfor %}
-                    </optgroup>
-                    {% endif %}
-                    {% if not targets and not sd.inventories %}
-                    <option value="" disabled>No targets or inventory files — add targets at /ansible/inventory/targets</option>
-                    {% endif %}
-                </select>
-            </div>
-            <div class="form-group">
-                <label>Extra vars <span style="color:var(--text-muted);font-weight:normal;">(optional, one key=value per line)</span></label>
-                <textarea name="extra_vars" rows="2" placeholder="version=1.2.3&#10;restart=true"
-                          style="width:100%;font-family:ui-monospace,monospace;font-size:0.85rem;"></textarea>
-            </div>
-            <div class="form-group">
-                <label>Limit to host(s) <span style="color:var(--text-muted);font-weight:normal;">(optional, --limit)</span></label>
-                <input type="text" name="limit" placeholder="web01,db*">
-            </div>
-            <div class="form-group">
-                <label>Tags <span style="color:var(--text-muted);font-weight:normal;">(optional, --tags)</span></label>
-                <input type="text" name="tags" placeholder="deploy,config">
-            </div>
-            <div class="form-group">
-                <label style="display:flex;align-items:center;gap:0.5rem;font-weight:normal;">
-                    <input type="checkbox" name="check">
-                    Dry-run (--check --diff) — preview changes without applying
-                </label>
-            </div>
-            <div style="display:flex;gap:0.75rem;align-items:center;">
-                <button type="submit" class="btn">Execute</button>
-                <a href="#" onclick="document.getElementById('run-form-{{ sd.source.name }}').style.display='none';return false;"
-                   class="btn btn-ghost btn-sm">Cancel</a>
-            </div>
-        </form>
-        <div id="run-result-{{ sd.source.name }}" style="margin-top:1rem;"></div>
-    </div>
     {% endif %}
 </div>
 {% endfor %}

tests/test_playbook_variables.py

@@ -0,0 +1,110 @@
+"""Unit tests for playbook variable discovery + run-time extra-vars file."""
+import json
+
+from steward.ansible.executor import build_extra_vars_file
+from steward.ansible.sources import discover_playbook_variables
+
+
+def _names(variables):
+    return [v["name"] for v in variables]
+
+
+def test_vars_block_defaults_surface_as_fields():
+    content = """
+- hosts: all
+  vars:
+    steward_url: ""
+    agent_interval: 30
+    enabled: true
+  tasks: []
+"""
+    variables = discover_playbook_variables(content)
+    assert _names(variables) == ["steward_url", "agent_interval", "enabled"]
+    interval = next(v for v in variables if v["name"] == "agent_interval")
+    assert interval["default"] == 30
+    assert interval["required"] is False
+    assert interval["secret"] is False
+
+
+def test_secretish_names_flagged():
+    content = """
+- hosts: all
+  vars:
+    db_password: ""
+    api_token: ""
+    vault_secret: ""
+    plain_value: "x"
+"""
+    by = {v["name"]: v for v in discover_playbook_variables(content)}
+    assert by["db_password"]["secret"] is True
+    assert by["api_token"]["secret"] is True
+    assert by["vault_secret"]["secret"] is True
+    assert by["plain_value"]["secret"] is False
+
+
+def test_vars_prompt_required_and_private():
+    content = """
+- hosts: all
+  vars_prompt:
+    - name: release_tag
+      prompt: "Which release?"
+    - name: admin_pw
+      prompt: "Admin password"
+      private: true
+      default: ""
+  tasks: []
+"""
+    by = {v["name"]: v for v in discover_playbook_variables(content)}
+    # No default → required; vars_prompt defaults to private=yes → secret.
+    assert by["release_tag"]["required"] is True
+    assert by["release_tag"]["secret"] is True
+    assert by["release_tag"]["prompt"] == "Which release?"
+    # Explicit private + has default → secret but not required.
+    assert by["admin_pw"]["secret"] is True
+    assert by["admin_pw"]["required"] is False
+
+
+def test_vars_prompt_wins_on_name_collision():
+    content = """
+- hosts: all
+  vars_prompt:
+    - name: dup
+      prompt: "from prompt"
+      private: false
+  vars:
+    dup: "from vars"
+"""
+    variables = discover_playbook_variables(content)
+    assert _names(variables) == ["dup"]
+    assert variables[0]["prompt"] == "from prompt"
+
+
+def test_non_scalar_vars_skipped():
+    content = """
+- hosts: all
+  vars:
+    scalar: 1
+    a_list: [1, 2]
+    a_map: {k: v}
+"""
+    assert _names(discover_playbook_variables(content)) == ["scalar"]
+
+
+def test_malformed_yaml_returns_empty():
+    assert discover_playbook_variables("this: : : not valid") == []
+    assert discover_playbook_variables("just a string") == []
+
+
+def test_extra_vars_file_roundtrip_and_space_safe():
+    merged = {"agent_interval": "30", "msg": "hello world", "q": 'a "quote"'}
+    args, files = build_extra_vars_file(merged, "/td")
+    assert args == ["-e", "@/td/extravars.json"]
+    path, content = files[0]
+    assert path == "/td/extravars.json"
+    # JSON keeps spaces/quotes intact — no shlex hazard like -e key=value.
+    assert json.loads(content) == merged
+
+
+def test_extra_vars_file_empty_is_noop():
+    assert build_extra_vars_file({}, "/td") == ([], [])
+    assert build_extra_vars_file(None, "/td") == ([], [])