Skip to content

Commit 699c0ff

Browse files
fix(install): retain the marketplace on unproven dependents and inventory receipts without a host probe
Codex review on #452: - uninstall: a same-id Claude row at another scope is a marketplace dependent (marketplace remove applies to every scope); a failed dependency re-read of plugin list --json is 'unknown' and retains the marketplace instead of reading as an empty inventory. - doctor: the receipt store is inventoried from disk even when the host executable cannot be probed; the host cross-check alone decides consistent/orphaned, otherwise unknown.
1 parent cd30520 commit 699c0ff

5 files changed

Lines changed: 123 additions & 31 deletions

File tree

‎docs/diagnostics.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -821,8 +821,11 @@ open issue). Every mutation is opt-in and bounded by the receipt:
821821
fails closed, `AB7004`), compares the cached copy with the receipt, runs
822822
`claude plugin uninstall <id> --scope <scope> --keep-data` /
823823
`codex plugin remove <id>`, then `plugin marketplace remove <marketplace>`
824-
unless another installed plugin still names that marketplace (`retained`),
825-
and removes the store receipt. A registration the host no longer holds is
824+
and removes the store receipt. Because `plugin marketplace remove` applies
825+
to every scope, the marketplace is `retained` when another installed plugin
826+
still names it, when the same plugin is installed at another Claude scope,
827+
or when the dependency re-read of `plugin list --json` fails (a failed read
828+
is not proof that nothing depends on it). A registration the host no longer holds is
826829
`already-absent`, so a receipt orphaned behind Agent Bundle's back is
827830
consumed without running any host verb.
828831

