feat: add compliant MCP tool surface via MCP_COMPLIANCE_MODE [DO NOT MERGE]#166
Open
Xrazik1 wants to merge 3 commits into
Open
feat: add compliant MCP tool surface via MCP_COMPLIANCE_MODE [DO NOT MERGE]#166Xrazik1 wants to merge 3 commits into
Xrazik1 wants to merge 3 commits into
Conversation
Contributor
WalkthroughAdds ChangesCompliance Mode Implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Environment
participant getConfig
participant registerSurface
participant CompliancePolicy
participant MCPTools
participant StatusResource
Environment->>getConfig: Read MCP_COMPLIANCE_MODE
getConfig->>registerSurface: Provide complianceMode
registerSurface->>CompliancePolicy: Evaluate isCompliant(config)
CompliancePolicy->>MCPTools: Select compliant or full registrations and schemas
MCPTools-->>registerSurface: Register selected surface
StatusResource-->>Environment: Report compliant or full surface
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Xrazik1
force-pushed
the
feat/connector-mode-poc
branch
from
d2ba0a6 to
7880934
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/index.ts`:
- Around line 124-135: The compliance surface logging in src/index.ts only
distinguishes compliant, unset, and explicit opt-out, but it silently treats
invalid MCP_COMPLIANCE_MODE values as compliant; update the startup logic around
registerSurface and the complianceSurface computation to detect any non-accepted
normalized value and emit an explicit warning before logging the surface. Use
the existing parseComplianceMode/complianceMode flow to identify invalid inputs,
and keep the current console.error boot log while adding a clear warning path
for typoed or mis-scoped environment values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44dbf51f-90a0-4761-94fa-77fd1e06e826
📒 Files selected for processing (11)
README.mdsrc/@types/types.d.tssrc/config.tssrc/index.tssrc/resources/status.tssrc/skills/cookie-consent.mdsrc/skills/dynamic-content.mdsrc/skills/index.tssrc/skills/modals.mdsrc/skills/screenshots.mdsrc/skills/shadow-dom.md
✅ Files skipped from review due to trivial changes (5)
- src/skills/dynamic-content.md
- src/skills/screenshots.md
- src/skills/cookie-consent.md
- src/skills/modals.md
- README.md
Xrazik1
force-pushed
the
feat/connector-mode-poc
branch
from
7880934 to
dd2239c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/tools/compliance.ts (1)
27-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHand-synced skill list risks drift.
COMPLIANT_SKILLS(Line 27) andCOMPLIANT_SKILL_TOOL_DESCRIPTION's bullet list (Line 82) are currently in sync but independently maintained. Adding/removing an id from the allowlist won't update the description text, and nothing will catch the mismatch.Consider generating the bullet list from
COMPLIANT_SKILLS(or a shared id→blurb map) so the two can't diverge.Also applies to: 77-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/compliance.ts` around lines 27 - 36, Eliminate the independently maintained skill bullet list by deriving COMPLIANT_SKILL_TOOL_DESCRIPTION from COMPLIANT_SKILLS, or from a shared skill-id-to-blurb mapping that also defines the allowlist. Ensure adding or removing a compliant skill automatically updates the description while preserving the existing ordering and wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/schemas.ts`:
- Around line 751-784: Update compliantCommandSchemas to remove the
action-capable ClickCommandSchema, TypeCommandSchema, SelectCommandSchema,
CheckboxCommandSchema, HoverCommandSchema, and UploadFileCommandSchema, keeping
only commands permitted by the navigation/read-only policy. Since
COMPLIANT_AGENT_METHODS derives from this list, ensure both schema validation
and the run-layer guard reject each removed method, and add rejection tests
covering all six methods.
---
Nitpick comments:
In `@src/tools/compliance.ts`:
- Around line 27-36: Eliminate the independently maintained skill bullet list by
deriving COMPLIANT_SKILL_TOOL_DESCRIPTION from COMPLIANT_SKILLS, or from a
shared skill-id-to-blurb mapping that also defines the allowlist. Ensure adding
or removing a compliant skill automatically updates the description while
preserving the existing ordering and wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0eab4659-a07c-43c3-9c93-8bf472d7c54f
📒 Files selected for processing (34)
README.mdsrc/@types/types.d.tssrc/config.tssrc/index.tssrc/resources/status.tssrc/skills/cookie-consent.mdsrc/skills/dynamic-content.mdsrc/skills/index.tssrc/skills/modals.mdsrc/skills/screenshots.mdsrc/skills/shadow-dom.mdsrc/skills/snapshot-misses.mdsrc/tools/agent.tssrc/tools/compliance.tssrc/tools/export.tssrc/tools/performance.tssrc/tools/register.tssrc/tools/schemas.tssrc/tools/search.tstest/integration/server.spec.tstest/lib/api-client.spec.tstest/lib/config.spec.tstest/resources/resources.spec.tstest/skills/skills.spec.tstest/tools/agent.spec.tstest/tools/annotations.spec.tstest/tools/compliance-mode.spec.tstest/tools/crawl.spec.tstest/tools/export.spec.tstest/tools/function.spec.tstest/tools/map.spec.tstest/tools/performance.spec.tstest/tools/search.spec.tstest/tools/smartscraper.spec.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- src/skills/screenshots.md
- src/skills/cookie-consent.md
- src/skills/dynamic-content.md
- src/tools/export.ts
- src/resources/status.ts
- README.md
- src/skills/shadow-dom.md
- src/skills/modals.md
- src/index.ts
- src/skills/index.ts
- src/tools/agent.ts
Xrazik1
force-pushed
the
feat/connector-mode-poc
branch
from
dd2239c to
f5293aa
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/tools/compliance-mode.spec.ts`:
- Around line 274-283: Update the compliance-mode test to define the eight
approved skill IDs as an independent expected allowlist rather than deriving
them from COMPLIANT_SKILLS. Use this fixed list to assert both the
COMPLIANT_SKILLS contents and schema acceptance, ensuring accidental changes to
COMPLIANT_SKILLS cannot make the assertions pass.
- Around line 439-450: Update the runtime allowlist test in
compliance-mode.spec.ts to invoke an unknown method instead of evaluate,
ensuring it verifies fail-closed behavior for non-allowlisted commands. Leave
the later batch-index test responsible for covering the known prohibited
evaluate method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e58d497-6929-4fce-acbc-0220dd45ecf6
📒 Files selected for processing (34)
README.mdsrc/@types/types.d.tssrc/config.tssrc/index.tssrc/resources/status.tssrc/skills/cookie-consent.mdsrc/skills/dynamic-content.mdsrc/skills/index.tssrc/skills/modals.mdsrc/skills/screenshots.mdsrc/skills/shadow-dom.mdsrc/skills/snapshot-misses.mdsrc/tools/agent.tssrc/tools/compliance.tssrc/tools/export.tssrc/tools/performance.tssrc/tools/register.tssrc/tools/schemas.tssrc/tools/search.tstest/integration/server.spec.tstest/lib/api-client.spec.tstest/lib/config.spec.tstest/resources/resources.spec.tstest/skills/skills.spec.tstest/tools/agent.spec.tstest/tools/annotations.spec.tstest/tools/compliance-mode.spec.tstest/tools/crawl.spec.tstest/tools/export.spec.tstest/tools/function.spec.tstest/tools/map.spec.tstest/tools/performance.spec.tstest/tools/search.spec.tstest/tools/smartscraper.spec.ts
🚧 Files skipped from review as they are similar to previous changes (30)
- src/skills/dynamic-content.md
- test/tools/crawl.spec.ts
- src/skills/screenshots.md
- test/tools/export.spec.ts
- src/@types/types.d.ts
- test/tools/annotations.spec.ts
- test/tools/search.spec.ts
- test/tools/map.spec.ts
- test/lib/api-client.spec.ts
- test/integration/server.spec.ts
- src/resources/status.ts
- test/tools/function.spec.ts
- src/tools/search.ts
- test/tools/smartscraper.spec.ts
- src/skills/cookie-consent.md
- src/skills/shadow-dom.md
- test/resources/resources.spec.ts
- src/skills/snapshot-misses.md
- src/config.ts
- src/tools/schemas.ts
- src/tools/export.ts
- src/skills/index.ts
- test/skills/skills.spec.ts
- src/skills/modals.md
- test/lib/config.spec.ts
- src/tools/performance.ts
- src/tools/register.ts
- src/tools/compliance.ts
- src/index.ts
- src/tools/agent.ts
Xrazik1
force-pushed
the
feat/connector-mode-poc
branch
from
f5293aa to
204e64f
Compare
andyMrtnzP
requested changes
Jul 13, 2026
…le I/O, proxy defense-in-depth
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/tools/compliance.ts (1)
73-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSkill bullet list duplicated in two files.
The same 7-skill bullet list (shadow-dom, cookie-consent, modals, snapshot-misses, dynamic-content, screenshots, tabs) is hand-maintained here and again in
COMPLIANT_AGENT_SYSTEM_PROMPT(src/skills/system-prompt.ts). SinceCOMPLIANT_SKILLSis the actual source of truth, consider generating both bullet lists from it (or fromskillsRegistrymetadata) to avoid manual drift when a skill is added/removed. The PR mentions drift-protection tests, which mitigates the risk somewhat.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/compliance.ts` around lines 73 - 84, Update COMPLIANT_SKILL_TOOL_DESCRIPTION and COMPLIANT_AGENT_SYSTEM_PROMPT to derive their skill bullet lists from the authoritative COMPLIANT_SKILLS or skillsRegistry metadata instead of maintaining duplicated literals. Preserve the existing bullet formatting and descriptions, and ensure additions or removals in the source-of-truth collection automatically appear in both prompts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/tools/compliance.ts`:
- Around line 73-84: Update COMPLIANT_SKILL_TOOL_DESCRIPTION and
COMPLIANT_AGENT_SYSTEM_PROMPT to derive their skill bullet lists from the
authoritative COMPLIANT_SKILLS or skillsRegistry metadata instead of maintaining
duplicated literals. Preserve the existing bullet formatting and descriptions,
and ensure additions or removals in the source-of-truth collection automatically
appear in both prompts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4bc4244a-904b-4e22-956a-6cab591fa7be
📒 Files selected for processing (7)
src/@types/types.d.tssrc/lib/agent-client.tssrc/skills/system-prompt.tssrc/tools/agent.tssrc/tools/compliance.tssrc/tools/schemas.tstest/tools/compliance-mode.spec.ts
💤 Files with no reviewable changes (1)
- test/tools/compliance-mode.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/tools/schemas.ts
- src/@types/types.d.ts
- src/tools/agent.ts
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
2 participants
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.
Summary
Adds an opt-in compliant tool surface to the MCP server, selected per-process by the
MCP_COMPLIANCE_MODEenv var. When enabled, the server advertises a reduced, policy-compliant surface intended for the OpenAI / Anthropic app-directory listings (which reject the full surface for arbitrary code, scraping, and circumvention). The default full surface (flag off) is unchanged — verified byte-identical tomain.The compliant policy is centralized in
src/tools/compliance.ts; registration is gated insrc/tools/register.ts.What changes in compliant mode
smartscraper,function,map, andcrawl, plus the smartscraper-centricapi-docsresource and thescrape-url/extract-contentprompts. Registration is a data-driven surface table — every tool is classifiedboth/full, so a newly added tool is off the compliant surface by default.browserless_agentde-fanged at the schema level (strict — rejects, never silently strips): navigation/read commands only; nosolve/evaluate/loadSecret; no raw-BQL method passthrough; no top-levelproxy,profile, orcreateProfile.profilehydrates a saved auth session, so it is dropped as circumvention-adjacent.export/search/performanceuse.pick().strict()allowlists: dropscrapeOptions(search),includeResources(export), andprofile(all three). Run-layer guards back each schema.captchas,autonomous-login,auth-profile). The 8 allowed skill bodies are de-fanged at render — full-only passages (evaluate techniques, captcha selectors) are stripped via<!-- compliant-omit -->markers, with<!-- compliant-only -->replacements so each recipe stays coherent. Markers are validated at load and fail closed on anything malformed. Site-recipe pointers are suppressed in compliant agent replies (recipes can prescribe proxy/evaluate/login).surface: "compliant" | "full"; the boot log states the active surface.MCP_COMPLIANCE_MODEparsing is fail-closed — any set value other thanfalse/0/no/offenables the compliant surface.Testing
test/tools/compliance-mode.spec.tsguards the surface (exact tool set, schema accept/reject, run-layer defense, skill-body de-fang, marker validation, drift guards);test/lib/config.spec.tscovers fail-closed flag parsing. 515 passing, lint clean.Deploy
Full stays on
mcp.browserless.io/mcp; compliant serves at/mcp/connector(a second container withMCP_COMPLIANCE_MODE=true). Path routing is infra, not this repo — browserless/terraform#293 (connector container + LB) and browserless/load-balancer#94 (nginx route). This image is the prerequisite.[DO NOT MERGE] — PoC.
Summary by CodeRabbit
MCP_COMPLIANCE_MODEto enable a reduced, directory-compliant tool surface, using fail-closed parsing with startup warnings for unrecognized values.complianceModein runtime configuration and centralized surface registration to differentiate compliant vs full availability.compliantvsfull).MCP_COMPLIANCE_MODEin the environment variables section.