Refuse to send into a busy thread unless the caller injects - #9
Conversation
threads send gains --if-busy reject|inject, as proposed in #2 by @jarekbird. reject is the default: send refuses with THREAD_BUSY and dispatches nothing while a turn runs or an earlier message waits for its turn. inject sends into the running turn, as send did before.
Mira PR WalkthroughThis PR makes graph LR
cli["src/cli.ts"] --> controls["src/threadControls.ts"]
cli --> service["src/service.ts"]
service --> controls
threadsTests["src/threads.test.ts"] --> service
cliTests["tests/cli.test.mjs"] --> cli
Confidence: 4/5 ◉◉◉◉○ Likely safe to merge
8 files reviewed
|
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: d099cd07a8
ℹ️ 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".
| const busy = busyState(thread); | ||
| if (busy && (options.ifBusy ?? "reject") === "reject") { |
There was a problem hiding this comment.
When the thread has queued messages but no running turn, busy is non-null with turnRunning: false, yet --if-busy inject bypasses this guard and dispatches another turn-start command. There is no running turn to inject into in that state, so this silently adds more queued work despite the option being documented as steering a running turn; queued-only threads should remain rejected until the existing message starts or finishes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Keeping this as is. inject means "send anyway", as #2 defined it ("sends immediately even when busy"). The queued-only state also occurs right after every send: in the live check, a second send hit "has a message waiting for its turn" before the first turn started. Rejecting inject there would make it fail unpredictably at the start of every turn. In that state T3 accepts the message, and the provider folds it into the starting turn or queues it, which --wait follows. reject stays the default, so nothing is added unless the caller asks for it.
1 participant
Adds the busy-thread guard that @jarekbird proposed in #2 to the merged
threads send.Changes
--if-busy reject|inject:rejectis the default. It refuses withTHREAD_BUSY(exit code 4) and dispatches nothing while a turn runs or an earlier message still waits for its turn.error.detailssays which, with the session and latest-turn state.inject: sends into the running turn, assenddid before. The provider then folds the message into that turn or queues it, and--waitfollows either way.sendwith new settings is still refused mid-turn, even withinject, because new settings can't reach a running turn.t3thread, anduse-t3code-cliupdated. The busy check is a snapshot, not a lock, as Add threads send with configurable busy-thread injection #2 also noted.Testing
pnpm checkpasses with 166 tests. Checked live against T3 0.0.45-nightly. A second send during a just-started turn returnedTHREAD_BUSYwith "has a message waiting for its turn", and then "is running a turn" once the turn ran. Neither message reached the thread.