feat(crews): answer a Crew thread's provider approvals and questions from the Inbox - #309
bryantderosier wants to merge 7 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: Jacksondr5/j5code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: Comment |
879b80d to
966519e
Compare
…from the Inbox A Captain's or seat's tool approvals and questions rendered inline in that thread's composer, where the person only saw them by opening the thread, so a Crew stalled unseen. The Inbox gets a Crew agent requests section, read from #263's route on every connected environment: each item names its Crew and seat (or the Captain), links to the thread, and is answered in place with the provider's approval choices or the question form. The bell counts them. On a live Crew thread, one appended J5 hook in ChatView (FORK.md case 44) hands the composer only the requests the Inbox does not hold, and CrewRosterGate shows a note pointing to the Inbox; a request the Inbox has not read yet stays inline, so nothing is hidden from both places. The roster gate still renders inline, non-Crew threads are unchanged, and mobile keeps its inline cards until it has an Inbox. Closes #264 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ead note aligns Found in the seeded dev-server pass: with no provider options the Inbox offered only Approve and Decline while the composer offers Cancel, Decline, Always allow this session, and Approve; the note sat on the composer's edge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
966519e to
bcef305
Compare
Conflicts in crews.md and personas.md: took the sync's handoff artifact wording and kept this branch's Inbox rule. FORK.md: this branch's case is renumbered 44 → 49, since the sync took 44 and 45 and the Captains stack takes 46–48; the inventory count line is left to the Captains stack so the two land without conflicting. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The upstream sync's Badge lint (shadcn/no-restyle) refuses spacing and typography classes on <Badge>; the Crew agent request row's seat badge now uses size="sm" like the other Crew badges. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…rest inline With #308's selector matching the composer, a request can be listed that the Inbox cannot answer: a question answered by sending a message, or one that is no longer resumable. The Inbox now offers answers only for live requests and points to the thread for the rest, and the composer keeps those requests inline instead of handing them to an Inbox that cannot answer them. FORK.md case 46 records the shared selector and the client-runtime test that holds it to derivePendingThreadRequests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| }); | ||
|
|
||
| /** The bell, the Inbox page, and every open thread share this one foreground poll. */ | ||
| export const useCrewRuntimeRequestsRefresh = createVisibleRefreshHook(() => { |
There was a problem hiding this comment.
[Review panel: Opus 5.5 + Astra] performance: this poll runs all the time, even when nothing is pending, and every hit makes the server load a full projection for each live Crew thread.
HumanInboxBell is always mounted and calls useCrewRuntimeRequests(). So every visible web or desktop window sends POST /crews/runtime-requests to every connected environment every 7.5s, including when no Crew exists or nothing is pending, which is nearly always. On the server, #308's list runs listLive and then a full getThreadProjection (all turn items, messages, and runs) for each Captain and seat thread. Two Crews of five seats each means 12 full projection loads every 7.5s per open window, on top of the existing 7.5s crew-proposal poll. #324 narrows the server read but leaves the request count unchanged.
Thread shells already push hasPendingApprovals / hasPendingUserInput for every thread (packages/client-runtime/src/state/models.ts:245). Suggest gating the poll on them:
- Poll an environment only while at least one of its thread shells has either flag set.
- Refresh as soon as a flag turns on or off, plus the existing forced refresh after an answer.
Keep the interval while the environment has pending requests: one pending request can be replaced by another while the flag stays true, so reacting to flag changes alone would miss the replacement. In the steady state, with nothing pending, the result is zero requests.
| ): ReadonlyArray<{ readonly decision: ProviderApprovalDecision; readonly label: string }> => | ||
| options ?? DEFAULT_APPROVAL_CHOICES; | ||
|
|
||
| const REQUEST_KIND_LABEL: Record<string, string> = { |
There was a problem hiding this comment.
[Review panel: Opus 5.5 + Astra] The Inbox approval buttons drop the provider's risk warning. Fix it by reusing the upstream approval components, which also lets this file lose its copied approval UI.
Bug: The approval buttons read only decision and label from each option (approvalChoices also narrows the type to those two fields), so a ProviderApprovalOption.warning never reaches the screen. That field is the provider's caution for an option, such as a prompt-injection warning (packages/contracts/src/providerPolicy.ts:62). The composer's ComposerPendingApprovalActions shows it as a warning icon, a tooltip, and an aria-description (:54-64, :84-96). A Crew approval can only be answered in the Inbox, so the user approves without the risk explanation the composer would have shown. A second, smaller bug: REQUEST_KIND_LABEL has no "permission" entry, so a permission approval is labelled plain "Approval" where the composer says "App permission approval".
Fix, which is also a simplification: mount the upstream components directly. Neither one uses hooks or context, and both take plain props:
ComposerPendingApprovalActionstakes{ requestId, isResponding, canRespond, options, onRespondToApproval }. It brings the warnings, the default choices, and the overflow menu. Its only composer-specific piece iscomposerFloatingLayerProps, a plain data attribute.ComposerPendingApprovalPaneltakes{ approval: PendingApproval, pendingCount }.PendingApprovalisThreadPendingApproval, the shape the selector already produces. The panel brings the kind label, the detail formatting, and the not-resumable text.
The imports come from this J5-owned file, so no upstream file changes. You can then delete DEFAULT_APPROVAL_CHOICES, approvalChoices, REQUEST_KIND_LABEL, and the approval JSX. The question path is tied more closely to the composer, so keeping its own UI with the shared pendingUserInput.ts helpers is reasonable.
|
[Review panel: Opus 5.5 + Astra] Live test evidence for #308 and #309, run by Astra against real provider turns on an isolated dev server. This fills the PR's "Not verified: a real provider turn resuming after an Inbox answer".
Not tested: the "waiting in your Inbox" note on a seat thread (only the Captain's note was checked), the native desktop shell, mobile, and a disconnect in the middle of a flow. Bryant's before-images in the PR description cover the baseline. Captain resumed after the Inbox answer: Seat resumed after the Inbox approval: Stale Inbox answer refused with 409: |
|
Decision (Jackson, 2026-09-26): slim #308/#309 to seat approvals only. See the matching comment on #308 for why. For #309 that means:
|







Important
#309 is based on #308, so #308 merges first. Neither has migrations or ordering against the other Crews PRs.
Problem
A Captain's or seat's provider approvals and questions render only inline in that thread's composer (
ChatViewfeeds every pending request toChatComposer), so I only see them if I open that thread, and the Crew stalls (#264). The Inbox and bell showed agent asks and Crew seat requests only.What I changed
packages/client-runtime/src/j5/http.tsandstate.ts:listCrewRuntimeRequestsandrespondCrewRuntimeRequest, plus a query atom family and a command, polled every 7.5 seconds like Crew gates.apps/web/src/j5/state.ts:crewRuntimeRequestSourcesAtom, read on every connected environment.apps/web/src/j5/crew/crewRuntimeRequests.logic.ts(new):mergeCrewRuntimeRequestSources,inboxRequestIdsForThread,withoutInboxRequests, andinboxBadgeCount.apps/web/src/j5/crew/crewRuntimeRequestsClient.ts(new): the answer call, the forced refresh, the shared poll, anduseJ5CrewRoutedRequests.apps/web/src/j5/crew/CrewRuntimeRequestsSection.tsx(new): the Inbox's "Crew agent requests" section. Each item names its Crew and seat (or Captain) and links to the thread. Approvals show the provider's choices, or Approve and Decline by default; questions reuse upstream'spendingUserInput.tshelpers so answers have the composer's shape.HumanInboxPage.tsxmounts the section and counts it;HumanInboxBell.tsxcounts it throughinboxBadgeCount.apps/web/src/components/ChatView.tsx(upstream, FORK.md case 46): the derived memo is renamedproviderPendingRequestsand passed throughuseJ5CrewRoutedRequests, whose result keeps the namependingRequests.CrewRosterGate.tsx: shows "A request from this agent is waiting in your Inbox" with a link when the Inbox holds requests for the thread; the roster card is unchanged.Screenshots are from an isolated dev server on a copy of my data. The fixture Crew "Release Crew" has the Captain's thread holding a pending question and a builder seat holding a pending command approval. Every thread in the copy points at a provider instance that doesn't exist, so nothing could start a turn.
Seat thread, before (the approval inline) → after (the note):
Captain thread, before (the question inline) → after:
Inbox, before → after (bell: "2 open"):
Answering with no live provider session is refused, and the request stays listed:
The pass turned up two fixes, both in
bcef3054c1: with no provider options, the Inbox offered only Approve and Decline while the composer offers four choices, so it now uses the composer's defaults; and the thread note sat on the composer's edge.Why this shape
The thread hides exactly the request ids the Inbox currently lists for that thread on that environment. So a request the Inbox hasn't read yet, or can't read on a server without #263's route, stays inline. A prompt is never hidden from both places, and the thread doesn't need its own "is this a Crew thread" check. Putting the note in
CrewRosterGate, which ChatView already mounts, keeps the upstream touch to one hook. The upstream approval and question panels are untouched.Invariants
withoutInboxRequestsreturns the same object when no ids match).rosterGatesForThreadis unchanged.Surfaces
packages/contracts)ChatView.tsx, recorded as FORK.md case 44.docs/user/personas.md.Out of scope
j5/main.Upgrade and data
None.
Verification
crewRuntimeRequests.logic.test.tscovers:vp test run apps/web/src/j5/crew apps/web/src/j5/a2a: 245 passed.ChatView.logic.test.ts: 125 passed.set-state-in-effectwarning inHumanInboxPage.tsx.Review focus
useJ5CrewRoutedRequestsinChatView:activeThreadIdis passed even for drafts, which have no Inbox requests. Confirm nothing downstream depends onpendingRequestsidentity changing only with the projection.CrewRuntimeRequestsSectionhandles multi-question requests all at once; check it against the composer panel's behavior.Closes #264
Claude Opus 5.5 via Claude Code
🤖 Generated with Claude Code