fix: prevent skill catalog infinite loop on persistent fetch errors#2012
Conversation
… infinite loop on persistent errors
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Approve. The one-line fix is correct and minimal: setting source: "http" in the refreshSkillCatalog catch block breaks the effect-guard re-fire loop on persistent fetch errors, mirroring the existing refreshSkillDetail error path. The other catch blocks in this file (addSkill/removeSkill/updateSkill/checkSkill) share the old pattern but are user-initiated (button handlers), not effect-driven, so they cannot loop — no change needed in this PR.
| const message = error instanceof Error ? error.message : String(error); | ||
| set((state) => ({ | ||
| skillCatalog: { ...state.skillCatalog, error: message }, | ||
| skillCatalog: { ...state.skillCatalog, source: "http", error: message }, |
There was a problem hiding this comment.
Nit (no action needed for this PR): Four other catch blocks below (addSkillToCatalog, removeSkillFromCatalog, updateSkillCatalog, checkSkillCatalog) still use { ...state.skillCatalog, error: message } without source: "http". They are safe today because they are invoked from button handlers, not the effect guard, so they cannot loop. A future consistency pass could align all skillCatalog error paths.
Holon Run Report
|
Summary
Follow-up fix for a regression introduced in #1999 (web-gui-followup-3).
Problem: The skills page effect guard was changed from
catalog.length > 0tosource !== "fixture"to fix an infinite reload loop when the catalog is legitimately empty. However, the catch block inrefreshSkillCatalog(runtime-store.ts:918) preserves the oldsourcevalue:Since
emptySkillCatalog.sourceis"fixture"and the error path does not change it,sourcestays"fixture"whileskillCatalogLoadingreturns tofalse. The effect guard passes again, re-firingrefreshSkillCatalogin a tight loop — the same freeze #1999 intended to fix, just shifted from empty-success to persistent-error.Fix: Set
source: "http"in the catch block so the guard recognizes a load attempt was made:This mirrors the existing pattern in
refreshSkillDetail(line 942), which already setssource: "http"on error.Verification
npm run build(typecheck + vite build) passes ✓Identified by holonbot review on #1999.