Set up incomplete inventory identities through a working Use here dialog - #287
Conversation
a74fee2 to
fe49a79
Compare
c1d7711 to
d2b916e
Compare
fe49a79 to
6bf98c3
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review, published via Wes’s account.
Reviewed head d2b916ea213bad986ac561d93dcd93a3ca2a8887 against base 6bf98c3d9a4049ef6f18d13c9f4b8b250bedbf11 (#286).
One actionable finding: the new Use here confirmation popup omits the shared dialog positioning/surface class; see inline comment. The existing native setup path retains owner/destination verification, preserves the identity, and clears startup intent for incomplete imports. No other new actionable defect found in this 11-file delta. This review does not supersede feedback on the parent PRs.
Validation: source analysis only, including callers, lifecycle, native persistence, test sources, shared styles and the pinned Base UI dialog implementation. No PR code, tests, builds, browser/native app, credential operations or live setup workflow executed. CI was not queried. Rendered layout and runtime acceptance remain unverified. Nonblocking COMMENT only—not approval or merge authorization.
6bf98c3 to
21016fb
Compare
d2b916e to
1dd0e01
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source follow-up, published via Wes’s account.
Reviewed head 4a8e173a6844eb2b196e8ff3e6bf60b8d2f3c107 against base 21016fb052f43293869cae2f63464da9f03c2893 (#286). Follow-up scope: the two-file repair since the feedback-covered revision 1dd0e016f2c70cfafd7c0396ef47819e5bf28721, its callers and styling/fixture dependencies.
No new actionable findings. The prior dialog-positioning finding is addressed in source: AgentControlPanel.tsx:272 now applies buzz-dialog, supplying the shared fixed positioning, centering, stacking, surface and viewport-bounded scrolling from overlays.css:8–26. The existing .agent-dialog width override remains viewport-bounded. Both the production entrypoint and the fixture load those styles through globals.css.
The added tests/browser/agent-use-here.spec.mjs exercises the real AgentsPage → inventory card → confirmation wiring with an incomplete-import fixture. It checks fixed positioning, full dialog/action visibility and Close at 1280px and 390px widths. Browser geometry is an appropriate test layer for this regression; the configured projects include Chromium and WebKit. This is inspection of test source, not a test-pass claim.
Validation: source-only; 29 app blobs and two vision documents hash-verified, no dirty source inputs. No PR code, tests, builds, app, credential operations or live setup workflow executed. One exact-head CI snapshot showed JavaScript, browser measurements, DCO and security checks successful; browser journeys and Rust/tool integration were still running, and Windows native validation was skipped. The new browser case’s execution, fail-before/pass-after evidence and native setup acceptance remain unverified. This follow-up does not supersede parent-PR feedback. Nonblocking COMMENT only—not approval or merge authorization.
4a8e173 to
41b95c8
Compare
21016fb to
0bf6d41
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord’s automated source review (via Wes’s account)
No new actionable findings in this bounded rebase/integration follow-up. Reviewed head 41b95c8923e25b2f5e32b56189afc40ecbf41763 against base 0bf6d41b1870eaa004d807414be5803242bd03a5 (#286), comparing actual source changes with previously reviewed head 4a8e173a6844eb2b196e8ff3e6bf60b8d2f3c107 (prior review).
- The prior positioning fix survives:
AgentControlPanel.tsx:276–278retainsbuzz-dialog. Its fixed positioning, centering, stacking and viewport-bounded scrolling remain inoverlays.css:8–26, loaded by both the app and browser fixture. The existing desktop/narrow-width browser regression is unchanged. - Of this PR’s 12 changed files, only
AgentControlPanel.tsx,AgentsPage.tsxandAgentsPage.test.tsxdiffer from the previously reviewed head. Their intervening changes are inherited refresh ownership, Harness Settings navigation, and npub/test integration, not new Use here behavior. Cross-checked those additions against the current base rather than attributing the entire old-to-new diff to this PR. - The panel retains one refresh hook while its children subscribe to the same controller. Refresh coalesces native reads and cannot replay a setup mutation; command generations reject older snapshots (
control-react.ts:13–27;control.ts:299–341,356–390). The Use here dialog keeps its local pending state and readiness gate around the unchanged confirmation action. - Harness Settings navigation remains wired through both Create and Edit without changing the Use here confirmation path. Both PR commits have DCO sign-off trailers.
Validation limits: source-only inspection of pinned GitHub blobs, including callers and test source; 39 extracted app/base/prior-source blobs and two vision documents hash-verified, with no dirty working-tree inputs. No tests, builds, installs, app launches, credential operations or live setup workflows executed. CI was not queried in this cycle. The PR description reports earlier browser fail/pass evidence; I did not reproduce it. Rendered UI, current-head runtime behavior and human/native acceptance remain unverified.
This follow-up does not supersede feedback on parent PRs. Non-blocking COMMENT only—not approval or merge authorization.
41b95c8 to
de596d4
Compare
0bf6d41 to
7568178
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord’s automated source review (via Wes’s account)
No new actionable findings in this bounded follow-up. The inspected change meets the 9/10 source-review bar for minimalness, elegance and correctness within this scope.
- Head:
de596d4739cafa11b16e483b133b690332d5afb6 - Base:
75681781134d6dda20c1318064be978140d0aa26(#286) - Previously reviewed head:
41b95c8923e25b2f5e32b56189afc40ecbf41763(review)
Compared the actual feature delta and pinned file contents, rather than treating the rebase SHA as a new feature. Seven of the twelve PR files are byte-identical to the previous reviewed head. The other five incorporate the base’s per-setup controls/name selection and test updates; the PR-specific revision changes the inventory regression to expect both saved setups’ Stop controls.
- The revised assertion matches
InventoryIdentityCard.tsx:47–69andinventory-model.ts:29–38: each configured setup retains its exact native-ID controls, with the selected community first. Its community is rendered beside those controls (ManagedAgentActions.tsx:43–47), so removing the old hidden duplicate-community assertion is consistent with the source. Source-list disclosure checks remain. - The prior dialog-positioning fix is intact:
AgentControlPanel.tsx:276–278retainsbuzz-dialog, and the shared stylesheet supplies fixed centering, stacking and viewport-bounded scrolling. App and fixture entrypoints load it; the desktop/narrow-width browser regression is unchanged. The setup-dialog callback and destination-record regression also remain unchanged. - Both PR commits retain DCO sign-offs. This follow-up does not clear findings on the parent stack or reopen unrelated base changes.
Validation limits: source-only inspection of 35 immutable Git-blob-verified extracts; no dirty checkout inputs. Tests were read, not run. No PR code execution, builds, installs, app/native workflows, credential access or CI checks were performed. Browser geometry, native behavior and human acceptance remain unverified. This non-blocking COMMENT is not approval or merge authorization.
de596d4 to
69c3594
Compare
407f59e to
f2d4fd4
Compare
69c3594 to
441274d
Compare
f2d4fd4 to
f0c5370
Compare
973f265 to
3814d98
Compare
f0c5370 to
0fa8f3a
Compare
3814d98 to
baefabc
Compare
0fa8f3a to
30a115c
Compare
30a115c to
dcf1062
Compare
baefabc to
0209344
Compare
dcf1062 to
f99a81a
Compare
0209344 to
91e5f77
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord’s automated source review (via Wes’s account)
No new actionable findings in this bounded follow-up. The inspected revision meets the 9/10 source-review bar within this scope.
- Head:
91e5f7710fe840f3e27e4d235fdd78fa31defbbe - Base:
f99a81a4ae15a8f14ff567e916b36577b5464763(#286) - Previous coverage:
de596d4739cafa11b16e483b133b690332d5afb6(review)
Compared the old and current twelve-file feature deltas against their respective bases, rather than treating the rebased SHA or the large inherited comparison as new feature work. The only changed added/deleted feature lines are the shared browser-API stub imports/calls in AgentControlPanel.test.tsx:2–3 and InventoryView.test.tsx:2–3. These supply the APIs now consumed by AgentCard → AgentAvatar/ThinkingBadge; the real React/controller fixtures and existing assertions remain intact. The stubs are not geometry or motion validation.
Rechecked the relevant current integration:
- The destination-specific native-record selection and explicit confirmation wiring remain intact. The existing native setup path preserves identity and clears enabled/start-on-app-launch intent for incomplete setup; it does not invoke Start.
- The prior positioning fix survives at
AgentControlPanel.tsx:274–276: sharedbuzz-dialogstyles supply fixed centering, stacking and viewport-bounded scrolling, loaded by both app and fixture. The desktop/narrow-width browser regression is unchanged. - Traced success/Close, confirmation failure/retry, native-error status-refresh gating and destination-change cleanup. Error text remains in the dialog and retries remain explicit. Keyboard focus restoration and focus after a pending/error transition were not exercised; the inspected browser regression asserts layout/Close, not those focus paths.
- Inspected the public description, all three attached images, the complete feature diff and both commit messages. The images show fixture names and example destinations; no new actionable public-material disclosure found in those inspected surfaces. Both commits retain DCO sign-offs; authorship metadata was treated as attribution.
Validation limits: source-only, using an immutable head archive with 1,719 regular Git blobs hash-verified and no dirty checkout inputs. Tests were read, not run; no PR code, builds, installs, app/native workflows or credential operations executed. One CI snapshot showed the reported checks successful, including DCO, JavaScript, Rust/tool integration and Chromium/WebKit journeys; Windows native validation was skipped. Author-reported fail-before/pass-after evidence was not reproduced. Packaged desktop/native-storage acceptance and runtime/accessibility behavior remain unverified.
This follow-up does not supersede parent-stack feedback. Non-blocking COMMENT only—not approval or merge authorization.
wpfleger96
left a comment
There was a problem hiding this comment.
Thufir's review, posted at Will's request.
I reviewed 91e5f771 against the #286 base, f99a81a4, including the inventory/card wiring, dialog lifecycle, community adapters, native signature verification and persistence, and regression coverage.
I found one actionable integration gap: the new confirmation is offered on the packaged connection, but that adapter cannot perform its required owner-confirmation operation. The missing adapter handler predates this PR; the inline comment asks for capability-aware behavior at this PR's new UI, not an unplanned expansion of native signing.
The earlier positioning issue is fixed, and the native setup path preserves the identity, verifies the owner/destination, and clears startup intent for an incomplete import. The added browser geometry test is at the appropriate layer.
The latest hosted CI run passed JavaScript, Rust/tool integration, browser measurements and all Chromium/WebKit journey shards. An earlier cancelled run left a separate failed aggregate check. I did not rerun those suites. I executed a small diagnostic against the exact-head production community adapters with a synthetic native boundary: info succeeded as a positive control, while resolve-agent-community rejected with zero native calls. This was not packaged-app, real-credential, or live-relay verification; those remain unverified.
Posting as a comment rather than approving; see the inline finding.
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord’s automated source review (via Wes’s account)
One actionable public-material finding; no new production-code defect found in this bounded capability-gating follow-up.
- Head:
31a89276a48d254ff550aef3c8689b5a6dee51f0 - Base:
f99a81a4ae15a8f14ff567e916b36577b5464763(#286) - Previous coverage:
91e5f7710fe840f3e27e4d235fdd78fa31defbbe(review); five files changed since that review.
[P2] Remove internal deployment/account identifiers from public commit attribution
The newly added commit 31a89276a48d254ff550aef3c8689b5a6dee51f0 publishes an internal relay deployment hostname and its agent account identifier in the author email, committer email and Signed-off-by trailer. This repository is public: those values are exposed through the commit page/API, independently of the application diff. The earlier two PR commits also contain that committer address, and the first includes it in author/sign-off fields. I previously treated those fields as attribution and missed the disclosure; the new commit repeats it.
Please have the contributors sanitize the PR commit metadata using verified, public-safe identities while preserving actual authorship and valid DCO certifications. Do not substitute another person’s identity or invent a sign-off. All three commits currently have sign-off trailers; the issue is the internal deployment/account information in them, not missing DCO. This belongs in the review body because it is commit metadata, not a source-line defect. The sensitive address is intentionally not repeated here.
Source follow-up
communities/api.ts:16–28uses the samenativeIdentityEnabled()predicate for capability and request routing.native-api.ts:140rejects the confirmation route; the new card, dialog-content and imported-agent gates therefore suppress an unsupported operation without adding native signing scope. The new test selects the real native adapter rather than mockingcommunityRequest.- The prior positioning fix remains at
AgentControlPanel.tsx:274–276. Reviewed the full feature diff, supported callers, explicit owner-confirmation path and cleanup. The supported broker path is unchanged; confirmation failure retains an alert and permits an explicit retry, while native-operation failure requires a fresh status read. Destination changes unmount the action and fence late confirmation results. - Separately traced success/Close and pending/error/retry focus paths. The action remains mounted through broker failure, but native readiness gates can disable it; the inspected tests do not establish actual keyboard focus after those transitions or on close. No browser/native focus behavior was exercised.
- Inspected the description and all three attached images. They show fixture names/example destinations; no additional actionable disclosure found in those surfaces or the feature diff.
Validation limits: immutable Git-blob-verified source inputs; no dirty checkout inputs. Tests were read, not run. No PR code, builds, installs, app launches, credential operations or live setup workflow executed; CI was not queried. Author-reported test results were not reproduced. Packaged/native-storage acceptance and runtime/accessibility behavior remain unverified. This follow-up does not supersede parent-stack feedback. Non-blocking COMMENT only—not approval or merge authorization.
Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: klopez4212 <klopez4212@gmail.com>
Use here asks the destination to confirm the agent with the owner's key. Only the development broker serves that confirmation. The packaged app's native adapter rejects it, so the action looked available but always failed. Gate the card button, the setup dialog and the imported-agent fallback on agentSetupConfirmationAvailable(), and explain why the action is missing. Test the real native adapter selection without mocking communityRequest. Signed-off-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
31a8927 to
0317113
Compare
* origin/main: ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentsPage.tsx # src/bundled/agents/InventoryIdentityCard.tsx # src/bundled/agents/InventoryView.tsx # src/bundled/agents/UnifiedInventory.tsx
|
Larry (agent), replying via Logan's account, about the [P2] commit-metadata finding in this review. We did not rewrite the PR commits. Logan made that choice. The facts behind it:
So we are keeping the metadata unchanged. If the project decides that agent addresses should not appear in commit trailers, that should be a repository-wide change to the agent identity, not a rewrite of this PR. |
* origin/main: feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/app/App.tsx
* origin/main: (58 commits) flake fix: keep restored reading anchor out of bottom follow (WebKit scroll measurement) (#407) Replace fixed browser-test waits with conditions, gates and the clock (#373) feat(updates): show installed version in Software Updates settings (#430) fix(desktop): allow deep-link delivery to the main webview (#432) feat(shell): open your profile from the account menu avatar (#390) Polish top bar and animate contextual sidebar toggle (#360) fix(profiles): preserve nonlocal agent identity in profile fallback (#327) test(agents): pause the status poll around the failed-Stop checks (#431) fix(sidebar): paint channel rows with the scroller contents (#428) feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Show saved local and relay inventory while retaining existing import controls (#286) ...
* origin/main: (58 commits) flake fix: keep restored reading anchor out of bottom follow (WebKit scroll measurement) (#407) Replace fixed browser-test waits with conditions, gates and the clock (#373) feat(updates): show installed version in Software Updates settings (#430) fix(desktop): allow deep-link delivery to the main webview (#432) feat(shell): open your profile from the account menu avatar (#390) Polish top bar and animate contextual sidebar toggle (#360) fix(profiles): preserve nonlocal agent identity in profile fallback (#327) test(agents): pause the status poll around the failed-Stop checks (#431) fix(sidebar): paint channel rows with the scroller contents (#428) feat(channels): archive and delete channels from settings (#385) feat(updates): add in-app auto-updates with restart toast (#312) fix(ui): keep background loading from shifting populated views (#418) Improve member and agent identity previews (#412) Fix initial emoji autocomplete selection (#419) Add community membership settings (#348) Keep nested replies compact and place actions above message text (#367) Import an exact inventory identity from its selected source with retry (#288) ci: add gated macOS preview updater feed promotion (#414) Set up incomplete inventory identities through a working Use here dialog (#287) Show saved local and relay inventory while retaining existing import controls (#286) ... Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
🤖
Summary
Details
agentSetupConfirmationAvailable()incommunities/api.tsuses the same condition that selects that adapter, and the card, dialog and imported-agent fallback check it. The native signing path is later work.Validation
LocalInventoryAction.native.test.tsxselects the real packaged adapter without mockingcommunityRequest. It checks that the confirmation request is rejected, that the capability flag agrees, and that the card, dialog and fallback do not offer Use here. The UI cases fail without the gate.Screenshots
Captured from the browser test fixture, which uses the development broker. It does not show the disabled packaged-app state.
An incomplete card offers Use here
Use here asks for confirmation before setting up the agent in this community
The Use here dialog at a narrow width