-
Notifications
You must be signed in to change notification settings - Fork 1
fix(devtools): preserve fail-closed testmon provenance #3900
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.
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
Changes from all commits
d339d53
b5ce090
3b1ab74
90a69dc
0575473
2b80951
3af9ec1
a422770
ac768db
657b35b
84c41dc
ab2e789
ef82c3b
a779761
708d069
5e01ad7
c626736
0c2390c
5a22ae7
f0cb999
e4ed36e
7d1cf1b
7f5840b
4590cda
7b3b29b
10265a1
14ea1ce
1d0b1c5
3ac00e8
4d6f93e
5bcbb41
2d95500
e27caee
cd3cbbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -68,6 +68,8 @@ | |||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import tomllib | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| from devtools.testmon_state import attempt_is_checkout_bound, seed_marker_is_checkout_bound | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| class CheckoutImportMismatchError(RuntimeError): | ||||||||||||||||||||||||||||||||||||||
| """``import polylogue`` resolved to a package outside the invoking checkout.""" | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -130,6 +132,7 @@ def as_dict(self) -> dict[str, object]: | |||||||||||||||||||||||||||||||||||||
| _TESTMON_STATE_DIR = Path(".cache/testmon") | ||||||||||||||||||||||||||||||||||||||
| _TESTMON_STATE_MARKER = _TESTMON_STATE_DIR / "seed.json" | ||||||||||||||||||||||||||||||||||||||
| _TESTMON_SEED_ATTEMPT = _TESTMON_STATE_DIR / "seed-attempt.json" | ||||||||||||||||||||||||||||||||||||||
| _TESTMON_SEED_PROTOCOL_VERSION = 5 | ||||||||||||||||||||||||||||||||||||||
| _VERIFY_STATE_DIR = Path(".cache/verify") | ||||||||||||||||||||||||||||||||||||||
| _VERIFY_STATE_MARKER = _VERIFY_STATE_DIR / "current-run.json" | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
@@ -249,6 +252,10 @@ def _marker_origin(marker: Path) -> Path | None: | |||||||||||||||||||||||||||||||||||||
| if not isinstance(payload, Mapping): | ||||||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||||||
| raw = payload.get("checkout_root") | ||||||||||||||||||||||||||||||||||||||
| if raw is None: | ||||||||||||||||||||||||||||||||||||||
| binding = payload.get("binding") | ||||||||||||||||||||||||||||||||||||||
| if isinstance(binding, Mapping): | ||||||||||||||||||||||||||||||||||||||
| raw = binding.get("checkout_root") | ||||||||||||||||||||||||||||||||||||||
| if raw is None: | ||||||||||||||||||||||||||||||||||||||
| fingerprint = payload.get("environment_fingerprint") | ||||||||||||||||||||||||||||||||||||||
| if isinstance(fingerprint, Mapping): | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -258,7 +265,7 @@ def _marker_origin(marker: Path) -> Path | None: | |||||||||||||||||||||||||||||||||||||
| return Path(raw).resolve() | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| def _is_valid_in_progress_testmon_seed_attempt(attempt: Path) -> bool: | ||||||||||||||||||||||||||||||||||||||
| def _is_valid_in_progress_testmon_seed_attempt(attempt: Path, *, checkout_root: Path) -> bool: | ||||||||||||||||||||||||||||||||||||||
| """Recognize the live seed ledger before its completion marker exists. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| ``verify --seed-testmon`` writes this receipt before pytest starts and | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -270,8 +277,25 @@ def _is_valid_in_progress_testmon_seed_attempt(attempt: Path) -> bool: | |||||||||||||||||||||||||||||||||||||
| payload = json.loads(attempt.read_text(encoding="utf-8")) | ||||||||||||||||||||||||||||||||||||||
| except (OSError, UnicodeDecodeError, json.JSONDecodeError): | ||||||||||||||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||||||||||||||
| if not isinstance(payload, Mapping) or payload.get("status") not in {"running", "incomplete"}: | ||||||||||||||||||||||||||||||||||||||
| if not isinstance(payload, Mapping) or payload.get("status") not in { | ||||||||||||||||||||||||||||||||||||||
| "running", | ||||||||||||||||||||||||||||||||||||||
| "incomplete", | ||||||||||||||||||||||||||||||||||||||
| "reusable", | ||||||||||||||||||||||||||||||||||||||
| "complete", | ||||||||||||||||||||||||||||||||||||||
| }: | ||||||||||||||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||||||||||||||
| if payload.get("status") == "complete": | ||||||||||||||||||||||||||||||||||||||
| return attempt_is_checkout_bound( | ||||||||||||||||||||||||||||||||||||||
| payload, | ||||||||||||||||||||||||||||||||||||||
| checkout_root=checkout_root, | ||||||||||||||||||||||||||||||||||||||
| protocol_version=_TESTMON_SEED_PROTOCOL_VERSION, | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| if payload.get("status") == "reusable": | ||||||||||||||||||||||||||||||||||||||
| return attempt_is_checkout_bound( | ||||||||||||||||||||||||||||||||||||||
| payload, | ||||||||||||||||||||||||||||||||||||||
| checkout_root=checkout_root, | ||||||||||||||||||||||||||||||||||||||
| protocol_version=_TESTMON_SEED_PROTOCOL_VERSION, | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+287
to
+298
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value Collapse the two identical status branches. The ♻️ Proposed consolidation- if payload.get("status") == "complete":
- return attempt_is_checkout_bound(
- payload,
- checkout_root=checkout_root,
- protocol_version=_TESTMON_SEED_PROTOCOL_VERSION,
- )
- if payload.get("status") == "reusable":
+ if payload.get("status") in {"complete", "reusable"}:
return attempt_is_checkout_bound(
payload,
checkout_root=checkout_root,
protocol_version=_TESTMON_SEED_PROTOCOL_VERSION,
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| protocol_version = payload.get("protocol_version") | ||||||||||||||||||||||||||||||||||||||
| if not isinstance(protocol_version, int) or isinstance(protocol_version, bool) or protocol_version <= 0: | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
299
to
300
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a linked worktree contains a markerless AGENTS.md reference: AGENTS.md:L338-L341 Useful? React with 👍 / 👎. |
||||||||||||||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -331,12 +355,32 @@ def _cache_artifact( | |||||||||||||||||||||||||||||||||||||
| marker_path = repo_root / marker | ||||||||||||||||||||||||||||||||||||||
| origin = _marker_origin(marker_path) | ||||||||||||||||||||||||||||||||||||||
| if origin == repo_root: | ||||||||||||||||||||||||||||||||||||||
| if state_dir == _TESTMON_STATE_DIR and not seed_marker_is_checkout_bound( | ||||||||||||||||||||||||||||||||||||||
| marker_path, | ||||||||||||||||||||||||||||||||||||||
| checkout_root=repo_root, | ||||||||||||||||||||||||||||||||||||||
| protocol_version=_TESTMON_SEED_PROTOCOL_VERSION, | ||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||
| origin, | ||||||||||||||||||||||||||||||||||||||
| EnvironmentArtifact( | ||||||||||||||||||||||||||||||||||||||
| kind="invalid_testmon_seed", | ||||||||||||||||||||||||||||||||||||||
| path=marker_path, | ||||||||||||||||||||||||||||||||||||||
| detail="testmon seed marker is stale, malformed, or its SQLite graph is incomplete", | ||||||||||||||||||||||||||||||||||||||
| remediation=( | ||||||||||||||||||||||||||||||||||||||
| f"remove {state_path} and run `devtools verify --seed-testmon` " | ||||||||||||||||||||||||||||||||||||||
| "to rebuild the typed testmon state" | ||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| return origin, None | ||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||
| origin is None | ||||||||||||||||||||||||||||||||||||||
| and not marker_path.exists() | ||||||||||||||||||||||||||||||||||||||
| and state_dir == _TESTMON_STATE_DIR | ||||||||||||||||||||||||||||||||||||||
| and _is_valid_in_progress_testmon_seed_attempt(repo_root / _TESTMON_SEED_ATTEMPT) | ||||||||||||||||||||||||||||||||||||||
| and _is_valid_in_progress_testmon_seed_attempt( | ||||||||||||||||||||||||||||||||||||||
| repo_root / _TESTMON_SEED_ATTEMPT, | ||||||||||||||||||||||||||||||||||||||
| checkout_root=repo_root, | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||
| return None, None | ||||||||||||||||||||||||||||||||||||||
| if origin is None: | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a linked-worktree seed is interrupted after
_finalize_testmon_seed_attemptwrites astatus: completeattempt but before it publishesseed.json, this branch callsattempt_is_checkout_boundwith its defaultreusable_only=True; that predicate requiresrelease_baseline_allowed is False, while a complete attempt records it as true.decide_testmon_bootstrapalready recognizes this same attempt throughstamp_from_attempt(..., published_marker=False)and therefore skips replacement, but the subsequent checkout guard rejects the cache, leaving the lane unable to run affected verification. Accept the complete receipt here as selection-only while continuing to withhold release authority untilseed.jsonexists.AGENTS.md reference: AGENTS.md:L338-L341
Useful? React with 👍 / 👎.