fix(copilot-acp): stop substituted models impersonating the requested one
Follow-up to the session/set_model wiring, caught in live use: picking an org-policy-disabled model (claude-fable-5) produced a response claiming to BE that model while Copilot actually served its default (Claude Sonnet 5). Two causes: 1. The prompt preamble injected 'Hermes requested model hint: <id>', so whatever model actually served the session parroted the requested name back as its identity. Remove the line entirely — the model is applied for real via session/set_model now, and identity must come from the backend, not prompt suggestion. 2. session/new advertises policy-disabled ids alongside enabled ones (_meta.copilotEnablement: 'disabled'); selecting one is accepted but silently serves the default. Exclude disabled ids from the offered set so the degrade-with-warning path handles them. Verified live: requesting claude-fable-5 logs the does-not-offer warning listing the 23 genuinely enabled models, serves the default, and the response truthfully self-identifies as Claude Sonnet 5.
This commit is contained in:
committed by
kshitij
parent
426dab7de0
commit
a94b68ad40
@@ -199,8 +199,11 @@ def _format_messages_as_prompt(
|
||||
"IMPORTANT: If you take an action with a tool, you MUST output tool calls using <tool_call>{...}</tool_call> blocks with JSON exactly in OpenAI function-call shape.",
|
||||
"If no tool is needed, answer normally.",
|
||||
]
|
||||
if model:
|
||||
sections.append(f"Hermes requested model hint: {model}")
|
||||
# Deliberately no "requested model" line in the prompt: the model is
|
||||
# applied for real via ACP session/set_model, and when the backend can't
|
||||
# honor it (org-policy-disabled id) a prompt-text mention makes the
|
||||
# serving model FALSELY self-identify as the requested one. Identity
|
||||
# must come from the backend, not from prompt suggestion.
|
||||
|
||||
# Copilot has no tools of its own that would collide with Hermes', so it
|
||||
# forwards the whole toolset (no allowlist).
|
||||
@@ -585,12 +588,22 @@ class CopilotACPClient:
|
||||
# and never fail the whole turn over model selection.
|
||||
if requested_model and requested_model != "copilot-acp":
|
||||
try:
|
||||
available = {
|
||||
str(m.get("modelId") or "").strip()
|
||||
advertised = [
|
||||
m
|
||||
for m in (
|
||||
(session.get("models") or {}).get("availableModels") or []
|
||||
)
|
||||
if isinstance(m, dict)
|
||||
]
|
||||
available = {
|
||||
str(m.get("modelId") or "").strip()
|
||||
for m in advertised
|
||||
# Org-policy-disabled ids can still appear in the list;
|
||||
# selecting one silently serves the default model, so
|
||||
# treat them as not offered.
|
||||
if str(
|
||||
((m.get("_meta") or {}).get("copilotEnablement")) or ""
|
||||
).strip().lower() != "disabled"
|
||||
}
|
||||
if not available or requested_model in available:
|
||||
_request(
|
||||
|
||||
@@ -200,7 +200,10 @@ def test_copilot_prompt_still_carries_the_contract_and_the_tools():
|
||||
assert "<tool_call>{...}</tool_call>" in prompt
|
||||
assert '"name": "memory"' in prompt
|
||||
assert '"name": "read_file"' in prompt # copilot forwards everything
|
||||
assert "Hermes requested model hint: gpt-5" in prompt
|
||||
# No prompt-text model mention: the model is applied via ACP
|
||||
# session/set_model, and a prompt hint makes a substituted backend
|
||||
# falsely self-identify as the requested model.
|
||||
assert "model hint" not in prompt
|
||||
assert "hi" in prompt
|
||||
|
||||
|
||||
|
||||
@@ -356,6 +356,9 @@ def _run_prompt_with_scripted_wire(model, session_result):
|
||||
str(m.get("modelId") or "").strip()
|
||||
for m in ((session.get("models") or {}).get("availableModels") or [])
|
||||
if isinstance(m, dict)
|
||||
and str(
|
||||
((m.get("_meta") or {}).get("copilotEnablement")) or ""
|
||||
).strip().lower() != "disabled"
|
||||
}
|
||||
if not available or requested_model in available:
|
||||
wire.request(
|
||||
@@ -405,6 +408,25 @@ def test_set_model_skipped_for_provider_virtual_slug():
|
||||
assert all(m != "session/set_model" for m, _ in reqs)
|
||||
|
||||
|
||||
def test_set_model_skipped_for_policy_disabled_model():
|
||||
# A policy-disabled id may still be advertised; selecting it silently
|
||||
# serves the default model, so it must not be treated as offered.
|
||||
session = {
|
||||
"sessionId": "s1",
|
||||
"models": {
|
||||
"availableModels": [
|
||||
{"modelId": "claude-sonnet-5"},
|
||||
{
|
||||
"modelId": "claude-fable-5",
|
||||
"_meta": {"copilotEnablement": "disabled"},
|
||||
},
|
||||
]
|
||||
},
|
||||
}
|
||||
reqs = _run_prompt_with_scripted_wire("claude-fable-5", session)
|
||||
assert all(m != "session/set_model" for m, _ in reqs)
|
||||
|
||||
|
||||
def test_run_prompt_receives_picker_model():
|
||||
# _create_chat_completion must forward `model` into _run_prompt — the
|
||||
# original wiring dropped it, reducing the selection to prompt text.
|
||||
|
||||
Reference in New Issue
Block a user