diff --git a/.project-docs/30-worklog/tasks/20260829-marketplace-mlm06-r4-remediation-b7e3c921.md b/.project-docs/30-worklog/tasks/20260829-marketplace-mlm06-r4-remediation-b7e3c921.md new file mode 100644 index 0000000..976da7e --- /dev/null +++ b/.project-docs/30-worklog/tasks/20260829-marketplace-mlm06-r4-remediation-b7e3c921.md @@ -0,0 +1,103 @@ +# Task: MakeLore Curated Plugin Marketplace Release A R4 Remediation + +## Identity + +- Task ID: 20260829-marketplace-mlm06-r4-remediation-b7e3c921 +- Mode: Feature +- Branch: codex/20260829-marketplace-mlm06-r4-remediation-b7e3c921-marketplace-mlm06-r4-remediation +- Worktree: D:\Datas\OthersProjects\makelore-plugin-marketplace-mlm06-r4-remediation-b7e3c921 +- Base commit: d04b031040a1c109a0c2c5ede2584bb0b75a3bb2 +- Owner: marketplace-client-coordinator-r4-remediator +- Status: Ready for Integration + +## Scope + +- Remediate the three accepted MLM-06 R4 fixed-range findings from exact reviewed + head `d04b031040a1c109a0c2c5ede2584bb0b75a3bb2`: + 1. keep installed local `skill_only` effective when the trusted Library cache is + stale, while server-backed runtime remains fail-closed; + 2. scope Marketplace resolve request identity to one logical sync operation so a + later sync can select a new Release and create a fresh Admission; + 3. project bundled/core Skill ownership collisions as an explicit unavailable + reason instead of silently presenting the Marketplace plugin as ready. +- Product ownership is limited to the existing Marketplace client/Package Store, + effective resolver, Main project projection, their closed shared vocabulary when + required, and focused tests. No Server, Operations, Renderer redesign, Release B, + hosted execution, Token Point, deployment, publication, or production key work. + +## Intent And Constraints + +- `maintain-project-docs` Concurrent and Planning Gates: Passed. Task-context owner, + branch, worktree, and base match exactly; the only initial worktree change is this + task record. Relevant coordinator and R4 review tasks are the same implementation + topic; reviewers are read-only and no concurrent writer owns these files. +- `implement-spec` remediation rule applies: one writer, test-first natural + boundaries, one source commit, repository-local integration, then fresh fixed-base + Standards and Spec review. The user root worktree remains untouched. +- Plan: + 1. Add failing effective-resolver tests for stale Library local-vs-hosted behavior. + 2. Add failing logical-sync identity tests for same-operation retry stability and + later-operation Release/Admission progress; make the identity explicit at the + Package Store boundary rather than content-global. + 3. Add failing collision snapshot/projection tests; introduce one closed reason and + map it to unavailable without fabricating a Skill owner. + 4. Run focused and adjacent Marketplace/Pi/Project Plugins regressions, typecheck, + lint, build/package proof as affected, full required verification, doc/diff + gates, then commit once and complete the task context. + +## Outcome + +- The effective resolver now distinguishes trusted stale Library state by runtime + kind: an already installed/acquired/enabled/assigned `skill_only` keeps its local + immutable Release and Skill, while a server-backed definition remains + `library_unavailable` and contributes no worker resources. +- Resolve content digest and logical operation identity are separated. A new + Marketplace/Package Store sync gets a new `makelore-resolve-` ID; a caller + can persist and replay an explicit ID for the same logical operation, and the + internal authenticated retry reuses the prepared identity. This lets later channel + state and expired Admissions be resolved instead of replaying one content-derived + request forever. +- Marketplace packages that collide with a core, bundled (including Data Service), + or earlier Marketplace Skill owner now emit closed reason + `skill_owner_conflict`. They contribute no Release/Skill resources and Project + Plugins projects them as `unavailable` rather than `ready`; assignments remain + preserved. +- No Server/Operations, project-file schema, package schema, Renderer page redesign, + Release B, hosted runtime, Token Point, deployment, publication, or signing-key + product changes were made. + +## Verification + +- TDD red boundaries: + - resolver/client suites: 5 failures / 34 passes, exactly stale `skill_only`, two + collision reasons, content-derived resolve identity, and missing Package Store + operation identity; + - Project Plugins projection: 1 failure / 5 passes, state was incorrectly `ready`. +- Final owned focused: 3 files / 46 tests passed. +- Marketplace/Main/Pi/Renderer adjacent regression: 17 files / 143 tests passed. +- Full unit suite: 208 files / 1,803 passed / 2 skipped; pressure test 1/1 passed. +- `pnpm run typecheck`: passed. +- scoped ESLint: passed. Full `pnpm run lint:check`: 0 errors and the unchanged five + out-of-scope warnings (Home one; Makelore four). +- `pnpm run build:vite`: passed (Renderer 2,261; Main 193; Preload 1; utility 9). +- Windows Electron Vitest: 2 files / 6 tests passed. +- Marketplace + Project Plugins Electron E2E: first run 2/3 with the known auth + bootstrap ordering failure before Library fetch; the sole failed case reran 1/1 + passed. No R4 product path appeared in the failure. +- `package:stage:win-x64` and Windows Electron builder passed. Packaged + `verify:artifact:pi` returned overall PASS: schema-2 Marketplace proof PASS, + Data Service ten tools, Pi 0.84.2 closure, and only the inherited real-Provider / + cross-platform partial-pass waivers. +- `git diff --check`, project-doc structure, doc drift, task-context completion, clean + source commit, and final clean-HEAD package proof are completed at handoff. + +## Follow-ups + +- Production Marketplace trust activation remains HOLD until the official Ed25519 + public key is supplied. No production private key was generated or committed. +- Release B Provider/hosted execution and XMA-01 remain closed until fresh fixed-base + Standards and Spec review both pass. + +## Promotion Candidates + +- None recorded. diff --git a/electron/api/coding-product-services.ts b/electron/api/coding-product-services.ts index 1800a5c..55f50d8 100644 --- a/electron/api/coding-product-services.ts +++ b/electron/api/coding-product-services.ts @@ -497,7 +497,7 @@ export function createCodingProjectPluginService( if (effective && effectiveReason?.code === 'project_disabled') state = 'disabled'; else if (effective && !hasEffectiveSkill && effectiveReason && ['account_required', 'library_required', 'library_unavailable', 'release_not_installed', - 'release_invalid', 'client_incompatible', 'runtime_suspended', 'policy_unavailable', + 'release_invalid', 'client_incompatible', 'skill_owner_conflict', 'runtime_suspended', 'policy_unavailable', 'policy_unsupported'] .includes(effectiveReason.code)) state = 'unavailable'; return { diff --git a/electron/coding-plugins/effective-resolver.ts b/electron/coding-plugins/effective-resolver.ts index 9be6abe..fda4670 100644 --- a/electron/coding-plugins/effective-resolver.ts +++ b/electron/coding-plugins/effective-resolver.ts @@ -46,6 +46,7 @@ export type PluginUnavailableReasonCode = | 'client_incompatible' | 'project_disabled' | 'skill_unassigned' + | 'skill_owner_conflict' | 'runtime_suspended' | 'policy_unavailable' | 'policy_unsupported' @@ -301,7 +302,14 @@ export class EffectivePluginResolver { } for (const { definition, installed, unavailableReason } of definitions) { - if (blockedMarketplacePlugins.has(definition.id)) continue; + if (blockedMarketplacePlugins.has(definition.id)) { + unavailableReasons.push(unavailable( + definition.id, + 'skill_owner_conflict', + 'Plugin Skill ID conflicts with an existing owner', + )); + continue; + } const selectedSkills = definition.skills.filter(({ id }) => assigned.includes(id)); if (selectedSkills.length === 0) { unavailableReasons.push(unavailable(definition.id, 'skill_unassigned', 'Plugin Skill is not assigned')); @@ -331,7 +339,7 @@ export class EffectivePluginResolver { unavailableReasons.push(unavailable(definition.id, 'account_required', 'Marketplace account is required')); continue; } - if (!library || library.stale) { + if (!library || (library.stale && definition.runtimeKind !== 'skill_only')) { unavailableReasons.push(unavailable(definition.id, 'library_unavailable', 'Marketplace Library is unavailable')); continue; } diff --git a/electron/coding-plugins/marketplace-client.ts b/electron/coding-plugins/marketplace-client.ts index 825cb98..d9bf018 100644 --- a/electron/coding-plugins/marketplace-client.ts +++ b/electron/coding-plugins/marketplace-client.ts @@ -1,5 +1,5 @@ import { Buffer } from 'node:buffer'; -import { createHash } from 'node:crypto'; +import { createHash, randomUUID } from 'node:crypto'; import { WORKS_SQUARE_CONFIG } from '../api/works-config'; import { getValidWorksSquareAccessToken, @@ -733,7 +733,9 @@ function prepareResolveInput(input: ResolveRequest): { const digest = input.resolveRequestDigest ?? derivedDigest; if (!SHA256_PATTERN.test(digest)) fail('marketplace_request_invalid', 'invalid resolveRequestDigest'); const requestedId = input.resolveRequestId ?? input.requestId; - const requestId = validateRequestId(requestedId ?? `makelore-resolve-${derivedDigest}`); + // The digest identifies request content; the ID identifies one logical sync. + // Deriving both from content would replay an expired Admission forever. + const requestId = validateRequestId(requestedId ?? `makelore-resolve-${randomUUID()}`); return { requestId, digest, diff --git a/electron/coding-plugins/package-store.ts b/electron/coding-plugins/package-store.ts index e35fb91..d09b68b 100644 --- a/electron/coding-plugins/package-store.ts +++ b/electron/coding-plugins/package-store.ts @@ -756,7 +756,7 @@ export class PluginPackageStore { makeloreVersion: input.makeloreVersion, channel, installed, - resolveRequestId: input.resolveRequestId, + resolveRequestId: input.resolveRequestId ?? `makelore-resolve-${randomUUID()}`, resolveRequestDigest: input.resolveRequestDigest, }; let resolved: MarketplaceResolveSnapshot; diff --git a/tests/unit/coding-plugin-composition.test.ts b/tests/unit/coding-plugin-composition.test.ts index b1bbeef..e68a3dc 100644 --- a/tests/unit/coding-plugin-composition.test.ts +++ b/tests/unit/coding-plugin-composition.test.ts @@ -186,6 +186,48 @@ describe('coding plugin bounded product service', () => { }); }); + it('projects a Marketplace Skill owner collision as unavailable', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'makelore-plugin-collision-')); + roots.push(root); + await createCodingProjectMetadata(root, { now: '2026-08-29T00:00:00.000Z' }); + const definition = { + ...DATA_SERVICE_PLUGIN_DEFINITION, + id: 'makelore.data-service-shadow', displayName: 'Data Service Shadow', + description: 'Conflicting package', requiresBackend: false, runtimeKind: 'skill_only' as const, + acquisitionMode: 'user_acquired' as const, releaseId: 'release-shadow', + provenance: { source: 'marketplace' as const, packageRoot: 'C:/packages/release-shadow' }, + adapterId: '', operations: [], tools: [], + skills: [{ id: 'data-service', entryPath: 'skills/data-service/SKILL.md', grants: [] }], + surfaces: {}, + }; + const service = createCodingProjectPluginService({ + projects: { getProject: vi.fn().mockResolvedValue({ id: 'local-a', path: root }) }, + projectPlugins: { + getEnabledPluginIds: vi.fn().mockResolvedValue([definition.id]), setEnabled: vi.fn(), + }, + policyClient: { + refresh: vi.fn(), + getState: () => ({ status: 'unavailable' as const, revision: 0, lastVerifiedAt: null, catalog: null }), + }, + adapters: [], definitions: [definition], + effectiveResolver: { + resolve: vi.fn().mockResolvedValue({ + accountSessionId: 'account-a\u00001', projectId: 'local-a', + pluginReleaseIds: [], effectiveSkillIds: [], skillEntries: [], + toolDefinitions: [], runtimePolicies: [], + unavailableReasons: [{ + pluginId: definition.id, code: 'skill_owner_conflict', + message: 'Plugin Skill ID conflicts with an existing owner', + }], + }), + } as never, + }); + + await expect(service.list('local-a')).resolves.toMatchObject({ + items: [{ id: definition.id, enabled: true, state: 'unavailable' }], + }); + }); + it('projects the exact three capabilities and fourteen mixed-policy operations', async () => { const root = await mkdtemp(path.join(tmpdir(), 'makelore-plugin-policy-join-')); roots.push(root); diff --git a/tests/unit/coding-plugin-effective-resolver.test.ts b/tests/unit/coding-plugin-effective-resolver.test.ts index 5c97443..02be35a 100644 --- a/tests/unit/coding-plugin-effective-resolver.test.ts +++ b/tests/unit/coding-plugin-effective-resolver.test.ts @@ -205,7 +205,10 @@ describe('effective plugin resolver', () => { { id: 'shared-skill', entryPath: 'skills/shared/SKILL.md', packageRoot: 'C:/packages/collision-a-1' }, ]); expect(result.pluginReleaseIds).toEqual(['collision-a-1']); - expect(result.unavailableReasons).toEqual([]); + expect(result.unavailableReasons).toEqual(expect.arrayContaining([ + expect.objectContaining({ pluginId: coreCollision.id, code: 'skill_owner_conflict' }), + expect.objectContaining({ pluginId: packageB.id, code: 'skill_owner_conflict' }), + ])); }); it('reserves every bundled Skill owner, including Data Service, before accepting Marketplace packages', async () => { @@ -238,9 +241,10 @@ describe('effective plugin resolver', () => { }); expect(result.pluginReleaseIds).toEqual([]); - expect(result.unavailableReasons).not.toEqual([ - expect.objectContaining({ pluginId: marketplaceDataServiceCollision.id }), - ]); + expect(result.unavailableReasons).toContainEqual(expect.objectContaining({ + pluginId: marketplaceDataServiceCollision.id, + code: 'skill_owner_conflict', + })); }); it('requires current Library, installed Release, project selection, and assignment', async () => { @@ -284,6 +288,40 @@ describe('effective plugin resolver', () => { }); }); + it('keeps installed skill-only local under a trusted stale Library while hosted stays fail-closed', async () => { + const staleNotesLibrary = { ...library(), stale: true }; + await expect(resolve(resolver({ getLibrary: vi.fn(async () => staleNotesLibrary) }))) + .resolves.toMatchObject({ + effectiveSkillIds: ['notes'], + pluginReleaseIds: ['notes-1'], + unavailableReasons: [], + }); + + const staleHostedLibrary = { + ...library(), + stale: true, + items: [{ ...library().items[0], pluginId: serverDefinition.id }], + }; + const hosted = createEffectivePluginResolver({ + definitions: [serverDefinition], + getAccountBinding: () => binding, + getLibrary: vi.fn(async () => staleHostedLibrary), + getInstalled: vi.fn(async () => installed(serverDefinition)), + getEnabledPluginIds: vi.fn(async () => [serverDefinition.id]), + policyClient: { getState: currentPolicy, refresh: vi.fn() }, + }); + await expect(hosted.resolve({ + projectId: 'project-a', projectPath: 'C:/project-a', assignedSkillIds: ['remote'], role: 'parent', + })).resolves.toMatchObject({ + effectiveSkillIds: [], + pluginReleaseIds: [], + unavailableReasons: [expect.objectContaining({ + pluginId: serverDefinition.id, + code: 'library_unavailable', + })], + }); + }); + it('never exposes plugin resources to a child worker', async () => { const result = await resolver().resolve({ projectId: 'project-a', projectPath: 'C:/project-a', assignedSkillIds: ['notes'], role: 'child', diff --git a/tests/unit/coding-plugin-marketplace-client.test.ts b/tests/unit/coding-plugin-marketplace-client.test.ts index 31404c2..736f8cf 100644 --- a/tests/unit/coding-plugin-marketplace-client.test.ts +++ b/tests/unit/coding-plugin-marketplace-client.test.ts @@ -374,7 +374,7 @@ describe('Marketplace client and account cache', () => { }); }); - it('derives stable resolve identity from one logical request and changes it when installed state changes', async () => { + it('creates a new resolve identity per logical sync and preserves an explicit replay identity', async () => { const fetcher = vi.fn().mockImplementation(async (_input, init) => { const request = JSON.parse(init?.body as string) as Record; return response({ @@ -400,9 +400,40 @@ describe('Marketplace client and account cache', () => { await client.resolve(base); const firstId = requestBody(fetcher, 0).resolve_request_id; const secondId = requestBody(fetcher, 1).resolve_request_id; - expect(firstId).toBe(secondId); - await client.resolve({ ...base, installed: [{ pluginId: PLUGIN_ID, releaseId: RELEASE_ID, sha256: SHA256 }] }); - expect(requestBody(fetcher, 2).resolve_request_id).not.toBe(firstId); + expect(secondId).not.toBe(firstId); + expect(requestBody(fetcher, 1).resolve_request_digest).toBe(requestBody(fetcher, 0).resolve_request_digest); + + const replay = { ...base, resolveRequestId: 'makelore-resolve-logical-sync-a' }; + await client.resolve(replay); + await client.resolve(replay); + expect(requestBody(fetcher, 2).resolve_request_id).toBe('makelore-resolve-logical-sync-a'); + expect(requestBody(fetcher, 3).resolve_request_id).toBe('makelore-resolve-logical-sync-a'); + }); + + it('keeps one generated resolve identity across the authenticated retry of a logical sync', async () => { + const fetcher = vi.fn() + .mockResolvedValueOnce(new Response('', { status: 401 })) + .mockImplementationOnce(async (_input, init) => { + const request = JSON.parse(init?.body as string) as Record; + return response({ + resolve_request_id: request.resolve_request_id, + resolve_request_digest: request.resolve_request_digest, + items: [], + catalog_generation: 7, + }, {}, { ETag: '"plugins-7-tp-none"' }); + }); + const client = createMarketplaceClient({ + fetchImpl: fetcher, + apiBaseUrl: 'https://square.example', + getAccessToken: async ({ forceRefresh } = {}) => forceRefresh ? 'refreshed-token' : 'initial-token', + getAccountBinding: () => ACCOUNT_A, + subscribeSession: () => () => undefined, + }); + + await client.resolve({ makeloreVersion: '1.0.0', channel: 'stable', installed: [] }); + + expect(requestBody(fetcher, 1).resolve_request_id).toBe(requestBody(fetcher, 0).resolve_request_id); + expect(requestBody(fetcher, 1).resolve_request_digest).toBe(requestBody(fetcher, 0).resolve_request_digest); }); it('rejects a response body above the bounded DTO limit', async () => { @@ -505,6 +536,38 @@ describe('PluginPackageStore', () => { expect(issueDownload).toHaveBeenCalledWith({ releaseId: RELEASE_ID, releaseAdmissionId: ADMISSION_ID }); }); + it('assigns a distinct logical resolve identity to each new Package Store sync', async () => { + temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-')); + const archive = buildSkillOnlyArchive(); + const { grant, publicKey } = signedGrant(archive); + const resolve = vi.fn(async (input: ResolveRequest) => makeResolveResult(input, { + sha256: grant.sha256, + sizeBytes: grant.sizeBytes, + })); + const marketplace: MarketplaceClient = { + resolve, + issueDownload: vi.fn(async () => grant), + downloadContent: async () => archive, + getCurrentAccountBinding: () => ACCOUNT_A, + } as MarketplaceClient; + const store = new PluginPackageStore({ + rootDir: temporaryRoot, + marketplace, + clientVersion: '1.0.0', + keyStore: new Map([['test-key', publicKey]]), + getAccountBinding: () => ACCOUNT_A, + }); + + await store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' }); + await store.resolveAndInstall({ pluginId: PLUGIN_ID, makeloreVersion: '1.0.0' }); + + const first = resolve.mock.calls[0]?.[0].resolveRequestId; + const second = resolve.mock.calls[1]?.[0].resolveRequestId; + expect(first).toMatch(/^makelore-resolve-/u); + expect(second).toMatch(/^makelore-resolve-/u); + expect(second).not.toBe(first); + }); + it('removes only releases with no account or active-worker reference', async () => { temporaryRoot = await mkdtemp(path.join(process.cwd(), '.marketplace-test-')); const archive = buildSkillOnlyArchive();