feat(admin): expose the retry-attempt controls in the UI (#4487 follow-up)

[Yeraze]

Follow-up to #4502.

#4502 shipped the retry mechanism and registered adminRetryAttempts in VALID_SETTINGS_KEYS — but never wired it into the Settings UI. The result: the feature could only be enabled by POSTing to the settings API by hand, and the per-send retryAttempts override only by crafting the request body yourself. Neither was reachable from the app, which made the setting look wired up when it wasn't. My miss.

What's added

Settings → Remote Administration (new, admin-only): an "Admin command attempts" input, 1–10, default 1. Wired through every place the settings recipe calls for — the SettingsDraft type, buildBaseline(), the initial* snapshot and its dep array, the server load path, the post-save re-seed, and the hand-maintained handleSave literal.

Admin Commands → Target Node: a per-send "Attempts for this command" box. Blank defers to the setting, so it only ever narrows scope to the node being worked on — the key is omitted from the request body entirely when unset, leaving the server's own fallback in charge.

Two things the tooling caught, not review

server.settings-persistence.test.ts failed with expected ['adminRetryAttempts'] to equal []. Every key SettingsTab sends must either be loaded by SettingsContext or be a known server-only key — this guards the #2048 class of bug where a setting is saved but never read back. adminRetryAttempts is genuinely server-only (only the admin routes read it, no frontend state depends on it), so it joins the allowlist beside meshcoreCliTimeoutSeconds with the same reasoning spelled out.

The lint ratchet flagged react-hooks/exhaustive-deps going 1→2. Not a style nit: the executeCommand useCallback now reads retryAttemptsOverride, so without the dep it captures a stale value and sends the previous override. tsc was perfectly happy, and it only misbehaves on the second send after changing the value — the ratchet was the only thing in the pipeline that would have caught it.

Test plan

• Full Vitest suite — success: true, 13017 passed, 0 failed.
• server.settings-persistence.test.ts passes, including the source-extraction check over the handleSave literal.
• npx tsc --noEmit clean; npm run lint:ci back to no FAIL lines.

Note

Retry remains opt-in: the default is still 1 attempt, so nothing changes until an operator sets it. That's deliberate — every extra attempt is real airtime on a shared band.

---

Generated by Claude Code

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: feat(admin): expose the retry-attempt controls in the UI

Tasks:

• Gather context
• Read changed files
• Review code quality and best practices
• Check for bugs/issues
• Review test coverage
• Post final review

---

Overall this is a clean, well-structured follow-up. The settings recipe is followed correctly across all the required touchpoints, and the PR description itself gives a clear accounting of what was done. A few observations below.

SettingsTab.tsx

Minor: step attribute missing on the settings input (SettingsTab.tsx:2356–2367)

The adminRetryAttempts input uses type="number" with min="1" and max="10" but no step attribute. Without it, browsers default to step="1", which happens to be correct here, but the sibling inputs for meshcoreCliTimeoutSeconds and cotFeedPort also omit it — so this is consistent with the file's existing style. Not a bug; just noting the implicit assumption.

Minor: parsing consistency between load path and onChange

Load path (line 560) uses parseInt(..., 10), onChange handler (line 2363) also uses parseInt. Both then clamp to [1, 10]. That's consistent. ✓

Observation: buildBaseline dep array (line 721)

initialAdminRetryAttempts is correctly included in the buildBaseline useMemo dep array. ✓

Observation: post-save re-seed (line 893)

setInitialAdminRetryAttempts(d.adminRetryAttempts) is called after a successful save, matching the pattern for every other category-C field. ✓

Observation: handleSave literal (line 961)

adminRetryAttempts: draft.adminRetryAttempts.toString() is in the hand-maintained literal. The source-extraction test (extractSettingsTabSends) will pick this up and cross-check it against VALID_SETTINGS_KEYS. ✓

AdminCommandsTab.tsx

Minor: retryAttemptsOverride doesn't reset on node change

retryAttemptsOverride is initialized once at component mount. When the user switches the target node (via selectedNodeNum), the override remains set from the previous node. This may be intentional ("I want to use this override for all commands in this session"), but it's slightly surprising UX — someone who sets attempts=5 to prod a flaky node and then switches to a healthy node will silently send 5-attempt commands to the new target too.

A useEffect that resets retryAttemptsOverride to null on selectedNodeNum changes would be the conventional fix:

useEffect(() => { setRetryAttemptsOverride(null); }, [selectedNodeNum]);

Not blocking, just worth flagging.

Good: useCallback dep array (line 1475)

retryAttemptsOverride was added to the dep array, which is what the PR description says the lint ratchet caught. This is correct — without it executeCommand would capture a stale closure value on the second send after the user changed the input. ✓

Good: conditional spread pattern (line 1457)

...(retryAttemptsOverride != null ? { retryAttempts: retryAttemptsOverride } : {}),

Omitting the key entirely when unset, rather than sending null or undefined, means the server falls back cleanly to its own adminRetryAttempts setting. This is the right approach. ✓

Minor: isExecuting guard on the override input (line 2479)

The input is disabled={isExecuting}, preventing mid-flight changes. ✓

Observation: no step attribute on the override input (line 2473), same as the settings input above — defaults to 1, which is correct.

server.settings-persistence.test.ts

Good: SERVER_ONLY_SETTINGS allowlist (lines 479–483)

adminRetryAttempts is added with a clear comment explaining why it's server-only (only the admin routes read it, no frontend state depends on it). The comment references the correct issue number. This pattern matches the preceding entries cleanly. ✓

Good: no validTestValue entry needed

The generic fallback (test-${key}) is used, which makes sense because the server's settings handler doesn't apply range validation for this key (unlike maxNodeAgeHours, etc. which need numeric values). Worth confirming the server doesn't reject a string value for this key — if it does, a validTestValue entry like '1' would be needed. Based on reading the PR description ("numeric range" validation), the server likely parseInts it server-side; if the parse fails it would fall back. Low risk.

Summary

The implementation is solid. The single functional observation worth considering is the per-node override not resetting on node switch — everything else is well-executed. The PR description explicitly calls out the two tooling catches (exhaustive-deps, persistence test), both of which were handled correctly.

---

| Branch: feat/admin-retry-settings-ui