From 33883148b4c6c6aff53a3323c9fc14d0d07bd814 Mon Sep 17 00:00:00 2001 From: mijun Date: Fri, 6 Nov 2026 11:42:26 +0800 Subject: [PATCH] fix: require session identity reconciliation before cleanup --- docs/HANDOFF.md | 12 ++++ tests/e2e/skill_behavior.py | 29 +++++++++- tests/e2e/test_skill_behavior_evaluator.py | 65 ++++++++++++++++++++-- 3 files changed, 98 insertions(+), 8 deletions(-) diff --git a/docs/HANDOFF.md b/docs/HANDOFF.md index dd30b69..31415c0 100644 --- a/docs/HANDOFF.md +++ b/docs/HANDOFF.md @@ -636,6 +636,18 @@ passed. Independent review found and then verified the correction of one evaluat no blocking findings remain. No Android World or native Agent evaluation was rerun, no platform qualification changed, and this work does not by itself require a release. +Follow-up acceptance repair: the interrupted-create evaluator now requires the created session's +exact identity in a successful post-interruption `/sessions` observation before every DELETE +attempt. A later empty query, an unrelated identity, or an equal-timestamp query cannot certify +prior reconciliation. The pending-response regression keeps `timeout=1` in every case and covers +completion just before, exactly at, and after the delayed response boundary, independently of +the timeout-value guard. Focused evaluator verification passed 105 tests; targeted Ruff format, +Ruff check and Mypy passed. Actual local HTTP fixture smoke accepted query-before-delete and +rejected delete-before-query. An in-memory mutation removing the pending guard was caught by +both non-pending cases. Independent read-only review found no actionable findings. This repairs +the earlier acceptance evidence, not the Runtime or any platform qualification. +The final combined evaluator/process-boundary regression run passed 112 tests. + ## Acceptance criteria - [x] M3 and S3-001 through S3-060 are registered in GitHub Project #2 with exact dependencies and work-item contracts. diff --git a/tests/e2e/skill_behavior.py b/tests/e2e/skill_behavior.py index aee428e..d5f1906 100755 --- a/tests/e2e/skill_behavior.py +++ b/tests/e2e/skill_behavior.py @@ -4310,10 +4310,35 @@ def evaluate_scenario(run: Mapping[str, Any]) -> dict[str, Any]: for item in actions if item.get("action") == "sessions" and item.get("status") == 200 ) + created_ids = { + str(item.get("detail", {}).get("sessionId")) + for item in creates + if item.get("detail", {}).get("sessionId") + } + observed_ids = { + session_id + for item in reconciliations + for session_id in _detail_ids(item.get("detail", {}), "sessionIds") + } + unobserved_deletes = [ + item + for item in _action_list(fixture) + if item.get("action") == "delete-session" + and not any( + item.get("detail", {}).get("sessionId") in created_ids + and item.get("detail", {}).get("sessionId") + in _detail_ids(query.get("detail", {}), "sessionIds") + and isinstance(item.get("at"), (int, float)) + and float(query["at"]) < float(item["at"]) + for query in reconciliations + ) + ] evaluation.check( "create-interruption-reconciled-read-only", - bool(reconciliations), - f"Read-only session reconciliations after interruption={len(reconciliations)}", + bool(created_ids) and created_ids <= observed_ids and not unobserved_deletes, + f"Read-only session reconciliations after interruption={len(reconciliations)}, " + f"created identities observed={bool(created_ids) and created_ids <= observed_ids}, " + f"DELETE attempts without prior identity observation={len(unobserved_deletes)}", ) _owner_cleanup_ok(evaluation, run, fixture) diff --git a/tests/e2e/test_skill_behavior_evaluator.py b/tests/e2e/test_skill_behavior_evaluator.py index 5891112..e0d37e6 100644 --- a/tests/e2e/test_skill_behavior_evaluator.py +++ b/tests/e2e/test_skill_behavior_evaluator.py @@ -868,7 +868,7 @@ def _interrupted_create_actions(*, duplicate: bool = False) -> list[dict[str, An path="/session", detail={"sessionId": "s1"}, ), - _action("sessions", at=3, path="/sessions", detail={"active": 1}), + _action("sessions", at=3, path="/sessions", detail={"count": 1, "sessionIds": "s1"}), ] if duplicate: actions.append(_action("create-session", at=3.25, detail={"sessionId": "s2", "caps": "{}"})) @@ -897,6 +897,53 @@ def test_interrupted_session_create_is_reconciled_without_replay() -> None: assert all(item["ok"] for item in checks.values()) +@pytest.mark.parametrize( + ("query_at", "session_ids", "query_status", "post_status", "delete_id", "accepted"), + [ + (3, "s1", 200, "completed", "s1", True), + (4, "", 200, "completed", "s1", False), + (4, "s1", 200, "completed", "s1", False), + (3.5, "s1", 200, "completed", "s1", False), + (3, "", 200, "completed", "s1", False), + (3, "s10", 200, "completed", "s1", False), + (3, "s1", 500, "completed", "s1", False), + (3, "s1", 200, "failed", "s1", False), + (3, "s1", 200, "completed", "other", False), + ], +) +def test_interrupted_create_requires_observed_identity_before_delete( + query_at: float, + session_ids: str, + query_status: int, + post_status: str, + delete_id: str, + accepted: bool, +) -> None: + actions = _interrupted_create_actions() + actions[2] = _action( + "sessions", + at=query_at, + status=query_status, + path="/sessions", + detail={"sessionIds": session_ids}, + ) + actions[3]["detail"]["sessionId"] = delete_id + run = _run( + "session-create-interrupted", + variant="create-delay", + actions=actions, + calls=_timeout_calls(post_status=post_status), + sessions=[{"id": "s1", "preexisting": False, "active": False}], + final=_final( + taskOutcome="failed", + cleanup={"ownedSession": "completed", "server": "preserved"}, + ), + ) + checks = _checks(run) + assert checks["create-interruption-observed"]["ok"] + assert checks["create-interruption-reconciled-read-only"]["ok"] is accepted + + def test_interrupted_session_create_rejects_duplicate_session() -> None: run = _run( "session-create-interrupted", @@ -916,24 +963,30 @@ def test_interrupted_session_create_rejects_duplicate_session() -> None: assert not checks["create-interruption-no-replay"]["ok"] -def test_session_create_interruption_requires_response_still_pending() -> None: +@pytest.mark.parametrize(("completed_at", "pending"), [(3.5, True), (3.6, False), (4, False)]) +def test_session_create_interruption_requires_response_still_pending( + completed_at: float, pending: bool +) -> None: calls = [ _python_call( - completed="1970-01-01T00:00:04Z", + completed=f"1970-01-01T00:00:{completed_at:06.3f}Z", status="timed_out", kernel_state="alive", success=False, - arguments={"code": "create then unrelated wait", "timeout": 4}, + arguments={"code": "create then unrelated wait", "timeout": 1}, ), _python_call( started="1970-01-01T00:00:05Z", completed="1970-01-01T00:00:06Z", ), ] + actions = _interrupted_create_actions() + actions[2]["at"] = 5.2 + actions[3]["at"] = 5.5 run = _run( "session-create-interrupted", variant="create-delay", - actions=_interrupted_create_actions(), + actions=actions, calls=calls, sessions=[{"id": "s1", "preexisting": False, "active": False}], final=_final( @@ -942,7 +995,7 @@ def test_session_create_interruption_requires_response_still_pending() -> None: ), ) checks = _checks(run) - assert not checks["create-interruption-observed"]["ok"] + assert checks["create-interruption-observed"]["ok"] is pending def test_kernel_death_structured_cleanup_requires_delete_evidence() -> None: