Repository navigation
Read full worker answers before closing child sessions - #153
pepechka-kun wants to merge 1 commit into
Conversation
Worker replies were truncated and completed children could be closed before their results were fetched. Retain bounded paged answers, enforce result receipts, and scope late-close status to the owner.
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: 55bc37da5c
ℹ️ 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 answer = result.answer; | ||
| // A running child can always be interrupted. A completed reply must first | ||
| // be fetched explicitly, even when wait_for_agent returned it in one page. | ||
| if (answer && result.status !== "working") { |
There was a problem hiding this comment.
After a child already has a captured answer, a successful send_to_agent returns as soon as TerminalManager.deliverInput writes the new submitted prompt; the old answer/status remain until the asynchronous working signal clears them in applyProviderSignal. In that window (or whenever that signal is absent), this condition sees the stale answer plus idle/unavailable, while send() has deleted the prior read receipt, so an immediate cancel_agent is refused instead of stopping the just-submitted work. Clear or supersede the old answer on accepted submission, or consult prompt/turn progress before enforcing completed-result reads.
Useful? React with 👍 / 👎.
| // Offsets use UTF-16 code units, but a page must not split an astral character. | ||
| if (end < answer.text.length && /[\uD800-\uDBFF]/u.test(answer.text[end - 1]!)) end -= 1; | ||
| this.answerReads.delete(sessionId); | ||
| this.answerReads.set(sessionId, { signature, through: Math.max(through, end), resultRead: resultRead || (previous?.signature === signature && previous.resultRead) === true }); |
There was a problem hiding this comment.
If the person starts a follow-up directly in the subagent card, renderer input reaches TerminalManager without passing through this handler's send(), so answerReads is never cleared. When that turn returns the same text/truncation flag (for example another short Done.), this assignment preserves the previous through and resultRead because answerSignature contains no turn identity; cancel_agent can then close the new completed turn without the required explicit result read (and, for a long reply, without reading its new pages). Include a monotonically changing answer revision/timestamp in the receipt identity or invalidate receipts on every turn start.
Useful? React with 👍 / 👎.
|
Superseded by #156. The replacement contains all local follow-up fixes in a single topic commit on current main, with fresh local validation. Closing this older history in favor of that PR. |
Final worker replies were truncated, and cleanup could dispose a completed child before its result had been read. A later wait then looked like an invalid session.
Retain masked, bounded final replies and return consecutive pages. Require an explicit result read and all pages before closing a completed child; active work remains cancellable. Return owner-scoped, bounded closed-child receipts without retaining answer text. Larger runtime frames are restricted to authenticated result capture.
Validation: typecheck; full local suite (2,167 tests: 2,141 passed, 26 platform skips), including pagination, escaped payloads, masking, frame limits, read-before-close and foreign/unknown child isolation.
Extracted from #151 into one topical commit directly on the current upstream main. Working installations and services are unchanged.
Companion PRs: #154, #152. All target main and have been tested independently. Their combined merge was also checked locally without conflicts.
Combined validation: typecheck and 153 focused contract checks (151 passed, 2 platform skips).