Skip to content

process SDK messages independently of prompt#396

Draft
SteffenDE wants to merge 2 commits intozed-industries:mainfrom
SteffenDE:sd-loop
Draft

process SDK messages independently of prompt#396
SteffenDE wants to merge 2 commits intozed-industries:mainfrom
SteffenDE:sd-loop

Conversation

@SteffenDE
Copy link
Contributor

This is a proof of concept of separating the SDK message loop from the prompt promise.

Relates to #386.
Relates to #336.

@cla-bot cla-bot bot added the cla-signed label Mar 9, 2026
if (done || !message) {
if (session.cancelled) {
return { stopReason: "cancelled" };
// Query stream ended
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assume that this should not normally happen and if it does, the session is done for good? If not, we may need to recursively call process, otherwise we don't handle subsequent messages.

cachedReadTokens: session.accumulatedUsage.cachedReadTokens,
cachedWriteTokens: session.accumulatedUsage.cachedWriteTokens,
inputTokens: message.usage.input_tokens,
outputTokens: message.usage.output_tokens,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed accumulating, since the prompt return usage should reflect "per-turn data" and we only expect one result message for each prompt. This means that usage for background tasks is not included here.

Comment on lines +873 to +876
return RequestError.internalError(
undefined,
"The Claude Agent process exited unexpectedly. Please start a new session.",
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now the question is if we should recurse the loop in case it's not one of these

};

// Start the background message processing loop
// TODO: depending on the error, we might want to recursively call this
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the other comments.

@SteffenDE
Copy link
Contributor Author

Note: I'll need to ensure /compact and /context still work, similar to #400.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant