Skip to content

fix(*): name the missing credential when an agent cannot inherit - #862

Merged
ZuyiZhou merged 13 commits into
mainfrom
fix/agents_oauth_inheritance_parity
Oct 8, 2026
Merged

ZuyiZhou merged 13 commits into
mainfrom
fix/agents_oauth_inheritance_parity

Conversation

@LivXue

@LivXue LivXue commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

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) in raven/config/product_render.py
    returns the MissingCredentialsError summary for the binding
    inherit_llm checks, or "" when it would inherit. inherit_llm
    keeps answering a refusal with "", the value every launcher branches
    on, 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.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.
  • The four keyed launchers and the raven agents new template print
    that reason and keep the <KEY> is not set prefix the scaffold smoke
    check matches; raven-design appends it to its own sentence. All six
    look inherit_refusal up with getattr and, on an older raven that
    lacks 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.
  • An agent is spawned with the login shell's environment, 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 normally reach it. This PR documents that rather
    than forwarding the variables: the spawn path adds the host's
    RAVEN_HOME when one is set, and with it unset both sides normally
    resolve ~/.raven, so the default token location is shared; a
    custom override can live in the login profile or the agent's stored
    row env. agents/README.md and the host_can_lend_a_key and
    host_config docstrings say so, and the host_can_lend_a_key
    docstring 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_HOME from the login
    profile or from the agent's stored row. Three older texts in files
    this PR edits, which said the launcher inherits RAVEN_HOME, say
    instead that it normally shares the host's home.
  • CHANGELOG records the clearer refusal.

Checked and deliberately left as is:

  • inherit_llm's docstring moved below the new helpers verbatim, apart
    from the pointer to inherit_refusal, so it keeps its older wording;
    git diff --color-moved shows the move.
  • The smoke check in raven agents new still matches the old "no
    provider key to inherit from" text, because scaffolded copies in
    users' homes keep printing it.
  • MissingCredentialsError.remedy is not appended: every summary this
    check 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.
  • A malformed host provider section still ends a launcher with a
    validation traceback, exactly as before; the call that raises it
    moved into a helper unchanged.
  • The same unconditional RAVEN_HOME claim also sits in
    raven/agent/subagent/role.py, twice in CONTEXT.md and in
    docs-site/docs/oncall.md with its Chinese twin. None is in a file
    this PR edits, and all predate it; they are left for a separate
    change.

Type

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

Verification

Targeted, on the head:

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

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_refusal reading its config (1), and, in each of the
six launchers, the reason and the parentheses around it (1 each; 2
for the template and for raven-design) and the getattr lookup (1
each, when replaced by a direct attribute read). The same run passes
(382 passed, 2 skipped)
with provider API keys exported and CODEX_HOME and
CHATGPT_TOKEN_DIR pointing at a fake sign-in, so the refusal tests
do not depend on the machine lacking credentials.

Full suite:

env -u HTTP_PROXY -u HTTPS_PROXY -u http_proxy -u https_proxy -u no_proxy uv run --frozen --python 3.12 --all-extras pytest -q -p no:randomly

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 check and ruff format --check over the twelve
changed Python files, ty check over the eight non-test ones, and
lint-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...HEAD and scripts/check_large_files.py origin/main...HEAD exit 0.

Rendered refusals, the first from a host with an OpenRouter key and an
unsigned Codex model, the second from raven agents new demo-agent in
a home with no key:

error: CODE_API_KEY is not set and the host's model cannot be inherited (openai_codex needs a sign-in -- run `raven provider login openai-codex`); put the key in <agent dir>/.env (see .env.example), export it, or configure a provider in the host raven
error: DEMO_AGENT_API_KEY is not set and the host's model cannot be inherited (no provider is configured yet -- run `raven onboard` for guided setup); put the key in <agent dir>/.env (see .env.example), export it, or configure a provider in the host raven
  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

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_refusal
    through getattr, because each can meet a raven that predates it: a
    folder raven agents new generated is never refreshed, and the
    packaged 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

LivXue and others added 3 commits October 4, 2026 17:07
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>
@LivXue
LivXue requested review from 0xKT and gloryfromca October 5, 2026 11:17

@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 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.

@0xKT

0xKT commented Oct 5, 2026

Copy link
Copy Markdown
Member

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.
change is good: naming the missing credential is a real improvement over the
generic sentence, and keeping inherit_llm's "" contract untouched while
adding inherit_refusal beside it is the right shape for a signature other
people's copies branch on.

1. A scaffold generated by this raven dies with AttributeError on an older
one.
raven/templates/agents_scaffold/run.py:77 now calls
render.inherit_refusal(...) through a plain from raven.config import product_render as render. The five shipped launchers are protected from this
skew by the version stamp in _install_packaged_tree
(vendored_agents.py:195-198 re-copies whenever the stamp differs from
__version__, so a downgrade restores a matching run.py). A scaffolded folder
is not in the packaged tree and is never refreshed, and nothing under
raven/templates/agents_scaffold/ pins a raven version -- I grepped for
raven>= / raven== there and found none.

So the breaking direction is new scaffold on old raven, where the user gets
a traceback instead of the refusal the line was written to produce. The benign
direction is old scaffold on new raven, which still works precisely because you
left inherit_llm's contract alone. Reproduced by copying the template to a
temp folder and deleting the attribute to present what an older raven presents:

THIS raven (HEAD):            SystemExit -> error: ... cannot be inherited (no provider is
                              configured yet -- run `raven onboard` for guided setup); ...
OLDER raven (no attribute):   AttributeError: module 'raven.config.product_render'
                              has no attribute 'inherit_refusal'

getattr(render, "inherit_refusal", None) in the template, falling back to the
old wording, would make the scaffold work against both.

2. The new host_can_lend_a_key docstring names a mechanism that only holds
when RAVEN_HOME is set.
It says an agent is "spawned with the login shell's
environment plus RAVEN_HOME". The child env is built at
acp_client/client.py:249 as {**base_env, **host_identity_env(), ...}, and
RAVEN_HOME can only enter through host_identity_env(), which is
backends/env.py:193-194:

RAVEN_HOME unset (the default install):  host_identity_env() = {}      -> child gets none
RAVEN_HOME set:                          host_identity_env() = {'RAVEN_HOME': '/tmp/some-home'}

The second line is the control: if it synthesised raven_home() it would print
a path either way. So on a default install the child gets no RAVEN_HOME at
all, and it reaches the same OAuth directory because home.py:55-56 makes host
and child both fall back to ~/.raven -- not because anything was propagated.

Your conclusion is unaffected and the sentence's second half is exactly right
(env.py:90 and :93 both return dict(os.environ)). I am raising it only
because this PR's whole purpose is making this paragraph true, so "plus
RAVEN_HOME when the host has one set, and the same default home otherwise"
seems worth the extra clause.

3. Nothing in the new suite pins that inherit_refusal reads its config.
Every new test drives the refusal through the host dict or the env riders. Of
the six call sites, four pass {} and two pass a key _inherited_defaults
never reads for this purpose -- so inherit_refusal can ignore its config
argument entirely and all 101 tests still pass. Mutating it to
_inherited_defaults({}, host) gives 101 passed; the control, making it
return "", gives 10 failed, 91 passed (including all five launcher tests),
so the suite does reach the line. A host that names no agents.defaults.provider
with a product that pins its own -- the situation your own new docstring
describes -- is the case that separates them, and the symptom of a future break
is the empty parenthesis in error: ... ().

LivXue and others added 4 commits October 5, 2026 16:26
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>
@LivXue

LivXue commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

All three reproduced on 33f8130b8, and each is fixed in its own
commit. A fourth commit fixes a vocabulary slip the same pass turned
up.

