Verify live_write notification teardown by end state - #313
Merged
Conversation
Teardown decided whether a seeded notification leaked by counting failed deleteNotification mutations. The bin holding the notification is unknown at teardown, so both bins are tried and at least one call is a no-op; the count tracks the API's error conventions rather than whether anything leaked. It false-alarms when the delete test already removed the entry, and hides a real failure when the wrong-bin delete no-ops successfully. delete_notification_any_state now pages both bins after the delete attempts and reports an orphan only when the id is still present, with every delete error attached to an ExceptionGroup. A read-back that itself fails is reported rather than treated as success. The session-end orphan scan parsed structured_content as a bare list, which the tool never returns, so it had never reported an orphan; it now unwraps the result envelope and scans the archive bin as well as unread. The helpers move to tests/live_write/_teardown.py, fixture-free like _gates.py, so unit tests can import them without activating the live suite's session-scoped autouse gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
_delete_notification_any_statedecided whether a seeded notification leaked by counting faileddeleteNotificationmutations. The bin holding the notification is unknown at teardown, so both bins are tried and at least one call is a no-op — and the API may report a no-op either as success or as an error. The count therefore tracks error conventions rather than whether anything leaked: it raises aCLEANUP FAILEDbanner for a notification the delete test already removed, swallows a real failure when the wrong-bin delete no-ops successfully, and surfaces onlyfailures[0], which is the benign wrong-bin error.Changes
delete_notification_any_statereads the end state back after both delete attempts and reports an orphan only when a bin still holds the id. Both bins are paged to the end, sinceQUERY_NOTIFICATIONSrequests no sort order. Every delete error is attached to anExceptionGroup, so both causes reach stderr throughrun_cleanup. A read-back that itself fails is reported rather than treated as success.structured_contentas a bare list. For a tool returninglist[Notification]FastMCP always returns a{"result": [...]}envelope, so theisinstance(notifs, list)guard returned early on every run and the scan had never reported an orphan. It now unwraps the envelope throughselect_mcptest_orphans, lists ARCHIVE as well as UNREAD, passes an explicit page limit instead of the tool's default of 50, and still names any orphans it collected when a later bin fails to enumerate.build_raw_clientand the teardown helpers move fromtests/live_write/conftest.pyto the new fixture-freetests/live_write/_teardown.py, following_gates.py: importing the conftest registers its session-scoped autouse fixtures and would skip the run.tests/unit/test_live_write_teardown.pydrives the helper offline against a fake client that models both wrong-bin API behaviours. It covers an already-deleted notification not being an orphan, a single real failure alongside a no-op success not being swallowed, both delete errors reaching the report, an orphan detected in the archive bin and past the first page, single-page bins costing one request, an unverifiable end state raising, and the client being closed on the raising path. A separate test pins thestructured_contentenvelope the orphan scan parses, so a FastMCP change fails in CI rather than silently emptying the backstop.Eight of the nineteen tests fail against the previous implementation.
tests/live_write/remains gated behind thelive_writemarker,UNRAID_ALLOW_LIVE_WRITES=1, and themcptest*name guard, and is deselected in CI.Closes #296
🤖 Generated with Claude Code