Skip to content

fix(mcp): bound delegation waits and recover completion delivery - #11997

Open
lnieuwenhuis wants to merge 1484 commits into
pingdotgg:t3code/codex-turn-mappingfrom
lnieuwenhuis:fix/v2-delegation-wait-recovery
Open

lnieuwenhuis wants to merge 1484 commits into
pingdotgg:t3code/codex-turn-mappingfrom
lnieuwenhuis:fix/v2-delegation-wait-recovery

Conversation

@lnieuwenhuis

Copy link
Copy Markdown
Contributor

Long delegate_task waits could exceed the MCP client's timeout before returning the child's IDs, leaving the caller unable to reconcile the child and encouraging duplicate dispatches.

Use a 30-second default and 45-second maximum wait budget for delegation and thread waits. Keep the child running, return recoverable handles on wait timeout, and upgrade completion delivery when a waiting call is interrupted or fails. Reconcile unobserved terminal results after parent settlement while preserving ownership of an existing wake, so recovery neither drops nor duplicates delivery. Tool guidance explains same-key retry while the parent is active in the same provider session and reconciliation when it is not.

Targets t3code/codex-turn-mapping for #2829. Refs #11168.

Validation: 40 focused service, toolkit, production-orchestrator, and durable completion-delivery tests pass. Regressions failed before each fix, including the missed and duplicate wake races found in adversarial review. Scoped server typecheck, targeted lint/format, and new-commit secrets scans pass.

Limits: the budget bounds waiting, not arbitrary dispatch/readback latency. Existing acknowledgement-before-HTTP-response semantics remain; a lost response after acknowledgement may suppress an automatic wake, but stored results remain available through status/list reads. No live-provider or socket-disconnect test was run.

The repository scanner config retains default rules and exempts only the exact synthetic idempotencyKey fixture in packages/effect-codex-app-server/src/protocol.test.ts, which otherwise flags twice in the inherited V2 history. Negative controls confirm other values, keys, paths, and credential rules remain detected.

Implemented with GPT-6 Astra via Codex.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Sep 16, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Macroscope has since reviewed this pull request. An earlier review was skipped by a cost limit; a review has now completed, so that notice no longer applies.

@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR changes production MCP wait defaults and the durable completion-delivery state machine, including behavior during timeout, interruption, and parent settlement. It also adds a repository-wide secret-scanner allowlist for a test fixture, so the default and static-analysis policy changes require human review.

You can add or adjust custom eligibility rules. Learn more.

@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:XXL 1,000+ changed lines (additions + deletions). labels Sep 16, 2026
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 2 times, most recently from 341b1ac to f63b335 Compare September 16, 2026 04:26
juliusmarminge and others added 22 commits September 15, 2026 23:58
…s survives shoulder tabs

The composer went frameless exactly when the stash or tasks badge was
showing: main's pingdotgg#7150 css hides the classic shell chrome whenever the
shell :has() a shoulder tab or top drawer and repaints the glass on
[data-chat-composer-main-surface] instead — an element the branch's
composer body never rendered, since it predates the drawer system while
the badges and css came through the rebase in main's new form.

