Repository navigation
Add Marketplace tab and shared agent profile panels - #1411
Conversation
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Co-authored-by: Arjun Mahanti <arjun@squareup.com> Signed-off-by: klopez4212 <klopez4212@gmail.com>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34cebad6fa
ℹ️ 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".
| const closeShared = () => { | ||
| setSharedSelection(undefined); | ||
| sharedTrigger.current?.focus(); |
There was a problem hiding this comment.
At widths up to 52rem, opening the profile sets data-side, and AgentsPage.module.css:185-191 hides the entire .main containing the selected tile. When the panel is closed, React batches setSharedSelection(undefined), so this immediate focus() runs while the tile is still under display: none; browsers ignore it and keyboard users lose the promised return focus after Close or Escape. Restore focus in a layout effect after the list has been revealed, or synchronously commit the close before focusing.
AGENTS.md reference: AGENTS.md:L46-L50
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes’s GitHub account: Addressed in 10d8e34. closeShared now commits the close with flushSync before refocusing the tile. A successful import also removes the panel and its trigger, so focus now goes to the Your agents tab.
Wait for the drawer animation before reading all row bounds atomically, avoiding comparisons between different animation frames. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <klopez4212@gmail.com>
Commit the close before refocusing the tile, which a narrow layout hides while the profile is open. After an import removes the panel and its trigger, focus the Your agents tab now shown. Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes’s GitHub account.
No blocking defects at 10d8e34a. This review covered three lanes: the import/config trust boundary, UI and design-system conformance, and tests/scope.
- Import trust:
importSharedrebuilds Goose/Codex defaults and copies only the publishedprovider/modelstrings plus instructions. A parser probe confirmed that arbitrary config, workspace, command andenv_varsfields are dropped. A missing or unknown runtime keeps Import disabled until a type is chosen. - Focus fix: verified in Chromium and WebKit at 390px and 1280px. Close returns focus to the tile, and a successful import moves focus to the Your agents tab. Reverting the fix leaves focus on
<body>. - Browser test edits (
todos,image-scroll): they fix the fixture/sampling side, and no assertions were loosened.
Before merge:
ProfileBackdrop.tsx/ProfileHero.module.cssare copies of the same files in open #1405, and the.heropadding differs. Whichever PR lands second gets an add/add conflict. Land #1405 first and rebase onto it.- Recommended: long published model/provider strings squeeze the row labels (inline comment).
Still unverified: a real native import that produces a usable agent, and enlarged text at mid widths. The other inline comments are optional.
| } | ||
| /> | ||
| <PreferenceRow | ||
| title="Model" |
There was a problem hiding this comment.
🤖 Recommended before merge. A long published model (and provider at :436/:439) in PreferenceRow's trailing slot squeezes the label to one letter per line, because the row grid is minmax(0,1fr) auto. Reproduced at 900px, and at 390px with 20px text. Any publisher can trigger it. trailing is meant for a control, so pass the value as subtitle instead, which wraps with overflow-wrap:anywhere.
| <span className={pageStyles.identityPortrait}> | ||
| <AgentPortrait agent={agent} community={community} /> | ||
| </span> | ||
| <h2 className="m-0 text-heading text-center break-words"> |
There was a problem hiding this comment.
🤖 Optional: a long unbroken agent name overflows the hero at 390px with 20px text and gets clipped on both edges. break-words doesn't shrink a centered flex item. Add overflow-wrap:anywhere plus max-width:100% on the heading.
| icon={<ModelIcon model={agent.model} />} | ||
| className={cardStyles.card} | ||
| aria-label={agent.name} | ||
| aria-pressed={selectedId === agent.id} |
There was a problem hiding this comment.
🤖 Optional: aria-pressed announces a toggle, but clicking a pressed tile does nothing. Your agents uses aria-selected with click-to-deselect. Either drop aria-pressed and keep data-selected, or make the second click deselect.
| import styles from "./ProfileHero.module.css"; | ||
|
|
||
| /** Decorative artwork; callers resolve images through their existing media owner. */ | ||
| export function ProfileBackdrop({ |
There was a problem hiding this comment.
🤖 Merge order: this file and ProfileHero.module.css duplicate the same paths in #1405. The TSX is identical, but .hero padding is --space-8 here and calc(--space-8 + --space-4) there. Whichever PR lands second hits an add/add conflict, and one padding silently wins. Land #1405 first and rebase onto it. Optional: the only caller passes just src, so kind/children (and the human selector) have no current consumer here.
| description: | ||
| "goose on this computer, with any provider goose supports. Each thread it is mentioned in is one continuing session.", | ||
| defaults: () => ({ config: DEFAULT_CONFIG }), | ||
| importShared: (source) => |
There was a problem hiding this comment.
🤖 Optional: this accepts any published provider ID, including one that isn't installed or configured locally. Import then saves settings that only fail at launch. This isn't an injection path, since the values travel over ACP/JSON-RPC, but a local availability warning in the import dialog would help.
Keep focus on the Import as selector when Change settings replaces the summary, name the summary row Import as like the selector, and mark the open profile's tile with aria-current rather than a toggle state. Match #1405's ProfileHero styles so the shared file merges cleanly. Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket (systems/lifecycle), Mantis (correctness/security) and Groot (product/accessibility) reviews.
Reviewed head 55223484e2dd2cea2e4106a541939f7259289331, base 1508ad261439982da9f0b2edc9ca55c7fa4bd4f5 (merge base 7170025ad2132720e56e29a0804479f982b76475). This follow-up compares the four-file endpoint delta against previously reviewed 10d8e34aa23c2075c171d17352ab0fc955720399, with relevant feature/caller checks.
The Change settings focus handoff, Import as label, current-tile semantics and profile padding changes look sound in source. The shared profile files match #1405 at cbf9ce1e.
One newly identified P3 keyboard retry-focus defect is detailed inline. It exists in the earlier feature revision too; it was not introduced by the latest polish commit. The existing long provider/model layout finding also remains applicable: those values still occupy the unconstrained trailing slot. Keep that existing thread as the repair location rather than duplicating it. Overall source assessment: 8.5/10, pending those focus and readable-layout repairs.
Validation: immutable source and test inspection only; no tests, builds, app launches or native imports executed. Public PR description and follow-up commit material inspected; the description has no attached images. A single hosted-check snapshot showed JavaScript (2/3) and CI required failing, with Windows skipped; failure cause was not investigated. Real-browser focus/retry, narrow/enlarged-text rendering, screen-reader behavior, native import and performance remain unverified. This is a non-blocking COMMENT review, not approval or merge authorization.
| </Button> | ||
| <Button | ||
| variant="primary" | ||
| disabled={!chosen || pending} |
There was a problem hiding this comment.
🤖 [P3] Preserve Import focus while the request is pending
A keyboard user activating Import loses the focused control when pending becomes true: passing both disabled={!chosen || pending} and loading={pending} overrides the shared Button's loading-focus behavior. Button.tsx:58–62 retains focus only when loading && !disabled. On rejection, run() displays the alert and clears pending but does not restore focus to Import, so retry requires locating the control again. This is source-derived, not a runtime reproduction, and was already present at 10d8e34a.
Use disabled={!chosen} with loading={pending} (as in ReportMessageDialog). Button already suppresses loading activation, and run() guards pending. Add a keyboard-driven rejected-import check that retains focus through pending/error and can retry. This preserves the shared control's focus/loading contract without changing import behavior.
…-sessions * origin/main: (23 commits) Add Marketplace tab and shared agent profile panels (#1411) fix: publish scoped typing indicators from conversation composers (#1435) feat: add configurable composer and pane focus shortcuts (#1418) perf(messages): reuse captured video opening frames on remount (#1437) FOR OCT 22: let agents clone and push their channel's repository (#1324) perf(relay): keep row profile map identity while new authors load (#1431) perf: avoid redundant relay channel derivation (#1412) Normalize dialog sizing, headers, spacing, and motion (#1403) Polish agent activity pills and popover actions (#1429) feat(agents): ask to resume claude and codex turns stopped by reload or quit (#1235) test(sign-out): join listed paths with / so the tests pass on Windows (#1247) fix(deps): remove distrusted emsign certificate roots (#1436) fix(plugins): distinguish usage and costs from agent activity (#1392) refactor(design-system): extract named Appearance palettes and revise plugin gradients (#1339) fix(channels): cancel inert activity popover opens (#1426) fix(codex): set up Codex in a dialog from the agent creation card, as Claude does (#1419) Add community-local settings (#1363) Fix sidebar and pane header alignment (#1409) Show recorded Codex scripts in Activity (#1413) fix(agents): restore emoji avatars in the agent avatar picker (#1404) ... # Conflicts: # crates/agent-controller/src/app_agent.rs
…-sessions * origin/main: (23 commits) Add Marketplace tab and shared agent profile panels (#1411) fix: publish scoped typing indicators from conversation composers (#1435) feat: add configurable composer and pane focus shortcuts (#1418) perf(messages): reuse captured video opening frames on remount (#1437) FOR OCT 22: let agents clone and push their channel's repository (#1324) perf(relay): keep row profile map identity while new authors load (#1431) perf: avoid redundant relay channel derivation (#1412) Normalize dialog sizing, headers, spacing, and motion (#1403) Polish agent activity pills and popover actions (#1429) feat(agents): ask to resume claude and codex turns stopped by reload or quit (#1235) test(sign-out): join listed paths with / so the tests pass on Windows (#1247) fix(deps): remove distrusted emsign certificate roots (#1436) fix(plugins): distinguish usage and costs from agent activity (#1392) refactor(design-system): extract named Appearance palettes and revise plugin gradients (#1339) fix(channels): cancel inert activity popover opens (#1426) fix(codex): set up Codex in a dialog from the agent creation card, as Claude does (#1419) Add community-local settings (#1363) Fix sidebar and pane header alignment (#1409) Show recorded Codex scripts in Activity (#1413) fix(agents): restore emoji avatars in the agent avatar picker (#1404) ... Co-authored-by: Will Pfleger <wpfleger@block.xyz> Signed-off-by: Will Pfleger <wpfleger@block.xyz>
* origin/main: (56 commits) Polish huddle portraits and thread layout (block#1406) fix(goose): keep long model names inside the Settings model row (block#1432) feat(agents): show a timestamp on each activity feed entry (block#1443) feat(agents): add buttons to compact and start new session (block#1434) feat(activity): request and show Claude Code and Codex thinking (block#1452) add inline renaming for Solo sidebar chats (block#1345) fix(goose): say why the profile has no model to choose (block#1445) Add Marketplace tab and shared agent profile panels (block#1411) fix: publish scoped typing indicators from conversation composers (block#1435) feat: add configurable composer and pane focus shortcuts (block#1418) perf(messages): reuse captured video opening frames on remount (block#1437) FOR OCT 22: let agents clone and push their channel's repository (block#1324) perf(relay): keep row profile map identity while new authors load (block#1431) perf: avoid redundant relay channel derivation (block#1412) Normalize dialog sizing, headers, spacing, and motion (block#1403) Polish agent activity pills and popover actions (block#1429) feat(agents): ask to resume claude and codex turns stopped by reload or quit (block#1235) test(sign-out): join listed paths with / so the tests pass on Windows (block#1247) fix(deps): remove distrusted emsign certificate roots (block#1436) fix(plugins): distinguish usage and costs from agent activity (block#1392) ... Signed-off-by: Nikolaj Goodger <ngoodger@squareup.com>
Marketplace is now a tab beside Your agents and Your teams. Selecting a tile opens a profile side panel using the header structure from #1405, with compact expandable instructions and an Import confirmation dialog.
Imports preserve supported published runtime/model/provider settings; unsupported or missing runtimes require an explicit choice. The import selector spans the full row, and redundant helper copy is removed.
Validation: TypeScript, focused catalog/service/provider tests, and live-data browser checks for panel selection, keyboard/focus, narrow layouts, expansion, and import-dialog cancellation. Kenny tested the native preview. Actual native import and the full visual matrix remain unverified; no live agent was imported during testing.
CI follow-up: fixed the existing Todos journey comparing geometry across drawer-animation frames. All original assertions remain; no browser cases added or removed. The full Todos file passes in Chromium and WebKit locally (2 tests).
Second CI follow-up: keep released offscreen-image decode gates open across virtualization remounts. The full image-scroll file passes in Chromium and WebKit (14 tests); no cases or assertions removed. Hosted recurrence remains to be checked.
Focus follow-up at
10d8e34a: closing the Marketplace profile commits before returning focus to the tile, which the narrow layout hides while the profile is open. After a successful import removes the panel, focus moves to the Your agents tab. The existing Marketplace journey test now covers import focus and fails without the fix. Narrow-width focus return depends on CSS, so jsdom cannot exercise it, and no browser run covered it.Polish follow-up at
55223484: Change settings keeps focus on the Import as selector that replaces it, and the summary row is now titled Import as to match. The open profile's tile usesaria-currentinstead of a toggle state.ProfileHero.module.cssnow matches #1405 exactly, so the duplicated files merge cleanly whichever PR lands first. Agents/profile Vitest suites (672 tests) and TypeScript pass. The new focus assertion fails without the fix.