Skip to content

fix(skill-gate): the person who asked sees how a gated request ended (#3653) - #3657

Merged
vybe merged 3 commits into
devfrom
feature/3653-gate-wake-channel-context
Oct 14, 2026
Merged

vybe merged 3 commits into
devfrom
feature/3653-gate-wake-channel-context

Conversation

@obasilakis

@obasilakis obasilakis commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • An agent that requests a gated skill while answering someone in a conversation is woken with the outcome and writes a reply. The wake-up run carried no channel context, so the reply reached no conversation (rc3 testing matrix, Workspace requester).
  • skill_gate_service.enforce now records the chat the requesting agent's turn is answering, at request time, only while that turn is the agent's own and still running (the rule _inherited_channel_context uses).
  • Wake-ups that report an ending (denied, expired, stale, not run, succeeded, failed) run with that context, so channel_completion_report posts the requester's reply into the conversation. The "approved, now running" wake-up carries none.

Changes

  • src/backend/services/skill_gate_service.py: REQUESTER_CHAT, _requester_chat, _wake_requester_agent(to_chat=); source_channel_agent classified DROP in DISPATCH_FIELDS.
  • src/backend/services/task_execution_service.py: execute_task accepts and persists source_channel_agent.
  • src/backend/services/turn_audience.py, src/backend/routers/agents.py: comments naming the writers of source_channel_agent.
  • tests/unit/test_3653_gate_wake_channel_context.py (new), two test stubs accept the new keyword.
  • docs/memory/feature-flows/skill-gate.md.

No migration. The context rides the record's existing dispatch JSON and is never replayed to the approved run.

Behaviour a reviewer should know

  • The requesting agent decides what to relay from the outcome text into the conversation. That text includes a rejecting approver's note and up to 1000 characters of the gated run's reply.
  • The Workspace report is headed "Finished" for every delivered wake-up reply, including a rejection: the header reflects the wake-up run's status.
  • A Workspace thread shows the message at its next history load.
  • A requester that is stopped, or has no free slot when woken, still delivers nothing to the conversation.
  • The wake-up row always carries source_channel_agent, so it reads as delegated work: file shares in it are owner-only and the voice route answers delegated_turn.

Test Plan

  • cd tests && pytest unit/test_3653_gate_wake_channel_context.py -v (8 pass; 5 fail without the fix)
  • Gate and completion-report suites: 2049 pass. TestPaidA2A (3) and test_ent679_a2a_payment_gate.py fail on the untouched base in the author's environment (payments_py import); CI is the reference for those.
  • Live: Workspace user asks a companion, companion calls a gated skill on a second agent, approver approves; the thread shows the companion's outcome reply after reload. Repeat with Reject.

Fixes #3653

🤖 Generated with Claude Code

obasilakis and others added 2 commits October 13, 2026 16:51
…3653)

An agent that requests a gated skill while answering someone in a
conversation is woken with the outcome and writes a reply. The wake-up
run carried no channel context, so that reply reached no conversation.

- `enforce` records the chat the requesting agent's turn is answering
  (`_requester_chat`), under `REQUESTER_CHAT` in the record's `dispatch`.
  It is recorded only while that turn is the agent's own and still
  running, the rule channel inheritance already uses: an agent can cite
  any of its past turns, and a finished one would address the outcome to
  whoever that turn was for.
- Every wake-up that reports an ending (denied, expired, stale, not run,
  succeeded, failed) runs with that context, so
  `channel_completion_report` posts the requester's reply into the
  conversation like any background completion. The "approved, now
  running" wake-up carries none.
- `execute_task` accepts and persists `source_channel_agent`, so the
  wake-up row reads as delegated work for the file-share audience and
  the voice route, and Slack/Telegram deliver through the binding the
  person addressed.

No migration: the context rides the existing `dispatch` JSON and is
never replayed to the approved run.

Fixes #3653

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#3653)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@webmixgamer

Copy link
Copy Markdown
Contributor

/review + /validate-pr

Verdict: APPROVE. No critical findings. The merge order with #3658 doesn't matter (see Interaction).

/review (structural)

Scope: clean. For #3653, every wake-up that reports an ending runs in the chat the requesting agent was answering.

Execution coverage

