refactor(mcp): collapse role ladder into independent capability config#3202
Merged
Conversation
…docs
Problem: the WIP rescue commit landed mid-refactor -- devtools render_agent_manual.py
still imported the retired ROLE_ORDER/ROLES from polylogue.agent_integration.spec,
several devtools/test call sites still passed positional "admin"/"read"/"write" role
strings into declared_tool_names()/build_server()/target_tool_names(), and
docs (mcp-reference.md, mcp-integration.md, architecture-spine.md,
daemon-threat-model.md) still described the retired --role ladder.
What changed:
- devtools/render_agent_manual.py, render_mcp_equivalence.py, render_mcp_tool_index.py,
verify_agent_integration.py, continuity_replay.py: updated to the MCPCapabilities
model (independent write/judge/maintenance booleans, no role ladder).
- polylogue/agent_integration/{spec,manifest}.py, cli/commands/agent.py: role/ROLES/
ROLE_ORDER replaced by required_capability/MCPCapabilities; `polylogue agent
manifest`/`install` gained --enable-write/--enable-judge/--enable-maintenance
flags instead of --role.
- Regenerated docs/generated/mcp-equivalence.json, docs/agent-manual.md,
docs/agent-integration-reference.md, and the packaged
polylogue/agent_integration/data/*.{md,json} assets via `devtools render agent-manual`
+ `devtools render mcp-equivalence` + `devtools render mcp-tool-index`.
- docs/mcp-reference.md, docs/mcp-integration.md, docs/architecture-spine.md,
docs/daemon-threat-model.md, docs/architecture-hotspots.md: replaced --role/role-ladder
prose with the config-opt-in description ([mcp] write_enabled/judge_enabled/
maintenance_enabled, POLYLOGUE_MCP_*_ENABLED).
- Remaining test call sites (tests/infra/surfaces.py, tests/unit/{storage,maintenance,
cost,archive/query,agent_integration}/..., tests/integration/test_mcp.py) updated to
build_server(capabilities=...)/target_tool_names()/declared_tool_names(...).
- devtools/render_mcp_equivalence.py: annotated capability_counts/capability_order as
plain str to satisfy mypy --strict against the new Literal-keyed Counter.
Verification:
- devtools render all --check -> sync OK, no drift.
- devtools verify --quick -> ruff format/check, mypy --strict, render all, topology,
layering, closure-matrix, manifests, ci-workflows, doc-commands, docs-coverage,
test-infra-currency, test-clock-hygiene, pytest-timeout-overrides, degrade-loudly
all exit 0.
- devtools test tests/unit/mcp tests/unit/agent_integration -> 214 passed, 1
pre-existing unrelated failure (test_manual_contract.py::...decodes_to_the_bound_result
asserts a stale q1. token prefix against the current q2 codec; fails identically on
unmodified master, confirmed via git show).
- devtools test tests/unit/maintenance tests/unit/storage/{test_query_parity,
test_retrieval_readiness_laws,test_surface_parity_adapters} tests/unit/cost/
test_contract_suite.py tests/unit/archive/query/test_continuation_surface_parity.py
tests/unit/core/test_config*.py -> 344 passed, 3 pre-existing unrelated failures
(KeyError 'search'/'list_sessions'/'list_tags' -- these tests still call retired
individual MCP tool names that predate the six-tool cutover; unrelated to
role-to-capability collapse).
- devtools test tests/integration/test_mcp.py -> 4 pre-existing unrelated failures,
same retired-tool-name cause.
Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (59)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cf4426919
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ``POLYLOGUE_MCP_WRITE_ENABLED=1`` to register the mutation | ||
| dispatchers. | ||
| """ | ||
| return bool(self._data.get("mcp_write_enabled")) |
There was a problem hiding this comment.
When POLYLOGUE_MCP_WRITE_ENABLED contains a typo such as flase (or TOML supplies a quoted/non-boolean value), _coerce_env_value preserves the nonempty string and this bool(...) converts it to True, registering the write/run mutation dispatchers despite the value not being an accepted opt-in. The same fail-open behavior affects judge and maintenance; these security-boundary properties should accept only an actual True or reject invalid configuration.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 52f4443: _coerce_env_value's boolean branch now raises ConfigError naming the env var/key/offending value for any _BOOL_CONFIG_KEYS entry that isn't a canonical 1/true/yes/on / 0/false/no/off token (case-insensitive), instead of passing the raw string through — this fixes the shared coercion path, not just the three new MCP keys. Also added _require_bool_config_value() (shared by mcp_write_enabled/mcp_judge_enabled/mcp_maintenance_enabled) to close the same hole for a quoted TOML value, which bypasses env coercion entirely. New tests in tests/unit/core/test_config_inventory.py::TestMCPCapabilityBooleanFailsClosed cover: typo raises, false/0/no/off (case-insensitive) disables, true/1/yes/on (case-insensitive) enables, TOML true works, and quoted TOML string raises.
Problem: CodeRabbit flagged (PR #3202) that POLYLOGUE_MCP_WRITE_ENABLED=flase (or any nonempty non-boolean string, or a quoted TOML value) passed through _coerce_env_value's bool branch unchanged as a raw string, and the mcp_write_enabled/mcp_judge_enabled/mcp_maintenance_enabled properties did bool(self._data.get(...)) -- bool("flase") is True, so a typo silently opened a capability boundary instead of leaving it closed. Same shape for every other _BOOL_CONFIG_KEYS entry (not special-cased to the new keys). What changed: - _coerce_env_value's boolean branch now raises ConfigError naming the env var, offending value, and config key when the string isn't one of the canonical 1/true/yes/on / 0/false/no/off tokens (case-insensitive), instead of silently passing the raw string through. This applies to every _BOOL_CONFIG_KEYS entry via the one shared coercion path, not a capability-only special case. - New _require_bool_config_value() (shared by all three capability properties) additionally closes the TOML-layer version of the same gap: a quoted TOML value (`write_enabled = "flase"`) bypasses env coercion entirely, so the property getter itself now validates the resolved value is a genuine bool or a canonical token string, raising ConfigError otherwise. Token-parsing logic is shared via _parse_bool_token() between the env-coercion path and this property-level guard. Verification: - devtools test tests/unit/core/test_config_inventory.py tests/unit/core/test_config.py -> 155 passed, including the new TestMCPCapabilityBooleanFailsClosed (typo raises, false/0/no/off/case-insensitive disables, true/1/yes/on/ case-insensitive enables, TOML true works, quoted TOML string raises). - devtools test tests/unit/mcp/test_cli.py tests/unit/cli/test_config_command.py -> 15 passed. - devtools verify --quick -> ruff format/check, mypy --strict, render all, topology, layering, docs-coverage, etc. all exit 0. - devtools test tests/unit/core -> 2327 passed; 4 failed / 6 errored, all in test_sync_surface_runtime.py/test_verification.py/test_models.py/ test_schema_generation.py over artifact-taxonomy/schema-generation naming (agent_transcript vs subagent_session_stream) -- no reference to config.py, load_polylogue_config, or boolean coercion in any of them; pre-existing drift from the concurrent master rebase (#3196-#3200 landed mid-session), unrelated to this fix. Co-Authored-By: Claude <noreply@anthropic.com>
Sinity
added a commit
that referenced
this pull request
Jul 20, 2026
Sinity
added a commit
that referenced
this pull request
Jul 20, 2026
## Summary Adds the missing automated actor for the MCP `judge` dispatcher's candidate queue: a policy engine plus a daemon periodic-loop trigger surface that decides which `CANDIDATE`-status assertions are auto-judgeable and calls the same storage chokepoint the MCP `judge` dispatcher uses, escalating the residue to an explicit, queryable `handoff` assertion. ## Problem Per the polylogue-800m design note (PR #3202), the MCP `judge` dispatcher lets an operator judge assertion candidates in bulk, but nothing calls it automatically. Every agent-authored candidate (pathology findings, ontology/ transform candidates, findings, ...) defaults to waiting on an explicit human decision, which the operator has said does not scale — most agent-authored annotations will never be seen by a human. ## Solution - **`polylogue/daemon/judgment_automation.py`** (new): a policy engine (`evaluate_candidate`) applies per-kind confidence thresholds (`auto_accept_min_confidence`/`auto_reject_max_confidence`) decoded from a new `[judgment_automation.policies]` config table (`parse_judgment_automation_policy`). A candidate is judged (`run_judgment_automation_sweep_once`) via the real `judge_assertion_candidates` storage chokepoint — the same one the MCP `judge` dispatcher calls — when the policy is unambiguous; every other candidate (no policy for its kind, no `confidence` value, or a confidence inside the undecided band) is escalated: a deterministic `handoff`-kind assertion pointing at the candidate is upserted so the residue is a queryable review queue, not silent limbo. - **`polylogue/daemon/cli.py`**: wires `periodic_judgment_automation_sweep` into the existing `periodic_loops` list next to `_periodic_db_optimize`/ `periodic_embedding_backlog_check`, following the established periodic- maintenance-loop pattern rather than a `DaemonConverger` stage — judgment candidates are not file/session-scoped, and `docs/retro/2026-05-24-1498-cascade.md` flags `convergence_stages.py` as due for a refactor before a 4th stage, so this keeps the bead's footprint out of that hot file entirely. - **`polylogue/config.py`**: four new layered config keys — `judgment_automation_enabled` (fail-closed bool, off by default, following the `mcp_judge_enabled` pattern), `judgment_automation_interval_s`, `judgment_automation_batch_limit`, and the raw `judgment_automation_policy` table (decoded by the owning module, same shape as `health_convergence_debt`). - **Dual capability gate**: the sweep only judges when *both* `judgment_automation_enabled` AND `mcp_judge_enabled` (polylogue-800m's independent capability boundary) are true — it exercises the same judge write authority as the MCP dispatcher, so its own opt-in must not, by itself, grant that authority. Both flags are re-read every tick (`reload_behavior="daemon-loop"`), so flipping either off in `polylogue.toml` takes effect on the next tick without a daemon restart. - Docs: `docs/configuration.md` (config key table) and `docs/daemon.md` (new "Judgment Automation" section with a worked `polylogue.toml` example). ## Verification - `devtools test tests/unit/daemon/test_judgment_automation.py tests/unit/core/test_config_inventory.py tests/unit/daemon/test_daemon_cli.py` → `183 passed` - `mypy --strict` on `polylogue/config.py polylogue/daemon/cli.py polylogue/daemon/judgment_automation.py tests/unit/daemon/test_judgment_automation.py` → `Success: no issues found in 4 source files` - `devtools verify --quick` (ruff format/check, mypy, render-all --check, topology/layering/closure-matrix/manifests/ci-workflows/doc-commands/docs-coverage/test-infra-currency/test-clock-hygiene/pytest-timeout-overrides/degrade-loudly) → `exit_code: 0` - Not run: full `devtools verify` (testmon-affected) / `--seed-testmon` — per-repo policy this is the default pre-PR baseline, but it was explicitly deferred for this lane in favor of the focused set above plus `--quick`. ## Residual risk - No real-world confidence-producing detector is wired to a `[judgment_automation.policies]` entry yet in this PR — the policy table starts empty by default, so the sweep is a bounded no-op until an operator configures at least one kind. This is intentional (config-gated OFF, fail-closed on missing policy) but means the automation has no live exercise beyond the test fixtures until an operator opts a real detector kind in. - The escalation `handoff` assertion is discoverable only via an explicit `kind=handoff` filter today (not in `ASSERTION_CLAIM_KINDS`'s default set); wiring a dedicated MCP/CLI "review queue" surface for it is left as follow-up rather than expanding this PR's scope. Ref polylogue-6qjc --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
Summary
Collapses the MCP server's role ladder (
read < write < review < admin) intoindependent, config-only capability opt-ins per polylogue-800m: the server is
read-only by default; write (
write/run), judge (judge), and maintenance(
maintenance) are each an explicit, independent boolean opt-in resolvedonce at server startup from
polylogue.toml[mcp]orPOLYLOGUE_MCP_*_ENABLEDenv vars. There is no ladder — enabling onecapability never implies another — and no
--roleCLI flag anywhere in thestack.
Problem
The operator decided (2026-07-20, polylogue-800m) that the MCP role ladder
is not a product concept worth keeping:
--role read/write/review/adminmodeled an ordered tier where each level silently included the previous
one's authority, which is both more machinery than the actual requirement
(independent per-capability opt-in) and a worse security default (enabling
reviewto unlockjudgealso silently re-exposedwrite/run). Per therepo's no-compat doctrine this is a hard removal, not a deprecation.
Solution
polylogue/config.py: three new layered config keys following theexisting single-resolver (
PolylogueConfig) pattern —mcp_write_enabled/mcp_judge_enabled/mcp_maintenance_enabled(
[mcp]TOML table,POLYLOGUE_MCP_*_ENABLEDenv, all defaultFalse).polylogue/mcp/declarations/models.py: newMCPCapabilitiesfrozendataclass (three independent booleans +
.allows(required)) replacesMCPRole/MCP_ROLE_ORDER/mcp_role_allows. Every declaration'sminimum_role: MCPRolebecomesrequired_capability: MCPCapabilityFlag | None(None= always-on read tool).polylogue/mcp/declarations/{registry,adapter}.py,polylogue/mcp/{server,server_cutover,server_support,cli}.py: the10-dispatcher registration/gating (6 read +
write/runbehindwrite,judgebehindjudge,maintenancebehindmaintenance) now readsMCPCapabilitiesinstead of a role string.polylogue-mcp(
mcp/cli.py) drops--roleentirely and resolves capabilities fromload_polylogue_config()at startup — config is the sole authority, not alaunch argument.
polylogue/agent_integration/{spec,manifest,installer}.py,polylogue/cli/commands/agent.py: the parallel "role" vocabulary inthe native-client installer/manifest system (
ROLES/ROLE_ORDER,polylogue agent manifest/install --role) collapses the same way —polylogue agent manifest/installgained--enable-write/--enable-judge/--enable-maintenanceflags. Theinstaller now emits
POLYLOGUE_MCP_WRITE_ENABLED=1etc. into thegenerated MCP client entry's
envblock instead ofargs: ["--role", ...]— this is real wiring (the client launchespolylogue-mcpwith thatenv, which
load_polylogue_config()picks up), not just a renamedno-op.
InstallOptions' on-disk state schema bumpedSTATE_SCHEMA_VERSION1→2 (role→capabilities field shape) so stale state fails closed via the
existing integrity gate instead of silently exposing a missing field.
nix/agent-integration-module.nix:mcpRoleenum option replaced bythree independent
mcpEnableWrite/mcpEnableJudge/mcpEnableMaintenancebooleans (Home Manager module), each off by default.
docs/mcp-reference.md,docs/mcp-integration.md,docs/architecture-spine.md(decision log entry),docs/daemon-threat-model.md,docs/architecture-hotspots.md— role/--roleprose replaced with theconfig-opt-in description.
docs/agent-manual.md,docs/agent-integration-reference.md, and the packagedpolylogue/agent_integration/data/*.{md,json}assets regenerated viadevtools render agent-manual/render mcp-equivalence/render mcp-tool-index.tests/infra/mcp.py::EXPECTED_TOOL_NAMES(now derived from
declared_tool_names(ALL_CAPABILITIES)), theTOOL_CONTRACTenvelope tests,docs/generated/mcp-equivalence.json(
render mcp-equivalence), anddocs/mcp-reference.md's generatedtool-index appendix (
render mcp-tool-index) — all regenerated andgreen.
tests/unit/mcp/test_privileged_tools.py'sTestRoleGatingbecame
TestCapabilityGating, asserting the independent-flag behaviordirectly (e.g.
judge=Truealone must not also exposewrite/run—the old ladder-based test asserted the opposite). All other
build_server(role=...)/declared_tool_names("admin")-style call sitesacross
tests/unit/mcp/,tests/unit/agent_integration/,tests/unit/{maintenance,storage,cost,archive/query}/,tests/integration/test_mcp.py, anddevtools/verify_agent_integration.pyupdated to the capability model.
Design note (deferred, not built here): the second half of polylogue-800m
— assertion judgment at agent scale (policy/automation-driven, human review
reserved for escalations, replacing the per-assertion human-review pipeline
that doesn't scale) — is design-only per the task brief. Summary for the
coordinator to bead separately: the
judgeMCP dispatcher andjudge_assertion_candidatesAPI already support bulk, policy-shapeddecisions (accept/reject/defer/supersede with reason + provenance); what's
missing is an automation actor that calls
judgeon a schedule/triggerusing declared acceptance policy per
AssertionKind, with an explicitescalation path (a
defer+flag that routes specifically to human review)rather than every candidate defaulting to human attention. That's a new
policy-engine + trigger surface, not a schema or MCP-surface change, and is
out of scope for this PR.
Sequencing with #3190: #3190 (README rewrite, branch
docs/readme-rewrite,currently open) contains an MCP config example using
--role read. Thatbranch was intentionally not touched here per the task brief. Whichever of
#3190 / this PR merges second should rebase and drop the
--roleflag fromits README example — the flag no longer exists after this PR merges.
Verification
devtools render all --check→ sync OK, no drift (all generated MCP/agentdocs and JSON assets committed after regeneration).
devtools verify --quick→ ruff format/check,mypy --strict, render all,topology, layering, closure-matrix, manifests, ci-workflows, doc-commands,
docs-coverage, test-infra-currency, test-clock-hygiene,
pytest-timeout-overrides, degrade-loudly — all exit 0.
devtools test tests/unit/mcp tests/unit/agent_integration→ 214 passed, 1pre-existing unrelated failure
(
test_manual_contract.py::test_generated_continuation_token_decodes_to_the_bound_resultasserts a stale
q1.continuation-token prefix against the currentq2codec; reproduced identically on unmodified
master, unrelated to thischange).
devtools test tests/unit/maintenance tests/unit/storage/{test_query_parity,test_retrieval_readiness_laws,test_surface_parity_adapters} tests/unit/cost/test_contract_suite.py tests/unit/archive/query/test_continuation_surface_parity.py tests/integration/test_mcp.py→ pre-existing unrelated failures only (
KeyError: 'search'/'list_sessions'/'list_tags'— these tests call individual MCP tool names retired by the earlier
six-tool cutover, unrelated to the role→capability collapse; same
failures reproduce on unmodified
master).Ref polylogue-800m