Stop duplicating and fabricating session hook rows - #761

Two independent defects made the /sessions picker show rows that no
reconcile pass could ever remove.
Fan-out amplification. One `wtcli agent-hook` invocation produces one COM
`agent_event`, which the protocol server broadcasts to every subscribed
helper. Each helper both updates its own registry and forwards the event to
wta-master, so master applied a single real hook once per live helper and
re-broadcast `sessions/changed` for each copy. A live master log shows one
SessionStarted arriving three times over HelperId(1..3) with nine resulting
`sessions/changed` writes; the same log shows exactly two copies earlier,
when only two helpers were connected.
`wtcli` now stamps a per-invocation `broadcast_id` GUID, helpers forward it,
and master keeps a bounded FIFO of ids it has already applied. The suffix
identifying which of an event's several SessionEvents is being published
comes from the emit site (`HookSlot`), never from a position in the emitted
sequence: `session_known` is per-helper local state, so an unaware helper
prepends a synthetic start while an aware one does not. Positional indices
would give the same logical event different keys on different helpers and,
worse, alias one helper's placeholder onto another's real event.
Fabricated terminal rows. `needs_synthetic_start` excluded only
`agent.session.started`, so an `agent.session.end` for a session WTA had
never seen invented a SessionStarted titled after the cwd basename, shipped
it to master, then shipped the real SessionStopped. That left a permanent
`Ended` row — observed as a "yuazha" row for an abandoned session with zero
turns and no on-disk state — which `is_stale_host_history_row` cannot prune,
since it only drops ids the listing agent itself returned and later stopped
returning. Terminal events no longer synthesize a start.
`agent.error` is deliberately not excluded. It describes a live but failing
session, and its `ConnectionFailed` reducer resolves the row through
`active_by_pane`, so removing the synthetic start would silently drop a
first-observed connection failure instead of surfacing it as `Error`.
An absent `broadcast_id` — born-bound registrations, resume bookkeeping, or
a `wtcli` predating the field — keeps the previous apply-every-copy
behavior, so old and new components interoperate in both directions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

The WTA unit tests exercise one helper's routing function and the master's
handler in isolation, inside a single process. Both defects this PR fixes live
in the fan-out itself: one `wtcli agent-hook` invocation becomes one COM
broadcast that reaches every subscribed helper process, each of which forwards
it to master over its own named pipe. No unit test can span that boundary.
Three cases, C287-C289:
- **One hook broadcast applies once per helper fan-out.** Injects
 `agent.tool.starting` for an unseen session, which deliberately expands into
 two published events, and asserts each is applied exactly once. The `start`
 and `primary` slots must appear as separate dedupe keys — a positional suffix
 would collide them, because whether a helper emits the synthetic start depends
 on its own local registry state. A guard asserts a replay was actually
 dropped, so the case cannot pass vacuously on a single-helper machine.
- **Terminal hook for an unknown session creates no row.** Injects
 `agent.session.end` for a fresh id and asserts no `SessionStarted` and no
 cwd-basename title follow it. The absence is only asserted after the matching
 `SessionStopped` proves the hook reached master.
- **Agent error for an unknown session still records the failure.** The
 false-positive control: `agent.error` is a live-but-failing session whose
 pane-keyed reducer needs the row, so it must keep creating one.
Extra tabs rather than agent panes supply the fan-out. Every eligible tab
pre-warms a stashed helper, which is already connected to master and already
subscribed to the broadcast, so the suite needs neither `winapp` nor an ACP
handshake to reach the boundary under test.
Oracle is the master log. `wta sessions list` would be the more direct state
oracle but is identity-gated outside the package, and the reducer is idempotent,
so a registry row looks the same whether it was applied once or N times.
Validation: 3/3 pass against the deployed Debug package with C287-C289 marked
`[x]` through `Invoke-ItE2EReport.ps1 -UpdateReport`; `Feature.HookTrace` 5/5
still passes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

Review round 1 on #761.
Copilot (2 threads): `SeenBroadcastIds` stored the full `broadcast_id` in both
its set and its queue, so `CAPACITY` bounded the entry count but not the memory
— a sender emitting long ids (still inside wtcli's `kMaxHookEventChars`
envelope) could inflate the window arbitrarily, and the "~60 bytes per id"
sizing note asserted an invariant nothing enforced. Adds `MAX_ID_LEN`, which
makes the capacity a real `CAPACITY * MAX_ID_LEN` bound, and rewrites the note
to reference it.
An overlong id degrades to "apply every copy" rather than being dropped: that
is exactly the behavior before deduplication existed, and the reducer is
idempotent, so a repeat is harmless — whereas dropping would lose session state
the window cannot key.
check-spelling (9 threads): reworded rather than adding dictionary entries.
Three findings were a developer username copied out of a live log into code
comments and test data, including one in production code; those now use a
neutral working directory, matching the `C:\Users\dev` other registry tests
already use. The rest were coined words (`dedupable`, `undedupable`) and two
forbidden patterns (`pre-existing`, `, otherwise`).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

Review round 2 on #761. The header comment still carried the placeholder IDs
C279-C281 written before `Set-ChecklistIds.ps1` ran; the suite's cases are
actually C287-C289, which is what `doc/release-check-list.md` and the generated
report use. Comment-only change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

Replace the helper-forwarded hook architecture introduced earlier in this PR.
Master already owns a WT protocol event subscription, so it should consume the
single COM broadcast directly instead of receiving one named-pipe copy from
every helper and deduplicating them afterwards.
- Route `agent_event` in `handle_master_wt_event` into the authoritative master
 registry. Helpers still process the broadcast locally because their OSC 133;A
 exit heuristic and autofix target lookup need synchronous pane-to-session
 bindings, but they never forward agent CLI hooks.