‎packages/agent-bundle/src/install/doctor.ts‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2215,19 +2215,19 @@ const doctorHost = async (
22152215
? { diagnostics: Object.freeze([]), inventory: freezeInventory('skipped') }
22162216
: publicHostInventory(host, listing, environment, home);
22172217
const diagnostics = [...probed.diagnostics, ...inventoried.diagnostics];
2218-
// Store receipts are lifecycle evidence Agent Bundle itself wrote; each is cross-checked against the
2219-
// host so an orphaned receipt (registration gone) is diagnosed instead of silently trusted.
2220-
const receipts = probed.probe.status !== 'available'
2221-
? { diagnostics: Object.freeze([]), receipts: Object.freeze([]) }
2222-
: host === 'cursor'
2223-
? await inspectStoreReceipts(host, join(home, '.cursor'), async (receipt) => receipt.mode === 'marketplace'
2224-
? (await exists(join(cursorMarketplaceRoot(join(home, '.cursor')), receipt.plugin)) ? 'consistent' : 'orphaned')
2225-
: 'unknown')
2226-
: await inspectStoreReceipts(
2227-
host,
2228-
publicHostRoot(host, environment, home),
2229-
async (receipt) => receiptRegistrationState(host, receipt, listing),
2230-
);
2218+
// Store receipts are lifecycle evidence Agent Bundle itself wrote, so the store is inventoried from
2219+
// the filesystem whether or not the host can be probed: malformed and migrated receipts are always
2220+
// reported. The host cross-check that separates `consistent` from `orphaned` needs the host's
2221+
// inventory; without it the registration state is `unknown`, never guessed.
2222+
const receipts = host === 'cursor'
2223+
? await inspectStoreReceipts(host, join(home, '.cursor'), async (receipt) => receipt.mode === 'marketplace'
2224+
? (await exists(join(cursorMarketplaceRoot(join(home, '.cursor')), receipt.plugin)) ? 'consistent' : 'orphaned')
2225+
: 'unknown')
2226+
: await inspectStoreReceipts(
2227+
host,
2228+
publicHostRoot(host, environment, home),
2229+
async (receipt) => receiptRegistrationState(host, receipt, listing),
2230+
);
22312231
diagnostics.push(...receipts.diagnostics);
22322232
let bundle: DoctorHostReport['bundle'];
22332233
if (options.from !== undefined) {

‎packages/agent-bundle/src/install/uninstall.ts‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -716,32 +716,45 @@ const marketplaceRegistered = async (
716716
: 'absent';
717717
};
718718

719-
/** Installed rows for other plugins from the same marketplace: the marketplace registration then stays. */
720-
const otherPluginsFromMarketplace = async (
719+
/**
720+
* Installed rows that still depend on the marketplace once the `id`@`scope` copy
721+
* is gone: other plugins from the same marketplace, plus (Claude) the same plugin
722+
* installed at another scope, because `plugin marketplace remove` unregisters the
723+
* marketplace for every scope. A second `plugin list --json` that cannot be read
724+
* is `'unknown'`, never an empty list: a failed read is not proof that nothing
725+
* depends on the marketplace, so the caller retains it (fail-closed).
726+
*/
727+
const marketplaceDependents = async (
721728
runner: InstallCommandRunner,
722729
identity: PluginIdentity,
723730
host: Exclude<InstallHost, 'cursor'>,
724731
marketplace: string,
725732
id: string,
726-
): Promise<readonly string[]> => {
733+
scope: InstallScope,
734+
): Promise<readonly string[] | 'unknown'> => {
727735
try {
728736
const result = await runner.run(host, ['plugin', 'list', '--json'], { cwd: identity.bundleRoot });
729-
if (result.code !== 0) return [];
737+
if (result.code !== 0) return 'unknown';
730738
const document = JSON.parse(result.stdout) as unknown;
731739
const rows = host === 'claude'
732740
? document
733741
: typeof document === 'object' && document !== null ? (document as { readonly installed?: unknown }).installed : undefined;
734-
if (!Array.isArray(rows)) return [];
735-
const ids: string[] = [];
742+
if (!Array.isArray(rows)) return 'unknown';
743+
const dependents: string[] = [];
736744
for (const row of rows) {
737745
if (typeof row !== 'object' || row === null) continue;
738-
const candidate = (row as { readonly id?: unknown; readonly pluginId?: unknown });
746+
const candidate = (row as { readonly id?: unknown; readonly pluginId?: unknown; readonly scope?: unknown });
739747
const rowId = host === 'claude' ? candidate.id : candidate.pluginId;
740-
if (typeof rowId === 'string' && rowId !== id && rowId.endsWith(`@${marketplace}`)) ids.push(rowId);
748+
if (typeof rowId !== 'string' || !rowId.endsWith(`@${marketplace}`)) continue;
749+
if (rowId !== id) {
750+
dependents.push(rowId);
751+
} else if (host === 'claude' && typeof candidate.scope === 'string' && candidate.scope !== scope) {
752+
dependents.push(`${rowId} (scope ${candidate.scope})`);
753+
}
741754
}
742-
return ids;
755+
return dependents;
743756
} catch {
744-
return [];
757+
return 'unknown';
745758
}
746759
};
747760

@@ -896,7 +909,10 @@ const uninstallPublicCli = async (
896909
const marketplaceState = marketplaceRegistration === undefined
897910
? 'absent'
898911
: await marketplaceRegistered(runner, identity, host, marketplace);
899-
const sharedBy = marketplaceState === 'absent' ? [] : await otherPluginsFromMarketplace(runner, identity, host, marketplace, id);
912+
const dependents = marketplaceState === 'absent'
913+
? []
914+
: await marketplaceDependents(runner, identity, host, marketplace, id, scope);
915+
const retainMarketplace = dependents === 'unknown' || dependents.length > 0;
900916
const planned = options.plan === true;
901917
const data = await publicHostData(host, policy, entry, hostRoot, id);
902918
const registrations: UninstallRegistrationReport[] = [];
@@ -914,12 +930,16 @@ const uninstallPublicCli = async (
914930
...marketplaceRegistration,
915931
action: marketplaceState === 'absent'
916932
? 'already-absent'
917-
: sharedBy.length > 0 ? 'retained' : planned ? 'planned' : 'removed',
933+
: retainMarketplace ? 'retained' : planned ? 'planned' : 'removed',
918934
detail: marketplaceState === 'absent'
919935
? `${host} no longer lists marketplace ${marketplace}.`
920-
: sharedBy.length > 0
921-
? `Marketplace ${marketplace} stays registered: ${sharedBy.join(', ')} still install from it.`
922-
: `\`${host} ${publicHostMarketplaceRemoveArguments(marketplace).join(' ')}\``,
936+
: dependents === 'unknown'
937+
? `Marketplace ${marketplace} stays registered: \`${host} plugin list --json\` could not be re-read to prove ` +
938+
'nothing else installs from it, and `plugin marketplace remove` applies to every scope. Remove it by hand once ' +
939+
'the inventory is readable.'
940+
: dependents.length > 0
941+
? `Marketplace ${marketplace} stays registered: ${dependents.join(', ')} still install from it.`
942+
: `\`${host} ${publicHostMarketplaceRemoveArguments(marketplace).join(' ')}\``,
923943
}));
924944
}
925945
const files = receipt === undefined && !await exists(receiptPath) ? [] : [receiptPath];
@@ -944,7 +964,7 @@ const uninstallPublicCli = async (
944964
if (entry !== undefined) {
945965
await runHostCommand(runner, identity, host, publicHostUninstallArguments(host, id, scope), 'removal');
946966
}
947-
if (marketplaceRegistration !== undefined && marketplaceState !== 'absent' && sharedBy.length === 0) {
967+
if (marketplaceRegistration !== undefined && marketplaceState !== 'absent' && !retainMarketplace) {
948968
await runHostCommand(runner, identity, host, publicHostMarketplaceRemoveArguments(marketplace), 'removal');
949969
}
950970
const purged: string[] = [];

‎packages/agent-bundle/tests/doctor.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,20 @@ it('inventories store receipts, diagnoses orphaned ones (AB7326), and reports pr
15201520
await writeFile(join(claudeConfig, 'agent-bundle', 'receipts', 'broken.user.json'), '{"format":"nope"}\n');
15211521
const broken = hostReport(await doctor(), 'claude');
15221522
expect(broken.diagnostics.filter((entry) => entry.code === 'AB7326').some((entry) => entry.message.includes('not a valid install receipt'))).toBe(true);
1523+
1524+
// The host executable cannot be probed at all: the store is still inventoried from disk, so the
1525+
// stored receipt (state unknown) and the malformed file are reported instead of hidden.
1526+
const absentHost: DoctorCommandRunner = async () => commandResult({ exitCode: 127, stderr: 'claude: command not found' });
1527+
const unprobed = hostReport(await runDoctor({
1528+
commandRunner: absentHost,
1529+
endpointDirectory: fixture.endpointDirectory,
1530+
environment: { CLAUDE_CONFIG_DIR: claudeConfig },
1531+
home: fixture.home,
1532+
hosts: ['claude'],
1533+
}), 'claude');
1534+
expect(unprobed.probe.status).not.toBe('available');
1535+
expect(unprobed.receipts).toEqual([expect.objectContaining({ path: result.receipt, state: 'unknown' })]);
1536+
expect(unprobed.diagnostics.filter((entry) => entry.code === 'AB7326').some((entry) => entry.message.includes('not a valid install receipt'))).toBe(true);
15231537
await rm(join(claudeConfig, 'agent-bundle', 'receipts', 'broken.user.json'));
15241538

15251539
// A Cursor local copy whose receipt predates format/2 is diagnosed as migrated, never rewritten by Doctor.

‎packages/agent-bundle/tests/uninstall.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,61 @@ it.each([
565565
expect(shared.calls.map((call) => call.args.join(' '))).toContain(uninstall);
566566
installed = false;
567567

568+
// The dependency re-read fails after the first inventory succeeded: a failed read is not proof that
569+
// nothing depends on the marketplace, so the marketplace is retained (fail-closed), never removed.
570+
await installBundle(options);
571+
let listCalls = 0;
572+
const flaky: { readonly calls: CommandCall[]; readonly runner: InstallCommandRunner } = { calls: [], runner: {
573+
run: async (command, args) => {
574+
const call = { args: [...args], command };
575+
flaky.calls.push(call);
576+
const verb = args.join(' ');
577+
if (verb === 'plugin list --json') {
578+
listCalls += 1;
579+
return listCalls === 1
580+
? { code: 0, stderr: '', stdout: listing(true, installPath) }
581+
: { code: 1, stderr: 'transient failure', stdout: '' };
582+
}
583+
if (verb === 'plugin marketplace list --json') return { code: 0, stderr: '', stdout: marketplaces(true) };
584+
return { code: 0, stderr: '', stdout: '' };
585+
},
586+
} };
587+
const unproven = await uninstallBundle({ ...options, commandRunner: flaky.runner });
588+
expect(unproven).toMatchObject({
589+
registrations: [
590+
{ action: 'removed', kind: `${host}-plugin` },
591+
{ action: 'retained', detail: expect.stringContaining('could not be re-read'), kind: `${host}-marketplace` },
592+
],
593+
state: 'uninstalled',
594+
});
595+
expect(flaky.calls.map((call) => call.args.join(' '))).not.toContain(removeMarketplace);
596+
installed = false;
597+
598+
if (host === 'claude') {
599+
// The same plugin installed at another Claude scope still depends on the marketplace, and
600+
// `plugin marketplace remove` applies to every scope: the marketplace is retained.
601+
await installBundle(options);
602+
const scoped = recordingRunner((call) => {
603+
const verb = call.args.join(' ');
604+
if (verb === 'plugin list --json') {
605+
return claudeListing([
606+
{ enabled: true, id: 'uninstall-fixture@uninstall-fixture-marketplace', installPath, scope: 'user', version: '1.2.3' },
607+
{ enabled: true, id: 'uninstall-fixture@uninstall-fixture-marketplace', installPath, scope: 'project', version: '1.2.3' },
608+
]);
609+
}
610+
if (verb === 'plugin marketplace list --json') return marketplaces(true);
611+
return '';
612+
});
613+
const otherScope = await uninstallBundle({ ...options, commandRunner: scoped.runner });
614+
expect(otherScope.registrations.find((registration) => registration.kind === 'claude-marketplace')).toMatchObject({
615+
action: 'retained',
616+
detail: expect.stringContaining('uninstall-fixture@uninstall-fixture-marketplace (scope project)'),
617+
});
618+
expect(scoped.calls.map((call) => call.args.join(' '))).toContain(uninstall);
619+
expect(scoped.calls.map((call) => call.args.join(' '))).not.toContain(removeMarketplace);
620+
installed = false;
621+
}
622+
568623
// An orphaned receipt (host already forgot the plugin) is consumed without any host verb.
569624
await installBundle(options);
570625
installed = false;

0 commit comments

Comments
 (0)