Skip to content

fix/api service test prefer login probe#152

Merged
xiami762 merged 2 commits intomainfrom
fix/api-service-test-prefer-login-probe
Apr 21, 2026
Merged

fix/api service test prefer login probe#152
xiami762 merged 2 commits intomainfrom
fix/api-service-test-prefer-login-probe

Conversation

@duguwanglong
Copy link
Copy Markdown
Contributor

ix(server/routes/provider): prefer login probe when testing API service

The API-service connectivity test (test_provider_credentials) used to
sort tools only by name keywords (ip/url/scan/query/domain)
and fall back to the required-parameter count. For services like
Qingteng whose tools are all named qingteng_*, the sort collapsed to
parameter count alone, so action-dispatch tools (e.g. qingteng_assets,
qingteng_asset_discovery) were tried first. Their JSON schema only
marks action as required, but the handler then validates each action
needs additional fields (assets.refresh requires resource/os_type,
assetdiscovery.job_execute requires specId, etc.), so every probe
failed with Missing required parameters for assets.refresh: ....

Prefer parameter-free login probes (*_login, *_whoami, *_ping,
*_health) by giving them the highest sort priority, gated by
requires_confirmation == False and zero required params to avoid
misclassifying write tools that happen to contain login in the name.
This makes the existing qingteng_login (and similar Skyeye/OneSEC
auth probes) the first thing tried, which exercises the full
credential pipeline without invoking business APIs.

Also aggregate every failed attempt into the response under a new
attempts field and a single-line summary in message, so the WebUI
and logs can show why each probe failed instead of only the very last
attempt's error. Add a structured warning log per failed attempt for
operational visibility.

@duguwanglong duguwanglong requested a review from xiami762 April 21, 2026 03:27
The API-service connectivity test (`test_provider_credentials`) used to
sort tools only by name keywords (`ip`/`url`/`scan`/`query`/`domain`)
and fall back to the required-parameter count. For services like
Qingteng whose tools are all named `qingteng_*`, the sort collapsed to
parameter count alone, so action-dispatch tools (e.g. `qingteng_assets`,
`qingteng_asset_discovery`) were tried first. Their JSON schema only
marks `action` as required, but the handler then validates each action
needs additional fields (`assets.refresh` requires `resource`/`os_type`,
`assetdiscovery.job_execute` requires `specId`, etc.), so every probe
failed with `Missing required parameters for assets.refresh: ...`.

Prefer parameter-free login probes (`*_login`, `*_whoami`, `*_ping`,
`*_health`) by giving them the highest sort priority, gated by
`requires_confirmation == False` and zero required params to avoid
misclassifying write tools that happen to contain `login` in the name.
This makes the existing `qingteng_login` (and similar Skyeye/OneSEC
auth probes) the first thing tried, which exercises the full
credential pipeline without invoking business APIs.

Also aggregate every failed attempt into the response under a new
`attempts` field and a single-line summary in `message`, so the WebUI
and logs can show why each probe failed instead of only the very last
attempt's error. Add a structured warning log per failed attempt for
operational visibility.

Made-with: Cursor
@duguwanglong duguwanglong force-pushed the fix/api-service-test-prefer-login-probe branch from 94705ff to 3c23870 Compare April 21, 2026 03:34
Address PR #152 review: the previous `any(kw in name)` check would
flag any tool whose name merely contains "login" / "ping" / etc. as a
parameter-free login probe. Concretely, TDP business endpoints
`tdp_login_api_list` and `tdp_login_weakpwd_list` (both
`requires_confirmation=False`, all-optional params) were promoted
above lightweight query tools like `tdp_ip_query`, defeating the
original goal — those endpoints actually run business queries and
should not be the first tool tried for a credential check.

Restrict the match to either the bare keyword (e.g. `login`) or the
conventional `<provider>_<keyword>` suffix (e.g. `qingteng_login`),
so genuine probes are still picked first while business tools fall
back to the existing ip/url/scan/query priority ladder.

Add `test_login_probe_does_not_overmatch_business_tools` as a
regression: with `tdp_login_api_list` and `tdp_ip_query` both
available, the connectivity test must now select `tdp_ip_query`.

Note: surfacing the `attempts` array in the WebUI/status cache (the
second review point) is intentionally left as a follow-up — the
aggregated `message` already gives the user a single-glance reason
for failure, and the structured field is exposed to API consumers.

Made-with: Cursor
@xiami762 xiami762 merged commit 4bc6784 into main Apr 21, 2026
2 checks passed
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.

2 participants