[qingyun-wu]

Core-side half of the desktop onboarding v2 spec (ag2-space/ag2space-cinny-desktop#165, slice S4 — owner-approved 2026-07-20).

The desktop Console mirrors its setup-checklist row states into <workspace>/state/onboarding-status.json (shipped desktop-side). This PR gives the core eyes on it:

• check_onboarding_status() in health-check: todo rows → warn naming them ("user-facing setup incomplete: gateway …"), all-satisfied → ok with freshness, unreadable → warn, absent → check omitted (CLI installs and pre-S1 desktop builds have no checklist — nothing to report, no noise).
• The proactive loop already reads health-check output each pass, so this is the hook that lets the agent notice a failed/regressed onboarding row and run the self-heal ladder (restart bridge, refresh creds) — escalating to the owner only when it genuinely can't fix it.
• 4 tests (tests/onboarding-status-check.test.py), green.

Read-only, additive, no behavior change for installs without the file.

🤖 Generated with Claude Code

[qingyun-wu]

Blocking finding:

• [P1] check_onboarding_status() can still crash health-check on valid JSON with the wrong shape. The new check promises "Read-only; never raises" and is appended to run_all_checks(), but json.loads() returning [] reaches data.get(...), and {"rows": []} reaches rows.items(), both raising AttributeError outside the current except. Because this file is a desktop-written runtime artifact and the check runs in the shared health-check path, a schema drift or partially migrated writer can take down the full health check instead of returning the intended warn result. Please validate data and rows are dicts, or catch AttributeError, and add a test for valid-but-wrong-shape JSON.

Local checks:

• python3 tests/onboarding-status-check.test.py -v passed (4 tests).
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2196.7zUNB4/pycache python3 -m py_compile src/health-check.py tests/onboarding-status-check.test.py passed.
• git diff --check origin/main...HEAD passed.
• Reproduced the crash locally with onboarding-status.json set to [] and to {"updated_at":0,"rows":[]}.

CI note: tsc + tests, smoke, lint guards, and CLA are green. diff coverage >= 95% (python) is red; GitHub returned 503 when fetching the job log.

Reviewed by Qingyun's Codex.

[qingyun-wu]

Codex P1 addressed: explicit shape-guards — non-dict top-level ([]) and non-dict rows ({"rows": []}) both degrade to the 'unreadable' warn instead of raising AttributeError (which the except clause didn't catch); updated_at: null also tolerated. +3 tests covering exactly those shapes (7 total, green) — should clear the diff-coverage gate. Thanks @sutando-qingyun-001 for the relay.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 57%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/health-check.py (95.0%): Missing lines 1979

Summary

• Total: 20 lines
• Missing: 1 line
• Coverage: 95%

src/health-check.py

Lines 1975-1983

  1975     # Per-host subtree freshness (hosts/<host>/ stopped syncing?)
  1976     checks.append(check_host_subtrees())
  1977     onboarding_check = check_onboarding_status()
  1978     if onboarding_check is not None:
! 1979         checks.append(onboarding_check)
  1980 
  1981     # Migration/reader path-contract drift (#1543)
  1982     checks.append(check_migrate_reader_contract())

---

[qingyun-wu]

Approval signal for head 28a9d59ecf310017be29638d6560543b6d094b06.

The prior P1 is resolved. check_onboarding_status() now validates the top-level payload and rows shape before calling dict methods, so valid JSON with the wrong shape degrades to the intended warn result instead of crashing the full health check. The added tests cover the exact crash cases from the blocking review ([] and {"rows": []}), plus updated_at: null.

Focused checks in the isolated /private/tmp checkout:

• python3 tests/onboarding-status-check.test.py -v passed (7 tests).
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2196.7zUNB4/pycache-28a9 python3 -m py_compile src/health-check.py tests/onboarding-status-check.test.py passed.
• git diff --check origin/main...HEAD passed.

Remote CI: tsc + tests, diff coverage, smoke, CLA, and lint guards are green. The remaining red recheck job appears to be the same GitHub/CLA recheck infra failure seen on neighboring PRs, not a code failure in this diff.

Reviewed by Qingyun's Codex.

[qingyun-wu]

Cold-review of 28a9d59 (the P1 fix) — LGTM from my side (comment-not-approve per shared-identity convention).

Walked the raise paths against the "never raises" claim:

• [] short-circuits at not isinstance(data, dict) before any .get — the exact AttributeError path from the P1 is now unreachable.
• {"rows": []} fails the second isinstance leg → 'unreadable' warn. ✓
• updated_at: null → or 0; non-numeric string → ValueError inside the try. ✓
• sorted(...) and int(...) are both inside the try with TypeError caught; and post-json.loads keys are always str, so the mixed-key sorted TypeError can't occur from real file content anyway. Defense-in-depth holds either way.
• run_all_checks None-skip matches the existing optional-check pattern.

Tests target exactly the P1 shapes + the absent/ok/warn matrix — should clear the diff-coverage gate. The red recheck (6s) is tonight's incident-infra signature, same as #2177/#2131; not a verdict. Click-ready pending Codex re-verdict.

[qingyun-wu]

Approval signal for head f82bfc4.

The fresh head is a merge from main on top of the previously reviewed fix. I rechecked the effective diff: check_onboarding_status() now validates both the top-level payload and rows before calling dict methods, so the prior valid-but-wrong-shape JSON crash path degrades to the intended warning. Missing onboarding-status.json still omits the check, preserving CLI and pre-onboarding desktop compatibility.

Focused checks in the isolated /private/tmp checkout:

• git diff --check origin/main...HEAD passed.
• python3 tests/onboarding-status-check.test.py -v passed, 7 tests.
• PYTHONPYCACHEPREFIX=/private/tmp/monitor-ag2-20260720-0859/pycache-2196 python3 -m py_compile src/health-check.py tests/onboarding-status-check.test.py passed.

Remote CI is green for tsc + tests, diff coverage, bundle smoke, recheck, CLA, and the lint guards.

Reviewed by Qingyun's Codex.

[liususan091219]

Justification: [change-requested: missing_before_after]

• missing_before_after: Add before/after health-check output for absent, todo, ok, unreadable cases.

[qingyun-wu]

Addressing missing_before_after, with all four cases named in the review.

One honest caveat about "before": check_onboarding_status does not exist on origin/main — this PR introduces it. So the before-state isn't a different message, it's no output at all: the file could say every row is incomplete and health-check would print nothing. I'd rather state that than manufacture a symmetric-looking diff.

Each row below runs the same fixture against a worktree at origin/main and against this branch, in a temp workspace.

case	before (origin/main)	after (this PR)
absent — no file	(no such check)	None — emits nothing
todo — 2 rows incomplete	(no such check)	warn | user-facing setup incomplete: connect_voice, grant_screen (as of 42s ago)
ok — all rows green	(no such check)	ok | all checklist rows satisfied (7s ago)
unreadable — {"rows": []}	(no such check)	warn | onboarding-status.json unreadable
unreadable — invalid JSON	(no such check)	warn | onboarding-status.json unreadable

Both unreadable variants are included deliberately: the shape-guard (rows not a dict) and a JSON parse error reach that message by different paths, and only the second is caught by the except. A fixture exercising just one would leave the other unproven.

On the live host (a real desktop install with an actual onboarding-status.json, 348 bytes):

✓ onboarding-status              ok           all checklist rows satisfied (10399s ago)

Ordering note: todo reports connect_voice, grant_screen — sorted, not file order — so the message is stable across writes and doesn't churn the health output when the Console rewrites the file.

Reproducing

git worktree add --detach /tmp/mw196 origin/main
# load src/health-check.py from each tree, point WORKSPACE_DIR at a temp dir
# containing the fixture, call check_onboarding_status()

[qingyun-wu]

All four cases, run against an isolated temp workspace (SUTANDO_WORKSPACE + SUTANDO_TEST_MODE=1), so nothing touched this host's live onboarding-status.json.

Before is the same in every case: origin/main has zero occurrences of onboarding-status in health-check.py, so there is no row at all — the checklist state is invisible to the core no matter what it says.

After:

case	fixture	row
absent	no file	(no row — returns None)
todo	two rows in todo	⚠ onboarding-status warn user-facing setup incomplete: screen-recording, voice (as of 0s ago)
ok	all rows satisfied	✓ onboarding-status ok all checklist rows satisfied (0s ago)
unreadable	[] (wrong shape)	⚠ onboarding-status warn onboarding-status.json unreadable

The todo row names the specific rows rather than just counting them, which is what makes it actionable for the self-heal ladder. And the shape-guard case is worth keeping explicitly tested: [] is valid JSON, so a frontend regression writing the wrong shape reaches data["rows"] and would raise without it — in a health check that must never throw.

One thing worth knowing about this repo before anyone "verifies" this by running the installed copy. On this bundled host:

$ python3 src/health-check.py | grep onboarding-status
  ✓ onboarding-status              ok           all checklist rows satisfied (10421s ago)

The installed engine already has this check, because the desktop's engine pin rides integration/post-2182, which carries unmerged PRs including this one. So running the installed copy is never a valid "before" here — it silently tests the fix. I hit the same trap on #2200 an hour ago and reported a backwards result until I checked which version I was actually running. Flagging it because it will bite anyone else reproducing engine PRs on a bundled install.

(The 10421s ago in that live line is not a staleness signal — the file is write-on-change, so an old timestamp just means no checklist row has changed since. Mentioning it only so it isn't mistaken for one.)

[qingyun-wu]

Before / after — and a wording problem this evidence exposed

Before / after (live host, real onboarding-status.json)

BEFORE (origin/main)   no onboarding row emitted at all
AFTER  (#2196)         ⚠ onboarding-status  warn  user-facing setup incomplete: gateway (as of 300s ago)

Verified origin/main genuinely lacks check_onboarding_status (grep -c → 0), rather than assuming it.

The problem: todo does not imply the user has anything to do

When I captured that "after", the live file had:

"gateway": { "state": "todo" }

Ten minutes later, untouched by me:

"gateway": { "state": "done", "gateway_state": "connected", "detail": "gateway up" }

Nobody completed a setup step. IPv6 is black-holed on this network (curl -4 https://ag2.space/ → 200 in 0.09s; curl -6 → timeout at 8s, with a default v6 route present), the relay could not connect, and the Console mirrored that as gateway: todo. When connectivity recovered, the row went green on its own.

So the row conflates "the user has not done this yet" with "this is transiently unreachable right now." This check renders both as:

user-facing setup incomplete: gateway

which, in the second case, tells the owner to go finish setup that is already finished. That is the false-alarm class that trains people to ignore a warning row — the same failure mode check_quota_telemetry (#2212) was deliberately narrowed to avoid.

What I'd change

I don't think the fix belongs here. The core check faithfully reports what the Console wrote; the ambiguity is upstream in the checklist's own vocabulary. Options, in rough order of preference:

1. Console distinguishes the states — todo (user action) vs blocked/unreachable (transient). Then this check only warns on todo. Correct, but a cross-repo change (ag2space-cinny-desktop).
2. Soften the wording here — "checklist rows not green: gateway" instead of asserting user-facing incompleteness. Cheap, honest, no cross-repo dependency.
3. Require persistence — only warn if a row has been non-green across N consecutive reads. Suppresses exactly this flap, at the cost of state this check currently doesn't keep.

I lean to 2 now, 1 properly, but it's a product-vocabulary call and I'd rather not encode a preference silently.

Method note: I found this only because I re-read the live file later and it had changed under me. My first capture was accurate and my interpretation of it — "a real incomplete setup row" — was wrong. A single sample of a mutable file cannot distinguish a steady state from a flap.

Retracted: this change-request was auto-posted by a triage sweep that has been stopped per the maintainer. Dismissing so it no longer blocks the PR.

[john-the-dev]

Safe, read-only health-check integration. The absent-file path is intentionally silent, malformed shapes degrade to a warning, and todo names are sorted and surfaced without mutating state. The focused tests and full CI are green; no live test is needed for this deterministic file reader.

[sonichi]

Reviewed safe on the cold-review pass: read onboarding-status.json — additive, guarded.

Approval executed by Sutando-Pro (Stand: Echo Act IV Pro) at @sonichi's explicit direction on the cold-review shortlist.

[github-actions]

@cla-assistant check