feat(dashboard): clearer agent quick presets - #52
Merged
Conversation
Presets no longer silently flip the single/multiple toggle: show only the
presets that fit the selected mode (One agent: Best available, Auto;
Several agents: Every effort level, Every model). Rename to plain language
and split the cross-provider bulk action ('One of each provider') below a
divider. Toggle reads 'One agent' / 'Several agents'.
|
NAX review completed with status: success
Final resultsReview · 2026-08-13T20-43-21-900Z-review
Contents
Review
Claude Results
Repository State
Structured Findings[
{
"id": "R1",
"category": "defect",
"severity": "high",
"status": "confirmed",
"file": "src/control-plane/idempotent-mutations.js",
"line": 97,
"claim": "A mutation that succeeds but returns a non-JSON-serializable result is permanently recorded as failed in the idempotency ledger.",
"evidence": "serializableMutationResult() runs inside the try block after execute() has already produced its side effect. JSON.parse(JSON.stringify(undefined)) throws, so any execute() resolving to undefined (or returning a circular/BigInt value) lands in the catch at line 99, which calls store.fail() and rethrows. Every later replay of that requestId then hits line 86 and throws replayFailure() forever, even though the run/cancel/follow-up actually happened.",
"suggested_fix": "Serialize the result before the side effect is considered final, or treat a serialization failure as a completion with a degraded result payload rather than routing it through store.fail(). At minimum, only call store.fail() for errors thrown by execute() itself.",
"confidence": "high"
},
{
"id": "R2",
"category": "defect",
"severity": "medium",
"status": "confirmed",
"file": "src/dashboard/web/src/App.tsx",
"line": 346,
"claim": "Switching the Netlify Agent Runner target silently does nothing when the request fails.",
"evidence": "selectNetlifyTargetSite awaits selectNetlifyTarget() with no try/catch. selectNetlifyTarget (src/dashboard/web/src/api.ts:143) delegates to fetchJson, which throws on a non-OK response — and the server throws 404 unknown_target for a site that is not in linkedSites (src/dashboard/server.js:1217). Every comparable handler in this file (App.tsx:781, 803, 911, 924, 952) catches and calls setRunError(...). Here the rejection is unhandled, the menu closes, and the header keeps showing the old target.",
"suggested_fix": "Wrap the call in try/catch and surface the message through setRunError (or a Mantine notification, matching RunFollowupModal.tsx:36).",
"confidence": "high"
},
{
"id": "R3",
"category": "defect",
"severity": "medium",
"status": "confirmed",
"file": "src/dashboard/api/app.js",
"line": 110,
"claim": "The Hono JSON body reader has no size limit, so the 1 MiB request cap no longer applies to any POST route.",
"evidence": "honoJsonBody() calls c.req.text() with no bound. The legacy reader it replaced, readJsonBody (src/dashboard/api/request.js:18), caps bodies at 1 MiB and returns 413 payload_too_large. Since isMutationDashboardApiPath (src/dashboard/server.js:380) now routes every POST through the Hono app, that cap is dead in practice. The MCP adapter that talks to this same server does bound its requests (DEFAULT_MAX_REQUEST_BYTES, src/mcp/adapters/local-dashboard-http.js:12), so the limit is inconsistent across the two hops.",
"suggested_fix": "Apply Hono's bodyLimit middleware (or check content-length plus a streaming byte count) in createDashboardApi with the same 1 MiB default, and keep the 413 payload_too_large code.",
"confidence": "high"
},
{
"id": "R4",
"category": "defect",
"severity": "medium",
"status": "likely",
"file": "src/dashboard/api/app.js",
"line": 479,
"claim": "Five POST routes dereference mutationResult() without the null guard the other eight use, turning an empty mutation response into a 500 TypeError.",
"evidence": "mutationResult() returns null for any falsy input (line 130). /api/files/open (479), /api/netlify/target (486), /api/workflows/:id/dry-run (494), /api/workflows/:id/runs (502) and /api/agent-runs (510) immediately read result.body, while /api/runs/:id/cancel (546), retry (582), followups (591) and the rest first check `if (!result) throw requestError(404, ...)`. The local wiring happens to always return an object, but createDashboardApi is a generic runtime boundary — the hosted path passes transport results straight through (src/dashboard/runtime/netlify-function.js:68-69), so a transport returning undefined yields 'Cannot read properties of null' as a 500 internal_error instead of a typed response.",
"suggested_fix": "Give the five routes the same `if (!result) throw requestError(404, ...)` guard, or make mutationResult() throw a typed invalid_service_response for null so no caller can deref it.",
"confidence": "medium"
},
{
"id": "R5",
"category": "defect",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 231,
"claim": "In 'Several agents' mode the active-preset highlight fires on selections that are not the preset, so a card reads as selected when it is not.",
"evidence": "activePreset returns 'all-efforts' for `models.length === 1 && efforts.length > 0` and 'all-models' for `models.length >= 1 && efforts.length === 0`. Toggling from 'One agent' to 'Several agents' keeps the single flagship model and its single effort (changeMode only truncates when moving to 'single', line 148-154), so the 'Every effort level' card immediately renders selected with aria-pressed=true despite one of N efforts being chosen. Clearing the effort then marks 'Every {provider} model' selected with one model chosen. The predicates test cardinality, not equality with what the preset would produce.",
"suggested_fix": "Compare against the preset's actual output the way the single-agent branch does — derive the all-efforts and all-models selections and use the existing sameSet() helper.",
"confidence": "high"
},
{
"id": "R6",
"category": "defect",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 184,
"claim": "'One of each provider' is a dead click once every provider flagship is already added.",
"evidence": "addFlagshipOfEveryProvider filters out existing ids, then `if (instances.length === 0) return` — the popover stays open, nothing is added, and no message explains why. The button is never disabled, unlike the 'Every effort level' card which at least gets a disabled state.",
"suggested_fix": "Disable the card when the computed instance list is empty and reuse the existing footer copy ('That exact provider, model, and effort configuration is already selected') to say so.",
"confidence": "high"
},
{
"id": "R7",
"category": "defect",
"severity": "medium",
"status": "confirmed",
"file": "src/mcp/autostart.js",
"line": 84,
"claim": "Auto-start never notices that the dashboard it spawned died, so a crash costs the caller a 30-second stall and discards the diagnostic.",
"evidence": "launch(projectRoot) spawns with stdio: 'ignore' and detached: true (line 37-41) and the return value is dropped. The poll loop only asks the registry whether an instance appeared, so a child that exits immediately — port already bound, bad naxEntry, missing permissions — is indistinguishable from one still booting. The caller waits the full timeoutMs before getting 'did not become healthy within 30s', and the child's stderr is gone.",
"suggested_fix": "Keep the ChildProcess handle, reject early on its 'error'/'exit' events, and capture stderr to a pipe or log file so the timeout error can quote the real failure.",
"confidence": "high"
},
{
"id": "R8",
"category": "defect",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 1942,
"claim": "Four legacy route handlers are unreachable dead code that duplicates logic now owned by the Hono app.",
"evidence": "handle() dispatches to the Hono app first (line 1937) whenever isHonoDashboardApiPath matches. isReadOnlyDashboardApiPath covers GET /api/health, /api/workflows and /api/runs (line 372) and isMutationDashboardApiPath covers POST /api/files/open (line 381). So the bodies at 1947-1975, 1983-1989, 1997-2012 and 2020-2024 can never run — only their `method !==` guards remain live. The dead health block also re-inlines the deploymentMode/capabilities computation that dashboardDeploymentMode() and legacyHealthCapabilities() (lines 109-127) already provide.",
"suggested_fix": "Delete the four bodies and keep a single method-not-allowed fallback for non-matching verbs.",
"confidence": "high"
},
{
"id": "R9",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 265,
"claim": "overlayLiveOnlyRuns exists twice with divergent signatures and sync/async behaviour.",
"evidence": "server.js:265 defines a synchronous version taking hasDurableRun(id) => boolean; src/dashboard/api/app.js:177 exports an async version taking getDurableRun(id) => Promise. Both dedupe durable runs and overlay live-only ones with the same offset-zero rule. Only the app.js copy is reachable for /api/runs (see R8), so the server.js copy is maintained but unused by that route.",
"suggested_fix": "Delete the server.js copy along with its only caller and import the exported one if it is still needed elsewhere.",
"confidence": "high"
},
{
"id": "R10",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 1093,
"claim": "The git branch list is snapshotted once at server start and never refreshed.",
"evidence": "branchCatalog = listKnownGitBranches(projectRoot) is a const computed at startup and copied into apiRuntime.branches at line 1208. Nothing reassigns it, so the header branch autocomplete (App.tsx:1319) keeps offering the branches that existed when the dashboard booted. A branch created during a long-lived session never appears without a restart.",
"suggested_fix": "Recompute on each /api/health read, or cache with a short TTL, since listKnownGitBranches is cheap relative to the health poll interval.",
"confidence": "high"
},
{
"id": "R11",
"category": "polish",
"severity": "medium",
"status": "confirmed",
"file": "tsconfig.json",
"line": 12,
"claim": "The rule that all JavaScript be precisely typed is enforced by a textual grep while the compiler runs with strict off.",
"evidence": "AGENTS.md requires JSDoc types and forbids `any`. check-jsdoc-types.js enforces that by regex over comment lines. But the root tsconfig sets \"strict\": false, so strictNullChecks and noImplicitAny are off across all of src/ and tests/ — the class of bug the JSDoc rule exists to prevent is exactly what strictNullChecks catches. The dashboard web tsconfig (src/dashboard/web/tsconfig.json:12) already sets strict: true, so the codebase is split.",
"suggested_fix": "Turn on strictNullChecks first (it is the highest-yield flag and can be adopted independently of full strict), fixing fallout subsystem by subsystem starting with src/control-plane and src/dashboard/api.",
"confidence": "high"
},
{
"id": "R12",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "scripts/check-jsdoc-types.js",
"line": 44,
"claim": "The scripts/ directory escapes both the JSDoc rule and the typechecker.",
"evidence": "The default file set is src/cli/nax.js plus a recursive walk of src/ only. tsconfig.json's include is src/**/* and tests/**/* — scripts/ appears in neither. Seven files live there, including scripts/run-mcp-agent-canary.mjs, which package.json ships in \"files\" as part of the published package.",
"suggested_fix": "Add scripts/ to the tsconfig include and to the check's default walk, or state the exemption explicitly in AGENTS.md.",
"confidence": "high"
},
{
"id": "R13",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 440,
"claim": "The 'Every effort level' preset is disabled with no explanation.",
"evidence": "disabled={models.length !== 1} with no title or helper text. After using 'Every {provider} model' (which selects many models), the card greys out and the user has no way to learn that it needs exactly one model selected. The Add button at line 140-146 sets a precedent by computing addDisabledTitle for each disabled reason.",
"suggested_fix": "Add a title/description like 'Select exactly one model to sweep its effort levels.'",
"confidence": "high"
},
{
"id": "R14",
"category": "polish",
"severity": "low",
"status": "likely",
"file": "src/mcp/autostart.js",
"line": 78,
"claim": "Concurrent MCP clients for the same project can each spawn a dashboard.",
"evidence": "ensureDashboardRunning does a discover-then-launch with no lock or claim file. Two `nax mcp` processes starting together both see no instance and both call launch(). The registry and port binding presumably let one win, but the loser's detached child is spawned and its failure is invisible (see R7).",
"suggested_fix": "Take an exclusive lock file in the registry directory around the discover/launch window, or have the loser detect the port conflict and exit quietly.",
"confidence": "medium"
}
]Recent Changes
The wider DefectsHighest impact first. R1 — idempotency ledger records successful mutations as failures. R7 — auto-start can't tell "booting" from "dead". R2 — silent failure switching Netlify target. R3 / R4 — Hono migration left two gaps. The 1 MiB body cap didn't survive the move from the hand-rolled router to Hono, and five of the thirteen POST routes lost the null guard their siblings kept. Both are small, mechanical fixes. R5 / R6 — preset affordances lie. The active-preset predicates test cardinality rather than equality with the preset's output, so toggling to "Several agents" immediately shows "Every effort level" as selected. "One of each provider" is a dead click when everything is already added. Both are in the file R8 — dead legacy handlers. Root CausesR3, R4 and R8 share one cause: the Hono migration was done by adding a dispatch check at the top of Improvements, ranked
What's well doneThe import-direction check is the strongest thing here — architectural intent expressed as a script that fails CI rather than as prose that decays. Verification notes
Items Considered And Rejected
Gemini Results
Repository State
Structured Findings[
{
"id": "R1",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 231,
"claim": "Multi-agent presets ('all-efforts' and 'all-models') are falsely marked active for single or partial selections in multiple mode.",
"evidence": "In activePreset (lines 231-233), `models.length === 1 && efforts.length > 0` returns 'all-efforts' even when only 1 effort level is selected out of multiple available efforts. Likewise, `models.length >= 1 && efforts.length === 0` returns 'all-models' when only a single model is selected out of multiple provider models, causing the preset buttons to display a selected state inaccurately.",
"suggested_fix": "Verify that `efforts` matches the full set of available effort IDs for that model before returning 'all-efforts', and verify that `models` matches all available models for that provider before returning 'all-models'.",
"confidence": "high"
},
{
"id": "R2",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 190,
"claim": "Clicking the 'Every effort level' preset fails silently when invoked while in an Auto (empty models) state.",
"evidence": "In selectAllEffortsForModel (lines 190-192), `models[0]` is read directly without fallback. When starting in Auto mode (where `models = []`), `models[0]` is undefined and the guard `if (!model || !definition) return` immediately exits without selecting the provider's default model or configuring its efforts.",
"suggested_fix": "Fall back to the provider's default model (`orderedModels[0]?.id` or `provider?.defaultModel`) when `models[0]` is not defined.",
"confidence": "high"
},
{
"id": "R3",
"category": "polish",
"severity": "suggestion",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 459,
"claim": "The 'One of each provider' action button lacks disabled state and user feedback when all provider flagships are already present in the step.",
"evidence": "In addFlagshipOfEveryProvider (lines 169-187), if all providers already have their flagship instance added, `instances.length === 0` causes an early return without feedback, leaving the user with an active-looking button that does nothing on click.",
"suggested_fix": "Disable the button or provide helper copy when all flagship instances are already present in existingInstances.",
"confidence": "high"
},
{
"id": "R4",
"category": "polish",
"severity": "suggestion",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 1234,
"claim": "Netlify target mutation reads account slug from netlifyContext.account instead of the site record.",
"evidence": "In server.js (line 1234), `setNetlifyTarget` constructs netlifyAccess using `netlifyContext.account?.slug`, but `DashboardNetlifyContext.account` only contains `{ email: string }`. The actual `accountSlug` property is returned on `site` by `checkNetlifyAccess`.",
"suggested_fix": "Read `accountSlug` from the matching linked site or target object instead of `netlifyContext.account`.",
"confidence": "high"
}
]Explore
Review
Improve
Items Considered And Rejected
Codex Results
Repository State
Structured Findings[
{
"id": "R1",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 109,
"claim": "Multiple-model selection can create model and effort combinations that the selected models do not support.",
"evidence": "The effort picker builds a union of efforts across selected models, then line 125 applies every selected effort to every model. OpenCode models expose different effort sets, and server validation rejects unsupported combinations.",
"suggested_fix": "Use the intersection of supported efforts, or generate only valid model-effort pairs and explain skipped combinations in the UI.",
"confidence": "high"
},
{
"id": "R2",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "tests/e2e/dashboard.spec.js",
"line": 608,
"claim": "The latest preset-copy change leaves dashboard E2E tests using obsolete accessible names.",
"evidence": "Tests still select Single agent, Multiple agents, All Claude models, and This model × all efforts, while the component now renders One agent, Several agents, Every Claude model, and Every effort level.",
"suggested_fix": "Update all affected selectors and add assertions covering the mode-specific preset visibility introduced by the latest commit.",
"confidence": "high"
},
{
"id": "R3",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 1244,
"claim": "Target selection reports success even when its preference cannot be persisted.",
"evidence": "writeTargetPreference returns a boolean after swallowing filesystem failures, but setNetlifyTarget ignores that result and returns HTTP 200. The target then appears selected but can disappear after restart.",
"suggested_fix": "Check the return value and return an actionable persistence error, or explicitly report that the selection is session-only.",
"confidence": "high"
},
{
"id": "R4",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "src/dashboard/web/src/App.tsx",
"line": 346,
"claim": "Failed Netlify target selections produce no user-facing error.",
"evidence": "The async menu handler has no try/catch or mutation error state, unlike the surrounding run operations. A failed request rejects the click handler and leaves the current UI unchanged without explanation.",
"suggested_fix": "Track pending and error state, catch request failures, and display a notification or inline actionable error.",
"confidence": "high"
},
{
"id": "R5",
"category": "polish",
"severity": "suggestion",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 231,
"claim": "Preset highlighting can claim a partial manual selection is an entire preset.",
"evidence": "Any single model with one or more efforts is marked Every effort level, and any nonempty model selection without efforts is marked Every provider model, without comparing against the complete preset sets.",
"suggested_fix": "Compute the exact expected model and effort sets for each preset and highlight only on full equality.",
"confidence": "high"
}
]ExploreProject Overview
Architecture
|
|
NAX review completed with status: success
Final resultsReview · 2026-08-13T20-43-31-414Z-review
Contents
Review
Claude Results
A review-only pass over the Repository State
Structured Findings[
{
"id": "R1",
"category": "defect",
"severity": "critical",
"status": "confirmed",
"file": "tests/e2e/dashboard.spec.js",
"line": 648,
"claim": "The dashboard e2e suite is broken by the preset rename and mode-gating in the pinned commit; at least three tests reference control labels and a preset behavior that no longer exist.",
"evidence": "AddAgentInstances.tsx:291-292 now renders 'One agent'/'Several agents'; the spec asserts radios named 'Single agent'/'Multiple agents' at 608, 636, 640, 644, 649, 660, 661, 729, 733. Preset buttons were renamed to 'Every {provider} model' (452), 'Every effort level' (446) and 'One of each provider' (462); the spec clicks 'All Claude models' (415, 648, 732), 'All Codex models' (773), 'This model x all efforts' (767) and 'Add flagship of every provider' (775). Worse than a rename: the test named 'dashboard only exposes model fan-out in explicit multiple-agent mode' asserts at 648-649 that clicking a bulk preset while in single mode flips the toggle to multiple - exactly the behavior commit e03fdcb deliberately removed, since those presets are now only rendered when mode === 'multiple' (AddAgentInstances.tsx:417-457).",
"suggested_fix": "Update tests/e2e/dashboard.spec.js to the new labels, and rewrite the 628-651 test so the bulk-preset path first selects 'Several agents' instead of asserting the removed auto-switch. Re-run `npm run dashboard:smoke`.",
"confidence": "high"
},
{
"id": "R2",
"category": "defect",
"severity": "high",
"status": "confirmed",
"file": ".github/workflows/ci.yml",
"line": 23,
"claim": "CI never runs the Playwright dashboard smoke test or the CLI help check, so UI regressions like R1 cannot be caught before a release.",
"evidence": "ci.yml runs check, test, dashboard:build and the site build only. `dashboard:smoke` and `check:cli-help` exist as scripts (package.json:41, 32) but are reachable solely through `release:verify` (package.json:49). Every preset behavior in AddAgentInstances.tsx is covered exclusively by the e2e spec - there is no unit test for the component - so the entire safety net for that file sits outside CI.",
"suggested_fix": "Add a CI job that installs Playwright browsers and runs `npm run dashboard:smoke` plus `npm run check:cli-help`, at minimum on pull requests touching src/dashboard.",
"confidence": "high"
},
{
"id": "R3",
"category": "defect",
"severity": "medium",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 1235,
"claim": "Selecting a Netlify target from the dashboard drops accountSlug from netlifyAccess because it reads a property that does not exist on the account object.",
"evidence": "The rebuilt netlifyAccess reads `netlifyContext.account?.slug`, but account is typed and produced as `{ email: string } | null` (src/integrations/netlify/dashboard-context.js:37, src/integrations/netlify/preflight.js:14, 112) - there is no `slug` key, so the spread is always empty. accountSlug is the real slug carried on the access verdict's `site`, and server.js:177 uses `netlifyAccess?.site?.accountSlug` when building the run target, so every run started after a dashboard target switch loses it. The linkedSites projection (dashboard-context.js:162-176) also never carries accountSlug forward, so the value is simply unavailable at this point.",
"suggested_fix": "Add accountSlug to DashboardLinkedSite from `verdict?.site?.accountSlug` in dashboard-context.js, then read `site.accountSlug` in setNetlifyTarget.",
"confidence": "high"
},
{
"id": "R4",
"category": "defect",
"severity": "medium",
"status": "confirmed",
"file": "src/dashboard/web/src/App.tsx",
"line": 346,
"claim": "Target selection has no error handling: a rejected request fails silently and produces an unhandled promise rejection.",
"evidence": "selectNetlifyTargetSite is a bare async function with no try/catch, and NetlifyTargetMenu.tsx:19 invokes it fire-and-forget via `onClick={() => onSelect?.(site.siteId)}`. api.ts throws DashboardApiError on any non-2xx, and the server throws a 404 `unknown_target` whenever the chosen site is no longer in the live linkedSites list (server.js:1223-1226) - reachable when state.json links change while the dashboard is open. The user sees the menu item click do nothing. This also bypasses the project's own convention: every other mutation goes through a useMutation hook in src/dashboard/web/src/queries/dashboard-mutations.ts.",
"suggested_fix": "Add a useSelectNetlifyTargetMutation alongside the other mutation hooks and surface failures with notifications.show, matching RunFollowupModal.tsx:36.",
"confidence": "high"
},
{
"id": "R5",
"category": "defect",
"severity": "medium",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 1244,
"claim": "A failed write of the target preference is silently swallowed, so the selection appears to succeed but is lost on restart.",
"evidence": "writeTargetPreference deliberately returns a boolean to report best-effort persistence (target-preference.js:43-57), but the call site discards it and always returns a 200. On a read-only or permission-denied project root the header updates and runs retarget for the current process, then revert to auto-resolution on the next `nax dashboard` start with no explanation anywhere.",
"suggested_fix": "Capture the boolean and return it in the response body (e.g. `persisted: false`), then have the UI warn that the choice applies to this session only.",
"confidence": "high"
},
{
"id": "R6",
"category": "defect",
"severity": "medium",
"status": "likely",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 54,
"claim": "The selected provider is captured once at mount and never resyncs, so a node that mounts before the health query resolves is stuck with an empty provider.",
"evidence": "`useState(firstProvider)` reads `catalog.providers[0]?.id || ''`. The fallback capabilities used while useDashboardHealthQuery is pending ship an empty provider list (dashboard-routes.ts:33-41), and WorkflowNode only requires catalogContext to be truthy (WorkflowNode.tsx:96) - the provider is always mounted, so an empty catalog still renders the control. When the real catalog arrives, firstProvider changes but the agent state does not; provider lookup stays undefined, flagshipSelection returns nothing, and add() bails at the `!agent` guard (line 157) with no feedback. The user can recover only by manually picking a provider.",
"suggested_fix": "Derive the effective agent instead of storing it raw: fall back to firstProvider whenever the stored id is not in catalog.providers.",
"confidence": "medium"
},
{
"id": "R7",
"category": "defect",
"severity": "medium",
"status": "confirmed",
"file": "src/integrations/netlify/target-preference.js",
"line": 1,
"claim": "The target-preference module and the POST /api/netlify/target endpoint have zero test coverage, including the new preferred-link branch that overrides target auto-resolution.",
"evidence": "Grepping tests/ for readTargetPreference, writeTargetPreference, target-preference, setNetlifyTarget and '/api/netlify/target' returns no matches. tests/unit/dashboard-netlify-context.test.js exercises resolveDashboardNetlifyContext but not the preferred branch at dashboard-context.js:105-116, which is the code that decides where every dashboard-launched run is sent. tests/unit/dashboard-server.test.js has no case for the new mutation, including its 404 path.",
"suggested_fix": "Add unit tests for read/write/clear round-trips including malformed JSON, for the preferred-link override and its stale-link fallback, and for the endpoint's 200/404/501 responses.",
"confidence": "high"
},
{
"id": "R8",
"category": "defect",
"severity": "low",
"status": "confirmed",
"file": "site/content/guides/use-the-dashboard.mdx",
"line": 102,
"claim": "The canonical user docs describe the previous preset set and omit the new target selection and persistence entirely.",
"evidence": "Lines 102-105 say quick presets cover 'one flagship from every provider, one model at every effort, or the four strongest models from one provider' with no mention that presets are now scoped to the One agent / Several agents toggle or that they were renamed. Line 70 still describes the header menu as inspect-and-open only, though it now selects the Agent Runner target and persists it to .nax/dashboard-target.json across restarts. AGENTS.md declares site/content the canonical docs source; the CHANGELOG 3.0.0 Added section also has no entry for target selection.",
"suggested_fix": "Update both passages in use-the-dashboard.mdx and add a CHANGELOG entry for the selectable, persisted Agent Runner target.",
"confidence": "high"
},
{
"id": "R9",
"category": "defect",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 184,
"claim": "'One of each provider' is a silent no-op when every provider flagship is already present.",
"evidence": "addFlagshipOfEveryProvider filters out existing ids and returns early on an empty list without closing the popover, emitting a message, or disabling the button. Every other dead-end path in this component explains itself through the summary line at 468-478. The button is also never disabled, unlike its sibling at line 440.",
"suggested_fix": "Precompute the additions, disable the card when the count is zero, and give it a title explaining why.",
"confidence": "high"
},
{
"id": "R10",
"category": "defect",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/web/src/components/NetlifyTargetMenu.tsx",
"line": 92,
"claim": "Once a target is chosen there is no way to return to automatic resolution without deleting a file by hand.",
"evidence": "The menu only offers per-site selection, and the preference wins over auto-resolution on every subsequent start (dashboard-context.js:103-116) for as long as the link exists. clearTargetPreference is implemented and exported (target-preference.js:63-71) but has no caller anywhere in src - the escape hatch was built and left unwired.",
"suggested_fix": "Add a 'Resolve automatically' menu item that posts an empty siteId and calls clearTargetPreference.",
"confidence": "high"
},
{
"id": "R11",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 423,
"claim": "The 'Best available' preset description overstates what it selects.",
"evidence": "The copy promises 'The strongest {provider} model at its highest effort', but flagshipSelection resolves the provider's configured defaultModel (ModelEffortFields.tsx:14-22), not a strength ranking, and modelSelection walks efforts high-to-low and picks the first one not already added (lines 29-36) - so it can legitimately land on medium.",
"suggested_fix": "Reword to something like 'The recommended {provider} model at its highest free effort', or make the copy reflect the fallback.",
"confidence": "high"
},
{
"id": "R12",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 231,
"claim": "The active-preset highlight reports presets the user never clicked.",
"evidence": "In multiple mode any single model plus any non-empty effort set reads as 'all-efforts', and any model set with no efforts reads as 'all-models'. Manually picking one model and one effort lights up 'Every effort level' as selected, which contradicts the aria-pressed state exposed at line 438.",
"suggested_fix": "Compare against the exact set the preset would produce, as the single-mode branch already does at 227-228.",
"confidence": "high"
},
{
"id": "R13",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 196,
"claim": "Preset handlers still force the mode they are now only reachable from.",
"evidence": "selectAllEffortsForModel (196) and selectAllProviderModels (208) call setMode('multiple') while being rendered only when mode is already 'multiple'; selectFlagshipPreset (164) and selectAutoPreset (216) do the same for 'single'. These are the leftovers of the auto-switching behavior removed in this commit and now read as if the switch still happens.",
"suggested_fix": "Drop the four setMode calls.",
"confidence": "high"
},
{
"id": "R14",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "src/integrations/netlify/target-preference.js",
"line": 52,
"claim": "The preference file is written non-atomically, unlike the project's other config writers.",
"evidence": "A direct writeFileSync can leave a truncated dashboard-target.json if the process is killed mid-write; readTargetPreference then silently returns null and the target reverts. The MCP setup path advertises atomic writes for the same class of file (CHANGELOG.md:24-25).",
"suggested_fix": "Write to a sibling temp file and rename.",
"confidence": "medium"
},
{
"id": "R15",
"category": "polish",
"severity": "low",
"status": "confirmed",
"file": "tsconfig.json",
"line": 10,
"claim": "The root TypeScript config runs with strict disabled, which weakens the repository's own no-any rule.",
"evidence": "`\"strict\": false` while AGENTS.md requires every JavaScript file to be JSDoc-typed with no any types; the docs site next door compiles with strict: true (site/tsconfig.json:9). Implicit any and null-unsafe access pass typecheck today - R3 is exactly the kind of silent property typo a stricter config narrows.",
"suggested_fix": "Enable strictNullChecks first, fix the fallout incrementally, then move toward full strict.",
"confidence": "medium"
}
]Recent changes and what they doThe last six commits add two features on top of the freshly landed MCP control plane: a selectable, persisted Agent Runner target ( Both features are well-shaped. The target preference correctly closes the loop on restart - Root causes worth namingR1 and R2 are one problem. The preset rename was a pure-UI change, and the repository's only coverage for that UI lives in a Playwright suite that CI does not run. AGENTS.md asks for R3, R4 and R5 share a shape. Positive observations
Suggested order of work
Items Considered And Rejected
Note on verification: the repository root has no installed dependencies in this environment, so the test and typecheck suites could not be executed. R1 was confirmed by exact string comparison between the spec's selectors and the component's rendered labels, not by running Playwright. Gemini Results
Repository State
Structured Findings[
{
"id": "R1",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 242,
"claim": "Active preset heuristic in 'Several agents' mode incorrectly highlights preset cards for partial manual selections.",
"evidence": "activePreset uses loose heuristics: `models.length >= 1 && efforts.length === 0` returns 'all-models' and `models.length === 1 && efforts.length > 0` returns 'all-efforts'. Selecting a single model out of multiple available provider models with no efforts highlights the 'Every {provider} model' preset card even when only 1 model is chosen, and selecting a single reasoning effort highlights the 'Every effort level' preset card.",
"suggested_fix": "Compare `models` and `efforts` against the exact full candidate sets (e.g. `sameSet(models, availableModels)` and `sameSet(efforts, availableEfforts)`) instead of using length inequalities.",
"confidence": "high"
},
{
"id": "R2",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "src/integrations/netlify/dashboard-context.js",
"line": 115,
"claim": "Target preference restoration sets configSource to the .netlify/state.json path instead of netlify.toml.",
"evidence": "In src/dashboard/server.js:1243, writeTargetPreference is called with `source: site.source` (which holds the `.netlify/state.json` file path). In src/integrations/netlify/dashboard-context.js:115, reading the preference assigns `configSource: preferred?.source || candidate?.source || ''`, populating `target.configSource` with `.netlify/state.json` rather than the Netlify config file path.",
"suggested_fix": "Save `configSource: site.configSource` in writeTargetPreference, and assign `configSource: preferred?.configSource || candidate?.source || ''` when hydrating the preference.",
"confidence": "high"
},
{
"id": "R3",
"category": "polish",
"severity": "suggestion",
"status": "confirmed",
"file": "src/dashboard/web/src/components/AddAgentInstances.tsx",
"line": 426,
"claim": "The 'Every effort level' preset button is disabled when switching into 'Several agents' mode with 0 models selected.",
"evidence": "The button specifies `disabled={models.length !== 1}`. When navigating from Auto (where `models` is empty) or when no model has been explicitly clicked in the MultiSelect yet, the preset button cannot be clicked to initialize the default model across all effort levels.",
"suggested_fix": "Allow clicking 'Every effort level' when `models.length === 0` by falling back to the flagship/default model definition.",
"confidence": "high"
},
{
"id": "R4",
"category": "polish",
"severity": "suggestion",
"status": "confirmed",
"file": "site/content/guides/use-the-dashboard.mdx",
"line": 81,
"claim": "Documentation for dashboard quick presets in the user guide is out of sync with the updated preset names and groupings.",
"evidence": "site/content/guides/use-the-dashboard.mdx lines 81-83 still reference 'the four strongest models from one provider' rather than the updated 'Best available', 'Auto', 'Every effort level', 'Every {provider} model', and 'One of each provider' actions introduced in commit e03fdcb.",
"suggested_fix": "Update the documentation copy in site/content/guides/use-the-dashboard.mdx to describe the updated single/several preset layout and cross-provider bulk button.",
"confidence": "high"
},
{
"id": "R5",
"category": "polish",
"severity": "suggestion",
"status": "confirmed",
"file": "tests/unit/dashboard-netlify-context.test.js",
"line": 46,
"claim": "Target preference overrides and fallback logic lack dedicated unit test assertions.",
"evidence": "tests/unit/dashboard-netlify-context.test.js covers default candidate resolution and monorepo filtering, but does not test readTargetPreference resolution, persisted target overrides, or recovery from missing links.",
"suggested_fix": "Add unit tests in tests/unit/dashboard-netlify-context.test.js verifying that existing .nax/dashboard-target.json entries take precedence and properly map config and state sources.",
"confidence": "high"
}
]Step 1: ExploreProject Overview
Architecture & Key Modules
Execution Flows
Test Coverage Gaps
Conventions
Step 2: ReviewRecent Changes Summary
Defects & Root Causes
Positive Observations
Step 3: ImproveQuick Wins
Polish Items
Refactoring & Design
Items Considered And Rejected
Codex Results
Repository State
Structured Findings[
{
"id": "R1",
"category": "defect",
"severity": "critical",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 1312,
"claim": "Selecting a different Netlify target does not update single-agent or follow-up submissions.",
"evidence": "These mutations keep using followupSiteId, followupSiteName, and followupNetlifyFilter captured when the server starts, while setNetlifyTarget only updates defaultRunOptions and the displayed context.",
"suggested_fix": "Resolve submission site, name, and filter from the current netlifyContext at mutation time, and add a test that switches targets before starting an agent and a follow-up.",
"confidence": "high"
},
{
"id": "R2",
"category": "defect",
"severity": "warning",
"status": "confirmed",
"file": "src/dashboard/server.js",
"line": 1171,
"claim": "MCP run-plan execution can retain a stale monorepo filter after the dashboard target changes.",
"evidence": "The lazy execution backend snapshots netlifyFilter when first created and is then cached, but later target selections mutate netlifyContext without rebuilding or updating that backend.",
"suggested_fix": "Resolve the filter from the plan target or current target for each execution instead of capturing it in the cached backend.", |
- split add-agent popover into choose (one/several/one-per-provider) then configure - close on Escape via document listener when focus leaves the dropdown - clearable model/effort multiselects; robust every-effort preset - provider-named flagship preset title; robot icon for one-agent - card-level Remove agents clears a step; add/remove live in node footer - summary reads "agents" not "instances"
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.
Makes the "Add new agent(s)" quick presets easier to understand (per discussion).
Changes
Best available,AutoEvery effort level,Every {provider} modelFlagship / highest→ Best available;This model × all efforts→ Every effort level;All {provider} models→ Every {provider} model; toggleSingle/Multiple agent→ One agent / Several agents. Auto keeps its name.Add flagship of every provider→ One of each provider, moved below an "or across providers" divider since it isn't single or multiple.Web-only; the derived active-preset highlight still applies. Reload the dashboard (no restart needed) to see it.
Verification:
dashboard:typecheck+dashboard:buildgreen.