feat(profiles): open targeted agent editor from owner profile - #254
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: one P2 recovery defect, detailed inline. Merge criteria: make the routed Agents destination reconnect its failed relay on Retry, and cover outage → retry → exact editor opening through the real App/navigation contract. No new recovery framework is needed.
Reviewed the whole feature at 2dc6c288f5dd257272dddf0954b9f3e375d48c0e against df7b7e7f45739f3e06e12d81623385701acdc51d: verified-owner profile ingress, exact native/community targeting, route validation, editor/save failure and navigation lifetime. The change is proportionate; missing/ambiguous-target presentation preferences and cosmetic nits are not blockers.
Existing hosted CI passed on the merge of those pins: 3,709 unit tests, 658 browser journeys and 7 measurements. The new journey is jsdom/component-only; its navigation stub cannot expose the shell recovery failure. This was source-only review on isolated Blox, not an executed reproduction. No PR code, local tests or CI reruns were executed; browser/native/live-relay acceptance of the new path remains unverified.
| else if ( | ||
| !control || | ||
| (connection.status !== "ready" && connection.status !== "connecting") | ||
| ) | ||
| request.complete({ status: "failed", reason: "unavailable" }); |
There was a problem hiding this comment.
[P2] Reconnect the relay when retrying the routed Agents destination
Reload a persisted agent-edit route while its community relay is unavailable. Once connection fails, this branch completes failed/unavailable; App.tsx:104–117 replaces the page with its generic failure notice. The offered Retry navigation calls useAppNavigation.retry(), but src/app/navigation.ts:227–241 only calls services.relay.retry() for Channels and Projects, not buzz.agents/agents. The relay service requires an explicit retry after failure, so Retry just remounts this page against the same error and fails again, even after network connectivity returns. The user must leave this destination to recover; this is not a whole-app lockout.
Include the Agents destination in the existing guarded relay-retry handling, or keep a working reconnect control mounted. Add a regression through the real App/navigation service: routed edit + failed relay → Retry reconnects → the exact editor opens. The new routed() test helper stubs complete and cannot observe the real shell unmount/retry behavior.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Re-review clear at 2a3fa19df8f9de7cbfb304a87073976b9f554203 against base df7b7e7f45739f3e06e12d81623385701acdc51d. The previous P2 is resolved; no new actionable blockers in the two-file repair since 2dc6c288.
- Agents now uses the existing guarded shell relay retry. Reconnection enters
connectingbefore navigation retries; the remounted page waits for the fresh session and opens only the exact same-community agent. - The added regression exercises the real mounted App, navigation and relay owners with mocked HTTP/native boundaries: failed destination → shell Retry → second connection attempt → exact editor and
openedacknowledgement. It is integration coverage, not a stubbed completion-helper test. - Hosted CI passed on merge
94ec3a5(this head + pinned base): 3,710 JS tests, 658 browser journeys, 7 measurements; Windows native validation skipped. Source-only review and independent challenge did not execute PR code. Persisted-address reload and live/native retry were not separately exercised.
No remaining code-review fix is requested. This is a COMMENTED review, not approval or dismissal of the earlier changes-requested review.
2a3fa19 to
319a557
Compare
Signed-off-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
Signed-off-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
Signed-off-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
Co-authored-by: Kalvin Chau <kalvin@block.xyz> Signed-off-by: Kalvin Chau <kalvin@block.xyz>
319a557 to
5b2bfd0
Compare
…-image * origin/main: (23 commits) fix(agents): recover status polling and scope failure diagnostics (#283) Share avatar editing across community profiles and managed agents (#271) feat(profiles): archive, unarchive and delete agents from the profile pane (#256) ci: run browser journeys on three shards per engine (#280) ci: publish scheduled macOS test prereleases (#262) feat: add private text feedback plugin (#242) 🤖 docs: add pre-PR checklist and review guidance to AGENTS.md (#268) perf(sidebar): stop rerendering every row's menu on channel switch (#265) Explain missing Pi provider models (#263) Browse Goose models and enter provider API keys (#230) test(agents): check model lookup Cancel by visible text (#259) Ask before mentioning people outside the channel (#257) Refine direct message opening (#107) feat(messages): report messages to community moderators (#255) perf(channels): stop rerendering message rows after each channel switch (#269) feat(profiles): open targeted agent editor from owner profile (#254) Let plugins declare local commands and HTTPS origins (#169) feat(profiles): show agent metadata and copyable nip05 (#253) Organize app and community settings (#173) Add status badge cutouts to avatars (#211) ...
Summary
Verification