1. A scaffold on an older raven -- 68c5585c3. Reproduced through the
real scaffold: raven agents new demo-agent --no-smoke, the generated
run.py loaded in-process, inherit_refusal deleted from
product_render, and render_config raised AttributeError at the
generated run.py:77. I also checked the premise against the
releases: every other function the template calls exists in v0.2.0
through v0.2.4, so this was the first change to raise a scaffold's
minimum raven, and my Risk line calling it "the coupling every
product_render addition carries" was wrong. It is rewritten. The
template now looks the function up with getattr. One deviation from
your shape: the fallback is the new sentence without the reason, not
the old wording, because on v0.2.4, which carries #841, "no provider
key to inherit from" is the untrue sentence this PR replaces.
test_a_scaffold_still_refuses_in_words_on_a_raven_without_inherit_refusal
asserts both branches in one test and was red with the
AttributeError before the template change. The five shipped
launchers keep the direct call, since _install_packaged_tree
re-copies them whenever the stamp differs from __version__.

2. RAVEN_HOME -- e87fd851a. Reproduced: host_identity_env() is
{} with RAVEN_HOME unset and {'RAVEN_HOME': '/tmp/some-home'}
with it set. The docstring and agents/README.md now say the agent
resolves the same home because the host passes its RAVEN_HOME when
it has one set, and both otherwise fall back to ~/.raven. The PR
description made the same claim and is corrected too. The docstring
edit stays docstring-only (AST identical to the base).

3. config unpinned -- cedb2bdc3. Reproduced, at a wider scope:
with inherit_refusal computing _inherited_defaults({}, host), the
ten test files that reach the refusal gave 565 passed, the same as the
baseline, against 12 failed for return "".
test_inherit_refusal_reads_the_config_when_the_host_names_no_provider
uses the case you named. With config read, the reason is
"openrouter needs an API key -- run raven provider set openrouter --api-key <key>"; under the mutant it is '', the empty parenthesis
you predicted, and the test fails.

Also -- 1078dd3de. Running the retired-vocabulary check over this
round's text showed that the docstring called an agent's
agents.defaults.provider its "pinned" provider. CONTEXT.md retires
that word for the field, which is the Configured provider. Round one's
check missed it because grep -w pin does not match pinned.

At the new head, deleting each construct this PR adds, one at a time,
turns the six-file run red, including the config read (1 failed) and
the getattr lookup (1 failed when replaced by a direct attribute
read). Six files: 377 passed, 2 skipped. Full suite: 2 failed, 27575
passed, 115 skipped, the same two failures as at the base. The round-one
head, run the same day, gives 27573 passed with the identical 115
skips, so the two new tests are the whole difference; the 116 skips
the description quoted before came from a run the day before. The
description's Summary, Verification and Risk sections are updated to
match.

@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 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.

@0xKT

0xKT commented Oct 5, 2026

Copy link
Copy Markdown
Member

Not a blocker. Round two on 1078dd3d. All three nits from round one are fixed, and

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.
I withdrew each fix to prove it rather than reading the green. Three new nits below, all
non-blocking; this comment holds nothing.

The three round-one nits are closed, with withdrawal mutations

Each mutant was asserted violating before any result was read.

  1. The untested seam. Mutating product_render.py:235 back to
    _credential_refusal(host, _inherited_defaults({}, host)) -- asserted violating by AST
    (no config name survives in the body; the control assertion is that the unmutated body
    does contain it) -- now gives 1 failed, 321 passed, the failure being
    test_inherit_refusal_reads_the_config_when_the_host_names_no_provider. Round one the
    same mutant gave 101 passed.

  2. The scaffold skew. Reverting the template's two getattr lines to the direct
    f" ({render.inherit_refusal(config, host)})" -- asserted violating first: no
    getattr(render survives in the file, and the control is that the original does contain
    one -- gives 1 failed, 71 passed with
    AttributeError: module 'raven.config.product_render' has no attribute 'inherit_refusal'.
    The traceback path
    (.../test_a_scaffold_still_refuses_0/home/agents/demo-agent/run.py:77) also answers what
    I could not check last round: the test drives the real raven agents new output, not a
    copied template.

  3. The prose. get_oauth_dir() is raven_home() / "oauth" (config/paths.py:99) and
    raven_home() reads only RAVEN_HOME, so --config moves get_config_path() without
    moving the OAuth directory. The rewritten sentence is about the right thing. One
    exception below.

Gates on the merged tree (base is an ancestor of head, so head is the merged tree):
gates.sh ruff/lint-imports/commit/large/lang rc=0; the four touched test files
322 passed in 7.36s (320 last round, +2 new tests); guard set
(guards/l3_open_world/l4_entrances) 67 passed in 8.83s; lint-imports 10 kept,
0 broken.

Nit 1 -- the stamp you leaned on does not run on a non-wheel install

raven/agent/subagent/vendored_agents.py:131. Leaving the five shipped launchers on a direct
render.inherit_refusal(...) is justified by _install_packaged_tree refreshing them whenever
the version stamp differs. That refresh sits behind if packaged.is_dir():, and packaged is
<package>/agents, which exists only in the wheel shape. On an editable install or a plain
checkout the directory is absent, the refresh never runs, and line 133 still returns the home
tree when it is stamped -- so a ~/.raven/agents written by a raven at or after this PR
survives untouched under an older editable raven, with the roster row pointing into it.

Reproduced on a temp RAVEN_HOME (never your ~/.raven): with the packaged tree absent, a home
tree stamped 99.0.0-from-a-newer-raven is selected and the stamp is still
99.0.0-from-a-newer-raven after agents_root() returns; the control is the wheel shape, where
the same probe does reconcile the stamp and does overwrite the launcher. So the
non-reconciliation is the code's behaviour, not the harness.

The window predates this PR -- git diff 3632e604..1078dd3d -- raven/agent/subagent/vendored_agents.py
changes only the host_can_lend_a_key docstring -- but what falls into it is new: at the base,
git grep -l inherit_refusal -- agents raven/templates matches 0 files (positive control:
inherit_llm matches 6 on that same tree), and at head it matches all six. The asymmetry is
what I am reporting: the template states the rule in a comment and obeys it with getattr,
and the five siblings do neither. Blast radius is confined to the already-refusing branch, so
the cost is an AttributeError traceback where a sentence should be.

Nit 2 -- the new sentence has one case it does not cover

raven/agent/subagent/vendored_agents.py:390-391, same sentence at agents/README.md:11-13.
"with none set both fall back to ~/.raven" is false when the login profile exports
RAVEN_HOME: the child's base environment is the login shell's, and host_identity_env()
adds nothing when this process has none, so the profile's value reaches the child while the
host resolves ~/.raven.

Probe: HOME pointed at a temp dir whose .zprofile and .zshrc both export
RAVEN_HOME=/tmp/raven-from-the-profile, SHELL=/bin/zsh, RAVEN_HOME absent from the host
process. Captured RAVEN_HOME from the login shell = /tmp/raven-from-the-profile,
host_identity_env() = {}, child env RAVEN_HOME = /tmp/raven-from-the-profile, host
raven_home() = <temp>/.raven, child raven_home() = /tmp/raven-from-the-profile. The
control is the same probe with the profile not exporting it, where both sides land on
~/.raven.

The code is unchanged and predates the PR, and cli_agent.py:427-429 already concedes the case
with "normally set on the command line rather than in a profile". The sentence you replaced was
false in the common case, so this is still a clear improvement -- it is your call whether it
wants an "unless a login profile exports one" clause.

Nit 3 -- a third reader of the same fact, in a file this PR edits

raven/config/product_render.py:66-68. host_config()'s docstring still says "the host
propagates its RAVEN_HOME into a launcher process (builtin_agents does), so raven_home()
here is the host's home". That is the unconditional-propagation claim the commit titled
"say how an agent reaches the host's home" just corrected in two other places, and the
parenthetical names the wrong module for these agents: agents/ products are spawned through
acp_client/client.py:249 and backends/cli_agent.py:440, whose host_identity_env()
propagates nothing when RAVEN_HOME is unset, while builtin_agents.py:299 serves only the one
builtin seed row.

Explicitly pre-existing, not yours: those six lines are byte-identical at the base
(git show 3632e604:raven/config/product_render.py | sed -n 63,70p diffs clean against head).
The conclusion still holds because both sides share the ~/.raven default, so this is prose
precision. It is here only because the sweep reached two of the three readers and the third
lives in a file the same commit edits.

Stated, not filed

  • The whole-tree vocabulary sweep for the retired word on agents.defaults.provider is clean:
    zero hits in this PR's added lines, with pin|pins|pinned|pinning over raven/ matching 616
    files as the positive control -- an empty result here is an answer, not a broken query. My
    first attempt used \b, which git grep -E does not know; it returned 0 on the control and
    was redone.
  • getattr(render, "inherit_refusal", None) hides absence only. An inherit_refusal that
    exists and raises still propagates, so nothing that used to reach a user is swallowed.
  • CONTEXT.md:1618 says templates/ is "packaged data assets, zero Python". There are 12 .py
    files under raven/templates/ at head, including the one this PR edits. The behavioural
    reading (copied out, never imported by the runtime) holds, and nothing in this PR made it
    untrue, so it is not yours -- noting it because it is the line that decides the template's
    seat.
  • The architecture tier was answered on this head for both areas: no seat moves, nothing frozen
    is touched, one entrance (cli) in two dispatches, no new admission door, nothing mutating
    what a turn resolved, and the refusal reason has exactly one owner (_credential_refusal,
    with inherit_llm and inherit_refusal as two faces of it). The one-definition question is
    where nit 1 came from.

Not covered

No two real raven installations were stood up, so the end-to-end older-raven scenario was
reproduced by its mechanism (the stamp probe plus the AttributeError) rather than by running
it. The five shipped launchers' own diffs were read only to trace the stamp question.

LivXue and others added 3 commits October 6, 2026 07:28
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>
@LivXue

LivXue commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

All three taken, one commit each.

Nit 1, the stamp outside the wheel shape -- a394790f3. Reproduced
on a temp RAVEN_HOME: with no raven/agents in this checkout, a
home tree stamped 99.0.0-from-a-newer-raven is what agents_root()
returns, and the stamp is unchanged afterwards under 0.2.4. Before
choosing the fix I ran the template's census over the five shipped
launchers: for each, inherit_refusal is the only name it calls that
v0.2.0 through v0.2.4 lack (render.raven_home is an import in every
one), so the same getattr lookup is a complete fix rather than one
guarded call among unguarded ones. All six launchers now share it, and
the Risk line no longer leans on the stamp.
test_launchers_still_refuse_in_words_on_a_raven_without_inherit_refusal
(one case per launcher) was red with the AttributeError for all five
before the change, and replacing any one launcher's lookup with a
direct attribute read turns its case red. The stamp gap itself -- no
reconcile outside the wheel shape, and a stamped home tree outranking
the checkout's own -- predates this PR and is untouched here.

Nit 2, the login-profile case -- 04c7ba111. Reproduced with bash,
since this machine has no zsh: a temp HOME whose .bash_profile and
.bashrc export RAVEN_HOME=/tmp/raven-from-the-profile, and no
RAVEN_HOME in the host process. The login-shell capture and the
child both carry the profile's value, host_identity_env() is {},
and the host resolves <temp>/.raven; with a profile that exports
nothing, both sides carry none. The host_can_lend_a_key docstring now
lists it as a third case in which setup can offer an inheritance the
launcher refuses, the sentence above it says "normally", and
agents/README.md says the same.

Nit 3, host_config -- 84cd85edb. Reworded to the same mechanism,
without the builtin_agents reference; docstring-only (AST identical).

Stated, not filed. The CONTEXT.md line on templates/ is stale
as you describe; nothing here changed it, so it is left for a separate
change. Agreed that getattr hides absence only.

Six files: 382 passed, 2 skipped (+5, the new cases). Full suite:
2 failed, 27580 passed, 115 skipped, the same two failures as at
the base. At the new head, deleting each construct this PR adds, one
at a time, turns the six-file run red, including each launcher's reason
and lookup separately, beside 14 red for inherit_refusal returning
empty. The description's Summary, Verification and Risk sections are
updated.

@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 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.

@0xKT

0xKT commented Oct 6, 2026

Copy link
Copy Markdown
Member

Not a blocker. Round three on 84cd85ed. All three round-two nits are closed and I proved

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.
each by withdrawal rather than by reading the green. Two new nits below, both non-blocking;
this comment holds nothing.

Gates on the merged tree: gates.sh ruff/lint-imports/commit/large/lang rc=0; the four touched
test files 327 passed in 6.59s (322 last round, +5 = the new parametrized case over five
launchers); guard set 67 passed in 8.86s; lint-imports 10 kept, 0 broken.

The three round-two nits are closed

Nit 1, the five shipped launchers. git grep "render\.inherit_refusal(" -- agents now
returns nothing, with render.inherit_llm( matching once in each of the five as the positive
control, so the empty result is an answer and not a broken query. The parametrization is real:
oauth_launcher (test_config_product_render.py:215) takes product and loads
agents/<product>/run.py by path at :221-224, so each case executes a different module -- I
checked that first because a fixture that ignored product would have run one launcher five
times. Reverting ONE launcher at a time gives exactly one red case each:
[raven-design] -> 1 failed, 106 passed, agents/raven-design/run.py:172: AttributeError;
[raven-ppt] -> 1 failed, 106 passed at run.py:532. The other four stay green both times,
so the case bites per launcher.

Nits 2 and 3, the prose. Both rewrites say what the code does. I also checked the question
your round-two change raised for raven-design: inherit_llm gets deepcopy(host) while the
reason is computed on the original. Probed with the production functions -- inherit_llm
mutates config, never host (the dict compares equal to its pre-call copy after both calls),
so the two see the same host and there is nothing there.

Nit 1 -- the new (reason) punctuation is unpinned on the four non-design launchers

tests/test_config_product_render.py:409. The reason-PRESENT assertion for raven-code,
raven-oncall, raven-research and raven-ppt is only
assert "raven provider login openai-codex" in message (:421), which holds with or without the
parentheses, and the new test (:427) covers only the reason-ABSENT branch. The scaffold's shape
IS pinned (test_cli_agents_commands.py:357 and :377, cannot be inherited \([^)]+\); put the key in) and raven-design's is pinned (test_agents_design_launcher.py:363), so four of six sites
carrying this new clause can lose their punctuation silently.

Measured, with the control first because a green mutant means nothing if the suite never reads
that line. Control: replacing the whole statement in those four with a marker string fails
test_launchers_name_why_they_cannot_inherit[raven-code|raven-oncall|raven-research|raven-ppt],
so the suite does execute it and does read the text. Survivor: changing only
f" ({explain(config, host)})" to f" {explain(config, host)}" in the same four leaves nine
test files -- the five launcher suites plus config_product_render, cli_agents_commands,
subagent_vendored_agents and cli_subagent_setup -- at 561 passed, identical to the unmutated
run. Nothing at head is wrong; the clause is simply new and unpinned in four of the six places
it now lives.

Nit 2 -- "in three cases" is a completeness claim, and there is a fourth

raven/agent/subagent/vendored_agents.py:394. The rewritten docstring enumerates three ways
host_can_lend_a_key() can answer True for a launch that is then refused. A fourth is
production-reachable: the agent's stored config row carries env, and that map is merged LAST
on both spawn paths --
{**base_env, **host_identity_env(), **subagent_role_env(), **(env or {})} at
acp_client/client.py:249 and
{**env_base, **host_identity_env(), **subagent_role_env(), **(runtime_env or {}), **self.env}
at backends/cli_agent.py:440 -- so a row-level RAVEN_HOME beats the host's.

Reproduced with a control: two temp homes, the openai_codex sign-in under the HOST's home
only, same config.json in both, child environment assembled in the documented order. With the
row naming the other home, host_can_lend_a_key() is True and the child's inherit_llm()
returns '' -- refused. With the row carrying no RAVEN_HOME (the one variable removed),
True and accepted. Caveat stated rather than hidden: the probe assembles the child environment
by hand in that merge order instead of running a real spawn; the order itself is read off the
two lines quoted above.

What makes this worth a line rather than a shrug: agents/README.md:20-21, one paragraph above
the sentence this round adds, already documents that exact channel -- "when the agent's stored
row carries it in env (a stored row replaces the discovered one whole)" -- but only for
CHATGPT_TOKEN_DIR / CHATGPT_AUTH_FILE. The new RAVEN_HOME sentence names only the login
profile. A maintainer debugging "the wizard offered inherit and the launch refused" is told
there are exactly three places to look.

Stated, not filed

  • Three more readers of the unconditional claim, none of them yours. raven/agent/subagent/role.py:16
    says "The host already injects RAVEN_HOME into every child it launches" -- false in exactly
    the way round two's nit was -- and git show <base>:raven/agent/subagent/role.py is identical
    to head, so it predates this PR. Same at CONTEXT.md:284 and :382 and
    docs-site/docs/oncall.md:54-59. Not filed as yours; noted because the sweep that fixed three
    surfaces is one git grep away from finishing the job.
  • One of them is in a file you did edit. product_render.py:318-319
    (inherit_plugin_opt_outs) still says "the launcher inherits RAVEN_HOME, so <home>/plugins
    is the host's", 250 lines below the docstring you corrected at :63-68. Pre-existing -- the
    string is present at the base -- so not this PR's, but it is the same sentence in the same file.
  • A relative RAVEN_HOME also falsifies the new sentence, since host_identity_env() passes
    the raw string through. Not filed: I could not show a reader who would be misled by it that is
    not already covered by the row-env case.
  • getattr(..., None) hides absence but not failure. An inherit_refusal that exists and
    raises propagates out of all five as the raw exception rather than the clean SystemExit.
    That is the same behaviour as before the guard, so nothing regressed.
  • The guard now lives in six places in lockstep (five launchers plus the scaffold template).
    That is the shape that usually wants one owner, but the whole point is that the launchers run
    against a DIFFERENT raven than the one that wrote them, so a shared helper would have to live
    in the very module that may be missing. Not filed; the duplication is the design.
  • The stamp sentence in the five comments is accurate but narrow. "A home tree copied out by
    a newer wheel also serves an older checkout" -- a downgrade to an older wheel also triggers the
    refresh, and the uncovered window is the non-wheel shape I filed last round. Not filed: the
    sentence is true as far as it goes and the comment is not the place for the whole table.

Not covered

No two real raven installations were stood up, so the older-raven scenario is reproduced by its
mechanism (monkeypatch.delattr plus the per-launcher mutants), not by running it. The lanes
ran the reproduce lens only, severity nit buying one lens; I checked origin myself on both
findings -- the base carries no parenthesised reason in any launcher, and
git show <base>:...vendored_agents.py has neither "in three cases" nor "resolves the same
home" -- so both are new in this PR, and neither has a rule I can quote, which is why both are
nits and not higher.

LivXue and others added 3 commits October 6, 2026 16:20
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>
@LivXue

LivXue commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

Both taken, plus the readers of the same claim in files this PR edits.

Nit 1, the parentheses -- c0c3d23b3. Reproduced: with the four
keyed launchers' reason written without parentheses, the ten files that
reach the refusal gave 572 passed.
test_launchers_name_why_they_cannot_inherit now matches
cannot be inherited \(.+\); put the key in for the keyed launchers
and before starting Design \(.+\)$ for raven-design. That mutant now
fails all four keyed cases, and a parenthesis mutant in any single
launcher fails that launcher's case (2 for the template and for
raven-design, which also have their own shape tests).

Nit 2, the count -- b9bd6d568. Reproduced with a control: two homes
with the same config and the sign-in under the host's only.
host_can_lend_a_key() is True, a child whose RAVEN_HOME names the
other home, as a stored row's env merged last would, gets '' from
inherit_llm, and with the host's home it inherits. 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.

Stated, not filed -- 7c82d3f54 takes the readers in files this PR
edits: inherit_plugin_opt_outs in product_render.py, and the
agent-home comments in the raven-design and raven-ppt launchers, which
also said the launcher inherits RAVEN_HOME. All three are docstring
or comment changes (AST identical). The readers outside this PR's
files -- role.py, CONTEXT.md twice, and docs-site/docs/oncall.md
with its Chinese twin -- predate it and are left for a separate change,
as the description now says. Agreed on the rest: the relative
RAVEN_HOME, getattr hiding absence only, the lockstep guard being
the design, and the stamp sentence being narrow but true.

Six files: 382 passed, 2 skipped (no new cases; one test asserts more).
Full suite: 2 failed, 27580 passed, 115 skipped, the same two failures as
at the base. At the new head every construct this PR adds,
deleted one at a time, turns the six-file run red, each launcher's
parentheses included.

@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 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.

@0xKT

0xKT commented Oct 6, 2026

Copy link
Copy Markdown
Member

Not a blocker. Round four on 7c82d3f5, and this one has nothing to file. All three

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.
items from round three are closed, and I proved each rather than reading the green.

The parenthesised reason is now pinned, per launcher. The mutant that survived nine test
files at 561 passed last round -- dropping the parentheses in the four non-design launchers
-- now gives 4 failed, 103 passed, exactly one failure per parametrized case, with
raven-design (which I deliberately left alone) staying green as the control that those four
reds are about the four I mutated. Mutating only raven-design gives 1 failed, 106 passed.
Both new regexes bite.

The completeness claim and the stale sentences are gone. in three cases and
in two cases return nothing. git grep "launcher inherits" now matches only claims about
the provider block and the image section -- a different and correct claim -- with the
RAVEN_HOME one removed from inherit_plugin_opt_outs and both launcher comments. The control
that these queries find things at all: RAVEN_HOME still matches in 23 files.

The delta changes no executable statement, measured rather than eyeballed: with every
docstring stripped, vendored_agents.py, product_render.py and the two launchers parse to
identical ASTs at 84cd85ed and 7c82d3f5, while tests/test_config_product_render.py --
the control -- differs. Comments never reach the AST, so the two comment edits are covered by
the same comparison.

Gates: gates.sh ruff/lint-imports/commit/large/lang rc=0; the four touched test files
327 passed; lint-imports 10 kept, 0 broken over 902 files. The guard set reports
1 failed, 173 passed; that one is
test_updates_install_guard.py::TestTheMarker::test_writing_it_stays_inside_the_kernel, which
is my board's own environment and not yours -- it spawns sys.executable -I, isolated mode
discards PYTHONPATH, and this board deliberately runs raven off PYTHONPATH rather than
installing it, so the child cannot import raven. Same single red at the base, and the
mechanism is written down in my own known-reds list.

Two corrections to things I said in earlier rounds, since they were mine and not yours: this
tree carries ten import contracts rather than the eight I had in my head, and CONTEXT.md
names four entrances, not six. Neither changes any measurement above.

The three pre-existing readers of the old unconditional claim I listed last round
(role.py:16, CONTEXT.md:284 and :382, docs-site/docs/oncall.md:54-59) are still
there and still not this PR's -- this PR has now swept every reader it touches.

@LivXue
LivXue requested a review from ZuyiZhou October 8, 2026 08:50
@ZuyiZhou
ZuyiZhou merged commit 2ab0658 into main Oct 8, 2026
23 checks passed
@ZuyiZhou
ZuyiZhou deleted the fix/agents_oauth_inheritance_parity branch October 8, 2026 08:54
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.

4 participants