Skip to content

log executeCode return values - #300

Open
maxwellpeterson wants to merge 5 commits into
mainfrom
mpeterson/execute-code-return-values
Open

log executeCode return values#300
maxwellpeterson wants to merge 5 commits into
mainfrom
mpeterson/execute-code-return-values

Conversation

@maxwellpeterson

@maxwellpeterson maxwellpeterson commented Aug 21, 2026

Copy link
Copy Markdown
Member

Kimi likes to return values when using the executeCode tool instead of logging them. Return values are currently discarded, which causes confusion. We can automatically log them instead.

This is probably a better alternative to #299

Fixes #209


Devin Review
@github-actions github-actions Bot added kernel Changes to the Workshop kernel gatekeeper Changes to a gatekeeper integration labels Aug 21, 2026
@github-actions

Copy link
Copy Markdown

Preview: pr300-mpeterson-exe-a7d98d39

https://pr300-mpeterson-exe-a7d98d39-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

@maxwellpeterson
maxwellpeterson marked this pull request as ready for review August 21, 2026 19:28
Comment thread packages/workshop-backend/src/overseer.ts Outdated
}
await agent(self, env, this.ctx);
let result = await agent(self, env, this.ctx);
if (result !== undefined) console.log(result);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we prefix this with some text explaining it is the return value of the function, to disambiguate from any logs appearing before that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added a "Return value: " prefix in d954141

Comment thread packages/workshop-backend/src/agent.ts Outdated
Comment on lines +283 to +285
* Return or print the schedule ID so it appears in the `executeCode` output. Empty output is not
* evidence that registration failed, and the disabled hook appearing in Connections confirms
* success. Do not retry solely because no ID was printed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This text is weird. Why are we explaining the UI to the agent?

Did we actually have problems with agents getting confused and thinking that schedules had failed to be registered? If so, is the solution perhaps to simply emphasize that if the call doesn't throw, then the schedule has been registered?

Why does an agent care about the schedule ID at all? What can it do with this? Why are we telling it to log or return it?

@ndisidore ndisidore Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The schedule id can be used by the agent to confirm the schedule exists via list()
Non-SOTA models would often get confused because they would call executeCode but to them nothing happened - it would just get a void output.
The would interpret this as a fail and attempt to re-register (often many times)

This is probably less important now that it won't be a void return, but returning the schedule id may still be useful so it knows how to check if it is still enabled.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When you experienced this, were those non-SOTA models specifically trying to return the schedule ID? If so, maybe simply the fact that we're logging now will mean that they no longer get confused, without the need for this text?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another thing we could do is: If the executeCode logs nothing, we could write "(function succeeded with no output)"

devin-ai-integration[bot]

This comment was marked as resolved.

- help kimi and other non-sota models understand that no output doesn't
  mean something when wrong, just that the function ran without logging
  or returning anything
@maxwellpeterson
maxwellpeterson force-pushed the mpeterson/execute-code-return-values branch from 3148589 to ebc13c4 Compare August 31, 2026 17:49
@ask-bonk

ask-bonk Bot commented Aug 31, 2026

Copy link
Copy Markdown
  • P2 packages/workshop-backend/src/overseer.ts:103: Workerd JSON-serializes plain-object console arguments. Valid returns such as { count: 1n } or cyclic objects become {}, producing misleading Return value: {} output. Format return values safely before logging, with explicit BigInt/cycle handling.

github run

@ask-bonk

ask-bonk Bot commented Aug 31, 2026

Copy link
Copy Markdown
  • P2 packages/gatekeeper-scheduler/src/types.d.ts:308: return scheduleId is at module top level, making the documented executeCode example invalid JavaScript. Wrap the snippet in the required exported async function.

github run

- report errors that serialize to "" as uncaught exceptions instead of
  reporting success
@ask-bonk

ask-bonk Bot commented Aug 31, 2026

Copy link
Copy Markdown

No additional actionable findings beyond the existing return-value serialization issue.

github run

@kentonv kentonv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for slow response.

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

Labels

gatekeeper Changes to a gatekeeper integration kernel Changes to the Workshop kernel

3 participants