Add hosted child fork run context helper - #1467
Conversation
Provide a reusable framework helper that assembles hosted child fork mirror state, pending tool lifecycle, execution buffers, and stream state. Verification: deno test --no-check --allow-all src/agent/hosted-child-fork-run-context.test.ts src/agent/hosted-child-pending-tool-lifecycle.test.ts src/agent/hosted-child-mirror.test.ts; deno task verify:quick Co-authored-by: OmX <omx@oh-my-codex.dev>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dd9567755
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| reasoningMessageId: input.reasoningMessageId, | ||
| }); | ||
| const streamMirrorContext: HostedChildForkStreamMirrorContext = { | ||
| durableRunMirror: Boolean(mirrorContext.mirror), |
There was a problem hiding this comment.
When this helper is used to feed executeHostedChildForkStream with a mirror but without the optional messageId, this sets durableRunMirror to true and the executor immediately mirrors start-step, making hasEmittedProgress() true. Subsequent text chunks are not mirrored because handleHostedChildForkStreamPart only emits text when durableMessageId is present, and final fallback chunks are also skipped because progress was already emitted, so the mirrored child run can end up with no assistant text. Either require/generate a message id for mirrored runs or do not enable the durable mirror without one.
Useful? React with 👍 / 👎.
Codex P1 on veryfront-code#4516. Forwarding the resolved error event to the replay relay meant a NON-provider failure -- turnPersistence.finalize() throwing, say -- reached a public RunError with its raw Error.message, where the path previously emitted a neutral boundary message. A persistence error can carry a database URL or an internal path, which AGENTS.md:118-128 forbids in user-facing output. resolveRelayableExecutionFailure returns a message only for curated provider terminal errors and explicitly public lifecycle messages; anything else keeps the relay's neutral default. The SSE stream path is unchanged -- it may still show a fallback message; a durable client-visible RunError may not. Fixed on BOTH relay sites. The review flagged the stream path; the generate path at the same file had the identical leak. Pinned by a test asserting a persistence failure's message reaches the SSE event but not the relay, and that a truncation still relays its PROVIDER_OUTPUT_TRUNCATED code -- the point of #1467. It fails if the fallback is relayed again.
Summary
Verification