Follow-up from #306. Two distinct problems that happen to share a cause, and they are not the same severity.
1. A roster read is now a write — this is the design property, not the optimisation gap
listAgents calls sweepStaleAgents synchronously before selecting, so GET /v1/agents issues UPDATE agents on every request. getAgentByName does the same on the detail endpoint.
// packages/engine/src/engine/agent.ts
export async function listAgents(db: Db, workspaceId: string, status?: string) {
await sweepStaleAgents(db, workspaceId);
const rows = await db.select().from(agents)...
A read that mutates is a property worth choosing deliberately rather than inheriting. Consequences, separate from cost:
- Unbounded write amplification on the first read after a quiet period. The sweep has no batch limit and no stagger; it flips every stale row in the workspace in one statement. On a workspace with 877 agents, of which 308 are stale-active, the first
agent list after a deploy writes 308 rows.
- Reads are no longer safe to issue freely — from a dashboard poll, a health check, a retry loop, or a read replica path. Anything that lists agents now takes a write lock.
- D1 has no interactive transactions, so this write sits outside any surrounding read consistency.
#306 removed the identity consequence of this: the cross-node name-reclaim guard used to branch on the status column that this sweep rewrites, so a roster read could widen who was allowed to claim an agent's identity. That coupling is gone — the guard now reads last_seen, which a read cannot write.
But the read-writes-on-every-GET behaviour itself remains. The security coupling and the read/write coupling shared a cause; only the first was fixed. Worth stating explicitly so this is not assumed closed by #306.
Options, not a recommendation:
- restore a scheduled sweep (Cloudflare Cron Trigger) and drop it from the read path — the original design before the Fly→Workers migration dropped the timer
- keep deriving status from
last_seen on read (which is already what makes correctness independent of the sweep) and let durable convergence be the scheduled job's problem
- keep the read-path sweep but make it best-effort and non-blocking, so a roster read does not await a write
- rate-limit or debounce it per workspace
Note the derived value already makes reads correct without the sweep — effectiveAgentStatus computes from last_seen. So the write on the read path is buying durable convergence, not correctness, which makes it a good candidate to move off the hot path.
2. The SQL status pushdown was replaced with an in-memory filter — P3
Reported by cubic on #306. listAgents now selects the whole workspace and filters after mapping:
const rows = await db.select().from(agents).where(and(eq(agents.workspaceId, workspaceId), ne(agents.status, RELEASED_AGENT_STATUS)));
...
.filter((agent) => !requestedStatus || requestedStatus === 'all' || agent.status === requestedStatus);
Previously ?status=active pushed the predicate into SQL and loaded only matching rows. Now every roster request materialises the full workspace: 877 rows here, 20,107 fleet-wide across workspaces.
This is a genuine regression but a mild one, and it is not trivially reversible — the filter is applied to the derived status, which differs from the stored column precisely for the stale active/online rows this feature is about. cubic's suggestion is sound: push the predicate into SQL when a filter is requested, accounting for the fact that derived and stored differ only in that one direction.
Not a blocker for #306
Both are pre-existing to that PR's design rather than introduced by its final commits, and fixing them means reworking the derive-vs-filter logic the presence contract rests on. Holding a security fix on a performance and design refactor is the wrong trade. Filed so it survives the merge rather than riding under a green check.
Follow-up from #306. Two distinct problems that happen to share a cause, and they are not the same severity.
1. A roster read is now a write — this is the design property, not the optimisation gap
listAgentscallssweepStaleAgentssynchronously before selecting, soGET /v1/agentsissuesUPDATE agentson every request.getAgentByNamedoes the same on the detail endpoint.A read that mutates is a property worth choosing deliberately rather than inheriting. Consequences, separate from cost:
agent listafter a deploy writes 308 rows.#306 removed the identity consequence of this: the cross-node name-reclaim guard used to branch on the
statuscolumn that this sweep rewrites, so a roster read could widen who was allowed to claim an agent's identity. That coupling is gone — the guard now readslast_seen, which a read cannot write.But the read-writes-on-every-GET behaviour itself remains. The security coupling and the read/write coupling shared a cause; only the first was fixed. Worth stating explicitly so this is not assumed closed by #306.
Options, not a recommendation:
last_seenon read (which is already what makes correctness independent of the sweep) and let durable convergence be the scheduled job's problemNote the derived value already makes reads correct without the sweep —
effectiveAgentStatuscomputes fromlast_seen. So the write on the read path is buying durable convergence, not correctness, which makes it a good candidate to move off the hot path.2. The SQL status pushdown was replaced with an in-memory filter — P3
Reported by cubic on #306.
listAgentsnow selects the whole workspace and filters after mapping:Previously
?status=activepushed the predicate into SQL and loaded only matching rows. Now every roster request materialises the full workspace: 877 rows here, 20,107 fleet-wide across workspaces.This is a genuine regression but a mild one, and it is not trivially reversible — the filter is applied to the derived status, which differs from the stored column precisely for the stale
active/onlinerows this feature is about. cubic's suggestion is sound: push the predicate into SQL when a filter is requested, accounting for the fact that derived and stored differ only in that one direction.Not a blocker for #306
Both are pre-existing to that PR's design rather than introduced by its final commits, and fixing them means reworking the derive-vs-filter logic the presence contract rests on. Holding a security fix on a performance and design refactor is the wrong trade. Filed so it survives the merge rather than riding under a green check.