Skip to content

Report Claude subagent activity and keep automatic session replies out of channels - #3587

Merged
trissim merged 4 commits into
mainfrom
work/slot-activity--agent-comms
Oct 29, 2026
Merged

trissim merged 4 commits into
mainfrom
work/slot-activity--agent-comms

Conversation

@trissim

@trissim trissim commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

4 commits, 13 files, +152/-34: Claude slots report work; automatic replies stay in DMs

Two observed failures in one Claude-session repair. Subagent PreToolUse events previously vanished at SubagentCall.judge; an occupied slot therefore looked ready while working. Separately, ReceivingMonitor.confirm treated every delivered destination as an automatic reply route, broadcasting ordinary final text to mentioned channels.

Activity: use the existing phase and activity owners

Before:

  • SubagentHook(SubagentSeats) alone owned the agent-id/seat mapping.
  • SubagentCall(GatedCall) discarded every tool call.
  • Native/bridge ThreadOutput.enter projected phases into activity.
  • A slot had no native turn, so even a recorded tool event would expire after 120 seconds.

After:

  • SeatedSubagent(SubagentSeats) owns the existing seat lookup and activity publication. Both SubagentHook(SeatedSubagent) and SubagentCall(GatedCall, SeatedSubagent) inherit it; commander forks use the existing agent-id-derived fork name.
  • Start enters ModelWaitPhase; PreToolUse enters ToolRunningPhase(tool_name).
  • AgentActivity.enter_phase owns phase-to-activity projection for native turns, bridge outputs and subagents, using the existing ActivityStore. ThreadOutput.enter consumes it rather than maintaining a second projection.
  • ThreadExecution.holds_activity retains the existing native-turn lifetime. SubagentExecution overrides it using the occupied registration, so both slots and commander forks stay current until stop/session-end.
  • SubagentCaller.end enters IdlePhase before the existing unregister/archive operation. No additional activity store, per-slot file, event-name switch, exception swallowing or process mirror.

The hook uses client_wire against its existing WatchRecord root, retaining Core's published-route client lifetime and refusal behavior.

Bound callbacks receive the record from its owner

The initial implementation copied *state.bindings() into its new activity callbacks. That supplied an unstated exactly-one-record assumption outside RecordedWatch, and mypy correctly found six errors. The fix changes the existing WatchState.by_role contract: fleet and commander callbacks take WatchRecord; RecordedWatch invokes the selected callback with self.record. The unbound implementation never invokes those callbacks. All five production callers migrate together: activity, subagent start, stop, session-end, and deck selection. The start/stop/end callers now pass their bound methods directly instead of tuple-splat lambdas. No callback binding cast, overload, fallback or extra state is added.

bindings() remains only for the actual loops in queue_bypass.py:216 and skill.py:248. Search rg -n 'by_role\\(|bindings\\(' src/agent_comms_hooks shows all five migrated callers and the two remaining iteration sites; *state.bindings() has zero matches in forks.py. Retained source map: /home/ts/.cache/agent-scratch/slot-activity/by-role-ast-before.log, 11 hook modules, zero parse omissions; dynamic callback resolution is not inferred by that AST map.

Targeted mypy on all three changed hook modules succeeds: no issues found in 3 source files, exit 0. The current runtime does not include mypy; the first attempt failed with No module named mypy, and the first cached invocation lacked librt. The completed check reused existing cached Python-3.14-compatible mypy 2.3.1, mypy_extensions, pathspec, librt and ast_serialize through PYTHONPATH without installing/changing runtime packages. Command: MYPYPATH=src timeout -s INT 300 python -m mypy --explicit-package-bases --follow-imports=silent --ignore-missing-imports src/agent_comms_hooks/forks.py src/agent_comms_hooks/watch.py src/agent_comms_hooks/skill.py. Full result: /home/ts/.cache/agent-scratch/slot-activity/by-role-mypy-result.log. The four behavior selectors still need the integration owner's run on the final head; no unit pytest was launched.

Rebased this owned branch onto main 86ced58 before the owner correction. Earlier test receipts continue to identify their original pre-rebase heads, not the final head.

Replies: destination types own automatic eligibility

Before: complete monitor confirmation returned (message.exact_reply_target,) for every destination.

After: NamedTarget.automatic_reply_targets returns no routes. ThreadTarget alone supplies its sender route. ReceivingMonitor.confirm asks that existing target family; channel sends remain explicit. It also preserves P924/#3581's independent starts_turn_for response-policy gate so informational/notice messages cannot acquire automatic replies merely by being consumed.

No startswith('#'), channel-name roster, message flag, new reply store or change to explicit channel sends. The existing staged ReplyingThreadOutput still publishes at actual completion, with its original clearing/idempotence behavior.

Source evidence

