fix(plugins): stop automatic detail retry loop
This commit is contained in:
1 parent
0bfabc0df2
commit
d7058e6383
3 files changed
+189
-1
No files matched your search
@@ -0,0 +1,112 @@
|
|||||||
|
# Task: Plugin navigation R3 detail retry remediation
|
||||||
|
|
||||||
|
## Identity
|
||||||
|
|
||||||
|
- Task ID: 20260903-plugin-navigation-remediation-r3-f3a8c4d1
|
||||||
|
- Mode: Feature
|
||||||
|
- Branch: codex/20260903-plugin-navigation-remediation-r3-f3a8c4d1-plugin-navigation-remediation-r3
|
||||||
|
- Worktree: C:\Users\7brot\.codex\worktrees\plugin-nav-remediation-r3-f3a8c4d1\makelore
|
||||||
|
- Base commit: 0bfabc0df24fdd41c37af498f7a4e49bc00d31db
|
||||||
|
- Owner: codex-01a0656d
|
||||||
|
- Status: Ready for Integration
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
- Remediate the R3 detail-loading retry loop against exact product base
|
||||||
|
`0bfabc0df24fdd41c37af498f7a4e49bc00d31db`.
|
||||||
|
- Make automatic official-detail loading selection-scoped: at most one automatic
|
||||||
|
request per detail open, a stable local error after rejection, and one new
|
||||||
|
attempt after explicitly closing and reopening the same detail.
|
||||||
|
- Add a public Renderer/page regression for reject/settle and close/reopen, while
|
||||||
|
preserving the existing A-to-B late-detail protection.
|
||||||
|
- Produce one remediation commit and a clean READY_FOR_INTEGRATION handoff.
|
||||||
|
|
||||||
|
## Intent And Constraints
|
||||||
|
|
||||||
|
- Follow the authoritative ML-PLUGIN-NAV-001 Spec, especially section 5.4 and
|
||||||
|
ERR-001/ERR-005 in section 7.2, plus the exact R3 acceptance finding supplied
|
||||||
|
by the source task.
|
||||||
|
- Work test-first through the public `Plugins` Renderer/page seam.
|
||||||
|
- Limit product edits to `src/pages/Plugins/index.tsx` and the necessary focused
|
||||||
|
plugin page test. This task record is the only project-document write owned by
|
||||||
|
the task.
|
||||||
|
- Do not modify stores, Electron Main, server, runtime, billing, router, Login,
|
||||||
|
dependencies, or any other worktree/root checkout.
|
||||||
|
- Do not add a cache, persistent authority, compatibility layer, feature flag,
|
||||||
|
broad retry framework, or speculative UI.
|
||||||
|
- Do not merge main, push, create a PR, deploy, publish, or install the app.
|
||||||
|
|
||||||
|
## Project Context Loaded
|
||||||
|
|
||||||
|
- Product goal: close the R3 P1 finding without changing the unified plugin
|
||||||
|
workspace's authority boundaries or its existing R1/R2 behavior.
|
||||||
|
- Current state: exact HEAD and merge-base are
|
||||||
|
`0bfabc0df24fdd41c37af498f7a4e49bc00d31db`; only this newly created task
|
||||||
|
record is untracked. Marketplace `loadDetail` sets `loading`, then `error` and
|
||||||
|
rejects on a supported request failure. The current page effect observes that
|
||||||
|
error and immediately calls it again.
|
||||||
|
- Relevant decisions: official detail is loaded on demand; source failures must
|
||||||
|
remain local and stable; Renderer must not create a second cache or retry
|
||||||
|
authority; stale A detail must never replace selected B.
|
||||||
|
- Evidence inspected: the complete ML-PLUGIN-NAV-001 Spec, current project
|
||||||
|
memory, the R2 task record, `src/pages/Plugins/index.tsx`, the Marketplace
|
||||||
|
store detail transition, and existing public page/model tests.
|
||||||
|
- Likely modules: `src/pages/Plugins/index.tsx` and
|
||||||
|
`tests/unit/plugins-page.test.tsx` only.
|
||||||
|
- Risks: an already-loading detail must count as the current open's automatic
|
||||||
|
attempt so a later failure cannot trigger a compensating request; changing or
|
||||||
|
clearing selection must reset the attempt boundary without weakening store
|
||||||
|
generation protection.
|
||||||
|
- Concurrent task assessment: task_context identity, worktree, branch, and base
|
||||||
|
match exactly. Remaining Planning peers are old Marketplace/ML-07 reviews or
|
||||||
|
unrelated modules and declare no ownership of this page/test or retry
|
||||||
|
semantic. Concurrent Task Gate: Passed. Planning Gate: Passed.
|
||||||
|
|
||||||
|
## Outcome
|
||||||
|
|
||||||
|
- `Plugins` now records the official detail selection's automatic attempt before
|
||||||
|
observing later `loading` or `error` state. A rejected request therefore
|
||||||
|
settles as one stable local detail failure instead of immediately starting an
|
||||||
|
unbounded retry loop.
|
||||||
|
- Clearing selection resets that local attempt boundary. Closing and reopening
|
||||||
|
the same official detail permits exactly one new automatic request; changing
|
||||||
|
A to B likewise establishes B's own attempt without changing Marketplace
|
||||||
|
store generation or late-result protection.
|
||||||
|
- The public page regression drives the real route composition, reproduces the
|
||||||
|
supported store `error` transition, proves one call while open, observes the
|
||||||
|
existing detail error surface, closes the dialog, and proves one additional
|
||||||
|
call only after reopening.
|
||||||
|
- No store, Electron Main, server, runtime, billing, router, Login, dependency,
|
||||||
|
or unrelated product file was changed.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
- TDD RED: `tests/unit/plugins-page.test.tsx` reported the new contract failure
|
||||||
|
exactly: `loadDetail` expected 1 call after the detail error settled but
|
||||||
|
received 2 (13 existing tests passed, 1 new test failed).
|
||||||
|
- TDD GREEN: `tests/unit/plugins-page.test.tsx` passed 14/14.
|
||||||
|
- Focused plugin suite (`plugins-page`, projection model, query, controller):
|
||||||
|
4 files / 39 tests passed, including the existing A-to-B late-detail model
|
||||||
|
regression and the prior R1/R2 behavior.
|
||||||
|
- `pnpm run typecheck`: passed.
|
||||||
|
- `pnpm run lint:check`: passed with 0 errors and 5 pre-existing warnings in
|
||||||
|
`src/pages/Home/index.tsx` and `src/pages/Makelore/index.tsx`, outside this
|
||||||
|
diff.
|
||||||
|
- `pnpm test`: 225 files / 1842 tests passed / 2 skipped; the separately run
|
||||||
|
coding-chat pressure test passed 1/1.
|
||||||
|
- `pnpm run build:vite`: Renderer, Electron Main, Preload, and utility worker
|
||||||
|
builds passed. Existing Browserslist age, mixed static/dynamic import, and
|
||||||
|
large-chunk warnings remained non-failing.
|
||||||
|
- Targeted Electron E2E (`plugin-marketplace.spec.ts` and
|
||||||
|
`project-plugins.spec.ts`): 4/4 passed after the script's production rebuild.
|
||||||
|
- `git diff --check`: passed; the product diff is limited to
|
||||||
|
`src/pages/Plugins/index.tsx` and `tests/unit/plugins-page.test.tsx`.
|
||||||
|
|
||||||
|
## Follow-ups
|
||||||
|
|
||||||
|
- Source task will run a fresh fixed-range R4 Standards + Spec review. Do not
|
||||||
|
merge to main before that independent acceptance.
|
||||||
|
|
||||||
|
## Promotion Candidates
|
||||||
|
|
||||||
|
- None recorded.
|
||||||
@@ -499,8 +499,15 @@ export function Plugins() {
|
|||||||
}, [activeProject, workspaceHydrated]);
|
}, [activeProject, workspaceHydrated]);
|
||||||
|
|
||||||
const selectedOfficialId = pluginIdFromKey(filters.selectedKey);
|
const selectedOfficialId = pluginIdFromKey(filters.selectedKey);
|
||||||
|
const attemptedDetailSelectionRef = useRef<string | null>(null);
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!selectedOfficialId || details[selectedOfficialId] || detailState[selectedOfficialId] === 'loading') return;
|
if (!selectedOfficialId) {
|
||||||
|
attemptedDetailSelectionRef.current = null;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (attemptedDetailSelectionRef.current === selectedOfficialId || details[selectedOfficialId]) return;
|
||||||
|
attemptedDetailSelectionRef.current = selectedOfficialId;
|
||||||
|
if (detailState[selectedOfficialId] === 'loading') return;
|
||||||
void pluginMarketplaceStore.getState().loadDetail(selectedOfficialId).catch(() => undefined);
|
void pluginMarketplaceStore.getState().loadDetail(selectedOfficialId).catch(() => undefined);
|
||||||
}, [detailState, details, selectedOfficialId]);
|
}, [detailState, details, selectedOfficialId]);
|
||||||
|
|
||||||
|
|||||||
@@ -348,6 +348,75 @@ describe('PluginsView', () => {
|
|||||||
expect(screen.getAllByRole('alert')).toHaveLength(2);
|
expect(screen.getAllByRole('alert')).toHaveLength(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('attempts a rejected official detail load once per open and retries only after close and reopen', async () => {
|
||||||
|
preparePluginsRouteState();
|
||||||
|
pluginMarketplaceStore.setState({
|
||||||
|
catalog: {
|
||||||
|
items: [{
|
||||||
|
pluginId: 'makelore.notes',
|
||||||
|
title: 'Notes',
|
||||||
|
summary: 'Write project notes',
|
||||||
|
category: 'productivity',
|
||||||
|
tags: ['notes'],
|
||||||
|
providerDisplayName: 'MakeLore',
|
||||||
|
runtimeKind: 'skill_only',
|
||||||
|
runtimeStatus: 'enabled',
|
||||||
|
acquisition: 'free',
|
||||||
|
usageBilling: 'token_point',
|
||||||
|
includedOperationCount: 0,
|
||||||
|
meteredOperationCount: 1,
|
||||||
|
stableVersion: '2.0.0',
|
||||||
|
betaVersion: null,
|
||||||
|
}],
|
||||||
|
nextCursor: null,
|
||||||
|
total: 1,
|
||||||
|
catalogGeneration: 1,
|
||||||
|
etag: 'catalog-1',
|
||||||
|
pricingVersionId: 'pricing-1',
|
||||||
|
stale: false,
|
||||||
|
fetchedAt: 1,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
let rejectFirstAttempt: (reason?: unknown) => void = () => undefined;
|
||||||
|
const firstAttempt = new Promise<void>((_resolve, reject) => {
|
||||||
|
rejectFirstAttempt = reject;
|
||||||
|
});
|
||||||
|
const loadDetail = vi.spyOn(pluginMarketplaceStore.getState(), 'loadDetail')
|
||||||
|
.mockImplementationOnce(async () => await firstAttempt)
|
||||||
|
.mockResolvedValue(undefined);
|
||||||
|
|
||||||
|
render(
|
||||||
|
<MemoryRouter initialEntries={[
|
||||||
|
'/plugins?scope=all&source=all&state=all&plugin=official%3Amakelore.notes',
|
||||||
|
]}>
|
||||||
|
<Plugins />
|
||||||
|
</MemoryRouter>,
|
||||||
|
);
|
||||||
|
|
||||||
|
await waitFor(() => expect(loadDetail).toHaveBeenCalledTimes(1));
|
||||||
|
await act(async () => {
|
||||||
|
pluginMarketplaceStore.setState((state) => ({
|
||||||
|
detailState: { ...state.detailState, 'makelore.notes': 'error' },
|
||||||
|
catalogError: 'detail offline',
|
||||||
|
}));
|
||||||
|
rejectFirstAttempt(new Error('detail offline'));
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
await Promise.resolve();
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
expect(loadDetail).toHaveBeenCalledTimes(1);
|
||||||
|
expect(screen.getByRole('alert', { hidden: true })).toHaveTextContent('插件详情刷新失败。 detail offline');
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: '关闭插件详情' }));
|
||||||
|
await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Notes' })).not.toBeInTheDocument());
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: '查看Notes详情' }));
|
||||||
|
await waitFor(() => expect(loadDetail).toHaveBeenCalledTimes(2));
|
||||||
|
});
|
||||||
|
|
||||||
it('renders the unified workspace and exposes URL-backed filter changes', () => {
|
it('renders the unified workspace and exposes URL-backed filter changes', () => {
|
||||||
const view = props();
|
const view = props();
|
||||||
view.sourceErrors = [
|
view.sourceErrors = [
|
||||||
|
|||||||
Reference in new issue
Block a user