refactor: always preserve marketplace search state in URL

[tomerqodo]

Benchmark PR from agentic-review-benchmarks#9

[claude]

🔴 The ! negation was removed from PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType) on line 43, inverting the search mode fallback logic. The comment on line 32-33 states the intent is to force search mode for categories without collections, but the new code forces it for categories with collections (only "all" and "tool"). This means categories like model, agent, extension, etc. will no longer default to search mode, and the UI will try to display non-existent collections for them.

Extended reasoning...

What the bug is

The useMarketplaceSearchMode hook in atoms.ts determines whether the marketplace UI should display in search mode or collection mode. The fallback expression on line 43 was changed from searchMode ?? (!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType)) to searchMode ?? (PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType)) — the ! negation operator was removed.

Why this is wrong

The comment directly above (lines 32-33) explains the intent: "Not all categories have collections, so we need to force the search mode for those categories." The PLUGIN_CATEGORY_WITH_COLLECTIONS set (defined in constants.ts lines 25-30) contains only "all" and "tool" — these are the only two categories that have collections to display. The old negated logic correctly returned true (force search mode) for categories NOT in this set (model, agent, extension, datasource, trigger, bundle). The new un-negated logic returns true for categories IN the set — the exact opposite of what the comment describes.

Concrete proof via code walkthrough

Consider a user selecting the "model" category:

• activePluginType = "model"
• PLUGIN_CATEGORY_WITH_COLLECTIONS.has("model") = false
• Old code: searchMode ?? (!false) = searchMode ?? true → defaults to search mode (correct — models have no collections)
• New code: searchMode ?? (false) = searchMode ?? false → defaults to collection mode (incorrect — there are no model collections to show)

Conversely, for the "all" category:

• PLUGIN_CATEGORY_WITH_COLLECTIONS.has("all") = true
• Old code: searchMode ?? (!true) = searchMode ?? false → defaults to collection mode (correct — "all" has collections)
• New code: searchMode ?? (true) = searchMode ?? true → defaults to search mode (incorrect — skips showing available collections)

Corroboration from hydration-server

The hydration-server.tsx file at line 19 uses if (!PLUGIN_CATEGORY_WITH_COLLECTIONS.has(params.category)) { return } — it skips hydrating collection data for categories without collections. This confirms the old negated logic in useMarketplaceSearchMode was correct: categories not in the set should use search mode because there is no collection data for them.

Impact

This affects every marketplace user. Categories without collections (model, agent, extension, datasource, trigger, bundle) will attempt to show a collection view with no data, while categories with collections (all, tool) will default to search mode hiding the curated collection view. The marketplace browsing experience is effectively broken for all category types.

Fix

Restore the negation on line 43: change PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType) back to !PLUGIN_CATEGORY_WITH_COLLECTIONS.has(activePluginType).

[claude]

🔴 Client/server URL parameter mismatch: useActivePluginType() now reads/writes ?tab=X (atoms.ts:23) but server-side hydration in hydration-server.tsx uses createLoader(marketplaceSearchParamsParsers) which reads ?category=X based on the object key in search-params.ts:6. This means server-side prefetching will never see ?tab= URLs from client navigation, and client-side state will never pick up ?category= URLs from server prefetching, breaking shared/bookmarked links.

Extended reasoning...

The Bug

The first argument to useQueryState in the nuqs library determines the URL parameter name that the hook reads from and writes to. In atoms.ts:23, useActivePluginType() was changed from useQueryState("category", ...) to useQueryState("tab", ...). This means the client now uses ?tab=tool in the URL instead of ?category=tool.

The Server-Side Mismatch

The server-side hydration in hydration-server.tsx:16 calls createLoader(marketplaceSearchParamsParsers). The createLoader function from nuqs/server reads URL parameters based on the object keys of the parsers passed to it. In search-params.ts:5-6, the parsers object has the key category, so the server reads the ?category= URL parameter. This was never updated to match the client-side rename to tab.

Step-by-Step Proof

1. A user visits the marketplace and clicks on a plugin type (e.g., "tool"). The client calls useActivePluginType() which calls useQueryState("tab", ...), updating the URL to ?tab=tool.
2. The user copies and shares this URL: https://example.com/marketplace?tab=tool.
3. When another user opens this shared URL, the server-side getDehydratedState() in hydration-server.tsx runs. It calls createLoader(marketplaceSearchParamsParsers) which looks for ?category= in the URL.
4. Since the URL has ?tab=tool (not ?category=tool), the server reads params.category as the default value "all" and prefetches data for "all" instead of "tool".
5. On the client, useActivePluginType() reads ?tab=tool correctly and shows the "tool" category, but with no prefetched data — causing an unnecessary re-fetch and a flash of wrong content.

Impact

This breaks the server-side rendering optimization entirely for the category/tab filter. Server-prefetched data will never match what the client expects when a category is selected, resulting in hydration mismatches, unnecessary client-side refetches, and broken shared/bookmarked URLs.

Fix

Either change useQueryState("tab", ...) back to useQueryState("category", ...) in atoms.ts:23, or update the key in search-params.ts:6 from category to tab so the server reads the same parameter name as the client.

[claude]

🟡 The selector parameter type in usePluginPageContext was degraded from (value: PluginPageContextValue) => any to bare any, removing type safety for all callers. This means non-function values can now be passed without compile-time error, and callers lose autocomplete on PluginPageContextValue fields. Restore the original typed signature or improve it to <T>(selector: (value: PluginPageContextValue) => T) => T.

Extended reasoning...

What changed

The function signature of usePluginPageContext in web/app/components/plugins/plugin-page/context.tsx was changed from:

export function usePluginPageContext(selector: (value: PluginPageContextValue) => any)

to:

export function usePluginPageContext(selector: any)

This removes the type constraint on the selector parameter entirely.

How it manifests

Previously, TypeScript enforced that selector must be a function that accepts a PluginPageContextValue argument. Callers like usePluginPageContext(v => v.currentPluginID) got full autocomplete and type-checking on the v parameter — TypeScript knew v was PluginPageContextValue and would flag typos like v.currentPlugnID. With the bare any type, all of that is lost. Additionally, completely invalid calls like usePluginPageContext(42) or usePluginPageContext("not a function") would now pass type-checking without error.

Why existing code doesn't catch it

All 15+ existing call sites already pass correct arrow function selectors (e.g., v => v.currentPluginID, v => v.filters), so no runtime errors will occur from this change. The useContextSelector function from use-context-selector still expects a function internally and will work correctly at runtime. The issue is purely at the type-checking level — future incorrect usage won't be caught at compile time.

Impact

This is a type safety regression rather than a runtime bug. It affects developer experience (lost autocomplete, lost compile-time validation) and violates the project's own AGENTS.md Rule 9 which states TypeScript code should avoid any type annotations. Since all current callers pass correct values, there is no immediate runtime impact.

Step-by-step proof

1. Before this PR, writing usePluginPageContext(123) would produce a TypeScript error: "Argument of type 'number' is not assignable to parameter of type '(value: PluginPageContextValue) => any'."
2. After this PR, usePluginPageContext(123) compiles without error because selector: any accepts any value.
3. Similarly, writing usePluginPageContext(v => v.nonExistentField) previously gave autocomplete showing valid fields (containerRef, currentPluginID, filters, etc.) and would error on invalid fields. Now v is implicitly any, so no checking occurs.

Fix

Restore the original signature, or better yet, make it generic:

export function usePluginPageContext<T>(selector: (value: PluginPageContextValue) => T): T {
  return useContextSelector(PluginPageContext, selector)
}