Repository navigation
feat(daemon): quiesce request that retires an idle daemon atomically - #3764
Conversation
Size Report
Startup median (7 runs, lower is better):
|
There was a problem hiding this comment.
All reported issues were addressed across 8 files
You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
5228ab7 to
a972b47
Compare
|
The code needs one fix before merge: a quiesce that arrives during daemon startup can crash the daemon. Reviewed commit a972b47. All 17 checks pass. I did not run the tests. I found this by reading the code, not by reproducing it. In daemon-runtime.ts, Not blocking, take or leave: the ADR at docs/adr/0006-daemon-rpc-protocol-version.md line 104 says a null start time is rejected, but the code accepts it when the daemon cannot read its own, so either reject null or state that the token is the instance fence and the start time is checked only when known; and Could the runtime own one admission state, On the open review threads: the drain/TDZ thread is marked resolved but still applies, and it is the finding above. The null start time thread and the hardcoded reason thread still apply and match the non-blocking notes. Three lower-priority threads also still apply, and I found none that should be resolved as stale. No conflicts. Live device validation is not needed here, since the change is daemon lifecycle only and the tests boot the real runtime. Before merge, make |
d80f096 to
415e080
Compare
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
1bacb95 to
edd4335
Compare
|
Thanks, you were right: the first TDZ fix only moved the throw. Fixed in edd4335.
|
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
You’re at about 95% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
edd4335 to
364af8c
Compare
|
The PR is ready. I reviewed 364af8c, and the startup-window crash from the earlier review is fixed and covered by a runtime regression test. CI is green: 17 of 17 checks pass at 364af8c, with no failing job. There are no conflicts. Nothing else must happen before a maintainer merges. Not blocking, take or leave: at startup the census reports I did not run the new runtime test or the eager-closure-budget test. I judged the regression coverage from the diff. These earlier inline threads are fixed at this head, so you can resolve them: startup counted in the census (#3764 (comment)), refusal when the start time is null (#3764 (comment)), in-flight test cleanup (#3764 (comment)), reworded doc comment (#3764 (comment)), bounded daemon.json poll (#3764 (comment)), and the corrected test comment (#3764 (comment)). The literal in session-store.ts is intentional, because the lifetime test asserts it equals DAEMON_SHUTTING_DOWN_REASON, so this thread can be resolved too (#3764 (comment)). |
A daemon now answers an authenticated, instance-fenced daemon_quiesce request: it closes admission and counts requests in flight, sessions, repair sessions and live leases in one synchronous step, refuses with daemon-busy (reopening admission) if it holds any, and otherwise drains and exits. It publishes capabilities in daemon.json so a client can tell a daemon that can quiesce from one that predates it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
364af8c to
ad93822
Compare
|
Thanks. Both non-blocking notes are taken in ad93822:
|
|
The delta at ad93822 fixes what the earlier review at 364af8c left open. The close-count-reopen step stays atomic, the daemon still refuses while it starts up, and the startingUp: true / inFlightRequests: 0 assertion would fail on 364af8c, so the new test guards the fix. I did not run the tests; I read the diff. All 17 checks pass at ad93822, and there are no conflicts. Not blocking, and you can take or leave it: the comment at src/daemon-client/daemon-client-metadata.ts#L55 says the client reads On the open threads: the cubic-dev-ai thread about the single-source reason string in session-store.ts (#3764 (comment)) still applies as a note, not a defect, since the test pins it to DAEMON_SHUTTING_DOWN_REASON. These threads no longer apply, so please resolve them: startup refusal before Nothing else stands between this PR and merge. |
|
…ment * origin/main: feat(cli): ensure the daemon explicitly before attaching in the CLI and MCP (#3766) feat(client): daemonLifecycle attach mode and ensureDaemon() (#3765) feat(daemon): quiesce request that retires an idle daemon atomically (#3764) fix(daemon): answer a named missing session on the session, not the device (#3701) test: route test/ and scripts/ off the daemon's testing exports (#3733) refactor(plugins): make the plugin framework provider-independent (#3722) fix(apple): name an Xcode too old for its host macOS as a runner startup failure (#3739) # Conflicts: # packages/daemon/src/server/http/daemon-runtime.ts # packages/daemon/src/sessions/handlers/session-appstate-device-runtime.ts # packages/daemon/test/nightly/concurrency-torture/real-scope-serialization.ts # src/__tests__/daemon-registration-owner.test.ts # src/agent-device-client.ts # src/cli.ts # src/cli/commands/proxy.ts # src/daemon-client/daemon-client-lifecycle.ts # src/daemon-client/daemon-client-metadata.ts # src/daemon-client/daemon-client.ts # test/integration/provider-scenarios/harness.ts
…refactor/daemon-runtime-no-process-globals * origin/refactor/host-environment: refactor(daemon): read configuration from the request host instead of the ambient scope test: merge the duplicate test-kit import in the stale provider admission scenarios feat(cli): ensure the daemon explicitly before attaching in the CLI and MCP (#3766) feat(client): daemonLifecycle attach mode and ensureDaemon() (#3765) feat(daemon): quiesce request that retires an idle daemon atomically (#3764) fix(daemon): answer a named missing session on the session, not the device (#3701) test: route test/ and scripts/ off the daemon's testing exports (#3733) refactor(plugins): make the plugin framework provider-independent (#3722) fix(apple): name an Xcode too old for its host macOS as a runner startup failure (#3739) # Conflicts: # packages/daemon/src/server/http/daemon-runtime.ts
Summary
Layer 1 of 3 for #3754 (umbrella #3747): the daemon half of replacing an incompatible daemon safely.
daemon_quiesce, authenticated with its token and fenced to its pid and start time. It closes admission and counts in-flight requests, sessions, repair sessions and live leases in one synchronous step. Any of them refuses withdaemon-busyand reopens admission; otherwise it answersdrainingand exits.capabilities: ['quiesce']indaemon.json; a daemon that predates it omits the field.daemon-resolution.ts, and the daemon side indaemon-idle-reap.ts, because both are already in the eager closures. ADR 0006 gains a short section, including what the census does not count (uploads, lease-less holds).No client behavior changes here. 10 files. The last PR in the stack closes #3754.
Validation
pnpm check:affected --run --head ad938223f9: passed (format, lint, typecheck, layering, fallow, build, vitest-related). New runtime tests boot the real daemon and quiesce it over its socket and HTTP: idle drain, busy refusal per cause with admission reopened, wrong token, pid and start-time fences, remote-origin refusal.🤖 Generated with Claude Code