Skip to content

fix(tests): pin rebuild budget in issue2513 custom-provider catalog test - #7054

Closed
carlotestor wants to merge 1 commit into
nesquena:masterfrom
carlotestor:fix/pin-issue2513-catalog-budget
Closed

fix(tests): pin rebuild budget in issue2513 custom-provider catalog test#7054
carlotestor wants to merge 1 commit into
nesquena:masterfrom
carlotestor:fix/pin-issue2513-catalog-budget

Conversation

@carlotestor

Copy link
Copy Markdown
Contributor

Summary

tests/test_issue2513_custom_provider_remote_models.py::test_custom_provider_model_field_does_not_block_remote_catalog is a pre-existing wall-clock flake. On a starved CI shard the cold provider-catalog rebuild overruns _LIVE_REBUILD_BUDGET_SECONDS (default 4s), get_available_models() serves the degraded fallback catalog, and the monkeypatched live model alpha/remote is dropped — leaving only the config-declared sticky model.

Seen on #7051 as the only red check (test (3.13, 4)):

assert "@custom:alpha-proxy:alpha/remote" in alpha_ids
AssertionError: assert '@custom:alpha-proxy:alpha/remote' in {'alpha/sticky'}
WARNING  api.config:config.py:8355 live provider-catalog rebuild exceeded 4.0s budget

This is not a session-sidebar regression. Split out of #7051 so that PR stays scoped to the subagent parent-in-window fix.

Fix

One test-only pin, matching tests/test_issue2540_models_endpoint_error.py:

monkeypatch.setattr(config, "_LIVE_REBUILD_BUDGET_SECONDS", 0.0, raising=False)

budget <= 0 takes the synchronous unbounded rebuild path, so the test asserts the catalog contract instead of racing the 4s budget.

No production code.

Test plan

  • Unpatched at a hostile budget reproduces the CI assertion
  • Patched passes at budget 0, 0.001s, and default
  • CI test (3.13, *) on this PR

test (3.13, 4) failed on this PR with:

  assert "@Custom:alpha-proxy:alpha/remote" in alpha_ids
  E  AssertionError: assert '@Custom:alpha-proxy:alpha/remote' in {'alpha/sticky'}
  WARNING api.config:config.py:8355 live provider-catalog rebuild exceeded
          4.0s budget - serving fallback, refreshing catalog out-of-band

Pre-existing wall-clock flake, not a regression from this PR: this branch
touches only api/agent_sessions.py and tests/test_subagent_parent_in_import_
window.py, and both api/config.py and this test file are byte-identical to
origin/master. The same shard passed on 3.11 and 3.12.

The test never pinned _LIVE_REBUILD_BUDGET_SECONDS, so it raced the global
4s budget in get_available_models(). On a starved runner the cold rebuild
overruns, the degraded fallback catalog is served, and the monkeypatched-
urlopen model alpha/remote is dropped - leaving only the config-declared
sticky model, exactly as CI observed.

Force the synchronous (unbounded) rebuild path, matching the existing
precedent in tests/test_issue2540_models_endpoint_error.py:20-24.

Verified: with the budget forced to 0.001s the unpatched test reproduces
the CI assertion verbatim; patched it passes at 0.001s, 0, and default.
@greptile-apps

greptile-apps Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This test-only change pins the provider-catalog rebuild budget to the synchronous unbounded path, preventing loaded CI shards from serving a degraded fallback during the custom-provider catalog assertion.

  • Sets _LIVE_REBUILD_BUDGET_SECONDS to 0.0 within the test.
  • Documents why the test should not exercise the separate asynchronous budget-fallback behavior.

Confidence Score: 5/5

The PR appears safe to merge because the test-local override reaches the intended synchronous rebuild branch and is automatically restored by the monkeypatch fixture.

The changed test pins an existing configuration variable that directly controls the exercised cold catalog rebuild, eliminating the documented timing race without altering runtime code.

Important Files Changed

Filename Overview
tests/test_issue2513_custom_provider_remote_models.py Deterministically disables the rebuild timeout for this catalog-contract test without changing production behavior.

