fix(edge-worker): post agent activities in one Linear request instead of three - #1448
michaelkd01 wants to merge 3 commits into
Conversation
…SDK getter `AgentActivityPayload.agentActivity` is a getter with no memoisation: each access constructs a fresh `AgentActivityQuery` and issues another round trip. Both activity-posting paths touched it twice per post -- once for truthiness in the `if`, once for the awaited assignment -- while consuming only `.id`. Every agent activity therefore cost three Linear requests instead of one. The promise produced by the `if`-condition access is never awaited, so a failed read-back also leaves an unhandled rejection. `ActivityPoster`'s try/catch does not cover it: the catch block never awaits that promise. `agentActivityId` returns the same id from the mutation response the SDK is already holding, at no request cost. Failure semantics are unchanged: a mutation that did not succeed takes exactly the same branch as before. Adds a request-counting harness that reproduces the SDK getter, because the existing mocks model `agentActivity` as a plain resolved promise property and cannot observe the cost of touching it twice.
|
Self-reported finding on this PR, before it merges. Switching both posting paths from
// Return success payload with agentActivity that can be awaited
// AgentSessionManager expects result.agentActivity to be promise-like
return {
agentActivity: Promise.resolve({ id: activityId }),
success: true,
lastSyncId: Date.now(),
} as AgentActivityPayload;
The fix I'd propose is to prefer the free field and fall back to the property, in both paths: const activityId = result.agentActivityId ?? (await result.agentActivity)?.idOn the Linear SDK path this costs nothing: A commit implementing this, plus a test double shaped like the CLI adapter's payload alongside the existing SDK-shaped double (so the request-count guarantee and this path are both covered), is following shortly on this branch. |
Reading `agentActivityId` alone is correct for the Linear SDK payload but not
for `CLIIssueTrackerService.createAgentActivity`, which returns
{
agentActivity: Promise.resolve({ id: activityId }),
success: true,
lastSyncId: Date.now(),
} as AgentActivityPayload
-- the id is carried only on `agentActivity`, as a plain already-resolved
promise property rather than the SDK's lazy getter, and `agentActivityId` is
absent. On that adapter `postActivityDirect` returned null and
`LinearActivitySink.postActivity` returned {}, where both previously returned
the id. The `as` cast is why the compiler did not catch it.
Both paths now read `result.agentActivityId ?? (await result.agentActivity)?.id`.
The fallback costs nothing on the Linear path: `agentActivityId` is present on
every successful mutation response, `??` short-circuits before the right-hand
side is evaluated, and the getter is never touched -- so the one-request-per-
activity property holds. On the CLI adapter the fallback awaits a promise that
is already settled, which is not a round trip. The getter stays out of the `if`
condition so a rejected read-back is never orphaned, and in LinearActivitySink
it is reached only once `success` holds, keeping failed mutations free.
Adds a CLI-adapter-shaped payload double alongside the SDK-shaped one, and a
guard that pins the id to a falsy-but-present value -- the only case that tells
`??` from `||`, since both short-circuit on a populated id. Failure branches are
unchanged.
Summary
Every agent activity Cyrus posts to Linear costs three API requests instead of one. Two of the three are read-back queries for an activity that was just created, and only its
idis ever used — an id the mutation response already carries.This PR removes both extra requests. No behaviour change is intended: it is a request-count fix.
Mechanism
AgentActivityPayload.agentActivityin@linear/sdkis an unmemoized getter. From the generated SDK (v64, the version this repo resolves):Each access of
agentActivityconstructs a freshAgentActivityQueryand issues another round trip. It caches nothing.agentActivityIdreads an id already present in the mutation response, at zero request cost.Both activity-posting paths touched the getter twice per post — once for truthiness in the
if, once for the awaited assignment — and consumed only.id:So: 1 mutation + 2 read-back queries = 3 requests per activity.
The promise from the first access is never awaited, so when a read-back fails its rejection is unhandled.
ActivityPoster'stry/catchdoes not cover this — a catch block cannot handle the rejection of a promise it never awaited. Under Node's default--unhandled-rejections=throwthis is a process-level failure, and it is most likely to fire exactly when the budget is already exhausted.The signature that identified it
In a single 17-minute burst we observed 793 awaited read-back queries against 796 unawaited ones. The near-equality is the tell: a read-back genuinely needed once per post would not be almost exactly doubled. Of the 796 unawaited queries, 761 landed within one second of their awaited twin, median separation 0.021s — two accesses of the same getter in the same synchronous stretch of one
postActivitycall, not two independent operations that happened to coincide.Operator-side impact
Tripling every write divides the usable write budget by three. Against Linear's 5,000 requests/hour, an integration that should sustain 5,000 activities/hour is capped at an effective ~1,667 activities/hour.
On 2026-08-31 this produced 4,161 rate-limit occurrences in 17 minutes. The pattern has recurred episodically since 2026-05-15, surfacing whenever activity volume rises — which is to say, whenever agents are busiest and their timeline updates matter most. Once the budget is spent, the read-backs are also the first requests to fail, which is what turns a throughput problem into the unhandled rejection described above.
The fix
Read the id from
result.agentActivityIdand keep the getter out of the condition entirely, so it is never evaluated:Failure semantics are preserved. A mutation that did not succeed takes exactly the same branch it does today, returns exactly the same value, and issues exactly the same one request;
agentActivityIdis a plain optional-chained read that cannot throw and cannot fetch. Tests covering both failure branches are included and pass unchanged before and after.Measurement: 3 → 1, and how it was counted
The count is taken at the transport, not at the caller. A test double reproduces
AgentActivityPayloadwithagentActivityas a real getter that appends one entry per access, andagentActivityIdas a plain accessor that appends nothing; thecreateAgentActivitymutation appends one entry when invoked. The request count for a call is thenlog.operations.length. This matters because the existing mocks inLinearActivitySink.test.tsmodelagentActivityas a plain already-resolved promise property — touching a property twice is free, so those mocks cannot observe this defect at all. Reproducing the getter is what makes the cost visible.Before, one
postActivitycall:and a burst of three posts logged 9 operations, with
Error: Ratelimit exceededarriving at theunhandledRejectionhandler.After: 1 operation per post, 3 across a burst of three, and nothing unhandled. Both posting paths are covered.
Changes
packages/edge-worker/src/sinks/LinearActivitySink.ts—postActivityusesagentActivityId; getter removed from the condition.packages/edge-worker/src/ActivityPoster.ts— same fix inpostActivityDirect, the second posting path, found by searching for the construct rather than the file. Happy to split this out if you would rather review it separately.packages/edge-worker/test/agent-activity-payload-double.ts— the payload double and request counter, shared by both suites (following the existingtest/*-utils.tshelper convention).packages/edge-worker/test/LinearActivitySink.request-count.test.ts,packages/edge-worker/test/ActivityPoster.request-count.test.ts— new.packages/edge-worker/test/LinearActivitySink.test.ts— existing mocks now also supplyagentActivityIdalongsideagentActivity, so they match the shape of the payload they stand in for. Their assertions are unchanged.CHANGELOG.md— entry under## [Unreleased]/### Fixed.Verification
Tracked internally as SOC-403.