Skip to content

fix(tui): release Mini admission before waiting for follow-ups - #59837

Open
Shubham-Padkonde wants to merge 1 commit into
anomalyco:devfrom
Shubham-Padkonde:release-mini-admission
Open

Shubham-Padkonde wants to merge 1 commit into
anomalyco:devfrom
Shubham-Padkonde:release-mini-admission

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

Issue for this PR

Closes #59819.

Type of change

  • Bug fix

What does this PR do?

Mini displays an initial prompt error and returns without signaling admission. A queued follow-up then waits for that signal while the active turn waits for the follow-up, leaving the queue stuck.

Resolve the active admission when the turn finishes, before awaiting dependent admissions. Keep the existing cleanup for errors and closing.

How did you verify your code works?

Both new cases time out before the fix: a handled initial failure followed by either successful or failed follow-up admission. After the fix, all 24 queue tests pass across ten repetitions, including during the full-suite run. The complete Mini suite passes: 416 tests, seven skipped. Package TypeScript checking and formatting pass.

The full Windows TUI run hit the previously observed environment failures plus renderer timeouts and was stopped after stalling. The nine affected renderer cases pass separately on unchanged upstream and alongside all 24 queue tests with the fix. The full suite is not claimed green. No live provider or server was used.

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72">
  <g fill="none" stroke="currentColor"><rect x="8" y="18" width="86" height="36" rx="6"/><path d="M102 36h30m-6-5 6 5-6 5"/><rect x="140" y="18" width="92" height="36" rx="6"/></g>
  <g fill="currentColor" font-family="sans-serif" font-size="11" text-anchor="middle"><text x="51" y="40">Request failed</text><text x="186" y="40">Follow-up runs<animate attributeName="opacity" values="1;.4;1" dur="2s" repeatCount="indefinite"/></text></g>
</svg>

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@opencodejr opencodejr Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The fix looks correct. I ran the reproduction from #59819 against the real runPromptQueue. On the base, the follow-up is never admitted and the queue is still pending after close. With this change, the follow-up is admitted and the queue finishes. The queue and interactive runtime tests in packages/tui/test/mini pass (30 tests), and the type-check results are the same as on the base.

An error still throws before the new admission.resolve() and a close still breaks out before it, so both paths still release the admission in the existing finally. Resolving twice does nothing, and shell turns aren't affected. The two regression tests are short and wait on promises rather than timers.

The public API is unchanged: this only touches Mini's internal prompt queue.

No blocking concerns.

This branch has not been deployed

No deployments
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.

Mini follow-up stays stuck after a prompt fails before admission

1 participant