The branch composer's frame div now carries the main-surface attribute
(with main's relative z-10 stacking) so attached mode paints background,
outline, and backdrop on it and the tab connects to the surface, and
ChatView applies chat-composer-glass-shell-attached while banner items
render in the drawer slot, matching main's externalComposerDrawerAttached
wiring. Without a tab or banner the attribute is inert and the classic
shell chrome paints as before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round 10 restored the branch's pre-pingdotgg#7150 composer body while its
satellites (shoulder badges, banner drawers, glass css) arrived in
main's new form, and every seam between the two was a visible bug:
opaque/frameless composer, overflowing stash tab, detached stash menu.

Rebuilt ChatComposer via a reverse three-way merge (main's body as the
base, branch delta re-applied): dispatchMode send boundary, live-capable
approval gates, latestRun reads, and the v2 context-window meter stay;
everything else now matches main, including ComposerPrimaryActions and
the sendDisabledReason send gating. Attachment uploads stay off until
the v2 claim path lands.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The v2 timeline rendered every tool call as its own raw row; main's
tool-group collapsing (generated "Ran N commands and changed M files"
summaries, the live "Running <program>" pill for the active tool run,
and the "+N previous log entries" clamp for mixed groups) never made it
into the v2 row model.

Ported the work-live/work-toggle row kinds and group summarization into
the v2 derive, keyed on v2 item types (command_execution, file_change,
file_search, dynamic_tool, subagent) and runId lifecycle instead of v1
activities. Expanded groups keep the branch's richer per-entry detail
rows (V2ItemInspector) — only the collapsed presentation converges on
main.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
v2 already projected todo_list plan artifacts (deriveActivePlanState
existed with zero consumers), but nothing rendered them: todo_list turn
items showed as a bare "Updated tasks" work row, the composer Tasks
drawer never appeared, and the working row had no current-step label.

todo_list items now become inline turn-plan chips (mini step segments,
current step, N/M count, expandable step list) that fold with their
settled turn, ChatView derives the composer Tasks drawer progress and
steps from the running run's plan artifact, and the working row shows
"Working for Xs · <current step>" like main.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collapsed command rows rendered "Command" plus the raw result JSON as
the preview; the command itself is the useful collapsed line, so it now
renders as the row text (whitespace-collapsed, truncated) with stdout
and the full payload behind the expander. Tool-like headings drop the
bold foreground for the muted secondary-label the summary rows use, and
the "+N previous tool calls" toggle loses its bold black label for the
same muted treatment.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extends the command-row fix to the whole preview: file-change rows were
still leaking raw diff lines into the collapsed line. workEntryPreview
now resolves input-first — command, then touched-file paths, then
detail (which is input for the remaining types: search patterns,
reasoning text, error messages) — so outputs only appear behind the
expander.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round-11 rebase onto main (25 commits). Reverse-merged main's new work
into the v2 cutover files: pingdotgg#8395 muted ordinary-tool-failure treatment
(v2-adapted workEntrySignalsSevereFailure keyed on error items), pingdotgg#5931
sidebar project-filter combobox + #4c51 keyboard pin/settle with their
ChatView support graph, the auto-settle-mode migration through
threadSettled/threadListV2, pingdotgg#8235 file/unknown attachment schemas moved
into chatAttachment.ts with nullable attachment paths, pingdotgg#8481 client
analytics through the v2 ws layer, pingdotgg#8480 OpenCode server owner wired
into the driver beside the v2 orchestration adapter, and the mobile
semantic-theme migration applied to branch-only components.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports the v2-applicable half of pingdotgg#8480 (the v1 adapter rewrite was not
carried; this branch's OpenCode path is OpenCodeAdapterV2). Permission
and question asks from child sessions — task subagents and their
descendants — were dropped because the adapter only looked up root
thread sessions. Related sessions now map back to the owning root
state (registered from task parts and session.created/updated parent
chains), and an ask that arrives before the relation is known resolves
it inline via session.get with a short forked backoff, then surfaces
the approval on the root turn. Replies already route by native request
id. Interrupts now tolerate the abort racing turn settlement instead
of failing the stop.

Covered by a new opencode_child_approval replay fixture where the
child asks for bash permission before the task part reveals the
relation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Small rebase onto main (4 commits). The composer stash-shortcut label
and the mobile start-task menu refactor merged onto the v2 composer
bodies, and main's new auto-settle list tests are ported to the v2
thread-list test file (latestRun/RunId shapes). The auto-settle
machinery itself already matched main from the round-11 reconcile.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round-13 rebase onto main (2 commits). Main reverted the auto-settle
opt-in (pingdotgg#8596 undoing pingdotgg#8321), so the branch drops the ported
autoSettleMode machinery and returns to sidebarAutoSettleOnMerge with
settling-by-default, keeping only the v2 shell-shape delta in
threadSettled. The unpin confirmation (pingdotgg#7313) merges into the v2
thread-actions hook, and the mobile list tests re-sync to main's
reverted semantics in v2 shapes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Main's pingdotgg#6636 workspace file drop (drag anywhere over the chat column to
attach, with the dashed overlay) split across ChatView and the
composer. The round-10 rebase restored ChatView from the pre-pingdotgg#6636
backup wholesale and only the composer half was ever re-applied, so
the drop target, overlay, and drag-state plumbing vanished while
addDroppedFiles sat unused on the composer handle. Re-applies main's
ChatView half verbatim; the shared workspaceFileDrop module was
already identical to main.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes the last gap from the rebase follow-up list (pingdotgg#8048/pingdotgg#8161 for
v2): pending uploads staged via the attachment upload URL flow were
never claimed by the v2 orchestrator, so the composer kept
supportsAttachmentUploads off and fell back to inline dataUrls with no
progress UI and no big-file support.

message.dispatch and thread.launch now claim pending refs at intake —
verify the staged file, copy it under a thread-scoped id (the pending
copy stays as the retry source), rewrite the refs, and release the
claimed copies if the dispatch fails. A launch carrying uploads
requires its thread id up front. The web composer reads the
attachmentUploads capability again like main, which lights up the
upload progress overlay, retry-on-failure, and PDF/ZIP attachments.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…omposer

Queued rows now render image thumbnails, and the pencil action loads the
queued message into the composer instead of an inline input: text and
stored attachments are editable (attachments removable, new images
addable), sending saves the queued run in place, and the user's own
draft is stashed untouched for the duration. queued-run.edit gains an
optional full-replacement attachments list end to end.

Built by Claude Fable 5 on Claude Code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the queue rows' up/down arrow buttons with a drag handle (arrow
keys still work on the focused handle). Also prunes optimistic queued
messages once the projection holds them: keying the prune on turn items
alone left a phantom clock row behind whenever a queued run was removed
or steered before it ever started.

Built by Claude Fable 5 on Claude Code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round-14 rebase onto main (7 commits, mostly the Expo SDK 57 upgrade
plus the mobile glass restore and codex app-server buffering fix). The
mobile composer merged main's restored glass chrome onto the v2 body,
and the lockfile is regenerated from main's SDK-57 lock with the
branch's extra dependencies installed on top.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…width

The rebase left a stale early copy of the composer glass-host, context-strip,
and shape() fallback rules that the identical later block always overrode.
The composer shell and queue/context strips also kept main's 48rem width while
the timeline moved to the 46rem content lane; they now share
--chat-content-max-width so one owner defines the chat column.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… tokens

The v2 thread surfaces still styled borders, fills, and status text with
dark:/light: pairs, which do not follow registered custom themes and now fail
the no-mobile-uniwind-theme-escape-hatches lint. Convert them to adaptive
tokens, adding the missing amber/sky badge and neutral hairline/fill entries
to the theme generator.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ThreadQueueControl and ThreadRelationshipsBanner read theme variables only to
tint SymbolView icons and color native modal chrome, the same reviewed interop
pattern as the existing thread-feed entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The plan.md path check read process.platform and process.env directly; thread
the host platform reference and the adapter's provider environment through
GrokAdapterV2Options instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The pull_request workflows never fired for 6c3b84b; only the
pull_request_target ones ran.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Its findings were fixed in 9abca06 and the review threads are resolved;
the check only re-evaluates on push.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round-16 rebase onto main (5 commits, headlined by pingdotgg#8236 file
attachments in the client). Main's upload-aware send path — capability
probes, upload await/retry gating, uploaded-ref turn attachments with
dataUrl fallback, draft release on success — is woven into the v2
dispatch flow, timeline user rows render file attachments as download
links with the ChatView download handler, and the provider settings
editor cleanup keeps the branch's environment-field rows. The codex
feedback client flow stays unported, replay testkit configs gained
main's environmentThemesDir, and the rpc/settings/docs unions carry
both sides.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 25 times, most recently from 6ca6a24 to 3e4ca4c Compare September 23, 2026 23:24
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 4 times, most recently from fe4f6ad to 87c67bd Compare September 25, 2026 05:55

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

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants