Classify browser_attach_* tools in ToolAccessProfiles - #4
Merged
Conversation
The assistant profile now permits the attach tool family (same 'search' category as the launch-mode browser tools); system-only profiles still exclude them, and a per-agent override can lock them off inside any profile.
Reviewer's GuideClassifies new browser_attach_* tools under the existing 'search' category and verifies that the assistant tool access profile permits them while still respecting per-agent override blocking. Sequence diagram for browser_attach_* tool authorization via ToolAccessProfilessequenceDiagram
actor User
participant Agent
participant ToolAccessProfiles
participant BrowserAttachTool
User->>Agent: request browser_attach_read
Agent->>ToolAccessProfiles: getToolCategory(browser_attach_read)
ToolAccessProfiles-->>Agent: search
Agent->>Agent: isToolAllowedForProfile(search, assistant)
Agent-->>User: tool permitted
Agent->>BrowserAttachTool: execute browser_attach_read
BrowserAttachTool-->>Agent: result
Agent-->>User: return research result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider extracting the list of
browser_attach_*tool IDs into a shared constant used both inTOOL_CATEGORY_MAPand the tests to avoid duplication and the risk of the lists diverging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the list of `browser_attach_*` tool IDs into a shared constant used both in `TOOL_CATEGORY_MAP` and the tests to avoid duplication and the risk of the lists diverging.
## Individual Comments
### Comment 1
<location path="src/autonomy/social/__tests__/ToolAccessProfiles.test.ts" line_range="387-396" />
<code_context>
+ }
+ });
+
+ it('assistant profile permits attach tools, and an override can still block them', () => {
+ const assistant = getToolAccessProfile('assistant');
+ expect(isToolAllowedByProfile(assistant, 'browser_attach_goto')).toBe(true);
+ expect(isToolAllowedByProfile(assistant, 'browser_attach_read')).toBe(true);
+ // A per-agent override wins over the category allow — attach can be locked
+ // off even inside the assistant profile.
+ expect(
+ isToolAllowedByProfile(assistant, 'browser_attach_goto', { additionalBlocked: ['browser_attach_goto'] }),
+ ).toBe(false);
+ });
});
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary assertion for a non-assistant profile to prove attach tools are blocked as intended.
This test confirms that the `assistant` profile allows attach tools and that per-agent overrides can still block them. To fully cover the behavior in the PR, please also add an assertion for a non-search-allowed profile (e.g. `getToolAccessProfile('system')`) verifying that `isToolAllowedByProfile(systemProfile, 'browser_attach_goto')` is `false`. That will prove that classifying these tools under `search` correctly prevents their use in system-only or other restricted profiles.
```suggestion
it('assistant profile permits attach tools, and an override can still block them', () => {
const assistant = getToolAccessProfile('assistant');
expect(isToolAllowedByProfile(assistant, 'browser_attach_goto')).toBe(true);
expect(isToolAllowedByProfile(assistant, 'browser_attach_read')).toBe(true);
// A per-agent override wins over the category allow — attach can be locked
// off even inside the assistant profile.
expect(
isToolAllowedByProfile(assistant, 'browser_attach_goto', { additionalBlocked: ['browser_attach_goto'] }),
).toBe(false);
// Non-search-allowed profiles (e.g. system) must not be able to use attach tools.
const systemProfile = getToolAccessProfile('system');
expect(isToolAllowedByProfile(systemProfile, 'browser_attach_goto')).toBe(false);
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+387
to
+396
| it('assistant profile permits attach tools, and an override can still block them', () => { | ||
| const assistant = getToolAccessProfile('assistant'); | ||
| expect(isToolAllowedByProfile(assistant, 'browser_attach_goto')).toBe(true); | ||
| expect(isToolAllowedByProfile(assistant, 'browser_attach_read')).toBe(true); | ||
| // A per-agent override wins over the category allow — attach can be locked | ||
| // off even inside the assistant profile. | ||
| expect( | ||
| isToolAllowedByProfile(assistant, 'browser_attach_goto', { additionalBlocked: ['browser_attach_goto'] }), | ||
| ).toBe(false); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Add a complementary assertion for a non-assistant profile to prove attach tools are blocked as intended.
This test confirms that the assistant profile allows attach tools and that per-agent overrides can still block them. To fully cover the behavior in the PR, please also add an assertion for a non-search-allowed profile (e.g. getToolAccessProfile('system')) verifying that isToolAllowedByProfile(systemProfile, 'browser_attach_goto') is false. That will prove that classifying these tools under search correctly prevents their use in system-only or other restricted profiles.
Suggested change
| it('assistant profile permits attach tools, and an override can still block them', () => { | |
| const assistant = getToolAccessProfile('assistant'); | |
| expect(isToolAllowedByProfile(assistant, 'browser_attach_goto')).toBe(true); | |
| expect(isToolAllowedByProfile(assistant, 'browser_attach_read')).toBe(true); | |
| // A per-agent override wins over the category allow — attach can be locked | |
| // off even inside the assistant profile. | |
| expect( | |
| isToolAllowedByProfile(assistant, 'browser_attach_goto', { additionalBlocked: ['browser_attach_goto'] }), | |
| ).toBe(false); | |
| }); | |
| it('assistant profile permits attach tools, and an override can still block them', () => { | |
| const assistant = getToolAccessProfile('assistant'); | |
| expect(isToolAllowedByProfile(assistant, 'browser_attach_goto')).toBe(true); | |
| expect(isToolAllowedByProfile(assistant, 'browser_attach_read')).toBe(true); | |
| // A per-agent override wins over the category allow — attach can be locked | |
| // off even inside the assistant profile. | |
| expect( | |
| isToolAllowedByProfile(assistant, 'browser_attach_goto', { additionalBlocked: ['browser_attach_goto'] }), | |
| ).toBe(false); | |
| // Non-search-allowed profiles (e.g. system) must not be able to use attach tools. | |
| const systemProfile = getToolAccessProfile('system'); | |
| expect(isToolAllowedByProfile(systemProfile, 'browser_attach_goto')).toBe(false); | |
| }); |
Mission emits CSV but not HTML/PDF; assistant research missions want a shareable report. Deterministic CSV (RFC-4180 escaping) + self-contained print-safe HTML + PDF via an ISOLATED headless Chrome --user-data-dir (never the live profile). PDF is best-effort; CSV + HTML always land.
…verify Guide covers the contract, prerequisites, transports, running, export, and troubleshooting. Example mission demonstrates the read-only attach flow. The E2E verify (WUNDERLAND_ATTACH_E2E-gated) asserts other tabs stay byte-identical across a session — the marker-tab contract.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the five
browser_attach_*tool ids toTOOL_CATEGORY_MAPundersearch, so theassistantprofile permits attach-mode browsing while system-only profiles don't. Override-blocking still works per agent. Pairs with framerslab/agentos-extensions#35 (the tools themselves).Summary by Sourcery
Classify browser attach-mode tools under the search category and verify assistant-profile access and override behavior.
Enhancements:
Tests: