fix: bound agent builder retries and improve chat scrolling - #89
Conversation
There was a problem hiding this comment.
6 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/agent/agent/subagents/agent_builder/agent.ts">
<violation number="1" location="apps/agent/agent/subagents/agent_builder/agent.ts:13">
P3: The builder output contract is now inconsistent with the repository documentation: `needs_input` is rejected by this schema even though the documented contract still tells maintainers and consumers to expect it. Updating `docs/agent.md` and any contract consumers to describe the durable `ask_question` flow would prevent incorrect integrations and future regressions.</violation>
</file>
<file name="apps/agent/agent/subagents/agent_builder/lib/execution-state.ts">
<violation number="1" location="apps/agent/agent/subagents/agent_builder/lib/execution-state.ts:29">
P2: The save lock is lost at the next turn boundary, allowing a later continuation to call `write_agent_file` or `save_agent_draft` again. Keep the saved flag across continuation turns, or reset it only when the runtime explicitly starts a new draft rather than whenever `turnId` changes.</violation>
<violation number="2" location="apps/agent/agent/subagents/agent_builder/lib/execution-state.ts:40">
P2: The save-lock guardrail is inconsistent when a single `actions.requested` batch contains both the save and a following tool. `current.saved` is read once before the loop and never updated while the batch is processed, so: (1) a batch with `save_agent_draft` then `write_agent_file` lets the file edit pass even though the draft is being saved in that same batch, bypassing the lock, and (2) a batch with `save_agent_draft` then `final_output` is falsely rejected with 'Save the draft before returning draft_ready.' even though it is a legitimate save-then-finish sequence. The instructions steer the model toward one tool call per message, so this is an edge case, but the guard should make per-batch ordering deterministic. Consider tracking a local `saved` flag that flips when a `save_agent_draft` action is recorded in the loop so later actions in the same batch are validated consistently.</violation>
</file>
<file name="apps/agent/agent/subagents/agent_builder/lib/draft-input.ts">
<violation number="1" location="apps/agent/agent/subagents/agent_builder/lib/draft-input.ts:12">
P3: Whitespace-only trigger names or summaries can be accepted and saved as blank labels or README content. Trimming before validation, as already done for the top-level name, description, and instructions, keeps the generated agent metadata usable.</violation>
</file>
<file name="apps/agent/agent/hooks/builder-delegation.ts">
<violation number="1" location="apps/agent/agent/hooks/builder-delegation.ts:18">
P1: A second `agent_builder` call can pass in the same creation turn after the first subagent returns: nested builder tool requests reach this hook, and the call to `recordBuilderDelegation` resets the parent state on their different `turnId` even though they contain no builder delegation. Ignoring action-request batches that contain no `agent_builder` action preserves the parent guard.</violation>
</file>
<file name="apps/agent/agent/subagents/agent_builder/instructions.md">
<violation number="1" location="apps/agent/agent/subagents/agent_builder/instructions.md:34">
P2: The new clarification flow tells the builder specialist to call `ask_question` directly when a build is blocked, and the parent task now assumes the specialist handles clarification that way. `ask_question` is not among the agent_builder subagent's registered tools (the tools/ directory only exposes bash, glob, grep, inspect_context, read_file, todo, web_fetch, web_search, write_agent_file, write_file, and save_agent_draft), and the previous `needs_input` path that let the parent issue the question was removed. If `ask_question` isn't injected into the specialist's tool set, a blocked build can no longer ask the user—it would either fail to find the tool or stall. Please confirm the specialist actually has access to `ask_question` (and its options/freeform policy) before merge, or wire the clarification back through the parent as before.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| return; | ||
| } | ||
|
|
||
| const next = recordBuilderDelegation( |
There was a problem hiding this comment.
P1: A second agent_builder call can pass in the same creation turn after the first subagent returns: nested builder tool requests reach this hook, and the call to recordBuilderDelegation resets the parent state on their different turnId even though they contain no builder delegation. Ignoring action-request batches that contain no agent_builder action preserves the parent guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/hooks/builder-delegation.ts, line 18:
<comment>A second `agent_builder` call can pass in the same creation turn after the first subagent returns: nested builder tool requests reach this hook, and the call to `recordBuilderDelegation` resets the parent state on their different `turnId` even though they contain no builder delegation. Ignoring action-request batches that contain no `agent_builder` action preserves the parent guard.</comment>
<file context>
@@ -0,0 +1,26 @@
+ return;
+ }
+
+ const next = recordBuilderDelegation(
+ builderDelegationState.get(),
+ event.data.turnId,
</file context>
| const current = | ||
| state.turnId === turnId | ||
| ? state | ||
| : { turnId, callIds: [], saveCallIds: [], saved: false }; |
There was a problem hiding this comment.
P2: The save lock is lost at the next turn boundary, allowing a later continuation to call write_agent_file or save_agent_draft again. Keep the saved flag across continuation turns, or reset it only when the runtime explicitly starts a new draft rather than whenever turnId changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/subagents/agent_builder/lib/execution-state.ts, line 29:
<comment>The save lock is lost at the next turn boundary, allowing a later continuation to call `write_agent_file` or `save_agent_draft` again. Keep the saved flag across continuation turns, or reset it only when the runtime explicitly starts a new draft rather than whenever `turnId` changes.</comment>
<file context>
@@ -0,0 +1,73 @@
+ const current =
+ state.turnId === turnId
+ ? state
+ : { turnId, callIds: [], saveCallIds: [], saved: false };
+ const callIds = new Set(current.callIds);
+ const saveCallIds = new Set(current.saveCallIds);
</file context>
| be safely represented in the reviewable draft. For a schedule, calculate a | ||
| future `nextRunAt` from the supplied current time and provide its recurrence in | ||
| minutes. | ||
| not call `save_agent_draft`. Call `ask_question` directly with one focused |
There was a problem hiding this comment.
P2: The new clarification flow tells the builder specialist to call ask_question directly when a build is blocked, and the parent task now assumes the specialist handles clarification that way. ask_question is not among the agent_builder subagent's registered tools (the tools/ directory only exposes bash, glob, grep, inspect_context, read_file, todo, web_fetch, web_search, write_agent_file, write_file, and save_agent_draft), and the previous needs_input path that let the parent issue the question was removed. If ask_question isn't injected into the specialist's tool set, a blocked build can no longer ask the user—it would either fail to find the tool or stall. Please confirm the specialist actually has access to ask_question (and its options/freeform policy) before merge, or wire the clarification back through the parent as before.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/subagents/agent_builder/instructions.md, line 34:
<comment>The new clarification flow tells the builder specialist to call `ask_question` directly when a build is blocked, and the parent task now assumes the specialist handles clarification that way. `ask_question` is not among the agent_builder subagent's registered tools (the tools/ directory only exposes bash, glob, grep, inspect_context, read_file, todo, web_fetch, web_search, write_agent_file, write_file, and save_agent_draft), and the previous `needs_input` path that let the parent issue the question was removed. If `ask_question` isn't injected into the specialist's tool set, a blocked build can no longer ask the user—it would either fail to find the tool or stall. Please confirm the specialist actually has access to `ask_question` (and its options/freeform policy) before merge, or wire the clarification back through the parent as before.</comment>
<file context>
@@ -31,23 +31,29 @@ not report.
-be safely represented in the reviewable draft. For a schedule, calculate a
-future `nextRunAt` from the supplied current time and provide its recurrence in
-minutes.
+not call `save_agent_draft`. Call `ask_question` directly with one focused
+question. Include two to four mutually exclusive options when they clarify a
+real choice, and allow freeform input when a custom answer is valid. Ask only
</file context>
| "The draft is already saved. Return the saved draft now without calling another tool.", | ||
| ); | ||
| } | ||
| if (!current.saved && action.toolName === "final_output") { |
There was a problem hiding this comment.
P2: The save-lock guardrail is inconsistent when a single actions.requested batch contains both the save and a following tool. current.saved is read once before the loop and never updated while the batch is processed, so: (1) a batch with save_agent_draft then write_agent_file lets the file edit pass even though the draft is being saved in that same batch, bypassing the lock, and (2) a batch with save_agent_draft then final_output is falsely rejected with 'Save the draft before returning draft_ready.' even though it is a legitimate save-then-finish sequence. The instructions steer the model toward one tool call per message, so this is an edge case, but the guard should make per-batch ordering deterministic. Consider tracking a local saved flag that flips when a save_agent_draft action is recorded in the loop so later actions in the same batch are validated consistently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/subagents/agent_builder/lib/execution-state.ts, line 40:
<comment>The save-lock guardrail is inconsistent when a single `actions.requested` batch contains both the save and a following tool. `current.saved` is read once before the loop and never updated while the batch is processed, so: (1) a batch with `save_agent_draft` then `write_agent_file` lets the file edit pass even though the draft is being saved in that same batch, bypassing the lock, and (2) a batch with `save_agent_draft` then `final_output` is falsely rejected with 'Save the draft before returning draft_ready.' even though it is a legitimate save-then-finish sequence. The instructions steer the model toward one tool call per message, so this is an edge case, but the guard should make per-batch ordering deterministic. Consider tracking a local `saved` flag that flips when a `save_agent_draft` action is recorded in the loop so later actions in the same batch are validated consistently.</comment>
<file context>
@@ -0,0 +1,73 @@
+ "The draft is already saved. Return the saved draft now without calling another tool.",
+ );
+ }
+ if (!current.saved && action.toolName === "final_output") {
+ throw new Error("Save the draft before returning draft_ready.");
+ }
</file context>
| versionId: z.string().min(1), | ||
| }), | ||
| ]), | ||
| outputSchema: z.object({ |
There was a problem hiding this comment.
P3: The builder output contract is now inconsistent with the repository documentation: needs_input is rejected by this schema even though the documented contract still tells maintainers and consumers to expect it. Updating docs/agent.md and any contract consumers to describe the durable ask_question flow would prevent incorrect integrations and future regressions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/subagents/agent_builder/agent.ts, line 13:
<comment>The builder output contract is now inconsistent with the repository documentation: `needs_input` is rejected by this schema even though the documented contract still tells maintainers and consumers to expect it. Updating `docs/agent.md` and any contract consumers to describe the durable `ask_question` flow would prevent incorrect integrations and future regressions.</comment>
<file context>
@@ -10,30 +10,15 @@ export default defineAgent({
- versionId: z.string().min(1),
- }),
- ]),
+ outputSchema: z.object({
+ status: z.literal("draft_ready"),
+ summary: z.string().min(1).max(1000),
</file context>
|
|
||
| const trigger = z.object({ | ||
| type: z.enum(["MANUAL", "SCHEDULE"]), | ||
| name: z.string().min(1).max(120), |
There was a problem hiding this comment.
P3: Whitespace-only trigger names or summaries can be accepted and saved as blank labels or README content. Trimming before validation, as already done for the top-level name, description, and instructions, keeps the generated agent metadata usable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/subagents/agent_builder/lib/draft-input.ts, line 12:
<comment>Whitespace-only trigger names or summaries can be accepted and saved as blank labels or README content. Trimming before validation, as already done for the top-level name, description, and instructions, keeps the generated agent metadata usable.</comment>
<file context>
@@ -0,0 +1,81 @@
+
+const trigger = z.object({
+ type: z.enum(["MANUAL", "SCHEDULE"]),
+ name: z.string().min(1).max(120),
+ summary: z.string().min(1).max(240),
+ nextRunAt: z.string().nullish(),
</file context>
Summary
Improve private agent chat scrolling and prevent agent-builder failures from turning into runaway retries.
Problem
The builder chat used ordinary overflow scrolling, so streamed responses did not reliably follow the reader's intent. Separately, a model structured-output miss could cause the parent to delegate again, guess invalid resource contracts, continue editing after a successful save, and eventually exhaust the session token budget.
Solution
Adopt the shared message-scroller behavior with stable turn anchors, saved-thread positioning, and a jump-to-latest control. Move clarification into the builder specialist's durable Eve question flow, enforce one delegation per creation turn, canonicalize integration inputs, lock the draft after save, and cap tool calls, save attempts, and session tokens.