Skip to content

Decision-policy responsibility for foreman serve (extracted from #14) - #25

Open
anchapin wants to merge 1 commit into
thruwire:mainfrom
anchapin:decision-policy-responsibility
Open

anchapin wants to merge 1 commit into
thruwire:mainfrom
anchapin:decision-policy-responsibility

Conversation

@anchapin

Copy link
Copy Markdown
Contributor

Follow-up to #14, per your direction on that thread. Drops the stdio ask_foreman MCP tool entirely (no mcp package, docs, CLI command, or tests remain) and keeps the reusable core as a serve-side capability aligned with the responsibility architecture: bounded decision models, the tighten-only abstention policy, and the audit trail.

What this adds

  • supervision.decision-policy responsibility (src/foreman/responsibilities/decision.py + packaged supervision.decision-policy.toml): contributes the decision-required check (floor 0.70, always armed in the packaged TOML), registered in builtin_registry() alongside the core responsibilities.
  • Decision models (src/foreman/models/decision.py): AbstainCategory (destructive / irreversible / external / credentials), DecisionRequest, Decision.
  • Pure policy (src/foreman/decision_policy.py): server baseline merged 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.
  • ForemanModel.decide() (base.py protocol, Jev Noul implementation, FakeForemanModel deterministic stub) — now called by the responsibility/serve path instead of the MCP tool.
  • Audit trail: FOREMAN_DECIDED events (models/events.py) appended to .foreman/runs/<session-id>/events.jsonl (persistence.py) with question, classification, choice, confidence, and effective policy; foreman inspect renders them.
  • Config: FOREMAN_DECISION_THRESHOLD (default 0.80) and FOREMAN_DECISION_ALWAYS_ABSTAIN via the existing FactoryConfig env conventions.
  • Lifecycle seam: a single async resolve_decision() — model verdict → tighten-only policy gate → audit → ESCALATE directive on abstain. Abstention routes to the human; it never terminates the worker. directives() stays a synchronous no-op so no hook point is hard-coded.
  • Tests: tests/test_decision_policy.py carried over; new tests/test_decision_responsibility.py covering route/checks/directives and the abstain-never-terminates invariant. 191 passed; Ruff clean.

Open questions

  1. Which lifecycle event should invoke resolve_decision() — BEFORE_TOOL (gate a risky tool call) or a dedicated decision event?
  2. Should this responsibility stay always armed, or be routed?
  3. Is ESCALATE the right ask-human representation, or should serve gain a distinct non-terminal human-routing directive?
  4. Keep the abstention taxonomy (destructive / irreversible / external / credentials)?
  5. Should the baseline policy move from env vars into the central serve/responsibility TOML config?

Follow-up to #14 and #13.

… no MCP)

Ports the decision-policy core from PR thruwire#14 onto the responsibility
architecture: tighten-only abstention policy, ForemanModel.decide(),
FOREMAN_DECIDED audit events, and a first-class DecisionPolicyResponsibility
registered alongside the builtins with a packaged TOML definition.

Drops all MCP stdio server code: no foreman mcp, no mcp package, no docs/mcp.md.

The decision path is a single async seam, resolve_decision(), kept
lifecycle-agnostic (no hard-coded BEFORE_TOOL); the hook point remains an
open maintainer question. Abstention routes to the human via ESCALATE and
never terminates the worker.
@JARosen

JARosen commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

I noticed two lifecycle gaps that make this look premature to merge even though the tests and Ruff pass:

  1. The production serve/hook path never calls decision_armed() or resolve_decision(), and directives() is always a no-op. That means the README's automatic decision gating and FOREMAN_DECIDED audit trail are currently unreachable, while the always-enabled decision-required check still runs during assessments.
  2. Abstention returns an ESCALATE directive, but the existing hook runtime treats ESCALATE as terminal for after-tool and worker-stopping events. That conflicts with this PR's stated invariant that abstention routes to a human without terminating the worker; the current test only checks the enum name rather than exercising lifecycle behavior.

Could we first choose the lifecycle event/contract, wire this through the production serve path with an end-to-end test, and either add a distinct non-terminal human-routing outcome or constrain the integration so ESCALATE cannot terminate the session?

@anchapin

Copy link
Copy Markdown
Contributor Author

Thanks, both gaps are fair. Here's what I'd propose for the rework:

Hook point: BEFORE_TOOL. The abstain categories are classifications of actions about to happen, so gating before the tool runs is where they have teeth. It also unifies both halves of the responsibility through the one seam: if the pending tool call is an ask-user-style question, resolve it via model.decide() and attach the answer so the tool never runs; otherwise classify the call against the denylist and abstain → route to human. Either way resolve_decision() gets invoked from the production hook path and the audit trail becomes reachable. I'll add an end-to-end test covering both: a denylisted tool call abstains to human routing, and a surfaced question gets a model-resolved answer.

Abstention outcome: a distinct non-terminal human-routing outcome rather than reusing ESCALATE. Overloading ESCALATE risks changing semantics for existing consumers; a separate outcome keeps the abstain-never-terminates invariant explicit. Happy to name it to fit your conventions.

Arming: scope the decision-required check to the serve path so it doesn't run (pointlessly) during assessments.

If BEFORE_TOOL sounds like the right event, I'll implement along these lines. If you'd rather see question-surfacing as a dedicated event, say the word and I'll reshape around that instead.

@JARosen

JARosen commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Thanks for waiting on the architectural answer. I think these should be two separate seams:

  • PreToolUse may gate a concrete risky operation.
  • Questions that Foreman might answer need a dedicated decision or question event and response contract.

The existing hook contract cannot inject a model-selected answer back into the worker, and ESCALATE remains terminal. Before rebasing this implementation, Foreman needs a distinct non-terminal human-routing outcome plus an adapter response shape for resolved decisions. Until those contracts exist, the current responsibility and audit path remain unreachable from production. I would prefer to define those seams first, then reopen or rework this as a focused integration.

@anchapin

Copy link
Copy Markdown
Contributor Author

Thanks — the two-seam split makes sense. Happy to rework this as the focused integration once the non-terminal routing outcome and decision response contracts land. Would you like me to leave #25 open until then, or close it and reopen fresh against the new seams?

@anchapin

anchapin commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Opened #42 to propose the non-terminal human-routing outcome and decision response contract discussed above, so we can agree on the seams before this gets reworked.

@JARosen

JARosen commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

I think this feature is showing promise. A worker being able to ask its supervisor a question feels relevant to Foreman.

What would make it compelling is the supervisor bringing its existing responsibilities, configuration, and evidence to the answer. If it mostly proxies a caller-supplied question and options to Jev, with a separate threshold and risk policy, I’m less convinced that Foreman adds enough value.

Could this integrate more directly with existing capabilities? A few possible starting points:

  • Completion: “Can I finish now?” Evaluate the existing completion checks and return a bounded answer identifying any unmet criteria.
  • Verification: “Do I need independent verification before proceeding?” Use the configured verification responsibility and collected test evidence.
  • Repository instructions: “Is this proposed action consistent with the repository’s instructions?” Evaluate the relevant checks using the proposed operation and existing evidence providers.

Ideally, these requests would use the normal responsibility routing, check definitions, thresholds, and supervisory rules, with the decision recorded in the audit trail. That would also help clarify whether a separate decision policy is needed.

Would you be interested in proposing one focused, end-to-end workflow along those lines—showing how the worker submits the question, how existing Foreman capabilities determine the answer, and how the worker receives it? I think that would give us a stronger foundation for agreeing on the lifecycle and response contracts.

This branch has not been deployed

No deployments
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