Skip to content

feat(tasks): wire agent server event ingest#2111

Open
tatoalo wants to merge 1 commit into
feat/tasks-event-stream-senderfrom
feat/tasks-agent-server-event-ingest
Open

feat(tasks): wire agent server event ingest#2111
tatoalo wants to merge 1 commit into
feat/tasks-event-stream-senderfrom
feat/tasks-agent-server-event-ingest

Conversation

@tatoalo
Copy link
Copy Markdown
Contributor

@tatoalo tatoalo commented May 8, 2026

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

  • read POSTHOG_TASK_RUN_EVENT_INGEST_TOKEN from the agent server env
  • construct the task run event stream sender when the ingest token is present
  • enqueue broadcast and terminal task events into the sender while still serving existing SSE clients
  • drain the sender during agent shutdown and task completion paths

Copy link
Copy Markdown
Contributor Author

tatoalo commented May 8, 2026

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.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@tatoalo tatoalo marked this pull request as ready for review May 8, 2026 15:38
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 8, 2026

Prompt To Fix All With AI
Fix 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

Comment thread packages/agent/src/server/agent-server.test.ts
@tatoalo tatoalo force-pushed the feat/tasks-agent-server-event-ingest branch from 0da1757 to 99cb219 Compare May 8, 2026 15:50
@tatoalo tatoalo force-pushed the feat/tasks-event-stream-sender branch from ba262d3 to 70c5092 Compare May 8, 2026 15:50
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 8, 2026

Prompt To Fix All With AI
Fix 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

@tatoalo tatoalo self-assigned this May 8, 2026
@tatoalo tatoalo requested a review from a team May 8, 2026 16:53
POSTHOG_CODE_REASONING_EFFORT: z
.enum(["low", "medium", "high", "xhigh", "max"])
.optional(),
POSTHOG_TASK_RUN_EVENT_INGEST_TOKEN: z.string().min(1).optional(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why don't we use the existing oauth token here, is there a reason to split this out into it's own auth?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

@tatoalo tatoalo force-pushed the feat/tasks-agent-server-event-ingest branch from 99cb219 to b24ade0 Compare May 18, 2026 15:08
@tatoalo tatoalo force-pushed the feat/tasks-event-stream-sender branch from 70c5092 to 0c05cf5 Compare May 18, 2026 15:08
@tatoalo tatoalo force-pushed the feat/tasks-agent-server-event-ingest branch from b24ade0 to bb9a95a Compare May 18, 2026 15:17
@tatoalo tatoalo force-pushed the feat/tasks-event-stream-sender branch from 0c05cf5 to 768b082 Compare May 18, 2026 15:17
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.

2 participants