Changed symbol Executed by Live consumer Verdict
_requester_chat / enforce 8 new tests through the real enforce → answer → resolve on the per-test DB enforce (the record's dispatch) ✅
_wake_requester_agent(to_chat=) / _notify the same tests (the real wake-up over a recorded execute_task) _notify → execute_task ✅
execute_task(source_channel_agent=) test_execute_task_writes_the_binding_agent_on_the_row (the real execute_task) report_completion and turn_audience read it off the row ✅
  • Tests: 106 passed locally (the new file plus the neighbouring gate suites).
  • Mutation: 6 of 6 red, each restored byte-identical afterwards:
    • chat not stored → 4 red;
    • "approved" also posted → 1;
    • running check removed → 1;
    • agent check removed → 1;
    • wake-up drops the chat → 3;
    • binding agent not persisted → 1.
  • Delivery chain (read in code):
    • All three terminal writers call spawn_completion_report unconditionally once they win the status update.
    • report_completion has no filter that skips a skill_gate row; INLINE_CHANNEL_TRIGGERS is slack/telegram/whatsapp/public.
    • The Workspace leg is consent by construction. Its recipient check passes because the client is copied from the requester's own turn.
  • Safe by construction:

Informational

  1. A rejection reads "Finished" in the Workspace (8/10). _portal_body takes the header from the wake-up run's status (head = f"**Finished{who}**" if done else …). A rejected or failed request therefore arrives as "Finished" with the done pill, above a body that says it was rejected. The PR body notes it. Suggest a neutral header for skill_gate rows; fine as a follow-up.
  2. Slack without proactive consent still gets nothing (7/10). _resolve_slack returns None when the binding's allow_proactive is off, so a Slack-originated request keeps the Gated skill: the person who asked never sees the outcome in their chat; the requesting agent's wake-up run carries no channel context #3653 symptom there. Worth a line in the flow doc and the 1.0 known limitations.
  3. A busy or stopped requester loses the outcome (6/10). The wake-up is fire-and-forget (except Exception: logger.info("… not woken …")). This predates the PR, and the PR body says so.

Low confidence: no test drives a skill_gate wake-up row through report_completion into the Workspace thread. I verified the chain above by reading the code.

Interaction with #3658

  • The code in skill_gate_service.py merges cleanly, and both PRs merge cleanly onto the current dev.
  • There is one trivial conflict between them in docs/memory/feature-flows/skill-gate.md: both add a test-table row at the same line. Keep both rows.
  • On a combined tree with the conflict resolved that way: the two new files plus the gate suites gave 170 passed, and all 65 gate, completion-report and turn-audience test files gave 1370 passed, 8 skipped.

/validate-pr

Lane B (no schema), 10 files (+341/−7), 2 commits, head 70b1b77db.

Category Status Notes
Commit / base / size ✅ conventional fix(skill-gate) + docs(security); targets dev; 10 files
Status auto-promotion ✅ Fixes abilityai/trinity#3653
Issue ✅ #3653: P1, type-bug, status-in-progress
Feature flow ✅ skill-gate.md updated; full standard section layout
Security §4.1–4.10 ✅ clean; the /cso --diff report is included
Disclosure guard ✅ clean over the added doc lines
Test adequacy / execution coverage ✅ named regression test file; mutation 6/6 red
Merge gate ✅ Tier 1 green: regression diff, pytest (head, seed 12345), the 4 required contexts, gitleaks, journey-smoke. dev's last green is the RC3 tip; the run for the newest dev merge (#3666) hasn't reported yet, so check dev-ci just before merging

Testing note: the author of this PR also owns the release-test row that re-tests it (A2 on abilityai/trinity-enterprise#847). That re-test needs a different tester for these legs.

@webmixgamer webmixgamer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving: no critical findings; the /review + /validate-pr report is in the comment above. The merge order with #3658 doesn't matter: whichever lands second keeps both rows in the skill-gate.md test table. The three informational items can be follow-ups.

…text (#3657) — mechanical, per the merge-train note on the PR

Resolves the adjacent-line collision with #3684 (ent#885) in
skill_gate_service.DISPATCH_FIELDS and task_execution_service.execute_task:
#3684 removed the source_host lines, this PR adds source_channel_agent
beside them. Resolution keeps source_channel_agent and drops source_host,
matching dev.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@vybe

vybe commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

merge-train (2026-10-14): pushed one merge commit to this branch — origin/dev merged in, resolving the adjacent-line collision with #3684 (ent#885) in skill_gate_service.DISPATCH_FIELDS and task_execution_service.execute_task. #3684 removed the source_host lines; this PR adds source_channel_agent right beside them. The resolution keeps source_channel_agent and drops source_host, matching dev (the branch now has the same source_host mentions as dev, i.e. none in code). Nothing else changed.

Validated at 70b1b77db (READY, 0 critical, 5/5 hand-off mutations red); the merge commit is the only delta. Neighbouring suites on the merged tree: 3653 / ent751 dispatch / ent751 callers / ent885 keyless / 3520 / ent886 / ent848 / 1482 — green, except test_ent751_gate_callers.py::TestPortal::test_a_held_turn_is_answered_with_the_notice_never_failed, which is intermittent on plain dev too (~1 in 5 runs on both trees) — not this PR's; follow-up issue to come.

@vybe
vybe merged commit b2fffe0 into dev Oct 14, 2026
25 checks passed
vybe pushed a commit that referenced this pull request Oct 14, 2026
…3658) — mechanical, per the merge-train note on the PR

Resolves the test-table collision with #3657 in
docs/memory/feature-flows/skill-gate.md: both rows kept, #3657's first.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
vybe pushed a commit that referenced this pull request Oct 17, 2026
…shed" (#3688) (#3757)

After #3657 the requester is woken (triggered_by="skill_gate") with how
its gated request ended, and the reply is reported into the Workspace
thread it came from. _portal_body chose its header, and _portal_source
its pill, from the wake-up row's own run status, which completes
normally for a rejected, expired or not-run request. The reader saw a
"Finished" header and a green "Finished" pill above "Your request was
rejected."

A skill_gate row that completed now gets the header "Approval update"
and the source completion:notice, which shows no outcome pill and is
narrated as a background report. One predicate decides both, so the
header and the pill cannot disagree. A wake-up that itself failed still
reads "Didn't finish"; the approved run keeps its requester's trigger
and still reads "Finished". triggered_by is passed to every resolver,
per the shared call shape.

Fixes #3688

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants