Skip to content

fix: react on the default token, and let the recipe pick the judge model - #42

Merged
ZhenghuaBao merged 1 commit into
mainfrom
fix/reactions-and-dsl-judge
Aug 26, 2026
Merged

fix: react on the default token, and let the recipe pick the judge model#42
ZhenghuaBao merged 1 commit into
mainfrom
fix/reactions-and-dsl-judge

Conversation

@ZhenghuaBao

Copy link
Copy Markdown
Contributor

Two fixes, both measured on a real run rather than reasoned about.

Reactions never happened

The gate read gh api user --jq .type and declined unless it saw "Bot". That endpoint needs a user-scoped token, and github-token defaults to ${{ github.token }} — an installation token, for which it returns no user. The || echo "Bot" fallback was meant to cover that and does not: the call can exit 0 with no .type, so ME_TYPE comes back empty, != "Bot" holds, and BOTH the 👀 step and the settle step skip.

That is every consumer on the default token, which is the case the feature exists to serve. Evidence from a run: the log says github-token belongs to a user, not an app while the 👀 already on the PR was authored by github-actions[bot].

Inverted to fail towards acting — a PAT identifies itself as type User and declines; 403, empty and Bot all react. That also unsticks the 👀 left behind by runs predating the settle step, since the clear now runs.

The judge model comes from the recipe

judge-model defaulted to a concrete model, so the L2 call named it directly and never reached the router — the one routing decision in the product a workspace could not make for itself. Its default is now empty, which sends the router alias, and judge.mjs stamps x-cr-lens: judge so the recipe's judge rule chooses.

No model change. The shipped recipe routes that rule to deepseek-v4-pro, which is what judge-model used to name. Setting judge-model explicitly still pins a model outright regardless of the recipe.

The header goes out unconditionally, including when --model names a concrete model: nothing resolves an alias then, so nothing reads it, and "is this an alias" is the gateway's judgement rather than this script's.

Order

OrcaRouter-O2#1433 must be deployed first. It puts the judge rule in the provisioned recipe. Without it the judge lands on the recipe's default — the model it is scoring — which agrees with itself while still reporting success.

Tests

judge.test.mjs pins the header. Suite counts match main exactly (installer 2 / platforms 1 / report 1) with one fewer failure in settings; those are pre-existing and unrelated.

Two things, both measured on a real run rather than reasoned about.

REACTIONS NEVER HAPPENED. The gate read `gh api user --jq .type` and declined
unless it saw "Bot". That endpoint needs a user-scoped token, and github-token
defaults to ${{ github.token }} — an installation token, for which it returns no
user. The `|| echo "Bot"` fallback was meant to cover that and did not: the call
can exit 0 with no `.type`, so ME_TYPE came back empty, `!= "Bot"` held, and both
the 👀 and the settle step skipped — for every consumer on the default token,
which is the case the feature exists to serve. A run logged "github-token
belongs to a user, not an app" while the reaction already on the PR was authored
by github-actions[bot].

Inverted to fail towards acting: a PAT identifies itself as type User and
declines; 403, empty and Bot all react. That also unsticks the 👀 left behind by
runs that predate the settle step, since the clear now runs.

THE JUDGE MODEL COMES FROM THE RECIPE. judge-model defaulted to a concrete
model, so the L2 call named it directly and never reached the router — the one
routing decision in the product that a workspace could not make for itself. Its
default is now empty, which sends the router alias, and judge.mjs stamps
`x-cr-lens: judge` so the recipe's judge rule chooses.

No model change: the shipped recipe routes that rule to deepseek-v4-pro, which
is what judge-model used to name. Setting judge-model explicitly still pins a
model outright, regardless of the recipe.

The header goes out unconditionally, including when --model names a concrete
model: nothing resolves an alias then, so nothing reads it, and "is this an
alias" is the gateway's judgement rather than this script's.

ORDER: OrcaRouter-O2#1433 puts the judge rule in the provisioned recipe and has
to be deployed first. Without it the judge lands on the recipe's default — the
model it is scoring — which agrees with itself while still reporting success.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ZhenghuaBao
ZhenghuaBao merged commit 5ff702c into main Aug 26, 2026
2 checks passed
@ZhenghuaBao
ZhenghuaBao deleted the fix/reactions-and-dsl-judge branch August 26, 2026 11:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6d7f459af

ℹ️ 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".

Comment thread action.yml
`precision-filter` is `"false"`.
required: false
default: "deepseek/deepseek-v4-pro"
default: ""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the judge override for existing router recipes

When an existing installation upgrades without explicitly setting judge-model, this empty default sends L2 through its existing workspace recipe. The checked-in recipes/orcacode-review.dsl.yaml has no x-cr-lens judge rule and explicitly notes that upgrading the Action does not update workspace recipes, so these installations route the judge to the recipe default—the same model performing the review—instead of the previous independent DeepSeek model. Keep the concrete fallback until existing recipes are migrated, or otherwise provide backward-compatible judge routing.

Useful? React with 👍 / 👎.

Comment thread action.yml
Comment on lines +1619 to +1620
ME_TYPE=$(gh api user --jq .type 2>/dev/null) || ME_TYPE=""
if [ "$ME_TYPE" = "User" ]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fall back the login when the user query returns empty

