Skip to content

Comments

feat: taskrunv2#178

Merged
jonathanlab merged 1 commit intomainfrom
12-01-feat_taskrunv2
Dec 2, 2025
Merged

feat: taskrunv2#178
jonathanlab merged 1 commit intomainfrom
12-01-feat_taskrunv2

Conversation

@jonathanlab
Copy link
Contributor

@jonathanlab jonathanlab commented Dec 1, 2025

Hooks up the claude adapter to agent.ts. Remove usage of EventAgent. Update example client to show usage.

@jonathanlab jonathanlab marked this pull request as ready for review December 1, 2025 17:50
@jonathanlab jonathanlab requested a review from a team as a code owner December 1, 2025 17:50
Copy link
Contributor Author

jonathanlab commented Dec 1, 2025

@wiz-7ad640923b
Copy link

wiz-7ad640923b bot commented Dec 1, 2025

Wiz Scan Summary

Scanner Findings
Vulnerability Finding Vulnerabilities 2 Medium 1 Low
Data Finding Sensitive Data -
Secret Finding Secrets -
IaC Misconfiguration IaC Misconfigurations -
SAST Finding SAST Findings -
Total 2 Medium 1 Low

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

Copy link
Contributor

@joshsny joshsny left a comment

Choose a reason for hiding this comment

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

Looks good generally, left a couple comments, approving not to block you

const taskSlug = (task as any).slug || task.id;

// Create a session for this task run
const sessionId = uuidv7();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the task run id as the session id here? Don't really see a need to seperate them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fully agree. I'm currently working on getting that in.

* Add a custom notification following ACP extensibility model.
* Method names should start with underscore (e.g., `_posthog/phase_start`).
*/
addNotification(
Copy link
Contributor

Choose a reason for hiding this comment

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

These will replace AgentEvent's, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct! In the next PR i delete all AgentEvent logic

* Update the task run associated with a session.
*/
async updateTaskRun(
sessionId: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here with sessions, it's confusing to me why we want to decouple these concepts, they seem like the same thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are, and are tied together int he next PR

posthogApiUrl: string;
posthogApiKey: string;
posthogProjectId: number;
// PostHog API configuration (optional - enables PostHog integration when provided)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we marking these as optional - we only want this to be used via the gateway and with a posthog integration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true, will fix

Copy link
Contributor Author

jonathanlab commented Dec 2, 2025

Merge activity

  • Dec 2, 4:14 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Dec 2, 4:20 PM UTC: Graphite rebased this pull request as part of a merge.
  • Dec 2, 4:20 PM UTC: @jonathanlab merged this pull request with Graphite.

@jonathanlab jonathanlab changed the base branch from 12-01-feat_add_session_loading to graphite-base/178 December 2, 2025 16:15
@jonathanlab jonathanlab changed the base branch from graphite-base/178 to main December 2, 2025 16:18
@jonathanlab jonathanlab merged commit 7717b56 into main Dec 2, 2025
8 of 11 checks passed
@jonathanlab jonathanlab deleted the 12-01-feat_taskrunv2 branch December 2, 2025 16:20
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