Catalog: honor a mis-cased Athena workgroup and stop masking the bucket default (5217 stack 7/9)

[nl0]

Description

Two bugs in how the Athena console chooses a workgroup, plus the seam that lets a
bucket's ui.athena.defaultWorkgroup reach that choice at all.

The console errors instead of honoring the default. The workgroup was matched
case-insensitively and then stored and used verbatim. An admin writing
ui.athena.defaultWorkgroup: Analytics-Prod against a workgroup named
analytics-prod matched here — and then listNamedQueries and
startQueryExecution sent the admin's casing, and AWS rejected it with
InvalidRequestException. The same hole swallowed a mis-cased :workgroup URL
segment. Candidates now resolve to the list's own spelling before being stored.

An unavailable stored workgroup masked a valid default. With
storage.getWorkgroup() || defaultWorkgroup, a stored workgroup that had been
deleted or lost access was picked by the ||, failed the availability check, and
dropped through to the alphabetically-first workgroup — never trying a bucket
default that was perfectly good. Candidates are tried in turn instead.

The seam. Model.Provider takes an optional preferences prop and hands
ui.athena.defaultWorkgroup to useWorkgroup as a string rather than the whole
object — the parsed preferences are rebuilt on every provider render, so an object
would re-fire the workgroup effect throughout the probe.

Safe on its own. Nothing passes the new prop yet, so the seam is a no-op in
isolation and this PR stands as a refactor plus two real bug fixes. The consumer
that supplies the prop is PR 9.

Review findings addressed

• f34 — the case-insensitive match with verbatim use, above.
• f16 — the masked bucket default, above. Strictly a bug fix: storage
  still outranks the bucket default; only the fall-through changed.
• f46 — the Provider → useWorkgroup seam was crossed by no test, so the
  prop could have been dropped on the floor unnoticed. Now driven through the
  provider.
• f33 (i) — nothing pinned the stored-vs-preference precedence in either
  direction. It is pinned now.
• f51 — recorded as the change's own benefit rather than left implicit:
  narrowing the third parameter to defaultWorkgroup?: string removes
  utils/BucketPreferences from the AWS-call layer entirely.

⚠️ f33 (ii) — a decision this PR deliberately does not take

storage.getWorkgroup() still outranks ui.athena.defaultWorkgroup. That means
any user who has ever switched workgroup never sees the bucket default — which
is narrower than "honors that bucket's defaultWorkgroup again" implies.

This precedence is not new: it exists verbatim in the 26.7.4 pin
(eda3016f,
Bucket/Queries/Athena/model/requests.ts:127), which is the baseline the
hardening review compared master
(6167dd82) against.
Reordering it changes behaviour for returning users, which is a product decision
rather than hardening — so it is not silently reordered here.

A maintainer should rule on one of two things: either the precedence is
reordered deliberately in its own change, or the changelog wording narrows to
match what the code does. The test added here pins the current order, so a
deliberate reversal will show up as a test to change rather than a silent drift.

f16's fall-through fix is independent of this and lands either way.

Recorded, not fixed here

• f19 — preferences?.defaultWorkgroup flattens a preference object to a
  primitive at the consumer to dodge an unmemoized shared provider. Real; the
  proper fix is to memoize BucketPreferences, which is wider than this branch.
• The same case-canonicalisation shape exists for databases and catalog names
  (listIncludes at five other call sites in this file). Untouched here: f34 named
  the workgroup, and those paths have their own storage-restore behaviour that
  deserves its own reading rather than a blanket change.

Verification

cd catalog && npx vitest run app/containers/Queries/Athena

102 tests. The four new ones in requests.spec.ts and the one in state.spec.tsx
were each checked against the pre-fix code and fail there.

Position in the stack

PR 7 of 9, based on
stack/5217-6-admin-service-read-only.

Part of the split of #5217 asked for in
f27. This
is the one hard mechanical dependency in that set, and the reason the Athena work
splits in two: the seam is a no-op in isolation, so it can land on its own merits
while the mount change that activates it (PR 9) waits on a direction call. Do
not squash this into PR 9 — that destroys the property that makes it
independently mergeable.

TODO

• Unit tests
• Security: Confirm that this change meets security best practices and does not violate the security model
• Open: Confirm that this change doesn't break the Open variant
• Changelog entry

🤖 Generated with Claude Code

Greptile Summary

This PR corrects Athena workgroup selection by canonicalizing case-insensitive matches to AWS-provided names and trying stored and bucket-default candidates independently. It also adds an optional provider seam for forwarding the bucket’s default workgroup and expands focused test coverage.

• Canonicalizes URL, stored, and configured workgroup names before downstream AWS calls.
• Falls through from an unavailable stored workgroup to a valid bucket default.
• Threads preferences.defaultWorkgroup through the Athena model provider.
• Adds regression tests for casing, precedence, fallback, and provider propagation.

