Catalog: keep the bucket scope on legacy Athena redirects (5217 stack 8/9)

[nl0]

Description

A legacy /b/:bucket/queries/athena/... URL redirects into the workspace-level
console. It dropped the bucket segment on the way, so the bucket's ui.athena
preferences stopped applying — the reader landed in the same console, unscoped.
The bucket segment now becomes the console's ?bucket= scope param on every
athena shape, with the rest of the search preserved alongside it.

This is a behaviour change to legacy deep links, and it deserves saying out
loud: a bookmarked /b/:bucket/queries/athena/:workgroup now lands somewhere
different from before — same path, but with ?bucket= set. Nothing in the
previous description or changelog said so, which is why this unit carries its own
entry.

Safe on its own. The param is carried and, until PR 9, ignored. That makes
this the producer half of a producer/consumer pair, and it is deliberately first:
PR 9 alone would leave the preference dead for exactly the legacy-URL readers the
fix is for.

Review findings addressed

• f12 (thread)
  — the incoming query string was spread after the route's bucket, so a
  ?bucket= riding along on the legacy URL overrode the bucket the URL was
  about. /b/source/queries/athena/primary?bucket=other redirected into the
  console scoped to other and loaded other's preferences. Both sites had the
  same spread order — the workgroup/execution helper and the root redirect — and
  both are reversed. Three cases are pinned.
• f13 — the undisclosed relocation of legacy deep links, above and in the
  changelog. Splitting the change is what made this statable: it is this PR's
  whole subject rather than one line in a six-subsystem diff.

Recorded:

• f14 — checked and refuted: the in-file "unchanged" claim is not inside the
  hunk that changes it.

Verification

cd catalog && npx vitest run app/containers/App/queryRedirects.spec.tsx

9 tests. The three added for f12 fail against the original spread order, as does
the param-preservation test whose expected string this PR updates — the query
string's param order reverses, which carries no meaning to any reader of these
params.

Position in the stack

PR 8 of 9, based on
stack/5217-7-athena-workgroup-seam.

Part of the split of #5217 asked for in
f27.
Producer before consumer: this PR makes legacy URLs carry ?bucket=, and PR 9
makes the console read it. The user-visible changelog claim about honoring
ui.athena.defaultWorkgroup lands with PR 9, because that is when it becomes
true.

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 preserves the bucket scope when legacy per-bucket Athena links redirect into the workspace query console.

• Adds the route-derived bucket as an authoritative ?bucket= parameter for root, workgroup, and execution links.
• Preserves other incoming search parameters on workgroup and execution redirects.
• Adds focused redirect tests and documents the user-visible behavior in the changelog.

Confidence Score: 5/5

The PR appears safe to merge, with the changed redirect behavior consistently applied and covered by focused tests.

The route-derived bucket is available on every affected legacy route, is applied after incoming parameters so it remains authoritative, and is serialized through the existing URL utility without an identified contract break.

Important Files Changed

Filename	Overview
catalog/app/containers/App/queryRedirects.jsx	Adds bucket-scoped search construction to legacy Athena workgroup and execution redirects and gives the route bucket precedence on every Athena redirect shape.
catalog/app/containers/App/queryRedirects.spec.tsx	Covers bucket propagation, conflicting query-string bucket precedence, and preservation of another query parameter.
catalog/CHANGELOG.md	Documents the changed destination scope and authoritative route-bucket behavior for legacy Athena links.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Legacy[Legacy /b/:bucket/queries/athena link] --> Parse[Parse incoming search parameters]
  Parse --> Override[Apply route bucket as authoritative scope]
  Override --> Console[Workspace Athena console with ?bucket=:bucket]
  Console --> Preferences[Bucket ui.athena preferences]

Loading

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

[codecov]

Codecov Report

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

Additional details and impacted files

@@                          Coverage Diff                           @@
##           stack/5217-7-athena-workgroup-seam    #5266      +/-   ##
======================================================================
+ Coverage                               36.01%   36.02%   +0.01%     
======================================================================
  Files                                     741      741              
  Lines                                   23836    23841       +5     
  Branches                                 6438     6438              
======================================================================
+ Hits                                     8584     8589       +5     
  Misses                                  13520    13520              
  Partials                                 1732     1732              

Flag	Coverage Δ
catalog	36.02% <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

Ran a local review of this layer only (stack/5217-7-athena-workgroup-seam..stack/5217-8-queries-legacy-redirect-scope). 15 candidate findings; 7 accepted and fixed in the two commits just pushed, 8 declined with reasons.

Accepted — fixed here

1. ?table= on an execution redirect overwrote the editor. Forwarding the whole incoming query string put ?table= on /queries/athena/:workgroup/:queryExecutionId. TabulatorTables renders outside the execution branch (Athena.tsx), so its one-shot autofill called queryBody.setValue(...) — replacing the SQL that came from the execution's own results, plus the catalog and database selectors. Neither of these two routes declares a search param other than the scope, and the Athena root is the only shape that takes a ?table= deep link, so the workgroup and execution redirects now carry the scope and nothing else.

2. A crash path on malformed input. parseSearch pulls in querystring, which in the browser bundle resolves to the npm querystring@0.2.0 shim, whose decode is an unguarded decodeURIComponent. /b/b1/queries/athena/primary?q=100% threw URIError: URI malformed out of the redirect and unwound to the error boundary — an error page instead of a redirect. These two routes never touched the search before this branch, so this was newly introduced. Dropping the parse removes it. Worth noting the tests could not have caught it: vitest resolves querystring to Node's lenient builtin, which returns { q: '100%' } rather than throwing.

3. A test gap behind (1). Nothing pinned ?table= on an execution redirect. Added a case for both shapes; both fail against the previous implementation.

4. Two spec comments described an intermediate state as shipped history. They said a ?bucket= on the incoming URL "used to win" on the workgroup and execution shapes. It never did — before this branch those two dropped the search entirely and landed unscoped; only the Athena root honoured the param. Reworded, and the note now says which of the three rows is a regression test.

5. A comment gave a false reason. "Through the route builder rather than useScopeSearch, so ?table= tabulator deep links keep their declared shape" — the helper forwarded table perfectly well; the builder is the leg that drops params. Rewritten to say what the whitelist actually does.

6. The file header was stale. It still claimed the components were extracted "unchanged" with targets wired "exactly as before", which stopped being true once the targets gained the scope.

7. The changelog entry claimed a payoff this change cannot deliver. It said the scope makes the bucket's ui.athena preferences apply. ui.athena holds exactly one preference, defaultWorkgroup, and useWorkgroup reads a workgroup named in the path ahead of it — both shapes changed here always name one, so the preference is never reached from them. (The Athena root is where it applies, and that shape already carried the scope.) The entry now claims the scope itself, and is tagged [Fixed] to match both authoring commits and the neighbouring entry of the same shape.

Declined

• Move the console's search into the url builders (a shared opts type across queriesAthena / queriesAthenaWorkgroup / queriesAthenaExecution, making an unscoped call a type error). Genuinely the better altitude, and it would subsume several items above. Declined here: it changes exported builder signatures in constants/routes.ts, outside this layer's three files, and the next PR in the stack already introduces a single place to read the scope. Better as its own change on top of the stack.
• In-console links drop the scope — the History execution rows, the "Query Executions" breadcrumb, the Athena nav tab, the catch-all redirect. Real, but fixed in the next PR in this stack, which is where the scope-reading lives.
• ui.athena is not wired into the console yet — also the next PR.
• BucketSearchRedirect in App.jsx has the identical { routeValue, ...params } precedence bug this PR fixes for Athena: /b/my-bucket/search?buckets=other searches other. Confirmed, but it is a different feature in a file this PR does not touch; it deserves its own one-line fix rather than riding along here.
• The ES leg still lands unscoped. True, and with elasticsearch-queries off by default it lands on the Athena console with no bucket. But the ES console holds its scope in component state and never reads the URL, so scoping it is a separate change. Narrowed the changelog wording to the Athena shapes instead of claiming all legacy queries links.
• A scoped execution load now issues a tabulator-tables GraphQL request it did not before. That is inherent to carrying a scope; no change.
• useScopeSearch has no invariant on :bucket — a missing param would be silently dropped and land unscoped. No current mount point lacks it (both consuming routes carry :bucket, and the pathless catch-all inherits the parent match), and the builder change above would make it a type error.
• Object-form to requires a query-free pathname (history v4 does not parse to.pathname). Holds for both builders today; it is the reason the root redirect cannot use the same helper. Would disappear with the builder change.

Greptile

No Greptile review or inline comments had appeared on this PR at the time of writing, across the full polling budget. Proceeded on the local review alone.