fix: check res.ok before parsing knowledge-graph.json in dashboard#418
Draft
eldar702 wants to merge 1 commit into
Draft
fix: check res.ok before parsing knowledge-graph.json in dashboard#418eldar702 wants to merge 1 commit into
eldar702 wants to merge 1 commit into
Conversation
The dashboard's knowledge-graph.json fetch parsed the response body unconditionally (`.then((res) => res.json())`) with no `res.ok` guard, while its four sibling fetches in the same component (meta.json, config.json, diff-overlay.json, domain-graph.json) all check `res.ok` first. When the graph file was missing or returned a non-2xx status, the 404/error body was handed to `validateGraph`, which then surfaced the misleading "Invalid knowledge graph: Missing or invalid project metadata" instead of a clear HTTP error. Diagnosis credited to collaborator ZebangCheng, who identified the exact missing guard. Fix: extract the fetch + HTTP-status guard + parse + validate chain into a small pure helper (`fetchAndValidateGraph`) that mirrors the sibling fetches by short-circuiting on `!res.ok` with an HTTP-status error, and wire App.tsx to use it. A RED-first unit test (mocked fetch) asserts that a 404 surfaces an HTTP error containing the status code rather than the schema-validation message; the 200-with-invalid-body and 401 paths are also covered. Closes Egonex-AI#288 AI-assisted contribution.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #288. The dashboard's
knowledge-graph.jsonfetch inApp.tsxdid not checkres.okbefore callingres.json()— the four sibling fetches in the same effect (meta.json,config.json,diff-overlay.json,domain-graph.json) all do. So when the graph file is missing (e.g. before/understandhas run → HTTP 404), the 404 body was parsed and run throughvalidateGraph, surfacing the misleading "Invalid knowledge graph: Missing or invalid project metadata" instead of a clear HTTP error.Thanks to @ZebangCheng for diagnosing the exact root cause and file in the issue.
Root cause
packages/dashboard/src/App.tsx(knowledge-graph fetch) used.then((res) => res.json())with nores.okguard, unlike the sibling fetches.Fix
Extracted the fetch+validate step into a small, testable helper
utils/fetchAndValidateGraph.tsthat guards onres.okand returns a discriminated result (loaded/http-error/network-error).App.tsxnow surfaces a clear HTTP/network error for a missing graph instead of a schema-validation message; the valid-graph and invalid-JSON paths are unchanged. (The helper extraction lets the behavior be unit-tested —packages/dashboardhad no test harness.)Testing
Added
utils/__tests__/fetchAndValidateGraph.test.ts: a mockedfetchreturning{ok:false, status:404}now surfaces an HTTP-status error (not the schema message); 200-with-invalid-JSON still yields a validation error; network rejection is handled. RED before the fix, GREEN after.AI-assisted contribution — drafted with Claude and verified against current
main.