Confidence Score: 5/5

The PR appears safe to merge, with the changed selection paths constrained to available Athena workgroups and covered by targeted regression tests.

The implementation resolves candidate names against Athena’s returned list, preserves the documented precedence, falls through unavailable candidates correctly, and introduces no active incompatibility because the new provider prop is optional.

Important Files Changed

Filename	Overview
catalog/app/containers/Queries/Athena/model/requests.ts	Refactors workgroup matching and precedence to use canonical available names and correctly fall through among candidates.
catalog/app/containers/Queries/Athena/model/requests.spec.ts	Adds focused regression coverage for case canonicalization, unavailable stored selections, and stored/default precedence.
catalog/app/containers/Queries/Athena/model/state.tsx	Adds a type-safe optional Athena preference prop and forwards only the stable default-workgroup string.
catalog/app/containers/Queries/Athena/model/state.spec.tsx	Verifies that the provider seam forwards the configured default through the integrated Athena model.
catalog/CHANGELOG.md	Documents the corrected casing and fallback behavior accurately.

_{Reviews (1): Last reviewed commit: "docs(changelog): entry for the workgroup..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 36.01%. Comparing base (dac681d) to head (5a2b996).

Additional details and impacted files

@@                           Coverage Diff                            @@
##           stack/5217-6-admin-service-read-only    #5265      +/-   ##
========================================================================
+ Coverage                                 35.99%   36.01%   +0.01%     
========================================================================
  Files                                       741      741              
  Lines                                     23829    23836       +7     
  Branches                                   6434     6438       +4     
========================================================================
+ Hits                                       8577     8584       +7     
  Misses                                    13520    13520              
  Partials                                   1732     1732              

Flag	Coverage Δ
catalog	36.01% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nl0]

Local code review — findings and dispositions

Reviewed this layer only (stack/5217-6-admin-service-read-only..stack/5217-7-athena-workgroup-seam). Three findings accepted and fixed in 96ddb83 and 125ac21, three declined.

Accepted

1. An exact workgroup match had to beat a case-insensitive one — regression in this layer. canonical() returned the first case-insensitive hit. Workgroup names are case-sensitive, so Alpha and alpha can both exist, and fetchWorkgroups ends with list.sort(), which puts Alpha first. Picking alpha from the dropdown then stored alpha and navigated to /queries/athena/alpha, which resolved back to Alpha — queries, history and named queries all ran against the workgroup the user did not choose, and the Select displayed a name they never picked. Before this layer an exactly-spelled request was stored verbatim and was correct, so this was introduced here. Now an exact hit wins; regression test added, and it fails without the fix.

2. A non-string defaultWorkgroup threw out of the effect. ui.athena is unconstrained by shared/schemas/bucketConfig.yml.json — there is no athena node and ui does not set additionalProperties: false — and parseAthena only spreads. So ui.athena.defaultWorkgroup: 2024, an all-digit workgroup name that is legal in Athena, survives validation as a number, and toLowerCase() on it took the console down instead of falling back. Not introduced here (the old listIncludes path had the same exposure), but this layer rewrites that code and #5267 is what makes it reachable, so it is guarded here: pick takes unknown and rejects non-strings. The complete fix is a schema entry for ui.athena; that is left as follow-up rather than widened into this layer.

3. The changelog entry claimed more than this layer ships. It announced ui.athena.defaultWorkgroup support and the unmasking of a bucket default, but the only Model.Provider render site is still prop-less at this layer, so defaultWorkgroup is always undefined at runtime here — and #5267 already carries its own entry for exactly that behaviour. If this layer merges ahead of #5267 the entry is false in the interim, so it now claims only the URL-capitalisation fix, which does work standalone.

Declined

4. useDatabase / useCatalogName share the match-insensitively-then-store-verbatim shape. Declined: unlike the workgroup, those values only ever originate from the AWS-returned list or a prior AWS response, so no mis-cased value can enter.

5. The preference document can resolve after the workgroup is seeded. Declined here: real, but #5267 gates the scoped mount on the resolved document, which is the right layer for it.

6. Provider's new preferences prop has no production caller. Not a defect — intentional decomposition; #5267's ScopedConsole supplies it.

Also swept

Comments were retelling what the change did and what the old code got wrong — the tests already pin that. Trimmed to what the code cannot show: why the canonical spelling must be used, why the preference crosses as a string rather than the ui.athena object, and that storage-before-default is inherited product behaviour rather than a choice made here.

Test hygiene: the stubbed-mock restores now sit in a finally. afterEach only calls vi.clearAllMocks(), which clears call data but keeps implementations, so a failing assertion leaked the stub into every later test in the file and turned one real failure into a cascade of unrelated ones.

Verification

80 tests pass in the two touched spec files (2 new, each confirmed failing without its fix); tsc --noEmit clean; oxlint and oxfmt clean.