Repository navigation
fix(daemon): preserve disabled UI on manual start - #2205
vincentkoc wants to merge 2 commits into
Conversation
Signed-off-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Thank you for the cold-start configuration fix. Current startup has separate loaded-configuration and explicit CLI override paths; review should prove that configured values survive a cold start while explicit flags retain precedence. Please keep the cold/warm and explicit-override cases as the acceptance boundary. This is queued for focused review. |
|
@vincentkoc, thank you for this, and for the unusually honest PR description. Calling out the clang-tidy baseline and the Windows guard failure yourself saved us real time. We've settled the contract question this raised: a bare Two small things before we merge:
Thanks again. This is a careful, well-tested fix. |
|
Thanks again for the disabled-UI fix and the clear validation limits. The earlier rebase request remains recorded; attribution of the Windows guard and the release-note follow-up are maintainer-side tasks. This sweep has not rerun that guard or changed the agreed startup semantics. |
|
Thanks again for the disabled-UI fix. The branch refresh remains the recorded contributor step; Windows-guard attribution and the release-note follow-up remain ours. Sorry for the delay on our part—I’ll follow up soon. |
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
|
@vincentkoc, thank you again for this fix and for the unusually careful validation notes, and sorry for how long it has waited. Your change does exactly what we settled on: a bare |
What does this PR do?
A cold
daemon startnow preserves a savedui_enabled=falsesetting. Previously it unconditionally requested UI configuration, rewrote the preference totrue, and started the HTTP listener.An explicit cold
--portor--openrequest keeps its existing UI-enabling behavior. Repeated bare starts report the disabled UI without incorrectly saying a browser request failed, and the configuration hint points toconfig set ui_enabled true. The configuration docs describe these semantics.Fixes #2114
Validation
The new disabled-UI regression failed against the unchanged base and passed with this fix. All seven real-process readiness cases passed, covering cold and warm starts, saved enabled/disabled settings, explicit UI requests, delayed readiness, and foreign-listener rejection.
scripts/test.sh BUILD_DIR=build/full-testspassed on macOS arm64: 8,022 tests passed, 10 platform skips, plus all prescribed process guards and security/ordering checks.git diff --check, formatting, cppcheck, and the NOLINT suppression check passed.Full lint is not green:
scripts/lint.shexited 2 with 6,972 repo-wide clang-tidy diagnostics. An exact-path VFS comparison of unchanged and candidatesrc/main.cproduced 125 diagnostics each with the same diagnostic categories; none point to changed lines. The existingmain_run_daemon_ctlcognitive-complexity diagnostic increases from 82 to 89, already above the threshold of 25. Tools: Apple clang-format 21, clang-tidy 22.1.8, cppcheck 2.21.0.Hosted CI at
e0a6133293556d87de6c2ca43de674e3a643e339is not green: Windows guards failedsection_cold_stormwhen client 0 reportedsecure CLI coordination could not be created (endpoint). The same failure was observed on sibling headc1dd44151fee8cfd815c693392a7c1c72def98fd, which shares base339b3f4097aa6ede22fc382ab7fd320d93c498b8. The guard runs six concurrentcli list_projectscalls; the error occurs in local CLI endpoint creation, before the daemon-control UI path changed here. This comparison does not replace an exact-head Windows pass. A maintainer needs to rerun/investigate that guard; no assertion has been weakened.Checklist
git commit -s).AI-assisted implementation and follow-up; submitted on behalf of @vincentkoc.