From 05f0132d4a3bbd6fead7c2bcbec4f743497ba542 Mon Sep 17 00:00:00 2001
From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com>
Date: Tue, 2 Jun 2026 18:20:44 +0200
Subject: [PATCH] fix(secrets): don't short-circuit `add` when var already
 declared
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`wads-secrets add` configures three independent layers: the pyproject
declaration, the ci.yml transport, and the GitHub secret value. When the env
var was already declared in [tool.wads.ci.env], `add` returned early with
'nothing to add' — skipping BOTH the transport edit and the gh secret set. A
half-configured secret (declared but not passed in ci.yml, or value not set on
GitHub) could therefore never be completed by re-running the command.

Report the existing pyproject declaration and continue to the transport + GitHub
layers (each already idempotent). Add regression tests covering the
pre-declared-but-not-transported case and full idempotency.
---
 wads/secrets_cli.py            | 23 +++++++++++++++-------
 wads/tests/test_secrets_cli.py | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/wads/secrets_cli.py b/wads/secrets_cli.py
index f9786f0..99b16d9 100644
--- a/wads/secrets_cli.py
+++ b/wads/secrets_cli.py
@@ -214,15 +214,24 @@ def add(
     changed, existing = add_env_var_to_pyproject(
         pyproject, var_name, secret_name, kind=kind
     )
-    if not changed and existing:
+    # `add` configures three independent layers (pyproject declaration, ci.yml
+    # transport, GitHub secret value). An already-present declaration must NOT
+    # short-circuit the other two — the secret may still be unpassed in ci.yml
+    # or unset on GitHub. So we report the pyproject state and keep going.
+    if changed:
+        print(f"✓ pyproject: added {var_name!r} to [tool.wads.ci.env].{_KINDS[kind]}")
+        if secret_name != var_name:
+            print(f"  ↳ aliased to secret {secret_name!r}")
+    else:
         print(
-            f"'{var_name}' is already configured in [tool.wads.ci.env].{existing}; "
-            "nothing to add."
+            f"· pyproject: {var_name!r} already declared in "
+            f"[tool.wads.ci.env].{existing}"
         )
-        return 0
-    print(f"✓ pyproject: added {var_name!r} to [tool.wads.ci.env].{_KINDS[kind]}")
-    if secret_name != var_name:
-        print(f"  ↳ aliased to secret {secret_name!r}")
+        if existing != _KINDS[kind]:
+            print(
+                f"  (left in {existing}; requested --kind {kind} would put it in "
+                f"{_KINDS[kind]} — move it by hand if that's intended)"
+            )
 
     stub_changed, reason = add_secret_to_stub(ci_file, secret_name)
     print(f"{'✓' if stub_changed else '·'} transport: {reason}")
diff --git a/wads/tests/test_secrets_cli.py b/wads/tests/test_secrets_cli.py
index 4327cf2..01fa104 100644
--- a/wads/tests/test_secrets_cli.py
+++ b/wads/tests/test_secrets_cli.py
@@ -4,6 +4,7 @@
 
 from wads.migration import migrate_ci_to_stub
 from wads.secrets_cli import (
+    add,
     add_env_var_to_pyproject,
     add_secret_to_stub,
     list_,
@@ -80,6 +81,41 @@ def test_list_reports_configured(repo, capsys):
     assert "HF_TOKEN" in out and "HF_WRITE_TOKEN" in out
 
 
+def test_add_does_not_short_circuit_when_already_declared(repo):
+    """Regression: a var already in pyproject must still get the ci.yml transport.
+
+    Previously ``add`` returned early on an existing declaration, skipping both
+    the transport edit and the GitHub secret set — so a half-configured secret
+    (declared but not passed in ci.yml) could never be completed by re-running.
+    """
+    pp = repo / "pyproject.toml"
+    ci = repo / ".github" / "workflows" / "ci.yml"
+    # Pre-declare in pyproject only; ci.yml does NOT yet pass it.
+    add_env_var_to_pyproject(pp, "OPENAI_API_KEY", "OPENAI_API_KEY", kind="test")
+    assert "secrets.OPENAI_API_KEY" not in ci.read_text()
+
+    rc = add(
+        "OPENAI_API_KEY",
+        kind="test",
+        github=False,
+        pyproject=str(pp),
+        ci_file=str(ci),
+    )
+    assert rc == 0
+    # The transport must now be present despite the pre-existing declaration.
+    assert "OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}" in ci.read_text()
+
+
+def test_add_is_idempotent_when_fully_configured(repo):
+    """Running ``add`` twice is a clean no-op (exit 0, single transport line)."""
+    pp = repo / "pyproject.toml"
+    ci = repo / ".github" / "workflows" / "ci.yml"
+    common = dict(kind="test", github=False, pyproject=str(pp), ci_file=str(ci))
+    assert add("OPENAI_API_KEY", **common) == 0
+    assert add("OPENAI_API_KEY", **common) == 0
+    assert ci.read_text().count("secrets.OPENAI_API_KEY") == 1
+
+
 def test_repo_from_git_parses_urls(tmp_path, monkeypatch):
     import wads.secrets_cli as m