Accept root plugin.json manifest and grant plugin dir access

[Wenjie Fan (gggdttt)]

What

Small fixes to the shared plugin loader so a read-heavy config plugin can be consumed during an evaluation, scoped so they do not broaden the loader's contract for agents or plugins that don't need it:

1. Accept a root plugin.json manifest - for Copilot only. _resolve_plugin previously required the manifest at .claude-plugin/plugin.json. Copilot CLI also loads a plugin.json at the plugin root (BCQuality keeps its manifest there), while Claude Code loads only the nested layout. resolve_config_plugins now takes allow_root_manifest: the Copilot agent passes True (root or nested accepted); the Claude agent keeps the strict default (False), so a root-only plugin is not forwarded to a Claude run that would silently ignore it. The AgentError message lists the location(s) actually accepted for that run.
2. Grant directory access only to plugins that opt in. run_agent passes --plugin-dir <dir> for every enabled plugin, which registers/auto-discovers its skills, but Copilot CLI keeps directory access as a sandbox separate from --allow-all-tools, so a knowledge-heavy plugin registers its skill yet cannot read its own files. --add-dir closes that gap - but it grants read+write, so applying it to every plugin would over-grant and couple "plugin enabled" with "directory access granted." A new per-plugin grant_dir_access flag (default false) gates it: the Copilot agent passes --add-dir only for plugins that set it, mirroring how the production engine grants --add-dir for its knowledge root.
3. Docstring clarifies which manifest layout each CLI loads.

Why

BC-Bench already loads config plugins via resolve_config_plugins -> --plugin-dir (the existing plugin mechanism). These gaps are what stop that mechanism working for a read-heavy plugin end-to-end, and the fixes are scoped so they change behavior for no other agent or plugin.

Scope / safety

• No shipped-config changes. All plugins remain enabled: false and no plugin sets grant_dir_access; the TestShippedConfig invariants (test_shipped_entries_are_disabled_by_default, test_shipped_config_resolves_on_any_os) stay green. Enabling a plugin (and granting it dir access) for a specific run is a separate, experiment-only change.
• Claude path stays strict. The shared resolver keeps the nested-only contract for Claude, so this PR cannot make a root-only plugin pass validation for a Claude run.

Tests

• test_local_plugin_with_root_manifest_resolves_for_copilot (root manifest resolves under allow_root_manifest=True) and test_root_manifest_is_rejected_without_opt_in (the default rejects it).
• TestPluginsNeedingDirAccess: only enabled, opted-in plugins receive dir access; none by default.
• Plugin suite green; ruff check clean.

[Copilot]

Pull request overview

Adds root-manifest plugin support and grants Copilot CLI access to configured plugin directories.

Changes:

• Recognizes both nested and root plugin.json manifests.
• Adds --add-dir for Copilot plugin directories.
• Tests root-manifest resolution.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
tests/test_agent_plugins.py	Tests root-manifest resolution.
src/bcbench/agent/shared/plugin.py	Broadens manifest discovery.
src/bcbench/agent/copilot/agent.py	Grants plugin-directory access.

[Wenjie Fan (gggdttt)]

You're right, the message did change. Rather than revert it, I updated the PR description to match: the error now lists the location(s) actually accepted for the run ({nested} for Claude, {nested} or {root} for Copilot), which is the more useful behavior. (cbf6a22)

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/bcbench/types.py:134

• Every shipped plugin entry in src/bcbench/agent/shared/config.yaml:104-120, including BCQuality, omits this new field, so the default remains false even when that plugin is enabled. Consequently the read-heavy plugin still receives no --add-dir, contrary to the PR's stated end-to-end fix and its claim that access is added for each configured plugin directory. Mark the intended entry as opted in (it can remain disabled), or change the contract to grant access to all enabled plugins.

    # Grant the agent filesystem access to this plugin's tree via Copilot `--add-dir`.
    # Off by default so enabling a plugin does not also widen the agent's sandbox access.
    grant_dir_access: bool = False

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

