Skip to content

feat(crews): a Crew thread's provider approvals and questions reach the Inbox API - #308

Merged
bryantderosier merged 9 commits into
j5/mainfrom
j5/crew-requests-server
Sep 28, 2026
Merged

bryantderosier merged 9 commits into
j5/mainfrom
j5/crew-requests-server

Conversation

@bryantderosier

@bryantderosier bryantderosier commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Important

⚠️ Merge order: merge these in this exact order

#309 is based on #308, so #308 merges first. Neither has migrations or ordering against the other Crews PRs.

  1. feat(crews): a Crew thread's provider approvals and questions reach the Inbox API #308: A Crew thread's provider approvals and questions reach the Inbox API ⬅️ this PR
  2. feat(crews): answer a Crew thread's provider approvals and questions from the Inbox #309: Answer a Crew thread's provider approvals and questions from the Inbox
  3. feat(crews): seats ask their Captain instead of the person #348: seats ask their Captain instead of the person (ship with feat(crews): answer a Crew thread's provider approvals and questions from the Inbox #309)

Scope now: seat approvals only (Jackson, 2026-09-26)

The sections below describe the original scope and have been narrowed; this summary is current.

  • The service lists and answers approvals on seat threads only. The Captain's thread is excluded: its approvals and questions stay inline where the person is watching.
  • No questions. The user_input path is removed from respond, the contract (CrewRuntimeRequestItem is now an approval item; the respond request requires decision), and the selector (inboxAnswerableApprovals in packages/shared/src/j5/crewRuntimeRequests.ts).
  • Only live approvals are listed; the rest stay in their thread and don't count on the bell.
  • A thread named by several Crew records is listed once.
  • FORK.md case 46 records the shared export and the parity test's dependency on derivePendingThreadRequests.
  • Store failures still propagate as 500s. A dispatch refusal is a 409; any other dispatch failure is CrewRuntimeRequestDispatchError (500).

Problem

When a thread is a Crew Captain or a live seat, every approval or question its provider raises (tool approvals, and user-input questions like Claude's AskUserQuestion or Codex's request_user_input) renders only in that thread's composer. Captains and seats run while I'm elsewhere, so those prompts sit unseen and the Crew stalls (#263). My rule: anything a Captain or seat needs from me goes to my Inbox; the initial roster card is the one exception.

What I changed

  • apps/server/src/j5/a2a/CrewRuntimeRequestService.ts (new):
    • list reads the pending requests on every live Crew's Captain and seat threads straight from their projections.
    • pendingCrewThreadRequests does the selection, mirroring client-runtime's derivePendingThreadRequests, which the server can't import: pending only, questions with their item, and no auth_refresh or dynamic_tool_call.
    • respond checks that the thread belongs to a live Crew and the request is still pending, then dispatches runtime-request.respond.
  • apps/server/src/j5/a2a/CrewRuntimeRequestsHttp.ts (new): POST /api/j5/a2a/crews/runtime-requests (read scope) and .../respond (operate scope). 404 for anything outside a live Crew, 409 once resolved, 400 for the wrong answer kind.
  • packages/contracts/src/j5.ts: CrewRuntimeRequestItem, the list response, and the respond request and response; both paths are added to J5_API_PATHS.
  • Wiring: the service in j5/a2a/runtimeLayer.ts, the routes in J5AuthenticatedRoutes.ts, and the two aggregate tests gain a service mock.

After Tyler's review

  • Thread reads go through getThreadProjectionIfPresent, so only a real not-found or an archived thread reads as absent. A store or listLive failure now propagates: list returns a logged 500 instead of an empty Inbox, and respond no longer calls a live request not found.
  • respond looks requests up through the same selector as list. Hidden kinds and questions without their item are never answered, and a request that isn't live is refused with the way out.
  • A dispatch refusal carrying the decider's string reason is a 409. Any other dispatch failure is CrewRuntimeRequestDispatchError, a logged 500.
  • list reads a Crew's threads concurrently. The narrower read is Crew request polls read only pending runtime requests #324.
  • The selector moved to J5-owned packages/shared/src/j5/crewRuntimeRequests.ts and matches derivePendingThreadRequests. packages/client-runtime/src/j5/crewRuntimeRequests.test.ts runs both over every branch and fails if they drift.

Why this shape

I serve this as its own Inbox source rather than as ledger Exchanges: HumanInboxService.answer writes an A2A reply, not a provider response. I read live state from the projection rather than copying it into a table, so there's nothing to keep in sync and a resolved or retired request just stops appearing. Each answer gets a fresh command id, so a second answer is refused by the request's own state instead of being silently deduplicated.

Invariants

  • Only live Crews count: a thread outside every live Crew is never listed or answered, and a retired Crew's requests leave the list.
  • An answer to a request that already resolved, including one answered inline on another device, is refused and dispatches nothing. A race lost between our read and the command is refused by the orchestrator and mapped to 409.
  • A reserved seat with no thread, or an archived thread, is skipped.

Surfaces

Surface Decision
Entry points (chat, Settings, command palette, keybinding) Unaffected; the web Inbox is #264 (next in this stack).
Clients (web, desktop, mobile) Server and contract only here.
Providers Provider-agnostic: it reads the orchestrator's runtime requests.
Contracts (packages/contracts) J5 contract additions only.
Reverse states A request leaves the list once resolved or its Crew retires.
Connection modes (local, remote, tunnel) Same authenticated J5 route pattern as the Crew stop and proposal routes; POST for the CORS allowlist.
Upstream files / FORK.md None.
Docs In #264.

Out of scope

Upgrade and data

None. No table, no migration.

Verification

  • CrewRuntimeRequestService.test.ts uses the real Crew store and a thread mock that resolves a request once, as the orchestrator does. It covers:
    • seat and Captain requests listed with their Crew and seat;
    • the stranger thread never listed;
    • one answer resolving and a second refused with nothing dispatched;
    • an answer after an inline answer refused;
    • a wrong-kind answer refused;
    • a retired Crew leaving the list.
  • CrewRuntimeRequestsHttp.test.ts: readers list, readers get 403 on answer, operators 200 then 409, plus 404 and 400.
  • A dispatch-race case: a request resolved between the read and the command is refused with the orchestrator's own reason (434e1f47ef, found in feat(crews): answer a Crew thread's provider approvals and questions from the Inbox #309's seeded dev-server pass, where the refusal only named the command id).
  • vp test run apps/server/src/j5: 686 passed at open; the J5 a2a suite passes after the fix. Server typecheck clean.

Review focus

  • respond reads, checks pending, then dispatches, and the orchestrator rechecks pending. Is the 409 mapping right for every dispatch error, or should some be 500?
  • list walks every live Crew's threads on each poll; there are usually a handful, but check the cost assumption.

Closes #263

Claude Opus 5.5 via Claude Code

🤖 Generated with Claude Code

…he Inbox API

Captains and seats run while the person is elsewhere, but their providers'
tool approvals and user-input questions only rendered inline in the thread's
composer, so a Crew stalled on a prompt nobody saw.

A J5-owned CrewRuntimeRequestService reads the pending approvals and
questions on every live Crew's Captain and seat threads straight from their
projections, with the same selection the composer uses, and answers them
through runtime-request.respond. Two J5 routes serve it: a list under read
scope and an answer under operate scope. An answer to a request that already
resolved, including one answered inline on another device, is refused with
409 and changes nothing. A thread outside every live Crew is never listed or
answered, and a retired Crew's requests leave the list. The web Inbox section
is #264.

Closes #263

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: Jacksondr5/j5code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 2c0584e7-32c5-407f-9408-503a98df3fe3


Comment @coderabbitai help to get the list of available commands.

@bryantderosier
bryantderosier added this pull request to stack #310 September 25, 2026 01:00
@bryantderosier
bryantderosier marked this pull request as draft September 25, 2026 01:00
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 effective changed lines (test files excluded in mixed PRs). labels Sep 25, 2026
Found in the seeded dev-server pass: a refused answer read "Failed to
dispatch orchestration command runtime-request.respond (<command id>)",
hiding why. The refusal now carries the orchestrator's own reason, such as
"Runtime request ... is resolved" or "Provider session ... was not found".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bryantderosier
bryantderosier marked this pull request as ready for review September 25, 2026 01:15
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.ts Outdated
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.ts Outdated
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.ts Outdated
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.ts Outdated
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.ts Outdated
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.ts Outdated
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.ts Outdated
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.test.ts Outdated
bryantderosier and others added 4 commits September 25, 2026 15:04
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Crew runtime request tests this branch adds still called NodeSqliteClient.layerMemory(), which the sync replaced with NodeSqliteClient.layer({ filename: ":memory:" }).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

# Conflicts:
#	apps/server/src/j5/a2a/runtimeLayer.ts
…nbox lists

From review on #308:
- A thread read goes through getThreadProjectionIfPresent: only a genuine
  not-found or an archived thread reads as absent. A store failure or a
  listLive SQL failure now propagates, so list returns 500 instead of an
  empty Inbox and respond no longer reports a live request as not found.
- respond looks the request up through the same selector list uses, so
  auth_refresh, dynamic_tool_call, and a question without its item are never
  answered, and a request that is not live is refused with the way out.
- A dispatch refusal with the decider's string reason is a 409; any other
  dispatch failure is a new CrewRuntimeRequestDispatchError the route logs
  as a 500.
- list reads a Crew's threads concurrently.
- The selector moves to J5-owned packages/shared/src/j5/crewRuntimeRequests.ts
  and matches the composer's derivePendingThreadRequests (an approval is
  answerable only when live; a question answered by message reads as
  message; multiSelect defaults to false). A client-runtime test runs both
  selectors over every branch and fails if they drift.