Retained AST map before editing at acdac6e: refactor-audit Package.load(Repository(...), 'HEAD', root) over src/agent_comms (588 modules) and src/agent_comms_hooks (11 modules), zero parse omissions. Declarations, inheritance, relevant imports and reads/writes/calls were retained at /home/ts/.cache/agent-scratch/slot-activity/ast-before.log. Sites were read semantically through slot start/seat/stop/session-end, ActivityStore and registry projection, monitor confirmation, and staged final publication. Dynamic callback resolution is not proved by this AST evidence.

Before/after derivation searches: rg -n 'enter_phase|holds_activity|automatic_reply_targets|class SeatedSubagent|class SubagentCall' src and rg -n 'exact_reply_target|delivered_targets|reply_targets' src.

IMPL-4 / fact: a subagent's tool phase must reach activity; at: old read_gate.py:110 no-op; derive: phase-owned activity through AgentActivity; by: SubagentCall(SeatedSubagent) and native/bridge ThreadOutput sharing enter_phase.
BOUND-8 / fact: which destination accepts an automatic final; at: old harness_delivery_states.py:118 flattened all destinations into one route; derive: existing target-family behavior; by: NamedTarget empty default and ThreadTarget override.

Proof and remaining acceptance

Completed source checks: all 11 changed Python files compile in memory; git diff --check passes; production imports of SubagentCall and ReceivingMonitor succeed. Branch push was verified against git ls-remote; exact head 69054ca.

Tests added/extended through real entrypoints, with no patches of product code:

  1. tests/test_commander_forks.py::test_a_subagent_start_registers_its_fork_and_a_resume_reuses_the_archived_thread: real start, PreToolUse and stop for a commander fork, checking thinking/tool/idle activity.
  2. tests/test_commander_forks.py::test_a_unit_sessions_subagents_take_role_slots_and_an_unbound_sessions_take_none: real hook start/PreToolUse/stop for an occupied seat, registry activity projection and a deliberately old event proving a long-running tool does not become idle.
  3. tests/test_claude_automatic_reply.py::test_channel_mention_and_dm_share_a_turn_without_broadcasting_the_final: real monitor socket/probe/confirmation for an outstanding #comms mention plus DM, Claude assistant/system record decode, final publication only to the DM, repeated-completion control and explicit channel-send control.

The existing read-gate test now creates the child its simulated PreToolUse belongs to; it no longer asks a bound session to report activity for an agent that never started.

The integration owner ran the three selectors on initial head 69054ca2c with the installed runtime: 2 passed, 1 failed. Both real hook lifecycle tests passed. The bridge control counted viewer's setup-generated channel join/leave announcements. The second commit tried to record a setup cut using message_history()[-1]; that was my error because Bus.message_history at bus.py:234 returns an Iterator. On be1bfe48a, the owner's expanded four-case run was 3 passed, 1 failed: both lifecycle controls and the existing read-gate control passed; the bridge control crashed on that subscript before exercising final publication.

The third commit removes that history lookup entirely. It uses channel.seq, already returned by the setup's committed send, and counts only viewer sends after that cut. No body filter or materialization of history is added. Production source is unchanged by both test corrections. The latest corrected head still needs the owner's rerun; compile checks are not claimed as execution proof.

Unit pytest is prohibited by standing instructions. The integration owner owns the combined run with PYTHONPATH=$PWD/src:$PWD/tools timeout -s INT 300 python -m pytest -p no:xdist -o faulthandler_timeout=60 .... No tests, full suite or ratchets were run by this unit. Installation and live Toad acceptance remain with the integration owner.

Integration

Read open-PR file inventory before editing. #3581/P924 repairs notice reply policy and overlaps the ReceivingMonitor return; its old mentioned-channel expectation conflicts with Tristan's newer no-channel-auto-reply instruction. Its owner and claude-code were told. The commander's decision is #3581 lands unchanged first; this PR then integrates current main, preserves the policy gate and changes that mentioned-channel expectation to no channel publication. At the test-correction fetch, #3581 is still open and its test is not yet on main. This PR owns the destination-type restriction and slot activity, not a second notice implementation. P918's nominal path migration may require import-path rebasing; no competing nominal migration is included.

Assumed: Claude emits SubagentStart before that agent's PreToolUse and SubagentStop or SessionEnd retires its registration; the existing WatchClaim role and seat mapping are authoritative; configured slot/fork root binding does not change during a tool event; integration owner runs the focused controls and verifies actual Toad activity plus a real answered DM after install.

seated slot runs Bash; now the status gets the memo instead of #comms getting the whole final

Copilot AI balanced review requested due to automatic review settings October 29, 2026 05:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim enabled auto-merge October 29, 2026 05:53
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

debt-src at 1d9c6b2, every finding:

::warning file=src/agent_comms/harness_delivery_states.py::LintFindings +1 (warns, never blocks)
grew TypeErrors:src/agent_comms_hooks/forks.py +6

@trissim
trissim force-pushed the work/slot-activity--agent-comms branch from 04d82b3 to 4b6e590 Compare October 29, 2026 06:17
@trissim
trissim added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit a427f64 Oct 29, 2026
9 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.

2 participants