fix(dashboard): scope reliability endpoints by projectId#1970
fix(dashboard): scope reliability endpoints by projectId#1970Automata-intelligentsia wants to merge 1 commit into
Conversation
Reliability GET and reset endpoints were always using the server's root store, ignoring projectId. Mirror the Command Center pattern by using getProjectIdFromRequest and getScopedStore so multi-project servers report per-project reliability stats. Refs FUX-042
📝 WalkthroughWalkthroughThe ChangesProject-scoped reliability endpoints
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| app.get("/api/health/reliability", async (req, res) => { | ||
| const projectId = getProjectIdFromRequest(req); | ||
| const scopedStore = projectId ? await getScopedStore(projectId) : store; |
There was a problem hiding this comment.
getScopedStore rejection in GET handler
Both the GET and POST endpoints call await getScopedStore(projectId) without wrapping it in try/catch. If store resolution fails for an unknown or misconfigured projectId (e.g. resolveScopedStore throws), Express 4 won't automatically propagate the async rejection to the error handler — the request will hang without a response. Compare with /api/engine/start (lines 1578–1594) and the terminal WebSocket handler (lines 2059–2077), both of which wrap getScopedStore/resolveScopedStore in try/catch and return a structured error response. The same guard is needed here so callers receive a 500 JSON response rather than a stalled connection.
| app.post("/api/health/reliability/reset", async (req, res) => { | ||
| const projectId = getProjectIdFromRequest(req); | ||
| const scopedStore = projectId ? await getScopedStore(projectId) : store; |
There was a problem hiding this comment.
The reset endpoint has the identical problem: await getScopedStore(projectId) can throw if the projectId is unknown or the underlying store resolution fails, and there is no try/catch to translate that into a structured HTTP error. A caller posting a reset for a non-existent project will get a hanging request instead of a 500 or 404 response.
| app.get("/api/health/reliability", async (req, res) => { | ||
| const projectId = getProjectIdFromRequest(req); | ||
| const scopedStore = projectId ? await getScopedStore(projectId) : store; |
There was a problem hiding this comment.
The project's coding convention (AGENTS.md) requires a FNXC:Area-of-product yyyy-MM-dd-hh:mm comment whenever new behavior is introduced or requirements change. The two modified blocks add multi-project scoping to the reliability endpoints but carry no FNXC annotation describing the FUX-042 requirement and the date of the change. This pattern appears consistently across the file for similar routing additions.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| app.post("/api/health/reliability/reset", async (req, res) => { | ||
| const projectId = getProjectIdFromRequest(req); | ||
| const scopedStore = projectId ? await getScopedStore(projectId) : store; | ||
| const resetAt = new Date().toISOString(); | ||
| await store.updateSettings({ reliabilityStatsResetAt: resetAt }); | ||
| await scopedStore.updateSettings({ reliabilityStatsResetAt: resetAt }); | ||
| res.json({ resetAt }); | ||
| }); |
There was a problem hiding this comment.
AGENTS.md requires a changeset under .changeset/<name>.md with "@runfusion/fusion": patch for any bug fix that changes observable behavior in the published CLI. This PR fixes incorrect aggregate reliability stats in multi-project setups (FUX-042), which is a user-visible behaviour change for Fusion operators — a patch changeset with a fix category is expected.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dashboard/src/server.ts (1)
1597-1709: 📐 Maintainability & Code Quality | 🟠 MajorBug fix (FUX-042) must include regression tests per coding guidelines.
This PR fixes reliability endpoints to be project-scoped, but tests are deferred to a follow-up. As per coding guidelines: "When fixing a bug, regression tests must assert the general invariant across all known surfaces, and bug-fix work must include symptom verification and surface enumeration rather than only the single reproduction." Both the GET and POST reset endpoints should have tests verifying project-scoped isolation (different
projectIdvalues return independent stats) and the fallback behavior whenprojectIdis absent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/server.ts` around lines 1597 - 1709, Add regression tests for the reliability endpoints to cover project-scoped isolation and fallback behavior. Update the tests around app.get("/api/health/reliability") and app.post("/api/health/reliability/reset") so they verify different projectId values read/write independent settings and stats, and that requests without a projectId continue to use the shared store. Use the existing route handlers and getProjectIdFromRequest/getScopedStore behavior as the reference points.Source: Coding guidelines
🧹 Nitpick comments (1)
packages/dashboard/src/server.ts (1)
1598-1599: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd error handling for scoped store resolution failures.
Neither endpoint wraps the
getScopedStorecall in a try-catch. If project store creation fails (e.g.,getOrCreateProjectStorethrows on database errors), the error falls through to the generic Express 500 handler with a vague message. The SSE endpoint at line 1108 handles this case explicitly with a targeted error response. Consider adding similar error handling here.♻️ Proposed error handling for GET endpoint
app.get("/api/health/reliability", async (req, res) => { const projectId = getProjectIdFromRequest(req); - const scopedStore = projectId ? await getScopedStore(projectId) : store; + let scopedStore: TaskStore; + try { + scopedStore = projectId ? await getScopedStore(projectId) : store; + } catch (err) { + sendErrorResponse(res, 500, err instanceof Error ? err.message : "Failed to resolve project store"); + return; + } const rawWindowDays = req.query.windowDays;Also applies to: 1704-1705
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/server.ts` around lines 1598 - 1599, The GET endpoint and the other affected handler currently call getScopedStore without guarding failures, so a project store creation error will fall through to the generic 500 handler. Add explicit try-catch around getScopedStore in the same places as the SSE flow, and return a targeted error response when project-scoped store resolution fails. Use the existing getProjectIdFromRequest and getScopedStore flow to locate the change, and mirror the endpoint-specific handling already used in the SSE handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/dashboard/src/server.ts`:
- Around line 1597-1709: Add regression tests for the reliability endpoints to
cover project-scoped isolation and fallback behavior. Update the tests around
app.get("/api/health/reliability") and app.post("/api/health/reliability/reset")
so they verify different projectId values read/write independent settings and
stats, and that requests without a projectId continue to use the shared store.
Use the existing route handlers and getProjectIdFromRequest/getScopedStore
behavior as the reference points.
---
Nitpick comments:
In `@packages/dashboard/src/server.ts`:
- Around line 1598-1599: The GET endpoint and the other affected handler
currently call getScopedStore without guarding failures, so a project store
creation error will fall through to the generic 500 handler. Add explicit
try-catch around getScopedStore in the same places as the SSE flow, and return a
targeted error response when project-scoped store resolution fails. Use the
existing getProjectIdFromRequest and getScopedStore flow to locate the change,
and mirror the endpoint-specific handling already used in the SSE handler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ffa07f4e-c3d4-452b-86ea-a0263fa1da9f
📒 Files selected for processing (1)
packages/dashboard/src/server.ts
1 participant
Problem
Reliability GET and reset endpoints always used the server's root store, ignoring projectId. In multi-project setups this meant every project saw the same aggregate reliability stats.
Change
Mirror the Command Center pattern by using
getProjectIdFromRequestandgetScopedStorein both endpoints:GET /api/health/reliability?projectId=...POST /api/health/reliability/reset?projectId=...When no projectId is supplied, behavior falls back to the root store (unchanged from before).
Notes
This is the FUX-042 fix. v0.57.0 introduced the project-scoping helpers used elsewhere but did not apply them to the reliability endpoints.
Testing
Existing unscoped tests continue to pass because the default path still uses the root store. Project-scoped regression tests are planned for a follow-up commit if desired.
Summary by CodeRabbit