1 Commits

Author	SHA1	Message	Date
xarmian	e03ba45b5c	feat(web): push-to-agent composer in the item view (TASK-2561) (#1099) ... * feat(web): push-to-agent composer in the item view (TASK-2561) PLAN-2558 S3 — the web half of IDEA-2544's push-to-harness. Adds `api.items.push`, a new `api.sessions.list`, and a "Push to agent…" row in the item pane's ⋯ menu that opens a small composer. The deliverable is the presence line, not the textarea. `pad push` is fire-and-forget — no durable inbox, no ack, no "nobody was listening" warning — which is defensible for a CLI verb typed by someone who knows their own session is running, and indefensible for a button. So the dialog answers "is anything listening?" before the click, and keeps three states apart rather than two: N > 0 send, worded "N session connected", never "will be delivered" — the registry can name a session that died up to ~30s ago and no push gets a receipt N == 0 send DISABLED. Nothing listening means the message is lost, not queued; the empty state offers the clipboard instead (the fallback S4 rules for quick actions) can't tell send ENABLED, uncertainty stated. A 503/401/network failure is not zero — rendering it as zero is the exact lie handleListSessions returns 503 rather than an empty list to avoid The menu row is gated on a resolved user, not on canEdit: push is self-addressed, so a viewer pushing an item into their own session is a read. Without a user the endpoint 401s. $lib/push/message mirrors the server's rune-after-collapse accounting so an over-length message is caught in the composer instead of coming back as a 400. It deliberately does not use JS `\s`: Go's unicode.IsSpace and `\s` disagree in both directions (U+0085 is whitespace to Go only, U+FEFF to JS only), so a `\s` client under-counts a pasted BOM and over-counts a pasted NEL. The agreement is pinned by a shared fixture (internal/server/testdata/push_message_cases.json) read by BOTH internal/server/push_message_collapse_test.go and the web unit test — a TS-only table would assert a belief about Go rather than Go's behaviour. Claude-Session: https://claude.ai/code/session_01QCMLhHQBrMHVML3YKdm4Cd * fix(web): close the push composer's races and ambiguity gaps (codex review) Round-1 review findings on the S3 composer, all real: - ItemDetail did not reset `pushDialogOpen` on an item switch. The dialog is {#key itemSlug}-remounted while `open` is owned by the parent, so a stale `true` silently REOPENED the composer pointed at the new item. The reset block's existing comment (written for copyDialogOpen) describes this exact failure. Verified live, with the counterfactual: reverting the one-line fix reopens the dialog on item B after a client-side navigation. (The typed draft does NOT carry over — the {#key} remount clears it — so the defect is the silent reopen, not a retargeted message.) - Presence polls shared one generation counter, which fences OPENINGS, not requests. A stalled poll could resolve after a later one and overwrite a fresh count with a stale one, re-arming Push against a session list already known to be empty. Added a per-request sequence; only a strictly newer response is applied. - Nothing bounded a `/sessions` read, and 'checking' disables Push, so a request that never settled stranded the composer with a dead button and no explanation. It now degrades to the honest "can't tell" state after 5s; a later response still lands and upgrades the answer. - A failed send re-armed Push unconditionally. The handler publishes BEFORE writing its response, so an unstructured failure (rejected fetch, non-JSON 502) leaves the outcome genuinely unknown and a second click can deliver the instruction twice on an endpoint with no idempotency key. Split on the same line CopyItemDialog draws (DR-13): a structured PadApiError means the server refused before publishing — re-arm; anything else latches an outcome-unknown state. - `willCollapse` compared against `String.trim()`, reintroducing the very JS-vs-Go whitespace mismatch $lib/push/message exists to avoid (JS trims a leading U+FEFF the server keeps; it leaves a U+0085 the server strips). Added `trimPushMessage`, which trims with Go's class. - The textarea described only the counter, so the collapse note and the over-length error reached no screen reader. Both now live in one stable referenced node that swaps text rather than mounting and unmounting — an aria-describedby pointing at an absent id resolves to nothing. - Positive presence wording implied the count was current. It now says "as of the last check" and names the ~30s window. Test changes: the Go fixture test duplicated `strings.Fields` rather than invoking the handler, so a change to the handler's normalization would have left BOTH suites green — demonstrated by mutating the join separator, which the copied-expression test did not notice and the new handler-driven test caught on 22 cases. The bound is likewise now asserted through the endpoint at 4096/4097 instead of comparing the constant to a copy of itself. Claude-Session: https://claude.ai/code/session_01QCMLhHQBrMHVML3YKdm4Cd * fix(web): fence the push composer against destroyed instances, unrecognised errors, and a frozen count (codex round 2) Three findings, one of them introduced by round 1's own fix: - The send/copy continuation fence used the generation counter, which cannot see a keyed REMOUNT. `{#key itemSlug}` gives item B a fresh instance with its own counter, so item A's in-flight send still saw its own `gen` unchanged and called the SHARED parent `onclose` — closing the composer the user had just opened for B. Added a per-instance `destroyed` flag, which is what actually distinguishes "still mine to close" from "I no longer exist". - The outcome-unknown split treated any PadApiError as proof the server refused before publishing. It isn't: the API client turns EVERY JSON error envelope into one, including a gateway 5xx invented after the handler published. Replaced with a whitelist of codes the handler and its middleware actually emit pre-publish; everything unrecognised is now ambiguous. The asymmetry is deliberate — an unnecessary "we can't tell" costs the user a check, a wrong re-arm delivers twice. - PRESENCE_STALL_MS only rescued the FIRST read. A later poll that hung froze the count at its last value indefinitely while the UI kept rendering "1 session connected" as fact. A known answer now expires to "can't tell" after 30s without a refresh — the server's own presence staleness bound, so past it our answer carries no more authority. Also dropped the status→alert role swap on the composer's live region: changing a live region's role and its text together is not reliably honoured, so the escalation was a promise the markup couldn't keep. The blocking condition rides `aria-invalid` on the textarea instead. The "latest ARRIVED, not latest ISSUED" behaviour of the sequence fence is kept and now documented as a choice: dropping an early-arriving response because a newer request exists strands the UI when that newer request is the one that never settles. Each fix mutation-tested 1:1 against its new test. Claude-Session: https://claude.ai/code/session_01QCMLhHQBrMHVML3YKdm4Cd * fix(web): complete the pre-publish whitelist and retire in-flight polls on expiry (codex round 3) Two of round 3's three findings were real: - `csrf_error` and `email_not_verified` are middleware refusals, written strictly before the handler runs, so they belong in PRE_PUBLISH_ERROR_CODES. Without them a CSRF mismatch told the user we couldn't tell whether their message was sent, when nothing had been. - The 30s staleness expiry didn't fence requests already in flight. A poll issued before the expiry could land after it and reinstate the very count we had just declared too old to trust. Expiry now advances `presenceAppliedSeq` to the current `presenceSeq`, retiring those responses; the poll issued in the same tick carries a newer seq and still applies. The third finding — that `archived` belongs in the whitelist, and that the launcher should be hidden for archived items because "the endpoint always rejects them" — is REFUTED. handlePushToItem has no archived gate (`requireItemVisible` admits archived items), and pushing to an archived item against a running server returns 200 with `pushed: true`. There is no `archived` error code on this path to whitelist, and hiding the launcher would remove a capability that works. Recorded rather than silently skipped so the next reader doesn't re-derive it. Claude-Session: https://claude.ai/code/session_01QCMLhHQBrMHVML3YKdm4Cd	2026-08-15 08:22:09 -04:00