Skip to content

Commit 672c1b0

Browse files
committed
fix(action): honor an explicit llm_protocol instead of forcing openai or anthropic
The Configure OCR step derived llm.protocol from llm_use_anthropic alone, so it always wrote openai or anthropic. Anyone on the Responses API was sent to /v1/chat/completions no matter what OCR_LLM_PROTOCOL they set. Add an optional llm_protocol input (anthropic, openai, openai-responses). When it is set, or when OCR_LLM_PROTOCOL is inherited from the job environment, that value is written and llm.use_anthropic is mirrored from it, the same way `ocr config set llm.protocol` mirrors the boolean. An unknown value fails the step with the accepted list. The explicit protocol joins the checkpoint fingerprint, appended only when set so existing checkpoints stay valid. Fixes #1134
1 parent 04284b5 commit 672c1b0

3 files changed

Lines changed: 167 additions & 2 deletions

File tree

‎action.yml‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,18 @@ inputs:
2222
Selects the LLM protocol (mapped to env OCR_USE_ANTHROPIC). An explicitly supplied empty
2323
string, true, 1, or yes selects Anthropic
2424
case-insensitively; every other value selects the OpenAI-compatible
25-
protocol, preserving the CLI environment contract.
25+
protocol, preserving the CLI environment contract. Ignored when
26+
llm_protocol names the protocol explicitly.
2627
required: true
28+
llm_protocol:
29+
description: >-
30+
Explicit LLM protocol written to `llm.protocol`: anthropic, openai or
31+
openai-responses (case-insensitive). When set it wins over
32+
llm_use_anthropic, which is mirrored from it the way `ocr config set
33+
llm.protocol` mirrors the boolean. Leave empty to keep deriving the
34+
protocol from llm_use_anthropic; an OCR_LLM_PROTOCOL variable in the
35+
job or step environment is honoured the same way when this is empty.
36+
required: false
2737
llm_auth_header:
2838
description: Custom auth header name (mapped to env OCR_LLM_AUTH_HEADER).
2939
required: false
@@ -504,11 +514,16 @@ runs:
504514
OCR_LLM_URL: ${{ inputs.llm_url }}
505515
OCR_LLM_MODEL: ${{ inputs.llm_model }}
506516
OCR_USE_ANTHROPIC: ${{ inputs.llm_use_anthropic }}
517+
OCR_LLM_PROTOCOL_INPUT: ${{ inputs.llm_protocol }}
507518
OCR_LLM_AUTH_HEADER: ${{ inputs.llm_auth_header }}
508519
OCR_EXTRA_BODY: ${{ inputs.llm_extra_body }}
509520
OCR_LANGUAGE: ${{ inputs.language }}
510521
shell: bash
511522
run: |
523+
# An explicit protocol wins over llm_use_anthropic: the input first,
524+
# then OCR_LLM_PROTOCOL inherited from the job or step environment.
525+
# Read it before the derivation below reuses that variable name.
526+
EXPLICIT_PROTOCOL="$(printf '%s' "${OCR_LLM_PROTOCOL_INPUT:-${OCR_LLM_PROTOCOL:-}}" | tr '[:upper:]' '[:lower:]')"
512527
NORMALIZED_USE_ANTHROPIC="$(printf '%s' "$OCR_USE_ANTHROPIC" | tr '[:upper:]' '[:lower:]')"
513528
case "$NORMALIZED_USE_ANTHROPIC" in
514529
""|true|1|yes)
@@ -520,6 +535,28 @@ runs:
520535
OCR_LLM_PROTOCOL="openai"
521536
;;
522537
esac
538+
# use_anthropic is mirrored from the explicit protocol the same way
539+
# `ocr config set llm.protocol` mirrors it, so the two never disagree.
540+
case "$EXPLICIT_PROTOCOL" in
541+
"")
542+
;;
543+
anthropic)
544+
OCR_USE_ANTHROPIC="true"
545+
OCR_LLM_PROTOCOL="anthropic"
546+
;;
547+
openai|openai-responses)
548+
OCR_USE_ANTHROPIC="false"
549+
OCR_LLM_PROTOCOL="$EXPLICIT_PROTOCOL"
550+
;;
551+
*)
552+
echo "::error::llm_protocol must be anthropic, openai or openai-responses (got '${EXPLICIT_PROTOCOL}')"
553+
exit 1
554+
;;
555+
esac
556+
# Exported for the checkpoint fingerprint: the normalized explicit
557+
# protocol, empty when neither the input nor the inherited variable
558+
# was set, so checkpoints taken before the input existed stay valid.
559+
echo "OCR_LLM_PROTOCOL_EXPLICIT=${EXPLICIT_PROTOCOL}" >> "$GITHUB_ENV"
523560
# reasoning_effort is OpenAI-compatible vocabulary; the Anthropic API
524561
# rejects unknown body fields, so fail fast instead of breaking every
525562
# request. Anthropic thinking control goes through an explicit
@@ -605,6 +642,7 @@ runs:
605642
OCR_FP_LLM_URL: ${{ inputs.llm_url }}
606643
OCR_FP_LLM_MODEL: ${{ inputs.llm_model }}
607644
OCR_FP_LLM_USE_ANTHROPIC: ${{ inputs.llm_use_anthropic }}
645+
OCR_FP_LLM_PROTOCOL: ${{ env.OCR_LLM_PROTOCOL_EXPLICIT }}
608646
OCR_FP_LANGUAGE: ${{ inputs.language }}
609647
OCR_FP_LLM_EXTRA_BODY: ${{ inputs.llm_extra_body }}
610648
# Normalized by Validate inputs, so spellings that mean the same thing
@@ -747,6 +785,12 @@ runs:
747785
process.env.OCR_FP_LLM_URL,
748786
process.env.OCR_FP_LLM_MODEL,
749787
process.env.OCR_FP_LLM_USE_ANTHROPIC,
788+
// The explicit protocol as Configure OCR resolved it — the input
789+
// first, then OCR_LLM_PROTOCOL inherited from the job — so a
790+
// protocol switched through the environment alone still fails
791+
// closed to a full review. Appended only when set, so
792+
// checkpoints taken before the input existed stay valid.
793+
...(process.env.OCR_FP_LLM_PROTOCOL ? [process.env.OCR_FP_LLM_PROTOCOL] : []),
750794
process.env.OCR_FP_LANGUAGE,
751795
process.env.OCR_FP_LLM_EXTRA_BODY,
752796
process.env.OCR_FP_LLM_REASONING_EFFORT,

