Skip to content

test(discord): fix stale component-view fixtures for fail-closed auth - #41334

Closed
teknium1 wants to merge 2 commits into
mainfrom
fix/discord-model-picker-test-allowlist
Closed

test(discord): fix stale component-view fixtures for fail-closed auth#41334
teknium1 wants to merge 2 commits into
mainfrom
fix/discord-model-picker-test-allowlist

Conversation

@teknium1

@teknium1 teknium1 commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Repairs stale test fixtures that broke main: several Discord component-view tests passed empty allowlists and expected the action to proceed, but the component-button auth hardening (f6f363662) made empty allowlists fail closed.

Root cause: _component_check_auth now returns False when both the user and role allowlists are empty (fail closed). Tests that constructed views with allowed_user_ids=set() and expected the interaction to succeed started getting rejected.

Changes

  • tests/gateway/test_discord_model_picker.py: test_model_picker_clears_controls...allowed_user_ids={"123"} (matches the test interaction's user.id=123).
  • tests/gateway/test_discord_clarify_buttons.py: the two success-path tests (test_choice_falls_back_to_label_text_when_entry_missing, test_other_flips_entry_to_awaiting_text) → allowed_user_ids={"42"} (matches _make_interaction's default user id).

Rejection / already-resolved tests keep their empty allowlists — those paths assert rejection or short-circuit before the auth check, and remain correct.

Validation

Suite Result
test_discord_model_picker.py 1 passed
test_discord_clarify_buttons.py 14 passed
test_discord_component_auth.py 29 passed
each file under per-test isolation green
  • Confirmed all four failures reproduce on clean origin/main (main CI is currently red on this cluster); fix is test-only, 3 lines across 2 files.

Infographic

test-fixture-fail-closed-fix

…closed auth

The component-button auth hardening (f6f3636) made `_component_check_auth`
return False when both allowlists are empty (fail closed). This test passed
`allowed_user_ids=set()` and expected the model switch to proceed, so it
started asserting `events == []` and failed on main.

Give the picker an allowlist containing the test user (id 123) so `_check_auth`
passes, matching the new contract. Verifies the control-clearing order as
intended.
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: fix/discord-model-picker-test-allowlist vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 10002 on HEAD, 10002 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5190 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@daimon-nous daimon-nous Bot added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have platform/discord Discord bot adapter labels Jun 7, 2026
Same fail-closed fallout as the model-picker fixture: the auth hardening
(f6f3636) rejects empty allowlists, so two ClarifyChoiceView tests that
expected the action to proceed (choice fallback when entry missing; Other
flips to awaiting-text) started failing on main.

Give those two views `allowed_user_ids={"42"}` (matching the default test
interaction's user id) so `_check_auth` passes. The rejection/already-
resolved tests keep their empty allowlists — those paths short-circuit
before the auth check or assert rejection, and remain correct.
@teknium1 teknium1 changed the title test(discord): fix stale model-picker fixture for fail-closed component auth test(discord): fix stale component-view fixtures for fail-closed auth Jun 7, 2026
@teknium1

teknium1 commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Duplicate of #41338, which was merged. Both applied the identical fail-closed fixture fix (allowed_user_ids matching the interaction user); #41338 carried the explanatory comments. Closing this one.

@teknium1 teknium1 closed this Jun 7, 2026
@teknium1
teknium1 deleted the fix/discord-model-picker-test-allowlist branch June 8, 2026 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have platform/discord Discord bot adapter type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant