Skip to content

fix(pr-management-stats): keep the parity tests from publishing a gist - #7

Draft
MakerFlyFly wants to merge 2 commits into
mainfrom
fix/pr-stats-tests-no-publish
Draft

MakerFlyFly wants to merge 2 commits into
mainfrom
fix/pr-stats-tests-no-publish

Conversation

@MakerFlyFly

@MakerFlyFly MakerFlyFly commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • The JSON parity tests ran dashboard.main() with gist publishing enabled, allowing an authenticated developer to overwrite or create a real gist while both tests passed. Pass --no-publish to the dashboard invocation only.
  • Guard gist_scope_available, publish_gist, read_stats_gist_id, and store_stats_gist_id with pytest.fail, so the parity tests reject publication and session-state access independently of call order, including under ordinary except Exception handling.
  • Preserve all existing JSON assertions and production behavior. The PR changes one test file with 15 added lines.

Type of change

  • Python package (tools/*/ with pyproject.toml)

Test plan

Verification applies to the unchanged head c8fdbc05, using a complete Linux checkout with CPython 3.12.12 and pytest 9.1.1.

  • Original bug reproduced at baseline 4556f693 with a recording gh replacement: two PATCH attempts with an existing ID, or gist creation followed by PATCH and state persistence without an ID, while both tests pass. The replacement never delegates to real gh.
  • Fixed parity tests, both state scenarios: both tests pass, zero gh calls, no session-state changes.
  • Removing --no-publish, including with an except Exception wrapper around the scope guard, makes both tests fail with the expected Failed.
  • Moving each of the four gist entry points ahead of the opt-out branch also fails under except Exception, with zero underlying sentinel calls and zero gh calls. Probe runs preserve source and session state.
  • Complete package pytest run on Linux: 625 passed, 15 subtests passed; JUnit contains 640 entries with zero failures, errors, or skips.
  • Package ruff check, ruff format --check, and mypy pass.
  • uv run zizmor --config .zizmor.yml . passes.
  • prek run --all-files passes, including lychee, all workspace checks, skill-script tests, and spec validation.
  • Normal commit hooks pass. Magpie pre-first-pr-check reports ready for both commits: zero blocking findings and zero advisory items; the PR-body template is checked.

RFC-AI-0004 compliance

  • Write-access discipline — the parity tests do not publish using the developer's credentials, and their four known gist entry points reject unexpected access.

Linked issues

Closes #6

Notes for reviewers

The reference entry point rejects --no-publish, so only dashboard argv receives it.
Each gist entry point is guarded separately; the publish guard protects against future reordering even though the scope guard currently fails first.

Independent follow-ups are tests for main() publication wiring, a stats-wide gh subprocess guard, removing the tracked legacy .apache-magpie.session-state.json, and explicit UTF-8 for production dashboard HTML reads and writes.
Linux UTF-8 success does not imply Windows GBK compatibility; the latter remains a known issue.

This Draft awaits human review.


Was generative AI tooling used to co-author this PR?
  • Yes — Codex (GPT-6). The patch and description are AI-generated and await human review. Both commits carry Generated-by: trailers.

Why: the JSON parity tests call dashboard.main() with publishing enabled,
and their stubs cover only data fetching. An authenticated gh can PATCH
a real dashboard gist or create a new secret gist while both tests pass.

Pass --no-publish to the dashboard invocation only, since reference.main()
does not accept it. Replace both publishing entry points with stubs that
fail the test if reached, so removing the flag cannot silently regress.

Generated-by: Codex (GPT-6)
@MakerFlyFly MakerFlyFly added bug Something isn't working capability:stats Read-only dashboards, metrics, governance evidence family:pr-management pr-management-* skills labels Oct 11, 2026 — with ChatGPT Codex Connector
Why: AssertionError can be caught by an ordinary Exception handler, and
the parity test stubs did not cover session-state reads or writes.

Use pytest.fail so a reached guard escapes ordinary exception handling.
Protect all four gist entry points independently of their current order.
Keep the dashboard-only --no-publish flag and the existing JSON assertions.

Generated-by: Codex (GPT-6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working capability:stats Read-only dashboards, metrics, governance evidence family:pr-management pr-management-* skills family:tools substrate:analytics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PR stats parity tests can publish or overwrite real gists

1 participant