Skip to content

test(hermes-cli): isolate Qwen OAuth fallthrough test from real credential pool - #77149

Closed
JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/qwen-oauth-fallthrough-test-pool-isolation
Closed

test(hermes-cli): isolate Qwen OAuth fallthrough test from real credential pool#77149
JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/qwen-oauth-fallthrough-test-pool-isolation

Conversation

@JoaoMarcos44

Copy link
Copy Markdown
Contributor

Summary

  • Fixes Qwen OAuth auto-fallthrough test depends on the local machine's real credential pool #77148: test_qwen_oauth_auto_fallthrough_on_auth_failure mocked resolve_provider and resolve_qwen_runtime_credentials but not load_pool. resolve_runtime_provider() checks the credential pool (runtime_provider.py:1852) and can return early via a pool entry (runtime_provider.py:1910) before it ever reaches the qwen-oauth singleton block (runtime_provider.py:1981) — so on a machine with a real Qwen OAuth pool entry, the mocked AuthError never fires and the test's outcome depends on the local auth profile instead of the fallthrough logic it's meant to cover.
  • Mocks load_pool to report no credentials, matching the pattern two sibling tests in the same file already use.
  • Adds an assertion that the mocked resolve_qwen_runtime_credentials was actually invoked, proving the fallthrough branch under test executed rather than just checking the final provider differs.

Root cause diagram

%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#00f0ff', 'mainBkg': '#0a0a16', 'primaryTextColor': '#ffffff', 'primaryBorderColor': '#ff007f', 'lineColor': '#00f0ff'}}}%%
flowchart TD
    A["resolve_runtime_provider(requested='auto')"] --> B["load_pool('qwen-oauth')"]
    B --> C{"Real pool entry present on this machine?"}
    C -- "Yes (before fix: unmocked)" --> D["Early return via _resolve_runtime_from_pool_entry"]
    D --> E["Mocked resolve_qwen_runtime_credentials NEVER called"]
    E --> F["Test outcome depends on local auth profile"]
    C -- "No (after fix: load_pool mocked)" --> G["Falls through to qwen-oauth singleton block"]
    G --> H["resolve_qwen_runtime_credentials() raises mocked AuthError"]
    H --> I["requested == 'auto' -> falls through to OpenRouter"]
    I --> J["Test verifies the actual fallthrough logic"]
Loading

Test plan

  • pytest tests/hermes_cli/test_runtime_provider_resolution.py -q — 55 passed.
  • Traced resolve_runtime_provider() end-to-end to confirm the pool-check-and-early-return block executes unconditionally before the qwen-oauth block, and that the fix's added assertion (fallback_creds_calls) would fail if the pool short-circuited the fallthrough path.

test_qwen_oauth_auto_fallthrough_on_auth_failure mocked
resolve_provider and resolve_qwen_runtime_credentials but not
load_pool. resolve_runtime_provider() checks the credential pool
before it ever reaches the qwen-oauth singleton block, so on a
machine with a real qwen-oauth pool entry the unmocked pool path
returns early with real credentials, the mocked AuthError never
fires, and the test's pass/fail depends on the local auth profile
instead of the fallthrough logic it's meant to cover.

Mock load_pool to report no credentials (matching the pattern already
used by the other tests in this file) and assert the fallback creds
mock actually ran, proving the target branch executed.

Fixes #HPA-03
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure comp/cli CLI entry point, hermes_cli/, setup wizard provider/qwen Qwen / Alibaba Cloud (OAuth) area/auth Authentication, OAuth, credential pools P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Aug 2, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #67465: its open broader test-only patch already forces an empty Qwen pool for both singleton and auto-fallthrough coverage. This patch adds a useful invocation assertion but implements the same isolation mechanism. #77148 is the related report.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Three PRs address the same Qwen OAuth test-isolation defect: an ambient credential pool can return before the mocked singleton resolver runs. #69608 patches two historical tests, #75235 isolates the retained fallthrough test, and #77149 adds the same isolation plus an assertion proving that the intended resolver path executed.

Related pull requests

Duplicates

#69608, #75235, and #77149 implement the same empty-Qwen-pool isolation; #69608 also includes a now-stale test hunk, while #77149 uniquely adds explicit invocation verification. Contributor triage further identifies all three mechanisms as overlapping #67465.

Suggested consolidation

Keep #77149 open with a salvage path limited to the retained fallthrough test's empty-pool stub and invocation assertion. Close #75235 as a duplicate of #77149, and close #69608 as superseded by #77149 despite its earlier best-fix and keep_open assessments, because commit 6b81590 removed its other target and #77149 preserves the remaining fix with stronger verification; reconcile the shared mechanism with #67465 before landing any version.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I69470(["issue #69470 (open)"])
    I77148(["issue #77148 (open)"])
    subgraph Dup69608 ["PRs duplicating each other"]
        P69608["PR #69608 (open)"]
        P75235["PR #75235 (open)"]
        P77149["PR #77149 (open)"]
    end
    P77149 -.->|partial| I69470
    P77149 -->|best fix| I77148
    class I69470 open
    class I77148 open
    class P69608 open
    class P75235 open
    class P77149 open
    class P69608 best
    class P77149 best
    class P77149 target
    click I69470 "https://github.com/NousResearch/hermes-agent/issues/69470"
    click I77148 "https://github.com/NousResearch/hermes-agent/issues/77148"
    click P69608 "https://github.com/NousResearch/hermes-agent/pull/69608"
    click P75235 "https://github.com/NousResearch/hermes-agent/pull/75235"
    click P77149 "https://github.com/NousResearch/hermes-agent/pull/77149"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 4 kB of PR diffs, 14 kB of issue/PR text, 3 kB of discussion (6 comments), 9 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have provider/qwen Qwen / Alibaba Cloud (OAuth) type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Qwen OAuth auto-fallthrough test depends on the local machine's real credential pool

3 participants