Let plugins declare local commands and HTTPS origins - #169
Conversation
|
@codex review |
Signed-off-by: Matthew Boston <mboston@squareup.com>
3956961 to
4ed6797
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: three P2 defects below. Merge criteria are to disclose previously unseen grants on folder reload, make the macOS fallback executable environment usable by interpreter-based CLIs, and bound Windows command descendants on timeout (or explicitly disable that unsupported host operation there).
Reviewed head 4ed67978751328e15ea08b25ed2872dbcd0e32d3 against base 6e6417d713235dcfc29969f38f278ef51c8e7d82, including install/update/reload/rollback, revision admission, service wiring and native execution/HTTPS boundaries. All automatic hosted checks succeeded; Windows validation was skipped. Independently reproduced the inherited-PATH/shebang failure with a disposable macOS OS-boundary fixture; reload and Windows findings are source-traced, not native end-to-end reproductions. No broad suites rerun, product edits, app launch or real credentials used. Packaged external-plugin/credential-CLI and HTTPS platform integration remain unverified.
The existing merge conflict is a separate integration gate. Trusted same-WebView plugins are an intentional non-goal for sandboxing; this review does not require a new permissions system or blanket re-approval of already reviewed immutable revisions.
Signed-off-by: Matthew Boston <mboston@squareup.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
|
🤖 note: Addressed the three findings in review 5297333288. Folder reload now rejects changed host access (2c59ba8); commands receive the lookup PATH (2c59ba8); Windows commands use a Job Object with descendant cleanup and regression tests (f937204, 36822c2). The branch includes current main at f89983e, and GitHub reports it mergeable. Local build and native tests pass. Windows runtime validation remains pending because the Windows job is dispatch-only and this account cannot dispatch it upstream. |
bb43ae5 to
5c0ed5e
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Re-review: the reload-disclosure and macOS child-PATH findings are resolved. The Windows process-tree finding remains partially unresolved: see the inline spawn/assignment race. Merge criterion is to establish job membership before the child can execute, or keep this host operation unavailable on Windows until that is supported, with regression coverage for the ordering.
Reviewed head 7dcc677a56e38fcab23a6bd18d1af352755d119f against base d6b01e6b5d159bfa16ace71978ea8230544b8dc4, focusing on the agreed fixes and overlapping integration changes. This is source/API-contract evidence, not a Windows runtime reproduction. No broad local suites, app launch, real credentials or product edits. Hosted JavaScript and Rust/tool jobs are successful; three browser shards were still running at the final snapshot and Windows was skipped. No Windows runtime or packaged external-plugin acceptance is claimed. CI status is a separate gate.
7dcc677 to
67285a2
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested. Reviewed head 67285a2657495f72b9f19f87df5c5a1fee7a7695 against base 119195ea331de33c8480bab180df0091ca8e9421.
- P1, newly identified: both desktop host operations are blocked by the app ACL; details and fix inline.
- P2, still unresolved: the Windows spawn-before-job-assignment finding is unchanged. Its existing fix and deterministic ordering-regression criteria still apply; no duplicate thread added.
The reload-disclosure and child-PATH fixes remain intact. Merge criteria are the ACL wiring/regression described inline and the Windows process-tree startup-ordering fix in the existing thread, or explicit unavailability of that operation on Windows.
Validation: source/API-contract review with independent checks, not a desktop or Windows runtime reproduction. Automatic hosted checks passed; Windows validation was skipped. No suites rerun, app launch, product edits, or live credentials. Packaged external-plugin/credential-CLI acceptance remains unverified. GitHub currently reports merge conflicts, a separate integration gate.
Signed-off-by: Matthew Boston <mboston@squareup.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
Signed-off-by: Matthew Boston <mboston@squareup.com>
67285a2 to
994c8de
Compare
Signed-off-by: Matthew Boston <mboston@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Re-review: no remaining code blockers. Reviewed head 7f6c03b843361471821932958e2eab81ebee49ce against base bebcd66e0adb27efa2c320f2bf72b8f3e63a8a45. This is a review comment, not an approval.
- The desktop ACL finding is resolved: both commands are wired through the generated permissions and main-WebView capability, with guest/remote denial regression coverage.
- The Windows startup-ordering finding is resolved in source: the child starts suspended, joins its kill-on-close job, then resumes. The new test checks the actual suspend count before assignment and subsequent descendant cleanup. Reload-disclosure and child-PATH fixes remain intact. Independent review found no further blockers.
- Hosted CI passes JavaScript (3,839 tests), Linux Rust/tool integration including the ACL regression, Chromium and browser measurements. Both WebKit shards were still pending at the final snapshot; Windows validation was skipped. CI tested synthetic merge
83cd3be4d22ca6b26d448234143b270e984650fcwith main220fab4ea1bca952d9fb624dffa3af860663d818, not the head-only tree.
Windows runtime and packaged external-plugin/credential-CLI plus real HTTPS acceptance remain unverified. No local suites, app launch, product edits or real credentials were used. Complete required CI separately; this closes the prior code findings, not those platform acceptance gaps.
…-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) ...
Why
Plugin authors need local credential tools and service APIs. A named authentication provider and fixed renderer network origin require a Buzz change for each new integration.
What
External plugins can declare exact local commands and HTTPS origins in their manifest. Buzz shows those declarations before install or update, checks the enabled installed revision, and provides bounded native command output and HTTPS requests. Each plugin interprets its own credentials and presents its own sign-in settings. An issue tracker or release dashboard can use its service without adding provider-specific Buzz code.
Folder reload rejects changed host access until the user imports and reviews the plugin again. Commands receive the same search path Buzz uses to find the executable. Cancellation and timeout terminate command descendants on Unix and Windows.
Browser builds do not offer native host operations. Plugins remain trusted code in one WebView; declarations help users review access but do not sandbox plugins.
This PR also verifies that a profile without an agent hint makes only the expected profile and status reads, and waits for narrow thread layout to settle after viewport resize.
Test plan