@claude review

Claude finished @axisrow's task in 2m 51s —— View job

Review: Credential precedence & env token save

• Read the PR diff against origin/main
• Review direct_cli/auth.py credential precedence changes
• Review direct_cli/commands/auth.py status/save changes
• Verify against CLAUDE.md credential-priority contract
• Check tests cover the new contract

Overall this is a clean, well-tested change. The refactor of get_credentials into _resolve_profile_credentials / _resolve_profile_or_raise is a nice readability win, docs/translations/CLAUDE.md are all kept in sync, and the new tests cover both precedence directions plus the .env-save prompt (yes / no / stdin-no-prompt) with a permission assertion. A few things worth addressing:

1. save_env_credentials doesn't tighten perms on an existing .env (security, low–medium)

direct_cli/auth.py:187-189

dotenv_path.write_text("".join(updated_lines), encoding="utf-8")
if not existed:
    os.chmod(dotenv_path, 0o600)

chmod 0o600 only runs when the file is newly created. If a .env already exists with looser permissions (e.g. 0644 from an editor or a prior write_text), the OAuth access token gets written into a potentially world-readable file and the permissions are left untouched. Since the whole point of this prompt is to persist a live token, I'd tighten unconditionally (or at least warn if the existing mode is broader than 0o600):

dotenv_path.write_text("".join(updated_lines), encoding="utf-8")
os.chmod(dotenv_path, 0o600)

Fix this →

A secondary, lower-severity point: the create path does write_text (default umask, often 0o644) and then chmod, leaving a brief window where the token sits with umask perms. Using os.open(path, O_WRONLY|O_CREAT|O_TRUNC, 0o600) would close that window, though for a local CLI the practical risk is small.

2. Base env token without a login no longer borrows the active profile's login (behavioral, intended?)

direct_cli/auth.py:857-870

The env-token branch only reads YANDEX_DIRECT_LOGIN when YANDEX_DIRECT_TOKEN is present, and once final_token is set the active-profile branch is skipped entirely. So with base YANDEX_DIRECT_TOKEN set but no base login, plus an active profile that has a login, the command runs with login=None rather than the profile's login. That's consistent with the "base env is self-contained, don't mix accounts" philosophy, and arguably the safest behavior — just confirming it's deliberate, since for agency tokens a missing login changes API results.

3. Headline behavior change worth a CHANGELOG/BREAKING note

Base env / cwd .env now wins over an active OAuth profile (direct auth use). A user with a leftover YANDEX_DIRECT_TOKEN exported in their shell will now silently override a profile they explicitly activated — the reverse of prior behavior. The PR documents this in README/CLAUDE.md (good), but per the repo's convention around precedence/breaking changes I'd add a CHANGELOG.md entry and consider a version bump so upgraders aren't surprised.

4. auth status "effective source" is narrower than get_credentials (minor consistency)

direct_cli/commands/auth.py:267-283

status (no --profile) reports source="env" if base env is set, else falls back to the active profile. But get_credentials also resolves 1Password / Bitwarden refs and profile-specific env vars. So in an op/bw-only setup, auth status prints No active profile. while a real command resolves fine — the reported source can disagree with what's actually used. Not blocking (status has always been profile-centric), but the command name now implies "effective credentials," so it's a slightly bigger gap than before.

5. README priority table dropped the profile-specific env-var row (doc nit)

The old table had an explicit YANDEX_DIRECT_TOKEN_AGENCY1 row. It's now folded implicitly into rows 2/4 (still works via get_env_profile), but a reader skimming the table loses the discoverability of that feature. Consider a footnote noting profile-specific env vars resolve as part of the selected/active profile.

Nits

• _format_dotenv_value("") returns '""' — harmless since token is never empty, but worth a guard if you want to reject empty tokens explicitly.

Nothing here is a correctness blocker for the happy path; #1 is the one I'd recommend fixing before merge.
• codex/env-precedence-oauth-dotenv

@codex review

@codex review

@codex review

Codex Review: Didn't find any major issues. Hooray!