test(cross-impl): make the Python signer parity check run for real#45
Open
ucekmez wants to merge 1 commit into
Open
test(cross-impl): make the Python signer parity check run for real#45ucekmez wants to merge 1 commit into
ucekmez wants to merge 1 commit into
Conversation
The sole cross-language signer wire-vector test always skipped: it imported eep_signer (not installed in test.sh or the cross-impl CI job) and, even when present, called verify() with a `now` kwarg the Python signer does not accept — skipping on the TypeError. Rewrite it to assert sign() reproduces the recorded fixture signature (deterministic, no frozen-time dependency), and to FAIL rather than skip when EEP_REQUIRE_PYTHON_SIGNER=1. Install eep-signer-python and set that flag in test.sh and both CI workflows so the parity check can never silently no-op where it is meant to run. Surfaced by the EEP protocol audit (finding python-signer-crossimpl-check-dead). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Ugur Cekmez <ucekmez@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR makes the cross-implementation conformance suite actually exercise the Python signer against the canonical signature fixtures, preventing the previous “green but skipped” parity check.
Changes:
- Replace the Python-signer cross-impl test from a
verify(..., now=...)flow (which could skip) to a deterministicsign()-equals-fixture assertion. - Add
EEP_REQUIRE_PYTHON_SIGNER=1gating so missingeep_signerbecomes a hard failure in CI /test.sh --full. - Install the Python signer in
test.sh --fulland in the cross-impl steps of thetest.ymlandpublish.ymlworkflows.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/cross-impl/test_conformance_fixtures.py |
Reworks Python signer parity test to compare sign() output to recorded fixture signature; adds env-gated import behavior. |
test.sh |
Installs the Python signer and sets EEP_REQUIRE_PYTHON_SIGNER=1 when running cross-impl tests in --full mode. |
.github/workflows/test.yml |
Ensures cross-impl CI job installs the Python signer and enforces it via EEP_REQUIRE_PYTHON_SIGNER=1. |
.github/workflows/publish.yml |
Ensures publish preflight cross-impl tests install the Python signer and enforce it via EEP_REQUIRE_PYTHON_SIGNER=1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
115
to
+119
| try: | ||
| from eep_signer import EEPSigner # type: ignore | ||
| except Exception: | ||
| except Exception as exc: # pragma: no cover - exercised via env in CI | ||
| if os.environ.get("EEP_REQUIRE_PYTHON_SIGNER") == "1": | ||
| raise AssertionError( |
|
|
||
| set +e | ||
| (cd "$ROOT_DIR" && pip install -r tests/cross-impl/requirements.txt -q && EEP_BASE_URL="${EEP_BASE_URL:-http://localhost:3002}" python3 -m pytest tests/cross-impl -v) | ||
| (cd "$ROOT_DIR" && pip install -r tests/cross-impl/requirements.txt -q && pip install -e packages/eep-signer-python -q && EEP_REQUIRE_PYTHON_SIGNER=1 EEP_BASE_URL="${EEP_BASE_URL:-http://localhost:3002}" python3 -m pytest tests/cross-impl -v) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The only cross-language test that exercises the Python signer against the canonical wire fixtures always skipped:
eep_signer, which is not installed intest.sh --fullor the cross-impl CI job (CI installs it only into a throwaway audit venv), andverify(..., now=...)— a kwarg the Python signer doesn't accept — skipping on theTypeError.So a green suite proved nothing about Python↔TS signing parity.
Fix
sign()reproduces the recorded fixture signature — deterministic, no frozen-time dependency, no silent skip.EEP_REQUIRE_PYTHON_SIGNER=1: a missing import becomes a failure, not a skip.eep-signer-pythonand set that flag intest.shand both CI workflows (publish.yml,test.yml).Verification
Surfaced by the EEP protocol audit (finding python-signer-crossimpl-check-dead).
🤖 Generated with Claude Code