mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-06 09:47:44 +02:00
[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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C9zwUaHucUgrdxvK8FvjYT
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
bc1e7acac9
commit
e2fd38d17a
@@ -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<Props> = ({ preflight, open, committing, onConfirm,
|
||||
</Box>
|
||||
<Box sx={{ px: 3, pb: 3 }}>
|
||||
<IncludesList summary={preflight.summary} />
|
||||
{preflight.review && preflight.review.findings.length > 0 && (
|
||||
<Box sx={{ mt: 1.75, display: 'flex', gap: 0.85, alignItems: 'center' }}>
|
||||
<ShieldOutlinedIcon sx={{ fontSize: 15, color: c.status.warning, flexShrink: 0 }} />
|
||||
<Typography sx={{ fontSize: '0.75rem', color: c.text.muted, lineHeight: 1.4 }}>
|
||||
{preflight.review.findings.join(' ')}
|
||||
</Typography>
|
||||
</Box>
|
||||
)}
|
||||
{preflight.review && preflight.review.findings.length > 0 && <ReviewFindings review={preflight.review} />}
|
||||
{preflight.conflicts.length > 0 && (
|
||||
<Typography sx={{ fontSize: '0.75rem', color: c.text.muted, mt: 1.5 }}>
|
||||
Some items already exist and will be added as copies.
|
||||
|
||||
@@ -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 (
|
||||
<Box sx={{ mt: 1.75 }}>
|
||||
<Box sx={{ display: 'flex', gap: 0.85, alignItems: 'flex-start' }}>
|
||||
<ShieldOutlinedIcon sx={{ fontSize: 15, color: tone, flexShrink: 0, mt: '1px' }} />
|
||||
<Typography sx={{ fontSize: '0.8125rem', color: c.text.secondary, lineHeight: 1.45 }}>{lead}</Typography>
|
||||
</Box>
|
||||
{rest.length > 0 && (
|
||||
<>
|
||||
<Box
|
||||
onClick={() => 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 } }}
|
||||
>
|
||||
<Typography sx={{ fontSize: '0.75rem' }}>{open ? 'Hide details' : `Details (${rest.length})`}</Typography>
|
||||
<KeyboardArrowDownIcon sx={{ fontSize: 14, transform: open ? 'rotate(180deg)' : 'none', transition: 'transform 0.18s' }} />
|
||||
</Box>
|
||||
{open && (
|
||||
<Box sx={{ mt: 0.5, ml: 3, maxHeight: 200, overflowY: 'auto', pr: 0.5 }}>
|
||||
{rest.map((f, i) => (
|
||||
<Typography key={i} sx={{ fontSize: '0.75rem', color: c.text.muted, lineHeight: 1.5, py: 0.15, fontFamily: c.font.mono, wordBreak: 'break-word' }}>
|
||||
{f}
|
||||
</Typography>
|
||||
))}
|
||||
</Box>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</Box>
|
||||
);
|
||||
};
|
||||
|
||||
export default ReviewFindings;
|
||||
@@ -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> = {}): 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('<ReviewFindings'), 'the findings component is gone');
|
||||
});
|
||||
@@ -0,0 +1,11 @@
|
||||
import type { ImportPreflight } from './shareTypes';
|
||||
|
||||
// A Get on a named, labelled listing is already the user's decision, the way the App Store treats
|
||||
// a tap on GET; a second sheet that everyone clicks through protects nobody. The only things worth
|
||||
// a stop are a review that BLOCKS (not the import-allowlist warnings every real app trips) and a
|
||||
// need the user must supply by hand (a key, a connector). A dropped file keeps importNeedsConfirm.
|
||||
export function marketplaceNeedsConfirm(pf: ImportPreflight): boolean {
|
||||
const blocked = !!pf.review && pf.review.verdict === 'block';
|
||||
const needsHand = pf.summary.requirements.some((r) => r.kind === 'api_key' || r.kind === 'mcp_action');
|
||||
return blocked || needsHand;
|
||||
}
|
||||
@@ -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' });
|
||||
|
||||
@@ -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, /<ImportModal/);
|
||||
const gate = tab.indexOf('importNeedsConfirm(preflight)');
|
||||
const gate = tab.indexOf('marketplaceNeedsConfirm(preflight)');
|
||||
const commit = tab.indexOf('else await commit(preflight');
|
||||
assert.ok(gate > 0 && commit > gate, 'the confirm gate is asked BEFORE anything is committed');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user