fix: session native tools and extensions details - #330

internal/extension/provenance.go (1)

126-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the ChecksumVerified rule explicit per source.

ChecksumVerified: source != SourceMarketplace marks every non-marketplace source as checksum-verified, including SourceUser and SourceWorkspace. The sibling helper LocalPathProvenance (lines 100-109) sets ChecksumVerified: false for local paths and attaches an unverified diagnostic. The two paths now disagree for the same kind of local install. A reader cannot tell which rule is authoritative.

Consider a switch on source so each source states its own verification and tier, instead of one negated comparison plus a follow-up if.

♻️ Suggested clarification

-	provenance := ExtensionProvenance{
-		InstalledFrom:    installedFromForSource(source),
-		SourceURL:        strings.TrimSpace(sourceURL),
-		ChecksumSHA256:   strings.TrimSpace(checksum),
-		ChecksumVerified: source != SourceMarketplace,
-		RegistryTier:     ExtensionRegistryTierUnverified,
-		Permissions:      append([]string(nil), permissions...),
-		InstalledAt:      installedAt.UTC(),
-		InstalledBy:      extensionTrustInstalledByOperator,
-	}
-	if source == SourceBundled {
-		provenance.RegistryTier = ExtensionRegistryTierOfficial
-	}
-	return provenance
+	provenance := ExtensionProvenance{
+		InstalledFrom:  installedFromForSource(source),
+		SourceURL:      strings.TrimSpace(sourceURL),
+		ChecksumSHA256: strings.TrimSpace(checksum),
+		RegistryTier:   ExtensionRegistryTierUnverified,
+		Permissions:    append([]string(nil), permissions...),
+		InstalledAt:    installedAt.UTC(),
+		InstalledBy:    extensionTrustInstalledByOperator,
+	}
+	switch source {
+	case SourceBundled:
+		// First-party artifact: checksum is computed from the embedded payload.
+		provenance.ChecksumVerified = true
+		provenance.RegistryTier = ExtensionRegistryTierOfficial
+	case SourceMarketplace:
+		// Remote archive: verification happens in the marketplace install path.
+		provenance.ChecksumVerified = false
+	default:
+		// Local path installs: checksum is recomputed at install time.
+		provenance.ChecksumVerified = true
+	}
+	return provenance

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/extension/provenance.go` around lines 126 - 135, Update the
provenance construction around ExtensionProvenance to use an explicit
source-based switch for ChecksumVerified and RegistryTier, rather than source !=
SourceMarketplace. Ensure SourceMarketplace, SourceUser, and SourceWorkspace
each state their intended verification and tier consistently with
LocalPathProvenance, while preserving the existing permissions, timestamps,
installer, and checksum fields.

internal/extension/registry_bundled.go (1)

30-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log the provenance rewrite.

This function rewrites persisted trust evidence for an installed extension and produces no log record and no event. An operator cannot tell later that the stored provenance changed, or why the trust decision moved from blocked to verified. Add an slog record with the extension name, the previous installed_from, and the previous registry_tier before the update.

The coding guidelines require slog in internal production code and a canonical event for every domain operation.

♻️ Suggested addition

 	if bytes.Equal(currentJSON, provenanceJSON) {
 		return nil
 	}
+	slog.Info(
+		"reconciling bundled extension provenance",
+		"extension", info.Name,
+		"previous_installed_from", info.Provenance.InstalledFrom,
+		"previous_registry_tier", info.Provenance.RegistryTier,
+		"registry_tier", provenance.RegistryTier,
+	)
 	result, err := r.db.ExecContext(

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/extension/registry_bundled.go` around lines 30 - 47, Before
rewriting persisted provenance in the function containing the
`extensionInstallProvenance` comparison, emit the canonical provenance-update
event and an `slog` record. Include the extension name plus the previous
provenance’s `installed_from` and `registry_tier`, and ensure the event/log
occurs only when `bytes.Equal(currentJSON, provenanceJSON)` is false and before
persistence.

Source: Coding guidelines

extensions/dev-cycle/embed_test.go (1)

172-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert against the exported constant instead of the literal.

Both assertions compare InstalledFrom with the literal "bundled". The PR adds extensionpkg.ExtensionInstalledFromBundled for this value, and the adjacent assertions already use exported constants. Use the constant so the test tracks the contract rather than a copied string.

💚 Proposed fix

-		if installed.Provenance.InstalledFrom != "bundled" ||
+		if installed.Provenance.InstalledFrom != extensionpkg.ExtensionInstalledFromBundled ||
 			installed.Provenance.RegistryTier != extensionpkg.ExtensionRegistryTierOfficial ||

-		if after.Provenance.InstalledFrom != "bundled" ||
+		if after.Provenance.InstalledFrom != extensionpkg.ExtensionInstalledFromBundled ||
 			after.Provenance.RegistryTier != extensionpkg.ExtensionRegistryTierOfficial ||

Also applies to: 334-334

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@extensions/dev-cycle/embed_test.go` at line 172, Update both assertions on
installed.Provenance.InstalledFrom in the relevant test to compare against the
exported extensionpkg.ExtensionInstalledFromBundled constant instead of the
duplicated "bundled" literal, matching the adjacent assertions.

internal/api/core/marketplace_installed_detail.go (1)

128-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Centralize the installed marketplace manage paths. internal/api/core/marketplace_paths.go already holds marketplaceExtensionsInstalledPath, but the MCP and skill paths remain inline literals at four sites. Declare matching constants and reference them everywhere.

• internal/api/core/marketplace_installed_detail.go#L128-L128: replace the /marketplace/mcps literal, and the /marketplace/skills literal at line 191, with the new constants.
• internal/api/core/marketplace_list.go#L257-L320: replace the /marketplace/mcps literal at line 257 and the /marketplace/skills literal at line 320 with the same constants.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/api/core/marketplace_installed_detail.go` at line 128, Centralize
the installed marketplace manage paths by declaring constants alongside
marketplaceExtensionsInstalledPath in marketplace_paths.go, then update
internal/api/core/marketplace_installed_detail.go lines 128-191 and
internal/api/core/marketplace_list.go lines 257-320 to use those constants
instead of inline MCP and skill path literals.

internal/api/core/marketplace_list.go (1)

257-257: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reuse shared path constants here too.

These two literals duplicate the values used in internal/api/core/marketplace_installed_detail.go. Replace them with the shared constants declared in internal/api/core/marketplace_paths.go.

Also applies to: 320-320

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/api/core/marketplace_list.go` at line 257, Replace the duplicated
managePath literals in the marketplace list entries with the shared path
constants from marketplace_paths.go, including the occurrence near line 320.
Reuse the existing constants without changing the surrounding marketplace
behavior.

web/src/systems/marketplace/components/marketplace-detail.tsx (1)

27-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider a kind-to-view map.

Three independent ternaries render the kind views. An unrecognized kind renders the lede with no body and no diagnostic. A small record keyed by kind makes the dispatch exhaustive and keeps the JSX flat.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/systems/marketplace/components/marketplace-detail.tsx` around lines
27 - 41, Replace the independent kind checks in the marketplace detail component
with a kind-to-view record keyed by the supported marketplace kinds, then render
the selected view through that map while preserving the existing props for each
view. Make the dispatch exhaustive so an unrecognized kind produces an explicit
diagnostic instead of rendering only the lede.

web/src/systems/marketplace/components/marketplace-kind-page.tsx (1)

113-122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider limiting config_scope to MCP routes.

page.mcpConfigScope always resolves to a value, because the hook derives a default when search.config_scope is absent. Every kind link therefore emits config_scope in the URL, including the skills and extensions routes where the value has no effect. Pass the parameter only when the destination is the MCP route, or only when the current search already carries it.

♻️ Proposed change

               render={
                 <Link
                   search={{
-                    config_scope: page.mcpConfigScope,
+                    config_scope: item.kind === "mcp" ? page.mcpConfigScope : undefined,
                     q: page.query || undefined,
                     tab: page.scope === "market" ? "market" : undefined,
                   }}
                   to={item.to}
                 />
               }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/systems/marketplace/components/marketplace-kind-page.tsx` around
lines 113 - 122, Update the kind-link `Link` search configuration so
`config_scope` is included only for MCP destinations, or when the current search
explicitly contains it, instead of always using the defaulted
`page.mcpConfigScope`. Preserve the existing query and tab behavior for skills,
extensions, and MCP routes.

web/src/systems/marketplace/components/marketplace-detail-shell.tsx (2)

17-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Confirm the fixed rail width matches the documented 320px contract.

The doc comment at Lines 6-10 states a 320px rail. The grid template uses 20rem. Those match only while the root font size is 16px. If the design system defines a rail width token, use it instead of the literal 20rem.

As per coding guidelines: "Use tokens from packages/ui/src/tokens.css and DESIGN.md for all color, type, radius, spacing, and motion values; do not invent design values."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/systems/marketplace/components/marketplace-detail-shell.tsx` around
lines 17 - 24, Update the rail column definition in MarketplaceDetailColumns to
use the existing design-system token for the documented 320px rail width, rather
than the literal 20rem value. Reuse the token defined in
packages/ui/src/tokens.css or DESIGN.md and preserve the current responsive grid
behavior.

Source: Coding guidelines

---

44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove manual useId wiring on Collapsible components. @compozy/ui’s Collapsible delegates to @base-ui/react/collapsible, which automatically associates trigger and content. The useId().replace(/:/g, "") value is not needed, at both marketplace-detail-shell.tsx:44 and marketplace-detail-shell.tsx:145.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/systems/marketplace/components/marketplace-detail-shell.tsx` at line
44, Remove the manual useId-based panelId wiring from both Collapsible usages in
marketplace-detail-shell.tsx at lines 44-44 and 145-145; remove the associated
panelId references and rely on Collapsible’s automatic trigger/content
association.

web/src/systems/marketplace/components/marketplace-detail-mcp.tsx (1)

322-327: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use cn for the status cell classes.

The rest of this file and marketplace-detail-shell.tsx compose classes with cn. This helper concatenates strings manually and encodes the grid position rules in index arithmetic, which breaks silently if the cell count changes.

♻️ Proposed refactor

-function cnStatusCell(index: number): string {
-  const base = "flex flex-col items-start gap-1.5 px-3.5 py-2.5";
-  const divider = index % 2 === 0 ? " border-r border-line-soft" : "";
-  const top = index >= 2 ? " border-t border-line-soft" : "";
-  return `${base}${divider}${top}`;
-}
+const STATUS_CELL_CLASS = cn(
+  "flex flex-col items-start gap-1.5 px-3.5 py-2.5",
+  "border-line-soft not-nth-[2n]:border-r [&:nth-child(n+3)]:border-t"
+);

An equivalent option is to keep the function and wrap the parts in cn.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/systems/marketplace/components/marketplace-detail-mcp.tsx` around
lines 322 - 327, Update cnStatusCell to compose its conditional status-cell
classes with the existing cn utility instead of manual string interpolation.
Preserve the current base, divider, and top-border class rules while passing
them as separate conditional arguments, avoiding index-derived concatenation
that can silently break when the grid changes.

web/src/systems/marketplace/hooks/use-marketplace-detail-mcp-server.ts (1)

22-24: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

The entry.name fallback can bind the wrong server.

entry.name is the catalog display name. server.name is the installed identity. If a catalog entry's display name equals an unrelated installed server name, this fallback resolves the wrong row, and the detail view then shows that server's status, tools, and registration. Consider restricting the fallback to catalog_entry, or comparing a normalized slug rather than the display name.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/systems/marketplace/hooks/use-marketplace-detail-mcp-server.ts`
around lines 22 - 24, Update the server lookup around the find callback to avoid
matching server.name against entry.name, since display names can identify
unrelated installed servers. Restrict matching to the catalog_entry identity, or
use an established normalized-slug comparison if one already exists, while
preserving the existing catalog entry match behavior.

web/src/systems/marketplace/components/marketplace-detail-mcp-topbar.tsx (1)

64-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Extract the duplicated fallback branch.

The branch at Lines 64-76 and the branch at Lines 82-99 render the same MarketplaceEntryStatus and MarketplaceEntryAction pair. Only the MCPAuthorizeDialog differs. Extract the shared pair into a local component and render the dialog conditionally.

♻️ Proposed refactor

+  const standardHead = (
+    <>
+      <MarketplaceEntryStatus entry={entry} />
+      <MarketplaceEntryAction
+        emphasis="primary"
+        entry={entry}
+        onAction={onAction}
+        pending={pending}
+      />
+    </>
+  );
+
+  if (!polledServer) return standardHead;
+
+  const status = composeMCPRowStatus(polledServer);
+  const label = status.authorizeLabel;
+  const authFilter = deriveMCPAuthFilter(polledServer);
+
+  if (!label) {
+    return (
+      <>
+        {standardHead}
+        <MCPAuthorizeDialog
+          authorize={authorize}
+          scope={authFilter?.scope ?? "global"}
+          server={polledServer}
+        />
+      </>
+    );
+  }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/systems/marketplace/components/marketplace-detail-mcp-topbar.tsx`
around lines 64 - 99, In the component containing the polledServer status flow,
extract the shared MarketplaceEntryStatus and MarketplaceEntryAction pair into a
local component or reusable render path. Use it in both the !polledServer branch
and the !label branch, while rendering MCPAuthorizeDialog only conditionally for
the latter with its existing authorize, scope, and server props.

web/src/systems/marketplace/components/marketplace-detail-extension-installed.tsx (1)

363-364: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Pass facts into the runtime card instead of recomputing it.

The parent computes facts at Line 96 and passes it to MarketplaceExtensionManageCard and MarketplaceExtensionProvenanceCard. MarketplaceExtensionRuntimeCard calls extensionTrustFacts(extension) again. Reuse the single computed value so all cards read identical facts and the derivation runs once per render.

♻️ Proposed refactor

-function MarketplaceExtensionRuntimeCard({ extension }: { extension: ExtensionEntry }) {
-  const facts = extensionTrustFacts(extension);
+function MarketplaceExtensionRuntimeCard({
+  extension,
+  facts,
+}: {
+  extension: ExtensionEntry;
+  facts: ReturnType<typeof extensionTrustFacts>;
+}) {

Update the call site at Line 121:

-            <MarketplaceExtensionRuntimeCard extension={extension} />
+            <MarketplaceExtensionRuntimeCard extension={extension} facts={facts} />

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@web/src/systems/marketplace/components/marketplace-detail-extension-installed.tsx`
around lines 363 - 364, Update MarketplaceExtensionRuntimeCard to accept the
already-derived facts value as a prop, remove its direct
extensionTrustFacts(extension) call, and pass facts from the parent call site
alongside the other cards so trust facts are computed once and shared
consistently.

web/src/systems/marketplace/components/marketplace-detail-skill-installed.tsx (1)

153-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reuse the shared manage-fallback instead of duplicating it.

MarketplaceSkillManageFallback repeats the layout, spinner and alert icons, testId scheme, error paragraph, and retry button of MarketplaceDetailManageFallbackCard in web/src/systems/marketplace/components/marketplace-detail-manage-state.tsx, which the installed-extension detail already uses. Two copies of the same failure-state contract will diverge. Extract the inner body of the shared component (for example MarketplaceDetailManageFallbackBody) and consume it from both files, keeping the card wrapper optional.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@web/src/systems/marketplace/components/marketplace-detail-skill-installed.tsx`
around lines 153 - 192, Extract the duplicated loading/error content from
MarketplaceSkillManageFallback into a shared component such as
MarketplaceDetailManageFallbackBody in marketplace-detail-manage-state.tsx.
Reuse that body from both MarketplaceDetailManageFallbackCard and
MarketplaceSkillManageFallback, keeping the card wrapper optional while
preserving the existing loading/error states, test IDs, message, error details,
icons, and retry behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes