Conversation
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
This PR is ready at ba4a5c2. The shared retirement path and the mixed-version cutover look correct from reading the code. I did not run the tests or your mutation controls. Not blocking, and you can take or leave these: (1) In daemon-stop.ts, a graceful stop can race with another client starting a successor. The retirement then returns 'lock-busy' or 'registration-replaced' and The legacy check rebuilds the old lock algorithm from source. It does not run a released old daemon binary, so old servers, takeover replies and shutdown teardown are not exercised. This PR is stacked on #3131, so the shared owner module is reviewed there. Smoke, Repo Guards, Coverage, Integration and cubic are still running, so there is no failure to attribute yet. Integration and Coverage run both changed test files, and they must pass on ba4a5c2 before merge. There are no conflicts. This review covers ba4a5c2. The newer head 2b3dcfa changes only the ordering in one cutover test in src/tests/daemon-registration-owner.test.ts, so it does not change the result above. |
|
This PR is ready at 2b3dcfa. All 14 checks pass, and the changed tests run in Integration/Coverage and are green. There are no conflicts. I read the code and did not run the cutover tests or the mutation controls. The legacy side rebuilds main's lock algorithm from source, so it does not run a released 0.21.x daemon binary. The real old shutdown and the full legacy info shape are not exercised. I also did not reproduce the successor-race timing below, since it comes from reading the code. Not blocking, and you can take or leave these. (1) After a graceful |
2afb5ec to
4fd7029
Compare
2b3dcfa to
febd631
Compare
4fd7029 to
814fb0c
Compare
febd631 to
31107eb
Compare
814fb0c to
bcce61b
Compare
31107eb to
91f9071
Compare
bcce61b to
50eafe5
Compare
91f9071 to
12781ae
Compare
|
Consolidated into #3127 as part of reducing #3116 to seven PRs. The composition preserves the complete pre-consolidation source tree, including tests and later review corrections. This PR is superseded; its review discussion and native evidence remain available. Outstanding findings transfer to the owning keeper in the implementation record. |
|
Summary
Manual daemon stop now awaits the shared ownership-checked retirement operation. Confirmed process exit alone cannot report success if registration retirement failed; primary errors retain typed reasons and normalized diagnostics. Existing CLI stop/report shapes are preserved.
Verifies mixed-version daemon exclusion at the actual shared lock path with owned child processes running the previous acquisition algorithm: an already-running old daemon, a new owner delaying metadata, and simultaneous startup. This proof does not expand host-kit's mixed-protocol support policy.
Depends on #3131. Ref #3116. Six files, 483 gross changed lines. ADR 0030 includes the ownership diagram and support boundary.
Validation
Tested
2b3dcfaae5c0422fcb36ea0fece9204bf87bfea4:pnpm check:quick, Fallow, andpnpm check:affected --base fix/daemon-timeout-retirement --runpass; 34 files/385 related tests.