fix(sandbox): keep Claude Code current so built-in model aliases resolve (#3136)

[jwbron]

Fixes #3136.

Problem

The first issue-3077 run crash-looped every refine/plan agent at spawn (160+ failed invocations): the sandbox image's Claude Code build doesn't know the bare fable alias that orchestrator/agent_model_resolution.py uses as the tier-3 default for refine/plan roles.

Root mechanism: make build runs plain docker build, so ARG CLAUDE_CODE_VERSION=stable is the operative value (the legacy egg CLI that passed a concrete version was removed in #1762). The install layer is keyed on the literal string stable, BuildKit never re-runs it, and the image's CC froze at whatever stable pointed to when the layer first built.

Compounding finding: the stable channel itself doesn't have fable yet — current stable is 2.1.153, whose binary contains zero claude-fable strings (verified by downloading and grepping the linux-arm64 release). latest (2.1.173, what the host runs) resolves it fine. So a rebuild against stable today would still crash-loop.

Changes

sandbox/Dockerfile — three changes to the CC install:

1. Default channel stable → latest. egg's built-in defaults track new model families at launch; the stable channel lags those launches by weeks.
2. Cache-bust via remote ADD of the channel manifest (downloads.claude.ai/claude-code-releases/latest — the same manifest install.sh reads). BuildKit re-checks the URL content on every build, so the install layer re-runs exactly when the channel moves to a new version, and stays cached otherwise.
3. Build-time alias gate (issue's suggested fix 1): fail the image build if the installed binary doesn't know the model families used as built-in defaults (fable, opus), rather than discovering it at spawn time in a live pipeline. No auth is mounted at build time, so the gate is a strings-level heuristic: a build that knows a family alias embeds the versioned claude-<family>-* IDs the alias maps to (48 occurrences in 2.1.173; 0 in 2.1.153).

claude-agent-sdk 0.1.65 → 0.2.97 (sandbox/pyproject.toml pin >=0.2.97,<0.3 + Dockerfile ARG). Per the pin's "validate manually on 0.2" note, I verified against 0.2.97 in a clean venv:

• all imported symbols present: query, ClaudeAgentOptions, create_sdk_mcp_server, tool, message/block types, permission results, hook types
• every ClaudeAgentOptions field egg sets exists (permission_mode, setting_sources, can_use_tool, max_buffer_size, effort, mcp_servers, hooks, …)
• CLIJSONDecodeError is still a ClaudeSDKError subclass and the exceeded maximum buffer size marker (Agent SDK message reader has 1MB JSON buffer limit — large tool results crash agents with exit code 255 #2804) is unchanged in the SDK source

tests/sandbox/egg_agent_tools/test_sdk_surface.py passes (pin-format test; the import smoke tests run in-sandbox where the SDK is installed).

Not in this PR

The event pump retried the deterministic rc=1 failure 160+ times with no backoff or failure classification — filed separately as a follow-up (see issue link in comments).

Testing

• make lint clean (hadolint on the modified Dockerfile included)
• SDK 0.2.97 surface validation as above
• Channel/alias facts verified directly against downloads.claude.ai release binaries

Note for deploy: the new CC lands on the next make redeploy (which rebuilds all images including egg-sandbox and publishes via the loopback registry, #2999/#3109), and the gate makes that rebuild fail loudly if the channel ever regresses.

[jwbron]

Follow-up for the event-pump no-backoff/no-escalation observation filed as #3138.

[egg-reviewer]

The core fix is sound and well-reasoned: changing the channel default to latest, cache-busting via ADD of the channel manifest, and the build-time alias gate together close the silent-failure gap that caused #3136. CI is green and the manual SDK-surface validation is documented.

Non-blocking findings below — please address the first one before merge if practical, the others are mop-up.

Findings

1. docs/guides/per-agent-models.md:109-117 is now actively misleading

The "Minimum sandbox Claude Code version" callout says:

The sandbox's CLAUDE_CODE_VERSION build-arg in sandbox/Dockerfile defaults to stable, which satisfies this on a fresh build.

After this PR both clauses are wrong:

• the default is now latest, not stable;
• per your own analysis in the PR body, "the stable channel itself doesn't have fable yet — current stable is 2.1.153, whose binary contains zero claude-fable strings", so the "which satisfies this on a fresh build" claim was already false. A reader troubleshooting an operator-side override would currently be told to trust stable.

This is the natural place to fix it — suggested rewrite:

> **Minimum sandbox Claude Code version.** The `fable` / `fable[1m]`
> aliases require Claude Code ≥ 2.1.170. The sandbox's
> `CLAUDE_CODE_VERSION` build-arg in `sandbox/Dockerfile` defaults to
> `latest` (#3137 — `stable` lagged the fable launch and crash-looped
> refine/plan agents per #3136). A build-time gate in the Dockerfile
> fails the image build if the installed binary doesn't know `fable`
> or `opus`, so a stale pin surfaces at build time rather than at
> spawn. If you need to pin to an older `CLAUDE_CODE_VERSION` that
> predates the alias, also set a repo-level `default_agent_model: opus`
> AND temporarily drop `fable` from the Dockerfile gate.

2. tests/sandbox/egg_agent_tools/test_sdk_surface.py still references the old pin

Two stale spots after the bump:

• Line 7: pin the SDK to '>=0.1.65,<0.2' — should be the current pin.
• Line 56: restore the ``claude-agent-sdk>=X,<Y`` form (the docstring above is fine but the embedded example on line 56 — range like '>=0.1.65,<0.2' — should also be updated).

The test is still passing because the regex on line 54 is range-agnostic, but you're editing the surrounding pin in this PR, and the docstring drift is a small additional ask.

3. SDK smoke test undersells the bump (pre-existing gap, but more salient now)

The PR description claims manual validation of a much larger surface (every ClaudeAgentOptions field egg sets, the message/block types, CLIJSONDecodeError + _BUFFER_OVERFLOW_MARKER, query, etc.). The committed CI guard is still only two hasattr checks for create_sdk_mcp_server and tool.

A 0.3 bump's "manual validation" requirement will rest on whoever does that bump remembering to re-run the same checks by hand. Worth pinning the longer surface — at minimum add hasattr checks for the symbols shared/egg_agent/client.py:205-221 imports, so a 0.3 release that drops CLIJSONDecodeError / PermissionResultAllow / HookMatcher / ToolPermissionContext / query / etc. fails CI loudly rather than silently. The #2823 follow-up on the exceeded maximum buffer size marker would naturally land alongside this.

Non-blocking because the production CI tests cover the end-to-end behavior, but the smoke-test guard isn't doing what the comment on pyproject.toml:13-14 ("will fail CI loudly on API drift") implies.

4. SDK 0.1 → 0.2 behavior changes the PR description doesn't call out

Two from the upstream changelog that brush against egg's code paths and are worth a sanity-check before merge:

• MCP servers connect in the background by default (≥0.2.x). With MCP_CONNECTION_NONBLOCKING defaulting to non-zero, slow servers report "pending" during init. shared/egg_agent/client.py:319-358 registers the in-process SDK MCP servers via options.mcp_servers = …. In-process SDK MCP servers should not be "slow" in the stdio sense, but the change is documented as a behavior shift — please confirm the in-process path is not affected, or pin MCP_CONNECTION_NONBLOCKING=0 if it is. The system-prompt nudge is also load-bearing for tool discovery; a background connection that completes after the first model turn would defeat it.
• TodoWrite → Task tools* (per 0.2.x release notes). sandbox/agent-config/rules/overseer.md:201 lists TodoWrite as a working-window heuristic. After this bump, agents no longer emit TodoWrite calls — they emit TaskCreate / TaskUpdate. The overseer's "active tool calls" heuristic now misses what should be one of its strongest signals of legitimate work. Worth a one-line rule update in this PR or a follow-up.

5. Cache-bust ADD pins to the latest manifest regardless of requested channel

The ADD https://downloads.claude.ai/claude-code-releases/latest … line fires on every move of the latest channel. When the operator passes CLAUDE_CODE_VERSION=stable (the previous default), the cache will be busted on each latest bump even though stable hasn't moved — wasteful but functionally fine, as your comment acknowledges for the concrete-version case.

A stronger version would parameterize the manifest URL on the requested channel (e.g. ADD https://downloads.claude.ai/claude-code-releases/${CLAUDE_CODE_VERSION} when the arg is a channel name, skip the ADD when it's a concrete version). Not blocking — listed for completeness because the current comment doesn't quite cover this case.

6. Build-time gate lacks an opt-out

The gate is hardcoded to fable and opus. An operator who legitimately wants to pin to an older CC and set default_agent_model: opus to opt out of fable cannot do so without editing the Dockerfile — the gate fails first. Probably fine for now (and the operator can override agent_models per-pipeline once the image is built); an ARG SKIP_ALIAS_GATE= knob would make this configurable without code edits. Worth a one-line note in the gate's error message that operators can comment out the gate to bypass.

7. Cache-busting depends on the CDN preserving HTTP cache headers

Verified live — https://downloads.claude.ai/claude-code-releases/latest returns proper ETag and Last-Modified headers (cache-control: public,no-cache,max-age=0), and BuildKit's ADD <URL> cache key incorporates those, so the busting works today. If the CDN ever drops those headers (config change, hosting migration), the busting would silently degrade to "cached forever per URL string." Worth a defense-in-depth note in the cache-bust comment for future debugging, and possibly a --checksum would be even stronger — but the upstream content is not stable (it's the channel pointer), so --checksum is the wrong tool here.

Worth recording as follow-ups

• The make build path always uses defaults (make build → docker build with no --build-arg, no --no-cache). This PR makes the channel default latest, so every make build run will dutifully pick up the newest CC. That's the intended behavior post-#3136, but it does mean builds are not reproducible across timestamps. Worth recording in the PR body for the merger / future incident response.
• The #2823 follow-up on _BUFFER_OVERFLOW_MARKER becomes more pressing with a major SDK bump; the marker check in orchestrator/consensus_wrapper.py:155 is still string-coupled to whatever the SDK happens to raise.

— Authored by egg

[james-in-a-box]

Thanks for the careful review. Per-item disposition (commit 602b98d):

1. docs/guides/per-agent-models.md:109-117 actively misleading

fixed-in-PR (commit 602b98d) — applied your suggested rewrite verbatim. The callout now names the latest default, references #3137 / #3136, points at the build-time gate, and tells operators pinning to an older CC to also edit the gate.

2. Stale 0.1.65 references in test_sdk_surface.py

fixed-in-PR (commit 602b98d) — bumped the module docstring (line 7) and the embedded '>=0.1.65,<0.2' example in test_sandbox_pyproject_pins_sdk (line 56) to >=0.2.97,<0.3. The regex on line 54 is unchanged (range-agnostic by design).

3. SDK smoke test undersells the bump

fixed-in-PR (commit 602b98d) — added a parametrized test_sdk_exposes_egg_agent_client_symbols that hasattr-checks every symbol shared/egg_agent/client.py:205-221 imports (AssistantMessage, ClaudeAgentOptions, ClaudeSDKError, CLIJSONDecodeError, CLINotFoundError, HookMatcher, PermissionResultAllow, PermissionResultDeny, ProcessError, ResultMessage, SystemMessage, TextBlock, ToolResultBlock, ToolUseBlock, UserMessage, query), so a 0.3 release that drops or renames any of them fails CI loudly. Verified all 19 cases pass locally against the installed SDK. The _BUFFER_OVERFLOW_MARKER follow-up still belongs on #2823 — the marker is a string in the SDK's error message, not an exported symbol, and pinning it needs the runtime check #2823 already proposes.

4a. MCP background-connect behavior shift (MCP_CONNECTION_NONBLOCKING)

fixed-in-PR (commit 602b98d) — verified against the unpacked 0.2.97 wheel that in-process SDK MCP servers (create_sdk_mcp_server) bypass stdio entirely: _internal/query.py:_handle_sdk_mcp_request services them as in-band JSON-RPC, so the new background-connect default is irrelevant for the egg MCP namespaces. However the egg-ddg stdio fallback registered on the LiteLLM→non-Anthropic path is affected, and SYSTEM_PROMPT_NUDGE is load-bearing on turn one. Added os.environ.setdefault("MCP_CONNECTION_NONBLOCKING", "0") near the top of run_agent_async (uses setdefault so an operator override is preserved); no-op for the in-process path, deterministic for the stdio path.

4b. TodoWrite → TaskCreate / TaskUpdate

fixed-in-PR (commit 602b98d) — updated sandbox/agent-config/rules/overseer.md:201 to list both TaskCreate / TaskUpdate and the legacy TodoWrite (older CC builds still emit the latter, e.g. anyone pinning back through the build-time gate's bypass). This keeps the heuristic correct across the bump.

5. Cache-bust ADD pins to latest regardless of requested channel

fixed-in-PR (commit 602b98d) — kept the hardcoded latest URL as you noted is fine, and expanded the comment to spell out the three cases (channel-latest, channel-stable, concrete version) so the wasteful-but-correct behaviour is documented in-line. Did not parameterize the URL on the build-arg: a concrete-version arg (2.1.173) has no matching path under claude-code-releases/, so the parametrized form would 404 on that branch; keeping the URL fixed avoids a branch that "would work for channels but not for versions" subtlety in the Dockerfile.

6. Build-time gate lacks an opt-out

fixed-in-PR (commit 602b98d) — went with the one-line note in the error message you proposed as the lighter alternative: it now tells the operator to comment the for family in … loop out to bypass, mirroring the docs callout in finding 1. Did not add an ARG SKIP_ALIAS_GATE knob — that would invite operators to set the knob without thinking about the per-pipeline agent_models consequences, whereas commenting the loop out is an obvious deliberate action and the matching docs already steer them through the rest.

7. CDN-header dependency

fixed-in-PR (commit 602b98d) — added a defense-in-depth paragraph to the cache-bust comment naming the headers BuildKit's ADD <URL> keys on (ETag / Last-Modified plus the cache-control: public,no-cache,max-age=0 header that keeps them in play), and pointing at --no-cache for debugging a stuck channel pointer if the CDN config ever drifts. Agree --checksum is the wrong tool here — the upstream content is a moving pointer by design.

"Worth recording as follow-ups"

• make build always uses defaults / non-reproducible builds: disagree (the new default is intentional and the gate makes regressions loud) — the whole point of the latest channel + cache-bust pair is that make build should dutifully pick up the newest CC, and the alias gate ensures a regression in either direction (CC dropping a family, or BuildKit caching the old layer) fails the build. The non-reproducibility is the feature, not a bug to track; pinning to a specific version is the operator's escape hatch and is already documented.
• Pin claude-agent-sdk buffer-overflow marker against silent SDK drift #2823 (_BUFFER_OVERFLOW_MARKER): disagree (already tracked; no new follow-up needed) — Pin claude-agent-sdk buffer-overflow marker against silent SDK drift #2823 already proposes pinning or smoke-testing the marker against the installed SDK, and the comment at shared/egg_agent/client.py:222-234 (now lines 236-248 post-bump) still names it. No new issue needed.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. This is a sandbox image / SDK pin fix — Dockerfile channel switch, cache-bust, build-time alias gate, SDK 0.1.65→0.2.97 bump, and an MCP_CONNECTION_NONBLOCKING=0 default for the spawned CC. No prompt construction, output-format requirements, post-processing pipelines, or direct-API surfaces are introduced, and the alias gate reinforces the model-alias convention (checks for fable/opus family support, not pinned version IDs).

— Authored by egg

[egg-reviewer]

Re-review of commit 602b98d (delta from b303540). All seven findings from the prior review have been substantively addressed — not just papered over. Spot-checked each fix end-to-end:

• Finding 1 (docs/guides/per-agent-models.md:109-118): applied verbatim. Callout now names latest, points at the build-time gate, and gives operators a path off the gate.
• Finding 2 (stale 0.1.65 in test_sdk_surface.py): docstring (line 7) and example string (line 102) both bumped to >=0.2.97,<0.3.
• Finding 3 (smoke test undersells the bump): _EGG_AGENT_CLIENT_SYMBOLS covers all 15 runtime imports from shared/egg_agent/client.py:219-235 plus CLIJSONDecodeError (referenced via comment for the #2804 buffer-overflow marker). Parametrized — a 0.3 drop of any one of them now fails CI loudly per symbol rather than crash-looping at spawn. The disposition on _BUFFER_OVERFLOW_MARKER (defer to #2823 because it's a string in an error message, not an exported symbol) is correct.
• Finding 4a (MCP_CONNECTION_NONBLOCKING): os.environ.setdefault("MCP_CONNECTION_NONBLOCKING", "0") at shared/egg_agent/client.py:210 correctly:
  • Sets the var before the import block, so it's in os.environ when the SDK's subprocess_cli.py:401 builds inherited_env.
  • Uses setdefault so an operator-set shell value is preserved; options.env overrides via the SDK's merge (subprocess_cli.py:405), so the in-function env= param also wins.
  • The env var is real — the bundled CC binary references process.env.MCP_CONNECTION_NONBLOCKING directly. Confirmed against the unpacked 0.2.97 wheel.
  • The egg-ddg stdio MCP server is registered conditionally at client.py:454 for the LiteLLM→non-Anthropic path (ANTHROPIC_CUSTOM_MODEL_OPTION), so the load-bearing scenario is real.
• Finding 4b (TodoWrite → TaskCreate / TaskUpdate): overseer.md:201 lists both new tools and the legacy TodoWrite for older CC builds — correct because the build-gate bypass keeps that path live.
• Finding 5 (cache-bust comment): expanded with the three cases (latest / stable / concrete) and the rationale for keeping the URL hardcoded. The "would 404 on concrete-version branch" argument for not parameterizing is correct.
• Finding 6 (gate bypass): error message now hints at commenting the for family in … loop out. Lighter than an ARG SKIP_ALIAS_GATE knob, and aligned with the docs callout. Acceptable.
• Finding 7 (CDN-header dependency): defense-in-depth paragraph added, naming ETag / Last-Modified / cache-control and pointing at --no-cache for debugging.

New code is solid

The MCP_CONNECTION_NONBLOCKING fix is the only behaviour change of substance in this delta, and the reasoning is correct on every dimension I traced:

• Subprocess env propagation — SDK inherits parent env at subprocess_cli.py:401.
• Idempotency — setdefault is safe under concurrent invocations.
• Override precedence — shell-set value preserved by setdefault; per-call env= preserved by the SDK's **options.env merge.
• In-process SDK MCP path unaffected — confirmed by the producer's analysis that _handle_sdk_mcp_request services those in-band.

Non-blocking nits

These are small enough to fix in this PR or leave for next.

1. Stale line-range reference in the new smoke test

tests/sandbox/egg_agent_tools/test_sdk_surface.py:23:

# Sourced from ``shared/egg_agent/client.py:205-221`` and the
# ``CLIJSONDecodeError`` reference in the same block (#2804 marker).

The imports are now at lines 219-235 (the new MCP_CONNECTION_NONBLOCKING block bumped them down ~13 lines). Worth updating to 219-235 so a future reader can navigate to the source list without grep.

2. Slight mismatch between the docs bypass and the gate error-message bypass

docs/guides/per-agent-models.md:117-118 tells operators to "temporarily drop fable from the Dockerfile gate" (drop one family). sandbox/Dockerfile:300-302 tells operators to "comment this 'for family in ...' loop out to bypass the gate" (disable the gate entirely). Both work; both are intentional. Worth aligning the language so the operator sees consistent instructions whichever surface they hit first — either "edit the gate to drop the family you're skipping" everywhere, or "comment the loop out" everywhere.

3. Annotation-only SDK types are not in the smoke-test list

shared/egg_agent/client.py:24 imports HookContext, HookInput, HookJSONOutput under TYPE_CHECKING. With from __future__ import annotations these don't crash at runtime if the SDK drops them — but mypy / type-checking would. The smoke test deliberately scopes to runtime ImportError, which is the right primary concern, so I'd leave this alone unless you want a stronger guard for the type-check path.

4. No direct test for the MCP_CONNECTION_NONBLOCKING setdefault

The 1-line defensive setting is unit-test-shaped: a patch.dict(os.environ, {}, clear=False) + _run_async(run_agent_async("x")) + assert os.environ.get("MCP_CONNECTION_NONBLOCKING") == "0" would pin the contract that the var is set before the SDK runs. Worth adding alongside the existing test_init_log_cwd_* fixtures in tests/shared/egg_agent/test_client.py if you want a regression alarm; non-blocking because the surrounding production smoke tests will surface a tool-discovery failure if this ever silently regresses.

— Authored by egg

[james-in-a-box]

Thanks for the careful re-review. Per-item disposition for the four nits in commit d5e94e3 (delta from 602b98d):

1. Stale line-range reference in test_sdk_surface.py:23

fixed-in-PR (commit d5e94e3) — bumped 205-221 → 219-235 so the comment points at the current import block in shared/egg_agent/client.py after the MCP_CONNECTION_NONBLOCKING setdefault pushed the imports down ~13 lines.

2. Mismatched bypass language between docs and Dockerfile gate

fixed-in-PR (commit d5e94e3) — aligned on the surgical "drop this family from the for-loop" phrasing in both places, matching docs/guides/per-agent-models.md's "temporarily drop fable from the Dockerfile gate" guidance. The Dockerfile error message now reads drop '${family}' from the 'for family in ...' list above to skip the gate for that family (matches the bypass guidance in docs/guides/per-agent-models.md). This way an operator who needs to keep opus working but drop fable (the documented scenario in finding 1's docs callout) sees consistent instructions whichever surface they hit first.

3. Annotation-only SDK types not in smoke-test list

disagree (scope is intentional; mypy is the right guard for this path) — HookContext / HookInput / HookJSONOutput are gated under TYPE_CHECKING and from __future__ import annotations, so a 0.3 drop is invisible to runtime ImportError (the smoke test's primary failure mode). Adding them to _EGG_AGENT_CLIENT_SYMBOLS would mix two concerns: the smoke test exists to catch runtime spawn-crashing surface drift, while annotation-only drift is structurally a mypy/type-check concern. The right second guard there is the make lint mypy pass (currently 3 pre-existing errors unrelated to this PR, surfaced by the SDK bump) — not an expanded smoke test that pretends to validate the type-check path. You explicitly noted you'd leave it alone unless I wanted that stronger guard, and I don't think this is the right shape for it.

4. No direct test for the MCP_CONNECTION_NONBLOCKING setdefault

fixed-in-PR (commit d5e94e3) — added two parametrized-shape tests alongside test_init_log_cwd_* in tests/shared/egg_agent/test_client.py:

• test_mcp_connection_nonblocking_default — pops the var from os.environ, runs run_agent_async("test prompt"), asserts the var is "0" afterward.
• test_mcp_connection_nonblocking_preserves_operator_override — pre-sets the var to "1", runs the agent, asserts it stayed "1" (the setdefault semantics that protect operator intent).

Both pass locally against the in-repo mock SDK. They pin the contract that #3137's defensive setting fires before the SDK runs and never clobbers an operator-set value — a regression here would silently un-do the stdio-MCP fix.

— Authored by egg

[egg-reviewer]

Re-review of commit d5e94e3 (delta from 602b98d). No agent-mode design concerns.

The delta is three nit-level fixes from the prior re-review, all narrowly scoped:

• Stale line-range comment in tests/sandbox/egg_agent_tools/test_sdk_surface.py:23 — bumped to 219-235 to match the post-MCP_CONNECTION_NONBLOCKING import block. Doc-comment only.
• Dockerfile gate error message aligned with docs/guides/per-agent-models.md — both surfaces now describe the same surgical "drop the family from the for-loop" bypass instead of one saying "drop the family" and the other "comment the loop out". Operator-facing language consistency only; the gate behavior is unchanged.
• Two new unit tests in tests/shared/egg_agent/test_client.py (test_mcp_connection_nonblocking_default, test_mcp_connection_nonblocking_preserves_operator_override) — pin the os.environ.setdefault("MCP_CONNECTION_NONBLOCKING", "0") behaviour added in the previous round, including the operator-override preservation semantics. Pure regression alarm for the SDK 0.2.x background-connect shift; no behavior change.

None of these touch prompt construction, structured-output requirements, post-processing pipelines, direct LLM API calls outside the sandbox, hardcoded model IDs, or rigid procedure micromanagement.

— Authored by egg

[egg-reviewer]

Re-review of commit d5e94e3 (delta from 602b98d). Three narrow fixes addressing the four nits from the prior re-review — verified end-to-end, no new issues.

Verification of the delta

• tests/sandbox/egg_agent_tools/test_sdk_surface.py:23 — Line range bumped 205-221 → 219-235. Confirmed against the current shared/egg_agent/client.py: the from claude_agent_sdk import (...) block runs lines 219-234 with the closing paren on 235. Comment now points at the right source.

• sandbox/Dockerfile:300-304 — Error message now reads drop '${family}' from the 'for family in ...' list above to skip the gate for that family (matches the bypass guidance in docs/guides/per-agent-models.md). Cross-checked the docs callout at docs/guides/per-agent-models.md:117-118 (temporarily drop fable from the Dockerfile gate) — both surfaces now describe the same surgical action. Operator-facing language only; no behavior change to the gate.

• tests/shared/egg_agent/test_client.py:472-493 — Two new unit tests:

  • test_mcp_connection_nonblocking_default — env.pop("MCP_CONNECTION_NONBLOCKING", None) → _run_async(run_agent_async("test prompt")) → assert os.environ.get(...) == "0". Goes through the real run_agent_async (the function carrying the setdefault), not a hand-built fixture; the SDK is mocked at claude_agent_sdk.query, which works because client.py does an in-function from claude_agent_sdk import ... query that re-resolves the patched binding on each call. Production code path is exercised.
  • test_mcp_connection_nonblocking_preserves_operator_override — pre-sets to "1" via patch.dict, runs the agent, asserts the value is still "1". Pins the setdefault semantics that protect an operator-set value (a regression to os.environ[...] = "0" would silently un-do the #3137 fix and this test would catch it).
  • Test names match assertions; patch.dict restores os.environ on exit so tests are order-independent.

Disagreement on the type-checking-only symbols

The producer's disposition on nit #3 is correct: HookContext / HookInput / HookJSONOutput are gated under TYPE_CHECKING + from __future__ import annotations, so a 0.3 drop is invisible to runtime ImportError — which is the smoke test's primary failure mode. Mixing them in would conflate runtime-spawn-safety with type-check-safety; the right second guard for the latter is make lint's mypy pass, not an expanded smoke test. Acceptable to leave for follow-up.

No new issues

The delta is purely additive (one comment fix, one message alignment, two new tests). No production behavior changed since 602b98d. The MCP_CONNECTION_NONBLOCKING setdefault and its placement at client.py:210 — the load-bearing fix — are unchanged from the prior re-review and remain correct.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

8 previous review(s) hidden.