- Extract `plan_agent_event`, a pure shared event taxonomy used by master and
 helper. Synthetic starts, terminal-event suppression, sessionless fallbacks,
 interactive-tool splitting and ignored events therefore cannot drift between
 the two registries.
- Preserve watcher coordination through a shared `apply_master_session_event`
 boundary. Per-session lifecycle gates serialize reducer and ownership changes;
 a late no-op ResumePaneAssigned no longer downgrades a direct hook's current
 `hook_owned` generation, while ResumeDispatched and born-bound registrations
 still mark a new hook-free generation.
- Apply all COM-hook reducer transitions and broadcast their immediate state
 before scheduling ACP title refresh, so a five-second session/list timeout
 cannot stall the sole COM consumer or leave a synthetic row temporarily Idle.
- Make the master listener load-bearing and observable. `wtcli listen` emits an
 internal ready marker after COM Subscribe succeeds; master refuses to start
 without it. Pre-subscribe failures retry with backoff, while a previously
 subscribed listener restarts immediately because COM broadcasts are not
 replayed. stderr is retained in the exit log instead of discarded.
- Make identical ConnectionFailed events idempotent in both registries. Every
 helper still observes WT connection_state failures, but only the first copy
 changes state and triggers sessions/changed; a different reason remains new
 information.
- Remove the now-obsolete broadcast GUID, HookSlot, SeenBroadcastIds and wire
 envelope machinery from Rust and wtcli.
- Rewrite C287-C289 to assert the real architecture using an info-level,
 post-reducer master breadcrumb: multiple helpers are connected, master
 processes one COM hook, helper-originated lifecycle records for the id are
 absent, and final state is Working / None / Error as appropriate.
Validation:
- cargo test: 1880 passed, 0 failed, 1 ignored
- full solution build: 0 errors
- SessionHookRouting ItE2E: 3 passed, 0 failed
- HookTrace ItE2E: 5 passed, 0 failed
- live: three helpers observed the injected sid, helper forwards=0, master
 applied one SessionStarted and one ToolStarting
- deployment preserved settings.json byte-for-byte
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

Review round after the master-owned hook routing rewrite.
- Drain `wtcli listen` stderr continuously while retaining only a 16 KiB
 diagnostic prefix. A simple bounded read would stop consuming after the cap
 and eventually block a noisy long-lived child when the OS pipe filled; the
 bounded reader discards the tail instead and reports whether it truncated.
- Treat both accepted real-start spellings (`agent.session.started` and
 `agent.session.start`) as superseding any earlier pane-keyed placeholder.
 Without the singular spelling, a missing-id hook could leave an orphan local
 row after the real session id arrived.
Added focused tests that force the bounded reader to drain 32 KiB through a
64-byte duplex pipe, and that create a pane placeholder before a singular real
start.
Validation: 1882 WTA tests passed, full solution built with 0 errors, and the
SessionHookRouting ItE2E suite passed 3/3 after deployment.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

The WT event listener is a session-status enhancement, not an availability gate
for the agent stack. Do not make agent-pane chat, Autofix, history listing or
resume wait for or fail with COM hook subscription readiness.
Master now accepts helpers immediately and starts the listener readiness/retry
loop in the background. A missing WT_COM_CLSID or a listener that remains
unready is logged as "live session status may be stale" while the rest of the
agent experience continues unchanged. Helpers likewise launch their listener
without holding ACP startup behind the 15-second readiness window.
Once the listener reconnects, direct master-owned hook routing resumes. No
special unavailable state is introduced in /sessions: it continues to list and
resume known history, with the same existing limitation that live status can be
stale while hooks are unavailable.
Validation: 1882 WTA tests passed, full solution built with 0 errors, the
SessionHookRouting ItE2E suite passed 3/3 after deployment, and settings.json
was unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

Stop retrying `wtcli listen` after eight consecutive unstable failures. This
keeps a permanently broken COM registration from turning every master/helper
into a process and log storm while preserving the agreed degradation contract:
chat, Autofix, session list and resume stay available; only live session status
remains stale until that WTA process restarts.
A subscription that stays healthy for 30 seconds resets the failure streak. The
first exit after a stable subscription still restarts immediately because COM
broadcasts are not replayed; repeated fast subscribe/exit cycles retain the
streak, enter exponential backoff, and eventually stop rather than tight-loop.
Added a focused policy test covering the eight-attempt ceiling, fast-exit
counting and stable-uptime reset.
Validation: 1883 WTA tests passed, full solution built with 0 errors, the
SessionHookRouting ItE2E suite passed 3/3 after deployment, and settings.json
was unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

Preserve upstream lifecycle, cancellation and settings changes while retaining master-owned hook routing. Resolve the event-dispatch conflict using consistent LF endings. Reconcile shell prompts in the ordered master COM stream so a helper exit cannot overtake a queued birth. Preserve exponential retry delays across unstable subscriptions and measure stability only after readiness. Count all E2E processing records and require currently subscribed helper listeners. Allocate fresh hook checklist IDs C296-C298 without changing upstream IDs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment

Add hookless regression cases for automatic Autofix prompt routing and negative controls, manual fix with auto-suggest disabled, snapshot rendering and resume dispatch without hook rows, ACP chat while listener readiness is pending, and an actual failed-then-recovered listener subprocess delivering a shell error into the Autofix prompt queue. The only new channel constructor is cfg(test); no shipping behavior or dependency changes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6ef3e46-71af-4e5a-82af-a172700ca3ce

Copilot AI left a comment