Skip to content

fix(agent): name grok login on a credential refusal - #792

Open
ZuyiZhou wants to merge 1 commit into
mainfrom
fix/grok_sign_in_hint
Open

ZuyiZhou wants to merge 1 commit into
mainfrom
fix/grok_sign_in_hint

Conversation

@ZuyiZhou

Copy link
Copy Markdown
Contributor

Summary

A Grok Build row that fails for want of a credential was told to "sign in to it and connect again", with no command. Claude Code, Codex, Hermes and Qwen Code already name the command read from the installed tool. grok login --help on Grok Build 1.0.41 titles that command "Sign in to Grok", so a credential refusal now says to run grok login.

The row launches grok agent stdio, a local install, so there is one spelling and no npx fallback. The binary is native, so a launch that quits is not reported as an old Node.js. A provider status (404, 402, 429) still gets the shared verdict and no in-agent command: none was measured that fixes a missing model, a missing credit and a rate limit together.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

  • uv run --frozen --python 3.12 --extra dev ruff check raven/agent/subagent/presets.py tests/test_subagent_third_party.py tests/test_subagent_node_runtime.py and ruff format --check on the same files: clean.
  • uv run --frozen --python 3.12 --extra dev pytest -q tests/test_subagent_third_party.py::test_grok_login_is_the_one_spelling tests/test_subagent_third_party.py::test_each_agent_s_fix_is_named_as_data_from_the_one_decision tests/test_subagent_third_party.py::test_a_row_with_one_spelling_is_not_offered_a_second tests/test_subagent_node_runtime.py::test_a_launch_that_quit_on_an_old_node_is_named_with_both_versions: 4 passed.
  • Reverting raven/agent/subagent/presets.py to origin/main makes test_grok_login_is_the_one_spelling fail with KeyError: 'grok'. The Node.js test stays green.
  • python scripts/check_source_language.py origin/main...HEAD: exit 0.
  • git merge-tree --write-tree HEAD origin/main: clean. git diff --check origin/main...HEAD: clean.

The full suite, the type checker and a live logged-out grok agent stdio were not run.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

No user-facing doc describes this refusal sentence. The page reads the remedy the connect path already returns.

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Only a credential-shaped refusal on the grok preset gains a command. A hand-written row that is not that preset is unchanged. Billing, quota, a missing model and a launch that quits keep the sentences they already had.

Rollback: revert the squash commit. There is no migration.

Related Issues

N/A

A Grok Build row that fails for want of a credential was told to sign
in with no command. grok login --help titles that command "Sign in to
Grok". The row is a local install, so there is one spelling, and a
provider status is still reported without an invented in-agent command.
@ZuyiZhou
ZuyiZhou requested a review from LivXue as a code owner September 24, 2026 16:06

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No blockers; this can merge as far as I am concerned.

Reviewed the full diff, the refusal/remedy callers, preset provenance and Grok ACP registration, relevant history, backward compatibility, test intent (including checks that non-auth provider failures remain unclassified), and the repository rules/domain context. I also checked that the branch merges cleanly with the target. The new command agrees with the official Grok Build authentication guide: grok login re-runs sign-in, with OAuth as the default flow. I found no weakened tests or architecture-rule conflicts.

Verification: uv run pytest tests/test_subagent_third_party.py tests/test_subagent_node_runtime.py -x (288 passed); Ruff check and format check on all changed files (passed); source-language check and git diff --check (passed).

@0xKT

0xKT commented Sep 24, 2026

Copy link
Copy Markdown
Member

Not a blocker. Accepted on 3de174e0 merged onto e1769411. Two lanes, three agents, zero findings. Six lines of data, so the review was sized to match -- here is what was attacked and held.

The row's own defence checks out, and not for the reason the comment gives

The comment says grok is "a local install, like hermes". I did not take that on its word, because SignIn's docstring (presets.py:346-350) makes it a rule rather than a style note: a shim-launched row must not leave anywhere empty, since naming the obvious command on a machine that never installed the CLI "would answer a credential failure with a second one". So the question is which kind grok is, answered from the preset definition:

acp_registry_presets.py:164   "command": "grok agent stdio"
acp_registry_presets.py:105   ACP_REGISTRY_SHIM_PRESETS = frozenset({"pi"})
presets.py:129                SHIM_LAUNCHED_PRESETS = {"claude_code", "codex"} | ACP_REGISTRY_SHIM_PRESETS

grok is in neither set, so it is a local install and anywhere=None is required, not merely permitted. The comment's conclusion is right; its stated reason ("like hermes") is weaker than the rule that actually licenses it.

What the omitted fields actually do

_remedy_for (probe.py:300-315) computes local = which(hint.exe, path=_login_path()) is not None, then Remedy(hint.does, hint.local if local or hint.anywhere is None else hint.anywhere, hint.then). With anywhere=None the ternary short-circuits, so grok renders hint.local on every machine, installed or not. Driven through the shipped path both ways -- real environment (grok genuinely absent) and with shutil.which replaced by one that pretends every CLI is installed, which is the only substitution and it is the environment:

grok    -> Remedy(kind='sign_in', command='grok login')
hermes  -> Remedy(kind='setup',   command='hermes model')

Identical in both environments, which is the point: with anywhere=None the answer cannot depend on PATH. The sentence is complete, no None leaks into it (asserted against 'None' not in text), and the one difference a reader would notice between grok and hermes is Remedy.kind, not the English.

One definition, checked with a control

grep -rn -i "grok login|grok auth|sign in to grok" over the tree hits only presets.py:392-397 and this PR's tests. That negative is only worth something with a positive control, so: the same query shape for "claude auth login" / "hermes model|kimi login|codex login" returns 10+ hits across ui-web tests and tests/test_rpc_subagents.py:677. The query finds second statements when they exist; there is none for grok.

There is a second decider, and I could not reach it: kimi_code.py:115 and :278-279 build a sign-in Remedy from hint.local alone, ignoring anywhere. Both public entry points check cfg.preset == "kimi_code" first (:233, :262), so a grok row cannot get there -- and it predates this PR either way.

The one claim I could not check, and what does corroborate it

grok is not installed here (command -v grok empty; which is None both in-process and against probe._login_path()). So "grok login --help titles Sign in to Grok", "--oauth names the default path" and "Grok Build 1.0.41" are your measurement, unchecked -- I neither assert nor contradict them. There is no in-tree record of grok's CLI surface to check against (same grep, same positive control: "kimi login" returns five hits in test_subagent_kimi_code.py, so the query works).

What does corroborate the structural half: ACP_REGISTRY_INSTALL_HINTS['grok'] = 'npm i -g @xai-official/grok' (acp_registry_presets.py:113, and ui-web/src/features/extAgents/catalogue.ts:86). A global npm install does put grok on PATH, which supports "a local install" -- not the subcommand spelling.

Receipts

lint-imports -> 892 files, 6346 dependencies, 10 kept, 0 broken. ruff check all passed, ruff format --check 3 files already formatted. Lane tests via the board's pyt.sh -> 342 passed (third_party + node_runtime + kimi_code) and 288 passed (the two touched files). Retraction mutations on the grok row: 1 failed / 287 passed and 2 failed / 286 passed -- the tests do pin it.

One note on the machine-gate set, because "red on both sides" is not by itself an answer: tests/test_updates_install_guard.py shows 5 failures here, and the identical five appear on base e1769411 (5 failed, 20 passed). The mechanism, not just the parity: those tests spawn python -I -c "from raven.updates import install_guard ...", -I drops PYTHONPATH, and the board venv carries no raven dist-info, so the child raises PackageNotFoundError: raven. A property of my measurement environment; this PR touches nothing under raven/updates/.

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.

3 participants