Repository navigation
fix(workspaces): read hosts concurrently so the page stops hanging - #2008
henfircreo wants to merge 27 commits into
Conversation
Every transcript read eager-loads `Message.files`, which is a `WHERE message_id IN (:page of message ids)` against `chat_files`. Every index on that table led with `user_id`, so opening a conversation scanned it whole - once per transcript, and again for every run transcript through `get_messages_by_run`. Verified with the chain applied forwards, back and forwards again, and `alembic check` reporting no drift against the model.
`MessageFileRead` serializes four scalars, so an attachment on a turn costs a card. The row behind it also holds `parsed_content` - the whole extracted text of the upload, a contract or a spreadsheet in full - and `selectinload(Message.files)` fetched every byte of it to serialize none of it. The column is `Text`, so the database decompressed each TOAST chunk to hand it over. Narrowed at the three query sites that serialize that card: the thread transcript, the run transcript and the whole-conversation read. Done per query rather than by deferring the column on the model, because `services/attachments.py` builds the model's prompt out of it and `api/routes/v1/files.py` previews it - under asyncio a deferred column those paths touch is a `MissingGreenlet`, not a second query. `raiseload=True` so a caller that does touch it is told which attribute and why, rather than being handed the event loop's complaint.
`GET /conversations/{id}/messages` awaited `list_messages` and
`conversation_cost`, and each called `get_conversation` for itself. So
every transcript read resolved its conversation twice - and on a thread
reached through a channel that is two membership checks, each able to
unseal a bot token and ask Slack or Telegram whether the reader is still
in the room, against a sixty-second cache that fails open to the network
call when Redis is unreachable.
The page and the total are one read, so they are one call now:
`ConversationService.transcript` answers with the items, the total and
the cost after authorizing once. Both former methods had exactly one
caller each - this route - so merging them is the change rather than a
wrapper around them; leaving them would have made them production-dead.
`_cost` is private because the authorization is its caller's. The
refusal still precedes the aggregate, which is its own test: an
implementation that totalled the thread first and threw afterwards would
still refuse, and would be one `try` away from a leak.
Nothing in the stack compressed anything - no `GZipMiddleware`, no
`gzip` in the Nginx template, no `compress` middleware in Traefik. A
transcript is what that cost: `GET /conversations/{id}/messages` answers
with up to a hundred turns carrying their reasoning, their `parts`
timeline and every tool call's arguments and result, and that JSON went
out raw over whatever link the reader happened to be on.
Registered outermost, which is a decision rather than a default. Every
layer below is a `BaseHTTPMiddleware` that re-emits the body through an
anyio stream, so compressing underneath them would push gzipped bytes
through two of those for nothing; and `Vary: Accept-Encoding` has to
reach an answer an inner layer short-circuits, because a response a
shared cache stored without it is served gzipped to a client that never
asked.
Starlette handles the hazards itself, so none of them needed a wrapper:
a non-HTTP scope passes straight through, which is the chat WebSocket;
`text/event-stream` and already-compressed media are excluded; a 206 is
left alone; a body already carrying `Content-Encoding` is untouched; a
streaming chunk is flushed with `Z_SYNC_FLUSH` rather than held back;
and a body over 128 KiB compresses in a worker thread rather than on the
event loop. Level 5 rather than the default 9 - on JSON the last four
levels buy a couple of percent for several times the CPU, on a
deployment whose load test already found it CPU-bound.
The exclusion list is Starlette's own, splatted so an addition upstream
is inherited, plus PDF, octet-stream and the OOXML types. Those are
already-compressed containers served by the download routes, and
compressing a streamed response drops the `Content-Length` the browser
draws its download progress from.
No proxy configuration follows from this. Both halves now compress what
they answer with, so `gzip on` at the proxy would be a line that reads
as a fix and does nothing; the hazard is the opposite, and the Nginx
template says so - blanking the client's `Accept-Encoding` is how the
usual "let the proxy compress it" recipe starts, and here it only turns
the compression off.
…pt-load Perf/conversation transcript load
Long report-generation runs were cut off at the provider's 600s read default. The agent model_settings.timeout cap and the Builder input now allow up to 1800s so a long run can finish. Backend: test_model_settings.py 46 passed.
…800s feat(agents): raise model request timeout ceiling to 1800s
…1800s updated dockerfile
…1800s update docker
A model writing a large tool call - a whole report handed to write_file - streams the arguments for minutes, and the step only appeared on tool_call, once they were complete. Until then the chat showed a blinking cursor and nothing else. part_start now carries tool_name and tool_call_id for a ToolCallPart. The web chat draws the step as pending straight away, counts the tool_call_delta characters into "N KB so far", and fills the same step in on tool_call. A call cut off by an error, a cancelled run or the stop button is marked unfinished rather than left spinning. Verified with the agent session and embed frame tests (run_stream at 100%) and the use-chat and tool-call-card vitest suites. The full frontend coverage gate could not run locally (sandbox EPERM on temp).
…ing-progress fix(chat): show a tool call while its arguments stream
pydantic-ai's web_fetch_tool hands a PDF back as BinaryContent, which the OpenAI-chat model sends as a `file` content part. A model behind an OpenAI-compatible endpoint that takes no document parts (self-hosted vLLM, directly or through LiteLLM) refused the whole next request with "Unsupported chat content part type: 'file'", and the agent then fetched the same PDF again. The local fetcher is now a WebFetchLocalTool subclass: a page and an image pass through unchanged; any other binary is read by the attachment parser (lifted into a session-less DocumentText base of FileUploadService, with an extract_text helper) and cut at max_content_chars; a binary with no readable text is a ModelRetry naming its media type. The download and its SSRF guard are still the library's. Verified with test_web_fetch (web_fetch at 100%) and the attachment, upload and classification suites.
…s-text fix(web-fetch): return fetched documents as text, not raw bytes
# Conflicts: # CHANGELOG.md # docs/channels.de.md # docs/channels.es.md # docs/channels.pl.md
chore: sync with vstorm-co/agenticos main (0.0.514)
Reuse detection ended the session whenever the refresh token a rotation had just spent was presented again. Honest clients do that all the time: two tabs refreshing on the shared cookie (every tab's timer fires at once after a laptop wakes), or a refresh whose response was lost to a VPN reconnect. People were signed out several times a day. - backend: a spent token refreshes once more within REFRESH_REUSE_GRACE_SECONDS (default 60) of its rotation, via SessionService.claim_refresh_grace; outside it, detection is unchanged. Adds sessions.rotated_at (0101_session_rotated_at). - frontend: refreshes and /auth/me reads are serialized across tabs with the Web Locks API, and a tab that finds a refresh from the last 10 s retries with the rotated cookies instead of spending them again. - the /api/auth/me and /api/auth/refresh proxies clear the cookies only on a 401, not on a 502 during a redeploy or a network error, and /me keeps the rotated cookies when the read after a refresh fails. Tradeoff: a stolen token replayed within the window is accepted rather than detected. Web Locks exist only in a secure context; over plain HTTP the backend window is the only protection.
fix(auth): stop spurious sign-outs from refresh-token rotation
Merge vstorm-co/agenticos main (0.0.515, 0.0.516) into the Creo fork. Upstream absorbed Creo's refresh-token grace fix as 0102_session_rotated_at. Creo's own 0101_session_rotated_at is dropped so the chain is linear (0100 -> 0101_artifact_pages -> 0102 -> 0103). Creo's burst reissue (same successor for every grace refresh) is kept, with upstream's guards against a rotation stamped in the future. Also removes pytest temp directories committed by accident in edb3ce3.
chore: sync main with upstream v0.0.516
The Workspaces page opens on the flat "every file" listing, which read up to 25 container-backed workspaces one after another, then fetched their thumbnails one after another, each on a fresh archive client. The page waited for the sum of those round trips, and a host that had gone away held it for the archive's 60-second default timeout. Workspaces and thumbnails are now read concurrently, at most eight at a time, and a listing gives up on a host after ten seconds. The thumbnail budget is still shared out in listing order before anything is fetched, so which tiles are drawn does not depend on which host answers first. Connection resolution is serialised behind a lock because the concurrent readers share one database session. The "Count files" switch reads hosts the same way.
DEENUU1
left a comment
DEENUU1
left a comment
There was a problem hiding this comment.
The concurrency change in sandbox_workspace.py fixes the problem it describes: results keep their listing order, the thumbnail budget is still shared out in listing order before any fetch, the connection cache sits behind a lock, and the barrier test is a good regression test. Two things block the merge.
Blocking
1. The branch carries unrelated changes from a fork's main (scope and merge gate)
The merge commit 04ea98766 ("Merge branch 'main' into fix/workspaces-slow-load") brought in Creo-Digital-AS/main, not upstream main. Of the 26 changed files, three belong to this fix. A squash would land all of the following under fix(workspaces): …, none of it described in the PR:
| Change | Files |
|---|---|
A new refresh-token rotation design (HMAC-derived deterministic successor, reissue_within_grace) |
api/routes/v1/auth.py, core/security.py, services/session.py, tests/integration/test_session_revocation.py, tests/test_session_refresh_grace.py, docs/security*.md, docs/configuration*.md, two CHANGELOG entries |
Logout on a refresh 401; /auth/me through apiClient |
frontend/src/lib/api-client.ts, frontend/src/hooks/use-chat.ts (+ tests) |
ModelSettingsSpec.timeout ceiling 600 → 1800 s, which changes publish-time validation of the agent spec |
app/agents/spec.py, model-settings-form.tsx (+ tests) |
| BuildKit cache and bind mounts removed from the image build | backend/Dockerfile |
| A fork-internal ops script ("Undoes Creo's 0101_session_rotated_at") | scripts/revert_0101_session_rotated_at.sql |
The auth change is a security-relevant redesign and needs its own PR and its own review. The Dockerfile change makes upstream builds slower and is not explained. The SQL script only applies to the fork's database.
Fix: rebuild the branch from upstream so it holds only 290439751, then force-push:
git fetch upstream
git switch -c fix/workspaces-slow-load upstream/main
git cherry-pick 290439751The other changes can come as separate PRs if you want them upstream. This also clears the current conflict, which is only CHANGELOG.md.
2. CI test fails on a new test (merge gate): see the inline comment on test_the_budget_goes_to_the_first_workspaces_listed. It is the only failure in the run (1 failed, 11248 passed, coverage 100%), and it reproduces locally.
Suggestions and nits
Inline: own the concurrent reads with a TaskGroup, the nested thumbnail fan-out, a test for the lock, and two small nits.
One more nit with no line to anchor to: _BROWSE_TIMEOUT_SECONDS goes into httpx.Timeout, so it limits each phase of each request, not the whole walk. A slow host can still take N×10 s over a deeper walk. The Limitations section says this already. The CHANGELOG's "reported after ten seconds" is only exact for a host that does not answer at all.
Verification
- Locally, at
04ea98766:tests/test_sandbox_workspace.py,tests/api/test_workspace_browser_routes.pyandtests/test_security_marker.py: 236 passed, 1 failed (the marker test below). The fullmake testandmake checkwere not run. - CI:
Security Scanfails onbun audit(braces, GHSA-vfj7-8cjw-p6xm). That is also red onmain(b2d8e70ca), so it does not come from this PR.lintanddocswere cancelled, so they have not been checked for this head.
| HOST_THUMBNAIL_BUDGET | ||
| ) | ||
|
|
||
| async def test_the_budget_goes_to_the_first_workspaces_listed( |
There was a problem hiding this comment.
Blocking (CI gate). This name matches the budget keyword in tests/test_security_marker.py, and the test is neither marked @pytest.mark.security nor listed in EXEMPT, so test_keyword_matching_tests_carry_the_marker_or_are_exempt fails. This is the only failure in the test job.
It is about the thumbnail fetch budget, not a spend-budget refusal, so add it to EXEMPT with a one-line reason, for example:
"tests/test_sandbox_workspace.py::TestDrawingAHostsImages::test_the_budget_goes_to_the_first_workspaces_listed": "thumbnail fetch budget ordering, not a spend budget",| async with gate: | ||
| return await call() | ||
|
|
||
| return list(await asyncio.gather(*(_one(call) for call in calls))) |
There was a problem hiding this comment.
Suggestion. asyncio.gather without return_exceptions re-raises the first exception but does not cancel the other calls. Every call catches its own errors today (_remote_entries, _host_thumbnail), so this is a latent risk, not a reproduced bug. But if a future change lets one of them raise, the remaining reads keep running after the request has failed, and they can still reach _connection, and through it the request's AsyncSession. A TaskGroup settles all of them before the block exits, and Python 3.12 is pinned:
async with asyncio.TaskGroup() as group:
tasks = [group.create_task(_one(call)) for call in calls]
return [task.result() for task in tasks](Sketch, not run. A failure then surfaces as an ExceptionGroup.)
| if scaled is not None: | ||
| drawn[path] = scaled | ||
| paths, budget = thumbnail_picks(entries, budget) | ||
| tiles = await _bounded([partial(self._host_thumbnail, ctx, row, path) for path in paths]) |
There was a problem hiding this comment.
Suggestion. This is a second, nested _bounded with its own semaphore, inside the one in flat_files (line 1052). The result is up to 8 concurrent read_bytes per host and up to 24 (HOST_THUMBNAIL_BUDGET) in flight per request. Each runs on its own httpx.Client with the 60 s default timeout. _tiled, the per-workspace view, now sends 8 parallel reads to one host where it used to send one.
All of them go through asyncio.to_thread, which uses the process-wide default executor (min(32, cpu+4) threads) that about 20 other modules in backend/app also use. One page load against a host that is slow on read_bytes can take most of that pool for up to a minute.
Options: share one semaphore per request between the listings and the thumbnails, or cap thumbnails per host lower. Either way, please update the description, which says "at most 8 hosts at a time".
| cached = await self.connections.resolve(ctx, connection_id) | ||
| self._connections_by_id[connection_id] = cached | ||
| return cached | ||
| async with self._resolving: |
There was a problem hiding this comment.
Test gap. Nothing fails if this lock is removed. The barrier test uses three different connection_ids with an instant AsyncMock resolve. A test where resolve awaits an asyncio.Event and records the maximum number of concurrent entries would show the lock does what its comment says: two workspaces on one connection resolve once, and resolves never overlap on the shared session.
| yield None | ||
| return | ||
| archive = WorkspaceArchive(resolved.row.base_url, token=resolved.token) | ||
| archive = ( |
There was a problem hiding this comment.
Nit. The two-branch constructor could be a single call: give _archive timeout: float = DEFAULT_TIMEOUT_SECONDS (from pydantic_ai_backends.remote.client) and always pass it through.
| if path != ".": | ||
| return [] | ||
| if session_id == "slow": | ||
| time.sleep(0.2) |
There was a problem hiding this comment.
Nit. The budget is shared out only after every listing finishes, so this test passes whatever order the hosts answer in, and the sleep does not test the ordering. It is harmless, but a comment saying so would help, or an Event that makes "slow" finish last.
|
Thanks @henfircreo — merged as #2017. Your commit was carried unchanged onto a |
### Summary Release 0.0.522: version, lock and changelog. ### Changes - `backend/pyproject.toml`, `frontend/package.json` and `backend/uv.lock` move to 0.0.522. - `CHANGELOG.md`: the `[Unreleased]` block becomes `[0.0.522] - 2026-10-06`, with a fresh empty `[Unreleased]` above it. - Ships #2017 (from #2008): the Workspaces page reads hosts and thumbnails side by side on a process-wide pool of eight threads of its own, and a listing gives up on an unanswered host call after ten seconds.
2 participants
Problem
/workspacesstays on its loading state for a long time before it draws. The page opens on the "All files" view, which callsGET /api/v1/workspaces/files. InSandboxWorkspaceService.flat_filesthe backend:lsround trips to its sandbox host.WorkspaceArchivedefault timeout (60 seconds) on a host that didn't answer, before it reported that workspace as unreadable.So the page waited for the sum of every round trip, or a whole minute for each dead host.
Change
_HOST_READS_AT_ONCE), and results keep their listing order.measured(hosts=True), which backs the "Count files" switch, uses the same path._BROWSE_TIMEOUT_SECONDS). File downloads keep the archive's default timeout.AsyncSession, and resolving a connection queries it and unwraps a vault secret.thumbnail_picks._tilesand_tiledkeep their contracts.Verification
tests/test_sandbox_workspace.py:lsat the same time (athreading.Barrier); read one at a time, all three time out there. Fails onmain.main.tests/test_sandbox_workspace.pyandtests/api/test_workspace_browser_routes.py, 231 passed.app/services/sandbox_workspace.pyis at 100% coverage. Lint: ruff, ruff format, vulture, deptry and the route guard pass, andtyis clean on the changed file.make testgate andmake check, so CI is the check for those. Two prune tests that use a real shell were left out because they fail onmainon Windows too, andtyon the whole tree flagssignal.SIGKILLinapp/core/watchdog.pyfor the same platform reason.Limitations