Skip to content

fix(workspace): a gated request's ending notice no longer reads "Finished" (#3688) - #3757

Merged
vybe merged 1 commit into
devfrom
fix/3688-gate-notice-header
Oct 17, 2026
Merged

vybe merged 1 commit into
devfrom
fix/3688-gate-notice-header

Conversation

@dolho

@dolho dolho commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • After fix(skill-gate): the person who asked sees how a gated request ended (#3653) #3657, the requester is woken (triggered_by="skill_gate") with how its gated request ended, and that reply goes into the Workspace chat it came from. _portal_body chose the header, and _portal_source the Inbox pill, from the wake-up run's own status. That run completes normally for a rejected, expired or not-run request, so 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";
    • the source completion:notice, which shows no outcome pill and is still narrated as a background report in chat history.
  • One predicate (_is_gate_notice) decides both, so the header and the pill can't disagree.
  • Unchanged: a wake-up that itself failed still reads "Didn't finish", and the approved run (which keeps its requester's trigger) still reads "Finished".
  • The issue named only the header, but the pill is a separate field, so a header-only fix would have left the green pill in place.

Changes

  • src/backend/services/channel_completion_report.py: COMPLETION_SOURCE_NOTICE, _is_gate_notice, the header and source choice, and triggered_by passed to every resolver (the shared call shape from the ent#457 review).
  • src/backend/client_portal/chat_previews.py: the notice maps to no outcome pill.
  • src/backend/client_portal/service.py: the notice counts among the completion sources for history narration.
  • tests/unit/test_3688_gate_notice_header.py (12), through the real report_completion → _resolve_portal → add_portal_message path:
    • rejected, expired and not-run notices;
    • the approved run unchanged;
    • a failed wake-up;
    • a delegated agent named in the header;
    • every resolver receives the trigger;
    • the pill source matches the header;
    • no Inbox pill;
    • history narration.
    • 7 of these fail on the old code.

Tested against a local instance

rc4 backend, SQLite. A "rejected" notice was filed through the real report_completion into two new test chats on proj-alpha: one on the current code, one with this branch's three files swapped in.

  • Before: **Finished** / Your request to run pay-invoice was rejected…, source completion:done.
  • After: **Approval update** / the same text, source completion:notice.

Both test chats were deleted and the files restored. Screenshots were shared with the reviewer separately (not committed).

Test Plan

  • test_3688_gate_notice_header.py: 12 passed.
  • Every unit file touching the report path, previews or completion sources: 871 passed, 3 failed. The 3 are TestPaidA2A, which also fail on unchanged dev on that machine and pass in CI.
  • On a real gated request: reject it from the Workspace; the requester's chat shows "Approval update" and no green pill.

Fixes #3688

🤖 Generated with Claude Code

…shed" (#3688)

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>

@AndriiPasternak31 AndriiPasternak31 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. The Workspace fix is right: it keys on triggered_by, not on the body text, so the header, the source marker and the Inbox pill can't disagree. test_3688_gate_notice_header.py is 7/12 red on dev and 12/12 green here. Across 31 related test files the only failures are the 3 in test_ent751_gate_callers.py::TestPaidA2A, and they fail the same way on dev.

One follow-up, plus three nits:

Follow-up: Slack and Telegram still head the notice "✅ Task finished". _summarize (channel_completion_report.py:136-139) doesn't look at triggered_by, and the resolvers take it but ignore it (the comments at :163 and :213 say so). A gate wake-up does reach those channels, because it copies the asking turn's source_channel; test_3653_gate_wake_channel_context.py:110-118 pins a rejected wake-up into a Slack thread. I drove report_completion with a triggered_by="skill_gate" row on each channel, using the ent#457 harness, and both legs sent ✅ Task finished\n\nOK, noted: my request to run pay-invoice was rejected; nothing was run. The fix is small: pass triggered_by into _summarize at :190 and :269, and reuse _is_gate_notice for the head line, so one predicate drives every surface. Here or as a follow-up issue, your call.

Nits:

  1. test_a_notice_shows_no_outcome_pill_in_the_inbox (:101-104) passes on dev. _OUTCOME_BY_SOURCE.get() already returns None for an unknown source, so the "completion:notice": None entry is documentation, not behaviour. The real guard is the test_the_pill_source_agrees_with_the_header[...completion:notice] param. Either drop the test, or drive a row through chat_previews._project.
  2. _is_gate_notice (:560) hard-codes "skill_gate" instead of using skill_gate_service.SKILL_GATE_TRIGGER. If the constant is renamed, the header silently goes back to "Finished" and every test stays green. A lazy import, like the one already done for db.skill_gate_requests, avoids the cycle.
  3. The marker docs still say completion:done | completion:failed: architecture/workspace.md:872, requirements/core-agent.md:3861, feature-flows/workspace-inbox.md:263,364 and feature-flows/skill-gate.md:33.

Not covered: I didn't drive a rejected request through the UI, and I didn't grep the enterprise submodule for other readers of completion:*.

@vybe
vybe merged commit 2762542 into dev Oct 17, 2026
22 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.

3 participants