Reviews (1): Last reviewed commit: "fix(tests): pin rebuild budget in issue2..." | Re-trigger Greptile

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

🔬 Exact-head gate — SAFE TO SHIP (queued for maintainer merge)

Thanks @carlotestor — correct flake-hardening. test_custom_provider_model_field_does_not_block_remote_catalog pins the custom-provider catalog contract, but on a slow/loaded CI shard the global 4s live-rebuild budget (_LIVE_REBUILD_BUDGET_SECONDS) could be exceeded, so the async fallback served the degraded catalog (last-known cache / minimal) — dropping the live-fetched models under test and leaving only the config-declared sticky model → spurious failure. Pinning the budget to 0.0 forces the synchronous rebuild path so the test deterministically exercises the real contract.

Gate — SAFE TO SHIP:

  • Test-only (zero product code; tests/test_issue2513_custom_provider_remote_models.py +6/-0), CI 20/20 green, full suite 14,579 passed.
  • Non-masking: _LIVE_REBUILD_BUDGET_SECONDS=0 is the officially-supported "legacy synchronous (unbounded)" mode (HERMES_WEBUI_MODELS_REBUILD_BUDGET=0), not a bypass — it forces the real custom-provider live-catalog contract rather than racing the budget. The test's assertions (Alpha/Beta proxy models must appear) are unchanged and still non-vacuous; the separate async-budget-fallback path keeps its own coverage.

Tight, additive, test-determinism only, zero product risk. Queuing gate-pass for a maintainer merge nod.

(Release-manager overnight gate — queued for maintainer merge, not autonomously tagged.)

@nesquena-hermes nesquena-hermes added gate-pass Full gate passed (Codex+Opus+suite+browser); queued Tier 1 for release agent merge-soon size:S Small PR (≤2 files, ≤30 LOC) labels Aug 15, 2026
nesquena-hermes added a commit that referenced this pull request Aug 16, 2026
* fix(goal): suppress reserved SILENT sentinel at /api/goal ingress

A wake relay POSTing the exact [SILENT] suppression sentinel to
/api/goal lets it reach _start_chat_stream_for_session, which persists
it as pending_user_message. If 8701 restarts while the turn is pending,
session recovery materializes it as a visible _recovered user turn —
the same phantom-recovered-turn exposure #7018 closed for chat ingress.

Apply the shared _is_silent_control_message() guard immediately after
/api/goal session-ID validation and before session lookup or goal-state
mutation, returning the same 200 no-op. Matching stays exact and
case-sensitive; ordinary kickoff text is unaffected.

Add tests mirroring test_silent_control_suppression.py for the goal
path (args and text fields, before-lookup suppression, exact-match
semantics).

Closes #7019

* fix(sessions): keep evicted subagent parents in the import window

The sidebar nests a subagent row under its parent only when the parent row
is present in the same payload. The visible-window limit was applied as a
flat per-row recency slice, so a frozen orchestrator (which stops writing
while its leaves keep streaming) lost the recency race against its own
leaves and fell outside the window -- promoting those leaves to top-level
sidebar rows.

Re-add subagent parents that the oversampled candidate set already
projected, after the slice. No extra queries, no change to
CLI_VISIBLE_SESSION_LIMIT. webui ancestors are deliberately not imported
because that sidebar bucket already owns them.

Supersedes #7031.

* fix(sessions): document and pin the parent-recovery bound (greptile review)

