feat(workspace): an answer given in the Workspace resumes the agent (ent#430)

[dolho]

Journey Impact: extends: J05

Closes abilityai/trinity-enterprise#430

The gate

Slice 5 of ent#364, and the one that makes the rest true. Until now the client route recorded an answer and returned:

# routers/operator_queue.py:262      — operator answers → agent resumes ✓
operator_resume_service.spawn_resume_dispatch(item, response=..., responded_by_email=...)

# client_portal/asks/service.py::answer_ask
updated = db.respond_to_operator_queue_item(...)      # …and returned ✗

So an ask addressed to a Workspace client — the entire point of ent#364/#428/#429 — was recorded, reached the agent's queue file, and re-triggered nothing.

Measured on a live instance before this change: answered from the Workspace, operator-queue.json flipped to responded carrying the answer in under 3 seconds, and no execution followed.

Unblocked because ent#329 is now in dev.

What this adds: one call

ent#430's body rules out the alternative — "a second dispatch surface for the same event is how the cost, trigger-label and loop-prevention questions get answered twice, differently."

So the per-agent opt-in, the idempotency key, the audit row and the failure handling all stay inside maybe_dispatch_resume. AC #2 and AC #3 are satisfied by reuse, not re-implementation — which is why the tests assert the call rather than re-testing what it does.

Four properties, each load-bearing

• CAS win only, like the operator route. The 409 above already returned for a lost race, so reaching the dispatch means this answer is the one that landed — two people answering at once produce one resume.
• updated, never item. The pre-answer read still says pending; a resume handed that row acts on an ask that doesn't yet carry its answer. Looks identical in a green test, so there's one for it.
• The spawn is wrapped. It's fire-and-forget, but a raise on the calling line would still propagate — and a 500 after the CAS landed would tell the client their answer failed while it's committed and already on its way to the agent.
• bug: POST /api/operator-queue/{id}/respond accepts any string as an approval decision — no layer checks response ∈ options #2376's validator runs first, so an answer that was never offered cannot spend.

AC #5 — and its honest limit

resume_requested rides the answer response, read from the same accessor the dispatch gates on so the two can't disagree. It reports intent, not success: the dispatch is backgrounded, so the only honest claim at that moment is whether it will be attempted. Fails closed — an unreadable flag claims nothing, because over-claiming is exactly what AC #5 forbids.

Residual, stated rather than implied: a dispatch that fails after this point surfaces as a FAILED execution row plus an operator_resume_dispatch audit entry (ent#329) — both operator-visible, and a client can see neither.

The client half of AC #5 is currently satisfied negatively: the ask surface says nothing about work starting, so it cannot mis-claim. resume_requested is the field a surface needs to say something true; consuming it is an ent#429 UI change and is deliberately not here.

The flag default is unchanged

operator_resume_enabled stays OFF, per-agent, owner-only.

"Turn the flag on" is an operator action per agent, not a code default. Flipping the default would hand every shared agent's client a spend button — the one thing AC #3 rules out ("so hosting asks does not hand a client a spend button").

Verification

pytest -k "ent430 or asks or ent428 or ent429 or ent364 or 2376 or ent329 or operator_resume"
  → 145 passed, 1 skipped
tests/unit/test_ent430_workspace_answer_resumes.py  → 10 passed

Mutation-checked:

Mutation	Result
Remove the dispatch call	4 failed
Pass the pre-answer row	1 failed
Make the opt-in read fail open	1 failed

ACs

• An answer given in the Workspace reaches the agent and it resumes
• Dispatch goes through ent#329's path only; no workspace-specific execution surface
• The opt-in is per-agent, not per-answer — unchanged, and confirmed as already built that way
• The feature flag flips ON only when this passes end-to-end — deliberately left to an operator per agent; see above
• A failed dispatch is visible as such (operator-side; client half noted as a residual)

[obasilakis]

The structure is right and that is the hard part: one call into operator_resume_service, no second dispatch surface, no re-decision of the opt-in, the idempotency key, the audit row or the failure handling. AC #2 is genuinely satisfied by reuse, and the CAS-win framing in the comment at asks/service.py:216-231 is the correct rule.

The wiring is what does not work. Three findings, all verified by direct read of 63f3456a, and all invisible to the 24 green checks because every test monkeypatches the one call whose runtime context is the defect.

Blocking

1. The dispatch structurally cannot run — the feature is inert in production

client_portal/asks/router.py:53 declares the endpoint as a sync def:

@router.post("/{item_id}/answer", response_model=WorkspaceAsk)
def answer_ask(...):

FastAPI runs a non-coroutine endpoint through run_in_threadpool, i.e. a worker thread with no running event loop. spawn_resume_dispatch (services/operator_resume_service.py:215) is asyncio.create_task(...), which calls get_running_loop() and raises RuntimeError: no running event loop there.

The try/except Exception you added at asks/service.py:232-244 then swallows it and logs. So on every client answer the answer is recorded, logger.exception fires, and nothing is dispatched — which is byte-for-byte the behaviour this PR exists to remove. AC #1 is not met.

The operator route works only because routers/operator_queue.py:199 is async def. main.py's mount of portal_asks_router is plain — no custom route_class — so standard dispatch applies, and service.answer_ask has exactly one production caller.

Note this is not a one-line flip to async def: answer_ask does blocking DB I/O at service.py:153 and :193, so that alone moves those onto the loop. It needs either a loop handle captured at request time (anyio.from_thread, or run_coroutine_threadsafe against the main loop) or an async def route that runs the sync service via run_in_threadpool and spawns afterwards.

2. A lost race dispatches a paid execution for an answer that was never recorded — and under a different idempotency key

db/operator_queue.py:389-404: respond_to_operator_queue_item returns None only when the row does not exist. When the row exists but has left pending — the actual lost race — it returns a truthy dict carrying _status_conflict = True, with no write performed.

asks/service.py:200 checks only if not updated: and never pops that flag. The operator route does, at routers/operator_queue.py:229, before the spawn at :240. So on this path the race loser falls through and dispatches.

Two consequences, the second worse than the first:

• money is spent on an answer that is not in the database, and the framed message carries the losing answer as though it were the recorded one;
• the key is operator_resume:{item_id}:{sha256(item_id, response, response_text)} (operator_resume_service.py:80-90), so the loser's differing text yields a different digest and two dispatches fire for one queue item. The body's "two people answering at once produce one resume" does not hold here.

ent#329's own suite already encodes this rule — tests/unit/test_ent329_operator_resume.py:367-379 asserts _status_conflict is consumed before spawn_resume_dispatch — but it hardcodes routers/operator_queue.py, so a second dispatch site is outside its reach. Worth extending that guard to enumerate all callers so the next site inherits the rule instead of re-losing it.

Before this PR the missing check was a wrong 200. This PR converts it into a spend.

3. resume_requested: true is returned when the spawn failed, which is the AC #5 over-claim

asks/service.py:249 computes resume_requested after the swallowed spawn and reads only db.get_operator_resume_enabled. The except branch sets nothing, so a spawn that raised still answers true.

The docstring at :252-264 says it "Fails CLOSED … over-claiming is precisely the failure AC #5 names — an ask that reads as acted upon while nothing happened." That is the right rule; the code does not implement it for the failure path the same commit introduces. Given finding 1, this is not an edge case — it is every production answer on an opted-in agent. Gate the flag on the spawn actually having been scheduled.

Tests

The reason all three survive a fully green CI is that the tests stub the defect out:

• every test replaces spawn_resume_dispatch with a synchronous lambda (:56-59, :113-114, :211);
• test_a_lost_race_dispatches_nothing:94-121 stubs respond_to_operator_queue_item to return None (:106-107), which is "item does not exist", not the lost race. The _status_conflict truthy-dict shape — the one that actually occurs — is untested, so the docstring's claim to prove "two people answering at once must not produce two resumes" is not supported;
• test_a_dispatch_failure_never_loses_the_answer:130-158 uses RuntimeError("event loop is closed") at :153 as its fixture. That is the production failure mode, asserted as an acceptable degraded path rather than caught as the bug.

A test that drives service.answer_ask through anyio.to_thread.run_sync (or a TestClient POST) with the real spawn_resume_dispatch fails today, and is the test this change most needs.

Non-blocking

• Closes ent#430 does not resolve — it names no repository, renders as plain text rather than a link, and does not match the trigger regex in .github/workflows/issue-status-on-merge.yml. Use Closes abilityai/trinity-enterprise#430. Cross-repo still needs a manual status-in-dev bump on ent#430 at merge either way.
• docs/memory/architecture.md:1246 — the "Respond → Resume Dispatch (ent#329)" section describes a single caller and states the CAS-win property that finding 2 breaks on the new one; the "Addressed asks (ent#364)" paragraph says answering "answers through the OSS respond path" with no mention of resume dispatch. WorkspaceAsk also gains a client-visible field (models.py:29-34). Per the tiered rule one of architecture or a flow doc needs a paragraph — and there is still no feature-flows/workspace-asks.md for the ent#364 chain at all.
• service.py:267 and operator_resume_service.py:118 are separate reads of the opt-in separated by a task hop. The docstring's "Read from the SAME accessor … so the two cannot disagree" is true of the accessor and not of the instant; they can disagree, and it costs an extra read per answer.
• service.py:74 — _project maps status to pending/expired only, so the response to a just-answered ask returns status: "pending" beside resume_requested: true. Pre-existing from ent#428, but the new field makes the pairing actively misleading, and the test at :158 papers over it with assert out.status in ("pending", "expired").
• models.py:34 — resume_requested on the shared WorkspaceAsk means GET /asks now serializes "resume_requested": null on every row. Deliberate per the comment; a distinct answer-response model, or response_model_exclude_none, would keep the list contract unchanged.
• Once finding 1 is fixed, keep the logger.exception at :242. As written today it fires on every single client answer.
• AC #4 is unverified end to end. The only live-instance measurement in the body is pre-change, and per finding 1 a post-change one could not have succeeded. Deferring the flag flip to an operator is reasonable on its own terms, but nothing here demonstrates the path works.

Happy to re-review as soon as the dispatch actually runs from this route.

[dolho]

Tested against a live local instance — and it found a blocker for this very PR

Booted a real backend from dev (real uvicorn, real Redis, real SQLite), signed in as a genuine external Workspace client via the OTP flow, and drove the surfaces with curl.

🔴 maybe_dispatch_resume has never run. This PR depended on it.

ent#430 wires a Workspace answer to spawn_resume_dispatch. That path was dead on arrival — the client answer would have hit exactly the wall the operator path has been hitting since ent#329 merged:

from services.task_execution_service import task_execution_service   # never existed

The module exports get_task_execution_service(). The import is on the first line of the function, above the try, so every dispatch raised ImportError before it even read the opt-in.

Why it stayed invisible, twice over:

1. The call is fire-and-forget, so the traceback surfaces only as asyncio's Task exception was never retrieved. Nothing 500s, the answer is recorded, the config audit row is written. It looks like it worked.
2. The ent#329 unit test stubbed the module as SimpleNamespace(task_execution_service=recorder) — manufacturing the very symbol whose absence was the bug. 21 tests green, feature dead.

Measured on the live instance, opt-in ON:

	before	after
answer / respond	200	200
executions created	0	1
triggered_by	—	operator_response
operator_resume_dispatch audit	absent	written, with the execution id
ImportError in log	present	0

The dispatched run then FAILED on a missing AGENT_AUTH_SECRET — a limitation of the test box, and correctly recorded as an honest FAILED row, which is ent#329's "never silent" requirement doing its job.

Fixed in 53bc8601, with a guard that parses the real module source (ast, never the stubbed sys.modules entry — that's what made this invisible) and asserts every from services.X import Y resolves. Mutation-checked: reverting the import turns 11 tests red.

---

Everything else verified on the same instance

feature	result
ent#428/429 asks	Client sees exactly their addressed ask, not the operator's; context not in the projection ✓
ent#364 addressed asks	addressed_to_email persisted and roster-validated ✓
ent#329 switch	GET default false; PUT true; portal token on the operator route → 401 ✓
ent#430 client path	Answer recorded, responded_by_email set, status responded ✓ — dispatch now fires with 53bc8601
ent#366 ratings	Unknown target → 404 (checked against the reader, not global ids) ✓
ent#365 deliverables	Portal /reports → 200 ✓
ent#155 cancel	Foreign execution → 404 uniform ✓
ent#457 agent page	200 with asks/ratings/recent_work/stats; work rows carry no agent-authored fields — no context, payload, execution_log or response ✓
#2128 capability channel	multi_agent_chat_available: true on the roster payload (rooms are OSS-core since ent#443) ✓
#2196 availability	availability: "unavailable" for a containerless agent — the row still appears, which is the point ✓

Skipped, honestly

• ent#438 (merge + canvas) and ent#425 (hosted deliverable pages) are not on dev — no code to test.
• ent#440 (voice conversation) is present but needs a browser + microphone; the backend /stt and /tts gates were reachable, the loop itself was not exercised.
• ent#253 and ent#458 are frontend-dominant; I verified their backend surfaces only.
• Anything behind requires_entitlement was untestable here: this is an OSS-only build (edition: oss, enterprise_features: []) — the private submodule isn't mounted.
• No live agent container, so no real turn ran end to end. That is the honest ceiling of this pass.

[dolho]

/review — fresh pass on ced40543

Branch: feature/ent430-workspace-answer-resumes → dev · +790/-11 across 7 files
Scope: CLEAN
Base: merge-base 8d9af18b

Both earlier passes' blockers are closed and I re-verified the two that depend on facts outside the diff rather than re-reading the comments:

• The lost-race shape is real. db/operator_queue.py:404 sets item["_status_conflict"] = True on a truthy row, and routers/operator_queue.py:250 pops it exactly this way — so if not updated or updated.pop("_status_conflict", False) is the established rule, not a new one, and popping keeps the sentinel out of _project.
• _status_of cannot disturb the listing. list_asks queries status="pending", so answered is unreachable from it; and clientPortal.js:1245 removes the answered row (this.asks.filter(...)) rather than splicing the response back, so openAsks is unaffected in either direction.

One finding, and I have not fixed it because the fix is a UI decision rather than a mechanical one.

---

I1 — resume_requested is rendered nowhere (Confidence 10/10)

$ grep -rn "resume_requested\|resumeRequested" src/frontend/src/
(no matches)

PortalAsks.vue:127 is the only caller:

await store.answerAsk(ask.id, { response, responseText: text || null })

The return value is discarded, and clientPortal.js:1249 has already dropped the ask from state.asks. So a client answering an ask on an opted-in agent sees exactly what they would see with the flag off: the ask disappears. Nothing tells them work started.

That matters more than a normally unused field would, because review pass 2 spent a blocker on this field over-claiming — an important correction to a value that currently has no consumer.

A correction to the PR body while I am here: there is no "AC #5" in ent#430. Its acceptance criteria are four, and the fifth-sounding reference is to ent#364 AC #5 — "The answer reaches the agent and it resumes — including an agent with no scheduled next turn" — which mandates the behaviour, not the field. So resume_requested is a self-imposed extra, and there are two honest resolutions:

1. Render it. One line in PortalAsks.vue after a successful answer — "Sent. is picking this up." vs plain "Sent." — is what makes ent#364 AC Setup improvements #5 visible rather than merely true. The row vanishes on answer, so the confirmation needs a home; that is the actual design question.
2. Drop it. The behaviour is the deliverable and it is delivered; a field on the wire that nothing reads is weight, and GET /asks now serialises "resume_requested": null on every row for it.

Either is defensible. Shipping it unrendered is the one option that is not, and it is not my call to pick — flagging rather than fixing.

I2 — the thread-hop wrapper can misattribute a failure (Confidence 7/10)

try:
    _run_sync_in_loop(_schedule_on_loop, item, kwargs)
except RuntimeError as e:
    raise RuntimeError(
        "resume dispatch could not be scheduled: called from a thread "
        f"with neither a running loop nor an anyio portal ({e})"
    ) from e

run_sync propagates the callable's own exception, so a RuntimeError raised inside _schedule_on_loop on the loop thread is re-reported as "no anyio portal" — a false diagnosis in the log for the one failure mode this code exists to make visible. Narrow (nothing in _schedule_on_loop raises RuntimeError today), but the cost of getting it wrong is a misleading incident. Catching only the portal-absent case, or naming the cause without asserting which it was, avoids it.

AC #4 remains open

"The feature flag flips ON only when this passes end-to-end." The dispatch now demonstrably runs from a real worker thread (test_ent430_dispatch_actually_runs.py drives it through anyio.to_thread, with no stub over the spawn), but there is still no live-instance measurement after the fix — the only one in the body predates it, and per the pass-2 finding it could not have succeeded. Not a code defect; it is the gate the issue sets, and it is unmet.

---

Clean, with what was checked

• The import fix — get_task_execution_service() exists; the module-level task_execution_service never did. test_the_names_this_service_imports_actually_exist_on_the_real_modules walks every from services.X import Y in the module with AST and resolves it against the real module, so the class of bug (a test stub manufacturing the symbol whose absence was the defect) cannot recur here.
• Thread safety — _inflight.add/discard runs only on the loop thread, in both the direct and the hopped path, because _schedule_on_loop is the sole mutator and is always the thing dispatched to the loop.
• CAS ordering — the spawn sits below the 409, so a lost race spends nothing; and the idempotency key digests the answer text, which is why the loser dispatching would have produced two paid executions for one item, not a harmless duplicate.
• dispatched is set after the spawn returns, so the swallowed-exception path reports false. Tested from both directions.
• Auth — unchanged. No new route, no new gate.
• Docs — architecture.md's ent#329 section now carries the second caller, the truthy-_status_conflict shape, the sync-endpoint/no-loop defect and what resume_requested reports. Closes is the qualified cross-tracker form, so the private issue needs a manual status-in-dev bump at merge.

Summary

• Critical: 0
• Informational: 2 — I1 needs a product decision (render or drop); I2 is a one-line log-accuracy fix
• Open gate: AC fix: add missing logging_config.py to backend Dockerfile #4, unverified end-to-end on a live instance
• Scope: clean

[obasilakis]

Re-reviewed on ced40543. All three blockers from the last pass are closed, and closed the right way — with tests that drive the real function instead of stubbing the thing under test.

• The dispatch runs. spawn_resume_dispatch now detects the no-loop case and hops back to the host loop via anyio.from_thread.run_sync. Fixing it in the spawn rather than by making the route async def is the right call for both reasons you give — the route does blocking DB I/O, and ent#430's stated shape is one dispatch site. test_the_spawn_works_from_a_worker_thread drives it through anyio.to_thread.run_sync, which is the same threadpool Starlette's run_in_threadpool uses, and asserts the premise (no running loop on that thread) rather than assuming it. The non-anyio-thread branch re-raises with the cause named instead of degrading to the silent no-op — correct, since a swallowed RuntimeError is what made the original invisible.
• The lost race. if not updated or updated.pop("_status_conflict", False) matches routers/operator_queue.py:229, popped rather than read so the sentinel cannot reach _project, and test_the_lost_race_shape_that_actually_happens_dispatches_nothing uses the truthy-dict shape that actually occurs rather than the None the old test used.
• resume_requested reports what was scheduled. Set on the line after the spawn returns, so the failure path answers false; the opt-in gate moved ahead of the spawn so an opted-out agent is not dispatched at all. Three tests cover raised / opted-out / real schedule.

The separate get_task_execution_service fix is a real find, and test_the_names_this_service_imports_actually_exist_on_the_real_modules is the right guard for it — parsing the real module with ast rather than importing, so a leaked sys.modules stub cannot satisfy it. _status_of returning answered before checking expiry is also right: an answer that landed is a fact a later expires_at does not undo.

One thing before it merges

docs/memory/architecture.md (Respond → Resume, the new "Two callers, one rule" bullet) says:

Guarded now by enumerating every caller rather than the one route ent#329 knew about, so a third site inherits the rule instead of re-losing it.

That guard does not exist. tests/unit/test_ent329_operator_resume.py::test_dispatch_hangs_off_the_cas_win_only is unchanged and still reads exactly one file:

source = _read("routers/operator_queue.py")
conflict = source.index("_status_conflict")
dispatch = source.index("spawn_resume_dispatch")
assert conflict < dispatch

client_portal/asks/service.py — the caller this PR adds, and the one that lost the rule — is outside its reach, so the third site inherits nothing. The sentence claims the protection that would prevent a recurrence of the defect this PR is fixing, which makes it worse than no sentence: the next person adding a dispatch site reads it and stops looking.

Either write the guard or correct the sentence; I'd take the guard, since the callers are already enumerable (grep -l spawn_resume_dispatch, or a literal two-entry list with the ordering assertion applied per file). This is the shape the sibling PR #2428 wrote a learnings.md entry about this morning — a comment that names a failure mode is a request for a guard — so it may as well land the same way here.

I'll approve on that push.

Non-blocking

• client_portal/asks/models.py:24 — the field comment still reads # pending | expired (terminal ones are not listed). _status_of now emits a third value on the answer response.
• resume_requested and the new answered status are still unconsumed: stores/clientPortal.js::answerAsk drops the ask from the list and discards the response body, so nothing renders either. AC #5's "a surface can say answered without implying the agent started working" is satisfied in the payload and not yet on a surface. Fine as a follow-up — flagging it so it is a decision rather than an oversight.
• The two-read window on the opt-in is now documented honestly, which is the right resolution — thanks for correcting the earlier "cannot disagree".
• CI is 23 green / 4 skipped on head.

[obasilakis]

Approved.

The three blockers are closed and the code is right; the one item I raised is documentation, so it is not worth holding the branch for.

Before merge, please push: docs/memory/architecture.md (Respond → Resume, "Two callers, one rule") says the CAS rule is "Guarded now by enumerating every caller". test_dispatch_hangs_off_the_cas_win_only still reads only routers/operator_queue.py, so client_portal/asks/service.py is outside it. Either widen the guard to both callers or drop that clause — as written it tells the next person adding a dispatch site that they are covered when they are not.

Smaller, whenever: client_portal/asks/models.py:24 still comments the status field as pending | expired, and resume_requested / answered are not consumed by any surface yet (answerAsk discards the response body).

[dolho]

Pushed f5da0f33 — the guard, plus the field comment.

The guard exists now, and writing it caught the same defect one level up

You were right that the sentence was worse than nothing. _dispatch_call_sites() walks the backend tree rather than listing files, because a hardcoded list structurally cannot catch the case that matters — the file it would need to check is the one being added.

The first version was blind, and a mutation proved it. I wrote it as "_status_conflict" in source against the raw file text, deleted the check from asks/service.py, and it still passed: the long comment above the check explaining the race still contains the string. A source-substring guard cannot tell a check from a paragraph about the check — which is the defect the guard exists to prevent, inside the guard.

It now parses each dispatching function and compares ast.unparse output, where comments do not survive. Three mutations, each caught:

mutation	result
delete the check in asks/service.py, keep the comment	FAIL
neuter the check in routers/operator_queue.py	FAIL
add a brand-new third caller with no check at all	FAIL

23 pass on the real tree. test_the_discovery_walk_finds_both_known_callers pins the floor so a rename of the helper cannot leave the loop iterating an empty list and passing in silence — the failure a discovery guard trades for the one it fixes.

models.py:24 corrected: it now says which values are reachable from a listing and which only from the answer response.

The unconsumed fields — filed as a decision

abilityai/trinity-enterprise#468. Kept out of this PR deliberately: where a transient confirmation lives is a product call (the ask row is removed on answer, so it needs a home), not a mechanical fix. The issue states both options and recommends rendering, with the honest note that this PR's review spent a blocker correcting a value that currently has no consumer.

One thing you should know before merging: ent#329 is inert on dev right now

I tested the Workspace wave against a live instance built from origin/dev (9b0ed63b) this afternoon. With the per-agent opt-in enabled, an operator answer through POST /api/operator-queue/{id}/respond returns 200 status: responded and then:

• schedule_executions WHERE triggered_by='operator_response' → 0
• audit_log WHERE event_action='operator_resume_dispatch' → 0

From the running backend:

Task exception was never retrieved
future: <Task finished coro=<maybe_dispatch_resume() defined at
/app/services/operator_resume_service.py:93>
exception=ImportError("cannot import name 'task_execution_service'
from 'services.task_execution_service'")>

That is the 53bc8601 fix in this PR. So ent#329 is merged, marked status-in-dev, counted against the release floor — and every operator answer on an opted-in agent records the answer and re-triggers nothing until this lands. Worth prioritising the merge over the usual queue.