fix(agent-runtime): stop run after local prompt config errors - #1199
fix(agent-runtime): stop run after local prompt config errors#1199mturac wants to merge 1 commit into
Conversation
When assembleLocalAgentTemplates returns validation errors, callMainPrompt now returns early with an error result instead of continuing into mainPrompt. Previously the orphaned run kept executing (burning credits, running tools) after the prompt-error banner was already shown. Closes CodebuffAI#1155 (sub-bug 1)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d14627eb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sendAction({ | ||
| action: { | ||
| type: 'prompt-response', |
There was a problem hiding this comment.
When local validation fails in CodebuffClient.run and the supported handleEvent callback returns a promise, the preceding prompt-error starts handlePromptResponse, which awaits that callback (sdk/src/run.ts lines 1243-1245), but sendAction does not await the handler. This immediately following prompt-response enters the synchronous response branch and resolves the run first (lines 1256-1287), so await client.run() can return while its error event is still being processed. Emit only one terminal action or serialize these dispatches so asynchronous error handling completes before the run resolves.
Useful? React with 👍 / 👎.
| it('returns early without calling mainPrompt when agent config validation fails', async () => { | ||
| const sentActions: Array<{ type: string }> = [] | ||
| const sendAction = ({ action }: { action: { type: string } }) => { | ||
| sentActions.push(action) | ||
| } | ||
|
|
There was a problem hiding this comment.
This test replaces assembleLocalAgentTemplates through an ES-module spy even though the validation path can be exercised directly by placing an invalid template in mockFileContext.agentTemplates. That unnecessarily couples the regression test to module-binding behavior and conflicts with the repository convention to prefer dependency injection over module mocking; use a real invalid fixture here, or inject the assembler if isolation is required.
AGENTS.md reference: AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
| const errorResult = { | ||
| sessionState: action.sessionState, | ||
| output: { type: 'error' as const, message: errorMessage }, |
There was a problem hiding this comment.
When validation fails before loopAgentSteps runs, the user's current prompt has not yet been inserted into messageHistory, but this result simply returns the unchanged incoming session. Consequently, an SDK caller that fixes the config and passes this failed result as previousRun loses the original request—for example, a follow-up such as “retry” has no request to retry. The SDK's thrown-error path explicitly adds the prompt when the runtime made no progress (sdk/src/run.ts lines 509-527); this early-return path should preserve it equivalently before sending the terminal state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d14627eb4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sendAction({ | ||
| action: { | ||
| type: 'prompt-response', |
There was a problem hiding this comment.
When local validation fails in CodebuffClient.run and the supported handleEvent callback returns a promise, the preceding prompt-error starts handlePromptResponse, which awaits that callback (sdk/src/run.ts lines 1243-1245), but sendAction does not await the handler. This immediately following prompt-response enters the synchronous response branch and resolves the run first (lines 1256-1287), so await client.run() can return while its error event is still being processed. Emit only one terminal action or serialize these dispatches so asynchronous error handling completes before the run resolves.
Useful? React with 👍 / 👎.
| it('returns early without calling mainPrompt when agent config validation fails', async () => { | ||
| const sentActions: Array<{ type: string }> = [] | ||
| const sendAction = ({ action }: { action: { type: string } }) => { | ||
| sentActions.push(action) | ||
| } | ||
|
|
There was a problem hiding this comment.
This test replaces assembleLocalAgentTemplates through an ES-module spy even though the validation path can be exercised directly by placing an invalid template in mockFileContext.agentTemplates. That unnecessarily couples the regression test to module-binding behavior and conflicts with the repository convention to prefer dependency injection over module mocking; use a real invalid fixture here, or inject the assembler if isolation is required.
AGENTS.md reference: AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
| const errorResult = { | ||
| sessionState: action.sessionState, | ||
| output: { type: 'error' as const, message: errorMessage }, |
There was a problem hiding this comment.
When validation fails before loopAgentSteps runs, the user's current prompt has not yet been inserted into messageHistory, but this result simply returns the unchanged incoming session. Consequently, an SDK caller that fixes the config and passes this failed result as previousRun loses the original request—for example, a follow-up such as “retry” has no request to retry. The SDK's thrown-error path explicitly adds the prompt when the runtime made no progress (sdk/src/run.ts lines 509-527); this early-return path should preserve it equivalently before sending the terminal state.
Useful? React with 👍 / 👎.
|
Good catch — continuing into A few things worth checking before this is ported:
Overall the diagnosis and shape of the fix look right — just want the type-correctness and unstable-mergeable status resolved before this gets ported. |
codebuff-team
added
bot:triaged
2 participants
When assembleLocalAgentTemplates returns validation errors, callMainPrompt now returns early with an error result and a prompt-response action, instead of continuing into mainPrompt. Before this the orphaned run kept executing after the prompt-error banner -- burning credits, executing tools, and mutating shared state while the UI already showed the error.
Also adds a regression test verifying early return on validation errors.
Fixes the "prompt-error orphans the run" sub-bug from #1155.
Tested with a new call-main-prompt.test.ts that verifies prompt-error and prompt-response are sent, no response-chunk events fire, and the result is an error. Existing main-prompt.test.ts tests still pass.