‎examples/github_actions/README.md‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ The action posts a summary issue comment plus inline review comments. Two inputs
266266
| `corrupt_checkpoint` | the summary carries no readable checkpoint marker (absent, malformed, or two of them) |
267267
| `schema_invalid` | the marker is for another PR, another marker version, or records a run that did not complete |
268268
| `base_changed` | the base ref or the merge-base moved, so the diff basis is no longer the one the checkpoint was taken against |
269-
| `config_changed` | the model, language, `llm_extra_body`, `llm_reasoning_effort`, `llm_extra_headers`, `llm_auth_header`, `llm_timeout`, `effort`, `max_tokens_budget`, `background`, routing inputs, the resolved OCR version, or the contents of `rule` / `.opencodereview/rule.json` changed — or `ocr version` printed nothing, so the version could not be established at all |
269+
| `config_changed` | the model, language, `llm_protocol` (or an inherited `OCR_LLM_PROTOCOL`), `llm_extra_body`, `llm_reasoning_effort`, `llm_extra_headers`, `llm_auth_header`, `llm_timeout`, `effort`, `max_tokens_budget`, `background`, routing inputs, the resolved OCR version, or the contents of `rule` / `.opencodereview/rule.json` changed — or `ocr version` printed nothing, so the version could not be established at all |
270270
| `not_ancestor` | the checkpoint commit is in this clone but is not on the new head's history (the branch was reset to an earlier commit) |
271271
| `unknown_object` | the checkpoint commit is not in this clone, so ancestry could not be checked — where a force-push usually lands, since the replaced commit is no longer fetched |
272272
| `rule_unreadable` | a rule file was given but could not be read, so no stored fingerprint can be trusted to mean "same rules" |
@@ -487,6 +487,20 @@ OCR supports both OpenAI and Anthropic API formats:
487487
- Self-hosted models (vLLM, Ollama, etc.)
488488
- **Anthropic APIs** (set variable `OCR_LLM_USE_ANTHROPIC=true`, i.e. `llm_use_anthropic: true`):
489489
- Anthropic Claude models
490+
- **OpenAI Responses API** (`llm_protocol: openai-responses`):
491+
- Reasoning models used with function tools, or endpoints that only serve `/v1/responses`
492+
493+
`llm_protocol` (`anthropic`, `openai` or `openai-responses`) takes precedence over `llm_use_anthropic` when set, and `llm.use_anthropic` is mirrored from it. An `OCR_LLM_PROTOCOL` variable in the job environment is honoured the same way when the input is empty.
494+
495+
```yaml
496+
- uses: alibaba/open-code-review@main
497+
with:
498+
llm_url: ${{ vars.OCR_LLM_URL }}
499+
llm_auth_token: ${{ secrets.OCR_LLM_TOKEN }}
500+
llm_model: ${{ vars.OCR_LLM_MODEL }}
501+
llm_use_anthropic: 'false'
502+
llm_protocol: openai-responses
503+
```
490504

491505
## Troubleshooting
492506

‎scripts/github-actions/action-contract.test.js‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,6 +1124,112 @@ function testConfigureProtocolTracksUseAnthropic() {
11241124
}
11251125
}
11261126

1127+
function testConfigureHonoursExplicitProtocol() {
1128+
const configure = stepNamed("Configure OCR");
1129+
assert.ok(configure, "action.yml must retain the Configure OCR step");
1130+
const base = {
1131+
llm_url: "https://llm.example.invalid/v1",
1132+
llm_model: "contract-model",
1133+
llm_auth_token: "protocol-token-sentinel",
1134+
};
1135+
// The explicit protocol wins, and use_anthropic follows it — the boolean
1136+
// alone is what forced every Responses API user onto chat/completions.
1137+
const cases = [
1138+
{ input: "openai-responses", useAnthropic: "false", expectAnthropic: "false", expectProtocol: "openai-responses" },
1139+
{ input: "openai-responses", useAnthropic: "true", expectAnthropic: "false", expectProtocol: "openai-responses" },
1140+
{ input: "Anthropic", useAnthropic: "false", expectAnthropic: "true", expectProtocol: "anthropic" },
1141+
{ input: "openai", useAnthropic: "true", expectAnthropic: "false", expectProtocol: "openai" },
1142+
];
1143+
for (const testCase of cases) {
1144+
const fixture = makeFixture();
1145+
try {
1146+
const values = inputValues(Object.assign({}, base, {
1147+
llm_use_anthropic: testCase.useAnthropic,
1148+
llm_protocol: testCase.input,
1149+
}));
1150+
const result = runStep(configure, values, fixture);
1151+
assert.strictEqual(
1152+
result.status,
1153+
0,
1154+
`Configure OCR failed for llm_protocol=${testCase.input}; ${resultDescription(result)}`
1155+
);
1156+
const configured = configValues(configOperations(fixture));
1157+
assert.strictEqual(configured["llm.protocol"], testCase.expectProtocol);
1158+
assert.strictEqual(configured["llm.use_anthropic"], testCase.expectAnthropic);
1159+
assert.strictEqual(
1160+
readEnvAssignments(path.join(fixture.dir, "github-env")).OCR_LLM_PROTOCOL_EXPLICIT,
1161+
testCase.expectProtocol,
1162+
"the normalized explicit protocol must be exported for the checkpoint fingerprint"
1163+
);
1164+
} finally {
1165+
removeFixture(fixture);
1166+
}
1167+
}
1168+
1169+
// OCR_LLM_PROTOCOL from the job environment is honoured when the input is
1170+
// empty: it is the variable the CLI itself reads, and the one the report
1171+
// set without effect.
1172+
const inherited = makeFixture();
1173+
try {
1174+
const values = inputValues(Object.assign({}, base, { llm_use_anthropic: "false" }));
1175+
const result = runStep(configure, values, inherited, { OCR_LLM_PROTOCOL: "openai-responses" });
1176+
assert.strictEqual(result.status, 0, `Configure OCR failed with inherited OCR_LLM_PROTOCOL; ${resultDescription(result)}`);
1177+
const configured = configValues(configOperations(inherited));
1178+
assert.strictEqual(configured["llm.protocol"], "openai-responses");
1179+
assert.strictEqual(configured["llm.use_anthropic"], "false");
1180+
assert.strictEqual(
1181+
readEnvAssignments(path.join(inherited.dir, "github-env")).OCR_LLM_PROTOCOL_EXPLICIT,
1182+
"openai-responses",
1183+
"a protocol switched through the environment alone must reach the checkpoint fingerprint"
1184+
);
1185+
} finally {
1186+
removeFixture(inherited);
1187+
}
1188+
1189+
// With neither source set the export is empty, so the fingerprint stays
1190+
// exactly what it was before the input existed.
1191+
const neither = makeFixture();
1192+
try {
1193+
const values = inputValues(Object.assign({}, base, { llm_use_anthropic: "false" }));
1194+
const result = runStep(configure, values, neither);
1195+
assert.strictEqual(result.status, 0, `Configure OCR failed without a protocol source; ${resultDescription(result)}`);
1196+
assert.strictEqual(readEnvAssignments(path.join(neither.dir, "github-env")).OCR_LLM_PROTOCOL_EXPLICIT, "");
1197+
} finally {
1198+
removeFixture(neither);
1199+
}
1200+
1201+
// The resolve step fingerprints that export, not the raw input.
1202+
const resolve = STEPS.find((step) => step.name === "Resolve review range");
1203+
assert.ok(resolve, "action.yml must retain the Resolve review range step");
1204+
assert.strictEqual(
1205+
resolve.env.OCR_FP_LLM_PROTOCOL,
1206+
"${{ env.OCR_LLM_PROTOCOL_EXPLICIT }}",
1207+
"the checkpoint fingerprint must read the protocol Configure OCR resolved"
1208+
);
1209+
1210+
// The input outranks the inherited variable.
1211+
const both = makeFixture();
1212+
try {
1213+
const values = inputValues(Object.assign({}, base, { llm_use_anthropic: "false", llm_protocol: "anthropic" }));
1214+
const result = runStep(configure, values, both, { OCR_LLM_PROTOCOL: "openai-responses" });
1215+
assert.strictEqual(result.status, 0, `Configure OCR failed with both protocol sources; ${resultDescription(result)}`);
1216+
assert.strictEqual(configValues(configOperations(both))["llm.protocol"], "anthropic");
1217+
} finally {
1218+
removeFixture(both);
1219+
}
1220+
1221+
// An unknown protocol fails the step rather than silently configuring one.
1222+
const unknown = makeFixture();
1223+
try {
1224+
const values = inputValues(Object.assign({}, base, { llm_use_anthropic: "false", llm_protocol: "grpc" }));
1225+
const result = runStep(configure, values, unknown);
1226+
assert.notStrictEqual(result.status, 0, "an unknown llm_protocol must fail the Configure OCR step");
1227+
assert.match(`${result.stdout}${result.stderr}`, /llm_protocol must be/, "the failure must name the accepted protocols");
1228+
} finally {
1229+
removeFixture(unknown);
1230+
}
1231+
}
1232+
11271233
function testConfigurePreservesLegacyUseAnthropicResolution() {
11281234
const configure = stepNamed("Configure OCR");
11291235
assert.ok(configure, "action.yml must retain the Configure OCR step");
@@ -1615,6 +1721,7 @@ const TESTS = [
16151721
["Configure OCR never persists the token", testConfigureNeverPersistsToken],
16161722
["Configure OCR neutralizes stale provider and static token", testConfigureNeutralizesStaleProviderAndStaticToken],
16171723
["Configure OCR sets a protocol consistent with use_anthropic", testConfigureProtocolTracksUseAnthropic],
1724+
["Configure OCR honours an explicit llm_protocol", testConfigureHonoursExplicitProtocol],
16181725
["Configure OCR preserves legacy use_anthropic resolution", testConfigurePreservesLegacyUseAnthropicResolution],
16191726
["Configure OCR clears stale persisted extra headers", testConfigureClearsStaleExtraHeadersBeforeTokenCommand],
16201727
["Configure OCR clears stale persisted retry codes", testConfigureClearsStaleRetryCodesBeforeEndpointConfig],

0 commit comments

Comments
 (0)