- The test mocks fail the way the real services do.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
bryantderosier added a commit that referenced this pull request Sep 25, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
bryantderosier added a commit that referenced this pull request Sep 25, 2026
…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>
@bryantderosier

Copy link
Copy Markdown
Collaborator Author

@tyler-barton-horizon Thanks for the careful review. All eight points were real, and I've addressed each one in its thread and resolved it. In short:

CI is running on both branches now.

Comment thread FORK.md
Comment thread apps/server/src/j5/a2a/CrewRuntimeRequestService.ts Outdated
@Jacksondr5

Copy link
Copy Markdown
Owner

Decision (Jackson, 2026-09-26): slim #308/#309 to seat approvals only.

The Captain is the thread the person watches by design, so the Captain's approvals and questions stay inline in its own thread. Seats are what the person isn't watching, and seats will no longer have native question tools; they ask their Captain instead (#325). What's left for the Inbox is seat approvals.

For #308 that means:

  • List and answer only requests on seat threads. The Captain's thread is excluded.
  • Approvals only. Drop the question-answer path from respond, the contract, and the selector.
  • List only requests the Inbox can answer (live). Requests that can only be answered in the thread stay there and don't count on the bell.
  • The panel's fixes that still apply:
    • the FORK.md row that cites a case before it exists;
    • list each thread's request once, even when a thread appears in more than one Crew record (dedupe by thread id).
  • Keep the J5-owned selector copy and its parity test. Don't import from client-runtime: the server shouldn't take a dependency on a client package.

Related: custom seats will default to Full access and the roster card will warn about seats with limited access (#326), so seat approvals should be the exception rather than the norm.

bryantderosier and others added 2 commits September 28, 2026 08:49
The Captain is the thread the person watches, so its approvals and
questions stay inline in its own thread. The Crew request route now lists
and answers only provider approvals on seat threads that the provider can
still take (responseCapability live), once per thread even when several
Crew records name it. Questions and non-live approvals stay in the seat's
thread and are neither listed nor answered here.

The contract drops the question variant and `answers`; an item carries
the composer's approval fields directly. FORK.md case 46 records the
shared selector export and its parity test against upstream's
derivePendingThreadRequests.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
bryantderosier added a commit that referenced this pull request Sep 28, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
bryantderosier added a commit that referenced this pull request Sep 28, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bryantderosier

Copy link
Copy Markdown
Collaborator Author

@Jacksondr5 #308 is slimmed to your 2026-09-26 decision (8873b2f):

  • It lists and answers seat approvals only, with the Captain's thread excluded.
  • No question path in respond, the contract, or the selector.
  • Only live approvals are listed.

The panel fixes are in too: #308 has its own FORK.md case, and each thread is listed once. The J5-owned selector and its parity test stay, and the server takes no client-runtime dependency. Also merged the latest j5/main. The PR description has a current-scope summary at the top. CI is running.

@Jacksondr5 Jacksondr5 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Posted by an AI agent on Jackson's behalf.

Approved. The review panel (Opus 5.5 + Astra) verified the slim-down to seat approvals against Jackson's 2026-09-26 decision, and every round-one finding is fixed: the FORK.md case now lives in #308, and each thread's request is listed once. Astra's live pass on #309's tip, which exercises this server side, passed every scenario.

Merge order: #308, then #309.

@bryantderosier
bryantderosier merged commit 1dab187 into j5/main Sep 28, 2026
35 of 37 checks passed
@bryantderosier
bryantderosier deleted the j5/crew-requests-server branch September 28, 2026 17:18
bryantderosier added a commit that referenced this pull request Sep 28, 2026
…from the Inbox (#309)

* feat(crews): answer a Crew thread's provider approvals and questions 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>

* fix(crews): Inbox approvals offer the composer's choices, and the thread 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>

* fix(crews): the Inbox seat badge uses Badge's own size

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>

* fix(crews): the Inbox answers only live Crew requests and leaves the 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>

* refactor(crews): the Inbox shows seat approvals with the composer's own approval UI

The Inbox now lists only Crew seat approvals, and nothing is hidden from
a thread: ChatView.tsx is back to upstream's, its FORK.md case and file
row are gone, and the roster gate no longer points to the Inbox.

CrewRuntimeRequestsSection renders each approval with upstream's
ComposerPendingApprovalPanel and ComposerPendingApprovalActions, so
provider option warnings and every request kind label match the
composer. The question UI and the copied approval choices are removed.

An environment is read only while one of its thread shells has
hasPendingApprovals: the source atom answers empty for the rest, the
7.5s poll runs only while some approval is pending, and a flag turning on
or off triggers an immediate re-read. crewApprovalPollPlan holds that
decision and is unit tested.

Docs describe seat approvals in the Inbox and the Captain's requests
staying in its thread.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 effective changed lines (test files excluded in mixed PRs). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A Crew thread's provider approvals and questions reach the Inbox (server)

3 participants