Add existing-thread commands, the t3thread skill, and thread waits - #7
Conversation
- threads read returns full message text without the inspect preview limit - --last-turn filters to messages assigned to the latest turn only - refactors T3ThreadApi.inspect to share readDetail with the new read method
Merge mhmdkhalaf's feat/thread-messaging branch onto current main. Conflict resolution: keep main's flush-then-exit tail and the branch's JSON usage envelopes for commander errors. Because the CLI now exits explicitly, the in-process CLI parsing tests move to tests/cli.test.mjs and run the built CLI as a child process, sharing a build helper with tests/http.test.mjs. The api tests gain the new runtime capabilities field.
Build on the merged thread messaging commands so an agent can be pointed at an existing T3 Code thread with `$t3thread <thread-id> <instruction>`. - threads read gains --detail answers|messages|full, --turns, --first-turn and --max-chars. T3 stores user messages without a turn id, so a transcript module assigns each one to the turn that handled it. Claude folds a message sent mid-turn into the running turn; Codex queues it as a new turn. Reads fetch the whole thread and window it locally, so turn numbers and the original request stay correct. - threads send --wait and threads wait poll until the awaited turn finishes, fails to start, or needs a person, then return that turn. A finished state must hold on two polls, and waits issue a T3 session that outlives the timeout. A timeout exits with code 6 and reports that the message was sent. - threads inspect shows the workspace, branch, turn count, context use, and pending approvals or questions, read from the whole thread. - Fix: the thread detail endpoint omits T3's pending flags, so pending requests are derived from activities. The settle guard relied on the missing flags too. - Thread commands look up projects in the local projection first. - New skills/t3thread; README and use-t3code-cli skill updates.
Mira PR WalkthroughThis PR adds commands for listing, inspecting, reading, messaging, and managing existing T3 Code threads, plus a t3thread skill for working with them. Transcript controls support selective reads, while send-and-wait and standalone waits return the relevant turn and stop when user input or approval is required. It also reads pending requests from the activity log and updates CLI tests to execute the built CLI. graph LR
skill["skills/t3thread/SKILL.md"] --> cli["src/cli.ts"]
cli --> service["src/service.ts"]
service --> api["src/threadApi.ts"]
service --> transcript["src/transcript.ts"]
Confidence: 3/5 ◉◉◉○○ Moderate confidence
Key files to review:
19 files reviewed · 1 comment (
|
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. |
![clark-review[bot]](https://avatars.githubusercontent.com/u/8057288?s=60&v=4){kind=small}
There was a problem hiding this comment.
Mira Review Summary
Message-specific waits in src/threadApi.ts can time out after their reply completes if a subsequent turn is running. They should finish when their own turn completes.
Key Issues
| Issue | Location | |
|---|---|---|
| 🔴 | Message-specific waits can time out after their reply completes because a subsequent turn is running. | src/threadApi.ts:231 |
| } | ||
| } | ||
| const session = thread.session?.status; | ||
| if (thread.latestTurn?.state === "running" || session === "starting" || session === "running") return null; |
There was a problem hiding this comment.
Bug
Finish message-specific waits when their own turn completes
When messageId is provided, turn already identifies the turn that handled that message. This check instead uses the latest turn and session for the entire thread. If the awaited turn has completed and a subsequent queued turn is running, send --wait continues waiting for that subsequent work and can time out despite its reply being complete. Check the selected turn's terminal state for message-specific waits; retain the thread-wide running checks for waits without a message ID.
Prompt for AI Agents
Update observeTurn in src/threadApi.ts around lines 211-234 so message-specific waits return once the selected message's owning turn is terminal, even when a later turn or the thread session is running. Preserve thread-wide waiting behavior when messageId is undefined and pending-request detection. Add a regression test in src/threadApi.test.ts with a completed checkpoint for the sent message's turn and a later running latestTurn, and assert that the wait returns the completed owning turn.
Not useful? Reply
@clark-review rejectto dismiss this suggestion.
There was a problem hiding this comment.
Fixed in 90bae91. A message-specific wait now treats the thread as busy only when the awaited turn is the latest one, so it returns once the message's own turn has ended while a later turn runs. Regression test: "returns a message's own turn while a later turn runs".
![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: 952a555b70
ℹ️ 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".
| if (limit === undefined || value.length <= limit) return { text: value, truncated: false }; | ||
| const marker = `\n… [${value.length - limit} characters omitted] …\n`; | ||
| const head = Math.ceil(limit * 0.6); | ||
| return { text: `${value.slice(0, head)}${marker}${value.slice(value.length - (limit - head))}`, truncated: true }; |
There was a problem hiding this comment.
When --max-chars truncates an entry, this retains limit source characters and then adds the omission marker, so the returned text always exceeds the requested bound; for small limits the marker can be many times larger than the limit, and the excess accumulates across every message and tool entry. Include the marker within the character budget so callers can reliably bound transcript size.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90bae91. The omission marker now counts toward --max-chars, so clipped text is exactly the limit, and a limit smaller than the marker falls back to a plain cut. The clip test asserts the length.
| let candidate: (TurnObservation & { snapshotSequence: number }) | null = null; | ||
| let last: { snapshotSequence: number; thread: T3Thread } | null = null; | ||
| for (;;) { | ||
| const read = await this.read(threadId).catch((error: unknown) => { |
There was a problem hiding this comment.
For a long-lived thread, each iteration performs the unwindowed read() request, including the complete message history, every two seconds for up to the default 600-second timeout. That can cause hundreds of large buffered responses and make threads wait or send --wait time out from transfer overhead rather than the turn itself; poll with the bounded inspection view and fetch the complete transcript once after reaching a terminal outcome.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90bae91. Polls now read the bounded inspection window (turnLimit=10), and the whole thread is read once, when the outcome is confirmed. If that full read shows a different outcome, polling continues. Test: "polls a bounded window and reads the whole thread once at the end".
| } else if (options.cwd) { | ||
| workspace = await resolveWorkspace(options.cwd, options.workspaceMode ?? config.workspaceMode); | ||
| project = projectForWorkspace(projects, workspace); |
There was a problem hiding this comment.
--cwd is an empty argument
When automation passes an empty value such as --cwd "$WORKSPACE" with an unset variable, Commander supplies "", but this truthiness check skips workspace resolution and silently returns threads from every project. Other workspace commands treat an empty path as the current directory, so this can unexpectedly broaden thread discovery and lead callers to select the wrong target; distinguish an omitted option from an explicitly supplied empty value and either resolve it consistently or reject it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90bae91. An explicitly empty --cwd now filters by the current folder, as the other workspace commands do, instead of listing every project. Test: "filters by the current folder when --cwd is empty".
- A message-specific wait returns once the message's own turn has ended, even while a later turn runs. - Waits poll a bounded window of recent turns and read the whole thread once, when the outcome is clear, instead of downloading it every poll. - --max-chars counts the omission marker, so clipped text stays within the limit. - threads list --cwd "" filters by the current folder, as other commands do, instead of listing threads from every project.
|
@codex review |
Combine the wait loop's bounded polling and single full read from #7 with the queue grace window and blocking-request rules of this branch.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90bae91c32
ℹ️ 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".
| (thread.session?.status === "starting" || | ||
| thread.session?.status === "running" || | ||
| hasPendingApprovals || | ||
| hasPendingUserInput) |
There was a problem hiding this comment.
When a Codex message is queued between turns, the projection can have a completed latest turn and a ready session while the unassigned user message is still waiting to start its own turn—the wait logic explicitly handles this state as pending. This predicate nevertheless allows threads settle, so a thread can be marked settled while submitted work is still queued. Include pending transcript messages (and the independent latestTurn.state === "running" signal) in the active-work check before dispatching settlement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 390cc2c. threads settle now also refuses while the latest turn runs or a user message waits for its turn, leaving out messages the provider already refused to start. Test: "refuses to settle a thread while a message waits for its turn".
|
|
||
| const ownerOf = (message: T3Message): TurnBuilder | undefined => { | ||
| if (message.turnId !== null) return byId.get(message.turnId); | ||
| const next = turns.find((turn) => turn.startedAt >= message.createdAt); |
There was a problem hiding this comment.
When multiple thread.turn.start messages are queued during the same Codex turn and T3 subsequently starts one turn for each message, this independent find assigns every queued prompt to the first later turn. As a result, send --wait for the second message can treat the first queued turn as its owner and return that turn's answer before the second message is handled. Consume later turns as queued prompts are assigned, rather than selecting the same first eligible turn for each prompt.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This grouping is replaced in the stacked #8, which merges right after this PR. #8 drops the provider-based rule and fixes this case in d44049a: messages queued during a turn claim the turns that start back to back after it, one each, and a message whose turn has not started yet stays pending. Test there: "gives each message queued during a turn its own queued turn".
| function turnState(turn: TurnBuilder, thread: T3Thread, checkpoint: Checkpoint | undefined): TurnState { | ||
| if (turn.turnId === null) return "pending"; | ||
| if (thread.latestTurn?.turnId === turn.turnId) return thread.latestTurn.state; | ||
| return checkpoint ? "completed" : null; |
There was a problem hiding this comment.
When the awaited turn is interrupted or errors and a queued later turn becomes latestTurn before the wait confirms the result, this classifies the older checkpointed turn as completed unconditionally. observeTurn then returns a successful completed outcome for the target instead of interrupted or error, so send --wait can report success for a failed turn. Preserve the per-turn terminal state from checkpoint or activity metadata instead of treating every older checkpoint as successful.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 390cc2c. The wait remembers each turn's state while it was the latest turn, and uses that state once a later turn replaces it, so an interrupted or failed turn is no longer reported as completed. Test: "keeps an interrupted turn interrupted after a later turn becomes the latest".
- threads settle refuses while the latest turn runs or a message waits for its turn, even when the session looks ready. - A wait remembers each turn's state while it was the latest turn, so an interrupted or failed turn is not reported as completed once a later turn starts.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 390cc2c417
ℹ️ 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".
| return { | ||
| ...threadSummary(thread), | ||
| messageCount: transcript.messages.length, | ||
| ...transcript, | ||
| }; |
There was a problem hiding this comment.
For JSON callers using the existing threads read --last-turn interface, the previous envelope included data.thread.messageFilter with the selected turn ID, but this replacement removes that field entirely. Keep the legacy field when --last-turn is requested while adding the new transcript metadata so existing automation does not break.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changing this. threads read and its messageFilter field come from #1, which was never merged or released. main and the published 0.1.3 package have no threads read command, so no released envelope loses this field. The AGENTS.md compatibility rule protects released command output. This PR is the first release of the command, and the documented JSON uses data.thread.view and data.thread.turns.
| if (waitsForPerson(thread)) { | ||
| return { outcome: "needs-attention", turnIndex: latest?.index ?? null }; |
There was a problem hiding this comment.
When send --wait targets a specific message and a later turn reaches an approval or user-input request before the next poll, this global check runs before resolving which turn owns that message. The wait therefore returns needs-attention with the later turn's transcript instead of the completed result for the requested message; only apply the attention state when the awaited message is still pending or belongs to the turn that raised the request.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 18f8d20. With a message id, the wait first finds the message's own turn. A pending request only means needs-attention while that turn is still the latest one or the message has no turn yet. Test: "returns a finished message's turn even when a later turn waits for an approval".
A request raised in a later turn no longer turns send --wait into needs-attention when the awaited message's own turn has already ended.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18f8d20e5f
ℹ️ 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".
| function turnState(turn: TurnBuilder, thread: T3Thread, checkpoint: Checkpoint | undefined): TurnState { | ||
| if (turn.turnId === null) return "pending"; | ||
| if (thread.latestTurn?.turnId === turn.turnId) return thread.latestTurn.state; | ||
| return checkpoint ? "completed" : null; |
There was a problem hiding this comment.
When an interrupted or failed turn has a checkpoint and a newer turn becomes latest, this unconditionally exposes the historical turn as completed, so threads read reports a false successful state. Fresh evidence after the prior wait fix is that the current read path still calls this unchanged function without the waiter's in-memory knownStates; derive the terminal state from per-turn activity metadata or leave it unknown instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 07114f2. T3 reports only the latest turn's state, so earlier turns now have a null state, and the heading leaves it out instead of claiming completed. The README says so. Waits keep the state they saw while a turn was the latest.
| const knownStates = new Map<string, T3LatestTurn["state"]>(); | ||
| for (;;) { | ||
| // Polls read a bounded window of recent turns; the whole thread is read once the outcome is clear. | ||
| const read = await this.inspect(threadId).catch((error: unknown) => { |
There was a problem hiding this comment.
When the thread-detail endpoint stalls, this await can consume the API client's full 30-second request timeout even if the caller passed --timeout 1; after it returns, the loop can even complete successfully before checking the expired deadline. The unwindowed confirmation read has the same issue, so both threads wait and send --wait can exceed their advertised timeout substantially; bound each request by the remaining deadline or check expiration immediately after every await.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 07114f2. Each poll and the final full read race the wait's deadline, so a stalled request ends the wait at --timeout. Test: "keeps to the timeout when a read stalls".
…meout - T3 reports only the latest turn's state, so threads read no longer labels earlier turns completed when they may have been interrupted. - Each wait poll and the final read stop at the wait's deadline, so a stalled request cannot stretch --timeout by the 30-second HTTP limit.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07114f2d5d
ℹ️ 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 remembered = turn.turnId === null ? undefined : knownStates.get(turn.turnId); | ||
| const state = thread.latestTurn?.turnId === turn.turnId ? turn.state : (remembered ?? turn.state); | ||
| const outcome = state === "interrupted" || state === "error" ? state : "completed"; |
There was a problem hiding this comment.
Fresh evidence beyond the prior fix is the case where the remembered state is still running: if the awaited turn is interrupted or errors and a queued successor starts entirely between two polling intervals, knownStates never observes the terminal state, and these lines coerce the remembered running (or an unknown state) to completed. Consequently, send --wait can still report success for a failed turn; only return completed when that terminal state was actually observed, or expose the result as unknown.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a271a87. The wait only uses a state it saw while the turn was latest, and a remembered running counts as unknown. If the ending is unknown, it reports the new outcome ended rather than completed. The README and skill document ended. Tests: "reports a completion it saw before a later turn replaced it" and the ended cases.
| const read = await beforeDeadline( | ||
| this.inspect(threadId).catch((error: unknown) => { |
There was a problem hiding this comment.
When more than ten newer turns become visible while waiting for a specific message, the awaited message is absent from every bounded turnLimit=10 inspection. observeTurn then remains null, and because the full read is only performed after a non-null outcome, an already-finished target eventually produces THREAD_WAIT_TIMEOUT. Preserve bounded polling normally, but fall back to a full read when messageId cannot be found in the inspection window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a271a87. When the awaited message is missing from the bounded window, that poll reads the whole thread instead. Test: "reads the whole thread when the awaited message falls outside the poll window".
| .sort((left, right) => (right.updatedAt ?? "").localeCompare(left.updatedAt ?? "")) | ||
| .map((thread) => ({ ...thread, status: threadStatus(thread) })); |
There was a problem hiding this comment.
When /api/orchestration/shell is unavailable, catalog() falls back to the full snapshot, whose thread objects can contain messages, activities, checkpoints, and proposed plans. Spreading those objects here makes threads list --json emit complete conversation and tool history instead of the advertised lightweight thread metadata, potentially producing a very large response and exposing unrelated transcript contents; map through the existing summary projection before returning list entries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a271a87. List entries go through the existing summary projection, which drops messages, activities, checkpoints, and plans, so the snapshot fallback returns metadata only. Test: "lists thread summaries when only the full snapshot is available".
…ist output - A wait that never saw how the awaited turn ended reports the new outcome ended instead of claiming completed. - When newer turns push the awaited message out of the bounded poll window, the wait reads the whole thread instead of timing out. - threads list returns thread summaries even when it falls back to the full snapshot, which carries whole transcripts.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a271a87a1b
ℹ️ 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 deadline = Date.now() + this.verificationTimeoutMs; | ||
|
|
||
| do { | ||
| const inspected = await this.inspect(command.threadId).catch(() => null); |
There was a problem hiding this comment.
When the thread-detail endpoint stalls, this inspect() can consume the API client's 30-second timeout and then fall back to another 30-second snapshot request, even though verification advertises a 5-second limit. Fresh evidence beyond the fixed wait-loop issue is that both dispatchTurn here and dispatchSettlement still await unbounded reads, so threads send, settle, and unsettle can hang for roughly a minute before reporting a 5-second verification failure; race each inspection against the remaining verification deadline.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 55d0204. Every verification read in dispatchTurn and dispatchSettlement races the verification deadline, so a stalled request ends at the 5-second limit. Test: "ends turn verification at its deadline when a read stalls".
Each verification read races the 5-second deadline, so a stalled request no longer holds threads send, settle, or unsettle for the HTTP timeout and its snapshot fallback.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55d0204d4d
ℹ️ 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 next = `\n… [${value.length - (limit - marker.length)} characters omitted] …\n`; | ||
| if (next.length === marker.length) break; | ||
| marker = next; |
There was a problem hiding this comment.
When recalculating the omitted-character count changes the marker text without changing its length, this branch breaks while retaining the previous marker. For example, clipping 31 characters to a limit of 30 keeps one source character but reports only 29 omitted instead of 30; longer inputs similarly underreport by roughly the marker length. Assign next to marker before stopping so truncated transcripts report the actual omitted count.
Useful? React with 👍 / 👎.
2 participants
Builds on #1 by @mhmdkhalaf, whose commits are merged here unchanged, and adds a skill for working with existing T3 Code threads.
Changes
threads list|inspect|read|send|settle|unsettle. The conflict resolution keeps main's flush-then-exit tail. Because the CLI now exits explicitly, the in-process CLI parsing tests run the built CLI instead.$t3thread <thread-id> <instruction>: a new skill inskills/t3thread. It inspects the thread and reads only what the instruction needs. It can brief you on the thread, answer questions about it, continue or review its work, message it and wait for the reply, or ask another model for a second opinion.threads readdetail levels:--detail answers|messages|full,--turns,--first-turn, and--max-chars. T3 stores user prompts without a turn id, so the CLI assigns each prompt to the turn it started.--last-turnnow includes the prompt that started the turn.threads send --waitandthreads wait: wait for the turn that handles a message, or for whatever the thread is doing, and return that turn. They stop early when the thread needs an approval or an answer.settleand the waits read pending requests from the activity log.Merge note
Merge this with a merge commit rather than a squash. That keeps the commits from #1, so GitHub marks #1 as merged.
Testing
pnpm checkpasses with 103 tests at this commit. Checked live against T3 0.0.45-nightly: reads on Claude and Codex threads, then send and wait on a throwaway thread, a wait blocked on an approval, and settle.