Revert "Fix starting racing condition"

[mcandeia]

Reverts #1107

---

Summary by cubic

Reverts the previous “starting race condition” fix and restores lazy dependency initialization. Removes readiness hooks and simplifies startup behavior for the daemon and sandbox tasks.

• Refactors
  • createDeps now returns only middleware and starts deps on first request.
  • Removed gitReady and ensureStarted from createSiteApp.
  • Sandbox AI tasks call ensureGit directly before starting.

^{Written for commit e5553e0. Summary will update on new commits.}

[github-actions]

Tagging Options

Should a new tag be published when this PR is merged?

• 👍 for Patch 1.176.1 update
• 🎉 for Minor 1.177.0 update
• 🚀 for Major 2.0.0 update

[coderabbitai]

📝 Walkthrough

Walkthrough

Simplifies startup and readiness lifecycle management by removing public ready/ensureStarted properties from createDeps, eliminating separate ready promise machinery, and refactoring initialization to use direct ensureGit calls with lazy middleware startup instead of eager readiness callbacks.

Changes

Cohort / File(s)	Summary
Readiness Lifecycle Simplification daemon/main.ts	Removed public readiness properties and callbacks from SiteAppResult; eliminated ready promise/resolve/reject machinery. Middleware now returns async function with lazy start() initialization (via ok ||= start()). Replaced currentSite.ensureStarted() and gitReady.then(...) patterns with direct ensureGit() calls. Updated return value to { app, dispose } only.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Ready states were once so grand,
With promises we'd softly strand,
But now we leap without delay,
Lazy initialization saves the day!
Simpler flows, no hooks to keep—
One less thing making our dev sleep. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the changeset as a revert of a previous fix for a racing condition, matching the core objective of this pull request.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch revert-1107-fix/racing-condition

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 1 file

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

daemon/main.ts (1)

266-346: ⚠️ Potential issue | 🟠 Major

Don’t cache a failed startup forever.

Line 339 stores the first start() promise in ok, including the rejected case. Since ensureGit() has transient failure paths in daemon/git.ts:586-679 (missing git binary, config/clone/submodule failures), one bad first request will poison ok and every later request will keep returning 424 until the process restarts. Reset the cache on rejection so a later request can retry startup.

Suggested fix

-  let ok: Promise<unknown> | null | false = null;
+  let ok: Promise<void> | null = null;
...
   return async (c, next) => {
     try {
-      ok ||= start();
-
-      await ok.then(next);
+      if (!ok) {
+        ok = start().catch((err) => {
+          ok = null;
+          throw err;
+        });
+      }
+
+      await ok;
+      await next();
     } catch (err) {
       console.log(err);

       c.res = new Response("Error while starting global deps", { status: 424 });
     }
   };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@daemon/main.ts` around lines 266 - 346, The code caches the initial start()
promise in ok and never clears it on rejection, so a failed start poisons future
requests; modify the logic where ok is assigned (the ok ||= start() / start
promise creation) to attach a rejection handler that resets ok to null on error
(reference the ok variable and the start() function) so that a rejected start()
does ok = null and rethrows the error, ensuring subsequent requests can retry
startup (keep the await ok.then(next) usage unchanged).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@daemon/main.ts`:
- Around line 266-346: The code caches the initial start() promise in ok and
never clears it on rejection, so a failed start poisons future requests; modify
the logic where ok is assigned (the ok ||= start() / start promise creation) to
attach a rejection handler that resets ok to null on error (reference the ok
variable and the start() function) so that a rejected start() does ok = null and
rethrows the error, ensuring subsequent requests can retry startup (keep the
await ok.then(next) usage unchanged).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 980a6597-6016-4dd7-826e-315c485dd34c

📥 Commits

Reviewing files that changed from the base of the PR and between 27183e2 and e5553e0.

📒 Files selected for processing (1)

• daemon/main.ts