ENG 696: structured result contract for side-effecting tools

[ea-rus]

What

Side-effecting tools returned prose or ad-hoc JSON, so a consumer (evals, monitoring, the model, a supervisor reconciling an ambiguous trace) could not reliably tell what committed, where, and whether a retry would duplicate it.

Adds a single SideEffectResult envelope (core/tools/side_effect.py) that every such tool returns:
success, resource_id, external_url, idempotency_key, committed_at, content_hash, message. It renders to a ToolOutcome and sets ok=success, so the ENG-1276 error streak keys on the explicit verdict.

Migrated create_artifact, update_artifact, launch_backend. Failures use SideEffectResult.failed(...) (nothing committed → committed_at=None). launch_backend's description updated to match the new return shape.

Notes

• content_hash populated only where committed content exists (publish, in the cowork-server PR); None for create/update/launch by design — hashing an empty scaffold would be false identity.
• Failures now correctly count toward the resilience nudge / circuit breaker; previously prose like PUBLISH FAILED (uppercase) missed the legacy substring markers and silently reset the streak.

Tests

tests/test_side_effect_result.py (new). Full suite green (test_tool_outcome_tracking + 460 session/turn tests).

Fixes https://linear.app/mindsdb/issue/ENG-696/side-effect-tool-contract-ambiguity

[alecantu7]

Adversarial review (deep) — head 0fc7c624, base origin/staging d70e8803

Reviewed together with cowork-server#286. Deep fan-out: 7 finders across the flagged dimensions, every finding put to 1–3 agents prompted to refute it. 20 findings raised, 8 survived.

The contract itself is sound and no defect breaks a user today. The risk I went in expecting — a JSON blob reaching something that string-matched the old prose — did not materialise: there is no prose-parsing consumer in the renderer, cowork_evals or mdb-ai, verified by executing the real dispatch path. Two findings below, both medium, plus a delivery note.

---

1. MEDIUM — every JSON envelope normalises to one identical ACC signature, so three different failures fire a false "repeated error" lesson

confirmed · reproduced end-to-end through the real detector · introduced by this PR

anton/core/memory/acc.py:238-252 _normalise_error_signature is unchanged by this PR — but its input shape is not. It collapses every quoted run ≤80 chars:

s = re.sub(r"'[^']{1,80}'", "'X'", s)
s = re.sub(r'"[^"]{1,80}"', '"X"', s)

JSON is almost entirely quoted runs. Reproduced in the pr-319 worktree over the three real failure messages of the three migrated tools:

=== PROSE (staging shape) ===                        distinct: 3
'Error: `name` is required.'
'Error: no artifact found for slug `foo`.'
'PUBLISH FAILED: settings module unavailable'

=== ENVELOPE (post-PR) ===                           distinct: 1
'{ "X": false, "X": "X", "X": null, "X": null, … }'   (x3)

The producer change is the whole cause: to_outcome() returns json.dumps(payload, …), and session.py:3780-3786 feeds it straight in as detail["error"]. Threshold is 3 (acc.py:212) and it is on by default (session.py:818 ANTON_ACC_MODE defaults "active"; cowork-server user_settings.py:448-458 defaults memory_mode="autopilot"). End-to-end through the real AnteriorCingulate: prose baseline → [], envelope → ['detect_repeated_error_signature']. Long messages don't escape — the 148-char ARTIFACT_TYPES message still collapses, because the single-quote pass shrinks the double-quoted run under 80 first.

Impact. Three unrelated failures in one turn inject "the same error message appears repeatedly… retrying won't help" into tool_results (session.py:2288-2295) and persist it as a cortex Lesson — telling the model the cause hasn't changed when each attempt failed differently. Bounded (one nudge per turn, one lesson via _acc_has_similar, true positives still fire), so it is a false-positive-rate regression rather than destroyed discrimination. tests/test_acc.py:196-202 test_silent_when_errors_are_distinct encodes exactly the broken invariant and stays green only because it hand-writes prose.

Worth noting the discriminator already exists and is discarded: ToolOutcome.reason is set distinctly on every failure path (missing_name, artifact_not_found, invalid_port) but is never plumbed into the ACC event.

Smallest fix. At session.py:3780-3786, when the outcome is an envelope, pass the human message (or outcome.reason) as detail["error"] rather than the raw JSON — outcome.reason or extracted_message is a one-line preference change.

Fails before / passes after:

def test_distinct_envelope_failures_stay_distinct():
    sigs = {_normalise_error_signature(SideEffectResult.failed(m).content[:300])
            for m in ["Error: `name` is required.",
                      "Error: no artifact found for slug `foo`.",
                      "PUBLISH FAILED: settings module unavailable"]}
    assert len(sigs) == 3

Fails on HEAD today (len == 1, verified), passes once the message is extracted.

---

2. MEDIUM — two of the three migrated handlers have no tests at all

confirmed · mutation-verified · the gap is pre-existing, the contract on top of it is new

Both signatures changed and both gained new semantics:

tool_handlers.py:141  async def handle_update_artifact_metadata(session, tc_input) -> ToolOutcome:
tool_handlers.py:235  async def handle_launch_backend(session, tc_input) -> ToolOutcome:

tests/test_side_effect_result.py imports one handler — every reference is handle_create_artifact (:8,65,85,95). Nothing anywhere touches the other two, and there is no type-checker (dev group is pytest + pytest-asyncio; tests.yml runs only pytest). The dispatch seam actively hides a regression: registry.py:142-144 coerces a stray str back into ToolOutcome(content=result) with ok=None — no crash, no signal.

Mutation: injecting return "MUT prose …" at the top of both handlers gives 1936 passed, 28 skipped (unit) and 39 passed (e2e). Both handlers fully gutted, suite green.

Impact is bounded — a direct probe confirms both are correct today (happy path → ok=True, resource_id="my-art", details.port=8080; not-found → ok=False, reason="artifact_not_found"), and git grep finds no consumer of external_url / resource_id / idempotency_key / committed_at / content_hash in cowork, cowork-server, cowork_evals or mdb-ai. Unguarded-but-correct, so cheap hardening rather than a blocker.

Smallest fix. Three cases per handler mirroring the create_artifact tests already in the file: not-found → ok is False + committed_at is None; validation reject → ok is False with the expected reason; success → ok is True, resource_id == slug, and for launch_backend external_url == url with details carrying port/pid/log_path. ~15 lines each. Each fails against the corresponding return "MUT prose …" mutant (.ok raises AttributeError on a str).

---

3. Delivery note — state the merge order in this PR's body

cowork-server#286's body says "Should be merged after #319"; this one says nothing. Worth adding, because the dependency is sharper than it looks: cowork-server pins anton at branch = "main" and its uv.lock resolves a specific sha, so #286's envelope path stays dead until #319 reaches anton main, not staging. Desktop staging builds pick it up earlier (ANTON_REF), hosted/Docker not at all until the lock moves.

Consequence for QA: "merge #319, then verify the envelope" against the wrong channel will show prose and read as a failed fix. /health returns anton_version — free signal for Step 0.

---

Checked and produced nothing

• Prose consumers across all repos. anton emits tool content only for scratchpad (session.py:3552, :3699, both name-guarded); grep for every removed prose string across cowork, cowork_evals and mdb-ai → zero hits; an executed probe confirmed the envelope never enters the SSE stream. Worth one line in the body so nobody re-litigates it.
• create_artifact's idempotency_key is unstable across identical calls. Raised as high, then refuted on three independent grounds: store.py is untouched by this PR (git diff empty), staging already exposed the same suffixed slug, tool_results are never re-executed on retry (session.py:2993), and no consumer reads the field. Residual is a docstring over-promise at side_effect.py:14-16 — "a re-run with the same key targets the same resource" is unachievable for a tool whose schema has no key parameter. One-line doc nit.
• "JSON stops parsing when the resilience nudge is appended." Real mechanism, pre-existing, and not in this diff — session.py is byte-identical to staging (md5 confirmed), and staging's select_path _status() JSON already gets the nudge glued to its closing brace. Success envelopes are never suffixed (the streak resets on ok=True). Separate ticket if wanted.
• "Validation errors now get scrape/fetch resilience advice." Already adjudicated in-tree: session.py:952-955 states "every non-scratchpad tool keeps the generic nudge" and tests/test_resilience_nudge.py:23 asserts it.
• launch_backend reports committed_at: null after reaping the previous backend. backend_launcher.py is byte-identical to staging and the behaviour reproduces there unchanged. This PR improves the signal — two of the three post-reap error strings previously matched no legacy marker and reset the streak.

Recommendation: approvable with a follow-up for the ACC extraction and the two handler tests. The higher-value item on this ticket is on cowork-server#286.

[alecantu7]

Approving — see the deep review above for the evidence.

No defects. The prose→JSON switch has no consumer anywhere that string-matched the old format (verified by executing the real dispatch path, plus a grep for every removed prose string across cowork, cowork_evals and mdb-ai — zero hits).

Two follow-ups, neither blocking:

• the ACC error-signature collapse (session.py:3780 — pass outcome.reason or the human message rather than the raw JSON)
• tests for handle_update_artifact_metadata and handle_launch_backend, which are correct but unguarded

Worth adding the merge order to the body: cowork-server#286 depends on this reaching anton main, not staging.