Repository navigation
fix(*): name the missing credential when an agent cannot inherit - #862
Conversation
inherit_llm answers a refusal with "", the value every launcher and every scaffolded copy branches on, so the reason gets a call of its own: inherit_refusal(config, host) returns the MissingCredentialsError summary of the binding inherit_llm checks, or "" when it would inherit. Both run on two private helpers, so the binding is composed in one place. A non-string agents.defaults.model with no provider now reaches Config validation instead of split_model_id, so host_can_lend_a_key answers False where it raised AttributeError past its except clause. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
Since #841 a launcher refuses when the selected host model cannot authenticate, which is no longer the same as the host having no provider key. The four keyed launchers and the scaffold template now say what is missing, for example "openai_codex needs a sign-in -- run `raven provider login openai-codex`", and keep the "<KEY> is not set" prefix that raven agents new's smoke check matches. raven-design appends the same reason to its own sentence. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
An agent is spawned with the login shell's environment plus RAVEN_HOME, and with raven's own environment only when that shell cannot be captured, so a CHATGPT_TOKEN_DIR or CHATGPT_AUTH_FILE set only in the host process does not reach it in the normal case. agents/README.md and the host_can_lend_a_key docstring now say where such an override has to live, and the docstring names the two cases in which setup can still offer an inheritance the launcher refuses. CHANGELOG records the clearer refusal. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the full github/main...HEAD diff and the relevant surrounding credential-selection, launcher, subagent-environment, and provider-authentication paths. I also checked the originating OAuth inheritance change and later history, all callers, backward compatibility of inherit_llm's empty-string refusal contract, the repository rules and canonical model-binding vocabulary, and whether the tests were weakened. The refactor keeps the inheritance decision and diagnostic reason on the same computed binding, preserves launcher behavior, and adds specific coverage across provider aliases, parent riders, malformed models, the scaffold, and all shipped launchers.
Verification: uv run --all-extras pytest tests/test_config_product_render.py tests/test_subagent_vendored_agents.py tests/test_agents_design_launcher.py tests/test_cli_agents_commands.py -q -> 319 passed, 1 skipped (the documented case-sensitive-filesystem skip). An initial run without --all-extras reached no test bodies because the global fixture could not import optional raven_everos; rerunning with the project extras resolved that setup issue. git diff --check github/main...HEAD also passed.
|
Not a blocker -- three small things, none of which holds the merge button. The Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text. 1. A scaffold generated by this raven dies with So the breaking direction is new scaffold on old raven, where the user gets
2. The new The second line is the control: if it synthesised Your conclusion is unaffected and the sentence's second half is exactly right 3. Nothing in the new suite pins that |
A folder that raven agents new generates is never refreshed when raven changes version, unlike the five shipped agents, which the packaged tree re-copies whenever the installed version differs. A scaffold written by this raven could therefore call render.inherit_refusal on a raven that predates it, and its refusal became an AttributeError. Every other function the template calls exists back to v0.2.0. The template now looks inherit_refusal up with getattr and, when it is missing, refuses with the same sentence without the reason. The old "no provider key to inherit from" wording is not the fallback: on a raven that carries #841 it is the untrue sentence this change replaces. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
The host_can_lend_a_key docstring and agents/README.md said an agent is spawned with RAVEN_HOME. The host adds RAVEN_HOME to the child's environment only when it has one set; on a default install neither side has it and both resolve ~/.raven, which is why the agent still reaches the host's OAuth sign-in. Both texts now say so. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
Every test drove the refusal through the host config or the dispatcher's riders, so inherit_refusal could ignore its config argument with the suite green. A host that names no agents.defaults.provider leaves an agent's own configured provider in force, and that case now separates the two: the reason names that provider's missing key, where a reason computed without the config is empty. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
The host_can_lend_a_key docstring called an agent's own agents.defaults.provider its "pinned" provider. CONTEXT.md retires "pin" for that field, which is the Configured provider; a Subsystem pin is a different setting. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
|
All three reproduced on 1. A scaffold on an older raven -- 2. 3. Also -- At the new head, deleting each construct this PR adds, one at a time, |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the four commits added since 33f8130b87af and rechecked the resulting full diff. The compatibility fallback preserves a generated scaffold's readable refusal when run with Raven versions predating inherit_refusal; the corrected home-sharing text now matches the spawn overlay and default-home behavior; the new config-sensitive test closes the identified coverage gap; and the provider terminology now follows CONTEXT.md. I also rechecked the affected callers and history, repository rules and architecture boundaries, backward compatibility, and whether the added tests weaken existing assertions.
Verification: uv run --frozen --python 3.12 --all-extras pytest -q -p no:randomly tests/test_config_product_render.py tests/test_subagent_vendored_agents.py tests/test_agents_design_launcher.py tests/test_cli_agents_commands.py tests/test_cli_smoke.py tests/test_agent_schemas.py -> 378 passed, 1 skipped (the documented case-sensitive-filesystem skip). git diff --check github/main...HEAD also passed. No review threads were open from my earlier review.
|
Not a blocker. Round two on Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text. The three round-one nits are closed, with withdrawal mutationsEach mutant was asserted violating before any result was read.
Gates on the merged tree (base is an ancestor of head, so head is the merged tree): Nit 1 -- the stamp you leaned on does not run on a non-wheel install
Reproduced on a temp The window predates this PR -- Nit 2 -- the new sentence has one case it does not cover
Probe: The code is unchanged and predates the PR, and Nit 3 -- a third reader of the same fact, in a file this PR edits
Explicitly pre-existing, not yours: those six lines are byte-identical at the base Stated, not filed
Not coveredNo two real raven installations were stood up, so the end-to-end older-raven scenario was |
The five shipped launchers called render.inherit_refusal directly on the grounds that the packaged tree is re-copied whenever the installed version changes. That refresh runs only in the wheel shape: in an editable install or a checkout there is no packaged tree, and a home tree a newer wheel copied out still outranks the checkout's own, so a launcher can run on a raven that predates the function. As with the template, inherit_refusal is the only function any of them calls that v0.2.0 through v0.2.4 lack, so the same getattr lookup and reason-less fallback make them refuse in words there too. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
With no RAVEN_HOME in the host process, a RAVEN_HOME exported by the login profile still reaches an agent, which then reads that home's config and sign-ins instead of the host's ~/.raven. The host_can_lend_a_key docstring lists it as a third case in which setup can offer an inheritance the launcher refuses, and agents/README.md says the same. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
The host_config docstring said the host propagates its RAVEN_HOME into a launcher, naming builtin_agents, which spawns none of the agents in agents/. The host passes RAVEN_HOME only when it has one set; with none set, host and launcher normally resolve the same ~/.raven. The docstring now says so, matching host_can_lend_a_key. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
|
All three taken, one commit each. Nit 1, the stamp outside the wheel shape -- Nit 2, the login-profile case -- Nit 3, Stated, not filed. The Six files: 382 passed, 2 skipped (+5, the new cases). Full suite: |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the three commits added since 1078dd3deb7e and rechecked the resulting full diff. The optional diagnostic lookup is now consistent across the scaffold and all five shipped launchers; the login-profile RAVEN_HOME divergence is accurately documented at each affected reader; and the stale host_config explanation no longer names the wrong spawn path. I independently compared every product_render symbol used by all six launchers against tags v0.2.0 through v0.2.4; inherit_refusal is the only missing symbol, so the compatibility guard is complete for those versions. I also rechecked callers and history, repository rules and domain vocabulary, architecture boundaries, backward compatibility, and test strength.
Verification: uv run --frozen --python 3.12 --all-extras pytest -q -p no:randomly tests/test_config_product_render.py tests/test_subagent_vendored_agents.py tests/test_agents_design_launcher.py tests/test_cli_agents_commands.py tests/test_cli_smoke.py tests/test_agent_schemas.py -> 383 passed, 1 skipped (the documented case-sensitive-filesystem skip). git diff --check github/main...HEAD also passed. No review threads were open from my earlier reviews.
|
Not a blocker. Round three on Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text. Gates on the merged tree: The three round-two nits are closedNit 1, the five shipped launchers. Nits 2 and 3, the prose. Both rewrites say what the code does. I also checked the question Nit 1 -- the new
|
The launcher test asserted only that the reason's text appears, which holds with or without the parentheses around it, so in four of the six launchers the clause's shape could change with the suite green. It now matches "cannot be inherited (...); put the key in" for the keyed launchers and "before starting Design (...)" for raven-design. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
The host_can_lend_a_key docstring counted three ways setup can offer an inheritance the launcher refuses, and missed a fourth: the agent's stored row env is merged last on both spawn paths, so a RAVEN_HOME there overrides the host's. The docstring now states why such cases exist -- the agent's environment and config are not this process's -- instead of a count, and names the stored row beside the login profile. agents/README.md names both routes. Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
Three more texts in files this change edits said the launcher inherits RAVEN_HOME: the inherit_plugin_opt_outs docstring and the agent-home comments in the raven-design and raven-ppt launchers. The host passes it only when it has one set; they now say the launcher normally shares the host's home. The rewrapped docstring paragraph also says "agent" where it said "product". Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
|
Both taken, plus the readers of the same claim in files this PR edits. Nit 1, the parentheses -- Nit 2, the count -- Stated, not filed -- Six files: 382 passed, 2 skipped (no new cases; one test asserts more). |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the three commits added since 84cd85edb007 and rechecked the resulting full diff. The strengthened test now pins the parenthesized diagnostic shape for every launcher branch. The documentation no longer makes a false exhaustive case count, names both login-profile and stored-row RAVEN_HOME precedence, and corrects the remaining readers in files touched by this change without altering runtime behavior. I also checked the affected callers and both environment-merge paths, repository rules and vocabulary, architecture boundaries, backward compatibility, and whether the assertion change weakened existing coverage.
Verification: uv run --frozen --python 3.12 --all-extras pytest -q -p no:randomly tests/test_config_product_render.py tests/test_subagent_vendored_agents.py tests/test_agents_design_launcher.py tests/test_cli_agents_commands.py tests/test_cli_smoke.py tests/test_agent_schemas.py -> 383 passed, 1 skipped (the documented case-sensitive-filesystem skip). git diff --check github/main...HEAD also passed. No review threads were open from my earlier reviews.
|
Not a blocker. Round four on Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text. The parenthesised reason is now pinned, per launcher. The mutant that survived nine test The completeness claim and the stale sentences are gone. The delta changes no executable statement, measured rather than eyeballed: with every Gates: Two corrections to things I said in earlier rounds, since they were mine and not yours: this The three pre-existing readers of the old unconditional claim I listed last round |
Summary
Follow-up to #841, which lets an agent inherit an OAuth-backed host
model. Since #841 a launcher refuses when the host's selected model
cannot authenticate, but the refusal still said the host config had no
provider key to inherit from. That is untrue in the case #841 made
reachable: the host holds an OpenRouter key while its selected model is
OpenAI Codex with no sign-in.
inherit_refusal(config, host)inraven/config/product_render.pyreturns the
MissingCredentialsErrorsummary for the bindinginherit_llmchecks, or""when it would inherit.inherit_llmkeeps answering a refusal with
"", the value every launcher brancheson, scaffolded copies outside this repository included. Both now run
on two private helpers, so the inherited binding is composed in one
place. A non-string
agents.defaults.modelwith no provider nowreaches
Configvalidation instead ofsplit_model_id, sohost_can_lend_a_keyanswers False where it raisedAttributeErrorpast its except clause.
raven agents newtemplate printthat reason and keep the
<KEY> is not setprefix the scaffold smokecheck matches; raven-design appends it to its own sentence. All six
look
inherit_refusalup withgetattrand, on an older raven thatlacks it, refuse with the same sentence without the reason: a folder
the template generates is never refreshed when raven changes
version, and a home tree a newer wheel copied out also serves an
older checkout.
raven's own environment only when that shell cannot be captured, so
a
CHATGPT_TOKEN_DIRorCHATGPT_AUTH_FILEset only in the hostprocess does not normally reach it. This PR documents that rather
than forwarding the variables: the spawn path adds the host's
RAVEN_HOMEwhen one is set, and with it unset both sides normallyresolve
~/.raven, so the default token location is shared; acustom override can live in the login profile or the agent's stored
row
env.agents/README.mdand thehost_can_lend_a_keyandhost_configdocstrings say so, and thehost_can_lend_a_keydocstring says why setup can still offer an inheritance the launcher
refuses -- the agent's environment and config are not the host's --
and names the routes, among them a
RAVEN_HOMEfrom the loginprofile or from the agent's stored row. Three older texts in files
this PR edits, which said the launcher inherits
RAVEN_HOME, sayinstead that it normally shares the host's home.
Checked and deliberately left as is:
inherit_llm's docstring moved below the new helpers verbatim, apartfrom the pointer to
inherit_refusal, so it keeps its older wording;git diff --color-movedshows the move.raven agents newstill matches the old "noprovider key to inherit from" text, because scaffolded copies in
users' homes keep printing it.
MissingCredentialsError.remedyis not appended: every summary thischeck raises carries its own command or is answered by the launcher's
closing "configure a provider in the host raven", and the generic
remedy is multi-line.
validation traceback, exactly as before; the call that raises it
moved into a helper unchanged.
RAVEN_HOMEclaim also sits inraven/agent/subagent/role.py, twice inCONTEXT.mdand indocs-site/docs/oncall.mdwith its Chinese twin. None is in a filethis PR edits, and all predate it; they are left for a separate
change.
Type
Verification
Targeted, on the head:
382 passed, 2 skipped. Deleting each construct this PR adds turns that
run red: the non-string guard (2 failed), the empty-summary fallback
(1),
inherit_refusalreading itsconfig(1), and, in each of thesix launchers, the reason and the parentheses around it (1 each; 2
for the template and for raven-design) and the
getattrlookup (1each, when replaced by a direct attribute read). The same run passes
(382 passed, 2 skipped)
with provider API keys exported and
CODEX_HOMEandCHATGPT_TOKEN_DIRpointing at a fake sign-in, so the refusal testsdo not depend on the machine lacking credentials.
Full suite:
2 failed, 27580 passed, 115 skipped. Both failures also fail at the
base, 3632e60, on this machine and touch nothing here:
test_install_script.py::test_resolve_node_dir_answers_each_case_it_exists_for(an npm elsewhere on PATH leaks past the fixture) and
test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothing(the run is root, which reads a file the test made unreadable). The
proxy is unset because five unrelated provider tests fail when the
shell carries this machine's proxy.
Lint and gates:
ruff checkandruff format --checkover the twelvechanged Python files,
ty checkover the eight non-test ones, andlint-imports(10 kept, 0 broken) are clean;scripts/check_commit_messages.py origin/main..HEAD,commitlint --from origin/main --to HEAD,scripts/check_source_language.py origin/main...HEADandscripts/check_large_files.py origin/main...HEADexit 0.Rendered refusals, the first from a host with an OpenRouter key and an
unsigned Codex model, the second from
raven agents new demo-agentina home with no key:
Risk
The refusal wording changes for raven-code, raven-oncall,
raven-research, raven-ppt, raven-design and the scaffold template; a
script matching the old "no provider key to inherit from" text stops
matching for these launchers.
The launchers and the template now reach
render.inherit_refusalthrough
getattr, because each can meet a raven that predates it: afolder
raven agents newgenerated is never refreshed, and thepackaged tree is re-copied on a version change only in the wheel
shape, so a home tree a newer wheel copied out also serves an older
checkout. Every other function they call exists back to v0.2.0, and
on such a raven they refuse without the reason.
No secret reaches the message: a summary names a provider and a
command, never a key value.
Rollback: revert the squash commit; nothing is stored or migrated.
Security impact considered
Backward compatibility considered
Rollback path is clear for risky changes
Related Issues
#841