Live agent collaboration: the event inbox, honest presence, and every way to wake an agent - #197
Open
HamptonMakes wants to merge 39 commits into
Open
Live agent collaboration: the event inbox, honest presence, and every way to wake an agent#197HamptonMakes wants to merge 39 commits into
HamptonMakes wants to merge 39 commits into
Conversation
…ridge Agents that author plans can now hear about comments the moment they land and collaborate visibly: - AgentEvent inbox + AgentSession (Linear-style pending/active/ awaiting_input/complete/stale) with fan-out from the existing notification choke point; agents never woken by their own activity. - GET /api/v1/agent/events long-poll + SSE with UUIDv7 cursor resume and explicit ack; POST/PATCH/DELETE /api/v1/plans/:id/agent_session drives a live presence pill on the plan masthead. - Content broadcasts now carry changed-section keys; live_update flashes changed blocks with word-level ins/del diffs that settle after ~2.5s. - API ergonomics: GET single comment thread, dismiss route alias (docs said dismiss, router said discard), API threads get the same initial- status rule as the web flow, agent_name on ApiToken. - script/coplan-bridge: harness-agnostic daemon (claude/codex/goose/ openhands/amp adapters + in-process demo agent) that drains the inbox and resumes your local harness session per event. - /agent-instructions documents the realtime loop and session etiquette. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Push-to-talk mic button on the plan page (Web Speech API tier): speaks feedback into a comment, listens for the agent pill to speak "Got it." / "Done — take a look." cues. Hidden when unsupported. - voice/ Pipecat sidecar scaffold (MLX Whisper + Kokoro via OpenAI-compatible endpoint) documenting the higher-fidelity local pipeline and its provider plugin seams. - docs/AGENT_COLLABORATION.md: the live feedback loop, bridge config, per-harness adapter recipes, permission-posture caveat, local demo. - API fix: wrap thread + first comment creation in a transaction so a failed comment (e.g. missing agent_name) can't leave an orphan empty thread; regression spec included. - Bridge: unbuffered stdout, drop a dead line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The bridge assumed the agent had exited and needed a harness resume. The normal case is an agent that is already alive and watching, which doesn't need waking — it needs interrupting. - script/coplan-attach: one held SSE connection, no config file, no adapters, no daemon. --once blocks until the first event, prints a brief, acks, and exits (the shape a turn-based agent wants); bare mode streams. Holds the presence pill while attached, detaches cleanly on exit. - /agent-instructions leads with the held-open stream and states that no daemon or harness integration is required; the bridge is now documented as the cold-start path only. - Event typing: derive comment.created vs comment.replied from whether the comment opens the thread, not from the notification reason — an agent opening a new thread was being reported as a reply. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two defects found by running the loop against a real agent session: - A killed agent left its session `pending` forever, showing a second ghost "Claude is on it…" pill. Staleness was enforced only by MarkStaleAgentSessionJob, and nothing enforces that a job worker is running. `AgentSession.visible` now computes staleness at read time with per-state windows (pending 30s, active 5min, awaiting_input 1hr, falling back to updated_at). The job remains, but only to broadcast the removal promptly — correctness no longer depends on it. - coplan-attach --once detached on exit, clearing the pill at exactly the moment the agent began working, so a slow first turn looked like nothing happening. It now hands the pill off as `active` before exiting instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Measured problem: with three agents attached and RAILS_MAX_THREADS=3, a plain page load timed out after 10s. Every held connection (SSE or long-poll) occupies a Rack thread, and nothing bounded how many agents could hold one. - New AgentEventBus owns two concerns: a held-connection budget (RAILS_MAX_THREADS - 2, override with COPLAN_MAX_AGENT_STREAMS) and wake/notify for waiting connections. - Over budget, long-poll degrades to a non-blocking read with "throttled": true and SSE is refused with 503 + Retry-After, so agents never queue ahead of ordinary requests. Same three-agent load now serves a page in ~25ms. - Waiting is signal-driven instead of a 500ms poll loop: AgentEvents::Publish signals the bus, measured ~180ms end-to-end from comment posted to long-poll returning it. Waiters still wake every CROSS_PROCESS_INTERVAL so writes from other Puma workers are caught. Ceiling is still thread-per-agent — fine for a team, not for hundreds of concurrent agents. Documented in docs/AGENT_COLLABORATION.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All three surfaced while a live Claude Code session worked a plan over the API, not in tests: - agent_name had to be repeated on every comment and reply even though the agent already declared it when claiming its session. It now falls back to the agent session's name, then the token's agent_name, then the token name; an explicit param still wins so one token can post as a different persona. An over-long name is truncated to the 20-char display limit instead of losing the comment mid-conversation. - Comment create/reply returned only comment_id/thread_id, while the rest of the API returns `id` for the created resource. `id` is now included alongside the existing keys. - Comment had no dependent: on its notifications, so destroying one died on a foreign key constraint. Notifications are delivery records for that comment and are deleted with it. Two comments_spec examples asserted the old "reject without agent_name" contract; they now assert the fallback attribution instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The presence pill read "Claude is working…", pulsing, while the agent sat idle on a socket doing nothing. There was no attached-but-idle state, so coplan-attach had to claim something on connect and picked `active`. A pill that animates when nothing is happening trains people to ignore it. - New `watching` state, displayed as "Claude is watching" — no ellipsis, no verb of effort — with muted chrome and a static dot. - agent_session#create honored neither `state` nor `detail`; it hardcoded "active" and dropped the detail. It now accepts both and defaults to `watching`, so claiming means "I'm here", not "I'm busy". - Liveness comes from the stream's own 15s heartbeat touching the session, so a watching pill lasts exactly as long as the connection and expires ~2min after it dies. - watching → pending on a new event. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three bugs found by using the feature: - AgentEvents::Publish woke *every* session for a token, including ones whose process was long dead, so a killed agent's pill reappeared on each new comment. Gate the wake on session.live?; events are still created for detached sessions so the durable inbox is unaffected. - Sessions had no attached-but-idle state, so a watching agent rendered as "Claude is working…" before it had done anything. Add a `watching` state that displays just the agent name with a pulsing green dot. - POST /agent_sessions hardcoded state: "active" and dropped `detail`, and clobbered `awaiting_input` when an agent reattached. Honor both params and preserve awaiting_input on a bare reattach. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two halves of the same question: what is an agent allowed to do, and who is it doing it as. **Authority.** Comment events now carry `from_principal` (the commenter is the human this token belongs to), `from_plan_author` (the commenter wrote the plan), and an `authority` summary of "principal" or "collaborator". The two flags come apart deliberately — an agent can be attached to someone else's plan, and its own principal still outranks that plan's author. The documented contract: a principal's comment may be acted on directly; anyone else gets a reply and a proposal in-thread, not an edit. Because a local_agent comment stores the user behind its token, a second agent working for the same human speaks with that human's authority. **Session tokens.** A token is the unit of event subscription, so two agents sharing one share an inbox and race for each other's wakes — which is why running the multi-agent demo meant hand-creating tokens in the settings UI. POST /api/v1/tokens mints a short-lived child from a long-lived one: same principal (never escalates), 12h default TTL clamped to 7 days, one level deep, and revoking the parent revokes everything it minted. DELETE /api/v1/tokens/current lets an agent clean up its own credential on exit. coplan-attach grows --mint-only (print a token to export for the session) and --mint (mint, use, revoke on exit). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Minting a token per agent run only helps if the token survives the run. An agent that re-minted every turn would get a new inbox and a new presence pill each time; an agent that kept the token in its context would lose it to compaction — and go looking for it in places (shell history, transcripts) where hunting for a credential is exactly the wrong shape. So the token lives in a file keyed to the agent run, and the tools read it: - script/coplan_session.rb stores one minted child token per session key in ~/.coplan/sessions/<key>.json (0700 dir, 0600 file), reusing it until it nears expiry. The key defaults to the working directory — one agent per checkout is the usual shape, and unlike a harness session id it survives compaction and restarts. Harness ids are checked first but not depended on: Claude Code's CLAUDE_SESSION_ID isn't consistently exported to tool subprocesses. - script/coplan is a small authenticated client (get/post/patch/put/ delete, plus reply/say/session/whoami) so no call an agent makes ever carries a token on the command line. A 401 — revoked token, revoked parent — silently re-mints once instead of surfacing an auth error. - coplan-attach uses the same sticky session, replacing the ephemeral --mint flags. The token deliberately outlives the process so successive --once turns share one inbox and one pill. Minting stays best-effort: if the server can't mint (older server, or the caller already holds a session token) everything falls back to the token as given. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dictating a comment worked, but the comment went nowhere visible. An
unanchored thread is filtered out of the plan page entirely
(show.html.erb selects anchored threads; the controller only broadcasts
anchored ones), so a voice note existed in the database, reached agents
through the event inbox, and appeared on the document not at all.
Voice comments are now anchored, by two mechanisms:
- Floor: the heading of the section on screen when you spoke. Cheap,
always available, and roughly where you were looking.
- Better: the AI picks the actual span. A new anchor_suggestions
endpoint sends the model only the visible text plus the transcript and
asks which passage the remark was about, so "this bit is too cautious"
highlights the sentence rather than the section. The model is never
trusted — a span that doesn't appear verbatim in the excerpt is a
paraphrase and is discarded. The call is time-boxed at 4s client-side
and every failure path (no AI configured, slow model, paraphrase,
network) falls back to the heading.
Two fixes found by testing it:
- anchor_occurrence is 1-based server-side (resolve_anchor_position
bails below 1), so a 0-based count silently produced a thread with
anchor_text and no resolved position — a pin pointing at nothing.
- The mic lived in the masthead toolbar, which scrolls away, so you had
to leave the passage you were talking about in order to talk about it.
It's now fixed to the viewport — and rendered at page level, because
.plan-actions sets backdrop-filter and would otherwise become the
containing block and pin it to the toolbar.
The UI also stops promising an agent. It says what happened ("Comment
added to …"); if an agent is attached and picks it up, its pill and
spoken ack say so, and if none is attached this is simply a dictated
comment that waits in the durable inbox.
Covered by a system spec driving the real controller against a stubbed
SpeechRecognition, asserting the highlight resolves to the exact
sentence, that the heading fallback still anchors, and that nothing
claims an agent is coming.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A dictated comment used to land verbatim: "this bit is like way too like cautious". Nobody wants that in a thread with their name on it. InterpretDictation replaces SuggestAnchor and does both jobs in one round trip, because both need the same two inputs — the transcript and what was on screen. It returns the remark cleaned up and the passage it refers to. Neither half is trusted: a span that isn't in the excerpt character-for-character is a paraphrase and can't be highlighted, and a rewrite that changes length dramatically has stopped being a cleanup and started being a summary. Anything that fails falls back to a local tidy-up of what the person actually said, which the server now does too — the client's version never ran, since a raw transcript comes back looking like a successful response. The tidy-up is deliberately timid about "like": it's a real word far more often than a tic, so it only goes where the sentence marks it as filler. Holding Shift starts listening; releasing sends. Shift is also held for capitals and selections, so three guards keep it from firing by accident: Shift alone, held past 350ms, and any other keystroke aborts without posting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two problems, both found by using it. **Nothing got pinned.** The excerpt sent to the model is the rendered text of what was on screen, one block per line. The anchor has to resolve against the markdown source. Those agree for a paragraph and part ways at a table: adjacent cells read as adjacent lines, so the model quoted "Voice (mic button)\n24%", which appears nowhere in "| Voice (mic button) | 24% |". The thread was created with anchor text and no position — a comment that is simply invisible on the page. InterpretDictation now takes the source document as well and narrows a span to the longest line that actually resolves, or gives up so the caller falls back to the section heading. **The transcription was the weak link**, not the pinning. Browser speech recognition is Chrome-only in practice and guesses phonetically at anything domain-specific. The control now prefers MediaRecorder and has the server transcribe with gpt-4o-transcribe, passing the visible text as the decoder prompt so product names and figures come back as themselves. That works in Safari and Firefox too, where the mic previously just hid itself. Recognition stays as the fallback when no provider is configured. The trade is a round trip and no live captions while you talk; accuracy is worth more, since a comment that says something you didn't is worse than no comment. Verified against the real API end to end: "the higher fidelity sidecar stays behind a flag until it is proven" comes back correctly hyphenated from the context hint. The WebM/Opus path Chrome uses is covered by format mapping and specs, not by a live round trip — no ffmpeg here to make one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment on the trunk-based development plan read "Adopt Trunk-Based Development." — the page's own heading, pinned to itself, attributed to someone who had said something else entirely. Whisper-family models answer silence by repeating their prompt, and we prompt with the text that was on screen to get the jargon right. Two seconds of digital silence through CoPlan::Ai.transcribe returns the first line of whatever context it was handed; confirmed twice against the live API. So an empty recording doesn't fail, it fabricates, and what it fabricates is always plausible because it came off the page. Three defences, because one is not enough for a failure that invents content in someone's name: - The browser meters the microphone while recording and won't send a take it heard nothing in. Peak level also drives a ring around the button, which is the only "it can hear you" signal the recording path has — there are no interim captions. - The server rejects any transcript wholly contained in the prompt it sent. Reading a sentence off the page aloud trips this too; being told to repeat yourself beats a comment putting words in your mouth. - Releasing before getUserMedia resolves — the norm on first use, while the permission prompt is up and you are already talking — now says the mic wasn't ready instead of leaving it stuck listening. Metering never blocks a recording: if it can't run, assume speech. Refusing to post what somebody said is the worse failure. Errors now carry the server's wording, since "heard nothing" and "couldn't reach the transcriber" call for different next moves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"It should be main and master", spoken clearly, twice, answered both times with "couldn't hear you". The mic was fine and the recording had the words in it — the silence guard from the previous commit was reading a meter that never turned on. An AudioContext starts suspended unless the browser saw a qualifying user gesture, and Chrome does not count a bare Shift keydown as one. So on the push-to-talk path the metering context routinely comes up dead, and a suspended analyser reads exactly like a silent room: peak 0. The guard meant to stop fabricated comments was throwing away real ones — and only on the hold-to-talk gesture, which made it look like the gesture was broken. The button path worked because a click is a gesture. The meter's verdict now only counts if the context actually reached "running" during the take (plus a resume() nudge for contexts that are merely waiting). A meter that never ran gets no vote and the recording is sent — the server's prompt-echo check remains the defence against true silence, and it distinguishes the cases anyway. Misses are now also spoken, not just printed: "Hmm, didn't hear anything" over speechSynthesis, same words as the status chip. In a voice flow you're talking, not watching a corner of the screen — the silent failure was why two rejected takes in a row read as mystery. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The dev log for today's session: one dictation posted, then three
rejected as prompt echoes in a row. A synthetic-audio experiment cleared
the transcriber — short and even mid-word-clipped real speech
transcribes fine, with or without the prompt. What actually happened:
hold-to-talk opened the microphone only after the 350ms hold delay plus
getUserMedia latency, so for a short remark ("oh, I meant both of
them") the recording began after most of the words. The near-empty tail
echoed the prompt, and the echo guard told the truth: the mic really
didn't hear anything. The bug was never in the guards — it was that the
capture missed the speech they were guarding.
Push-to-talk now opens an "ear" at the Shift keydown itself: stream and
recorder start buffering immediately, silently, and confirming the hold
adopts a capture already in progress. A tap, a Shift-selection, or a
shortcut discards the take unheard — the cost of a false start is a
blink of the recording indicator. Releasing while the mic is still
opening posts whatever the ear caught instead of demanding a retry.
The retry-without-prompt alternative was tested and rejected: silence
transcribed with no prompt returns hallucinated filler ("Na przykład"),
which would post gibberish instead. Echo rejections are now logged with
the transcript so the next false positive is diagnosable from the log.
Also, since dictation is conversational in a way typing isn't, the
interpreter now receives the last three comments — "oh, I meant both of
them" has no "them" without the comment it follows. And the mic button
reads as a recorder: filled red while listening, level ring, 18px glyph.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"What does 'releasable' mean?" transcribed perfectly and landed pinned
to the page's H1 — under the mark already owned by an earlier comment,
where the highlighter's one-thread-per-mark rule made it unreachable.
The span was fine; the resolution wasn't. The model reads rendered text
("main is always releasable") and the anchor must resolve against
markdown ("`main` is always releasable"), so any span crossing inline
markup was rejected whole, and rejection means the heading fallback.
The table fix only handled multi-line spans. Now resolution degrades in
steps: whole span, whole lines, then the longest contiguous run of
words still present in the source — here "is always releasable", which
pins to the sentence being asked about. Runs under 8 characters don't
count: a pin on a salvaged "the" points at noise.
One thread per mark remains the rule when two threads genuinely claim
the same range — overlap handling is its own piece of work. This change
just stops manufacturing collisions out of comments that named their
own, distinct target.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e resolver can place
A comment whose anchor never resolved renders nowhere — no highlight, no
popover, no way to reach it — yet we created it and said "comment
posted". Three changes close that hole:
- CommentThread validates on create that a present anchor resolved to
positions; resolution moves to before_validation so validation can see
it. Content drift after create stays the out_of_date flow.
- InterpretDictation#anchorable now checks spans against the resolver's
own stripped-markdown translation instead of a raw-substring test that
was stricter than the resolver — "main is always releasable" survives
backticks whole instead of being narrowed past the word it points at.
- The resolver learns one more capture shape: mermaid labels broken by
literal <br/> tags select as concatenated text ("firstfetching"), now
matched by dropping the tags with the position map carried along.
On refusal, the HTML selection form keeps the draft and shows the error
inline; the voice client retries once with its viewport-heading anchor
before giving up; the JSON API already returned 422 for RecordInvalid.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* origin/main: Refuse comments whose anchor doesn't resolve — no pin, no comment (#173) Make ai_model and ai_base_url actually do something (#171) Fix overlapping comment highlights (#172) Make the engine portable across MySQL and PostgreSQL hosts (#170) # Conflicts: # engine/app/controllers/coplan/comment_threads_controller.rb # engine/app/services/coplan/ai_providers/open_ai.rb
…ntersecting
Two fixes from the same field report ("it's attaching this last comment
to a place I can't even see"):
- _visibleText counted any block intersecting the viewport, so a
paragraph with one line poking over the fold was quotable and the pin
could land below anything the speaker had read. A block now has to be
readably on screen — most of itself visible, or a real slice of the
viewport.
- Nothing showed where a voice comment landed. The speaker never chose a
spot — the model did — so the client now renders the create response's
streams immediately, then scrolls to the thread and opens its popover
(same treatment as arriving via ?thread=ID, via a coplan:open-thread
event the text-selection controller listens for).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s actions The excerpt was a snapshot of the viewport at the moment talking began. People start on one paragraph and scroll to another mid-sentence, so it now accumulates every block that was readably on screen at any point — sampled at start, on each listening tick, and once more at submit. The last thing you looked at is always in; what never crossed the screen stays off the wire. Document order and live-update survival come from filtering a fresh block list against the seen set rather than serializing the set. The auto-open handoff also drops its hand-rolled document listener for the idiom this element already uses: the voice controller dispatches coplan:open-thread via this.dispatch, and show.html.erb routes it with a data-action descriptor next to keydown.esc@document. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ents
The interpret contract widens from one {text, span} to
{"comments": [{text, span}, ...]}, capped at four. Repeating a span is
how two copies of the same text are addressed: the client bumps the
occurrence for each repeat, so both mentions of "main" get their own
pin. Every span is vetted individually — a paraphrase falls while its
siblings stand — and the length trust band applies to the rewrite as a
whole, with flat per-comment headroom (standing alone costs roughly a
sentence per split, a constant, not a multiple of the remark).
The client posts each comment with the existing per-comment fallbacks —
except the heading fallback, which belongs to the remark as a whole and
only the first comment takes; the rest post unpinned rather than piling
onto one heading. The first thread auto-opens and the status counts the
rest. The old singular body/anchor_text keys stay in the dictation
response as the first comment.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gpt-4o-transcribe is a chat model with ears: given an instruction-shaped remark and a context prompt, it sometimes answers instead of transcribing. Live, 'add some content about how editing works' came back as a whole essay with figures lifted from the prompt, and posted as though the person had said it. The existing guards missed it by design: the echo check catches transcripts contained in the prompt, not ones expanded from it, and the interpreter's length band compares cleanup to transcript — and the transcript already was the essay. The physical bound is the fix: the client now sends how long the take was, and a transcript past ~30 chars/sec was generated, not heard. First response: retry without the prompt, which leaves the model nothing to answer from. If the retry still outruns the clock, 422 'Didn't catch that' — the client already voices that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The coplan--comment-nav div was gated on @threads.any?, decided at page render. But comments arrive live — voice dictation, selection comments, other viewers' broadcasts — so on a fresh plan the first comment appeared, its popover auto-opened, and d/j/k/r/a/s did nothing at all: the controller holding the key listeners was never on the page. Render it unconditionally; it no-ops fine with zero highlights. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * origin/main: Voice commenting: push-to-talk dictation pinned to what you were reading (#175) Add agent-first library organization API (#176) Stabilize human editing system spec (#174) # Conflicts: # db/schema.rb # engine/app/assets/stylesheets/coplan/application.css # engine/app/controllers/coplan/dictations_controller.rb # engine/app/javascript/controllers/coplan/voice_controller.js # engine/app/views/coplan/plans/_voice_control.html.erb # spec/requests/dictations_spec.rb
Two live failures from Hampton's testing:
- 'Hmmm, not enough information on tax attach' came back as 'I will
add more information' — the model replied to the remark instead of
editing it, signing a promise in the speaker's name. The prompt now
frames the job as writing the comment the speaker would have typed
(their voice, their point of view, appears under their name), with
that exact failure as a counter-example, and notes the remark
usually names its own target ('tax attach' → the passage about it).
- Anchors kept landing just above the visible area. The excerpt
accumulates everything that scrolled past during the take, and the
model had no idea which part was actually being read. The client
now samples the blocks crossing the middle band of the viewport
(people read the middle of the screen) at the moment of release and
sends them as 'focus'; the prompt marks that as the span's likeliest
home. Omitted when it would just repeat the excerpt.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* origin/main: Teach agents to file, type, and template new plans (#182) Complete agent attribution coverage (#181) Agent identity: session tokens, Bearer-only API, honest history attribution (#178) Add containing-folder navigation (#179) Require a plan type on every plan, defaulting to General (#180) Add rich reference previews and back matter (#177) # Conflicts: # engine/app/controllers/coplan/api/v1/comments_controller.rb # engine/app/controllers/coplan/api/v1/tokens_controller.rb # engine/app/models/coplan/api_token.rb # engine/app/models/coplan/comment.rb # engine/app/views/coplan/agent_instructions/show.text.erb # engine/config/routes.rb # spec/models/api_token_spec.rb # spec/requests/api/v1/comments_spec.rb # spec/requests/api/v1/tokens_spec.rb
The merge-back of main (#177–#182) brought the token identity work this branch originally drafted, evolved: hook-auth bootstrap minting, Bearer-required API, token metadata, api_token_id provenance. This commit finishes the reconciliation: - Self-wake suppression now keys on api_token_id explicitly. AgentEvents::Publish takes actor_token_id; comment events suppress on the comment's api_token_id and content events on the version writer's token. The old actor_id matching silently broke when attribution started storing the human's id — an agent would have been woken by its own edits. - Agent name resolution is token-first everywhere via api_agent_name, so an agent can't sign comments under one label while its versions carry another. The session-claim name no longer leaks into writes. - Dropped the branch's superseded token minting draft (controller, model methods, add_parent migration) in favor of main's. - Guarded the agent-tables migration: tables may pre-exist on schema-loaded databases, and api_tokens.agent_name ships with the identity migration on hosts that install this one later. - Removed per-controller token checks that BaseController's require_api_token! made unreachable. Suite: 1554 examples, 0 failures (incl. system). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The timeout was only checked between reconnects, but the server holds an SSE stream open for minutes with heartbeats — so "--timeout 6" actually meant "when the server retires the stream". Now the remaining budget becomes the socket read deadline; Net::ReadTimeout loops back to the top, where the check exits 64 as documented. Verified live: --timeout 6 exits 64 at ~6s; --once wakes on a comment in under a second, prints the brief, hands the pill to active, exit 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Field-tested by an Amp local thread: it held the SSE stream, received every event, and its model never woke — while the pill said "Amp is on it…". Delivery is not action, and no state copy should promise more than the server can verify. - pending now renders "Waking <agent>…" — what the server actually did. "On it" is the agent's own claim, made by PATCHing active. - Claims can only arrive in watching or active: a fresh session claimed into awaiting_input parked an unearned "asked a question" pill for up to its hour-long stale window. - pending and stale are now rejected via PATCH too — both are verdicts the server reaches about the agent, not reports an agent can file. - /agent-instructions and docs/AGENT_COLLABORATION.md spell out the harness requirement the loop stands on: event arrival must become a model turn (blocking tool call / background-exit re-invocation / sidecar resume), else drain the durable inbox per turn. Amp's webhook ask is recorded as an open design question, not built. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The pill only knows two true things: whether a connection is parked (SSE heartbeats and long-poll parks now both stamp a transport clock — long-poll agents previously read as absent and went stale mid-loyalty), and whether this session has ever turned a delivery into a model turn. So an event only starts the 30-second pending countdown when transport is live or a wake URL is registered, and the pill only says "Waking Claude…" once a wake has been answered before — the first one is quietly a test, shown as plain presence. New wake path for agents that can't hold a connection or be resumed: claim with a wake_url and CoPlan POSTs a signed "you have inbox items" ping per event (HMAC secret minted once at registration, event_id for dedupe, retries with backoff, fires even on complete sessions — waking them is the point). The ping carries no payload: the agent pulls and acks through the cursor API, so at-least-once and authority don't fork. Fits Amp orbs' createWebhook exactly. The bridge grows an "acp" adapter: one live agent subprocess speaking the Agent Client Protocol (initialize → session/new → session/prompt per event, permission asks answered per config), replacing per-harness resume dialects for everything in the ACP registry — goose acp, Gemini --acp, claude-agent-acp, codex-acp, amp-acp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An adversarial review of the wake plumbing confirmed eleven defects; this fixes all of them. The headline: MarkStaleAgentSessionJob had never fired. wake! passed woken_at through bare iso8601, truncating to whole seconds, while the column is datetime(6) — so the wake's own transition always read as "activity after the wake" and the 30-second retraction behind every pending pill was a dead letter. woken_at now carries microseconds, and the job finally has a spec (including the mid-second regression case). Honesty of the wake proof: - Only active/awaiting_input count as answering a wake (PROOF_STATES). complete and watching are filed mechanically by detach paths and supervising loops, and were marking dead harnesses wake-proven. - coplan-attach --once no longer PATCHes active on its way out for the same reason: it cannot know whether its exit wakes anything. The woken model's own fast-ack takes the pill over, or pending goes stale in 30s — now truthfully. - Instructions spell out "PATCH, don't re-claim, when answering a wake": a fresh claim resets the session instead of answering it. Safety of the webhook egress: - New CoPlan::WakeUrlPolicy: wake URLs must resolve entirely to public address space (SSRF; one private A record among public ones is refused too). Checked at registration and re-checked before every POST, since DNS may change its answer between the two. Hosts override via config.wake_url_policy; dev/test allow localhost. - DeliveryFailed messages carry the session id, never the URL — the URL can embed a capability token and the messages land in logs and solid_queue_failed_executions. - Rescue now covers EOFError (via IOError), Net::ProtocolError, and Net::HTTPBadResponse, which previously escaped straight to the failed-executions table. - Dead-URL circuit breaker: wake_failures_count tracks exhausted retry runs; after three, the URL and secret are unregistered — mirroring how expired web push subscriptions are destroyed, not hammered. - The enqueue moved inside ActiveRecord.after_all_transactions_commit: the queue lives in a separate database, so a worker could pick the ping up before the AgentEvent row was visible and no-op the wake. Robustness of the ACP bridge: - pump_until now shares one deadline per turn (acp_turn_timeout, default 600s) via IO.select; a wedged agent is killed instead of wedging the bridge on a blocking read forever. - A replayed prompt after a respawn is prefixed with a recovery note so a half-completed first attempt doesn't get duplicated replies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ge flags A new agent meeting a CoPlan server previously needed a repo checkout to go live. Now the server hands over everything itself. - The agent scripts move into the engine (engine/agent_tools/) and every CoPlan deployment serves them read-only at /agent-tools/coplan-attach, /agent-tools/coplan_session.rb, and /agent-tools/coplan-bridge — whitelisted names only, rendered inline for the same X-Sendfile reason as the service worker, public for the same reason /agent-instructions is. Thin shims keep script/* working for local development. - /agent-instructions gains "Setup: Your First Five Minutes as a Live Agent": download the tools with curl, then follow the one branch that matches what your harness can do — background attach (exit is the wake), blocking attach, ACP bridge, webhook wake, or per-turn inbox drain. Verified end-to-end: fresh directory, two curls, one command, clean exit 64 on a quiet plan. - coplan-bridge learns flags so the simple path needs no config file: --acp "goose acp" --plan <id> --name Goose (plus --adapter/--session/ --base/--token/--approve). Flags win over the config file; base URL and token fall back to COPLAN_BASE/COPLAN_TOKEN. One deliberate behavior change: the adapter must now be named explicitly — the old silent default was "demo", which replies to threads and edits the plan, a rude surprise for anyone running a freshly downloaded script with only a --plan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An adversarial review of the first-encounter bootstrap found seven
real defects, all in the seams between flags, config files, and
deployment shapes:
- A leftover ~/.config/coplan/bridge.json silently steered flags-only
runs: the file's base_url/token beat $COPLAN_BASE/$COPLAN_TOKEN, so a
stale credential could claim sessions nobody asked for. Precedence is
now flags > ENV > file, and the bridge announces which config file it
loaded. Malformed JSON aborts naming the file instead of raising.
- --plan unioned with the file's plan list instead of replacing it, so
"watch this one plan" quietly watched the old ones too.
- The adapter was validated at the first wake, not at startup: a typo'd
--adapter claimed sessions, flipped pills active, then died hours
later on the first event. Same for adapter acp with no command, and
--acp "" (an unset shell var) turned every event into ack-and-lose.
All four now abort before anything is claimed.
- URI.join discards an engine mount prefix ("http://host/coplan" +
"/api/v1/..." → "http://host/api/v1/..."), which broke every served
script on prefix-mounted deployments — token minting 404'd silently
and SSE aborted. All four scripts now concatenate.
- Setup branch C rooted the ACP agent in ~/.coplan/bin via a chained
cd; the bridge now takes --cwd, and the instructions say to run it
from the directory the agent should work in.
- Instructions referenced sections by names that don't exist and
pointed at repo paths for revocation; both now reference the served
/agent-tools/ scripts and real section titles.
Verified end-to-end with a fixture stale config in a fake HOME: ENV
wins, plans replace, the announcement prints, and all four startup
aborts fire before any session is claimed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "first five minutes" section told a brand-new agent to download Ruby scripts from the server and execute them — backwards on two counts Hampton called out: Ruby is not on every machine, and agents should not be taught to blindly run network-fetched code. Flipped: the section now opens with the three HTTP calls the live loop actually is (claim, wait, ack), gives the branch-A wake as a seven-line shell loop any agent can port to its own runtime, and tells the agent to make the wiring durable — save it as a skill or saved command, a custom tool, or a standing ACP bridge config, so tomorrow's run attaches with one action instead of rediscovering the page. The served scripts drop to an optional convenience behind a read-before-you-run checklist scoped per script (attach + helper: this server only, writes only under ~/.coplan/$COPLAN_HOME, no subprocesses; the bridge: reads its config file and execs exactly the agent command you configured), with the download and the run in separate blocks so the paste-able unit never fetch-then-executes. An adversarial review of the rewrite (4 lenses, 18 findings) caught the checklist being literally false for the bridge, the example loop spinning silently forever on a dead token (now it exits loudly — a process exit is the wake either way), a literal-secret export in the one place the page otherwise keeps tokens out of command lines, and curl -O saving error pages as scripts (now -f, verified empirically). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hread silence) into agent-collab Conflict notes: agent-tools route re-rooted alongside agent-instructions as a published address; agent event publication stays ahead of (and outside) the new closed-thread notification lock — silence is for human unread rows, agents still hear replies and closes; wake_url_policy and reserved_handles config accessors both kept. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ChangedSections now returns keys + rewritten (#192); the agent event payload carries that richer shape and the specs assert it. The URL catchall (#191) swallows the encoded-slash traversal probe before the agent-tools route sees it, so the spec now asserts the property that matters (no file leaves the server) plus a direct single-segment probe. agent-tools joins the reserved handles next to agent-instructions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07cac44e23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| let changedKeys = [] | ||
| try { | ||
| changedKeys = JSON.parse(this.getAttribute("data-changed-sections") || "[]") |
There was a problem hiding this comment.
Broadcaster.replace_plan_content serializes a Plans::ChangedSections::Result, so this parses to an object such as { "keys": [...], "rewritten": false }, not the array this controller expects. snapshotSections subsequently evaluates new Set(keys) and throws because that object is not iterable, aborting before replaceChildren; consequently, viewing tabs do not receive live content edits. Pass changed_sections.keys in the attribute or explicitly read the parsed object's keys.
AGENTS.md reference: AGENTS.md:L76-L82
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
🤖 Fixed in 01cdf1a — confirmed real: the attribute carries the ChangedSections::Result JSON, so new Set(parsed) threw and viewer tabs never rendered remote edits. The controller now reads parsed.keys (bare arrays still accepted for older servers), and a rewrite arrives with no keys so the swap stays flash-free rather than lighting up the whole document.
| end | ||
| if (next_cursor = resp["cursor"]) && next_cursor != cursor | ||
| cursor = next_cursor | ||
| request(:post, "/api/v1/agent/events/ack", body: { cursor: cursor }) |
There was a problem hiding this comment.
When dispatch(event) raises, the per-event rescue logs and continues, but this request-level cursor is still acknowledged afterward. Because the ack endpoint marks every event through that cursor processed, a harness failure, timeout, or nonzero adapter error permanently drops the failed event—and possibly earlier failed events in the same batch—instead of preserving the advertised at-least-once delivery. Advance and ack the cursor only through the last successfully handled event.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
🤖 Fixed in 01cdf1a. The bridge now acks each event individually after its dispatch returns, and a failed dispatch stops the batch (acking later events would ack the failed one too — the cursor is a high-water mark). The next poll redelivers everything unacked; a 3s sleep keeps a wedged adapter from spinning.
Comment on lines
+87
to
+90
| AgentEvents::Publish.call( | ||
| plan: @comment_thread.plan, | ||
| event_type: event_type, | ||
| actor_token_id: @comment&.api_token_id, |
There was a problem hiding this comment.
For status_change, the notification job has no comment, so @comment&.api_token_id is always nil even when an agent performed the resolve/discard action. The event therefore fans back out to that agent's own inbox; a bridge can finish its turn and then start another model turn from its own status update, while webhook-backed sessions may be woken unnecessarily. Propagate the API token ID through the status-change job and use it here for self-event suppression.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
🤖 Fixed in 01cdf1a. The API resolve/discard endpoints now pass actor_api_token_id through CreateNotificationsJob into Notifications::Create, and self-suppression keys on comment&.api_token_id || actor_api_token_id. An agent resolving a thread no longer starts a turn from its own status event. Human resolves still fan out (they should — that's news to an attached agent).
Comment on lines
+76
to
+78
| response = Net::HTTP.start(uri.hostname, uri.port, | ||
| use_ssl: uri.scheme == "https", open_timeout: OPEN_TIMEOUT, read_timeout: READ_TIMEOUT) do |http| | ||
| http.request(request) |
There was a problem hiding this comment.
For an attacker-controlled wake hostname, WakeUrlPolicy.allowed? resolves and validates one set of addresses, but those addresses are discarded and Net::HTTP.start resolves the hostname again. A DNS-rebinding host can answer publicly during the policy check and return a private or link-local address for the connection, bypassing the SSRF protection and reaching internal services. Pin the connection to an address returned by the policy check while retaining the original hostname for Host/SNI.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
🤖 Fixed in 01cdf1a. WakeUrlPolicy now exposes vetted_addresses (resolve-and-vet in one step) and the job pins the connection via Net::HTTP's ipaddr: to an address the check actually approved, keeping the hostname for Host/SNI/cert verification. A configured custom policy vets URIs rather than addresses, so it comes back :unpinned and resolves normally — that override is the deployment's own trust decision. Spec asserts the pin.
| } | ||
|
|
||
| if (flashed.length > 0) { | ||
| flashed[0].scrollIntoView({ behavior: "smooth", block: "nearest" }) |
There was a problem hiding this comment.
Whenever a remote edit changes an off-screen section, this unconditionally scrolls the first changed block into view, pulling a reader away from the passage they are reviewing even though they initiated no navigation. The section can still flash without moving the viewport; scrolling should require an explicit user action.
AGENTS.md reference: AGENTS.md:L76-L82
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
🤖 Fixed in 01cdf1a — the scrollIntoView is gone, with a comment making it deliberate: a remote edit must never move a reader who didn't ask to navigate. On-screen changes flash; off-screen ones settle unseen.
| # (or its bridge daemon) drains via GET /api/v1/agent/events. IDs are | ||
| # UUIDv7, so lexicographic order is creation order and the id doubles as | ||
| # the resume cursor. | ||
| class AgentEvent < ApplicationRecord |
There was a problem hiding this comment.
A repo-wide check of app/admin/ finds no registration for either new persisted model, AgentEvent or AgentSession, and neither model defines the required ransackable_attributes/ransackable_associations. This leaves collaboration inboxes and live sessions unavailable to the established administrative interface, including when operators need to inspect stuck or stale rows.
AGENTS.md reference: AGENTS.md:L114-L117
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
🤖 Fixed in 01cdf1a. Both models registered in app/admin/ with ransackable allowlists. AgentEvent is index/show only (inbox rows are platform-written); AgentSession keeps destroy so an operator can clear a stuck row. wake_secret is deliberately excluded from both the allowlist and the show page — it's a credential, not a search key.
Comment on lines
+27
to
+28
| add_foreign_key :coplan_agent_events, :coplan_api_tokens, column: :api_token_id | ||
| add_foreign_key :coplan_agent_events, :coplan_plans, column: :plan_id |
There was a problem hiding this comment.
These foreign keys use the default restrictive delete behavior, while neither Plan nor ApiToken declares dependent associations for the new event/session rows. Once a plan or token has collaboration data, deleting that plan through ActiveAdmin or destroying its owning user (which destroys API tokens) raises a foreign-key violation. Add dependent cleanup associations or database cascades for both collaboration tables.
Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment.
🤖 Fixed in 01cdf1a. Plan and ApiToken now delete_all their agent_events/agent_sessions before their own row goes (transient state, no teardown of its own, so delete_all over destroy). Spec covers both parents — with a note that bare plan.destroy! was already impossible for an unrelated pre-existing reason (the current_plan_version self-reference), which this doesn't try to solve.
- Live updates broke on main's ChangedSections struct: the stream
attribute now parses {keys, rewritten} (bare arrays still accepted),
so viewer tabs render remote edits again; a rewrite arrives with no
keys and stays flash-free by design.
- The flash no longer scrolls: a remote edit must never move a reader
who didn't ask to navigate.
- The bridge acks per dispatched event and stops the batch on failure —
the request-level ack was silently swallowing events whose harness
dispatch raised, breaking at-least-once.
- Status changes carry the acting token through CreateNotificationsJob
into self-suppression: an agent that resolves a thread no longer
wakes itself from its own status event.
- Wake webhooks connect to the address the policy vetted (ipaddr pin,
hostname kept for Host/SNI): resolving twice let a rebinding host
answer the check publicly and the connection privately.
- AgentEvent and AgentSession get ActiveAdmin registrations and
ransackable allowlists (wake_secret deliberately excluded).
- Plans and API tokens delete their collaboration rows (delete_all)
instead of raising on the new FKs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
The live feedback loop: a human comments on a plan, the agent that authored it hears about it within a second, shows a presence pill, replies on the thread, and edits the document while every open tab watches the changed sections flash. 58 files, built and field-tested incrementally over three weeks.
The pieces
Agent event inbox (
AgentEvent,GET /api/v1/agent/events). Every comment, reply, status change, and content edit fans out to the inbox of each agent session on the plan — never back to the actor itself. Pull-based on purpose (agents live on laptops behind NAT): one endpoint serves long-poll and SSE, cursor = last event id, delivery is at-least-once with explicit ack. Signal-driven, not polled: ~180ms comment-to-delivery.Presence that can't lie. The
AgentSessionstate machine (watching/pending/active/awaiting_input/complete) drives the pill humans see, on two principles: a wake is only attempted where a path for it exists (recent transport or a registered wake URL), and wakeability is demonstrated, never declared — "Waking Claude…" only appears after Claude has answered a wake before. Staleness is computed at read time, so a dead agent's pill retracts (30s frompending) even with no job worker running. The API refuses to claim a session directly into a turn state: no unearned "asked a question" pill on arrival.Capacity budget, measured. Held connections each occupy a Rack thread; three attached agents with default threads made page loads time out entirely.
AgentEventBuscaps held connections atRAILS_MAX_THREADS - 2and degrades honestly past it (long-poll answersthrottled: true, SSE gets 503 + Retry-After). Same three-agent load after the cap: ~25ms page loads. The known ceiling is thread-per-agent — right for a team, would want real pub/sub for hundreds.Authority model. Comment events carry
from_principal/from_plan_author/authority, and the instructions set the contract: your own principal's comment → act directly; anyone else's → propose in the thread and wait. A drive-by comment is a request, not an instruction.Every way to wake an agent, because delivery is not a wake — transport hands a process the event; starting a model turn is the harness's job:
coplan-attach— one held SSE socket;--onceblocks, prints one event brief, acks, exits (the shape a turn-based harness wants, foreground or background)wake_urlat claim; CoPlan POSTs an HMAC-signed, metadata-only ping (the agent still pulls and acks through the cursor API, so at-least-once and authority don't fork)coplan-bridge— cold-start sidecar: pushes each event into a live agent over ACP (--acp "goose acp", anything in the ACP registry) or a per-harness resume commandwait=0each turn and you're correct, just not liveFirst encounter is curl-first.
/agent-instructionsgained a "Setup: Your First Five Minutes" section that leads with the raw three-call loop (claim, wait, ack — no downloads, no Ruby), tells the agent to save the wiring as a durable skill/tool/ACP config, and only then offers the served reference scripts (/agent-tools/…) behind an explicit read-before-you-run checklist scoped per script. Agents are not taught to blindly execute network-fetched code.Security posture. Wake URLs must resolve to public address space, checked at registration and before every POST (DNS rebinding); deployments override via
config.wake_url_policy.wake_secretis shown once; ping failures are redacted (session id, never the capability URL) and a URL that eats whole retry runs is presumed dead and unregistered — mirroring expired web-push subscriptions. Each agent run holds its own short-lived minted token (on #178's identity layer), so revocation is per-run and attribution survives.What's proven vs. what isn't
Field-tested: the attach loop end-to-end (comment → wake → reply → edit < 1s via Claude Code background-exit), and an Amp local thread that held SSE for a full session — which is also the honest caveat: Amp received every event and its model never woke, because most harnesses can't turn a background process's output into a turn. That failure mode is why presence is designed not to over-promise. The webhook wake and ACP bridge are spec-tested and adversarially reviewed but have not yet been run against a real hosted agent or a real ACP harness in anger — treat them as launch-and-learn.
Review history
Three adversarial review rounds fixed 30+ confirmed defects before this PR, including:
MarkStaleAgentSessionJobnever firing (iso8601 truncation vsdatetime(6)— the 30s pill retraction was a dead letter), self-wake suppression keyed on the wrong id post-#178 (agents woken by their own edits),--timeoutunable to fire mid-SSE-stream, wake-proof poisoning from mechanical state changes, SSRF via wake URLs, a leftover bridge config silently hijacking flags-only runs, andURI.joindiscarding engine mount prefixes in every served script.Test plan
🤖 Generated with Claude Code