Skip to content

Foreman MCP decision tool (ask_foreman) - #14

Closed
anchapin wants to merge 4 commits into
thruwire:mainfrom
anchapin:mcp-decision-tool
Closed

anchapin wants to merge 4 commits into
thruwire:mainfrom
anchapin:mcp-decision-tool

Conversation

@anchapin

Copy link
Copy Markdown
Contributor

Implements #13.

What this adds

foreman mcp starts a stdio MCP server (minimal JSON-RPC 2.0, no new dependencies) exposing one tool, ask_foreman. Point your coding agent at it and the agent can delegate multiple-choice questions to the foreman instead of asking you — you keep driving the session with your own skills and agents.

  • Decision models (foreman/models/decision.py): AbstainCategory (destructive / irreversible / external / credentials), DecisionRequest, Decision, plus a new FOREMAN_DECIDED event type.
  • ForemanModel.decide(): the protocol gains a second method. JevForemanModel implements it with a dedicated Noul prompt — per-option scores, per-category risk scores, and an explicit should-abstain question — reusing the existing SDK patterns. FakeForemanModel gets a deterministic stub so tests stay offline.
  • foreman/decision_policy.py (pure, well tested): the effective policy merges the server baseline with the request tighten-only (max of thresholds, union of denylists — a caller can never loosen the floor). An answer is returned only if the model didn't abstain, confidence >= threshold, the choice is one of the options, and the classification doesn't intersect the denylist. Otherwise the question goes back to the human; abstaining never terminates anything (unlike ESCALATE, which is terminal in the supervision loop).
  • Config: FOREMAN_DECISION_THRESHOLD (default 0.80, mirroring the other thresholds) and FOREMAN_ALWAYS_ABSTAIN (comma-separated, default all four categories), via the existing FactoryConfig env conventions.
  • Auditability: every decision/abstention is appended to .foreman/runs/<session-id>/events.jsonl with the question, classification, choice, confidence, and effective policy; foreman inspect renders them.
  • Tests: tests/test_decision_policy.py (merge semantics, tighten-only, threshold gating, denylist abstain) and tests/test_mcp_server.py (in-process JSON-RPC: list/call, answered/abstain paths, invalid choice rejected, caller-cannot-weaken). Full suite: 116 passed; ruff check clean.
  • Docs: docs/mcp.md, linked from the README near the worker-backends section.

Open questions for the maintainer

  1. Denylist taxonomy — is destructive / irreversible / external / credentials the right set? Should any be split (e.g. network calls vs. external side effects) or added (e.g. exfiltration as its own category)?
  2. Config surface — env vars fit the current conventions, but would a repo-level config file be preferable for the baseline policy?
  3. Progress events — should foreman mcp also stream progress/status events, or stay strictly request/response?
  4. The --skill / --agent passthrough for foreman run is intentionally left out; happy to file it as a follow-up issue if this direction looks right.

Implements thruwire#13: a stdio MCP server exposing the foreman as an autonomous
decision-maker inside interactive coding sessions.

- New decision models: AbstainCategory (destructive/irreversible/external/
  credentials), DecisionRequest, Decision, plus a FOREMAN_DECIDED event type.
- ForemanModel protocol gains decide(); JevForemanModel implements it with a
  dedicated Noul prompt (option scores, per-category risk scores, explicit
  should-abstain); FakeForemanModel gets a deterministic stub for offline tests.
- New decision_policy module: effective policy merges the server baseline
  with the request tighten-only (max threshold, union denylist); apply_policy
  answers only confident, valid, denylist-clean verdicts, otherwise abstains.
- New foreman.mcp stdio JSON-RPC 2.0 server with an ask_foreman tool;
  every decision is logged to .foreman/runs/<session>/events.jsonl.
- foreman mcp CLI command; foreman inspect renders FOREMAN_DECIDED events.
- Config: FOREMAN_DECISION_THRESHOLD (default 0.80) and
  FOREMAN_ALWAYS_ABSTAIN (comma-separated, default all four categories).
- Tests: test_decision_policy.py, test_mcp_server.py; docs/mcp.md.
@anchapin
anchapin marked this pull request as ready for review September 20, 2026 18:17
- Switch MCPServer.serve_stdio from LSP Content-Length framing to standard
  MCP newline-delimited JSON (ndjson) transport.
- Route foreman mcp startup error messages to stderr to prevent stdout
  corruption in stdio mode.
- Add tests for serve_stdio ndjson request/response handling and parse errors.
- Fix non-deterministic directory ordering in test_persistence.
@anchapin anchapin changed the title Draft: foreman MCP decision tool (ask_foreman) Foreman MCP decision tool (ask_foreman) Sep 20, 2026
- Tune FactoryConfig defaults: lower decision_threshold from 0.80 to 0.70
  and scope always_abstain to destructive,credentials to improve autonomous
  answer rates for technical trade-offs and workflow coordination.
- Add foreman decisions CLI command to compute and display decision metrics
  (answer rate, average confidence, abstain breakdown) with Rich table and
  --json output, supporting single repo or multi-repo auto-discovery.
- Fix foreman inspect to support event-only runs (MCP sessions without state.json).
- Add RunStore.list_run_ids() and RunStore.load_decision_events().
- Enrich question formulation guidelines and AGENTS.md template in docs/mcp.md
  to discourage open-ended meta-questions and administrative gate bypass queries.
- Add test coverage in tests/test_cli.py and tests/test_persistence.py.
@JARosen

JARosen commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Thanks for putting this together, Alex. I pulled the branch down and confirmed that the implementation is healthy in isolation: the full test suite and Ruff both pass.

Since this was opened, Foreman has gained pluggable responsibilities and centralized responsibility routing. We are also defining the next phase around mandatory supervision of human-initiated Codex sessions: plugin hooks will automatically send lifecycle events to a central foreman serve process, rather than relying on the agent to choose to call Foreman.

That means the current local stdio ask_foreman tool solves a somewhat different problem, and the branch now conflicts with several of the files changed by the responsibility work. I do think the bounded decision models, abstention policy, audit trail, and tests may still be useful as a separate responsibility or server capability.

For now, this is blocked on that server contract rather than being left open without direction. Once the foreman serve boundary is defined, would you be interested in rebasing and extracting the reusable decision-policy portion into a smaller follow-up PR? Thanks again for the thoughtful contribution.

@anchapin

Copy link
Copy Markdown
Contributor Author

Yes, I can make a new PR with the decision-policy portion. Should I wait for the foreman serve boundary to land first?

@JARosen

JARosen commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

We’ve now settled on the foreman serve direction, and the architectural changes around responsibilities, routing, attached-worker hooks, and extensions are complete for now. You do not need to wait for another boundary to land.

Please feel free to rebase on current main and extract the reusable decision-policy portion into a focused follow-up PR. The main thing is to align it with the responsibility architecture rather than restoring a separate global decision path. Thanks for checking.

@JARosen

JARosen commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Closing this in favor of #25. The reusable decision-policy ideas moved there, while the standalone stdio MCP path no longer matches Foreman serve and responsibility architecture. Thank you for the original work and for extracting the reusable portion.

@JARosen JARosen closed this Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants