Skip to content

fix(daemon): retire only the captured failed startup - #3129

Closed
thymikee wants to merge 6 commits into
fix/private-replay-retirementfrom
fix/daemon-client-startup
Closed

thymikee wants to merge 6 commits into
fix/private-replay-retirementfrom
fix/daemon-client-startup

Conversation

@thymikee

@thymikee thymikee commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Summary

Failed startup cleanup now acts on the PID and process birth captured from that launch. It delegates stopping, exit confirmation, registration matching and lock release to the shared retirement operation. A spawn failure without a captured child performs abandoned recovery without signaling a discovered daemon.

Unknown state is retained with typed outcomes. The former global metadata cleanup helper is removed, together with tests that required deleting malformed state or reconstructed its ownership checks. Registration readers and retirement now own those checks.

Six files changed; 466 gross lines. Builds on #3127; part of #3116. Startup contention/deadline handling and timeout retirement follow in dependent layers.

Validation

Tested 359e453514:

  • The prior head 98cc7666f9 passed 89 focused tests with zero skips; the rebased head passed all related tests. The pre-fix client removes the malformed metadata and fails the new retention assertion.
  • Allowing the shared owner to delete corrupt metadata fails the surviving root ownership test. Deleted helper coverage remains at that stronger boundary.
  • pnpm check:quick passed.
  • AGENT_DEVICE_REQUIRE_LOOPBACK_TESTS=1 pnpm check:affected --base fix/private-replay-retirement --run passed: 138 related files, 918 tests; all runnable checks completed.
  • Independent review found no remaining findings in this slice. GitHub owns full coverage and native lanes.

@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.95 MB 4.95 MB -528 B
Package (unpacked) 4.95 MB 4.95 MB -528 B
Package (download) 1.49 MB 1.49 MB -171 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 30.6 ms 29.0 ms -1.6 ms
CLI --help 90.4 ms 86.5 ms -3.8 ms

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

Re-trigger cubic

@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Thanks for the PR. At 359e453 the startup timeout path kills a daemon that is still booting, and I do not think this can merge as is. CI is likely related, because the failing iOS Smoke step is a cold daemon start through the function this PR rewrites. I am not aware of any conflicts.

On a cold start, startLocalDaemon now uses one total deadline of 15s. When it passes, stopAndRetireDaemon stops the captured child, and only early_exit retries (L375). The base gave a cold start about 30s. It left the first child alive on timeout, started attempt 2 with a fresh 15s wait, and adopted the first child once it published daemon.json. It also waited another 15s when a live info or lock holder existed. A daemon that needs 15-30s to publish now fails every client command with daemon_startup_failed. The iOS Smoke preflight after clean:daemon shows this: one startup attempt, the captured pid retired gracefully, no info and no lock, an empty log tail, and failure 15s after the child started. #3131 is stacked on this branch and shows the same signature. The rule to satisfy: retire the captured child on timeout only after the full startup budget is spent. That budget must size every wait in startLocalDaemon and be at least the base's effective budget, DAEMON_STARTUP_ATTEMPTS x DAEMON_STARTUP_TIMEOUT_MS, or longer while the captured child is alive and owns the lock. Can the deadline be that budget, or can the code keep waiting on the same launch while the captured child is alive?

The PR deletes cleanupFailedDaemonStartupMetadata retains live startup daemon on timeout in daemon-client.test.ts and adds no sendToDaemon test for the new timeout branch. Two behaviors now have no coverage: a captured child that publishes after the per-wait timeout but inside the budget is adopted, and the child is stopped only after the budget is spent. Please add a sendToDaemon test with a fake launch that publishes a matching daemon.json after more than 15s but inside the budget, and assert it returns startedByClient: true. Add a second test where the child never publishes, and assert the retired cleanup result and that only the captured pid was signaled. The first test must fail on 359e453.

I did not rerun the focused tests or check:affected from the PR body. I also cannot say whether the CI daemon would have published within the base's 30s budget, since its log was empty and I had no base run on the same runner. Before merge, please restore the full startup budget, add the slow-publish test, and show a green iOS Smoke Preflight iOS runner through public CLI on the fixed head. That run should cold start after clean:daemon, reach daemon readiness, and have prepare ios-runner return success:true.

@thymikee
thymikee added this pull request to stack #3146 October 3, 2026 08:46
@thymikee
thymikee force-pushed the fix/private-replay-retirement branch from 681b785 to 3bc70bf Compare October 3, 2026 14:40
@thymikee
thymikee force-pushed the fix/daemon-client-startup branch 3 times, most recently from c4f6ab8 to 34c1f64 Compare October 3, 2026 17:49
@thymikee
thymikee force-pushed the fix/private-replay-retirement branch from 8acd13c to 0effe16 Compare October 3, 2026 17:49
@thymikee
thymikee removed this pull request from stack #3146 October 3, 2026 19:38
@thymikee
thymikee force-pushed the fix/daemon-client-startup branch from 34c1f64 to 3e5be9a Compare October 3, 2026 19:39
@thymikee
thymikee force-pushed the fix/private-replay-retirement branch from 0effe16 to 2a359db Compare October 3, 2026 19:39
@thymikee
thymikee added this pull request to stack #3187 October 3, 2026 19:45
@thymikee
thymikee removed this pull request from stack #3187 October 3, 2026 20:58
@thymikee
thymikee force-pushed the fix/private-replay-retirement branch from 2a359db to c188d9c Compare October 3, 2026 21:00
@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

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.

@thymikee thymikee closed this Oct 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant