Skip to content

Commit 354f035

Browse files
committed
fix(sync): surface invalid subscription entries in stdout summary
Resolves four pre-existing test failures across claude, codex, and bob sync tests that asserted "invalid subscription name" appeared in stdout when an entry in evolve.config.yaml had an unsafe name (e.g. '../evil', '.', '..'). Root cause: every platform's sync.py used `normalize_repos(cfg)`, which routes through `_coerce_repo` in lib/config.py. _coerce_repo silently filtered invalid entries (after a stray stderr print with a slightly different phrasing — "ignoring repo entry 'X' — invalid name") and returned None. The downstream "skipped — invalid subscription name" branch in each sync.py ran on already-filtered entries, so it never fired. The user saw "No subscriptions configured" and a stderr log with a different message; the tests saw neither in stdout. Fix: - lib/config.py: drop the stderr prints inside _coerce_repo. They were leaky from a library function (callers, not the lib, should decide where to surface a rejection). Add `classify_repo_entry` which returns (repo, rejection) for one raw entry — exactly one is non-None — so callers can iterate raw `cfg["repos"]` and report rejections per their own UX. - claude/claw-code/codex/bob sync.py: replace `normalize_repos(cfg)` with manual iteration over raw entries via classify_repo_entry. Rejection reasons are added to the same `summaries` list that already collects per-repo sync results, so they appear in the user-visible "Synced N repo(s): …" stdout line. Dedup by name is preserved inline. - test_config.py::test_invalid_scope_entries_dropped: replaced its capsys assertion (which depended on the now-removed stderr print) with a direct call to classify_repo_entry that returns the same rejection reason structurally. Test impact: - Fixes test_sync.py::test_skips_invalid_subscription_name - Fixes test_bob_sharing.py::test_skips_invalid_subscription_name - Fixes test_bob_sharing.py::test_rejects_dot_and_double_dot_names - Fixes test_codex_sharing.py::test_sync_skips_invalid_subscription_name - One pre-existing failure remains: test_subscribe_warns_when_audit_write_fails in test_codex_sharing.py. That test asserts subscribe.py warns and continues when the audit log can't be written; the current subscribe.py rolls back and exits 1 (claude and codex both). That's a separate design decision (fail-open UX vs fail-closed security) that deserves its own focused commit. Refs #219
1 parent 298413e commit 354f035

9 files changed

Lines changed: 283 additions & 82 deletions

File tree

platform-integrations/bob/evolve-lite/skills/evolve-lite-sync/scripts/sync.py

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
break
2525

2626
from audit import append as audit_append # noqa: E402
27-
from config import is_valid_repo_name, load_config, normalize_repos # noqa: E402
27+
from config import classify_repo_entry, load_config # noqa: E402
2828

2929

3030
_GIT_TIMEOUT = 30 # seconds
@@ -107,11 +107,27 @@ def main():
107107
project_root = "."
108108

109109
cfg = load_config(project_root)
110-
repos = normalize_repos(cfg)
111110

112-
if not repos:
111+
raw_entries = cfg.get("repos") if isinstance(cfg, dict) else None
112+
if not isinstance(raw_entries, list):
113+
raw_entries = []
114+
115+
repos = []
116+
rejections = []
117+
seen = set()
118+
for entry in raw_entries:
119+
repo, rejection = classify_repo_entry(entry)
120+
if rejection is not None:
121+
rejections.append(rejection)
122+
continue
123+
if repo["name"] in seen:
124+
continue
125+
seen.add(repo["name"])
126+
repos.append(repo)
127+
128+
if not repos and not rejections:
113129
if not args.quiet:
114-
print("No subscriptions configured. Add one with the evolve-lite:subscribe skill to start syncing shared guidelines.")
130+
print("No subscriptions configured. Add one with the evolve-lite-subscribe skill to start syncing shared guidelines.")
115131
sys.exit(0)
116132

117133
identity = cfg.get("identity", {})
@@ -121,22 +137,18 @@ def main():
121137
total_delta = {}
122138
any_changes = False
123139

140+
for rejection in rejections:
141+
raw_name = rejection["raw_name"]
142+
reason = rejection["reason"]
143+
label = repr(raw_name) if raw_name else "<unnamed entry>"
144+
summaries.append(f"{label} (skipped — {reason})")
145+
124146
for repo in repos:
125-
raw_name = repo.get("name", "unknown")
147+
name = repo["name"]
126148
scope = repo.get("scope", "read")
127149
branch = repo.get("branch", "main")
128150
remote = repo.get("remote")
129151

130-
if not is_valid_repo_name(raw_name):
131-
summaries.append(f"{raw_name!r} (skipped — invalid subscription name)")
132-
continue
133-
name = raw_name.strip()
134-
135-
if not isinstance(branch, str) or not branch.strip():
136-
summaries.append(f"{raw_name!r} (skipped — invalid subscription config)")
137-
continue
138-
branch = branch.strip()
139-
140152
subscribed_base = (evolve_dir / "entities" / "subscribed").resolve()
141153
repo_path = (evolve_dir / "entities" / "subscribed" / name).resolve()
142154

platform-integrations/claude/plugins/evolve-lite/lib/config.py

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import pathlib
99
import re
10-
import sys
1110

1211

1312
VALID_SCOPES = ("read", "write")
@@ -328,29 +327,26 @@ def save_config(cfg, project_root="."):
328327

329328

330329
def _coerce_repo(entry):
331-
"""Normalize a single repo dict. Returns None if required fields are missing."""
330+
"""Normalize a single repo dict. Returns None if required fields are missing.
331+
332+
Rejection is silent — callers that want to surface why a particular entry
333+
was dropped should use ``classify_repo_entry`` to get the rejection reason
334+
and report it however they choose.
335+
"""
332336
if not isinstance(entry, dict):
333337
return None
334338
name = entry.get("name")
335339
remote = entry.get("remote")
336340
if not isinstance(name, str) or not name.strip():
337341
return None
338342
if not is_valid_repo_name(name.strip()):
339-
print(
340-
f"evolve-lite: ignoring repo entry {name!r} — invalid name (only A-Z, a-z, 0-9, '.', '_', '-' allowed)",
341-
file=sys.stderr,
342-
)
343343
return None
344344
if not isinstance(remote, str) or not remote.strip():
345345
return None
346346
scope = entry.get("scope", "read")
347347
if isinstance(scope, str):
348348
scope = scope.strip()
349349
if scope not in VALID_SCOPES:
350-
print(
351-
f"evolve-lite: ignoring repo entry {name!r} — unknown scope {entry.get('scope')!r} (expected one of {', '.join(VALID_SCOPES)})",
352-
file=sys.stderr,
353-
)
354350
return None
355351
branch = entry.get("branch", "main")
356352
if not isinstance(branch, str) or not branch.strip():
@@ -389,6 +385,51 @@ def normalize_repos(cfg):
389385
return result
390386

391387

388+
def classify_repo_entry(entry):
389+
"""Return ``(repo, rejection)`` for one raw ``repos:`` list entry.
390+
391+
Exactly one of ``repo`` or ``rejection`` is non-None:
392+
- ``repo`` is the normalized dict (same shape as ``normalize_repos``
393+
items) when the entry is valid.
394+
- ``rejection`` is a dict ``{"raw_name": str_or_None, "reason": str}``
395+
describing why the entry was dropped. ``reason`` is one of
396+
"invalid subscription name", "missing remote", "unknown scope", or
397+
"malformed entry".
398+
399+
Used by sync.py (and similar) to surface skipped entries in user-facing
400+
output without re-implementing validation.
401+
"""
402+
if not isinstance(entry, dict):
403+
return None, {"raw_name": None, "reason": "malformed entry"}
404+
raw_name = entry.get("name")
405+
if not isinstance(raw_name, str) or not raw_name.strip():
406+
return None, {"raw_name": raw_name, "reason": "invalid subscription name"}
407+
name = raw_name.strip()
408+
if not is_valid_repo_name(name):
409+
return None, {"raw_name": raw_name, "reason": "invalid subscription name"}
410+
remote = entry.get("remote")
411+
if not isinstance(remote, str) or not remote.strip():
412+
return None, {"raw_name": raw_name, "reason": "missing remote"}
413+
scope = entry.get("scope", "read")
414+
if isinstance(scope, str):
415+
scope = scope.strip()
416+
if scope not in VALID_SCOPES:
417+
return None, {"raw_name": raw_name, "reason": "unknown scope"}
418+
branch = entry.get("branch", "main")
419+
if not isinstance(branch, str) or not branch.strip():
420+
branch = "main"
421+
notes = entry.get("notes", "")
422+
if not isinstance(notes, str):
423+
notes = ""
424+
return {
425+
"name": name,
426+
"scope": scope,
427+
"remote": remote.strip(),
428+
"branch": branch.strip(),
429+
"notes": notes,
430+
}, None
431+
432+
392433
def get_repo(cfg, name):
393434
"""Return the repo with the given name, or None."""
394435
for repo in normalize_repos(cfg):

platform-integrations/claude/plugins/evolve-lite/skills/sync/scripts/sync.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
# Add lib to path
2323
sys.path.insert(0, str(Path(__file__).resolve().parent.parent.parent.parent / "lib"))
24-
from config import is_valid_repo_name, load_config, normalize_repos # noqa: E402
24+
from config import classify_repo_entry, load_config # noqa: E402
2525
from audit import append as audit_append # noqa: E402
2626

2727

@@ -149,9 +149,24 @@ def main():
149149
if args.session_start and isinstance(sync_cfg, dict) and sync_cfg.get("on_session_start") is False:
150150
sys.exit(0)
151151

152-
repos = normalize_repos(cfg)
152+
raw_entries = cfg.get("repos") if isinstance(cfg, dict) else None
153+
if not isinstance(raw_entries, list):
154+
raw_entries = []
155+
156+
repos = []
157+
rejections = []
158+
seen = set()
159+
for entry in raw_entries:
160+
repo, rejection = classify_repo_entry(entry)
161+
if rejection is not None:
162+
rejections.append(rejection)
163+
continue
164+
if repo["name"] in seen:
165+
continue
166+
seen.add(repo["name"])
167+
repos.append(repo)
153168

154-
if not repos:
169+
if not repos and not rejections:
155170
if not args.quiet:
156171
print("No subscriptions configured. Add one with /evolve-lite:subscribe to start syncing shared guidelines.")
157172
sys.exit(0)
@@ -163,16 +178,18 @@ def main():
163178
total_delta = {}
164179
any_changes = False
165180

181+
for rejection in rejections:
182+
raw_name = rejection["raw_name"]
183+
reason = rejection["reason"]
184+
label = repr(raw_name) if raw_name else "<unnamed entry>"
185+
summaries.append(f"{label} (skipped — {reason})")
186+
166187
for repo in repos:
167188
name = repo.get("name")
168189
scope = repo.get("scope", "read")
169190
branch = repo.get("branch", "main")
170191
remote = repo.get("remote")
171192

172-
if not is_valid_repo_name(name):
173-
summaries.append(f"{name!r} (skipped — invalid subscription name)")
174-
continue
175-
176193
repo_path = evolve_dir / "entities" / "subscribed" / name
177194

178195
head_before = None

platform-integrations/claw-code/plugins/evolve-lite/lib/config.py

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import pathlib
99
import re
10-
import sys
1110

1211

1312
VALID_SCOPES = ("read", "write")
@@ -328,29 +327,26 @@ def save_config(cfg, project_root="."):
328327

329328

330329
def _coerce_repo(entry):
331-
"""Normalize a single repo dict. Returns None if required fields are missing."""
330+
"""Normalize a single repo dict. Returns None if required fields are missing.
331+
332+
Rejection is silent — callers that want to surface why a particular entry
333+
was dropped should use ``classify_repo_entry`` to get the rejection reason
334+
and report it however they choose.
335+
"""
332336
if not isinstance(entry, dict):
333337
return None
334338
name = entry.get("name")
335339
remote = entry.get("remote")
336340
if not isinstance(name, str) or not name.strip():
337341
return None
338342
if not is_valid_repo_name(name.strip()):
339-
print(
340-
f"evolve-lite: ignoring repo entry {name!r} — invalid name (only A-Z, a-z, 0-9, '.', '_', '-' allowed)",
341-
file=sys.stderr,
342-
)
343343
return None
344344
if not isinstance(remote, str) or not remote.strip():
345345
return None
346346
scope = entry.get("scope", "read")
347347
if isinstance(scope, str):
348348
scope = scope.strip()
349349
if scope not in VALID_SCOPES:
350-
print(
351-
f"evolve-lite: ignoring repo entry {name!r} — unknown scope {entry.get('scope')!r} (expected one of {', '.join(VALID_SCOPES)})",
352-
file=sys.stderr,
353-
)
354350
return None
355351
branch = entry.get("branch", "main")
356352
if not isinstance(branch, str) or not branch.strip():
@@ -389,6 +385,51 @@ def normalize_repos(cfg):
389385
return result
390386

391387

388+
def classify_repo_entry(entry):
389+
"""Return ``(repo, rejection)`` for one raw ``repos:`` list entry.
390+
391+
Exactly one of ``repo`` or ``rejection`` is non-None:
392+
- ``repo`` is the normalized dict (same shape as ``normalize_repos``
393+
items) when the entry is valid.
394+
- ``rejection`` is a dict ``{"raw_name": str_or_None, "reason": str}``
395+
describing why the entry was dropped. ``reason`` is one of
396+
"invalid subscription name", "missing remote", "unknown scope", or
397+
"malformed entry".
398+
399+
Used by sync.py (and similar) to surface skipped entries in user-facing
400+
output without re-implementing validation.
401+
"""
402+
if not isinstance(entry, dict):
403+
return None, {"raw_name": None, "reason": "malformed entry"}
404+
raw_name = entry.get("name")
405+
if not isinstance(raw_name, str) or not raw_name.strip():
406+
return None, {"raw_name": raw_name, "reason": "invalid subscription name"}
407+
name = raw_name.strip()
408+
if not is_valid_repo_name(name):
409+
return None, {"raw_name": raw_name, "reason": "invalid subscription name"}
410+
remote = entry.get("remote")
411+
if not isinstance(remote, str) or not remote.strip():
412+
return None, {"raw_name": raw_name, "reason": "missing remote"}
413+
scope = entry.get("scope", "read")
414+
if isinstance(scope, str):
415+
scope = scope.strip()
416+
if scope not in VALID_SCOPES:
417+
return None, {"raw_name": raw_name, "reason": "unknown scope"}
418+
branch = entry.get("branch", "main")
419+
if not isinstance(branch, str) or not branch.strip():
420+
branch = "main"
421+
notes = entry.get("notes", "")
422+
if not isinstance(notes, str):
423+
notes = ""
424+
return {
425+
"name": name,
426+
"scope": scope,
427+
"remote": remote.strip(),
428+
"branch": branch.strip(),
429+
"notes": notes,
430+
}, None
431+
432+
392433
def get_repo(cfg, name):
393434
"""Return the repo with the given name, or None."""
394435
for repo in normalize_repos(cfg):

platform-integrations/claw-code/plugins/evolve-lite/skills/sync/scripts/sync.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
# Add lib to path
2323
sys.path.insert(0, str(Path(__file__).resolve().parent.parent.parent.parent / "lib"))
24-
from config import is_valid_repo_name, load_config, normalize_repos # noqa: E402
24+
from config import classify_repo_entry, load_config # noqa: E402
2525
from audit import append as audit_append # noqa: E402
2626

2727

@@ -149,9 +149,24 @@ def main():
149149
if args.session_start and isinstance(sync_cfg, dict) and sync_cfg.get("on_session_start") is False:
150150
sys.exit(0)
151151

152-
repos = normalize_repos(cfg)
152+
raw_entries = cfg.get("repos") if isinstance(cfg, dict) else None
153+
if not isinstance(raw_entries, list):
154+
raw_entries = []
155+
156+
repos = []
157+
rejections = []
158+
seen = set()
159+
for entry in raw_entries:
160+
repo, rejection = classify_repo_entry(entry)
161+
if rejection is not None:
162+
rejections.append(rejection)
163+
continue
164+
if repo["name"] in seen:
165+
continue
166+
seen.add(repo["name"])
167+
repos.append(repo)
153168

154-
if not repos:
169+
if not repos and not rejections:
155170
if not args.quiet:
156171
print("No subscriptions configured. Add one with /evolve-lite:subscribe to start syncing shared guidelines.")
157172
sys.exit(0)
@@ -163,16 +178,18 @@ def main():
163178
total_delta = {}
164179
any_changes = False
165180

181+
for rejection in rejections:
182+
raw_name = rejection["raw_name"]
183+
reason = rejection["reason"]
184+
label = repr(raw_name) if raw_name else "<unnamed entry>"
185+
summaries.append(f"{label} (skipped — {reason})")
186+
166187
for repo in repos:
167188
name = repo.get("name")
168189
scope = repo.get("scope", "read")
169190
branch = repo.get("branch", "main")
170191
remote = repo.get("remote")
171192

172-
if not is_valid_repo_name(name):
173-
summaries.append(f"{name!r} (skipped — invalid subscription name)")
174-
continue
175-
176193
repo_path = evolve_dir / "entities" / "subscribed" / name
177194

178195
head_before = None

0 commit comments

Comments
 (0)