fix(tools): resolve workflow:<id> before image vision gates and self-heal wedged replays

[detail-app]

Detail bug report: View on Detail

Summary

• Bug: browser_screenshot_handler and _read_image gated image inlining on supports_vision(model), but passed the raw model string from get_session_model — including workflow:<id> bindings — without resolving the binding to its declared output_model (the loop's capability gates do this via _resolve_capability_model). supports_vision("workflow:…") calls litellm.get_model_info, which raises → returns None; after 551ce0f the gate only degrades on is False, so None fell through and inlined the screenshot. For a workflow-bound session whose inner model is text-only, the replayed image_url tool result 400s every wake, latching the session terminal errored with no retry/strip.
• Fix: Both handlers now resolve the binding via _resolve_capability_model before the vision gate (function-local import, avoiding the browser → loop → tools → browser module-load cycle), so a catalogued text-only inner model degrades to a text marker and a vision-capable one inlines. Added a build-time safety net _strip_image_parts_for_non_vision_model in build_messages, sibling to the oversize-clamp / mime-correction passes: it downgrades any persisted image_url part to a text marker when supports_vision(resolved_model) is False, self-healing already-wedged sessions on the next wake. None/True are left untouched to preserve the optimistic-inline policy.

Substrate state changes

None — code-only change.

Test plan

• Unit tests, typecheck, lint, and format all pass (uv run mypy src tests clean on 1084 files; ruff check/ruff format --check clean). Added regression coverage:
  • test_browser_tools.py::TestScreenshot — workflow binding with text-only inner model degrades; vision-capable inlines; unresolved/unknown inlines optimistically; raw model passes through the resolver unchanged.
  • test_read_image.py::TestImageBranch — same four shapes for the read path.
  • test_context.py::TestVisionCapabilityReplayPass — text-only strips persisted image_url (incl. any-role, multi-image, ordering preserved, immutability of Event.data); vision/unknown/no-model keep the part (optimistic-inline policy preserved).
• Integration (new test_workflow_screenshot_vision_gate.py, testcontainer Postgres): drives the real _resolve_capability_model DB lookup — text-only output_model degrades the screenshot, vision output_model inlines it, and a persisted image_url tool-result event is stripped by the real compose_step_context on the next build. Pre-existing test_model_workflow_park_sweep.py still green.
• E2E (live Docker sandbox): test_workspace_image_read.py + test_extensionless_image_read.py pass — read-image inline path unchanged.
• Browser e2e (test_browser_image_contract.py, test_browser_driver_behavior.py) could not be fully run: the aios-browser:dev image builds and a plain docker run works, but the host cgroup /sys/fs/cgroup/user/workload is in domain threaded mode, so Docker cannot apply the resource-limit controllers (--cpus/--memory/--pids-limit) that start_browser_container hardcodes to mirror production. 4 image-metadata tests pass; the rest are blocked by that host restriction (not a fix defect). CI's docker shard runs these green.

Risk / rollback

Low-risk, additive. The trigger fix only changes behavior for workflow:<id> bindings (the raw-passthrough path is unchanged). The new build-time pass only acts on an explicit supports_vision is False (vision/unknown/no-model are untouched), so it cannot strip a legitimate image from a vision-capable or uncatalogued model. If a regression appears, revert the commit — no schema/env/migration changes are involved.

---

Automatic Fixes PRs can be configured here.

[eumemic-bot]

Code review

Verdict: pass. The diagnosis is correct, the fix is at the right two layers (trigger + backlog), and the regression tests are genuinely load-bearing. Three non-blocking observations below; none of them blocks the merge.

---

Scope verified

57d411ce, 7 files, +944/−1: src/aios/harness/context.py (new _strip_image_parts_for_non_vision_model build pass), src/aios/tools/browser.py + src/aios/tools/read.py (resolve workflow:<id> before the vision gate), plus unit/integration coverage.

What I checked and how

The trigger fix is correct. get_session_model (db/queries/sessions.py:1839) returns COALESCE(s.model, av.model, a.model) verbatim — a workflow:<id> binding included. supports_vision (harness/vision.py:73) calls litellm.get_model_info, which raises on that string and returns None by design, and both handlers gate on is False, so the pre-fix path inlined. I confirmed the raised-exception→None path empirically:

vision.litellm_lookup_failed error="This model isn't mapped yet. model=workflow:wf_text_only..."

Both handlers now route through _resolve_capability_model (loop.py:278), the same resolver run_session_step uses at loop.py:947 — so the tool path and the loop's gates key on the same string. The function-local import is justified (browser → loop → tools → browser) and matches existing precedent in this tree (tool_dispatch.py:709, step_context.py:333, obligations.py:281).

The tests are real, not tautological. I reverted the three src/ files to HEAD~1 and re-ran the new tests: 4 fail (test_text_only_model_strips_persisted_tool_result_image, test_text_only_model_strips_multiple_images_preserves_other_parts, test_workflow_binding_with_text_only_inner_model_degrades_to_text, test_raw_model_skips_workflow_resolution_unchanged), plus the read-path workflow test. All green on head. This is the property under test failing on the old code, not a test written to its own patch.

Focused runs on head, all green (repo-wide suites left to CI):

• test_context.py + test_browser_tools.py — 230 passed
• test_read_image.py + test_browser_tools.py + test_model_binding.py + test_switch_channel.py — 105 passed
• test_context_image_cache.py + test_switch_channel_recap_vision.py + test_vision.py — 47 passed (the adjacent clamp/persist and recap-vision invariants are undisturbed)

The build-time pass respects the invariants it claims to. It replaces part dicts with fresh copies and rebuilds the message via {**msg, "content": new_content} — never mutating the aliasing Event.data; the test snapshots [e.data for e in events] and asserts equality, which is the right shape. It is placed with its siblings at context.py:1813-1815 and short-circuits on model falsy, matching the bool(model) and … convention used ten lines below. The is not False guard genuinely preserves the optimistic-inline policy — None and True both fall through untouched.

No new litellm import cost. I initially suspected the unconditional supports_vision(model) call would newly force the ~1.2s litellm bootstrap into every build_messages, since previously it was only reached via can_inline_image when a message carried attachments. It does not: build_messages already unconditionally calls model_descriptor(model) (context.py:1836), which imports litellm at completion.py module top. The strip pass only moves that a few lines earlier. TestLazyLitellmBoundary::test_context_import_does_not_import_litellm still passes — the boundary it pins is import-time, not call-time. Measured steady-state cost of the extra call: 0.0003 ms for a catalogued model, 0.19 ms on the raise path.

The self-heal reaches the wedged sessions it claims to. compose_step_context builds with gate_model = capability_model if capability_model is not None else agent.model (step_context.py:702), and loop.py:947 supplies the resolved value, so the worker path keys the new pass on the inner model. The integration test drives the real compose_step_context against a real workflow row rather than a mocked seam, which is the right level for this claim.

Non-blocking observations

1. switch_channel's recap is the third image_url-producing surface and still keys on the raw binding. switch_channel.py:194 reads queries.get_session_model and threads it unresolved into render_reorient_block(model=…) → render_user_event → can_inline_image → supports_vision("workflow:…") → None → inlines. _assemble_recap then widens the tool result to a content-parts list, which is persisted.

   Property (not blocking): for every surface that constructs an image_url part from a session's bound model, the capability decision is made against the resolved inner model, not the raw binding string — so no surface persists an image part that a later build is guaranteed to strip.

   I am flagging this as non-blocking rather than failing the PR because the new build-time pass does catch it before the provider sees it: the model never receives the part, so there is no wedge. The residual cost is a multi-MB base64 payload written into a text-only session's event log to be discarded on every subsequent build — real waste, given how much this codebase's own vision.py docstring cares about persisted image mass, but not a correctness defect and reasonably out of scope for a bug-fix PR.

2. GET /v1/sessions/:id/context does not pass capability_model, so the preview does not strip. routers/sessions.py:1062 calls compose_step_context without the kwarg, so gate_model falls back to agent.model — the raw workflow: string — and supports_vision returns None, leaving the image parts in. The preview will therefore show an image_url part the worker would have stripped. This divergence predates the PR (the same endpoint already diverges on attachment inlining for bound models) and the endpoint is documented as having known divergences, so it is not a regression. Worth a line in that docstring's divergence list if someone touches it.

3. parse_workflow_model raises outside _resolve_capability_model's try block, and two new call sites now inherit that. loop.py:292 calls parse_workflow_model before the try: at line 298, so a malformed binding (workflow:, workflow:wf@abc — nothing in AgentCreate.model or enforce_workflow_binding_privilege rejects these) raises BindingBoundaryError rather than degrading. In a tool handler that is neither ToolBail nor AiosError, so _classify_tool_error (tool_dispatch.py:96) returns evict=True — a malformed binding would now fail the read/screenshot and evict the session's sandbox. Reachability is essentially nil: run_session_step resolves at line 947 before dispatching any tool, so such a session cannot reach a tool call at all except via an update_agent racing an in-flight background tool task. Noting it because the docstring at loop.py:287-290 promises "never an error," and that promise is now relied on by two more callers than when it was written.

Conventions

Docstrings match the density and the "state the failure mode and why this posture" style of the surrounding passes; the function-local imports carry the cycle rationale inline, as the neighbouring lazy imports do; the integration test's _VISION_OVERRIDES save/restore in the fixture finally matches the pattern in test_read_image.py, test_switch_channel_recap_vision.py, and test_context_image_cache.py. mypy/ruff were left to CI per scope.

[eumemic]

@cursor update to latest master

[cursor]

This branch is already on latest master. I fetched origin/master and compared it to detail/bug-fix/fix-tools-resolve-workflow-id-before-image-vision-c8e05e — no further merge or rebase was needed.

• Latest master: 7271ae68 (fix(workflows): pin legacy bash rows on re-drive… fix(workflows): pin legacy bash rows on re-drive so the sweep horizon tracks the env ceiling #2415)
• Branch HEAD: 17ec21f3 — merge of master into this branch (by eumemic, 17:40 UTC)
• Merge-base with master: 7271ae68 — identical, so the branch is fully up to date

PR #2412 already targets that master SHA. CI is running on the merge commit (lint, unit, integration, both e2e shards, plus eumemic-bot review). I did not push or change the PR.

  

[eumemic-bot]

Code review

Verdict: pass. This head (17ec21f3) is a merge of master (7271ae68, PR #2415) into the previously-reviewed 57d411ce. The substantive PR diff is byte-identical to what was reviewed at 57d411ce; the merge adds only #2415's workflow-bash timeout-pin code, which touches no file this PR touches. I re-verified the PR's own claims independently against the merged tree rather than inheriting the prior verdict.

---

Scope verified

git -C /mnt/review rev-parse HEAD → 17ec21f3003e1a671f8cbce577d1b22b7a8d4769, confirmed before reading or testing.

7 files, +944/−1. git diff 57d411ce..HEAD touches only src/aios/db/queries/workflows.py, src/aios/workflows/step.py, tests/integration/test_wf_step.py, tests/integration/test_wf_sweep.py — all from the merged master commit, disjoint from context.py / browser.py / read.py. So the merge cannot have re-broken this PR's logic by edit; the only risk was semantic interference, which the test runs below rule out.

What I checked and how

The trigger diagnosis holds. get_session_model (db/queries/sessions.py:1859) selects COALESCE(s.model, av.model, a.model) and returns it verbatim — a workflow:<id> binding included. supports_vision (vision.py:89-103) has no cache and no workflow: awareness; it calls litellm.get_model_info, which raises for that string. I confirmed the raise→None path empirically on this tree:

vision.litellm_lookup_failed error="This model isn't mapped yet. model=workflow:wf_abc..."
workflow: None

Both gates test is False (browser.py:462, read.py:271), so None fell through and inlined. Both handlers now route the model through _resolve_capability_model (loop.py:278) before the gate — the same resolver run_session_step uses at loop.py:947, so the tool path and the loop's gates key on the same string. The function-local import is genuinely required (browser → loop → tools → browser) and matches existing precedent in this tree.

The tests are load-bearing, not tautological. I reverted only the three src/ files to 57d411ce^ and re-ran the new tests on the merged tree: 5 fail, all green once restored.

FAILED test_context.py::TestVisionCapabilityReplayPass::test_text_only_model_strips_persisted_tool_result_image
FAILED test_context.py::TestVisionCapabilityReplayPass::test_text_only_model_strips_multiple_images_preserves_other_parts
FAILED test_browser_tools.py::TestScreenshot::test_workflow_binding_with_text_only_inner_model_degrades_to_text
FAILED test_browser_tools.py::TestScreenshot::test_raw_model_skips_workflow_resolution_unchanged
FAILED test_read_image.py::TestImageBranch::test_workflow_binding_with_text_only_inner_model_degrades_to_text
5 failed, 60 passed

Focused runs on the merged head (repo-wide suites left to CI per scope):

• TestVisionCapabilityReplayPass + test_browser_tools.py + test_read_image.py — 65 passed
• test_context.py + test_vision.py + test_model_binding.py — 259 passed (the adjacent clamp/mime/thinking-strip invariants and TestLazyLitellmBoundary are undisturbed by the merge)

The build-time pass respects the invariants it claims. It never mutates in place — it copies the part list, replaces the offending dicts, and rebuilds the message via {**msg, "content": new_content}, so the aliased immutable Event.data is untouched; the test asserts this with a deepcopy snapshot comparison, which is the right shape. It sits with its siblings at context.py:1813-1815 and short-circuits on falsy model. The is not False guard genuinely preserves the optimistic-inline policy, and it is consistent with can_inline_image (vision.py:171), which uses the identical is not False test — so the write-time and replay-time gates cannot disagree about the same model.

The self-heal reaches the wedged sessions. compose_step_context computes gate_model = capability_model if capability_model is not None else agent.model (step_context.py:702) and threads it into build_messages(model=gate_model) (step_context.py:744); loop.py:947 supplies the resolved value. So the worker path keys the new pass on the inner model, which is the claim that matters for the backlog.

Perf. supports_vision is uncached, so I checked the added per-build cost. Steady-state: ~0.001 ms for a catalogued model, ~0.03–0.04 ms on the raise path (first call ~0.11 ms). build_messages already unconditionally calls model_descriptor(model) a few lines later, which imports litellm at completion.py module top, so no new import cost is introduced. Negligible.

Unevaluable, and why it does not gate the verdict. The new integration test test_workflow_screenshot_vision_gate.py needs a testcontainer Postgres; this sandbox has no Docker, so I could not execute it. It does collect cleanly (3 tests, no import/signature errors). I am not treating this as UNKNOWN-blocking because CI's integration shard is green on this exact SHA (check-runs for 17ec21f3: integration success, plus unit, e2e (docker), e2e (non-docker), connectors, lint, migration-head all success). mergeable_state: unstable reflects only the review check still in progress, not a failure.

Non-blocking observations

These carry forward from the prior round unchanged — the diff did not change, and I re-confirmed each still reads as described. None blocks the merge.

1. switch_channel's recap is a third image_url-producing surface still keyed on the raw binding. switch_channel.py:194 reads queries.get_session_model and passes it unresolved into render_reorient_block(model=…), which widens the tool result to a content-parts list that is then persisted.

   Property (non-blocking): for every surface that constructs an image_url part from a session's bound model, the capability decision is made against the resolved inner model rather than the raw binding string, so no surface persists an image part a later build is guaranteed to strip.

   Non-blocking because the new build-time pass catches it before the provider — the model never sees the part, so there is no wedge. The residual cost is base64 mass written into a text-only session's log and discarded every build: waste, not a correctness defect, and reasonably out of scope for a bug-fix PR.

2. GET /v1/sessions/:id/context does not pass capability_model. api/routers/sessions.py:1036-1060 calls compose_step_context without the kwarg, so gate_model falls back to agent.model — the raw workflow: string — and the preview shows an image_url part the worker would strip. This predates the PR and the endpoint already documents known worker divergences; worth a line in that list if someone touches it.

3. parse_workflow_model raises outside _resolve_capability_model's try block, and two new call sites inherit that. loop.py:292 calls it before the try: at line 298, so a malformed binding (workflow: with empty id, workflow:wf@abc) raises BindingBoundaryError rather than degrading — and nothing in AgentCreate.model (a plain str field) or enforce_workflow_binding_privilege (a prefix probe only) rejects those. In a tool handler that error is neither ToolBail nor AiosError, so it would evict the sandbox. Reachability is essentially nil: run_session_step resolves at loop.py:947 before dispatching any tool, so such a session cannot reach a tool call at all except via an update_agent racing an in-flight background tool task. Noting it only because the docstring at loop.py:287-290 promises "never an error," and two more callers now lean on that promise.

Conventions

Docstrings match the surrounding "state the failure mode and the reason for this posture" density; the function-local imports carry their cycle rationale inline as neighbouring lazy imports do; the integration fixture's _VISION_OVERRIDES save/restore in finally matches the pattern in test_read_image.py and test_context_image_cache.py. mypy/ruff/repo-wide suites left to CI per scope — all green on this SHA.

[eumemic-bot]

Code review

Verdict: pass. Head 87c93402 is a merge of master (b3cdc118, PRs #2365/#2375/#2381/#2385) into the previously-reviewed 17ec21f3. The substantive PR diff is byte-identical to what was reviewed at 57d411ce/17ec21f3; I re-verified the PR's own claims independently against this merged tree rather than inheriting the prior verdict, and re-checked the two standing properties carried forward from those rounds.

---

Scope verified

git -C /mnt/review rev-parse HEAD → 87c93402ba4abe36b4bd9e2e59bbdcacc1eafab3, confirmed before reading or testing.

7 files, +944/−1 vs. base. git diff 17ec21f3..HEAD touches only scripts/pooled_connection_lint.py, src/aios/api/_log_redaction.py, src/aios/lanes/activate_script.py, src/aios/sandbox/secret_egress_proxy.py, src/aios/services/wake.py and their tests — all from the four merged master commits, disjoint from this PR's files. git diff 17ec21f3 HEAD -- <the 7 PR files> is empty, so the merge cannot have re-broken this PR's logic by edit; the only residual risk was semantic interference, which the runs below rule out.

What I checked and how

The trigger diagnosis holds on this tree. get_session_model returns COALESCE(s.model, av.model, a.model) verbatim — a workflow:<id> binding included. supports_vision (vision.py:73-104) has no workflow: awareness; it calls litellm.get_model_info, which raises for that string and returns None by design. Both gates test is False (browser.py:462, read.py:271), so None fell through and inlined. Both handlers now route the model through _resolve_capability_model (loop.py:278) before the gate — the same resolver run_session_step uses at loop.py:947 — so the tool path and the loop's gates key on the same string. The function-local import is genuinely required (browser → loop → tools → browser) and matches existing precedent in this tree.

The tests are load-bearing, not tautological. I reverted only the three src/ files to 57d411ce^ on this merged tree and re-ran the new tests: 5 fail, all green once restored.

FAILED test_context.py::TestVisionCapabilityReplayPass::test_text_only_model_strips_persisted_tool_result_image
FAILED test_context.py::TestVisionCapabilityReplayPass::test_text_only_model_strips_multiple_images_preserves_other_parts
FAILED test_browser_tools.py::TestScreenshot::test_workflow_binding_with_text_only_inner_model_degrades_to_text
FAILED test_browser_tools.py::TestScreenshot::test_raw_model_skips_workflow_resolution_unchanged
FAILED test_read_image.py::TestImageBranch::test_workflow_binding_with_text_only_inner_model_degrades_to_text
5 failed, 60 passed

Focused runs on the merged head (repo-wide suites left to CI per scope):

• TestVisionCapabilityReplayPass + test_browser_tools.py + test_read_image.py — 65 passed
• test_context.py + test_vision.py + test_model_binding.py — 259 passed (the adjacent clamp / mime-correction / thinking-strip invariants, TestEventDataImmutability, and TestLazyLitellmBoundary are all undisturbed by the merge)

The build-time pass respects the invariants it claims. It never mutates in place — it copies the part list, replaces the offending dicts, and rebuilds the message via {**msg, "content": new_content} — so the aliased immutable Event.data is untouched; the test asserts this with a deepcopy snapshot comparison, which is the right shape. It sits with its siblings at context.py:1813-1815 and short-circuits on falsy model. The is not False guard preserves the optimistic-inline policy and is identical to the write-time test in can_inline_image (vision.py:171), so the write-time and replay-time gates cannot disagree about the same model.

Degenerate shape checked directly. I ran the case the unit tests do not cover — an image-only tool result with no sibling text part — against the real build_messages on this tree. It yields [{'type': 'text', 'text': '[image omitted: …]'}]: a non-empty list content, so no empty-content 400 and no orphaned tool message. Good.

The self-heal reaches the wedged sessions. compose_step_context computes gate_model = capability_model if capability_model is not None else agent.model (step_context.py:702) and threads it into build_messages(model=gate_model) (step_context.py:744); loop.py:947 supplies the resolved value. So the worker path keys the new pass on the inner model, which is the claim that matters for the backlog.

Unevaluable, and why it does not gate the verdict. The new integration test test_workflow_screenshot_vision_gate.py needs a testcontainer Postgres; this sandbox has no Docker (docker info fails), so I could not execute it. It collects cleanly (3 tests, no import/signature errors). I am not treating this as UNKNOWN-blocking because CI's integration shard is green on this exact SHA — check-runs for 87c93402: integration, unit, e2e (docker), e2e (non-docker), connectors, lint, migration-head, detect all success; only review is still in_progress, which is what mergeable_state: unstable reflects.

Standing properties re-checked

Both carry forward from the prior rounds; I re-verified each against this head rather than inheriting the earlier assessment. Neither is newly violated, and neither blocks.

1. "Every surface that constructs an image_url part from a session's bound model decides capability against the resolved inner model, not the raw binding string." Still not fully satisfied: switch_channel.py:194 reads queries.get_session_model and passes it unresolved into render_reorient_block(model=…), which can widen the recap into a content-parts list that is then persisted. I confirmed by enumerating all get_session_model( call sites on this head — services/sessions.py:1344 (the service wrapper), browser.py:445 and read.py:228 (both now resolved), and switch_channel.py:194 (still raw). Non-blocking for the same reason as before, and I re-confirmed the reason rather than assuming it: the new build-time pass strips such a part before the provider sees it, so no wedge results. The residual cost is base64 mass written into a text-only session's event log and discarded on every subsequent build — waste, not a correctness defect, and out of scope for a bug-fix PR.

2. GET /v1/sessions/:id/context does not pass capability_model. Re-confirmed at api/routers/sessions.py:1062-1071 — the call omits the kwarg, so gate_model falls back to agent.model (the raw workflow: string), supports_vision returns None, and the preview shows an image_url part the worker would strip. Predates the PR; the docstring at sessions.py:988-1001 already enumerates known worker divergences and would be the place to note this if someone touches it.

3. parse_workflow_model raises outside _resolve_capability_model's try block, and two new call sites inherit that. loop.py:292 calls it before the try: at line 298, so a malformed binding (workflow: with empty id, workflow:wf@abc) raises BindingBoundaryError rather than degrading — and nothing in AgentCreate.model or enforce_workflow_binding_privilege rejects those. In a tool handler that error is neither ToolBail nor AiosError, so it would evict the sandbox. Reachability is essentially nil: run_session_step resolves at loop.py:947 before dispatching any tool, so such a session cannot reach a tool call except via an update_agent racing an in-flight background tool task. Noted only because the docstring at loop.py:287-290 promises "never an error," and two more callers now lean on that promise.

Conventions

Docstrings match the surrounding "state the failure mode and the reason for this posture" density; the function-local imports carry their cycle rationale inline as neighbouring lazy imports do; the integration fixture's _VISION_OVERRIDES save/restore in finally matches the pattern in test_read_image.py and test_context_image_cache.py. mypy/ruff/repo-wide suites left to CI per scope — all green on this SHA.

[eumemic]

Labelled needs:human/merge-approval. It had no gate label, so it sat in no queue for two days despite carrying three passing reviews.

Review state: three PASS verdicts, none inherited

What makes this PR unusually well-reviewed is that the reviewer refused to inherit its own prior verdicts across master merges:

• 57d411ce — pass, three non-blocking observations.
• 17ec21f3 — pass, re-verified independently against the merged tree.
• 87c93402 — pass, re-verified again, and re-checked the two standing properties carried forward.

That is the correct discipline: a green earned against an older tree is a stale green, and the reviewer explicitly re-derived rather than assuming the merge was inert.

The gap I found, stated precisely

The live head is 2fdb0261. The newest verdict covers 87c93402. A fourth master merge landed at 19:14:06, eight minutes after the last review at 19:06:17. No review covers the current head.

I checked what that unreviewed merge actually contains — 87c93402...2fdb0261 is 3 commits touching 9 files:

src/aios/config.py                    src/aios/services/sessions.py
src/aios/db/queries/sandboxes.py      tests/… (5 files)
src/aios/sandbox/backends/docker.py

The PR itself touches 7 files: harness/context.py, tools/browser.py, tools/read.py, and four test files.

Overlap: NONE. The unreviewed merge and the PR modify entirely disjoint file sets.

What that does and does not establish

It is good evidence the verdict carries — the same reasoning the reviewer applied at rounds 2 and 3, and it held both times. But I want to be exact about its strength: "no file overlap" is weaker than "re-verified against this tree." Disjoint files can still interact through shared behaviour, import-time effects, or a changed config default. src/aios/config.py is in the merge, and config changes are precisely the kind of thing that reaches code that does not import it.

So this is not a green at the live head; it is a strong prior that the green survives. Under the standing rule, merge requires an uncorrelated review green at the live head and CI green at that head. The honest state is: two rounds of evidence say the merge is inert, and nobody has actually checked.

Recommendation

Cheapest correct path is a re-verify of 2fdb0261 — likely a fast confirmation given the disjointness — rather than either merging on a stale green or discarding three good reviews. mergeable_state is behind, so it needs a rebase regardless, which will move the head again and require that re-verification anyway.

Both facts point the same way: one more verification round at whatever head the rebase produces, then merge. Blocked on the aios#2396 freeze until then.

[eumemic-bot]

Code review

Verdict: pass. Head a3c72919 is a fourth master merge (01f76bb8, PRs #2369/#2414/#2377/#2367) into the previously-reviewed 87c93402. The substantive PR diff is byte-identical to what was reviewed at 57d411ce/17ec21f3/87c93402, but per the triage note on this PR I re-derived the verdict against this tree rather than inheriting it — including re-running the revert-and-fail check and re-checking the three standing properties.

---

Scope verified

git -C /mnt/review rev-parse HEAD → a3c72919a6efa98f9a88511041958fe1df691b25, confirmed before reading or testing.

7 files, +944/−1 vs. base. git diff 87c93402..HEAD touches src/aios/config.py, src/aios/db/queries/inbound_grants.py, src/aios/db/queries/sandboxes.py, src/aios/sandbox/backends/docker.py, src/aios/sandbox/volumes.py, src/aios/services/sessions.py and 9 test files — all from the four merged master commits. git diff 87c93402 HEAD -- <the 7 PR files> is empty, so the merge cannot have re-broken this PR's logic by edit.

The triage comment correctly flagged that "disjoint files" is weaker evidence than re-verification, and named config.py as the specific worry. I read that hunk rather than assuming: it deletes sandbox_snapshot_throughput_ewma_alpha / sandbox_snapshot_throughput_state_path and rewords one description — snapshot-timeout settings only, no default reachable from the vision/context path. services/sessions.py adds a normalized_workspace_path call inside clone_session and touches nothing on get_session_model's path. Semantic interference is further ruled out by the runs below, which execute the PR's logic on this exact tree.

What I checked and how

The trigger diagnosis holds on this tree. get_session_model returns COALESCE(s.model, av.model, a.model) verbatim — a workflow:<id> binding included. supports_vision (vision.py:73-104) has no workflow: awareness; it calls litellm.get_model_info, which raises for that string and returns None by design. I saw the raise→None path fire live during the revert run on this tree:

vision.litellm_lookup_failed error="This model isn't mapped yet. model=workflow:wf_text_only…"

Both gates test is False (browser.py:462, read.py:271), so None fell through and inlined. Both handlers now route the model through _resolve_capability_model (loop.py:278) before the gate — the same resolver run_session_step uses at loop.py:947 — so the tool path and the loop's gates key on the same string. The function-local import is genuinely required (browser → loop → tools → browser) and matches existing lazy-import precedent in this tree.

The tests are load-bearing, not tautological. I reverted only the three src/ files to the base commit (01f76bb8) on this merged tree and re-ran the new tests: 5 fail; all 65 green once restored.

FAILED test_context.py::TestVisionCapabilityReplayPass::test_text_only_model_strips_persisted_tool_result_image
FAILED test_context.py::TestVisionCapabilityReplayPass::test_text_only_model_strips_multiple_images_preserves_other_parts
FAILED test_browser_tools.py::TestScreenshot::test_workflow_binding_with_text_only_inner_model_degrades_to_text
FAILED test_browser_tools.py::TestScreenshot::test_raw_model_skips_workflow_resolution_unchanged
FAILED test_read_image.py::TestImageBranch::test_workflow_binding_with_text_only_inner_model_degrades_to_text
5 failed, 60 passed

Focused runs on this head (repo-wide suites left to CI per scope):

• TestVisionCapabilityReplayPass + test_browser_tools.py + test_read_image.py — 65 passed
• test_context.py + test_vision.py + test_model_binding.py + test_switch_channel.py — 278 passed (the adjacent clamp / mime-correction / thinking-strip invariants, TestEventDataImmutability, and TestLazyLitellmBoundary are undisturbed by this merge)

Working tree restored to a clean a3c72919 after the revert experiment (git status --porcelain empty).

The build-time pass respects the invariants it claims. It copies the part list, replaces the offending dicts, and rebuilds the message via {**msg, "content": new_content} — never mutating the aliased immutable Event.data; the unit test asserts this with a deepcopy snapshot comparison, which is the right shape. It sits with its siblings at context.py:1813-1815 and short-circuits on falsy model. The is not False guard is identical to the write-time test in can_inline_image (vision.py:171), so the write-time and replay-time gates cannot disagree about the same model.

Degenerate shape re-checked directly on this tree. An image-only tool result with no sibling text part — not covered by the unit tests — yields [{'type': 'text', 'text': '[image omitted: …]'}]: non-empty list content, so no empty-content 400 and no orphaned tool message.

The self-heal reaches the wedged sessions. compose_step_context computes gate_model = capability_model if capability_model is not None else agent.model (step_context.py:702) and threads it into build_messages(model=gate_model) (step_context.py:744); loop.py:947 supplies the resolved value. The worker path therefore keys the new pass on the inner model, which is the claim that matters for the backlog.

Unevaluable, and why it does not gate the verdict. tests/integration/test_workflow_screenshot_vision_gate.py needs a testcontainer Postgres; this sandbox has no Docker (docker info fails), so I could not execute it. It collects cleanly (3 tests, no import/signature errors). CI's integration shard is still in_progress on this SHA at review time — unit, lint, connectors, detect, migration-head are success; integration, e2e (docker), e2e (non-docker) are running. I am recording that honestly as UNKNOWN, not green: the integration test's claims are a strict superset of unit claims I verified directly against the real build_messages and the real handlers, and the same test passed on the prior three heads whose PR-file content is byte-identical, so the residual risk is confined to the DB-resolution seam. The launcher/merge gate should still require CI green at this head before merging — this review does not substitute for it.

Standing properties re-checked

All three carry forward from earlier rounds. Re-verified against this head rather than inherited; none newly violated, none blocking.

1. "Every surface that constructs an image_url part from a session's bound model decides capability against the resolved inner model, not the raw binding string." Still not fully satisfied. I re-enumerated get_session_model( call sites on this head: services/sessions.py:1344 (wrapper), browser.py:445 and read.py:228 (both now resolved), and switch_channel.py:194 — still raw, passed unresolved into render_reorient_block(model=…), which can widen the recap into a persisted content-parts list. Non-blocking, and I re-confirmed the reason rather than assuming it: the new build-time pass strips such a part before the provider sees it, so no wedge results. The residual cost is base64 mass written into a text-only session's event log and discarded on every build — waste, not a correctness defect, and out of scope for a bug-fix PR.

2. GET /v1/sessions/:id/context does not pass capability_model. Re-confirmed at api/routers/sessions.py:1062-1073 — the call omits the kwarg, so gate_model falls back to agent.model (the raw workflow: string), supports_vision returns None, and the preview shows an image_url part the worker would strip. Predates the PR; the endpoint's docstring already enumerates known worker divergences and would be the place to note this.

3. parse_workflow_model raises outside _resolve_capability_model's try block, and two new call sites inherit that. Re-confirmed: loop.py:292 calls it before the try: at line 298, so a malformed binding (workflow: with empty id, workflow:wf@abc) raises BindingBoundaryError rather than degrading, and in a tool handler that error is neither ToolBail nor AiosError, so it would evict the sandbox. Reachability remains essentially nil — run_session_step resolves at loop.py:947 before dispatching any tool. Noted only because the docstring at loop.py:287-290 promises "never an error," and two more callers now lean on that promise.

Conventions

Docstrings match the surrounding "state the failure mode and the reason for this posture" density; the function-local imports carry their cycle rationale inline as neighbouring lazy imports do; the integration fixture's _VISION_OVERRIDES save/restore in finally matches the pattern in test_read_image.py and test_context_image_cache.py. mypy/ruff/repo-wide suites left to CI per scope — lint and unit are green on this SHA.