Skip to content

test(metadata): stop conflating drift with the warning gate #446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mosheabr merged 2 commits into from
Aug 12, 2026
Merged

test(metadata): stop conflating drift with the warning gate #446

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7faec51
test(metadata): stop conflating drift with the warning gate
mosheabr Aug 12, 2026
acd83d9
Merge branch 'main' into fix/test-drift-conflation
mosheabr Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions .github/scripts/marketplace/test_generate_skill_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ def capture(obj):
# with AI available must fail. A synthetic skill is injected into discovery
# so the outcome does not depend on whether the working tree happens to
# contain an unenriched skill today.
#
# --check returns 1 for drift as well as for warnings, and a PR that edits
# skill content legitimately drifts the checked-in metadata. Asserting on
# the bare exit code therefore failed on unrelated PRs. Capture the output
# and judge the warning path on its own.
original_discover = gen.discover_skills
ghost = gen.Skill(
path="skills/zzz-unenriched-fixture",
Expand All @@ -224,19 +229,32 @@ def discover_with_ghost(exclusions):
found, excluded = original_discover(exclusions)
return list(found) + [ghost], excluded

def exit_code_for(argv, client):
original_diff = gen.diff_text

def run_for(argv, client):
gen.discover_skills = discover_with_ghost
gen.build_ai_client = lambda allow_ai=True: client
# Suppress drift reporting for the duration of the run. --check returns
# 1 for drift as well as for warnings, and any PR that edits skill
# content drifts the checked-in metadata legitimately. Neutralising the
# comparison makes the exit code attributable to the warning gate alone,
# which is what this case is about. Drift itself is covered by case 4.
gen.diff_text = lambda *a, **k: ""
out, err = io.StringIO(), io.StringIO()
try:
with contextlib.redirect_stderr(io.StringIO()), \
contextlib.redirect_stdout(io.StringIO()):
return gen.main(argv)
with contextlib.redirect_stderr(err), contextlib.redirect_stdout(out):
rc = gen.main(argv)
finally:
gen.discover_skills = original_discover
gen.build_ai_client = original_client
gen.diff_text = original_diff
return rc, out.getvalue() + err.getvalue()

rc_no_ai, log_no_ai = run_for(["--check", "--no-ai"], None)
rc_with_ai, _ = run_for(["--check"], failing_api)

rc_no_ai = exit_code_for(["--check", "--no-ai"], None)
rc_with_ai = exit_code_for(["--check"], failing_api)
r.check("unenriched new skill under --no-ai -> reported as a warning",
"PARTIAL SUCCESS" in log_no_ai)
r.check("unenriched new skill under --no-ai -> does not fail PR CI", rc_no_ai == 0,
f"rc={rc_no_ai}")
r.check("unenrichable new skill with AI available -> fails the run", rc_with_ai == 1,
Expand Down
Loading