feat(tasks): wire agent server event ingest#2111
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/agent/src/server/agent-server.test.ts:272-449
**Repeated `AgentServer` setup violates OnceAndOnlyOnce**
The three `signalTaskComplete` tests each repeat an identical 20+ line `new AgentServer({...}) as unknown as { eventStreamSender, posthogAPI, signalTaskComplete }` cast. Extracting a shared helper (e.g. `createSignalTaskCompleteTestServer()`) would remove the duplication and make future type-shape changes a single-line fix instead of three.
Reviews (1): Last reviewed commit: "feat(tasks): wire agent server event ing..." | Re-trigger Greptile |
0da1757 to
99cb219
Compare
ba262d3 to
70c5092
Compare
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/agent/src/server/agent-server.ts:1808-1813
`TASK_COMPLETE` is included in the union type but `enqueueTaskTerminalEvent` is only ever called with `ERROR` in this PR (and nowhere else in the codebase). Listing a type option that is never exercised is a superfluous part — if it becomes needed later it can be added then.
```suggestion
private enqueueTaskTerminalEvent(
method: typeof POSTHOG_NOTIFICATIONS.ERROR,
params: Record<string, unknown>,
): void {
```
Reviews (2): Last reviewed commit: "feat(tasks): wire agent server event ing..." | Re-trigger Greptile |
| POSTHOG_CODE_REASONING_EFFORT: z | ||
| .enum(["low", "medium", "high", "xhigh", "max"]) | ||
| .optional(), | ||
| POSTHOG_TASK_RUN_EVENT_INGEST_TOKEN: z.string().min(1).optional(), |
There was a problem hiding this comment.
why don't we use the existing oauth token here, is there a reason to split this out into it's own auth?
There was a problem hiding this comment.
Mainly to have cleaner mindset against the token and what it does/needs to do.
I added ^ just for event-ingestion since it has a narrower job for exactly one task run, if we were to re-use the existing oauth token, we would need to make the ingest endpoint added in the BE stack also accept a broader user-scope token, we only authorize event ingestion for the single run, if you feel strongly about it we could reuse the existing one but we would then have duplicate oauth validation and some extra checks bits, no biggie though
99cb219 to
b24ade0
Compare
70c5092 to
0c05cf5
Compare
b24ade0 to
bb9a95a
Compare
0c05cf5 to
768b082
Compare

Problem
once the backend provides a sandbox event ingest token, the agent server needs to opt into the new sender while preserving the existing local SSE behavior and task completion signaling
Changes
POSTHOG_TASK_RUN_EVENT_INGEST_TOKENfrom the agent server env