feat: add Failed status support for Auto Master

[AustinChangLinksys]

Summary

• Add failed status to AutoMasterStatus enum to support FW's new status
• When Auto Master status is Failed, continue normal PnP flow instead of redirecting to login page, since password is still admin

Background

FW team added a new Failed status for GetAutoMasterStatus JNAP (LinksysWRT#383). This status indicates Auto Master exited without configuring the device (e.g., found another Master on network). In this case, the admin password remains admin, so the app should continue the normal PnP flow.

Changes

• lib/core/jnap/models/auto_master_status.dart: Add failed enum value
• lib/page/instant_setup/pnp_admin_view.dart: Handle Failed status in polling loop

Behavior

Auto Master Status	Password	App Behavior
Complete	Changed to WiFi password	Redirect to login page
Idle	May have changed	Redirect to login page
Failed	Still admin	Continue PnP flow

Test plan

• Test with FW v1.2.3.26070216 or later that supports Failed status
• Verify PnP flow continues correctly when Auto Master returns Failed

Ref: LinksysWRT#383

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Support Auto Master "Failed" status and continue PnP flow

✨ Enhancement 🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Add Failed to Auto Master status model to match new firmware responses.
• Treat Auto Master Failed as non-blocking and continue PnP (password remains admin).
• Avoid redirecting users to login when Auto Master exits without configuring.

Diagram

graph TD
  A["PnP Admin View"] --> B["PnP Provider"] --> C[("JNAP: GetAutoMasterStatus")] --> D["AutoMasterStatus"] --> E{"Status?"}
  E -->|"Complete / Idle"| F["Redirect: Login"]
  E -->|"Failed"| G["Continue PnP"]

  subgraph Legend
    direction LR
    _ui["UI/View"] ~~~ _db[("Device/JNAP")] ~~~ _dec{"Decision"}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Forward-compatible unknown-status handling

• ➕ Automatically tolerates new firmware statuses without app updates
• ➕ Avoids treating unmapped values as polling failures/timeouts
• ➖ Risky: unknown statuses may require login redirect or different UX
• ➖ Harder to reason about and test than explicit enum values

2. Centralize Auto Master terminal-status handling in provider

• ➕ Single source of truth for what statuses end polling and how to proceed
• ➕ Reduces duplicated branching logic across multiple PnP views
• ➖ Broader refactor; higher risk and more review surface
• ➖ Requires carefully defining behavior for each caller context

Recommendation: The explicit failed enum + UI-level branching is the right minimal change for the known new firmware status. Consider a follow-up to centralize/standardize “terminal” Auto Master statuses in the PnP provider (and/or add a safe fallback for unknown statuses) so new statuses don’t inadvertently turn into timeouts or misroutes in other PnP screens.

Files changed (2) +16 / -1

Enhancement (1) +5 / -1

auto_master_status.dartAdd Auto Master 'failed' enum value and JNAP mappings +5/-1

Add Auto Master 'failed' enum value and JNAP mappings

• Extends 'AutoMasterStatus' with a new 'failed' value to reflect the firmware’s 'Failed' response. Updates 'toValue()'/'fromValue()' mappings and documentation to clarify password behavior in this state.

lib/core/jnap/models/auto_master_status.dart

Bug fix (1) +11 / -0

pnp_admin_view.dartContinue PnP when Auto Master reports 'Failed' during polling +11/-0

Continue PnP when Auto Master reports 'Failed' during polling

• Updates the Auto Master polling loop to treat 'Failed' as a non-blocking terminal state. Stops waiting and continues the normal PnP flow instead of interrupting to the login/password route.

lib/page/instant_setup/pnp_admin_view.dart

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Setup ignores Failed status ✓ Resolved 🐞 Bug ≡ Correctness

Description

Because 'Failed' is now mapped to a non-null AutoMasterStatus, the PnP setup save flow no longer
treats it as a polling failure (null) but also doesn’t handle it as a terminal status, so it can
wait until maxRetry and then go down the timeout/error path. The shared pollAutoMasterStatus stop
condition also ignores Failed, so polling won’t naturally terminate on Failed.

Code

lib/core/jnap/models/auto_master_status.dart[29]

+      'Failed' => AutoMasterStatus.failed,

Evidence

The PR makes JNAP 'Failed' parse to a concrete enum value, but the setup/save polling loop and the
polling helper only terminate on Complete/Idle; Failed therefore becomes a non-null, non-terminal
status that resets failure counters and can lead to the timeout/error path.

lib/core/jnap/models/auto_master_status.dart[24-32]
lib/page/instant_setup/pnp_setup_view.dart[672-714]
lib/page/instant_setup/pnp_setup_view.dart[713-739]
lib/page/instant_setup/data/pnp_provider.dart[758-776]
lib/page/instant_setup/pnp_admin_view.dart[538-589]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AutoMasterStatus.failed` is now parsed from JNAP (`fromValue('Failed')`), but parts of the PnP flow still only treat `complete`/`idle` as terminal.
Concretely, `PnpSetupView._saveChanges()`:
- Treats `null` poll results as failures and exits after 3 consecutive failures.
- Treats only `complete`/`idle` as terminal statuses.
- Does **not** handle `failed`, so `failed` resets `consecutiveFailures` and the UI can remain in the waiting step until `maxRetry` is exceeded, after which it executes the timeout/error handling.
Additionally, `PnpNotifier.pollAutoMasterStatus()` only stops the scheduled polling when the status is `complete` or `idle`, not `failed`.
### Issue Context
The PR adds support for FW returning `Failed` and intends the app to continue normal PnP flow when Auto Master fails (password remains `admin`). The setup/save flow should mirror the admin view behavior and treat `failed` as terminal and non-redirecting.
### Fix Focus Areas
- lib/page/instant_setup/pnp_setup_view.dart[672-739]
- lib/page/instant_setup/data/pnp_provider.dart[758-776]
- lib/page/instant_setup/pnp_admin_view.dart[538-589]
### Suggested fix
1. In `PnpNotifier.pollAutoMasterStatus()`, update the `condition` to also stop when `status == AutoMasterStatus.failed`.
2. In `PnpSetupView._saveChanges()` polling loop, add explicit handling for `AutoMasterStatus.failed` to:
 - exit the waiting step (`_setupStep`), and
 - continue the normal save flow (do not redirect to login; avoid falling into the timeout block). This may require a local flag / early return pattern so the post-loop “timeout” block doesn’t execute when the terminal status was `failed`.
3. (Optional) Add/update a unit test to cover the `failed` status path in the setup/save flow.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[HankYuLinksys]

Code Review — Failed status support (independent pass)

Traced all three consumers of AutoMasterStatus and the scheduledCommand stream mechanics. Direction and intent are correct, but the PR only updates the admin_view polling loop and misses the symmetric loop in setup_view. Impact analysis below with concrete "what happens if not fixed".

---

🔴 Critical — setup_view's second defense line doesn't handle Failed; user gets stuck on the waiting screen for ~5 minutes

There are two polling loops that consume AutoMasterStatus, and this PR only touched the one in pnp_admin_view.dart. The one in pnp_setup_view.dart _saveChanges() (the pre-save "second defense") is unchanged:

} else {
  consecutiveFailures = 0;
  if (pollStatus == AutoMasterStatus.complete ||
      pollStatus == AutoMasterStatus.idle) {
    context.goNamed(RouteNamed.localLoginPassword);
    return;
  }
  // `failed` lands here: not null (not counted as a failure),
  // doesn't match complete/idle (no return) → the await-for just keeps waiting
}

This compounds with the provider's stop condition, which also wasn't updated:

// pollAutoMasterStatus() -> scheduledCommand(condition:)
condition: (result) {
  if (result is JNAPSuccess) {
    final status = AutoMasterStatus.fromValue(result.output['autoMasterStatus'] as String?);
    return status == AutoMasterStatus.complete ||
        status == AutoMasterStatus.idle;   // `failed` not here → generator never breaks
  }
  return false;
},

What happens if not fixed: if Auto Master goes Running → Failed before save, the setup_view loop neither returns nor breaks, and the underlying async* generator keeps re-sending every 5 s until maxRetry: 60 is exhausted. The user is stuck on the waiting screen for ~5 minutes, after which it falls through to the timeout branch (reconnect + retry save, up to _maxAutoMasterSaveAttempts). This is exactly the stuck state the FW Failed status was introduced to avoid.

Fix: two parts, both needed —

1. In pollAutoMasterStatus()'s condition, add || status == AutoMasterStatus.failed so the poll stops cleanly on the terminal state.
2. In setup_view's loop, add a failed branch mirroring admin_view (return to continue the save flow).

Note: admin_view itself is correct. Its new failed branch does return, which cancels the await for subscription and terminates the generator at its next suspension point — so the missing condition entry has no observable effect there. The bug surfaces only through setup_view, because it's the one consumer that doesn't return on failed. So this is one Critical (setup_view + shared condition), not a problem in the code the PR did touch.

---

🟡 Medium (needs FW confirmation, not a proven bug) — old FW may report the same scenario as Idle, which still routes to login with a password the user doesn't have

The enum comment quietly changes idle semantics — from "not running or failed" to "not running" — pulling the "failed" meaning out into the new Failed value. Per the PR table, Idle → redirect to login while Failed → continue.

What happens if not fixed / if FW mismatches: if an older FW reports the "found another Master, exited without configuring" scenario as Idle (rather than the new Failed), the app routes to login — but the device password is still admin, not the WiFi password. The user lands on the login page without knowing what password to enter, effectively stuck.

I can't verify this from the codebase — it depends entirely on old-FW behavior. The test plan mentions "FW v1.2.3.26070216 or later"; please confirm with the FW team whether older FW ever reports this case as Idle, and whether version gating / backward compat is needed. If old FW never hits this path, there's no new impact.

---

🟢 Low

• No tests. With the feat: add Auto Master status check in PnP flow #994/fix: correct PnP saveChanges timing for AutoParent scenario #1007 Auto Master test scaffolding already in place, stubbing a Failed case ("poll returns Failed → continues PnP flow, no redirect to login") is cheap and should be added.
• toValue()/fromValue() both updated for Failed — good, symmetric. 👍
• admin_view's failed → return comment is clear. 👍

---

Verdict

Correct direction, but not mergeable as-is — one Critical: setup_view's second defense line plus the shared pollAutoMasterStatus condition don't handle Failed, so a Failed before save leaves the user on the waiting screen for ~5 minutes. Fix both spots and add a Failed test. The FW-compat question (old Idle vs new Failed) is worth confirming with the FW team before merge.

As a broader note: this is the concrete cost of the duplicated polling logic flagged in #1007 — the two defense lines drifted again, and Failed was missed in the second one. Worth seriously considering pushing the poll-convergence logic down into the provider so there's a single place to handle status values.

[AustinChangLinksys]

🤖 Automated Review — Round 1 · 4b7a7d0..a86ba0b (full)

Verdict: 💬 Self-review (comment only) — Austin's own PR; automated flow never self-approves. Review findings below are for reference and blind-spot catching only.

	Conf.	Where	Issue (one-liner)
⚠️	High	auto_master_status.dart:36	[both reviewers] GetAutoMasterStatusResponse doc comment missing "Failed" from wire format
⚠️	Med	pnp_setup_view.dart:675,723,771	After failed break, _setupStep stays waitingAutoMaster until line 771 save setState — brief visual gap
⚠️	High	pnp_admin_view.dart:570–573	Comment "Auto Master completed successfully" misleadingly covers idle case too
⚠️	High	pnp_admin_view_test.dart / pnp_setup_view_test.dart	Zero test coverage for AutoMasterStatus.failed polling path in either view
⚠️	Med	pnp_setup_view.dart:723–753	if (autoMasterFailed) true branch is no-op log; intentional fall-through to save is implicit and fragile
💡	High	pnp_admin_view.dart:621	Pre-existing unsafe as cast in _retryAutoMasterCheck (test: guard makes it safe, but fragile pattern; not new in this PR)
💡	High	pnp_setup_view.dart:681	autoMasterFailed boolean for loop-break control; admin_view.dart's return pattern is cleaner
💡	High	pnp_setup_view.dart:722–765	autoMasterFailed path falls through to idle→complete edge-case check unnecessarily (harmless but latent)
💡	Med	pnp_provider.dart:782–790	Non-success JNAP results (error/timeout) collapse to null with no log differentiation — harder to triage
💡	Med	pnp_admin_view.dart:12, pnp_setup_view.dart:9	View layers import core/jnap/models directly (pre-existing pattern, extended by this PR)

Confidence: High = code-verified · Med = located + reasoned, not fully confirmed · Low = speculative, please double-check.
Items marked [both reviewers] were independently flagged by two agents → higher confidence.

⚠️ Warning Details

1. Stale Response format doc comment — High confidence [both reviewers]

• Location: lib/core/jnap/models/auto_master_status.dart:36
• Evidence:

/// Response format: { "autoMasterStatus": "Idle" | "Running" | "Complete" }
class GetAutoMasterStatusResponse extends Equatable {

"Failed" was added to the enum and fromValue/toValue, but the class-level doc comment still shows only 3 values. Any consumer reading this comment will have an incomplete API contract.

• Fix: Update to "Idle" | "Running" | "Complete" | "Failed"

---

2. _setupStep stays waitingAutoMaster during brief async window after failed — Med confidence [single, Reviewer A]

• Location: lib/page/instant_setup/pnp_setup_view.dart:675, 716–723, 771
• Evidence: Line 675 sets _setupStep = waitingAutoMaster. When failed is received at lines 714–717 (autoMasterFailed = true; break;), the if (autoMasterFailed) block at line 723 only logs. _setupStep is NOT reset until line 771 (_setupStep = saving). During any async work between lines 723 and 771, the UI remains on PnpAutoMasterWaitingView (lines 244–246 widget switch). Not a permanent hang (setState at 771 corrects it), but the user sees the Auto Master waiting spinner during the save phase.
• Fix: Add setState(() { _setupStep = _PnpSetupStep.config; }); inside if (autoMasterFailed) at line 723, or add a comment explaining the intentional fall-through.

---

3. Misleading comment on idle/complete terminal block — High confidence [single, Reviewer B]

• Location: lib/page/instant_setup/pnp_admin_view.dart:570–573
• Evidence:

if (pollStatus == AutoMasterStatus.complete ||
    pollStatus == AutoMasterStatus.idle) {
  // Auto Master completed successfully, password may have changed
  setState(() { _isWaitingForAutoMaster = false; });
  throw ExceptionInterruptAndExit(route: RouteNamed.localLoginPassword);
}

idle means Auto Master is not running, not that it completed successfully. This comment misrepresents the semantics of the idle case.

• Fix: // Auto Master either completed (password changed) or was idle (never ran); redirect to login.

---

4. No test coverage for AutoMasterStatus.failed polling path — High confidence [single, Reviewer B]

• Location: test/page/instant_setup/localizations/pnp_admin_view_test.dart, pnp_setup_view_test.dart
• Evidence: grep -rn "AutoMasterStatus.failed" test/ → zero results. Existing tests cover running, idle, complete, and null-3-times connection error. The failed path produces a different observable outcome (continue PnP vs. redirect to login) — safety-critical distinction, untested.
• Fix: Add tests mocking pollAutoMasterStatus() emitting AutoMasterStatus.failed, asserting no navigation to login and normal flow continues; one test per affected view.

---

5. Implicit fall-through from autoMasterFailed to save logic — Med confidence [single, Reviewer B]

• Location: lib/page/instant_setup/pnp_setup_view.dart:723–753
• Evidence:

if (autoMasterFailed) {
  logger.d('[PnP]: Auto Master failed, skipping timeout handling');
  // no return, no setState — intentional fall-through to save
} else {
  // timeout reconnection logic
}
// falls through to save logic at line 771

The true branch is a no-op log followed by silent fall-through. A future developer inserting return here would break the save flow without a compile error.

• Fix: Add comment // intentional fall-through: proceed to save on failed status, or negate: if (!autoMasterFailed) { /* timeout logic */ }.

✅ What looks good

• auto_master_status.dart: enum correctly adds failed with matching toValue() / fromValue() entries; exhaustive switch patterns maintained.
• pnp_provider.dart:773: condition in scheduledCommand correctly includes || status == AutoMasterStatus.failed — polling terminates on Failed as intended. (Reviewer B initially considered this a Critical gap; re-reading head code confirms the fix is present and correct.)
• pnp_admin_view.dart:579–588: failed path correctly sets _isWaitingForAutoMaster = false and returns — consistent with the admin view's exception-based flow structure.
• pnp_setup_view.dart: failed correctly breaks polling, skips timeout reconnection logic, and proceeds to save — preserving the existing recovery strategy without triggering false connection error UI.
• Enum doc comment on line 4 corrected from "idle: Auto Master is not running or failed" to the cleaner "idle: Auto Master is not running".

Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.

[HankYuLinksys]

Re-review — Critical fixed ✅

Verified a86ba0b8, traced the full Failed path. Both spots are correctly handled:

1. Provider condition — pollAutoMasterStatus()'s condition now stops on failed too, so the scheduled poll terminates cleanly instead of running to maxRetry.

2. setup_view second defense — handled more carefully than admin_view, and correctly so: setup_view can't just return (it needs to continue saving), so the fix uses autoMasterFailed = true; break; to exit the loop, then an if (autoMasterFailed) flag to skip the timeout/reconnect block, then falls through to the save logic. I confirmed the fall-through: failed → skip timeout → edge-case check (idle→complete, not matched) → _setupStep = saving. So the UI transitions from the waiting screen straight into saving — no more ~5-minute stall. 👍

Remaining follow-ups (non-blocking)

• FW compatibility (Medium): still open — please confirm with the FW team whether older FW ever reports the "found another Master" case as Idle (which would route to login with the password still admin). Only matters if old FW hits this path.
• Test: a Failed-path test still isn't included. Cheap to add on top of the existing Auto Master test scaffolding; worth a follow-up.

Critical resolved — approving.

[HankYuLinksys]

Critical from the earlier review is correctly fixed in a86ba0b8 — pollAutoMasterStatus condition now terminates on failed, and setup_view's second defense uses a break + flag to skip timeout handling and fall through to the save flow (no more ~5-min stall). LGTM 🚀 FW-compat (old Idle) and a Failed test remain good follow-ups.