sdk: add spawn/release lifecycle hooks in TS and Python#477
sdk: add spawn/release lifecycle hooks in TS and Python#477willwashburn merged 4 commits intomainfrom
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR adds per-call spawn and release lifecycle hooks (onStart, onSuccess, onError) to both the TypeScript and Python SDKs. These hooks allow callers to observe the outcome of individual spawn / release calls without relying solely on relay-level event hooks. The changes maintain full backward compatibility (e.g., agent.release("reason") still works in both languages).
Changes:
- TypeScript (
relay.ts): New lifecycle context/hook interfaces, updatedspawnPty,spawn,createSpawner, andmakeAgentto fireonStart/onSuccess/onErrorhooks around each network call; new helper methodsinvokeLifecycleHook,resetAgentLifecycleState,normalizeReleaseOptions. - Python (
relay.py): Same lifecycle hook semantics added toSpawnOptions,AgentSpawner.spawn(),AgentRelay.spawn(), andAgent.release(), with matching helper methods. - Tests & Docs: New TS and Python test cases for success/error hook paths, updated READMEs and reference docs with hook options.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/sdk/src/relay.ts |
Core hook implementation for TS SDK: new interfaces, updated spawn/release methods, helper utilities |
packages/sdk/src/__tests__/orchestration-upgrades.test.ts |
New TS tests covering spawn and release hook success/error paths |
packages/sdk/README.md |
Updated quick-start example to show spawn and release lifecycle hooks |
packages/sdk-py/src/agent_relay/relay.py |
Core hook implementation for Python SDK: updated SpawnOptions, AgentSpawner, AgentRelay, and Agent.release() |
packages/sdk-py/tests/test_relay_lifecycle_hooks.py |
New Python tests for spawn and release hook success/error paths |
packages/sdk-py/README.md |
Updated quick-start example to show Python lifecycle hooks |
docs/reference/sdk.mdx |
Updated TS reference: spawn options table with hook columns, updated Agent.release() signature |
docs/reference/sdk-py.mdx |
Updated Python reference: spawn options table with hook columns, updated Agent.release() signature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self._relay._invoke_lifecycle_hook( | ||
| on_start, | ||
| context, | ||
| f'spawn("{agent_name}") on_start', | ||
| ) | ||
| client = await self._relay._ensure_started() |
There was a problem hiding this comment.
In AgentSpawner.spawn(), on_start is invoked (lines 318-322) before _ensure_started() is called (line 323). This means:
- If
_ensure_started()raises (e.g., broker fails to start),on_startwill have already fired buton_errorwill never be called, leaving the caller with a started hook and no error notification. - This is inconsistent with
AgentRelay.spawn()(and the TypeScript implementation), where_ensure_started()/ensureStarted()is always called before any lifecycle hooks fire.
The fix is to move _ensure_started() to before the on_start invocation, matching both the AgentRelay.spawn() pattern and the TypeScript behavior.
| self._relay._invoke_lifecycle_hook( | |
| on_start, | |
| context, | |
| f'spawn("{agent_name}") on_start', | |
| ) | |
| client = await self._relay._ensure_started() | |
| client = await self._relay._ensure_started() | |
| self._relay._invoke_lifecycle_hook( | |
| on_start, | |
| context, | |
| f'spawn("{agent_name}") on_start', | |
| ) |
| }): Promise<Message>; | ||
|
|
||
| release(reason?: string): Promise<void>; | ||
| release(reasonOrOptions?: string | ReleaseOptions): Promise<void>; |
There was a problem hiding this comment.
The release(reasonOrOptions?: string | ReleaseOptions) signature references ReleaseOptions, but the documentation does not describe the shape of this type (its reason, onStart, onSuccess, and onError fields). Users who only refer to the reference docs won't know how to use the hook-based overload. Consider adding a ReleaseOptions properties table similar to the spawn options table, or at least a brief inline description near the Agent section.
|
Addressed both review findings:\n\n1) Python shorthand spawner ordering: moved ensure_started before on_start in AgentSpawner.spawn, so startup failures no longer emit on_start without a matching completion path. Added regression test: test_shorthand_spawn_does_not_fire_start_hook_if_broker_startup_fails.\n\n2) TS docs clarity: added a ReleaseOptions section in docs/reference/sdk.mdx documenting reason, onStart, onSuccess, and onError.\n\nAlso re-ran Python lifecycle hook tests (7 passed). |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Addressed the latest lifecycle-hook comment about release startup failures:\n\n- Moved ensureStarted/_ensure_started before onStart in both TS and Python release paths, so startup failures do not emit onStart without completion hooks.\n- Added regression tests in both SDKs verifying no release lifecycle hooks fire when startup fails before release begins.\n\nValidation:\n- cd packages/sdk && npx vitest run src/tests/orchestration-upgrades.test.ts (33 passed)\n- cd packages/sdk-py && uv run --with pytest --with pytest-asyncio pytest tests/test_relay_lifecycle_hooks.py (8 passed) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async release(reasonOrOptions?: string | ReleaseOptions) { | ||
| const releaseOptions = relay.normalizeReleaseOptions(reasonOrOptions); | ||
| const releaseContext: ReleaseLifecycleContext = { | ||
| name, | ||
| reason: releaseOptions.reason, | ||
| }; | ||
| const client = await relay.ensureStarted(); | ||
| await client.release(name, reason); | ||
| await relay.invokeLifecycleHook(releaseOptions.onStart, releaseContext, `release("${name}") onStart`); |
There was a problem hiding this comment.
In Agent.release, onStart fires on line 1070 before ensureStarted() is called on line 1071. If ensureStarted() throws (e.g., broker connection lost), the error propagates without calling onError. This creates an inconsistency: onStart fired, signaling that the release process has begun, but onError is skipped even though the operation failed.
To fix this, either:
- Move
ensureStarted()before theonStartcall (matching the spawn behavior, where the broker is started before any hooks fire), or - Wrap the
ensureStarted()call inside the existingtry/catchblock soonErroris also invoked for connection failures.
| context = { | ||
| "name": self._name, | ||
| "reason": reason, | ||
| } | ||
| client = await self._relay._ensure_started() | ||
| await client.release(self._name, reason) | ||
| await self._relay._invoke_lifecycle_hook( | ||
| on_start, | ||
| context, | ||
| f'release("{self._name}") on_start', | ||
| ) |
There was a problem hiding this comment.
In Agent.release, on_start fires on line 114 before _ensure_started() is called on line 119. If _ensure_started() throws (e.g., broker connection lost), the error propagates without invoking on_error. This creates an inconsistency where on_start fires but on_error is skipped even though the operation failed.
To fix this, either:
- Move
_ensure_started()before theon_startcall (matching the spawn behavior where the broker is started before any hooks fire), or - Wrap the
_ensure_started()call inside the existingtry/exceptblock soon_erroris also invoked for connection failures.
Summary
Validation