For the default installation token, the scenario described here—gh api user --jq ... succeeding with no selected value—also affects the subsequent .login query on line 1624. Its || echo "github-actions[bot]" fallback therefore does not run, ME remains empty, and clear_eyes selects no reactions, leaving the newly posted 👀 behind after every run. Checked gh api --help: --jq only selects values from the response and provides no missing-value fallback, so the login needs an explicit empty-result fallback too.

Useful? React with 👍 / 👎.

akf66 added a commit that referenced this pull request Aug 26, 2026
…ps itself (#44)

Cuts the release that carries #41 (App mode gone from the skill) and #42
(reactions on the default token, judge model from the recipe), and fixes a
dependency the package should never have had.

THE PACKAGE DEPENDED ON ITSELF. 1.1.0 added `@orcarouter/code-review: ^1.0.2`
to its own `dependencies` and five releases carried it. Every `npx` therefore
downloaded a second, older copy of the CLI into node_modules before running the
one it came for — latency on the first thing a new user does, and RELEASE.md
already said the CLI has no dependencies. Nothing failed, which is why it
survived: the bin resolves from the top level, so the nested copy is dead
weight rather than a wrong entry point. A self-reference is also a registry
dependent, and npm reads dependents when deciding whether a version may be
withdrawn.

Pinned by a test rather than a note — `dependencies`, `peerDependencies` and
`optionalDependencies` must all be empty. The invariant was already documented
in prose and still broken for five releases.

2.0.0, not 1.5.1. 1.5.0 shipped the App-mode removal as a minor, and the
1.x line is being unpublished inside npm's 72-hour window, so 2.0.0 is the
first version on the registry that a user can actually install — the major is
where the break belongs, and a lone `1.5.1` would imply a history the packument
no longer has.

RELEASE.md records what went with the withdrawn versions, because a packument
with one version reads as a truncated upload. It also says plainly that
unpublishing was defensible only for a two-day-old line with no known
consumers, and that deprecation is the default everywhere else.

Three versions move together (gate 1): package.json, plugin.json,
marketplace.json.
akf66 added a commit that referenced this pull request Aug 26, 2026
* release: 2.0.0 — the Action is the only install path, and the CLI drops itself

Cuts the release that carries #41 (App mode gone from the skill) and #42
(reactions on the default token, judge model from the recipe), and fixes a
dependency the package should never have had.

THE PACKAGE DEPENDED ON ITSELF. 1.1.0 added `@orcarouter/code-review: ^1.0.2`
to its own `dependencies` and five releases carried it. Every `npx` therefore
downloaded a second, older copy of the CLI into node_modules before running the
one it came for — latency on the first thing a new user does, and RELEASE.md
already said the CLI has no dependencies. Nothing failed, which is why it
survived: the bin resolves from the top level, so the nested copy is dead
weight rather than a wrong entry point. A self-reference is also a registry
dependent, and npm reads dependents when deciding whether a version may be
withdrawn.

Pinned by a test rather than a note — `dependencies`, `peerDependencies` and
`optionalDependencies` must all be empty. The invariant was already documented
in prose and still broken for five releases.

2.0.0, not 1.5.1. 1.5.0 shipped the App-mode removal as a minor, and the
1.x line is being unpublished inside npm's 72-hour window, so 2.0.0 is the
first version on the registry that a user can actually install — the major is
where the break belongs, and a lone `1.5.1` would imply a history the packument
no longer has.

RELEASE.md records what went with the withdrawn versions, because a packument
with one version reads as a truncated upload. It also says plainly that
unpublishing was defensible only for a two-day-old line with no known
consumers, and that deprecation is the default everywhere else.

Three versions move together (gate 1): package.json, plugin.json,
marketplace.json.

* ci: a manual, guarded way to withdraw a version from npm

Publishing 2.0.0 was the easy half. Withdrawing 1.0.2-1.5.0 needs a credential
that can unpublish, and the only one this project has is the NPM_TOKEN
repository secret — nobody's laptop holds it, and npm's window is 72 hours from
publish, so "log in later" is not a plan. This runs the withdrawal where the
token already lives and leaves a run log of who removed what.

Guards, because the failure mode is deleting the live release rather than the
dead ones:

  * `confirm` must be typed as the word "unpublish". A dispatch button is one
    click; this job's blast radius is the whole package.
  * The version in package.json is refused outright. It is what dist-tags.latest
    points at and what every npx resolves.
  * Verification reads the ANONYMOUS packument afterwards and fails if the live
    version is missing or if `latest` moved. `npm unpublish` exiting 0 is not
    evidence, for the same reason gate 6 exists in publish.yml.

A version that cannot be withdrawn warns and the loop continues, then the job
fails at the end. Aborting mid-list would leave the line half-withdrawn, which
is the one state worse than either end.

Inputs reach the shell through `env`, never interpolated into a run block. This
job holds a publish token, and `${{ inputs.versions }}` inside a script is a
command-injection hole.

Shares the publish-npm concurrency group so it can never race a publish.
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.

1 participant