feat(plugins): extend public subagent lifecycle contract - #72630
Conversation
Related: #72501 merged the v1 lifecycle API; this PR extends that API to contract v2 with distinct process-local context, progress, and relaunch behavior. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for extending the lifecycle boundary while keeping child objects opaque and reusing the existing durable delivery rail.
Problems
tools/async_delegation.py:679-685has a race for deterministic notification ids. Two concurrent publishers can both pass the lookup at:732-734; the loser then hits the durable uniqueness error and unconditionally removes_records[delegation_id]at:685, which may be the winner's record. The winner subsequently cannot finalize because_begin_finalization()requires that in-memory record (:969-972), leaving its durable row running without a completion event.tests/agent/test_subagent_lifecycle.py:397-415exercises duplicate publication sequentially, so it does not cover this interleaving.
Suggested changes
- Make registration/reservation ownership-aware and atomic; only remove the exact record created by the failed attempt.
- Add a barrier-based concurrent duplicate-publication regression test asserting one terminal row and one delivered event.
Current main remains at lifecycle contract v1 (agent/subagent_lifecycle.py:25), so the extension is not superseded. GitHub currently marks this branch dirty against main, so salvage also requires reconciling current production-file changes.
Automated hermes-sweeper review.
| "external_producer": True, | ||
| } | ||
| with _records_lock: | ||
| _records[delegation_id] = record |
There was a problem hiding this comment.
This assignment is not ownership-safe against concurrent deterministic notification registration. Two callers can both pass the lookup in publish_external_notification; if the later durable INSERT raises IntegrityError, its cleanup at line 685 unconditionally removes this id and can delete the winning caller's in-memory record. Reserve atomically or only remove the exact record created by this attempt.
Summary
subagent.progressmilestones through the existing host surfaceSafety boundaries
Validation
python -m pytest -q tests/agent/test_subagent_lifecycle.py tests/tools/test_async_delegation.py— 66 passedorigin/main— cleanThe consumer connector and any provider/live acceptance remain outside this public host PR.