src/bcbench/agent/copilot/agent.py:94

• The PR description says TestPluginsNeedingDirAccess verifies that only enabled, opted-in plugins receive --add-dir and that none do by default, but no such tests or equivalent command assertions are included in the changed test file. This permission-boundary behavior is therefore both untested and inconsistent with the stated test coverage; add the described cases or correct the PR's Tests section.

        cmd_args.extend(
            f"--add-dir={plugins[PluginConfig(**plugin_entry).record]}" for plugin_entry in copilot_config["plugins"] if plugin_entry.get("enabled") and plugin_entry.get("grant_dir_access")
        )

src/bcbench/types.py:136

• This flag materially changes the agent's filesystem capability, but it is not carried into ExperimentConfiguration: run_copilot_agent still records only the plugin records, and result export serializes that model. Consequently, runs with and without read/write access produce identical experiment metadata, so benchmark results cannot be reproduced or distinguished. Record the opted-in plugin records in the experiment configuration as well.

    # Grant the agent filesystem access to this plugin's tree via Copilot `--add-dir`.
    # Off by default so enabling a plugin does not also widen the agent's sandbox access.
    # This is a (hopefully temporary) accommodation for BCQuality, whose skill reads its own
    # knowledge files at runtime; the long-term fix is to serve that content via skills / an MCP server.
    grant_dir_access: bool = False

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

src/bcbench/agent/shared/plugin.py:84

• This checks collisions only between the final plugin subfolders, not between GitHub clone destinations. Two entries with the same name but different revisions and different path values resolve to distinct subfolders, so this guard passes even though both clones use <plugin_root>/<name> and the second clone has already replaced the first. The first record can then point to a missing folder or to content from the wrong revision. Detect conflicting clone destinations before cloning, or make clone destinations unique.

    folders = [str(path) for path in resolved.values()]
    shared_folders = sorted({folder for folder in folders if folders.count(folder) > 1})
    if shared_folders:
        raise AgentError(f"Distinct plugins resolve to the same folder: {', '.join(shared_folders)}")

src/bcbench/agent/copilot/agent.py:94

• The permission decision uses raw YAML values instead of the validated PluginConfig booleans. Because Pydantic coerces values such as grant_dir_access: "false" to False while the non-empty raw string is truthy here, that configuration still receives read/write access. Base this security-sensitive opt-in on the parsed model so an explicit false value cannot widen the sandbox.

        cmd_args.extend(
            f"--add-dir={plugins[PluginConfig(**plugin_entry).record]}" for plugin_entry in copilot_config["plugins"] if plugin_entry.get("enabled") and plugin_entry.get("grant_dir_access")
        )

src/bcbench/agent/claude/agent.py:89

• The new opt-in is documented and described as Copilot-specific, but this also passes --add-dir to Claude for every opted-in plugin. That broadens filesystem access for a different agent despite the PR's stated scope that other agents are unchanged. Keep this argument construction on the Copilot path only (and remove the now-unused Claude import).

        # --add-dir grants read+write (unlike --plugin-dir, which only registers a plugin), so hand it
        # only to plugins that opt in via grant_dir_access - currently a temporary accommodation for
        # BCQuality, whose skill reads its own knowledge files at runtime. Enabling a plugin must not
        # silently widen the agent's sandbox access.
        cmd_args.extend(
            f"--add-dir={plugins[PluginConfig(**plugin_entry).record]}" for plugin_entry in claude_config["plugins"] if plugin_entry.get("enabled") and plugin_entry.get("grant_dir_access")
        )

src/bcbench/agent/copilot/agent.py:94

• The PR description says TestPluginsNeedingDirAccess verifies that only enabled, opted-in plugins receive --add-dir, but that test class is absent and the plugin tests never exercise command construction. The newly introduced permission behavior therefore is not covered as claimed; add the advertised command-argument tests, including the default/no-opt-in case.

        cmd_args.extend(
            f"--add-dir={plugins[PluginConfig(**plugin_entry).record]}" for plugin_entry in copilot_config["plugins"] if plugin_entry.get("enabled") and plugin_entry.get("grant_dir_access")
        )

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

src/bcbench/agent/copilot/agent.py:94

• This authorization decision reads the raw YAML value rather than the validated PluginConfig.grant_dir_access. Pydantic accepts values such as "false"/"0" and normalizes them to False, but those raw strings are truthy here, so a plugin explicitly configured not to receive directory access is still passed via --add-dir. Base both predicates on the parsed plugin while retaining the raw-enabled short circuit that keeps disabled, OS-specific entries inert.

        cmd_args.extend(
            f"--add-dir={plugins[PluginConfig(**plugin_entry).record]}" for plugin_entry in copilot_config["plugins"] if plugin_entry.get("enabled") and plugin_entry.get("grant_dir_access")
        )

src/bcbench/agent/copilot/agent.py:93

• This changes the agent's effective experiment configuration, but the returned ExperimentConfiguration still records only the plugin record. Consequently, runs with the same plugin and grant_dir_access set to true versus false serialize identically even though one has read/write access outside the workspace, making benchmark results impossible to audit or reproduce from their recorded configuration. Record the granted plugin records (or make the plugin result entries structured) when constructing ExperimentConfiguration.

        cmd_args.extend(
            f"--add-dir={plugins[PluginConfig(**plugin_entry).record]}" for plugin_entry in copilot_config["plugins"] if plugin_entry.get("enabled") and plugin_entry.get("grant_dir_access")

src/bcbench/agent/copilot/agent.py:93

• The PR description claims TestPluginsNeedingDirAccess verifies that only enabled, opted-in plugins receive --add-dir and that none do by default, but the test suite contains no such class or command-construction assertion; the added tests only exercise resolver behavior. This security-sensitive opt-in wiring can therefore regress (for example, by dropping either predicate) while the stated plugin suite remains green. Add the described Copilot command tests.

This issue also appears on line 92 of the same file.

        cmd_args.extend(
            f"--add-dir={plugins[PluginConfig(**plugin_entry).record]}" for plugin_entry in copilot_config["plugins"] if plugin_entry.get("enabled") and plugin_entry.get("grant_dir_access")

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/bcbench/agent/claude/agent.py:88

• grant_dir_access is documented as a Copilot-only capability, and the PR scope says this change does not broaden behavior for other agents, but this also emits --add-dir for Claude. Any opted-in plugin used in a Claude evaluation now receives read/write access outside the evaluation repository. Remove this Claude branch, or explicitly make and document the permission as cross-agent behavior.

        # --add-dir grants read+write (unlike --plugin-dir, which only registers a plugin), so hand it
        # only to plugins that opt in via grant_dir_access - currently a temporary accommodation for
        # BCQuality, whose skill reads its own knowledge files at runtime. Enabling a plugin must not
        # silently widen the agent's sandbox access. Read the opt-in from the validated PluginConfig, not
        # the raw YAML, so a non-boolean like `grant_dir_access: "false"` is not treated as truthy.
        cmd_args.extend(f"--add-dir={plugins[plugin.record]}" for plugin in (PluginConfig(**entry) for entry in claude_config["plugins"]) if plugin.enabled and plugin.grant_dir_access)

src/bcbench/agent/copilot/agent.py:93

• The PR description says TestPluginsNeedingDirAccess verifies the default, enabled, and opted-in cases, but that test is absent and no test exercises this command-construction branch. The new read/write permission path can therefore regress without detection; add the claimed tests for both granting and non-granting configurations.

        # --add-dir grants read+write (unlike --plugin-dir, which only registers a plugin), so hand it
        # only to plugins that opt in via grant_dir_access - currently a temporary accommodation for
        # BCQuality, whose skill reads its own knowledge files at runtime. Enabling a plugin must not
        # silently widen the agent's sandbox access. Read the opt-in from the validated PluginConfig, not
        # the raw YAML, so a non-boolean like `grant_dir_access: "false"` is not treated as truthy.
        cmd_args.extend(f"--add-dir={plugins[plugin.record]}" for plugin in (PluginConfig(**entry) for entry in copilot_config["plugins"]) if plugin.enabled and plugin.grant_dir_access)

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

src/bcbench/agent/copilot/agent.py:93

• The PR description says TestPluginsNeedingDirAccess verifies this permission plumbing, but no such test exists in this change or the repository; the added tests cover only manifest resolution and duplicate names. Add the claimed command-argument coverage for enabled/disabled and opted-in/default plugins, or correct the PR description.

        cmd_args.extend(f"--add-dir={plugins[plugin.record]}" for plugin in (PluginConfig(**entry) for entry in copilot_config["plugins"]) if plugin.enabled and plugin.grant_dir_access)

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (6)

src/bcbench/agent/copilot/agent.py:95

• The raw prefilter treats enabled: "false" as truthy, while PluginConfig parses it as False. Because this predicate checks only grant_dir_access, such a disabled entry can still receive --add-dir. Gate on the validated plugin.enabled value as well.

        cmd_args.extend(f"--add-dir={plugins[plugin.record]}" for plugin in (PluginConfig(**entry) for entry in copilot_config["plugins"] if entry.get("enabled", False)) if plugin.grant_dir_access)

src/bcbench/agent/claude/agent.py:90

• The raw prefilter treats enabled: "false" as truthy, while PluginConfig parses it as False. Since the final predicate ignores plugin.enabled, this Claude path can grant --add-dir to an entry whose validated configuration is disabled.

        cmd_args.extend(f"--add-dir={plugins[plugin.record]}" for plugin in (PluginConfig(**entry) for entry in claude_config["plugins"] if entry.get("enabled", False)) if plugin.grant_dir_access)

src/bcbench/types.py:136

• This flag materially changes an evaluation's filesystem capabilities, but ExperimentConfiguration still records only the plugin names. Runs with grant_dir_access on and off therefore serialize identical experiment metadata, making results impossible to distinguish or reproduce from their recorded configuration. Persist the granted plugin records in the experiment configuration as well.

    grant_dir_access: bool = False

src/bcbench/types.py:132

• This field is not Copilot-specific: the changed Claude runner also translates it to Claude's --add-dir. Describing it as a Copilot capability contradicts the implementation and can mislead future config consumers.

    # Grant the agent filesystem access to this plugin's tree via Copilot `--add-dir`.

src/bcbench/agent/copilot/agent.py:95

• The PR description says TestPluginsNeedingDirAccess verifies this permission boundary, but no such class or any --add-dir assertion exists in the test suite. The core behavior—only enabled, opted-in plugins receiving directory access in each runner—is therefore both untested and inaccurately described by the PR.

        cmd_args.extend(f"--add-dir={plugins[plugin.record]}" for plugin in (PluginConfig(**entry) for entry in copilot_config["plugins"] if entry.get("enabled", False)) if plugin.grant_dir_access)

src/bcbench/agent/shared/plugin.py:47

• The PR description documents this new keyword as allow_root_manifest, but the actual API exposes allow_copilot_manifest. Any caller following the documented name gets an unexpected-keyword TypeError; rename the parameter as described or update the PR documentation to match.

def resolve_config_plugins(agent_config: dict, *, allow_copilot_manifest: bool = False) -> dict[str, Path]:

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/bcbench/agent/copilot/agent.py:92

• resolve_config_plugins prefilters on raw YAML truthiness, so enabled: "false" is admitted and then parsed as plugin.enabled == False. Because this permission check ignores the parsed enabled field, an entry that is semantically disabled but has grant_dir_access: true still receives read/write access. Gate on both validated fields.

        cmd_args.extend(f"--add-dir={plugin.plugin_dir}" for plugin in plugins if plugin.grant_dir_access)

src/bcbench/agent/claude/agent.py:87

• resolve_config_plugins prefilters on raw YAML truthiness, so enabled: "false" is admitted and then parsed as plugin.enabled == False. This condition checks only the grant flag, allowing a semantically disabled entry to add its directory with read/write access. Gate on both validated fields.

        cmd_args.extend(f"--add-dir={plugin.plugin_dir}" for plugin in plugins if plugin.grant_dir_access)

src/bcbench/types.py:132

• This model is shared by both agents, and run_claude_code also translates this field into Claude's --add-dir (line 87). Describing the capability as Copilot-specific is therefore inaccurate and obscures the intentionally shared permission behavior.

    # Grant the agent filesystem access to this plugin's tree via Copilot `--add-dir`.

[Sun Haoran (haoranpb)]

Thanks for working through those iterations, I think we have found the right design here

[Sun Haoran (haoranpb)]

Unfortunately, this change here will introduce an unexpected side effect => we'll be able to specify plugin_dir on the config.yaml.

[Wenjie Fan (gggdttt)]

Good catch, fixed in 04c047a. I dropped plugin_dir from PluginConfig entirely, so the config schema stays pure and config.yaml can no longer set it. resolve_config_plugins now returns list[tuple[PluginConfig, Path]], pairing each enabled plugin with the folder it resolved to.

[Sun Haoran (haoranpb)]

Maybe the best way to achieve this is, not putting plugin_dir on PluginConfig class after all:

def resolve_config_plugins(...) -> list[tuple[PluginConfig, Path]]:
    return [
        (plugin, _resolve_plugin(plugin, allow_copilot_manifest))
        for plugin in plugins
    ]

---

And then it could be used like:

cmd_args.extend(f"--plugin-dir={plugin_dir}" for _, plugin_dir in plugins)
cmd_args.extend(
    f"--add-dir={plugin_dir}"
    for plugin, plugin_dir in plugins
    if plugin.grant_dir_access
)

[Wenjie Fan (gggdttt)]

Done in 04c047a, exactly as suggested - resolve_config_plugins returns list[tuple[PluginConfig, Path]], and both agents consume it as for _, plugin_dir in plugins / for plugin, plugin_dir in plugins if plugin.grant_dir_access. Bonus: the resolved Path is now non-optional, so the earlier Path | None is gone and the tests dropped their None-narrowing.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/bcbench/agent/shared/plugin.py:75

• This duplicate check is case-sensitive, but GitHub plugins clone to <plugin_root>/<name>. On Windows, names such as BCQuality and bcquality refer to the same directory, so both entries pass this guard and the second clone replaces the first. Normalize/compare the GitHub clone destinations using filesystem case semantics (or reject case-folded duplicate names) before cloning.

    names = [plugin.name for plugin in plugins]
    duplicates = sorted({name for name in names if names.count(name) > 1})
    if duplicates:
        raise AgentError(f"Duplicate plugin name(s) among enabled plugins: {', '.join(duplicates)}")

src/bcbench/agent/shared/plugin.py:77

• The earlier filter uses raw YAML truthiness, so enabled: "false" passes it even though PluginConfig parses that value as False. This returned entry is then loaded by both agents and can even receive --add-dir when grant_dir_access is true. Coerce/validate enabled before deciding whether to resolve the full entry, while retaining the requirement that genuinely disabled entries remain otherwise inert.

    return [(plugin, _resolve_plugin(plugin, allow_copilot_manifest)) for plugin in plugins]

src/bcbench/agent/copilot/agent.py:92

• The PR description says TestPluginsNeedingDirAccess verifies this permission boundary, but that class does not exist and the test suite contains no --add-dir assertion. Add the described command-construction tests so default, disabled, and opted-in plugins are verified before granting read/write access.

        cmd_args.extend(f"--add-dir={plugin_dir}" for plugin, plugin_dir in plugins if plugin.grant_dir_access)