PR #536 — Wiring HEALTH_CHECKS Into the Real synlynk doctor CLI Path
The Broader Goal at the End of the Previous PR
PR #517 added two new entries to HEALTH_CHECKS — checks for un-provisioned identity roles and for identity-file permission/gitignore drift — as part of shipping per-role GitHub App identity. Its Security Review noted, without fixing, that these checks were unreachable in practice: synlynk doctor run from the actual CLI (cli.py:1071's bare cmd_doctor() call, with no checks argument) never touched HEALTH_CHECKS at all. It only ran the interactive TC1-5 agent-testing wizard. Filed as #525.
Strategic Shifts in This PR
The investigation here turned up more nuance than a simple oversight. cmd_doctor(args=None, checks: _List = None) already had two intentional, separately-tested code paths: an explicit checks=[...] call (used by tests at tests/test_synlynk.py:6570-6588) that runs only the given checks, and the real CLI's checks=None default (tested at tests/test_synlynk.py:188,617,677) that ran only the TC1-5 wizard. Neither path was broken on its own terms — they just never overlapped, and the CLI default was the one missing HEALTH_CHECKS entirely. Rather than guess at a fix, this was scoped via AskUserQuestion before dispatch: the chosen design runs both in sequence on the checks=None path — print the HEALTH_CHECKS report first, then fall through unchanged into TC1-5 — with no new flags or subcommands added.
What This PR Shipped
_print_health_check_report(checks) extracted as its own function in synlynk/doctor.py, formatting and printing the pass/warn/fail report (with fix hints) for a given list of checks, returning whether any failed.
cmd_doctor(args=None, checks=None) behavior change, scoped precisely:
checks=[...](explicit): unchanged —return 1 if _print_health_check_report(checks) else 0, exactly as before.checks=None(the real CLI default): now calls_print_health_check_report(HEALTH_CHECKS)first, then falls through — unchanged — into the existing TC1-5 wizard logic (agent filter, DB connection, TC1-5 test runs).
Verification against spec. The dispatched diff (job-a6115feb, story-abbccee7) matched the design decision exactly on first read — no fix needed before integration, unlike its sibling fix in #524/PR #535. New test test_wire_health_checks_into_real_synlynk_doc (in tests/test_agent_quota_tracking.py — not the most obvious file for a doctor-related test, but functionally correct and not worth blocking on) monkeypatches HEALTH_CHECKS and the TC1-5 runner functions, calls sl.cmd_doctor() with no arguments, and asserts the health-check output ("identity_roles" in out) and the TC1-5 wizard output ("doctor [agy]" in out) both appear.
Unrelated upstream churn during integration. Between dispatch and integration, origin/main advanced past the jobs' original base (cc9998c) to include an unrelated, already-merged fix for issue #526 (6b57600, "synlynk jobs --all permanently shows 'unkn'"), which happened to touch the exact same region of tests/test_agent_quota_tracking.py. Cherry-picking the #525 fix commit onto a fresh integration branch produced a genuine but unrelated merge conflict — resolved by keeping both new tests side by side, since neither touched the other's logic. Full suite after resolution: pytest tests/ -q → 1410 passed, 2 skipped.
Brainstorm Visuals Used
None — the design question (run both in sequence vs. add a flag) was resolved directly with Nikhil via a scoped multiple-choice question rather than a full brainstorm session.
What This Achieved on the Path to Autonomy
synlynk doctor now actually surfaces all 10 registered health checks — including the two identity-related ones added in #517 — to anyone running the CLI normally, not just to tests that pass checks= explicitly. Combined with #524/PR #535, this closes out every follow-up #517 left open: the per-role GitHub identity feature is now fully diagnosable and its redaction guarantee holds across process boundaries.
Strategic Note: The Goal at the End of This PR
With #517, #524, and #525 all closed, the identity/dispatch-integrity arc that started with issue #423 is complete. No new follow-ups were identified during this work; the next thread of work is whatever's next on the roadmap rather than a continuation of this arc.