Skip to content

fix(runner): use export endpoint for get_session_events to prevent freeze#1019

Merged
Gkrumbach07 merged 2 commits intomainfrom
worktree-better-acp-tools
Mar 25, 2026
Merged

fix(runner): use export endpoint for get_session_events to prevent freeze#1019
Gkrumbach07 merged 2 commits intomainfrom
worktree-better-acp-tools

Conversation

@Gkrumbach07
Copy link
Copy Markdown
Contributor

Summary

  • Root cause: acp_get_session_status was calling the /agui/events endpoint, which is an SSE (Server-Sent Events) stream that never closes. urllib.request.urlopen blocks waiting for the response to finish, causing the agent to freeze for 30 seconds (the timeout).
  • Fix: Switch to the /export endpoint which returns all historical AG-UI events as a regular JSON response, no streaming.

Follows up on #1006.

Test plan

  • 22 tests pass
  • Manual: call acp_get_session_status on a running session — should return immediately with status + recent messages

🤖 Generated with Claude Code

…n_events

The /agui/events endpoint is an SSE stream that never closes, causing
urllib.request.urlopen to block indefinitely (30s timeout). Switch to
the /export endpoint which returns historical events as regular JSON.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

Walkthrough

Session events retrieval was changed to use the session /export endpoint and a fixed max-events approach. The BackendAPIClient signature removed thread_id and limit in favor of max_events and now parses aguiEvents (list or JSON string). Caller code was updated to call the API with only the session identifier.

Changes

Cohort / File(s) Summary
Backend API — session export
components/runners/ambient-runner/ambient_runner/tools/backend_api.py
Replaced /agui/events SSE-style fetch with /export fetch. Removed thread_id/limit parameters; added max_events and class constant _MAX_EXPORT_EVENTS = 200. Parses aguiEvents as list or JSON string and returns the last N events.
Caller update — session status
components/runners/ambient-runner/ambient_runner/bridges/claude/backend_tools.py
Updated acp_get_session_status to call get_session_events(session_name=...) without heuristic limit scaling; removed the related explanatory comment. Event extraction/truncation logic unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: switching from the SSE /agui/events endpoint to the /export endpoint to prevent agent freeze.
Description check ✅ Passed The description is directly related to the changeset, explaining the root cause (SSE streaming blocking behavior) and the solution (switching to /export endpoint).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-better-acp-tools

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/runners/ambient-runner/ambient_runner/tools/backend_api.py`:
- Around line 293-305: The current export call fetches the entire session
history which can be huge; modify the request to use a bounded contract (append
a limit query param to the path variable before calling _make_request, e.g.
"/projects/{self.project_name}/agentic-sessions/{session_name}/export?limit={MAX_EXPORT_EVENTS}")
and introduce a local cap constant (e.g. MAX_EXPORT_EVENTS) to guard
client-side; after receiving response in the current method, parse as before but
if events is a list or a decoded JSON string, trim/slice it to MAX_EXPORT_EVENTS
before returning to avoid full-history transfers and parses.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7799a783-198f-493b-9f57-18604988ff32

📥 Commits

Reviewing files that changed from the base of the PR and between 0467664 and a3a71bb.

📒 Files selected for processing (2)
  • components/runners/ambient-runner/ambient_runner/bridges/claude/backend_tools.py
  • components/runners/ambient-runner/ambient_runner/tools/backend_api.py

…essions

Tail-slice events to last 200 (configurable via max_events param) to
avoid fetching/parsing entire session histories. The export endpoint
returns all events, which can be huge for long-running sessions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/runners/ambient-runner/ambient_runner/tools/backend_api.py`:
- Around line 277-278: The constant _MAX_EXPORT_EVENTS was lowered to 200, which
breaks the previous behavior of fetching limit=max_messages * 10 events; to fix
this, update the call in backend_tools.py that fetches exported events (the call
that currently only passes max_messages) to pass an explicit
max_events=max_messages * 10 (or alternatively restore _MAX_EXPORT_EVENTS to
500) so the event fetch retains the previous multiplier and sparse
TEXT_MESSAGE_CONTENT events don't drop messages.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 60b14e65-42a5-4d9a-b7e2-173c4231797a

📥 Commits

Reviewing files that changed from the base of the PR and between a3a71bb and c2b7a32.

📒 Files selected for processing (1)
  • components/runners/ambient-runner/ambient_runner/tools/backend_api.py

@Gkrumbach07 Gkrumbach07 merged commit 93b0fe4 into main Mar 25, 2026
21 of 23 checks passed
@Gkrumbach07 Gkrumbach07 deleted the worktree-better-acp-tools branch March 25, 2026 16:02
jeremyeder pushed a commit to jeremyeder/platform that referenced this pull request Mar 26, 2026
…eeze (ambient-code#1019)

## Summary

- **Root cause**: `acp_get_session_status` was calling the
`/agui/events` endpoint, which is an SSE (Server-Sent Events) stream
that never closes. `urllib.request.urlopen` blocks waiting for the
response to finish, causing the agent to freeze for 30 seconds (the
timeout).
- **Fix**: Switch to the `/export` endpoint which returns all historical
AG-UI events as a regular JSON response, no streaming.

Follows up on ambient-code#1006.

## Test plan

- [x] 22 tests pass
- [ ] Manual: call `acp_get_session_status` on a running session —
should return immediately with status + recent messages

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Ambient Code Bot <bot@ambient-code.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant