Skip to content

refactor(auth): extract auth errors and telemetry (S1) - #80673

Open
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:gfg/auth-s1-20260806-ship
Open

refactor(auth): extract auth errors and telemetry (S1)#80673
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:gfg/auth-s1-20260806-ship

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What changed

Extract the S1 auth errors and OAuth telemetry cluster from hermes_cli/auth.py into hermes_cli/auth_errors.py.

  • Exact pinned source window: auth.py lines 871–993.
  • Pure extraction with zero behavior change.
  • hermes_cli.auth keeps the legacy names, object identity, monkeypatch paths, lazy imports, and deletion semantics.
  • The source module shrinks; no tests or unrelated files are changed.

This is one independently reviewable slice of the auth.py god-file . The remaining auth.py slices are tracked separately under the same file issue and are not included here.

Verification

Base: origin/main 0957277f2f468bac22bbfcfa7c43029858c9597e.

  • Two independent pin-based W3 witnesses agreed: exact byte/AST fidelity, zero orphans, identity, monkeypatch/delattr seams, importability, and reverse-apply all pass.
  • Current-main ship worktree commit: 56c41562bb8437131ec42c122e303ed1a09ffe4f.
  • Changed files: hermes_cli/auth.py, hermes_cli/auth_errors.py only.
  • git diff --check: clean.
  • py_compile: both changed Python modules pass.
  • Focused auth/credential suites: 259 passed, 4 skipped, 1 deselected, 0 failed.
  • The deselected test is the known Windows mode-bit assertion; four POSIX-only mode tests skip on Windows.

Why this matters to users

The extraction reduces the auth god-file without changing the public or monkeypatch-visible hermes_cli.auth contract. Existing callers and tests continue to resolve the same names and function/class identities while the code becomes independently maintainable.

Links

Part of #78647
Part of #78637

Scope note

This refactor slice is intentionally limited to the S1 cluster. It does not claim the auth god-file is ; the remaining slices stay tracked by #78637 and the tracker ledger.

Credit

The shipped commit is authored and DCO-signed by andrexibiza <84248988+andrexibiza@users.noreply.github.com>.

Interlock

The per-file and its scoreboard live on #78637. This PR is bound to both the Feature Package tracker and the per-file tracker with separate Part of lines above.

Part of #78647
Part of #78637

Signed-off-by: andrexibiza <84248988+andrexibiza@users.noreply.github.com>
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 7, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Nice extraction — moving the error types/formatting/telemetry into hermes_cli/auth_errors.py is the right shape for the S1 split. One substantive concern:

1. _bind_auth_error_function rebuilds moved functions with types.FunctionType(implementation.__code__, globals(), ...), rebinding their __globals__ to auth.py (≈ lines 875-905). This works today only because auth.py still happens to define every module-level name the moved functions touch — hashlib, json, os, logger, AuthError, CODEX_RATE_LIMITED_CODE — in its own namespace. A future import cleanup in auth.py (e.g. dropping a now-"unused" import hashlib) would turn any of those lookups into a runtime NameError inside a function that lives in another module, with no import-time failure and no obvious blame. Prefer the boring options: plain re-export (from hermes_cli.auth_errors import AuthError, CODEX_RATE_LIMITED_CODE, is_rate_limited_auth_error, format_auth_error, ...) or a module-level __getattr__ delegating to _auth_errors for the private helpers.

2. Scope the facade to what is actually imported externally. I grepped the tree: is_rate_limited_auth_error (gateway/run.py), format_auth_error (hermes_cli/main.py) and _token_fingerprint (agent/auxiliary_client.py, agent/secret_sources/bitwarden.py) are genuinely consumed via from hermes_cli.auth import .... _parse_retry_after_seconds, _format_nous_entitlement_auth_error, _oauth_trace / _oauth_trace_enabled appear to be used only inside auth.py itself — those call sites can reference _auth_errors.<name> directly instead of going through the FunctionType loop, shrinking the hacky surface to the public names that actually need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants