Skip to content

feat(crews): edit and approve every member runtime - #192

Merged
bryantderosier merged 7 commits into
crews/11-conversational-collaborationfrom
crews/12-runtime-approval
Sep 21, 2026
Merged

bryantderosier merged 7 commits into
crews/11-conversational-collaborationfrom
crews/12-runtime-approval

Conversation

@bryantderosier

@bryantderosier bryantderosier commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

Crew approvals previously showed generic inherited access without the actual model or reasoning level. Initial roster and inbox addition cards show compact member summaries with right-aligned Edit/Remove actions and a shared Add/Edit modal: choose a saved persona or create a custom member, then edit harness, model, reasoning, and access for either. Captain-proposed and manually added members share the same controls. Persona defaults load when selected; overrides apply to this crew member without changing the saved definition. Each member shows the server-resolved configuration, why it is needed, and its instructions.

Roster and runtime edits refresh the preview. Explicit runtime choices for saved and custom members persist through proposal storage and launch. Approval validates a token over the roster and runtime, materializes advertised defaults, and launches that exact resolution; drift or an incompatible partial-launch retry requires a fresh decision. Failed previews keep approval disabled while allowing decline and refresh. Cancel discards modal edits. Instructions-only edits preserve saved-persona defaults; runtime controls remain usable when a default route is unavailable so the person can choose an available harness. Routine Codex coordination tools no longer require a separate runtime prompt in interactive modes; roster and Inbox membership approval remain authoritative.

Base: crews/11-conversational-collaboration; head: crews/12-runtime-approval. Entry 12 (top) of stack pingdotgg#153; merge after #191.

Validation: focused runtime, HTTP, contract, and client tests pass, including inherited ACP access rejection, provider-default pinning, and refusal of changed already-dispatched briefs on retry. Scoped server, web, contracts, and client-runtime typechecks pass. Disposable browser verification at 1280×800 and 1024×600 confirmed a readable Captain label, bounded roster scrolling with expanded instructions, reachable Approve/Decline actions, and an accessible composer. No seats were launched in this verification pass.

Before After
Unbounded roster Bounded roster with readable Captain

Scrolled actions · Smaller window · Scrolling recording

Implemented with Codex.

Closes #212.

@bryantderosier
bryantderosier added this pull request to stack #153 September 18, 2026 18:17
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ effective changed lines (test files excluded in mixed PRs). labels Sep 18, 2026
@bryantderosier bryantderosier changed the title crews/12 runtime approval feat(crews): edit and approve custom member runtimes Sep 18, 2026
@bryantderosier bryantderosier changed the title feat(crews): edit and approve custom member runtimes feat(crews): edit and approve every member runtime Sep 18, 2026
@bryantderosier bryantderosier self-assigned this Sep 18, 2026
@bryantderosier
bryantderosier marked this pull request as ready for review September 18, 2026 18:48

@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.

Reviewed the runtime approval feature with two independent passes. The preview-and-token shape is sound as a mechanism: the token is a content digest the server recomputes at resolve, drift is detected before the claim, overrides ride the existing JSON seat columns so no migration is needed, and both the roster card and the inbox render the same dialog. The findings below are cases where the code does not do what the feature says it does; scope and the ACP functionality itself are not judged here.

Requesting changes on four:

  1. The ACP access check validates the wrong variable (inline on CrewLaunchService.ts L274; same pattern at L336).
  2. A partial-launch retry ignores changed instructions on a seat whose brief already dispatched (inline on assertReusableSeats).
  3. The "exact resolution" token does not pin provider defaults for persona seats without an override (inline on crewRuntimePreview.ts).
  4. The Codex interactive-mode pre-approval reverses a recorded decision and FORK.md does not record it (inline on codexToolApproval.ts).

Comment: approvalToken is optional in the contract (inline on j5.ts) while approve refuses without it.

Reviewed by Claude Fable 5.1 in Claude Code, with an independent pass by GPT-6-Astra in Codex.

Comment thread apps/server/src/j5/a2a/CrewLaunchService.ts Outdated
Comment thread apps/server/src/j5/a2a/CrewLaunchService.ts
Comment thread apps/server/src/j5/a2a/crewRuntimePreview.ts
Comment thread apps/server/src/j5/a2a/mcp/codexToolApproval.ts
Comment thread packages/contracts/src/j5.ts Outdated
Jacksondr5 added a commit that referenced this pull request Sep 20, 2026
…icit (#148, #191, #192)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Jacksondr5

Copy link
Copy Markdown
Owner

From Jackson's hands-on review, not blocking; fix or defer is your call.

The roster gate cannot scroll once the roster is tall. Six members with instructions expanded pushes Approve and Decline below the viewport, and the gate is mounted above the composer outside the chat scroll region (the move out of the form in #148), so nothing scrolls it.

roster gate with six members, no scroll

CrewRosterGate.tsx renders a plain mb-3 container with no height bound. A max-h with overflow-y-auto on the gate, sized against the viewport, keeps the actions reachable; the alternative is mounting inside the scrollable timeline, which changes the "answered above the composer" placement.

@bryantderosier

Copy link
Copy Markdown
Collaborator Author

The runtime findings are addressed and replied to inline. The roster scrolling issue is fixed with a viewport-bounded scroll area; the shared card also resolves the Captain’s readable label. Browser checks at 1280×800 and 1024×600 confirmed expanded instructions, reachable Approve/Decline actions, and the composer. Before/after images and a scrolling recording are linked in the PR description.

@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.

All five round-five items are fixed and verified: the effective runtime mode is validated for custom and persona seats; changed briefs are refused before any retry dispatch; persona selections materialize provider defaults before persistence and hashing (in CrewLaunchService, with the token hashing the stored assignment); approve requires the token through a discriminated union; the roster gate is bounded to 60dvh with dialogs portaled out and the inbox card inside its own scroll area; and the Captain label resolves through the identity read. CI green. Approving.

Retraction: my round-five note said FORK.md did not record the interactive-mode pre-approval. It did, at the head I reviewed; my search missed it. Bryant's reply was right.

Two notes for #187 or a follow-up, not blocking (inline): refusing changed briefs closes the documented rename-and-drop retry once any brief has dispatched, and FORK.md still promises that convergence; and the persona-inheritance branch of the ACP check has no test.

Reviewed by Claude Fable 5.1 in Claude Code, with an independent pass by GPT-6-Astra in Codex.

Comment thread apps/server/src/j5/a2a/CrewLaunchService.ts
Comment thread apps/server/src/j5/a2a/CrewLaunchService.ts
@bryantderosier
bryantderosier force-pushed the crews/12-runtime-approval branch from d200fe8 to 98f4405 Compare September 21, 2026 14:16
@bryantderosier

Copy link
Copy Markdown
Collaborator Author

Both follow-up notes are addressed in 98f4405e2: the retry brief check ignores the roster block (FORK.md updated), and the persona branch of the ACP check has its test.

bryantderosier and others added 6 commits September 21, 2026 11:07
…ss is tested

A partial launch's retry compared each seat's whole brief against the one already dispatched, and the roster block inside it names every seat, so dropping or renaming a failed seat refused the seats that had started. The comparison now strips the platform's crew-context block and checks only the Captain's brief and the seat's own instructions. The persona branch of the ACP access check gains the focused test the custom-seat branch already had.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@bryantderosier
bryantderosier force-pushed the crews/12-runtime-approval branch from 98f4405 to 4ec484b Compare September 21, 2026 15:07
@bryantderosier
bryantderosier merged commit b446efd into j5/main Sep 21, 2026
20 checks passed
@bryantderosier
bryantderosier deleted the crews/12-runtime-approval branch September 21, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL 1,000+ 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.

Edit and approve every Crew member's runtime before launch

2 participants