feat(plugins): expose the agent protection service - #421
Conversation
| #[test] | ||
| #[cfg(unix)] | ||
| fn provider_runs_through_controller_start_restart_and_missing_provider_fails_closed() { | ||
| for worker in ["buzz-agent", "goose"] { |
There was a problem hiding this comment.
we should extract this out so more support can be added over time
| let tools = tempfile::tempdir().unwrap(); | ||
| let mut saved = agent(dir.path()); | ||
| let runtime = bundle(tools.path()); | ||
| if worker == "goose" { |
There was a problem hiding this comment.
Also not generic
| fs::write( | ||
| &provider, | ||
| r#"#!/bin/sh | ||
| [ "$1" = --launch ] || exit 2 |
There was a problem hiding this comment.
Why do this
8c4af10 to
d841c28
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Two lifecycle blockers; details and fixes are in the inline threads. Please add regressions for both orderings before merging.
Reviewed head d841c2815ec1b09464af1cf49ea536a7140c5138 against stacked base f3d36738272b8d765e26899a4a958a2f60a84442. Independent native integration review plus isolated probes using the actual Cordis, AgentSecurityService and PluginRuntime confirmed the failure paths; native transport was injected. The existing in-flight-registration disposal ordering also passed a control probe. No source changes or live-agent operations.
Hosted JavaScript and Rust/tool integration passed. Required CI remains red on the WebKit new-message viewport assertion; I have not established a connection to this change or classified it as a flake. Live provider/Goose UI and human acceptance remain unverified, as noted in the PR description.
| const pending = this.host<{ lease: string }>({ | ||
| kind: "register", | ||
| provider, | ||
| executable, | ||
| }); |
There was a problem hiding this comment.
[P1] Establish the scope's cleanup ownership before sending registration
A callback can outlive plugin disable (for example, launcher discovery finishes after unload). The retained service still has pluginOwner, so this sends the native register request. Only afterward does line 76 call ctx.effect(), which Cordis rejects for a disposed/unloading scope. The function exits without installing cleanup, and the native provider remains registered despite the plugin being disabled. It can also replace a newer registration of the same plugin ID and trigger the native auto-restore path.
Reproduced with the actual service and Cordis: retain scope.agentSecurity, dispose the plugin, then call register(). The call rejects with cannot create effect on inactive context, but the transport receives register and never unregister, even after root disposal. The existing test only covers registration begun before disposal.
Establish an active, cleanup-owning effect before invoking native registration; reject a retired scope without sending IPC. Add a regression for registration initiated after disposal, retaining coverage for disposal while registration is in flight.
| } | ||
| _ => None, | ||
| }; | ||
| let result = run(owner.clone(), move |host| host.controller.security(request)).await?; |
There was a problem hiding this comment.
[P2] Wait for native host readiness before activating protection providers
AgentHost::initialize() returns immediately with an error placeholder while a background task hashes the runtime resources, opens/migrates the store, and installs Host (lines 471–500). Plugin catalog loading runs independently, and agentSecurity is immediately injectable. An enabled provider that awaits register() during apply() can therefore reach this run() before initialization finishes and receive Agent runtime is initializing; retry shortly.
That transient condition becomes permanent for this activation: PluginRuntime marks the plugin failed, while unchanged catalog reconciliation does not retry it. Protected start-on-launch agents then fail closed with no registered provider and stay down until an explicit plugin retry/reload. A production-service/runtime probe returning that exact native error confirmed that making the host ready and reconciling the same catalog leaves the plugin failed with only one registration attempt.
Gate registration on host readiness, or use a bounded retry restricted to this known pre-execution initialization error (not ambiguous registration failures). Add an integration regression holding host initialization until plugin activation has attempted registration, then release it and verify successful provider registration and protected restore.
d841c28 to
b287d15
Compare
f3d3673 to
a4f6457
Compare
b287d15 to
7ca1a0c
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Both previous blockers are fixed; no new material defects found in this re-review. Reviewed head 7ca1a0cd8535b3627373fc86a64d8a0be764d662 against stacked base a4f64575dddda0d79cebc4b6356b786e55710a54.
- P1 cleanup ownership: the active Cordis effect now owns cleanup before registration IPC. Regression coverage includes a retained service after disposal, in-flight registration disposal, one-shot cleanup, and failed registration.
- P2 native initialization: initialization holds admission until the real host result is installed, releasing it before restore re-enters. The held-initialization, failure, and shutdown regressions pass; an independent native lifecycle review found no blocker.
- Current JavaScript and Rust CI passed, including all four security-service tests and both new native regressions. CI merge
baa941c9f3d6ecf4a91d8f014b0834cfa7ffade7has the same tree as this head. No local suite duplication or live-app operations in this review.
Readiness remains incomplete: CI required failed with three browser shards cancelled; Windows validation was skipped. Live app/relay and human acceptance remain pending in the PR description. This clears my two code findings, not those acceptance gates. Comment-only review, not approval.
a4f6457 to
727c513
Compare
7ca1a0c to
bbe8e3d
Compare
727c513 to
05b6f46
Compare
374cec1 to
0a6ff97
Compare
05b6f46 to
d6a403f
Compare
Signed-off-by: Alex Rosenzweig <arosenzweig@squareup.com>
0a6ff97 to
cb89276
Compare
* origin/main: (82 commits) Test provider connections before model selection (#500) Bundle Goose ACP with Buzz (#497) Discover saved identities across joined communities with names, pictures and retry (#291) Clarify design-system documentation and unify component examples (#498) feat(composer): convert typed Markdown live and refuse control characters committed as text (#455) fix(messages): stop three timeline scroll races that flake CI (#456) Improve Agent defaults pickers and provider keys (#392) fix(threads): keep thread history painted after scroll corrections (#493) feat(plugins): expose the agent protection service (#421) perf(sidebar): re-render only the changed row on a channel-list publish (#480) feat(agents): copy protection defaults into new agents (#420) feat(agents): support native launch protection providers (#415) fix(composer): prevent WebKit overpainting mention selections (#490) fix(composer): prevent arrow keys from inserting control characters (#488) perf(channels): fall back to one exact roster read when confirming agent adds (#485) fix(media): pause video only on comment composer focus (#483) fix(channels): dismiss management modals with outside clicks (#479) perf: reuse message date formats and stable reaction shortcuts (#477) feat(profile): run an unattended scenario file in web profiling (#476) feat(channels): administer channel members and roles (#453) ... Signed-off-by: John Tennant <jtennant@block.xyz> # Conflicts: # src/app/shell/usePanelLauncher.ts # src/bundled/agents/AgentsPage.tsx # src/bundled/agents/InventoryIdentityCard.tsx # src/bundled/agents/InventoryView.tsx # src/bundled/agents/UnifiedInventory.tsx # src/bundled/agents/index.tsx
* origin/main: (82 commits) Test provider connections before model selection (#500) Bundle Goose ACP with Buzz (#497) Discover saved identities across joined communities with names, pictures and retry (#291) Clarify design-system documentation and unify component examples (#498) feat(composer): convert typed Markdown live and refuse control characters committed as text (#455) fix(messages): stop three timeline scroll races that flake CI (#456) Improve Agent defaults pickers and provider keys (#392) fix(threads): keep thread history painted after scroll corrections (#493) feat(plugins): expose the agent protection service (#421) perf(sidebar): re-render only the changed row on a channel-list publish (#480) feat(agents): copy protection defaults into new agents (#420) feat(agents): support native launch protection providers (#415) fix(composer): prevent WebKit overpainting mention selections (#490) fix(composer): prevent arrow keys from inserting control characters (#488) perf(channels): fall back to one exact roster read when confirming agent adds (#485) fix(media): pause video only on comment composer focus (#483) fix(channels): dismiss management modals with outside clicks (#479) perf: reuse message date formats and stable reaction shortcuts (#477) feat(profile): run an unattended scenario file in web profiling (#476) feat(channels): administer channel members and roles (#453) ... Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/app/shell/usePanelLauncher.ts # src/bundled/agents/AgentsPage.tsx # src/bundled/agents/InventoryIdentityCard.tsx # src/bundled/agents/InventoryView.tsx # src/bundled/agents/UnifiedInventory.tsx # src/bundled/agents/index.tsx
Expose native launch protection through the trusted-plugin
agentSecurityservice and desktop IPC. Plugins register launchers, save global or per-agent policies, and release registrations on unload. Policy UI, experimental opt-in and enforcement belong to the external plugin.Registration acquires scope cleanup before native IPC; native requests wait for host initialization and return its actual result. The plugin store is included in protected control paths. This layer adds the plugin API section to the contract documentation supplied by #415/#420.
Stack
main, after feat(agents): copy protection defaults into new agents #420Validation
Final head
cb892760is based directly onmainat4e9bf8cc, containing the merged defaults layer. The plugin feature patch is unchanged by rebase. Main now contains the sidebar scroll synchronization fix, so the earlier duplicate test-only commit was removed; this PR no longer changes browser tests.cb892760, the mandatory push checks passed again: TypeScript, 48 related frontend tests, design checks and workspace Clippy.Real Goose/inference, native UI/human acceptance and non-macOS enforcement have not been verified by this work.
Hosted CI
All required CI lanes passed at
cb892760, including all twelve browser shards. DCO and security checks passed. One WebKit job was retried after a GitHub HTTP 500 prevented downloading pnpm before tests began; the rerun passed without code or test-setting changes. Squash-merged as55f96c25.