Greptile flagged (P1) that a selected subagent child whose parent ranks below
the limit * 8 oversample is still promoted to a top-level row. That is real and
measured (the parent drops out at candidate #25 of 24), but it is the bound of
the design, not a regression: the walk reuses rows the projection already
fetched and never issues an extra query. Resolving arbitrarily old ancestors
needs an unbounded per-row lookup on the hot sidebar path -- the approach
rejected in #7031 -- so the bound is documented and pinned instead.

- Docstring: state that `limit` bounds the recency slice, not the row count,
  so callers must iterate rather than assume len(rows) <= limit; state that
  recovery is bounded by the oversampled candidate set.
- Inline comment: mark the bound at the exact lines Greptile flagged and point
  at `candidate_limit` as the knob if the window proves too tight.
- Tests: cover the candidate-window exhaustion Greptile said was untested --
  parent inside the oversample is recovered, parent beyond it stays unresolved
  -- plus the over-limit return contract and a parent-cycle guard.

No behaviour change; 7 tests pass.

* fix(share): add _hadAppearance guard to prevent fabricated appearance choice

Summary:
The inline appearance bootstrap in static/share.html writes hermes-theme and
hermes-skin to localStorage unconditionally on every page load, even when the
browser had no prior appearance state. This fabricates an explicit user choice
on first access via a shared link, making the server-side SETTINGS_DEFAULTS
unreachable for deployments that customise the default theme or skin.

Root Cause:
share.html:9 — the boot IIFE resolves a theme+skin and calls
localStorage.setItem() without guarding on whether the user had previously
chosen an appearance. The same bug was fixed in index.html by PR #6808
(commit tomtong2015) but share.html was left unchanged.

Change:
1. Added _hadAppearance guard before the two localStorage.setItem() calls:
   var _hadAppearance = localStorage.getItem('hermes-theme') !== null ||
                        localStorage.getItem('hermes-skin') !== null;
   if (_hadAppearance) { setItem('hermes-theme', t); setItem('hermes-skin', s); }
2. The first-paint DOM mutations (classList.add('dark'), dataset.skin) remain
   outside the guard — only persistence is protected.
3. Synced the skin allowlist with index.html: added neon-soft and neon-paint
   (zeus and verdigris were already present).

Verification:
- test_6808_appearance_bootstrap_no_fabricated_choice.py: 11/11 passed
  covering fresh-browser (no writes), pre-paint fallback, explicit state
  normalisation, and legacy migration survival.

Closes #7030

* fix(tests): pin rebuild budget in issue2513 custom-provider catalog test

test (3.13, 4) failed on this PR with:

  assert "@Custom:alpha-proxy:alpha/remote" in alpha_ids
  E  AssertionError: assert '@Custom:alpha-proxy:alpha/remote' in {'alpha/sticky'}
  WARNING api.config:config.py:8355 live provider-catalog rebuild exceeded
          4.0s budget - serving fallback, refreshing catalog out-of-band

Pre-existing wall-clock flake, not a regression from this PR: this branch
touches only api/agent_sessions.py and tests/test_subagent_parent_in_import_
window.py, and both api/config.py and this test file are byte-identical to
origin/master. The same shard passed on 3.11 and 3.12.

The test never pinned _LIVE_REBUILD_BUDGET_SECONDS, so it raced the global
4s budget in get_available_models(). On a starved runner the cold rebuild
overruns, the degraded fallback catalog is served, and the monkeypatched-
urlopen model alpha/remote is dropped - leaving only the config-declared
sticky model, exactly as CI observed.

Force the synchronous (unbounded) rebuild path, matching the existing
precedent in tests/test_issue2540_models_endpoint_error.py:20-24.

Verified: with the budget forced to 0.001s the unpatched test reproduces
the CI assertion verbatim; patched it passes at 0.001s, 0, and default.

* fix(#7013): preserve media deny coverage across platforms

* fix(docker): keep repository agents out of runtime context

* fix(docker): gate image runtime proof behind integration job

* test(models): stabilize custom provider catalog regression

* Release batch A: 6 low-risk gate-passed fixes (experimental)

Batched contributor fixes, each individually Codex-gated during the overnight
certifier cycles and re-verified clean-to-ship as a combined stage (Codex SAFE
TO SHIP on the combined diff; full suite green except 8 pre-existing approval
tests that fail identically on clean origin/master — CI green on same commit).

- #7019 (@webtecnica) suppress reserved [SILENT] sentinel at /api/goal
- #7031 (@carlotestor) keep evicted subagent parents in the sidebar import window
- #7030 (@webtecnica) share.html first-visit appearance guard + skin-id sync
- #6853 (@rodboev) exclude repo-root AGENTS.md from the Docker runtime image
- #7013 (@webtecnica) test-only: platform-neutral test portability (playwright
  import guard + media tests served from allowed roots)
- test-only (@carlotestor) pin custom-provider catalog rebuild budget (#7054)

Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
Co-authored-by: carlotestor <carlotestor@users.noreply.github.com>
Co-authored-by: rodboev <rodboev@users.noreply.github.com>

---------

Co-authored-by: webtecnica <webtecnica@gmail.com>
Co-authored-by: carlotestor <carlotestor@users.noreply.github.com>
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Co-authored-by: n <a@n>
Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in experimental release exp-v0.52.229 (batched with 5 other low-risk gate-passed fixes). Merged onto master via the batch release PR #7089 — your commit content is in master and is live on the experimental channel now.

Thanks @carlotestor for the fix.

🤖 Closed by the release-manager agent.

alai04 pushed a commit to alai04/hermes-webui that referenced this pull request Aug 31, 2026
…a#7089)

* fix(goal): suppress reserved SILENT sentinel at /api/goal ingress

A wake relay POSTing the exact [SILENT] suppression sentinel to
/api/goal lets it reach _start_chat_stream_for_session, which persists
it as pending_user_message. If 8701 restarts while the turn is pending,
session recovery materializes it as a visible _recovered user turn —
the same phantom-recovered-turn exposure nesquena#7018 closed for chat ingress.

Apply the shared _is_silent_control_message() guard immediately after
/api/goal session-ID validation and before session lookup or goal-state
mutation, returning the same 200 no-op. Matching stays exact and
case-sensitive; ordinary kickoff text is unaffected.

Add tests mirroring test_silent_control_suppression.py for the goal
path (args and text fields, before-lookup suppression, exact-match
semantics).

Closes nesquena#7019

* fix(sessions): keep evicted subagent parents in the import window

The sidebar nests a subagent row under its parent only when the parent row
is present in the same payload. The visible-window limit was applied as a
flat per-row recency slice, so a frozen orchestrator (which stops writing
while its leaves keep streaming) lost the recency race against its own
leaves and fell outside the window -- promoting those leaves to top-level
sidebar rows.

Re-add subagent parents that the oversampled candidate set already
projected, after the slice. No extra queries, no change to
CLI_VISIBLE_SESSION_LIMIT. webui ancestors are deliberately not imported
because that sidebar bucket already owns them.

Supersedes nesquena#7031.

* fix(sessions): document and pin the parent-recovery bound (greptile review)

Greptile flagged (P1) that a selected subagent child whose parent ranks below
the limit * 8 oversample is still promoted to a top-level row. That is real and
measured (the parent drops out at candidate nesquena#25 of 24), but it is the bound of
the design, not a regression: the walk reuses rows the projection already
fetched and never issues an extra query. Resolving arbitrarily old ancestors
needs an unbounded per-row lookup on the hot sidebar path -- the approach
rejected in nesquena#7031 -- so the bound is documented and pinned instead.

- Docstring: state that `limit` bounds the recency slice, not the row count,
  so callers must iterate rather than assume len(rows) <= limit; state that
  recovery is bounded by the oversampled candidate set.
- Inline comment: mark the bound at the exact lines Greptile flagged and point
  at `candidate_limit` as the knob if the window proves too tight.
- Tests: cover the candidate-window exhaustion Greptile said was untested --
  parent inside the oversample is recovered, parent beyond it stays unresolved
  -- plus the over-limit return contract and a parent-cycle guard.

No behaviour change; 7 tests pass.

* fix(share): add _hadAppearance guard to prevent fabricated appearance choice

Summary:
The inline appearance bootstrap in static/share.html writes hermes-theme and
hermes-skin to localStorage unconditionally on every page load, even when the
browser had no prior appearance state. This fabricates an explicit user choice
on first access via a shared link, making the server-side SETTINGS_DEFAULTS
unreachable for deployments that customise the default theme or skin.

Root Cause:
share.html:9 — the boot IIFE resolves a theme+skin and calls
localStorage.setItem() without guarding on whether the user had previously
chosen an appearance. The same bug was fixed in index.html by PR nesquena#6808
(commit tomtong2015) but share.html was left unchanged.

Change:
1. Added _hadAppearance guard before the two localStorage.setItem() calls:
   var _hadAppearance = localStorage.getItem('hermes-theme') !== null ||
                        localStorage.getItem('hermes-skin') !== null;
   if (_hadAppearance) { setItem('hermes-theme', t); setItem('hermes-skin', s); }
2. The first-paint DOM mutations (classList.add('dark'), dataset.skin) remain
   outside the guard — only persistence is protected.
3. Synced the skin allowlist with index.html: added neon-soft and neon-paint
   (zeus and verdigris were already present).

Verification:
- test_6808_appearance_bootstrap_no_fabricated_choice.py: 11/11 passed
  covering fresh-browser (no writes), pre-paint fallback, explicit state
  normalisation, and legacy migration survival.

Closes nesquena#7030

* fix(tests): pin rebuild budget in issue2513 custom-provider catalog test

test (3.13, 4) failed on this PR with:

  assert "@Custom:alpha-proxy:alpha/remote" in alpha_ids
  E  AssertionError: assert '@Custom:alpha-proxy:alpha/remote' in {'alpha/sticky'}
  WARNING api.config:config.py:8355 live provider-catalog rebuild exceeded
          4.0s budget - serving fallback, refreshing catalog out-of-band

Pre-existing wall-clock flake, not a regression from this PR: this branch
touches only api/agent_sessions.py and tests/test_subagent_parent_in_import_
window.py, and both api/config.py and this test file are byte-identical to
origin/master. The same shard passed on 3.11 and 3.12.

The test never pinned _LIVE_REBUILD_BUDGET_SECONDS, so it raced the global
4s budget in get_available_models(). On a starved runner the cold rebuild
overruns, the degraded fallback catalog is served, and the monkeypatched-
urlopen model alpha/remote is dropped - leaving only the config-declared
sticky model, exactly as CI observed.

Force the synchronous (unbounded) rebuild path, matching the existing
precedent in tests/test_issue2540_models_endpoint_error.py:20-24.

Verified: with the budget forced to 0.001s the unpatched test reproduces
the CI assertion verbatim; patched it passes at 0.001s, 0, and default.

* fix(nesquena#7013): preserve media deny coverage across platforms

* fix(docker): keep repository agents out of runtime context

* fix(docker): gate image runtime proof behind integration job

* test(models): stabilize custom provider catalog regression

* Release batch A: 6 low-risk gate-passed fixes (experimental)

Batched contributor fixes, each individually Codex-gated during the overnight
certifier cycles and re-verified clean-to-ship as a combined stage (Codex SAFE
TO SHIP on the combined diff; full suite green except 8 pre-existing approval
tests that fail identically on clean origin/master — CI green on same commit).

- nesquena#7019 (@webtecnica) suppress reserved [SILENT] sentinel at /api/goal
- nesquena#7031 (@carlotestor) keep evicted subagent parents in the sidebar import window
- nesquena#7030 (@webtecnica) share.html first-visit appearance guard + skin-id sync
- nesquena#6853 (@rodboev) exclude repo-root AGENTS.md from the Docker runtime image
- nesquena#7013 (@webtecnica) test-only: platform-neutral test portability (playwright
  import guard + media tests served from allowed roots)
- test-only (@carlotestor) pin custom-provider catalog rebuild budget (nesquena#7054)

Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
Co-authored-by: carlotestor <carlotestor@users.noreply.github.com>
Co-authored-by: rodboev <rodboev@users.noreply.github.com>

---------

Co-authored-by: webtecnica <webtecnica@gmail.com>
Co-authored-by: carlotestor <carlotestor@users.noreply.github.com>
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Co-authored-by: n <a@n>
Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate-pass Full gate passed (Codex+Opus+suite+browser); queued Tier 1 for release agent merge-soon size:S Small PR (≤2 files, ≤30 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants