From e2fd38d17af91d50de5a3ec53000e355e2dd8b00 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Thu, 3 Sep 2026 09:18:24 -0700 Subject: [PATCH] [eric] marketplace: Get installs on one click unless the review blocks or a key is needed; the review sheet leads with one line and folds 94 import findings behind Details Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01C9zwUaHucUgrdxvK8FvjYT --- .../src/app/components/share/ImportModal.tsx | 11 +---- .../app/components/share/ReviewFindings.tsx | 46 +++++++++++++++++++ .../share/marketplaceNeedsConfirm.test.ts | 35 ++++++++++++++ .../share/marketplaceNeedsConfirm.ts | 11 +++++ .../pages/Directory/DirectoryPackagesTab.tsx | 4 +- .../pages/Directory/packages/catalog.test.ts | 5 +- 6 files changed, 99 insertions(+), 13 deletions(-) create mode 100644 frontend/src/app/components/share/ReviewFindings.tsx create mode 100644 frontend/src/app/components/share/marketplaceNeedsConfirm.test.ts create mode 100644 frontend/src/app/components/share/marketplaceNeedsConfirm.ts diff --git a/frontend/src/app/components/share/ImportModal.tsx b/frontend/src/app/components/share/ImportModal.tsx index b5121a8c..13b81bb7 100644 --- a/frontend/src/app/components/share/ImportModal.tsx +++ b/frontend/src/app/components/share/ImportModal.tsx @@ -7,11 +7,11 @@ import Button from '@mui/material/Button'; import IconButton from '@mui/material/IconButton'; import CircularProgress from '@mui/material/CircularProgress'; import CloseIcon from '@mui/icons-material/Close'; -import ShieldOutlinedIcon from '@mui/icons-material/ShieldOutlined'; import { useClaudeTokens } from '@/shared/styles/ThemeContext'; import IncludesList from './IncludesList'; +import ReviewFindings from './ReviewFindings'; import { ImportPreflight } from './shareTypes'; interface Props { @@ -52,14 +52,7 @@ const ImportModal: React.FC = ({ preflight, open, committing, onConfirm, - {preflight.review && preflight.review.findings.length > 0 && ( - - - - {preflight.review.findings.join(' ')} - - - )} + {preflight.review && preflight.review.findings.length > 0 && } {preflight.conflicts.length > 0 && ( Some items already exist and will be added as copies. diff --git a/frontend/src/app/components/share/ReviewFindings.tsx b/frontend/src/app/components/share/ReviewFindings.tsx new file mode 100644 index 00000000..35d6ea12 --- /dev/null +++ b/frontend/src/app/components/share/ReviewFindings.tsx @@ -0,0 +1,46 @@ +import React, { useState } from 'react'; +import Box from '@mui/material/Box'; +import Typography from '@mui/material/Typography'; +import KeyboardArrowDownIcon from '@mui/icons-material/KeyboardArrowDown'; +import ShieldOutlinedIcon from '@mui/icons-material/ShieldOutlined'; +import { useClaudeTokens } from '@/shared/styles/ThemeContext'; +import type { ReviewSummary } from './shareTypes'; + +// The review's first line is the one that matters ("this app runs code on your computer"); the rest is one entry per flagged import, 94 of them on an ordinary FastAPI app, and used to land as a single 10,000-character paragraph. +const ReviewFindings: React.FC<{ review: ReviewSummary }> = ({ review }) => { + const c = useClaudeTokens(); + const [open, setOpen] = useState(false); + const [lead, ...rest] = review.findings; + if (!lead) return null; + const tone = review.verdict === 'block' ? c.status.error : c.status.warning; + return ( + + + + {lead} + + {rest.length > 0 && ( + <> + setOpen((v) => !v)} + sx={{ mt: 0.75, ml: 3, display: 'inline-flex', alignItems: 'center', gap: 0.25, color: c.text.tertiary, cursor: 'pointer', '&:hover': { color: c.accent.primary } }} + > + {open ? 'Hide details' : `Details (${rest.length})`} + + + {open && ( + + {rest.map((f, i) => ( + + {f} + + ))} + + )} + + )} + + ); +}; + +export default ReviewFindings; diff --git a/frontend/src/app/components/share/marketplaceNeedsConfirm.test.ts b/frontend/src/app/components/share/marketplaceNeedsConfirm.test.ts new file mode 100644 index 00000000..f1cf7fb9 --- /dev/null +++ b/frontend/src/app/components/share/marketplaceNeedsConfirm.test.ts @@ -0,0 +1,35 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { marketplaceNeedsConfirm } from './marketplaceNeedsConfirm'; +import { importNeedsConfirm } from './importNeedsConfirm'; +import type { ImportPreflight } from './shareTypes'; + +// Measured 2026-09-03 on the real Git Graph listing: verdict "warn" with 95 findings, 94 of them "imports X (outside the safe-data-shaping allowlist)", zero requirements. The App Store shows no sheet for that; we showed a 918px one. +const base = (over: Partial = {}): ImportPreflight => ({ + ok: true, staging_token: 't', conflicts: [], warnings: [], + summary: { root: { type: 'app', name: 'Git Graph' }, includes: [], requirements: [], counts: {} }, + review: { verdict: 'warn', findings: ['This app runs code on your computer.', ...Array.from({ length: 94 }, (_, i) => `f${i}: Imports os`)], scanned_files: [] }, + ...over, +} as ImportPreflight); + +test('an ordinary app with import warnings installs on Get alone', () => { + assert.equal(marketplaceNeedsConfirm(base()), false); + assert.equal(importNeedsConfirm(base()), true, 'the dropped-file rule stays stricter'); +}); + +test('a blocked review still stops the install', () => { + assert.equal(marketplaceNeedsConfirm(base({ review: { verdict: 'block', findings: ['Reads your keychain'], scanned_files: [] } })), true); +}); + +test('a need the user must supply by hand still gets a sheet', () => { + const pf = base(); pf.summary.requirements = [{ kind: 'api_key', key: 'k', label: 'OpenAI key' }] as ImportPreflight['summary']['requirements']; + assert.equal(marketplaceNeedsConfirm(pf), true); +}); + +test('the review sheet never joins findings into one paragraph', () => { + const src = fs.readFileSync(path.join(process.cwd(), 'src/app/components/share/ImportModal.tsx'), 'utf8'); + assert.ok(!src.includes("findings.join("), 'findings joined into a wall of text again'); + assert.ok(src.includes(' r.kind === 'api_key' || r.kind === 'mcp_action'); + return blocked || needsHand; +} diff --git a/frontend/src/app/pages/Directory/DirectoryPackagesTab.tsx b/frontend/src/app/pages/Directory/DirectoryPackagesTab.tsx index aa54bd54..508ed66e 100644 --- a/frontend/src/app/pages/Directory/DirectoryPackagesTab.tsx +++ b/frontend/src/app/pages/Directory/DirectoryPackagesTab.tsx @@ -13,7 +13,7 @@ import { fetchOutputs } from '@/shared/state/outputsSlice'; import { fetchWorkflows } from '@/shared/state/workflowsSlice'; import { addViewCard, openWorkflowsApp } from '@/shared/state/dashboardLayoutSlice'; import ImportModal from '@/app/components/share/ImportModal'; -import { importNeedsConfirm } from '@/app/components/share/importNeedsConfirm'; +import { marketplaceNeedsConfirm } from '@/app/components/share/marketplaceNeedsConfirm'; import { importCommit } from '@/app/components/share/shareApi'; import type { ImportPreflight } from '@/app/components/share/shareTypes'; import DirectoryFilterBar from './DirectoryFilterBar'; @@ -131,7 +131,7 @@ const DirectoryPackagesTab: React.FC<{ onOpenSkill?: (skillId: string) => void } setInstallingId(listingId); try { const preflight = await stagePackageInstall(listingId); - if (importNeedsConfirm(preflight)) setConfirm({ preflight, listingId }); + if (marketplaceNeedsConfirm(preflight)) setConfirm({ preflight, listingId }); else await commit(preflight, listingId); } catch (e: unknown) { setToast({ message: e instanceof Error ? e.message : "We couldn't download this package.", severity: 'error' }); diff --git a/frontend/src/app/pages/Directory/packages/catalog.test.ts b/frontend/src/app/pages/Directory/packages/catalog.test.ts index 51cde0f6..d96bf2fa 100644 --- a/frontend/src/app/pages/Directory/packages/catalog.test.ts +++ b/frontend/src/app/pages/Directory/packages/catalog.test.ts @@ -62,10 +62,11 @@ test('Packages is the default marketplace tab and the old skills store is gone', // Install must not grow a second write path; it stages and lets the shared confirm surface decide. test('the packages tab installs through the shared bundle import, not its own writer', () => { const tab = fs.readFileSync(path.join(process.cwd(), 'src/app/pages/Directory/DirectoryPackagesTab.tsx'), 'utf8'); - assert.match(tab, /importNeedsConfirm/); + // The store's own gate (block or a hand-supplied need), asked before the shared commit; a dropped file keeps the stricter importNeedsConfirm. + assert.match(tab, /marketplaceNeedsConfirm/); assert.match(tab, /importCommit/); assert.match(tab, / 0 && commit > gate, 'the confirm gate is asked BEFORE anything is committed'); });