Skip to content

fix(skill_hub): cap the catalog search query string at the hub edge limit - #348

Open
LivXue wants to merge 1 commit into
EverMind-AI:mainfrom
LivXue:fix/skill_hub_query_length_cap
Open

fix(skill_hub): cap the catalog search query string at the hub edge limit#348
LivXue wants to merge 1 commit into
EverMind-AI:mainfrom
LivXue:fix/skill_hub_query_length_cap

Conversation

@LivXue

@LivXue LivXue commented Aug 18, 2026

Copy link
Copy Markdown

Summary

SkillHubClient.search put the caller's query into the q parameter verbatim. The
deployed hub is fronted by a load balancer (server: awselb/2.0) that answers
403 Forbidden -- not 414 -- once a request's query string passes 2048 bytes, and q
carries a retrieval query that can be as long as a user's whole message. So a long
prompt did not search on a prefix; it lost hub discovery outright. The failure is quiet:
SkillForgeRouter._safe_search turns a source error into an empty candidate list, so the
only symptom is one warning line and skills silently never being found.

Non-ASCII made the real ceiling far lower than it looks. One CJK character costs 9 bytes
encoded (three UTF-8 bytes, three %XX characters each), so the budget was about 226
characters of Chinese -- under which essentially every non-trivial task ran with no hub
candidates at all.

The fix trims q to the longest prefix that fits, measured in encoded width rather than
character count, and sizes its budget from what the other params leave over rather than
assuming q is alone. It also logs what was dropped, so a trimmed query does not read as
a whole one. Measured boundary against the deployed hub: a 2048-byte query string is
served, 2049 is refused.

Two decisions worth naming, so the next reader does not re-derive them:

  • The cap lives in the client, not the caller. The limit is a property of this endpoint's
    wire contract, and the client is the one place every caller passes through.
  • Trimming is per character, not a slice of the encoded form. Slicing the encoding would
    cut a %XX escape in half and produce invalid UTF-8.

Type

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

Verification

Reproduced against the deployed hub before the fix, bisecting the boundary: a query
string of 2048 bytes returns 200, 2049 returns 403, with server: awselb/2.0 on the
refusal. In CJK terms, 226 characters pass (2045 bytes) and 227 fail (2054 bytes).

End-to-end through the real client, on the prompt shape that produced the original 403
(an 860-character mixed Chinese/ASCII query):

DEBUG raven.skill_hub.client: hub search query trimmed to fit the 2048-byte
      query-string cap (366 of 860 characters kept)
RESULT: HTTP 200, 10 items returned

The same query untrimmed is a 4951-byte query string and returns 403.

uv run pytest tests/test_skill_hub_source.py tests/test_skill_hub_client.py \
  tests/test_skill_hub_tools.py tests/test_skill_hub_policy.py \
  tests/test_skill_segment_builder.py -q
-> 97 passed

Full suite, and the three new tests proved not to be theatre by reverting only the
client change and watching them go red:

uv run pytest -q -p no:randomly
-> 72 failed, 6572 passed, 36 skipped, 13 deselected

# same command on the base commit, failing test IDs diffed:
-> 72 failed; identical set. 0 introduced, 0 fixed by accident.

# revert client.py only, keep the tests:
-> 3 failed (the 3 new ones), 2 passed

The 72 failures are pre-existing on the base commit and unrelated to this change
(test_tui_rpc_session.py, test_cli_cron_commands.py, test_cli_import_commands.py,
test_cli_onboard_commands.py, and four single-test files).

uv run ruff check raven/skill_hub/client.py tests/test_skill_hub_source.py
-> All checks passed!
uv run ruff format --check raven/skill_hub/client.py tests/test_skill_hub_source.py
-> 2 files already formatted
  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

Disclosed gaps: make lint also runs lint-tui, which could not run here because
eslint is not installed in this checkout; this change is Python-only and touches no
TypeScript. mypy is not part of this project's dev dependencies, so no type check beyond
ruff was available. No docs changed: nothing in docs/ stated a length contract for this
parameter, so no text went stale.

Risk

Behaviour change: a query whose encoded form exceeds the budget is now truncated instead
of being sent whole. Before this change such a request was refused outright and the
candidate list came back empty, so a truncated query is strictly more useful than the
status quo -- there is no case that previously worked and now returns less. Queries inside
the budget are byte-identical to before, which the pre-existing envelope test pins.

The parameter order in the emitted URL changes (q now serialises after category and
sort) because the budget is computed from what the other params consume. Query
parameters are order-independent on this endpoint.

Rollback is reverting the commit; there is no migration, no persisted state, and no
config surface involved.

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Related Issues

N/A

…imit

The hub is fronted by a load balancer that answers 403 Forbidden once a
request's query string passes 2048 bytes. search() put the caller's query
into q verbatim, and that query carries a retrieval query as long as a
user's whole message, so a long prompt did not search on a prefix, it lost
hub discovery outright: the router swallows a source failure into an empty
candidate list, leaving one warning line as the only symptom.

Non-ASCII made the real ceiling far lower than it looks. One CJK character
costs 9 bytes encoded (three UTF-8 bytes, three characters each), so the
budget was about 226 characters of Chinese, under which essentially every
non-trivial task ran with no hub candidates at all.

Trim q to the longest prefix that fits, measured in encoded width rather
than character count, and size its budget from what the other params leave
over. Log what was dropped so a trimmed query does not read as a whole one.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
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.

1 participant