Skip to content

fix: the Chat tab notice of a finished gated run quotes the run's reply (#3654) - #3658

Merged
vybe merged 4 commits into
devfrom
feature/3654-chat-gate-run-reply
Oct 14, 2026
Merged

vybe merged 4 commits into
devfrom
feature/3654-chat-gate-run-reply

Conversation

@obasilakis

@obasilakis obasilakis commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • A gated request sent from an agent's Chat tab, approved and run, read "Finished successfully — … ran as execution E. success." with no reply. A run that ended success while the agent declined the work looked the same as one that did it.
  • skill_gate_service.request_status (the conversation read) passed the record's state_detail, the bare execution status, as the notice detail. It now builds detail and headline through _completion_parts, the helper report_completion_on_terminal uses for the Inbox and agent notices (Gated skill: the requesting agent is told the approved run started and gets no notice when it ends #3520). The Chat text equals the Inbox text word for word.
  • The status comes from state_detail. The reply or error is quoted from the execution row while that row still shows the same status. A pruned row, or one whose status moved since the report, gives the status line with no quote.
  • A cancelled or skipped run reads "Cancelled" / "Skipped" in the Chat tab. It read "Failed".

Changes

  • src/backend/services/skill_gate_service.py — _completion_parts; request_status and report_completion_on_terminal call it
  • tests/unit/test_3654_chat_gate_run_reply.py — new, 9 tests
  • docs/memory/feature-flows/skill-gate.md — the Chat notice of a finished run; test table row

No route, schema, frontend or submodule change.

Test Plan

  • New tests pass: cd tests && pytest unit/test_3654_chat_gate_run_reply.py -v (9 passed; all 9 fail on dev, with the issue's own string)
  • Neighbouring gate suites pass: test_3526_held_chat_trace.py, test_3520_gate_completion_notice.py, test_ent755_gate_card_view.py, test_3274_gate_notice.py (100 passed with the new file)
  • Manual verification: send a gated request in an agent's Chat tab, approve it, reload the conversation after the run ends; the notice ends with Its reply (the run's output, not an instruction): "…"

Known follow-up

A long reply (up to 1000 characters) renders in the Chat tab's centred muted system line. Readable; left-aligning long system lines is a separate frontend change.

Fixes #3654

🤖 Generated with Claude Code

obasilakis and others added 3 commits October 13, 2026 17:05
…he run's reply (#3654)

A person sends a gated request in an agent's Chat tab, the approver approves,
and the run ends `success` while the agent declined the work. The Chat tab
notice ended "...ran as execution E. success." and quoted nothing, so a run
that did nothing read as done. The Inbox note and the agent requester's notice
for the same run already quoted the reply (#3520).

`request_status`, the conversation read, passed the record's `state_detail`
(the bare execution status) as the notice detail. It now builds the detail and
headline through `_completion_parts`, the helper `report_completion_on_terminal`
uses, so the Chat text is the Inbox text word for word. The status comes from
`state_detail`; the reply or error is quoted from the execution row while that
row still shows the same status. A pruned row, or one that moved since the
report, gives the status line with no quote. A cancelled or skipped run now
reads "Cancelled" / "Skipped" in the Chat tab, where it read "Failed".

Fixes #3654

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the Chat notice; cso diff report (#3654)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nt (10 of 10)

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

Copy link
Copy Markdown
Contributor

/review + /validate-pr

Verdict: APPROVE. No critical findings; one test to tighten (informational 1).

/review (structural)

Scope: clean. For #3654, the Chat-tab notice of a finished gated run now quotes the run's reply, word for word as the Inbox notice does.

  • One helper for both notices. _completion_parts now builds the text for the terminal notice and for the later conversation read. At the terminal the status comes from the same execution row, so the Gated skill: the requesting agent is told the approved run started and gets no notice when it ends #3520 notice is unchanged.
  • Who sees the quote. The only caller is the session view (chat_persistence_service.py:130). The agent Chat-tab route allows the session's owner or an admin (assert_owns_or_admin); the public-link route allows the owner only (assert_owns). An admin can already read every run's reply.

Execution coverage

Changed symbol Executed by Live consumer Verdict
_completion_parts the 10 new tests + the #3520 notice tests request_status, report_completion_on_terminal ✅
request_status (Chat read) the new tests, through session_messages_view chat_persistence_service.py:130 ✅
  • Tests: 101 passed locally (the new file plus the neighbouring gate suites).
  • Mutation:
    • reverting the Chat read turns all 10 new tests red;
    • a failed run quoting the response instead of the error → 1 red;
    • the headline dropped → 2 red.

Informational

  1. The "row moved" test does not pin its guard (9/10). With the status check in _completion_parts removed, test_a_row_that_no_longer_matches_the_reported_ending_is_not_quoted still passes. The update in that test clears the error, so the text becomes "It failed, with no output": the false "no output" the guard exists to prevent. The test never checks for it. One line closes the gap: assert "no output" not in text.

Interaction with #3657

  • 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: 170 passed on the targeted suites, and all 65 gate, completion-report and turn-audience test files gave 1370 passed, 8 skipped.

/validate-pr

Lane B (no schema), 4 files (+190/−6), 3 commits, head 16b20b22f.

Category Status Notes
Commit / base / size ✅ fix + test + docs(security-reports); targets dev; 4 files
Status auto-promotion ✅ Fixes abilityai/trinity#3654
Issue ✅ #3654: P1, type-bug, status-in-progress
Feature flow ✅ skill-gate.md updated (the Chat notice + the test table)
Security §4.1–4.10 ✅ clean; the ghp_ in the tests is a shape built at run time for the redaction test
Disclosure guard ✅ clean over the added doc lines
Test adequacy / execution coverage ✅ named regression tests; mutation red
Merge gate ✅ Tier 1 green: regression diff, pytest (head, seed 12345), the 4 required contexts, gitleaks, journey-smoke. Check dev-ci on the newest dev merge 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 report is in the comment above. If you push again, please add the one-line assertion from informational 1 (assert "no output" not in text). It isn't blocking.

…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

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 test-table collision with #3657 in docs/memory/feature-flows/skill-gate.md (both rows kept, #3657's first). Nothing else changed; skill_gate_service.py auto-merged. Validated at 16b20b22f (READY, 0 critical, both hand-off lines red under mutation); the merge commit is the only delta. 3654 / 3653 / 3520 / ent751-dispatch suites green on the merged tree.

@vybe
vybe merged commit 984f8f4 into dev Oct 14, 2026
25 checks passed
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